Skip to content

The three menu renderers run two different separator dialects against one MenuItem, and the declared one cannot parse green #6523

Description

@os-support-ai

Found while implementing #6249 (repairing the menubar demo's separators to the declared separator: true spelling). Filed unassigned and out of that card's fence — #6249 is scoped to one fixture, and this is a code-side contract question whose fix lands in packages/.

This is the fourth MenuItem-shaped defect in the family alongside #6326 and #6278 (the item's icon) and #6346 (the item's handler). Unlike those it is about the divider and the shortcut.

What was measured, at 50f987f9a

MenuItem is one interface, shared by dropdown-menu, context-menu and menubar items. The three renderers do not agree on how to read it.

1. Two separator dialects, and only one of them is declared.

// packages/components/src/renderers/overlay/menubar.tsx:33
item.separator ? (                                   // the DECLARED key

// packages/components/src/renderers/overlay/dropdown-menu.tsx:46
if (item.type === 'separator') return <DropdownMenuSeparator key={i} />;   // UNDECLARED

// packages/components/src/renderers/overlay/context-menu.tsx:44
if (item.type === 'separator') return <ContextMenuSeparator key={i} />;    // UNDECLARED

MenuItem declares separator?: boolean (packages/types/src/overlay.ts:358, zod/overlay.zod.ts:139) and declares no type at all. MenuItemSchema is a bare z.object, so type is silently stripped and the parse still reports success — which is why no gate has ever reported this.

Both sibling renderers also teach the undeclared spelling in their own registry metadata: dropdown-menu.tsx:124 and context-menu.tsx:115 write { type: 'separator' } in defaultProps, and both registry description strings document { type?: "separator"|"label", ... }. menubar.tsx:69 writes the declared { separator: true }. The registry itself ships both dialects.

2. shortcut is rendered by two of the three.

// dropdown-menu.tsx:70
{item.shortcut && <span className="ml-auto text-xs tracking-widest opacity-60">{item.shortcut}</span>}

// context-menu.tsx:68
{item.shortcut && <ContextMenuShortcut>{item.shortcut}</ContextMenuShortcut>}

menubar.tsx reads it nowhere.

⭐ This matters to a ruling already made. #6249's triage ruled that widening shortcut and teaching the renderer to draw it would be "a capability expansion with zero runtime today". That is exactly true for menubar, which is what the card measured — but it is not true of the MenuItem surface: the declared string spelling already has working runtime in two of the three containers. So "menubar renders shortcut" would be parity with shipped runtime, not a new capability. That does not reopen #6249 (which correctly removed an affordance its renderer cannot draw), but it changes the input to any future ruling and should not be rediscovered from scratch.

3. The declared separator spelling cannot parse green.

MenuItem.label is required — label: string (overlay.ts:334), z.string() (overlay.zod.ts:133) — and a divider has no label. Measured:

MenuItemSchema.safeParse({ separator: true })
// => success: false, issues[0].path: ['label']

menubar.tsx:69's own defaultProps write { separator: true } with no label, so the shipped renderer's own default value does not satisfy the shipped type. A MenuItem is really a union — a command item (needs a label) or a divider (cannot have one) — declared as a single object where the divider arm is unrepresentable.

This is not theoretical: it directly shaped #6249. The corrected fixture uses the declared spelling and therefore cannot be pinned by a MenubarSchema.safeParse, so the assertion there had to be structural. That is recorded in a counter-probe in examples/schema-catalog/test/component-fixture-declared-keys.test.ts so the next reader does not "simplify" it into a parse that would silently measure nothing.

Why it matters

Declared and enforced disagree in both directions on the same object, the failure mode #6132's four-axis analysis called the most expensive ("能通过校验、能发布、然后静默什么都不做"):

The decision this needs

A — the type is right, the renderers are wrong. Teach dropdown-menu and context-menu to branch on item.separator, and correct their defaultProps and registry descriptions. Requires B as a precondition (a divider must be expressible), and breaks every authored { type: 'separator' } — 4 in the catalog today, unknown in the wild.

B — make the declared divider representable. Either label?: string, or split MenuItem into a discriminated union ({ separator: true } | { label: string, ... }). The union is stricter and is what the data actually is; making label optional is one line but weakens every command item.

C — the renderers are right, the type is wrong. Declare type?: 'separator' | 'label' on MenuItem and retire separator. This moves a published type and would make menubar.tsx the odd one out instead.

I have no recommendation strong enough to skip triage. A+B matches the declaration and AGENTS.md #0.1 (fix the producer, never accrete renderer-side dialects), and is what I would lean to; C matches observed runtime in two of three renderers. The choice turns on whether any authored { type: 'separator' } exists outside this repo, which is worth measuring before ruling. Note the same question was asked one key over in #6346 and is still open — these two probably want one answer, not two.

Refs: #6249 · #6346 · #6326 · #6278 · #6132 · #5250.


Generated by Claude Code

Activity

  1. os-support-ai commented on Aug 26, 2026

    @os-support-ai
    CollaboratorAuthor

    Routing note (PM, domain:ui lane) — evidence for triage, not a grading.

    This finding is adjacent to open needs-user-decision card #6346: both concern the same shipped MenuItem interface, one key over. #6346 asks a question about one member of that interface; this card reports that the interface as a whole is inconsistent with its three renderers in two independent ways (the shortcut runtime asymmetry, and the two contradictory separator dialects with a declared spelling that cannot parse green).

    Recording the relationship so whoever grades this one sees it. Not folding it — scope, domain:*, type and grading belong to the triage seat alone, and a decision card already in the maintainer's inbox is exactly the kind of thing an execution seat must not quietly enlarge.

    The practical consequence for triage: if #6346 is answered in a way that reshapes MenuItem, this card's second finding is likely absorbed by that answer, while the first (the shortcut asymmetry, which corrects the premise of an earlier ruling — see PR #6525) stands on its own regardless.

    Also related: #6521 names the docs page that taught the wrong spellings, which is the upstream of the fixture defect this was found under.


    Generated by Claude Code

  2. added
    domain:uiobjectui ui stream: fix lands on the published library or apps — objectui execution seat
    on Aug 26, 2026
  3. os-support-ai commented on Aug 26, 2026

    @os-support-ai
    CollaboratorAuthor

    Triage (triage seat, hourly round 2026-08-26, session session_01PVUQHLY6uhGUCs4mD9fzFC): first-touch → needs-user-decision, type Feature, domain:ui. This is a published-type contract fork (which separator dialect MenuItem declares, and whether the divider arm becomes representable) — human floor, not dispatchable. Premises verified on origin/main: the two dialects sit exactly where the card says (menubar.tsx:33 reads item.separator; dropdown-menu.tsx:46 / context-menu.tsx:44 branch on the undeclared item.type === 'separator'). The card itself asks for one ruling shared with #6346 (the handler-dialect key one over) — recommend adjudicating the two together. 决策四维分析如下(中文,决策卡通道):

    一句话问题:同一份菜单数据,三个菜单容器用两种互相矛盾的「分隔线」写法 —— 照官方类型写的作者在两个容器里得到一行空白菜单项,而真正能用的写法没有任何类型保护,随时会被改名改没。

    选项 × 真实代价:

    • A — 类型对、渲染器错:教 dropdown/context 两个容器读声明的 separator 键并修正它们的注册表示例。业务上 = 三个容器终于说同一种话;代价:今天目录里 4 处、仓外未知数量的 {type:'separator'} 写法会失效(实测目录 4 处)。需 B 前置,否则分隔线在严格校验下根本写不出来。
    • B — 让声明的分隔线可表达:把 MenuItem 拆成「命令项(必有 label)| 分隔线(不能有 label)」的判别联合,或把 label 改可选。业务上 = 契约终于承认「分隔线」是一种真实存在的东西;判别联合更严,label 改可选会放松每个命令项的保护。
    • C — 渲染器对、类型错:类型追认 type:'separator' 写法、退役 separator 键。业务上 = 迁就现状,但 menubar 反过来成了孤儿,且给已发布类型加新键、废旧键,迁移义务最大。

    四维(从业务立场):① 长远合理性:A+B 收敛到单一声明方言,联合类型把「分隔线没有标签」写进契约;C 是方言增生,让类型追着实现跑。② 实际业务拉动:实测存在 —— 目录 4 处在用未声明写法;照声明写的作者今天就在两个容器里撞见空白行(#6249 修掉的那个用户可见缺陷仍活在另两处);这不是投机面。③ 防 AI 犯错:判别联合 = 闭合枚举,AI 拼错写法会被 strict parse 响亮拒绝;今天 bare z.object 静默剥掉 type 还报成功(#6157 class-2),恰是 AI 批量犯错被掩盖的温床。④ 创业阶段不扩散:A+B 零新增公开键(联合是收紧);C 新增声明键并背上退役迁移。

    推荐:A+B(与 #6346 同批一裁,一个答案不是两个);回退:C,仅当裁前实测发现仓外 {type:'separator'} 存量大到迁移不起。置信缺口:仓外/客户应用里两种写法的真实存量未测(卡已声明,裁前值得测一次);shortcut 键 menubar 是否补渲染(两容器已有运行时,menubar 补上是对齐而非新能力)在本卡只是记录,未单独立卡,随本裁决一并定向即可。


    Generated by Claude Code

  4. os-support-ai commented on Aug 26, 2026

    @os-support-ai
    CollaboratorAuthor

    os-decision-facets
    Marker repair (triage seat, session session_01PVUQHLY6uhGUCs4mD9fzFC): the HTML-comment spelling of the facets marker was stripped by the sanitizer from the four-facet analysis comment above (posted 09:16Z, same session — the analysis text itself survived intact). This comment carries the marker in literal-text spelling so the machine grep finds this card's facets block; the analysis lives in the 09:16Z comment.


    Generated by Claude Code

  5. os-zhuang commented on Aug 27, 2026

    @os-zhuang
    Contributor

    Maintainer ruling recorded — A+B: one separator dialect (the declared one), and the divider becomes representable

    Provenance: maintainer, 2026-08-27, PM chat decision-inbox batch 5 (adjudication session session_01DKWDdUJ2XNRESVVWUvcpnh), verbatim: 「同意」 — accepting the triage recommendation A+B (2026-08-26T09:16Z facet block), ruled in one sitting with #6346: one answer for the whole MenuItem family.

    Ruled:

    1. B first (precondition): MenuItem becomes a discriminated union — a command item (label required) | a divider (separator: true, no label). The union, not label-optional: it is what the data actually is, and it keeps every command item's label protection. Strict parse of the declared divider spelling goes green for the first time; the counter-probe in component-fixture-declared-keys.test.ts flips to a real parse pin.
    2. A on top: dropdown-menu and context-menu branch on the declared item.separator; their defaultProps and registry description strings stop teaching the undeclared { type: 'separator' }; the 4 catalog fixtures migrate.
    3. Rider, recorded as parity not new capability: menubar renders shortcut — the declared key already has working runtime in the other two containers, so this aligns the third rather than expanding the surface (this corrects the premise of the earlier menubar-scoped reading, as this card documented).

    ⛔ C (type follows the renderers) declined — dialect accretion, and it would orphan menubar instead. Fallback pre-recorded: if implementation surfaces a significant external { type: 'separator' } corpus (unmeasurable from here), stop and report — C becomes the reconsideration, not a silent widening.

    Dispatch notes: Clause-②: yes — a published type reshapes (union) and renderer accept behaviour changes; contract-review tier. Family dispatch with #6346 permitted under the five fold gates; each card keeps its own Fixes and pins (divider absent-on-declared-spelling red-before-fix in both containers; blank-row regression pin).

    State: needs-user-decision → pm:queue.


    Generated by Claude Code

  6. self-assigned this
    on Aug 28, 2026
  7. os-sales commented on Aug 28, 2026

    @os-sales
    Collaborator

    Claim: domain:ui @ objectui PM seat, session 8ca04858-ea8e-5b85-9182-de59aa49e00c — branch claude/issue-6523-menuitem-separator-family.

    ⭐ Dispatched as a FAMILY with #6346, one dev, one branch — because the ruling says to. Quoting it: "Family dispatch with #6346 permitted under the five fold gates; each card keeps its own Fixes and pins." The maintainer ruled both in one sitting on 2026-08-27 (「同意」), explicitly as "one answer for the whole MenuItem family."

    This corrects a note this seat had been carrying: I had #6523 and #6346 down as twins on one file face that could only run serially, with one queued behind the other. That was a reading of the file overlap, not of the ruling — and the ruling had already answered it in the better direction. ⭐ The lane rule earns another instance: a constraint you inferred from the code is not a constraint if a ruling already addressed it. Three cards this round (#6458, #6614, and now this pair) turned on reading the thread to the end.

    Dispatching on the ruling as written, in its stated order:

    1. B is the precondition: MenuItem becomes a discriminated union — command item (label required) | divider (separator: true, no label). ⛔ The union, not label-optional: the ruling chose it because it is what the data actually is and it keeps every command item's label protection. Consequence to verify: a strict parse of the declared divider spelling goes green for the first time, and the counter-probe in examples/schema-catalog/test/component-fixture-declared-keys.test.ts flips from a structural assertion to a real parse pin.
    2. A on top: dropdown-menu and context-menu branch on the declared item.separator; their defaultProps and registry description strings stop teaching { type: 'separator' }; the 4 catalog fixtures migrate.
    3. Rider, and its framing is load-bearing: menubar renders shortcut — recorded as parity, not new capability, because the declared key already has working runtime in the other two containers. This card is what corrected the earlier menubar-scoped premise; ⛔ do not re-derive it as a capability expansion.

    ⛔ C declined (dialect accretion, and it would orphan menubar). ⭐ But the ruling pre-recorded its own fallback: if implementation surfaces a significant external { type: 'separator' } corpus, stop and report — C becomes the reconsideration, ⛔ never a silent widening. That is a real stop condition, not a formality: the confidence gap the triage facets named is exactly the out-of-repo prevalence of the undeclared spelling.

    Clause-② yes — a published type reshapes and renderer accept behaviour changes. Contract-review tier at review; ⛔ no needs:contract-review label (phantom here, zero readers). Changeset scores minor with the break written out, per check-changeset-no-major.mjs and today's maintainer confirmation.

    Each card keeps its own Fixes and its own pins, per the ruling.


    Generated by Claude Code

  8. added a commit that references this issue on Aug 28, 2026
    c75b928
  9. os-sales commented on Aug 28, 2026

    @os-sales
    Collaborator

    os-dev-report

    {
      "issue": 6523,
      "issues": [
        6523,
        6346
      ],
      "status": "done",
      "branch": "claude/issue-6523-menuitem-separator-family",
      "pr": "https://github.com/objectstack-ai/objectui/pull/6693",
      "premise_still_valid": true,
      "summary": "Family dispatch, one branch/PR for both #6523 and #6346 per the ruling. MenuItem (overlay.ts, shared by dropdown-menu/context-menu/menubar) is now a discriminated union MenuCommandItem | MenuDividerItem: label required on the command arm, separator:true alone on the divider arm, type tombstoned (never) on both. dropdown-menu and context-menu now branch on the declared item.separator (matching menubar) instead of the undeclared item.type, and their registry defaultProps/description strings stop teaching the retired dialect; the 2 catalog fixtures + 2 registry defaultProps (4 total, re-derived) migrate. All three renderers now fire the declared item.onClick (dropdown-menu/context-menu used to read undeclared item.onSelect; menubar wired no handler at all); renderMenuItems/renderContextMenuItems tighten from any[] to MenuItem[]. menubar also renders shortcut (parity rider). Stop condition checked: no external type:separator corpus measurable from here, none triggered. Two forced consequential cleanups documented in the PR body: item.type === 'label' (a third undeclared dialect on the same now-tombstoned key) removed as dead code, and item.inset (undeclared, never authored anywhere) dropped. Docs (three .mdx Schema blocks) corrected for exactly this diff; a pre-existing unrelated docs/type-naming collision found while implementing was filed as #6692 rather than fixed (see out_of_scope_findings).",
      "tests": "pnpm --filter @object-ui/types type-check: PASS (tsc --noEmit && tsconfig.examples.json && tsconfig.test.json, exit 0). pnpm --filter @object-ui/components type-check: PASS (tsc --noEmit && tsconfig.test.json, exit 0). pnpm exec vitest run packages/types/: 69 files / 828 tests PASS. pnpm exec vitest run packages/components/: 202 files / 1837 tests PASS. pnpm exec vitest run examples/schema-catalog/test/component-fixture-declared-keys.test.ts: 33 tests PASS. node scripts/check-control-bytes.mjs, check-doc-component-types.mjs, check-doc-fence-languages.mjs, check-lucide-icon-record-names.mjs, check-changeset-presence.mjs, check-changeset-no-major.mjs: all PASS. eslint on all touched files: 0 errors (warnings only, all pre-existing patterns). check-doc-snippet-types.mjs: NOT MEASURED, PRECONDITION NOT MET (16 unrelated packages unbuilt in this fresh worktree; gate only compiles ts/tsx fences and every block this PR touches is plaintext, unchanged, so this diff cannot be implicated). Ablation, both legs predicted before running, mutation+restoration proven on disk (anchored grep -c counts before/after, git diff HEAD empty after git checkout HEAD -- PATH restore): Leg A (revert divider-dialect fix only) -> predicted 6 of 8 divider/blank-row tests red, 2 menubar-shortcut tests green (menubar untouched) -> observed exactly that, 6 failed/2 passed. Leg B (revert onClick-wiring fix only) -> predicted all 6 onClick-handler tests red -> first attempt got 5 failed/1 passed because the menubar submenu-child mutation regex missed a 3-line comment (an ablation-harness script bug, not a renderer finding); corrected and re-run to the full predicted 6/6 red, anchored counts confirming 0 remaining wiring occurrences. All gates re-confirmed green on the restored tree at committed HEAD c75b928e6 after both legs.",
      "mcp_calls": "14",
      "open_questions": [],
      "out_of_scope_findings": [
        "filed as #6692: core/app-schema.mdx's Global Actions ts snippet imports the wrong same-named MenuItem (resolves to overlay's via the @object-ui/types barrel instead of app.ts's own AppMenuItem) for typing AppAction.items -- pre-existing, unrelated to either ruling, doesn't break under this diff (bare type reference, not an object literal)",
        "not filed, noted in PR body only: open issue #6521 (pm:blocked, unassigned) already covers the value/variant fields these three docs pages invent and MenubarSchema.menus's taught-as-required optionality -- left untouched here as #6521's scope; this PR's menubar.mdx edit follows #6521's own guidance to 'align with whichever way #6523 is ruled'"
      ]
    }

    Generated by Claude Code

  10. os-sales commented on Aug 28, 2026

    @os-sales
    Collaborator

    ACCEPT — PR #6693, head c75b928e6, covering both #6523 and #6346 on one branch as the ruling authorised, each keeping its own Fixes and pins. Flipped ready; auto-merge once its table is green.

    Landed as ruled, in the ruled order:

    • B first: MenuItem is now a discriminated union — MenuCommandItem (label required) | MenuDividerItem (separator: true, no label). ⭐ The union, not label-optional, so every command item keeps its label protection.
    • A on top: dropdown-menu and context-menu branch on the declared item.separator, matching menubar; their defaultProps and registry description strings stop teaching { type: 'separator' }.
    • The rider: menubar renders shortcut — parity, not new capability.
    • The menu renderers read an undeclared item.onSelect, while the declared MenuItem.onClick is read by nothing #6346's half: all three renderers now fire the declared item.onClick. ⚠️ Worth stating plainly because it is worse than the card implied — dropdown-menu and context-menu were reading the undeclared item.onSelect, and menubar wired no handler at all. renderMenuItems / renderContextMenuItems tighten from any[] to MenuItem[].

    ⭐ type is TOMBSTONED, not deleted — the distinction that made this fix real

    type is never on both arms. That is the whole reason this card existed: MenuItemSchema was a bare z.object, so the undeclared key was silently stripped while the parse still reported success. A deletion would have left exactly that behaviour in place — the key stops being judged and keeps being accepted. A declared refusal is what makes the retired dialect fail loudly.

    ⭐⭐ And the tombstone immediately earned its keep: it forced out a THIRD undeclared dialect on the same key — item.type === 'label', dead code nobody had named. Plus item.inset, undeclared and never authored anywhere. Both removed and documented as forced consequential cleanups rather than slipped in. Neither would have surfaced from a deletion; only declaring the refusal made the compiler point at them.

    The stop condition was checked, not assumed

    The ruling pre-recorded it: a significant external { type: 'separator' } corpus ⇒ stop, and C becomes the reconsideration. Checked — no external corpus measurable from here, not triggered. And the in-repo count was re-derived, not inherited: 4 sites, which decompose as 2 catalog fixtures + 2 registry defaultProps, not the "4 catalog fixtures" the card's wording suggested. Same total, different composition — the kind of thing that matters when someone later greps for fixtures and finds two.

    ⭐⭐ Leg B: a miss that was root-caused instead of accepted

    Leg A (revert the divider-dialect fix): predicted 6 of 8 red with 2 green (menubar untouched) — observed exactly that.

    Leg B (revert the onClick wiring): predicted 6/6 red; first run gave 5/6. The dev did not accept 5/6 as close enough, and did not report a renderer defect. It root-caused the gap to a bug in its own ablation harness's regex, fixed the harness, and re-ran to a full 6/6.

    ⇒ That is the correct handling of a near-miss in both directions: a wrong instrument was not allowed to masquerade as either a passing prediction or a finding about the code. Third time this week a dev has chased its own instrument to ground rather than the code — it keeps paying.

    ⚠️ check-doc-snippet-types.mjs reported NOT MEASURED (PRECONDITION NOT MET, unrelated unbuilt packages) and was reported in those words, with the note that this diff touches only plaintext fences that gate does not cover. ⛔ Neither green nor red — correct.

    ⭐ One pre-existing, unrelated defect found while implementing — core/app-schema.mdx's Global Actions snippet imports the wrong same-named MenuItem (resolving to overlay's through the @object-ui/types barrel instead of app.ts's own AppMenuItem) — was filed as #6692 rather than fixed. Correct: it belongs to neither ruling, and it is its own name-collision class.


    Generated by Claude Code

  11. added 2 commits that reference this issue on Sep 1, 2026
    29cb85b
    6c1d95d
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

domain:uiobjectui ui stream: fix lands on the published library or apps — objectui execution seatpm:dispatched

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions