fix(spec): overlay screen nodes inside ADR-0031 regions in translateFlow

.changeset/translate-flow-walks-adr-0031-regions.md

@@ -0,0 +1,34 @@
+---
+'@objectstack/spec': patch
+---
+
+`translateFlow` overlays screen nodes inside ADR-0031 regions, at any depth
+
+`translateFlow` (`system/i18n-resolver.ts`) read the flat `flow.nodes` array and
+nothing else. But `FlowNode.config` carries ADR-0031 regions —
+`loop.config.body`, `parallel.config.branches[].nodes`,
+`try_catch.config.try`/`.catch` — each holding a full `nodes` array that nests
+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 `flows.<name>.screens.<node_id>.{title,fields.*}` was authored for such a
+node, parsed (the bundle schema is keyed by node id and knows nothing about
+depth) and was then silently never applied. The wizard step rendered its
+source-locale heading and field labels while its siblings one level up were
+translated.
+
+The descent now runs through `mapFlowNodeList`, a per-flow region-aware
+copy-on-write walk shared with the ADR-0087 conversions' `mapFlowNodes`, which
+reads `FLOW_REGION_SLOTS_BY_TYPE` — the single declaration of where a region
+lives (`automation/region-slots.ts`). This resolver is therefore not a fifth
+hand-rolled reader of that table; the fourth pass written against the flat
+one-liner is the last one that had to be.
+
+Reference identity is unchanged and is pinned: a node that resolves nothing
+comes back as the same reference, every container `config` and region `nodes`
+array on the way down is copied only when a descendant actually changed, and a
+flow the bundle does not carry is returned as the same object.
+
+⛔ No wiring changed. `translateFlow` is still deliberately absent from
+`translateMetadataDocument`'s dispatch table and no liveness row moved — that
+decision belongs to the downstream runner card, as its docblock records.

packages/spec/src/conversions/walk.ts

@@ -114,6 +114,53 @@ function mapNodeTree(
   return nextConfig === config ? mapped : { ...mapped, config: nextConfig };
 }
 
+/**
+ * Immutably map ONE flow's `nodes[]` — **including the nodes nested inside
+ * ADR-0031 structured regions** (`loop.config.body`,
+ * `parallel.config.branches[]`, `try_catch.config.try`/`.catch`), to any depth.
+ *
+ * Returns the **same array reference** when nothing under it changed, so a
+ * caller can tell "rewritten" from "untouched" by identity — the copy-on-write
+ * contract this module states for the stack, one level in.
+ *
+ * `mapper` receives each node dict and its path (`${basePath}[i]`, or
+ * `${basePath}[i].config.body.nodes[j]` for a nested one) and returns either the
+ * same reference (no change) or a new dict. Non-dict elements pass through
+ * untouched.
+ *
+ * **Exported because the flow-node unit has more than one consumer, and every
+ * consumer that hand-rolled it walked `nodes` FLAT.** {@link mapFlowNodes} is
+ * the stack-shaped consumer (the ADR-0087 conversions); `translateFlow`
+ * (`system/i18n-resolver.ts`) is the document-shaped one — it is handed a
+ * single flow, never a stack, so it could not reach `mapFlowNodes` at all and
+ * walked `flow.nodes` flat instead, leaving a `type: 'screen'` node inside any
+ * region unoverlaid. Both now descend through this one function, so the region
+ * table has one reader per unit rather than one per call site: the "why does
+ * the flat walk keep being reachable" question is answered by there being a
+ * region-aware helper at the shape a document-level caller actually holds.
+ *
+ * ⚠️ Deliberately NOT re-exported from `conversions/index.js`: this is
+ * cross-module reuse inside `packages/spec`, not a published entry-point
+ * export. `walk.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.
+ */
+export function mapFlowNodeList(
+  nodes: readonly unknown[],
+  basePath: string,
+  mapper: (node: Dict, path: string) => Dict,
+): readonly unknown[] {
+  let changed = false;
+  const next = nodes.map((node, ni) => {
+    if (!isDict(node)) return node;
+    const mapped = mapNodeTree(node, `${basePath}[${ni}]`, mapper, 0);
+    if (mapped !== node) changed = true;
+    return mapped;
+  });
+  return changed ? next : nodes;
+}
+
 /**
  * Immutably map every flow node in `stack.flows[].nodes[]` — **including the
  * nodes nested inside ADR-0031 structured regions** (`loop.config.body`,
@@ -124,6 +171,9 @@ function mapNodeTree(
  * the same reference (no change) or a new dict. The stack, the `flows` array, an
  * individual flow, its `nodes` array, and every container `config` on the way
  * down are each copied only when a descendant actually changed.
+ *
+ * The per-flow descent is {@link mapFlowNodeList}'s — one implementation, so
+ * this walker and the document-overlay one cannot drift about which nodes exist.
  */
 export function mapFlowNodes(
   stack: Dict,
@@ -135,14 +185,8 @@ export function mapFlowNodes(
   let flowsChanged = false;
   const nextFlows = flows.map((flow, fi) => {
     if (!isDict(flow) || !Array.isArray(flow.nodes)) return flow;
-    let nodesChanged = false;
-    const nextNodes = flow.nodes.map((node, ni) => {
-      if (!isDict(node)) return node;
-      const mapped = mapNodeTree(node, `flows[${fi}].nodes[${ni}]`, mapper, 0);
-      if (mapped !== node) nodesChanged = true;
-      return mapped;
-    });
-    if (!nodesChanged) return flow;
+    const nextNodes = mapFlowNodeList(flow.nodes, `flows[${fi}].nodes`, mapper);
+    if (nextNodes === flow.nodes) return flow;
     flowsChanged = true;
     return { ...flow, nodes: nextNodes };
   });

packages/spec/src/system/i18n-resolver.test.ts

@@ -3472,6 +3472,207 @@ describe('translateFlow (#11287)', () => {
   });
 });
 
+describe('translateFlow — screens inside ADR-0031 regions (#11745)', () => {
+  // The fourth pass in this repo written against a flat `flow.nodes` walk
+  // (#4347 `applyConversionsToFlow`, #4380 the flow lint rules, #5383
+  // `flow-inert-node-condition` were the first three). `FlowNode.config`
+  // carries regions — `loop.config.body`, `parallel.config.branches[].nodes`,
+  // `try_catch.config.try`/`.catch` — each a full `nodes` array that nests
+  // arbitrarily. A `type: 'screen'` node inside one is a real screen: the
+  // executor pauses on it and the client receives its `ScreenSpec.nodeId`, so
+  // `flows.<name>.screens.<node_id>` is authored for it and parses (the bundle
+  // schema is keyed by node id and knows nothing about depth). The flat walk
+  // accepted those keys and silently never applied them.
+  const bundle: FlowTestBundle = {
+    'zh-CN': {
+      flows: {
+        nested_regions: {
+          label: '嵌套区域',
+          screens: {
+            top_screen: { title: '顶层', fields: { f_top: { label: '顶层字段' } } },
+            loop_screen: { title: '循环内', fields: { f_loop: { label: '循环字段', placeholder: '请输入' } } },
+            branch_screen: { title: '分支内' },
+            try_screen: { title: 'try 内' },
+            catch_screen: { title: 'catch 内' },
+            deep_screen: { title: '深层' },
+          },
+        },
+      },
+    },
+  };
+
+  /**
+   * One flow carrying a screen in EVERY region slot the table declares, plus
+   * one two-levels-deep (a screen inside a loop inside a try's `try` region)
+   * and two negative controls the bundle deliberately omits — a non-screen
+   * node inside a region, and a nested screen with no bundle entry.
+   */
+  const regionFlow = (): any => ({
+    name: 'nested_regions',
+    label: 'Nested Regions',
+    type: 'screen',
+    nodes: [
+      {
+        id: 'top_screen',
+        type: 'screen',
+        label: 'Top',
+        config: { title: 'Top', fields: [{ name: 'f_top', label: 'Top Field' }] },
+      },
+      {
+        id: 'the_loop',
+        type: 'loop',
+        label: 'Loop',
+        config: {
+          collection: '{items}',
+          body: {
+            nodes: [
+              {
+                id: 'loop_screen',
+                type: 'screen',
+                label: 'In Loop',
+                config: { title: 'In Loop', fields: [{ name: 'f_loop', label: 'Loop Field' }] },
+              },
+              { id: 'loop_script', type: 'script', label: 'Loop Script' },
+              {
+                id: 'untranslated_screen',
+                type: 'screen',
+                label: 'Untranslated',
+                config: { title: 'Untranslated' },
+              },
+            ],
+            edges: [],
+          },
+        },
+      },
+      {
+        id: 'the_parallel',
+        type: 'parallel',
+        label: 'Parallel',
+        config: {
+          branches: [
+            { nodes: [{ id: 'branch_script', type: 'script', label: 'Branch Script' }], edges: [] },
+            {
+              nodes: [
+                { id: 'branch_screen', type: 'screen', label: 'In Branch', config: { title: 'In Branch' } },
+              ],
+              edges: [],
+            },
+          ],
+        },
+      },
+      {
+        id: 'the_try',
+        type: 'try_catch',
+        label: 'Try',
+        config: {
+          try: {
+            nodes: [
+              { id: 'try_screen', type: 'screen', label: 'In Try', config: { title: 'In Try' } },
+              {
+                id: 'inner_loop',
+                type: 'loop',
+                label: 'Inner Loop',
+                config: {
+                  collection: '{rows}',
+                  body: {
+                    nodes: [
+                      { id: 'deep_screen', type: 'screen', label: 'Deep', config: { title: 'Deep' } },
+                    ],
+                    edges: [],
+                  },
+                },
+              },
+            ],
+            edges: [],
+          },
+          catch: {
+            nodes: [{ id: 'catch_screen', type: 'screen', label: 'In Catch', config: { title: 'In Catch' } }],
+            edges: [],
+          },
+        },
+      },
+    ],
+    edges: [],
+  });
+
+  // Explicit paths rather than a recursive finder: the assertion then names
+  // the SLOT it is about, so a failure says which region stopped being walked.
+  const loopScreen = (d: any) => d.nodes[1].config.body.nodes[0];
+  const branchScreen = (d: any) => d.nodes[2].config.branches[1].nodes[0];
+  const tryScreen = (d: any) => d.nodes[3].config.try.nodes[0];
+  const deepScreen = (d: any) => d.nodes[3].config.try.nodes[1].config.body.nodes[0];
+  const catchScreen = (d: any) => d.nodes[3].config.catch.nodes[0];
+
+  it('overlays a screen in EVERY region slot — loop.body, parallel.branches[], try_catch.try and .catch', () => {
+    const out: any = translateFlow(regionFlow(), bundle, { locale: 'zh-CN' });
+    expect(loopScreen(out).config.title).toBe('循环内');
+    expect(branchScreen(out).config.title).toBe('分支内');
+    expect(tryScreen(out).config.title).toBe('try 内');
+    expect(catchScreen(out).config.title).toBe('catch 内');
+  });
+
+  it('reaches ARBITRARY depth — a screen inside a loop inside a try_catch', () => {
+    const out: any = translateFlow(regionFlow(), bundle, { locale: 'zh-CN' });
+    expect(deepScreen(out).config.title).toBe('深层');
+  });
+
+  it('overlays per-FIELD copy on a nested screen, not just its title', () => {
+    const out: any = translateFlow(regionFlow(), bundle, { locale: 'zh-CN' });
+    const field = loopScreen(out).config.fields.find((f: any) => f.name === 'f_loop');
+    expect(field.label).toBe('循环字段');
+    expect(field.placeholder).toBe('请输入');
+  });
+
+  it('still overlays the FLAT top-level screen — the reach grew, it did not move', () => {
+    const out: any = translateFlow(regionFlow(), bundle, { locale: 'zh-CN' });
+    expect(out.label).toBe('嵌套区域');
+    expect(out.nodes[0].config.title).toBe('顶层');
+    expect(out.nodes[0].config.fields[0].label).toBe('顶层字段');
+  });
+
+  it('preserves REFERENCE IDENTITY for everything that resolved nothing', () => {
+    // Load-bearing: `nodesChanged` is read by identity, so a node copied
+    // without cause turns every untouched document into a changed one.
+    const doc = regionFlow();
+    const out: any = translateFlow(doc, bundle, { locale: 'zh-CN' });
+
+    // A non-screen node inside a region, and a nested screen the bundle omits.
+    expect(loopScreen(out) === loopScreen(doc)).toBe(false);          // this one DID resolve
+    expect(out.nodes[1].config.body.nodes[1]).toBe(doc.nodes[1].config.body.nodes[1]);
+    expect(out.nodes[1].config.body.nodes[2]).toBe(doc.nodes[1].config.body.nodes[2]);
+
+    // A whole branch under which nothing resolved keeps its own reference,
+    // including its `nodes` array.
+    expect(out.nodes[2].config.branches[0]).toBe(doc.nodes[2].config.branches[0]);
+    expect(out.nodes[2].config.branches[0].nodes).toBe(doc.nodes[2].config.branches[0].nodes);
+
+    // The input document is never mutated.
+    expect(doc.nodes[1].config.body.nodes[0].config.title).toBe('In Loop');
+  });
+
+  it('returns the SAME flow reference when nothing resolved, regions included', () => {
+    // The negative control for the identity discipline one level up: a flow
+    // the bundle does not carry must come back untouched by reference, or
+    // `nodesChanged` starts reporting untouched documents as changed.
+    const doc = { ...regionFlow(), name: 'no_such_flow' };
+    expect(translateFlow(doc, bundle, { locale: 'zh-CN' })).toBe(doc);
+  });
+
+  it('leaves a region-shaped value that is not a region alone (`config` is an open record)', () => {
+    // `body` is also an ordinary key elsewhere — an `http` node's request
+    // payload. The descent checks the SHAPE (`{ nodes: [...] }`), never the
+    // key name alone, so a `loop` whose `body` is a string is passed through.
+    const doc: any = {
+      name: 'nested_regions',
+      label: 'Nested Regions',
+      nodes: [{ id: 'the_loop', type: 'loop', label: 'Loop', config: { collection: '{i}', body: 'not a region' } }],
+    };
+    const out: any = translateFlow(doc, bundle, { locale: 'zh-CN' });
+    expect(out.nodes[0]).toBe(doc.nodes[0]);
+    expect(out.label).toBe('嵌套区域');
+  });
+});
+
 describe('resolveFlowScreenTitle (#11287)', () => {
   const bundle: FlowTestBundle = {
     'zh-CN': {

packages/spec/src/system/i18n-resolver.ts

@@ -55,6 +55,8 @@
  * here.
  */
 
+import { mapFlowNodeList } from '../conversions/walk.js';
+
 import type { TranslationBundle, TranslationData } from './translation.zod';
 
 /**
@@ -3478,6 +3480,27 @@ export function resolveFlowScreenTitle(
  * `help`) is ignored, never overlaid (the negative `translatePage` pins for
  * the retired `submitLabel`).
  *
+ * **Every screen node, at any DEPTH.** `FlowNode.config` carries ADR-0031
+ * regions — `loop.config.body`, `parallel.config.branches[]`,
+ * `try_catch.config.try`/`.catch` — each holding a full `nodes` array, 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
+ * `flows.<name>.screens.<node_id>` is authored for it, parses (the bundle
+ * schema is keyed by node id and knows nothing about depth) and must be
+ * overlaid. A flat `flow.nodes.map(…)` walked straight past those, accepting
+ * the translation and silently never applying it — the fourth pass in this
+ * repo to be written against the flat one-liner. The descent is
+ * {@link mapFlowNodeList}'s, which reads `FLOW_REGION_SLOTS_BY_TYPE`: WHERE a
+ * region lives is declared once (`automation/region-slots.ts`) and walked by
+ * one function per unit, so this resolver is not a fifth hand-rolled reader of
+ * that table.
+ *
+ * That helper is also what preserves the identity discipline through the
+ * descent: it returns the SAME `nodes` array when no node under it resolved
+ * anything, and every container `config` on the way down is copied only when a
+ * descendant actually changed — so `nodesChanged` below stays a true reading
+ * and an untouched document still comes back as the same reference.
+ *
  * ⚠️ Deliberately NOT registered in {@link translateMetadataDocument}'s
  * dispatch table: that table reaches the REST metadata boundary by itself
  * (`TRANSLATABLE_METADATA_TYPES` drives `@objectstack/rest`, #3786), which
@@ -3497,14 +3520,14 @@ export function translateFlow<T extends FlowLike>(
 
   const label = lookupFlowLabel(bundle, name, opts);
 
-  let nodesChanged = false;
   const nodes = Array.isArray(flow.nodes)
-    ? flow.nodes.map((node) => {
-        const next = translateScreenNode(node, name, bundle, opts);
-        if (next !== node) nodesChanged = true;
-        return next;
-      })
+    ? mapFlowNodeList(
+        flow.nodes,
+        `flows.${name}.nodes`,
+        (node) => translateScreenNode(node as FlowNodeLike, name, bundle, opts) as Record<string, unknown>,
+      ) as FlowNodeLike[]
     : undefined;
+  const nodesChanged = nodes !== undefined && nodes !== flow.nodes;
 
   if (label === undefined && !nodesChanged) return flow;
   return {