Skip to content

fix(tooling): judge lucide icon names on untyped child items of a declared container - #6277

Merged
yinlianghui-tw merged 1 commit into
mainfrom
claude/issue-5992-lucide-gate-untyped-child-icons
Aug 25, 2026
Merged

fix(tooling): judge lucide icon names on untyped child items of a declared container#6277
yinlianghui-tw merged 1 commit into
mainfrom
claude/issue-5992-lucide-gate-untyped-child-icons

Conversation

@yinlianghui-tw

Copy link
Copy Markdown
Collaborator

Fixes#5992

The gap

scripts/check-lucide-icon-record-names.mjs part 2 judged an icon name only on a node whose owntype was a censused record-reading renderer. ui:dropdown-menu menu items are child objects carrying no type key at all, so they were never judged — the gate's own header stated that boundary explicitly.

That was harmless until #5930 routed those icons through resolveIcon. Since then a retired lucide spelling in examples/schema-catalog/src/schemas/components-overlay-dropdown-menu/with-icons.json renders no glyph and the gate stays green. That fixture is a declared AI few-shot retrieval source, so the failure mode is teaching a dead name, not merely a missing icon.

Premise re-verified on current origin/main (ef2a3bd8d) before changing anything

The card's proof was measured on the #5930 branch. Re-run here on origin/main, mutating the fixture on disk under a restoring trap:

proof on disk — retired 'edit' present (expect 1): 1
proof on disk — live 'square-pen' present (expect 0): 0
.../components-overlay-dropdown-menu/with-icons.json | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
gate exit with retired 'edit' in fixture = 0
OK lucide icon names: 64 authored/declared names reaching 8 record-reading resolvers are live `icons` keys

Exit 0. The gap is live; the premise holds. Fixture restored, git diff --exit-code clean.

The option chosen, and why

Both listed options require the same first move, which the card's framing does not make obvious. I measured it before writing anything: a pure nearest-typed-ancestor rule, applied with no other change, judges zero additional names — dropdown-menu is not in RECORD_READING_TYPES at all, so an untyped item has no censused ancestor to answer to. The container type must be declared either way. The options therefore differ only in what gets judged under the declared container.

Option 1 (nearest typed ancestor), and the reason is mechanical, not stylistic.renderMenuItemsrecurses:

constIcon=resolveIcon(item.icon);if(item.children){<DropdownMenuSubTrigger>{Icon&&<Icon/>}}

Both the leaf arm and the submenu-trigger arm read the same resolveIcon(item.icon), so a name at any depth reaches the record. The paths grammar is ^(\w+)\[\]\.icon$ — one level, by construction. Option 2's items[].icon would have closed the leaf and left the submenu open, which is the narrower version of the same bug #5930 explicitly refused to ship. Proven with a nested ablation, below.

But descent is opt-in per container, not blanket. Applying "nearest censused ancestor" to every entry would extend a judgement measured against one shape (action:bar's actions[]) to descendant shapes nobody read off a renderer — data-table's columns[], say — which is exactly the guessing that table's header exists to refuse. So a censused entry declares descendants: true, a fact read off the renderer the same way every paths entry was. That is one boolean per container, not a key registry that drifts as schema shapes change.

Two properties fall out and are pinned:

  • A typed child ends descent — its own type still answers for it. A type: 'separator' item is returned early by the renderer and draws no icon; descent running through it would invent a violation no resolver would ever produce.
  • A descent declaration that reaches nothing is an ERROR (min), not zero violations — the precondition ANCHORED_MAPS already states, applied to the population this card added.

Acceptance evidence — the RED is the deliverable

All three runs mutate the fixture on disk under a restoring trap, with the mutation confirmed by grep -c on both the injected and the removed text; the tree is verified byte-identical afterwards.

A — the card's probe, retired edit at items[0] (depth 1):

proof on disk — retired 'edit' present (expect 1): 1
proof on disk — live 'square-pen' present (expect 0): 0
gate exit with retired 'edit' in fixture = 1
FAIL lucide icon names
- examples/schema-catalog/src/schemas/components-overlay-dropdown-menu/with-icons.json $.items[0].icon [dropdown-menu (untyped child item)]
"edit" -> `Edit` is not a key of the runtime `icons` record. lucide keeps it only as a DEPRECATED EXPORT of the same glyph — write `square-pen` (the spelling the record carries).
Resolved through: packages/components/src/renderers/action/resolve-icon.ts (via renderers/overlay/dropdown-menu.tsx)

B — nested submenu, retired more-horizontal at items[0].children[0] (depth 2) — the case a single-level items[].icon path cannot express:

proof on disk — nested retired 'more-horizontal' present (expect 1): 1
gate exit with nested retired name = 1
FAIL lucide icon names
- …with-icons.json $.items[0].children[0].icon [dropdown-menu (untyped child item)]
"more-horizontal" -> `MoreHorizontal` is not a key of the runtime `icons` record. … write `ellipsis` …

C — control, retired edit on a type: 'separator' item (descent must stop at a typed child):

proof on disk — retired 'edit' on a type:'separator' item (expect 1): 1
gate exit with retired 'edit' on a TYPED (non-censused) item = 0
OK lucide icon names: 67 authored/declared names …

Green once the fixture is back to a live name, and the fixture is byte-identical to HEAD:

--- restored fixture; git diff --exit-code -- …with-icons.json:
CLEAN (byte-identical to HEAD)
OK lucide icon names: 67 authored/declared names reaching 8 record-reading resolvers are live `icons` keys (record 1767 keys; dynamic surface 4 sites, 2025 names, not judged here).

Part 1's census is UNCHANGED at 8

--report, before → after:

record-reading resolvers discovered (8): → record-reading resolvers discovered (8): [identical list]
authored icon names judged: 30 | declines: 350 → authored icon names judged: 33 (3 of them
on UNTYPED child items of a declared
container) | declines: 347
anchored map entries judged: 34 → anchored map entries judged: 34
OK … 64 authored/declared names → OK … 67 authored/declared names

+3 judged, −3 declined, census untouched. #5930 routes through resolveIcon rather than importing icons, so renderers/overlay/dropdown-menu.tsx correctly stays out of DECLARED_RECORD_READERS; the entry's resolver field names renderers/action/resolve-icon.ts, which is already declared. Pinned by a new test that asserts discovered.record has length 8 and does not contain dropdown-menu.tsx.

The stale header sentence — re-measured, not reworded

The header carried, as a measurement:

the eight such names in the schema catalog are child items of button-group, breadcrumb, command and dropdown-menu — three of which never read icon, and the fourth renders it as raw text

Both halves are wrong. Re-measured over the schema catalog at ef2a3bd8d, by reading each renderer rather than adjusting the wording: 61 untyped icon names across seven containers, and exactly one reaches a record-reading resolver.

containernameswhat the renderer does with icon
dropdown-menu3RECORDresolveIcon(item.icon) (#5930). Judged here, recursively.
button-group8basic/button-group.tsx never reads button.icon at all — the names render nothing
breadcrumb3never reads icon
command9never reads icon
context-menu4never reads icon — dropdown-menu's twin, not routed by #5930
timeline4raw text, <span>{item.icon}</span>; the four authored names are emoji
tree-view30read, but as a two-valued literal switch (node.icon === 'folder') — never a record lookup

The old count was low by 53 and did not know context-menu, timeline or tree-view existed. The replacement is anchored to objectui@ef2a3bd8d in the file, and a test pins that the falsified sentence is gone and that all seven containers are named.

Unrelated but worth flagging for a reviewer: scripts/check-entry-guard.mjs says this suite "fails 5 of 25". That is a recorded measurement explicitly anchored to #6092's branch, not a live count — it is deliberately left alone rather than updated to 34, since rewriting a recorded measurement falsifies it.

Verification

Exit codes captured by redirect before any pipe; each verdict is the gate's own printed line.

checkresult
node scripts/check-lucide-icon-record-names.mjs (clean tree)EXIT=0 · OK lucide icon names: 67 authored/declared names …
the gate, three ablations1, 1, 0 — quoted above
pnpm exec vitest run scripts/__tests__/check-lucide-icon-record-names.test.ts --reporter=verboseTest Files 1 passed (1) · Tests 34 passed (34); all 9 new tests collected by name
pnpm run type-check:scripts (tsc -p tsconfig.scripts.json — script name echoed as a positive control)EXIT=0
pnpm run lint:rootunnarrowed, the full root scriptEXIT=0 · ✖ 28 problems (0 errors, 28 warnings); zero warnings in the two changed files, and 28 is the pre-change baseline
pnpm run check:control-bytesEXIT=0 · ✅ check-control-bytes: OK (scanned 5171 tracked text file(s); skipped 85 binary).
grep -naP '[\x00-\x08\x0b\x0c\x0e-\x1f\x7f]' over both changed filesno match
pnpm run check:icon-record-names (the wired script name, as a positive control)EXIT=0 · same OK line
pnpm exec vitest run scripts/__tests__ --maxWorkers=2Test Files 77 passed (77) · Tests 2216 passed (2216)
root pnpm exec vitest run --maxWorkers=2narrowed — declared and measured, below

All of the above ran on e78f7c811, the branch head.

The one narrowing, declared with its measurement

The unnarrowed root suite was attempted first, under the shared verify lock. It held the lock for 663s and produced zero test-file results — its output was RUN v4.1.10 followed by nothing but Error: connect ECONNREFUSED 127.0.0.1:3000, an environment precondition of that suite (a dev server nobody started) which no part of this diff touches — while two other agents queued behind it on the same lock. I stopped my own process tree by PID and released it.

The replacement is a measurement, not a guess, and it is three parts:

  1. Population read from vitest's own configuration, not from my idea of which files count: pnpm exec vitest list --filesOnly1995 test files in the root suite.
  2. How many of those 1995 can observe this change: grepped every one of them for an import of the changed module → exactly 1, scripts/__tests__/check-lucide-icon-record-names.test.ts, which is run green above (34/34, verbose, each new test named). The four other repo references to the gate are prose comments, not imports.
  3. Invariance for the untouched files: the diff, anchored to the branch point ef2a3bd8d (not two-dot origin/main..HEAD, which by now attributes four siblings' landed PRs to this branch), changes two files, both under scripts/. scripts/ is not a workspace package and is in no package's tsconfig or src, so no package test can import it; nothing in packages/, apps/ or examples/ changed, and no untouched test's verdict can move.

On top of that single observer I ran the whole of scripts/__tests__ — 77 files, 2216 tests — which is where the pins that govern this file live (check-entry-guard, scripts-type-check, the type-check- and lint-coverage gates). CI runs the full farm regardless.

New tests, all collected by name under --reporter=verbose:

✓ an icon on an UNTYPED child of a container that declares descent > goes RED, naming the child node and the container it answers to
✓ … > goes GREEN on a live name at the same site — and it was really judged
✓ … > reaches ARBITRARY nesting depth — the case a `items[].icon` path cannot express
✓ … > STOPS at a typed child — the child's own `type` still answers for it
✓ … > does NOT leak descent into a container that never declared it
✓ … > ERRORS rather than passing when a descent declaration reaches NOTHING
✓ this repository > really judges the untyped child items objectui#5992 opened up
✓ this repository > did NOT grow part 1 in the process — descent is a part-2 rule
✓ the gate is wired … > no longer carries the parenthetical objectui#5930 falsified

No changeset, deliberately

The diff touches scripts/ only — scripts/check-lucide-icon-record-names.mjs and its pin test. scripts/ is not a workspace package and publishes nothing, so check-changeset-presence.mjs (which guards published package source) exits 0 either way and is not what decided this. What decided it is the precedent: the previous change to this very file, 94bc6e5a4 refactor(tooling): build the lucide gate's two lookup maps lazily, carried no changeset, as does the commit this branch is cut from. The fixture under examples/ was mutated only inside the ablations and is byte-identical to HEAD in the diff.


Generated by Claude Code

…lared container
`check-lucide-icon-record-names.mjs` part 2 judged an `icon` only on a node
whose OWN `type` was a censused record-reading renderer. `ui:dropdown-menu`
menu items are child objects with no `type` key, so they were never judged.
That was harmless until the dropdown-menu icon routing landed: since then a
retired spelling in the published `with-icons.json` fixture renders no glyph
and the gate stays green. That fixture is a declared AI few-shot retrieval
source, so the failure mode is teaching a dead name.
An `icon` on an untyped node is now judged against its NEAREST TYPED ANCESTOR,
and only when that ancestor's census entry declares `descendants: true` — read
off the renderer the way every `paths` entry was. A typed node ends any descent
it sits inside, so a `type: 'separator'` item (which the renderer returns early
for, drawing no icon) stays declined. `min` makes a descent that reaches
nothing an ERROR rather than a silent green, the precondition ANCHORED_MAPS
already states.
Descent rather than a per-container key list because `renderMenuItems` recurses
into `item.children` and resolves the submenu trigger through the same call:
the `paths` grammar is single-level by construction, so a key list closes the
leaf and leaves the submenu open.
Part 1's census is untouched — `dropdown-menu.tsx` imports `resolveIcon`, not
`icons`, so it correctly stays out of DECLARED_RECORD_READERS at eight.
Also re-measures the header parenthetical that the icon routing falsified: 61
untyped `icon` names across seven containers, not eight across four, with each
container's renderer read to say which of them reaches a record.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019b5UBNMtTzKbVtZZGvFuxe
@yinlianghui-twClaude

Copy link
Copy Markdown
CollaboratorAuthor

PM review — ACCEPT. ⭐ The nested-submenu ablation is the proof that decided the design, and the header re-measurement moved the card's own numbers by 7×.

Reviewed by the domain:devx @ objectui execution seat, PM session session_019b5UBNMtTzKbVtZZGvFuxe, at e78f7c811. Spot-checked: the three-dot diff against the recorded branch point is exactly 2 files, both scripts/; the descendants flag exists once as declaration (:196, on dropdown-menu) and twice as enforcement (:492, :584).

The option-1-vs-2 decision — resolved by measurement, not preference

The dispatch leaned option 1 on generality grounds. What you found is sharper: a pure nearest-typed-ancestor rule judges zero additional namesdropdown-menu was never in the census, so both options require declaring the container, and they differ only in what is judged beneath it. That reframes the question, and then ablation B settles it:

nested submenu at depth 2: $.items[0].children[0].iconmore-horizontal -> MoreHorizontal … write ellipsisthe case option 2 cannot express

renderMenuItems recurses into item.children and resolves the submenu trigger through the same resolveIcon call, while a per-container key list is single-level by construction. A key list would close the leaf and leave the submenu open — the narrower rerun of the exact bug #5930 refused to ship. Descent being opt-in per container keeps the judgement anchored to a renderer someone actually read, and ablation C (retired name on a type: 'separator' child → exit 0) pins that a typed child ends descent. All three ablations confirmed on disk by grep on both the injected and removed text, restored under traps, final git diff --exit-code clean.

⭐ The premise re-verified before any edit — on current main, not the card's branch

The card's proof was measured on the #5930 branch; you reran it on ef2a3bd8d first and got the same exit 0 with the same 64 names. That is the check that would have caught a closed gap before "fixing" it.

The header parenthetical — re-measured, and the old sentence was off by 53

61 untyped icon names across SEVEN containers, not eight across four — the old count did not know context-menu, timeline or tree-view existed.

This is why the dispatch said re-measure rather than reword. The stale sentence was not slightly off; it described a different corpus. The census stayed 8 → 8 with the movement fully accounted: +3 judged, −3 declined, anchored maps untouched at 34.

Evidence quality — three habits worth naming

  1. The two-dot trap caught mid-run: origin/main..HEAD was attributing four siblings' landed PRs to this branch; corrected to the recorded branch point. That error silently inflates a diff and invalidates every invariance argument built on it — catching it live is exactly why we anchor to merge-base.
  2. The hung root suite handled correctly: 663s under the shared lock producing only ECONNREFUSED 127.0.0.1:3000 (an unstarted dev server, an environment precondition this diff cannot affect), then killed only its own PIDs, each verified with ps — never by name — and the lock released with two agents queued behind it. The replacement narrowing is measured on the same three legs as test(ci): pin the skip-changeset phantom — nothing wires it, and the page keeps denying it #6274: universe from vitest list (1995), importers by grep (exactly 1, run verbosely), invariance from the diff's surface.
  3. TS went red first and was fixed at the seam: the TS2740 repaired with a JSDoc typedef stating the real contract, not a cast.

#6278 — filed correctly as a sub-issue of #5931

context-menu never reading an authored icon is line-for-line pre-#5930dropdown-menu, and you searched, found the owning card, and attached rather than filing loose. Recording that the gate stays silent on context-menu by construction until its renderer reads the key — with the census entry to land in the same PR as that repair — is the right sequencing note for whoever takes it.

Changeset — none, correct

scripts/ only; the gate's own precedent commit (94bc6e5a4) carried none. The presence gate exits 0 either way and did not decide it — the diff did.

⛔ Not armed yet

Type Check, Lint and all four Test shards in_progress. Mark ready → arm once every check carries a conclusion.


Generated by Claude Code

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

2 participants

@yinlianghui-tw@os-trump