Stop a catch-all namespace from shadowing specific fallback mappings

kgx/utils/kgx_utils.py

@@ -220,6 +220,24 @@ def format_biolink_slots(s: str) -> str:
         return f"biolink:{formatted}"
 
 
+def _most_specific(uri: str, primary: List[str], fallback: List[str]) -> List[str]:
+    """Return whichever candidate list consumed the longest IRI prefix.
+
+    The length of the IRI prefix a CURIE consumed is ``len(uri) - len(local id)``,
+    so the candidate with the shortest local part is the most specific one.
+    ``primary`` wins ties, keeping the caller's prefix map canonical.
+    """
+
+    def local_len(curie: str) -> int:
+        return len(curie.split(":", 1)[-1])
+
+    if not primary:
+        return fallback
+    if min(map(local_len, fallback)) < min(map(local_len, primary)):
+        return fallback
+    return primary
+
+
 def contract(
     uri: str, prefix_maps: Optional[List[Dict]] = None, fallback: bool = True
 ) -> str:
@@ -252,12 +270,17 @@ def contract(
     ]
     if prefix_maps:
         curie_list = contract_uri(uri, prefix_maps)
-        if len(curie_list) == 0:
-            if fallback:
-                curie_list = contract_uri(uri, default_curie_maps)
-                if curie_list:
-                    curie = curie_list[0]
-        else:
+        if fallback:
+            # A catch-all namespace in prefix_maps (e.g. biolink's
+            # OBO -> http://purl.obolibrary.org/obo/) matches every OBO IRI, so
+            # a bare "did we get any match" test would shadow the far more
+            # specific mappings the default maps carry (DDPHENO_, FBbt_, ...).
+            # Consider both tiers and keep the most specific match, i.e. the one
+            # that consumed the longest IRI prefix; prefix_maps wins a tie.
+            fallback_list = contract_uri(uri, default_curie_maps)
+            if fallback_list:
+                curie_list = _most_specific(uri, curie_list, fallback_list)
+        if curie_list:
             curie = curie_list[0]
     else:
         curie_list = contract_uri(uri, default_curie_maps)

tests/unit/test_kgx_utils.py

@@ -398,3 +398,64 @@ def test_sanitize_export_property(query):
         assert query[1] == value
     else:
         assert query[1] in value
+
+
+# A catch-all namespace, i.e. one whose IRI is a bare prefix of many more
+# specific namespaces. biolink's `OBO -> http://purl.obolibrary.org/obo/` is the
+# real-world example that motivated these tests.
+CATCH_ALL_PREFIX_MAP = {"OBO": "http://purl.obolibrary.org/obo/"}
+
+
+@pytest.mark.parametrize(
+    "uri,expected",
+    [
+        # obo_context has a DDPHENO_ mapping; the catch-all must not shadow it.
+        ("http://purl.obolibrary.org/obo/DDPHENO_0000001", "DDPHENO:0000001"),
+        ("http://purl.obolibrary.org/obo/FBbt_00000001", "FBbt:00000001"),
+        ("http://purl.obolibrary.org/obo/EMAPA_16040", "EMAPA:16040"),
+        # Nothing more specific exists for these, so the catch-all still applies.
+        (
+            "http://purl.obolibrary.org/obo/fbbt#has_function_in",
+            "OBO:fbbt#has_function_in",
+        ),
+        (
+            "http://purl.obolibrary.org/obo/go/extensions/ro_0002092",
+            "OBO:go/extensions/ro_0002092",
+        ),
+    ],
+)
+def test_contract_catch_all_does_not_shadow_specific_mapping(uri, expected):
+    """
+    Test that a catch-all namespace in prefix_maps does not suppress the fallback.
+
+    A catch-all matches every IRI beneath it, so gating the fallback on "did
+    prefix_maps match anything" means a more specific mapping in the default
+    maps is never reached.
+    """
+    assert contract(uri, prefix_maps=[CATCH_ALL_PREFIX_MAP], fallback=True) == expected
+
+
+def test_contract_prefix_maps_win_ties():
+    """
+    Test that prefix_maps stays canonical when it is as specific as the fallback.
+    """
+    # obo_context maps GO -> http://purl.obolibrary.org/obo/GO_; an equally
+    # specific caller-supplied mapping must not be overridden by it.
+    curie = contract(
+        "http://purl.obolibrary.org/obo/GO_0008150",
+        prefix_maps=[{"GENE_ONTOLOGY": "http://purl.obolibrary.org/obo/GO_"}],
+        fallback=True,
+    )
+    assert curie == "GENE_ONTOLOGY:0008150"
+
+
+def test_contract_catch_all_is_kept_without_fallback():
+    """
+    Test that fallback=False still contracts using only the given prefix_maps.
+    """
+    curie = contract(
+        "http://purl.obolibrary.org/obo/DDPHENO_0000001",
+        prefix_maps=[CATCH_ALL_PREFIX_MAP],
+        fallback=False,
+    )
+    assert curie == "OBO:DDPHENO_0000001"

tests/unit/test_prefix_manager.py

@@ -120,3 +120,38 @@ def test_prefix_manager_contract(query):
     """
     pm = PrefixManager()
     assert pm.contract(query[0]) == query[1]
+
+
+@pytest.mark.parametrize(
+    "query",
+    [
+        # OBO ontologies with no entry of their own in the JSON-LD context. The
+        # context's catch-all `OBO -> http://purl.obolibrary.org/obo/` matches
+        # them, and must not suppress the specific mappings in obo_context.
+        ("http://purl.obolibrary.org/obo/DDPHENO_0000001", "DDPHENO:0000001"),
+        ("http://purl.obolibrary.org/obo/FBbt_00000001", "FBbt:00000001"),
+        ("http://purl.obolibrary.org/obo/EMAPA_16040", "EMAPA:16040"),
+        ("http://purl.obolibrary.org/obo/WBbt_0005733", "WBbt:0005733"),
+        ("http://purl.obolibrary.org/obo/ZFA_0000001", "ZFA:0000001"),
+        ("http://purl.obolibrary.org/obo/XAO_0000001", "XAO:0000001"),
+        ("http://purl.obolibrary.org/obo/ZFS_0000001", "ZFS:0000001"),
+        ("http://purl.obolibrary.org/obo/CHR_0000001", "CHR:0000001"),
+        # Ontologies that do have their own context entry are unaffected.
+        ("http://purl.obolibrary.org/obo/HP_0000001", "HP:0000001"),
+        ("http://purl.obolibrary.org/obo/MONDO_0000001", "MONDO:0000001"),
+        ("http://purl.obolibrary.org/obo/RO_0002162", "RO:0002162"),
+        ("http://purl.obolibrary.org/obo/BFO_0000050", "BFO:0000050"),
+        # IRIs beneath the OBO namespace that no per-ontology mapping covers
+        # keep the catch-all, as before.
+        (
+            "http://purl.obolibrary.org/obo/fbbt#has_function_in",
+            "OBO:fbbt#has_function_in",
+        ),
+    ],
+)
+def test_prefix_manager_contract_obo_idspaces(query):
+    """
+    Test that the OBO catch-all does not shadow per-ontology mappings.
+    """
+    pm = PrefixManager()
+    assert pm.contract(query[0]) == query[1]