Add search-xrefs command with OLS4 and MyChem.info providers - #21
Draft
gaurav wants to merge 2 commits into
Draft
Conversation
Surfaces candidate cross-references from external mapping sources and diffs them against Babel's local Concord.parquet, to help maintainers find xrefs worth importing into Babel (e.g. NCATSTranslator/Babel#715, where CHEBI:31941 / PUBCHEM.COMPOUND:43805 / UMLS:C1314429 should form a single clique but don't). Providers live in src/babel_explorer/core/providers/ behind a small Protocol + registry so new sources (UniChem, SSSOM/Mapping Commons, BridgeDb) can be added by writing one class and appending to PROVIDERS. Each candidate carries query/target CURIEs, provider name, predicate, confidence, evidence URL, and an in_babel flag. --ignore-known filters out candidates Babel already knows about. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
gaurav
added a commit
that referenced
this pull request
Sep 1, 2026
…kDB and NodeNorm (#20) babel-explorer is a CLI for asking Babel *why* two identifiers are considered the same thing. It reads Babel's intermediate Parquet files through DuckDB and, optionally, enriches the results with labels from NodeNorm. `BabelDownloader` handles caching and freshness, `BabelXRefs` handles querying, `NodeNorm` handles labels, and `cli.py` wires them together with Click. Supersedes #1, which GitHub closed and refused to reopen after this branch's history was rewritten. Closes #12. ## What's here Three commands: - **`xrefs`** — cross-references for one or more CURIEs. `--recurse` expands transitively through a single `WITH RECURSIVE` DuckDB query; `--paths` shows the shortest paths connecting the given CURIEs; `--labels` adds NodeNorm labels and Biolink types. - **`ids`** — identifier records from `Identifiers.parquet`, with `--labels`. - **`test-concord`** — compare a proposed concordance change against NodeNorm's current cliques. `xrefs` and `ids` also take `--format json|tsv|csv` for machine-readable output. `--paths` is console-only, and both of its preconditions — a console format, and at least two CURIEs — are checked before anything is downloaded. Getting either wrong otherwise costs a multi-gigabyte `Concord.parquet` download and a full recursive query before the run is rejected, because `--paths` implies `--recurse`. Failures from the two services this tool talks to are reported as errors rather than tracebacks. `MissingBabelFileError` explains that a release does not publish the DuckDB files; `requests.RequestException` reaching the top means NodeNorm, since the downloader handles its own network failures. ## Configuring which Babel release to query A Babel release is addressed as a **releases directory plus a version**, which is how both the public and internal trees are actually laid out — one subdirectory per release, plus a `latest/` symlink: | Variable | CLI option | Default | |---|---|---| | `BABEL_RELEASES_URL` | `--babel-releases-url` | `https://stars.renci.org/var/babel/` | | `BABEL_VERSION` | `--babel-version` | `latest` | | — | `--babel-url` | *(overrides both)* | Pinning a release is a one-word change rather than a URL edit, which matters because pinning is the honest fix for a NodeNorm version mismatch. Precedence runs **flag > environment variable > `.env` > built-in default**, and only public URLs are committed. `--babel-url` takes a complete URL for a tree that does not follow that layout. It is **command-line only, with no `envvar=`, deliberately**: two variables already feed the composed URL, and a third that silently outranked both would make "which release am I actually querying?" unanswerable from the environment alone. `cli()` warns if the pre-refactor `BABEL_URL` is still set, so a stale `.env` fails loudly instead of silently pointing somewhere else. `compose_babel_url` lives in `core/downloader.py` rather than `cli.py` because `tests/constants.py` needs the identical composition and must not import Click to get it. The committed template is `env.default` — visible in a plain `ls`, unlike a dotfile. ## Babel version handling The release is resolved from `VERSION.txt`, falling back to the final URL path segment — which under this scheme is exactly `BABEL_VERSION`, so a pinned release still resolves when `VERSION.txt` is unreachable, while `latest` yields `None` as before. `BABEL_LOCAL_DIR` holds one release at a time. When it changes, only `last_checked` is cleared from the `.meta` sidecars, so the existing ETag path re-checks each file and re-downloads only what actually changed — the Parquet files are never deleted, and an unchanged file costs one HEAD rather than a fresh multi-gigabyte download. The `.babel-version` marker records the release the server *resolved* to rather than the one requested, so `latest` and an equivalent pinned version share a cache instead of thrashing it. That marker is written **after** the cache catches up, not when the change is spotted. It claims "the local cache holds this release", which is only true once every cached file has been re-validated against it, so `_write_version_marker_if_synced()` stamps it once no `.meta` sidecar is still missing its `last_checked`. Writing it up front would leave a run interrupted between `Concord.parquet` and `Identifiers.parquet` with a marker naming the new release over a half-old cache, and the next run would see it match and skip the refresh entirely. The cost is that a cached file nobody asks for holds the marker back indefinitely, at one HEAD per run; that is the honest answer, since the file really is still from the previous release. `--check-download never` suppresses re-checks *within* a release, not across one. `_is_within_freshness()` therefore tests for a missing `last_checked` **before** the `float("inf")` shortcut — reversed, `never` would hand back the previous release's Parquet with no network call at all, under a marker naming the new release, and nothing would ever notice. `--labels` refuses to mix a NodeNorm built from one release with cross-references from another, since the result would be silently wrong rather than obviously wrong. `--allow-version-mismatch` overrides it; pinning `BABEL_VERSION` fixes it properly. `--recurse` never consults NodeNorm, so it does not trigger the check. ## Partial downloads cannot corrupt the cache A corrupt Parquet here is *permanent*: whatever lands on disk gets stamped with the correct remote ETag and then passes every later freshness check. Five routes to that are closed: - A `.tmp` left by a killed process is discarded before each download rather than resumed. Resume is by byte offset, the only way to reach the download at all is that the remote bytes changed, and an orphaned `.tmp` carries no record of which version its bytes came from. Cleanup catches `BaseException`, so Ctrl-C leaves nothing resumable behind. - In-run resumes send `If-Range`, so a file rebuilt mid-download restarts instead of splicing two versions together. - HTTP 416 counts as "already complete" only once the local size matches the remote `Content-Length` — 416 is also what a server returns when the file *shrank* below the resume offset. - A stream ending short of `Content-Length` raises `IncompleteDownloadError` and is retried, rather than being promoted as complete. - A failed HEAD returns "unknown", not "unchanged", and no longer refreshes `last_checked`. One flaky HEAD could otherwise pin the previous release's Parquet as freshly validated for the whole freshness window. `.tmp` files are deleted in two places on purpose; `CLAUDE.md` records which one is the safety guarantee and which is housekeeping, so neither gets removed later as redundant. ## Querying DuckDB connections are ephemeral and in-memory, but "in-memory" is not "touches no disk": a larger-than-memory query spills, and DuckDB's default `temp_directory` is `.tmp` in the *current working directory*. The recursive expansion materialises the whole Concord relation, so every connection goes through `BabelXRefs._connect()`, which points `temp_directory` at `<BABEL_LOCAL_DIR>/duckdb-spill/` — the directory the user already chose to hold multi-gigabyte files, rather than wherever they happened to be standing. ## What it deliberately does not do - **Resume a download across runs.** A leftover `.tmp` cannot be proven to belong to the file being fetched, so it is discarded. Making it safe means persisting the validator alongside the `.tmp`; scoped in #15. - **Enforce the no-private-URLs rule outside `env.default`.** `TestCommittedConfigTemplate` guards the template, but the original leak came through a default value in `cli.py`. A tree-wide scan is #25. - **Reuse a DuckDB connection across queries.** A known performance cost, deferred to #13. (Batching NodeNorm lookups *did* ship — `normalize_curies()` collapses a clique into one request per 100 CURIEs, which is why this closes #12.) ## Known limitations at v0.1.0 This ships knowingly non-functional against its own defaults, so people can try the tool now rather than after the Babel side catches up. The CLI says so clearly rather than failing mid-download. - Public Babel releases do not publish `duckdb/Concord.parquet` or `duckdb/Identifiers.parquet`. **#16 — must close before v1.0.0.** - NodeNorm dev reports Babel `2025sep1` against public `2025dec11`, so `--labels` fails the skew check against the defaults. #17 - CI therefore cannot run the 28 Parquet integration tests. #18 - NodeNorm integration tests fail rather than skip when the API is unreachable. #19 ## Testing 261 unit tests and 13 NodeNorm integration tests pass. 28 Parquet-dependent integration tests skip without a Babel release publishing the files — that is the expected result, not a broken environment. Verified end to end against a Translator releases tree: the composed URL resolves `2026jul22`, downloads `Concord.parquet`, writes a correct `.babel-version` marker, and returns cross-references for `MONDO:0004979`. The version-marker sequencing was verified separately by simulating a `2025dec11` → `2026jul22` change under `--check-download never`: both Parquet files are re-fetched, and the marker flips only after the second one lands. Note that `not slow` is **not** a promise of "small". `Concord.parquet` is 4.6 GB in `2026jul22` and its tests are not marked `slow`, because excluding them would leave the non-slow integration set covering nothing that touches real data. This matters for sizing #18. ## Nothing is blocking this merge Everything outstanding is tracked in #12, #13, #15, #16, #17, #18, #19 and #25. None of it makes what ships here wrong — #16 and #17 constrain what the defaults can *do*, and both are stated plainly in the README, `CHANGELOG.md` and the CLI's own error messages. <details> <summary><b>Review history</b> — three review rounds and a history rewrite. Kept for anyone tracing why a particular line looks the way it does; the durable conclusions are in the code comments, CLAUDE.md and the sections above.</summary> **First review round** found six defects, each fixed in its own commit with a regression test: `--recurse` triggered the NodeNorm version check it no longer needed; `ids --labels` silently dropped the NodeNorm label because Identifiers.parquet's own `label` column overwrote it; a version change forced a full re-download instead of an ETag re-check; a stale `.tmp` could splice two releases into one corrupt Parquet; the HTTP 416 fast path persisted the error response's headers as the file's metadata; and the integration skip probe did not normalise a slashless `BABEL_URL`, so it probed `.../latestduckdb/...` and silently skipped the entire integration suite. **Second review round** found six more, all but one in the downloader's resume path — the five now described under "Partial downloads cannot corrupt the cache", plus `record_to_dict` dropping Identifiers.parquet's own `label` column whenever it was empty, because the omit-when-absent rule matched on a `label` name suffix rather than the three NodeNorm-derived field names. That made `label` present on some rows of a json/tsv/csv run and missing on others. The first round's `.tmp` fix was narrower than it looked: it swept `.tmp` files only on a Babel *version* change, which does not cover a content change within one release or a rebuild in place. The second round replaced it with an unconditional discard before every download. Both deletes are kept, for different reasons, which is why `CLAUDE.md` spells out which is which. **Copilot review** found four threads and one suppressed comment, all five real. Two were in the resume path and are folded into the rules above: a retry sent a bare `Range` whenever no validator was known, which is exactly the case where a splice cannot be detected afterwards; and a 416 whose HEAD carried no `Content-Length` was read as "already complete" when it is equally the answer for a file that shrank. Two existing tests had encoded the first behaviour by seeding a partial file with no validator — a state `get_downloaded_file` never produces — and were rewritten to reach the resume the way production does. The others: ten `get_curie_xref.cache_clear()` calls left over from an `lru_cache` that no longer exists, raising `AttributeError` in integration tests that skip and so never ran; `BABEL_ALLOW_VERSION_MISMATCH` missing from `env.default`, which survived because the test guarding that rule listed the settings by hand rather than reading them off the CLI; and this PR's own batched NodeNorm lookups still listed as future work. **Third review round** found seven, in five commits. Two were the same defect from opposite ends and are the reason the version-marker rules above are stated so explicitly: `_is_within_freshness()` returned `True` on `float("inf")` before looking at `last_checked`, so the whole cross-release refresh was a no-op under `--check-download never`; and the marker was stamped before anything had been re-downloaded, so an interrupted refresh looked complete to the next run. Both let `Concord.parquet` and `Identifiers.parquet` be read together across two Babel builds, which is precisely what the marker exists to prevent. The other three code findings: `xrefs --paths` rejected a single CURIE only inside `_print_paths`, after the 4.6 GB download; NodeNorm's deliberately-uncaught HTTP errors reached the user as a stack trace; and DuckDB spilled into the working directory. Two were documentation drifting from the code — `CLAUDE.md` described `@functools.lru_cache` in three places where none exists (disk caching by ETag, `cached_property`, and per-instance dicts respectively), and `CHANGELOG.md` carried a hard-coded test count that was already wrong, in a repo whose own `CLAUDE.md` says not to record them because they drift silently and then mislead. **History rewrite (2026-09-01).** The internal releases URL had been the hardcoded default from the initial commit until it moved into `.env`, leaving it in 178 of 180 commits across six branches of a public repository. Every commit was rewritten to remove it, verified by four independent checks before anything was pushed, with every changed line confirmed to be that substitution and nothing else. GitHub then refused to reopen the affected PRs, because their original head commits no longer exist — hence #20 here, and #21-#24 replacing #4, #6, #7 and #11. The rewrite does not un-publish anything: GitHub retains pre-rewrite objects reachable by SHA, and the tree the URL pointed at still serves 200 unauthenticated. Both are being handled outside this repo. **Verifying the release caught a stale claim.** `Concord.parquet` is 4.6 GB in `2026jul22`, not the ~626 MB a fixture docstring had claimed, so `pytest -m "integration and not slow"` — documented in two places as avoiding 2 GB+ downloads — would in fact pull 4.6 GB. The docs were corrected rather than the marker, and hard byte figures were dropped from the fixture docstrings for the same reason the repo already gives for test counts: they drift silently and then mislead. </details>
gaurav
added a commit
that referenced
this pull request
Sep 1, 2026
…t assumes (#31) `CLAUDE.md` had grown to 396 lines by accretion — each hard-won rule from a review round appended wherever it fit. It was accurate about this repository's internals and silent about the system those internals exist to read: it never said what Babel is, what a clique or a concord is, or why NodeNorm's version matters. An agent arriving at the `Concord.parquet` query code had no model of the data. This renames the file to `AGENTS.md`, gives it that missing context, moves the deep caching invariants to `docs/`, and removes what was being said twice. **Documentation only — no change to `src/`.** ## What's here **Renamed to `AGENTS.md`**, the filename coding agents other than Claude Code look for; the guidance was never Claude-specific. `CLAUDE.md` remains as a four-line pointer, so Claude Code still finds it. Four test docstrings that referenced the old name are updated. The rename is the first commit, so the substantive edits that follow read as edits rather than as a delete plus an add. **A Domain context section** covering only the terms this codebase actually uses — clique, preferred identifier, concord, conflation, Biolink type — and NodeNorm's relationship to a specific Babel release. It is a summary with links rather than copied upstream prose, so it cannot drift from upstream unnoticed. The part that earns the most space is the boundary between concords and cliques. Babel's own `AGENTS.md` says to answer clique-membership questions from a finished build *"never from the concords that fed it"* — and concords are exactly what this tool reads. That is not a defect, it is the purpose: `xrefs` reports the evidence Babel read, which is what you want when a merge looks wrong, while NodeNorm reports the verdict. Written down explicitly so that nobody later "improves" `xrefs` into a clique oracle. **The caching invariants moved to [`docs/Downloading.md`](docs/Downloading.md)** — near-verbatim, because the wording is hard-won and each rule records a specific failure that actually happened. `AGENTS.md` keeps a four-sentence summary and a pointer. That is ~90 lines an agent needs rarely, which had been sitting above the orientation it needs every session. **Deduplication.** The DuckDB spill directory was described in three places, the caching model in three, and the never-commit-the-internal-URL rule in three. Each is now stated once, where it belongs. `## Important Notes` is deleted outright — all three of its bullets restated earlier sections. The command examples now point at `README.md`, which documents every flag and is the user-facing reference. **Four factual corrections:** - `MissingBabelFileError` is raised on **any** 404, not only for `duckdb/` paths, as the file claimed in two places. `downloader.py:546` has no path condition. - CI budgeting pointed only at #18; #28 covers the same ground more precisely. - "recreated as #20-#24" predated #20 merging. - `README.md`'s configuration table was missing `BABEL_ALLOW_VERSION_MISMATCH`, which `env.default` has carried since #27. ## What upstream does not document Worth recording, because it shaped what could honestly be written. Neither Babel's `README.md` nor its `releases/ARTIFACTS.md` describes `Concord.parquet`, `Identifiers.parquet` or `Metadata.parquet`. `conflate` / `drug_chemical_conflate` and the `/status` `babel_version` field are absent from NodeNormalization's README. So the file schema in the new section is what **this repository's code assumes**, verified against real files — and it is labelled that way rather than presented as a published contract. Release naming, `latest` and `VERSION.txt` are undocumented upstream too. Filed upstream as [NCATSTranslator/Babel#1077](NCATSTranslator/Babel#1077), which links back to the Domain context section as the shortest statement of what is missing. ## What it deliberately does not do - **Document `search-xrefs`** — that is #21, not on `main`. - **Rewrite the version-skew wording** — #30 will change that behaviour; editing the prose now means editing it twice. - **Split Architecture or Testing into `docs/`** — those are context an agent needs often, and spreading them costs more than the length saves. ## Verification 272 unit tests pass and ruff is clean (no source changes, so both should be unchanged). Every relative link and intra-file anchor in `AGENTS.md`, `docs/Downloading.md` and `CLAUDE.md` resolves, no `.py` file still references `CLAUDE.md`, and the symbols the reworked text names were re-checked against the code. Nothing is blocking this merge. **One thing to judge:** `AGENTS.md` is 370 lines, down from 396, with a further 84 in `docs/`. Total documentation went *up*, because the Domain context section came out at 61 lines rather than the ~25 originally scoped. The extra went on the concord/clique boundary above. If that reads as too much for an agent file, the obvious trim is `## Testing` (73 lines, now the largest section).
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
1 participant
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.
Supersedes #11, which GitHub closed and refused to reopen after the branch history was rewritten. Original description below, unchanged.
Surfaces candidate cross-references from external mapping sources and diffs them against Babel's local Concord.parquet, to help maintainers find xrefs worth importing into Babel (e.g. NCATSTranslator/Babel#715, where CHEBI:31941 / PUBCHEM.COMPOUND:43805 / UMLS:C1314429 should form a single clique but don't).
Providers live in src/babel_explorer/core/providers/ behind a small Protocol + registry so new sources (UniChem, SSSOM/Mapping Commons, BridgeDb) can be added by writing one class and appending to PROVIDERS.
Each candidate carries query/target CURIEs, provider name, predicate, confidence, evidence URL, and an in_babel flag. --ignore-known filters out candidates Babel already knows about.