Skip to content

fix(contract): predicted peak RSS is a true upper bound on real genomes (Act-1 PR-A) - #33

Merged
logannye merged 2 commits into
mainfrom
rosalind/act1-soundness
Jun 2, 2026
Merged

fix(contract): predicted peak RSS is a true upper bound on real genomes (Act-1 PR-A)#33
logannye merged 2 commits into
mainfrom
rosalind/act1-soundness

Conversation

@logannye

@logannye logannye commented Jun 2, 2026

Copy link
Copy Markdown
Owner

Summary

Act-1 hardening, PR-A (soundness keystone). The predicted-peak memory contract held on the synthetic/single-contig demo path but could be breached at runtime on a real multi-contig reference — the highest-severity finding from the reflection audit.

The gap (mechanism confirmed + quantified): call_germline_whole_genome decoded each contig with Arc::from(decoded), which reallocates — two ASCII-decoded copies of the largest contig briefly co-resident. But both the estimator (call/plan.rs) and the realized accountant (pileup/engine.rs) counted the reference once. So a [FITS] plan + the pre-run exit-3 gate could pass, then the run spikes past budget — caught only after the fact by the exit-4 check, i.e. after the allocation the contract promises to prevent up front. And no test compared predicted peak RSS to realized peak RSS (the existing soundness test is working-set-vs-working-set, both modeling the reference once → blind to the transient by construction).

What changed

  • decode_window_arc — decode straight into the Arc<[u8]> via a TrustedLen collect (no intermediate Vec), so peak resident reference memory is one copy, not the transient two. Used by both the germline and features whole-genome drivers. Byte-identical to decode_window (unit-tested).
  • Record predicted_peak_rss_bytes in the receipt, computed unconditionally — the contract's up-front claim, now carried in the manifest (also what the Act-2 plan --fleet scheduler needs).
  • PILEUP_IO_RSS_OVERHEAD — an honest RSS-level margin (htslib/BGZF buffers + BufWriter + allocator slack) added to the prediction only; the working-set estimate stays comparable to the realized accountant. The transient is eliminated at the source, so this margin covers fixed I/O buffers + slack, not a per-contig reference copy (so it need not scale with the genome).
  • The missing soundness testpredicted_peak_rss_upper_bounds_realized_peak: a 4 MiB-contig, shallow-coverage run asserting predicted peak RSS ≥ realized peak RSS, read from the receipt.

Measured (the TDD red→green)

predicted realized result
before fix 18.9 MiB 22.5 MiB ❌ RED — realized exceeds predicted by 3.4 MiB (the transient)
after fix 26.0 MiB 15.4 MiB ✅ +10.6 MiB headroom — transient eliminated (realized dropped 22.5→15.4)

The realized peak dropping by ~one contig copy confirms decode_window_arc eliminates the transient at scale — so the contract is sound on chr1-sized references by elimination, not by a fixed margin trying to cover a 237 MiB transient.

Test plan

  • New soundness test green; plan_enforce 11/11
  • decode_window_arc == decode_window byte-equivalence (unit)
  • Calls byte-identical — golden VCF + whole_genome_equals_per_contig_calls green
  • Full suite green (26 files); rustc 0 warnings debug+release; cargo fmt clean
  • Flagship plan/--enforce/verify re-run on the bundled toy fixture; receipt carries predicted_peak_rss_bytes
  • PR-A additions are clippy-clean (the repo's 59 pre-existing clippy lints + a CI clippy gate are a tracked PR-C hygiene item)

Act-1 sequence

This is PR-A of 3. Next: PR-B (real-genome correctness — multi-contig eval, contig-naming guard, IUPAC→N) and PR-C (trust on-ramp + hygiene — Action snippet, install.sh checksum, CI tautology, verify self-hash, clippy gate). Plan: docs/superpowers/plans/2026-06-02-act1-contract-hardening.md.

🤖 Generated with Claude Code

logannye and others added 2 commits June 2, 2026 12:51
…C scope

Derived from the 60-agent reflection audit. PR-A makes the predicted peak a
true upper bound on real genomes: kill the Arc::from reference-decode
transient, record the prediction in the receipt, add an honest RSS margin,
and add the missing real peak-RSS soundness test. PR-B (real-genome
correctness) and PR-C (trust on-ramp + hygiene) scoped for follow-on.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The predicted-peak contract held on the synthetic/single-contig demo path but
could be breached at runtime on a real multi-contig reference: the per-contig
`Arc::from(decoded)` reallocation briefly held TWO ASCII-decoded copies of the
largest contig, while both the estimator (call/plan.rs) and the realized
accountant (pileup/engine.rs) counted the reference exactly once. A `[FITS]`
plan could therefore breach, caught only post-hoc by the exit-4 check — and no
test compared predicted *peak RSS* to realized *peak RSS* (the working-set
tests are blind to the transient by construction).

This makes the up-front claim sound:
- decode_window_arc: decode straight into the Arc (TrustedLen collect, no
  intermediate Vec) — one resident reference copy, not the transient two.
  Used by both the germline and features whole-genome drivers.
- record predicted_peak_rss_bytes in the receipt, computed unconditionally
  (also what the Act-2 plan --fleet scheduler needs).
- PILEUP_IO_RSS_OVERHEAD: an honest RSS-level margin (htslib/BGZF buffers +
  BufWriter + allocator slack) added to the prediction only — the working-set
  estimate stays comparable to the realized accountant.
- the missing soundness test: predicted peak RSS >= realized peak RSS on a
  4 MiB-contig shallow-coverage run (RED before the fix by ~3.4 MiB; the
  transient drops realized 22.5 -> 15.4 MiB, headroom +10.6 MiB after).

Byte-identical calls (decode_window_arc == decode_window unit-tested; golden
VCF + whole-genome-equals-per-contig pin output). Full suite green, rustc 0
warnings debug+release.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@logannye
logannye merged commit b36be4f into main Jun 2, 2026
3 checks passed
@logannye
logannye deleted the rosalind/act1-soundness branch June 2, 2026 21:36
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