ECS follow-on PRs: test coverage todos for reviewer-flagged theoretical fixes

_Requested by **Christian Byrne** · [Slack thread](https://comfy-organization.slack.com/archives/C0AR9UCKR9V/p1787720594470379?thread_ts=1787720594.470379&cid=C0AR9UCKR9V)_

Sub-issue of #15929 (part of FE-1857).

Tracking issue aggregating test-coverage gaps discovered while analyzing review activity across the ECS follow-on PR cluster (#15882, #15731, #15924, #15689, #15720, #15767, #15613, #15688, #15779, #15966, #15394, #15422, #15723, #15729, #15393, #15920, #15810, #15857). Each item below was flagged because a reviewer (human or bot) identified a theoretical bug or asserted a data/API contract by reasoning about the diff, and the corresponding regression test does not yet exist in the source PR. Items already carrying test coverage in their source PR were excluded. No test code is written here — this issue only tracks the gaps.

## E2E test todos

No standalone Playwright/`browser_tests` gaps met the inclusion bar independently of unit coverage — every e2e-shaped scenario surfaced during analysis was attached to a fix that already shipped with test coverage in its source PR (e.g. #15422, #15393, #15767's declined-promotion fix). If follow-up work turns up a case that needs true UI-level (not unit-level) coverage, add it here as a new item rather than leaving this section empty by omission.

## Unit test todos

- [ ] **Subgraph inherits the remint-remap fix via `super.configure()`** (PR #15882 — [PR](https://github.com/Comfy-Org/ComfyUI_frontend/pull/15882)): Add a Subgraph-flavored counterpart of `LGraph.remintLinkRemap.test.ts` — construct a `Subgraph` (not `LGraph`), register an incumbent node id, call `subgraph.configure()` with a payload whose colliding node has links naming it by the requested id (regular + floating), and assert the resulting link endpoints follow the reminted id.
- [ ] **`BaseWidget.name` enumerability after becoming an accessor** (PR #15731 — [PR](https://github.com/Comfy-Org/ComfyUI_frontend/pull/15731)): Construct a `BaseWidget` with a non-default `name`, then assert what `Object.keys(widget)`, `{...widget}`, and `JSON.stringify(widget)` actually produce for `name`, pinning the current spread/enumeration behavior so a future change to `_name`'s declaration doesn't silently break custom nodes that spread/serialize widgets.
- [ ] **Extension `configure()` hooks must receive an isolated clone, not a live reference** (PR #15924 — [discussion](https://github.com/Comfy-Org/ComfyUI_frontend/pull/15924#discussion_r3858723898)): Run the configure-hook/`hydrateExtensionPayload` path on a node with extensions-namespaced fields, mutate the object handed to the hook, and assert it is not reference-equal to the caller's canonical/serialized JSON and that mutations don't leak back; also assert namespaced extension keys aren't promoted to top-level legacy fields on subsequent `serialize()`.
- [ ] **`WidgetDetachMode: 'keep-values'` may be unreachable dead code** (PR #15924 — [discussion](https://github.com/Comfy-Org/ComfyUI_frontend/pull/15924#discussion_r3858723911)): Call `LGraph.removeNode` on a node with distinct widget values, simulate the undo-of-delete re-add path in `'keep-values'` mode, and assert the widget values are actually recoverable rather than already cleared by `clearNodeOwnedStoreState` running before `detachNodeFromStores`.
- [ ] **`getNodeOnPos` must resolve z-order, not insertion order** (PR #15924 — [discussion](https://github.com/Comfy-Org/ComfyUI_frontend/pull/15924#discussion_r3858723872)): Create two overlapping nodes, reorder z-index via `setNodeOrder`/`bringToFront` without touching insertion order in `_nodes`, then call `graph.getNodeOnPos(x, y)` with no explicit `nodeList` and assert it returns the highest-z node, not the most recently added one.
- [ ] **Store/adapter consistency guard is one-directional** (PR #15924 — [discussion](https://github.com/Comfy-Org/ComfyUI_frontend/pull/15924#discussion_r3858723924)): Push a node directly onto `owner._nodes` without a matching `nodeDataStore` entry, call `graph.serialize()`, and assert the guard fires or the node is still present in the output rather than silently dropped (verify it also checks `ordered.length !== owner._nodes.length`, not only `serialisers.length !== ordered.length`).
- [ ] **`ensureUniqueWidgetNames` must not write to the store from a read path** (PR #15924 — [discussion](https://github.com/Comfy-Org/ComfyUI_frontend/pull/15924#discussion_r3858723880)): Spy on `useWidgetValueStore().renameWidget`; construct a node with duplicate widget names and read `BaseWidget.widgetId` / evaluate `processedWidgets` in a reactive context; assert the rename action is never invoked from the read path.
- [ ] **`mapLiveWidgetsById` must not collapse to an empty map on a single registration failure** (PR #15924 — [discussion](https://github.com/Comfy-Org/ComfyUI_frontend/pull/15924#discussion_r3858723878)): Force `ensureUniqueWidgetNames` to fail for one widget on a node with several widgets, call `mapLiveWidgetsById`, and assert it still returns one entry per existing widget instead of an empty map.
- [x] **`transformLinkReferences` coverage parity across all serialized reference kinds — closed/superseded by remap-only ruling** (PR #15689 — [commit](https://github.com/Comfy-Org/ComfyUI_frontend/commit/86688bc32ae8f4283c46198364cf8075cf30c31b)): Christian selected remap-only after the ECS-oriented review in [blocked-on-christian #205](https://github.com/christian-byrne/blocked-on-christian/issues/205#issuecomment-5492121547). Different-origin prune never existed on `main`, so the original replace/remove parity request would invent behavior. Remap coverage across the five reference locations remains tracked separately.
- [ ] **`LinkDedupDropMetadata` telemetry event must actually fire with the correct payload** (PR #15689 — [discussion](https://github.com/Comfy-Org/ComfyUI_frontend/pull/15689#discussion_r3849031714)): Mock/spy the telemetry emitter and, for a two-different-origin-links-target-same-slot fixture, assert the event fires exactly once with `{ droppedLinkId, survivorLinkId, target }` — independent of the existing `console.warn` string assertions — plus a same-origin (remap) case asserting the event does NOT fire.
- [ ] **`widgetValueStore.registerWidget` return-value contract** (PR #15767 — [review](https://github.com/Comfy-Org/ComfyUI_frontend/pull/15767#pullrequestreview-5024107702)): In `widgetValueStore.test.ts`, call `registerWidget()` twice with the same id/matching type and assert the second call returns truthy; call it with an id derived from an empty widget name and assert it returns falsy — pinning both halves of the contract `SubgraphNode._setWidget` relies on.
- [ ] **`widgetId()` percent-encoding / un-keyable classification contract** (PR #15767 — [review](https://github.com/Comfy-Org/ComfyUI_frontend/pull/15767#pullrequestreview-5024107702)): Assert `widgetId()` percent-encodes separator-colliding names/ids so distinct `(nodeId, name)` pairs never collide on the composed key, and that the un-keyable/failure classification fires only for the empty-name-derived key.
- [ ] **`ISerialisedNode.widgets_values` must model `null` slots, and `configure()` must handle them explicitly** (PR #15688 — [discussion](https://github.com/Comfy-Org/ComfyUI_frontend/pull/15688#discussion_r3846672121)): Add a type-level check that `{ widgets_values: [30, null, 12345] } satisfies ISerialisedNode` compiles without an `as unknown as` cast, paired with a `LGraphNode.configure()` unit test that explicitly asserts the `null`-slot case is handled by name.
- [x] **Input-slot rehydration outside `LGraphNode` construction — closed/superseded by derived-view slot architecture** (PR #15779 — [discussion](https://github.com/Comfy-Org/ComfyUI_frontend/pull/15779#discussion_r3856788841)): Christian selected Decision A in [blocked-on-christian #206](https://github.com/christian-byrne/blocked-on-christian/issues/206#issuecomment-5492113374), following Alex's recorded D-dq-08 and D-xc-102-slot-owner decisions. ID-based slot records remain authoritative and class instances remain derived views, so eager non-constructor `createInputSlotView` rehydration is intentionally not restored.
- [x] **Assigned plain input-slot object identity vs. `getInputSlotPos` — resolved by store-backed facade mapping**: Christian deferred to Alex’s `D-xc-102-slot-owner` derived-view architecture in [blocked-on-christian #214](https://github.com/christian-byrne/blocked-on-christian/issues/214#issuecomment-5493248488). Draft PR [#16510](https://github.com/Comfy-Org/ComfyUI_frontend/pull/16510) keeps the installed `NodeInputSlot` authoritative and maps a retained legacy assignment reference to that stable view, with reorder regression coverage.
- [ ] **Store collision-handling contracts across all four entity stores** (PR #15810 — [discussion](https://github.com/Comfy-Org/ComfyUI_frontend/pull/15810#discussion_r3848177912), EX-002; check whether `storeCollisionContracts.test.ts` added in #15720 already covers this before adding a duplicate): For `nodeDataStore`, `linkStore`, `rerouteStore`, insert an entity at an existing identity key and assert the store rejects/re-mints rather than overwriting; for `widgetValueStore`, insert at a colliding structural key and assert it resolves per documented structural-key semantics.
- [ ] **`LLink` forwarding-descriptor properties must reflect live store state on spread** (PR #15810 — [discussion](https://github.com/Comfy-Org/ComfyUI_frontend/pull/15810#discussion_r3848177912), EX-003; check PR #15654/#15778 for existing coverage first): Construct an `LLink` backed by a store record, mutate the underlying store state, `{...link}` the instance, and assert the spread contains exactly the seven documented keys (`id`, `type`, `origin_id`, `origin_slot`, `target_id`, `target_slot`, `parentId`) reflecting the post-mutation state, not a stale captured copy.
- [ ] **Detached-node `setPos`/`setSize` writes are local-only, not full no-ops** (PR #15857 — [discussion](https://github.com/Comfy-Org/ComfyUI_frontend/pull/15857#discussion_r3854894759)): Add a node to a graph, remove it, call `setPos`/`setSize` on the detached reference, and assert the local `pos`/`size` fields update while the graph's/layout store's record for that id is unaffected.
- [ ] **Node-removal callback ordering, specifically where `onConnectionsChange` falls** (PR #15857 — [discussion](https://github.com/Comfy-Org/ComfyUI_frontend/pull/15857#discussion_r3854894764)): Remove a node with a connected peer while spying on `node:before-removed`, both nodes' `onConnectionsChange`, `onRemoved`, `node.graph` nullness, `onNodeRemoved`, and `node:removed`; assert the full five-step order end-to-end (existing tests only cover it in two disjoint partial sequences).
- [ ] **`onSerialize`/`onConfigure` receiver (`this`) vs. argument identity** (PR #15857 — [discussion](https://github.com/Comfy-Org/ComfyUI_frontend/pull/15857#discussion_r3855858004)): Register `node.onSerialize`/`graph.onSerialize` hooks, trigger serialize, and assert `this` is the live instance while the callback argument is a distinct plain data object, with mutation via `this` reflected on the live node afterward.
- [ ] **Detached-reference write API shape differs per entity type** (PR #15857 — [discussion](https://github.com/Comfy-Org/ComfyUI_frontend/pull/15857#discussion_r3855858021)): Assert `LGraphNode` exposes callable `setPos`/`setSize`; `LGraphGroup` exposes only `pos`/`size` property setters (no methods); `Reroute` exposes only a `pos` setter, no size, and rejects malformed `pos` values regardless of attachment state.
- [ ] **`slotLinks.ts` helpers must not silently become public API** (PR #15857 — [discussion](https://github.com/Comfy-Org/ComfyUI_frontend/pull/15857#discussion_r3854894770)): Assert `litegraph.ts`'s public export surface does not include `inputHasLink`/`outputHasLinks`/`outputLinkIds`/`inputLink`/`outputLinks`.

## Flagged for human decision

Review-driven commits and comments that surfaced during this analysis but don't cleanly fit either bucket above (docs-only fixes, unresolved/deferred bugs with no fix commit, or ambiguous test-only commits) — included for visibility rather than silently dropped. A human should decide whether any need a follow-up issue of their own.

- [ ] **Two commits didn't cleanly fit either bucket** (PR #15882): [`c44767e`](https://github.com/Comfy-Org/ComfyUI_frontend/commit/c44767e98cd9a0786e9cb19fa59bd4003acabbc0) is a coverage-only response to a CodeRabbit nit (test-coverage gap, not a reasoned bug) — it IS the test addition, so neither bucket applies. [`7575bcd`](https://github.com/Comfy-Org/ComfyUI_frontend/commit/7575bcd1b860ce65cee9ee52d19489ff6c15b580), the final pre-merge commit, adapts remint test expectations with no linked review thread in its message, so its exact motivation couldn't be confirmed.
- [ ] **Type-assertion cleanup with no behavioral change** (PR #15731): [`1c1a26a`](https://github.com/Comfy-Org/ComfyUI_frontend/commit/1c1a26a7a547d4ad9e05548c7df773f3e7c3fb5f) removes an unneeded `WidgetState` assertion per a CodeRabbit "Major" finding; runtime behavior is identical, so no test applies beyond what `vue-tsc` already checks.
- [ ] **BLOCKER, unfixed: dynamic-combo widget value snaps back on toggle-away/toggle-back** (PR #15924 — [discussion](https://github.com/Comfy-Org/ComfyUI_frontend/pull/15924#discussion_r3858723868)): `widgetValueStore.graphWidgetRestorations` lives for a node's whole lifetime and is consulted by `addCustomWidget` for any later widget re-add, not just initial `configure()`. User-reachable via load workflow → edit a dynamic-combo widget → toggle it away and back. Review posted ~13 min after merge; no fix commit exists; deferred to #15929 per the merge note.
- [ ] **BLOCKER, unfixed: PrimitiveNode silently adopts target node's widget value instead of its own on load** (PR #15924 — [discussion](https://github.com/Comfy-Org/ComfyUI_frontend/pull/15924#discussion_r3858723875)): The `widgets_values` restore loop was deleted from `onAfterGraphConfigured`; positional restore inside `addCustomWidget` now runs before `widget.value = theirWidget.value`, flipping precedence. With `control_after_generate: randomize`, seeds silently revert on reload. Existing `primitive_node.json` E2E fixture can't catch this (both values equal). No fix commit exists.
- [ ] **SHOULD-FIX, unfixed: `nodeOutputStore.setNodeOutputImages` is O(N²) per execution** (PR #15924 — [discussion](https://github.com/Comfy-Org/ComfyUI_frontend/pull/15924#discussion_r3858723886)): Triggers a full `createMutationView` snapshot/rekey/whole-ref-replace on every single output write; scalability regression on the ordinary "run a workflow" path with N output nodes. No fix commit exists.
- [ ] **SHOULD-FIX, unfixed: stale clipspace image shown after mask-editor save** (PR #15924 — [discussion](https://github.com/Comfy-Org/ComfyUI_frontend/pull/15924#discussion_r3858723890)): Deleting the `node.images = [mainRef]` write in `useMaskEditorSaver.ts` leaves `useMaskEditorLoader.getNodeImageUrl` pointing at a stale image. Concrete path: paste an image from clipspace → mask-edit → reopen shows the stale pasted image. No fix commit exists.
- [ ] **SHOULD-FIX, unfixed: regression test deleted instead of adapted (testing-process violation)** (PR #15924 — [discussion](https://github.com/Comfy-Org/ComfyUI_frontend/pull/15924#discussion_r3858723894)): `BrushCursor.test.ts`'s "should read the container rect once per position, not once per axis" test was deleted rather than adapted when the hand-rolled rect read was replaced with `useElementBounding()`, which doesn't observe ancestor movement/transforms. Reviewer cites AGENTS.md's "never delete tests to make them pass" rule. No commit restores/replaces the test.
- [ ] **SHOULD-FIX, unfixed: missing-node replacement partial-failure suppresses per-node warnings** (PR #15924 — [discussion](https://github.com/Comfy-Org/ComfyUI_frontend/pull/15924#discussion_r3858723915)): `useNodeReplacement`'s `replacedTypes` is marked replaced per-type as soon as any one node of that type succeeds; a partial failure can leave unreplaced placeholders with their "missing node" warning already dismissed for the whole type. No fix commit exists.
- [ ] **SHOULD-FIX, unfixed: promoted widget values leak across unrelated SubgraphNode instances** (PR #15924 — [discussion](https://github.com/Comfy-Org/ComfyUI_frontend/pull/15924#discussion_r3858723921)): Shared `nodeId=-1` registration key during construction lets Ctrl+C-copy-then-discard leave a stale registration that a later unrelated same-named/typed promoted input inherits. `SubgraphDuplicateDeleteOrder.test.ts` doesn't exercise this. No fix commit exists.
- [ ] **`normalizeSubgraphDefinitionIds` nested-definition normalization still unaddressed** (PR #15924 — [commit](https://github.com/Comfy-Org/ComfyUI_frontend/commit/774a7ab01a102b7b37fa02127aac2de961fbad05)): The one commit touching the flagged function only patches a test fixture id to stay UUID-shaped; it does not add the recursive nested-subgraph-definition normalization the reviewer actually asked for.
- [ ] **Docs-only JSDoc fix, no behavior change** (PR #15689): [`9edd1a5`](https://github.com/Comfy-Org/ComfyUI_frontend/commit/9edd1a5752ba7973e79ffc27ae5c85c3438d4f7e) describes all three link-dedup survivor rules in a comment; no test applies.
- [ ] **Deferred, unfixed twin bug: `seedNestedPromotedInputState` has the same registration-order defect this PR fixes elsewhere** (PR #15767 — [review](https://github.com/Comfy-Org/ComfyUI_frontend/pull/15767#pullrequestreview-5024107702)): `src/core/graph/subgraph/promotionUtils.ts:326` assigns `hostInput.widgetId = id` before calling `registerWidget(...)` and ignores the return value — the same defect class this PR fixes, on the path that backs the user-facing promote action. Author suggested filing a follow-up issue instead of fixing here; left unfixed at merge.
- [ ] **Ambiguous test-only commit, motivation unconfirmed** (PR #15688): [`07460c3f`](https://github.com/Comfy-Org/ComfyUI_frontend/commit/07460c3f746b57f507a1bba12b181dc7a65515ea) removes casts from three test files shortly after a related review comment, but touches no production type/restore code and adds no new test; unclear whether it closes the underlying type gap or was only made safe by #15924 landing separately.
- [ ] **Docs-only commit dropped by branch rewrite, concern re-raised and still open** (PR #15779): [`af6410d3`](https://github.com/Comfy-Org/ComfyUI_frontend/commit/af6410d392ac8e2d34cf766e4d8201069d5e49d4) documented assigned-input-slot identity semantics but isn't on the current PR head; see the corresponding Unit test todo above for the still-open version of this concern.
- [ ] **Perf cleanup dropped by branch rewrite, concern neutralized by architecture change** (PR #15779): [`5e5a3b81`](https://github.com/Comfy-Org/ComfyUI_frontend/commit/5e5a3b811eaba022efee0ffc89af5e8b46a1dc56) isn't on the current head; the redundant-writeback cost it removed is now a no-op in the common case under the new write-time Proxy design, so likely no action needed — flagged for confirmation only.
- [ ] **Pure refactor dropped by branch rewrite, moot under current design** (PR #15779): [`43d8784a`](https://github.com/Comfy-Org/ComfyUI_frontend/commit/43d8784a41dfe992e2355375f7c00f19e0c71b77) deduplicated slot-array factory code that no longer exists under the current `slotDescriptorView.ts` design.
- [ ] **Consolidation commit doesn't cleanly fit either bucket** (PR #15729): [`ebe6ddb`](https://github.com/Comfy-Org/ComfyUI_frontend/commit/ebe6ddbab443e05388e7dadb6f489dc36daa3a9f) merges test suites and resolves both review threads by being the coverage itself — a pure test-only PR, so there's no app-code bug or residual gap, logged only so it isn't silently dropped.
- [ ] **Type-guard refactor, behavior-preserving** (PR #15393): [`8ffbdf0`](https://github.com/Comfy-Org/ComfyUI_frontend/commit/8ffbdf0f461bed4a8b0b8fd7f9d0ac6b3c1ec153) replaces an inline type assertion with a proper type guard per a CodeRabbit maintainability nit; no observable behavior changed.
- [ ] **ADR doc-accuracy fix: which code path performs merge-boundary reconciliation** (PR #15920): [`496f96b`](https://github.com/Comfy-Org/ComfyUI_frontend/commit/496f96bae16b9507740ccd06ffd78ea106e7c4d4) corrects ADR-0018 wording; no runtime code changed.
- [ ] **ADR doc-accuracy fix: rejection-before-remint ordering description** (PR #15920): [`e224106`](https://github.com/Comfy-Org/ComfyUI_frontend/commit/e22410654444e0981f9a734b97d6fbd5ccdeac73) corrects ADR-0018 wording; no runtime code changed.
- [ ] **ADR consistency fix describing a not-yet-implemented CRDT applier** (PR #15920): [`f9b8be5`](https://github.com/Comfy-Org/ComfyUI_frontend/commit/f9b8be5fa04c77c852e7c4bf6a9c3afc906ef65d) fixed a real logical inconsistency in the ADR's diagram/model, but the whole section describes future, not-yet-shipped CRDT semantics — nothing exists yet to test.
- [ ] **ADR readability pass, unexplained internal decision-log ids** (PR #15920): [`530dc18`](https://github.com/Comfy-Org/ComfyUI_frontend/commit/530dc180d8a81d722501b34b7fff65c95119800b) replaced internal `D-gl-A*` references with plain-language descriptions per human reviewer DrJKL; docs-only.
- [ ] **Markdown lint fix, docs-only** (PR #15810): [`a0541c2`](https://github.com/Comfy-Org/ComfyUI_frontend/commit/a0541c28fae103564966dae09fdde0b3ff765e61) fixes an MD018 heading-punctuation issue in an ADR; two further DrJKL rewording threads on this PR's docs remain unresolved/unaddressed as of merge.
- [ ] **Docs-only accuracy corrections to migration guides (round 1)** (PR #15857): [`96882c7`](https://github.com/Comfy-Org/ComfyUI_frontend/commit/96882c759ad9ff3ac15b59df363d3249cfcd28a8) corrects 3 factual inaccuracies (detached writes, internal API, a test-coverage claim) in `docs/extensions/connection-callbacks-migration.md` and `docs/extensions/node-geometry-migration.md` per reviewer christian-byrne; the underlying invariants are captured as Unit test todos above.
- [ ] **Docs-only accuracy corrections to migration guides (round 2)** (PR #15857): [`eedb879`](https://github.com/Comfy-Org/ComfyUI_frontend/commit/eedb87930510424a17a630d39dc4642986a2514c) corrects callback receiver, widget example, and geometry API scope per review; one of the three threads (a non-type-checking TS example) is a pure doc-example fix with no runtime invariant.

---
_Generated by [Claude Code](https://claude.ai/code)_