Skip to content

Commit d07fc17

Browse files
os-salesclaude
andauthored
fix(cli): reach screen nodes inside ADR-0031 regions in os i18n extract (#17644)
* fix(cli): reach screen nodes inside ADR-0031 regions in i18n extract `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> * chore: changeset for the region-aware i18n extract walk Claude-Session: https://claude.ai/code/session_01TSf4DV7ziu4V5j73e46b7c Co-authored-by: Claude <noreply@anthropic.com> Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 4be4e04 commit d07fc17

3 files changed

Lines changed: 426 additions & 1 deletion

File tree

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
---
2+
'@objectstack/cli': patch
3+
---
4+
5+
`os i18n extract` reaches a `screen` node nested inside an ADR-0031 flow region
6+
7+
`walkScreenFlows` (`packages/cli/src/utils/i18n-extract.ts`) iterated
8+
`flow.nodes` flat, so a `type: 'screen'` node inside a region —
9+
`loop.config.body`, `parallel.config.branches[].nodes`,
10+
`try_catch.config.try` / `.catch`, nesting arbitrarily — was never reached. It
11+
emitted **no** `flows.NAME.screens.NODE_ID.title` / `.fields.*` skeleton entry
12+
and **no** coverage row.
13+
14+
**Why that pairing is the defect and not just a missing translation.** A nested
15+
wizard step is a real screen: the executor pauses on it and the client receives
16+
its `ScreenSpec.nodeId`, so `translateFlow` overlays the bundle onto it and the
17+
key is live. With no entry emitted, a translator was never shown the key AND
18+
`os lint` / `pnpm check:i18n-coverage` had no row to demand — the gap was
19+
invisible to the mechanism built to report gaps. A green i18n gate on a tree
20+
whose nested steps render source-locale text was green because the surface was
21+
unreachable, not because the app was translated.
22+
23+
The node universe now comes from a region-aware descent that reads the one
24+
shared declaration of WHERE a region lives, `FLOW_REGION_SLOTS_BY_TYPE` from
25+
`@objectstack/spec/automation` — the same table `packages/lint`'s
26+
`walkFlowNodes` reads. No local copy of the slot list is introduced: a second
27+
region table in a fourth package is the very shape this defect is an instance
28+
of.
29+
30+
**Depth deliberately does not enter the key.** Entries stay
31+
`flows.NAME.screens.NODE_ID.*` at every depth, because `lookupFlowScreenCopy`
32+
is keyed by node id alone and the bundle schema knows nothing about depth; a
33+
region path segment would offer a key nothing resolves. A node id repeated at
34+
two depths therefore addresses one bundle slot and collapses to a single entry
35+
(first emission wins, outer before inner) — one slot can serve only one string,
36+
and the resolver overlays that string onto both nodes.
37+
38+
Seeding is unchanged and applies at every depth: a screen `title` falls back to
39+
the node `label` (what `ScreenSpec.title` draws), and a field `label` falls back
40+
to its `name` as a *derived* seed, so the skeleton stays usable while the
41+
coverage gate demands no translation of a string nobody authored.
42+
43+
⛔ No authorable key, bundle shape or export moves — an author who wrote a
44+
nested screen now gets scaffolding and a coverage row where both were silently
45+
absent. Existing keys are byte-unchanged.

packages/cli/src/utils/i18n-extract.ts

Lines changed: 103 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -130,6 +130,7 @@ import {
130130
globalFilterKey,
131131
walkAddressedPageComponents,
132132
} from '@objectstack/spec/system';
133+
import { FLOW_REGION_SLOTS_BY_TYPE } from '@objectstack/spec/automation';
133134
import { DEFAULT_METADATA_TYPE_REGISTRY } from '@objectstack/spec/kernel';
134135
import { deriveFieldGroupLayout } from '@objectstack/spec/data';
135136
import { expandViewContainer, InlineLocaleMapSchema } from '@objectstack/spec/ui';
@@ -1546,6 +1547,84 @@ function walkDatasets(config: any, out: ExpectedEntry[]): void {
15461547
*/
15471548
const SCREEN_NODE_TYPE = 'screen';
15481549

1550+
/**
1551+
* Depth ceiling for the region recursion, mirroring the ceiling the spec-side
1552+
* walks use (`conversions/walk.ts`, `automation/control-flow.zod.ts`) and for
1553+
* the same reason: a stack handed to `defineStack` is hand-built objects rather
1554+
* than parsed JSON, so a region that contains itself is reachable and would
1555+
* otherwise be unbounded recursion on the extract path.
1556+
*/
1557+
const MAX_REGION_DEPTH = 32;
1558+
1559+
/**
1560+
* Every flow node of one flow, container FIRST and depth-first — **including
1561+
* the nodes nested inside ADR-0031 structured regions** (`loop.config.body`,
1562+
* `parallel.config.branches[]`, `try_catch.config.try`/`.catch`), to any depth.
1563+
*
1564+
* **WHERE a region lives is imported, never restated.**
1565+
* {@link FLOW_REGION_SLOTS_BY_TYPE} (`@objectstack/spec/automation`) is the one
1566+
* declaration of that fact, and `automation/region-slots.ts` is explicit that
1567+
* the *table* is the shared thing while the *walks* are deliberately not merged
1568+
* — they take different inputs and yield different units (a graph, a
1569+
* copy-on-write rewrite, a node with a diagnostic path). This pass is a fourth
1570+
* unit again: it collects nodes to harvest KEYS from, rewriting nothing. So it
1571+
* reads that table exactly as `packages/lint`'s `walkFlowNodes` does, and a
1572+
* local copy of the slot list — the defect this walker's own card is an
1573+
* instance of, one package over — is what the import exists to prevent.
1574+
*
1575+
* ⚠️ The region-bearing descent could NOT be imported: `mapFlowNodeList`
1576+
* (`spec/conversions/walk.ts`) is reachable from no `exports` subpath of
1577+
* `@objectstack/spec` by deliberate design — its docblock says so and
1578+
* `packages/spec/api-surface/*.json` lists none of its symbols — and
1579+
* `packages/lint`'s `walkFlowNodes` is not exported from that package's entry
1580+
* either. Both would have to widen a package's public surface to be reused
1581+
* here, so the shared table is the whole of what can honestly be shared.
1582+
*
1583+
* A value that is not region-shaped passes through untouched: `config` is an
1584+
* open record and `body` in particular is also an ordinary key elsewhere (an
1585+
* `http` node's request payload), so the shape is checked, never assumed.
1586+
*/
1587+
function collectFlowNodesDeep(nodes: unknown): any[] {
1588+
const out: any[] = [];
1589+
1590+
const visit = (list: unknown, depth: number): void => {
1591+
if (!Array.isArray(list) || depth > MAX_REGION_DEPTH) return;
1592+
for (const node of list) {
1593+
if (!node || typeof node !== 'object' || Array.isArray(node)) continue;
1594+
out.push(node);
1595+
1596+
// Keyed off the node's own `type` through the Map, never an object
1597+
// literal: `type` is author-controlled and an open namespace (ADR-0018),
1598+
// so a lookup on a plain object would resolve `'constructor'` through
1599+
// `Object`'s prototype chain and hand this walk something that is not a
1600+
// slot list.
1601+
const slots = typeof node.type === 'string' ? FLOW_REGION_SLOTS_BY_TYPE.get(node.type) : undefined;
1602+
if (!slots) continue;
1603+
const config = node.config;
1604+
if (!config || typeof config !== 'object' || Array.isArray(config)) continue;
1605+
1606+
for (const { key, arity } of slots) {
1607+
const raw = (config as any)[key];
1608+
if (arity === 'many') {
1609+
// `parallel`: an array of regions, each with its own `nodes`.
1610+
if (!Array.isArray(raw)) continue;
1611+
for (const branch of raw) visitRegion(branch, depth + 1);
1612+
} else {
1613+
visitRegion(raw, depth + 1);
1614+
}
1615+
}
1616+
}
1617+
};
1618+
1619+
const visitRegion = (region: unknown, depth: number): void => {
1620+
if (!region || typeof region !== 'object' || Array.isArray(region)) return;
1621+
visit((region as any).nodes, depth);
1622+
};
1623+
1624+
visit(nodes, 0);
1625+
return out;
1626+
}
1627+
15491628
/**
15501629
* Emit the screen-flow copy surface (#7646, resolver landed in #11287).
15511630
*
@@ -1588,6 +1667,29 @@ const SCREEN_NODE_TYPE = 'screen';
15881667
* A screen node whose `waitForInput` is `false` is deliberately NOT skipped:
15891668
* `translateFlow` overlays every screen node, and a walker that skipped one
15901669
* would re-open the extractable-but-ungated gap in miniature.
1670+
*
1671+
* **Every screen node, at any DEPTH** (#17511). The node universe comes from
1672+
* {@link collectFlowNodesDeep}, not from `flow.nodes` flat: a `type: 'screen'`
1673+
* node inside an ADR-0031 region is a real screen — the executor pauses on it
1674+
* and the client receives its `ScreenSpec.nodeId` — so `translateFlow` overlays
1675+
* it and the bundle key is live for it. The flat walk reached the container and
1676+
* stopped, which was the same extractable-but-ungated gap the paragraph above
1677+
* refuses, one level in and worse: with no entry emitted there is no skeleton
1678+
* key for a translator to fill AND no coverage row to demand it, so the hole
1679+
* was invisible to the mechanism built to report holes.
1680+
*
1681+
* **Depth does not enter the key, on purpose.** The entry stays
1682+
* `flows.<flow>.screens.<node_id>.…` at every depth because that is what the
1683+
* resolver reads: `lookupFlowScreenCopy(bundle, flowName, nodeId)` is keyed by
1684+
* node id alone and, as `translateFlow`'s docblock puts it, "the bundle schema
1685+
* is keyed by node id and knows nothing about depth". A path segment for the
1686+
* region would offer a key nothing resolves — precisely the producer/consumer
1687+
* drift the imported key face exists to prevent. Consequence for a node id
1688+
* REPEATED at two depths: both screens address one bundle slot, so
1689+
* {@link dedupeByPath} collapses them to a single entry, first emission wins,
1690+
* and the walk is outer-before-inner so which one that is stays deterministic.
1691+
* That is not a loss — one slot can serve only one string, and the resolver
1692+
* overlays that string onto both nodes.
15911693
*/
15921694
function walkScreenFlows(config: any, out: ExpectedEntry[]): void {
15931695
const flows: any[] = Array.isArray(config?.flows) ? config.flows : [];
@@ -1601,7 +1703,7 @@ function walkScreenFlows(config: any, out: ExpectedEntry[]): void {
16011703
// keeps a label-less flow from seeding an empty string anyway.
16021704
pushOptional(out, ['flows', flowName, 'label'], flow.label, 'flow', scope);
16031705

1604-
const nodes: any[] = Array.isArray(flow.nodes) ? flow.nodes : [];
1706+
const nodes: any[] = collectFlowNodesDeep(flow.nodes);
16051707
for (const node of nodes) {
16061708
if (!node || typeof node !== 'object' || node.type !== SCREEN_NODE_TYPE) continue;
16071709
const nodeId = typeof node.id === 'string' && node.id.length > 0 ? node.id : undefined;

0 commit comments

Comments
 (0)