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

.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(