Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 30 additions & 0 deletions .changeset/data-scope-honesty-inputs.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
---
"@pretable/react": patch
---

Data-honesty checks now read every input from one commit, and the engine-sort
rule finally runs.

**A narrowing query no longer accuses you of a broken total.** `rows` and
`resultMeta.total` arrive together, but the row model ingests rows in a layout
effect — after the render that already read the new total. The contiguous-window
check therefore compared a new total against the previous query's row count:
filter 480 rows down to 120 and it reported that 120 records "cannot be a
contiguous window", then settled at the right `aria-rowcount` a render later.
Because these warnings fire once per page load, that spurious first one
permanently disarmed the check for the rest of the session — the real defect. In
rows mode the loaded count now comes from the `rows` the consumer just handed
over, and the "no total supplied" fallback counts the same records; explicit-model
mode still reads the model, which has no such skew and whose `rows` prop is an
empty array rather than an absent one.

**`processing: { filter: "external", sort: "engine" }` over a partial window now
warns.** The rule was written, unit-tested, and never called from a render.
Sorting a server-selected window locally presents the wrong sample under a
truthful-looking `aria-sort`, and it fires only where that is provable: an exact
`resultMeta.total` counting more records than the grid holds. Wiring it depended
on the fix above — the same one-render skew made an ordinary widening query look
like a partial window.

Settled behaviour is unchanged: the same counts, the same scope answers, and the
same warning for a `resultMeta.total` that really is inconsistent with the rows.
2 changes: 2 additions & 0 deletions apps/website/content/docs/server-data/query-ownership.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@ It is a claim about **authority**, and what a claim changes is what the grid is

Splitting the two slices is legal and sometimes right — a grid that loads the entire matching result can let the engine order it — but mixed authority over a **partial** window is the one combination to avoid. Sorting a server-selected window locally reorders a sample, not the population, and the header still reports an ordinary `aria-sort` over it. If the server chose the records, let it choose the order too.

The grid says so when it can prove it: external `filter`, engine `sort`, and an exact `resultMeta.total` counting more records than are loaded is the one case where "partial" is not a guess, and it warns once per page load. Silence is not a clearance — without an exact total there is nothing to measure the window against, and the combination is no safer for being unprovable.

### What it does not do

It does not turn the engine off. Nothing hands `processing` to the row model — the value is read while rendering and never travels further, and the word does not appear in `@pretable/core` at all. The engine goes on applying the published query to whatever rows you passed, whatever the authority says.
Expand Down
19 changes: 2 additions & 17 deletions apps/website/content/docs/server-data/totals.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -38,24 +38,9 @@ The asymmetry that makes this worth stating is that the three kinds do not fail

**A false `exact` is a different thing, because `exact` unlocks.** It is the one kind that makes the grid say something it otherwise refuses to say, so a wrong one propagates instead of sitting there. An exact count **larger** than the truth is published verbatim, so `aria-rowcount` tells a screen-reader user there are ten thousand rows in a table that stops at 480, and every row position is announced against that figure. An exact count **smaller** than the records already loaded fails in the other direction: `resolveDataScope` reads it as "you hold every matching record already" and answers `"all"`, which is how a window onto a larger result comes to be exported under a heading saying it is the whole thing.

The grid catches exactly one case of that, and only because the case contradicts itself: an exact total claiming fewer records than the grid has loaded cannot describe those rows at all. The count is refused for `aria-rowcount` — the loaded-model count goes out instead — and a warning says so once. That is a floor, not a verification. A total wrong in any way the loaded rows do not contradict is published as given, because there is nothing there to contradict it with.
The grid catches exactly one case of that, and only because the case contradicts itself: an exact total claiming fewer records than the grid has loaded cannot describe those rows at all. The count is refused for `aria-rowcount` — the loaded-model count goes out instead — and a warning says so once, in production builds too. That is a floor, not a verification. A total wrong in any way the loaded rows do not contradict is published as given, because there is nothing there to contradict it with.

### One warning you may see, and cannot prevent

That same check has a false positive you should know about before it appears in your console, because it fires on correct code:

```text
[pretable] resultMeta.total claims fewer matching records than the loaded
window's end (start + loaded count), so the loaded records cannot be a
contiguous window of the result set at the claimed offset (see
PretableResultMeta). Reporting the loaded-model count instead.
```

It shows up when a query **narrows** — 480 rows and an exact 480 replaced by 120 rows and an exact 120 — and the reason is a one-render skew inside the grid rather than anything in your data. Your `rows` and your `total` arrive on the same commit, but the row model ingests rows in a layout effect, which runs _after_ the render that already read the new total. For that one render the check compares the new count against the old row count, sees 120 against 480, and says what it would say if you really had over-committed.

It is transient and self-correcting: the next render has both numbers, `aria-rowcount` settles at the right value — filtering the overview grid to one region announces 121 a beat later, over 120 rows — and the message is printed once per page load however many times the condition recurs. What it is not is avoidable. Committing rows and their total together is what the props ask for, and the skew is on the grid's side of that commit; splitting them so the total lands a render after its rows only moves the problem, because for that render the grid holds the new rows and the old population's count, and announces the second over the first. Nor is the warning stripped from production builds.

So read it as noise on a narrowing query. Everywhere else, take it seriously: it means what it says, and what it says is that the total and the rows cannot both be right.
Both halves of that comparison are read from the same commit, so committing rows and their total together — which is what the props ask for — never trips it. Filtering the overview grid to one region replaces 480 rows and an exact 480 with 120 and an exact 120, and announces 121 with nothing in the console. If you do see the warning, it means what it says: the total and the rows cannot both be right.

## Exporting under external authority

Expand Down
38 changes: 37 additions & 1 deletion apps/website/e2e/server-data.spec.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { expect, test, type Page, type Request } from "@playwright/test";

import { waitForGridReady } from "./helpers";
import { openFilterMenu, waitForGridReady } from "./helpers";

/**
* The four claims the /docs/server-data section makes, checked against the
Expand Down Expand Up @@ -227,6 +227,42 @@ test("the total's confidence changes the report, the export scope and the announ
expect(new Set(seen).size).toBe(3);
});

test("a narrowing query settles the count without accusing the total", async ({
page,
}) => {
// Attached before the first navigation: the honesty rules run during the
// very first render, and they warn ONCE per page load — a warning printed
// then would latch and silence the real check for the rest of the session,
// which is why this reads the console rather than only the attribute.
const warnings: string[] = [];
page.on("console", (message) => {
if (message.text().includes("[pretable]")) warnings.push(message.text());
});

await openExample(page, OVERVIEW);
const grid = page.getByRole("grid");
await expect(grid).toHaveAttribute("aria-rowcount", "481");

const dialog = await openFilterMenu(page, "Region");
await dialog.getByRole("checkbox", { name: "North", exact: true }).click();
await page.keyboard.press("Escape");

// 120 matching orders plus the header row, published from the server's
// exact total — the settled value, which was never in doubt.
await expect(grid).toHaveAttribute("aria-rowcount", "121", {
timeout: 20_000,
});
await expect(page.locator(PHASE)).toHaveAttribute(
"data-pretable-data-phase",
"idle",
{ timeout: 20_000 },
);

// The claim the attribute cannot make: the count arrived without the grid
// first announcing that these rows and this total contradict each other.
expect(warnings).toEqual([]);
});

test("notify-only reports a query change without owning the query", async ({
page,
}) => {
Expand Down
5 changes: 1 addition & 4 deletions apps/website/lib/docs/__tests__/docs-api-surface.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2176,10 +2176,7 @@ function fixtureBoundPages(): Set<string> {
* ways: an entry for a fence that is now transcribed, or for one that no longer
* exists, fails.
*/
const UNTRANSCRIBED_FENCES: Record<string, string> = {
"server-data/totals.mdx#One warning you may see, and cannot prevent":
"A `text` fence quoting the console warning verbatim, not code. Its wording is pinned where the warning is emitted, in packages/react.",
};
const UNTRANSCRIBED_FENCES: Record<string, string> = {};

/**
* Bindings a fixture renames, per fence: fence identifier → fixture identifier.
Expand Down
215 changes: 213 additions & 2 deletions packages/react/src/__tests__/server-authority-aria.test.tsx
Original file line number Diff line number Diff line change
@@ -1,16 +1,42 @@
import "@testing-library/jest-dom/vitest";
import { cleanup, render, screen } from "@testing-library/react";
import { act, cleanup, render, screen } from "@testing-library/react";
import * as React from "react";
import { afterEach, describe, expect, it } from "vitest";
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";

import { createColumnHelper, createLocalRowModel } from "@pretable/core";

import { resetDevWarnings } from "../dev-warn";
import { PretableSurface } from "../pretable-surface";
import type { SerializeCsvArgs } from "../csv";
import type { PretableSurfaceGrid } from "../pretable-surface";
import type {
PretableMatchingTotal,
PretableProcessingOptions,
} from "@pretable/core";

afterEach(cleanup);

// The honesty rules warn once per process, so a latch set by one test would
// silence the next one asserting the same message. Spied rather than left to
// print, because several tests here render configurations that warn on purpose.
let warn: ReturnType<typeof vi.spyOn>;

beforeEach(() => {
resetDevWarnings();
warn = vi.spyOn(console, "warn").mockImplementation(() => {});
});

afterEach(() => {
warn.mockRestore();
});

/** Every `console.warn` message this test's render produced, joined. */
function warnings(): string {
return warn.mock.calls
.map((call: readonly unknown[]) => String(call[0]))
.join("\n");
}

type Row = { id: string; name: string; team: string };

const rows: Row[] = [
Expand Down Expand Up @@ -177,3 +203,188 @@ describe("aria-rowcount honesty rules", () => {
);
});
});

const THIRD_ROW: Row = { id: "c", name: "Cara", team: "z" };
const ALL_ROWS: Row[] = [...rows, THIRD_ROW];

/**
* `rows` and `resultMeta.total` arrive on the same commit, but the row model
* only ingests rows in a layout effect — after the render that already read the
* new total. Every honesty input has to be read from the SAME commit, or the
* checks compare a new total against the previous query's row count and report
* a contradiction the consumer never committed.
*/
describe("honesty inputs come from one commit", () => {
it("stays silent when a narrowing query commits rows and total together", () => {
const view = render(
<PretableSurface<Row>
ariaLabel="People"
columns={columns}
rows={ALL_ROWS}
getRowId={(row) => row.id}
viewportHeight={400}
processing={EXTERNAL}
resultMeta={{ total: { kind: "exact", count: 3 } }}
/>,
);
expect(screen.getByRole("grid")).toHaveAttribute("aria-rowcount", "4");

view.rerender(
<PretableSurface<Row>
ariaLabel="People"
columns={columns}
rows={[ALL_ROWS[0]!]}
getRowId={(row) => row.id}
viewportHeight={400}
processing={EXTERNAL}
resultMeta={{ total: { kind: "exact", count: 1 } }}
/>,
);

expect(warnings()).not.toContain("fewer matching records");
expect(screen.getByRole("grid")).toHaveAttribute("aria-rowcount", "2");
});

it("stays silent when a widening query commits rows under the fallback total", () => {
// No `resultMeta.total`: the surface falls back to "the population is
// whatever you handed me", which has to be the SAME count the contiguity
// check measures the window against.
const view = render(
<PretableSurface<Row>
ariaLabel="People"
columns={columns}
rows={[ALL_ROWS[0]!]}
getRowId={(row) => row.id}
viewportHeight={400}
processing={EXTERNAL}
/>,
);

view.rerender(
<PretableSurface<Row>
ariaLabel="People"
columns={columns}
rows={ALL_ROWS}
getRowId={(row) => row.id}
viewportHeight={400}
processing={EXTERNAL}
/>,
);

expect(warnings()).not.toContain("fewer matching records");
expect(screen.getByRole("grid")).toHaveAttribute("aria-rowcount", "4");
});

it('keeps an explicit model\'s scope "all" when its total covers the model', () => {
// The `rows` prop is `[]` — not `undefined` — in explicit-model mode, so a
// loaded count read off it would report zero records and answer "loaded"
// for a grid that demonstrably holds everything.
const helper = createColumnHelper<Row>();
const model = createLocalRowModel({
rows: ALL_ROWS,
columns: [
helper.accessor("name", { type: "text" }),
helper.accessor("team", { type: "text" }),
] as const,
getRowId: (row: Row) => row.id,
});
const seen: SerializeCsvArgs<Row, string, never>[] = [];
let grid: PretableSurfaceGrid<Row, string, never> | null = null;

render(
<PretableSurface
ariaLabel="People"
model={model as never}
viewportHeight={400}
processing={EXTERNAL}
resultMeta={{ total: { kind: "exact", count: 3 } }}
onExport={(args) => {
seen.push(args as never);
return null;
}}
saveFile={() => undefined}
onGridReady={(ready) => {
grid = ready as never;
}}
/>,
);

act(() => {
(grid as unknown as PretableSurfaceGrid<Row, string, never>).exportCsv();
});

expect(seen[0]?.scope).toBe("all");
model.dispose();
});
});

/**
* Engine sort over a window the server chose reorders a SAMPLE and labels it
* with an ordinary `aria-sort`. The rule has to fire from a real render — it
* sat fully unit-tested and entirely unwired for months precisely because
* nothing rendered it.
*/
describe("engine sort over a partial window", () => {
it("warns from a real render when the window is provably partial", () => {
render(
<PretableSurface<Row>
ariaLabel="People"
columns={columns}
rows={ALL_ROWS}
getRowId={(row) => row.id}
viewportHeight={400}
processing={{ filter: "external", sort: "engine" }}
resultMeta={{ total: { kind: "exact", count: 100 } }}
/>,
);

expect(warnings()).toContain('sort authority is "engine"');
});

it("stays silent when the loaded rows are the whole population", () => {
render(
<PretableSurface<Row>
ariaLabel="People"
columns={columns}
rows={ALL_ROWS}
getRowId={(row) => row.id}
viewportHeight={400}
processing={{ filter: "external", sort: "engine" }}
resultMeta={{ total: { kind: "exact", count: 3 } }}
/>,
);

expect(warnings()).not.toContain('sort authority is "engine"');
});

it("stays silent when a widening query commits rows and total together", () => {
// The shape of the lifecycle docs example: external filter, engine sort,
// and an exact total that always equals the delivered row count. A search
// that widens must not make the loaded window look partial for one render.
const view = render(
<PretableSurface<Row>
ariaLabel="People"
columns={columns}
rows={[ALL_ROWS[0]!]}
getRowId={(row) => row.id}
viewportHeight={400}
processing={{ filter: "external" }}
resultMeta={{ total: { kind: "exact", count: 1 } }}
/>,
);

view.rerender(
<PretableSurface<Row>
ariaLabel="People"
columns={columns}
rows={ALL_ROWS}
getRowId={(row) => row.id}
viewportHeight={400}
processing={{ filter: "external" }}
resultMeta={{ total: { kind: "exact", count: 3 } }}
/>,
);

expect(warnings()).not.toContain('sort authority is "engine"');
});
});
Loading