feat: add duplicate-key QC gate to qcModelChecks

.github/workflows/model-qc.yml

@@ -2,10 +2,21 @@ name: Model QC checks
 
 on: [pull_request]
 
+env:
+  # All committed result files, fetched from the target branch so buildReport can
+  # show a delta, and stamped so a stale set shows as pending.
+  RESULT_FILES: >-
+    qc_duplicate_keys.csv qc_empty_reactions.csv qc_annotation_consistency.csv
+    qc_unused_entities.csv qc_duplicate_reactions.csv qc_metabolite_completeness.csv
+    qc_reaction_sanity.csv qc_annotation_issues.csv qc_growth.txt memote_score.md
+    macaw_results.csv balance_results.csv
+
 jobs:
-  # Fast checks (seconds to a couple of minutes). Posts the shared model-quality
-  # comment straight away so the pull request gets feedback without waiting for
-  # MEMOTE, and is a quick status check that can be required by branch protection.
+  # Fast structural checks (seconds to a couple of minutes). Writes a detailed CSV
+  # for every finding, commits them, and posts the shared model-quality comment
+  # straight away - even when a gate fails - so the comment always shows what is
+  # wrong. The build is failed at the end if a gate failed, so it can be a required
+  # check for branch protection.
   checks:
     runs-on: ubuntu-latest
     timeout-minutes: 20
@@ -20,24 +31,35 @@ jobs:
           python-version: "3.11"
 
       - name: Install dependencies
-        run: pip install cobra
+        run: pip install cobra pyyaml
 
-      - name: Metabolite completeness, GPR/bounds and growth checks
+      - name: Structural QC checks (gates + reports)
+        id: qc
+        continue-on-error: true
         run: python code/test/qcModelChecks.py
 
       - name: Annotation and cross-reference validation
+        continue-on-error: true
         run: python code/test/annotationTest.py
 
+      - name: Stamp results with the head commit
+        run: echo "${{ github.event.pull_request.head.sha }}" > data/testResults/qc_checks.sha
+
       - name: Fetch target-branch results for comparison
         env:
           BASE_REF: ${{ github.event.pull_request.base.ref }}
         run: |
           git fetch --depth=1 origin "$BASE_REF" || true
           mkdir -p "$RUNNER_TEMP/base"
-          for f in qc_metabolite_completeness.csv qc_reaction_sanity.csv qc_annotation_issues.csv qc_growth.txt memote_score.md; do
+          for f in $RESULT_FILES; do
             git show "origin/$BASE_REF:data/testResults/$f" > "$RUNNER_TEMP/base/$f" 2>/dev/null || rm -f "$RUNNER_TEMP/base/$f"
           done
 
+      - name: Mention PR# in README.md
+        env:
+          PR_NUMBER: ${{ github.event.number }}
+        run: sed -i -e "s/[[:digit:]]\{3,4\}\*\* (model QC/$PR_NUMBER\*\* (model QC/" data/testResults/README.md
+
       - name: Update local branch before committing changes
         env:
           BRANCH_NAME: ${{ github.head_ref || github.ref_name }}
@@ -65,6 +87,7 @@ jobs:
           BASE_RESULTS_DIR: ${{ runner.temp }}/base
           BASE_REF: ${{ github.event.pull_request.base.ref }}
           COMMIT_SHA: ${{ github.event.pull_request.head.sha }}
+          RESULTS_URL_BASE: ${{ github.server_url }}/${{ github.repository }}/blob/${{ github.head_ref }}/data/testResults
         run: |
           python code/test/buildReport.py
           {
@@ -83,6 +106,14 @@ jobs:
           TEST_RESULTS: ${{ steps.report.outputs.results }}
           GH_ACTION_URL: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}
 
+      # Fail the build if a gate failed, but only after the detail is committed and
+      # the comment posted, so the failure is visible in both.
+      - name: Fail if a build gate failed
+        if: steps.qc.outcome == 'failure'
+        run: |
+          echo "::error::A build gate failed; see the Build gates table in the PR comment and the linked CSVs."
+          exit 1
+
   # MEMOTE snapshot. A fast core subset runs on every pull request; the full
   # suite (which does FVA / a loopless MILP over every reaction and is far
   # slower) runs only on pull requests to main. It reuses the fast checks'
@@ -132,16 +163,24 @@ jobs:
           timeout "$LIMIT" python code/test/memoteSnapshot.py \
             || echo "::warning::MEMOTE did not finish within ${LIMIT}s; score unavailable this run."
 
+      - name: Stamp results with the head commit
+        run: echo "${{ github.event.pull_request.head.sha }}" > data/testResults/qc_memote.sha
+
       - name: Fetch target-branch results for comparison
         env:
           BASE_REF: ${{ github.event.pull_request.base.ref }}
         run: |
           git fetch --depth=1 origin "$BASE_REF" || true
           mkdir -p "$RUNNER_TEMP/base"
-          for f in qc_metabolite_completeness.csv qc_reaction_sanity.csv qc_annotation_issues.csv qc_growth.txt memote_score.md; do
+          for f in $RESULT_FILES; do
             git show "origin/$BASE_REF:data/testResults/$f" > "$RUNNER_TEMP/base/$f" 2>/dev/null || rm -f "$RUNNER_TEMP/base/$f"
           done
 
+      - name: Mention PR# in README.md
+        env:
+          PR_NUMBER: ${{ github.event.number }}
+        run: sed -i -e "s/[[:digit:]]\{3,4\}\*\* (MEMOTE/$PR_NUMBER\*\* (MEMOTE/" data/testResults/README.md
+
       - name: Update local branch before committing changes
         env:
           BRANCH_NAME: ${{ github.head_ref || github.ref_name }}
@@ -176,6 +215,7 @@ jobs:
           BASE_RESULTS_DIR: ${{ runner.temp }}/base
           BASE_REF: ${{ github.event.pull_request.base.ref }}
           COMMIT_SHA: ${{ github.event.pull_request.head.sha }}
+          RESULTS_URL_BASE: ${{ github.server_url }}/${{ github.repository }}/blob/${{ github.head_ref }}/data/testResults
         run: |
           python code/test/buildReport.py
           {

.github/workflows/qc-tests.yml

@@ -2,6 +2,13 @@ name: Run QC tests
 
 on: [pull_request]
 
+env:
+  RESULT_FILES: >-
+    qc_duplicate_keys.csv qc_empty_reactions.csv qc_annotation_consistency.csv
+    qc_unused_entities.csv qc_duplicate_reactions.csv qc_metabolite_completeness.csv
+    qc_reaction_sanity.csv qc_annotation_issues.csv qc_growth.txt memote_score.md
+    macaw_results.csv balance_results.csv
+
 jobs:
   qc-tests:
     runs-on: ubuntu-latest
@@ -19,7 +26,11 @@ jobs:
       - name: Install dependencies
         run: pip install git+https://github.com/Devlin-Moyer/macaw.git@main numpy==1.26.4
 
+      # continue-on-error so a MACAW crash does not skip the balance report and the
+      # comment; a MACAW crash leaves macaw_results.csv unchanged, so the comment
+      # shows those rows as pending rather than as this commit's result.
       - name: Run MACAW tests
+        continue-on-error: true
         run: python code/test/macawTests.py | tee "$RUNNER_TEMP/macaw_summary.txt"
 
       - name: Mass and charge balance report
@@ -46,20 +57,23 @@ jobs:
             echo '```'
           } > data/testResults/qc_summary.md
 
+      - name: Stamp results with the head commit
+        run: echo "${{ github.event.pull_request.head.sha }}" > data/testResults/qc_macaw.sha
+
       - name: Fetch target-branch results for comparison
         env:
           BASE_REF: ${{ github.event.pull_request.base.ref }}
         run: |
           git fetch --depth=1 origin "$BASE_REF" || true
           mkdir -p "$RUNNER_TEMP/base"
-          for f in qc_metabolite_completeness.csv qc_reaction_sanity.csv qc_annotation_issues.csv qc_growth.txt memote_score.md; do
+          for f in $RESULT_FILES; do
             git show "origin/$BASE_REF:data/testResults/$f" > "$RUNNER_TEMP/base/$f" 2>/dev/null || rm -f "$RUNNER_TEMP/base/$f"
           done
 
       - name: Mention PR# in README.md
         env:
           PR_NUMBER: ${{ github.event.number }}
-        run: sed -i -e "s/[[:digit:]]\{3,4\}\*\* (QC)/$PR_NUMBER\*\* (QC)/" data/testResults/README.md
+        run: sed -i -e "s/[[:digit:]]\{3,4\}\*\* (MACAW/$PR_NUMBER\*\* (MACAW/" data/testResults/README.md
 
       - name: Update local branch before committing changes
         env:
@@ -89,6 +103,7 @@ jobs:
           BASE_RESULTS_DIR: ${{ runner.temp }}/base
           BASE_REF: ${{ github.event.pull_request.base.ref }}
           COMMIT_SHA: ${{ github.event.pull_request.head.sha }}
+          RESULTS_URL_BASE: ${{ github.server_url }}/${{ github.repository }}/blob/${{ github.head_ref }}/data/testResults
         run: |
           python code/test/buildReport.py
           {

.github/workflows/yaml-validation.yml

@@ -25,8 +25,7 @@ jobs:
           ## Return non-zero exit code on warnings as well as errors
           # strict: # optional, default is false
 
-      - name: Basic MEMOTE tests
-        run: python code/test/memoteTest.py
-
-      - name: Test import with cobrapy and consistency with annotation files
-        run: python code/test/sanityCheck.py
+      # The former semantic checks here (cobra load, model<->annotation consistency,
+      # duplicate/empty reactions) were consolidated into code/test/qcModelChecks.py,
+      # which runs in the Model QC checks workflow and writes a detailed CSV plus a
+      # pull-request comment for each finding.

code/test/balanceTest.py

@@ -21,14 +21,20 @@
 def main():
     model = cobra.io.load_yaml_model("model/Human-GEM.yml")
     rows = []
+    errored = []
     for rxn in model.reactions:
         if rxn.boundary:
             continue
         if "biomass" in rxn.id.lower() or "biomass" in (rxn.name or "").lower():
             continue
         try:
             imbalance = rxn.check_mass_balance()
-        except Exception:
+        except Exception as exc:  # noqa: BLE001 - record, do not silently drop
+            # A reaction whose balance cannot be evaluated (usually a malformed
+            # formula) is a finding, not something to hide: record it as its own
+            # row so it shows up in the committed diff and the count.
+            errored.append(rxn.id)
+            rows.append((rxn.id, rxn.name or "", f"check_failed:{exc}", ""))
             continue
         if imbalance:
             mass = {k: v for k, v in imbalance.items() if k != "charge"}
@@ -44,12 +50,15 @@ def main():
         writer = csv.writer(fh)
         writer.writerow(["reaction", "name", "mass_imbalance", "charge_imbalance"])
         writer.writerows(rows)
-    n_mass = sum(1 for r in rows if r[2])
+    n_mass = sum(1 for r in rows if r[2])  # includes uncheckable reactions (surfaced, not hidden)
     n_charge = sum(1 for r in rows if r[3])
     print(
         f"Unbalanced reactions (excluding boundary and biomass): {len(rows)} "
-        f"({n_mass} mass, {n_charge} charge)"
+        f"({n_mass} mass, {n_charge} charge, {len(errored)} could not be checked)"
     )
+    if errored:
+        print(f"::warning::{len(errored)} reaction(s) could not be balance-checked: "
+              f"{';'.join(errored[:20])}{' ...' if len(errored) > 20 else ''}")
 
 
 if __name__ == "__main__":