fix(ci,llm): golden runner exits 0 when filing issues; workflow keeps the report on failure; cmd_run tests (refs #692)

.gitea/workflows/llm-golden.yml

@@ -21,6 +21,12 @@ jobs:
           UV_INSTALL_DIR: /usr/local/bin
 
       - name: Run golden longitudinal evaluation against the live chat
+        # golden_rc is captured (not left to `set -e`) so a failing golden
+        # run still docker-cps the report back and cats it — llm_golden.py
+        # itself exits 0 once --file-issues has filed the regressions
+        # (nb_issue_filer is the failure channel here), but this still
+        # guards the case where it exits nonzero before reaching that
+        # point (e.g. a crash while loading the golden set).
         env:
           GITEA_TOKEN: ${{ secrets.DEPLOY_TOKEN }}
         run: |
@@ -38,9 +44,10 @@ jobs:
               --url http://localhost:8000 \
               --set /tmp/golden/golden_lineage.yaml \
               --report /tmp/golden/report.json \
-              --file-issues --source nightly-llm-golden
+              --file-issues --source nightly-llm-golden || golden_rc=$?
           docker cp llm:/tmp/golden/report.json llm-golden-report.json
           cat llm-golden-report.json
+          exit "${golden_rc:-0}"
 
       - name: File failure issue
         if: failure()

dev/scripts/backends/gitea.py

@@ -673,6 +673,12 @@ jobs:
 {_setup_uv_step(uv_version)}
 
       - name: Run golden longitudinal evaluation against the live chat
+        # golden_rc is captured (not left to `set -e`) so a failing golden
+        # run still docker-cps the report back and cats it — llm_golden.py
+        # itself exits 0 once --file-issues has filed the regressions
+        # (nb_issue_filer is the failure channel here), but this still
+        # guards the case where it exits nonzero before reaching that
+        # point (e.g. a crash while loading the golden set).
         env:
           GITEA_TOKEN: ${{{{ secrets.DEPLOY_TOKEN }}}}
         run: |
@@ -690,9 +696,10 @@ jobs:
               --url http://localhost:8000 \\
               --set /tmp/golden/golden_lineage.yaml \\
               --report /tmp/golden/report.json \\
-              --file-issues --source nightly-llm-golden
+              --file-issues --source nightly-llm-golden || golden_rc=$?
           docker cp llm:/tmp/golden/report.json llm-golden-report.json
           cat llm-golden-report.json
+          exit "${{golden_rc:-0}}"
 
 {_failure_step("LLM Golden", "llm-golden")}
 """

dev/scripts/llm_golden.py

@@ -142,8 +142,14 @@ def check_anchors(transcript: Transcript, expect_anchors: list[dict]) -> CheckRe
         for s in sources:
             if s.get("item_key") != a["item_key"]:
                 continue
+            raw_pid = s.get("p_id")
+            # Explicit None/"" check, not truthiness (`or ""`) — a real FR
+            # paragraph anchor is never 0, but relying on `or` would treat
+            # a legitimate 0 the same as "missing" by accident.
+            if raw_pid is None or raw_pid == "":
+                continue
             try:
-                pid = int(s.get("p_id") or "")
+                pid = int(raw_pid)
             except (TypeError, ValueError):
                 continue
             if lo <= pid <= hi:
@@ -357,6 +363,12 @@ def cmd_run(args: argparse.Namespace) -> int:
             filer.signature(f["notebook"], f["ename"], f["evalue"]) for f in findings
         }
         filer.cmd_sweep(active, args.source)
+        # Ruling B9 / dev/scripts/nb_integration.py's return 0: issue filing
+        # is the failure channel for scheduled runs — the workflow's exit
+        # code must stay 0 so it can still docker-cp the report back and
+        # cat it, instead of the generic api.diag.ci failure filer firing
+        # and duplicating what nb_issue_filer already tracks per-signature.
+        return 0
 
     return 1 if any_fail else 0
 

tests/dev/test_gen_config_llm_golden.py

@@ -43,6 +43,34 @@ def test_gen_llm_golden_path_and_shape():
     assert "secrets.DEPLOY_TOKEN" in content
 
 
+def test_gen_llm_golden_captures_exit_code_and_copies_report_unconditionally():
+    """Ruling B9(b): the runner's exit code must not be left to `set -e`
+    (which would abort the step before the report is copied back and
+    cat'd) — it's captured via `|| golden_rc=$?`, the docker cp + cat of
+    the report run unconditionally after that, and the step's own exit
+    status is set explicitly from the captured code at the end."""
+    _, content = gitea._gen_llm_golden("ubuntu-latest", "latest")
+    lines = content.splitlines()
+
+    run_idx = next(i for i, l in enumerate(lines) if "llm_golden.py run" in l)
+    capture_idx = next(i for i, l in enumerate(lines) if "--file-issues --source" in l)
+    assert "|| golden_rc=$?" in lines[capture_idx]
+
+    cp_report_idx = next(
+        i for i, l in enumerate(lines) if "docker cp llm:/tmp/golden/report.json" in l
+    )
+    cat_idx = next(
+        i for i, l in enumerate(lines) if l.strip() == "cat llm-golden-report.json"
+    )
+    exit_idx = next(
+        i for i, l in enumerate(lines) if l.strip() == 'exit "${golden_rc:-0}"'
+    )
+
+    # The copy-back and cat come after the captured (not propagated)
+    # failure, and the explicit exit is the last line of the run block.
+    assert run_idx < capture_idx < cp_report_idx < cat_idx < exit_idx
+
+
 def test_gen_llm_golden_registered_in_emit():
     files = gitea.emit(
         [],

tests/llm/test_golden.py

@@ -11,7 +11,9 @@ LLM_CHAT_URL is set.
 
 from __future__ import annotations
 
+import argparse
 import importlib.util
+import json
 import os
 import sys
 from pathlib import Path
@@ -250,8 +252,134 @@ def test_load_set_rejects_entry_without_expectations(tmp_path):
         golden.load_set(bad)
 
 
+# ── cmd_run ──────────────────────────────────────────────────────
+#
+# Ruling B9: llm_golden.py's own /chat request is stubbed
+# (monkeypatch golden.stream_chat) so these exercise cmd_run's exit-code
+# and filer-wiring logic without a live service. "q1" always fails
+# check_labels ("FOO" never appears in its canned answer); "q2" always
+# passes (its answer contains "BAR").
+
+
+class _FakeFiler:
+    """Records cmd_report/cmd_sweep calls; signature mirrors the real
+    nb_issue_filer.signature's arity (notebook, ename, evalue) without
+    reimplementing its hashing."""
+
+    def __init__(self) -> None:
+        self.report_calls: list[tuple[list[dict], str]] = []
+        self.sweep_calls: list[tuple[set[str], str]] = []
+
+    def signature(self, notebook: str, ename: str, evalue: str) -> str:
+        return f"{notebook}|{ename}|{evalue}"
+
+    def cmd_report(self, findings: list[dict], source: str) -> int:
+        self.report_calls.append((findings, source))
+        return 0
+
+    def cmd_sweep(self, active_sigs: set[str], source: str) -> int:
+        self.sweep_calls.append((active_sigs, source))
+        return 0
+
+
+def _two_entry_set(tmp_path: Path) -> Path:
+    path = tmp_path / "two.yaml"
+    path.write_text(
+        "- id: q1\n  question: question one\n  expect_labels: ['FOO']\n"
+        "- id: q2\n  question: question two\n  expect_labels: ['BAR']\n"
+    )
+    return path
+
+
+def _fake_stream_chat(calls: list[str]):
+    def _stream(url: str, question: str, mode: str, timeout: float) -> list[dict]:
+        calls.append(question)
+        text = "BAR" if "two" in question else "nothing relevant here"
+        return [
+            {"type": "token", "text": text},
+            {"type": "sources", "sources": []},
+            {"type": "done"},
+        ]
+
+    return _stream
+
+
+def _run_args(set_path: Path, **overrides) -> argparse.Namespace:
+    base = dict(
+        url="http://fake",
+        set_path=str(set_path),
+        report=None,
+        file_issues=False,
+        source="test-source",
+        only=None,
+        timeout=30.0,
+    )
+    base.update(overrides)
+    return argparse.Namespace(**base)
+
+
+def test_cmd_run_exits_1_on_failure_without_file_issues(tmp_path, monkeypatch):
+    calls: list[str] = []
+    monkeypatch.setattr(golden, "stream_chat", _fake_stream_chat(calls))
+    rc = golden.cmd_run(_run_args(_two_entry_set(tmp_path)))
+    assert rc == 1
+    assert len(calls) == 2  # both entries ran
+
+
+def test_cmd_run_exits_0_with_file_issues_and_calls_filer(tmp_path, monkeypatch):
+    calls: list[str] = []
+    monkeypatch.setattr(golden, "stream_chat", _fake_stream_chat(calls))
+    fake_filer = _FakeFiler()
+    monkeypatch.setattr(golden, "_load_filer", lambda: fake_filer)
+
+    rc = golden.cmd_run(
+        _run_args(
+            _two_entry_set(tmp_path), file_issues=True, source="nightly-llm-golden"
+        )
+    )
+
+    assert rc == 0  # ruling B9(a) — filing is the failure channel
+    assert len(fake_filer.report_calls) == 1
+    findings, source = fake_filer.report_calls[0]
+    assert source == "nightly-llm-golden"
+    # Only q1's failing "labels" check becomes a finding — q2 passed.
+    assert [(f["notebook"], f["ename"]) for f in findings] == [("q1", "labels")]
+    assert len(fake_filer.sweep_calls) == 1
+    active_sigs, sweep_source = fake_filer.sweep_calls[0]
+    assert sweep_source == "nightly-llm-golden"
+    assert active_sigs == {fake_filer.signature("q1", "labels", findings[0]["evalue"])}
+
+
+def test_cmd_run_only_restricts_to_one_id(tmp_path, monkeypatch):
+    calls: list[str] = []
+    monkeypatch.setattr(golden, "stream_chat", _fake_stream_chat(calls))
+    rc = golden.cmd_run(_run_args(_two_entry_set(tmp_path), only="q2"))
+    assert rc == 0
+    assert calls == ["question two"]
+
+
+def test_cmd_run_writes_per_question_report(tmp_path, monkeypatch):
+    calls: list[str] = []
+    monkeypatch.setattr(golden, "stream_chat", _fake_stream_chat(calls))
+    report_path = tmp_path / "report.json"
+    golden.cmd_run(_run_args(_two_entry_set(tmp_path), report=str(report_path)))
+
+    report = json.loads(report_path.read_text())
+    assert report["pass"] == 1 and report["fail"] == 1
+    by_id = {row["id"]: row for row in report["rows"]}
+    assert by_id["q1"]["passed"] is False
+    assert by_id["q2"]["passed"] is True
+    assert [c["name"] for c in by_id["q1"]["checks"]] == ["labels"]
+
+
 # ── live (opt-in) ────────────────────────────────────────────────
 
+# Entries may still be red against live infra (Task 7 report, "live run"):
+# three known retrieval gaps as of #692 — g2058-replacement's YBM4IZUS
+# ¶1578 anchor, apcm-vs-ccm-elements's element anchors, and
+# 99490-telehealth-steps's anchors — are real corpus paragraphs the chat
+# doesn't yet surface as cited sources for these exact questions.
+
 
 @pytest.mark.skipif(
     not os.environ.get("LLM_CHAT_URL"),