fix(core): lower $icontains, the canonical operator convertFiltersToAST refused (objectui#8976)

.changeset/8976-icontains-lowers-in-converter.md

@@ -0,0 +1,34 @@
+---
+'@object-ui/core': minor
+---
+
+`convertFiltersToAST` accepts `$icontains`, the canonical case-insensitive `contains`
+the rest of the stack already spoke (objectui#8976).
+
+`$icontains` is a member of `@objectstack/spec`'s `FILTER_OPERATORS`, `ValueDataSource`
+executes it, `FilterConditionField` emits it for its "contains (ignore case)" builder
+row, and `packages/core/src/adapters/README.md` prescribes it as the repair when
+`$like` / `$ilike` / `$regex` are refused. `convertOperatorToAST` had no row for it, so
+the ObjectStack lowering path answered a `FilterOperatorError` (`code:
+'INVALID_FILTER'`, `httpStatus: 400`) with the generic unknown-operator paragraph — the
+one spelling this repo tells an author to write was the one spelling it rejected. An
+admin who picked "contains (ignore case)" in the filter builder authored criteria that
+the in-memory matcher honoured and the ObjectStack data source refused.
+
+`{ name: { $icontains: 'john' } }` now lowers to `['name', 'icontains', 'john']`. The
+value is an identity because `icontains` is itself a member of the spec's
+`VALID_AST_OPERATORS`: unlike `$startsWith` → `startswith` there is no case to squash,
+and the spelling the author writes is the spelling the AST carries. No existing filter
+changes shape — this is a refusal becoming an acceptance, so nothing that lowered
+before lowers differently now.
+
+Two smaller repairs ride along, both consequences of the same gap. The
+unknown-operator message now enumerates `$icontains` among the supported operators, and
+the `$regex` refusal now prescribes it by name for a case-insensitive substring —
+`@objectstack/spec`'s own `FILTER_TEXT_CASES` requires that refusal to mention
+`$icontains`, and it could not while the converter did not accept it.
+
+This is the opposite leg of objectui#8568, which retired four lowercase aliases the
+converter accepted and the matcher refused. There the converter was more tolerant than
+the contract; here it was less tolerant than it. The two needed different fixes: a
+single "make the two sides agree" change would have widened the matcher instead.

packages/core/src/utils/__tests__/filter-icontains-alignment-8976.test.ts

@@ -0,0 +1,229 @@
+/**
+ * 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.
+ */
+
+/**
+ * `$icontains` lowers, and the two faces of the `$` dialect agree about it
+ * (objectui#8976).
+ *
+ * ## The defect
+ *
+ * `$icontains` is a canonical member of `@objectstack/spec`'s
+ * `FILTER_OPERATORS`; `ValueDataSource` executes it; `FilterConditionField`
+ * emits it for the `containsCaseInsensitive` builder row; and
+ * `packages/core/src/adapters/README.md` PRESCRIBES it as the repair for
+ * `$like` / `$ilike` / `$regex`. `convertOperatorToAST` had no row for it, so
+ * `convertFiltersToAST` refused it with the generic unknown-operator paragraph
+ * — an `INVALID_FILTER` / 400 on a spelling this repo tells authors to write.
+ *
+ * That is the OPPOSITE leg of objectui#8568, which retired four lowercase
+ * aliases the converter accepted and the matcher refused. There the converter
+ * was more tolerant than the contract; here it was less tolerant than the
+ * contract. ⛔ The two are not one change: making the acceptance sets "agree"
+ * in a single sweep would have widened the matcher instead.
+ *
+ * ## What is asserted, and why in this shape
+ *
+ * The weak version of this file would be `expect(convertOperatorToAST('$icontains'))
+ * .not.toBeNull()` — true the moment a row is added, and blind to whether the
+ * row means anything. Three stronger claims are made instead.
+ *
+ *   1. **The spelling the adapters README PRESCRIBES must work.** The
+ *      prescriptions are read out of that page's refusal table and intersected
+ *      with the spec's `FILTER_OPERATORS`, so the population comes from prose a
+ *      human wrote and from the contract — never from the function under test.
+ *      This is the claim that fails if the converter is repaired but the page
+ *      keeps prescribing something else, and the claim that would have failed
+ *      before this card in the way an author actually meets the bug.
+ *   2. **Both faces answer the same rows.** `convertFiltersToAST` and
+ *      `ValueDataSource` are probed over one fixture, with a case-SENSITIVE
+ *      control (`$contains`) that must select a strictly smaller set. Without
+ *      that control "both select rows" would pass on a matcher that ignored the
+ *      case fold entirely.
+ *   3. **`icontains` survives to the AST unchanged.** The lowered operator must
+ *      be a member of the spec's `VALID_AST_OPERATORS` — the set that gates
+ *      `isFilterAST()`, and therefore the difference between a filter the wire
+ *      carries and one `driver-sql` silently DROPS (objectstack#3948).
+ *
+ * ⚠️ Deliberately NOT claimed: end-to-end reach from a stored `criteria_json`
+ * into this converter. The criteria store is not in this tree — the
+ * consumer-local caveat objectui#6839 established. What is provable here, and
+ * all this file asserts, is the acceptance-set disagreement and the producer
+ * arm.
+ */
+
+import { describe, it, expect, vi, afterEach } from 'vitest';
+import { existsSync, readFileSync } from 'node:fs';
+import { dirname, join, resolve } from 'node:path';
+import { fileURLToPath } from 'node:url';
+import { FILTER_OPERATORS, VALID_AST_OPERATORS } from '@objectstack/spec/data';
+import { convertFiltersToAST, convertOperatorToAST } from '../filter-converter';
+import { ValueDataSource } from '../../adapters/ValueDataSource';
+
+function repoRoot(): string {
+  let dir = dirname(fileURLToPath(import.meta.url));
+  for (let i = 0; i < 10; i += 1) {
+    if (existsSync(join(dir, 'pnpm-workspace.yaml'))) return dir;
+    dir = resolve(dir, '..');
+  }
+  throw new Error('repo root (pnpm-workspace.yaml) not found from this test file');
+}
+
+const ADAPTERS_README_PATH = 'packages/core/src/adapters/README.md';
+const ADAPTERS_README = readFileSync(join(repoRoot(), ADAPTERS_README_PATH), 'utf8');
+
+/**
+ * The `$`-spellings `packages/core/src/adapters/README.md` tells an author to
+ * write INSTEAD of something it refuses — the third column of its refusal
+ * table, whose header is "write instead".
+ *
+ * Narrowed to members of the spec's `FILTER_OPERATORS` on purpose. That column
+ * also carries `$field` (a comparand marker, not an operator) and prose cells
+ * with no code span at all, and asserting those "lower" would be a category
+ * error. The intersection is still two INDEPENDENT sources — hand-written prose
+ * and the contract — and neither is the function under test.
+ */
+function prescribedOperators(): string[] {
+  const heading = '| spelling | why | write instead |';
+  const start = ADAPTERS_README.indexOf(heading);
+  if (start === -1) {
+    throw new Error(
+      `${ADAPTERS_README_PATH} no longer has the "write instead" refusal table (objectui#8976): `
+        + 're-point this reader at wherever the prescriptions now live. Do not delete the case — '
+        + 'a page that prescribes a spelling the converter refuses is exactly this card',
+    );
+  }
+  const canonical = new Set(FILTER_OPERATORS as readonly string[]);
+  const found = new Set<string>();
+  for (const line of ADAPTERS_README.slice(start).split('\n')) {
+    if (!line.startsWith('|')) break;
+    const cells = line.replace(/^\|/, '').replace(/\|\s*$/, '').split('|');
+    const writeInstead = cells[cells.length - 1] ?? '';
+    for (const match of writeInstead.matchAll(/`(\$[A-Za-z]+)`/g)) {
+      if (canonical.has(match[1])) found.add(match[1]);
+    }
+  }
+  return [...found].sort();
+}
+
+const PRESCRIBED = prescribedOperators();
+
+const ROWS = [
+  { id: 'a', name: 'ACME Corporation' },
+  { id: 'b', name: 'acme holdings' },
+  { id: 'c', name: 'Globex' },
+];
+
+async function selectedIds(filter: unknown): Promise<string[]> {
+  const ds = new ValueDataSource({ items: ROWS });
+  const result = await ds.find('rows', { $filter: filter as any });
+  return result.data.map((r) => r.id as string);
+}
+
+function spyWarn() {
+  return vi.spyOn(console, 'warn').mockImplementation(() => {});
+}
+
+afterEach(() => {
+  vi.restoreAllMocks();
+});
+
+// ---------------------------------------------------------------------------
+// 0. Controls — every population below was really read, and can really fail
+// ---------------------------------------------------------------------------
+
+describe('objectui#8976 — controls', () => {
+  it('read the adapters README prescriptions, and $icontains is among them', () => {
+    // Guards the vacuous pass: a reader that matched nothing would make the
+    // prescription case below iterate zero spellings and report success.
+    expect(
+      PRESCRIBED.length,
+      `no canonical operator was read out of ${ADAPTERS_README_PATH}'s "write instead" column`,
+    ).toBeGreaterThanOrEqual(3);
+    expect(
+      PRESCRIBED,
+      'this card exists because that page prescribes $icontains; if the prescription is gone, '
+        + 'the premise changed and this file must be re-read, not re-pointed',
+    ).toContain('$icontains');
+  });
+
+  it('$icontains is still canonical in the spec, in both vocabularies', () => {
+    // If either of these is ever false, objectui#8976 stops being an
+    // invariant restoration and becomes a ruling. Fail loudly rather than
+    // quietly keeping a row the contract no longer declares.
+    expect(FILTER_OPERATORS).toContain('$icontains');
+    expect(VALID_AST_OPERATORS.has('icontains')).toBe(true);
+  });
+
+  it('the matcher discriminates, so "selects rows" below is not vacuous', () => {
+    // A case-SENSITIVE probe must select a strictly smaller set than the
+    // case-insensitive one. Without this, a matcher that folded case for
+    // everything (or for nothing) would satisfy the agreement case.
+    expect(ROWS.filter((r) => r.name.includes('ACME')).map((r) => r.id)).toEqual(['a']);
+    expect(
+      ROWS.filter((r) => r.name.toLowerCase().includes('acme')).map((r) => r.id),
+    ).toEqual(['a', 'b']);
+  });
+});
+
+// ---------------------------------------------------------------------------
+// 1. The prescription must work — the claim an author actually meets
+// ---------------------------------------------------------------------------
+
+describe('objectui#8976 — every spelling the adapters README prescribes lowers', () => {
+  it.each(PRESCRIBED)('%s is accepted by convertFiltersToAST', (spelling) => {
+    let node: unknown;
+    expect(() => {
+      node = convertFiltersToAST({ name: { [spelling]: 'acme' } });
+    }, `${ADAPTERS_README_PATH} prescribes ${spelling}, and this converter refuses it`).not.toThrow();
+    expect(Array.isArray(node), `${spelling} did not lower to a comparison node`).toBe(true);
+    expect((node as unknown[])[0]).toBe('name');
+  });
+
+  it.each(PRESCRIBED)('%s lowers to an operator the AST gate accepts', (spelling) => {
+    const node = convertFiltersToAST({ name: { [spelling]: 'acme' } }) as unknown[];
+    // `isFilterAST()` is gated on this set; an operator outside it reaches the
+    // wire and is DROPPED rather than refused (objectstack#3948).
+    expect(VALID_AST_OPERATORS.has(node[1] as string)).toBe(true);
+  });
+});
+
+// ---------------------------------------------------------------------------
+// 2. The two faces agree — the disagreement this card measured
+// ---------------------------------------------------------------------------
+
+describe('objectui#8976 — convertFiltersToAST and ValueDataSource agree on $icontains', () => {
+  it('the converter lowers $icontains to `icontains`, unsquashed', () => {
+    // The identity is the point: `icontains` is itself an AST operator, so
+    // unlike `$startsWith` -> `startswith` there is no case to fold away.
+    expect(convertOperatorToAST('$icontains')).toBe('icontains');
+    expect(convertFiltersToAST({ name: { $icontains: 'acme' } })).toEqual([
+      'name',
+      'icontains',
+      'acme',
+    ]);
+  });
+
+  it('the matcher selects the case-insensitive set, with no refusal logged', async () => {
+    const warn = spyWarn();
+    expect(await selectedIds({ name: { $icontains: 'acme' } })).toEqual(['a', 'b']);
+    expect(warn).not.toHaveBeenCalled();
+  });
+
+  it('the case-SENSITIVE sibling still selects the smaller set on both faces', async () => {
+    // The discriminating control for the case above: this is what proves the
+    // agreement is about the case fold and not about the fixture.
+    expect(convertFiltersToAST({ name: { $contains: 'acme' } })).toEqual([
+      'name',
+      'contains',
+      'acme',
+    ]);
+    const warn = spyWarn();
+    expect(await selectedIds({ name: { $contains: 'acme' } })).toEqual(['b']);
+    expect(warn).not.toHaveBeenCalled();
+  });
+});

packages/core/src/utils/filter-converter.ts

@@ -112,6 +112,22 @@ export function convertOperatorToAST(operator: string): string | null {
     '$notContains': 'notcontains',
     '$startsWith': 'startswith',
     '$endsWith': 'endswith',
+    // Case-insensitive contains. A canonical `FILTER_OPERATORS` member that
+    // `ValueDataSource` executes and `FilterConditionField` emits (for its
+    // `containsCaseInsensitive` builder row), while this map refused it with the
+    // generic unknown-operator paragraph — so ONE authored filter selected rows
+    // through the in-memory matcher and 400'd on the ObjectStack lowering path
+    // (objectui#8976). The other direction of the same split objectui#8568 fixed:
+    // there the map was MORE tolerant than the matcher, here it was LESS tolerant
+    // than the contract. Restored by aligning the map with the spec, per AGENTS.md
+    // #0 — not by removing the operator from the builder, which the spec declares.
+    //
+    // The VALUE is an IDENTITY, and that is not a typo: `icontains` is itself a
+    // member of the spec's `VALID_AST_OPERATORS`, so unlike its camelCase siblings
+    // above there is no case to squash. Same identity row, for the same stated
+    // reason, that `FILTER_OPERATOR_ALIASES` carries in
+    // `packages/data-objectstack/src/index.ts`.
+    '$icontains': 'icontains',
   };
 
   return operatorMap[operator] || null;
@@ -526,7 +542,8 @@ export function convertFiltersToAST(filter: Record<string, any>): FilterNode | R
             `converted to 'contains', which matches a literal substring rather than a ` +
             `pattern — a different result, not a degraded one. ` +
             `Field: '${field}', Value: ${JSON.stringify(operatorValue)}. ` +
-            `Use $contains, $startsWith or $endsWith.`
+            `Use $contains for a case-sensitive substring, $icontains for a ` +
+            `case-insensitive one, or $startsWith / $endsWith.`
           );
         }
 
@@ -565,7 +582,7 @@ export function convertFiltersToAST(filter: Record<string, any>): FilterNode | R
           throw new FilterOperatorError(
             `[ObjectUI] Unknown filter operator '${operator}' for field '${field}'. ` +
             `Supported operators: $eq, $ne, $gt, $gte, $lt, $lte, $in, $nin, $between, ` +
-            `$contains, $notContains, $startsWith, $endsWith, $null, $exists. ` +
+            `$contains, $notContains, $startsWith, $endsWith, $icontains, $null, $exists. ` +
             `If you need exact object matching, use the value directly without an operator.`
           );
         }

packages/data-objectstack/README.md

@@ -131,6 +131,16 @@ no aliases: the four lowercase spellings this table used to list beside the
 camelCase keys — `$notin`, `$notcontains`, `$startswith`, `$endswith` — were
 retired by objectui#8568 and moved to the refused table below.
 
+`$icontains` is the case-insensitive member of the `$contains` family, and its
+ObjectStack spelling is the SAME word: `icontains` is itself a member of the
+spec's `VALID_AST_OPERATORS`, so nothing is squashed on the way down. It had no
+row here at all until objectui#8976 — `convertFiltersToAST` refused it as an
+unknown operator while `ValueDataSource` executed it and this repo's own filter
+builder emitted it. The pin now holds these two tables complete against the
+spec's `FILTER_OPERATORS` as well as against the code, so a canonical operator
+that is documented NEITHER as supported NOR as refused fails the suite instead
+of going unnoticed.
+
 | MongoDB Operator | ObjectStack Operator | Example |
 |------------------|---------------------|---------|
 | plain value (no operator) | `=` | `{ status: 'active' }` → `['status', '=', 'active']` |
@@ -147,6 +157,7 @@ retired by objectui#8568 and moved to the refused table below.
 | `$notContains` | `notcontains` | `{ name: { $notContains: 'test' } }` → `['name', 'notcontains', 'test']` |
 | `$startsWith` | `startswith` | `{ email: { $startsWith: 'admin' } }` → `['email', 'startswith', 'admin']` |
 | `$endsWith` | `endswith` | `{ email: { $endsWith: '@example.com' } }` → `['email', 'endswith', '@example.com']` |
+| `$icontains` | `icontains` | `{ name: { $icontains: 'john' } }` → `['name', 'icontains', 'john']` |
 | `$null` | `is_null` / `is_not_null` | `{ email: { $null: true } }` → `['email', 'is_null', true]` |
 | `$exists` | `is_not_null` / `is_null` | `{ email: { $exists: true } }` → `['email', 'is_not_null', true]` |
 
@@ -178,7 +189,7 @@ the call site rather than as a `400` from the server or as an empty list.
 
 | Shape | Why | Example |
 |-------|-----|---------|
-| `$regex` | The spec has no `$regex`, and it is not downgraded to `contains`: a pattern match and a substring match are different questions, not stronger and weaker forms of one. Use `$contains`, `$startsWith` or `$endsWith`. | `{ name: { $regex: '^J' } }` → throws `INVALID_FILTER` |
+| `$regex` | The spec has no `$regex`, and it is not downgraded to `contains`: a pattern match and a substring match are different questions, not stronger and weaker forms of one. Use `$contains` for a case-sensitive substring, `$icontains` for a case-insensitive one, or `$startsWith` / `$endsWith`. | `{ name: { $regex: '^J' } }` → throws `INVALID_FILTER` |
 | `$not` | The AST has no negation keyword, and rewriting the negation inward would be silently partial. Use a negated operator instead: `$ne`, `$nin`, `$notContains`. | `{ $not: { status: 'open' } }` → throws `INVALID_FILTER` |
 | a bare array as a field's value | The AST has no array-equality node, and the array is deliberately not read as `$in` (see below). | `{ tags: ['a', 'b'] }` → throws `INVALID_FILTER` |
 | `$notin` / `$notcontains` / `$startswith` / `$endswith` | Retired lowercase aliases (objectui#8568). The `$` dialect follows `@objectstack/spec`'s spellings, and this repo's in-memory matcher already refused these; accepting them here made one authored filter behave differently depending on the data source behind the view. The refusal names the canonical spelling for the alias you wrote — rename the key, the operator is unchanged. | `{ email: { $startswith: 'a' } }` → throws `INVALID_FILTER` |