Skip to content

ECS: slot rehydration, not write interception, is the fix for the {...slot} read break (promptchain) #15667

Description

@christian-byrne

The planned plain-object slot shim intercepts the write, and the promptchain break is on the read. As specified it would pass its own acceptance test and leave the pack broken. Rehydration at the assignment site is the fix.

Mechanism

link is a prototype accessor on the ECS branch (NodeInputSlot.ts:28-41), so {...slot} copies no link key at all. At the merge base it was an own data field (this.link = slot.link), so the spread carried it. That single change is the whole defect.

mobcat40/ComfyUI-PromptChain (HEAD dcc31e4) replaces slots with spread copies and then reads link off the copy:

// js/lib/order-chain.js:158  (updateInputLabels)
node.inputs[i] = { ...slot }

// js/main.js:515  (trimEmptyAutogrowSlots)
if (agInputs[i].link != null) { lastConnected = i; break; }

updateInputLabels runs, then one rAF later runs again immediately followed by trimEmptyAutogrowSlots (js/main.js:1313, :1323, :1325), so the trim always reads copies. Every autogrow input reads as disconnected and connected inputs get spliced away.

Measured at 907ca2b147:

Assertion Result
link is a getter on NodeInputSlot.prototype true
live slot.link != null on a connected input true
{...slot} has own property link false
copy.link != null, the predicate the pack evaluates false

Why a write shim does not fix it

The write already succeeds. legacySlotLinkMutations.test.ts:223-236 asserts updateEndpoints returns {ok: true} across the full promptchain sequence. A shim whose acceptance test is "the write lands" passes on day one and changes nothing for the pack.

Proposed fix

Rehydrate a plain object assigned to a numeric index of node.inputs into a NodeInputSlot bound to that node, at the moment of assignment.

The boundary is not negotiable: link resolves positionally via indexOf(slot) in the owning node's inputs array (NodeInputSlot.ts:126-130). A rehydrated slot that has not been installed at an index reads null. Verified both ways:

What link != null
new NodeInputSlot(copy, node), detached false
same object assigned to node.inputs[0] true

Read-side or serialize-time normalisation cannot work, because the pack reads its own copy in the same turn.

Prototype measurement

Scope: src/lib/litegraph/src/node/ + src/stores/linkStore.test.ts + src/lib/litegraph/src/LLink.store.test.ts, at 907ca2b147.

Arm Result
baseline 117 passed, 6 expected-fail, 123 total
rehydration on 117 passed, 3 expected-fail, 3 it.fails now passing, 123 total

Three markers flip, zero regressions: legacySlotLinkMutations.test.ts:140, :258, :268. The last two are the promptchain reproduction landed in #15595.

Cost, 20k indexed writes, 3 repetitions, with a known-non-zero control in every run: real slot write 0.57 to 0.89 us (x1.5), plain-object write 0.51 to 2.36 us (x4.6). Absolute impact is negligible against workflow load.

Out of scope

  1. Outputs. NodeOutputSlot.links has the same spread-copy loss (NodeOutputSlot.ts:36-51). Inputs only here, because that is where the promptchain break is. Three of the six it.fails markers stay red for this reason.
  2. The creation direction. fix: proxy legacy slot link mutations #15501's shim is removal-only by construction; an id alone carries no endpoint context, so node.connect() cannot be reconstructed from it.
  3. Copies a pack keeps in a local variable and never assigns back.

Acceptance criteria

The original criterion, "the write lands", is void.

  • legacySlotLinkMutations.test.ts:140, :258, :268 converted to ordinary it(...) and passing
  • 0 regressions against the 117-passing baseline
  • Mutation-verified: remove the rehydration branch, confirm exactly those three go red, restore, confirm green, report the numbers
  • A test pins the positional requirement, so a later refactor cannot move the boundary quietly
  • Manual pass on promptchain: build an autogrow chain, connect inputs, confirm connected inputs are not spliced away
  • The three uncovered cases stay it.fails asserting the wanted behaviour

Open question for review

Restoring link as an own enumerable data field would fix every spread-copy reader at once, including outputs and detached copies, but reintroduces the mirror the ECS design removed. Not costed. This is the alternative most likely to come up in review.

Note on verification

The compat battery cannot confirm this fix. It drives only the stock 7-node workflow with a pack merely resident and never invokes pack menu handlers or pack-owned node types, which is where promptchain's autogrow logic lives. A green matrix is not evidence that promptchain works. See the seeded-defect control for what the battery does and does not detect.

Full specification, including provenance and the corrected figures, is in the ECS migration workspace at program/a5-rehydration-spec.md.

Metadata

Metadata

Assignees

No one assigned

    Labels

    Potential BugUntriaged bugPublic APIAffects or interacts with the public API surface (affecting custom node or extension authors)area:nodes

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions