From 5d1d6caf8a7ff124cf1f6682a6d441c5f520f137 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 14 Aug 2026 08:44:14 +0000 Subject: [PATCH] refactor(spec): one union-branch selection policy, imported by both walks (#8318) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `shared/error-map.zod.ts` (the prose renderer) and `api/zod-issues-to-fields.ts` (the ADR-0114 D3 wire mapper) carried the same selection policy as two separate implementations: kind-mismatch drop, fewest-issues ranking, `unrecognized_keys` tie-break, declaration-order determinism, depth limit 3, branch cap 3, and the `invalid_key` / `invalid_element` container codes. #8124 moved the mapper into this package, so the historical reason for the fork is gone. The policy now lives in `src/shared/union-branch-policy.ts`, package-internal and absent from every barrel — no public export moves, and `api-surface/` / `export-origins/` are untouched. The two walks stay separate: the renderer owns the prose and the "... and N more branches" line, the mapper owns the D3 code table and the `{field, code, message}` shape. `selectUnionBranches` returns `{selected, omitted}`; the mapper destructures `selected` alone at a commented line, making the wire's omission a recorded decision rather than an absence. `src/shared/union-branch-policy.parity.test.ts` drives both walks from one `safeParse` per fixture and compares their outputs pair for pair, over a corpus covering every rule of the policy plus container descent. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01E5tUwGM3LQoqErTfkvRW7W --- .../union-branch-policy-one-implementation.md | 47 +++ packages/spec/src/api/zod-issues-to-fields.ts | 134 ++------ packages/spec/src/shared/error-map.zod.ts | 129 +------- .../shared/union-branch-policy.parity.test.ts | 286 ++++++++++++++++++ .../spec/src/shared/union-branch-policy.ts | 221 ++++++++++++++ 5 files changed, 597 insertions(+), 220 deletions(-) create mode 100644 .changeset/union-branch-policy-one-implementation.md create mode 100644 packages/spec/src/shared/union-branch-policy.parity.test.ts create mode 100644 packages/spec/src/shared/union-branch-policy.ts diff --git a/.changeset/union-branch-policy-one-implementation.md b/.changeset/union-branch-policy-one-implementation.md new file mode 100644 index 0000000000..3913a48a9a --- /dev/null +++ b/.changeset/union-branch-policy-one-implementation.md @@ -0,0 +1,47 @@ +--- +"@objectstack/spec": patch +--- + +refactor(spec): the union-branch selection policy has ONE implementation, and a parity test that keeps it that way (#8318) + +`shared/error-map.zod.ts` (the prose renderer, #4971/#5389) and +`api/zod-issues-to-fields.ts` (the ADR-0114 D3 wire mapper, #8124) carried the +SAME union-branch selection policy as two separate implementations — +kind-mismatch drop, fewest-issues ranking, `unrecognized_keys` tie-break, +declaration-order determinism, depth limit 3, branch cap 3, and the +`invalid_key` / `invalid_element` container codes. While the mapper still lived +in `@objectstack/rest` the duplication was forced; #8124 moved it into this +package, so the two sat one directory apart with their module headers — and +nothing mechanical — asking whoever edits one to edit the other. + +The policy now lives in one package-internal module, +`src/shared/union-branch-policy.ts`, which both walks import. It is deliberately +NOT a public export: it is absent from every barrel, and `api-surface/` and +`export-origins/` do not move. + +The two WALKS stay separate implementations, as they should — one renders +indented `✗ path: message` prose for a terminal, the other produces +`{field, code, message}` entries for a JSON envelope, and only the renderer +emits the trailing "… and N more branches rejected this value" line. That +asymmetry is now explicit rather than implicit: `selectUnionBranches` returns +`{selected, omitted}`, the renderer prints `omitted`, and the mapper +destructures `selected` alone at a commented line, because a `fields[]` entry +must name a real field and carry a catalog code and an omission count has +neither. + +`src/shared/union-branch-policy.parity.test.ts` is the enforcement the module +headers lacked: one `safeParse` per fixture feeds BOTH walks, and their outputs +are compared pair for pair after a normalisation that removes the indent, the +`✗` glyph and the `(root)` spelling — nothing else. The corpus covers every rule +of the policy (kind-mismatch drop, all-kind-mismatch, fewest-issues ranking, the +`unrecognized_keys` tie-break, declaration-order determinism, the depth limit, +the branch cap, and container descent for both `invalid_key` and +`invalid_element`), and the one deliberate asymmetry is asserted rather than +normalised away. + +Behaviour is unchanged for every issue zod produces: the ranking, both limits +and the container-code set are byte-identical to what each walk applied before. +The single deliberate widening is that the shared policy reads a missing or +non-array `path` as the root — the wire mapper's already-shipped normalisation, +now applied to the renderer too, which previously threw on such an issue object. +No value satisfying the renderer's own `ZodIssueMinimal` type is affected. diff --git a/packages/spec/src/api/zod-issues-to-fields.ts b/packages/spec/src/api/zod-issues-to-fields.ts index c2b798a7ad..232d5349c8 100644 --- a/packages/spec/src/api/zod-issues-to-fields.ts +++ b/packages/spec/src/api/zod-issues-to-fields.ts @@ -25,18 +25,31 @@ * * ## Relation to `shared/error-map.zod.ts` * - * The union-branch selection policy below (limits, ranking, container descent) - * is the one `formatZodIssue`'s STRING renderer applies (#4971/#5389, one - * directory over in `shared/error-map.zod.ts`). The two walks stay separate + * The union-branch selection policy (limits, ranking, container descent) is the + * one `formatZodIssue`'s STRING renderer applies (#4971/#5389, one directory + * over in `shared/error-map.zod.ts`). The two walks stay separate * implementations on purpose — one renders indented prose lines, this one * produces structured `{field, code, message}` entries — but the *verdict* * (which branches explain a failure) must match, or one mistake gets two * different prescriptions depending on whether the author published from the - * terminal or POSTed to the API (#5014). Change the policy in either file and - * the sibling moves in the same PR. + * terminal or POSTed to the API (#5014). + * + * [#8318] That agreement used to rest on two copies of the ranking and a + * module header asking whoever edits one to edit the other. It is now + * structural: both walks import `../shared/union-branch-policy.ts`, and + * `../shared/union-branch-policy.parity.test.ts` pins the two outputs over one + * fixture corpus. ⛔ Do not re-declare the ranking, the limits or + * `CONTAINER_ISSUE_CODES` in this file — what belongs here is the D3 code + * table and the `{field, code, message}` shape, not which branch explains a + * failure. */ import type { FieldErrorCode } from './errors.zod'; +import { + CONTAINER_ISSUE_CODES, + NESTED_EXPANSION_DEPTH_LIMIT, + selectUnionBranches, +} from '../shared/union-branch-policy'; /** * A Zod issue → the field-level catalog (ADR-0114 D3). @@ -116,105 +129,6 @@ function valueAtPath(input: unknown, path: unknown): unknown { return cur; } -/** - * How many levels of nested issues are expanded below a top-level issue, and - * how many equally-informative union branches are emitted at one level. - * - * Both bounds — and the whole selection policy below — are the ones - * `formatZodError` landed for the CLI/spec side of this defect (#4971, - * `shared/error-map.zod.ts`). See the module header for why the two walks are - * siblings that must agree rather than one shared implementation. - */ -const NESTED_EXPANSION_DEPTH_LIMIT = 3; -const UNION_BRANCH_EMIT_LIMIT = 3; - -/** - * [#5389] The issue codes that hang their real diagnosis on `issue.issues` - * rather than on `invalid_union`'s `issue.errors`. - * - * `invalid_key` is raised when `z.record(K, V)`'s KEY schema rejects a key (and - * by `z.map` for a non-`PropertyKey` key); `invalid_element` when `z.map`'s - * VALUE schema rejects the value under such a key. Both carry a bare wrapper - * message ("Invalid key in record") with everything the client needs one level - * down — the same defect as #5014, one property name over. Kept in step with - * `CONTAINER_ISSUE_CODES` in `shared/error-map.zod.ts`. - */ -const CONTAINER_ISSUE_CODES: ReadonlySet = new Set(['invalid_key', 'invalid_element']); - -/** A Zod issue path, normalised to the array Zod always produces. */ -function issuePathOf(issue: any): Array { - return Array.isArray(issue?.path) ? issue.path : []; -} - -/** - * True when a branch only complains that the value is the wrong *kind* at the - * branch root — `expected string, received object` for the string member of - * `z.union([z.string(), SomeObject])`. - * - * Such a branch carries no prescription: the author never intended it, and - * emitting it is the "N branches, N times the noise" failure. An empty branch - * (zod's "matched multiple" variant carries `errors: []`) counts as - * uninformative too — `every` on an empty list is `true`. - */ -function isKindMismatchOnly(issues: readonly any[]): boolean { - return issues.every( - (issue) => - issuePathOf(issue).length === 0 - && (issue?.code === 'invalid_type' || issue?.code === 'invalid_value'), - ); -} - -/** True when a branch carries the #4001 campaign's unknown-key prescription. */ -function carriesUnknownKey(issues: readonly any[]): boolean { - return issues.some((issue) => issue?.code === 'unrecognized_keys'); -} - -/** - * Pick the branch(es) of a failed union whose issues actually explain the - * failure. Ranking, in order (identical to `selectUnionBranches` in - * `shared/error-map.zod.ts`): - * - * 1. **Kind-mismatch-only branches are dropped entirely.** If *every* branch is - * one — a plain `z.union([z.string(), z.number()])` handed an object — - * nothing is selected and the union reports exactly what it always has. - * 2. **Fewest issues wins.** The branch the author was closest to hitting - * complains least, so "fewest" is what keeps ONE unknown key from arriving as - * N `fields[]` entries, one per branch. - * 3. **A branch carrying `unrecognized_keys` breaks a tie**, because that is - * where the curated prose lives. - * 4. Declaration order breaks what remains, so the wire is deterministic. - * - * Branches that tie at the top are all emitted (capped): when two shapes explain - * the failure equally well, privileging the first by accident of declaration - * order would be a lie about which shape was expected. - */ -function selectUnionBranches(branches: readonly (readonly any[])[]): readonly (readonly any[])[] { - const informative = branches - .map((issues, index) => ({ issues, index })) - .filter((branch) => !isKindMismatchOnly(branch.issues)); - if (informative.length === 0) return []; - - const rank = (branch: { issues: readonly any[] }): [number, number] => [ - branch.issues.length, - carriesUnknownKey(branch.issues) ? 0 : 1, - ]; - - const sorted = [...informative].sort((a, b) => { - const [aCount, aKeys] = rank(a); - const [bCount, bKeys] = rank(b); - return aCount - bCount || aKeys - bKeys || a.index - b.index; - }); - - const [bestCount, bestKeys] = rank(sorted[0]!); - return sorted - .filter((branch) => { - const [count, keys] = rank(branch); - return count === bestCount && keys === bestKeys; - }) - .slice(0, UNION_BRANCH_EMIT_LIMIT) - .map((branch) => branch.issues); -} - /** * One issue → its `fields[]` entries, appended to `out`. * @@ -246,7 +160,11 @@ function selectUnionBranches(branches: readonly (readonly any[])[]): readonly (r * Deliberate divergence from the spec-side renderer: where it prints a trailing * "… and N more branches rejected this value", this emits nothing. That line is * a rendering affordance; a `fields[]` entry must name a real field and carry a - * catalog code, and the omission note has neither. + * catalog code, and the omission note has neither. [#8318] Since the shared + * policy computes the number either way, the divergence is now VISIBLE rather + * than implicit: `selectUnionBranches` hands back `{selected, omitted}` and + * this walk destructures `selected` alone, at the one line below — the wire + * dropping `omitted` is a decision recorded in code, not an absence. */ function collectIssueFields( issue: any, @@ -289,7 +207,11 @@ function collectIssueFields( if (!expandable) return; if (branches.length > 0) { - for (const branch of selectUnionBranches(branches)) { + // `omitted` is deliberately unread here — see the note above. The + // renderer turns it into a trailing prose line; the wire has nowhere to + // put a count that names no field. + const { selected } = selectUnionBranches(branches); + for (const branch of selected) { for (const nested of branch) { collectIssueFields(nested, path, depth + 1, seen, input, inputProvided, out); } diff --git a/packages/spec/src/shared/error-map.zod.ts b/packages/spec/src/shared/error-map.zod.ts index 7ca0657306..8410fd41ad 100644 --- a/packages/spec/src/shared/error-map.zod.ts +++ b/packages/spec/src/shared/error-map.zod.ts @@ -3,6 +3,11 @@ import { z } from 'zod'; import { suggestFieldType, formatSuggestion, findClosestMatches } from './suggestions.zod'; import { FieldType } from '../data/field.zod'; +import { + CONTAINER_ISSUE_CODES, + NESTED_EXPANSION_DEPTH_LIMIT, + selectUnionBranches, +} from './union-branch-policy'; /** * Zod v4 raw issue type used by the error map. @@ -155,123 +160,9 @@ interface ZodIssueMinimal { issues?: readonly ZodIssueMinimal[]; } -/** - * [#5389] The issue codes that hang their real diagnosis on `issue.issues`. - * - * Zod raises these when a CONTAINER's inner schema rejects a key or an element - * that cannot be addressed by a path segment: - * - * - `invalid_key` — `z.record(K, V)`'s **key** schema rejected a key, and - * `z.map(K, V)`'s key schema rejected a non-`PropertyKey` key; - * - `invalid_element` — `z.map(K, V)`'s **value** schema rejected the value - * under a non-`PropertyKey` key. - * - * In both cases the issue's own `message` is a bare wrapper ("Invalid key in - * record") and everything the author needs sits one level down, exactly as - * `invalid_union` hides a branch's prescription in `errors` (#4971). Zod's own - * `treeifyError` / `formatError` descend both codes with `[...path, - * ...issue.path]` as the parent path; this renderer did not, which is the - * defect #5389 records. - */ -const CONTAINER_ISSUE_CODES: ReadonlySet = new Set(['invalid_key', 'invalid_element']); - /** One indent step of a formatted issue line. */ const ISSUE_INDENT = ' '; -/** - * How many levels of nested issues are expanded below a top-level issue — - * `invalid_union` branches and, since #5389, `invalid_key` / `invalid_element` - * container issues alike. Both nest (a union member that is itself a union — - * `StateMachine → on.GO → actions[0]` is two levels in this repo today; a - * record whose value schema is a union is another), and a union level can - * render several branches, so the expansion is bounded rather than left to the - * shape of whatever the author typed. - */ -const NESTED_EXPANSION_DEPTH_LIMIT = 3; - -/** How many equally-informative branches are rendered at one level. */ -const UNION_BRANCH_RENDER_LIMIT = 3; - -/** - * True when a branch only complains that the value is the wrong *kind* at the - * branch root — `expected string, received object` for the string member of - * `z.union([z.string(), SomeObject])`. - * - * Such a branch carries no prescription: the author never intended it, and - * printing it is the "N branches, N times the noise" failure that made - * `view.zod.ts`'s `submitBehavior` reach for `discriminatedUnion`. An empty - * branch (the `invalid_union` "matched multiple" variant carries `errors: []`) - * counts as uninformative too — `every` on an empty list is `true`. - */ -function isKindMismatchOnly(issues: readonly ZodIssueMinimal[]): boolean { - return issues.every( - (issue) => - issue.path.length === 0 && - (issue.code === 'invalid_type' || issue.code === 'invalid_value'), - ); -} - -/** True when a branch carries the #4001 campaign's unknown-key prescription. */ -function carriesUnknownKey(issues: readonly ZodIssueMinimal[]): boolean { - return issues.some((issue) => issue.code === 'unrecognized_keys'); -} - -/** - * Pick the branch(es) of a failed union whose issues actually explain the - * failure. - * - * Ranking, in order: - * - * 1. **Kind-mismatch-only branches are dropped entirely** (see - * {@link isKindMismatchOnly}). If *every* branch is one — a plain - * `z.union([z.string(), z.number()])` handed an object — nothing is - * selected and the union renders exactly as it always has. - * 2. **Fewest issues wins.** The branch the author was closest to hitting - * complains least: given `z.union([A, B, C])` of strict objects and one - * mistyped key, the intended member reports *only* that key while the other - * two also report a wrong discriminator and their own missing requireds. So - * "fewest" is what keeps a single unknown key from being reported once per - * branch. - * 3. **A branch carrying `unrecognized_keys` breaks a tie**, because that is - * where the curated prose lives. - * 4. Declaration order breaks what remains, so the output is deterministic. - * - * Branches that tie at the top are *all* rendered (capped): when two shapes - * explain the failure equally well, privileging the first one by accident of - * declaration order would be a lie about which shape was expected. - */ -function selectUnionBranches( - branches: readonly (readonly ZodIssueMinimal[])[], -): { selected: readonly (readonly ZodIssueMinimal[])[]; omitted: number } { - const informative = branches - .map((issues, index) => ({ issues, index })) - .filter((branch) => !isKindMismatchOnly(branch.issues)); - - if (informative.length === 0) return { selected: [], omitted: 0 }; - - const rank = (branch: { issues: readonly ZodIssueMinimal[] }): [number, number] => [ - branch.issues.length, - carriesUnknownKey(branch.issues) ? 0 : 1, - ]; - - const sorted = [...informative].sort((a, b) => { - const [aCount, aKeys] = rank(a); - const [bCount, bKeys] = rank(b); - return aCount - bCount || aKeys - bKeys || a.index - b.index; - }); - - const [bestCount, bestKeys] = rank(sorted[0]!); - const tied = sorted.filter((branch) => { - const [count, keys] = rank(branch); - return count === bestCount && keys === bestKeys; - }); - - return { - selected: tied.slice(0, UNION_BRANCH_RENDER_LIMIT).map((branch) => branch.issues), - omitted: Math.max(0, tied.length - UNION_BRANCH_RENDER_LIMIT), - }; -} - /** Render a path array the way the CLI has always rendered it. */ function renderPath(path: PropertyKey[]): string { return path.length > 0 ? path.join('.') : '(root)'; @@ -289,6 +180,16 @@ function renderPath(path: PropertyKey[]): string { * key/element schema actually produced, so every one of them is rendered — * there is no branch to choose between and nothing to omit. * + * [#8318] The ranking, the two limits and {@link CONTAINER_ISSUE_CODES} are NOT + * declared here: they are the package-internal policy in + * `./union-branch-policy.ts`, which `../api/zod-issues-to-fields.ts` imports + * too. This walk owns the PROSE — the indent, the `✗` glyph, the `(root)` + * spelling and the trailing "… and N more branches" line the wire deliberately + * omits — and nothing about which branches explain the failure. ⛔ Do not + * re-declare the ranking here to tune the rendering; a verdict that differs + * between the terminal and the API is the defect #5014 named, and + * `union-branch-policy.parity.test.ts` will refuse it. + * * `seen` de-duplicates leaf lines *within one top-level issue*: two branches * that reject the same key with the same words say it once. Expanded lines * (union and container heads) are themselves never de-duplicated, since two diff --git a/packages/spec/src/shared/union-branch-policy.parity.test.ts b/packages/spec/src/shared/union-branch-policy.parity.test.ts new file mode 100644 index 0000000000..efa36e2865 --- /dev/null +++ b/packages/spec/src/shared/union-branch-policy.parity.test.ts @@ -0,0 +1,286 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#8318] The union-branch selection policy has ONE implementation, and the two + * walks that import it reach the SAME verdict on the same value. + * + * ## Why this file exists + * + * `shared/error-map.zod.ts` (the prose renderer, #4971/#5389) and + * `api/zod-issues-to-fields.ts` (the ADR-0114 D3 wire mapper, #8124) used to + * carry two copies of the ranking, the two limits and `CONTAINER_ISSUE_CODES`. + * Nothing mechanical held them together: each module header asked whoever edits + * one to edit the other in the same PR, and that was the whole enforcement. + * #8318 extracted the policy into `./union-branch-policy.ts`; this file is the + * enforcement the headers lacked. + * + * It is deliberately an END-TO-END comparison rather than a unit test of the + * extracted module. A unit test would pass trivially — there is one + * implementation now, so it agrees with itself. What can still regress is a + * future author re-inlining a copy into one walk to tune its output, which is + * exactly what the corpus below catches: both walks are driven from ONE + * `safeParse` per fixture, and their outputs are compared after a normalisation + * that removes formatting and nothing else. + * + * ## What "byte-identical" means here, precisely + * + * The two walks emit different things on purpose — indented `✗ path: message` + * prose vs `{field, code, message}` entries — so the comparable projection is + * the ordered list of `(path, message)` pairs each one visited. The + * normalisation below removes exactly three formatting facts and asserts the + * only deliberate asymmetry rather than hiding it: + * + * 1. the renderer's indent and `✗` glyph; + * 2. its `(root)` spelling of the empty path, which the wire writes as `''`; + * 3. its trailing "… and N more branches rejected this value" line, which the + * wire deliberately omits (a `fields[]` entry must name a real field and + * carry a catalog code; an omission count has neither). §4 pins that this + * line is present on the prose side and absent on the wire side for a capped + * fixture — the asymmetry is asserted, not normalised away silently. + * + * Everything else — which branches were selected, in which order, with which + * messages, to which depth — must match pair for pair. + */ + +import { describe, expect, it } from 'vitest'; +import { z } from 'zod'; +import { formatZodIssue } from './error-map.zod'; +import { zodIssuesToFields } from '../api/zod-issues-to-fields'; +import { + CONTAINER_ISSUE_CODES, + NESTED_EXPANSION_DEPTH_LIMIT, + UNION_BRANCH_SELECTION_LIMIT, + selectUnionBranches, +} from './union-branch-policy'; +import * as SharedBarrel from './index'; +import * as ApiBarrel from '../api/index'; + +/** `(path, message)` — the projection both walks can be read as producing. */ +type Visited = { path: string; message: string }; + +/** The prose renderer's output, normalised to {@link Visited} pairs. */ +function renderedPairs(issues: readonly unknown[]): Visited[] { + return issues + .flatMap((issue) => formatZodIssue(issue as never).split('\n')) + .filter((line) => line.includes('✗')) + .map((line) => { + const body = line.slice(line.indexOf('✗') + 1).trimStart(); + const split = body.indexOf(': '); + const path = body.slice(0, split); + return { + // `(root)` is the CLI's spelling of the empty path; the wire writes ''. + path: path === '(root)' ? '' : path, + message: body.slice(split + 2), + }; + }); +} + +/** The wire mapper's output, normalised to {@link Visited} pairs. */ +function wirePairs(issues: readonly unknown[], input: unknown): Visited[] { + return zodIssuesToFields(issues, input).map((entry) => ({ + field: entry.field, + message: entry.message, + })).map(({ field, message }) => ({ path: field, message })); +} + +/** Every renderer line, including the ones §4 asserts are prose-only. */ +function renderedLines(issues: readonly unknown[]): string[] { + return issues.flatMap((issue) => formatZodIssue(issue as never).split('\n')); +} + +/** Parse, assert the fixture really fails, and hand both walks one issue list. */ +function issuesFor(schema: { safeParse: (v: unknown) => any }, value: unknown): readonly unknown[] { + const result = schema.safeParse(value); + expect(result.success, 'every parity fixture must actually fail to parse').toBe(false); + return result.error.issues as readonly unknown[]; +} + +// --------------------------------------------------------------------------- +// The corpus. One fixture per rule of the policy, plus the container descent +// (#5389) both walks share and the junk shapes the wire has always tolerated. +// --------------------------------------------------------------------------- + +/** A strict object member requiring exactly one key. */ +const member = (key: string) => z.object({ [key]: z.string() }).strict(); + +/** A union nested `depth` levels deep, to drive the expansion limit. */ +function nestedUnion(depth: number): z.ZodTypeAny { + if (depth === 0) return z.object({ leaf: z.string() }).strict(); + return z.union([ + z.object({ next: nestedUnion(depth - 1) }).strict(), + z.object({ sibling: z.number() }).strict(), + ]); +} + +const FIXTURES: Array<[string, { safeParse: (v: unknown) => any }, unknown]> = [ + [ + 'kind-mismatch drop — the string member complains only about the kind, so it is not selected', + z.object({ u: z.union([z.string(), z.object({ k: z.string() }).strict()]) }), + { u: { kk: 1 } }, + ], + [ + 'all branches kind-mismatch — nothing is selected and the union stands alone', + z.object({ u: z.union([z.string(), z.number()]) }), + { u: {} }, + ], + [ + 'fewest issues wins — the near-miss member reports one key, the others report three', + z.object({ + u: z.union([ + z.object({ kind: z.literal('a'), one: z.string(), two: z.string(), three: z.string() }).strict(), + z.object({ kind: z.literal('b'), only: z.string() }).strict(), + ]), + }), + { u: { kind: 'b' } }, + ], + [ + 'unrecognized_keys breaks a tie at equal issue counts', + z.object({ + u: z.union([ + z.object({ needed: z.string() }).strict(), + z.object({}).strict(), + ]), + }), + { u: { surprise: 1 } }, + ], + [ + 'declaration order breaks a full tie — every tied branch is kept, in order', + z.object({ u: z.union([member('alpha'), member('beta')]) }), + { u: {} }, + ], + [ + 'the branch cap keeps three of five tied branches', + z.object({ + u: z.union([member('one'), member('two'), member('three'), member('four'), member('five')]), + }), + { u: {} }, + ], + [ + 'a union nested inside a union, expanded to the shared depth limit', + z.object({ u: nestedUnion(4) }), + { u: { next: { next: { next: { next: { wrong: 1 } } } } } }, + ], + [ + 'container descent — invalid_key on a constrained z.record key (#5389)', + z.object({ fields: z.record(z.string().regex(/^[a-z_]+$/, 'Must be snake_case.'), z.number()) }), + { fields: { 'First Name': 1 } }, + ], + [ + 'container descent — invalid_element on a z.map with a non-PropertyKey key (#5389)', + z.object({ m: z.map(z.object({ id: z.string() }), z.string()) }), + { m: new Map([[{ id: 'a' }, 42]]) }, + ], + [ + 'a union whose branch is itself a record with a bad key — container under union', + z.object({ + u: z.union([ + z.object({ fields: z.record(z.string().regex(/^[a-z_]+$/, 'Must be snake_case.'), z.number()) }).strict(), + z.object({ other: z.string() }).strict(), + ]), + }), + { u: { fields: { 'Bad Key': 1 } } }, + ], +]; + +describe('[#8318] the two walks reach the same union-branch verdict', () => { + for (const [name, schema, value] of FIXTURES) { + it(`agrees pair for pair — ${name}`, () => { + const issues = issuesFor(schema, value); + expect(wirePairs(issues, value)).toEqual(renderedPairs(issues)); + }); + + it(`…and is deterministic across runs — ${name}`, () => { + // Declaration order is the last tiebreak, so a second parse of the same + // value must select the same branches in the same order. A ranking that + // fell back on sort instability would show up here first. + const first = renderedPairs(issuesFor(schema, value)); + const second = renderedPairs(issuesFor(schema, value)); + expect(second).toEqual(first); + }); + } + + it('the corpus really exercises the union machinery, not just leaves', () => { + // A corpus that stopped producing unions would make every assertion above + // vacuously true — this is the guard against that silent failure. + const withUnions = FIXTURES.filter(([, schema, value]) => { + const issues = issuesFor(schema, value); + return issues.some((i: any) => i.code === 'invalid_union' + || JSON.stringify(i).includes('invalid_union')); + }); + expect(withUnions.length).toBeGreaterThanOrEqual(7); + }); +}); + +describe('[#8318] the ONE deliberate asymmetry, asserted rather than assumed', () => { + const [, cappedSchema, cappedValue] = FIXTURES.find( + ([name]) => name.startsWith('the branch cap'), + )!; + + it('the prose renderer says how many branches it dropped; the wire says nothing', () => { + const issues = issuesFor(cappedSchema, cappedValue); + + const omission = renderedLines(issues).filter((line) => line.includes('more branch')); + expect(omission).toHaveLength(1); + expect(omission[0]).toContain('… and 2 more branches rejected this value'); + + // The wire's entries name fields and carry catalog codes; no entry carries + // the count. Nothing in `fields[]` mentions the omission at all. + const entries = zodIssuesToFields(issues, cappedValue); + expect(entries.some((e) => e.message.includes('more branch'))).toBe(false); + }); + + it('both keep exactly the same three branches, cap included', () => { + const issues = issuesFor(cappedSchema, cappedValue); + // Five tied branches in, three out, two omitted — the numbers the renderer + // prints are the numbers the wire silently drops. + const union: any = (issues as any[]).find((i) => i.code === 'invalid_union') + ?? (issues as any[])[0]; + const { selected, omitted } = selectUnionBranches(union.errors as any[][]); + expect(selected).toHaveLength(UNION_BRANCH_SELECTION_LIMIT); + expect(omitted).toBe(2); + expect(wirePairs(issues, cappedValue)).toEqual(renderedPairs(issues)); + }); +}); + +describe('[#8318] the policy constants are the ones both walks were pinned on', () => { + it('depth limit 3, branch cap 3', () => { + expect(NESTED_EXPANSION_DEPTH_LIMIT).toBe(3); + expect(UNION_BRANCH_SELECTION_LIMIT).toBe(3); + }); + + it('the container codes are exactly invalid_key and invalid_element', () => { + expect([...CONTAINER_ISSUE_CODES].sort()).toEqual(['invalid_element', 'invalid_key']); + }); + + it('an empty branch list selects nothing and omits nothing', () => { + expect(selectUnionBranches([])).toEqual({ selected: [], omitted: 0 }); + }); +}); + +describe('[#8318] ⛔ the policy stays package-internal', () => { + // The #4001 pitfall the card names: this module is machinery two siblings + // need, not a contract anyone should author against. If a barrel ever + // re-exports it, `api-surface/` and `export-origins/` move with it and this + // goes red first — before the ledger drift reaches a reviewer. + const INTERNAL = [ + 'selectUnionBranches', + 'isKindMismatchOnly', + 'carriesUnknownKey', + 'unionIssuePath', + 'CONTAINER_ISSUE_CODES', + 'NESTED_EXPANSION_DEPTH_LIMIT', + 'UNION_BRANCH_SELECTION_LIMIT', + ]; + + it('no policy symbol reaches the `shared` entry point', () => { + for (const name of INTERNAL) { + expect(Object.keys(SharedBarrel), `${name} must not be public`).not.toContain(name); + } + }); + + it('no policy symbol reaches the `api` entry point either', () => { + for (const name of INTERNAL) { + expect(Object.keys(ApiBarrel), `${name} must not be public`).not.toContain(name); + } + }); +}); diff --git a/packages/spec/src/shared/union-branch-policy.ts b/packages/spec/src/shared/union-branch-policy.ts new file mode 100644 index 0000000000..53fc1a1164 --- /dev/null +++ b/packages/spec/src/shared/union-branch-policy.ts @@ -0,0 +1,221 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * The union-branch selection policy — ONE implementation, two walks (#8318). + * + * ## What lives here, and why it is here rather than in each walk + * + * When a `z.union` rejects a value, zod folds every branch's issues into ONE + * top-level `invalid_union` whose own message is the literal `"Invalid input"`. + * Something has to decide *which* branches actually explain the failure, or the + * author gets one mistake reported once per member ("N branches, N times the + * noise"). That decision — drop kind-mismatch-only branches, rank by fewest + * issues, break ties on `unrecognized_keys` then declaration order, cap the + * result — is a POLICY, not a rendering detail: the same body must get the same + * verdict whether the author published from the terminal or POSTed to the API + * (#5014). Its two consumers are one directory apart: + * + * - `../shared/error-map.zod.ts` — `formatZodIssue`'s prose renderer (#4971), + * indented `✗ path: message` lines for a terminal; + * - `../api/zod-issues-to-fields.ts` — the ADR-0114 D3 wire mapper (#8124), + * `{field, code, message}` entries for a JSON error envelope. + * + * Until #8318 each carried its own copy of the ranking, the two limits and + * {@link CONTAINER_ISSUE_CODES}, bound to move together by their module headers + * and by nothing mechanical. `packages/spec/src/shared/union-branch-policy.parity.test.ts` + * is that mechanical enforcement, and this module is what makes it structural: + * a verdict can no longer drift on one side, because there is only one side. + * + * ## What deliberately does NOT live here + * + * The two **walks** stay separate implementations. They differ in more than + * formatting — the renderer emits a trailing "… and N more branches rejected + * this value" line that the wire deliberately omits (a `fields[]` entry must + * name a real field and carry a catalog code; an omission note has neither), + * their de-duplication keys differ, and only the mapper maps codes to the + * ADR-0114 catalog. Merging them would trade a shared verdict for a shared + * output format, which is not what either consumer wants. + * + * ## ⛔ Package-internal — NOT a public export + * + * This module is reachable only from inside `@objectstack/spec` and is + * deliberately absent from `shared/index.ts` and from the root barrel: it is + * machinery two sibling modules need, not a contract anyone should author + * against (the #4001 pitfall — do not export internals only these modules + * need). `api-surface/` and `export-origins/` must not move for it. A third + * consumer OUTSIDE this package (`packages/metadata-protocol/src/protocol.ts` + * carries its own copy of the same ranking) is therefore not served by this + * module today; sharing with it is a public-surface decision, not a refactor. + */ + +/** + * As much of a zod issue as branch SELECTION reads. + * + * Both fields are `unknown` and optional on purpose: the wire mapper receives + * issue objects it has not type-checked (`zodIssuesToFields` takes `unknown` + * and tolerates junk by contract), while the renderer's own `ZodIssueMinimal` + * — `path: PropertyKey[]`, `code?: string` — is assignable to this, so both + * pass their real types through {@link selectUnionBranches} unchanged. + */ +export interface UnionBranchIssue { + readonly path?: unknown; + readonly code?: unknown; +} + +/** + * How many levels of nested issues are expanded below a top-level issue — + * `invalid_union` branches and, since #5389, `invalid_key` / `invalid_element` + * container issues alike. Both nest (a union member that is itself a union — + * `StateMachine → on.GO → actions[0]` is two levels in this repo today; a + * record whose value schema is a union is another), and a union level can + * render several branches, so the expansion is bounded rather than left to the + * shape of whatever the author typed. + * + * Read by both walks; it bounds the DESCENT, which is why it lives beside the + * selection rather than inside it. + */ +export const NESTED_EXPANSION_DEPTH_LIMIT = 3; + +/** + * How many equally-informative branches are kept at one level. + * + * The renderer prints them and then says how many it dropped; the wire emits + * them and says nothing. Both keep the same ones — that is the point. + */ +export const UNION_BRANCH_SELECTION_LIMIT = 3; + +/** + * [#5389] The issue codes that hang their real diagnosis on `issue.issues` + * rather than on `invalid_union`'s `issue.errors`. + * + * Zod raises these when a CONTAINER's inner schema rejects a key or an element + * that cannot be addressed by a path segment: + * + * - `invalid_key` — `z.record(K, V)`'s **key** schema rejected a key, and + * `z.map(K, V)`'s key schema rejected a non-`PropertyKey` key; + * - `invalid_element` — `z.map(K, V)`'s **value** schema rejected the value + * under a non-`PropertyKey` key. + * + * In both cases the issue's own `message` is a bare wrapper ("Invalid key in + * record") and everything the author needs sits one level down, exactly as + * `invalid_union` hides a branch's prescription in `errors` (#4971). + * + * ⚠️ These issues are NOT ranked. A union's branches are competing + * *candidates*, so they are selected between; a container has one inner schema, + * so every issue it produced is a true statement about the value and dropping + * any would be dropping a real diagnosis. The codes live here because both + * walks must descend the same set, not because the set feeds + * {@link selectUnionBranches}. + */ +export const CONTAINER_ISSUE_CODES: ReadonlySet = new Set([ + 'invalid_key', + 'invalid_element', +]); + +/** + * A zod issue path, normalised to the array zod always produces. + * + * The normalisation is the wire mapper's, adopted for both walks by #8318: it + * reads a missing or non-array `path` as the root rather than trusting a + * declared type its caller may not have honoured. For every input zod itself + * produces — and for every value satisfying the renderer's `ZodIssueMinimal`, + * whose `path` is a required array — this is byte-identical to reading + * `issue.path.length` directly. It differs only for issue objects that violate + * that type, where the renderer previously threw on the spread a few lines + * later; a shared policy cannot have two readings of "the branch root", and the + * tolerant one is the reading already shipped and pinned on the wire side. + */ +export function unionIssuePath(issue: UnionBranchIssue | undefined): Array { + return Array.isArray(issue?.path) ? (issue.path as Array) : []; +} + +/** + * True when a branch only complains that the value is the wrong *kind* at the + * branch root — `expected string, received object` for the string member of + * `z.union([z.string(), SomeObject])`. + * + * Such a branch carries no prescription: the author never intended it, and + * surfacing it is the "N branches, N times the noise" failure that made + * `view.zod.ts`'s `submitBehavior` reach for `discriminatedUnion`. An empty + * branch (the `invalid_union` "matched multiple" variant carries `errors: []`) + * counts as uninformative too — `every` on an empty list is `true`. + */ +export function isKindMismatchOnly(issues: readonly UnionBranchIssue[]): boolean { + return issues.every( + (issue) => + unionIssuePath(issue).length === 0 + && (issue?.code === 'invalid_type' || issue?.code === 'invalid_value'), + ); +} + +/** True when a branch carries the #4001 campaign's unknown-key prescription. */ +export function carriesUnknownKey(issues: readonly UnionBranchIssue[]): boolean { + return issues.some((issue) => issue?.code === 'unrecognized_keys'); +} + +/** + * Pick the branch(es) of a failed union whose issues actually explain the + * failure. + * + * Ranking, in order: + * + * 1. **Kind-mismatch-only branches are dropped entirely** (see + * {@link isKindMismatchOnly}). If *every* branch is one — a plain + * `z.union([z.string(), z.number()])` handed an object — nothing is + * selected and the union surfaces exactly as it always has. + * 2. **Fewest issues wins.** The branch the author was closest to hitting + * complains least: given `z.union([A, B, C])` of strict objects and one + * mistyped key, the intended member reports *only* that key while the other + * two also report a wrong discriminator and their own missing requireds. So + * "fewest" is what keeps a single unknown key from being reported once per + * branch. + * 3. **A branch carrying `unrecognized_keys` breaks a tie**, because that is + * where the curated prose lives. + * 4. Declaration order breaks what remains, so the output is deterministic. + * + * Branches that tie at the top are *all* selected (capped at + * {@link UNION_BRANCH_SELECTION_LIMIT}): when two shapes explain the failure + * equally well, privileging the first one by accident of declaration order + * would be a lie about which shape was expected. + * + * `omitted` is how many tied branches the cap dropped. The prose renderer turns + * it into a trailing "… and N more branches rejected this value" line; the wire + * mapper ignores it, because that line is a rendering affordance and a + * `fields[]` entry has nowhere to put it (ADR-0114: every entry names a real + * field and carries a catalog code). One selection, two dispositions of the + * same number — which is why it is returned rather than assumed. + * + * The element type is passed through, so each caller keeps its own issue type: + * the renderer gets `ZodIssueMinimal[]` back, the mapper gets its `any[]`. + */ +export function selectUnionBranches( + branches: readonly (readonly T[])[], +): { selected: readonly (readonly T[])[]; omitted: number } { + const informative = branches + .map((issues, index) => ({ issues, index })) + .filter((branch) => !isKindMismatchOnly(branch.issues)); + + if (informative.length === 0) return { selected: [], omitted: 0 }; + + const rank = (branch: { issues: readonly T[] }): [number, number] => [ + branch.issues.length, + carriesUnknownKey(branch.issues) ? 0 : 1, + ]; + + const sorted = [...informative].sort((a, b) => { + const [aCount, aKeys] = rank(a); + const [bCount, bKeys] = rank(b); + return aCount - bCount || aKeys - bKeys || a.index - b.index; + }); + + const [bestCount, bestKeys] = rank(sorted[0]!); + const tied = sorted.filter((branch) => { + const [count, keys] = rank(branch); + return count === bestCount && keys === bestKeys; + }); + + return { + selected: tied.slice(0, UNION_BRANCH_SELECTION_LIMIT).map((branch) => branch.issues), + omitted: Math.max(0, tied.length - UNION_BRANCH_SELECTION_LIMIT), + }; +}