perf(renderer-dom): grouping apply pays one height-index replacement, not three (#522)

* perf(renderer-dom): a column change stops re-ingesting the height index

A grouping apply paid three full-set cooperative height-index replacements
at 50k rows (~0.6s before READY): the engine's reset commit, then two
setColumns restarts — the group-column roster commit and the merged-width
commit a render later — each cancelling the previous build to re-ingest the
same 50,004 rows. Traced in headed Chromium with cause-tagged starts.

The replacement source never reads columns; only the estimator does, and
estimates are applied at publish off the live layoutColumns. So setColumns
now absorbs the change in place: idle controllers clear estimates
synchronously via the new layout-core RowHeightIndex.clearEstimates
(measurements survive — they are DOM facts, not arithmetic) and republish;
during an active replacement the live columns update and the in-flight
build's own finishing publish honors them. New diagnostics counters
(columnsResetPathCount / columnsResetFallbackCount) follow the
reorder/refilter pattern, fallback included.

Pinned end to end: a react grouping apply costs exactly ONE height-index
replacement (was two in jsdom, three in a browser); the columns reset keeps
measurements, clears offscreen estimates, and keeps the dense lane dense.
All pins mutation-verified.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* test(renderer-dom): the spacer anchor test's comment names the real mechanism

setColumns no longer runs a cooperative replacement; the anchor here is
restored through the columns reset's restoreAnchorRequest. The assertions
were already mechanism-agnostic and unchanged.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* test: review follow-ups — the columns-reset fallback fires, and a data cell paints

1. The columns-reset fallback now has its firing pinned, like the sibling
   reorder/refilter fallbacks: an estimator that vetoes the first publish
   under the new columns advances columnsResetFallbackCount, starts a full
   replacement instead of an error state, and the recovery keeps the
   measurement. Mutation-verified (swallowing the increment reddens it).
2. The grouping-apply pin additionally asserts a known data row's cell
   content is in the DOM — blank-grid insurance against the
   counts-agree-nothing-paints defect class.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>

Author: blove

.changeset/grouping-commit-height-index.md

@@ -0,0 +1,19 @@
+---
+"@pretable-internal/layout-core": patch
+"@pretable-internal/renderer-dom": patch
+---
+
+A column change no longer re-ingests the whole height index.
+
+`setColumns` changes only the estimator's inputs — the height-index
+replacement source never reads columns — yet it restarted a full cooperative
+replacement, which made one grouping apply cost three full-set height-index
+passes at 50k rows (the engine's reset, then the group-column roster commit
+and the merged-width commit, each cancelling the build before it). The
+controller now absorbs a column change in place: on an idle controller it
+clears estimates synchronously (`RowHeightIndex.clearEstimates`, new in
+layout-core — measurements survive; they are DOM facts, not arithmetic) and
+republishes, and during an active replacement it just updates the live
+columns, which the in-flight build's finishing publish already reads. A
+grouping apply now costs exactly one height-index replacement, test-pinned
+end to end.

packages/layout-core/src/__tests__/row-height-index.test.ts

@@ -2785,3 +2785,98 @@ describe("dense refilter and reorder (Amendment I, Task 3)", () => {
     expect(withXBack.getHeight(2)).toBe(25);
   });
 });
+
+describe("clearEstimates", () => {
+  test("drops estimates to the default height, keeps measurements, and leaves totals honest", () => {
+    const a = data("a");
+    const b = data("b");
+    const c = data("c");
+    const base = createIndex(
+      [entry(a, 50), entry(b), entry(c, 70)],
+      30,
+    ).measure(2, c, 66);
+    expect(base.getTotalHeight()).toBe(50 + 30 + 66);
+
+    const cleared = base.clearEstimates();
+    // Estimated, unmeasured rows return to the default height...
+    expect(cleared.getHeight(0)).toBe(30);
+    expect(cleared.getHeight(1)).toBe(30);
+    // ...while a measured row keeps the height the DOM reported — the
+    // measurement is a fact, not arithmetic, and must survive.
+    expect(cleared.getHeight(2)).toBe(66);
+    expect(cleared.hasMeasurement(c)).toBe(true);
+    expect(cleared.getTotalHeight()).toBe(30 + 30 + 66);
+    // A later re-estimate is accepted (the estimatedHeight slot really is
+    // empty now — an update carrying the SAME estimate as before must not
+    // no-op away).
+    const reEstimated = cleared.apply([
+      { kind: "update", ref: a, index: 0, estimatedHeight: 50 },
+    ]);
+    expect(reEstimated.getHeight(0)).toBe(50);
+  });
+
+  test("is an identity no-op when nothing is estimated", () => {
+    const a = data("a");
+    const b = data("b");
+    const base = createIndex([entry(a), entry(b)], 24).measure(1, b, 40);
+    expect(base.clearEstimates()).toBe(base);
+    const empty = createIndex([], 24);
+    expect(empty.clearEstimates()).toBe(empty);
+  });
+
+  test("preserves retained (tombstoned) measurements and the retention order", () => {
+    const a = data("a");
+    const gone = data("gone");
+    const base = createIndex([entry(a, 20), entry(gone)], 30)
+      .measure(1, gone, 55)
+      .apply([{ kind: "remove", ref: gone, previousIndex: 1 }]);
+    expect(base.hasRetainedState).toBe(true);
+
+    const cleared = base.clearEstimates();
+    expect(cleared.hasRetainedState).toBe(true);
+    // The removed row's measurement still restores through a membership
+    // change after the clear.
+    const source: RowHeightReplacementSource<Key> = {
+      rowCount: 2,
+      entryAt: (index) => [entry(a), entry(gone)][index]!,
+    };
+    const widened = cleared.refilter(source);
+    expect(widened.getHeight(1)).toBe(55);
+  });
+
+  test("keeps the dense lane dense", () => {
+    const denseEntry = (
+      key: Key,
+      denseKey: number,
+      estimatedHeight?: number,
+    ): RowHeightEntry<Key> => ({ key, estimatedHeight, denseKey });
+    const rows = [denseEntry(data("a"), 0, 20), denseEntry(data("b"), 3, 40)];
+    const builder = createIndex([]).beginReplacement({
+      rowCount: rows.length,
+      denseCapacity: 8,
+      entryAt: (index) => rows[index]!,
+    });
+    while (!builder.done) builder.advance({ maxUnits: 256 });
+    const dense = builder.finish();
+
+    const cleared = dense.clearEstimates();
+    expect(cleared.getHeight(0)).toBe(30);
+    expect(cleared.getHeight(1)).toBe(30);
+    // Still a DENSE generation: an op without a denseKey is refused.
+    expect(() =>
+      cleared.apply([{ kind: "update", ref: data("a"), index: 0 }]),
+    ).toThrow(/dense/i);
+    // And the slot bitset still answers: a duplicate slot insert is refused.
+    expect(() =>
+      cleared.apply([
+        {
+          kind: "insert",
+          ref: data("x"),
+          index: 2,
+          estimatedHeight: 10,
+          denseKey: 3,
+        },
+      ]),
+    ).toThrow(/duplicate/i);
+  });
+});

packages/layout-core/src/row-height-index.ts

@@ -1613,6 +1613,59 @@ class PersistentRowHeightIndex<TKey> implements RowHeightIndex<TKey> {
     return builder.finish();
   }
 
+  /**
+   * Synchronous BY DESIGN, like `reorder`/`refilter`: the row set stands
+   * still, only the estimator's inputs changed, so one in-order pass over
+   * the existing entries plus a balanced rebuild answers it — no source, no
+   * identity re-hash, no HAMT work. Measurements, tombstones, retention
+   * order, and the lane (dense bitset included) are carried through
+   * verbatim; only entries with an `estimatedHeight` are rewritten. See the
+   * interface docblock for the semantics.
+   */
+  clearEstimates(): RowHeightIndex<TKey> {
+    if (this.#root === null) return this;
+    const work = createWork();
+    const values: HeightValue<TKey>[] = [];
+    let changed = false;
+    const stack: SequenceNode<TKey>[] = [];
+    let cursor: SequenceNode<TKey> | null = this.#root;
+    while (cursor !== null || stack.length > 0) {
+      while (cursor !== null) {
+        stack.push(cursor);
+        cursor = cursor.left;
+      }
+      const node = stack.pop()!;
+      const value = node.value;
+      work.previousEntriesScanned += 1;
+      if (value.estimatedHeight === undefined) {
+        values.push(value);
+      } else {
+        changed = true;
+        values.push({
+          ...value,
+          estimatedHeight: undefined,
+          // A measurement is a fact the DOM reported and survives; an
+          // estimate is arithmetic over the inputs that just changed.
+          height: value.measured ? value.height : this.#defaultHeight,
+        });
+      }
+      cursor = node.right;
+    }
+    if (!changed) return this;
+    const root = buildBalancedSequence(values, 0, values.length, work);
+    return this.#next(
+      root,
+      this.#visibleKeys,
+      this.#denseCapacity,
+      this.#visibleSlots,
+      this.#measurements,
+      this.#tombstones,
+      this.#tombstoneOrder,
+      this.#nextTicket,
+      work,
+    );
+  }
+
   /**
    * Synchronous BY DESIGN: a sort-only commit reorders EXISTING rows whose
    * heights are already known, so only the ordered structure and its prefix

packages/layout-core/src/types.ts

@@ -244,6 +244,21 @@ export interface RowHeightIndex<TKey> extends RowMetricsReader {
    * rowCount); callers fall back to `beginReplacement` on any throw.
    */
   refilter(source: RowHeightReplacementSource<TKey>): RowHeightIndex<TKey>;
+  /**
+   * Drops every entry's ESTIMATE, synchronously, leaving everything else —
+   * order, membership, lane, measurements, tombstones, retention order —
+   * untouched: an estimated, unmeasured row returns to the default height
+   * (and to an empty `estimatedHeight` slot, so a later re-estimate of the
+   * same value is not mistaken for a no-op), while a measured row keeps the
+   * height the DOM reported. Identity (`=== this`) when nothing is
+   * estimated.
+   *
+   * This is the "estimator inputs changed, rows did not" reset: estimates
+   * are arithmetic over inputs (columns, fonts) that can change while the
+   * row set stands still, and re-deriving them must not cost a full
+   * re-ingest of an index whose entries are already correct.
+   */
+  clearEstimates(): RowHeightIndex<TKey>;
   beginReplacement(
     source: RowHeightReplacementSource<TKey>,
   ): RowHeightReplacementBuilder<TKey>;

packages/react/src/__tests__/grouping-apply-layout-cost.test.tsx

@@ -0,0 +1,154 @@
+// @vitest-environment jsdom
+import "@testing-library/jest-dom/vitest";
+import { act, cleanup, render } from "@testing-library/react";
+import * as React from "react";
+import { afterEach, describe, expect, it, vi } from "vitest";
+
+import type { RowLayoutController } from "@pretable-internal/renderer-dom";
+// The diagnostics seam is a direct-module export deliberately kept off
+// renderer-dom's barrel; `vitest.config.ts` aliases this subpath to its
+// source and `tsconfig.typecheck.json` resolves it through the package's
+// built `dist/*.d.ts`.
+import { getRowLayoutControllerDiagnosticsForTesting } from "@pretable-internal/renderer-dom/row-layout-controller";
+
+/**
+ * The grouping-apply layout bill, pinned END TO END: applying a grouping to a
+ * mounted react grid must cost exactly ONE height-index replacement — the
+ * engine's reset commit, which really does change the row set (group rows
+ * enter). It must NOT also pay full-set replacements for the column commits
+ * that ride the same gesture (the group column swapping into the roster, and
+ * the merged engine widths landing a render later) — those change only the
+ * estimator's inputs, and the controller absorbs them in place.
+ *
+ * Before the columns-reset path existed this gesture cost THREE full 50k
+ * ingests in a headed browser (engine reset + two `setColumns` restarts,
+ * each cancelling the previous build) — ~0.6s of the grouping-apply settle
+ * at S2 target scale.
+ */
+
+type Row = { id: number; sector: string; name: string; qty: number };
+
+type Controller = RowLayoutController<Row, number, unknown>;
+
+const controllers: Controller[] = [];
+
+vi.mock("@pretable-internal/renderer-dom", async (importOriginal) => {
+  const actual =
+    await importOriginal<typeof import("@pretable-internal/renderer-dom")>();
+  const createRowLayoutController: typeof actual.createRowLayoutController = (
+    options,
+  ) => {
+    const controller = actual.createRowLayoutController(options);
+    controllers.push(controller as unknown as Controller);
+    return controller;
+  };
+  return { ...actual, createRowLayoutController };
+});
+
+const { PretableSurface } = await import("../pretable-surface");
+
+afterEach(() => {
+  cleanup();
+  controllers.length = 0;
+  vi.clearAllMocks();
+});
+
+const SECTORS = ["Tech", "Energy", "Health", "Retail", "Bank"];
+const rows: Row[] = Array.from({ length: 500 }, (_, index) => ({
+  id: index,
+  sector: SECTORS[index % SECTORS.length]!,
+  name: `row ${index}`,
+  qty: index % 97,
+}));
+
+const columns = [
+  { id: "sector", header: "Sector", widthPx: 100, type: "text" as const },
+  { id: "name", header: "Name", widthPx: 140, type: "text" as const },
+  {
+    id: "qty",
+    header: "Qty",
+    widthPx: 100,
+    type: "number" as const,
+    aggregate: "sum" as const,
+  },
+];
+
+describe("grouping-apply layout cost", () => {
+  it("costs exactly one height-index replacement, with the column commits absorbed in place", async () => {
+    let grid: { setQuery: (query: unknown) => unknown } | undefined;
+    const view = render(
+      <PretableSurface
+        ariaLabel="grouping-cost-grid"
+        columns={columns}
+        getRowId={(row: Row) => row.id}
+        initialExpansion={{ kind: "expanded" }}
+        onGridReady={(readyGrid) => {
+          grid = readyGrid as unknown as typeof grid;
+        }}
+        overscan={0}
+        rows={rows}
+        viewportHeight={600}
+      />,
+    );
+    await expect
+      .poll(() => view.container.querySelectorAll("[data-pretable-row]").length)
+      .toBeGreaterThan(0);
+    expect(controllers).toHaveLength(1);
+    const controller = controllers[0]!;
+    await expect.poll(() => controller.getState().status.kind).toBe("ready");
+
+    const base = getRowLayoutControllerDiagnosticsForTesting(controller);
+
+    await act(async () => {
+      const transition = grid!.setQuery({
+        filters: [],
+        sort: [],
+        rowGroups: [{ columnId: "sector" }],
+      }) as { finished?: Promise<unknown> };
+      await transition?.finished;
+    });
+    await expect
+      .poll(
+        () =>
+          view.container.querySelectorAll("[data-pretable-group-row]").length,
+        { timeout: 20_000 },
+      )
+      .toBeGreaterThan(0);
+    await expect
+      .poll(() => controller.getState().status.kind, { timeout: 20_000 })
+      .toBe("ready");
+    // Let the roster/width column effects settle (they land a render after
+    // the grouped snapshot publishes).
+    await act(async () => {
+      await new Promise((resolve) => setTimeout(resolve, 50));
+    });
+
+    const after = getRowLayoutControllerDiagnosticsForTesting(controller);
+    // ONE replacement: the engine's grouping reset. The group-column roster
+    // commit and the merged-width commit that follow it are absorbed by the
+    // columns-reset path (or by an in-flight replacement) — never by another
+    // full-set ingest.
+    expect(after.replacementStartCount - base.replacementStartCount).toBe(1);
+    expect(
+      after.columnsResetPathCount - base.columnsResetPathCount,
+    ).toBeGreaterThanOrEqual(1);
+    expect(after.columnsResetFallbackCount).toBe(0);
+    // And the grid really is grouped — the cheap path must not have bought
+    // its count by dropping the commit.
+    expect(controller.getState().snapshot?.visibleRowCount).toBe(
+      rows.length + SECTORS.length,
+    );
+    // Blank-grid insurance (this repo has shipped a windowed grid that
+    // painted offscreen while every count agreed): a known DATA row's cell
+    // is actually in the DOM with its content. Groups sort ascending, so
+    // "Bank" leads and its first member (id 4, name "row 4") sits inside the
+    // 600px viewport.
+    const dataRows = view.container.querySelectorAll("[data-pretable-row]");
+    expect(dataRows.length).toBeGreaterThan(0);
+    expect(
+      Array.from(dataRows).some((row) =>
+        (row.textContent ?? "").includes("row 4"),
+      ),
+    ).toBe(true);
+  }, 60_000);
+});