Hardening fast-follow (sort tie-break, @SQ check, BSD RSS, honesty pass) - #25
Merged
Conversation
…BSD RSS, honesty pass) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
HeapItem::cmp now tie-breaks equal sort keys by source_idx (reversed for the max-heap, so the lower index pops first = input order). Equal-key records (duplicate-marked reads, primary+overlapping-split sharing qname/pos/strand) therefore merge in a deterministic order independent of how they were partitioned into spill chunks — i.e. the sorted BAM is byte-identical across --memory-mb budgets. PartialEq made consistent with the now-total Ord. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
record_to_aligned_read mapped the reference by NAME only, so a BAM aligned to a different-length contig (a different assembly/patch) was silently accepted and produced coordinate-shifted/truncated calls. validate_contig_lengths now checks every shared @sq name's LN against the index ContigSet at open and errors on mismatch (naming the contig + both lengths), matching samtools/bcftools. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… KiB) The old not(linux) branch returned ru_maxrss raw, assuming macOS-style bytes — a silent 1024x UNDER-count on the BSDs (which report KiB), the dangerous direction: --enforce/verify would pass a job that breached its budget. Inverted the cfg: bytes only on Darwin (macos/ios), KiB->bytes everywhere else, so the unknown-target default is the safe over-estimate direction. Added a magnitude test that catches a dropped conversion on the run platform. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…tract The README/OPEN_PROBLEMS honestly demote √t to 'not yet load-bearing', but three skimmable surfaces still sold it as shipped. Fixed: Cargo.toml description and the CLI --help about-string now lead with the memory contract (not 'O(√t) space'); deleted scale_test_results.txt (an unreferenced root artifact whose 'O(√t) Space Complexity Verification' measured the tautological SpaceTracker counter, not real RSS); corrected CONTRACT.md's false 'byte-identical manifest' claim (the manifest embeds the machine-dependent realized peak_rss_bytes). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.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.
Summary
The cross-cutting hardening items the keystone (PR #24) deferred — four independent fixes, each tested where it's code.
HeapItem::cmpnow tie-breaks equal sort keys bysource_idx, so equal-key records (duplicate-marked reads, primary+overlapping-split sharing qname/pos/strand) merge in input order regardless of the spill-chunk partition → the sorted BAM is byte-identical across--memory-mbbudgets.PartialEqmade consistent with the now-totalOrd.@SQlength cross-check (finding Phase A4: spec-valid VCFv4.2 writer + BLAKE3 provenance receipt #7) —StreamingBamSource::newnow validates each shared@SQcontig'sLNagainst the indexContigSetand rejects a BAM aligned to a different-length reference (previously silently accepted → coordinate-shifted/truncated calls), matching samtools/bcftools.ru_maxrssunit fix (finding Phase A3: calibrated, abstention-aware genotype caller (call/) #6) — inverted the cfg to bytes iff Darwin, else KiB×1024, fixing a silent 1024× under-count on the BSDs (the dangerous direction for--enforce/verify) and making the unknown-target default the safe over-estimate. Magnitude test catches a dropped conversion on the run platform.Cargo.tomldescription + CLI--helpabout-string now lead with the memory contract (not "O(√t) space"); deleted the unreferenced rootscale_test_results.txt(its "O(√t) Verification" measured the tautologicalSpaceTrackercounter, not real RSS); fixedCONTRACT.md's false "byte-identical manifest" claim (the manifest embeds the machine-dependent realizedpeak_rss_bytes).Spec:
docs/superpowers/specs/2026-06-02-hardening-fast-follow-design.md.Deferred (further follow-up, not this PR): cgroup-awareness + manifest os/arch provenance (the heavier container-correctness work).
Test plan
cargo testgreen (incl. new merge-tie-break,@SQ-mismatch, and RSS-magnitude tests)cargo fmt --all -- --checkclean; 0 warnings (debug + release)🤖 Generated with Claude Code