Skip to content

Commit 95b7214

Browse files
os-zhuangclaude
andauthored
fix(list,grid,detail,tree,core): every column resolver reads one key (#3104 PR2) (#3122)
PR1 (#3119) put a canonicalizing fold at ListView's ingestion boundary. This converges the 22 read sites themselves onto `columnIdentity()`, so a surface that is NOT downstream of that fold resolves the same identity anyway. That distinction is the user-visible part. A standalone `object-grid` node — authored directly on a page, with no `list-view` above it — never passed through `normalizeListViewSchema`. Its `getSelectFields` read `c.field` alone while the `ensureId` probe one line above read `f?.name || f?.field`, so a legacy `{name:'account'}` column reached `$select` as a literal `undefined` hole: the server never returned the field and every cell in that column came back empty. Same for ObjectTree, RelatedList and the record:details / record:related_list renderers. Converged: ListView ×9 + its 2 request builders -> columnIdentity() RelatedList ×8 -> accessorKey || columnIdentity() ObjectGrid (probe + projection) -> columnIdentity() ObjectTree -> columnIdentity() || key buildExpandFields -> columnIdentity() record-details / record-related-list -> columnIdentity() (|| key) `accessorKey` keeps its precedence in RelatedList — it is TanStack Table's column key, not ObjectStack metadata identity, and only the `field || name` tail was converged. `key` stays a tail fallback in ObjectTree and record-related-list for the same reason: it is a generic entry key. Two incidental fixes TypeScript surfaced once the resolver stopped returning `any`: ListView's filter-field options and its hide-fields popover both built entries keyed `undefined` for a column with no resolvable identity. Those entries could never match a column; they are now dropped. Inventory re-triage: PR1 recorded 24 family members. Two were mis-classified and are reclassified rather than converged — reading what they actually feed shows they are not column reads at all. ViewPreview adapts a ViewItem FORM section to what object-form selects by (#3090's two-layer join); SchemaForm renders an arbitrary metadata ARRAY into a popover summary and guesses at a display key. So the family was 22, and it is now 0. The ratchet asserts that, asserts each converged surface actually routes through the shared reader (a surface that dropped identity resolution instead of converging it goes red), and pins accessorKey's precedence in RelatedList. Refs: objectstack#4115, #3090 (playbook), #3119 (PR1) Claude-Session: https://claude.ai/code/session_01C2pdPmf2yZSd4wFDs1NHY5 Co-authored-by: Claude <noreply@anthropic.com>
1 parent 6ae818e commit 95b7214

11 files changed

Lines changed: 314 additions & 148 deletions

File tree

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
---
2+
"@object-ui/core": patch
3+
"@object-ui/plugin-list": patch
4+
"@object-ui/plugin-grid": patch
5+
"@object-ui/plugin-detail": patch
6+
"@object-ui/plugin-tree": patch
7+
---
8+
9+
fix(list,grid,detail,tree,core): every column resolver reads one key (#3104 PR2)
10+
11+
PR1 (#3119) put a canonicalizing fold at ListView's ingestion boundary. This
12+
converges the 22 read sites themselves onto `columnIdentity()` from
13+
`@object-ui/core`, so a surface that is NOT downstream of that fold resolves
14+
the same identity anyway.
15+
16+
That distinction is the user-visible part. A standalone `object-grid` node —
17+
authored directly on a page, with no `list-view` above it — never passed
18+
through `normalizeListViewSchema`. Its `getSelectFields` read `c.field` alone
19+
while the `ensureId` probe one line above read `f?.name || f?.field`, so a
20+
legacy `{ name: 'account' }` column reached `$select` as a literal `undefined`
21+
hole: the server never returned the field and every cell in that column came
22+
back empty. Same for `ObjectTree`, `RelatedList` and the `record:details` /
23+
`record:related_list` renderers.
24+
25+
Converged:
26+
27+
| Surface | Was | Now |
28+
|---|---|---|
29+
| `ListView` ×9 + its 2 request builders | `name \|\| fieldName \|\| field` vs `f?.field` | `columnIdentity()` |
30+
| `RelatedList` ×8 | `accessorKey \|\| field \|\| name` | `accessorKey \|\| columnIdentity()` |
31+
| `ObjectGrid` | name-first probe vs `c.field` projection | `columnIdentity()` |
32+
| `ObjectTree` | `name \|\| fieldName \|\| field \|\| key` | `columnIdentity() \|\| key` |
33+
| `buildExpandFields` | `field ?? name ?? fieldName` | `columnIdentity()` |
34+
| `record-details` / `record-related-list` | `field \|\| name (\|\| key)` | `columnIdentity() (\|\| key)` |
35+
36+
`accessorKey` keeps its precedence in `RelatedList` — it is TanStack Table's
37+
column key, not ObjectStack metadata identity, and only the `field || name`
38+
tail was converged. `key` stays a tail fallback in `ObjectTree` and
39+
`record-related-list` for the same reason: it is a generic entry key.
40+
41+
Two incidental fixes that TypeScript surfaced once the resolver stopped
42+
returning `any`: ListView's filter-field options and its hide-fields popover
43+
both built entries keyed `undefined` for a column with no resolvable identity.
44+
Those entries could never match a column; they are now dropped.
45+
46+
**Inventory re-triage.** PR1 recorded 24 family members. Two were mis-classified
47+
and are reclassified here rather than converged — reading what they actually
48+
feed shows they are not column reads at all:
49+
50+
- `ViewPreview.tsx` adapts a ViewItem **form** section to what `object-form`
51+
selects by (`field``name`) — the #3090 two-layer join.
52+
- `SchemaForm.tsx` renders an arbitrary metadata **array** into a popover
53+
summary and guesses at a display key; the entries are validations, actions,
54+
or whatever the JSON schema declares.
55+
56+
So the family was 22, and it is now **0**. The ratchet asserts that, asserts
57+
each converged surface actually routes through the shared reader (a surface
58+
that dropped identity resolution instead of converging it goes red), and pins
59+
`accessorKey`'s precedence in `RelatedList`.

content/docs/guide/troubleshooting.md

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -309,12 +309,19 @@ never expanded.
309309
{ "field": "account" }
310310
```
311311

312-
**Fix:** Author columns with `field`. Metadata reaching a `list-view` is
313-
canonicalized at the component boundary by `normalizeListViewSchema`, which
314-
stamps `field` from whichever spelling is present and makes any legacy key it
315-
already carries agree — so mixed metadata resolves to one field everywhere
316-
instead of two. Legacy keys are still accepted, but they are a migration
317-
bridge, not a second contract: fix the producer.
312+
**Fix:** Author columns with `field`. Two mechanisms keep legacy metadata
313+
working, and both resolve the canonical key first:
314+
315+
- Metadata reaching a `list-view` is canonicalized at the component boundary by
316+
`normalizeListViewSchema`, which stamps `field` from whichever spelling is
317+
present and makes any legacy key it already carries agree.
318+
- Every renderer that resolves a column — list, grid, tree, related lists, the
319+
`$expand` / `$select` builders — reads that identity through one shared
320+
function, so a surface rendered outside the fold (a standalone `object-grid`
321+
node, for instance) still resolves the same field the request asked for.
322+
323+
Legacy keys are still accepted, but they are a migration bridge, not a second
324+
contract: fix the producer.
318325

319326
If you read column identity in your own code, use the one reader rather than
320327
spelling out a fallback chain — it resolves canonical-first, so it agrees with

packages/core/src/utils/__tests__/column-identity.ratchet.test.ts

Lines changed: 80 additions & 83 deletions
Original file line numberDiff line numberDiff line change
@@ -4,11 +4,17 @@
44
*
55
* Ratchet — the column-identity dual read (`field` ?? `name`) may only shrink.
66
*
7-
* objectui#3104. A column's identity is read in two incompatible precedences
8-
* across the repo, and the two halves land on different fields for the same
9-
* column (see `ListView.columnIdentity.test.tsx` for the live repro). This file
10-
* freezes the inventory so the family cannot grow while it is being retired:
11-
* a new dual read in a new file fails, and an extra one in a listed file fails.
7+
* objectui#3104. A column's identity used to be read in two incompatible
8+
* precedences across the repo, and the two halves landed on different fields
9+
* for the same column (see `ListView.columnIdentity.test.tsx` for the repro).
10+
*
11+
* **The family is now at zero** (PR2): every column-identity read goes through
12+
* `columnIdentity()`. What remains in the inventory below is the residue the
13+
* scanner still matches — reads that share the key names but not the concept.
14+
* They are listed, not converged, and each says why.
15+
*
16+
* This file freezes that state: a new dual read in a new file fails, and an
17+
* extra one in a listed file fails.
1218
*
1319
* Shrinking also fails, deliberately — lower the number here in the same commit
1420
* that removes the read, so the count in the tree and the count on record never
@@ -78,71 +84,25 @@ interface Entry {
7884
}
7985

8086
/**
81-
* The inventory, as of objectui#3104 PR1.
87+
* The inventory, as of objectui#3104 PR2.
88+
*
89+
* The column-identity family is EMPTY: all 22 reads now go through
90+
* `columnIdentity()`. What is left below matched the scanner but was never the
91+
* same concept — two-layer joins where both precedences are correct, the form
92+
* cluster that #3090 settled the other way, and generic display fallbacks that
93+
* merely share the key names. They stay listed so the scanner cannot quietly
94+
* grow a real dual read inside a file we already decided about.
8295
*
83-
* The headline is not that two precedences exist — it is that ListView and
84-
* ObjectGrid each disagree with THEMSELVES. Both build their `$expand` /
85-
* `$select` from `f?.field` alone (a single-key read the scanner cannot see,
86-
* because it is not an alternation) while every render-side read in the same
87-
* file is name-first. That is the mechanism behind "relation column shows a
88-
* bare id": the request asks for one field, the renderer shows another.
96+
* PR1 recorded 24 family members; two of those were mis-triaged and are
97+
* reclassified here after reading what they actually feed:
98+
* - `ViewPreview.tsx` converts a ViewItem FORM section into `object-form`'s
99+
* runtime shape (`field` -> `name`) — the #3090 two-layer join, not a column.
100+
* - `SchemaForm.tsx` renders an arbitrary metadata ARRAY into a summary string
101+
* and guesses at a display key; the entries are validations/actions/whatever
102+
* the JSON schema declares, so there is no column vocabulary to converge.
89103
*/
90104
const INVENTORY: Record<string, Entry> = {
91-
// ── the column-identity family (24) ──────────────────────────────────────
92-
'plugin-list/src/ListView.tsx': {
93-
count: 9,
94-
order: 'name-first',
95-
verdict: 'column-identity',
96-
why: 'FLS gate, hidden-field filter, fieldOrder, export ×2, hide-fields list — all name-first, while $expand/$select in the same file read `f?.field` only.',
97-
},
98-
'plugin-detail/src/RelatedList.tsx': {
99-
count: 8,
100-
order: 'adapter-first',
101-
verdict: 'column-identity',
102-
why: '`accessorKey || field || name` — merges TanStack\'s column key with metadata identity. The adapter key is out of scope; the `field || name` tail is not.',
103-
},
104-
'core/src/utils/expand-fields.ts': {
105-
count: 1,
106-
order: 'field-first',
107-
verdict: 'column-identity',
108-
why: 'The request path. Canonical-first, so it is the side the renderers must converge ONTO.',
109-
},
110-
'plugin-grid/src/ObjectGrid.tsx': {
111-
count: 1,
112-
order: 'name-first',
113-
verdict: 'column-identity',
114-
why: 'The `ensureId` probe is name-first while `getSelectFields` right below it reads `c.field` only — the same self-disagreement as ListView.',
115-
},
116-
'plugin-tree/src/ObjectTree.tsx': {
117-
count: 1,
118-
order: 'name-first',
119-
verdict: 'column-identity',
120-
why: 'Tree column identity.',
121-
},
122-
'plugin-detail/src/renderers/record-details.tsx': {
123-
count: 1,
124-
order: 'field-first',
125-
verdict: 'column-identity',
126-
why: 'Detail field entry identity.',
127-
},
128-
'plugin-detail/src/renderers/record-related-list.tsx': {
129-
count: 1,
130-
order: 'field-first',
131-
verdict: 'column-identity',
132-
why: 'Related-list column identity.',
133-
},
134-
'app-shell/src/views/metadata-admin/previews/ViewPreview.tsx': {
135-
count: 1,
136-
order: 'field-first',
137-
verdict: 'column-identity',
138-
why: 'Studio preview column identity.',
139-
},
140-
'app-shell/src/views/metadata-admin/SchemaForm.tsx': {
141-
count: 1,
142-
order: 'field-first',
143-
verdict: 'column-identity',
144-
why: 'Renders a column array into a summary string.',
145-
},
105+
// ── the column-identity family: EMPTY (was 24 in PR1; 22 after re-triage) ──
146106

147107
// ── two layers, not two spellings (6) ────────────────────────────────────
148108
'app-shell/src/utils/resolveActionParams.ts': {
@@ -166,7 +126,7 @@ const INVENTORY: Record<string, Entry> = {
166126
why: 'Action param name, same layering as resolveActionParams.',
167127
},
168128

169-
// ── the form cluster — settled the other way (#3090) (2) ─────────────────
129+
// ── the form cluster — settled the other way (#3090) (3) ─────────────────
170130
'plugin-form/src/ObjectForm.tsx': {
171131
count: 1,
172132
verdict: 'form-cluster',
@@ -177,8 +137,13 @@ const INVENTORY: Record<string, Entry> = {
177137
verdict: 'form-cluster',
178138
why: 'The #3090 hub itself.',
179139
},
140+
'app-shell/src/views/metadata-admin/previews/ViewPreview.tsx': {
141+
count: 1,
142+
verdict: 'form-cluster',
143+
why: 'Re-triaged in PR2: `toFormFieldEntry` adapts a ViewItem FORM section to what `object-form` selects by (`name`), which is the #3090 join — not a list column. Converging it onto `columnIdentity` would be a category error.',
144+
},
180145

181-
// ── not identity reads (2) ───────────────────────────────────────────────
146+
// ── not identity reads (3) ───────────────────────────────────────────────
182147
'app-shell/src/views/metadata-admin/widgets.tsx': {
183148
count: 1,
184149
verdict: 'unrelated',
@@ -189,6 +154,11 @@ const INVENTORY: Record<string, Entry> = {
189154
verdict: 'unrelated',
190155
why: '`c.object ?? c.name` — a citation label fallback that happens to sit on a line mentioning `c.field`.',
191156
},
157+
'app-shell/src/views/metadata-admin/SchemaForm.tsx': {
158+
count: 1,
159+
verdict: 'unrelated',
160+
why: 'Re-triaged in PR2: `summariseComposite` renders ANY composite/array metadata value into a popover summary and guesses at a display key. The array items are whatever the JSON schema declares, so this is a display fallback over arbitrary objects, not a column read.',
161+
},
192162
};
193163

194164
function collectSourceFiles(): string[] {
@@ -265,28 +235,55 @@ describe('column identity dual-read ratchet (#3104)', () => {
265235
expect(drift).toEqual([]);
266236
});
267237

268-
it('keeps the family total from growing', () => {
269-
// The number that has to reach zero. The other verdicts are inventory, not
270-
// worklist: `two-layer` and `form-cluster` are settled decisions, and
271-
// `unrelated` is scanner noise recorded so it cannot hide a real one.
272-
expect(sum((e) => e.verdict === 'column-identity')).toBe(24);
273-
expect(sum(() => true)).toBe(34);
238+
it('holds the family at zero', () => {
239+
// This is the number the battle was about, and it is done. The remaining
240+
// verdicts are inventory, not worklist: `two-layer` and `form-cluster` are
241+
// settled decisions, and `unrelated` is scanner residue recorded so it
242+
// cannot hide a real one.
243+
expect(sum((e) => e.verdict === 'column-identity')).toBe(0);
244+
expect(sum(() => true)).toBe(12);
274245
});
275246

276-
it('records a precedence for every read in the family', () => {
247+
it('records a precedence for every read left in the family', () => {
277248
const missing = Object.entries(INVENTORY)
278249
.filter(([, e]) => e.verdict === 'column-identity' && !e.order)
279250
.map(([file]) => file);
280251
expect(missing).toEqual([]);
281252
});
282253

283-
it('still shows both precedences in the family — the pollution this closes', () => {
284-
const orders = new Set(
285-
Object.values(INVENTORY)
286-
.filter((e) => e.verdict === 'column-identity')
287-
.map((e) => e.order),
254+
it('routes every converged surface through the one reader', () => {
255+
// The counterpart to "the family is zero": the reads did not vanish, they
256+
// moved onto `columnIdentity`. If a surface silently dropped its identity
257+
// resolution instead of converging it, this goes red.
258+
const CONVERGED = [
259+
'core/src/utils/expand-fields.ts',
260+
'plugin-list/src/ListView.tsx',
261+
'plugin-grid/src/ObjectGrid.tsx',
262+
'plugin-detail/src/RelatedList.tsx',
263+
'plugin-detail/src/renderers/record-details.tsx',
264+
'plugin-detail/src/renderers/record-related-list.tsx',
265+
'plugin-tree/src/ObjectTree.tsx',
266+
];
267+
const missing = CONVERGED.filter((rel) => {
268+
const src = readFileSync(path.join(packagesRoot, rel), 'utf8');
269+
return !/\bcolumnIdentity\s*\(/.test(src);
270+
});
271+
expect(missing).toEqual([]);
272+
});
273+
274+
it('keeps the table-adapter key ahead of metadata identity in RelatedList', () => {
275+
// A source-level pin, and labelled as one: RelatedList's columns can arrive
276+
// in TanStack's shape (`accessorKey`) or ObjectStack's (`field`). The
277+
// convergence replaced only the `field || name` tail — if `accessorKey`
278+
// ever loses its precedence here, imported TanStack columns stop resolving
279+
// and every cell in them goes blank.
280+
const src = readFileSync(
281+
path.join(packagesRoot, 'plugin-detail/src/RelatedList.tsx'),
282+
'utf8',
288283
);
289-
// When PR2 lands this becomes a single order and this assertion inverts.
290-
expect(orders.size).toBeGreaterThan(1);
284+
const identityReads = [...src.matchAll(/\bcolumnIdentity\s*\(/g)].length;
285+
const adapterFirst = [...src.matchAll(/accessorKey\s*\|\|\s*columnIdentity\s*\(/g)].length;
286+
expect(identityReads).toBeGreaterThan(0);
287+
expect(adapterFirst).toBe(identityReads);
291288
});
292289
});

packages/core/src/utils/expand-fields.ts

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@
66
* LICENSE file in the root directory of this source tree.
77
*/
88

9+
import { columnIdentity } from './column-identity.js';
10+
911
/**
1012
* Relational ("reference-bearing") field types whose stored value is a foreign
1113
* key into another object — and which therefore benefit from `$expand` so a
@@ -101,12 +103,12 @@ export function buildExpandFields(
101103
if (columns && Array.isArray(columns) && columns.length > 0) {
102104
const columnFieldNames = new Set<string>();
103105
for (const col of columns) {
104-
if (typeof col === 'string') {
105-
columnFieldNames.add(col);
106-
} else if (col && typeof col === 'object') {
107-
const name = col.field ?? col.name ?? col.fieldName;
108-
if (name) columnFieldNames.add(name);
109-
}
106+
// `columnIdentity` handles all three entry shapes (bare string, spec
107+
// `{field}`, legacy `{name}`/`{fieldName}`) and resolves canonical-first
108+
// — the same precedence every renderer now uses, so what gets expanded
109+
// and what gets rendered can no longer name two different fields (#3104).
110+
const name = columnIdentity(col);
111+
if (name) columnFieldNames.add(name);
110112
}
111113
return referenceFieldNames.filter((f) => columnFieldNames.has(f));
112114
}

0 commit comments

Comments
 (0)