Compare grRules as logic in diff_models (order-insensitive)

CHANGELOG.md

@@ -4,6 +4,16 @@ Milestones in the raven-toolbox port. For function-level status see
 [docs/raven_migration.md](https://github.com/SysBioChalmers/raven-toolbox/blob/develop/docs/reference/migration.md); for open work see
 [docs/todo.md](https://github.com/SysBioChalmers/raven-toolbox/blob/develop/docs/reference/todo.md).
 
+## Unreleased
+
+* **`diff_models` compares grRules as logic, not text.** The GPR check now DNF-expands each rule (via the
+  existing `manipulation.gpr_to_dnf`), sorts the genes within each isozyme clause and sorts the clauses, so
+  operand order no longer registers as a difference: `a and b` == `b and a` and `a or b` == `b or a`. The
+  previous heuristic only lowercased and collapsed whitespace, so it flagged logically identical rules that
+  differed only in operand order. This brings `diff_models` in line with MATLAB RAVEN's `diffModels`
+  ([RAVEN #686](https://github.com/SysBioChalmers/RAVEN/pull/686)); a rule cobra cannot parse falls back to
+  the old string comparison, so malformed rules are still compared rather than silently equated.
+
 ## 0.3.0 — 2026-07-16
 
 Compartment localisation and per-reaction confidence tracking, new gap-filling and flux-sampling

src/raven_toolbox/comparison/diff.py

@@ -10,8 +10,8 @@
 Diff scope: reaction / metabolite / gene id sets, stoichiometry (within
 tolerance), bounds, objective coefficients, GPR rules, metabolite
 formula/charge/compartment, and a configurable set of annotation keys.
-Formatting differences (key ordering, whitespace, float repr) are
-explicitly **not** failures.
+Formatting differences (key ordering, whitespace, float repr, and GPR
+operand order — ``a and b`` == ``b and a``) are explicitly **not** failures.
 """
 from __future__ import annotations
 
@@ -20,6 +20,8 @@
 
 import cobra
 
+from raven_toolbox.manipulation.expand import gpr_to_dnf
+
 # Annotation keys checked by default. Add via ``extra_annotations`` or
 # remove via ``ignore_annotations`` in :func:`diff_models`.
 DEFAULT_ANNOTATION_KEYS: tuple[str, ...] = (
@@ -152,10 +154,9 @@ def _diff_reactions(
                 f"{rxn_id}: objective A={ra.objective_coefficient} B={rb.objective_coefficient}",
             )
 
-        ga = _normalise_gpr(ra.gene_reaction_rule)
-        gb = _normalise_gpr(rb.gene_reaction_rule)
-        if ga != gb:
-            _push(diffs, counters, "gpr", cap, f"{rxn_id}: GPR A={ga!r} B={gb!r}")
+        if _canonical_gpr(ra) != _canonical_gpr(rb):
+            _push(diffs, counters, "gpr", cap,
+                  f"{rxn_id}: GPR A={ra.gene_reaction_rule!r} B={rb.gene_reaction_rule!r}")
 
         _diff_annotations(
             f"rxn {rxn_id}", ra.annotation, rb.annotation,
@@ -218,16 +219,30 @@ def _diff_annotations(
             _push(diffs, counters, "anno", cap, f"{label}.annotation[{k!r}]: A={va} B={vb}")
 
 
-def _normalise_gpr(rule: str) -> str:
-    """Lowercase + collapse whitespace.
+def _canonical_gpr(reaction: cobra.Reaction) -> str:
+    """Canonical, order-insensitive form of a reaction's GPR, for logic-level comparison.
+
+    DNF-expand the parsed GPR, then sort the genes within each isozyme clause and sort the clauses, so
+    operand order never registers as a difference: ``a and b`` == ``b and a`` and ``a or b`` ==
+    ``b or a``. Mirrors MATLAB RAVEN's ``diffModels`` (`RAVEN #686
+    <https://github.com/SysBioChalmers/RAVEN/pull/686>`_), which compares grRules as logic rather than
+    text. Gene ids are lowercased and duplicate genes/clauses collapse, preserving the formatting-drift
+    tolerance the previous string heuristic gave.
 
-    A more robust comparator would parse to a GPR AST and compare
-    structures; this is the cheap heuristic that catches the formatting
-    drift we see between different SBML writers.
+    Falls back to that whitespace/lowercase heuristic for a rule cobra could not parse (``gpr.body`` is
+    ``None`` while the rule string is non-empty), so a malformed rule is still compared rather than
+    silently equated to the empty rule.
     """
-    if not rule:
-        return ""
-    return " ".join(rule.lower().split())
+    gpr = reaction.gpr
+    if gpr is not None and gpr.body is not None:
+        try:
+            clauses = gpr_to_dnf(gpr)
+        except ValueError:
+            clauses = None
+        if clauses is not None:
+            canon = sorted({tuple(sorted({g.lower() for g in clause})) for clause in clauses})
+            return " | ".join(" & ".join(clause) for clause in canon)
+    return " ".join((reaction.gene_reaction_rule or "").lower().split())
 
 
 def _normalise_annotation_value(v):

tests/test_comparison_diff.py

@@ -5,7 +5,7 @@
 import pytest
 
 from raven_toolbox.comparison import DiffReport, diff_models
-from raven_toolbox.comparison.diff import _normalise_gpr
+from raven_toolbox.comparison.diff import _canonical_gpr
 
 
 def _mini_model(model_id: str = "m") -> cobra.Model:
@@ -98,16 +98,55 @@ def test_extra_annotations_picked_up():
     assert not diff_models(a, b, extra_annotations={"custom-key"}).equal
 
 
+def _canon(rule: str) -> str:
+    r = cobra.Reaction("r")
+    r.gene_reaction_rule = rule
+    return _canonical_gpr(r)
+
+
+@pytest.mark.parametrize(
+    "ga, gb",
+    [
+        ("A and B", "a AND b"),                 # case + AND spelling
+        ("A  and   B", "A and B"),              # whitespace
+        ("(A or B) and C", "(a OR b) AND c"),   # case, nested
+        ("A and B", "B and A"),                 # AND operand order  (the back-port)
+        ("A or B", "B or A"),                   # OR isozyme order   (the back-port)
+        ("(A or B) and C", "C and (B or A)"),   # nested, reordered at both levels
+        ("A and A", "A"),                       # duplicate gene collapses
+    ],
+)
+def test_canonical_gpr_is_order_insensitive(ga, gb):
+    assert _canon(ga) == _canon(gb)
+
+
 @pytest.mark.parametrize(
     "ga, gb",
     [
-        ("A and B", "a AND b"),
-        ("A  and   B", "A and B"),
-        ("(A or B) and C", "(a OR b) AND c"),
+        ("A and B", "A or B"),   # AND vs OR is a real logic difference, not formatting
+        ("A and B", "A and C"),  # a different gene really differs
     ],
 )
-def test_gpr_normalisation(ga, gb):
-    assert _normalise_gpr(ga) == _normalise_gpr(gb)
+def test_canonical_gpr_keeps_real_differences(ga, gb):
+    assert _canon(ga) != _canon(gb)
+
+
+def test_diff_models_ignores_gpr_operand_order():
+    # Two models identical but for GPR operand order must compare equal — the RAVEN diffModels
+    # behaviour this back-port adds. The string heuristic used to flag this as a GPR difference.
+    a = _mini_model("a")  # r1 GPR is "g1 AND g2"
+    b = _mini_model("b")
+    b.reactions.r1.gene_reaction_rule = "g2 and g1"
+    assert diff_models(a, b).equal
+
+
+def test_diff_models_flags_real_gpr_difference():
+    a = _mini_model("a")  # r1 GPR is "g1 AND g2"
+    b = _mini_model("b")
+    b.reactions.r1.gene_reaction_rule = "g1 or g2"  # AND -> OR is genuine
+    report = diff_models(a, b)
+    assert not report.equal
+    assert any("GPR" in d for d in report.differences)
 
 
 def test_max_per_category_truncates():