Repository navigation
Commit 536f2d5
Fixes #20098
Clause-②: no
## What this changes
`ObjectQLStrategy.convertFilter`
(`packages/services/service-analytics/src/strategies/objectql-strategy.ts`)
translates each normalized filter leaf into the `FilterCondition`
operand the engine expects. The normalizer emits an `icontains` leaf for
`$icontains`, and for the `FilterArray` `icontains`, which
`parseFilterAST` lowers to `$icontains` first. `convertFilter` had no
arm for that leaf, so every `$icontains` fell to its catch-all and threw
a bare `Error` with no `code` and no `status`. That included a valid
`'acme'`.
The new arm is one line in the shape the four sibling text arms use:
```ts
case 'icontains': return { $icontains: String(v0) };
```
- It passes the canonical spec operator through. `FILTER_OPERATORS`
declares `$icontains`, and `StringOperatorSchema` types it `z.string()`.
- The ASCII-only fold (#4706 Q1 = A) stays with the engine and the
driver, as on every other face. ⛔ No local fold, no `$regex` (#5557 is
the precedent), and no second implementation of the text semantics.
- PR #20096's comparand refusals still come first and are unchanged. The
`where` door refuses an empty or non-string `$icontains` before a leaf
exists, so `String(…)` is an identity on everything that can reach this
arm. Ablation D below measures what that ordering protects.
- The same arm serves the other two callers of `filterNodeToCondition`:
a compiled dataset's scope and a measure filter.
- The catch-all throw is untouched. It stays loud for an operator the
normalizer never emits.
## Measured first (recorded on this branch as `75a7c920aa`, before any
source change)
Base `origin/main` `7c1039b388`. The fixture is a real `ObjectQL` engine
over a real `SqliteWasmDriver` (sql.js). Rows (field `name`): a1 'Acme
Corp', a2 'ACME ltd', a3 'beta', a4 NULL, a5 '', a6 'CAFÉ', a7 'café'.
The echo's SQL was run on the same database. The throwaway harnesses
were not committed.
**In process (the `where` door, caller-authored):**
| cell | `engine.find` direct | `ObjectQLStrategy.execute` |
`AnalyticsService.query` (ObjectQL) | native execute | echo (run) |
`AnalyticsService.query` (native) |
|:--|:--|:--|:--|:--|:--|:--|
| `$icontains: 'acme'` | a1 a2 | bare `Error`, `code` undefined,
`status` undefined: "[analytics] ObjectQL strategy cannot express filter
operator "icontains"" | same bare `Error` | a1 a2 | a1 a2 | a1 a2 |
| `$icontains: 'ACME'` | a1 a2 | bare `Error` | bare `Error` | a1 a2 |
a1 a2 | a1 a2 |
| `$icontains: 'CAFÉ'` (ASCII-only fold) | a6 | bare `Error` | bare
`Error` | a6 | a6 | a6 |
| `$icontains: 'café'` | a7 | bare `Error` | bare `Error` | a7 | a7 | a7
|
| `$not` over `$icontains: 'acme'` | a3 a4 a5 a6 a7 | bare `Error` |
bare `Error` | a3 a4 a5 a6 a7 | a3 a4 a5 a6 a7 | a3 a4 a5 a6 a7 |
| `FilterArray` `['name','icontains','acme']` | a1 a2 | bare `Error` |
bare `Error` | a1 a2 | a1 a2 | a1 a2 |
| `$contains: 'acme'` (case-exact control) | (none) | (none) | (none) |
(none) | (none) | (none) |
**The HTTP door, `POST /api/v1/analytics/query`, with an ObjectQL-served
datasource.** The card recorded this leg as a code reading. This PR
measures it: `packages/runtime`'s real route (`createDispatcherPlugin`,
the fake-server shape `analytics-query-read-scope-withhold.test.ts`
uses) over a real `AnalyticsService` imported from source, with
`objectqlAggregate: true`, `nativeSql: false`, and `executeAggregate`
bound to `engine.aggregate`. Read and measured only; `packages/runtime`
is not edited.
| cell | status | `body.error` | `engine.aggregate` calls |
|:--|:--|:--|:--|
| `$icontains: 'acme'` | **500** | `INTERNAL_ERROR`, message
"[analytics] ObjectQL strategy cannot express filter operator
"icontains". Treating it as an equality …" relayed | 0 |
| `$icontains: 'CAFÉ'` | 500 | same | 0 |
| `$not` over `$icontains: 'acme'` | 500 | same | 0 |
| `FilterArray` `['name','icontains','acme']` | 400 |
`VALIDATION_FAILED`, `where`: expected record, received array (the door
is object-only) | 0 |
| `$icontains: ''` (the #20068 refusal) | 400 | `VALIDATION_FAILED` at
`where.name.$icontains`, the spec's reason | 0 |
| `$contains: 'cme'` (control) | 200 | rows a1 | 1 |
**`convertFilter`'s input vocabulary against its arms.** Each
`FILTER_OPERATORS` spelling was sent through
`normalizeAnalyticsFilterTree`, then the ObjectQL strategy, the engine
and the native face:
| spelling | leaf operator(s) the normalizer emits | `engine.find` |
ObjectQL execute | native | verdict |
|:--|:--|:--|:--|:--|:--|
| `$eq` | `equals` | a3 | a3 | a3 | covered |
| `$ne` | `notSet`, `notEquals` | a1 a2 a4 a5 a6 a7 | same | same |
covered |
| `$gt` / `$gte` / `$lt` / `$lte` | `gt` / `gte` / `lt` / `lte` | equal
on all three faces | = | = | covered |
| `$in` / `$nin` | `in` / `notSet`, `notIn` | equal on all three | = | =
| covered |
| `$between` | `gte`, `lte` | a2 a5 a6 | same | same | covered |
| `$contains` / `$notContains` / `$startsWith` / `$endsWith` |
same-named leaves | equal on all three | = | = | covered |
| `$icontains` | `icontains` | a1 a2 | **bare `Error`** | a1 a2 |
**missing: fixed here** |
| `$null` / `$exists` | `notSet` / `set` | a4 | a4 | a4 | covered |
| `$like` / `$ilike` (staged, not in `FILTER_OPERATORS`) | refused at
the door, `INVALID_FILTER` / 400 | a1 a2 (the engine answers) | refused
at the door | refused at the door | intentionally refused: the spec's
staging table names `service-analytics` a REFUSING face |
| `$regex` (retired) | refused at the door, `INVALID_FILTER` / 400 |
refused, `INVALID_FILTER` / 400 | refused at the door | refused at the
door | intentionally refused |
`icontains` is the only operator the normalizer emits that
`convertFilter` has no arm for. No other operator is missing, so this PR
does not widen.
## After (head `109691058d`, `origin/main` `6780e34af5` merged in)
- In process, every cell above answers on every face, and each face
answers the rows `engine.find` answers. `'acme'` / `'ACME'` give a1 a2.
`'CAFÉ'` gives a6 only and `'café'` gives a7 only. `$not` gives a3 a4 a5
a6 a7. The `FilterArray` gives a1 a2.
- HTTP re-measure: `$icontains: 'acme'` is **200, rows a1 a2**, with 1
engine call. `'CAFÉ'` is 200, a6. `$not` is 200, a3 a4 a5 a6 a7. The
`FilterArray` and `''` rows are unchanged: 400 `VALIDATION_FAILED` at
the door parse.
- The engine receives `{ name: { $icontains: 'acme' } }` for `'acme'`
(the comparand verbatim, whatever its case), and nothing else.
## Tests
New file
`packages/services/service-analytics/src/__tests__/objectql-icontains-arm.test.ts`,
27 cases, over a real `ObjectQL` + `SqliteWasmDriver`:
- Six cells: `'acme'`, `'ACME'`, `'CAFÉ'`, `'café'`, `$not` and the
`FilterArray` spelling. Each takes its reference from `engine.find`
first, then asserts the same row ids on `ObjectQLStrategy.execute`,
`AnalyticsService.query` (ObjectQL), native execute and
`AnalyticsService.query` (native).
- The engine receives the canonical `$icontains` with the comparand
verbatim (`'acme'`, `'ACME'`, `'CAFÉ'`).
- A dataset scope and a measure filter reach the same arm, with the
native face as the control.
- Every `FILTER_OPERATORS` spelling: a completeness assertion over the
sample table, then per spelling the engine's rows (non-trivial) on the
ObjectQL and native faces, with every `$` key the engine received drawn
from the declared vocabulary.
- The catch-all CONTROL, through the private translator: an operator the
normalizer never emits still throws, naming it. Every leaf operator the
normalizer emits for the declared vocabulary translates without a throw.
`icontains-text-comparand-refusal.test.ts` (PR #20096): its `CONTROL`
case had excluded the ObjectQL faces from the `'acme'` row "while
`convertFilter` had no `icontains` arm". It now runs every face. **Every
refusal case in that file is unchanged and green.**
At head `109691058d`:
- `pnpm --filter @objectstack/service-analytics exec vitest run
--maxWorkers=2`: **128 files, 3017 tests passed**.
- `pnpm --filter @objectstack/service-analytics run typecheck`: exit 0.
`tsc --noEmit --listFiles` includes all three touched `.ts` files.
### Ablations (each through `scripts/ablation-replace.mjs`, WRAP mode,
anchor hit x1, all at head `109691058d`)
The tests import the subject by relative `src` path, and
`@objectstack/objectql` is aliased to `src` in this package's vitest
config, so no `dist` is in the path.
| leg | mutation | red / green (two files, 71 cases, unless noted) |
restore |
|:--|:--|:--|:--|
| A (reverse verification) | the `icontains` arm removed | **11 red** /
60 green: all six cells, the verbatim pin, dataset scope + measure
filter, the `$icontains` vocabulary row, the emitted-leaf translation
case, and PR #20096's widened control. Its refusal cases stay green: the
door refuses before the arm | blob `91aacf6cf586` == HEAD, `git diff
HEAD` empty |
| B (local fold) | `String(v0).toLowerCase()` in the arm | **2 red** /
69 green: `'CAFÉ'` answered **a7 instead of a6** on the ObjectQL face,
and the verbatim pin. `'acme'` / `'ACME'` / `'café'` stay green, as
predicted: only the non-ASCII upper-case row shows a local fold | blob
== HEAD |
| C (catch-all) | `default:` returns `v0`, the equality #4128 removed |
**1 red** / 26 green (new file only): the catch-all CONTROL | blob ==
HEAD |
| D (door first) | #20068's `where`-door refusal disabled
(`filter-normalizer.ts`) | **6 red** / 38 skipped (PR #20096's file, `-t
ObjectQL`): on both ObjectQL faces, `$icontains: 42` was **answered
`[]`**, laundered to `'42'` by the arm's `String(…)`. `''` and `$not`
over `''` reached the engine (1 call instead of 0). This measures why
the refusal must stay first | blob `a7bdec971782` == HEAD |
| E (folded onto the case-exact sibling) | the arm returns `$contains` |
**8 red** / 63 green: `'acme'`, `'ACME'`, `$not`, `FilterArray`, the
verbatim pin, dataset scope + measure filter, the `$icontains`
vocabulary row, PR #20096's control. `'CAFÉ'` / `'café'` stay green, as
predicted: case-exact and ASCII-folded agree on those two | blob == HEAD
|
The first attempt at leg C was a no-op. Its replacement contained the
anchor text, so the tool refused (`anchor count moved 1 -> 1`),
restored, and ran no test. It was re-run with a non-overlapping
replacement, which is the reading above.
## Gates (head `109691058d`)
`node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack
--commands` derived 60 families from the real diff (4 paths, +383 /
-12). 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`. `--ran` reconciliation:
**60 derived, 60 run, 0 NOT-MEASURED, 0 UNRUN** (a derived zero: every
line carries its exit code).
- All 60 families: exit 0. Among them: `check-adr-0087-registration
--base origin/main` ("this PR adds no declared-breaking changeset"),
`check-changeset-no-major`, `check-empty-changeset`, `check:nul-bytes`,
`check:cross-package-test-inputs` and `check:test-source-alias`.
`check:type-check-debt` re-measured 4 ledger entries with none above its
number. `check:dual-build-cjs-loads` passed: 105 published require entry
points across 67 packages load.
- On the previous head (`08dd00660f`, before `origin/main` moved again),
the same 60 were run. Two refused their prerequisite (exit 3) until
their closure was built: `check:type-check-debt` then gave exit 0, and
`check:dual-build-cjs-loads` stayed NOT MEASURED there. Both are
measured green at this head.
- `GITHUB_TOKEN=… node scripts/check-issue-citations.mjs`: exit 0, 8
citations, all resolve. The `--census` spelling takes a workflow value
and is NOT MEASURED locally.
- Lint, narrowed and proven: `eslint --no-inline-config --format json`
over the 3 changed `.ts` files gave 3 files, 0 errors, 0 warnings.
`isPathIgnored` answers false for each. `eslint.config.mjs` never
enables type-aware linting (no `parserOptions.project`), so this diff
cannot move the verdict of any untouched file.
- The rebuilt `service-analytics/dist/index.js` and `index.cjs` each
carry `return { $icontains: String(v0) }` once, and `index.cjs` loads
under `require` (17 exports).
## Acceptance notes
- **`String(…)` and the ordering it relies on.** The arm follows its
four siblings' `String(…)`. Today that is an identity, because the door
refuses every empty or non-string `$icontains` first. Ablation D shows
the cost if that door ever regressed: on this face, `42` would be
laundered to `'42'` and answered with no row. Passed unconverted,
`driver-sql`'s own refusal would have been a second net. PR #20096's
pins go red either way, so the door's absence cannot land silently. The
sibling shape was kept, as the dispatch specified.
- **The `FilterArray` spelling reaches this face in process only.**
`POST /analytics/query` types `where` as an object and refuses an array
whole (400 `VALIDATION_FAILED`, measured above). The spelling reaches
`ObjectQLStrategy` through direct `AnalyticsService.query` callers, and
its row pin here covers that.
- **The catch-all still throws a bare `Error`.** After this PR, no
operator the normalizer emits reaches it; the vocabulary block pins
that. An arrival would mean the two tables drifted again, which is a
server fault by construction, so the uncoded 500 stays as it was.
- The fixture adds two rows (a6 'CAFÉ', a7 'café') to #20068's five. The
pair differs only in the case of a letter the fold must not touch, which
is what makes the ASCII-only boundary observable.
## Out-of-scope findings
None. The vocabulary enumeration above found no other operator that
`convertFilter` lacks and the engine accepts. `$like` / `$ilike` /
`$regex` are refused at the door on purpose.
---
_Generated by [Claude
Code](https://claude.ai/code/session_01Evb5jFDZGKQE9KG4jbMfMF)_
---------
Co-authored-by: Claude <noreply@anthropic.com>
1 parent bc80e16 commit 536f2d5
4 files changed
Lines changed: 383 additions & 12 deletions
File tree
- .changeset
- packages/services/service-analytics/src
- __tests__
- strategies
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
254 | 254 | | |
255 | 255 | | |
256 | 256 | | |
257 | | - | |
258 | | - | |
259 | | - | |
260 | | - | |
| 257 | + | |
| 258 | + | |
| 259 | + | |
261 | 260 | | |
262 | 261 | | |
263 | 262 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
| 72 | + | |
| 73 | + | |
| 74 | + | |
| 75 | + | |
| 76 | + | |
| 77 | + | |
| 78 | + | |
| 79 | + | |
| 80 | + | |
| 81 | + | |
| 82 | + | |
| 83 | + | |
| 84 | + | |
| 85 | + | |
| 86 | + | |
| 87 | + | |
| 88 | + | |
| 89 | + | |
| 90 | + | |
| 91 | + | |
| 92 | + | |
| 93 | + | |
| 94 | + | |
| 95 | + | |
| 96 | + | |
| 97 | + | |
| 98 | + | |
| 99 | + | |
| 100 | + | |
| 101 | + | |
| 102 | + | |
| 103 | + | |
| 104 | + | |
| 105 | + | |
| 106 | + | |
| 107 | + | |
| 108 | + | |
| 109 | + | |
| 110 | + | |
| 111 | + | |
| 112 | + | |
| 113 | + | |
| 114 | + | |
| 115 | + | |
| 116 | + | |
| 117 | + | |
| 118 | + | |
| 119 | + | |
| 120 | + | |
| 121 | + | |
| 122 | + | |
| 123 | + | |
| 124 | + | |
| 125 | + | |
| 126 | + | |
| 127 | + | |
| 128 | + | |
| 129 | + | |
| 130 | + | |
| 131 | + | |
| 132 | + | |
| 133 | + | |
| 134 | + | |
| 135 | + | |
| 136 | + | |
| 137 | + | |
| 138 | + | |
| 139 | + | |
| 140 | + | |
| 141 | + | |
| 142 | + | |
| 143 | + | |
| 144 | + | |
| 145 | + | |
| 146 | + | |
| 147 | + | |
| 148 | + | |
| 149 | + | |
| 150 | + | |
| 151 | + | |
| 152 | + | |
| 153 | + | |
| 154 | + | |
| 155 | + | |
| 156 | + | |
| 157 | + | |
| 158 | + | |
| 159 | + | |
| 160 | + | |
| 161 | + | |
| 162 | + | |
| 163 | + | |
| 164 | + | |
| 165 | + | |
| 166 | + | |
| 167 | + | |
| 168 | + | |
| 169 | + | |
| 170 | + | |
| 171 | + | |
| 172 | + | |
| 173 | + | |
| 174 | + | |
| 175 | + | |
| 176 | + | |
| 177 | + | |
| 178 | + | |
| 179 | + | |
| 180 | + | |
| 181 | + | |
| 182 | + | |
| 183 | + | |
| 184 | + | |
| 185 | + | |
| 186 | + | |
| 187 | + | |
| 188 | + | |
| 189 | + | |
| 190 | + | |
| 191 | + | |
| 192 | + | |
| 193 | + | |
| 194 | + | |
| 195 | + | |
| 196 | + | |
| 197 | + | |
| 198 | + | |
| 199 | + | |
| 200 | + | |
| 201 | + | |
| 202 | + | |
| 203 | + | |
| 204 | + | |
| 205 | + | |
| 206 | + | |
| 207 | + | |
| 208 | + | |
| 209 | + | |
| 210 | + | |
| 211 | + | |
| 212 | + | |
| 213 | + | |
| 214 | + | |
| 215 | + | |
| 216 | + | |
| 217 | + | |
| 218 | + | |
| 219 | + | |
| 220 | + | |
| 221 | + | |
| 222 | + | |
| 223 | + | |
| 224 | + | |
| 225 | + | |
| 226 | + | |
| 227 | + | |
| 228 | + | |
| 229 | + | |
| 230 | + | |
| 231 | + | |
| 232 | + | |
| 233 | + | |
| 234 | + | |
| 235 | + | |
| 236 | + | |
| 237 | + | |
| 238 | + | |
| 239 | + | |
| 240 | + | |
| 241 | + | |
| 242 | + | |
| 243 | + | |
| 244 | + | |
| 245 | + | |
| 246 | + | |
| 247 | + | |
| 248 | + | |
| 249 | + | |
| 250 | + | |
| 251 | + | |
| 252 | + | |
| 253 | + | |
| 254 | + | |
| 255 | + | |
| 256 | + | |
| 257 | + | |
| 258 | + | |
| 259 | + | |
| 260 | + | |
| 261 | + | |
| 262 | + | |
| 263 | + | |
| 264 | + | |
| 265 | + | |
| 266 | + | |
| 267 | + | |
| 268 | + | |
| 269 | + | |
| 270 | + | |
| 271 | + | |
| 272 | + | |
| 273 | + | |
| 274 | + | |
| 275 | + | |
| 276 | + | |
| 277 | + | |
| 278 | + | |
| 279 | + | |
| 280 | + | |
| 281 | + | |
| 282 | + | |
| 283 | + | |
| 284 | + | |
| 285 | + | |
| 286 | + | |
| 287 | + | |
| 288 | + | |
| 289 | + | |
| 290 | + | |
| 291 | + | |
| 292 | + | |
| 293 | + | |
| 294 | + | |
| 295 | + | |
| 296 | + | |
| 297 | + | |
| 298 | + | |
| 299 | + | |
| 300 | + | |
| 301 | + | |
| 302 | + | |
| 303 | + | |
| 304 | + | |
| 305 | + | |
| 306 | + | |
| 307 | + | |
| 308 | + | |
| 309 | + | |
| 310 | + | |
| 311 | + | |
| 312 | + | |
| 313 | + | |
| 314 | + | |
| 315 | + | |
| 316 | + | |
| 317 | + | |
| 318 | + | |
| 319 | + | |
| 320 | + | |
| 321 | + | |
| 322 | + | |
| 323 | + | |
| 324 | + | |
| 325 | + | |
| 326 | + | |
| 327 | + | |
| 328 | + | |
| 329 | + | |
| 330 | + | |
| 331 | + | |
| 332 | + | |
| 333 | + | |
| 334 | + | |
0 commit comments