refactor(spec): one union-branch selection policy, imported by both walks, with a shared-fixture parity pin (#8318) - #8659
Merged
Conversation
…alks (#8318) `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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01E5tUwGM3LQoqErTfkvRW7W
…on-branch-policy-extract
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
Contributor
📓 Docs Drift CheckThis PR changes 1 package(s): 106 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:
⛔ 7 release-owned page(s) also reference the affected code. These are read-only:
|
github-actions
Bot
added
documentation
This was referenced Aug 14, 2026
os-zhuang
marked this pull request as ready for review
August 14, 2026 12:40
os-zhuang
enabled auto-merge
August 14, 2026 12:40
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #8318
The union-branch selection policy had two sibling implementations inside
packages/spec. It now has one, plus the mechanical enforcement the module headers were standing in for.What moved
src/shared/error-map.zod.ts(the prose renderer, #4971/#5389) andsrc/api/zod-issues-to-fields.ts(the ADR-0114 D3 wire mapper, #8124) each declared the same policy:isKindMismatchOnly,carriesUnknownKey,selectUnionBranches,CONTAINER_ISSUE_CODES, the depth limit 3 and the branch cap 3. While the mapper lived in@objectstack/restthe fork was forced; #8124 removed that reason, leaving the two one directory apart with nothing but their headers binding them together.All of it now lives in
src/shared/union-branch-policy.ts, imported by both walks.⛔ Package-internal, as the card requires. The module is in no barrel, and no public export moves:
check:api-surfaceandcheck:export-originsare both green with a clean tree, andcheck:generatedreports all 13 artifacts up to date — zero ledger drift. A test in the parity file asserts directly that none of the seven policy symbols is reachable from thesharedorapientry points, so a future barrel line goes red before the ledger drift reaches a reviewer.What deliberately did NOT move
The two walks stay separate implementations. The renderer owns the indent, the glyph, the
(root)spelling and the trailing "... and N more branches rejected this value" line; the mapper owns the D3 code table and the{field, code, message}shape.The
{selected, omitted}wrinkle is handled, not papered over.selectUnionBranchesreturns both; the renderer printsomitted, and the mapper destructuresselectedalone at a commented line. The wire dropping the count is now a decision recorded in code rather than an absence you have to infer — and the parity test asserts the asymmetry in both directions (the prose line is present, nofields[]entry mentions it) instead of normalising it away silently.The parity test
src/shared/union-branch-policy.parity.test.ts. OnesafeParseper fixture feeds both walks, and their outputs are compared pair for pair after a normalisation that removes exactly three formatting facts (the indent, the glyph, the(root)spelling) and nothing else.The corpus covers every rule the card named: kind-mismatch drop, the all-kind-mismatch case, fewest-issues ranking, the
unrecognized_keystie-break, declaration-order determinism, the depth limit, the branch cap, and container descent for bothinvalid_keyandinvalid_element(#5389), plus a container nested under a union. A guard case asserts the corpus really produces unions, so the parity assertions cannot pass vacuously.It is deliberately end-to-end rather than a unit test of the extracted module: a unit test would pass trivially now that there is one implementation. What can still regress is a future author re-inlining a copy, and that is what this catches.
Reverse verification
Prediction stated first: re-forking the cap in the wire mapper should redden only the fixtures that select more than one branch, not the whole file.
Measured, on the committed tree, by slicing the mapper's selection to one branch:
declaration order breaks a full tie,the branch cap keeps three of five tied branches,both keep exactly the same three branches, cap included). Direction as predicted.src/api/zod-issues-to-fields.test.tsandsrc/shared/error-map.test.ts, 42 tests — stayed green under the same re-fork. That is the measurement that says this file is new coverage rather than a restatement of what was already pinned.Restored with
git restore --source=HEADand proven byte-identical viagit hash-objectagainstHEAD(232d5349c8…).Behaviour
Unchanged for every issue zod produces. The ranking, both limits and the container-code set are byte-identical to what each walk applied before, and the full spec suite passes unmodified.
One deliberate widening, stated plainly rather than buried: the shared policy reads a missing or non-array
pathas the root — the wire mapper's already-shipped normalisation, now applied to the renderer too. The renderer previously threw a TypeError on such an issue object, so nothing that used to produce output produces different output. No value satisfying the renderer's ownZodIssueMinimaltype (whosepathis a required array) is affected, and zod never emits one that is not.Verification
All at
6097b7846(the head this PR pushes; the union was run after the final commit).@objectstack/specsuite@objectstack/restsuite (the wire consumer)@objectstack/spectypecheckcheck:scripts-typecheck+check:test-typecheck(debt ledger unchanged)check:generatedcheck:dual-source-exports,check:exported-anycheck:doc-formula-expressions,check:error-code-casing,check:merge-driver,check:spec-parsed-alias,check:type-source-resolution,check:nul-bytes— all greendispatch-gates.mjscheck:objectui-changeset,check:query-options-erasure,check:type-check-coverage,check-changeset-no-major,check-empty-changeset— all greenTwo gates are red on pristine
origin/mainas well, verified in a comparison worktree at189a73225and already filed by others: #8654 (check-changeset-no-major --self-testcontrol) and #8658 (check-adr-0087-registrationinput assertion). Both are the post-release-cut drained-.changesetcondition, not this diff.check-dev-prereqsneeds the whole workspace built (43 of 67 packages unbuilt here) and is a property of the worktree, not of this change.Synced with
origin/mainat189a73225(clean merge, nopackages/specincoming).Generated by Claude Code