fix(plugin-detail): one definition of emptiness for the whole record page (#8394) - #8457
Merged
Merged
Conversation
…page (#8394) objectui#8350 gave the `record:details` dedupe ladder the page H1's authority and objectui#8376 converged `DetailSection`'s three raw `null | undefined | ''` tests onto `hasCellValue`. Four more raw spellings were left on the same page, none trimming, so a whitespace-only value read EMPTY at the H1 and the body grid and FILLED at every band between and around them — each painting a blank. `hasCellValue` moves out of `DetailSection.tsx` into `./emptiness`, and the highlight strip, the summary chips, the audit timeline and the record footer read it. Objects stay values: they go to type-aware renderers that draw them, which is why the scalar half delegates to `recordDisplayValueAt` and the object half does not. Two sites are not a plain predicate swap: * `DetailView` decides this TWICE — `autoSummaryFields`' picker as well as the render. Raw, a whitespace-only `status` won the single status slot and the render then dropped the chip, so a render-only fix turns a blank chip into a missing one. * `RecordMetaFooter` asks it four times over one value and only the last of them reaches `UserRef`; converging that alone leaves `Created by · 5m ago` standing over an actor that is not there. Normalized at the read instead. Pinned on the DOM in `detailPage.emptinessAuthority-8394.test.tsx`, whose non-regression cases are red for a wholesale delegation and red for an emptiness test that answers EMPTY for everything. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YBWFb5YgMU5dw8p2VKj16S
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-justin
marked this pull request as ready for review
September 8, 2026 00:34
os-justin
enabled auto-merge
September 8, 2026 00:34
This was referenced Sep 8, 2026
os-justin
deleted the
claude/issue-8394-remaining-emptiness-spellings
branch
September 8, 2026 01:01
This was referenced Sep 8, 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 free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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 #8394
hasCellValuemoves out ofDetailSection.tsxintopackages/plugin-detail/src/emptiness.ts, and every remaining band of the record page reads it instead of its own rawnull | undefined | ''test.Per-site verdict — three convergences, one genuine difference, and a fifth spelling the card's census missed
HeaderHighlight.tsx:171isEmptychooses between the em-dash affordance andCellRenderer— the same two branchesDetailSection'sdisplayValuechooses between, with the samedetail.noValuelabel. Objects reach the type-aware renderer here exactly as they do there.HistoryTimeline.tsx:134formatDiffValuechooses between'—'and content, and its object branch isJSON.stringify— real content, so the object half must not delegate for the same reason.DetailView.tsx:979hasCellValueis the non-regressive choice — it moves whitespace-only strings and nothing else.DetailView.tsx:438autoSummaryFields'has()decides the same thing one rung earlier, overdata?.[n]with the operands reversed, which is why the card's grep did not see it. It picks which field becomes a chip. Raw, a whitespace-onlystatussatisfied it and won the single status slot — and a render-only fix then dropped that chip, so the header showed no status chip at all where a genuinely filledstageshould have taken the slot. Fixing only the line the card names turns a blank chip into a missing one.RecordMetaFooter.tsx:82hasCreated/hasUpdated, thesameUsersuppression, the choice between theCreated byand the "by"-lessCreatedlabel, andMetaEntry's own{user ? … }gate. Only the last reachesUserRef, so line 82 is unreachable fornull/undefined/''in the first place, and converging it alone removes the blank while leavingCreated by · 5m agostanding over an actor that is not there — the exact dangling phrase the label branch's own comment says it prevents. The footer is normalized at the read instead, so all four agree; line 82 stays as a converged defensive floor.The card's guess about
UserRefis contradicted by measurement. It suggested this site "may genuinely want the title predicate". It does not:UserRefhands objects toLookupCellRenderer, which resolves them through its own display chain. Measured on this branch — a bare{ id: 'u1' }actor rendersu1on screen, while the title authority calls that payload empty. Delegating would have dropped the actor and fallen the label back toCreated. That is now a pinned non-regression case.What was deliberately left alone
ConcurrentUpdateDialog.tsx:65has the same shape but a different question: it renders''as""on purpose, because a conflict dialog reports what is stored, where "empty string" and "absent" are facts the reader must be able to tell apart. Converging it would delete information. The reason is written into the shared module's docblock so the next census does not pattern-match its way in.The pin, and whether something worse than the bug would pass
packages/plugin-detail/src/__tests__/detailPage.emptinessAuthority-8394.test.tsx— 11 cases, all asserting the rendered outcome, never the predicate alone. Every case carries a control that rendered by value, because "the blank is gone" is trivially true of a document that rendered nothing.Ablations, each from the committed implementation, each proven to reach disk (
git hash-objectdiffers fromgit rev-parse HEAD:PATH) and restored by state (git diff HEADempty and the hash back to the HEAD blob), never by an exit code:HeaderHighlightback to rawAFFORDANCE — a whitespace-only highlight draws the No value em-dashDetailViewrender back to rawCHIP — a whitespace-only summary field renders no chipDetailViewpicker back to raw (i.e. the render-only fix)PICKER AND RENDER AGREEHistoryTimelineback to rawPLACEHOLDER — a whitespace-only audit value reads as —RecordMetaFooterrenderer-only fix (reads un-normalized)THE "BY"-LESS LABELhasCellValuereturns EMPTY for everything (deleting the feature)0/falsecaseshasCellValuedelegates the object half wholesale{ id }actor, and the measurementSo no: an implementation strictly worse than the bug does not pass — the two dedicated non-regression axes (objects still render;
0andfalseare still values) are what refuse it.Verification
pnpm --filter @object-ui/plugin-detail test— 138 files, 1242 tests, all passingpnpm --filter @object-ui/plugin-detail type-check— clean (tsc --noEmitandtsc -p tsconfig.test.json)eslint .— 0 errors (905 pre-existing warnings, unchanged)app-shell649 files / 6261 tests,console89 / 1069,plugin-kanban30 / 204,plugin-calendar30 / 216,plugin-gantt63 / 496,runner4 / 17,schema-catalog29 / 2088 — all greennode scripts/check-changeset-presence.mjsverdict:5 source file(s) of 1 released package(s) changed, and this change declares 1 changeset(s). Also green:check-changeset-no-major,check-changeset-fixed,check-changeset-overwrite,check-control-bytes,check-unreferenced-sourcesnode scripts/check-governed-queue-guard.mjs --teston all eight changed paths:NOT GOVERNEDChangeset:
minoron@object-ui/plugin-detail, matching the landed precedent for arecord:detailsrender change (PR #8396). Nomajor, andskip-changesetis not used — it is a phantom label here.In-flight overlap
claude/issue-8395-copyfield-object-valuesis open against the same base and is expected to touchDetailSection.tsx. The overlap point is textual only and far apart: this PR removeshasCellValueplus its docblock from the top of that file (and adds one import), while #8395's subject ishandleCopyField'sString(value)further down. Taking the union is a plain merge; the merge queue rebuilds on currentmaineither way.🤖 Generated with Claude Code
https://claude.ai/code/session_01YBWFb5YgMU5dw8p2VKj16S
Generated by Claude Code