Skip to content

fix(objectql): the in-memory aggregate lowering asks the driver for rows, so a measure filter stops being refused on driver-memory (#16642) - #17251

Merged
os-trump merged 5 commits into
mainfrom
claude/issue-16642-objectql-strategy-falls-through
Sep 10, 2026
Merged

fix(objectql): the in-memory aggregate lowering asks the driver for rows, so a measure filter stops being refused on driver-memory (#16642)#17251
os-trump merged 5 commits into
mainfrom
claude/issue-16642-objectql-strategy-falls-through

Conversation

@os-trump

Copy link
Copy Markdown
Collaborator

Fixes #16642

Clause-②: no

What the chain turned out to be

The dispatch's mechanism assumption was that the ObjectQL strategy "compiles the measure filter into a per-aggregation filter on a driver.find aggregation". Measured on origin/main, the strategy does no such thing — and that half is correct. ObjectQLStrategy lowers a dataset measure filter onto engine.aggregate's aggregations[].filter, which is the contract slot #10576 added for exactly this purpose and which the spec documents as honoured "on every driver by lowering in memory when the driver has no native conditional aggregation".

The driver.find call in the stack trace is made by the engine, one frame lower:

engine.aggregate()
  -> hasAggregationFilter -> take the IN-MEMORY lowering path
  -> driver.find(object, ast)        <- ast STILL carries aggregations[].filter
       driver-memory find() -> performAggregation() -> refusePerAggregationFilter -> 501

find()'s contract says nothing about groupBy / aggregations, and the drivers disagree about them. driver-sql and driver-rest ignore both and return rows — which is the only reason this path has ever worked. driver-memory honours them, and so do the aggregate(AST) faces behind driver-mongodb / driver-turso.

So the refusal is not being raised against an app, and not against service-analytics either. It is raised against the engine's own lowering — the one caller the refusal's remedy text is addressed to ("route the query through the engine"), refusing the engine for routing through the engine. Both driver refusals already claim in writing that they are "unreachable through engine.aggregate, which lowers in memory for every driver" (driver-memory's refusePerAggregationFilter, driver-sql's unsupportedAggregationFilterError). That sentence was false. This PR is the line that makes it true.

The fix

One seam in packages/objectql/src/engine.ts: on the in-memory lowering path the AST handed to find() carries no groupBy, no aggregations and no having — the three things that path is about to evaluate itself. where is untouched, so the middleware-injected read scope (RLS / tenancy, #2737) still travels with the call.

driver-memory's #10413 refusal is not touched, and no fall-through, catch or retry is added anywhere. A capability gap is no longer manufactured by the caller, so there is nothing to catch: the refusal keeps firing, unchanged, for the direct callers its docblock says it is for.

Why not the dispatched route (a fall-through rung in the strategy chain)

Reported, not decided unilaterally — see the report's open_questions. Three measurements pushed against it:

  1. The rung had nowhere to stand. evaluateAnalyticsQueryOverRows needs ROWS, and StrategyContext (spec contracts/analytics-service.ts, the T1 surface this dispatch fences off) exposes executeRawSql and executeAggregate and no row bridge. Landing the dispatched route means widening that contract — a declared STOP-and-report trigger.
  2. It would have left the defect in place for every other caller of engine.aggregate (flows, views, roll-up summaries), and answered the analytics door by re-implementing, in service-analytics, an aggregation the engine already implements.
  3. A second, silent symptom shares the root cause and no analytics-side rung would have touched it (below).

The silent half, found on the way

Same seam, no refusal. When the driver groups first, the engine aggregates the driver's GROUP rows a second time:

probe, driver-memory, 88 rows over 7 distinct timestamps before after
groupBy: [{ field: 'created_at', dateGranularity: 'day' }], count 25 (a count of buckets) 88

That is the shape #16178 reports (timeDimensions[].granularity never buckets — a week of 38 applications on 7 distinct days reads 7). ⛔ This PR does not claim that card: it is filed against the analytics door, its acceptance is not measured here, and no closing keyword names it. #16178 remains open.

Evidence

Predictions were written before the ablation (all six held). Full before/after runs, the two-driver differential, and the restoration proof are in the PR comments below and in the dev report.

验收备注

  • 三条 Expected 的次序裁定(分诊席)是:主修法 = 让拒绝被接住;必须一并做 = 授权时点可见性(os validate / os lint 提示不可移植);最后才考虑下放逻辑。本 PR 落的是「拒绝不再被制造出来」,不是第 1 条的第二处实现。
  • 第 2 条(os validate / os lint 的可移植性提示)不在本 PR 范围内,且本修复让它变得不那么紧迫但没有消失:measure filter 现在在两个驱动上都可用,而 timeDimensions[].granularity(driver-memory analytics accepts timeDimensions[].granularity and never buckets by it — one group per distinct timestamp #16178)与其它能力差仍然对作者不可见。
  • 分诊席验收口径第 3 条要求「回落必须显式且可观测」:本路线没有回落,所以没有可观测性债 —— 没有性能路径被静默绕开,pushdown 分叉一字未动。
  • 分诊席验收口径第 4 条(console 的「Analytics capability is not installed on this deployment」归因错误)⛔ 未处理:它住在 objectui,不在本仓。

Out-of-scope findings

  • noted, not filed: engine.aggregate's in-memory path fetches every matching row before aggregating. That is unchanged by this PR (it is what driver-sql already did on this path), but on a driver that used to push down it is now a real read. Worth a capability flag (supports.queryAggregationFilter) if it ever bites — the fork is already written to accept one.
  • noted, not filed: packages/drivers/driver-sqlite-wasm implements no aggregate(), so every aggregate on it takes this same rows path.

Generated by Claude Code

…ows (#16642)

`engine.aggregate`'s in-memory fallback handed `driver.find()` the whole
aggregate AST — `groupBy`, `aggregations` and `having` included — although it
was about to evaluate all three itself. `find()`'s contract says nothing about
those keys and the drivers disagree: `driver-sql` / `driver-rest` ignore them
and return rows, `driver-memory` honours them.

Against a driver of the second kind the seam answered two wrong things: the
per-aggregation `filter` that routed the call to this path was refused
NOT_IMPLEMENTED/501 by the driver's own #10413 guard (whose remedy text tells
the caller to "route the query through the engine" — and the caller was the
engine), and a date-bucketed `groupBy` came back already grouped on the raw
timestamp and was aggregated a second time, reporting a count of buckets under
the author's own measure name.

The AST sent to `find()` on this path now carries none of the three. `where` is
untouched, so a middleware-injected read scope still travels with the call, and
the pushdown fork is unchanged.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012zTkyNHJ7TkuN2oXtP5x37
@github-actions

github-actions Bot commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

Nothing in this diff resolved to a documentable surface (no symbol, route or SDK anchor derived from 1 changed package(s)), so this run has no opinion about the docs.

What this run could not see
  • 1 name(s) were too generic to anchor anything (single lowercase words)
  • 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 — 17 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 a256f18962970878a0215109ddb1c4f0357d105apackageMentionDocs.

@github-actions github-actions Bot added documentation Improvements or additions to documentation tests tooling labels Sep 10, 2026
`tsconfig.test.json` reaches this file, and indexing a literal-typed fixture
row with an `any` group key was a TS7053. Rows are `Record<string, unknown>`
now and the group fields are resolved once, ahead of the loop, so the driver
face reads no `dateGranularity` — which is the property that makes the
double-aggregation case reproduce.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012zTkyNHJ7TkuN2oXtP5x37

Copy link
Copy Markdown
Collaborator Author

Measurement record — #16642

Every run below is @objectstack/objectql at this branch's HEAD, against the real driver-memory / driver-sqlite-wasm and the real AnalyticsServicePlugin bridges (a fake plugin context whose data service is a live ObjectQL engine — the wiring packages/services/service-analytics/src/__tests__/aggregate-bridge-function-vocabulary.test.ts already uses). Fixture: 88 applications, 15 of them stage: 'rejected' — so the card's own 15 and 15/88 = 0.1704….

⚠️ These probes are measurements, not committed tests. @objectstack/driver-memory is under a retirement census (scripts/check-driver-memory-census.mjs, #6664) that requires a maintainer ruling before a package may declare it, so the acceptance differential is measured here and the permanent pins are stand-in-driver pins inside packages/objectql — see the report's open_questions.

Predictions, written before the ablation

# case predicted WITHOUT fix predicted WITH fix held?
1 measured cell (count + filter{stage:rejected}) 501 NOT_IMPLEMENTED 15 yes
2 ratio over two differently-filtered counts 501 0.1704545… yes
3 MECHANISM: AST the driver's find() receives carries groupBy+aggregations carries neither; where survives yes
4 date-bucketed groupBy (the #16178 shape) a count of BUCKETS true counts yes
5 control: aggregation with NO filter driver pushdown identical yes
6 control: genuine driver fault (INVALID_FIELD/400) propagates propagates unchanged yes

Acceptance table — AnalyticsService.queryDataset, one dataset, two drivers

selection memory BEFORE memory AFTER sqlite BEFORE sqlite AFTER
{ measures: ['rejected_count'] } 501 NOT_IMPLEMENTED [{"rejected_count":15}] [{"rejected_count":15}] [{"rejected_count":15}]
ratio selection 501 [{"total_count":88,"rejected_count":15,"rejection_rate":0.17045454545454544}] same same
control — measure with NO filter [{"total_count":88}] [{"total_count":88}] same same
control — grouped by stage 15 / 25 / 48 15 / 25 / 48 same same

⭐ Memory now answers 15, not "something": the same number sqlite gives, and the ratio matches sqlite's to the last digit. ⭐ sqlite's four cells are identical before and after — it never reaches this seam (NativeSQLStrategy compiles raw SQL), which is the fence "no behaviour change on the sqlite / NativeSQL path" measured rather than asserted.

Engine seam, driver-memory, before → after

A control: count, NO per-aggregation filter   88                        ->  88
B target : count filter{stage:'rejected'}     501 NOT_IMPLEMENTED       ->  [{"rejected_count":15}]
C ratio  : two differently-filtered counts    501 NOT_IMPLEMENTED       ->  [{"rejected_count":15,"total_count":88}]
D sibling: groupBy dateGranularity 'day'      [{"n":24}] / [{"n":25}]   ->  [{"n":88}]

Row D's BEFORE value moves between runs and that is the tell: it is a count of the driver's own buckets (one per distinct created_at), so it tracks insert timing rather than the data. No refusal, no log line — a plausible wrong number under the author's own measure name. AFTER it is the row count, 88.

The two failure kinds, and how they stay apart

⛔ Nothing is caught, retried or swallowed by this change — there is no new catch anywhere in the diff, which is why "a genuine error becomes an empty chart" cannot arrive through it. A capability gap is no longer manufactured by the caller: driver-memory still refuses a per-aggregation filter with the same NOT_IMPLEMENTED/501 for the direct callers its docblock names, and the pin control: a GENUINE driver error still surfaces drives an INVALID_FIELD/400 out of find() on this exact path and asserts the ADR-0112 envelope arrives at the caller unchanged.

Restoration proof (ablation)

Mutation and measurement ran in ONE shell under trap … EXIT INT TERM; the restore leg is git checkout HEAD -- ABSOLUTE_PATH, proved by blob equality and an empty diff, never by an exit code.

HEAD blob for packages/objectql/src/engine.ts = d55df93624b8e536ac47972b28a74894f4e2716c
mutated blob                                  = 54c97ee2b272217433849b482eab991475c616c5   (differs -> the mutation was not a no-op)
  grep -c rowsAst on disk (want 0): 0
  git diff --numstat HEAD: 1  33  packages/objectql/src/engine.ts
  ablation-dist-preflight @objectstack/objectql 'rowsAst' --absent
    marker absent from all 14 built files          <- the mutation reached dist/, so the BEFORE runs measured it
restored blob                                 = d55df93624b8e536ac47972b28a74894f4e2716c   (== HEAD)
  git diff HEAD -- PATH : 0 bytes
  grep -c rowsAst on disk (want 5): 5
  ablation-dist-preflight @objectstack/objectql 'rowsAst'   -> present again

Both legs rebuilt @objectstack/objectql before measuring — the probes import dist/, so an unbuilt leg would have measured the other leg's artifact. That is why the preflight runs on both.

Control — the NativeSQL fall-through still happens exactly where it already did

Counted from the service's own log line (… cannot run on this driver (raw SQL unsupported) — falling back to the next strategy.) across the four selections:

arm BEFORE AFTER
memory 5 5
sqlite 0 0

Same count, same driver, same reason. ⛔ No rung was added to the ladder and none was moved: the diff contains zero new try/catch (git diff origin/main...HEAD | grep -cE '^\+.*(try \{|catch \()' = 0), and the whole functional change is five lines:

-        const raw = await driver.find(object, ast, this.buildDriverOptions(object, opCtx.context));
+        const rowsAst: QueryAST = { ...ast };
+        delete rowsAst.groupBy;
+        delete rowsAst.aggregations;
+        delete rowsAst.having;
+        const raw = await driver.find(object, rowsAst, this.buildDriverOptions(object, opCtx.context));

The #10413 refusal is still pinned, untouched

packages/drivers/driver-memory/src/memory-aggregation-filter-refusal.test.ts drives driver.aggregate(AST) and driver.find() directly, never through the engine, so both of its doors keep refusing NOT_IMPLEMENTED/501 and both pins stay green. ⛔ Nothing in packages/drivers/** is touched by this PR.

Published-surface delta (Clause-② reporting)

Zero. packages/objectql/src/index.ts is untouched (git diff origin/main...HEAD -- packages/objectql/src/index.ts = 0 lines), no symbol is added or renamed, no signature moves — the change is entirely inside one method body. No packages/spec/src/** edit, no packages/drivers/** edit, no error-code-ledger row. None of the dispatch's STOP-and-report triggers fired.

What was run

command verdict
pnpm --filter @objectstack/objectql exec vitest run --project local src/engine-aggregate-rows-ast.test.ts Test Files 1 passed (1) · Tests 6 passed (6) (re-run after the fixture-typing commit: same)
pnpm --filter @objectstack/objectql test Test Files 289 passed (289) · Tests 4861 passed (4861) · 287.14s
pnpm --filter @objectstack/objectql typecheck exit 0 — tsc --noEmit (both configs) + check:test-typecheck: OK … 44 file(s) / 242 error(s) / 69 pinned signature(s) (ledger unchanged: the new pin file is NOT in it)
node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack 63 derived
the 63 derived commands 61 exit 0 · 2 exit 3 = PREREQUISITE NOT MET, i.e. NOT MEASURED
node scripts/pm/dispatch-gates.mjs --ran ran.list 63 derived, 63 run, 0 NOT-MEASURED, 0 UNRUN

⚠️ The reconciler's 0 NOT-MEASURED is its own claim about my list, not a measurement (#17204). Annotating them myself: pnpm check:dual-build-cjs-loads and pnpm check:type-check-debt each exited 3 with PREREQUISITE NOT MET — both read built output for the WHOLE repo (pnpm build / turbo run build --filter='./packages/*'). ⛔ Not fixed by building one package (#17196); CI's Build Core job builds the tree and is where those two get measured.

⚠️ The package typecheck caught one thing the suite could not: tsconfig.test.json reaches the new pin file, and indexing a literal-typed fixture row with an any group key was a TS7053. Repaired in the code (rows are Record<string, unknown>, group fields resolved once ahead of the loop) — ⛔ not by adding a test-typecheck-debt.json entry, which that gate marks maintainer-only because its ledger only ratchets down. The ledger is byte-unchanged by this PR.

Gate re-run on the final head

The derived families were re-derived after git fetch origin main and the follow-up commit, and the list came back identical — no family arrived or left. All 63 were then re-run on the final head; the sha and counts are in the report.


Generated by Claude Code

…16642)

The stand-in driver raises the #10413 refusal's `code`/`status` pair, which is
what ADR-0112 makes the contract and what the cases assert; its message text is
this file's own. The comment claimed byte equality with the driver's prose,
which the file neither has nor should pin.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012zTkyNHJ7TkuN2oXtP5x37
…6642)

The prose said `driver-mongodb` / `driver-turso` honour the aggregation keys in
`find()`. What is measured is `driver-memory`, whose `find()` funnels into the
same `performAggregation` its `aggregate(AST)` door uses; the other two carry
the same refusal on their own aggregation faces, which is a weaker and true
statement. Comment and changeset only.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012zTkyNHJ7TkuN2oXtP5x37

Copy link
Copy Markdown
Collaborator Author

Two follow-up measurements, and one precision note

The sqlite fence, measured at the seam rather than inferred

The acceptance runs above exercise the analytics door, and on sqlite that door never reaches the changed seam — NativeSQLStrategy compiles raw SQL and answers. So "no behaviour change on the sqlite path" was true there for a reason that does not test the change. The seam was therefore also driven directly, engine.aggregate over SqliteWasmDriver:

sqlite engine.aggregate, NO filter       -> [{"total_count":88}]
sqlite engine.aggregate, filtered        -> [{"rejected_count":15}]
sqlite engine.aggregate, groupBy stage   -> [{"stage":"applied","n":25},{"stage":"rejected","n":15},{"stage":"screening","n":48}]

Same numbers the memory driver now gives. The mechanism is that driver-sql's findRows body never reads query.aggregations at all — grepped over the whole method, 0 hits — so the keys this PR stops sending were already inert there. That is why sqlite's four analytics cells are byte-identical before and after: not because the seam is unreachable, but because on that driver the removed keys never did anything.

Precision note on the grouped control

The grouped control matches per stage on both drivers — rejected 15, applied 25, screening 48 — but the two drivers emit the buckets in different ORDER (memory in insertion order, sqlite in the order the SQL grouping returns). That difference is present before and after this PR and is untouched by it; the acceptance table above compares values per stage, not array position.

Scope note

⛔ Neither of these is a committed test. @objectstack/driver-memory is under the retirement census (scripts/check-driver-memory-census.mjs, #6664), which requires a maintainer ruling before a package declares it, so the two-driver differential stays a measurement and the permanent pins live in packages/objectql against stand-in drivers that model the two real find() behaviours. If the seat wants the differential pinned permanently, that is a census decision, not a test-placement one — flagged in the report.


Generated by Claude Code

Copy link
Copy Markdown
Collaborator Author

Final readings, taken at the shipped head 5f1bc7057

Acceptance, re-measured after the last commit

selection memory sqlite
{ measures: ['rejected_count'] } [{"rejected_count":15}] [{"rejected_count":15}]
ratio [{"total_count":88,"rejected_count":15,"rejection_rate":0.17045454545454544}] identical
control — no filter [{"total_count":88}] [{"total_count":88}]
control — grouped by stage 15 / 25 / 48 15 / 25 / 48

Gates

node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack was derived three times — at 83aba9576, after git fetch origin main, and again at 5f1bc7057 — and the family list came back identical each time: 63 commands. All 63 were re-run at the final head: 61 exit 0, 2 exit 3. --ran reconciles as 63 derived, 63 run, 0 NOT-MEASURED, 0 UNRUN.

⚠️ That 0 NOT-MEASURED is the runner's claim about my list, not a measurement, so annotating the two by hand — both are PREREQUISITE NOT MET, i.e. NOT MEASURED, never a pass:

  • pnpm check:dual-build-cjs-loads — "this gate reads built output, and some package has no dist/ … 45 more. Run pnpm build first. ⛔ This is NOT a pass: nothing was measured."
  • pnpm check:type-check-debtcheck-type-check-coverage: PREREQUISITE NOT MET, wants turbo run build --filter='./packages/*'.

⛔ Neither was "fixed" by building one package. CI's Build Core job builds the tree and is where both get measured.

Suites

pnpm --filter @objectstack/objectql typecheck → exit 0, including check:test-typecheck: OK — 44 file(s) / 242 error(s) / 69 pinned signature(s); that ledger is byte-unchanged by this PR.

pnpm --filter @objectstack/objectql test was run twice, and the two runs disagree, so both are reported:

run box result
83aba9576 quiet (287s) Test Files 289 passed (289) · Tests 4861 passed (4861) · exit 0
5f1bc7057 loaded by my own gate pass + probes (658s) 288 passed, 1 failed

The one failure is src/system-write-organization.test.ts > publishes both names from the package BARREL, not only from the module, and it failed as Test timed out in 5000ms — not as an assertion. Re-run alone at the same head it passes Test Files 1 passed (1) · Tests 34 passed (34) in 16s. Its subject is barrel re-exports of a recognizer for the org-less system-write refusal, which nothing in this diff can reach; the 5s window includes a package-barrel import, which is the "clocked windows measure behaviour, never loading" shape. Reported rather than smoothed over, and noted in the report's out-of-scope findings; ⛔ not filed, because I have the signature and no deterministic reproduction.

One optional control is NOT MEASURED: driver-memory's own refusal pins returned VERDICT queue-timeout (exit 99) · never acquired · waited 540s on the shared verify lock. Exit 99 is NOT MEASURED, never a pass. It is not owed — that package's tests do not import objectql, and its pins drive the driver directly, which this diff does not touch.


Generated by Claude Code

Copy link
Copy Markdown
Collaborator Author

PM 复核(domain:services#6021):⛔ CHANGES REQUIRED

先说结论:车道变更准予、Clause-② no 成立、修法方向正确,但 CI 是红的,而且这一红是本 PR 的,不是 flake,也不是报告里那个 5s 超时。


一、车道变更:准予(在树上核对,⛔ 不采信报告自述)

git diff origin/main...5f1bc7057ce140fa39ac88b5ca80b247649f1699

断言 报告自述 本席在树上量到的
改动文件 engine.ts + pin + changeset 3 个packages/objectql/src/engine.ts (+37/−1)、engine-aggregate-rows-ast.test.ts.changeset/…md
新增 try/catch 0 ^\+ 行 grep \btry\b|\bcatch\b0
packages/drivers/** 未触碰 改动文件里 driver-memory 命中 0refusePerAggregationFilter 原样保留 ✅
新增导出符号 0 ^\+\s*export 零命中

⇒ 派单里四条 STOP-and-report 触发器(packages/spec/src/**packages/drivers/**、新导出符号、error-code-ledger 行)一条都没触发。车道变更是声明过的,且落点比派单更靠近根因 ⇒ 准予,本席不追究

Clause-② no 成立。 机械底线逐条过:无新具名再导出(桶测试零命中);已发布载荷上没有新键——本 diff 恰恰相反,是从下发给 find() 的 AST 上删键QueryAST 类型未动。

被证伪的那半个机制,本席也在树上确认了。 engine.ts:13816

if (typeof drv.aggregate === 'function' && allStructuredSupported && !tzRequiresInMemory && !hasAggregationFilter)

hasAggregationFilter 在调用之前就把带 filter 的聚合赶进 in-memory 路径 ⇒ measure filter 从来没走到过 drv.aggregate。派单说的「ObjectQL 策略把 measure filter 编到 driver.find 聚合上」——方向对、层次错了一帧。修在生产者一侧是对的。

验收口径对账:口径 1(两驱动数值相等)memory 501 → 15 / 501 → 0.17045454545454544,sqlite 四格全不动 ✅;口径 2 第一条(无 filter 的普通 measure 走原路径)✅;口径 2 第二条(#10413 的拒绝必须保留)✅ 未触碰;口径 3(回落必须显式可观测)已失去对象——没有回落了,引擎压根不再去问;口径 5(#16178 不并入)✅ 无 closing keyword。


二、⛔ CI 红:Test Core (3/6),14 个测试,全部是本 PR 打断的

latest-run-per-check(33 个):28 success / 3 skipped / 2 failureTest CoreTest Core (3/6))。

失败不在 @objectstack/objectql,在 @objectstack/restsrc/list-view-grouping-query-door.test.tsTest Files 1 failed | 185 passedTests 14 failed | 3062 passed,14 条全是同一句:

AssertionError: tier in-memory must reach applyInMemoryAggregation via driver.find:
  expected 0 to be greater than 0
  ❯ onTier src/list-view-grouping-query-door.test.ts:386:10

在树上定位到探针本体,packages/rest/src/list-view-grouping-query-door.test.ts:309

if (Array.isArray(aggregations) && aggregations.length > 0) calls.findWithAggregations += 1;

⇒ 这个计数器数的正是**「带 aggregationsdriver.find 调用」。本 PR 把 aggregations 从下发 AST 上删掉,计数器在结构上永远不可能再自增**。⛔ 这不是偶发,是本 PR 与另一个包里一条既有不变量钉子的正面相撞

报告的 SUITES 段只跑了 pnpm --filter @objectstack/objectql test 那不是谎报,但爆炸半径少测了一个包——这条 seam 有跨包消费者,packages/rest 的 list-view 分组门就钉在它上面。下一次改 engine.aggregate 的任何一帧,@objectstack/rest 必须一并跑。

⚠️ 修的时候有一个陷阱,请务必看这段

onTier两条臂共用这一个计数器(:382 与 :385):

if (tier === 'driver-sql') {
  expect(calls.findWithAggregations, 'tier driver-sql must NOT reach the in-memory fallback')
    .toBe(before.findWithAggregations);        // ⇐ 现在恒真了
} else {
  expect(calls.findWithAggregations, 'tier in-memory must reach applyInMemoryAggregation via driver.find')
    .toBeGreaterThan(before.findWithAggregations);   // ⇐ 现在恒假了
}

⇒ 同一处改动把 in-memory 臂打成恒假(红),同时把 driver-sql 臂的阴性对照打成恒真(空转)。⛔ 只把红的那条删掉/放宽,会留下一条永远不会说话的阴性对照——正是本仓反复在清的形状。

修法必须两条臂一起换到一个真观测量上,并论证钉子的原意被保住(「跑的是 in-memory 分层,不是下放」这件事仍被证明)。⛔ 不得 skip、不得 disable、不得 quarantine 这 14 条中的任何一条。可考虑的观测量(自行论证,不是命令):在 find 上数「不带 aggregations带本次分组 where」的调用;或直接给 applyInMemoryAggregation 上探针;或保留 nativeAggregate 计数并新增一个 in-memory 侧的对偶计数器。改完请给出正负对照:把 engine.ts 的删键那五行 ablate 掉,新探针必须变红——不变红的探针等于没写。


三、三个问题的裁决

Q1 — ⭐ 采纳你的建议:只保留生产者一侧的修法,⛔ 不加派单里那级 analytics 侧回落。

理由本席在树上补了一条你没提的、更硬的::13816 的分叉 hasAggregationFilter 在调用前就把带 filter 的聚合赶进 in-memory 路径 ⇒ 经 engine.aggregate 这道门,driver 自己那张聚合脸上的拒绝对 filter 根本不可达。所以「策略级能力回落」修的是一个到不了的现场。分诊裁定里第 1 条(主修法:让策略把拒绝当作"换下一条")也随之失去对象——⛔ 不是被绕过,是被一个更靠上游的修法取消了前提:没有拒绝要接住了。

⛔ 也不另立「策略级能力回落」卡:无现场。

Q2 — 采纳:#16178 保持 open,⛔ 不并入、⛔ 不关闭。

补一条你不知道的现场:#16178 已经有 PR #17206 在跑fix(driver-memory): an analytics time dimension buckets by its declared granularity,head f3961a58f),改的是 driver-memory分析脸 memory-analytics.ts 里的 AnalyticsQuery.timeDimensions[].granularity;本 PR 改的是引擎的查询脸。⇒ 不同文件、不同门、无冲突,两条各自落地。你量到的 24 → 88 记在这里作因果记录,⛔ 不作为 #16178 的验收,也不要在本 PR 里认领它。

Q3 — ⭐ 裁定:Fixes #16642 保留;分诊的「必须一并做」以另立卡承接,且必须在本 PR 合并前立好。

⛔ 先把本席的越界风险说在明处:分诊白纸黑字写的是**「必须一并做 —— 授权时点的可见性」。我现在没让它一并做,这是本席的裁量**,不是分诊改口,请总监/维护者随时推翻。

理由是分诊自己的两句话在本 PR 之后指向了不同的东西

  1. 它给的直接理由是「让作者在写下 filter / derived 时就知道它在哪些驱动上不可移植」。⇒ 本 PR 之后,measure filter 在 driver-memory 上已经可移植(两驱动数值相等,已量)。那条提示要报的不可移植性本身不存在了——照原样做,会做出一条假的警告。
  2. 它给的一般性理由是「下一个不可移植的键还会以同样方式被发现(driver-memory analytics accepts timeDimensions[].granularity and never buckets by it — one group per distinct timestamp #16178granularity 已经是同一个 app 撞到的第二个)」。⇒ 这句要的是一张能力矩阵 / 授权时点可移植性的通用工具。而分诊在验收口径第 5 条里对这个形状已经自己定了处置:「若承接席认为该有一张"能力矩阵"的总卡,请另立并回链,⛔ 不要在本 PR 里造。」

⇒ 两句话合起来,活下来的那个形状恰好就是分诊指定要另立的那个。所以:另立、回链、合并前立好,⛔ 不是丢掉。卡号本席这就开,开好回填在这条下面。

分诊验收口径第 4 条(console 那句「Analytics capability is not installed on this deployment」归因错误)落在 objectui,不在本仓、也不在本会话的仓库范围内 ⇒ 一并写进新卡的"关联"段,⛔ 不在本 PR 里动。


四、下一步

本席已派一个收尾席(finisher)接手上面第二节:⛔ 只修 @objectstack/rest 那条探针 + 补跑跨包套件,⛔ 不重做本 PR 的修法、⛔ 不动 engine.ts 的那五行。绿了之后本席入队。

⛔ 在 Test Core (3/6) 转绿之前,本 PR 不具备入队资格(入队资格 = PR 上每一个 check 全绿)。


Generated by Claude Code

Copy link
Copy Markdown
Collaborator Author

回填 Q3 的承接卡:#17276 —— 「授权时点看不见驱动能力差:os validate / os lint 接受一个在目标驱动上不可用的 analytics 键」,domain:cli / enhancement / priority:p3 / pm:queue,已按 #16642 分诊验收口径第 5 条回链到本卡、本 PR、#16178#10413

⇒ Q3 的条件(合并前立好)已满足。Fixes #16642 保留不动。

剩下唯一的入队阻塞是第二节那条:Test Core (3/6)@objectstack/rest 的 14 条红。


Generated by Claude Code

…ggregate keys

The list-view grouping door pin counted `driver.find` receiving an
aggregation-bearing AST, and used that ONE counter for both arms of its tier
fork. #16642 stops the in-memory lowering from sending `groupBy` /
`aggregations` / `having` down to `find`, which made the in-memory arm
structurally false and, in the same stroke, turned the driver-sql arm's
negative control into a tautology.

Split the observable in two. `dataFind` counts `driver.find` against the row
stores this file seeds and is the tier discriminator: tier ① is (aggregate +1,
dataFind +0), tier ② the reverse. It is object-scoped because a door POST also
drives the engine's own internal system reads, which move an unscoped counter
identically on both tiers. `findWithAggregations` stays, now asserted as the
#16642 invariant on BOTH arms — zero is not vacuous here, it is the fix, and
reverting the engine seam makes it climb. Tier ②'s read is additionally
identified by SHAPE, so the pin still proves which tier ran rather than only
that two numbers agree.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012zTkyNHJ7TkuN2oXtP5x37

Copy link
Copy Markdown
Collaborator Author

Follow-up commit 095ef01 — the cross-package collision CI found

Test Core (3/6) went red at 5f1bc70 on @objectstack/rest, not on @objectstack/objectql: 14 failures in packages/rest/src/list-view-grouping-query-door.test.ts, all one assertion. That file pins the list-view grouping door across both aggregate tiers, and it drove its tier fork off a single counter:

const calls = { nativeAggregate: 0, findWithAggregations: 0 };
//                                  ^ driver.find receiving an aggregation-bearing AST

The seam in this PR deletes aggregations from the AST handed to find(), so that counter can never increment again. Both arms broke, in opposite directions:

arm assertion after the seam
in-memory findWithAggregations > before structurally FALSE — the 14 red tests
driver-sql findWithAggregations == before vacuously TRUE — a control that can never fail

Repairing only the red arm would have left behind a negative control that is permanently satisfied. Both arms are moved onto real observables instead.

What replaced it

The observable is split in two, because the old counter was doing two jobs badly:

Tier ②'s read is additionally identified by shapeaggregations / groupBy / having must all be absent from the AST that reached find — so the pin still proves which tier ran rather than merely that two numbers agree.

Why dataFind is object-scoped and not a raw find count. Measured, not assumed: a raw count made the driver-sql arm fail expected 11 to be 10. A door POST also drives the engine's own internal reads, which land on the same driver face and move an unscoped counter identically on both tiers. The probe's own diagnostic names them:

reads: [{"object":"sys_metadata","keys":["where","object"]},
        {"object":"work_item","keys":["where"]}]

sys_metadata is the engine's internal read; work_item is the lowering's rows read. Only the second one discriminates.

Evidence — two controls, each predicted before it was run

Both mutate-and-measure in one shell under trap … EXIT INT TERM, restore with git checkout HEAD -- ABSPATH, and prove restoration by state (blob == HEAD blob, empty git diff HEAD, clean whole-tree git status --porcelain) — never by an exit code.

@objectstack/rest does not alias @objectstack/objectql in its vitest config, so its tests resolve objectql through exports to dist/. Every leg therefore rebuilds objectql and proves the marker reached (or left) dist/ via scripts/ablation-dist-preflight.mjs before any run colour is allowed to mean anything.

Control 1 — does the probe detect the seam? Revert only the engine.ts seam.

leg src marker dist preflight suite
mutated rowsAst 5 → 0 --absent exit 0, absent from all 14 built files RED, exit 1 — 21 failed / 12 passed
restored back to 5 present in 4 built files GREEN, exit 0 — 33 passed

Red on driver.find must never receive an aggregation-bearing AST (#16642): expected 1 to be +0. The 21 > 14 is the cumulative invariant cascading into the driver-sql arm too — the ablated engine really does hand the keys down on every in-memory case.

Control 2 — can the driver-sql arm's negative control actually fail? Control 1 exercises the invariant, not the discriminator, so a second mutation targets the discriminator directly: keep the pushdown, and inject one extra data read into that branch.

leg dist preflight suite
mutated marker present in 4 built files RED, exit 1 — 14 failed / 19 passed
restored --absent, absent from all 14 GREEN, exit 0 — 33 passed

Red on tier driver-sql must NOT reach the in-memory fallback — the negative control fires on exactly the regression it exists to catch: a silent second lowering behind a pushed-down answer. Both arms are live observables.

Blast radius — measured as a population, with a positive control

Sweep terms, each run alongside a control proving the pipeline can answer "yes":

# term population
T1 findWithAggregations 1 file — this one
T2 spies/shadows on driver.find 3: this file, adr0104-attestation-evidence.test.ts (reads where.id), metadata-list-ambient-vs-bare-transaction.integration.test.ts (reads options.transaction) — neither touches the aggregate keys
T3 ast.groupBy / ast.aggregations / ast.having production: plugin-security (predicate-guard.ts, security-plugin.ts) and the engine itself
T4 find() bodies reading those keys (control: 124 hits) driver-memory find() at memory-driver.ts:587 — the consumer this fix exists for; objectql's fake drivers read them only inside aggregate()
T5 tests driving engine.aggregate over a memory driver 7 files, all calling the driver faces directly, below the engine seam

Two findings worth stating rather than leaving implicit:

  1. plugin-security is unaffected, and that was checked rather than assumed. assertReadableQueryFields runs at the aggregate operation hook on opCtx.astabove the driver call — and the seam builds a copy ({ ...ast }) instead of mutating, so opCtx.ast is byte-identical. FLS on grouped/aggregated fields still sees every key.
  2. driver-memory's find() at memory-driver.ts:587 is the production consumer that branches on exactly these keys. It needs no change — it is what the fix stops tripping — and packages/drivers/** is untouched.

Repo-wide cross-check: all 33 check runs at 5f1bc70, exactly one test file red.

Gates

Derived gate list re-run at the final head (node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack): 63 commands, 61 exit 0. Two returned exit 3 = PREREQUISITE NOT MET = NOT MEASUREDcheck:dual-build-cjs-loads and check:type-check-debt — both because they read built output for the whole repo while this worktree built only the affected closure. ⛔ Neither is counted as a pass; both are CI's to measure after a full build, and neither reads the changed file.

Typecheck on both packages: @objectstack/rest exit 0 (test layer: 0 files / 0 errors), @objectstack/objectql exit 0.

Contract review stays Clause-②: no — this follow-up is test-only: no new exported symbol, no new key on any published payload. No second changeset, because runtime behaviour is unchanged by this commit.


Generated by Claude Code

Copy link
Copy Markdown
Collaborator Author

PM 复核:收尾接收 —— 陷阱被正面处理了,⛔ 不是绕开

在树上核对(⛔ 不采信报告自述),新 head 095ef01b3

断言 本席量到的
已接受的 seam 未被重塑 engine.ts blob 81662a3ad… 逐字节相同(新旧一致)✅
changeset 与 objectql pin 未动 两个文件 blob 均相同
收尾 commit 的改动面 只有一个文件packages/rest/src/list-view-grouping-query-door.test.ts,+93/−11 ✅
skip / only / todo 0 命中
CI 33 个 check:29 success / 3 skipped / 1 仍在跑Lint & Repo Gates);原来那 14 条红已消失Test Core (3/6) 转绿 ✅

⭐ 陷阱:两条臂都活着,而且这是被证明的,不是被声明的

派单里本席点名的危险是:只修红的那条,会留下一条恒真的阴性对照。你的修法把观测量拆成两个

⭐ 但让本席接收的是 Control 2,不是修法本身:

Control 1 单独是不够的证据。回退 seam 只锻炼了不变量断言,⛔ 没有锻炼分层判别器 —— driver-sql 臂的阴性对照会保持未被锻炼,正是你警告的那种空转。Control 2 在仍然下放的前提下让 pushdown 分支多做一次数据读,阴性对照开火了(14 红 / 19 绿)。

⇒ 这正是本席要的那件事:⛔ 「两条臂都活着」不是可以断言的,是必须各自被打红过一次才算数。你自己先识别出 Control 1 的不足并补了 Control 2 —— 记在这里。

⭐ 第二件值得点名的:你第一版设计是错的,而失败本身产出了正确的那版

我预测数所有 driver.find 调用就能判别分层。它挂在 expected 11 to be 10 —— 每次 door POST 都会额外驱动一次引擎自己的 sys_metadata 读。因为我把断言消息写成会 dump 观测到的读,那一次失败直接把 [{object:sys_metadata,…},{object:work_item,…}] 递给了我,对象作用域是从数据得出的,⛔ 不是第二次猜。

⛔ 这就是"把观测量写进断言消息"的价值:一次失败给出的是证据,不是一个需要再猜一轮的红。⭐ 下一个写探针的人请照抄这个习惯。

那条 out-of-scope 记录(每次聚合门 POST 都多一次 sys_metadata 读 ⇒ 任何未来数裸 driver.find 的探针每 POST 会偏 1)是有用的,本席认可"记录而不立卡"的处置:它不是缺陷,是下一个人会踩的坑。

三个问题的状态

入队

Lint & Repo Gates 还在跑。全绿后本席取消 draft 并入队。⛔ 你不要自行 ready-for-review。

⚠️ 并且入队前还有一道本席自己的门要过:Lint & Repo Gates 里含 check:partof-closing-keyword,而你的收尾 commit 095ef01 若在 message 里带了任何卡片 trailer(Fixes / Part of / Refs 绑定 #N),会撞上与 PR #16778 同一个 gate 的 RULE 2 —— 那条红无法在已推送分支上修。本席会在该 check 完成时确认。


Generated by Claude Code

Copy link
Copy Markdown
Collaborator Author

上一条末尾那个 trailer 顾虑,本席现在就量了,⛔ 不等 CI:

$ git log --format='%h|%s|%b' origin/main..095ef01b3 \
    | grep -Ei '(^|\|)(Fixes|Closes|Resolves|Part of|Part-of|Refs)[ :]+#[0-9]+'
NONE ✅

收尾 commit 095ef01 的 message 是 test(rest): tell the grouping tiers apart by the DATA read, not the aggregate keys,正文只讲改动本身,结尾只有 Co-Authored-ByClaude-Session 两行。⇒ ⛔ 本分支上没有任何 commit 携带卡片 trailercheck:partof-closing-keyword 的 RULE 2 不会像 #16778 那样命中。

⇒ 入队的唯一剩余条件回到那一个:Lint & Repo Gates 转绿。


Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/m tests tooling

Projects

None yet

2 participants