History query performance: bounded /stats, benchmark-justified indexes, Postgres-tier tests (#141) #176

Merged

claude-bot merged 3 commits from feat/history-performance into main

2026-08-01 07:24:47 +00:00

claude-bot commented

2026-08-01 07:18:16 +00:00

Contributor

Closes #141. Closes #174. The milestone's final engineering item — every decision here is measured, not guessed.

/stats bounding

The unbounded whole-history scan the issue called out is gone: ?range= selector (30d/90d/365d/all-retained, default 90d), plain-GET form per convention. "All-retained" is still bounded at the unified horizon — retention has already deleted anything older, so it's the widest window that can return rows, and every variant stays an indexed range scan. Python aggregation kept (the portable-date-bucketing rationale holds for the bounded result sizes; comment updated with measured row counts).

Indexes — migration 0036, five created, one measured-and-rejected

Benchmarked via the new scripts/benchmark_history.py: a seeded ~13-month fixture (60k sent_alerts / 120k snapshots / 600k lightning / 5k events + 20k members) on ephemeral Postgres 16, EXPLAIN (ANALYZE, BUFFERS) over every history query shape, before/after on identical data. Highlights (p95):

index	serves	before → after
`ix_sent_alerts_dashboard_sent_at` (partial, `channel_id IS NULL`)	/alerts, bounded /stats, per-location history, rebuild	/alerts 51.7 → 0.38 ms; /stats 90d 84.3 → 5.8 ms
`ix_sent_alerts_location_sent_at`	ai_context recent/active reads	10.0 → 0.10 ms
`ix_nws_alert_snapshots_received_at`	snapshot retention sweep (had no index)	1878 → 496 ms
`ix_notification_deliveries_updated_at`	deliveries retention sweep	88 → 23 ms
`ix_weather_events_started_at`	calendar/day cross-location range	1.27 → 0.81 ms (Seq→Index)

Rejected with numbers: ai_summary_records(created_at) — plan flips but time doesn't move (28.3→28.7 ms); that delete's cost is the cascade into ai_summary_attempts, so the index would be pure write overhead. Kept in the script's REJECTED_INDEXES for re-verification. Equal value in what the audit didn't add: five suspect query shapes were already served by existing unique/composite indexes and were left alone.

Partitioning verdict: NOT justified — recorded with the numbers

Decision rule from the pickup comment: reads <100 ms p95 and retention deletes <5 s ⇒ plain indexes win. Measured: slowest read 12.4 ms, slowest delete 0.93 s — an order of magnitude inside, stable across three runs. The migration docstring records the verdict, the honesty caveats about seq-scan timing noise, and the scheme to reach for first if scale ever changes (monthly RANGE partitioning of lightning_clusters — 10× everything else's rows — and explicitly not sent_alerts, whose dedup constraints wouldn't survive).

Postgres-tier tests

7 new @pytest.mark.postgres tests: month aggregation vs date_trunc ground truth, local-vs-UTC day bucketing, JSONB operators, real-timestamptz cutoff, and — after a real alembic upgrade head from empty — that 0036's indexes exist with the right partial predicate and the rejected index stayed out, plus model↔migration index parity.

The #174 "flake" — root cause was the wall clock, not test ordering

The prompt metadata embeds current_utc_time with microseconds; when the clock reads second 38 with microseconds in [600000,700000), the timestamp renders …:38.6xxxxx and the test's assert str(loc.lat) not in prompt (lat 38.6) matched the timestamp — P≈1/600 per run. Proven deterministically by pinning the colliding instant. Fixed at the root: an autouse frozen-clock fixture for the prompt-shape tests, a real contract assertion on the filtered payload, and a regression test that pins the colliding instant.

Verification

Dev host, final code: bulk suite 979 passed; Postgres tier 13 passed against ephemeral PG 16 (including the from-empty migration test). ruff clean. Migration 0036 is index-only — no data changes — so per [migration-real-db-verify] the empty-DB CI run is representative; index builds on prod's row counts are sub-second.

🤖 Generated with Claude Code

Closes #141. Closes #174. The milestone's final engineering item — every decision here is measured, not guessed. ## /stats bounding The unbounded whole-history scan the issue called out is gone: `?range=` selector (30d/90d/365d/all-retained, default 90d), plain-GET form per convention. "All-retained" is still bounded at the unified horizon — retention has already deleted anything older, so it's the widest window that can return rows, and every variant stays an indexed range scan. Python aggregation kept (the portable-date-bucketing rationale holds for the bounded result sizes; comment updated with measured row counts). ## Indexes — migration 0036, five created, one measured-and-rejected Benchmarked via the new `scripts/benchmark_history.py`: a seeded ~13-month fixture (60k sent_alerts / 120k snapshots / 600k lightning / 5k events + 20k members) on ephemeral Postgres 16, `EXPLAIN (ANALYZE, BUFFERS)` over every history query shape, before/after on identical data. Highlights (p95): | index | serves | before → after | |---|---|---| | `ix_sent_alerts_dashboard_sent_at` (partial, `channel_id IS NULL`) | /alerts, bounded /stats, per-location history, rebuild | /alerts **51.7 → 0.38 ms**; /stats 90d **84.3 → 5.8 ms** | | `ix_sent_alerts_location_sent_at` | ai_context recent/active reads | **10.0 → 0.10 ms** | | `ix_nws_alert_snapshots_received_at` | snapshot retention sweep (had no index) | **1878 → 496 ms** | | `ix_notification_deliveries_updated_at` | deliveries retention sweep | **88 → 23 ms** | | `ix_weather_events_started_at` | calendar/day cross-location range | 1.27 → 0.81 ms (Seq→Index) | **Rejected with numbers**: `ai_summary_records(created_at)` — plan flips but time doesn't move (28.3→28.7 ms); that delete's cost is the cascade into `ai_summary_attempts`, so the index would be pure write overhead. Kept in the script's `REJECTED_INDEXES` for re-verification. Equal value in what the audit *didn't* add: five suspect query shapes were already served by existing unique/composite indexes and were left alone. ## Partitioning verdict: NOT justified — recorded with the numbers Decision rule from the pickup comment: reads <100 ms p95 and retention deletes <5 s ⇒ plain indexes win. Measured: slowest read **12.4 ms**, slowest delete **0.93 s** — an order of magnitude inside, stable across three runs. The migration docstring records the verdict, the honesty caveats about seq-scan timing noise, and the scheme to reach for first if scale ever changes (monthly RANGE partitioning of `lightning_clusters` — 10× everything else's rows — and explicitly *not* `sent_alerts`, whose dedup constraints wouldn't survive). ## Postgres-tier tests 7 new `@pytest.mark.postgres` tests: month aggregation vs `date_trunc` ground truth, local-vs-UTC day bucketing, JSONB operators, real-`timestamptz` cutoff, and — after a real `alembic upgrade head` from empty — that 0036's indexes exist with the right partial predicate and the rejected index stayed out, plus model↔migration index parity. ## The #174 "flake" — root cause was the wall clock, not test ordering The prompt metadata embeds `current_utc_time` with microseconds; when the clock reads second 38 with microseconds in [600000,700000), the timestamp renders `…:38.6xxxxx` and the test's `assert str(loc.lat) not in prompt` (lat 38.6) matched the *timestamp* — P≈1/600 per run. Proven deterministically by pinning the colliding instant. Fixed at the root: an autouse frozen-clock fixture for the prompt-shape tests, a real contract assertion on the filtered payload, and a regression test that pins the colliding instant. ## Verification Dev host, final code: bulk suite **979 passed**; Postgres tier **13 passed** against ephemeral PG 16 (including the from-empty migration test). `ruff` clean. Migration 0036 is index-only — no data changes — so per [migration-real-db-verify] the empty-DB CI run is representative; index builds on prod's row counts are sub-second. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

claude-bot added 3 commits

2026-08-01 07:18:17 +00:00

Bound the /stats query to a selectable range (#141 ) c050b60800

/stats scanned every retained sent_alerts row for every visible location on
every load. That was tolerable when the table held a few thousand rows; with
the unified 13-month horizon (#133) accumulating history for the Explorer it is
the unbounded-history anti-pattern the Explorer's own calendar deliberately
avoided repeating. On the #141 benchmark fixture (60k sent_alerts, 13 months)
the unbounded query measured 101.52 ms p95 and rose linearly with the table.

A plain GET ?range= selector (30d / 90d / 365d / all-retained, default 90d)
bounds it, following the same filter-bar convention as /public-links. Combined
with 0036's partial index the default range is 5.75 ms p95 and all-retained
12.42 ms.

"all-retained" is bounded too, at HISTORY_HORIZON_DAYS: retention.py has
already deleted everything older, so the horizon is the widest window that can
return rows, and using it keeps every variant of the page an indexed range scan
instead of a full scan. The cutoff compares against sent_at while retention
prunes on coalesce(cleared_at, expires, sent_at), so a long-running alert sent
before the horizon can outlive it -- it would bucket into a month outside the
horizon anyway, so excluding it is consistent rather than lossy.

Unrecognised ?range= values fall back to the default rather than erroring; this
is a filter on a read-only page. Aggregation stays in Python: the
portable-date-bucketing rationale (no cross-dialect "group by calendar month"
between Postgres date_trunc and SQLite strftime) is unchanged, and bounding the
scan -- not rewriting the group-by -- was the actual problem. The page stays
separate from the Explorer, now with a link across to it.

The page's tests moved from fixed calendar dates to dates relative to now:
against a rolling window, fixed dates silently fall out of range as time passes.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Add the history indexes the benchmark justified, and the benchmark (#141 ) 6df7c1fda9

The v2.0.0 Explorer reads history from six directions -- calendar, day view,
event detail, /stats, the correlation rebuild, retention -- and nobody had
measured any of them. This adds the harness first, then only the indexes it
justified.

scripts/benchmark_history.py seeds a ~13-month fixture (6 locations, 60k
sent_alerts, 120k nws_alert_snapshots, 600k lightning_clusters over its own
90-day window, 5k weather_events / 20k members, 40k notification_deliveries)
into a throwaway PostgreSQL and runs EXPLAIN (ANALYZE, BUFFERS) over 22 query
shapes -- each one the hand-written equivalent of a specific ORM query, named
with the call site it mirrors -- once without the candidate indexes and once
with them, against identical data in a single run. Deletes run inside
rolled-back transactions and every phase is preceded by VACUUM ANALYZE, so the
second phase is not measured against a heap the first phase bloated.

The audit found several suspects already covered and left them alone: event
detail's snapshot lookup rides the (location_id, lifecycle_id, content_hash)
unique constraint, its delivery/AI/radar lookups ride existing FK and unique
indexes, and lightning_clusters' retention delete already had
(occurred_at, lat, lon). Migration 0036 adds five indexes, each carrying the
query it serves and the number it moved (p95):

- weather_events(started_at): the calendar and day views range on started_at
  alone; ix_weather_events_location_started leads with location_id and cannot
  serve that. 1.27 -> 0.81 ms. The smallest win here, kept because
  weather_events is the one history table retention.py does not prune.
- sent_alerts(sent_at) WHERE channel_id IS NULL: /stats, /alerts, per-location
  history and the correlation rebuild all carry that predicate.
  /alerts 51.72 -> 0.38 ms, /stats 90d 84.27 -> 5.75 ms.
- sent_alerts(location_id, sent_at): the ai_context reads have no channel_id
  predicate, so the partial index cannot serve them. 10.01 -> 0.10 ms.
- nws_alert_snapshots(received_at): retention's delete had no index at all on
  the second-largest history table. 1877.74 -> 496.01 ms.
- notification_deliveries(updated_at): same, 88.15 -> 22.70 ms.

ai_summary_records(created_at) was measured and REJECTED: the plan flips to an
Index Scan but the time does not move (28.33 -> 28.69 ms), because that
delete's cost is the per-row cascade into ai_summary_attempts, not finding the
rows. It stays in the script's REJECTED_INDEXES so the finding can be
re-verified rather than rediscovered as an obvious-looking missing index.

PARTITIONING: not justified. Against #141's rule -- partition only if some read
misses 100 ms p95 or a retention delete misses 5 s at this scale with plain
indexes -- the slowest read is 12.42 ms and the slowest delete 0.93 s
(186,670 lightning rows), both an order of magnitude inside the thresholds.
Partitioning would cost partition maintenance and, for sent_alerts, the dedup
unique constraints in their current form. The migration records what the
recommendation would be if that ever changes.

tests/test_history_postgres.py covers what the SQLite tier structurally cannot:
the calendar's month aggregation against date_trunc ground truth (plus the
local-day bucketing that deliberately disagrees with it), real JSONB ->> and @>
on a snapshots query, and that 0036's indexes -- including the partial
predicate -- exist in pg_indexes after a real alembic upgrade head from an
empty database, not merely in the ORM metadata.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Fix the wall-clock flake in the AI summarizer prompt tests (#174 )

CI / test (pull_request) Successful in 6m8s

Details

c3c3a90a6e

test_cloud_provider_prompt_filters_disallowed_metadata failed once in a full
suite and never reproduced, isolated or on rerun. It was not a polluter and not
an ordering problem -- collection order is file order and the suite has no
random-order plugin, so an intermittent failure could not come from ordering
unless something accumulated nondeterministically. It came from the clock.

_build_prompts embeds current_utc_time -- a full-microsecond
datetime.now(timezone.utc).isoformat() -- in every prompt's metadata block. The
test fixture's latitude is 38.6, and the test asserted the coordinate had been
filtered out with `assert str(loc.lat) not in user_prompt`. Whenever the real
clock read second 38 with a microsecond in [600000, 700000) the rendered
timestamp contained the literal "38.6" and the assertion matched the TIMESTAMP
rather than a leaked coordinate. That is P(second == 38) * P(microsecond decile
== 6) = 1/600 per run. Pinning the clock to 06:16:38.612345 reproduces the
failure every time; 06:16:12.123456 passes every time.

Two root causes, both fixed:

- A prompt-SHAPE unit test had no business reading the wall clock. A module
  autouse fixture now freezes ai_summarizer.utc_now, via monkeypatch so it is
  undone per test and cannot leak into other modules -- the same pinning the
  neighbouring local-time-context test already did by hand.
- The assertion was a bare substring search for a four-character numeric
  literal across a ~4 KB prompt. The test now asserts the actual contract
  first -- the filtered payload carries no coordinates, no NWS metadata and no
  local context -- so a real regression is reported as "coordinates survived
  filtering" instead of an unexplained substring match.

test_cloud_filter_holds_when_the_clock_collides_with_the_latitude pins the
colliding instant and asserts the filtering still holds, so re-introducing a
wall-clock-sensitive assertion fails immediately rather than once a year in CI.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

claude-bot merged commit 984bb752b4 into main

2026-08-01 07:24:47 +00:00

claude-bot referenced this pull request from a commit

2026-08-01 07:24:48 +00:00

Merge pull request 'History query performance: bounded /stats, benchmark-justified indexes, Postgres-tier tests (#141)' (#176) from feat/history-performance into main

claude-bot deleted branch feat/history-performance

2026-08-01 07:24:48 +00:00

claude-bot referenced this pull request

2026-08-01 07:25:10 +00:00

Historical Weather Event Explorer #22