fix(types,cli): discriminate AnyComponentSchema on type so a refusal costs one arm, not 106 (#8498) - #8544
Merged
Merged
Conversation
…sal costs one arm `AnyComponentSchema` was a flat `z.union`, so a refusal carried every arm's issue list, and Zod's `$ZodError` initializer stringifies that whole tree into `.message` eagerly — in the constructor, not behind a getter. Measured on zod 4.4.3, a root refusal cost 14,624 chars and grew ~25x per level of nesting until `safeParse` threw `RangeError: Invalid string length` out of `safeValidateSchema`, which is documented as validating "without throwing errors". Discriminating on `type` selects one arm from the authored literal: same document, 164 chars. `ObjectQLComponentSchema` and `CRUDComponentSchema` are discriminated too because zod 4.4.3 refuses a plain `z.union` as a discriminated member. The accept set does not move: the 13 arms declare 107 `type` literals with zero collisions, and flat vs discriminated agree on all 440 example documents. `@object-ui/cli`'s `union-arm-diagnostics` learns the discriminated issue shape, so `objectui validate` still prints the 2026-09-02 maintainer ruling's output — the selected arm alone, or a note plus a capped candidate list. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CZY49skxUBYyJcdnTcYPrE
Contributor
❌ Console Performance Budget
The eager closure is every chunk the entry reaches through static imports — what the browser fetches and parses before the app renders. The entry chunk on its own is a small fraction of it. Which half objected:
📦 Bundle Size Report
Size Limits
|
This was referenced Sep 8, 2026
Collaborator
|
| tree | framework gzip |
vs PER_CHUNK_GZIP_CEILINGS.framework = 71_000 |
|---|---|---|
merge base 3bc187b7f, before any change of this PR's |
72,245 | over by 1,245 |
| this branch | 72,248 | over by 1,248 |
⇒ this PR contributes +3 gzipped bytes and its aggregate eager closure DROPS 28. The breach is 415× its contribution and predates it. Independently: Bundle Analysis has failed on every main push since f76f43628 (05:44:56Z), four consecutive merges, last green 40a7c538a.
Filed, twice and by two seats that did not see each other: #8541 (domain:devx, 08:22:50Z, first) and #8542 (this seat, minutes later). ⛔ Neither closed — the merge is triage's. ⭐ Read #8541 first; it is the earlier card and it makes the sharper point that this is not a required check, so nothing is mechanically blocked.
No fix exists to port. The four candidate remedies — raise the ceiling, trim #8512's diagnostic strings, re-chunk, or re-rule the per-chunk gate — are all maintainer-floor or another card's, and the attribution build that would choose between them has not been run by anyone yet.
⛔ What was NOT done
- ⛔ No re-run. A size verdict is a measurement, not a flake, and a re-run cannot change a red base.
- ⛔
PER_CHUNK_GZIP_CEILINGS/PER_CHUNK_BASELINEuntouched. Maintainer floor — the gate's own failure text says "do not widen it just to get a green check." - ⛔ Nothing trimmed for bytes. This PR would have to remove ~1,245 bytes from
frameworkto go green — 415× what it adds — which means gutting the repair for a defect it did not cause. - ⛔ No revert of fix(core): execute the declared
$operators in ValueDataSource, refuse the rest #8512 proposed. It closed a silent fail-open where an unrecognised operator matched every row.
⇒ This PR is parked on the base branch, in the same position as #8540 and #8501. It stays draft and ⛔ is not enqueued regardless: the pre-enqueue in-seat contract review at the contract-review tier is running, per SKILL.md:643.
Generated by Claude Code
… a non-object root its own message Two defects from the in-seat contract review of PR #8544. 1. `noMatchingDiscriminator` routed on `note`/`options`/empty-`errors` alone, so a discriminated union keyed on anything else was summarised as if it were keyed on `type`. `@objectstack/spec`'s `ViewDataSchema` is `discriminatedUnion('provider', ...)` and rides `.data` on `object-grid` and three siblings, so an authored `{ type: 'object-grid', data: { ... } }` printed a confident, ruling-shaped sentence naming the wrong key and offering provider names as component types. The merge base printed nothing there — silent, not wrong — so this was a silent-to-wrong regression. Guarded on both the path's last segment and the `discriminator` field: they establish different facts, and a shape that ever separates them now falls through to silent rather than to wrong. 2. The union's message override was unconditional, so it was scoped to the whole schema and also replaced its `invalid_type`: a non-object root lost "expected object, received number". Narrowed to `invalid_union`. Also corrects stale prose the discriminated root falsified, in the reader's header and in one figure in the new test's header. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CZY49skxUBYyJcdnTcYPrE
Contributor
❌ Console Performance Budget
The eager closure is every chunk the entry reaches through static imports — what the browser fetches and parses before the app renders. The entry chunk on its own is a small fraction of it. Which half objected:
📦 Bundle Size Report
Size Limits
|
…root falsified Comment-only, authorised as the single follow-up to the contract re-review PASS on 42344e9. No code changes. `validate.ts` still described `AnyComponentSchema` as a `z.union` whose every no-arm document yields one top-level issue at `path: []`, and still credited `explainUnionIssue` with selecting the arm. Both went false when the union began discriminating on `type`: zod does the selecting, a matched discriminator yields the arm's own issues as top-level entries at absolute paths, a missed one is judged at `['type']`, and a root path now means the document is not a component object at all. Also corrects a line citation in `union-arm-diagnostics.ts`: zod fills `discriminator` and `path` from the same `def.discriminator` at `schemas.js:1187` and `:1190`, not `:1189`/`:1191`. The cited fact was right, the line numbers were not. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CZY49skxUBYyJcdnTcYPrE
Contributor
❌ Console Performance Budget
The eager closure is every chunk the entry reaches through static imports — what the browser fetches and parses before the app renders. The entry chunk on its own is a small fraction of it. Which half objected:
📦 Bundle Size Report
Size Limits
|
Collaborator
Enqueued with landing check ② UNSATISFIED — recorded, ⛔ not silently passed
Why it cannot be cleared honestly. The three tells are the literal Why the direction is nonetheless established. Structurally a discriminated union tries exactly one arm where the flat union tried thirteen ⇒ its accept set is a subset; a flat→discriminated conversion cannot widen. And the in-seat contract review at Landing state: ① ✅ contract-tier PASS on Merging on the maintainer's instruction to land the finished work. ⛔ The gate defect is not closed by this merge — objectstack#16822 owns it. Generated by Claude Code |
os-sales
marked this pull request as ready for review
September 8, 2026 11:04
os-sales
pushed a commit
that referenced
this pull request
Sep 8, 2026
…ated union `main` landed the `AnyComponentSchema` discrimination (objectui#8498, PR #8544) on the same line this branch wraps, so the two changes conflicted textually. Resolved by COMPOSITION, not by choosing: the arm list is now `defineNodeComponentUnion(z.discriminatedUnion('type', [...]))`, keeping the discriminated fan-out AND the node option-slot fill. Both docblock halves kept. The mechanism is unaffected by construction and it was re-measured, not assumed: the option slot lives in `base.zod.ts`'s own plain `z.union(nodeUnionOptions)`, which `main` does not touch, and `defineNodeComponentUnion` writes whatever schema it is handed into slot 0. Build, all three type-check projects and the 148 `packages/types` test files (2805 tests, including the incoming `any-component-union-fanout` pins and this branch's `fill is LIVE` leg) are green on the composed tree. Also records, in `base.zod.ts`, the measured limit of its own claim: the "no published entry point can reach the pre-fill window" sentence is about module graphs, and a bundler honouring `"sideEffects": false` can drop the fill outright. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CZY49skxUBYyJcdnTcYPrE
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
1 participant
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 #8498
AnyComponentSchemawas a flatz.union. On a refusal Zod builds oneinvalid_unioncarrying every arm's issue list, and$ZodError's initializer stringifies that whole tree into.messageeagerly —zod/v4/core/errors.js:13, in the constructor, not behind a getter. So the cost was paid whether or not anyone read the message, and it compounded per level of nesting untilsafeParsethrewRangeError: Invalid string lengthout ofsafeValidateSchema, whichindex.zod.ts:414documents as validating "without throwing errors".Discriminating on
typeselects ONE arm from the authored literal, so a refusal carries that arm's diagnosis alone.The first spelling of the patch round's guard used
(issue.path ?? []).at(-1). Vitest transpiled it and the whole suite went green — 163 files, 3,047 tests, no signal.tscthen rejected it outright:error TS2339: Property 'at' does not exist on type 'readonly PropertyKey[]', under@object-ui/cli's lib target. A green test run alone would have shipped it.That is the second instance of this family on this lane in one day; PR #8501 hit the build-project-versus-test-project version of it hours earlier. One shape underneath both: the test runner and the type checker do not read the same rules, so a suite passing says nothing about whether the package compiles. The repo's own instruction — run the package's
type-checkscript separately, and note that it is spelled with a hyphen — is what caught it here, and it earned its keep twice today.The repair chosen, and the measurement that chose it
Option A in the card's alphabet (⚠️ Distinct from the 2026-09-02 maintainer ruling's own A/B/C/D, which is about what to print — nothing here contradicts that ruling.
z.discriminatedUnion), not B (error map).B was measured and does not do the job. Zod's
.messageisJSON.stringify(issues), and a message-returning error map leaves theerrorsarray the size comes from exactly as it was. Same document, same zod 4.4.3:.messageerrorsz.unionz.union+ error map returningInvalid inputerrorsarray" is not categorically true:finalizeIssue(zod/v4/core/util.js:557-565) hands the map the raw mutable issue before copying it, so a map that assignsiss.errors = []does shrink the message — 14,624 to 105, measured. That variant is still not the repair: it leans on undocumented ordering, it throws away the veryerrorsthe 2026-09-02 ruling requiresobjectui validateto print, and it still computes all 13 arm subtrees at every level, so the depth cliff survives it untouched. ⇒ the selection stands; only the premise sentence was wrong. (An earlier draft also quoted 14,638 for the mapped message; that number depends on the map's own string and did not reproduce — withInvalid inputit is 14,624.)Before / after, this branch vs its merge base
3bc187b7ftypeselects an armtypematches nothingThe cliff itself, with card objectui#8344's redirect applied as an uncommitted measurement control
Never committed, never pushed on this branch — both legs restored and verified by blob hash against
HEAD. The document is the card's own.RangeError: Invalid string lengthafter 98 sThe left column reproduces the card's table to the character. Growth becomes additive (~+7,000 chars per level) instead of ~25x per level; at depth 3 the message is 30,120x smaller.
The accept set does not move
Two independent readings:
typeliterals with zero collisions, so the arm a literal selects was already the only arm that could have accepted a document carrying it.examples/, flat and discriminated agree on every verdict (393 accepted by both, 47 refused by both). Firing control: a deliberately crippled union differs on 273 of the same 440, so the comparison can see a difference when there is one.packages/types/src/__tests__/zod-mirror-parity.test.tsis untouched and green (31 tests) — a discriminated union over the same arms infers the same output type, so no ledger row moves.Why three files changed, not one
Zod 4.4.3 refuses a plain
z.unionas a member of a discriminated union — it computes nopropValues, so it declares no literals to dispatch on (Invalid discriminated union option at index "9").ObjectQLComponentSchemaandCRUDComponentSchemawere the last two flat sites, so the root could not discriminate until they did.crud.zod.tscarries one type assertion.ActionSchema'sz.ZodType[ActionDeclaration, ActionDeclaration]annotation is a maintainer ruling (objectui#7760) and does not move; that type declarespropValuesas possibly-undefined, sotsccannot see through thez.lazyto thetype: z.literal('action')the body really declares. The runtime can — measured,Set { 'action' }. The intersection asserts that one fact, leaves the output type intact, and the fact is pinned at runtime so the cast cannot rot into a lie.util.cached, at FIRST PARSE. A rotted member constructs fine and then throwsInvalid discriminated union optionout ofsafeValidateSchemaon a GREEN document. So the failure mode is a throw, which is worth naming on a card that exists to stop one; the runtime pin and every parse in CI catch it first. (Angle brackets spelled as square ones above: GitHub deletes tag-shaped fragments from a body, reversed brackets and all.)The
packages/clihalfPer the triage routing, it rides this PR. But not for the reason triage gave — its claim that
union-arm-diagnostics.tsrests onerrors[i]aligning with arm i is contradicted by the file's own:36("Selection here never relies on that alignment"), and that line is correct. The real cost is different and smaller: a discriminated root produces noerrorsarray at all, so the reader met an issue shape it had no branch for and silently printed nothing where the ruling requires a note.The module learns that shape — one guarded predicate plus a note builder shared with the existing branch — and header fact 2's "two shapes, and they are the only two" is restated as three, because it was a totality claim and it went false.
objectui validateoutput, unchanged in substance:Two output shapes DO move, and both test files were opened and restated by hand rather than patched:
typeselects an arm no longer produces a rootinvalid_union— the arm's own issues are the top-level ones, already at absolute paths;typeno arm claims is reported attyperather than at(root). The root-path line this file exists for is re-anchored on a document that is not a component object at all, where the path genuinely is the root.Tests, and why they are not vacuous
The trap this card is most exposed to:
AnyComponentSchemadoes not yet recurse into child slots (objectui#7869 / objectui#8344), so a nested document is simply ACCEPTED onmainand a naive "does not throw at depth 4" test would be green for the wrong reason. So the depth case is built onMenuItemSchema, which already refuses at depth on the merge base.Proven by ablation, not asserted: the three schema conversions were reverted to the merge base in place, the suites re-run, and the tree restored — restore verified by empty
git diff HEADplus blob-hash equality on all three files.Against the flat union, 5 of the 7 new
packages/typescases go red:The 2 that stay green are the two firing controls, as they should. 3 of the restated
packages/clicases also go red on the merge base.Note the assertion in the middle: it counts the per-arm subtrees directly. That is the mechanism, not a symptom — an array of 13, stringified once per level, IS the exponent.
Patch round — the two defects the in-seat contract review found (
42344e912)The review returned FAIL on two one-line defects and held on everything else; the verdict is on the card at comment 5582392999.
① The no-arm note is for
type-keyed unions only.noMatchingDiscriminatorrouted onnote/options/ emptyerrorsalone and never checked which key the union dispatches on — so a discriminated union keyed on anything else was summarised as if it were keyed ontype. There is a reachable one:@objectstack/spec'sViewDataSchemaisdiscriminatedUnion('provider', …)and rides.dataonobject-grid,object-form,object-viewandobject-calendar. An authored{ type: 'object-grid', objectName: 'x', data: { type: 'rest' } }printed— the wrong key, and
ViewDataSchema's provider names offered as component types. The merge base printed nothing for that node, so this was a silent-to-wrong regression, and no gate on the PR could have seen it.Guarded on both the path's last segment and the
discriminatorfield, deliberately. They establish different facts and neither implies the other: the path ending attypeis what makespath.slice(0, -1)the node, and the discriminator beingtypeis what makes the note's wording andauthoredTypeAtcorrect. Zod 4.4.3 fills both from one source (def.discriminator,schemas.js:1189/1191) so they cannot disagree today — written this way, a shape that ever separates them falls through to silent, never to wrong. That is the direction the guard exists for. Output now:② The message override was scoped to the whole schema.
{ error: () => 'Invalid input' }also replaced this union'sinvalid_type, so a non-object root lost zod's own detail:42said onlyInvalid inputinstead ofInvalid input: expected object, received number. Narrowed toerror: (issue) => issue.code === 'invalid_union' ? 'Invalid input' : undefined, which declines to the locale for everything else. The re-anchored1. Invalid inputpin still passes — verified rather than assumed, since that pin is exactly the one that would keep passing either way — and it now carries the type detail with it.Three guard tests, all three proved non-vacuous by removing both fixes in place and re-running: end-to-end
objectui validateon the realobject-griddocument (expected … not to contain 'No arm accepts type'), a unit case covering all three ways the two conditions can disagree plus the positive control that they still route atype-keyed union, and the non-object-root message pin (expected 'Invalid input' to contain 'expected object, received number'). Exactly those 3 go red, 19 others stay green; restore verified by emptygit diff HEADand blob-hash equality.Stale prose the discriminated root falsified is corrected in⚠️
union-arm-diagnostics.ts's header and in one figure in the new test's header (the unknown-typebase reading is 14,855, not 14,624 — this body always had it right).validate.ts:112-114carries the same falsified sentence and is not corrected here: it lies outside the scope this patch round was confined to. Flagged for the next round rather than widened into silently.Console Performance Budgetis red onmain, and this PR cannot make it greenMeasured with a real
apps/consolebuild on both trees:frameworkgzipPER_CHUNK_GZIP_CEILINGS.framework = 71_0003bc187b7f, no change of mineThis PR's own cost is +3 gzipped bytes (+8 raw); the aggregate eager closure actually drops 28 bytes. The breach predates the card. Independent confirmation: the
Bundle Analysisworkflow has been failing onmainsincef76f43628— three consecutive pushes,40a7c538abeing the last green.⇒ the "≈1 byte of headroom / 70,999" figure both earlier comments carry is stale, and designing against it was never possible.
PER_CHUNK_GZIP_CEILINGSandPER_CHUNK_BASELINEare maintainer-floor and are not touched here; that ceiling decision is already with the maintainer.Gates
Every exit code captured by redirect before any pipe; the gate's own verdict line is quoted where it prints one.
pnpm exec vitest run packages/types/ packages/cli/pnpm --filter @object-ui/types run type-check·@object-ui/clipnpm type-check(whole repo)pnpm lint(whole repo)pnpm turbo run build --filter @object-ui/consolecheck-control-bytes·check-shell-escape-residuecheck-changeset-no-major·-fixed·-presence·-overwritecheck-vi-mock-specifiers·check-vi-mock-inheritcheck:spec-symbols·check:unreferenced-sources·check:handler-key-readscheck:self-import·check:phantom-deps·check:entry-guard·check:side-effects-arraycheck:esm-specifierscheck:node-esm-load --force-buildcheck:readme-exportscheck:eager-closuremain, see abovecheck-governed-queue-guard --testTwo gates first returned a PREREQUISITE-NOT-MET reading rather than a verdict, and are recorded because the distinction matters:
check:readme-exportsfailed only becausepackages/plugin-aihad never been built in this worktree, andcheck:node-esm-loadREFUSED two entries on provenance — turbo shares one cache across every worktree of a checkout, so it had replayed artifacts built in a sibling agent's tree. Both green once the prerequisite was met, the second exactly as its own output instructed.Serial order
This PR carries none of PR #8501's redirect. That branch is parked on this card by the seat ruling at issue objectui#8344 comment 5578545366; it rebases after this lands. Checked while measuring the control above: its
defineNodeComponentUnionwrites intoSchemaNodeSchema's own plainz.unionoption array, not intoAnyComponentSchema, so its by-reference mechanism is untouched by this change — both legs of the control built and ran clean.Draft, and enqueued by nobody here:
*.zod.tscontract schemas put this on the contract path limb, so the pre-enqueue in-seat contract review is the PM's step.Net added lines: 469 (536 added, 67 deleted) across source, tests and changeset, against a ceiling the PM raised to 470 on the reasoning that a line budget guards against scope creep and not against work a contract review mandated. The first round landed at 390; the patch round cost 71 net (two guarded conditions, three guard tests, the prose corrections), and the comment-only follow-up costs 8 more. ⛔ Nothing was trimmed to fit and the ceiling was never moved by this branch.
Session:
session_01CZY49skxUBYyJcdnTcYPrE.Generated by Claude Code