From 73d41bdfb5f99093e992db04fcd1cca85395e3a2 Mon Sep 17 00:00:00 2001 From: "marcin p. joachimiak" <4625870+realmarcin@users.noreply.github.com> Date: Fri, 7 Aug 2026 21:34:07 -0700 Subject: [PATCH 1/3] Decline a node grounding the graph already carries (#361) ground_causal_nodes.py wrote any mapped CURIE into any ungrounded node, including one that 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 against us. Concretely: #352 removed GO:0004096 from the `catalase` node and GO:0009039 from `urease`, on the argument that a protein is not its activity and the graphs already say so with `catalase -enables-> catalase_function`. 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. Measured on that branch's corpus: files modifiable 2, nodes grounded 2 - exactly the two that had just been ungrounded. DELETING THE MAPPING ROWS WOULD OVER-CORRECT. #352's own reasoning says the protein -> GO-activity shorthand is fine wherever the graph does not also model the function as its own node, and 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. So the guard belongs in the writer, not the mapping table. ground_nodes_in_doc now tracks the CURIEs present in each graph - seeded from existing groundings and updated as it writes, so two ungrounded nodes mapping to one CURIE cannot both take it - and declines a candidate that collides. Declined nodes stay ungrounded, so they are counted into `residual` too: that report describes what the corpus actually looks like, and an ungrounded node belongs there however it got that way. The summary prints what was withheld and why rather than silently skipping it. Scoped per graph, not per record, because DUPLICATE_GROUNDING is a within-graph defect - the same CURIE in a different graph of the same record is not a duplicate and is still written. Verified against the fix/352 branch state: 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 one. 523 tests pass (4 new) - just qc green - ruff clean. Co-Authored-By: Claude Opus 5 --- ...2026-08-08T043349Z-claude-code-d0a8f5.yaml | 38 ++++++++ scripts/ground_causal_nodes.py | 58 +++++++++++- tests/test_ground_causal_nodes.py | 94 +++++++++++++++++-- 3 files changed, 180 insertions(+), 10 deletions(-) create mode 100644 history/infrastructure/ground-nodes-duplicate-guard/2026-08-08T043349Z-claude-code-d0a8f5.yaml diff --git a/history/infrastructure/ground-nodes-duplicate-guard/2026-08-08T043349Z-claude-code-d0a8f5.yaml b/history/infrastructure/ground-nodes-duplicate-guard/2026-08-08T043349Z-claude-code-d0a8f5.yaml new file mode 100644 index 00000000..43edb29c --- /dev/null +++ b/history/infrastructure/ground-nodes-duplicate-guard/2026-08-08T043349Z-claude-code-d0a8f5.yaml @@ -0,0 +1,38 @@ +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 stay ungrounded so they + are counted into residual as well, and 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. 523 tests pass (4 new), qc green.' diff --git a/scripts/ground_causal_nodes.py b/scripts/ground_causal_nodes.py index 1e7a1e65..6e90e4d5 100644 --- a/scripts/ground_causal_nodes.py +++ b/scripts/ground_causal_nodes.py @@ -101,9 +101,25 @@ 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). + + Declined nodes stay ungrounded on disk, so they are also counted into + ``residual`` -- the residual TSV describes what the corpus actually looks + like, and an ungrounded node belongs there however it got that way. + Returns ------- grounded : int @@ -111,17 +127,29 @@ def ground_nodes_in_doc( per_curie : Counter Map from target CURIE → grounded-node count. residual : Counter - (label, node_type) → count of nodes that had no mapping entry. + (label, node_type) → count of nodes left ungrounded, whether because + no mapping entry exists or because the mapped CURIE was declined. 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 +161,18 @@ 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 + residual[key] += 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 +199,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,7 +211,7 @@ 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(): @@ -184,6 +219,13 @@ def _record_residual(keys_counter: Counter) -> None: 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 +291,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) diff --git a/tests/test_ground_causal_nodes.py b/tests/test_ground_causal_nodes.py index 3b11c8c6..0a7299dd 100644 --- a/tests/test_ground_causal_nodes.py +++ b/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,85 @@ 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}) + # Still ungrounded on disk, so it belongs in the residual report. + assert residual == Counter({("catalase", "GENE_OR_PROTEIN"): 1}) + assert "grounding" not in doc["causal_graphs"][0]["nodes"][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" From ebfbed2205396a0de76595684e957f10d6b5a07c Mon Sep 17 00:00:00 2001 From: "marcin p. joachimiak" <4625870+realmarcin@users.noreply.github.com> Date: Fri, 7 Aug 2026 21:54:48 -0700 Subject: [PATCH 2/3] Keep declined nodes out of the residual TSV (#362 review) The first pass counted a declined node into `residual` on the reasoning that the report describes what the corpus looks like and an ungrounded node belongs there however it got that way. Review showed that is the wrong model of the file: its consumers treat it as a WORK QUEUE, not a census. 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. So once #360 lands, `catalase` would appear in the residual TSV, earn a second mapping row conflicting with its GO:0004096 one, and load_mapping() raises on exactly that shape - taking out `just ground-nodes` and the derived-reports freshness check with it. .claude/commands/ground-or-propose-metpo.md reads the same file the same way. The semantics settle it independently of the breakage: a declined node is NOT awaiting a grounding. It has one, deliberately withheld. Proposing a UniProt accession for it would be actively wrong even if nothing downstream broke. So declined nodes are reported only through the `declined` counter, which is what that counter is for. This also means the residual TSV is byte-identical to before in every case, so no consumer is perturbed at all. Verified on #360's corpus state: catalase and urease are declined and absent from the residual TSV's GENE_OR_PROTEIN rows - only `oxyr`, a genuinely unmapped protein, remains a target for the matcher. 524 tests pass - just qc green - ruff clean. Co-Authored-By: Claude Opus 5 --- scripts/ground_causal_nodes.py | 20 ++++++++++++++------ tests/test_ground_causal_nodes.py | 30 ++++++++++++++++++++++++++++-- 2 files changed, 42 insertions(+), 8 deletions(-) diff --git a/scripts/ground_causal_nodes.py b/scripts/ground_causal_nodes.py index 6e90e4d5..716f2dd8 100644 --- a/scripts/ground_causal_nodes.py +++ b/scripts/ground_causal_nodes.py @@ -116,9 +116,18 @@ def ground_nodes_in_doc( says the same thing is. Without this guard the next ``--apply`` re-created both duplicates (#361). - Declined nodes stay ungrounded on disk, so they are also counted into - ``residual`` -- the residual TSV describes what the corpus actually looks - like, and an ungrounded node belongs there however it got that way. + 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 ------- @@ -127,8 +136,8 @@ def ground_nodes_in_doc( per_curie : Counter Map from target CURIE → grounded-node count. residual : Counter - (label, node_type) → count of nodes left ungrounded, whether because - no mapping entry exists or because the mapped CURIE was declined. + (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 @@ -163,7 +172,6 @@ def ground_nodes_in_doc( curie, _src = mapping[key] if curie in taken: declined[(label.lower(), node_type, curie)] += 1 - residual[key] += 1 continue node["grounding"] = curie taken.add(curie) diff --git a/tests/test_ground_causal_nodes.py b/tests/test_ground_causal_nodes.py index 0a7299dd..70b9af63 100644 --- a/tests/test_ground_causal_nodes.py +++ b/tests/test_ground_causal_nodes.py @@ -228,9 +228,35 @@ def test_ground_nodes_declines_curie_already_in_graph(): assert per_curie == Counter() assert grounded_keys == Counter() assert declined == Counter({("catalase", "GENE_OR_PROTEIN", "GO:0004096"): 1}) - # Still ungrounded on disk, so it belongs in the residual report. - assert residual == Counter({("catalase", "GENE_OR_PROTEIN"): 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(): From 886b20bf6063876abef02cf4851da2a499a704ac Mon Sep 17 00:00:00 2001 From: "marcin p. joachimiak" <4625870+realmarcin@users.noreply.github.com> Date: Fri, 7 Aug 2026 22:02:50 -0700 Subject: [PATCH 3/3] Correct the history record to match the shipped behaviour (#362 review) The record was written against the first pass and still said "declined nodes stay ungrounded so they are counted into residual as well" - the exact behaviour ebfbed22 removed, and the one that would re-arm the mapping-conflict break if someone acted on it. It is the only committed explanation of why the code is shaped this way, so it saying the opposite of the code is worse than it saying nothing. Rewritten to state what ships and why: declined nodes are reported only via the `declined` counter and kept OUT of residual, because the residual TSV's consumers treat it as a work queue rather than a census. Test tally corrected 523/4-new -> 524/5-new. Co-Authored-By: Claude Opus 5 --- ...2026-08-08T043349Z-claude-code-d0a8f5.yaml | 43 +++++++++++-------- 1 file changed, 25 insertions(+), 18 deletions(-) diff --git a/history/infrastructure/ground-nodes-duplicate-guard/2026-08-08T043349Z-claude-code-d0a8f5.yaml b/history/infrastructure/ground-nodes-duplicate-guard/2026-08-08T043349Z-claude-code-d0a8f5.yaml index 43edb29c..1639122c 100644 --- a/history/infrastructure/ground-nodes-duplicate-guard/2026-08-08T043349Z-claude-code-d0a8f5.yaml +++ b/history/infrastructure/ground-nodes-duplicate-guard/2026-08-08T043349Z-claude-code-d0a8f5.yaml @@ -18,21 +18,28 @@ events: 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 stay ungrounded so they - are counted into residual as well, and 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. 523 tests pass (4 new), qc green.' + 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.'