feat(advisor-comparison): add build_advisor_comparison diagnosis diffing

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

docs/superpowers/plans/2026-07-02-advisor-comparison.md

@@ -553,8 +553,10 @@ class TestBuildAdvisorComparison:
             tmp_path, "r1", "scn-a", "2026-01-01T00:00:00+00:00", "gpt-5",
             [{"sample_id": "s1", "faithfulness": 0.5}],
         )
-        # Corrupt the previous run's scores.csv after it was written.
-        (previous_dir / "scores.csv").write_text("not,a,valid\ncsv,,,", encoding="utf-8")
+        # Corrupt the previous run's scores.csv with invalid-encoding bytes so
+        # pd.read_csv raises UnicodeDecodeError (a ValueError subclass caught
+        # by run_reader.read_scores_frame, which then returns an empty frame).
+        (previous_dir / "scores.csv").write_bytes(b"\xff\xfe\x00\x01broken binary data \x00\x00")
 
         current_dir = _write_fake_run(
             tmp_path, "r2", "scn-a", "2026-01-02T00:00:00+00:00", "gpt-5",

tests/test_advisor_comparison.py

@@ -6,7 +6,7 @@ from pathlib import Path
 
 import pandas as pd
 
-from webapp.services.advisor_comparison import find_previous_run
+from webapp.services.advisor_comparison import build_advisor_comparison, find_previous_run
 
 
 def _write_fake_run(
@@ -86,3 +86,221 @@ class TestFindPreviousRun:
 
         assert previous is not None
         assert previous.run_id == "r1"
+
+
+class TestBuildAdvisorComparison:
+    def test_resolved_status_when_previously_triggered_now_healthy(self, tmp_path: Path) -> None:
+        _write_fake_run(
+            tmp_path, "r1", "scn-a", "2026-01-01T00:00:00+00:00", "gpt-5",
+            [{"sample_id": "s1", "faithfulness": 0.5}, {"sample_id": "s2", "faithfulness": 0.5}],
+        )
+        current_dir = _write_fake_run(
+            tmp_path, "r2", "scn-a", "2026-01-02T00:00:00+00:00", "gpt-5",
+            [{"sample_id": "s1", "faithfulness": 0.95}, {"sample_id": "s2", "faithfulness": 0.95}],
+        )
+
+        comparison = build_advisor_comparison(
+            current_dir, "scn-a", ["faithfulness"], extra_roots=[tmp_path]
+        )
+
+        assert comparison is not None
+        assert len(comparison.entries) == 1
+        entry = comparison.entries[0]
+        assert entry.metric == "faithfulness"
+        assert entry.status == "resolved"
+        assert entry.previous_score == 0.5
+        assert entry.current_score is None  # not triggered now → no Diagnosis on current side
+
+    def test_regressed_status_when_previously_healthy_now_triggered(self, tmp_path: Path) -> None:
+        _write_fake_run(
+            tmp_path, "r1", "scn-a", "2026-01-01T00:00:00+00:00", "gpt-5",
+            [{"sample_id": "s1", "faithfulness": 0.95}, {"sample_id": "s2", "faithfulness": 0.95}],
+        )
+        current_dir = _write_fake_run(
+            tmp_path, "r2", "scn-a", "2026-01-02T00:00:00+00:00", "gpt-5",
+            [{"sample_id": "s1", "faithfulness": 0.5}, {"sample_id": "s2", "faithfulness": 0.5}],
+        )
+
+        comparison = build_advisor_comparison(
+            current_dir, "scn-a", ["faithfulness"], extra_roots=[tmp_path]
+        )
+
+        assert comparison is not None
+        assert len(comparison.entries) == 1
+        entry = comparison.entries[0]
+        assert entry.status == "regressed"
+        assert entry.previous_score is None
+        assert entry.current_score == 0.5
+
+    def test_still_triggered_status_shows_score_and_severity_change(self, tmp_path: Path) -> None:
+        _write_fake_run(
+            tmp_path, "r1", "scn-a", "2026-01-01T00:00:00+00:00", "gpt-5",
+            [{"sample_id": "s1", "faithfulness": 0.55}, {"sample_id": "s2", "faithfulness": 0.55}],
+        )
+        current_dir = _write_fake_run(
+            tmp_path, "r2", "scn-a", "2026-01-02T00:00:00+00:00", "gpt-5",
+            [{"sample_id": "s1", "faithfulness": 0.45}, {"sample_id": "s2", "faithfulness": 0.45}],
+        )
+
+        comparison = build_advisor_comparison(
+            current_dir, "scn-a", ["faithfulness"], extra_roots=[tmp_path]
+        )
+
+        assert comparison is not None
+        entry = comparison.entries[0]
+        assert entry.status == "still_triggered"
+        assert entry.previous_severity == "warning"
+        assert entry.current_severity == "critical"
+        assert entry.previous_score == 0.55
+        assert entry.current_score == 0.45
+
+    def test_new_metric_status_when_metric_not_measured_before(self, tmp_path: Path) -> None:
+        # Previous run only measured context_recall (healthy); faithfulness wasn't tracked at all.
+        _write_fake_run(
+            tmp_path, "r1", "scn-a", "2026-01-01T00:00:00+00:00", "gpt-5",
+            [{"sample_id": "s1", "context_recall": 0.95}, {"sample_id": "s2", "context_recall": 0.95}],
+        )
+        current_dir = _write_fake_run(
+            tmp_path, "r2", "scn-a", "2026-01-02T00:00:00+00:00", "gpt-5",
+            [{"sample_id": "s1", "faithfulness": 0.5}, {"sample_id": "s2", "faithfulness": 0.5}],
+        )
+
+        comparison = build_advisor_comparison(
+            current_dir, "scn-a", ["faithfulness"], extra_roots=[tmp_path]
+        )
+
+        assert comparison is not None
+        assert len(comparison.entries) == 1
+        entry = comparison.entries[0]
+        assert entry.metric == "faithfulness"
+        assert entry.status == "new_metric"
+        assert entry.previous_score is None
+        assert entry.current_score == 0.5
+
+    def test_metrics_healthy_in_both_are_omitted(self, tmp_path: Path) -> None:
+        _write_fake_run(
+            tmp_path, "r1", "scn-a", "2026-01-01T00:00:00+00:00", "gpt-5",
+            [{"sample_id": "s1", "faithfulness": 0.95}],
+        )
+        current_dir = _write_fake_run(
+            tmp_path, "r2", "scn-a", "2026-01-02T00:00:00+00:00", "gpt-5",
+            [{"sample_id": "s1", "faithfulness": 0.96}],
+        )
+
+        comparison = build_advisor_comparison(
+            current_dir, "scn-a", ["faithfulness"], extra_roots=[tmp_path]
+        )
+
+        assert comparison is None  # nothing to show → overall None
+
+    def test_metric_dropped_from_current_scope_is_not_marked_resolved(self, tmp_path: Path) -> None:
+        # Previous run triggered on context_precision, but current run doesn't
+        # evaluate that metric at all — must NOT claim "resolved" without a
+        # fair current-side measurement.
+        _write_fake_run(
+            tmp_path, "r1", "scn-a", "2026-01-01T00:00:00+00:00", "gpt-5",
+            [{"sample_id": "s1", "context_precision": 0.3}],
+        )
+        current_dir = _write_fake_run(
+            tmp_path, "r2", "scn-a", "2026-01-02T00:00:00+00:00", "gpt-5",
+            [{"sample_id": "s1", "faithfulness": 0.95}],
+        )
+
+        comparison = build_advisor_comparison(
+            current_dir, "scn-a", ["faithfulness"], extra_roots=[tmp_path]
+        )
+
+        assert comparison is None
+
+    def test_returns_none_when_no_previous_run(self, tmp_path: Path) -> None:
+        current_dir = _write_fake_run(
+            tmp_path, "r1", "scn-a", "2026-01-01T00:00:00+00:00", "gpt-5",
+            [{"sample_id": "s1", "faithfulness": 0.5}],
+        )
+
+        comparison = build_advisor_comparison(
+            current_dir, "scn-a", ["faithfulness"], extra_roots=[tmp_path]
+        )
+
+        assert comparison is None
+
+    def test_judge_model_changed_flag_set_when_models_differ(self, tmp_path: Path) -> None:
+        _write_fake_run(
+            tmp_path, "r1", "scn-a", "2026-01-01T00:00:00+00:00", "gpt-4o",
+            [{"sample_id": "s1", "faithfulness": 0.5}],
+        )
+        current_dir = _write_fake_run(
+            tmp_path, "r2", "scn-a", "2026-01-02T00:00:00+00:00", "gpt-5",
+            [{"sample_id": "s1", "faithfulness": 0.5}],
+        )
+
+        comparison = build_advisor_comparison(
+            current_dir, "scn-a", ["faithfulness"], extra_roots=[tmp_path]
+        )
+
+        assert comparison is not None
+        assert comparison.judge_model_changed is True
+        assert comparison.previous_judge_model == "gpt-4o"
+        assert comparison.current_judge_model == "gpt-5"
+
+    def test_judge_model_changed_false_when_same(self, tmp_path: Path) -> None:
+        _write_fake_run(
+            tmp_path, "r1", "scn-a", "2026-01-01T00:00:00+00:00", "gpt-5",
+            [{"sample_id": "s1", "faithfulness": 0.5}],
+        )
+        current_dir = _write_fake_run(
+            tmp_path, "r2", "scn-a", "2026-01-02T00:00:00+00:00", "gpt-5",
+            [{"sample_id": "s1", "faithfulness": 0.5}],
+        )
+
+        comparison = build_advisor_comparison(
+            current_dir, "scn-a", ["faithfulness"], extra_roots=[tmp_path]
+        )
+
+        assert comparison is not None
+        assert comparison.judge_model_changed is False
+
+    def test_gracefully_returns_none_on_corrupt_previous_scores_csv(self, tmp_path: Path) -> None:
+        previous_dir = _write_fake_run(
+            tmp_path, "r1", "scn-a", "2026-01-01T00:00:00+00:00", "gpt-5",
+            [{"sample_id": "s1", "faithfulness": 0.5}],
+        )
+        # Corrupt the previous run's scores.csv with invalid-encoding bytes so
+        # pd.read_csv raises UnicodeDecodeError (a ValueError subclass caught
+        # by run_reader.read_scores_frame, which then returns an empty frame).
+        (previous_dir / "scores.csv").write_bytes(b"\xff\xfe\x00\x01broken binary data \x00\x00")
+
+        current_dir = _write_fake_run(
+            tmp_path, "r2", "scn-a", "2026-01-02T00:00:00+00:00", "gpt-5",
+            [{"sample_id": "s1", "faithfulness": 0.5}],
+        )
+
+        comparison = build_advisor_comparison(
+            current_dir, "scn-a", ["faithfulness"], extra_roots=[tmp_path]
+        )
+
+        assert comparison is None
+
+    def test_worse_statuses_sorted_before_resolved(self, tmp_path: Path) -> None:
+        """regressed/still_triggered/new_metric surface above resolved for visibility."""
+        _write_fake_run(
+            tmp_path, "r1", "scn-a", "2026-01-01T00:00:00+00:00", "gpt-5",
+            [
+                {"sample_id": "s1", "faithfulness": 0.5, "context_recall": 0.95},
+            ],
+        )
+        current_dir = _write_fake_run(
+            tmp_path, "r2", "scn-a", "2026-01-02T00:00:00+00:00", "gpt-5",
+            [
+                {"sample_id": "s1", "faithfulness": 0.95, "context_recall": 0.5},
+            ],
+        )
+
+        comparison = build_advisor_comparison(
+            current_dir, "scn-a", ["faithfulness", "context_recall"], extra_roots=[tmp_path]
+        )
+
+        assert comparison is not None
+        statuses = [entry.status for entry in comparison.entries]
+        # context_recall regressed → must appear before faithfulness resolved.
+        assert statuses.index("regressed") < statuses.index("resolved")