Skip to content

Commit a8bcce6

Browse files
fix(service-analytics)!: both analytics doors refuse the two $icontains comparands FILTER_TEXT_CASES declares refused, each in its own envelope (#20068) (#20096)
Fixes #20068 Clause-②: no (narrowing) ## What this changes `FILTER_TEXT_CASES` (`@objectstack/spec/data`) declares two REJECTION rows for `$icontains`: an EMPTY comparand and a NON-STRING one. Each has `code: 'INVALID_FILTER'` and `mustMention: ['$icontains']`. The spec publishes the discrimination as `isRefusedTextComparand` and the reason half as `textComparandRefusalReason` (#18113). `FilterConditionSchema` and `driver-sql` (#5702) refuse both rows. `service-analytics` never asked, so one filter got two answers. Both analytics doors now call the published predicate and seat the published reason in their own envelope. Neither restates the rule. - **The `where` door** (`strategies/filter-normalizer.ts`, `assertCompilableComparand`). The door is inside the #7693 text fence, after the renderability check. It refuses as `INVALID_FILTER` / 400, with the message `[analytics] The` + the published reason. The message names `$icontains`, the spelling that arrived; a `FilterArray` `icontains` has been lowered to `$icontains` before any comparand gate runs. This covers every consumer of the tree at once: the native SQL execute path, the `/analytics/sql` echo, the ObjectQL engine path, `AnalyticsService.query`, and dataset and measure filters through `queryDataset`. - **The read scope, lowering** (`read-scope-sql.ts`, `compileOperator`'s `case '$icontains'`, via the new `assertIcontainsComparandNotRefused`). The check comes after `assertRenderableText` and before `textOverNonTextColumn` and any bind. It refuses as `READ_SCOPE_COMPILE_FAILED` / 500, with the message withheld (#5367, re-affirmed as #7598 Q2 = A). This covers `compileScopedFilterToSql`, `NativeSQLStrategy.applyReadScope` and the echo. - **The read scope, at the engine-bound merges** (`read-scope-sql.ts`, `assertReadScopeComparandsRunnable`, via the new walk `findRefusedIcontainsComparand`). It runs after the two shared faces, in the same envelope. `ObjectQLStrategy.withReadScope` and `resolveFkAttr` already call this function, so `objectql-strategy.ts` is untouched. This bullet goes past the claim's file note (`case '$icontains'`); why it is here is under **Declared scope extension** below. Only the two declared rows. `$contains`, `$notContains`, `$startsWith` and `$endsWith` keep their answer to `''`. The module says widening by analogy is the table's decision. ### The order chosen, and why - **Where door.** Existing refusals keep their order. The shape and type faces in `lowerAnalyticsWhere` still answer first: an object is refused by the type face, in its sentence. The #5234 renderability check still answers an array or a `{ $field }` next. The new question comes third, still inside `fieldLeaves`, so it runs before any emitter. The 2026-09-05 non-text-column constant is untouched and answers only a comparand the contract accepts. `driver-sql` takes the same stance: its constant is reached only after its validating walk. The predicate's `undefined` carve-out is owned upstream: the type face and `assertDefinedComparands` refuse an `undefined` before this line can see one. - **Read scope, lowering.** The same order inside the arm: `assertRenderableText` first, so an object or array keeps its #5234 sentence. Then the table's rows. Then the non-text constant. `undefined` is refused earlier, by `assertDefinedComparands` in `compileField`. - **Read scope, merges.** After `assertListComparandShapes` and `normalizeFilterComparandTypes`. A doubly-refused scope therefore carries the faces' sentence, as it would on the engine, and the type face has already refused `undefined`, the carve-out. ### Declared scope extension (in-place, one file) The claim names the read scope's `case '$icontains'`. Measured before the fix, the same cell on the ObjectQL execute face did not serve rows. It reached `engine.aggregate`, and `driver-sql`'s #5702 refusal came back as `INVALID_FILTER` / 400, with the policy's field name, path and comparand in the relayed message. That is the envelope the dispatch rules out for a read scope: never a 4xx carrying policy content. A gate in `compileOperator` alone would have left one scope with two envelopes. All four in-place conditions hold: 1. It is the same defect class and the same cell. 2. It is mechanical: the same published predicate, re-raised through `readScopeCompileError`, in the function #19995 and #20018 built for exactly this. 3. `read-scope-sql.ts` is already in this claim, and #20075 is queued behind it. 4. It adds no verification surface. It needs no engine-lane export, which is what separates it from the four classes in #19995's decision. #19995 remains open. **The seat is asked to amend the claim's file note** to include `assertReadScopeComparandsRunnable`. ## Measured first (recorded on this branch as `fe47363cd9`, before any source change) Base `origin/main` `7ddf396109`. A real sql.js engine (`driver-sqlite-wasm`) over rows a1 'Acme Corp', a2 'ACME ltd', a3 'beta', a4 NULL and a5 '' (field `name`), with a real `ObjectQL` engine behind the ObjectQL face. The echo's SQL was run on the same database. The HTTP legs were read and measured only: `packages/runtime`'s dispatcher route and `packages/rest`'s dataset route, each over a real `AnalyticsService`. **The `where` door (caller-authored):** | cell | native execute | echo (run) | ObjectQL engine path | draft preview | `AnalyticsService.query` (native) | |:--|:--|:--|:--|:--|:--| | `$icontains: ''` | a1 a2 a3 a5 (every non-NULL row) | a1 a2 a3 a5 | bare `Error`, no code (see finding 1) | 400 `INVALID_FILTER`, names `$icontains` ("not evaluated") | a1 a2 a3 a5 | | `$icontains: 'acme'` (control) | a1 a2 | a1 a2 | bare `Error`, no code | 400, not evaluated | a1 a2 | | `$icontains: 42` / `true` / `null` | no row (bound as text) | no row | bare `Error` | 400, not evaluated | no row | | `$icontains: {a:1}` | 400, type face (every face) | = | = | = | = | | `$not` over `$icontains: ''` | a4 only | a4 only | bare `Error` | 400, not evaluated | a4 only | | `FilterArray` `['name','icontains','']` | a1 a2 a3 a5 | a1 a2 a3 a5 | bare `Error` | no row (the preview does not lower a `FilterArray`) | a1 a2 a3 a5 | | `$contains: ''` (no-widening control) | a1 a2 a3 a5 | a1 a2 a3 a5 | a1 a2 a3 a5 | a1 through a5 | a1 a2 a3 a5 | **The read scope (policy, host `getReadScope`):** | cell | `compileScopedFilterToSql` (run) | native `applyReadScope` | echo (run) | ObjectQL execute (`withReadScope`) | |:--|:--|:--|:--|:--| | `$icontains: ''` | a1 a2 a3 a5, `instr(lower("acct"."name"), lower(?)) > 0` with `''` | a1 a2 a3 a5 | a1 a2 a3 a5 | 400 `INVALID_FILTER` from `driver-sql`, message relayed with field, path and comparand | | `$icontains: 'acme'` (control) | a1 a2 | a1 a2 | a1 a2 | a1 a2 | | `$icontains: 42` / `true` / `null` | no row (bound as text) | no row | no row | 400 relayed | | `$icontains: {a:1}` | 500 `READ_SCOPE_COMPILE_FAILED`, withheld (every face) | = | = | = | | `$not` over `$icontains: ''` | a4 only | a4 only | a4 only | 400 relayed | | `$contains: ''` (no-widening control) | a1 a2 a3 a5 | = | = | = | **The HTTP door (read and measure only; `packages/rest` and `packages/runtime` are not edited):** - `POST /api/v1/analytics/query` and `/analytics/sql` type `where` as `FilterConditionSchema`. `$icontains: ''` and `42` get **400 `VALIDATION_FAILED`** carrying the spec's reason at `where.name.$icontains`; the service is never called. `'acme'` gets 200. A `FilterArray` `where` is refused whole, because the door is object-only. - `POST /analytics/dataset/query`: `selection.runtimeFilter` or an inline `dataset.filter` carrying `''` or `42` gets **400 `VALIDATION_FAILED`** at the door parse. A **host `getReadScope`** carrying `$icontains: ''` got **200, served**, with the statement's `WHERE` carrying the `''` pattern. The read-scope cell reached HTTP callers; the caller-authored cell did not. **Read-scope reach.** `compileCelToFilter` never emits `$icontains`. `name.icontains('x')` and `name.lowerAscii().contains('x')` are refused as `unsupported`, and `STRING_METHOD` maps only `contains` / `startsWith` / `endsWith`. A placeholder cannot resolve to `''`: `resolveFilterTokens` throws `FILTER_TOKEN_UNRESOLVED` for an empty `userId` / `tenantId`, and the date macros resolve to dates. **Verdict:** today, only a host-supplied `getReadScope`, or a direct caller of the exported `compileScopedFilterToSql`, carries `$icontains: ''` into a read scope. ## After (head `ec20b23f97`) - Every `where` face refuses `''`, `42`, `true`, `null` and a `Date` under `$icontains` as `INVALID_FILTER` / 400, in both spellings and at any depth. Nothing reaches the database or the engine: 0 statements, 0 `engine.aggregate` calls. `queryDataset` refuses the same comparand in a stored dataset filter or a measure filter. - Every read-scope face (native, echo, ObjectQL execute, `AnalyticsService.query` either way) refuses as `READ_SCOPE_COMPILE_FAILED` / 500. The provenance is `declared`, `declaredRefusalMessage` is `undefined`, and nothing reaches the database or the engine. - HTTP re-measure: the dataset route with a host `getReadScope` carrying `$icontains: ''` now answers **500 `{"error":"Internal server error","code":"READ_SCOPE_COMPILE_FAILED"}`**, with 0 statements. Every caller-authored cell is unchanged: 400 at the door parse. - Controls: `'acme'` still serves a1 a2 on every face that could before. `$contains: ''` still serves a1 a2 a3 a5 on every face, in both the `where` and the read scope. - The draft preview is not edited. It already refuses every `$icontains` in the same envelope, `INVALID_FILTER` / 400 naming `$icontains` (#19810), so the verdict for this cell agrees; only the sentence differs. ## Tests New file `packages/services/service-analytics/src/__tests__/icontains-text-comparand-refusal.test.ts`, 44 cases: - the `where` door's rows, both spellings, every position; - sentence precedence for an array and an object; - every face over a real engine, with statement and engine-call counters; - the non-text-column order; - every read-scope face, withheld; - the export's lowering-time placement; - stored dataset and measure filters; - the no-widening block. Every refusal asserts `code` + `status`, and the `where` ones also assert that the message contains `textComparandRefusalReason(field, '$icontains', comparand)`. Pins re-judged, none deleted. The census was over `service-analytics`, `packages/rest` and `packages/runtime`, taken before the change; `rest` and `runtime` had 0 hits. - `cross-field-reference-refusal.test.ts`, "the fence is NARROW": it asserted `$icontains: 5` and `$icontains: null` compile as legitimate comparands. The table declares both refused. **Flipped to refusals, with a note; the `'admin'` control is kept.** - `text-match-sqlite-nul.test.ts`: its header said these compilers answer `$icontains: ''` with every non-NULL row. **The prose is re-judged.** The grid's exclusion of that cell stays, since there are no rows to hold. Head `ec20b23f97`: - `pnpm --filter @objectstack/service-analytics` suite: **124 files, 2886 tests passed**. - `typecheck`: exit 0. `--listFiles` includes all 3 touched test files. - `packages/rest`'s five analytics route suites, run against the rebuilt `dist`: 68 passed. That is a consumer sanity run, not a pin. ### Ablations (each through `scripts/ablation-replace.mjs`, WRAP mode, anchor hit ×1) `service-analytics` tests import the subject by relative `src` paths, so no `dist` is in the path. | leg | mutation | red / green | restore | |:--|:--|:--|:--| | A | `where` gate disabled (`if (false && …)`) | 26 red / 136 green, over the new file plus `cross-field-reference-refusal.test.ts`. The re-judged pin is red too; the preview case stays green, as predicted | blob `164f4ad4fa40` == HEAD, `git diff HEAD` empty | | B | read-scope arm gate removed | 1 red / 43 green: the lowering-time placement pin (a doubly-refused scope is answered for the text comparand). The merge walk still refuses every other scope in the same envelope, so this is the arm's only observable, as predicted | blob `403440964f36` == HEAD | | C | merge walk disabled | 6 red / 38 green: the six read-scope refusal cases, where the ObjectQL faces fell back to the driver's relayed 400 | blob == HEAD | | D | `where` gate widened to all four case-exact operators | 2 red / 42 green: both no-widening pins | blob == HEAD | | E | read-scope `$contains` arm given the gate | 2 red / 42 green: both no-widening pins | blob == HEAD | ## Gates (head `ec20b23f97`) `node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack --commands` derived 61 families from the real diff. That is 6 more than the dispatch-time list: `check:engine-double-contract`, `check:objectql-double-limit`, `check:query-options-erasure`, `check:type-check-coverage`, `check:type-check-debt`, `check:where-matcher`. - 60 families: exit 0. Among them are `check-adr-0087-registration --base origin/main` (disposition `not-required (already-registered filter-icontains-comparand-refused-at-parse)`), `check-changeset-no-major`, `check-empty-changeset`, `check:nul-bytes` and `check:issue-citations` (22 citations, all resolve). - `check:type-check-debt`: exit 0, after building `@objectstack/runtime` (the first run was `PREREQUISITE NOT MET`). - `check:dual-build-cjs-loads`: **NOT MEASURED** (exit 3, `PREREQUISITE NOT MET`). It reads every package's `dist`, and 38 are unbuilt in this worktree. The diff changes no build config, `exports` or `package.json`. As a narrow proxy, `service-analytics/dist/index.cjs` loads under `require` (17 exports). CI builds the whole closure. - `--ran` reconciliation: 61 derived, 60 run, 1 NOT-MEASURED (claimed, with its reason), 0 UNRUN. - Lint, narrowed and proven: `eslint --no-inline-config --format json` over the 5 changed `.ts` files gave 5 files, 0 errors, 0 warnings. `isPathIgnored` answers false for them. `eslint.config.mjs` never enables type-aware linting, so this diff cannot move the verdict of any untouched file. ## Acceptance notes - **`$contains: ''` in a read scope, from CEL (not filed).** Measured: `compileCelToFilter('name.contains(current_user.department)')` with an empty `department` lowers to `{ name: { $contains: '' } }`, which admits every non-NULL row on every face. `$contains` has no REJECTION row, and this card must not widen by analogy. Whether the table should add one, or the CEL lowering should refuse an empty string-method argument, is the table owner's call. It is recorded here, not filed; carrier: none. - **The draft preview does not lower a `FilterArray` `where` (not filed).** `evaluateAnalyticsQueryOverRows` hands the array to `matchesWhere` as an object keyed `0` / `1` / `2`, so every `FilterArray` answers no row. The published door lowers it. Reachability through `queryDataset` (dataset and runtime filters are object-only at every schema door) was not proven, so this stays a note; carrier: none. - **Precedence change on the read scope.** A scope carrying both `$icontains: ''` and a shape only the shared faces refuse (such as a `null` `$in` member) is now answered for the text comparand, at lowering time. The envelope is the same and only the log sentence differs. That is the placement the ruling asked for. ## Out-of-scope findings (reported to the seat, not filed here) 1. **class (a).** `ObjectQLStrategy.convertFilter` has no `icontains` arm. Any `$icontains`, including a valid `'acme'`, on a query the ObjectQL strategy serves throws a bare `Error` with no `code` or `status`: "ObjectQL strategy cannot express filter operator "icontains"". Measured in-process through `ObjectQLStrategy.execute` and `AnalyticsService.query` with `objectqlAggregate`, which returned no rows. The HTTP door admits the filter, so a caller on a datasource served by the ObjectQL strategy gets a 5xx for a valid query; that HTTP leg is read from the code, not measured. After this PR, `''` and non-strings are refused before `convertFilter`; a valid comparand still fails there. Dedupe words: `ObjectQLStrategy convertFilter icontains` · `cannot express filter operator icontains` · `analytics objectql icontains bare Error`. --- _Generated by [Claude Code](https://claude.ai/code/session_01Evb5jFDZGKQE9KG4jbMfMF)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 0318faf commit a8bcce6

6 files changed

Lines changed: 689 additions & 10 deletions

File tree

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
---
2+
"@objectstack/service-analytics": minor
3+
---
4+
5+
fix(service-analytics)!: both analytics doors refuse the two `$icontains` comparands `FILTER_TEXT_CASES` declares refused, an empty one and a non-string one, each door in its own envelope (#20068)
6+
7+
Clause-②: no (narrowing)
8+
9+
<!-- adr-0087: not-required (already-registered filter-icontains-comparand-refused-at-parse) this is the transition that entry already records: its surface names the $icontains comparand in FilterConditionSchema, read-scope rules and analytics filters included, empty or not a string, and its replacement is a non-empty string or no condition. This change adds no new transition; it brings the analytics compile faces under the refusal the entry declares, which the parse door and the drivers already make. -->
10+
11+
**BREAKING**: this narrows what the analytics faces of `@objectstack/service-analytics` accept. An `$icontains` condition whose comparand is the empty string, or is not a string at all (a number, a boolean, `null`, a `Date`), used to compile on the analytics compilers. It is now refused before any SQL statement runs or any `engine.aggregate` call is made. It ships as `minor` under the launch-window convention for accept-set narrowings.
12+
13+
`@objectstack/spec` declares both refusals as data: `FILTER_TEXT_CASES` carries a REJECTION row for an empty `$icontains` comparand and one for a non-string comparand, each `INVALID_FILTER` naming `$icontains`. It publishes the discrimination as `isRefusedTextComparand` and the reason as `textComparandRefusalReason`. The spec's parse door (`FilterConditionSchema`) and `driver-sql` already refused both. This package never asked, so one filter got two answers. Both analytics doors now call the published predicate and seat the published reason in their own envelope.
14+
15+
| where the condition sits | before | now |
16+
|:--|:--|:--|
17+
| a caller's `where`, a dataset `filter` or a measure `filter`, either spelling, at any depth | `''` matched every row whose column has a value; a non-string was bound as its text and matched nothing. Native SQL, the `/analytics/sql` echo and `AnalyticsService.query` all served it | `INVALID_FILTER` / 400, with the spec's reason, naming `$icontains` |
18+
| a row-level read scope, on the native SQL face and the echo | the same predicate: `''` admitted every row that has a value | `READ_SCOPE_COMPILE_FAILED` / 500, with the message withheld |
19+
| a row-level read scope, on the ObjectQL face | the driver refused it as `INVALID_FILTER` / 400, and the message named the policy's field and comparand | `READ_SCOPE_COMPILE_FAILED` / 500, with the message withheld |
20+
21+
The migration is the ledger entry named above: write a non-empty string, or drop the condition. An empty comparand was a predicate that constrained nothing, so the repair is to delete the condition. A number, boolean or `null` comparand is written as the string it was meant to match, or the operator was the wrong one.
22+
23+
Over HTTP, `POST /api/v1/analytics/query`, `/analytics/sql` and `/analytics/dataset/query` already refused a caller-authored condition carrying either comparand, at their body parse (`400 VALIDATION_FAILED`). What this changes for an HTTP caller is the read scope. A row-level read scope supplied by the host (`getReadScope`) is refused in the withheld envelope on every analytics face. It is no longer served on the native SQL face, and it is no longer answered with a 4xx that carries policy content on the ObjectQL face. The CEL policy lowering never emits `$icontains`, and a filter placeholder never resolves to an empty string.
24+
25+
Who is affected: nothing in this repository's examples, seeds, docs or package sources authors either comparand; every hit outside tests is a code comment. Stored datasets, dashboard filters and host-supplied read scopes in a deployment were NOT measured. A stored row saved before the parse-door refusal can still carry an empty comparand, and it is now refused at query time, where it used to answer every row that has a value.
26+
27+
Not changed: a non-empty string comparand, whatever its case or content; an object or array comparand, still refused in its existing sentence; the non-text-column constant for an accepted comparand. `$contains`, `$notContains`, `$startsWith` and `$endsWith` keep their answer to an empty comparand: the table has no REJECTION row for them, and widening by analogy is the table's decision.

‎packages/services/service-analytics/src/__tests__/cross-field-reference-refusal.test.ts‎

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -550,12 +550,20 @@ describe('[#7693] `$icontains` is fenced on the `where` door, like its four sibl
550550
expect(tree({ name: { $icontains: 'admin' } })).toEqual({
551551
kind: 'leaf', member: 'name', operator: 'icontains', values: ['admin'],
552552
});
553-
expect(tree({ name: { $icontains: 5 } })).toEqual({
554-
kind: 'leaf', member: 'name', operator: 'icontains', values: [5],
555-
});
556-
expect(tree({ name: { $icontains: null } })).toEqual({
557-
kind: 'leaf', member: 'name', operator: 'icontains', values: [null],
558-
});
553+
// [#20068] RE-JUDGED: `5` and `null` were in this control group as
554+
// "legitimate" comparands, and they are not. `FILTER_TEXT_CASES` declares a
555+
// non-string `$icontains` comparand REFUSED (`INVALID_FILTER`, naming
556+
// `$icontains`), and this door now asks the published predicate, so both are
557+
// refused here as they are at the spec's parse door and on `driver-sql`.
558+
// Kept, flipped: the fence stays narrow for a non-empty string (above) and
559+
// refuses exactly the table's rows. `icontains-text-comparand-refusal.test.ts`
560+
// carries the rows on every face.
561+
for (const refused of [5, null]) {
562+
const err = refusalOf(() => tree({ name: { $icontains: refused } }));
563+
expect(err.code).toBe('INVALID_FILTER');
564+
expect(err.status).toBe(400);
565+
expect(err.message).toContain('$icontains');
566+
}
559567
// …and the sibling door is unmoved, which is the no-regression half.
560568
expect(scope({ name: { $icontains: 'admin' } }).params).toEqual(['%admin%', '\\']);
561569
});

0 commit comments

Comments
 (0)