fix/docs/test: additional correctness fixes, doc sweep, and expanded coverage (6/6) - #463

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@CHANGELOG.md`:
- Around line 25-31: The changelog entry for fgumi runall incorrectly suggests
it can start from extract, but runall’s documented stage range begins after
extraction. Update the wording in the runall bullet to describe it as the fused
post-extraction pipeline, and if mentioning end-to-end usage, reference the
composed fgumi extract | fgumi runall --start-from correct flow instead of
implying extract is part of runall.

In `@docs/src/guide/best-practices.md`:
- Around line 71-76: Clarify the `fgumi runall` tip so it does not read like an
end-to-end raw FASTQ pipeline: update the wording around `runall` in the
best-practices guide to state that it is post-extract only and starts after the
separate `extract` step, covering `correct`, `align`, `zipper`, `sort`, `group`,
and `consensus`. If helpful, mention the composed workflow explicitly or show
that `extract` must be run first, and keep the existing
`--start-from`/`--stop-after` guidance tied to `runall`.

In `@docs/src/index.md`:
- Around line 74-76: Update the `runall` entry in the command summary so it no
longer describes the pipeline as full FASTQ→consensus; revise the wording in the
table near `runall` to reflect that it starts post-extraction, such as
BAM→consensus, and keep the surrounding help text unchanged.

In `@tests/integration/test_runall_parity.rs`:
- Around line 902-939: `deterministic_simplex_fixture` is not actually
generating reads across multiple template-coordinate positions, so the
determinism test is weaker than intended. Update the fixture used by
`runall_simplex_record_stream_is_deterministic` to place each family at a
distinct position, using a positioned helper like `create_paired_umi_family_at`
or an equivalent single-end positioned builder instead of `create_umi_family`.
Keep the existing `Sort`/`run_standalone` flow, but ensure the generated BAM
spans many coordinates so `ByItemOrdinal` and worker-order nondeterminism are
exercised.

In `@tests/integration/test_sort_correctness.rs`:
- Around line 598-629: The test coordinate_sort_consistent_across_threads
currently only validates each thread count independently, so it does not enforce
the stated cross-thread consistency contract. Update this test to sort the same
input with threads set to 1, 2, and 4, then compare the resulting
coordinate-ordered record stream (or equivalent key sequence) across runs in
addition to keeping the existing sortedness and preservation checks. Use the
existing helpers fgumi_sort_in_process, verify_sorted,
assert_coordinate_ordered, and assert_records_preserved to keep the test focused
on comparing outputs rather than just self-validating each case.

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/lib/commands/runall.rs`:
- Around line 563-567: The `sort` and `group` help text in `runall.rs` is too
absolute and implies fixed downstream stages even when `--stop-after` changes
the chain. Update the doc comments around the `sort`/`group` bullets to describe
only the input contract for each start point, and make the mention of following
stages conditional on `--stop-after` so the text aligns with the valid
`sort→sort` and `group→group` flows handled by the `runall` command logic.

In `@tests/integration/test_dedup_command.rs`:
- Around line 211-225: The test in test_dedup_command uses
alignment_start().unwrap().unwrap().get() and asserts 1-based coordinates, but
the inline projection comment still says 0-based positions 99/199. Update the
comment near the surviving/projection check to match the actual oracle values,
or adjust the projection/assertion to use a consistent coordinate system,
keeping the is_first_segment and is_reverse_complemented checks aligned with the
expected pair.

---

Outside diff comments:
In `@src/lib/pipeline/chains/builder.rs`:
- Around line 1504-1508: Update the stale inline comments in add_correct so they
match the current add_align behavior: when the chain tail is a BamTemplateBatch,
add_align skips GroupByQueryname instead of re-wiring it. Keep the wording
consistent with the corrected rustdoc above and the chain-tail logic in
add_align, and revise the comments near the Intermediate/SerializeBamRecords
explanation as well as the duplicate later comment block.

---

Duplicate comments:
In `@tests/integration/test_sort_correctness.rs`:
- Around line 444-453: The oracle in assert_name_order_matches only compares
record_key(r).name, so it can miss per-record differences like swapped mates
within the same name group. Update the comparison to use a per-record projection
that includes the full identity you want to validate, such as record_key(r)
itself or the full RecordBuf, for both fgumi_out and samtools_out, and keep the
count check plus the direct equality assertion so the natural-sort test verifies
exact output identity rather than only qname order.

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/lib/umi/parallel_assigner.rs`:
- Around line 1525-1561: The new invalid-UMI adjacency test only compares
AdjacencyUmiAssigner and ParallelAdjacencyAssigner against each other, so it can
miss a shared regression from fgbio. Update the adjacency invalid-UMI test
around adjacency_invalid_umi_maps_to_none_in_both_paths to include a
programmatic fgbio oracle for the mixed and all-invalid inputs, or reuse an
existing adjacency baseline that already covers these cases. Use the existing
assign() calls and MoleculeId assertions to verify both implementations still
match the expected fgbio behavior, especially that invalid UMIs map to
MoleculeId::None.

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@tests/integration/test_dedup_command.rs`:
- Around line 211-225: The surviving-pair assertion in the dedup integration
test is too weak because it only checks “not first” for the second mate, so a
malformed record could still pass. Update the projection and/or final assertion
in the test around surviving records to require flags.is_last_segment() for the
R2 mate (or explicitly assert one first and one last) alongside the existing
position and reverse-complement checks, using the rec.flags() projection in
test_dedup_command.rs.

---

Outside diff comments:
In `@src/lib/pipeline/steps/coalesce.rs`:
- Around line 250-259: The coalescing fixture in coalesce.rs never hits the
intended “threshold + one block” overshoot because COALESCE_INPUT_BLOCK_BYTES
divides COALESCE_THRESHOLD_BYTES exactly, so the threshold check in the
coalescing path only ever flushes at an exact boundary. Adjust the test setup
around COALESCE_INPUT_BLOCK_BYTES, COALESCE_THRESHOLD_BYTES, and
COALESCE_INPUT_BLOCKS so the input block size does not evenly divide the
threshold, ensuring at least one flush in the coalescer overshoots by a partial
block and the per-block ceiling assertion actually verifies the intended bound.

---

Duplicate comments:
In `@src/lib/commands/runall.rs`:
- Around line 429-439: The help text for the reference FASTA argument still
mentions the stale internal `ref_is_aligner_only` exemption. Update the
documentation in `runall.rs` so it refers instead to the derived-chain
align/zipper exemption in `execute`, while keeping the rest of the conditions
described by the `validate_align_and_merge` and `Stage::Align` logic unchanged.