Add diagnostics for OpenSwath benchmark failures

.github/workflows/benchmark.yml

@@ -774,6 +774,28 @@ jobs:
           name: benchmark-openswath-${{ inputs.runtime_source == 'package' && needs.package-runtime.outputs.openms_sha || needs.build-openms.outputs.openms_sha }}
           path: results-openswath
           if-no-files-found: error
+
+      # A failed "Run OpenSwath benchmark" step skips every later step (they
+      # default to success()), so the normalization and the results upload
+      # above would both be skipped and the evidence the failure needs would be
+      # lost with the runner. Capture it here instead. `failure() && !cancelled()`
+      # keeps the upload off cancelled runs; the explicit paths plus
+      # if-no-files-found: ignore mean a run that died before writing anything
+      # cannot turn one failure into two.
+      - name: Upload OpenSwath diagnostics on failure
+        if: ${{ failure() && !cancelled() }}
+        uses: actions/upload-artifact@v7
+        with:
+          name: benchmark-openswath-diagnostics-${{ inputs.runtime_source == 'package' && needs.package-runtime.outputs.openms_sha || needs.build-openms.outputs.openms_sha }}
+          path: |
+            results-openswath/openswath.json
+            results-openswath/output.featureXML
+            results-openswath/output.json
+            results-openswath/summary.txt
+            results-openswath/stages.tsv
+            results-openswath/meta.txt
+            results-openswath/logs/**
+          if-no-files-found: ignore
   # ---------------------------------------------------------------------------
   # ProSE + PeptDeep ONNX pipeline validation.
   #

benchmark/openswath_diagnostics.py

@@ -0,0 +1,145 @@
+#!/usr/bin/env python3
+"""Explain a failed OpenSwath benchmark report from openswath.json.
+
+The runner used to print only::
+
+    library join failed - see library_error in openswath.json
+
+which names the field but not its value, so a CI failure did not say *why* the
+TraML <-> featureXML join failed. This module renders that value, or states
+precisely why there is none (missing file, unreadable file, malformed JSON,
+absent or empty ``correctness.library_error``), and lists the non-passing
+stages and the artifacts the join consumed.
+
+Everything here is best-effort: the caller is already on a failure path, so
+``describe_failure`` never raises for bad input and ``main`` never changes the
+benchmark's exit status. Output is bounded - it never dumps the report itself.
+"""
+
+import json
+import os
+import sys
+
+#: A library_error is a short message from openswath_metrics, but clip anyway:
+#: a future error could embed a whole transition list, and the log is not a
+#: place for that.
+MAX_ERROR_CHARS = 2000
+
+#: Stages are already summarised in stages.tsv; the log only needs the ones
+#: that did not pass.
+MAX_STAGE_LINES = 8
+
+#: Files the join consumes or produces, reported as present/absent with size.
+ARTIFACTS = ("output.featureXML", "output.chrom.mzML", "output.json")
+
+
+def _clip(value):
+    if len(value) <= MAX_ERROR_CHARS:
+        return value
+    return "%s ... [%d more characters]" % (
+        value[:MAX_ERROR_CHARS],
+        len(value) - MAX_ERROR_CHARS,
+    )
+
+
+def _stage_lines(report):
+    """One line per stage that did not pass, bounded, required stages first."""
+    stages = report.get("stages")
+    if not isinstance(stages, list):
+        return []
+
+    failing = [stage for stage in stages if isinstance(stage, dict)
+               and stage.get("status") != "pass"]
+    failing.sort(key=lambda stage: stage.get("required") is not True)
+
+    lines = []
+    for stage in failing[:MAX_STAGE_LINES]:
+        lines.append(
+            "stage %s: status=%s exit_code=%s reason=%s"
+            % (
+                stage.get("name", "(unnamed)"),
+                stage.get("status", "(missing)"),
+                stage.get("exit_code", "(missing)"),
+                stage.get("reason") or "(none)",
+            )
+        )
+    if len(failing) > MAX_STAGE_LINES:
+        lines.append("... %d more non-passing stages" % (len(failing) - MAX_STAGE_LINES))
+    return lines
+
+
+def _artifact_lines(directory):
+    """Size of each result file, so a missing featureXML is visible at a glance."""
+    lines = []
+    for name in ARTIFACTS:
+        path = os.path.join(directory, name)
+        try:
+            size = os.path.getsize(path)
+        except OSError:
+            lines.append("artifact %s: missing" % name)
+        else:
+            lines.append("artifact %s: %d bytes" % (name, size))
+    return lines
+
+
+def describe_failure(json_path):
+    """Return bounded, human-readable lines describing why the run did not pass."""
+    try:
+        with open(json_path, "rb") as handle:
+            raw = handle.read()
+    except FileNotFoundError:
+        return ["openswath.json is missing: %s" % json_path]
+    except OSError as exc:
+        return ["openswath.json could not be read: %s" % exc]
+
+    try:
+        report = json.loads(raw)
+    except ValueError as exc:
+        # json.JSONDecodeError and UnicodeDecodeError are both ValueErrors.
+        return ["openswath.json is not valid JSON (%d bytes): %s" % (len(raw), exc)]
+
+    if not isinstance(report, dict):
+        return ["openswath.json is not a JSON object (got %s)" % type(report).__name__]
+
+    lines = []
+    verdict = report.get("verdict")
+    lines.append("verdict: %s" % (verdict if isinstance(verdict, str) else "(missing)"))
+
+    correctness = report.get("correctness")
+    if not isinstance(correctness, dict):
+        lines.append("library_error: (correctness section missing or not an object)")
+    elif "library_error" not in correctness:
+        lines.append("library_error: (key absent - the join did not report an error)")
+    else:
+        error = correctness["library_error"]
+        if not isinstance(error, str):
+            lines.append("library_error: (not a string: %s)" % type(error).__name__)
+        elif not error.strip():
+            lines.append("library_error: (empty string)")
+        else:
+            clipped = _clip(error)
+            if "\n" in clipped:
+                lines.append("library_error:")
+                lines.extend("  %s" % part for part in clipped.splitlines())
+            else:
+                lines.append("library_error: %s" % clipped)
+
+    lines.extend(_stage_lines(report))
+    lines.extend(_artifact_lines(os.path.dirname(os.path.abspath(json_path))))
+    return lines
+
+
+def main(argv=None):
+    argv = list(sys.argv[1:] if argv is None else argv)
+    if len(argv) != 1:
+        print("usage: openswath_diagnostics.py <openswath.json>", file=sys.stderr)
+        return 2
+
+    for line in describe_failure(argv[0]):
+        print(line)
+    # Diagnostics never change the benchmark's exit status.
+    return 0
+
+
+if __name__ == "__main__":
+    sys.exit(main())

benchmark/run_openswath_benchmark.sh

@@ -118,6 +118,14 @@ run_stage() {
   record_stage "${name}" "${required}" "${rc}" "${wall_s}" "${cpu_s}" "${peak_kb}" "${status}" "${reason}"
 }
 
+# Print why openswath.json did not pass, without changing the exit status.
+# Best effort: a missing interpreter, a missing file or malformed JSON must not
+# replace the benchmark failure with a diagnostic error.
+report_openswath_failure() {
+  python3 "${SCRIPT_DIR}/openswath_diagnostics.py" "${RESULTS_DIR}/openswath.json" 2>&1 \
+    | while IFS= read -r line; do log "  ${line}"; done
+}
+
 # --- checks ------------------------------------------------------------------
 
 echo "=== OpenMS benchmark: OpenSwath DIA ===" | tee "${RESULTS_DIR}/summary.txt"
@@ -304,7 +312,10 @@ if python3 -c 'import json,sys; sys.exit(0 if json.load(open(sys.argv[1]))["verd
   : # openswath.json verdict already combines stage + library-join outcomes
 else
   verdict="fail"
-  log "library join failed - see library_error in openswath.json"
+  log "openswath.json verdict is not pass - diagnostics:"
+  if ! report_openswath_failure; then
+    log "  (openswath diagnostics unavailable - see openswath.json)"
+  fi
 fi
 
 log ""

benchmark/test_openswath_diagnostics.py

@@ -0,0 +1,165 @@
+#!/usr/bin/env python3
+"""Tests for openswath_diagnostics.py (OpenSwath DIA failure diagnostics).
+
+Covers the cases the runner has to survive once openswath.json says "not pass":
+a real library_error, an empty one, a missing key, malformed JSON, a JSON value
+that is not an object, and a missing file. Also checks that the rendered output
+stays bounded, because it is written to a CI log.
+
+Run:  python3 benchmark/test_openswath_diagnostics.py
+"""
+
+import json
+import os
+import sys
+import tempfile
+
+HERE = os.path.dirname(os.path.abspath(__file__))
+sys.path.insert(0, HERE)
+
+import openswath_diagnostics as od  # noqa: E402
+
+
+def _write(tmp, report, name="openswath.json"):
+    path = os.path.join(tmp, name)
+    with open(path, "w") as handle:
+        if isinstance(report, str):
+            handle.write(report)
+        else:
+            json.dump(report, handle)
+    return path
+
+
+def _report(**correctness):
+    return {
+        "verdict": "fail",
+        "stages": [
+            {"name": "openswath_workflow", "required": True, "exit_code": 0,
+             "status": "pass", "reason": ""},
+            {"name": "join", "required": True, "exit_code": 1,
+             "status": "fail", "reason": "required stage failed"},
+        ],
+        "correctness": correctness,
+    }
+
+
+def _joined(lines):
+    return "\n".join(lines)
+
+
+def test_valid_library_error_is_reported():
+    with tempfile.TemporaryDirectory() as tmp:
+        message = (
+            "feature 1 references transition native_id 'group_Precursor_i0', "
+            "which the library does not define"
+        )
+        path = _write(tmp, _report(library_error=message))
+        out = _joined(od.describe_failure(path))
+
+        assert "verdict: fail" in out, out
+        assert ("library_error: " + message) in out, out
+        # the failing stage is listed, the passing one is not
+        assert "stage join:" in out, out
+        assert "stage openswath_workflow" not in out, out
+        # artifacts are reported even though none exist here
+        assert "artifact output.featureXML: missing" in out, out
+    print("PASS: a valid library_error is printed verbatim")
+
+
+def test_empty_library_error_is_named():
+    with tempfile.TemporaryDirectory() as tmp:
+        path = _write(tmp, _report(library_error="   "))
+        out = _joined(od.describe_failure(path))
+
+        assert "library_error: (empty string)" in out, out
+    print("PASS: an empty library_error is named instead of printed as blank")
+
+
+def test_missing_library_error_key_is_distinguished():
+    with tempfile.TemporaryDirectory() as tmp:
+        path = _write(tmp, _report(actual_features=6))
+        out = _joined(od.describe_failure(path))
+
+        assert "library_error: (key absent" in out, out
+        # the non-passing stage is still reported, so a pure stage failure is
+        # not mistaken for a silent join failure
+        assert "stage join:" in out, out
+    print("PASS: an absent library_error key does not look like an empty one")
+
+
+def test_malformed_json_does_not_raise():
+    with tempfile.TemporaryDirectory() as tmp:
+        path = _write(tmp, '{"verdict": "fail", "correctness": {')
+        lines = od.describe_failure(path)
+
+        assert len(lines) == 1, lines
+        assert "not valid JSON" in lines[0], lines
+        # no fragment of the file's content leaks into the log
+        assert "correctness" not in lines[0], lines
+    print("PASS: malformed JSON is reported without echoing its content")
+
+
+def test_non_object_json_is_reported():
+    with tempfile.TemporaryDirectory() as tmp:
+        path = _write(tmp, json.dumps(["fail"]))
+        lines = od.describe_failure(path)
+
+        assert "not a JSON object" in lines[0], lines
+        assert "list" in lines[0], lines
+    print("PASS: a JSON value that is not an object is reported")
+
+
+def test_missing_file_is_reported():
+    with tempfile.TemporaryDirectory() as tmp:
+        path = os.path.join(tmp, "absent.json")
+        lines = od.describe_failure(path)
+
+        assert len(lines) == 1, lines
+        assert "is missing" in lines[0], lines
+    print("PASS: a missing openswath.json is reported")
+
+
+def test_long_error_is_clipped():
+    with tempfile.TemporaryDirectory() as tmp:
+        path = _write(tmp, _report(library_error="x" * (od.MAX_ERROR_CHARS * 3)))
+        out = _joined(od.describe_failure(path))
+
+        assert "more characters]" in out, out
+        for line in od.describe_failure(path):
+            assert len(line) <= od.MAX_ERROR_CHARS + 200, len(line)
+    print("PASS: a long library_error is clipped to a bounded log line")
+
+
+def test_non_string_library_error_is_reported():
+    with tempfile.TemporaryDirectory() as tmp:
+        path = _write(tmp, _report(library_error={"detail": "nested"}))
+        out = _joined(od.describe_failure(path))
+
+        assert "library_error: (not a string: dict)" in out, out
+    print("PASS: a non-string library_error is reported by type, not dumped")
+
+
+def test_main_never_changes_exit_status():
+    with tempfile.TemporaryDirectory() as tmp:
+        missing = os.path.join(tmp, "absent.json")
+        assert od.main([missing]) == 0
+        # wrong usage is the only non-zero case, and it is not the failure path
+        assert od.main([]) == 2
+    print("PASS: main() returns 0 on a missing report (diagnostics never fail the run)")
+
+
+def main():
+    test_valid_library_error_is_reported()
+    test_empty_library_error_is_named()
+    test_missing_library_error_key_is_distinguished()
+    test_malformed_json_does_not_raise()
+    test_non_object_json_is_reported()
+    test_missing_file_is_reported()
+    test_long_error_is_clipped()
+    test_non_string_library_error_is_reported()
+    test_main_never_changes_exit_status()
+    print("\nAll openswath_diagnostics tests passed.")
+
+
+if __name__ == "__main__":
+    main()