From f6776dd45782726b76d6dfc444d68d6e4b2dc007 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 8 Sep 2026 11:32:35 +0000 Subject: [PATCH 1/2] fix(core): lower a Date comparand in convertFiltersToAST instead of dropping it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A `Date` passed both halves of the operator-object gate (`typeof value === 'object' && !Array.isArray(value)`), so it entered the operator loop — and `Object.entries(someDate)` is `[]`, so the loop body never ran and no condition was pushed for the field at all. `{ status: 'a', created: someDate }` lowered to `['status', '=', 'a']`: not refused, not lowered wrongly, ABSENT. The result set got wider than the author asked for, silently. The defect also depended on the field's siblings — a lone Date left `conditions` empty, so the original object came back untouched and nothing looked wrong. Lowered rather than refused, and the spec decides that — the opposite answer to objectui#8514, which was a refusal precisely because the spec declined to rule. Measured against @objectstack/spec 17.3.0: `ACCEPTED_FILTER_COMPARAND_TYPES` is `['string','number','bigint','boolean','null','Date']`, and $gt/$gte/$lt/$lte/ $between declare `z.ZodDate` in comparand position. The leaf carries the Date INSTANCE. `parseFilterAST(['created','=',d])` hands back `{ created: d }` with the Date intact, and the operator arm already emits `{ created: { $gte: d } }` as `['created','>=',d]`, so stringifying to ISO here would give the shorthand and the operator form two different comparand types for one author intent. The gate is the spec's own `isAcceptedFilterComparand`, not a local `instanceof Date` — the same reason operators route through the spec's `normalizeFilterOperator` instead of a second map. Refs: objectui#8555 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01YBWFb5YgMU5dw8p2VKj16S --- .../8555-filter-converter-date-comparand.md | 39 +++ .../filter-date-comparand-8555.test.ts | 265 ++++++++++++++++++ packages/core/src/utils/filter-converter.ts | 52 ++++ 3 files changed, 356 insertions(+) create mode 100644 .changeset/8555-filter-converter-date-comparand.md create mode 100644 packages/core/src/utils/__tests__/filter-date-comparand-8555.test.ts diff --git a/.changeset/8555-filter-converter-date-comparand.md b/.changeset/8555-filter-converter-date-comparand.md new file mode 100644 index 0000000000..e654a95fc1 --- /dev/null +++ b/.changeset/8555-filter-converter-date-comparand.md @@ -0,0 +1,39 @@ +--- +'@object-ui/core': patch +--- + +`convertFiltersToAST` now lowers a `Date` comparand instead of silently dropping +the field it sits on (objectui#8555). + +The operator-object arm opened on `typeof value === 'object' && +!Array.isArray(value)`, and a `Date` passes both tests. `Object.entries` of a +Date is `[]`, so the operator loop body never ran and NO condition was pushed: +`{ status: 'a', created: someDate }` lowered to `['status', '=', 'a']`. Nothing +threw and nothing warned — the result set simply got WIDER than the author asked +for, which is the one failure direction this file exists to avoid. The defect +also depended on the field's siblings: with the Date alone, `conditions` ended +empty and the original object came back untouched, so it only became visible +once a second field was present. + +It is LOWERED, not refused, and `@objectstack/spec` is what decides that — +the opposite answer to objectui#8514, which was a refusal precisely because the +spec declined to rule on that shape. Here it rules twice over (measured against +spec 17.3.0): `ACCEPTED_FILTER_COMPARAND_TYPES` is +`['string','number','bigint','boolean','null','Date']`, and +`$gt` / `$gte` / `$lt` / `$lte` / `$between` declare `z.ZodDate` in comparand +position. + +The AST leaf carries the `Date` INSTANCE. The wire form is deliberately not this +adapter's question to answer: `parseFilterAST(['created', '=', d])` hands back +`{ created: d }` with the Date intact, and the operator arm has always emitted +`{ created: { $gte: d } }` as `['created', '>=', d]` — so converting to an ISO +string or an epoch here would make the shorthand and the operator form emit two +different comparand types for one author intent. The gate is the spec's own +`isAcceptedFilterComparand` rather than a local `instanceof Date`, the same +reason this file already routes operators through the spec's +`normalizeFilterOperator` instead of a second map. + +Operator objects are untouched: `{ age: { $gt: 26 } }`, `$in` / `$nin` / +`$between` members, `$null` / `$exists`, the `$regex` / `$not` / bare-array +refusals, and an empty `{}` operator object (still the TRUE identity, still +constraining nothing) all behave exactly as before. diff --git a/packages/core/src/utils/__tests__/filter-date-comparand-8555.test.ts b/packages/core/src/utils/__tests__/filter-date-comparand-8555.test.ts new file mode 100644 index 0000000000..31cfd4bf6d --- /dev/null +++ b/packages/core/src/utils/__tests__/filter-date-comparand-8555.test.ts @@ -0,0 +1,265 @@ +/** + * ObjectUI + * Copyright (c) 2024-present ObjectStack Inc. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +/** + * A `Date` comparand through `convertFiltersToAST` — objectui#8555. + * + * ## The defect + * + * The operator-object arm opened on `typeof value === 'object' && + * !Array.isArray(value)`. A `Date` passes both tests, so it entered the + * operator loop — and `Object.entries(someDate)` is `[]`, so the loop body + * never ran and NO condition was pushed for that field. + * + * Not refused, not lowered wrongly: ABSENT. `{ status: 'a', created: someDate }` + * lowered to `['status', '=', 'a']`, which is a WIDER result set than the author + * asked for, with nothing thrown and nothing logged — the one failure direction + * this file exists to avoid. + * + * It also made the field's fate depend on its SIBLINGS. With the Date alone, + * `conditions` ended empty and `convertFiltersToAST` returned the ORIGINAL + * OBJECT untouched, so the shape survived by accident and the defect only + * appeared once a second field was present. That asymmetry is part of the bug; + * section 2 is what pins it closed. + * + * ## The ruling — LOWER it, and the spec is what decides that + * + * This is the OPPOSITE answer to objectui#8514, which was resolved as a refusal + * precisely because the spec DECLINED to rule on that shape. Here it rules, + * measured against `@objectstack/spec` 17.3.0 and pinned in section 3: + * + * - `ACCEPTED_FILTER_COMPARAND_TYPES` is + * `['string','number','bigint','boolean','null','Date']` — `Date` is a + * first-class literal comparand, and `isAcceptedFilterComparand(new Date())` + * is `true`. + * - `parseFilterAST(['created', '=', d])` hands back `{ created: d }` with the + * Date INSTANCE intact. + * + * So the wire-form question the card raised ("ISO string? epoch?") is answered + * by NOT answering it here: the AST leaf carries the `Date`, exactly as the + * operator arm has always emitted it for `{ created: { $gte: d } }`. + * Stringifying in this arm would make the shorthand and the operator form emit + * two different comparand types for one author intent. + * + * ## What carries the weight — three legs, RUN rather than predicted + * + * Each was applied to the committed implementation, proved on disk in both + * directions, and restored by state. The counts and MODES below are measured, + * not expected — one of them corrected the prediction written here first. + * + * 1. **Ablation** — the arm removed. 7 of 16 red. MODE: a MISSING condition. + * `{ status, created }` produces `['status', '=', 'a']` and `node[2]` is + * `undefined`. Sections 1 and 2 both carry it. + * 2. **`value.toISOString()`** — the plausible wrong fix, since "what wire + * form?" is the question the card asks. 6 of 16 red. MODE: the right node + * with the WRONG COMPARAND TYPE — `'2026-01-01T00:00:00.000Z'` where a Date + * was expected. + * ⚠️ This paragraph first claimed only the `instanceof Date` and + * operator-parity pins would discriminate. Measured, six do: every + * `toEqual([..., D])` in sections 1 and 2 reddens too, because a string is + * not deep-equal to a Date. The correction that matters runs the other way — + * §2's "lone and sibling forms lower the SAME field" pin does NOT redden + * here, because the caricature is symmetric too. Symmetry alone never + * discriminates a wrong comparand type; the `instanceof` / `toBe(D)` pins + * are what do, and they are in their own `it()` blocks so no earlier + * assertion can stop them running (objectui#8506, objectui#8514). + * 3. **`Object.keys(value).length === 0` as the gate** — handles the Date and + * reads EVERY entry-less object as an equality comparand. 2 of 16 red, both + * in section 4, and NOTHING else: for a Date input this caricature is + * byte-identical to the shipped fix, which is exactly why section 4 has to + * exist. MODE: over-promotion — `{}` and `/x/` appear in comparand position + * (`['created', '=', {}]`), and a RegExp comparand is one + * `normalizeFilterComparandTypes` refuses outright, so the caricature moves + * the failure downstream instead of removing it. + */ + +import { describe, it, expect } from 'vitest'; +import { + isFilterAST, + parseFilterAST, + isAcceptedFilterComparand, + ACCEPTED_FILTER_COMPARAND_TYPES, +} from '@objectstack/spec/data'; +import { + convertFiltersToAST, + toFilterNode, + mergeFilterNodes, +} from '../filter-converter'; + +const D = new Date('2026-01-01T00:00:00.000Z'); + +// --------------------------------------------------------------------------- +// 1. The Date is lowered — as a Date, not as a string this layer invented +// --------------------------------------------------------------------------- + +describe('objectui#8555 — a Date comparand lowers to an equality node', () => { + it('lowers { status, created } to BOTH conditions — the sibling no longer eats the Date', () => { + // The whole node, so the pre-fix failure MODE is legible in the diff: the + // old emission was `['status', '=', 'a']` — one condition, not two, and no + // group at all. + const node = convertFiltersToAST({ status: 'a', created: D }); + expect(node).toEqual(['and', ['status', '=', 'a'], ['created', '=', D]]); + }); + + it('emits the Date INSTANCE, not an ISO string', () => { + // Its own block on purpose: this is the single assertion that separates the + // shipped fix from `value.toISOString()`, and an earlier failing assertion + // in a shared block would stop it ever running (objectui#8506 / #8514). + const node = convertFiltersToAST({ created: D }) as [string, string, unknown]; + expect(node[2]).toBeInstanceOf(Date); + expect(node[2]).toBe(D); + expect(typeof node[2]).not.toBe('string'); + }); + + it('emits the SAME comparand the operator arm has always emitted', () => { + // Parity with `{ created: { $gte: d } }` is the reason the shorthand is not + // stringified: one author intent, one comparand type. A stringifying fix + // breaks this and nothing else. + const shorthand = convertFiltersToAST({ created: D }) as [string, string, unknown]; + const operatorForm = convertFiltersToAST({ created: { $gte: D } }) as [string, string, unknown]; + expect(shorthand[2]).toBe(operatorForm[2]); + }); + + it('produces a node the spec accepts, with the Date intact through its doors', () => { + const node = convertFiltersToAST({ created: D }); + expect(isFilterAST(node)).toBe(true); + const parsed = parseFilterAST(node) as { created: unknown }; + expect(parsed).toEqual({ created: D }); + expect(parsed.created).toBeInstanceOf(Date); + }); + + it('lowers a Date inside $and / $or and through both public sinks', () => { + expect(convertFiltersToAST({ $or: [{ created: D }, { status: 'open' }] })) + .toEqual(['or', ['created', '=', D], ['status', '=', 'open']]); + expect(toFilterNode({ status: 'a', created: D })) + .toEqual(['and', ['status', '=', 'a'], ['created', '=', D]]); + // One AST-node source beside the object source; `mergeFilterNodes` keeps + // each source as its own child of the `and`. + expect(mergeFilterNodes({ created: D }, ['stage', '=', 'won'])) + .toEqual(['and', ['created', '=', D], ['stage', '=', 'won']]); + }); +}); + +// --------------------------------------------------------------------------- +// 2. The ASYMMETRY — a Date ALONE behaves the same as a Date with siblings +// --------------------------------------------------------------------------- + +describe('objectui#8555 — a lone Date is lowered, not handed back unchanged', () => { + it('no longer returns the original filter object when the Date is the only field', () => { + // Pre-fix: `conditions` was empty, so the `if (conditions.length === 0)` + // fallback returned the INPUT OBJECT — `isFilterAST` false, and the wire + // answers 400. The defect hid here, because the shape came back "unharmed". + const node = convertFiltersToAST({ created: D }); + expect(node).toEqual(['created', '=', D]); + expect(node).not.toBe(undefined); + expect(Array.isArray(node)).toBe(true); + expect(isFilterAST(node)).toBe(true); + }); + + it('lone and sibling forms lower the SAME field to the SAME condition', () => { + const alone = convertFiltersToAST({ created: D }); + const withSibling = convertFiltersToAST({ status: 'a', created: D }) as unknown[]; + // The `created` child of the group is byte-for-byte the lone emission. + expect(withSibling[2]).toEqual(alone); + }); +}); + +// --------------------------------------------------------------------------- +// 3. Why LOWER and not REFUSE — the spec's own ruling, pinned +// --------------------------------------------------------------------------- + +describe('objectui#8555 — the spec rules Date IN, which is why this is not a refusal', () => { + it('declares Date one of its six accepted literal comparand types', () => { + expect(ACCEPTED_FILTER_COMPARAND_TYPES).toContain('Date'); + expect(isAcceptedFilterComparand(D)).toBe(true); + }); + + it('accepts a Date in the AST leaf and keeps it a Date', () => { + // The wire-form question, answered by the spec rather than by this adapter. + expect(isFilterAST(['created', '=', D])).toBe(true); + expect((parseFilterAST(['created', '=', D]) as { created: unknown }).created) + .toBeInstanceOf(Date); + }); + + it('has Date as the ONLY object-typed member, so this arm is a Date arm', () => { + // The gate is `isAcceptedFilterComparand`, not `instanceof Date`. This pin + // is what makes that safe to read: every OTHER accepted comparand type is a + // primitive and can never reach the `typeof value === 'object'` branch. If + // the spec ever adds a second object-shaped literal, this reddens and the + // reader learns the arm's reach just widened. + const objectTyped = ACCEPTED_FILTER_COMPARAND_TYPES.filter( + (t) => t !== 'string' && t !== 'number' && t !== 'bigint' && t !== 'boolean' && t !== 'null', + ); + expect(objectTyped).toEqual(['Date']); + }); +}); + +// --------------------------------------------------------------------------- +// 4. NON-regression — the operator-object path `Object.entries` is there for. +// This is the axis that separates the fix from "anything entry-less is a +// comparand". Every case asserts the PRODUCED node. +// --------------------------------------------------------------------------- + +describe('objectui#8555 — operator objects lower exactly as before', () => { + it('$gt / $gte / $lt / $lte still reach the operator loop', () => { + expect(convertFiltersToAST({ age: { $gt: 26 } })).toEqual(['age', '>', 26]); + expect(convertFiltersToAST({ age: { $gte: 18, $lte: 65 } })) + .toEqual(['and', ['age', '>=', 18], ['age', '<=', 65]]); + }); + + it('an operator object carrying a Date value still lowers through the loop', () => { + // The Date arm must not swallow `{ created: { $gte: d } }`: the VALUE of the + // field is a plain object there, and only its member is a Date. + expect(convertFiltersToAST({ created: { $gte: D } })).toEqual(['created', '>=', D]); + expect(convertFiltersToAST({ created: { $between: [D, D] } })) + .toEqual(['created', 'between', [D, D]]); + }); + + it('$in / $nin / $null / $exists and the refusals are untouched', () => { + expect(convertFiltersToAST({ status: { $in: ['a', 'b'] } })).toEqual(['status', 'in', ['a', 'b']]); + expect(convertFiltersToAST({ deleted: { $null: true } })).toEqual(['deleted', 'is_null', true]); + expect(convertFiltersToAST({ deleted: { $exists: false } })).toEqual(['deleted', 'is_null', true]); + expect(() => convertFiltersToAST({ name: { $regex: 'a.c' } })).toThrow(/\$regex/); + expect(() => convertFiltersToAST({ tags: ['a'] })).toThrow(/bare ARRAY/); + }); + + it('an EMPTY operator object still constrains nothing — it is not a comparand', () => { + // `{}` has no entries either, so a fix gated on `Object.keys(value).length + // === 0` would lower it to `['created', '=', {}]`. It must stay the TRUE + // identity it has always been: no operators means no constraint, the same + // reading `{ $and: [] }` gets in `lowerLogicalGroup`. + expect(convertFiltersToAST({ status: 'a', created: {} })).toEqual(['status', '=', 'a']); + }); + + it('a non-Date exotic object is NOT promoted into comparand position', () => { + // A `RegExp` is entry-less too, so the same `Object.keys` caricature would + // emit `['created', '=', /x/]` — a comparand `normalizeFilterComparandTypes` + // refuses outright (INVALID_FILTER / 400), i.e. the failure moves downstream + // rather than going away. + // + // ⚠️ This pins the NEGATIVE only. What this arm does with a RegExp today is + // the same silent drop objectui#8555 describes, left in place deliberately: + // the spec rules Date IN and rules RegExp OUT, so the two shapes need + // different answers and only one of them was in scope. Filed separately. + expect(isAcceptedFilterComparand(/x/)).toBe(false); + expect(convertFiltersToAST({ status: 'a', created: /x/ })) + .not.toEqual(['and', ['status', '=', 'a'], ['created', '=', /x/]]); + }); + + it('plain scalars, null and undefined are exactly what they were', () => { + expect(convertFiltersToAST({ status: 'active' })).toEqual(['status', '=', 'active']); + expect(convertFiltersToAST({ count: 0, ok: false })).toEqual([ + 'and', + ['count', '=', 0], + ['ok', '=', false], + ]); + // A null/undefined value is skipped before any of this, and an all-skipped + // filter still returns the original object. + expect(convertFiltersToAST({ a: null, b: undefined })).toEqual({ a: null, b: undefined }); + }); +}); diff --git a/packages/core/src/utils/filter-converter.ts b/packages/core/src/utils/filter-converter.ts index 92cd70dc37..bead9c555b 100644 --- a/packages/core/src/utils/filter-converter.ts +++ b/packages/core/src/utils/filter-converter.ts @@ -14,6 +14,7 @@ */ import { normalizeFilterOperator } from '@objectstack/spec/ui'; +import { isAcceptedFilterComparand } from '@objectstack/spec/data'; /** * FilterNode AST type definition @@ -216,6 +217,13 @@ function falseIdentityLeaf(field: string, value: unknown[]): FilterNode { * convertFiltersToAST({ $or: [{ status: 'open' }, { status: 'blocked' }] }) * // => ['or', ['status', '=', 'open'], ['status', '=', 'blocked']] * + * @example + * // A Date is a comparand, not an operator map (objectui#8555). It lowers as + * // the Date INSTANCE — the spec accepts one, and the operator form already + * // emits one. + * convertFiltersToAST({ created: new Date('2026-01-01') }) + * // => ['created', '=', Date(2026-01-01)] + * * @throws {FilterOperatorError} If an unknown operator is encountered, if * `$not` is used — see the `$not` arm for why the AST cannot carry it — or if a * field's value is a bare ARRAY (`{ tags: ['a', 'b'] }`) — see the array arm for @@ -297,6 +305,50 @@ export function convertFiltersToAST(filter: Record): FilterNode | R // Check if value is a complex operator object if (typeof value === 'object' && !Array.isArray(value)) { + // A `Date` is a COMPARAND, not an operator map — objectui#8555. + // + // `typeof new Date()` is `'object'` and a Date is not an array, so it used + // to enter the loop below; `Object.entries(someDate)` is `[]`, the body + // never ran, and NO condition was pushed for the field. Not refused, not + // lowered wrongly — ABSENT, so `{ status: 'a', created: someDate }` + // lowered to `['status', '=', 'a']` and the result set got WIDER than the + // author asked for, silently. That is the one failure direction this file + // exists to avoid. It also made the field's behaviour depend on its + // SIBLINGS: a Date alone left `conditions` empty, so the original object + // came back untouched and the defect was invisible until a second field + // appeared. + // + // Lowered rather than refused, and the spec is what decides it — the + // opposite answer to objectui#8514, which was a refusal precisely because + // the spec DECLINED to rule on that shape. Here it rules, twice over + // (measured against @objectstack/spec 17.3.0): + // `ACCEPTED_FILTER_COMPARAND_TYPES` is + // `['string','number','bigint','boolean','null','Date']`, and + // `$gt`/`$gte`/`$lt`/`$lte`/`$between` declare `z.ZodDate` in comparand + // position. So a Date is a first-class filter comparand, not a shape this + // layer has to invent an answer for. + // + // ⛔ NOT converted to an ISO string or an epoch here. The wire form is not + // this adapter's question to answer: `parseFilterAST(['created', '=', d])` + // hands back `{ created: d }` with the Date INSTANCE intact (measured), and + // `normalizeFilterComparandTypes` accepts it as-is. The operator arm below + // already passes a Date through untouched (`{ created: { $gte: d } }` → + // `['created', '>=', d]`), so stringifying here would make the shorthand + // and the operator form emit two different comparand types for the same + // author intent — a second dialect, in the file whose whole job is to have + // one. + // + // The gate is the spec's own predicate rather than a local `instanceof + // Date`, the same reason `normalizeFilterOperator` is used below instead of + // a second operator map. Today `Date` is its only object-typed member + // (pinned in filter-date-comparand-8555.test.ts), so this arm is a Date arm + // in practice; if the spec ever accepts another object-shaped literal, this + // reads it as a comparand instead of silently dropping it. + if (isAcceptedFilterComparand(value)) { + conditions.push([field, '=', value]); + continue; + } + // Handle operator-based filters for (const [operator, operatorValue] of Object.entries(value)) { // `$regex` is refused, not downgraded. It used to become `contains` From dd0fa8eda87d0123c7e1ec984f5a47c58477e7a3 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 8 Sep 2026 11:48:18 +0000 Subject: [PATCH 2/2] fix(core): refuse an array on a single-value operator in viewFilterRuleToNode MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `viewFilterRuleToNode` never inspected `rule.value`, so a stored view rule `{ field: 'tags', operator: 'equals', value: ['a'] }` lowered to `['tags', 'equals', ['a']]`. Measured against @objectstack/spec 17.3.0 the spec's doors accept that node unjudged — `isFilterAST` is true and `parseFilterAST` returns `{ tags: ['a'] }` — so the refusal arrived two layers away, as driver-sql's 400 INVALID_FILTER or as an empty list from an in-memory matcher, attributable to nothing. It is the same array-in-a-scalar-slot shape objectui#8530 refused in the object arm, which deliberately did not reach this door: a hand-authored `{ tags: ['a'] }` failed fast naming `$in`, the same mistake saved into a view stayed silent. Keyed on the operator's ARITY, never on `Array.isArray(value)`. The two array-valued classes are the spec's own exports — VIEW_FILTER_LIST_VALUE_OPERATORS (`in`, `not_in`) and VIEW_FILTER_PAIR_VALUE_OPERATORS (`between`) — so `in` / `not_in` / `between` rules keep their arrays, and the check runs after normalization so `nin` is judged as `not_in`. Two classes are left alone on measurement: an operator the spec does not know is still passed through verbatim (the misspelling is already the loud failure), and the valueless operators are not refused because the spec discards their value anyway (`parseFilterAST(['tags','is_null',['a']])` is `{ tags: { $null: true } }`). A pin holds the four classes to an exact partition of VIEW_FILTER_OPERATORS. The refusal throws, so a saved view with one bad rule fails at render rather than returning a narrower answer. Not a new blast radius: plugin-list's `buildEffectiveFilter` and plugin-view's `ObjectView` both call this sink inside their load `try` and have caught this error class since objectui#8530, and `classifyLoadError` reads INVALID_FILTER / 400 — so the user sees the "filter is malformed" panel. Dropping the rule would widen the result set; rewriting `equals` into `in` would change what the view means. Refs: objectui#8557 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01YBWFb5YgMU5dw8p2VKj16S --- .changeset/8557-view-filter-rule-arity.md | 45 +++ .../filter-view-rule-arity-8557.test.ts | 317 ++++++++++++++++++ packages/core/src/utils/filter-converter.ts | 133 +++++++- 3 files changed, 494 insertions(+), 1 deletion(-) create mode 100644 .changeset/8557-view-filter-rule-arity.md create mode 100644 packages/core/src/utils/__tests__/filter-view-rule-arity-8557.test.ts diff --git a/.changeset/8557-view-filter-rule-arity.md b/.changeset/8557-view-filter-rule-arity.md new file mode 100644 index 0000000000..e2f4bc73fd --- /dev/null +++ b/.changeset/8557-view-filter-rule-arity.md @@ -0,0 +1,45 @@ +--- +'@object-ui/core': patch +--- + +`viewFilterRuleToNode` now refuses an ARRAY on a single-value operator instead +of passing it through verbatim (objectui#8557). + +A stored view rule never had its `value` inspected, so +`{ field: 'tags', operator: 'equals', value: ['a'] }` lowered to +`['tags', 'equals', ['a']]` — and the spec's own doors accept that node unjudged +(`isFilterAST` is `true`, `parseFilterAST` hands back `{ tags: ['a'] }`, measured +against spec 17.3.0). The refusal therefore arrived two layers away, as +`@objectstack/driver-sql`'s `400 INVALID_FILTER` or as an empty list from an +in-memory matcher, with nothing to attribute it to. It is the same +array-in-a-scalar-slot shape objectui#8530 refused in `convertFiltersToAST`'s +object arm, which deliberately did not reach this door — so a hand-authored +`{ tags: ['a'] }` failed fast with a message naming `$in` while the same mistake +saved into a view stayed silent. That asymmetry is closed. + +The guard keys on the operator's ARITY, never on `Array.isArray(value)`: `in`, +`not_in` and `between` legitimately carry arrays through this same function and +are untouched. The arity comes from the spec's own +`VIEW_FILTER_LIST_VALUE_OPERATORS` and `VIEW_FILTER_PAIR_VALUE_OPERATORS` rather +than a local list — the docblock on the first of them names a hard-coded +`["in", "notIn"]` as the mistake it exists to prevent — and the check runs after +`normalizeFilterOperator`, so an alias is judged by what it means (`nin` is +`not_in`, and keeps its array). Two classes are deliberately left alone: an +operator the spec does not know is still passed through verbatim, because the +misspelling is already the loud failure and refusing here would report the wrong +problem; and the valueless operators (`is_null`, `is_empty`, …) are not refused, +because the spec discards their value anyway +(`parseFilterAST(['tags', 'is_null', ['a']])` is `{ tags: { $null: true } }`), so +a stray array there cannot select the wrong rows. A pin holds the four arity +classes to an exact partition of `VIEW_FILTER_OPERATORS`, so a new spec operator +reddens rather than silently inheriting a verdict. + +The refusal is a `FilterOperatorError` (`INVALID_FILTER` / 400), which means a +saved view with one bad rule now fails at render rather than returning a +narrower result. That is not a new blast radius: both sinks already catch this +error class from this same file — `plugin-list`'s `buildEffectiveFilter` and +`plugin-view`'s `ObjectView` each call it inside their load `try` — and +`classifyLoadError` reads the code and status, so what a user sees is the +"filter is malformed" panel rather than a network fault or a crashed page. The +alternatives are both silent: dropping the rule widens the result set, and +rewriting `equals` into `in` changes what the saved view means. diff --git a/packages/core/src/utils/__tests__/filter-view-rule-arity-8557.test.ts b/packages/core/src/utils/__tests__/filter-view-rule-arity-8557.test.ts new file mode 100644 index 0000000000..176d6d7cfe --- /dev/null +++ b/packages/core/src/utils/__tests__/filter-view-rule-arity-8557.test.ts @@ -0,0 +1,317 @@ +/** + * ObjectUI + * Copyright (c) 2024-present ObjectStack Inc. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +/** + * An ARRAY on a single-value operator in the STORED-VIEW arm — objectui#8557. + * + * ## The defect + * + * `viewFilterRuleToNode` never inspected `rule.value`, so a saved view rule + * carrying an array on a scalar operator travelled through verbatim: + * + * `toFilterNode([{ field: 'tags', operator: 'equals', value: ['a'] }])` + * → `[['tags', 'equals', ['a']]]` + * → `parseFilterAST` → `{ tags: ['a'] }` + * + * Section 1 pins that the spec's doors accept that node unjudged (measured + * against `@objectstack/spec` 17.3.0), which is why the refusal used to arrive + * two layers away as a `400 INVALID_FILTER` from `driver-sql`, or as an empty + * list from an in-memory matcher. + * + * It is the SAME array-in-a-scalar-slot shape objectui#8530 / PR #8551 refused + * in `convertFiltersToAST`'s object arm. That fix deliberately did not reach + * here — the two shapes enter the lowering by different doors — so a + * hand-authored `{ tags: ['a'] }` failed fast with a message naming `$in` while + * the same mistake SAVED INTO A VIEW stayed silent. This closes that asymmetry. + * + * ## The two things the card said must be got right + * + * 1. **Keyed on the operator's ARITY, never on `Array.isArray(value)`.** `in`, + * `not_in` and `between` legitimately carry arrays through this very + * function, and that path had no pin protecting it. Section 3 is that pin, + * and it is the axis that separates the fix from "refuse anything + * array-shaped". The arity comes from the spec's own exported sets, so this + * file also pins (section 4) that the four arity classes PARTITION + * `VIEW_FILTER_OPERATORS` exactly — an operator added to the spec falls into + * no class, that pin reddens, and someone classifies it. + * 2. **Where the refusal belongs.** It throws from the lowering. Section 5 + * records the measurement that made that safe: both sinks already catch a + * `FilterOperatorError` from this same file, so a malformed saved view lands + * in the list's "filter is malformed" panel — the blast radius the object arm + * has had since objectui#8530, not a new one. + * + * ## Legs, RUN rather than predicted + * + * Applied to the committed implementation, proved on disk both ways, restored + * by state. Counts and MODES below are measured, not expected. + * + * - **Ablation** (the arm removed): 7 of 16 red — all of section 2 AND + * section 5's envelope-parity pin, which this paragraph did not predict. + * MODE: no throw at all — `captureRefusal` fails on its own first line, + * printing the node that travelled through (`[['tags','equals',['a']]]`), + * so nothing downstream of it runs on a stale assumption. + * - **Caricature `Array.isArray(value)` alone** (the card's named one, no + * arity key): 6 of 16 red, ALL in section 3 — and section 2 stays entirely + * green. That is the shape of the warning: the caricature passes every + * "the array is now refused" assertion and eats `in` / `not_in` / + * `between` with it. MODE: a refusal where a lowering was expected. + * - **Caricature "refuse on every known operator"** (the valueless class not + * carved out): exactly 1 of 16 red — the `is_null` pin in section 3, the + * single assertion written for it, and nothing else. + * + * Refusal pins assert the envelope on a CAPTURED error AND the message's first + * sentence. Envelope-only is not sufficient in this file: objectui#8530 saw + * `code` + `httpStatus` go green for the wrong reason, on a refusal that was + * really "Unknown filter operator 0". + */ + +import { describe, it, expect } from 'vitest'; +import { isFilterAST, parseFilterAST } from '@objectstack/spec/data'; +import { + VIEW_FILTER_OPERATORS, + VIEW_FILTER_LIST_VALUE_OPERATORS, + VIEW_FILTER_PAIR_VALUE_OPERATORS, +} from '@objectstack/spec/ui'; +import { toFilterNode, mergeFilterNodes, FilterOperatorError } from '../filter-converter'; + +/** + * Run the lowering and hand back the refusal it raised. Fails on the FIRST + * line, printing what was produced, when it did not refuse — so no later + * assertion is silently skipped by a non-throwing lowering. + */ +function captureRefusal(run: () => unknown): FilterOperatorError { + let caught: unknown; + let produced: unknown; + try { + produced = run(); + } catch (e) { + caught = e; + } + expect( + caught, + `expected the lowering to refuse, it produced ${JSON.stringify(produced)}`, + ).toBeInstanceOf(FilterOperatorError); + return caught as FilterOperatorError; +} + +const rule = (operator: string, value?: unknown) => + value === undefined ? { field: 'tags', operator } : { field: 'tags', operator, value }; + +// --------------------------------------------------------------------------- +// 1. Why the producer must refuse: the spec doors pass the old node unjudged +// --------------------------------------------------------------------------- + +describe('objectui#8557 — the pre-fix node reaches the wire unjudged', () => { + it('is accepted by isFilterAST and lowered to an array comparand', () => { + // If the spec ever starts refusing this, the pin reddens and the reader + // learns the refusal gained a sibling — not that this arm can go. + const preFixNode = ['tags', 'equals', ['a']]; + expect(isFilterAST(preFixNode)).toBe(true); + expect(parseFilterAST(preFixNode)).toEqual({ tags: ['a'] }); + }); +}); + +// --------------------------------------------------------------------------- +// 2. The refusal — envelope AND first sentence, on a captured error +// --------------------------------------------------------------------------- + +describe('objectui#8557 — an array on a single-value operator is refused', () => { + it('refuses the card`s measured case with the INVALID_FILTER / 400 envelope', () => { + const err = captureRefusal(() => toFilterNode([rule('equals', ['a'])])); + expect(err.code).toBe('INVALID_FILTER'); + expect(err.httpStatus).toBe(400); + expect(err.name).toBe('FilterOperatorError'); + }); + + it('names the field, the operator and the comparand in its first sentence', () => { + // Its own block: objectui#8530 measured an envelope pin going green for the + // wrong reason ("Unknown filter operator 0"), and only the first-sentence + // assertion discriminated. Separate `it()` so nothing reddens ahead of it. + const err = captureRefusal(() => toFilterNode([rule('equals', ['a', 'b'])])); + expect(err.message).toMatch( + /^\[ObjectUI\] The stored view rule on field 'tags' carries an ARRAY as the comparand of 'equals', which takes a single value: \["a","b"\]\./, + ); + }); + + it('prescribes the spellings that work and says it did not rewrite to `in`', () => { + const err = captureRefusal(() => toFilterNode([rule('equals', ['a'])])); + expect(err.message).toContain("{ field: 'tags', operator: 'in', value: [...] }"); + expect(err.message).toContain("'not_in'"); + expect(err.message).toContain("{ operator: 'between', value: [min, max] }"); + expect(err.message).toMatch(/NOT rewritten to 'in'/); + expect(err.message).toContain('objectui#8557'); + // Not the sibling diagnostics — this is neither an unknown operator, nor a + // combinator problem, nor the object arm's bare-array refusal. + expect(err.message).not.toMatch(/Unknown filter operator/); + expect(err.message).not.toMatch(/combinator/); + expect(err.message).not.toMatch(/bare ARRAY/); + }); + + it('judges the operator by what it MEANS — an alias is normalized first', () => { + // `eq` is an alias of `equals`; it must be refused the same way, and the + // message must name the canonical spelling rather than what was stored. + const err = captureRefusal(() => toFilterNode([rule('eq', ['a'])])); + expect(err.message).toContain("comparand of 'equals'"); + }); + + it('refuses on every single-value operator the spec declares, not just equals', () => { + for (const op of ['not_equals', 'contains', 'starts_with', 'greater_than', 'before', 'icontains']) { + const err = captureRefusal(() => toFilterNode([rule(op, ['a'])])); + expect(err.code, `operator ${op}`).toBe('INVALID_FILTER'); + expect(err.message, `operator ${op}`).toContain(`comparand of '${op}'`); + } + }); + + it('refuses the empty array too, and wherever the rule sits', () => { + // `[]` is exactly as unanswerable as a populated one, and reading it as + // "no constraint" would widen the result set. + expect(captureRefusal(() => toFilterNode([rule('equals', [])])).message) + .toContain("which takes a single value: []."); + // Mixed source: rules concatenated with URL triples, as ObjectView builds them. + expect(captureRefusal( + () => toFilterNode([['stage', '=', 'won'], rule('equals', ['a'])]), + ).code).toBe('INVALID_FILTER'); + // Through the sink every renderer actually calls. + expect(captureRefusal( + () => mergeFilterNodes({ status: 'x' }, [rule('equals', ['a'])]), + ).message).toContain("field 'tags'"); + }); +}); + +// --------------------------------------------------------------------------- +// 3. NON-regression — the arity axis. This is what separates the fix from +// "refuse anything array-shaped", and the path the card said had no pin. +// --------------------------------------------------------------------------- + +describe('objectui#8557 — array-valued operators keep their arrays', () => { + it('`in` lowers with its member list intact, through the spec doors', () => { + const node = toFilterNode([rule('in', ['a', 'b'])]); + expect(node).toEqual([['tags', 'in', ['a', 'b']]]); + expect(isFilterAST(node)).toBe(true); + expect(parseFilterAST(['tags', 'in', ['a', 'b']])).toEqual({ tags: { $in: ['a', 'b'] } }); + }); + + it('`not_in` lowers — and so does its `nin` alias, which normalizes into it', () => { + expect(toFilterNode([rule('not_in', ['a'])])).toEqual([['tags', 'not_in', ['a']]]); + // The alias is the case a hard-coded `["in", "notIn"]` list gets wrong; the + // gate reads the NORMALIZED operator, so it cannot. + expect(toFilterNode([rule('nin', ['a'])])).toEqual([['tags', 'not_in', ['a']]]); + }); + + it('`between` lowers with its [min, max] pair', () => { + const node = toFilterNode([{ field: 'amount', operator: 'between', value: [1, 10] }]); + expect(node).toEqual([['amount', 'between', [1, 10]]]); + expect(isFilterAST(node)).toBe(true); + expect(parseFilterAST(['amount', 'between', [1, 10]])).toEqual({ amount: { $between: [1, 10] } }); + }); + + it('an operator the spec does NOT know is still passed through verbatim', () => { + // A misspelling is already the loud failure: `normalizeFilterOperator` leaves + // it alone so `isFilterAST` refuses it and the server names it. Refusing + // HERE would report "use $in" for what is actually a typo. + const node = toFilterNode([rule('bogus_op', ['a'])]); + expect(node).toEqual([['tags', 'bogus_op', ['a']]]); + expect(isFilterAST(['tags', 'bogus_op', ['a']])).toBe(false); + }); + + it('a valueless operator is not refused — the spec discards its value anyway', () => { + // Measured: `parseFilterAST(['tags', 'is_null', ['a']])` is + // `{ tags: { $null: true } }`. The stray array produces no wrong node, so + // refusing would turn a harmless input into a render-time throw — and would + // prescribe `in` for an operator that takes no value at all. + expect(toFilterNode([rule('is_null', ['a'])])).toEqual([['tags', 'is_null', ['a']]]); + expect(parseFilterAST(['tags', 'is_null', ['a']])).toEqual({ tags: { $null: true } }); + }); + + it('an AST GROUP node is not a rule, so the arity gate never sees its array', () => { + // objectui#8456 (`617707a48`) made `$and` / `$or` lower to real AST group + // nodes, and a group node carries an ARRAY of children legitimately. It + // cannot collide with this gate: `isViewFilterRule` requires a plain OBJECT + // with a non-empty string `field`, and every AST node is an ARRAY — so a + // group is passed through by `toFilterNode` without ever entering + // `viewFilterRuleToNode`. Pinned because the two features are one function + // apart and both are about arrays. + const group = ['or', ['status', '=', 'open'], ['status', '=', 'blocked']]; + expect(toFilterNode([group])).toEqual([group]); + // And mixed with a rule that IS lowered, which is how ObjectView builds them. + expect(toFilterNode([group, { field: 'amount', operator: 'in', value: [1, 2] }])) + .toEqual([group, ['amount', 'in', [1, 2]]]); + // The object arm still produces the group, unaffected by this card. + expect(mergeFilterNodes({ $or: [{ status: 'open' }, { status: 'blocked' }] })) + .toEqual(group); + }); + + it('scalar values on scalar operators, and valueless rules, are untouched', () => { + expect(toFilterNode([{ field: 'status', operator: 'equals', value: 'in_progress' }])) + .toEqual([['status', 'equals', 'in_progress']]); + // The shipped `showcase_task.in_progress` view — the case objectui#3431 was + // verified against a real backend. + expect(toFilterNode([rule('is_empty')])).toEqual([['tags', 'is_empty']]); + // A blank `field` is still not a rule, so it is still left unlowered. + expect(toFilterNode([{ field: '', operator: 'equals', value: ['a'] }])) + .toEqual([{ field: '', operator: 'equals', value: ['a'] }]); + }); +}); + +// --------------------------------------------------------------------------- +// 4. The arity classes PARTITION the spec's vocabulary — the ratchet +// --------------------------------------------------------------------------- + +describe('objectui#8557 — the arity classification cannot drift from the spec', () => { + it('assigns every VIEW_FILTER_OPERATORS member to exactly one arity class', () => { + // The list and pair classes are the spec's own exports. The valueless class + // is the one written out in the source, because the spec exports no set for + // it — so this pin is what keeps it honest. A new spec operator lands in the + // scalar remainder by default, which is the REFUSING class, so this pin is + // also the review trigger for that choice. + const valueless = ['is_empty', 'is_not_empty', 'is_null', 'is_not_null']; + const arrayValued: string[] = [ + ...VIEW_FILTER_LIST_VALUE_OPERATORS, + ...VIEW_FILTER_PAIR_VALUE_OPERATORS, + ]; + const all = [...VIEW_FILTER_OPERATORS] as string[]; + + for (const op of [...valueless, ...arrayValued]) expect(all).toContain(op); + expect(arrayValued).toEqual(['in', 'not_in', 'between']); + + const scalar = all.filter((op) => !valueless.includes(op) && !arrayValued.includes(op)); + // The refusing class, spelled out. If the spec adds an operator this list + // changes and the reader has to say which class it belongs to. + expect(scalar).toEqual([ + 'equals', 'not_equals', 'contains', 'not_contains', 'icontains', + 'starts_with', 'ends_with', 'greater_than', 'less_than', + 'greater_than_or_equal', 'less_than_or_equal', 'before', 'after', + ]); + // Exact partition: no operator in two classes, none in none. + expect(scalar.length + valueless.length + arrayValued.length).toBe(all.length); + }); +}); + +// --------------------------------------------------------------------------- +// 5. Where the refusal lands — the blast radius, as an envelope contract +// --------------------------------------------------------------------------- + +describe('objectui#8557 — the throw is the blast radius the object arm already had', () => { + it('raises the SAME error class and envelope as the object arm`s refusals', () => { + // `plugin-list`'s `buildEffectiveFilter` and `plugin-view`'s `ObjectView` + // both call this sink inside their load `try`, and have had to survive a + // throw from it since objectui#8530. `classifyLoadError` reads this code and + // status, which is what makes a malformed saved view render as "the filter + // is malformed" rather than as a network fault (objectui#3066). A bare + // `Error` here would classify as the latter. + const fromViewArm = captureRefusal(() => toFilterNode([rule('equals', ['a'])])); + const fromObjectArm = captureRefusal(() => toFilterNode({ tags: ['a'] })); + expect(fromViewArm.constructor).toBe(fromObjectArm.constructor); + expect(fromViewArm.code).toBe(fromObjectArm.code); + expect(fromViewArm.httpStatus).toBe(fromObjectArm.httpStatus); + // Same envelope, DIFFERENT diagnosis — the reader must be able to tell which + // door refused, because the fix differs (a rule to edit vs. a literal). + expect(fromViewArm.message).toContain('stored view rule'); + expect(fromObjectArm.message).not.toContain('stored view rule'); + }); +}); diff --git a/packages/core/src/utils/filter-converter.ts b/packages/core/src/utils/filter-converter.ts index bead9c555b..47d5aa378b 100644 --- a/packages/core/src/utils/filter-converter.ts +++ b/packages/core/src/utils/filter-converter.ts @@ -13,7 +13,12 @@ * to ObjectStack FilterNode AST format. */ -import { normalizeFilterOperator } from '@objectstack/spec/ui'; +import { + normalizeFilterOperator, + VIEW_FILTER_OPERATORS, + VIEW_FILTER_LIST_VALUE_OPERATORS, + VIEW_FILTER_PAIR_VALUE_OPERATORS, +} from '@objectstack/spec/ui'; import { isAcceptedFilterComparand } from '@objectstack/spec/data'; /** @@ -461,9 +466,135 @@ function isViewFilterRule(value: unknown): value is ViewFilterRuleLike { * turns a hole in an array into `null`, and `['x', 'equals', null]` is a real * `{x: null}` predicate, i.e. a silently-wrong filter. Same rule the write side * applies (`if (c.value !== undefined)`). + * + * The value's SHAPE is checked too, against the operator's arity — an array on a + * single-value operator is refused rather than passed through. See the arm + * itself for the reasoning and for why the refusal is a throw (objectui#8557). + * + * @throws {FilterOperatorError} If the rule carries an ARRAY on an operator the + * spec declares single-valued. + */ +/** + * The view-filter operators whose `value` is legitimately an ARRAY. + * + * Not a local list — both halves are the spec's own, and it exports them for + * exactly this question: `VIEW_FILTER_LIST_VALUE_OPERATORS` (`['in', 'not_in']`) + * says which operators take a membership list, `VIEW_FILTER_PAIR_VALUE_OPERATORS` + * (`['between']`) which take a `[min, max]` pair, and the docblock on the first + * of them names a hard-coded `["in", "notIn"]` elsewhere in this repo as the + * mistake it exists to prevent (`notIn` is an alias, not the canonical member). + * Same reason the operator itself goes through `normalizeFilterOperator` rather + * than a second map. */ +const ARRAY_VALUED_VIEW_OPERATORS: ReadonlySet = new Set([ + ...VIEW_FILTER_LIST_VALUE_OPERATORS, + ...VIEW_FILTER_PAIR_VALUE_OPERATORS, +]); + +/** + * The view-filter operators that carry NO comparand — their direction comes from + * the operator NAME. + * + * The spec exports the two array-valued sets but no set for these, so this is + * the one classification written out here. It is pinned against + * `VIEW_FILTER_OPERATORS` (filter-view-rule-arity-8557.test.ts) so the four sets + * partition the vocabulary exactly: an operator added to the spec lands in no + * class, the pin reddens, and someone classifies it rather than it silently + * inheriting a verdict. + * + * They are excluded from the refusal below deliberately, and on a measurement: + * the spec DISCARDS a value on these operators — `parseFilterAST(['tags', + * 'is_null', ['a']])` is `{ tags: { $null: true } }` — so a stray array here + * cannot produce a node that selects the wrong rows. Refusing it would turn a + * harmless input into a render-time throw, and the message would prescribe `in` + * for an operator that takes no value at all. + */ +const VALUELESS_VIEW_OPERATORS: ReadonlySet = new Set([ + 'is_empty', + 'is_not_empty', + 'is_null', + 'is_not_null', +]); + +/** Every operator spelling the spec knows, canonical forms only. */ +const KNOWN_VIEW_OPERATORS: ReadonlySet = new Set(VIEW_FILTER_OPERATORS); + function viewFilterRuleToNode(rule: ViewFilterRuleLike): FilterNode { const operator = normalizeFilterOperator(rule.operator as string); + + // An ARRAY on an operator that takes ONE value is refused here — objectui#8557. + // + // `rule.value` used to travel through unread, so a stored view rule + // `{ field: 'tags', operator: 'equals', value: ['a'] }` became + // `['tags', 'equals', ['a']]`. Measured against @objectstack/spec 17.3.0, the + // spec's doors pass that node through unjudged — `isFilterAST` is `true` and + // `parseFilterAST` hands back `{ tags: ['a'] }` — which is the SAME + // array-in-a-scalar-slot shape the object arm above refuses (objectui#8530): + // the ObjectQL AST has no array-equality node, `@objectstack/driver-sql` + // answers `400 INVALID_FILTER`, and every in-memory matcher excludes every + // row. So a hand-authored `{ tags: ['a'] }` already failed fast with a message + // naming `$in`, while the same mistake SAVED INTO A VIEW still travelled + // silently to a 400 the author could not attribute to their filter. This + // closes that asymmetry. + // + // Keyed on the operator's ARITY, never on `Array.isArray(value)` alone: + // `in` / `not_in` / `between` rules legitimately carry arrays through this + // very function, and that path had no pin protecting it until this card. + // The check runs AFTER normalization so an alias is judged by what it means — + // `nin` is `not_in`, and is array-valued. + // + // An operator the spec does NOT know is left alone. `normalizeFilterOperator` + // passes a misspelling through verbatim precisely so `isFilterAST` refuses it + // and the server names it; refusing it HERE would report the wrong problem + // ("use $in") for what is actually a typo. + // + // ## Why the refusal is a THROW, and why it belongs here + // + // It throws from the lowering, which for a saved view means the view fails at + // render rather than returning a narrower answer. That is a real blast radius + // and it was weighed: + // + // - It is not a NEW blast radius. Both sinks already catch a + // `FilterOperatorError` from this same file — `plugin-list`'s + // `buildEffectiveFilter` runs inside `ListView`'s load `try` and lands in + // `setLoadError`, `plugin-view`'s `ObjectView` calls `mergeFilterNodes` + // inside its own load `try` — because the object arm has thrown for + // `$regex`, `$not` and bare arrays since objectui#8530. `classifyLoadError` + // reads this error's `INVALID_FILTER` / `400`, so what a user sees is the + // "filter is malformed" panel, not a crashed page and not a network error. + // - The alternatives are both silent. DROPPING the rule widens the result + // set, the one direction this file exists to avoid — and a stored view's + // whole purpose can be to hide rows. Rewriting `equals` into `in` changes + // the author's meaning, the lenient second contract objectui#8514 was + // resolved against on this same data shape. + // - The rule was authored long ago by someone who is not present, which + // argues for the LOUDER answer, not the quieter one: a silent 400 two + // layers away is attributable to nothing, whereas this names the view's + // field and operator at the moment the filter is built. + if ( + Array.isArray(rule.value) + && typeof operator === 'string' + && KNOWN_VIEW_OPERATORS.has(operator) + && !ARRAY_VALUED_VIEW_OPERATORS.has(operator) + && !VALUELESS_VIEW_OPERATORS.has(operator) + ) { + throw new FilterOperatorError( + `[ObjectUI] The stored view rule on field '${rule.field}' carries an ARRAY ` + + `as the comparand of '${operator}', which takes a single value: ` + + `${JSON.stringify(rule.value)}. It cannot be lowered: the ObjectQL filter ` + + `AST has no array-equality node, so the lowered node ` + + `[${rule.field}, '${operator}', [...]] is refused by @objectstack/driver-sql ` + + `(400 INVALID_FILTER) and matches no row in the in-memory matchers — it can ` + + `never select anything. It is deliberately NOT rewritten to 'in': that ` + + `would change what the saved view means, and 'in' is already spellable in ` + + `the rule itself. Spell membership as ` + + `{ field: '${rule.field}', operator: 'in', value: [...] }, its negation as ` + + `'not_in', or a range as { operator: 'between', value: [min, max] } — the ` + + `three operators the spec declares array-valued, which are untouched ` + + `(objectui#8557; the same ruling objectui#8530 applied to the object arm).` + ); + } + return ( rule.value === undefined ? [rule.field, operator]