From e470ac102b93823892a95eb52a525f7e545503cc Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 11 Aug 2026 12:20:06 +0000 Subject: [PATCH 1/2] fix(driver-memory): make the $contains family case-exact and answer count_distinct MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Clears both remaining driver-memory DEBT rows in scripts/check-driver-conformance.mjs, with the suites that replace them. #6682 — the `$contains` family folded case on nine sites: the AST spelling's four arms (`convertConditionToMongo`), the `$`-spelling's four (`normalizeFieldOperators`) and `filterSubstringPattern`, the #5374 shared rule the analytics face borrows rather than re-derives. The `i` flag folds the WHOLE Unicode range, wider even than the ASCII boundary `$icontains` is held to, so the family returned rows the filter excludes — over-reach on an RLS read scope (#3948). This driver's reference matcher was case-exact all along, so the two folding faces move onto the answer the third already gave (#4706 Q2 = A, #5374). `escapeRegex` is untouched; `$icontains` keeps its ASCII fold in the pattern source. #6814 — `MemoryDriver.computeAggregate` had no `count_distinct` arm, so a function the Query Protocol declares fell to `default: return null` and `aggregate()` resolved with `{ n: null }`. It now counts distinct NON-NULL values. Executing the case-set also corrected the card's reading of the analytics face: `buildAggregator` emitted `{ $addToSet }` and nothing sized it, so the measure answered the raw ARRAY under a field its own metadata types as `number`. Fixed beside it, null-excluded. Both cells are enrolled with real in-process executions of the shared case-sets (`memory-filter-text-conformance.test.ts`, `memory-aggregation-conformance.test.ts`), driving every face of the package. Pins that encoded the fold are flipped to the ruled substance rather than deleted. Gate: 40 covered cells, 0 DEBT, 0 exempt — the ledger is empty for the first time. Fixes #6814 Fixes #6682 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01NuzQE844ut8m8vypXcdBYD --- ...memory-contains-case-and-count-distinct.md | 35 +++ .../docs/protocol/objectql/query-syntax.mdx | 35 ++- .../memory-aggregation-conformance.test.ts | 252 ++++++++++++++++ .../driver-memory/src/memory-analytics.ts | 53 +++- ...ry-driver-filter-logic-conformance.test.ts | 20 +- .../driver-memory/src/memory-driver.test.ts | 28 +- .../driver-memory/src/memory-driver.ts | 96 ++++-- .../memory-filter-text-conformance.test.ts | 273 ++++++++++++++++++ .../spec/src/data/aggregation-conformance.ts | 32 +- .../spec/src/data/filter-text-conformance.ts | 34 ++- packages/spec/src/data/filter.zod.ts | 27 +- scripts/check-driver-conformance.mjs | 101 +++---- 12 files changed, 834 insertions(+), 152 deletions(-) create mode 100644 .changeset/memory-contains-case-and-count-distinct.md create mode 100644 packages/drivers/driver-memory/src/memory-aggregation-conformance.test.ts create mode 100644 packages/drivers/driver-memory/src/memory-filter-text-conformance.test.ts diff --git a/.changeset/memory-contains-case-and-count-distinct.md b/.changeset/memory-contains-case-and-count-distinct.md new file mode 100644 index 0000000000..2bbd1b8ceb --- /dev/null +++ b/.changeset/memory-contains-case-and-count-distinct.md @@ -0,0 +1,35 @@ +--- +'@objectstack/driver-memory': patch +--- + +driver-memory: the `$contains` family is case-SENSITIVE, and `count_distinct` answers a number + +Two user-visible answers change on the in-memory driver. Both bring it onto the +answer the SQL family, MongoDB and the protocol already give, so a filter or an +aggregate now means the same thing whether your tests run on this double or your +production runs a real database. + +**`$contains` / `$notContains` / `$startsWith` / `$endsWith` no longer fold +case.** They matched with a case-insensitive regex on the query path and on the +analytics face — over the whole Unicode range, wider even than the ASCII +boundary `$icontains` is held to — so `{ name: { $contains: 'acme' } }` returned +`ACME Corp` here and did not on any other backend. This driver's reference +matcher (`match()`) was already case-exact, so the two folding faces have moved +onto the answer the third one always gave. The comparand stays literal: `%`, +`_` and `.` were never wildcards here and still are not. + +**This is a ROW-SET change.** If you relied on the fold, write `$icontains` — +the operator that spells it, implemented on every backend since #6520 and +folding ASCII case only. + +**`count_distinct` answers.** `MemoryDriver.computeAggregate` had no arm for it, +so an aggregation the Query Protocol declares resolved with `{ alias: null }` — +no error, no log, no refusal. It now counts distinct NON-NULL values, matching +`COUNT(DISTINCT col)`. The analytics face was wrong in its own way and is fixed +beside it: it collected the distinct values and never sized them, so a +`count_distinct` measure came back as the raw array of values under a field its +own response metadata types as `number`. + +Both are held to `@objectstack/spec/data`'s shared case-sets from now on +(`FILTER_TEXT_CASES`, `AGGREGATION_CASES`), executed in process against every +face of the package. diff --git a/content/docs/protocol/objectql/query-syntax.mdx b/content/docs/protocol/objectql/query-syntax.mdx index 6c0f491b63..b1549a25a0 100644 --- a/content/docs/protocol/objectql/query-syntax.mdx +++ b/content/docs/protocol/objectql/query-syntax.mdx @@ -337,15 +337,16 @@ metacharacters — `{ name: { $icontains: 'a.b' } }` matches `a.b` and not `axb` compilers). A filter using it means the same thing whether your tests run on the in-memory double or your production runs SQL. - One half of the case rules above is still landing: `$contains` / + The other half of the case rules above has landed too: `$contains` / `$startsWith` / `$endsWith` / `$notContains` are case-**sensitive** by ruling and - are so on the SQL family and on MongoDB — + are so on every backend — [#6682](https://github.com/objectstack-ai/objectstack/issues/6682) removed the - hardcoded `$options: 'i'` that had folded them there. The in-memory driver's - query and analytics faces still fold over the whole Unicode range. Until that - half lands, prefer `$icontains` when you *want* a fold rather than relying on - `$contains` being loose on that backend. The shared standard both halves are - measured against is `FILTER_TEXT_CASES` (`@objectstack/spec/data`). + hardcoded `$options: 'i'` that folded them on MongoDB and the case-insensitive + regex that folded them on the in-memory driver's query and analytics faces. + **If you were relying on `$contains` being loose on the in-memory driver, that + is a row-set change: write `$icontains` when you want a fold.** The shared + standard both halves are measured against is `FILTER_TEXT_CASES` + (`@objectstack/spec/data`), which all five drivers now run. ### `$regex` — removed @@ -963,19 +964,21 @@ rule in [Case Sensitivity](#case-sensitivity) above. Note what that means for se a user typing `acme` does not find `ACME Corp`. Only `select` / `status` option *labels* are matched case-insensitively by the expansion itself. - - **Measured today: one driver still does not match that rule.** The `$contains` - alignment landed in two steps — + + **Measured today: every driver matches that rule.** The `$contains` alignment + landed in three steps — [#6518](https://github.com/objectstack-ai/objectstack/issues/6518) made `SqlDriver` case-exact per dialect (`GLOB` on the SQLite dialects, `LIKE` unchanged on Postgres, `LIKE` over a binary cast on MySQL), and [#6682](https://github.com/objectstack-ai/objectstack/issues/6682) removed - `driver-mongodb`'s hardcoded `$options: 'i'`. `driver-memory`'s query path still - matches with a case-insensitive regex, so running your tests on the in-memory - double can still return rows a SQL or MongoDB deployment would not. Whether the - expansion should emit `$icontains` instead of `$contains` — i.e. whether search is - case-insensitive by definition — is a separate question that rides with that issue, - because it can only be answered once both operators mean one thing everywhere. + `driver-mongodb`'s hardcoded `$options: 'i'` and then the case-insensitive regex + `driver-memory` used on its query and analytics faces. So running your tests on the + in-memory double no longer returns rows a SQL or MongoDB deployment would not — + the divergence this callout warned about is closed, and + `FILTER_TEXT_CASES` holds all five drivers to it. Whether the expansion should emit + `$icontains` instead of `$contains` — i.e. whether search is case-insensitive by + definition — remains a separate open question, and one that can now actually be + answered, since both operators mean one thing everywhere. `fuzzy`, `boost`, `operator`, `minScore`, `language`, and `highlight` carry `[EXPERIMENTAL — not enforced]` markers (#4286): the schema accepts them, the diff --git a/packages/drivers/driver-memory/src/memory-aggregation-conformance.test.ts b/packages/drivers/driver-memory/src/memory-aggregation-conformance.test.ts new file mode 100644 index 0000000000..fa7c05eea6 --- /dev/null +++ b/packages/drivers/driver-memory/src/memory-aggregation-conformance.test.ts @@ -0,0 +1,252 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#6814] Aggregate-vocabulary conformance for `driver-memory` — the shared + * `@objectstack/spec/data` cases, on rows, executed in process. + * + * The SQL twins (`sql-driver-aggregation-conformance.test.ts`, + * `turso-remote-aggregation-conformance.test.ts`, + * `sqlite-wasm-aggregation-conformance.test.ts`) run this same table against a + * real database. This file is the reason the cases live in the spec package + * rather than beside one driver: an aggregate that answers differently here + * than under SQL pushdown is one query with two numbers, decided by a driver + * capability bit the caller never sees. + * + * ## Why nothing here is a "modelled" evaluation + * + * This driver runs in process, so every case below is a REAL execution — no + * server-free half in the shape `driver-mongodb` needs (#5517), and no emitted- + * string assertion standing in for a number. That matters for the defect this + * file was written against: `computeAggregate` had no `count_distinct` arm at + * all, so the function fell to `default: return null` and `aggregate()` resolved + * with `{ n: null }` — no error, no log, no refusal. Only executing the case + * says so; a lowering-shape assertion has nothing to look at. + * + * ## Both doors of the data face, and the analytics face beside it + * + * `find()` and `aggregate(AST)` are two entries to the same + * `performAggregation`, and objectql's engine uses the second one. Both are + * driven, because "the aggregate works" measured through one door is what let + * this package answer one declared function two ways for as long as it did + * (#5374). The analytics face (`memory-analytics.ts`) is driven in the last + * block for the same reason — it implements `count_distinct` independently, so + * it is a third answer unless something demands they agree. + * + * ## Reverse verification — direction predicted BEFORE it was run + * + * **(A) the `count_distinct` arm removed** (the pre-#6814 state). Predicted: + * the three `count_distinct` cases fail on `null` — the value, not a throw — + * while every arithmetic case stays green, because the missing arm is a silent + * fall-through rather than a broken computation. + * + * **(B) the arm present but written `new Set(values).size`** — null NOT + * excluded, the mistake `driver-mongodb`'s `$addToSet` made (#6814's other + * half). Predicted: `count_distinct(stage)` answers 3 instead of 2 and the + * grouped case answers `west` 3 / `east` 2 instead of 2 / 1, while + * `count_distinct(score)` stays GREEN at 6 — that column has no nulls, so it + * cannot see the mistake. (B) is the direction this file exists for. + * + * Measured after writing the above, of 34: + * + * - **(A) 7 failed / 27 passed.** Every failure was on the VALUE `null` + * (`expected [{ group: null, value: null }] to deeply equal + * [{ group: null, value: 2 }]`), through BOTH doors, plus the + * never-answers-null row — not one on a throw, as predicted. Every + * arithmetic case stayed green. The analytics block stayed green too, which + * is the point of driving the faces separately: this revert is one face's + * defect and the file says which one. + * - **(B) 6 failed / 28 passed**, on `expected 3 to be 2` ungrouped and + * `east` 2 / `west` 3 grouped, through both doors, plus the two analytics + * rows over the same column. `count_distinct(score)` stayed green at 6 + * throughout, exactly as predicted — which is why the table carries both + * columns, and why (B) is unreachable by a suite that only tests one. + * + * Pre-fix, on unmodified `origin/main` @ `21888ab`: **11 failed / 23 passed** — + * (A)'s seven plus four more the analytics face contributed on its own account + * (see the last block). + */ + +import { describe, it, expect, beforeEach } from 'vitest'; +import { AGGREGATION_CASES, AGGREGATION_ROWS } from '@objectstack/spec/data'; +import type { AggregationCase, Cube } from '@objectstack/spec/data'; +import { InMemoryDriver } from './memory-driver.js'; +import { MemoryAnalyticsService } from './memory-analytics.js'; + +const TABLE = 'conformance_agg'; + +/** The case as the `DriverQuery` shape both doors consume. */ +const queryFor = (c: AggregationCase) => ({ + aggregations: [{ function: c.function, ...(c.field ? { field: c.field } : {}), alias: 'n' }], + // [#6401] A case carrying `groupByAlias` is sent as the STRUCTURED node, so + // the face receives the union member that declares `alias`. Without this the + // alias axis would send a bare string and pin nothing. + ...(c.groupBy + ? { groupBy: [c.groupByAlias ? { field: c.groupBy, alias: c.groupByAlias } : c.groupBy] } + : {}), +}); + +/** + * The rows a case must produce, in the table's own order: `group` ascending for + * a grouped case, one `null`-grouped row otherwise. + * + * [#6401] The group value is read from the column the case SAYS it lands in — + * `groupByAlias ?? groupBy`. Reading `c.groupBy` unconditionally is the mistake + * this axis exists to catch: green on a face that ignores the alias. + * + * `value` is deliberately NOT coerced with `Number()`. The defect this file was + * written against answers `null`, and `Number(null)` is `0` — a coercion here + * would turn "no arm at all" into an ordinary off-by-one and hide the shape of + * the failure. + */ +const actualFor = (c: AggregationCase, rows: Array>) => { + const groupKey = c.groupByAlias ?? c.groupBy; + return rows + .map((r) => ({ group: groupKey ? String(r[groupKey]) : null, value: r.n })) + .sort((x, y) => String(x.group).localeCompare(String(y.group))); +}; + +const expectedFor = (c: AggregationCase) => + [...c.expected] + .map((e) => ({ group: e.group, value: e.value })) + .sort((x, y) => String(x.group).localeCompare(String(y.group))); + +async function seed(): Promise { + const driver = new InMemoryDriver(); + for (const row of AGGREGATION_ROWS) await driver.create(TABLE, { ...row }); + return driver; +} + +describe('[#6814] InMemoryDriver — aggregate vocabulary conformance', () => { + let driver: InMemoryDriver; + beforeEach(async () => { driver = await seed(); }); + + /** + * The fixture first, read back rather than trusted — a case that answers 2 + * because only two rows landed is not a case that deduplicated correctly, and + * the null-bearing column is the one a seed is most likely to mangle. + */ + it('the fixture is all six rows, with the nulls stored AS nulls', async () => { + const rows = await driver.find(TABLE, { orderBy: [{ field: 'id', order: 'asc' }] }); + expect(rows.map((r: any) => String(r.id))).toEqual(['1', '2', '3', '4', '5', '6']); + for (const r of rows as any[]) { + const seeded = AGGREGATION_ROWS.find((s) => s.id === String(r.id))!; + expect([r.region, r.stage, r.score], r.id).toEqual([seeded.region, seeded.stage, seeded.score]); + } + // The property every null case hangs off, asserted directly: an empty + // string in place of a null keeps the count_distinct cases green at the + // wrong number. + expect((rows as any[]).filter((r) => r.stage === null)).toHaveLength(2); + }); + + for (const c of AGGREGATION_CASES) { + it(`find(): ${c.name}`, async () => { + const rows = await driver.find(TABLE, queryFor(c) as any); + expect(actualFor(c, rows as any[]), c.note ?? c.name).toEqual(expectedFor(c)); + }); + + /** + * The SECOND door onto the same computation — objectql's engine calls + * `aggregate(object, AST)`, not `find()`. Two doors that can disagree is + * this package's recurring defect class (#5374), so neither is trusted to + * stand for the other. + */ + it(`aggregate(AST): ${c.name}`, async () => { + const rows = await driver.aggregate(TABLE, queryFor(c) as any); + expect(actualFor(c, rows as any[]), c.note ?? c.name).toEqual(expectedFor(c)); + }); + } + + /** + * The #4157 shape, asserted as a property rather than per case: an aggregate + * the Query Protocol declares must never resolve with `null`. That is what + * `default: return null` produced here — a wrong ANSWER rather than a wrong + * number, and the one failure mode a value comparison per case could be + * "passed" by if a future case-set row ever expected zero. + */ + it('never answers null for a declared aggregate function', async () => { + for (const c of AGGREGATION_CASES) { + const rows = await driver.find(TABLE, queryFor(c) as any); + for (const row of rows as any[]) { + expect(row.n, `${c.name} — a declared function resolving null is the #6814 defect`).not.toBeNull(); + expect(typeof row.n, c.name).toBe('number'); + } + } + }); +}); + +/** + * [#5374] The ANALYTICS face answers the same function the same way. + * + * This package's recurring defect is not "a face is wrong", it is "the faces + * disagree" — and `count_distinct` was exactly that. #6814 read this face as + * the one that "DOES implement `count_distinct`", which executing it corrects: + * `buildAggregator` emitted `{ $addToSet }` under a comment reading "Will need + * post-processing for count", and no post-processing existed. So the measure + * answered the raw ARRAY — `['won','lost',null]` — under a field + * `measureTypeToFieldType` describes as `number`. + * + * One declared function, three answers: `null` on the data face, an array here, + * and the standard's number nowhere. Aligning the data face alone would have + * left this one free to keep its own. + */ +describe('[#6814] the analytics face answers count_distinct the same number', () => { + const cube: Cube = { + name: 'agg', + title: 'Agg', + sql: TABLE, + measures: { + distinctStage: { name: 'distinct_stage', label: 'Distinct stage', type: 'count_distinct', sql: 'stage' }, + distinctScore: { name: 'distinct_score', label: 'Distinct score', type: 'count_distinct', sql: 'score' }, + }, + dimensions: { + region: { name: 'region', label: 'Region', type: 'string', sql: 'region' }, + }, + } as unknown as Cube; + + let service: MemoryAnalyticsService; + + beforeEach(async () => { + const driver = await seed(); + service = new MemoryAnalyticsService({ driver, cubes: [cube] }); + }); + + /** The ungrouped pair, against the same numbers `AGGREGATION_CASES` states. */ + it('count_distinct(stage) is 2 — distinct NON-NULL values, not 3', async () => { + const result = await service.query({ cube: 'agg', measures: ['agg.distinctStage'] } as any); + expect(result.rows[0]['agg.distinctStage']).toBe(2); + }); + + it('count_distinct(score) is 6 — the all-distinct control', async () => { + const result = await service.query({ cube: 'agg', measures: ['agg.distinctScore'] } as any); + expect(result.rows[0]['agg.distinctScore']).toBe(6); + }); + + /** + * Grouped, because a face computing the aggregate over the whole table and + * repeating it per group answers 2/2 and the ungrouped case above cannot see + * it — the same argument `AGGREGATION_CASES`' grouped row is built on. + */ + it('count_distinct(stage) grouped by region is east 1 / west 2', async () => { + const result = await service.query({ + cube: 'agg', + measures: ['agg.distinctStage'], + dimensions: ['agg.region'], + } as any); + const byRegion = Object.fromEntries( + result.rows.map((r: any) => [r['agg.region'], r['agg.distinctStage']]), + ); + expect(byRegion).toEqual({ east: 1, west: 2 }); + }); + + /** + * The declared TYPE is `number` (`measureTypeToFieldType`), so the value has + * to be one. An `$addToSet` handed back unsized is an ARRAY under a field the + * response describes as numeric — a shape divergence a value comparison alone + * would report as an ordinary wrong number. + */ + it('answers a NUMBER, matching the field type the response declares', async () => { + const result = await service.query({ cube: 'agg', measures: ['agg.distinctStage'] } as any); + expect(result.fields.find((f: any) => f.name === 'agg.distinctStage')?.type).toBe('number'); + expect(typeof result.rows[0]['agg.distinctStage']).toBe('number'); + }); +}); diff --git a/packages/drivers/driver-memory/src/memory-analytics.ts b/packages/drivers/driver-memory/src/memory-analytics.ts index 776eacdd06..42334d3e9c 100644 --- a/packages/drivers/driver-memory/src/memory-analytics.ts +++ b/packages/drivers/driver-memory/src/memory-analytics.ts @@ -259,6 +259,38 @@ const CUBE_OPERATOR_TO_MONGO_PREDICATE: Readonly ({ $exists: raw.length > 0 ? Boolean(raw[0]) : true }), }); +/** + * [#6814] The size of a collected `$addToSet`, as `count_distinct` defines it: + * distinct NON-NULL values of the column. + * + * That is what `COUNT(DISTINCT col)` computes on SQLite, PostgreSQL and MySQL + * alike, what objectql's fallback computes (`in-memory-aggregation.ts`), what + * this package's own data face computes (`MemoryDriver.computeAggregate`), and + * what `AGGREGATION_CASES` says — 2 over `AGGREGATION_ROWS`. + * + * ## Why the exclusion is HERE and not in the `$group` expression + * + * The two server-side spellings were considered and not taken, for the same + * reasons `driver-mongodb`'s twin records (#6814): + * + * - **`$ne: null` before the `$addToSet`** — as a `$match` it drops the row from + * the WHOLE pipeline, so a `count` or `sum` measure sharing the query would + * silently lose the null rows too. Correct only for a pipeline carrying one + * measure, which this builder cannot assume. + * - **`$size` of a `$setDifference` against `[null]`** — sound, but it puts the + * rule in the `$project` stage while the collection stays in `$group`, so the + * two halves of one definition sit in different stages built by different + * methods. Here they are one expression next to its own explanation. + * + * `undefined` is excluded beside `null`: mingo's `$addToSet` skips a MISSING + * field the way MongoDB's does, so this arm sees `undefined` only via an + * explicitly-undefined stored value — one state with `null` in SQL, and there is + * no third. + */ +function sizeDistinctSet(values: readonly unknown[]): number { + return new Set(values.filter((v) => v !== null && v !== undefined)).size; +} + /** * Configuration for MemoryAnalyticsService */ @@ -476,6 +508,22 @@ export class MemoryAnalyticsService implements IAnalyticsService { const tableName = this.extractTableName(cube.sql); const rawRows = await this.driver.aggregate(tableName, pipeline); + // [#6814] `$addToSet` COLLECTS; a `count_distinct` measure has to ANSWER a + // number. Without this step the value reached the caller as the raw array + // of values — under a field `measureTypeToFieldType` describes as `number`, + // so the response's own metadata disagreed with the cell beside it — and it + // included `null`, so even sizing it where it landed would have answered + // one HIGHER than the standard on any nullable column. + if (query.measures) { + for (const measure of query.measures) { + if (this.resolveMeasure(cube, measure)?.type !== 'count_distinct') continue; + const shortName = this.getShortName(measure); + for (const row of rawRows) { + if (Array.isArray(row[shortName])) row[shortName] = sizeDistinctSet(row[shortName]); + } + } + } + // Rename fields from short names to full cube.field names const rows = rawRows.map(row => { const renamedRow: Record = {}; @@ -904,7 +952,10 @@ export class MemoryAnalyticsService implements IAnalyticsService { case 'max': return { $max: `$${fieldPath}` }; case 'count_distinct': - return { $addToSet: `$${fieldPath}` }; // Will need post-processing for count + // Collects the distinct values; {@link sizeDistinctSet} turns the array + // into the NUMBER, excluding null — see the note there for why the + // exclusion is on that side rather than in this expression. + return { $addToSet: `$${fieldPath}` }; default: return { $sum: 1 }; // Default to count } diff --git a/packages/drivers/driver-memory/src/memory-driver-filter-logic-conformance.test.ts b/packages/drivers/driver-memory/src/memory-driver-filter-logic-conformance.test.ts index 4e7c55552e..cef5ef41cd 100644 --- a/packages/drivers/driver-memory/src/memory-driver-filter-logic-conformance.test.ts +++ b/packages/drivers/driver-memory/src/memory-driver-filter-logic-conformance.test.ts @@ -532,11 +532,21 @@ const OPERATOR_CASES: Array<[name: string, where: FilterCondition, expected: str ['$notContains treats a metacharacter as a literal', { name: { $notContains: 'a.p' } } as FilterCondition, ['1', '2', '3']], // ── Case folding, borrowed rather than re-derived ────────────────────────── - // The live path matches `/…/i`; this face built a case-SENSITIVE regex, so one - // `where` meant two different things depending on which face read it (#5240). - ['$contains is case-insensitive, as the live path is', { name: { $contains: 'ALPHA' } } as FilterCondition, ['1']], - ['$notContains is case-insensitive, as the live path is', { name: { $notContains: 'ALPHA' } } as FilterCondition, ['2', '3']], - ['$contains matches a mixed-case comparand', { name: { $contains: 'Bet' } } as FilterCondition, ['2']], + // These rows have always asserted that the two faces answer ONE rule, and they + // still do — what changed is WHICH rule. #5374 pinned the shared rule as the + // live path's `/…/i`, because this face was the one that had drifted then. + // #6682 ruled the whole `$contains` family case-SENSITIVE (#4706 Q2 = A), the + // flag came off `filterSubstringPattern`, and this face moved with the live + // path — which is the property #5374 bought, working in the direction it was + // built for. Flipped to the ruled substance rather than deleted: "the faces + // agree on case" is exactly the assertion that must survive the ruling. + ['$contains is case-SENSITIVE, as the live path is', { name: { $contains: 'ALPHA' } } as FilterCondition, []], + ['$notContains is case-SENSITIVE, as the live path is', { name: { $notContains: 'ALPHA' } } as FilterCondition, ['1', '2', '3']], + ['$contains misses a mixed-case comparand', { name: { $contains: 'Bet' } } as FilterCondition, []], + // The positive control beside them, so the three rows above cannot be passed + // by a predicate that stopped matching anything at all: the exactly-cased + // comparand still selects its row. + ['$contains still matches an exactly-cased comparand', { name: { $contains: 'bet' } } as FilterCondition, ['2']], // ── A pattern is not a comparand (#4047) ─────────────────────────────────── // Every operand used to go through the storage-form conversion, so on a diff --git a/packages/drivers/driver-memory/src/memory-driver.test.ts b/packages/drivers/driver-memory/src/memory-driver.test.ts index 02b3ca0e7b..e29eaec97b 100644 --- a/packages/drivers/driver-memory/src/memory-driver.test.ts +++ b/packages/drivers/driver-memory/src/memory-driver.test.ts @@ -614,10 +614,28 @@ describe('InMemoryDriver', () => { expect(results[0].name).toBe('Evan Davis'); }); - it('should filter with $contains case-insensitively', async () => { + // [#6682] Flipped, not deleted. This row pinned the fold; #4706 Q2 = A rules + // the `$contains` family case-SENSITIVE on every backend, so the same input + // now selects nothing — and the case-insensitive answer it used to assert + // has a spelling of its own, `$icontains`, pinned beside it so the pair + // still covers both behaviours rather than losing one. + it('should filter with $contains case-SENSITIVELY', async () => { const results = await driver.find(testTable, { where: { name: { $contains: 'alice' } }, }); + expect(results).toHaveLength(0); + + const exact = await driver.find(testTable, { + where: { name: { $contains: 'Alice' } }, + }); + expect(exact).toHaveLength(1); + expect(exact[0].name).toBe('Alice Johnson'); + }); + + it('should filter case-insensitively with $icontains, the operator that spells it', async () => { + const results = await driver.find(testTable, { + where: { name: { $icontains: 'alice' } }, + }); expect(results).toHaveLength(1); expect(results[0].name).toBe('Alice Johnson'); }); @@ -668,9 +686,15 @@ describe('InMemoryDriver', () => { expect(results).toHaveLength(3); }); + // [#6682] The subject here is COMBINATION — a `$contains` under `$and` — so + // the comparand moved rather than the expectation: `'a'` matched + // `Alice Johnson` only through the fold this card removed (that name carries + // no lower-case `a`). `'o'` is case-exact for both surviving rows, and it + // also makes the conjunction do visible work: on its own it selects + // `Bob Smith` too, whom the age bound excludes. it('should handle $contains inside $and', async () => { const results = await driver.find(testTable, { - where: { $and: [{ name: { $contains: 'a' } }, { age: { $gte: 30 } }] }, + where: { $and: [{ name: { $contains: 'o' } }, { age: { $gte: 30 } }] }, }); expect(results).toHaveLength(2); expect(results.map((r: any) => r.name).sort()).toEqual(['Alice Johnson', 'Charlie Brown']); diff --git a/packages/drivers/driver-memory/src/memory-driver.ts b/packages/drivers/driver-memory/src/memory-driver.ts index b0c31f8827..7957f9014d 100644 --- a/packages/drivers/driver-memory/src/memory-driver.ts +++ b/packages/drivers/driver-memory/src/memory-driver.ts @@ -804,8 +804,15 @@ export class InMemoryDriver implements IDataDriver { return { [field]: { $in: store(value) } }; case 'nin': case 'not_in': case 'notin': case 'not in': return { [field]: { $nin: store(value) } }; + // [#6682] The `$contains` family is case-SENSITIVE (#4706 Q2 = A), here + // and on every other spelling of it in this file. The `i` flag these four + // arms used to carry was the FULL Unicode fold — wider even than the + // ASCII-only boundary `$icontains` is held to (Q1 = A) — so a filter + // returned rows it excludes, which on an RLS read scope is over-reach + // rather than a loose filter (#3948). `escapeRegex` stays: the comparand + // was always literal, and that half was never the defect. case 'contains': - return { [field]: { $regex: new RegExp(this.escapeRegex(value), 'i') } }; + return { [field]: { $regex: new RegExp(this.escapeRegex(value)) } }; // [#7536] `like` / `ilike` are NOT `contains`, and sharing this arm with // it was the memory-face twin of the wire defect #7536 closed: the // comparand was regex-ESCAPED (so a caller's `%` matched a literal percent @@ -823,11 +830,11 @@ export class InMemoryDriver implements IDataDriver { }; } case 'notcontains': case 'not_contains': - return { [field]: { $not: { $regex: new RegExp(this.escapeRegex(value), 'i') } } }; + return { [field]: { $not: { $regex: new RegExp(this.escapeRegex(value)) } } }; case 'startswith': case 'starts_with': - return { [field]: { $regex: new RegExp(`^${this.escapeRegex(value)}`, 'i') } }; + return { [field]: { $regex: new RegExp(`^${this.escapeRegex(value)}`) } }; case 'endswith': case 'ends_with': - return { [field]: { $regex: new RegExp(`${this.escapeRegex(value)}$`, 'i') } }; + return { [field]: { $regex: new RegExp(`${this.escapeRegex(value)}$`) } }; // Null / empty predicates. These are in `VALID_AST_OPERATORS` and were // absent here, so every one of them fell to `default: return null` and was // dropped — `is_null` narrowed nothing instead of matching null rows. @@ -981,32 +988,39 @@ export class InMemoryDriver implements IDataDriver { for (const op of Object.keys(ops)) { const val = ops[op]; switch (op) { + // [#6682] Case-SENSITIVE, the same four arms as the AST spelling one + // method up (`convertConditionToMongo`) and for the same reason — see + // the note there. The comparand stays `escapeRegex`-literal; only the + // Unicode-folding `i` flag is gone. case '$contains': - regexConditions.push({ $regex: new RegExp(this.escapeRegex(val), 'i') }); + regexConditions.push({ $regex: new RegExp(this.escapeRegex(val)) }); break; case '$notContains': - result.$not = { $regex: new RegExp(this.escapeRegex(val), 'i') }; + result.$not = { $regex: new RegExp(this.escapeRegex(val)) }; break; case '$startsWith': - regexConditions.push({ $regex: new RegExp(`^${this.escapeRegex(val)}`, 'i') }); + regexConditions.push({ $regex: new RegExp(`^${this.escapeRegex(val)}`) }); break; case '$endsWith': - regexConditions.push({ $regex: new RegExp(`${this.escapeRegex(val)}$`, 'i') }); + regexConditions.push({ $regex: new RegExp(`${this.escapeRegex(val)}$`) }); break; // [#6520] `$icontains` — case-insensitive over ASCII and NOTHING else. // - // Note what this arm does NOT do, because every neighbour above does it: - // it never passes the `i` flag. That flag is the FULL Unicode fold, so - // it would match `CAFÉ` against `café` — the answer the SQL family - // cannot give (SQLite folds ASCII only) and therefore the one the - // protocol forbids (#4706 Q1 = A). The fold instead lives in the pattern - // SOURCE, one `[Aa]` class per ASCII letter, built by the spec's shared - // `asciiCaseInsensitiveRegexSource` — the same source `driver-mongodb` - // binds, so the two document-shaped faces fold identically. + // Note what this arm does NOT do: it never passes the `i` flag. That + // flag is the FULL Unicode fold, so it would match `CAFÉ` against + // `café` — the answer the SQL family cannot give (SQLite folds ASCII + // only) and therefore the one the protocol forbids (#4706 Q1 = A). The + // fold instead lives in the pattern SOURCE, one `[Aa]` class per ASCII + // letter, built by the spec's shared `asciiCaseInsensitiveRegexSource` — + // the same source `driver-mongodb` binds, so the two document-shaped + // faces fold identically. // - // The neighbours' `i` flags are NOT a precedent to copy here: they are - // the `$contains` family folding Unicode, which is the open defect #6682 - // tracks on this face, not the behaviour to extend. + // [#6682] The neighbours above carried that flag until this operator's + // sibling family was made case-exact; the two are still not the same + // mechanism, and this arm's pattern-source fold is the only one on this + // face that survives. A bare `new RegExp(v)` beside a bare + // `new RegExp(escapeRegex(v))` is the shape to keep: this arm folds in + // the SOURCE, the family does not fold at all. case '$icontains': regexConditions.push({ $regex: new RegExp(asciiCaseInsensitiveRegexSource(val)) }); break; @@ -1220,6 +1234,28 @@ export class InMemoryDriver implements IDataDriver { return valid.reduce((max, v) => (v > max ? v : max), valid[0]); } + // [#6814] Distinct NON-NULL values — what `COUNT(DISTINCT col)` + // computes on SQLite, PostgreSQL and MySQL alike, what objectql's + // fallback computes (`in-memory-aggregation.ts`, the same expression + // written the same way on purpose) and what `AGGREGATION_CASES` says + // (2 over `AGGREGATION_ROWS`). + // + // This arm was ABSENT, so a function the Query Protocol declares and + // every SQL face lowers (#6409) fell to `default: return null` and + // `aggregate()` resolved with `{ n: null }` — no error, no log, no + // refusal. A wrong ANSWER rather than a wrong number, and the + // `default:`-arm shape the `aggregation-lockstep` guard exists to stop + // one layer up, reached here through a different door (#4157). + // + // The null exclusion is the half a `new Set(values).size` would miss: + // it answers one HIGHER on any nullable column (3 where the standard + // says 2), which is exactly the divergence #6814's driver-mongodb half + // fixed on `$addToSet`. `undefined` is excluded beside `null` for the + // same reason the neighbours above exclude it — a missing key and an + // explicit null are one state in SQL, and this store has both. + case 'count_distinct': + return new Set(values.filter(v => v !== null && v !== undefined)).size; + default: return null; } @@ -1332,15 +1368,27 @@ export class InMemoryDriver implements IDataDriver { * * Same reasoning as {@link filterComparandStorageForm} one method up, on the * other half of what a `contains` predicate needs. This driver's rule is - * `escapeRegex` + the `i` flag ({@link normalizeFieldOperators}): the comparand - * is a LITERAL substring, matched case-insensitively. The analytics face has to + * `escapeRegex` and NO flags ({@link normalizeFieldOperators}): the comparand + * is a LITERAL substring, matched case-EXACTLY. The analytics face has to * build a `$regex` too, and every byte of that rule it re-derives is a way for * the two faces to answer one `where` differently — which is what happened * before this method existed. That face emitted a bare `{$regex: value}`: * - unescaped, so `{name: {$contains: 'a.p'}}` matched `alpha` through the * regex `.`, where `find()` matched nothing; and - * - case-SENSITIVE, so `{name: {$contains: 'ALPHA'}}` matched nothing where - * `find()` matched the row. + * - case-SENSITIVE, where `find()` folded and matched more rows. + * + * [#6682] That second divergence is now closed from the OTHER side, and this + * method is where it closed: the rule lost its `i` flag rather than the + * analytics face gaining one. `find()` was the face that was wrong — the `i` + * flag folded the whole Unicode range, so the `$contains` family returned rows + * the filter excludes (#4706 Q2 = A), and this driver's own reference matcher + * (`memory-matcher.ts`, `String.includes`) had been answering case-exactly the + * whole time. One operator, one answer, three faces (#5374). + * + * Note what does NOT come through here: `$icontains`. Its ASCII-only fold + * lives in the pattern SOURCE (`asciiCaseInsensitiveRegexSource`), which the + * analytics face binds directly — so this method is the `$contains` family's + * rule alone, and taking the flag off it cannot move the ASCII boundary. * * Returning the built `RegExp` rather than a source string is deliberate: a * string leaves the flags for the caller to re-choose, which is the half that @@ -1350,7 +1398,7 @@ export class InMemoryDriver implements IDataDriver { * — so it exposes the convention without exposing the filter pipeline. */ filterSubstringPattern(value: unknown): RegExp { - return new RegExp(this.escapeRegex(value as string), 'i'); + return new RegExp(this.escapeRegex(value as string)); } /** diff --git a/packages/drivers/driver-memory/src/memory-filter-text-conformance.test.ts b/packages/drivers/driver-memory/src/memory-filter-text-conformance.test.ts new file mode 100644 index 0000000000..98cce970cf --- /dev/null +++ b/packages/drivers/driver-memory/src/memory-filter-text-conformance.test.ts @@ -0,0 +1,273 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#6682] Text-operator conformance for `driver-memory` — the WHOLE shared + * `FILTER_TEXT_CASES` table, executed on every face of this package. + * + * This is the file that enrolls this driver's `FILTER_TEXT_CASES` cell: + * `scripts/check-driver-conformance.mjs` judges coverage by whether a package + * names the marker export, so importing it here is the claim that this package + * answers the whole table — every row, refusals included — and the DEBT row + * that stood in its place is deleted in the same PR. + * + * ## Why the cell could not be enrolled before + * + * The table asks three things of a backend, and this package answered two. + * Requirement 1 (`$icontains`, the ASCII-only fold) landed at #6520 on all + * three faces; requirement 3 (`$regex` / `$options` refused inside the + * ADR-0112 envelope) landed at #5702. Requirement 2 — the `$contains` family + * being case-SENSITIVE (#4706 Q2 = A) — is what #6682 closes here, and until it + * did, importing this table would have flipped the cell to "covered" while a + * third of it failed. + * + * ## What was wrong, measured rather than read + * + * The query path and the analytics face lowered the `$contains` family to + * `new RegExp(escapeRegex(v), 'i')` — a literal comparand (so the `%` / `_` / + * `.` rows always held) matched case-INSENSITIVELY over the whole Unicode + * range. The reference matcher (`memory-matcher.ts`, `String.includes`) was + * case-exact all along, so this package answered ONE operator two ways + * depending on which face you entered — the divergence class #5374 closed for + * the same operator between two other faces of the same package. + * + * Both defects were OVER-matching: rows the filter excludes came back. On an + * RLS read scope a wider predicate is over-reach, not a loose filter (#3948). + * + * ## Why every face runs every case + * + * Because this package's recurring defect is not "a face is wrong", it is "the + * faces disagree" — #5374, #5324/#5328, #5347, and #6682 itself. A per-face + * file lets one arm rot without the others noticing, so each case below runs + * through the live query path and the reference matcher and demands one answer; + * the analytics face runs the subset its cube vocabulary can express + * (`contains` / `notContains` / `icontains` — it has no `startsWith` / + * `endsWith` row in `MONGO_TO_CUBE_OPERATOR`). + * + * ## Pre-fix measurement, recorded before the diff existed + * + * Run against unmodified `origin/main` @ `21888ab`: **9 failed / 43 passed** of + * 52. Every failure was a `$contains`-family case — five on the query path + * (`expected ['1','2'] to deeply equal ['2']`, the extra folded row), three on + * the analytics face, and the cross-face agreement row. Every `$icontains` row, + * every literal-comparand row, every refusal row and every reference-matcher + * row passed BEFORE the fix. That is what says the flag was the whole gap and + * that nothing here was widened to reach green: after it, 52/52. + */ + +import { describe, it, expect, beforeEach } from 'vitest'; +import { FILTER_TEXT_CASES, FILTER_TEXT_ROWS } from '@objectstack/spec/data'; +import type { FilterTextCase, Cube } from '@objectstack/spec/data'; +import { InMemoryDriver } from './memory-driver.js'; +import { MemoryAnalyticsService } from './memory-analytics.js'; +import { match } from './memory-matcher.js'; + +const TABLE = 'text_rows'; + +/** The conformance fixture's rows, as this driver stores them. */ +const ROWS = FILTER_TEXT_ROWS.map((r) => ({ ...r })); + +const byId = (a: string, b: string) => a.localeCompare(b); + +async function seed(): Promise { + const driver = new InMemoryDriver(); + for (const row of ROWS) await driver.create(TABLE, { ...row }); + return driver; +} + +/** Ids the LIVE QUERY PATH returns (mingo), ascending. */ +const queryIds = async (driver: InMemoryDriver, filter: unknown): Promise => + (await driver.find(TABLE, { where: filter as any })).map((r: any) => String(r.id)).sort(byId); + +/** Ids the REFERENCE MATCHER returns, ascending. */ +const matcherIds = (filter: unknown): string[] => + ROWS.filter((r) => match(r, filter)).map((r) => r.id).sort(byId); + +const isRejection = (c: FilterTextCase): c is Extract => + c.expectRejection === true; + +const rowCases = FILTER_TEXT_CASES.filter((c) => !isRejection(c)); +const rejectionCases = FILTER_TEXT_CASES.filter(isRejection); + +describe('[#6682] InMemoryDriver — text-operator conformance, the query path', () => { + let driver: InMemoryDriver; + beforeEach(async () => { driver = await seed(); }); + + it('the fixture is all nine rows, stored verbatim', async () => { + const rows = await driver.find(TABLE, { orderBy: [{ field: 'id', order: 'asc' }] }); + expect((rows as any[]).map((r) => [String(r.id), r.name])) + .toEqual(FILTER_TEXT_ROWS.map((r) => [r.id, r.name])); + }); + + for (const c of rowCases) { + it(c.name, async () => { + expect(await queryIds(driver, c.filter), c.note ?? c.name).toEqual([...c.expected]); + }); + } +}); + +describe('[#6682] the reference matcher answers the same table', () => { + for (const c of rowCases) { + it(c.name, () => { + expect(matcherIds(c.filter), c.note ?? c.name).toEqual([...c.expected]); + }); + } +}); + +describe('[#5374] the two general-purpose faces agree, case by case', () => { + let driver: InMemoryDriver; + beforeEach(async () => { driver = await seed(); }); + + /** + * Not redundant with the two blocks above, and this is the row that would + * have been red for the whole life of the defect: a package can satisfy a + * table face-by-face at two different times and still be the thing #6682 was + * filed about in between. Stated as an equality between the faces so it fails + * on the DIVERGENCE rather than on the table. + */ + it('every case returns the same ids through find() and match()', async () => { + for (const c of rowCases) { + expect(await queryIds(driver, c.filter), c.name).toEqual(matcherIds(c.filter)); + } + }); + + /** + * The #3948 pin, as a property: every case here selects a strict subset of + * the fixture, so a face that DROPPED the predicate would answer all nine and + * still look like it worked. Over-matching is the failure mode both halves of + * this card are about. + */ + it('no case answers every row — a dropped predicate WIDENS', async () => { + for (const c of rowCases) { + expect((await queryIds(driver, c.filter)).length, c.name).toBeLessThan(ROWS.length); + expect(matcherIds(c.filter).length, c.name).toBeLessThan(ROWS.length); + } + }); +}); + +/** + * The refusals, on both general-purpose faces. + * + * `code` AND `status`, never a bare `toThrow()` (ADR-0112): a rejection test + * that only asserts "something threw" carries one bit where the defect has two, + * and this package's own history is the argument — #5324 spent an issue routing + * uncoded engine errors back into the envelope, and a throw-only assertion + * cannot tell a correct refusal from an uncoded one. `mustMention` is checked + * because a refusal that does not name the replacement sends the author to the + * docs, which is what `RETIRED_FILTER_OPERATORS` exists to prevent. + */ +describe('[#6682] refusals, in the ADR-0112 envelope, on both faces', () => { + let driver: InMemoryDriver; + beforeEach(async () => { driver = await seed(); }); + + const thrownBy = async (fn: () => unknown | Promise): Promise => { + try { + await fn(); + return null; + } catch (e) { + return e; + } + }; + + for (const c of rejectionCases) { + it(`query path: ${c.name}`, async () => { + const err = await thrownBy(() => driver.find(TABLE, { where: c.filter as any })); + expect(err, `${c.name} — resolving is the silent wrong answer the retirement ended`).toBeInstanceOf(Error); + expect(err.code).toBe(c.code); + expect(err.status).toBe(400); + for (const fragment of c.mustMention) expect(err.message).toContain(fragment); + }); + + it(`reference matcher: ${c.name}`, async () => { + const err = await thrownBy(() => match(ROWS[0], c.filter)); + expect(err, c.name).toBeInstanceOf(Error); + expect(err.code).toBe(c.code); + expect(err.status).toBe(400); + for (const fragment of c.mustMention) expect(err.message).toContain(fragment); + }); + } +}); + +/** + * [#5374] The ANALYTICS face, on the cases its cube vocabulary can express. + * + * This face is not a second copy of the rule — it asks the driver for it + * (`filterSubstringPattern`), which is the shape #5374 introduced precisely so + * the two could not drift. That makes it the face most likely to be believed + * without being checked, and it was wrong here for exactly as long as the query + * path was. + */ +describe('[#6682] the analytics face answers the same text rules', () => { + const cube: Cube = { + name: 'texts', + title: 'Texts', + sql: TABLE, + measures: { + count: { name: 'count', label: 'Count', type: 'count', sql: 'id' }, + }, + dimensions: { + id: { name: 'id', label: 'Id', type: 'string', sql: 'id' }, + name: { name: 'name', label: 'Name', type: 'string', sql: 'name' }, + }, + } as unknown as Cube; + + let service: MemoryAnalyticsService; + beforeEach(async () => { + service = new MemoryAnalyticsService({ driver: await seed(), cubes: [cube] }); + }); + + /** + * Ids this face returns for one case's filter, ascending. + * + * The filter goes in as `where` — a `FilterCondition`, the case-set's own + * shape and the only one this face accepts since #5375 (the API layer rejects + * a `{member, operator, values}` array on the wire). So these rows drive the + * SHARED cases rather than a cube-dialect restatement of them. + */ + const analyticsIds = async (filter: unknown): Promise => { + const result = await service.query({ + cube: 'texts', + measures: ['texts.count'], + dimensions: ['texts.id'], + where: filter, + } as any); + return result.rows.map((r: any) => String(r['texts.id'])).sort(byId); + }; + + /** + * The subset this face can express: `MONGO_TO_CUBE_OPERATOR` carries + * `$contains` / `$notContains` / `$icontains` and has no `$startsWith` / + * `$endsWith` row, so those two are refused here rather than answered — a + * LOUD `uncompilableFieldOperatorError`, not a silent drop, and out of this + * card's scope. Selected by operator off the shared table rather than by + * name, so a new case joins this face automatically. + */ + const EXPRESSIBLE = ['$contains', '$notContains', '$icontains']; + const analyticsCases = rowCases.filter((c) => + Object.values(c.filter as Record>) + .every((ops) => Object.keys(ops).every((op) => EXPRESSIBLE.includes(op))), + ); + + it('covers the whole expressible subset — eleven cases, not an accidental one', () => { + expect(analyticsCases.length).toBe(11); + }); + + for (const c of analyticsCases) { + it(c.name, async () => { + expect(await analyticsIds(c.filter), c.note ?? c.name).toEqual([...c.expected]); + }); + } + + /** + * The fix must not be applied one level too deep. `filterSubstringPattern` is + * shared with the `$contains` family only — `$icontains` builds its pattern + * from the spec's `asciiCaseInsensitiveRegexSource` instead — so taking the + * fold out of that helper must leave the ASCII boundary exactly where #6520 + * put it. Stated separately from the loop above because it is the regression + * this diff could plausibly cause, not a rule this diff establishes. + */ + it('leaves $icontains folding ASCII and NOTHING else', async () => { + expect(await analyticsIds({ name: { $icontains: 'acme' } })).toEqual(['1', '2']); + expect(await analyticsIds({ name: { $icontains: 'café' } })).toEqual(['4']); + expect(await analyticsIds({ name: { $icontains: 'CAFÉ' } })).toEqual(['3']); + }); +}); diff --git a/packages/spec/src/data/aggregation-conformance.ts b/packages/spec/src/data/aggregation-conformance.ts index 8fe6dad936..1af58a1989 100644 --- a/packages/spec/src/data/aggregation-conformance.ts +++ b/packages/spec/src/data/aggregation-conformance.ts @@ -67,8 +67,9 @@ * a distinct value would answer `3` where {@link AGGREGATION_CASES} says `2`, * and that is not a theoretical failure mode: it is what `driver-mongodb`'s * `$addToSet` → `$size` lowering did until #6850/#6814 enrolled it — measured by - * running this table against the emitted pipeline, not predicted (see the DEBT - * list below, where `driver-memory` still carries the open half). + * running this table against the emitted pipeline, not predicted — and what + * `driver-memory`'s analytics face did too, from the same `$addToSet`, until + * #6814 (see the DEBT list below, now empty). * * The `count` cases sit beside them on purpose. `count(stage)` is `4` while * `count_distinct(stage)` is `2` and `count(*)` is `6`: three different numbers @@ -95,6 +96,12 @@ * answer "does MongoDB agree?", which is the question a real-mongod half would * own. Recorded here rather than left to be discovered, because a green cell * reads as more than that. + * - **`driver-memory`** — `memory-aggregation-conformance.test.ts` [#6814]. The + * one enrolled face that needs no engine AND no model: this driver runs in + * process, so the suite is a real execution of the table through BOTH doors of + * its data face (`find()` and `aggregate(AST)`, the one objectql's engine + * uses) plus its analytics face. It is therefore the counter-example to the + * caveat on `driver-mongodb` above — same package family, opposite instrument. * - **`objectql`'s in-memory fallback** — `in-memory-aggregation-conformance.test.ts` * in `packages/objectql`. [#6401] Not a SQL face and not a driver, which is * why #6409 left it out; enrolled here because it is the face that has always @@ -118,19 +125,20 @@ * * | Backend | Verdict | Evidence | * |---|---|---| - * | `driver-memory` (data face) | **RED** — answers `null` | `MemoryDriver.computeAggregate` has no `count_distinct` arm; the `switch` falls to `default: return null`, so the aggregation resolves with no value and no error. | - * | `driver-memory` (analytics face) | **agrees** | `memory-analytics.ts` collects `$addToSet` and sizes it — the same NULL question as MongoDB below; not executed against this table. | + * | ~~`driver-memory` (data face)~~ | **CLEARED** [#6814] | Was RED and answering `null`: `MemoryDriver.computeAggregate` had no `count_distinct` arm, so the `switch` fell to `default: return null` and the aggregation resolved with no value and no error. It now has the arm, null-excluded, beside its neighbours. | + * | ~~`driver-memory` (analytics face)~~ | **CLEARED** [#6814] | The row above it read this face as "agrees" from the source — `memory-analytics.ts` "collects `$addToSet` and sizes it". Executing the table showed it collected and never sized: the measure answered the raw ARRAY (`['won','lost',null]`) under a field its own `fields` metadata types as `number`. Wrong on the NULL question the row anticipated *and* on the shape it did not. The reason the row was wrong is the reason the DEBT list says "not executed against this table" — and why enrolling is the only thing that clears a cell. | * | ~~`driver-mongodb`~~ | **CLEARED** [#6850/#6814] | Was RED and under-stated: the `count_distinct` null (3 for the standard's 2), the `"[object Object]"` `$group._id` below, AND a third divergence neither row named — `count(col)` ignored `field` and answered the ROW count (6 for the standard's 4). All three fixed and enrolled; see the list above. | - * | `driver-memory` — the #6401 alias cases | **agrees** | `MemoryDriver.performAggregation`'s `normalizeGroupBy` (`memory-driver.ts:1066-1068`) already returns `{ field, alias: node.alias ?? node.field }` and projects the group value under `alias`. It reached the enforce answer independently, so the alias leg needed NO mechanical alignment here — measured, not assumed. | + * | ~~`driver-memory` — the #6401 alias cases~~ | **CLEARED** [#6814] — and this one WAS right | `MemoryDriver.performAggregation`'s `normalizeGroupBy` (`memory-driver.ts:1066-1068`) already returns `{ field, alias: node.alias ?? node.field }` and projects the group value under `alias`. It reached the enforce answer independently, so the alias leg needed NO mechanical alignment here — measured, not assumed. | * | ~~`driver-mongodb` — the #6401 alias cases~~ | **CLEARED** [#6850] | Was RED and wider than the alias: `buildAggregationPipeline` typed `groupBy` as `string[]` and did `groupId[field] = '$' + field`, so a STRUCTURED node — aliased or not — stringified into a `"[object Object]"` `$group._id` keyed on a field path that matches nothing. The alias was unreachable rather than ignored. It now reads the union, keys `_id` on `alias ?? field`, and refuses a `dateGranularity` node with NOT_IMPLEMENTED/501 rather than dropping a declared key; `mongodb-driver.ts` spells the declared type instead of `(query as any).groupBy`, so the next drift is a `tsc` error. | * - * `driver-memory` is inside the **#5499 investment freeze**, which is why its - * row is a DEBT row and not a fix: #6409's ruling put it explicitly out of scope - * and left its partial implementation untouched. Enrolling it means lifting the - * freeze for it first — the row is here so that decision is made against a - * measured verdict instead of an assumption that it already agrees. The - * maintainer lifted the freeze for `driver-mongodb` alone on 2026-08-11, which - * is why the two rows above are struck through and this one is not. + * Every row is struck through: the maintainer lifted the **#5499 investment + * freeze** for `driver-mongodb` on 2026-08-11 and for `driver-memory` later the + * same day, and both cells were enrolled rather than argued. What the list + * bought is worth keeping after it emptied — each row recorded a MEASURED + * verdict, and in both packages the verdict read from the source turned out to + * UNDER-state the divergence (mongodb's `count(col)`, memory's unsized + * `$addToSet`). A row read off the code is a hypothesis; the suite is the + * measurement. Add the next row the same way, and expect it to be optimistic. * * What the strike-throughs are worth keeping for: every one of those verdicts * was reached by READING, and when the suite finally executed the case-set it diff --git a/packages/spec/src/data/filter-text-conformance.ts b/packages/spec/src/data/filter-text-conformance.ts index 4d8a855f0d..62956cfbad 100644 --- a/packages/spec/src/data/filter-text-conformance.ts +++ b/packages/spec/src/data/filter-text-conformance.ts @@ -55,17 +55,20 @@ * - `driver-mongodb` answers the `$contains` family case-exactly since #6682, * and its suite (`mongodb-filter-text-conformance.test.ts`) imports this * whole table — every row, rejections included — so its DEBT row is gone. - * - `driver-memory` still folds the `$contains` family over the whole Unicode - * range on its query and analytics faces while its reference matcher does - * not (#6682), which is why it still carries a measured DEBT row in that - * gate's ledger rather than importing this table: coverage is judged by - * IMPORT, and a cell that answers one requirement and not the other must not - * claim the whole set. The ledger is RECONCILED against the imports on every - * run, so read the open set THERE rather than trusting a count written in - * prose here. + * - `driver-memory` answers it too since #6682's second half: the `i` flag came + * off all nine sites on its query path and off `filterSubstringPattern`, the + * shared rule its analytics face borrows, so the two folding faces moved onto + * the reference matcher's already-case-exact answer rather than the other way + * round. `memory-filter-text-conformance.test.ts` imports this whole table and + * runs it on every face, and that DEBT row is gone with it. * - * Rule 2 above still governs the open cells: the rows join a driver's suite - * in the PR that closes its gap, not before. + * So the ledger is EMPTY: all five drivers import this table. The ledger is + * RECONCILED against the imports on every run, so read the open set THERE rather + * than trusting a count written in prose here — this paragraph is the thing that + * goes stale, which is why the gate and not the prose is the authority. + * + * Rule 2 above still governs any future gap: the rows join a driver's suite + * in the PR that closes it, not before. * * The `$regex` rejection cases carried a further ordering constraint when * this table landed: `plugin-auth`'s ObjectQL adapter still emitted @@ -256,15 +259,16 @@ export const FILTER_TEXT_CASES: readonly FilterTextCase[] = [ // family case-exact (GLOB on the SQLite dialects), so those three drivers // answer these rows today, and #6682 took mongo\'s hardcoded `$options: 'i'` // off all four arms, so `translateFilter` now lowers `$contains` to a bare - // `$regex` and that driver answers them too. driver-memory\'s query and - // analytics faces still fold Unicode — that remainder is #6682\'s open half, - // not #5702\'s. (`formula` and driver-memory\'s reference matcher measured - // case-exact both then and now.) + // `$regex` and that driver answers them too. #6682\'s second half then took + // the same flag off driver-memory\'s query path and off the rule its analytics + // face borrows, which was the last folding face on the platform. (`formula` + // and driver-memory\'s reference matcher measured case-exact both then and + // now — they are what the other faces were moved onto.) { name: '$contains is case-SENSITIVE — a lower-case comparand misses the upper-case row', filter: { name: { $contains: 'acme' } }, expected: ['2'], - note: 'Row 1 (ACME Corp) must NOT match. SQLite\'s LIKE folds ASCII — the defect #6518 replaced with GLOB on the SQLite dialects; a backend returning both here has regressed to it (driver-memory still folds — #6682).', + note: 'Row 1 (ACME Corp) must NOT match. SQLite\'s LIKE folds ASCII — the defect #6518 replaced with GLOB on the SQLite dialects; a JS backend\'s equivalent is a RegExp carrying the `i` flag, which #6682 took off the last two. A backend returning both here has regressed to one of them.', }, { name: '$contains is case-SENSITIVE — an upper-case comparand misses the lower-case row', diff --git a/packages/spec/src/data/filter.zod.ts b/packages/spec/src/data/filter.zod.ts index 1e99376886..3c9da6b723 100644 --- a/packages/spec/src/data/filter.zod.ts +++ b/packages/spec/src/data/filter.zod.ts @@ -353,7 +353,7 @@ export const RangeOperatorSchema = lazySchema(() => z.object({ * | surface | `$contains` case behaviour | mechanism | * |---|---|---| * | `formula` `matchesFilterCondition` | SENSITIVE | `actual.includes(v)` | - * | `driver-memory` — query path and analytics face | INSENSITIVE, full Unicode | `new RegExp(escapeRegex(v), 'i')` | + * | `driver-memory` — query path and analytics face | INSENSITIVE, full Unicode | `new RegExp(escapeRegex(v), 'i')` — the last one standing, until #6682 | * | `driver-memory` — reference matcher (`memory-matcher`) | SENSITIVE | `value.includes(target)` | * | `driver-mongodb` | INSENSITIVE, full Unicode | hardcoded `$options: 'i'` | * | `driver-sql` family | the DIALECT's | `LIKE '%v%'` — ASCII-insensitive on SQLite (so also turso and sqlite-wasm), sensitive on Postgres, collation-dependent on MySQL | @@ -413,20 +413,21 @@ export const RangeOperatorSchema = lazySchema(() => z.object({ * `matches-filter.ts`; what #6520 changed is that no operator this array * DECLARES is answered that way any more. * - * The `$contains`-family alignment the ruling above requires is likewise part - * done rather than pending: #6518 made that family case-EXACT across the SQL - * dialects and #6682 did the same on `driver-mongodb` (the hardcoded - * `$options: 'i'` is off all four arms, and that driver's every face — query, - * count, write and the aggregation `$match` — routes through the one - * `translateFilter`, so there is no second answer). `driver-memory`'s query and - * analytics faces still fold the whole Unicode range while its reference - * matcher does not — the one row #6682 still tracks, and the one package that - * answers this operator two ways. + * The `$contains`-family alignment the ruling above requires is now DONE on + * every backend: #6518 made that family case-EXACT across the SQL dialects, + * #6682 did the same on `driver-mongodb` (the hardcoded `$options: 'i'` is off + * all four arms, and that driver's every face — query, count, write and the + * aggregation `$match` — routes through the one `translateFilter`, so there is + * no second answer) and then on `driver-memory`, whose query path and analytics + * face lost the `i` flag their shared rule carried (`filterSubstringPattern`). + * That last one closed the package that answered this operator two ways: its + * reference matcher had been case-exact all along, so the fix moved the two + * that were wrong onto the one that was right. * * `FILTER_TEXT_CASES` (`filter-text-conformance.ts`) is the standard that - * measures all of the above, and the driver-conformance ledger still carries a - * DEBT row for `driver-memory` — on requirement 2 alone now, since #6520 closed - * requirement 1 — so what is open stays counted rather than assumed. + * measures all of the above, and every driver now IMPORTS it — the + * driver-conformance ledger is empty. Read the open set from a run of that gate + * rather than from this paragraph. * * @see FILTER_TEXT_CASES — the conformance standard for every operator here. * @see RETIRED_FILTER_OPERATORS — why `$regex` is not in this list. diff --git a/scripts/check-driver-conformance.mjs b/scripts/check-driver-conformance.mjs index 9b8d624936..35a447c4f0 100644 --- a/scripts/check-driver-conformance.mjs +++ b/scripts/check-driver-conformance.mjs @@ -346,10 +346,16 @@ const CASE_SETS = [ // `$options: 'i'` on any of the four arms, and because every face of that // driver — `find`/`count`/`update`/`delete` and the aggregation `$match` // — routes through the one `translateFilter`, there is no second answer to -// align. **STILL OPEN on driver-memory**, which folds the full Unicode -// range on its live query path while its REFERENCE matcher answers the -// same operator case-sensitively, so that package disagrees with itself. -// It is the remaining #5499 frozen half; tracked as #6682. +// align. **DONE on driver-memory too** (#6682, the last cell): the `i` +// flag came off all NINE sites — the AST spelling's four arms +// (`convertConditionToMongo`), the `$`-spelling's four +// (`normalizeFieldOperators`) and `filterSubstringPattern`, the #5374 +// shared rule the ANALYTICS face borrows rather than re-derives, so that +// face moved with the query path in the direction #5374 was built for. The +// reference matcher (`String.prototype.includes`) was case-exact all along +// and is untouched — it was the face that had been RIGHT, which is why the +// package disagreed with itself until this landed. Requirement 2 is now +// answered on all five drivers. // // Two faces #6518 measured and did NOT have to change, recorded because // "not mentioned" reads as "not checked": `formula`'s `matchesFilter` and @@ -400,67 +406,34 @@ const CASE_SETS = [ // the emitted pipeline through an in-process evaluator; the real-mongod half // is still absent and is recorded as such on #6814 rather than implied here. // -// `driver-memory`'s row stands. The 2026-08-11 ruling lifted the freeze for -// `driver-mongodb` alone, so that cell — a missing `count_distinct` arm and the -// two-face divergence beside it — stays open by the same decision as before, -// and #6814 stays OPEN for it. +// [#6814] `driver-memory`'s cell is now cleared too — the maintainer lifted the +// rest of the #5499 freeze later the same day, and +// `memory-aggregation-conformance.test.ts` replaced the row. This driver runs +// in process, so that suite is a REAL execution of the case-set through BOTH +// doors of the data face (`find()` and `aggregate(AST)`, the one objectql's +// engine uses) — no server-free half to model. +// +// What the row predicted held, and it under-read the ANALYTICS face. The row +// said that face "DOES implement `count_distinct`"; executing it showed the +// implementation stopped half way. `buildAggregator` emitted `{ $addToSet }` +// with the comment "Will need post-processing for count" and no post-processing +// existed, so the measure answered the raw ARRAY of values — `['won','lost', +// null]` — under a field `measureTypeToFieldType` describes as `number`. So the +// package answered one declared function THREE ways: `null` on the data face, +// an array on the analytics face, and the standard's number nowhere. Both are +// fixed here; the array is sized excluding null, which is the same +// null-exclusion the driver-mongodb half made on `$addToSet` (#7550). +// +// The ledger is now EMPTY, which is the state its header calls the intended +// steady one. Read the open set from a run, not from this prose. + +// The intended steady state, reached on 2026-08-11: every (driver x case-set) +// cell is covered by an imported case-set, and nothing is deferred. Keep it +// that way by writing the suite, not by adding a row — a DEBT entry is a +// MEASURED, tracked exception the maintainer has agreed to, never the cheaper +// half of "enroll the driver". +const LEDGER = []; -const LEDGER = [ - { - driver: 'driver-memory', - marker: 'FILTER_TEXT_CASES', - kind: 'DEBT', - why: - 'Re-measured after #6518, which cleared requirement 2 on the SQL family and NOT here — this package is ' - + 'in the #5499 frozen family, so the freeze rather than the difficulty is why the cell is open. ' - + 'Requirement 3 is DONE (#5702): `$regex`/`$options` are no longer in `SUPPORTED_FIELD_OPERATORS`, the ' - + 'matcher\'s `$regex` arm (the only live regex evaluator in the repo, and the one that answered an ' - + 'ILLEGAL pattern with `false`) is deleted, and both faces refuse them with the spec prescription ' - + 'naming `$icontains`. Requirement 2 is still the one row where "which face" changes the answer — do ' - + 'NOT take a single reading here. The QUERY path (`find()` -> `normalizeFieldOperators`, and the ' - + 'analytics face via `filterSubstringPattern`) lowers `$contains` to `new RegExp(escapeRegex(v), "i")`: ' - + 'literal comparand (requirement 2\'s escaping half holds) but case-INSENSITIVE over the whole Unicode ' - + 'range, which fails requirement 2 and overshoots requirement 1\'s ASCII boundary. The reference ' - + 'matcher (`memory-matcher.ts` `match()`, the record-at-a-time evaluator `filter-logic-conformance.ts` ' - + 'counts as a backend) uses String.prototype.includes and is case-SENSITIVE — i.e. this package ' - + 'answers one `$contains` two ways today, the divergence class #5374 fixed between the other two ' - + 'faces. Whichever suite clears this cell has to pick one and align both: tracked as #6682, which is ' - + 'the successor #6518 left behind for exactly this pair of packages. Requirement 1 is DONE here ' - + 'since #6520: all THREE of this package\'s faces answer `$icontains` with the spec\'s shared ' - + 'ASCII-only fold — the query path and the analytics face bind a pattern from ' - + '`asciiCaseInsensitiveRegexSource` (no `i` flag, which folds Unicode), the reference matcher calls ' - + '`asciiCaseInsensitiveContains`, and the NON-EMPTY-string comparand rule is driver-sql\'s word for ' - + 'word. So this row now carries ONE open requirement, not two. It still cannot go: coverage is ' - + 'judged by importing the WHOLE case-set, so a cell answering one requirement and not the other must ' - + 'not import it — #6520\'s suites drive `FILTER_TEXT_ROWS` and spell their own `$icontains` cases ' - + 'rather than naming the marker, which would flip this cell to covered and fail RECONCILED while ' - + 'requirement 2 is open.', - issue: 'https://github.com/objectstack-ai/objectstack/issues/6682', - }, - { - driver: 'driver-memory', - marker: 'AGGREGATION_CASES', - kind: 'DEBT', - why: - 'Measured on this branch by reading `MemoryDriver.computeAggregate` (`memory-driver.ts`): it has arms ' - + 'for count/sum/avg/min/max and then `default: return null`. There is NO `count_distinct` arm, so an ' - + 'aggregation the Query Protocol declares — and that every SQL face now lowers (#6409) — resolves with ' - + '`{ n: null }`: no error, no log, no refusal. The case-set says 2 over `AGGREGATION_ROWS`. That is a ' - + 'wrong ANSWER rather than a wrong number, and it is the `default:`-arm shape the ' - + '`aggregation-lockstep` guard exists to stop one layer up, reached here through a different door. ' - + 'The package is partial in the way #6409\'s ruling described: its ANALYTICS face ' - + '(`memory-analytics.ts`) DOES implement `count_distinct`, so this package answers one declared ' - + 'function two ways depending on which face you enter — the divergence class #5374 fixed for ' - + '`$contains` in this same package. #5499 freezes it, so the cell is open by decision, not by ' - + 'difficulty: the fix is one arm beside its neighbours. Tracked as #6814. ' - + '[#6401] Re-measured when the case-set gained its `groupByAlias` axis: on THAT axis this driver ' - + 'AGREES. `performAggregation`\'s `normalizeGroupBy` (`memory-driver.ts:1066-1068`) already returns ' - + '`{ field, alias: node.alias ?? node.field }` and projects the group value under `alias` — the answer ' - + '#6401 converged the three SQL faces onto. It had reached it independently, so the enforce leg needed ' - + 'NO mechanical alignment here. The cell stays open on `count_distinct` alone.', - issue: 'https://github.com/objectstack-ai/objectstack/issues/6814', - }, -]; // ── Discovery ─────────────────────────────────────────────────────────────── From 2a8c150b3f0b506ff3b7ddbbe1d60cddb2ee2c42 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 11 Aug 2026 14:20:51 +0000 Subject: [PATCH 2/2] test(driver-memory): type the aggregation conformance query instead of erasing it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The query-options-erasure ratchet (#4918) went red: the new aggregation suite added 3 counted sites (242 -> 245) by casting `queryFor(c) as any` at each of its three call sites — the two doors of the data face plus the never-answers- null property row. Typed rather than exempted. `queryFor` now declares `DriverQuery` as its return type (import-reachable from `@objectstack/spec/contracts`, the same type `MemoryDriver.find` takes), so all three arguments are checked by `tsc` and no call site needs a cast. The `as unknown as` spelling would have been wrong here: every case in this file is deliberately ON contract — the whole point is that the standard's own vocabulary reaches the driver — so there is no bypassed contract to name. The baseline is NOT raised: the count returns to the 242 ceiling. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01NuzQE844ut8m8vypXcdBYD --- .../memory-aggregation-conformance.test.ts | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/packages/drivers/driver-memory/src/memory-aggregation-conformance.test.ts b/packages/drivers/driver-memory/src/memory-aggregation-conformance.test.ts index fa7c05eea6..e3d2e58acb 100644 --- a/packages/drivers/driver-memory/src/memory-aggregation-conformance.test.ts +++ b/packages/drivers/driver-memory/src/memory-aggregation-conformance.test.ts @@ -69,13 +69,22 @@ import { describe, it, expect, beforeEach } from 'vitest'; import { AGGREGATION_CASES, AGGREGATION_ROWS } from '@objectstack/spec/data'; import type { AggregationCase, Cube } from '@objectstack/spec/data'; +import type { DriverQuery } from '@objectstack/spec/contracts'; import { InMemoryDriver } from './memory-driver.js'; import { MemoryAnalyticsService } from './memory-analytics.js'; const TABLE = 'conformance_agg'; -/** The case as the `DriverQuery` shape both doors consume. */ -const queryFor = (c: AggregationCase) => ({ +/** + * The case as the `DriverQuery` shape both doors consume. + * + * [#4918] The return type is DECLARED rather than left to inference and erased + * at each call site. Every case here is deliberately ON contract — the whole + * point of the file is that the standard's own vocabulary reaches the driver — + * so there is nothing for an `as any` to bypass, and typing it puts the two + * doors' argument under `tsc` instead of exempting it. + */ +const queryFor = (c: AggregationCase): DriverQuery => ({ aggregations: [{ function: c.function, ...(c.field ? { field: c.field } : {}), alias: 'n' }], // [#6401] A case carrying `groupByAlias` is sent as the STRUCTURED node, so // the face receives the union member that declares `alias`. Without this the @@ -140,7 +149,7 @@ describe('[#6814] InMemoryDriver — aggregate vocabulary conformance', () => { for (const c of AGGREGATION_CASES) { it(`find(): ${c.name}`, async () => { - const rows = await driver.find(TABLE, queryFor(c) as any); + const rows = await driver.find(TABLE, queryFor(c)); expect(actualFor(c, rows as any[]), c.note ?? c.name).toEqual(expectedFor(c)); }); @@ -151,7 +160,7 @@ describe('[#6814] InMemoryDriver — aggregate vocabulary conformance', () => { * stand for the other. */ it(`aggregate(AST): ${c.name}`, async () => { - const rows = await driver.aggregate(TABLE, queryFor(c) as any); + const rows = await driver.aggregate(TABLE, queryFor(c)); expect(actualFor(c, rows as any[]), c.note ?? c.name).toEqual(expectedFor(c)); }); } @@ -165,7 +174,7 @@ describe('[#6814] InMemoryDriver — aggregate vocabulary conformance', () => { */ it('never answers null for a declared aggregate function', async () => { for (const c of AGGREGATION_CASES) { - const rows = await driver.find(TABLE, queryFor(c) as any); + const rows = await driver.find(TABLE, queryFor(c)); for (const row of rows as any[]) { expect(row.n, `${c.name} — a declared function resolving null is the #6814 defect`).not.toBeNull(); expect(typeof row.n, c.name).toBe('number');