Skip to content

Decision: do a flow's top-level nodes[] and its region bodies (loop / try_catch / parallel) share ONE node-id space, or two? — uniqueness is now enforced inside each, never across #16134

Description

@huangyiirene

Filed by the os-dev seat working #15713 (session session_01T6HeZvT9wdSJD1ZxJb5Eno, branch claude/issue-15713-flow-top-level-node-id-uniqueness), as the out-of-scope fork that card's dispatch ruled OUT of its PR (Zone 1, ruling 2: "the shared-namespace question is OUT of scope for this PR — implement the narrow rule only"). Decision-class, unassigned, no domain:*; routing and grading belong to triage. Not a defect report: nothing here is broken today, and the two answers give two different accept sets.

The fork

After #14964 (edge ids) and #15713 (top-level node ids) land, FlowSchema refuses a duplicate id within each of these arrays:

Nothing judges the id space across the two: a loop body node may carry the same id as a top-level node (or as a node in a sibling region), and both rules stay green. Whether that is correct is the open question. analyzeRegion today judges each region alone by design (ADR-0031: a region is a self-contained single-entry / single-exit sub-graph executing in the enclosing variable scope), and the #15713 card noted that an edge from a top-level node into a region node "is already a documented shape" — if such an edge is resolved by node id, a cross-namespace collision makes it ambiguous exactly the way a top-level duplicate did.

Why it is a decision, not an implementation detail

  • One id space narrows a published accept set a second time (clause ②): every region node id must differ from every top-level and every sibling-region id. It also changes what a region IS — no longer self-contained. Blast radius measured below.
  • Two id spaces keeps regions self-contained and leaves the cross-namespace collision legal — then every reader that flattens a flow (designer canvas, BPMN export, flow diff, completedNodeIds on a checkpoint, collectFlowGraphs) must key on a scoped path rather than the bare id, which is a contract those readers do not spell today.

Either answer should be recorded once, in the PR that takes it, and pinned; #15713's PR pins today's boundary (a region node reusing a top-level id parses) so that the decision, when taken, moves it deliberately rather than by drift.

Measurements (tree origin/main 1f2a02ba, AST scan over packages/** and examples/**, TypeScript/JavaScript literals only)

  • Outermost literal nodes: [ … ] arrays: 915 including tests (66 excluding tests); of these, 53 (6) hold at least one nested region nodes: [ … ]; region arrays 82 (13); region nodes with a literal id 66 (9).
  • Region node ids that collide with a top-level id of the SAME outer array: 0 (0 excluding tests).
  • Lit control on the same instrument: a planted fixture with a loop.config.body.nodes[0].id equal to a top-level id, scanned alongside the repo, reads 1, at the planted line.
  • Not covered by the scan (declared, not measured): flows assembled as call arguments (flowWith([ … ])) or generated in code, YAML/JSON flow files (none found under these roots for nodes arrays), and the pinned objectui / hotcrm trees.

So the "one id space" answer would refuse no in-repo flow today; the reading says nothing about downstream apps.

Where each half lives today

Refs: #15713 (top-level node ids, ruled narrow), #14964 (edge ids, ruled A), ADR-0031 (regions), ADR-0018 (open node-type vocabulary — an invalid node type is NOT a control on this schema).

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions