fix: handle changes to supportsModelTypeTags - #14160
Conversation
|
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 PR adds HTTP refresh handling for ChangesModel type capability
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Suggested labels: Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant GraphView
participant FeaturesAPI
participant useFeatureFlags
participant modelStore
GraphView->>FeaturesAPI: fetch /features
FeaturesAPI-->>GraphView: supports_model_type_tags
GraphView->>useFeatureFlags: update HTTP capability
useFeatureFlags-->>modelStore: resolve supportsModelTypeTags
modelStore->>modelStore: reloadModels when capability changes
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
🎭 Playwright: ✅ 1788 passed, 0 failed · 1 flaky📊 Browser Reports
🎨 Storybook: ✅ Built — View Storybook📦 Bundle: 8.25 MB gzip 🔴 +102 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 — 112 kB (baseline 112 kB) • ⚪ 0 BTop-level views, pages, and routed surfaces
Status: 13 added / 13 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 — 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 — 70.8 kB (baseline 70.8 kB) • ⚪ 0 BReusable component library chunks
Status: 6 added / 6 removed / 9 unchanged Data & Services — 3.47 MB (baseline 3.47 MB) • 🔴 +232 BStores, services, APIs, and repositories
Status: 14 added / 14 removed / 3 unchanged Utilities & Hooks — 386 kB (baseline 386 kB) • 🔴 +36 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 / 211 unchanged ⚡ Performance Report
Show regressions
All metrics
Historical variance (last 15 runs)
Trend (last 15 commits on main)
Raw data{
"timestamp": "2026-08-04T22:09:15.241Z",
"gitSha": "4edb5d5ca0aac168943f2d8228ae0452b973cf18",
"branch": "synap5e/feat/model-type-flag-features-endpoint",
"measurements": [
{
"name": "canvas-idle",
"durationMs": 2018.5730000000035,
"styleRecalcs": 8,
"styleRecalcDurationMs": 7.075000000000001,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 477.72999999999996,
"heapDeltaBytes": 5476084,
"heapUsedBytes": 69783360,
"domNodes": 16,
"jsHeapTotalBytes": 24117248,
"scriptDurationMs": 16.638000000000005,
"eventListeners": 4,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "canvas-idle",
"durationMs": 2022.4660000000085,
"styleRecalcs": 10,
"styleRecalcDurationMs": 9.534999999999998,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 477.97100000000006,
"heapDeltaBytes": 5456840,
"heapUsedBytes": 69446868,
"domNodes": 20,
"jsHeapTotalBytes": 24903680,
"scriptDurationMs": 18.751000000000005,
"eventListeners": 4,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66333333333335,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "canvas-mouse-sweep",
"durationMs": 1948.5139999999888,
"styleRecalcs": 79,
"styleRecalcDurationMs": 49.270999999999994,
"layouts": 12,
"layoutDurationMs": 3.798,
"taskDurationMs": 941.216,
"heapDeltaBytes": -11800188,
"heapUsedBytes": 52295956,
"domNodes": -281,
"jsHeapTotalBytes": 23572480,
"scriptDurationMs": 131.652,
"eventListeners": -151,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "canvas-mouse-sweep",
"durationMs": 1874.5090000001028,
"styleRecalcs": 74,
"styleRecalcDurationMs": 38.504000000000005,
"layouts": 12,
"layoutDurationMs": 3.476,
"taskDurationMs": 908.8889999999999,
"heapDeltaBytes": -13359212,
"heapUsedBytes": 50820948,
"domNodes": -281,
"jsHeapTotalBytes": 24621056,
"scriptDurationMs": 129.64199999999997,
"eventListeners": -153,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66333333333332,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "canvas-zoom-sweep",
"durationMs": 1770.3180000000316,
"styleRecalcs": 32,
"styleRecalcDurationMs": 18.846999999999998,
"layouts": 6,
"layoutDurationMs": 0.644,
"taskDurationMs": 396.148,
"heapDeltaBytes": 8703884,
"heapUsedBytes": 72640056,
"domNodes": 77,
"jsHeapTotalBytes": 24117248,
"scriptDurationMs": 20.662,
"eventListeners": 19,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "canvas-zoom-sweep",
"durationMs": 1754.8849999999447,
"styleRecalcs": 32,
"styleRecalcDurationMs": 18.192,
"layouts": 6,
"layoutDurationMs": 0.627,
"taskDurationMs": 409.529,
"heapDeltaBytes": 8609132,
"heapUsedBytes": 72784812,
"domNodes": 77,
"jsHeapTotalBytes": 24903680,
"scriptDurationMs": 23.951,
"eventListeners": 19,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "dom-widget-clipping",
"durationMs": 626.102000000003,
"styleRecalcs": 10,
"styleRecalcDurationMs": 9.196,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 426.799,
"heapDeltaBytes": -11458288,
"heapUsedBytes": 52740284,
"domNodes": 16,
"jsHeapTotalBytes": 25690112,
"scriptDurationMs": 64.124,
"eventListeners": 0,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66333333333332,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "dom-widget-clipping",
"durationMs": 573.0189999999311,
"styleRecalcs": 11,
"styleRecalcDurationMs": 8.163,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 387.59900000000005,
"heapDeltaBytes": -11334636,
"heapUsedBytes": 52638132,
"domNodes": 18,
"jsHeapTotalBytes": 24903680,
"scriptDurationMs": 62.018,
"eventListeners": 0,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "large-graph-idle",
"durationMs": 2052.3000000000025,
"styleRecalcs": 9,
"styleRecalcDurationMs": 8.391,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 675.008,
"heapDeltaBytes": 7966700,
"heapUsedBytes": 66632800,
"domNodes": -281,
"jsHeapTotalBytes": 2990080,
"scriptDurationMs": 106.05399999999999,
"eventListeners": -147,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "large-graph-idle",
"durationMs": 2037.7070000000685,
"styleRecalcs": 9,
"styleRecalcDurationMs": 8.34,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 691.8900000000001,
"heapDeltaBytes": 7982888,
"heapUsedBytes": 67712232,
"domNodes": -280,
"jsHeapTotalBytes": 4038656,
"scriptDurationMs": 109.672,
"eventListeners": -177,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66333333333332,
"p95FrameDurationMs": 16.799999999999272
},
{
"name": "large-graph-pan",
"durationMs": 2214.332000000013,
"styleRecalcs": 69,
"styleRecalcDurationMs": 15.708000000000004,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 1321.839,
"heapDeltaBytes": 5215232,
"heapUsedBytes": 65560456,
"domNodes": -282,
"jsHeapTotalBytes": 3969024,
"scriptDurationMs": 444.363,
"eventListeners": -147,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "large-graph-pan",
"durationMs": 2254.4090000000097,
"styleRecalcs": 68,
"styleRecalcDurationMs": 14.194999999999999,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 1371.2140000000002,
"heapDeltaBytes": 4346012,
"heapUsedBytes": 65344368,
"domNodes": -285,
"jsHeapTotalBytes": 3706880,
"scriptDurationMs": 460.1460000000001,
"eventListeners": -151,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66333333333332,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "large-graph-zoom",
"durationMs": 3182.7710000000025,
"styleRecalcs": 65,
"styleRecalcDurationMs": 15.350000000000003,
"layouts": 60,
"layoutDurationMs": 7.370000000000001,
"taskDurationMs": 1504.0650000000003,
"heapDeltaBytes": 23685504,
"heapUsedBytes": 85616120,
"domNodes": 12,
"jsHeapTotalBytes": 7340032,
"scriptDurationMs": 534.151,
"eventListeners": 8,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "large-graph-zoom",
"durationMs": 3196.8460000000505,
"styleRecalcs": 64,
"styleRecalcDurationMs": 14.032,
"layouts": 60,
"layoutDurationMs": 7.393999999999999,
"taskDurationMs": 1515.5210000000002,
"heapDeltaBytes": 23886512,
"heapUsedBytes": 86077404,
"domNodes": 10,
"jsHeapTotalBytes": 7864320,
"scriptDurationMs": 545.583,
"eventListeners": 8,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66333333333332,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "minimap-idle",
"durationMs": 2034.1370000000438,
"styleRecalcs": 8,
"styleRecalcDurationMs": 7.297999999999999,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 670.083,
"heapDeltaBytes": 7043944,
"heapUsedBytes": 68043936,
"domNodes": -283,
"jsHeapTotalBytes": 2727936,
"scriptDurationMs": 105.601,
"eventListeners": -149,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66333333333335,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "minimap-idle",
"durationMs": 2040.4809999999998,
"styleRecalcs": 5,
"styleRecalcDurationMs": 4.975999999999998,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 726.4300000000001,
"heapDeltaBytes": 6934612,
"heapUsedBytes": 68099916,
"domNodes": -285,
"jsHeapTotalBytes": 3252224,
"scriptDurationMs": 121.919,
"eventListeners": -149,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.670000000000012,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "subgraph-dom-widget-clipping",
"durationMs": 589.0579999999659,
"styleRecalcs": 47,
"styleRecalcDurationMs": 10.767,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 396.72900000000004,
"heapDeltaBytes": -10367892,
"heapUsedBytes": 53673940,
"domNodes": 20,
"jsHeapTotalBytes": 25952256,
"scriptDurationMs": 118.792,
"eventListeners": 6,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66333333333335,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "subgraph-dom-widget-clipping",
"durationMs": 633.1750000000511,
"styleRecalcs": 47,
"styleRecalcDurationMs": 11.495,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 438.207,
"heapDeltaBytes": -10446868,
"heapUsedBytes": 53650488,
"domNodes": 20,
"jsHeapTotalBytes": 25427968,
"scriptDurationMs": 124.20899999999999,
"eventListeners": 6,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.799999999999272
},
{
"name": "subgraph-idle",
"durationMs": 2011.261999999988,
"styleRecalcs": 10,
"styleRecalcDurationMs": 8.504000000000001,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 493.34200000000004,
"heapDeltaBytes": -17739840,
"heapUsedBytes": 46634836,
"domNodes": -280,
"jsHeapTotalBytes": 23310336,
"scriptDurationMs": 14.112,
"eventListeners": -153,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "subgraph-idle",
"durationMs": 2012.4419999999645,
"styleRecalcs": 9,
"styleRecalcDurationMs": 8.226999999999999,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 535.7989999999999,
"heapDeltaBytes": -17711012,
"heapUsedBytes": 46309852,
"domNodes": -280,
"jsHeapTotalBytes": 23834624,
"scriptDurationMs": 18.26,
"eventListeners": -153,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66333333333335,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "subgraph-mouse-sweep",
"durationMs": 1720.4899999999839,
"styleRecalcs": 78,
"styleRecalcDurationMs": 40.062,
"layouts": 16,
"layoutDurationMs": 4.962999999999999,
"taskDurationMs": 858.623,
"heapDeltaBytes": -18431420,
"heapUsedBytes": 45742836,
"domNodes": 20,
"jsHeapTotalBytes": 24096768,
"scriptDurationMs": 97.004,
"eventListeners": -153,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "subgraph-mouse-sweep",
"durationMs": 1676.2050000000954,
"styleRecalcs": 75,
"styleRecalcDurationMs": 34.32,
"layouts": 16,
"layoutDurationMs": 3.7619999999999996,
"taskDurationMs": 764.304,
"heapDeltaBytes": -4222600,
"heapUsedBytes": 59867080,
"domNodes": 63,
"jsHeapTotalBytes": 24903680,
"scriptDurationMs": 92.739,
"eventListeners": 4,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.699999999999818
},
{
"name": "subgraph-transition-enter",
"durationMs": 1418.8769999999522,
"styleRecalcs": 19,
"styleRecalcDurationMs": 31.101000000000003,
"layouts": 15,
"layoutDurationMs": 13.254999999999999,
"taskDurationMs": 1012.8959999999998,
"heapDeltaBytes": 3825620,
"heapUsedBytes": 76447496,
"domNodes": 13673,
"jsHeapTotalBytes": 15204352,
"scriptDurationMs": 38.89700000000001,
"eventListeners": 2375,
"totalBlockingTimeMs": 152,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "viewport-pan-sweep",
"durationMs": 8455.059000000005,
"styleRecalcs": 249,
"styleRecalcDurationMs": 36.89099999999999,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 4583.59,
"heapDeltaBytes": 5196976,
"heapUsedBytes": 64605396,
"domNodes": -284,
"jsHeapTotalBytes": 5017600,
"scriptDurationMs": 1422.041,
"eventListeners": -163,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.799999999999272
},
{
"name": "viewport-pan-sweep",
"durationMs": 8410.961000000043,
"styleRecalcs": 247,
"styleRecalcDurationMs": 35.427,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 4694.972,
"heapDeltaBytes": 11347472,
"heapUsedBytes": 71924672,
"domNodes": -286,
"jsHeapTotalBytes": 5804032,
"scriptDurationMs": 1415.819,
"eventListeners": -133,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66333333333332,
"p95FrameDurationMs": 16.799999999999272
},
{
"name": "vue-large-graph-idle",
"durationMs": 17652.502000000026,
"styleRecalcs": 0,
"styleRecalcDurationMs": 0,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 17621.492000000002,
"heapDeltaBytes": -44525152,
"heapUsedBytes": 165725128,
"domNodes": -8321,
"jsHeapTotalBytes": -11476992,
"scriptDurationMs": 624.409,
"eventListeners": -16385,
"totalBlockingTimeMs": 0,
"frameDurationMs": 17.773333333333238,
"p95FrameDurationMs": 16.799999999999272
},
{
"name": "vue-large-graph-idle",
"durationMs": 17219.394999999964,
"styleRecalcs": 0,
"styleRecalcDurationMs": 0,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 17189.626,
"heapDeltaBytes": -49588556,
"heapUsedBytes": 167126352,
"domNodes": -8312,
"jsHeapTotalBytes": -9900032,
"scriptDurationMs": 564.766,
"eventListeners": -16389,
"totalBlockingTimeMs": 0,
"frameDurationMs": 17.780000000000047,
"p95FrameDurationMs": 16.799999999999272
},
{
"name": "vue-large-graph-pan",
"durationMs": 21088.49600000002,
"styleRecalcs": 147,
"styleRecalcDurationMs": 19.30599999999999,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 21035.801,
"heapDeltaBytes": -49451268,
"heapUsedBytes": 166562208,
"domNodes": -8312,
"jsHeapTotalBytes": -10428416,
"scriptDurationMs": 909.944,
"eventListeners": -16385,
"totalBlockingTimeMs": 335,
"frameDurationMs": 17.776666666666763,
"p95FrameDurationMs": 16.80000000000291
},
{
"name": "vue-large-graph-pan",
"durationMs": 21110.52099999995,
"styleRecalcs": 146,
"styleRecalcDurationMs": 18.24600000000004,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 21068.837000000003,
"heapDeltaBytes": -37161956,
"heapUsedBytes": 167978692,
"domNodes": -8312,
"jsHeapTotalBytes": -6758400,
"scriptDurationMs": 890.4439999999998,
"eventListeners": -16385,
"totalBlockingTimeMs": 448,
"frameDurationMs": 17.776666666666642,
"p95FrameDurationMs": 16.799999999999272
},
{
"name": "workflow-execution",
"durationMs": 458.09299999996256,
"styleRecalcs": 12,
"styleRecalcDurationMs": 18.607,
"layouts": 3,
"layoutDurationMs": 0.743,
"taskDurationMs": 115.992,
"heapDeltaBytes": 5309596,
"heapUsedBytes": 68268484,
"domNodes": 132,
"jsHeapTotalBytes": 5505024,
"scriptDurationMs": 10.774,
"eventListeners": 99,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.699999999999818
},
{
"name": "workflow-execution",
"durationMs": 461.7150000000265,
"styleRecalcs": 14,
"styleRecalcDurationMs": 18.404999999999998,
"layouts": 3,
"layoutDurationMs": 0.6480000000000001,
"taskDurationMs": 110.40300000000002,
"heapDeltaBytes": 5074484,
"heapUsedBytes": 68065960,
"domNodes": 123,
"jsHeapTotalBytes": 4718592,
"scriptDurationMs": 10.051,
"eventListeners": 97,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66333333333332,
"p95FrameDurationMs": 16.699999999999818
}
]
} |
Codecov Report❌ Patch coverage is
@@ Coverage Diff @@
## main #14160 +/- ##
==========================================
+ Coverage 78.57% 78.86% +0.28%
==========================================
Files 1767 1794 +27
Lines 102351 107480 +5129
Branches 31515 35409 +3894
==========================================
+ Hits 80421 84759 +4338
- Misses 21510 22239 +729
- Partials 420 482 +62
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 201 files with indirect coverage changes 🚀 New features to boost your workflow:
|
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 `@src/platform/assets/composables/useModelTypeTagsRefresh.test.ts`:
- Around line 93-150: Add tests in the useSupportsModelTypeTagsRefresh suite
covering both hidden-tab skip paths: mock document.hidden as true before
dispatching visibilitychange and assert fetchApiSpy is not called again, then
use fake timers and assert the polling interval also does not refetch while
hidden. Restore existing timer and document mocks through the suite cleanup.
In `@src/platform/assets/composables/useModelTypeTagsRefresh.ts`:
- Around line 19-34: Update refreshSupportsModelTypeTags to use a monotonic
request id, mirroring modelStore.ts's loadModelFolders pattern. Increment the id
for each invocation, capture the current value before fetchApi, and only update
httpSupportsModelTypeTags.value when the response belongs to the latest request;
ensure stale responses cannot overwrite newer results.
🪄 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: 59e8a832-1e5f-412e-89c6-ed62e6266bde
📒 Files selected for processing (7)
src/composables/useFeatureFlags.test.tssrc/composables/useFeatureFlags.tssrc/platform/assets/composables/useModelTypeTagsRefresh.test.tssrc/platform/assets/composables/useModelTypeTagsRefresh.tssrc/stores/modelStore.test.tssrc/stores/modelStore.tssrc/views/GraphView.vue
| export async function refreshSupportsModelTypeTags(): Promise<void> { | ||
| const sequence = ++refreshSequence | ||
| try { | ||
| const response = await api.fetchApi('/features', { cache: 'no-store' }) |
There was a problem hiding this comment.
why are we doing another fetch of features here? Why can't we piggyback off of useFeatureFlags?
There was a problem hiding this comment.
Fair question — the duplication with refreshRemoteConfig's /api/features poller is real, and the comment that originally justified this file (claiming a dynamic-config entry could "shadow" the server capability) was wrong: remoteConfig is the verbatim /features response, so nothing shadows anything. That comment has been rewritten at head 837eaaa.
The real reasons this doesn't piggyback on refreshRemoteConfig today:
- Clear-on-error flap:
refreshRemoteConfigsetsremoteConfigto{}on any fetch error, which would flipsupports_model_type_tagsto undefined and trigger spurious model-library reloads on transient network blips. This poller keeps the last known value on error. - No reconnect/visibility triggers: it only refreshes on a 600s interval (cloud-only), so a flag flip or rollback wouldn't reach an open tab for up to 10 minutes; this feature needs refresh on WS reconnect and tab-visible.
- No response-ordering guard: overlapping fetches could let a stale response overwrite a newer one; this poller has a monotonic sequence guard.
The right fix is to harden refreshRemoteConfig (keep-last-value on error, ordering guard, reconnect/visibility triggers) and fold this poller into it so there's one /features poller and one source of truth. That's tracked in FE-1439, assigned to Simon.
Generated by Claude Code
| ServerFeatureFlag.SUPPORTS_MODEL_TYPE_TAGS, | ||
| false | ||
| return ( | ||
| httpSupportsModelTypeTags.value ?? |
There was a problem hiding this comment.
why different pattern for this flag?
There was a problem hiding this comment.
This flag resolves differently because it's a server capability that must be able to change mid-session: the WS copy (api.getServerFeature) arrives once per connection and is never re-pushed, and the remoteConfig-based resolution used by the other flags inherits refreshRemoteConfig's problems — it clears remoteConfig to {} on any fetch error (which would flap this flag and cause spurious model-library reloads), has no reconnect/visibility triggers, and has no out-of-order response guard. So this one prefers a dedicated refreshable HTTP value and falls back to the WS copy for backends that don't serve the key over HTTP.
To be clear, the earlier code comment here claiming a dynamic-config entry could "shadow" the capability was wrong (remoteConfig is the verbatim /features response) and has been rewritten at head 837eaaa with the real rationale above. The duplication is acknowledged as temporary: hardening refreshRemoteConfig and folding the dedicated poller into it is tracked in FE-1439 (assigned to Simon), after which this flag can resolve through the common path.
Generated by Claude Code
The websocket copy of the flag is requested once per connection and never re-pushed, so a dynamic-config flip or rollback on cloud never reaches open sessions; after a rollback, stale namespaced-mode tabs show an empty model library until a manual page reload. Prefer the flag from HTTP GET /features (read from the raw response so dynamic config cannot shadow it), refresh it on reconnect, tab visibility, and a slow interval, and reload the model library when the effective value changes. Backends without the HTTP key keep today's websocket-only behavior.
The refresh triggers (reconnect, visibility, interval) can overlap; without an ordering guard a slow pre-flip response could commit after a newer one and revert the capability until the next poll. Also cover the hidden-tab skip paths in tests.
The removed comments claimed a dynamic-config entry could shadow the server capability; remoteConfig is the verbatim /features response, so that was wrong. Document the real reasons for not reusing refreshRemoteConfig (clear-on-error flap, no reconnect/visibility triggers, no ordering guard) and link FE-1439 for consolidation. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EoxqQwDLUP2VnbBPLX5E4y
83cafc6 to
837eaaa
Compare
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 `@src/platform/assets/composables/useModelTypeTagsRefresh.test.ts`:
- Around line 148-188: Update the timer advancement in the three
tests—“re-fetches on the polling interval,” “does not fetch on visibility events
or interval ticks while hidden,” and “stops all refresh triggers when the scope
is disposed”—to use vi.advanceTimersByTimeAsync for each polling interval,
preserving the existing assertions and test behavior.
- Around line 25-28: Update the beforeEach hook in useModelTypeTagsRefresh tests
to call vi.resetAllMocks() instead of vi.clearAllMocks(), while preserving the
httpSupportsModelTypeTags.value reset.
In `@src/platform/assets/composables/useModelTypeTagsRefresh.ts`:
- Around line 28-38: Update the response parsing in the refresh flow around the
features fetch to validate the untyped JSON with a small Zod schema containing
the optional supports_model_type_tags boolean field. Replace the ad hoc object
and property checks with the schema parse result, while preserving the sequence
guard and assigning undefined when the field is absent or invalid.
🪄 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: 9da3f3d2-6e87-4bda-9de2-ab35c636b9a2
📒 Files selected for processing (4)
src/composables/useFeatureFlags.test.tssrc/composables/useFeatureFlags.tssrc/platform/assets/composables/useModelTypeTagsRefresh.test.tssrc/platform/assets/composables/useModelTypeTagsRefresh.ts
- beforeEach uses vi.resetAllMocks() per path instructions - interval tests use await vi.advanceTimersByTimeAsync(...) - /features response parsed with a zod schema instead of ad hoc typeof checks; sequence guard and error semantics unchanged Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EoxqQwDLUP2VnbBPLX5E4y
There was a problem hiding this comment.
🔍 Cursor Review — Consolidated panel
Triggered by @claude[bot].
Found 6 finding(s).
| Severity | Count |
|---|---|
| 🟡 Medium | 4 |
| 🟢 Low | 2 |
Panel: 8/8 reviewers contributed findings.
| const sequence = ++refreshSequence | ||
| try { | ||
| const response = await api.fetchApi('/features', { cache: 'no-store' }) | ||
| if (!response.ok) return |
There was a problem hiding this comment.
🟡 Medium — Returning early on any !response.ok retains the last HTTP-served value, so once /features has served true and the endpoint later disappears (e.g. a backend downgrade returning 404), the flag is stuck true: the ?? in useFeatureFlags never falls through to the correct websocket value. Clear httpSupportsModelTypeTags on a 404 (endpoint gone) while keeping it on transient 5xx. Raised by 1 of 8 reviewers (gemini-3.1-pro edge-case).
There was a problem hiding this comment.
Retain-on-error is deliberate and test-pinned ("keeps the last known value on an error response" in useModelTypeTagsRefresh.test.ts): clearing on 404 would reintroduce flag flap during rolling deploys, where mixed pod sets behind a load balancer can briefly 404 the route while the websocket copy lacks the key — flipping the effective flag and firing a spurious full model-library reload, which is exactly the failure mode this composable exists to avoid. Note a 200 response without the key already clears the value (useModelTypeTagsRefresh.ts:45-47), so the common downgrade case converges correctly. The residual window is only a downgrade to a backend predating /features entirely, where the websocket value takes over on the next page load — an accepted trade-off for this PR's scope.
Generated by Claude Code
| if (!usesAssetApi()) return | ||
| try { | ||
| await reloadModels() | ||
| } catch (error) { |
There was a problem hiding this comment.
🟡 Medium — A failed reloadModels() is only logged, and because the watcher fires only on value change, a later poll that re-reports the same capability will not retrigger it — leaving the library built under the previous tagging scheme until an unrelated reload or another flag transition. Consider retrying or setting a 'needs reload' flag that a subsequent trigger can honor. Raised by 1 of 8 reviewers (gpt-5.6-sol-max edge-case).
There was a problem hiding this comment.
Real but narrow, and largely self-healing: reloadModels() calls assetService.invalidateModelBuckets() synchronously before anything can throw (src/stores/modelStore.ts:455), so even on failure the stale bucket cache is gone and any later folder load walks fresh data under the new scheme. The library also recovers on manual refresh, any scan-complete event (reloadAfterScan), or the next flag transition, and hitting the window requires the capability flip to coincide with a transient reload failure. A retry state machine isn't warranted within this PR's scope — happy to split it into a follow-up if wanted.
Generated by Claude Code
| // tagging scheme without a page reload. reloadModels also discards the | ||
| // asset service's bucket cache. | ||
| watch( | ||
| () => flags.supportsModelTypeTags, |
There was a problem hiding this comment.
🟡 Medium — The async watcher on flags.supportsModelTypeTags does not cancel or serialize an in-flight reloadModels(); if the capability flaps (true→false→true, e.g. during deploy thrash) overlapping reloads can interleave and leave the store inconsistent. Guard with an in-flight token or debounce the handler. Raised by 2 of 8 reviewers (gemini-3.1-pro adversarial, kimi-k2.7-code adversarial).
There was a problem hiding this comment.
The store already serializes this: overlapping reloadModels() calls cannot interleave commits because prepareModelFolders bumps and re-checks a monotonic request id (src/stores/modelStore.ts:351-353), and reloadModels re-checks it again right before its single atomic commit (src/stores/modelStore.ts:473-474). A superseded reload builds detached folder objects off-screen and discards them without touching live state, so a true→false→true flap wastes requests but ends with only the newest-started reload committing. No change needed.
Generated by Claude Code
…v override The refresh triggers (reconnect, visibility, interval) could overlap, so a stalled /features endpoint accumulated hung requests and a later failed fetch's sequence bump could discard an earlier successful response. Coalesce overlapping refreshes into one in-flight request with an AbortSignal timeout, matching the existing fetch-timeout pattern; the single request always commits or retains, so the start-order guard is no longer needed. Also consult the DEV-only ff: localStorage override before preferring the HTTP-served value, so the documented supports_model_type_tags override keeps working after an HTTP refresh, consistent with every other overridable flag.
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 `@src/composables/useFeatureFlags.ts`:
- Around line 252-255: In the SUPPORTS_MODEL_TYPE_TAGS override handling, update
the guard after getDevOverride<boolean> to return only when override has typeof
"boolean"; let all other values fall through to the existing HTTP/websocket
resolution. Add a regression test covering a string value such as "false" and
confirming it is not returned as the boolean override.
🪄 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: 6146c3cb-5bd0-4ff8-99dc-e0f05967a5f5
📒 Files selected for processing (4)
src/composables/useFeatureFlags.test.tssrc/composables/useFeatureFlags.tssrc/platform/assets/composables/useModelTypeTagsRefresh.test.tssrc/platform/assets/composables/useModelTypeTagsRefresh.ts
getDevOverride casts parsed JSON without validation, so a localStorage value like '"false"' (a JSON string) was returned as a truthy string from the boolean getter. Narrow the guard to typeof override === 'boolean' and fall through to the HTTP/websocket resolution otherwise, with a regression test covering the string-valued override.
…pe-flag-features-endpoint # Conflicts: # src/composables/useFeatureFlags.test.ts
📄 Knowledge reviewDosu skipped reviewing this PR because your organization has used its |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ccb7424521
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| await reloadModels() | ||
| } catch (error) { | ||
| console.error( | ||
| 'Failed to reload the model library after a capability change', | ||
| error | ||
| ) |
There was a problem hiding this comment.
Retry a failed capability-change reload
If reloadModels() encounters a transient /experiment/models or asset-walk failure during the flag transition, this catch only logs it. Subsequent 120-second /features refreshes retain the same boolean, so the watcher never runs again and the library remains built using the old tagging scheme until another action manually reloads it. Retry the reload after failure or retrigger convergence after later successful capability refreshes.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Mechanically accurate but already triaged: an earlier review round raised this exact point and the no-retry design was kept deliberately (see #14160 (comment)). The failure is largely self-healing because reloadModels() calls assetService.invalidateModelBuckets() synchronously before anything can throw (src/stores/modelStore.ts:455), so even when the walk fails the stale bucket cache is gone and any later folder load, manual refresh, scan-complete reload, or next flag transition converges to the new tagging scheme. Hitting the residual window requires the capability flip to coincide with a transient fetch failure, so a retry state machine stays out of this PR's scope; a follow-up remains an option if maintainers want one.
Generated by Claude Code
refreshRemoteConfig previously cleared remoteConfig and window.__CONFIG__
to {} on any fetch error or abort, flapping every remote-config-backed
flag to its default on a transient blip. It now only clears on a 401/403
auth transition and otherwise keeps the last-known-good value.
This lets supportsModelTypeTags move off its bespoke HTTP poller
(useModelTypeTagsRefresh, now deleted) onto the same resolveFlag path as
every other server flag.
Requested by Simon P · Slack thread
Scope narrowed following review from Simon P: this PR now ships only the approved fix — hardening
refreshRemoteConfig's error handling and sourcingsupportsModelTypeTagsfrom it directly. The OSS reconnect/visibility-trigger scope from the original proposal is deliberately cut (see below).What changed
refreshRemoteConfigused to wiperemoteConfig/window.__CONFIG__to{}on any fetch failure, including a transient network blip or the bootstrap fetch's own abort-on-timeout. Every flag sourced fromremoteConfigwould flap to its default for the rest of that tick, and anything that readswindow.__CONFIG__directly (dialogService.ts,useSubscription.ts,useSettingUI.ts) would momentarily see no config at all.AbortError) →remoteConfig.value = {}andwindow.__CONFIG__ = {}→ every remote-config-backed flag drops to its default until the next successful poll.remoteConfigState = 'error', but leaveremoteConfig.valueandwindow.__CONFIG__at their last-known-good value.Because
refreshRemoteConfigno longer flaps on transient errors,supportsModelTypeTagscan now be sourced the same way as every other server flag:useModelTypeTagsRefresh.ts(bespoke fetch + zod schema + single-flight + timeout + its own reconnect/visibility/interval triggers) and its test file are deleted outright — nothing replaces them.modelStore.ts's existing watcher onflags.supportsModelTypeTagsis untouched; it still reloads the model library when the flag flips, just fed byremoteConfignow instead of the deleted composable's ref.The cloud-only 600s poller in
cloudRemoteConfig.ts(gated behindisCloudat theextensions/core/index.tsimport site) is unchanged — still cloud-only, no reconnect/visibility triggers added anywhere.Why the OSS/reconnect scope was cut
The original proposal additionally added reconnect- and visibility-based refresh triggers for OSS/non-cloud builds. That's dropped here because the asset-API model listing this flag governs isn't reachable off-cloud today:
Comfy.Assets.UseAssetAPIdefaults tofalseoff-cloud and is markedexperimental: true(src/platform/settings/constants/coreSettings.ts).Comfy.ModelLibrary.UseAssetBrowserlikewise defaults tofalseoff-cloud and isexperimental: true.assetService.ts'susesAssetApi()(if (!isCloud) return false) anduseMediaAssetActions.ts(if (!isCloud) { ... }).An OSS user has to opt into an experimental, off-by-default setting before this flag matters at all, and even then the asset-browser surface stays inert off-cloud. Adding reconnect/visibility plumbing for a path that's unreachable in practice isn't worth the extra surface area; it can be revisited if/when the asset API ships for OSS. Consolidating the two
/featurespollers was already tracked in FE-1439 — this PR is that consolidation, minus the OSS trigger scope.Test plan
refreshRemoteConfig.test.ts: the two error-handling tests are rewritten (retains the last-known-good config when the request aborts,retains the last-known-good config on a transient fetch error) to assert retention instead of clearing; the 401/403-clears and 500-preserves tests are unchanged.useFeatureFlags.test.ts:supportsModelTypeTagstests rewritten to the same shape as otherresolveFlag-backed flags (remote-config value wins, falls back to the server feature, defaults false).modelStore.test.ts: the capability-change tests now driveremoteConfig.valueinstead of the deleted composable's ref; same reload-on-flip/no-reload-on-legacy-path assertions.pnpm typecheck,pnpm lint, and the targeted suite (refreshRemoteConfig.test.ts,useFeatureFlags.test.ts,modelStore.test.ts) all pass — 88/88 tests.Net effect
Deletes
useModelTypeTagsRefresh.tsand its test file and simplifiessupportsModelTypeTagsto a one-lineresolveFlagcall: 9 files changed, 29 insertions(+), 410 deletions(-).Generated by Claude Code