fix(objectql)!: one key for the empty group bucket — real null, on both aggregation paths (#3839) (#3848)

`engine.aggregate` has two implementations of one feature and picks between
them per query: it pushes the aggregate down as SQL when the driver advertises
every requested granularity and the reference timezone is UTC, otherwise it
fetches rows and buckets them in JS. The two disagreed about how to spell
"empty" — SQL NULL against the in-memory literal `'(null)'` — so the same
dataset produced a different bucket key type on SQLite+UTC+`month` than on
`week`, a non-UTC timezone, or driver-rest / driver-memory / a remote Turso.

The measures were always right; only the key's type and literal differed, which
is why it went unnoticed. It was never date-specific either — a plain
`groupBy: ['stage']` over a NULL column diverged the same way.

Consumers are written against `null`: they check `== null` and supply their own
empty label. The sentinel defeated every one of them, leaking a raw English
debug string into the UI and compiling a drill on the empty bucket to
`field = '(null)'`, which matches nothing. The comment justifying the string
cited the client `useReportData` hook, removed with ADR-0021 — and the literal
never appeared in it.

- `applyInMemoryAggregation` / `bucketDateValue` key the empty bucket as `null`;
  `bucketDateValue` returns `string | null`. Null and unparseable instants still
  share one bucket, because SQL cannot tell them apart either.
- The internal composite bucket id is JSON-encoded, so the empty bucket stays
  distinct from a row whose value is the literal string `"null"`. Same in
  service-analytics' cross-object rebucketing.
- `bucketKeyToCalendarRange` accepts `string | null` — the empty bucket has no
  calendar span, so a drill on it opens the unscoped superset as before.
- The driver output contract in spec now states the rule.

Gates: `checkDateBucketParity`'s fixture deliberately had no null instant
because the divergence would have failed it for a reason it was not about; it
has one now. Two fixes made that meaningful — the check folded labels through
`String(value)`, which turns SQL NULL into `'null'` and could compare equal to a
sentinel string, and it compared label sets with `JSON.stringify`, which is
sensitive to key insertion order that the two paths legitimately differ on (a
correct driver could be flagged, with an empty diff message). A new dogfood
check covers the non-date half against real drivers.

Both gates were confirmed to fail with the sentinel restored.

Co-authored-by: Jack Zhuang <277994282+os-zhuang@users.noreply.github.com>
Co-authored-by: Claude <noreply@anthropic.com>

Author: os-zhuang

.changeset/empty-group-bucket-key-null.md

@@ -0,0 +1,87 @@
+---
+"@objectstack/objectql": minor
+"@objectstack/verify": minor
+"@objectstack/core": patch
+"@objectstack/service-analytics": patch
+"@objectstack/spec": patch
+---
+
+fix(objectql)!: one key for the empty group bucket — real `null`, on both aggregation paths (#3839)
+
+A grouped row whose dimension value is empty now carries `null` for that
+dimension no matter which way the aggregate ran. Downstream code can test the
+empty bucket with a plain `value == null` again: charts render their own empty
+label, drill-through on that bucket builds `field = null` and returns the rows
+it should, and a dashboard no longer changes shape when the driver, the
+granularity or the reference timezone changes.
+
+### What was wrong
+
+`engine.aggregate` has two implementations of one feature. It pushes the
+aggregate down as SQL when the driver advertises every requested granularity and
+the reference timezone is UTC; otherwise it fetches rows and buckets them in JS.
+The two disagreed about how to spell "empty":
+
+```
+--- same dataset, same query, one row with a NULL value ---
+  pushed-down SQL : [{ "key": null,     "type": "null",   "total": 2 }, …]
+  in-memory       : [{ "key": "(null)", "type": "string", "total": 2 }, …]
+```
+
+The measures were always right — only the key's type and literal differed —
+which is why this went unnoticed for so long: every total reconciled. But the
+engine picks a path per query, so the same data produced a different bucket key
+on SQLite-plus-UTC-plus-`month` than on `week` (which SQLite does not advertise),
+a non-UTC timezone, or `driver-rest` / `driver-memory` / a remote Turso, all of
+which bucket in memory unconditionally.
+
+It was never date-specific either. A plain `groupBy: ['stage']` over a NULL
+column diverged the same way.
+
+Consumers are written against `null` — they check `== null` and supply their own
+empty label ('—', '(empty)', a localized "Uncategorized"). The sentinel defeated
+every one of them: it rendered a raw English debug string in the UI, and a drill
+on the empty bucket compiled to `field = '(null)'` and matched nothing.
+
+The in-memory path's comment justified the string as staying "consistent with
+the client `useReportData` hook". That hook was removed with ADR-0021, and the
+literal never appeared in it.
+
+### What changed
+
+- `applyInMemoryAggregation` and `bucketDateValue` (`@objectstack/objectql`) key
+  the empty bucket as `null`. `bucketDateValue` now returns `string | null`. A
+  null instant and an unparseable one still share one bucket, because SQL cannot
+  tell them apart either (`strftime('%Y-%m', 'not-a-date')` is NULL).
+- The internal composite bucket id is JSON-encoded, so the empty bucket stays
+  distinct from a row whose value is the literal string `"null"`.
+- `bucketKeyToCalendarRange` (`@objectstack/core`) accepts `string | null`. The
+  empty bucket has no calendar span, so a drill on it opens the unscoped
+  superset instead of an invented bound — unchanged behavior, honest signature.
+- The driver output contract in `@objectstack/spec` now states the rule: a row
+  with no value keys as `null`, never a sentinel. Propagating NULL through the
+  bucket expression is the whole of it; a driver only breaks it by adding a
+  `COALESCE`.
+
+### Gates
+
+`checkDateBucketParity` (`@objectstack/verify`) deliberately carried no null
+instant, because the divergence would have failed it for a reason it was not
+about. Its fixture now has one, so the convergence is held in place — including
+for out-of-tree drivers that run the check against themselves.
+
+Two fixes were needed to make that fixture meaningful:
+
+- The check folded bucket labels through `String(value)`, which turns SQL NULL
+  into `'null'` — a label a TEXT column can genuinely hold. A driver spelling
+  "empty" as a string could compare equal to one returning real NULL. The empty
+  bucket is now keyed out of band.
+- Label sets were compared with `JSON.stringify`, which is sensitive to key
+  insertion order. Row order is not part of this contract and the two paths
+  naturally differ (SQL sorts its groups; the in-memory path emits first-seen
+  order), so a driver with entirely correct buckets could be reported as
+  disagreeing — with an empty diff message, since nothing actually differed.
+  The comparison is now order-insensitive.
+
+A new dogfood check covers the non-date half against real drivers: same dataset,
+plain and date-bucketed `groupBy`, both paths, one key.

packages/core/src/utils/datetime.ts

@@ -144,13 +144,17 @@ function isoWeekLabelUtc(d: Date): string {
  * `datetime` field in a reference timezone layers that on top (and, per
  * ADR-0053, a `date` field compares against these `YYYY-MM-DD` bounds directly).
  *
- * Returns `null` for the null/empty bucket, an unparseable key, or a key that
- * is shape-valid but out of range (e.g. `2026-13`, a `-W53` in a 52-week year,
+ * Returns `null` for the empty bucket, an unparseable key, or a key that is
+ * shape-valid but out of range (e.g. `2026-13`, a `-W53` in a 52-week year,
  * `2026-02-30`). Callers drop the range and fall back to an unscoped (superset)
  * drill rather than emit a wrong bound.
+ *
+ * `key` admits `null` because that IS the empty bucket's key on both aggregation
+ * paths (#3839); callers pass a grouped row's dimension value straight through
+ * rather than casting a lie.
  */
 export function bucketKeyToCalendarRange(
-  key: string,
+  key: string | null | undefined,
   granularity: BucketGranularity,
 ): { start: string; end: string } | null {
   if (typeof key !== 'string' || key.length === 0) return null;

packages/objectql/src/date-bucket-range.test.ts

@@ -73,6 +73,12 @@ describe('bucketKeyToCalendarRange exact boundaries', () => {
 
 describe('bucketKeyToCalendarRange rejects unbucketable / out-of-range keys → null (superset fallback)', () => {
   it('null and empty buckets', () => {
+    // #3839 — the empty bucket's key IS `null`, on both the pushed-down and the
+    // in-memory path. It has no calendar span, so the caller drops the range and
+    // drills the unscoped superset instead of inventing a bound.
+    expect(bucketKeyToCalendarRange(null, 'month')).toBeNull();
+    expect(bucketKeyToCalendarRange(undefined, 'month')).toBeNull();
+    // The pre-#3839 sentinel, in case one survives in stored/replayed data.
     expect(bucketKeyToCalendarRange('(null)', 'month')).toBeNull();
     expect(bucketKeyToCalendarRange('', 'day')).toBeNull();
   });

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

@@ -76,13 +76,44 @@ describe('applyInMemoryAggregation', () => {
     expect(east!.owner_str).toBe('alice,alice,bob');
   });
 
-  it('treats null group values as the literal (null) bucket', () => {
-    const dataset = [{ stage: null, amount: 10 }, { stage: 'won', amount: 5 }];
+  // #3839 — this used to be the literal string `'(null)'`, which the pushed-down
+  // SQL path never produced (a NULL group column stays SQL NULL). The engine
+  // picks between the two paths per query, so the bucket key's TYPE changed
+  // under a dashboard when the driver, the granularity or the timezone changed.
+  it('keys the empty bucket as real null, like the pushed-down SQL', () => {
+    const dataset = [
+      { stage: null, amount: 10 },
+      { stage: undefined, amount: 1 },
+      { stage: 'won', amount: 5 },
+      { amount: 2 }, // field absent entirely
+    ];
     const out = applyInMemoryAggregation(dataset, {
       groupBy: ['stage'],
       aggregations: [{ function: 'sum', field: 'amount', alias: 'total' }],
     });
-    expect(out.find((r) => r.stage === '(null)')!.total).toBe(10);
+    // null / undefined / absent all describe the same emptiness → one bucket.
+    const empty = out.filter((r) => r.stage === null);
+    expect(empty).toHaveLength(1);
+    expect(empty[0].total).toBe(13);
+    // …and it is a real null, not a string that happens to read like one.
+    expect(out.some((r) => typeof r.stage === 'string' && /null/i.test(r.stage))).toBe(false);
+  });
+
+  // The empty bucket's key is now `null`, and `${null}` is the string 'null' —
+  // so a row whose value IS the string "null" would merge into the empty bucket
+  // if the internal bucket id were built by plain interpolation.
+  it('keeps the empty bucket distinct from the literal string "null"', () => {
+    const dataset = [
+      { stage: null, amount: 10 },
+      { stage: 'null', amount: 5 },
+    ];
+    const out = applyInMemoryAggregation(dataset, {
+      groupBy: ['stage'],
+      aggregations: [{ function: 'sum', field: 'amount', alias: 'total' }],
+    });
+    expect(out).toHaveLength(2);
+    expect(out.find((r) => r.stage === null)!.total).toBe(10);
+    expect(out.find((r) => r.stage === 'null')!.total).toBe(5);
   });
 });
 
@@ -101,9 +132,13 @@ describe('bucketDateValue', () => {
     expect(bucketDateValue('2024-12-30', 'week')).toBe('2025-W01');
   });
 
-  it('returns (null) for null / invalid dates', () => {
-    expect(bucketDateValue(null, 'month')).toBe('(null)');
-    expect(bucketDateValue('not-a-date', 'month')).toBe('(null)');
+  // #3839 — `null`, not a sentinel string. SQL propagates NULL through the
+  // bucket expression for both of these (`strftime('%Y-%m', 'not-a-date')` is
+  // NULL), so the two paths agree on the empty bucket as well as the full ones.
+  it('returns null for null / invalid dates', () => {
+    expect(bucketDateValue(null, 'month')).toBeNull();
+    expect(bucketDateValue(undefined, 'month')).toBeNull();
+    expect(bucketDateValue('not-a-date', 'month')).toBeNull();
   });
 
   // #3773 — parity with the pushed-down SQL. SQLite stores a `Field.datetime`

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

@@ -21,9 +21,25 @@
 //     possible; the in-memory fallback ignores the per-aggregation filter and
 //     logs a warning if one is present.
 //
-// Date bucketing uses ISO-8601 conventions (weeks start Monday). Null /
-// invalid values bucket as the literal string `'(null)'` to remain
-// consistent with the client `useReportData` hook.
+// Date bucketing uses ISO-8601 conventions (weeks start Monday).
+//
+// THE EMPTY BUCKET'S KEY IS REAL `null` — not a sentinel string (#3839). A
+// grouped row whose value is null/absent (or, for a date bucket, unparseable)
+// carries `null` for that dimension, which is what the pushed-down SQL path
+// emits for the same row: the group column is SQL NULL, or `strftime(...)` /
+// `date_trunc(...)` returns NULL for a NULL input. Both paths therefore
+// describe "empty" the same way, and `engine.aggregate` picking one per query
+// — by driver, granularity, or reference timezone — can no longer change the
+// TYPE of a bucket key under a dashboard that drills across the seam.
+//
+// This previously emitted the literal `'(null)'` "to remain consistent with
+// the client `useReportData` hook". That hook was deleted with ADR-0021 (its
+// epitaph is objectui `packages/plugin-report/src/index.tsx`), and the string
+// never appeared in it anyway. What the string DID do was defeat every
+// downstream `== null` check: consumers render their own empty label ('—',
+// '(empty)', a localized "Uncategorized") and build drill filters as
+// `field = null`, so a sentinel leaked a raw English debug string into the UI
+// and turned the empty bucket's drill-through into a zero-row query.
 
 import { calendarPartsInTzOrUtc } from '@objectstack/core';
 import type { QueryAST, GroupByNode, AggregationNode, DateGranularityValue } from '@objectstack/spec/data';
@@ -59,7 +75,11 @@ export function applyInMemoryAggregation(
       const fieldName = typeof g === 'string' ? g : (g.alias ?? g.field);
       const value = projectGroupValue(row, g, timezone);
       key[fieldName] = value;
-      parts.push(`${fieldName}=${value}`);
+      // JSON-encoded so the empty bucket's `null` key cannot collide with a row
+      // whose value is the literal STRING `"null"` — plain interpolation renders
+      // both as `null` and would merge two distinct groups. This id is internal
+      // to the bucketing loop; only `key` is emitted.
+      parts.push(`${fieldName}=${JSON.stringify(value)}`);
     }
     const id = parts.join('\u0001');
     let bucket = buckets.get(id);
@@ -78,13 +98,15 @@ export function applyInMemoryAggregation(
   return out;
 }
 
-function projectGroupValue(row: any, g: GroupByNode, timezone?: string): string {
+function projectGroupValue(row: any, g: GroupByNode, timezone?: string): string | null {
   const field = typeof g === 'string' ? g : g.field;
   const v = row?.[field];
   if (typeof g !== 'string' && g.dateGranularity) {
     return bucketDateValue(v, g.dateGranularity, timezone);
   }
-  return v == null ? '(null)' : String(v);
+  // `null`, not a sentinel string — same key the pushed-down SQL gives a NULL
+  // group column (#3839). See the empty-bucket note at the top of this file.
+  return v == null ? null : String(v);
 }
 
 function aggregateBucket(rows: any[], aggregations: AggregationNode[]): Record<string, any> {
@@ -199,23 +221,29 @@ function toNumber(v: any): number {
  * A finite NUMBER is read as epoch milliseconds — the form SQLite stores a
  * `Field.datetime` in, and what any driver that hands back raw storage values
  * yields. `new Date(String(1767225600000))` is an Invalid Date, so without this
- * branch such a row bucketed as `'(null)'` while the pushed-down SQL bucketed it
- * correctly (#3773) — the two paths must label the same instant identically or a
- * drill-down built on one breaks against the other.
+ * branch such a row landed in the empty bucket while the pushed-down SQL
+ * bucketed it correctly (#3773) — the two paths must label the same instant
+ * identically or a drill-down built on one breaks against the other.
+ *
+ * Returns `null` for a null/absent or unparseable instant — the same key the
+ * pushed-down SQL yields, where the bucket expression propagates NULL (#3839).
+ * Null and unparseable deliberately share one bucket: SQL cannot tell them
+ * apart either (`strftime('%Y-%m', 'not-a-date')` is NULL), and splitting them
+ * here would re-open the seam this function exists to close.
  */
 export function bucketDateValue(
   value: unknown,
   granularity: DateGranularityValue,
   timezone?: string,
-): string {
-  if (value == null) return '(null)';
+): string | null {
+  if (value == null) return null;
   const d =
     value instanceof Date
       ? value
       : typeof value === 'number'
         ? new Date(value)
         : new Date(String(value));
-  if (Number.isNaN(d.getTime())) return '(null)';
+  if (Number.isNaN(d.getTime())) return null;
   const { year: y, month: m, day } = calendarPartsInTzOrUtc(d, timezone);
   switch (granularity) {
     case 'year':

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

@@ -37,12 +37,12 @@ const GRANULARITIES: Granularity[] = ['day', 'month', 'quarter', 'year'];
  * in `packages/qa/dogfood/test/date-bucket-parity-conformance.test.ts`, where
  * the reference side is the REAL `applyInMemoryAggregation`.
  */
-function bucketDateValue(value: unknown, g: Granularity): string {
-  if (value == null) return '(null)';
+function bucketDateValue(value: unknown, g: Granularity): string | null {
+  if (value == null) return null;
   // A finite number is epoch milliseconds — SQLite's `Field.datetime` storage.
   const d =
     value instanceof Date ? value : typeof value === 'number' ? new Date(value) : new Date(String(value));
-  if (Number.isNaN(d.getTime())) return '(null)';
+  if (Number.isNaN(d.getTime())) return null;
   const y = d.getUTCFullYear();
   const m = d.getUTCMonth() + 1;
   switch (g) {
@@ -73,11 +73,21 @@ const FIXTURE: Array<{ id: string; iso: string; amount: number }> = [
   { id: 'r7', iso: '2026-07-01T00:00:00.000Z', amount: 64 }, // first instant of Q3
 ];
 
+/**
+ * Test-local key for the empty bucket. `String(null)` is `'null'` — a label a
+ * TEXT column could genuinely hold, and the coercion that used to hide the
+ * #3839 divergence: it made the SQL side's NULL and a sentinel string compare
+ * as different strings only by luck. Keying it out of band keeps "empty"
+ * unmistakable on both sides.
+ */
+const EMPTY = '‹empty bucket›';
+const labelOf = (v: unknown): string => (v == null ? EMPTY : String(v));
+
 /** The labels the in-memory path would produce, folded into bucket → sum. */
 function expectedBuckets(g: Granularity): Record<string, number> {
   const out: Record<string, number> = {};
   for (const row of FIXTURE) {
-    const key = bucketDateValue(row.iso, g);
+    const key = labelOf(bucketDateValue(row.iso, g));
     out[key] = (out[key] ?? 0) + row.amount;
   }
   return out;
@@ -88,7 +98,7 @@ async function bucketSums(driver: SqlDriver, field: string, g: Granularity) {
     groupBy: [{ field, dateGranularity: g }],
     aggregations: [{ function: 'sum', field: 'amount', alias: 'total' }],
   } as any);
-  return Object.fromEntries(rows.map((r: any) => [String(r[field]), Number(r.total)]));
+  return Object.fromEntries(rows.map((r: any) => [labelOf(r[field]), Number(r.total)]));
 }
 
 describe('SqlDriver date bucketing is storage-form independent (#3773)', () => {
@@ -218,9 +228,11 @@ describe('SqlDriver date bucketing over a MIXED-form datetime column (#3773)', (
 
   it('leaves a NULL instant in its own bucket', async () => {
     const byMonth = await bucketSums(driver, 'closed_at', 'month');
-    // SQL NULL aliases to the string 'null' through `String(r[field])` — a
-    // pre-existing divergence from the in-memory label `'(null)'`, unchanged
-    // here and equally true of a TEXT-stored column.
-    expect(byMonth.null).toBe(8);
+    // SQL NULL propagates through the bucket expression, so the row lands in the
+    // empty bucket instead of under some real month. Since #3839 the in-memory
+    // path keys that bucket as `null` as well, so the two paths agree on it —
+    // the executable proof is `checkDateBucketParity`, whose fixture now carries
+    // a null instant for exactly this.
+    expect(byMonth[EMPTY]).toBe(8);
   });
 });

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

@@ -17,6 +17,16 @@ import { SqlDriver } from '../src/index.js';
 
 type Granularity = 'day' | 'week' | 'month' | 'quarter' | 'year';
 
+/**
+ * Test-local key for the empty bucket, on both sides of the comparison below.
+ * `String(null)` is `'null'` — a label a TEXT column could genuinely hold — so
+ * folding the empty bucket through it makes "empty" indistinguishable from a
+ * real value. Keying it out of band is what lets a sentinel-vs-NULL divergence
+ * (#3839) show up as a difference instead of comparing equal by coincidence.
+ */
+const EMPTY = '‹empty bucket›';
+const labelOf = (v: unknown): string => (v == null ? EMPTY : String(v));
+
 /**
  * ⚠️ Keep in sync with `packages/objectql/src/in-memory-aggregation.ts#bucketDateValue`.
  *
@@ -27,12 +37,12 @@ type Granularity = 'day' | 'week' | 'month' | 'quarter' | 'year';
  * in `packages/qa/dogfood/test/date-bucket-parity-conformance.test.ts`, where
  * the reference side is the REAL `applyInMemoryAggregation`.
  */
-function bucketDateValue(value: unknown, g: Granularity): string {
-  if (value == null) return '(null)';
+function bucketDateValue(value: unknown, g: Granularity): string | null {
+  if (value == null) return null;
   // A finite number is epoch milliseconds — SQLite's `Field.datetime` storage.
   const d =
     value instanceof Date ? value : typeof value === 'number' ? new Date(value) : new Date(String(value));
-  if (Number.isNaN(d.getTime())) return '(null)';
+  if (Number.isNaN(d.getTime())) return null;
   const y = d.getUTCFullYear();
   const m = d.getUTCMonth() + 1;
   switch (g) {
@@ -113,13 +123,13 @@ describe('SqlDriver date bucket (dateGranularity)', () => {
 
         const expectedBuckets = new Map<string, number>();
         for (const r of FIXTURE) {
-          const key = bucketDateValue(r.ts, g);
+          const key = labelOf(bucketDateValue(r.ts, g));
           expectedBuckets.set(key, (expectedBuckets.get(key) ?? 0) + 1);
         }
 
         const actualBuckets = new Map<string, number>();
         for (const row of rows) {
-          actualBuckets.set(String(row.ts), Number(row.n));
+          actualBuckets.set(labelOf(row.ts), Number(row.n));
         }
 
         expect([...actualBuckets.entries()].sort()).toEqual(