Realignment simplify pass: one rule, one target constant, docs back on their own items

[JamesKane]

A four-angle review (reuse, simplification, efficiency, altitude) of the realignment surface, prompted by two bugs that both traced to one rule existing in several copies.

The duplication that caused those bugs was still there

DEFAULT_TARGET_BUILD was exported specifically so the UI could name the build it offers — and then only the Simple-mode modal used it. Both Advanced cards still carried the literal, and sources.rs compared builds with eq_ignore_ascii_case where the job uses builds_match, which trims. A stored " chm13v2.0" would be offered by the card and then refused by the job.

is_target_build is now pub, and the UI asks it rather than spelling out its own comparison. The per-alignment card was also missing the bam_path condition, so it offered to re-map rows with no file — the job fails those at MissingPaths.

Four doc comments documented the wrong item

A new item inserted between an existing doc block and the function it documents, four times, twice on public API. The worst had one sentence severed across two items with its tail orphaned twenty lines below. Three were mine from this week.

Measured, not guessed

revert/writer.rs — 7 write_all per read into the gzip encoder	1,882 → 539 ns/record for identical bytes; ~13 min of CPU per realignment, on the stage that already holds the scratch peak
realignable_in_project — N+1 over members	17.7 ms → 2.7 ms on a 2,504-member project, using the grouped query that already existed and that project_report uses on the same tab; runs twice per batch

One bug, fixed at its own altitude

Simple mode's running card matched on alignment id — and once the offer was gone it had nothing to match against, so it claimed any running job. A page open on subject A during a job on subject B told A their genome was being rebuilt.

RealignState now carries the owning subject, resolved once per job by App::subject_of_alignment, and the card matches on that. Tightening the old predicate to is_some_and would have broken the "Done" card instead — the ownership question had to be answered where the alignment→subject mapping actually lives.

Also

preflight/resume_preflight were the same function twice differing in sizing and one noun; discard had two implementations; the project card cloned a Vec per frame to read its length; finished states carried progress fields no consumer reads; open_bam was a one-line alias; the confirm state re-spelled RealignOffer as a tuple; and a user-visible string held an 18-space run (visible in a screenshot of the project card).

Deliberately not done

• PacedFile doesn't reach navigator-align. mapped.bam — ~60 GB, the pipeline's largest write — is unpaced and uncounted, which is why the run log reads 0 MB/s through the whole mapping stage. Real, and the module exists precisely to record that; but it needs the accounting moved to a crate both sides depend on. Own change.
• cram.rs/finalize.rs bypass index::ensure_index, which is the guarded path for the CRAM multi-reference-slice panic. Correctness, not simplification — belongs to /code-review.
• Merge inflate is single-threaded (~10-20 min serialized), the reference index build is serialized behind the revert, and the finished 60 GB BAM is read twice (.bai, then SHA-256). All real; all performance features rather than cleanups.
• Batch policy lives in worker.rs rather than behind App — genuine altitude finding, but a refactor of a path that cannot be exercised end-to-end in a quality pass.
• The Advanced cards bypass i18n while Simple mode is fully keyed. Real asymmetry, but a pile of new strings plus Spanish.

The review also defended several things: the resume machinery is not redundant (resumable_by_marker has a production caller and the two rules catch different failures), the global write counter is the right shape, open vs open_many should stay a caller choice, and the Simple-only confirmation is correct because confirmation belongs to the reader's context rather than to the job.

Validation

983 tests, 0 failed — unchanged from before the pass, which is what a behavior-preserving cleanup plus one bug fix should look like. Clippy clean under -D warnings; fmt clean.

🤖 Generated with Claude Code