Skip to content

ci: overhaul model-QC comment and result-file handling - #1062

Merged
edkerk merged 11 commits into
developfrom
fix/qc-comment-running-state
Jul 16, 2026
Merged

ci: overhaul model-QC comment and result-file handling#1062
edkerk merged 11 commits into
developfrom
fix/qc-comment-running-state

Conversation

@edkerk

@edkerk edkerk commented Jul 15, 2026

Copy link
Copy Markdown
Member

Summary

A set of related improvements to the model-QC pull-request comment and the committed result files under data/testResults/.

Changes

  • Running-state fix. The "Model file and metabolic tasks" rows now show as running until the checks phase finishes, instead of displaying the previous run's committed values.
  • New check — deprecation completeness. Flags reactions/metabolites that were removed since the base branch but not moved to deprecatedReactions.tsv / deprecatedMetabolites.tsv (qc_deprecation_completeness.csv). Non-blocking report; needs the base-branch model tables, so it runs in CI only.
  • Merged comment tables. The former Structural checks and Model QC reports tables are now one Model checks table. Every check name links to its explanation in the testResults README.
  • README reorganised into three parts: per-file provenance (which PR produced each file), a per-test explanation for every check shown in the comment (headings match the comment names), and a file index.
  • Combined one-line files. qc_roundtrip_*.txt, qc_yamllint.txt, qc_tasks_*.txt and qc_growth.txt are replaced by a single qc_status.tsv, updated through a qcStatus.py upsert helper.
  • Comment posted only after commit. The comment is updated in two phases — fast checks, then MEMOTE — with each commit landing before its comment update, so the numbers shown and the CSV links always reflect what is on the branch.
  • Normalised model files. The QC run also rewrites model/Human-GEM.yml in a normalised, ordered form and sorts the model/*.tsv tables into a stable order. These land in a separate commit from the data/testResults/* results commit, so the model files on the branch stay canonically formatted.
  • MEMOTE core vs full suite. memote_score.md now has separate Core subset and Full suite sections; each run rewrites only its own, so a routine subset run never overwrites a committed full-suite score, and deltas only ever compare like with like.
  • Node 20 deprecation. actions/github-script bumped v7 → v9 (Node 24).
  • Gene-essentiality README stamping fixed (its PR-number step was gated on a condition that is never true for a dispatched run).

Validation

Workflows and the composite action parse; the Python scripts compile; buildReport.py renders correctly across the running/fast/final phases; all comment→README anchor links resolve; the MEMOTE section-preservation and qcStatus upsert were unit-checked. End-to-end CI behaviour will be exercised when this runs as a pull request.

- Show "Model file and metabolic tasks" rows as running until the checks
  phase completes, instead of displaying the previous run's committed values.
- Add QC check: flag reactions/metabolites removed since the base branch that
  were not moved to the deprecated identifier lists (qc_deprecation_completeness.csv).
- Merge the structural-checks and model-QC-reports tables into one; link every
  check name to its explanation in the testResults README.
- Reorganise data/testResults/README.md: per-file provenance, per-test
  explanations (anchors matching the comment), and a file index.
- Combine the one-line result files (round-trip, YAML lint, metabolic tasks,
  growth) into a single qc_status.tsv via a qcStatus.py upsert helper.
- Update the PR comment only after results are committed, in two phases (fast
  checks, then MEMOTE), so shown numbers and CSV links are always on the branch.
- Store MEMOTE core-subset and full-suite scores in separate sections of
  memote_score.md so a routine run never overwrites a full-suite score; each
  section is compared only against the same section on the base branch.
- Bump actions/github-script to v9 (Node 24) to clear the Node 20 deprecation.
- Fix gene-essentiality README PR-number stamping (guarded on a never-true condition).
@github-actions

github-actions Bot commented Jul 15, 2026

Copy link
Copy Markdown

Model quality report

⚠️ 6 pre-existing finding(s), no regressions vs develop. Non-blocking.

Each check name links to its explanation in the testResults README.

Model checks

Duplicate keys (model unloadable) and no growth block the merge; every other row is a non-blocking report.

Check Result Δ vs develop
Duplicate !!omap keys 0 0
Growth (biomass producible) 125 new
Reactions with no metabolites 0 0
Model / annotation-table inconsistencies 0 0
Removed reactions or metabolites not deprecated 0 new
Metabolites missing formula 0 0
Metabolites missing charge 0 0
Reaction bound / GPR issues 0 0
Exact-duplicate reaction groups 0 0
Unused metabolites 0 0
Unused genes 0 0
Malformed cross-references 0 0
Cross-refs inconsistent across compartments 3 0 ⚠️

MACAW and mass/charge balance

Check Result Δ vs develop
Reactions flagged by MACAW dead-end test 2510 0 ⚠️
Reactions flagged as MACAW duplicates 377 0 ⚠️
Mass-imbalanced reactions 87 0 ⚠️
Charge-imbalanced reactions 234 0 ⚠️
Structure vs formula/charge inconsistencies 397 0 ⚠️

Model file and metabolic tasks

Check Result
YAML round-trip (cobrapy) pass
YAML round-trip (RAVEN) pass
YAML lint pass
Essential metabolic tasks 57 passed
Verification metabolic tasks 21 passed

MEMOTE

Total score: 63.2% (core subset)   +43.0 ✅

Section Score Δ vs base
consistency 42.4% 0
annotation_met 73.0% +48.0 ✅
annotation_rxn 72.7% +47.7 ✅
annotation_gene 46.7% +46.7 ✅
annotation_sbo 81.7% +81.7 ✅
Per-test scores
Section Test Score
Consistency Stoichiometric Consistency 100.0%
Consistency Mass Balance 0.8%
Consistency Charge Balance 2.1%
Consistency Metabolite Connectivity 0.0%
Consistency Unbounded Flux In Default Medium 100.0%
Annotation - Metabolites Presence of Metabolite Annotation 0.0%
Annotation - Metabolites Metabolite Annotations Per Database 62.3%
Annotation - Metabolites Metabolite Annotation Conformity Per Database 45.8%
Annotation - Metabolites Uniform Metabolite Identifier Namespace 0.0%
Annotation - Reactions Presence of Reaction Annotation 0.0%
Annotation - Reactions Reaction Annotations Per Database 75.9%
Annotation - Reactions Reaction Annotation Conformity Per Database 33.3%
Annotation - Reactions Uniform Reaction Identifier Namespace 0.0%
Annotation - Genes Presence of Gene Annotation 0.0%
Annotation - Genes Gene Annotations Per Database 80.0%
Annotation - Genes Gene Annotation Conformity Per Database 80.0%
Annotation - SBO Terms Metabolite General SBO Presence 0.0%
Annotation - SBO Terms Metabolite SBO:0000247 Presence 0.1%
Annotation - SBO Terms Reaction General SBO Presence 0.0%
Annotation - SBO Terms Metabolic Reaction SBO:0000176 Presence 0.0%
Annotation - SBO Terms Transport Reaction SBO:0000185 Presence 0.7%
Annotation - SBO Terms Exchange Reaction SBO:0000627 Presence 0.0%
Annotation - SBO Terms Demand Reaction SBO:0000628 Presence 100.0%
Annotation - SBO Terms Sink Reactions SBO:0000632 Presence 100.0%
Annotation - SBO Terms Gene General SBO Presence 0.0%
Annotation - SBO Terms Gene SBO:0000243 Presence 0.0%
Annotation - SBO Terms Biomass Reactions SBO:0000629 Presence 0.0%

Full suite not run for this commit; comment /run memote to add it.

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)

Not run automatically (it takes hours). Comment /run gene-essentiality to run it on this pull request; the result posts as its own comment.

❌ = a count rose vs the target branch (regression) · ⚠️ = a pre-existing non-zero finding (non-blocking) · ⏳ = still running. Counts link to the CSV listing the exact entries.

Full workflow run · this comment is edited as results come in

edkerk and others added 10 commits July 15, 2026 20:52
The committed YAML model carries only ids and names, so MEMOTE scored every
annotation section 0% even though the cross-references exist in the annotation
tables. Add annotateModel.py, which attaches the database identifiers from
metabolites.tsv / reactions.tsv / genes.tsv to an in-memory model, and call it in
memoteSnapshot.py before writing the temporary SBML.

- Maps only registry (identifiers.org) namespaces MEMOTE can validate; legacy-only
  columns (EHMN, HepatoNET1, Recon3D, HMR2, Ratcon) are skipped.
- Normalises values to each namespace: Rhea loses its "RHEA:" prefix, KEGG
  metabolite ids split into compound/glycan/drug by prefix, genes get ensembl (from
  the id) plus uniprot and ncbigene.
- The enriched model exists only in memory for the temporary SBML; nothing extra is
  committed.
…or MEMOTE

Bring the canonical annotation helper code/annotateGEM.py (a port of annotateGEM.m)
and its release caller code/io/increaseHumanGEMVersion.py into the repo, and use
annotate_gem from memoteSnapshot.py instead of an ad-hoc helper. This attaches the
TSV cross-references and SBO terms to the in-memory model before the temporary SBML
that MEMOTE reads (nothing extra is committed).

Extend annotateGEM's SBO assignment to everything MEMOTE checks:
- metabolites get SBO:0000247 (simple chemical) and genes SBO:0000243 (gene);
- boundary reactions split into exchange / demand / sink (SBO:0000627 / 0000628 /
  0000632) via cobra's own classification, which MEMOTE also uses, so each reaction
  carries the term its check expects. Falls back to exchange-for-all if cobra
  cannot classify.

Remove the interim code/test/annotateModel.py in favour of annotateGEM.
The MEMOTE fast subset finishes quickly, so the interim fast-checks commit and
comment update are unnecessary. Run every check, commit once at the end, and post
the comment from the committed files - keeping the invariant that the comment never
shows numbers or CSV links that are not yet on the branch.
annotateGEM now delegates metabolite and reaction SBO assignment to the canonical
raven_toolbox.annotation.add_sbo_terms (passing Human-GEM's biomass reaction name)
instead of a hand-rolled version; it keeps the Human-GEM-specific TSV cross-reference
merge, and still sets the gene SBO term (SBO:0000243) that add_sbo_terms does not
cover.

memoteSnapshot loads the model with raven_toolbox.io.read_yaml_model, like the other
RAVEN-based tests.

increaseHumanGEMVersion writes its exports with raven_toolbox.io.export_for_git (yml/
mat plain, xml/xlsx/txt annotated) instead of calling cobra's writers and hand-rolled
txt / dependencies writers directly.

Validated on the full model: SBO terms assigned to all metabolites/reactions/genes
(biomass MAR13082 -> SBO:0000629), cross-references merged without clobbering existing
ones, and export_for_git writes the annotated SBML.
export_for_git writes the .mat with cobra's default variable name (the model id,
HumanGEM). Write the plain YAML/MATLAB exports explicitly instead - YAML via
raven-toolbox, MATLAB via cobra with varname=humanGEM - and keep export_for_git for
the annotated xml/xlsx/txt exports.
Revert the increaseHumanGEMVersion workaround (explicit write_yaml_model +
save_matlab_model) now that raven-toolbox's export_for_git takes a varname argument.
The plain yml/mat export is a single export_for_git call again, with
varname='humanGEM' pinning the MATLAB struct name.
@edkerk edkerk mentioned this pull request Jul 16, 2026
3 tasks
@edkerk
edkerk merged commit 6ba2e23 into develop Jul 16, 2026
1 check passed
@edkerk
edkerk deleted the fix/qc-comment-running-state branch July 16, 2026 19:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant