Merge pull request #4 from microsoft-foundry/copilot/fix-quality-metrics-n-a

Fix compare_results quality metric parsing for judge-enabled runs

Author: leestott

scripts/compare_results.py

@@ -31,6 +31,9 @@ def _load_results(path: Path) -> dict:
         return json.load(f)
 
 
+_MISSING = object()
+
+
 def _safe_get(data: dict, *keys, default=None):
     """Safely traverse nested dict keys."""
     current = data
@@ -42,6 +45,31 @@ def _safe_get(data: dict, *keys, default=None):
     return current
 
 
+def _quality_metric(data: dict, *keys, default=None):
+    """Read a quality metric from nested results.json layouts."""
+    value = _safe_get(data, "quality", *keys, default=_MISSING)
+    if value is not _MISSING:
+        return value
+
+    quality = data.get("quality")
+    if not isinstance(quality, dict):
+        return default
+    if len(keys) == 1:
+        return quality.get(keys[0], default)
+    if keys[0] == "pairwise" and len(keys) == 2:
+        return quality.get(keys[1], default)
+
+    legacy_map = {
+        ("absolute_scores", "router_overall"): "router_mean_score",
+        ("absolute_scores", "baseline_overall"): "baseline_mean_score",
+    }
+    legacy_key = legacy_map.get(keys)
+    if legacy_key:
+        return quality.get(legacy_key, default)
+
+    return default
+
+
 def _fmt_delta(a, b, unit="", lower_is_better=True):
     """Format a delta with direction indicator."""
     if a is None or b is None:
@@ -98,11 +126,28 @@ def _add(category, metric, val_a, val_b, unit="", lower_is_better=True):
 
     # Quality
     for metric_key in ["router_win_rate", "baseline_win_rate", "tie_rate"]:
-        val_a = _safe_get(run_a, "quality", metric_key)
-        val_b = _safe_get(run_b, "quality", metric_key)
+        val_a = _quality_metric(run_a, "pairwise", metric_key)
+        val_b = _quality_metric(run_b, "pairwise", metric_key)
         better = metric_key == "router_win_rate"  # higher router wins is better
         _add("Quality", metric_key, val_a, val_b, "", lower_is_better=not better)
 
+    for metric_key in ["router_overall", "baseline_overall"]:
+        val_a = _quality_metric(run_a, "absolute_scores", metric_key)
+        val_b = _quality_metric(run_b, "absolute_scores", metric_key)
+        _add("Quality", metric_key, val_a, val_b, "", lower_is_better=False)
+
+    _cat_a = _safe_get(run_a, "quality", "win_rate_by_category")
+    cat_a = _cat_a if isinstance(_cat_a, dict) else {}
+    _cat_b = _safe_get(run_b, "quality", "win_rate_by_category")
+    cat_b = _cat_b if isinstance(_cat_b, dict) else {}
+    for category in sorted(set(cat_a) | set(cat_b)):
+        for metric_key in ["router_win_rate", "baseline_win_rate", "tie_rate"]:
+            val_a = _safe_get(cat_a.get(category, {}), metric_key)
+            val_b = _safe_get(cat_b.get(category, {}), metric_key)
+            better = metric_key == "router_win_rate"
+            _add("Quality by Category", f"{category} {metric_key}", val_a, val_b, "",
+                 lower_is_better=not better)
+
     # Requests
     for endpoint in ["model_router", "baseline"]:
         req_a = _safe_get(run_a, endpoint, "total_requests")
@@ -178,12 +223,12 @@ def _add(category, metric, val_a, val_b, unit="", lower_is_better=True):
     graders = foundry.get("grader_summary", {})
 
     # Quality: map local quality scores to Foundry graders
-    local_router_score = _safe_get(local, "quality", "router_mean_score")
+    local_router_score = _quality_metric(local, "absolute_scores", "router_overall")
     foundry_router_score = _safe_get(graders, "quality_absolute_router", "mean")
     _add("Quality", "router_absolute_score", local_router_score, foundry_router_score, "",
          lower_is_better=False)
 
-    local_baseline_score = _safe_get(local, "quality", "baseline_mean_score")
+    local_baseline_score = _quality_metric(local, "absolute_scores", "baseline_overall")
     foundry_baseline_score = _safe_get(graders, "quality_absolute_baseline", "mean")
     _add("Quality", "baseline_absolute_score", local_baseline_score, foundry_baseline_score, "",
          lower_is_better=False)

tests/test_compare_results.py

@@ -0,0 +1,117 @@
+"""Regression tests for scripts.compare_results."""
+
+from __future__ import annotations
+
+import importlib.util
+from pathlib import Path
+
+
+def _load_compare_results_module():
+    script_path = Path(__file__).resolve().parents[1] / "scripts" / "compare_results.py"
+    spec = importlib.util.spec_from_file_location("compare_results", script_path)
+    assert spec is not None
+    module = importlib.util.module_from_spec(spec)
+    assert spec.loader is not None
+    spec.loader.exec_module(module)
+    return module
+
+
+def _row_map(rows):
+    return {(row["category"], row["metric"]): row for row in rows}
+
+
+def _base_run():
+    return {
+        "model_router": {
+            "total_requests": 10,
+            "latency": {"mean_ms": 100.0, "p90_ms": 120.0, "p99_ms": 140.0},
+            "cost": {"estimated_cost_usd": 1.0},
+        },
+        "baseline": {
+            "total_requests": 10,
+            "latency": {"mean_ms": 150.0, "p90_ms": 180.0, "p99_ms": 210.0},
+            "cost": {"estimated_cost_usd": 2.0},
+        },
+    }
+
+
+def test_compare_reads_nested_quality_metrics():
+    mod = _load_compare_results_module()
+    run_a = _base_run() | {
+        "quality": {
+            "pairwise": {
+                "router_win_rate": 0.4,
+                "baseline_win_rate": 0.2,
+                "tie_rate": 0.4,
+            },
+            "absolute_scores": {
+                "router_overall": 4.2,
+                "baseline_overall": 3.8,
+            },
+            "win_rate_by_category": {
+                "math": {
+                    "router_win_rate": 1.0,
+                    "baseline_win_rate": 0.0,
+                    "tie_rate": 0.0,
+                },
+            },
+        },
+    }
+    run_b = _base_run() | {
+        "quality": {
+            "pairwise": {
+                "router_win_rate": 0.5,
+                "baseline_win_rate": 0.3,
+                "tie_rate": 0.2,
+            },
+            "absolute_scores": {
+                "router_overall": 4.5,
+                "baseline_overall": 4.0,
+            },
+            "win_rate_by_category": {
+                "math": {
+                    "router_win_rate": 0.5,
+                    "baseline_win_rate": 0.5,
+                    "tie_rate": 0.0,
+                },
+            },
+        },
+    }
+
+    rows = _row_map(mod.compare(run_a, run_b, "run-a", "run-b"))
+
+    assert rows[("Quality", "router_win_rate")]["run_a"] == 0.4
+    assert rows[("Quality", "router_win_rate")]["run_b"] == 0.5
+    assert rows[("Quality", "baseline_overall")]["run_a"] == 3.8
+    assert rows[("Quality", "router_overall")]["run_b"] == 4.5
+    assert rows[("Quality by Category", "math router_win_rate")]["run_a"] == 1.0
+    assert rows[("Quality by Category", "math baseline_win_rate")]["run_b"] == 0.5
+
+
+def test_compare_keeps_legacy_flat_quality_metrics():
+    mod = _load_compare_results_module()
+    run_a = _base_run() | {
+        "quality": {
+            "router_win_rate": 0.4,
+            "baseline_win_rate": 0.2,
+            "tie_rate": 0.4,
+            "router_mean_score": 4.1,
+            "baseline_mean_score": 3.7,
+        },
+    }
+    run_b = _base_run() | {
+        "quality": {
+            "router_win_rate": 0.6,
+            "baseline_win_rate": 0.1,
+            "tie_rate": 0.3,
+            "router_mean_score": 4.4,
+            "baseline_mean_score": 3.9,
+        },
+    }
+
+    rows = _row_map(mod.compare(run_a, run_b, "run-a", "run-b"))
+
+    assert rows[("Quality", "router_win_rate")]["run_a"] == 0.4
+    assert rows[("Quality", "baseline_win_rate")]["run_b"] == 0.1
+    assert rows[("Quality", "router_overall")]["run_a"] == 4.1
+    assert rows[("Quality", "baseline_overall")]["run_b"] == 3.9