[nej1gotnochill]

Description

Diagnostic-only change: make the next failed package-mode OpenSwath run self-diagnosing.

The package-mode OpenSwath benchmark has repeatedly failed in CI with only the message
library join failed - see library_error in openswath.json — the log named the field but never
printed its value, and the workflow's failure path skipped every upload step, so neither
openswath.json nor the actual output.featureXML was preserved for inspection. That recurring
library-join failure is the entire motivation for this PR.

The actual cause of the failure remains unconfirmed. The suspected precursor-ID mismatch —
real OpenSwath output containing MS1 precursor subordinate features whose native_id values such
as <group>_Precursor_i0 are not TraML transition IDs — is a hypothesis, not a proven root
cause. We have not inspected the actual output.featureXML from a failing CI run; the committed
synthetic fixture passes and does not cover this case. This PR therefore does not change the
parser's join semantics, weaken any correctness check, or convert any failure into a pass — it only
collects evidence.

Changes:

1. benchmark/run_openswath_benchmark.sh — when the verdict read back from openswath.json
   is not pass, a new report_openswath_failure helper runs benchmark/openswath_diagnostics.py
   against the result file and prints its output into summary.txt. The call is best-effort: a
   missing interpreter, missing file, malformed JSON, or any diagnostic error is caught and
   reported as (openswath diagnostics unavailable - see openswath.json) without altering the
   verdict or the non-zero exit status. The existing correctness verdicts and failure exit
   behavior are preserved unchanged.

2. benchmark/openswath_diagnostics.py (new) — bounded, never-raising renderer of the failure
   state: the verdict, the actual correctness.library_error value (clipped, multi-line aware),
   or precisely why there is none (key absent / empty / non-string / malformed JSON / missing
   file / non-object JSON), plus non-passing stages and present/absent sizes of the join-relevant
   artifacts. It never dumps the report itself.

3. .github/workflows/benchmark.yml — a new final step in the run-openswath-benchmark job,
   if: ${{ failure() && !cancelled() }}, uploads openswath.json, output.featureXML,
   output.json, summary.txt, stages.tsv, meta.txt, and logs/** as
   benchmark-openswath-diagnostics-<sha> with if-no-files-found: ignore, so evidence survives
   a failed benchmark step and a run that died before writing anything cannot cause a secondary
   error to mask the original failure.

4. Tests (new) — benchmark/test_openswath_diagnostics.py (unit tests for the renderer) and
   benchmark/test_openswath_diagnostics.sh (extracts the shipped runner functions and drives
   valid / empty / absent / malformed / missing-file cases plus the broken-diagnostics path).

Local validation: bash -n on both shell scripts, workflow YAML parse + structural checks,
python benchmark/test_openswath_diagnostics.py (9/9), bash benchmark/test_openswath_diagnostics.sh
(9/9), and the pre-existing benchmark/test_openswath_metrics.py regression suite all passed
locally. Not yet verified by a real GitHub Actions run: the failure() && !cancelled()
artifact-upload path has been validated only by YAML parsing and structural reasoning — the first
actual failed package-mode run after this merges is its real test.

No inputs, thresholds, verdict rules, package selection, or unrelated workflow logic were touched;
benchmark/openswath_metrics.py is unchanged.

Next step after merge: re-run the workflow in package mode and inspect the real library_error
and output.featureXML before deciding whether the precursor-ID hypothesis is confirmed. This PR
deliberately stops at evidence collection.

Checklist

• Make sure that you are listed in the AUTHORS file
  (N/A: this repository has no AUTHORS file)
• Add relevant changes and new features to the CHANGELOG file
  (N/A: this repository has no CHANGELOG file)
• I have commented my code, particularly in hard-to-understand areas
  (the workflow if: condition and the best-effort failure path carry explanatory comments)
• New and existing unit tests pass locally with my changes
  (test_openswath_diagnostics.py, test_openswath_diagnostics.sh, and the pre-existing
  test_openswath_metrics.py all pass; bash -n and YAML parse clean)
• Updated or added python bindings for changed or new classes
  → Tick: no C++ classes changed; benchmark-only Python.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing.

[coderabbitai]

Important

• 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration

• Configuration used: Organization UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: 25b0ad19-b26c-4626-aa62-f01904dd6e5a

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[nej1gotnochill]

This PR is intentionally diagnostic only. The recurring package-mode OpenSwath failure currently reports library join failed without exposing the underlying library_error, and the relevant artifacts are not preserved when the benchmark step fails.

This change makes the error visible and uploads the relevant diagnostic files on failure, without changing parser behavior, correctness checks, or verdict rules.

The suspected precursor-ID mismatch remains a hypothesis; I have not yet inspected the actual failing output.featureXML. The next step is to capture that evidence from a real package-mode run before proposing any parser changes.

Local tests and validation passed. The failure-path artifact upload has not yet been verified in a real GitHub Actions run.

[nej1gotnochill]

Update on the same @jpfeuffer and Timo : I validated this PR with a real package mode run using the PR branch and commit 5e2558ad41f491d6ea5eadb0a68b187fb400d63e.

Run: https://github.com/OpenMS/OpenMS-benchmarking/actions/runs/39891350513

The OpenSwathWorkflow stage completed successfully, but the benchmark correctly failed at the library-join check. The new failure-path upload step ran successfully and preserved openswath.json and the actual output.featureXML in artifact benchmark-openswath-diagnostics-8ab290cc80e1150986b37530ac02dee6d8d6ce58.

The reported error was:
feature f_11267478275115286042 references transition native_id '0', which the library does not define

The captured output contains transition native IDs 0, 1, and 2, while the TraML transition IDs are 1–18. This points to an ID contract mismatch rather than the previously suspected precursorID failure. I have not changed parser behavior or correctness checks. The diagnostic and artifact-upload path is now verified on a real failing run. Could you review the diagnostic only PR independently of the follow up investigation into the OpenMS output/library ID mismatch?