From bd5e62c53c573040c27c30769612e89d566606bf Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 11 Sep 2026 06:42:46 +0000 Subject: [PATCH 1/2] fix(cli): reach screen nodes inside ADR-0031 regions in i18n extract MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `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 Co-Authored-By: Claude Opus 5 --- packages/cli/src/utils/i18n-extract.ts | 104 ++++++- .../test/i18n-flow-screen-coverage.test.ts | 278 ++++++++++++++++++ 2 files changed, 381 insertions(+), 1 deletion(-) diff --git a/packages/cli/src/utils/i18n-extract.ts b/packages/cli/src/utils/i18n-extract.ts index 158aceff6d..2bff68950c 100644 --- a/packages/cli/src/utils/i18n-extract.ts +++ b/packages/cli/src/utils/i18n-extract.ts @@ -130,6 +130,7 @@ import { globalFilterKey, walkAddressedPageComponents, } from '@objectstack/spec/system'; +import { FLOW_REGION_SLOTS_BY_TYPE } from '@objectstack/spec/automation'; import { DEFAULT_METADATA_TYPE_REGISTRY } from '@objectstack/spec/kernel'; import { deriveFieldGroupLayout } from '@objectstack/spec/data'; import { expandViewContainer, InlineLocaleMapSchema } from '@objectstack/spec/ui'; @@ -1546,6 +1547,84 @@ function walkDatasets(config: any, out: ExpectedEntry[]): void { */ const SCREEN_NODE_TYPE = 'screen'; +/** + * Depth ceiling for the region recursion, mirroring the ceiling the spec-side + * walks use (`conversions/walk.ts`, `automation/control-flow.zod.ts`) and for + * the same reason: a stack handed to `defineStack` is hand-built objects rather + * than parsed JSON, so a region that contains itself is reachable and would + * otherwise be unbounded recursion on the extract path. + */ +const MAX_REGION_DEPTH = 32; + +/** + * Every flow node of one flow, container FIRST and depth-first — **including + * the nodes nested inside ADR-0031 structured regions** (`loop.config.body`, + * `parallel.config.branches[]`, `try_catch.config.try`/`.catch`), to any depth. + * + * **WHERE a region lives is imported, never restated.** + * {@link FLOW_REGION_SLOTS_BY_TYPE} (`@objectstack/spec/automation`) is the one + * declaration of that fact, and `automation/region-slots.ts` is explicit that + * the *table* is the shared thing while the *walks* are deliberately not merged + * — they take different inputs and yield different units (a graph, a + * copy-on-write rewrite, a node with a diagnostic path). This pass is a fourth + * unit again: it collects nodes to harvest KEYS from, rewriting nothing. So it + * reads that table exactly as `packages/lint`'s `walkFlowNodes` does, and a + * local copy of the slot list — the defect this walker's own card is an + * instance of, one package over — is what the import exists to prevent. + * + * ⚠️ The region-bearing descent could NOT be imported: `mapFlowNodeList` + * (`spec/conversions/walk.ts`) is reachable from no `exports` subpath of + * `@objectstack/spec` by deliberate design — its docblock says so and + * `packages/spec/api-surface/*.json` lists none of its symbols — and + * `packages/lint`'s `walkFlowNodes` is not exported from that package's entry + * either. Both would have to widen a package's public surface to be reused + * here, so the shared table is the whole of what can honestly be shared. + * + * A value that is not region-shaped passes through untouched: `config` is an + * open record and `body` in particular is also an ordinary key elsewhere (an + * `http` node's request payload), so the shape is checked, never assumed. + */ +function collectFlowNodesDeep(nodes: unknown): any[] { + const out: any[] = []; + + const visit = (list: unknown, depth: number): void => { + if (!Array.isArray(list) || depth > MAX_REGION_DEPTH) return; + for (const node of list) { + if (!node || typeof node !== 'object' || Array.isArray(node)) continue; + out.push(node); + + // Keyed off the node's own `type` through the Map, never an object + // literal: `type` is author-controlled and an open namespace (ADR-0018), + // so a lookup on a plain object would resolve `'constructor'` through + // `Object`'s prototype chain and hand this walk something that is not a + // slot list. + const slots = typeof node.type === 'string' ? FLOW_REGION_SLOTS_BY_TYPE.get(node.type) : undefined; + if (!slots) continue; + const config = node.config; + if (!config || typeof config !== 'object' || Array.isArray(config)) continue; + + for (const { key, arity } of slots) { + const raw = (config as any)[key]; + if (arity === 'many') { + // `parallel`: an array of regions, each with its own `nodes`. + if (!Array.isArray(raw)) continue; + for (const branch of raw) visitRegion(branch, depth + 1); + } else { + visitRegion(raw, depth + 1); + } + } + } + }; + + const visitRegion = (region: unknown, depth: number): void => { + if (!region || typeof region !== 'object' || Array.isArray(region)) return; + visit((region as any).nodes, depth); + }; + + visit(nodes, 0); + return out; +} + /** * Emit the screen-flow copy surface (#7646, resolver landed in #11287). * @@ -1588,6 +1667,29 @@ const SCREEN_NODE_TYPE = 'screen'; * A screen node whose `waitForInput` is `false` is deliberately NOT skipped: * `translateFlow` overlays every screen node, and a walker that skipped one * would re-open the extractable-but-ungated gap in miniature. + * + * **Every screen node, at any DEPTH** (#17511). The node universe comes from + * {@link collectFlowNodesDeep}, not from `flow.nodes` flat: a `type: 'screen'` + * node inside an ADR-0031 region is a real screen — the executor pauses on it + * and the client receives its `ScreenSpec.nodeId` — so `translateFlow` overlays + * it and the bundle key is live for it. The flat walk reached the container and + * stopped, which was the same extractable-but-ungated gap the paragraph above + * refuses, one level in and worse: with no entry emitted there is no skeleton + * key for a translator to fill AND no coverage row to demand it, so the hole + * was invisible to the mechanism built to report holes. + * + * **Depth does not enter the key, on purpose.** The entry stays + * `flows..screens..…` at every depth because that is what the + * resolver reads: `lookupFlowScreenCopy(bundle, flowName, nodeId)` is keyed by + * node id alone and, as `translateFlow`'s docblock puts it, "the bundle schema + * is keyed by node id and knows nothing about depth". A path segment for the + * region would offer a key nothing resolves — precisely the producer/consumer + * drift the imported key face exists to prevent. Consequence for a node id + * REPEATED at two depths: both screens address one bundle slot, so + * {@link dedupeByPath} collapses them to a single entry, first emission wins, + * and the walk is outer-before-inner so which one that is stays deterministic. + * That is not a loss — one slot can serve only one string, and the resolver + * overlays that string onto both nodes. */ function walkScreenFlows(config: any, out: ExpectedEntry[]): void { const flows: any[] = Array.isArray(config?.flows) ? config.flows : []; @@ -1601,7 +1703,7 @@ function walkScreenFlows(config: any, out: ExpectedEntry[]): void { // keeps a label-less flow from seeding an empty string anyway. pushOptional(out, ['flows', flowName, 'label'], flow.label, 'flow', scope); - const nodes: any[] = Array.isArray(flow.nodes) ? flow.nodes : []; + const nodes: any[] = collectFlowNodesDeep(flow.nodes); for (const node of nodes) { if (!node || typeof node !== 'object' || node.type !== SCREEN_NODE_TYPE) continue; const nodeId = typeof node.id === 'string' && node.id.length > 0 ? node.id : undefined; diff --git a/packages/cli/test/i18n-flow-screen-coverage.test.ts b/packages/cli/test/i18n-flow-screen-coverage.test.ts index 617c64df72..d49b486b4e 100644 --- a/packages/cli/test/i18n-flow-screen-coverage.test.ts +++ b/packages/cli/test/i18n-flow-screen-coverage.test.ts @@ -308,3 +308,281 @@ describe('`os i18n extract` scaffolds the flows skeleton', () => { expect((filtered.bundles.en as any).objects).toBeUndefined(); }); }); + +// ── objectstack#17511 — the same hole, one region deep ──────────────────── +// +// Everything above walks `flow.nodes` at the TOP level only, which is what the +// walker did: `FlowNode.config` carries ADR-0031 regions (`loop.config.body`, +// `parallel.config.branches[].nodes`, `try_catch.config.try`/`.catch`, nesting +// arbitrarily), and a `type: 'screen'` node inside one is a real screen — the +// executor pauses on it and the client receives its `ScreenSpec.nodeId`, so +// `translateFlow` overlays it (#11745 / PR #17521) and the bundle key is live. +// +// The flat walk reached the container and stopped, so the nested step got NO +// skeleton entry and NO coverage row. That pairing is why the defect outranks +// "untranslated": a missing coverage row reads as "nothing to do here", so the +// gap was invisible to the very mechanism built to report gaps — a silent zero +// rather than a visible failure, the same shape as the #11485 census above one +// level in. +// +// ## The lit control lives in the fixture, not next to it +// +// `welcome` below is a TOP-level screen, reached by the old flat walk too. It +// is in the same flow as the nested ones on purpose: without it, a green on the +// nested keys could be read as "the fixture loaded and the walk ran", and a red +// could be read as "the fixture never loaded at all". With it, the two +// explanations separate — the control asserts the harness, the nested keys +// assert the descent. + +/** + * One flow carrying a screen in EVERY region slot the table declares, and at + * three different depths, so a descent that handles `loop` but forgets + * `parallel`'s array-of-regions arity fails on a named key rather than on a + * count. + * + * welcome depth 0 (the lit control) + * pick_region depth 1 loop.config.body + * accept_terms depth 2 parallel.config.branches[0] + * card_details depth 3 try_catch.config.try + * payment_failed depth 3 try_catch.config.catch + */ +const nestedOnboarding = { + name: 'onboarding', + label: 'Onboarding', + type: 'screen', + nodes: [ + { id: 'welcome', type: 'screen', label: 'Welcome', config: { title: 'Welcome aboard' } }, + { + id: 'per_region', + type: 'loop', + label: 'For each region', + config: { + body: { + nodes: [ + { + id: 'pick_region', + type: 'screen', + label: 'Pick Region Step', + config: { + title: 'Pick a region', + fields: [{ name: 'region_code', label: 'Region' }, { name: 'notes' }], + }, + }, + { + id: 'fan_out', + type: 'parallel', + config: { + branches: [ + { + name: 'legal', + nodes: [{ id: 'accept_terms', type: 'screen', config: { title: 'Accept the terms' } }], + }, + { + name: 'billing', + nodes: [ + { + id: 'guard_payment', + type: 'try_catch', + config: { + // No `config.title`: the executor draws the node label + // for a nested screen exactly as for a top-level one. + try: { nodes: [{ id: 'card_details', type: 'screen', label: 'Card Details' }] }, + catch: { nodes: [{ id: 'payment_failed', type: 'screen', config: { title: 'Payment failed' } }] }, + }, + }, + ], + }, + ], + }, + }, + ], + }, + }, + }, + ], + edges: [], +}; + +/** The #11485 tree shape, with the nested wizard as its only flow. */ +const nestedTree = (flowTranslations?: Record) => ({ + i18n: { defaultLocale: 'en', supportedLocales: ['en', 'zh-CN'] }, + objects: [{ name: 'crm_lead', label: 'Lead', fields: { company: { label: 'Company' } } }], + flows: [nestedOnboarding], + translations: [ + { + 'zh-CN': { + objects: { crm_lead: { label: '线索', fields: { company: { label: '公司' } } } }, + ...(flowTranslations ? { flows: flowTranslations } : {}), + }, + }, + ], +}); + +describe('a screen inside an ADR-0031 region (#17511)', () => { + it('harvests every nested screen, at every region slot and depth', () => { + // The exact face, so a key that should NOT exist fails here too. Eight of + // these ten were absent before the descent landed; `flows.onboarding.label` + // and `screens.welcome.title` are the two the flat walk already reached. + expect(flowKeys({ flows: [nestedOnboarding] }).sort()).toEqual([ + 'flows.onboarding.label', + 'flows.onboarding.screens.accept_terms.title', + 'flows.onboarding.screens.card_details.title', + 'flows.onboarding.screens.payment_failed.title', + 'flows.onboarding.screens.pick_region.fields.notes.label', + 'flows.onboarding.screens.pick_region.fields.notes.placeholder', + 'flows.onboarding.screens.pick_region.fields.region_code.label', + 'flows.onboarding.screens.pick_region.fields.region_code.placeholder', + 'flows.onboarding.screens.pick_region.title', + 'flows.onboarding.screens.welcome.title', + ]); + }); + + it('gives the coverage gate a row for the nested step, not just the top-level one', () => { + const zh = userIssues(computeI18nCoverage(nestedTree())).filter((i) => i.locale === 'zh-CN'); + const keys = zh.map((i) => i.key); + + // The control: present before this card, and still present. + expect(keys).toContain('flows.onboarding.screens.welcome.title'); + // The rows that did not exist — one per region slot the table declares. + expect(keys).toContain('flows.onboarding.screens.pick_region.title'); + expect(keys).toContain('flows.onboarding.screens.pick_region.fields.region_code.label'); + expect(keys).toContain('flows.onboarding.screens.accept_terms.title'); + expect(keys).toContain('flows.onboarding.screens.card_details.title'); + expect(keys).toContain('flows.onboarding.screens.payment_failed.title'); + // Attributed to the flow bucket, so `os lint` renders `i18n/missing-flow`. + for (const issue of zh) expect(issue.source).toBe('flow'); + // The object surface IS translated: the whole report is the wizard. + expect(keys.filter((k) => !k.startsWith('flows.'))).toEqual([]); + }); + + it('seeds a nested screen exactly as a top-level one, never more narrowly', () => { + const entries = collectExpectedEntries({ flows: [nestedOnboarding] }); + const at = (key: string) => entries.find((e) => e.path.join('.') === key); + + // `title` falls back to the node `label` — `ScreenSpec.title` is + // `config.title ?? node.label` at every depth, so the nested screen owes + // the same demand as `summary` does at the top level. + expect(at('flows.onboarding.screens.card_details.title')).toMatchObject({ + sourceValue: 'Card Details', + inline: 'Card Details', + source: 'flow', + }); + // A nested field's `label` falls back to its `name` as a DERIVED seed: + // usable skeleton, no demand for a string nobody authored. Getting this + // wrong is how a region-aware walk starts failing the gate on machine ids. + expect(at('flows.onboarding.screens.pick_region.fields.notes.label')).toMatchObject({ + sourceValue: 'notes', + inline: undefined, + }); + const gated = computeI18nCoverage(nestedTree()).issues.map((i) => i.key); + expect(gated).not.toContain('flows.onboarding.screens.pick_region.fields.notes.label'); + // And the authored one IS demanded. + expect(at('flows.onboarding.screens.pick_region.fields.region_code.label')).toMatchObject({ + sourceValue: 'Region', + inline: 'Region', + }); + }); + + it('scaffolds the nested drill into a bundle the strict schema accepts', () => { + const result = extractTranslations({ flows: [nestedOnboarding] }, { locales: ['en', 'zh-CN'] }); + const en = result.bundles.en as any; + const zh = result.bundles['zh-CN'] as any; + + expect(en.flows.onboarding.screens.card_details.title).toBe('Card Details'); + expect(en.flows.onboarding.screens.accept_terms.title).toBe('Accept the terms'); + // The translator's empty slots for the nested steps — the vocabulary that + // did not exist in the skeleton at all before this card. + expect(zh.flows.onboarding.screens.payment_failed.title).toBe(''); + // Keyed by node id at the TOP of `screens`, flat: depth is not in the key, + // because `lookupFlowScreenCopy` is keyed by node id and knows nothing + // about depth. A region segment here would offer a key nothing reads. + expect(Object.keys(en.flows.onboarding.screens).sort()).toEqual([ + 'accept_terms', + 'card_details', + 'payment_failed', + 'pick_region', + 'welcome', + ]); + expect(TranslationDataSchema.safeParse(en).success).toBe(true); + }); + + it('goes quiet once the nested steps are translated', () => { + const report = computeI18nCoverage( + nestedTree({ + onboarding: { + label: '入职', + screens: { + welcome: { title: '欢迎' }, + pick_region: { title: '选择区域', fields: { region_code: { label: '区域' } } }, + accept_terms: { title: '接受条款' }, + card_details: { title: '银行卡信息' }, + payment_failed: { title: '支付失败' }, + }, + }, + }), + ); + + expect(userIssues(report)).toEqual([]); + }); + + it('reads the region TABLE, so a `body` that is not a region slot stays unwalked', () => { + // `config` is an open record and `body` is an ordinary key elsewhere — an + // `http` node's request payload. `http` owns no slot in + // `FLOW_REGION_SLOTS_BY_TYPE`, so a screen-shaped object sitting in its + // payload is data, not a flow node, and must not become a bundle key. + // This is what consulting the table buys over walking every `body`. + expect( + flowKeys({ + flows: [ + { + name: 'callout', + label: 'Callout', + nodes: [ + { + id: 'post', + type: 'http', + config: { body: { nodes: [{ id: 'not_a_screen', type: 'screen', config: { title: 'Payload' } }] } }, + }, + ], + }, + ], + }), + ).toEqual(['flows.callout.label']); + }); + + it('collapses a node id repeated at two depths onto its one bundle slot', () => { + // The bundle addresses a screen by node id alone, so two screens sharing an + // id share one slot — one string is all the resolver can overlay onto both. + // `dedupeByPath` therefore keeps ONE entry, and the walk being + // outer-before-inner makes which one deterministic. + const entries = collectExpectedEntries({ + flows: [ + { + name: 'dup', + label: 'Dup', + nodes: [ + { id: 'step', type: 'screen', config: { title: 'Outer' } }, + { id: 'wrap', type: 'loop', config: { body: { nodes: [{ id: 'step', type: 'screen', config: { title: 'Inner' } }] } } }, + ], + }, + ], + }).filter((e) => e.path.join('.') === 'flows.dup.screens.step.title'); + + expect(entries).toHaveLength(1); + expect(entries[0]).toMatchObject({ sourceValue: 'Outer' }); + }); + + it('terminates on a region that contains itself', () => { + // A stack handed to `defineStack` is hand-built objects, so a self + // -referencing region is reachable; the depth ceiling is what keeps this a + // finite walk rather than a stack overflow on the extract path. + const loop: any = { id: 'spin', type: 'loop', config: { body: { nodes: [] } } }; + loop.config.body.nodes.push(loop); + + expect( + flowKeys({ + flows: [{ name: 'cyclic', label: 'Cyclic', nodes: [loop, { id: 'real', type: 'screen', config: { title: 'Reached' } }] }], + }).sort(), + ).toEqual(['flows.cyclic.label', 'flows.cyclic.screens.real.title']); + }); +}); From faf7b1b834cc203dc8913a9dc69388f0883e3074 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 11 Sep 2026 07:19:31 +0000 Subject: [PATCH 2/2] chore: changeset for the region-aware i18n extract walk Claude-Session: https://claude.ai/code/session_01TSf4DV7ziu4V5j73e46b7c Co-authored-by: Claude Co-Authored-By: Claude Opus 5 --- .../17511-i18n-extract-region-screens.md | 45 +++++++++++++++++++ 1 file changed, 45 insertions(+) create mode 100644 .changeset/17511-i18n-extract-region-screens.md diff --git a/.changeset/17511-i18n-extract-region-screens.md b/.changeset/17511-i18n-extract-region-screens.md new file mode 100644 index 0000000000..add4430664 --- /dev/null +++ b/.changeset/17511-i18n-extract-region-screens.md @@ -0,0 +1,45 @@ +--- +'@objectstack/cli': patch +--- + +`os i18n extract` reaches a `screen` node nested inside an ADR-0031 flow region + +`walkScreenFlows` (`packages/cli/src/utils/i18n-extract.ts`) iterated +`flow.nodes` flat, so a `type: 'screen'` node inside a 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. + +**Why that pairing is the defect and not just a missing translation.** A nested +wizard step is a real screen: the executor pauses on it and the client receives +its `ScreenSpec.nodeId`, so `translateFlow` overlays the bundle onto it and the +key is live. With no entry emitted, a translator was never shown the key AND +`os lint` / `pnpm check:i18n-coverage` had no row to demand — the gap was +invisible to the mechanism built to report gaps. A green i18n gate on a tree +whose nested steps render source-locale text was green because the surface was +unreachable, not because the app was translated. + +The node universe now comes from a region-aware descent that reads the one +shared declaration of WHERE a region lives, `FLOW_REGION_SLOTS_BY_TYPE` from +`@objectstack/spec/automation` — the same table `packages/lint`'s +`walkFlowNodes` reads. No local copy of the slot list is introduced: a second +region table in a fourth package is the very shape this defect is an instance +of. + +**Depth deliberately does not enter the key.** Entries stay +`flows.NAME.screens.NODE_ID.*` at every depth, because `lookupFlowScreenCopy` +is keyed by node id alone and the bundle schema knows nothing about depth; a +region path segment would offer a key nothing resolves. A node id repeated at +two depths therefore addresses one bundle slot and collapses to a single entry +(first emission wins, outer before inner) — one slot can serve only one string, +and the resolver overlays that string onto both nodes. + +Seeding is unchanged and applies at every depth: a screen `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 +coverage gate demands no translation of a string nobody authored. + +⛔ No authorable key, bundle shape or export moves — an author who wrote a +nested screen now gets scaffolding and a coverage row where both were silently +absent. Existing keys are byte-unchanged.