refactor: switch from Pacific timezone to UTC day boundaries#323
refactor: switch from Pacific timezone to UTC day boundaries#323
Conversation
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| ✅ Deployment successful! View logs |
agent-news | b4113da | Apr 13 2026, 07:15 PM |
|
Preview deployed: https://agent-news-staging.hosting-962.workers.dev This preview uses sample data — beats, signals, and streaks are seeded automatically. |
There was a problem hiding this comment.
Pull request overview
This PR migrates the codebase’s “calendar day” semantics from Pacific time to UTC midnight boundaries across the Durable Object, Worker routes, helper utilities, and the static frontend, including a one-time streak date backfill.
Changes:
- Replaced Pacific-time helper functions with UTC equivalents and updated call sites across DO/routes.
- Updated API/UX semantics to UTC (e.g.,
pacificDate→utcDate,X-Timezone→UTC, docs strings). - Added migration v12 to backfill
streaks.last_signal_datefrom stored signal timestamps; updated helper tests and frontend day grouping.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/lib/helpers.ts | Replaces Pacific timezone utilities with UTC date/window helpers. |
| src/objects/news-do.ts | Switches all day-boundary logic to UTC and adds migration v12 for streak backfill. |
| src/routes/signals.ts | Renames response field to utcDate and updates X-Timezone header + date error message. |
| src/routes/init.ts | Uses getUTCDate() for “today” bundle logic. |
| src/routes/brief.ts | Uses getUTCDate() and updates comments to UTC semantics. |
| src/routes/brief-compile.ts | Uses UTC default date and UTC timestamp formatting in compiled text. |
| src/routes/manifest.ts | Updates API docs string to “defaults to today UTC”. |
| src/lib/do-client.ts | Updates SignalFilters.date documentation to UTC semantics. |
| src/tests/helpers.test.ts | Rewrites tests to assert UTC helper behavior and UTC boundary windows. |
| public/index.html | Updates frontend day grouping/pagination cursor to UTC and removes Pacific display formatting. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
arc0btc
left a comment
There was a problem hiding this comment.
Switches all day-boundary semantics from Pacific time to UTC midnight — a long-overdue simplification that removes DST complexity from the core data pipeline.
What works well:
getUTCDayStart(date)becomingdate + "T00:00:00.000Z"is exactly right — replaces 12 lines ofIntl.DateTimeFormatDST probing with a trivial string concat. Same correctness, vastly less complexity.- The simplify commit (
fix(simplify): use getUTCDayEnd consistently, re-throw migration errors) already addressed the main quality improvements before this review — unifiedgetUTCDayEndusage, removed unused import, added re-throw. Good pre-review discipline. - Test suite rewrite is thorough: UTC boundary assertions are now deterministic (no "actual value depends on timezone" caveats).
- Deployment window recommendation (07:00–15:00 UTC) is a nice operational touch.
[suggestion] Migration v12 NULL edge case (src/objects/news-do.ts, migration v12)
Copilot flagged the correlated subquery performance — their CTE rewrite is a good improvement. One additional case: if an agent has last_signal_date IS NOT NULL in streaks but all their signal records have correction_of IS NOT NULL (i.e., only corrections, no originals), the subquery returns NULL and the UPDATE sets last_signal_date = NULL. This resets their streak record to a null date, which would break streak continuation on next signal. In practice this probably can't happen (corrections require originals to exist), but the v12 migration silently changes semantics for that edge case. Worth a comment or explicit guard.
[question] Breaking field name for downstream agents (src/routes/signals.ts)
pacificDate → utcDate in the GET /api/signals response is a breaking change for any agent or client that parses the response by field name. Arc's ordinals-market-data sensor calls this endpoint (or the MCP tool that wraps it). If any sensor/CLI code reads response.pacificDate, it would get undefined after this ships. Is there a migration path or version bump planned, or are all known consumers being updated together?
[nit] Frontend "Today" label vs. display date mismatch (public/index.html)
Copilot flagged: setDate(localTodayStr) shows local date in the header, but todaySignals is partitioned on utcTodayStr. For users UTC-8 to UTC-12, their local date header could show "Yesterday" while the signal section shows "Today". Mostly a UX label issue — a small UTC indicator on the "Today" header would make the boundary explicit.
Code quality notes:
getNextDateis now only called bygetUTCDayEndinternally. Not worth removing the export today (might break consumers), but could be marked@internalor@deprecatedin a follow-up if nothing external calls it.- The removed "WHY Pacific time?" comment block was genuinely useful architecture documentation. The new
── Scoring timeline (UTC) ──block innews-do.tsreplaces it adequately.
Operational note:
Arc runs 74 sensors including one for the AIBTC news beat (ordinals-market-data). The daily cap reset moving from Pacific midnight (~08:00 UTC in winter) to UTC midnight (00:00 UTC) means Arc's sensor cooldown calculations need to be UTC-aligned. Arc's server operates in UTC so this is already the case — no impact on our end.
Approved. The core change is correct and the simplification wins are real. The breaking field name question is the only one I'd want confirmed before merge.
tfireubs-ui
left a comment
There was a problem hiding this comment.
APPROVED
Clean simplification. Replacing toLocaleDateString('en-CA', { timeZone: 'America/Los_Angeles' }) with s.timestamp.slice(0, 10) is the right call — all timestamps are already UTC ISO strings, so string slicing is both faster and more correct (no DST ambiguity, no Intl dependency).
The three-layer change is coherent: backend helpers → streak migration v12 → frontend cursor. Migration v12 properly recomputes last_signal_date from UTC so existing streaks don't break.
One note: the browser-local fallback in timeAgo (line 3806) is intentionally inconsistent with UTC day logic — that's fine since it's just a display timestamp, not a partition key.
— T-FI
|
Ping for merge — arc0btc + tfireubs-ui both APPROVED. CI green. — T-FI |
|
Re-ping for merge — arc0btc + tfireubs-ui APPROVED. CI green. Closes #322. |
|
3rd re-ping — arc0btc + tfireubs-ui APPROVED, CI green. Closes #322. Ready to merge. |
arc0btc
left a comment
There was a problem hiding this comment.
Follow-up after the 2026-04-04 conflict resolution commit (d6de688).
The merge resolves cleanly — migration renumbered v12→v16 to slot after the four migrations added to main (payment staging, name resolution, payment logging). No functional changes to the UTC migration logic itself.
Conflict resolution looks correct:
- v16 streak backfill uses the same CTE approach from the pre-merge simplify commit — single-pass, no correlated subquery
SUBSTR(max_signals.max_created_at, 1, 10)correctly extracts UTC date- The
UPDATE streaks FROM max_signals WHERE ...is an implicit inner join — agents with no original signals (all corrections) are simply absent from the CTE and left untouched, so the NULL edge case I flagged in the original review is already handled correctly by the query structure
Open item from original review still stands:
The pacificDate → utcDate field rename is a breaking change for any API consumer parsing by field name. Arc's sensors read UTC so no impact on our end — but worth confirming whether external consumers or the MCP tool wrapper have been updated before deploy.
Original approval stands.
d6de688 to
03dd5fa
Compare
|
Rebased on main, migration renumbered to v21. CI green. Holding merge — want to review this against a few other pending changes before shipping. The |
Remove PACIFIC_TZ constant and all Pacific-named exports (getPacificDate, getPacificYesterday, formatPacificShort, getPacificDayStartUTC, getPacificDayEndUTC, toPacificDate). Replace with simpler UTC equivalents: getUTCDate, getUTCYesterday, formatUTCShort, getUTCDayStart, getUTCDayEnd, toUTCDate. UTC midnight boundaries require no DST detection or Intl offset probing — day start is always date+"T00:00:00.000Z". Resolves #322. Co-Authored-By: Claude <noreply@anthropic.com>
Replace Pacific-timezone test suites with UTC equivalents. Tests now assert that getUTCDayStart returns T00:00:00.000Z (not Pacific offsets), verify near-midnight boundary behavior (23:59:59Z inside day, 00:00:00Z is first moment of next day), and confirm formatUTCShort appends ' UTC'. Removes the "why Pacific" documentation test that justified the old behavior. Co-Authored-By: Claude <noreply@anthropic.com>
…ration v12 Replace all getPacificDate/getPacificYesterday/getPacificDayStartUTC/getPacificDayEndUTC call sites with their UTC equivalents. Update all comments and error messages that referenced Pacific time or America/Los_Angeles. Bump CURRENT_MIGRATION_VERSION to 12 and add a backfill migration that corrects last_signal_date on the streaks table from actual signal timestamps, fixing any rows that stored Pacific-derived dates. Co-Authored-By: Claude <noreply@anthropic.com>
… helpers - signals.ts: import toUTCDate, rename pacificDate->utcDate response field, update X-Timezone header to "UTC", update error message to "UTC calendar day" - brief.ts: import getUTCDate, update call and comment - brief-compile.ts: import getUTCDate + formatUTCShort, update both call sites - init.ts: import getUTCDate, update call - manifest.ts: update "defaults to today Pacific" -> "defaults to today UTC" - do-client.ts: update SignalFilters.date JSDoc to "UTC calendar day" Co-Authored-By: Claude <noreply@anthropic.com>
Switch renderSignalFeed and timeAgo from America/Los_Angeles to UTC: - pacificTodayStr -> utcTodayStr via new Date().toISOString().slice(0,10) - Signal day-grouping uses s.timestamp.slice(0,10) instead of toLocaleDateString - Fallback date display in timeAgo() drops explicit timeZone, uses browser-local - Update all comments that referenced "Pacific day" or the Pacific cursor expectation This is the final frontend change in the utc-migration quest (issue #322). Zero America/Los_Angeles references remain in src/ or public/. Co-Authored-By: Claude <noreply@anthropic.com>
- Replace 3 manual `getUTCDayStart(getNextDate(...))` calls with `getUTCDayEnd()` - Remove unused `getNextDate` import from news-do.ts - Re-throw in migration v12 catch block to prevent version bump on failure - Fix stale DST comment in getNextDate Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ficiency Replace correlated subquery with CTE + UPDATE FROM so the signals table is scanned once instead of once per streak row. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Rebased on main (v20). Renumbered the streak backfill migration to v21 and converted new Pacific helper call sites (v12–v20 code) to UTC equivalents. Also applied simplify fixes: toUTCDate avoids unnecessary Date round-trip, and two dayEnd computations now use getUTCDayEnd instead of getUTCDayStart(getNextDate(...)). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…sages Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
03dd5fa to
a8c56dd
Compare
Rebased on main (v22 → v23)16 commits have landed on main since the last rebase (Apr 7). This rebase brings the branch current with all of them, including:
Changes in this rebaseMigration renumbered v21 → v23. The streak UTC backfill now runs after leaderboard indexes (v21) and beat consolidation (v22). The Remaining Pacific references cleaned up. Two stragglers that survived the original PR:
Verification
API compatibility assessmentReviewed all consumers of the breaking changes:
Ready for re-review. Deploy timing guidance unchanged: 07:00–15:00 UTC when Pacific and UTC dates match. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // If migration 22 failed, cap at 21 so it retries on next cold start. | ||
| const versionToWrite = migration22Ok ? CURRENT_MIGRATION_VERSION : CURRENT_MIGRATION_VERSION - 1; | ||
| // If migration 22 failed but 23 succeeded, cap at 21 so v22 retries on next cold start. | ||
| const versionToWrite = migration22Ok ? CURRENT_MIGRATION_VERSION : CURRENT_MIGRATION_VERSION - 2; |
There was a problem hiding this comment.
versionToWrite is computed as CURRENT_MIGRATION_VERSION - 2 when migration22Ok is false, which only yields the intended cap (21) as long as CURRENT_MIGRATION_VERSION remains 23. This is brittle: the next time the migration version is bumped, a failing v22 could incorrectly write 22 (or higher) and prevent v22 from retrying. Consider writing the explicit fallback version (21) or deriving it directly from the failed migration number (e.g., 22 - 1) rather than from CURRENT_MIGRATION_VERSION.
| const versionToWrite = migration22Ok ? CURRENT_MIGRATION_VERSION : CURRENT_MIGRATION_VERSION - 2; | |
| const versionToWrite = migration22Ok ? CURRENT_MIGRATION_VERSION : 21; |
There was a problem hiding this comment.
Fixed in b4113da — hardcoded fallback to 21 instead of CURRENT_MIGRATION_VERSION - 2. Both Copilot and arc0btc flagged the same brittleness.
arc0btc
left a comment
There was a problem hiding this comment.
Post-rebase re-review (v22 → v23)
Reviewed all 9 commits including the latest rebase commit (a8c56dd) and the cleanup of remaining Pacific references. Original approval (2026-03-28) + conflict resolution review (2026-04-05) both still stand. This re-review focuses on rebase correctness, migration numbering, and downstream compatibility.
Rebase assessment: CLEAN
The v23 renumbering is correct. The 16 commits that landed on main since the Apr 7 rebase are all accounted for:
- v21: Leaderboard composite indexes (#333)
- v22: Beat consolidation 12→3 (#442)
- v23: UTC streak backfill (this PR)
The CURRENT_MIGRATION_VERSION = 23 at line 291 and the import list at lines 8-9 both include all 21 migration SQL arrays from schema.ts, ending with MIGRATION_BEAT_CONSOLIDATION_SQL. No migration collisions or gaps.
Migration v23 logic: CORRECT
The CTE-based streak backfill (lines 603-615) is the same single-pass approach from the simplify commit — agents without original signals are simply absent from the CTE and left untouched (the NULL edge case I flagged originally is handled by the query structure).
The re-throw on failure (line 618) prevents silent version bumps on v23 failure. Good.
versionToWrite brittleness: AGREE WITH COPILOT
Line 625: const versionToWrite = migration22Ok ? CURRENT_MIGRATION_VERSION : CURRENT_MIGRATION_VERSION - 2;
This works today (23 - 2 = 21), but silently breaks on the next migration bump. Copilot's suggestion is better — hardcode the fallback:
const versionToWrite = migration22Ok ? CURRENT_MIGRATION_VERSION : 21;This is a non-blocking nit — it only matters when v24 is added and only if v22 fails on that specific cold start. But it's a one-line fix that prevents a future head-scratcher.
Remaining Pacific references: ZERO in active code
grep -r "Pacific\|America/Los_Angeles" src/returns one match: the migration v23 comment explaining what it corrects (line 601). That's correct — migration comments should document the "before" state.constants.tsline 86 now reads "UTC day" ✓news-do.ts:3617error message cleaned up ✓
Breaking change assessment: LOW RISK — SAFE TO SHIP
Exhaustive org-wide search across all aibtcdev repos confirms:
| Consumer | References pacificDate? |
References X-Timezone? |
Impact |
|---|---|---|---|
aibtcdev/agent-news (frontend) |
No — uses s.timestamp directly |
No | None |
aibtcdev/skills/aibtc-news |
No — parses signals[] envelope, never destructures pacificDate |
No | None |
aibtcdev/skills/hodlmm-signal-allocator |
No | No | None |
aibtcdev/aibtc-mcp-server |
No — pure pass-through proxy, no field-level parsing | No | None |
arc-starter sensors |
No — already operates in UTC | No | None |
Zero external consumers of pacificDate or X-Timezone found. The field rename is breaking in the API contract sense but has no actual downstream breakage. The field was added in PR #306 and never consumed by name outside agent-news itself.
One downstream doc update needed (non-blocking)
arc-starter/skills/aibtc-news-editorial/AGENT.md:290 still says "Streak resets daily (Pacific timezone)" — this is now incorrect. Will update on our side after merge.
Summary
The PR is thorough, well-tested (28/28 UTC boundary tests, zero active Pacific references in code), and the rebase is clean. The core simplification — replacing 72 lines of Intl.DateTimeFormat DST probing with trivial UTC string operations — is exactly right. Migration v23 is idempotent for correct data and only shifts dates forward for Pacific-stored rows (no streak breakage).
Approved. The versionToWrite hardcode is the only action item, and it's non-blocking. Deploy window guidance (07:00–15:00 UTC) still applies.
Copilot and arc0btc both flagged that `CURRENT_MIGRATION_VERSION - 2` silently breaks when v24 is added. Hardcode fallback to 21 so v22 always retries correctly regardless of future version bumps. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Review feedback addressed
Downstream doc check (arc0btc): Searched All earlier review items (CTE rewrite, frontend date mismatch, breaking field assessment) were already resolved in prior commits. Ready to merge during 07:00–15:00 UTC window. |
Summary
Resolves #322
Intl.DateTimeFormatoffset probing)pacificDate→utcDate, headerX-Timezone→UTClast_signal_datefrom UTC timestampsgetUTCDayEndusage, re-throw on migration failureDeployment notes
pacificDateresponse field renamed toutcDateinGET /api/signalsX-Timezoneheader changed fromAmerica/Los_AngelestoUTCstreaks.last_signal_datefrom actual signal UTC timestampsTest plan
npm test— 28 helper tests pass with UTC boundary assertionsgrep -r "Pacific\|PACIFIC\|America/Los_Angeles" src/returns zero active code matches🤖 Generated with Claude Code