Skip to content

fix(cli): reach screen nodes inside ADR-0031 regions in os i18n extract - #17644

Merged
os-sales merged 2 commits into
mainfrom
claude/issue-17511-i18n-extract-region-screens
Sep 11, 2026
Merged

fix(cli): reach screen nodes inside ADR-0031 regions in os i18n extract#17644
os-sales merged 2 commits into
mainfrom
claude/issue-17511-i18n-extract-region-screens

Conversation

@os-sales

@os-sales os-sales commented Sep 11, 2026

Copy link
Copy Markdown
Collaborator

Fixes #17511

Clause-②: no

Declared by the dispatching domain:cli PM seat (session session_01TSf4DV7ziu4V5j73e46b7c), matching the claim comment on #17511. Verified against the DIFF, not the card: the extractor now reaches screen nodes it was always meant to reach, emitting keys under the already-declared flows.NAME.screens.NODE_ID.* shape — no new key shape, no new error code (0 added upper-case code literals), and no change to what INPUT is accepted (a nested screen was always legal to author; only the walker failed to see it). check-i18n-coverage confirms the ratchet does not move.

walkScreenFlows (packages/cli/src/utils/i18n-extract.ts) iterated flow.nodes flat, so a type: 'screen' node nested inside an ADR-0031 region — loop.config.body, parallel.config.branches[].nodes, try_catch.config.try / .catch, nesting arbitrarily — was never reached. It emitted no flows.NAME.screens.NODE_ID.title / .fields.* skeleton entry and no coverage row.

That pairing is the defect, not the missing translation: a translator was never shown the key AND the coverage gate had no row to demand, so the hole was invisible to the mechanism built to report holes. A silent zero.

Premise check — both the card and its triage comment are stale, so neither is quoted

Everything else in the card re-measured true at 7d350a46, with fresh lit controls (see below).

The dispatch assumption that turned out FALSE, and what it changed

The dispatch flagged one assumption as unmeasured: "mapFlowNodeList is importable from packages/cli without a new dependency edge." It is not, and this is the one finding worth reading before the diff:

  • packages/spec/src/conversions/index.ts re-exports types / registry / apply / stored only — not walk.js.
  • walk.ts's own docblock states the boundary deliberately: "Deliberately NOT re-exported from conversions/index.jswalk.ts reaches no exports subpath of the package (packages/spec/api-surface/*.json lists none of its symbols), and that is the boundary that keeps this a repair rather than a widening of the package's public surface."
  • Confirmed mechanically: mapFlowNodeList appears in no file under packages/spec/api-surface/.
  • packages/lint's walkFlowNodes (src/flow-walk.ts) is also not exported from that package's entry (src/index.ts / src/runtime.ts).

So neither landed descent is reachable across a package boundary, and making one reachable means editing packages/spec or packages/lint — outside this card's file surface, and packages/spec is the spec seat's unconditionally. Deep-importing past the boundary was ruled out by the dispatch and is not done here.

The route taken, and why it is not a second region table

The card's own "Shape of the fix" authorises exactly this: "either through the spec-side slot table or by whichever walker packages/cli already depends on." The slot table is reachable — FLOW_REGION_SLOTS_BY_TYPE is exported from @objectstack/spec/automation and recorded at packages/spec/api-surface/automation.json:116 — so the node universe now comes from a local descent that reads that one table.

This is the architecture automation/region-slots.ts declares, not a deviation from it:

The walks are deliberately NOT merged: they take different inputs … yield different units (a graph, a node, a rewritten tree) … Only the fact they all need — this table — is shared.

packages/lint/src/flow-walk.ts:51 reaches the table the same way, from outside packages/spec. walk.ts:175's "one implementation" is scoped to the per-flow descent inside packages/spec, which is why walk.ts refuses to publish itself. No copy of the slot list is introduced — the thing this card is an instance of one package over.

The lookup goes through the Map, never an object literal: node.type is author-controlled and an open namespace (ADR-0018), so an object lookup would resolve 'constructor' through the prototype chain. A MAX_REGION_DEPTH of 32 mirrors both spec-side walks, because a defineStack tree is hand-built objects and a self-containing region is reachable.

Depth does not enter the key — the key-path question the dispatch asked

Entries stay flows.NAME.screens.NODE_ID.* at every depth, because that is what the resolver reads: lookupFlowScreenCopy(bundle, flowName, nodeId) is keyed by node id alone, and translateFlow's docblock says the bundle schema "is keyed by node id and knows nothing about depth". A region segment in the key would offer a key nothing resolves — the exact producer/consumer drift the imported key face exists to prevent.

A node id repeated at two depths therefore addresses one bundle slot, and the existing dedupeByPath collapses it to a single entry, first emission wins; the walk is outer-before-inner so which one wins is deterministic. That is not a loss: one slot can serve only one string, and the resolver overlays that string onto both nodes. Pinned.

Seeding is identical at every depth, not narrower — title falls back to the node label (what ScreenSpec.title draws) and a field label falls back to its name as a derived seed, so the skeleton stays usable while the gate demands no translation of a string nobody authored. Both pinned, because getting the second wrong is how a region-aware walk starts failing the gate on machine identifiers.

Acceptance — the named criterion, with the lit control in the same fixture

nestedOnboarding carries a screen in every slot the table declares, at four depths, and a top-level screen as the control:

node depth slot
welcome 0 — (the lit control)
pick_region 1 loop.config.body
accept_terms 2 parallel.config.branches[0]
card_details 3 try_catch.config.try
payment_failed 3 try_catch.config.catch

Pins assert the exact key face (eight of the ten keys were absent before), that the coverage gate gains a row per nested screen attributed to the flow bucket, that the skeleton still parses under strictObject, and the negative half — an http node's config.body is not walked, since http owns no slot, which is what consulting the table buys over walking every body.

Reverse verification

Reverting the one line to the flat walk, with the mutation proven on disk (anchor counts 1→0 and 0→1, blob hash differing from HEAD) and restored by git checkout HEAD -- ... verified against the HEAD blob hash and an empty git diff HEAD:

Test Files  1 failed (1)
     Tests  4 failed | 17 passed (21)
   × harvests every nested screen, at every region slot and depth
   × gives the coverage gate a row for the nested step, not just the top-level one
   × seeds a nested screen exactly as a top-level one, never more narrowly
   × scaffolds the nested drill into a bundle the strict schema accepts

Predicted direction was "goes red" and that is what it did. The 17 that stayed green include the pre-existing #11485 suite and the pins measuring collision handling, cycle termination and table consultation — properties the descent does not decide. The subject resolves through a relative import into src/, so no dist rebuild is involved in the ablation.

Verification

  • Closure build turbo build --filter '@objectstack/cli^...'Tasks: 56 successful, 56 total, exit 0. Full pnpm build afterwards: 73 successful, 73 cached.

  • Typecheck pnpm --filter @objectstack/cli typechecktsc --noEmit plus check:test-typecheck: OK, exit 0.

  • Unit tier vitest run --project unitTest Files 195 passed (195), Tests 2708 passed (2708). The integration tier is declared to CI: the diff touches no integration-layer file and no spawn entry point.

  • Gate floor re-derived on the final 3-path diff with scripts/pm/dispatch-gates.mjs, every command redirected with $? captured before any pipe: 62 derived, 62 run, 0 NOT-MEASURED, 0 UNRUN — a derived zero, all 62 records carrying an exit code and none of them 3.

    • Four gates first returned exit 3 — PREREQUISITE NOT MET, "NOTHING was measured" (check:i18n, check:i18n-coverage, check:i18n-walk-parity, check:dual-build-cjs-loads): they read built output that did not exist yet. Recorded as unmeasured, the prerequisite closures each gate names were built, and all four re-run green:
      • check-i18n-bundles: OK (9 package(s) — all bundles in sync, no undeclared authoring keys).
      • check-i18n-coverage: OK (13 config(s), 621 baselined untranslated string(s), none new).
      • check-i18n-walk-parity: 11 declared group(s), 8 walked, 3 exempted — every declared group has an extractor face.
      • check:dual-build-cjs-loads — 104 published require entry point(s) across 67 package(s) load (floors held).
    • check-undeclared-dep-imports — the gate the dispatch flagged as most likely to bite the import — exit 0. @objectstack/cli already declares @objectstack/spec, and @objectstack/spec/automation is a published subpath, so no new dependency edge.
  • pnpm lint (the standing dispatch-gates blind spot) delivered as a declared narrowing, with all three readings:

    1. population read from ESLint's own config resolution (isPathIgnored over every tracked lintable file): 6612 in scope, 0 ignored — not a guess;
    2. files linted read from --format json output: 2, 0 errors / 0 warnings, exit 0;
    3. invariance: eslint.config.mjs states in its own header that this repo "never enables type-aware linting (no parserOptions.project, no typed @typescript-eslint rules) for ANY file, test or not" — measured there with a positive control, and confirmed independently (every parserOptions occurrence carries only ecmaVersion / sourceType). Each file is linted in isolation, so this diff cannot move the verdict on any of the 6610 untouched files.

    The whole-repo run stays CI's, as the repo-scan rule assigns it.

A patch changeset is included: @objectstack/cli publishes dist, and the extractor's emitted output changes.

Acceptance notes (out of scope, filed nowhere)

  • packages/lint's walkFlowNodes is not exported from @objectstack/lint. i18n-extract.ts already imports walkPageComponents from that package, so the CLI consuming a lint walker is established practice, and publishing this one would let two consumers share a descent rather than a table. Noted, not filed: it is a surface-widening proposal for the lint seat, not a defect — the runtime rejects nothing and no declared contract is broken. Successor: whoever next needs a region-aware node walk outside packages/spec.
  • The dispatch's file surface names packages/cli/src/ for the tests, but every sibling i18n-extract pin lives in packages/cli/test/ (eight files). The new pins were added to packages/cli/test/i18n-flow-screen-coverage.test.ts — this card's own surface — rather than stranded in src/ away from their siblings; that file also already carries the vi.mock of the liveness ledger that makes a flows coverage row observable at all, which the acceptance criterion requires. Declared as a deviation in the report.

Generated by Claude Code


Generated by Claude Code

`walkScreenFlows` iterated `flow.nodes` flat, so a `type: 'screen'` node
nested in a region (`loop.config.body`, `parallel.config.branches[].nodes`,
`try_catch.config.try`/`.catch`) was never reached: no
`flows.NAME.screens.NODE_ID.*` skeleton entry and no coverage row. A silent
zero — the gap was invisible to the mechanism built to report gaps.

Route the node universe through a region-aware descent that reads the one
shared table, `FLOW_REGION_SLOTS_BY_TYPE` from `@objectstack/spec/automation`,
exactly as `packages/lint`'s `walkFlowNodes` does. Depth stays out of the key:
`lookupFlowScreenCopy` is keyed by node id alone, so a repeated id collapses
onto its single bundle slot via the existing `dedupeByPath`.

Claude-Session: https://claude.ai/code/session_01TSf4DV7ziu4V5j73e46b7c
Co-authored-by: Claude <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TSf4DV7ziu4V5j73e46b7c
Co-authored-by: Claude <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

3 anchor(s) derived from 1 changed package(s); no hand-written page names any of them, so this run has nothing to listnot a clean bill of health. This check sees only pages that NAME a derived anchor: one that documents this change in prose, or enumerates it in an authoring dialect, names none and stays invisible to it on every run.

What this run could not see
  • the SDK route bridge reached 60 of 215 client-bound route-ledger rows — the other 155 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 155: 0 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 55 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 100 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 — 23 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 155b875d0f563c18d7c085545c2a9d68250215a7packageMentionDocs.

Which tree this was computed on

This run read content/docs from 52fda8428805f9184be451ea17e6e01c01ba663d — the merge of head faf7b1b834cc203dc8913a9dc69388f0883e3074 into base 155b875d0f563c18d7c085545c2a9d68250215a7, 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 52fda8428805f9184be451ea17e6e01c01ba663d && git checkout 52fda8428805f9184be451ea17e6e01c01ba663d
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 155b875d0f563c18d7c085545c2a9d68250215a7 faf7b1b834cc203dc8913a9dc69388f0883e3074 && git checkout -B drift-repro 155b875d0f563c18d7c085545c2a9d68250215a7 && git merge --no-ff faf7b1b834cc203dc8913a9dc69388f0883e3074

node scripts/docs-audit/affected-docs.mjs --json 155b875d0f563c18d7c085545c2a9d68250215a7

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

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

Development

Successfully merging this pull request may close these issues.

i18n-extract: walkScreenFlows walks flow.nodes flat — a screen inside an ADR-0031 region gets no skeleton entry and no coverage row

2 participants