diff --git a/.changeset/eviction-population-and-gate.md b/.changeset/eviction-population-and-gate.md new file mode 100644 index 00000000..a12ea5de --- /dev/null +++ b/.changeset/eviction-population-and-gate.md @@ -0,0 +1,55 @@ +--- +"@pretable/core": minor +"@pretable/react": minor +--- + +Eviction: two publicly-reachable correctness fixes, both of which contradicted +what the docs promised. + +**An evicted selection no longer paints rows the reader never selected.** +`datasetKey` identifies the QUERY, not the population — deliberately, and +[the docs](/docs/server-data/lifecycle#datasetkey) tell consumers to keep it +stable while they page within one result. So an insert or a delete made by +someone else, upstream of a selection whose own rows are unloaded, arrived with +the key unchanged and silently re-filled the remembered dataset positions with +different rows. Reproduced through `` with the honesty gate +fully passing: a `row-1..row-8` selection, both endpoints evicted, five rows +prepended to the same result, and the returning window painted five rows +selected — four of which had not existed when the user selected — while the +eight they did choose painted nothing. + +A span now records the population's SIZE alongside its key +(`PretableIndexedDatasetRowSpan.datasetTotal`, from the exact +`resultMeta.total.count` the gate already requires), and a mismatch fails closed +exactly as a key mismatch does: nothing paints from the span, and +`getCellSelectionSummary()` reports `verified: false` until a window covering +both endpoints re-measures it. A proven deletion is the one allowance — a total +short by exactly the rows the engine watched vanish is accounted for, so +endpoint narrowing still works. What this does **not** catch is a change that +leaves the size identical; `eviction.mdx` now says so rather than promising +otherwise. + +`ɵPretableIndexedSelectionWindow.datasetTotal` is required, not optional: the +gate that builds a window cannot pass without an exact total, so an optional +field would only be a way to fail open by omission. + +**One closed-gate revision no longer destroys the selection and the cursor.** +With `resultMeta.total` reporting `{kind: "estimate"}` for a single render — an +in-flight count query, a backend that stops counting past 10k — or one revision +of `processing.sort: "engine"`, a window slide dropped every range and emptied +the cursor irrecoverably. Restoring the exact total brought neither back. +Uncontrolled consumers only; a controlled one was accidentally immune because +the `state.selection` echo re-supplied what the engine had discarded. + +A null window was two different situations sharing one representation. The +engine is now told which: `windowed` says whether the consumer publishes +`resultMeta.window` at all, independent of any gate, so a windowed grid with no +window this revision means "I cannot verify", not "those rows were deleted" — +and it holds the selection and the cursor byte-for-byte instead of asserting a +deletion it could not have observed. Local mode, where an absent row genuinely +is a deleted row, is unchanged in every branch and pinned by tests that run the +same fixture both ways. + +`CreateGridUiCoreOptions.getSelectionWindow` is replaced by `getWindowing`, +which returns both facts from one read so they cannot describe different +instants. diff --git a/apps/bench/tests/eviction.spec.ts b/apps/bench/tests/eviction.spec.ts index 187f015c..88836943 100644 --- a/apps/bench/tests/eviction.spec.ts +++ b/apps/bench/tests/eviction.spec.ts @@ -27,6 +27,8 @@ import { expect, test, type Page } from "@playwright/test"; const ROW_HEIGHT = 48; const PAGE_SIZE = 50; const WINDOW_START = 5_000; +/** `TOTAL_ROWS` in `windowed-harness.tsx`, which this drives. */ +const HARNESS_TOTAL_ROWS = 10_000; /** The selection under test: 11 rows, at dataset positions 5,010–5,020. */ const SELECT_FROM = 5_010; @@ -598,6 +600,10 @@ test.describe("a cell selection survives its rows being evicted", () => { start: SELECT_FROM, end: SELECT_TO, datasetKey: "windowed-harness", + // The population the positions were measured in. `datasetKey` says + // which QUERY; this says how big its result was, which is what + // catches somebody else inserting rows above an evicted selection. + datasetTotal: HARNESS_TOTAL_ROWS, }); // 1a. The evicted endpoint really is gone from the DOM — otherwise diff --git a/apps/website/content/docs/server-data/eviction.mdx b/apps/website/content/docs/server-data/eviction.mdx index 638de0b9..6ec50a1e 100644 --- a/apps/website/content/docs/server-data/eviction.mdx +++ b/apps/website/content/docs/server-data/eviction.mdx @@ -32,6 +32,8 @@ Two things it deliberately does not do. It will not name the selected records wi **The cursor survives too.** While the focused row is unloaded its cell element is gone, so focus parks on the grid's scroll viewport — never on ``, which would drop the reader out of the grid entirely and take the keyboard with them. When the row returns the cursor is back on the same cell, exactly one cell holds it, and an arrow key moves on from the row it was left on rather than from wherever the viewport happens to be parked. +**A revision the grid cannot vouch for holds, rather than discards.** The honesty gate below can shut for a render without anything being wrong: a count query still in flight reports `total.kind: "estimate"`, a backend that stops counting past ten thousand never reports anything else, and a single revision of engine-side sort takes positional authority away. On such a render the grid does not know where the loaded rows sit, and it treats that as not knowing rather than as evidence — the selection and the cursor are held exactly as they were, and the next render that can place them carries on. A grid with no `resultMeta.window` at all is a different case and behaves as it always has: there the whole result arrives on every render, so a row that is absent has genuinely been removed, and the selection prunes. + ## What it costs you One thing, and it is not optional: **`resultMeta.datasetKey`**. @@ -40,6 +42,8 @@ Everything above turns on dataset positions, and a position only means something Because that loss is invisible and the type says nothing about it, a grid that publishes a trusted `window` and no `datasetKey` says so — one `console.warn`, in production builds too, naming what it refused. [The `datasetKey` section on Loading, staleness, errors](/docs/server-data/lifecycle#datasetkey) is where the rule for choosing one lives: change it when the result set changes, keep it stable while you page within one result, and derive it from something that commits rather than from something that ticks. +The exact total the honesty gate already requires does the other half of the job. A key that is correctly held stable across pages cannot report an insert or a delete made by someone else, and the count is what does: a span measured when the population held 20 rows is refused once it holds 25, because the positions it remembers now name different rows. That costs you nothing extra — [Totals and honesty](/docs/server-data/totals) already requires the exact count for the window to be trusted at all — but it is worth knowing that the number is load-bearing for selection and not only for the scrollbar. + ## What it does not do **It does not decide memory pressure for you.** There is no block limit, no cache ceiling, and no policy you configure — the grid never releases a row on its own and never asks you to. [`telemetry.windowGap`](/docs/server-data/windowing#knowing-when-to-fetch) prompts the fetch half of the decision and says nothing at all about the release half; how many blocks are worth holding is a judgement about your data and your users' machines, and it stays with you. @@ -48,6 +52,8 @@ Because that loss is invisible and the type says nothing about it, a grid that p **It does not survive a query change, and must not.** A new `datasetKey` is a statement that the positions now hold different rows, so the spans measured under the old key are refused rather than repainted onto whatever occupies those positions today. A range whose endpoints are both present in the new population is re-measured there and kept — it is fully locatable, so there is nothing to guess — and a range that is only half locatable collapses onto the endpoint that survived, with its span dropped so the smaller count that results is not presented as a proven one. Selection surviving a re-sort is out of scope by design, not by accident. +**It does not notice a change that leaves the population the same size.** Rows inserted or removed upstream of an evicted selection move every position after them while `datasetKey` stays correctly unchanged, so the grid compares the exact total as well: when the size differs from the size a span was measured at, the span is refused, nothing paints from it, and `getCellSelectionSummary()` reports `verified: false` until a window covering both endpoints re-measures it. A revision that removes one row and adds another leaves that count identical, and there the grid still answers from the remembered positions — which by then name the neighbours of the rows the reader chose. It reports `verified: false` throughout, which is the whole of what it can honestly say; distinguishing a re-shuffled population from a scrolled one would take a token that changes on every write, and only your backend can mint that. If your result set is written to while readers hold selections over it, treat `verified: false` as the signal it is and re-derive from a fresh read rather than from the span. + ## See also - [Windowing](/docs/server-data/windowing) — `resultMeta.window`, the `windowGap` signal that prompts the fetch, and the honesty gate all of this rides. diff --git a/apps/website/content/docs/server-data/lifecycle.mdx b/apps/website/content/docs/server-data/lifecycle.mdx index 81e04519..604d6407 100644 --- a/apps/website/content/docs/server-data/lifecycle.mdx +++ b/apps/website/content/docs/server-data/lifecycle.mdx @@ -92,7 +92,9 @@ Two things worth knowing about that callback. Returning `null` does not suppress Change it when the result set changes — a new filter, a different sort, a different search — so that interaction state cannot silently reattach itself to a different dataset. Keep it stable while you page within one result: a second page of the same query is more of the same population, not a new one. -The reason is that some interaction state is recorded against positions in the result rather than against row ids, and positions are only meaningful within one population. A re-sort re-fills position 40 with a different row; a filter change re-fills every position. The dataset key is the evidence the engine uses to tell "the reader scrolled" from "this is a different table now", and it fails closed: when the key a selection was measured under and the key the grid now reports disagree — or when there is no key at all — the engine refuses what it recorded rather than repainting it onto whatever rows occupy those positions today. What that costs is a selection that shrinks to the rows actually loaded, visibly. What it buys is never painting a row the reader did not select. +The reason is that some interaction state is recorded against positions in the result rather than against row ids, and positions are only meaningful within one population. A re-sort re-fills position 40 with a different row; a filter change re-fills every position. The dataset key is the evidence the engine uses to tell "the reader scrolled" from "this is a different table now", and it fails closed: when the key a selection was measured under and the key the grid now reports disagree — or when there is no key at all — the engine refuses what it recorded rather than repainting it onto whatever rows occupy those positions today. What that costs is a selection that shrinks to the rows actually loaded, visibly. + +The key answers "is this the same query?" — it does not answer "is this the same set of rows?", and keeping it stable while you page is exactly what stops it from trying. Rows inserted or removed by someone else, upstream of a selection whose own rows are unloaded, arrive with the key unchanged and shift every position after them. The engine compares `resultMeta.total.count` alongside the key for that, and refuses the recorded positions when the population has changed size — so the two together are what keep a returning window from painting rows the reader never selected. Neither can see a change that leaves the size alone; [Eviction](/docs/server-data/eviction#what-it-does-not-do) says what remains. Deriving it from the query is the usual answer, and the derivation only has to be stable: `JSON.stringify(query)` is a fine key. What it must not be is early or arbitrary. The example above sets its key when a search **commits**, not on each keystroke, because until the new rows arrive the ones on screen still answer the previous search — and a key recomputed per render from something like a timestamp says "different table" on every frame. diff --git a/packages/core/core.api.md b/packages/core/core.api.md index 8db7688d..c1666116 100644 --- a/packages/core/core.api.md +++ b/packages/core/core.api.md @@ -73,7 +73,7 @@ export interface CreateGridUiCoreOptions[]; // @internal - readonly getSelectionWindow?: () => ɵPretableIndexedSelectionWindow | null; + readonly getWindowing?: () => ɵPretableIndexedWindowing | null; // (undocumented) readonly rowModel: PretableRowModel; // (undocumented) @@ -848,6 +848,7 @@ export interface PretableIndexedCellSelectionSummary { // @public export interface PretableIndexedDatasetRowSpan { readonly datasetKey?: string; + readonly datasetTotal?: number; readonly end: number; readonly start: number; } @@ -1325,12 +1326,21 @@ export type RowOf = TModel extends { // @internal export interface ɵPretableIndexedSelectionWindow { readonly datasetKey?: string; + readonly datasetTotal: number; // (undocumented) readonly length: number; // (undocumented) readonly start: number; } +// Warning: (ae-internal-missing-underscore) The name "ɵPretableIndexedWindowing" should be prefixed with an underscore because the declaration is marked as @internal +// +// @internal +export interface ɵPretableIndexedWindowing { + // (undocumented) + readonly window: ɵPretableIndexedSelectionWindow | null; +} + // Warning: (ae-internal-missing-underscore) The name "ɵsetLocalRowModelFilterAuthority" should be prefixed with an underscore because the declaration is marked as @internal // // @internal diff --git a/packages/core/src/public_api.ts b/packages/core/src/public_api.ts index 96bb5465..aae90c9c 100644 --- a/packages/core/src/public_api.ts +++ b/packages/core/src/public_api.ts @@ -145,6 +145,9 @@ export type { // tells an evicted row from a deleted one, and `@pretable/react` is the only // caller that knows whether the honesty gate has passed. export type { PretableIndexedSelectionWindow as ɵPretableIndexedSelectionWindow } from "@pretable-internal/grid-core"; +// Named by `CreateGridUiCoreOptions.getWindowing`'s signature, so it ships +// alongside the window type it wraps. +export type { PretableIndexedWindowing as ɵPretableIndexedWindowing } from "@pretable-internal/grid-core"; // Re-declares who selected the loaded records on a model this package created. // Exported because `processing` is a render-time prop on `@pretable/react`, so // the authority a rows-mode model is built with can change while it is alive, diff --git a/packages/grid-core/src/__tests__/grid-ui-core.test.ts b/packages/grid-core/src/__tests__/grid-ui-core.test.ts index 74c2ed57..a4ea8c7f 100644 --- a/packages/grid-core/src/__tests__/grid-ui-core.test.ts +++ b/packages/grid-core/src/__tests__/grid-ui-core.test.ts @@ -719,7 +719,7 @@ describe("UI-only grid core", () => { /** * A grid whose consumer serves a moving WINDOW over `all`, exactly as the * windowed-data contract describes: `setRows` gets the loaded slice and - * `getSelectionWindow` reports where that slice sits in the dataset. + * `getWindowing` reports where that slice sits in the dataset. * * Every eviction test below drives gestures through the real store rather * than calling `reconcileIndexedSelection` with a hand-built fixture. That @@ -737,12 +737,15 @@ describe("UI-only grid core", () => { readonly start: number; readonly length: number; readonly datasetKey?: string; + readonly datasetTotal: number; } | null = null; const rowModel = createLocalRowModel({ rows: [], columns: modelColumns }); const grid = createGridUiCore({ rowModel, columns: visualColumns, - getSelectionWindow: () => selectionWindow, + // Windowed throughout: `windowing` is non-null even before the first + // slide, when the window itself is still unknown. + getWindowing: () => ({ window: selectionWindow }), }); // A published `datasetKey` by default: spans are fail-closed on it (see // `spanReadableInWindow`), so a windowed consumer that never sets one @@ -754,7 +757,10 @@ describe("UI-only grid core", () => { datasetKey = "population-1", ) => { rowModel.setRows(all.slice(start, start + length)); - selectionWindow = { start, length, datasetKey }; + // `total`, not `length`: the population is the whole of `all`, and the + // window is a slice of it. Publishing the slice's own size here would + // make every slide look like a population change. + selectionWindow = { start, length, datasetKey, datasetTotal: total }; grid.observeRowModelRevision(rowModel.getState().snapshot.revision); }; /** The shape every surface gesture builds: a fresh range, ids only. */ @@ -953,10 +959,16 @@ describe("UI-only grid core", () => { const grid = createGridUiCore({ rowModel, columns: visualColumns, - getSelectionWindow: () => ({ - start: 0, - length, - datasetKey: "population-1", + getWindowing: () => ({ + window: { + start: 0, + length, + datasetKey: "population-1", + // The whole dataset is resident here, so the population size and + // the window length are the same number -- and a deletion moves + // both, which is what makes the narrowing below provable. + datasetTotal: length, + }, }), }); grid.observeRowModelRevision(rowModel.getState().snapshot.revision); @@ -1023,6 +1035,7 @@ describe("UI-only grid core", () => { start: 10, end: 40, datasetKey: "population-1", + datasetTotal: 200, }, }, ], diff --git a/packages/grid-core/src/__tests__/indexed-focus.test.ts b/packages/grid-core/src/__tests__/indexed-focus.test.ts index 1c38bcec..6fa983fc 100644 --- a/packages/grid-core/src/__tests__/indexed-focus.test.ts +++ b/packages/grid-core/src/__tests__/indexed-focus.test.ts @@ -290,6 +290,16 @@ describe("indexed focus", () => { /** A stable population identity; see `spanReadableInWindow`. */ const DATASET_KEY = "population-1"; + /** + * The population these windows are slices of. Required on every window, and + * INERT for the cursor: focus carries no dataset span, so nothing here ever + * compares one. It is held constant across each test's two windows so that + * `datasetTotal` cannot be the thing making an assertion pass -- the only + * discriminator in this file is still the window's coverage of the absent + * row's old position. + */ + const DATASET_TOTAL = 10_000; + const EMPTY_FOCUS = { ref: null, columnId: null }; describe("eviction", () => { @@ -303,6 +313,7 @@ describe("indexed focus", () => { start: 2_000, length: 100, datasetKey: DATASET_KEY, + datasetTotal: DATASET_TOTAL, }; const focus = { ref: data("row-2010"), columnId: "score" as const }; expect(previousSnapshot.dataIndexOf(focus.ref)).toBe(10); @@ -315,6 +326,7 @@ describe("indexed focus", () => { start: 3_000, length: 30, datasetKey: DATASET_KEY, + datasetTotal: DATASET_TOTAL, }; expect(snapshot.indexOf(focus.ref)).toBe(-1); @@ -341,6 +353,7 @@ describe("indexed focus", () => { start: 2_000, length: 100, datasetKey: DATASET_KEY, + datasetTotal: DATASET_TOTAL, }; const focus = { ref: data("row-2010"), columnId: "score" as const }; @@ -353,6 +366,7 @@ describe("indexed focus", () => { start: 2_000, length: remaining.length, datasetKey: DATASET_KEY, + datasetTotal: DATASET_TOTAL, }; expect( @@ -383,6 +397,7 @@ describe("indexed focus", () => { start: 2_000, length: 10, datasetKey: DATASET_KEY, + datasetTotal: DATASET_TOTAL, }; const focus = { ref: data("row-2002"), columnId: "score" as const }; const west = previousSnapshot.parentGroupOf(focus.ref); @@ -397,7 +412,12 @@ describe("indexed focus", () => { // The window has not moved, so it still covers dataset position // 2,002: the row is absent from a span that is loaded, which is the // one thing eviction can never explain. - window: { start: 2_000, length: 10, datasetKey: DATASET_KEY }, + window: { + start: 2_000, + length: 10, + datasetKey: DATASET_KEY, + datasetTotal: DATASET_TOTAL, + }, previous: { snapshot: previousSnapshot, window: previousWindow }, }), ).toEqual({ @@ -435,7 +455,12 @@ describe("indexed focus", () => { const previousSnapshot = modelFor(datasetSlice(2_000, 2_100)); const previous = { snapshot: previousSnapshot, - window: { start: 2_000, length: 100, datasetKey: DATASET_KEY }, + window: { + start: 2_000, + length: 100, + datasetKey: DATASET_KEY, + datasetTotal: DATASET_TOTAL, + }, }; const snapshot = modelFor(datasetSlice(3_000, 3_030)); const focus = { ref: data("row-2010"), columnId: "score" as const }; @@ -444,7 +469,12 @@ describe("indexed focus", () => { snapshot, focus, eviction: { - window: { start: 3_000, length: 30, datasetKey: DATASET_KEY }, + window: { + start: 3_000, + length: 30, + datasetKey: DATASET_KEY, + datasetTotal: DATASET_TOTAL, + }, previous, }, }; @@ -452,6 +482,31 @@ describe("indexed focus", () => { const columnIds = ["team", "score"] as const; + test("a shut honesty gate holds the cursor, while local mode still re-seats", () => { + // The cursor half of the same rule the selection follows, from ONE + // fixture whose only difference is `windowed`. It has to be the same + // rule: a grid that keeps a selection under a cursor that jumped + // somewhere else is not a coherent grid. + const { snapshot, focus, eviction } = evictedCursor(); + const blind = { window: null, previous: eviction.previous }; + + // Windowed, window unknown this revision: absence proves nothing. + expect( + reconcileIndexedFocus(focus, snapshot, { ...blind, windowed: true }), + ).toBe(focus); + + // Local mode: absence IS deletion, and a flat model has no survivor to + // re-seat onto, so the cursor clears exactly as it did before eviction + // existed. + expect( + reconcileIndexedFocus(focus, snapshot, { ...blind, windowed: false }), + ).toEqual(EMPTY_FOCUS); + // ...and so does a caller that says nothing at all. + expect(reconcileIndexedFocus(focus, snapshot, blind)).toEqual( + EMPTY_FOCUS, + ); + }); + test("an arrow key from an evicted cursor holds the cursor instead of dropping it", () => { // `moveIndexedFocus` reconciled two-argument, so it could not tell an // evicted row from a deleted one: pressing an arrow WHILE the cursor's @@ -541,6 +596,7 @@ describe("indexed focus", () => { start: 2_000, length: 100, datasetKey: DATASET_KEY, + datasetTotal: DATASET_TOTAL, }; const focus = { ref: data("row-2010"), columnId: "score" as const }; const remaining = [ @@ -560,6 +616,7 @@ describe("indexed focus", () => { start: 2_000, length: remaining.length, datasetKey: DATASET_KEY, + datasetTotal: DATASET_TOTAL, }, previous: { snapshot: previousSnapshot, window: previousWindow }, }, @@ -608,10 +665,20 @@ describe("indexed focus", () => { focus, movement: "down", eviction: { - window: { start: 3_000, length: 0, datasetKey: DATASET_KEY }, + window: { + start: 3_000, + length: 0, + datasetKey: DATASET_KEY, + datasetTotal: DATASET_TOTAL, + }, previous: { snapshot: previousSnapshot, - window: { start: 2_000, length: 100, datasetKey: DATASET_KEY }, + window: { + start: 2_000, + length: 100, + datasetKey: DATASET_KEY, + datasetTotal: DATASET_TOTAL, + }, }, }, }), @@ -629,10 +696,20 @@ describe("indexed focus", () => { expect( reconcileIndexedFocus(focus, snapshot, { - window: { start: 3_000, length: 30, datasetKey: "sort=score" }, + window: { + start: 3_000, + length: 30, + datasetKey: "sort=score", + datasetTotal: DATASET_TOTAL, + }, previous: { snapshot: previousSnapshot, - window: { start: 2_000, length: 100, datasetKey: "sort=name" }, + window: { + start: 2_000, + length: 100, + datasetKey: "sort=name", + datasetTotal: DATASET_TOTAL, + }, }, }), ).toEqual(EMPTY_FOCUS); diff --git a/packages/grid-core/src/__tests__/indexed-selection.test.ts b/packages/grid-core/src/__tests__/indexed-selection.test.ts index 2a1ccacd..62a258b3 100644 --- a/packages/grid-core/src/__tests__/indexed-selection.test.ts +++ b/packages/grid-core/src/__tests__/indexed-selection.test.ts @@ -936,7 +936,7 @@ describe("indexed row selection", () => { const previousSnapshot = previousModel.getState().snapshot; // "x" and "y" sat at dataset positions 100 and 101 the last time they // were loaded. - const previousWindow = { start: 100, length: 2 }; + const previousWindow = { start: 100, length: 2, datasetTotal: 1_000 }; const selection = { rows: { kind: "explicit" as const, rowIds: new Set() }, @@ -961,7 +961,7 @@ describe("indexed row selection", () => { const snapshot = currentModel.getState().snapshot; // Dataset position 500: nowhere near where "x"/"y" used to sit, so // "outside the window" isn't a coincidence of small numbers. - const loadedWindow = { start: 500, length: 1 }; + const loadedWindow = { start: 500, length: 1, datasetTotal: 1_000 }; const reconciled = reconcileIndexedSelection(selection, snapshot, { window: loadedWindow, @@ -991,7 +991,7 @@ describe("indexed row selection", () => { }); const previousSnapshot = model.getState().snapshot; // Dataset positions 100 (x), 101 (y), 102 (z). - const previousWindow = { start: 100, length: 3 }; + const previousWindow = { start: 100, length: 3, datasetTotal: 1_000 }; const selection = { rows: { kind: "explicit" as const, rowIds: new Set() }, @@ -1009,7 +1009,8 @@ describe("indexed row selection", () => { // window's span. model.applyTransaction({ remove: ["y"] }); const snapshot = model.getState().snapshot; - const loadedWindow = { start: 100, length: 2 }; + // One row deleted, so the population is one shorter than it was. + const loadedWindow = { start: 100, length: 2, datasetTotal: 999 }; const reconciled = reconcileIndexedSelection(selection, snapshot, { window: loadedWindow, @@ -1043,7 +1044,7 @@ describe("indexed row selection", () => { }); const previousSnapshot = model.getState().snapshot; // Dataset positions 100 (x), 101 (y), 102 (z). - const previousWindow = { start: 100, length: 3 }; + const previousWindow = { start: 100, length: 3, datasetTotal: 1_000 }; const selection = { rows: { kind: "explicit" as const, rowIds: new Set() }, @@ -1067,7 +1068,7 @@ describe("indexed row selection", () => { // Covers only "x"'s old absolute position (100), not "z"'s (102): "x" is // provably deleted (window still covers where it was); "z" is merely // evicted (unprovable, same as the pure-eviction case). - const loadedWindow = { start: 100, length: 1 }; + const loadedWindow = { start: 100, length: 1, datasetTotal: 999 }; const reconciled = reconcileIndexedSelection(selection, snapshot, { window: loadedWindow, @@ -1133,6 +1134,7 @@ describe("indexed row selection", () => { start: 0, length: SPAN_LENGTH, datasetKey: DATASET_KEY, + datasetTotal: SPAN_LENGTH, }; const selection = cellRangeSelection("row-0", `row-${SPAN_LENGTH - 1}`); @@ -1154,6 +1156,9 @@ describe("indexed row selection", () => { start: keptStart, length: 30, datasetKey: DATASET_KEY, + // Nothing was added or removed -- rows were RELEASED. The population is + // the size it always was, which is what makes the span still readable. + datasetTotal: SPAN_LENGTH, }; expect(keptSnapshot.sourceRowCount).toBe(30); @@ -1184,6 +1189,7 @@ describe("indexed row selection", () => { start: 0, length: all.length, datasetKey: DATASET_KEY, + datasetTotal: all.length, }; const selection = cellRangeSelection("row-0", `row-${all.length - 1}`); @@ -1196,7 +1202,12 @@ describe("indexed row selection", () => { ).toBe(true); const keptSnapshot = modelFor(all.slice(2_000, 2_030)); - const keptWindow = { start: 2_000, length: 30, datasetKey: DATASET_KEY }; + const keptWindow = { + start: 2_000, + length: 30, + datasetKey: DATASET_KEY, + datasetTotal: 4_901, + }; const afterEviction = reconcileIndexedSelection(whileLoaded, keptSnapshot, { window: keptWindow, previous: { snapshot: loadedSnapshot, window: loadedWindow }, @@ -1214,6 +1225,7 @@ describe("indexed row selection", () => { start: 0, length: all.length, datasetKey: DATASET_KEY, + datasetTotal: all.length, }; // Two ranges, deliberately. A contiguous loaded window and a contiguous @@ -1243,7 +1255,12 @@ describe("indexed row selection", () => { }); const keptSnapshot = modelFor(all.slice(2_000, 2_030)); - const keptWindow = { start: 2_000, length: 30, datasetKey: DATASET_KEY }; + const keptWindow = { + start: 2_000, + length: 30, + datasetKey: DATASET_KEY, + datasetTotal: 4_901, + }; const afterEviction = reconcileIndexedSelection(whileLoaded, keptSnapshot, { window: keptWindow, previous: { snapshot: loadedSnapshot, window: loadedWindow }, @@ -1295,7 +1312,12 @@ describe("indexed row selection", () => { // at once (what the tests above do) never reaches this branch. const all = datasetRows(200); const firstSnapshot = modelFor(all.slice(0, 100)); - const firstWindow = { start: 0, length: 100, datasetKey: DATASET_KEY }; + const firstWindow = { + start: 0, + length: 100, + datasetKey: DATASET_KEY, + datasetTotal: 200, + }; const selection = cellRangeSelection("row-10", "row-90"); const stamped = reconcileIndexedSelection(selection, firstSnapshot, { @@ -1308,7 +1330,12 @@ describe("indexed row selection", () => { // Slide by 50. "row-10" (dataset position 10) falls out of [50, 150); // "row-90" is still loaded, at rank 40 of the new window. const slidSnapshot = modelFor(all.slice(50, 150)); - const slidWindow = { start: 50, length: 100, datasetKey: DATASET_KEY }; + const slidWindow = { + start: 50, + length: 100, + datasetKey: DATASET_KEY, + datasetTotal: 200, + }; const slid = reconcileIndexedSelection(stamped, slidSnapshot, { window: slidWindow, @@ -1339,7 +1366,12 @@ describe("indexed row selection", () => { // pruning one row out of an 81-row span leaves 80. const all = datasetRows(200); const firstSnapshot = modelFor(all.slice(0, 100)); - const firstWindow = { start: 0, length: 100, datasetKey: DATASET_KEY }; + const firstWindow = { + start: 0, + length: 100, + datasetKey: DATASET_KEY, + datasetTotal: 200, + }; const selection = cellRangeSelection("row-10", "row-90"); const stamped = reconcileIndexedSelection(selection, firstSnapshot, { @@ -1355,6 +1387,10 @@ describe("indexed row selection", () => { start: 0, length: afterDelete.length, datasetKey: DATASET_KEY, + // One row really was deleted, so the population is 199. The span was + // measured at 200 -- and it stays readable only because the deletion it + // is short by is one this call can PROVE. + datasetTotal: 199, }; const reconciled = reconcileIndexedSelection(stamped, deletedSnapshot, { @@ -1429,7 +1465,12 @@ describe("indexed row selection", () => { test("an evicted anchor keeps its identity instead of migrating to the range's start", () => { const all = datasetRows(200); const loadedSnapshot = modelFor(all.slice(100, 200)); - const loadedWindow = { start: 100, length: 100, datasetKey: DATASET_KEY }; + const loadedWindow = { + start: 100, + length: 100, + datasetKey: DATASET_KEY, + datasetTotal: 200, + }; const selection = upwardSelection(); const stamped = reconcileIndexedSelection(selection, loadedSnapshot, { @@ -1441,7 +1482,12 @@ describe("indexed row selection", () => { // are still in the dataset, just not loaded -- so the anchor is still a // real cell the next shift-click has to extend from. const keptSnapshot = modelFor(all.slice(20, 60)); - const keptWindow = { start: 20, length: 40, datasetKey: DATASET_KEY }; + const keptWindow = { + start: 20, + length: 40, + datasetKey: DATASET_KEY, + datasetTotal: 200, + }; const afterEviction = reconcileIndexedSelection(stamped, keptSnapshot, { window: keptWindow, previous: { snapshot: loadedSnapshot, window: loadedWindow }, @@ -1460,7 +1506,12 @@ describe("indexed row selection", () => { // two ranges the anchor is in neither endpoint of `ranges[0]`. const all = datasetRows(200); const loadedSnapshot = modelFor(all.slice(100, 200)); - const loadedWindow = { start: 100, length: 100, datasetKey: DATASET_KEY }; + const loadedWindow = { + start: 100, + length: 100, + datasetKey: DATASET_KEY, + datasetTotal: 200, + }; const selection = { rows: { kind: "explicit" as const, rowIds: new Set() }, ranges: [ @@ -1480,7 +1531,12 @@ describe("indexed row selection", () => { window: loadedWindow, }); const keptSnapshot = modelFor(all.slice(20, 60)); - const keptWindow = { start: 20, length: 40, datasetKey: DATASET_KEY }; + const keptWindow = { + start: 20, + length: 40, + datasetKey: DATASET_KEY, + datasetTotal: 200, + }; const afterEviction = reconcileIndexedSelection(stamped, keptSnapshot, { window: keptWindow, previous: { snapshot: loadedSnapshot, window: loadedWindow }, @@ -1497,7 +1553,12 @@ describe("indexed row selection", () => { // builds its range straight from that address. const all = datasetRows(200); const loadedSnapshot = modelFor(all.slice(100, 200)); - const loadedWindow = { start: 100, length: 100, datasetKey: DATASET_KEY }; + const loadedWindow = { + start: 100, + length: 100, + datasetKey: DATASET_KEY, + datasetTotal: 200, + }; const selection = upwardSelection(); const stamped = reconcileIndexedSelection(selection, loadedSnapshot, { @@ -1512,6 +1573,7 @@ describe("indexed row selection", () => { start: 100, length: afterDelete.length, datasetKey: DATASET_KEY, + datasetTotal: 199, }; const reconciled = reconcileIndexedSelection(stamped, deletedSnapshot, { window: deletedWindow, @@ -1531,7 +1593,12 @@ describe("indexed row selection", () => { // this design exists to remove. const all = datasetRows(200); const loadedSnapshot = modelFor(all.slice(0, 100)); - const loadedWindow = { start: 0, length: 100, datasetKey: DATASET_KEY }; + const loadedWindow = { + start: 0, + length: 100, + datasetKey: DATASET_KEY, + datasetTotal: 200, + }; const selection = cellRangeSelection("row-10", "row-90"); const stamped = reconcileIndexedSelection(selection, loadedSnapshot, { @@ -1548,7 +1615,12 @@ describe("indexed row selection", () => { // "row-90" at 89. const afterDelete = [...all.slice(0, 10), ...all.slice(11)]; const keptSnapshot = modelFor(afterDelete.slice(0, 40)); - const keptWindow = { start: 0, length: 40, datasetKey: DATASET_KEY }; + const keptWindow = { + start: 0, + length: 40, + datasetKey: DATASET_KEY, + datasetTotal: 199, + }; expect(keptSnapshot.indexOf({ kind: "data", rowId: "row-10" })).toBe(-1); expect(keptSnapshot.indexOf({ kind: "data", rowId: "row-90" })).toBe(-1); @@ -1567,6 +1639,9 @@ describe("indexed row selection", () => { start: 10, end: 89, datasetKey: DATASET_KEY, + // Re-stamped at the POST-deletion size, not the size it was measured + // at, or the range would be unreadable the instant after it narrowed. + datasetTotal: 199, }); // The deleted row's IDENTITY is gone from the range too, replaced by the // row that now holds the narrowed boundary. Leaving "row-10" there would @@ -1599,14 +1674,24 @@ describe("indexed row selection", () => { // eviction, not deletion, and the span keeps every one of its 81 rows. const all = datasetRows(200); const loadedSnapshot = modelFor(all.slice(0, 100)); - const loadedWindow = { start: 0, length: 100, datasetKey: DATASET_KEY }; + const loadedWindow = { + start: 0, + length: 100, + datasetKey: DATASET_KEY, + datasetTotal: 200, + }; const selection = cellRangeSelection("row-10", "row-90"); const stamped = reconcileIndexedSelection(selection, loadedSnapshot, { window: loadedWindow, }); const keptSnapshot = modelFor(all.slice(120, 160)); - const keptWindow = { start: 120, length: 40, datasetKey: DATASET_KEY }; + const keptWindow = { + start: 120, + length: 40, + datasetKey: DATASET_KEY, + datasetTotal: 200, + }; const after = reconcileIndexedSelection(stamped, keptSnapshot, { window: keptWindow, @@ -1618,6 +1703,7 @@ describe("indexed row selection", () => { start: 10, end: 90, datasetKey: DATASET_KEY, + datasetTotal: 200, }); expect( getIndexedCellSelectionSummary(after, keptSnapshot, keptWindow).rowCount, @@ -1630,7 +1716,12 @@ describe("indexed row selection", () => { // and an empty span is not a selection. const all = datasetRows(200); const loadedSnapshot = modelFor(all.slice(0, 100)); - const loadedWindow = { start: 0, length: 100, datasetKey: DATASET_KEY }; + const loadedWindow = { + start: 0, + length: 100, + datasetKey: DATASET_KEY, + datasetTotal: 200, + }; const selection = cellRangeSelection("row-10", "row-11"); const stamped = reconcileIndexedSelection(selection, loadedSnapshot, { @@ -1640,6 +1731,7 @@ describe("indexed row selection", () => { start: 10, end: 11, datasetKey: DATASET_KEY, + datasetTotal: 200, }); // Both removed, window unmoved: positions 10 and 11 are still covered. @@ -1651,6 +1743,8 @@ describe("indexed row selection", () => { start: 0, length: afterDelete.length, datasetKey: DATASET_KEY, + // Two rows deleted, and both of them proven. + datasetTotal: 198, }, previous: { snapshot: loadedSnapshot, window: loadedWindow }, }); @@ -1658,13 +1752,223 @@ describe("indexed row selection", () => { expect(after.ranges).toEqual([]); }); + test("an insert upstream of an evicted span refuses it, and recovers later", () => { + // `datasetKey` identifies the QUERY. Paging within one result is + // documented to keep it stable, so somebody else's insert arrives with + // the key unchanged and silently re-fills the positions the span + // remembers. The population SIZE is what notices. + const all = datasetRows(200); + const loadedSnapshot = modelFor(all.slice(0, 100)); + const loadedWindow = { + start: 0, + length: 100, + datasetKey: DATASET_KEY, + datasetTotal: 200, + }; + const stamped = reconcileIndexedSelection( + cellRangeSelection("row-10", "row-90"), + loadedSnapshot, + { window: loadedWindow }, + ); + + // Five rows inserted at the head. `row-10`..`row-90` now live at 15..95, + // and positions 10..90 name rows the user never selected. + const grown = [ + ...Array.from({ length: 5 }, (_, index) => ({ + id: `new-${index}`, + team: "a", + score: -1 - index, + })), + ...all, + ]; + const returnedSnapshot = modelFor(grown.slice(0, 8)); + const returnedWindow = { + start: 0, + length: 8, + datasetKey: DATASET_KEY, + datasetTotal: 205, + }; + const after = reconcileIndexedSelection(stamped, returnedSnapshot, { + window: returnedWindow, + previous: { snapshot: loadedSnapshot, window: loadedWindow }, + }); + + // The range is RETAINED -- nothing proves those rows are gone -- but its + // span is no longer readable, so nothing paints and nothing is counted. + expect(after.ranges).toHaveLength(1); + const contains = (rowId: string) => + indexedRangeContainsCell( + after.ranges[0]!, + { kind: "data", rowId }, + "team", + returnedSnapshot, + ["team", "score"], + returnedWindow, + ); + expect(["new-1", "new-4", "row-0", "row-2"].map(contains)).toEqual([ + false, + false, + false, + false, + ]); + expect( + getIndexedCellSelectionSummary(after, returnedSnapshot, returnedWindow), + ).toEqual({ rowCount: 0, verified: false }); + + // Failing closed is not failing permanently. A window covering both + // endpoints in the NEW population re-stamps the span against the new + // total, and the rows the user really chose paint again. + const healedSnapshot = modelFor(grown.slice(10, 100)); + const healedWindow = { + start: 10, + length: 90, + datasetKey: DATASET_KEY, + datasetTotal: 205, + }; + const healed = reconcileIndexedSelection(after, healedSnapshot, { + window: healedWindow, + previous: { snapshot: returnedSnapshot, window: returnedWindow }, + }); + expect(healed.ranges[0]?.datasetRowSpan).toEqual({ + start: 15, + end: 95, + datasetKey: DATASET_KEY, + datasetTotal: 205, + }); + expect( + getIndexedCellSelectionSummary(healed, healedSnapshot, healedWindow), + ).toEqual({ rowCount: 81, verified: true }); + }); + + test("KNOWN GAP: an equal insert and delete leaves the size unchanged, and is not caught", () => { + // Written down rather than left to be discovered. The fingerprint is the + // population's SIZE, so a mutation that does not change the size is + // invisible to it: here one row is removed from the head and one appended + // to the tail, everything shifts down by one, and the span keeps + // answering off positions that now name their neighbours. + // + // This is not the assertion anyone wants -- it is what the engine can + // honestly do with the evidence it has. Closing it needs a population + // token only the consumer can mint (see `eviction.mdx`), and this test is + // the place that will go red the day one exists. + const all = datasetRows(200); + const loadedSnapshot = modelFor(all.slice(0, 100)); + const loadedWindow = { + start: 0, + length: 100, + datasetKey: DATASET_KEY, + datasetTotal: 200, + }; + const stamped = reconcileIndexedSelection( + cellRangeSelection("row-10", "row-90"), + loadedSnapshot, + { window: loadedWindow }, + ); + + // `row-0` deleted, `row-200` appended: still 200 rows. + const churned = [...all.slice(1), { id: "row-200", team: "a", score: 200 }]; + const churnedSnapshot = modelFor(churned.slice(40, 60)); + const churnedWindow = { + start: 40, + length: 20, + datasetKey: DATASET_KEY, + datasetTotal: 200, + }; + const after = reconcileIndexedSelection(stamped, churnedSnapshot, { + window: churnedWindow, + previous: { snapshot: loadedSnapshot, window: loadedWindow }, + }); + + // `row-41` sits at position 40 now; the user's selection was 10..90 by + // POSITION, so it still paints -- correctly, as it happens. `row-91`, + // which shifted down to position 90, also still paints, and it is the + // one at the boundary that a token would have to arbitrate. + const contains = (rowId: string) => + indexedRangeContainsCell( + after.ranges[0]!, + { kind: "data", rowId }, + "team", + churnedSnapshot, + ["team", "score"], + churnedWindow, + ); + expect(contains("row-41")).toBe(true); + expect( + getIndexedCellSelectionSummary(after, churnedSnapshot, churnedWindow) + .verified, + ).toBe(false); + }); + + test("a shut honesty gate retains, while local mode still prunes", () => { + // The two things a null window can mean, side by side, from ONE fixture + // whose only difference is `windowed`. Every input to the gate is a + // transient property of a single render -- an in-flight count query, one + // revision of engine-side sort -- so reading its closure as "these rows + // were deleted" destroyed selections permanently and irrecoverably. + const all = datasetRows(200); + const loadedSnapshot = modelFor(all.slice(0, 100)); + const loadedWindow = { + start: 0, + length: 100, + datasetKey: DATASET_KEY, + datasetTotal: 200, + }; + const stamped = reconcileIndexedSelection( + cellRangeSelection("row-10", "row-90"), + loadedSnapshot, + { window: loadedWindow }, + ); + expect(stamped.ranges).toHaveLength(1); + + // The window slides off both endpoints on a revision that could not + // publish a window at all. + const blindSnapshot = modelFor(all.slice(150, 200)); + const blind = { snapshot: loadedSnapshot, window: loadedWindow }; + + const windowed = reconcileIndexedSelection(stamped, blindSnapshot, { + window: null, + windowed: true, + previous: blind, + }); + expect(windowed.ranges).toHaveLength(1); + expect(windowed.ranges[0]?.datasetRowSpan).toEqual({ + start: 10, + end: 90, + datasetKey: DATASET_KEY, + datasetTotal: 200, + }); + // Byte-for-byte untouched, so the very next revision that CAN see a + // window carries on as if the blip had not happened. + expect(windowed).toBe(stamped); + + // Local mode, same absent rows, same everything else: absence really is + // deletion there, and it still prunes exactly as it did before eviction + // existed. + const local = reconcileIndexedSelection(stamped, blindSnapshot, { + window: null, + windowed: false, + previous: blind, + }); + expect(local.ranges).toEqual([]); + // ...and a caller that says nothing at all gets the local answer too. + expect( + reconcileIndexedSelection(stamped, blindSnapshot, { window: null }) + .ranges, + ).toEqual([]); + }); + test("a population change resets spans instead of re-reading them", () => { // `resultMeta.datasetKey` is the signal that the positions a span // remembers now hold DIFFERENT rows. Spec scope: "selection surviving a // query change" is out -- a new datasetKey resets everything, as today. const all = datasetRows(200); const loadedSnapshot = modelFor(all.slice(0, 100)); - const loadedWindow = { start: 0, length: 100, datasetKey: "sort=name" }; + const loadedWindow = { + start: 0, + length: 100, + datasetKey: "sort=name", + datasetTotal: 200, + }; const selection = cellRangeSelection("row-10", "row-90"); const stamped = reconcileIndexedSelection(selection, loadedSnapshot, { @@ -1675,7 +1979,12 @@ describe("indexed row selection", () => { ).toEqual({ rowCount: 81, verified: true }); const resortedSnapshot = modelFor(all.slice(120, 160)); - const resortedWindow = { start: 120, length: 40, datasetKey: "sort=score" }; + const resortedWindow = { + start: 120, + length: 40, + datasetKey: "sort=score", + datasetTotal: 200, + }; const after = reconcileIndexedSelection(stamped, resortedSnapshot, { window: resortedWindow, previous: { snapshot: loadedSnapshot, window: loadedWindow }, @@ -1698,7 +2007,12 @@ describe("indexed row selection", () => { // that a reset caused it does not make the claim true. const all = datasetRows(200); const loadedSnapshot = modelFor(all.slice(0, 100)); - const loadedWindow = { start: 0, length: 100, datasetKey: "sort=name" }; + const loadedWindow = { + start: 0, + length: 100, + datasetKey: "sort=name", + datasetTotal: 200, + }; const selection = cellRangeSelection("row-10", "row-90"); const stamped = reconcileIndexedSelection(selection, loadedSnapshot, { @@ -1707,7 +2021,12 @@ describe("indexed row selection", () => { // A window that still covers "row-90" but not "row-10", under a new key. const resortedSnapshot = modelFor(all.slice(60, 100)); - const resortedWindow = { start: 60, length: 40, datasetKey: "sort=score" }; + const resortedWindow = { + start: 60, + length: 40, + datasetKey: "sort=score", + datasetTotal: 200, + }; const after = reconcileIndexedSelection(stamped, resortedSnapshot, { window: resortedWindow, previous: { snapshot: loadedSnapshot, window: loadedWindow }, @@ -1732,7 +2051,12 @@ describe("indexed row selection", () => { // `onSelectionChange` and persisted by a consumer. const all = datasetRows(200); const loadedSnapshot = modelFor(all.slice(0, 100)); - const loadedWindow = { start: 0, length: 100, datasetKey: "sort=name" }; + const loadedWindow = { + start: 0, + length: 100, + datasetKey: "sort=name", + datasetTotal: 200, + }; const selection = cellRangeSelection("row-10", "row-40"); const stamped = reconcileIndexedSelection(selection, loadedSnapshot, { @@ -1742,11 +2066,17 @@ describe("indexed row selection", () => { start: 10, end: 40, datasetKey: "sort=name", + datasetTotal: 200, }); // Same rows, re-sorted so they now sit 30 positions further along. const resortedSnapshot = modelFor(all.slice(0, 100)); - const resortedWindow = { start: 30, length: 100, datasetKey: "sort=score" }; + const resortedWindow = { + start: 30, + length: 100, + datasetKey: "sort=score", + datasetTotal: 200, + }; const after = reconcileIndexedSelection(stamped, resortedSnapshot, { window: resortedWindow, previous: { snapshot: loadedSnapshot, window: loadedWindow }, @@ -1757,6 +2087,7 @@ describe("indexed row selection", () => { start: 40, end: 70, datasetKey: "sort=score", + datasetTotal: 200, }); expect( getIndexedCellSelectionSummary(after, resortedSnapshot, resortedWindow), @@ -1800,11 +2131,16 @@ describe("indexed row selection", () => { const first = { start: 0, length: 100, + datasetTotal: 200, ...(datasetKey === undefined ? {} : { datasetKey }), }; const second = { start: 30, length: secondLength, + // A re-sort of the same 200 rows, so the SIZE is unchanged. That is + // the point: the population fingerprint cannot see a re-sort, which + // is exactly why `datasetKey` still has to. + datasetTotal: 200, ...(datasetKey === undefined ? {} : { datasetKey }), }; const stamped = reconcileIndexedSelection(selection, loadedSnapshot, { @@ -1862,8 +2198,18 @@ describe("indexed row selection", () => { run("sort=name", modelFor(all.slice(30, 130)), 100, "row-30"), ).toEqual({ summary: { rowCount: 31, verified: false }, - stampedWhileLoaded: { start: 10, end: 40, datasetKey: "sort=name" }, - stampedSpan: { start: 10, end: 40, datasetKey: "sort=name" }, + stampedWhileLoaded: { + start: 10, + end: 40, + datasetKey: "sort=name", + datasetTotal: 200, + }, + stampedSpan: { + start: 10, + end: 40, + datasetKey: "sort=name", + datasetTotal: 200, + }, paintsPosition30: true, }); }); @@ -1881,7 +2227,12 @@ describe("indexed row selection", () => { const range = { start: { rowId: "row-10" as Row["id"], columnId: "team" as const }, end: { rowId: "row-90" as Row["id"], columnId: "score" as const }, - datasetRowSpan: { start: 10, end: 90, datasetKey: "sort=name" }, + datasetRowSpan: { + start: 10, + end: 90, + datasetKey: "sort=name", + datasetTotal: 200, + }, }; const contains = (datasetKey: string) => @@ -1891,7 +2242,7 @@ describe("indexed row selection", () => { "team", snapshot, ["team", "score"], - { start: 40, length: 21, datasetKey }, + { start: 40, length: 21, datasetKey, datasetTotal: 200 }, ); // Positive twin first, so "false" below cannot be passing vacuously. diff --git a/packages/grid-core/src/create-grid-ui-core.ts b/packages/grid-core/src/create-grid-ui-core.ts index d30319b8..b6fb70be 100644 --- a/packages/grid-core/src/create-grid-ui-core.ts +++ b/packages/grid-core/src/create-grid-ui-core.ts @@ -39,6 +39,7 @@ import type { PretableIndexedRowRangeIndex, PretableIndexedSelectionState, PretableIndexedSelectionWindow, + PretableIndexedWindowing, PretableViewportState, } from "./types"; @@ -79,17 +80,21 @@ export interface CreateGridUiCoreOptions< readonly columns: readonly PretableGridUiColumn[]; readonly viewport?: PretableViewportState; /** - * @internal Late-bound getter for the loaded span (dataset-index terms) - * behind the same honesty gate as `aria-rowindex` and the scroll-extent - * spacers — see `WindowSpacers`/`getWindowSpacers` in `@pretable/react`'s - * `pretable-model.ts`, which this mirrors and is fed by. Read fresh on - * every row-model revision (never cached) so `reconcileIndexedSelection` - * can tell an evicted row from a deleted one — see - * {@link PretableIndexedSelectionWindow}. Undefined, or a getter that - * returns null, reproduces pre-eviction behavior exactly: every absent - * row is treated as deleted. + * @internal Late-bound getter for what the presentation layer knows about + * the loaded window — see `WindowState`/`getWindowing` in + * `@pretable/react`'s `pretable-model.ts`, which this mirrors and is fed + * by. Read fresh on every row-model revision (never cached) so + * `reconcileIndexedSelection` can tell an evicted row from a deleted one — + * see {@link PretableIndexedSelectionWindow}. + * + * Undefined, or a getter returning null, is LOCAL MODE and reproduces + * pre-eviction behaviour exactly: every absent row is treated as deleted. A + * non-null result whose `window` is null says the consumer IS windowed but + * this revision's honesty gate did not pass, which is a different thing and + * gets the opposite answer — see + * {@link PretableIndexedEvictionContext.windowed}. */ - readonly getSelectionWindow?: () => PretableIndexedSelectionWindow | null; + readonly getWindowing?: () => PretableIndexedWindowing | null; } const EMPTY_VIEWPORT: Readonly = Object.freeze({ @@ -307,7 +312,8 @@ function sameDatasetRowSpan( return ( left.start === right.start && left.end === right.end && - left.datasetKey === right.datasetKey + left.datasetKey === right.datasetKey && + left.datasetTotal === right.datasetTotal ); } @@ -394,6 +400,8 @@ export function createGridUiCore< | { readonly snapshot: PretableRowModelSnapshot; readonly window: PretableIndexedSelectionWindow | null; + /** See {@link PretableIndexedEvictionContext.windowed}. */ + readonly windowed: boolean; } | undefined; let state: PretableGridUiState = Object.freeze({ @@ -521,7 +529,13 @@ export function createGridUiCore< const interactionContext = (): { readonly snapshot: PretableRowModelSnapshot; readonly window: PretableIndexedSelectionWindow | null; - } => observed ?? { snapshot: snapshotForInteraction(), window: null }; + readonly windowed: boolean; + } => + observed ?? { + snapshot: snapshotForInteraction(), + window: null, + windowed: false, + }; const navigationColumnIds = (): readonly TColumnId[] => { if (cachedNavigationLayout !== state.columnLayout) { @@ -603,7 +617,7 @@ export function createGridUiCore< // that revision was observed; what is left absent is either // evicted or an address a consumer handed to `setFocus`, and // holding the cursor is the right answer to both. - eviction: { window: context.window }, + eviction: { window: context.window, windowed: context.windowed }, }); } catch (cause) { throw observationError( @@ -998,7 +1012,12 @@ export function createGridUiCore< ) { return; } - const selectionWindow = options.getSelectionWindow?.() ?? null; + // ONE read of the window channel per revision, into one object: + // windowed-ness and the window itself must describe the same + // instant or the engine can conclude "local mode" from a windowed + // grid's momentary gate closure. + const windowing = options.getWindowing?.() ?? null; + const selectionWindow = windowing?.window ?? null; // `observed` is read here, before the sole reassignment below (at // the end of this same block, after every read), so it still // holds the snapshot/window pairing from the LAST successful @@ -1008,7 +1027,11 @@ export function createGridUiCore< // The cursor and the selection are handed the SAME context, from // the same two reads: they have to reach the same verdict about a // row, or a grid retains a selection under a cursor that moved. - const eviction = { window: selectionWindow, previous: observed }; + const eviction = { + window: selectionWindow, + windowed: windowing !== null, + previous: observed, + }; const focus = reconcileIndexedFocus(state.focus, snapshot, eviction); const selection = reconcileIndexedSelection( observed === undefined @@ -1045,7 +1068,11 @@ export function createGridUiCore< } if (disposed || activeProjectionToken !== token) return; - observed = { snapshot, window: selectionWindow }; + observed = { + snapshot, + window: selectionWindow, + windowed: windowing !== null, + }; committed = true; publish({ ...state, diff --git a/packages/grid-core/src/index.ts b/packages/grid-core/src/index.ts index 35859472..682dea60 100644 --- a/packages/grid-core/src/index.ts +++ b/packages/grid-core/src/index.ts @@ -69,6 +69,7 @@ export type { PretableIndexedSelectionState, PretableIndexedSelectionSummary, PretableIndexedSelectionWindow, + PretableIndexedWindowing, PretableRowSelectionState, } from "./types"; export type { diff --git a/packages/grid-core/src/indexed-focus.ts b/packages/grid-core/src/indexed-focus.ts index a09dfe55..13d057fb 100644 --- a/packages/grid-core/src/indexed-focus.ts +++ b/packages/grid-core/src/indexed-focus.ts @@ -9,7 +9,11 @@ import type { PretableVisibleRowRef, } from "@pretable-internal/row-model"; -import { evictionRetentionWindow, provenDeletedRow } from "./indexed-selection"; +import { + evictionRetentionWindow, + evictionWindowUnknown, + provenDeletedRow, +} from "./indexed-selection"; import type { PretableHeaderRowRef, PretableIndexedEvictionContext, @@ -99,6 +103,13 @@ export function reconcileIndexedFocus< // streaming patch. if (focus.ref.kind === "header") return focus; if (snapshot.indexOf(focus.ref) >= 0) return focus; + // The window is unknown this revision, so this absence proves nothing — the + // same verdict `reconcileIndexedSelection` reaches through the same + // predicate, on the same revisions, which is the point of sharing it. A + // group ref is excluded for the reason it is excluded below: it has no + // dataset position, so no window could ever have said anything about it. + if (focus.ref.kind === "data" && evictionWindowUnknown(eviction)) + return focus; const retentionWindow = evictionRetentionWindow(eviction); if ( retentionWindow !== null && diff --git a/packages/grid-core/src/indexed-selection.ts b/packages/grid-core/src/indexed-selection.ts index f4fb8056..d0d90059 100644 --- a/packages/grid-core/src/indexed-selection.ts +++ b/packages/grid-core/src/indexed-selection.ts @@ -1737,13 +1737,34 @@ interface ResolvedDatasetSpan { * * Local mode is untouched: with no window there is no span to read in the * first place (see `endpointPositions`). + * + * ## The population is a second question, and the key does not answer it + * + * `datasetKey` identifies the QUERY. `lifecycle.mdx` tells consumers to keep + * it stable while they page within one result, so an insert or a delete made + * upstream of an evicted selection leaves the key matching while re-filling + * the remembered positions with entirely different rows. Measured: + * `row-1..row-8` selected and evicted, five rows prepended to the same + * result, and the returning window painted four rows that did not exist when + * the user selected — while the eight they did select painted nothing. + * + * So the size of the population is compared too, and it fails closed the same + * way. `provenDeletions` is the one allowance: rows this very call has + * PROVEN gone are a population change the engine observed, so a total short + * by exactly that many is fully accounted for rather than unexplained. Only + * {@link narrowDeletedEndpoints} passes a non-zero value; every reader that + * merely wants to trust a remembered position uses the strict form. */ function spanReadableInWindow( span: PretableIndexedDatasetRowSpan, window: PretableIndexedSelectionWindow, + provenDeletions = 0, ): boolean { return ( - window.datasetKey !== undefined && span.datasetKey === window.datasetKey + window.datasetKey !== undefined && + span.datasetKey === window.datasetKey && + span.datasetTotal !== undefined && + window.datasetTotal === span.datasetTotal - provenDeletions ); } @@ -2145,12 +2166,20 @@ function narrowDeletedEndpoints< deleted: { readonly start: boolean; readonly end: boolean }, ): PretableIndexedCellRange | undefined { const span = range.datasetRowSpan; + const count = (deleted.start ? 1 : 0) + (deleted.end ? 1 : 0); // No positional identity, so there is nothing to narrow BY. A windowed grid // that publishes no `datasetKey` lands here on every range, deliberately: // its spans are refused everywhere else too (see `spanReadableInWindow`). - if (span === undefined || !spanReadableInWindow(span, loadedWindow)) + // + // `count` is passed as the proven-deletion allowance: a deletion IS a + // population change, so the strict read would refuse every span the moment + // it had something to narrow. Requiring the total to be short by exactly + // the rows this call proved gone keeps the check meaningful — a revision + // that also inserted, or that deleted a third row elsewhere, does not add + // up and is refused, which drops the range rather than moving it to a + // position the engine cannot justify. + if (span === undefined || !spanReadableInWindow(span, loadedWindow, count)) return undefined; - const count = (deleted.start ? 1 : 0) + (deleted.end ? 1 : 0); const startIsHigh = span.start >= span.end; let nextStart = startIsHigh ? span.start - count : span.start; let nextEnd = startIsHigh ? span.end : span.end - count; @@ -2189,6 +2218,11 @@ function narrowDeletedEndpoints< start: nextStart, end: nextEnd, datasetKey: span.datasetKey, + // The CURRENT total, not the span's. These positions have just been + // rewritten to describe the post-deletion population, so stamping the + // pre-deletion size would leave the range permanently unreadable the + // instant after it was successfully narrowed. + datasetTotal: loadedWindow.datasetTotal, }), }); } @@ -2204,7 +2238,8 @@ function sameSpan( return ( left.start === right.start && left.end === right.end && - left.datasetKey === right.datasetKey + left.datasetKey === right.datasetKey && + left.datasetTotal === right.datasetTotal ); } @@ -2267,6 +2302,7 @@ function stampDatasetRowSpan< start, end, datasetKey: loadedWindow.datasetKey, + datasetTotal: loadedWindow.datasetTotal, }); if (sameSpan(range.datasetRowSpan, next)) return range; return Object.freeze({ ...range, datasetRowSpan: next }); @@ -2328,6 +2364,7 @@ export function adoptIndexedCellRangeSpans< start, end, datasetKey: loadedWindow.datasetKey, + datasetTotal: loadedWindow.datasetTotal, }); if (sameSpan(range.datasetRowSpan, next)) return range; return Object.freeze({ ...range, datasetRowSpan: next }); @@ -2367,6 +2404,44 @@ export function evictionRetentionWindow< return populationChanged ? null : suppliedWindow; } +/** + * Whether this revision's window is UNKNOWN rather than absent. + * + * A null window is two entirely different situations and they want opposite + * answers. In LOCAL mode the consumer hands over the whole result on every + * `setRows`, so an absent row really has been deleted and pruning is correct. + * In a WINDOWED grid whose honesty gate did not pass this revision — an + * in-flight count query, a backend that estimates past 10k, one revision of + * engine-side sort — the engine has been told nothing at all about which rows + * exist. Dropping a selection there is the engine ASSERTING a deletion it + * cannot have observed, from inputs that are every one of them transient + * properties of a single render. Measured: one such revision destroyed an + * eight-row selection AND the cursor permanently, and restoring the exact + * total afterwards brought neither back, because the span was already gone. + * + * So: absent proof of deletion, retain. The same rule the rest of this file + * already follows, applied to the case where the proof is not merely missing + * but unobtainable. + * + * NOT the same question as {@link evictionRetentionWindow}, which also + * returns null on a POPULATION CHANGE — a new `datasetKey`, where the engine + * has been told plenty and what it was told is "everything you remember is + * about a different table". That still resets, as the spec says it must. + * + * @internal Shared with `reconcileIndexedFocus`: the cursor and the selection + * must reach the same verdict on the same revisions, or a grid retains a + * selection under a cursor that moved. + */ +export function evictionWindowUnknown< + TRow extends object, + TRowId extends PretableRowId, + TColumns, +>( + eviction: PretableIndexedEvictionContext | undefined, +): boolean { + return (eviction?.windowed ?? false) && (eviction?.window ?? null) === null; +} + export function reconcileIndexedSelection< TRow extends object, TRowId extends PretableRowId, @@ -2389,6 +2464,15 @@ export function reconcileIndexedSelection< const retentionWindow = evictionRetentionWindow(eviction); let changed = false; rowSelectionProgram(selection, snapshot); + // The window is unknown this revision, so nothing below has anything to + // reason FROM: absence is not evidence of deletion, and a position stamped + // against no window would be a position in no coordinate system. A + // reconciliation that learned nothing changes nothing. Returned after the + // `rowSelectionProgram` call above, which rebuilds the visible-rank cache + // and is required on every revision regardless of what this decides. + // + // Local mode never reaches this line — see `evictionWindowUnknown`. + if (evictionWindowUnknown(eviction)) return selection; const ranges: PretableIndexedCellRange[] = []; for (const range of selection.ranges) { const startVisible = visibleAddress(range.start, snapshot); diff --git a/packages/grid-core/src/types.ts b/packages/grid-core/src/types.ts index 0e654266..18b979df 100644 --- a/packages/grid-core/src/types.ts +++ b/packages/grid-core/src/types.ts @@ -371,6 +371,33 @@ export interface PretableIndexedDatasetRowSpan { * key could qualify. */ readonly datasetKey?: string; + /** + * How many rows the population held when these positions were measured — + * `resultMeta.total.count`, which the honesty gate already forces to be + * EXACT before any of this is reachable. + * + * `datasetKey` identifies the QUERY, not the population, and the docs tell + * consumers to keep it stable while they page within one result. So an + * insert or a delete made by somebody else, upstream of an evicted + * selection, leaves the key matching while re-filling the remembered + * positions with different rows. Measured: a selection of `row-1..row-8` + * evicted, five rows prepended to the same result, and the returning window + * painted four rows that had not existed when the user selected — while the + * rows they did select painted nothing. + * + * The count is the cheapest thing that changes on any NET population + * change, and it is already required to be exact, so it costs the consumer + * nothing. It fails CLOSED for the same reason `datasetKey` does: an absent + * count is not agreement, it is the absence of evidence. + * + * **What it does not catch:** a change that leaves the size alone — an + * insert and a delete in the same revision, or a row replaced in place. + * Containment still answers from the remembered span there, and the summary + * still reports `verified: false`, which is all the engine can say. Closing + * that gap needs a population token only the consumer can mint; see + * `docs/content/server-side-data/eviction.mdx`. + */ + readonly datasetTotal?: number; } /** Inclusive data-cell range; group rows can never be endpoints. @public */ @@ -493,12 +520,41 @@ export interface PretableIndexedSelectionWindow { * none are read back. That is fail-closed by design, not an oversight. */ readonly datasetKey?: string; + /** + * How many rows the population currently holds — `resultMeta.total.count`. + * REQUIRED, not optional like `datasetKey`: the honesty gate that produces + * this window already refuses to pass without an exact total, so a window + * that cannot state its population size does not exist, and making the + * field optional would only create a way to fail OPEN by omission. See + * {@link PretableIndexedDatasetRowSpan.datasetTotal} for what comparing it + * catches and what it does not. + */ + readonly datasetTotal: number; +} + +/** + * What the presentation layer knows about the loaded window on one revision. + * + * A single object, read once, rather than a window getter beside a + * windowed-ness getter: two reads at two instants can disagree, and the whole + * of `create-grid-ui-core`'s `observed` pairing exists because that class of + * skew has already cost this engine a permanently wrong span. + * + * `null` from the getter means the consumer is not windowed at all — local + * mode. A non-null value with a null `window` means it IS windowed and this + * revision's honesty gate did not pass. See + * {@link PretableIndexedEvictionContext.windowed}. + * + * @internal + */ +export interface PretableIndexedWindowing { + readonly window: PretableIndexedSelectionWindow | null; } /** * What a reconciliation pass needs in order to tell an evicted row from a - * deleted one. Absent, or a null `window` (local mode, or the honesty gate not - * passing), makes every consumer behave exactly as it did before eviction + * deleted one. Absent, or absent `windowed` with a null `window` (local + * mode), makes every consumer behave exactly as it did before eviction * existed: absence alone still means deletion. * * ONE shape, shared by `reconcileIndexedSelection` and @@ -516,6 +572,29 @@ export interface PretableIndexedEvictionContext< /** The loaded span for the snapshot being reconciled, in dataset-index * terms. See {@link PretableIndexedSelectionWindow}. */ readonly window: PretableIndexedSelectionWindow | null; + /** + * Whether the consumer is serving a WINDOW at all — it publishes + * `resultMeta.window` — regardless of whether this revision's honesty gate + * passed. + * + * This is what separates the two things a null `window` can mean, and they + * demand opposite answers: + * + * - `windowed: false` — **local mode.** The consumer hands over the whole + * result every time, so a row that is absent genuinely has been deleted. + * Prune, exactly as before eviction existed. + * - `windowed: true` with a null `window` — **the window is UNKNOWN this + * revision.** An in-flight count query, a backend that estimates past + * 10k, one revision of engine-side sort. The engine has learned nothing + * about which rows exist, and dropping a selection here would be it + * ASSERTING a deletion it cannot possibly have observed. Retain. + * + * Derived per render from what the consumer publishes, never latched: + * a remembered bit would keep claiming a window long after a grid stopped + * serving one. Defaults to `false`, so a caller that says nothing gets the + * pre-eviction behaviour rather than the retaining one. + */ + readonly windowed?: boolean; /** * The snapshot/window pairing as of the last successful reconciliation, if * any — read to prove deletion (see `provenDeletedRow`); never mutated. A diff --git a/packages/react/react.api.md b/packages/react/react.api.md index 5a0eea0f..3a40d630 100644 --- a/packages/react/react.api.md +++ b/packages/react/react.api.md @@ -87,7 +87,7 @@ export interface CreateGridUiCoreOptions[]; // @internal - readonly getSelectionWindow?: () => ɵPretableIndexedSelectionWindow | null; + readonly getWindowing?: () => ɵPretableIndexedWindowing | null; // (undocumented) readonly rowModel: PretableRowModel; // (undocumented) @@ -1400,6 +1400,7 @@ export interface PretableIndexedCellSelectionSummary { // @public export interface PretableIndexedDatasetRowSpan { readonly datasetKey?: string; + readonly datasetTotal?: number; readonly end: number; readonly start: number; } @@ -2661,12 +2662,21 @@ export function ɵmeasureRenderedRowHeight(row: HTMLElement, minRowHeight?: numb // @internal export interface ɵPretableIndexedSelectionWindow { readonly datasetKey?: string; + readonly datasetTotal: number; // (undocumented) readonly length: number; // (undocumented) readonly start: number; } +// Warning: (ae-internal-missing-underscore) The name "ɵPretableIndexedWindowing" should be prefixed with an underscore because the declaration is marked as @internal +// +// @internal +export interface ɵPretableIndexedWindowing { + // (undocumented) + readonly window: ɵPretableIndexedSelectionWindow | null; +} + // Warning: (ae-internal-missing-underscore) The name "ɵROW_SELECT_COLUMN_ID" should be prefixed with an underscore because the declaration is marked as @internal // // @internal diff --git a/packages/react/src/__tests__/eviction-gate-blip.test.tsx b/packages/react/src/__tests__/eviction-gate-blip.test.tsx new file mode 100644 index 00000000..d8a3164a --- /dev/null +++ b/packages/react/src/__tests__/eviction-gate-blip.test.tsx @@ -0,0 +1,229 @@ +import "@testing-library/jest-dom/vitest"; +import { act, cleanup, fireEvent, render } from "@testing-library/react"; +import * as React from "react"; +import { afterEach, describe, expect, it } from "vitest"; + +import type { + PretableMatchingTotal, + PretableProcessingOptions, +} from "@pretable/core"; +import { PretableSurface } from "../pretable-surface"; + +/** + * One revision on which the honesty gate does not pass, while the window + * moves. + * + * The gate closing is a statement about what the engine can VERIFY this + * render — an in-flight count query, a backend that estimates past 10k, a + * single revision of engine-side sort. It is not a statement about which rows + * exist. Before this, that one revision made every absent row read as deleted + * and destroyed the selection AND the cursor irrecoverably: restoring the + * exact total afterwards brought nothing back, because the span had already + * been discarded. + * + * UNCONTROLLED on purpose. A controlled consumer is accidentally immune — + * the `state.selection` echo re-supplies on the next render what the engine + * threw away — so the controlled shape cannot see this at all. + */ + +type Row = { id: string; name: string; score: number }; + +const TOTAL = 20; +const ALL: Row[] = Array.from({ length: TOTAL }, (_, index) => ({ + id: `row-${index}`, + name: `name-${index}`, + score: index, +})); + +const columns = [ + { id: "name", header: "Name", widthPx: 120 }, + { id: "score", header: "Score", widthPx: 120 }, +]; + +const EXTERNAL: PretableProcessingOptions = { + filter: "external", + sort: "external", +}; + +const POPULATION = "sort=name"; +const QUERY = { filters: [], sort: [], rowGroups: [] }; +const EXACT: PretableMatchingTotal = { kind: "exact", count: TOTAL }; +/** An in-flight count query, or a backend that stops counting past 10k. */ +const ESTIMATE: PretableMatchingTotal = { kind: "estimate", count: TOTAL }; + +async function settle() { + await act(async () => { + await new Promise((resolve) => setTimeout(resolve, 20)); + }); +} + +function WindowedGrid({ + windowStart, + length = 10, + total = EXACT, + processing = EXTERNAL, +}: { + windowStart: number; + length?: number; + total?: PretableMatchingTotal; + processing?: PretableProcessingOptions; +}) { + return ( + + ariaLabel="Windowed" + columns={columns} + rows={ALL.slice(windowStart, windowStart + length)} + getRowId={(row) => row.id} + viewportHeight={800} + processing={processing} + resultMeta={{ + total, + window: { + start: windowStart, + hasMore: windowStart + length < TOTAL, + }, + datasetKey: POPULATION, + }} + query={QUERY} + onQueryChange={() => undefined} + /> + ); +} + +function bodyCell( + container: HTMLElement, + rowId: string, + columnId: string, +): HTMLElement { + const cell = container.querySelector( + `[data-pretable-row-id="${rowId}"] [data-pretable-column-id="${columnId}"]`, + ); + if (!cell) throw new Error(`no cell ${columnId}@${rowId}`); + return cell as HTMLElement; +} + +/** Row ids whose `name` cell is painting selected, in DOM order. */ +function painted(container: HTMLElement): string[] { + return Array.from(container.querySelectorAll("[data-pretable-row-id]")) + .filter( + (node) => + node + .querySelector('[data-pretable-column-id="name"]') + ?.getAttribute("data-pretable-selected") === "true", + ) + .map((node) => node.getAttribute("data-pretable-row-id") ?? ""); +} + +/** `[rowId, columnId]` of the cursor, or `[]` when there is none. */ +function focused(container: HTMLElement): string[] { + const cell = container.querySelector( + '[data-pretable-column-id][data-pretable-focused="true"]', + ); + if (cell === null) return []; + return [ + cell + .closest("[data-pretable-row-id]") + ?.getAttribute("data-pretable-row-id") ?? "", + cell.getAttribute("data-pretable-column-id") ?? "", + ]; +} + +async function selectThenSlideAndReturn(blip: { + total?: PretableMatchingTotal; + processing?: PretableProcessingOptions; +}) { + const { container, rerender } = render(); + fireEvent.click(bodyCell(container, "row-1", "name")); + fireEvent.click(bodyCell(container, "row-8", "name"), { shiftKey: true }); + const whileLoaded = painted(container); + + // The window slides on the one render whose gate is shut. + rerender(); + await settle(); + // Gate restored, exactly as it was before the blip. + rerender(); + await settle(); + // Scroll back to where the selection lives. + rerender(); + await settle(); + + return { whileLoaded, after: painted(container), cursor: focused(container) }; +} + +const SELECTED = [ + "row-1", + "row-2", + "row-3", + "row-4", + "row-5", + "row-6", + "row-7", + "row-8", +]; + +afterEach(cleanup); + +describe("a window that slides while the honesty gate is shut", () => { + it("CONTROL: an ordinary slide with the gate open keeps both", async () => { + // The positive twin. Without it every assertion below is satisfied by a + // fixture that never selected anything in the first place. + const result = await selectThenSlideAndReturn({}); + expect(result.whileLoaded).toEqual(SELECTED); + expect(result.after).toEqual(SELECTED); + expect(result.cursor).toEqual(["row-8", "name"]); + }); + + it("keeps the selection and the cursor across an estimated total", async () => { + const result = await selectThenSlideAndReturn({ total: ESTIMATE }); + expect(result.whileLoaded).toEqual(SELECTED); + expect(result.after).toEqual(SELECTED); + expect(result.cursor).toEqual(["row-8", "name"]); + }); + + it("LOCAL MODE: a row that genuinely disappears still loses its selection", async () => { + // The other side of the discriminator, at the level the bug was found. + // A grid with no `resultMeta.window` hands over the whole result on every + // render, so an absent row really has been deleted — and "retain when the + // window is unknown" must not leak into it, or a local grid would keep + // painting rows the consumer removed. + const { container, rerender } = render( + + ariaLabel="Local" + columns={columns} + rows={ALL} + getRowId={(row) => row.id} + viewportHeight={800} + />, + ); + fireEvent.click(bodyCell(container, "row-1", "name")); + fireEvent.click(bodyCell(container, "row-8", "name"), { shiftKey: true }); + expect(painted(container)).toEqual(SELECTED); + + // `row-1`..`row-8` are gone from the data. No window, so there is nothing + // to call this an eviction. + rerender( + + ariaLabel="Local" + columns={columns} + rows={[...ALL.slice(0, 1), ...ALL.slice(9)]} + getRowId={(row) => row.id} + viewportHeight={800} + />, + ); + await settle(); + + expect(painted(container)).toEqual([]); + expect(focused(container)).toEqual([]); + }); + + it("keeps them across one revision of engine-side sort", async () => { + // A second, entirely different way to shut the same gate — so the fix + // cannot be a special case for `total.kind`. + const result = await selectThenSlideAndReturn({ + processing: { filter: "external", sort: "engine" }, + }); + expect(result.whileLoaded).toEqual(SELECTED); + expect(result.after).toEqual(SELECTED); + expect(result.cursor).toEqual(["row-8", "name"]); + }); +}); diff --git a/packages/react/src/__tests__/eviction-population-change.test.tsx b/packages/react/src/__tests__/eviction-population-change.test.tsx new file mode 100644 index 00000000..521a0e8f --- /dev/null +++ b/packages/react/src/__tests__/eviction-population-change.test.tsx @@ -0,0 +1,273 @@ +import "@testing-library/jest-dom/vitest"; +import { act, cleanup, fireEvent, render } from "@testing-library/react"; +import * as React from "react"; +import { afterEach, describe, expect, it } from "vitest"; + +import type { + PretableProcessingOptions, + PretableSelectionState, +} from "@pretable/core"; +import { PretableSurface } from "../pretable-surface"; + +/** + * A selection whose rows are evicted, when the POPULATION changes underneath + * it while they are gone. + * + * `datasetKey` identifies the QUERY, not the population — by design, and the + * docs say so out loud ("keep it stable while you page within one result"). + * So an insert or a delete upstream of an evicted selection leaves the key + * matching while silently re-filling the remembered dataset positions with + * different rows. Before the population fingerprint, that painted five rows + * selected of which four had not existed when the user selected, and painted + * nothing on the rows the user actually chose. + * + * Driven through the public surface, because the failure IS the paint: a + * grid-core test can only observe the span, and the span was never the lie. + */ + +type Row = { id: string; name: string; score: number }; + +const BASE: Row[] = Array.from({ length: 20 }, (_, index) => ({ + id: `row-${index}`, + name: `name-${index}`, + score: index, +})); + +/** Five rows inserted UPSTREAM of the selection, by someone else, same query. */ +const PREPENDED: Row[] = [ + ...Array.from({ length: 5 }, (_, index) => ({ + id: `new-${index}`, + name: `fresh-${index}`, + score: -1 - index, + })), + ...BASE, +]; + +const columns = [ + { id: "name", header: "Name", widthPx: 120 }, + { id: "score", header: "Score", widthPx: 120 }, +]; + +const EXTERNAL: PretableProcessingOptions = { + filter: "external", + sort: "external", +}; + +/** + * ONE key for the whole file, deliberately. The bug is precisely that the + * consumer is doing the documented right thing: same query, so same key. + */ +const POPULATION = "sort=name"; + +const QUERY = { filters: [], sort: [], rowGroups: [] }; + +async function settle() { + await act(async () => { + await new Promise((resolve) => setTimeout(resolve, 20)); + }); +} + +function WindowedGrid({ + dataset, + windowStart, + length, + onSelection, +}: { + dataset: readonly Row[]; + windowStart: number; + length: number; + onSelection?: (next: PretableSelectionState) => void; +}) { + const [selection, setSelection] = React.useState({ + ranges: [], + anchor: null, + }); + return ( + + ariaLabel="Windowed" + columns={columns} + rows={dataset.slice(windowStart, windowStart + length)} + getRowId={(row) => row.id} + viewportHeight={800} + processing={EXTERNAL} + resultMeta={{ + total: { kind: "exact", count: dataset.length }, + window: { + start: windowStart, + hasMore: windowStart + length < dataset.length, + }, + datasetKey: POPULATION, + }} + query={QUERY} + onQueryChange={() => undefined} + state={{ selection }} + onSelectionChange={(next) => { + setSelection(next); + onSelection?.(next); + }} + /> + ); +} + +function bodyCell( + container: HTMLElement, + rowId: string, + columnId: string, +): HTMLElement { + const cell = container.querySelector( + `[data-pretable-row-id="${rowId}"] [data-pretable-column-id="${columnId}"]`, + ); + if (!cell) throw new Error(`no cell ${columnId}@${rowId}`); + return cell as HTMLElement; +} + +/** + * Every rendered row, in DOM order, with what its `name` cell is PAINTING. + * + * The CELL attribute, not the row's. A range spanning one column of two is + * not a fully-selected row, so `data-pretable-selected` on the row element is + * `false` for every row here — including the ones the user really did select. + * Reading it would make every assertion in this file vacuously true. + */ +function paintReport( + container: HTMLElement, +): { rowId: string; selected: boolean }[] { + return Array.from(container.querySelectorAll("[data-pretable-row-id]")).map( + (node) => ({ + rowId: node.getAttribute("data-pretable-row-id") ?? "", + selected: + node + .querySelector('[data-pretable-column-id="name"]') + ?.getAttribute("data-pretable-selected") === "true", + }), + ); +} + +afterEach(cleanup); + +describe("an evicted selection when the population changes underneath it", () => { + it("paints nothing rather than rows the reader never selected", async () => { + const seen: PretableSelectionState[] = []; + const { container, rerender } = render( + seen.push(next)} + />, + ); + + fireEvent.click(bodyCell(container, "row-1", "name")); + fireEvent.click(bodyCell(container, "row-8", "name"), { shiftKey: true }); + expect(seen.at(-1)?.ranges[0]?.datasetRowSpan).toMatchObject({ + start: 1, + end: 8, + datasetKey: POPULATION, + }); + + // Both endpoints evict. + rerender( + seen.push(next)} + />, + ); + await settle(); + expect( + container.querySelector('[data-pretable-row-id="row-1"]'), + ).toBeNull(); + expect( + container.querySelector('[data-pretable-row-id="row-8"]'), + ).toBeNull(); + + // Somebody else inserts five rows at the head of the SAME result. The + // query has not changed, so `datasetKey` correctly has not either — and + // dataset positions 1..8 now name entirely different rows. + rerender( + seen.push(next)} + />, + ); + await settle(); + + const report = paintReport(container); + expect(report.map((entry) => entry.rowId)).toEqual([ + "new-0", + "new-1", + "new-2", + "new-3", + "new-4", + "row-0", + ]); + // Not one of these rows was in the selection. Four of them did not exist + // when it was made. + expect(report.filter((entry) => entry.selected)).toEqual([]); + }); + + it("still paints the selection back when the population did NOT change", async () => { + // The positive twin. Without it the assertion above is satisfied by a + // fixture that simply never paints anything, and eviction's whole promise + // — "a selected row returns selected" — could be deleted with the test + // still green. + const { container, rerender } = render( + , + ); + + fireEvent.click(bodyCell(container, "row-1", "name")); + fireEvent.click(bodyCell(container, "row-8", "name"), { shiftKey: true }); + + rerender(); + await settle(); + rerender(); + await settle(); + + expect(paintReport(container)).toEqual([ + { rowId: "row-0", selected: false }, + { rowId: "row-1", selected: true }, + { rowId: "row-2", selected: true }, + { rowId: "row-3", selected: true }, + { rowId: "row-4", selected: true }, + { rowId: "row-5", selected: true }, + ]); + }); + + it("recovers the real rows once both endpoints are loaded again", async () => { + // Failing closed is not the same as failing permanently. Once the window + // covers the selection's actual rows in the NEW population, both + // endpoints resolve first-hand, the span is re-stamped against the new + // total, and the right rows paint again. + const { container, rerender } = render( + , + ); + + fireEvent.click(bodyCell(container, "row-1", "name")); + fireEvent.click(bodyCell(container, "row-8", "name"), { shiftKey: true }); + + rerender(); + await settle(); + rerender(); + await settle(); + // `row-1`..`row-8` now live at dataset positions 6..13. + rerender(); + await settle(); + + const selected = paintReport(container) + .filter((entry) => entry.selected) + .map((entry) => entry.rowId); + expect(selected).toEqual([ + "row-1", + "row-2", + "row-3", + "row-4", + "row-5", + "row-6", + "row-7", + "row-8", + ]); + }); +}); diff --git a/packages/react/src/__tests__/eviction-selection-surface.test.tsx b/packages/react/src/__tests__/eviction-selection-surface.test.tsx index 3cbc11b8..1e6d9737 100644 --- a/packages/react/src/__tests__/eviction-selection-surface.test.tsx +++ b/packages/react/src/__tests__/eviction-selection-surface.test.tsx @@ -157,6 +157,7 @@ describe("a cell selection whose rows get evicted", () => { start: 1, end: 8, datasetKey: POPULATION, + datasetTotal: TOTAL, }); // Scroll on by five rows. `row-1` (dataset position 1) leaves the loaded @@ -186,6 +187,7 @@ describe("a cell selection whose rows get evicted", () => { start: 1, end: 12, datasetKey: POPULATION, + datasetTotal: TOTAL, }); }); @@ -215,6 +217,7 @@ describe("a cell selection whose rows get evicted", () => { start: 5, end: 14, datasetKey: POPULATION, + datasetTotal: TOTAL, }); }); @@ -236,7 +239,12 @@ describe("a cell selection whose rows get evicted", () => { endRowId: "row-8", startColumnId: "name", endColumnId: "name", - datasetRowSpan: { start: 1, end: 8, datasetKey: POPULATION }, + datasetRowSpan: { + start: 1, + end: 8, + datasetKey: POPULATION, + datasetTotal: TOTAL, + }, }, ], anchor: { rowId: "row-1", columnId: "name" }, @@ -253,6 +261,39 @@ describe("a cell selection whose rows get evicted", () => { start: 1, end: 15, datasetKey: POPULATION, + datasetTotal: TOTAL, }); }); + + it("refuses a restored span that cannot say what population it measured", () => { + // The fail-closed twin of the test above, and the reason `datasetTotal` + // is not optional on a window. A span carrying positions but no + // population size is exactly what a consumer persisted BEFORE the + // population could change under it -- or hand-wrote -- and reading it + // would resurrect the bug the field exists to close. The same restore, + // one field short, recovers nothing. + const seen: PretableSelectionState[] = []; + const { container } = render( + seen.push(next)} + />, + ); + + fireEvent.click(bodyCell(container, "row-15", "name"), { shiftKey: true }); + + expect(seen.at(-1)?.ranges[0]?.datasetRowSpan).toBeUndefined(); + }); }); diff --git a/packages/react/src/pretable-model.ts b/packages/react/src/pretable-model.ts index 2240fe2d..b0d2205f 100644 --- a/packages/react/src/pretable-model.ts +++ b/packages/react/src/pretable-model.ts @@ -484,10 +484,37 @@ export interface WindowSpacers { * `resultMeta.datasetKey`, carried on the same honesty-gated push as the * row counts rather than on a second channel — a dataset position and the * population it was measured in must never be able to disagree. The row - * layout controller ignores it; only `getSelectionWindow` below reads it, - * to invalidate selection spans when the population changes. + * layout controller ignores it; only `getWindowing` below reads it, to + * invalidate selection spans when the QUERY changes. */ readonly datasetKey?: string; + /** + * `resultMeta.total.count` — exact by the time this object exists, because + * the gate that builds it does not pass otherwise. The row layout + * controller ignores it too; `getWindowing` reads it to invalidate + * selection spans when the POPULATION changes, which `datasetKey` does not + * report and is not meant to. See + * `PretableIndexedDatasetRowSpan.datasetTotal`. + */ + readonly datasetTotal?: number; +} + +/** + * The window channel's value: what this render knows about the loaded window. + * + * Two facts, pushed together because separating them is what caused a + * windowed grid to be mistaken for a local one. `spacers` is honesty-gated + * and null whenever the gate does not pass; `windowed` says only whether the + * consumer publishes `resultMeta.window` at all, which no gate can change. + * A grid that is windowed with null `spacers` has an UNKNOWN window this + * render — not an absent one — and the engine must not read absence as + * deletion there. See {@link PretableIndexedEvictionContext.windowed}. + * + * @internal + */ +export interface WindowState { + readonly spacers: WindowSpacers | null; + readonly windowed: boolean; } /** Internal indexed implementation shared by the public ownership overloads. */ @@ -499,8 +526,8 @@ export function usePretableModelInternal< >( options: UseIndexedPretableOptions, ): PretableModel & { - /** @internal See {@link WindowSpacers}. */ - readonly setWindowSpacers: (spacers: WindowSpacers | null) => void; + /** @internal See {@link WindowState}. */ + readonly setWindowState: (next: WindowState) => void; } { const columnSource = options.columns; const rowModel = options.rowModel; @@ -525,17 +552,17 @@ export function usePretableModelInternal< // anything reachable from a `useRef` into a function called there — even a // getter that only reads `.current` when invoked later. Same reasoning as // `queryChangeChannel` just above using `createLatestValueChannel` instead - // of a ref. `setWindowSpacers` and `getWindowSpacers` both have stable + // of a ref. `setWindowState` and `getWindowSpacers` both have stable // identity, so a caller never has to list either as a changing dependency. const [windowSpacersChannel] = useState(() => - createLatestValueChannel(null), + createLatestValueChannel({ spacers: null, windowed: false }), ); - const setWindowSpacers = useCallback( - (spacers: WindowSpacers | null) => windowSpacersChannel.set(spacers), + const setWindowState = useCallback( + (next: WindowState) => windowSpacersChannel.set(next), [windowSpacersChannel], ); const getWindowSpacers = useCallback( - () => windowSpacersChannel.get(), + () => windowSpacersChannel.get().spacers, [windowSpacersChannel], ); const schemaColumns = rowModel.getColumns() as readonly { @@ -566,7 +593,7 @@ export function usePretableModelInternal< height: options.viewportHeight, width: options.viewportWidth ?? 0, }, - // Adapts `getWindowSpacers` (see `WindowSpacers` above) to the + // Adapts the window channel (see `WindowState` above) to the // dataset-index span `reconcileIndexedSelection` needs to tell an // evicted row from a deleted one — the SAME honesty-gated channel the // row layout controller reads, not a second one. `leadingRows` is @@ -585,15 +612,30 @@ export function usePretableModelInternal< // If a consumer ever lands rows in a commit whose // `resultMeta.window.start` has not caught up, this pairing is a // chimera and a genuinely evicted row can be judged deleted. - getSelectionWindow: () => { - const spacers = getWindowSpacers(); - if (spacers?.leadingRows === undefined) return null; + getWindowing: () => { + // ONE read of the channel. `windowed` and `spacers` describing + // different instants is precisely the confusion this shape exists to + // prevent. + const { spacers, windowed } = windowSpacersChannel.get(); + if (!windowed) return null; + if ( + spacers?.leadingRows === undefined || + spacers.datasetTotal === undefined + ) { + // Windowed, but this revision's window cannot be trusted. NOT the + // same as local mode: the engine must hold what it has rather than + // conclude that every unloaded row was deleted. + return { window: null }; + } return { - start: spacers.leadingRows, - length: rowModel.getState().snapshot.sourceRowCount, - ...(spacers.datasetKey === undefined - ? {} - : { datasetKey: spacers.datasetKey }), + window: { + start: spacers.leadingRows, + length: rowModel.getState().snapshot.sourceRowCount, + datasetTotal: spacers.datasetTotal, + ...(spacers.datasetKey === undefined + ? {} + : { datasetKey: spacers.datasetKey }), + }, }; }, }); @@ -921,6 +963,6 @@ export function usePretableModelInternal< TColumns >, status: rowModelState.status, - setWindowSpacers, + setWindowState, }; } diff --git a/packages/react/src/pretable-surface.tsx b/packages/react/src/pretable-surface.tsx index a86793ce..51b1a71e 100644 --- a/packages/react/src/pretable-surface.tsx +++ b/packages/react/src/pretable-surface.tsx @@ -87,7 +87,11 @@ import type { PretableSurfaceState, PretableTelemetry, } from "./surface-types"; -import type { PretableReactGrid, WindowSpacers } from "./pretable-model"; +import type { + PretableReactGrid, + WindowSpacers, + WindowState, +} from "./pretable-model"; import { useResolvedHeights, useResolvedPx } from "./density"; import { DEFAULT_ROW_HEIGHT, @@ -1943,8 +1947,8 @@ export function PretableSurface< PretableRowId, readonly PretableColumn[] > & { - /** @internal See {@link WindowSpacers} in `pretable-model.ts`. */ - readonly setWindowSpacers: (spacers: WindowSpacers | null) => void; + /** @internal See {@link WindowState} in `pretable-model.ts`. */ + readonly setWindowState: (next: WindowState) => void; }; const { renderSnapshot, rowModelSnapshot } = indexed; const presentationQuery = @@ -2857,6 +2861,14 @@ export function PretableSurface< // readable while the population it was measured in is still the // one on screen (see `PretableIndexedDatasetRowSpan.datasetKey`). ...(datasetKey === undefined ? {} : { datasetKey }), + // The QUERY identity above answers "is this the same result?". + // This answers "is it the same SIZE?", which the key deliberately + // does not — consumers are told to hold the key stable while they + // page, so somebody else's insert or delete arrives with the key + // unchanged and silently re-fills the positions an evicted + // selection remembers. See + // `PretableIndexedDatasetRowSpan.datasetTotal`. + datasetTotal: matchingTotal.count, // Rows the population claims exist past this window's end. Never // negative: a window whose end already meets or exceeds the // claimed total — the ordinary un-windowed case, or a window's @@ -2900,11 +2912,14 @@ export function PretableSurface< // the announced position contradict each other for a frame. const selectionWindow = useMemo( () => - windowSpacers === null || windowSpacers.leadingRows === undefined + windowSpacers === null || + windowSpacers.leadingRows === undefined || + windowSpacers.datasetTotal === undefined ? null : { start: windowSpacers.leadingRows, length: rowModelSnapshot.sourceRowCount, + datasetTotal: windowSpacers.datasetTotal, ...(windowSpacers.datasetKey === undefined ? {} : { datasetKey: windowSpacers.datasetKey }), @@ -2919,7 +2934,15 @@ export function PretableSurface< // current before the controller's own layout effect next reads it, which // runs on every commit regardless. useInsertionEffect(() => { - indexed.setWindowSpacers(windowSpacers); + // `windowed` is NOT gated. Whether the consumer serves a window is a fact + // about the consumer; whether this render could verify one is a fact + // about this render. Collapsing the two is what let a single estimated + // total read as local mode and destroy a selection permanently — see + // `WindowState` in `pretable-model.ts`. + indexed.setWindowState({ + spacers: windowSpacers, + windowed: windowStart !== undefined, + }); }); // Same honesty gate as the offset and the spacers above (`windowSpacers` // null means the window cannot be trusted, so there is nothing honest to diff --git a/packages/react/src/public_api.ts b/packages/react/src/public_api.ts index d228ca49..1c7a82cb 100644 --- a/packages/react/src/public_api.ts +++ b/packages/react/src/public_api.ts @@ -270,6 +270,10 @@ export { useResolvedHeights as ɵuseResolvedHeights } from "./density"; export type { DensityScopeRef as ɵDensityScopeRef } from "./density"; export { measureRenderedRowHeight as ɵmeasureRenderedRowHeight } from "./row-height"; export { ROW_SELECT_COLUMN_ID as ɵROW_SELECT_COLUMN_ID } from "./constants"; -// Named by `CreateGridUiCoreOptions.getSelectionWindow`'s signature, which this -// package re-exports from `@pretable/core`, so it has to ship with it here too. -export type { ɵPretableIndexedSelectionWindow } from "@pretable/core"; +// Named by `CreateGridUiCoreOptions.getWindowing`'s signature, which this +// package re-exports from `@pretable/core`, so both have to ship with it here +// too. +export type { + ɵPretableIndexedSelectionWindow, + ɵPretableIndexedWindowing, +} from "@pretable/core";