From b393fc12c0b935b1669caba51d9ba81590d316d8 Mon Sep 17 00:00:00 2001 From: Eduard Kerkhoven Date: Fri, 17 Jul 2026 05:42:08 +0000 Subject: [PATCH 1/3] fix: retry transient GitHub API errors when posting the QC comment The full MEMOTE run does its real work (running the suite, committing the scores) hours before it posts the Model QC comment, but the final github-script step made unguarded listComments/updateComment/createComment calls. A transient 503 from the GitHub API on that last call raised an unhandled HttpError and failed the whole run even though the results were already produced and committed. Wrap those calls in an exponential-backoff retry that only retries transient errors (429 and 5xx), so an API blip on the cosmetic comment update can no longer sink a multi-hour run. --- .github/actions/post-qc-comment/action.yml | 31 +++++++++++++++++++--- 1 file changed, 28 insertions(+), 3 deletions(-) diff --git a/.github/actions/post-qc-comment/action.yml b/.github/actions/post-qc-comment/action.yml index fae13a5d..e85d7b1c 100644 --- a/.github/actions/post-qc-comment/action.yml +++ b/.github/actions/post-qc-comment/action.yml @@ -63,10 +63,35 @@ runs: const body = `${marker}\n${report}${footer}`; const { owner, repo } = context.repo; const issue_number = process.env.ISSUE_NUMBER ? Number(process.env.ISSUE_NUMBER) : context.issue.number; - const comments = await github.paginate(github.rest.issues.listComments, { owner, repo, issue_number }); + + // The GitHub API occasionally returns a transient 5xx (e.g. a 503 "Unicorn" + // page) or a secondary-rate-limit 429. This hours-long run has already done + // its real work by the time it posts, so a blip here must not fail it: retry + // transient errors with exponential backoff before giving up. + const sleep = ms => new Promise(resolve => setTimeout(resolve, ms)); + async function withRetry(label, fn) { + const maxAttempts = 5; + for (let attempt = 1; ; attempt++) { + try { + return await fn(); + } catch (error) { + const status = error.status || (error.response && error.response.status); + const transient = status === 429 || (status >= 500 && status < 600); + if (!transient || attempt >= maxAttempts) throw error; + const delay = Math.min(2000 * 2 ** (attempt - 1), 30000); + core.warning(`${label} failed with ${status || error.message}; retrying in ${delay} ms (attempt ${attempt}/${maxAttempts - 1})`); + await sleep(delay); + } + } + } + + const comments = await withRetry('listComments', () => + github.paginate(github.rest.issues.listComments, { owner, repo, issue_number })); const existing = comments.find(c => c.body && c.body.includes(marker)); if (existing) { - await github.rest.issues.updateComment({ owner, repo, comment_id: existing.id, body }); + await withRetry('updateComment', () => + github.rest.issues.updateComment({ owner, repo, comment_id: existing.id, body })); } else { - await github.rest.issues.createComment({ owner, repo, issue_number, body }); + await withRetry('createComment', () => + github.rest.issues.createComment({ owner, repo, issue_number, body })); } From 66bfdbbb33abb6ce028a67420f6214b6ec22980a Mon Sep 17 00:00:00 2001 From: edkerk <7326655+edkerk@users.noreply.github.com> Date: Fri, 17 Jul 2026 06:37:38 +0000 Subject: [PATCH 2/3] chore: update model QC results [skip ci] --- data/testResults/README.md | 10 ++-- data/testResults/model_qc_summary.md | 72 ++++++++++++++-------------- 2 files changed, 41 insertions(+), 41 deletions(-) diff --git a/data/testResults/README.md b/data/testResults/README.md index 6ed6d67d..b3f0ff13 100644 --- a/data/testResults/README.md +++ b/data/testResults/README.md @@ -21,11 +21,11 @@ own files. The pull request in each row is the one whose run last wrote those fi | Result file(s) | Produced by | Last updated by | | --- | --- | --- | -| `qc_duplicate_keys.csv`, `qc_empty_reactions.csv`, `qc_annotation_consistency.csv`, `qc_deprecation_completeness.csv`, `qc_metabolite_completeness.csv`, `qc_reaction_sanity.csv`, `qc_duplicate_reactions.csv`, `qc_unused_entities.csv`, `qc_growth_blockers.csv` | `qcModelChecks.py` | **PR #1061** (model QC checks) | -| `qc_annotation_issues.csv` | `annotationTest.py` | **PR #1061** (model QC checks) | -| `qc_status.tsv` (round-trip, YAML lint, metabolic tasks, growth) | `testYamlConversion.py`, `testMetabolicTasks.py`, `action-yamllint`, `qcModelChecks.py` (via `qcStatus.py`) | **PR #1061** (model QC checks) | -| `macaw_results.csv`, `balance_results.csv`, `qc_structure_consistency.csv` | `macawTests.py`, `balanceTest.py`, `structureConsistencyTest.py` | **PR #1061** (MACAW and balance) | -| `memote_score.md` | `memoteSnapshot.py` (fast subset every PR; full suite via `/run memote`) | **PR #1061** (MEMOTE) | +| `qc_duplicate_keys.csv`, `qc_empty_reactions.csv`, `qc_annotation_consistency.csv`, `qc_deprecation_completeness.csv`, `qc_metabolite_completeness.csv`, `qc_reaction_sanity.csv`, `qc_duplicate_reactions.csv`, `qc_unused_entities.csv`, `qc_growth_blockers.csv` | `qcModelChecks.py` | **PR #1067** (model QC checks) | +| `qc_annotation_issues.csv` | `annotationTest.py` | **PR #1067** (model QC checks) | +| `qc_status.tsv` (round-trip, YAML lint, metabolic tasks, growth) | `testYamlConversion.py`, `testMetabolicTasks.py`, `action-yamllint`, `qcModelChecks.py` (via `qcStatus.py`) | **PR #1067** (model QC checks) | +| `macaw_results.csv`, `balance_results.csv`, `qc_structure_consistency.csv` | `macawTests.py`, `balanceTest.py`, `structureConsistencyTest.py` | **PR #1067** (MACAW and balance) | +| `memote_score.md` | `memoteSnapshot.py` (fast subset every PR; full suite via `/run memote`) | **PR #1067** (MEMOTE) | | `gene-essential.csv`, `gene-essential_summary.md` | `geneEssentiality.py` via `/run gene-essentiality` | **PR #1027** (gene essentiality) | ## 2. What each check means diff --git a/data/testResults/model_qc_summary.md b/data/testResults/model_qc_summary.md index bac34694..6f81a432 100644 --- a/data/testResults/model_qc_summary.md +++ b/data/testResults/model_qc_summary.md @@ -1,59 +1,59 @@ ## Model quality report -:warning: **6 pre-existing finding(s), no regressions vs `main`.** Non-blocking. +:warning: **6 pre-existing finding(s), no regressions vs `develop`.** Non-blocking. -_Each check name links to its explanation in the [testResults README](https://github.com/SysBioChalmers/Human-GEM/blob/develop/data/testResults/README.md)._ +_Each check name links to its explanation in the [testResults README](https://github.com/SysBioChalmers/Human-GEM/blob/fix/qc-comment-transient-retry/data/testResults/README.md)._ ### Model checks _Duplicate keys (model unloadable) and no growth block the merge; every other row is a non-blocking report._ -| Check | Result | Δ vs `main` | | +| Check | Result | Δ vs `develop` | | | --- | ---: | ---: | :---: | -| [Duplicate `!!omap` keys](https://github.com/SysBioChalmers/Human-GEM/blob/develop/data/testResults/README.md#duplicate-omap-keys) | 0 | new | :white_check_mark: | -| [Growth (biomass producible)](https://github.com/SysBioChalmers/Human-GEM/blob/develop/data/testResults/README.md#growth-biomass-producible) | 125 | new | :white_check_mark: | -| [Reactions with no metabolites](https://github.com/SysBioChalmers/Human-GEM/blob/develop/data/testResults/README.md#reactions-with-no-metabolites) | 0 | new | :white_check_mark: | -| [Model / annotation-table inconsistencies](https://github.com/SysBioChalmers/Human-GEM/blob/develop/data/testResults/README.md#model--annotation-table-inconsistencies) | 0 | new | :white_check_mark: | -| [Removed reactions or metabolites not deprecated](https://github.com/SysBioChalmers/Human-GEM/blob/develop/data/testResults/README.md#removed-reactions-or-metabolites-not-deprecated) | 0 | new | :white_check_mark: | -| [Metabolites missing formula](https://github.com/SysBioChalmers/Human-GEM/blob/develop/data/testResults/README.md#metabolites-missing-formula) | 0 | new | :white_check_mark: | -| [Metabolites missing charge](https://github.com/SysBioChalmers/Human-GEM/blob/develop/data/testResults/README.md#metabolites-missing-charge) | 0 | new | :white_check_mark: | -| [Reaction bound / GPR issues](https://github.com/SysBioChalmers/Human-GEM/blob/develop/data/testResults/README.md#reaction-bound--gpr-issues) | 0 | new | :white_check_mark: | -| [Exact-duplicate reaction groups](https://github.com/SysBioChalmers/Human-GEM/blob/develop/data/testResults/README.md#exact-duplicate-reaction-groups) | 0 | new | :white_check_mark: | -| [Unused metabolites](https://github.com/SysBioChalmers/Human-GEM/blob/develop/data/testResults/README.md#unused-metabolites) | 0 | new | :white_check_mark: | -| [Unused genes](https://github.com/SysBioChalmers/Human-GEM/blob/develop/data/testResults/README.md#unused-genes) | 0 | new | :white_check_mark: | -| [Malformed cross-references](https://github.com/SysBioChalmers/Human-GEM/blob/develop/data/testResults/README.md#malformed-cross-references) | 0 | new | :white_check_mark: | -| [Cross-refs inconsistent across compartments](https://github.com/SysBioChalmers/Human-GEM/blob/develop/data/testResults/README.md#cross-refs-inconsistent-across-compartments) | [3](https://github.com/SysBioChalmers/Human-GEM/blob/develop/data/testResults/qc_annotation_issues.csv) | new | :warning: | +| [Duplicate `!!omap` keys](https://github.com/SysBioChalmers/Human-GEM/blob/fix/qc-comment-transient-retry/data/testResults/README.md#duplicate-omap-keys) | 0 | 0 | :white_check_mark: | +| [Growth (biomass producible)](https://github.com/SysBioChalmers/Human-GEM/blob/fix/qc-comment-transient-retry/data/testResults/README.md#growth-biomass-producible) | 125 | 0 | :white_check_mark: | +| [Reactions with no metabolites](https://github.com/SysBioChalmers/Human-GEM/blob/fix/qc-comment-transient-retry/data/testResults/README.md#reactions-with-no-metabolites) | 0 | 0 | :white_check_mark: | +| [Model / annotation-table inconsistencies](https://github.com/SysBioChalmers/Human-GEM/blob/fix/qc-comment-transient-retry/data/testResults/README.md#model--annotation-table-inconsistencies) | 0 | 0 | :white_check_mark: | +| [Removed reactions or metabolites not deprecated](https://github.com/SysBioChalmers/Human-GEM/blob/fix/qc-comment-transient-retry/data/testResults/README.md#removed-reactions-or-metabolites-not-deprecated) | 0 | 0 | :white_check_mark: | +| [Metabolites missing formula](https://github.com/SysBioChalmers/Human-GEM/blob/fix/qc-comment-transient-retry/data/testResults/README.md#metabolites-missing-formula) | 0 | 0 | :white_check_mark: | +| [Metabolites missing charge](https://github.com/SysBioChalmers/Human-GEM/blob/fix/qc-comment-transient-retry/data/testResults/README.md#metabolites-missing-charge) | 0 | 0 | :white_check_mark: | +| [Reaction bound / GPR issues](https://github.com/SysBioChalmers/Human-GEM/blob/fix/qc-comment-transient-retry/data/testResults/README.md#reaction-bound--gpr-issues) | 0 | 0 | :white_check_mark: | +| [Exact-duplicate reaction groups](https://github.com/SysBioChalmers/Human-GEM/blob/fix/qc-comment-transient-retry/data/testResults/README.md#exact-duplicate-reaction-groups) | 0 | 0 | :white_check_mark: | +| [Unused metabolites](https://github.com/SysBioChalmers/Human-GEM/blob/fix/qc-comment-transient-retry/data/testResults/README.md#unused-metabolites) | 0 | 0 | :white_check_mark: | +| [Unused genes](https://github.com/SysBioChalmers/Human-GEM/blob/fix/qc-comment-transient-retry/data/testResults/README.md#unused-genes) | 0 | 0 | :white_check_mark: | +| [Malformed cross-references](https://github.com/SysBioChalmers/Human-GEM/blob/fix/qc-comment-transient-retry/data/testResults/README.md#malformed-cross-references) | 0 | 0 | :white_check_mark: | +| [Cross-refs inconsistent across compartments](https://github.com/SysBioChalmers/Human-GEM/blob/fix/qc-comment-transient-retry/data/testResults/README.md#cross-refs-inconsistent-across-compartments) | [3](https://github.com/SysBioChalmers/Human-GEM/blob/fix/qc-comment-transient-retry/data/testResults/qc_annotation_issues.csv) | 0 | :warning: | ### MACAW and mass/charge balance -| Check | Result | Δ vs `main` | | +| Check | Result | Δ vs `develop` | | | --- | ---: | ---: | :---: | -| [Reactions flagged by MACAW dead-end test](https://github.com/SysBioChalmers/Human-GEM/blob/develop/data/testResults/README.md#reactions-flagged-by-macaw-dead-end-test) | [2510](https://github.com/SysBioChalmers/Human-GEM/blob/develop/data/testResults/macaw_results.csv) | -703 | :warning: | -| [Reactions flagged as MACAW duplicates](https://github.com/SysBioChalmers/Human-GEM/blob/develop/data/testResults/README.md#reactions-flagged-as-macaw-duplicates) | [377](https://github.com/SysBioChalmers/Human-GEM/blob/develop/data/testResults/macaw_results.csv) | -2 | :warning: | -| [Mass-imbalanced reactions](https://github.com/SysBioChalmers/Human-GEM/blob/develop/data/testResults/README.md#mass-imbalanced-reactions) | [87](https://github.com/SysBioChalmers/Human-GEM/blob/develop/data/testResults/balance_results.csv) | new | :warning: | -| [Charge-imbalanced reactions](https://github.com/SysBioChalmers/Human-GEM/blob/develop/data/testResults/README.md#charge-imbalanced-reactions) | [234](https://github.com/SysBioChalmers/Human-GEM/blob/develop/data/testResults/balance_results.csv) | new | :warning: | -| [Structure vs formula/charge inconsistencies](https://github.com/SysBioChalmers/Human-GEM/blob/develop/data/testResults/README.md#structure-vs-formulacharge-inconsistencies) | [397](https://github.com/SysBioChalmers/Human-GEM/blob/develop/data/testResults/qc_structure_consistency.csv) | new | :warning: | +| [Reactions flagged by MACAW dead-end test](https://github.com/SysBioChalmers/Human-GEM/blob/fix/qc-comment-transient-retry/data/testResults/README.md#reactions-flagged-by-macaw-dead-end-test) | [2510](https://github.com/SysBioChalmers/Human-GEM/blob/fix/qc-comment-transient-retry/data/testResults/macaw_results.csv) | 0 | :warning: | +| [Reactions flagged as MACAW duplicates](https://github.com/SysBioChalmers/Human-GEM/blob/fix/qc-comment-transient-retry/data/testResults/README.md#reactions-flagged-as-macaw-duplicates) | [377](https://github.com/SysBioChalmers/Human-GEM/blob/fix/qc-comment-transient-retry/data/testResults/macaw_results.csv) | 0 | :warning: | +| [Mass-imbalanced reactions](https://github.com/SysBioChalmers/Human-GEM/blob/fix/qc-comment-transient-retry/data/testResults/README.md#mass-imbalanced-reactions) | [87](https://github.com/SysBioChalmers/Human-GEM/blob/fix/qc-comment-transient-retry/data/testResults/balance_results.csv) | 0 | :warning: | +| [Charge-imbalanced reactions](https://github.com/SysBioChalmers/Human-GEM/blob/fix/qc-comment-transient-retry/data/testResults/README.md#charge-imbalanced-reactions) | [234](https://github.com/SysBioChalmers/Human-GEM/blob/fix/qc-comment-transient-retry/data/testResults/balance_results.csv) | 0 | :warning: | +| [Structure vs formula/charge inconsistencies](https://github.com/SysBioChalmers/Human-GEM/blob/fix/qc-comment-transient-retry/data/testResults/README.md#structure-vs-formulacharge-inconsistencies) | [397](https://github.com/SysBioChalmers/Human-GEM/blob/fix/qc-comment-transient-retry/data/testResults/qc_structure_consistency.csv) | 0 | :warning: | ### Model file and metabolic tasks | Check | Result | | | --- | ---: | :---: | -| [YAML round-trip (cobrapy)](https://github.com/SysBioChalmers/Human-GEM/blob/develop/data/testResults/README.md#yaml-round-trip-cobrapy) | pass | :white_check_mark: | -| [YAML round-trip (RAVEN)](https://github.com/SysBioChalmers/Human-GEM/blob/develop/data/testResults/README.md#yaml-round-trip-raven) | pass | :white_check_mark: | -| [YAML lint](https://github.com/SysBioChalmers/Human-GEM/blob/develop/data/testResults/README.md#yaml-lint) | pass | :white_check_mark: | -| [Essential metabolic tasks](https://github.com/SysBioChalmers/Human-GEM/blob/develop/data/testResults/README.md#essential-metabolic-tasks) | 57 passed | :white_check_mark: | -| [Verification metabolic tasks](https://github.com/SysBioChalmers/Human-GEM/blob/develop/data/testResults/README.md#verification-metabolic-tasks) | 21 passed | :white_check_mark: | +| [YAML round-trip (cobrapy)](https://github.com/SysBioChalmers/Human-GEM/blob/fix/qc-comment-transient-retry/data/testResults/README.md#yaml-round-trip-cobrapy) | pass | :white_check_mark: | +| [YAML round-trip (RAVEN)](https://github.com/SysBioChalmers/Human-GEM/blob/fix/qc-comment-transient-retry/data/testResults/README.md#yaml-round-trip-raven) | pass | :white_check_mark: | +| [YAML lint](https://github.com/SysBioChalmers/Human-GEM/blob/fix/qc-comment-transient-retry/data/testResults/README.md#yaml-lint) | pass | :white_check_mark: | +| [Essential metabolic tasks](https://github.com/SysBioChalmers/Human-GEM/blob/fix/qc-comment-transient-retry/data/testResults/README.md#essential-metabolic-tasks) | 57 passed | :white_check_mark: | +| [Verification metabolic tasks](https://github.com/SysBioChalmers/Human-GEM/blob/fix/qc-comment-transient-retry/data/testResults/README.md#verification-metabolic-tasks) | 21 passed | :white_check_mark: | -### [MEMOTE](https://github.com/SysBioChalmers/Human-GEM/blob/develop/data/testResults/README.md#memote) +### [MEMOTE](https://github.com/SysBioChalmers/Human-GEM/blob/fix/qc-comment-transient-retry/data/testResults/README.md#memote) -**Total score: 63.2%** (core subset)   +**Total score: 63.2%** (core subset)   0 | Section | Score | Δ vs base | | --- | ---: | ---: | -| consistency | 42.4% | | -| annotation_met | 73.0% | | -| annotation_rxn | 72.7% | | -| annotation_gene | 46.7% | | -| annotation_sbo | 81.7% | | +| consistency | 42.4% | 0 | +| annotation_met | 73.0% | 0 | +| annotation_rxn | 72.7% | 0 | +| annotation_gene | 46.7% | 0 | +| annotation_sbo | 81.7% | 0 |
Per-test scores @@ -89,11 +89,11 @@ _Duplicate keys (model unloadable) and no growth block the merge; every other ro
-_Full suite not run for this commit; comment_ `/run memote` _to add it._ +**Full suite: 64.2%**   0 · _from the last_ `/run memote`. _The score above is the fast core subset. Comment_ `/run memote` _to run the full suite on this pull request; the score updates here when it finishes._ -### [Gene essentiality (Hart 2015)](https://github.com/SysBioChalmers/Human-GEM/blob/develop/data/testResults/README.md#gene-essentiality-hart-2015) +### [Gene essentiality (Hart 2015)](https://github.com/SysBioChalmers/Human-GEM/blob/fix/qc-comment-transient-retry/data/testResults/README.md#gene-essentiality-hart-2015) _Not run automatically (it takes hours). Comment_ `/run gene-essentiality` _to run it on this pull request; the result posts as its own comment._ From a81ea654b6c83d7fe6d7c653efff74599676cacb Mon Sep 17 00:00:00 2001 From: Eduard Kerkhoven Date: Fri, 17 Jul 2026 20:03:25 +0000 Subject: [PATCH 3/3] fix: fast-forward local to the remote before the MEMOTE results commit MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The full MEMOTE suite runs for ~40 minutes, during which develop can advance — a concurrent results run, or (on a re-run) this workflow's own earlier attempt, may already have pushed a "full MEMOTE result" commit. The "Update local branch before committing changes" step fetched but never integrated the remote, so it left local behind origin and the auto-commit push was rejected as non-fast-forward (which failed run 29538381748 attempt 2). Reset local to origin/ before committing. memote_score.md — the only file this workflow commits — is preserved across the reset when the run actually produced a new score, so it wins over the remote while every other result file comes from the up-to-date remote. memote_result.json is untracked at the repo root, so the reset leaves it in place for the artifact upload. --- .github/workflows/memote-full.yml | 29 ++++++++++++++++++++++++----- 1 file changed, 24 insertions(+), 5 deletions(-) diff --git a/.github/workflows/memote-full.yml b/.github/workflows/memote-full.yml index 9ba711c6..97cd2142 100644 --- a/.github/workflows/memote-full.yml +++ b/.github/workflows/memote-full.yml @@ -94,12 +94,31 @@ jobs: done - name: Update local branch before committing changes + env: + BRANCH_NAME: ${{ github.ref_name }} run: | - git stash - git fetch - git checkout ${{ github.ref_name }} - if git stash list | grep -q 'stash@{'; then - git stash pop + # The full suite runs for ~40 min, during which the branch can move on: a + # concurrent results run, or (on a re-run) this workflow's own earlier + # attempt, may already have pushed a "full MEMOTE result" commit. The old + # `git checkout` left local behind origin, so the auto-commit push below was + # rejected as non-fast-forward. Reset local to the remote tip first. + # + # memote_score.md is the only file this workflow commits, so it is preserved + # across the reset (it wins over the remote); every other result file is + # taken from the up-to-date remote. memote_result.json is untracked at the + # repo root, so `git reset --hard` leaves it in place for the artifact upload. + git fetch origin "$BRANCH_NAME" + # Preserve the score only if this run actually rewrote it (a timed-out run + # leaves it unchanged); otherwise keep whatever the remote already has. + changed="" + if ! git diff --quiet -- data/testResults/memote_score.md; then + cp -a data/testResults/memote_score.md "$RUNNER_TEMP/memote_score.md" + changed=1 + fi + git checkout "$BRANCH_NAME" + git reset --hard "origin/$BRANCH_NAME" + if [ -n "$changed" ]; then + cp -a "$RUNNER_TEMP/memote_score.md" data/testResults/memote_score.md fi - name: Auto-commit results