Skip to content

Fix data-loss and alignment defects from the post-1.2.0 audit (plan A) - #74

Open
druvus wants to merge 12 commits into
mainfrom
fix-audit-data-safety
Open

druvus wants to merge 12 commits into
mainfrom
fix-audit-data-safety

Conversation

@druvus

@druvus druvus commented Oct 1, 2026

Copy link
Copy Markdown
Contributor

Implements plan A of docs/superpowers/specs/2026-10-01-post-1.2.0-audit-design.md (items A1-A10): defects that lose user data or produce a wrong alignment. The spec and the three plans (A, B, C) are the first commit of this branch.

What changes

  • MAFFT backend reorients reverse-strand genomes and contigs (--adjustdirection). Before: identity about 0.40 for a reverse-complemented query, 0.97 after.
  • reassort alignment-fraction filter compared in skani's percent scale (it never fired).
  • fill-references aligns the last round's downloads (final.msa.fasta); --curate runs before each build, including round 1 for a supplied collection; --reference is never removed by curation.
  • find-references --download <collection> --curate removes only what the run downloaded.
  • reassort --scan-segments cannot write outside its output directory; names that sanitise alike get separate directories.
  • MAF backends keep uncovered backbone contigs and write unplaced genomes as all-gap rows, named in a warning.
  • progressiveMauve rows named from the staged label.
  • Whitespace in sequence lines is cleaned at staging; malformed aligner output is reported by file.
  • A run no longer clears an existing, non-empty <output>/collection that no earlier run created (found by the branch review; pre-existing).

No region caller, default or recomb output schema changes. The two shipped examples give the same regions as on main.

Checks

  • ruff check src tests validation, mypy src: clean.
  • pytest -m "not requires_binary": 628 passed (586 on main).
  • pytest -m requires_binary with mafft 7.526 and minimap2 2.31: 5 passed.
  • Each new test was run against the pre-fix source and failed there.

Not verified

  • progressiveMauve: checked against a simulated XMFA only; the tool is not installed here.
  • Cactus path of the MAF change: no binary.
  • validation/run_reassort_benchmark.py after the filter fix, and validation/run_hybrids.py: not run (both need network). The reassort precision/recall may move now that the filter is active.
  • --adjustdirection on short or highly divergent contigs and inverted repeats: only the two integration cases were measured.

Review

One independent whole-branch review; its four important findings are fixed in 351d47d:
--reference had become the sibling test's anchor (a regression of this branch: a distant reference caused every closer genome to be dropped), a silent all-gap row for genomes aligned only away from the backbone, and the unguarded removal of <output>/collection. One limit is documented, not changed: a sibling that is itself the closest genome in a supplied collection becomes the curation anchor and is kept.

Deferred minor findings from that review:

  • an empty supplied --collection is seeded and then curated before round 1;
  • unique_scan_dirs compares names case-sensitively (HA / ha share a directory on a case-insensitive filesystem);
  • segment_scan.tsv does not record the scan directory name;
  • ref_contigs lengths are not checked against the MAF src_size;
  • two ValueErrors in maf_to_fasta.py still surface as "Unexpected error";
  • the whitespace-cleaning warning repeats on every build of the fill loop;
  • docs/aligners.md says an unplaceable genome is named in a warning for all backends; only the MAF path warns;
  • a curate_collection_dir docstring overstates what happens to a download that duplicates a protected genome;
  • after a final build, the last row of fill_summary.tsv describes the alignment before the last downloads.

Plan B (fix-audit-report-faithfulness) is stacked on this branch.

🤖 Generated with Claude Code

druvus and others added 12 commits October 1, 2026 22:32
A third audit of main (457bdfb) found defects at feature interactions that
the two earlier hardening passes did not reach. The spec records each
finding with its evidence and the design chosen; three plans split the work
by risk: data safety (A), report faithfulness (B), caller behaviour (C).
Each plan's code was applied in a scratch worktree before it was written.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
A genome or contig on the opposite strand to the backbone was aligned as given and
came out at chance-level identity. Run MAFFT with --adjustdirection.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
MIN_AF was a fraction compared against skani's percentage, so the filter never
fired and a partially aligning tip could become a segment's nearest strain.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
A segment record named '..' resolved the scan directory to the parent of the
output, whose collection/ was then replaced. Sanitise segment names with one shared
helper and give names that sanitise alike separate scan directories.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
With --download pointing at the collection itself, --curate deleted genomes that
were already there, and could pick a downloaded sibling as the backbone. Files
present before the run are now protected and excluded from backbone choice.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…-references

On a max-rounds exit the final downloads were counted but never aligned; --curate
did nothing unless a round both found a gap and downloaded a reference; and
--curate could delete the genome named by --reference. Curation now runs before a
build whenever the collection holds unfiltered genomes, --reference anchors it, and
a final alignment is built when the last round added references. A freshly seeded
collection is still not curated before round 1.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The MAF converter inferred the backbone layout and the genome set from the MAF,
which omits contigs and genomes that fall in no block. The sibeliaz adapter now
passes the backbone's contigs in file order, and both MAF backends pass the full
genome list, so a missing genome becomes an all-gap row with a warning.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Rows were named from the resolved path echoed in the XMFA, so symlinked
collections, unrecognised extensions and whitespace in a path gave wrong or
duplicate names. Rows are now taken by position and labelled from the staged file.
Checked with a simulated XMFA only; the tool was not available.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
A FASTA with trailing spaces, tabs or CRLF line endings gave a ragged alignment.
Such a genome is now staged as a cleaned copy and the reader ignores whitespace in
sequence lines, so the aligner and Tessera agree on its length. A nameless header,
a truncated MAF block, an XMFA without the reference and an empty MAFFT result are
reported as errors that name the file.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Correct the statements that MAFFT keeps insertions and that curation runs each
round; describe backbone contig layout, unplaced genomes and cleaned staging; add
the changelog entries.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…row, guard the working copy

The whole-branch review found one regression and three gaps.

- --reference had become the sibling test's anchor. That test is relative to
  its anchor, so with a distant coordinate reference every closer genome was
  dropped as a sibling. The anchor is again the query's closest whole-genome
  relative; the reference is protected from removal instead.
- A genome aligned only in MAF blocks without the backbone was written as an
  all-gap row with no warning; only genomes absent from the whole MAF were named.
- <output>/collection was removed unconditionally at the start of a run. A
  non-empty directory that no earlier run created is now refused, not cleared.
- The changelog and docs state the remaining limit of curation before round 1: a
  sibling that is the closest genome in the collection becomes the anchor and stays.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
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