fix(plugin-kanban): key the leftover sweep the way the injection reads (objectui#8993)

.changeset/8993-kanban-lane-id-sweep-key.md

@@ -0,0 +1,38 @@
+---
+'@object-ui/plugin-kanban': patch
+'@object-ui/types': patch
+---
+
+Fix `bucketCardsIntoColumns` double-bucketing every record on a board whose lane
+ids are not strings (objectui#8993).
+
+The function decided lane membership **twice**, with two key types:
+
+- **injection** reads `groups[col.id]`, and a property read coerces its key — a
+  lane `{ id: 1 }` correctly picks up the group stored under `'1'`;
+- **the leftover sweep** (objectui#2792) built its known-id `Set` from the RAW
+  `col.id` and filtered `Object.keys(groups)`, which are always strings. Since
+  `new Set([1]).has('1')` is `false`, every record the injection had just placed
+  was swept a second time into the trailing "Uncategorized" lane.
+
+⇒ A numeric-id board rendered **every card twice**. Silent by nature: the board
+draws, only the totals fail to reconcile.
+
+The repair is on the sweep side — the set now holds the same key spelling the
+property read uses, so both decisions agree. ⛔ Not on the injection side:
+making that read strict would break the coercion lanes depend on, and the string
+control could not catch it. A symbol id is the one key a property read does not
+stringify, so it is kept as-is rather than pushed through `String()`, which
+throws on symbols; `Object.keys` never yields a symbol, so such a lane keeps the
+reading it has today.
+
+**Visible change.** A board whose metadata already hits this — a picklist whose
+option `value` is a number, which reaches the renderer as a lane id through a
+path no schema guards — showed each card twice, once in its lane and once in
+"Uncategorized". It now shows each card once. The trailing lane still collects
+records whose group value matches no lane, which is objectui#2792's job and is
+pinned alongside the repair.
+
+`@object-ui/types` carries prose only: the lane-id `describe()` and TSDoc that
+stated the double-render as current behaviour now state it as repaired. The
+authored `id` stays `string` — one declared lane-id type beats two.

content/docs/api/schema-reference.md

@@ -945,7 +945,7 @@ A drag-and-drop Kanban board. The `object-kanban` type key validates the shape t
 
 > `groupField` is refused by name (objectui#7322): the renderer reads `groupBy`.
 
-> `columns` is declared on this face since objectui#8913, as the pair of array shapes `@objectstack/spec` declares — an array of `{ id, title }` lanes, **or** an array of bare value strings. A **mixed** array is refused: the renderer decides which shape it has from the first element alone, so a mix yields a blank lane and mis-bucketed cards. A lane accepts `id`, `title`, `cards`, `limit`, `className` and `collapsed`, which are the members the board implementations read; `id` is a **string** — a non-string lane id makes the board render every card twice, once in its lane and once in "Uncategorized" (objectui#8993). When a lane carries `cards`, each card is judged — a card with no `title` is refused. An undeclared lane key is accepted and dropped, not refused, which is this tolerant face's posture; the strict authoring face refuses it by name.
+> `columns` is declared on this face since objectui#8913, as the pair of array shapes `@objectstack/spec` declares — an array of `{ id, title }` lanes, **or** an array of bare value strings. A **mixed** array is refused: the renderer decides which shape it has from the first element alone, so a mix yields a blank lane and mis-bucketed cards. A lane accepts `id`, `title`, `cards`, `limit`, `className` and `collapsed`, which are the members the board implementations read; `id` is a **string** — the authored face keeps that narrowing, and since objectui#8993 a non-string lane id no longer renders every card twice: the bucketer's leftover sweep keys membership the way the injection already did (a lane `1` takes the group `'1'`). When a lane carries `cards`, each card is judged — a card with no `title` is refused. An undeclared lane key is accepted and dropped, not refused, which is this tolerant face's posture; the strict authoring face refuses it by name.
 >
 > ⚠️ The **bare-string array is accepted but inert on this block.** It is declared so this package does not refuse an authoring the protocol allows. The renderer reads a bare-string lane list only when a board has no `groupBy`, and `groupBy` is required here — so on `object-kanban` the strings are always ignored and the lanes come from the group field's picklist options or from the data. Write the `{ id, title }` array to control the lanes. The requiredness of `groupBy` is tracked as objectui#8990.
 >

content/docs/plugins/plugin-kanban.mdx

@@ -149,7 +149,7 @@ An undeclared lane key is accepted and dropped, not refused.
 
 | Property | Type | Description |
 |----------|------|-------------|
-| `id` | string | Lane identifier, matched against the `groupBy` value. Strings only: a non-string id makes the board render every card twice, once in its lane and once in "Uncategorized" (objectui#8993) |
+| `id` | string | Lane identifier, matched against the `groupBy` value. Declare a string; since objectui#8993 a non-string id is matched by its string spelling (a lane `1` takes the group `'1'`) instead of rendering every such card twice |
 | `title` | string | Lane title, localized against the `groupBy` picklist's option labels |
 | `cards` | KanbanCard[] | Cards this lane carries — **optional**, and a static board's option; an object-bound board's cards arrive from the record source |
 | `limit` | number | Max cards allowed (WIP limit). Never reaches the query — the fetch window is the board's `limit` |

packages/plugin-kanban/src/__tests__/laneIdCoercion-8993.test.ts

@@ -0,0 +1,180 @@
+/**
+ * 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.
+ */
+
+/**
+ * objectui#8993 — `bucketCardsIntoColumns` decides lane membership TWICE, and
+ * before this pin the two decisions used different key types.
+ *
+ * ## The mechanism
+ *
+ * - **Injection** reads `groups[col.id]`. A property read coerces its key, so
+ *   a lane `{ id: 1 }` correctly picks up the group stored under `'1'`.
+ * - **The leftover sweep** (#2792) built its known-id Set from the RAW value
+ *   and filtered `Object.keys(groups)` — which are ALWAYS strings. Since
+ *   `new Set([1]).has('1')` is `false`, every record the injection had just
+ *   placed was swept a second time into the trailing "Uncategorized" lane.
+ *
+ * ⇒ A numeric-id board rendered every card twice. Silent: the board draws, the
+ * totals just do not reconcile, which is why the counting row below exists.
+ *
+ * ## What each row is for
+ *
+ * 1. NUMERIC — the reported reading. Was `'1:r1, 2:r2, __uncolumned__:r1+r2'`.
+ * 2. STRING CONTROL — a live control, green on BOTH sides of the repair. It is
+ *    what makes row 1 a statement about the id TYPE rather than about bucketing
+ *    in general; if it ever reddens, the repair went the wrong way.
+ * 3. THE CLASS — the defect is "the id is not a string", not "the id is a
+ *    number", so every non-string id type the function can be handed is pinned,
+ *    including the falsy ones (`0`, `false`) that also skip the label map.
+ * 4. THE LABEL PATH — a record can reach `groups` through the label→id map,
+ *    which stores the RAW id as its VALUE. That path double-bucketed too, so
+ *    one row keeps it honest; keying only the direct path would leave it open.
+ * 5. THE SWEEP STILL SWEEPS — the control that separates a repair from a
+ *    deletion. Rows 1-4 would all pass if the sweep were simply removed, and
+ *    that would resurrect #2792 (records silently dropped). An unmatched record
+ *    must still surface in the trailing lane.
+ * 6. NO NEW COLLISION — coercing a key can MINT a collision where there was
+ *    none, so the boundary is asserted rather than argued: `String(null)` is
+ *    `'null'` and `String(undefined)` is `'undefined'`, and neither may swallow
+ *    the empty key that a null/absent VALUE produces; a lane declared with the
+ *    `KANBAN_UNCOLUMNED_ID` spelling keeps its existing reading; and a symbol
+ *    id — the one key a property read does NOT stringify — must keep behaving
+ *    as it does today instead of throwing inside `String()`.
+ *
+ * ⛔ The repair belongs on the SWEEP side. Making the injection strict would
+ * pass rows 1-2 while breaking the coercion lanes depend on, and the string
+ * control could not catch it.
+ */
+
+import { describe, it, expect } from 'vitest';
+import { bucketCardsIntoColumns, KANBAN_UNCOLUMNED_ID } from '../index';
+
+/** `id:card+card` per lane, in lane order — the whole board in one string. */
+const summarise = (lanes: Array<any>): string =>
+  lanes
+    .map((l: any) => `${String(l.id)}:${(l.cards || []).map((c: any) => c.id).join('+')}`)
+    .join(', ');
+
+const bucket = (columns: Array<any>, data: Array<any>): string =>
+  summarise(bucketCardsIntoColumns(columns, data, 'status', undefined, 'Uncategorized'));
+
+/** Every card id the board renders, across every lane, duplicates included. */
+const renderedCardIds = (columns: Array<any>, data: Array<any>): string[] =>
+  bucketCardsIntoColumns(columns, data, 'status', undefined, 'Uncategorized').flatMap(
+    (l: any) => (l.cards || []).map((c: any) => c.id),
+  );
+
+describe('bucketCardsIntoColumns — lane id key coercion (objectui#8993)', () => {
+  it('row 1 — NUMERIC lane ids bucket each record exactly once', () => {
+    expect(
+      bucket(
+        [
+          { id: 1, title: 'One' },
+          { id: 2, title: 'Two' },
+        ],
+        [
+          { id: 'r1', status: 1 },
+          { id: 'r2', status: '2' },
+        ],
+      ),
+    ).toBe('1:r1, 2:r2');
+  });
+
+  it('row 2 — STRING control: unchanged, green on both sides of the repair', () => {
+    expect(bucket([{ id: 'one', title: 'One' }], [{ id: 'r1', status: 'one' }])).toBe('one:r1');
+  });
+
+  describe('row 3 — the class is "id is not a string", not "id is a number"', () => {
+    const cases: Array<{ name: string; id: unknown; value: unknown; lane: string }> = [
+      { name: 'number', id: 1, value: 1, lane: '1' },
+      { name: 'zero (falsy, so the label map skips it)', id: 0, value: 0, lane: '0' },
+      { name: 'boolean true', id: true, value: true, lane: 'true' },
+      { name: 'boolean false (falsy)', id: false, value: false, lane: 'false' },
+      { name: 'null', id: null, value: 'null', lane: 'null' },
+      { name: 'undefined (a lane with no id at all)', id: undefined, value: 'undefined', lane: 'undefined' },
+      { name: 'NaN', id: NaN, value: NaN, lane: 'NaN' },
+      { name: 'array', id: [1, 2], value: '1,2', lane: '1,2' },
+      { name: 'plain object', id: { a: 1 }, value: '[object Object]', lane: '[object Object]' },
+    ];
+
+    for (const { name, id, value, lane } of cases) {
+      it(`${name} — one lane, one card, no trailing "Uncategorized"`, () => {
+        const columns = [{ id, title: 'Lane' }];
+        const data = [{ id: 'r1', status: value }];
+        expect(bucket(columns, data)).toBe(`${lane}:r1`);
+        // The counting row: the visible card total must reconcile with the
+        // record count. This is the user-visible symptom, stated directly.
+        expect(renderedCardIds(columns, data)).toEqual(['r1']);
+      });
+    }
+  });
+
+  it('row 4 — the label→id path is keyed the same way (it stores the RAW id)', () => {
+    // 'One' matches the lane's TITLE, so the group key comes out of
+    // `labelToColumnId`, whose stored VALUE is the raw numeric id.
+    expect(bucket([{ id: 1, title: 'One' }], [{ id: 'r1', status: 'One' }])).toBe('1:r1');
+  });
+
+  it('row 5 — the sweep still sweeps: an unmatched record still surfaces (#2792)', () => {
+    // A repair that simply deleted the sweep would pass every row above and
+    // silently drop this record — the defect #2792 closed.
+    expect(
+      bucket(
+        [
+          { id: 1, title: 'One' },
+          { id: 2, title: 'Two' },
+        ],
+        [
+          { id: 'r1', status: 1 },
+          { id: 'r9', status: 'retired_option' },
+        ],
+      ),
+    ).toBe(`1:r1, 2:, ${KANBAN_UNCOLUMNED_ID}:r9`);
+  });
+
+  describe('row 6 — the coercion mints no new collision', () => {
+    it("a null lane id does not swallow the empty key a null VALUE produces", () => {
+      // `String(null)` is 'null', but a record whose group VALUE is null is
+      // keyed '' (`String(item[groupBy] ?? '')`). The two must stay apart.
+      expect(bucket([{ id: null, title: 'N' }], [{ id: 'r1', status: null }])).toBe(
+        `null:, ${KANBAN_UNCOLUMNED_ID}:r1`,
+      );
+    });
+
+    it("an undefined lane id does not swallow the empty key an ABSENT value produces", () => {
+      expect(bucket([{ title: 'U' }], [{ id: 'r1' }])).toBe(
+        `undefined:, ${KANBAN_UNCOLUMNED_ID}:r1`,
+      );
+    });
+
+    it('a lane declared with the sentinel spelling keeps its own records', () => {
+      const lanes = bucketCardsIntoColumns(
+        [{ id: KANBAN_UNCOLUMNED_ID, title: 'Declared' }],
+        [{ id: 'r1', status: KANBAN_UNCOLUMNED_ID }],
+        'status',
+        undefined,
+        'Uncategorized',
+      );
+      expect(summarise(lanes)).toBe(`${KANBAN_UNCOLUMNED_ID}:r1`);
+      expect(lanes).toHaveLength(1); // no second lane under the same id
+    });
+
+    it("a symbol lane id keeps today's reading instead of throwing", () => {
+      // A symbol is the one key a property read does NOT stringify, so it is
+      // never pushed through `String()` (which throws on symbols). `Object.keys`
+      // never yields a symbol, so such a lane matches nothing — exactly as
+      // before this repair.
+      const columns = [{ id: Symbol('lane'), title: 'S' }];
+      const data = [{ id: 'r1', status: 'x' }];
+      expect(() => bucketCardsIntoColumns(columns, data, 'status', undefined, 'Uncategorized'))
+        .not.toThrow();
+      expect(renderedCardIds(columns, data)).toEqual(['r1']);
+      expect(bucket(columns, data)).toBe(`Symbol(lane):, ${KANBAN_UNCOLUMNED_ID}:r1`);
+    });
+  });
+});

packages/plugin-kanban/src/index.tsx

@@ -97,7 +97,20 @@ export function bucketCardsIntoColumns(
   // still counts. Surface them in a trailing "Uncategorized" lane; dragging
   // one out to a real column repairs its status (the drag handler refuses to
   // persist a move INTO here).
-  const knownIds = new Set(columns.map((col: any) => col.id));
+  // ⚠️ Key this membership test the way the injection above keys its READ.
+  // `groups[col.id]` is a property read, so it coerces the id: a lane
+  // `{ id: 1 }` correctly picks up the group stored under `'1'`, and every key
+  // `Object.keys(groups)` yields is a string. A Set built from the RAW id
+  // therefore answers `new Set([1]).has('1') === false` and sweeps the very
+  // records the injection already took — the board renders each of them twice,
+  // once in its lane and once in "Uncategorized" (objectui#8993). Membership is
+  // decided twice here, so both decisions must use the same key spelling.
+  // A symbol is the one id a property read does NOT stringify, so it is kept
+  // as-is rather than pushed through `String()` (which throws on symbols):
+  // `Object.keys` never yields a symbol, so such a lane keeps today's reading.
+  const knownIds = new Set<PropertyKey>(
+    columns.map((col: any) => (typeof col.id === 'symbol' ? col.id : String(col.id))),
+  );
   const uncolumnedCards = Object.keys(groups)
     .filter((key) => !knownIds.has(key))
     .flatMap((key) => groups[key]);

packages/types/src/__tests__/object-kanban-columns-declared-8913.test.ts

@@ -63,12 +63,13 @@
  *   - **a NUMERIC lane id.** The first cut admitted one, justified by "the
  *     renderer coerces with `String(col.id)`". That is true at the i18n lookup
  *     in `localizeColumn` and FALSE where lane membership is decided: the
- *     bucketer builds `knownIds` from the RAW `col.id` and compares it with
+ *     bucketer built `knownIds` from the RAW `col.id` and compared it with
  *     `Object.keys(groups)`, which are strings. Lanes `{ id: 1 }, { id: 2 }`
- *     with records `status: 1` / `status: '2'` come back
+ *     with records `status: 1` / `status: '2'` came back
  *     `1:r1, 2:r2, __uncolumned__:r1+r2` — every card rendered TWICE — against
- *     a clean `one:r1` string control. Carrier for the renderer defect:
- *     objectui#8993. `KanbanColumn.id` and its mirror are `string`, and the
+ *     a clean `one:r1` string control. objectui#8993 repaired that sweep; the
+ *     refusal stays because one declared lane-id type beats two, not because
+ *     the renderer is still broken. `KanbanColumn.id` and its mirror are `string`, and the
  *     protocol names no type, so refusing it here is not a narrowing below the
  *     protocol.
  *   - **a MIXED array.** `columns` is a UNION OF TWO ARRAYS, not an array of a
@@ -208,7 +209,7 @@ const REFUSED: Array<readonly [string, unknown]> = [
   // before this card AND under objectui#8913's first cut, which is why they
   // belong here with the same control as every other row: the failure they
   // guard is a declaration blessing a shape the renderer mishandles.
-  ['⭐ a NUMERIC lane id — the bucketer renders every such card twice (objectui#8993)', [{ id: 1, title: 'Stage one' }]],
+  ['⭐ a NUMERIC lane id — the bucketer rendered every such card twice until objectui#8993', [{ id: 1, title: 'Stage one' }]],
   ['⭐ a MIXED array, string first — the renderer ignores the whole list', ['todo', { id: 'done', title: 'Done' }]],
   ['⭐ a MIXED array, object first — the renderer emits a blank lane and mis-buckets', [{ id: 'done', title: 'Done' }, 'todo']],
 ];

packages/types/src/objectql.ts

@@ -3067,17 +3067,19 @@ export interface ObjectKanbanSchema extends BaseSchema {
          *
          * STRING only. objectui#8913's first cut admitted a number here on the
          * reading that "the renderer coerces with `String(col.id)`" — TRUE at
-         * the i18n lookup in `localizeColumn`, and FALSE at the site that
-         * decides lane membership: `bucketCardsIntoColumns` builds its
-         * `knownIds` set from the RAW `col.id` and compares it against
+         * the i18n lookup in `localizeColumn`, and FALSE, at the time, where
+         * lane membership is decided: `bucketCardsIntoColumns` built its
+         * `knownIds` set from the RAW `col.id` and compared it against
          * `Object.keys(groups)`, which are always strings. Measured by calling
          * the real function with lanes `{ id: 1 }, { id: 2 }` and records
          * `status: 1` / `status: '2'`: `1:r1, 2:r2, __uncolumned__:r1+r2` —
          * every card rendered TWICE, once in its lane and once in
-         * "Uncategorized"; the string control is a clean `one:r1`. The retired
-         * arm ({@link KanbanColumn.id}) and its mirror are `string` too, and the
-         * protocol names no type. Carrier for the bucketer defect itself:
-         * objectui#8993.
+         * "Uncategorized"; the string control was a clean `one:r1`.
+         * objectui#8993 repaired that sweep, so the two decisions now use one
+         * key spelling — the narrowing stays because the retired arm
+         * ({@link KanbanColumn.id}) and its mirror are `string` too and one
+         * declared type per lane id beats two, not because the renderer is
+         * still broken.
          */
         id: string;
         /** Lane heading; localized against the `groupBy` picklist's option labels. */