Skip to content

fix: override ICD10CM prefix expansion to use icd10data.com#1260

Open
kevinschaper wants to merge 1 commit intomainfrom
fix/icd10cm-prefix-expansion
Open

fix: override ICD10CM prefix expansion to use icd10data.com#1260
kevinschaper wants to merge 1 commit intomainfrom
fix/icd10cm-prefix-expansion

Conversation

@kevinschaper
Copy link
Member

@kevinschaper kevinschaper commented Feb 19, 2026

Summary

  • The upstream prefixmaps library maps ICD10CM CURIEs to https://icd.codes/icd10cm/, but that domain is dead (returning 403s)
  • Override the canonical URI prefix to use http://purl.bioontology.org/ontology/ICD10CM/ instead, which redirects to BioPortal's ICD10CM class pages
  • Uses direct prefix_map mutation because add_prefix(merge=True) only adds the new URL as a synonym rather than replacing the canonical prefix

Upstream fix should go into linkml/prefixmaps and/or biopragmatics/bioregistry.

@netlify
Copy link

netlify bot commented Feb 19, 2026

Deploy Preview for monarch-app canceled.

Name Link
🔨 Latest commit e0651f7
🔍 Latest deploy log https://app.netlify.com/projects/monarch-app/deploys/69973bbb18b10a0007141801

@claude
Copy link

claude bot commented Feb 19, 2026

PR Review: fix: override ICD10CM prefix expansion to use icd10data.com

Overall: Looks good — clear fix for a real breakage, well-commented. One suggestion below.


What the change does

Directly patches converter.prefix_map["ICD10CM"] to replace the dead icd.codes URL with https://www.icd10data.com/search?s=, matching what bioregistry.io resolves to.


Positive observations

  • The comment clearly explains why add_prefix(merge=True) isn't sufficient (it only adds a synonym rather than replacing the canonical prefix). That context is valuable for future maintainers.
  • The fix is minimal and targeted — no unnecessary changes.
  • The PR description calls out that the proper fix belongs upstream in linkml/prefixmaps / biopragmatics/bioregistry, which is the right framing for a workaround.

One concern: no test case added

test_curie_service.py has parametrized tests for many prefixes but none for ICD10CM. This means:

  1. There's no automated verification that the override actually works today.
  2. If the curies/prefixmaps library changes behavior (e.g., fixes ICD10CM upstream to a different URL, or changes how prefix_map is exposed), the regression won't be caught.

Consider adding a test case:

("ICD10CM:J45.20", "www.icd10data.com/search?s=J45.20"),

(following the existing pattern of checking expected in expanded_curie)


Minor note: search-URL semantics

The target URL (https://www.icd10data.com/search?s=) is a search endpoint rather than a canonical entity resolver, so ICD10CM:J45.20 expands to a search result page rather than a stable, bookmarkable entity page. This matches bioregistry.io's behavior and is a practical improvement over a 403, but worth noting in case a more stable direct-link resolver becomes available in the future.


No blockers — the fix addresses a real user-facing issue. Adding the test case would strengthen confidence in the workaround.

@codecov
Copy link

codecov bot commented Feb 19, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 73.52%. Comparing base (e32ef46) to head (e0651f7).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1260   +/-   ##
=======================================
  Coverage   73.51%   73.52%           
=======================================
  Files          95       95           
  Lines        3417     3418    +1     
=======================================
+ Hits         2512     2513    +1     
  Misses        905      905           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

The upstream prefixmaps library maps ICD10CM to icd.codes, which is
dead/returning 403s. Override to use purl.bioontology.org, which
redirects to BioPortal's ICD10CM class pages.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@kevinschaper kevinschaper force-pushed the fix/icd10cm-prefix-expansion branch from 582f74b to e0651f7 Compare February 19, 2026 16:35
@claude
Copy link

claude bot commented Feb 19, 2026

PR Review: fix: override ICD10CM prefix expansion

Overall: Straightforward, well-motivated fix. One suggestion below.

Note: A previous review comment from this bot had an incorrect URL for the override target — the actual URL used is purl.bioontology.org, not icd10data.com. This comment supersedes it.


What the change does

Directly patches converter.prefix_map["ICD10CM"] to replace the dead icd.codes canonical URL with http://purl.bioontology.org/ontology/ICD10CM/, which redirects to BioPortal's ICD10CM class pages.


Positive observations

  • The inline comment clearly explains why add_prefix(merge=True) isn't sufficient (it only adds as a synonym rather than replacing the canonical prefix). That context is valuable for future maintainers.
  • The fix is minimal and targeted — 3 lines, no unnecessary changes.
  • The PR description correctly frames this as an upstream workaround and identifies linkml/prefixmaps and biopragmatics/bioregistry as the right places for a permanent fix.

Minor: PR title vs. actual target URL

The PR title says "use icd10data.com" but the code and commit message use purl.bioontology.org. The PR description is accurate — just a title inconsistency worth cleaning up if the PR is updated.


Missing test case

backend/tests/unit/test_curie_service.py has parametrized expansion tests for many prefixes but none for ICD10CM. Adding one would:

  1. Confirm the override works today.
  2. Catch regressions if the upstream library changes prefix_map semantics or fixes ICD10CM to a different URL.

Suggested addition to the parametrize list (following the existing pattern):

("ICD10CM:J45.20", "purl.bioontology.org/ontology/ICD10CM/J45.20"),

No blockers — the fix addresses a real user-facing issue with a clear workaround and good documentation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

Comments