Uh oh!
There was an error while loading. Please reload this page.
test(test-support): one enum-options walk for the top-level reader family (#6924) - #7024
Merged
Conversation
…mily (#6924) 17 hand-written `(Schema as { options?: readonly string[] }).options` casts across 16 spec-parity test files in 11 workspace packages read `.options` straight off a node that IS the enum. The cast is unchecked: an enum that acquires a wrapper, or a Zod build that moves `.options`, yields `undefined` and the derived vocabulary silently becomes the empty set. `@object-ui/test-support` already carried the wrapper walk, behind a signature this family could not use: `shapeEnumOptions(schema, key)` opens with `resolvePropsShape` and then indexes `shape[key]`, so a bare enum has no shape to resolve and no key to index. That is a missing ENTRY POINT, not a missing reader. So the walk is exported as `enumOptions(node)` and `shapeEnumOptions` delegates to it -- one walk, two entry points, no second copy. Verdict preservation measured, not assumed: against the installed pin, all 23 readings return the identical array in the identical order before and after. `palette-discussion-alias.test.tsx` gains the non-vacuity assertion the reader's docblock makes every caller owe; the other 15 files already carried one. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013hfmP9hoMd3dJwTh85J4yB
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. 📦 Bundle Size Report
Size Limits
|
Append-collision with PR #7022 (`refactor(test-support): one shared home for the Zod wrapper-key list`), which landed on main after this branch was cut and touches the same package barrel and README. Both sides kept, in two files: - `packages/test-support/src/index.ts` -- exports BOTH #7022's `ZOD_WRAPPER_KEYS` (with its docblock) and this branch's `enumOptions, shapeEnumOptions`. - `packages/test-support/README.md` -- this branch's rewritten `spec-enum-options.ts` entry (two exports, one walk) AND #7022's two `zod-wrapper-keys` entries, in main's ordering. One sentence of this branch's text was corrected rather than kept verbatim: it said the wrapper-key walk is "NOT confined here yet and still hand-copied", which #7022 made false. The merged sentence carries #7022's fact (the wrapper-key list is now shared as DATA) and this branch's remaining half (array-element unwrapping is still hand-copied). `pnpm-lock.yaml` auto-merged; `pnpm install` left it unchanged, so it needed no regeneration and none was hand-written. No behaviour from either side was dropped or altered. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013hfmP9hoMd3dJwTh85J4yB
ContributorAuthor
os-dev-report Conflict against Two files, both append-collisions with PR #7022, both sides kept.
export{enumOptions,shapeEnumOptions}from'./spec-enum-options';/* ...#7022's docblock... */export{ZOD_WRAPPER_KEYS}from'./zod-wrapper-keys';
One sentence was corrected rather than kept verbatim, and it is worth naming because it is the one place a "keep both sides" resolution would have produced a false statement. This branch's text said the other censused reader classes, "array-element unwrapping, the wrapper-key walk — are NOT confined here yet and are still hand-copied". #7022 made the wrapper-key half false. The merged sentence carries #7022's fact and this branch's surviving half:
Re-run on the merge commit (root-relative vitest only):
Diffs above were computed from the merge-base, never two-dot: Still draft; not marked ready, no auto-merge, not merged. No scope was added while in there — resolution only. Generated by Claude Code |
os-sam
marked this pull request as ready for review
August 31, 2026 14:14
os-sam
enabled auto-merge
August 31, 2026 14:14
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. 📦 Bundle Size Report
Size Limits
|
Uh oh!
There was an error while loading. Please reload this page.
os-sam pushed a commit
that referenced
this pull request
Aug 31, 2026
…the shared reader (#7025) 9 spec-parity test files in 7 packages cast an enum node NON-optionally to an options-bearing shape and read `.options` straight off it -- the SIXTH spelling of the Zod-internals reader hazard objectui#5872 catalogues, and the one objectui#6924's instrument (`options?:`) structurally cannot see. They converge onto `@object-ui/test-support`'s `enumOptions(node)`, the walk PR #7024 exported for objectui#6924's family. The decisive difference from that family is the FAILURE MODE, and it decides the shape of every conversion here. #6924's sites guarded with `Array.isArray(raw) ? [...raw] : []`, so a failed cast went quietly empty. These sites spread the cast result directly, so a failed cast THROWS. `enumOptions` deliberately answers `[]` rather than raising, so a bare conversion would have downgraded a loud throw into a silent empty vocabulary -- turning this population into #6924's, backwards. Every site therefore keeps its own non-vacuity duty: - 7 module-scope reads gain a throwing wrapper, modelled on the two sites that already solved this in PR #7024 (`types/spec-derived-unions.test.ts` and `examples/schema-catalog/.../component-fixture-declared-keys.test.ts`); - 2 in-test reads (`plugin-grid/spec-symbol-batch7`, `types/spec-subschema-parity`) already carried an explicit non-vacuity assertion, which is the sanctioned alternative, so they keep theirs. The four `expandableFamily.identity-*` files are textually identical across four packages -- #5872 class (1)'s sharpest shape, where a reviewer diffing one copy cannot see that the other three did not move -- and were converted as one unit. Verdict preservation measured, not assumed: all 9 readings return the identical array in the identical order before and after (FieldType n=49, ColumnSummarySchema n=11) against the installed pin. The retired spelling is deliberately not quoted in the new comments: the card's enumeration instrument is a grep for it, and a comment carrying the literal text would make every future re-derivation read a false positive. Out of scope, measured and left alone: `types/spec-subschema-parity.test.ts:161` casts to an `unwrap().def.options` shape -- a further spelling, and a #5872 class (2)/(4) shape rather than this one. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013hfmP9hoMd3dJwTh85J4yB
akarma-synetal pushed a commit
to akarma-synetal/objectui
that referenced
this pull request
Sep 1, 2026
…the shared reader (objectstack-ai#7025) (objectstack-ai#7057) 9 spec-parity test files in 7 packages cast an enum node NON-optionally to an options-bearing shape and read `.options` straight off it -- the SIXTH spelling of the Zod-internals reader hazard objectui#5872 catalogues, and the one objectui#6924's instrument (`options?:`) structurally cannot see. They converge onto `@object-ui/test-support`'s `enumOptions(node)`, the walk PR objectstack-ai#7024 exported for objectui#6924's family. The decisive difference from that family is the FAILURE MODE, and it decides the shape of every conversion here. objectstack-ai#6924's sites guarded with `Array.isArray(raw) ? [...raw] : []`, so a failed cast went quietly empty. These sites spread the cast result directly, so a failed cast THROWS. `enumOptions` deliberately answers `[]` rather than raising, so a bare conversion would have downgraded a loud throw into a silent empty vocabulary -- turning this population into objectstack-ai#6924's, backwards. Every site therefore keeps its own non-vacuity duty: - 7 module-scope reads gain a throwing wrapper, modelled on the two sites that already solved this in PR objectstack-ai#7024 (`types/spec-derived-unions.test.ts` and `examples/schema-catalog/.../component-fixture-declared-keys.test.ts`); - 2 in-test reads (`plugin-grid/spec-symbol-batch7`, `types/spec-subschema-parity`) already carried an explicit non-vacuity assertion, which is the sanctioned alternative, so they keep theirs. The four `expandableFamily.identity-*` files are textually identical across four packages -- objectstack-ai#5872 class (1)'s sharpest shape, where a reviewer diffing one copy cannot see that the other three did not move -- and were converted as one unit. Verdict preservation measured, not assumed: all 9 readings return the identical array in the identical order before and after (FieldType n=49, ColumnSummarySchema n=11) against the installed pin. The retired spelling is deliberately not quoted in the new comments: the card's enumeration instrument is a grep for it, and a comment carrying the literal text would make every future re-derivation read a false positive. Out of scope, measured and left alone: `types/spec-subschema-parity.test.ts:161` casts to an `unwrap().def.options` shape -- a further spelling, and a objectstack-ai#5872 class (2)/(4) shape rather than this one. Claude-Session: https://claude.ai/code/session_013hfmP9hoMd3dJwTh85J4yB Co-authored-by: Claude <noreply@anthropic.com>
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 freeto 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#6924
17 hand-written
(Schema as { options?: readonly string[] }).optionscasts across16 test files in 11 workspace packages read
.optionsstraight off a node thatIS the enum. The cast is unchecked: an enum that acquires a wrapper, or a Zod build
that moves
.options, yieldsundefined, the derived vocabulary silently becomesthe empty set, and every assertion built on it passes over nothing.
The shape, and why it is not a fifth approach
The triage ruling required reading #5872's current conclusion first and not
inventing a fifth way. #5872 class (1) landed a shared reader,
packages/test-support/src/spec-enum-options.ts. Reading it on disk: it alreadywalks the wrapper chain, already accepts
unwrap()/def.innerType/_def.innerType, and its loop checksnode.optionsbefore unwrapping — so itanswers a bare enum correctly the moment it is handed one.
The one thing it could not do for this family is reach the node: it opens with
resolvePropsShape(schema)and then indexesshape[key], so given a node that isalready the enum there is no shape to resolve and no key to index, and it
returns
[].That is a missing entry point, not a missing reader. So:
shapeEnumOptionsnow delegates — its body isresolvePropsShapeplus onecall. There is exactly one wrapper walk in the repository, and a new pin
(
the two entry points are one walk) fails if that stops being true; the wrapperfixtures alone would keep passing against a second copy.
The enumeration, re-derived
Instrument:
grep -rn 'options?: readonly string\[\]' --include='*.ts' --include='*.tsx'over the whole tree, not just
packages/, onorigin/main@501041f97.16 call-site files / 17 occurrences, plus the reader module itself (17 paths
matched in total).
The card said 15 files under
packages/. That count is right forpackages/;the 16th,
examples/schema-catalog/test/component-fixture-declared-keys.test.ts,is outside it and was missed by a
packages/-scoped grep.plugin-reportcarries two occurrences in one file, which is where the card's "2 sites" for that
package comes from.
Every one of the 16 is a test file. No production module reads the vocabulary
this way.
packages/fields' site isFieldEditWidget.test.ts:111, a differentfile from
FieldEditWidget.tsx.Non-vacuity: measured, and the card's expectation was wrong
The card says "nothing in this family is known to carry such an assertion — that is
the first thing a round on this card should measure". Measured: 16 of 17
occurrences already carried one, in three shapes.
examples/schema-catalog/.../component-fixture-declared-keys.test.tsnot an enum-bearing fieldapp-shell/.../color-variant-field.test.tsxit('reads a non-empty enum from the spec')app-shell/.../previews/__tests__/block-config.test.tsit('reads a non-empty enum from the spec')app-shell/.../previews/__tests__/palette-discussion-alias.test.tsxtoContaincomponents/__tests__/toaster-position-spec-parity.test.tsxit('reads a non-empty enum from the spec')components/notifications/__tests__/notification-surfaces.test.tsx.not.toEqual([])inside the coverage testfields/FieldEditWidget.test.tsit('reads a non-empty enum from the spec')plugin-charts/__tests__/chart-type-spec-parity.test.tsxit('reads a non-empty enum from the spec')plugin-dashboard/__tests__/DatasetWidget.colorVariant.test.tsxit('reads a non-empty enum from the spec')plugin-dashboard/__tests__/pivot-aggregation-spec-parity.test.tsxit('reads a non-empty enum from the spec')plugin-dashboard/__tests__/widget-dispatch-spec-parity.test.tsit('reads a non-empty enum from the spec')plugin-detail/__tests__/inlineEditTypeCoverage.test.tsxit('reads a non-empty enum from the spec')plugin-grid/__tests__/summary-spec-parity.test.tsit('reads a non-empty enum from the spec')plugin-report/__tests__/report-spec-parity.test.tsx(chart)it('reads a non-empty enum from the spec')plugin-report/__tests__/report-spec-parity.test.tsx(report)it('reads a non-empty enum from the spec')react/hooks/__tests__/animation-notification-spec-parity.test.tsxassertParitytypes/__tests__/spec-derived-unions.test.tsoptionsOfthrows on emptyThe one gap,
palette-discussion-alias.test.tsx, gains the named assertion here.It was not silent before — its
toContainprobes would red on an empty list —but they say "the spec dropped this member", which sends the next reader to the
contract instead of to the reader.
This is worth recording because it changes the card's risk framing without
changing its premise: the duplication was real, the silent-emptying was already
guarded almost everywhere.
Verdict preservation: measured, not assumed
A throwaway suite reproduced the exact hand-written reading each site carried on
501041f97and compared it toenumOptions(node)for the same node —23 readings, all IDENTICAL, same members, same order, all non-empty (so the
equality cannot pass by both sides being
[]). Examples:The measurement suite is deleted, not shipped — the same disposition #5872 class (1)
took. What ships instead is the permanent pin that the two entry points agree.
Ablation: an empty reader reds every converted site
Forced
enumOptionstoreturn []on top of the landed commit, then ran all 16converted files. Mutation proved on disk before the run (injected-marker count 1,
worktree blob hash moved off the HEAD blob); restore proved after (hash back to
the HEAD blob,
git diff HEADempty, marker count 0); the script carries atrap ... EXIT INT TERMrestore with an absolute repo path. No build leg is owed:@object-ui/test-supporthas nodist, and both itsexportsmap and the rootvitest alias point at
src/, so the mutated source IS what every suite loads.Result: 16 of 16 files fail loudly. 15 fail on assertions (41 failing tests);
component-fixture-declared-keys.test.tsfails at collection, because its readerthrows at module scope. Zero silently-green suites. This is the assertion the
card asked for: the family cannot go quietly permissive.
What changed, per site
Converted, all 16:
examples/schema-catalog/test/component-fixture-declared-keys.test.ts,app-shellcolor-variant-field / block-config / palette-discussion-alias,componentstoaster-position / notification-surfaces,fieldsFieldEditWidget,plugin-chartschart-type,plugin-dashboardDatasetWidget.colorVariant /pivot-aggregation / widget-dispatch,
plugin-detailinlineEditTypeCoverage,plugin-gridsummary,plugin-reportreport-spec-parity (both),reactanimation-notification,
typesspec-derived-unions.Exclusions: none. Every site the instrument found is converted.
Two sites keep a local wrapper around the shared reader rather than replacing
their behaviour:
types/spec-derived-unions.test.ts:optionsOfandschema-catalog:enumOptionsOfboth throw on an unreadable enum. The walk movesto the shared reader; the throw stays, because the reader deliberately answers
[]and the throw is those files' non-vacuity duty.
Six packages gain the
"@object-ui/test-support": "workspace:*"devDependency edgethe README's convention requires:
plugin-charts,plugin-dashboard,plugin-grid,plugin-report,react,types. Nodependenciesand no published type moved;packages/types' change is confined to its test file and a devDependency.Checks run, all on
7663a4892pnpm exec vitest runover test-support, types, schema-catalog, plugin-list, plugin-timeline, components, fields, react, plugin-charts, plugin-report, plugin-grid, plugin-dashboard, plugin-detail, app-shell (2 shards)pnpm exec vitest run scripts/Test Files 90 passed (90)/Tests 2572 passed (2572)turbo run type-checkover the 11 affected packagesTasks: 41 successful, 41 totalturbo run lint(whole farm, plain form)Tasks: 47 successful, 47 total, 0 errorsnode scripts/check-changeset-presence.mjs15 source file(s) of 10 released package(s) changed, and this change declares 1 changeset(s)node scripts/check-control-bytes.mjsOK (scanned 5818 tracked text file(s))pnpm check:phantom-depsEvery in-scope import is declared by the package that publishes it.pnpm check:self-importNo package names itself inside its own src/.pnpm check:readme-exports(afterturbo run build)OK (386 self-imports judged, 386 real, 0 fabricated)pnpm check:spec-symbols1325 files scanned, 0 untriaged collisionsnode scripts/check-type-check-coverage.mjs45/46 via type-check,41/41 packages compile their testsnode scripts/check-lint-coverage.mjs46/46 packages linted, 0 with outstanding errorspnpm check:vi-mock-specifiers,check:vi-mock-inheritnode scripts/check-pre-install-import-graph.mjspnpm check:side-effects-array,check:entry-guard,check:esm-specifierspackages/types'type-checkwas confirmed to actually read the edited file:tsc -p tsconfig.test.json --listFileslists bothspec-derived-unions.test.tsand
test-support/src/spec-enum-options.ts. A "typecheck is clean" that excludedthem would have said nothing.
One transient red is worth recording so nobody re-finds it:
scripts/__tests__/check-sdui-registration-pins.test.tsfails only while a localturbo run buildoutput is on disk — it then derivespackages/app-shell/dist/console/connect/ConnectAgentWidget.jswhere it expects thesrc/path. Withpackages/*/distremoved it passes 11/11, and this branch toucheszero files under
app-shell/src/console/. Pre-existing environment sensitivity, notthis diff.
Out of scope, filed separately
A sibling family turned up that this card's instrument does not select: 9 sites
spelling the cast non-optionally (
as { options: readonly string[] }), so afailed cast spreads
undefinedand throws rather than going quietly empty. Differentfailure class, same hand-copied reader; recorded rather than folded in, and the
follow-up card names them. Not addressed here; #6924 is the quiet-permissive half.
Generated by Claude Code