docs(react): toRenderableSchema's header says the bridge is permanent (#4622) - #5602
Merged
Merged
Conversation
…#4622) The closing paragraph of `toRenderableSchema`'s JSDoc said the two competing repo-wide `SchemaNode` spellings "have not been reconciled" and that "when it lands, the call sites using this can go back to forwarding directly". Both halves went false when PR #4608 merged, and the second half is the harmful one: it is an instruction whose trigger condition has now fired, sitting directly above the function a future author is about to call. The reconciliation (objectui#4580 / PR #4608) resolved the collision in favour of `@object-ui/types`' union — `packages/core/src/types/index.ts:41` is now `export type { SchemaNode } from '@object-ui/types';` — while SchemaRenderer's prop stays deliberately narrow per objectui#4548 ruling Q2 (`schema: BaseSchema | string | null | undefined`, no `number` / `boolean`). So a `SchemaNode` became LESS assignable to that prop, not more, and the bridge is a permanent crossing between two intentionally different types. Following the old instruction has a measured cost, and the new paragraph names it: five `apps/site` call sites were forwarding directly when PR #4608 landed, and `Build Docs` was red on `main` for roughly five hours until PR #4621 routed all five through this function (objectui#4617). Comment-only. The emitted JavaScript is byte-identical (both revisions of the file transpiled with `tsc --removeComments`, sha256 ce9ba5c4ce7590dce63e5ca853cfb7a79d55df338d1fd32403b215b39cefda06 on each side), and no changed line falls outside the JSDoc block. The paragraph does ship, which is why a patch changeset is owed: this package builds with plain `tsc`, so the comment is carried into `dist/schema-input.d.ts`. Fixes #4622 Claude-Session: https://claude.ai/code/session_012u2pRjcqAYtoEjgr3wwhnK Co-authored-by: Claude <noreply@anthropic.com>
Contributor
✅ 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
|
…hip as bytes
The changeset on the previous commit said "the emitted JavaScript is byte-identical".
That is FALSE, and the way it became false is worth recording, because it is the same
failure mode this card exists to prevent — a confident claim that does not survive being
checked.
The measurement behind it transpiled both revisions with `tsc --removeComments` and
compared sha256s. That is a correct measurement of a build this repo does not perform:
`tsconfig.base.json:22` sets `"removeComments": false` deliberately, and this package
builds with plain `tsc`, so comments are emitted into `dist/*.js`. The flag measured away
the very thing being measured.
Re-measured by building the package the way the repo builds it, at both revisions, with
`dist/` cleared between runs:
dist/schema-input.js 1,486 B -> 2,316 B (1.45 KB -> 2.26 KB)
gzipped 850 B -> 1,246 B (0.83 KB -> 1.22 KB)
+830 B raw, +396 B gzipped
which reproduces CI's bundle-size rows for this PR (2.26 KB / 1.22 KB) and for two
sibling PRs cut from the same merge-base that do not touch this file (1.45 KB / 0.83 KB).
What IS true, and is what the changeset now says: all 18 differing lines in the emitted
file are JSDoc continuations, and the three executable lines are byte-identical, so the
growth is the paragraph and nothing else. The source change remains comment-only — the
`git diff -U0` structural check is unaffected and still holds.
The doc paragraph itself is unchanged: it makes no claim about emitted bytes, and the
size is not being chased. Roughly 0.4 KB gzipped is the deliberate price of not repeating
a five-hour `Build Docs` outage.
Claude-Session: https://claude.ai/code/session_012u2pRjcqAYtoEjgr3wwhnK
Co-authored-by: Claude <noreply@anthropic.com>
Contributor
✅ 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
|
…o stale PM ruling on the wording of the TS2322 clause: neither "against this function's return type" alone nor a bare `string | BaseSchema | null | undefined` alone, but both, with the literal spelling scoped as a historical quotation. The reasoning is the axis I had not weighed. Describing the type by its ROLE is self-updating — if `SchemaRendererProps['schema']` gains or loses a member tomorrow, the sentence stays true with no edit. Hardcoding today's spelling as a present-tense claim is NOT self-updating, and would have added a fresh way for this paragraph to go false, which is the exact defect the paragraph exists to fix. Attributing the spelling to what #4617's logs said in August 2026 is a fixed fact that cannot rot however the union evolves, and it hands a reader with the log open the exact string to match in one hop. - `number` against this function's return type — until PR #4621 routed all five - through here (objectui#4617). + `number` against this function's return type, spelled + `string | BaseSchema | null | undefined` in objectui#4617's logs — until + PR #4621 routed all five through here. The paragraph grew, so both byte figures in the changeset are re-measured rather than left stale — the same build, at both revisions, with `dist/` cleared between runs: dist/schema-input.js 1,486 B -> 2,377 B (1.45 KB -> 2.32 KB) +891 B gzipped 850 B -> 1,266 B (0.83 KB -> 1.24 KB) +416 B previously 2,316 B / 1,246 B for the shorter paragraph. All 19 differing lines in the emitted file are JSDoc continuations (was 18) and the three executable lines remain byte-identical, so the growth is still the paragraph and nothing else. Noted for next time, per the PM: quoting exact byte counts in a changeset couples it to the comment's length, which is what made this edit cost a re-measure. Still the right call here — the numbers are the honest replacement for a claim that was wrong — but a cost to choose deliberately rather than by default. Claude-Session: https://claude.ai/code/session_012u2pRjcqAYtoEjgr3wwhnK Co-authored-by: Claude <noreply@anthropic.com>
Contributor
✅ 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
|
Collaborator
Author
PM review — ACCEPT (card #4622)Gates. 22 named check runs read individually for This PR took two correction rounds, and both were worth itRound one — a false claim I caught, and the trap that produced it. The PR body and changeset asserted the emitted JavaScript was byte-identical, measured by transpiling both revisions with The root cause is worth recording for the whole repo: Both artifacts were then corrected with the experiment retracted in place rather than swapped out silently, which is the difference between a fix and a cover-up. Round two — a phrasing judgement I ruled on. The new paragraph cites #4617's failure as "a TS2322 naming The implementer then did better than the ruling required — it established that the role-based half is machine-enforced: the function is annotated The measurement, with a control
All 19 differing lines in the emitted file are JSDoc continuations; the three executable lines are byte-identical. The trade, stated plainly0.4 KB gzipped of shipped comment, deliberately accepted, to prevent a repeat of a measured 5h31m Generated by Claude Code |
os-sales
marked this pull request as ready for review
August 21, 2026 17:42
This was referenced Aug 21, 2026
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 free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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 #4622.
One paragraph of prose, at the bottom of
toRenderableSchema's JSDoc inpackages/react/src/schema-input.ts. The first two paragraphs are accurate and are untouched.What was wrong
The closing paragraph said the two competing repo-wide
SchemaNodespellings "have not been reconciled" and that "when it lands, the call sites using this can go back to forwarding directly". That is not a stale fact — it is an instruction whose trigger condition has already fired, sitting directly above the function every future author calls. Acting on it reproduces objectui#4617 exactly.Every claim in the replacement, verified against
origin/mainrather than taken from the cardThe card exists because a header made a confident claim that later went false, so each new claim was read out of the code on
7a28e1e3fbefore it was written down.1. The reconciliation landed, and core re-exports the union —
packages/core/src/types/index.ts:41:and the declaration it now points at,
packages/types/src/base.ts:334:2.
SchemaRenderer's prop is still deliberately narrow, and still excludesnumber/boolean—packages/react/src/SchemaRenderer.tsx, the interface and the sentence above it:So the two types are intentionally different, in the direction that makes the bridge permanent: a value typed
SchemaNodeis now less assignable to that prop than it was, not more.3. The ruling and PR #4608 agree with the code — from #4608's own must-not-change section: "
toRenderableSchemainpackages/reactremains. SchemaRenderer's component-level union deliberately excludesnumber/boolean(#4548 ruling Q2), so the bridge still normalizes those after reconciliation."4. The measured cost of the old reading — objectui#4617: five
apps/sitecall sites (InteractiveDemo.tsxtwice,LiveSplitDemo.tsx,SchemaThumbnail.tsx,playground/page.tsx) were forwarding directly when #4608 landed, each reported aserror TS2322: Type 'SchemaNode' is not assignable to type 'string | BaseSchema | null | undefined'. Merge timestamps bound the red window: #4608 merged2026-08-13T20:24:13Z, #4621 merged2026-08-14T01:55:18Z— 5h31m, i.e. the "roughly five hours" the new paragraph states. All five sites route through this function today (grepfortoRenderableSchemainapps/site).Nothing had moved, so there was nothing to stop for.
The replacement paragraph
Why the TS2322 clause is worded in two halves
A paragraph whose subject is a claim that rotted should not introduce a new way to rot. Naming the type by its role ("this function's return type") is self-updating: if
SchemaRendererProps['schema']gains or loses a member, that clause stays true with no edit. Naming today's spelling as a present-tense fact would not be — it would hardcode a type into prose, which is the defect this card exists to fix.So the literal spelling is scoped as a historical quotation of what #4617's logs said, which is a fixed fact that cannot go stale however the union evolves, and which hands a reader with the log open the exact string to match in one hop. The role-based half and the quoted half do different jobs; keeping both costs about twenty bytes.
The equivalence behind the role-based half is not folklore either: the function is annotated
: SchemaRendererProps['schema'], andSchemaRenderer.propsResolution.test.tspinsEqual<ReturnType<typeof toRenderableSchema>, SchemaRendererProps['schema']>, so a future edit that breaks the correspondence fails a type assertion rather than quietly falsifying this sentence.The source change is comment-only; the emitted artifact is NOT unchanged
Two different questions, and the first revision of this PR ran the second one wrong.
Retracted: the
--removeCommentsexperimentIt transpiled both revisions with
tsc --ignoreConfig --removeComments --noResolve --skipLibCheck, got matching sha256s, and concluded the emitted JS was byte-identical. That measured a build this repo does not perform.tsconfig.base.json:22sets"removeComments": falsedeliberately, and this package's build script is plaintsc, so comments are emitted intodist/*.js. Passing--removeCommentsoverrode the exact setting that decides the answer — it measured away the thing being measured. What that experiment does still establish is narrow and not what was claimed: the two revisions are identical once comments are stripped, i.e. no executable statement changed.The real measurement
The package built the way the repo builds it (
pnpm --filter @object-ui/react build,dist/cleared between runs), at both revisions, at the current head21d9d4726:packages/react/dist/schema-input.js7a28e1e3f)Cross-checked against CI's own bot. The bundle-size row is
wc -candgzip -c | wc -cof this very file (performance-budget.yml, "Generate package size report"), so it and this table measure the same artifact. At head012c57cb8the bot reported 2.26 KB / 1.22 KB forreact (schema-input.js)against 1.45 KB / 0.83 KB on two sibling PRs cut from the same merge-base that do not touch this file — and building that same tree locally reproduced 2.26 KB / 1.22 KB exactly. The current head is slightly larger (2.32 KB / 1.24 KB) because the paragraph gained the quoted spelling above; the before figure is unchanged and re-measured at 1,486 B / 850 B on this round.(One trap worth naming, since it moved a number:
gzipstores the source filename in its header, so gzipping a renamed copy of the same bytes gives a different size — 862 B fordist-before-schema-input.jsversus 850 B forschema-input.js. Every gzip figure above is measured at the realdist/path, which is what CI measures.)What did not change. All 19 differing lines in the emitted file are JSDoc continuations, and the three executable lines are byte-identical on both sides:
Structural check on the source, unaffected by the above and still sound — every added and removed line is a JSDoc continuation:
The size is not being chased. Roughly 0.4 KB gzipped is the deliberate price of not repeating a 5h31m
Build Docsoutage onmain; shrinking the paragraph to win those bytes back would trade the thing of value for the thing of no value.No test and no ablation is possible here, and none is manufactured. There is no executable behaviour to pin, and mutating a doc comment cannot turn any suite red — a "reverse verification" of this diff would be a ritual that proves nothing. The measurements above stand in its place.
Why a changeset, and why patch
The gate said so first, red before the changeset was written:
Patch rather than the empty frontmatter, because the paragraph genuinely ships — in
dist/schema-input.d.ts, where it is what an editor shows on hover at each call site, and indist/schema-input.jsas the bytes measured above. The changeset carries the corrected, re-measured numbers, not the retracted claim. Documentation of a published API changed; behaviour did not.No
skip-changesetlabel, deliberately — that label does not exist in this repo; PR #4621 recorded that it was one of the phantom workflows objectui#3724 deleted.content/docs/releases/**untouched.Gates
Re-derived from
.github/workflows/against this diff (packages/react/src/**plus.changeset/**), not taken from a list. Exit codes captured before any pipe, and each line below is the gate's own verdict.Re-run at the current head 21d9d47, working tree clean:
pnpm --filter @object-ui/react type-checktsc --noEmit && tsc -p tsconfig.test.json— no diagnosticspnpm --filter @object-ui/react lint✖ 347 problems (0 errors, 347 warnings)— all pre-existing; 0 attributable to the touched file (grep -c schema-input.tson the log = 0)pnpm exec vitest run packages/react/ --maxWorkers=2(repo root)Test Files 49 passed (49)·Tests 680 passed (680)check-control-bytes.mjs✅ check-control-bytes: OK (scanned 4654 tracked text file(s); skipped 85 binary).check-changeset-presence.mjs✅ 1 source file(s) of 1 released package(s) changed, and this change declares 1 changeset(s)check-changeset-no-major.mjs✅ No changeset declares a 'major' bump.check-changeset-fixed.mjs✅ All workspace packages are in the changeset fixed group.From head 012c57c, whose tree differs from the current one only in prose inside these same two files — no gate below reads that prose:
check-type-check-coverage.mjs✅ type-check coverage: 45/46 via 'type-check', 0 via their own build, 0 known-broken, 1 not compiled.check-lint-coverage.mjs✅ lint coverage: 46/46 packages linted, 0 with outstanding errors (0 total).check-doc-links.mjsLinks are valid across 13 scan roots.check-doc-component-types.mjs✅ Every documented component type is registered.check-doc-snippet-types.mjsSemantic phase: 88 of 88 block(s) judged, 0 failed.(needed the workspace built first — the unbuilt run reports a precondition, not a finding)check-skills-paths.mjs✅ check-skills-paths: OK (95/96 stated path(s) resolve across 18 guide file(s); 1 baselined).check-package-self-import.mjs✅ No package names itself inside its own src/.check-phantom-dependencies.mjs✅ Every in-scope import is declared by the package that publishes it.check-node-esm-load.mjs --specifiers-onlySpecifier leg: no un-ledgered package emits an extensionless relative specifier.check-spec-symbol-derivation.mjs✅ spec symbol derivation: 1289 files scanned against 4912 spec export namescheck-action-forward-parity.mjspayload excess-property CHECKEDcheck-i18n-call-site-keys.mjsEvery in-scope call-site key resolves against the en pack (2918 keys)check-i18n-en-drift.mjsNo en value changed in this range.Build Docsspecifically, given this card's history —apps/siteis the consumer that went red last time:The full workspace build the snippet gate needed also passed:
Tasks: 43 successful, 43 total.Declared narrowing
Repo-wide
pnpm lintwas not run; the package-scoped run above was, and it covers this diff completely. Three pieces of evidence, so this reads as a measurement rather than a gap:eslint.config.js:28selectsfiles: ['**/*.{ts,tsx}']. The changeset.mdis outside eslint's universe entirely, sopackages/react/src/schema-input.tsis the only lintable file in the diff.--format json—eslint --format json packages/react/src/schema-input.tsreportsfiles linted: 1,errors 0,warnings 0.tseslint.configs.recommended(notrecommendedTypeChecked) and sets noparserOptions.project, so linting is not type-aware and this diff cannot move a verdict on any file it does not contain.Repo-wide
pnpm type-checkand the fullpnpm testfarm were likewise left to CI, which runs both in full on this PR.Scope
packages/react/src/schema-input.ts(one paragraph) and one changeset. No other package, noapps/site, nocontent/docs/releases/**, no governed surface (docs/adr/**,.claude/**,skills/**,AGENTS.md,CLAUDE.md). Nogit stashat any point — the worktree is dedicated to this issue, and every before/after build comparison used a committed revision plusgit checkout <rev> -- <path>(or a patch file for an in-flight edit), restored and verified byte-identical by sha256 afterwards.Draft on purpose: the PM lands it.
Generated by Claude Code