perf: add a benchmark harness for ingest + explain (#85)

[leo-aa88]

Summary

#85's first work item — "establish a benchmark ... commit it so regressions are visible". There was no benchmark, so "what's the largest window raglogs can explain?" had no answer and perf regressions were invisible (including whether the pool-sizing and bulk-insert fixes from #98 actually help).

Adds a harness that ingests N synthetic, incident-shaped log lines and explains the window, recording wall time and query counts per phase.

What's here

• src/perf/bench.py — pure, unit-tested pieces: deterministic synthetic-load generation (a deploy trigger + an error spike + benign traffic), a QueryCounter (a before_cursor_execute listener), and BenchResult / report formatting with a pass/fail against a target.
• scripts/benchmark.py + make bench — drive it against a live database (--lines, --json).
• .github/workflows/bench.yml — runs on a schedule and on main (not per-PR, since it ingests + runs the pipeline), uploading bench_results.json so the numbers are tracked over time.
• README — states the working target (explain a window in < 10s) and points at the benchmark output as the real source of truth, with a note on the remaining No performance baseline: pool sizing, per-row ClusterMember inserts, no partitioning, no load test #85 items (time-partitioning, full load test).

Design

The pure logic is unit-tested without Postgres — the QueryCounter is verified against in-memory SQLite, and the generator/report are pure. The real ingest+explain numbers come from make bench or the scheduled workflow (I can't run Postgres in this environment), mirroring how the eval harness (#77) was delivered.

Testing

• make lint → All checks passed
• make test-unit → 959 passed (9 new)
• bench.yml validates as YAML

Refs #85

🤖 Generated with Claude Code

[leo-aa88]

APPROVE

LGTM. Clean, well-tested benchmark harness that gives #85 its missing baseline, delivered in the same honest shape as the eval harness (#77): pure logic unit-tested, real numbers from the scheduled/make bench run. Reviewed at head 61f535a.

CONFIRMED — the pure pieces are correct and tested. generate_jsonl_lines is deterministic (fixed base, deploy trigger at i=0, ~1/3 error lines from one service, benign otherwise) with ordered timestamps; QueryCounter attaches/detaches a before_cursor_execute listener per with block, and test_counts_within_block_only proves it counts only inside the block (so the listener is actually removed — no leak). BenchResult/format_report/meets_target behave as documented. 9 unit tests, lint clean.

CONFIRMED — runner contracts match and no core is touched. run_benchmark times ingest and explain separately, builds the window from the first/last generated timestamps, and calls explain_window(..., no_llm=True, ingestion_job_id=, scope=) / ingest_files(..., scope=) — all valid signatures — reading result.total_logs and stats.parsed_count, both real fields. git diff shows no src/core/ changes; bench_results.json is gitignored; bench.yml migrates → runs 20k → uploads the artifact if: always(), and is deliberately not a PR gate.

One MINOR/consistency note inline: the DB-backed run_benchmark path (like the pre-#97 eval runner) only executes in the nightly/main job — a tiny integration smoke would exercise it on every CI run; plus a note that the wall-clock pass/fail on main is runner-variance-sensitive.

VERDICT

Correct, boundary-respecting, well-tested where it can be, and honestly scoped (harness + target now; time-partitioning and the full load test left as #85 follow-ups). No BLOCKING/MAJOR findings. Merge it; the integration smoke is a nice-to-have fast follow. (Comment rather than Approve — GitHub blocks self-approval on your own PR.)

[leo-aa88]

MINOR (consistency, non-blocking) — the DB-backed run_benchmark path has no in-CI execution except the scheduled/main bench job. Same shape as the eval runner before #97's canary: the pure pieces (generator, QueryCounter, report) are unit-tested, but run_benchmark (ingest → explain_window → BenchResult) first runs against real Postgres only on the schedule/main push. I verified the contracts (explain_window kwargs, ExplainResult.total_logs, IngestionStats.parsed_count), so runtime risk is low.

Consider the same fast follow you added for the eval harness: a tiny tests/integration/ test that runs run_benchmark(db, engine, 500, tmp) and asserts the BenchResult fields populate (ingested == 500, explain_queries > 0). It exercises the wiring on every CI run under the pgvector service the test job already has, without waiting for the nightly bench job — and gives the QueryCounter-on-real-Postgres path its first real exercise.

Also minor: bench.yml runs on push to main and scripts/benchmark.py exits 1 when the 10s target is missed, so a slow/noisy CI runner could occasionally red-line the (non-gating) bench job on main from variance rather than a real regression. Fine as a visibility signal; just flagging that the pass/fail is wall-clock-sensitive on shared runners.

[leo-aa88]

APPROVE

LGTM. The follow-up commit 8a4af14 adds exactly the integration smoke I suggested, and closes the MINOR from my previous review.

CONFIRMED — the DB-backed runner is now exercised on every CI run, and it passes. tests/integration/test_bench.py::test_run_benchmark_smoke runs run_benchmark(db, get_engine(), n_lines=200, tmp_path) against real Postgres and asserts structural results (n_lines == 200, ingested == 200, total_logs > 0, ingest_queries > 0, explain_queries > 0, non-negative wall times). I verified it ran, not skipped: test_bench.py::test_run_benchmark_smoke PASSED, full matrix green (test 3.10/3.12, docker, openapi).

Nicely, it asserts only structure, never wall-clock — which also addresses my second note about runner-variance sensitivity; the docstring calls that out explicitly. Same isolation pattern (drop_all/create_all, DB_URL/INTEGRATION_TESTS guard) as the other integration tests.

Everything from my prior pass still holds: pure generator/QueryCounter/report unit-tested, no src/core changes, contracts matched, non-gating bench workflow.

VERDICT

Correct, boundary-respecting, and now proven end-to-end on real Postgres via a variance-safe smoke test, with the full CI matrix green. No BLOCKING/MAJOR/MINOR findings remain. Merge it. (Comment rather than Approve — GitHub blocks self-approval on your own PR.)