Comfy-Org / Comfy-Org/ComfyUI_frontend

ECS branch: four entity registries have four collision contracts; pick one or write down why not

Open
#15,702 0 comments 0 reactions 0 assignees View on GitHub
Public API
Dominant language
TypeScript
Stars
2k
Forks
699
Avg merge
1d 7h
Merged PRs (30d)
490

Description

`registerNode`, `registerWidget`, `replaceLink` and the reroute registry each answer a colliding registration differently, and a caller cannot tell from the return value which of those it is talking to.

Repo `Comfy-Org/ComfyUI_frontend`, branch `feature/ecs-migration` (PR #14246), measured at `907ca2b147`.

| Store | On collision | Returns | Logs |
| -- | -- | -- | -- |
| `widgetValueStore.ts:103-106` | incumbent if `existing.type === init.type`, otherwise **overwrite** | the record | no |
| `linkStore.ts:198-234` | reject | `undefined` | yes, `:210`, `:223` |
| `rerouteStore.ts` | reject | `undefined` | yes, `:131` |
| `nodeDataStore.ts:56` | reject | `undefined` | no |

## The questions, in the order they need answering

1. **Do call sites read the return value at all?** This is the cheap half and it decides the rest. `LGraph.ts:1171` reads it as a boolean retry signal; enumerate the others before designing anything.
2. **Is a collision an invariant violation or a recoverable condition?** If it is a violation, `undefined` is the wrong shape and it should assert. If it is recoverable, `undefined` is still the wrong shape because it collapses "already registered, same object" and "conflict, different object" into one value.
3. **If it stays recoverable, should the contract be explicit?** `registered | alreadyRegistered | conflict` was the proposal. That would let the `LGraph.add` retry loop distinguish a re-mint from a real identity bug.
4. **Should returning the canonical incumbent be the rule**, so all consumers converge on one object, or does that hide bugs?
5. **If the answer differs per store, where is that written** so an extension author finds it? The divergence may be principled, but the boundary is currently drawn only in the code.

## Scope note

The extension-facing half of question 5 overlaps FE-1781 (seven documents, including ADR 0008, state a slot-mirror contract the code does not implement) and FE-216 (legacy extension API deprecation). Fold the answer into one of those rather than opening a third document.

Blocks a decision needed for the CRDT work: whether duplicate-id reconciliation can live at the merge boundary while these in-memory registries keep a strict no-collision invariant.

## Provenance

Found while answering Christian Byrne's questions in Slack `#p-frontend-graph-improvements`
on 2026-08-22 (threads `p1787385648421529`, `p1787386156545179`, `p1787388764721129`).
Every code claim above was re-derived against `origin/feature/ecs-migration` at `907ca2b1479381eff7a617e656c9002806b5fa15`
and `origin/main` at `a08a7598aa` on 2026-08-23T06:30Z with `git show :`, not
read from a working tree.

---
_Mirrored from Linear [FE-1811](https://linear.app/comfyorg/issue/FE-1811/ecs-branch-four-entity-registries-have-four-collision-contracts-pick)._

Contributor guide

Open the contributing guide

Research direction

Start by enumerating return-value consumers for registerNode, registerWidget, replaceLink, and the reroute registry, with LGraph.ts:1171 as the named entry point. Compare the collision behavior in widgetValueStore.ts, linkStore.ts, rerouteStore.ts, and nodeDataStore.ts; done means choosing and implementing one explicit contract, or recording the principled differences in FE-1781, FE-216, or the existing ADRs.

Written by the indexing model from the issue text.

Assessment

Tech stack
typescript
Domain
frontend
Issue type
Refactor
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
28/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.