Skip to content

Add conservation-score tracks and a unified annotation catalog - #246

Open
lorenzoruggerii wants to merge 6 commits into
mainfrom
feat/2026-08-19-conservation-annotation-store
Open

lorenzoruggerii wants to merge 6 commits into
mainfrom
feat/2026-08-19-conservation-annotation-store

Conversation

@lorenzoruggerii

Copy link
Copy Markdown
Collaborator

GPN-Star, PhyloP 20-way, and PhastCons 7-way become optional IGV tracks on variant reports (analyze_variant_multilayer(..., show_conservation=True)), bulk-downloaded and cached like oracle weights rather than streamed per-region. A new AnnotationStore unifies these with GENCODE GTF annotations and user-added custom entries behind one list/describe/download interface (CLI chorus conservation / chorus annotation, and matching MCP tools), with downloaded bigwigs' declared genome build physically verified against the file's own chromosome-1 length.

lorenzoruggerii and others added 3 commits August 19, 2026 18:33
GPN-Star, PhyloP 20-way, and PhastCons 7-way become optional IGV tracks on
variant reports (analyze_variant_multilayer(..., show_conservation=True)),
bulk-downloaded and cached like oracle weights rather than streamed per-region.
A new AnnotationStore unifies these with GENCODE GTF annotations and
user-added custom entries behind one list/describe/download interface (CLI
`chorus conservation` / `chorus annotation`, and matching MCP tools), with
downloaded bigwigs' declared genome build physically verified against the
file's own chromosome-1 length.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Fixes from reviewing #246. Nine defects, each with a test that fails without the fix, plus the
documentation the new surface was missing.

**Silent wrong data**
* Conservation tracks were plotted onto non-hg38 reports. Every source is hg38, and a bigwig read
  against another assembly returns values rather than erroring, so an mm10 report showed human
  conservation against mouse coordinates. Now raises, checked before any work is done.
* No coverage rendered as MAXIMUM conservation: NaN -> 0.0, then invert -> clip(1-0,0,1) = 1.0,
  drawn as a solid full-height bar with autoscale off. Uncovered positions are dropped; genuine
  zeros still plot (both cases tested).
* The sequence-logo track sat one base left of every coolbox track above it -- coolbox passes
  0-based half-open, the height function reads 1-based inclusive. The existing test encoded the
  wrong expectation (4 values for a 3-base range); corrected, and pinned with a property test that
  the logo covers exactly the span coolbox does.

**Data loss**
* `chorus annotation remove --delete-file` unlinked the user's own registered file for kind="local"
  entries -- files chorus never downloaded, on a flag whose help says "the downloaded file". A test
  asserted this behaviour; it now asserts the opposite, with a second test for the case the flag is
  actually for.

**Reproducibility**
* The GPN-Star downloads passed no revision=, fetching songlab/gpn-star-scores at its head. Pinned
  to a7b13bbf. The existing pin guard reads only normalization.py, so it could not see the new
  sites; it now covers the artefact-download modules, and a new guard checks the track configs
  rather than the call site (the call moved into a shared helper, so a call-site scan would pass
  while the pin was None).

  Note: widening that guard package-wide flags 13 pre-existing unpinned model-weight sites
  (alphagenome_pt, cherimoya, chrombpnet, epinformerseq, legnet, sei, utils/annotations,
  _igv_report). Left alone deliberately -- a real question, but a separate policy call, not this
  PR's job.

**Crashes on ordinary input**
* A `custom_annotations.yaml` with an empty `annotations:` key took down every listing path with an
  AttributeError naming neither the file nor the key. setdefault only fires when a key is absent.
* A source with no derivable filename was accepted at registration, then failed at download with
  `'NoneType' object has no attribute 'exists'`. Now raises with advice at the point of use.
* A 0-byte file printed the optimistic size estimate rather than its real size -- the one case a
  user needs to see -- because the check was truthiness rather than `is not None`.
* The assembly check never ran on the path that reads the data: it was called from
  describe_annotation/add_annotation, which a report never touches. _bigwig_path verifies after
  download, with a confident mismatch raising and an unreadable file warning (the second half
  matters -- an earlier version of this fix turned a stub fixture into a hard failure of the
  download path).

**Duplication**
* One `hf_download_flat` helper replaces two line-for-line copies of the HF download-and-flatten
  block. They differed only in whether they passed revision= -- which is exactly how the unpinned
  download shipped.

**Documentation, which was absent**
* Nothing user-facing mentioned any of this: `show_conservation`, `chorus conservation`,
  `chorus annotation`, `AnnotationStore` and the three MCP tools appeared in 0 files across README,
  docs/ and examples/. README gains a Conservation tracks section, its MCP count goes 24 -> 27 with
  the new tools in the catalogue, and docs/API_DOCUMENTATION.md gains Annotations & Conservation.
* Three copies of the blurb (build_variant_report, analyze_variant_multilayer, and the text
  rendered into every report's HTML) described the logo track as IGV's dynseq showing
  clip(1-entropy,0,1). It is chorus's own stacked-logo track showing p(base)*(2-H) on a 0-2 bit
  scale. The same copies said ~25 GB of downloads; with the logo track it is ~70 GB.

**Recorded, not changed:** `igv.min.js` was replaced (1.35 -> 1.50 MB, dropping bundled jQuery
3.3.1) with no mention in the changelog. The 19 committed example reports still inline the OLD
bundle, and the browser check renders committed reports -- so the new bundle currently ships with
no CI coverage at all.

Also left for the author's judgement: add_annotation validating genome_build against a 5-entry
chr1-fingerprint table (rejects dm6/ce11, which chorus otherwise supports), _write_html_report
re-deriving a path instead of learning it from to_html, and discover_variant rendering the report
twice.

Suite on this branch: 2,257 passed / 38 skipped.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
lorenzoruggerii and others added 2 commits August 28, 2026 08:49
…eview-fixes

Review fixes for the conservation / annotation-store work
…alf the igv.min.js CI gap, ship the conservation notebook

Addresses items PR #247 explicitly left open rather than fixed:

- resolve_html_path: VariantReport and CausalResult now expose it, and both to_html and
  the MCP _write_html_report helper call it instead of re-deriving the output path
  independently. They used to disagree for an output_dir like /data/run.v2, where the
  dotted .v2 makes Path.suffix non-empty. Regression tests included.
- discover_variant no longer renders and writes its HTML report twice. It now builds the
  AnalysisRequest before calling discover_variant_effects (which already accepted one for
  exactly this) instead of stamping it on after the first write and writing again.
  Regression test included.
- Regenerated rs12740374_SORT1_chrombpnet_report.html, one of the two reports
  CHORUS_BROWSER_SMOKE=1 renders, so it embeds the new igv.min.js bundle (1.50 MB, no
  jQuery) instead of the old one. Verified with Playwright/Chromium against
  test_committed_reports_render_in_a_browser.py: the CI smoke subset and the full
  19-report corpus both pass. The other smoke report (cherimoya) and 17 more still carry
  the old bundle -- regenerating them needs envs unavailable in this session.
- single_oracle_quickstart.ipynb gains a Conservation Tracks section (show_conservation on
  the notebook's own GATA1 variant, plus an AnnotationStore listing), executed with real
  data rather than left unexecuted -- show_conservation, chorus conservation, chorus
  annotation and AnnotationStore appeared in no notebook before this.

Left alone, documented rather than patched: add_annotation's genome_build check rejects
dm6/ce11 even though chorus supports those genomes elsewhere, but the underlying
chr1-fingerprint verification scheme can't apply to them at all (no chromosome named
chr1) -- a real design decision, not a quick fix.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@lorenzoruggerii

Copy link
Copy Markdown
Collaborator Author

Follow-up commit responding to #247's review: 3d0b6b1.

Merged #247 in full first (fast-forward, no changes). This commit then addresses the items it explicitly left open:

Fixed (both were "left for your judgement" in #247, not required fixes):

  • resolve_html_path: VariantReport/CausalResult now expose it, and both to_html and the MCP _write_html_report helper call it instead of re-deriving the output path independently — they used to disagree for an output_dir like /data/run.v2, where the dotted .v2 makes Path.suffix non-empty. Regression test included.
  • discover_variant no longer renders and writes its HTML report twice. Builds the AnalysisRequest before calling discover_variant_effects (which already accepted one for exactly this) instead of stamping it on after the first write. Regression test included.

Partly closed — the igv.min.js CI-coverage gap:

  • Regenerated rs12740374_SORT1_chrombpnet_report.html, one of the two reports CHORUS_BROWSER_SMOKE=1 renders, so it now embeds the new 1.50 MB bundle instead of the old 1.35 MB one.
  • Verified with Playwright/Chromium against test_committed_reports_render_in_a_browser.py — not just inferred from size/content: the exact CI smoke subset (11 passed, 1 skipped) and the full 19-report corpus (46 passed) both pass, every canvas paints, no console/page errors.
  • Still open: the other smoke report (rs12740374_SORT1_cherimoya_report.html) and 17 more still carry the old bundle. Regenerating cherimoya's needs the chorus-cherimoya env, unavailable in the session that did this work — flagging rather than guessing at a fix.

Shipped — the doc gap:

  • single_oracle_quickstart.ipynb gains a Conservation Tracks section (show_conservation=True on the notebook's own GATA1 variant, plus an AnnotationStore.list_annotations() listing), executed end-to-end with real downloaded data rather than left unexecuted. show_conservation, chorus conservation, chorus annotation, and AnnotationStore appeared in no notebook before this.

Left alone, documented rather than patched:

  • add_annotation's genome_build check rejects dm6/ce11 even though chorus supports those genomes elsewhere — but the chr1-fingerprint verification scheme it's built on can't apply to them at all (neither has a chromosome named chr1). That's a real design decision about how (or whether) to verify non-mammalian builds, not a whitelist fix.
  • _write_html_report's remaining call sites, add_annotation's genome_build table, and the other 17 stale-bundle reports are all still real, just out of scope for this pass.

Full suite: 2182 passed (a pre-existing, unrelated set of ~40 environment-specific failures — stale background/provenance fixtures on this machine — confirmed identical with and without this commit's changes).

@lucapinello

Copy link
Copy Markdown
Contributor

Could you add a worked example for this before it lands? Right now the feature has no user-facing
entry point: show_conservation, chorus conservation, chorus annotation, AnnotationStore and
the three MCP tools appear in 0 files across README.md, docs/ and examples/, and the PR
touches no notebook or walkthrough. Someone reading the docs cannot discover any of it.

I've opened #247 against this branch with the prose half — a README Conservation tracks section,
the MCP count 24 → 27, and a docs/API_DOCUMENTATION.md entry — plus fixes for nine defects. But the
runnable half is better placed by you, since you know what the tracks are for scientifically.

Either of these would do it; no need for both:

a) A section in an existing notebook — examples/notebooks/single_oracle_quickstart.ipynb already
ends with a coolbox view, so conservation_logo_track() alongside it is a natural few cells. The
catch is the download: ~9.9 GB for entropy, ~45 GB more for the LLR bigwigs the logo needs. A
notebook that can't run without 70 GB is worse than none, so it may need to be phyloP-only (~9.2 GB),
or gated on has_gpn_star_bigwig() with a clear "download this first" cell.

b) A minimal walkthrough — examples/walkthroughs/conservation/<locus>/ following the shape of
the others (prompt at the top, MD + JSON + TSV + HTML). This is probably the better fit: it shows the
feature where it actually pays off — a variant whose conservation context changes the read — and it
sidesteps the notebook download problem because the artefacts are committed.

What would make either genuinely useful rather than a smoke test: a variant where conservation
changes the interpretation.
A strong predicted effect at a poorly conserved position, or a weak one
at a highly constrained base. That's the case a reader can't get from the oracle tracks alone, and it
answers "why would I turn this on?" in one figure.

Two things to know if you go the notebook route, both from #247:

  • It's hg38-only now — a non-hg38 report raises rather than plotting human scores against other
    coordinates.
  • The logo track is p(base) × (2 − H) on a 0–2 bit scale, not clip(1 - entropy, 0, 1). Three
    docstrings (including the blurb rendered into every report) said the latter; Review fixes for the conservation / annotation-store work #247 corrects them, so
    worth not re-copying the old wording.

…hlighting a strong predicted effect in a poorly conserved region
@lorenzoruggerii

Copy link
Copy Markdown
Collaborator Author

Hi Luca!

Thank you for pointing this out. I decided to add a minimal walkthrough.

examples/walkthroughs/conservation/SORT1_rs12740374/: scores rs12740374 with ChromBPNet DNase in HepG2 (show_conservation=True) — same variant and effect as the existing variant_analysis/SORT1_chrombpnet walkthrough (+1.376, ≥99th percentile, 1.18× the null max) — and reads phyloP/phastCons/GPN-Star directly at the variant's own base: phyloP −0.046, phastCons 0.001, GPN-Star entropy 0.772/2 bits. So here we have a strong opening effect in a poorly conserved region.

Committed via a new regenerate_examples.py entry (not hand-written) and the codegen'd notebook path, so it's regenerable like every other walkthrough.

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.

2 participants