Uh oh!
There was an error while loading. Please reload this page.
fix(components)!: data-table reads only the declared accessorKey (retire the col.name alias) - #6963
Merged
Merged
Conversation
Retire the undeclared `col.name` tolerance on the data-table adapter, and
migrate the instruction corpus that taught it in the same commit.
`data-table`'s column normalization read `accessorKey: col.accessorKey ||
col.name` at two sites, but `TableColumn` (`@object-ui/types`) declares only
`accessorKey`. The declared surface admitted one spelling while the runtime
admitted two — the second de-facto contract AGENTS.md #0.1 forbids. The
maintainer ruling of 2026-08-20 settled the direction for the family: retire
the consumer-side alias, translate at the producers. `label` retired first;
this closes the family.
The two published skill guides that taught `{ name, label }` columns migrate
to the declared `{ header, accessorKey }` here, not in a follow-up. They are
executable fixtures — `skill-guide-data-table-binding.test.tsx` lifts their
fenced JSON at run time and renders it — so moving the runtime without the
instruction would leave the platform refusing a spelling it still ships.
Blast radius is adapter-only: columns arriving through ObjectDataTable,
RelatedList or ObjectGrid are unaffected, because each producer folds `name`
into `accessorKey` via `columnIdentity` before delivery. What narrows is a
column authored directly onto a `data-table` node.
Tests follow the change rather than being retrofitted to it:
- the HOLD pin in `data-table-declared-column-keys.test.tsx` becomes a
retirement pin, plus a legibility pin (live header over blank cells,
neighbour untouched) and a producer-resolved pin bounding the radius;
- the guide gate's docblock claimed it "stays true whichever way #5120
lands — nothing below asserts a column key spelling". That was false
(objectui#5479): its assertions are on cell text, so they transitively pin
the accessor spelling. Corrected, and an explicit spelling assertion added
so the failure names the key and the card instead of diffing cell arrays;
- `skill-guide-provider-envelope.test.tsx` carried a hardcoded `{ name,
label }` COLUMNS fixture mirroring the guides; migrated with them.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013hfmP9hoMd3dJwTh85J4yBContributor
✅ 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
|
This was referenced Aug 31, 2026
… fold it Contract review on PR #6963 caught a mechanism mis-attribution. The blast-radius CONCLUSION was verified correct — columns arriving through ObjectDataTable, RelatedList or ObjectGrid are all unaffected by the `col.name` retirement — but the stated REASON held for only two of the three. - ObjectDataTable and RelatedList genuinely resolve the legacy spelling, stamping `accessorKey` from `columnIdentity(col)` before delivery. - ObjectGrid does NOT fold. Since #5068 it refuses undeclared spellings at intake: `resolvesToDataColumn` requires a non-empty string `field`, so a name-spelled entry is dropped before delivery and never reaches the adapter at all — before and after this PR. What it delivers carries `accessorKey` stamped from `field`. This matters beyond pedantry: the 2026-08-20 ruling's item 2 told ObjectGrid to "connect to the same columnIdentity resolution", and #5478 measured that following it literally would have re-widened exactly what #5068 narrowed. The claim has now been restated in the wrong direction by two separate seats. A third repetition is how it becomes settled fact, so it is corrected in the durable artifacts rather than only in a review thread. Corrected in the changeset (author-facing, and the one that would have shipped the false mechanism into release notes), and in the two test comments that carried it. Comment and changeset text only — no assertion, no source line, and no behaviour changes; the three affected pin files stay 44/44 green. 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
|
This was referenced Aug 31, 2026
os-zhuang
approved these changes
Aug 31, 2026
os-zhuang
marked this pull request as ready for review
August 31, 2026 07:30
Uh oh!
There was an error while loading. Please reload this page.
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.
Fixes#5120
Last step of the 2026-08-20 maintainer ruling on the column-alias family: the
data-table adapter now reads only the declared
accessorKey, and the twopublished skill guides that taught the undeclared
namespelling migrate inthe same commit.
Three-quarters of this card already landed (#5353 producer identity, #5478
labelretirement +columnHeader, #5350 the statictablerenderer). This isthe held quarter.
⛔ Governed surface — human merge only
The diff touches
skills/**.check:governed-queue-guard --teston this exactfile list answers, mechanically:
So this PR stays draft. Not marked ready, not enqueued, no auto-merge. The
merge-queue guard refuses it without an approval from
GOVERNED_APPROVERS(
os-zhuang,hotlong) pinned to the current head. Card #5120 additionallycarries
needs:contract-review(the label sits on the card, not on this PR);that review has since returned ACCEPT-WITH-CONDITIONS, and both conditions
are addressed in
8be968586.What changed
packages/components/src/renderers/complex/data-table.tsx— bothaccessorKey: col.accessorKey || col.namereads, found by content at:870and:882(the ruling's:777/:786are from 2026-08-18 and hadshifted twice; they now sit at
:875/:887after the docblock rewrite).skills/objectui/guides/data-integration.mdandschema-expressions.md— the4 column entries move
{ name, label }→{ header, accessorKey }, matchingthe declared spelling
schema-expressions.mdalready teaches fortable.Blast radius — adapter-only, and bounded by measurement
A
{ name, label }column arriving through ObjectDataTable, RelatedList orObjectGrid is unaffected — but for two different reasons, and contract
review caught me conflating them (condition 1, corrected in
8be968586)::463-484) and RelatedList (:985-988) resolvethe legacy spelling, stamping
accessorKeyfromcolumnIdentity(col)beforedelivery.
ObjectGridSchema.columns上容忍未声明的accessorKey/header拼写 —— 声明类型是 strict 的 ListColumn,#3104 的列身份门禁结构上看不见这条支路 #5068 it refuses undeclared spellings atintake —
resolvesToDataColumnrequires a non-empty stringfield(
columnSpellingDiagnostics.ts:85-90), so aname-spelled entry is droppedbefore delivery and never reaches the adapter at all, before and after
this PR (that silence is finding(plugin-grid/docs): the published docs and skills teach
{ name, label }columns on a directly-authoredobject-grid, which ObjectGrid silently drops — before and after #5068 #5352's territory). What it delivers carriesaccessorKeystamped fromfield(ObjectGrid.tsx:2237).The conclusion — unaffected — holds for all three. The mechanism held for two.
Worth a commit rather than a shrug: the 2026-08-20 ruling's item 2 told
ObjectGrid to "connect to the same
columnIdentityresolution", #5478 measuredthat following it would have re-widened what #5068 narrowed, and the claim
has now been restated in the wrong direction by two separate seats. A third
repetition is how it becomes settled fact, so it is corrected in the changeset
(author-facing, and the copy that would otherwise ship into release notes) and
in the two test comments that carried it.
What narrows is a column authored directly onto a
data-tablenode.That bound is now a test, not a claim —
a PRODUCER-resolved column is unaffected — the narrowing is adapter-only.It is also why the
related[]example incontent/docs/api/schema-reference.md(a
detail-viewrelated list oftype: "table",{ name, label }columns) doesnot break: it reaches data-table through RelatedList, which resolves it. That
file is left byte-untouched (verified by blob hash), as is
renderers/complex/table.tsx.Census — pathspec bound stated, hot control carried
Two independent methods, both counter-probed.
Method 1 — bracket-matched extraction of every
columns: [...], split intotop-level entries, each classified by its enclosing node's
type.name-spelleddata-tablecolumnsexamples/(482),content/docs/(204),apps/(231),skills/(29),e2e/(31),packages/*/README.md(43)Same 4 at both bounds, all in the two adjudicated guides. Zero outside them.
entries. A zero elsewhere is therefore a reading, not a broken search.
accessorKeyentries found repo-wide, so theinstrument sees both spellings.
Other
name-spelled buckets are other vocabularies and were adjudicated, notwaved past:
grid(26 —nameis its declared field key),table(7 —separate renderer, #5350),
crud(7 — no registered renderer),list(5),untyped form/report arrays (84 —
fields[], master-detail, Excel export).Method 2 — enumerate every file containing a
data-tablenode, then readits column spellings directly. Same answer. It also cleared the two files whose
raw
namecounts looked suspicious:skills/objectui/rules/protocol.md(itsnamekeys are a flow action'sfields[], and itsdata-tableblocks use"columns": [...]as an ellipsis) andcontent/docs/utilities/vscode-extension.mdx(a form field and a bar-chart data row).
The prior measurement was re-measured, and it has changed
The 2026-08-21 reading was:
labelretired +nameheld = 15/15 green;retiring
name= 2 red.measurement never saw:
skill-guide-provider-envelope.test.tsx(added byfinding(components): the only working route from a provider
dataSourceinto adata-tableis thepropertiesenvelope protocol.md tells authors not to use #5372, after that reading) carries a hardcoded{ name, label }COLUMNSfixture mirroring the guides. It is not lifted from the guide bytes, so
migrating the guides does not fix it — it went red in the full package run and
is migrated here. Declared as a file-surface increment: it is
packages/componentstests, same defect class, mechanically forced by the guide migration.
Reverse verification — direction predicted before each run
No build artifact sits between edit and test: root
vitest.config.mtsaliasesevery
@object-ui/*specifier to that package'ssrc/(read out of the config,:274-303, not assumed). Each leg mutated from the committed state, provedthe mutation on disk by grep counts of injected and deleted text plus a blob
hash differing from the HEAD blob, ran, then restored with
git checkout HEAD -- PATHand proved restoration bygit diff HEADemptyand blob hash identical to HEAD. Both legs carried an absolute-path
trap … EXIT INT TERM.Leg 1 — alias restored, everything else kept. Predicted 2 red by name;
observed exactly those 2, 28 green:
does not resolve an accessor from 'name',LEGIBILITY: an unresolvable column keeps its header and spares its neighbour. The guide gate stayed 16/16green — the asymmetry that matters: once the guides spell
accessorKey, theyrender with or without the alias.
Leg 2 — guides reverted, alias kept retired. Predicted 3 red by name;
observed exactly those 3, 27 green: my new spelling assertion plus the two
the inline-'data' table puts its rows on screencases. This is there-measurement of the prior "2 red" split.
One honest miss: an annotation on leg 2 predicted
remaining(declared)=1andobserved 0 — my fixed-string probe carried a trailing comma the pre-existing
:513example does not have. The load-bearinginjected=2assertion held.Reported as observed rather than trimmed.
Tests
At
7538f0ac3, clean tree, from the repo root (a package-scoped run uses adifferent config than CI):
packages/components— 219 files / 2014 tests passedpackages/plugin-dashboard+plugin-detail+plugin-grid+core—404 files / 4874 tests passed (the producer direction; predicted green,
observed green)
pnpm --filter '@object-ui/components' type-check— exit 0, script echoed byname. It runs
tsc -p tsconfig.test.jsontoo, and--listFilesconfirms all3 edited test files are in that file set — measured, so "typecheck clean"
really covers the edits.
.ts/.tsxfiles — 0 errors, 35 pre-existing stylewarnings (
no-explicit-any×24 etc.).Lint narrowing, declared with its evidence (rather than a repo-wide run):
① the 2 guide
.mdfiles are outside eslint's population — eslint says soitself,
File ignored because no matching configuration was supplied;②
--format jsonreports 4 files judged; ③ type-aware linting is off(
--print-config→parserOptions: {}, noproject/projectService), so thisdiff cannot move the verdict on any untouched file.
Gate union re-run at
7538f0ac3on a clean tree, all exit 0:check:skills-paths·check:doc-types·check:doc-fences·check:control-bytes(5,785 files) ·check:self-import·check:phantom-deps·check:esm-specifiers·check:vi-mock-specifiers·check:governed-queue-guard·check:doc-snippets(271/271 blocks judged, 0 failed).check:doc-snippetsfirst exited 2, which is not a red gate — its ownverdict line says
PRECONDITION NOT MET … The snippet program was NOT run. Builtwhat it names, re-ran, green. Recorded so the 2 is not read as a failure.
The ruling clause that contradicts the tree — reported, not silently dropped
The ruling's item 2 says all three producers "connect to the same
columnIdentityresolution", naming ObjectGrid. #5478's dev measured that ObjectGrid must not
(it would re-widen what #5068 narrowed), and the dispatching seat accepted that
correction. This PR implements the ruling's direction and does not touch
ObjectGrid — consistent with that accepted correction, and recorded here so the
next reader does not re-derive the wrong action.
Changeset
minor, graded and justified in the file itself: this narrows the acceptedinput set, which is breaking for any author who used the tolerated spelling —
so it is not
patch, and it is written in author-facing terms with thebefore/after and the recognisable failure shape (live header over blank cells),
not as "internal cleanup". Not
major, per this repo's fixed-group convention.Also corrected here
skill-guide-data-table-binding.test.tsx's docblock claimed it "stays truewhichever way #5120 lands — nothing below asserts a column key spelling". That
was false (filed as #5479): its assertions are on rendered cell text, so
they transitively pin the accessor spelling. This is the coupling that made this
step invisible when the family was ruled on. Corrected, and an explicit spelling
assertion added so the failure names the key and the card instead of diffing
two cell arrays.
Authored by Claude Code in session
https://claude.ai/code/session_013hfmP9hoMd3dJwTh85J4yB(recorded in prosebecause editing a PR body rewrites the footer's session form).
Generated by Claude Code
Generated by Claude Code