ogar-obo: write the reason module (OWL-EL completion + edge walk) and close the bake - #232
Conversation
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 3 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 72b8046. Configure here.
| if line == "[Term]" { | ||
| commit(&mut cur, &mut node, &mut name, &mut graph); | ||
| continue; | ||
| } |
There was a problem hiding this comment.
Non-Term stanzas corrupt parsing
High Severity
from_obo_str only commits the current stanza when it sees [Term]. Real OBO files (mondo.obo, hp.obo, etc.) interleave [Typedef], [Instance], and other bracket headers. A later id: line can retarget cur without flushing the in-progress term, so terms are dropped or stored under the wrong id with another stanza’s edges.
Reviewed by Cursor Bugbot for commit 72b8046. Configure here.
| && let Some(t) = TermId::parse(obj) | ||
| { | ||
| node.rel.push((map_relationship(pred), t)); | ||
| } |
There was a problem hiding this comment.
Obsolete terms keep edges and rows
Medium Severity
Docs state obsolete terms are kept for id only with no edges, and OboNode marks them as not tiled, but parsing still records is_a/relationship after is_obsolete: true, and bake_rows packs every node without checking obsolete. Reasoning and the SoA buffer can include retired terms and their relationships.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 72b8046. Configure here.
| } | ||
| frontier = next; | ||
| } | ||
| out |
There was a problem hiding this comment.
Cyclic is_a includes query term
Medium Severity
ancestors documents excluding the query term, but seen never includes the starting id. If the is_a graph cycles back to that term (including a direct self-parent), the term can appear in the returned ancestor list and pollute related_via_ancestry.
Reviewed by Cursor Bugbot for commit 72b8046. Configure here.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 72b8046db2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| Namespace::Uberon => 0x0A02, | ||
| Namespace::Mondo => 0x0B01, | ||
| Namespace::Hpo => 0x0B02, | ||
| Namespace::Pato => 0x0B03, | ||
| Namespace::Ro => 0x0B04, |
There was a problem hiding this comment.
Every baked OBO row currently aliases an existing canonical class: 0x0A02 is SKELETON, while 0x0B01 through 0x0B04 are AUTH_STORE, AUTH_ZITADEL, AUTH_ZANZIBAR, and AUTH_ORY_KETO in ogar-vocab. Consequently, loaders that route or select schemas by classid will interpret Uberon as skeleton and the clinical ontologies as IAM records. Allocate non-conflicting IDs in the canonical codebook and consume those constants here rather than hard-coding occupied slots.
Useful? React with 👍 / 👎.
| deg[Predicate::IsA as usize] = node.is_a.len() as u16; | ||
| for (p, _) in &node.rel { | ||
| deg[*p as usize] = deg[*p as usize].saturating_add(1); | ||
| } | ||
| for (i, d) in deg.iter().enumerate() { | ||
| row.0[EDGES_OFFSET + i] = (*d).min(255) as u8; |
There was a problem hiding this comment.
For any node with an is_a or relationship edge, this writes predicate counts into bytes that the stated NodeRow consumer interprets as the canonical 12-in-family/4-out-of-family EdgeBlock; the existing compiled_class_to_noderow path in crates/ogar-from-ruff/src/lance_sink.rs accordingly constructs an actual EdgeBlock, not a histogram. node_rows_from_le_bytes will accept these rows structurally but downstream traversal will read degree bytes as adjacency data. Emit a valid EdgeBlock, or leave it reserved and return the promised out-of-line triples separately.
Useful? React with 👍 / 👎.
| if line == "[Term]" { | ||
| commit(&mut cur, &mut node, &mut name, &mut graph); | ||
| continue; |
There was a problem hiding this comment.
When a normal full OBO file places [Typedef] stanzas after its terms, only [Term] triggers commit; the following typedef id: therefore overwrites the last term's cur, dropping that term and eventually inserting an RO typedef as an OboNode with accumulated term/typedef state. Commit and clear on every stanza header, and only consume term tags while inside a [Term] stanza.
Useful? React with 👍 / 👎.
| } else if let Some(rest) = line.strip_prefix("relationship: ") { | ||
| let mut it = rest.split_whitespace(); | ||
| if let (Some(pred), Some(obj)) = (it.next(), it.next()) | ||
| && let Some(t) = TermId::parse(obj) | ||
| { | ||
| node.rel.push((map_relationship(pred), t)); |
There was a problem hiding this comment.
For HPO genus-differentia definitions expressed as intersection_of: <role> <CURIE>, this parser falls through because it recognizes only relationship: edges. As a result, the advertised anatomy and quality edges harvested from HPO logical definitions are never added—particularly HasAnatomy, which no current parsing branch can produce—so reasoning walks over those standard definitions return empty results. Parse intersection_of role/object forms and map the supported anatomy and quality roles.
Useful? React with 👍 / 👎.
72b8046 to
23426c6
Compare
…_of / phenotypes_of)
`saturate` computes the aggregate EL closure; a consumer resolving ONE entity
(a disease → its anatomy site + phenotypes, e.g. medcare-first-thought's
ontology grounding) needs the per-term walk. These operate directly on
`parse_obo`'s `id -> node` map, applying the same is_a-saturation +
existential-role-propagation rules to a single subject:
* `ancestors` — transitive is_a closure of a term (self excluded, cycle-safe:
the start id is pre-seeded into `seen` so a self/loop parent never re-emits
the query term), deterministic order, depth-capped.
* `related_via_ancestry` — every `pred` target reachable from the term or its
is_a ancestors (the subject inherits its ancestors' existential edges).
* `anatomy_of` / `phenotypes_of` — the Uberon (Mondo→Uberon = HasLocation) /
HPO (Mondo→Hpo = HasPhenotype) conveniences over that walk, matching the
crate's namespace-pair `classify`.
Additive over the merged #231 crosswalk/saturate work; no change to the bake,
crosswalk, concept-ids, or `saturate`. 1 new unit test (ancestry + inherited
anatomy + own phenotype + self-exclusion).
Generated by [Claude Code](https://claude.com/claude-code)
23426c6 to
2fd1675
Compare
2 participants
ogar-obo: write the
reasonmodule (OWL-EL completion + edge walk) and close the bakeThe
ogar-obocrate declaredpub mod reason;with no file — the OWL-ELcompletion the OBO EL profile exercises, and the "CURIE edge-propagation"
obo-crosswalk.jsondefers to "a later session". So the crate did not compile.This writes that module and closes the bake.
reasonOboGraph::from_obo_str— parse the bake's source OBO form (mondo.obo/hp.obo/uberon.obo, or a faithful slice) into typed edges over thecrate's own
TermId/Predicate/OboNodemodel.ancestors— theis_asubsumption saturation (transitive closure).related_via_ancestry+anatomy_of(→Uberon) /phenotypes_of(→HPO) — transitive-role existential propagation: a subject inherits its
is_aancestors' existential edges (pneumonia inherits its parents'disease_has_location).map_relationship— OBO predicate token → RO byte palette(
disease_has_location→HasLocation,disease_has_feature/has_phenotype→HasPhenotype,
has_characteristic→HasQuality,part_of→PartOf).bake_rows— parse → the canonical 512-byte SoANodeRowbuffer (oneRow512per term;classid = namespace.render_classid(0); CURIE numeric asthe 24-bit identity; per-predicate edge-degree histogram), deterministic key
order. This closes the crate's stated purpose and exercises the previously
dead
pack_key/pack_rowprimitives (madepub(crate)).Posture
Pure public CC-BY / CC0 OBO reference — no PHI, no serialization, no cross-repo
compile dep (the loader tie stays a byte-layout contract). 6 unit tests (parse;
is_asaturation transitivity; existential propagation through ancestry;deterministic bake; unknown-prefix drop). Clippy-clean,
missing_docs-clean.Consumer
AdaWorldAPI/MedCare-rs(PR #333) consumesogar_obo::reason::OboGraphto walkdisease → anatomy/phenotype reasoning edges, replacing a local mirror it carried
while this crate did not compile ("consume the Core, never re-implement").
🤖 Generated with Claude Code