Skip to content

fix(service-automation): runRegion carries the outer region's index through nesting; the parallel branch index moves to branch (#15230) - #16367

Merged
os-warren merged 4 commits into
mainfrom
claude/issue-15230-region-tagger-branch-key
Sep 6, 2026
Merged

fix(service-automation): runRegion carries the outer region's index through nesting; the parallel branch index moves to branch (#15230)#16367
os-warren merged 4 commits into
mainfrom
claude/issue-15230-region-tagger-branch-key

Conversation

@os-warren

Copy link
Copy Markdown
Collaborator

Fixes #15230

The engine half of the maintainer ruling of 2026-09-03 (recorded on #14414). The contract half — ExecutionStepLogSchema.branch, and iteration re-described as single-valued — landed with PR #15227 and is untouched here.

The defect

Two facts combined to make loop { body: [ parallel { branches } ] } unreadable.

parallel wrote its branch index into iteration, the same key loop writes its row index into — one field, two meanings, told apart only by reading regionKind first. And runRegion's tagger let the innermost region win outright: a step that already carried a parentNodeId was skipped entirely, so an enclosing region contributed nothing to it.

Together: every branch step of a loop { parallel } recorded its branch index, and no step of that branch recorded the row it ran for. A per-row failure inside a branch was attributable to a branch and never to a row — which is precisely the shape a fan-out inside a scheduled sweep has, and the one an operator most needs to read.

Measured on the unmodified tree, three rows x two branches:

leafA@iteration=0/branch=undefined
leafA@iteration=0/branch=undefined
leafA@iteration=0/branch=undefined
leafB@iteration=1/branch=undefined
leafB@iteration=1/branch=undefined
leafB@iteration=1/branch=undefined

All three of leafA's rows report iteration=0 and all three of leafB's report iteration=1: that is the BRANCH index, constant per branch. The row index — 0, 1, 2 — appears nowhere, and branch is never written at all.

The change

runRegion's tagger splits what "innermost wins" governs. The two halves answer different questions:

  • IDENTITYparentNodeId, regionKind, retryAttempt — answers which region ran this step. Innermost wins outright, exactly as before: a step a nested region already claimed keeps naming that region, and an enclosing region never relabels it.
  • INDEXiteration, branch — answers which pass of which region. Nested regions contribute different indices that are both true of the same step, so an enclosing region now fills the index the inner region left undefined instead of being turned away with the identity fields.

It is still "fill only what is undefined" on both halves, so an index a nested region did set wins: loop { loop } keeps the inner loop's iteration, unchanged.

parallel writes branch: i, not iteration: i (builtin/parallel-node.ts). This file is beyond the surface the card named, and unavoidably so: the tagger writes what grouping hands it, and the branch index is produced at this call site. The alternative — having the tagger key on regionKind === 'parallel-branch' and re-home the value — puts implicit magic in the shared tagger to keep one file out of the diff.

StepLogEntry gains branch?: number and the overloaded comment is rewritten to one meaning per key. The interface is exported, and the change is exactly one optional output key, so the dispatch's Clause-②: no stop condition ("more than one optional output key") is not met.

try / catch inside a loop is unchanged, deliberately — the control arm, not a subject. Such a region has no index of its own, so its steps keep carrying the enclosing loop's iteration with regionKind still naming the region, and gain no branch. builtin/try-catch-node.ts is in the diff for its comment only: the comment asserted "a loop's own tagger can never reach past this one to a try/catch step", which this change makes false. The forwarding it explains stays (same value, and it is what $error.iteration is bound from); only the claim about the tagger is corrected.

Pins

packages/services/service-automation/src/builtin/region-index-keys.test.ts — the ruling's own three cases, plus two guards, plus a type-level pin.

Red-first, with the predictions written before the run (they held exactly):

pin unmodified tree after
loop { parallel } — both indices on one step RED — branch=undefined, iteration = the branch index green
bare parallelbranch written, iteration absent RED — expected undefined to be +0 green
loop { try_catch } — unchanged (control arm) green green
every step parses under ExecutionStepLogSchema green green
branch refuses -1 / 1.5 at the branch path green green
type pin: engine region keys == spec region keys RED under tsc green

The last row is the card's "kept in step with the spec type by a pin, not by prose". It is a type-level assertion, so vitest cannot see it — esbuild strips types and a green pnpm test says nothing about it. It is measured by tsc, where on the unmodified tree it read:

src/builtin/region-index-keys.test.ts(70,27): error TS2344: Type 'RegionKeys' does not
  satisfy the constraint 'keyof StepLogEntry'.
  Type '"branch"' is not assignable to type 'keyof StepLogEntry'.

Both tsconfig.json and tsconfig.test.json reach src/** with no test exclusion, so the pin is genuinely compiled — the errors above came out of that file, which is the positive proof.

Two fixtures replaced, not re-spelled

Both pinned exactly the branch the ruling deleted, so re-spelling the assertion would have left the retired overload free to come back as a second writer.

Swept the rule's consumption radius rather than the edited package: regionKind has no other reader in this repo outside packages/spec (whose own tests already carry the new shape, landed with PR #15227) and the docs reference, which is generated from the spec describe and already regenerated there. The objectui FlowRunsPanel grouping key is out of scope by the card and gated on a spec release.

Also in the diff

.changeset/contained-failure-visibility.md (the pending #14456 entry) ended with "parallel branch tagging is unchanged." That was true of #14456 and is falsified by this change; both entries are pending, so shipping them unedited puts a flat contradiction in one release note. Trimmed to say it was unchanged by that change and to point at this one. Nothing else in that file is touched.

Changeset

.changeset/region-tagger-branch-key.md, @objectstack/service-automation: minornot skip-changeset shape. The engine publishes different structured data into the run step log: branch appears, and iteration changes meaning on a parallel-branch step. That is observable to any flow author or run-observability consumer, and the exported StepLogEntry gains a key. minor rather than major to match the contract half of the same ruling, which shipped its declaration change that way; the entry states how to read a run recorded before the change, since nothing is migrated or defaulted.

Checklist

docs/qa/platform-checklist/areas/automation.json, automation.flow-run-step-nesting -> revision 3: the loop { parallel } clause the card asks for, with the three-part verify (branch: 0 survives; every (iteration, branch) pair appears once — a constant iteration per branch is what the old engine produced, so counting distinct values alone does not catch it; the container step carries the row and no branch).

Its fixture does not exist. showcase declares loop, parallel and try_catch as three separate flows and nests none in another, so the clause scores blocked(fixture) until one lands. Filed as #16356 and recorded in the item's fixtures.requires rather than left silent — an absent clause is what let this overload sit unmeasured, so the clause is written now and the gap is named in the open. #16356 is not addressed here.

What was run, and on which tree

The gate family was derived mechanically — node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack --commands, from the tool's own merge-base change set, plus the four rosters it flags as kept in a directory one of these paths is in. Every exit code captured after a single redirection, never through a pipe.

  • e64991ffa — the full union, 62/62 exit 0. On the first pass two came back exit 3 PREREQUISITE NOT MET (check:dual-build-cjs-loads, check:type-check-debt, both naming an unbuilt dist/). Those are neither a pass nor a finding: the prerequisite was satisfied (turbo run build --filter='./packages/*' --filter='./packages/*/*') and both re-run as real readings. pnpm lint (eslint . --no-inline-config, whole repo, not narrowed): exit 0. pnpm --filter @objectstack/service-automation test: 121 files / 1426 tests passed. typecheck (tsc --noEmit + check:test-typecheck): exit 0.
  • 93fa7157b — head. Its only delta from the above is one prose string inside automation.json (an unsupported "through two releases" claim, dropped). Re-run there: every gate that reads that file (check:platform-checklist, check-platform-checklist-watchdog, check:nul-bytes, check:doc-authoring, check:pm-governed-prose, check-affected-docs, check-drift-comment, check:keyed-text-bounds, check:pm-label-desc-cap) plus the ratchet family (check:type-check-debt, check:type-check-coverage, check:dual-build-cjs-loads, check:skills-token-ratchet, check:pm-skill-ratchet) — all exit 0.

🤖 Generated with Claude Code

https://claude.ai/code/session_01XpTx2tbq3pZRYAdoGt6E6Y


Generated by Claude Code

… change (#15230)

Red-first. On this unmodified tree the three ruling pins read:

  loop { parallel }  RED  — branch=undefined on every branch step, and
                            `iteration` carries the BRANCH index (0/1) instead
                            of the row (0..2): the loop's index is discarded.
  bare parallel      RED  — expected undefined to be +0 at `branch`.
  loop { try_catch } GREEN — the control arm; try/catch already forwards the
                            enclosing loop's iteration at its own call site.
  spec refusal       GREEN — `branch` already refuses -1 / 1.5 at the `branch`
                            path (the contract half landed with PR #15227).

The type-level pin is invisible to vitest (esbuild strips types) and is
measured by tsc: TS2344 `Type '"branch"' is not assignable to type
'keyof StepLogEntry'`.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XpTx2tbq3pZRYAdoGt6E6Y
…hrough nesting; the parallel branch index moves to `branch` (#15230)

The engine half of the maintainer ruling of 2026-09-03; the contract half
(`ExecutionStepLogSchema.branch`) already landed in `@objectstack/spec`.

`runRegion`'s tagger let the innermost region win outright, skipping any step
a nested region had already tagged, and `parallel` wrote its branch index into
`iteration`. Together those made `loop { parallel }` unreadable: every branch
step recorded its branch and no step of that branch recorded its row.

The tagger now splits what "innermost wins" governs. IDENTITY fields
(`parentNodeId` / `regionKind` / `retryAttempt`) answer WHICH REGION ran the
step and still belong to the innermost region outright. INDEX fields
(`iteration` / `branch`) answer WHICH PASS of which region — nested regions
contribute different ones, both true of the same step — so an enclosing region
fills the index the inner one left undefined instead of being skipped along
with the identity fields. Still "fill only what is undefined", so `loop { loop }`
keeps the inner loop's `iteration`.

`try` / `catch` inside a loop is unchanged (the control arm): such a region has
no index of its own, so its steps keep the loop's `iteration` and gain no
`branch`.

Two fixtures pinned the retired branch and are replaced, not re-spelled:
parallel-node.test.ts asserted the branch index ON `iteration` for a bare
parallel, and contained-failure-visibility.test.ts carried the "#14414 fence"
asserting that #14456 had left the overload standing. #15230 is the card that
retires it, so the fence is spent and now reads the two indices apart.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XpTx2tbq3pZRYAdoGt6E6Y
…15230)

The clause's `fixtures.requires` said "filed separately"; the card is #16356.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XpTx2tbq3pZRYAdoGt6E6Y
…allel } fixture note (#15230)

"sit unmeasured through two releases" asserted a span I did not measure. The
point the note makes — that a missing clause is what let the overload go
unmeasured — stands without it.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XpTx2tbq3pZRYAdoGt6E6Y
@github-actions github-actions Bot added size/m documentation Improvements or additions to documentation tests tooling labels Sep 6, 2026
@github-actions

github-actions Bot commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/service-automation, touching 5 documentable anchor(s).

2 release-owned page(s) name something this change touched. These are read-only:

  • content/docs/releases/v16.mdx (via AutomationEngine (symbol, a top-level class))
  • content/docs/releases/v17.mdx (via AutomationEngine (symbol, a top-level class))

content/docs/releases/ is RELEASE-OWNED (AGENTS.md "Documentation Guardrails"): release
notes are written centrally at release time, and a code PR that edits them is the exact PR
that guardrail exists to stop. They are still audited — read-only. If one of them is actually
wrong, file an issue or open a dedicated docs-only PR; do not edit it here.

What this run could not see
  • 1 name(s) were too generic to anchor anything (single lowercase words)
  • the SDK route bridge reached 61 of 219 client-bound route-ledger rows — the other 158 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 158: 0 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 56 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 102 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.

Coarse fallback — 5 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json 055848ee708d13bb0839cac39b340103239ac781packageMentionDocs.

Which tree this was computed on

This run read content/docs from 5b522257023f165cbb210a252e2ad958d04e4872 — the merge of head 93fa7157bed23e5639b18f5c0d70d83919a26997 into base 055848ee708d13bb0839cac39b340103239ac781, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 5b522257023f165cbb210a252e2ad958d04e4872 && git checkout 5b522257023f165cbb210a252e2ad958d04e4872
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 055848ee708d13bb0839cac39b340103239ac781 93fa7157bed23e5639b18f5c0d70d83919a26997 && git checkout -B drift-repro 055848ee708d13bb0839cac39b340103239ac781 && git merge --no-ff 93fa7157bed23e5639b18f5c0d70d83919a26997

node scripts/docs-audit/affected-docs.mjs --json 055848ee708d13bb0839cac39b340103239ac781

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs 055848ee708d13bb0839cac39b340103239ac781 → pass the list as
args.docs, on the commit named under Which tree this was computed on.

@os-warren
os-warren marked this pull request as ready for review September 6, 2026 16:04
@os-warren
os-warren enabled auto-merge September 6, 2026 16:04
@os-warren
os-warren added this pull request to the merge queue Sep 6, 2026
Merged via the queue into main with commit 65ec530 Sep 6, 2026
35 checks passed
@os-warren
os-warren deleted the claude/issue-15230-region-tagger-branch-key branch September 6, 2026 16:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/m tests tooling

Projects

None yet

2 participants