refactor(plugin-dashboard): one relation predicate, not two that agree by coincidence

.changeset/5876-one-lookup-predicate.md

@@ -0,0 +1,34 @@
+---
+'@object-ui/plugin-dashboard': patch
+---
+
+The dashboard package now holds ONE relation predicate instead of two that
+agreed only because a sweep had just aligned them (objectui#5876).
+
+`computeLookupExpand` in `ObjectDataTable.tsx` carried its own `isLookup`,
+byte-identical to the exported `isLookupType` in `recordFields.tsx` after
+objectui#5692 pointed both at `@object-ui/core`'s `EXPANDABLE_FIELD_TYPES`.
+Nothing kept them aligned: a future edit to either — a member added, the
+retirement gate moved — would have re-forked the `$expand` decision from the
+predicate whose docblock claims to drive it. `computeLookupExpand` now calls
+`isLookupType`, which gains its first production consumer, and the module no
+longer imports the shared family or the retirement gate at all.
+
+**No behaviour changes**, and that is measured rather than assumed:
+
+- The two bodies were identical, so every boolean answer — `tree` is expanded,
+ `reference` is not, ordinary relations are — is the same before and after.
+- The retired-spelling warning is not emitted a different number of times.
+ `reportRetiredFieldType` dedupes per SPELLING in one module-level set inside
+ `@object-ui/core`, which both bodies already shared, so routing two callers
+ through one function cannot change the count.
+
+Nothing published moves: `isLookupType` is not re-exported from
+`@object-ui/plugin-dashboard`'s entry, so this is internal shape only.
+
+Because a refactor with no observable delta cannot be pinned by a behavioural
+test — a byte-identical local copy satisfies every assertion you can write
+about `$expand` — the pin is identity, in
+`__tests__/expandableFamily.identity-5692.test.ts`: `computeLookupExpand` is
+observed CALLING `isLookupType`, and `ObjectDataTable.tsx` is read at source
+level to confirm no second body survives for it to call instead.

packages/plugin-dashboard/src/ObjectDataTable.tsx

@@ -13,13 +13,6 @@ import {
  isDrillEnabled,
  columnIdentity,
  columnHeader,
- // The retirement gate (objectui#4914, ruling B) — `@object-ui/fields`
- // re-exports the same function object; read here from its home.
- isRetiredFieldType,
- reportRetiredFieldType,
- // The reference-bearing field family (objectui#5692). Read, never copied —
- // see the convergence note on `computeLookupExpand`.
- EXPANDABLE_FIELD_TYPES,
 } from '@object-ui/core';
 import type { DrillDownConfig } from '@object-ui/types';
 import { Skeleton, RefreshIndicator, cn } from '@object-ui/components';
@@ -30,6 +23,9 @@ import {
  renderFieldValue,
  isNumericFieldMeta,
  isSystemField,
+ // The package's single relation predicate (objectui#5876). The retirement
+ // gate and the family read live in ITS body — never restated here.
+ isLookupType,
 } from './recordFields';
 import { RecordDetailDrawer } from './RecordDetailDrawer';
 
@@ -176,8 +172,10 @@ export function normalizeColumns(columns: (string | Record<string, any>)[]): Nor
 /**
  * Compute the list of lookup-typed accessors that should be expanded when
  * fetching rows. Returns column accessors whose object schema field type is
- * a relation. Which types those are is NOT restated here: it is
- * {@link EXPANDABLE_FIELD_TYPES}, the family `@object-ui/core` publishes. Used
+ * a relation. Neither the type family nor the test itself is restated here:
+ * this delegates to {@link isLookupType} in `recordFields.tsx`, the package's
+ * single relation predicate, which reads `EXPANDABLE_FIELD_TYPES` — the family
+ * `@object-ui/core` publishes. Used
  * by the dashboard table widget to ask the data adapter to populate referenced
  * records (e.g. `account: { id, name }`) so cells don't show raw FK ids.
  *
@@ -211,6 +209,19 @@ export function normalizeColumns(columns: (string | Record<string, any>)[]): Nor
  * data: the spelling is absent from `@objectstack/spec`'s closed `FieldType`
  * and refused by `FieldSchema.safeParse`, so no object schema can declare a
  * field whose stored type is `reference`.
+ *
+ * ## One predicate, not two that agree by coincidence (objectui#5876)
+ *
+ * This function used to carry its own `isLookup`, byte-identical to
+ * `isLookupType` once objectui#5692 had pointed both at the same set — two
+ * bodies that agreed because one sweep aligned them, with nothing keeping them
+ * aligned afterwards. The test IS `isLookupType` now, so this module no longer
+ * IMPORTS the shared family or the retirement gate and no longer CALLS either
+ * (they are named in this prose and nowhere else in the file). That absence is
+ * the assertion: a BEHAVIOURAL test cannot see this change, because a
+ * byte-identical local copy satisfies every boolean claim you can make about
+ * `$expand`. The pin that can see it is the identity pin in
+ * `__tests__/expandableFamily.identity-5692.test.ts`.
  */
 export function computeLookupExpand(
  schema: { columns?: any[]; objectName?: string },
@@ -223,17 +234,6 @@ export function computeLookupExpand(
  } else {
  for (const [name, def] of Object.entries(objectSchema.fields)) fieldsByName[name] = { name, ...(def as any) };
  }
- const isLookup = (t: unknown) => {
- if (typeof t === 'string' && isRetiredFieldType(t)) {
- reportRetiredFieldType(t);
- return false;
- }
- // Never `new Set([...EXPANDABLE_FIELD_TYPES, …])` and never a re-listing of
- // its members: a copy re-forks the table, which is the defect this removed,
- // and the identity pin fails on it by design.
- return EXPANDABLE_FIELD_TYPES.has(t as string);
- };
-
  const cols = Array.isArray(schema.columns) ? schema.columns : [];
  const out = new Set<string>();
 
@@ -251,14 +251,14 @@ export function computeLookupExpand(
  .filter(Boolean);
  for (const acc of accessors) {
  const def = fieldsByName[acc];
- if (def && isLookup(def.type)) out.add(acc);
+ if (def && isLookupType(def.type)) out.add(acc);
  }
  } else {
  // No columns whitelist (auto-derive mode, e.g. drill-down drawer):
  // expand every lookup-type field known from the schema so cells show
  // the related record's display name instead of a bare FK id.
  for (const [name, def] of Object.entries(fieldsByName)) {
- if (isLookup((def as any)?.type)) out.add(name);
+ if (isLookupType((def as any)?.type)) out.add(name);
  }
  }
  return Array.from(out);

packages/plugin-dashboard/src/__tests__/expandableFamily.identity-5692.test.ts

@@ -45,7 +45,29 @@
  * pin goes red too and its `reference` pin flips; the ordinary-relation
  * regression controls stay GREEN in both directions, which is what makes them
  * controls rather than duplicates of the pins.
+ *
+ * ## objectui#5876 — one predicate, and why NO behavioural test can pin it
+ *
+ * #5692 left the package with TWO byte-identical bodies: `isLookupType` here
+ * and a local `isLookup` inside `computeLookupExpand`. Collapsing the second
+ * into the first is observationally FREE — every boolean claim about `$expand`
+ * (`tree` expands, `reference` does not, ordinary relations do) answers the
+ * same before and after, so every assertion in the three describes above would
+ * stay GREEN on a revert. They are controls for this change, not pins of it.
+ * The `reportRetiredFieldType` count does not move either: its dedupe key is
+ * the SPELLING, in one module-level Set inside `@object-ui/core`, which both
+ * bodies already shared (`retired-field-types.ts` — "the dedupe is per
+ * SPELLING, not per face").
+ *
+ * So the pin below is IDENTITY at the call level, in two halves that fail for
+ * different reasons: `computeLookupExpand` must be observed CALLING
+ * `isLookupType`, and `ObjectDataTable.tsx` must hold no second body for it to
+ * call instead. Predicted ablation: restore the local `isLookup` and BOTH go
+ * RED while everything above stays green.
  */
+import { readFileSync } from 'node:fs';
+import path from 'node:path';
+import { fileURLToPath } from 'node:url';
 import { describe, it, expect, vi } from 'vitest';
 import { EXPANDABLE_FIELD_TYPES } from '@object-ui/core';
 import { FieldType } from '@objectstack/spec/data';
@@ -115,6 +137,72 @@ describe("the dashboard's relation rule is core's object, not a copy (objectui#5
  });
 });
 
+describe('one relation predicate, not two that agree by coincidence (objectui#5876)', () => {
+ it('`computeLookupExpand` CALLS `isLookupType` — in BOTH column modes', () => {
+ // Surgical by construction: `vi.doMock` is not hoisted, so it binds only
+ // the dynamic import below and leaves every other test in this file on the
+ // real module. The double delegates to the real implementation, so the
+ // exercise still answers correctly — what is being read is WHO answered.
+ return (async () => {
+ const actual = await import('../recordFields');
+ const double = vi.fn(actual.isLookupType);
+ vi.resetModules();
+ vi.doMock('../recordFields', () => ({ ...actual, isLookupType: double }));
+ try {
+ const { computeLookupExpand: subject } = await import('../ObjectDataTable');
+ const modes: [string, () => string[]][] = [
+ ['explicit whitelist', () => subject({ columns: ALL_COLUMNS }, objectSchema())],
+ ['auto-derive', () => subject({}, objectSchema())],
+ ];
+ for (const [label, exercise] of modes) {
+ double.mockClear();
+ // Control: the mode still answers, so an assertion about WHO was
+ // asked cannot pass on a subject that did nothing at all.
+ expect(exercise(), `${label} expanded nothing`).toContain('account');
+ expect(
+ double.mock.calls.map(([t]) => t),
+ `${label} answered the relation question without asking isLookupType`,
+ ).toContain('lookup');
+ }
+ } finally {
+ vi.doUnmock('../recordFields');
+ vi.resetModules();
+ }
+ })();
+ });
+
+ it('`ObjectDataTable.tsx` holds no second predicate body', () => {
+ // The call pin above can be satisfied while a dead copy still sits in the
+ // file; this half is what makes "one predicate" true of the SOURCE. Read
+ // with comments stripped, so the prose that NAMES these symbols in the
+ // convergence note cannot fake a hit.
+ const code = readFileSync(
+ path.join(path.dirname(fileURLToPath(import.meta.url)), '..', 'ObjectDataTable.tsx'),
+ 'utf8',
+ )
+ .replace(/\/\*[\s\S]*?\*\//g, '')
+ .replace(/\/\/[^\n]*/g, '');
+
+ // Controls — chosen to be INVARIANT under the ablation this pin guards
+ // against, so that a mis-resolved path or an over-eager stripper fails
+ // HERE, as a broken probe, while a genuine revert fails on the subject
+ // lines below with an honest message. (Measured: an earlier draft used the
+ // `isLookupType` call count as its control, and a real revert then reported
+ // itself as "probe stripped the code away" — the two failures were
+ // indistinguishable, which is the whole thing a control exists to prevent.)
+ expect(code, 'probe read the wrong file').toContain('export function computeLookupExpand(');
+ expect(code, 'probe stripped the code away').toContain('out.add(acc)');
+
+ // Subject — each of these moves if the local copy comes back.
+ expect(
+ code.match(/isLookupType\(/g) ?? [],
+ 'computeLookupExpand stopped calling the shared predicate',
+ ).toHaveLength(2);
+ expect(code, 'a second family read lives here').not.toContain('EXPANDABLE_FIELD_TYPES');
+ expect(code, 'a second retirement gate lives here').not.toContain('RetiredFieldType');
+ });
+});
+
 describe('the ordinary relations are untouched — regression control', () => {
  // These must stay green through BOTH ablation legs. If they move, the
  // convergence took the whole whitelist with it and the pins above are