Uh oh!
There was an error while loading. Please reload this page.
refactor(components,app-shell,plugin-detail,plugin-list,plugin-view): one lucide icon-name seam, seven resolvers down to one - #7491
Conversation
… one lucide icon-name seam, seven resolvers down to one
Seven modules resolved authored icon names against lucide's runtime `icons`
record, each with its own copy: three tokenisers (`split('-')` on five,
`split(/[-_\s]/)` on one, `split(/[-_\s]+/)` on one) and the `Home` -> `House`
rename on four of seven. The same authored name therefore rendered on one
surface and not another. `resolveIcon` is now exported from
`@object-ui/components` and the other six call it.
The tokeniser is `split(/[-_\s]+/)` with the rename universal — measured, not
chosen: its regression set is empty against the authored population, against an
every-name-by-every-surface cross-product, and against a bound-free differential
over 8,298 spellings derived from all 1,767 live keys.
The seam answers `name -> component | null` and decides nothing else. Every call
site keeps its own fallback, at the call site: `ui:icon` its `SquareDashed`
placeholder and warning (objectui#5631, untouched), `RelatedList` and `ListView`
their `Inbox`, `ActionPreview` its name chip, the rest `null`.
`scripts/check-lucide-icon-record-names.mjs` is simplified in the same change:
its census goes from seven sites to one, and its normalisation stops being a
widest-common approximation of three disagreeing resolvers.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EMrWaQw3XS5DxTHxp4yRyC✅ 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
|
os-project-manager
commented
Sep 3, 2026
Seat bookkeeping + the Clause ② call. ⛔ NOT a contract review.
⚖️ Open question RULED: A — the declaration stands as Clause ② YES.The card recorded "Clause ②: no expected (renderer-internal; re-declare from the diff)". The dev re-derived it against the actual diff and declared yes, because The criterion has two limbs — changes accept/reject behaviourorwidens the public surface. The second is met: a symbol that was package-internal is published, into a 45-package consumer closure. ⇒ ⭐ The reason I am not taking the softer reading: the card's own instruction was "re-declare from the diff", not "confirm the expectation". A
Verified on the branch by content, ⛔ not from the report
Call-site fallbacks intact and local, exactly as option C requires: ⭐ The trap this card named, caught in the actDropping ⭐ And two ablation attempts were refused by the harness rather than reported: injecting Two more worth the reviewer's eyeThe census was re-derived over a WIDER bound than the gate's own. All 4,101 tracked Ruling point 4 ships as mechanical enforcement, not a note. 「本裁定后新容器 ⛔ 不得再自带解析器」 is now held by the gate itself: it rediscovers record readers from source every run and fails in both directions, so a container that hand-rolls a lookup reds the commit that adds it. Out-of-scope #7492 filed (the gate's Generated by Claude Code |
os-project-manager
commented
Sep 3, 2026
Clause-② in-seat contract review — PASS WITH REQUIRED AMENDMENTS. |
…state the discovery bound (objectui#5935) Both required amendments from the Clause-② contract review of PR #7491 (comment 5524970713). The seam, the call sites, the alias and the five new suites are untouched. Amendment 1 — the two-copies pin is now BEHAVIOURAL. `scripts/__tests__/check-lucide-icon-record-names.test.ts` asserted only the resolver's SOURCE TEXT, so it was blind to the gate moving on its own: the resolver's text is then unchanged and every source-text row still passes. The review measured it — widening ONLY `toRecordKey` to `split(/[-_.\s]+/)` left the file green 40/40 — and that is the gate-wider direction, exactly the under-reporting class this card exists to close. The file now imports `describeIconLookup` from the resolver and asserts `toRecordKey(s) === describeIconLookup(s).key` over a corpus derived from the runtime record: every one of the 1,767 live keys re-spelled six ways, plus a per-character separator sweep. The sweep is not decoration — no key of the record contains a separator at all, so re-spellings alone cannot see a character-class widening, which is why the measured hole survived them. A second row is the control that the corpus can in fact separate the two, modelling the drifted gate by substitution on the shipped `toRecordKey` rather than transcribing the rule a third time. The source-text rows are kept; they catch a different thing. TypeScript is imported straight out of `packages/` the way `known-schema-types-derivation-5115.test.ts` already does, so the pin costs no build step, and by a RELATIVE path so `describeIconLookup` stays off the package entry and the published widening stays at one symbol. Amendment 2 — the enforcement bound is stated instead of over-claimed. The `DECLARED_RECORD_READERS` comment said a new container that hand-rolls a lookup turns the gate red. It holds for one syntactic shape. Measured against this gate: named `icons` import (rename included) plus element access on that binding, through parens/`as`/`!`, any argument expression — red. Namespace import, a binding re-bound through another const, property access, destructuring, `require()`/dynamic `import()`, and anything inside a string or template literal — green. objectui#7472 is cited as the known outside-predicate resolver and is outside three ways at once. The discovery predicate is ⛔ NOT widened; that is a separate design question. The Normalisation comment's "either side ... fails loudly" is likewise made true rather than reworded away: it now names the behavioural row that carries it and records that the source-text rows do not. Verification: gate exits 0 at 1 record-reading resolver; the 7 relevant suites 96 green (94 before, +2 rows); ablation red/green with the restore proven by state; `type-check:scripts`, `lint:root` (0 errors on changed files) and `check-control-bytes` green. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EMrWaQw3XS5DxTHxp4yRyC
✅ 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
|
os-project-manager
commented
Sep 3, 2026
Amendments 1 and 2 applied — |
| leg | reading |
|---|---|
| pre-mutation, on disk | shipped .split(/[-_\s]+/) present 1, mutated form present 0 |
| post-mutation, on disk | shipped 0, mutated 1; blob 4985efe76 vs HEAD e1600563f; git diff HEAD numstat 1 1 |
| gate script under the mutation | exit 0, still "1 record-reading resolver" — the drift is invisible to the gate's own run |
| pin suite under the mutation | 2 failed, 40 passed (42) — the two new rows, and only those |
| restore | git checkout HEAD -- ABSOLUTE_PATH under trap ... EXIT INT TERM; git diff HEAD empty and blob back to e1600563f |
| green leg on the restored tree | 42 passed (42) |
The 40 pre-existing rows staying green under the mutation is the review's finding reproduced exactly. The .mjs Normalisation comment is made true rather than reworded away: it now names the behavioural row that carries "either side fails loudly" and records that the source-text rows do not.
Amendment 2 — the enforcement bound is stated
Replaced at DECLARED_RECORD_READERS, and the same over-claim corrected in the PR body. Every line of the new comment is measured against discoverResolvers, not reasoned about — 19 syntactic shapes through the real predicate:
Inside (red): named icons import; a renamed named import ({ icons as reg }); (icons as Record)[k]; and any argument expression, a template literal included — the predicate never looks at the argument.
Outside (green, not caught): namespace import; a binding re-bound through another const; property access (icons.House); destructuring; require(); dynamic import(); and anything inside a string or template literal.
app-generator.ts emits, inside a template literal, import * as LucideIcons from 'lucide-react' re-bound to a lucideIcons const and then indexed. ⛔ The discovery predicate is not widened.
Verification, on the pushed head
- 7 relevant suites: 7 files, 96 tests passed (the review's 94, plus the 2 rows). This file alone 42, was 40.
- Icon union re-run at
089e35043: 18 files, 184 tests passed — 182 plus the same 2. Re-run because a push after review invalidates a sha-stamped union reading. - Every other suite naming the edited gate script — entry-guard wiring, scripts type-check parity, breadcrumb / command / context-menu item icons,
icon-renderer-declared-default,ViewSwitcher: 7 files, 62 tests passed. node scripts/check-lucide-icon-record-names.mjs— exit 0,OK lucide icon names: 182 authored/declared names reaching 1 record-reading resolver are live icons keys.pnpm type-check:scripts— VERDICT command-exit 0.pnpm lint:root(the scope that ownsscripts/) — 0 errors, 32 warnings, 0 of them in either changed file; all 32 are pre-existingno-explicit-anyin e2e specs, two unrelatedscripts/__tests__files and two vitest setup files.node scripts/check-control-bytes.mjs— OK, 6,158 tracked text files.
Exit codes were captured by redirecting first (cmd > log 2>&1; EXIT=$?), never after a pipe, and each verdict is quoted from the line the tool printed.
/ escapes — a literal one is a character no reviewer can see and no grep can be aimed at. Verified: 0 invisible or control characters in either changed file.
The changeset was checked and not edited: it makes no claim about the discovery predicate, and its "the under-reporting is closed rather than merely bounded" is what amendment 1 makes true.
⛔ Not done, deliberately: PR not flipped ready, no auto-merge, needs:contract-review left on. The package: components / plugin / tests labels appeared from the labeler, not from me.
Generated by Claude Code
os-project-manager
commented
Sep 3, 2026
✅ Amendments verified — |
Uh oh!
There was an error while loading. Please reload this page.
…ecord the third zero objectui#5936 asked for the consumer of a component registration's `icon` meta before extending the icon gate to it. Three first-party populations were measured and none has one (objectui, objectstack, cloud), so the gate is not extended — adjudicated 2026-09-04. Records that where the next reader hits it (the gate's own "Deliberate boundaries"), with the three readings, their controls, the adjudication, and the caveat that three zeros are not a proof of absence. Retires the membership half of the `ui:icon` local pin, which was kept at objectui#5633 on the premise that the palette lives outside this repo. It guarded 1 of the 45 registrations declaring an `icon` meta while the gate is adjudicated not to cover the other 44, and its hand-copied tokeniser had already gone stale against the seam objectui#5935 / PR #7491 created. The coupling half stays: it needs no external vocabulary and cannot drift. The published `ComponentMeta.icon` key is untouched. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KbJQ1y1J12nZxYzFWhP8Q3
Fixes#5935
Seven modules resolved authored lucide icon names, each with its own copy of the logic. There is now one,
resolveIcon, and the other six call it.Both rulings on the card execute here as written:
iconNameMap, one seam; and point 4, 「本裁定后新容器 ⛔ 不得再自带解析器,一律走 seam」.name -> componentand nothing else. NoonUnresolvableparameter. Each call site keeps its own fallback.The census, re-measured before starting
Bound declared first, because this card has been handed a wrong number four times: all 4,101 git-tracked
.ts/.tsxfiles, whole repo, no root subset — wider than the gate's ownSCAN_ROOTS(3,945 of them). Re-run of the gate's own AST discovery over that wider set.c1fe272ad)e307c9896Home->HousealiasRECORD_READING_TYPESZero drift. The only difference from the ruling's measurement is the scanned-source count (3,945 vs 3,943 — two files added to the tree since).
What changed at each of the seven sites
renderers/action/resolve-icon.tssplit('-')+ aliassplit(/[-_\s]+/)+ aliasrenderers/basic/icon.tsxtoPascalCase+ own map + own record indexSquareDashedplaceholder + warning byte-identicalapp-shellpreviews/ActionPreview.tsxsplit(/[-_\s]+/)+ inline ternaryplugin-detail/RelatedList.tsxsplit(/[-_\s]/), no aliasresolveIcon(name) ?? Inboxplugin-list/ListView.tsxsplit('-'), no aliasresolveIcon(iconName) ?? Inboxplugin-list/components/TabBar.tsxsplit('-'), no aliasnullplugin-view/ViewSwitcher.tsxtoPascalCase+ own mapnullfor views and its own `resolveIconis exported from@object-ui/components, the one package all five consumers already depend on — no new dependency edge.The tokeniser was not a choice made in this PR
split(/[-_\s]+/)with the rename universal, established by the pre-dispatch enumeration the 2026-08-31 ruling required (comment 5522254814): regression set empty against the authored population, against an every-name-by-every-surface cross-product, and against a bound-free differential over 8,298 spellings derived from all 1,767 live record keys.split('-')regresses 4,748 name-surface pairs in that last reading and is not adoptable.Widening only, and structurally so: no record key contains
_, whitespace or-(0 of 1,767, with a firing control), so whenever the narrow tokeniser produced a live key the wide one produces the same key. The old resolving sets are strict subsets. Sixteen name-surface pairs start resolving; none stops. Stated in words in the changeset.⭐ The load-bearing claim: zero user-visible change on all four unresolvable behaviours
Each of the four is pinned, and each pin is proven non-vacuous two ways — a control in the same run that asserts the opposite state, and a red-then-green ablation on the committed baseline
675571f8f.nullTabBar,ViewSwitcher(+ its own `SquareDashed+ warnbasic/icon.tsxInboxglyphRelatedList(header and toolbar),ListViewempty stateRelatedList.iconSeam-5935.test.tsx,icon-seam-fallbacks-5935.test.tsxActionPreviewActionPreview.iconSeam-5935.test.tsxThe ablation, on the committed baseline, restore proven BY STATE (
git diff HEADempty and blob equality against theHEADblob;trap ... EXIT INT TERMwith absolute paths; restore pinned togit checkout HEAD -- path, never the bare form that reads back from the index). Each mutation was proven on disk in both directions — removed text gone, injected text present — before its run was read:split('-')Home->Houseentry disabledRelatedList:?? InboxdroppedListView:?? InboxdroppedActionPreview: chip emptiedicon.tsx: placeholder glyph swapped outRelatedList's?? Inboxalso turned its CONTROL row red — the component threw and drew nothing, exactly the "a test that passes because a component failed to render" failure this card warned about. That is why every fallback describe opens with a row proving the surface rendered and asserts its fallback positively, rather than only asserting an absence.Two earlier ablation attempts were refused by the harness rather than reported, and are named so the numbers above are readable: injecting
SquareDashedNOPEleft the removed text present as a substring (both-directions probe unsatisfiable), and injecting the empty string made the injected-text probe degenerate. Both were re-run with clean anchors.The gate, simplified in the same PR
scripts/check-lucide-icon-record-names.mjs:DECLARED_RECORD_READERSgoes 7 to 1, and its normalisation stops being a widest-common approximation. Its verdict line, before and after:Every part-2 counter is identical across the two runs (148 judged, 22 descendant-judged, 239 declined, 34 anchored) — the census moved, what is judged did not. The under-reporting disclosed at #5932 is closed, not merely bounded: with one resolver there is nothing left to approximate.
Ruling point 4 ships as enforcement within a stated bound. Discovery is re-run from source every run and disagreement fails in both directions.⚠️ Corrected here after the contract review measured the earlier wording as an over-claim: the predicate holds one syntactic shape — a named
iconsimport fromlucide-react(a rename counts) plus an element access on that binding, seen through parens,ascasts and!, with any argument expression. Measured red on that shape; measured green, i.e. not caught, on a namespace import, a binding re-bound through another const, property access, destructuring,require()/dynamicimport(), and anything inside a string or template literal. #7472 is the known outside-predicate resolver and is outside three ways at once. So a new container that hand-rolls a lookup in the conforming shape reds the commit that adds it; one written another way does not, and the ruling still binds it by review. The bound is now stated atDECLARED_RECORD_READERSitself. ⛔ The discovery predicate is not widened — that is a separate design question.The two-copies pin is behavioural, not textual. The gate's
.mjsnormalisation and the resolver's TypeScript one are held together by importingdescribeIconLookupand assertingtoRecordKey(s)equalsdescribeIconLookup(s).keyover a corpus derived from the runtime record — all 1,767 live keys re-spelled six ways, plus a per-character separator sweep — with a control that the corpus can in fact separate the two. The sweep is load-bearing: no record key contains a separator at all, so re-spellings alone cannot see a character-class widening. The source-text rows are kept; they catch a different thing. Measured need: widening only the gate tosplit(/[-_.\s]+/)left the suite green 40/40 before this, and reds 2 of 42 after it, with the gate script itself still exiting 0 under that mutation — which is exactly why the pin, not the gate, has to hold this.Verification
089e35043(the amendment commit): 18 files, 184 tests, all passing — the 5 new plus every pre-existing icon suite (ui:iconreads the SDUI identity keynameas its lucide icon name, so any icon node that authorsnamerenders nothing at all #5631's 13-test pin, finding(components):ui:buttoninlines its own copy of resolveIcon instead of importing it, so any future fix to the shared resolver silently misses it #5993'sbuttondedupe pin, breadcrumb/command/context-menu/dropdown-menu item icons, the gate's own suite, now 42 tests,TabBar.i18nLabel,ListView.emptyStateFilteredCopy,ViewSwitcher,ActionPreview.locations). That is the675571f8freading of 182 plus the 2 rows the behavioural pin adds, and it is quoted from a run on the current head rather than carried forward.pnpm exec turbo run type-check --filter='...@object-ui/components'— 66 tasks, all successful, over the 45-package DOWNSTREAM CONSUMER closure (prefix filter = consumers, the direction a contract change travels). The five new test files were confirmed present in each package'stsconfig.test.jsonprogram via--listFiles, so "type-check is green" actually covers them.675571f8f:icon-record-names,control-bytes,vi-mock-specifiers,vi-mock-inherit,readme-exports,phantom-deps,self-import,entry-guard,side-effects-array,sdui-registration-pins,eager-closure,dist-completeness,docs-route-closure.LucideIconimport, an unusedReactimport, and areact-hooks/static-componentserror on the ActionPreview call site, which now carries the same annotation the other five call sites do). 250 warnings, all pre-existingany/ react-hooks noise on untouched lines of two 4,000-line files.minoron the five packages whose source moved.Scope
Clause-②: yes — and this is a deliberate re-derivation against the ruling's stated expectation of
no. The 2026-09-03 ruling recorded «Clause-②: noexpected (renderer-internal; re-declare from the diff)», which holds for the half it was about: no contract accept/reject behaviour moves, no schema, validator or refusal changes, nothing an author may write changes. But the diff widens a published surface —resolveIconis a new export from@object-ui/components's public entry, which the ruling's "renderer-internal" reading did not anticipate and which is unavoidable because the five other call sites live in five other packages. Declaringnohere would have been an auditable false declaration, so it isyeswith the reason attached.⛔ #7472 stays out.
packages/cli/src/utils/app-generator.ts:842emits an eighth resolver inside a template literal, against lucide's named-export namespace, with zero normalisation. It is correctly absent fromDECLARED_RECORD_READERS(it reads neither censused vocabulary), and the gate's discovery predicate is not widened to see it.Nothing under
content/docs/releases/is touched. No gate threshold is raised or weakened.🤖 Generated with Claude Code
https://claude.ai/code/session_01EMrWaQw3XS5DxTHxp4yRyC
Generated by Claude Code