Skip to content

refactor: extract entity ID allocation into idAllocation module #15001

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
DrJKL merged 1 commit into from
Aug 10, 2026
Merged

refactor: extract entity ID allocation into idAllocation module #15001

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
73 changes: 24 additions & 49 deletions src/lib/litegraph/src/LGraph.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,16 @@ import {
import { layoutStore } from '@/renderer/core/layout/store/layoutStore'
import { toLinkId } from '@/types/linkId'
import { toRerouteId } from '@/types/rerouteId'
import {
createLGraphState,
mintGroupId,
mintNodeId,
mintRerouteId,
observeGroupId,
observeNodeId,
observeRerouteId
} from './idAllocation'
import type { LGraphState } from './idAllocation'
Comment on lines +21 to +30

@coderabbitai coderabbitai Bot Aug 10, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🗄️ 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.

import { useLinkStore } from '@/stores/linkStore'
import { useNodeDataStore } from '@/stores/nodeDataStore'
import { useRerouteStore } from '@/stores/rerouteStore'
Expand All @@ -44,7 +54,6 @@ import type { DragAndScaleState } from './DragAndScale'
import { LGraphCanvas } from './LGraphCanvas'
import { Rectangle } from './infrastructure/Rectangle'
import { LGraphGroup } from './LGraphGroup'
import { toGroupId } from '@/types/groupId'
import {
LGraphNode,
registerNodeState,
Expand Down Expand Up @@ -144,22 +153,11 @@ function isLGraphTriggerAction(action: string): action is LGraphTriggerAction {
return validTriggerActions.has(action as LGraphTriggerAction)
}

function nextNodeId(state: LGraphState): NodeId {
return toNodeId(++state.lastNodeId)
}

function numericNodeId(id: NodeId): number | null {
const numericId = Number(id)
return Number.isInteger(numericId) ? numericId : null
}

function syncLastNodeId(state: LGraphState, id: NodeId): void {
const numericId = numericNodeId(id)
if (numericId !== null && state.lastNodeId < numericId) {
state.lastNodeId = numericId
}
}

export type RendererType = 'LG' | 'Vue' | 'Vue-corrected'

/**
Expand All @@ -168,13 +166,7 @@ export type RendererType = 'LG' | 'Vue' | 'Vue-corrected'
*/
export type SubgraphId = UUID

export interface LGraphState {
/** Counter, not an id — brand at the point a group is constructed. */
lastGroupId: number
lastNodeId: number
lastLinkId: LinkId
lastRerouteId: RerouteId
}
export type { LGraphState } from './idAllocation'

type ParamsArray<T, K extends MethodNames<T>> = Parameters<
Extract<T[K], (...args: never[]) => unknown>
Expand Down Expand Up @@ -349,12 +341,7 @@ export class LGraph
list_of_graphcanvas: LGraphCanvas[] | null
status: number = LGraph.STATUS_STOPPED

private _state: LGraphState = {
lastGroupId: 0,
lastNodeId: 0,
lastLinkId: toLinkId(0),
lastRerouteId: toRerouteId(0)
}
private _state: LGraphState = createLGraphState()

get state(): LGraphState {
return this._state
Expand Down Expand Up @@ -516,12 +503,7 @@ export class LGraph
this.id = zeroUuid
this.revision = 0

this.state = {
lastGroupId: 0,
lastNodeId: 0,
lastLinkId: toLinkId(0),
lastRerouteId: toRerouteId(0)
}
this.state = createLGraphState()

// used to detect changes
this._version = -1
Expand Down Expand Up @@ -1078,9 +1060,8 @@ export class LGraph
// groups
if (node instanceof LGraphGroup) {
// Assign group ID
if (node.id == null || node.id === -1)
node.id = toGroupId(++state.lastGroupId)
if (node.id > state.lastGroupId) state.lastGroupId = node.id
if (node.id == null || node.id === -1) node.id = mintGroupId(state)
observeGroupId(state, node.id)

this._groups.push(node)
this.setDirtyCanvas(true)
Expand All @@ -1097,7 +1078,7 @@ export class LGraph
console.warn(
'LiteGraph: there is already a node with this ID, changing it'
)
node.id = nextNodeId(state)
node.id = mintNodeId(state)
}

if (this._nodes.length >= LiteGraph.MAX_NUMBER_OF_NODES) {
Expand All @@ -1106,9 +1087,9 @@ export class LGraph

// give him an id
if (node.id == null || node.id === UNASSIGNED_NODE_ID) {
node.id = nextNodeId(state)
node.id = mintNodeId(state)
} else {
syncLastNodeId(state, node.id)
observeNodeId(state, node.id)
}

// Set ghost flag before registration so the node state carries it
Expand Down Expand Up @@ -1627,12 +1608,8 @@ export class LGraph
floating
}: OptionalProps<SerialisableReroute, 'id'>): Reroute {
const rerouteId =
id === undefined
? toRerouteId(Number(this.state.lastRerouteId) + 1)
: toRerouteId(id)
if (rerouteId > this.state.lastRerouteId) {
this.state.lastRerouteId = rerouteId
}
id === undefined ? mintRerouteId(this.state) : toRerouteId(id)
observeRerouteId(this.state, rerouteId)

const existingReroute = this.reroutes.get(rerouteId)
const reroute = existingReroute ?? new Reroute(rerouteId, this, pos)
Expand All @@ -1655,8 +1632,7 @@ export class LGraph
if (!(before instanceof LLink) && !(before instanceof Reroute)) {
return
}
const rerouteId = toRerouteId(Number(this.state.lastRerouteId) + 1)
this.state.lastRerouteId = rerouteId
const rerouteId = mintRerouteId(this.state)
const chainLinks =
before instanceof Reroute
? [
Expand Down Expand Up @@ -2116,7 +2092,7 @@ export class LGraph
}
}

const newNodeId = nextNodeId(this.state)
const newNodeId = mintNodeId(this.state)
nodeIdMap.set(toNodeId(n_info.id), newNodeId)
node.id = newNodeId
n_info.id = newNodeId
Expand Down Expand Up @@ -2219,7 +2195,7 @@ export class LGraph
// Shared definitions may survive, so unpacked groups need fresh layout
// ids, like the reroutes below.
for (const groupInfo of groups) {
groupInfo.id = ++this.rootGraph.state.lastGroupId
groupInfo.id = mintGroupId(this.rootGraph.state)
const group = new LGraphGroup(groupInfo.title, groupInfo.id)
this.add(group, true)
group.configure(groupInfo)
Expand Down Expand Up @@ -2291,8 +2267,7 @@ export class LGraph
const rerouteIdMap = new Map<RerouteId, RerouteId>()
const oldReroutes = subgraphNode.subgraph.reroutes
for (const reroute of oldReroutes.values()) {
const migratedId = toRerouteId(Number(this.state.lastRerouteId) + 1)
this.state.lastRerouteId = migratedId
const migratedId = mintRerouteId(this.state)
const migratedReroute = new Reroute(migratedId, this, [
reroute.pos[0] + offsetX,
reroute.pos[1] + offsetY
Expand Down
4 changes: 2 additions & 2 deletions src/lib/litegraph/src/LGraphNode.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import { useLayoutMutations } from '@/renderer/core/layout/operations/layoutMuta
import { layoutStore } from '@/renderer/core/layout/store/layoutStore'
import { LayoutSource } from '@/renderer/core/layout/types'
import { toLinkId } from '@/types/linkId'
import { mintLinkId } from './idAllocation'
import { useNodeDataStore } from '@/stores/nodeDataStore'
import { useWidgetValueStore } from '@/stores/widgetValueStore'
import { UNASSIGNED_NODE_ID, toNodeId, serializeNodeId } from '@/types/nodeId'
Expand Down Expand Up @@ -3124,8 +3125,7 @@ export class LGraphNode
const maybeCommonType =
input.type && output.type && commonType(input.type, output.type)

const linkId = toLinkId(Number(graph.state.lastLinkId) + 1)
graph.state.lastLinkId = linkId
const linkId = mintLinkId(graph.state)

const link = new LLink(
linkId,
Expand Down
57 changes: 57 additions & 0 deletions src/lib/litegraph/src/idAllocation.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
import { describe, expect, it } from 'vitest'

import {
createLGraphState,
mintGroupId,
mintLinkId,
mintNodeId,
mintRerouteId,
observeGroupId,
observeLinkId,
observeNodeId,
observeRerouteId
} from '@/lib/litegraph/src/idAllocation'
import { toGroupId } from '@/types/groupId'
import { toLinkId } from '@/types/linkId'
import { toNodeId } from '@/types/nodeId'
import { toRerouteId } from '@/types/rerouteId'

describe('idAllocation', () => {
it('mints increasing ids for each entity kind', () => {
const state = createLGraphState()

expect([mintNodeId(state), mintNodeId(state)]).toEqual(['1', '2'])
expect([mintGroupId(state), mintGroupId(state)]).toEqual([1, 2])
expect([mintLinkId(state), mintLinkId(state)]).toEqual([1, 2])
expect([mintRerouteId(state), mintRerouteId(state)]).toEqual([1, 2])
})

it('observes higher ids and ignores lower ids', () => {
const state = createLGraphState()

observeNodeId(state, toNodeId(4))
observeNodeId(state, toNodeId(2))
observeGroupId(state, toGroupId(5))
observeGroupId(state, toGroupId(3))
observeLinkId(state, toLinkId(6))
observeLinkId(state, toLinkId(4))
observeRerouteId(state, toRerouteId(7))
observeRerouteId(state, toRerouteId(5))

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)
Comment on lines +41 to +55

@coderabbitai coderabbitai Bot Aug 10, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 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

})
})
62 changes: 62 additions & 0 deletions src/lib/litegraph/src/idAllocation.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
import { toGroupId } from '@/types/groupId'
import type { GroupId } from '@/types/groupId'
import { toLinkId } from '@/types/linkId'
import type { LinkId } from '@/types/linkId'
import { toNodeId } from '@/types/nodeId'
import type { NodeId } from '@/types/nodeId'
import { toRerouteId } from '@/types/rerouteId'
import type { RerouteId } from '@/types/rerouteId'

export interface LGraphState {
/** Counter, not an id — brand at the point a group is constructed. */
lastGroupId: number
lastNodeId: number
lastLinkId: LinkId
lastRerouteId: RerouteId
}

export function createLGraphState(): LGraphState {
return {
lastGroupId: 0,
lastNodeId: 0,
lastLinkId: toLinkId(0),
lastRerouteId: toRerouteId(0)
}
}

export function mintNodeId(state: LGraphState): NodeId {
return toNodeId(++state.lastNodeId)
}

export function mintGroupId(state: LGraphState): GroupId {
return toGroupId(++state.lastGroupId)
}

export function mintLinkId(state: LGraphState): LinkId {
state.lastLinkId = toLinkId(Number(state.lastLinkId) + 1)
return state.lastLinkId
}

export function mintRerouteId(state: LGraphState): RerouteId {
state.lastRerouteId = toRerouteId(Number(state.lastRerouteId) + 1)
return state.lastRerouteId
}

export function observeNodeId(state: LGraphState, id: NodeId): void {
const numericId = Number(id)
if (Number.isInteger(numericId) && numericId > state.lastNodeId) {
state.lastNodeId = numericId
}
}

export function observeGroupId(state: LGraphState, id: GroupId): void {
if (id > state.lastGroupId) state.lastGroupId = id
}

export function observeLinkId(state: LGraphState, id: LinkId): void {
if (id > state.lastLinkId) state.lastLinkId = id
}

export function observeRerouteId(state: LGraphState, id: RerouteId): void {
if (id > state.lastRerouteId) state.lastRerouteId = id
}
5 changes: 2 additions & 3 deletions src/lib/litegraph/src/subgraph/SubgraphInput.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { inputLink } from '@/lib/litegraph/src/node/slotLinks'
import type { LGraphNode } from '@/lib/litegraph/src/LGraphNode'
import { LLink } from '@/lib/litegraph/src/LLink'
import { toLinkId } from '@/types/linkId'
import { mintLinkId } from '../idAllocation'
import { anchorRerouteChain } from '@/lib/litegraph/src/Reroute'
import type { RerouteId } from '@/lib/litegraph/src/Reroute'
import { CustomEventTarget } from '@/lib/litegraph/src/infrastructure/CustomEventTarget'
Expand Down Expand Up @@ -102,8 +102,7 @@ export class SubgraphInput extends SubgraphSlot {
this.events.dispatch('input-connected', { input: slot })
}

const linkId = toLinkId(Number(subgraph.state.lastLinkId) + 1)
subgraph.state.lastLinkId = linkId
const linkId = mintLinkId(subgraph.state)

const link = new LLink(
linkId,
Expand Down
5 changes: 2 additions & 3 deletions src/lib/litegraph/src/subgraph/SubgraphInputNode.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import type { CanvasPointer } from '@/lib/litegraph/src/CanvasPointer'
import type { LGraphNode } from '@/lib/litegraph/src/LGraphNode'
import type { NodeId } from '@/types/nodeId'
import { LLink, slotFloatingLinks } from '@/lib/litegraph/src/LLink'
import { toLinkId } from '@/types/linkId'
import { mintLinkId } from '../idAllocation'
import type { RerouteId } from '@/lib/litegraph/src/Reroute'
import type { LinkConnector } from '@/lib/litegraph/src/canvas/LinkConnector'
import { SUBGRAPH_INPUT_ID } from '@/lib/litegraph/src/constants'
Expand Down Expand Up @@ -108,8 +108,7 @@ export class SubgraphInputNode
if (outputIndex === -1 || inputIndex === -1)
throw new Error('Invalid slot indices.')

const linkId = toLinkId(Number(subgraph.state.lastLinkId) + 1)
subgraph.state.lastLinkId = linkId
const linkId = mintLinkId(subgraph.state)

return new LLink(
linkId,
Expand Down
5 changes: 2 additions & 3 deletions src/lib/litegraph/src/subgraph/SubgraphOutput.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import type { LGraphNode } from '@/lib/litegraph/src/LGraphNode'
import { LLink } from '@/lib/litegraph/src/LLink'
import { toLinkId } from '@/types/linkId'
import { mintLinkId } from '../idAllocation'
import { anchorRerouteChain } from '@/lib/litegraph/src/Reroute'
import type { RerouteId } from '@/lib/litegraph/src/Reroute'
import type {
Expand Down Expand Up @@ -60,8 +60,7 @@ export class SubgraphOutput extends SubgraphSlot {
existingLink.disconnect(subgraph, 'input')
}

const linkId = toLinkId(Number(subgraph.state.lastLinkId) + 1)
subgraph.state.lastLinkId = linkId
const linkId = mintLinkId(subgraph.state)

const link = new LLink(
linkId,
Expand Down
Loading
Loading