refactor(assets): remove isCloud guards on L1 UI surfaces (FE-732)#12417
Open
Conversation
Remove isCloud guards from the filter bar, asset browser modal, sidebar delete site, context-menu delete site, and input-asset delete throw — all five UI surfaces enumerated in FE-732 now render and execute unconditionally against the unified /api/assets surface. Drop the dead cloud-only docstring on AssetBrowserModal.overrideAssets and the dead shouldShowDeleteButton computed in AssetsSidebarTab (the latter was the last caller of the :show-delete-button pass-through). Fix the latent Vue Boolean prop-default-false trap in MediaAssetContextMenu by defaulting showDeleteButton to true via destructure default; the previous "propAllows = showDeleteButton ?? true" never fired because Vue normalizes absent Boolean props to false. The bug was masked while AssetsSidebarTab explicitly passed the prop. Drop the now-unused mediaAsset.deletingImportedFilesCloudOnly i18n key from all 12 locales. Tests: add OSS + Cloud input-asset deletion paths in useMediaAssetActions, add input-mode delete visibility coverage in MediaAssetContextMenu, add an @oss describe block in assetDeleteClearsLoadImage so both Playwright projects run the deletion flow. Refresh stale cloud-only comments in the filter and sort sidebar specs; their @cloud tags stay because /api/jobs is still cloud-only. L4 residual: filter-bar query params job_ids, asset_hash, and include_public will start being accepted by OSS once BE-886 and BE-891 merge — no FE change needed at that point. Stacked on top of FE-731. Depends on BE-786 (OSS removes --enable-assets) before this can ship to production. - Fixes FE-732
dosubot
Bot
added
the
size:M
Contributor
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ 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 |
Codecov Report✅ All modified and coverable lines are covered by tests. @@ Coverage Diff @@
## jaewon/fe-731-l14-l1-widgets-remove-iscloud-guards-in-usecombowidget #12417 +/- ##
========================================================================================================
+ Coverage 62.14% 62.18% +0.03%
========================================================================================================
Files 1458 1458
Lines 74992 74991 -1
Branches 21116 21113 -3
========================================================================================================
+ Hits 46604 46633 +29
+ Misses 28044 28016 -28
+ Partials 344 342 -2
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
Lock the post-FE-732 contracts surfaced by the latent Boolean prop-default-true fix and the unconditional filter rendering: - MediaAssetContextMenu.test.ts — add an explicit showDeleteButton: false case so the opt-out path of the prop is asserted, complementing the input-mode default-true case from the parent commit. Reshape mountComponent to take an opts object. - MediaAssetFilterBar.test.ts — new component test verifies the filter button renders unconditionally and that the settings menu receives show-sort-options=true after the constant pin.
dante01yoon
added a commit
that referenced
this pull request
May 22, 2026
The pre-FE-732 sidebar logic hid the bulk Delete footer button on the Imported tab when isCloud was false. After removing the gate the button must render in both Cloud and OSS builds. Add a Playwright case that runs on the default chromium project — i.e. the OSS build — to lock the new invariant, mirroring the existing output-asset case in the same describe block.
dante01yoon
added a commit
that referenced
this pull request
May 26, 2026
Align with the L1 stack pattern (FE-729 #12322 / FE-730 #12335 / FE-731 #12375 / FE-732 #12417): the Generated tab now resolves to `useFlatOutputAssetsGrouped()` unconditionally. OSS reaches the same asset-API path as cloud once BE-786 lands and `_ASSETS_ENABLED` is no longer a gate; no transitional fallback is kept, in line with FE-730's "no temporary isCloud fallback" stance. FE-740
…ode dropdown open (FE-732 fix) (#12465) https://github.com/user-attachments/assets/1baf8283-8170-4d50-aa22-25c05598874d Stacks on #12417 (FE-732). The isCloud guard removal in the M1 stack (FE-731/FE-732) exposed two latent regressions on the inline `FormDropdown` asset path; this PR fixes both. ## Symptoms (verified via CDP against `pr-3809.testenvs.comfy.org` cloud BE and a local `synap5e/assets-m1 --enable-assets` OSS BE) Reported by Simon in #m1-fe-integration testing against PR #12411: 1. **Model dropdown stale list** — `r` to refresh doesn't update the dropdown; the model list is fetched once on first dropdown open (`/api/assets?include_tags=models,checkpoints&limit=500&exclude_tags=missing`) and cached. Reopening shows stale data. Newly imported models don't appear until full page reload. 2. **`/api/jobs` per dropdown open** — each model dropdown open hits `/api/jobs?status=completed,failed,cancelled&limit=200&offset=0` for no reason. This is the OSS history path being triggered by the output-media refresh hook in the dropdown. ## Root cause - `useAssetWidgetData.watch(immediate:true)` is the only trigger for model fetch; once `assetsStore.hasAssetKey(...)` returns true, the watch short-circuits forever. Dropdown reopen has no refresh hook. - `WidgetSelectDropdown.handleIsOpenUpdate` calls `outputMediaAssets.refresh()` on every open regardless of widget kind. For asset-mode (model) dropdowns this is irrelevant — but the call still routes through `useAssetsApi('output')` → `assetsStore.updateHistory` → `api.getHistory` → `/jobs?status=completed,failed,cancelled` on OSS. Cloud distribution swaps the provider for `useFlatOutputAssets` which hits `/api/assets?include_tags=output` — still wasted work, just a different URL. The handler and cache guard are pre-existing (#6734, #8090), but were only reachable from cloud distributions before FE-731 unwrapped `if (isCloud)` in `useAssetWidgetData`. ## Fix - `useAssetWidgetData`: expose `refresh()` that re-invokes `assetsStore.updateModelsForNodeType` for the current node type, guarded against re-entry while a fetch is in flight. - `WidgetSelectDropdown.handleIsOpenUpdate`: branch on `props.isAssetMode` — asset-mode dropdowns refresh model assets via `assetData.refresh()`; non-asset-mode dropdowns continue to refresh `outputMediaAssets` as before (preserved cloud / OSS behavior). ## Verification **CDP — Vue Nodes 2.0 ON** Same dropdown opened twice (`qwen_image_vae` VAELoader widget): | Open # | `/api/jobs?status=...` | `/api/assets?include_tags=models,vae` | |---|---|---| | 1st | 0 | 1 (fresh fetch) | | 2nd | 0 | 1 (fresh fetch — newly added models would appear) | **Before the fix** (same setup): 1st open fired both `/api/jobs` and the model fetch; 2nd open fired only `/api/jobs` (model list stale). **Other paths preserved (verified empirically + by code)**: - OSS + non-asset-mode dropdown → `/api/jobs` still fires (existing OSS behavior). - Cloud + non-asset-mode dropdown → `/api/assets?include_tags=output` still fires (existing cloud behavior; branch in `WidgetSelectDropdown.vue` outputMediaAssets ternary is untouched). - WS-status queue/history polling (`limit=64` `/api/jobs`) still fires on page load — unrelated to dropdown. ## Test plan - [x] `pnpm vitest run src/renderer/extensions/vueNodes/widgets/composables/useAssetWidgetData.test.ts` — 10/10 (3 new tests for `refresh()`). - [x] `pnpm vitest run src/renderer/extensions/vueNodes/widgets/components/WidgetSelectDropdown.test.ts` — 9/9 (4 new tests cover handleIsOpenUpdate branches: asset-mode skips outputMediaAssets.refresh, asset-mode reopen refetches model assets, non-asset-mode preserves outputMediaAssets.refresh, close event is no-op). - [x] `pnpm typecheck` clean. - [ ] Re-test in `m1-fe-integration` once stacked PRs merge: import a new model via cloud import flow → reopen dropdown → new model appears without page reload. Uploading Screen Recording 2026-05-27 at 12.43.13 AM.mov…
dante01yoon
added a commit
that referenced
this pull request
May 26, 2026
dante01yoon
added a commit
that referenced
this pull request
Jun 2, 2026
…ed feature-flag test infra Per review (DrJKL): mockFeatureFlags and the exported FeatureFlags type have no live consumer in the open stack — FE-729~732 (#12322/#12335/#12375/#12417) don't use them, and FE-780/781 (#12485/#12486) still hand-roll inline vi.hoisted mocks. Shipping them here adds a public surface with no caller, so split them out; they'll be reintroduced bundled with the first PR that actually adopts the util. This PR is now scoped to the getAssetStoredFilename extraction, which is consumed at its two call sites in useMediaAssetActions. Retained-code review fixes: - M3: spread importOriginal in the distribution/types mock so isDesktop and isNightly survive the wholesale replacement. - L2: trim getAssetStoredFilename JSDoc; demote the BE-933/934 collapse to a one-line TODO. Removed: src/test-utils/mockFeatureFlags.ts, the FeatureFlags type export, and its knip ignore entry. Pre-split state preserved on local branch jaewon/fe-733-presplit-snapshot. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
dante01yoon
added a commit
that referenced
this pull request
Jun 2, 2026
…ed feature-flag test infra Per review (DrJKL): mockFeatureFlags and the exported FeatureFlags type have no live consumer in the open stack — FE-729~732 (#12322/#12335/#12375/#12417) don't use them, and FE-780/781 (#12485/#12486) still hand-roll inline vi.hoisted mocks. Shipping them here adds a public surface with no caller, so split them out; they'll be reintroduced bundled with the first PR that actually adopts the util. This PR is now scoped to the getAssetStoredFilename extraction, which is consumed at its two call sites in useMediaAssetActions. Retained-code review fixes: - M3: spread importOriginal in the distribution/types mock so isDesktop and isNightly survive the wholesale replacement. - L2: trim getAssetStoredFilename JSDoc; demote the BE-933/934 collapse to a one-line TODO. Removed: src/test-utils/mockFeatureFlags.ts, the FeatureFlags type export, and its knip ignore entry. Pre-split state preserved on local branch jaewon/fe-733-presplit-snapshot. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
mathtone
pushed a commit
to sketchyplans/ComfyUI-frontend-fork1
that referenced
this pull request
Jun 17, 2026
…fy-Org#12287) ## Summary L1 prerequisite cleanup, scoped to a single type-preserving refactor: extract `getAssetStoredFilename(asset)` to collapse the duplicated `isCloud && asset.asset_hash ? asset.asset_hash : asset.name` branch from `useMediaAssetActions.ts` into one helper in `assetMetadataUtils.ts`. No behavior change. Once BE-933/934 emit `file_path` and the cloud spec sync brings the field into generated types, only the helper internals change (collapse to `asset.file_path ?? asset.name`). ## Scope change (per review) The `mockFeatureFlags` test util and the exported `FeatureFlags` type that this PR originally also added have been **split out**. They had no live consumer in the open stack — FE-729~732 (Comfy-Org#12322 / Comfy-Org#12335 / Comfy-Org#12375 / Comfy-Org#12417) don't use them, and FE-780 / FE-781 (Comfy-Org#12485 / Comfy-Org#12486) still hand-roll inline `vi.hoisted` mocks — so shipping them here would add a public surface with no caller. They will be reintroduced bundled with the first PR that actually adopts the util, where `featureFlag`'s return type and the "all flags off vs. production defaults" semantics can be validated against a real consumer. ## Review fixes carried in this PR - Mock `@/platform/distribution/types` via an `importOriginal` spread so `isDesktop` / `isNightly` survive the wholesale replacement (only `isCloud` was re-hoisted before). - Trimmed the `getAssetStoredFilename` JSDoc; the BE-933/934 future-collapse is now a one-line `TODO` rather than a design-doc paragraph. ## Review Focus - The helper is intentionally named `getAssetStoredFilename` to disambiguate from the existing `getAssetFilename` (which targets `user_metadata.filename` / `metadata.filename` for serialized-identifier contexts — missing-model matching, filename schema validation) and `getAssetDisplayFilename` (UI labels). Folding the `isCloud && asset_hash` fallback into either of those would regress display/identifier sites where the cloud hash is never meant to surface. - Fixes FE-733 - Parent: FE-601 (L1 umbrella) - RFC: [Asset Identity Semantics](https://www.notion.so/comfy-org/RFC-Asset-Identity-Semantics-35a6d73d365080e59d59c98cebae779b) - Survey: [Asset FE Divergence Survey M1 Scope](https://www.notion.so/comfy-org/Assets-FE-Divergence-Survey-M1-Scope-3616d73d365080d0a9cbf5f2394c12f8) - Slack thread: https://comfy-organization.slack.com/archives/C0AUUTS2RQV/p1778815571949519 ## Screenshots (if applicable) N/A — no UI change. ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-12287-refactor-assets-extract-getAssetStoredFilename-helper-add-mockFeatureFlags-test-util-3616d73d365081c9a1c6e1982728a38a) by [Unito](https://www.unito.io) --------- Co-authored-by: GitHub Action <action@github.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Co-authored-by: Alexander Brown <drjkl@comfy.org> Co-authored-by: Matt Miller <matt@miller-media.com>
…secombowidget' into jaewon/fe-732-l15-l1-ui-surfaces-remove-iscloud-guards-filter-modal-3
📦 Bundle Size
⚡ Performance Report
Absolute values
Raw data{
"timestamp": "2026-06-18T04:09:05.829Z",
"gitSha": "9d359c3adf8b5fde9cd94d1c1cdd62952bbaa336",
"branch": "jaewon/fe-732-l15-l1-ui-surfaces-remove-iscloud-guards-filter-modal-3",
"measurements": [
{
"name": "canvas-idle",
"durationMs": 2118.6199999999926,
"styleRecalcs": 7,
"styleRecalcDurationMs": 7.640000000000001,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 486.58700000000005,
"heapDeltaBytes": -9170624,
"heapUsedBytes": 59688952,
"domNodes": -323,
"jsHeapTotalBytes": 20111360,
"scriptDurationMs": 21.918,
"eventListeners": -189,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66333333333332,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "canvas-idle",
"durationMs": 2117.4990000000093,
"styleRecalcs": 10,
"styleRecalcDurationMs": 10.051,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 520.071,
"heapDeltaBytes": -8135780,
"heapUsedBytes": 61008260,
"domNodes": -313,
"jsHeapTotalBytes": 18276352,
"scriptDurationMs": 33.822,
"eventListeners": -191,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66333333333335,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "canvas-mouse-sweep",
"durationMs": 1874.0060000000085,
"styleRecalcs": 76,
"styleRecalcDurationMs": 43.199,
"layouts": 12,
"layoutDurationMs": 3.974,
"taskDurationMs": 801.3629999999999,
"heapDeltaBytes": -1316596,
"heapUsedBytes": 61991588,
"domNodes": 59,
"jsHeapTotalBytes": 15990784,
"scriptDurationMs": 131.329,
"eventListeners": 4,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66333333333332,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "canvas-mouse-sweep",
"durationMs": 1807.7060000000529,
"styleRecalcs": 73,
"styleRecalcDurationMs": 37.952,
"layouts": 12,
"layoutDurationMs": 3.5249999999999995,
"taskDurationMs": 811.072,
"heapDeltaBytes": -7305648,
"heapUsedBytes": 51478860,
"domNodes": 56,
"jsHeapTotalBytes": 26214400,
"scriptDurationMs": 129.76600000000002,
"eventListeners": 4,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "canvas-zoom-sweep",
"durationMs": 1741.5269999999623,
"styleRecalcs": 31,
"styleRecalcDurationMs": 17.453,
"layouts": 6,
"layoutDurationMs": 0.6579999999999999,
"taskDurationMs": 366.363,
"heapDeltaBytes": 1922364,
"heapUsedBytes": 60533724,
"domNodes": 77,
"jsHeapTotalBytes": 26738688,
"scriptDurationMs": 33.70099999999999,
"eventListeners": 19,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "canvas-zoom-sweep",
"durationMs": 1739.15599999998,
"styleRecalcs": 32,
"styleRecalcDurationMs": 19.089,
"layouts": 6,
"layoutDurationMs": 0.745,
"taskDurationMs": 360.574,
"heapDeltaBytes": 1967248,
"heapUsedBytes": 60774604,
"domNodes": 79,
"jsHeapTotalBytes": 25165824,
"scriptDurationMs": 29.851,
"eventListeners": 19,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "dom-widget-clipping",
"durationMs": 572.6960000000076,
"styleRecalcs": 13,
"styleRecalcDurationMs": 8.898,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 348.066,
"heapDeltaBytes": 7858476,
"heapUsedBytes": 66428972,
"domNodes": 22,
"jsHeapTotalBytes": 19136512,
"scriptDurationMs": 62.95100000000001,
"eventListeners": 0,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000273
},
{
"name": "dom-widget-clipping",
"durationMs": 590.6659999999988,
"styleRecalcs": 12,
"styleRecalcDurationMs": 8.576,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 369.601,
"heapDeltaBytes": 7084880,
"heapUsedBytes": 65652780,
"domNodes": 20,
"jsHeapTotalBytes": 19398656,
"scriptDurationMs": 66.54099999999998,
"eventListeners": 0,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66333333333335,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "large-graph-idle",
"durationMs": 2022.7550000000178,
"styleRecalcs": 11,
"styleRecalcDurationMs": 9.729000000000001,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 597.2040000000001,
"heapDeltaBytes": -9364932,
"heapUsedBytes": 62498432,
"domNodes": 22,
"jsHeapTotalBytes": 11415552,
"scriptDurationMs": 95.576,
"eventListeners": 4,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66333333333332,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "large-graph-idle",
"durationMs": 2070.6880000000183,
"styleRecalcs": 9,
"styleRecalcDurationMs": 8.944999999999999,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 719.8439999999999,
"heapDeltaBytes": -25599788,
"heapUsedBytes": 57335588,
"domNodes": -299,
"jsHeapTotalBytes": 3260416,
"scriptDurationMs": 133.50300000000001,
"eventListeners": -191,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "large-graph-pan",
"durationMs": 2160.19,
"styleRecalcs": 70,
"styleRecalcDurationMs": 22.165999999999997,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 1193.8690000000001,
"heapDeltaBytes": 10196764,
"heapUsedBytes": 83237468,
"domNodes": 18,
"jsHeapTotalBytes": 9756672,
"scriptDurationMs": 405.646,
"eventListeners": 6,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66333333333332,
"p95FrameDurationMs": 16.699999999999818
},
{
"name": "large-graph-pan",
"durationMs": 2125.800999999967,
"styleRecalcs": 69,
"styleRecalcDurationMs": 21.098000000000003,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 1197.4009999999998,
"heapDeltaBytes": 10772548,
"heapUsedBytes": 83881980,
"domNodes": 16,
"jsHeapTotalBytes": 10805248,
"scriptDurationMs": 421.849,
"eventListeners": 6,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.799999999999272
},
{
"name": "large-graph-zoom",
"durationMs": 3114.285999999993,
"styleRecalcs": 65,
"styleRecalcDurationMs": 21.206,
"layouts": 60,
"layoutDurationMs": 9.065999999999999,
"taskDurationMs": 1391.2089999999998,
"heapDeltaBytes": 14321976,
"heapUsedBytes": 70115608,
"domNodes": 14,
"jsHeapTotalBytes": 6815744,
"scriptDurationMs": 533.0010000000001,
"eventListeners": 8,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "large-graph-zoom",
"durationMs": 3249.446999999975,
"styleRecalcs": 65,
"styleRecalcDurationMs": 22.365,
"layouts": 60,
"layoutDurationMs": 9.195,
"taskDurationMs": 1507.6429999999998,
"heapDeltaBytes": 16795748,
"heapUsedBytes": 72476872,
"domNodes": 12,
"jsHeapTotalBytes": 8126464,
"scriptDurationMs": 583.501,
"eventListeners": 8,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66333333333335,
"p95FrameDurationMs": 16.699999999999818
},
{
"name": "minimap-idle",
"durationMs": 2003.9479999999799,
"styleRecalcs": 4,
"styleRecalcDurationMs": 3.261,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 546.802,
"heapDeltaBytes": 5913148,
"heapUsedBytes": 60364652,
"domNodes": -288,
"jsHeapTotalBytes": 0,
"scriptDurationMs": 96.724,
"eventListeners": 6,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "minimap-idle",
"durationMs": 2034.7750000000815,
"styleRecalcs": 10,
"styleRecalcDurationMs": 9.082,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 562.0640000000001,
"heapDeltaBytes": -8300608,
"heapUsedBytes": 64827540,
"domNodes": 20,
"jsHeapTotalBytes": 10629120,
"scriptDurationMs": 98.911,
"eventListeners": 6,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.799999999999272
},
{
"name": "subgraph-dom-widget-clipping",
"durationMs": 605.2000000000248,
"styleRecalcs": 49,
"styleRecalcDurationMs": 13.136,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 390.49899999999997,
"heapDeltaBytes": 7351084,
"heapUsedBytes": 66539836,
"domNodes": 24,
"jsHeapTotalBytes": 18350080,
"scriptDurationMs": 130.726,
"eventListeners": 8,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666682,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "subgraph-dom-widget-clipping",
"durationMs": 604.1189999999688,
"styleRecalcs": 48,
"styleRecalcDurationMs": 12.665,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 381.84299999999996,
"heapDeltaBytes": 8111112,
"heapUsedBytes": 67121308,
"domNodes": 22,
"jsHeapTotalBytes": 19660800,
"scriptDurationMs": 129.911,
"eventListeners": 6,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.663333333333338,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "subgraph-idle",
"durationMs": 2006.1689999999999,
"styleRecalcs": 11,
"styleRecalcDurationMs": 9.578999999999999,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 365.812,
"heapDeltaBytes": -2717144,
"heapUsedBytes": 56389628,
"domNodes": 22,
"jsHeapTotalBytes": 25690112,
"scriptDurationMs": 18.952,
"eventListeners": 4,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "subgraph-idle",
"durationMs": 1998.768000000041,
"styleRecalcs": 11,
"styleRecalcDurationMs": 10.169999999999998,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 420.24500000000006,
"heapDeltaBytes": -6209156,
"heapUsedBytes": 47393344,
"domNodes": 22,
"jsHeapTotalBytes": 25427968,
"scriptDurationMs": 21.375,
"eventListeners": 4,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66333333333335,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "subgraph-mouse-sweep",
"durationMs": 1701.9600000000423,
"styleRecalcs": 75,
"styleRecalcDurationMs": 35.099,
"layouts": 16,
"layoutDurationMs": 4.179,
"taskDurationMs": 676.734,
"heapDeltaBytes": -11211632,
"heapUsedBytes": 47926584,
"domNodes": 62,
"jsHeapTotalBytes": 25165824,
"scriptDurationMs": 91.626,
"eventListeners": 4,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66333333333335,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "subgraph-mouse-sweep",
"durationMs": 1687.8150000000005,
"styleRecalcs": 76,
"styleRecalcDurationMs": 40.001,
"layouts": 16,
"layoutDurationMs": 4.2780000000000005,
"taskDurationMs": 733.0380000000001,
"heapDeltaBytes": -10934368,
"heapUsedBytes": 48114972,
"domNodes": 65,
"jsHeapTotalBytes": 25952256,
"scriptDurationMs": 99.65899999999999,
"eventListeners": 4,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.670000000000012,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "subgraph-transition-enter",
"durationMs": 1048.751999999979,
"styleRecalcs": 15,
"styleRecalcDurationMs": 28.413,
"layouts": 4,
"layoutDurationMs": 12.745000000000003,
"taskDurationMs": 776.209,
"heapDeltaBytes": 4434780,
"heapUsedBytes": 80001088,
"domNodes": 13833,
"jsHeapTotalBytes": 17825792,
"scriptDurationMs": 31.471,
"eventListeners": 2527,
"totalBlockingTimeMs": 153,
"frameDurationMs": 16.66333333333335,
"p95FrameDurationMs": 16.799999999999272
},
{
"name": "viewport-pan-sweep",
"durationMs": 8201.654999999959,
"styleRecalcs": 251,
"styleRecalcDurationMs": 63.96500000000001,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 3807.363,
"heapDeltaBytes": -2302968,
"heapUsedBytes": 68047936,
"domNodes": 22,
"jsHeapTotalBytes": 17620992,
"scriptDurationMs": 1248.2330000000002,
"eventListeners": 20,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "viewport-pan-sweep",
"durationMs": 8227.655000000028,
"styleRecalcs": 251,
"styleRecalcDurationMs": 63.544000000000004,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 3983.8669999999997,
"heapDeltaBytes": 18281276,
"heapUsedBytes": 83608232,
"domNodes": 20,
"jsHeapTotalBytes": 23855104,
"scriptDurationMs": 1247.549,
"eventListeners": 20,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66333333333332,
"p95FrameDurationMs": 16.80000000000109
},
{
"name": "vue-large-graph-idle",
"durationMs": 14574.95799999998,
"styleRecalcs": 0,
"styleRecalcDurationMs": 0,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 14548.538,
"heapDeltaBytes": 7362972,
"heapUsedBytes": 172437332,
"domNodes": -27,
"jsHeapTotalBytes": 15437824,
"scriptDurationMs": 639.841,
"eventListeners": -4,
"totalBlockingTimeMs": 0,
"frameDurationMs": 17.220000000000073,
"p95FrameDurationMs": 16.799999999999272
},
{
"name": "vue-large-graph-idle",
"durationMs": 13639.93800000003,
"styleRecalcs": 1,
"styleRecalcDurationMs": 0.6980000000000042,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 13625.596000000001,
"heapDeltaBytes": -11879596,
"heapUsedBytes": 156182580,
"domNodes": -22,
"jsHeapTotalBytes": 17010688,
"scriptDurationMs": 623.839,
"eventListeners": -6,
"totalBlockingTimeMs": 0,
"frameDurationMs": 17.220000000000073,
"p95FrameDurationMs": 16.799999999999272
},
{
"name": "vue-large-graph-pan",
"durationMs": 15982.266000000038,
"styleRecalcs": 83,
"styleRecalcDurationMs": 22.04500000000001,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 15958.527,
"heapDeltaBytes": 10234868,
"heapUsedBytes": 164990368,
"domNodes": -23,
"jsHeapTotalBytes": 16486400,
"scriptDurationMs": 886.7289999999999,
"eventListeners": 0,
"totalBlockingTimeMs": 0,
"frameDurationMs": 17.219999999999953,
"p95FrameDurationMs": 16.799999999999272
},
{
"name": "vue-large-graph-pan",
"durationMs": 15551.066999999988,
"styleRecalcs": 77,
"styleRecalcDurationMs": 21.048999999999985,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 15527.035,
"heapDeltaBytes": 13779176,
"heapUsedBytes": 168404572,
"domNodes": -23,
"jsHeapTotalBytes": 15699968,
"scriptDurationMs": 867.853,
"eventListeners": -2,
"totalBlockingTimeMs": 0,
"frameDurationMs": 17.776666666666642,
"p95FrameDurationMs": 16.80000000000291
},
{
"name": "workflow-execution",
"durationMs": 134.7949999999969,
"styleRecalcs": 13,
"styleRecalcDurationMs": 22.467000000000002,
"layouts": 4,
"layoutDurationMs": 1.885,
"taskDurationMs": 100.407,
"heapDeltaBytes": 3537768,
"heapUsedBytes": 56113564,
"domNodes": 158,
"jsHeapTotalBytes": 0,
"scriptDurationMs": 16.352999999999998,
"eventListeners": 37,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66333333333332,
"p95FrameDurationMs": 16.700000000000273
},
{
"name": "workflow-execution",
"durationMs": 469.99000000005253,
"styleRecalcs": 18,
"styleRecalcDurationMs": 24.564999999999998,
"layouts": 6,
"layoutDurationMs": 1.7469999999999999,
"taskDurationMs": 119.06199999999998,
"heapDeltaBytes": 5128328,
"heapUsedBytes": 65270644,
"domNodes": 155,
"jsHeapTotalBytes": 3145728,
"scriptDurationMs": 18.123,
"eventListeners": 71,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.800000000000182
}
]
}🎨 Storybook: ✅ Built — View Storybook🎭 Playwright: ❌ 1408 passed, 129 failed · 3 flaky❌ Failed Tests📊 Browser Reports
|
…ove-iscloud-guards-in-usecombowidget' into HEAD # Conflicts: # browser_tests/tests/sidebar/assets-filter.spec.ts
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
Remove
isCloudguards from the five L1 UI surfaces enumerated in FE-732 so the filter bar, asset browser modal, sidebar delete site, context-menu delete site, and input-asset delete throw all render and execute unconditionally against the unified/api/assetssurface.asset side panel
importedtab delete buttonbefore
after
Changes
MediaAssetFilterBar.vue— dropv-if="isCloud"on the filter button, pin:show-sort-options="true"on the settings menu, drop theisCloudimport.MediaAssetContextMenu.vue— simplifyshouldShowDeleteButton(nowassetType === 'output' || 'input'), drop theisCloudimport. Fix latent Vue Boolean prop-default-false trap by defaultingshowDeleteButtontotruevia destructure default — the previouspropAllows = showDeleteButton ?? truenever fired because Vue normalizes absent Boolean props tofalse. The bug was masked whileAssetsSidebarTabexplicitly passed the prop.useMediaAssetActions.ts— drop the!isCloudthrow insidedeleteAssetApi's input branch;isCloudimport stays because lines 298/442 (asset_hash filename selection — M5 follow-up per the Asset Identity RFC) are out of scope.AssetBrowserModal.vue— refreshoverrideAssetsdocstring (no "cloud-only" framing post-FE-730 store unification). No code branch in the modal itself.AssetsSidebarTab.vue— remove deadshouldShowDeleteButtoncomputed, twov-ifconsumers on the bulk Delete buttons, the:show-delete-buttonprop binding onMediaAssetContextMenu, and theisCloudimport.mediaAsset.deletingImportedFilesCloudOnlykey.--enable-assetsso/api/assetsis always on); until then the new code path is unreachable on OSS in production.Test coverage
8 new / extended cases across unit + e2e — see commits 2 and 3 for the reinforcement round added after the first adversarial pass.
useMediaAssetActions.test.ts— OSS + Cloud input asset deletion viaassetService.deleteAsset(asset.id)(cross-backend lock).MediaAssetContextMenu.test.ts— input-mode Delete visibility (default-true path) + explicitshowDeleteButton: falseopt-out (locks both directions of the Boolean default fix).MediaAssetFilterBar.test.ts— new component test: filter button renders unconditionally + settings menu receivesshow-sort-options=true.assetDeleteClearsLoadImage.spec.ts—@ossdescribe added beside the existing@cloudone; the full input-asset delete → widget cleanup flow now runs on both Playwright projects.assets.spec.ts—Footer shows delete button when input assets selected (FE-732)runs in the default chromium project (OSS build) and asserts the bulk Delete footer button renders on the Imported tab.Stale
cloud-onlycomments inassets-filter.spec.tsandassets-sort.spec.tsrefreshed; the@cloudtags stay because/api/jobsis still cloud-only — once OSS gets jobs the tags can drop.Gaps left intentionally:
/api/assetsend-to-end depends on BE-786.job_ids,asset_hash,include_public) light up on OSS once BE-886 + BE-891 merge — no FE change at that point.asset_hash→file_pathfilename migration inuseMediaAssetActions.ts:298/442is RFC M5 follow-up (BE-933/934 prerequisites).Live verification (Chrome CDP)
Spun up two FE-732 dev servers and drove them through Chrome DevTools Protocol to exercise the changed UI surfaces end-to-end against real backends.
Cloud regression —
https://local.comfy.org:5180/proxying tocloud.comfy.org(authenticated session, mkcert +local.comfy.orghost):/api/assets?include_tags=input)data-testid="assets-delete-selected"+data-testid="assets-download-selected")No regression observed in any Cloud flow that previously rendered.
OSS smoke —
http://localhost:5182/proxying to a local ComfyUI backend started with--enable-assets. Compares pre/post FE-732:v-if="isCloud"):show-sort-options="isCloud")activeTab === 'input' && !isCloud)assets-delete-selected)assetType === 'input' && isCloud)All four divergence-removal sites confirmed active on the OSS build. Actual DELETE was not dispatched in this run to protect the user's local asset; the menu-visibility precondition (the only piece this PR can change) is what's verified end-to-end.
Cleanup: both dev servers stopped, local-only mkcert patch reverted, worktree
git statusclean.Review Focus
MediaAssetContextMenu.showDeleteButtondefault fix is the only non-mechanical change. It is technically scope-adjacent (latent bug, not an FE-732 guard) but in-scope cleanup ofAssetsSidebarTab(removing the explicit:show-delete-button="shouldShowDeleteButton"pass-through) exposes it — without the fix, the context-menu Delete item never renders in any caller, including the existing cloud flow. The fix tightens the contract:showDeleteButton?: booleannow means "absent ⇒ show", matching the original?? trueintent. Locked by the new default-true + opt-out unit cases.job_ids,asset_hash,include_public) will start being accepted by OSS once BE-886 + BE-891 merge — no FE change at that point.isCloudremoval). This PR only refreshes the stale "cloud-only" docstring; the actual unification already shipped in FE-730.Stack
Depends on BE-786 (OSS removes
--enable-assets) before this can ship to production.