diff --git a/.changeset/analytics-time-dimension-granularity-buckets.md b/.changeset/analytics-time-dimension-granularity-buckets.md new file mode 100644 index 0000000000..73deae054a --- /dev/null +++ b/.changeset/analytics-time-dimension-granularity-buckets.md @@ -0,0 +1,120 @@ +--- +"@objectstack/core": minor +"@objectstack/objectql": minor +"@objectstack/driver-memory": minor +--- + +fix(driver-memory)!: an analytics time dimension buckets by its declared `granularity`, and refuses a sub-day one instead of ignoring it (#16178) + + + +**BREAKING** in three senses, all on `driver-memory`'s analytics face, landing in +the launch window as `minor` under the lockstep convention this cluster's +siblings already use: + +- an accepted request now answers **differently**: a time dimension carrying a + `granularity` folds its rows into calendar buckets instead of returning one + group per distinct timestamp. Every affected answer was wrong before; +- a **trend query answers rows where it used to answer one total**: a + `granularity` on a member `dimensions` does not also list is now a group + column of its own, so `{measures, timeDimensions: [{dimension, granularity}]}` + — the canonical trend shape — comes back one row per bucket, carrying the + member and a `fields` entry for it, instead of a single ungrouped total with + no such column; +- an accepted request is now **refused**: `granularity: 'second' | 'minute' | + 'hour'` answers `NOT_IMPLEMENTED` / 501 instead of being silently dropped. + +## What was wrong + +`AnalyticsQuery.timeDimensions[].granularity` is declared by the spec and a cube +dimension enumerates the granularities it offers (`granularities: ['day']`). +`memory-analytics.ts` read neither. The `$group` stage keyed on the raw field +path, so a time dimension bucketed **one group per distinct timestamp** — one bar +per row in a "new accounts by month" chart, which is the symptom #3588 +catalogued and repaired for `service-analytics`. + +Measured through the public entry against the built package, two rows on one UTC +calendar day (`2026-09-06T01:00:00Z` and `2026-09-06T23:00:00Z`) under +`granularity: 'day'`: + +| | before | after | +|:--|--:|--:| +| `granularity: 'day'` | **2 groups**, keyed on the raw instants | 1 group, `2026-09-06` | +| no granularity (control) | 2 groups | 2 groups, unchanged | +| `granularity: 'hour'` | **2 groups**, silently | `NOT_IMPLEMENTED` / 501 | +| same, but with no `dimensions` | **`{count: 2}`** — one total, no time column, and no `fields` entry naming it | `{'events.createdAt': '2026-09-06', count: 2}`, `fields` naming both | +| `granularity: 'fortnight'` past the schema door | — | `INVALID_QUERY` / 400 | + +The emitted pipeline was byte-identical across all three, which is the whole +finding: the request was accepted, no warning was emitted, and the key was inert. + +## What it does now + +- **One forward labeller, in `@objectstack/core`.** `bucketDateKey(value, + granularity, timezone)` sits beside the inverse `bucketKeyToCalendarRange` and + the `calendarPartsInTzOrUtc` primitive it builds on, and it is now the only + statement of the rule. `BUCKET_GRANULARITIES` and `isBucketGranularity` name + the five granularities that HAVE a canonical key, so a face that must refuse + the other three quotes the accepted set instead of hand-listing it. +- **`@objectstack/objectql`'s `bucketDateValue` is a delegate**, export name and + signature unchanged, answers unchanged — pinned across granularity, timezone + and input form rather than asserted. A driver that pushes the bucket down into + SQL and this in-memory path must label one instant identically or a drill-down + breaks at the seam, and that is now one function rather than an agreement + between two. +- **A granular time dimension is a group column, listed or not.** `dimensions` + no longer decides alone what `$group` keys on: every `timeDimensions` entry + carrying a `granularity` is grouped, projected and named in `fields`, deduped + against `dimensions` on the resolved member so two spellings of one member + stay one column. This is the rule the SQL/ObjectQL face already records + (`projectedDimensions`, #4033/#5688) — one set feeding grouping, row mapping + and field metadata, because rows carrying a bucket under a `fields` list that + never mentions it is a trend chart with no x-axis. ⛔ An entry carrying only a + `dateRange` is a predicate and is still **not** projected. +- **`driver-memory` folds by granularity before its `$group`.** The pipeline is + cut at that stage: the `$match` half still runs in the driver, the bucket keys + are written onto the selected rows, and the grouping half runs over those. The + key travels under a synthetic field rather than overwriting the row's own, so a + member that is both a group key and a measure's aggregand still ranks instants + in `max()` while grouping on the label. +- **The output vocabulary is the published one** — `2026`, `2026-Q3`, `2026-09`, + `2026-09-06`, `2026-W36`. The week label is `YYYY-Www`, never the Monday's + `YYYY-MM-DD`: `DriverCapabilitiesSchema.queryDateGranularity` calls this an + output contract, and a second spelling is what breaks a drill-down across a + backend seam. +- **Bucketing honours `AnalyticsQuery.timezone`** — the same reference zone + #16042 threaded through the `dateRange` window resolver, so the window that + selects the rows and the bucket that folds them agree on where a calendar day + starts. The same two rows answer one group in UTC, two in `America/New_York` + and two in `Asia/Tokyo`. An absent zone buckets in UTC, the resolver's default. + + ⚠️ That agreement is about the PRESET arm of `dateRange`, which the resolver + reads in the reference zone. An explicit `[start, end]` array is the caller's + own **instant** window and keeps its published reading (#16179), while the + bucket beside it is always a **calendar** label (ADR-0053) — so an array + window and a bucket can still disagree about where a day starts. That + combination is legitimate and is not refused; it is stated here rather than + left to be discovered. +- **`second` / `minute` / `hour` are refused at compile**, in the ADR-0112 + envelope this driver's other capability gaps speak (`NOT_IMPLEMENTED` / 501, + the class `refusePerAggregationFilter` uses for the same reason: the query is + spelled correctly, the spec declares the value, and it is this backend that + compiles nothing for it). The canonical key vocabulary defines no label for a + sub-day bucket, so there is no string another backend's pushed-down SQL would + agree with. Passing it through unbucketed is this card's own defect wearing a + new name. +- **An undeclared granularity is a 400, not a 501.** A 501 says "this backend + cannot", which is only honest about a value the contract declares. + `TimeUpdateInterval` is checked first, so a spelling it never declared — + reachable past the schema door, where `POST /analytics/dataset/query` types + `selection.timeDimensions` without Zod-parsing them — answers `INVALID_QUERY` + / 400 rather than a 501 asserting the spec declared it. The same separation + the `dateRange` half of this face already draws (#16322 / #16041). + +## If a caller is refused + +A stored widget or a request asking for a sub-day granularity was never bucketed +by this backend — it received one group per distinct timestamp under an ordinary +200. Nothing that worked stops working. Ask for `day` or coarser and the answer +is a real bucket; keep the raw timestamps deliberately by dropping the key, which +is the behaviour that key used to produce by accident. diff --git a/packages/core/src/utils/datetime.test.ts b/packages/core/src/utils/datetime.test.ts index cb01628980..cf423ce08f 100644 --- a/packages/core/src/utils/datetime.test.ts +++ b/packages/core/src/utils/datetime.test.ts @@ -11,6 +11,9 @@ import { zonedWallClockToUtcMs, calendarPartsInTz, nextUtcCalendarDay, + bucketDateKey, + BUCKET_GRANULARITIES, + isBucketGranularity, } from './datetime.js'; const iso = (s: string) => Date.parse(s); @@ -223,3 +226,77 @@ describe('nextUtcCalendarDay — re-exported from @objectstack/spec (ADR-0053 D- expect(nextUtcCalendarDay('2026-07-28T12:00:00Z')).toBeNull(); }); }); + +// ── [#16178] `bucketDateKey` — the forward labeller ────────────────────────── +// +// Hoisted here so `driver-memory`'s analytics face and `@objectstack/objectql`'s +// `bucketDateValue` label one instant identically without a driver depending on +// objectql and without a third hand copy of the rule. These cells pin the OUTPUT +// CONTRACT (`DriverCapabilitiesSchema.queryDateGranularity` calls it that), not +// a display preference: a driver that pushes the bucket down into SQL has to +// emit these exact strings. + +describe('bucketDateKey — canonical labels', () => { + it('labels each of the five granularities in its canonical vocabulary', () => { + expect(bucketDateKey('2024-05-15T00:00:00Z', 'year')).toBe('2024'); + expect(bucketDateKey('2024-05-15T00:00:00Z', 'quarter')).toBe('2024-Q2'); + expect(bucketDateKey('2024-05-15T00:00:00Z', 'month')).toBe('2024-05'); + expect(bucketDateKey('2024-05-15T00:00:00Z', 'day')).toBe('2024-05-15'); + expect(bucketDateKey('2024-05-15T00:00:00Z', 'week')).toBe('2024-W20'); + }); + + it('numbers ISO weeks by the first-Thursday rule, across the year boundary', () => { + expect(bucketDateKey('2024-01-01T00:00:00Z', 'week')).toBe('2024-W01'); + // 2024-12-30 is a Monday whose week's Thursday falls in 2025. + expect(bucketDateKey('2024-12-30T00:00:00Z', 'week')).toBe('2025-W01'); + }); + + it('reads a finite number as epoch milliseconds (#3773)', () => { + const ms = Date.parse('2026-01-10T08:30:00Z'); + expect(bucketDateKey(ms, 'day')).toBe('2026-01-10'); + for (const g of BUCKET_GRANULARITIES) { + // The three input spellings a driver can hand back must agree. + expect(bucketDateKey(ms, g)).toBe(bucketDateKey(new Date(ms), g)); + expect(bucketDateKey(ms, g)).toBe(bucketDateKey(new Date(ms).toISOString(), g)); + } + }); + + it('answers null for an absent or unparseable instant (#3839)', () => { + expect(bucketDateKey(null, 'month')).toBeNull(); + expect(bucketDateKey(undefined, 'month')).toBeNull(); + expect(bucketDateKey('not-a-date', 'month')).toBeNull(); + }); + + it('resolves the calendar day in the reference timezone (ADR-0053 Phase 2)', () => { + // 23:30 UTC on Feb 29 is still Feb 29 in UTC and already Mar 1 in Tokyo. + const nearMidnight = '2024-02-29T23:30:00Z'; + expect(bucketDateKey(nearMidnight, 'day', 'UTC')).toBe('2024-02-29'); + expect(bucketDateKey(nearMidnight, 'day', 'Asia/Tokyo')).toBe('2024-03-01'); + expect(bucketDateKey(nearMidnight, 'month', 'Asia/Tokyo')).toBe('2024-03'); + // The week math runs on the parts ALREADY shifted into the zone, so the + // week boundary moves with the day rather than re-applying the offset. + const mondayUtc = '2024-03-04T02:00:00Z'; // Monday 03-04 UTC, Sunday 03-03 in NY + expect(bucketDateKey(mondayUtc, 'week', 'UTC')).toBe('2024-W10'); + expect(bucketDateKey(mondayUtc, 'week', 'America/New_York')).toBe('2024-W09'); + }); +}); + +describe('BUCKET_GRANULARITIES / isBucketGranularity', () => { + it('is exactly the set bucketDateKey can label', () => { + expect([...BUCKET_GRANULARITIES]).toEqual(['day', 'week', 'month', 'quarter', 'year']); + for (const g of BUCKET_GRANULARITIES) { + expect(isBucketGranularity(g)).toBe(true); + // Every accepted member really produces a label — the guard and the + // labeller cannot disagree about the set without this failing. + expect(bucketDateKey('2024-05-15T00:00:00Z', g)).toMatch(/^\d{4}/); + } + }); + + it('rejects the three sub-day intervals TimeUpdateInterval also declares', () => { + // These have no canonical key anywhere in the contract, which is why a face + // that receives one refuses rather than bucketing it (#16178). + for (const g of ['second', 'minute', 'hour']) expect(isBucketGranularity(g)).toBe(false); + expect(isBucketGranularity('decade')).toBe(false); + expect(isBucketGranularity(undefined)).toBe(false); + }); +}); diff --git a/packages/core/src/utils/datetime.ts b/packages/core/src/utils/datetime.ts index 2707233d14..f041cc4bef 100644 --- a/packages/core/src/utils/datetime.ts +++ b/packages/core/src/utils/datetime.ts @@ -202,15 +202,112 @@ export { nextUtcCalendarDay, utcInstantMs } from '@objectstack/spec/data'; */ export type BucketGranularity = 'day' | 'week' | 'month' | 'quarter' | 'year'; +/** + * The granularities that HAVE a canonical bucket key — the accepted set of + * {@link bucketDateKey}, in ascending order. + * + * `@objectstack/spec`'s `TimeUpdateInterval` declares three more (`second`, + * `minute`, `hour`) for which the contract defines no canonical key vocabulary + * anywhere. Exported so a face that has to refuse one of those names the + * accepted set FROM HERE: a hand-listed copy in a refusal message agrees with + * this one on the day it is typed and never again. + */ +export const BUCKET_GRANULARITIES: readonly BucketGranularity[] = [ + 'day', + 'week', + 'month', + 'quarter', + 'year', +]; + +/** + * Is `value` one of the five granularities {@link bucketDateKey} can label? + * + * The guard a caller holding a wider vocabulary (`TimeUpdateInterval`) uses to + * split "bucket it" from "refuse it" without restating either set. + */ +export function isBucketGranularity(value: unknown): value is BucketGranularity { + return typeof value === 'string' && (BUCKET_GRANULARITIES as readonly string[]).includes(value); +} + +/** + * The canonical date-bucket KEY an instant falls in, as seen in a reference + * timezone — the FORWARD direction of {@link bucketKeyToCalendarRange}, and the + * one labeller the in-memory bucketing faces delegate to. + * + * ⚠️ **The label vocabulary is an output contract, not a display choice.** A + * driver that advertises `supports.queryDateGranularity[g]` buckets that + * granularity in SQL instead and `engine.aggregate` picks between the two per + * query, so a label produced here must equal the label that driver's SQL + * produces for the same instant, or a drill-down breaks when it crosses the + * seam. `2026`, `2026-Q2`, `2026-06`, `2026-06-15`, `2026-W23` — editing them + * means editing every driver's bucket expression too. The seam is enforced by + * `checkDateBucketParity` (`@objectstack/verify`). + * + * `timezone` (ADR-0053 Phase 2) resolves the calendar day in a reference zone so + * an instant near a tz day-boundary buckets where a user in that zone would + * expect. An unset / `'UTC'` / invalid zone keeps UTC bucketing. The y/m/d are + * taken in the reference zone and the ISO-week math then runs on a UTC date + * built from those parts — the parts already carry the zone shift, so the week + * boundary lands correctly without re-applying any offset. + * + * 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 lands in the empty bucket while the pushed-down SQL buckets + * 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 bucketDateKey( + value: unknown, + granularity: BucketGranularity, + timezone?: string, +): 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; + const { year: y, month: m, day } = calendarPartsInTzOrUtc(d, timezone); + switch (granularity) { + case 'year': + return String(y); + case 'quarter': + return `${y}-Q${Math.floor((m - 1) / 3) + 1}`; + case 'month': + return `${y}-${String(m).padStart(2, '0')}`; + case 'day': + return `${y}-${String(m).padStart(2, '0')}-${String(day).padStart(2, '0')}`; + case 'week': + return isoWeekLabelFromCalendarDay(y, m, day); + default: + // Unreachable through `BucketGranularity`. Kept as the same echo + // `@objectstack/objectql`'s `bucketDateValue` has always answered an + // off-type JS caller — this function is that one's delegate, so it must + // not change the answer for any input that already had one. + return String(value); + } +} + /** * ISO-8601 week label (Mon-start weeks, week 1 = the week of the first - * Thursday) of a UTC calendar day. The forward-direction companion used to - * *validate* a reconstructed week boundary; it mirrors the week branch of - * `@objectstack/objectql`'s `bucketDateValue` (kept in lockstep by the - * round-trip parity test in objectql). + * Thursday) of a calendar day given that day's parts (`month` is 1-12). + * + * The ONE statement of the week rule in this package: {@link bucketDateKey}'s + * `week` branch and {@link isoWeekLabelUtc} both call it, so the forward label + * and the round-trip validator that checks it cannot drift apart. */ -function isoWeekLabelUtc(d: Date): string { - const target = new Date(d.getTime()); +function isoWeekLabelFromCalendarDay(year: number, month: number, day: number): string { + const target = new Date(Date.UTC(year, month - 1, day)); const dayNum = (target.getUTCDay() + 6) % 7; // Mon=0..Sun=6 target.setUTCDate(target.getUTCDate() - dayNum + 3); // shift to that week's Thursday const firstThursday = new Date(Date.UTC(target.getUTCFullYear(), 0, 4)); @@ -225,6 +322,14 @@ function isoWeekLabelUtc(d: Date): string { return `${target.getUTCFullYear()}-W${String(weekNo).padStart(2, '0')}`; } +/** + * ISO-8601 week label of a UTC calendar day — the forward-direction companion + * used to *validate* a reconstructed week boundary below. + */ +function isoWeekLabelUtc(d: Date): string { + return isoWeekLabelFromCalendarDay(d.getUTCFullYear(), d.getUTCMonth() + 1, d.getUTCDate()); +} + /** * The half-open calendar span `[start, end)` of a canonical date-bucket KEY, * as `YYYY-MM-DD` strings (`start` inclusive, `end` exclusive — the next diff --git a/packages/drivers/driver-memory/src/filter-refusal.ts b/packages/drivers/driver-memory/src/filter-refusal.ts index fd9491707c..daa595b4ad 100644 --- a/packages/drivers/driver-memory/src/filter-refusal.ts +++ b/packages/drivers/driver-memory/src/filter-refusal.ts @@ -38,6 +38,13 @@ import { ACCEPTED_FILTER_COMPARAND_TYPES_SENTENCE } from '@objectstack/spec/data // [#7536] The `$like` pattern language's shared gate, so this driver refuses // the same malformed patterns as every other face. import { hasDanglingLikeEscape } from '@objectstack/spec/data'; +// [#16178] The canonical bucket-key vocabulary, quoted rather than hand-listed — +// the refusal below names the accepted set from its one definition. +import { BUCKET_GRANULARITIES } from '@objectstack/core'; +// [#16178] and the DECLARED interval vocabulary, quoted the same way, so the +// refusal can tell a value the contract never declared apart from one it +// declares and this backend cannot label. +import { TimeUpdateInterval } from '@objectstack/spec/data'; import { StandardErrorCode } from '@objectstack/spec/api'; /** @@ -92,6 +99,78 @@ export function refusePerAggregationFilter(alias: string): never { throw err; } +/** + * [#16178] A `timeDimensions[].granularity` this backend cannot BUCKET. + * + * `@objectstack/spec`'s `TimeUpdateInterval` declares eight intervals; the + * canonical bucket-key vocabulary (`@objectstack/core`'s + * {@link BUCKET_GRANULARITIES}) defines a label for five of them. The three + * sub-day names — `second`, `minute`, `hour` — have no canonical key anywhere in + * the contract, so there is no label this backend could emit that another + * backend's pushed-down SQL would agree with. + * + * The same NOT_IMPLEMENTED/501 class, and for the same reason, as + * {@link refusePerAggregationFilter} (#5907, ADR-0112): the query is spelled + * correctly and the spec declares the value, and it is this backend that + * compiles no bucket for it — a capability gap, not a mistake in the query. + * + * Refused rather than passed through, because passing it through is #16178's own + * defect wearing a new name: an unbucketed time dimension answers one group per + * distinct timestamp under an ordinary 200, which is a chart with one bar per + * row and no warning anywhere. + * + * ## Two conditions, two answers + * + * "Capability gap" is a claim about THIS BACKEND, and it is only true of a value + * the contract actually declares. A caller past the schema door — `POST + * /analytics/dataset/query` types `selection.timeDimensions` from + * `AnalyticsQuery` and does not Zod-parse it, which is the same door + * `analyticsDateRangeUnrecognizedError` documents — can send `'fortnight'`, + * and answering that with 501 plus a sentence asserting "the spec declares the + * value" tells the caller something false and points them at the backend when + * the mistake is in the query. So the vocabulary is checked FIRST: an + * undeclared spelling is a 400 validation-class refusal, and only a declared + * interval this backend cannot label reaches the 501 arm. Same separation the + * `dateRange` half of this face already draws (#16322 / #16041). + * + * ⚠️ The 400 arm answers the GENERAL `StandardErrorCode.INVALID_QUERY` rather + * than a dedicated `ANALYTICS_GRANULARITY_UNRECOGNIZED` — the shape its + * `dateRange` sibling uses — because a dedicated code has to be registered in + * `packages/spec`'s `error-code-ledger.zod.ts`, which is a contract decision + * and a `domain:spec` card, not a driver patch. Recorded here so the choice is + * visible when that card is written. + */ +export function unsupportedTimeGranularityError(dimension: string, granularity: string): Error { + const declaredIntervals = TimeUpdateInterval.options as readonly string[]; + if (!declaredIntervals.includes(granularity)) { + const outOfVocabulary = new Error( + `Time dimension "${dimension}" asks for granularity "${granularity}", which @objectstack/spec's ` + + `TimeUpdateInterval does not declare — the declared intervals are ` + + `${declaredIntervals.join(', ')}. This is a mistake in the query rather than a gap in this ` + + `backend (driver-memory), so it answers a 400 rather than the 501 a declared-but-unbucketable ` + + `interval gets. Ask for one of ${BUCKET_GRANULARITIES.join(', ')}, which this backend buckets.`, + ) as Error & { code?: string; status?: number }; + outOfVocabulary.code = StandardErrorCode.enum.INVALID_QUERY; + outOfVocabulary.status = 400; + return outOfVocabulary; + } + + const err = new Error( + `Time dimension "${dimension}" asks for granularity "${granularity}", which this backend ` + + `(driver-memory) cannot bucket. The query is spelled correctly and @objectstack/spec's ` + + `TimeUpdateInterval declares the value — but the canonical bucket-key vocabulary defines a ` + + `label only for ${BUCKET_GRANULARITIES.join(', ')}, and a sub-day bucket has no key any ` + + `other backend's pushed-down SQL would agree with. It is refused rather than silently left ` + + // The tracker id stays in this function's doc comment, where a reader who can + // resolve it is looking; a runtime string reaches operators who cannot. + `unbucketed, which answers one group per distinct timestamp. Ask for a coarser ` + + `granularity, or drop the key and group on the raw timestamp deliberately.`, + ) as Error & { code?: string; status?: number }; + err.code = StandardErrorCode.enum.NOT_IMPLEMENTED; + err.status = 501; + return err; +} + /** * [#5158] A `FilterArray` reached the driver unlowered. * diff --git a/packages/drivers/driver-memory/src/memory-analytics-time-granularity.test.ts b/packages/drivers/driver-memory/src/memory-analytics-time-granularity.test.ts new file mode 100644 index 0000000000..9289de6ec8 --- /dev/null +++ b/packages/drivers/driver-memory/src/memory-analytics-time-granularity.test.ts @@ -0,0 +1,397 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +// +// [#16178] `AnalyticsQuery.timeDimensions[].granularity` — declared by the spec, +// enumerated per dimension by the cube (`granularities: ['day']`), and until this +// card read by NEITHER on this face. The `$group` stage keyed on the raw field +// path, so a time dimension answered ONE GROUP PER DISTINCT TIMESTAMP under an +// ordinary 200: one bar per row in a "new accounts by month" chart, which is the +// symptom #3588 catalogued and repaired for `service-analytics`. +// +// The measurement that opens this file is the card's own, and the `no +// granularity` cell beside it is the control: without it, "one group" proves +// nothing, because a fixture that collapses for an unrelated reason reads the +// same. +// +// Orthogonal to #16042/#16179, which repaired the `dateRange` WINDOW (which rows +// are SELECTED) on these same entries. This is the GROUPING (how selected rows +// are FOLDED) — and the two are held to one reference timezone here, since a +// window resolved in one zone and a bucket folded in another is a chart whose +// bars do not add up to its own total. + +import { describe, it, expect } from 'vitest'; +import { InMemoryDriver } from './memory-driver.js'; +import { MemoryAnalyticsService } from './memory-analytics.js'; +import { AnalyticsQuerySchema } from '@objectstack/spec/data'; +import type { AnalyticsQuery, Cube } from '@objectstack/spec/data'; + +/** Every query goes through the schema, the route a real request body takes. */ +const asQuery = (input: AnalyticsQuery): AnalyticsQuery => AnalyticsQuerySchema.parse(input); + +const cubes: Cube[] = [ + { + name: 'events', + title: 'Events', + sql: 'events', + measures: { + count: { name: 'count', label: 'Event Count', type: 'count', sql: 'id' }, + latest: { name: 'latest', label: 'Latest Event', type: 'max', sql: 'created_at' }, + }, + dimensions: { + createdAt: { + name: 'createdAt', + label: 'Created At', + type: 'time', + sql: 'created_at', + granularities: ['day', 'week', 'month', 'quarter', 'year'], + }, + }, + }, +]; + +/** Two rows on ONE UTC calendar day, fourteen hours apart — the card's fixture. */ +const TWO_ROWS_ONE_UTC_DAY = [ + { id: 1, created_at: '2026-09-06T01:00:00.000Z' }, + { id: 2, created_at: '2026-09-06T23:00:00.000Z' }, +]; + +async function query( + input: AnalyticsQuery, + rows: Record[] = TWO_ROWS_ONE_UTC_DAY, +) { + const driver = new InMemoryDriver({ initialData: { events: rows } }); + await driver.connect(); + const service = new MemoryAnalyticsService({ driver, cubes }); + return service.query(asQuery(input)); +} + +const BASE = { + cube: 'events', + measures: ['events.count'], + dimensions: ['events.createdAt'], +} satisfies Partial; + +const labels = (result: { rows: Record[] }) => + result.rows.map((r) => r['events.createdAt']); + +/** + * The in-process door PAST the schema — `AnalyticsQuerySchema` would refuse an + * undeclared spelling itself, so a query that tests what the DRIVER does with + * one must not be parsed first. This is the reachability + * `analyticsDateRangeUnrecognizedError` records for its own out-of-vocabulary + * refusal: `POST /analytics/dataset/query` types `selection.timeDimensions` + * from `AnalyticsQuery` and never Zod-parses them. + */ +async function unparsed(granularity: string) { + const driver = new InMemoryDriver({ initialData: { events: TWO_ROWS_ONE_UTC_DAY } }); + await driver.connect(); + const service = new MemoryAnalyticsService({ driver, cubes }); + return service.query({ + ...BASE, + timeDimensions: [{ dimension: 'events.createdAt', granularity }], + } as unknown as AnalyticsQuery); +} + +describe('[#16178] a time dimension buckets by its declared granularity', () => { + it("folds two rows on one UTC day into ONE group under granularity 'day'", async () => { + const result = await query({ + ...BASE, + timeDimensions: [{ dimension: 'events.createdAt', granularity: 'day' }], + }); + + expect(result.rows).toHaveLength(1); + expect(result.rows[0]).toMatchObject({ 'events.createdAt': '2026-09-06', 'events.count': 2 }); + }); + + it('CONTROL — the same two rows stay two groups when no granularity is asked for', async () => { + // The control that makes the cell above a measurement rather than a + // coincidence: this face still groups on the raw instant when nothing asks + // it to bucket, so "one group" above is the granularity doing the work. + const result = await query({ + ...BASE, + timeDimensions: [{ dimension: 'events.createdAt' }], + }); + + expect(result.rows).toHaveLength(2); + expect(labels(result)).toEqual([ + '2026-09-06T01:00:00.000Z', + '2026-09-06T23:00:00.000Z', + ]); + }); + + it('labels every granularity in the canonical output vocabulary', async () => { + // `DriverCapabilitiesSchema.queryDateGranularity` calls this vocabulary an + // "Output contract (CRITICAL)": a driver that pushes the bucket down into + // SQL emits these exact strings, so a drill-down survives the seam. The week + // label is `YYYY-Www` — never the Monday's `YYYY-MM-DD`. + const expected: Record = { + day: '2026-09-06', + week: '2026-W36', + month: '2026-09', + quarter: '2026-Q3', + year: '2026', + }; + + for (const [granularity, label] of Object.entries(expected)) { + const result = await query({ + ...BASE, + timeDimensions: [{ dimension: 'events.createdAt', granularity: granularity as 'day' }], + }); + expect(labels(result), granularity).toEqual([label]); + } + }); + + it('buckets a member named without its cube prefix in `dimensions`', async () => { + // The two keys are matched on the RESOLVED field path, so `createdAt` and + // `events.createdAt` are one member rather than two. + const result = await query({ + ...BASE, + dimensions: ['createdAt'], + timeDimensions: [{ dimension: 'events.createdAt', granularity: 'day' }], + }); + + expect(result.rows).toHaveLength(1); + expect(result.rows[0]).toMatchObject({ createdAt: '2026-09-06', 'events.count': 2 }); + }); +}); + +describe('[#16178] a granular time dimension is a projected group column on its own', () => { + /** Measures + one granular time dimension, `dimensions` absent entirely. */ + const TREND = { + cube: 'events', + measures: ['events.count'], + timeDimensions: [{ dimension: 'events.createdAt', granularity: 'day' }], + } satisfies Partial; + + it('groups and projects a `timeDimensions` member that `dimensions` never lists', async () => { + // The canonical trend-query shape. Keying `$group` on `query.dimensions` + // alone answered ONE TOTAL for it (`_id: null`) — a chart with a y-value + // and no x-axis, which is the #4033 symptom the SQL/ObjectQL face already + // repaired: `objectql-strategy.ts` groups every granular entry not listed + // in `dimensions` (:163-167) and `projectedDimensions` (:1889-1893) hands + // that one set to grouping, row mapping and field metadata alike. Accepted, + // silent and inert is this card's own defect class, so the two faces agree + // here rather than one of them declaring a key it never reads. + const result = await query(TREND); + + expect(result.rows).toHaveLength(1); + expect(result.rows[0]).toMatchObject({ 'events.createdAt': '2026-09-06', 'events.count': 2 }); + }); + + it('names the projected bucket in `fields`, on the terms the listed shape gets', async () => { + // Rows without their `fields` entry is half of #4033: the x-axis exists in + // the data and not in the metadata beside it. Pinned as AGREEMENT with the + // shape that lists the member, so the two spellings of one query cannot + // drift into two answers. + const projected = await query(TREND); + const listed = await query({ + ...BASE, + timeDimensions: [{ dimension: 'events.createdAt', granularity: 'day' }], + }); + + expect(projected.fields).toEqual(listed.fields); + expect(projected.fields.map((f) => f.name)).toEqual(['events.createdAt', 'events.count']); + }); + + it('CONTROL — a `dateRange`-only entry is NOT projected (#5688)', async () => { + // The rule's other half, and what keeps the cells above from reading as + // "every time dimension becomes a column". An entry carrying only a window + // is a PREDICATE: it selects rows and contributes no group key, so this + // answers one total under a `fields` list that never mentions the member. + const result = await query({ + cube: 'events', + measures: ['events.count'], + timeDimensions: [{ dimension: 'events.createdAt', dateRange: ['2026-09-06', '2026-09-06'] }], + }); + + expect(result.rows).toEqual([{ 'events.count': 2 }]); + expect(result.fields.map((f) => f.name)).toEqual(['events.count']); + }); + + it('keys a member listed BOTH ways exactly once', async () => { + // Matched on the resolved field path, the same way the `dimensions` loop + // already folds a bucketed member — so the unprefixed spelling collides + // with the prefixed one instead of adding a second column. + const result = await query({ + ...BASE, + dimensions: ['createdAt'], + timeDimensions: [{ dimension: 'events.createdAt', granularity: 'day' }], + }); + + expect(result.rows).toHaveLength(1); + expect(result.fields.map((f) => f.name)).toEqual(['createdAt', 'events.count']); + }); +}); + +describe('[#16178] bucketing and the reference timezone', () => { + it('folds in `AnalyticsQuery.timezone` — the zone the dateRange repair resolves against', async () => { + // ⛔ UTC is not the only case. 01:00Z and 23:00Z are one UTC day, but in New + // York the first is still the previous evening, and in Tokyo the second is + // already the next morning — so the SAME two rows answer one group or two + // depending on the reference zone, exactly as a user in that zone reads it. + const utc = await query({ + ...BASE, + timezone: 'UTC', + timeDimensions: [{ dimension: 'events.createdAt', granularity: 'day' }], + }); + expect(labels(utc)).toEqual(['2026-09-06']); + + const newYork = await query({ + ...BASE, + timezone: 'America/New_York', + timeDimensions: [{ dimension: 'events.createdAt', granularity: 'day' }], + }); + expect(labels(newYork).sort()).toEqual(['2026-09-05', '2026-09-06']); + + const tokyo = await query({ + ...BASE, + timezone: 'Asia/Tokyo', + timeDimensions: [{ dimension: 'events.createdAt', granularity: 'day' }], + }); + expect(labels(tokyo).sort()).toEqual(['2026-09-06', '2026-09-07']); + }); + + it('an absent timezone buckets in UTC, the same default the window resolver takes', async () => { + const absent = await query({ + ...BASE, + timeDimensions: [{ dimension: 'events.createdAt', granularity: 'day' }], + }); + expect(labels(absent)).toEqual(['2026-09-06']); + }); + + it('does not disturb the `dateRange` window on the same entry (#16042/#16179)', async () => { + // Both keys on one entry: the window still SELECTS by its own published + // semantics and the granularity FOLDS what survives. + const inWindow = await query({ + ...BASE, + timeDimensions: [ + { dimension: 'events.createdAt', granularity: 'day', dateRange: ['2026-09-06', '2026-09-06'] }, + ], + }); + expect(inWindow.rows).toHaveLength(1); + expect(inWindow.rows[0]).toMatchObject({ 'events.createdAt': '2026-09-06', 'events.count': 2 }); + + const outOfWindow = await query({ + ...BASE, + timeDimensions: [ + { dimension: 'events.createdAt', granularity: 'day', dateRange: ['2026-09-04', '2026-09-04'] }, + ], + }); + expect(outOfWindow.rows).toHaveLength(0); + }); +}); + +describe('[#16178] a sub-day granularity is refused, not dropped', () => { + it.each(['second', 'minute', 'hour'])( + 'refuses %s with the NOT_IMPLEMENTED/501 envelope', + async (granularity) => { + // Asserted on `code` and `status` — the ADR-0112 envelope — never on the + // message text. A bare `toThrow()` would pass against a driver that threw + // for any unrelated reason. + await expect( + query({ + ...BASE, + timeDimensions: [{ dimension: 'events.createdAt', granularity: granularity as 'hour' }], + }), + ).rejects.toMatchObject({ code: 'NOT_IMPLEMENTED', status: 501 }); + }, + ); + + it('refuses on an EMPTY table too, so the refusal is the compile and not the data', async () => { + // An unbucketed query over no rows answers `{rows: []}`; this one still + // refuses, which places the refusal at compile time where the ruling put it. + await expect( + query( + { + ...BASE, + timeDimensions: [{ dimension: 'events.createdAt', granularity: 'hour' }], + }, + [], + ), + ).rejects.toMatchObject({ code: 'NOT_IMPLEMENTED', status: 501 }); + }); + + it('answers 400, not 501, for a granularity the CONTRACT never declared', async () => { + // 501 is a claim about this BACKEND, and it is only honest about a value the + // contract actually declares. `'fortnight'` is a mistake in the query, and + // the 501 sentence asserting "the spec declares the value" would have been + // false of it. Reached the way its `dateRange` sibling documents — past the + // schema door, which is where `POST /analytics/dataset/query` types + // `selection.timeDimensions` without Zod-parsing them — so this query + // deliberately does NOT go through `asQuery`. + await expect(unparsed('fortnight')).rejects.toMatchObject({ + code: 'INVALID_QUERY', + status: 400, + }); + }); + + it('CONTROL — the same unparsed door still answers 501 for a DECLARED interval', async () => { + // Without this the cell above proves only "the unparsed door throws". The + // two answers differ on exactly one thing: whether `TimeUpdateInterval` + // declares the value. + await expect(unparsed('hour')).rejects.toMatchObject({ + code: 'NOT_IMPLEMENTED', + status: 501, + }); + }); +}); + +describe('[#16178] the fold does not corrupt what it is not asked to bucket', () => { + it('leaves a measure over the SAME member ranking instants, not labels', async () => { + // The bucket key travels under a synthetic field rather than overwriting the + // row's own: `created_at` is both the group key and `max()`'s aggregand here, + // and folding it in place would have made `latest` the largest bucket LABEL. + const result = await query({ + cube: 'events', + measures: ['events.count', 'events.latest'], + dimensions: ['events.createdAt'], + timeDimensions: [{ dimension: 'events.createdAt', granularity: 'day' }], + }); + + expect(result.rows).toHaveLength(1); + expect(result.rows[0]).toMatchObject({ + 'events.createdAt': '2026-09-06', + 'events.count': 2, + 'events.latest': '2026-09-06T23:00:00.000Z', + }); + }); + + it('gives null and unparseable instants the one bucket SQL gives them (#3839)', async () => { + const result = await query( + { + ...BASE, + timeDimensions: [{ dimension: 'events.createdAt', granularity: 'day' }], + }, + [ + { id: 1, created_at: null }, + { id: 2, created_at: 'not-a-date' }, + { id: 3, created_at: '2026-09-06T01:00:00.000Z' }, + ], + ); + + expect(result.rows).toHaveLength(2); + expect(result.rows).toEqual( + expect.arrayContaining([ + expect.objectContaining({ 'events.createdAt': null, 'events.count': 2 }), + expect.objectContaining({ 'events.createdAt': '2026-09-06', 'events.count': 1 }), + ]), + ); + }); + + it('buckets an epoch-millis row with an ISO row (#3773)', async () => { + // The in-memory table holds whatever the writer produced; a driver handing + // back raw storage values yields numbers. Both must land in one bucket. + const result = await query( + { + ...BASE, + timeDimensions: [{ dimension: 'events.createdAt', granularity: 'day' }], + }, + [ + { id: 1, created_at: Date.parse('2026-09-06T01:00:00.000Z') }, + { id: 2, created_at: '2026-09-06T23:00:00.000Z' }, + ], + ); + + expect(result.rows).toHaveLength(1); + expect(result.rows[0]).toMatchObject({ 'events.createdAt': '2026-09-06', 'events.count': 2 }); + }); +}); diff --git a/packages/drivers/driver-memory/src/memory-analytics.ts b/packages/drivers/driver-memory/src/memory-analytics.ts index 550c431bae..8502857779 100644 --- a/packages/drivers/driver-memory/src/memory-analytics.ts +++ b/packages/drivers/driver-memory/src/memory-analytics.ts @@ -15,11 +15,24 @@ import { // the ONE refusal for a string outside it, shared with the SQL analytics // path so the two backends cannot answer one input differently again. resolveAnalyticsDateRangeString, + // [#16178] The ONE forward bucket labeller, and the guard that says which + // granularities it can label. Hoisted into core precisely so this driver can + // bucket with the SAME rule the objectql aggregation path uses, without a + // driver depending on objectql and without a third hand copy of the labels. + bucketDateKey, + isBucketGranularity, + type BucketGranularity, } from '@objectstack/core'; +// [#16178] The pipeline below is split at its `$group` when a time dimension +// buckets, so the bucket key can be folded in JS between the two halves — mingo +// has no expression that produces the canonical labels, and writing one would +// be the second dialect this repair exists to avoid. +import { Aggregator } from 'mingo'; import { assertFilterConditionShape, uncompilableCombinatorError, uncompilableFieldOperatorError, + unsupportedTimeGranularityError, type FilterFaceCapabilities, } from './filter-refusal.js'; @@ -624,6 +637,53 @@ export interface MemoryAnalyticsConfig { logger?: Logger; } +/** + * [#16178] A `timeDimensions[]` entry that asks its dimension to be BUCKETED, + * resolved to everything the fold needs. + * + * `granularity` is already narrowed to the five the canonical vocabulary can + * label — the three sub-day names `TimeUpdateInterval` also declares are refused + * at compile, before this is built. + */ +interface TimeBucket { + /** The member as the CALLER spelled it, which is how a projected bucket is named back. */ + readonly dimension: string; + /** The row field the instant is read from. */ + readonly fieldPath: string; + /** The bucket size, narrowed to what `bucketDateKey` can label. */ + readonly granularity: BucketGranularity; + /** The synthetic field the bucket key is written to. */ + readonly bucketKey: string; +} + +/** + * The synthetic field a dimension's bucket key travels under. + * + * Synthetic rather than an overwrite of the source field, because one member can + * be both a group key and a measure's aggregand: folding `created_at` in place + * would leave `max(created_at)` ranking `'2026-W23'` strings. The `$` prefix a + * mingo expression adds is applied by the caller, so the name itself carries + * none; the double underscore keeps it clear of any real column. + */ +function bucketFieldFor(dimName: string): string { + return `__bucket__${dimName}`; +} + +/** + * Read a row value at a resolved field path, dotted paths included — the same + * traversal mingo performs for the `$` the `$group` stage would have used, + * so a nested dimension buckets from the value it would have grouped on. + */ +function readFieldPath(row: Record, fieldPath: string): unknown { + if (!fieldPath.includes('.')) return row[fieldPath]; + let cursor: any = row; + for (const segment of fieldPath.split('.')) { + if (cursor == null || typeof cursor !== 'object') return undefined; + cursor = cursor[segment]; + } + return cursor; +} + /** * [#16179] A `timeDimensions[].dateRange` resolved to its two bounds, together * with what the UPPER one means. @@ -775,9 +835,35 @@ export class MemoryAnalyticsService implements IAnalyticsService { } // Stage 2: Time dimension filters + // + // [#16178] and their GRANULARITY, which this face used to accept and never + // read. The two keys are orthogonal and both live on the same entry: + // `dateRange` decides WHICH rows are selected (#16042/#16179), `granularity` + // decides how the selected rows are FOLDED. Collected here, applied between + // the `$match` half of the pipeline and its `$group` (see + // {@link aggregateWithTimeBuckets}). + const timeBuckets: TimeBucket[] = []; if (query.timeDimensions && query.timeDimensions.length > 0) { for (const timeDim of query.timeDimensions) { const fieldPath = this.resolveFieldPath(cube, timeDim.dimension); + if (timeDim.granularity !== undefined) { + // Refused, not dropped: `TimeUpdateInterval` declares three sub-day + // names the canonical bucket-key vocabulary has no label for, and + // passing one through is this card's own defect under a new name. + if (!isBucketGranularity(timeDim.granularity)) { + throw unsupportedTimeGranularityError(timeDim.dimension, timeDim.granularity); + } + // The bucket travels under its own synthetic key rather than + // overwriting the row's field: the SAME member can be both a group key + // and a measure's aggregand (`max(created_at)`), and folding the field + // in place would silently rank bucket LABELS instead of instants. + timeBuckets.push({ + dimension: timeDim.dimension, + fieldPath, + granularity: timeDim.granularity, + bucketKey: bucketFieldFor(this.getShortName(timeDim.dimension)), + }); + } if (timeDim.dateRange) { // [#16179] The union's two arms are discriminated HERE, and the // answer travels the two lines down to the bound construction rather @@ -848,13 +934,48 @@ export class MemoryAnalyticsService implements IAnalyticsService { const groupStage: Record = { _id: {} }; // Add dimensions to _id + const keyedBucketPaths = new Set(); if (query.dimensions && query.dimensions.length > 0) { for (const dim of query.dimensions) { const fieldPath = this.resolveFieldPath(cube, dim); const dimName = this.getShortName(dim); - groupStage._id[dimName] = `$${fieldPath}`; + // [#16178] A dimension that a time dimension buckets keys on the FOLDED + // value. Matched on the resolved field path, so `createdAt` in + // `dimensions` and `events.createdAt` in `timeDimensions` are one member. + const bucketed = timeBuckets.find(b => b.fieldPath === fieldPath); + if (bucketed) keyedBucketPaths.add(bucketed.fieldPath); + groupStage._id[dimName] = bucketed ? `$${bucketed.bucketKey}` : `$${fieldPath}`; } - } else { + } + + // [#16178] A GRANULAR time dimension is a group column in its own right, + // whether or not `dimensions` also lists it. Keying `$group` on + // `query.dimensions` alone answered ONE TOTAL (`_id: null`) for the + // canonical trend shape — `{measures, timeDimensions:[{dimension, + // granularity}]}` with no `dimensions` — so the granularity was accepted, + // silent and inert: this card's own defect class under a different name. + // + // The rule and its exception are the SQL/ObjectQL face's, recorded there: + // every granular entry not already listed groups and projects + // (`objectql-strategy.ts` :163-167), and one set — `projectedDimensions` + // (:1889-1893) — feeds grouping, row mapping and field metadata alike, + // because rows carrying a bucket under a `fields` list that never mentions + // it is a trend chart with no x-axis (#4033). An entry carrying only a + // `dateRange` is a PREDICATE and is NOT projected (#5688) — which needs no + // test here, since `timeBuckets` only ever admits an entry that declared a + // granularity. + // + // Deduped on the resolved field path, the same way the loop above folds a + // bucketed member, so two spellings of one member cannot become two columns. + const projectedBuckets: TimeBucket[] = []; + for (const bucket of timeBuckets) { + if (keyedBucketPaths.has(bucket.fieldPath)) continue; + keyedBucketPaths.add(bucket.fieldPath); + projectedBuckets.push(bucket); + groupStage._id[this.getShortName(bucket.dimension)] = `$${bucket.bucketKey}`; + } + + if (Object.keys(groupStage._id).length === 0) { groupStage._id = null; // No grouping, aggregate all } @@ -881,6 +1002,10 @@ export class MemoryAnalyticsService implements IAnalyticsService { projectStage[dimName] = `$_id.${dimName}`; } } + for (const bucket of projectedBuckets) { + const dimName = this.getShortName(bucket.dimension); + projectStage[dimName] = `$_id.${dimName}`; + } if (query.measures && query.measures.length > 0) { for (const measure of query.measures) { const measureName = this.getShortName(measure); @@ -917,7 +1042,12 @@ export class MemoryAnalyticsService implements IAnalyticsService { // Execute the aggregation pipeline const tableName = this.extractTableName(cube.sql); - const rawRows = await this.driver.aggregate(tableName, pipeline); + // [#16178] Unbucketed queries keep the single-call path they always had, + // byte for byte; only a query that actually asks for a granularity pays the + // split. + const rawRows = timeBuckets.length === 0 + ? await this.driver.aggregate(tableName, pipeline) + : await this.aggregateWithTimeBuckets(tableName, pipeline, timeBuckets, query.timezone); // [#6814] `$addToSet` COLLECTS; a `count_distinct` measure has to ANSWER a // number. Without this step the value reached the caller as the raw array @@ -948,6 +1078,13 @@ export class MemoryAnalyticsService implements IAnalyticsService { } } } + // [#16178] and a granular time dimension `dimensions` never listed. + for (const bucket of projectedBuckets) { + const shortName = this.getShortName(bucket.dimension); + if (shortName in row) { + renamedRow[bucket.dimension] = row[shortName]; + } + } // Rename measures if (query.measures) { @@ -974,6 +1111,19 @@ export class MemoryAnalyticsService implements IAnalyticsService { }); } } + + // [#16178] On the declared type, not on `string`: the value is a bucket + // LABEL either way, and the shape that DOES list the member has always + // answered the member's own type for exactly that folded value. Two + // spellings of one query answer one `fields` list — the same choice the + // ObjectQL face records at `buildFieldMeta`. + for (const bucket of projectedBuckets) { + const dimension = this.resolveDimension(cube, bucket.dimension); + fields.push({ + name: bucket.dimension, + type: dimension?.type || 'string' + }); + } if (query.measures) { for (const measure of query.measures) { @@ -994,6 +1144,52 @@ export class MemoryAnalyticsService implements IAnalyticsService { }; } + /** + * [#16178] Run a pipeline whose time dimensions BUCKET, folding the bucket key + * in between the pipeline's two halves. + * + * The fold has to happen in JavaScript. mingo has no expression that produces + * the canonical bucket keys (`2026-Q2`, `2026-W23`) and building one out of + * `$isoWeek`/`$concat` would be a SECOND implementation of the label rule — + * exactly the divergence `checkDateBucketParity` exists to catch, and exactly + * what hoisting `bucketDateKey` into `@objectstack/core` was ruled to avoid. + * So the pipeline is cut at its `$group`: the `$match` half still runs in the + * driver (which is where the rows live, and which is where the tenancy guard + * sits), the bucket keys are written onto the selected rows, and the grouping + * half runs over those rows with the same mingo the driver would have used. + * + * `timezone` is `AnalyticsQuery.timezone` — the SAME reference zone + * `parseDateRangeString` resolves a `dateRange` preset against, so the window + * that selects the rows and the bucket that folds them agree on where a + * calendar day starts. Unset means UTC, on both. + */ + private async aggregateWithTimeBuckets( + tableName: string, + pipeline: Record[], + timeBuckets: readonly TimeBucket[], + timezone?: string, + ): Promise[]> { + const groupIndex = pipeline.findIndex(stage => '$group' in stage); + // Stage 3 pushes `$group` unconditionally, so this cannot miss. Stated as a + // throw rather than left to a `-1` slicing the pipeline inside out. + if (groupIndex < 0) { + throw new Error( + 'Analytics pipeline carries no $group stage to fold a time bucket into (driver-memory).', + ); + } + const selected = await this.driver.aggregate(tableName, pipeline.slice(0, groupIndex)); + for (const row of selected) { + for (const bucket of timeBuckets) { + row[bucket.bucketKey] = bucketDateKey( + readFieldPath(row, bucket.fieldPath), + bucket.granularity, + timezone, + ); + } + } + return new Aggregator(pipeline.slice(groupIndex)).run(selected) as Record[]; + } + /** * Get available cube metadata for discovery */ diff --git a/packages/objectql/src/in-memory-aggregation.test.ts b/packages/objectql/src/in-memory-aggregation.test.ts index 7c87b76f77..956e3f1deb 100644 --- a/packages/objectql/src/in-memory-aggregation.test.ts +++ b/packages/objectql/src/in-memory-aggregation.test.ts @@ -2,6 +2,7 @@ import { describe, it, expect } from 'vitest'; import { applyInMemoryAggregation, bucketDateValue } from './in-memory-aggregation.js'; +import { bucketDateKey, BUCKET_GRANULARITIES } from '@objectstack/core'; const rows = [ { region: 'East', closed_at: '2024-01-15', amount: 100, owner: 'alice' }, @@ -492,3 +493,41 @@ describe('applyInMemoryAggregation — per-aggregation filter (#10576)', () => { expect(out).toEqual([{ not_won: 50 }]); }); }); + +// ── [#16178] The delegation, pinned ────────────────────────────────────────── +// +// `bucketDateValue` is now a thin delegate to `@objectstack/core`'s +// `bucketDateKey`, hoisted so `driver-memory`'s analytics face can bucket with +// the SAME rule without a driver depending on objectql. This suite's cells above +// already pin the labels; these pin that the two functions cannot come apart — +// re-inlining a private copy here (the drift `checkDateBucketParity` exists to +// catch, one layer earlier) turns them red. +describe('bucketDateValue delegates to the one core labeller (#16178)', () => { + const instants = [ + '2024-05-15T00:00:00Z', + '2024-01-01T00:00:00Z', + '2024-12-30T00:00:00Z', // ISO week rolls into the next year + '2024-02-29T23:30:00Z', // near a tz day boundary + Date.parse('2026-01-10T08:30:00Z'), // epoch millis (#3773) + null, // the empty bucket (#3839) + 'not-a-date', + ]; + + it('answers exactly what bucketDateKey answers, across granularity × timezone', () => { + for (const g of BUCKET_GRANULARITIES) { + for (const tz of [undefined, 'UTC', 'America/New_York', 'Asia/Tokyo']) { + for (const value of instants) { + expect(bucketDateValue(value, g, tz)).toBe(bucketDateKey(value, g, tz)); + } + } + } + }); + + it('still emits the canonical week label, which is the half drivers push down', () => { + // `YYYY-Www` — NOT the Monday's `YYYY-MM-DD`. A driver that buckets a week + // in SQL emits this string; a second spelling breaks a drill-down across + // the seam. + expect(bucketDateValue('2024-01-01', 'week')).toBe('2024-W01'); + expect(bucketDateValue('2026-09-06T01:00:00Z', 'week')).toBe('2026-W36'); + }); +}); diff --git a/packages/objectql/src/in-memory-aggregation.ts b/packages/objectql/src/in-memory-aggregation.ts index f12515f615..0d522f0800 100644 --- a/packages/objectql/src/in-memory-aggregation.ts +++ b/packages/objectql/src/in-memory-aggregation.ts @@ -84,7 +84,7 @@ // every entry: labels fell back to raw ids, and cross-object rebucketing filed // every row under `'(restricted)'` while the grand total still reconciled. -import { calendarPartsInTzOrUtc } from '@objectstack/core'; +import { bucketDateKey } from '@objectstack/core'; import type { QueryAST, GroupByNode, AggregationNode, DateGranularityValue } from '@objectstack/spec/data'; import { matchesAggregationFilter } from './having-filter.js'; @@ -273,12 +273,20 @@ function toNumber(v: any): number { * Bucket a date-like value into an ISO-formatted period label. Weeks start * Monday and use ISO week numbering. * - * ⚠️ **This is one of two implementations of the same contract.** A driver that - * advertises `supports.queryDateGranularity[g]` buckets that granularity in SQL - * instead, and `engine.aggregate` picks between them per query — so a label - * produced here must equal the label that driver's SQL produces for the same - * instant, or a drill-down breaks when it crosses the seam. Editing the labels - * below means editing every driver's bucket expression too. + * ⚠️ **The label rule itself no longer lives here.** It is + * `@objectstack/core`'s `bucketDateKey`, and this function is a thin delegate + * with its export name and signature unchanged — the hoist ruled on #16178, so + * that `driver-memory`'s analytics face can bucket with the SAME labeller + * without a driver taking a dependency on objectql and without a third hand + * copy of the rule. Read `bucketDateKey`'s header for the output-contract + * vocabulary, the timezone semantics, the epoch-millis branch (#3773) and the + * null/unparseable bucket (#3839). + * + * ⚠️ **This is still one of two implementations of the same contract.** A driver + * that advertises `supports.queryDateGranularity[g]` buckets that granularity in + * SQL instead, and `engine.aggregate` picks between them per query — so the + * label produced through here must equal the label that driver's SQL produces + * for the same instant, or a drill-down breaks when it crosses the seam. * * The seam is enforced by `checkDateBucketParity` (@objectstack/verify), run * against the real drivers in @@ -286,62 +294,11 @@ function toNumber(v: any): number { * driver test files also hand-copy this function for self-containment; those * copies cannot detect their own drift, which is why the executable check * exists. - * - * `timezone` (ADR-0053 Phase 2) resolves the calendar day in a reference zone - * so an instant near a tz day-boundary buckets where a user in that zone would - * expect. An unset / `'UTC'` / invalid zone keeps the historical UTC bucketing. - * The y/m/d are taken in the reference zone and the ISO-week math then runs on - * a UTC date built from those parts — the parts already carry the zone shift, - * so the week boundary lands correctly without re-applying any offset. - * - * 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 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 | 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; - const { year: y, month: m, day } = calendarPartsInTzOrUtc(d, timezone); - switch (granularity) { - case 'year': - return String(y); - case 'quarter': - return `${y}-Q${Math.floor((m - 1) / 3) + 1}`; - case 'month': - return `${y}-${String(m).padStart(2, '0')}`; - case 'day': - return `${y}-${String(m).padStart(2, '0')}-${String(day).padStart(2, '0')}`; - case 'week': { - // ISO-8601 week date: week 1 contains the first Thursday of the year. - const target = new Date(Date.UTC(y, m - 1, day)); - const dayNum = (target.getUTCDay() + 6) % 7; // Mon=0..Sun=6 - target.setUTCDate(target.getUTCDate() - dayNum + 3); - const firstThursday = new Date(Date.UTC(target.getUTCFullYear(), 0, 4)); - const weekNo = 1 + Math.round( - ((target.getTime() - firstThursday.getTime()) / 86400000 - 3 + ((firstThursday.getUTCDay() + 6) % 7)) / 7, - ); - return `${target.getUTCFullYear()}-W${String(weekNo).padStart(2, '0')}`; - } - default: - return String(value); - } + return bucketDateKey(value, granularity, timezone); }