A15-04: Ratchet(stopAtScore=) returns the INPUT tree carrying the IMPROVED score — silent wrong answer, plus two more early-exit bookkeeping failures

[ms609]

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

Covers red-team candidates A15-O-01 (high), A15-O-05 (med), A15-O-06 (med). One root cause with three symptoms: Ratchet()'s early-exit paths bypass the bookkeeping that its return statement depends on.

1. Ratchet(stopAtScore =) returns the input tree carrying the improved score

This is a silent wrong answer on the documented success path of a documented parameter.

R/Ratchet.R:167-179 — when the target score is met, the loop sets bestScore <- candScore and breaks. But edgeList <- candidate lives only in the acceptance block at :190/:195, below the if (BREAK) break at :178. The return at :253-254 therefore rebuilds the tree from the stale edgeList and attaches the better score to it.

Reproduced on Lobo (Ratchet(njtree, loboData, stopAtScore = 225, ratchIter = 50, searchHits = 5), seed 1):

start TreeLength:                230
attr(result, "score"):           223     <- the improved score, attached
independent TreeLength(result):  230     <- the tree is the untouched input
RF distance to input:              0

The caller receives a tree that is not the tree whose score is reported. Nothing errors and nothing warns. Any downstream use of attr(, "score") — including MultiRatchet()'s own — propagates the wrong number.

2. Ratchet(returnAll = TRUE, stopAtScore = X) throws when it succeeds

Same cause, one line up. R/Ratchet.R:184-187 stores the candidate in forest only below the BREAK check, so on the success path the target-meeting tree is never stored at all — it is simply lost. :214 then finds keepers all-FALSE, and :242 raises:

Error: No trees!? Is suboptimal set to a sensible (positive) value?

The error message actively misdirects: suboptimal is fine, and the search succeeded. The same error fires for ratchIter = 0.

3. Ratchet()'s already-met early return breaks its own return contract and MultiRatchet()

R/Ratchet.R:120-125 — when the starting tree already meets stopAtScore, the function returns tree with no "score" attribute, and as a phylo even when returnAll = TRUE (documented to return multiPhylo).

MultiRatchet() then dies at R/Ratchet.R:289:

Error in vapply(trees, function(x) attr(x, "score"), double(1)) :
  values must be length 1, but FUN(X[[1]]) result is length 0

So the fast path — the answer was already correct — is the path that breaks the caller.

Suggested fix

All three follow from the same edit: move the edgeList <- candidate assignment and the forest append above the if (BREAK) break, or restructure so that break cannot skip the bookkeeping the return depends on. Then make the :120-125 early return construct the same shape as the normal return (score attribute attached, multiPhylo when returnAll = TRUE).

A regression test should assert the property that failed here rather than the code path: the returned tree's independently-recomputed TreeLength() must equal its own "score" attribute, on every exit path. That single assertion catches all three.

Verification

All three confirmed REAL by an independent peer-tier verifier against commit 826d332b0, reproduced with a built package rather than by inspection; symptom 1 was confirmed by re-scoring the returned tree with TreeLength() rather than trusting the reported attribute. The verifier additionally established that under returnAll = TRUE the target-meeting candidate is never added to forest — the link between symptoms 1 and 2.

Found by /red-team area 15, 2026-08-05, opus (Opus 5). Not found by a sonnet pass over the same files earlier the same day.

[ms609]

Claiming — working on branch claude/issue-136-assignment-0c7689 (worktree issue-136-assignment-0c7689).