Skip to content

Commit 680e8e8

Browse files
os-zhuangclaude
andauthored
feat(verify): checkDateBucketParity — pin the seam between pushed-down and in-memory date bucketing (#3813)
A driver that advertises `supports.queryDateGranularity[g]` is telling `engine.aggregate` it may push that granularity down as SQL instead of fetching rows and bucketing them in JS. The two are then not two features but one feature with two implementations, and the engine picks between them per query — a granularity the driver advertises goes down as SQL, one it does not goes to `applyInMemoryAggregation`, and a non-UTC timezone forces the in-memory path regardless. A dashboard can cross that seam mid-drill-down. Nothing checked that they agree. That is how #3773 shipped: every gate was green while a trend chart collapsed into one bar, because the driver's own bucket suites build fixtures with `knex.schema.createTable` + `t.string(...)` — ISO TEXT, the half `strftime` parses natively — and the engine never second-guesses a granularity a driver claims to support. `checkDateBucketParity(driver)` compares the driver's pushed-down result against the REAL `applyInMemoryAggregation` over the driver's own `find()` rows, for every granularity it advertises, across both temporal storage forms under one object (`Field.datetime` and `Field.date` naming the same calendar days) so a storage-form leak surfaces as the two columns bucketing differently. A granularity the driver does not advertise is skipped, never faulted. Follows `checkReadCoercion`: human-readable problems (empty = conformant), no test-runner dependency, driver taken structurally — so cloud's driver-turso, remote SQLite with exactly the epoch storage that broke here, can run the identical contract against itself. Verified against the real regression, not just fakes: reverting the #3773 fix turns the gate red on BOTH drivers with a diagnostic naming the collapsed bucket (`null: sql=7 in-memory=—`), and the cross-column pass fires alongside it while the TEXT-stored date column correctly stays quiet. Four negative controls pin what the checker can detect, including a driver that advertises a granularity it then throws on, and one that advertises nothing (must stay silent). The three hand copies of `bucketDateValue` now state what their `⚠️ Keep in sync` comments cannot enforce — a copy that stops tracking its original leaves the copy and the SQL agreeing with each other while both are wrong — and point at the executable check. The same pointer is on `bucketDateValue` itself, where an edit would start the drift. Co-authored-by: Jack Zhuang <277994282+os-zhuang@users.noreply.github.com> Co-authored-by: Claude <noreply@anthropic.com>
1 parent 2343099 commit 680e8e8

10 files changed

Lines changed: 458 additions & 3 deletions

File tree

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
---
2+
"@objectstack/verify": minor
3+
---
4+
5+
feat(verify): `checkDateBucketParity` — pin the seam between pushed-down and in-memory date bucketing
6+
7+
A driver that advertises `supports.queryDateGranularity[g]` is telling
8+
`engine.aggregate` it may push `dateGranularity: g` down as SQL instead of
9+
fetching rows and bucketing them in JS. The two are then not two features but
10+
one feature with two implementations, and the engine picks between them per
11+
query — a granularity the driver advertises goes down as SQL, one it does not
12+
goes to `applyInMemoryAggregation`, and a non-UTC timezone forces the in-memory
13+
path regardless. A dashboard can cross that seam mid-drill-down.
14+
15+
Nothing checked that they agree. That is how #3773 shipped: SQLite stores a
16+
`Field.datetime` as INTEGER epoch milliseconds, `strftime` read the bare integer
17+
as a Julian day number, and every row bucketed as NULL — a trend chart collapsed
18+
into a single bar while every gate stayed green. The driver's own bucket suites
19+
build their fixtures with `knex.schema.createTable` + `t.string(...)`, which is
20+
ISO TEXT — the half `strftime` parses natively — and the engine never
21+
second-guesses a granularity a driver claims to support.
22+
23+
`checkDateBucketParity(driver)` rounds a fixture through the driver and, for
24+
every granularity it advertises, compares its pushed-down result against the
25+
REAL `applyInMemoryAggregation` over the driver's own `find()` rows. Both
26+
temporal storage forms are probed under one object (`Field.datetime` and
27+
`Field.date` naming the same calendar days), so a storage-form leak shows up as
28+
the two columns bucketing differently even when each is internally consistent.
29+
A granularity the driver does not advertise is skipped, never faulted.
30+
31+
It follows `checkReadCoercion`: human-readable problems (empty = conformant), no
32+
test-runner dependency, driver taken structurally — so an out-of-tree driver
33+
runs the identical contract against itself. That matters most for cloud's
34+
`driver-turso`, which is remote SQLite with exactly the epoch storage that broke
35+
here.
36+
37+
Wired up in `packages/qa/dogfood/test/date-bucket-parity-conformance.test.ts`
38+
against driver-sql and driver-sqlite-wasm, with negative controls that pin what
39+
the checker can detect. Verified against the real regression, not just fakes:
40+
reverting the #3773 fix turns the gate red on both drivers with a diagnostic
41+
naming the collapsed bucket.
42+
43+
The three test files that hand-copy `bucketDateValue` (driver-sql cannot depend
44+
on objectql) now say what their `⚠️ Keep in sync` comments cannot enforce — a
45+
copy that stops tracking its original leaves the copy and the SQL agreeing with
46+
each other while both are wrong — and point at the executable check. The same
47+
pointer is on `bucketDateValue` itself, which is where an edit would start the
48+
drift.

packages/objectql/src/in-memory-aggregation.ts

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -175,6 +175,20 @@ function toNumber(v: any): number {
175175
* Bucket a date-like value into an ISO-formatted period label. Weeks start
176176
* Monday and use ISO week numbering.
177177
*
178+
* ⚠️ **This is one of two implementations of the same contract.** A driver that
179+
* advertises `supports.queryDateGranularity[g]` buckets that granularity in SQL
180+
* instead, and `engine.aggregate` picks between them per query — so a label
181+
* produced here must equal the label that driver's SQL produces for the same
182+
* instant, or a drill-down breaks when it crosses the seam. Editing the labels
183+
* below means editing every driver's bucket expression too.
184+
*
185+
* The seam is enforced by `checkDateBucketParity` (@objectstack/verify), run
186+
* against the real drivers in
187+
* `packages/qa/dogfood/test/date-bucket-parity-conformance.test.ts`. Three
188+
* driver test files also hand-copy this function for self-containment; those
189+
* copies cannot detect their own drift, which is why the executable check
190+
* exists.
191+
*
178192
* `timezone` (ADR-0053 Phase 2) resolves the calendar day in a reference zone
179193
* so an instant near a tz day-boundary buckets where a user in that zone would
180194
* expect. An unset / `'UTC'` / invalid zone keeps the historical UTC bucketing.

packages/plugins/driver-sql/src/sql-driver-date-bucket-storage.test.ts

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,16 @@ type Granularity = 'day' | 'month' | 'quarter' | 'year';
2727
/** Every granularity SQLite advertises natively (week is bucketed in-memory). */
2828
const GRANULARITIES: Granularity[] = ['day', 'month', 'quarter', 'year'];
2929

30-
/** ⚠️ Keep in sync with `packages/objectql/src/in-memory-aggregation.ts#bucketDateValue` */
30+
/**
31+
* ⚠️ Keep in sync with `packages/objectql/src/in-memory-aggregation.ts#bucketDateValue`.
32+
*
33+
* This copy exists because driver-sql cannot depend on objectql. Nothing here
34+
* can detect it drifting from its original — a stale copy and the SQL agree
35+
* with each other while both are wrong. The executable check is
36+
* `checkDateBucketParity` (@objectstack/verify), run against the real drivers
37+
* in `packages/qa/dogfood/test/date-bucket-parity-conformance.test.ts`, where
38+
* the reference side is the REAL `applyInMemoryAggregation`.
39+
*/
3140
function bucketDateValue(value: unknown, g: Granularity): string {
3241
if (value == null) return '(null)';
3342
// A finite number is epoch milliseconds — SQLite's `Field.datetime` storage.

packages/plugins/driver-sql/src/sql-driver-date-bucket.test.ts

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,16 @@ import { SqlDriver } from '../src/index.js';
1717

1818
type Granularity = 'day' | 'week' | 'month' | 'quarter' | 'year';
1919

20-
/** ⚠️ Keep in sync with `packages/objectql/src/in-memory-aggregation.ts#bucketDateValue` */
20+
/**
21+
* ⚠️ Keep in sync with `packages/objectql/src/in-memory-aggregation.ts#bucketDateValue`.
22+
*
23+
* This copy exists because driver-sql cannot depend on objectql. Nothing here
24+
* can detect it drifting from its original — a stale copy and the SQL agree
25+
* with each other while both are wrong. The executable check is
26+
* `checkDateBucketParity` (@objectstack/verify), run against the real drivers
27+
* in `packages/qa/dogfood/test/date-bucket-parity-conformance.test.ts`, where
28+
* the reference side is the REAL `applyInMemoryAggregation`.
29+
*/
2130
function bucketDateValue(value: unknown, g: Granularity): string {
2231
if (value == null) return '(null)';
2332
// A finite number is epoch milliseconds — SQLite's `Field.datetime` storage.

packages/plugins/driver-sqlite-wasm/src/sqlite-wasm-driver-date-bucket.test.ts

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,16 @@ import { SqliteWasmDriver } from '../src/index.js';
1717

1818
type Granularity = 'day' | 'week' | 'month' | 'quarter' | 'year';
1919

20-
/** ⚠️ Keep in sync with `packages/objectql/src/in-memory-aggregation.ts#bucketDateValue` */
20+
/**
21+
* ⚠️ Keep in sync with `packages/objectql/src/in-memory-aggregation.ts#bucketDateValue`.
22+
*
23+
* This copy exists because driver-sql cannot depend on objectql. Nothing here
24+
* can detect it drifting from its original — a stale copy and the SQL agree
25+
* with each other while both are wrong. The executable check is
26+
* `checkDateBucketParity` (@objectstack/verify), run against the real drivers
27+
* in `packages/qa/dogfood/test/date-bucket-parity-conformance.test.ts`, where
28+
* the reference side is the REAL `applyInMemoryAggregation`.
29+
*/
2130
function bucketDateValue(value: unknown, g: Granularity): string {
2231
if (value == null) return '(null)';
2332
// A finite number is epoch milliseconds — SQLite's `Field.datetime` storage.

packages/qa/dogfood/package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
"@objectstack/core": "workspace:*",
3131
"@objectstack/driver-memory": "workspace:*",
3232
"@objectstack/driver-sql": "workspace:*",
33+
"@objectstack/driver-sqlite-wasm": "workspace:*",
3334
"@types/node": "^26.1.1",
3435
"typescript": "^6.0.3",
3536
"vitest": "^4.1.10"
Lines changed: 141 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,141 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
//
3+
// Date-bucket parity conformance — exercises the reusable `checkDateBucketParity`
4+
// helper (from @objectstack/verify) against the framework's own SQL drivers.
5+
// A granularity a driver ADVERTISES via `supports.queryDateGranularity` must
6+
// bucket identically whether `engine.aggregate` pushes it down as SQL or falls
7+
// back to `applyInMemoryAggregation` — they are one feature with two
8+
// implementations, and the engine picks between them per query.
9+
//
10+
// This is the seam #3773 crossed silently. SQLite stores a `Field.datetime` as
11+
// INTEGER epoch ms; `strftime` read the bare integer as a Julian day number, so
12+
// every row bucketed as NULL and a trend chart collapsed into one bar. Every
13+
// existing gate was green: the driver's own bucket suites build their tables
14+
// with `knex.schema.createTable` + `t.string(...)` (ISO TEXT — the half
15+
// `strftime` parses natively), and the engine never second-guesses a
16+
// granularity a driver claims to support.
17+
//
18+
// It also closes the drift the `⚠️ Keep in sync` comments cannot: three test
19+
// files hand-copy `bucketDateValue` because driver-sql cannot depend on
20+
// objectql, and a copy that stops tracking its original leaves the copy and the
21+
// SQL agreeing with each other while both are wrong. Here the reference side is
22+
// the REAL `applyInMemoryAggregation`.
23+
24+
import { describe, it, expect } from 'vitest';
25+
import { checkDateBucketParity } from '@objectstack/verify';
26+
import { SqlDriver } from '@objectstack/driver-sql';
27+
import { SqliteWasmDriver } from '@objectstack/driver-sqlite-wasm';
28+
29+
const DRIVERS = [
30+
{
31+
name: 'driver-sql (better-sqlite3 :memory:)',
32+
make: () =>
33+
new SqlDriver({
34+
client: 'better-sqlite3',
35+
connection: { filename: ':memory:' },
36+
useNullAsDefault: true,
37+
}),
38+
},
39+
{
40+
// Inherits `buildDateBucketExpr` from SqlDriver, so it inherited #3773 too —
41+
// and resolves its base class from BUILT dist, which is its own way to drift.
42+
name: 'driver-sqlite-wasm (:memory:)',
43+
make: () => new SqliteWasmDriver({ filename: ':memory:' }),
44+
},
45+
];
46+
47+
describe.each(DRIVERS)('date-bucket parity conformance: $name', ({ make }) => {
48+
it('buckets identically pushed-down and in-memory, on both storage forms', async () => {
49+
const problems = await checkDateBucketParity(make() as never, {
50+
createOptions: { bypassTenantAudit: true },
51+
});
52+
expect(problems).toEqual([]);
53+
});
54+
});
55+
56+
describe('checkDateBucketParity detects a driver whose SQL bucketing is wrong', () => {
57+
// The negative control. A gate nobody has watched fail is a gate nobody knows
58+
// is wired up — and this one is cheap to fake, because the #3773 shape is
59+
// exactly "advertise the granularity, then bucket everything as NULL".
60+
function brokenDriver(overrides: Record<string, unknown> = {}) {
61+
const rows = [
62+
{ id: 'b1', at: '2024-01-15T10:00:00.000Z', on: '2024-01-15', n: 1 },
63+
{ id: 'b2', at: '2024-06-30T23:59:59.000Z', on: '2024-06-30', n: 1 },
64+
{ id: 'b3', at: '2024-07-01T00:00:00.000Z', on: '2024-07-01', n: 1 },
65+
{ id: 'b4', at: '2024-12-30T12:00:00.000Z', on: '2024-12-30', n: 1 },
66+
{ id: 'b5', at: '2025-01-01T00:00:00.000Z', on: '2025-01-01', n: 1 },
67+
{ id: 'b6', at: '2025-05-19T09:00:00.000Z', on: '2025-05-19', n: 1 },
68+
{ id: 'b7', at: '2025-05-19T22:30:00.000Z', on: '2025-05-19', n: 1 },
69+
];
70+
return {
71+
async connect() {},
72+
async disconnect() {},
73+
async syncSchema() {},
74+
async create() {},
75+
async find() {
76+
return rows;
77+
},
78+
// Everything collapses into one null bucket — the pre-#3773 SQLite shape.
79+
async aggregate(_object: string, query: any) {
80+
const field = query.groupBy[0].field;
81+
return [{ [field]: null, n: rows.length }];
82+
},
83+
supports: { queryDateGranularity: { day: true, month: true, quarter: true, year: true, week: false } },
84+
...overrides,
85+
};
86+
}
87+
88+
it('flags every advertised granularity on both columns', async () => {
89+
const problems = await checkDateBucketParity(brokenDriver() as never);
90+
// 4 advertised granularities × 2 storage forms = 8 disagreements, and the
91+
// cross-column pass stays quiet because both columns are broken the same way.
92+
expect(problems).toHaveLength(8);
93+
expect(problems.join('\n')).toMatch(/Field\.datetime 'at' @ month/);
94+
expect(problems.join('\n')).toMatch(/Field\.date 'on' @ year/);
95+
expect(problems.join('\n')).toMatch(/pushed-down SQL and in-memory bucketing disagree/);
96+
});
97+
98+
it('flags a driver that advertises a granularity it cannot actually run', async () => {
99+
const problems = await checkDateBucketParity(
100+
brokenDriver({
101+
async aggregate() {
102+
throw new Error("dateGranularity 'month' not supported on dialect");
103+
},
104+
}) as never,
105+
);
106+
expect(problems.join('\n')).toMatch(/advertises this granularity but aggregate\(\) threw/);
107+
});
108+
109+
it('flags a storage-form leak — one column right, the other collapsed', async () => {
110+
// The #3773 shape exactly: the TEXT-stored `date` column buckets fine while
111+
// the epoch-stored `datetime` column does not.
112+
const problems = await checkDateBucketParity(
113+
brokenDriver({
114+
async aggregate(_object: string, query: any) {
115+
const field = query.groupBy[0].field;
116+
if (field === 'on') {
117+
return [
118+
{ on: '2024', n: 4 },
119+
{ on: '2025', n: 3 },
120+
];
121+
}
122+
return [{ at: null, n: 7 }];
123+
},
124+
supports: { queryDateGranularity: { year: true } },
125+
}) as never,
126+
);
127+
expect(problems.join('\n')).toMatch(/Field\.datetime 'at' @ year/);
128+
expect(problems.join('\n')).toMatch(
129+
/describe the same days but bucket differently/,
130+
);
131+
});
132+
133+
it('stays quiet on a driver that advertises nothing', async () => {
134+
const problems = await checkDateBucketParity(
135+
brokenDriver({ supports: { queryDateGranularity: {} } }) as never,
136+
);
137+
// Nothing advertised ⇒ the engine buckets everything in-memory ⇒ nothing to
138+
// disagree about. A driver is never faulted for declining.
139+
expect(problems).toEqual([]);
140+
});
141+
});

0 commit comments

Comments
 (0)