fix(scripts,review): gate skipped-cell rendering on status != "ok"

Addresses both Copilot inline comments on PR #235.

The previous fix used `if head_status or base_status:` to detect skipped
matrix cells, but `benchmarks/benchmark.py::MatrixCell.status` defaults
to `"ok"` for every real cell — making `"ok"` truthy and causing the
delta script to render `_skipped_ (ok)` for every backend/size cell.
The benchmark-delta bot comment on this PR caught the bug live: all 9
real cells (bm25/fuzzy/tfidf × 100/500/1000) were marked as skipped.

The gate is now `status != "ok"`, mirroring the existing convention in
`scripts/render_scorecard.py:150-151`. Cells with no `status` field are
treated as "ok" for backwards compat.

Test updates:
- `test_matrix_delta_ok_cells_render_metrics` (renamed from
  `test_matrix_delta_real_cells_still_render_metrics`) now explicitly
  sets `status: "ok"` on the fixtures and asserts metrics render — this
  is the regression test that would have caught the original bug.
- `test_matrix_delta_missing_status_treated_as_ok` added for backwards
  compatibility on cells that omit `status` entirely.

Verification:
- ruff format/lint/mypy clean
- pytest -q: 1012 passed, 5 skipped (one more test than before the fix)

Author: claude

scripts/benchmark_delta.py

@@ -137,13 +137,16 @@ def _render_matrix_section(base: dict[str, Any], head: dict[str, Any]) -> str:
         b = base_idx.get((backend, size), {})
         h = head_idx.get((backend, size), b)
         # Skip-cells carry zeroed metrics by design (e.g. fuzzy with no
-        # rapidfuzz). Treating them as accuracy/latency regressions would
-        # produce false-positive ⚠️ markers on every PR; emit a single
-        # "skipped" row that surfaces the reason text instead.
-        head_status = str(h.get("status", ""))
-        base_status = str(b.get("status", ""))
-        if head_status or base_status:
-            reason = head_status or base_status
+        # rapidfuzz, status="skipped: ..."). Treating them as accuracy/
+        # latency regressions would produce false-positive ⚠️ markers on
+        # every PR; emit a single "skipped" row that surfaces the reason
+        # instead. Real cells carry status="ok" (the MatrixCell default in
+        # benchmarks/benchmark.py), so the gate is status != "ok",
+        # aligning with scripts/render_scorecard.py's existing convention.
+        head_status = str(h.get("status", "ok"))
+        base_status = str(b.get("status", "ok"))
+        if head_status != "ok" or base_status != "ok":
+            reason = head_status if head_status != "ok" else base_status
             lines.append(f"| {backend} | {size} | _skipped_ ({reason}) | — | — |")
             continue
         br = float(b.get("recall_at_k", 0.0))

tests/test_benchmark_delta.py

@@ -52,8 +52,16 @@ def test_matrix_delta_renders_skipped_row_for_status_bearing_cell() -> None:
     assert "⚠️" not in out
 
 
-def test_matrix_delta_real_cells_still_render_metrics() -> None:
-    """Status-less cells still produce a full recall/MRR/latency row."""
+def test_matrix_delta_ok_cells_render_metrics() -> None:
+    """Cells with the canonical ``status="ok"`` must render full metrics.
+
+    Regression: an earlier version of `_render_matrix_section` checked
+    `if head_status or base_status`, which matched every real cell
+    (``MatrixCell.status`` defaults to ``"ok"``) and suppressed every
+    metric row. The benchmark-delta bot comment on PR #235 caught this
+    in production. The gate is now `status != "ok"`, aligned with
+    `scripts/render_scorecard.py`.
+    """
     base = _payload(
         [
             {
@@ -62,6 +70,7 @@ def test_matrix_delta_real_cells_still_render_metrics() -> None:
                 "recall_at_k": 0.5,
                 "mrr": 0.4,
                 "latency_ms_p99": 1.0,
+                "status": "ok",
             }
         ]
     )
@@ -73,15 +82,45 @@ def test_matrix_delta_real_cells_still_render_metrics() -> None:
                 "recall_at_k": 0.5,
                 "mrr": 0.4,
                 "latency_ms_p99": 1.0,
+                "status": "ok",
             }
         ]
     )
     out = _render_matrix_section(base, head)
-    # Numeric markers present, no skipped marker, no regression marker.
+    # Numeric markers present; no skipped marker for "ok" cells.
     assert "_skipped_" not in out
     assert "0.5000" in out
 
 
+def test_matrix_delta_missing_status_treated_as_ok() -> None:
+    """Backwards-compatibility: a cell with no `status` field still renders metrics."""
+    base = _payload(
+        [
+            {
+                "backend": "bm25",
+                "catalog_size": 100,
+                "recall_at_k": 0.3,
+                "mrr": 0.2,
+                "latency_ms_p99": 5.0,
+            }
+        ]
+    )
+    head = _payload(
+        [
+            {
+                "backend": "bm25",
+                "catalog_size": 100,
+                "recall_at_k": 0.3,
+                "mrr": 0.2,
+                "latency_ms_p99": 5.0,
+            }
+        ]
+    )
+    out = _render_matrix_section(base, head)
+    assert "_skipped_" not in out
+    assert "0.3000" in out
+
+
 def test_matrix_delta_empty_payload_returns_empty_string() -> None:
     """No matrix rows in either side → no section emitted."""
     assert _render_matrix_section({}, {}) == ""