Fix cloud output deletion without deleting sibling assets - #14493
Closed
dante01yoon wants to merge 6 commits into
Closed
Fix cloud output deletion without deleting sibling assets#14493dante01yoon wants to merge 6 commits into
dante01yoon wants to merge 6 commits into
Conversation
Amp-Thread-ID: https://ampcode.com/threads/T-019fb910-e134-75f8-9e68-ab218bcabca6 Co-authored-by: Amp <amp@ampcode.com>
dosubot
Bot
added
the
size:L
📄 Knowledge reviewDosu skipped reviewing this PR because your organization has used its
|
🎨 Storybook: ✅ Built — View Storybook🎭 Playwright: ✅ 1792 passed, 0 failed📊 Browser Reports
📦 Bundle: 8.26 MB gzip 🔴 +769 BDetailsSummary
Category Glance App Entry Points — 3.64 kB (baseline 3.64 kB) • ⚪ 0 BMain entry bundles and manifests
Status: 1 added / 1 removed Graph Workspace — 1.35 MB (baseline 1.35 MB) • ⚪ 0 BGraph editor runtime, canvas, workflow orchestration
Status: 1 added / 1 removed / 1 unchanged Views & Navigation — 124 kB (baseline 124 kB) • ⚪ 0 BTop-level views, pages, and routed surfaces
Status: 13 added / 13 removed / 4 unchanged Panels & Settings — 552 kB (baseline 552 kB) • ⚪ 0 BConfiguration panels, inspectors, and settings screens
Status: 11 added / 11 removed / 16 unchanged User & Accounts — 27 kB (baseline 27 kB) • ⚪ 0 BAuthentication, profile, and account management bundles
Status: 6 added / 6 removed / 4 unchanged Editors & Dialogs — 124 kB (baseline 124 kB) • ⚪ 0 BModals, dialogs, drawers, and in-app editors
Status: 6 added / 6 removed / 1 unchanged UI Components — 72.1 kB (baseline 72.1 kB) • ⚪ 0 BReusable component library chunks
Status: 6 added / 6 removed / 9 unchanged Data & Services — 3.49 MB (baseline 3.48 MB) • 🔴 +4.61 kBStores, services, APIs, and repositories
Status: 14 added / 14 removed / 3 unchanged Utilities & Hooks — 387 kB (baseline 387 kB) • ⚪ 0 BHelpers, composables, and utility bundles
Status: 16 added / 16 removed / 20 unchanged Vendor & Third-Party — 15.7 MB (baseline 15.7 MB) • ⚪ 0 BExternal libraries and shared vendor chunks Status: 16 unchanged Other — 12.8 MB (baseline 12.8 MB) • ⚪ 0 BBundles that do not match a named category
Status: 70 added / 70 removed / 214 unchanged ⚡ Performance Report
Show regressions
All metrics
Historical variance (last 15 runs)
Trend (last 15 commits on main)
Raw data{
"timestamp": "2026-08-05T22:58:28.992Z",
"gitSha": "af3cf134d9feaa76132d3b756978f4ecb679068d",
"branch": "fix/ir-91-output-asset-delete",
"measurements": [
{
"name": "canvas-idle",
"durationMs": 2034.0529999999717,
"styleRecalcs": 10,
"styleRecalcDurationMs": 7.55,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 506.48499999999996,
"heapDeltaBytes": -18291952,
"heapUsedBytes": 45749884,
"domNodes": 20,
"jsHeapTotalBytes": 23048192,
"scriptDurationMs": 15.466000000000001,
"eventListeners": 4,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "canvas-idle",
"durationMs": 2041.3519999999608,
"styleRecalcs": 10,
"styleRecalcDurationMs": 7.017999999999998,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 455.07399999999996,
"heapDeltaBytes": 5648512,
"heapUsedBytes": 69710384,
"domNodes": 20,
"jsHeapTotalBytes": 24379392,
"scriptDurationMs": 15.467000000000002,
"eventListeners": 4,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "canvas-mouse-sweep",
"durationMs": 2108.967000000007,
"styleRecalcs": 84,
"styleRecalcDurationMs": 48.626000000000005,
"layouts": 12,
"layoutDurationMs": 3.972,
"taskDurationMs": 1140.803,
"heapDeltaBytes": -12101108,
"heapUsedBytes": 52094296,
"domNodes": -281,
"jsHeapTotalBytes": 24096768,
"scriptDurationMs": 150.46099999999998,
"eventListeners": -153,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "canvas-mouse-sweep",
"durationMs": 1855.5969999999888,
"styleRecalcs": 74,
"styleRecalcDurationMs": 34.573,
"layouts": 12,
"layoutDurationMs": 3.4530000000000003,
"taskDurationMs": 870.666,
"heapDeltaBytes": -12703976,
"heapUsedBytes": 51416396,
"domNodes": -281,
"jsHeapTotalBytes": 24096768,
"scriptDurationMs": 105.711,
"eventListeners": -153,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "canvas-zoom-sweep",
"durationMs": 1723.9849999999706,
"styleRecalcs": 30,
"styleRecalcDurationMs": 14.928999999999998,
"layouts": 6,
"layoutDurationMs": 0.5710000000000001,
"taskDurationMs": 361.712,
"heapDeltaBytes": 8707788,
"heapUsedBytes": 72676448,
"domNodes": 77,
"jsHeapTotalBytes": 24903680,
"scriptDurationMs": 17.665,
"eventListeners": 19,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66333333333332,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "canvas-zoom-sweep",
"durationMs": 1756.3580000000911,
"styleRecalcs": 33,
"styleRecalcDurationMs": 15.887000000000002,
"layouts": 6,
"layoutDurationMs": 0.6769999999999998,
"taskDurationMs": 374.492,
"heapDeltaBytes": 8716240,
"heapUsedBytes": 72684568,
"domNodes": 79,
"jsHeapTotalBytes": 24379392,
"scriptDurationMs": 21.264,
"eventListeners": 19,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "dom-widget-clipping",
"durationMs": 625.7800000000202,
"styleRecalcs": 13,
"styleRecalcDurationMs": 8.49,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 397.776,
"heapDeltaBytes": -10972388,
"heapUsedBytes": 53014720,
"domNodes": 22,
"jsHeapTotalBytes": 25690112,
"scriptDurationMs": 56.513000000000005,
"eventListeners": 2,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.699999999999818
},
{
"name": "dom-widget-clipping",
"durationMs": 606.3609999999926,
"styleRecalcs": 11,
"styleRecalcDurationMs": 6.084999999999998,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 363.36,
"heapDeltaBytes": -11079600,
"heapUsedBytes": 53097220,
"domNodes": 18,
"jsHeapTotalBytes": 24379392,
"scriptDurationMs": 53.474,
"eventListeners": 0,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.670000000000012,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "large-graph-idle",
"durationMs": 2020.1220000000149,
"styleRecalcs": 8,
"styleRecalcDurationMs": 6.255,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 672.462,
"heapDeltaBytes": 6274456,
"heapUsedBytes": 66737380,
"domNodes": -282,
"jsHeapTotalBytes": 3514368,
"scriptDurationMs": 94.958,
"eventListeners": -149,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.699999999999818
},
{
"name": "large-graph-idle",
"durationMs": 2014.0589999999747,
"styleRecalcs": 10,
"styleRecalcDurationMs": 8.084000000000001,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 666.04,
"heapDeltaBytes": 6603304,
"heapUsedBytes": 65863472,
"domNodes": -281,
"jsHeapTotalBytes": 1679360,
"scriptDurationMs": 87.008,
"eventListeners": -149,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "large-graph-pan",
"durationMs": 2190.8899999999676,
"styleRecalcs": 69,
"styleRecalcDurationMs": 13.793000000000003,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 1221.722,
"heapDeltaBytes": 4915660,
"heapUsedBytes": 65271656,
"domNodes": -284,
"jsHeapTotalBytes": 3706880,
"scriptDurationMs": 393.451,
"eventListeners": -147,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "large-graph-pan",
"durationMs": 2157.149000000004,
"styleRecalcs": 69,
"styleRecalcDurationMs": 14.124999999999998,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 1214.376,
"heapDeltaBytes": 5252984,
"heapUsedBytes": 65578712,
"domNodes": -282,
"jsHeapTotalBytes": 3706880,
"scriptDurationMs": 394.573,
"eventListeners": -149,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66333333333332,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "large-graph-zoom",
"durationMs": 3247.068000000013,
"styleRecalcs": 66,
"styleRecalcDurationMs": 15.778,
"layouts": 60,
"layoutDurationMs": 7.643000000000001,
"taskDurationMs": 1489.4050000000002,
"heapDeltaBytes": 669584,
"heapUsedBytes": 62692128,
"domNodes": -287,
"jsHeapTotalBytes": 5873664,
"scriptDurationMs": 519.9149999999998,
"eventListeners": -153,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "large-graph-zoom",
"durationMs": 3146.335000000022,
"styleRecalcs": 64,
"styleRecalcDurationMs": 13.437999999999999,
"layouts": 60,
"layoutDurationMs": 7.643000000000001,
"taskDurationMs": 1467.779,
"heapDeltaBytes": -5871720,
"heapUsedBytes": 56167068,
"domNodes": -289,
"jsHeapTotalBytes": 6397952,
"scriptDurationMs": 507.858,
"eventListeners": -153,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "minimap-idle",
"durationMs": 2039.527000000021,
"styleRecalcs": 8,
"styleRecalcDurationMs": 6.109,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 659.5389999999999,
"heapDeltaBytes": 6811344,
"heapUsedBytes": 68068564,
"domNodes": -282,
"jsHeapTotalBytes": 2990080,
"scriptDurationMs": 88.901,
"eventListeners": -179,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "minimap-idle",
"durationMs": 2020.9299999999075,
"styleRecalcs": 8,
"styleRecalcDurationMs": 6.023999999999998,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 657.086,
"heapDeltaBytes": 6371056,
"heapUsedBytes": 66664388,
"domNodes": -282,
"jsHeapTotalBytes": 3776512,
"scriptDurationMs": 89.60300000000001,
"eventListeners": -149,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66333333333335,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "subgraph-dom-widget-clipping",
"durationMs": 577.951999999982,
"styleRecalcs": 47,
"styleRecalcDurationMs": 9.697000000000001,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 393.312,
"heapDeltaBytes": -10559168,
"heapUsedBytes": 53703736,
"domNodes": 20,
"jsHeapTotalBytes": 25952256,
"scriptDurationMs": 124.45899999999999,
"eventListeners": 6,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.663333333333338,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "subgraph-dom-widget-clipping",
"durationMs": 576.9030000000157,
"styleRecalcs": 47,
"styleRecalcDurationMs": 9.969999999999999,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 401.137,
"heapDeltaBytes": -10380312,
"heapUsedBytes": 53824428,
"domNodes": 20,
"jsHeapTotalBytes": 25690112,
"scriptDurationMs": 124.91600000000001,
"eventListeners": 6,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66333333333332,
"p95FrameDurationMs": 16.699999999999818
},
{
"name": "subgraph-idle",
"durationMs": 1997.5039999999922,
"styleRecalcs": 10,
"styleRecalcDurationMs": 6.784000000000002,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 446.498,
"heapDeltaBytes": 4910764,
"heapUsedBytes": 69151704,
"domNodes": 20,
"jsHeapTotalBytes": 25427968,
"scriptDurationMs": 12.776,
"eventListeners": 4,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "subgraph-idle",
"durationMs": 2005.4329999999254,
"styleRecalcs": 10,
"styleRecalcDurationMs": 6.568999999999998,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 490.7,
"heapDeltaBytes": -18706800,
"heapUsedBytes": 45703972,
"domNodes": 20,
"jsHeapTotalBytes": 23310336,
"scriptDurationMs": 12.653,
"eventListeners": 4,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.799999999999272
},
{
"name": "subgraph-mouse-sweep",
"durationMs": 1753.7490000000275,
"styleRecalcs": 79,
"styleRecalcDurationMs": 33.69,
"layouts": 16,
"layoutDurationMs": 4.329,
"taskDurationMs": 783.9309999999999,
"heapDeltaBytes": -18451248,
"heapUsedBytes": 45956232,
"domNodes": 20,
"jsHeapTotalBytes": 22786048,
"scriptDurationMs": 82.727,
"eventListeners": -153,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "subgraph-mouse-sweep",
"durationMs": 1721.2429999999586,
"styleRecalcs": 76,
"styleRecalcDurationMs": 33.783,
"layouts": 16,
"layoutDurationMs": 4.474,
"taskDurationMs": 766.054,
"heapDeltaBytes": -16719576,
"heapUsedBytes": 47432024,
"domNodes": -280,
"jsHeapTotalBytes": 23048192,
"scriptDurationMs": 83.239,
"eventListeners": -153,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.799999999999272
},
{
"name": "subgraph-transition-enter",
"durationMs": 1384.555999999975,
"styleRecalcs": 18,
"styleRecalcDurationMs": 27.877999999999993,
"layouts": 14,
"layoutDurationMs": 10.775,
"taskDurationMs": 907.046,
"heapDeltaBytes": 1159528,
"heapUsedBytes": 88963724,
"domNodes": 13673,
"jsHeapTotalBytes": 13107200,
"scriptDurationMs": 32.44,
"eventListeners": 2375,
"totalBlockingTimeMs": 124,
"frameDurationMs": 16.670000000000012,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "viewport-pan-sweep",
"durationMs": 8254.863999999998,
"styleRecalcs": 250,
"styleRecalcDurationMs": 38.091,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 4194.518,
"heapDeltaBytes": 10587224,
"heapUsedBytes": 69950636,
"domNodes": -282,
"jsHeapTotalBytes": 3706880,
"scriptDurationMs": 1272.199,
"eventListeners": -133,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66333333333332,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "viewport-pan-sweep",
"durationMs": 8195.373000000018,
"styleRecalcs": 250,
"styleRecalcDurationMs": 36.763999999999996,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 4218.689,
"heapDeltaBytes": 11299544,
"heapUsedBytes": 70715116,
"domNodes": -282,
"jsHeapTotalBytes": 5279744,
"scriptDurationMs": 1248.8509999999999,
"eventListeners": -133,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66333333333332,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "vue-large-graph-idle",
"durationMs": 17126.934999999947,
"styleRecalcs": 0,
"styleRecalcDurationMs": 0,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 17086.608,
"heapDeltaBytes": -54193236,
"heapUsedBytes": 167010856,
"domNodes": -8312,
"jsHeapTotalBytes": -6492160,
"scriptDurationMs": 548.258,
"eventListeners": -16389,
"totalBlockingTimeMs": 0,
"frameDurationMs": 17.776666666666642,
"p95FrameDurationMs": 16.799999999999272
},
{
"name": "vue-large-graph-idle",
"durationMs": 16916.922,
"styleRecalcs": 0,
"styleRecalcDurationMs": 0,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 16892.422000000002,
"heapDeltaBytes": -37241640,
"heapUsedBytes": 168989100,
"domNodes": -8312,
"jsHeapTotalBytes": -5447680,
"scriptDurationMs": 512.832,
"eventListeners": -16385,
"totalBlockingTimeMs": 0,
"frameDurationMs": 17.776666666666642,
"p95FrameDurationMs": 16.799999999999272
},
{
"name": "vue-large-graph-pan",
"durationMs": 20853.160000000003,
"styleRecalcs": 142,
"styleRecalcDurationMs": 17.10800000000004,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 20790.675,
"heapDeltaBytes": -29033680,
"heapUsedBytes": 176509056,
"domNodes": -8312,
"jsHeapTotalBytes": -9969664,
"scriptDurationMs": 858.71,
"eventListeners": -16383,
"totalBlockingTimeMs": 204,
"frameDurationMs": 17.77333333333336,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "vue-large-graph-pan",
"durationMs": 21122.95800000004,
"styleRecalcs": 145,
"styleRecalcDurationMs": 18.398999999999997,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 21071.642,
"heapDeltaBytes": -38769404,
"heapUsedBytes": 166908660,
"domNodes": -8312,
"jsHeapTotalBytes": -7544832,
"scriptDurationMs": 859.4119999999999,
"eventListeners": -16379,
"totalBlockingTimeMs": 231,
"frameDurationMs": 17.776666666666642,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "workflow-execution",
"durationMs": 448.5730000000103,
"styleRecalcs": 13,
"styleRecalcDurationMs": 19.715999999999998,
"layouts": 3,
"layoutDurationMs": 0.7030000000000001,
"taskDurationMs": 118.02200000000002,
"heapDeltaBytes": 5317368,
"heapUsedBytes": 68518952,
"domNodes": 134,
"jsHeapTotalBytes": 5242880,
"scriptDurationMs": 9.819,
"eventListeners": 97,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.699999999999818
},
{
"name": "workflow-execution",
"durationMs": 466.5390000000116,
"styleRecalcs": 11,
"styleRecalcDurationMs": 17.162000000000003,
"layouts": 3,
"layoutDurationMs": 0.673,
"taskDurationMs": 100.576,
"heapDeltaBytes": 5073840,
"heapUsedBytes": 68375908,
"domNodes": 119,
"jsHeapTotalBytes": 5505024,
"scriptDurationMs": 9.982,
"eventListeners": 97,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.663333333333338,
"p95FrameDurationMs": 16.700000000000728
}
]
} |
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:
📝 WalkthroughWalkthroughThe change adds cloud-aware media asset deletion, linked job-asset resolution, deleted-output history tracking, and history replacement. It updates output stacks and sidebar behavior. Tests cover cloud, OSS, service, composable, store, and browser flows. ChangesMedia asset cleanup
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant AssetsSidebar
participant useMediaAssetActions
participant findJobOutputAsset
participant assetService
participant assetsStore
AssetsSidebar->>useMediaAssetActions: Delete generated output
useMediaAssetActions->>findJobOutputAsset: Resolve linked job asset
findJobOutputAsset->>assetService: Get job assets
assetService-->>findJobOutputAsset: Matching asset
findJobOutputAsset-->>useMediaAssetActions: Asset record
useMediaAssetActions->>assetService: Delete asset
useMediaAssetActions->>assetsStore: Mark deleted output and refresh history
assetsStore-->>AssetsSidebar: Updated picker state
Possibly related PRs
Suggested labels: Suggested reviewers: Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 inconclusive)
✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/platform/assets/composables/useMediaAssetActions.ts (1)
108-133: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winGroup cloud output deletions by resolved
jobIdbefore callinggetJobAssetIds.
deleteAssetsaccepts arrays and deletes withPromise.allSettled(assetArray.map((asset) => deleteAssetApi(...))), so selecting multiple outputs from the same job callsassetService.getJobAssetIds(jobId)for each selected asset under the same job.getJobAssetIdspages using server offsets and can fail later pages withInvalid job assets pagination offsetif another concurrent cleanup call deletes assets before the later page fetch resolves. ResolvejobIdfirst, callgetJobAssetIdsonce per unique cloud output job, and mark all assets that share that job based on that one cleanup result.🤖 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/composables/useMediaAssetActions.ts` around lines 108 - 133, The deleteAssets flow should group cloud output and temp assets by their resolved jobId before cleanup. Update deleteAssetApi and its caller so getJobAssetIds is invoked once per unique job, then apply that single deletion result and deleteHistoryJob outcome to all selected assets sharing the job; preserve individual input-asset deletion behavior and existing error handling.
🤖 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.
Inline comments:
In `@src/platform/assets/services/assetService.ts`:
- Around line 882-921: Add the same MAX_PAGINATION_BATCHES guard used by
getAllAssetsByTag to the getJobAssetIds pagination loop, tracking batches before
each request and stopping with the existing truncated-list behavior when the
limit is reached. Preserve the current offset validation, response handling, and
normal completion paths.
---
Outside diff comments:
In `@src/platform/assets/composables/useMediaAssetActions.ts`:
- Around line 108-133: The deleteAssets flow should group cloud output and temp
assets by their resolved jobId before cleanup. Update deleteAssetApi and its
caller so getJobAssetIds is invoked once per unique job, then apply that single
deletion result and deleteHistoryJob outcome to all selected assets sharing the
job; preserve individual input-asset deletion behavior and existing error
handling.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: b6d3ac5d-6ecc-4919-9f14-cd1b016b3693
📒 Files selected for processing (4)
src/platform/assets/composables/useMediaAssetActions.test.tssrc/platform/assets/composables/useMediaAssetActions.tssrc/platform/assets/services/assetService.test.tssrc/platform/assets/services/assetService.ts
Codecov Report❌ Patch coverage is @@ Coverage Diff @@
## main #14493 +/- ##
==========================================
- Coverage 78.71% 78.66% -0.05%
==========================================
Files 1804 1803 -1
Lines 107023 109336 +2313
Branches 30683 33828 +3145
==========================================
+ Hits 84238 86011 +1773
- Misses 22330 22846 +516
- Partials 455 479 +24
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 219 files with indirect coverage changes 🚀 New features to boost your workflow:
|
dante01yoon
added
needs-backport
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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.
Inline comments:
In `@browser_tests/tests/sidebar/assetsSidebarTab.spec.ts`:
- Around line 243-263: Type both mocked route payloads: annotate the jobs
response in the `/api/jobs` handler with the existing job list response type,
and annotate the job-assets payload in the corresponding job-assets route using
the shape derived from the service schema. Apply the `satisfies` annotations to
the objects passed to `route.fulfill()` while preserving the current response
values and pagination behavior.
In `@src/platform/assets/composables/useMediaAssetActions.ts`:
- Around line 120-134: Update deleteAssets to derive deletedValues from all
resolved output asset items when grouped job deletion runs, not only the
selected assetArray entries. Reuse the asset IDs returned by
assetService.getJobAssetIds(jobId) to find the corresponding [output] widget
values and preview caches, while preserving the existing selected-item behavior
for non-grouped deletions.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: eb8a01d2-31a8-4d5c-9f44-fcea95e62487
📒 Files selected for processing (5)
browser_tests/tests/sidebar/assetsSidebarTab.spec.tssrc/platform/assets/composables/useMediaAssetActions.test.tssrc/platform/assets/composables/useMediaAssetActions.tssrc/platform/assets/services/assetService.test.tssrc/platform/assets/services/assetService.ts
Co-authored-by: Amp <amp@ampcode.com> Amp-Thread-ID: https://ampcode.com/threads/T-019fb9b3-7fcf-705f-93b9-c7ff0b98a7ee
dante01yoon
force-pushed
the
fix/ir-91-output-asset-delete
branch
from
July 31, 2026 20:46
5c5466d to
2e51602
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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.
Inline comments:
In `@browser_tests/tests/sidebar/assetsSidebarTab.spec.ts`:
- Line 425: Update the route pattern in the page.route call to use
**/api/jobs/alpha/assets**, allowing requests both with and without query
parameters to match the mock.
- Around line 438-442: Filter both in-test routes by HTTP method: in
browser_tests/tests/sidebar/assetsSidebarTab.spec.ts#L438-L442, update the route
handler to process only DELETE requests before mutating deletedAssetIds and
outputAssetsByPage, calling route.fallback() for other methods; at
browser_tests/tests/sidebar/assetsSidebarTab.spec.ts#L460-L464, process only
POST requests before updating historyDeleteRequests and generatedJobsByPage,
falling back otherwise.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 72e99757-ec85-4833-825b-a315f53b3e4e
📒 Files selected for processing (5)
browser_tests/tests/sidebar/assetsSidebarTab.spec.tssrc/platform/assets/composables/useMediaAssetActions.test.tssrc/platform/assets/composables/useMediaAssetActions.tssrc/platform/assets/services/assetService.test.tssrc/platform/assets/services/assetService.ts
Amp-Thread-ID: https://ampcode.com/threads/T-019fba65-dc5b-74db-b535-85b9ad32854c Co-authored-by: Amp <amp@ampcode.com>
dante01yoon
changed the title
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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.
Inline comments:
In `@browser_tests/tests/sidebar/assetsSidebarTab.spec.ts`:
- Around line 518-524: Update the assertion in the test around deleteAssets to
await the sidebar refresh and verify that getAssetCardByName('multi-output-a')
is no longer visible. Remove the contradictory visibility expectation while
preserving the existing image-picker assertions that confirm the deleted output
is gone.
In `@src/platform/assets/services/assetService.test.ts`:
- Around line 552-559: Add an invalid-response test alongside the existing
getJobAssetId cases, using fetchApiMock to return a successful response with
malformed job-assets data such as a missing pagination block or non-string id,
and assert that assetService.getJobAssetId('job-1', 'selected.png') rejects with
“Invalid job assets response.”
In `@src/platform/assets/services/assetService.ts`:
- Around line 888-897: Add a JSDoc block immediately before getJobAssetId
documenting its parameters, fail-closed null-return cases, and that transport,
schema, and pagination faults are thrown. Explicitly state that callers must
treat null as “do not delete” for deleteAssetApi, and that assetName only
verifies the uniquely matched hash rather than selecting among duplicate hashes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: db4616a2-aac9-454a-acf5-0b5f51981f82
📒 Files selected for processing (5)
browser_tests/tests/sidebar/assetsSidebarTab.spec.tssrc/platform/assets/composables/useMediaAssetActions.test.tssrc/platform/assets/composables/useMediaAssetActions.tssrc/platform/assets/services/assetService.test.tssrc/platform/assets/services/assetService.ts
christian-byrne
requested changes
Aug 5, 2026
christian-byrne
left a comment
christian-byrne
left a comment
Contributor
There was a problem hiding this comment.
Ran a multi-reviewer pass over this (correctness + test lenses) and verified every claim against source before writing it up. The direction is right and the lifecycle semantics are correct — deleting the asset and leaving the job alone is exactly what we want, and the Promise.allSettled → serial-loop rewrite quietly fixes a real pre-existing index-misalignment bug in the failure-logging path. Thank you for that.
But there are two blocking problems, and the second one is why the first wasn't caught.
1. Blocking — the hash comparison can never match, so cloud deletion always fails
useMediaAssetActions.ts computes const assetHash = asset.hash ?? asset.name.
Sidebar cards come from mapTaskOutputToAssetItem (src/platform/assets/composables/media/assetMappers.ts:39-51), which never sets hash. So assetHash is always output.filename — e.g. ComfyUI_00012_.png.
It's then compared against the hash field of GET /api/jobs/{job_id}/assets, which is blake3:-prefixed. This repo already documents that, in src/platform/assets/utils/outputAssetUtil.ts:138-141:
The endpoint's
hashis deliberately not copied: downstreamgetAssetUrlFilenametreatshashas the storage filename, which the endpoint's (blake3:-prefixed) value is not.
Corroborated by the existing fixture at src/platform/remote/comfyui/jobs/fetchJobs.test.ts:416 — hash: 'blake3:abc' (and null at :475).
So: 'blake3:9af3c1' === 'ComfyUI_00012_.png' → 0 matches → matchingAssets.length !== 1 → null → deleteAssetApi throws → "Failed to delete asset". Nothing is ever deleted on the cloud sidebar path. That trades Incident 91's "deletes the wrong thing" for "deletes nothing", and it fails closed silently.
There's already a resolver for exactly this in the codebase — findOutputAsset() (src/platform/assets/utils/assetPreviewUtil.ts:41-51) goes filename → asset record via GET /api/assets?hash=<filename> (the server normalizes both hash representations) with a name_contains fallback. One request, no pagination, no ambiguity logic.
Separately worth noting: for expanded stack children the real asset UUID is already in hand — overlayJobAsset (outputAssetUtil.ts:144-153) sets id: asset.id. The branch discriminator is "does user_metadata exist", so those items take the lookup path anyway and discard an id we already resolved correctly.
2. Blocking — the deleted output stays visible in Media Assets
browser_tests/tests/sidebar/assetsSidebarTab.spec.ts:522 asserts the card is still visible after deletion, then :526-530 asserts the same output is gone from the Load Image picker. Both pass, and they're not contradictory — they're describing two surfaces that genuinely disagree:
| Surface | Composable | Endpoint |
|---|---|---|
| Media Assets → Generated | useAssetsApi('output') (AssetsSidebarTab.vue:299) |
/api/jobs |
| Load Image / Load Video | useFlatOutputAssets() (WidgetSelectDropdown.vue:63) |
/api/assets |
mapHistoryToAssets builds every sidebar card from the JobListItem, and cloud serves preview_output/outputs_count from the job table (services/ingest/server/implementation/job.go:601). So the job surviving — which is correct per the lifecycle decision — means the sidebar keeps rendering the output, and a hard refresh doesn't clear it.
That's the mirror image of the original bug, in the panel the user clicked delete in, and it misses the incident's own acceptance criterion: "Deleted output no longer appears in Media Assets, Load Image, or Load Video after refresh and a new session."
This isn't a reason to abandon the approach — the data-level fix is right and strictly better than today. But it needs either an optimistic client-side removal from historyAssets after a successful delete, or /api/jobs needing to stop listing outputs whose asset row is gone (worth folding into the histories.outputs eviction work rather than doing standalone). To be explicit, that is not the pattern we rejected in Comfy-Org/cloud#5864 — that made asset listings depend on job state; this is the inverse, and it's legitimate.
Why CI is green
The fixtures encode a contract that contradicts the documented one. hash: 'multi-output-a.png' in the e2e mock (:446, :454) and hash: 'first.png'/'shared-hash' in assetService.test.ts are all filename-shaped, so the comparison in #1 matches in tests and can't in production. And the jobs mock never removes the deleted output, so #2 is locked in as expected behavior.
Also worth fixing
- Schema is stricter than the endpoint. The new
jobAssetsResponseSchemarequirespaginationand all four sub-fields non-nullable. The existing schema for the same endpoint —zJobAssetsResponse(src/platform/remote/comfyui/jobs/jobTypes.ts:127-138) — makes them optional/nullable, with a comment that the cloud serialiser drops null fields (exclude_none=True). A response withoutpaginationthrows here whileenrichWithJobAssetshandles it fine. - Duplicate implementation.
fetchJobAssets(src/platform/remote/comfyui/jobs/fetchJobs.ts:202-244) already paginates this endpoint, with a 20-page cap and an LRU cache (src/services/jobOutputCache.ts:127). This adds a second one with a 1000-batch cap and no cache. Two implementations of one unconfirmed contract will drift — and a backend wrongly pinninghas_more: truewould fire 1000 sequential requests per delete click. jobAssetsCacheis never invalidated after delete, so collapsing and re-expanding a stack re-shows the deleted child.assetNamenever disambiguates. It's only checked after narrowing to exactly one match, so two same-hash siblings are permanently undeletable rather than resolved by name. Both tests atassetService.test.ts:498and:514hit the earlierlength !== 1guard, so line 947 is never exercised — rewriting it toreturn asset.idpasses all 10 tests.- 404-as-success is too broad. It now covers the
deleteAsset(asset.id)fallback path, whereasset.idmay be a history-composite id — that 404s, gets counted as success, and shows "deleted successfully" while the asset is still live. - 409
ASSET_IN_USE(asset pinned by a workflow version) surfaces only as the generic failure toast — indistinguishable from a network error. - Route mocks don't filter by HTTP method, unlike
mockViewFiles/mockInputFilesin the same file.**/api/assets/multi-output-a-asset-idmatches any verb, so a futureGET/PUTon that id would be recorded as a deletion. - Uncovered: bulk partial-failure (criterion 7) entirely, Load Video, post-refresh/new-session, malformed-response and non-404 HTTP errors in
getJobAssetId.
Happy to take the findOutputAsset rewrite myself if that's easier — say the word. Not resolving the existing threads since the code they point at is still current.
Amp-Thread-ID: https://ampcode.com/threads/T-019fd2f7-28eb-73de-ae04-24e2965f3c61 Co-authored-by: Amp <amp@ampcode.com>
Contributor
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/platform/assets/services/assetService.ts (1)
850-867: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDocument the 404 no-op in the
deleteAssetcontract.The JSDoc still states
@throws Error if deletion fails. After this change a 404 resolves successfully. Callers treat a resolved promise as proof of deletion, so record that a 404 is accepted as "already absent" and does not confirm the record existed.♻️ Proposed documentation
/** * Deletes an asset by ID * Only available in cloud environment * + * Idempotent: a 404 response resolves successfully because the record is + * already absent. A resolved call therefore does not prove the record + * existed under the supplied id. + * * `@param` id - The asset ID (UUID) * `@returns` Promise<void> - * `@throws` Error if deletion fails + * `@throws` Error if the server returns a non-404 failure status */🤖 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/services/assetService.ts` around lines 850 - 867, Update the JSDoc for deleteAsset to document that a 404 resolves successfully as an already-absent asset and does not confirm the record existed, while retaining the `@throws` description for other deletion failures.
♻️ Duplicate comments (1)
browser_tests/tests/sidebar/assetsSidebarTab.spec.ts (1)
267-277: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winType the mocked jobs and job-assets payloads.
The assets route at Lines 250-256 uses
satisfies ListAssetsResponse. The jobs payload here and the job-assets payload at Lines 293-298 remain untyped object literals. The job-assets payload sendspagination: { has_more: false }withoutoffset,limit, ortotal. If the production schema requires those fields, the mock drifts from the contract and the failure surfaces only at run time.Add
satisfiesannotations to both payloads, derived from the job list and job-assets response types.As per coding guidelines: type mocked API responses in
route.fulfill().Also applies to Lines 293-298.
🤖 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/sidebar/assetsSidebarTab.spec.ts` around lines 267 - 277, Type both mocked response payloads in the jobs and job-assets route.fulfill handlers using satisfies annotations with the corresponding job-list and job-assets response types. Include all required pagination fields, including offset, limit, and total, in the job-assets payload so both mocks conform to their production contracts.Source: Coding guidelines
🤖 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.
Inline comments:
In `@browser_tests/tests/sidebar/assetsSidebarTab.spec.ts`:
- Line 282: Update the route glob in the multi-output assets handler passed to
page.route so it uses **/api/jobs/multi-output/assets**, matching requests both
with and without query parameters while preserving the existing mock behavior.
- Around line 502-506: After pressing Escape in the image-picker flow, wait for
the picker itself to be hidden before calling assetsTab.open(). Update the
imagePicker locator to target the picker-specific element rather than the broad
page.getByRole('dialog') locator, so it remains unambiguous when the confirm
dialog appears later.
- Around line 522-525: Move the historyDeleteRequests assertion to after the
retrying sidebar assertions that settle the deletion flow, preserving the
existing empty-array expectation. Keep the deletedAssetIds poll unchanged and
ensure the negative history-request check runs only after the sidebar end state
is confirmed.
In `@src/platform/assets/composables/useMediaAssetActions.ts`:
- Around line 728-731: Update the hasOutputAssets calculation in the asset
deletion flow to include only assets whose corresponding results entry has
status "fulfilled", matching the hasInputAssets logic. Preserve the existing
output/temp type check so updateHistory and updateFlatOutputs run only when a
successful deletion includes an output or temporary asset.
- Around line 756-804: Wrap each per-job reconciliation body in the loop over
deletedOutputKeysByJob with its own try/catch, including resolveOutputAssetItems
and subsequent replacement logic. On failure, swallow or log the reconciliation
error and continue processing remaining jobs so the outer cleanup
steps—markDeletedAssetsAsMissingMedia, clearNodePreviewCacheForValues,
clearDeletedAssetWidgetValues, and invalidateModelsForCategory—always execute;
preserve the existing successful reconciliation behavior.
In `@src/platform/assets/composables/useOutputStacks.test.ts`:
- Around line 231-234: Update the mocks in the relevant useOutputStacks test to
use a reactive Set matching the production deletedOutputKeysByJob ref, then
remove the assets reassignment so adding the deletion key alone triggers
assetItems recomputation. Use a settled or retrying assertion for the reactive
refresh, and ensure the test title reflects that it verifies deletion-driven
updates rather than an asset-list refresh.
In `@src/platform/assets/composables/useOutputStacks.ts`:
- Around line 66-75: Extract the shared output-key computation from
isDeletedOutput and the delete action into a helper in outputAssetUtil.ts,
preserving the getOutputKey(...) fallback to the asset filename. Update both
call sites to use this helper, while leaving excludeOutputKey’s ?? undefined
behavior unchanged.
In `@src/platform/assets/utils/outputAssetUtil.ts`:
- Around line 177-182: Replace the `nodeScopedMatches` comparison with a direct
comparison of normalized node IDs, adding the proposed `normalizedNodeId` helper
alongside `nodeScopedName`. Preserve the existing uniqueness guard so exactly
one matching node-scoped asset resolves, including the intentional
null-versus-null match for node-id-less entries.
In `@src/stores/assetsStore.test.ts`:
- Around line 635-657: The replaceHistoryAsset fallback lacks behavioral
coverage. In src/stores/assetsStore.test.ts:635-657, add a test calling
replaceHistoryAsset(jobId) without a replacement and assert the corresponding
row is removed from historyAssets. In
src/platform/assets/composables/useMediaAssetActions.test.ts:1447-1498, add a
single-output deletion test asserting replaceHistoryAsset is called with 'job-1'
and undefined.
---
Outside diff comments:
In `@src/platform/assets/services/assetService.ts`:
- Around line 850-867: Update the JSDoc for deleteAsset to document that a 404
resolves successfully as an already-absent asset and does not confirm the record
existed, while retaining the `@throws` description for other deletion failures.
---
Duplicate comments:
In `@browser_tests/tests/sidebar/assetsSidebarTab.spec.ts`:
- Around line 267-277: Type both mocked response payloads in the jobs and
job-assets route.fulfill handlers using satisfies annotations with the
corresponding job-list and job-assets response types. Include all required
pagination fields, including offset, limit, and total, in the job-assets payload
so both mocks conform to their production contracts.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 9a871c26-57dd-4c82-91cd-0d8beacb44ea
📒 Files selected for processing (11)
browser_tests/tests/sidebar/assetsSidebarTab.spec.tssrc/platform/assets/composables/useMediaAssetActions.test.tssrc/platform/assets/composables/useMediaAssetActions.tssrc/platform/assets/composables/useOutputStacks.test.tssrc/platform/assets/composables/useOutputStacks.tssrc/platform/assets/services/assetService.test.tssrc/platform/assets/services/assetService.tssrc/platform/assets/utils/outputAssetUtil.test.tssrc/platform/assets/utils/outputAssetUtil.tssrc/stores/assetsStore.test.tssrc/stores/assetsStore.ts
Amp-Thread-ID: https://ampcode.com/threads/T-019fd2f7-28eb-73de-ae04-24e2965f3c61 Co-authored-by: Amp <amp@ampcode.com>
christian-byrne
requested changes
Aug 8, 2026
christian-byrne
left a comment
christian-byrne
left a comment
Contributor
There was a problem hiding this comment.
This PR is somewhat outdated because we are moving the asset sidebar to be asset-driven rather than job-driven now. Also, we decided that deleting an asset without deleting the job has potential value. To compensate, we will explore some tooltips/warnings about the behavior (if the user expects deleting an asset also deletes the job)
Contributor
|
Closing this — the approach has been overtaken by decisions made after it was opened, not because the work was wrong. Thanks @dante01yoon for two solid rounds on it, and sorry it landed this way. Why it's superseded. The Domain Modeling Weekly on 2026-08-06 decided the Media Assets sidebar becomes asset-backed rather than jobs-backed (FE-1442, now owned by @AustinMroz, tracked under the "Assets sidebar is assets-backed" milestone). Both blockers I raised in the 08-05 review dissolve under that change rather than needing fixes here:
And the incident itself is being closed from the backend. Simon's RFC (Asset deletion should be soft and backfilled from job deletion, 2026-08-07) adds What should carry forward, and shouldn't be lost in the close:
Also worth recording, since it was found here: CI was green throughout because every fixture uses filename-shaped hashes ( Follow-on work lives in FE-1520 (gate deletion on the |
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
Fix IR-91 without widening an individual output deletion into a job-wide deletion.
A cloud-generated card in the Assets sidebar is history-backed, while Load Image options are asset-record-backed. Deleting only history caused the original stale Load Image option. The earlier implementation in this PR went too far in the other direction: it deleted every asset linked to the job and then deleted history, so deleting one output from a multi-output job removed its siblings.
This revision resolves the selected output to one exact asset UUID, deletes only that asset record, and preserves both sibling assets and the job/history record.
Root cause
The two UI surfaces currently read from different sources:
/api/assetsrecordsConsequently, a history-only deletion cannot remove the Load Image option, while a job-wide cleanup destroys unrelated outputs.
AS IS
The first version of this PR replaced the stale-record bug with a broader destructive flow:
TO BE
flowchart TD A[User deletes one cloud-generated output] --> B[Read its job ID and content hash] B --> C[Fetch every page from GET /api/jobs/:jobId/assets] C --> D{Exactly one hash match and optional name is consistent?} D -- Yes --> E[Resolve the exact asset UUID] E --> F[DELETE /api/assets/:assetId] F --> G[Refresh history and flat-output stores] G --> H[Selected Load Image option disappears] G --> I[Sibling assets remain] G --> J[Job and history remain] D -- No or endpoint failure --> K[Fail closed] K --> L[Delete no sibling asset and no history]Implementation
GET /api/jobs/{jobId}/assetsusing the selected output's content hash.display_nameas an additional consistency check, never as permission to choose between duplicate hashes./api/assets/{assetId}record./api/historyfor an individual cloud output deletion.Scope and known limitation
This PR fixes the IR-91 Load Image inconsistency and prevents sibling loss. It does not migrate the Assets sidebar to asset records.
Because the generated sidebar card is still reconstructed from the preserved history job, the deleted output can remain visible in the sidebar even though its asset record and Load Image option are gone. FE-1442 is the follow-up required to make the sidebar asset-backed (or to provide a backend per-output history mutation) so the card itself can disappear while the job and surviving outputs remain.
Also out of scope:
Regression coverage
The browser regression now models a two-output job and verifies:
DELETE /api/assets/:assetId;/api/historyis not called;Unit coverage additionally verifies pagination, unstable pagination rejection, duplicate-hash ambiguity, optional filename consistency, endpoint unavailability, repeated DELETE idempotency, cloud/OSS routing, and store refresh behavior.
Visual evidence
AS IS
Before deletion, the generated output appears in the real Load Image picker.
TO BE (single-output visual flow)
After deleting the only output, the real Load Image picker no longer contains it. For a multi-output job, only the selected option is removed and siblings remain.
Existing real-app flow
The visual capture uses the real app UI. API responses are mocked, so it demonstrates the user-facing flow rather than live backend deletion semantics.
Validation
pnpm test:unit src/platform/assets/composables/useMediaAssetActions.test.ts src/platform/assets/services/assetService.test.ts— 122 tests passedpnpm typecheck— passedpnpm typecheck:browser— passedoxlinton all modified files — passedoxfmt --checkon all modified files — passedknip --cache— passedgit diff --check— passedThe updated two-output Playwright regression is committed, but a standalone local Vite run could not reach the assertion because the cloud fixture setup requires the backend
/api/usersendpoint. This PR therefore proves frontend request selection and state behavior through unit tests and typed browser mocks; it does not claim live-backend E2E coverage.