Uh oh!
There was an error while loading. Please reload this page.
fix(examples): drop the 21 inert value keys from the catalog overlay-menu fixtures - #7103
Merged
Merged
Conversation
…ixtures The four overlay-menu catalog fixtures authored a `value` key on 21 menu items. No arm of the shipped `MenuItem` union declares it (`MenuCommandItem` at packages/types/src/overlay.ts:363-401, `MenuDividerItem` at :409-419), and none of the three menu renderers reads it. `MenuItemSchema` builds its union from bare, non-strict `z.object`s, so zod strips the key and reports success — the key has been inert and invisible since it was authored. Delete the keys rather than declare them: nothing reads `value`, and #6523 deliberately narrowed this union instead of widening it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012wwHa4aaFybxXrfmfHioDM
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012wwHa4aaFybxXrfmfHioDM
…ed `value` key The objectui#6810 ruling (2026-08-30) prescribes `逐例修 + 补钉` for a key this class has regenerated on: the case-by-case fix PLUS a named pin carrying its own counter-probe. The fixture deletions landed without the pin half; this adds it. Deliberately a SEPARATE block from objectui#6249's menubar sweep rather than a widening of it. That sweep walks one node shape and its header states on purpose that extending it to dropdown-menu/context-menu is a separate verification surface. `value` was authored across all three containers, so a menubar-scoped pin would have covered 11 of 21 while reading as though it covered the class. The counter-probe is corpus-level, not schema-level: MenuItemSchema strips `value` and returns success, so a .safeParse probe stayed green through all 21 authored keys and is structurally blind to this defect. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012wwHa4aaFybxXrfmfHioDM
os-warren
marked this pull request as ready for review
September 1, 2026 00:34
Uh oh!
There was an error while loading. Please reload this page.
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
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#7072
Twenty-one menu items in the four overlay-menu catalog fixtures authored a
valuekey that no arm of the shippedMenuItemunion declares and no menu renderer reads. This deletes them, and pins that they stay gone.Re-derived on current
main, because the card's numbers were days oldThe card measured at
592acafb;mainis now2c3cd1b. The count has not moved — 21, distributed exactly as the card's table claims:valuekeyscomponents-overlay-menubar/application-menubar.jsoncomponents-overlay-context-menu/basic-context-menu.jsoncomponents-overlay-dropdown-menu/basic-dropdown-menu.jsoncomponents-overlay-dropdown-menu/with-icons.jsonCounted by a walker that classifies each
valuekey by its position in the tree rather than by filename, so the neighbouringbuttons[].valuepopulation (#7077, button-group) is separated by construction and cannot leak in. Catalog-wide the walker sees 288 objects carryingvaluebefore and 267 after — a delta of exactly 21, so nothing outside the four fixtures moved.MenuCommandItemispackages/types/src/overlay.ts:363-401(card said:363-400) andMenuDividerItemis:409-419(card said:409-421).The authority, re-verified
MenuCommandItem(overlay.ts:363-401) declareslabel,icon,disabled,onClick,shortcut,children,separator?: false,type?: never. Novalue.MenuDividerItem(:409-419) declaresseparator: true,type?: never. Novalue.packages/components/src/renderers/overlay/perform zero.valuereads. Stated precisely, because it is not zero occurrences of the token:context-menu.tsx:95contains{ type: 'text', value: "Right click here" }— a write constructing atextnode, whose renderer genuinely readsvalue. No menu item'svalueis ever read.MenuItemSchema(packages/types/src/zod/overlay.zod.ts:147-168) builds its union from bare, non-strictz.objects, so zod stripsvalueand reports success — the finding: JSON doc snippets are checked by nothing — the ts/tsx gate cannot see them, andBaseSchema.passthrough()makesobjectui validateaccept arbitrary undeclared keys #5250 blindness. Nothing red covered this.Deleted rather than declared: nothing reads
value, and #6523 deliberately narrowed this union rather than widening it.The pin — #6810's second half
#6810's ruling (2026-08-30, maintainer, verbatim「同意」) prescribes
逐例修 + 补钉for a key this class has regenerated on: the case-by-case fix plus a named pin carrying its own counter-probe, in the existingcomponent-fixture-declared-keys.test.tsledger form. This PR now carries both halves.⛔ Not a widening of #6249's menubar sweep — a new, separately named block. That sweep walks one node shape and its own header states that extending it to
dropdown-menu/context-menuis "a separate verification surface this fix does not need"; it is untouched.valuewas authored across all three containers, so a menubar-scoped pin would have covered 11 of 21 while reading as though it covered the class, which is worse than no pin.The new block sweeps the whole overlay-menu corpus — all three container shapes, including the deeper
menus[].items[]nesting onlymenubarhas, and submenuchildren:value, withlabel(21) andseparator(4) as in-query controls, so an empty or unparsed corpus cannot masquerade as a pass.MenuItemSchemastripsvalueand returns success, so a.safeParseprobe is structurally blind here — it stayed green through all 21 authored keys. The probe re-authorsvalueinto a real fixture taken from the corpus and asserts the identical sweep reports it at the right position, with the untouched fixture clean alongside.⭐ The pre-existing declared-key controls in the menubar block already fed
{label, icon, disabled, shortcut, value}toMenuItemSchemaand asserted only the first four survive. That pinned the type-level fact and was green the whole time the 21 keys sat in the corpus. The gap was only ever the corpus assertion.Ablation — the pin demonstrably goes red. With
"value": "profile"re-authored intobasic-dropdown-menu.jsonon disk (mutation confirmed:valuekey count 0 → 1, injected text present, blob hash7e72503b→ece5381c, still valid JSON), the suite fails:The non-vacuity test stayed green throughout, correctly — a key was added, not an item. Restore leg proven rather than assumed:
git checkout HEAD -- <abs path>,valuecount back to 0, blob hash matches the HEAD blob exactly,git diff HEADempty.Evidence
The diff is only what it claims. Every changed fixture line is one of: a removed
"value": …line (21), or a"label"line re-punctuated becausevaluehad been the last key on that item (14 removed / 14 added, identical but for the trailing comma). No content changed. All four files round-trip byte-identically throughJSON.stringify(…, null, 2), so the edit was made by parse-and-reserialise and the diff is minimal by construction.Zero-hit control. After the deletion,
valuehits 0 in all four files while the controllabelstill hits 14 / 4 / 4 / 4 in the same query — a zero for both would have meant the files were emptied, not fixed. Structure intact: menubar still has 13 items (2 separators, 11 labelled), matching the existingexpect(items).toHaveLength(13)pin.Rendered output is unchanged — which is the correct expectation here, the opposite of a fixture-content fix. The keys were inert, so the DOM must not move. Measured rather than assumed: a scratch harness rendered each dropdown fixture at
2c3cd1band at this branch withdefaultOpen: trueinjected (a closed menu renders only its trigger, which never carried the key, and would have made the comparison vacuous), normalised Radix's per-instance generated ids, and asserted identical HTML, text and[role="menuitem"]counts. Its control — mutating a key the renderer does read (label) — was required to produce a difference, and did. Scratch-only; not part of this diff.dropdown-menufixtures.menubar(11) andcontext-menu(4) cannot be forced open the same way — Radix opens them via a menubarvalueand a real right-click — so their inertness rests on the zero-read derivation above plus the corpus suites. My first attempt at this harness was discarded outright: without registering the renderers it fell back to a raw-JSON dump, and its control caught it.Suites, on the final commit
8595e42:659 = the prior 656 plus this PR's 3 new tests. Confirmed by name under
--reporter=verboserather than by an aggregate count:Run root-relative from the repo root, per #3378 — the
RUN v4.1.10 /home/user/objectui-issue-7072banner confirms it did not silently re-root ontoapps/console.Type-check.⚠️ The package's main
pnpm --filter @object-ui/example-schema-catalog type-check(the package spells it hyphenated) => EXIT 0, 0 errors, with the script body echoed (tsc --noEmit && tsc -p tsconfig.test.json) so a zero-match filter cannot pass for a green.tsconfig.jsonexcludestest/, so that green would say nothing about the new pin on its own;--listFilesconfirmscomponent-fixture-declared-keys.test.tsis really in thetsconfig.test.jsonprogram (889 files). The dependency closure was built first — before it, every test file in the package reportedCannot find module '@object-ui/types/zod', including ones this PR never touches.Gates, on
8595e42, exit codes captured before any pipe:check:control-byteschangeset:checkcheck:doc-typescheck:icon-record-namescheck:docs-route-closurecheck-changeset-presence.mjsGate family derived by hand from this repo's own
package.jsonand.github/workflows/—scripts/pm/dispatch-gates.mjslives only inobjectstackand would have answered about that tree.Changeset. The presence gate's verdict line, verbatim:
examples/schema-catalogisprivate: true, so the artifact is an empty-frontmatter changeset (.changeset/7072-catalog-menu-value-keys.md) — ⛔ not apatchbump, which would falsely claim a released package changed. Adding a test does not change that. ⛔ Noskip-changesetlabel: in this repo that label is a historical mis-mint that no workflow reads.Lint.
pnpm exec eslint examples/schema-catalog/test/component-fixture-declared-keys.test.ts=> EXIT 0, and--format jsonconfirms the file was actually linted (0 errors, 0 warnings, no "File ignored" message) rather than silently skipped. The four JSON fixtures and the changeset carry no eslint configuration and report as ignored. Plain form; ⛔ no--no-inline-config.Absorber check — #6810 is ruled, and it rules for this shape
The dispatch flagged #6810 as a possible absorber, expected to be undecided. It is not — it was ruled twice, and the second ruling adopted the middle state: narrow ratchet, ⛔ no general extractor, with option A (the family-wide sweep)
明确不采and never-before-seen keys taking option C —人手撞见、逐例修 + 补钉. So this card is not a row of a sweep; it is exactly the hand fix that ruling prescribes, and the pin above is its second half.Filed, not fixed
variant: "destructive", a key no arm ofMenuItemdeclares and no menu renderer reads — but the repair has a real fork #7101 — the same fixture authorsvariant: "destructive"on its Delete item; also undeclared on both arms and unread by all three renderers (dropdown-menu.tsxmaps items with no spread). ⛔ Not a mechanical delete: shadcn'sDropdownMenuItemreally does ship a destructive variant, so "declare and render it" is a defensible opposite repair. That is the finding(examples): seven catalogbadgenodes author their text underchildren, which ui:badge never reads — three demos draw nothing at all #6829 shape and needs a decision.valuekey, so the phantom spelling survives #7072 as copyable example code #7102 —context-menu-item-icon.test.tsx:169-173anddropdown-menu-item-icon.test.tsx:122-124transcribe these fixtures inline includingvalue, so the phantom spelling survives this PR as copyable example code. Mechanical to fix, but inpackages/components— a different package and gate family.⛔
#7077is not addressed here; itsbuttons[].valuepopulation is a different component family.Generated by Claude Code