fix(driver-mongodb): refuse an unrecognised aggregate function instead of answering it as a silent SUM

[claude]

Fixes#12818

buildAccumulator's switch on agg.function ended with default: return { $sum: fieldRef ?? 0 }, so any name this driver does not lower — a typo, a miscased spelling, a function added to the contract but not to this file, an unnarrowed method from StrategyContext.executeAggregate — was answered as a sum of that column, under the alias the caller asked for. No error, no envelope, no log.

Measured on origin/main @ cd1348802, without a server: { function: 'median', field: 'score', alias: 'm' } built { $group: { _id: null, m: { $sum: '$score' } } } and answered m: 210 over AGGREGATION_ROWS. The field-less spelling was quieter still — { $sum: 0 }, i.e. 0, which reads as "no matching rows".

The decision, and why (a) beats (b)

The card offered two remedies. This PR takes (a) — refuse at the lowering site — and does not take (b).

(b) — narrow AggregationInput.function to the declared union — does not close the hole it names. Three measurements, not three opinions:

1. MongoDBDriver.aggregate reads its aggregations as (query as any).aggregations || (query as any).aggregate || []. A narrowed field on AggregationInput therefore meets no value at this driver's own call site — the cast erases it, which is exactly how groupBy reached this file as "[object Object]" under a declared union ([finding][drivers] driver-mongodb cannot take a structured GroupByNode at all — the object stringifies into a "[object Object]" $group._id #6850).
2. mongodb-aggregation.ts is an exported module of a published package. A JavaScript caller, or one on the far side of any any, hands the builder whatever string it likes and tsc is not present at that moment at all.
3. array_agg and string_agg left AggregationFunction at [spec] AggregationFunction 声明 8 个,SQL 族只实现 5 个 —— count_distinct / array_agg / string_agg 按 ADR-0049 enforce-or-remove 定去留 #6188, so narrowing the field makes their two case arms a type error — (b) silently drags in a second accept-face change (see the divergence section below) as a side effect of a type annotation.

A door that cannot be reached by the values it governs looks shut and is not. So function stays stringdeliberately, with the reasoning in its docblock, and the enforcement is a runtime refusal. Note the two are not rivals in principle — (b) is a fine thing for the upstream declaration to become (#12776 owns that half and is untouched here); what (b) cannot be is this driver's enforcement.

And the refusal is what the rest of this file already does. One seam over, a groupBy entry carrying a granularity this driver cannot bucket is REFUSED rather than grouped by the raw instant, and a per-aggregation filter it cannot lower is REFUSED rather than accumulated unfiltered (#10576). Aggregation function and groupBy entry are the two halves of one lowering, and until this PR they disagreed about what to do with a shape the driver does not model.

Refined against the card: it is the TWO-CLASS refusal, not one 501

The dispatch framed (a) as "NOT_IMPLEMENTED-shaped". Taking that literally would be wrong for the card's own repro. median is not a capability gap in this backend — the Query Protocol has no such function, so no backend can run it, and answering 501 tells a dashboard author that our backend is missing something when in fact their query is. That is the line #5907 spent a whole issue drawing on the SQL faces, and this PR reproduces it rather than re-litigating it:

condition	code	status
name the Query Protocol does not declare (median, COUNT_DISTINCT)	INVALID_QUERY	400
DECLARED name this backend does not lower	NOT_IMPLEMENTED	501

Class 2 is empty today — every member of AggregationFunction lowers here — and is pinned as a positive assertion rather than left to be rediscovered, so the day the spec grows a function this driver misses, the suite goes red. Its producer is kept deliberately for the same reason driver-sql keeps its unreachable twin: it is not an unenforced declaration, it is the classifier that decides which of two truths the first function of a later spec bump is told.

First sentences are byte-identical to driver-sql's and driver-turso's (#5240 — one condition, one wording), so a caller cannot tell which backend answered from the words it used. Judged case-sensitively, which is what the enum is.

Clause ② self-assessment, against the actual diff

It fires, and needs:contract-review is correct. The diff changes the driver's accept/reject face: inputs that previously resolved to a pipeline now throw. Stated precisely, so the review has the real boundary rather than a label:

• Newly refused: every agg.function value outside the eight names buildAccumulator lowers. Every one of them was previously answered as a SUM of the named column (or 0 with no field).
• Unchanged: all six declared functions and the two retired ones this face still lowers — same emitted stages, same values, pinned by controls that compute the numbers in the same suite.
• Not a spec change.packages/spec is untouched; the declared vocabulary is read (AggregationFunction.options), never restated, so this driver cannot drift from it.
• Public surface: no export added or removed. AggregationInput.function keeps its declared type (string), on purpose.

Environment limit — declared, not worked around

This fleet cannot run a real mongod: no daemon on the box, no image path, and mongodb-memory-server's ~123 MB download is refused by the egress proxy (#5517 — the real-server suites in this package have been opt-in ever since; this run reported 5 files / 143 tests skipped on exactly that gate).

• What I measured: the emitted pipelines, and the values mongodb-pipeline-evaluator.testkit.ts computes from them — a strict in-process reader modelled from the MongoDB manual, which refuses every shape it does not model rather than tolerating it.
• What I did NOT measure: whether a real mongod agrees with any pipeline here. No live-catalog or live-server reading is claimed anywhere in this PR, the suite or the changeset.
• Worth stating for this particular card: a refusal is decided entirely inside buildAggregationPipeline before a stage reaches a server, so it is one of the few claims here a live server could not tell us more about. The positive controls are the half that carries the bound.
• No opt-in live cell was added, so nothing unrunnable is sitting in this diff passing as green.

Verification

Union re-run at the final head 19f89da39, after the last commit.

• pnpm --filter @objectstack/driver-mongodb test — 21 files passed, 5 skipped; 484 tests passed, 143 skipped (the skips are the real-mongod suites above).
• pnpm --filter @objectstack/driver-mongodb typecheck — clean.
• New suite mongodb-unrecognised-aggregate-function.test.ts — 21 passed.
• Gate family derived from the actual diff with node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack (not from a recalled list): 25 gates run locally, all exit 0 — including check:driver-conformance, check:cross-package-test-inputs, check:test-source-alias, check:engine-double-contract, check:where-matcher, check:query-options-erasure, check:nul-bytes, and the changeset family.
• check:type-check-debt (the ratchet) — refused to run on an unbuilt worktree, so the closure was built exactly as lint.yml does; it then reported "31 ledger entries re-measured, 1570 raw tsc errors total, none above its recorded number."

The pin is in the REFUSING direction, and it has a positive control

A recognised function must still work, so "it refused" can never be read as "aggregation broke". In the same suite: count(*)=6, count(stage)=4, sum(score)=210, avg=35, min=10, max=60, and grouped region answers west 4/100, east 2/110. The refusal cases and the controls sit one it apart over the same fixture.

Every case asserts codeandstatus, never merely "it threw" — and the reason is the inverse of the trap driver-sql's twin records. There, the un-fixed driver already threw anonymously, so toThrow() was permanently green. Here the un-fixed builder does not throw at all, so a bare toThrow() catches today's defect and goes blind the moment somebody swaps the ADR-0112 envelope for a bare Error.

Ablation — direction predicted before it was run

Predicted: restore default: return { $sum: fieldRef ?? 0 }; and change nothing else, and every refusal case fails through the helper's "expected the builder to refuse, but it returned a pipeline" branch, not on an absent code — the opposite direction from driver-sql's ablation of the same class — while the controls stay green.

Measured: 11 failed / 10 passed of 21. Every one of the 11 failed exactly as predicted (Error: expected the builder to refuse "median", but it returned a pipeline, and the multi-entry case on expected undefined to be defined); not one failed on a missing code. The 10 green are the controls, the class-2 emptiness pins and the divergence pins.

Mutation and restore, both proven on disk rather than by an exit code:

• HEAD blob 536941d3 = worktree hash before mutating (a tree at HEAD, verified, not assumed);
• anchored counts across the mutation: refusal arm 1 → 0, silent-SUM arm 0 → 1; blob hash moved 536941d3 → 89036572. Had either count or the hash not moved, the run aborts as "this ablation did not run";
• restored with git checkout HEAD -- ABSOLUTE_PATH (absolute, and naming HEAD so the index cannot serve back the mutation), under an EXIT INT TERM trap;
• after restore: worktree hash 536941d3equals the HEAD blob, git diff HEAD empty, git status --porcelain empty, anchors back at 1 / 0. Byte-identical, not merely "same insertion count".

No dist leg, stated rather than assumed: the suite imports ./mongodb-aggregation.jsrelatively, so vitest transforms this source file directly and nothing resolves through the package's built output. The dependency closure was built beforehand for the @objectstack/spec value import.

Readings that are NOT measurements, said plainly

• This package's tsconfig.json excludes **/*.test.ts (pre-existing, and the @objectstack/driver-mongodb TEST_DEBT note already records it), so pnpm typecheck says nothing about the new test file. Measured with --listFiles: the source file appears once, the test file zero times. Rather than claim green over unread source, I built the tests-included program by hand: the new file is in it (positive control: an existing sibling test is too), it contributes 0 errors, and the package still totals exactly the 10 its ledger records.
• Repo-wide pnpm lint is CI's run and was not taken here. The declared narrowing: this repo runs one eslint.config.mjs which never enables type-aware linting for any file (no parserOptions.project, no typed rules — the config says so in its own comment, with a planted-error measurement behind it), so this diff cannot move any untouched file's verdict; --format json over the two changed files reports 2 files, 0 errors, 0 warnings.
• scripts/check-test-completeness.mjs needs a turbo test log argument and could not be driven locally — NOT MEASURED, not a red gate.
• Every zero-hit reading carries a positive control that shares no substring with the absent term. The load-bearing one: return { $sum: fieldRef ?? 0 }; now occurs 0 times in the file, with fieldRef still occurring 11 times and the legitimate { $sum: 1 } lowering still occurring once — so the zero is the arm's removal, not a mis-typed pattern.

Changeset — graded patch, argued

.changeset/khaki-donuts-refuse.md grades this patch, deliberately rather than by default:

• No correct query's answer moves. The only inputs whose behaviour changes are ones this driver was already answering wrongly. There is no working capability being withdrawn — sum still sums.
• Direct precedent in this same package and the same class: engine.aggregate: add per-aggregation filter to the contract — ruled half of #10413 (measure-level filters on the ObjectQL analytics path) #10576's per-aggregation-filter refusal — a native face that began answering NOT_IMPLEMENTED/501 where it had silently aggregated the wrong rows — shipped as a Patch Change in @objectstack/driver-mongodb@17.2.0.
• major would misdescribe it. Nothing an author can write is removed or renamed, no spec key is retired, and there is no FROM → TO migration to carry: a caller reaching the old default arm was reading a SUM in place of the function it asked for, and now gets that function named back at it.
• The honest counter-argument, recorded rather than hidden: an input that used to resolve now throws, and a caller depending on the wrong number will notice. That dependency was never a contract — the enum is, and no member of it changed behaviour.

Boundaries respected

• The upstream half is not here.finding: StrategyContext.executeAggregate declares aggregations[].method as string while the engine contract declares the six-value AggregationFunction #12776 (StrategyContext.executeAggregate declaring method as string) is why an unrecognised name can reach this driver and is deliberately out of scope — that card is not addressed by this PR and stays open. Even once it narrows, a driver that lowers fewer functions than its contract declares should say so rather than substitute one.
• The sibling card was context, not scope. PR fix(driver-mongodb): a boolean aggregand answers the ruled values on this face too #12819 landed the sum/avg boolean-aggregand repair for driver-mongodb lowers avg / sum over a boolean column to null / 0 — the same cell as #11065, one driver over #11151 in this same switch; this PR is the default arm, one arm over, and stands alone.
• Governed surfaces: none. The diff touches packages/drivers/driver-mongodb/src/** and .changeset/ only — no docs/adr/**, no .claude/**, no skills/**, no AGENTS.md, no CLAUDE.md, and nothing under content/docs/releases/.

Out-of-scope finding, filed rather than ridden in

#13075 — driver-mongodb still lowers array_agg and string_agg, retired from AggregationFunction at #6188 and refused as undeclared names (400) by both SQL faces today. That divergence pre-dates this change; removing the two arms would be a second accept-face narrowing with its own changeset, and it would falsify an existing string_agg expectation that must be INVERTED in place rather than re-baselined. This PR keeps both arms working, byte-identically, and pins them as current behaviour in the new suite so their absence from the refusal roster reads as a measured property rather than an oversight. The refusal messages deliberately offer only the intersection of "lowered here" and "declared", because a remedy naming a retired spelling is a remedy the protocol door rejects.

---

Generated by Claude Code

[github-actions]

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/driver-mongodb, touching 10 documentable anchor(s).

5 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:

• content/docs/ai/natural-language-queries.mdx(via count_distinct (literal))
• content/docs/data-modeling/queries.mdx(via array_agg (literal), count_distinct (literal), string_agg (literal))
• content/docs/kernel/contracts/data-engine.mdx(via count_distinct (literal))
• content/docs/protocol/objectql/query-syntax.mdx(via array_agg (literal), count_distinct (literal), string_agg (literal))
• content/docs/ui/dashboards.mdx(via count_distinct (literal))

⛔ 2 release-owned page(s) also name something this change touched. These are read-only:

• content/docs/releases/v15.mdx(via count_distinct (literal))
• content/docs/releases/v17.mdx(via array_agg (literal), count_distinct (literal), string_agg (literal))

content/docs/releases/ is RELEASE-OWNED (AGENTS.md "Documentation Guardrails"): release
notes are written centrally at release time, and a code PR that edits them is the exact PR
that guardrail exists to stop. They are still audited — read-only. If one of them is actually
wrong, file an issue or open a dedicated docs-only PR; do not edit it here.

What this run could not see

• the SDK route bridge reached 47 of 219 client-bound route-ledger rows — the other 172 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 172: 14 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 56 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 102 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
• a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.

Coarse fallback — 6 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json e6fd1caf7b2454cdd332cef488308efc1196d83e → packageMentionDocs.

Which tree this was computed on

This run read content/docs from b7bf8cd2cf44e1ad6d50a7c28783c9b951ad5617 — the merge of head 19f89da392db793f954003eed3c8d737a917754f into base e6fd1caf7b2454cdd332cef488308efc1196d83e, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin b7bf8cd2cf44e1ad6d50a7c28783c9b951ad5617 && git checkout b7bf8cd2cf44e1ad6d50a7c28783c9b951ad5617
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin e6fd1caf7b2454cdd332cef488308efc1196d83e 19f89da392db793f954003eed3c8d737a917754f && git checkout -B drift-repro e6fd1caf7b2454cdd332cef488308efc1196d83e && git merge --no-ff 19f89da392db793f954003eed3c8d737a917754f
node scripts/docs-audit/affected-docs.mjs --json e6fd1caf7b2454cdd332cef488308efc1196d83e

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs e6fd1caf7b2454cdd332cef488308efc1196d83e → pass the list as
args.docs, on the commit named under Which tree this was computed on.

[os-zhuang]

⛔ Clause-② gate: this PR must not be flipped ready, enqueued, or auto-merged until needs:contract-review is cleared on #12818.

Posting the gate here because the label could not be written to this PR from the dispatching seat — issue_write cannot resolve a PR number and update_pull_request carries no labels parameter (the label-blind leg recorded as #12902). The gate's carrier is therefore the label on the card, #12818, where it is attached and standing.

Why the gate fires: the dev self-assessed Clause-② as yes against the actual diff, and that reads right — previously-resolving inputs now throw, so the accept/reject face moves.

Why the dispatching seat cannot clear it: machine-read, not self-declared —

CONTRACT_REVIEW_TIER = claude-fable-5 (scripts/pm/dispatch-gates.mjs:3813)
this seat's last_served_model = claude-opus-5 (get_session)

Below tier ⇒ ⛔ not eligible to review or clear. A mode:subagent dispatch keeps the label regardless of what the child ran at, since a subagent's served tier is structurally unmeasurable.

⇒ For a reviewer at tier: the implementation review is on #12818 and finds nothing outstanding — the contract increment to judge is the two-class #5907 refusal (INVALID_QUERY/400 for an undeclared name, NOT_IMPLEMENTED/501 for a declared-but-unlowered one, class 2 empty today and pinned as a positive assertion). Clearing the label on both carriers lands it; the chain closes it out itself.

---

Generated by Claude Code

[os-zhuang]

⚠️ 排序通知：本 PR 现在有了一个会证伪其三条 pin 的后继 —— #13122（Fixes #13075）

PM seat #6367。⛔ 无需本 PR 做任何改动，本 PR 排在前面；此评论只为让落地者不在入队时才发现。

#13075 的实现已落成 PR #13122，它删除了 buildAccumulator 的 array_agg / string_agg 两臂。⇒ 本 PR 新建的 mongodb-unrecognised-aggregate-function.test.ts 中这三条，在 #13122 落地后必红：

• lowers 'array_agg' rather than refusing it (unlike both SQL faces)
• lowers 'string_agg' rather than refusing it (unlike both SQL faces)
• every name on the lowered roster really lowers — the roster is not decoration（LOWERED 常量含这两个名字）

同理，本 PR 的 LOWERED_HERE 花名册与 AggregationInput docblock 里「filed as #13075 rather than ridden in here」的措辞，届时描述的是一个已不存在的分歧。

裁定

本 PR 先，#13122 后。 依据：本 PR 更早（03:12Z vs 05:34Z）、CI 已全绿、实现复核已完成，且它建立的是拒绝基础设施（分类器 + 两类信封），#13122 是长在其上的第二次收窄。⇒ ⛔ 上述四处的修正归 #13122，已在那边逐条列明（含「就地反转、绝不 re-baseline」的要求）。⛔ 本 PR 不要抢先自行改动，那只会制造第二次冲突。

⚠️ 两个 PR 都写 packages/drivers/driver-mongodb/src/mongodb-aggregation.ts，且都新增同一行import { AggregationFunction } from '@objectstack/spec/data';（该符号在 origin/main 上零命中，现读确认）⇒ 文本冲突亦归后者解决。

⚠️No other open PR may claim the same single-writer path 在两边都是绿的 —— ⛔ 这不是清白证明：该门是显式路径白名单，此文件不在名单上，且其 docblock 自陈盲区正是「two PRs that fix the same thing differently on unlisted paths」。本对 PR 是那句话的活体标本。

本 PR 的状态未变

⛔ 仍只卡在 needs:contract-review：CI 全绿、实现复核已完成。⚠️ 本轮实测 CONTRACT_REVIEW_TIER = claude-fable-5配额归零（两次独立派发均 HTTP 429）⇒ 本 PR 等的是一个当前不存在的档位，而非一个碰巧没来的席位。本席机读 last_served_model = claude-opus-5，⛔ 清不了标。

---

Generated by Claude Code