fix(core): read find() answers as QueryResult declares them in extractRecords

.changeset/6839-extract-records-records-arm.md

@@ -0,0 +1,56 @@
+---
+'@object-ui/core': minor
+---
+
+`extractRecords` reads a `find()` answer as `QueryResult` declares it: the
+`records` arm is gone (objectui#6839, following objectui#5945 / #6726 / #6840).
+
+`QueryResult` (`@object-ui/types`) declares exactly one rows member, `data`.
+This shared normaliser's ladder was `array -> records -> data -> value`, i.e.
+the undeclared spelling AHEAD of the contract's own member — the same
+precedence inversion objectui#5945 was filed about and objectui#6726 repaired by
+hand at seven other seams. The ladder is now `array -> data -> value`.
+
+`records` is the below-the-adapter spelling: `ObjectStackAdapter
+.normalizeQueryResult` and `ApiDataSource.normalizeQueryResult` (its
+`['data','items','results','records','value']` envelope loop) both CONSUME the
+server/SDK `records` envelope and return `data` before an answer reaches this
+helper, and every consumer calls it strictly above that fold. So no producer
+changes behaviour, because there is no producer; what changes is that a
+non-conforming one is refused instead of silently absorbed — and, being first,
+the arm used to outrank `data` when a producer emitted both.
+
+Reach, re-derived on this tree rather than taken from the card (which was
+measured six days before filing and is stale in three places): ten call sites in
+nine packages. Seven call `extractRecords` directly — `ObjectChart` (x2: the
+chart rows, and the group-by lookup label domain inside the exported
+`resolveGroupByLabels`), `ObjectDataTable`, `ObjectPivotTable`, `ObjectGantt`
+(the quick-filter option domain), `ObjectKanban`, `ObjectTimeline`. Four more
+reach it through `applyNonGridRowCeiling` (`@object-ui/react`), which is itself
+a published export and a sink in its own right: `ObjectCalendar`, `ObjectGantt`
+(its rows), `ObjectMap`, `ObjectTree`.
+
+Producer measurement, per consumer rather than once for all of them: no `find()`
+in any of those nine packages, nor in the apps and examples that mount them,
+emits a `records` envelope. CONTROL, so the zero is a reading rather than a
+miss — the same sweep finds `records` envelopes elsewhere: `ViewDataProvider`'s
+own `ResolvedData` (served by that module's own private reader of the same
+name), the raw Cloud HTTP payloads, the client-SDK doubles below
+`normalizeQueryResult`, the record-visibility batch route stubs, and one live
+`find()` double at `plugin-list`'s ObjectGallery — a consumer with its OWN
+unwrap ladder, which does not come through here and is untouched.
+
+The `value` arm STAYS. objectui#6840 removed `value` from `ObjectView`'s ladder
+on a measured zero at that seam and stated that its zero must not transfer here;
+it does not. Five `find()` doubles emit `{ value: [...] }` into this helper
+today (three in `plugin-kanban`, two in `plugin-calendar`), so the arm is live
+and its removal is a separate card with its own measurement.
+
+`QueryResult` is NOT widened to bless `records` — that is a published-type
+change and the maintainer's call, the same floor objectui#6726 and #6840
+respected.
+
+One refusal pin per module (`*.contractEnvelope-6839.*`), each keeping the live
+arms green alongside the deleted one, because live and dead is the whole
+distinction — plus a direct pin on the helper for the precedence question a
+per-module render pin cannot ask ("when BOTH keys are present, which wins").

packages/core/src/utils/__tests__/extract-records.contractEnvelope-6839.test.ts

@@ -0,0 +1,112 @@
+/**
+ * ObjectUI
+ * Copyright (c) 2024-present ObjectStack Inc.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ */
+
+/**
+ * `extractRecords` reads a `find()` answer as `QueryResult` DECLARES it — and
+ * does NOT read `records` (objectui#6839, following #5945 / #6726 / #6840).
+ *
+ * `QueryResult` (`@object-ui/types`) declares exactly one rows member: `data`.
+ * This helper's ladder was `array -> records -> data -> value`, i.e. `records`
+ * AHEAD of the contract's own member — the same precedence inversion #5945 was
+ * filed about and #6726 repaired by hand at seven other seams. `records` is
+ * the below-the-adapter spelling that `ObjectStackAdapter.normalizeQueryResult`
+ * and `ApiDataSource.normalizeQueryResult` already fold into `data` BELOW every
+ * consumer of this helper.
+ *
+ * ## What this file measures that the per-module pins cannot
+ *
+ * The nine renderers get their own `*.contractEnvelope-6839.*` pins, because a
+ * single "nothing reads `records`" assertion would pass even if one module
+ * routed around the helper. This file is the other half: the ARM ORDER itself.
+ * A per-module render pin answers "did a `records` envelope paint rows"; only a
+ * direct call can answer "when BOTH keys are present, which one wins" — and
+ * that question is the defect this card names in its title.
+ *
+ * ⚠️ Read the refusal cases together with the live ones. Every "returns []"
+ * assertion below is also satisfied by a helper that returns `[]` for
+ * EVERYTHING — an implementation strictly worse than the bug. The `data`, bare
+ * array and `value` cases are what refuse that implementation, and the two
+ * precedence cases refuse a helper that merely REORDERED the arms instead of
+ * deleting `records` (a reorder leaves `records` reachable whenever `data` is
+ * absent, which is precisely the tolerance AGENTS.md #0.1 is about).
+ *
+ * ⛔ `value` is NOT under test for removal here. It is LIVE at this seam — five
+ * `find()` doubles in `plugin-kanban` (3) and `plugin-calendar` (2) answer with
+ * `{ value: [...] }` today — and objectui#6840 explicitly refused to let its own
+ * zero at `ObjectView`'s seam transfer here. Its case below is a NON-REGRESSION
+ * case, not an endorsement of the key.
+ */
+
+import { describe, it, expect } from 'vitest';
+import { extractRecords } from '../extract-records';
+
+const ROWS = [{ id: 'r1', name: 'Ada' }, { id: 'r2', name: 'Grace' }];
+/** Distinct rows, so "which arm answered" is readable off the RESULT. */
+const OTHER = [{ id: 'x9', name: 'Nobody' }];
+
+describe('extractRecords — the find() envelope it reads (objectui#6839)', () => {
+  describe('the shapes it still reads — the live arms', () => {
+    it("reads the contract's `data` member", () => {
+      expect(extractRecords({ data: ROWS, total: 2 })).toEqual(ROWS);
+    });
+
+    it('reads a bare array — the live non-envelope shape fakes answer with', () => {
+      expect(extractRecords(ROWS)).toEqual(ROWS);
+    });
+
+    it('still reads `value` — LIVE at this seam (objectui#6840 refused to transfer its zero)', () => {
+      expect(extractRecords({ value: ROWS, total: 2 })).toEqual(ROWS);
+    });
+  });
+
+  describe('the shape it refuses', () => {
+    it('does NOT read `records` — not a QueryResult member', () => {
+      // Before the fix this returned the two rows, off a key the adapters
+      // below this seam have already folded into `data`.
+      expect(extractRecords({ records: ROWS, total: 2 })).toEqual([]);
+    });
+  });
+
+  describe('the arm ORDER — the defect this card is named for', () => {
+    it('`data` OUTRANKS `records`: a producer emitting both is read as QueryResult', () => {
+      // Before the fix `records` was tried FIRST, so this returned OTHER — the
+      // contract's own member was ignored in favour of a key it does not
+      // declare. This is the case a mere reorder would also pass, which is why
+      // the `records`-only case above is asserted alongside it.
+      expect(extractRecords({ data: ROWS, records: OTHER })).toEqual(ROWS);
+    });
+
+    it('`value` outranks `records` too — the deleted arm outranks nothing', () => {
+      expect(extractRecords({ value: ROWS, records: OTHER })).toEqual(ROWS);
+    });
+
+    it('a bare array outranks every envelope key', () => {
+      // The array leg sits above the object branch, so an array that also
+      // carries envelope-shaped own properties is read as the array it is.
+      // `Array.from` strips those own properties before the compare —
+      // `toEqual` weighs them, and would fail on the decoration rather than
+      // on the ELEMENTS, which is the only thing this case is about.
+      const withProps = Object.assign([...ROWS], { records: OTHER, data: OTHER });
+      expect(Array.from(extractRecords(withProps))).toEqual(ROWS);
+    });
+  });
+
+  describe('the shapes that were never rows', () => {
+    it.each([
+      ['null', null],
+      ['undefined', undefined],
+      ['a number', 42],
+      ['a string', 'rows'],
+      ['an empty object', {}],
+      ['a non-array `data`', { data: { id: 'r1' } }],
+      ['a non-array `value`', { value: 'rows' }],
+    ])('answers [] for %s', (_label, input) => {
+      expect(extractRecords(input)).toEqual([]);
+    });
+  });
+});

packages/core/src/utils/extract-records.ts

@@ -7,21 +7,76 @@
  */
 
 /**
- * Extract an array of records from various API response formats.
- * Supports: raw array, { records: [] }, { data: [] }, { value: [] }.
+ * Extract the rows array out of a `DataSource.find()` answer.
  *
- * This utility normalises the different shapes returned by ObjectStack,
- * OData, and MSW mock endpoints so that every data-fetching component
- * can rely on a single extraction path.
+ * Accepted shapes, in the order they are tried:
+ *
+ *   1. a bare array — the live non-envelope shape fakes and `ValueDataSource`
+ *      answer with;
+ *   2. `{ data: [] }` — the ONE rows member `QueryResult` (`@object-ui/types`)
+ *      declares;
+ *   3. `{ value: [] }` — the OData spelling, still LIVE at this seam (see
+ *      below).
+ *
+ * ## `records` is NOT read here (objectui#6839, following #5945 / #6726)
+ *
+ * A `records` arm used to sit AHEAD of `data` — the precedence inversion
+ * objectui#5945 was filed about and objectui#6726 repaired by hand in seven
+ * modules. `records` is the below-the-adapter spelling: both
+ * `ObjectStackAdapter.normalizeQueryResult` (`@object-ui/data-objectstack`)
+ * and `ApiDataSource.normalizeQueryResult` (its `['data','items','results',
+ * 'records','value']` envelope loop) CONSUME the server/SDK `records` envelope
+ * and return `data` before the answer ever reaches this helper. Every consumer
+ * below calls it strictly ABOVE that fold, so the arm was unreachable — and an
+ * unreachable tolerant arm is exactly where a non-conforming producer keeps
+ * working unrejected (AGENTS.md #0.1). Worse, being FIRST, it outranked the
+ * contract's own member: a producer emitting both would have had `data`
+ * ignored.
+ *
+ * Measured on this tree, per consumer rather than once for all of them, since
+ * this helper is reached from ten call sites in nine packages:
+ *
+ *   packages/plugin-charts/src/ObjectChart.tsx        (x2: rows, ref labels)
+ *   packages/plugin-dashboard/src/ObjectDataTable.tsx
+ *   packages/plugin-dashboard/src/ObjectPivotTable.tsx
+ *   packages/plugin-gantt/src/ObjectGantt.tsx         (ref option labels)
+ *   packages/plugin-kanban/src/ObjectKanban.tsx
+ *   packages/plugin-timeline/src/ObjectTimeline.tsx
+ *   packages/react/src/utils/nonGridRowCeiling.tsx    (applyNonGridRowCeiling,
+ *     itself the seam for ObjectCalendar, ObjectGantt, ObjectMap, ObjectTree)
+ *
+ * ZERO of them can be reached by a `records` envelope: no `find()` in any of
+ * those packages, nor in the apps and examples that mount them, emits one.
+ * CONTROL, so the zero is a reading rather than a miss — the same sweep DOES
+ * find `records` envelopes elsewhere: `ViewDataProvider`'s own `ResolvedData`
+ * (a different contract, served by that module's own private reader), the raw
+ * Cloud HTTP payloads, the client-SDK doubles below `normalizeQueryResult`,
+ * the record-visibility batch route stubs, and one live `find()` double at
+ * `plugin-list`'s ObjectGallery — a consumer with its OWN unwrap ladder, which
+ * does not come through here.
+ *
+ * ⛔ Do not restore the arm, and ⛔ do not widen `QueryResult` to bless
+ * `records` instead — that is a published-type change and the maintainer's
+ * call, the same floor objectui#6726 and #6840 respected. A producer that
+ * really does speak `records` belongs behind an adapter that folds it into
+ * `data`, which is what both adapters above already do.
+ *
+ * ## `value` STAYS — it is live here, and that is seam-local
+ *
+ * objectui#6840 / PR #6916 deleted the `value` arm from `ObjectView`'s ladder
+ * on a measured zero at THAT seam, and said in as many words that its zero
+ * must not be carried here. It does not: five `find()` doubles emit
+ * `{ value: [...] }` into this helper today (three in `plugin-kanban`, two in
+ * `plugin-calendar`). Deleting it here would break them. Whether `value`
+ * should survive at this seam is its own card with its own measurement.
+ *
+ * Pinned per module by the `*.contractEnvelope-6839.*` suites.
  */
 export function extractRecords(results: unknown): any[] {
   if (Array.isArray(results)) {
     return results;
   }
   if (results && typeof results === 'object') {
-    if (Array.isArray((results as any).records)) {
-      return (results as any).records;
-    }
     if (Array.isArray((results as any).data)) {
       return (results as any).data;
     }