T-306: gate HSJ/XFORM SPR/NNI accept-paths to full_rescore

[ms609]

T-306 (P3) — HSJ/XFORM SPR/NNI accept-paths omit hierarchy DP contribution from best_score

Problem

The SPR accept path in tbr_search() (src/ts_tbr.cpp) and the accept path in nni_search() (src/ts_search.cpp) updated best_score with a Fitch-only (EW) or IW/profile incremental delta. For HSJ and XFORM scoring, score_tree() additionally adds a topology-dependent hierarchy-DP (HSJ) / Sankoff (XFORM) term that the delta omits, so best_score drifted from the authoritative score and the search's internal accept/reject decisions tracked the wrong objective.

Final user-visible scores stayed correct (run_single_replicate always recomputes via score_tree() before pooling), so this was a silent search-quality regression, not a wrong-output bug.

Fix

Gate the incremental fast path on ds.scoring_mode ∈ {EW, IW, XPIWE, PROFILE}; HSJ and XFORM fall back to full_rescore() in both accept paths — the same scoring-mode classification used by the T-275/T-303 guards. The NNI rejection-restore path likewise rebuilds the postorder for HSJ/XFORM rather than running a stale incremental downpass. hsj_score()/score_tree() call fitch_score_ew() internally, so the fallback leaves the Fitch state arrays in the same state as the EW full_rescore branch already exercised for TBR rerooting moves — safe for continued iteration.

The standalone spr_search() already full_rescores on accept, so it was unaffected; scope matches the task (only the two named paths needed changes).

Tests

New Tier-2 test tests/testthat/test-ts-t306-accept-guard.R drives the full HSJ and XFORM driven search on a brute-forceable 7-tip dataset (945 unrooted trees) and asserts:

• the search reaches the true global optimum (brute-forced via TreeLength);
• every returned tree's independent score equals the reported score (no best_score desync);
• identical seeds are deterministic.

The HSJ optimum on the test dataset is a strict subset of the Fitch optimum (3 ⊊ 15 of 945), so reaching it exercises the hierarchy-aware scoring, not just the Fitch component.

Note on discrimination: the bug is silent at the reported-score level — an empirical sweep confirmed that a buggy build still reaches the optimum from every Fitch-optimal "trap" start tree, because the driven pipeline's TBR rerooting moves (always full_rescore) recover the optimum regardless. A black-box MaximizeParsimony() test therefore cannot discriminate buggy from fixed (matching the T-303 sibling finding). The test instead locks in that the new full_rescore fallback yields a correct, consistent, deterministic search.

Validation

• Local targeted tests pass: new T-306 test (42 assertions) + existing test-ts-hsj (37), test-ts-xform (68), test-ts-tbr-search (28), test-ts-iw (86), test-ts-spr-nni-opt (45), test-ts-nni-iw-rescore (20) — no regressions.
• GHA agent-check dispatched: run 27528341506.
• Vignette search-algorithm.Rmd updated to document the HSJ/XFORM full-rescore fallback (per the AGENTS.md scoring-change rule).

Sibling task T-303 (sector path, same bug family) is handled separately in a different file set.

🤖 Generated with Claude Code

[ms609]

⚠️ GHA agent-check (run 27528341506) is red, but the failures are pre-existing on cpp-search and unrelated to this PR.

R CMD check reported 2 ERRORs, 2 WARNINGs, 1 NOTE, all in the WideSample/MaxMin feature area:

• examples ERROR: Error in MaxMin::FarFirst(colFn, m = n, N = nTrees, progress = FALSE) (from the MaxMin/highs solver work, e.g. b527a0f3).
• dependency WARNINGs: 'loadNamespace'/'library' call not declared from: 'highs'.
• spelling ERROR: DropAdd, FarFirst, Fathi, MMDP, MaxMin, Porumbel, Resende, Sayyady, WideSample, medoid, pkg — all WideSample/MaxMin terms missing from inst/WORDLIST.

This PR touches only src/ts_tbr.cpp, src/ts_search.cpp, tests/testthat/test-ts-t306-accept-guard.R, and vignettes/search-algorithm.Rmd — none of the failing areas, and it introduces no new spelling errors. cpp-search HEAD has not had a green agent-check since the MaxMin/WideSample work merged (last success was 2026-03-29 at an old SHA), so this branch simply inherits that breakage.

Local targeted tests on the built branch pass: the new test-ts-t306-accept-guard.R (42 assertions) plus test-ts-hsj (37), test-ts-xform (68), test-ts-tbr-search (28), test-ts-iw (86), test-ts-spr-nni-opt (45), and test-ts-nni-iw-rescore (20) — no regressions.

Recommend merging the MaxMin/WideSample CI fix (example dependency + WORDLIST) on cpp-search first, after which a re-run of agent-check on this branch should be green.