Support OBO edit files + ontology-neutral wording

clara_workflow/agent_instructions.md

@@ -3,6 +3,11 @@
 You are verifying factual claims implied by routed CLARA targets against their
 cited references.
 
+Ontology-agnostic: this runs against any OBO Foundry ontology's edit file, not
+just CL. `CL_4033094`-style ids and "cell type" below are illustrative only —
+substitute the term's own prefix and kind of entity (e.g. an anatomical
+structure in UBERON) throughout.
+
 Work one term at a time, but process **all routed targets for that term**
 together.
 
@@ -107,9 +112,9 @@ For `ntr` targets:
    `definition_changes`).
 2. Decompose its `value` into atomic assertions.
 3. Tag each assertion as:
-   - `core` — the subject is the cell type itself
+   - `core` — the subject is the term itself
    - `background` — the subject is a molecule, gene, process, or external
-     concept rather than the cell type
+     concept rather than the term itself
 4. Also take each `added` change from `relationship_changes` and convert it
    into one atomic `core` assertion.
 
@@ -238,7 +243,7 @@ For every decomposable `textual_changes` entry:
 - One fact per assertion.
 - Split conjunctions ("secretes X, Y, and Z" → three assertions).
 - Split clauses that bundle identity + location + function.
-- Preserve the cell-type subject in every `core` assertion so it stands alone.
+- Preserve the term's subject in every `core` assertion so it stands alone.
 - Strip hedges ("crucial for", "key") but keep the factual core.
 - Do not invent claims the text does not make.
 - Use only the routed textual change's `value` as the prose source of truth.

clara_workflow/stage1/extract.py

@@ -18,6 +18,7 @@
 import dataclasses
 import json
 import os
+import re
 import shutil
 import subprocess
 import sys
@@ -43,6 +44,27 @@ def _git_show(repo: Path, ref: str, path: str, out: Path) -> None:
         )
 
 
+# Matches an OBO `import:` line, or an OWL functional-syntax `Import(<iri>)`
+# line (the two serializations edit files are known to use).
+_IMPORT_LINE_RE = re.compile(r"^(import:\s|Import\()")
+
+
+def _strip_imports(text: str) -> str:
+    """Drop owl:imports declarations from edit-file text.
+
+    Used only as a fallback when ROBOT fails to resolve the real import
+    graph (e.g. a moved PURL) — not the default path. Stripping unconditionally
+    would also strip label resolution for entities defined only in an import
+    (e.g. BFO's `part of`, or an UBERON class referenced from cl-edit.owl),
+    degrading axiom text like `part of some mouth mucosa` down to
+    `BFO_0000050 some UBERON_0003729`. That's a real quality loss for the
+    downstream verification agent, so this is a last resort, not a default.
+    """
+    return "\n".join(
+        line for line in text.splitlines() if not _IMPORT_LINE_RE.match(line)
+    ) + "\n"
+
+
 def _robot_diff(left: Path, right: Path, out: Path, robot: str = "robot") -> None:
     subprocess.run(
         [
@@ -57,6 +79,67 @@ def _robot_diff(left: Path, right: Path, out: Path, robot: str = "robot") -> Non
     )
 
 
+def _robot_diff_with_import_fallback(
+    left: Path, right: Path, out: Path, robot: str = "robot"
+) -> None:
+    """Diff with the edit files as-is first (full import graph -> real
+    labels); only fall back to import-stripped copies if that fails (e.g. a
+    moved PURL). Keeps today's label quality for every repo whose imports
+    still resolve, while not hard-failing for one that doesn't.
+    """
+    try:
+        _robot_diff(left, right, out, robot=robot)
+        return
+    except subprocess.CalledProcessError:
+        pass
+    stripped_left = left.with_name(left.name + ".noimports")
+    stripped_right = right.with_name(right.name + ".noimports")
+    stripped_left.write_text(_strip_imports(left.read_text()))
+    stripped_right.write_text(_strip_imports(right.read_text()))
+    _robot_diff(stripped_left, stripped_right, out, robot=robot)
+
+
+# `definition_refs()` scans for the compact `obo:`/`oboInOwl:` curie form (it
+# has to — that's what real edit files use), so a conversion must declare the
+# same prefixes ROBOT doesn't add by default.
+_OFN_PREFIXES = (
+    "obo: http://purl.obolibrary.org/obo/",
+    "oboInOwl: http://www.geneontology.org/formats/oboInOwl#",
+)
+
+
+def _robot_convert_to_ofn(input_path: Path, output_path: Path, robot: str = "robot") -> None:
+    """Convert `input_path` to OWL functional syntax, in the curie form
+    `definition_refs()` expects, regardless of the input's own format.
+
+    This is what lets `definition_refs()` work against an OBO edit file: it
+    only understands functional-syntax `AnnotationAssertion(...)` lines, so
+    non-OWL edit files (e.g. `uberon-edit.obo`) need converting first.
+    """
+    cmd = [robot, "convert", "-i", str(input_path), "-f", "ofn"]
+    for prefix in _OFN_PREFIXES:
+        cmd += ["--add-prefix", prefix]
+    cmd += ["-o", str(output_path)]
+    subprocess.run(cmd, check=True)
+
+
+def _robot_convert_to_ofn_with_import_fallback(
+    input_path: Path, output_path: Path, robot: str = "robot"
+) -> None:
+    """Same fallback strategy as `_robot_diff_with_import_fallback`: definitions'
+    own dbxrefs never depend on an import, so stripping only kicks in when the
+    full-import conversion fails outright.
+    """
+    try:
+        _robot_convert_to_ofn(input_path, output_path, robot=robot)
+        return
+    except subprocess.CalledProcessError:
+        pass
+    stripped = input_path.with_name(input_path.name + ".noimports")
+    stripped.write_text(_strip_imports(input_path.read_text()))
+    _robot_convert_to_ofn(stripped, output_path, robot=robot)
+
+
 def _change_to_dict(c: Change) -> dict:
     d = dataclasses.asdict(c)
     d.pop("raw", None)  # drop the debug field from serialised output
@@ -80,13 +163,20 @@ def extract(
         tdp = Path(td)
         left = tdp / "left.owl"
         right = tdp / "right.owl"
+        right_ofn = tdp / "right.ofn"
         diff_md = tdp / "diff.md"
         _git_show(repo, left_ref, edit_file, left)
         _git_show(repo, right_ref, edit_file, right)
-        _robot_diff(left, right, diff_md, robot=robot)
+        # Full import graph first (real labels on cross-ontology references);
+        # only degrade to import-stripped copies if that fails outright (e.g.
+        # a moved PURL) -- see _robot_diff_with_import_fallback.
+        _robot_diff_with_import_fallback(left, right, diff_md, robot=robot)
+        # Read definition refs off a functional-syntax conversion rather than
+        # the right file directly, so this also works for non-OWL edit files.
+        _robot_convert_to_ofn_with_import_fallback(right, right_ofn, robot=robot)
         return (
             parse_diff_markdown(diff_md.read_text()),
-            definition_refs(right.read_text()),
+            definition_refs(right_ofn.read_text()),
         )
 
 

tests/test_stage1_extract.py

@@ -0,0 +1,221 @@
+"""Tests for the stage-1 extractor's ROBOT-facing plumbing.
+
+Unlike test_stage1_parse.py (pure functions over cached fixtures), these
+exercise the parts of extract.py that actually shell out to `robot`, so they
+skip outright if `robot` isn't on PATH.
+"""
+
+from __future__ import annotations
+
+import shutil
+import subprocess
+from pathlib import Path
+
+import pytest
+
+from clara_workflow.stage1.extract import (
+    _robot_convert_to_ofn,
+    _robot_diff_with_import_fallback,
+    _strip_imports,
+    extract,
+)
+from clara_workflow.stage1.parse import definition_refs
+
+ROBOT = shutil.which("robot")
+requires_robot = pytest.mark.skipif(ROBOT is None, reason="robot not on PATH")
+
+
+# --- _strip_imports ----------------------------------------------------
+
+def test_strip_imports_drops_obo_import_lines():
+    text = (
+        "format-version: 1.2\n"
+        "import: http://purl.obolibrary.org/obo/uberon/components/foo.owl\n"
+        "ontology: uberon/test\n"
+    )
+    stripped = _strip_imports(text)
+    assert "import:" not in stripped
+    assert "ontology: uberon/test" in stripped
+
+
+def test_strip_imports_drops_owl_functional_import_lines():
+    text = (
+        "Prefix(obo:=<http://purl.obolibrary.org/obo/>)\n"
+        "Ontology(<http://purl.obolibrary.org/obo/cl.owl>\n"
+        "Import(<http://purl.obolibrary.org/obo/cl/components/foo.owl>)\n"
+        "Declaration(Class(obo:CL_0000001))\n"
+        ")\n"
+    )
+    stripped = _strip_imports(text)
+    assert "Import(" not in stripped
+    assert "Declaration(Class(obo:CL_0000001))" in stripped
+
+
+def test_strip_imports_leaves_unrelated_lines_untouched():
+    text = "def: \"An import-related structure.\" [GOC:test]\n"
+    assert _strip_imports(text) == text
+
+
+# --- _robot_diff_with_import_fallback -------------------------------------
+#
+# Hermetic (mocked subprocess) regression tests for the fallback ordering
+# itself. Stripping imports loses label resolution for any entity defined
+# only in an import (e.g. BFO's `part of`, or an UBERON class referenced from
+# cl-edit.owl) -- confirmed by comparing real output on a live CL PR before
+# and after this fix. So stripping must be a last resort, never the default:
+# these tests fail loudly if that ordering regresses back to "always strip".
+
+def test_prefers_full_imports_when_diff_succeeds(tmp_path, monkeypatch):
+    calls: list[list[str]] = []
+
+    def fake_run(cmd, check):
+        calls.append(cmd)
+        Path(cmd[cmd.index("--output") + 1]).write_text("ok")
+
+    monkeypatch.setattr(subprocess, "run", fake_run)
+
+    left = tmp_path / "left.owl"
+    right = tmp_path / "right.owl"
+    left.write_text("import: http://example.org/foo.owl\nfoo\n")
+    right.write_text("import: http://example.org/foo.owl\nbar\n")
+    out = tmp_path / "diff.md"
+
+    _robot_diff_with_import_fallback(left, right, out, robot="robot")
+
+    assert len(calls) == 1, "must not strip imports when the first attempt succeeds"
+    assert str(left) in calls[0] and str(right) in calls[0]
+    assert not (tmp_path / "left.owl.noimports").exists()
+    assert out.read_text() == "ok"
+
+
+def test_falls_back_to_stripped_copies_only_after_a_failure(tmp_path, monkeypatch):
+    calls: list[list[str]] = []
+
+    def fake_run(cmd, check):
+        calls.append(cmd)
+        if len(calls) == 1:
+            raise subprocess.CalledProcessError(1, cmd)
+        Path(cmd[cmd.index("--output") + 1]).write_text("ok")
+
+    monkeypatch.setattr(subprocess, "run", fake_run)
+
+    left = tmp_path / "left.owl"
+    right = tmp_path / "right.owl"
+    left.write_text("import: http://example.org/broken.owl\nfoo\n")
+    right.write_text("import: http://example.org/broken.owl\nbar\n")
+    out = tmp_path / "diff.md"
+
+    _robot_diff_with_import_fallback(left, right, out, robot="robot")
+
+    assert len(calls) == 2
+    # First attempt used the original, import-carrying files.
+    assert str(left) in calls[0] and str(right) in calls[0]
+    # Retry used stripped copies, not the originals.
+    assert str(left) not in calls[1] and str(right) not in calls[1]
+    assert out.read_text() == "ok"
+
+
+# --- _robot_convert_to_ofn ----------------------------------------------
+
+_OBO_TERM = """format-version: 1.2
+ontology: uberon/test
+
+[Term]
+id: UBERON:0004177
+name: hemopoietic organ
+def: "Organ that produces blood cells." [GOC:Obol, DOI:10.9999/test.uberon.4177]
+"""
+
+
+@requires_robot
+def test_robot_convert_to_ofn_preserves_dbxrefs_from_obo(tmp_path: Path):
+    """OBO input, once converted, must still yield refs via definition_refs().
+
+    This is the crux of the OBO-support fix: definition_refs() only
+    understands functional-syntax AnnotationAssertion(...) lines, so an OBO
+    edit file needs converting first, with the obo:/oboInOwl: curie prefixes
+    it expects.
+    """
+    src = tmp_path / "right.obo"
+    src.write_text(_OBO_TERM)
+    out = tmp_path / "right.ofn"
+
+    _robot_convert_to_ofn(src, out, robot=ROBOT)
+
+    refs = definition_refs(out.read_text())
+    assert refs["UBERON:0004177"] == ["DOI:10.9999/test.uberon.4177", "GOC:Obol"]
+
+
+# --- extract() end to end -------------------------------------------------
+
+def _init_repo(tmp_path: Path) -> Path:
+    repo = tmp_path / "repo"
+    repo.mkdir()
+    subprocess.run(["git", "init", "-q"], cwd=repo, check=True)
+    subprocess.run(["git", "config", "user.email", "test@example.org"], cwd=repo, check=True)
+    subprocess.run(["git", "config", "user.name", "Test"], cwd=repo, check=True)
+    return repo
+
+
+@requires_robot
+def test_extract_end_to_end_against_obo_edit_file_with_broken_import(tmp_path: Path):
+    """Regression test for two real-world failures found reviewing uberon:
+
+    1. `robot diff` fails outright on an edit file with an unresolvable
+       `import:` PURL, unless imports are stripped first.
+    2. Without converting to functional syntax first, `definition_refs()`
+       silently returns nothing for an OBO edit file.
+    """
+    repo = _init_repo(tmp_path)
+    edit_file = repo / "uberon-edit.obo"
+
+    base = """format-version: 1.2
+ontology: uberon/test
+import: http://purl.obolibrary.org/obo/uberon/components/does-not-exist.owl
+
+[Term]
+id: UBERON:0004177
+name: hemopoietic organ
+def: "Organ that is part of the hematopoietic system." [GOC:Obol]
+"""
+    edit_file.write_text(base)
+    subprocess.run(["git", "add", "uberon-edit.obo"], cwd=repo, check=True)
+    subprocess.run(["git", "commit", "-q", "-m", "base"], cwd=repo, check=True)
+    base_sha = subprocess.run(
+        ["git", "rev-parse", "HEAD"], cwd=repo, check=True, capture_output=True, text=True
+    ).stdout.strip()
+
+    revised = base.replace(
+        'def: "Organ that is part of the hematopoietic system." [GOC:Obol]',
+        'def: "Organ that produces, stores, or regulates the maturation of '
+        'blood cells, and is part of the hematopoietic system." '
+        '[GOC:Obol, DOI:10.9999/test.uberon.4177]',
+    )
+    edit_file.write_text(revised)
+    subprocess.run(["git", "add", "uberon-edit.obo"], cwd=repo, check=True)
+    subprocess.run(["git", "commit", "-q", "-m", "revise def"], cwd=repo, check=True)
+    head_sha = subprocess.run(
+        ["git", "rev-parse", "HEAD"], cwd=repo, check=True, capture_output=True, text=True
+    ).stdout.strip()
+
+    changes, head_definition_refs = extract(
+        repo=repo,
+        left_ref=base_sha,
+        right_ref=head_sha,
+        edit_file="uberon-edit.obo",
+        robot=ROBOT,
+    )
+
+    added_defs = [c for c in changes if c.kind == "text_def" and c.side == "added"]
+    assert len(added_defs) == 1
+    assert added_defs[0].term_id == "UBERON:0004177"
+    assert "DOI:10.9999/test.uberon.4177" in added_defs[0].refs
+
+    # This term's definition wasn't left untouched, but the fallback path
+    # (used when a *different* axiom needs justifying from an existing,
+    # unchanged definition) must be populated too, proving OBO input reaches
+    # definition_refs() correctly rather than silently returning {}.
+    assert head_definition_refs.get("UBERON:0004177") == [
+        "DOI:10.9999/test.uberon.4177",
+        "GOC:Obol",
+    ]