Skip to content

Commit c81e7ff

Browse files
claude[bot]claude
andauthored
fix(service-analytics): lower a dateRange preset in runCompare instead of refusing it as an invalid date (#18241)
Fixes #17973 Clause-②: no ## What was wrong `DatasetExecutor.runCompare` read the STRING arm of `dateRange` as `[range, range]` — the degenerate fallback PR #17015 removed from every other analytics face. `parseUTC` was handed the preset NAME, so a declared, honoured member of the closed vocabulary was refused outright. **Reproduced first**, driven through the real executor on `b3b43b6ea` (the card's `2c87a48f` line numbers had moved; the shape had not — the degenerate arm was at `:1318`): ``` ### PRESET last_30_days + compareTo ### threw: Error code=DATASET_INVALID status=400 msg=[dataset-executor] invalid date in dateRange: "last_30_days" ### CONTROL explicit window -> [[["2026-01-01","2026-01-31"]],[["2025-12-01","2025-12-31"]]] ### CONTROL bogus string threw: code=DATASET_INVALID status=400 ``` The severity is the FALSE diagnostic. `last_30_days` is exactly what the schema, the dashboard date filter and the docs tell an author to write, so "invalid date" sends them to check a date that is already correct — a repair that does not exist. ## What changed Both arms now go through one face lowering, `lowerDatasetCompareDateRange`, which calls the shared `resolveAnalyticsDateRangeString` for the string arm — the same call the ObjectQL strategy, the native-SQL strategy, the draft-preview evaluator and driver-memory's cube face make. ⛔ No fourth spelling of the lowering was invented. The resolved window is then projected onto the comparison math's UTC calendar by `inclusiveUtcDayWindow`, because `shiftRange` measures `previousPeriod` as whole days between two day-starts and `alignedCompareBucketKey` reads the upper bound as the LAST bucket. `endExclusive` is load-bearing there: the ten calendar presets report the first instant AFTER the window, so reading their bound as inclusive would make every one of them a day too long. That split — report the window the vocabulary resolved, keep the per-face calendar translation downstream — is the one `lowerPreviewDateRange` already makes when it leaves the bare-day widening in its own predicate. Timezone precedence is `buildQuery`'s, verbatim (`selection.timezone` then `context.timezone` then `UTC`), so the comparison window resolves in the same calendar as the primary pass it is compared against. **The array arm is untouched, bound for bound.** After: ``` ### PRESET last_30_days + compareTo ### threw: NOTHING ### windows: [["last_30_days"],[["2026-07-16","2026-08-15"]]] ### CONTROL explicit window -> [[["2026-01-01","2026-01-31"]],[["2025-12-01","2025-12-31"]]] (byte-identical) ### CONTROL bogus string threw: code=ANALYTICS_DATE_RANGE_UNRECOGNIZED status=400 ``` `runCompare` is also registered as the fourth face in the shared conformance kit, as the card asks. ## ⚠️ One consequence worth reviewing On this path an out-of-vocabulary `dateRange` STRING used to answer `DATASET_INVALID`; it now answers the ADR-0112 `ANALYTICS_DATE_RANGE_UNRECOGNIZED` / 400 envelope the other four faces already raise. One condition, one envelope. The changeset states it as a consumer-facing note. The `DATASET_INVALID "invalid date in dateRange"` refusal for an ARRAY bound that is not a date is unchanged. ## Pins The card asks for a pin that asserts the preset **lowers-and-shifts**, not merely that the string changed. Five cases, driven end to end through `DatasetExecutor.execute`, expectations stated literally against the frozen clock and only cross-checked against the shared resolver — ⛔ never computed with `shiftRange`, which would restate the implementation and pass for any window at all: - a CALENDAR preset lowers with its exclusive end honoured (September stops on the 30th, not October 1st) and shifts a year back; - a ROLLING preset reaches NOW's own day and `previousPeriod` shifts by the window's own length; - five distinct presets select five DISTINCT comparison windows (the fallback's real signature was twelve of thirteen names collapsing onto one); - an unrecognised string is refused on the ENVELOPE, not on `toThrow()` — this face DID throw before the fix; - CONTROL: the caller's explicit window is still shifted bound for bound. ⛔ No existing test pinned the defect as setup, so none was repaired, skipped or weakened. The two fixtures that assert `invalid date in dateRange` (`packages/rest/src/analytics-dataset-unlisted-refusal-envelope.test.ts:186` and `packages/services/service-analytics/src/__tests__/unlisted-refusal-envelope.test.ts:264`) both drive the ARRAY arm and are unaffected — measured, not assumed. ## Reverse verification The fix was committed first, then mutated back out on disk. - Mutation landed: anchor `inclusiveUtcDayWindow(lowered)` 1 to 0, degenerate `[td.dateRange as string, td.dateRange as string]` 0 to 1; blob `e0f74df` to `99f8cfb`. - ⛔ Deliberately NOT rebuilt: the conformance test imports `../dataset-executor.js` by relative specifier, so vitest reads SOURCE. Going red without a build is itself the proof of that resolution path — a dist-resolved test would have stayed falsely green. - Result: **4 failed, 32 passed**, and the four are exactly the four wiring controls. - ⭐ The direction is the interesting half. The conformance-kit face registration stayed GREEN, because `lowerDatasetCompareDateRange` itself was untouched — which is precisely the "an exported helper conforms while the live path keeps its old fallback" gap, and precisely why the end-to-end block exists. The array-arm control and the arity pins also stayed green, as they must. - Restored: blob back to `e0f74df` (equal to the HEAD blob), `git diff HEAD` empty, `git status --porcelain` empty, anchors back to 1 and 0. ## Clause-② re-derivation Re-derived from the DELIVERED diff, not from the claim's prediction. `lowerDatasetCompareDateRange` is exported from its source module so the conformance runner can import it relatively, but `index.ts` re-exports a CLOSED named list from `./dataset-executor.js` that does not include it. | read | my symbol | positive control `shiftRange` | negative control `lowerPreviewDateRange` | |---|---|---|---| | in `dist/index.d.ts` export statement | absent | present | absent | | `Symbol in await import(entry)` | **false** | true | false | | grep in `dist/index.js` | **2 hits** | 4 hits | 2 hits | The third row is the point: the symbol IS in the bundle and is NOT reachable. A grep would have answered `yes`. The negative control is a symbol exported from its own source file, not re-exported by `index.ts`, and predating this diff — it proves the method distinguishes the two. ⇒ **Clause-②: no**, so the changeset is `patch`. ## Verification Gate denominator, reconciled against `node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack --ran`, derived at `3a1d131fa`: ``` Run reconciliation — 62 derived, 62 run, 0 NOT-MEASURED, 0 UNRUN. ✓ dispatch-gates --ran: 62 derived famil(ies) accounted for — 62 run, 0 NOT-MEASURED (a DERIVED zero — all 62 recorded an exit code and none of them is 3). ``` Four of the 62 first returned exit **3, PREREQUISITE NOT MET** (`check:dual-build-cjs-loads`, `check:lean-entry-closure`, `check:type-check-debt`) or timed out (`check:query-options-erasure`). ⛔ Not read as green: the full workspace closure was built (`turbo run build`, 72/72 successful) and all four re-run to a real exit 0. Tests: - `@objectstack/service-analytics` full suite — **111 files, 2398 tests, 0 failures**. - `@objectstack/rest` analytics door plus refusal envelope — **35 tests, 0 failures**. Run because the behaviour change is observable through the REST door; the first attempt failed on an unbuilt `dist`, which is a resolution error and not a red gate, so service-analytics was built and it was re-run to a real verdict. - `@objectstack/service-analytics typecheck` — exit 0, and `tsc --listFiles` confirms the program really contains both the edited source and the edited test (1 and 1), with a negative control at 0. - Lint, as a PROVEN narrowing: eslint over the 2 changed source files, **0 errors 0 warnings**, file count read from `--format json`. The invariance claim is read from eslint's own config — `eslint.config.mjs` sets no `parserOptions.project` and no typed rules, so type-aware linting is off and this diff cannot move the verdict on any untouched file. Repo-wide `pnpm lint` is CI's run. ## Consumption-radius sweep Swept by the RULE's consumers, not by the edited package. The changed rule is `runCompare`'s string-arm reading and its refusal envelope; its call chain is `DatasetExecutor.execute`, reached through `AnalyticsService.queryDataset`. - `packages/services/service-analytics` — full suite green. - `packages/rest` — the only other package that drives the executor; both analytics fixtures green. - `packages/client` — SDK over a mocked transport, never runs the executor; no `compareTo` fixture. - `examples/app-crm` — asserts widget metadata shape only, with `{current_quarter_start}` filter tokens rather than a `dateRange` preset; no executor run. - `driver-memory` DST and timezone suites exercise the memory cube face, not this executor, and neither `@objectstack/core` nor driver-memory was touched. ## Acceptance notes - `DatasetExecutor`'s compare math is UTC-calendar throughout (`parseUTC` reads a bare day as UTC midnight, `toISODate` emits a UTC day). For a non-UTC `selection.timezone` a preset's resolved bounds are therefore projected onto UTC day boundaries, which can make the comparison window a day longer than the org's own calendar would say. Pre-existing for the array arm and strictly better than the 400 it replaces, so ⛔ not widened into this PR. --- _Generated by [Claude Code](https://claude.ai/code/session_01URLHobLUJB9K1ABV6ofdjj)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent ecf91cb commit c81e7ff

3 files changed

Lines changed: 314 additions & 3 deletions

File tree

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
---
2+
"@objectstack/service-analytics": patch
3+
---
4+
5+
fix(analytics): a `dateRange` preset plus `compareTo` is lowered and shifted instead of refused as an "invalid date" (#17973)
6+
7+
`DatasetExecutor.runCompare` read the STRING arm of `dateRange` as
8+
`[range, range]` — the degenerate fallback #17015 removed from every other
9+
analytics face. `parseUTC` was handed the preset NAME, so a declared, honoured
10+
member of the closed vocabulary was refused outright. Measured end to end
11+
through the executor, a valid preset plus `compareTo`:
12+
13+
```
14+
DATASET_INVALID 400 [dataset-executor] invalid date in dateRange: "last_30_days"
15+
```
16+
17+
The diagnostic is not merely unhelpful, it is FALSE. `last_30_days` is exactly
18+
what the schema, the dashboard date filter and the docs tell an author to
19+
write, so "invalid date" sends them to check a date that is already correct —
20+
a repair that does not exist. This face was not in #17015's kit, so nothing
21+
measured it and nothing noticed.
22+
23+
Both arms now go through one face lowering, which calls the shared
24+
`resolveAnalyticsDateRangeString` for the string arm — the same call the
25+
ObjectQL strategy, the native-SQL strategy, the draft-preview evaluator and
26+
driver-memory's cube face make — and the lowered window is then projected onto
27+
the comparison math's UTC calendar, with `endExclusive` honoured so that a
28+
calendar preset's exclusive upper bound does not itself add a day to the
29+
projected window. On the UTC calendar, `this_month` plus
30+
`compareTo: { kind: 'previousYear' }` now compares September against the
31+
previous September, rather than refusing. ⚠️ Outside UTC the projection costs a
32+
day of its own — third note below.
33+
34+
Three consequences worth knowing when you upgrade:
35+
36+
- **A string outside the vocabulary now answers the shared envelope.** On this
37+
path it used to be `DATASET_INVALID`; it is now
38+
`ANALYTICS_DATE_RANGE_UNRECOGNIZED` / 400, the ADR-0112 envelope the other
39+
faces already raise, with the message that lists the thirteen declared preset
40+
names. One condition, one envelope. Code keying on `DATASET_INVALID` for an
41+
unrecognised `dateRange` STRING should key on
42+
`ANALYTICS_DATE_RANGE_UNRECOGNIZED` instead.
43+
- **The caller's explicit `[start, end]` window is untouched**, bound for bound,
44+
with the inclusive upper reading it has always had — including the
45+
`DATASET_INVALID "invalid date in dateRange"` refusal for a bound that is not
46+
a date, which is unchanged.
47+
- **⚠️ A calendar preset lowered in a NON-UTC zone gives a comparison window one
48+
day too wide** — in either direction, depending on which side of UTC the zone
49+
sits. The comparison math is UTC-calendar throughout (`parseUTC` reads a bare
50+
day as UTC midnight, `toISODate` emits a UTC day), so a window computed
51+
against another zone's calendar is projected onto UTC day boundaries: east of
52+
UTC the start lands a day early, west of UTC the end lands a day late.
53+
Measured through the executor, `this_month` plus
54+
`compareTo: { kind: 'previousYear' }` frozen at `2026-09-09` —
55+
`UTC` gives `['2025-09-01','2025-09-30']` (30 days, correct),
56+
`Asia/Shanghai` gives `['2025-08-31','2025-09-30']` and `America/New_York`
57+
gives `['2025-09-01','2025-10-01']` (31 days each). ⛔ This is NOT a
58+
regression: the same input used to be refused outright, so no
59+
previously-working input behaves differently — what changed is that the
60+
preset arm produces a window at all, which is what makes the projection
61+
observable. Tracked in #18245. It is deliberately not repaired here, because
62+
a timezone-aware calendar-day extraction in this module would be the second
63+
implementation `analytics-date-range.ts`'s own header exists to refuse.
64+
65+
`runCompare` is also registered as a face in the shared `dateRange` conformance
66+
kit, so the next face that forgets to lower a preset is caught by a test rather
67+
than by a customer.

‎packages/services/service-analytics/src/__tests__/analytics-date-range-conformance.test.ts‎

Lines changed: 145 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,8 +48,11 @@ import {
4848
import { DatasetSchema } from '@objectstack/spec/ui';
4949
import type { Cube } from '@objectstack/spec/data';
5050
import type { ExecutionContext } from '@objectstack/spec/kernel';
51+
import type { AnalyticsQuery, AnalyticsResult, IAnalyticsService } from '@objectstack/spec/contracts';
5152
import { AnalyticsService } from '../analytics-service.js';
5253
import { evaluateAnalyticsQueryOverRows, lowerPreviewDateRange } from '../preview-evaluator.js';
54+
import { compileDataset } from '../dataset-compiler.js';
55+
import { DatasetExecutor, lowerDatasetCompareDateRange } from '../dataset-executor.js';
5356

5457
/** Frozen so the three rolling presets, whose bound is NOW, are comparable. */
5558
const NOW = new Date('2026-09-09T12:34:56.789Z');
@@ -130,10 +133,33 @@ async function lowerViaPreview(range: string | readonly string[]): Promise<Lower
130133
return lowerPreviewDateRange(range);
131134
}
132135

136+
/**
137+
* [#17973] The dataset executor's `compareTo` window — the FOURTH face in this
138+
* package, and the one #17015 never reached. It kept the degenerate
139+
* `[range, range]` fallback every sibling shed, so a DECLARED preset plus
140+
* `compareTo` was refused outright. MEASURED on `b3b43b6ea`, before the fix:
141+
*
142+
* ```
143+
* DATASET_INVALID 400 [dataset-executor] invalid date in dateRange: "last_30_days"
144+
* ```
145+
*
146+
* ⛔ Read off the lowering rather than an emitted filter, for the preview
147+
* face's reason and one more of its own: what this face EMITS is the SHIFTED
148+
* window, so the window it resolved cannot be recovered from the query it
149+
* issues without inverting the shift. `lowerDatasetCompareDateRange` is the ONE
150+
* call site `runCompare` uses for either arm; the `APPLIES the window it
151+
* reports` block below is the control that it really is wired there, driven end
152+
* to end through the executor.
153+
*/
154+
async function lowerViaCompare(range: string | readonly string[]): Promise<LoweredDateRangeWindow> {
155+
return lowerDatasetCompareDateRange(range);
156+
}
157+
133158
const FACES: AnalyticsDateRangeFace[] = [
134159
{ name: 'service-analytics (ObjectQL strategy)', lower: lowerViaObjectql },
135160
{ name: 'service-analytics (native SQL strategy)', lower: lowerViaNativeSql },
136161
{ name: 'service-analytics (draft-preview evaluator)', lower: lowerViaPreview },
162+
{ name: 'service-analytics (dataset executor compareTo)', lower: lowerViaCompare },
137163
];
138164

139165
beforeEach(() => {
@@ -253,3 +279,122 @@ describe('#16322 — the draft-preview evaluator APPLIES the window it reports',
253279
])).toEqual(['lower', 'upper']);
254280
});
255281
});
282+
283+
/**
284+
* [#17973] ⭐ The executor face is registered above on its LOWERING; these cases
285+
* are why that registration means anything — and they are the pin the card
286+
* asks for, because "the string changed" is not the claim. The claim is that a
287+
* declared preset is LOWERED to a real window and that window is SHIFTED.
288+
*
289+
* Driven end to end through `DatasetExecutor.execute`, reading the `dateRange`
290+
* of every pass the executor issues. ⛔ The expectations are stated LITERALLY
291+
* against the frozen clock and only cross-checked against the shared resolver —
292+
* ⛔ never computed with `shiftRange`, which would restate the implementation
293+
* and pass for any window at all.
294+
*
295+
* ⚠️ The PRIMARY pass keeps the preset NAME on purpose: the strategy behind it
296+
* is itself a registered face and lowers it there (#16322). Only the comparison
297+
* pass carries a resolved window, because only it is shifted.
298+
*/
299+
const CMP_DATASET = DatasetSchema.parse({
300+
name: 'trend', label: 'Trend', object: 'events', include: [],
301+
dimensions: [{ name: 'created_at', field: 'created_at', type: 'date', dateGranularity: 'month' }],
302+
measures: [{ name: 'count', aggregate: 'count' }],
303+
});
304+
305+
/** The `dateRange` of every pass the executor issues — primary first, then the shifted compare pass. */
306+
async function comparePasses(
307+
dateRange: string | readonly unknown[],
308+
kind: 'previousPeriod' | 'previousYear',
309+
): Promise<unknown[]> {
310+
const seen: AnalyticsQuery[] = [];
311+
const svc: IAnalyticsService = {
312+
query: vi.fn(async (q: AnalyticsQuery): Promise<AnalyticsResult> => {
313+
seen.push(q);
314+
return { rows: [], fields: [] };
315+
}),
316+
getMeta: async () => [],
317+
};
318+
await new DatasetExecutor(svc).execute(
319+
compileDataset(CMP_DATASET),
320+
{
321+
dimensions: ['created_at'], measures: ['count'],
322+
timeDimensions: [{ dimension: 'created_at', dateRange, granularity: 'month' }],
323+
compareTo: { kind },
324+
} as never,
325+
CTX,
326+
);
327+
return seen.map((q) => (q.timeDimensions ?? []).map((t) => (t as { dateRange?: unknown }).dateRange));
328+
}
329+
330+
describe('#17973 — the dataset executor APPLIES the window it reports', () => {
331+
// The frozen clock is the file-level `beforeEach`; the rolling presets' upper
332+
// bound is NOW, so it has to be the same NOW here.
333+
334+
it('a CALENDAR preset is LOWERED, its exclusive end honoured, and the window SHIFTED', async () => {
335+
// The shared resolver's own answer for the frozen clock: September 2026,
336+
// ending at OCTOBER's first instant because the calendar presets stop
337+
// BEFORE their end.
338+
const w = resolveAnalyticsDateRangeString('this_month', { now: NOW });
339+
expect([w.start, w.end, w.endExclusive])
340+
.toEqual(['2026-09-01T00:00:00.000Z', '2026-10-01T00:00:00.000Z', true]);
341+
342+
// ⇒ the days that window COVERS are the 1st to the 30th — ⛔ not to the
343+
// 1st of October, which is the off-by-one an inclusive reading of an
344+
// exclusive bound produces — and `previousYear` is those same days a
345+
// calendar year back.
346+
expect(await comparePasses('this_month', 'previousYear')).toEqual([
347+
['this_month'],
348+
[['2025-09-01', '2025-09-30']],
349+
]);
350+
});
351+
352+
it('a ROLLING preset reaches NOW\'s own day and `previousPeriod` shifts by the window\'s length', async () => {
353+
const w = resolveAnalyticsDateRangeString('last_30_days', { now: NOW });
354+
// Ends at NOW, a moment it REACHES (#16179 leaves the rolling leg inclusive).
355+
expect([w.start, w.end, w.endExclusive])
356+
.toEqual(['2026-08-10T00:00:00.000Z', '2026-09-09T12:34:56.789Z', false]);
357+
358+
// 2026-08-10 through 2026-09-09 inclusive is 31 days; `previousPeriod` is
359+
// the equal-length window ending the day before this one starts.
360+
expect(await comparePasses('last_30_days', 'previousPeriod')).toEqual([
361+
['last_30_days'],
362+
[['2026-07-10', '2026-08-09']],
363+
]);
364+
});
365+
366+
it('⛔ distinct presets select DISTINCT comparison windows', async () => {
367+
// The fallback's real signature: twelve of thirteen names collapsed onto
368+
// one window, so a green "it did not throw" is not enough — the windows
369+
// have to differ from each other.
370+
const windows = new Set<string>();
371+
for (const preset of ['today', 'this_week', 'this_month', 'this_quarter', 'this_year']) {
372+
windows.add(JSON.stringify((await comparePasses(preset, 'previousYear'))[1]));
373+
}
374+
expect(windows.size).toBe(5);
375+
});
376+
377+
it('⛔ an unrecognised string is REFUSED with the ADR-0112 envelope, not DATASET_INVALID', async () => {
378+
// ⛔ On the ENVELOPE, not `toThrow()`: this face DID throw before the fix
379+
// — it threw `DATASET_INVALID "invalid date in dateRange"`, its own
380+
// envelope for a condition four faces already answered with one.
381+
let thrown: (Error & { code?: string; status?: number }) | undefined;
382+
try {
383+
await comparePasses('not a range at all', 'previousPeriod');
384+
} catch (e) {
385+
thrown = e as Error & { code?: string; status?: number };
386+
}
387+
expect(thrown?.code).toBe('ANALYTICS_DATE_RANGE_UNRECOGNIZED');
388+
expect(thrown?.status).toBe(400);
389+
});
390+
391+
it('⛔ CONTROL — the CALLER\'s explicit window is still shifted bound for bound', async () => {
392+
// ⭐ Without this, every assertion above is satisfied by a face that
393+
// rewrote the array arm too. The answer is byte-identical to the one
394+
// #17124 pinned before this change.
395+
expect(await comparePasses(['2026-01-01', '2026-01-31'], 'previousPeriod')).toEqual([
396+
[['2026-01-01', '2026-01-31']],
397+
[['2025-12-01', '2025-12-31']],
398+
]);
399+
});
400+
});

‎packages/services/service-analytics/src/dataset-executor.ts‎

Lines changed: 102 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,13 @@ import type {
99
} from '@objectstack/spec/contracts';
1010
import { emptyGroupValueFor, type FilterCondition } from '@objectstack/spec/data';
1111
import type { ExecutionContext } from '@objectstack/spec/kernel';
12-
import { bucketKeyToCalendarRange, filterTokenContextFrom, resolveFilterTokens } from '@objectstack/core';
12+
import {
13+
bucketKeyToCalendarRange,
14+
filterTokenContextFrom,
15+
resolveAnalyticsDateRangeString,
16+
resolveFilterTokens,
17+
type LoweredDateRangeWindow,
18+
} from '@objectstack/core';
1319
import type { CompiledDataset, DerivedMeasureSpec } from './dataset-compiler.js';
1420
import { explicitDateRangeWindow } from './date-range-array-arm.js';
1521
import { datasetInvalidError } from './dataset-refusal.js';
@@ -500,6 +506,79 @@ function shiftYear(date: string, years: number): string {
500506
return toISODate(d.getTime());
501507
}
502508

509+
/**
510+
* [#17973] Lower one `dateRange` the way `compareTo` reads it — this executor's
511+
* face in the shared `dateRange` conformance kit
512+
* (`analyticsDateRangeConformanceFindings`), and the ONE call site
513+
* {@link DatasetExecutor.runCompare} uses for either arm.
514+
*
515+
* ## What was wrong
516+
*
517+
* The STRING arm was spelled `[td.dateRange, td.dateRange]` — the degenerate
518+
* `[range, range]` fallback #17015 removed from every OTHER analytics face.
519+
* `parseUTC` was handed the preset NAME, so a DECLARED, honoured member of the
520+
* closed vocabulary was refused. MEASURED on `b3b43b6ea`, `last_30_days` plus
521+
* `compareTo`:
522+
*
523+
* ```
524+
* DATASET_INVALID 400 [dataset-executor] invalid date in dateRange: "last_30_days"
525+
* ```
526+
*
527+
* ⇒ the diagnostic is not merely unhelpful, it is FALSE, and it sends the
528+
* author to check a date that is exactly what the schema and the docs tell them
529+
* to write. This face was not in #17015's kit, so nothing measured it.
530+
*
531+
* ## Why it reports INSTANTS while `runCompare` shifts DAYS
532+
*
533+
* What is reported here is the window the VOCABULARY resolved, which is what
534+
* the kit holds every face to. Projecting it onto this module's UTC calendar is
535+
* a per-face calendar translation and stays downstream, in
536+
* {@link inclusiveUtcDayWindow} — the same split `lowerPreviewDateRange` makes
537+
* when it leaves the #3777 bare-day widening in its own predicate.
538+
*
539+
* ⛔ The ARRAY arm is the CALLER's explicit window and is handed back bound for
540+
* bound, with the inclusive upper reading it has always had (#16179) — the
541+
* refusal for anything that is not a two-bound window is
542+
* `explicitDateRangeWindow`'s, unchanged (#17124).
543+
*
544+
* @throws the ADR-0112 `400 ANALYTICS_DATE_RANGE_UNRECOGNIZED` envelope for a
545+
* string outside `DATE_RANGE_PRESETS` — the same envelope the sibling faces
546+
* raise, replacing the `DATASET_INVALID` this arm used to answer with.
547+
*/
548+
export function lowerDatasetCompareDateRange(
549+
dateRange: string | readonly unknown[],
550+
timezone?: string,
551+
): LoweredDateRangeWindow {
552+
if (!Array.isArray(dateRange)) {
553+
const window = resolveAnalyticsDateRangeString(dateRange as string, { timezone });
554+
return { start: window.start, end: window.end, endExclusive: window.endExclusive };
555+
}
556+
const [start, end] = explicitDateRangeWindow(dateRange as readonly unknown[]);
557+
return { start, end, endExclusive: false };
558+
}
559+
560+
/**
561+
* [#17973] The UTC calendar days a lowered window covers, as the INCLUSIVE
562+
* `[first, last]` pair every piece of `compareTo` math in this module takes.
563+
*
564+
* {@link shiftRange} measures `previousPeriod`'s length as a count of whole
565+
* days between two day-starts, and {@link alignedCompareBucketKey} reads
566+
* `currentRange[1]` as the LAST bucket the window contains — so both bounds
567+
* must name days the window really covers.
568+
*
569+
* ⚠️ `endExclusive` is therefore load-bearing, not decoration: the ten calendar
570+
* presets report the first instant AFTER the window (`this_month` ends at the
571+
* next month's start), so the last day they cover is the one holding the
572+
* instant before that bound. Reading their bound as inclusive would make every
573+
* calendar preset one day too long and shift `previousPeriod` by a day. The
574+
* three rolling presets end at NOW, a moment they REACH, so their bound is
575+
* already the last day.
576+
*/
577+
function inclusiveUtcDayWindow(window: LoweredDateRangeWindow): [string, string] {
578+
const endMs = parseUTC(window.end);
579+
return [toISODate(parseUTC(window.start)), toISODate(window.endExclusive ? endMs - 1 : endMs)];
580+
}
581+
503582
/**
504583
* Resolve which time dimension a `compareTo` shifts (#5011).
505584
*
@@ -1313,9 +1392,29 @@ export class DatasetExecutor {
13131392
// bound in from the lower one, so a one-element array silently became a
13141393
// point window HERE while the primary pass it is compared against may have
13151394
// read the same document as all of history.
1395+
//
1396+
// [#17973] The STRING arm is the CLOSED preset vocabulary, lowered by the one
1397+
// shared `resolveAnalyticsDateRangeString` every other face calls and then
1398+
// projected onto this module's UTC calendar. ⛔ What this replaced was the
1399+
// degenerate `[range, range]` fallback #17015 removed everywhere else: it
1400+
// handed `parseUTC` the preset NAME, so `last_30_days` — declared, honoured,
1401+
// and exactly what the schema tells an author to write — came back as
1402+
// `DATASET_INVALID "invalid date in dateRange"`. A false diagnostic on a
1403+
// valid input has no repair to send the author to.
1404+
//
1405+
// The timezone precedence is `buildQuery`'s, verbatim, so the comparison
1406+
// window is resolved in the SAME calendar as the primary pass it is
1407+
// compared against — a preset resolved here in UTC while the primary pass
1408+
// read it in the org's zone would misalign the two grids by a day.
1409+
const lowered = lowerDatasetCompareDateRange(
1410+
td.dateRange as string | readonly unknown[],
1411+
selection.timezone ?? context?.timezone ?? 'UTC',
1412+
);
13161413
const range: [string, string] = Array.isArray(td.dateRange)
1317-
? explicitDateRangeWindow(td.dateRange as readonly unknown[])
1318-
: [td.dateRange as string, td.dateRange as string];
1414+
? // The caller's own bounds, untouched — `explicitDateRangeWindow` already
1415+
// refused anything that is not a two-bound window (#17124).
1416+
[lowered.start, lowered.end]
1417+
: inclusiveUtcDayWindow(lowered);
13191418
const shifted = shiftRange(range, cmp.kind);
13201419
const shiftedTd = (selection.timeDimensions ?? []).map((t) =>
13211420
t.dimension === dimension ? { ...t, dateRange: shifted } : t,

0 commit comments

Comments
 (0)