Correct the compendium format docs, test the links, and make CI use the locked ruff - #973
Conversation
Three things in the output-format docs no longer matched the pipeline or its consumers. DataFormats.md said preferred_name was "not currently used by NodeNorm, but will be in the future". NodeNorm has consumed it since its #299; it is returned as the label of the normalized clique. The entry now says so, and keeps the more useful warning that the preferred name is not necessarily the clique leader's label. The synonym-file note said the files are built with GeneProtein conflation turned off. That is still true of the per-type files, but v1.13 added a separate synonyms/GeneProteinConflated.txt.gz so NameRes can be loaded with that conflation on. Both are now described. Architecture.md listed a clique-level "descriptions" field. write_compendium() does not emit one -- descriptions are per-identifier, in the d key, sorted shortest first -- so the entry is replaced with a description of what the identifiers list actually carries. The taxa entry is likewise clarified as the union of the per-identifier t values. Also removes a pointer to docs/debugging/Conflation.md, which does not exist. Whether that walkthrough should be written is now tracked in #971. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Conflation.md explains what NodeNorm does with the conflation files but never says how a caller actually turns a conflation on, so it now points at NodeNorm's API documentation for the flags. Deployment.md step 3.1 says to "split the Babel outputs into smaller files to improve load times" without saying what the split sizes are or why they matter. That is documented in detail on the NodeNorm side, so this links there instead of growing a second copy that can drift. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
"Descriptions are not included in NameRes" is a fact about a downstream frontend, asserted in Babel's docs, where nothing will notice if it stops being true. The durable Babel-side fact is that descriptions are written to the compendium files and the synonym files have no description field at all (see the document built in babel_utils.write_synonyms; it has curie, names, types, preferred_name, shortest_name_length, clique_identifier_count, curie_suffix and taxa). Which frontend shows a description then follows from which files it was built from, without this document having to track anyone else's behaviour. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
NCATSTranslator's Babel, NodeNormalization and NameResolution all default to main. A /blob/master/ URL resolves only through GitHub's post-rename redirect, which is the kind of thing that disappears quietly. Seven of them here: two notebook links in README.md, one in docs/Babel.ipynb, and one each in Conflation.md, Deployment.md (x2) and the disease concord URL reported by diseasephenotype.snakefile. CITATION.cff still named TranslatorSRI/Babel as repository-code, which is what Zenodo and GitHub's citation widget read. Deliberately not touched: every TranslatorSRI/babel-validation reference. That repository really does still live under TranslatorSRI -- NCATSTranslator/babel-validation is a 404 -- so those are correct, not rot. The TranslatorSRI strings in input_data/manual_concords/drugchemical.tsv are data, not links. Verified: all eight rewritten URLs return 200, the notebook is still valid JSON, CITATION.cff still parses, and rumdl is clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Without something failing CI, the master links just fixed drift back. Babel, NodeNorm and NameRes now all run the same three checks: relative links resolve, heading anchors exist, and nothing is pinned to `master`. Offline, so a slow GitHub cannot make it flaky. Adapted rather than copied. Babel has no API server, so the endpoint-anchor check the other two have has no equivalent; instead the master ban also covers the snakefiles and src/, which report download and provenance URLs to users, and CITATION.cff. The exclusion list covers the gitignored data/, babel_outputs/ and babel_downloads/ directories, which accumulate generated reports that are not ours to check. The master pattern is scoped to NCATSTranslator so it does not flag biolink-model, which really does default to master, or TranslatorSRI/babel-validation, which really does still live there. It found two dead links on its first run: releases/TranslatorFuguJuly2024.md and releases/TranslatorMay2024.md both link to the previous release without the .md extension, so both render as dead links on GitHub. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Two link commits here. Cross-repo links now point at Deliberately not touched: every A test to stop it drifting back ( It found two dead links immediately: Verified: all eight rewritten URLs return 200, the notebook is still valid JSON, |
Babel is already clean, so this is a guard rather than a fix -- it keeps it that way, and matches the same check going into NameResolution#262 and NodeNormalization#403 alongside their sweeps. Scoped to the three repositories that actually moved, by name. Babel is the repo where a blanket ban on the TranslatorSRI string would do real damage: TranslatorSRI/babel-validation is referenced from docs/Deployment.md, docs/Triage.md and src/exporters/sapbert.py, and really does still live under that org, so an unscoped rule would push someone to turn three correct links into 404s. The comment names them. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The explicit list of link-bearing files missed docs/Babel.ipynb, whose link I had to fix by hand earlier in this branch, and would miss anything added later. Globbed by extension instead, which also picks up the workflow and issue- template YAML under .github/. Matches the same change in NameResolution#259 and NodeNormalization#405. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR updates Babel’s documentation to accurately reflect the current Babel→NodeNormalization contract (especially around compendium fields, synonym/conflation variants, and descriptions), and replaces drift-prone restatements with links to canonical NodeNorm docs. It also adds an offline test to keep documentation/source links consistent across Babel/NodeNorm/NameRes.
Changes:
- Correct compendium/schema documentation details (e.g.,
preferred_nameusage, description/taxa semantics) and remove a dangling reference to a non-existent doc. - Replace stale
master/old-org GitHub links withmainandNCATSTranslator, and add cross-repo doc links to NodeNorm where appropriate. - Add an offline documentation link checker test intended to enforce shared link rules across repos.
Reviewed changes
Copilot reviewed 12 out of 15 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
tests/test_docs_links.py |
Adds offline tests to validate relative links, heading anchors, and ban stale GitHub URL forms. |
src/snakefiles/diseasephenotype.snakefile |
Updates a GitHub URL from blob/master to blob/main. |
releases/TranslatorMay2024.md |
Fixes an internal release-note link to include the .md extension. |
releases/TranslatorFuguJuly2024.md |
Fixes an internal release-note link to include the .md extension. |
README.md |
Updates NodeNorm/NameRes notebook links from master to main. |
docs/Understanding.md |
Clarifies where descriptions exist (compendia-only) and how downstream frontends can expose them. |
docs/Deployment.md |
Links to NodeNorm loader documentation instead of duplicating split-size rationale; updates NameRes deployment link to main. |
docs/DataFormats.md |
Corrects/clarifies preferred_name usage, synonym file variants re: GeneProtein conflation, and removes a pointer to a non-existent debugging doc. |
docs/Conflation.md |
Links to NodeNorm API docs for how to enable conflation flags at query time. |
docs/Babel.ipynb |
Updates a GitHub link from blob/master to blob/main. |
docs/Architecture.md |
Corrects compendium field descriptions (identifier-level descriptions/taxa; removes nonexistent clique-level descriptions). |
CITATION.cff |
Updates repository URL to https://github.com/NCATSTranslator/Babel. |
Comments suppressed due to low confidence (1)
tests/test_docs_links.py:43
SOURCE_WITH_LINKS = REPO_ROOT.rglob("*")still traverses excluded directories like.git,babel_outputs/, andbabel_downloads/(the filter happens after traversal). On developer machines and even in CI, that can make this unit test unexpectedly slow and/or hit the global timeout.
Consider walking with os.walk() and pruning EXCLUDED dirs, so excluded trees are never descended into.
SOURCE_WITH_LINKS = sorted(
p for p in REPO_ROOT.rglob("*")
if p.is_file() and _ours(p)
and (p.suffix in LINK_BEARING_SUFFIXES or p.name.startswith("Dockerfile"))
)
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
These rode along with the cross-repo link fixes and belong to none of them: a Finder .DS_Store, an Apple Numbers working copy of drugchemical.tsv that nothing references, and an EMAPA source-impact detail CSV that #781 had already decided not to commit (docs/sources/EMAPA/impact-report.md says so in as many words), so it arrives here without the report that would explain it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
It carried no mark, so `pytest -m unit` -- what CI runs on every PR --
deselected all of it; a guard test nobody runs is worse than none, because it
reads as coverage. It also failed `ruff format --check`, which would have
failed the lint job.
The file lists came from rglob("*") over the repo root with the exclusions
applied afterwards, so babel_downloads/, babel_outputs/, data/ and .venv/ were
all descended into in full before being discarded -- at import time, so even a
run that deselected these tests paid for it. Asking `git ls-files` instead
scopes the walk to tracked files, which is also the right scope: an untracked
file cannot carry a link anyone will follow.
The rest is upkeep. The two banned-URL tests were the same loop twice, so
they are one parametrized test; the `master` ban now covers the tree and raw
forms and a bare `/blob/master`, without catching a branch really named
`master-something`; reads are explicitly UTF-8; and the scan skips this file
rather than relying on the backslashes in its own patterns to keep them from
matching themselves.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Architecture.md is the format tour, so "there is no clique-level descriptions field" was answering a question the reader had not asked; saying where the descriptions do live is enough. Also an en-dash pair left as `--`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Ruff 0.16 formats the Python inside fenced Markdown blocks. Ours are illustrative fragments rather than modules, and the reformatting is not an improvement: tests/pipeline/README.md shows two comma-terminated entries of a check list, and 0.16 "fixes" each into a parenthesised tuple, which is not what a reader should copy. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The formatting job installed whatever ruff was newest, so it was checking the repo with a version nobody has locally: 0.16.0 in CI against the 0.15.21 in uv.lock. That is how this PR -- which touches no Python at all -- failed on three Markdown files. `version-file: uv.lock` makes the job run the locked version, so a ruff upgrade arrives as a lockfile change that can be formatted for in the same PR, rather than as a red X on an unrelated one. v4 while here: it runs on node24, so the job stops warning that node20 is deprecated. It has to be pinned exactly -- releases from v4 on are immutable and there is no moving `v4` tag. The setup-python step goes too; ruff is a standalone binary and never used it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
# Conflicts: # pyproject.toml
#981 added the warning that a ruff or rumdl bump can change their output; now that the ruff job reads its version from uv.lock, say where that lands. It is the upgrade PR's job, not a surprise for whoever opens the next PR. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Documentation fixes found while working out a clean division of labour between Babel's docs and
NodeNorm's, plus a test that keeps the cross-repo links from rotting again. Companion to
NCATSTranslator/NodeNormalization#403.
Corrections
preferred_nameis used by NodeNorm.DataFormats.mdsaid it was "not currently used byNodeNorm, but will be in the future". NodeNorm has consumed it since its #299 and returns it as the
label of the normalized clique. The entry now says so, and keeps the more useful warning that the
preferred name is not necessarily the clique leader's label.
Synonym files and GeneProtein conflation. The note that synonym files are built with GeneProtein
conflation turned off is still true of the per-type files, but v1.13 added a separate
synonyms/GeneProteinConflated.txt.gzso NameRes can be loaded with that conflation on. Both arenow described.
Architecture.mdlisted a clique-leveldescriptionsfield.write_compendium()does not emitone — descriptions are per-identifier, in the
dkey, sorted shortest first. Replaced with adescription of what the
identifierslist actually carries, and clarifiedtaxaas the union ofthe per-identifier
tvalues.Removed a pointer to
docs/debugging/Conflation.md, which does not exist.Cross-references
Conflation.mdexplains what NodeNorm does with the conflation files but never says how a callerturns a conflation on, so it now points at NodeNorm's API documentation for the flags.
Deployment.mdstep 3.1 says to "split the Babel outputs into smaller files to improve load times"without saying what the sizes are or why it matters. That is documented in detail on the NodeNorm
side, so this links there rather than growing a second copy that can drift.
Link hygiene, and a test that enforces it
Several cross-repo links pointed at
master, which none of these repositories uses any more:README.md,docs/Babel.ipynb,docs/Deployment.md, and a provenance URL reported to users fromsrc/snakefiles/diseasephenotype.snakefile. They resolve today only through GitHub's branch-renameredirect.
CITATION.cffstill namedTranslatorSRI/Babelasrepository-code, which is the URLZenodo reads. Two release notes linked
[May 2024](TranslatorMay2024)with no.md, so they neverresolved at all.
tests/test_docs_links.pynow enforces four rules across every tracked Markdown file, and — forthe URL rules — every tracked file that can carry a GitHub link (snakefiles,
CITATION.cff,notebooks, YAML, Python):
masterbranch of an NCATSTranslator repositoryTranslatorSRIorg for Babel, NameRes or NodeNormBoth bans are deliberately scoped by name: biolink-model really does default to
master, andTranslatorSRI/babel-validationreally is still under that org, so a blanket ban would push someoneto "fix" correct links into 404s.
Everything is offline — nothing fetches a URL, because a test that fails when GitHub is slow is a
test people learn to ignore. It is marked
unit, so per-PR CI runs it, and it discovers filesthrough
git ls-filesrather than walking the repo, which keeps it out ofbabel_downloads/andbabel_outputs/. Ported from NameResolution's test of the same name (NameResolution#262,NodeNormalization#403); the three copies should be kept in sync.
CI runs the ruff in
uv.locknowThe formatting job installed whatever ruff was newest rather than the project's, so it was checking
the repo with a version nobody has locally — 0.16.0 in CI against 0.15.21 in the lockfile. That is
how this PR, which touches no Python at all, came to fail on three Markdown files: ruff 0.16 began
formatting the Python inside fenced blocks.
version-file: "uv.lock"makes the job run the lockedversion, so a formatter upgrade arrives as a lockfile change that can be reformatted for in the
same PR instead of as a red X on an unrelated one. #981 has since landed the other half of this —
extend-exclude = ["*.md"], so ruff leaves the fragments in our docs alone — andCONTRIBUTING.mdnow says where a bump's reformatting is due.
While in the file:
astral-sh/ruff-actionv3 → v4.1.0, which runs on node24 and so drops the"Node.js 20 is deprecated" warning. It is pinned exactly because releases from v4 on are immutable
— there is no moving
v4tag, and@v4does not resolve.The
actions/setup-pythonstep is gone: ruff is a standalone binary and never used the Python itinstalled. That was the repository's only use of the action, so this supersedes #966 (Dependabot
bumping it 6 → 7), which can be closed.
Also removed
Three files committed by accident at some point, belonging to none of the above:
.DS_Store(nowgitignored),
input_data/manual_concords/drugchemical.numbers(an Apple Numbers working copy ofdrugchemical.tsvthat nothing references), anddocs/sources/EMAPA/impact-report/modified-cliques.csv, which #781 had already decided not tocommit.
Deliberately not changed
DataFormats.mdsays there are "only two conflation files". I initially had this down as wrongbecause
DrugChemicalSmaller.txt.gzappears in two release notes — but it is a SapBERTtraining-data export (
src/exporters/sapbert.py), not a conflation file. The existing text iscorrect.
Conflation.mddocuments NodeNorm's query-time behaviour, which looks at first like it belongs inthe other repo. Leaving it: it is where readers actually ask the question, and NodeNorm now
restates the contract from its own side.
Verification
uv run pytest -m unit— 406 passed (402 before; the four new link tests are the difference)ruff check,ruff format --check,snakefmt --check,rumdl check .— all clean, re-runagainst ruff 0.16.0 after merging main
uvx ruff@0.16.0 format --check .and the locked 0.15.21 agreed at 191 files before Upgrade all dependencies, and document how to do it #981 raisedthe lock to 0.16.0
Follow-ups filed
#971 (three documented paths that do not exist:
docs/pipelines/,docs/sources/EMAPA/*.md, andthe
docs/debugging/Conflation.mdwalkthrough removed here — the walkthrough still seems worthwriting), #972 (the "General releases" section of
releases/README.mdis an empty TODO, whichmatters because that index is effectively the provenance table behind NodeNorm's
/statusbabel_version).🤖 Generated with Claude Code