fix: canonicalize reviewer-bot timestamps

Author: PLeVasseur

scripts/reviewer_bot_lib/deferred_gap_bookkeeping.py

@@ -6,6 +6,8 @@
 from dataclasses import dataclass
 from datetime import datetime, timedelta, timezone
 
+from .timestamps import normalize_iso8601_utc_string, parse_iso8601_utc
+
 
 @dataclass(frozen=True)
 class DeferredGap:
@@ -216,15 +218,7 @@ def _observer_now_iso(bot) -> str:
 
 
 def _parse_observer_timestamp(value: object) -> datetime | None:
-    if not isinstance(value, str) or not value.strip():
-        return None
-    try:
-        timestamp = datetime.fromisoformat(value.replace("Z", "+00:00"))
-    except ValueError:
-        return None
-    if timestamp.tzinfo is None:
-        return timestamp.replace(tzinfo=timezone.utc)
-    return timestamp
+    return parse_iso8601_utc(value)
 
 
 def _configured_seconds(bot, name: str, default: int) -> int:
@@ -247,6 +241,8 @@ def begin_observer_surface_scan(
     scan_started_at = now or bot.clock.now()
     if scan_started_at.tzinfo is None:
         scan_started_at = scan_started_at.replace(tzinfo=timezone.utc)
+    else:
+        scan_started_at = scan_started_at.astimezone(timezone.utc)
     lookback_seconds = _configured_seconds(bot, "DEFERRED_DISCOVERY_OVERLAP_SECONDS", 3600)
     bootstrap_window_seconds = _configured_seconds(bot, "DEFERRED_DISCOVERY_BOOTSTRAP_WINDOW_SECONDS", 604800)
     watermark.update(
@@ -266,11 +262,12 @@ def begin_observer_surface_scan(
 def record_observer_watermark_event(bot, review_data: dict, surface: str, event_time: str, event_id: str) -> None:
     current = _ensure_observer_discovery_watermark(review_data, surface)
     now = _observer_now_iso(bot)
+    safe_event_time = normalize_iso8601_utc_string(event_time) or event_time
     current.update(
         {
             "last_scan_started_at": current.get("last_scan_started_at") or now,
             "last_scan_completed_at": now,
-            "last_safe_event_time": event_time,
+            "last_safe_event_time": safe_event_time,
             "last_safe_event_id": event_id,
             "lookback_seconds": _configured_seconds(bot, "DEFERRED_DISCOVERY_OVERLAP_SECONDS", 3600),
             "bootstrap_window_seconds": _configured_seconds(bot, "DEFERRED_DISCOVERY_BOOTSTRAP_WINDOW_SECONDS", 604800),
@@ -304,7 +301,8 @@ def get_deferred_gap_reason(review_data: dict, source_event_key: str) -> str | N
 
 
 def _now_iso(bot) -> str:
-    return bot.clock.now().isoformat()
+    now = bot.clock.now()
+    return normalize_iso8601_utc_string(now) or now.isoformat()
 
 
 def _reconciled_at_now() -> str:
@@ -360,7 +358,7 @@ def mark_reconciled_source_event(
     reconciled_at: str | None = None,
 ) -> bool:
     reconciled = _reconciled_source_events(review_data)
-    timestamp = reconciled_at or _reconciled_at_now()
+    timestamp = normalize_iso8601_utc_string(reconciled_at) or reconciled_at or _reconciled_at_now()
     existing = reconciled.get(source_event_key)
     if isinstance(existing, dict):
         if not _is_valid_reconciled_source_event(existing, source_event_key):
@@ -424,14 +422,15 @@ def _copy_source_evidence(fields: dict, payload: dict, existing: dict) -> None:
 
 
 def _source_event_created_at(payload: dict, existing: dict):
-    return (
+    value = (
         payload.get("source_created_at")
         or payload.get("comment_created_at")
         or payload.get("source_submitted_at")
         or payload.get("source_dismissed_at")
         or payload.get("source_event_created_at")
         or existing.get("source_event_created_at")
     )
+    return normalize_iso8601_utc_string(value) or value
 
 
 def record_deferred_gap_diagnostic(
@@ -469,7 +468,7 @@ def record_deferred_gap_diagnostic(
     }
     source_dismissed_at = _payload_or_existing(payload, existing, "source_dismissed_at")
     if source_dismissed_at is not None:
-        fields["source_dismissed_at"] = source_dismissed_at
+        fields["source_dismissed_at"] = normalize_iso8601_utc_string(source_dismissed_at) or source_dismissed_at
     _copy_source_evidence(fields, payload, existing)
     existing.update(fields)
     changed = previous != existing

scripts/reviewer_bot_lib/event_inputs.py

@@ -4,7 +4,6 @@
 
 import json
 from dataclasses import dataclass
-from datetime import datetime, timezone
 from pathlib import Path
 
 from scripts.reviewer_bot_core.comment_routing_policy import PrCommentRouterOutcome
@@ -21,6 +20,7 @@
     PullRequestSyncRequest,
 )
 from .runtime_protocols import EventInputsContext
+from .timestamps import normalize_iso8601_utc_string
 
 
 class InvalidEventInput(RuntimeError):
@@ -137,16 +137,7 @@ def parse_issue_labels(bot: EventInputsContext) -> list[str]:
 
 
 def _normalize_iso8601_timestamp(value: str) -> str | None:
-    value = value.strip()
-    if not value:
-        return None
-    try:
-        timestamp = datetime.fromisoformat(value.replace("Z", "+00:00"))
-    except ValueError:
-        return None
-    if timestamp.tzinfo is None:
-        return timestamp.replace(tzinfo=timezone.utc).isoformat()
-    return value
+    return normalize_iso8601_utc_string(value)
 
 
 def _is_parseable_iso8601(value: str) -> bool:

scripts/reviewer_bot_lib/overdue.py

@@ -3,12 +3,13 @@
 from __future__ import annotations
 
 from dataclasses import dataclass
-from datetime import datetime, timezone
+from datetime import datetime
 
 from . import assignment_flow
 from .config import TRANSITION_NOTICE_MARKER_PREFIX, TRANSITION_WARNING_MARKER_PREFIX
 from .reminder_comments import ReminderCommentScan, scan_reviewer_reminder_comments
 from .repair_records import clear_repair_marker, store_repair_marker
+from .timestamps import parse_iso8601_utc
 
 _TRANSITION_NOTICE_AUTHORS = {"github-actions[bot]", "guidelines-bot"}
 
@@ -418,15 +419,7 @@ def build_reminder_delivery_persistence_result(
 
 
 def _parse_reminder_timestamp(value: object) -> datetime | None:
-    if isinstance(value, datetime):
-        return value if value.tzinfo is not None else value.replace(tzinfo=timezone.utc)
-    if not isinstance(value, str) or not value.strip():
-        return None
-    try:
-        timestamp = datetime.fromisoformat(value.replace("Z", "+00:00"))
-    except ValueError:
-        return None
-    return timestamp if timestamp.tzinfo is not None else timestamp.replace(tzinfo=timezone.utc)
+    return parse_iso8601_utc(value)
 
 
 def derive_reminder_cadence_decision(

scripts/reviewer_bot_lib/reconcile_payloads.py

@@ -3,9 +3,10 @@
 from __future__ import annotations
 
 from dataclasses import dataclass
-from datetime import datetime, timezone
 from enum import StrEnum
 
+from .timestamps import normalize_iso8601_utc_string
+
 
 class DeferredPayloadKind(StrEnum):
     DEFERRED_COMMENT = "deferred_comment"
@@ -520,13 +521,10 @@ def _first_string(payload: dict, field_names: tuple[str, ...], diagnostic_name:
 
 def _recoverable_timestamp(payload: dict, field_names: tuple[str, ...]) -> str:
     timestamp = _first_string(payload, field_names, "source event timestamp")
-    try:
-        parsed = datetime.fromisoformat(timestamp.replace("Z", "+00:00"))
-    except ValueError as exc:
-        raise RuntimeError("Deferred context payload source event timestamp is not parseable ISO-8601") from exc
-    if parsed.tzinfo is None:
-        return parsed.replace(tzinfo=timezone.utc).isoformat()
-    return timestamp
+    normalized = normalize_iso8601_utc_string(timestamp)
+    if normalized is None:
+        raise RuntimeError("Deferred context payload source event timestamp is not parseable ISO-8601")
+    return normalized
 
 
 def _optional_nonempty_string(payload: dict, field_name: str) -> str | None:
@@ -536,6 +534,19 @@ def _optional_nonempty_string(payload: dict, field_name: str) -> str | None:
     return None
 
 
+def _timestamp_string(value: object, diagnostic_name: str) -> str:
+    normalized = normalize_iso8601_utc_string(value)
+    if normalized is None:
+        raise RuntimeError(f"Deferred context payload {diagnostic_name} is not parseable ISO-8601")
+    return normalized
+
+
+def _optional_timestamp_string(value: object) -> str | None:
+    if value is None:
+        return None
+    return normalize_iso8601_utc_string(value) or str(value)
+
+
 def _diagnostic_payload(payload: dict, contract: DeferredIdentityContract, *, source_run_id: int, source_run_attempt: int, pr_number: int, source_event_key: str, source_object_id: int, actor_login: str, source_event_created_at: str) -> dict:
     diagnostic = {
         "source_run_id": source_run_id,
@@ -556,6 +567,8 @@ def _diagnostic_payload(payload: dict, contract: DeferredIdentityContract, *, so
         "source_dismissed_at",
     ):
         value = _optional_nonempty_string(payload, field_name)
+        if value is not None and field_name == "source_dismissed_at":
+            value = normalize_iso8601_utc_string(value) or value
         if value is not None:
             diagnostic[field_name] = value
     actor_id = payload.get("source_actor_id", payload.get("comment_author_id", payload.get("actor_id")))
@@ -831,7 +844,7 @@ def parse_deferred_context_payload(payload: dict) -> DeferredReviewPayload | Def
                 identity=identity,
                 comment_id=comment_id,
                 comment_body="",
-                comment_created_at=str(payload["source_created_at"]),
+                comment_created_at=_timestamp_string(payload["source_created_at"], "source_created_at"),
                 comment_author=str(payload["actor_login"]),
                 comment_author_id=comment_author_id,
                 comment_user_type=str(payload.get("actor_user_type") or "User"),
@@ -853,7 +866,7 @@ def parse_deferred_context_payload(payload: dict) -> DeferredReviewPayload | Def
             identity=identity,
             comment_id=comment_id,
             comment_body=str(payload["comment_body"]),
-            comment_created_at=str(payload["comment_created_at"]),
+            comment_created_at=_timestamp_string(payload["comment_created_at"], "comment_created_at"),
             comment_author=str(payload["comment_author"]),
             comment_author_id=int(payload["comment_author_id"]),
             comment_user_type=str(payload["comment_user_type"]),
@@ -873,7 +886,7 @@ def parse_deferred_context_payload(payload: dict) -> DeferredReviewPayload | Def
         common = dict(
             identity=identity,
             review_id=review_id,
-            source_submitted_at=(str(payload["source_submitted_at"]) if payload.get("source_submitted_at") is not None else None),
+            source_submitted_at=_optional_timestamp_string(payload.get("source_submitted_at")),
             source_review_state=(str(payload["source_review_state"]) if payload.get("source_review_state") is not None else None),
             source_commit_id=(str(payload["source_commit_id"]) if payload.get("source_commit_id") is not None else None),
             actor_login=(str(payload["actor_login"]) if payload.get("actor_login") is not None else None),
@@ -883,7 +896,7 @@ def parse_deferred_context_payload(payload: dict) -> DeferredReviewPayload | Def
             return DeferredReviewSubmittedPayload(**common)
         return DeferredReviewDismissedPayload(
             **common,
-            source_dismissed_at=(str(payload["source_dismissed_at"]) if payload.get("source_dismissed_at") is not None else None),
+            source_dismissed_at=_optional_timestamp_string(payload.get("source_dismissed_at")),
         )
     raise RuntimeError("Unsupported deferred workflow_run payload")
 

scripts/reviewer_bot_lib/reconcile_reads.py

@@ -3,7 +3,8 @@
 from __future__ import annotations
 
 from dataclasses import dataclass
-from datetime import datetime, timezone
+
+from .timestamps import normalize_iso8601_utc_string
 
 
 class ReconcileReadError(RuntimeError):
@@ -48,13 +49,7 @@ def _valid_exact_timestamp(value: object) -> str | None:
     timestamp = value.strip()
     if "T" not in timestamp:
         return None
-    try:
-        parsed = datetime.fromisoformat(timestamp.replace("Z", "+00:00"))
-    except ValueError:
-        return None
-    if parsed.tzinfo is None:
-        return parsed.replace(tzinfo=timezone.utc).isoformat()
-    return timestamp
+    return normalize_iso8601_utc_string(timestamp)
 
 
 def _dismissed_review_source_time(payload: dict | None) -> DismissalTimeResolution | None:

scripts/reviewer_bot_lib/timestamps.py

@@ -0,0 +1,26 @@
+"""Reviewer-bot timestamp normalization helpers."""
+
+from __future__ import annotations
+
+from datetime import datetime, timezone
+from typing import Any
+
+
+def parse_iso8601_utc(value: Any) -> datetime | None:
+    if isinstance(value, datetime):
+        timestamp = value
+    elif isinstance(value, str) and value.strip():
+        try:
+            timestamp = datetime.fromisoformat(value.strip().replace("Z", "+00:00"))
+        except ValueError:
+            return None
+    else:
+        return None
+    if timestamp.tzinfo is None:
+        return timestamp.replace(tzinfo=timezone.utc)
+    return timestamp.astimezone(timezone.utc)
+
+
+def normalize_iso8601_utc_string(value: Any) -> str | None:
+    timestamp = parse_iso8601_utc(value)
+    return timestamp.isoformat() if timestamp is not None else None

tests/integration/reviewer_bot/test_reconcile_workflow_run.py

@@ -294,7 +294,7 @@ def test_deferred_review_dismissal_replay_uses_source_dismissal_time(monkeypatch
     harness.stub_head_repair(changed=False)
 
     assert harness.run(state) is True
-    assert review["review_dismissal"]["accepted"]["timestamp"] == "2026-03-17T10:10:00Z"
+    assert review["review_dismissal"]["accepted"]["timestamp"] == "2026-03-17T10:10:00+00:00"
     assert "pull_request_review_dismissed:12" in _reconciled_source_events(review)
     assert "pull_request_review_dismissed:12" not in _deferred_gaps(review)
     assert "issues/42/timeline?per_page=100&page=1" not in harness.github.requested_endpoints()
@@ -333,7 +333,7 @@ def test_deferred_review_dismissal_replay_uses_exact_timeline_dismissed_at(monke
     harness.stub_head_repair(changed=False)
 
     assert harness.run(state) is True
-    assert review["review_dismissal"]["accepted"]["timestamp"] == "2026-03-17T10:12:00Z"
+    assert review["review_dismissal"]["accepted"]["timestamp"] == "2026-03-17T10:12:00+00:00"
     assert "pull_request_review_dismissed:12" in _reconciled_source_events(review)
 
 
@@ -385,7 +385,7 @@ def test_deferred_review_dismissal_replay_finds_exact_time_on_paginated_timeline
 
     assert harness.run(state) is True
 
-    assert review["review_dismissal"]["accepted"]["timestamp"] == "2026-03-17T10:12:00Z"
+    assert review["review_dismissal"]["accepted"]["timestamp"] == "2026-03-17T10:12:00+00:00"
     assert "issues/42/timeline?per_page=100&page=1" in harness.github.requested_endpoints()
     assert "issues/42/timeline?per_page=100&page=2" in harness.github.requested_endpoints()
 
@@ -623,7 +623,7 @@ def test_deferred_comment_missing_live_object_preserves_source_time_freshness(mo
     assert state["active_reviews"]["42"]["reviewer_comment"]["accepted"] is None
     gap = state["active_reviews"]["42"]["sidecars"]["deferred_gaps"]["issue_comment:99"]
     assert gap["reason"] == "reconcile_failed_closed"
-    assert gap["source_event_created_at"] == "2026-03-17T10:00:00Z"
+    assert gap["source_event_created_at"] == "2026-03-17T10:00:00+00:00"
     assert gap["source_actor_login"] == "alice"
     assert gap["source_actor_id"] == 7001
     assert gap["source_actor_user_type"] == "User"
@@ -865,7 +865,7 @@ def test_deferred_review_comment_missing_live_object_preserves_source_time_fresh
     assert review["reviewer_comment"]["accepted"] is None
     gap = _deferred_gaps(review)["pull_request_review_comment:303"]
     assert gap["reason"] == "reconcile_failed_closed"
-    assert gap["source_event_created_at"] == "2026-03-17T10:00:00Z"
+    assert gap["source_event_created_at"] == "2026-03-17T10:00:00+00:00"
     assert gap["source_actor_login"] == "alice"
     assert gap["source_actor_id"] == 6
     assert gap["source_actor_user_type"] == "User"
@@ -1210,7 +1210,7 @@ def test_deferred_issue_comment_parse_failure_records_artifact_invalid_gap(monke
     gap = _deferred_gaps(review)["issue_comment:212"]
     assert gap["reason"] == "artifact_invalid"
     assert gap["source_comment_id"] == 212
-    assert gap["source_event_created_at"] == "2026-03-17T10:00:00Z"
+    assert gap["source_event_created_at"] == "2026-03-17T10:00:00+00:00"
 
 
 def test_deferred_issue_comment_parse_failure_rejects_mismatched_source_object_key(monkeypatch):
@@ -1259,7 +1259,7 @@ def test_deferred_review_submitted_parse_failure_records_artifact_invalid_gap(mo
     gap = _deferred_gaps(review)["pull_request_review:13"]
     assert gap["reason"] == "artifact_invalid"
     assert gap["source_review_id"] == 13
-    assert gap["source_event_created_at"] == "2026-03-17T10:00:00Z"
+    assert gap["source_event_created_at"] == "2026-03-17T10:00:00+00:00"
 
 
 def test_deferred_review_dismissed_parse_failure_records_artifact_invalid_gap(monkeypatch):
@@ -1283,7 +1283,7 @@ def test_deferred_review_dismissed_parse_failure_records_artifact_invalid_gap(mo
     gap = _deferred_gaps(review)["pull_request_review_dismissed:14"]
     assert gap["reason"] == "artifact_invalid"
     assert gap["source_review_id"] == 14
-    assert gap["source_event_created_at"] == "2026-03-17T10:10:00Z"
+    assert gap["source_event_created_at"] == "2026-03-17T10:10:00+00:00"
 
 
 def test_deferred_legacy_review_comment_hydrates_source_commit_id_from_live_comment(monkeypatch):