fix(plugin-detail): one definition of emptiness for the whole record page (#8394)

.changeset/8394-record-page-emptiness-authority.md

@@ -0,0 +1,58 @@
+---
+'@object-ui/plugin-detail': minor
+---
+
+The whole record page now shares ONE definition of emptiness, and it **trims**
+(objectui#8394).
+
+objectui#8350 gave `record:details`' dedupe ladder the page H1's authority;
+objectui#8376 converged `DetailSection`'s three spellings onto it. Four raw
+`null | undefined | ''` tests were left on the same page, none of them trimming.
+So for a whitespace-only value the H1 said "empty", the body grid said "empty",
+and the bands between and around them said "filled" and painted nothing — a
+contradiction visible in a single screenful.
+
+**What a reader saw, per band.**
+
+- **The highlight strip** (`HeaderHighlight`, ADR-0085) sits between the H1 and
+  the body grid, and after objectui#8376 it was the last band on the page still
+  calling a whitespace-only value FILLED: it painted a **blank chip** where the
+  em-dash affordance belongs.
+- **The summary chips beside the H1** (`DetailView`) rendered a **blank Badge**.
+  ⭐ And this surface decides emptiness **twice**: the auto-detection that picks
+  which field becomes a chip asked the same raw question one rung earlier, so a
+  whitespace-only `status` won the single status slot — and then the render
+  dropped it, leaving **no status chip at all** where a genuinely filled `stage`
+  would have shown one. Fixing only the render site would have turned a blank
+  chip into a missing chip.
+- **The audit timeline** (`HistoryTimeline`) printed the spaces instead of the
+  `—` it uses to mean "nothing".
+- **The record footer** (`RecordMetaFooter`) rendered a **blank actor**. ⭐ Here
+  the fix is at the READ, not at the renderer: four consumers ask "is there an
+  actor?" about one value — the presence gate, the `sameUser` suppression, the
+  choice between the `Created by` and the "by"-less `Created` label, and the
+  gate that actually mounts the renderer. Only the last reaches the renderer, so
+  converging it alone would have removed the blank and left `Created by · 5m
+  ago` standing over an actor that is not there — the dangling phrase that label
+  branch exists to prevent. Normalized once at the read, all four agree.
+
+**The change.** `hasCellValue` — the predicate objectui#8376 measured into
+existence — moves out of `DetailSection.tsx` into a small shared module, and
+every band above reads it. Its scalar answer is `@object-ui/core`'s
+`recordDisplayValueAt`, the same authority the H1 uses, rather than a fifth
+hand-written test.
+
+**Objects are still values, deliberately.** `recordDisplayValueAt` answers "does
+this resolve to a NAME", so an object goes through the Salesforce-style display
+chain and is empty when that yields nothing. Right for a title, wrong for a
+cell: on these surfaces an object is handed to a type-aware renderer that knows
+how to draw it — `{ latitude, longitude }` as coordinates, an option array as
+badges, an expanded `{ id, name }` reference through the lookup renderer's own
+display chain, anything else as JSON. Delegating that half would have replaced
+populated chips, cells and actors with placeholders. This applies to the record
+footer too, which the filing card guessed might want the title predicate: it
+does not, because its renderer draws objects.
+
+The only values whose rendering changes are strings that contain nothing but
+whitespace. `0`, `false`, `''`, `null`, `undefined` and every object value are
+classified exactly as they were.

packages/plugin-detail/src/DetailSection.tsx

@@ -26,7 +26,6 @@ import {
 } from '@object-ui/components';
 import { ChevronDown, ChevronRight, Copy, Check, Eye, EyeOff, Pencil } from 'lucide-react';
 import { SchemaRenderer, toRenderableSchema, useInlineEdit } from '@object-ui/react';
-import { recordDisplayValueAt } from '@object-ui/core';
 import { getCellRenderer, resolveCellRendererType } from '@object-ui/fields';
 import type { DetailViewSection as DetailViewSectionType, DetailViewField, FieldMetadata } from '@object-ui/types';
 import { applyDetailAutoLayout } from './autoLayout';
@@ -36,75 +35,13 @@ import { PermissionFacetLink } from './renderers/PermissionFacetLink';
 import { NON_EDITABLE_SYSTEM_FIELDS } from './systemFields';
 import { InlineFieldInput } from './InlineFieldInput';
 import { headerColorClass } from './headerColor';
+import { hasCellValue } from './emptiness';
 import {
   enrichDetailField,
   isComputedFieldType,
   isInlineExcludedDetailFieldType,
 } from './fieldEnrichment';
 
-/**
- * Does this cell have anything to render? **THE** definition of emptiness on
- * this surface (objectui#8376) — read by every one of the three places that
- * used to spell it out for itself:
- *
- *  - `isEmptyValue`, the row filter behind `emptyCount`, the reader's
- *    "Show N empty fields" toggle and the auto-hide heuristic;
- *  - the `isEmpty` branch of `displayValue`, which draws the muted em-dash +
- *    `No value` affordance;
- *  - `canCopy`, which offers the copy affordance on the row.
- *
- * The three MUST agree. "Show N empty fields" means "N rows show the em-dash",
- * the skeleton rule ("a section that is entirely empty keeps its labels") means
- * "every row shows the em-dash", and a row that says `No value` must not also
- * offer to copy that value. Three raw `null | undefined | ''` tests happened to
- * agree; one definition cannot stop agreeing.
- *
- * ## Why the scalar half DELEGATES (objectui#8350's authority)
- *
- * All three tests were raw and none TRIMMED, so `'   '` counted as FILLED
- * while `@object-ui/core`'s `recordDisplayValueAt` — the definition the page H1
- * and the `record:details` dedupe ladder both read — calls it EMPTY. The
- * consequences were not cosmetic: the row painted a visually blank cell (the
- * exact UI the em-dash exists to prevent), it escaped `emptyCount` so the
- * toggle read one too low, and because `shouldAutoHideEmpty` only needs
- * `filledCount > 0` ONE such value suppressed the all-empty skeleton and hid
- * every genuinely empty row in its section. So the scalar answer is not
- * re-spelled here: it is the authority's, and a `.trim()` written at this call
- * site would be the second implementation that drifts next.
- *
- * ## Why the OBJECT half does NOT delegate — measured, not assumed
- *
- * `recordDisplayValueAt` answers "does this resolve to a NAME", so an object
- * value goes through `displayNameOfEmbeddedObject` and is EMPTY whenever that
- * Salesforce-style chain yields nothing. That is right for a title and WRONG
- * for a cell: here an object value is handed to a TYPE-AWARE cell renderer that
- * knows how to draw it. `{ latitude, longitude }` renders as coordinates
- * (`LocationCellRenderer`), `{ street, city, … }` as a formatted postal address
- * (`AddressCellRenderer`, objectui#4037), `['alpha','beta']` as select badges,
- * any other object as JSON — none of which carries a name-ish key, so
- * delegating this half would replace populated cells with `No value`, drop them
- * out of `filledCount`, and let auto-hide bury them. An object is therefore a
- * VALUE here, exactly as it was before this change: this function moves
- * whitespace-only strings and nothing else.
- *
- * Pinned end-to-end (DOM, not predicate) in
- * `__tests__/DetailSection.emptinessAuthority-8376.test.tsx`, whose
- * NON-REGRESSION cases are red for a wholesale delegation.
- */
-function hasCellValue(value: unknown): boolean {
-  // Object/array values belong to the cell renderers, not to the display-name
-  // chain — see the docblock above. `typeof null === 'object'`, so null is
-  // excluded here and answered by the authority below.
-  if (value !== null && typeof value === 'object') return true;
-  // A one-key synthetic record is how a VALUE asks the authority its question:
-  // `recordDisplayValueAt` is keyed `(record, field)` because its callers read
-  // a field off a record, while this site's value has two sources (the record,
-  // then the authored `field.value` fallback) and is already resolved by the
-  // time emptiness is asked. Re-typing the test to take a value is precisely
-  // the extra implementation this function exists to remove.
-  return recordDisplayValueAt({ value }, 'value') !== undefined;
-}
-
 /**
  * Section-header icon. `fieldGroups[].icon` declares a Lucide name (spec),
  * so ASCII-identifier-ish values render as the real icon; anything else
@@ -225,8 +162,9 @@ export const DetailSection: React.FC<DetailSectionProps> = ({
   }, []);
 
   // Identify empty fields once for both filtering and the toggle counter —
-  // through `hasCellValue`, the ONE definition this file now shares with the
-  // em-dash affordance and the copy affordance (objectui#8376).
+  // through `hasCellValue` in `./emptiness`, the ONE definition this file
+  // shares with the em-dash affordance, the copy affordance (objectui#8376)
+  // and, since objectui#8394, every other band of the record page.
   const isEmptyValue = React.useCallback((field: DetailViewField) => {
     return !hasCellValue(data?.[field.name] ?? field.value);
   }, [data]);

packages/plugin-detail/src/DetailView.tsx

@@ -51,6 +51,7 @@ import { useLocalization, resolveFieldCurrency } from '@object-ui/i18n';
 import type { DetailViewSchema, DataSource, ActionSchema, SchemaNode } from '@object-ui/types';
 import { useDetailTranslation } from './useDetailTranslation';
 import { useRecordEditable } from './useRecordEditable';
+import { hasCellValue } from './emptiness';
 
 /** Default page size for related lists in the detail view */
 const DEFAULT_RELATED_PAGE_SIZE = 5;
@@ -435,7 +436,12 @@ export const DetailView: React.FC<DetailViewProps> = ({
         fieldDefMap[name] = { ...(fieldDefMap[name] || {}), ...def, name };
       }
     }
-    const has = (n: string) => data?.[n] !== undefined && data?.[n] !== null && data?.[n] !== '';
+    // The picker and the chip renderer below MUST ask the same question. This
+    // spelling is the same defect one rung earlier (objectui#8394): a
+    // whitespace-only `status` satisfied a raw test, so it won the single
+    // status slot — and then the render dropped it for being empty, leaving no
+    // status chip at all where a genuinely filled `stage` would have shown one.
+    const has = (n: string) => hasCellValue(data?.[n]);
     const picks: string[] = [];
     // 1) status / stage / state / select with options
     const statusKeys = ['status', 'stage', 'state', 'phase'];
@@ -976,7 +982,12 @@ export const DetailView: React.FC<DetailViewProps> = ({
                 </h1>
                 {effectiveSummaryFields.map((fieldName) => {
                   const val = data?.[fieldName];
-                  if (val === null || val === undefined || val === '') return null;
+                  // Same definition as `has` above and as every other band of
+                  // the page (`./emptiness`, objectui#8376/#8394). A raw test
+                  // here rendered a whitespace-only value as a visually blank
+                  // Badge beside the H1 — while the H1's own authority called
+                  // that field empty.
+                  if (!hasCellValue(val)) return null;
                   // Format value based on field type from schema or objectSchema.
                   // Best-effort: currency → localized currency, date/datetime →
                   // localized date string, others → String(val).

packages/plugin-detail/src/HeaderHighlight.tsx

@@ -28,6 +28,7 @@ import {
 } from './fieldEnrichment';
 import { NON_EDITABLE_SYSTEM_FIELDS } from './systemFields';
 import { useDetailTranslation } from './useDetailTranslation';
+import { hasCellValue } from './emptiness';
 
 export interface HeaderHighlightProps {
   fields: HighlightField[];
@@ -168,7 +169,15 @@ export const HeaderHighlight: React.FC<HeaderHighlightProps> = ({
               resolvedType === 'textarea' ||
               (!!resolvedType && EXPANDABLE_FIELD_TYPES.has(resolvedType));
             const isBoolean = resolvedType === 'boolean';
-            const isEmpty = value === null || value === undefined || value === '';
+            // The SAME definition the body grid draws its em-dash from
+            // (`./emptiness`, objectui#8376/#8394). This strip sits between the
+            // page H1 and the body grid, and the H1's own authority
+            // (`recordDisplayValueAt`) trims — so a raw test here made this the
+            // one band on the page still calling a whitespace-only value FILLED
+            // and painting a blank chip where the em-dash belongs. Objects stay
+            // values: they go to `CellRenderer` below, which knows how to draw
+            // them.
+            const isEmpty = !hasCellValue(value);
 
             // Compact-layout UX: an editor (select / date / lookup) needs more
             // room than a KPI number, so an actively-edited column widens to the

packages/plugin-detail/src/HistoryTimeline.tsx

@@ -27,6 +27,7 @@ import {
   TooltipTrigger,
   cn,
 } from '@object-ui/components';
+import { hasCellValue } from './emptiness';
 
 export interface HistoryChange {
   /** Raw field name from the schema (e.g. "industry"). */
@@ -130,8 +131,18 @@ function initialsFromName(name?: string | null): string {
     .join('');
 }
 
+/**
+ * An audit value as the timeline shows it, or the `'—'` that means "nothing".
+ *
+ * Emptiness is the record page's ONE definition (`./emptiness`,
+ * objectui#8376/#8394), not a raw test: a whitespace-only stored value used to
+ * fall through to the `typeof value === 'string'` branch below and print its
+ * spaces, so the cell the timeline means to read as "nothing" rendered blank
+ * instead of the em-dash. Objects are still values here for the same reason
+ * they are in a cell — the `JSON.stringify` branch below draws them.
+ */
 function formatDiffValue(value: unknown): string {
-  if (value === null || value === undefined || value === '') return '—';
+  if (!hasCellValue(value)) return '—';
   if (typeof value === 'string') return value;
   if (typeof value === 'number' || typeof value === 'boolean') return String(value);
   try {

packages/plugin-detail/src/RecordMetaFooter.tsx

@@ -12,6 +12,7 @@ import { getCellRenderer, resolveCellRendererType } from '@object-ui/fields';
 import { AUDIT_FIELD_BY_ROLE } from '@object-ui/types';
 import type { FieldMetadata } from '@object-ui/types';
 import { useDetailTranslation } from './useDetailTranslation';
+import { hasCellValue } from './emptiness';
 
 /**
  * Audit field names auto-injected by the framework's `applySystemFields` —
@@ -44,6 +45,34 @@ function toDate(value: unknown): Date | null {
   return null;
 }
 
+/**
+ * The actor a `created_by` / `updated_by` column names, or `undefined` when the
+ * record names none.
+ *
+ * ⚠️ Normalized HERE, at the read, and not inside `UserRef` — this is the one
+ * site on the record page where converging the emptiness predicate at the
+ * renderer would have been the wrong fix (objectui#8394). FOUR consumers below
+ * ask "is there an actor?" about the same value: `hasCreated` / `hasUpdated`,
+ * the `sameUser` comparison that suppresses a redundant "Updated" segment, the
+ * `label` choice between `detail.createdBy` and the "by"-less `detail.created`,
+ * and `MetaEntry`'s own `{user ? … }` gate — and only that last one ever
+ * reaches `UserRef`. So a whitespace-only `created_by` fixed at the renderer
+ * alone would still pick the "Created by" label and still draw the `·`
+ * separator, rendering "Created by · 5m ago" with nothing in between: exactly
+ * the dangling phrase the label branch exists to prevent, just with the blank
+ * moved. Answered once, all four agree.
+ *
+ * Emptiness is the record page's ONE definition (`./emptiness`), so an EXPANDED
+ * reference payload (`{ id, name }`) stays an actor — `UserRef` hands objects
+ * to `LookupCellRenderer`, which resolves them through its own display chain.
+ * The card guessed this site might want the TITLE predicate instead; it does
+ * not, and that is why: a bare `{ id }` payload has no display name, yet the
+ * renderer still draws it.
+ */
+function actorOrNone<T>(value: T): T | undefined {
+  return hasCellValue(value) ? value : undefined;
+}
+
 function formatRelativeTime(date: Date, t: TFn): string {
   const ms = Date.now() - date.getTime();
   // Future timestamps (clock skew, scheduled records) — fall through to "just now".
@@ -79,7 +108,9 @@ interface UserRefProps {
  * as DetailSection so reference resolution (ID → display name) is consistent.
  */
 const UserRef: React.FC<UserRefProps> = ({ value, objectSchema, fieldName }) => {
-  if (value === null || value === undefined || value === '') return null;
+  // Defensive floor only: `RecordMetaFooter` already normalizes through
+  // `actorOrNone`, so this agrees with the caller rather than deciding alone.
+  if (!hasCellValue(value)) return null;
   const fieldDef = objectSchema?.fields?.[fieldName];
   // created_by / updated_by are ALWAYS user references on ObjectStack, but many
   // fetched schemas omit the audit system fields from `fields`. Without a
@@ -180,8 +211,8 @@ export const RecordMetaFooter: React.FC<RecordMetaFooterProps> = ({
 
   const createdAt = toDate(data[AUDIT_FIELDS.createdAt]);
   const updatedAt = toDate(data[AUDIT_FIELDS.updatedAt]);
-  const createdBy = data[AUDIT_FIELDS.createdBy];
-  const updatedBy = data[AUDIT_FIELDS.updatedBy];
+  const createdBy = actorOrNone(data[AUDIT_FIELDS.createdBy]);
+  const updatedBy = actorOrNone(data[AUDIT_FIELDS.updatedBy]);
 
   const hasCreated = !!(createdAt || createdBy);
   // Treat updated_at within ~2s of created_at as "never touched" — covers