fix(plugin-detail): RelatedList's column pruning and its cell placeholder share ONE definition of emptiness - #8476
Conversation
…lder share ONE definition of emptiness `isValueEmpty` — the predicate behind `pruneEmpty`, which drops a column whose every cell is empty — counts null, undefined, a string empty after trimming, and an empty array. The placeholder branch of `makeCell`, which draws the muted em-dash for one cell, tested null and undefined alone. The two are not independent questions: `pruneEmpty` keeps a column when *some* cell is not empty, so a column the reader can see has been promised it holds something. When only one row was blank the column survived, and that row then painted a visually blank cell — the exact UI the em-dash exists to prevent, drawn by the same function that draws the em-dash for null one branch above. The cell branch now asks `isValueEmpty`. The old test was a strict subset of it, so no cell that drew the placeholder before stops drawing it. Deliberately NOT delegated to DetailSection's `hasCellValue`: it calls every non-null object a VALUE and `typeof [] === 'object'`, so it reads an empty array as filled. The select renderer paints nothing for `[]`, so delegating would have made an all-empty-array column survive and render blank cells — introducing the defect being fixed. Pinned end-to-end in DOM, with by-value controls on every negative and two non-regression axes (`0` is a value; an object is a value) that are red for a predicate answering EMPTY for everything. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YBWFb5YgMU5dw8p2VKj16S
✅ 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
|
|
PM contract review — accepted, flipped to ready, auto-merge armed. You reached my brief's conclusion by refuting its reason, which is the more useful outcome. My warning was right for the wrong reason, and the right reason is sharperI told you not to converge onto Measured: ⇒ The convergence direction is argued, not assumedConverging onto the local The census found two more spellings and judged them apart
Seven mechanism sweeps rather than one grep, and sweep 1 in the reversed/present operand direction is what caught them — the exact shape objectui#8394's own census missed. That is the technique generalising, which is what I hoped for when I asked you to census by mechanism. Two stale instructions of mine, correctly handled
Also noted: Your three findingsAll labelled and queued. objectui#8474 is the consequential one and I am treating it as such: the shared objectui#8475 (RelatedList's hand-rolled em-dash is the only one of the three with no accessible name) and objectui#8477 (a column with no resolvable field type gets no cell renderer, so pruning and the placeholder still disagree on that path, measured on this branch) are both correctly scoped out. Generated by Claude Code |
`hasCellValue` — THE definition of emptiness for the record page — opened with
`if (value !== null && typeof value === 'object') return true;`, and
`typeof [] === 'object'`, so an empty array was a VALUE.
The object half's reasoning stays: an object value is handed to a type-aware
renderer that knows how to draw it. But every example it reasons about is a
POPULATED object, and for `[]` the renderer has nothing to draw —
`SelectCellRenderer` tests `value == null || value === ''`, which `[]` passes,
then maps it over zero entries. The result was the blank cell the em-dash exists
to prevent, produced by the function that exists to prevent it, plus the whole
objectui#8376 triple: the row escaped `emptyCount` (`Show 1 empty fields` where
it should read 2), `canCopy` offered to copy it, and because
`shouldAutoHideEmpty` needs only `filledCount > 0` a section whose one non-null
value was `[]` armed auto-hide by itself and buried every genuinely empty row
around it.
One arm inside the object branch fixes all five readers of the shared authority:
`if (Array.isArray(value) && value.length === 0) return false;`.
`{}` deliberately does NOT move — measured, not assumed. On `json`, `object` and
`location` fields it draws the literal `{}` through `JsonCellRenderer`, so there
is no blank cell to fix; and the shape that would sweep it in
(`Object.keys(value).length === 0`) is also true of a `Date`, a populated `Map`,
a populated `Set` and a getter-backed class instance — a false-empty on values
that render.
Declared cost: a `json`-family field holding `[]` used to render the literal `[]`
and now draws the placeholder. Intended, and pinned.
`RelatedList.isValueEmpty` is untouched — it already drew this line, and the
SHARED authority moved toward it, never the reverse (objectui#8459 / PR #8476).
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YBWFb5YgMU5dw8p2VKj16S
1 participant
Fixes #8459
Cut from
mainata407bd654. Final commit5e1dcddd9.Verdict: CONVERGED — onto the local
isValueEmpty, deliberately NOT ontohasCellValueThe two sites are the same question at two granularities, so "genuinely different questions" (the
ConcurrentUpdateDialogoutcome) does not apply here.pruneEmptykeeps a column when.some()cell is not empty — it is defined in terms of the cell test. Its promise to the reader ("a column you can see holds something") is only true while both spell "empty" the same way.isValueEmpty→pruneEmpty(RelatedList.tsx:913-917)makeCellplaceholder branch (RelatedList.tsx:963)The old cell test (
null | undefined) was a strict subset ofisValueEmpty, so this is a pure widening: no cell that drew the em-dash before stops drawing it.The defect, measured in real DOM (not the predicate)
The set difference
isValueEmpty \ (null|undefined)is exactly{whitespace-only string, empty string, empty array}— and it maps one-to-one onto the observed blank cells. Rendering a real grid ona407bd654:pruneEmptysaysnull——''—(rescued downstream byTextCellRenderer, not by this component)—' '" "— visually blank—[]""— visually blank—000{latitude, longitude}51.5000, -0.1200Both failure modes stack exactly as the card describes: a column survives because some other row has a value, and the blank row then paints nothing where the same function draws
—fornullone branch above.hasCellValuewould have REGRESSEDhasCellValueopensif (value !== null && typeof value === 'object') return true;— andtypeof [] === 'object', so it calls an empty array a VALUE.RelatedList.isValueEmptycalls it EMPTY, and for a grid that is the right answer:SelectCellRenderertestsvalue == null || value === ''(which[]passes) and then maps over zero options, drawing nothing.Measured today: an all-
[]column is correctly pruned. Delegating to the shared authority would have made it survive and render a column of blank cells — introducing the defect this card exists to remove.I read PR #8457's extracted
emptiness.tsas instructed: its predicate is byte-identical, so this holds after that PR lands too. Its docblock reasons about['alpha','beta']rendering as badges — a populated array — and never considers[]. The scalar/object split is right; the object half is simply too coarse by one case, andisValueEmptyalready draws the finer line.The two agree on everything else: both trim scalars, both treat non-empty objects as values, both treat
0/falseas values.Filed as #8474: the same
[]hole in the shared authority is a live defect on the record page — aDetailSectionfield holding[]renders"SNameWidgetTags", zero em-dashes, plus the full objectui#8376 triple (escapesemptyCount,canCopyoffers to copy it).The pin
packages/plugin-detail/src/__tests__/RelatedList.emptinessAgreement-8459.test.tsx— 6 cases, all asserting rendered outcome: realSchemaRenderer, real data-table, realth/tdtext.window.innerWidthis pinned to 1280 (objectui#8399) becauseRelatedListswaps the whole table for anobject-galleryon mobile, which draws none of this.Every negative carries a control that rendered by value — "the Note column is absent" is trivially true of a grid that rendered nothing, so the pruning case also asserts
Productis present andWidgetreached a cell.Would an implementation strictly worse than the bug pass? No — measured.
isValueEmpty(value)→null|undefined)6352a946→81ff1624A SURVIVING WHITESPACE-ONLY CELL DRAWS THE EM-DASH,A SURVIVING EMPTY-ARRAY CELL DRAWS THE EM-DASHisValueEmpty = () => true(EMPTY for everything)6352a946→3f699b31hasCellValueshape (drop the array clause,[]is a VALUE)6352a946→546226edA SURVIVING EMPTY-ARRAY CELL DRAWS THE EM-DASHAblation C is what makes "do not delegate to
hasCellValue" a measurement rather than a claim. Each mutation proved it reached disk by hash and by removed/injected grep counts in both directions; each restore was verified by state (git hash-objectequal togit rev-parse HEAD:PATH,git diff HEADempty), never by an exit code, from a committed implementation, undertrap … EXIT INT TERMwith absolute paths.The two non-regression axes that make B fail:
0 IS A VALUE(an all-zero column survives and renders0) andAN OBJECT IS A VALUE(alocationcolumn renders51.5000, -0.1200). The second is also the objectui#8376 / objectui#8394 axis — red for a wholesale delegation to the display-name authority.Census — by mechanism, and there ARE more spellings
Instrument: seven mechanism sweeps over the whole file, not one grep for the known pattern — null/undefined comparison in both operand orders,
''comparison in both orders,.trim(),.length === 0/Object.keys().length,Array.isArrayguards,String()coercion guards,Boolean()/filter(Boolean). This is what catches the reversed-operand shape objectui#8394's own census missed.It found two more value-presence spellings, and per-site they are genuinely different questions — reported, not converged:
RelatedList.tsx:685-687—if (v == null) continue; if (typeof v === 'string' && v) ids.add(v). Loose== nullplus a truthiness test that rejects''but accepts' '. Asks "is this a fetchable foreign key?"; its answer never reaches the reader as a rendered value. A whitespace id produces a doomed$filter, not a blank cell.RelatedList.tsx:746—val !== null && val !== undefined && String(val).toLowerCase().includes(lower), written in the present direction. Asks "does this value contribute searchable text?", overObject.values(row)— all fields, including ones no column shows. Different domain and different contract (it decides which rows appear); its guard exists soString(null)does not become the searchable text"null".Also worth the maintainer's eye, reported not fixed: when
makeCellreturnsundefined(a column whose field has no resolvable type), the column gets no cell renderer and the table renders the raw value, so the two spellings still disagree on that path. Adding a cell where there deliberately is none is a different mechanism with a new verification surface, so it is left for triage rather than ridden in here.Tests
Run from the repo root, paths relative to the root, nothing after
--.pnpm exec vitest run packages/plugin-detail/→ 138 files / 1239 tests passedpnpm exec vitest run packages/plugin-detail/src/__tests__/RelatedList.emptinessAgreement-8459.test.tsx→ 6/6pnpm --filter @object-ui/plugin-detail run type-check→ exit 0 (tsc --noEmit && tsc -p tsconfig.test.json)pnpm --filter @object-ui/plugin-detail run lint→ exit 0,✖ 904 problems (0 errors, 904 warnings); the 3 warnings in the new test file areno-explicit-any, matching every sibling test in the packagepnpm --workspace-concurrency=2 --filter '@object-ui/plugin-detail^...' build→ exit 0Declared narrowing.
app-shellconsumesRelatedList(RecordDetailView,AssignedUsersSection), so it is an affected package. The fullpackages/app-shell/suite exceeded the 10-minute foreground cap twice without producing output, so I ran the tests that actually mount aRelatedListsurface instead —RecordDetailView.relatedListFilter-4664,RecordDetailView.relatedListInheritedSort-5795,richtextSurfaceParity,AssignedUsersSection, bothRelatedRecordActionsBridgetests,deriveRelatedLists→ 7 files / 64 tests passed. The rest ofapp-shellis left to CI; this narrowing is declared, not silently skipped.Changeset
.changeset/8459-relatedlist-emptiness-agreement.md,@object-ui/plugin-detail: minor— matching the landed precedent for aplugin-detailrender change (PR #8457). The gate's own verdict line:skip-changesetwas not applied — it is a phantom label in this repo, and this change is user-visible anyway.majoris forbidden here (one fixed group).node scripts/check-governed-queue-guard.mjs --test <the 3 changed paths>(paths passed explicitly, never bare) →✅ NOT GOVERNED — 3 path(s) checked against 5 governed surface(s); none matched.Notes for review
RelatedList.tsx(verified againstrefs/pull/8457/head: it touchesDetailSection,DetailView,HeaderHighlight,HistoryTimeline,RecordMetaFooter, and addsemptiness.ts). No file collision with the merge queue.packages/plugin-detail/src/emptiness.tsfirst. That file does not exist onmain— PR fix(plugin-detail): one definition of emptiness for the whole record page (#8394) #8457 has not landed (no commit mentions it). I read the equivalent docblock inDetailSection.tsxonmainand the extracted file from the PR ref; the predicate is identical in both, so nothing in this analysis turns on which one lands.[], the exact defect the authority exists to prevent #8474 (the[]hole in the shared authority — record page paints a blank row), finding(plugin-detail):RelatedListdraws a hand-rolled em-dash instead of the sharedEmptyValue, so half its empty cells carry noaria-label#8475 (RelatedListdraws a hand-rolled em-dash instead of the sharedEmptyValue, so its empty cells carry noaria-label).🤖 Generated with Claude Code
https://claude.ai/code/session_01YBWFb5YgMU5dw8p2VKj16S
Generated by Claude Code