Skip to content

A constraint is silently ignored when the caller supplies a violating start tree via tree =: the search freezes on it, reports a better-than-constrained score, evicts every compliant tree other replicates find, and warns about nothing. #18

Description

@ms609

Severity: P1 · Area: 13 (Constraint), 9 (Wagner/constraints)

Location: src/ts_driven.cpp:100-103 (result.tree = *starting_tree; with no constraint call at all); path in R/MaximizeParsimony.R:1581 -> src/ts_rcpp.cpp:1690-1739 (shape validation only) -> src/ts_driven.cpp:856-866 / src/ts_parallel.cpp:203-213

No impose_constraint, map_constraint_nodes or violates_constraint_posthoc exists on the startEdge path at any of the four layers; the known impose_constraint() sites (ts_driven.cpp:1203, :1414, ts_nni_perturb.cpp:99, ts_parallel.cpp:93) are accurate at HEAD and none covers it. Verified REAL with a deterministic repro (opus verifier, clean git archive HEAD 5ac9e280 build, .agent-rt11 deliberately not trusted). 8 taxa, 6 binary characters, unconstrained optimum 6, {a,b}-constrained optimum 8: constraint + random start -> 8, 1/1 compliant (correct); constraint + tree = (((a,e),b),((c,d),(f,(g,h)))) -> 6, 0/1 compliant, returned tree == the supplied start, at every effort level -1..3 and with nniPerturbCycles = 4 / driftCycles = 4; the same start unconstrained moves freely, proving TBR could move. Also confirmed on congreveLamsdellMatrices[[1]] with T-390's own 6-tip constraint: baseline 203, 100/100 compliant; with tree = the unconstrained 192, 0/2 compliant, 0 warnings. The finder's mechanism was wrong and the correction matters for the fix: the search is frozen, not exploring unconstrained space — the reported score is the supplied start's own score (verbosity = 3 shows Starting tree 6 -> TBR 6 -> Ratchet 6 -> TBR 6), because regraft_violates_constraint rejects every move once constraint_node[s] < 0. Blast radius is wider than one replicate: the illegal score evicts legitimate trees, so an 8-replicate run returned 6, 0/1 compliant even though replicates 2-8 each built a constrained Wagner start and reached the true constrained optimum 8. Conversely it is masked when the violating start scores worse than the constrained optimum — so it bites precisely when the user supplies a good tree, i.e. ?MaximizeParsimony's own advertised workflow of "continuing a search from previously found optima" with a multiPhylo of prior MPTs. No warning anywhere, and the package promises the opposite in two places: @param constraint ("returned trees will be perfectly compatible with each character in constraint... all tree rearrangements are filtered to respect the constraint topology") and src/ts_rcpp.cpp:1032, which tells AdditionTree() users to "use MaximizeParsimony(), whose rearrangement phase enforces the constraint". No test covers a violating startEdge under a constraint (test-ts-constraint-rooting.R uses startEdge but always with a satisfying start). Distinct ID, deliberately: this is the THIRD entry point in a class already filed twice. T-390 is sprFirst = TRUE (phase has no constraint parameter); T-391 is pruneReinsertCycles > 0 (constraint arrives and is lost inside re-insertion, "Distinct from T-390: here the constraint reaches the phase and is still lost"); T-402 is the user-supplied start, never imposed. Different entry point, different fix location, different reachability. P1 by the project's own stated rule: T-390 and T-391 are each held at "Severity P2 not P1 only because no shipped configuration reaches it" (sprFirst = FALSE, pruneReinsertCycles = 0L in every preset) — a gate T-402 lacks, needing only two documented top-level arguments used together. Cross-reference T-324 but do not conflate: T-324 is entry-point-scoped to the Wagner constructor and its severity is gated on an unconfirmed 100-reshuffle reachability question; T-402's repro does not settle it. What the two share is T-324's downstream half verbatim — the ungated pool capture (ts_driven.cpp:1066, :1074, ts_parallel.cpp:236) and the confirmed absence of any downstream filter — so they must be fixed together, with T-402's repro as the standing regression test for that shared half. Two mis-patch traps: (1) a verify-and-revert gate of the T-390/T-391 shape is not sufficientnni_perturb snapshots the violating start at src/ts_nni_perturb.cpp:80 before repair and then rejects the repaired legal (necessarily worse-scoring) tree at :120 via accept = tbr_result.best_score < best_score, so the illegal score is an unbeatable baseline; (2) gating the pool capture alone leaves the pool empty at maxReplicates = 1, and R/MaximizeParsimony.R:1682-1684 then falls back to outTrees <- list(treeTpl) where treeTpl traces to startTrees[[1]] — returning the user's violating start anyway. The fix must act at the startEdge boundary (impose-and-verify, or reject with a warning), ideally R-side where consSplitMatrix is already in hand, and must use violates_constraint_posthoc rather than a constraint_node[s] < 0 check (has_posthoc == true for every user constraint, src/ts_rcpp.cpp:1541).


Migrated 2026-08-04 from dev/red-team/findings.md; pre-tracker ID T-402. References to T-402 in source comments, dev/red-team/log.md, commit messages and PR bodies mean this issue — see dev/red-team/migration-map.tsv. Cross-repo references use the fully-qualified form agent-issues/TreeSearch#<n>; a bare #n means this repo.

Metadata

Metadata

Assignees

No one assigned

    Labels

    area:13Red-team focus area 13area:9Red-team focus area 9in-progressBeing fixed; claiming comment names the branchred-teamFiled by the /red-team rotationsev:highP1: wrong user-visible result / crash

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions