Skip to content

fix(service-analytics): compareTo resolves its comparison window on the reference calendar, not UTC - #18596

Merged
os-project-manager merged 2 commits into
mainfrom
claude/issue-18245-compareto-timezone-projection
Sep 17, 2026
Merged

os-project-manager merged 2 commits into
mainfrom
claude/issue-18245-compareto-timezone-projection

Conversation

@os-project-manager

Copy link
Copy Markdown
Collaborator

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:

Neither is filed: the first is not measured, the second is declared.


Generated by Claude Code

…s red

Drives DatasetExecutor.execute with `this_month` + previousYear frozen at
2026-09-09 and reads the shifted comparison window off the wire. Reproduces
the reported table byte-for-byte: UTC 30 days (lit positive control, green),
Asia/Shanghai starts a day early, America/New_York ends a day late.

Claude-Session: https://claude.ai/code/session_01WmBwEiWPff9JZPd5BSGNeH
Co-authored-by: Claude <noreply@anthropic.com>
…e calendar

Deletes dataset-executor's local parseUTC/toISODate pair — a UTC-calendar
duplicate of @objectstack/core's shared datetime vocabulary — and routes the
compareTo day math through that vocabulary instead.

A bare YYYY-MM-DD is a calendar day, so the year shift, the previous-period
length and the bucket ordinals keep running on the zone-free UTC proxy
zonedDateStartToUtcMs yields for an unset zone. The one seam a reference zone
reaches is the projection of the lowered window's INSTANTS onto days, which now
calls bucketDateKey at 'day' 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; threading it only into the lowering (as before) left the projection
on UTC and misaligned the two grids by a day in opposite directions either side
of the meridian.

Claude-Session: https://claude.ai/code/session_01WmBwEiWPff9JZPd5BSGNeH
Co-authored-by: Claude <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

11 anchor(s) derived from 1 changed package(s); no hand-written page names any of them, so this run has nothing to list — not a clean bill of health. This check sees only pages that NAME a derived anchor: one that documents this change in prose, or enumerates it in an authoring dialect, names none and stays invisible to it on every run.

What this run could not see
  • the SDK route bridge reached 60 of 215 client-bound route-ledger rows — the other 155 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 155: 0 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 55 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 100 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.

Coarse fallback — 9 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json e0d05538c0d0275728d25acfafbab259e23d0710 → packageMentionDocs.

Which tree this was computed on

This run read content/docs from e12e9730266a82d2c8edf76a09e84501e2d9a047 — the merge of head dfb909bd052b63982a795a1a00b27cbb1735daed into base e0d05538c0d0275728d25acfafbab259e23d0710, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin e12e9730266a82d2c8edf76a09e84501e2d9a047 && git checkout e12e9730266a82d2c8edf76a09e84501e2d9a047
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin e0d05538c0d0275728d25acfafbab259e23d0710 dfb909bd052b63982a795a1a00b27cbb1735daed && git checkout -B drift-repro e0d05538c0d0275728d25acfafbab259e23d0710 && git merge --no-ff dfb909bd052b63982a795a1a00b27cbb1735daed

node scripts/docs-audit/affected-docs.mjs --json e0d05538c0d0275728d25acfafbab259e23d0710

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

@github-actions github-actions Bot added documentation Improvements or additions to documentation tests tooling labels Sep 17, 2026
@os-project-manager
os-project-manager marked this pull request as ready for review September 17, 2026 07:46
@os-project-manager
os-project-manager added this pull request to the merge queue Sep 17, 2026
Merged via the queue into main with commit ad067ad Sep 17, 2026
36 checks passed
@os-project-manager
os-project-manager deleted the claude/issue-18245-compareto-timezone-projection branch September 17, 2026 08:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/m tests tooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[finding] service-analytics: a non-UTC calendar preset under compareTo is projected onto UTC days, so the comparison window is one day too wide

2 participants