Skip to content

fix(desktop): prevent double-localized SQL timestamps - #12351

Open
tianmind-studio wants to merge 1 commit into
BasedHardware:mainfrom
tianmind-studio:codex/sql-timestamp-double-localtime
Open

fix(desktop): prevent double-localized SQL timestamps#12351
tianmind-studio wants to merge 1 commit into
BasedHardware:mainfrom
tianmind-studio:codex/sql-timestamp-double-localtime

Conversation

@tianmind-studio

@tianmind-studio tianmind-studio commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

What changed and why

Fixes #12350. After #12349, Desktop Chat could apply the local offset twice when a SQL projection used SQLite localtime, while the prompt and daily recap still built local-day bounds in UTC-first order. This focused follow-up to the still-open macOS work in #12326 keeps projected timestamps raw until the result formatter, rejects query-layer localtime in SELECT expressions, and converts local-midnight boundaries back to UTC before comparing stored UTC values.

Product invariants affected

  • INV-CHAT-1

How it was verified

  • Reproduced the failure with SQLite on Windows in Asia/Shanghai: the old boundary resolves to 08:00 local, while localtime -> start of day -> utc resolves to 16:00 UTC (local midnight).
  • git diff --check
  • Validated the changelog JSON with jq 1.8.2.
  • Ran local prompt and projection-guard contract assertions, including acceptance of localtime in UTC boundary predicates and rejection in projected timestamp expressions.
  • Full macOS Swift tests cannot run on this Windows host; Desktop Swift Static & Test Contracts is the authoritative compile/test gate.

Tests

  • ChatPromptsTests.testDesktopChatSQLGuidanceLocalizesTimestampResultsExactlyOnce
  • ChatPromptsTests.testDesktopChatSQLGuidanceComparesUTCColumnsToUTCBounds
  • ChatPromptsTests.testDesktopChatSQLDayBoundsKeepLocalCalendarWindowsInUTC
  • ChatToolExecutorSQLTests.testExecuteSQLRendersDatetimeColumnsInLocalTimeWithZoneLabel
  • ChatToolExecutorSQLTests.testExecuteSQLRejectsProjectedLocaltimeButAllowsUTCBoundaryConversion

Failure class (fixes)

Failure-Class: FC-naive-utc-timestamp-in-llm-surface

Review in cubic

Failure-Class: FC-naive-utc-timestamp-in-llm-surface
@tianmind-studio
tianmind-studio force-pushed the codex/sql-timestamp-double-localtime branch from b62c275 to edbff15 Compare August 28, 2026 12:10

@Git-on-my-level Git-on-my-level left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved — clean, focused fix for the double-localization regression from #12349 (#12350), and it matches the merged design: the result formatter owns localization, so the query layer stays UTC.

Per-file notes:

  • desktop/macos/Desktop/Sources/Chat/ChatPrompts.swift — examples now select raw timestamp/*At columns (MIN(timestamp) AS firstSeenAt, MAX(timestamp) AS lastSeenAt) and express local-day bounds as UTC instants (datetime('now', 'localtime', 'start of day', '-1 day', 'utc')). I verified the modifier order empirically against SQLite in Asia/Shanghai: the old form resolves yesterday's lower bound to 08:00 UTC while the new form gives 16:00 UTC — true local midnight. The old "convert them to {tz}" display guidance is correctly replaced by "quote execute_sql output as-is".
  • desktop/macos/Desktop/Sources/Chat/SQLQueryResultProjection.swift — the new projectsSQLiteLocalTime guard returns an actionable error instead of letting a datetime(col, 'localtime') projection reach the formatter that would shift it a second time; localtime inside UTC WHERE-boundary computation is still allowed (covered by the new test). Minor non-blocking corner case: a projection like SELECT datetime('now','localtime','start of day','utc') AS today_start also trips the guard even though that value is already UTC — it degrades to a clear error message, fine to leave as-is for a conservative guard.
  • desktop/macos/Desktop/Sources/Providers/ChatToolExecutor.swifttimeZone is threaded explicitly through executeSQLexecuteSelectQuery → the projection (default .current keeps existing call sites unchanged), and DesktopChatSQLTime.localDayStartAsUTC/exclusiveEndAsUTC replace the six hand-rolled localtime bounds in the daily recap queries. The recap's first_seen/last_seen stay SQL-localized and now carry an explicit zone label, so both paths localize exactly once.
  • desktop/macos/Desktop/Tests/ChatPromptsTests.swift — pins the new guidance (raw projections, UTC-bound comparisons) and the DesktopChatSQLTime boundary strings, including the daysAgo=0 and negative-days clamping cases.
  • desktop/macos/Desktop/Tests/ChatToolExecutorSQLTests.swift — the pinned America/New_York timezone makes rendering deterministic on any CI machine, and the new test exercises both the allowed boundary form and the rejected projection form through the real executeSQL path. Swapping the magic Date(timeIntervalSince1970:) for ISO8601DateFormatter parsing is a readability win.
  • desktop/macos/changelog/unreleased/20260828-chat-sql-timezone-boundaries.json — accurate one-liner matching the unreleased-changelog convention.

Coordination note for maintainers: #12326 (SCA-364) is also open and touches the same five Swift files with the same UTC-boundary idiom, though it does not include this projection guard. Sequencing the two is a maintainer call; this PR is the focused fix for the live regression and is mergeable as-is against current main.

Thanks for the fast, well-reproduced report in #12350 and the tight follow-up.


by AI on behalf of David — if you need David’s attention urgently, please @Git-on-my-level and escalate with need human response.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Desktop SQL timezone guidance can double-shift results and local-day bounds

2 participants