From 001fbb0b58963a165b6de1bc43d89b5bbfbe9ea7 Mon Sep 17 00:00:00 2001 From: wangwei Date: Thu, 2 Jul 2026 16:41:36 +0800 Subject: [PATCH] feat(advisor-comparison): add find_previous_run same-scenario lookup Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- tests/test_advisor_comparison.py | 88 +++++++++++++++ webapp/services/advisor_comparison.py | 149 ++++++++++++++++++++++++++ 2 files changed, 237 insertions(+) create mode 100644 tests/test_advisor_comparison.py create mode 100644 webapp/services/advisor_comparison.py diff --git a/tests/test_advisor_comparison.py b/tests/test_advisor_comparison.py new file mode 100644 index 0000000..dae6bdb --- /dev/null +++ b/tests/test_advisor_comparison.py @@ -0,0 +1,88 @@ +"""Tests for webapp.services.advisor_comparison: same-scenario run comparison.""" +from __future__ import annotations + +import json +from pathlib import Path + +import pandas as pd + +from webapp.services.advisor_comparison import find_previous_run + + +def _write_fake_run( + root: Path, + run_id: str, + scenario_name: str, + finished_at: str, + judge_model: str, + rows: list[dict], +) -> Path: + """Write a minimal run directory discoverable by run_reader.list_run_summaries().""" + run_dir = root / run_id + run_dir.mkdir(parents=True, exist_ok=True) + pd.DataFrame(rows).to_csv(run_dir / "scores.csv", index=False) + metadata = { + "run_id": run_id, + "scenario_name": scenario_name, + "judge_model": judge_model, + "embedding_model": "embed-model", + "finished_at": finished_at, + "started_at": finished_at, + "valid_samples": len(rows), + "invalid_samples": 0, + } + (run_dir / "metadata.json").write_text(json.dumps(metadata), encoding="utf-8") + return run_dir + + +class TestFindPreviousRun: + def test_finds_most_recent_prior_run_with_same_scenario(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}]) + _write_fake_run(tmp_path, "r2", "scn-a", "2026-01-02T00:00:00+00:00", "gpt-5", + [{"sample_id": "s1", "faithfulness": 0.6}]) + _write_fake_run(tmp_path, "r3", "scn-a", "2026-01-03T00:00:00+00:00", "gpt-5", + [{"sample_id": "s1", "faithfulness": 0.9}]) + + previous = find_previous_run( + "scn-a", "r3", "2026-01-03T00:00:00+00:00", extra_roots=[tmp_path] + ) + + assert previous is not None + assert previous.run_id == "r2" + + def test_excludes_runs_with_different_scenario_name(self, tmp_path: Path) -> None: + _write_fake_run(tmp_path, "r1", "scn-other", "2026-01-01T00:00:00+00:00", "gpt-5", + [{"sample_id": "s1", "faithfulness": 0.5}]) + _write_fake_run(tmp_path, "r2", "scn-a", "2026-01-02T00:00:00+00:00", "gpt-5", + [{"sample_id": "s1", "faithfulness": 0.6}]) + + previous = find_previous_run( + "scn-a", "r2", "2026-01-02T00:00:00+00:00", extra_roots=[tmp_path] + ) + + assert previous is None + + def test_returns_none_when_no_history(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}]) + + previous = find_previous_run( + "scn-a", "r1", "2026-01-01T00:00:00+00:00", extra_roots=[tmp_path] + ) + + assert previous is None + + def test_ignores_runs_at_or_after_current_time(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}]) + _write_fake_run(tmp_path, "r2", "scn-a", "2026-01-05T00:00:00+00:00", "gpt-5", + [{"sample_id": "s1", "faithfulness": 0.6}]) + + # Current run finished at 2026-01-02, i.e. AFTER r1 but BEFORE r2. + previous = find_previous_run( + "scn-a", "r-current", "2026-01-02T00:00:00+00:00", extra_roots=[tmp_path] + ) + + assert previous is not None + assert previous.run_id == "r1" diff --git a/webapp/services/advisor_comparison.py b/webapp/services/advisor_comparison.py new file mode 100644 index 0000000..ad9a38d --- /dev/null +++ b/webapp/services/advisor_comparison.py @@ -0,0 +1,149 @@ +"""Advisor-diagnosis comparison against the same scenario's previous run. + +The report detail page shows, for the run currently being viewed, whether +metrics flagged by the optimization advisor in the immediately preceding run +of the same scenario have since improved, regressed, or remain unresolved. + +No new artifact files are written: diagnoses are recomputed on demand from +each run's scores.csv via the existing rag_eval.advisor.diagnose(), so the +comparison always reflects the current threshold rules in rules.py. +""" + +from __future__ import annotations + +import logging +from pathlib import Path +from typing import Any + +import pandas as pd + +from rag_eval.advisor import diagnose +from webapp.models import AdvisorComparison, AdvisorComparisonEntry, RunSummary +from webapp.services import run_reader + +logger = logging.getLogger("webapp.services.advisor_comparison") + +# Worse-first ordering so regressions and unresolved issues surface above +# resolved ones in the rendered comparison list. +_STATUS_ORDER = {"regressed": 0, "still_triggered": 1, "new_metric": 2, "resolved": 3} + + +def find_previous_run( + scenario_name: str, + current_run_id: str, + current_finished_at: str, + extra_roots: list[Path] | None = None, +) -> RunSummary | None: + """Return the most recent prior run with the same scenario_name, or None. + + "Prior" means finished_at strictly earlier than current_finished_at, so a + run that finished later (e.g. a concurrent run) is never mistaken for a + historical baseline. + """ + candidates = [ + summary + for summary in run_reader.list_run_summaries(extra_roots) + if summary.scenario_name == scenario_name + and summary.run_id != current_run_id + and (summary.finished_at or "") < (current_finished_at or "") + ] + if not candidates: + return None + candidates.sort(key=lambda summary: summary.finished_at or "", reverse=True) + return candidates[0] + + +def _rows_as_records(frame: pd.DataFrame) -> list[dict[str, Any]]: + """Convert a scores dataframe into plain dict records, NaN -> None.""" + if frame.empty: + return [] + return frame.where(pd.notnull(frame), None).to_dict("records") + + +def build_advisor_comparison( + run_dir: Path, + scenario_name: str, + metrics: list[str], + extra_roots: list[Path] | None = None, +) -> AdvisorComparison | None: + """Build the current run's advisor-diagnosis delta vs. its same-scenario predecessor. + + Returns None when there is no predecessor, the predecessor's data cannot be + read, or there is nothing worth surfacing (no metric changed status). Never + raises — any failure degrades to None so the report detail page is + unaffected (mirrors run_advisor()'s defensive error handling). + """ + try: + metadata = run_reader._read_json(run_dir / "metadata.json") + current_run_id = str(metadata.get("run_id") or run_dir.name) + current_finished_at = str(metadata.get("finished_at") or "") + current_judge_model = str(metadata.get("judge_model", "")) + + previous = find_previous_run( + scenario_name, current_run_id, current_finished_at, extra_roots + ) + if previous is None: + return None + + previous_dir = Path(previous.output_path) + previous_frame = run_reader.read_scores_frame(previous_dir) + if previous_frame.empty: + return None + previous_rows = _rows_as_records(previous_frame) + previous_metrics = previous.metrics + + current_frame = run_reader.read_scores_frame(run_dir) + current_rows = _rows_as_records(current_frame) + + previous_diagnoses = {d.metric: d for d in diagnose(previous_rows, previous_metrics)} + current_diagnoses = {d.metric: d for d in diagnose(current_rows, metrics)} + + entries: list[AdvisorComparisonEntry] = [] + for metric in sorted(set(previous_diagnoses) | set(current_diagnoses)): + prev_d = previous_diagnoses.get(metric) + curr_d = current_diagnoses.get(metric) + + if prev_d is not None and curr_d is None: + if metric not in metrics: + # Dropped from the current scope entirely — cannot fairly + # claim "resolved" without a current-side measurement. + continue + status = "resolved" + elif prev_d is None and curr_d is not None: + status = "regressed" if metric in previous_metrics else "new_metric" + else: + status = "still_triggered" + + entries.append(AdvisorComparisonEntry( + metric=metric, + status=status, + previous_score=prev_d.mean_score if prev_d else None, + previous_severity=prev_d.severity if prev_d else None, + current_score=curr_d.mean_score if curr_d else None, + current_severity=curr_d.severity if curr_d else None, + )) + + if not entries: + return None + + entries.sort(key=lambda e: (_STATUS_ORDER.get(e.status, 99), e.metric)) + + previous_judge_model = previous.judge_model + return AdvisorComparison( + previous_run_id=previous.run_id, + previous_finished_at=previous.finished_at, + previous_judge_model=previous_judge_model, + current_judge_model=current_judge_model, + judge_model_changed=( + bool(previous_judge_model) + and bool(current_judge_model) + and previous_judge_model != current_judge_model + ), + entries=entries, + ) + except Exception as exc: # noqa: BLE001 + logger.warning( + "[advisor_comparison] failed to build comparison for run_dir=%s: %s", + run_dir, exc, + ) + return None