fix(spec,drivers): the view filter vocabulary and the AST vocabulary now agree (#3948)

.changeset/view-ast-operator-parity.md

@@ -0,0 +1,41 @@
+---
+"@objectstack/spec": minor
+"@objectstack/driver-sql": patch
+"@objectstack/driver-memory": patch
+---
+
+fix(spec,drivers): the view filter vocabulary and the AST vocabulary now agree (#3948)
+
+`VIEW_FILTER_OPERATORS` (`ui/view.zod.ts`) is what an author may declare on a
+`ViewFilterRule`. `VALID_AST_OPERATORS` (`data/filter.zod.ts`) gates
+`isFilterAST()`, which decides whether a filter is parsed into a query at all.
+They disagreed on **8 of 19** members: `equals`, `not_equals`, `greater_than`,
+`less_than`, `greater_than_or_equal`, `less_than_or_equal`, `before`, `after`.
+
+An author could declare any of them, `ViewFilterRuleSchema` validated them,
+`defineStack` accepted them — and then `isFilterAST()` refused the filter, the
+protocol passed the array through unconverted, and the driver could not apply it.
+Six of the eight were reachable only in theory because ObjectUI's adapter alias
+table happened to translate them; the safety of the query path was resting on a
+hand-written table in another repository being complete, and for `before`/`after`
+it wasn't.
+
+**`AST_OPERATOR_MAP` is now the single source of truth.** `VALID_AST_OPERATORS`
+is derived from its keys rather than restated, so an operator can no longer be
+accepted by the gate without also having a lowering — the two were separate
+hand-written lists that happened to agree, with nothing enforcing it. The map
+gained the eight canonical view spellings plus the squashed/short forms stored
+metadata carries (`notequals`, `greaterthanorequal`, `eq`, `gt`, …).
+
+**New export `canonicalAstOperator(op)`** folds every accepted spelling of one
+comparison onto a single infix form. Both drivers now call it instead of growing
+private alias lists, which is what let them accept different vocabularies.
+`like`/`ilike` are deliberately not folded onto `contains`: driver-sql passes them
+to SQL verbatim, so folding would silently wrap the value in `%…%`.
+
+Widening only — no spelling was removed, so no stored filter stops validating.
+A filter that previously produced an error (after #4029) or was silently dropped
+(before it) now compiles. `filter-view-operator-parity.test.ts` asserts every
+`VIEW_FILTER_OPERATORS` member and every `VIEW_FILTER_OPERATOR_ALIASES` key has a
+lowering that is a real `$`-operator rather than the `$${op}` fallback, so the
+next operator the view layer gains fails a test instead of a query.

packages/plugins/driver-memory/src/memory-driver.ts

@@ -1,6 +1,7 @@
 // Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license.
 
 import type { QueryAST, QueryInput, DriverOptions } from '@objectstack/spec/data';
+import { canonicalAstOperator } from '@objectstack/spec/data';
 import type { IDataDriver } from '@objectstack/spec/contracts';
 import { Logger, createLogger } from '@objectstack/core';
 import { Query, Aggregator } from 'mingo';
@@ -760,7 +761,12 @@ export class InMemoryDriver implements IDataDriver {
    * Convert a single ObjectQL condition to MongoDB operator format.
    */
   private convertConditionToMongo(field: string, operator: string, value: any): Record<string, any> | null {
-    switch (operator) {
+    // Fold every accepted spelling of one comparison onto a single infix form,
+    // so this switch has one case per comparison rather than one per spelling —
+    // `VALID_AST_OPERATORS` accepts `>`, `gt`, `greater_than`, `greaterthan` and
+    // `after` for the same thing. A private alias list here is what let this
+    // driver and driver-sql accept different vocabularies. #3948.
+    switch (canonicalAstOperator(operator)) {
       case '=': case '==':
         return { [field]: value };
       case '!=': case '<>':

packages/plugins/driver-sql/src/sql-driver.ts

@@ -10,6 +10,7 @@
 import type { QueryAST, DriverOptions, SchemaMode } from '@objectstack/spec/data';
 import { parseAutonumberFormat, renderAutonumber, missingFieldValues, isTenancyDisabled, isGlobalUnique, isUniqueDeclared, type AutonumberToken } from '@objectstack/spec/data';
 import { STRUCTURED_JSON_TYPES, FILE_REFERENCE_TYPES, MULTI_OPTION_TYPES, NUMERIC_VALUE_TYPES } from '@objectstack/spec/data';
+import { canonicalAstOperator } from '@objectstack/spec/data';
 import type { IDataDriver } from '@objectstack/spec/contracts';
 import { StorageNameMapping } from '@objectstack/spec/system';
 import { ExternalSchemaModeViolationError } from '@objectstack/spec/shared';
@@ -4766,7 +4767,12 @@ export class SqlDriver implements IDataDriver {
     const where = join === 'or' ? 'orWhere' : 'where';
     const whereNull = join === 'or' ? 'orWhereNull' : 'whereNull';
     const whereNotNull = join === 'or' ? 'orWhereNotNull' : 'whereNotNull';
-    const opLower = String(op).toLowerCase();
+    // Fold every accepted spelling of one comparison onto a single infix form so
+    // the switch below has one case per comparison rather than one per spelling.
+    // `VALID_AST_OPERATORS` accepts `>`, `gt`, `greater_than`, `greaterthan` and
+    // `after` for the same thing; growing a private alias list here is how this
+    // driver and driver-memory drifted apart. #3948.
+    const opLower = canonicalAstOperator(String(op));
 
     // Value comparisons on a mixed-storage column read it through the CASE; every
     // other operator (null predicates, the LIKE family, a malformed `between`)

packages/spec/api-surface.json

@@ -549,6 +549,7 @@
     "WindowFunctionNodeSchema (const)",
     "WindowSpec (type)",
     "WindowSpecSchema (const)",
+    "canonicalAstOperator (function)",
     "canonicalizeSqlType (function)",
     "classifyFilterToken (function)",
     "countAuthorableFields (function)",

packages/spec/src/data/filter-view-operator-parity.test.ts

@@ -0,0 +1,143 @@
+// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license.
+
+/**
+ * The view vocabulary and the AST vocabulary must agree. (#3948)
+ *
+ * `VIEW_FILTER_OPERATORS` (`ui/view.zod.ts`) is what an author may declare on a
+ * `ViewFilterRule`, and what `ViewFilterRuleSchema` validates against.
+ * `VALID_AST_OPERATORS` (`data/filter.zod.ts`) gates `isFilterAST()`, which
+ * decides whether a filter is parsed into a query at all.
+ *
+ * They disagreed on **8 of 19** members — `equals`, `not_equals`,
+ * `greater_than`, `less_than`, `greater_than_or_equal`, `less_than_or_equal`,
+ * `before`, `after`. An author could declare any of them, the schema accepted
+ * them, `defineStack` accepted them, and then `isFilterAST()` refused the filter,
+ * the protocol passed the array through unconverted, and the driver dropped it:
+ * an unfiltered result set with no error anywhere.
+ *
+ * Six of the eight were reachable only in theory, because ObjectUI's adapter
+ * alias table happened to translate them. The safety of the query path was
+ * resting on a hand-written table in a different repository being complete, and
+ * it wasn't — `before`/`after` had no entry, which is how this surfaced.
+ *
+ * `data/` cannot import `ui/` (that direction is already taken, so it would be
+ * circular), which is why the AST map is not literally derived from the view
+ * vocabulary. This test is the enforcement instead: it fails the moment the view
+ * layer gains an operator the AST layer cannot lower.
+ */
+
+import { describe, it, expect } from 'vitest';
+import {
+  VALID_AST_OPERATORS,
+  isFilterAST,
+  parseFilterAST,
+} from './filter.zod';
+import {
+  VIEW_FILTER_OPERATORS,
+  VIEW_FILTER_OPERATOR_ALIASES,
+} from '../ui/view.zod';
+
+/**
+ * View operators that resolve to a value-shape before an operator is ever
+ * emitted, so they legitimately need no AST lowering.
+ *
+ * Empty today: `is_empty`/`is_not_empty` DO have lowerings (to `$null`), because
+ * clients send them as operators rather than resolving them client-side. Kept as
+ * an explicit, empty exemption list so that adding to it is a visible decision
+ * rather than a quiet edit to the assertion.
+ */
+const NO_AST_LOWERING_REQUIRED = new Set<string>([]);
+
+describe('every view filter operator has an AST lowering', () => {
+  it('reads both vocabularies', () => {
+    // Guards the assertions below from passing vacuously.
+    expect(VIEW_FILTER_OPERATORS.length).toBeGreaterThan(0);
+    expect(VALID_AST_OPERATORS.size).toBeGreaterThan(0);
+  });
+
+  it('VALID_AST_OPERATORS covers every canonical view operator', () => {
+    const missing = VIEW_FILTER_OPERATORS
+      .filter((op) => !NO_AST_LOWERING_REQUIRED.has(op))
+      .filter((op) => !VALID_AST_OPERATORS.has(op.toLowerCase()));
+    expect(
+      missing,
+      'an author can declare these on a ViewFilterRule and the schema validates them, '
+        + 'but isFilterAST() refuses the filter — it is passed through unconverted and '
+        + 'the driver cannot apply it. Add a lowering to AST_OPERATOR_MAP.',
+    ).toEqual([]);
+  });
+
+  it('VALID_AST_OPERATORS covers every legacy alias spelling too', () => {
+    // `saveMeta` persists the authored body verbatim, so the schema's own
+    // `z.preprocess` normalization never reaches the stored row — every alias in
+    // this table is live in metadata, not merely historical.
+    const missing = Object.keys(VIEW_FILTER_OPERATOR_ALIASES)
+      .filter((alias) => !NO_AST_LOWERING_REQUIRED.has(VIEW_FILTER_OPERATOR_ALIASES[alias]))
+      .filter((alias) => !VALID_AST_OPERATORS.has(alias.toLowerCase()));
+    expect(
+      missing,
+      'these spellings exist in stored view metadata and have no AST lowering',
+    ).toEqual([]);
+  });
+
+  it.each([...VIEW_FILTER_OPERATORS])('%s survives isFilterAST as a bare triple', (op) => {
+    // The bare triple is the shape that used to vanish: a single AND condition
+    // is emitted as `[field, op, value]`, and when isFilterAST() rejects it the
+    // whole filter is silently dropped.
+    if (NO_AST_LOWERING_REQUIRED.has(op)) return;
+    const value = op === 'in' || op === 'not_in' ? ['a'] : op === 'between' ? [1, 2] : 'x';
+    expect(isFilterAST(['some_field', op, value]), `isFilterAST rejects "${op}"`).toBe(true);
+  });
+
+  it('lowers each view operator to a real $-operator, never the $${op} fallback', () => {
+    // `convertComparison` ends in `{ [field]: { [`$${op}`]: value } }` for an
+    // unmapped operator, which produces e.g. `$before` — a key no driver knows,
+    // so the failure moves from "silently unfiltered" to "driver throws". Both
+    // are wrong; this asserts we produce a real operator.
+    const KNOWN = new Set([
+      '$eq', '$ne', '$gt', '$gte', '$lt', '$lte', '$in', '$nin',
+      '$between', '$contains', '$notContains', '$startsWith', '$endsWith',
+      '$null', '$exists',
+    ]);
+    const bad: string[] = [];
+    for (const op of VIEW_FILTER_OPERATORS) {
+      if (NO_AST_LOWERING_REQUIRED.has(op)) continue;
+      const value = op === 'in' || op === 'not_in' ? ['a'] : op === 'between' ? [1, 2] : 'x';
+      const parsed = parseFilterAST(['some_field', op, value]) as Record<string, unknown>;
+      const arm = parsed.some_field;
+      // The equality shorthand is `{ field: value }` — a bare value, not an
+      // operator object. That is a real lowering, not a fallback.
+      if (arm === null || typeof arm !== 'object') continue;
+      const keys = Object.keys(arm as Record<string, unknown>);
+      if (!keys.every((k) => KNOWN.has(k))) bad.push(`${op} → ${keys.join(',')}`);
+    }
+    expect(bad, 'these fell through to the $${op} fallback').toEqual([]);
+  });
+
+  it('the date comparisons that regressed now lower correctly', () => {
+    expect(parseFilterAST(['close_date', 'before', '2024-01-01']))
+      .toEqual({ close_date: { $lt: '2024-01-01' } });
+    expect(parseFilterAST(['close_date', 'after', '2024-01-01']))
+      .toEqual({ close_date: { $gt: '2024-01-01' } });
+  });
+
+  it('spells equality one way regardless of which alias the author used', () => {
+    const shorthand = { status: 'active' };
+    for (const op of ['=', '==', 'equals', 'eq']) {
+      expect(parseFilterAST(['status', op, 'active']), `via "${op}"`).toEqual(shorthand);
+    }
+  });
+
+  it('keeps null-direction keyed on the operator name, not the filler value', () => {
+    // Clients send a truthy placeholder for both directions.
+    expect(parseFilterAST(['note', 'is_empty', true])).toEqual({ note: { $null: true } });
+    expect(parseFilterAST(['note', 'isempty', true])).toEqual({ note: { $null: true } });
+    expect(parseFilterAST(['note', 'is_not_empty', true])).toEqual({ note: { $null: false } });
+    expect(parseFilterAST(['note', 'isnotempty', true])).toEqual({ note: { $null: false } });
+  });
+
+  it('still refuses an operator in neither vocabulary', () => {
+    // Widening must not turn the gate off.
+    expect(isFilterAST(['some_field', 'sounds_like', 'x'])).toBe(false);
+  });
+});