diff --git a/.changeset/filter-tree-core.md b/.changeset/filter-tree-core.md new file mode 100644 index 000000000..b3a301e89 --- /dev/null +++ b/.changeset/filter-tree-core.md @@ -0,0 +1,44 @@ +--- +"@pretable/core": minor +--- + +Filters are an AND/OR tree. + +`PretableQueryFor.filters` is still an array, and an array of plain leaves +still means exactly what it meant before — the top level is an implicit AND. +What is new is that an element may also be a **group**: + +```ts +interface PretableFilterGroupFor { + readonly op: "and" | "or"; + readonly children: readonly PretableFilterNodeFor[]; +} +``` + +Groups nest, so a query can express any AND/OR shape. `PretableFilterNodeFor` +is the union of a typed leaf and a group — the type most call sites reading +`filters` want — and `isPretableFilterGroup(node)` narrows one to a group. The +guard checks the group's own fields positively, so an unrecognized shape fails +closed rather than being treated as a branch with no children. + +Two rules a consumer has to know: + +- **An EMPTY group holds — for BOTH operators.** `{ op: "or", children: [] }` + keeps every row, exactly like `{ op: "and", children: [] }`. Naive algebra + says an empty OR is false; that answer is wrong for a product, because a + group the user is still assembling in a filter builder would blank the grid + the moment it appeared. An empty group constrains nothing. +- **Nesting is bounded at 64 levels below the root.** Top-level elements sit at + depth 0, so a node at depth 65 — a group nested more than 64 deep — makes + `compileQuery` fail the query with `code: "invalid-query"` and a + `query.filters[i].children[j]…` path. This is a new reason for an existing + rejection, and the only way an otherwise well-formed query can now be + refused. + +Evaluation, query equality (so plan reuse and recompile decisions), capture and +freezing, and `distinctValues` all recurse. Equality stays order-insensitive +per level, which AND and OR both license. + +Nothing in this release builds a group on its own — no UI renders or authors +one yet. `@pretable/react` ships the surface half alongside: funnels light on a +filter at any depth, and the per-column menu owns only its top-level leaf. diff --git a/.changeset/filter-tree-surface.md b/.changeset/filter-tree-surface.md new file mode 100644 index 000000000..b6bc16f14 --- /dev/null +++ b/.changeset/filter-tree-surface.md @@ -0,0 +1,39 @@ +--- +"@pretable/react": minor +--- + +The surface speaks filter trees: funnels, the column menu, and controlled +state. + +`query.filters` is now an AND/OR tree — each element is either a typed leaf or +a `{ op, children }` group, and groups nest (see `@pretable/core` for the node +type, the `isPretableFilterGroup` guard, and the empty-group rule). The +surface's chrome follows: + +- The **funnel** lights on ANY occurrence of a column, at any depth. A filter + the user built inside a group still removes their rows, so it still shows as + a filter on that column. Previously the surface kept a per-column record + projected out of the query; a group carries no `columnId`, so that record + would have collapsed every group onto the single key `undefined` and left the + funnel dark. The record is gone — the surface holds the tree verbatim. +- The **column filter menu** owns exactly its column's FIRST top-level leaf. It + hydrates from that leaf (never from one nested in a group), and a commit + replaces it in its existing slot rather than removing it and appending at the + end. Every group element passes through by reference: a menu commit cannot + edit, reorder, or drop a branch it did not author, and clearing a column + removes only its top-level leaf. Two ordering details change for a + hand-authored `filters` that carries duplicate top-level leaves for one + column — nothing the menu can produce: the menu now reads the FIRST of them + (the per-column record it replaced was last-wins), and a commit collapses + them to the single leaf it just wrote. +- **Controlled queries** take the tree shape. A controlled `query.filters` + containing groups renders funnels and filters rows exactly as the engine + evaluates it. +- `LabeledGridSurface`'s `is-filtered` header decoration walks the tree by the + same "occurrence anywhere" rule. + +`isPretableFilterGroup`, `PretableFilterGroupFor` and `PretableFilterNodeFor` +are re-exported from `@pretable/react` — a consumer reading `onQueryChange`'s +`filters` needs the guard to tell leaves from groups. + +No UI builds groups yet; nothing in this release deepens a tree on its own. diff --git a/apps/website/app/api/docs/rows/__tests__/dataset.test.ts b/apps/website/app/api/docs/rows/__tests__/dataset.test.ts index 86b8e2a60..e2239af50 100644 --- a/apps/website/app/api/docs/rows/__tests__/dataset.test.ts +++ b/apps/website/app/api/docs/rows/__tests__/dataset.test.ts @@ -457,6 +457,55 @@ describe("queries this fixture cannot answer", () => { /array of selected values/, ); }); + + /* + * On the wire `query.filters` is an AND/OR tree, and this fixture answers + * leaves only. Rejecting is the posture the server-data overview documents, + * so it is pinned here — including in the shapes where the rejection was + * NOT reached before the check moved ahead of the row loop. Each of these + * three returned 200 with zero rows, which is a result computed from less + * than the query asked for and reads to a reader as "nothing matched". + */ + const GROUP = { + op: "or", + children: [], + } as unknown as DocsQuery["filters"][number]; + + test("a filter group is rejected by name, not by a message about a column", () => { + expect(() => + applyDocsQuery(DOCS_ORDERS, { ...EMPTY_DOCS_QUERY, filters: [GROUP] }), + ).toThrow(/carried a filter group at query\.filters\[0\]/); + }); + + test("a filter group behind a leaf that matches nothing is still rejected", () => { + expect(() => + applyDocsQuery(DOCS_ORDERS, { + ...EMPTY_DOCS_QUERY, + filters: [ + { columnId: "region", operator: "isAnyOf", value: ["Nowhere"] }, + GROUP, + ], + }), + ).toThrow(/carried a filter group at query\.filters\[1\]/); + }); + + test("a filter group over no rows at all is still rejected", () => { + expect(() => + applyDocsQuery([], { ...EMPTY_DOCS_QUERY, filters: [GROUP] }), + ).toThrow(DocsQueryError); + }); + + test("but a leaf-only query over the same shapes still answers", () => { + expect( + applyDocsQuery(DOCS_ORDERS, { + ...EMPTY_DOCS_QUERY, + filters: [ + { columnId: "region", operator: "isAnyOf", value: ["North"] }, + ], + }).length, + ).toBeGreaterThan(0); + expect(applyDocsQuery([], EMPTY_DOCS_QUERY)).toEqual([]); + }); }); describe("totalFor", () => { diff --git a/apps/website/app/api/docs/rows/dataset.ts b/apps/website/app/api/docs/rows/dataset.ts index c57b8ef14..bd0eb7ff5 100644 --- a/apps/website/app/api/docs/rows/dataset.ts +++ b/apps/website/app/api/docs/rows/dataset.ts @@ -9,6 +9,39 @@ export interface DocsOrder { } export interface DocsQuery { + /** + * LEAF-ONLY, AND KNOWINGLY BEHIND THE ENGINE. `PretableQueryFor.filters` is + * an AND/OR TREE: an element is either a typed leaf or a + * `{ op, children }` GROUP, nestable. This shape admits leaves only. + * + * Nothing catches the mismatch at compile time, and that is not an + * oversight to be fixed by a cast: the type boundary is genuinely severed + * by `JSON.stringify` in each example's `fetch-rows.ts` — a query leaves the + * client as text and arrives here as `unknown`, so `pnpm typecheck` is green + * over a real gap. + * + * So the rejection is a RUNTIME one, and it is deliberate rather than + * incidental: `applyDocsQuery` scans `filters` for `children` BEFORE it + * reads a row and throws `DocsQueryError` naming the group, and the route + * answers with an error rather than with wrongly-filtered rows. + * + * Before the scan, per-row was the only check, and it was reachable only + * when a row survived the leaves ahead of it — so a leaf matching nothing, + * or an empty dataset, answered 200 with zero rows and no throw at all. See + * `applyDocsQuery` for why well-formedness is asked once, of the query. + * (Left to itself the mismatch also failed — a group has no `columnId`, so + * `columnTypeFor` threw — but about a column, which is not what went + * wrong.) + * + * Nothing in the docs builds a group yet — the built-in column menu writes + * top-level leaves only — so no example can reach this today. A server + * meeting a real tree has three honest choices (reject, flatten when every + * join is AND, or implement the recursion). This fixture REJECTS, by name, + * in `matches()`: it is a demo of the wire contract, not a filter engine, + * and implementing the recursion here would teach nothing the engine does + * not already do. The contract itself is stated on the section overview, + * `content/docs/server-data/index.mdx`. + */ filters: readonly { columnId: string; operator: string; @@ -429,10 +462,36 @@ function matchesText( } } +/** + * One wording for the one thing this fixture refuses, so the up-front scan in + * `applyDocsQuery` and the per-row branch in `matches()` cannot drift apart. + */ +function rejectFilterGroup(index?: number): never { + const where = index === undefined ? "" : ` at query.filters[${index}]`; + throw new DocsQueryError( + `This fixture answers leaf filters only, and this query carried a filter group${where}. ` + + "A server that does not implement AND/OR groups must say so rather " + + "than drop them: see /docs/server-data.", + ); +} + function matches( row: DocsOrder, filter: DocsQuery["filters"][number], ): boolean { + /* + * The rejection this fixture owes the wire contract, said out loud. On the + * wire `query.filters` is an AND/OR tree (see `DocsQuery` above), and a + * group carries `children` where a leaf carries `columnId`. + * + * Without this branch a group was already rejected — `columnTypeFor` + * throws on the missing `columnId` — but with `Unknown column + * "undefined"`, a message about the wrong thing entirely. A fixture whose + * job is to teach that the server applied the filter has to name the + * reason it did not. + */ + if ("children" in filter) rejectFilterGroup(); + const type = columnTypeFor(filter.columnId); assertUsable(filter.columnId, type, filter.operator, filter.value); @@ -459,6 +518,24 @@ export function applyDocsQuery( rows: readonly DocsOrder[], query: DocsQuery, ): DocsOrder[] { + /* + * The group rejection has to happen HERE, before a single row is read. + * `matches()` carries the same test, but it runs per row inside the loop + * below, so it is reachable only if some row survives every earlier leaf: + * `[{ region isAnyOf ["Nowhere"] }, ]` — and any query at all over an + * empty `rows` — short-circuited to zero matches and answered 200 with no + * throw. A result quietly computed from less than the reader asked for is + * the one failure these pages exist to argue against, and it does not stop + * being that because the result happens to be empty. + * + * A query is well-formed or it is not, independently of the data; the check + * belongs where that question is asked once. `matches()` keeps its branch as + * belt-and-braces for any future caller that reaches it directly. + */ + for (const [index, filter] of query.filters.entries()) { + if ("children" in filter) rejectFilterGroup(index); + } + const filtered = rows.filter((row) => query.filters.every((filter) => matches(row, filter)), ); diff --git a/apps/website/app/docs/__tests__/server-data.types.tsx b/apps/website/app/docs/__tests__/server-data.types.tsx index 6b04389e2..1a86edcf8 100644 --- a/apps/website/app/docs/__tests__/server-data.types.tsx +++ b/apps/website/app/docs/__tests__/server-data.types.tsx @@ -7,9 +7,10 @@ * fence under that heading. The preamble above the first marker is prepended to * every region, which is what lets several snippets share one import. * - * `server-data/index.mdx` is deliberately NOT bound here: both of its fences - * are JSON request and response bodies for `POST /api/docs/rows`, and there is - * no TypeScript on the page to anchor a region to. Binding it would buy two + * `server-data/index.mdx` is deliberately NOT bound here: all three of its + * fences are JSON — the request and response bodies for `POST /api/docs/rows`, + * and a grouped `filters` payload — and there is no TypeScript on the page to + * anchor a region to. Binding it would buy two * `UNTRANSCRIBED_FENCES` excuses and nothing else. The route's own shapes are * held by the app's typecheck where they are declared. */ diff --git a/apps/website/content/docs/server-data/index.mdx b/apps/website/content/docs/server-data/index.mdx index 5ff0782f7..f7aaa4ec3 100644 --- a/apps/website/content/docs/server-data/index.mdx +++ b/apps/website/content/docs/server-data/index.mdx @@ -73,10 +73,62 @@ The response is the three things it takes to describe a result — the rows, how } ``` -Two behaviors are deliberate. Every response waits 500 ms before it is sent, which is long enough that `loading` and `stale` are states you can watch rather than infer. And any filter whose value contains **fail** returns a 500, which is how the [lifecycle page](/docs/server-data/lifecycle) reaches the error phase on demand. A filter the fixture genuinely cannot answer — an unknown column, an operator its column's type cannot use, a missing operand — also returns a 500 with a message saying which, rather than quietly returning every row. +Two behaviors are deliberate. Every response waits 500 ms before it is sent, which is long enough that `loading` and `stale` are states you can watch rather than infer. And any filter whose value contains **fail** returns a 500, which is how the [lifecycle page](/docs/server-data/lifecycle) reaches the error phase on demand. A filter the fixture genuinely cannot answer — an unknown column, an operator its column's type cannot use, a missing operand, or an AND/OR group — also returns a 500 with a message saying which, rather than quietly returning every row. That last point is a rule to copy, not a fixture quirk: a backend that ignores a filter it does not understand produces a grid that looks filtered and is not. +## What a filter looks like on the wire + +`query.filters` is an array, and it always was. What changed is what an element of it may be: either a **leaf** — the `{ columnId, operator, value }` shape in the request above — or a **group**, `{ "op": "and" | "or", "children": [...] }`, whose children are themselves leaves or groups, to any depth. `filters` is a tree, and it reaches `onQueryChange` and then your endpoint exactly as the grid built it. Nothing flattens, rewrites, or simplifies it on the way out, and [external filter authority](/docs/server-data/query-ownership#what-external-authority-suppresses) does not either — suppression decides what the engine _applies_, never what it _reports_. + +```json +{ + "query": { + "filters": [ + { "columnId": "total", "operator": "gt", "value": 500 }, + { + "op": "or", + "children": [ + { "columnId": "region", "operator": "isAnyOf", "value": ["North"] }, + { "columnId": "customer", "operator": "contains", "value": "Labs" } + ] + } + ], + "sort": [], + "rowGroups": [] + } +} +``` + +That payload reads `total > 500 AND (region is North OR customer contains "Labs")`, and the four rules that make it mean that are the contract. + +**The top-level array is an implicit AND.** It is what a list of filters has always meant — each entry narrows the result further — so groups became _elements_ of that array rather than a new field beside it, and the ordinary one-leaf-per-column case stays the flat list it was. Two consequences follow from the same rule. A payload written before groups existed is still a correct payload. And `"filters": []` constrains nothing, because an AND over no conditions excludes no rows. + +**Leaves and groups discriminate on structure, not on a tag.** There is no `kind` field to switch on: a group is the node carrying `op` and `children`, a leaf is the node carrying `columnId` and `operator`. On the client edge, `isPretableFilterGroup` — exported from both `@pretable/core` and `@pretable/react` — makes that test and narrows `PretableFilterNodeFor` to `PretableFilterGroupFor`, so nothing has to hand-roll it. On the server there are no types left to narrow: the query arrived as JSON over HTTP, so write the test yourself, and test for `children`. That is the field a group cannot exist without, and the field a leaf never has. + +**An empty group matches every row, under either `op`.** Naive boolean algebra says an empty `or` is false, and that is exactly the wrong answer here: a group with nothing in it is a group someone is part-way through building, and a half-built condition that blanks the grid mid-edit is a bug the reader will read as data loss. So an empty group constrains nothing whichever way it joins — the same answer an empty top-level array gives, for the same reason. Copy that rule into your backend rather than deriving it, or the two sides will disagree about a query the grid considers unfiltered. + +**Nesting is bounded at 64 levels.** A tree deeper than that is rejected with the same typed `invalid-query` error an unknown column gets, and the message breadcrumbs the offending node — `query.filters[0].children[3].children[1]` — so you are told where, not just that. The bound exists because every consumer of a captured query recurses over it, and it sits far above any tree a person or a builder UI produces and far below the depth at which any of that recursion is at risk. In practice the grid rejects a too-deep tree before it can publish one, so your endpoint should never see it; bound your own recursion anyway, since a query can also arrive from a saved view, a URL, or a client that is not this grid. + +### A server that only understands flat filters has to decide + +It cannot be left implicit, because the failure mode of guessing is the one this page keeps warning about: a grid that looks filtered and is not. Three answers are defensible, and which one is right is a property of your backend, not of the grid. + +- **Reject.** Answer with an error the moment a group appears, naming it. Cheapest by far, correct at once, and it fails where a reader can see it — an error strip over the rows they had, rather than a result quietly computed from half of what they asked for. This is what the fixture endpoint above does: `applyDocsQuery` in `app/api/docs/rows/dataset.ts` scans `filters` for `children` before it reads a single row, and returns a 500 that says so. Scanning up front rather than inside the row predicate is the part worth copying — a per-row check is reachable only if some row survives the leaves ahead of it, so a leaf matching nothing would have answered an empty result instead of an error. Whether a query is one you can answer is a question about the query, and it is asked once. It is the right posture for a demo, and the right first commit for a real backend too, because it buys you the freedom to implement groups later without having shipped a wrong answer in the meantime. +- **Flatten — but only when every join is `and`.** A tree whose groups all carry `"op": "and"` is genuinely equivalent to the flat list of its leaves, nesting and all, so collecting them loses nothing. The trap is that this is only true until the first `"op": "or"`, and an `or` is precisely what a user reaches for a group to express. So the flattening has to be _conditional_, and its else-branch has to be reject, never best-effort: a tree containing an `or` cannot be approximated by an AND of its leaves in either direction. Note that an empty group contributes no leaves, which is the correct reading of the rule above. +- **Implement the recursion.** It is smaller than it sounds — map a leaf to a predicate as you already do, join a group's children with `AND` or `OR`, parenthesize each group, and return the identity `TRUE` for an empty one. The work you actually owe is the parameter binding you owe leaves anyway, over a shape that now nests; a tree of user-supplied operators and operands assembled into SQL by string concatenation is an injection hole whatever its depth. + + + No UI builds a group yet. The header funnel writes, edits, and removes its own + column's top-level leaf, exactly as it always has; the tool panel's filter + builder is a later sub-project. Until it ships, a group can only reach your + endpoint from a query you put there yourself — seeded through the controlled + `query` prop, restored from a URL, or loaded from a saved view. The contract + is documented now rather than alongside that UI because a shape a server can + receive is a contract from the moment it is possible, not from the moment it + is common. + + ## Where to go next - [Query ownership](/docs/server-data/query-ownership) — the `processing` and `query`/`onQueryChange` contract, what external filtering suppresses, and what it deliberately does not. diff --git a/apps/website/content/docs/server-data/query-ownership.mdx b/apps/website/content/docs/server-data/query-ownership.mdx index d3501e4fb..34a94a69d 100644 --- a/apps/website/content/docs/server-data/query-ownership.mdx +++ b/apps/website/content/docs/server-data/query-ownership.mdx @@ -37,7 +37,7 @@ The grid says so when it can prove it: external `filter`, engine `sort`, and an `filter: "external"` is not a hint. In rows mode the surface hands the authority to the row model it owns, and the compiled query plan keeps two versions of the query: the one it **reports** and the one it **applies**. Under external filtering the reported one keeps your filters and the applied one has none; under external sorting the same split holds for `query.sort`. `get query()` — and through it the snapshot, the funnel menu, and `onQueryChange` — reads the reported version; row evaluation reads the applied one. So the funnel still shows the filter you set, the callback still hands it to you, the header still reports `aria-sort` for the column you sorted, and the engine simply stops re-selecting and re-ordering the records you were given. -Suppression changes what is applied, never what is reported. +Suppression changes what is applied, never what is reported. Shape included: `query.filters` is an AND/OR tree, so a group nested inside it is published through `onQueryChange` exactly as the grid holds it, under either authority — [what a filter looks like on the wire](/docs/server-data#what-a-filter-looks-like-on-the-wire) is what your endpoint receives, and what it owes an answer to. That distinction costs nothing while the rows and the query agree — the server already filtered by that query, so filtering its answer again removes nothing — and it is the whole point the moment they **disagree**, which is exactly what `stale` and `error` are: a query the reader has moved on to, and rows that answer a different one. [The lifecycle](/docs/server-data/lifecycle) deliberately keeps the previous result on screen while the next one loads, and re-applying the new filter to the old rows would empty a body that still holds a perfectly readable result. @@ -112,4 +112,5 @@ Two are surface-only, for different reasons. `query` is absent from `` - [Server-side data](/docs/server-data) — the section overview, its endpoint, and what the grid keeps owning. - [Loading, staleness, errors](/docs/server-data/lifecycle) — the six `dataState` phases, and why a failure never discards rows. +- [The filter wire contract](/docs/server-data#what-a-filter-looks-like-on-the-wire) — the filter payload's tree shape, and the three things a server that only understands flat filters can do about it. - [Filtering](/docs/grid/filtering) — operator semantics, the funnel menu, and the controlled-query idiom in local form. diff --git a/apps/website/content/examples/column-filters/ColumnFiltersGrid.tsx b/apps/website/content/examples/column-filters/ColumnFiltersGrid.tsx index 20d5c397f..5bbbd642d 100644 --- a/apps/website/content/examples/column-filters/ColumnFiltersGrid.tsx +++ b/apps/website/content/examples/column-filters/ColumnFiltersGrid.tsx @@ -2,7 +2,7 @@ import { useState, type ComponentProps } from "react"; -import { PretableSurface } from "@pretable/react"; +import { isPretableFilterGroup, PretableSurface } from "@pretable/react"; import { columns } from "./columns"; import { type Order, orders } from "./data"; @@ -50,7 +50,16 @@ export function ColumnFiltersGrid() { {query.filters.length > 0 ? query.filters - .map((filter) => `${filter.columnId} ${filter.operator}`) + // `filters` is an AND/OR tree: an element is either a typed + // leaf or a group of nodes. The built-in column menu only ever + // writes top-level leaves, so no group appears here — but the + // type says one can, and `isPretableFilterGroup` is how a + // consumer tells them apart. + .map((filter) => + isPretableFilterGroup(filter) + ? `(${filter.op} group)` + : `${filter.columnId} ${filter.operator}`, + ) .join(" · ") : "(none)"} diff --git a/docs/superpowers/plans/2026-08-25-filter-tree-sp2a.md b/docs/superpowers/plans/2026-08-25-filter-tree-sp2a.md new file mode 100644 index 000000000..c930a97b2 --- /dev/null +++ b/docs/superpowers/plans/2026-08-25-filter-tree-sp2a.md @@ -0,0 +1,157 @@ +# Filter Tree (SP2a) Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Filters become an arbitrary-depth tree of AND/OR groups over the existing typed leaves — the engine prerequisite for the tool panel's filter builder (SP2b). + +**Architecture:** The row-model query is already the single source of truth for filters (grid-core holds no filter state; the surface's per-column record is a derived projection). A recursive group node joins the filter union; capture, evaluation, and equality in `compiled-query.ts` recurse; the surface's projection, funnel, and FilterMenu write path become tree-aware. No UI renders groups in SP2a. + +**Tech Stack:** TypeScript, vitest, API Extractor, changesets. No CSS, no new components. + +**Spec:** `docs/superpowers/specs/2026-08-25-filter-tree-design.md` — decisions there are settled: arbitrary nesting; `filters` stays an array with groups as elements (implicit top-level AND); **empty group evaluates TRUE regardless of `op`**; funnel lights on any occurrence; menu owns only its column's top-level leaf. + +--- + +## Ground truth (verified, 2026-08-25 — line numbers may drift, anchors won't) + +- `packages/row-model/src/column-types.ts:566` — `PretableQueryFor.filters: readonly PretableFilterFor[]`; `PretableFilterFor` is the big distributive conditional at `:470-530`. +- `packages/row-model/src/compiled-query.ts`: + - `:251` `RuntimeFilter { columnId, operator, value? }`; `:263` `RuntimeQuery`. + - `:572` `captureFilter(raw, index)` — validates and freezes each incoming leaf, `fail()`s with a `query.filters[i]` path. + - `:1491` evaluation: `this.#runtimeQuery.filters.every((filter) => …)`. + - `:906` `queryEqual` → `:914` `filtersEqual` — **order-insensitive multiset** match (used-set + findIndex). AND/OR are commutative, so order-insensitivity stays correct per level under recursion. + - `:150/:1170` `filterAuthority === "external"` gates evaluation — the tree does not touch this seam. +- `packages/react/src/pretable-surface.tsx`: + - `:2180-2190` builds the derived `Record` **from `rowModelSnapshot.query.filters`, with a cast that assumes every element is a leaf** (`entry.columnId`) — a group element would today land under key `undefined`. This is the projection that becomes tree-aware. + - `:403` the controlled `state.filters: Readonly>`. + - `:5848` funnel: `active={Boolean(snapshot.filters[column.id])}`; `:6616` menu reads `snapshot.filters[columnId]`. + - The menu's commit path goes through the surface's query write (find it from `:6616`'s handler; it rebuilds the filters array and calls the model's `setQuery` path). +- `ColumnFilter` (`packages/grid-core/src/types.ts:124`) is `{ operator, value? }` — **column-id-less**; the record's key carries the id. Grid-core has zero filter state or methods; nothing in grid-core changes in SP2a. +- Server-side docs live under `apps/website/content/docs/server-data/`. + +## File map + +| File | Responsibility | +|---|---| +| `packages/row-model/src/column-types.ts` | `PretableFilterGroupFor`, widen `PretableQueryFor.filters`, `isPretableFilterGroup` guard | +| `packages/row-model/src/compiled-query.ts` | recursive capture / evaluation / equality | +| `packages/row-model/src/__tests__/filter-tree.test.ts` | new — semantics suite | +| row-model's existing type-tests file (find: `ls packages/row-model/src/__tests__/*type*`) | `IsNever` probes | +| `packages/react/src/pretable-surface.tsx` | snapshot projection, funnel, menu read/write, `state.filters` shape | +| `packages/react/src/__tests__/` | extend the filter/controlled-state suites | +| `apps/website/content/docs/server-data/` filter page | wire contract | +| `packages/core` / `packages/react` `.api.md` + `.changeset/` | reports, changesets | + +## Standing rules + +- TDD; prettier before trusting a test; mutation-check every guard-like assertion; `pnpm build` before `pnpm api`; drawn-order/`getColumns()` rules don't apply here but the audit discipline does; subscribe to snapshots, never `getState`, in anything reactive; no stash/checkout with uncommitted work — restore mutations by targeted edit. +- **Pre-1.0, no compatibility aliases** — but "array of leaves keeps compiling" is a design requirement, not backcompat courtesy: verify it with an untouched existing test. + +--- + +### Task 1: row-model — the group type, guard, and capture + +**Files:** `packages/row-model/src/column-types.ts`, `packages/row-model/src/compiled-query.ts`, create `packages/row-model/src/__tests__/filter-tree.test.ts`, extend the row-model type-tests + +- [ ] **Step 1: Failing type probes first.** In the row-model type-test file, following its existing `IsNever`/assertion idiom: + - `PretableFilterGroupFor` is not `never` for a representative column set; + - `PretableQueryFor["filters"][number]` accepts both a leaf and a group (assignability probes both directions); + - a group with a misspelled `op: "nor"` is rejected; + - **the pre-existing leaf-only probes stay untouched and green** — that is the "array of leaves keeps compiling" requirement, checked by not editing them. +- [ ] **Step 2: Failing runtime tests** in `filter-tree.test.ts` (copy the harness of the nearest compiled-query test): + - `isPretableFilterGroup` true for `{op:"and",children:[]}`, false for every leaf shape (probe each operator family: text, number-between, date, enum-set, isEmpty); + - capture: a query whose filters include a nested group round-trips into the compiled query without throwing; `op: "xor"` fails with a `query.filters[1].op` path; a group whose `children` is not an array fails with its path; deep-frozen output (mutating a nested child throws in strict mode). +- [ ] **Step 3: Run both, confirm failures for the right reasons.** +- [ ] **Step 4: Implement.** + - `column-types.ts`: the interface exactly as the spec writes it; the guard narrow and total: + +```ts +/** @public */ +export function isPretableFilterGroup( + node: PretableFilterFor | PretableFilterGroupFor, +): node is PretableFilterGroupFor { + // Structural: groups carry `op` + `children`; every leaf carries `operator`. + // Checked positively on the group's fields so an unknown shape fails closed. + return ( + typeof node === "object" && + node !== null && + "children" in node && + ("op" in node + ? (node as { op: unknown }).op === "and" || + (node as { op: unknown }).op === "or" + : false) + ); +} +``` + + - `compiled-query.ts`: `RuntimeFilterNode = RuntimeFilter | RuntimeFilterGroup { op, children }`; `captureFilter` becomes the leaf half of a recursive `captureFilterNode(raw, path)` that validates `op` ∈ {and, or}, requires an array `children`, recurses with `path.children[i]`, and freezes each level. Keep `fail()`'s message voice. +- [ ] **Step 5:** Tests green; whole package: `pnpm --filter @pretable-internal/row-model test` (check the actual package name in its package.json first). Prettier. +- [ ] **Step 6: Commit** `feat(row-model): filter groups — the type, the guard, and capture`. + +### Task 2: recursion — evaluation and equality + +**Files:** `packages/row-model/src/compiled-query.ts`, extend `filter-tree.test.ts` + +- [ ] **Step 1: Failing tests.** Fixtures must be able to disprove (the repo rule — an OR fixture whose expected rows differ from the same tree under AND): + - rows `[{n:1},{n:5},{n:9}]`, tree `[{gt 4} , {op:"or", children:[{lt 2},{gt 8}]}]` → rows 9 only; same tree with the group's op flipped to `and` → no rows. Both asserted, so a connective mix-up cannot pass. + - nesting three deep evaluates correctly (compose the above inside another `or`). + - **empty group ⇒ TRUE for both ops** — `{op:"or",children:[]}` alongside a real leaf filters exactly as the leaf alone; same for `and`. (This is the spec's product-safety convention; the naive algebra would say empty-OR ⇒ false.) + - short-circuit is NOT observable behavior — do not test call counts; outcomes only. + - equality both directions: two trees equal up to sibling permutation at each level ARE equal (no recompile); trees differing only in a nested leaf's value are NOT equal. Assert through whatever the compiled query exposes for plan reuse (find how existing tests observe recompile-vs-reuse — follow that mechanism, not internals). +- [ ] **Step 2: Confirm failures.** +- [ ] **Step 3: Implement.** `:1491` → `evaluateFilterNode(node, row)`: leaf → existing single-filter evaluation unchanged; group → `op === "and" ? children.every : children.length === 0 ? true : children.some` (write the empty-OR case explicitly with the WHY comment — `some` on empty already returns false, which is precisely the wrong answer here). `filtersEqual` → `filterNodesEqual`: same used-set multiset shape, recursing when both sides are groups (`op` must match; children compared as an order-insensitive multiset per level). +- [ ] **Step 4:** Green; full package; prettier. **Mutation round, all restored by targeted edit:** flip `every`/`some` → the OR/AND twin tests fail; delete the empty-group special case → the empty-OR test fails; make `filterNodesEqual` ignore `op` → the equality test fails. +- [ ] **Step 5: Commit** `feat(row-model): filter trees evaluate and compare recursively`. + +### Task 3: API reports and the core changeset + +> **ORDERING CORRECTION (2026-08-25, after Task 1):** widening `PretableQueryFor.filters` +> breaks `@pretable/react`'s typecheck at its leaf-assuming sites — exactly the flush-out +> the plan predicted, but it means **`pnpm build` cannot succeed until Task 4 lands**, and +> this task depends on a successful build. **Execute Task 4 BEFORE Task 3**, and let Task 3 +> then cover both packages' reports in one pass instead of regenerating twice. + + +- [ ] **Step 1:** `pnpm build && pnpm api && pnpm api:check`. Expected surfacing: `PretableFilterGroupFor`, `isPretableFilterGroup`, the widened `filters` element type — in `core.api.md` and `react.api.md` (the query types flow through both). Anything else surfacing is a stop-and-report. +- [ ] **Step 2:** Changeset `@pretable/core` minor: the group node, the guard, implicit-AND array preserved, empty-group-TRUE semantics named (a CHANGELOG reader must learn that rule here). +- [ ] **Step 3: Commit** `chore: api reports and changeset for filter groups`. + +### Task 4: react — projection, funnel, menu, controlled state + +**Files:** `packages/react/src/pretable-surface.tsx`, extend the surface filter/controlled-state test files + +The audit is the heart of this task. `grep -n "query.filters\|state.filters\|snapshot.filters" packages/react/src apps/website apps/bench --include="*.ts*" -r` — every site gets a verdict (tree-aware / leaf-only-by-design / display-only), recorded as a code comment where non-obvious. Known sites: the projection (`:2180` — its leaf-assuming cast is the bug-in-waiting), funnel (`:5848`), menu read (`:6616`), menu commit, controlled `state.filters` (`:403`), CSV/export omissions, bench adapters, docs examples. + +- [ ] **Step 1: Failing tests:** + - **snapshot shape**: `snapshot.filters` becomes the query's array verbatim (leaves carry `columnId`; groups nest). Existing consumers of the old record shape inside the repo are part of this task's churn — the type change finds them. + - **funnel-anywhere**: a filter on column `a` buried two groups deep lights `a`'s funnel; no top-level leaf needed. (New recursive helper `columnHasFilter(filters, columnId)` — pure, exported from a surface-adjacent module or local, unit-tested directly.) + - **menu reads only its top-level leaf**: with `a` filtered both at top level and inside a group, the menu shows the top-level leaf's operator/value. + - **menu write splices, groups survive** (the survives-test): commit a new filter from the menu for `a` while a group mentioning `a` and `b` exists → the group element is byte-identical in the resulting query; clearing `a` from the menu removes only the top-level leaf. + - **controlled `state.filters`** takes the array shape; a controlled tree renders funnels and filters rows (jsdom: assert visible row count through the model, the way controlled-query tests do). +- [ ] **Step 2:** Failures confirmed. **Step 3:** Implement — projection passes the array through (delete the record-building loop), funnel uses the helper, menu read/write scoped to top-level leaves, `state.filters` type + application path updated, audit verdicts written. +- [ ] **Step 4:** Full `pnpm --filter @pretable/react test` and `pnpm --filter @pretable/app-website test` (docs examples may consume `snapshot.filters` — expected churn belongs here). Prettier. +- [ ] **Step 5:** `pnpm build && pnpm api && pnpm api:check` (react.api.md moves for the snapshot/state types); react changeset (minor) written now. +- [ ] **Step 6: Commit** `feat(react): the surface speaks filter trees — funnels, menu, controlled state`. + +### Task 5: the wire contract docs + +**Files:** the filter page under `apps/website/content/docs/server-data/` (find the one documenting `onQueryChange`'s filter payload) + +- [ ] **Step 1:** Read the page and the docs guard's current reach (`apps/website/lib/docs/__tests__/docs-api-surface.test.ts` — it checks fenced imports, prose `Pretable*` identifiers, and registered tables; it was hardened four rounds, assume it sees more than you expect). +- [ ] **Step 2:** Document: groups arrive verbatim in `query.filters`; leaves vs groups discriminate on `op`/`children` (name `isPretableFilterGroup` for consumers on the client edge); a server that only understands flat filters must decide explicitly (reject, flatten-if-all-AND, or implement) — present the three honestly; empty-group-TRUE is part of the contract. +- [ ] **Step 3:** `pnpm --filter @pretable/app-website test -- docs-api-surface` green (register any new table). Full website suite. Prettier (markdown tables). +- [ ] **Step 4: Commit** `docs(server-data): the filter wire contract grows groups`. + +### Task 6: final battery + +- [ ] All package suites (row-model, core, react, ui, website) — real counts; typecheck, lint, `pnpm format`. +- [ ] `pnpm build && pnpm api && pnpm api:check`; `git status` clean of stale reports. +- [ ] Website e2e, FULL suite, production build, root playwright binary from inside `apps/website`, `--workers=1` (the cockpit filter smoke exercises the menu path end-to-end). +- [ ] Re-verify both changesets against what actually shipped. +- [ ] Confirm the audit table is complete: re-run the grep, zero unverdicted sites. + +## Self-review + +**Spec coverage:** type+guard (T1), evaluation+equality+empty-group (T2), reports/changesets (T3), surface+funnel+menu+state (T4), wire docs (T5), audit (T4+T6), verification (T6). External authority needs no task — the seam is evaluation-side and T2's tests run under default authority; T4's audit confirms no authority-path reader assumes leaves. + +**Judgment calls made here, flagged:** (1) `snapshot.filters` becomes the query array *verbatim* rather than keeping a parallel per-column record for chrome — one vocabulary, and the funnel helper is cheaper than maintaining a projection that lies by omission; (2) the menu's write path is defined by splice-preserving-groups rather than rebuild-from-record — the survives-test enforces it. diff --git a/docs/superpowers/specs/2026-08-25-filter-tree-design.md b/docs/superpowers/specs/2026-08-25-filter-tree-design.md new file mode 100644 index 000000000..956d56787 --- /dev/null +++ b/docs/superpowers/specs/2026-08-25-filter-tree-design.md @@ -0,0 +1,136 @@ +# Filter Tree (Tool Panel SP2a) — Design + +**Status:** approved direction; SP2a specced in full, SP2b (the builder UI) outlined. +**Parent:** `docs/superpowers/specs/2026-08-24-tool-panel-design.md` (SP2 of the tool panel). + +## What this is + +Boolean filter composition for the engine: filters become an **arbitrary-depth +tree** of AND/OR groups over the existing typed leaves, so a query can express +`price > 10 AND (status isAnyOf [a,b] OR owner contains "x")`. This is the +engine prerequisite for SP2b, the tool panel's filter builder section — the +same engine-first split SP1 used for column visibility, because the UI must +compose against a model that exists. + +## Decisions locked (and why) + +1. **Full OR groups, not multi-filter AND.** Chosen over the cheaper options + (one-per-column AND; flat multi-filter AND) — the builder's value is + expressing what the per-column menu cannot. +2. **Engine first (SP2a), builder second (SP2b).** A combined PR would span + row-model → core → react → ui → docs; SP1 was 30 commits _without_ an + engine rewrite. Each half ships and is testable alone. +3. **Arbitrary nesting.** The engine cost over a fixed two-level shape is + recursion versus a loop; the recursive type is cleaner than a special-cased + one; and SP2b may still _render_ a bounded depth while the model keeps the + full algebra. Deciding expressiveness once beats a pre-1.0 rework later. +4. **`filters` stays an array; groups are elements.** + `filters: readonly (Leaf | Group)[]`, top level an implicit AND — every + existing call site keeps compiling and the simple case stays one-liner + simple. Rejected: a single root group (ceremony on every simple consumer + forever) and a parallel `filterTree` field (two sources of truth — the + declared-but-read-by-nothing failure mode this repo keeps paying for). + +## The type + +```ts +/** @public */ +export interface PretableFilterGroupFor { + readonly op: "and" | "or"; + readonly children: readonly ( + PretableFilterFor | PretableFilterGroupFor + )[]; +} + +// PretableQueryFor.filters: +// readonly (PretableFilterFor | PretableFilterGroupFor)[] +``` + +- Leaves and groups discriminate structurally (`columnId`+`operator` vs + `op`+`children`). A public runtime guard `isPretableFilterGroup` ships so + consumers never hand-roll the discrimination; internal code uses the same + guard. +- **Type-level trap, named:** `PretableFilterFor` is a large distributive + conditional type. Extending the union around it must be probed with the + repo's `IsNever` discipline — a conditional that collapses to `never` + compiles every downstream guard while checking nothing, and this exact + failure shipped once before. + +## Evaluation semantics + +- Top-level array: AND over elements (unchanged behavior for all-leaf arrays). +- Group: `and` → `children.every`, `or` → `children.some`, short-circuiting, + recursion for nested groups. +- Leaf evaluation is untouched — the existing compiled `RuntimeFilter` path is + reused; the tree changes combination only, never leaf semantics. +- **Empty group ⇒ TRUE, regardless of `op`.** The naive algebra says empty-OR + is false, but that would let a half-built group in SP2b's UI blank the whole + grid mid-edit. Identity-true is the product-safe convention; it is stated in + the TSDoc and pinned by a test. An all-empty tree therefore filters nothing, + exactly like `filters: []` today. +- The query-equality comparison that gates recompiles (currently a flat + `every` over leaves) learns deep structural tree comparison. Getting this + wrong in the "always unequal" direction recompiles per publish; in the + "always equal" direction it never recompiles — both directions get tests. + +## Surface and controlled state + +- `state.filters` changes shape from `Record` (one per + column) to the query's own array-of-leaf-or-group — one vocabulary + everywhere, no projection layer. Pre-1.0, no compatibility aliases. +- **Header funnel semantics, scoped honestly:** + - The per-column FilterMenu **creates / edits / removes its column's + top-level leaf** — its exact job today, unchanged in capability. + - The funnel **lights when its column appears anywhere in the tree** + (recursive scan). + - A column filtered only inside groups shows a lit funnel and a menu with no + editable leaf; the explanatory "also filtered in advanced groups" line in + the menu belongs to SP2b, when a builder exists to point at. +- If the menu's write path today replaces the whole per-column record, it now + splices only the top-level leaf and must not disturb group elements — a + survives-test (assert the old behavior AND the group's integrity). + +## External authority and the wire + +- `filter: "external"` suppresses local evaluation exactly as today — + suppression happens at evaluation, and the tree does not change that seam. +- The tree flows through `onQueryChange` verbatim. **The server-side data docs' + filter contract updates in SP2a** (not SP2b), because the wire shape a + server can receive changes the moment this merges. + +## The audit (SP1's discipline) + +Every reader of `query.filters` / `state.filters` gets a recorded verdict — +recursive-aware, flat-assuming (fix), or display-only. Known candidates, to be +completed by grep during planning: the funnel projection, the filter-count / +post-filter row-count paths, CSV/export omission reporting, copy, +announcements, bench adapters, docs examples, the headless docs example. + +## Verification + +- row-model unit tests: evaluation (AND/OR/nested/empty-group), with fixtures + that can disprove — an OR fixture whose expected rows differ from the same + tree with AND, so a connective mix-up cannot pass. +- Deep-equality tests in both failure directions (never-equal / always-equal). +- Type-level `IsNever` probes on the extended union; api reports regenerate + (core + react move; build before `pnpm api`). +- FilterMenu write-path survives-tests; funnel-anywhere tests. +- Docs guard: any new table registered; the server-side filter page's wire + examples must keep passing the fence/import checks. +- Changesets: core minor (or row-model as versioning dictates), react minor. + +## Out of scope for SP2a + +The builder UI (SP2b), any rendering of groups anywhere, the FilterMenu's +"advanced groups" note, NOT/negation as a group op (leaves already carry +negated operators: `notContains`, `isNoneOf`, `notEquals` — a group-level NOT +is redundant today and can be added compatibly if SP2b finds a need), saved +views, and the section-strings i18n pass (SP2b, alongside its new strings). + +## SP2b outline (for continuity, not specced here) + +The tool panel's filter section renders the tree — likely with a rendered +depth cap over the full underlying algebra (its call); typed value editors +reused from the shipped cell-editor set; add/remove/re-op/regroup +interactions; the i18n pass over section strings; the shared menu-keyboard +extraction that the third `role="menu"` triggers. diff --git a/packages/core/core.api.md b/packages/core/core.api.md index 44b37e9bc..be69493b3 100644 --- a/packages/core/core.api.md +++ b/packages/core/core.api.md @@ -165,6 +165,9 @@ export type FilterValue = string | number | readonly [number, number] | readonly // @public export const GROUP_COLUMN_ID = "__pretable_group__"; +// @public +export function isPretableFilterGroup(node: PretableFilterNodeFor): node is PretableFilterGroupFor; + // @public export const numberFormats: { readonly money: (options: PretableCurrencyFormatOptions) => Intl.NumberFormatOptions; @@ -671,6 +674,17 @@ export type PretableFilterFor = TColumns extends readonly (infer TColu readonly value: PretableFilterOperandFor; }) : never : never; +// @public +export interface PretableFilterGroupFor { + // (undocumented) + readonly children: readonly PretableFilterNodeFor[]; + // (undocumented) + readonly op: "and" | "or"; +} + +// @public +export type PretableFilterNodeFor = PretableFilterFor | PretableFilterGroupFor; + // @public export type PretableFilterOperandFor = TType extends "text" ? string : TType extends "number" ? number : TType extends "date" ? string | number | Date : TType extends "boolean" ? boolean : [Extract, string>] extends [never] ? string : Extract, string>; @@ -1014,7 +1028,7 @@ export interface PretableProcessingOptions { // @public (undocumented) export interface PretableQueryFor { // (undocumented) - readonly filters: readonly PretableFilterFor[]; + readonly filters: readonly PretableFilterNodeFor[]; // (undocumented) readonly rowGroups: readonly PretableRowGroupFor[]; // (undocumented) diff --git a/packages/core/src/public_api.ts b/packages/core/src/public_api.ts index 68069be17..7bb3a52e9 100644 --- a/packages/core/src/public_api.ts +++ b/packages/core/src/public_api.ts @@ -11,6 +11,7 @@ export { createColumnHelper } from "./create-column-helper"; export { createLocalRowModel } from "./create-local-row-model"; export { numberFormats } from "./number-formats"; export type { PretableCurrencyFormatOptions } from "./number-formats"; +export { isPretableFilterGroup } from "@pretable-internal/row-model"; export { PretableDisposedModelError, PretableInvalidGroupKeyError, @@ -76,6 +77,8 @@ export type { PretableExpansionDefault, PretableExpansionState, PretableFilterFor, + PretableFilterGroupFor, + PretableFilterNodeFor, PretableFilterOperandFor, PretableFocusDirection, PretableFocusState, diff --git a/packages/core/src/types.ts b/packages/core/src/types.ts index ff293ff70..206b2a0a2 100644 --- a/packages/core/src/types.ts +++ b/packages/core/src/types.ts @@ -84,6 +84,8 @@ export type { PretableExpansionDefault, PretableExpansionState, PretableFilterFor, + PretableFilterGroupFor, + PretableFilterNodeFor, PretableFilterOperandFor, PretableFormatInput, PretableGroupId, diff --git a/packages/react/react.api.md b/packages/react/react.api.md index a242282cb..3e2cb76b7 100644 --- a/packages/react/react.api.md +++ b/packages/react/react.api.md @@ -195,6 +195,9 @@ export type FilterOperator = "contains" | "notContains" | "equals" | "notEquals" // @public (undocumented) export type FilterValue = string | number | readonly [number, number] | readonly [string, string] | readonly string[] | null; +// @public +export function isPretableFilterGroup(node: PretableFilterNodeFor): node is PretableFilterGroupFor; + // @beta export function LabeledGridSurface = TColumns extends readonly (infer TColu readonly value: PretableFilterOperandFor; }) : never : never; +// @public +export interface PretableFilterGroupFor { + // (undocumented) + readonly children: readonly PretableFilterNodeFor[]; + // (undocumented) + readonly op: "and" | "or"; +} + +// @public +export type PretableFilterNodeFor = PretableFilterFor | PretableFilterGroupFor; + // @public export type PretableFilterOperandFor = TType extends "text" ? string : TType extends "number" ? number : TType extends "date" ? string | number | Date : TType extends "boolean" ? boolean : [Extract, string>] extends [never] ? string : Extract, string>; @@ -1603,7 +1617,7 @@ export type PretableProps { // (undocumented) - readonly filters: readonly PretableFilterFor[]; + readonly filters: readonly PretableFilterNodeFor[]; // (undocumented) readonly rowGroups: readonly PretableRowGroupFor[]; // (undocumented) diff --git a/packages/react/src/__tests__/filter-menu-surface.test.tsx b/packages/react/src/__tests__/filter-menu-surface.test.tsx index 9e1b18430..3d492edde 100644 --- a/packages/react/src/__tests__/filter-menu-surface.test.tsx +++ b/packages/react/src/__tests__/filter-menu-surface.test.tsx @@ -10,7 +10,7 @@ import { import * as React from "react"; import { afterEach, describe, expect, it, vi } from "vitest"; -import type { ColumnFilter } from "@pretable/core"; +import { isPretableFilterGroup, type ColumnFilter } from "@pretable/core"; import { PretableSurface, type PretableSurfaceProps, @@ -98,15 +98,25 @@ function renderSurface(extra: TestOptions = {}) { onQueryChange={(next) => { if (controlledFilters === undefined) setQuery(next as typeof query); extra.onSortChange?.(next.sort); + // This harness is LEAF-ONLY BY DESIGN: the suites below it drive the + // per-column menu, which only ever writes top-level leaves. The + // tree-shaped cases live in the "filter trees" describe at the foot + // of this file and read `next.filters` unprojected. extra.onFiltersChange?.( Object.fromEntries( - next.filters.map((filter) => [ - filter.columnId, - { - operator: filter.operator, - ...("value" in filter ? { value: filter.value } : {}), - }, - ]), + next.filters.flatMap((filter) => + isPretableFilterGroup(filter) + ? [] + : [ + [ + filter.columnId, + { + operator: filter.operator, + ...("value" in filter ? { value: filter.value } : {}), + }, + ] as const, + ], + ), ), ); }} @@ -436,3 +446,209 @@ describe("PretableSurface — built-in filter funnel", () => { ); }); }); + +/** + * SP2a: `query.filters` is an AND/OR TREE — each top-level element is either a + * typed leaf or a group, and groups nest. These pin the three surface + * behaviors the tree changes: the funnel lights on ANY occurrence of a column, + * the menu owns only that column's TOP-LEVEL leaf, and a menu commit must + * leave every group element it did not author untouched. + */ +describe("PretableSurface — filter trees", () => { + type TreeNode = Record; + + function renderTreeSurface( + initialFilters: readonly TreeNode[], + extra: { + onQueryChange?: (query: { filters: readonly TreeNode[] }) => void; + onGridReady?: PretableSurfaceProps["onGridReady"]; + } = {}, + ) { + function Harness() { + const [query, setQuery] = React.useState(() => ({ + filters: initialFilters, + sort: [], + rowGroups: [], + })); + return ( + + ariaLabel="Bug grid" + columns={columns} + getRowId={getRowId} + onGridReady={extra.onGridReady} + overscan={0} + rows={rows} + query={query as never} + onQueryChange={(next) => { + setQuery(next as never); + extra.onQueryChange?.(next as never); + }} + viewportHeight={300} + /> + ); + } + return render(); + } + + const renderedRowIds = (view: ReturnType) => + view + .getAllByTestId("pretable-row") + .map((r) => r.getAttribute("data-pretable-row-id")); + + it("carries a controlled tree to the model verbatim and joins it with OR", async () => { + let grid: Parameters>[0] | null = + null; + // Chosen so OR and AND disagree: under `or` this is b1 (count 3 > 2) and + // b2 (title contains beta); under `and` it would be b2 alone. + const tree = [ + { + op: "or", + children: [ + { columnId: "title", operator: "contains", value: "beta" }, + { columnId: "count", operator: "gt", value: 2 }, + ], + }, + ]; + const view = renderTreeSurface(tree, { + onGridReady: (ready) => { + grid = ready; + }, + }); + + await expect + .poll(() => grid?.rowModel.getState().snapshot.query.filters) + .toEqual(tree); + await waitFor(() => expect(renderedRowIds(view)).toEqual(["b1", "b2"])); + }); + + it("lights a column's funnel for a leaf buried two groups deep", async () => { + const view = renderTreeSurface([ + { + op: "and", + children: [ + { + op: "or", + children: [ + { columnId: "title", operator: "contains", value: "alpha" }, + ], + }, + ], + }, + ]); + + // No top-level leaf mentions `title` at all — the old per-column record + // projection would have keyed the group under `undefined` and left this + // funnel dark. + await waitFor(() => + expect( + view.getByRole("button", { name: "Filter Title" }), + ).toHaveAttribute("data-pretable-filter-active", "true"), + ); + // Control: a column the tree never mentions stays dark. + expect(view.getByRole("button", { name: "Filter Count" })).toHaveAttribute( + "data-pretable-filter-active", + "false", + ); + }); + + it("hydrates the menu from the TOP-LEVEL leaf, not the nested one", () => { + const view = renderTreeSurface([ + { + op: "or", + children: [ + { columnId: "title", operator: "contains", value: "nested" }, + ], + }, + { columnId: "title", operator: "endsWith", value: "crash" }, + ]); + + fireEvent.click(view.getByRole("button", { name: "Filter Title" })); + const dialog = view.getByRole("dialog", { name: "Filter Title" }); + expect(within(dialog).getByLabelText("Filter operator")).toHaveValue( + "endsWith", + ); + expect(within(dialog).getByLabelText("Filter value")).toHaveValue("crash"); + }); + + it("a menu commit splices the top-level leaf and leaves the group untouched", async () => { + const group = { + op: "or", + children: [ + { columnId: "title", operator: "contains", value: "crash" }, + { columnId: "severity", operator: "isAnyOf", value: ["high"] }, + ], + }; + const onQueryChange = vi.fn(); + let grid: Parameters>[0] | null = + null; + const view = renderTreeSurface( + [{ columnId: "title", operator: "contains", value: "alpha" }, group], + { + onQueryChange, + onGridReady: (ready) => { + grid = ready; + }, + }, + ); + + // The group as the MODEL holds it. `captureFilterNode` allocates and + // freezes a fresh node on every capture, so this is a different object + // from the `group` literal above — and it is the one the menu's write path + // reads, because `setColumnFilter` builds its next query from + // `currentQuery()`. + const capturedGroup = await vi.waitUntil( + () => grid?.rowModel.getState().snapshot.query.filters[1], + ); + + fireEvent.click(view.getByRole("button", { name: "Filter Title" })); + const dialog = view.getByRole("dialog", { name: "Filter Title" }); + act(() => { + fireEvent.change(within(dialog).getByLabelText("Filter value"), { + target: { value: "leak" }, + }); + }); + + await waitFor(() => expect(onQueryChange).toHaveBeenCalled()); + const next = onQueryChange.mock.lastCall?.[0] as { + filters: readonly TreeNode[]; + }; + // The leaf is REPLACED in place, and the group element the menu never + // authored comes through in its original slot. + expect(next.filters).toEqual([ + { columnId: "title", operator: "contains", value: "leak" }, + group, + ]); + // BYTE-IDENTICAL, with teeth: `onQueryChange` is handed the surface's own + // `next` object BEFORE `rowModel.setQuery` re-captures it (see `setQuery` + // in `pretable-model.ts`), so this element is literally the object + // `withTopLevelColumnFilter` passed through — not a copy that merely + // compares equal. A write path that rebuilt groups from a projection, or + // cloned them defensively, would satisfy the `toEqual` above and fail + // here. + expect(next.filters[1]).toBe(capturedGroup); + }); + + it("clearing from the menu removes only the top-level leaf", async () => { + const group = { + op: "or", + children: [ + { columnId: "severity", operator: "isAnyOf", value: ["high"] }, + ], + }; + const onQueryChange = vi.fn(); + const view = renderTreeSurface( + [{ columnId: "title", operator: "contains", value: "alpha" }, group], + { onQueryChange }, + ); + + fireEvent.click(view.getByRole("button", { name: "Filter Title" })); + const dialog = view.getByRole("dialog", { name: "Filter Title" }); + fireEvent.click(within(dialog).getByText("Clear")); + + await waitFor(() => expect(onQueryChange).toHaveBeenCalled()); + const next = onQueryChange.mock.lastCall?.[0] as { + filters: readonly TreeNode[]; + }; + expect(next.filters).toEqual([group]); + }); +}); diff --git a/packages/react/src/__tests__/filter-tree.test.ts b/packages/react/src/__tests__/filter-tree.test.ts new file mode 100644 index 000000000..71bfaae1d --- /dev/null +++ b/packages/react/src/__tests__/filter-tree.test.ts @@ -0,0 +1,192 @@ +import { describe, expect, it } from "vitest"; + +import { + columnHasFilter, + topLevelColumnFilter, + withTopLevelColumnFilter, + type SurfaceFilterNode, +} from "../filter-tree"; + +describe("columnHasFilter", () => { + it("finds a top-level leaf", () => { + const filters: readonly SurfaceFilterNode[] = [ + { columnId: "a", operator: "contains", value: "x" }, + ]; + expect(columnHasFilter(filters, "a")).toBe(true); + expect(columnHasFilter(filters, "b")).toBe(false); + }); + + it("finds a leaf buried two groups deep, with no top-level leaf for it", () => { + const filters: readonly SurfaceFilterNode[] = [ + { columnId: "b", operator: "isNotEmpty" }, + { + op: "or", + children: [ + { columnId: "c", operator: "contains", value: "q" }, + { + op: "and", + children: [{ columnId: "a", operator: "equals", value: 3 }], + }, + ], + }, + ]; + expect(columnHasFilter(filters, "a")).toBe(true); + expect(columnHasFilter(filters, "c")).toBe(true); + expect(columnHasFilter(filters, "b")).toBe(true); + expect(columnHasFilter(filters, "d")).toBe(false); + }); + + it("an empty group mentions no column", () => { + expect(columnHasFilter([{ op: "and", children: [] }], "a")).toBe(false); + expect(columnHasFilter([], "a")).toBe(false); + }); + + it("does not mistake a group's `op` for a column match", () => { + // A group carries no `columnId`; a naive `entry.columnId === columnId` + // read would compare `undefined` and could match a column literally + // named "undefined". Neither is a hit here. + const filters: readonly SurfaceFilterNode[] = [{ op: "or", children: [] }]; + expect(columnHasFilter(filters, "undefined")).toBe(false); + }); + + it("walks a tree deeper than the engine bound without blowing up", () => { + // The engine rejects trees deeper than 64 at `compileQuery`; the surface + // helper must not hang before that rejection is reachable. + let node: SurfaceFilterNode = { + columnId: "a", + operator: "isEmpty", + }; + for (let i = 0; i < 200; i += 1) node = { op: "and", children: [node] }; + expect(columnHasFilter([node], "a")).toBe(true); + }); +}); + +describe("topLevelColumnFilter", () => { + it("reads the top-level leaf, not the one nested in a group", () => { + const filters: readonly SurfaceFilterNode[] = [ + { + op: "or", + children: [{ columnId: "a", operator: "contains", value: "nested" }], + }, + { columnId: "a", operator: "endsWith", value: "top" }, + ]; + expect(topLevelColumnFilter(filters, "a")).toEqual({ + operator: "endsWith", + value: "top", + }); + }); + + it("drops the `columnId` and omits an absent `value`", () => { + expect( + topLevelColumnFilter([{ columnId: "a", operator: "isEmpty" }], "a"), + ).toEqual({ operator: "isEmpty" }); + expect( + "value" in + (topLevelColumnFilter([{ columnId: "a", operator: "isEmpty" }], "a") ?? + {}), + ).toBe(false); + }); + + it("is null when only a nested leaf mentions the column", () => { + const filters: readonly SurfaceFilterNode[] = [ + { + op: "and", + children: [{ columnId: "a", operator: "contains", value: "nested" }], + }, + ]; + expect(topLevelColumnFilter(filters, "a")).toBeNull(); + }); +}); + +it("reads the FIRST of two top-level leaves for the same column", () => { + // Only a hand-authored `filters` can hold duplicates — the menu never + // writes a second leaf for a column. When one does, first wins. The + // per-column record this replaced was LAST-wins (each entry overwrote the + // key), so this is a deliberate change of answer, not an accident. + const filters: readonly SurfaceFilterNode[] = [ + { columnId: "a", operator: "contains", value: "one" }, + { columnId: "a", operator: "equals", value: "two" }, + { op: "or", children: [] }, + ]; + expect(topLevelColumnFilter(filters, "a")).toEqual({ + operator: "contains", + value: "one", + }); +}); + +describe("withTopLevelColumnFilter", () => { + const group: SurfaceFilterNode = { + op: "or", + children: [ + { columnId: "a", operator: "contains", value: "nested" }, + { columnId: "b", operator: "isNotEmpty" }, + ], + }; + + it("replaces the top-level leaf in place and leaves the group identical", () => { + const filters: readonly SurfaceFilterNode[] = [ + { columnId: "a", operator: "contains", value: "old" }, + group, + ]; + const next = withTopLevelColumnFilter(filters, "a", { + operator: "endsWith", + value: "new", + }); + expect(next).toEqual([ + { columnId: "a", operator: "endsWith", value: "new" }, + group, + ]); + // Reference identity, not just structural equality: the group element the + // menu did not touch must be the very object the query already held. + expect(next[1]).toBe(group); + }); + + it("appends when there is no top-level leaf yet, after the group", () => { + const next = withTopLevelColumnFilter([group], "a", { + operator: "equals", + value: "x", + }); + expect(next).toEqual([ + group, + { columnId: "a", operator: "equals", value: "x" }, + ]); + expect(next[0]).toBe(group); + }); + + it("clearing removes only the top-level leaf; the group survives", () => { + const filters: readonly SurfaceFilterNode[] = [ + { columnId: "a", operator: "contains", value: "old" }, + group, + ]; + const next = withTopLevelColumnFilter(filters, "a", null); + expect(next).toEqual([group]); + expect(next[0]).toBe(group); + }); + + it("collapses two top-level leaves for the same column into one", () => { + // The read side takes the FIRST duplicate, so the write side must replace + // that same one and drop the rest: leaving a second `a` leaf behind would + // make the commit look inert, because the query would still carry the + // operand the user just replaced. + const filters: readonly SurfaceFilterNode[] = [ + { columnId: "a", operator: "contains", value: "one" }, + { columnId: "a", operator: "equals", value: "two" }, + group, + ]; + const next = withTopLevelColumnFilter(filters, "a", { + operator: "startsWith", + value: "three", + }); + expect(next).toEqual([ + { columnId: "a", operator: "startsWith", value: "three" }, + group, + ]); + expect(next[1]).toBe(group); + }); + + it("omits `value` when the committed filter has none", () => { + const next = withTopLevelColumnFilter([], "a", { operator: "isEmpty" }); + expect(next).toEqual([{ columnId: "a", operator: "isEmpty" }]); + expect("value" in next[0]!).toBe(false); + }); +}); diff --git a/packages/react/src/__tests__/labeled-grid-surface.test.tsx b/packages/react/src/__tests__/labeled-grid-surface.test.tsx index 4102372eb..51603129c 100644 --- a/packages/react/src/__tests__/labeled-grid-surface.test.tsx +++ b/packages/react/src/__tests__/labeled-grid-surface.test.tsx @@ -288,6 +288,54 @@ describe("LabeledGridSurface", () => { expect(timestampHeader).not.toHaveClass("is-filtered"); }); + it("applies the filter-active class for a leaf nested inside a group", () => { + // `filters` is an AND/OR tree. A column constrained only from inside a + // group is still constrained, so its header still reads as filtered — the + // same "occurrence anywhere" rule the surface's funnel uses. + const view = render( + row.id} + headerCellClassName="inspection-header-cell" + query={ + { + sort: [], + filters: [ + { + op: "or", + children: [ + { + op: "and", + children: [ + { + columnId: "severity", + operator: "contains", + value: "error", + }, + ], + }, + ], + }, + ], + rowGroups: [], + } as never + } + onQueryChange={() => {}} + overscan={0} + rows={rows} + viewportHeight={132} + />, + ); + + expect( + view.getByRole("columnheader", { name: "Sort Severity" }), + ).toHaveClass("is-filtered"); + expect( + view.getByRole("columnheader", { name: "Sort Timestamp" }), + ).not.toHaveClass("is-filtered"); + }); + it("passes query and onQueryChange through to the underlying surface", () => { const onQueryChange = vi.fn(); const view = render( diff --git a/packages/react/src/filter-tree.ts b/packages/react/src/filter-tree.ts new file mode 100644 index 000000000..df4b18952 --- /dev/null +++ b/packages/react/src/filter-tree.ts @@ -0,0 +1,152 @@ +import { isPretableFilterGroup, type ColumnFilter } from "@pretable/core"; + +/** + * The value-erased twin of `PretableFilterFor`: one typed leaf of + * the filter tree, seen from chrome that works from DRAWN column ids and + * runtime operand values rather than from a static column tuple. + */ +export interface SurfaceFilterLeaf { + readonly columnId: string; + readonly operator: ColumnFilter["operator"]; + readonly value?: ColumnFilter["value"]; +} + +/** The value-erased twin of `PretableFilterGroupFor`. */ +export interface SurfaceFilterGroup { + readonly op: "and" | "or"; + readonly children: readonly SurfaceFilterNode[]; +} + +/** + * One node of the surface's view of `query.filters`: a leaf or a group. + * + * The surface holds the filter tree VERBATIM — it keeps no per-column record + * beside it. A record cannot describe a group (a group has no `columnId`, so + * every group in a query would collapse onto the single key `undefined`), and + * a projection that lies by omission is worse than the two small walks below. + */ +export type SurfaceFilterNode = SurfaceFilterLeaf | SurfaceFilterGroup; + +/** + * `PretableQueryFor["filters"]` read as value-erased nodes. + * + * `PretableFilterNodeFor` discriminates its leaves over the column tuple's + * static `accessor` return types and literal `type`s. The surface's columns are + * runtime-supplied and value-erased, so that union collapses and no assignment + * between the two shapes is checkable — the same collapse `queryWith` and + * `distinctValues` document, and the reason this is a cast and not a + * conversion. It is the single place the erasure is spelled out; call sites + * carry only their own tree-semantics comment. + */ +export function asSurfaceNodes( + filters: readonly unknown[], +): readonly SurfaceFilterNode[] { + return filters as readonly SurfaceFilterNode[]; +} + +/** + * Narrows a value-erased node to a group — the surface's `isPretableFilterGroup`. + * + * The engine's guard is generic over a static column tuple, and the surface's + * nodes are value-erased, so `as never` is what satisfies the parameter. It + * does not weaken the check: the guard is structural at runtime and tests the + * group's own fields. Same collapse, and the same remedy, as the + * `distinctValues` call in `pretable-surface.tsx`. + * + * Exported so the SECOND walk over the tree — `LabeledGridSurface`'s + * `is-filtered` header decoration, which cannot share these functions because + * it gates on `isColumnFilterActive` — narrows through this one explanation + * instead of repeating the casts. + */ +export const isSurfaceFilterGroup = ( + node: SurfaceFilterNode, +): node is SurfaceFilterGroup => isPretableFilterGroup(node as never); + +/** + * Does ANY leaf anywhere in the tree constrain `columnId`? + * + * This is what lights a column's funnel. Occurrence, not position: a filter + * the user built two groups deep in the filter builder still means "this + * column is filtered", and a funnel that only noticed top-level leaves would + * tell them their column was unconstrained while it removed their rows. + */ +export function columnHasFilter( + nodes: readonly SurfaceFilterNode[], + columnId: string, +): boolean { + return nodes.some((node) => + isSurfaceFilterGroup(node) + ? columnHasFilter(node.children, columnId) + : node.columnId === columnId, + ); +} + +/** + * The column's FIRST top-level leaf, as the per-column filter menu understands + * it — or `null` when only a group mentions the column. + * + * The menu edits one column with one operator and one operand; it cannot + * express a group, so it owns exactly that leaf and reports nothing about the + * rest of the tree. Reaching into groups here would let a menu commit silently + * overwrite a branch the user built elsewhere. + * + * FIRST, definitely, and it matters: only a hand-authored `filters` can hold + * two top-level leaves for one column, but when it does, this reads the + * earlier one and `withTopLevelColumnFilter` replaces that same one — the two + * halves agree, which is the point. The per-column record this replaced was + * LAST-wins (each entry overwrote the key), so a consumer with duplicates sees + * the other leaf now. + */ +export function topLevelColumnFilter( + nodes: readonly SurfaceFilterNode[], + columnId: string, +): ColumnFilter | null { + for (const node of nodes) { + if (isSurfaceFilterGroup(node) || node.columnId !== columnId) continue; + return { + operator: node.operator, + ...(node.value === undefined ? {} : { value: node.value }), + }; + } + return null; +} + +/** + * The menu's write path: replace (or, cleared, remove) the column's top-level + * leaf and pass every other element through BY REFERENCE. + * + * Replacement is in place rather than remove-then-append so that committing a + * new operand does not reshuffle the array, and groups keep their slots. Every + * element this function did not author is the caller's own object, unchanged — + * that is the contract the surface's group elements survive on. + */ +export function withTopLevelColumnFilter( + nodes: readonly SurfaceFilterNode[], + columnId: string, + filter: ColumnFilter | null, +): readonly SurfaceFilterNode[] { + const replacement: SurfaceFilterLeaf | null = + filter === null + ? null + : { + columnId, + operator: filter.operator, + ...(filter.value === undefined ? {} : { value: filter.value }), + }; + let replaced = false; + const next: SurfaceFilterNode[] = []; + for (const node of nodes) { + if (isSurfaceFilterGroup(node) || node.columnId !== columnId) { + next.push(node); + continue; + } + // Only the FIRST top-level leaf is the menu's; any duplicate a consumer + // wrote is dropped, because the menu can only show one of them and + // leaving the others would make the commit look inert. + if (replaced || replacement === null) continue; + next.push(replacement); + replaced = true; + } + if (!replaced && replacement !== null) next.push(replacement); + return next; +} diff --git a/packages/react/src/labeled-grid-surface.tsx b/packages/react/src/labeled-grid-surface.tsx index 6bb6de41f..9087bd02f 100644 --- a/packages/react/src/labeled-grid-surface.tsx +++ b/packages/react/src/labeled-grid-surface.tsx @@ -4,6 +4,11 @@ import type { PretableSortDirection, PretableQueryFor, } from "@pretable/core"; +import { + asSurfaceNodes, + isSurfaceFilterGroup, + type SurfaceFilterNode, +} from "./filter-tree"; import type { HTMLAttributes } from "react"; import type { PretableTelemetry } from "./surface-types"; import { SortAscIcon, SortDescIcon } from "./icons"; @@ -30,6 +35,27 @@ function isColumnFilterActive(filter: { return true; // number } +/** + * Every column an ACTIVE leaf constrains, at any depth. Groups carry no + * `columnId`; they are recursed into, never counted. + * + * A SECOND walk rather than `columnHasFilter`: this one gates on + * `isColumnFilterActive`, so a filter with no usable operand yet decorates + * nothing. The narrowing is shared even though the walk is not. + */ +function collectActiveFilterColumns( + nodes: readonly SurfaceFilterNode[], + into: Set, +): void { + for (const node of nodes) { + if (isSurfaceFilterGroup(node)) { + collectActiveFilterColumns(node.children, into); + } else if (isColumnFilterActive(node)) { + into.add(node.columnId); + } + } +} + /** * Input passed to a {@link LabeledGridSurface} format function. * @@ -182,10 +208,14 @@ export function LabeledGridSurface< // write back to the prop. const getPinnedClassName = (pinned: "left" | "right" | null) => pinned != null && pinnedClassName ? pinnedClassName : undefined; - const activeFilterColumns = new Set( - (query?.filters ?? []) - .filter((filter) => isColumnFilterActive(filter)) - .map((filter) => filter.columnId), + const activeFilterColumns = new Set(); + // TREE-AWARE: `query.filters` is an AND/OR tree, and this label decoration + // means "this column is constrained", which a leaf nested inside a group + // makes just as true as a top-level one. See `columnHasFilter` in + // `./filter-tree` for the same walk on the surface's own funnel. + collectActiveFilterColumns( + asSurfaceNodes(query?.filters ?? []), + activeFilterColumns, ); const getFormattedValue = ({ column, diff --git a/packages/react/src/pretable-surface.tsx b/packages/react/src/pretable-surface.tsx index 4bd2b4528..6ec68fcb8 100644 --- a/packages/react/src/pretable-surface.tsx +++ b/packages/react/src/pretable-surface.tsx @@ -391,6 +391,13 @@ import { type PretableBodyStateKind, type PretableDataState, } from "./data-state"; +import { + asSurfaceNodes, + columnHasFilter, + topLevelColumnFilter, + withTopLevelColumnFilter, + type SurfaceFilterNode, +} from "./filter-tree"; /** Local interaction facade used while the surface maps UI commands onto the * indexed grid and row model. Row data, queries, grouping, and expansion @@ -400,7 +407,12 @@ interface SurfaceFacade { getSnapshot(): { readonly viewport: PretableViewportState; readonly sort: readonly PretableSortEntry[]; - readonly filters: Readonly>; + // The query's filter TREE, verbatim — leaves and groups, nested. Not a + // per-column record: a group carries no `columnId`, so a record would + // collapse every group in the query onto the single key `undefined`. + // Chrome that wants a per-column answer asks `columnHasFilter` / + // `topLevelColumnFilter` (see `./filter-tree`). + readonly filters: readonly SurfaceFilterNode[]; readonly selection: PretableSelectionState; readonly focus: PretableFocusState & { readonly ref: PretableIndexedFocusRef | null; @@ -2177,17 +2189,9 @@ export function PretableSurface< [indexed.rowModel], ); const snapshot = useMemo(() => { - const filters: Record = {}; - for (const entry of rowModelSnapshot.query.filters as readonly { - readonly columnId: string; - readonly operator: ColumnFilter["operator"]; - readonly value?: ColumnFilter["value"]; - }[]) { - filters[entry.columnId] = { - operator: entry.operator, - ...(entry.value === undefined ? {} : { value: entry.value }), - }; - } + // TREE-AWARE, by passing through: the filter tree reaches the chrome + // exactly as the model holds it — leaves, groups and nesting intact. + const filters = asSurfaceNodes(rowModelSnapshot.query.filters); const ranges = indexedSnapshot.selection.ranges.map(flattenIndexedRange); const ref = indexedSnapshot.focus.ref; return { @@ -2420,6 +2424,9 @@ export function PretableSurface< // `setQuery` on the engine or the surface reconstructing the // discriminated filter union from runtime data — both outside this file. const next = { + // TREE-AGNOSTIC PASS-THROUGH: an unnamed axis is re-submitted exactly + // as the model holds it, groups and nesting included. Only the caller + // that names `filters` decides what the tree becomes. filters: (parts.filters ?? current.filters) as never, sort: (parts.sort ?? current.sort) as never, rowGroups: (parts.rowGroups ?? current.rowGroups) as never, @@ -2467,20 +2474,11 @@ export function PretableSurface< const projectedQuery = currentQuery(); if (projectedQuery === surfaceContextRef.current.rowModelSnapshot.query) return current; - const projectedFilters: Record = {}; - for (const entry of projectedQuery.filters as readonly { - readonly columnId: string; - readonly operator: ColumnFilter["operator"]; - readonly value?: ColumnFilter["value"]; - }[]) { - projectedFilters[entry.columnId] = { - operator: entry.operator, - ...(entry.value === undefined ? {} : { value: entry.value }), - }; - } return { ...current, - filters: projectedFilters, + // Tree-aware by the same pass-through as the committed projection + // above; this one just reads the not-yet-settled query. + filters: asSurfaceNodes(projectedQuery.filters), sort: projectedQuery.sort as readonly PretableSortEntry[], rowGroups: ( projectedQuery.rowGroups as readonly { @@ -2693,15 +2691,17 @@ export function PretableSurface< queryWith({ sort }); }, setColumnFilter(columnId: string, filter: ColumnFilter | null) { - const current = currentQuery(); - const filters = ( - current.filters as readonly { - readonly columnId: string; - }[] - ).filter((entry) => entry.columnId !== columnId); + // LEAF-ONLY BY DESIGN, and scoped to the TOP LEVEL: the per-column + // menu can express one operator over one operand, so it owns exactly + // this column's top-level leaf. Every group element passes through by + // reference — a menu commit must never edit or drop a branch the user + // assembled somewhere else. queryWith({ - filters: - filter === null ? filters : [...filters, { columnId, ...filter }], + filters: withTopLevelColumnFilter( + asSurfaceNodes(currentQuery().filters), + columnId, + filter, + ), }); }, setRowGroups(columnIds: readonly string[]) { @@ -3218,6 +3218,8 @@ export function PretableSurface< : null; const current = rowModelSnapshot.query; indexedGrid.setQuery({ + // Tree-agnostic pass-through, as in `queryWith`: grouping changes + // resubmit the filter tree untouched. filters: current.filters, sort: current.sort, // `PretableRowGroupFor` — `never` for the same reason as the @@ -5845,7 +5847,10 @@ export function PretableSurface< togglePopover("filter", id, anchor) @@ -6612,9 +6617,10 @@ export function PretableSurface< type={col.type ?? "text"} allowedOperators={col.filterOperators} options={options} - initialFilter={ - snapshot.filters[filterOpenState.columnId] ?? null - } + initialFilter={topLevelColumnFilter( + snapshot.filters, + filterOpenState.columnId, + )} {...(col.type === "enum" && col.options === undefined ? { loadDistinctValues } : {})} diff --git a/packages/react/src/public_api.ts b/packages/react/src/public_api.ts index 4e87b58cf..8c7702a6d 100644 --- a/packages/react/src/public_api.ts +++ b/packages/react/src/public_api.ts @@ -153,7 +153,11 @@ export type { DensityHeights } from "@pretable/ui"; // Re-exports from @pretable/core (the engine types react users typically // touch — full headless surface lives in @pretable/core) -export { describeRowSelection, numberFormats } from "@pretable/core"; +export { + describeRowSelection, + isPretableFilterGroup, + numberFormats, +} from "@pretable/core"; export type { AutosizeOptions, ColumnAlign, @@ -198,6 +202,8 @@ export type { PretableFocusDirection, PretableFocusState, PretableFilterFor, + PretableFilterGroupFor, + PretableFilterNodeFor, PretableFilterOperandFor, PretableHeaderRowRef, PretableGridUiColumn, diff --git a/packages/row-model/src/__tests__/distinct-values.test.ts b/packages/row-model/src/__tests__/distinct-values.test.ts index bdba28bbe..198b2169a 100644 --- a/packages/row-model/src/__tests__/distinct-values.test.ts +++ b/packages/row-model/src/__tests__/distinct-values.test.ts @@ -560,6 +560,99 @@ describe("bounded distinct-value dictionaries", () => { }); }); + describe("filter groups", () => { + interface FilterRow { + id: number; + primary: string; + secondary: string; + } + const filterHelper = createColumnHelper(); + const filterColumns = [ + filterHelper.accessor("primary", { type: "enum" }), + filterHelper.accessor("secondary", { type: "enum" }), + ] as const; + const andGroup = (primary: readonly string[]) => ({ + op: "and" as const, + children: [ + { + columnId: "primary" as const, + operator: "isAnyOf" as const, + value: primary, + }, + { + columnId: "secondary" as const, + operator: "isAnyOf" as const, + value: ["x"], + }, + ], + }); + + function makeModel(filters: readonly unknown[]) { + const scheduler = new ManualScheduler(); + const model = createLocalRowModel({ + rows: [ + { id: 1, primary: "a", secondary: "x" }, + { id: 2, primary: "b", secondary: "x" }, + ], + columns: filterColumns, + query: { filters, sort: [], rowGroups: [] } as never, + transitionScheduler: scheduler, + transitionClock: tickingClock(), + transitionBudgetMs: 1, + transitionMaxUnitsPerSlice: 1, + }); + const distinctPrimary = () => + model.distinctValues("primary", { population: "filtered", limit: 10 }); + return { model, scheduler, distinctPrimary }; + } + + test("distinguishes filter values that differ only INSIDE a group", async () => { + const { model, scheduler, distinctPrimary } = makeModel([ + andGroup(["a"]), + ]); + const initial = distinctPrimary(); + scheduler.flushAll(); + await expect(initial.finished).resolves.toMatchObject({ + values: [{ value: "a", count: 1 }], + }); + + // Identical outside the group; only a child operand changed. A cache key + // that does not recurse collapses both queries onto one entry and serves + // the first answer back. + const changed = model.setQuery({ + filters: [andGroup(["b"])], + sort: [], + rowGroups: [], + } as never); + scheduler.flushAll(); + await changed.finished; + const rebuilt = distinctPrimary(); + expect(rebuilt.status).toBe("pending"); + scheduler.flushAll(); + await expect(rebuilt.finished).resolves.toMatchObject({ + values: [{ value: "b", count: 1 }], + }); + }); + + test("reuses the cache when a group's children are merely reordered", async () => { + const { model, scheduler, distinctPrimary } = makeModel([ + andGroup(["a"]), + ]); + const initial = distinctPrimary(); + scheduler.flushAll(); + await initial.finished; + + const group = andGroup(["a"]); + const reordered = model.setQuery({ + filters: [{ op: "and", children: [...group.children].reverse() }], + sort: [], + rowGroups: [], + } as never); + await expect(reordered.finished).resolves.toBe(0); + expect(distinctPrimary().status).toBe("ready"); + }); + }); + test("supports bounded search and ranges with explicit blank inclusion and ordering", async () => { const scheduler = new ManualScheduler(); const rows: readonly Row[] = [ diff --git a/packages/row-model/src/__tests__/filter-tree.test.ts b/packages/row-model/src/__tests__/filter-tree.test.ts new file mode 100644 index 000000000..03e21ea1e --- /dev/null +++ b/packages/row-model/src/__tests__/filter-tree.test.ts @@ -0,0 +1,513 @@ +import { describe, expect, test } from "vitest"; + +import { + CompiledQueryValidationError, + compileQuery, + createColumnHelper, + createLocalRowModel, + getCapturedFilterTreeForTesting, + isPretableFilterGroup, + type PretableFilterNodeFor, + type PretableQueryFor, +} from "../index"; + +interface Holding { + id: number; + sector: string | null; + quantity: number | null; + asOf: Date; + status: "open" | "closed"; +} + +const column = createColumnHelper(); +const columns = [ + column.accessor("sector", { type: "text" }), + column.accessor("quantity", { type: "number" }), + column.accessor("asOf", { type: "date" }), + column.accessor("status", { type: "enum" }), +] as const; + +type Columns = typeof columns; +type Node = PretableFilterNodeFor; + +function queryFor(value: PretableQueryFor): PretableQueryFor { + return value; +} + +function compile(query: unknown) { + return compileQuery({ + derivations: columns, + query, + } as never); +} + +function recompile(query: unknown, previous: unknown) { + return compileQuery({ + derivations: columns, + query, + previous, + } as never); +} + +function caughtFrom(query: unknown): unknown { + try { + compile(query); + } catch (error) { + return error; + } + return undefined; +} + +describe("isPretableFilterGroup", () => { + test("accepts both join operators, including an empty group", () => { + expect(isPretableFilterGroup({ op: "and", children: [] })).toBe( + true, + ); + expect( + isPretableFilterGroup({ + op: "or", + children: [{ columnId: "sector", operator: "isEmpty" }], + }), + ).toBe(true); + }); + + test.each([ + ["text", { columnId: "sector", operator: "contains", value: "x" }], + [ + "number range", + { columnId: "quantity", operator: "between", value: [1, 2] }, + ], + ["date", { columnId: "asOf", operator: "on", value: "2026-01-01" }], + ["enum set", { columnId: "status", operator: "isAnyOf", value: ["open"] }], + ["valueless", { columnId: "sector", operator: "isEmpty" }], + ] as const)("rejects a %s leaf", (_family, leaf) => { + expect(isPretableFilterGroup(leaf as Node)).toBe(false); + }); + + test.each([ + ["null", null], + ["undefined", undefined], + ["a bare string", "and"], + ["a number", 42], + ["an empty object", {}], + ["a join with no children", { op: "and" }], + ["children with no join", { children: [] }], + ["an unknown join", { op: "nor", children: [] }], + ["a miscased join", { op: "AND", children: [] }], + ] as const)("fails closed on %s", (_shape, value) => { + expect(isPretableFilterGroup(value as never)).toBe(false); + }); +}); + +describe("capture of a filter tree", () => { + test("round-trips a nested group into the compiled query", () => { + const plan = compile( + queryFor({ + filters: [ + { columnId: "quantity", operator: "gte", value: 4 }, + { + op: "or", + children: [ + { columnId: "sector", operator: "contains", value: "tech" }, + { + op: "and", + children: [ + { columnId: "status", operator: "isAnyOf", value: ["open"] }, + { columnId: "sector", operator: "isNotEmpty" }, + ], + }, + ], + }, + ], + sort: [], + rowGroups: [], + }), + ); + + expect(plan.query.filters).toHaveLength(2); + const group = plan.query.filters[1]; + expect(isPretableFilterGroup(group)).toBe(true); + if (!isPretableFilterGroup(group)) throw new Error("unreachable"); + expect(group.op).toBe("or"); + expect(group.children).toHaveLength(2); + expect(group.children[0]).toMatchObject({ + columnId: "sector", + operator: "contains", + value: "tech", + }); + const nested = group.children[1]; + if (!isPretableFilterGroup(nested)) throw new Error("unreachable"); + expect(nested.op).toBe("and"); + expect(nested.children[0]).toMatchObject({ + columnId: "status", + operator: "isAnyOf", + value: ["open"], + }); + }); + + test("rejects an unknown join operator at its own path", () => { + const caught = caughtFrom({ + filters: [ + { columnId: "quantity", operator: "gte", value: 4 }, + { op: "xor", children: [] }, + ], + sort: [], + rowGroups: [], + }); + + expect(caught).toBeInstanceOf(CompiledQueryValidationError); + expect(caught).toMatchObject({ + code: "invalid-query", + path: "query.filters[1].op", + }); + }); + + test("rejects a group whose children are not an array at its own path", () => { + const caught = caughtFrom({ + filters: [{ op: "and", children: { length: 0 } }], + sort: [], + rowGroups: [], + }); + + expect(caught).toBeInstanceOf(CompiledQueryValidationError); + expect(caught).toMatchObject({ + code: "invalid-query", + path: "query.filters[0].children", + }); + }); + + test("reports a nested failure with its full breadcrumb", () => { + const caught = caughtFrom({ + filters: [ + { + op: "and", + children: [ + { columnId: "sector", operator: "isEmpty" }, + { op: "and", children: [{ op: "nope", children: [] }] }, + ], + }, + ], + sort: [], + rowGroups: [], + }); + + expect(caught).toBeInstanceOf(CompiledQueryValidationError); + expect(caught).toMatchObject({ + path: "query.filters[0].children[1].children[0].op", + }); + }); + + test("deep-freezes every level of the tree it captured", () => { + const plan = compile( + queryFor({ + filters: [ + { + op: "and", + children: [ + { + op: "or", + children: [{ columnId: "sector", operator: "isEmpty" }], + }, + ], + }, + ], + sort: [], + rowGroups: [], + }), + ); + + // Deliberately NOT `plan.query`: that getter re-freezes everything it + // hands back, so it proves `snapshotQuery` froze a copy and says nothing + // about capture. This reads the plan's own captured tree. + const captured = getCapturedFilterTreeForTesting(plan) as readonly Node[]; + const outer = captured[0]; + if (!isPretableFilterGroup(outer)) throw new Error("unreachable"); + const inner = outer.children[0]; + if (!isPretableFilterGroup(inner)) throw new Error("unreachable"); + const leaf = inner.children[0]; + + expect(Object.isFrozen(captured)).toBe(true); + expect(Object.isFrozen(outer)).toBe(true); + expect(Object.isFrozen(outer.children)).toBe(true); + expect(Object.isFrozen(inner)).toBe(true); + expect(Object.isFrozen(inner.children)).toBe(true); + expect(Object.isFrozen(leaf)).toBe(true); + expect(() => { + (outer as { op: string }).op = "or"; + }).toThrow(TypeError); + expect(() => { + (inner as { op: string }).op = "and"; + }).toThrow(TypeError); + expect(() => { + (leaf as { columnId: string }).columnId = "quantity"; + }).toThrow(TypeError); + }); + + test("re-freezes every level of the tree it publishes", () => { + const plan = compile( + queryFor({ + filters: [ + { + op: "and", + children: [ + { + op: "or", + children: [{ columnId: "sector", operator: "isEmpty" }], + }, + ], + }, + ], + sort: [], + rowGroups: [], + }), + ); + + const outer = plan.query.filters[0]; + if (!isPretableFilterGroup(outer)) throw new Error("unreachable"); + const inner = outer.children[0]; + if (!isPretableFilterGroup(inner)) throw new Error("unreachable"); + + expect(Object.isFrozen(outer)).toBe(true); + expect(Object.isFrozen(inner)).toBe(true); + expect(Object.isFrozen(inner.children[0])).toBe(true); + expect(() => { + (inner as { op: string }).op = "and"; + }).toThrow(TypeError); + }); + + test("copies the incoming tree rather than retaining it", () => { + const child = { columnId: "sector", operator: "isEmpty" }; + const group = { op: "and", children: [child] }; + const plan = compile({ filters: [group], sort: [], rowGroups: [] }); + + const captured = plan.query.filters[0]; + expect(captured).not.toBe(group); + if (!isPretableFilterGroup(captured)) + throw new Error("unreachable"); + expect(captured.children[0]).not.toBe(child); + }); + + test("still accepts an array of plain leaves", () => { + const plan = compile( + queryFor({ + filters: [ + { columnId: "quantity", operator: "gte", value: 4 }, + { columnId: "sector", operator: "contains", value: "tech" }, + ], + sort: [], + rowGroups: [], + }), + ); + expect(plan.query.filters).toHaveLength(2); + expect( + plan.query.filters.every((node) => !isPretableFilterGroup(node)), + ).toBe(true); + }); +}); + +describe("group identity", () => { + const groupOf = (children: readonly unknown[]) => ({ + op: "and", + children, + }); + const containing = (value: string) => ({ + columnId: "sector", + operator: "contains", + value, + }); + const queryOf = (children: readonly unknown[]) => ({ + filters: [groupOf(children)], + sort: [], + rowGroups: [], + }); + + test("rebuilds when a group child's value changes", () => { + const first = compile(queryOf([containing("a"), containing("b")])); + expect(recompile(queryOf([containing("a"), containing("b")]), first)).toBe( + first, + ); + expect( + recompile(queryOf([containing("a"), containing("c")]), first), + ).not.toBe(first); + }); + + test("reuses a plan when a group's children are merely reordered", () => { + const first = compile(queryOf([containing("a"), containing("b")])); + expect(recompile(queryOf([containing("b"), containing("a")]), first)).toBe( + first, + ); + }); + + test("a child value cannot forge a sibling and impersonate a larger group", () => { + // Built against the concatenated descriptor key: a leaf keys as + // `columnId\0operator\0string:` and a group joins its children + // with \u0001, none of it length-framed. This single operand reproduces + // the two-child key above byte for byte. + const forged = containing("a\u0001sector\u0000contains\u0000string:b"); + const first = compile(queryOf([containing("a"), containing("b")])); + + expect(recompile(queryOf([forged]), first)).not.toBe(first); + }); + + test("distinguishes groups that differ only in their join operator", () => { + const children = [containing("a"), containing("b")]; + const first = compile({ + filters: [{ op: "and", children }], + sort: [], + rowGroups: [], + }); + expect( + recompile( + { filters: [{ op: "or", children }], sort: [], rowGroups: [] }, + first, + ), + ).not.toBe(first); + }); + + test("recurses into nested groups rather than stopping at the top", () => { + const nested = (value: string) => ({ + filters: [ + { + op: "and", + children: [ + containing("a"), + { op: "or", children: [containing(value)] }, + ], + }, + ], + sort: [], + rowGroups: [], + }); + const first = compile(nested("deep")); + expect(recompile(nested("deep"), first)).toBe(first); + expect(recompile(nested("deeper"), first)).not.toBe(first); + }); + + test("never matches a group against a leaf", () => { + const first = compile(queryOf([containing("a")])); + expect( + recompile({ filters: [containing("a")], sort: [], rowGroups: [] }, first), + ).not.toBe(first); + }); +}); + +describe("evaluation of a filter tree", () => { + interface Reading { + readonly id: string; + readonly n: number; + } + + const reading = createColumnHelper(); + const readingColumns = [ + reading.accessor("n", (row: Reading) => row.n, { type: "number" }), + ] as const; + + const ROWS: readonly Reading[] = Object.freeze([ + { id: "1", n: 1 }, + { id: "5", n: 5 }, + { id: "9", n: 9 }, + ]); + + const gt4 = { columnId: "n", operator: "gt", value: 4 } as const; + const lt2 = { columnId: "n", operator: "lt", value: 2 } as const; + const gt8 = { columnId: "n", operator: "gt", value: 8 } as const; + + /** The ids the model actually keeps visible under `filters`. */ + function visibleUnder(filters: readonly unknown[]): readonly string[] { + const model = createLocalRowModel({ + rows: [...ROWS], + columns: readingColumns, + getRowId: (row) => row.id, + query: { filters, sort: [], rowGroups: [] } as never, + }); + return model + .getState() + .snapshot.range(0, Number.MAX_SAFE_INTEGER) + .flatMap((row) => + (row as { kind: string }).kind === "data" + ? [String((row as { rowId: unknown }).rowId)] + : [], + ); + } + + test("an or group disjoins, and the same tree under and does not", () => { + // The pair is the point: the OR fixture's rows differ from the identical + // tree joined with `and`, so a connective mix-up cannot pass both. + expect(visibleUnder([gt4, { op: "or", children: [lt2, gt8] }])).toEqual([ + "9", + ]); + expect(visibleUnder([gt4, { op: "and", children: [lt2, gt8] }])).toEqual( + [], + ); + }); + + test("nesting three deep evaluates at every level", () => { + const nested = (innerOp: "and" | "or") => [ + gt4, + { op: "or", children: [lt2, { op: innerOp, children: [lt2, gt8] }] }, + ]; + expect(visibleUnder(nested("or"))).toEqual(["9"]); + expect(visibleUnder(nested("and"))).toEqual([]); + }); + + test.each(["and", "or"] as const)( + "an empty %s group is true, so a half-built group never blanks the grid", + (op) => { + expect(visibleUnder([gt4, { op, children: [] }])).toEqual( + visibleUnder([gt4]), + ); + expect(visibleUnder([gt4, { op, children: [] }])).toEqual(["5", "9"]); + }, + ); + + test("a group of groups still resolves when only one branch holds", () => { + expect( + visibleUnder([ + { + op: "or", + children: [ + { op: "and", children: [gt4, gt8] }, + { op: "and", children: [lt2, gt8] }, + ], + }, + ]), + ).toEqual(["9"]); + }); +}); + +describe("filter tree depth", () => { + // Mirrors the (unexported) `MAX_FILTER_TREE_DEPTH` in `compiled-query.ts`. + // Not imported: `index.ts` re-exports that module with `export *`, so an + // export here would move the package's public API surface. Drift is caught + // rather than hidden — both sides of the bound are asserted below, so + // raising or lowering the real limit fails one of them. + const MAX_FILTER_TREE_DEPTH = 64; + const leaf = { columnId: "sector", operator: "isEmpty" }; + const nest = (depth: number): unknown => + depth === 0 ? leaf : { op: "and", children: [nest(depth - 1)] }; + const queryAt = (depth: number) => ({ + filters: [nest(depth)], + sort: [], + rowGroups: [], + }); + + test("accepts a tree at the limit", () => { + expect(() => compile(queryAt(MAX_FILTER_TREE_DEPTH))).not.toThrow(); + }); + + test("rejects one level past the limit as a validation error, not a stack overflow", () => { + const caught = caughtFrom(queryAt(MAX_FILTER_TREE_DEPTH + 1)); + expect(caught).toBeInstanceOf(CompiledQueryValidationError); + expect(caught).toMatchObject({ code: "invalid-query" }); + expect((caught as CompiledQueryValidationError).path).toContain( + ".children[0]", + ); + }); + + test("a tree deep enough to overflow the stack never reaches equality", () => { + // The dangerous window: capture used to SUCCEED here and the RangeError + // surfaced later, from equality on a plan the engine had accepted. + const caught = caughtFrom(queryAt(5_000)); + expect(caught).toBeInstanceOf(CompiledQueryValidationError); + }); +}); diff --git a/packages/row-model/src/__tests__/types.test.ts b/packages/row-model/src/__tests__/types.test.ts index 59a86d883..7719a004a 100644 --- a/packages/row-model/src/__tests__/types.test.ts +++ b/packages/row-model/src/__tests__/types.test.ts @@ -11,6 +11,8 @@ import { type PretableDerivationTransition, type PretableDerivationsFor, type PretableExpansionDefault, + type PretableFilterFor, + type PretableFilterGroupFor, PretableDisposedModelError, type PretableGroupId, type PretableGroupKey, @@ -447,6 +449,92 @@ const badValueQuery: PretableQueryFor = { void badOperatorQuery; void badValueQuery; +// --- filter tree (SP2a) type probes --------------------------------------- +// +// `PretableFilterFor` is a distributive conditional type; widening the union +// it feeds can silently collapse it (or the new group type) to `never`, which +// would make every downstream narrowing probe vacuously green. These probes +// pin both halves as inhabited before anything else is asserted. +type IsNever = [T] extends [never] ? true : false; + +type _leafUnionIsInhabited = Expect< + Equal>, false> +>; +type _groupTypeIsInhabited = Expect< + Equal>, false> +>; +type _filterNodeIsInhabited = Expect< + Equal["filters"][number]>, false> +>; + +type FilterNode = PretableQueryFor["filters"][number]; + +// Assignability in both directions: the node slot accepts a leaf and a group, +// and neither half alone is the whole slot. +type _leafAssignableToNode = Expect< + [PretableFilterFor] extends [FilterNode] ? true : false +>; +type _groupAssignableToNode = Expect< + [PretableFilterGroupFor] extends [FilterNode] ? true : false +>; +type _nodeIsNotOnlyLeaf = Expect< + Equal< + [FilterNode] extends [PretableFilterFor] ? true : false, + false + > +>; +type _nodeIsNotOnlyGroup = Expect< + Equal< + [FilterNode] extends [PretableFilterGroupFor] + ? true + : false, + false + > +>; + +const treeQuery: PretableQueryFor = { + filters: [ + { columnId: "quantity", operator: "gte", value: 4 }, + { + op: "or", + children: [ + { columnId: "sector", operator: "contains", value: "tech" }, + { + op: "and", + children: [ + { columnId: "quantity", operator: "between", value: [1, 2] }, + { columnId: "sector", operator: "isEmpty" }, + ], + }, + ], + }, + ], + sort: [], + rowGroups: [], +}; +const badGroupOperatorQuery: PretableQueryFor = { + filters: [ + // @ts-expect-error a filter group joins with "and" or "or", never "nor" + { op: "nor", children: [] }, + ], + sort: [], + rowGroups: [], +}; +const badGroupChildQuery: PretableQueryFor = { + filters: [ + { + op: "and", + // @ts-expect-error a group child is still a typed leaf + children: [{ columnId: "quantity", operator: "contains", value: 4 }], + }, + ], + sort: [], + rowGroups: [], +}; +void treeQuery; +void badGroupOperatorQuery; +void badGroupChildQuery; + declare const model: PretableRowModel; type _row = Expect, Holding>>; type _rowId = Expect, number>>; diff --git a/packages/row-model/src/column-types.ts b/packages/row-model/src/column-types.ts index 5c0eec336..1538b8431 100644 --- a/packages/row-model/src/column-types.ts +++ b/packages/row-model/src/column-types.ts @@ -531,6 +531,45 @@ export type PretableFilterFor = : never : never; +/** + * A branch of the filter tree: a join operator and the nodes it joins. A group + * may hold leaves, further groups, or nothing at all, and may nest. + * + * An `and` group holds when every child holds; an `or` group holds when any + * child does. An EMPTY group holds under both operators — it constrains + * nothing, so a group still being assembled never removes rows. + * @public + */ +export interface PretableFilterGroupFor { + readonly op: "and" | "or"; + readonly children: readonly PretableFilterNodeFor[]; +} + +/** + * One node of the filter tree: either a typed leaf or a group of nodes. + * @public + */ +export type PretableFilterNodeFor = + PretableFilterFor | PretableFilterGroupFor; + +/** + * Narrows one filter-tree node to a group. Checked positively on the group's + * own fields, so an unknown shape fails closed rather than being treated as a + * branch with no children. + * @public + */ +export function isPretableFilterGroup( + node: PretableFilterNodeFor, +): node is PretableFilterGroupFor { + return ( + typeof node === "object" && + node !== null && + "children" in node && + ((node as { op: unknown }).op === "and" || + (node as { op: unknown }).op === "or") + ); +} + /** @public */ export type PretableSortFor = Prettify< (TColumns extends readonly (infer TColumn)[] @@ -564,7 +603,7 @@ export type PretableRowGroupFor = Prettify< /** @public */ export interface PretableQueryFor { - readonly filters: readonly PretableFilterFor[]; + readonly filters: readonly PretableFilterNodeFor[]; readonly sort: readonly PretableSortFor[]; readonly rowGroups: readonly PretableRowGroupFor[]; } diff --git a/packages/row-model/src/compiled-query.ts b/packages/row-model/src/compiled-query.ts index c42e3559f..5ed9d4b9d 100644 --- a/packages/row-model/src/compiled-query.ts +++ b/packages/row-model/src/compiled-query.ts @@ -254,6 +254,117 @@ interface RuntimeFilter { readonly value?: unknown; } +interface RuntimeFilterGroup { + readonly op: "and" | "or"; + readonly children: readonly RuntimeFilterNode[]; +} + +type RuntimeFilterNode = RuntimeFilter | RuntimeFilterGroup; + +/** + * The internal twin of `isPretableFilterGroup`, deliberately WEAKER: the + * public guard re-checks `op` because it runs on whatever a caller hands it, + * whereas capture has already rejected any node carrying `children` without a + * valid `op`. Over a captured tree the presence of `children` is therefore + * decisive on its own. Never call this on un-captured input. + */ +function isRuntimeFilterGroup( + node: RuntimeFilterNode, +): node is RuntimeFilterGroup { + return "children" in node; +} + +/** + * The leaves of a filter tree, in depth-first order — the tree's COLUMN + * DEPENDENCY set, flattened deliberately. Join operators are irrelevant here: + * a column is read if any leaf anywhere in the tree mentions it, whatever + * joins that leaf to its siblings. Evaluation does NOT go through this — see + * `compileFilterNodes`. + */ +function filterLeavesOf( + nodes: readonly RuntimeFilterNode[], +): readonly RuntimeFilter[] { + const leaves: RuntimeFilter[] = []; + const visit = (node: RuntimeFilterNode): void => { + if (isRuntimeFilterGroup(node)) node.children.forEach(visit); + else leaves.push(node); + }; + nodes.forEach(visit); + return leaves; +} + +/* + * A filter node compiled for evaluation: a closure answering that node's + * question about one row. The whole tree collapses into a nest of these at + * construction — column lookups resolved, operands normalized, joins baked + * in — so a verdict never inspects a `RuntimeFilterNode` and never branches + * on a node kind. A leaf and a group are the same callable to their parent, + * which is what lets ONE evaluation path serve a grouped query and a flat + * one without either paying for the other. + */ +type CompiledFilterMatcher = ( + valueOf: (columnId: string) => unknown, +) => boolean; + +/* + * Its own const rather than the `FilterPredicate` twin a thousand lines down: + * the two are structurally identical and tsc would accept either, but a + * predicate answers about a VALUE and a matcher about a ROW, and borrowing + * one for the other is a pun a reader has to unpick. + */ +const alwaysMatches: CompiledFilterMatcher = () => true; + +/** + * Compiles a sibling list joined by `op` into a single matcher. Used for + * groups and for the query's root list alike — the roots are an `and`, which + * is exactly what a top-level filter list has always meant. + * + * The join loops are indexed rather than `every`/`some`, which is not a style + * preference: a callback join allocates a closure per group PER ROW, on the + * hottest loop in the package. Deliberately unquantified — two harnesses + * measured the gap differently enough to disagree, and a figure pasted here + * would rot where no reader could re-derive it. Measure it yourself on an + * isolated verdict loop; a whole-model benchmark cannot resolve it. + * + * Both operators are written once here, so there is one implementation of + * `and` and one of `or` whatever shape the tree has. + */ +function compileFilterNodes( + nodes: readonly RuntimeFilterNode[], + op: "and" | "or", + byId: ReadonlyMap, +): CompiledFilterMatcher { + /* + * An EMPTY list is TRUE under BOTH joins — it constrains nothing, so it + * removes nothing. The branch is not decoration: the `or` loop below falls + * through to `false` on no children, which would make a half-built `or` + * group in a builder UI blank the grid the instant a user adds it and + * before they fill it in. + */ + if (nodes.length === 0) return alwaysMatches; + + const matchers = nodes.map((node) => { + if (isRuntimeFilterGroup(node)) + return compileFilterNodes(node.children, node.op, byId); + const { columnId } = node; + const predicate = compileFilterPredicate(node, byId.get(columnId)!); + return (valueOf: (columnId: string) => unknown) => + predicate(valueOf(columnId)); + }); + + if (op === "and") + return (valueOf) => { + for (let index = 0; index < matchers.length; index += 1) + if (!matchers[index](valueOf)) return false; + return true; + }; + return (valueOf) => { + for (let index = 0; index < matchers.length; index += 1) + if (matchers[index](valueOf)) return true; + return false; + }; +} + interface RuntimeOrdering { readonly columnId: string; readonly direction?: string; @@ -261,7 +372,7 @@ interface RuntimeOrdering { } interface RuntimeQuery { - readonly filters: readonly RuntimeFilter[]; + readonly filters: readonly RuntimeFilterNode[]; readonly sort: readonly RuntimeOrdering[]; readonly rowGroups: readonly RuntimeOrdering[]; } @@ -501,7 +612,7 @@ function captureQuery(source: object): RuntimeQuery { rawFilters, "query.filters", "filters must be an array", - captureFilter, + (entry, index) => captureFilterNode(entry, `query.filters[${index}]`), ), sort: captureDenseArray( rawSort, @@ -569,10 +680,62 @@ function captureDenseArray( return Object.freeze(captured); } -function captureFilter(raw: unknown, index: number): RuntimeFilter { - const path = `query.filters[${index}]`; +/* + * The deepest a captured filter tree may nest, counting root nodes as depth 0. + * + * Capture is the chokepoint: validation, snapshotting, descriptor keys, leaf + * collection, structural equality and per-row evaluation all recurse over a + * tree only AFTER it has been captured, so bounding it here bounds every one + * of them at once. Without the bound the failure mode was not merely a deep + * tree — measured against this file's pre-bound revision, a 1000-level tree + * CAPTURED cleanly and then overflowed the stack in `filterNodeListEqual` on + * the next recompile, so the `RangeError` surfaced from a later `setQuery` on + * a plan the engine had already accepted; at 2000 `compileQuery` threw a raw + * `RangeError` instead of this module's validation error. + * + * 64 is chosen as far beyond any tree a human or a builder UI produces (real + * filter trees nest a handful of levels) while sitting an order of magnitude + * below the depth at which any of the downstream recursions is at risk. + */ +const MAX_FILTER_TREE_DEPTH = 64; + +/** + * One node of the filter tree. A node carrying `children` is a group and is + * captured recursively, breadcrumbed as `.children[i]`; anything else is + * captured as a leaf. Every level is frozen on the way out, so the captured + * tree is owned by the plan rather than aliasing the caller's objects. + * + * `depth` is the node's own nesting level and is bounded — see + * `MAX_FILTER_TREE_DEPTH` for why the bound lives here and nowhere else. + */ +function captureFilterNode( + raw: unknown, + path: string, + depth = 0, +): RuntimeFilterNode { + if (depth > MAX_FILTER_TREE_DEPTH) + fail(`filter group nesting exceeds ${MAX_FILTER_TREE_DEPTH} levels`, path); if (raw === null || typeof raw !== "object") fail("filter entry is not an object", path); + const children = captureProperty(raw, "children", `${path}.children`); + if (children === undefined) return captureFilter(raw, path); + + const op = captureProperty(raw, "op", `${path}.op`); + if (op !== "and" && op !== "or") + fail("filter group must join with and or or", `${path}.op`); + return Object.freeze({ + op, + children: captureDenseArray( + children, + `${path}.children`, + "filter group children must be an array", + (entry, index) => + captureFilterNode(entry, `${path}.children[${index}]`, depth + 1), + ), + }); +} + +function captureFilter(raw: object, path: string): RuntimeFilter { const columnId = captureProperty(raw, "columnId", `${path}.columnId`); const contextId = typeof columnId === "string" ? columnId : undefined; const operator = captureProperty( @@ -740,12 +903,25 @@ function validateOrdering( } } +function validateFilterNode( + node: RuntimeFilterNode, + columns: ReadonlyMap, + path: string, +): void { + if (isRuntimeFilterGroup(node)) { + node.children.forEach((child, index) => + validateFilterNode(child, columns, `${path}.children[${index}]`), + ); + return; + } + validateFilter(node, columns, path); +} + function validateFilter( filter: RuntimeFilter, columns: ReadonlyMap, - index: number, + path: string, ): void { - const path = `query.filters[${index}]`; if (!filter || typeof filter !== "object") fail("filter entry is not an object", path); const column = resolveColumn(columns, filter.columnId, `${path}.columnId`); @@ -855,7 +1031,9 @@ function validateQuery( fail("filters, sort, and rowGroups must be arrays"); } const byId = new Map(columns.map((column) => [column.id, column])); - query.filters.forEach((filter, index) => validateFilter(filter, byId, index)); + query.filters.forEach((node, index) => + validateFilterNode(node, byId, `query.filters[${index}]`), + ); query.sort.forEach((entry, index) => validateOrdering(entry, byId, "sort", index), ); @@ -905,25 +1083,52 @@ function orderingEqual( function queryEqual(left: RuntimeQuery, right: RuntimeQuery): boolean { return ( - filtersEqual(left.filters, right.filters) && + filterNodeListEqual(left.filters, right.filters) && orderingEqual(left.sort, right.sort) && orderingEqual(left.rowGroups, right.rowGroups) ); } -function filtersEqual( - left: readonly RuntimeFilter[], - right: readonly RuntimeFilter[], +/* + * Nodes are matched STRUCTURALLY, never by a serialized key: the descriptor + * key is raw concatenation over unframed user operands, so a filter value can + * forge the separators and impersonate a sibling — harmless for the ordering + * job the key exists for, a wrong-results bug as an identity test (the plan + * would be reused and the incoming query silently discarded). + * + * Groups match when their join operators match and their children match as an + * unordered multiset, recursively — the same used-set shape the node list one + * level up uses, for the same reason: both joins are commutative. + */ +function filterNodeEqual( + left: RuntimeFilterNode, + right: RuntimeFilterNode, +): boolean { + if (isRuntimeFilterGroup(left) || isRuntimeFilterGroup(right)) { + return ( + isRuntimeFilterGroup(left) && + isRuntimeFilterGroup(right) && + left.op === right.op && + filterNodeListEqual(left.children, right.children) + ); + } + return ( + left.columnId === right.columnId && + left.operator === right.operator && + semanticValueEqual(left.value, right.value) + ); +} + +function filterNodeListEqual( + left: readonly RuntimeFilterNode[], + right: readonly RuntimeFilterNode[], ): boolean { if (left.length !== right.length) return false; const used = new Set(); return left.every((filter) => { const index = right.findIndex( (candidate, candidateIndex) => - !used.has(candidateIndex) && - filter.columnId === candidate.columnId && - filter.operator === candidate.operator && - semanticValueEqual(filter.value, candidate.value), + !used.has(candidateIndex) && filterNodeEqual(filter, candidate), ); if (index < 0) return false; used.add(index); @@ -931,15 +1136,22 @@ function filtersEqual( }); } +/* + * `filterLeaves` is the caller's own `#filterLeaves`, passed rather than + * re-derived: this runs on every recompile check (`semanticallyMatches`, so + * every `setQuery`), and walking the tree here would allocate a fresh leaf + * array per call for a set the plan already holds. + */ function derivationsEqualForPlan( left: readonly RuntimeColumn[], right: readonly RuntimeColumn[], query: RuntimeQuery, + filterLeaves: readonly RuntimeFilter[], ): boolean { if (left.length !== right.length) return false; const accessorIds = new Set(); const comparatorIds = new Set(); - query.filters.forEach((entry) => accessorIds.add(entry.columnId)); + filterLeaves.forEach((entry) => accessorIds.add(entry.columnId)); query.sort.forEach((entry) => { accessorIds.add(entry.columnId); comparatorIds.add(entry.columnId); @@ -1125,15 +1337,29 @@ function snapshotQuery( path: string, canonicalFilters = false, ): RuntimeQuery { - const filters = query.filters.map((filter, index) => - Object.freeze({ - ...filter, - value: cloneOwnedValue( - filter.value, - `${path}.filters[${index}].value`, - new WeakSet(), - ), - }), + const snapshotNode = ( + node: RuntimeFilterNode, + nodePath: string, + ): RuntimeFilterNode => + isRuntimeFilterGroup(node) + ? Object.freeze({ + op: node.op, + children: Object.freeze( + node.children.map((child, index) => + snapshotNode(child, `${nodePath}.children[${index}]`), + ), + ), + }) + : Object.freeze({ + ...node, + value: cloneOwnedValue( + node.value, + `${nodePath}.value`, + new WeakSet(), + ), + }); + const filters = query.filters.map((node, index) => + snapshotNode(node, `${path}.filters[${index}]`), ); if (canonicalFilters) filters.sort(compareFilterDescriptors); return Object.freeze({ @@ -1145,7 +1371,7 @@ function snapshotQuery( }); } -const EMPTY_FILTERS = Object.freeze([]) as readonly RuntimeFilter[]; +const EMPTY_FILTERS = Object.freeze([]) as readonly RuntimeFilterNode[]; const EMPTY_SORT = Object.freeze([]) as RuntimeQuery["sort"]; /** @@ -1176,14 +1402,23 @@ function canonicalRuntimeQuery( } function compareFilterDescriptors( - left: RuntimeFilter, - right: RuntimeFilter, + left: RuntimeFilterNode, + right: RuntimeFilterNode, ): number { return filterDescriptorKey(left).localeCompare(filterDescriptorKey(right)); } -function filterDescriptorKey(filter: RuntimeFilter): string { - return `${filter.columnId}\u0000${filter.operator}\u0000${filterValueKey(filter.value)}`; +function filterDescriptorKey(node: RuntimeFilterNode): string { + if (isRuntimeFilterGroup(node)) { + // Children are keyed then sorted for the same reason the roots are + // sorted in `canonicalRuntimeQuery`: both joins are commutative, so a + // reordered group is the same question and must reuse the same plan. + return `group\u0000${node.op}\u0000[${node.children + .map(filterDescriptorKey) + .sort() + .join("\u0001")}]`; + } + return `${node.columnId}\u0000${node.operator}\u0000${filterValueKey(node.value)}`; } function filterValueKey(value: unknown): string { @@ -1455,10 +1690,26 @@ class CompiledQueryPlan readonly #runtimeColumns: readonly RuntimeColumn[]; readonly #runtimeQuery: RuntimeQuery; readonly #byId: ReadonlyMap; - // Parallel to `#runtimeQuery.filters`: one compiled predicate per filter, - // built once at construction so no verdict ever re-normalizes operands or - // re-resolves columns per row. - readonly #compiledPredicates: readonly FilterPredicate[]; + /* + * The LEAVES of `#runtimeQuery.filters`, depth-first — not the filter list + * itself, which under a tree holds groups and has a different length. This + * is a DEPENDENCY set (which columns the filters read), never the evaluation + * order: joins are honoured by `#compiledFilterTree`. + */ + readonly #filterLeaves: readonly RuntimeFilter[]; + /* + * The whole filter tree as ONE closure, built at construction so no verdict + * re-normalizes an operand, re-resolves a column, or reads a join per row. + * + * Built the same way for EVERY query, grouped or flat. A flat query briefly + * had a second, separate predicate array and loop of its own; that bought a + * real cost (a grouped query compiled every leaf predicate twice) and the + * standing risk of two implementations of one semantics drifting apart, + * catchable only after the fact. It bought no speed either: the flat loop + * it saved allocated a closure per row of its own, so the single path is + * measurably the FASTER of the two on an isolated verdict loop. + */ + readonly #compiledFilterTree: CompiledFilterMatcher; readonly #active: readonly RuntimeColumn[]; readonly #aggregateColumns: readonly RuntimeColumn[]; readonly #operation: "set-query" | "set-derivations"; @@ -1490,6 +1741,7 @@ class CompiledQueryPlan this.#runtimeColumns, derivations, this.#runtimeQuery, + this.#filterLeaves, ) && queryEqual(this.#publicQuery, query), }; @@ -1529,13 +1781,14 @@ class CompiledQueryPlan this.#byId = new Map( this.#runtimeColumns.map((column) => [column.id, column]), ); - this.#compiledPredicates = this.#runtimeQuery.filters.map((filter) => - compileFilterPredicate(filter, this.#byId.get(filter.columnId)!), + this.#filterLeaves = filterLeavesOf(this.#runtimeQuery.filters); + this.#compiledFilterTree = compileFilterNodes( + this.#runtimeQuery.filters, + "and", + this.#byId, ); const activeIds = new Set(); - this.#runtimeQuery.filters.forEach((entry) => - activeIds.add(entry.columnId), - ); + this.#filterLeaves.forEach((entry) => activeIds.add(entry.columnId)); this.#runtimeQuery.rowGroups.forEach((entry) => activeIds.add(entry.columnId), ); @@ -1699,19 +1952,32 @@ class CompiledQueryPlan } /* - * The one filter-predicate loop, parameterized over the value source the - * same way `#finalizeMetadata` is: `evaluate` supplies its collected value - * map, the verdict-only path supplies live accessor reads. Predicate - * semantics live in `compileFilterPredicate`, applied here through the - * construction-time `#compiledPredicates` array (parallel to - * `#runtimeQuery.filters`) — no `#byId` lookup and no operand - * re-normalization per row. + * The ONE filter verdict, parameterized over the value source the same way + * `#finalizeMetadata` is: `evaluate` supplies its collected value map, the + * verdict-only path supplies live accessor reads. Predicate semantics live + * in `compileFilterPredicate` and join semantics in `compileFilterNodes`, + * both reached through the construction-time `#compiledFilterTree` — no + * `#byId` lookup and no operand re-normalization per row. + * + * The ROOT list joins conjunctively, as it always has — a query's top-level + * filters all have to hold. Below the roots, groups join by their own `op`. */ #filterVerdict(valueOf: (columnId: string) => unknown): boolean { - const filters = this.#runtimeQuery.filters; - return this.#compiledPredicates.every((predicate, index) => - predicate(valueOf(filters[index].columnId)), - ); + return this.#compiledFilterTree(valueOf); + } + + /** + * The plan's OWN captured filter tree — the objects `captureFilterNode` + * produced, not the re-frozen copy the `query` getter hands out. + * @internal + */ + static capturedFilterTreeForTesting( + plan: CompiledQuery, + ): readonly unknown[] { + if (!(plan instanceof CompiledQueryPlan)) { + throw new TypeError("Captured filters require a compiled query plan."); + } + return plan.#publicQuery.filters; } /** @@ -2069,14 +2335,16 @@ class CompiledQueryPlan previous.#runtimeColumns, next.#runtimeColumns, previous.#runtimeQuery, + previous.#filterLeaves, ) && derivationsEqualForPlan( previous.#runtimeColumns, next.#runtimeColumns, next.#runtimeQuery, + next.#filterLeaves, ) ); - const filtersChanged = !filtersEqual( + const filtersChanged = !filterNodeListEqual( previous.#runtimeQuery.filters, next.#runtimeQuery.filters, ); @@ -2173,6 +2441,18 @@ export function compareRecordRows( ); } +/** + * The plan's OWN captured filter tree — the objects `captureFilterNode` + * produced, not the re-frozen copy the `query` getter hands out. Exists so + * capture-level invariants (freezing, ownership) are assertable at all. + * @internal + */ +export function getCapturedFilterTreeForTesting( + plan: CompiledQuery, +): readonly unknown[] { + return CompiledQueryPlan.capturedFilterTreeForTesting(plan); +} + /** * Orders two rows by keys the caller already resolved (via `sortKeysOf` or * `fillSortKeysFromPrevious`) — no store lookups. Exists so O(n log n) sorts diff --git a/packages/row-model/src/distinct-values.ts b/packages/row-model/src/distinct-values.ts index 33c8ba9a5..7202270db 100644 --- a/packages/row-model/src/distinct-values.ts +++ b/packages/row-model/src/distinct-values.ts @@ -1,4 +1,4 @@ -import type { PretableRowId } from "./column-types"; +import { isPretableFilterGroup, type PretableRowId } from "./column-types"; import { runCooperativeTransitionSlice, type CooperativeTransitionRuntime, @@ -421,23 +421,42 @@ function filterSemanticKey< const derivations = root.queryPlan .derivations as unknown as readonly RuntimeColumn[]; const byId = new Map(derivations.map((column) => [column.id, column])); - return [...root.queryPlan.query.filters] - .map((filter) => { - const runtime = filter as { - readonly columnId: string; - readonly operator: string; - readonly value?: unknown; + const nodeKey = (node: unknown): string => { + if (isPretableFilterGroup(node as never)) { + const group = node as { + readonly op: string; + readonly children: readonly unknown[]; }; + /* + * Children are keyed then sorted for the same reason the roots are: + * and/or are commutative, so reordering a group is not a new question. + * + * Belt and braces today — deleting the sort breaks no test, because a + * reordered group is matched structurally by `filterNodesEqual` and so + * reuses its plan, and this cache never sees a root whose tree differs + * only in child order. It stops being redundant the moment plan reuse + * stops absorbing that case. + */ return frame( - "f", - frame("c", runtime.columnId) + - frame("i", String(identityId(byId.get(runtime.columnId)?.accessor))) + - frame("p", runtime.operator) + - frame("v", semanticValueKey(runtime.value)), + "g", + frame("p", group.op) + + frame("k", group.children.map(nodeKey).sort().join("")), ); - }) - .sort() - .join(""); + } + const runtime = node as { + readonly columnId: string; + readonly operator: string; + readonly value?: unknown; + }; + return frame( + "f", + frame("c", runtime.columnId) + + frame("i", String(identityId(byId.get(runtime.columnId)?.accessor))) + + frame("p", runtime.operator) + + frame("v", semanticValueKey(runtime.value)), + ); + }; + return [...root.queryPlan.query.filters].map(nodeKey).sort().join(""); } function columnForDerivations(