refactor: extract entity ID allocation into idAllocation module - #15001
Merged
Conversation
Collapses the copy-pasted lastLinkId/lastNodeId/lastGroupId/lastRerouteId increment-and-sync idioms across LGraph, LGraphNode, subgraph slots, and subgraphDeduplication into mint*/observe* helpers. No behavior change.
dosubot
Bot
added
the
size:L
Contributor
📝 WalkthroughWalkthroughLiteGraph now centralizes graph, node, group, link, and reroute ID allocation. Graph initialization, creation, subgraph unpacking, deduplication, and link connections use shared minting and observation functions. Tests cover sequential allocation and ID observation. ChangesCentralized ID allocation
Estimated code review effort: 3 (Moderate) | ~20 minutes 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)
Comment |
🎨 Storybook: ✅ Built — View Storybook🎭 Playwright: ✅ 1759 passed, 0 failed · 4 flaky📊 Browser Reports
📦 Bundle Size
⚡ Performance Report
Absolute values
Raw data{
"timestamp": "2026-08-10T20:39:55.349Z",
"gitSha": "3afd57d2f56db237f97e2395a947257d06d2d099",
"branch": "drjkl/split-id-allocation",
"measurements": [
{
"name": "canvas-idle",
"durationMs": 2037.9640000000165,
"styleRecalcs": 9,
"styleRecalcDurationMs": 7.835,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 495.776,
"heapDeltaBytes": 4506588,
"heapUsedBytes": 73999068,
"domNodes": 18,
"jsHeapTotalBytes": 19263488,
"scriptDurationMs": 26.943,
"eventListeners": 4,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "canvas-idle",
"durationMs": 2038.1209999999896,
"styleRecalcs": 9,
"styleRecalcDurationMs": 8.095,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 488.64500000000004,
"heapDeltaBytes": 4294292,
"heapUsedBytes": 74108548,
"domNodes": 18,
"jsHeapTotalBytes": 19001344,
"scriptDurationMs": 27.078000000000003,
"eventListeners": 4,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66333333333335,
"p95FrameDurationMs": 16.799999999999272
},
{
"name": "canvas-mouse-sweep",
"durationMs": 2091.6629999999827,
"styleRecalcs": 79,
"styleRecalcDurationMs": 44.713,
"layouts": 12,
"layoutDurationMs": 3.594,
"taskDurationMs": 1071.0959999999998,
"heapDeltaBytes": -14869288,
"heapUsedBytes": 54842604,
"domNodes": -275,
"jsHeapTotalBytes": 20439040,
"scriptDurationMs": 207.122,
"eventListeners": -152,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "canvas-mouse-sweep",
"durationMs": 1903.519000000074,
"styleRecalcs": 74,
"styleRecalcDurationMs": 36.797,
"layouts": 12,
"layoutDurationMs": 3.1499999999999995,
"taskDurationMs": 944.8599999999999,
"heapDeltaBytes": -17643156,
"heapUsedBytes": 52127936,
"domNodes": -277,
"jsHeapTotalBytes": 19652608,
"scriptDurationMs": 174.26500000000001,
"eventListeners": -152,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66333333333332,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "canvas-zoom-sweep",
"durationMs": 1737.2950000000174,
"styleRecalcs": 31,
"styleRecalcDurationMs": 17.467999999999996,
"layouts": 6,
"layoutDurationMs": 0.603,
"taskDurationMs": 399.53999999999996,
"heapDeltaBytes": 7885892,
"heapUsedBytes": 77423924,
"domNodes": 78,
"jsHeapTotalBytes": 19001344,
"scriptDurationMs": 27.588,
"eventListeners": 19,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "canvas-zoom-sweep",
"durationMs": 1704.8070000000735,
"styleRecalcs": 31,
"styleRecalcDurationMs": 18.342,
"layouts": 6,
"layoutDurationMs": 0.6209999999999998,
"taskDurationMs": 394.154,
"heapDeltaBytes": 7792120,
"heapUsedBytes": 77433520,
"domNodes": 79,
"jsHeapTotalBytes": 19001344,
"scriptDurationMs": 32.37800000000001,
"eventListeners": 19,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "dom-widget-clipping",
"durationMs": 594.8570000000188,
"styleRecalcs": 11,
"styleRecalcDurationMs": 8.166,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 409.589,
"heapDeltaBytes": -9842136,
"heapUsedBytes": 59798900,
"domNodes": 18,
"jsHeapTotalBytes": 19525632,
"scriptDurationMs": 90.533,
"eventListeners": 0,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "dom-widget-clipping",
"durationMs": 609.6090000000913,
"styleRecalcs": 10,
"styleRecalcDurationMs": 7.066000000000001,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 411.151,
"heapDeltaBytes": -10159420,
"heapUsedBytes": 59594588,
"domNodes": 16,
"jsHeapTotalBytes": 20312064,
"scriptDurationMs": 89.616,
"eventListeners": 2,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66333333333335,
"p95FrameDurationMs": 16.799999999999272
},
{
"name": "large-graph-idle",
"durationMs": 2080.287999999996,
"styleRecalcs": 8,
"styleRecalcDurationMs": 7.6530000000000005,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 887.16,
"heapDeltaBytes": -3666056,
"heapUsedBytes": 75222532,
"domNodes": -275,
"jsHeapTotalBytes": 3063808,
"scriptDurationMs": 286.32599999999996,
"eventListeners": -146,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66333333333332,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "large-graph-idle",
"durationMs": 2085.044000000039,
"styleRecalcs": 8,
"styleRecalcDurationMs": 7.511,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 888.368,
"heapDeltaBytes": -3703952,
"heapUsedBytes": 75011572,
"domNodes": -273,
"jsHeapTotalBytes": 3588096,
"scriptDurationMs": 283.75600000000003,
"eventListeners": -146,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "large-graph-pan",
"durationMs": 2792.476000000022,
"styleRecalcs": 68,
"styleRecalcDurationMs": 12.341000000000001,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 1980.313,
"heapDeltaBytes": -931920,
"heapUsedBytes": 79784944,
"domNodes": -278,
"jsHeapTotalBytes": 3268608,
"scriptDurationMs": 1101.5890000000002,
"eventListeners": -146,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "large-graph-pan",
"durationMs": 2744.503000000009,
"styleRecalcs": 68,
"styleRecalcDurationMs": 11.706000000000001,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 1937.707,
"heapDeltaBytes": -4837492,
"heapUsedBytes": 75613972,
"domNodes": -279,
"jsHeapTotalBytes": 3792896,
"scriptDurationMs": 1090.568,
"eventListeners": -148,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "large-graph-zoom",
"durationMs": 4012.0709999999917,
"styleRecalcs": 64,
"styleRecalcDurationMs": 13.313000000000002,
"layouts": 60,
"layoutDurationMs": 7.199000000000001,
"taskDurationMs": 2187.111,
"heapDeltaBytes": 5237048,
"heapUsedBytes": 65732248,
"domNodes": -280,
"jsHeapTotalBytes": 2158592,
"scriptDurationMs": 1204.501,
"eventListeners": -150,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "large-graph-zoom",
"durationMs": 3953.856000000087,
"styleRecalcs": 62,
"styleRecalcDurationMs": 12.001999999999999,
"layouts": 60,
"layoutDurationMs": 7.2940000000000005,
"taskDurationMs": 2159.5879999999997,
"heapDeltaBytes": -370644,
"heapUsedBytes": 60284240,
"domNodes": -284,
"jsHeapTotalBytes": 2420736,
"scriptDurationMs": 1183.146,
"eventListeners": -150,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66999999999998,
"p95FrameDurationMs": 16.799999999999272
},
{
"name": "legacy-node-drag",
"durationMs": 4154.177000000004,
"styleRecalcs": 45,
"styleRecalcDurationMs": 7.898000000000002,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 2764.5930000000003,
"heapDeltaBytes": -2867140,
"heapUsedBytes": 58824744,
"domNodes": -280,
"jsHeapTotalBytes": 7925760,
"scriptDurationMs": 657.314,
"eventListeners": -105,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.799999999999272
},
{
"name": "legacy-node-drag",
"durationMs": 4180.3619999999455,
"styleRecalcs": 44,
"styleRecalcDurationMs": 7.581000000000001,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 2797.0249999999996,
"heapDeltaBytes": 6999284,
"heapUsedBytes": 69263632,
"domNodes": -282,
"jsHeapTotalBytes": 8855552,
"scriptDurationMs": 662.624,
"eventListeners": -103,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66333333333332,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "minimap-idle",
"durationMs": 2052.0779999999945,
"styleRecalcs": 8,
"styleRecalcDurationMs": 7.352000000000001,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 873.91,
"heapDeltaBytes": 20070740,
"heapUsedBytes": 80445140,
"domNodes": -270,
"jsHeapTotalBytes": -724992,
"scriptDurationMs": 285.182,
"eventListeners": -146,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "minimap-idle",
"durationMs": 2038.411999999994,
"styleRecalcs": 7,
"styleRecalcDurationMs": 6.586000000000001,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 860.347,
"heapDeltaBytes": 17362780,
"heapUsedBytes": 76854152,
"domNodes": -277,
"jsHeapTotalBytes": 61440,
"scriptDurationMs": 271.946,
"eventListeners": -146,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.660000000000007,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "subgraph-dom-widget-clipping",
"durationMs": 604.8680000000104,
"styleRecalcs": 48,
"styleRecalcDurationMs": 10.898000000000001,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 423.453,
"heapDeltaBytes": -9521660,
"heapUsedBytes": 60099876,
"domNodes": 22,
"jsHeapTotalBytes": 19263488,
"scriptDurationMs": 133.94400000000002,
"eventListeners": 6,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.699999999999818
},
{
"name": "subgraph-dom-widget-clipping",
"durationMs": 611.5969999999606,
"styleRecalcs": 47,
"styleRecalcDurationMs": 11.398,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 414.749,
"heapDeltaBytes": -10117748,
"heapUsedBytes": 59422880,
"domNodes": 20,
"jsHeapTotalBytes": 19787776,
"scriptDurationMs": 136.17499999999998,
"eventListeners": 6,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.670000000000012,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "subgraph-idle",
"durationMs": 2009.0739999999983,
"styleRecalcs": 10,
"styleRecalcDurationMs": 9.666999999999998,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 502.132,
"heapDeltaBytes": 3962444,
"heapUsedBytes": 73773756,
"domNodes": 20,
"jsHeapTotalBytes": 19263488,
"scriptDurationMs": 19.126000000000005,
"eventListeners": 4,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66333333333335,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "subgraph-idle",
"durationMs": 2015.8270000000584,
"styleRecalcs": 9,
"styleRecalcDurationMs": 8.290000000000001,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 476.49100000000004,
"heapDeltaBytes": 3980292,
"heapUsedBytes": 73769492,
"domNodes": 18,
"jsHeapTotalBytes": 19001344,
"scriptDurationMs": 19.534999999999997,
"eventListeners": 4,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "subgraph-mouse-sweep",
"durationMs": 1757.4920000000134,
"styleRecalcs": 76,
"styleRecalcDurationMs": 37.922000000000004,
"layouts": 16,
"layoutDurationMs": 4.462,
"taskDurationMs": 862.4110000000001,
"heapDeltaBytes": -21245496,
"heapUsedBytes": 48419556,
"domNodes": -272,
"jsHeapTotalBytes": 18341888,
"scriptDurationMs": 118.31800000000001,
"eventListeners": -152,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.699999999999818
},
{
"name": "subgraph-mouse-sweep",
"durationMs": 1706.5170000000762,
"styleRecalcs": 75,
"styleRecalcDurationMs": 34.384,
"layouts": 16,
"layoutDurationMs": 4.425999999999999,
"taskDurationMs": 828.981,
"heapDeltaBytes": -19732844,
"heapUsedBytes": 49840204,
"domNodes": -274,
"jsHeapTotalBytes": 19390464,
"scriptDurationMs": 117.98100000000001,
"eventListeners": -152,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "subgraph-transition-enter",
"durationMs": 1383.0939999999714,
"styleRecalcs": 19,
"styleRecalcDurationMs": 31.10099999999999,
"layouts": 15,
"layoutDurationMs": 12.562999999999997,
"taskDurationMs": 998.8569999999999,
"heapDeltaBytes": 26554280,
"heapUsedBytes": 95037172,
"domNodes": 13671,
"jsHeapTotalBytes": 14942208,
"scriptDurationMs": 49.774999999999984,
"eventListeners": 2379,
"totalBlockingTimeMs": 129,
"frameDurationMs": 16.66333333333335,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "viewport-pan-sweep",
"durationMs": 9252.811000000009,
"styleRecalcs": 250,
"styleRecalcDurationMs": 35.734,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 6693.984,
"heapDeltaBytes": 5670428,
"heapUsedBytes": 84640568,
"domNodes": -275,
"jsHeapTotalBytes": 5337088,
"scriptDurationMs": 3550.756,
"eventListeners": -130,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "viewport-pan-sweep",
"durationMs": 9397.487999999954,
"styleRecalcs": 250,
"styleRecalcDurationMs": 35.910000000000004,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 6736.321,
"heapDeltaBytes": 9697120,
"heapUsedBytes": 88891804,
"domNodes": -273,
"jsHeapTotalBytes": 6123520,
"scriptDurationMs": 3575.122,
"eventListeners": -130,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "vue-large-graph-idle",
"durationMs": 17566.337000000032,
"styleRecalcs": 0,
"styleRecalcDurationMs": 0,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 17546.391,
"heapDeltaBytes": -34330028,
"heapUsedBytes": 163781388,
"domNodes": -6548,
"jsHeapTotalBytes": 61440,
"scriptDurationMs": 1101.4470000000001,
"eventListeners": -7471,
"totalBlockingTimeMs": 0,
"frameDurationMs": 17.776666666666763,
"p95FrameDurationMs": 16.799999999999272
},
{
"name": "vue-large-graph-idle",
"durationMs": 18093.944999999963,
"styleRecalcs": 0,
"styleRecalcDurationMs": 0,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 18077.813999999995,
"heapDeltaBytes": -35666824,
"heapUsedBytes": 164408792,
"domNodes": -6548,
"jsHeapTotalBytes": -462848,
"scriptDurationMs": 1183.5739999999998,
"eventListeners": -7469,
"totalBlockingTimeMs": 0,
"frameDurationMs": 17.780000000000047,
"p95FrameDurationMs": 16.799999999999272
},
{
"name": "vue-large-graph-pan",
"durationMs": 20788.815,
"styleRecalcs": 141,
"styleRecalcDurationMs": 18.143000000000022,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 20765.788,
"heapDeltaBytes": -36666372,
"heapUsedBytes": 163213948,
"domNodes": -6548,
"jsHeapTotalBytes": 1110016,
"scriptDurationMs": 1583.623,
"eventListeners": -7467,
"totalBlockingTimeMs": 346,
"frameDurationMs": 17.776666666666642,
"p95FrameDurationMs": 16.80000000000291
},
{
"name": "vue-large-graph-pan",
"durationMs": 22081.538000000022,
"styleRecalcs": 162,
"styleRecalcDurationMs": 21.01200000000003,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 22058.795,
"heapDeltaBytes": -37623936,
"heapUsedBytes": 157824608,
"domNodes": -6548,
"jsHeapTotalBytes": 1110016,
"scriptDurationMs": 1632.869,
"eventListeners": -7467,
"totalBlockingTimeMs": 594,
"frameDurationMs": 17.776666666666642,
"p95FrameDurationMs": 16.80000000000291
},
{
"name": "workflow-execution",
"durationMs": 451.3939999999934,
"styleRecalcs": 15,
"styleRecalcDurationMs": 21.177,
"layouts": 2,
"layoutDurationMs": 0.4730000000000001,
"taskDurationMs": 120.57800000000002,
"heapDeltaBytes": -16255420,
"heapUsedBytes": 51971328,
"domNodes": 130,
"jsHeapTotalBytes": 7204864,
"scriptDurationMs": 11.015,
"eventListeners": 65,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "workflow-execution",
"durationMs": 453.7710000000743,
"styleRecalcs": 13,
"styleRecalcDurationMs": 19.730999999999998,
"layouts": 3,
"layoutDurationMs": 0.5770000000000001,
"taskDurationMs": 112.09,
"heapDeltaBytes": -16508660,
"heapUsedBytes": 51820808,
"domNodes": 119,
"jsHeapTotalBytes": 7467008,
"scriptDurationMs": 10.346,
"eventListeners": 65,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66333333333332,
"p95FrameDurationMs": 16.700000000000728
}
]
} |
Codecov Report❌ Patch coverage is
@@ Coverage Diff @@
## feature/ecs-migration #15001 +/- ##
========================================================
Coverage ? 79.24%
========================================================
Files ? 1770
Lines ? 102930
Branches ? 33392
========================================================
Hits ? 81566
Misses ? 20895
Partials ? 469
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
christian-byrne
approved these changes
Aug 10, 2026
![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: 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/lib/litegraph/src/idAllocation.test.ts`:
- Around line 41-55: Update the allocation tests around createLGraphState and
the observeNodeId test to assert behavior through mintGroupId, mintNodeId,
mintLinkId, and mintRerouteId rather than inspecting LGraphState fields. Verify
the next allocated IDs are 5, 6, 7, and 8, and after observing numeric-string ID
'12' plus 'named', verify mintNodeId(state) returns '13'.
In `@src/lib/litegraph/src/LGraph.ts`:
- Around line 21-30: Update the ID allocation imports to include observeLinkId,
then call observeLinkId from _addLink before registering the link so existing
link IDs advance the allocator. Preserve the existing _links.set(link.id, link)
behavior while ensuring configure-loaded links cannot collide with subsequently
minted IDs.
🪄 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: d18c3993-09cf-4bd6-bbdd-04769094709e
📒 Files selected for processing (8)
src/lib/litegraph/src/LGraph.tssrc/lib/litegraph/src/LGraphNode.tssrc/lib/litegraph/src/idAllocation.test.tssrc/lib/litegraph/src/idAllocation.tssrc/lib/litegraph/src/subgraph/SubgraphInput.tssrc/lib/litegraph/src/subgraph/SubgraphInputNode.tssrc/lib/litegraph/src/subgraph/SubgraphOutput.tssrc/lib/litegraph/src/subgraph/subgraphDeduplication.ts
Comment on lines
+41
to
+55
| expect(state).toEqual({ | ||
| lastGroupId: 5, | ||
| lastNodeId: 4, | ||
| lastLinkId: 6, | ||
| lastRerouteId: 7 | ||
| }) | ||
| }) | ||
|
|
||
| it('observes numeric-string node ids', () => { | ||
| const state = createLGraphState() | ||
|
|
||
| observeNodeId(state, toNodeId('12')) | ||
| observeNodeId(state, toNodeId('named')) | ||
|
|
||
| expect(state.lastNodeId).toBe(12) |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Assert the next allocated IDs after observation.
The assertions read LGraphState internals. They do not prove that mint*Id consumes observed IDs. Assert the next IDs are 5, 6, 7, and 8. After observing '12' and 'named', assert that mintNodeId(state) returns '13'.
Proposed test update
- expect(state).toEqual({
- lastGroupId: 5,
- lastNodeId: 4,
- lastLinkId: 6,
- lastRerouteId: 7
- })
+ expect(mintNodeId(state)).toBe('5')
+ expect(mintGroupId(state)).toBe(6)
+ expect(mintLinkId(state)).toBe(7)
+ expect(mintRerouteId(state)).toBe(8)
@@
- expect(state.lastNodeId).toBe(12)
+ expect(mintNodeId(state)).toBe('13')As per path instructions, “Review the allocation tests for behavioral coverage rather than implementation details.”
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| expect(state).toEqual({ | |
| lastGroupId: 5, | |
| lastNodeId: 4, | |
| lastLinkId: 6, | |
| lastRerouteId: 7 | |
| }) | |
| }) | |
| it('observes numeric-string node ids', () => { | |
| const state = createLGraphState() | |
| observeNodeId(state, toNodeId('12')) | |
| observeNodeId(state, toNodeId('named')) | |
| expect(state.lastNodeId).toBe(12) | |
| expect(mintNodeId(state)).toBe('5') | |
| expect(mintGroupId(state)).toBe(6) | |
| expect(mintLinkId(state)).toBe(7) | |
| expect(mintRerouteId(state)).toBe(8) | |
| }) | |
| it('observes numeric-string node ids', () => { | |
| const state = createLGraphState() | |
| observeNodeId(state, toNodeId('12')) | |
| observeNodeId(state, toNodeId('named')) | |
| expect(mintNodeId(state)).toBe('13') |
🤖 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/lib/litegraph/src/idAllocation.test.ts` around lines 41 - 55, Update the
allocation tests around createLGraphState and the observeNodeId test to assert
behavior through mintGroupId, mintNodeId, mintLinkId, and mintRerouteId rather
than inspecting LGraphState fields. Verify the next allocated IDs are 5, 6, 7,
and 8, and after observing numeric-string ID '12' plus 'named', verify
mintNodeId(state) returns '13'.
Source: Path instructions
Comment on lines
+21
to
+30
| import { | ||
| createLGraphState, | ||
| mintGroupId, | ||
| mintNodeId, | ||
| mintRerouteId, | ||
| observeGroupId, | ||
| observeNodeId, | ||
| observeRerouteId | ||
| } from './idAllocation' | ||
| import type { LGraphState } from './idAllocation' |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Observe IDs when registering existing links.
Import observeLinkId and call it in _addLink. configure() can load links when data.state is absent or stale. The next mintLinkId can then reuse an existing key. At Line 1535, _links.set(link.id, link) replaces the previous link and corrupts graph topology.
Proposed fix
import {
createLGraphState,
mintGroupId,
mintNodeId,
mintRerouteId,
observeGroupId,
+ observeLinkId,
observeNodeId,
observeRerouteId
} from './idAllocation'
@@
_addLink(link: LLink): void {
+ observeLinkId(this.state, link.id)
this._links.set(link.id, link)
registerLinkTopology(this, link)
}🤖 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/lib/litegraph/src/LGraph.ts` around lines 21 - 30, Update the ID
allocation imports to include observeLinkId, then call observeLinkId from
_addLink before registering the link so existing link IDs advance the allocator.
Preserve the existing _links.set(link.id, link) behavior while ensuring
configure-loaded links cannot collide with subsequently minted IDs.
4 tasks
DrJKL
added a commit
that referenced
this pull request
Aug 13, 2026
Split 2/6 of #14480. Stacked on #15001. Fixes cross-subgraph-definition topology contamination: `linkStore`/`rerouteStore` buckets were keyed only by root graph id, but every subgraph definition numbers links from 1 and shares the IO pseudo-node ids (−10/−20). Two definitions under one root therefore shared an origin index and reroute membership — one definition's links joined another definition's reroute chains, driving wrong reroute pruning on configure. Repro: `browser_tests/tests/subgraph/subgraphLinkIdentity.spec.ts` + fixture. Changes: - Adds `GraphScope` (`rootGraphId`, `owningGraphId`) and scopes topology queries, owner-local iteration, and teardown to the graph that directly owns each entity. Root-wide identity lookup remains keyed by `rootGraphId`. - Makes `linkStore`, `rerouteStore`, and `nodeDataStore` the authoritative collision boundaries. Registrations are first-wins; rejected entities remain detached and are not assigned to the requesting owner. - Shares link and reroute ID allocation across a root and its subgraphs. Serialized subgraphs are deduplicated before registration so link, reroute, node, widget, and promoted-widget references are remapped together. - Keeps endpoint swaps, rotations, and removals atomic within `linkStore`: an entire batch is validated before indexes or topology fields change. `LLink` retains the store-held reactive topology object across ID and endpoint updates. - Treats configure payloads as authoritative for their owner by clearing owner-local registrations before rebuilding them, with empty root-bucket pruning. - Derives topology scope from the directly owning graph at callers such as `NodeSlots.vue` and `useProcessedWidgets.ts`. - Positions promoted DOM widgets immediately on mount instead of waiting for a watched value to change. - Removes redundant graph-local collision checks, floating-link ID bookkeeping, unreachable tuple deduplication, obsolete compatibility types, and mock-only tests. - Documents root-vs-owner query guidance, topology transaction boundaries, and the deferred measurement of root-wide group scans before introducing another persistent index. Behavior notes: - Constructing or configuring an `LGraph` from serialized data requires an active Pinia because entities register with topology stores during deserialization. - Runtime duplicate IDs are rejected by the root-scoped store; persisted collisions are remapped before object registration. - Registration failures remain explicit return values and do not throw errors. --------- Co-authored-by: Amp <amp@ampcode.com> Co-authored-by: github-actions <github-actions@github.com>
This was referenced Aug 13, 2026
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.
Split 1/6 of #14480 (see that PR for the full map).
Collapses the copy-pasted
lastLinkId/lastNodeId/lastGroupId/lastRerouteIdincrement-and-sync idioms acrossLGraph,LGraphNode, subgraph slots, andsubgraphDeduplicationintomint*/observe*helpers insrc/lib/litegraph/src/idAllocation.ts. No behavior change.Notes for review:
src/lib/litegraph/src/(litegraph-only behavior), notsrc/types/as in fix(layout): enforce per-instance geometry ownership #14480.snapshotIdState/restoreIdStatefrom fix(layout): enforce per-instance geometry ownership #14480 are omitted — no callers until the layout rollback PR; they land there.LGraphStateis re-exported fromLGraph.tsso existing importers are unaffected.