perf: batched record_metrics gives every metric an identical timestamp, breaking its documented parity with record_metric

[groupthinking]

Summary

PerformanceMonitor.record_metrics (src/youtube_extension/backend/services/performance_monitor.py) documents itself as a drop-in for the serial path:

The observable behaviour is identical to calling record_metric once per entry: the buffer, the fast-access collections and the alert thresholds are all updated the same way, in the same order.

It is not identical. The batch hoists a single clock read out of the comprehension:

now = datetime.now(timezone.utc)
records = [
    PerformanceMetric(
        ...
        timestamp=entry.get("timestamp") or now,
    )
    for entry in metrics
]

so every record in a batch carries the same timestamp, whereas record_metric stamps each metric at the moment it is recorded (timestamp=datetime.now(timezone.utc), one call per metric).

Why it matters

The impact is bounded — timestamps are consumed by range queries only (retention pruning WHERE timestamp < ? and windowed reads WHERE timestamp >= ?), so a sub-millisecond collapse changes no current query result. This is a correctness-of-contract defect, not a live incident.

What makes it worth closing rather than living with:

1. The docstring is load-bearing. It is the justification for callers migrating from record_metric to record_metrics, and it is currently false for the one field a batched implementation is tempted to diverge on.
2. Per-sample ordering is erased for any caller that submits genuinely distinct samples in one call. Nothing prevents that today — the signature accepts an arbitrary sequence.
3. It is untested in either direction, so the divergence is free to persist or widen. It was raised on perf: batch performance-metric writes into one SQLite round-trip #1341 (Copilot review) and again on Claude/determined maxwell rswptp #1356, and has survived both.

Acceptance criteria

• record_metrics consults the clock once per record, matching record_metric.
• An explicitly supplied entry["timestamp"] is still honoured; the clock is read only for entries that omit it.
• Tests pin per-record stamping deterministically — not by asserting timestamps merely differ, since wall-clock resolution lets several now() calls in a tight loop legitimately return the same value.
• The new tests fail against the shared-now implementation and pass against the fix.
• No regression in tests/unit/test_performance_monitor.py or tests/unit/test_v1_router_extended.py.

Test paths

• tests/unit/test_performance_monitor.py::TestRecordMetricsBatch
• tests/unit/test_v1_router_extended.py

Related

• perf: batch performance-metric writes into one SQLite round-trip #1341 — introduced record_metrics; the shared-now was flagged in review there and not changed.
• Claude/determined maxwell rswptp #1356 — carries this same fix, but is conflicted against main, blocked on a red truth gate, and its other three files are already on main via perf: batch performance-metric writes into one SQLite round-trip #1341. Superseded by whatever closes this issue.

[linear-code]

GRV-331

[github-actions]

👋 Thanks for opening this issue! It has been automatically labeled for triage.