From 5798f3889602502dd2c6fdddb806f7c849e0e8c9 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 27 Aug 2026 23:06:08 +0000 Subject: [PATCH 1/4] fix(driver-mongodb): a boolean aggregand answers the ruled values MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two independent halves of one cell, in the same package: 1. `mongodb-aggregation.ts` lowered `sum` / `avg` as bare `$sum` / `$avg` over the field path. Those are arithmetic accumulators and ignore every non-numeric value, so a boolean column summed to `0` and averaged to `null`. They now wrap the aggregand in the #11065 boolean-only coercion, answering 3 / 0.5 on the 3-true/3-false fixture. 2. `mongodb-pipeline-evaluator.testkit.ts` applied its "arithmetic accumulators ignore non-numeric values" filter one arm too far: `$min` / `$max` consumed it too and answered `null` over a boolean column. They are order statistics over BSON canonical order, so they now ignore only null and missing, compare by type-then-value, and return a member of the input — `false` / `true`, the #11249 ruling. `bsonRank` is the single place that order is written down and refuses any type it does not model, so the arms raise rather than silently answering `null`, which is what the file's head note has always promised. `$type` is modelled for the same reason: the coercion above emits it. The coercion in (1) is deliberately NOT applied to `$min` / `$max` — that would answer 0 / 1 where #11249 ruled false / true. Part of #11151 --- .../driver-mongodb/src/mongodb-aggregation.ts | 61 ++++++++- .../src/mongodb-pipeline-evaluator.testkit.ts | 117 +++++++++++++++--- 2 files changed, 159 insertions(+), 19 deletions(-) diff --git a/packages/drivers/driver-mongodb/src/mongodb-aggregation.ts b/packages/drivers/driver-mongodb/src/mongodb-aggregation.ts index 876623b8f5..de6a296ef5 100644 --- a/packages/drivers/driver-mongodb/src/mongodb-aggregation.ts +++ b/packages/drivers/driver-mongodb/src/mongodb-aggregation.ts @@ -467,6 +467,60 @@ export function buildAggregationPipeline(opts: { return pipeline; } +/** + * [#11151] `path`, with a BOOLEAN rendered as the number it is worth — the + * aggregand expression `$sum` and `$avg` consume on this face. + * + * ## What it is for + * + * MongoDB's `$sum` and `$avg` are ARITHMETIC accumulators and ignore every + * non-numeric value, a boolean included. So a whole boolean column summed to + * `$sum`'s identity `0` and averaged to `null` here, while `SUM(col)` / + * `AVG(col)` answer `3` / `0.5` over the same 3-true/3-false rows on every SQL + * dialect (#11635), `driver-memory` answers those numbers on both of its faces + * (#11065), and objectql's in-memory fallback answers them too because its + * `toNumber` is `Number(v)` and `Number(true) === 1`. A rate measure over a + * flag column — an SLA-violation rate, a win rate — is the ordinary shape of + * that query, and the two answers are not two spellings of one: a dashboard + * tile bound to the measure renders a percentage under SQL and a blank here, + * indistinguishable from "no matching rows". `sum`'s `0` is the worse half, + * being a plausible number rather than a visible hole. + * + * The expression is the one #11065 landed on `driver-memory`'s analytics face + * (`memory-analytics.ts`, `numericAggregandExpr`), reproduced rather than + * imported: this driver shares no line of code with that one, and the shared + * contract between them is the VALUES in `@objectstack/spec/data`, not a + * helper. + * + * ## Why an EXPRESSION and not post-processing + * + * {@link postProcessAggregation} runs after the pipeline's own `$sort` and + * `$limit` stages, so a measure left unresolved until then would be sorted as + * whatever it was — and `order` over a `sum` or `avg` measure is an ordinary + * analytics query. Keeping the rule inside the `$group` expression leaves every + * later stage looking at the number it expects. + * + * ## ⛔ Why it is NOT applied to `min` / `max` + * + * `$min` / `$max` are ORDER STATISTICS over BSON canonical comparison order, + * not arithmetic accumulators: they rank booleans and return a MEMBER of the + * input domain. #11249 ruled (maintainer 2026-08-23) that a boolean aggregand + * answers `false` / `true` there — the JSON boolean, not `0` / `1` — so + * wrapping those two arms in this coercion would break the ruled contract in + * the opposite direction from the defect it fixes. + * + * ## The narrowness is deliberate + * + * Only `bool` is rewritten. Null, missing and a non-numeric string reach the + * accumulator exactly as before and are ignored by it exactly as before. + * Coercing wider would mean adopting `toNumber`'s other half, which maps a + * non-numeric string to `0` and so averages garbage as zero rather than + * excluding it — a separate question from this one. + */ +function numericAggregandExpr(path: string): Document { + return { $cond: [{ $eq: [{ $type: path }, 'bool'] }, { $cond: [path, 1, 0] }, path] }; +} + /** * Build a single MongoDB accumulator expression from an aggregation descriptor. */ @@ -490,11 +544,14 @@ function buildAccumulator(agg: AggregationInput): Document { ? { $sum: 1 } : { $sum: { $cond: [{ $eq: [{ $ifNull: [fieldRef, null] }, null] }, 0, 1] } }; + // [#11151] `sum` / `avg` coerce a BOOLEAN aggregand; see + // {@link numericAggregandExpr} for why, and for why `min` / `max` below + // deliberately do NOT. case 'sum': - return { $sum: fieldRef ?? 0 }; + return { $sum: fieldRef === null ? 0 : numericAggregandExpr(fieldRef) }; case 'avg': - return { $avg: fieldRef ?? 0 }; + return { $avg: fieldRef === null ? 0 : numericAggregandExpr(fieldRef) }; case 'min': return { $min: fieldRef ?? 0 }; diff --git a/packages/drivers/driver-mongodb/src/mongodb-pipeline-evaluator.testkit.ts b/packages/drivers/driver-mongodb/src/mongodb-pipeline-evaluator.testkit.ts index d2363bed55..c5c44ea426 100644 --- a/packages/drivers/driver-mongodb/src/mongodb-pipeline-evaluator.testkit.ts +++ b/packages/drivers/driver-mongodb/src/mongodb-pipeline-evaluator.testkit.ts @@ -30,7 +30,8 @@ * (proxy 403 on the download — #5517), and a suite nobody has executed against * the real thing is a claim, not a check. The date operators added for #7580 * (`$convert` → date, `$dateToString`, `$concat`, `$switch`, `$lte`) carry that - * bound exactly as the #6850/#6814 ones do. + * bound exactly as the #6850/#6814 ones do, and so do `$type` and the BSON-order + * `$min` / `$max` added for #11151. * * The single most important discipline that makes the bound survivable: this * file models the DOCUMENTED semantics, never the behaviour the lowering @@ -180,25 +181,72 @@ function formatDate(d: Date, format: string): string { } /** - * BSON canonical sort order, for the two types `$lte` meets here. Null sorts - * BELOW every string — which is why the `quarter` lowering can let its `$switch` - * run on a null instant without a guard: it answers a digit, and the surrounding - * `$concat` has already decided the whole label is null. + * BSON canonical TYPE order, for the types this evaluator models. The manual's + * order runs MinKey, Null, Numbers, String, Object, Array, BinData, ObjectId, + * **Boolean**, Date, Timestamp, Regex, MaxKey — so a boolean ranks ABOVE every + * number and every string. That ranking is the whole reason `$min` / `$max` can + * answer over a boolean column where `$sum` / `$avg` cannot: order statistics + * compare by type-then-value and return a MEMBER of the input, while the + * arithmetic accumulators ignore what they cannot add. + * + * ⛔ Every type not listed here is a thrown {@link UnsupportedShape}, never a + * silent rank. This function is the one place the order is written down, so a + * type it does not model refuses in BOTH of its callers rather than being ranked + * one way by `$lte` and dropped by `$min`. + * + * [#11151] Boolean was added here. The three ranks that existed before keep + * their relative order exactly, so `$lte` — whose only operands are the null + * and the string a date-bucket label reaches it as — is unchanged. + */ +function bsonRank(v: unknown): number { + if (v === MISSING || v === null) return 0; + if (typeof v === 'number') return 1; + if (typeof v === 'string') return 2; + if (typeof v === 'boolean') return 3; + throw new UnsupportedShape(`BSON canonical order over an unmodelled type: ${typeof v}`); +} + +/** + * `left <= right` under BSON canonical order — type rank first, value second. + * Null sorts BELOW every string, which is why the `quarter` lowering can let its + * `$switch` run on a null instant without a guard: it answers a digit, and the + * surrounding `$concat` has already decided the whole label is null. */ function bsonLte(left: unknown, right: unknown): boolean { - const rank = (v: unknown): number => { - if (v === MISSING || v === null) return 0; - if (typeof v === 'number') return 1; - if (typeof v === 'string') return 2; - throw new UnsupportedShape(`$lte over an unmodelled BSON type: ${typeof v}`); - }; - const lr = rank(left); - const rr = rank(right); + const lr = bsonRank(left); + const rr = bsonRank(right); if (lr !== rr) return lr < rr; if (lr === 0) return true; // null <= null + // `false` sorts below `true`; TypeScript refuses a relational operator on two + // booleans, so the comparison is spelled through their numeric images. + if (lr === 3) return Number(left) <= Number(right); return (left as string | number) <= (right as string | number); } +/** + * The manual's `$type`: the BSON type NAME of a value, and the string + * `'missing'` for a field path that resolved to nothing — the one type name + * that is not a type. Modelled because the `sum` / `avg` boolean coercion + * (#11151) emits `$type`, and an evaluator that guessed here would be blessing + * a lowering nobody checked. + * + * A JS number is reported as `'double'`: every unboxed number crosses the wire + * as a BSON double, and this evaluator has no boxed integers to distinguish, so + * `'double'` is the honest answer rather than a size-dependent guess between + * `'int'`, `'long'` and `'double'`. + */ +function bsonTypeName(v: unknown): string { + if (v === MISSING) return 'missing'; + if (v === null) return 'null'; + if (typeof v === 'boolean') return 'bool'; + if (typeof v === 'number') return 'double'; + if (typeof v === 'string') return 'string'; + if (v instanceof Date) return 'date'; + if (Array.isArray(v)) return 'array'; + if (typeof v === 'object') return 'object'; + throw new UnsupportedShape(`$type over an unmodelled BSON type: ${typeof v}`); +} + /** MongoDB's expression truthiness: `false`, `null`, `0` and missing are false. */ function truthy(v: unknown): boolean { return !(v === false || v === null || v === MISSING || v === 0 || v === undefined); @@ -247,6 +295,17 @@ export function evalExpr(doc: Doc, expr: unknown): unknown { if (!Array.isArray(arg) || arg.length !== 2) throw new UnsupportedShape('$lte takes two operands'); return bsonLte(evalExpr(doc, arg[0]), evalExpr(doc, arg[1])); } + case '$type': { + // [#11151] `$type` takes exactly ONE operand, unwrapped or wrapped in a + // one-element array; the manual accepts both spellings and the + // boolean-aggregand lowering emits the unwrapped one. A longer array is + // an error on a real server, so it is refused here rather than reported + // as `'array'`. + if (Array.isArray(arg) && arg.length !== 1) { + throw new UnsupportedShape(`$type takes one operand, got ${arg.length}`); + } + return bsonTypeName(evalExpr(doc, Array.isArray(arg) ? arg[0] : arg)); + } case '$cond': { if (!Array.isArray(arg) || arg.length !== 3) throw new UnsupportedShape('$cond takes [if, then, else]'); return truthy(evalExpr(doc, arg[0])) ? evalExpr(doc, arg[1]) : evalExpr(doc, arg[2]); @@ -319,7 +378,21 @@ export function accumulate(rows: Doc[], acc: unknown): unknown { const [op] = keys; const arg = (acc as Doc)[op]; const values = rows.map((row) => evalExpr(row, arg)); - /** MongoDB's arithmetic accumulators ignore missing and non-numeric values. */ + /** + * MongoDB's ARITHMETIC accumulators ignore missing and non-numeric values. + * + * [#11151] ⛔ This filter belongs to `$sum` and `$avg` and to nothing else. It + * used to be computed once for the whole switch and consumed by `$min` and + * `$max` as well — one arm too far, and the comment above it was accurate the + * whole time. `$min` / `$max` are ORDER STATISTICS over BSON canonical order + * (see {@link bsonRank}), not arithmetic: they rank every type, so a boolean + * column has a real minimum and a real maximum. Filtering to numbers left + * them with nothing and they answered `null` — SILENTLY, which is the one + * thing this file's head note promises it never does. A wrong answer from a + * strict evaluator is worse than a refusal, because the red it produces reads + * as a defect in the lowering under test; that misreading really happened, and + * cost a card's dispatch a wrong diagnosis. + */ const numbers = values.filter((v): v is number => typeof v === 'number'); switch (op) { @@ -328,9 +401,19 @@ export function accumulate(rows: Doc[], acc: unknown): unknown { case '$avg': return numbers.length === 0 ? null : numbers.reduce((a, b) => a + b, 0) / numbers.length; case '$min': - return numbers.length === 0 ? null : Math.min(...numbers); - case '$max': - return numbers.length === 0 ? null : Math.max(...numbers); + case '$max': { + // The manual's rule for both: null and missing are IGNORED, whatever is + // left is compared by BSON canonical order, and a group in which every + // value is null or missing answers `null`. The result is a MEMBER of the + // input — a boolean in, a boolean out (the #11249 contract) — never a + // number derived from one. `bsonRank` refuses any type it does not model, + // so an unmodelled aggregand raises rather than collapsing to `null`. + const present = values.filter((v) => v !== MISSING && v !== null); + if (present.length === 0) return null; + return present.reduce((best, v) => + (op === '$min' ? bsonLte(v, best) : bsonLte(best, v)) ? v : best, + ); + } case '$addToSet': { // `$addToSet` skips a MISSING field and keeps an explicit `null` — the // whole of #6814 lives in that second half. From 72c6d953a10d9d2e92acb445ec6430fbb6b163b1 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 27 Aug 2026 23:08:28 +0000 Subject: [PATCH 2/4] test(driver-mongodb): pin all four ruled boolean-aggregand cells, plus controls Ungrouped and grouped, on the `FLAG_BY_ID` distribution already landed on `main` in driver-sql's #11635 suite (west [T,F,F,F], east [T,T]), so the two faces' grouped numbers are comparable. The load-bearing pin is the emitted-lowering block: it reads the stages rather than the values to assert the `sum`/`avg` coercion did NOT reach `$min`/`$max`. Once the evaluator ranks booleans both spellings produce an answer, so the values alone can no longer tell them apart. Controls kept beside them: `count` / `count_distinct` (which already agreed), all four functions over the numeric column, and sum/avg over a string column to pin that the coercion stays boolean-only. Part of #11151 --- ...mongodb-boolean-aggregand-ruled-answers.md | 48 ++++ ...db-11151-boolean-aggregand-answers.test.ts | 272 ++++++++++++++++++ .../src/mongodb-pipeline-evaluator.testkit.ts | 5 + 3 files changed, 325 insertions(+) create mode 100644 .changeset/mongodb-boolean-aggregand-ruled-answers.md create mode 100644 packages/drivers/driver-mongodb/src/mongodb-11151-boolean-aggregand-answers.test.ts diff --git a/.changeset/mongodb-boolean-aggregand-ruled-answers.md b/.changeset/mongodb-boolean-aggregand-ruled-answers.md new file mode 100644 index 0000000000..6512270601 --- /dev/null +++ b/.changeset/mongodb-boolean-aggregand-ruled-answers.md @@ -0,0 +1,48 @@ +--- +"@objectstack/driver-mongodb": patch +--- + +fix(driver-mongodb): a boolean aggregand answers the ruled values (#11151) + +`sum` and `avg` over a **boolean** column answered `0` and `null` on this +driver, where every SQL dialect (#11635), `driver-memory` (#11065) and +objectql's in-memory fallback already answered `3` and `0.5` over the same +3-true/3-false rows. The lowering passed the boolean straight to MongoDB's +`$sum` / `$avg`, which are arithmetic accumulators and ignore every non-numeric +value: with nothing numeric to fold, `$sum` returns its identity `0` and `$avg` +returns `null`. Both arms now wrap the aggregand in the boolean-only `$cond` +coercion #11065 landed, so a rate measure over a flag column reads the same on +this driver as on the others. + +**⛔ `min` / `max` are deliberately NOT coerced.** They are order statistics +over BSON canonical comparison order, which ranks booleans and returns a member +of the input domain — #11249 ruled they answer `false` / `true`, and coercing +them would have answered `0` / `1`, breaking that contract in the opposite +direction from the defect being fixed. Their lowering is unchanged; a pin reads +the emitted stages to keep it that way. + +**The coercion stays boolean-only.** `null`, a missing key and a non-numeric +string reach the accumulators exactly as before and stay excluded. Widening to +the other half of objectql's `toNumber` — which maps a non-numeric string to +`0` — would average garbage as zero rather than excluding it, a separate +question this change does not open; a control pins the exclusion. + +**Why `patch` and not `minor`.** This changes what an existing operation +returns, which ordinarily argues for `minor`. It is graded `patch` because the +returned values were **already ruled** before this change (#11065 for the +arithmetic pair, #11249 for the order statistics) and are stated as shared +values in `@objectstack/spec/data`; every other face already produced them, and +the sibling repair on `driver-memory` shipped as a patch. There is no new API, +no option, and no opt-out to describe — nothing here is a feature, and the only +behaviour a consumer could have depended on is a value this project has ruled +wrong and that no other driver produces. Calling it `minor` would advertise a +capability that does not exist and imply the old answer had standing. + +Not user-visible, and shipped in the same change because the two are one cell: +`mongodb-pipeline-evaluator.testkit.ts` — the server-free instrument that holds +this lowering to the shared table — applied its "arithmetic accumulators ignore +non-numeric values" filter to `$min` / `$max` as well, one arm too far, and so +answered `null` for them over a boolean column while the lowering under test was +correct. Those arms now ignore only null and missing, compare by BSON canonical +order, and refuse a type the evaluator does not rank instead of silently +answering `null`. diff --git a/packages/drivers/driver-mongodb/src/mongodb-11151-boolean-aggregand-answers.test.ts b/packages/drivers/driver-mongodb/src/mongodb-11151-boolean-aggregand-answers.test.ts new file mode 100644 index 0000000000..e29e0834ff --- /dev/null +++ b/packages/drivers/driver-mongodb/src/mongodb-11151-boolean-aggregand-answers.test.ts @@ -0,0 +1,272 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#11151] Boolean aggregands answer the RULED values on this face too — all + * four cells, ungrouped and grouped. + * + * ## The ruling this suite pins + * + * - **`sum` / `avg` answer arithmetic** — `3` / `0.5` over a 3-true/3-false + * fixture. The #11065 family shape, landed on `driver-memory` and on every + * SQL dialect (#11635). + * - **`min` / `max` answer `false` / `true`** — #11249 (maintainer 2026-08-23, + * recorded on that card's comment 5386670755, verbatim and untranslated: + * 「10950 不考虑存量，其他接受你的建议」). Order statistics return a member of + * the input domain, so the JSON boolean IS the contract and `0` / `1` is not + * a spelling of it. + * + * Measured on `origin/main` @ `23843d3f4` before the fix, through the harness + * below: `sum` = `0`, `avg` = `null`, `min` = `null`, `max` = `null` — all four + * wrong, whole-table and per group, while `count` = 6 and `count_distinct` = 2 + * already agreed. + * + * ## The two independent defects behind those four cells + * + * They are NOT one fix applied twice, and each is asserted here against the + * half it governs: + * + * 1. **The lowering** (`mongodb-aggregation.ts`) emitted a bare + * `{$sum: '$flag'}` / `{$avg: '$flag'}`. MongoDB's arithmetic accumulators + * ignore non-numeric values, so with no numeric value `$sum` folds to its + * identity `0` and `$avg` answers `null`. Fixed by the boolean-only `$cond` + * coercion. + * 2. **The instrument** (`mongodb-pipeline-evaluator.testkit.ts`) applied that + * same "ignore non-numeric" rule to `$min` / `$max`, which are order + * statistics over BSON canonical order and rank booleans perfectly well. The + * `{$min: '$flag'}` lowering was, and remains, correct. + * + * ⛔ Applying (1)'s coercion to `$min` / `$max` would answer `0` / `1` and break + * #11249 in the opposite direction. {@link describe} block "the emitted lowering + * keeps the two halves apart" pins that it was not, reading the emitted stages + * rather than trusting the values — the values alone cannot tell a `$min` over a + * boolean from a `$min` over a coerced `1`/`0` once the evaluator ranks both. + * + * ## ⚠️ What this suite deliberately does NOT answer + * + * Whether a real mongod agrees. `runPipeline` is the in-process evaluator + * `mongodb-aggregation-translation.test.ts` uses: it holds the LOWERING to the + * shared table by MongoDB's documented semantics, and this fleet cannot fetch a + * mongod binary at all (#5517). Every operator it models is read from the + * manual, not observed — `$type` and the BSON-order `$min`/`$max` this card + * added included. No test name here claims otherwise. + * + * ## The fixture + * + * `AGGREGATION_ROWS` — the shared aggregate-vocabulary fixture — plus a boolean + * `flag` column. The distribution is {@link FLAG_BY_ID}, the one already landed + * on `main` in `driver-sql`'s #11635 suite, chosen over the other distribution + * in this card's record (`true,false,true,true,false,false`) so the two faces' + * grouped numbers are comparable rather than merely both 3-true/3-false. + */ + +import { describe, it, expect } from 'vitest'; +import { AGGREGATION_ROWS } from '@objectstack/spec/data'; +import { + buildAggregationPipeline, + postProcessAggregation, + type AggregationInput, +} from './mongodb-aggregation.js'; +import { + runPipeline, + UnsupportedShape, + type Doc, +} from './mongodb-pipeline-evaluator.testkit.js'; + +/** The alias every measure is projected under — never a fixture column. */ +const MEASURE = 'measure'; + +/** + * The `flag` column, keyed by fixture row id: 3 true / 3 false, with `east` + * (rows 5–6) all-true. The per-group split is deliberately asymmetric — + * `west` is `[T,F,F,F]` and `east` `[T,T]` — so `east`'s grouped `min` is + * `true`: a measure computed over the whole table, or a sticky per-column + * constant, goes red on that cell rather than passing by symmetry. + */ +const FLAG_BY_ID: Record = { + '1': true, + '2': false, + '3': false, + '4': false, + '5': true, + '6': true, +}; + +/** + * The six shared rows plus `flag`, and two columns that exist only to drive the + * empty-input branch of an order statistic: `voidcol` is an explicit `null` on + * every row, and no row carries `absent` at all. + */ +const ROWS: Doc[] = (AGGREGATION_ROWS as ReadonlyArray>).map((row) => ({ + ...row, + flag: FLAG_BY_ID[row.id as string], + voidcol: null, +})); + +/** Run one aggregation end to end and answer the whole-table measure. */ +function measure(func: string, field: string): unknown { + const aggregations = [{ function: func, field, alias: MEASURE }] as AggregationInput[]; + const pipeline = buildAggregationPipeline({ aggregations }); + return postProcessAggregation(runPipeline(ROWS, pipeline), aggregations)[0]?.[MEASURE]; +} + +/** Run one aggregation grouped by `region`, keyed by group value. */ +function byRegion(func: string, field: string): Record { + const aggregations = [{ function: func, field, alias: MEASURE }] as AggregationInput[]; + const pipeline = buildAggregationPipeline({ aggregations, groupBy: ['region'] }); + const rows = postProcessAggregation(runPipeline(ROWS, pipeline), aggregations); + return Object.fromEntries(rows.map((row) => [String(row.region), row[MEASURE]])); +} + +describe('[#11151] driver-mongodb — the fixture this suite measures', () => { + // The fixture read back rather than trusted: a seed that dropped a row or + // folded the flags would turn every value below into a test of another table. + it('is six rows, 3 true / 3 false, with east all-true', () => { + expect(ROWS).toHaveLength(6); + expect(ROWS.filter((r) => r.flag === true), 'true rows').toHaveLength(3); + expect(ROWS.filter((r) => r.region === 'east').map((r) => r.flag)).toEqual([true, true]); + expect(ROWS.filter((r) => r.region === 'west').map((r) => r.flag)).toEqual([ + true, + false, + false, + false, + ]); + }); +}); + +describe('[#11151] the ruled arithmetic half — sum / avg count a boolean as 1 or 0', () => { + it('sum(flag) answers 3, not $sum’s identity 0', () => { + expect(measure('sum', 'flag')).toBe(3); + }); + + it('avg(flag) answers 0.5, not null', () => { + expect(measure('avg', 'flag')).toBe(0.5); + }); + + it('grouped sum/avg answer per group: west [T,F,F,F], east [T,T]', () => { + expect(byRegion('sum', 'flag')).toEqual({ west: 1, east: 2 }); + expect(byRegion('avg', 'flag')).toEqual({ west: 0.25, east: 1 }); + }); +}); + +describe('[#11151] the ruled order-statistic half — min / max answer JSON booleans', () => { + // Asserted STRICTLY. `0` / `1` satisfies a `Number()` reading and is exactly + // the answer #11249 ruled against, so a loose comparison here would pass on + // the one wrong value this half exists to exclude. + it('min(flag) answers false — the boolean, not 0 and not null', () => { + expect(measure('min', 'flag')).toBe(false); + }); + + it('max(flag) answers true — the boolean, not 1 and not null', () => { + expect(measure('max', 'flag')).toBe(true); + }); + + it('grouped min/max answer per-group members, and east’s min is true', () => { + // `east` is the load-bearing cell: all-true, so its `min` is `true`. A + // whole-table computation or a sticky `false` fails here and only here. + expect(byRegion('min', 'flag')).toEqual({ west: false, east: true }); + expect(byRegion('max', 'flag')).toEqual({ west: true, east: true }); + }); + + it('min/max over a column that is null or absent everywhere answer null', () => { + // The manual's rule: null and missing are IGNORED, and a group left with + // nothing answers `null`. Not folded to `false`, which is what a boolean + // face that manufactured a default would do. + expect(measure('min', 'voidcol'), 'explicit null on every row').toBeNull(); + expect(measure('max', 'voidcol'), 'explicit null on every row').toBeNull(); + expect(measure('min', 'absent'), 'a column no row carries').toBeNull(); + expect(measure('max', 'absent'), 'a column no row carries').toBeNull(); + }); +}); + +describe('[#11151] the emitted lowering keeps the two halves apart', () => { + const emit = (func: string): unknown => + buildAggregationPipeline({ + aggregations: [{ function: func, field: 'flag', alias: MEASURE }] as AggregationInput[], + })[0]; + + const COERCED = { + $cond: [{ $eq: [{ $type: '$flag' }, 'bool'] }, { $cond: ['$flag', 1, 0] }, '$flag'], + }; + + it('sum and avg wrap the aggregand in the boolean-only coercion', () => { + expect(emit('sum')).toEqual({ $group: { _id: null, [MEASURE]: { $sum: COERCED } } }); + expect(emit('avg')).toEqual({ $group: { _id: null, [MEASURE]: { $avg: COERCED } } }); + }); + + it('⛔ min and max are left BARE — the coercion is not applied to them', () => { + // The load-bearing pin of this file. `$min`/`$max` over a coerced aggregand + // would answer `0`/`1` — arithmetic where #11249 ruled for a member of the + // input domain — and the VALUES cannot catch it once the evaluator ranks + // booleans, because both spellings then produce an answer. Only the emitted + // stage distinguishes them. + expect(emit('min')).toEqual({ $group: { _id: null, [MEASURE]: { $min: '$flag' } } }); + expect(emit('max')).toEqual({ $group: { _id: null, [MEASURE]: { $max: '$flag' } } }); + expect(JSON.stringify(emit('min')), 'no $cond reached the min arm').not.toContain('$cond'); + expect(JSON.stringify(emit('max')), 'no $cond reached the max arm').not.toContain('$cond'); + }); + + it('a fieldless sum/avg is unchanged — the coercion needs a path to coerce', () => { + const fieldless = buildAggregationPipeline({ + aggregations: [{ function: 'sum', alias: MEASURE }] as AggregationInput[], + })[0]; + expect(fieldless).toEqual({ $group: { _id: null, [MEASURE]: { $sum: 0 } } }); + }); +}); + +describe('[#11151] CONTROLS — what neither half was allowed to move', () => { + // These two already agreed with every other face before this card. A suite + // holding only the broken cells cannot show that the fix was targeted. + it('count(flag) / count_distinct(flag) are unchanged', () => { + expect(measure('count', 'flag'), 'count over the boolean column').toBe(6); + expect(measure('count_distinct', 'flag'), 'count_distinct over the boolean column').toBe(2); + expect(byRegion('count', 'flag')).toEqual({ west: 4, east: 2 }); + expect(byRegion('count_distinct', 'flag')).toEqual({ west: 2, east: 1 }); + }); + + it('all four functions over the NUMERIC column are untouched', () => { + expect(measure('sum', 'score'), 'sum(score)').toBe(210); + expect(measure('avg', 'score'), 'avg(score)').toBe(35); + expect(measure('min', 'score'), 'min(score)').toBe(10); + expect(measure('max', 'score'), 'max(score)').toBe(60); + }); + + it('sum/avg still IGNORE a non-numeric string — the coercion is boolean-only', () => { + // `stage` is a string column with two explicit nulls. Coercing wider would + // mean adopting `Number('won') === NaN` or a `toNumber` that maps it to 0; + // both are separate questions from this card, and neither is adopted. + expect(measure('sum', 'stage'), 'sum over a string column').toBe(0); + expect(measure('avg', 'stage'), 'avg over a string column').toBeNull(); + }); +}); + +describe('[#11151] the evaluator REFUSES a type it does not rank, rather than answering null', () => { + // The head note of `mongodb-pipeline-evaluator.testkit.ts` promises this + // instrument "refuses every shape it does not model". The `$min`/`$max` arms + // were the exception: they filtered to numbers and answered `null` for + // everything else, silently. A wrong answer from a strict evaluator is worse + // than a refusal, because the red it produces reads as a defect in the driver + // under test — which is how this card's own `min`/`max` half was first + // misattributed to the lowering. + const withDate: Doc[] = ROWS.map((row) => ({ ...row, when: new Date('2026-01-01T00:00:00Z') })); + + for (const func of ['min', 'max'] as const) { + it(`${func} over an unmodelled BSON type raises UnsupportedShape`, () => { + const aggregations = [ + { function: func, field: 'when', alias: MEASURE }, + ] as AggregationInput[]; + const pipeline = buildAggregationPipeline({ aggregations }); + expect(() => runPipeline(withDate, pipeline)).toThrow(UnsupportedShape); + expect(() => runPipeline(withDate, pipeline)).toThrow(/unmodelled type/); + }); + } + + // The refusal is a property of the evaluator's coverage, NOT a statement + // about MongoDB: a real mongod ranks dates fine. Extending `bsonRank` is the + // way to model one, and until someone does the instrument says so out loud + // instead of answering `null`. + it('the types it DOES rank all answer, so the refusal above is not blanket', () => { + expect(measure('min', 'score'), 'number').toBe(10); + expect(measure('min', 'stage'), 'string').toBe('lost'); + expect(measure('min', 'flag'), 'boolean').toBe(false); + }); +}); diff --git a/packages/drivers/driver-mongodb/src/mongodb-pipeline-evaluator.testkit.ts b/packages/drivers/driver-mongodb/src/mongodb-pipeline-evaluator.testkit.ts index c5c44ea426..a268c29696 100644 --- a/packages/drivers/driver-mongodb/src/mongodb-pipeline-evaluator.testkit.ts +++ b/packages/drivers/driver-mongodb/src/mongodb-pipeline-evaluator.testkit.ts @@ -410,6 +410,11 @@ export function accumulate(rows: Doc[], acc: unknown): unknown { // so an unmodelled aggregand raises rather than collapsing to `null`. const present = values.filter((v) => v !== MISSING && v !== null); if (present.length === 0) return null; + // Rank every candidate BEFORE folding, so the refusal is a property of + // the aggregand's TYPE and not of the group's cardinality: a fold alone + // never compares a one-element group, and would hand back an unmodelled + // value unexamined — the same silence this arm was fixed to stop. + for (const v of present) bsonRank(v); return present.reduce((best, v) => (op === '$min' ? bsonLte(v, best) : bsonLte(best, v)) ? v : best, ); From b87754eb1a76cf4bdfec71400e6838c292acb526 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 27 Aug 2026 23:08:57 +0000 Subject: [PATCH 3/4] chore: ABLATION PREDICTIONS for #11151, committed BEFORE either mutation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two independent halves, ablated separately, because a single ablation that reds everything proves neither. ABLATION 1 — revert ONLY `mongodb-aggregation.ts`'s sum/avg coercion (`{ $sum: numericAggregandExpr(fieldRef) }` -> `{ $sum: fieldRef ?? 0 }`, same for `$avg`). PREDICTED DIRECTION: RED, and NARROWLY. - RED: 'sum(flag) answers 3' (expect 0), 'avg(flag) answers 0.5' (expect null), 'grouped sum/avg answer per group', and 'sum and avg wrap the aggregand in the boolean-only coercion'. - GREEN, untouched: every min/max test, including 'min and max are left BARE', the empty-window nulls, the refusal block, and all controls. Expected red count: 4. ABLATION 2 — revert ONLY `mongodb-pipeline-evaluator.testkit.ts`'s $min/$max arms to the number-filtered form (`numbers.length === 0 ? null : Math.min(...numbers)`, same for max). PREDICTED DIRECTION: RED, and NARROWLY. - RED: 'min(flag) answers false' (expect null), 'max(flag) answers true' (expect null), 'grouped min/max answer per-group members', 'min over an unmodelled BSON type raises UnsupportedShape' and the max twin (the arms would answer null instead of raising), and 'the types it DOES rank all answer' (min over the string column would answer null rather than 'lost'). - GREEN, untouched: every sum/avg test, the emitted-lowering block in full (it reads stages, not values, and the lowering is not mutated), 'min/max over a column that is null or absent everywhere answer null' (both forms answer null there — this one CANNOT discriminate and is predicted green in both legs), and the numeric-column control (min(score)=10 / max(score)=60 survive a number-only filter). Expected red count: 5. No rebuild is involved in either leg: the pin suite imports `./mongodb-aggregation.js` and `./mongodb-pipeline-evaluator.testkit.js` as in-package relative specifiers, which vitest resolves to `src/*.ts`, not to this package's `dist/`. The only built dependency in the closure is `@objectstack/spec/data`, which neither leg mutates. Part of #11151 From 4f25b63b8922a84743a3d836dcb2f5867fdc5b00 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 27 Aug 2026 23:16:26 +0000 Subject: [PATCH 4/4] test(driver-mongodb): retriage the two literal-spelling pins the coercion reds MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `builds $group with groupBy fields` and `builds multiple aggregations` spelled the `sum`/`avg` accumulator literally while their actual subject is alias routing and stage shape. Disposition: change the spelling — the accumulator's own internals are pinned, with reasons, in the new #11151 suite. The wrapper is written once as a local helper so the two pins stay readable as routing pins, and its note records that `min`/`max` deliberately do NOT take it (the `builds min/max aggregations` case below it is the pin that says so). Also: the new suite's `AGGREGATION_ROWS` cast goes through `unknown`, matching the spelling `mongodb-aggregation-translation.test.ts` already uses. Measured rather than assumed — this package's `typecheck` script EXCLUDES `**/*.test.ts` via its tsconfig, so `pnpm typecheck` was green over a program that had never read either test file. Checked with an ad-hoc program that includes them: my two files are clean; ten pre-existing errors in eight untouched test files are filed separately, not fixed here. Part of #11151 --- ...db-11151-boolean-aggregand-answers.test.ts | 2 +- .../src/mongodb-aggregation.test.ts | 22 ++++++++++++++++--- 2 files changed, 20 insertions(+), 4 deletions(-) diff --git a/packages/drivers/driver-mongodb/src/mongodb-11151-boolean-aggregand-answers.test.ts b/packages/drivers/driver-mongodb/src/mongodb-11151-boolean-aggregand-answers.test.ts index e29e0834ff..fb8f2ff788 100644 --- a/packages/drivers/driver-mongodb/src/mongodb-11151-boolean-aggregand-answers.test.ts +++ b/packages/drivers/driver-mongodb/src/mongodb-11151-boolean-aggregand-answers.test.ts @@ -96,7 +96,7 @@ const FLAG_BY_ID: Record = { * empty-input branch of an order statistic: `voidcol` is an explicit `null` on * every row, and no row carries `absent` at all. */ -const ROWS: Doc[] = (AGGREGATION_ROWS as ReadonlyArray>).map((row) => ({ +const ROWS: Doc[] = (AGGREGATION_ROWS as unknown as Doc[]).map((row) => ({ ...row, flag: FLAG_BY_ID[row.id as string], voidcol: null, diff --git a/packages/drivers/driver-mongodb/src/mongodb-aggregation.test.ts b/packages/drivers/driver-mongodb/src/mongodb-aggregation.test.ts index b3388635f5..6dfdcf8851 100644 --- a/packages/drivers/driver-mongodb/src/mongodb-aggregation.test.ts +++ b/packages/drivers/driver-mongodb/src/mongodb-aggregation.test.ts @@ -3,6 +3,22 @@ import { describe, it, expect } from 'vitest'; import { buildAggregationPipeline, postProcessAggregation } from './mongodb-aggregation.js'; +/** + * [#11151] The aggregand `sum` and `avg` now consume: the field path, with a + * BOOLEAN rendered as the number it is worth. Spelled once here because the + * pins below are about ALIAS ROUTING and stage shape — which accumulator lands + * under which key — and the accumulator's own internals are pinned, with the + * reasons, in `mongodb-11151-boolean-aggregand-answers.test.ts`. + * + * ⛔ `min` / `max` deliberately do NOT take this wrapper: they are order + * statistics and #11249 ruled they answer `false` / `true`, not `0` / `1`. The + * `builds min/max aggregations` case below reads their bare field path and is + * the pin that says so from this file. + */ +const coerced = (path: string) => ({ + $cond: [{ $eq: [{ $type: path }, 'bool'] }, { $cond: [path, 1, 0] }, path], +}); + describe('MongoDB Aggregation Pipeline Builder', () => { it('builds empty pipeline for no options', () => { expect(buildAggregationPipeline({})).toEqual([]); @@ -32,7 +48,7 @@ describe('MongoDB Aggregation Pipeline Builder', () => { groupBy: ['region'], }); expect(pipeline).toEqual([ - { $group: { _id: { region: '$region' }, total_amount: { $sum: '$amount' } } }, + { $group: { _id: { region: '$region' }, total_amount: { $sum: coerced('$amount') } } }, { $project: { _id: 0, region: '$_id.region', total_amount: 1 } }, ]); }); @@ -50,8 +66,8 @@ describe('MongoDB Aggregation Pipeline Builder', () => { const groupStage = pipeline[0]; expect(groupStage.$group._id).toEqual({ customer_id: '$customer_id' }); expect(groupStage.$group.order_count).toEqual({ $sum: 1 }); - expect(groupStage.$group.total).toEqual({ $sum: '$amount' }); - expect(groupStage.$group.average).toEqual({ $avg: '$amount' }); + expect(groupStage.$group.total).toEqual({ $sum: coerced('$amount') }); + expect(groupStage.$group.average).toEqual({ $avg: coerced('$amount') }); }); it('adds $sort stage', () => {