feat(observations): configurable observe_molecules; fix the secondary/supplementary filters (#82)

[rhshah]

Two commits, reviewable separately. Closes #82.

---

1. feat: observe_molecules takes the settings it actually uses

observe_molecules reads seven settings. GbcmsDnaConfig requires four. They do not overlap at all:

Required by GbcmsDnaConfig	variant_file, bam_files, reference_fasta, output
Actually read by observe_molecules	filters, quality, alignment, threads, apply_baq, umi_tag, library_type

And it is not merely four extra keyword arguments — variant_file and reference_fasta must name files that exist on disk, bam_files is a dict, and output is an OutputConfig. Changing one read filter meant fabricating paths that read as load-bearing and are not.

result = observe_molecules(
    "sample.bam", variants, reference_fasta="ref.fa",
    filters=ReadFilters(improper_pair=False),
    umi_tag="MI",
)

config= keeps working; an individual argument overrides the matching field.

umi_tag needs a sentinel. It is the one argument where None is a choice — "group by read pair, not UMI family". Under the uniform "None = not supplied" rule, passing umi_tag=None alongside a config that sets one would silently inherit it and change what counts as a molecule. Tested.

---

2. fix: the secondary/supplementary filters actually do something now

Both flags previously could not change any output — an unconditional skip discarded those records before fragment evidence, "regardless of the user filter flags". I audited all six read filters on real MSK-ACCESS data:

flag	before	evidence
filter_duplicates	✅ worked	dp 1202 → 1866
filter_improper_pair	✅ worked	dp 1202 → 0
filter_indel	✅ worked	dp 303 → 285
filter_secondary	✖ inert	unconditional skip
filter_supplementary	✖ inert	unconditional skip
filter_qc_failed	⚠️ untestable — no QC-failed reads in any available data

The line this draws

Simply honoring the flags breaks three existing tests, and they are right to break. test_supplementary_shared_qname_not_double_counted builds a primary and its supplementary over the same locus and asserts dp == 1: they are one physical read, and counting both reports depth 2. Two more tests assert the same read-level contract, matching the CLI help.

So the correct split is read-level vs fragment-level, not on/off:

	default (on)	opted out (off)
read-level dp/rd/ad	excluded	still excluded — same physical read
fragment dpf/rdf/adf	excluded	admitted
observation export	excluded	admitted

Fragments are immune to double-counting either way: hash_molecule keys on QNAME, so a primary and its supplementary collapse into one FragmentEvidence.

What it fixes

A locus reached only by a supplementary segment reported dpf=0 — not a filtered read but a wrong answer, since a molecule demonstrably covers it. That is what made a molecule spanning a large deletion invisible to cross-locus phasing. New test covers that arrangement; it is the other half of the double-count test's contract.

Verification

A) split read, locus reached only by the supplementary
   filtered  → dpf=0, observations at locus 1 only        (unchanged)
   opted out → dpf=1, observations at BOTH loci           (fixed)

B) primary AND supplementary over the same locus
   filtered  → dp=1
   opted out → dp=1                                        (contract preserved)

C) reconciliation invariant rows == dpf
   HOLDS under both settings

Default runs are byte-identical: total depth unchanged at 1202 across 40 real loci, and binned↔legacy parity passes (18 tests).

New consequence, documented

With the filter off, an admitted supplementary raises dpf while leaving dp unchanged — the two stop moving together. Recorded in the read-filters page, the observations page, and the CLI help.

Also documented the foot-gun found while auditing: MSK-ACCESS v1 alignments are 100% paired but 0% PROPER_PAIR, so --filter-improper-pair discards every read there.

---

Gate

• pytest — 421 passed; parity suite green
• cargo test — 215 passed; clippy --all-targets -D warnings clean
• ruff check ., black --check, mypy — clean
• mkdocs build --strict — 1 warning, unchanged from develop (pre-existing)

🤖 Generated with Claude Code