fix(plugin-dashboard): resolve column identity at the producer, before data-table (part of #5120) - #5353
Conversation
…e data-table
`ObjectDataTable.normalizeColumns` returned object columns raw, so a column
authored in the spec-canonical spelling (`{ field: 'stage' }`) reached the
data-table adapter with no `accessorKey` and rendered a header over
`row[undefined]`. `computeLookupExpand` missed the same column for the same
reason, dropping `field`-spelled lookups out of `$expand`.
Identity is now resolved once, here, via the shared `columnIdentity` reader and
stamped onto `accessorKey` — the adapter's own key, which `@object-ui/core`
deliberately holds outside the metadata identity fold. Metadata vocabulary in,
adapter vocabulary out; one translation, one place (objectui#5022, #5068).
An author-supplied `accessorKey` is never overwritten, an already-canonical
entry is returned by reference, and an entry with no resolvable identity is
returned untouched.
Part of #5120 — the consumer half (retiring data-table's undeclared `col.name`
alias) is held pending the maintainer's ruling; the census found real authorized
usage of that spelling in the published skills guides.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RV6yuVCxymHYE16PL9vQkE
✅ Console Performance Budget
📦 Bundle Size Report
Size Limits
|
|
ACCEPT — PM review, round 17.
Both deviations from my dispatch text are forced, and both are right
What it delivers, and why it stands whichever way the fork is ruled
Measured alias-independent (leg B: both alias sites cut → 234 files / 2122 tests green), so it is a prerequisite for the deletion rather than a substitute: it is what stops The leg that matters most, and it is not the one that went redLeg A predicted 5 red / 7 green by name and observed exactly that. Then the second half: with the producer reverted, the pre-existing suite stayed green — The census, done properlySix corpora ( And the finding that stopped the deletion: Legibility, measured rather than assumedAn unresolvable column is not dropped and does not throw — header over empty cells, neighbour unaffected. Not quite silent either, and that is worse: the adapter keys each cell by the accessor ( Three findings filed rather than folded in: #5350 (the static Merging via the queue. #5120 stays open in the decision box — Q1 (retirement order vs. the corpus that teaches it, decided together with #5350) and Q2 ( Generated by Claude Code |
1 participant
Part of #5120
Deliberately
Part of, not a closing keyword: only one of this card's two halves is here. The other half went back to the inbox because the card's own census-first fork clause tripped. Details below.What landed — the producer half
ObjectDataTable.normalizeColumnsconverted thestring[]shorthand and returned every object column raw.data-tableis an adapter, and its column key isaccessorKey— a key@object-ui/coredeliberately holds outside the metadata identity fold, wherecolumn-identity.tsnames itTABLE_ADAPTER_COLUMN_KEY. So a column authored in the spec-canonical spelling reached the adapter carrying noaccessorKeyat all, and the widget rendered a header overrow[undefined]: every cell blank, nothing said.computeLookupExpand's$expandwhitelist resolvedc.accessorKey || c.nameand missed the same column, so afield-spelled lookup also lost its related record and showed a raw FK id.Identity is now resolved once, in the producer, through the shared
columnIdentityreader, and stamped onto the adapter's own key. This is the move #5022 made inRelatedListand PR5345 (#5068) generalized inObjectGrid, whose docblock states the rule in one line: metadata vocabulary in, adapter vocabulary out; one translation, one place.Three rules carried over verbatim from
RelatedList, each pinned:accessorKeyis never overwritten, even against a divergentfield— a deliberate divergence between the table slot and the metadata key belongs to the author;tablewidget with inline static data crashes into the error boundary — "Maximum update depth exceeded" #4618, and this widget rebuilds its node every render);What did NOT land, and why — the census-first fork clause tripped
The card also rules that
data-table's undeclaredcol.namealias retires atdata-table.tsx:777/:786. That deletion is gated on a census, and the census found real authorized usage.Two files in the published
skills/corpus — the authoring guide agents read to write ObjectUI metadata — instruct authors to spell adata-tablecolumn exactly the way this card would retire:skills/objectui/guides/data-integration.md:185-192skills/objectui/guides/schema-expressions.md:377-384Both carry the identical example: a node of type
data-tablewith"bind": "customers"and"columns": [{ "name": "name", "label": "Name" }, { "name": "email", "label": "Email" }]. That is thenamelimb, reachingdata-table.tsx:777directly rather than through any of the three producers. Per the clause — real authorized usage found, stop, report, inbox — the deletion is the maintainer's call, so it is not in this PR.Census method and corpora
Corpora swept:
examples/(469 files),content/docs/(202),apps/(226),skills/(29),e2e/(33), andpackages/*/README.md(39) — the READMEs included from the start, per the lesson recorded from #5068's sweep.Three independent methods, each counter-probed with a term known present, through that same method:
.json, walks to eachcolumnsarray, records the enclosing node'stype, classifies each entry by identity keyaccessorKey→ 25 entries foundcolumns: [ … ]in.ts/.tsx/.md/.mdx/.yml, splits top-level entries, records nearest precedingtypeaccessorKey→ 11 + 10 entries;field→ 34 + 22typetoken, reads the next 25 lines, reports spellings presentaccessorKeyexample JSONs and the READMENo method was discarded — all three controls held. Method 3 produced three false positives that method 2 correctly excluded (
name:keys belonging todatarows, not columns, atcontent/docs/guide/quick-start.md:83,content/docs/index.md:23,examples/.../simple-table.json), which is the cross-validation earning its keep. The 4 unparseable JSON files aretsconfig*.json(JSONC with comments); none contains acolumnsarray.Every hit was classified by the enclosing node's
type, as the card requires. Everything below thedata-tablerow is a different component's own vocabulary and never reaches this adapter:typedata-tableobject-data-tableplugin-dashboard.mdx, the package README, andregister-plugins.tsgrid(form field)nameis its declared spelling —GridColumnDefinitiondeclaresname, which is #3951's outcome; the form layer means the opposite thing by these key names, ascolumn-identity.tssays outrighttable(static renderer)name/label— filed as #5350crudschema-reference.md:537; nocrudrenderer is registered in this repo (types, zod schema, validator branch and builder exist; noComponentRegistry.register). Reported as an unverified observation, not filedlistnametofieldvianormalizeColumnIdentities, then emitsobject-grid, whereObjectGridwritesaccessorKeyon the way out — never touches the aliasobject-grid{ name, label }here andObjectGridsilently drops it, before and after #5068 — filed as #5352object-master-detail-form,exportExcelWithFormulasnameitselfThe measurement that matters for the decision
The held deletion was ablated to price it: both alias sites cut, then
packages/components(the adapter's own package) andpackages/plugin-dashboardrun together.234 test files, 2122 tests, all green. Not one test in the repo pins the limb. A repo-wide scan of the source and test corpus finds exactly one
name-spelled column on a data-table-family node —packages/core/src/validation/__tests__/schema-validator.test.ts:26, acrudschema in a validator test, never rendered.So the retirement is invisible to CI. Had the census been skipped, it would have merged green and silently broken the column vocabulary the shipped skills guides teach. The census is the only thing standing in front of it.
Reverse verification — predicted before each run, then observed
No build artifact sits between any edit and the thing under test, on any leg: the root
vitest.config.mts(L245-261) aliases every workspace package —@object-ui/core,@object-ui/components,@object-ui/plugin-dashboard— to that package'ssrc/, so vitest transforms source directly and nodist/is in the path. Checked before the legs were run, not assumed.Leg A — producer-only revert (
ObjectDataTable.tsxrestored toorigin/main, tests kept).Predicted red: the two identity-stamp tests, the
field$expandtest, thefieldseam render, and the legibility pin (because{ field: … }becomes unresolvable there too). Predicted green: the other seven.Observed: exactly those 5 red, 7 green — every name matched.
Leg A, second half — the silent-regression check. Predicted: the pre-existing suite stays fully green.
Observed: 65 of 66 files and 601 of 606 tests green, the only red being this PR's new file. The same shape #5068 measured: three
ObjectDataTabletest files already existed and every one of them authors its columnsaccessorKey-spelled, so the defect was invisible to all of them.Leg B — the held half, ablated forward, together with this PR's change (both alias sites cut, producer change in place).
Predicted: nothing goes red, and this PR's 12 tests stay green.
Observed: 234 files / 2122 tests green. The second half of that prediction is the load-bearing one — it shows this change is alias-independent: it survives whichever way the maintainer rules, and it is what stops
object-data-tablefrom depending on the alias at all. That makes it a prerequisite for the deletion, not a substitute for it.Legibility, measured rather than assumed
The card asks what a now-unresolvable column does. It is not dropped and does not throw: the header renders over empty cells and the neighbouring column is unaffected. It is also not quite silent, and the noise is the interesting part — the adapter keys each cell by the accessor (
key={col.accessorKey},data-table.tsx:1829), so an unresolved column hands Reactundefinedand React emits its generic missing-key warning. That warning namestrandDataTableRenderer; it names neither the column nor the metadata that produced it. Nothing is said in ObjectUI's own voice.Unchanged by this PR — such a column carried no
accessorKeybefore it either — and pinned as behaviour so #5349 has something to measure against. No diagnostic is implemented here, per the dispatch.Tests
Run from the repo root (a package-scoped run would use a different config than CI does), at final
HEADa3920c3:pnpm exec vitest run packages/plugin-dashboard/src/__tests__/ObjectDataTable.columnIdentity.test.tsx→ 12 passedpnpm exec vitest run packages/plugin-dashboard --maxWorkers=2→ 66 files, 606 tests passedpnpm --filter @object-ui/plugin-dashboard type-check→ clean (afterpnpm --filter '@object-ui/plugin-dashboard^...' build; the dependency closure has to exist first ortscreports every workspace import as a missing module)pnpm --filter @object-ui/plugin-dashboard lint→ 0 errors (321 pre-existing warnings)check:control-bytes,check:phantom-deps,check:self-import→ all OKFindings filed along the way
All unassigned,
findingclass, none of them touched here: #5350, #5351, #5352.Generated by Claude Code