Skip to content

Commit ad067ad

Browse files
fix(service-analytics): compareTo resolves its comparison window on the reference calendar, not UTC (#18596)
Fixes #18245 Clause-②: no A non-UTC calendar preset under `compareTo` produced a comparison window one day too wide, silently, under an ordinary `200`. ## Mechanism `analytics-date-range.ts` renders both bounds of the ten calendar presets through `zonedDateStartToUtcMs`, so a lowered window is a pair of **instants** that open and close at the *reference zone's* midnight. `DatasetExecutor` then projected those instants onto **UTC** days, through a local `parseUTC` / `toISODate` pair it carried itself. Whenever the zone's midnight is not UTC's, the projection moves a boundary — and it moves it in **opposite directions** either side of the meridian, which is the signature of a day-boundary projection and not of an off-by-one constant. MEASURED through `DatasetExecutor.execute`, `this_month` + `compareTo: { kind: 'previousYear' }`, frozen at `2026-09-09T12:00:00Z` (noon, so all three zones read the same calendar day): | timezone | before (`e0d05538c`) | after (`dfb909bd0`) | |---|---|---| | `UTC` | `['2025-09-01','2025-09-30']` — 30 days, the positive control | `['2025-09-01','2025-09-30']` — 30 days, unchanged | | `Asia/Shanghai` | `['2025-08-31','2025-09-30']` — 31 days, **starts a day early** | `['2025-09-01','2025-09-30']` — 30 days | | `America/New_York` | `['2025-09-01','2025-10-01']` — 31 days, **ends a day late** | `['2025-09-01','2025-09-30']` — 30 days | The UTC row is committed as a **lit positive control**, not a comment: a fix that shifted a constant would green one non-UTC row, break UTC, and read as a pass to any suite that measured a single zone. Two further zones at the extremes of the offset range (`Pacific/Kiritimati` at +14, `Pacific/Niue` at −11) are pinned in the same file. Not a regression of #18241. Before it this input was a hard `DATASET_INVALID` / 400 — the preset arm could not produce a window at all. What changed is **reachability**: a refusal became a slightly-too-wide answer for non-UTC orgs and a correct one for UTC orgs. ## The fix is a deletion, and `packages/core` takes zero edits The local `parseUTC` / `toISODate` pair is **removed**. Nothing timezone-aware is written in its place — the shared vocabulary already exports both directions and this package already calls one of them one file over (`analytics-service.ts:23` / `:1623`): - **A bare `YYYY-MM-DD` is a calendar day, not an instant.** The year shift, the `previousPeriod` length and the bucket ordinals are calendar arithmetic that no zone changes, so they run on the **zone-free UTC proxy** `zonedDateStartToUtcMs` yields for an unset zone — the pattern `analytics-date-range.ts`'s own header prescribes ("anchors on the reference timezone's calendar day and does its arithmetic on a UTC proxy"). - **One seam reaches a reference zone**: turning the lowered window's instants into days. It calls `@objectstack/core`'s `bucketDateKey` at `'day'` — the same `Intl`-backed extraction the runtime's grouping labels rows with, and the exact inverse of the `zonedDateStartToUtcMs` that produced those bounds — threaded with the timezone `buildQuery` already resolves the primary pass in. Threading a zone into the *arithmetic* instead would put DST in the middle of a year shift: `2026-03-09` is `04:00Z` in `America/New_York` (EDT) and that same instant a year earlier reads `2025-03-08T23:00` EST — a different day. That is why the zone stops at the seam. Zero edits in `packages/core`, zero in `packages/spec`, and zero change to this file's exported surface (18 `export` lines before, 18 after, no diff) — hence `Clause-②: no`. ## Ablation — a green alone is not evidence The fix was committed first, then the UTC projection was put back at that one seam (the two `timezone` arguments dropped from `inclusiveCalendarDayWindow`), proven on disk before the run (injected spellings present 1/1, replaced spellings remaining 0/0, blob hash moved `9bd0156…` to `aef7dd9…`), and restored afterwards to byte-identical `9bd0156…` with `git diff HEAD` empty: ``` × Asia/Shanghai — does not start a day early (was 2025-08-31, 31 days) AssertionError: the zone is EAST of UTC, so its midnight is the PREVIOUS UTC day: expected '2025-08-31' to be '2025-09-01' × America/New_York — does not end a day late (was 2025-10-01, 31 days) AssertionError: the zone is WEST of UTC, so its next-month midnight is the NEXT UTC day: expected '2025-10-01' to be '2025-09-30' × every reference zone reports the same 30-day window AssertionError: expected { UTC: 30, 'Asia/Shanghai': 31, …(3) } to deeply equal { UTC: 30, 'Asia/Shanghai': 30, …(3) } Tests 3 failed | 1 passed (4) ``` The UTC control stayed green under ablation, which is the half that distinguishes this defect from a constant. The test subject is imported by a relative specifier (`../dataset-executor.js`), so vitest reads the mutated source directly — there is no `dist` leg in this ablation's resolution path, and the package declares no vitest alias. ## Verification All at `dfb909bd0`, exit codes captured to disk before being read. - `pnpm --filter '@objectstack/service-analytics^...' build` — exit 0. - `pnpm --filter @objectstack/service-analytics test` — **112 files, 2403 tests passed**. - `pnpm --filter @objectstack/service-analytics typecheck` — exit 0; `tsc --noEmit --listFiles` confirms the new test file is in the program. - `pnpm lint` (repo-wide `eslint . --no-inline-config`) — exit 0. Whole population, no narrowing claimed. - `node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack` derived **61** families; all 61 were run and reconciled with `--ran` carrying each exit code. **57 green.** Three exited 3 (`PREREQUISITE NOT MET`, i.e. NOT MEASURED, not a pass and not a finding): `check:dual-build-cjs-loads`, `check:lean-entry-closure`, `check:type-check-debt` — each needs a whole-repo `dist`, which CI builds. - One exited 1, **pre-existing and not this diff's**: `check:cross-package-test-inputs` flags `packages/cli/test/init-created-files-summary.e2e.test.ts` descending into `packages/spec/dist/`. Control: reverting all three of this PR's paths to the merge base, leaving the same tree and the same on-disk `packages/spec/dist`, reproduces the identical failure. It is a local build-state artefact — the gate walks a gitignored directory — and a sibling checkout with a different `packages/spec/dist` exits 0. ## Acceptance notes Observed while in the file, deliberately **not** changed here: - **The explicit-array arm's timestamp bounds are still read on the UTC calendar.** `['2026-09-01T00:00:00Z', …]` bypasses the seam entirely and reaches `shiftRange` unprojected, so a bound falling between the zone's midnight and UTC's would shift the same way the preset arm did. **Unmeasured** — and the arm's own contract states that a bare day versus a full timestamp is a per-face calendar translation (#3777 / #4042) that its arity rule does not touch, so it is not obviously a defect rather than a declared boundary. Successor: the next card in the analytics `dateRange` lane (#17015 / #17124 / #17596) that opens `date-range-array-arm.ts`. - **`isoWeekKeyOfUtcMs` is a hand-copy of core's ISO-week rule.** It is documented as deliberate (core's `isoWeekLabelFromCalendarDay` is module-private) and held honest by a round-trip pin against the exported `bucketKeyToCalendarRange`, so it is a recorded decision rather than drift. No successor needed. Neither is filed: the first is not measured, the second is declared. --- _Generated by [Claude Code](https://claude.ai/code/session_01WmBwEiWPff9JZPd5BSGNeH)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 0bd7dae commit ad067ad

3 files changed

Lines changed: 275 additions & 34 deletions

File tree

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
---
2+
'@objectstack/service-analytics': patch
3+
---
4+
5+
fix(service-analytics): resolve `compareTo`'s comparison window on the reference calendar, not UTC
6+
7+
`DatasetExecutor`'s `compareTo` day math carried its own local `parseUTC`/`toISODate` pair
8+
and read every bound on the UTC calendar. The lowered preset window is a pair of INSTANTS
9+
that open and close at the *reference zone's* midnight, so projecting them onto UTC days
10+
moved a boundary in every non-UTC zone — and in opposite directions either side of the
11+
meridian. `this_month` + `compareTo: { kind: 'previousYear' }` frozen at 2026-09-09 compared
12+
30-day September against a 31-day window: `Asia/Shanghai` opened at `2025-08-31`,
13+
`America/New_York` closed at `2025-10-01`. No error, no warning — a slightly-too-wide
14+
comparison leg rendered exactly like a correct one.
15+
16+
The local pair is deleted. The bare-calendar-day arithmetic (year shift, previous-period
17+
length, bucket ordinals) now runs through `@objectstack/core`'s `zonedDateStartToUtcMs` on
18+
its zone-free UTC proxy, and the one seam that turns instants into days — the lowered
19+
window's projection — goes through the same package's `bucketDateKey`, threaded with the
20+
timezone `buildQuery` already resolves the primary pass in. UTC callers are unaffected.
Lines changed: 143 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,143 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* [#18245] A non-UTC calendar preset under `compareTo` must produce the SAME
5+
* number of days as the window it compares against.
6+
*
7+
* ## What was wrong
8+
*
9+
* `DatasetExecutor`'s `compareTo` day math was UTC-calendar throughout: a local
10+
* `parseUTC` read a bound as UTC midnight and a local `toISODate` emitted a UTC
11+
* day. The lowered preset window, however, is a pair of INSTANTS — the ten
12+
* calendar presets open at the reference zone's midnight and close at the next
13+
* period's midnight in that same zone (`analytics-date-range.ts` renders both
14+
* bounds through `zonedDateStartToUtcMs`). Projecting those instants onto UTC
15+
* days therefore moved a boundary whenever the zone's midnight is not UTC's.
16+
*
17+
* MEASURED on `e0d05538c`, driven through `DatasetExecutor.execute`,
18+
* `this_month` + `compareTo: { kind: 'previousYear' }`, frozen at 2026-09-09:
19+
*
20+
* | timezone | comparison window | reading |
21+
* |---|---|---|
22+
* | `UTC` | `['2025-09-01','2025-09-30']` | 30 days — correct |
23+
* | `Asia/Shanghai` | `['2025-08-31','2025-09-30']` | 31 days — starts a day early |
24+
* | `America/New_York` | `['2025-09-01','2025-10-01']` | 31 days — ends a day late |
25+
*
26+
* ⭐ **The two failures go in OPPOSITE directions.** That is the signature of a
27+
* day-boundary projection, ⛔ not an off-by-one constant — and it is why the
28+
* `UTC` row below is a LIT positive control rather than a comment: a fix that
29+
* shifted a constant would make one non-UTC row 30 days, break `UTC`, and read
30+
* as a pass to any suite that measured a single zone.
31+
*
32+
* ⛔ Not a regression of #18241. Before it this input was a hard
33+
* `DATASET_INVALID` / 400 — the preset arm could not produce a window at all.
34+
* What #18241 changed is REACHABILITY, so a refusal became a slightly-too-wide
35+
* answer for non-UTC orgs and a correct one for UTC orgs.
36+
*/
37+
38+
import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
39+
import type { IAnalyticsService, AnalyticsQuery } from '@objectstack/spec/contracts';
40+
import { DatasetSchema } from '@objectstack/spec/ui';
41+
import { compileDataset } from '../dataset-compiler.js';
42+
import { DatasetExecutor } from '../dataset-executor.js';
43+
44+
/**
45+
* Noon UTC on the frozen day, so all three zones below read the SAME calendar
46+
* day (`2026-09-09`) and no row is measuring a "which day is it" difference
47+
* instead of the projection this file is about.
48+
*/
49+
const FROZEN_NOW = new Date('2026-09-09T12:00:00.000Z');
50+
51+
const pipeline = DatasetSchema.parse({
52+
name: 'pipeline',
53+
label: 'Pipeline',
54+
object: 'opportunity',
55+
dimensions: [
56+
{ name: 'lead_source', field: 'lead_source', type: 'string' },
57+
{ name: 'close_date', field: 'close_date', type: 'date' },
58+
],
59+
measures: [{ name: 'revenue', aggregate: 'sum', field: 'amount' }],
60+
});
61+
62+
function recordingService(): { service: IAnalyticsService; seen: AnalyticsQuery[] } {
63+
const seen: AnalyticsQuery[] = [];
64+
const service: IAnalyticsService = {
65+
query: vi.fn(async (q: AnalyticsQuery) => {
66+
seen.push(q);
67+
return { rows: [], fields: [] };
68+
}),
69+
getMeta: async () => [],
70+
};
71+
return { service, seen };
72+
}
73+
74+
/**
75+
* The `compareTo` window the executor actually shifted to, read off the wire.
76+
*
77+
* The primary pass forwards the selection's own `dateRange` — the preset STRING
78+
* — while `runCompare` replaces it with the shifted explicit `[start, end]`
79+
* array, so the array arm is an unambiguous discriminator between the two
80+
* passes and needs no ordering assumption.
81+
*/
82+
async function comparisonWindow(timezone: string): Promise<[string, string]> {
83+
const { service, seen } = recordingService();
84+
await new DatasetExecutor(service).execute(compileDataset(pipeline), {
85+
dimensions: ['lead_source'],
86+
measures: ['revenue'],
87+
timeDimensions: [{ dimension: 'close_date', dateRange: 'this_month' }],
88+
compareTo: { kind: 'previousYear' as const, dimension: 'close_date' },
89+
timezone,
90+
});
91+
const shifted = seen
92+
.map((q) => (q.timeDimensions ?? []).find((t) => t.dimension === 'close_date')?.dateRange)
93+
.filter((r): r is [string, string] => Array.isArray(r));
94+
expect(shifted.length, 'exactly one shifted comparison pass').toBeGreaterThan(0);
95+
return shifted[0];
96+
}
97+
98+
/** Inclusive length of a `[YYYY-MM-DD, YYYY-MM-DD]` window, in calendar days. */
99+
function inclusiveDays([start, end]: [string, string]): number {
100+
return Math.round((Date.parse(`${end}T00:00:00Z`) - Date.parse(`${start}T00:00:00Z`)) / 86_400_000) + 1;
101+
}
102+
103+
describe('compareTo — a calendar preset keeps its length in every reference zone (#18245)', () => {
104+
beforeEach(() => {
105+
vi.useFakeTimers({ toFake: ['Date'] });
106+
vi.setSystemTime(FROZEN_NOW);
107+
});
108+
afterEach(() => {
109+
vi.useRealTimers();
110+
});
111+
112+
// ── the positive control, and it stays lit ────────────────────────────────
113+
it('UTC — September 2026 compares against all 30 days of September 2025', async () => {
114+
const window = await comparisonWindow('UTC');
115+
expect(window).toEqual(['2025-09-01', '2025-09-30']);
116+
expect(inclusiveDays(window)).toBe(30);
117+
});
118+
119+
// ── the two defects, failing in OPPOSITE directions ───────────────────────
120+
it('Asia/Shanghai — does not start a day early (was 2025-08-31, 31 days)', async () => {
121+
const window = await comparisonWindow('Asia/Shanghai');
122+
expect(window[0], 'the zone is EAST of UTC, so its midnight is the PREVIOUS UTC day').toBe('2025-09-01');
123+
expect(window).toEqual(['2025-09-01', '2025-09-30']);
124+
expect(inclusiveDays(window)).toBe(30);
125+
});
126+
127+
it('America/New_York — does not end a day late (was 2025-10-01, 31 days)', async () => {
128+
const window = await comparisonWindow('America/New_York');
129+
expect(window[1], 'the zone is WEST of UTC, so its next-month midnight is the NEXT UTC day').toBe('2025-09-30');
130+
expect(window).toEqual(['2025-09-01', '2025-09-30']);
131+
expect(inclusiveDays(window)).toBe(30);
132+
});
133+
134+
// ── one assertion the three rows share, stated as the invariant ───────────
135+
it('every reference zone reports the same 30-day window', async () => {
136+
const zones = ['UTC', 'Asia/Shanghai', 'America/New_York', 'Pacific/Kiritimati', 'Pacific/Niue'];
137+
const windows = await Promise.all(zones.map(async (tz) => [tz, await comparisonWindow(tz)] as const));
138+
expect(Object.fromEntries(windows.map(([tz, w]) => [tz, inclusiveDays(w)]))).toEqual(
139+
Object.fromEntries(zones.map((tz) => [tz, 30])),
140+
);
141+
for (const [tz, w] of windows) expect(w, tz).toEqual(['2025-09-01', '2025-09-30']);
142+
});
143+
});

0 commit comments

Comments
 (0)