perf: get_health_summary uses one query instead of 4 per source

src/tendertracker/pipeline/health.py

@@ -19,28 +19,33 @@ class SourceHealth:
 def get_health_summary(session: Session) -> list[SourceHealth]:
     """Per-source rollup — the "at a glance" view an ops user actually
     wants, versus `status`'s flat recent-runs-across-all-sources list where
-    a quiet source's last run can get buried by a noisy one."""
-    sources = sorted(row[0] for row in session.query(SyncLog.source).distinct().all())
+    a quiet source's last run can get buried by a noisy one.
+
+    One query fetches every SyncLog row (newest first), then aggregates per
+    source in Python — previously 4 separate queries per source (4N+1
+    total), fine at this project's real scale (a handful of sources) but a
+    real inefficiency if it ever needs to track many.
+    """
+    logs = session.query(SyncLog).order_by(SyncLog.started_at.desc()).all()
+
+    by_source: dict[str, list[SyncLog]] = {}
+    for log in logs:
+        by_source.setdefault(log.source, []).append(log)
 
     summary = []
-    for source in sources:
-        last_run = session.query(SyncLog).filter_by(source=source).order_by(SyncLog.started_at.desc()).first()
-        last_success = (
-            session.query(SyncLog)
-            .filter_by(source=source, status="success")
-            .order_by(SyncLog.started_at.desc())
-            .first()
-        )
-        error_count = session.query(SyncLog).filter(SyncLog.source == source, SyncLog.error_message.isnot(None)).count()
-        total_runs = session.query(SyncLog).filter_by(source=source).count()
+    for source in sorted(by_source):
+        source_logs = by_source[source]  # already newest-first
+        last_run = source_logs[0]
+        last_success = next((log for log in source_logs if log.status == "success"), None)
+        error_count = sum(1 for log in source_logs if log.error_message is not None)
         summary.append(
             SourceHealth(
                 source=source,
-                last_run_at=last_run.started_at if last_run else None,
-                last_status=last_run.status if last_run else None,
+                last_run_at=last_run.started_at,
+                last_status=last_run.status,
                 last_success_at=last_success.started_at if last_success else None,
                 error_count=error_count,
-                total_runs=total_runs,
+                total_runs=len(source_logs),
             )
         )
     return summary

tests/test_health.py

@@ -0,0 +1,90 @@
+from tendertracker.pipeline.health import get_health_summary, get_recent_errors
+from tendertracker.storage.models import SyncLog, utcnow
+
+
+def _log(source: str, status: str, error_message: str | None = None) -> SyncLog:
+    return SyncLog(source=source, started_at=utcnow(), finished_at=utcnow(), status=status, error_message=error_message)
+
+
+def test_empty_db_returns_empty_summary(db_session):
+    assert get_health_summary(db_session) == []
+
+
+def test_single_source_single_run(db_session):
+    db_session.add(_log("sandbox", "success"))
+    db_session.commit()
+
+    summary = get_health_summary(db_session)
+    assert len(summary) == 1
+    s = summary[0]
+    assert s.source == "sandbox"
+    assert s.last_status == "success"
+    assert s.last_success_at is not None
+    assert s.error_count == 0
+    assert s.total_runs == 1
+
+
+def test_last_success_skips_a_more_recent_failure(db_session):
+    """The whole point of tracking last_success separately from last_run —
+    a source that's currently failing should still show when it last
+    actually worked, not just 'never'."""
+    import time
+
+    db_session.add(_log("sandbox", "success"))
+    db_session.commit()
+    time.sleep(0.01)
+    db_session.add(_log("sandbox", "failed", error_message="boom"))
+    db_session.commit()
+
+    summary = get_health_summary(db_session)
+    s = summary[0]
+    assert s.last_status == "failed"  # most recent run failed
+    assert s.last_success_at is not None  # but a success exists earlier
+    assert s.error_count == 1
+    assert s.total_runs == 2
+
+
+def test_never_succeeded_reports_none(db_session):
+    db_session.add(_log("sandbox", "failed", error_message="boom"))
+    db_session.commit()
+
+    s = get_health_summary(db_session)[0]
+    assert s.last_success_at is None
+
+
+def test_multiple_sources_kept_separate_and_sorted(db_session):
+    db_session.add(_log("zzz-source", "success"))
+    db_session.add(_log("aaa-source", "success"))
+    db_session.commit()
+
+    summary = get_health_summary(db_session)
+    assert [s.source for s in summary] == ["aaa-source", "zzz-source"]
+
+
+def test_error_count_only_counts_rows_with_error_message(db_session):
+    db_session.add(_log("sandbox", "success"))
+    db_session.add(_log("sandbox", "dry-run"))
+    db_session.add(_log("sandbox", "failed", error_message="boom"))
+    db_session.commit()
+
+    s = get_health_summary(db_session)[0]
+    assert s.error_count == 1
+    assert s.total_runs == 3
+
+
+def test_get_recent_errors_only_returns_rows_with_error_message(db_session):
+    db_session.add(_log("sandbox", "success"))
+    db_session.add(_log("sandbox", "failed", error_message="boom"))
+    db_session.commit()
+
+    errors = get_recent_errors(db_session)
+    assert len(errors) == 1
+    assert errors[0].error_message == "boom"
+
+
+def test_get_recent_errors_respects_limit(db_session):
+    for i in range(5):
+        db_session.add(_log("sandbox", "failed", error_message=f"error {i}"))
+    db_session.commit()
+
+    assert len(get_recent_errors(db_session, limit=2)) == 2