From 13d7898371cbf846879149ec6f9f66466378a79e Mon Sep 17 00:00:00 2001 From: Claude Fable 5 Date: Fri, 7 Aug 2026 10:58:36 +0000 Subject: [PATCH] =?UTF-8?q?fix(service-analytics):=20compareTo=20=E6=8A=8A?= =?UTF-8?q?=E6=AF=94=E8=BE=83=E6=A1=B6=E9=94=AE=E5=B9=B3=E7=A7=BB=E5=9B=9E?= =?UTF-8?q?=E5=BD=93=E6=9C=9F,=E8=AE=A9=E6=97=A5=E6=9C=9F=E7=BB=B4?= =?UTF-8?q?=E5=BA=A6=E5=8D=B3=E7=BD=91=E6=A0=BC=E7=BB=B4=E5=BA=A6=E6=97=B6?= =?UTF-8?q?=E7=9A=84=E5=90=8C=E6=AF=94=E7=9C=9F=E6=AD=A3=E5=AF=B9=E9=BD=90?= =?UTF-8?q?=20(#6007)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 趋势图 + 同比是 compareTo 最常见的形状:日期维度既写进 selection.dimensions (它就是图表的时间轴),又被 compareTo 用作锚点。这个形状下比较趟从来没有对齐过。 比较趟查询的是**平移后**的窗口,所以它的行按平移后的桶键落地;而 mergeByDimensions 按 selection.dimensions 元组建键 —— 2025-01 不等于 2026-01,于是没有一条比较行合并 得进去,全部作为新行追加。两趟各自只报告了自己那一半,fillEmptyGroups 把另一半填成自信 的 0,再加上平移后的桶键坐在网格里,而它们落在调用方筛选窗口之外。一个 2 桶窗口回来是 4 行、每行一个 0、两行在窗口外。 按维护者 2026-08-07 裁决(方向 1)实施:合并之前,把每个比较桶键用当期的说法重述一遍。 - previousYear —— 窗口是按日历年平移的,逆运算就是按日历年往前推一年:对桶自己的首日 平移再重新分桶。刻意是 shiftRange 那套年运算的精确逆运算(含 setUTCFullYear 溢出), 窗口与桶键因此不可能对「一年」有两种理解。 - previousPeriod —— 任意天数窗口没有日历对应物,按桶序(bucket ordinal)对齐:上一窗口 的第 n 个桶对上本窗口的第 n 个桶。序号由日历算出而不是数组下标,所以本期网格里的空档 不会让其后每个桶都错位一格。 响应形状不变(仍是 __compare 列),objectui#3337 不受影响。 不确定时一律保持改动前的行为而不是猜:空桶(键为 null,两趟本来就互相合并)、未分桶的日期 维度(分组的是原始时间戳)、平移回来落在当期窗口之外的桶。范围限定在锚点既是网格维度又被 分桶的形状;两趟通过同一个 granularityOf 读桶大小,重述的桶大小按构造即分组用的桶大小。 bucketKeyAtOrdinal 是本包唯一自己铸造桶键的地方,它铸出的键要和运行时 GROUP BY 产出的 键逐字节相等,所以对 @objectstack/core 的 bucketKeyToCalendarRange(同一套词汇的规范 逆函数)做了 55 例往返钉,而不是靠眼看。 dataset-window-timedimension-bucketing.test.ts 的「bucketed anchor」用例按裁决有意 翻红并改写为成对的 2×2;新增 dataset-compare-bucket-alignment.test.ts,其 fake 按 filter 真的分桶(旧 pin 的 fake 两趟返回同一固定桶键,合并必然成功,够不到这一层)。 Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_015a5qkLzpGXhLL2F5gvJ7dD --- .changeset/compare-to-bucket-alignment.md | 46 ++ .../dataset-compare-bucket-alignment.test.ts | 698 ++++++++++++++++++ ...set-window-timedimension-bucketing.test.ts | 52 +- .../service-analytics/src/dataset-executor.ts | 242 +++++- 4 files changed, 1020 insertions(+), 18 deletions(-) create mode 100644 .changeset/compare-to-bucket-alignment.md create mode 100644 packages/services/service-analytics/src/__tests__/dataset-compare-bucket-alignment.test.ts diff --git a/.changeset/compare-to-bucket-alignment.md b/.changeset/compare-to-bucket-alignment.md new file mode 100644 index 0000000000..f978c3832d --- /dev/null +++ b/.changeset/compare-to-bucket-alignment.md @@ -0,0 +1,46 @@ +--- +"@objectstack/service-analytics": patch +--- + +fix(service-analytics): `compareTo` 在「日期维度本身就是网格维度」时把比较桶键平移回当期 (#6007) + +趋势图 + 同比是 `compareTo` 最常见的形状:日期维度既写进 `selection.dimensions` +(它就是图表的时间轴),又被 `compareTo` 用作锚点。这个形状下比较趟从来没有对齐过。 + +比较趟查询的是**平移后**的窗口,所以它的行按平移后的桶键落地;而 +`mergeByDimensions` 按 `selection.dimensions` 元组建键 —— `2025-01` 不等于 +`2026-01`,于是**没有一条**比较行合并得进去,全部作为新行追加。两趟各自只报告了自己 +那一半,`fillEmptyGroups` 把另一半填成自信的 `0`,再加上平移后的桶键坐在网格里,而它们 +落在调用方筛选窗口之外。一个 2 桶窗口的「今年 vs 去年同期」回来是这样的: + +``` +[{"close_date":"2025-01","opp_count__compare":5,"opp_count":0}, + {"close_date":"2025-02","opp_count__compare":7,"opp_count":0}, + {"close_date":"2026-01","opp_count":1,"opp_count__compare":0}, + {"close_date":"2026-02","opp_count":2,"opp_count__compare":0}] +``` + +四行、每行一个 0、两行在窗口外;期望是 2 行 × 2 列。 + +**修法(维护者裁决 2026-08-07,方向 1):合并之前,把每个比较桶键用当期的说法重述一遍。** +上例现在返回 `[{close_date:'2026-01',opp_count:1,opp_count__compare:5}, +{close_date:'2026-02',opp_count:2,opp_count__compare:7}]`。 + +- `previousYear` —— 窗口是按日历年平移的,所以逆运算就是按日历年往前推一年:对桶自己的 + 首日做平移再重新分桶。`2025-01` → `2026-01`、`2025-Q1` → `2026-Q1`、 + `2025-W03` → `2026-W03`。它刻意是 `shiftRange` 那套年运算的精确逆运算(含 + `setUTCFullYear` 的溢出行为),窗口与桶键因此不可能对「一年」有两种理解。 +- `previousPeriod` —— 任意天数窗口没有日历对应物,所以按**桶序(bucket ordinal)**对齐: + 上一窗口的第 n 个桶对上本窗口的第 n 个桶,n 各自从自己窗口的起点数起。序号由**日历**算出 + 而不是数组下标,所以本期网格里某个桶没有数据(存在空档)不会让其后每个桶都错位一格。 + +**响应形状不变** —— 仍然是 `__compare` 列,行仍然是网格维度元组,所以消费端 +(objectui#3337 正在收敛的那条契约)不受影响。 + +不确定时一律**保持原样**(即改动前的行为),而不是猜:空桶(两条聚合路径上键都是 `null`, +两趟本来就互相合并)、未分桶的日期维度(分组的是原始时间戳,不是桶键)、以及平移回来落在 +当期窗口之外的桶(两个等长的天数窗口可以切出不同的桶数)。 + +范围严格限定在坏掉的那个形状:锚点必须是**网格维度**(仅作窗口的锚点两趟都不是列,#5688 +之后本来就对齐)且必须**被分桶**。两趟通过同一个 `granularityOf` 读取桶大小,所以这里重述 +的桶大小按构造就是查询分组用的桶大小。 diff --git a/packages/services/service-analytics/src/__tests__/dataset-compare-bucket-alignment.test.ts b/packages/services/service-analytics/src/__tests__/dataset-compare-bucket-alignment.test.ts new file mode 100644 index 0000000000..73e22471ec --- /dev/null +++ b/packages/services/service-analytics/src/__tests__/dataset-compare-bucket-alignment.test.ts @@ -0,0 +1,698 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #6007 — `compareTo` when the date dimension IS the grid dimension. + * + * A trend chart with a year-over-year comparison selects the date as a grid + * dimension AND anchors `compareTo` on it. The comparison pass then groups the + * SHIFTED window, so its rows key to shifted buckets, while `mergeByDimensions` + * keys on `selection.dimensions` — `2025-01` is not `2026-01`, so not one + * comparison row merged. Every one was appended, `fillEmptyGroups` filled the + * half each pass never reported, and a 2-bucket window came back as: + * + * ``` + * [{"close_date":"2025-01","opp_count__compare":5,"opp_count":0}, + * {"close_date":"2025-02","opp_count__compare":7,"opp_count":0}, + * {"close_date":"2026-01","opp_count":1,"opp_count__compare":0}, + * {"close_date":"2026-02","opp_count":2,"opp_count__compare":0}] + * ``` + * + * Four rows, a confident `0` on each, and two of them keyed OUTSIDE the window + * the caller filtered to. The maintainer's ruling (2026-08-07, direction 1): + * shift the comparison bucket key back onto the current period, leaving the + * response SHAPE untouched (still `__compare` columns, so objectui#3337 + * is unaffected). `previousYear` shifts back by a calendar year; `previousPeriod` + * — an arbitrary-length day window with no calendar counterpart — aligns by + * **bucket ordinal**, the n-th bucket of the previous window onto the n-th + * bucket of this one. + * + * ## Why the old pin could not have caught this + * + * `dataset-selection-window.test.ts`'s "buckets the compareTo pass identically" + * case has a fake that returns `created_at: '2026-06'` for BOTH passes, + * regardless of the filter. Its merge therefore succeeded by construction. What + * it genuinely pins — that the two passes resolve the same GROUP BY granularity + * (#4870) — it pins well, and it stays green here; but no fake that ignores the + * window can reach the layer below it, where real data buckets into the keys of + * the window it was actually filtered to. + * + * So every fake in this file **buckets for real**: it applies the lowered + * `filter` to a row set and groups what survives, exactly as a `GROUP BY` would, + * which is what makes the shifted pass return shifted keys at all. + */ + +import { describe, it, expect } from 'vitest'; +import { DatasetSchema } from '@objectstack/spec/ui'; +import type { ExecutionContext } from '@objectstack/spec/kernel'; +import { bucketKeyToCalendarRange } from '@objectstack/core'; +import { AnalyticsService } from '../analytics-service.js'; +import { + alignedCompareBucketKey, + bucketKeyAtOrdinal, + bucketOrdinalOfDay, + type DateGranularityValue, +} from '../dataset-executor.js'; + +const CTX = { tenantId: 'org_A' } as ExecutionContext; + +// ── fixture ───────────────────────────────────────────────────────────────── + +const dataset = DatasetSchema.parse({ + name: 'opportunity_metrics', + label: 'Opportunity Metrics', + object: 'opportunity', + include: [], + dimensions: [ + { name: 'owner', field: 'owner_id', type: 'lookup', label: 'Owner' }, + { name: 'close_date', field: 'close_date', type: 'date', label: 'Close Date', dateGranularity: 'month' }, + // The control: a date dimension declaring NO granularity, so nothing buckets + // it and the grid groups raw values. + { name: 'created_at', field: 'created_at', type: 'date', label: 'Created' }, + ], + measures: [ + { name: 'opp_count', aggregate: 'count', label: 'Opps' }, + { name: 'won_count', aggregate: 'count', label: 'Won', filter: { stage: 'won' } }, + ], +}); + +interface Opp { + owner_id: string; + close_date: string; + created_at: string; + stage: string; +} + +/** `n` opportunities on `day`, alternating owners, all won unless stated. */ +function opps(day: string, n: number, stage = 'won'): Opp[] { + return Array.from({ length: n }, (_, i) => ({ + owner_id: i % 2 === 0 ? 'u1' : 'u2', + close_date: day, + created_at: day, + stage, + })); +} + +type GroupByItem = string | { field: string; dateGranularity?: string }; +type Aggregation = { field: string; method: string; alias: string }; +type AggOptions = { groupBy?: unknown; aggregations?: unknown; filter?: unknown }; + +function matches(row: Record, filter: unknown): boolean { + if (filter == null || typeof filter !== 'object') return true; + for (const [key, cond] of Object.entries(filter as Record)) { + if (key === '$and') { + if (!(cond as unknown[]).every((c) => matches(row, c))) return false; + continue; + } + if (key === '$or') { + if (!(cond as unknown[]).some((c) => matches(row, c))) return false; + continue; + } + const value = row[key]; + if (cond != null && typeof cond === 'object' && !Array.isArray(cond)) { + const ops = cond as Record; + if ('$gte' in ops && !(String(value) >= String(ops.$gte))) return false; + if ('$lte' in ops && !(String(value) <= String(ops.$lte))) return false; + continue; + } + if (value !== cond) return false; + } + return true; +} + +/** + * Canonical bucket keys for the granularities this fixture drives end to end. + * + * ISO weeks are deliberately absent: reproducing that arithmetic in a fixture + * would only prove the fixture agrees with the implementation. The week cases + * use a scripted fake whose hand-written keys are validated against + * `bucketKeyToCalendarRange` — `@objectstack/core`'s canonical inverse — inside + * the test itself. + */ +function bucketOf(raw: unknown, granularity?: string): unknown { + const s = String(raw); + switch (granularity) { + case 'year': + return s.slice(0, 4); + case 'quarter': + return `${s.slice(0, 4)}-Q${Math.floor((Number(s.slice(5, 7)) - 1) / 3) + 1}`; + case 'month': + return s.slice(0, 7); + case 'day': + return s.slice(0, 10); + default: + return raw; + } +} + +/** + * The ObjectQL-aggregate service over a REAL row set: the lowered `filter` is + * applied, the survivors are grouped by the lowered `groupBy` (date items at + * their stated granularity), and each aggregation is computed over the group. + * This is what makes the comparison pass come back keyed to the shifted window. + */ +function svcOver(rows: Opp[]) { + const calls: AggOptions[] = []; + const service = new AnalyticsService({ + queryCapabilities: () => ({ nativeSql: false, objectqlAggregate: true, inMemory: false }), + executeAggregate: async (_object: string, options: Record) => { + const opts = options as AggOptions; + calls.push(opts); + const groupBy = (opts.groupBy ?? []) as GroupByItem[]; + const aggregations = (opts.aggregations ?? []) as Aggregation[]; + const groups = new Map; rows: Opp[] }>(); + for (const row of rows) { + if (!matches(row as unknown as Record, opts.filter)) continue; + const key: Record = {}; + for (const g of groupBy) { + const field = typeof g === 'string' ? g : g.field; + key[field] = bucketOf( + (row as unknown as Record)[field], + typeof g === 'string' ? undefined : g.dateGranularity, + ); + } + const id = JSON.stringify(groupBy.map((g) => key[typeof g === 'string' ? g : g.field] ?? null)); + let group = groups.get(id); + if (!group) { + group = { key, rows: [] }; + groups.set(id, group); + } + group.rows.push(row); + } + return [...groups.values()].map(({ key, rows: members }) => { + const out: Record = { ...key }; + for (const a of aggregations) out[a.alias] = members.length; + return out; + }); + }, + }); + return { service, calls }; +} + +const groupByOf = (call: AggOptions) => (call.groupBy ?? []) as GroupByItem[]; + +// ── 1) the issue, verbatim ────────────────────────────────────────────────── + +/** The issue's own numbers: 2026-01 → 1, 2026-02 → 2, 2025-01 → 5, 2025-02 → 7. */ +const ISSUE_ROWS: Opp[] = [ + ...opps('2026-01-05', 1), + ...opps('2026-02-11', 2), + ...opps('2025-01-07', 5), + ...opps('2025-02-03', 7), +]; + +describe('#6007 — the comparison bucket key is restated in current-period terms', () => { + it('the issue verbatim: 2 rows × 2 columns, not 4 rows each holding one 0', async () => { + const { service, calls } = svcOver(ISSUE_ROWS); + const result = await service.queryDataset( + dataset, + { + dimensions: ['close_date'], + measures: ['opp_count'], + timeDimensions: [{ dimension: 'close_date', dateRange: ['2026-01-01', '2026-02-28'] }], + compareTo: { kind: 'previousYear', dimension: 'close_date' }, + }, + CTX, + ); + // Both passes still bucket by month (#4870), and the comparison pass really + // did query the shifted window — the precondition that made the keys differ. + expect(groupByOf(calls[0])).toEqual([{ field: 'close_date', dateGranularity: 'month' }]); + expect(groupByOf(calls[1])).toEqual([{ field: 'close_date', dateGranularity: 'month' }]); + expect(calls[1].filter).toMatchObject({ close_date: { $gte: '2025-01-01', $lte: '2025-02-28' } }); + + expect(result.rows).toEqual([ + { close_date: '2026-01', opp_count: 1, opp_count__compare: 5 }, + { close_date: '2026-02', opp_count: 2, opp_count__compare: 7 }, + ]); + }); + + it('no row is keyed outside the window the caller filtered to', async () => { + const { service } = svcOver(ISSUE_ROWS); + const result = await service.queryDataset( + dataset, + { + dimensions: ['close_date'], + measures: ['opp_count'], + timeDimensions: [{ dimension: 'close_date', dateRange: ['2026-01-01', '2026-02-28'] }], + compareTo: { kind: 'previousYear', dimension: 'close_date' }, + }, + CTX, + ); + // The second half of the defect, asserted on its own: `2025-01` / `2025-02` + // used to sit in a grid whose date filter was 2026-01..2026-02. + expect(result.rows.map((r) => r.close_date)).toEqual(['2026-01', '2026-02']); + }); + + it('the fake really buckets — the comparison pass DOES return shifted keys', async () => { + // The guard on this whole file: if the fake returned the same keys for both + // passes (as the #4870 pin's does), every assertion above would pass with + // the alignment deleted. Query the shifted window directly and read them. + const { service } = svcOver(ISSUE_ROWS); + const shifted = await service.queryDataset( + dataset, + { + dimensions: ['close_date'], + measures: ['opp_count'], + timeDimensions: [{ dimension: 'close_date', dateRange: ['2025-01-01', '2025-02-28'] }], + }, + CTX, + ); + expect(shifted.rows).toEqual([ + { close_date: '2025-01', opp_count: 5 }, + { close_date: '2025-02', opp_count: 7 }, + ]); + }); +}); + +// ── 2) alignment is CALENDAR, not array position ──────────────────────────── + +/** + * A gap in the middle of the current window: 2026-03 has no opportunities at + * all, while the previous year has data in all four months. + */ +const GAP_ROWS: Opp[] = [ + ...opps('2026-01-05', 1), + ...opps('2026-02-11', 2), + // 2026-03: nothing + ...opps('2026-04-09', 4), + ...opps('2025-01-07', 5), + ...opps('2025-02-03', 7), + ...opps('2025-03-21', 3), + ...opps('2025-04-02', 9), +]; + +describe('#6007 — a gap in the current grid does not shift the alignment', () => { + it('pairs by calendar bucket, so 2025-03 lands on 2026-03 and not on 2026-04', async () => { + const { service } = svcOver(GAP_ROWS); + const result = await service.queryDataset( + dataset, + { + dimensions: ['close_date'], + measures: ['opp_count'], + timeDimensions: [{ dimension: 'close_date', dateRange: ['2026-01-01', '2026-04-30'] }], + compareTo: { kind: 'previousYear', dimension: 'close_date' }, + }, + CTX, + ); + // The tempting implementation — sort each pass's keys and pair by index — + // would map 2025-03 onto 2026-04 (the third key the primary grid HAS) and + // leave 2025-04 homeless: every bucket after a gap reads its neighbour's + // history. Ordinals are computed from the calendar, so the gap costs + // nothing and 2026-03 arrives as its own row, in-window, with an honest 0. + expect(result.rows).toEqual([ + { close_date: '2026-01', opp_count: 1, opp_count__compare: 5 }, + { close_date: '2026-02', opp_count: 2, opp_count__compare: 7 }, + { close_date: '2026-03', opp_count: 0, opp_count__compare: 3 }, + { close_date: '2026-04', opp_count: 4, opp_count__compare: 9 }, + ]); + }); +}); + +// ── 3) every granularity, and the second grid dimension ───────────────────── + +describe('#6007 — the shift-back at each bucket size the selection can ask for', () => { + const CROSS_YEAR: Opp[] = [ + ...opps('2026-02-10', 1), + ...opps('2026-05-10', 2), + ...opps('2025-02-10', 3), + ...opps('2025-05-10', 4), + ]; + + it('quarter', async () => { + const { service } = svcOver(CROSS_YEAR); + const result = await service.queryDataset( + dataset, + { + dimensions: ['close_date'], + measures: ['opp_count'], + dateGranularity: 'quarter', + timeDimensions: [{ dimension: 'close_date', dateRange: ['2026-01-01', '2026-06-30'] }], + compareTo: { kind: 'previousYear', dimension: 'close_date' }, + }, + CTX, + ); + expect(result.rows).toEqual([ + { close_date: '2026-Q1', opp_count: 1, opp_count__compare: 3 }, + { close_date: '2026-Q2', opp_count: 2, opp_count__compare: 4 }, + ]); + }); + + it('year', async () => { + const { service } = svcOver(CROSS_YEAR); + const result = await service.queryDataset( + dataset, + { + dimensions: ['close_date'], + measures: ['opp_count'], + dateGranularity: 'year', + timeDimensions: [{ dimension: 'close_date', dateRange: ['2026-01-01', '2026-12-31'] }], + compareTo: { kind: 'previousYear', dimension: 'close_date' }, + }, + CTX, + ); + expect(result.rows).toEqual([{ close_date: '2026', opp_count: 3, opp_count__compare: 7 }]); + }); + + it('day', async () => { + const { service } = svcOver([ + ...opps('2026-03-02', 1), + ...opps('2026-03-03', 2), + ...opps('2025-03-02', 5), + ...opps('2025-03-03', 6), + ]); + const result = await service.queryDataset( + dataset, + { + dimensions: ['close_date'], + measures: ['opp_count'], + dateGranularity: 'day', + timeDimensions: [{ dimension: 'close_date', dateRange: ['2026-03-01', '2026-03-04'] }], + compareTo: { kind: 'previousYear', dimension: 'close_date' }, + }, + CTX, + ); + expect(result.rows).toEqual([ + { close_date: '2026-03-02', opp_count: 1, opp_count__compare: 5 }, + { close_date: '2026-03-03', opp_count: 2, opp_count__compare: 6 }, + ]); + }); + + /** + * ISO weeks, end to end, on a SCRIPTED fake: it answers each pass with the + * canonical week keys of the window that pass filtered to. The keys are + * hand-written, so they are validated here against `bucketKeyToCalendarRange` + * rather than trusted — a fixture that re-implemented ISO weeks could only + * prove it agreed with the implementation. + */ + it('week', async () => { + // 2026-W02 = Mon 2026-01-05 … Sun 2026-01-11; 2025-W02 = Mon 2025-01-06 … + for (const [key, start] of [ + ['2026-W02', '2026-01-05'], + ['2026-W03', '2026-01-12'], + ['2025-W02', '2025-01-06'], + ['2025-W03', '2025-01-13'], + ] as const) { + expect(bucketKeyToCalendarRange(key, 'week')?.start, key).toBe(start); + } + const service = new AnalyticsService({ + queryCapabilities: () => ({ nativeSql: false, objectqlAggregate: true, inMemory: false }), + executeAggregate: async (_object: string, options: Record) => { + const from = String( + ((options as AggOptions).filter as Record>).close_date.$gte, + ); + return from.startsWith('2025') + ? [{ close_date: '2025-W02', opp_count: 30 }, { close_date: '2025-W03', opp_count: 40 }] + : [{ close_date: '2026-W02', opp_count: 3 }, { close_date: '2026-W03', opp_count: 4 }]; + }, + }); + const result = await service.queryDataset( + dataset, + { + dimensions: ['close_date'], + measures: ['opp_count'], + dateGranularity: 'week', + timeDimensions: [{ dimension: 'close_date', dateRange: ['2026-01-05', '2026-01-18'] }], + compareTo: { kind: 'previousYear', dimension: 'close_date' }, + }, + CTX, + ); + expect(result.rows).toEqual([ + { close_date: '2026-W02', opp_count: 3, opp_count__compare: 30 }, + { close_date: '2026-W03', opp_count: 4, opp_count__compare: 40 }, + ]); + }); + + it('aligns the ANCHOR column only — the other grid dimension is matched as-is', async () => { + const { service } = svcOver(ISSUE_ROWS); + const result = await service.queryDataset( + dataset, + { + dimensions: ['owner', 'close_date'], + measures: ['opp_count'], + timeDimensions: [{ dimension: 'close_date', dateRange: ['2026-01-01', '2026-02-28'] }], + compareTo: { kind: 'previousYear', dimension: 'close_date' }, + }, + CTX, + ); + // `opps` alternates owners, so 2025-01's five split 3/2 and 2025-02's seven + // split 4/3 — numbers no single-dimension merge could produce by accident. + expect(result.rows).toEqual([ + { owner: 'u1', close_date: '2026-01', opp_count: 1, opp_count__compare: 3 }, + { owner: 'u2', close_date: '2026-01', opp_count: 0, opp_count__compare: 2 }, + { owner: 'u1', close_date: '2026-02', opp_count: 1, opp_count__compare: 4 }, + { owner: 'u2', close_date: '2026-02', opp_count: 1, opp_count__compare: 3 }, + ]); + }); + + it('a measure-scoped filter still means the same thing in both columns (#4820 × #6007)', async () => { + const { service } = svcOver([ + ...opps('2026-01-05', 1, 'won'), + ...opps('2026-01-06', 2, 'lost'), + ...opps('2025-01-07', 5, 'won'), + ...opps('2025-01-08', 4, 'lost'), + ]); + const result = await service.queryDataset( + dataset, + { + dimensions: ['close_date'], + measures: ['opp_count', 'won_count'], + timeDimensions: [{ dimension: 'close_date', dateRange: ['2026-01-01', '2026-01-31'] }], + compareTo: { kind: 'previousYear', dimension: 'close_date' }, + }, + CTX, + ); + // The measure-scoped pass fans out into its own shifted sub-query, so its + // rows key to the shifted bucket too and need the same realignment. + expect(result.rows).toEqual([ + { + close_date: '2026-01', + opp_count: 3, + won_count: 1, + opp_count__compare: 9, + won_count__compare: 5, + }, + ]); + }); +}); + +// ── 4) previousPeriod — the ordinal rule ──────────────────────────────────── + +describe('#6007 — previousPeriod aligns by bucket ordinal (the ruling’s added semantic)', () => { + it('day buckets: the n-th day of the previous window onto the n-th day of this one', async () => { + const { service, calls } = svcOver([ + ...opps('2026-05-01', 1), + ...opps('2026-05-02', 2), + ...opps('2026-05-03', 3), + ...opps('2026-05-04', 10), + ...opps('2026-05-05', 20), + ...opps('2026-05-06', 30), + ]); + const result = await service.queryDataset( + dataset, + { + dimensions: ['close_date'], + measures: ['opp_count'], + dateGranularity: 'day', + timeDimensions: [{ dimension: 'close_date', dateRange: ['2026-05-04', '2026-05-06'] }], + compareTo: { kind: 'previousPeriod', dimension: 'close_date' }, + }, + CTX, + ); + // The equal-length window immediately before: 2026-05-01..03. + expect(calls[1].filter).toMatchObject({ close_date: { $gte: '2026-05-01', $lte: '2026-05-03' } }); + expect(result.rows).toEqual([ + { close_date: '2026-05-04', opp_count: 10, opp_count__compare: 1 }, + { close_date: '2026-05-05', opp_count: 20, opp_count__compare: 2 }, + { close_date: '2026-05-06', opp_count: 30, opp_count__compare: 3 }, + ]); + }); + + it('month buckets: a 59-day window pairs its two months with the previous two', async () => { + // [2026-02-01, 2026-03-31] is 59 days, so the previous period is + // [2025-12-04, 2026-01-31] — two month buckets against two, aligned by + // position: 2025-12 → 2026-02, 2026-01 → 2026-03. No calendar arithmetic + // relates those pairs; the ordinal rule is what defines them. + const { service, calls } = svcOver([ + ...opps('2025-12-10', 5), + ...opps('2026-01-10', 6), + ...opps('2026-02-10', 1), + ...opps('2026-03-10', 2), + ]); + const result = await service.queryDataset( + dataset, + { + dimensions: ['close_date'], + measures: ['opp_count'], + timeDimensions: [{ dimension: 'close_date', dateRange: ['2026-02-01', '2026-03-31'] }], + compareTo: { kind: 'previousPeriod', dimension: 'close_date' }, + }, + CTX, + ); + expect(calls[1].filter).toMatchObject({ close_date: { $gte: '2025-12-04', $lte: '2026-01-31' } }); + expect(result.rows).toEqual([ + { close_date: '2026-02', opp_count: 1, opp_count__compare: 5 }, + { close_date: '2026-03', opp_count: 2, opp_count__compare: 6 }, + ]); + }); +}); + +// ── 5) the shapes it must NOT touch ───────────────────────────────────────── + +describe('#6007 — scoped to the broken shape, and no wider', () => { + it('a window-only anchor is untouched: it is not a column, so there is nothing to realign', async () => { + const { service } = svcOver(ISSUE_ROWS); + const result = await service.queryDataset( + dataset, + { + dimensions: ['owner'], + measures: ['opp_count'], + timeDimensions: [{ dimension: 'close_date', dateRange: ['2026-01-01', '2026-02-28'] }], + compareTo: { kind: 'previousYear', dimension: 'close_date' }, + }, + CTX, + ); + // #5688's half of the pair: neither pass buckets the anchor, both grids key + // on `owner`, and the merge already worked. The comparison totals span the + // WHOLE shifted window (u1: 3 + 4, u2: 2 + 3), so they are only reachable by + // merging it onto the owner rather than per month. + expect(result.rows).toEqual([ + { owner: 'u1', opp_count: 2, opp_count__compare: 7 }, + { owner: 'u2', opp_count: 1, opp_count__compare: 5 }, + ]); + }); + + it('an UNBUCKETED date grid dimension is left exactly as it was', async () => { + const { service } = svcOver([...opps('2026-01-05', 1), ...opps('2025-01-05', 5)]); + const result = await service.queryDataset( + dataset, + { + dimensions: ['created_at'], + measures: ['opp_count'], + timeDimensions: [{ dimension: 'created_at', dateRange: ['2026-01-01', '2026-02-28'] }], + compareTo: { kind: 'previousYear', dimension: 'created_at' }, + }, + CTX, + ); + // `created_at` declares no `dateGranularity` and the selection states none, + // so the runtime groups the raw column: these are instants, not bucket keys, + // and there is no "the same bucket, one year on" for them. The pre-#6007 + // append is kept deliberately — inventing an alignment for a raw timestamp + // is how a comparison column ends up on a row it does not describe. + expect(result.rows).toEqual([ + { created_at: '2025-01-05', opp_count: 0, opp_count__compare: 5 }, + { created_at: '2026-01-05', opp_count: 1, opp_count__compare: 0 }, + ]); + }); + + it('the #4870 pin still holds: a fake that ignores the window merges as it always did', async () => { + // `dataset-selection-window.test.ts`'s fake returns `2026-06` on BOTH passes. + // That key is not one the shift-back maps FROM, so the alignment declines and + // the row keeps it — which is exactly what that test asserts. Restated here + // so the interaction is visible from this side too. + let call = 0; + const service = new AnalyticsService({ + queryCapabilities: () => ({ nativeSql: false, objectqlAggregate: true, inMemory: false }), + executeAggregate: async () => [{ close_date: '2026-06', opp_count: ++call }], + }); + const result = await service.queryDataset( + dataset, + { + dimensions: ['close_date'], + measures: ['opp_count'], + timeDimensions: [{ dimension: 'close_date', dateRange: ['2026-06-01', '2026-06-30'] }], + compareTo: { kind: 'previousPeriod', dimension: 'close_date' }, + }, + CTX, + ); + expect(result.rows).toEqual([{ close_date: '2026-06', opp_count: 1, opp_count__compare: 2 }]); + }); +}); + +// ── 6) the key arithmetic, pinned against core's canonical inverse ────────── + +const GRANULARITIES: DateGranularityValue[] = ['day', 'week', 'month', 'quarter', 'year']; + +/** Cross-year, cross-quarter, cross-month, leap and ISO-week-boundary days. */ +const DAYS = [ + '2024-01-15', + '2024-02-29', // leap day + '2024-04-01', + '2024-06-30', + '2024-12-30', // ISO 2025-W01 + '2025-01-01', + '2025-05-19', + '2026-01-04', // last day of ISO 2026-W01 + '2026-01-05', // first day of ISO 2026-W02 + '2026-02-15', + '2026-12-31', +]; + +describe('#6007 — bucketOrdinalOfDay ↔ bucketKeyAtOrdinal, against bucketKeyToCalendarRange', () => { + /** + * `bucketKeyAtOrdinal` is the only place this package MINTS a bucket key, and + * the keys it mints are compared byte-for-byte against keys the runtime's + * `GROUP BY` produced. Anything less than a round trip against + * `@objectstack/core`'s canonical inverse would be a copy of the bucket + * vocabulary that cannot detect its own drift — the failure mode + * `bucketDateValue`'s own doc warns about for the copies that already exist. + */ + for (const g of GRANULARITIES) { + for (const day of DAYS) { + it(`${day} @ ${g} round-trips`, () => { + const key = bucketKeyAtOrdinal(bucketOrdinalOfDay(day, g), g); + const span = bucketKeyToCalendarRange(key, g); + expect(span, `core rejects the minted key ${key}`).not.toBeNull(); + // the day lies inside the half-open span of the key minted for it + expect(span!.start <= day).toBe(true); + expect(day < span!.end).toBe(true); + // and the span's own first day mints the same key back + expect(bucketKeyAtOrdinal(bucketOrdinalOfDay(span!.start, g), g)).toBe(key); + // ordinals advance by exactly one per bucket + expect(bucketOrdinalOfDay(span!.end, g)).toBe(bucketOrdinalOfDay(day, g) + 1); + }); + } + } +}); + +describe('#6007 — alignedCompareBucketKey declines rather than guessing', () => { + const CUR: [string, string] = ['2026-01-01', '2026-02-28']; + const PREV: [string, string] = ['2025-01-01', '2025-02-28']; + + it('shifts a previousYear bucket forward one calendar year', () => { + expect(alignedCompareBucketKey('2025-01', 'month', 'previousYear', CUR, PREV)).toBe('2026-01'); + expect(alignedCompareBucketKey('2025-02', 'month', 'previousYear', CUR, PREV)).toBe('2026-02'); + }); + + it('returns null for the EMPTY bucket, whose key is null on both aggregation paths (#3839)', () => { + // Both passes already key their empty bucket the same way, so they merge + // with each other; "one year after nothing" is not a date. + expect(alignedCompareBucketKey(null, 'month', 'previousYear', CUR, PREV)).toBeNull(); + expect(alignedCompareBucketKey(undefined, 'month', 'previousYear', CUR, PREV)).toBeNull(); + expect(alignedCompareBucketKey('', 'month', 'previousYear', CUR, PREV)).toBeNull(); + }); + + it('returns null for a value that is not a bucket key of this granularity', () => { + // A raw timestamp from an unbucketed date dimension, and a key of the wrong + // bucket size — neither has a span to shift. + expect(alignedCompareBucketKey('2025-01-07T09:30:00Z', 'month', 'previousYear', CUR, PREV)).toBeNull(); + expect(alignedCompareBucketKey('2025-Q1', 'month', 'previousYear', CUR, PREV)).toBeNull(); + expect(alignedCompareBucketKey('2025-13', 'month', 'previousYear', CUR, PREV)).toBeNull(); + }); + + it('returns null when the shifted-back bucket falls outside the current window', () => { + // Two equal-length day windows can tile into different bucket counts, so the + // ordinal rule can have a last bucket with no counterpart. Moving it to a + // bucket the caller never asked for would swap a visibly foreign row for a + // plausible-looking wrong one; it keeps its own key and appends instead. + expect(alignedCompareBucketKey('2025-03', 'month', 'previousYear', CUR, PREV)).toBeNull(); + expect(alignedCompareBucketKey('2024-12', 'month', 'previousYear', CUR, PREV)).toBeNull(); + }); + + it('previousPeriod shifts by the ordinal distance between the two window starts', () => { + const cur: [string, string] = ['2026-05-04', '2026-05-06']; + const prev: [string, string] = ['2026-05-01', '2026-05-03']; + expect(alignedCompareBucketKey('2026-05-01', 'day', 'previousPeriod', cur, prev)).toBe('2026-05-04'); + expect(alignedCompareBucketKey('2026-05-03', 'day', 'previousPeriod', cur, prev)).toBe('2026-05-06'); + // one bucket past the window — declined + expect(alignedCompareBucketKey('2026-05-04', 'day', 'previousPeriod', cur, prev)).toBeNull(); + }); +}); diff --git a/packages/services/service-analytics/src/__tests__/dataset-window-timedimension-bucketing.test.ts b/packages/services/service-analytics/src/__tests__/dataset-window-timedimension-bucketing.test.ts index 3ce25279fc..89ef7f208d 100644 --- a/packages/services/service-analytics/src/__tests__/dataset-window-timedimension-bucketing.test.ts +++ b/packages/services/service-analytics/src/__tests__/dataset-window-timedimension-bucketing.test.ts @@ -48,6 +48,21 @@ * was outside the merge key: with several month-split rows per owner the * comparison value landed on whichever row the index happened to hold last and * the others read a confident `0`. Both numbers are asserted below. + * + * ## …and bucketing alike was only HALF of it (#6007) + * + * Identical bucketing makes the two grids the same SHAPE; it does not make them + * the same KEYS. For a bucketed anchor the comparison pass groups the shifted + * window, so its rows key to `2025-01` where the primary grid keys to `2026-01` + * — nothing merges, every comparison row is appended, and the grid doubles into + * half-real/half-zero rows plus buckets from outside the caller's own window. + * The bucketed-anchor case below asserted exactly that grid as "long-standing + * behaviour, unchanged by #5688", pointing at #6007; #6007 ruled it a defect and + * flipped it to the paired 2×2. The whole-file pairing is unchanged: the + * window-only anchor is still asserted NOT to align (it is not a column on + * either pass, so there is nothing to realign), and the alignment itself — + * every granularity, both `compareTo` kinds, and every case it declines — lives + * in `dataset-compare-bucket-alignment.test.ts`. */ import { describe, it, expect } from 'vitest'; @@ -389,7 +404,7 @@ describe('#5688 — compareTo buckets both passes alike, whichever verdict it re expect(names(result.fields)).toEqual(['owner', 'opp_count', 'opp_count__compare']); }); - it('bucketed anchor: BOTH passes bucket by month — the #4870 fix, unregressed', async () => { + it('bucketed anchor: BOTH passes bucket by month, and the comparison lands on the SAME row (#6007)', async () => { const { service, calls } = svc(); const result = await service.queryDataset( dataset, @@ -411,16 +426,35 @@ describe('#5688 — compareTo buckets both passes alike, whichever verdict it re expect(descriptor(result.fields, 'close_date')).toEqual({ name: 'close_date', type: 'time', label: 'Close Date', }); - // Long-standing behaviour, unchanged by #5688 and asserted so the pair is - // read as a whole: the shifted grid keys by its OWN buckets, so a comparison - // bucket with no current-window twin arrives as its own row (the append - // `fillEmptyGroups` is documented to expect). + // **This assertion was deliberately flipped by #6007**, and it is the whole + // point of the pair. Bucketing the two passes ALIKE (#4870, above) was only + // half the requirement: the comparison pass still grouped the SHIFTED + // window, so its rows keyed to `2025-01` / `2025-02` while the merge keys on + // `selection.dimensions` — nothing matched, every comparison row was + // APPENDED, and this is what the grid used to be: + // + // ``` + // [{ close_date: '2025-01', opp_count: 0, opp_count__compare: 2 }, + // { close_date: '2025-02', opp_count: 0, opp_count__compare: 2 }, + // { close_date: '2026-01', opp_count: 2, opp_count__compare: 0 }, + // { close_date: '2026-02', opp_count: 1, opp_count__compare: 0 }] + // ``` + // + // Four rows for a two-bucket window, a confident `0` on every one of them + // (each pass reported only its own half, and `fillEmptyGroups` filled the + // other), and two rows keyed OUTSIDE the window the caller filtered to. The + // ruling on #6007 restates each comparison bucket key in current-period + // terms — `2025-01` → `2026-01` — so a "this year vs last year" trend is + // what it reads as: 2 rows × 2 columns, one row per bucket of the requested + // window, both numbers real. expect(result.rows).toEqual([ - { close_date: '2025-01', opp_count: 0, opp_count__compare: 2 }, - { close_date: '2025-02', opp_count: 0, opp_count__compare: 2 }, - { close_date: '2026-01', opp_count: 2, opp_count__compare: 0 }, - { close_date: '2026-02', opp_count: 1, opp_count__compare: 0 }, + { close_date: '2026-01', opp_count: 2, opp_count__compare: 2 }, + { close_date: '2026-02', opp_count: 1, opp_count__compare: 2 }, ]); + // Said as an invariant rather than only as a row list, because it is the + // invariant a future change is most likely to break silently: no row keys to + // a bucket outside the filtered window. + expect(result.rows.every((r) => String(r.close_date).startsWith('2026-'))).toBe(true); }); it('window-only anchor with NO grid dimensions: one row, both totals comparable', async () => { diff --git a/packages/services/service-analytics/src/dataset-executor.ts b/packages/services/service-analytics/src/dataset-executor.ts index 70d6b37a29..7204a307f2 100644 --- a/packages/services/service-analytics/src/dataset-executor.ts +++ b/packages/services/service-analytics/src/dataset-executor.ts @@ -9,7 +9,7 @@ import type { } from '@objectstack/spec/contracts'; import { emptyGroupValueFor, type FilterCondition } from '@objectstack/spec/data'; import type { ExecutionContext } from '@objectstack/spec/kernel'; -import { filterTokenContextFrom, resolveFilterTokens } from '@objectstack/core'; +import { bucketKeyToCalendarRange, filterTokenContextFrom, resolveFilterTokens } from '@objectstack/core'; import type { CompiledDataset, DerivedMeasureSpec } from './dataset-compiler.js'; import { datasetInvalidError } from './dataset-refusal.js'; import type { OrderLabelResolver } from './dimension-labels.js'; @@ -35,7 +35,11 @@ export type CompareTo = DatasetCompareTo; * - evaluates derived measures (ratio/sum/difference/product) row-by-row (Q1), * - shifts the queries for `compareTo` (previousPeriod / previousYear) and * attaches `__compare` columns, re-running the same measure pass - * so a filtered measure means the same thing in both columns, + * so a filtered measure means the same thing in both columns, and — when + * the shifted dimension is the grid's own time axis — restating the + * comparison rows' bucket keys in CURRENT-period terms so the two grids + * merge into one row per bucket instead of stacking two windows on one + * axis (#6007), * - computes server-side totals (`selection.totals.groupings`, #1753) by * re-running the selection per dimension subset, so matrix subtotals and * the grand total use each measure's true aggregate, @@ -575,6 +579,173 @@ export function shiftRange(range: [string, string], kind: CompareTo['kind']): [s return [toISODate(prevStartMs), toISODate(prevEndMs)]; } +// ── compareTo bucket alignment (#6007) ─────────────────────────────────────── + +/** + * The ISO-8601 week label (`2026-W23`) of the UTC calendar day at `ms`. + * + * Mirrors the week branch of `@objectstack/objectql`'s `bucketDateValue` — the + * function that MINTS the bucket keys this executor then has to realign. It is + * copied rather than imported because `service-analytics` does not depend on + * `objectql` (it talks to the runtime through `IAnalyticsService`), and the + * copy is not a blind one: {@link bucketKeyAtOrdinal} is pinned round-trip + * against `bucketKeyToCalendarRange` — `@objectstack/core`'s exported INVERSE + * of the same vocabulary, which rejects an impossible week outright — so a + * drift in either direction fails a test rather than mislabelling a bucket. + */ +function isoWeekKeyOfUtcMs(ms: number): string { + const target = new Date(ms); + const dayNum = (target.getUTCDay() + 6) % 7; // Mon=0..Sun=6 + target.setUTCDate(target.getUTCDate() - dayNum + 3); // that week's Thursday + const firstThursday = new Date(Date.UTC(target.getUTCFullYear(), 0, 4)); + const weekNo = + 1 + + Math.round( + ((target.getTime() - firstThursday.getTime()) / DAY_MS - 3 + ((firstThursday.getUTCDay() + 6) % 7)) / 7, + ); + return `${target.getUTCFullYear()}-W${String(weekNo).padStart(2, '0')}`; +} + +/** + * The ORDINAL of the bucket a UTC calendar day falls in: a monotone integer + * that advances by exactly 1 per bucket, at every granularity. + * + * This is what makes "the same relative position in the other window" a + * computable thing rather than an array index. Counting POSITIONS in the two + * result sets would have been the obvious implementation and is wrong for the + * reason every gap-sensitive alignment is wrong: a bucket the current window + * reported no rows for is missing from that array, so every later bucket + * silently shifts by one and the comparison column lands on its neighbour. + * Ordinals are computed from the CALENDAR, so a gap costs nothing. + * + * @param ymd - a `YYYY-MM-DD` UTC calendar day. + */ +export function bucketOrdinalOfDay(ymd: string, granularity: DateGranularityValue): number { + const ms = parseUTC(ymd); + const d = new Date(ms); + const y = d.getUTCFullYear(); + const m = d.getUTCMonth(); // 0-11 + switch (granularity) { + case 'year': + return y; + case 'quarter': + return y * 4 + Math.floor(m / 3); + case 'month': + return y * 12 + m; + // 1970-01-01 was a Thursday, so shifting by 3 days puts the Monday boundary + // on a multiple of 7 and the ordinal advances exactly at each ISO week start. + case 'week': + return Math.floor((ms + 3 * DAY_MS) / (7 * DAY_MS)); + case 'day': + default: + return Math.floor(ms / DAY_MS); + } +} + +/** + * The canonical bucket KEY at an ordinal — the inverse of + * {@link bucketOrdinalOfDay}, and the only place this package mints a bucket key + * of its own. + * + * The keys produced here MUST be byte-identical to the ones the runtime's + * grouping produced for the primary pass, because they are compared as merge + * keys: `2026-01`, `2026-Q1`, `2026`, `2026-01-07`, `2026-W03`. That equality is + * pinned round-trip against `bucketKeyToCalendarRange` rather than asserted by + * eye — see `dataset-compare-bucket-alignment.test.ts`. + */ +export function bucketKeyAtOrdinal(ordinal: number, granularity: DateGranularityValue): string { + switch (granularity) { + case 'year': + return String(ordinal); + case 'quarter': + return `${Math.floor(ordinal / 4)}-Q${(ordinal % 4) + 1}`; + case 'month': + return `${Math.floor(ordinal / 12)}-${String((ordinal % 12) + 1).padStart(2, '0')}`; + case 'week': + return isoWeekKeyOfUtcMs(ordinal * 7 * DAY_MS - 3 * DAY_MS); + case 'day': + default: + return toISODate(ordinal * DAY_MS); + } +} + +/** + * Restate ONE comparison bucket key in CURRENT-period terms (#6007) — the + * maintainer's ruling of 2026-08-07, direction 1. + * + * ## What goes wrong without it + * + * When the dimension `compareTo` anchors on is ALSO a grid dimension — a trend + * chart's own time axis, the standard "this period vs the same period last + * year" shape — the comparison pass groups the SHIFTED window, so its rows key + * to shifted buckets. {@link mergeByDimensions} keys on `selection.dimensions`, + * and `2025-01` is not `2026-01`, so not one comparison row ever merged: every + * one of them was APPENDED as a new row. The grid then read half real value and + * half confident `0` on every row (`fillEmptyGroups` filling the halves each + * pass never reported), with the shifted buckets sitting in it as rows from + * outside the very window the caller filtered to. A 2×2 answer came back as + * 4 rows × 2 columns with a zero in each. + * + * ## The two shift-backs + * + * - **`previousYear`** — the shift is a calendar year, so the inverse is a + * calendar year forward, applied to the bucket's own first day and re-bucketed. + * `2025-01` → `2026-01`, `2025-Q1` → `2026-Q1`, `2025-W03` → `2026-W03` + * (the week one year on, which is how a year-over-year weekly trend reads). + * It is deliberately the exact inverse of the {@link shiftRange} arithmetic + * that produced the window, `setUTCFullYear` rollover included, so window and + * key can never disagree about what "one year" meant. + * - **`previousPeriod`** — an arbitrary-length day window has no calendar + * counterpart, so alignment is by **bucket ordinal**: the n-th bucket of the + * previous window is the n-th bucket of this one, with `n` counted from each + * window's own start. This is a semantic the ruling states, not one derived + * from anything already here. + * + * ## Fail-closed, in both directions + * + * Returns `null` — meaning "leave this row's key alone", i.e. exactly the + * pre-#6007 behaviour — whenever the alignment is not certain: + * + * - the key is not a string (the empty bucket keys as `null` on both + * aggregation paths, #3839; the two passes' empty buckets already merge with + * each other, and "one year after nothing" is not a date); + * - the key is not a bucket key of this granularity, so there is no span to + * shift (a raw timestamp from an unbucketed date dimension lands here); + * - the shifted-back bucket falls outside the CURRENT window. Two equal-length + * day windows can tile into different bucket counts (a 31-day window + * straddling a month boundary yields two month buckets, its neighbour one), + * and the ordinal rule then has a last bucket with no counterpart. Moving it + * to a bucket the caller did not ask for would trade a visibly foreign row + * for a plausible-looking wrong one; it keeps its own key and appends, as it + * did before. + * + * @param currentRange - the selection's own window for the anchor dimension. + * @param shiftedRange - what {@link shiftRange} made of it. + */ +export function alignedCompareBucketKey( + key: unknown, + granularity: DateGranularityValue, + kind: CompareTo['kind'], + currentRange: [string, string], + shiftedRange: [string, string], +): string | null { + if (typeof key !== 'string' || key.length === 0) return null; + const span = bucketKeyToCalendarRange(key, granularity); + if (!span) return null; + + const targetOrdinal = + kind === 'previousYear' + ? bucketOrdinalOfDay(shiftYear(span.start, 1), granularity) + : bucketOrdinalOfDay(span.start, granularity) + + (bucketOrdinalOfDay(currentRange[0], granularity) - + bucketOrdinalOfDay(shiftedRange[0], granularity)); + + const first = bucketOrdinalOfDay(currentRange[0], granularity); + const last = bucketOrdinalOfDay(currentRange[1], granularity); + if (targetOrdinal < first || targetOrdinal > last) return null; + return bucketKeyAtOrdinal(targetOrdinal, granularity); +} + export class DatasetExecutor { /** * @param service - The analytics service the executor issues its queries to. @@ -735,7 +906,13 @@ export class DatasetExecutor { // - the compareTo pass appends a row for every bucket that existed in the // PREVIOUS window and not in this one, on which *every* base measure is // absent — including unfiltered ones, which is why the fill covers all - // base measures rather than only the filter-scoped ones. + // base measures rather than only the filter-scoped ones. Since #6007 + // that append is once again the EDGE it was always documented to be + // ("this group sold last month and nothing this month"): when the + // shifted dimension is the grid's own time axis the comparison keys are + // realigned onto the current period first, so the append no longer + // fires on every single row and no longer carries a bucket from outside + // the caller's window. // Running it before the compare merge left that last class blank, so a lead // source that sold last month and nothing this month rendered as "no data" // instead of 0 — the same worst-row bias, one merge later. @@ -895,6 +1072,29 @@ export class DatasetExecutor { return dimensions.filter((d) => compiled.cube.dimensions[d]?.type === 'time'); } + /** + * The EFFECTIVE bucket size one dimension is grouped at for this selection, + * or `undefined` when it is not a date dimension or nothing states a size (in + * which case the runtime groups the raw column). + * + * One definition, two readers, deliberately: {@link buildQuery} uses it to + * decide the `GROUP BY`, and {@link runCompare} uses it to realign the + * comparison pass's bucket keys (#6007). Those two MUST agree — realigning + * `month` keys a query grouped by `quarter` would move every comparison value + * onto a bucket that does not exist — and the way to make them agree is to + * have one of them, not two that look alike. + */ + private granularityOf( + compiled: CompiledDataset, + selection: DatasetSelection, + name: string, + ): DateGranularityValue | undefined { + const cd = compiled.cube.dimensions[name]; + if (cd?.type !== 'time') return undefined; + const datasetDefault = cd.granularities?.length === 1 ? String(cd.granularities[0]) : undefined; + return resolveDimensionGranularity(selection, name, datasetDefault); + } + private buildQuery( compiled: CompiledDataset, opts: { @@ -977,12 +1177,8 @@ export class DatasetExecutor { const selTimeDims = opts.selection.timeDimensions ?? []; const selDims = new Set(selTimeDims.map((t) => t.dimension)); const groupedDims = new Set(opts.dimensions); - const granularityFor = (name: string): string | undefined => { - const cd = compiled.cube.dimensions[name]; - if (cd?.type !== 'time') return undefined; - const datasetDefault = cd.granularities?.length === 1 ? String(cd.granularities[0]) : undefined; - return resolveDimensionGranularity(opts.selection, name, datasetDefault); - }; + const granularityFor = (name: string): string | undefined => + this.granularityOf(compiled, opts.selection, name); /** * Does a caller-supplied entry that stated NO granularity get one filled in? * @@ -1067,10 +1263,38 @@ export class DatasetExecutor { { ...selection, timeDimensions: shiftedTd }, { measures, dimensions, baseFilter, context }, ); + + // #6007 — when the anchor is ALSO a grid dimension, the comparison rows key + // to the SHIFTED buckets they were grouped into, and `mergeByDimensions` + // keys on `dimensions`: `2025-01` never equals `2026-01`, so every + // comparison row was appended instead of merged and the grid came back with + // twice the rows, a confident `0` in each, and the shifted buckets showing + // as rows from outside the caller's own window. Restate each comparison + // bucket key in current-period terms BEFORE the merge — see + // {@link alignedCompareBucketKey} for the two shift-backs and for every + // case it deliberately declines to align. + // + // Scoped to exactly the shape that is broken, and no wider: + // - the anchor must be a GRID dimension. A window-only anchor is not a + // column on either pass, so the two grids already align on the + // dimensions the merge keys by (#5688) and there is nothing to move; + // - it must be BUCKETED. An ungrouped date dimension groups raw + // timestamps, which are instants and not bucket keys — there is no + // "same bucket, one year on" for them, so they are left alone. + // Both passes read the bucket size through the same `granularityOf`, so the + // size realigned here is by construction the size grouped by. + const granularity = dimensions.includes(dimension) + ? this.granularityOf(compiled, selection, dimension) + : undefined; + // Rename measure columns to `__compare` so they merge alongside primary. return sub.rows.map((row) => { const out: Record = {}; for (const dim of dimensions) out[dim] = row[dim]; + if (granularity) { + const aligned = alignedCompareBucketKey(row[dimension], granularity, cmp.kind, range, shifted); + if (aligned != null) out[dimension] = aligned; + } for (const m of measures) out[`${m}__compare`] = row[m]; return out; });