ogar-obo: producer flip — MONDO/HP/UBERON under the domain reference tree (S3) - #293
Conversation
…ence tree (S3) Namespace::concept_id now renders the OBO three under their domain compartments — Mondo 0x9101 (disease), Hpo 0x9202 (phenotype), Uberon 0x9303 (anatomy) — per the staged migration's S3 step; Pato 0x0304 and Ro 0x0305 stay in the legacy page (no domain_classids.tsv row: quality axis / predicate namespace). from_concept_id accepts BOTH forms, so every reader resolves legacy artifacts unchanged. The enum ORDER is a wire contract (ns as u8 = the value[96] category byte, TermId.ns ordinal) and is untouched; the ascending-concept-id sweep property retired with the 0x03 block and its test is re-pinned to set-equality + explicit ordinal assertions. Registry NsSpec ids, spine-collision and domain-membership guards re-pinned to the two-domain reality; test literals now derive addresses via target_classid/render_classid instead of pinning raw numbers. examples/rekey_domain.rs carries an existing obo-core .soa across WITHOUT a re-bake: rewrites exactly the key classid (bytes 0..4) and the 23 edge-lane header classids per row, preserves row order (the label slab is positional), and verifies on its own output that no byte outside those positions changed. Measured on the real artifact: 60,478 rows, 58,587 keys moved / 1,891 kept, census exact, foreign_diffs 0.
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_15bb3bf1-5bb0-4236-a389-6de8f66ff332) |
![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: e4f7d8169a
ℹ️ 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::Mondo => 0x9101, | ||
| Namespace::Hpo => 0x9202, | ||
| Namespace::Uberon => 0x9303, | ||
| Namespace::Pato => 0x0304, | ||
| Namespace::Ro => 0x0305, |
There was a problem hiding this comment.
These assignments no longer sort in the enum/registry order used by bake_with: a full bake emits 0x9101, 0x9202, 0x9303, then 0x0304, 0x0305. The re-key tool explicitly preserves that same row order, but SpineLens::resolve binary-searches rows by (classid, num) and documents ascending classids as a precondition. Thus any full artifact containing Uberon followed by PATO/RO fails is_sorted() and can silently miss rows during ancestry or filler lookup; generation/re-keying must restore the lookup order (while moving positional labels consistently), or the resolver's ordering contract must change.
Useful? React with 👍 / 👎.
| Namespace::Mondo => 0x9101, | ||
| Namespace::Hpo => 0x9202, | ||
| Namespace::Uberon => 0x9303, |
There was a problem hiding this comment.
After these canonical IDs change, crosswalk::Crosswalk::from_bake returns 0x9101_0000 for MONDO and 0x9303_0000 for Uberon, while the public DISEASE_CLASSID and ANATOMY_CLASSID constants remain 0x0301_0000 and 0x0303_0000. Consumers following those constants' documented purpose—confirming the type of a resolved address—will now reject every newly baked crosswalk result, so the constants should derive from the new canonical namespace IDs.
Useful? React with 👍 / 👎.
| 0x9101 | 0x0301 => Namespace::Mondo, | ||
| 0x9202 | 0x0302 => Namespace::Hpo, | ||
| 0x9303 | 0x0303 => Namespace::Uberon, |
There was a problem hiding this comment.
Although these aliases make pre-migration classids readable through Namespace::from_concept_id, layout::row_schema_of still recognizes concepts only by equality against the now-migrated registry::OBO_CORE entries. Consequently row_schema_of(0x0301), 0x0302, and 0x0303 returns None, so schema-driven readers reject the legacy artifacts that this migration explicitly promises remain readable; that lookup must normalize through these aliases or include their legacy IDs.
Useful? React with 👍 / 👎.
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughThe change migrates MONDO, HPO, and Uberon to domain-form IDs while retaining legacy aliases. It adds a ChangesOBO domain re-keying
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant CLI
participant SOAArtifact
participant Namespace
participant Validator
CLI->>SOAArtifact: read input `.soa`
SOAArtifact->>Namespace: resolve eligible classids
Namespace-->>SOAArtifact: return mapped classids
SOAArtifact->>Validator: verify keys and permitted byte changes
Validator-->>CLI: return statistics and censuses
CLI->>SOAArtifact: write output `.soa`
Poem
✨ Finishing Touches📝 Generate docstrings
Comment |
…space The S3 producer flip (#293) set the reader rule at from_concept_id — both 0x0301 and 0x9101 resolve Mondo — but row_schema_of still probed the registry with the RAW concept id, so a pre-flip artifact's legacy ids stopped resolving the carve the moment the registry minted the domain forms. Fold through the namespace alias before the registry probe; concepts without an alias (PATO/RO, spine, foreign) probe as themselves, and 0x0306 / non-OBO ids stay refused.
2 participants
What
The producer-side S3 step of the staged domain-classid migration, for the OBO five:
Namespace::concept_idnow renders Mondo 0x9101 (disease compartment), Hpo 0x9202 (phenotype), Uberon 0x9303 (anatomy). Pato 0x0304 / Ro 0x0305 stay in the legacy page — neither has adomain_classids.tsvrow (quality axis / predicate namespace), so there is nothing to flip them to.from_concept_idaccepts both forms (0x9101 | 0x0301 => Mondo, …), so every reader keeps resolving legacy artifacts unchanged. The alias stays a computation, per the staging doctrine.Wire contract preserved
The enum order is a wire contract (
ns as u8= the value[96] category byte 0..4,TermId.nsordinal) and is untouched. The old "concept ids ascend with ordinal" property retired with the 0x03 block; its sweep test is re-pinned to set-equality over all 8 accepted ids plus explicit per-variant ordinal assertions. RegistryNsSpecids, the spine-collision guard, and the domain-membership guard are re-pinned to the two-domain reality (hi == 0x03 || 0x90..=0x9D); test literals now derive addresses viatarget_classid/render_classidinstead of pinning raw numbers.The artifact carries over without a re-bake
examples/rekey_domain.rstransforms an existing obo-core.soain place of a re-bake: it rewrites exactly the key classid (bytes 0..4) and the 23 edge-lane header classids per 512-byte row, preserves row order (the label slab is positional), and verifies on its own output that no byte outside those positions changed.Measured on the real 60,478-row artifact:
Gates
cargo test -p ogar-obo: 48/48; workspace suites greencargo clippy -p ogar-obo --all-targets: cleancargo fmt: cleanConsumer-side (MedCare) follow-up: upload the re-keyed artifact under a new tag, flip crosswalk/rails/census constants, symmetric fold in obo_store — separate PR in the private repo.
🤖 Generated with Claude Code
https://claude.ai/code/session_01PFnYKqw6d7TTiB9cT8eFdK
Generated by Claude Code
Summary by CodeRabbit