refactor(variant): pass add_var a named var_fields struct (#136)

[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.

ctgVariants::add_var took 27 positional parameters, 17 of them defaulted. Inserting a parameter
anywhere but the end silently rebound every argument after it, and the compiler could not object
because the neighbouring types are interchangeable. #116 shipped that bug once already: a caller's
supercluster argument became rec_idx, and it compiled clean under -Wall -Wextra.

Every field now travels in a var_fields aggregate, matched by designator instead of position.

vars.add_var(var_fields{.pos = 500, .rlen = 2, .type = TYPE_CPX, .loc = BED_OUTSIDE, .ref = "AC",
        .alt = "GT", .orig_gt = GT_ALT1_ALT1, .gt_qual = 21, .var_qual = 22, .phase_set = 33,
        .rec_idx = 12, .alt_idx = 3, .ploidy = 2, .supercluster = 7, .calc_gt = GT_ALT1_REF,
        .hap = {{.errtype = ERRTYPE_FN, .sync_group = 4, ...},
                {.errtype = ERRTYPE_TP, .sync_group = 5, ...}}});

What this buys

Insertion is safe. A new field is a compile-safe no-op for every call site that does not set
it, so the per-variant vectors planned in #48, #47, and #46 can land without the rebinding hazard.

Transposition is inexpressible. The 12 per-haplotype parameters are nested as hap[HAPS],
indexed by HAP1/HAP2, mirroring the storage layout. There is no longer a pair of adjacent
same-typed arguments to swap.

Required fields stay required. The 10 required members carry no default initializer and the 17
optional ones do, which is what makes -Werror=missing-field-initializers (added to both
Makefiles) reject an omission:

error: missing initializer for member 'var_fields::rlen' [-Werror=missing-field-initializers]

Two limitations worth naming. This is GCC-only — clang does not diagnose it even with the flag
passed explicitly, so macOS developers rely on CI (ubuntu-24.04) for the check. And it means a
future compiler release that reports this warning somewhere new becomes a build failure for
source-installing users; scoping the flag to the single warning rather than blanket -Werror
bounds that risk.

The hand-copies are gone. The four cluster.cpp merge sites each copied 13 parallel vectors
by hand — the exact shape that produced the #116 miswiring. They now read one var_fields through
a new ctgVariants::get_var accessor:

var_fields var = vars[HAP1][ctg]->get_var(var_idx[HAP1]);
var.orig_gt = GT_ALT1_ALT1; // present on both haps
merged_vars->add_var(var);

Why call sites name the type

Call sites write add_var(var_fields{...}) rather than add_var({...}). GCC 13.3.0 — the compiler
on ubuntu-24.04, and therefore on CI — rejects a bare designated-initializer list as a function
argument once any member is initialized from a non-constant expression:

error: cannot convert '<brace-enclosed initializer list>' to 'const var_fields&'

Upstream GCC 13.4.0 accepts the identical file, so this is a patch-level compiler difference rather
than anything about the code. Measured on GCC 13.3.0:

Form	Result
add_var({...}) into a const & parameter	error
add_var({...}) into a by-value parameter	error
add_var(var_fields{...})	compiles
named local, then add_var(var)	compiles

-std=c++20 does not help — the same error appears — so this is not a standard-level problem
and the C++17 setting is unchanged, as is the README's GCC 9.1+ floor. Naming the type still
enforces the required members: omitting one through var_fields{...} reports every missing member.
The reason is recorded in var_fields' own documentation so a later cleanup does not strip the
prefix and break the build again.

Behavioral equivalence

The old cluster.cpp calls stopped at ploidy, leaving supercluster, calc_gt, and the
per-haplotype fields at their defaults, whereas get_var copies the source's actual values. These
are equivalent because the merge runs from the superclusterData constructor before
supercluster(), and those fields are only ever written to the merged containers — the per-hap
sources stay pristine. add_var also still clamps both quals to g.max_qual, which is idempotent
on an already-clamped value.

Verified rather than assumed: on the chr20 fixture, every output file is unchanged against dev.
The only two files that differ are parameters.tsv and summary.vcf, each solely on the line that
records the invocation, because the two binaries were run from different paths; stripping those
lines leaves both identical, and precision-recall-summary.tsv matches metric for metric.

Verification

Check	Result
Build + unit suite, GCC 13.3.0 (ubuntu-24.04 container)	clean; 580 passed
Build + unit suite, local clang 15	clean; 580 passed
pytest	82 passed
chr20 fixture	unchanged vs dev, apart from the two invocation-recording lines above
Doxygen	no warnings
Enforcement	omitting .rlen fails the build on GCC

One tradeoff, unmeasured: get_var copies ref/alt into the struct and add_var copies again
into the vectors, so the merge path does one extra string copy per variant. The default build is
-g -pg -O1, so no performance claim is made in either direction; a move-aware overload would
remove it if it ever matters.

Notes for review

• var_desc in test_helpers.h is kept rather than folded into var_fields, so tests can keep
  describing a variant with a single qual and terse positional literals.
• Three comments documenting the old positional hazard were rewritten to describe what guards the
  fields now (test_variant.cpp, test_harness.cpp, test_helpers.cpp).
• Designators must appear in declaration order, which is why the cluster.cpp genotype override
  mutates a local instead of appearing in the initializer.
• This branch was rebased onto dev after fix(variant): clamp gt_qual to --max-qual in add_var (#135) #184 landed, which added a gt_qual clamp inside
  add_var and strengthened the quality tests; both are carried through here.

Closes #136