Skip to content

fix(core,driver-memory,service-analytics): sum / avg add with one compensated sum on every face (#20544) - #20739

Merged
objectstack-fleet[bot] merged 5 commits into
mainfrom
claude/issue-20544-compensated-sum-family
Sep 30, 2026
Merged

objectstack-fleet[bot] merged 5 commits into
mainfrom
claude/issue-20544-compensated-sum-family

Conversation

@objectstack-fleet

Copy link
Copy Markdown
Contributor

Fixes #20544

Clause-②: yes

What changed

sum / avg now give the same double on every face the platform owns. One compensated fold does the adding everywhere.

  • Hoist. compensatedSum moves from @objectstack/objectql's rows path (in-memory-aggregation.ts, private, :317 at base d2820876f) to packages/core/src/utils/compensated-sum.ts. It is exported from the @objectstack/core root, beside bucketDateKey, which is the bucketDateKey precedent. in-memory-aggregation.ts imports it and no longer keeps a copy. The function body is byte-identical: it is SQLite's kahanBabuskaNeumaierStep plus the finalizers' overflow guard.
  • The three naive folds now call it:
    • packages/drivers/driver-memory/src/memory-driver.ts, computeAggregate's sum / avg arm. This is the door engine.aggregate's native path takes on driver-memory, and find() with aggregations uses it too.
    • packages/drivers/driver-memory/src/memory-analytics.ts, buildAggregator. A sum / avg measure is now one $group $accumulator, whose finalize calls compensatedSum. It replaces mingo's $sum / $avg. The aggregand expression (numericAggregandExpr, the boolean rule) is unchanged. So are the addend predicate (mingo's isNumber) and the empty answers (sum 0, avg null).
    • packages/services/service-analytics/src/preview-evaluator.ts, the sum arm and the avg arm. The operand lists are unchanged. The default: arm and the region PR docs(service-analytics): re-anchor the dead tracker citations to the commits that decided them #20729 re-anchored are not touched.
  • Pipeline dump. pipelineDumpReplacer renders a function by its name. Without that, JSON.stringify drops the accumulator's functions, and a sum measure and an avg measure would dump identically in result.sql.
  • ⛔ There is no wrapping of PostgreSQL or MySQL native accumulation, per aggregate sum / avg over 3+ fractional addends: SQLite native adds with compensation (0.1+0.2+0.3 = 0.6), every other face and the rows path naively (0.6000000000000001), so having $eq 0.6 keeps the group on SQLite native only #20489's ruling. count, min and max are untouched.

Why Clause-②: yes: the hoist adds one root export (compensatedSum) to @objectstack/core, which widens its published surface. The changeset .changeset/20544-compensated-sum-every-face.md is @objectstack/core minor, with patch for @objectstack/objectql, @objectstack/driver-memory and @objectstack/service-analytics.

Reproduction: before and after

Scratch script (not committed), run with tsx over the packages' sources and a rebuilt @objectstack/core. Setup: a driver-memory ObjectQL engine, one number column w, one group per fixture.

  • Native path: engine.aggregate with sum / avg. A spy counted 1 driver.aggregate call.
  • Rows path (the control): the same query plus a filtered sibling count. The spy counted 0 driver.aggregate calls.
  • Analytics face: MemoryAnalyticsService.query over a cube on the same table.
  • Draft preview: evaluateAnalyticsQueryOverRows over the same rows.

Before, at base d2820876f (sum / avg):

face 0.1, 0.2, 0.3 1e16, 1, -1e16 0.1, 0.2 (control) 1, 2, 3, 40, 500 (control)
rows path (control) 0.6 / 0.19999999999999998 1 / 0.3333333333333333 0.30000000000000004 / 0.15000000000000002 546 / 109.2
driver-memory native (engine.aggregate) 0.6000000000000001 / 0.20000000000000004 0 / 0 the same as rows the same as rows
driver-memory analytics face 0.6000000000000001 / 0.20000000000000004 0 / 0 the same the same
draft preview 0.6000000000000001 / 0.20000000000000004 0 / 0 the same the same

having { s: { $eq: 0.6 } } through engine.aggregate kept no group on the native path and kept card on the rows path.

After, at 37a825e0c with core rebuilt: all four faces answer the rows-path row in every column. having { s: { $eq: 0.6 } } keeps card on both paths.

The analytics face's measured order over the sum measure changed from cancel=0, two, card=0.6000000000000001, ints to two, card=0.6, cancel=1, ints.

Mechanism hypotheses: which held

  • H1 held. compensatedSum was private at in-memory-aggregation.ts:317, and bucketDateKey reaches the core root through export * from './utils/datetime.js' (index.ts:50). Core's exports map has only . and ./logger. So the helper cannot be shared without widening the published surface: a new subpath would widen it too, and a copy per package is what triage ruled out. The line Clause-②: yes / core minor stands.
  • H2 held, and the analytics-face route was measured before it was chosen (mingo 7.2.4, scratch probe):
    • A caller's Context cannot replace $sum. Context.from merges the built-ins first and addOps keeps an operator that is already there. A context whose own $sum returns 42 still answered 0.6000000000000001. A new operator name works ($mySum answered 42), but only in an Aggregator built with that context. This face builds two: the driver's public aggregate() and its own time-bucket half.
    • $accumulator is in the default operator set. ComputeOptions.init defaults scriptEnabled to true: the probe ran with default options, and with scriptEnabled: false it refused ($accumulator requires 'scriptEnabled' option to be true).
    • A post-group recompute is ruled out by the constraint numericAggregandExpr's header already records: it runs after $sort / $limit.
    • ⇒ $accumulator. The time-bucketed pipeline and order by the measure are pinned.
    • The other two folds were where H2 put them: memory-driver.ts:2022 and preview-evaluator.ts:514 / :550 at base.
  • H3 held. Every avg is the compensated sum divided by the count: 0.19999999999999998 on all four faces, which is the rows path's answer on the same values.
  • H4 held. count / min / max arms are untouched, and integers are unchanged on every face (pinned). Stated boundary: this holds while the running total stays within 2^53. Beyond it the compensated total is the exact one, as PR fix(objectql): the rows path adds sum / avg with compensated summation, as SQLite does #20543 recorded for the rows path (2^53, 1, 1 → 9007199254740994). That boundary now applies to driver-memory's faces and the preview too.

Tests

New pins. Each face gets the card's 0.1 + 0.2 + 0.3 fixture, the 1e16 cancellation, a two-addend control and an integers control. Each asserts the naive fold's answer beside the expected one, so a fixture that cannot tell the folds apart fails.

  • packages/core/src/utils/compensated-sum.test.ts: 6 cases, including the empty list and non-finite totals (Object.is against the naive answer).
  • packages/drivers/driver-memory/src/memory-compensated-sum.test.ts: 12 cases.
    • Data face: aggregate(AST), find(), the having reading, the addend rule, the empty group.
    • Analytics face: grouped, the time-bucketed split pipeline, order by the measure, the addend rule, the empty group, and the dump naming each measure's fold.
  • packages/services/service-analytics/src/__tests__/preview-compensated-sum.test.ts: 3 cases. It is a differential between the preview and the live face (NativeSQLStrategy's SQL on sql.js SQLite): two AnalyticsService instances that differ only in draftRowsResolver.
  • objectql's existing in-memory-aggregation-compensated-sum.test.ts is unchanged and now runs through the core export.

Suites and typecheck, at head b7e98273 (after merging origin/main f927864ea):

  • pnpm --filter typecheck over @objectstack/core, @objectstack/objectql, @objectstack/driver-memory and @objectstack/service-analytics: all four Done. core check:test-typecheck OK (4 files / 4 errors held). objectql check:test-typecheck OK (40 / 234 / 65 held).
  • tsc --listFiles counts the three new test files once each in their packages' programs.
  • Tests:
    • core: 58 files / 1542 tests passed;
    • driver-memory: 65 / 1470;
    • service-analytics: 138 / 3219;
    • objectql (--project local): 337 / 6687.
    • Before the merge, at e07690e1d, the counts were the same except objectql at 336 / 6679. Main added one objectql test file.
  • Declared to CI: objectql's and core's test:repo projects. Their files do not read this surface.

Reverse verification (one-time, from committed state e07690e1d)

  • Tool. scripts/ablation-replace.mjs in wrap mode on packages/core/src/utils/compensated-sum.ts. The anchor return Number.isFinite(c) ? s + c : s; became const ablation20544 = s; return ablation20544;, which is the naive running sum.
    • Anchor count went 1 → 0, and the blob went 30e811c70045 → c2886a45b2d8.
  • Build. pnpm --filter @objectstack/core build, then ablation-dist-preflight.mjs @objectstack/core ablation20544 found the marker in 2 built files. objectql's and service-analytics' suites resolve core through dist/; driver-memory's aliases core to src/.
  • Predicted before the run: core 2 red / 4 green, driver-memory 6 / 6, preview 2 / 1, objectql 6 / 5. Observed: the same in every package.
    • core: 2 failed / 4 passed of 6;
    • driver-memory: 6 failed / 6 passed of 12;
    • preview: 2 failed / 1 passed of 3;
    • objectql: 6 failed / 5 passed of 11.
    • The objectql red is also the proof that the rows path now runs the core export.
  • Restore leg.
    • The restore was proven: blob equal to HEAD, and git diff HEAD empty.
    • Then core was rebuilt. --absent found the marker absent from all 14 built files, and the tree was clean.
    • All four files were green again: 6 / 12 / 3 / 11.

Gates

  • node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack (no paths) at b7e98273 derived 67 commands. All 67 were run.
    • 65 exited 0 on the first run.
    • check:dual-build-cjs-loads and check:type-check-debt first answered PREREQUISITE NOT MET (exit 3, no dist/ for other packages). After turbo run build --filter='./packages/*' --filter='./packages/*/*' (71 / 71 tasks), both exited 0.
    • --ran with exit codes: 67 derived, 67 run, 0 NOT-MEASURED, 0 UNRUN (a derived zero).
  • check:driver-conformance, before (base d2820876f) and after (b7e98273): 50 covered cell(s), 0 in the DEBT ledger, 0 exempt both times. The dialect axis is unchanged.
  • pnpm --filter @objectstack/spec check:api-surface: public API surface + factory signatures unchanged.
    • That gate snapshots @objectstack/spec only, and spec is untouched.
    • No gate snapshots @objectstack/core's exports. Its one-export widening is declared by Clause-②: yes and the minor changeset.
  • Also green: check:adr-0087-registration (1 non-breaking changeset), check-changeset-no-major, check:empty-changeset, check:issue-citations (10 citations, all resolve), check:doc-authoring, check:nul-bytes, check:engine-double-contract, check:cross-package-test-inputs, check:test-source-alias and check:undeclared-dep-imports.

Lint: a declared narrowing, not a full run. pnpm exec eslint --no-inline-config --format json over the 9 touched source files, at b7e98273, gave 9 files linted, 0 errors and 0 warnings.

  • Population. It is read from eslint.config.mjs: files: ['**/*.{ts,tsx,mts,cts,js,jsx,mjs,cjs}'] and the packages/** blocks, minus NEVER_LINTED. All 9 .ts files are in it. The changeset .md matches no files glob.
  • Invariance. The config never enables type-aware linting: no parserOptions.project and no typed @typescript-eslint rules, as its own comment near QUERY_OPTIONS_TEST_GLOBS states. So this diff cannot move the verdict on any untouched file.
  • The full pnpm lint is CI's.

Acceptance notes

  • preview-evaluator.ts's default: arm (custom-SQL metric types) still adds with reduce. The file records it as the historical answer with no live standard to move towards, and no card names it. Not changed. Carrier: none.
  • driver-sql.ts's AGGREGATE_ACCUMULATION residual note still points at the rows path (in-memory-aggregation.ts, compensatedSum). That is still true, because the rows path calls it by that name. The note does not mention that the helper now lives in core, or that driver-memory's faces use it too. It is outside this card's file surface, so it was not edited. Carrier: none.
  • A third-party pipeline passed straight to InMemoryDriver.aggregate(object, pipeline) with its own $sum / $avg still gets mingo's plain loop. The platform's own producer of that arm (the analytics face) no longer emits them for sum / avg measures, and count keeps $sum: 1, whose integers are exact.
  • The PR docs(service-analytics): re-anchor the dead tracker citations to the commits that decided them #20729 region of preview-evaluator.ts (about :625) is untouched. That PR has landed on main, and this branch merged it.

Generated by Claude Code

…um / avg with the one compensated sum

`compensatedSum` moves from objectql's rows path to `@objectstack/core` as a
root export, as `bucketDateKey` did, and the three folds that still added
naively call it: driver-memory's native `sum` / `avg` arm, its analytics
face (a `$group` `$accumulator` in place of mingo's `$sum` / `$avg`), and
service-analytics' draft preview. objectql imports the helper instead of
keeping its own copy.

Claude-Session: https://claude.ai/code/session_01DEvba2nBuD4tWzfq8r8NFY
Co-authored-by: Claude <noreply@anthropic.com>
… avg on every face

The card's `0.1 + 0.2 + 0.3` fixture, the `1e16` cancellation, a two-addend
control and integers: on the core helper itself, on driver-memory's data face
(both doors) and analytics face (plain, time-bucketed, ordered, and its
pipeline dump), and on the draft preview as a differential against the live
face on a real SQLite.

Claude-Session: https://claude.ai/code/session_01DEvba2nBuD4tWzfq8r8NFY
Co-authored-by: Claude <noreply@anthropic.com>
…ers with valid identifiers

Claude-Session: https://claude.ai/code/session_01DEvba2nBuD4tWzfq8r8NFY
Co-authored-by: Claude <noreply@anthropic.com>
@github-actions github-actions Bot added size/l documentation Improvements or additions to documentation tests tooling labels Sep 30, 2026
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

10 anchor(s) derived from 4 changed package(s); no hand-written page names any of them. ⚠️ 1 changed file(s) yielded no anchor (packages/core/src/index.ts), so the pages documenting them are NOT COVERED by this run — this is not a clean bill of health for those files.

What this run could not see
  • 1 changed file(s) yielded no anchor (packages/core/src/index.ts) — pages documenting those are invisible to this run
  • 1 name(s) were too generic to anchor anything (single lowercase words)
  • the SDK route bridge reached 54 of 206 client-bound route-ledger rows — the other 152 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 152: 0 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 55 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 97 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.
  • a key NAME is not a key, so the hand re-read the line above prescribes can land on the wrong schema. The same spelling is authorable on one governed type and a [REMOVED] tombstone on another for each of active, aria, joins, objects, template, tools and version (censused on [finding] tools is a key on BOTH AgentSchema (tombstoned, dead) and SkillSchema (live, cloud-attested), so a name-based search attributes skill examples to the agent key — it produced a false stop-the-line alarm on PR #19059 #19093 over the liveness ledger's governed types, top-level keys); nothing in a search result distinguishes the two, so a grep hit on a LIVE example reads as evidence about the DEAD key. Measured on fix(spec): the agent.tools liveness row says dead — it claimed live on a key the schema tombstoned #19059: content/docs/ai/agents.mdx was reported as contradicting the agent.tools tombstone over its tools: example at :161, which is inside the defineSkill({ block opened at :155 — the page was already correct. Settle ownership by PARSING the value against both schemas, never by the name: that literal PASSES SkillSchema, and as an AgentSchema it FAILS at tools with the tombstone prescription. ⛔ These names are not the whole class — a key retired through a .strict() guidance map leaves no tombstone in the walked shape and none of them here (tool.category, live as AIToolDefinition.category).

Coarse fallback — 41 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 fbec216e2d184afc03b456b0bd8013ad5d47bc6f → packageMentionDocs.

Which tree this was computed on

This run read content/docs from af50d27a14e153e88f8af7d9c16ee69550a2c5a1 — the merge of head b7e98273898d2247d48b36ee976137573f7a20b4 into base fbec216e2d184afc03b456b0bd8013ad5d47bc6f, 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 af50d27a14e153e88f8af7d9c16ee69550a2c5a1 && git checkout af50d27a14e153e88f8af7d9c16ee69550a2c5a1
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin fbec216e2d184afc03b456b0bd8013ad5d47bc6f b7e98273898d2247d48b36ee976137573f7a20b4 && git checkout -B drift-repro fbec216e2d184afc03b456b0bd8013ad5d47bc6f && git merge --no-ff b7e98273898d2247d48b36ee976137573f7a20b4

node scripts/docs-audit/affected-docs.mjs --json fbec216e2d184afc03b456b0bd8013ad5d47bc6f

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

@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

Contract review

Served-tier: CONTRACT_REVIEW_TIER
Head-sha: b7e98273898d2247d48b36ee976137573f7a20b4
Local-runs: none

Inputs: card #20544 (body; comments 5882237833 triage, 5900737084 claim, 5901507878 os-dev-report), PR #20739 (body, its 10-file list, the net diff origin/main...head, merge-base f927864ea), and the head's check-runs. The head is still b7e98273898d2247d48b36ee976137573f7a20b4: a merge of the dev's series (37a825e0c → 7b2740bee) with origin/main f927864ea. The accumulator claims were judged against mingo 7.2.4's published source (the version pnpm-lock.yaml pins), read from the registry tarball in scratch; nothing was built, run or re-run.

Check-runs on the head as read: 31 runs — 12 success, 3 skipped (Console Pin Gate, Build Docs, Packed-tarball smoke), 16 in_progress, 0 failure. Of the seven required contexts: Governed Surface Queue Guard success; Lint & Repo Gates, Build Core, Test Core (6 shards), Dogfood Regression Gate (3 shards) and Temporal Conformance (live PG + MySQL) in_progress; the type-check family split as Type Check · source gates success and consumer gates / debt ledger / workspace in_progress. Check Changeset, Check PR Size and the three claim guards are success. Landing still waits on the in-progress required set; this record judges the contract, not the gates.

① Derived judgments

The hoist is byte-identical — right. The nine-line body of compensatedSum at the merge-base (packages/objectql/src/in-memory-aggregation.ts:317, private) and the body of packages/core/src/utils/compensated-sum.ts hash equal once the export keyword is stripped (git hash-object 0f550c968a on both). objectql now imports it from @objectstack/core (bucketDateKey, compensatedSum) and its two call sites (sum, avg) are unchanged, so the rows path's answers cannot move; its existing pin in-memory-aggregation-compensated-sum.test.ts is untouched by the diff and, since objectql's vitest config carries no core alias, now runs through core's built export.

The new export as a contract — right. Name compensatedSum, signature (nums: readonly number[]): number, one exported symbol in the file, reachable through export * from './utils/compensated-sum.js' at the core root beside bucketDateKey (index.ts:50), no new subpath in core's exports map (. and ./logger only). Documented behaviour checked against the code: addition in input order with the naive running sum s and a compensation term c; an empty list is 0; two addends equal the naive a + b (the error term is 0 exactly); integers whose partial sums stay within 2^53 are unchanged because every addition is exact; a non-finite total returns s, which is exactly the naive running sum — c becomes non-finite if and only if s does (a non-finite t or a non-finite operand makes both), so Object.is(compensatedSum(x), naiveSum(x)) holds for every fixture the core test lists. Above 2^53 the compensated total is the exact one (2^53, 1, 1 → 9007199254740994), as the PR body and the rows-path pin state. Which values count as addends is left to the caller, and the doc says so.

Fold 1, driver-memory data face (computeAggregate) — right. Only the naive reduce over nums (the arrow-function fold with seed 0) became compensatedSum(nums). Addends: booleans → 1 / 0 (the #11065 rule), then typeof v === 'number' — nulls, undefined and strings dropped, a stored NaN kept — unchanged from base. avg divides by nums.length, empty → null, sum empty → 0: unchanged. Both doors reach it: aggregate(object, AST) → performAggregation and find() with groupBy/aggregations → performAggregation. count, min, max arms untouched.

Fold 2, driver-memory analytics face (buildAggregator) — right, and the scriptEnabled question is answered. sum / avg measures are now { $accumulator: { init, accumulateArgs: [numericAggregandExpr(path)], accumulate, finalize, lang: 'js' } }. Against mingo 7.2.4's source: $group evaluates each non-_id key per partition; computeExpression sees the single $accumulator key and dispatches straight to the accumulator operator with the spec object passed raw (function values untouched); $accumulator asserts options.scriptEnabled, evaluates accumulateArgs per row through $push in partition order (the same order mingo's own $sum / $avg add in), coalesces a missing value to null, calls init afresh per group, folds with accumulate, then finalize. ComputeOptions.init defaults scriptEnabled: true; the root mingo entry registers $accumulator in its default accumulator set; Context.from(CONTEXT, user) merges built-ins first and addOps keeps an existing operator, so the doc comment's claim that a caller's context cannot replace $sum is true. Both Aggregators that run this face's pipeline — InMemoryDriver.aggregate(object, pipeline) (memory-driver.ts:1309) and the time-bucket half (memory-analytics.ts:1443) — are constructed with no options. git grep scriptEnabled on the head over packages, examples, scripts, apps finds only the new doc comment: no in-repo construction of MemoryAnalyticsService, Aggregator or Query sets scriptEnabled: false (all 28 new MemoryAnalyticsService({ driver, cubes }) sites are test files passing no mingo options), and memory-driver.ts and memory-analytics.ts are the only two mingo consumers in the repo. Addends: numericAggregandExpr unchanged (bool → 1 / 0), then collectAddend's typeof value === 'number' && !Number.isNaN(value), which is mingo's isNumber (!Number.isNaN(v) && typeof v === "number") — null, missing key and non-numeric strings ignored as before. avg divisor addends.length, empty → null; sum empty → 0: what mingo's $sum / $avg answered. lang: 'js' is never read by mingo 7.2.4 — inert, harmless. Under $sort / $limit: the pipeline order is $match → $group → $project (measure: $measure) → $sort → $skip / $limit, so the finalized number is what later stages sort and cut; the time-bucket split cuts at $group and runs $group onward in one Aggregator, so the accumulator runs there identically (pinned: grouped, bucketed by day, ordered by the measure). count ($sum: 1), min, max, count_distinct arms untouched.

The pipeline dump's function arm — right. pipelineDumpReplacer now renders a function as [function NAME]; without it JSON.stringify silently drops the three functions and a sum and an avg measure dump identically in result.sql (a debugging echo, per its own comment). Pinned. The root tsup.config.ts does not minify, so the names survive the build. The dump's shape for sum / avg measures changes (an $accumulator spec instead of {"$sum": ...}); it is a debug string, not a contract.

Fold 3, service-analytics draft preview — right. sum: compensatedSum(nums) over the arm's existing nums (Number(r[field]) filtered finite — a null reads as 0, an exact no-op under either fold; booleans 1 / 0; non-numeric strings dropped). avg: compensatedSum(operands) / operands.length over the #16219 operand list (non-null rows, finite), the 0 / emptyGroupValueFor tail unchanged. The default: arm, count, count_distinct, min, max untouched. Pinned as a differential against NativeSQLStrategy on sql.js SQLite (declared devDependency), two AnalyticsService instances differing only in draftRowsResolver.

PostgreSQL and MySQL untouched — right. No file under packages/drivers/driver-sql or any SQL driver is in the diff; AGGREGATE_ACCUMULATION is unchanged. Triage's ⛔ is honoured.

Pins per face — right. Each face carries the 0.1 + 0.2 + 0.3 fixture, the 1e16, 1, -1e16 cancellation, the two-addend control and the integer control, each asserting the naive fold's answer beside the expected one so the fixtures discriminate: core 6 cases, driver-memory 12 (5 data face incl. find(), the having reading, the addend rule, the empty group; 6 analytics face incl. the split pipeline, order by the measure, the addend rule, the empty group, the dump; 1 discriminator), preview 3.

Shipped prose, sentence by sentence — true. Changeset: SQLite 3.43+ uses Kahan-Babuska-Neumaier for sum / avg — true; the move from objectql's rows path, which now imports it — true; the three folds named — true; 0.6000000000000001 / 0.20000000000000004 → 0.6 / 0.19999999999999998 and 0 → 1 — follow from the function; having { s: { $eq: 0.6 } } now kept on both paths — follows (the native path answers the same double the rows path did); two addends, integers within 2^53, a non-finite total unchanged — proved above; addend rules unchanged — true (the preview's sum reads a null as a 0 operand rather than leaving it out, which is answer-identical, and the sentence's claim is that nothing changed); count / min / max untouched — true; the dump renders functions by name — true; the residual (PostgreSQL / MySQL add natively without compensation, the platform does not wrap it, three or more fractions can differ in the last place, compare with a range) — true and consistent with sql-driver.ts's own AGGREGATE_ACCUMULATION note (0.6000000000000001 on PG / MySQL). Core doc comment: every claim checked above; the "Why it lives here" paragraph is true (neither driver-memory nor service-analytics depends on objectql; all three depend on core). memory-analytics doc comment: the three-route argument is true against mingo's source (post-group recompute runs after $sort / $limit; a context cannot override $sum; $accumulator is default-registered and default-enabled). PR body: H1–H4 hold as stated; the base line numbers (:317, :2022, :514 / :550) match the merge-base; core's exports map is . and ./logger; objectql and service-analytics resolve core through dist/ and driver-memory aliases it to src/ — true per the three vitest configs; the measured order change (two, card=0.6, cancel=1, ints) is what the code implies. Commit trailers use the model-free pair; no model identifier in the PR body, changeset or code comments.

② Semver level

.changeset/20544-compensated-sum-every-face.md: @objectstack/core: minor, @objectstack/objectql: patch, @objectstack/driver-memory: patch, @objectstack/service-analytics: patch; body carries Clause-②: yes with no arm, matching the PR body's Clause-②: yes. Judged right: the one public-surface change the diff publishes is the new root export compensatedSum on @objectstack/core — a widening, at least minor under the Clause-② rule, and minor is what is declared. objectql publishes no surface change (the function was private; its two call sites are unchanged) and now consumes core's export — patch is right, and workspace:* pins it to the core that carries the symbol. driver-memory and service-analytics change answers in the last place and the driver's debug dump text — bug fixes, patch is right. Nothing else widens or narrows: no spec path is touched (check:api-surface, spec-only, reports unchanged), no export is removed or renamed, no authorable key moves, no error code or refusal text changes. Not breaking, so no ADR-0087 marker is owed. Clause-②: yes and @objectstack/core: minor stand.

③ Boundary flags

  • origin/main merge commit on the branch — answered, acceptable. b7e98273 is a true merge (parents 7b2740bee, f927864ea); the main side touches none of the PR's 10 files (first-parent diff over those paths is empty), the net diff equals the two-dot diff from the merge-base (10 files, +624 / -46), and AGENTS.md §10 asks for exactly this pull before opening a PR. No generated-artifact conflict was possible on the PR's paths.
  • The pipeline dump's function arm — answered, right (see ①): same file as the claimed fold, a direct consequence of the accumulator, pinned, names preserved by the unminified build.
  • The committed pin at driver.aggregate(AST) rather than engine.aggregate — answered, acceptable. objectql and driver-memory declare no dependency on each other, and packages/rest (which hosts the aggregate sum / avg over 3+ fractional addends: SQLite native adds with compensation (0.1+0.2+0.3 = 0.6), every other face and the rows path naively (0.6000000000000001), so having $eq 0.6 keeps the group on SQLite native only #20489 engine-level pin) depends on driver-sql, not driver-memory, so no existing package hosts an engine.aggregate-over-InMemoryDriver differential without a new dependency. The native path's door is driver.aggregate(AST) → performAggregation → computeAggregate, now pinned; the rows path stays pinned in objectql against the same literals; the dispatch between them is objectql's and is not touched by this diff. The card asked for pins per face, which this satisfies.
  • check:api-surface is spec-only; no gate snapshots core's exports — answered, true: api-surface/ exists only under packages/spec, and @objectstack/core has no surface snapshot. The widening is therefore declared by Clause-②: yes plus the minor changeset and judged here: the diff adds exactly one exported symbol to core (compensatedSum), and none elsewhere (the four new memory-analytics functions are module-private). A pre-existing repo gap, not this PR's to close.
  • Out-of-scope note, carrier none: the preview's default: arm — answered, acceptable. It sums with reduce for custom-SQL metric types that no producer names as sum; the file itself records it as the historical answer with no live standard, and the card names only the sum / avg arms. Not a reproducible defect against a contract; a note is the right carrier.
  • Out-of-scope note, carrier none: sql-driver.ts's AGGREGATE_ACCUMULATION comment — answered, acceptable. Its residual paragraph still says "the engine's rows path (in-memory-aggregation.ts, compensatedSum) ... those two"; after this PR the compensated set also includes driver-memory's two faces and the preview, and the helper lives in core. Comment drift outside the claimed file surface, factual as far as it goes; a note is the right carrier.
  • Out-of-scope note, carrier none: a third-party pipeline passed to InMemoryDriver.aggregate(object, pipeline) — answered, acceptable. That arm is a mingo-dialect passthrough; a caller's own $sum keeps mingo's plain loop. The platform's only producer of that arm (this analytics face) no longer emits $sum / $avg for sum / avg measures, and count's $sum: 1 is exact. Not a platform fold.
  • open_questions — empty in the os-dev-report; nothing to escalate.

Residuals stated, none blocking: result.sql's text for sum / avg measures changes shape (debug echo); the data face still counts a stored NaN as an addend while the analytics face excludes it (unchanged on both, outside this card).

Implemented-by: claude/issue-20544-compensated-sum-family
Reviewed-by: session_01DEvba2nBuD4tWzfq8r8NFY

VERDICT: PASS


Generated by Claude Code

@github-actions

Copy link
Copy Markdown
Contributor

⛔ merge queue 构建失败 — 先分诊,再决定要不要重排

队列构建 36651106552 红了。队列跑的是全量套件(PR 侧 CI 只跑 affected 子集),
所以失败的测试可能在本 PR 没碰过的包里 —— 那不是重排能修的。每次盲目重排都会让排在后面的所有 PR 重建一轮。

失败的 job(日志抽取,best effort):

  • Console Pin Gate — 失败步骤: Build the Console SPA at the pinned objectui SHA

    ✗ Neither spec appears in the built console — no @objectstack/spec
    

↳ 失败原因 是判读的关键:超时(Test timed out in … / Hook timed out in …)多半是负载/时序,不是本 PR 的回归;
断言(AssertionError: …)才指向真实的行为改变。两者的 FAIL 行长得一模一样,只有这一行能区分。

⚠️ 断言这一侧有一类例外,判据是断言在测什么,不是它是不是 AssertionError。 断言的对象是产品行为(一个值、一个形状、一次拒收)⇒ 照上面读:真实的行为改变,去查,⛔ 不要重排掉;
断言的对象是这次实验自身的有效性前提(跑完的耗时、负载下的先后、任何只在时间预算内才成立的条件)⇒ 它跟超时是同一类,同样对负载敏感,重排一次是合法的判别手段。
识别是机械的:断言的消息或它比较的值本身点名了一段时长、一个时间戳、一个耗时计数。实测过的一对 —— AssertionError: SecurityPlugin.init() ran: expected false to be true 测的是产品行为(真回归);
AssertionError: this run took over a second, so second-precision stamps could have differed too: expected 1006 to be less than 1000 测的是实验前提:它守护的那条不变式当时是绿的,同一个 head 原样重排一次即成功。
穿着 AssertionError 外衣的时间测量,仍然是时间测量。(⛔ 这只改「怎么读一次红」,不改「哪些测试可以重排」——后者由别处管。)

跨 PR 相同签名(24h,按失败测试文件聚合):

  • ⚠️ 本次没有可用的聚合签名(日志里没有能解析出测试文件名的 FAIL 行)—— 这不是「没有同签名的其他 PR」,是这一轮没测到。跨 PR 聚合本次不可用,请手工比对其他 PR 的同类评论。
  • ⚠️ 24h 评论账本没读完(超过 5 页仍未读到窗口尽头),所以上面的「不同 PR 数」是下界,不是全量。

历史信号:

  • 本 PR 过去 24h 无队列失败记录(首次)。
  • 过去 24h 队列共有 5 个失败构建(不含本次)。

分诊清单:

  1. 失败测试在本 PR 改动的包里 → 真回归,修 PR。
  2. 失败测试与本 PR 无关 → 看上面的「跨 PR 相同签名」;已有汇总 issue ⇒ flaky/环境问题实锤,去那张 issue 上谈,修好前重排只会再烧一轮全队列。
  3. 两者都不是 → 可能与同组 PR 语义冲突;等前面的 PR 落地或失败出队后再重排一次即可,不要连续重排。

Generated by Claude Code · merge-queue-triage workflow (#4859)

Merged via the queue into main with commit b785c3b Sep 30, 2026
36 checks passed
@objectstack-fleet
objectstack-fleet Bot deleted the claude/issue-20544-compensated-sum-family branch September 30, 2026 00:57
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/l tests tooling

Projects

None yet

2 participants