Skip to content

refactor(spec): one union-branch selection policy, imported by both walks, with a shared-fixture parity pin (#8318)#8659

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
os-zhuang merged 3 commits into from
Aug 14, 2026
Merged

refactor(spec): one union-branch selection policy, imported by both walks, with a shared-fixture parity pin (#8318) #8659

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
5d1d6ca
refactor(spec): one union-branch selection policy, imported by both w…
claude Aug 14, 2026
6097b78
Merge remote-tracking branch 'origin/main' into claude/issue-8318-uni…
claude Aug 14, 2026
b1b7178
merge origin/main (gate repair + memory placeholder refusal absorbed)
claude Aug 14, 2026
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
47 changes: 47 additions & 0 deletions .changeset/union-branch-policy-one-implementation.md
View file
Open in desktop
Original file line numberDiff line numberDiff line change
@@ -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.
134 changes: 28 additions & 106 deletions packages/spec/src/api/zod-issues-to-fields.ts
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -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).
Expand DownExpand Up@@ -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<string> = new Set(['invalid_key', 'invalid_element']);

/** A Zod issue path, normalised to the array Zod always produces. */
function issuePathOf(issue: any): Array<string | number> {
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`.
*
Expand DownExpand Up@@ -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,
Expand DownExpand Up@@ -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);
}
Expand Down
129 changes: 15 additions & 114 deletions packages/spec/src/shared/error-map.zod.ts
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -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.
Expand DownExpand Up@@ -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<string> = 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)';
Expand All@@ -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
Expand Down
Loading
Loading