Extract shared build_metric_registry factory (DRY)

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

tests/test_advisor_llm_analyzer.py

@@ -0,0 +1,141 @@
+"""Tests for the optimization advisor's direct-LLM analyzer.
+
+These tests inject a fake async chat client so no network call is made. They
+verify that analyze() uses a plain chat.completions call (not the removed
+langchain path), returns the text from choices[0].message.content, embeds the
+worked-example instructions and the low-sample contexts in the prompt, and
+selects the correct token parameter for reasoning vs. legacy models.
+"""
+
+from __future__ import annotations
+
+import asyncio
+
+from rag_eval.advisor.llm_analyzer import analyze, _is_reasoning_model
+from rag_eval.advisor.rules import Diagnosis
+
+
+class _FakeMessage:
+    def __init__(self, content: str) -> None:
+        self.content = content
+
+
+class _FakeChoice:
+    def __init__(self, content: str) -> None:
+        self.message = _FakeMessage(content)
+
+
+class _FakeResponse:
+    def __init__(self, content: str) -> None:
+        self.choices = [_FakeChoice(content)]
+
+
+class _FakeCompletions:
+    def __init__(self, captured: dict) -> None:
+        self._captured = captured
+
+    async def create(self, **kwargs):
+        self._captured.update(kwargs)
+        return _FakeResponse("## faithfulness [警告]\n\n针对该问题的具体优化建议")
+
+
+class _FakeChat:
+    def __init__(self, captured: dict) -> None:
+        self.completions = _FakeCompletions(captured)
+
+
+class _FakeClient:
+    def __init__(self, captured: dict) -> None:
+        self.chat = _FakeChat(captured)
+        self.closed = False
+
+    async def close(self) -> None:
+        self.closed = True
+
+
+class _Settings:
+    ragas_llm_max_tokens = 4096
+
+
+def _diagnosis() -> Diagnosis:
+    return Diagnosis(
+        metric="faithfulness",
+        mean_score=0.55,
+        threshold=0.7,
+        severity="warning",
+        root_causes=["生成未严格 grounding"],
+        suggested_actions=["强化 grounding 约束"],
+        low_samples=[
+            {
+                "sample_id": "s1",
+                "question": "球管寿命如何评估？",
+                "answer": "球管寿命约 3 年。",
+                "ground_truth": "球管寿命取决于使用强度。",
+                "contexts": "球管寿命与扫描负载相关 |||| 高负载会缩短寿命",
+                "faithfulness": 0.4,
+            }
+        ],
+    )
+
+
+def test_analyze_uses_direct_chat_and_returns_content() -> None:
+    captured: dict = {}
+    text = asyncio.run(
+        analyze([_diagnosis()], "scn", "gpt-4o", _Settings(), chat_client=_FakeClient(captured))
+    )
+    assert "优化建议" in text
+    assert captured["model"] == "gpt-4o"
+    prompt = captured["messages"][0]["content"]
+    assert "举例拆解" in prompt          # worked-example instruction present
+    assert "球管寿命与扫描负载相关" in prompt  # low-sample contexts embedded
+    assert "max_tokens" in captured       # legacy model uses max_tokens
+    assert "max_completion_tokens" not in captured
+
+
+def test_analyze_reasoning_model_uses_max_completion_tokens() -> None:
+    captured: dict = {}
+    asyncio.run(
+        analyze([_diagnosis()], "scn", "gpt-5", _Settings(), chat_client=_FakeClient(captured))
+    )
+    assert "max_completion_tokens" in captured
+    assert "max_tokens" not in captured
+
+
+def test_analyze_empty_diagnoses_returns_empty() -> None:
+    assert asyncio.run(analyze([], "scn", "gpt-4o", _Settings())) == ""
+
+
+def test_analyze_closes_client_it_creates(monkeypatch) -> None:
+    """A self-created client is closed in-loop to avoid 'Event loop is closed'."""
+    captured: dict = {}
+    fake = _FakeClient(captured)
+
+    import openai
+    import rag_eval.metrics.factory as factory_mod
+
+    monkeypatch.setattr(openai, "AsyncOpenAI", lambda **kwargs: fake)
+    monkeypatch.setattr(
+        factory_mod, "resolve_openai_client_kwargs", lambda *a, **k: {"api_key": "x"}
+    )
+
+    # No chat_client passed → analyze() builds (and must close) its own client.
+    text = asyncio.run(analyze([_diagnosis()], "scn", "gpt-4o", _Settings()))
+
+    assert "优化建议" in text
+    assert fake.closed is True
+
+
+def test_analyze_does_not_close_injected_client() -> None:
+    """An injected client is owned by the caller and must not be closed."""
+    fake = _FakeClient({})
+    asyncio.run(analyze([_diagnosis()], "scn", "gpt-4o", _Settings(), chat_client=fake))
+    assert fake.closed is False
+
+
+def test_is_reasoning_model_detection() -> None:
+    assert _is_reasoning_model("gpt-5")
+    assert _is_reasoning_model("gpt-5.5")
+    assert _is_reasoning_model("o1-mini")
+    assert _is_reasoning_model("o3")
+    assert not _is_reasoning_model("gpt-4o")
+    assert not _is_reasoning_model("deepseek-v4-flash")

tests/test_dashboard.py

@@ -0,0 +1,197 @@
+"""Tests for the Dashboard module's pure data functions and MetricPresenter additions.
+
+The pure data functions (_buildTrendDatasets, _buildComparisonData) and the new
+MetricPresenter helpers (passThreshold, meetsTarget) are tested via Node.js so no
+browser or network is required.
+"""
+from __future__ import annotations
+
+import subprocess
+from pathlib import Path
+
+REPO_ROOT = Path(__file__).resolve().parents[1]
+
+
+def _run_node(script: str) -> str:
+    """Execute a Node.js script and return stdout."""
+    completed = subprocess.run(
+        ["node", "-e", script],
+        cwd=REPO_ROOT,
+        capture_output=True,
+        text=True,
+        encoding="utf-8",
+        check=True,
+    )
+    return completed.stdout.strip()
+
+
+def _load_js() -> str:
+    """Return a Node-runnable bootstrap that loads MetricPresenter + Dashboard."""
+    presenter_path = (REPO_ROOT / "webapp" / "static" / "js" / "metric_presenter.js").as_posix()
+    dashboard_path = (REPO_ROOT / "webapp" / "static" / "js" / "dashboard.js").as_posix()
+    return f"""
+const fs = require("fs");
+const vm = require("vm");
+
+// Shared sandbox (window object shared by both scripts)
+const sandbox = {{ window: {{}}, console }};
+
+// MetricPresenter
+vm.runInNewContext(fs.readFileSync("{presenter_path}", "utf8"), sandbox);
+const MetricPresenter = sandbox.window.MetricPresenter;
+
+// Minimal App stub required by dashboard.js
+sandbox.App = {{
+  escape: (s) => String(s == null ? "" : s),
+  shortMetric: (m) => m,
+  shortTime: (t) => (t || "").slice(0, 16),
+  scoreClass: (m, v) => MetricPresenter.scoreClass(m, v),
+}};
+sandbox.MetricPresenter = MetricPresenter;
+// Stub Chart.js (not exercised by pure functions)
+sandbox.Chart = function() {{ this.destroy = () => {{}}; }};
+
+// Dashboard (attaches itself to sandbox.window.Dashboard)
+vm.runInNewContext(fs.readFileSync("{dashboard_path}", "utf8"), sandbox);
+const Dashboard = sandbox.window.Dashboard;
+"""
+
+
+def test_pass_threshold_higher_better() -> None:
+    """All higher-better metrics should have passThreshold 0.85."""
+    script = _load_js() + """
+const result = {
+  faith: MetricPresenter.passThreshold("faithfulness"),
+  ans: MetricPresenter.passThreshold("answer_relevancy"),
+  recall: MetricPresenter.passThreshold("context_recall"),
+  prec: MetricPresenter.passThreshold("context_precision"),
+  fact: MetricPresenter.passThreshold("factual_correctness"),
+  sem: MetricPresenter.passThreshold("semantic_similarity"),
+};
+console.log(JSON.stringify(result));
+"""
+    out = _run_node(script)
+    assert '"faith":0.85' in out
+    assert '"ans":0.85' in out
+    assert '"recall":0.85' in out
+    assert '"prec":0.85' in out
+    assert '"fact":0.85' in out
+    assert '"sem":0.85' in out
+
+
+def test_pass_threshold_noise_sensitivity_lower_better() -> None:
+    """noise_sensitivity (lower-better) should have passThreshold 0.15."""
+    script = _load_js() + """
+console.log(JSON.stringify(MetricPresenter.passThreshold("noise_sensitivity")));
+"""
+    out = _run_node(script)
+    assert out.strip() == "0.15"
+
+
+def test_meets_target_higher_better() -> None:
+    """meetsTarget should return true only at/above 0.85 for higher-better metrics."""
+    script = _load_js() + """
+const result = {
+  at085: MetricPresenter.meetsTarget("faithfulness", 0.85),
+  above: MetricPresenter.meetsTarget("faithfulness", 0.90),
+  below: MetricPresenter.meetsTarget("faithfulness", 0.84),
+  zero: MetricPresenter.meetsTarget("faithfulness", 0),
+  nul: MetricPresenter.meetsTarget("faithfulness", null),
+};
+console.log(JSON.stringify(result));
+"""
+    out = _run_node(script)
+    assert '"at085":true' in out
+    assert '"above":true' in out
+    assert '"below":false' in out
+    assert '"zero":false' in out
+    assert '"nul":false' in out
+
+
+def test_meets_target_noise_sensitivity() -> None:
+    """meetsTarget for noise_sensitivity: true only at/below 0.15."""
+    script = _load_js() + """
+const result = {
+  at015: MetricPresenter.meetsTarget("noise_sensitivity", 0.15),
+  below: MetricPresenter.meetsTarget("noise_sensitivity", 0.10),
+  above: MetricPresenter.meetsTarget("noise_sensitivity", 0.16),
+};
+console.log(JSON.stringify(result));
+"""
+    out = _run_node(script)
+    assert '"at015":true' in out
+    assert '"below":true' in out
+    assert '"above":false' in out
+
+
+def test_build_trend_datasets_time_order_and_null_gap() -> None:
+    """_buildTrendDatasets returns metrics in appearance order, null for missing values."""
+    script = (
+        _load_js()
+        + """
+const runs = [
+  { run_id: "r1", scenario_name: "scn", finished_at: "2026-01-01T00:00:00",
+    metrics: ["faithfulness"], metric_means: { faithfulness: 0.60 } },
+  { run_id: "r2", scenario_name: "scn", finished_at: "2026-02-01T00:00:00",
+    metrics: ["faithfulness", "noise_sensitivity"],
+    metric_means: { faithfulness: 0.80, noise_sensitivity: 0.20 } },
+  { run_id: "r3", scenario_name: "scn", finished_at: "2026-03-01T00:00:00",
+    metrics: ["faithfulness"], metric_means: { faithfulness: 0.90 } },
+];
+const { labels, datasets } = Dashboard._buildTrendDatasets(runs);
+console.log(JSON.stringify({ labels, datasets }));
+"""
+    )
+    import json
+
+    out = json.loads(_run_node(script))
+    assert len(out["labels"]) == 3
+    # faithfulness dataset: all 3 points
+    faith_ds = next(d for d in out["datasets"] if "faithfulness" in d["label"])
+    assert faith_ds["data"] == [0.60, 0.80, 0.90]
+    # noise_sensitivity only present in r2 → null in r1 and r3
+    noise_ds = next(d for d in out["datasets"] if "noise_sensitivity" in d["label"])
+    assert noise_ds["data"] == [None, 0.20, None]
+    # noise label should note lower-is-better
+    assert "越低越好" in noise_ds["label"]
+
+
+def test_build_comparison_data_structure() -> None:
+    """_buildComparisonData returns correct labels, actuals, thresholds and targetMet."""
+    script = (
+        _load_js()
+        + """
+const run = {
+  run_id: "r1", scenario_name: "scn",
+  metrics: ["faithfulness", "noise_sensitivity"],
+  metric_means: { faithfulness: 0.90, noise_sensitivity: 0.10 },
+};
+const result = Dashboard._buildComparisonData(run);
+console.log(JSON.stringify(result));
+"""
+    )
+    import json
+
+    out = json.loads(_run_node(script))
+    assert out["labels"] == ["faithfulness", "noise_sensitivity"]
+    assert out["actual"] == [0.90, 0.10]
+    assert out["thresholds"] == [0.85, 0.15]
+    assert out["targetMet"] == [True, True]  # 0.90 >= 0.85 ✓; 0.10 <= 0.15 ✓
+
+
+def test_build_comparison_data_unmet_targets() -> None:
+    """targetMet is False when metrics are below threshold."""
+    script = (
+        _load_js()
+        + """
+const run = {
+  run_id: "r1", scenario_name: "scn",
+  metrics: ["faithfulness", "noise_sensitivity"],
+  metric_means: { faithfulness: 0.60, noise_sensitivity: 0.40 },
+};
+const result = Dashboard._buildComparisonData(run);
+console.log(JSON.stringify(result.targetMet));
+"""
+    )
+    out = _run_node(script)
+    assert out.strip() == "[false,false]"

tests/test_metric_registry.py

@@ -0,0 +1,27 @@
+"""Tests for the shared metric registry factory."""
+
+from unittest.mock import MagicMock
+
+from ragas.llms.base import InstructorBaseRagasLLM
+from ragas.embeddings.base import BaseRagasEmbedding
+
+from rag_eval.metrics.factory import build_metric_registry
+
+
+def _mock_llm():
+    """Return a mock that passes RAGAS InstructorLLM type checks."""
+    return MagicMock(spec=InstructorBaseRagasLLM)
+
+
+def _mock_emb():
+    """Return a mock that passes RAGAS embedding type checks."""
+    return MagicMock(spec=BaseRagasEmbedding)
+
+
+def test_build_metric_registry_has_all_seven_metrics():
+    """The registry exposes every supported metric keyed by its canonical name."""
+    registry = build_metric_registry(llm=_mock_llm(), embeddings=_mock_emb())
+    assert set(registry) == {
+        "faithfulness", "answer_relevancy", "context_recall", "context_precision",
+        "noise_sensitivity", "factual_correctness", "semantic_similarity",
+    }

tests/test_question_history.py

@@ -0,0 +1,131 @@
+"""Tests for the cross-run question-history index used by the report page."""
+
+from __future__ import annotations
+
+from pathlib import Path
+
+from webapp.services.question_history import (
+    build_question_history_index,
+    lookup,
+    normalize_question,
+)
+
+
+def _write_run(
+    run_dir: Path,
+    *,
+    run_id: str,
+    scenario_name: str,
+    finished_at: str,
+    rows: list[tuple[str, float]],
+    metric: str = "faithfulness",
+) -> None:
+    """Create a minimal run directory (metadata.json + scores.csv)."""
+    run_dir.mkdir(parents=True, exist_ok=True)
+    import json
+
+    (run_dir / "metadata.json").write_text(
+        json.dumps(
+            {
+                "run_id": run_id,
+                "scenario_name": scenario_name,
+                "finished_at": finished_at,
+                "valid_samples": len(rows),
+                "invalid_samples": 0,
+            }
+        ),
+        encoding="utf-8",
+    )
+    lines = [f"sample_id,question,{metric}"]
+    for i, (question, score) in enumerate(rows):
+        lines.append(f"s{i},{question},{score}")
+    (run_dir / "scores.csv").write_text("\n".join(lines), encoding="utf-8")
+
+
+# A question unlikely to collide with anything under the real outputs/ tree.
+_Q = "UNIQTESTQ ball tube lifetime evaluation method 9f3a"
+
+
+def test_normalize_question_is_case_and_whitespace_insensitive() -> None:
+    assert normalize_question("  Hello   World ") == normalize_question("hello world")
+
+
+def test_index_matches_question_across_runs_newest_first(tmp_path: Path) -> None:
+    _write_run(
+        tmp_path / "runA",
+        run_id="runA",
+        scenario_name="scnA",
+        finished_at="2026-01-01T00:00:00",
+        rows=[(_Q, 0.40)],
+    )
+    _write_run(
+        tmp_path / "runB",
+        run_id="runB",
+        scenario_name="scnB",
+        finished_at="2026-02-01T00:00:00",
+        rows=[(_Q, 0.80)],
+    )
+
+    index = build_question_history_index(extra_roots=[tmp_path])
+    entries = lookup(index, _Q)
+
+    assert [e["run_id"] for e in entries] == ["runB", "runA"]  # newest first
+    assert entries[0]["metrics"]["faithfulness"] == 0.80
+    assert entries[1]["scenario_name"] == "scnA"
+
+
+def test_index_excludes_current_run(tmp_path: Path) -> None:
+    _write_run(
+        tmp_path / "cur",
+        run_id="cur",
+        scenario_name="scn",
+        finished_at="2026-03-01T00:00:00",
+        rows=[(_Q, 0.50)],
+    )
+    _write_run(
+        tmp_path / "prev",
+        run_id="prev",
+        scenario_name="scn",
+        finished_at="2026-01-01T00:00:00",
+        rows=[(_Q, 0.60)],
+    )
+
+    index = build_question_history_index(exclude_run_id="cur", extra_roots=[tmp_path])
+    entries = lookup(index, _Q)
+
+    assert [e["run_id"] for e in entries] == ["prev"]
+
+
+def test_index_keeps_latest_occurrence_within_a_run(tmp_path: Path) -> None:
+    _write_run(
+        tmp_path / "run1",
+        run_id="run1",
+        scenario_name="scn",
+        finished_at="2026-01-01T00:00:00",
+        rows=[(_Q, 0.30), (_Q, 0.70)],  # same question twice
+    )
+
+    index = build_question_history_index(extra_roots=[tmp_path])
+    entries = lookup(index, _Q)
+
+    assert len(entries) == 1
+    assert entries[0]["metrics"]["faithfulness"] == 0.70  # last occurrence wins
+
+
+def test_lookup_caps_at_limit(tmp_path: Path) -> None:
+    for i in range(7):
+        _write_run(
+            tmp_path / f"r{i}",
+            run_id=f"r{i}",
+            scenario_name="scn",
+            finished_at=f"2026-01-0{i + 1}T00:00:00",
+            rows=[(_Q, 0.1 * i)],
+        )
+
+    index = build_question_history_index(extra_roots=[tmp_path])
+    assert len(lookup(index, _Q, limit=3)) == 3
+
+
+def test_lookup_unknown_question_returns_empty(tmp_path: Path) -> None:
+    index = build_question_history_index(extra_roots=[tmp_path])
+    assert lookup(index, "NO SUCH QUESTION zzz 0000") == []