Stop a catch-all namespace from shadowing specific fallback mappings - #559
Open
kevinschaper wants to merge 1 commit into
Open
Stop a catch-all namespace from shadowing specific fallback mappings#559kevinschaper wants to merge 1 commit into
kevinschaper wants to merge 1 commit into
Conversation
contract() consulted the default prefix maps only when prefix_maps returned zero matches. The JSON-LD context carries OBO -> http://purl.obolibrary.org/obo/, a catch-all that matches every OBO IRI, so the fallback never ran and obo_context's per-ontology mappings were unreachable: DDPHENO, FBbt, WBbt, EMAPA, ZFA, XAO, CHR, ZFS, OBA and FBdv all contracted to OBO:DDPHENO_0000001 rather than DDPHENO:0000001. The MONARCH catch-all hid BNODE, ISBN, ISBN-10, ISBN-13 and OMIA-breed the same way. Consider both tiers and keep the most specific match -- the candidate that consumed the longest IRI prefix -- with prefix_maps winning ties so a caller-supplied map stays canonical. fallback=False is unchanged. This was latent until 2.7.0. TsvSource.set_prefix_map used to be destructive, so the transformer's set_prefix_map({}) wiped the map down to five entries and every OBO IRI fell through to the fallback and contracted correctly. Making it additive (#548) restored the map, and with it the dormant catch-all. Closes #558. Claude-Session: https://claude.ai/code/session_01BzdpULg2jTEP8wN3jBNzrN
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #558.
Problem
contract()consults the default prefix maps only whenprefix_mapsreturns zero matches:The JSON-LD context carries
OBO -> http://purl.obolibrary.org/obo/, a catch-all that matches every OBO IRI. Socurie_listis never empty for one, the fallback never runs, andobo_context's per-ontology mappings are unreachable:165 OBO idspaces are affected, including DDPHENO, FBbt, WBbt, EMAPA, ZFA, XAO, CHR, ZFS, OBA and FBdv. The
MONARCH -> https://monarchinitiative.org/catch-all hidesBNODE,ISBN,ISBN-10,ISBN-13andOMIA-breedthe same way.Latent until 2.7.0:
TsvSource.set_prefix_mapused to be destructive, and the transformer callssource.set_prefix_map({})when the caller supplies no map, so the map was wiped to five entries and every OBO IRI fell through to the fallback and contracted correctly. Making it additive (#548) restored the map, and with it the dormant catch-all.Fix
Consider both tiers and keep the most specific match — the candidate that consumed the longest IRI prefix — with
prefix_mapswinning ties so a caller-supplied map stays canonical. No OBO-specific special-casing;fallback=Falseis untouched.prefixcommons.contract_urialready prefers the shortest CURIE when both mappings are in scope, so this only changes which tiers get compared, not how a single tier resolves.Blast radius
Probed every namespace in all three contexts (488 URIs), patched vs unpatched
PrefixManager.contract:OBO:catch-all → specific idspaceMONARCH:catch-all → specific (BNODE,ISBN,ISBN-10,ISBN-13,OMIA-breed)WIKIDATA:Property:xvsWIKIDATA_PROPERTY:x, which is nondeterministic on master anywayEverything that resolves correctly today is unchanged:
HP:,MONDO:,GO:,RO:,BFO:,CHEBI:,UBERON:,HGNC:viaidentifiers.org, and allbiolink:vocab terms. IRIs beneath the OBO namespace that no per-ontology mapping covers still contract to the catch-all —OBO:fbbt#has_function_in,OBO:go/extensions/ro_0002092— becausemonarch_contextsuppliesOBOin the fallback tier.Test plan
tests/unit/test_kgx_utils.pycovers catch-all vs fallback,prefix_mapswinning a tie against an equally specific fallback mapping, andfallback=Falsestill yieldingOBO:DDPHENO_0000001.tests/unit/test_prefix_manager.pycovers the eight regressed idspaces, four controls, and a relation IRI that must keepOBO:.master, all pass with the fix.test_sink/test_jsonl_sink.py::test_write_jsonl2, which fails identically on unpatchedmaster— pre-existing and unrelated.Downstream
This is what untyped ~52k nodes in kg-phenio from the 2026-06-03 release on: its transform looks node categories up by CURIE prefix, and
OBOisn't an ontology, so DDPHENO/FBbt/EMAPA/WBbt/ZFA/XAO/CHR/ZFS terms have been shipping asbiolink:NamedThing.https://claude.ai/code/session_01BzdpULg2jTEP8wN3jBNzrN