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

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 42 additions & 0 deletions .changeset/filter-operator-parity-discrimination-3641.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
---
---

Test-only change, no behaviour and no authoring-surface change (objectui#3641).

Two filter-operator parity tests had lost their discriminating power to upstream
vocabulary growth. Both rested on the same pivot — is this operator a member of
`VALID_AST_OPERATORS`? — and that set is derived upstream from `AST_OPERATOR_MAP`
(`new Set(Object.keys(...))`). It grew until it spelled all 19 canonical
`VIEW_FILTER_OPERATORS` verbatim, `before` and `after` among them, which are the
exact pair whose missing bridge entries caused the silent full-table read these
tests were written for (objectstack#3948). Measured on the workspace's resolved
`@objectstack/spec@17.0.0-rc.5`: 19 view operators, 51 AST operators, 0 view
operators absent from the AST set.

With that overlap complete, both guards were satisfiable by doing nothing:

- `packages/plugin-list` — `mapOperator` degraded to the identity function kept
every assertion in the file green, because each view spelling is itself
AST-valid.
- `packages/data-objectstack` — the coverage sweep mirrored production's
`?? op` tail, so a missing row fell back to the raw view spelling, which is now
always AST-valid. Emptying `FILTER_OPERATOR_ALIASES` entirely left it green —
precisely what that file's own header warns is "not a validation failure, it is
an unfiltered query".

Re-anchored on pivots that vocabulary growth cannot cancel. plugin-list now pins
`mapOperator`'s **output** per canonical view operator against an explicit
19-row table read off the switch in `ListView.tsx` (not captured from its output,
which would fossilise a bug), with a totality ratchet failing in both directions
when the pinned rows and the spec vocabulary stop lining up — so a spec addition
lands red instead of as quiet slack, and so does a retirement (#3628 / #3601, the
mirror image). data-objectstack asserts the mapping row **exists**, with no
fallback. The membership and `isFilterAST()` sweeps are kept as a secondary
check: they still name why a wrong target matters, they are just no longer what
gives the files their teeth. The stale header claim that "8 of the 19 canonical
view operators are absent from the AST set" is replaced with what the guarantee
now rests on, stated without any count of the overlap — a hand-written number
beside a vocabulary that moves is what rotted here in the first place.

No production code changed: `mapOperator` and `FILTER_OPERATOR_ALIASES` are byte
for byte as they were on `main`.
55 changes: 44 additions & 11 deletions packages/data-objectstack/src/filter-operator-ast-parity.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -7,7 +7,7 @@
*/

/**
* Adapter operator table → filter-AST parity (#2901, objectstack#3948).
* Adapter operator table → filter-AST parity (#2901, objectstack#3948, #3641).
*
* `FILTER_OPERATOR_ALIASES` is the last translation a filter passes through
* before it goes on the wire, and `normalizeFilterOperator` ends in `?? op` —
Expand All@@ -20,7 +20,31 @@
* `VIEW_FILTER_OPERATORS` — were missing, which is exactly how a stored
* "close_date before X" view came back unfiltered.
*
* These tests pin the table against the spec vocabularies in both directions.
* That failure description is historical, and one part of it has moved (measured
* for #3641, not assumed): on the published `@objectstack/driver-sql@17.0.0-rc.5`
* the array form is lowered by `parseFilterAST()` at the engine and protocol
* doors, and an array reaching the driver anyway is refused with a 400
* (`unsupportedFilterError`, objectstack#5158) rather than skipped. What the
* protocol door does with an array the AST gate REJECTS is not measurable from
* this repo, so whether a missing row costs an unfiltered read or a hard 400
* today is deliberately left open. Both are a broken query.
*
* ## What the coverage guarantee rests on (#3641)
*
* The coverage sweep below used to mirror production's `?? op` tail: resolve the
* operator through the table, fall back to the raw view spelling, then ask
* whether the result was a member of `VALID_AST_OPERATORS`. That reads as a
* coverage assertion and is not one. `VALID_AST_OPERATORS` is derived upstream
* from `AST_OPERATOR_MAP`, it grew until it spelled the canonical view operators
* verbatim — `before` and `after` included — and from that moment the fallback
* arm was always AST-valid. Emptying this entire table left the sweep green, so
* the one thing it existed to catch, a missing row, had become invisible to it.
*
* The sweep therefore asserts the row EXISTS, with no fallback: the header above
* says a missing row is an unfiltered query rather than a validation failure, so
* a missing row is what is asserted, directly. What the rows resolve TO is a
* separate question, kept as its own assertion below — and the pair that
* actually regressed is pinned by spelling, not by membership.
*/
import { describe, it, expect } from 'vitest';
import { VALID_AST_OPERATORS } from '@objectstack/spec/data';
Expand DownExpand Up@@ -81,17 +105,26 @@ describe('FILTER_OPERATOR_ALIASES lands inside the spec AST vocabulary', () => {
).toEqual([]);
});

it('covers every canonical view operator the spec defines', () => {
const uncovered = VIEW_FILTER_OPERATORS
it('has a mapping row for every canonical view operator the spec defines', () => {
// Resolution mirrors `normalizeFilterOperator` — lowercased spelling first,
// then the operator as written — but stops short of its `?? op` tail. That
// tail is production behaviour and must stay there; reproducing it HERE is
// what cancelled this assertion (#3641), because the value it falls back to
// is the raw view spelling and the AST vocabulary now accepts all of those.
const hasRow = (op: string) =>
Object.prototype.hasOwnProperty.call(FILTER_OPERATOR_ALIASES, op.toLowerCase())
|| Object.prototype.hasOwnProperty.call(FILTER_OPERATOR_ALIASES, op);

const missing = VIEW_FILTER_OPERATORS
.filter((op) => !NOT_THIS_ADAPTERS_JOB.has(op))
.filter((op) => {
const target = FILTER_OPERATOR_ALIASES[op] ?? op;
return !VALID_AST_OPERATORS.has(String(target).toLowerCase());
});
.filter((op) => !hasRow(op));
expect(
uncovered,
'an author can declare these on a ViewFilterRule and the spec validates them, '
+ 'but they reach the wire unmapped and the filter is silently dropped',
missing,
'FILTER_OPERATOR_ALIASES has no row for these. An author can declare them on a '
+ 'ViewFilterRule and the spec validates them, so they reach the wire as the raw '
+ 'view spelling. Do not settle for "the AST gate happens to accept that" — the '
+ 'gate accepting a spelling is not the driver compiling it into a WHERE clause, '
+ 'and an unmapped operator is how this adapter shipped an unfiltered query',
).toEqual([]);
});

Expand Down
Loading
Loading