seam: PH6.6's /metrics solver_run test updated for PH1.2 (solver_run now exists) #67

New issue

Open

opened 2026-09-11 11:43:18 +00:00 by patrick · 0 comments

patrick commented

2026-09-11 11:43:18 +00:00

Owner

Discovered while building PH1.2 (#10, feature/db-schema-planning). backend/tests/integration/api/test_metrics_db.py and backend/src/polaris/api/metrics.py are outside PH1.2's owned paths (backend/alembic/versions/**, backend/src/polaris/db/models/**) — they belong to PH6.6 (#47, already merged) — but PH1.2's whole purpose (creating the solver_run table) breaks a pre-existing PH6.6 test by design, so I fixed the test directly in the PH1.2 PR (#66) rather than leave a regression or widen the ticket, per the seam-request process in CLAUDE.md (mirroring issue #57 from PH1.1).

What was stale: api/metrics.py::_render_solver_metrics was written "best-effort" against solver_run not existing yet (sa_inspect(bind).has_table("solver_run") gates the whole function), with a comment: "once the migration lands these lines start appearing with no further change here." The matching test, test_metrics_omits_solver_run_lines_when_table_does_not_exist_yet, asserted "polaris_solver_run_total" not in resp.text — but once the table exists (even empty), the function always emits the # HELP/# TYPE header lines for polaris_solver_run_total, which contain that exact substring, so the assertion fails against a real, correct, empty-table render.

Fix (already in PH1.2's PR, backend/tests/integration/api/test_metrics_db.py): no change to api/metrics.py itself — the module's own docstring already anticipated this and needed nothing further. The test was split into two:

test_metrics_renders_solver_run_gauges_when_table_is_empty: asserts the HELP/TYPE headers and a zero-valued polaris_solver_run_duration_seconds_avg line appear, but no per-status polaris_solver_run_total{status=...} line, when solver_run has no rows.
test_metrics_reports_solver_run_counts_against_running_postgres: inserts a couple of SolverRun rows and asserts the per-status counter lines render correctly, exercising the query path PH6.6 could previously only test as "never runs, table absent".

Also updated tests/integration/db/test_migrations.py: the PH1.1 test test_0002_upgrade_then_downgrade_minus_one_round_trips downgraded -1 from head to check 0002's round-trip — now that head is 0003, that no longer tests what its name says. Changed it to upgrade explicitly to 0002 first, and added test_0003_upgrade_then_downgrade_minus_one_round_trips as the 0003 equivalent at head. This file is squarely within PH1.1/PH1.2's own Owns column (it's the test companion to backend/alembic/versions/**), not a seam, but noting it here for the same traceability reason.

No action needed unless another session already has local changes to api/metrics.py or tests/integration/api/test_metrics_db.py on a different branch. Opened for traceability, not as a blocker (already resolved in #66).

Discovered while building PH1.2 (#10, `feature/db-schema-planning`). `backend/tests/integration/api/test_metrics_db.py` and `backend/src/polaris/api/metrics.py` are outside PH1.2's owned paths (`backend/alembic/versions/**`, `backend/src/polaris/db/models/**`) — they belong to PH6.6 (#47, already merged) — but PH1.2's whole purpose (creating the `solver_run` table) breaks a pre-existing PH6.6 test by design, so I fixed the test directly in the PH1.2 PR (#66) rather than leave a regression or widen the ticket, per the seam-request process in CLAUDE.md (mirroring issue #57 from PH1.1). **What was stale**: `api/metrics.py::_render_solver_metrics` was written "best-effort" against `solver_run` not existing yet (`sa_inspect(bind).has_table("solver_run")` gates the whole function), with a comment: "once the migration lands these lines start appearing with no further change here." The matching test, `test_metrics_omits_solver_run_lines_when_table_does_not_exist_yet`, asserted `"polaris_solver_run_total" not in resp.text` — but once the table exists (even empty), the function always emits the `# HELP`/`# TYPE` header lines for `polaris_solver_run_total`, which contain that exact substring, so the assertion fails against a real, correct, empty-table render. **Fix** (already in PH1.2's PR, `backend/tests/integration/api/test_metrics_db.py`): no change to `api/metrics.py` itself — the module's own docstring already anticipated this and needed nothing further. The test was split into two: - `test_metrics_renders_solver_run_gauges_when_table_is_empty`: asserts the HELP/TYPE headers and a zero-valued `polaris_solver_run_duration_seconds_avg` line appear, but no per-status `polaris_solver_run_total{status=...}` line, when `solver_run` has no rows. - `test_metrics_reports_solver_run_counts_against_running_postgres`: inserts a couple of `SolverRun` rows and asserts the per-status counter lines render correctly, exercising the query path PH6.6 could previously only test as "never runs, table absent". Also updated `tests/integration/db/test_migrations.py`: the PH1.1 test `test_0002_upgrade_then_downgrade_minus_one_round_trips` downgraded `-1` from `head` to check `0002`'s round-trip — now that `head` is `0003`, that no longer tests what its name says. Changed it to upgrade explicitly *to* `0002` first, and added `test_0003_upgrade_then_downgrade_minus_one_round_trips` as the `0003` equivalent at `head`. This file is squarely within PH1.1/PH1.2's own `Owns` column (it's the test companion to `backend/alembic/versions/**`), not a seam, but noting it here for the same traceability reason. No action needed unless another session already has local changes to `api/metrics.py` or `tests/integration/api/test_metrics_db.py` on a different branch. Opened for traceability, not as a blocker (already resolved in #66).

patrick referenced this issue

2026-09-11 11:43:31 +00:00

PH1.2 — Schema 0003: planning tables and typed mapped classes #10