Decline a node grounding the graph already carries (#361)

history/infrastructure/ground-nodes-duplicate-guard/2026-08-08T043349Z-claude-code-d0a8f5.yaml

@@ -0,0 +1,45 @@
+history_version: 1
+target:
+  kind: infrastructure
+  path: scripts/ground_causal_nodes.py
+  slug: ground-nodes-duplicate-guard
+session:
+  id: 2026-08-08T043349Z-claude-code-d0a8f5
+  timestamp: '2026-08-08T04:33:49Z'
+  actors:
+  - type: ai_agent
+    name: claude-code
+links:
+  issues:
+  - https://github.com/CultureBotAI/TraitMech/issues/361
+events:
+- type: EDIT
+  outcome: changed
+  sections:
+  - grounding
+  summary: Decline a node grounding the graph already carries, instead of writing the duplicate
+  details: 'ground_causal_nodes.py wrote any mapped CURIE into any ungrounded node, including one another
+    node in the same graph already carried - which is exactly the shape audit-graphs reports as DUPLICATE_GROUNDING.
+    The writer manufactured findings the auditor then reported. Concretely: #352 removed GO:0004096 from
+    the catalase node and GO:0009039 from urease, but mappings/node_grounding.tsv still maps both proteins
+    to those GO ACTIVITY terms, so the next ''just ground-nodes --apply'' re-created both duplicates (verified:
+    2 files modifiable, 2 nodes grounded, exactly the two that had been ungrounded). Deleting the mapping
+    rows would over-correct, because protein -> GO-activity is the corpus''s accepted shorthand wherever
+    the graph does not also model the function as its own node, and 72 GENE_OR_PROTEIN nodes rely on it.
+    So the guard lives in the writer: ground_nodes_in_doc now tracks the CURIEs already present in each
+    graph, seeded from existing groundings and updated as it writes, and declines a candidate that collides.
+    Declined nodes are reported ONLY through the declined counter and are deliberately kept OUT of residual:
+    the residual TSV reads like a census of ungrounded nodes but its consumers treat it as a work queue
+    of labels still needing a mapping. match_uniprot_to_proteins.py''s load_target_labels takes every
+    GENE_OR_PROTEIN row from it and under --apply appends a UniProtKB row to mappings/node_grounding.tsv
+    with no existing-row check, so listing catalase there would earn it a second mapping row conflicting
+    with its GO one, and load_mapping raises on exactly that - taking out just ground-nodes and the derived-reports
+    freshness check with it (caught in review of PR 362). The semantics settle it independently of the
+    breakage: a declined node is not awaiting a grounding, it has one that was deliberately withheld,
+    so proposing a UniProt accession for it would be wrong even if nothing downstream broke. A consequence
+    worth stating: the residual TSV is byte-identical to before in every case, so no consumer is perturbed
+    at all. The summary prints what was withheld and why. Scoped per graph rather than per record, since
+    DUPLICATE_GROUNDING is a within-graph defect. Verified against the fix/352 branch state in memory:
+    grounded 2 -> 0, declined 2. On main the corpus is unaffected - 0 modifiable, residual TSV byte-identical.
+    ground_causal_predicates.py has no analogous gap: nothing flags a duplicate predicate_id and nothing
+    should, since many edges legitimately share a predicate. 524 tests pass (5 new), qc green.'

scripts/ground_causal_nodes.py

@@ -101,9 +101,34 @@ def load_mapping(path: Path) -> dict[MappingKey, tuple[str, str]]:
 def ground_nodes_in_doc(
     doc: dict[str, Any],
     mapping: dict[MappingKey, tuple[str, str]],
-) -> tuple[int, Counter, Counter, Counter]:
+) -> tuple[int, Counter, Counter, Counter, Counter]:
     """Mutate ``doc`` in place, grounding empty grounding slots.
 
+    A candidate CURIE already carried by another node in the SAME graph is
+    declined rather than written, because ``audit-graphs`` reports exactly
+    that shape as DUPLICATE_GROUNDING -- so writing it would mean this script
+    manufacturing a finding the audit then reports against us. #352 burned
+    down three such duplicates by hand; two of them (`catalase` GO:0004096,
+    `urease` GO:0009039) are still live rows in the mapping table, because the
+    protein-to-GO-activity shorthand is correct wherever the graph does NOT
+    also model the function as its own node -- 72 GENE_OR_PROTEIN nodes rely
+    on it. The row is not the defect; writing it into a graph that already
+    says the same thing is. Without this guard the next ``--apply`` re-created
+    both duplicates (#361).
+
+    A declined node is NOT counted into ``residual``, and that is deliberate.
+    The residual TSV reads like a census of ungrounded nodes but its consumers
+    treat it as a WORK QUEUE of labels still needing a mapping:
+    match_uniprot_to_proteins.py's ``load_target_labels`` takes every
+    GENE_OR_PROTEIN row from it and, under ``--apply``, appends a UniProtKB row
+    to mappings/node_grounding.tsv with no existing-row check. Listing
+    `catalase` there would earn it a second, conflicting mapping row, and
+    ``load_mapping`` raises on exactly that -- taking out ``just ground-nodes``
+    and the freshness check with it (#362 review). A declined node is not
+    awaiting a grounding; it has one, deliberately withheld. Proposing a
+    UniProt accession for it would be actively wrong. It is reported through
+    ``declined`` instead, which is what that counter is for.
+
     Returns
     -------
     grounded : int
@@ -112,16 +137,28 @@ def ground_nodes_in_doc(
         Map from target CURIE → grounded-node count.
     residual : Counter
         (label, node_type) → count of nodes that had no mapping entry.
+        Declined nodes are excluded; see above.
     grounded_keys : Counter
         (label, node_type) → count of nodes that **were** grounded.
         Caller needs this to re-classify them as residual if a later
         validation step rejects the file.
+    declined : Counter
+        (label, node_type, curie) → count of nodes whose mapped CURIE was
+        withheld because the graph already carried it.
     """
     grounded = 0
     per_curie: Counter = Counter()
     residual: Counter = Counter()
     grounded_keys: Counter = Counter()
+    declined: Counter = Counter()
     for graph in (doc.get("causal_graphs") or []):
+        # Seeded from what the graph already carries, then updated as we go,
+        # so two ungrounded nodes mapping to one CURIE cannot both take it.
+        taken = {
+            (n.get("grounding") or "").strip()
+            for n in (graph.get("nodes") or [])
+            if (n.get("grounding") or "").strip()
+        }
         for node in (graph.get("nodes") or []):
             label = (node.get("label") or "").strip()
             node_type = (node.get("node_type") or "").strip()
@@ -133,13 +170,17 @@ def ground_nodes_in_doc(
             key = (label.lower(), node_type)
             if key in mapping:
                 curie, _src = mapping[key]
+                if curie in taken:
+                    declined[(label.lower(), node_type, curie)] += 1
+                    continue
                 node["grounding"] = curie
+                taken.add(curie)
                 grounded += 1
                 per_curie[curie] += 1
                 grounded_keys[key] += 1
             else:
                 residual[key] += 1
-    return grounded, per_curie, residual, grounded_keys
+    return grounded, per_curie, residual, grounded_keys, declined
 
 
 def main() -> int:
@@ -166,6 +207,8 @@ def main() -> int:
     per_curie_total: Counter = Counter()
     residual_total: Counter = Counter()
     residual_examples: dict[MappingKey, list[str]] = defaultdict(list)
+    declined_total: Counter = Counter()
+    declined_examples: dict[tuple[str, str, str], list[str]] = defaultdict(list)
 
     for path in files:
         try:
@@ -176,14 +219,21 @@ def main() -> int:
         if not isinstance(doc, dict):
             continue
 
-        grounded, per_curie, residual, grounded_keys = ground_nodes_in_doc(doc, mapping)
+        grounded, per_curie, residual, grounded_keys, declined = ground_nodes_in_doc(doc, mapping)
 
         def _record_residual(keys_counter: Counter) -> None:
             for key, n in keys_counter.items():
                 residual_total[key] += n
                 if len(residual_examples[key]) < 3:
                     residual_examples[key].append(str(path.relative_to(REPO_ROOT)))
 
+        # Recorded regardless of whether the file is written: a declined
+        # grounding is a fact about the corpus, not about this run's mode.
+        for dkey, n in declined.items():
+            declined_total[dkey] += n
+            if len(declined_examples[dkey]) < 3:
+                declined_examples[dkey].append(str(path.relative_to(REPO_ROOT)))
+
         if grounded == 0:
             _record_residual(residual)
             continue
@@ -249,6 +299,14 @@ def _record_residual(keys_counter: Counter) -> None:
         print("  by target CURIE:", file=sys.stderr)
         for curie, n in per_curie_total.most_common():
             print(f"    {curie:30s} {n:>6d}", file=sys.stderr)
+    if declined_total:
+        print(f"  declined (already in graph): {sum(declined_total.values())}", file=sys.stderr)
+        print("    withheld because another node in the same graph already carries the CURIE,",
+              file=sys.stderr)
+        print("    which audit-graphs would report as DUPLICATE_GROUNDING (#361):", file=sys.stderr)
+        for (label, node_type, curie), n in declined_total.most_common():
+            examples = ", ".join(declined_examples[(label, node_type, curie)])
+            print(f"    {label} ({node_type}) -> {curie}  ×{n}  [{examples}]", file=sys.stderr)
     if not args.apply and files_modified:
         print("", file=sys.stderr)
         print("  Re-run with --apply to write the changes.", file=sys.stderr)

tests/test_ground_causal_nodes.py

@@ -120,7 +120,7 @@ def test_ground_nodes_basic():
         ("molecular oxygen", "CHEMICAL"): ("CHEBI:15379", "CHEBI"),
         ("photosynthesis", "BIOLOGICAL_PROCESS"): ("GO:0015979", "GO"),
     }
-    grounded, per_curie, residual, grounded_keys = ground_nodes_in_doc(doc, mapping)
+    grounded, per_curie, residual, grounded_keys, _ = ground_nodes_in_doc(doc, mapping)
     assert grounded == 2
     assert per_curie == Counter({"CHEBI:15379": 1, "GO:0015979": 1})
     assert residual == Counter()
@@ -141,7 +141,7 @@ def test_ground_nodes_skips_existing_grounding():
         {"node_id": "b", "label": "molecular oxygen", "node_type": "CHEMICAL"},
     ])
     mapping = {("molecular oxygen", "CHEMICAL"): ("CHEBI:15379", "CHEBI")}
-    grounded, _, _, _ = ground_nodes_in_doc(doc, mapping)
+    grounded, _, _, _, _ = ground_nodes_in_doc(doc, mapping)
     assert grounded == 1
     nodes = doc["causal_graphs"][0]["nodes"]
     assert nodes[0]["grounding"] == "CHEBI:99999"
@@ -159,7 +159,7 @@ def test_ground_nodes_node_type_keyed_lookup():
     mapping = {
         ("terminal electron acceptor", "CHEMICAL"): ("METPO:1007504", "METPO"),
     }
-    grounded, _, residual, _ = ground_nodes_in_doc(doc, mapping)
+    grounded, _, residual, _, _ = ground_nodes_in_doc(doc, mapping)
     assert grounded == 1
     assert residual == Counter({("terminal electron acceptor", "PATHWAY"): 1})
 
@@ -170,7 +170,7 @@ def test_ground_nodes_idempotent_second_pass():
     ])
     mapping = {("photosynthesis", "BIOLOGICAL_PROCESS"): ("GO:0015979", "GO")}
     ground_nodes_in_doc(doc, mapping)
-    grounded2, _, residual2, grounded_keys2 = ground_nodes_in_doc(doc, mapping)
+    grounded2, _, residual2, grounded_keys2, _ = ground_nodes_in_doc(doc, mapping)
     assert grounded2 == 0
     assert residual2 == Counter()
     assert grounded_keys2 == Counter()
@@ -182,7 +182,7 @@ def test_ground_nodes_skips_nodes_without_label_or_type():
         {"node_id": "b", "label": "x"},                              # no node_type
         {"node_id": "c", "label": "", "node_type": "CHEMICAL"},      # empty label
     ])
-    grounded, _, residual, _ = ground_nodes_in_doc(doc, {("x", "CHEMICAL"): ("X:1", "X")})
+    grounded, _, residual, _, _ = ground_nodes_in_doc(doc, {("x", "CHEMICAL"): ("X:1", "X")})
     assert grounded == 0
     assert residual == Counter()
 
@@ -196,7 +196,7 @@ def test_ground_nodes_grounded_keys_separable_from_residual():
         {"node_id": "b", "label": "unmapped thing", "node_type": "CHEMICAL"},
     ])
     mapping = {("molecular oxygen", "CHEMICAL"): ("CHEBI:15379", "CHEBI")}
-    grounded, _, residual, grounded_keys = ground_nodes_in_doc(doc, mapping)
+    grounded, _, residual, grounded_keys, _ = ground_nodes_in_doc(doc, mapping)
     assert grounded == 1
     assert residual == Counter({("unmapped thing", "CHEMICAL"): 1})
     assert grounded_keys == Counter({("molecular oxygen", "CHEMICAL"): 1})
@@ -206,3 +206,111 @@ def test_ground_nodes_grounded_keys_separable_from_residual():
         ("molecular oxygen", "CHEMICAL"): 1,
         ("unmapped thing", "CHEMICAL"): 1,
     })
+
+
+# ------------------------------------------------- duplicate-grounding guard (#361)
+
+
+def test_ground_nodes_declines_curie_already_in_graph():
+    """The exact shape #352 burned down by hand: a protein node and the
+    function node it enables, where the mapping table sends the protein to
+    the function's GO ACTIVITY term. Writing it would re-create the
+    DUPLICATE_GROUNDING that audit-graphs reports."""
+    doc = _doc_with_nodes([
+        {"node_id": "catalase_function", "label": "catalase activity",
+         "node_type": "MOLECULAR_FUNCTION", "grounding": "GO:0004096"},
+        {"node_id": "catalase", "label": "catalase", "node_type": "GENE_OR_PROTEIN"},
+    ])
+    mapping = {("catalase", "GENE_OR_PROTEIN"): ("GO:0004096", "GO")}
+    grounded, per_curie, residual, grounded_keys, declined = ground_nodes_in_doc(doc, mapping)
+
+    assert grounded == 0
+    assert per_curie == Counter()
+    assert grounded_keys == Counter()
+    assert declined == Counter({("catalase", "GENE_OR_PROTEIN", "GO:0004096"): 1})
+    assert "grounding" not in doc["causal_graphs"][0]["nodes"][1]
+    # NOT residual -- see the next test for why that distinction is load-bearing.
+    assert residual == Counter()
+
+
+def test_declined_nodes_stay_out_of_residual():
+    """The residual TSV is a WORK QUEUE, not a census (#362 review).
+
+    match_uniprot_to_proteins.py's load_target_labels() takes every
+    GENE_OR_PROTEIN row from reports/node_grounding_residual.tsv and, under
+    --apply, appends a UniProtKB row to mappings/node_grounding.tsv with no
+    existing-row check. A declined node listed there would earn `catalase` a
+    second mapping row conflicting with its GO one, and load_mapping() raises
+    on exactly that -- taking out `just ground-nodes` and the freshness check.
+    A declined node is not awaiting a grounding; it has one, withheld.
+    """
+    doc = _doc_with_nodes([
+        {"node_id": "fn", "label": "catalase activity",
+         "node_type": "MOLECULAR_FUNCTION", "grounding": "GO:0004096"},
+        {"node_id": "prot", "label": "catalase", "node_type": "GENE_OR_PROTEIN"},
+        {"node_id": "other", "label": "genuinely unmapped", "node_type": "GENE_OR_PROTEIN"},
+    ])
+    mapping = {("catalase", "GENE_OR_PROTEIN"): ("GO:0004096", "GO")}
+    _, _, residual, _, declined = ground_nodes_in_doc(doc, mapping)
+
+    # Only the genuinely unmapped label is a target for the UniProt matcher.
+    assert residual == Counter({("genuinely unmapped", "GENE_OR_PROTEIN"): 1})
+    assert ("catalase", "GENE_OR_PROTEIN") not in residual
+    assert declined == Counter({("catalase", "GENE_OR_PROTEIN", "GO:0004096"): 1})
+
+
+def test_ground_nodes_declines_second_node_mapping_to_same_curie():
+    """`taken` is updated as we go, not just seeded once -- otherwise two
+    ungrounded nodes sharing a mapped CURIE would both take it and produce
+    the duplicate this guard exists to prevent."""
+    doc = _doc_with_nodes([
+        {"node_id": "a", "label": "catalase", "node_type": "GENE_OR_PROTEIN"},
+        {"node_id": "b", "label": "catalase (KatA)", "node_type": "GENE_OR_PROTEIN"},
+    ])
+    mapping = {
+        ("catalase", "GENE_OR_PROTEIN"): ("GO:0004096", "GO"),
+        ("catalase (kata)", "GENE_OR_PROTEIN"): ("GO:0004096", "GO"),
+    }
+    grounded, _, _, _, declined = ground_nodes_in_doc(doc, mapping)
+
+    assert grounded == 1
+    assert declined == Counter({("catalase (kata)", "GENE_OR_PROTEIN", "GO:0004096"): 1})
+    nodes = doc["causal_graphs"][0]["nodes"]
+    assert nodes[0]["grounding"] == "GO:0004096"
+    assert "grounding" not in nodes[1]
+
+
+def test_ground_nodes_guard_is_per_graph_not_per_record():
+    """DUPLICATE_GROUNDING is scoped to one graph, so the same CURIE in a
+    DIFFERENT graph of the same record is not a duplicate and must still be
+    written -- otherwise the guard would suppress legitimate groundings."""
+    doc = {"causal_graphs": [
+        {"nodes": [{"node_id": "a", "label": "catalase activity",
+                    "node_type": "MOLECULAR_FUNCTION", "grounding": "GO:0004096"}]},
+        {"nodes": [{"node_id": "b", "label": "catalase",
+                    "node_type": "GENE_OR_PROTEIN"}]},
+    ]}
+    mapping = {("catalase", "GENE_OR_PROTEIN"): ("GO:0004096", "GO")}
+    grounded, _, _, _, declined = ground_nodes_in_doc(doc, mapping)
+
+    assert grounded == 1
+    assert declined == Counter()
+    assert doc["causal_graphs"][1]["nodes"][0]["grounding"] == "GO:0004096"
+
+
+def test_ground_nodes_guard_leaves_the_72_protein_shorthand_alone():
+    """The mapping rows are NOT the defect: where a graph models the protein
+    but not its function, protein -> GO activity is the corpus's accepted
+    shorthand and must still be written (#361)."""
+    doc = _doc_with_nodes([
+        {"node_id": "catalase", "label": "catalase", "node_type": "GENE_OR_PROTEIN"},
+        {"node_id": "h2o2", "label": "hydrogen peroxide", "node_type": "CHEMICAL",
+         "grounding": "CHEBI:16240"},
+    ])
+    mapping = {("catalase", "GENE_OR_PROTEIN"): ("GO:0004096", "GO")}
+    grounded, per_curie, _, _, declined = ground_nodes_in_doc(doc, mapping)
+
+    assert grounded == 1
+    assert declined == Counter()
+    assert per_curie == Counter({"GO:0004096": 1})
+    assert doc["causal_graphs"][0]["nodes"][0]["grounding"] == "GO:0004096"