A15-03: eight contract/documentation drifts in the legacy search API (stopAtScore crash, stale edge.length, ignored suboptimal, three epsilons, no-op hits)

[ms609]

Severity: sev:low · Area: 15 (Legacy pure-R search API)

Covers red-team candidates A15-O-13, -14, -19, -21, -22, -23, -25, -26. Eight small defects in TreeSearch()/Ratchet()/SuccessiveApproximations() where the code and its documented contract have drifted apart, or where an internal invariant is inconsistent. Filed as one issue because each is a few lines and they are best swept in a single pass; none is individually worth a branch.

All eight were confirmed by an independent verifier against commit 826d332b0.

---

1. TreeSearch(..., stopAtScore = X) errors when the start tree already meets X (A15-O-13)

R/CustomSearch.R:52-59 — the early-return path sets edgeList[[3]] but never edgeList[[4]]; R/CustomSearch.R:237 then reads edgeList[[4]].

Error in edgeList[[4]] : subscript out of bounds

stopAtScore is not a TreeSearch() formal, but it reaches EdgeListSearch() through ... and matches its formal there, so this is reachable from the public API. The success case — the search already having achieved the target — is the case that errors.

2. TreeSearch()/Ratchet() return stale edge.length and node.label (A15-O-14)

R/CustomSearch.R:235 and R/Ratchet.R:253 replace tree[["edge"]] wholesale and leave every other list element untouched. But R/CustomSearch.R:134 documents:

Edge lengths are not supported and will be removed.

They are not removed. After a topology change both vectors survive, silently re-associated with edges and nodes they no longer describe — worse than either honouring the doc or rejecting the input, because the numbers look valid.

3. SuccessiveApproximations() ignores tree and suboptimal, and never returns a tree set (A15-O-19)

R/SuccessiveApproximations.R:32-41 accepts tree but uses it only as a fallback when the kernel returns an empty edge matrix (:120-123) — a supplied starting tree has no effect on the search. suboptimal (:36, documented at :20-21) is never referenced anywhere in the body. The return documentation at :23-24 promises "optimal (and slightly suboptimal, if suboptimal > 0) trees", but :138-144 always wraps exactly one tree; suboptimal = 0.5 still yields length(result) == 1.

Distinct from #126, which is about the absent sectorial/fuse/pool parameters.

4. TreeSearch() computes bestScore and discards it (A15-O-21)

R/CustomSearch.R:228 computes bestScore <- attr(tree, "score") and never passes it to EdgeListSearch(), which has a bestScore formal (:37) and recomputes from scratch. The return documentation at :170-173 describes the abandoned behaviour:

the parsimony score will be inherited from the tree's attributes, which is only valid if it was generated using the same data

Either wire it up or delete both the assignment and the caveat — the current state documents a behaviour that does not happen, and the caveat warns about a risk that does not exist.

5. Three different equality epsilons across one accept chain (A15-O-22)

Location	Value
R/CustomSearch.R:40	epsilon <- 1e-07
R/tree_rearrangement.R:40	eps <- .Machine[["double.eps"]] ^ 0.5 (≈1.49e-8)
R/Ratchet.R:94	epsilon <- sqrt(.Machine[["double.eps"]]) (≈1.49e-8)

The latter two are the same value written two ways; the first is ~6.7× larger. Consequence: EdgeListSearch() accepts and adopts trees that RearrangeEdges() classified as strictly worse, by up to 1e-7 per iteration, re-anchoring bestScore to the worse value each time. The drift is bounded by maxIter × 1e-7 and is negligible in absolute terms at default settings — but it means Ratchet()'s "the returned tree is never worse than the input" invariant holds only to within an inconsistent tolerance, and the tolerance that governs is not the one either function declares.

Worth a single shared constant.

6. A no-op swap counts as a hit toward maxHits (A15-O-23)

R/tree_rearrangement.R:77-78 — whenever an EdgeSwapper returns its input unchanged, candidateScore == scoreToBeat and hits increments. No-op sources include the nEdge < 5 early returns (R/SPR.R:174-176, R/TBR.R:118-120) and every SPRWarning/TBRWarning return path.

On small trees the search can therefore terminate after maxHits iterations having performed no rearrangement at all, reporting the hit count as though it had converged. This interacts with the sampler identity-move defects filed separately — the same accounting error inflates hits there too, so fixing one without the other leaves the symptom in place.

7. Dead code (A15-O-25)

• R/TBR.R:161 and R/TBR.R:371 both assign edgesRemaining, never read.
• R/SPR.R:102-107 is unreachable after the stop() at :101 (see A15-01: the documented edgeToBreak = -1 "enumerate all one-step trees" contract is broken at 4 of 6 exported entry points, one of them silently #125, which covers the -1 decision those lines belong to — remove them as part of whichever resolution is chosen there, not independently).

8. MultiRatchet() prints unconditionally at verbosity = 0 (A15-O-26)

R/Ratchet.R:291 calls message("Found ", length(trees), " unique trees ...") outside any verbosity gate, although the function's own default is verbosity = 0L.

An identical defect in SuccessiveApproximations() — the "Stability not reached" branch was ungated while the "converged" branch was gated — was fixed inline this round; no test or vignette referenced the string. This one is left for the sweep because it sits in a file with other pending changes.

---

Verification

All eight confirmed REAL by an independent verifier reading the cited lines and reproducing the two runtime errors. Items 2 and 5 were additionally spot-checked by the orchestrator, since a batch returning 13/13 confirmations warrants a sample check: the roxygen text in item 2 is quoted verbatim above, and all three epsilon literals in item 5 were read directly (note tree_rearrangement.R names its variable eps, not epsilon, which defeats a naive grep).

Found by /red-team area 15, 2026-08-05, opus (Opus 5) — the second of two paired passes over this area.