fix(contigs): guard the out-of-bounds ploidy read and the supercluster-merge null deref (#250)

[TimD1-bot]

Note

Authorship: the content below was drafted by Claude Opus 5 (an AI coding agent) and
filed via gh under @TimD1-bot, a bot account operated by @TimD1. It reflects the
agent's analysis, not a statement authored by @TimD1.

Closes #250.

What

Guards the two memory-safety defects in the contig-handling path so that a violated
invariant degrades to a skipped contig rather than undefined behaviour.

Neither defect is reachable on dev today. Both are held unreachable by a single
unstated invariant, so this changes no output and no metric — vcfdist's behaviour on any
input that reaches these lines today is byte-identical.

The invariant

intersect_contigs() leaves the query and truth callsets carrying identical contigs
lists, with every contig in those lists indexable in both callsets' variants[hap] maps.
It gets there by pruning both callsets to the BED contigs and injecting every BED contig
either one lacks, or — with no BED — by cross-injecting query into truth and truth into
query. Two consumers read past the end of a vector or dereference null without it, which
made the pruning load-bearing rather than merely tidy.

#250 proposed documenting that in intersect_contigs()'s doc comment; both added doc
paragraphs were removed on review, so the invariant is recorded in #250 rather than in the
source. Each guard carries its own one-line reason at the site.

The two guards

Site	Before	After
src/bed.cpp:643	a failed std::find became query->contigs.size() and indexed observed_ploidies out of bounds	the find result is kept as an iterator; a truth contig with no query counterpart is skipped
src/cluster.cpp:100	vars[h][ctg] inserted a default-constructed null shared_ptr for an absent contig, then ->n dereferenced it	the contig is looked up; an undeclared contig is treated as empty

superclusterData builds this->contigs as the union of both callsets' contig lists,
which is why the merge can be handed a contig one callset never declared at all. Since
supercluster() reads superclusters[ctg]->callset_vars[c]->n for every contig in that
same union, the merge still has to leave a container behind for a contig it skips — so the
undeclared case routes into the existing !nvars branch, which already pushes the single
trailing boundary that read needs.

Both guards cost nothing on the hot path: the merge runs once per contig per callset, not
per variant.

Verification

dev's test suite cannot distinguish these builds, because the invariant holds on every
input it feeds them — 839 unit tests and 132 integration workflows pass identically before
and after. So the guards were verified against a deliberately broken tree instead: an
AddressSanitizer build with the cross-injection and the FASTA pruning removed from
intersect_contigs(), run on a two-contig synthetic pair where one callset omits sc2
from its VCF header (a contig present in the header but never called is still
indexable, so header absence is what the second defect needs).

With the guards removed, both defects reproduce in exactly the two frames above (the
bed.cpp line number is lower on that tree, since removing the injection shortened the
file):

==3303==ERROR: AddressSanitizer: heap-buffer-overflow ... READ of size 8
    #0 intersect_contigs(...) bed.cpp:633
    #1 main main.cpp:55

==4411==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000168
==4411==Hint: address points to the zero page.
    #0 superclusterData::load_and_merge_callset_vars_across_haps(...) cluster.cpp:97
    #1 superclusterData::superclusterData(...) cluster.cpp:342
    #2 main main.cpp:97

With the guards in place and the invariant still broken, both directions exit 0 with no
sanitizer report. The tampering was local to the experiment and is not part of this
branch.

Tests

None added, at the author's request. #250 proposed three — the invariant asserted
directly, a superclusterData built from mismatched contig lists, and the ploidy
comparison under ASan — and they remain unwritten. Two of them would also need the ASan
workarounds #250 records, since CaptureStderr() swallows the sanitizer report and the
prebuilt libgtest is uninstrumented.

declare_contig()'s doc comment in tests/unit/src/test_cluster.cpp described the merge
as dereferencing without a null check, which is no longer true, so it now describes the
guard instead.

[TimD1]

remove this comment

[TimD1-bot]

Claude Opus 5 🤖: Removed in 4acd15e.

[TimD1]

remove this comment

[TimD1-bot]

Claude Opus 5 🤖: Removed in 4acd15e. I also reverted the two injection-site comments to their original wording, since they referred to "the invariant" and nothing defined it any more. Each guard keeps its own one-line reason at the site.