feat: filters become an AND/OR tree (tool panel SP2a) - #493
Merged
Conversation
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`PretableQueryFor.filters` becomes an arbitrary-depth tree: each entry is either a typed leaf or a `PretableFilterGroupFor` joining its children with "and" or "or". `isPretableFilterGroup` narrows one node, checked positively on the group's own fields so an unknown shape fails closed. Capture is now recursive: `captureFilterNode` validates the join operator, requires a dense array of children, breadcrumbs failures as `query.filters[i].children[j]…`, and freezes every level. Validation recurses with the same breadcrumb, so a bogus column or operator inside a group is rejected at compile time rather than at evaluation. Evaluation still applies every leaf conjunctively (`filterLeavesOf`) and group equality is a conservative descriptor-key match — real tree semantics land in the next change. Flat queries are byte-for-byte unaffected. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…hat tests capture `filterSemanticKey` cast every filter to a leaf, so a group keyed as three `undefined`s: two queries differing only INSIDE a group collided on one `population: "filtered"` cache entry and the second was served the first one's answer. The key now recurses, sorting each group's child keys the way `canonicalRuntimeQuery` sorts the roots — both joins are commutative, so a reordered group is the same question. `filterDescriptorKey` sorts children for the same reason, which also lets a reordered group reuse its plan. The deep-freeze test read `plan.query`, which re-freezes everything it hands back: it proved `snapshotQuery` froze a copy and said nothing about capture, and passed with the capture-level freeze deleted. It now reads the plan's own captured tree through an internal test seam, with the snapshot half split off into its own test. Also corrects two comments that still called the compiled predicates parallel to `#runtimeQuery.filters` — under a tree they are parallel to `#filterLeaves` — and marks the temporary conjunctive flattening at both sites. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
… forge a sibling The group arm of `filterNodesEqual` compared serialized descriptor keys, and that key is raw concatenation over unframed user operands. A filter VALUE could reproduce the separators and impersonate a sibling: an `and` group of `contains "a"` and `contains "b"` keyed identically to a one-child group whose operand was `a<US>sector<NUL>contains<NUL>string:b`. The keys matched, so `filtersEqual` and then `semanticallyMatches` did too, `compileQuery` handed back the PREVIOUS plan, and the incoming query was silently discarded with the old filters left applied. Groups now match structurally — join operator, then children as an unordered multiset through `filtersEqual`, recursing for nested groups. That is the comparison the tree needs anyway, brought forward rather than patched around. `filterDescriptorKey` keeps its original ordering job, where the ambiguity is harmless. Also documents why the child-key sort in `distinct-values`'s `nodeKey` is belt-and-braces: structural plan reuse absorbs a reordered group before this cache can see one, and the sort stops being redundant if that changes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…on-free recompile check Three comment defects this branch introduced. Two new functions had been inserted BETWEEN an existing doc block and the function it documented, so `filterVerdict`'s paragraph described the test seam and `compareWithSortKeys` was left undocumented under a stacked pair; both are moved clear. `isRuntimeFilterGroup` claimed the "same positive check" as the public guard when it is deliberately weaker, and now says why: capture has already rejected any node carrying `children` without a valid `op`, so over a captured tree `children` alone is decisive. `PretableFilterGroupFor`'s public TSDoc read as a finished feature, and now carries the one line saying `op: "or"` is accepted and validated but still evaluated as `and`. `PretableFilterNodeFor` names the leaf-or-group union that was spelled out at three sites and reinvented locally in the tests. The near-identical `filtersEqual`/`filterNodesEqual` pair becomes `filterNodeListEqual` and `filterNodeEqual`. `derivationsEqualForPlan` now takes the caller's `#filterLeaves` instead of re-deriving them: it runs on every recompile check, where the pre-tree code allocated nothing. `filterLeavesOf` is once again called exactly once per plan, in the constructor. Also drops a guard ternary equivalent to the equality checks it wrapped, drops an object check `validateFilter` already performs, corrects two wording drifts (captured vs published, leaf list vs node list), extracts a model factory in the distinct-values group tests, and turns a bare assertion loop into `test.each` so a failure names the shape. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Task 1 accepted, validated and compared filter groups but flattened them for evaluation, so an `op: "or"` group behaved as an `and`. Evaluation now walks the join structure: the root list stays conjunctive, `and` groups hold when every child holds, `or` groups when any child does, recursively. An EMPTY group is TRUE under both joins, by an explicit branch rather than by accident: `some([])` is `false`, which would blank the grid the moment a builder UI adds a group and before the user fills it in. The tree is compiled ONCE per plan into closures — the same treatment the flat leaf predicates always got — and a query with no groups at all keeps the unchanged flat loop, so the per-row hot path is untouched where there is no join to honour. Capture now bounds nesting at 64 levels. Capture is the chokepoint every other recursion runs downstream of, and without the bound a ~1000-level tree was CAPTURED successfully and then overflowed the stack later, inside equality on a plan the engine had already accepted; past ~2000 levels `compileQuery` threw a raw RangeError instead of this module's validation error. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Evaluation briefly kept two implementations: a compiled tree for queries holding groups, and the older flat predicate array for queries that did not. Two implementations of one semantics is a divergence waiting to happen, and the second one was already costing something concrete — a grouped query compiled every leaf predicate twice, once into the flat array it then never read. There is now one representation. Every node, leaf or group, compiles to a `CompiledFilterMatcher` closure; a sibling list compiles to one matcher for its join; the query's root list is that same call with `and`, which is what a top-level filter list has always meant. `#compiledPredicates` and the flat-vs-tree fork are gone. `#filterLeaves` stays — it is the column dependency set, not an evaluation order. The join loops are indexed rather than `every`/`some`, and that detail is the whole perf story. A callback join allocates a closure per group per row: on an isolated 200k-row four-leaf verdict loop the callback form measured 52ms against the old flat loop's 30ms, and the indexed form measures 30ms — level with the code it replaces, and level again on a 100k-row model build where the sign flips between rounds. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…er-tree comments Two comments pointed at `compileFilterNode` and `evaluateCompiledFilterNode`, neither of which exists — the collapse to one evaluation path merged both into `compileFilterNodes`, and one of the two stale pointers was written by that same commit rather than inherited from before it. The indexed-join comment carried hard figures for the closure-allocation cost it explains. Two harnesses disagreed about that pair by enough to matter, and the difference is invisible to a whole-model benchmark, so the figures come out and the instruction to measure on an isolated verdict loop goes in. The mechanism stands; only the numbers were unsafe to write down. The same block claimed the single path runs "level with" the flat loop it replaced. It does not: the flat loop was itself a callback join allocating a closure per row, so collapsing is a net win, not a wash. Said accurately now. Also gives the matcher tree its own `alwaysMatches` rather than borrowing the `FilterPredicate` twin from a thousand lines away — structurally identical, so tsc never minded, but a predicate answers about a value and a matcher about a row. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…led state `query.filters` became an AND/OR tree in the row model; the surface's chrome was still reading it as a flat list of leaves, through a cast that would have keyed every group element under `undefined`. The per-column record projection is deleted. `snapshot.filters` is now the query's array verbatim — leaves and groups, nested — and the two questions the chrome actually asks are answered by walks in the new `./filter-tree`: - `columnHasFilter` lights a funnel on any occurrence of a column at any depth, because a filter nested in a group still removes that column's rows; - `topLevelColumnFilter` / `withTopLevelColumnFilter` scope the column menu to its top-level leaf, read and write. A commit replaces that leaf in place and passes every other element through by reference, so a group the menu never authored survives byte-identical. `LabeledGridSurface`'s `is-filtered` decoration walks the tree by the same occurrence rule. `isPretableFilterGroup` and the node/group types are re-exported from `@pretable/core` and `@pretable/react` — the surface needs the guard, and so does any consumer reading `onQueryChange`'s filters. This is the first commit on the branch where `pnpm build` succeeds again. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ce teeth for the survives-test Three review findings from the surface filter-tree commit. `@pretable/core` gains three public exports and a widened `PretableQueryFor.filters` in that commit but had no changeset. It has one now, and it names the two rules a CHANGELOG reader has to learn before upgrading: an EMPTY group holds for both `and` and `or` (naive algebra disagrees, and would blank the grid on a half-built group), and a tree nesting deeper than 64 levels is rejected by `compileQuery` with `code: "invalid-query"` — a new reason an otherwise well-formed query can be refused. The docs fixture server's `DocsQuery.filters` is leaf-only while the engine's is a tree, and nothing catches it: the type boundary is severed by `JSON.stringify` in each example's `fetch-rows.ts`, so typecheck is green over a real gap. It carries a verdict comment now, including the part that is better than it looks — a group reaches `columnTypeFor(undefined)`, which throws, so the route fails loudly rather than serving wrongly-filtered rows. The survives-test asserted only structural equality of the pass-through group, which a defensive clone would have satisfied. `setQuery` in `pretable-model.ts` hands `onQueryChange` the surface's own object before the row model re-captures it, so the group element IS assertable by reference — and now is. Cloning the group in `withTopLevelColumnFilter` previously failed three helper tests and left all eighteen surface tests green; it now fails the surface test too. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`query.filters` is an AND/OR tree as of this branch, and the shape a server receives changed the moment it merges — so it is documented now, not alongside the builder UI that will produce trees from the UI. The section overview grows "What a filter looks like on the wire": a grouped JSON payload, and the four rules that make it mean what it says — the top-level array is an implicit AND (which is what a filter list has always meant, and what keeps a pre-group payload valid), leaves and groups discriminate on structure rather than a tag (`isPretableFilterGroup` on the client edge, `children` on the server, where no types survive `JSON.stringify`), an empty group is TRUE under either `op` (naive algebra says empty-OR is false, which would blank the grid under a half-built group), and nesting is bounded at 64 with a breadcrumbed `invalid-query` rejection. Then the part a server cannot leave implicit: reject, flatten only when every join is `and`, or implement the recursion — presented with what each one actually costs, because guessing produces the failure this section keeps warning about, a grid that looks filtered and is not. Says plainly that no UI builds a group yet: the funnel still writes its column's top-level leaf, and a group reaches an endpoint only from a query the consumer seeded themselves. The docs' own example server now REJECTS a group explicitly rather than incidentally. It already failed — a group has no `columnId`, so `columnTypeFor` threw — but with "Unknown column undefined", a message about the wrong thing. `matches()` tests for `children` and names the real reason, which is what lets the page cite it as the reject posture and what the page's own rule (a fixture that cannot answer a filter says which) already demanded. No group evaluation: it is a demo of the wire contract, not a filter engine. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…sure helper Three semantics moved silently when the per-column record projection was deleted, and none of them was written down. The record was LAST-wins: `filters[entry.columnId] = …` let a later top-level leaf overwrite an earlier one. `topLevelColumnFilter` takes the FIRST, and `withTopLevelColumnFilter` replaces that same one and drops the rest — the two halves agreed at runtime, but only the write side said so. The read side's doc said "the TOP-LEVEL leaf", singular and definite, which left the duplicate case undefined in prose. It says FIRST now, names the change of answer, and both rules have a test: nothing previously constructed two top-level leaves for one column, so neither first-wins nor the duplicate-drop branch was exercised outside the clearing path. The changeset carries the clause too — only a hand-authored `filters` can reach it, but a consumer who does deserves to read it there rather than discover it. Two pieces of duplication go with them. `LabeledGridSurface`'s walk — a second walk for a real reason, since it gates on `isColumnFilterActive` — narrowed through four raw casts because the guard was private; it is exported as `isSurfaceFilterGroup`, and the `as never` rationale now lives in one place. The value-erasure cast was spelled out at three call sites with three cross-referencing comments; `asSurfaceNodes` holds the single explanation and each site keeps only its own tree-semantics note, which is the part that differs. Also corrects the core changeset's depth bound, which was off by one: the check is `depth > 64` with the root at depth 0, so 65 node levels are accepted. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…e promises
The overview says the fixture endpoint errors "the moment a group
appears". It did not. The rejection lived inside `matches()`, which
`applyDocsQuery` calls per ROW inside `rows.filter(...)`, so it was
reachable only when some row survived every earlier leaf. Two shapes
therefore answered 200 with zero rows and no throw:
filters: [{ region isAnyOf ["Nowhere"] }, { op: "or", children: [] }]
any group at all over an empty `rows`
Zero rows and an error are not the same answer. The first reads as
"nothing matched" — a result quietly computed from less than the reader
asked for, which is the failure this whole section argues against, and
it does not stop being that because the result happens to be empty.
So `applyDocsQuery` now scans `filters` for `children` before it reads a
row. Whether a query is one this fixture can answer is a question about
the QUERY, independent of the data, and it is asked once, where that is
true. `matches()` keeps its branch as belt-and-braces for a direct
caller, and both throw through one `rejectFilterGroup` so the wording
cannot drift; the up-front path also breadcrumbs the offending index.
Four tests pin it, including the two shapes that used to slip through
and a positive control that leaf-only queries over the same shapes still
answer. Removing the scan fails exactly the three group tests.
Two staleness fixes alongside:
- `server-data.types.tsx` said index.mdx has "both of its fences"; the
grouped payload made it three. The page is still deliberately
unbound — all three fences are JSON.
- `query-ownership.mdx`'s See-also had two bullets titled "Server-side
data" pointing at the same page. Mine is now "The filter wire
contract".
The Reject bullet on the page now describes the up-front scan and says
why the placement, not just the check, is the part worth copying.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
Contributor
Vercel preview readyPreview: https://pretable-nr4by2wby-cacheplane.vercel.app Updated automatically by the |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this is
Filters become an arbitrary-depth AND/OR tree, so a query can express
price > 10 AND (status isAnyOf [a,b] OR owner contains "x"). This is SP2a — the engine prerequisite for the tool panel's filter-builder section (SP2b), split engine-first for the same reason SP1 split out column visibility: the UI must compose against a model that exists.Spec:
docs/superpowers/specs/2026-08-25-filter-tree-design.md. Plan:docs/superpowers/plans/2026-08-25-filter-tree-sp2a.md.No UI builds groups yet. The per-column funnel still writes top-level leaves; groups reach the engine only from a controlled
queryprop, a URL, or a saved view. The builder is SP2b.The shape
The top level stays an implicit AND array, so every existing call site kept compiling and simple cases stay one-liners. Rejected alternatives: a single root group (ceremony on every simple consumer forever) and a parallel
filterTreefield (two sources of truth — the declared-but-read-by-nothing failure this repo keeps paying for).isPretableFilterGroupships so consumers never hand-roll the discrimination.Two semantics worth knowing
An empty group evaluates TRUE, under both operators. The naive algebra says empty-OR is false — which would let a half-built group in SP2b's builder blank the entire grid mid-edit. Identity-true is the product-safe convention, stated in the TSDoc, pinned by tests, and named in the changeset.
Nesting is bounded at 64 levels below the root, rejected by
compileQuerywithcode: "invalid-query"and aquery.filters[i].children[j]…breadcrumb. This closes a measured hazard: before the bound, depth 1000 captured successfully and then blew the stack inside equality on a latersetQuery— a crash in the steady-state path on a plan the engine had already accepted. Capture is the single chokepoint every other recursion runs downstream of, so one bound there protects validate, snapshot, equality, and evaluation at once.Engine
Capture, validate, snapshot, evaluation, equality, and
distinctValuesall recurse. Evaluation compiles the tree once per plan into a matcher closure — there is exactly oneandimplementation and oneor, whatever the tree's shape, and no separate flat path.That single path is also ~12% faster than the flat loop it replaces, because the old loop was itself a callback join allocating a closure per row. Getting that right took disproving my own instruction: I asked for the collapse on the grounds that the cost was noise, based on two full-model benchmarks. An isolated verdict loop (0.6% A/A floor, versus the full-model bench's 12%) showed the naive collapse was 4× the noise floor, a bisect proved the cost was in evaluation rather than the extra compile, and the culprit turned out to be
every/someallocating per group per row — replaced with indexed loops. A variant that fused leaf-only sibling lists measured faster still and was rejected, because it put join semantics in a second place.Surface
snapshot.filtersis now the query array verbatim — the per-column record projection is gone (it would have keyed a group underundefined). Funnels light on a column's occurrence at any depth. The per-column menu reads and writes only its column's first top-level leaf and passes groups through by reference, pinned by a reference-identity assertion atonQueryChange, before the model re-captures.One behavior moved and is documented: the deleted record was last-wins for a duplicate top-level leaf; the tree path is first-wins, and a commit collapses duplicates to one. Only a consumer hand-authoring
filterscan reach it.Docs
The server-data page documents the wire contract: groups arrive verbatim, how to discriminate them server-side (where
JSON.stringifyhas stripped the types), and that a server which only understands flat filters has to decide explicitly — reject / flatten-only-when-every-join-is-and/ implement the recursion — with the flatten trap called out (the else-branch must still reject). The docs' own example server now rejects a group up front by name, because the page already promised an unanswerable filter returns an error saying which; its previous per-row rejection was reachable only if a row survived the leaves ahead of it, so a leaf matching nothing quietly answered 200 with zero rows.Review
Six review rounds across the tasks, ~25 mutations. Wrong-results defects found and fixed before merge, none of which the green suite caught:
distinct-valuescache key (unchecked leaf cast), serving stale values — reproduced, then fixed and pinned.�/`` could impersonate an entire sibling, making two different groups compare equal so the plan was reused and the incoming query silently discarded. Fixed by comparing structurally instead of by key.Verification
Changesets:
@pretable/coreminor,@pretable/reactminor. Norow-model.api.mdexists — row-model is private, so the group types surface throughcore.api.mdandreact.api.mdonly. Every temporary marker from the intermediate flattening state is gone (swept and confirmed).Follow-up filed
external-filter-authority.test.tsx:214asserts synchronously on header state that settles a slice later. It fails 5/5 in isolation and passes only because earlier tests in the file warm the path — the real cause of failures previously written off as load flake. Pre-existing onmain, so it doesn't block this; filed with the one-word fix and the instruction to check siblings for the same shape.🤖 Generated with Claude Code