Uh oh!
There was an error while loading. Please reload this page.
fix(types): model code-editor and bar-chart, repair three catalog fixtures - #6944
Merged
Merged
Conversation
… fixtures Part of #6318. Bucket B — two REGISTERED component types that `AnyComponentSchema` did not model, so `safeValidateSchema` refused every document naming them regardless of content. Both render today: `@object-ui/plugin-editor` registers `code-editor` and `@object-ui/plugin-charts` registers `bar-chart`. Every key on the new `CodeEditorSchema` / `BarChartSchema` (and their Zod mirrors) is taken from a read site — `plugin-editor/src/index.tsx:43-49` and `plugin-charts/src/ChartRenderer.tsx:28-38` — not from a view of what either component's authorable surface ought to be. Bucket A — three fixtures that were wrong about their own renderer, each confirmed by rendering it: `basic-select`'s third option spelled its label `type` (the open list showed two options, now three); `icon-toolbar`'s buttons carried only `icon`/`value`, neither of which `button-group` reads (three blank buttons, now labelled — the sibling `with-icons.json` already carried all three keys); `basic-tabs` gave its items no `value` and no `defaultValue`, so no panel could be selected (the tab content now paints). The remaining 28 entries in the bucket are NOT fixture bugs and are left untouched: each is a Zod declaration that contradicts what its renderer reads, reported on the card rather than swept. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013hfmP9hoMd3dJwTh85J4yB
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
|
…inor Two fixes to the review of #6318's PR. 1. objectui#6273 (the 2026-08-25 family ruling objectui#6172 / 甲-A1) went red: the two schemas this branch added to `@object-ui/types` collided by name with the pre-existing declarations in their owning plugins. The two shapes were compared STRUCTURALLY before choosing, not by reading: a tsc probe asserting mutual assignability in both directions, per pair, plus a per-member probe for `language` — the one member the two spelled differently (`'javascript' | … | string`, which TypeScript collapses to exactly `string`). Every relation held, and a deliberate counter-probe against an unrelated shape failed as required, so the green was a measurement rather than a vacuous assertion. Same heritage, same member set, same per-member types and optionality ⇒ these are re-points, not renames. Direction is forced: `@object-ui/types` is the lower layer and cannot import from a plugin without a cycle, so the plugins re-point at types'. `export type { X } from '@object-ui/types'` is a re-export, which the gate does not count as a second declaration. ⛔ `KNOWN_COLLISIONS` is untouched (shrink-only), and so is the gate's own test file — #6936 and #6937 both have unlanded edits to it. The published import paths are unchanged: `check:readme-exports` still resolves both README self-imports through the re-export (386 judged, 0 wrong-path, 0 fabricated), and `objectui check` reports a byte-identical file list, so the accept set of the validator did not move. 2. Regrade the changeset `patch` → `minor` for `@object-ui/types`, per contract review: the package's own precedent (the `visible` widen, and `ViewNavigationConfig`) grades a change of this size `minor` on the stated grounds that it is more than patch describes. Purely additive, so no migration note is owed beyond the regrade. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013hfmP9hoMd3dJwTh85J4yB
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
|
os-sam
marked this pull request as ready for review
August 31, 2026 03:32
Uh oh!
There was an error while loading. Please reload this page.
This was referenced Aug 31, 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 freeto 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.
Refs #6318. Parts 1 and 2 of that card land here; part 3 was not needed. The remaining 28 entries are filed as #6939, #6940 and #6942 — they are schema questions, not fixture bugs, and this PR deliberately leaves them alone.
Review follow-up (head
0e123f16c): theobjectui#6273one-authority gate went red on the first push and is fixed below under Name authority; the changeset is regradedpatchtominorper contract review. Session: https://claude.ai/code/session_013hfmP9hoMd3dJwTh85J4yBPremise re-derived — the counts moved, the finding did not
090927f4f. Re-measured on a clean checkout at40c479af2:090927f4f)40c479af2)objectui checkThe 19 that left were repaired by cards that landed in between —
button(14), toast/sonner (2),timeline(3),menubar(1),dropdown-menu(1) — and 2chartentries joined, which the card had not enumerated. The card's thesis is intact: two distinct causes, both real, bucket B still exactly 4 files / 2 types. Only the arithmetic decayed, so this ispremise_still_valid: truewith the delta stated.#6320 verified, not assumed
The card warns that in a built workspace every count roughly doubles, because the ignore list only excluded a top-level
dist/. #6320 was supposed to fix that. Measured here, both ways:packages/types/dist/: anchored624(unchanged), unanchored625.dist/directories holding 433 JSON/YAML files: anchored 624, unanchored 1057. The doubling reproduces exactly, and the anchored form is immune to it.objectui checkreportedAnalyzing 624 files/28 files carry a registered ObjectUI component type/Skipped 46 filesidentically on the clean tree and on the fully built one. The numbers in this PR are build-state-independent.Part 1 — bucket B (4 files / 2 types), no fork
code-editorandbar-chartboth RENDER —@object-ui/plugin-editorand@object-ui/plugin-chartsregister them — andAnyComponentSchemamodelled neither, sosafeValidateSchemarefused every document naming them whatever it said. Searchingpackages/types/src/zod/for az.literalnaming either type returned zero, as the card reported.Neither forced a product-shape decision, so neither is reported as a fork. Every key is a read site:
code-editor—packages/plugin-editor/src/index.tsx:43-49forwardsschema.value,schema.language,schema.theme,schema.height,schema.onChange,schema.readOnly,schema.classNameinto the lazy Monaco implementation; the registration'sinputs/defaultProps(same file, 64-77) name the same set;plugin-editor/src/types.tsdeclared the same interface, and now re-exports the one in@object-ui/types(see Name authority below).bar-chart—packages/plugin-charts/src/ChartRenderer.tsx:28-38forwardsschema.data,schema.dataKey,schema.xAxisKey,schema.height,schema.className,schema.color;ChartBarRendererProps.schema(same file, 12-21) declares exactly that set;plugin-charts/src/index.tsx:45-64registers the same five asinputsanddefaultProps;plugin-charts/src/types.tsdeclared the same interface, and now re-exports the one in@object-ui/types(see Name authority below).Two judgement calls, both anchored rather than invented:
languageisz.string(), not the registration's six-name enum. The renderer hands it to Monaco verbatim, and the plugin's own declaration already widens its list withor string. A picker shortlist and an accepted set are different claims.data— which the registration marksrequired: truefor authoring. The renderer readsschema.datawith no guard and the implementation defaults an absent array, so requiring it would refuse documents the renderer draws.Placement follows⚠️ The plugins no longer carry their own copy — that is the
markdownandkanban: a TS declaration in@object-ui/typesplus a Zod mirror, registered inzod-mirror-parity.test.ts'sMIRRORS.@object-ui/typeshas zero dependencies and cannot import either plugin, so this is the direction the layering allows.objectui#6273fix below, and it means there is exactly one declaration per name rather than a twin.Part 2 — bucket A: a verdict per group, and only three fixtures were wrong
Hand-triage turned out to be entirely practical, so part 3 (the type-keyed discriminated union) was not needed and is not in this diff. The card is right that
invalid_union — Invalid inputtells a reader nothing; that was solved offline, by resolving the union member from the roottypeand parsing against it directly, without touching the shipped union.Each verdict below was then checked the way #6318's triage asks — by rendering it through the real
SchemaRenderer— because a "correction" that renders identically proves the edit was wrong.Fixture wrong — 3 files, all fixed here
components-form-select/basic-select.jsontype;select.tsx:61renders{opt.label}and nothing elseOption 1 Option 2(24 elements)Option 1 Option 2 Option 3(25)components-basic-button-group/icon-toolbar.jsonicon/value;button-group.tsx:43readslabeland neither of the other two. The siblingwith-icons.json— which already validated — carries all three"", three blank buttonsCopyper buttoncomponents-layout-tabs/basic-tabs.jsonvalue(read attabs.tsx:52,53,63,64) and nodefaultValue(the registration marks itrequired: true)Tab 1 Tab 2— no panel selectableTab 1 Tab 2 Content 1basic-tabsis the one that shows why the render check is not decoration: addingvaluealone renders identically, sovalueplusdefaultValueis the single repair, not two.Schema under-declares — 10 groups, 28 files, reported not swept
Verdicts and evidence in full on the filed issues. Summary, with the render each "correction" would have produced:
tooltipchildrengives a blank tile; the fixture was already movedchildren→triggeronce on render evidence (objectui#4626)context-menutree-viewnodesanddatarender identically; the fixtures are the renderer's owndefaultPropskanbancardsrendersTo Do 1+ the card;itemsrendersTo Do 0+No cards. Two declarations of one component disagreefilter-builderchartobject-mapobjectNamerenders identically —staticDatastill wins the three-arm resolverobject-ganttdata-tabletrueand[]render identicallytextsmall,body,h1and no-variant all render byte-identically — theui:textrenderer reads no variant at all⛔ The 54th file (
packages/vscode-extension/schemas/objectui-schema.json) is untouched: it is the known pinned false positive. ⛔ The 36-file adjacent measurement is untouched.Name authority — the
objectui#6273fixAdding
BarChartSchemaandCodeEditorSchemato@object-ui/typescollided with the pre-existingdeclarations in their owning plugins, and
Test (shard 4/4)went red onscripts/__tests__/one-authority-per-exported-name-6273.test.ts— the 2026-08-25 family ruling(objectui#6172, decision 甲/A1): every exported schema name has exactly one authority.
Rename or re-point was decided structurally, not by reading. A tsc probe asserted mutual
assignability in both directions for each pair, plus a per-member probe for
language— the onemember the two spelled differently, where
'javascript' | … | stringcollapses to exactlystring:Local extends OwnerOwner extends LocalBarChartSchemaBaseSchemaCodeEditorSchemaBaseSchema(asserting
BarChartSchemaandChartSchemaare the same type) failed as required withType 'true' is not assignable to type 'false'.A second counter-probe on the consumer path rejected
height: '300px'withType 'string' is not assignable to type 'number'.Direction is forced.
@object-ui/typesis the lower layer and cannot import from a plugin withouta cycle, so of the two possible authorities only one is legal: the plugins re-point at types'.
export type { X } from '@object-ui/types'is a re-export, which the gate does not count.⛔
KNOWN_COLLISIONSis untouched — it is shrink-only and fails in both directions. ⛔ The gate's owntest file is untouched; #6936 and #6937 both carry unlanded edits to it.
Nothing published moved.
check:readme-exportsstill resolves both README self-imports through there-export (
386 self-imports judged (386 real, 0 wrong-path, 0 fabricated)), the built.d.tsstillexports both names from the plugin entry points, and
objectui checkreturns a byte-identical filelist — so the schema still models exactly what the renderer reads, which was bucket B's whole point.
Routing and the declared-equals-enforced control
Recorded from contract review rather than claimed here:
the rule keys on. "These components already render, the union just never modelled them" is a true
description of the change but does not defeat the routing, and this PR does not lean on it as if
it did.
NEWLY_PASS=4,NEWLY_FAIL=0, against a reversed hot control ofREVERSED_CONTROL_NEWLY_FAIL=4. The control is theload-bearing half — without it the
NEWLY_FAIL=0reading would be indistinguishable from a probe thatcannot fail. So this fix does not introduce the defect class the card exists to find.
Verification
Exit codes captured before any pipe; each gate quoted by its own verdict line.
pnpm --filter @object-ui/types type-check(hyphenated) — echoedtsc --noEmit && tsc -p tsconfig.examples.json && tsc -p tsconfig.test.json, exit 0.pnpm exec vitest run packages/types/ examples/schema-catalog/, run at08571fe50:40c479af2, byte-identity proven by blob hash):Test Files 93 passed (93)/Tests 2767 passed (2767)Test Files 94 passed (94)/Tests 2775 passed (2775)+1 file / +8 tests= exactly the new pin file. Sanity check against the target's own count:find packages/types examples/schema-catalog -name '*.test.ts*'= 94, matching; vitest root reported as the repo root, so this is not theapps/consolesubstitution (that would read 85/22).pnpm exec eslint .in each touched package, plain form:packages/typesexit 0 (260 pre-existingno-explicit-anywarnings, none on a changed line);examples/schema-catalogexit 0 (1 pre-existing warning).node scripts/check-changeset-presence.mjs— exit 0:✅ 10 source file(s) of 3 released package(s) changed, and this change declares 1 changeset(s). The changeset is gradedminorfor@object-ui/typesper contract review — the package's own precedent (thevisiblewiden, andViewNavigationConfig) grades a change of this sizeminorbecause it is more than patch describes. Purely additive, so no migration note is owed beyond the regrade.pnpm changeset:check— exit 0.objectui check, its own verdict lines:⚠️ 35 files carry a registered ObjectUI component type but did not validate as an ObjectUI schema:/Skipped 46 files/✓ All checks passed⚠️ 28 files carry a registered ObjectUI component type but did not validate as an ObjectUI schema:/Skipped 46 files/✓ All checks passedpackages/types/src/**,examples/**,.changeset/*), all exit 0:check:spec-symbols,check:self-import,check:phantom-deps,check:side-effects-array,check:esm-specifiers,check:element-data-source-declaration,check:doc-types,check:control-bytes,check:node-esm-load,check:entry-guard,changeset:check, andcheck:readme-exports.check:readme-exportswas RED on first read with 312 findings, every one of themits type entry ./dist/index.d.ts is not on disk — run pnpm build first, and none inpackages/types. That is the unbuilt-closure false RED:.github/workflows/readme-exports.yml:92builds./packages/*before running it. Rebuilt and re-read: exit 0,0 unbuilt.check:published-distis RED, on a file this diff does not touch —@object-ui/fieldsshipsdist/__tests__/numberInputBrowserReadings.d.ts. Present at40c479af2, so pre-existing onmain. Filed ascheck:published-distis RED onmain:@object-ui/fieldsshipsdist/__tests__/numberInputBrowserReadings.d.tsin its tarball #6943.The new pin (
examples/schema-catalog/test/safe-validate-corpus-6318.test.ts, 8 tests) asserts the seven repaired entries validate, and pairs that with counter-probes on declared keys —code-editorwiththeme: 'solarized',bar-chartwith a stringheight— so.successis not satisfied by a mirror that accepts everything. It deliberately does not pin the size of the remaining bucket, which would turn red on whichever card repairs one of the 28.Re-verified at
0e123f16cpnpm exec vitest run scripts/__tests__/one-authority-per-exported-name-6273.test.ts— wasTest Files 1 failed (1)/Tests 1 failed | 10 passed (11), nowTest Files 1 passed (1)/Tests 11 passed (11), exit 0.pnpm exec vitest run scripts/ packages/plugin-charts/ packages/plugin-editor/ packages/types/ examples/schema-catalog/—
Test Files 219 passed (219)/Tests 5523 passed (5523), exit 0.pnpm --filter @object-ui/types type-check— exit 0; both re-pointed pluginstype-checkandbuild— exit 0, and the re-export reaches the shipped.d.ts.examples/schema-catalog/test/safe-validate-corpus-6318.test.ts—Test Files 1 passed (1)/Tests 8 passed (8), exit 0.pnpm exec eslint .inpackages/plugin-chartsandpackages/plugin-editor— exit 0 each(267 and 5 pre-existing warnings, 0 errors).
check:spec-symbols,check:self-import,check:phantom-deps,check:esm-specifiers,check:side-effects-array,check:control-bytes,check:node-esm-load,check:readme-exports,check:changeset-presence,changeset:check.check:published-distremains RED on the same unrelated pre-existing file(
@object-ui/fields, filed ascheck:published-distis RED onmain:@object-ui/fieldsshipsdist/__tests__/numberInputBrowserReadings.d.tsin its tarball #6943) — unchanged by this commit.scripts/__tests__/check-sdui-registration-pins.test.tsfails only on a fully built tree, because the derivation picks
packages/app-shell/dist/console/connect/ConnectAgentWidget.jsover thesrc/…tsxit asserts.Removing
packages/app-shell/distturns it green; it does not touch this diff, and CI's test jobnever builds. Already filed as finding(test-infra):
check-sdui-registration-pins.test.tsfails on any tree wherepackages/app-shell/distexists #6893 — searched before filing, so no duplicate was raised.Filed rather than swept
safeValidateSchemabecause the Zod mirror names a key its renderer never reads (8 components) #6939 — 24 entries, 8 components: the Zod mirror names a key the renderer never readsDataTableSchema.rowActionsisz.arrayin the zod mirror andbooleaneverywhere else — the mirror is the outlier #6940 —DataTableSchema.rowActions:z.arrayin the mirror,booleanin its own TS twin, the renderer, the registered input and the docblockTextSchema.variantis a nine-value enum that theui:textrenderer never reads — declared-but-unenforced, and it refuses 5 catalog entries #6942 —TextSchema.variant: a nine-value enum with no read site, refusing five entries; the enforce-or-remove decision is the maintainer'scheck:published-distis RED onmain:@object-ui/fieldsshipsdist/__tests__/numberInputBrowserReadings.d.tsin its tarball #6943 —check:published-distred onmain(unrelated to this card)⛔ Draft, not enqueued, no auto-merge — the PM lands it.