Skip to content

A15-10: Ratchet(returnAll = TRUE) reports a wrong MPT set - omits an equal-best start tree, and counts one unrooted topology twice #155

Description

@ms609-agent

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

Covers A15-24 and A15-25. Two independent defects with one consequence: Ratchet(returnAll = TRUE) reports an MPT set that is wrong — sometimes missing a tree that belongs in it, sometimes counting one tree twice. Both are silent.

Verification upgraded A15-25 from low to med and found A15-24 worse than reported.


1. The starting tree never enters the forest

R/Ratchet.R:130-134, :184-187 — only ratchet candidates are appended to forest; the tree the user supplied is never a member.

Silent mode (the one that will actually bite). Reproduced on Lobo with an MPT as the start tree (ratchIter = 3, searchIter = 1, bootstrapIter = 500):

Found 3 unique MPTs and 0 suboptimal trees.
start-tree score:  205 ;  best returned: 205
start topology present in returned set?  FALSE

The start tree is an equal-best MPT and is simply absent from the reported set. No error, no warning. Needs no unusual settings — it follows directly from forest only ever receiving candidate at :185.

Error mode. If every iteration returns a tree worse than a good start tree, keepers at :214 is empty and :242 raises:

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

Reproduced on homoplasy-free data (20-tip pectinate tree, characters = its own splits, start tree = the unique MPT, score 17) at searchIter 1, 2 and 5, with default swappers and default suboptimal. The message misdirects: suboptimal is fine, and the start tree was optimal. Under the package's own quick-run idiom (ratchIter = 2, searchHits = 3) and under full defaults it recovers, so this mode needs a hobbled search — but it is reachable without touching any non-default argument.

2. De-duplication compares rooted edge matrices

R/Ratchet.R:228 uses unique(forest) on phylo objects, and .UniqueExceptHits() (:264) shares the blind spot. The default swappers (TBRSwap, SPRSwap, NNISwap) all move the root, so one unrooted topology at two root positions survives as two entries — and is reported as two distinct MPTs at :231/:245.

Synthetic demonstration: (t1,(t2,(t3,(t4,t5)))); and (t4,(t5,(t3,(t2,t1)))); are the same unrooted tree (RobinsonFoulds = 0) with different $edge matrices; both length(unique(list(a, b))) and length(.UniqueExceptHits(list(a, b))) return 2.

Real run (Lobo, default swappers, returnAll = TRUE):

Found 3 unique MPTs and 0 suboptimal trees.
pairwise unrooted RF:   1-2 = 0 ,  1-3 = 2 ,  2-3 = 2
tree 1: root children   1 50    (rooted on tip 1)
tree 2: root children  50 51    (rooted on an internal node)

Trees 1 and 2 are one unrooted topology at two rootings. The correct count is 2, not the 3 reported — both the returned multiPhylo and the user-facing message are inflated.

Under equal-weights parsimony the rooting is irrelevant to the score, so nothing distinguishes these as different results. An inflated MPT count is a silently wrong scientific result, not a cosmetic one — which is why this was upgraded to med.

Suggested fix

Both are small, and they interact — fixing only the dedup would still omit the start tree, and fixing only the seeding would then duplicate it against a rooting-variant candidate.

  1. Seed forest with the starting tree (and its score) before the loop, so an already-optimal input is a member of its own result set.
  2. De-duplicate on unrooted topology — a split-set or RobinsonFoulds == 0 comparison — rather than on $edge. Note the trap recorded in TBRMoves() omits every TBR move that bisects the root edge, so its output is not a superset of SPRMoves() #147: sort(as.character(as.Splits(...))) is not injective, so validate any key by confirming it yields exactly NUnrooted(n) distinct values before trusting a count derived from it.

A regression test should assert the property, not the path: an already-optimal starting tree must appear in returnAll = TRUE output, and the reported count must equal the number of distinct unrooted topologies.

Verification

Both confirmed REAL by an independent peer-tier verifier against commit eb6e26ce1, reproduced end-to-end on real data rather than by inspection. A15-24's silent mode was not in the original finding, which reported only the error; A15-25 was raised as low and is filed here at med on the verifier's evidence.

Found by /red-team area 15, 2026-08-06, fable (Fable 5).

Metadata

Metadata

Assignees

No one assigned

    Labels

    area:15Red-team focus area 15red-teamFiled by the /red-team rotationsev:medP2: wrong on edge input / search quality

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions