feat(variant): subset Number=A/R/G fields to the emitted allele (#105)

[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 #105. Step 5 of 8 toward #48, stacked on #209 (104_td_keep-id-qual-filter-info-format).

Rebased onto the post-#229 writer. The previous step retained every source field whose values are
independent of the ALT list and deliberately omitted the Number=A/R/G ones, since their
values are positionally tied to the original ALT list while every output record carries a
normalized, split allele. This completes field retention by subsetting them instead of dropping
them, and removes the WARN that named what was dropped.

Subsetting

For a variant whose original ALT ordinal is alt_idx:

Class	htslib constant	Action
Number=A	BCF_VL_A	element alt_idx - 1
Number=R	BCF_VL_R	elements 0 and alt_idx
Number=G	BCF_VL_G	the genotypes over alleles {0, alt_idx}, via index k(k+1)/2 + j
Number=1, fixed, ., Flag	BCF_VL_FIXED, BCF_VL_VAR	verbatim

The ALT ordinal belongs to the variant, not to the record, so retention keeps each value whole
and the subset is taken at write time in src_info/src_fmt_vals (src/variant.cpp:566), ahead
of the re-typing set_info_field/set_source_formats already do. That is what makes the het-alt
case work: its two entries share one retained copy of the source columns and come out holding
different elements. Each field's length class is read from the input header once by
retain_header_lines into srcRecords::info_lens/fmt_lens, since the writer subsets without a
header to look it up in.

A value that cannot be tied to the emitted allele — an unknown ALT ordinal, or a list shorter
than the ordinal indexing it — is reported as . in FORMAT and omitted from INFO, which is how
each column expresses "missing".

Header cardinality: nothing is rewritten

An earlier revision of this PR rewrote Number=A→1 and Number=R→2 on the propagated
declarations. That is unnecessary and is now gone: set_var_record builds every record with
bcf_update_alleles(..., 2), so the records really are biallelic and the input's own declaration
already states the resulting cardinality — Number=A is one value, Number=R two, Number=G that
record's own genotype count. Rewriting to a literal is also actively wrong at some ploidy; htslib
rejects PL declared as anything else outright:

$ bcftools view -Ob -o /dev/null hap.vcf     # PL declared as Number=3
[W::bcf_hdr_check_sanity] PL should be declared as Number=G

All three declarations now propagate verbatim, and output_number() plus the bcf_hrec_dup
rewriting are deleted.

On bcf_remove_allele_set (the open comment on this PR)

The comment above proposed replacing the hand-rolled subsetting with htslib's
bcf_remove_allele_set() once #229 landed. #229 has landed, and I did not adopt it. Reason:
bcf_remove_allele_set selects among the alleles a record already holds, and no record in this
writer ever holds the source's ALT list.

#229's plan had #209 switch retention to bcf_dup() per source record, which is what would make
the call natural. #209 landed with the text-based srcRecords store instead — grep -c 'bcf_dup\|bcf_hdr_merge\|bcf_translate\|bcf_remove_allele' src/*.cpp on the base is 0.
set_var_record synthesizes each record from vcfdist's internal arrays, and those alleles are not
generally a subset of the source's:

• a CPX CAAGA>TT at 210 becomes G>GTT and GCAAGA>G at 209 — neither allele nor the
  position exists in the source record
• srcRecords retains no REF/ALT at all, so there is nothing to reconstruct one from

Using it as a pure value-subsetting engine on a scratch bcf1_t is possible but comes out larger
than the comma-split it replaces: it needs a separate one-sample header, typed values in and out, a
valid synthesized GT (per the ordering invariant the comment itself notes), and a placeholder
allele list. The part of the comment that does apply — that the cardinality rewriting is
unnecessary — is adopted above.

If the bcf_dup retention model from #229's table is still wanted, it belongs in a follow-up that
revisits #209's store, not here; I have not opened one.

Tests

• tests/unit/src/test_variant.cpp: 9 SubsetAltIndexed tests over the two entries of a het-alt
  record — A, R, and G each taking different elements per entry, the haploid G shape,
  pass-through of ALT-independent fields, and the ways a value can fail to be subset. Retention
  tests reworked from "dropped" to "kept whole", plus the recorded length classes and
  PropagatesAltIndexedHeaderCardinalityVerbatim.
• tests/unit/src/test_phase.cpp: HetAltEntriesReceiveDifferentAltIndexedValues pins the same
  thing end to end through the htslib writer, reading the two co-located records out of the file.
• tests/integration/test-integration.yml: the preserved_fields fixture's het-alt record gained
  AF=0.4,0.6 and distinct PL values so the alt_idx == 2 case (G elements 0, 3, 5) is
  observably different from alt_idx == 1. Both records are pinned in full, the three verbatim
  declarations are asserted, and must_not_contain guards that no whole unsubset list reaches the
  output.

Verification

• pytest: 134 passed (includes 779 gtest unit tests).
• make at -Wall -Wextra: clean, no warnings.
• doxygen: 0 warnings.
• bcftools 1.21 on the fixture's output: view, stats, and view -Ob (typed encoding, where a
  cardinality mismatch would surface) each with zero bytes on stderr. bcftools query reads
  back AF=0.4/0.6, AD=0,15/0,16, and PL=255,60,0/255,44,7 on the two het-alt records.

[TimD1-bot]

Claude Opus 5 🤖: Planned rework, filed as #229.

htslib's bcf_remove_allele_set() already does what this PR implements by hand: "Number=A,R,G INFO and FORMAT fields will be updated accordingly." Verified on htslib 1.17 against a het-alt A>C,G record — INFO/AF 0.1,0.2->0.1, FORMAT/AD 5,3,2->5,3, FORMAT/PL 0,1,2,3,4,5->0,1,2.

So subset_value/subset_info/subset_fmt_vals plus length_class/alt_indexed/output_number all go, and the SubsetAltIndexed suite becomes htslib's responsibility — roughly 250 lines down to ~10. The Number=A->1 rewriting is unnecessary once records are genuinely biallelic.

Ordering constraint: the synthesized GT must be written with bcf_update_genotypes before alleles are dropped, or the call fails with Problem updating genotypes.

No action needed until #229 lands.

[TimD1-bot]

Claude Opus 5 🤖: #229 has landed and this branch is rebased onto it. Taking the two halves of the comment separately.

Adopted — the cardinality rewriting is gone. set_var_record builds every record with bcf_update_alleles(..., 2), so the records are genuinely biallelic and the input's declaration already states the right cardinality. output_number() and the bcf_hrec_dup/bcf_hrec_set_val rewriting are deleted; Number=A, R, and G all propagate verbatim. This also retires the Number=G->3 deviation the earlier revision had to flag, since htslib rejects PL declared as anything but Number=G ([W::bcf_hdr_check_sanity] PL should be declared as Number=G).

Not adopted — bcf_remove_allele_set has no record to act on. It selects among the alleles a record already holds, and no record in this writer ever holds the source's ALT list. The plan in #229 had #209 switch retention to bcf_dup() per source record, which is what would make the call natural, but #209 landed with the text-based srcRecords store: on this base, grep -c 'bcf_dup\|bcf_hdr_merge\|bcf_translate\|bcf_remove_allele' src/*.cpp is 0. set_var_record synthesizes each record from vcfdist's internal arrays, and those alleles are not generally a subset of the source's — a CPX CAAGA>TT at 210 becomes G>GTT and GCAAGA>G at 209, where neither the alleles nor the position exist in the source record, and srcRecords retains no REF/ALT to reconstruct one from.

Driving it as a pure value-subsetting engine on a scratch bcf1_t is possible, but it needs a separate one-sample header, typed values in and out, a valid synthesized GT (the ordering invariant noted above), and a placeholder allele list — more code than the comma-split it would replace, not less. So the subsetting stays hand-rolled here, at ~40 lines in src_info/src_fmt_vals ahead of the re-typing the writer already does.

If the bcf_dup retention model is still wanted it belongs in a follow-up against #209's store rather than in this PR; I have not opened one.

Verification on the rebased branch: pytest 134 passed (779 gtest), make at -Wall -Wextra clean, doxygen 0 warnings, and bcftools view/stats/view -Ob on the fixture output each with zero bytes on stderr.