From 4d94392beb5c39a09ab0c8507ee9326506214326 Mon Sep 17 00:00:00 2001
From: Eduard Kerkhoven <eduardk@chalmers.se>
Date: Fri, 17 Jul 2026 14:14:02 +0200
Subject: [PATCH] fix: metabolite names starting with a number, and a spurious
 addRxns warning

Both found by comparing the raven-toolbox test suite against RAVEN's.

constructS read the leading number of every "number name" entry as a
coefficient, so an equation using a metabolite whose name starts with a digit
was silently mis-stoichiometried: "2 oxoglutarate => succinate" produced
S = [-2 1] over a metabolite "oxoglutarate" rather than [-1 1] over
"2 oxoglutarate". When the caller supplies a metabolite list, an entry that
matches it in full is now taken as a name. A list derived from the equations
cannot disambiguate, so that path is unchanged, as is the ordinary coefficient
reading.

addRxns raised its "metabolites both as substrate and product" warning outside
any check of whether any equation had that problem, so every call warned, with
an empty list of offending reactions.
---
 manipulation/addRxns.m                 |  6 ++++--
 queries/constructS.m                   |  9 +++++++++
 testing/function_tests/tManipulation.m | 10 ++++++++++
 testing/function_tests/tQueries.m      | 18 ++++++++++++++++++
 4 files changed, 41 insertions(+), 2 deletions(-)

diff --git a/manipulation/addRxns.m b/manipulation/addRxns.m
index 8bb1cb2d..3a08b335 100755
--- a/manipulation/addRxns.m
+++ b/manipulation/addRxns.m
@@ -252,8 +252,10 @@
 %Parse the equations. This is done at this early stage since I need the
 %reversibility info
 [S, mets, badRxns, reversible]=constructS(rxnsToAdd.equations);
-EM='The following equations have one or more metabolites both as substrate and product. Only the net equations will be added:';
-warning('RAVEN:warning', '%s', ravenList(EM, rxnsToAdd.rxns(badRxns)));
+if any(badRxns)
+    EM='The following equations have one or more metabolites both as substrate and product. Only the net equations will be added:';
+    warning('RAVEN:warning', '%s', ravenList(EM, rxnsToAdd.rxns(badRxns)));
+end
 
 newModel.rev=[newModel.rev;reversible];
 newModel.rxns=[newModel.rxns;rxnsToAdd.rxns(:)];
diff --git a/queries/constructS.m b/queries/constructS.m
index ab7aa4d0..a0e6ef60 100755
--- a/queries/constructS.m
+++ b/queries/constructS.m
@@ -65,6 +65,10 @@
 equations=strtrim(equations);
 equations=fixEquations(equations);
 
+%A supplied list is authoritative, so an entry in it can be matched in full
+%before the leading number of "2 oxoglutarate" is read as a coefficient. A
+%list derived from the equations cannot disambiguate that.
+metsSupplied=~isempty(mets);
 if isempty(mets)
     mets=parseRxnEqu(equations);
 end
@@ -125,6 +129,11 @@
             %No coefficient
             coeff=1;
             name=metabolites{j};
+        elseif metsSupplied && any(strcmp(strtrim(metabolites{j}),mets))
+            %The whole entry is a known metabolite, so its leading number is
+            %part of its name ("2 oxoglutarate") rather than a coefficient
+            coeff=1;
+            name=strtrim(metabolites{j});
         else
             coeff=str2double(metabolites{j}(1:space(1)));
             
diff --git a/testing/function_tests/tManipulation.m b/testing/function_tests/tManipulation.m
index cebf6a1e..302ad3cd 100644
--- a/testing/function_tests/tManipulation.m
+++ b/testing/function_tests/tManipulation.m
@@ -85,6 +85,16 @@ function addTransportAddsRxn(testCase)
             testCase.verifyGreaterThan(numel(m2.rxns), numel(testCase.model.rxns));
         end
 
+        function addRxnsCleanEquationDoesNotWarn(testCase)
+            % The "metabolite on both sides" warning must only fire for the
+            % equations it names, not on every call.
+            rxnsToAdd.rxns      = {'newRxn'};
+            rxnsToAdd.equations = {[testCase.model.mets{1} ' => ' testCase.model.mets{2}]};
+            lastwarn('');
+            evalc('addRxns(testCase.model, rxnsToAdd, 1);');
+            testCase.verifyEmpty(strfind(lastwarn, 'both as substrate and product')); %#ok<STRIFY>
+        end
+
         function changeGrRulesUpdatesRule(testCase)
             m2 = changeGrRules(testCase.model, 'ACKr', 'b2296 and b1849', true);
             idx = strcmp(m2.rxns, 'ACKr');
diff --git a/testing/function_tests/tQueries.m b/testing/function_tests/tQueries.m
index d9800601..6b353168 100644
--- a/testing/function_tests/tQueries.m
+++ b/testing/function_tests/tQueries.m
@@ -45,6 +45,24 @@ function constructSSimple(testCase)
             testCase.verifyEqual(full(S), [-1;-1;1]);
         end
 
+        function constructSLeadingNumberIsPartOfName(testCase)
+            % A metabolite whose name starts with a number must not have that
+            % number read as a stoichiometric coefficient when the metabolite
+            % list says the whole entry is a metabolite.
+            mets = {'2 oxoglutarate';'succinate'};
+            [S, outMets] = constructS({'2 oxoglutarate => succinate'}, 'mets', mets);
+            testCase.verifyEqual(outMets, mets);
+            testCase.verifyEqual(full(S), [-1;1]);
+        end
+
+        function constructSLeadingNumberStillReadsCoefficient(testCase)
+            % With no such metabolite, the leading number is a coefficient.
+            mets = {'oxoglutarate';'succinate'};
+            [S, outMets] = constructS({'2 oxoglutarate => succinate'}, 'mets', mets);
+            testCase.verifyEqual(outMets, mets);
+            testCase.verifyEqual(full(S), [-2;1]);
+        end
+
         function getAllRxnsFromGenesType(testCase)
             % Use a reaction that has a gene association.
             withGpr = testCase.model.rxns(find(~cellfun(@isempty, ...