refactor(missingMedia): adopt file_path as primary detection key (FE-746)#12398
Open
dante01yoon wants to merge 9 commits into
Open
refactor(missingMedia): adopt file_path as primary detection key (FE-746)#12398dante01yoon wants to merge 9 commits into
dante01yoon wants to merge 9 commits into
Conversation
…746) - Add `file_path` to AssetItem schema (nullable per BE-933/BE-934 wire shape) - Rewrite `getAssetDetectionNames`: use `file_path` alone when emitted, fall back to the legacy `asset_hash` / `name` / `subfolder + name` union when null (hash-only Core registrations, tagless Cloud rows, legacy data) - Drop `isCloud` parameter from `scanAllMediaCandidates`, `scanNodeMediaCandidates`, and `MediaVerificationOptions` — both backends consult the same asset listing oracle post-BE-933/934 - Remove the OSS synchronous-shortcut branch; all candidates now resolve asynchronously through `verifyMediaCandidates` against the unified listing - Rename `MediaVerificationOptions.isCloud` to `allowCompactSuffix` (N1-deferred annotation grammar flag, no longer a backend identity field) - Always call `assetService.getInputAssetsIncludingPublic`; retain internal `isCloud` only for the generated-assets oracle (Cloud /api/assets vs OSS history) — unifying that path is tracked separately - Update callers in app.ts, useErrorClearingHooks.ts, markDeletedAssetsAsMissingMedia.ts - Update tests with module-level `isCloud` mock for generated-asset oracle control; add `file_path`-primary matching test case Per RFC BE-808 v2 (Asset Identity Semantics). DO NOT MERGE until BE-933 (Comfy-Org/ComfyUI#14005) and BE-934 (Comfy-Org/cloud#3744) merge and ship — both backends must emit `file_path` for the new path to fire. Fixes FE-746
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughScans no longer pass cloud-mode and emit candidates with undefined isMissing. Verification now accepts allowCompactSuffix to control compact-suffix normalization. Asset schema adds optional file_path. Resolver imports isCloud at module level and selects cloud/history behavior internally instead of via options. All tests switch to a runtime isCloudHolder mock pattern. ChangesMissing Media Pipeline Refactor
Estimated code review effort🎯 4 (Complex) | ⏱️ ~70 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 6 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (6 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Codecov Report❌ Patch coverage is
@@ Coverage Diff @@
## main #12398 +/- ##
===========================================
- Coverage 75.99% 62.97% -13.02%
===========================================
Files 1573 1462 -111
Lines 89008 74584 -14424
Branches 27397 19357 -8040
===========================================
- Hits 67640 46972 -20668
- Misses 20710 27267 +6557
+ Partials 658 345 -313
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 1126 files with indirect coverage changes 🚀 New features to boost your workflow:
|
…ts-to-unified-file_path-based
…ase B) `getAssetDetectionNames` previously short-circuited on `asset.file_path`, returning only that key when emitted. That premise — `file_path` as the primary match key — doesn't match RFC BE-808 v2: §3 names `id` as the identity field and §4 explicitly calls `file_path` "a standalone locator/display string" emitted on a BEST EFFORT basis. Workflow widget values predate the BE-933 / BE-934 `file_path` rollout and may still be bare filenames, hashes, or annotated paths. A pre-BE workflow paired with a post-BE asset would false-negative under the early-return: the asset emits `file_path: "input/sub/photo.png"` while the widget value is still `"photo.png"`, and the previous code would mark the reference missing even though the file is present. Union `file_path`, `asset_hash`, `name`, and `subfolder + name` variants so workflows authored against any historical asset shape keep matching. Both backends round-trip `name` through the BE-792 deprecation window, so the legacy keys stay valid for the duration of the rollout. Adversarial review (Codex) flagged the false-negative; RFC re-read confirmed `id` (not `file_path`) is the identity field. Plan and docblock terminology updated to drop the misleading "canonical match key" framing. Adds a regression test for the bare-filename × file_path-emitting asset case.
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (3)
src/platform/missingMedia/missingMediaScan.test.ts (1)
512-515: ⚡ Quick winRemove redundant narrative comments from the test body.
The test title already communicates intent; these lines add maintenance overhead without changing behavior.
As per coding guidelines, "Write code that is expressive and self-documenting to minimize need for code comments" and "Avoid new usage of code comments; do not add or retain redundant comments."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/platform/missingMedia/missingMediaScan.test.ts` around lines 512 - 515, Remove the redundant narrative comments in the test that start with "Pre-BE-933/934 workflow: widget value is the bare filename..." — open the test in missingMediaScan.test.ts and delete those explanatory comment lines inside the test body so the test title/description alone documents intent; leave assertions and setup (the existing test function and its assertions) unchanged.src/platform/assets/schemas/assetSchema.ts (1)
9-14: ⚡ Quick winKeep schema code self-documenting and move RFC prose out of inline comments.
This block is policy documentation rather than implementation detail and is likely to drift. Keep only minimal field intent in code and move rollout/RFC context to docs/tests.
As per coding guidelines, "Write code that is expressive and self-documenting to minimize need for code comments" and "Avoid new usage of code comments; do not add or retain redundant comments."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/platform/assets/schemas/assetSchema.ts` around lines 9 - 14, The large RFC/prose comment in assetSchema.ts should be replaced with a concise, self-documenting comment for the schema field(s): remove the rollout/RFC paragraph and retain a short intent line such as "namespace-rooted locator/display string (optional) — consumers MUST NOT rely on this; use id for identity." Update references in the repo by moving the RFC details into a dedicated docs or tests file (e.g., RFC/BE-808 or tests that assert behavior) and ensure any code comments reference the resolver symbol missingMediaAssetResolver.getAssetDetectionNames and field names file_path and id so future readers can find the external RFC documentation.src/platform/missingMedia/missingMediaAssetResolver.ts (1)
80-89: ⚡ Quick winTrim inline rollout prose in implementation code.
This explanation is valuable, but it’s too policy-heavy for a hot-path function comment and will age quickly. Prefer a concise comment plus test/docs as source of truth.
As per coding guidelines, "Write code that is expressive and self-documenting to minimize need for code comments" and "Avoid new usage of code comments; do not add or retain redundant comments."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/platform/missingMedia/missingMediaAssetResolver.ts` around lines 80 - 89, The long rollout/policy prose in the comment above the detection keys should be shortened: replace the paragraph with a concise one-line summary stating the intent (e.g., "Detection keys include file_path, asset_hash, name, and subfolder+name to preserve legacy matching semantics for asset identity per BE-808"), remove the historical/rollout details, and add or update relevant tests/docs (unit/integration tests that cover matching behavior and a docs note) to capture the removed explanation; update the comment located near the detection logic in missingMediaAssetResolver (the block referencing `file_path`, `asset_hash`, `name`, and `subfolder + name`) accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/platform/assets/schemas/assetSchema.ts`:
- Around line 9-14: The large RFC/prose comment in assetSchema.ts should be
replaced with a concise, self-documenting comment for the schema field(s):
remove the rollout/RFC paragraph and retain a short intent line such as
"namespace-rooted locator/display string (optional) — consumers MUST NOT rely on
this; use id for identity." Update references in the repo by moving the RFC
details into a dedicated docs or tests file (e.g., RFC/BE-808 or tests that
assert behavior) and ensure any code comments reference the resolver symbol
missingMediaAssetResolver.getAssetDetectionNames and field names file_path and
id so future readers can find the external RFC documentation.
In `@src/platform/missingMedia/missingMediaAssetResolver.ts`:
- Around line 80-89: The long rollout/policy prose in the comment above the
detection keys should be shortened: replace the paragraph with a concise
one-line summary stating the intent (e.g., "Detection keys include file_path,
asset_hash, name, and subfolder+name to preserve legacy matching semantics for
asset identity per BE-808"), remove the historical/rollout details, and add or
update relevant tests/docs (unit/integration tests that cover matching behavior
and a docs note) to capture the removed explanation; update the comment located
near the detection logic in missingMediaAssetResolver (the block referencing
`file_path`, `asset_hash`, `name`, and `subfolder + name`) accordingly.
In `@src/platform/missingMedia/missingMediaScan.test.ts`:
- Around line 512-515: Remove the redundant narrative comments in the test that
start with "Pre-BE-933/934 workflow: widget value is the bare filename..." —
open the test in missingMediaScan.test.ts and delete those explanatory comment
lines inside the test body so the test title/description alone documents intent;
leave assertions and setup (the existing test function and its assertions)
unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 82b96acf-ffe2-41ed-a859-ad1da2b3fd7c
📒 Files selected for processing (4)
src/platform/assets/schemas/assetSchema.tssrc/platform/missingMedia/missingMediaAssetResolver.test.tssrc/platform/missingMedia/missingMediaAssetResolver.tssrc/platform/missingMedia/missingMediaScan.test.ts
…se.allSettled) `resolveMissingMediaAssetSources` previously ran the input-asset fetch (`/api/assets`) and the generated-asset fetch (Cloud `/api/assets` or OSS `/history`) under `Promise.all` with `abortSiblingsOnFailure`, so a single rejection took down both. With the FE-746 unification, the input fetch now always hits `assetService.getInputAssetsIncludingPublic` — including against backends that may legitimately fail (pre-BE-786 OSS without `/api/assets`, BE-934 partial deploys, transient network errors). Switch to `Promise.allSettled` and per-branch soft-degrade. Each oracle is independent; failure in one shouldn't cancel the other. On rejection, return an empty list from that branch; abort errors stay silent, real errors get a one-line console warning so we can spot them without swallowing the rest of the verification. Caller (`verifyMediaCandidates`) already treats an empty asset list as "nothing matches this candidate," which surfaces affected candidates as missing rather than silently dropping them via the previous toast-warn path. False-positive missing is recoverable (user sees the issue in the Errors tab); silent-fail was not. Adversarial review (Codex) flagged the cascade. RFC §4's BEST EFFORT framing for `file_path` already implies the contract has soft-fail expectations, so this aligns the resolver shape with the RFC's stated tolerance. Removes the now-unused `abortSiblingsOnFailure` helper. Replaces the abort-cascade regression test with two independence tests covering each oracle failing in isolation.
dosubot
Bot
added
the
size:L
…ests (FE-746) Adds an e2e regression spec for the FE-746 behavior changes that unit tests can only assert with mocks: 1. Case B union — a workflow stored before BE-933/934 keeps its widget value as a bare filename. Once the backend starts emitting `file_path` on the same asset (a namespace-rooted locator that diverges from `name`), detection must still match via the `name` arm. The pre-correction Case A early-return would false-negative here. 2. BE-933 hash-only registration — assets registered via POST /assets/from-hash come back with `file_path: null` and `display_name: null`. The legacy `name` arm has to keep working through the BE-792 deprecation window. 3. Negative path — a listing that does not cover the widget value via any key still surfaces the missing-media overlay. Guards against an accidental "match everything" regression when the early-return was removed. 4. Soft-degrade — when `/api/assets` returns 500 (pre-BE-786 OSS without the endpoint, partial BE-934 deploys, transient errors), `Promise.allSettled` lets the verifier finish and mark the candidate missing instead of leaving `isMissing` stuck at undefined behind a silent toast. `Asset` from `@comfyorg/ingest-types` does not yet carry `file_path` / `display_name` (regen is BE-932). Spec extends the type locally so the mocked wire shape stays strongly typed. The companion workflow asset is the minimum LoadImage graph needed to fire the missing-media pipeline; widget value is a fixed sentinel filename used by every test.
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
browser_tests/tests/missingMediaAssetUnion.spec.ts (1)
16-19: ⚡ Quick winAvoid brittle overlap when extending
PostBEAsset
PostBEAssetis defined asAsset & { file_path?: string | null; display_name?: string | null }, but intersection types don’t “override” overlapping keys—they combine constraints, which can narrow unexpectedly ifAssetregenerates with different nullability/optionality. PreferOmit+ re-add for stable “override-like” behavior.♻️ Proposed fix
-type PostBEAsset = Asset & { +type PostBEAsset = Omit<Asset, 'file_path' | 'display_name'> & { file_path?: string | null display_name?: string | null }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@browser_tests/tests/missingMediaAssetUnion.spec.ts` around lines 16 - 19, PostBEAsset currently intersects Asset with a new shape which can produce brittle/contradictory constraints if Asset later defines file_path/display_name differently; change the type definition for PostBEAsset to first omit the overlapping keys from Asset (omit 'file_path' and 'display_name') and then intersect that omitted Asset with your desired optional nullable properties so the new optional/nullability for file_path and display_name reliably overrides Asset's definition.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@browser_tests/tests/missingMediaAssetUnion.spec.ts`:
- Around line 16-19: PostBEAsset currently intersects Asset with a new shape
which can produce brittle/contradictory constraints if Asset later defines
file_path/display_name differently; change the type definition for PostBEAsset
to first omit the overlapping keys from Asset (omit 'file_path' and
'display_name') and then intersect that omitted Asset with your desired optional
nullable properties so the new optional/nullability for file_path and
display_name reliably overrides Asset's definition.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 488881c8-7b5d-44ab-a823-596c3be4bc42
📒 Files selected for processing (2)
browser_tests/assets/missing/fe746_load_image_bare_filename.jsonbrowser_tests/tests/missingMediaAssetUnion.spec.ts
✅ Files skipped from review due to trivial changes (1)
- browser_tests/assets/missing/fe746_load_image_bare_filename.json
# Conflicts: # src/platform/assets/schemas/assetSchema.ts # src/platform/missingMedia/missingMediaAssetResolver.ts # src/platform/missingMedia/missingMediaScan.ts
🎨 Storybook: ✅ Built — View Storybook📦 Bundle: 7.45 MB gzip 🔴 +711 BDetailsSummary
Category Glance App Entry Points — 46.7 kB (baseline 46.7 kB) • ⚪ 0 BMain entry bundles and manifests
Status: 1 added / 1 removed Graph Workspace — 1.25 MB (baseline 1.25 MB) • 🔴 +11 BGraph editor runtime, canvas, workflow orchestration
Status: 1 added / 1 removed Views & Navigation — 95.3 kB (baseline 95.3 kB) • ⚪ 0 BTop-level views, pages, and routed surfaces
Status: 9 added / 9 removed / 3 unchanged Panels & Settings — 525 kB (baseline 525 kB) • ⚪ 0 BConfiguration panels, inspectors, and settings screens
Status: 11 added / 11 removed / 15 unchanged User & Accounts — 19.9 kB (baseline 19.9 kB) • ⚪ 0 BAuthentication, profile, and account management bundles
Status: 6 added / 6 removed / 3 unchanged Editors & Dialogs — 112 kB (baseline 112 kB) • ⚪ 0 BModals, dialogs, drawers, and in-app editors
Status: 4 added / 4 removed / 1 unchanged UI Components — 57.2 kB (baseline 57.2 kB) • ⚪ 0 BReusable component library chunks
Status: 5 added / 5 removed / 8 unchanged Data & Services — 268 kB (baseline 268 kB) • ⚪ 0 BStores, services, APIs, and repositories
Status: 13 added / 13 removed / 3 unchanged Utilities & Hooks — 3.32 MB (baseline 3.32 MB) • 🔴 +1.24 kBHelpers, composables, and utility bundles
Status: 14 added / 14 removed / 16 unchanged Vendor & Third-Party — 15.3 MB (baseline 15.3 MB) • ⚪ 0 BExternal libraries and shared vendor chunks Status: 16 unchanged Other — 10.4 MB (baseline 10.4 MB) • ⚪ 0 BBundles that do not match a named category
Status: 62 added / 62 removed / 89 unchanged ⚡ Performance Report
Show regressions
All metrics
Historical variance (last 15 runs)
Trend (last 15 commits on main)
Raw data{
"timestamp": "2026-06-19T13:12:29.250Z",
"gitSha": "08fc0f06ccaa68def5000e6bf6e13417a66f4382",
"branch": "jaewon/fe-746-l3-fe-migrate-missingmediascants-to-unified-file_path-based",
"measurements": [
{
"name": "canvas-idle",
"durationMs": 2014.8130000000037,
"styleRecalcs": 8,
"styleRecalcDurationMs": 6.446999999999999,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 356.296,
"heapDeltaBytes": -2419964,
"heapUsedBytes": 56370352,
"domNodes": 16,
"jsHeapTotalBytes": 26738688,
"scriptDurationMs": 17.692999999999998,
"eventListeners": 4,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "canvas-idle",
"durationMs": 2023.949000000016,
"styleRecalcs": 11,
"styleRecalcDurationMs": 9.063000000000002,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 391.36299999999994,
"heapDeltaBytes": -3111960,
"heapUsedBytes": 53557340,
"domNodes": 22,
"jsHeapTotalBytes": 24903680,
"scriptDurationMs": 26.782,
"eventListeners": 4,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "canvas-mouse-sweep",
"durationMs": 1978.5759999999755,
"styleRecalcs": 76,
"styleRecalcDurationMs": 42.278000000000006,
"layouts": 12,
"layoutDurationMs": 4.0169999999999995,
"taskDurationMs": 903.4649999999999,
"heapDeltaBytes": -7108624,
"heapUsedBytes": 51770068,
"domNodes": 59,
"jsHeapTotalBytes": 26476544,
"scriptDurationMs": 144.012,
"eventListeners": 4,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66999999999998,
"p95FrameDurationMs": 16.799999999999272
},
{
"name": "canvas-mouse-sweep",
"durationMs": 1769.7679999999991,
"styleRecalcs": 74,
"styleRecalcDurationMs": 34.172999999999995,
"layouts": 12,
"layoutDurationMs": 3.274,
"taskDurationMs": 763.296,
"heapDeltaBytes": -7068340,
"heapUsedBytes": 51474276,
"domNodes": 58,
"jsHeapTotalBytes": 26476544,
"scriptDurationMs": 115.627,
"eventListeners": 4,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "canvas-zoom-sweep",
"durationMs": 1741.2969999999746,
"styleRecalcs": 31,
"styleRecalcDurationMs": 18.287000000000003,
"layouts": 6,
"layoutDurationMs": 0.7840000000000001,
"taskDurationMs": 367.541,
"heapDeltaBytes": -5342124,
"heapUsedBytes": 48489964,
"domNodes": -268,
"jsHeapTotalBytes": 15642624,
"scriptDurationMs": 28.766999999999996,
"eventListeners": -184,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66333333333332,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "canvas-zoom-sweep",
"durationMs": 1724.4859999999562,
"styleRecalcs": 32,
"styleRecalcDurationMs": 17.308,
"layouts": 6,
"layoutDurationMs": 0.702,
"taskDurationMs": 319.94,
"heapDeltaBytes": 1240652,
"heapUsedBytes": 58052568,
"domNodes": 79,
"jsHeapTotalBytes": 25165824,
"scriptDurationMs": 27.837999999999997,
"eventListeners": 19,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66333333333332,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "dom-widget-clipping",
"durationMs": 644.3219999999883,
"styleRecalcs": 10,
"styleRecalcDurationMs": 7.132,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 427.70399999999995,
"heapDeltaBytes": -23393568,
"heapUsedBytes": 45473080,
"domNodes": -291,
"jsHeapTotalBytes": 6729728,
"scriptDurationMs": 54.492999999999995,
"eventListeners": -201,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66333333333335,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "dom-widget-clipping",
"durationMs": 579.1409999999928,
"styleRecalcs": 13,
"styleRecalcDurationMs": 8.245,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 339.80699999999996,
"heapDeltaBytes": 7333728,
"heapUsedBytes": 65661588,
"domNodes": 22,
"jsHeapTotalBytes": 18612224,
"scriptDurationMs": 56.06700000000001,
"eventListeners": 2,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "large-graph-idle",
"durationMs": 2076.116000000013,
"styleRecalcs": 9,
"styleRecalcDurationMs": 8.098,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 591.8399999999999,
"heapDeltaBytes": -25029692,
"heapUsedBytes": 57417692,
"domNodes": -300,
"jsHeapTotalBytes": 4820992,
"scriptDurationMs": 103.657,
"eventListeners": -194,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66333333333332,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "large-graph-idle",
"durationMs": 2012.8459999999677,
"styleRecalcs": 11,
"styleRecalcDurationMs": 10.057,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 542.418,
"heapDeltaBytes": -8719000,
"heapUsedBytes": 61813240,
"domNodes": 22,
"jsHeapTotalBytes": 10891264,
"scriptDurationMs": 97.241,
"eventListeners": 4,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66333333333335,
"p95FrameDurationMs": 16.799999999999272
},
{
"name": "large-graph-pan",
"durationMs": 2116.0379999999464,
"styleRecalcs": 70,
"styleRecalcDurationMs": 19.188,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 1069.213,
"heapDeltaBytes": 11943072,
"heapUsedBytes": 81131212,
"domNodes": -277,
"jsHeapTotalBytes": -1531904,
"scriptDurationMs": 368.11100000000005,
"eventListeners": -195,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66333333333335,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "large-graph-pan",
"durationMs": 2087.7269999999726,
"styleRecalcs": 69,
"styleRecalcDurationMs": 18.779999999999998,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 1076.6989999999998,
"heapDeltaBytes": 10334868,
"heapUsedBytes": 83079172,
"domNodes": 18,
"jsHeapTotalBytes": 11591680,
"scriptDurationMs": 377.613,
"eventListeners": 6,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.699999999999818
},
{
"name": "large-graph-zoom",
"durationMs": 3150.367000000017,
"styleRecalcs": 65,
"styleRecalcDurationMs": 19.402999999999995,
"layouts": 60,
"layoutDurationMs": 7.8759999999999994,
"taskDurationMs": 1331.7590000000002,
"heapDeltaBytes": 15105736,
"heapUsedBytes": 70576548,
"domNodes": 12,
"jsHeapTotalBytes": 7077888,
"scriptDurationMs": 467.5969999999999,
"eventListeners": 8,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66999999999998,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "large-graph-zoom",
"durationMs": 3161.185000000046,
"styleRecalcs": 67,
"styleRecalcDurationMs": 20.736,
"layouts": 60,
"layoutDurationMs": 8.050999999999998,
"taskDurationMs": 1322.343,
"heapDeltaBytes": -5143512,
"heapUsedBytes": 68735868,
"domNodes": 16,
"jsHeapTotalBytes": 8531968,
"scriptDurationMs": 469.34800000000007,
"eventListeners": 8,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "minimap-idle",
"durationMs": 2002.615999999989,
"styleRecalcs": 9,
"styleRecalcDurationMs": 8.123000000000001,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 563.1270000000001,
"heapDeltaBytes": -9332176,
"heapUsedBytes": 64546364,
"domNodes": 18,
"jsHeapTotalBytes": 7745536,
"scriptDurationMs": 100.81899999999999,
"eventListeners": 4,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66333333333335,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "minimap-idle",
"durationMs": 2026.6009999999142,
"styleRecalcs": 8,
"styleRecalcDurationMs": 8.242,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 533.894,
"heapDeltaBytes": -3315728,
"heapUsedBytes": 58622100,
"domNodes": -302,
"jsHeapTotalBytes": -540672,
"scriptDurationMs": 89.108,
"eventListeners": -199,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "subgraph-dom-widget-clipping",
"durationMs": 547.3100000000386,
"styleRecalcs": 47,
"styleRecalcDurationMs": 10.926,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 367.736,
"heapDeltaBytes": 7691928,
"heapUsedBytes": 66439328,
"domNodes": 20,
"jsHeapTotalBytes": 19136512,
"scriptDurationMs": 123.382,
"eventListeners": 8,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000273
},
{
"name": "subgraph-dom-widget-clipping",
"durationMs": 562.0910000000094,
"styleRecalcs": 47,
"styleRecalcDurationMs": 10.798,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 363.969,
"heapDeltaBytes": -11077284,
"heapUsedBytes": 52076432,
"domNodes": 20,
"jsHeapTotalBytes": 16515072,
"scriptDurationMs": 114.604,
"eventListeners": 6,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000273
},
{
"name": "subgraph-idle",
"durationMs": 2030.7050000000118,
"styleRecalcs": 11,
"styleRecalcDurationMs": 16.019,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 386.947,
"heapDeltaBytes": -2529816,
"heapUsedBytes": 56386796,
"domNodes": 22,
"jsHeapTotalBytes": 26214400,
"scriptDurationMs": 17.43,
"eventListeners": 4,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "subgraph-idle",
"durationMs": 2011.985999999979,
"styleRecalcs": 10,
"styleRecalcDurationMs": 8.204,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 368.57099999999997,
"heapDeltaBytes": -2155836,
"heapUsedBytes": 56699904,
"domNodes": 20,
"jsHeapTotalBytes": 26214400,
"scriptDurationMs": 21.991,
"eventListeners": 4,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "subgraph-mouse-sweep",
"durationMs": 1719.268999999997,
"styleRecalcs": 77,
"styleRecalcDurationMs": 39.184999999999995,
"layouts": 16,
"layoutDurationMs": 4.8260000000000005,
"taskDurationMs": 734.431,
"heapDeltaBytes": -11040216,
"heapUsedBytes": 47763528,
"domNodes": 63,
"jsHeapTotalBytes": 26476544,
"scriptDurationMs": 95.111,
"eventListeners": 4,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66333333333335,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "subgraph-mouse-sweep",
"durationMs": 1711.026000000004,
"styleRecalcs": 76,
"styleRecalcDurationMs": 37.40200000000001,
"layouts": 16,
"layoutDurationMs": 4.538,
"taskDurationMs": 731.3199999999999,
"heapDeltaBytes": -10657088,
"heapUsedBytes": 48021584,
"domNodes": 63,
"jsHeapTotalBytes": 26476544,
"scriptDurationMs": 93.066,
"eventListeners": 4,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66333333333335,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "subgraph-transition-enter",
"durationMs": 898.0589999999893,
"styleRecalcs": 16,
"styleRecalcDurationMs": 24.432999999999996,
"layouts": 4,
"layoutDurationMs": 11.216000000000001,
"taskDurationMs": 695.0300000000001,
"heapDeltaBytes": 4366600,
"heapUsedBytes": 79786556,
"domNodes": 13833,
"jsHeapTotalBytes": 17825792,
"scriptDurationMs": 26.796,
"eventListeners": 2527,
"totalBlockingTimeMs": 151,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.699999999999818
},
{
"name": "viewport-pan-sweep",
"durationMs": 8198.935000000005,
"styleRecalcs": 251,
"styleRecalcDurationMs": 59.736,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 4253.659000000001,
"heapDeltaBytes": 4346020,
"heapUsedBytes": 74520072,
"domNodes": 20,
"jsHeapTotalBytes": 25747456,
"scriptDurationMs": 1455.3700000000001,
"eventListeners": 20,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "viewport-pan-sweep",
"durationMs": 8226.949999999988,
"styleRecalcs": 252,
"styleRecalcDurationMs": 57.28399999999999,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 3971.1580000000004,
"heapDeltaBytes": -100816,
"heapUsedBytes": 69985944,
"domNodes": 22,
"jsHeapTotalBytes": 19980288,
"scriptDurationMs": 1315.7859999999998,
"eventListeners": 20,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "vue-large-graph-idle",
"durationMs": 11906.45600000005,
"styleRecalcs": 0,
"styleRecalcDurationMs": 0,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 11892.967999999997,
"heapDeltaBytes": -39477972,
"heapUsedBytes": 170487932,
"domNodes": -3308,
"jsHeapTotalBytes": 21991424,
"scriptDurationMs": 560.229,
"eventListeners": -16472,
"totalBlockingTimeMs": 0,
"frameDurationMs": 17.220000000000073,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "vue-large-graph-idle",
"durationMs": 10796.127999999953,
"styleRecalcs": 0,
"styleRecalcDurationMs": 0,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 10780.414999999999,
"heapDeltaBytes": -16239952,
"heapUsedBytes": 169819252,
"domNodes": -3304,
"jsHeapTotalBytes": 20185088,
"scriptDurationMs": 565.581,
"eventListeners": -16466,
"totalBlockingTimeMs": 0,
"frameDurationMs": 17.216666666666665,
"p95FrameDurationMs": 16.799999999999272
},
{
"name": "vue-large-graph-pan",
"durationMs": 13626.798000000008,
"styleRecalcs": 65,
"styleRecalcDurationMs": 19.441000000000013,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 13609.033999999998,
"heapDeltaBytes": -23710560,
"heapUsedBytes": 184785252,
"domNodes": -3308,
"jsHeapTotalBytes": 17711104,
"scriptDurationMs": 841.676,
"eventListeners": -16470,
"totalBlockingTimeMs": 38,
"frameDurationMs": 17.220000000000073,
"p95FrameDurationMs": 16.799999999999272
},
{
"name": "vue-large-graph-pan",
"durationMs": 13508.115999999973,
"styleRecalcs": 65,
"styleRecalcDurationMs": 19.128000000000007,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 13488.428,
"heapDeltaBytes": -25129964,
"heapUsedBytes": 180583252,
"domNodes": -3308,
"jsHeapTotalBytes": 20594688,
"scriptDurationMs": 870.1239999999999,
"eventListeners": -16472,
"totalBlockingTimeMs": 49,
"frameDurationMs": 17.223333333333358,
"p95FrameDurationMs": 16.80000000000291
},
{
"name": "workflow-execution",
"durationMs": 510.0460000000453,
"styleRecalcs": 19,
"styleRecalcDurationMs": 26.476,
"layouts": 6,
"layoutDurationMs": 2.449,
"taskDurationMs": 176.45799999999997,
"heapDeltaBytes": -18164732,
"heapUsedBytes": 51290236,
"domNodes": -81,
"jsHeapTotalBytes": 438272,
"scriptDurationMs": 17.358999999999998,
"eventListeners": -142,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000273
},
{
"name": "workflow-execution",
"durationMs": 459.67700000005607,
"styleRecalcs": 15,
"styleRecalcDurationMs": 23.023000000000003,
"layouts": 5,
"layoutDurationMs": 1.681,
"taskDurationMs": 120.678,
"heapDeltaBytes": 5123404,
"heapUsedBytes": 64966676,
"domNodes": 155,
"jsHeapTotalBytes": 2883584,
"scriptDurationMs": 16.45,
"eventListeners": 69,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.663333333333338,
"p95FrameDurationMs": 16.700000000000273
}
]
}🎭 Playwright: ❌ 1667 passed, 2 failed · 1 flaky❌ Failed Tests📊 Browser Reports
|
🎭 Playwright: ⏳ Running... |
…ot abort graph load (FE-746)
…ts-to-unified-file_path-based
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Aligns
missingMediaScanwith RFC BE-808 v2 (Asset Identity Semantics). Removes theisCloudbranch from the scanner / verifier surface so both backends consult the same asset-listing oracle once BE-933 / BE-934 ship.Changes & rationale
Schema —
assetSchema.tsAdd
file_path: z.string().nullish()toAssetItem. Both BE PRs emitfile_pathas nullable optional (str | Nonein BE-933,*stringin BE-934). Nullish (not optional-only) because backends explicitly sendnullfor hash-only assets, not absence.Detection —
getAssetDetectionNamesinmissingMediaAssetResolver.tsUnion
file_path,asset_hash,name, andsubfolder + namevariants into the detection-name set for every asset.Why union, not file_path-only. I originally short-circuited on
file_pathon the premise that it was the canonical match key. Re-reading RFC BE-808 v2 directly shows that's not what the RFC says: §3 makesidthe identity field, and §4 callsfile_path"a standalone locator/display string" emitted on a BEST EFFORT basis. The workflow widget value (the thing on the other side of the comparison) does not auto-upgrade when the backend response shape changes — a workflow authored a month ago still stores the bare filename"photo.png". An asset that newly emitsfile_path: "input/sub/photo.png"plus the bare-filename widget would false-negative the match under the early-return, marking a present file as missing. Codex adversarial review caught this; the RFC re-read confirmed the framing was wrong. Union match keeps every legacy widget shape (bare filename, hash, annotated path, subfolder/name) verifiable through the BE-792 deprecation window.Scanner —
scanAllMediaCandidates/scanNodeMediaCandidatesDrop the
isCloudparameter. Delete the OSS synchronous-shortcut branch at the previousmissingMediaScan.ts:103-118. All candidates now leaveisMissingasundefinedand resolve asynchronously throughverifyMediaCandidatesagainst the unified asset listing.Why drop the sync shortcut. Keeping a backend-conditional sync path against
widget.options.valueswould re-introduce the very divergence FE-746 removes. Cloud has always taken the async path; OSS moves onto it post-BE-786. The Errors-tab UX already tolerates async resolution.Verification —
verifyMediaCandidatesRename
MediaVerificationOptions.isCloud→allowCompactSuffix. The flag still selects between\s+\[(input|output)\]$(OSS, space-required to avoid stripping literal brackets from filenames) and\s*\[(input|output)\]$(Cloud, compact form). Notion §4.2 classifies this regex split as N1 / workflow-identity territory and explicitly leaves it out of M1. The rename severs the lexical coupling to backend identity so the residual flag describes what it actually does (annotation grammar), not who emits it.Resolver —
resolveMissingMediaAssetSourcesisCloudfrom the public options.assetService.getInputAssetsIncludingPublic(previously OSS wasPromise.resolve([])).Promise.all+abortSiblingsOnFailuretoPromise.allSettledwith per-branch soft-degrade.Why soft-degrade. The two oracles (
/api/assetsfor input, Cloud/api/assets?include_tags=outputor OSS/historyfor generated) are independent data sources. Under the previous cascade, an/api/assets404 — possible on pre-BE-786 self-hosted OSS, or during a BE-934 partial deploy / zod schema skew — aborted the generated-history fetch too and surfaced as a single toast warning, leaving every candidate'sisMissingstuck atundefined. Codex adversarial review flagged this silent-fail. WithallSettled, each branch degrades to[]independently;verifyMediaCandidatesthen marks affected candidates missing rather than swallowing the verification. RFC §4's BEST EFFORT framing already implies the contract has soft-fail expectations, so this aligns the resolver shape with the RFC.Generated-assets oracle still branches internally
fetchGeneratedAssetsstill picks Cloud asset API vs OSS job-history based on a module-levelisCloudimport. The public API of the resolver no longer carriesisCloud, but unifying this fork depends on whether OSS auto-registers output files as assets post-BE-786 — out of FE-746's stated scope. Tracked as a follow-up.Callers
Caller-arg cleanup that follows mechanically from the signature changes:
src/scripts/app.ts:1508, 1523src/composables/graph/useErrorClearingHooks.ts:249, 305src/platform/assets/utils/markDeletedAssetsAsMissingMedia.ts:41Tests
isCloudmock with mutable holder so each test can drive the residual generated-assets oracle (Cloud/api/assetsvs OSS history) per case.getAssetDetectionNames: union behaviour assertion, file_path-null fallback, empty asset, slash/backslash subfolder variants.verifyMediaCandidates: bare-filename widget × file_path-emitting asset regression test (the false-negative scenario Codex flagged)./api/assetsfailure leaves generated fetch alive;/historyfailure leaves input fetch alive.isMissing: undefined).MUST NOT MERGE until backend ships
Both BE PRs must land and roll out before this can merge — until then the schema accepts
file_pathbut no response actually emits it, so the file_path branch of the union is inert until BE deploys.Detailed plan, BE PR diffs, and scope analysis live at
temp/plans/fe-746-missing-media-scan-unify.md. Adversarial-review write-up attemp/summaries/fe-746-adversarial-review.html.Test plan
pnpm typecheck— cleanpnpm lint— 0 errors (3 pre-existing warnings unrelated)pnpm test:unit --run src/platform/missingMedia src/composables/graph/useErrorClearingHooks.test.ts src/platform/assets/utils/markDeletedAssetsAsMissingMedia— 135/135 passsrc/platform/assets src/composables src/scripts) — 1963/1963 passFixes FE-746