test(assets): e2e coverage for the assets-mode model library sidebar - #13574
Merged
Conversation
Contributor
📝 WalkthroughWalkthroughAdds browser-test infrastructure and coverage for model library asset mode, seed rescans, feature flags, routing, model placement, and bare-tag backends. It also updates a graph regression assertion to use branded link IDs. ChangesModel library asset and routing coverage
Graph link assertion alignment
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Sidebar
participant AssetHelper
participant AppAPI
Sidebar->>AssetHelper: POST /assets/seed
AssetHelper-->>Sidebar: status started
AppAPI-->>Sidebar: assets.seed.fast_complete
Sidebar-->>Sidebar: refresh tree and active search results
Possibly related PRs
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 unit tests (beta)
Comment |
synap5e
added
the
cursor-review
synap5e
marked this pull request as ready for review
July 10, 2026 11:24
🎭 Playwright: ✅ 1757 passed, 0 failed📊 Browser Reports
📦 Bundle: 8.13 MB gzip 🔴 +345 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.28 MB (baseline 1.28 MB) • ⚪ 0 BGraph editor runtime, canvas, workflow orchestration
Status: 1 added / 1 removed / 1 unchanged Views & Navigation — 112 kB (baseline 112 kB) • ⚪ 0 BTop-level views, pages, and routed surfaces
Status: 12 added / 12 removed / 4 unchanged Panels & Settings — 551 kB (baseline 551 kB) • ⚪ 0 BConfiguration panels, inspectors, and settings screens
Status: 11 added / 11 removed / 15 unchanged User & Accounts — 29.1 kB (baseline 29.1 kB) • ⚪ 0 BAuthentication, profile, and account management bundles
Status: 7 added / 7 removed / 3 unchanged Editors & Dialogs — 121 kB (baseline 121 kB) • ⚪ 0 BModals, dialogs, drawers, and in-app editors
Status: 5 added / 5 removed / 1 unchanged UI Components — 64.6 kB (baseline 64.6 kB) • ⚪ 0 BReusable component library chunks
Status: 6 added / 6 removed / 8 unchanged Data & Services — 3.4 MB (baseline 3.4 MB) • 🔴 +1.07 kBStores, services, APIs, and repositories
Status: 14 added / 14 removed / 3 unchanged Utilities & Hooks — 367 kB (baseline 367 kB) • ⚪ 0 BHelpers, composables, and utility bundles
Status: 15 added / 15 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.5 MB (baseline 12.5 MB) • ⚪ 0 BBundles that do not match a named category
Status: 69 added / 69 removed / 208 unchanged ⚡ Performance Report
Show regressions
All metrics
Historical variance (last 15 runs)
Trend (last 15 commits on main)
Raw data{
"timestamp": "2026-07-28T09:04:30.665Z",
"gitSha": "63ade8038324b868ed4688f8d3c3a188e0f973eb",
"branch": "synap5e/test/assets-sidebar-e2e",
"measurements": [
{
"name": "canvas-idle",
"durationMs": 2121.4089999999715,
"styleRecalcs": 10,
"styleRecalcDurationMs": 9.033000000000001,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 586.517,
"heapDeltaBytes": 3518284,
"heapUsedBytes": 70749100,
"domNodes": 20,
"jsHeapTotalBytes": 20844544,
"scriptDurationMs": 24.462000000000003,
"eventListeners": 4,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66333333333332,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "canvas-idle",
"durationMs": 2037.8670000000056,
"styleRecalcs": 9,
"styleRecalcDurationMs": 8.866000000000001,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 513.626,
"heapDeltaBytes": 3459288,
"heapUsedBytes": 71422916,
"domNodes": 18,
"jsHeapTotalBytes": 20844544,
"scriptDurationMs": 22.203,
"eventListeners": 4,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66333333333332,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "canvas-mouse-sweep",
"durationMs": 2107.797000000005,
"styleRecalcs": 82,
"styleRecalcDurationMs": 55.42700000000001,
"layouts": 12,
"layoutDurationMs": 4.382000000000001,
"taskDurationMs": 1084.703,
"heapDeltaBytes": -16463652,
"heapUsedBytes": 51487704,
"domNodes": -271,
"jsHeapTotalBytes": 21233664,
"scriptDurationMs": 145.15200000000002,
"eventListeners": -148,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "canvas-mouse-sweep",
"durationMs": 1942.7239999999983,
"styleRecalcs": 78,
"styleRecalcDurationMs": 45.458,
"layouts": 12,
"layoutDurationMs": 3.934,
"taskDurationMs": 969.249,
"heapDeltaBytes": -15640904,
"heapUsedBytes": 52315732,
"domNodes": -272,
"jsHeapTotalBytes": 20185088,
"scriptDurationMs": 129.502,
"eventListeners": -148,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66333333333335,
"p95FrameDurationMs": 16.699999999999818
},
{
"name": "canvas-zoom-sweep",
"durationMs": 1754.173000000037,
"styleRecalcs": 32,
"styleRecalcDurationMs": 18.174000000000003,
"layouts": 6,
"layoutDurationMs": 0.644,
"taskDurationMs": 390.388,
"heapDeltaBytes": 7337264,
"heapUsedBytes": 75185396,
"domNodes": 79,
"jsHeapTotalBytes": 19795968,
"scriptDurationMs": 20.727999999999998,
"eventListeners": 19,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.699999999999818
},
{
"name": "canvas-zoom-sweep",
"durationMs": 1754.0780000000495,
"styleRecalcs": 31,
"styleRecalcDurationMs": 19.139,
"layouts": 6,
"layoutDurationMs": 0.685,
"taskDurationMs": 408.971,
"heapDeltaBytes": 7204976,
"heapUsedBytes": 75125524,
"domNodes": 77,
"jsHeapTotalBytes": 20582400,
"scriptDurationMs": 23.807,
"eventListeners": 19,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66333333333332,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "dom-widget-clipping",
"durationMs": 628.8499999999999,
"styleRecalcs": 11,
"styleRecalcDurationMs": 8.266999999999998,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 406.065,
"heapDeltaBytes": -12003740,
"heapUsedBytes": 55825552,
"domNodes": 18,
"jsHeapTotalBytes": 21106688,
"scriptDurationMs": 59.24199999999999,
"eventListeners": 2,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.699999999999818
},
{
"name": "dom-widget-clipping",
"durationMs": 646.9859999999699,
"styleRecalcs": 13,
"styleRecalcDurationMs": 10.16,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 423.793,
"heapDeltaBytes": -12605332,
"heapUsedBytes": 55522992,
"domNodes": 22,
"jsHeapTotalBytes": 21630976,
"scriptDurationMs": 59.952999999999996,
"eventListeners": 2,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "large-graph-idle",
"durationMs": 2028.4010000000308,
"styleRecalcs": 7,
"styleRecalcDurationMs": 6.109999999999998,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 662.8810000000001,
"heapDeltaBytes": 6646256,
"heapUsedBytes": 66013872,
"domNodes": -276,
"jsHeapTotalBytes": 4263936,
"scriptDurationMs": 93.389,
"eventListeners": -144,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.670000000000012,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "large-graph-idle",
"durationMs": 2023.5440000000153,
"styleRecalcs": 8,
"styleRecalcDurationMs": 8.156,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 747.215,
"heapDeltaBytes": 5734568,
"heapUsedBytes": 64547216,
"domNodes": -276,
"jsHeapTotalBytes": 4526080,
"scriptDurationMs": 117.49000000000001,
"eventListeners": -144,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "large-graph-pan",
"durationMs": 2168.290000000013,
"styleRecalcs": 69,
"styleRecalcDurationMs": 15.621999999999996,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 1335.3139999999999,
"heapDeltaBytes": 5034332,
"heapUsedBytes": 65396216,
"domNodes": -278,
"jsHeapTotalBytes": 5517312,
"scriptDurationMs": 434.267,
"eventListeners": -142,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66333333333332,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "large-graph-pan",
"durationMs": 2251.0019999999713,
"styleRecalcs": 69,
"styleRecalcDurationMs": 17.531,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 1443.362,
"heapDeltaBytes": 1551296,
"heapUsedBytes": 61423188,
"domNodes": -279,
"jsHeapTotalBytes": 4993024,
"scriptDurationMs": 454.175,
"eventListeners": -144,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "large-graph-zoom",
"durationMs": 3259.883000000002,
"styleRecalcs": 64,
"styleRecalcDurationMs": 15.735000000000003,
"layouts": 60,
"layoutDurationMs": 8.683,
"taskDurationMs": 1607.5140000000001,
"heapDeltaBytes": 1280688,
"heapUsedBytes": 63228836,
"domNodes": -282,
"jsHeapTotalBytes": 7671808,
"scriptDurationMs": 556.636,
"eventListeners": -148,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "large-graph-zoom",
"durationMs": 3411.330000000021,
"styleRecalcs": 64,
"styleRecalcDurationMs": 16.677000000000003,
"layouts": 60,
"layoutDurationMs": 9.199,
"taskDurationMs": 1731.853,
"heapDeltaBytes": 8777580,
"heapUsedBytes": 70201100,
"domNodes": -283,
"jsHeapTotalBytes": 7147520,
"scriptDurationMs": 599.4300000000001,
"eventListeners": -148,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "minimap-idle",
"durationMs": 2023.253000000011,
"styleRecalcs": 9,
"styleRecalcDurationMs": 8.240999999999998,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 548.43,
"heapDeltaBytes": 6222072,
"heapUsedBytes": 66535736,
"domNodes": -275,
"jsHeapTotalBytes": 5050368,
"scriptDurationMs": 95.56800000000001,
"eventListeners": -144,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66333333333335,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "minimap-idle",
"durationMs": 2018.2179999999335,
"styleRecalcs": 8,
"styleRecalcDurationMs": 8.005000000000003,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 731.905,
"heapDeltaBytes": 5982868,
"heapUsedBytes": 66449492,
"domNodes": -276,
"jsHeapTotalBytes": 5050368,
"scriptDurationMs": 115.262,
"eventListeners": -144,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66333333333335,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "subgraph-dom-widget-clipping",
"durationMs": 611.8710000000078,
"styleRecalcs": 47,
"styleRecalcDurationMs": 11.526000000000002,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 418.43500000000006,
"heapDeltaBytes": -11473124,
"heapUsedBytes": 56481884,
"domNodes": 20,
"jsHeapTotalBytes": 21630976,
"scriptDurationMs": 126.26599999999999,
"eventListeners": 8,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "subgraph-dom-widget-clipping",
"durationMs": 647.4420000000691,
"styleRecalcs": 48,
"styleRecalcDurationMs": 12.496,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 441.22299999999996,
"heapDeltaBytes": -12241680,
"heapUsedBytes": 55948536,
"domNodes": 22,
"jsHeapTotalBytes": 22417408,
"scriptDurationMs": 129.267,
"eventListeners": 8,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "subgraph-idle",
"durationMs": 2007.2789999999827,
"styleRecalcs": 8,
"styleRecalcDurationMs": 7.283999999999999,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 512.5649999999999,
"heapDeltaBytes": 3410828,
"heapUsedBytes": 71455932,
"domNodes": 16,
"jsHeapTotalBytes": 21106688,
"scriptDurationMs": 18.79,
"eventListeners": 4,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.799999999999272
},
{
"name": "subgraph-idle",
"durationMs": 2019.9210000000676,
"styleRecalcs": 9,
"styleRecalcDurationMs": 8.943999999999999,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 532.645,
"heapDeltaBytes": 3474200,
"heapUsedBytes": 71727028,
"domNodes": 18,
"jsHeapTotalBytes": 20582400,
"scriptDurationMs": 21.010000000000005,
"eventListeners": 4,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66333333333335,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "subgraph-mouse-sweep",
"durationMs": 1677.3220000000038,
"styleRecalcs": 75,
"styleRecalcDurationMs": 38.171,
"layouts": 16,
"layoutDurationMs": 4.792,
"taskDurationMs": 779.384,
"heapDeltaBytes": -5715300,
"heapUsedBytes": 62261448,
"domNodes": 63,
"jsHeapTotalBytes": 20582400,
"scriptDurationMs": 95.158,
"eventListeners": 4,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66333333333332,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "subgraph-mouse-sweep",
"durationMs": 1806.6430000000082,
"styleRecalcs": 78,
"styleRecalcDurationMs": 46.650999999999996,
"layouts": 16,
"layoutDurationMs": 5.24,
"taskDurationMs": 913.9900000000001,
"heapDeltaBytes": -21251344,
"heapUsedBytes": 46700140,
"domNodes": -275,
"jsHeapTotalBytes": 20447232,
"scriptDurationMs": 106.229,
"eventListeners": -148,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "subgraph-transition-enter",
"durationMs": 1403.8140000000112,
"styleRecalcs": 18,
"styleRecalcDurationMs": 30.805,
"layouts": 14,
"layoutDurationMs": 11.763,
"taskDurationMs": 1018.2270000000002,
"heapDeltaBytes": 31427300,
"heapUsedBytes": 99426244,
"domNodes": 13673,
"jsHeapTotalBytes": 13107200,
"scriptDurationMs": 39.287,
"eventListeners": 2371,
"totalBlockingTimeMs": 124,
"frameDurationMs": 16.66999999999998,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "viewport-pan-sweep",
"durationMs": 8176.490000000002,
"styleRecalcs": 249,
"styleRecalcDurationMs": 42.203,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 4438.506,
"heapDeltaBytes": 9905860,
"heapUsedBytes": 69317572,
"domNodes": -277,
"jsHeapTotalBytes": 7090176,
"scriptDurationMs": 1340.994,
"eventListeners": -128,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66333333333332,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "viewport-pan-sweep",
"durationMs": 8373.616999999967,
"styleRecalcs": 250,
"styleRecalcDurationMs": 45.35499999999999,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 4784.123,
"heapDeltaBytes": 1778172,
"heapUsedBytes": 60622772,
"domNodes": -278,
"jsHeapTotalBytes": 6623232,
"scriptDurationMs": 1451.461,
"eventListeners": -128,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.799999999999272
},
{
"name": "vue-large-graph-idle",
"durationMs": 18421.437999999966,
"styleRecalcs": 0,
"styleRecalcDurationMs": 0,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 18403.058,
"heapDeltaBytes": -30556024,
"heapUsedBytes": 177831612,
"domNodes": -8312,
"jsHeapTotalBytes": -11472896,
"scriptDurationMs": 581.032,
"eventListeners": -16385,
"totalBlockingTimeMs": 0,
"frameDurationMs": 17.780000000000047,
"p95FrameDurationMs": 16.799999999999272
},
{
"name": "vue-large-graph-idle",
"durationMs": 18572.875999999953,
"styleRecalcs": 0,
"styleRecalcDurationMs": 0,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 18545.383,
"heapDeltaBytes": -32596440,
"heapUsedBytes": 170383640,
"domNodes": -8312,
"jsHeapTotalBytes": -11210752,
"scriptDurationMs": 612.621,
"eventListeners": -16389,
"totalBlockingTimeMs": 0,
"frameDurationMs": 17.780000000000047,
"p95FrameDurationMs": 16.80000000000291
},
{
"name": "vue-large-graph-pan",
"durationMs": 23120.734000000026,
"styleRecalcs": 171,
"styleRecalcDurationMs": 25.89199999999997,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 23073.000999999997,
"heapDeltaBytes": -72377756,
"heapUsedBytes": 155920000,
"domNodes": -8312,
"jsHeapTotalBytes": -15929344,
"scriptDurationMs": 930.9749999999999,
"eventListeners": -16379,
"totalBlockingTimeMs": 542,
"frameDurationMs": 18.886666666666617,
"p95FrameDurationMs": 16.799999999999272
},
{
"name": "vue-large-graph-pan",
"durationMs": 23544.035000000007,
"styleRecalcs": 178,
"styleRecalcDurationMs": 29.463000000000015,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 23493.977000000003,
"heapDeltaBytes": -50410412,
"heapUsedBytes": 166876880,
"domNodes": -8312,
"jsHeapTotalBytes": -15142912,
"scriptDurationMs": 1013.8009999999999,
"eventListeners": -16381,
"totalBlockingTimeMs": 954,
"frameDurationMs": 17.776666666666642,
"p95FrameDurationMs": 16.80000000000291
},
{
"name": "workflow-execution",
"durationMs": 466.72799999998915,
"styleRecalcs": 13,
"styleRecalcDurationMs": 21.552000000000003,
"layouts": 3,
"layoutDurationMs": 0.8409999999999997,
"taskDurationMs": 126.23200000000001,
"heapDeltaBytes": -16002404,
"heapUsedBytes": 50979488,
"domNodes": 130,
"jsHeapTotalBytes": 7737344,
"scriptDurationMs": 11.256,
"eventListeners": 67,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "workflow-execution",
"durationMs": 460.7099999999491,
"styleRecalcs": 12,
"styleRecalcDurationMs": 17.968,
"layouts": 3,
"layoutDurationMs": 0.596,
"taskDurationMs": 110.79400000000001,
"heapDeltaBytes": -16316224,
"heapUsedBytes": 50559892,
"domNodes": 117,
"jsHeapTotalBytes": 7737344,
"scriptDurationMs": 9.831,
"eventListeners": 65,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.800000000000182
}
]
}🎨 Storybook: ✅ Built — View Storybook |
dosubot
Bot
added
the
size:L
synap5e
marked this pull request as draft
July 10, 2026 11:25
Contributor
Author
|
@coderabbitai review |
Contributor
✅ Action performedReview finished.
|
synap5e
marked this pull request as ready for review
July 10, 2026 11:28
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
🔍 Cursor Review — Consolidated panel
Triggered by @synap5e.
Found 1 finding(s).
| Severity | Count |
|---|---|
| 🟢 Low | 1 |
Panel: 6/8 reviewers contributed findings.
Reviewers that did not contribute: kimi-k2.5:adversarial (empty), kimi-k2.5:edge-case (empty)
![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
🤖 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/modelLibraryAssetMode.spec.ts`:
- Around line 103-118: Update the “Refresh seeds a backend rescan” test to
validate the POST mutation’s request body in addition to its method and
endpoint. Use AssetHelper’s recorded mutation.body to assert the seed request
contains the exact payload { roots: ['models'] }, ensuring the correct root and
payload shape are sent.
🪄 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: 7241ae27-81c7-4ce4-abfa-964ae8b03671
📒 Files selected for processing (8)
browser_tests/fixtures/components/SidebarTab.tsbrowser_tests/fixtures/data/assetFixtures.tsbrowser_tests/fixtures/helpers/AssetHelper.tsbrowser_tests/fixtures/helpers/FeatureFlagHelper.tsbrowser_tests/fixtures/utils/dispatchApiEvent.tsbrowser_tests/tests/graph.spec.tsbrowser_tests/tests/sidebar/modelLibraryAssetMode.spec.tsbrowser_tests/tests/sidebar/modelLibraryRouting.spec.ts
Codecov Report✅ All modified and coverable lines are covered by tests. @@ Coverage Diff @@
## main #13574 +/- ##
==========================================
+ Coverage 79.44% 79.47% +0.02%
==========================================
Files 1743 1744 +1
Lines 96915 97108 +193
Branches 32208 30916 -1292
==========================================
+ Hits 76997 77172 +175
- Misses 19556 19566 +10
- Partials 362 370 +8
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 53 files with indirect coverage changes 🚀 New features to boost your workflow:
|
christian-byrne
left a comment
christian-byrne
left a comment
Contributor
There was a problem hiding this comment.
Automated multi-agent review of this PR's delta against its parent branch (concurrent reviewers; B-group tooling and dependency scans skipped since the review checkout has no installed deps). No blocking issues. Findings below, most inline. The route-pattern narrowing (/assets to /api/assets) was verified to regress no existing spec and to fix a latent JS-chunk-intercept bug.
Contributor
|
Round 2 review — e2e test completeness Three findings covering test robustness angles not addressed in round 1: 1. Remaining
2. No unhappy-path test for API error (suggestion, non-blocking)
3. Tree node selectors rely on PrimeVue internal class names (nitpick, non-blocking)
|
Contributor
Author
|
Re the round-2 items:
Posted by Claude Code on Simon's behalf. |
christian-byrne
left a comment
christian-byrne
left a comment
Contributor
There was a problem hiding this comment.
Thanks for building this out — this is a real improvement over zero e2e coverage on the assets-mode sidebar, and the fixture/helper layer (AssetHelper, assetFixtures, dispatchApiEvent) is reusable well beyond these two specs. The registration-order test and the seed-flow test in particular cover things that were previously only reachable by hand.
Most of what follows is non-blocking. One item is blocking-ish (the flag/WS race, inline on FeatureFlagHelper), and one is a product bug the suite surfaced by omission rather than a problem with the suite itself. I'm leaving this as a comment, not un-approving.
AC3 isn't just untested — the sidebar code violates it
FE-1076's third acceptance criterion:
under flag-on, an asset with no
model_type:tag falls back to legacy bare grouping; uncovered assets are handled, not a TODO.
The sidebar path does the opposite of that. src/platform/assets/services/assetService.ts:224 modelFolderFromTag(tag, modelTypeMode) returns undefined for any non-namespaced tag once modelTypeMode is true:
if (modelTypeMode) {
return tag.startsWith(MODEL_TYPE_TAG_PREFIX)
? tag.slice(MODEL_TYPE_TAG_PREFIX.length)
: undefined
}buildModelBuckets (assetService.ts:407) then hits the folders.length === 0 branch at assetService.ts:417 and drops the asset with a console.warn. So on a model_type:-capable backend, a user with a legacy bare-tagged model loses it from the sidebar entirely — not "falls back to legacy bare grouping".
The browser path does implement the fallback. src/platform/assets/utils/assetMetadataUtils.ts:279 getAssetCategories returns getBareTagCategories(asset) (filtered for namespace residue) when there are no model_type: values:
if (modelTypeMode) {
const modelTypes = getModelTypeTagValues(asset)
if (modelTypes.length > 0) return modelTypes
return getBareTagCategories(asset).filter(
(category) => !category.startsWith(MODEL_TYPE_TAG_PREFIX)
)
}Two surfaces, two semantics, one flag.
Nothing currently catches it:
src/platform/assets/services/assetService.test.ts:483— the only bare-tag test — flips the flag off (mockSupportsModelTypeTags.value = false), so it exercises the legacy path, not the fallback.assetService.test.ts:500"drops uncategorized model assets with a warning" usestags: ['models']with no bare tag at all, so it asserts the correct behaviour for a genuinely uncovered asset and says nothing about a bare-tagged one.
The missing case is one line: mockSupportsModelTypeTags.value = true with tags: ['models', 'checkpoints'], which currently returns [].
Not asking you to fix the service in a test-only PR — flagging it because it's the AC the e2e suite was meant to cover, and it turns out the code can't satisfy it yet. Happy to take the service fix separately if you'd rather keep this PR scoped to tests.
What the mocks currently hide
(non-blocking — posting top-level because browser_tests/fixtures/helpers/AssetHelper.ts:227-241 is outside this PR's diff hunks, so it won't take an inline anchor.)
Worth writing this down so nobody reads green here as "the asset path is covered".
Cursor pagination is not exercised at all, and the mock's pagination branch is dead code against the real caller. AssetHelper.handleListAssets (browser_tests/fixtures/helpers/AssetHelper.ts:227) slices by offset:
const offset = parseInt(url.searchParams.get('offset') ?? '0', 10)
filtered = filtered.slice(offset, offset + limit)but getAllAssetsByTag in src/platform/assets/services/assetService.ts never sends offset — it sends limit + after:
const data = await getAssetsPageByTag(tag, includePublic, { limit: pageSize, after, signal })and the mock always returns has_more: false (AssetHelper.ts:241) and never a next_cursor. So the cursor walk never runs a second iteration, and the non-advancing-cursor backstop (data.next_cursor === after) is never hit. Aligning the mock on after/next_cursor would make the multi-page path real.
Other things green here doesn't cover:
- Libraries above
DEFAULT_LIMIT(500) — single-page mock only. - The zod validation failure path — the mock is schema-valid by construction, so a malformed payload never reaches the parser.
- Whether core's
/experiment/modelsactually emitsextensions. The extension-allowlist test rests on that field (modelLibraryAssetMode.spec.ts:24-29setsextensions: ['.safetensors', '.gguf']). If core omits it,effectiveModelExtensionsfalls back toDEFAULT_MODEL_EXTENSIONS, which excludes.gguf— the exact model that test asserts is visible. Worth confirming against a real core response once. - The real
assets.seed.fast_completeWS message. It's synthesised client-side viadispatchApiCustomEvent; the name and payload shape have never been verified against core. The seed409"already running" path is also untested. - Fixture realism. Every fixture emits only the namespaced tag (
tags: ['models', 'model_type:checkpoints']), butassetMetadataUtils.ts:166-170states core keeps the bare directory-path twin in sync. The realistic dual-tag payload — precisely where a label leak or a double-listing would surface — is never rendered.
| * field that `supports_model_type_tags` backends emit (see | ||
| * `src/platform/assets/schemas/assetSchema.ts`). | ||
| */ | ||
| type CoreModelAsset = Asset & Pick<AssetItem, 'loader_path'> |
Contributor
There was a problem hiding this comment.
question — this intersection type is a symptom worth naming, and I think the answer is "not your bug".
type CoreModelAsset = Asset & Pick<AssetItem, 'loader_path'> exists because loader_path has zero hits anywhere in packages/. The generated Asset in packages/ingest-types/src/types.gen.ts carries file_path instead (lines 3057, 3105, 3918). packages/ingest-types is generated from the Cloud Ingest OpenAPI spec, not from core — and core PR #14796 removed file_path and made loader_path authoritative. So the generated types now carry a field core deleted and lack the one core added.
AGENTS.md:229 rule 4 asks for mocks typed from packages/ingest-types; this intersection is the workaround for that not being possible on this field. So: no objection to the type as written.
The consequence worth flagging is that these e2e mocks can't be contract-checked against core at all for the field the whole suite hinges on. The real fix is getting core's OpenAPI into the type-generation pipeline. Worth a ticket — happy to file it.
(This also answers the question I asked you earlier about moving these to generated types: we can't yet, for exactly this field.)
Contributor
Author
There was a problem hiding this comment.
Agreed, and thanks for tracing it to core #14796 dropping file_path. The intersection is the right local workaround; the real fix is getting core's OpenAPI into the type-generation pipeline so these mocks can be contract-checked on the field the suite hinges on. Please do file that ticket, it's a cross-repo pipeline change that outlives this PR.
Posted by Claude Code on Simon's behalf.
Contributor
There was a problem hiding this comment.
The in-PR intersection type is the right local workaround and I'm not asking to widen it — but per our standard this thread stays open and active rather than being closed on "please file that ticket". The risk it names (the field the whole e2e suite hinges on can't be contract-checked against core at all) outlives this PR precisely because it's cross-repo, which makes losing the thread more likely, not less.
Concrete next step: I'll file the pipeline ticket (get core's OpenAPI into packages/ingest-types generation so loader_path is authoritative) and link it on this thread. Until that link is here with an owner, this is not resolved — keeping it open.
| } | ||
| }) | ||
|
|
||
| test.describe('Model library sidebar - asset mode', () => { |
Contributor
There was a problem hiding this comment.
suggestion (non-blocking) — coverage gaps measured against the ACs rather than against the code:
- No negative-membership assertion. Nothing asserts that a
model_type:lorasasset is absent fromcheckpoints. Delete the tag matching inmodelFolderFromTagand dump every asset into every folder, and the current tests stay green. - No assertion that
model_type:never appears in a rendered label.grep -rn model_type browser_tests/only hits fixture tag definitions and the flag name — never a label assertion. GivengetAssetModelType's doc warns that a namespaced literal leaking through round-trips into the tag set, a "no visible text containsmodel_type:" assertion would be cheap insurance. - Flag ABSENT (vs. explicitly
false) is untested at every level.modelLibraryAssetMode.spec.ts:262forcessupports_model_type_tags: falserather than omitting it, and every unit consumer stubs the getter. Nothing in the suite would fail if the default flipped totrue. - No e2e orphan asset.
loader_path: nullis unit-covered only; there's no e2e fixture asserting an orphan is absent from the tree.
Contributor
Author
There was a problem hiding this comment.
Good gaps. The negative-membership assertion and the "no visible label contains model_type:" check are cheap insurance; the flag-absent-vs-false and e2e-orphan cases are more involved. I'd prefer to take these as a follow-up on the e2e suite rather than grow this PR further, but the two cheap ones are low-effort if you'd rather they land here now.
Posted by Claude Code on Simon's behalf.
Contributor
There was a problem hiding this comment.
Per our standard, let's not fold the whole set into an unbacked e2e follow-up. The two cheap ones you agree are low-effort should land in this PR now, since this is the PR adding the coverage:
- Negative-membership assertion: a
model_type:lorasasset is absent fromcheckpoints(today, deleting the tag match inmodelFolderFromTagkeeps the suite green). - "No visible label contains
model_type:" assertion (cheap insurance against a namespaced literal leaking into a rendered label).
The more involved two — flag ABSENT vs explicit false, and an e2e orphan (loader_path: null absent from the tree) — can be tracked, but only as a linked issue referenced on this thread with an owner, not a verbal follow-up. Keeping open.
| '[data-node-id="preview-CheckpointLoaderSimple"]' | ||
| ) | ||
| await expect(ghost).toBeVisible() | ||
| expect(await comfyPage.nodeOps.getGraphNodesCount()).toBe(0) |
Contributor
There was a problem hiding this comment.
suggestion (non-blocking) — snapshot-then-assert.
expect(await comfyPage.nodeOps.getGraphNodesCount()).toBe(0) snapshots a value and then asserts on it. browser_tests/FLAKE_PREVENTION_RULES.md §7 lists this as the first entry in the common-flake table:
| Pattern | Bad | Fix |
|---|---|---|
| Snapshot-then-assert | expect(await evaluate()).toBe(x) |
await expect.poll(() => evaluate()).toBe(x) |
It fails open rather than retrying. Same shape at modelLibraryRouting.spec.ts:71-76 (expect(await comfyPage.settings.getSetting(...)).toBe(true), twice). await expect.poll(...) in all three places.
Separately, at lines 66-67: folderNodes.nth(0) / .nth(1) positional assertions will shift if the fixtures ever gain a folder — folderNodes matches nested non-leaf nodes too, and asset mode hides empty folders, so the index-to-folder mapping isn't stable under fixture growth. Asserting on the ordered text array (or filtering to top-level nodes) would survive that.
Contributor
Author
There was a problem hiding this comment.
Valid, these match the FLAKE_PREVENTION_RULES snapshot-then-assert entry and the positional nth() fragility is real given asset mode hides empty folders. The expect.poll swaps are genuinely low-effort; I'm inclined to fold them into the same e2e-suite follow-up as the coverage gaps above so this PR stays put, but happy to do the poll swaps here if you'd prefer them in now.
Posted by Claude Code on Simon's behalf.
Contributor
There was a problem hiding this comment.
You agree the expect.poll swaps are genuinely low-effort, so per our standard let's land them here rather than fold into a follow-up — this is the PR introducing these specs, and FLAKE_PREVENTION_RULES §7 lists snapshot-then-assert as the first common-flake entry.
Concrete fix:
- Swap the three snapshot-then-assert sites to
await expect.poll(...): this file :185, plusmodelLibraryRouting.spec.ts:71-76(twice). - Replace the positional
folderNodes.nth(0)/.nth(1)at :66-67 with an assertion on the ordered text array (or filter to top-level nodes), since asset mode hides empty folders and the index-to-folder mapping isn't stable under fixture growth.
Keeping this thread open.
Group the asset-mode Model Library by namespaced model_type:* tags. When an asset carries model_type:* tags, the stripped values are the authoritative categories and all other tags are disregarded; otherwise fall back to the legacy bare-tag top-level grouping. A shared getAssetCategories helper keeps the "By type" nav and category filtering in agreement. Resolve node-provider lookup from the same tags via getAssetNodeCategory: prefer the most specific (deepest parent/child) candidate so a flat model_type:LLM does not shadow a resolvable hierarchical tag, and strip the model_type: prefix before the modelNodeMappings lookup.
Drive the model_type: grouping/resolution off the backend-reported supports_model_type_tags capability (getServerFeature) instead of sniffing tags per asset. Default false preserves the legacy bare-tag grouping, so a backend that does not declare the capability is unchanged. When the flag is true, assets group by their model_type: values and an uncovered asset (no model_type: tag) still routes by its bare tags. The thread-through is a modelTypeMode flag on getAssetCategories / getAssetNodeCategory / filterByCategory (keeping them pure); call sites read the capability via useFeatureFlags.
Unwrap the if (isCloud) gate in assetsStore getModelState so the asset-mode Model Library works on non-cloud distributions (it previously returned an empty no-op store off-cloud, leaving the library blank even with the asset API enabled). Scoped to the model-asset store only; the rest of the isCloud forks are out of scope here.
The card type badge derived from the first raw non-models tag, so in model_type mode it rendered the literal `model_type:checkpoints`. Route it through a model_type-aware getAssetTypeBadge and key the display cache on the flag so a late-arriving feature flag re-derives the badge. Also adds docstrings to the tag helpers to satisfy coverage.
Also realigns the card type badge to the same rule: keep the existing "first non-models tag" selection and only strip the model_type: namespace prefix, rather than reordering to prefer the model_type: value. So an asset like [models, foo, model_type:bar] still shows foo, while [models, model_type:checkpoints] shows checkpoints. Title (primaryCategoryTag) strips the same prefix when the flag is on.
…NodeFromAsset main (#12765) renamed createModelNodeFromAsset -> resolveModelNodeFromAsset. Re-apply FE-1076's flag-gated node-category resolution there: use getAssetNodeCategory(asset, flags.supportsModelTypeTags) instead of the inline first-non-reserved-tag lookup, so a model_type:* tag resolves the loader when the supports_model_type_tags flag is on.
Importing MODELS_TAG/MISSING_TAG from assetService made this leaf util pull the assetService -> i18n (createI18n) chain. Test suites that import assetMetadataUtils (e.g. useAssetFilterOptions, AssetsSidebarListView) with a minimal vue-i18n mock then failed to load. Define the two reserved-tag literals locally so the util has no heavy transitive dependency.
christian-byrne
left a comment
christian-byrne
left a comment
Contributor
There was a problem hiding this comment.
Re-reviewing at b68776b.
Leading with an acknowledgement: the reconnect-safe setServerFlagsPersistent (re-applying the override on each feature_flags handshake, keeping the WS as the source of truth) is the correct fix for the flag-race I raised in round 2 - and my earlier mockServerFeatures pointer was wrong. mockServerFeatures intercepts /api/features, which never populates serverFeatureFlags (only the WS feature_flags message does), so it would have silently dropped these specs into the wrong mode. Thanks for tracing the actual mechanism rather than just taking the JSDoc pointer.
My approval stands. One issue worth considering plus a few nits inline below - all non-blocking.
| id: 'mt-checkpoint-001', | ||
| name: 'sd_xl_base_1.0.safetensors', | ||
| tags: ['models', 'model_type:checkpoints'], | ||
| loader_path: 'SDXL/sd_xl_base_1.0.safetensors', |
Contributor
There was a problem hiding this comment.
issue (non-blocking) - CI blind spot on the cutover.
Every MODEL_TYPE_* fixture in this block pairs supports_model_type_tags: true (the specs force it on via setServerFlagsPersistent) with a loader_path. There is no fixture for flag-on + no loader_path - which is exactly the cutover-before-cloud#5604 scenario: buildModelBuckets (assetService.ts:428) skips every asset that has no loader_path while the flag is on, so the sidebar tree renders empty. As written, that regression cannot be caught by CI.
Suggest adding a supports_model_type_tags: true + no-loader_path fixture with a spec asserting the skip/empty behaviour - it turns a latent cross-repo BE ordering dependency (BE-4728) into a visible CI gate.
(Adjacent to, but distinct from, the orphan-asset gap in the thread on line 47: that one is a single loader_path: null among otherwise-valid assets; this is the all-assets-lack-it cutover window that empties the whole tree.)
| // Expanding a folder re-attempts the failing load without crashing or | ||
| // producing leaves, proving the failure path stays contained. | ||
| await tab.getFolderRowByLabel('checkpoints').click() | ||
| await expect(tab.leafNodes).toHaveCount(0) |
Contributor
There was a problem hiding this comment.
nit - this "Degrades gracefully" test asserts tab.leafNodes toHaveCount(0) both before (line 237) and after (this line) expanding checkpoints. The second assertion can pass instantly against the still-empty tree, before an expand-triggered reload would have produced any leaves - so it is weakly synchronized and would not catch a regression where expansion does surface leaves. Consider awaiting a load marker (e.g. the folder's loading state settling) before the final count.
| assetApi.configure( | ||
| withModels([...WALK_ASSETS, MODEL_TYPE_CHECKPOINT_SCANNED]) | ||
| ) | ||
| await dispatchApiCustomEvent(comfyPage.page, 'assets.seed.fast_complete') |
Contributor
There was a problem hiding this comment.
nit - the wire string 'assets.seed.fast_complete' is hardcoded here (and again at line 164), duplicating the literal assetService uses in onModelsScanned. If the event name ever changes, these specs will not fail-fast - they will just quietly stop exercising the path. Consider importing the shared constant instead of re-typing the string.
| }) | ||
|
|
||
| function createCoreModelAsset( | ||
| overrides: Partial<CoreModelAsset> |
Contributor
There was a problem hiding this comment.
nit - createCoreModelAsset(overrides) requires the arg, while its sibling createModelAsset(overrides = {}) (line 11) defaults it. Minor inconsistency in the factory signatures; defaulting to {} here too would match.
- assert negative membership, no model_type: label leak, flag-absent
default, and orphan (loader_path: null) exclusion
- pin the flag-on/no-loader_path cutover window (BE-4728) with a
dedicated fixture and empty-tree spec so the cross-repo ordering
dependency fails visibly in CI
- swap snapshot-then-assert sites to expect.poll and replace positional
folder assertions with an ordered-text assertion per flake rules
- synchronize the degraded-walk expansion retry on its 500 response
- share the assets.seed.fast_complete wire constant between service and
specs; default createCoreModelAsset overrides to {}
Specs authored against mocked routes; not fully verified locally (app
boot exceeds the fixture timeout under load). Routing spec passed 3/3;
lint, typecheck, knip, and unit tests pass.
christian-byrne
previously approved these changes
Jul 28, 2026
christian-byrne
left a comment
christian-byrne
left a comment
Contributor
There was a problem hiding this comment.
Approving. Final pass: the commit since my last review closes the e2e coverage gaps I raised. It adds the orphan and pre-cutover (namespaced tag, no loader_path) fixtures so the cutover risk is now CI-visible, a reconnect-safe clearServerFlagsPersistent helper, a negative-membership bucket assertion (a lora leaf must not appear under checkpoints), and ordered-array assertions replacing positional nth() per the flake rules. Clean reparent plus exactly the coverage that was owed. Green.
synap5e
dismissed stale reviews from christian-byrne and coderabbitai[bot]
July 28, 2026 08:19
The base branch was changed.
dosubot
Bot
added
size:XXL
…load Search expansion ran only when the query committed, so a folder whose first match arrived from a later reload (a scan completing while the search was active) rendered collapsed and hid the result. Expand result folders once per key instead: newly appearing folders open, folders the user collapsed stay collapsed, and an unchanged tree costs no expand pass.
dosubot
Bot
added
size:L
6 tasks
Contributor
Author
|
@AustinMroz @christian-byrne — this is the last one of the assets model-library stack; the other four (#12948/#12972/#13302/#13464) have merged. CI is green and the mid-search scan/expand fix is in — #13574 is blocked only on a re-approval, since the fix commits dismissed the prior stamps. Could one of you re-review the delta and approve so it can land? Thanks. Created by Claude Code. |
christian-byrne
approved these changes
Jul 28, 2026
christian-byrne
assigned
synap5e and unassigned
synap5e,
AustinMroz and
christian-byrne
4 tasks
pull Bot
pushed a commit
to Penguinjanator/ComfyUI_frontend
that referenced
this pull request
Aug 12, 2026
…type mode (Comfy-Org#14848) ## Summary Fixes a bug where a legacy bare-tagged model asset silently disappears from the model library sidebar once the backend advertises `model_type:` support, instead of falling back to legacy bare-tag grouping. _Recreated from Comfy-Org#14217 — same change, opened directly under my own account for tracking. The High finding from Cursor's panel on that PR is already folded into this diff._ ## Changes - **What**: `modelFolderFromTag` returned `undefined` for any tag that didn't carry the `model_type:` prefix as soon as `modelTypeMode` was `true`. `buildModelBuckets` treats an asset whose tags resolve to no folder as "uncategorized" and drops it with a `console.warn`, so a model still carrying a legacy bare tag (e.g. `checkpoints` instead of `model_type:checkpoints`) vanished from the sidebar entirely on a `model_type:`-capable backend. Changed in `src/platform/assets/services/assetService.ts`. - **Breaking**: none ## How The fix deletes the local `modelFolderFromTag` helper and buckets via `getAssetCategories` from `assetMetadataUtils`, which already implements exactly the intended semantics and is already used by the other asset surfaces: `model_type:*` values are authoritative when present, and an asset with no `model_type:` tag still routes by its bare tags (with namespace residue filtered out). This makes the sidebar consistent with the rest of the asset code rather than carrying a second, subtly different grouping rule. A side effect worth calling out: because `getAssetCategories` returns `model_type:` values *alone* when an asset has any, an asset covered by `model_type:` tags can no longer be cross-listed into a second folder by a leftover bare-tag twin from a partial re-tagging. The e2e suite covers that case (the "mid-retag twin" scenario). Added an e2e scenario, `Model library sidebar - asset mode with a legacy bare tag`, in `browser_tests/tests/sidebar/modelLibraryAssetMode.spec.ts`, plus a matching fixture (`MODEL_TYPE_CHECKPOINT_LEGACY_TAG` in `browser_tests/fixtures/data/assetFixtures.ts`): a bare-tagged asset that still carries a `loader_path`, walked with `supports_model_type_tags: true`, asserting it still renders in the `checkpoints` folder rather than being dropped. ## Review Focus - This targets `main` directly. The stack it was originally written on top of (Comfy-Org#13574) has since merged, so the diff here is just the three files. - Coverage for the regression lives in the e2e spec rather than a unit test — the unit-level version was dropped as duplicating it. Flagging that explicitly since it is a judgement call about test placement. - Verified against `main` that the bug is still live: `modelFolderFromTag` on `main` still returns `undefined` for bare tags whenever `modelTypeMode` is true. - Checked that the existing `assetService.test.ts` cases on `main` remain valid under the new path: `getBareTagCategories` filters out the reserved `models` tag, so the "drops uncategorized model assets with a warning" case still drops its `tags: ['models']` asset; the bare-tag grouping cases all run with `supports_model_type_tags = false`, an unchanged path. ## Test plan - [ ] `pnpm typecheck` — **not run**: no Node/pnpm toolchain provisioned on the host I authored this on. Relying on CI. - [ ] `pnpm exec vitest run src/platform/assets/services/assetService.test.ts` — **not run**, same reason. Compatibility with the existing cases was verified by reading them (see Review Focus). - [x] Static check that the refactor is complete: `MODEL_TYPE_TAG_PREFIX` import removed with no remaining uses in the file, `modelFolderFromTag` has zero references repo-wide, `getAssetCategories` is exported from `assetMetadataUtils` on `main`. - [x] The identical three-file diff passed a full CI run on Comfy-Org#14217 (Playwright 1759 passed / 0 failed / 1 flaky; Codecov reported all modified lines covered). --- **Review coverage note:** no Cursor panel has run on this PR — verified by querying for the panel review itself rather than inferring from an empty findings list. That is a *did-not-run*, not a clean result; the checks API reports success either way. CodeRabbit was rate-limited. Codex did review it, and its one finding was addressed in `f16560c`. _(An earlier version of this note blamed an org-wide Cursor outage running to 2026-08-22. That attribution was wrong and is withdrawn — panels have since been seen running normally elsewhere. The measured fact, that none ran here, is unchanged.)_ --------- Co-authored-by: Austin Mroz <austin@comfy.org>
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
4 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
E2E coverage for the assets-backed model library sidebar (#13464), testing the integration seams the unit suites mock away.
Stacked on #13464 (
synap5e/assets-mode-sidebar). All assets endpoints are mocked, so the suite passes regardless of whether the CI container backend has the assets API yet.Changes
sidebar/modelLibraryRouting.spec.ts:Comfy.ModelLibrary.UseAssetBrowser+Comfy.Assets.UseAssetAPItab routing (both on → asset browser modal; either off → sidebar tree;@clouddefaults → modal).sidebar/modelLibraryAssetMode.spec.ts: asset-mode tree from a mocked/api/assetswalk +/api/experiment/modelsregistration (folder registration order, eager load with no load-all button, extension allowlists verbatim vs match-all default filtering), refresh →POST /api/assets/seed, live tree/search updates on theassets.seed.fast_completeevent, ghost placement filling the loader widget withloader_path, bare-tag backend fallback.model_type:/loader_pathasset fixtures,AssetHelperseed-endpoint handling,FeatureFlagHelper.setServerFlags(forcesapi.serverFeatureFlags; theff:localStorage override is dev-only and tree-shaken from production builds),dispatchApiCustomEventutil for wire-level events outside the typedApiEventTypesmap,getFolderRowByLabelpage-object locator (ancestor tree rows contain descendant text, so the existing label locator misclicks nested folders).AssetHelperroute pattern narrowed**/assets**→**/api/assets**: the old pattern intercepted the built frontend's/assets/*.jschunks on any navigation aftermock(). No existing consumer is affected (all target/api/assets).graph.spec.ts: mirrors main's branded-LinkIdfix (9e5fb67) verbatim; now superseded by main having been merged through the stack (identical content, resolved cleanly).Review Focus
assets.seed.fast_completeinjection dispatches directly on the api singleton (EventTarget.prototype.dispatchEvent), replicating the socket handler's default case; the real WS frame path above that dispatch is not exercised.supports_model_type_tags: falseexplicitly so the real test backend'sfeature_flagshandshake can't decide which mode is under test.Test plan
pnpm typecheck:browser, eslint, oxlint, knip clean--cpu --multi-user)@cloudrouting test green against the built cloud distsidebar/modelLibrary.spec.ts+assetHelper.spec.ts(34 tests) still green with the shared-fixture changes