Repository navigation
Fix nightly benchmark run discovery - #12
Merged
Merged
Conversation
library_coverage was identified_peptides / library_peptides, where the denominator counted every declared TraML <Peptide> including ones carrying no transitions. Such a peptide has no precursor, so no transition and no native_id a feature could reference: it can never be identified. Including it capped coverage at a ceiling fixed by library construction rather than by the tool, so a flawless run reported 0.857143 instead of 1.0. On the reference fixture the run is already perfect on every other axis (18/18 transitions, 6/6 precursor m/z, 6/6 peptides); the only unreachable peptide is PEPTIDEA_Extra, which the vendored upstream test library declares without transitions. Keep library_peptides as the full declared count so such an orphan stays visible, add library_peptides_identifiable and library_peptides_orphan to report the split explicitly, and divide coverage by the identifiable subset. No existing metric key is removed or renamed. BREAKING FOR TRENDS: this changes the MEANING of library_coverage, not just its accuracy. Historical values are identified/library_peptides and are not comparable with values recorded after this change. Runs recorded either side of the change will plot as one series with a step at the change point. Also render coverage as a percentage rather than a bare scalar, since it is a fraction of a population and was previously indistinguishable from any other sub-10 metric, and replace a renderer assertion that compared against a live _fmt_metric() call with a literal expected rendering so it can actually fail.
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configuration
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. Comment |
…dentifiable-denominator
jpfeuffer
approved these changes
Oct 11, 2026
Collaborator
Author
|
Thank you for merging this @jpfeuffer , really appreciate the same (; ! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Summary
gh run list --head_shaoption with--commit.Root cause
The nightly workflow used an unsupported
gh run listflag. The command failed on each attempt, but|| truesuppressed the error. The discovery loop consequently ran for approximately 20 minutes before reporting that no benchmark run appeared.Validation
actionlint -ignore SC .github/workflows/nightly-benchmark.ymlpassed.bash -nchecks on the relevant extracted shell blocks passed.git diff --checkpassed.These results were established during the prior review. No live GitHub CLI query or end-to-end workflow run was performed.
Known limitations
mainadvances between resolvingMAIN_SHAand dispatching againstref=main.gh run viewerror handling and job-timeout budgeting were outside this patch's scope.Scope
Only
.github/workflows/nightly-benchmark.ymlis changed. No benchmark implementation, historical results, F5 metrics, or Pages configuration is modified.Checklist
How can I get additional information on failed tests during CI
Click to expand
If your PR is failing you can check out - The details of the action statuses at the end of the PR or the "Checks" tab. - http://cdash.seqan.de/index.php?project=OpenMS and look for your PR. Use the "Show filters" capability on the top right to search for your PR number. If you click in the column that lists the failed tests you will get detailed error messages.Click to expand
- `/reformat` (experimental) applies the clang-format style changes as additional commit. Note: your branch must have a different name (e.g., yourrepo:feature/XYZ) than the receiving branch (e.g., OpenMS:develop). Otherwise, reformat fails to push.