Skip to content

fix(spec): re-measure the element:button icon record at objectui 9602dc820 — the describe named a normaliser the renderer no longer has - #13029

Closed
os-trump wants to merge 2 commits into
mainfrom
claude/issue-12968-button-icon-describe
Closed

fix(spec): re-measure the element:button icon record at objectui 9602dc820 — the describe named a normaliser the renderer no longer has#13029
os-trump wants to merge 2 commits into
mainfrom
claude/issue-12968-button-icon-describe

Conversation

@os-trump

Copy link
Copy Markdown
Collaborator

Fixes #12968

The .describe() on ElementButtonPropsSchema.icon shipped a clause that had gone false: it told authors the renderer resolves the name "using its own PascalCase normaliser and rename map". objectui#5993 deleted button.tsx's file-local toPascalCase + iconNameMap + icons index and routed the button through the shared resolveIcon every action:* site already used. The duplicate had been the defect in its own right — a rename added to resolve-icon.ts to absorb a lucide retirement reached every action site and silently missed ui:button.

Three steps, one edit, exactly as the card ruled: correct the prose, re-point both read-point records' anchors, and move exactly those two citations to `.objectui-sha` = `9602dc820`.

⛔ MUST READ FIRST — this PR cannot land on its own

Measured, not predicted. check-objectui-pin-citations classifies a citation as asserting and then requires pin.startsWith(citedSha). With this tree's .objectui-sha still at 190fbd01d0615e2e168faf9e08b8ad7844bc039d, the two moved citations do not prefix-match, so the gate reds. Its own verdict line, run on this branch:

✗ 2 spec source(s) assert an objectui pin this repo does NOT build against.

  .objectui-sha = 190fbd01d0615e2e168faf9e08b8ad7844bc039d

    packages/spec/src/ui/component.test.ts:2437 cites `9602dc820`
    packages/spec/src/ui/component.zod.ts:1825 cites `9602dc820`

That step runs in job typecheck-source-gates, which is a member of the TypeScript Type Check aggregator (OS_AGGREGATOR_MEMBERS in lint.yml) — a required context. So the merge queue will not land this PR by itself, and ⛔ auto-merge must not be armed on it.

The deadlock is mutual, and the resolution is coordination

tree .objectui-sha the two records cite gate
main today 190fbd01d 190fbd01d green
this PR alone 190fbd01d 9602dc820 red (measured above)
#12955 alone 9602dc820 190fbd01d red (the card's deliberately-accepted state)
both together 9602dc820 9602dc820 green

Neither PR is green in isolation; only the joint tree is. Two ways out, for the PM / maintainer to pick:

  1. Preferred — land them as one tree. Once this PR is reviewed and approved, the pin bump on chore(console): bump the objectui pin 190fbd01 → 9602dc82 so a stock console build resolves automation:packaged #12955 merges this branch into its own (or the reverse), so a single PR carries both the bump and the re-measured records. That tree is green, needs no override, and main is never red. The split ruling that created this card was about who authors and authorises the contract prose, not about which merge commit it rides in — that authorship has happened here, on this card, under domain:spec, so combining afterwards re-litigates nothing.
  2. Fallback — one accepted-red landing, in this order: chore(console): bump the objectui pin 190fbd01 → 9602dc82 so a stock console build resolves automation:packaged #12955 first, this PR second. chore(console): bump the objectui pin 190fbd01 → 9602dc82 so a stock console build resolves automation:packaged #12955 lands carrying the red the card already declares accepted (an admin merge, since the queue will refuse it); the moment .objectui-sha moves to 9602dc820, this PR is green and rides the queue normally. Exactly one override, and it is spent on the red that already has a ruling behind it.

⛔ The mirror-image order — this PR first — is available but strictly worse: it manufactures a second accepted-red that nobody has ruled on, and during the window main would tell authors about a shared resolver while the console it actually ships is built at 190fbd01d, where the button still carries its private copy.

I have flipped nothing and armed nothing. The review chain owns enqueue.

What was re-measured, and against what

Every anchor was re-derived by reading the objectui tree at 9602dc820450 (git show 9602dc820450:...), not carried over from the card:

record cites at 190fbd01d at 9602dc820 — confirmed
icon resolution button.tsx:44-47 button.tsx:36const Icon = resolveIcon(schema.icon);, and it now hops into a second file
the normaliser itself button.tsx:14-27 resolve-icon.ts:14-24 (toPascalCase at :14-19, iconNameMap at :21-24) and :30-35 (resolveIcon). button.tsx:14-27 is now imports plus the forwardRef opening
render, left / right button.tsx:69 / :71 button.tsx:57 / :59mr-2 h-4 w-4 / ml-2 h-4 w-4, unchanged
registration input list button.tsx:82-98 button.tsx:70-87 (label, variant, size, className; no icon), :88-92 is now defaultProps

One substantive correction beyond line numbers: the old prose said an unknown name resolves to undefined. resolveIcon ends return (icons as Record)[mapped] ?? null, so the record now says null. The author-visible outcome is the same — no glyph, no diagnostic — and the LazyIcon contrast survives intact (lazy-icon.tsx is byte-identical across the move; getLazyIcon still degrades an unknown name to the Database glyph).

The card's four "what did NOT break" facts were taken as given and not re-established, per the ruling.

Two-pin coexistence is intentional and holds

Exactly the two ruled records moved. The other citations in these files stay where they are: blanket-updating them would fabricate reviews that never happened. The gate's --list on this branch confirms the population — the six other asserting citations still naming 190fbd01d remain current, and the historical spelling is not checked at all. No record other than the two moved ones reds, so there is no fork to report on the card.

The one addition beyond the card's three steps

The #10053 pin block asserted only that the describe names lucide-react and LazyIcon. Both survived the clause going false — which is how it shipped false. Added one assertion, toContain('resolveIcon'), so the corrected claim is pinned by the block that exists to pin it.

Ablated to prove it can fail rather than assuming it: reverted the describe to the stale wording, confirmed the mutation landed on disk by occurrence count (injected clause 1, removed clause 0) and by blob hash (e86ff735… to 88aaef5a…), then re-ran:

AssertionError: expected 'Lucide icon name rendered inside the …' to contain 'resolveIcon'
    2491|     expect(shape.icon?.description).toContain('resolveIcon');
      Tests  1 failed | 229 passed (230)

Exactly the new assertion, nothing else. The restore leg was proven too, not assumed: git checkout HEAD -- ... (absolute path, trap-guarded), restored blob e86ff735c569aa73738dad72b8efbd624103267f equal to the HEAD blob, git diff HEAD empty, git status --porcelain carrying no MM.

Changeset: measured, not guessed

check:generated proved exactly one artifact stale — content/docs/references/** — and gen:docs moved one line of content/docs/references/ui/component.mdx, the customer-facing docs projection of this describe. A reader-visible artifact moves, so this is not a skip-changeset diff: patch on @objectstack/spec, text face. Regenerated by repo tooling only (check:generated --fix, which regenerated only the one it proved stale).

Gates

All runs below are on 5593b54e3, the pushed head (the ratchet-relevant families were re-run on it after the final amend; that amend touched the commit message only — git diff a080d453a 5593b54e3 is empty, so the earlier full sweep and this one read the same tree). Exit codes captured before any pipe, and each verdict quoted from the gate's own output rather than from a bare shell status. Union derived at this tree by node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack (which reads its own change set from the merge base — 4 paths).

Green (49): check:doc-authoring ("13306 customer-facing string(s) across 672 spec sources clean — no internal issue-id references (message 778 · strictObject 3282 · tombstone 751 · describe 8495)" — the widened Rule 3 holds for the new prose), check:generated (14/14 after the gen:docs regeneration), check:docs, check:authorable-surface, check:skill-examples, check:skill-refs, check:liveness, check:empty-state, check:strictness-ledger, check:variant-docs, check:doc-anchors, check:doc-formula-expressions, check:doc-security-posture, check:docs-audit-scope, check:docs-redirects, check:docs-single-h1, check:quick-reference-counts, check:role-word, check:published-readme-links, check:published-files, check:react-page-adapter-contract, check:spec-parsed-alias, check:slot-lookup, check:page-declaration-shape, check:test-source-alias, check:type-source-resolution, check:merge-driver, check:cross-package-test-inputs, check:objectql-double-limit, check:engine-double-contract, check:where-matcher, check:query-options-erasure, check:nul-bytes, check:changeset-gate-self-tests, check:objectui-changeset, check:pm-half-states, and the node scripts/… families (doc-frontmatter, doc-route-spelling, docs-section-name, section-landing-index, comment-mask-adoption, keyed-text-bounds, plugin-teardown-shape, undeclared-dep-imports, ci-filter-parity, adr-0087-registration, changeset-no-major, empty-changeset, release-rehearsal-clone --self-test, docs-audit/check-affected-docs, docs-audit/check-drift-comment).

Tests and types:

pnpm --filter @objectstack/spec exec vitest run --maxWorkers=2 src/ui/component.test.ts
  Test Files  1 passed (1)      Tests  230 passed (230)          exit=0

pnpm --filter @objectstack/spec typecheck                          exit=0
  check:test-typecheck: OK — @objectstack/spec's test layer compiles under
  packages/spec/tsconfig.test.json

Both edited files confirmed inside that tsc program rather than assumed — tsc -p tsconfig.test.json --listFiles lists src/ui/component.test.ts and src/ui/component.zod.ts once each, so "typecheck green" really covers this diff.

Red — one, and it is the subject of this PR:

  • check:objectui-pin-citationsred by design until the pin bump lands. See the deadlock section above.

Not measured (reported as such, never as green or red):

  • node scripts/pm/check-half-states.mjs — exit 3, its own PREREQUISITE-NOT-MET path: the container's GITHUB_TOKEN is a proxy placeholder, so "no issue was listed, no predicate (H1–H16) ran … it is no reading at all". A PM board sweep, unrelated to this diff.
  • node scripts/check-dev-prereqs.mjs — refuses on 66 of 67 workspace packages having no built dist/ in this worktree; a local-environment prerequisite, not a finding.

Repo-wide pnpm lint was not run locally and is left to CI, which runs the farm regardless.


Generated by Claude Code

claude added 2 commits August 28, 2026 17:18
…dc820

The `.describe()` on `ElementButtonPropsSchema.icon` told authors the renderer
resolved the name "using its own PascalCase normaliser and rename map". That
clause went false when objectui#5993 deleted `button.tsx`'s file-local
`toPascalCase` + `iconNameMap` + `icons` index and routed the button through the
shared `resolveIcon` every `action:*` site already used — the duplicate was
itself the defect, since a rename added to `resolve-icon.ts` to absorb a lucide
retirement reached every action site and silently missed `ui:button`.

Correct the shipped prose, re-point both read-point records' anchors at the new
pin (the resolution anchor now hops into `renderers/action/resolve-icon.ts`),
and move exactly those two citations to `.objectui-sha` = `9602dc820`.

Nothing an author may write changed: the key is still read, still rendered per
`iconPosition`, still suppressed while `loading`, and an unknown name still
resolves to nothing rather than a fallback glyph.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LpRNHxWZgSUgVnFT9mQQo4
@github-actions github-actions Bot added size/s documentation Improvements or additions to documentation protocol:ui tests tooling labels Aug 28, 2026
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

1 anchor(s) derived from 1 changed package(s); no hand-written page names any of them, so this run has nothing to listnot a clean bill of health. This check sees only pages that NAME a derived anchor: one that documents this change in prose, or enumerates it in an authoring dialect, names none and stays invisible to it on every run.

What this run could not see
  • the SDK route bridge reached 47 of 219 client-bound route-ledger rows — the other 172 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 172: 14 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 56 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 102 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.

Coarse fallback — 126 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json 0da7cd2c1114649755b7f9f45a7dbbe34985ac6epackageMentionDocs.

Which tree this was computed on

This run read content/docs from 07cd6864a94f7c2a11b31cdf9e0ad09e13d4b329 — the merge of head 5593b54e3680285b20e02d89348ba647e1d0dfae into base 0da7cd2c1114649755b7f9f45a7dbbe34985ac6e, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 07cd6864a94f7c2a11b31cdf9e0ad09e13d4b329 && git checkout 07cd6864a94f7c2a11b31cdf9e0ad09e13d4b329
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 0da7cd2c1114649755b7f9f45a7dbbe34985ac6e 5593b54e3680285b20e02d89348ba647e1d0dfae && git checkout -B drift-repro 0da7cd2c1114649755b7f9f45a7dbbe34985ac6e && git merge --no-ff 5593b54e3680285b20e02d89348ba647e1d0dfae

node scripts/docs-audit/affected-docs.mjs --json 0da7cd2c1114649755b7f9f45a7dbbe34985ac6e

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

Copy link
Copy Markdown
Collaborator Author

Closing as superseded — plan A executed. This PR's entire content (head 5593b54e3, both commits) is merged into PR #12955's branch as merge commit 262a921c5, on the maintainer's direct ask in the seat session (2026-08-29, 「13044 13030 13029 为什么不合并」) and per the plan-A record on #12687 (comment 5455935354). The deciding gate was re-run on the joint tree before pushing: check:objectui-pin-citations EXIT=0, "8 asserting objectui pin citation(s) match .objectui-sha (9602dc820)" — the joint tree is the only green configuration, exactly as measured.

#12968's closure now rides #12955's landing: the seat closes it with a pointer when #12955 merges (kept out of #12955's body to avoid a double-claim window against this PR while it was open, and recorded here instead).


Generated by Claude Code

@os-trump os-trump closed this Aug 29, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation protocol:ui size/s tests tooling

Projects

None yet

2 participants