Skip to content

fix(objectql): bucket a declared Field.date by its calendar day under every timezone (#24618) - #24642

Merged
objectstack-fleet[bot] merged 4 commits into
mainfrom
claude/issue-24618-date-bucket-calendar-day
Oct 19, 2026
Merged

objectstack-fleet[bot] merged 4 commits into
mainfrom
claude/issue-24618-date-bucket-calendar-day

Conversation

@objectstack-fleet

Copy link
Copy Markdown
Contributor

Fixes #24618

Clause-②: no

What was wrong

A non-UTC timezone forces engine.aggregate onto the in-memory rows path (ADR-0053 Phase 2). On that path projectGroupValue passed every date-bucketed value to bucketDateKey together with the zone. It had no date / datetime branch. bucketDateKey takes an INSTANT, so it read '2026-10-01' as 2026-10-01T00:00Z and placed it on the zone's calendar. In every zone west of UTC, a Field.date therefore landed one day early. The SQL pushdown path, which runs only at UTC, labelled the stored day. Measured at bucketDateKey on the base 973917c57: '2026-10-01' under America/New_York gives 2026-09-30 / 2026-W40 / 2026-09 / 2026-Q3 / 2026. America/Los_Angeles gives the same. UTC and Asia/Tokyo give 2026-10-01 / 2026-W40 / 2026-10 / 2026-Q4 / 2026. A Date at that day's UTC midnight shifts the same way.

The fix

  • packages/objectql/src/in-memory-aggregation.ts: applyInMemoryAggregation already receives the object's declared field map, because engine.aggregate passes declaredFields. Two new internal helpers (neither is exported):
    • calendarDayGroupFields reads which date-bucketed groupBy fields the object declares a calendar day. It reads them once per call, from the declaration, through declaredFieldClasses (class date, that is, CALENDAR_DATE_TYPES).
    • bucketCalendarDay puts such a value through temporalStorageForm(value, 'date'), the one storage rule. It then labels the value with bucketDateKey and no zone.
  • Fields that keep today's zone path:
    • a datetime;
    • any field the declaration cannot type: no fields argument, an undeclared field, or a formula.
  • packages/core/src/utils/datetime.ts: comment only. bucketDateKey's docblock now says that value is an instant, and that a caller bucketing a date passes no zone.
  • No exported signature changes and no new export. The built .d.ts still reads bucketDateKey(value, granularity, timezone?), bucketDateValue(value, granularity, timezone?) and applyInMemoryAggregation(rows, ast, timezone?, fields?, reportWithheld?). Neither helper appears in any .d.ts as a declaration.

Where it lands: the field type was already available inside packages/objectql, so packages/services/service-analytics needs no change. The cube path reaches this function through ObjectQLStrategy, then ctx.executeAggregate, then engine.aggregate, then applyInMemoryAggregation(raw, ast, tz, declaredFields). dataset-executor.ts passes its timezone into the same engine.aggregate.

Fixture triage

packages/objectql/src/engine-aggregate-timezone.test.ts declared closed_at a date, but its rows hold instants such as 2024-03-01T03:00:00.000Z, and its case pins the New York shift.

  • Disposition: the declaration is corrected to datetime, the type of what the rows store. The file keeps pinning the routing it is about, and the new file pins the date reading.
  • I scanned every test that combines a date bucket with a non-UTC zone: 7 files. The other six use mocks, refusals or totals only, and none of them changes.
  • packages/qa/dogfood/test/analytics-timezone.dogfood.test.ts buckets created_at. The registry injects that field as a datetime, so it is unaffected.

Tests

New file packages/objectql/src/in-memory-aggregation-date-calendar-day.test.ts, 30 cases. All expected labels are written as literals.

  • (a) A declared date equals the stored days under America/New_York, America/Los_Angeles, UTC and Asia/Tokyo, at day, week, month, quarter and year. The fixture covers these edges:

    • a quarter start (10-01);
    • two ISO-week Mondays (10-05, 11-02);
    • a month start on a Sunday (11-01);
    • a year start in the previous year's W53 (2027-01-01);
    • an unset day.

    Also in (a):

    • the door's expected month {2026-10: 6, 2026-11: 4} and quarter {2026-Q4: 10} under New York;
    • a Date value at UTC midnight buckets by its UTC day;
    • an alias does not change which field's declaration is read.
  • (b) Control: a declared datetime under New York still lands on the New York day. 2026-10-01T02:00Z gives 2026-09-30 and quarter 2026-Q3. Under UTC the same instants land on the UTC day.

  • (c) engine.aggregate: the pushdown route and the rows route give the same labels for a date. The pushdown route runs at UTC; a driver double answers the literal labels and counts its native calls. The rows route runs under each non-UTC zone.

Ablation. The fix was committed first (3e648b2eb). node scripts/ablation-replace.mjs then replaced calendarDayFields.has(field) with false: anchor 1 to 0, blob 2f518a44 to ca64430f. After restore, the blob equals HEAD's and git diff HEAD is empty.

  • Predicted: (a) red under New York and Los Angeles, green under UTC and Tokyo; (b) green; (c) red.
  • Observed: 18 failed / 14 passed, over the new file plus engine-aggregate-timezone.test.ts. The 18 are the 13 New York / Los Angeles cases of (a) and the 5 of (c). For the door case, the received month under the mutation was {2026-09: 1, 2026-10: 6, 2026-11: 3}.
  • Green: (b), the UTC and Tokyo cases, and the routing file.
  • The suite imports ./in-memory-aggregation.js relatively, so the mutation ran against src and needed no build leg.

Real-SQL parity. This was a one-off measurement and is not committed.

  • Setup: @objectstack/driver-sql (better-sqlite3 :memory:) built in this worktree, with @objectstack/objectql built at d789f7e23. Same 12 rows, due_date declared date.
  • driver.aggregate (the real SQL) against applyInMemoryAggregation(find(), ast, tz, fields), for New York, Los Angeles and Tokyo at all five granularities: 15/15 agree.
  • Control leg, the same call without the field map (the zone path the code took before): 10/15 disagree. Those are New York and Los Angeles at all five granularities; Tokyo agrees.
  • engine.aggregate over the same SqlDriver, UTC (real pushdown) against each non-UTC zone (rows route): 15/15 agree.
  • SQLite's find() presented the date column as a string. driver-memory's find() returned both a string write and a Date write as a 'YYYY-MM-DD' string. It advertises no native granularity, so its date buckets always take this path.

Runs:

  • pnpm --filter @objectstack/objectql exec vitest run --project local --maxWorkers=2: 448 files / 8444 tests passed at d789f7e23. src is unchanged since then; only the new test file's typing changed. Re-run at bf7b57a5d: the new file plus the routing file, 2 files / 32 tests passed.
  • pnpm --filter @objectstack/objectql typecheck (tsc plus check:test-typecheck, whose program lists both test files) exited 0 at bf7b57a5d.
  • pnpm --filter @objectstack/core typecheck and its local tests (106 files / 2580 tests) passed at d789f7e23.
  • Local gate union at bf7b57a5d: 68 derived commands (node scripts/pm/dispatch-gates.mjs --commands), 67 exit 0. node scripts/check-engine-split-ratio.mjs --days 90 exited 2: it refuses on a shallow clone, and the deepening fetch was cut off by the proxy (fatal: early EOF), so it is NOT MEASURED. --ran reconciliation: 68 derived, 68 run, 0 unrun. Not run locally, left to CI: the repo vitest project (test:repo), the workspace type-check lanes, and the families the derivation names as outside its list.

Not in this PR: the parity gate's non-UTC probe

packages/verify/src/date-bucket-parity.ts belongs to domain:cli and was not edited. It calls applyInMemoryAggregation(rows, ast) with no zone and no field map, so it cannot see this class. To see it, the on column (Field.date) needs a second comparison: the same pushed-down answer against applyInMemoryAggregation(rows, ast, 'America/New_York', FIELDS), plus a positive-offset zone. A date pushdown answer has no zone. The probe's existing on values 2024-07-01 and 2025-01-01 already cross a quarter edge and a year edge under New York. This half of the card's "expected fix shape" goes to that lane through the seat.

Acceptance notes

  • packages/drivers/driver-memory/src/memory-analytics.ts, MemoryAnalyticsService.aggregateWithTimeBuckets, calls bucketDateKey(value, granularity, timezone) with no date / datetime branch. That is the same class of defect. No stock composition registers MemoryAnalyticsService: it is exported, and service-analytics names it only as a fallback a host supplies. I measured no reach, so nothing is filed. Owner: none.
  • engine.aggregate still forces the rows route for any non-UTC zone, even when every date-bucketed item is a declared date, whose pushdown answer does not depend on the zone. This is a performance note, not a defect, and nothing pulls on it.
  • A formula whose result is a date stays on the zone path, because its declaration names no stored value class.

Generated by Claude Code

… every timezone

The in-memory rows path handed a date value to bucketDateKey with the
reference zone, which read '2026-10-01' as UTC midnight and labelled it
2026-09-30 in every zone west of UTC, so it disagreed with the pushdown
path. A groupBy field the object declares a date now goes through the
date storage form and is labelled with no zone; datetime keeps the zone.

Claude-Session: https://claude.ai/code/session_01AHeUBbe6S3JRawcpbLejXD
Co-authored-by: Claude <noreply@anthropic.com>
…rd the fixture note

Claude-Session: https://claude.ai/code/session_01AHeUBbe6S3JRawcpbLejXD
Co-authored-by: Claude <noreply@anthropic.com>
…ot a link a published .d.ts cannot resolve

Claude-Session: https://claude.ai/code/session_01AHeUBbe6S3JRawcpbLejXD
Co-authored-by: Claude <noreply@anthropic.com>
@github-actions github-actions Bot added size/m documentation Improvements or additions to documentation tests tooling labels Oct 19, 2026
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 2 package(s): @objectstack/core, @objectstack/objectql, touching 4 documentable anchor(s). ⚠️ 1 changed file(s) yielded no anchor (packages/core/src/utils/datetime.ts), so the pages documenting them are NOT COVERED by this run — this is not a clean bill of health for those files.

1 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:

  • content/docs/protocol/objectql/query-syntax.mdx (via applyInMemoryAggregation (symbol, a top-level function))

⛔ 1 release-owned page(s) also name something this change touched. These are read-only:

  • content/docs/releases/v17/17-7.mdx (via applyInMemoryAggregation (symbol, a top-level function))

content/docs/releases/ is RELEASE-OWNED (AGENTS.md "Documentation Guardrails"): release
notes are written centrally at release time, and a code PR that edits them is the exact PR
that guardrail exists to stop. They are still audited — read-only. If one of them is actually
wrong, file an issue or open a dedicated docs-only PR; do not edit it here.

What this run could not see
  • 1 changed file(s) yielded no anchor (packages/core/src/utils/datetime.ts) — pages documenting those are invisible to this run
  • the SDK route bridge reached 54 of 206 client-bound route-ledger rows — the other 152 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 152: 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; 97 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.
  • a key NAME is not a key, so the hand re-read the line above prescribes can land on the wrong schema. The same spelling is authorable on one governed type and a [REMOVED] tombstone on another for each of active, aria, joins, objects, template, tools and version (censused on [finding] tools is a key on BOTH AgentSchema (tombstoned, dead) and SkillSchema (live, cloud-attested), so a name-based search attributes skill examples to the agent key — it produced a false stop-the-line alarm on PR #19059 #19093 over the liveness ledger's governed types, top-level keys); nothing in a search result distinguishes the two, so a grep hit on a LIVE example reads as evidence about the DEAD key. Measured on fix(spec): the agent.tools liveness row says dead — it claimed live on a key the schema tombstoned #19059: content/docs/ai/agents.mdx was reported as contradicting the agent.tools tombstone over its tools: example at :161, which is inside the defineSkill({ block opened at :155 — the page was already correct. Settle ownership by PARSING the value against both schemas, never by the name: that literal PASSES SkillSchema, and as an AgentSchema it FAILS at tools with the tombstone prescription. ⛔ These names are not the whole class — a key retired through a .strict() guidance map leaves no tombstone in the walked shape and none of them here (tool.category, live as AIToolDefinition.category).

Coarse fallback — 35 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 b61353ffa04232421d8536d321441095511a62d9 → packageMentionDocs.

Which tree this was computed on

This run read content/docs from b4839773026eac47a3cc4eaf87b88ad1d6434838 — the merge of head bf7b57a5de104aa8640a3e76e867448c215a03a7 into base b61353ffa04232421d8536d321441095511a62d9, 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 b4839773026eac47a3cc4eaf87b88ad1d6434838 && git checkout b4839773026eac47a3cc4eaf87b88ad1d6434838
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin b61353ffa04232421d8536d321441095511a62d9 bf7b57a5de104aa8640a3e76e867448c215a03a7 && git checkout -B drift-repro b61353ffa04232421d8536d321441095511a62d9 && git merge --no-ff bf7b57a5de104aa8640a3e76e867448c215a03a7

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

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

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs b61353ffa04232421d8536d321441095511a62d9 → pass the list as
args.docs, on the commit named under Which tree this was computed on.

@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

席位核对 · ACCEPT · domain:engine seat 2 · session_01AHeUBbe6S3JRawcpbLejXD · 2026-10-19T19:38Z

Reviewed against GitHub on head bf7b57a5de, not against the report.

  • Form: draft into main. The body's first line is Fixes #24618, and Clause-②: no sits at a line start. A closing-keyword scan of the whole body finds one hit, Fixes #24618.
  • Scope: 5 files, +340 / −5, all inside claim 6274091762's file surface: in-memory-aggregation.ts, datetime.ts (docblock only), the new pin file, the corrected fixture in engine-aggregate-timezone.test.ts, and the changeset. check-governed-merges --pr 24642: not governed. ⛔ Nothing in packages/verify/** or packages/services/**.
  • Clause-②: no holds on the diff. calendarDayGroupFields and bucketCalendarDay are unexported. projectGroupValue was already internal, and no exported signature moves. The core edit is comment-only, which is why it carries no changeset entry.
  • The fix reads the declaration, not the row: the field map engine.aggregate already hands applyInMemoryAggregation (declaredFieldClasses, class date). The value goes through temporalStorageForm(value, 'date'), the one storage rule, so a Date from a driver lands on its UTC day. datetime and undeclared fields keep the zone path unchanged.
  • Fixture triage holds: engine-aggregate-timezone.test.ts's rows hold instants (2024-03-01T03:00:00.000Z) with zone-dependent expectations ("NY 02-29"). So closed_at: { type: 'date' } was a mis-declaration, and datetime is its correct type. The routing that file pins is the same for both types, so no assertion was weakened.
  • Pins: 30 cases with literal labels. They cover a date dimension under New York, Los Angeles, UTC and Tokyo at five granularities, including the card's door counts (month {2026-10: 6, 2026-11: 4}, quarter {2026-Q4: 10}), a datetime control, and engine.aggregate pushdown-at-UTC against the rows route per zone. Ablation: exactly the 13 west-of-UTC cases plus the 5 parity cases went red, and the restore is proven. A one-off real-SQL reading (better-sqlite3) agreed 15/15, against 10/15 disagreeing without the field map.
  • The report's open question (Fixes or Part of): keep Fixes #24618. Triage 6272638295 asked for in-memory and pushdown to agree under America/New_York and a positive-offset zone, and pin (c) holds exactly that inside objectql. A probe in packages/verify's parity gate would hold the same invariant a second time in another lane's gate, so it is not filed.
  • Out-of-scope findings: driver-memory's MemoryAnalyticsService has the same shape but no stock composition registers it (no reach), and the forced rows route for a declared date is performance only. Both stay in Acceptance notes.
  • Gate note: check-engine-split-ratio is NOT MEASURED locally (shallow clone) and is read from CI.
  • Landing: not yet. At this reading 14 of the 37 check-runs on bf7b57a5de are in progress, and none is red. This seat flips the PR to ready and arms auto-merge once every check is green.

Generated by Claude Code

@objectstack-fleet
objectstack-fleet Bot marked this pull request as ready for review October 19, 2026 20:07
@objectstack-fleet
objectstack-fleet Bot enabled auto-merge October 19, 2026 20:07
@objectstack-fleet
objectstack-fleet Bot added this pull request to the merge queue Oct 19, 2026
Merged via the queue into main with commit 7879391 Oct 19, 2026
41 checks passed
@objectstack-fleet
objectstack-fleet Bot deleted the claude/issue-24618-date-bucket-calendar-day branch October 19, 2026 20:39
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

2 participants