Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 45 additions & 0 deletions .changeset/17511-i18n-extract-region-screens.md
Original file line number Diff line number Diff line change
@@ -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.
104 changes: 103 additions & 1 deletion packages/cli/src/utils/i18n-extract.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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).
*
Expand Down Expand Up @@ -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.<flow>.screens.<node_id>.…` 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 : [];
Expand All @@ -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;
Expand Down
Loading
Loading