fix!: write MA-family tags as Ma/Aq/An (SAM local-use spelling) + legacy-tag hygiene - #124
Merged
Conversation
The SAM spec reserves all-uppercase two-letter tags for future standardization; tags with a lowercase letter are for local use. Add ma_family_aux(), which resolves an MA-family tag under its canonical uppercase spelling with fallback to the lowercase-second-letter variant (canonical wins when both are present), and use it on every read path. Writers still emit only the canonical spelling, but rewrites now strip both spellings so a stale copy can't survive under the other casing. This keeps BAMs readable through a possible future spec migration to the local-use spellings. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
MA-writing subcommands left consumed legacy tags (ns/nl/as/al/aq and fibertig fs/fl/fa) on records, so legacy readers saw stale calls forever and convert-tags could never clean them (its provenance guard skips records that already carry MA). ma_io::write_record now strips exactly the tags the reader consumed: set-level gates shared with read_record (type-checked — a foreign ns:Z or al:f never opens them) plus a per-tag aux-type check, and records already carrying MA are never touched. convert-tags drops its now-redundant removal loop and finally cleans fibertig tags too. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
mrvollger
pushed a commit
that referenced
this pull request
Aug 13, 2026
Keeps this PR scoped to the FIRE-qual overlay fixes (qc, decorators, filter expressions, mock-fire). The write_record legacy-tag hygiene and its tests move to fix/legacy-tag-hygiene, where they ship together with the Ma/Aq/An read-tolerance work. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
mrvollger
added a commit
that referenced
this pull request
Aug 13, 2026
…mock-fire (#123) Systematic sweep for the 0.10 MA-refactor bug class (FIRE quals moved off MSPs onto the `fire` type). This PR fixes the FIRE-qual overlay consumers: - **`ft qc --m6a-per-msp`**: `is_fire` was always false (read MSP quals directly). Now uses the shared `FiberseqData::msp_fire_quals()` overlay. - **`ft track-decorators`**: precision-0 MSPs lost their LINKER decorations vs 0.6 (only the `fire` view was iterated). All MSPs decorate again, and output ordering is now deterministic (was HashMap-ordered). - **`-x 'qual(msp)'` filter expressions**: compared against always-zero MSP quals, so `>N` dropped every MSP including called FIREs and `<N` kept them; `len(msp)` orphaned paired fire entries. Fire quals are overlaid by molecular start and msp/fire are dropped by a shared kept-start set. - **`ft mock-fire`**: emitted fire-only annotations invisible to msp-driven consumers; now writes the msp set alongside fire, matching real fire-scored records. - Doc cleanup: retired stale `AL`/separate-lengths references (lengths are inline in `MA:Z`). Every fix has regression coverage: insta snapshots pin the extract/qc/decorator FIRE rows, plus assertion tests for the filters and mock-fire tag layout. 20 regression tests pass; fmt/clippy clean. Related PRs: #124 carries the legacy-tag hygiene (provenance-checked stripping of consumed `ns/nl/as/al/aq` + fibertig tags) and the `Ma`/`Aq`/`An` read-tolerance helper, split out of this PR. Still outstanding for their own PRs: the py-ft port (compile-dead since 0.10) and the molecular-annotation python soft-clip liftover offset. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Mitchell R. Vollger <mvollger@gmail.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
… branch Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> # Conflicts: # tests/regression/fire.rs
The hts-specs proposal (samtools/hts-specs#862) names the tags Ma:Z, Aq:B:C, and An:Z — lowercase second letter, per the SAM reservation of lowercase-containing tags for local use. Writers now emit that spelling. Compatibility: readers accept both spellings, so all existing uppercase-tagged BAMs (fibertools 0.10-0.12) stay readable, and convert-tags/any rewrite migrates them to Ma. Uppercase wins when both spellings are present on a record: only an uppercase-only tool can produce that state (by replacing MA while leaving a stale Ma behind), so the uppercase copy is always the fresher write. BREAKING: fibertools <= 0.12.x cannot read Ma-spelled annotations and will see such BAMs as unannotated. Upgrade all fibertools installations together. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
mrvollger
changed the title
mrvollger
changed the title
The python bindings use the Rust core only for tag content; the aux tag names are read/written via pysam. Read both spellings (uppercase wins, same rationale as ma_family_aux), write the canonical Ma/Aq/An, and strip both spellings before writing so rewrites never leave a stale copy under the other casing. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…change Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Runtime write+read round trips can be self-consistently wrong; a committed fixture with only the canonical Ma/Aq/An spellings pins the format independently of the current writer. Generated once with ft convert-tags from msp_nuc.bam. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…evel Adversarial review of the spelling flip found three real defects: - Per-tag spelling fallback could pair a fresh uppercase MA (written by an 0.10-0.12 tool editing a Ma-spelled file) with the STALE canonical Aq/An siblings — misattaching quals/names, or wiping every annotation when the stale Aq count mismatched (from_record's parse-failure fallback substitutes an empty model, which the rewrite then persists). ma_family_aux is replaced by ma_family_tags(), which picks the spelling once per record from the type-checked main tag and reads the whole family in that spelling only. A wrong-typed foreign MA can no longer shadow a valid Ma:Z. - strip_consumed_legacy_tags gated at set level but the reader consumes pairs, so a lone `as`, or `aq` without an msp pair, was deleted without ever entering the model. Gates are now pair-level (ns+nl, as+al, aq only inside the msp pair, fa only with fs+fl), matching read_legacy_nuc_msp exactly. - CI never ran the molecular-annotation test suite (including the spelling tests); a dedicated step is added, plus regression coverage for uppercase->Ma migration via convert-tags and unit coverage for family atomicity, wrong-typed shadowing, and pair-level stripping. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The critic pass on the spelling flip found the python reader still resolved MA/Aq/An per-tag with no type check — retaining the stale-sibling misattachment and wrong-typed-shadow bugs the Rust side fixed with ma_family_tags(). _ma_family_tags() now mirrors it: spelling chosen once from the string-typed main tag (uppercase wins), siblings read only in the winning spelling. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> # Conflicts: # molecular-annotation/python/tests/test_molecular_annotation.py
…pelling, ignore profiler artifact Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Break-check follow-ups: - The reader bails the entire legacy parse on any length-mismatched pair, and writers then fall back to an empty model — so on such records NOTHING was consumed and the legacy tags are the only copy of the data. strip_consumed_legacy_tags now runs the same parse first and strips nothing when it errors, making strip and read consumption identical by construction (with a unit test pinning the mismatch case). Dead LEGACY_*_TAGS consts removed. - Mutation testing showed the python family-atomicity test did not pin the fix (suite passed with per-tag fallback reverted): it now asserts annotation names are None and adds a fresh-MA/stale-Aq-only case asserting the parse raises instead of borrowing the stale sibling. - python README no longer claims uppercase tags are generated. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Merged
mrvollger
pushed a commit
that referenced
this pull request
Aug 13, 2026
## 🤖 New release * `molecular-annotation`: 0.0.2 -> 0.0.3 (✓ API compatible changes) * `fibertools-rs`: 0.12.1 -> 0.13.0 (✓ API compatible changes) <details><summary><i><b>Changelog</b></i></summary><p> ## `molecular-annotation` <blockquote> ## [0.0.3](molecular-annotation-v0.0.2...molecular-annotation-v0.0.3) - 2026-08-13 ### Fixed - [**breaking**] write MA-family tags as Ma/Aq/An (SAM local-use spelling) + legacy-tag hygiene ([#124](#124)) - leading soft clips double-counted in pyMA liftover ([#126](#126)) - MA-refactor fallout — qc, track-decorators, filter expressions, mock-fire ([#123](#123)) </blockquote> ## `fibertools-rs` <blockquote> ## [0.13.0](v0.12.1...v0.13.0) - 2026-08-13 ### Fixed - [**breaking**] write MA-family tags as Ma/Aq/An (SAM local-use spelling) + legacy-tag hygiene ([#124](#124)) - make the Read the Docs (py-ft) build independent of the pyft Rust build ([#127](#127)) - MA-refactor fallout — qc, track-decorators, filter expressions, mock-fire ([#123](#123)) ### Other - cache cargo artifacts, parallelize jobs and tests ([#128](#128)) </blockquote> </p></details> --- This PR was generated with [release-plz](https://github.com/release-plz/release-plz/). Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
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.
Four changes to MA-family tag handling:
Ma/Aq/An— the spelling proposed to hts-specs (Describe Ma, Aq, and An molecular annotation tags samtools/hts-specs#862; the SAM spec reserves all-uppercase tags, so 0.10–0.12'sMA/AQ/ANwas wrong). Breaking: ft ≤ 0.12.x cannot seeMa-spelled annotations — upgrade installations together.ma_family_aux(); uppercase wins if both are present.ns/nl/as/al/aq, fibertigfs/fl/fa), provenance-guarded: records already carrying an MA-family tag are untouched, gates are type-checked so foreign same-named tags are never removed.Mabehavior.