Skip to content

Protonation invariant: detect and repair rows that are not protonations; four root causes fixed; the 26.1 run rolled out - #301

Merged
samseaver merged 15 commits into
ModelSEED:devfrom
freiburgermsu:protonation-invariant-gate
Sep 28, 2026
Merged

samseaver merged 15 commits into
ModelSEED:devfrom
freiburgermsu:protonation-invariant-gate

Conversation

@freiburgermsu

Copy link
Copy Markdown
Member

Follow-up to #296, which merged into dev on 2026-09-23 (f7527bce); the repaired rows depend on its a7200948. It was held on the fork as freiburgermsu#7 while #296 was pending and is proposed here unchanged, as its own PR. It merges cleanly onto dev (git merge-tree, verified) and is independent of the manuscript PRs #297–#299.

What was found

The critique named 37 compounds carrying chemically impossible formulas — hydrogens gained with no charge change — and recommended keeping the run and fixing them separately. Investigating it found the class is larger than 37, has four independent root causes, three of them not in Marvin at all, and can be detected and closed by one rule rather than by a list.

Root cause 1 — Marvin is protonating fragments that aren't molecules

InChI disconnects metal–ligand bonds by design. For Fe₄S₄ the InChI is 4Fe.4S; Marvin 26.1 protonates four free sulfides to H₂S and the row comes back H8Fe4S4, charge unchanged. It also protonates bare atoms in both representations: elemental S/Se/P/O → H₂S/H₂Se/PH₃/H₂O. None of these is a protonation state.

A protonation moves protons, so between a row and its source ΔH must equal Δcharge with heavy atoms conserved. That single, representation-agnostic invariant reproduces the critique's count exactly and shows what it missed:

impossible rows compounds shipped formulas that would change impossibly
23.4 bundle 42 22 (already shipping)
26.1 bundle 139 96 69 — 26 H-gained + 11 charge-only = the 37, plus 32 mixed

"Prefer the SMILE row" fixes most but not all, and for the wrong reason on the Fe–S cubanes: MetaCyc's own SMILES writes [SH] on the bridging sulfides, so its smiles.tsv says H4Fe4S4 where its inchi.tsv says Fe4S4 — a source-level disagreement (39 compounds), not a Marvin one.

Root cause 2 — parse_structure drops charge when its parsers fail the string

Checking each inchi.tsv row against what its InChI declares found 58 rows disagreeing with their own /q and /p layers:

  • RDKit rejects hypervalent halogen oxides (chlorate, InChI=1S/ClO3/c2-1(3)4/q-1); the OpenBabel fallback warns "Charge(s): Do not match" and returns 0. Nine anions stored as neutral radicals.
  • On multi-component strings with both /q and /p (the Mg porphyrins, .../q-1;+2/p-1), RDKit keeps the pre-/p hydrogen count — 49 rows stored one H and one charge unit too many. A neutral chlorophyll shipped as a cation.

This predates 26.1 and propagated into every bundle refreshed through that function (202 bundle InChI rows).

Root cause 3 — the picker never expected two bundles

Found by the new key check on the first regeneration, before anything was committed: 5,567 InChIKey rows in the regenerated pick file were not the key of their own InChI row, and 2,006 regenerated compound records carried a formula/charge from one bundle and a SMILES from the other (95 in the shipped records). List_ModelSEED_Structures.py resolves each structure type independently, and BiochemPy.loadStructures globbed every protonations/*.tsv into the Charged stage — so with 23.4 and 26.1 both present, at different protonation states for 6,893 compounds, it took the InChI from one vintage and the key or SMILES from the other. The shipped pick file was last regenerated 2026-07-04, before the 26.1 bundle existed. Any regeneration after #289 would have shipped this, with or without the rest of this branch.

Root cause 4 — the reaction refresh left old charges in every stoichiometry

Rebuild_Stoichiometry.py refreshes the formula embedded in each reaction's stoichiometry entries but never the embedded charge, and balanceReaction() reads that embedded pair. Running the documented Refresh_Reactions.sh after the records changed, every recharged compound left its old charge behind in every reaction it appears in: the rebalance saw a phantom hydrogen imbalance, Adjust_Reaction_Protons.py "fixed" it by adding a proton, and 5,935 reactions that were balanced against their records went OK → CI:1, with proton coefficients rewritten on 17,457. Latent until a refresh recharged thousands of compounds at once — which is exactly what keeping this run does. Found by recomputing the imbalance from the records rather than trusting the status column.

And Rebalance_Reactions.py rejected its own documented invocation — its argument guard admitted verbose but not the save that Refresh_Reactions.sh passes — so the rebalance step of the sequence has silently not been running at all.

Two more, found along the way

  • Print_Structure_Formula_Charge.py had never refreshed inchi.tsv/smiles.tsv since the layout migration — it looks for a structure column those files don't have, so every source row was silently skipped. Found because a refresh that should have moved 58 rows moved none.
  • Its OpenBabel path strips the R group the run script puts on wildcard SMILES — 8 rows of the 26.1 MetaCyc bundle lose their R on any refresh.

What changed (commits 1–7: the fix)

  1. parse_structure derives a standard InChI's formula and charge from its own layers (inchi_layers) — deterministic, parser-version-independent. Over 44,297 source InChIs it agrees with the parsed result on 44,239 and differs on exactly the 58 above. Source-file refresh fixed; R-stripping fixed; --types scopes a refresh. This commit carries the InChI-only refresh (58 source rows, 202 bundle rows). SMILE rows are deliberately not refreshed here — a full refresh also fills 109 previously-empty 23.4 rows the current parsers can read, and is its own change.
  2. Validate_Protonations.py — five checks (row, bundle, source, layers, key), strict vs INFO kinds, --fail-on-violation for CI, an allowlist file, --tsv with ModelSEED ids. Repair_Protonation_Rows.py — the rule: a row failing the invariant is replaced by its unprotonated source row, its InChIKey row re-hashed, every replacement recorded in _reports/<bundle>_passthrough_<source>.tsv. Dry-run by default.
  3. 26.1 bundle repaired: 130 rows + 96 InChIKey re-hashes. Validates with zero strict failures.
  4. 23.4 bundle repaired: 38 rows — at that point it was still consumed (loadStructures globbed both); commit 7 stops consuming it, and it stays validated. Separable: drop this commit and 23.4 stays as it was. Plus the regression test, Scripts/Tests/test_protonation_invariant.py.
  5. The gate in Run_Marvin_Protonations.py: the same compare() applied to each row as it's written (passthrough_invariant in the stats; InChIKey hashed from whichever InChI is actually written). Smoke-tested on KEGG --limit 100: elemental sulfur passes through in both rows, output validates clean. A bundle repaired post hoc and one gated at generation are now identical by construction.
  6. Docs: README, per-bundle invariant_gate notes in sources.yaml, a section in the run report.
  7. One consumed protonation bundle per source. sources.yaml already marks pKa tables consumed_by_production and Add_Compound_pKa_Sources.py is manifest-driven on it; protonation bundles now carry the same flag (26.1 true, 23.4 false — kept for provenance and still validated) and loadStructures honours it. Absent flag or manifest reads as consumed, so nothing else changes behaviour.

Why "pass the source through" is safe: the rule only touches rows the invariant rejects. The 297 disconnected-InChI compounds Marvin legitimately protonates — the /p-2 chlorophylls and cobalamins that the "protonate rather than pass through" decision in #296 exists to protect — all pass, and are untouched by construction.

One defect in my own first cut, caught by the gate before it shipped: the first repair replaced InChI rows but left their InChIKey rows hashed from the discarded protonated string, so elemental sulfur regenerated keyed as hydrosulfide. The key check exists because of it, the repair now re-hashes, and the commits were rebuilt.

What changed (commits 8–15: the rollout — "keep the run")

Each of the three documented steps run unchanged, in order, with an acceptance check between:

step what moves acceptance
List_ModelSEED_Structures.py picked InChI formula/charge for 6,893 compounds 0 impossible against any source row of the compound; 0 InChIKey rows not the key of their InChI row
Update_Compound_Structures_Formulas_Charge.py formula/charge on 7,544 records (7,434 a protonation vs shipped, 110 corrections of values that were already wrong); SMILES/InChIKey only on 3,738 0 impossible against source
commit 10: fix Rebalance_Reactions.py's guard and Rebuild_Stoichiometry.py's missing charge refresh — (the two pipeline bugs above; prerequisite for the next row)
Refresh_Reactions.sh → Refresh_Aliases.sh → Reprint_Biochemistry.py stoichiometry formulas/charges on 33,005 reactions (57,280 embedded charges refreshed); proton coefficients on 17,457 and water on 23, each resolving a paired H/charge imbalance; status on 8,490 — 5,288 of them charge-imbalanced → OK (26.1's states are more mutually consistent than 23.4's); 63 reactions newly flagged obsolete as duplicates, 18 restored, links/aliases propagated on 81 — flagged per the README, not removed every embedded formula/charge equals its record; Reprint is a fixed point after one pass (it re-renders 7,412 equation/definition strings); Check_Database_Consistency.py passes on 56,012 reactions and 45,708 compounds

The three compounds the critique named: cpd24682 stays Fe4S4/0 and cpd10112 stays C18H15ClSn/0 (both would have become H8Fe4S4 and C18H18ClSn); cpd33420 goes from Mo7O24/+12 to Mo7O24/−6. Elemental Se and P stop shipping as HSe⁻ and PH₃.

One number to read carefully

Each record's formula/charge checked against its own SMILES field, shipped → regenerated: disagreeing by more than a protonation 96 → 27, heavy-atom disagreement 161 → 151, off by exactly a protonation 95 → 224. Of the 224, 161 are new and every one is a compound whose InChI is more fragmented than its SMILES: Marvin protonated ligands InChI had detached from their metal, the protonation is charge-consistent so it passes the invariant, and the pick order (InChI@Charged first) feeds that formula into the record while the SMILES field carries the connected molecule — ferricyanide records as C6H3FeN6/0 with a SMILES of C6FeN6/−3. 160 of the 161 were consistent in the shipped records only because the 23.4 InChI row was unprotonated. This is #296's "protonate the InChI form" decision surfacing in the records; #296's own regeneration ships the same 161. Netting them out, the pre-existing class falls from 95 to 63.

The invariant cannot see this class by design. Adopted as commits 12–15 (the pick rule): when a compound's InChI is the more fragmented of its two representations, its formula and charge come from its least-fragmented SMILE structure — the test Run_Marvin_Protonations.py already applies to choose its input — applied in List_ModelSEED_Structures.py, where the compound-level values are decided. One guard, found on the first run: only switch when the two sources agree up to a protonation; where they don't (MetaCyc's [SH] on the Fe₄S₄ bridging sulfides), the SMILE row is no more trustworthy and the compound keeps the InChI-derived formula. 338 compounds take the route (_reports/Formula_From_SMILE_Row.txt; tagged in Pick_Reasons.txt), 226 records change, 0 are inconsistent with any source row, and the formula-vs-own-SMILES class goes 224 → 10 (below the shipped baseline of 95). The reaction refresh then touches 479 reactions and leaves the balanced count at 33,496.

What this costs, stated plainly

Keeping the run recharges ~7,500 compounds. The reaction refresh in commit 9 changes H⁺ stoichiometry and balance status on every reaction touched by them. The thermodynamic layer, the evidence grades, and every count in the manuscript PRs #297–#300 were measured before this and are stale against it; they need their own regeneration after merge (re-run Papers/NAR_Update_2026/analysis/population_basis_table.py first — it will say exactly what moved). Not attempted here.

Commits 8, 9, 11, 14 and 15 are tens of thousands of lines of regenerated data; commits 1–7, 10 and 13 are the ~20-file fix. They can be split into two PRs on request.

Left for curation, listed by the validator

  • 39 compounds whose inchi.tsv and smiles.tsv disagree beyond a protonation (source check) — the [SH] Fe–S cubanes and similar. The invariant cannot pick a side; a curator can.
  • 108 SMILE rows with no source formula at all (unchecked) and 67 wildcard rows whose H count is convention-dependent — reported, never failed.
  • The 27 rows that were impossible in both bundles (chlorate as a neutral radical, Se as H₂Se) are now fixed in both, but they had been the shipped values for years; downstream users of the 23.4-era formulas may want to know.

Verification

  • Scripts/Tests/test_protonation_invariant.py passes: invariant cases, inchi_layers cases, both bundles with zero strict row failures, every InChIKey row the key of its InChI row, inchi.tsv agreeing with its own layers.
  • Every repaired file was rebuilt from the base commit in order and compared byte-for-byte against the working copy before committing.
  • Acceptance projection on the regenerated pick file and compound records: zero formula changes impossible against any source row of the compound.
  • The reaction refresh was run three times before it was right: once revealing that the rebalance step never runs, once revealing the embedded-charge bug (the OK → CI:1 mass), once clean. Only the clean run is committed.

Relationship to the manuscript PRs

#297, #298 and #299 are text-only and based on dev; nothing in them depends on this PR. Once this merges, a handful of their counts move — most by the 19 reactions the refresh here retires (48,403 → 48,384 live), and M13's mass-and-charge-balanced share from 58% to 69% because the refresh now persists the proton-rebalance step and reads refreshed embedded charges. That re-measurement is prepared as one commit per branch on the fork (nar2026-transport-and-template-scope-after-invariant-gate, nar2026-llm-ensemble-role-after-invariant-gate, nar2026-figures-and-growth-basis-after-invariant-gate) and is applied after this lands, not before.

🤖 Generated with Claude Code

freiburgermsu and others added 15 commits September 24, 2026 18:52
…parser

A standard InChI declares its formula (the formula layer, with component
multipliers), its protonation (/p) and its charges (/q). The molecule a
parser builds from it can only agree with those or be wrong, and in this
repository's files it was wrong in two ways:

  - RDKit rejects hypervalent halogen oxides (chlorate, chlorite, iodate:
    "explicit valence for Cl, 6"), parse_structure falls back to OpenBabel,
    OpenBabel warns "Charge(s): Do not match" and returns charge 0. Nine
    inchi.tsv rows stored an anion as a neutral radical.
  - On multi-component strings carrying both /q and /p -- the Mg porphyrins,
    `.../q-1;+2/p-1` -- RDKit keeps the pre-/p hydrogen count and its
    charge. 49 rows stored one H and one charge unit too many: a neutral
    chlorophyll shipped as a cation.

Both are parser defects, and both propagated into every protonation bundle
refreshed through parse_structure. inchi_layers() now returns what the
identifier itself says for any standard InChI, deterministically and
independently of parser version; RDKit/OpenBabel remain for SMILES and as
the fallback for a non-standard InChI or one with a /f or /r layer. Over the
44,297 standard InChIs in the four source files it agrees with the parsed
result on 44,239 and differs on exactly the 58 above.

Two more things in this script, found while re-running it:

  - It had never refreshed inchi.tsv or smiles.tsv since the layout
    migration: main() passed the default `structure` column name and the
    source files name theirs `inchi` and `smiles`, so every source row was
    skipped and only the bundles were touched. A refresh that should have
    moved 58 rows moved none, which is how it showed.
  - Its OpenBabel path stripped the R group that Run_Marvin_Protonations.py
    puts on a wildcard SMILES (OpenBabel omits dummy atoms from its
    formula): 8 rows of the 26.1 MetaCyc bundle lost their R on a refresh.
    The wildcards are now counted in the string itself.

--types scopes a refresh to one representation. This commit is the InChI
refresh: 58 inchi.tsv rows and 202 bundle InChI rows (83 in 26.1, 119 in
23.4) re-derived from their layers. SMILE rows are untouched here -- a full
refresh also fills 109 previously empty 23.4 rows the current parsers can
read, and is left as its own change.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…he ones that fail

A protonation moves protons. Between a source structure and its pH-adjusted
form, therefore, dH must equal dcharge and no other element may change. A
row that breaks this did not undergo a protonation -- and in this repository
"something else" has one recurring cause: InChI disconnects metal-ligand
bonds, Marvin is handed the ligands as free ions and protonates them. Fe4S4
comes back as H8Fe4S4, triphenyltin chloride as C18H18ClSn, heptamolybdate
as H48Mo7O24; bare S, Se, P and O atoms come back as H2S, H2Se, PH3 and H2O
in both representations. 37 such formulas were about to ship into the
compound records.

Validate_Protonations.py checks every row of a bundle against that
invariant. It is representation-agnostic and needs no list of metals, which
is the point: whatever future engine change or source quirk produces an
impossible row, this catches it. Four checks -- each row against its own
source row; the bundle's InChI row against its SMILE row; inchi.tsv against
smiles.tsv; and each inchi.tsv row against its own InChI layers -- with
strict kinds that fail a gated run and INFO kinds (no source formula to
compare against; wildcard structures, whose H count is convention-
dependent) that are reported only. --fail-on-violation makes it a CI gate;
--tsv lists every finding with its ModelSEED ids; --root validates a copy.
An allowlist file (header only for now) lets a curated case be tolerated
deliberately rather than by everyone learning to ignore the check.

Repair_Protonation_Rows.py applies the rule: a row that fails is replaced by
its own representation's source row, unprotonated, and every replacement is
recorded next to the other run reports. The protonation state of a
shattered fragment set is undefined; the source is at least a
self-consistent description. The rule only touches rows the invariant
rejects, so the 297 disconnected-InChI compounds Marvin legitimately
protonates -- the chlorophylls and cobalamins picking up /p-2 -- are
untouched by construction. Dry run by default.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Applied Repair_Protonation_Rows.py to marvin_26.1_ph7 in all four sources:
130 InChI and SMILE rows replaced by their unprotonated source row (MetaCyc
77, KEGG 34, ChEBI 19, Rhea 0), and the InChIKey row of every compound
whose InChI row was replaced re-hashed from the InChI now written (96
rows). Each replaced row and what it replaced is in
_reports/marvin_26.1_ph7_passthrough_<source>.tsv.

By kind: 58 hydrogens gained with no charge change (the detached-ligand and
bare-atom protonations), 44 mixed dH != dcharge, 15 hydrogens lost with no
charge change, 13 charge changed with no hydrogen change. Run through the
structure picker before this, the bundle would have changed 6,920 shipped
formulas, 69 of them impossible against their source; after it, 0 are
impossible, the legitimate 26.1 protonation-state changes are untouched,
and previously shipped formulas that were already impossible are corrected
(elemental Se had shipped as HSe-, phosphorus as PH3, heptamolybdate as +12
where its InChI declares -6).

139 rows failed before the InChI-layer refresh; 9 of those were "impossible"
only because the parser had stored the wrong charge, and pass now.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
… with a test

The 23.4 bundle is still consumed: BiochemPy.loadStructures globs every
protonations/*.tsv, so its rows compete with 26.1's in the picker. It
carried 42 rows breaking the invariant before the layer refresh and 38
after (MetaCyc 13, KEGG 22, ChEBI 3) -- elemental Se as H2Se, elemental
P as PH3, chlorate as a neutral radical -- some of which had been the
shipped values for years. Same rule, same sidecar, InChIKey rows re-hashed
the same way, same reversibility.

This commit is separable: dropping it leaves 26.1 clean and 23.4 as it was,
at the cost of the test below failing on 23.4.

Scripts/Tests/test_protonation_invariant.py holds all of it: the invariant
on named cases, inchi_layers on acetate / chlorate / a shattered Fe4S4 /
triphenyltin / a Mg porphyrin / a multiplied /q, and both bundles with zero
strict failures, every InChIKey row the key of its InChI row, and inchi.tsv
agreeing with its own layers. Exit 1 on any failure, in the style of its
neighbours.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…tten

The same compare() the post-hoc repair uses, applied to each SMILE and InChI
row after its formula and charge are computed: a row that fails dH ==
dcharge against its source is written as the source row instead, counted as
passthrough_invariant in the per-source stats, and listed in
_reports/marvin_<ver>_<ph>_passthrough_<source>.tsv. INFO kinds are never
replaced. The InChIKey is hashed from whichever InChI is actually written,
so a passed-through InChI row does not carry the key of the discarded
protonated one.

A bundle repaired after the fact and one gated at generation are identical
by construction, so the repaired files in this branch are what a
regeneration now produces.

Smoke-tested on KEGG --limit 100 in a throwaway checkout: elemental sulfur
(C00087, row 77) passes through in both rows, passthrough_invariant=2, the
key written is that of InChI=1S/S, and the gated output validates clean.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…port

Each protonation bundle entry in sources.yaml gains an invariant_gate note
with how many of its rows were replaced and how many InChI rows were
re-derived from their layers, and the claim that re-running
Print_Structure_Formula_Charge.py is a no-op is qualified -- it is for
SMILE rows, not InChI rows. The run report gains a section that restates
the "visible cost" of the InChI-row decision as what it is (not a
protonation state), gives the numbers, and records the parser defect found
underneath it. The README describes the two new scripts and the changes to
the refresh script.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Found by the key check on the first regeneration: 5,567 InChIKey rows in
the regenerated pick file were not the key of their own InChI row, and
2,006 regenerated compound records carried a formula/charge from one
bundle and a SMILES from the other (95 in the shipped records). The picker
resolves each structure TYPE independently, and BiochemPy.loadStructures
globbed every protonations/*.tsv into the Charged stage -- so with Marvin
23.4 and 26.1 both present, at different protonation states for 6,893
compounds, it took the InChI from one vintage and the InChIKey or SMILES
from the other. The shipped pick file was last regenerated on 2026-07-04,
before the 26.1 bundle existed, so it had only ever seen one bundle; this
would have shipped with any regeneration after ModelSEED#289, with or without the
rest of this branch.

sources.yaml already marks pKa tables consumed_by_production, and
Add_Compound_pKa_Sources.py is manifest-driven on it. Protonation bundles
now carry the same flag -- 26.1 true, 23.4 false, kept for provenance --
and loadStructures skips a bundle flagged false. A bundle without the flag,
an entry without a manifest, or a tree without sources.yaml all read as
consumed, so nothing else changes behaviour. The 23.4 bundle is still
validated by the regression test.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
List_ModelSEED_Structures.py, run unchanged, with 26.1 the consumed
bundle. The picked InChI formula/charge changes for 6,893 compounds.
Acceptance, all required to commit this: every one of those is consistent
with at least one source row of its compound by the protonation invariant
(0 impossible); every InChIKey row is the key of its InChI row (0
mismatches, against 5,567 with both bundles loaded). The change is the 26.1
protonation-state shift the run was kept for, without the 69 impossible
rows it would otherwise have carried.

The "Duplicate InChIKey" warning the script prints for elemental sulfur now
names cpd00074, cpd27013 and cpd27311 together: three records of the same
compound (KEGG, MetaCyc, ChEBI) that key identically once the impossible
H2S rows are gone -- the warning working as intended.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Update_Compound_Structures_Formulas_Charge.py, run unchanged. 7,544
compounds change formula or charge and 3,738 more change only their SMILES
or InChIKey (26.1 writes SMILES with its own canonical atom order, as the
run report notes). Against the shipped values, 7,434 of the 7,544 are a
protonation (dH == dcharge) and 110 are corrections, each consistent with
the compound's source row where the shipped value was not. None is
impossible against its source.

Checked each record's formula/charge against its own SMILES field, shipped
vs regenerated: agreeing by more than a protonation 96 -> 27, heavy-atom
disagreement 161 -> 151, off by a protonation 95 -> 224. That last number
is the one to read carefully. 161 of the 224 are new, and every one of them
is a compound whose InChI is more fragmented than its SMILES: Marvin
protonated ligands that InChI had detached from their metal, the
protonation is charge-consistent so it passes the invariant, and the pick
order (InChI@Charged first) feeds that formula into the record while the
SMILES field carries the connected molecule -- ferricyanide records as
C6H3FeN6/0 with a SMILES of C6FeN6/-3. 160 of the 161 were consistent in
the shipped records only because the 23.4 InChI row was unprotonated. This
is the "protonate the InChI form" decision of the parent branch surfacing
in the records, and its own regeneration ships the same 161; netting them
out, the pre-existing class falls from 95 to 63. The fix is a pick rule,
not a repair -- take formula and charge from the SMILE row when the InChI
is the more fragmented, the test the run script already applies to its
input -- and is raised in the PR rather than taken here.

The three compounds the critique named: cpd24682 stays Fe4S4/0 and
cpd10112 stays C18H15ClSn/0 (both would have become H8Fe4S4 and
C18H18ClSn); cpd33420 goes from Mo7O24/+12 to Mo7O24/-6. Elemental
selenium (cpd01079) and phosphorus (cpd20420) stop shipping as HSe- and
PH3.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…s with formulas

Two bugs in Scripts/Biochemistry/Refresh_DB_after_Changes, both found by
running the documented Refresh_Reactions.sh after the compound records
changed, and both latent until a refresh recharged thousands of compounds
at once.

Rebalance_Reactions.py rejected its own documented invocation. The body
reads "save", "print" and "verbose" out of argv, but the argument guard
admitted only "verbose", so `Rebalance_Reactions.py save` -- the line in
Refresh_Reactions.sh -- failed with "invalid choice: 'save'" and the
rebalance step of the sequence silently never ran. The guard now admits
what the body consumes.

Rebuild_Stoichiometry.py refreshed only the formula embedded in each
stoichiometry entry, never the charge. balanceReaction() reads that
embedded pair. So every compound whose protonation state changed left its
old charge behind in every reaction it appears in; the rebalance saw a
phantom hydrogen imbalance with no charge imbalance, and
Adjust_Reaction_Protons.py "fixed" it by adding a proton -- manufacturing
a real charge imbalance on 5,935 reactions that were balanced against
their records (OK -> CI:1) and rewriting proton coefficients on 17,457.
Recomputed from the records, those reactions were balanced all along.
Charge is now refreshed with the formula: 57,280 stoichiometry entries in
this tree, against 53,199 formula updates.

With both fixed, the same refresh resolves paired imbalances instead of
creating them (MI:H:-1|CI:-1 -> OK on 7,813 reactions, MI:H:1|CI:1 -> OK
on 6,172), and the net effect on the shipped statuses is 5,288 reactions
going from charge-imbalanced to OK.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Refresh_Reactions.sh then Refresh_Aliases.sh, run unchanged, followed by
Reprint_Biochemistry.py, which is a fixed point after one pass (it
re-renders the equation and definition text of 7,412 reactions from their
stoichiometry; a second run changes nothing). Check_Database_Consistency.py
passes on all 56,012 reactions and 45,708 compounds.

What moved, against the shipped records: stoichiometry formulas and
charges on 33,005 reactions; proton coefficients on 17,457 and water on
23, every one resolving a paired hydrogen/charge imbalance; status on
8,490, of which 5,288 are charge-imbalanced -> OK -- the 26.1 protonation
states are more mutually consistent than the 23.4 ones were; obsolescence
on 18 (63 reactions newly marked obsolete as duplicates of another, 18
restored as the primary), with linked_reaction and aliases propagated on
81. The newly obsolete reactions are flagged, as the README prescribes,
and not removed.

Every stoichiometry entry's embedded formula and charge now equals its
compound record's; the only differences are compounds with no formula,
where the record holds None and the entry the string "null".

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…variant cannot see

The run report is what the next maintainer reads to understand this bundle,
so it now carries what the PR body carries: the 161 charge-consistent
protonations of detached ligands that the invariant passes by design (a
pick question, left as a follow-up), the two-bundle picker defect and the
consumed_by_production flag that closes it, and the two
Refresh_DB_after_Changes bugs. The test's stated reason for validating the
23.4 bundle is corrected -- it is no longer globbed; it is validated as
hygiene -- and the Biochemistry README says that Rebalance_Reactions.py
needs `save` to write, and that it rejected it until this branch.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…I is the more fragmented

The invariant cannot see this class by design: InChI detaches metal
ligands, Marvin protonates the detached ligands as free ions, and the
result is charge-consistent (dH == dcharge) while describing a molecule
that does not exist. Ferricyanide's InChI row reads C6H3FeN6/0 beside a
SMILES of the connected C6FeN6/-3, and 161 records shipped that way after
the rollout -- consistent before it only because the 23.4 InChI row was
unprotonated.

The pick file writes one compound-level formula and charge across its
three rows, taken from the InChI row (sources.yaml structure_pick_order),
so the rule lives where that choice is made, in List_ModelSEED_Structures:
when the compound's InChI representation is the more fragmented of the two
(the test Run_Marvin_Protonations.py already applies to choose its input),
the formula and charge come from its least-fragmented SMILE structure.
The InChI and InChIKey rows are unchanged -- they still carry what InChI
can represent.

One guard, found on the first run: only switch when the two SOURCE
representations agree up to a protonation. Where they do not -- MetaCyc
writes [SH] on the bridging sulfides of Fe4S4, so its smiles.tsv says
H4Fe4S4 against its inchi.tsv's Fe4S4 -- switching traded a
detached-ligand formula for a source quirk, and two compounds whose
MetaCyc SMILES carries no source formula at all came out consistent with
no source row. Those keep the InChI-derived formula; they are the `source`
findings of Validate_Protonations.py, and curation.

338 compounds take the SMILE route, listed in
_reports/Formula_From_SMILE_Row.txt and tagged
formula_from_smile:disconnected_inchi in Pick_Reasons.txt.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
List_ModelSEED_Structures.py then Update_Compound_Structures_Formulas_Charge.py.
226 compound records change formula or charge; none is inconsistent with
its source rows in either representation; every InChIKey row is still the
key of its InChI row. Each record's formula and charge checked against its
own SMILES field: off by a protonation 224 -> 10, the rest unchanged. The
Fe4S4 cubane (cpd24682) stays Fe4S4/0 -- the guard held.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Refresh_Reactions.sh, Refresh_Aliases.sh, Reprint_Biochemistry.py (a
fixed point after one pass), Check_Database_Consistency.py clean. 479
reactions change: stoichiometry formulas/charges on all of them, proton
coefficients on 231 and water on 1, status on 26, obsolescence on 1. Every
embedded formula and charge equals its record; the live balanced count is
unchanged at 33,496.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@samseaver

Copy link
Copy Markdown
Contributor

Looks good per a local review, thanks @freiburgermsu !

@samseaver
samseaver merged commit 3bec85c into ModelSEED:dev Sep 28, 2026
samseaver pushed a commit that referenced this pull request Oct 1, 2026
… Contribution, eQuilibrator, dGPredictor ingested, operators backfilled, LLM directions restored, --cv graded, canonical picks promoted

PR #301 changed 33,005 reactions' embedded formulas and charges and
81 obsolescence/canonical relationships, which invalidated every prior
thermodynamics estimate keyed to the old stoichiometry. This is the
full pipeline rerun against the merged 26.1 records: Group Contribution
and eQuilibrator dG estimates regenerated from the current stoichiometry,
dGPredictor ingested fresh, reaction operators backfilled where the
protonation refresh had left them stale, LLM-derived reaction directions
restored where the direction field had been dropped by the refresh, and
grade_reactions.py run with --cv to produce out-of-fold evidence grades
before canonical promotion.

opentecr_comparison.csv and opentecr_comparison_diagnostics.json (new)
hold the eQuilibrator/OpenTECR calibration pass this run is graded
against: 4,544 OpenTECR rows against 29,617 predicted reactions, 1,361
matched (802 stereo-exact, 559 skeleton-only), 511 full and 479
skeleton-only TECR groups.

Grade movement, reaction_grades.tsv (56,012 reactions unchanged in
count):
  GOLD    3,434 -> 5,358  (+1,924)
  SILVER 18,388 -> 17,184
  BRONZE 11,277 -> 10,556

The net gain is corroboration surfacing once GC/eQuilibrator/dGPredictor
agree on the re-picked stoichiometry, not new evidence sources --
SILVER and BRONZE both shrink by roughly the amount GOLD grows.

81 reactions changed their canonical thermodynamic direction under the
refreshed dG estimates (grade_frontier.tsv). Compound records are an
exact fixed point of this run: no cpd*.json or compound TSV differs
from origin/dev, confirming the rerun only touches thermodynamics/
grading artifacts and does not disturb the #301 structure picks.

Reaction shards (Biochemistry/reaction_00.json..reaction_60.json,
matching .tsv shards) carry the operator backfill and LLM direction
restoration described above; no stoichiometry, formula, or charge
changes are introduced by this run.
samseaver pushed a commit that referenced this pull request Oct 1, 2026
Run Repair_Split_Notes.py save against the current database. Rejoins the
character-split "WB" fragments the Adjust_Reaction_Water bug produced (39
reactions, e.g. ['GCP','EQP','|','W','B'] -> ['GCP','EQP','WB']) and drops
the empty-string note entries riding along in the same records (17
reactions). No unrecognised, unrejoinable runs were found -- every corrupted
record repaired cleanly.

Note: the count (56) is higher than the 15 estimated from the pre-PR-#301
local working tree, because origin/dev has since run additional
Rebalance/Adjust_Reaction_* refresh cycles that hit more reactions with the
same still-unfixed bug. This commit cleans up all of them, not just the
original 15.

Only the "notes" field changes in every touched record; stoichiometry,
status, and all other fields are untouched, confirmed by diff.
samseaver pushed a commit that referenced this pull request Oct 1, 2026
…-notes fix, structure-pick safety net

Brings in the four Stage-1 fixes from ck-preservation-and-structure-fallback-fixes
(branched off PR #301 / origin/dev@3bec85cc, independent of the thermodynamics
rerun and post-2020 reaction deletion already on this branch):

1. Reactions.preserveCK() + wiring into Rebalance_Reactions, Adjust_Reaction_Protons
   and Adjust_Reaction_Water, so a curator-checked (CK) reaction whose recomputed
   status disagrees with its stored one is rewritten (with CK preserved) instead
   of silently kept stale.
2. Adjust_Reaction_Water's notes += "|WB" list-corruption bug, fixed to
   notes.append("WB") to match Adjust_Reaction_Protons' existing pattern.
3. List_ModelSEED_Structures' INCUMBENT no-downgrade guard: an unresolved
   formula_conflict now falls back to the pre-existing Unique_ModelSEED_Structures.txt
   record (tagged formula_conflict_kept_incumbent) instead of writing an empty
   pick, layered on top of the existing fragment-count/protonation-consistency
   rule without touching it.
4. Check_Formulas: removed the long-dead sys.exit() stub, fixed
   missing_atoms.update(atom) -> .add(atom), added a stdout summary.

Only the code changes are taken as a straight merge. The data-fix commit from
the source branch (Repair_Split_Notes.py save, run against origin/dev@3bec85cc)
is NOT merged as a raw diff: this branch's reaction records have since moved
through a full thermodynamics rerun and a 6-reaction deletion, so the 13
reaction shards the source branch's data commit touched are reset here to
this branch's current (post-rerun, post-deletion) content. Repair_Split_Notes.py
is re-run fresh against this branch's current data in the next commit to apply
the notes fix without clobbering the thermodynamics fields or losing the
deletion.
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.

2 participants