Commit b785c3b
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 #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
#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 #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 #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](https://claude.ai/code/session_01DEvba2nBuD4tWzfq8r8NFY)_
---------
Co-authored-by: Claude <noreply@anthropic.com>
1 parent 03cdb9a commit b785c3b
10 files changed
Lines changed: 624 additions & 46 deletions
File tree
- .changeset
- packages
- core/src
- utils
- drivers/driver-memory/src
- objectql/src
- services/service-analytics/src
- __tests__
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
49 | 49 | | |
50 | 50 | | |
51 | 51 | | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
52 | 60 | | |
53 | 61 | | |
54 | 62 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
28 | 28 | | |
29 | 29 | | |
30 | 30 | | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
31 | 35 | | |
32 | 36 | | |
33 | 37 | | |
| |||
647 | 651 | | |
648 | 652 | | |
649 | 653 | | |
| 654 | + | |
| 655 | + | |
| 656 | + | |
| 657 | + | |
| 658 | + | |
| 659 | + | |
| 660 | + | |
| 661 | + | |
| 662 | + | |
| 663 | + | |
| 664 | + | |
| 665 | + | |
| 666 | + | |
| 667 | + | |
| 668 | + | |
| 669 | + | |
| 670 | + | |
| 671 | + | |
| 672 | + | |
| 673 | + | |
| 674 | + | |
| 675 | + | |
| 676 | + | |
| 677 | + | |
| 678 | + | |
| 679 | + | |
| 680 | + | |
| 681 | + | |
| 682 | + | |
| 683 | + | |
| 684 | + | |
| 685 | + | |
| 686 | + | |
| 687 | + | |
| 688 | + | |
| 689 | + | |
| 690 | + | |
| 691 | + | |
| 692 | + | |
| 693 | + | |
| 694 | + | |
| 695 | + | |
| 696 | + | |
| 697 | + | |
| 698 | + | |
| 699 | + | |
| 700 | + | |
| 701 | + | |
| 702 | + | |
| 703 | + | |
| 704 | + | |
| 705 | + | |
| 706 | + | |
| 707 | + | |
| 708 | + | |
| 709 | + | |
| 710 | + | |
| 711 | + | |
| 712 | + | |
| 713 | + | |
| 714 | + | |
| 715 | + | |
| 716 | + | |
| 717 | + | |
| 718 | + | |
| 719 | + | |
| 720 | + | |
| 721 | + | |
| 722 | + | |
| 723 | + | |
| 724 | + | |
| 725 | + | |
| 726 | + | |
| 727 | + | |
650 | 728 | | |
651 | 729 | | |
652 | 730 | | |
| |||
700 | 778 | | |
701 | 779 | | |
702 | 780 | | |
703 | | - | |
| 781 | + | |
| 782 | + | |
| 783 | + | |
| 784 | + | |
| 785 | + | |
| 786 | + | |
| 787 | + | |
704 | 788 | | |
705 | 789 | | |
706 | 790 | | |
| |||
1728 | 1812 | | |
1729 | 1813 | | |
1730 | 1814 | | |
| 1815 | + | |
| 1816 | + | |
1731 | 1817 | | |
1732 | | - | |
| 1818 | + | |
1733 | 1819 | | |
1734 | | - | |
| 1820 | + | |
1735 | 1821 | | |
1736 | 1822 | | |
1737 | 1823 | | |
| |||
0 commit comments