From 826d332b01f0c9f8e6b9a8fc4e8e939bee0a15eb Mon Sep 17 00:00:00 2001 From: R script <1695515+ms609@users.noreply.github.com> Date: Wed, 5 Aug 2026 15:18:04 +0100 Subject: [PATCH 01/22] red-team: area 15 (legacy pure-R search API), first-ever review Fixes the one defect small enough to fix inline, and records the round. `EdgeListSearch()` looped `for (iter in 1:maxIter)`. `1:0` is `c(1, 0)`, so `maxIter = 0` silently performed two rearrangement iterations instead of none -- and since `RearrangeEdges()` accepts any candidate scoring `<= scoreToBeat`, a caller asking for zero rearrangements could get a different tree back. `maxIter` is user-facing and reaches this loop from `Bootstrap()`, `Jackknife()` and `Ratchet()`, so `maxIter = 0` is a reachable "score without searching" idiom. Switched to `seq_len(maxIter)`, pre-initialising `iter <- 0L` because `iter` is read after the loop. Pinned by a regression test using mocks that error if called; `EdgeListSearch()` is exported, so it needs no dataset and no C++. The round's other findings are filed as agent-issues/TreeSearch#125 (the documented `edgeToBreak = -1` contract is broken at 4 of 6 exported entry points, one of them silently) and #126 (`SuccessiveApproximations()` runs without the sectorial/fuse/pool machinery, undocumented). Co-Authored-By: Claude Opus 5 --- R/CustomSearch.R | 5 +++-- dev/red-team/focus-areas.md | 30 ++++++++++++++++++++---------- dev/red-team/log.md | 25 ++++++++++++++++++++++++- tests/testthat/test-CustomSearch.R | 15 +++++++++++++++ 4 files changed, 62 insertions(+), 13 deletions(-) diff --git a/R/CustomSearch.R b/R/CustomSearch.R index 188f5df4b..859e91142 100644 --- a/R/CustomSearch.R +++ b/R/CustomSearch.R @@ -59,8 +59,9 @@ EdgeListSearch <- function (edgeList, dataset, } hits <- 0L unimprovedSince <- 0L - - for (iter in 1:maxIter) { + iter <- 0L + + for (iter in seq_len(maxIter)) { candidateLists <- RearrangeEdges(edgeList[[1]], edgeList[[2]], dataset = dataset, TreeScorer = TreeScorer, diff --git a/dev/red-team/focus-areas.md b/dev/red-team/focus-areas.md index 4868adbf1..1c4f10444 100644 --- a/dev/red-team/focus-areas.md +++ b/dev/red-team/focus-areas.md @@ -166,14 +166,24 @@ top of `log.md`; seams that a version bump has made re-eligible are queued in (`test-MaddisonSlatkin.R`, `test-Concordance.R`, `test-ParsSim.R`, `test-Consistency.R`, `test-ScoreSpectrum.R`, `test-QuartetResolution.R`, `test-TaxonInfluence.R`, `test-WideSample.R`, `test-pp-*.R`) is a useful first read. -- **15 Legacy pure-R search API — sonnet, UNMEASURED / no inherited maturity.** Added +- **15 Legacy pure-R search API — MEASURED 2026-08-05 at sonnet, still yielding.** Added 2026-08-05 from #42's scope-coverage diff: 2,183 lines across 9 files backing the - still-shipped pre-C++-engine search functions, owned by no area. **Higher urgency than its - size suggests:** #16 (`sev:high`) names `EdgeListScore()` as *"the default `TreeScorer` for - `TreeSearch()`/`Ratchet()`/`Jackknife()`"* and one of four confirmed-vulnerable entry points, - so this family is a second, wholly unreviewed exposure surface for an already-confirmed bug — - take that question first. #42 offered "review once as frozen legacy, then deprioritise"; - **the maintainer chose a full rotation area instead (2026-08-05): keep revisiting until the - seam stops yielding.** Legacy is not the same as clean, and this code is still shipped and - still the documented entry point for users who have not moved to the C++ engine. Treat "it - isn't growing" as a reason the seam should *exhaust* quickly, not as a reason to stop early. + still-shipped pre-C++-engine search functions, owned by no area. #42 offered "review once as + frozen legacy, then deprioritise"; **the maintainer chose a full rotation area instead + (2026-08-05): keep revisiting until the seam stops yielding.** Legacy is not the same as + clean, and this code is still shipped and still the documented entry point for users who have + not moved to the C++ engine. Treat "it isn't growing" as a reason the seam should *exhaust* + quickly, not as a reason to stop early. + **The `#16`/`EdgeListScore()` urgency that originally justified this row is now discharged** + — #16 is closed, its guard landed in PR #50, and the first round measured the pure-R layer + above it as *doubly* guarded (`R/CustomSearch.R:209` and `R/Ratchet.R:96` both reject + non-bifurcating input independently of the C++ fix; a 600-iteration fuzz of the default + rearrangement path produced no non-binary trees). Do not spend a second round re-asking it; + reopen only if the C++ guard is relaxed or a new caller bypasses those entry checks. + `start_tier` remains `sonnet` but is now inert — the seam yielded, so routing keeps the next + visit at the tier the log's experiment paragraph specifies. **Next visit starts here:** the + round's own leads — `SuccessiveWeights()` (`R/SuccessiveApproximations.R:175-189`) scores via + `CharacterLength()` rather than the kernel and was never checked for agreement with it, which + is exactly the shape of the already-confirmed #83 divergence; and the `-1` contract class of + #125, whose fix is one maintainer decision (refuse uniformly, per the existing deliberate + `stop()` at `R/SPR.R:101`) rather than four separate repairs. diff --git a/dev/red-team/log.md b/dev/red-team/log.md index 6d8901573..e9e8aa129 100644 --- a/dev/red-team/log.md +++ b/dev/red-team/log.md @@ -60,6 +60,29 @@ persistently-dry reputation leans on pre-tier rounds (areas 3 and 10 both do) ha --- +area: 15 (Legacy pure-R search API) — first-ever review +reviewed_by: sonnet finder afbe83c1d183868a3 + sonnet peer verifier abd8f225317a23c4c (3 med, R-semantics verdicts) + haiku verifier af328ab30afd622f0 (2 low, artifact-tracing verdicts) +date: 2026-08-05 +tier: sonnet (Sonnet 5) +yield: **3 confirmed findings, 0 refuted of 5 candidates.** Filed: #125 (A15-01, sev:med — groups candidates 1/2/3/5), #126 (A15-02, sev:low — candidate 4). Fixed inline, logged not filed: A15-03. **Zero sev:high.** +notes: **THIS ROUND IS THE SONNET ARM OF A DELIBERATE TIER-ECONOMICS EXPERIMENT — DO NOT DISPATCH THE OPUS ARM WITHOUT READING THIS PARAGRAPH.** The question under test: is it cheaper to let sonnet find what sonnet can find and then have opus find only the remainder, than to send opus first? Area 15 was chosen because it was the only never-visited area — nothing here had been pre-harvested, so a first pass measures the cheap tier honestly. **The opus arm must run over the identical scope with a fresh agent, with #125/#126 AND the A15-03 inline fix in the "do not re-investigate" block**, and its yield recorded here for comparison. Anything else measures a different question. Prior to this round the record was: sonnet 6 rounds / **0** sev:high ever; opus 24 rounds / 11 producing at least one. This round did not break that pattern. + +**The scope row's headline question was stale and would have burned the round.** The row still cited #16 (`TreeState::init_from_edge` OOB on multifurcating trees) as live, naming `EdgeListScore()` as a vulnerable entry point. **#16 is CLOSED/COMPLETED** — the boundary guard landed in PR #50 — so the brief reframed it before dispatch: the C++ boundary now *rejects* non-binary trees, so the live question is whether the pure-R layer above hands it inputs it will now reject. **Answer: no, and the seam is doubly guarded.** `TreeSearch()`/`Ratchet()` both already `stop("tree must be bifurcating")` on entry (`R/CustomSearch.R:209`, `R/Ratchet.R:96`), predating and independent of the C++ fix, and `EdgeListScore()` itself carries its own `tabulate(parent)` binary check (`R/tree_length.R:611-618`) that looks to have been added in response to #16's family. A 600-iteration chained fuzz of `RootedNNI`/`RootedSPR`/`RootedTBR` on the default random-move path, with full structural-sanity checks, found zero corruption — **the default heuristic-search rearrangement path does not spontaneously build non-binary trees.** Recorded as measured, not assumed: reopen if the C++ guard is ever relaxed, or if a new caller bypasses those two entry checks. + +**The actual find is a documented contract that is broken almost everywhere it is documented.** `man/NNI.Rd`/`man/TBR.Rd` promise that `edgeToBreak = -1` returns "a complete list of all trees one step from the input tree". Six exported functions inherit that promise; **one honours it.** `NNI()` works; `RootedNNI()` errors *unused arguments* (`R/NNI.R:196` — the anonymous function's body is the bare symbol `` `[[<-` ``, so `lapply`'s `...` hits a one-formal closure); `RootedSPR()`/`RootedSPRSwap()` error *object 'tree' not found* (`AllSPR()` references a `tree` that is not among its formals); `TBR()`/`RootedTBR()` have **no `-1` branch at all** and fall into the ordinary `edgeToBreak < 1` bounds check, so they warn and return the input tree **unchanged** — the only silent failure of the four, and the dangerous one. `SPR()` alone refuses deliberately, via an unconditional `stop("Negative edgeToBreak not yet supported; please request on GitHub")` at `R/SPR.R:101` that makes `:102-107` dead code. **That `stop()` is the key to the fix and reframes the finding**: this is one maintainer decision, not four bugs — refusing uniformly at the other five sites (and dropping the `-1` sentence from the docs) is a handful of lines and eliminates the silent class outright. No test anywhere passes `edgeToBreak = -1` to any of the six. + +**A15-03, fixed inline, not filed:** `EdgeListSearch()` looped `for (iter in 1:maxIter)` (`R/CustomSearch.R:62`). `1:0` is `c(1, 0)`, so `maxIter = 0` silently performed **two** rearrangement iterations instead of none — and since `RearrangeEdges()` accepts any candidate scoring `<= scoreToBeat`, a caller asking for zero rearrangements could get a *different tree back*. `maxIter` is user-facing (default 100) and propagates from `Bootstrap()`, `Jackknife()` and `Ratchet()`, so `maxIter = 0` is reachable as a plausible "score without searching" idiom. Fixed to `seq_len(maxIter)` with `iter <- 0L` pre-initialised, which is load-bearing: `iter` is read after the loop at `:111`. **Pinned by a regression test** (`test-CustomSearch.R`) using mocks that error if called — `EdgeListSearch()` is exported, so this needs no data and no C++. Verified pre/post by rebuilding the pre-fix loop bound in an isolated copy of the function: post-fix the loop body is never entered, pre-fix it is. + +**Verifier routing note worth keeping.** Candidates 1/2/3 were `sev:med`, which the severity rule alone would send to haiku — but all three turn on **R language/roxygen semantics** rather than on tracing project code, so they went peer-tier under the "route by what decides the verdict" rule. That paid: on candidate 2 the peer verifier found that **a global variable named `tree` masks the bug entirely** (lexical scoping reaches it from `AllSPR`'s namespace parent; a *local* `tree` in the caller does not), and it initially produced a false negative that way before re-running in a clean environment. A cheap verifier would plausibly have refuted a real finding. That is now the third time this project has been bitten by cheap-verifying a library/language-fact verdict. + +**Not chased, for the next reviewer:** (a) `SuccessiveWeights()` (`R/SuccessiveApproximations.R:175-189`) calls `CharacterLength()` rather than the kernel — whether its scoring agrees with the C++ kernel for the same tree and weights is untraced, and #83 already shows `TreeLength()`/`EdgeListScore()` disagreeing by default on extended IW, so this looks live; (b) `RearrangeEdges()` (`R/tree_rearrangement.R:36-94`) calls `TreeScorer()` **twice per iteration** when `scoreToBeat` is not supplied — once for the default argument, once for `candidateScore` — a perf observation only, not investigated for correctness, and arguably a `/profile` item rather than a red-team one; (c) `MultiRatchet()`/`Ratchet()`'s `swappers=` list combined with a caller passing `edgeToBreak = -1` through `...` would reach the broken paths — no such call site exists in this codebase, but the docs promise it, so a downstream user script plausibly could. + +**Re-verified, not re-filed:** #119's `R/Bootstrap.R` `sample()` length-1 trap. `deindexedChars` can have length 1 only when the entire dataset compresses to a single unique pattern of weight exactly 1 — genuinely degenerate, so #119's "unreachable with normal weights" characterisation holds. **Not re-filed:** #83, confirmed still present; its specific two-path shape does **not** extend to `SuccessiveApproximations()`, which builds `contrast`/`tip_data`/`weight` itself and calls the kernel directly, so it has only one path. + +Seam status: **still yielding** (5 candidates, 5 confirmed, 0 refuted, on a first pass). **NEXT VISIT: the opus arm of the experiment above — same scope, fresh agent, #125/#126/A15-03 in the do-not-re-investigate block.** This is not an escalation on the usual trigger (the seam did not run dry); it is a deliberate paired measurement. + +--- + area: 14 (Statistics & support-metrics cluster) — first-ever review, area enacted this round reviewed_by: opus finder a01961467a9937203 + opus verifier a13226f4cad264aea (7 high-sev/memory-safety candidates) + haiku verifier a02207b4e4057b65e (30 low/med batch) + orchestrator direct code-read (A14-11, A14-12 — omitted by the haiku batch's return) date: 2026-08-05 @@ -1376,4 +1399,4 @@ tier: n/a (directed single-finding fix) yield: 1 filed-and-fixed same session (T-366, P3) notes: Handed a pre-verified finding for `expand_and_reinsert` (`ts_prune_reinsert.cpp:396`): it scored the rebuilt backbone with `score_tree()`, which on `has_inapplicable` data falls through to `fitch_na_score` and writes NA-regime `prelim`, while the insertion loop's `wagner_incremental_rescore` (`ts_wagner.cpp:131-166`) only maintains standard-Fitch `prelim` with no NA branch — `compute_insertion_edge_sets` then reads this mixed-regime array to choose reinsertion edges. The two sibling backbone-scoring call sites (`ts_wagner.cpp:449`, `ts_sector.cpp:917`) both already use the EW-proxy `fitch_score`, so this one call site reads as an oversight. **Fix applied:** swapped to `fitch_score(tree, ds)`. **Verification performed this session:** built clean; ran an NA repro (`Vinther2008`, then `Dikow2009` for a stronger test) with `pruneReinsertCycles` forced nonzero (default is `0L`, fully inert otherwise) — confirmed the path was actually exercised via `prune_reinsert_ms` timing (0ms before forcing the params right, ~1.3s after). Direct A/B (temporarily reverted the fix, rebuilt, re-ran identical seeds): on `Dikow2009` with 6 fixed RNG seeds, 5/6 gave byte-identical final score AND topology (`write.tree` hash) before vs. after; seed 4 diverged (1614 before → 1616 after) — confirms the fix changes search trajectory on this now-live path, exactly as the finding predicted, with no crash and no corrupted score in either arm. Existing `test-ts-prune-reinsert.R` (52 tests) and `test-ts-sector.R` (52 tests) both still pass. **Not done, flagged as a separate follow-up (do not conflate with this fix):** `fitch_na_score`'s `local_cost` is only written on its non-NA branch, which independently corrupts `wagner_incremental_rescore`'s `old_cost` subtraction for NA blocks — this changes placement further and needs its own A/B before landing. **This entry was not independently re-verified by a second reviewer** (no red-team-verifier pass) — the A/B above is empirical evidence, not a peer confirmation; a future round should sanity-check the reasoning, not just re-trust this note. This was a directed fix task, not a rotation round, so `last_focus` is left untouched. -last_focus: 14 +last_focus: 15 diff --git a/tests/testthat/test-CustomSearch.R b/tests/testthat/test-CustomSearch.R index 4abcd037b..f8f352e4b 100644 --- a/tests/testthat/test-CustomSearch.R +++ b/tests/testthat/test-CustomSearch.R @@ -108,3 +108,18 @@ test_that("Profile parsimony works in tree search", { test_that("Ratchet fails gracefully", { expect_error(Ratchet(unrooted11, data11)) }) + +test_that("EdgeListSearch() performs no rearrangements when maxIter = 0", { + # `1:maxIter` evaluates to c(1, 0) when maxIter is zero, so the loop silently + # ran two rearrangement iterations; seq_len() is empty, as intended. + edge <- PectinateTree(letters[1:6])[["edge"]] + MustNotRun <- function (...) { + stop("No rearrangement should be attempted when maxIter = 0") + } + searched <- EdgeListSearch(list(edge[, 1], edge[, 2], 99), dataset = NULL, + TreeScorer = MustNotRun, + EdgeSwapper = MustNotRun, + maxIter = 0, verbosity = 0L) + expect_equal(searched[[3]], 99) # Starting score returned unchanged + expect_equal(searched[[4]], 0L) # No hits recorded +}) From eb6e26ce11b71c1ee01f8ab7a78ef584eaebcf9c Mon Sep 17 00:00:00 2001 From: R script <1695515+ms609@users.noreply.github.com> Date: Wed, 5 Aug 2026 20:03:59 +0100 Subject: [PATCH 02/22] red-team: area 15 opus pass, and the tier-economics result Second pass over the same scope with the sonnet arm's entire yield in the do-not-re-investigate block. 26 candidates, 26 confirmed, 0 refuted -- four of them sev:high, against the sonnet arm's zero. Filed as seven grouped issues: #136, #137, #138, #139 (high), #143, #144 (med), #131 (low). The paired result answers the question the two passes were run to settle: a cheap first pass did not reduce the expensive pass's work, so sonnet-first is an added pass rather than a substituted one. Recorded in log.md; the rationale in focus-areas.md now says not to re-run it. Fixes one defect inline: the "Stability not reached" branch of `SuccessiveApproximations()` emitted its message regardless of `verbosity` while the "converged" branch was gated, so a default `verbosity = 0` call was noisy. Both branches now sit inside one gate. Nothing in tests/, vignettes/, man/ or R/ referenced the string. Co-Authored-By: Claude Opus 5 --- R/SuccessiveApproximations.R | 14 ++++++++------ dev/red-team/focus-areas.md | 27 +++++++++++++++++++-------- dev/red-team/log.md | 25 ++++++++++++++++++++++++- 3 files changed, 51 insertions(+), 15 deletions(-) diff --git a/R/SuccessiveApproximations.R b/R/SuccessiveApproximations.R index 52361f9c5..bc9c18fc6 100644 --- a/R/SuccessiveApproximations.R +++ b/R/SuccessiveApproximations.R @@ -108,12 +108,14 @@ SuccessiveApproximations <- function (tree, dataset, outgroup = NULL, k = 3, c(searchArgs, .KernelConstraintArgs(consArgs), profileArgs)) - if (result$converged && verbosity > 0) { - message("Successive approximations converged after ", - result$sa_iterations, " iteration(s).") - } else if (!result$converged) { - message("Stability not reached after ", result$sa_iterations, - " iteration(s).") + if (verbosity > 0) { + if (result$converged) { + message("Successive approximations converged after ", + result$sa_iterations, " iteration(s).") + } else { + message("Stability not reached after ", result$sa_iterations, + " iteration(s).") + } } # Reconstruct phylo from C++ edge matrix diff --git a/dev/red-team/focus-areas.md b/dev/red-team/focus-areas.md index 1c4f10444..b811a50b1 100644 --- a/dev/red-team/focus-areas.md +++ b/dev/red-team/focus-areas.md @@ -166,7 +166,10 @@ top of `log.md`; seams that a version bump has made re-eligible are queued in (`test-MaddisonSlatkin.R`, `test-Concordance.R`, `test-ParsSim.R`, `test-Consistency.R`, `test-ScoreSpectrum.R`, `test-QuartetResolution.R`, `test-TaxonInfluence.R`, `test-WideSample.R`, `test-pp-*.R`) is a useful first read. -- **15 Legacy pure-R search API — MEASURED 2026-08-05 at sonnet, still yielding.** Added +- **15 Legacy pure-R search API — MEASURED 2026-08-05 at BOTH sonnet and opus; yielding + heavily. The `start_tier` column still reads `sonnet` — the maintainer's recorded choice on + #42 — but the experiment below falsifies the reasoning behind it. Left unchanged pending the + maintainer's call; it is inert either way, since routing governs a visited area.** Added 2026-08-05 from #42's scope-coverage diff: 2,183 lines across 9 files backing the still-shipped pre-C++-engine search functions, owned by no area. #42 offered "review once as frozen legacy, then deprioritise"; **the maintainer chose a full rotation area instead @@ -180,10 +183,18 @@ top of `log.md`; seams that a version bump has made re-eligible are queued in non-bifurcating input independently of the C++ fix; a 600-iteration fuzz of the default rearrangement path produced no non-binary trees). Do not spend a second round re-asking it; reopen only if the C++ guard is relaxed or a new caller bypasses those entry checks. - `start_tier` remains `sonnet` but is now inert — the seam yielded, so routing keeps the next - visit at the tier the log's experiment paragraph specifies. **Next visit starts here:** the - round's own leads — `SuccessiveWeights()` (`R/SuccessiveApproximations.R:175-189`) scores via - `CharacterLength()` rather than the kernel and was never checked for agreement with it, which - is exactly the shape of the already-confirmed #83 divergence; and the `-1` contract class of - #125, whose fix is one maintainer decision (refuse uniformly, per the existing deliberate - `stop()` at `R/SPR.R:101`) rather than four separate repairs. + **This row carried the tier-economics experiment, and it settled the tier question for good.** + Paired passes over the identical scope on 2026-08-05: `sonnet` returned 5 candidates and **0 + sev:high**; `opus`, handed sonnet's entire yield as off-limits, returned **26 candidates and 4 + sev:high**, all 26 confirmed. The cheap pass removed no work from the expensive one. Sonnet + found broken *documented contracts*; opus found *silent wrong answers* — different classes, + not different amounts. See the log's two 2026-08-05 entries; **do not re-run this experiment.** + **Next visit starts here** (stay at `opus`, fresh agent, and prefer a targeted shape over + another general finder): the **decayed custom-criterion façade** — #137 (`Ratchet()` never + forwards `TreeScorer`), #126 (`SuccessiveApproximations()`'s undocumented capability gap) and + the dead `SuccessiveWeights()` are one story, and the useful work is auditing *which advertised + custom-criterion entry points work end to end*, each with a test whose scorer is + distinguishable from `EdgeListScore` (a test using an equivalent scorer cannot see #137); + and **porting the validated neighbourhood enumerator to the C++ `all_spr`/`all_tbr` paths**, + which were never audited — the R-side harness (validated against 2(n−3) and 2(n−3)(2n−7)) + found four distinct sampler defects in one pass and should generalise. diff --git a/dev/red-team/log.md b/dev/red-team/log.md index e9e8aa129..6bdf70274 100644 --- a/dev/red-team/log.md +++ b/dev/red-team/log.md @@ -60,7 +60,30 @@ persistently-dry reputation leans on pre-tier rounds (areas 3 and 10 both do) ha --- -area: 15 (Legacy pure-R search API) — first-ever review +area: 15 (Legacy pure-R search API) — second pass, opus arm of the tier-economics experiment +reviewed_by: opus finder afdeee5c3ba129361 + opus peer verifier af6672385451a1c75 (4 high + the weight corruption) + sonnet verifier a2466e9113439999b (sampler coverage + guard ordering) + haiku verifier ab75625e2caf8410b (13 code-tracing rows) +date: 2026-08-05 +tier: opus (Opus 5) +yield: **26 candidates, 26 confirmed, 0 refuted.** Filed as 7 grouped issues: #136 (high), #137 (high), #138 (high), #139 (high), #143 (med), #144 (med), #131 (low). One further defect fixed inline, logged not filed. +notes: **THE TIER-ECONOMICS EXPERIMENT IS ANSWERED, AND THE ANSWER IS "SEND OPUS FIRST". DO NOT RE-RUN THIS.** The hypothesis under test was that it is cheaper to let sonnet find what sonnet can find and have opus find only the remainder, than to send opus first. **Refuted empirically.** This arm ran over the *identical* scope with the sonnet arm's entire yield in its do-not-re-investigate block, and still returned **26 candidates including 4 sev:high** against the sonnet arm's 5 candidates and **0 sev:high** — after which every one of the 26 survived independent verification. The cheap pass did not reduce the expensive pass's work: opus still read all 2,185 loc and found five times as much, so `C_sonnet + C_opus > C_opus` held exactly as the cost argument predicted. Finder tokens were 137k (sonnet) vs 180k (opus) — the cheap rung was not even much cheaper on this scope. **The severity split is the real result:** sonnet found broken *documented contracts*; opus found *silent wrong answers*. Those are different classes, not different amounts, which is the capability cliff made concrete. Standing conclusion: **`start_tier: sonnet` on a never-visited area is not a saving.** The genuinely cheap lever this round exposed is verifier routing (the haiku batch cost 41k against the finder's 180k and confirmed 13 rows), not finder tier. + +**The four sev:high, all silent-failure class:** (a) **#136** — `Ratchet(stopAtScore=)` returns the **input** tree carrying the **improved** score: `edgeList <- candidate` sits at `R/Ratchet.R:190`, *below* the `break` at `:178`. Measured on Lobo: returned tree RF-0 from the input, `attr(,"score")` 223, independent `TreeLength()` 230. Two more symptoms share the cause (target-meeting tree never added to `forest` → the success path errors "No trees!?"; the already-met early return omits the score attribute and kills `MultiRatchet()`). (b) **#137** — `Ratchet()` never forwards `TreeScorer` to its `Bootstrapper`, and **`...` cannot rescue it because `TreeScorer` is a named formal** (`R/Ratchet.R:82`), so it is consumed by argument matching; instrumented run showed the custom scorer called **0 times** during both bootstrap phases against 391 overall. The perturbation and search phases optimise different objectives, in the one function whose reason to exist is a custom criterion. `Jackknife()` forwards it correctly. (c) **#138** — `TreeSearch()`'s default `RootedTBRSwap` holds the root fixed; the root split is a **hard invariant** (one distinct root bipartition over 20,000 calls), so a 150k-step walk reached **45 of 945** 7-tip topologies. A mid-rooted start confines the whole search to ~4.8% of tree space, permanently. `EdgeListScore` verified root-invariant (230 across rootings), so the loss is pure. (d) **#139** — `original_weight` is filled by `as.integer()` **truncation** (`R/PrepareData.R:76`) while scoring uses `.ScaleWeight()`; uniform fractional weights floor to all-zero, every character resamples to weight 0, every tree ties at 0, and `R/CustomSearch.R:74` accepts equal scores, so `BootstrapTree()`/`JackknifeTree()` degenerate into a random walk — **with zero errors, warnings or messages**, confirmed via `withCallingHandlers`. + +**VERIFICATION CORRECTED THE FINDER THREE TIMES — DO NOT TREAT A FINDER'S NUMBERS AS THE RECORD.** (i) A15-O-03 **downgraded** high→med: it is a loud immediate error, not a silent one. (ii) **#138's mechanism is worse but its motivating example was wrong** — the finder wrote "`NJTree()`/`rtree()`/`read.tree()` all yield mid-rooted trees"; **`NJTree()` is tip-rooted** (`RootTree(tree, names(dataset)[[1]])`, measured `1|47` on Lobo), so the package's own `?TreeSearch` example is in the benign 78-81% regime and real exposure is narrower than first stated. `ape::rtree(30)` (`24|6`), `read.tree` of a balanced Newick (`4|4`) and `BalancedTree(20)` (`10|10`) are the genuinely confined cases. (iii) **Three findings came back WORSE:** the `NNISwap` self-hit rate is `2/(nTips-2)` not `1/(nTips-2)` (40% at 7 tips, not 19.8%) because a root with two internal children makes **both** root-adjacent edges self-hitting — **the fix must exclude both, which the finder's numbers would not have told a reader**; `NNI(-1)` is not merely padded with duplicates but is **missing one internal edge's entire pair of true neighbours** (6 of 8 reachable, not 8 of 8); and `TBRSwap()` on a trifurcating root does not merely misbehave, it **HANGS INDEFINITELY** (killed after 2+ min), which is why #144 is `med` not `low`. + +**Verifier methodology worth reusing, not re-deriving.** Both peer verifiers built independent unrooted-neighbourhood enumerators (bisect every edge, suppress both cut nodes, reconnect at every edge pair, canonicalise by split set) and **validated them against the closed forms 2(n-3) for NNI and 2(n-3)(2n-7) for SPR before using them as ground truth** — *n* = 5-10, several tree shapes. That validation is what let them contradict the finder's numbers with confidence rather than deferring to them. One recorded a canonicalisation trap for whoever rebuilds it: **tied n/2|n/2 splits need lexicographic, not size-based, tie-breaking.** This harness would generalise to the C++ `all_spr`/`all_tbr` enumerators, which were **not** audited this round. + +**Fixed inline, not filed:** `R/SuccessiveApproximations.R:111-119` — the "Stability not reached" branch emitted its message unconditionally while the "converged" branch was gated on `verbosity`, so the default `verbosity = 0` call was noisy. Both branches now sit inside one gate. Nothing in `tests/`, `vignettes/`, `man/` or `R/` referenced the string. **Note an identical ungated `message()` survives at `R/Ratchet.R:291` in `MultiRatchet()`** — left for #131's sweep because that file has other pending changes. + +**Ruled out, do not redo:** `morphy-deprecated.R` is **clean** — all eight shims delegate to native replacements, none reaches a removed MorphyLib path, none loads a DLL, `UnloadMorphy()` correctly returns `invisible(0L)`, and `.GapHandler()`'s `pmatch` still accepts the legacy `"inapp"` abbreviation. Only cosmetic residue: `MorphyLength()`'s `nTaxa` and `PhyDat2Morphy()`'s `gap` are accepted and not forwarded, contradicting the `@param ... Passed to the replacement function` roxygen. Also ruled out: the `1:n` idiom fixed in the sonnet arm survives **nowhere else in scope** (grep clean); `RearrangeEdges()`'s double `TreeScorer()` call is **correctness-safe for deterministic scorers** (`parent`/`child` are not mutated between the two evaluations) and is a hazard only for a stochastic custom scorer, because the swapper's RNG draws are interleaved — a documentation sentence, not a fix; `Ratchet()`'s accept criterion **is** monotone on the normal exit path (`R/Ratchet.R:188-196`), the failures being confined to the `stopAtScore` short-circuit and the epsilon mismatch; `.NonDuplicateRoot()` (`R/SPR.R:42-54`) **is correct** including its asymmetric branching — do not "simplify" it; and `TBRSwap()` is the only sampler here with **full** neighbourhood coverage (64/64, 106/106). + +**Not chased, for the next reviewer:** `TBRSwap()`'s residual identity rate (1.7-2.65% under `BalancedTree`'s own rooting) is measured but its mechanism is unidentified — it is *not* the `brokenRootDaughters` bug and *not* a warning path; whether `SPRSwap()` can reach `nCandidates == 0` and die on `parent[[integer(0)]]` (`R/SPR.R:231-248`, guarded only by a commented-out assertion the author clearly worried about); `PrepareDataSA()` (`R/SuccessiveApproximations.R:191-216`) is reachable only through the broken `SuccessiveWeights()` and so is entirely untested — its taxon-major fill was read and looks correct, but nothing has exercised it; and `NNISwap()`/`RootedNNISwap()` derive `rootNode <- nTips + 1L` rather than reading it from the edge list (`R/NNI.R:117`, `:214`) — holds for every current caller, would silently mis-classify a hand-built `phylo`. + +Seam status: **still yielding, abundantly — nowhere near dry.** Next visit stays **opus** with a fresh agent. Highest-value next work-shape is **not** another general finder: the finder itself proposed, and the evidence supports, a targeted audit of *which advertised custom-criterion entry points actually work end to end* (#137 + #126 + the dead `SuccessiveWeights()` are one decayed-façade story), plus porting the validated neighbourhood enumerator to the C++ `all_spr`/`all_tbr` paths. + +--- + +area: 15 (Legacy pure-R search API) — first-ever review, sonnet arm of the tier-economics experiment reviewed_by: sonnet finder afbe83c1d183868a3 + sonnet peer verifier abd8f225317a23c4c (3 med, R-semantics verdicts) + haiku verifier af328ab30afd622f0 (2 low, artifact-tracing verdicts) date: 2026-08-05 tier: sonnet (Sonnet 5) From 7ec9357b623bc3bd5770f726babd1eeeed0c143e Mon Sep 17 00:00:00 2001 From: ms609-agent <313734811+ms609-agent@users.noreply.github.com> Date: Thu, 6 Aug 2026 08:06:55 +0100 Subject: [PATCH 03/22] chore(agents): post as ms609-agent, not as the maintainer GitHub will not let an account approve its own PR, so a PR opened as ms609 cannot be reviewed by ms609. Attributing agent work to the ms609-agent machine account is what makes the review step possible at all. Records the two mechanisms, both applied per command so the human's global git config and `gh auth` keep working interactively: `git -c user.name/-c user.email` for commit attribution, and a `GH_TOKEN=$CLAUDE_GH_TOKEN` prefix for `gh`, since a PR's author is whichever account authenticated the call and there is no per-PR override. Pushes and the GHA dispatch scripts deliberately stay on the human's credentials -- push identity does not affect PR authorship, and dispatching a workflow needs an actions:write scope the agent token need not carry. Notes that an org billing per seat cannot absorb a machine account this way, so StratoBayes needs a GitHub App or no review step. Co-Authored-By: Claude Opus 5 --- AGENTS.md | 48 +++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 47 insertions(+), 1 deletion(-) diff --git a/AGENTS.md b/AGENTS.md index 93c7038dd..13210c232 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -39,6 +39,52 @@ means this repo and upstream numbers separately. Pre-tracker `T-nnn` ids are **f retired**: they persist in shipped source comments and in `dev/red-team/log.md`, and `dev/red-team/migration-map*.tsv` resolve them. +### Agent identity — commit and post as `ms609-agent`, never as the maintainer + +Agent work is attributed to the machine account **`ms609-agent`**, a collaborator on +`agent-issues`. This is not cosmetic: GitHub will not let an account approve its own PR, so a +PR opened as `ms609` **cannot be reviewed by `ms609`**. Posting as the machine account is what +makes review possible at all. + +Two different mechanisms, both applied **per command** — never change the global git config or +`gh auth`, which belong to the human and must keep working interactively. + +**Commits.** Author/committer is free text in the commit object; GitHub links it to an account +by matching a verified email. Pass it inline: + +```bash +git -c user.name="ms609-agent" -c user.email="313734811+ms609-agent@users.noreply.github.com" commit -F - <<'MSG' +... +MSG +``` + +**Issues, PRs, comments, labels.** A PR's author is *whichever account authenticated the API +call* — there is no per-PR override, so this is the only way to change it. `gh` reads +`GH_TOKEN` in preference to stored auth, so prefix the single call: + +```bash +GH_TOKEN=$CLAUDE_GH_TOKEN gh pr create --base cpp-search --head feature/ --title "..." --body-file +``` + +The same prefix goes on every `gh issue create`, `gh issue comment` and `gh issue edit`. Do +**not** export `GH_TOKEN` for the session — that would silently re-identify anything the human +runs in the same shell. + +**Pushes stay as the human's credentials.** Push identity has no bearing on PR authorship, so +`git push` needs no change. + +**`gha-dispatch.sh` / `gha-poll.sh` stay as the human too** — dispatching a workflow needs an +`actions: write` scope the agent token is not required to carry. Run them unprefixed. + +If `$CLAUDE_GH_TOKEN` is unset, **stop and say so** rather than silently falling back to the +maintainer's identity — a PR that lands under the wrong account cannot be re-attributed +afterwards, only closed and reopened. + +> **Other projects need their own answer.** This works because `agent-issues` is free to add +> collaborators. An org billing a paid seat per member (e.g. StratoBayes) cannot absorb a +> machine account this way; there the options are a GitHub App installation, which is not a +> billable member, or accepting single-identity working without the review step. + ### GHA dispatch (primary validation path) Checks run in the **fork's** Actions. Workflows are disabled on a new fork until enabled @@ -195,7 +241,7 @@ agent-issues/TreeSearch 5. On GHA success, open a PR — `Fixes #N` per issue, and `--base cpp-search` so the closing actually fires: ```bash - gh pr create --base cpp-search --head feature/ --title "" --body "Fixes #N ..." + GH_TOKEN=$CLAUDE_GH_TOKEN gh pr create --base cpp-search --head feature/ --title "" --body "Fixes #N ..." ``` 6. Human reviews and merges. The merge closes the issues; nothing to update by hand. 7. After merge, clean up: From e060c30572d8a1aec462bb46b5e4be4b88907839 Mon Sep 17 00:00:00 2001 From: ms609-agent <313734811+ms609-agent@users.noreply.github.com> Date: Thu, 6 Aug 2026 09:13:40 +0100 Subject: [PATCH 04/22] red-team: move area 15's round records to Discussions The round record has no business waiting on a code review. This file sits on a protected branch, and four completed rounds with 56 filed findings once sat stranded on an unmerged PR while `last_focus:` here still named a stale area -- so the next dispatch would have re-swept an area already reviewed twice that day. Area 15's three rounds now live as one Discussion post each, under that area's category. Removes the two entries added earlier on this branch and replaces them with a pointer. `last_focus:` stays live and stays current: the new scheme picks the stalest category rather than following a pointer, but that ordering cannot be computed until every area has a discussion, and the backfill has to post oldest-first so createdAt reproduces true staleness. Only area 15 is migrated so far. The historical entries below the pointer stay put -- eleven in-repo files and the /red-team skill cite this path, and the T-nnn ids are frozen. Co-Authored-By: Claude Opus 5 --- dev/red-team/log.md | 71 ++++++++++++++++++--------------------------- 1 file changed, 28 insertions(+), 43 deletions(-) diff --git a/dev/red-team/log.md b/dev/red-team/log.md index 6bdf70274..6459e0c6d 100644 --- a/dev/red-team/log.md +++ b/dev/red-team/log.md @@ -60,49 +60,34 @@ persistently-dry reputation leans on pre-tier rounds (areas 3 and 10 both do) ha --- -area: 15 (Legacy pure-R search API) — second pass, opus arm of the tier-economics experiment -reviewed_by: opus finder afdeee5c3ba129361 + opus peer verifier af6672385451a1c75 (4 high + the weight corruption) + sonnet verifier a2466e9113439999b (sampler coverage + guard ordering) + haiku verifier ab75625e2caf8410b (13 code-tracing rows) -date: 2026-08-05 -tier: opus (Opus 5) -yield: **26 candidates, 26 confirmed, 0 refuted.** Filed as 7 grouped issues: #136 (high), #137 (high), #138 (high), #139 (high), #143 (med), #144 (med), #131 (low). One further defect fixed inline, logged not filed. -notes: **THE TIER-ECONOMICS EXPERIMENT IS ANSWERED, AND THE ANSWER IS "SEND OPUS FIRST". DO NOT RE-RUN THIS.** The hypothesis under test was that it is cheaper to let sonnet find what sonnet can find and have opus find only the remainder, than to send opus first. **Refuted empirically.** This arm ran over the *identical* scope with the sonnet arm's entire yield in its do-not-re-investigate block, and still returned **26 candidates including 4 sev:high** against the sonnet arm's 5 candidates and **0 sev:high** — after which every one of the 26 survived independent verification. The cheap pass did not reduce the expensive pass's work: opus still read all 2,185 loc and found five times as much, so `C_sonnet + C_opus > C_opus` held exactly as the cost argument predicted. Finder tokens were 137k (sonnet) vs 180k (opus) — the cheap rung was not even much cheaper on this scope. **The severity split is the real result:** sonnet found broken *documented contracts*; opus found *silent wrong answers*. Those are different classes, not different amounts, which is the capability cliff made concrete. Standing conclusion: **`start_tier: sonnet` on a never-visited area is not a saving.** The genuinely cheap lever this round exposed is verifier routing (the haiku batch cost 41k against the finder's 180k and confirmed 13 rows), not finder tier. - -**The four sev:high, all silent-failure class:** (a) **#136** — `Ratchet(stopAtScore=)` returns the **input** tree carrying the **improved** score: `edgeList <- candidate` sits at `R/Ratchet.R:190`, *below* the `break` at `:178`. Measured on Lobo: returned tree RF-0 from the input, `attr(,"score")` 223, independent `TreeLength()` 230. Two more symptoms share the cause (target-meeting tree never added to `forest` → the success path errors "No trees!?"; the already-met early return omits the score attribute and kills `MultiRatchet()`). (b) **#137** — `Ratchet()` never forwards `TreeScorer` to its `Bootstrapper`, and **`...` cannot rescue it because `TreeScorer` is a named formal** (`R/Ratchet.R:82`), so it is consumed by argument matching; instrumented run showed the custom scorer called **0 times** during both bootstrap phases against 391 overall. The perturbation and search phases optimise different objectives, in the one function whose reason to exist is a custom criterion. `Jackknife()` forwards it correctly. (c) **#138** — `TreeSearch()`'s default `RootedTBRSwap` holds the root fixed; the root split is a **hard invariant** (one distinct root bipartition over 20,000 calls), so a 150k-step walk reached **45 of 945** 7-tip topologies. A mid-rooted start confines the whole search to ~4.8% of tree space, permanently. `EdgeListScore` verified root-invariant (230 across rootings), so the loss is pure. (d) **#139** — `original_weight` is filled by `as.integer()` **truncation** (`R/PrepareData.R:76`) while scoring uses `.ScaleWeight()`; uniform fractional weights floor to all-zero, every character resamples to weight 0, every tree ties at 0, and `R/CustomSearch.R:74` accepts equal scores, so `BootstrapTree()`/`JackknifeTree()` degenerate into a random walk — **with zero errors, warnings or messages**, confirmed via `withCallingHandlers`. - -**VERIFICATION CORRECTED THE FINDER THREE TIMES — DO NOT TREAT A FINDER'S NUMBERS AS THE RECORD.** (i) A15-O-03 **downgraded** high→med: it is a loud immediate error, not a silent one. (ii) **#138's mechanism is worse but its motivating example was wrong** — the finder wrote "`NJTree()`/`rtree()`/`read.tree()` all yield mid-rooted trees"; **`NJTree()` is tip-rooted** (`RootTree(tree, names(dataset)[[1]])`, measured `1|47` on Lobo), so the package's own `?TreeSearch` example is in the benign 78-81% regime and real exposure is narrower than first stated. `ape::rtree(30)` (`24|6`), `read.tree` of a balanced Newick (`4|4`) and `BalancedTree(20)` (`10|10`) are the genuinely confined cases. (iii) **Three findings came back WORSE:** the `NNISwap` self-hit rate is `2/(nTips-2)` not `1/(nTips-2)` (40% at 7 tips, not 19.8%) because a root with two internal children makes **both** root-adjacent edges self-hitting — **the fix must exclude both, which the finder's numbers would not have told a reader**; `NNI(-1)` is not merely padded with duplicates but is **missing one internal edge's entire pair of true neighbours** (6 of 8 reachable, not 8 of 8); and `TBRSwap()` on a trifurcating root does not merely misbehave, it **HANGS INDEFINITELY** (killed after 2+ min), which is why #144 is `med` not `low`. - -**Verifier methodology worth reusing, not re-deriving.** Both peer verifiers built independent unrooted-neighbourhood enumerators (bisect every edge, suppress both cut nodes, reconnect at every edge pair, canonicalise by split set) and **validated them against the closed forms 2(n-3) for NNI and 2(n-3)(2n-7) for SPR before using them as ground truth** — *n* = 5-10, several tree shapes. That validation is what let them contradict the finder's numbers with confidence rather than deferring to them. One recorded a canonicalisation trap for whoever rebuilds it: **tied n/2|n/2 splits need lexicographic, not size-based, tie-breaking.** This harness would generalise to the C++ `all_spr`/`all_tbr` enumerators, which were **not** audited this round. - -**Fixed inline, not filed:** `R/SuccessiveApproximations.R:111-119` — the "Stability not reached" branch emitted its message unconditionally while the "converged" branch was gated on `verbosity`, so the default `verbosity = 0` call was noisy. Both branches now sit inside one gate. Nothing in `tests/`, `vignettes/`, `man/` or `R/` referenced the string. **Note an identical ungated `message()` survives at `R/Ratchet.R:291` in `MultiRatchet()`** — left for #131's sweep because that file has other pending changes. - -**Ruled out, do not redo:** `morphy-deprecated.R` is **clean** — all eight shims delegate to native replacements, none reaches a removed MorphyLib path, none loads a DLL, `UnloadMorphy()` correctly returns `invisible(0L)`, and `.GapHandler()`'s `pmatch` still accepts the legacy `"inapp"` abbreviation. Only cosmetic residue: `MorphyLength()`'s `nTaxa` and `PhyDat2Morphy()`'s `gap` are accepted and not forwarded, contradicting the `@param ... Passed to the replacement function` roxygen. Also ruled out: the `1:n` idiom fixed in the sonnet arm survives **nowhere else in scope** (grep clean); `RearrangeEdges()`'s double `TreeScorer()` call is **correctness-safe for deterministic scorers** (`parent`/`child` are not mutated between the two evaluations) and is a hazard only for a stochastic custom scorer, because the swapper's RNG draws are interleaved — a documentation sentence, not a fix; `Ratchet()`'s accept criterion **is** monotone on the normal exit path (`R/Ratchet.R:188-196`), the failures being confined to the `stopAtScore` short-circuit and the epsilon mismatch; `.NonDuplicateRoot()` (`R/SPR.R:42-54`) **is correct** including its asymmetric branching — do not "simplify" it; and `TBRSwap()` is the only sampler here with **full** neighbourhood coverage (64/64, 106/106). - -**Not chased, for the next reviewer:** `TBRSwap()`'s residual identity rate (1.7-2.65% under `BalancedTree`'s own rooting) is measured but its mechanism is unidentified — it is *not* the `brokenRootDaughters` bug and *not* a warning path; whether `SPRSwap()` can reach `nCandidates == 0` and die on `parent[[integer(0)]]` (`R/SPR.R:231-248`, guarded only by a commented-out assertion the author clearly worried about); `PrepareDataSA()` (`R/SuccessiveApproximations.R:191-216`) is reachable only through the broken `SuccessiveWeights()` and so is entirely untested — its taxon-major fill was read and looks correct, but nothing has exercised it; and `NNISwap()`/`RootedNNISwap()` derive `rootNode <- nTips + 1L` rather than reading it from the edge list (`R/NNI.R:117`, `:214`) — holds for every current caller, would silently mis-classify a hand-built `phylo`. - -Seam status: **still yielding, abundantly — nowhere near dry.** Next visit stays **opus** with a fresh agent. Highest-value next work-shape is **not** another general finder: the finder itself proposed, and the evidence supports, a targeted audit of *which advertised custom-criterion entry points actually work end to end* (#137 + #126 + the dead `SuccessiveWeights()` are one decayed-façade story), plus porting the validated neighbourhood enumerator to the C++ `all_spr`/`all_tbr` paths. - ---- - -area: 15 (Legacy pure-R search API) — first-ever review, sonnet arm of the tier-economics experiment -reviewed_by: sonnet finder afbe83c1d183868a3 + sonnet peer verifier abd8f225317a23c4c (3 med, R-semantics verdicts) + haiku verifier af328ab30afd622f0 (2 low, artifact-tracing verdicts) -date: 2026-08-05 -tier: sonnet (Sonnet 5) -yield: **3 confirmed findings, 0 refuted of 5 candidates.** Filed: #125 (A15-01, sev:med — groups candidates 1/2/3/5), #126 (A15-02, sev:low — candidate 4). Fixed inline, logged not filed: A15-03. **Zero sev:high.** -notes: **THIS ROUND IS THE SONNET ARM OF A DELIBERATE TIER-ECONOMICS EXPERIMENT — DO NOT DISPATCH THE OPUS ARM WITHOUT READING THIS PARAGRAPH.** The question under test: is it cheaper to let sonnet find what sonnet can find and then have opus find only the remainder, than to send opus first? Area 15 was chosen because it was the only never-visited area — nothing here had been pre-harvested, so a first pass measures the cheap tier honestly. **The opus arm must run over the identical scope with a fresh agent, with #125/#126 AND the A15-03 inline fix in the "do not re-investigate" block**, and its yield recorded here for comparison. Anything else measures a different question. Prior to this round the record was: sonnet 6 rounds / **0** sev:high ever; opus 24 rounds / 11 producing at least one. This round did not break that pattern. - -**The scope row's headline question was stale and would have burned the round.** The row still cited #16 (`TreeState::init_from_edge` OOB on multifurcating trees) as live, naming `EdgeListScore()` as a vulnerable entry point. **#16 is CLOSED/COMPLETED** — the boundary guard landed in PR #50 — so the brief reframed it before dispatch: the C++ boundary now *rejects* non-binary trees, so the live question is whether the pure-R layer above hands it inputs it will now reject. **Answer: no, and the seam is doubly guarded.** `TreeSearch()`/`Ratchet()` both already `stop("tree must be bifurcating")` on entry (`R/CustomSearch.R:209`, `R/Ratchet.R:96`), predating and independent of the C++ fix, and `EdgeListScore()` itself carries its own `tabulate(parent)` binary check (`R/tree_length.R:611-618`) that looks to have been added in response to #16's family. A 600-iteration chained fuzz of `RootedNNI`/`RootedSPR`/`RootedTBR` on the default random-move path, with full structural-sanity checks, found zero corruption — **the default heuristic-search rearrangement path does not spontaneously build non-binary trees.** Recorded as measured, not assumed: reopen if the C++ guard is ever relaxed, or if a new caller bypasses those two entry checks. - -**The actual find is a documented contract that is broken almost everywhere it is documented.** `man/NNI.Rd`/`man/TBR.Rd` promise that `edgeToBreak = -1` returns "a complete list of all trees one step from the input tree". Six exported functions inherit that promise; **one honours it.** `NNI()` works; `RootedNNI()` errors *unused arguments* (`R/NNI.R:196` — the anonymous function's body is the bare symbol `` `[[<-` ``, so `lapply`'s `...` hits a one-formal closure); `RootedSPR()`/`RootedSPRSwap()` error *object 'tree' not found* (`AllSPR()` references a `tree` that is not among its formals); `TBR()`/`RootedTBR()` have **no `-1` branch at all** and fall into the ordinary `edgeToBreak < 1` bounds check, so they warn and return the input tree **unchanged** — the only silent failure of the four, and the dangerous one. `SPR()` alone refuses deliberately, via an unconditional `stop("Negative edgeToBreak not yet supported; please request on GitHub")` at `R/SPR.R:101` that makes `:102-107` dead code. **That `stop()` is the key to the fix and reframes the finding**: this is one maintainer decision, not four bugs — refusing uniformly at the other five sites (and dropping the `-1` sentence from the docs) is a handful of lines and eliminates the silent class outright. No test anywhere passes `edgeToBreak = -1` to any of the six. - -**A15-03, fixed inline, not filed:** `EdgeListSearch()` looped `for (iter in 1:maxIter)` (`R/CustomSearch.R:62`). `1:0` is `c(1, 0)`, so `maxIter = 0` silently performed **two** rearrangement iterations instead of none — and since `RearrangeEdges()` accepts any candidate scoring `<= scoreToBeat`, a caller asking for zero rearrangements could get a *different tree back*. `maxIter` is user-facing (default 100) and propagates from `Bootstrap()`, `Jackknife()` and `Ratchet()`, so `maxIter = 0` is reachable as a plausible "score without searching" idiom. Fixed to `seq_len(maxIter)` with `iter <- 0L` pre-initialised, which is load-bearing: `iter` is read after the loop at `:111`. **Pinned by a regression test** (`test-CustomSearch.R`) using mocks that error if called — `EdgeListSearch()` is exported, so this needs no data and no C++. Verified pre/post by rebuilding the pre-fix loop bound in an isolated copy of the function: post-fix the loop body is never entered, pre-fix it is. - -**Verifier routing note worth keeping.** Candidates 1/2/3 were `sev:med`, which the severity rule alone would send to haiku — but all three turn on **R language/roxygen semantics** rather than on tracing project code, so they went peer-tier under the "route by what decides the verdict" rule. That paid: on candidate 2 the peer verifier found that **a global variable named `tree` masks the bug entirely** (lexical scoping reaches it from `AllSPR`'s namespace parent; a *local* `tree` in the caller does not), and it initially produced a false negative that way before re-running in a clean environment. A cheap verifier would plausibly have refuted a real finding. That is now the third time this project has been bitten by cheap-verifying a library/language-fact verdict. - -**Not chased, for the next reviewer:** (a) `SuccessiveWeights()` (`R/SuccessiveApproximations.R:175-189`) calls `CharacterLength()` rather than the kernel — whether its scoring agrees with the C++ kernel for the same tree and weights is untraced, and #83 already shows `TreeLength()`/`EdgeListScore()` disagreeing by default on extended IW, so this looks live; (b) `RearrangeEdges()` (`R/tree_rearrangement.R:36-94`) calls `TreeScorer()` **twice per iteration** when `scoreToBeat` is not supplied — once for the default argument, once for `candidateScore` — a perf observation only, not investigated for correctness, and arguably a `/profile` item rather than a red-team one; (c) `MultiRatchet()`/`Ratchet()`'s `swappers=` list combined with a caller passing `edgeToBreak = -1` through `...` would reach the broken paths — no such call site exists in this codebase, but the docs promise it, so a downstream user script plausibly could. - -**Re-verified, not re-filed:** #119's `R/Bootstrap.R` `sample()` length-1 trap. `deindexedChars` can have length 1 only when the entire dataset compresses to a single unique pattern of weight exactly 1 — genuinely degenerate, so #119's "unreachable with normal weights" characterisation holds. **Not re-filed:** #83, confirmed still present; its specific two-path shape does **not** extend to `SuccessiveApproximations()`, which builds `contrast`/`tip_data`/`weight` itself and calls the kernel directly, so it has only one path. - -Seam status: **still yielding** (5 candidates, 5 confirmed, 0 refuted, on a first pass). **NEXT VISIT: the opus arm of the experiment above — same scope, fresh agent, #125/#126/A15-03 in the do-not-re-investigate block.** This is not an escalation on the usual trigger (the seam did not run dry); it is a deliberate paired measurement. +## ⚠ Round records have moved to GitHub Discussions — this file is closed to new entries + +**Area 15's three rounds of 2026-08-05/06 are deliberately NOT below.** They are the first +records written under the new scheme, and they live in the repository's Discussions, one post +per round under that area's category: + + + +Why the move: this file sits on a protected branch, so a finished round's record was hostage to +a code review it has nothing to do with. Four completed rounds and 56 filed findings once sat +stranded on an unmerged PR while this file still named a stale `last_focus:` — so the next +dispatch would have re-swept an area that had already been reviewed twice that day. Discussions +decouple the record from the merge. + +**Do not add new round entries here.** Post to the area's Discussions category instead. Title +format `RT - area - () - yield `; first line of the body +` () | effort: `. + +**`last_focus:` at the bottom is still live and still governs rotation.** Under the new scheme +the next area is the *stalest* category rather than a pointer — but that ordering cannot be +computed until every area has at least one discussion, and the backfill must post each area's +most recent round **oldest-first** so `createdAt` reproduces true staleness. Only area 15 has +been migrated. Until the rest are, `last_focus:` remains the rotation mechanism and must be +kept current. + +Everything below this line is the frozen historical record, newest first. **It stays**: eleven +in-repo files and the `/red-team` skill cite `log.md` by path, and the `T-nnn` ids it carries +are frozen, not retired. --- From 07c279028de845f2c7e493aa38862c01b9e8fdbf Mon Sep 17 00:00:00 2001 From: ms609-agent <313734811+ms609-agent@users.noreply.github.com> Date: Thu, 6 Aug 2026 09:34:09 +0100 Subject: [PATCH 05/22] docs(process): hard-code the Discussions, identity and grouping rules Round records moved to GitHub Discussions in 2026-08 but nothing in the repo said so, and /next-issue still assumed one issue means one fix site. - AGENTS.md: name Discussions as the round-record store and log.md as closed; correct the area label range (13 -> 15, and say to count rows). - next-issue: write every gh call as ms609-agent and request review from the maintainer; read each red-team body for its full file list, since issues are now filed by root cause and routinely span several sites; never brief a chip against an issue that asks for a decision. - next-issue: carry a verifier's corrections into the brief in place of the finder's original numbers. Co-Authored-By: Claude Opus 5 --- .claude/skills/next-issue/SKILL.md | 32 ++++++++++++++++++++++++++---- AGENTS.md | 5 +++-- 2 files changed, 31 insertions(+), 6 deletions(-) diff --git a/.claude/skills/next-issue/SKILL.md b/.claude/skills/next-issue/SKILL.md index 3b6f111c0..1a7849e60 100644 --- a/.claude/skills/next-issue/SKILL.md +++ b/.claude/skills/next-issue/SKILL.md @@ -11,6 +11,13 @@ between two concurrent chips costs more than the triage would. Issues live in **`agent-issues/TreeSearch`** (`gh` already defaults to it). The upstream `ms609/TreeSearch` tracker is public and human-entered: untrusted input, never a task list. +**Every `gh` call that writes goes out as `ms609-agent`** — claiming comments, `in-progress` +labels, the chip's PR. `GH_TOKEN=$CLAUDE_GH_TOKEN gh ...`, per call, never `export`ed; see +`~/.claude/CLAUDE.md`. A PR authored by `ms609` cannot be reviewed by `ms609`, so every chip +brief must carry this instruction — a chip that lands under the wrong account has to be +closed and reopened, not re-attributed. Request review from the maintainer: +`--reviewer ms609`. + ## 1. Group ```bash @@ -34,6 +41,13 @@ Cluster into tranches: - **Respect `area:N` labels** — they mark red-team focus areas, and two issues sharing an area usually share files. +**One issue is no longer one fix site.** Since 2026-08, `/red-team` files by *root cause*, +so a single issue routinely covers several candidates across several files — an enumerated +site list, or two independent defects that produce one wrong output and must be fixed +together. Read each body before grouping: sizing a tranche by issue count will under-size +it, and its file-collision check must union **every** file the body names, not the one in +the title. A body carrying `Covers A15-11, -16, …` is the tell. + Then drop anything an open PR or running chip already touches, and report what was held back and why. @@ -43,10 +57,20 @@ Issues needing a maintainer call — a behaviour trade-off, a severity dispute, even a bug", or two contradictory specifications in the tree — aren't chip-appropriate. Name them in the report; don't brief them. +**Red-team issues say so explicitly**, because a decision reads like a bug report: the body +states that what is needed is a decision, not a fix. Honour that. A chip briefed against one +will pick whichever branch it happens to prefer and present it as the fix — the expensive +failure, since the resulting PR looks reviewable and quietly settles a question the +maintainer meant to answer. Also exclude anything whose issue records only a *reopening +condition* rather than live work. + ## 3. Brief (one per cleared tranche, fully self-contained) -- **Issues verbatim**: number, title, `file:line`, mechanism. Include the pre-tracker - `T-nnn` where one exists — it is what source comments and `dev/red-team/log.md` cite. +- **Issues verbatim**: number, title, **every** `file:line` the body names, and mechanism. + Include the pre-tracker `T-nnn` where one exists — it is what source comments and the + frozen `dev/red-team/log.md` cite. Where the issue records a verifier *correcting* the + finder's mechanism, numbers or severity, carry the correction and drop the original: a + chip fixing to the finder's figure has twice left half a bug live. - **Minimal-diff fix**, and point the chip at `AGENTS.md`'s build and worktree non-negotiables rather than restating them here — they change there, not here. - **A regression test per issue, confirmed to fail pre-fix.** Assert only what the code @@ -69,9 +93,9 @@ Name them in the report; don't brief them. chip dedupes the three lists and judges plausibility itself. - **PR body**: `Fixes #N` per issue. **This only closes the issue on merge into `cpp-search`**, the fork's default branch — target anything else and the issue stays - open silently. + open silently. Open it as `ms609-agent` with `--reviewer ms609`. - **Claim each issue** before starting: add the `in-progress` label and a comment naming - the branch, so a parallel chip can see it is taken. + the branch (as `ms609-agent`), so a parallel chip can see it is taken. - **Last step**: `mcp__ccd_session_mgmt__archive_session` with `session_id: "self"`. - Branch from `cpp-search` unless a genuine code dependency forces a stack. - Comments per `AGENTS.md`'s conventions and the `r-conventions` rubric — a comment only diff --git a/AGENTS.md b/AGENTS.md index 2e894f7e4..a71cfde62 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -20,7 +20,7 @@ collaborators. |-------|---------| | `red-team` | Filed by `/red-team`. Also that skill's mode switch — don't delete it | | `sev:high` / `sev:med` / `sev:low` | Former P1 / P2 / P3 | -| `area:1`…`area:13` | Red-team focus area, per `dev/red-team/focus-areas.md` | +| `area:1`…`area:15` | Red-team focus area, per `dev/red-team/focus-areas.md` — **count the rows, this range grows** | | `task` | Planned work migrated from the retired `to-do.md` | | `deferred` | Assessed and parked; not scheduled | | `chore` | Infrastructure / process work | @@ -246,7 +246,8 @@ These recur; they are activities, not issues, and have no tracker entry: | File | Purpose | |------|---------| | **GitHub issues** (`agent-issues/TreeSearch`) | The task queue and the findings tracker | -| `dev/red-team/` | Rotation state: `focus-areas.md`, `log.md`, frozen `findings-archive.md`, `migration-map*.tsv` | +| **GitHub Discussions**, one category per `area:N` | Red-team round records. `dev/red-team/log.md` is **closed to new entries** — it keeps only the model-version legend, `last_focus:`, and the frozen pre-2026-08 history | +| `dev/red-team/` | Scope and tiers: `focus-areas.md`; frozen `log.md`, `findings-archive.md`, `migration-map*.tsv` | | `dev/strategy.md` | Historical strategic narrative (was `coordination.md`; **not** kept current) | | `completed-tasks.md` | **Frozen.** Pre-tracker decisions worth not re-litigating; still worth grepping | | `dev/expertise/*.md` | Standing-practice methodology references | From 932dfc401811257d69b05964a55f689dd9edd291 Mon Sep 17 00:00:00 2001 From: R script Date: Fri, 21 Aug 2026 07:19:15 +0100 Subject: [PATCH 06/22] In production --- inst/REFERENCES.bib | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/inst/REFERENCES.bib b/inst/REFERENCES.bib index 22e2babfb..ebaf3e73d 100644 --- a/inst/REFERENCES.bib +++ b/inst/REFERENCES.bib @@ -644,7 +644,7 @@ @article{SmithConc title = {Which characters support which clades? Exploring the distribution of phylogenetic signal using mutual information}, author = {Smith, Martin R.}, journal = {Systematic Biology}, - pages = {Under review}, + pages = {in production}, year = {2026} } From 8e5d5472352ed80ff686feaf812439224191afb7 Mon Sep 17 00:00:00 2001 From: ms609-agent <313734811+ms609-agent@users.noreply.github.com> Date: Sat, 22 Aug 2026 07:34:25 +0100 Subject: [PATCH 07/22] docs(red-team): retire last_focus now the backfill is complete MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit All 15 areas have a Discussion record, so rotation reads staleness from createdAt and the pointer is dead. Records the invariant createdAt relies on — creation order equals review-recency order — which the backfill broke and discussion #184 restored. Co-Authored-By: Claude Opus 5 --- dev/red-team/log.md | 27 +++++++++++++++------------ 1 file changed, 15 insertions(+), 12 deletions(-) diff --git a/dev/red-team/log.md b/dev/red-team/log.md index 6459e0c6d..34e82fa17 100644 --- a/dev/red-team/log.md +++ b/dev/red-team/log.md @@ -62,11 +62,9 @@ persistently-dry reputation leans on pre-tier rounds (areas 3 and 10 both do) ha ## ⚠ Round records have moved to GitHub Discussions — this file is closed to new entries -**Area 15's three rounds of 2026-08-05/06 are deliberately NOT below.** They are the first -records written under the new scheme, and they live in the repository's Discussions, one post -per round under that area's category: +Every round is now one post in its area's Discussions category, `NN-`: - + Why the move: this file sits on a protected branch, so a finished round's record was hostage to a code review it has nothing to do with. Four completed rounds and 56 filed findings once sat @@ -76,14 +74,19 @@ decouple the record from the merge. **Do not add new round entries here.** Post to the area's Discussions category instead. Title format `RT - area - () - yield `; first line of the body -` () | effort: `. - -**`last_focus:` at the bottom is still live and still governs rotation.** Under the new scheme -the next area is the *stalest* category rather than a pointer — but that ordering cannot be -computed until every area has at least one discussion, and the backfill must post each area's -most recent round **oldest-first** so `createdAt` reproduces true staleness. Only area 15 has -been migrated. Until the rest are, `last_focus:` remains the rotation mechanism and must be -kept current. +` () | effort: | `. + +**Migration complete as of 2026-08-06.** All 15 areas have a record: area 15's three rounds +(#152-154), plus each other area's most recent round backfilled verbatim from this file +(#158-171). + +**`last_focus:` is retired.** The next area is the one whose most recent Discussion has the +oldest `createdAt`. That rests on an invariant — **creation order equals review-recency +order** — which the backfill broke and discussion #184 restored: area 15's three records were +posted before the other fourteen areas were backfilled, so by creation order the most recently +reviewed area looked like the stalest one. Any future backfill or out-of-order re-post must +restore the invariant the same way, with a marker record. The value below is left as a +historical marker and is **not** to be updated. Everything below this line is the frozen historical record, newest first. **It stays**: eleven in-repo files and the `/red-team` skill cite `log.md` by path, and the `T-nnn` ids it carries From 228177e9baeeefb1cda6cc86c34a8139cc8828bb Mon Sep 17 00:00:00 2001 From: ms609-agent <313734811+ms609-agent@users.noreply.github.com> Date: Sat, 22 Aug 2026 07:38:08 +0100 Subject: [PATCH 08/22] docs(red-team): the log.md header still described last_focus rotation The header claimed /red-team appends an entry and updates last_focus, which the same file now says is retired. Replaced with what the file actually is. Co-Authored-By: Claude Opus 5 --- dev/red-team/log.md | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/dev/red-team/log.md b/dev/red-team/log.md index 34e82fa17..7d82ad6dc 100644 --- a/dev/red-team/log.md +++ b/dev/red-team/log.md @@ -1,13 +1,9 @@ # Red-team round log — TreeSearch -Append-only record of every red-team round. **Newest first.** Each invocation of -`/red-team` adds one entry and updates `last_focus:` at the **bottom** of this file. The -next area is `(last_focus mod N) + 1`, where `N` is the current row count in -`focus-areas.md` (**15 as of 2026-08-05**, when areas 14 and 15 were added to close #42's -scope-coverage gap — previously 13 as of 2026-07-03, and **not** the stale `10` this line -said until then, which made areas 11-13 mathematically unreachable by normal rotation; see -RT12-01, 2026-07-03 area-12 round below). **Count the rows; do not trust this number.** -Recompute `N` whenever a row is added. +**Closed to new entries.** A round's record is a GitHub Discussion, one post per round in +that area's category, and rotation reads staleness from those posts — see the pointer block +below. What this file still carries: the model-version legend, the `T-nnn` ids that shipped +source comments cite, and the frozen pre-2026-08 history, **newest first**. **Entry format** (per round): `area`, `reviewed_by`, `date`, `tier` — **which now records the model *version* that ran, not just the rung** (`tier: opus (Opus 4.8)`) — `yield` (count of From 0768b4e3a9d4c67b5c25ffa419c8ca7aaa25a5d1 Mon Sep 17 00:00:00 2001 From: ms609-agent <313734811+ms609-agent@users.noreply.github.com> Date: Sat, 22 Aug 2026 07:44:35 +0100 Subject: [PATCH 09/22] fix(search): report 0 rearrangements when maxIter = 0 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The `iter <- 0L` added alongside the seq_len() fix was dead: `for` binds its variable unconditionally, so a zero-length sequence leaves `iter` NULL rather than falling through to the earlier assignment. The verbosity report then printed "after rearrangements." with a blank. Handle it where it is read, and cover it — the existing maxIter = 0 test runs at verbosity 0, so it could not see this. Co-Authored-By: Claude Opus 5 --- R/CustomSearch.R | 5 +++-- tests/testthat/test-CustomSearch.R | 7 +++++++ 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/R/CustomSearch.R b/R/CustomSearch.R index 859e91142..56d91fa60 100644 --- a/R/CustomSearch.R +++ b/R/CustomSearch.R @@ -59,7 +59,6 @@ EdgeListSearch <- function (edgeList, dataset, } hits <- 0L unimprovedSince <- 0L - iter <- 0L for (iter in seq_len(maxIter)) { candidateLists <- RearrangeEdges(edgeList[[1]], edgeList[[2]], @@ -108,7 +107,9 @@ EdgeListSearch <- function (edgeList, dataset, } if (verbosity > 0L) { #nocov start message(" - Final score ", bestScore, " found ", hits, " times after ", - iter, " rearrangements.", if (verbosity > 1L) "\n" else "") + # A zero-length loop leaves `iter` NULL rather than unset + if (is.null(iter)) 0L else iter, + " rearrangements.", if (verbosity > 1L) "\n" else "") } #nocov end edgeList[3:4] <- c(bestScore, hits) diff --git a/tests/testthat/test-CustomSearch.R b/tests/testthat/test-CustomSearch.R index f8f352e4b..fd2ed3511 100644 --- a/tests/testthat/test-CustomSearch.R +++ b/tests/testthat/test-CustomSearch.R @@ -122,4 +122,11 @@ test_that("EdgeListSearch() performs no rearrangements when maxIter = 0", { maxIter = 0, verbosity = 0L) expect_equal(searched[[3]], 99) # Starting score returned unchanged expect_equal(searched[[4]], 0L) # No hits recorded + + # A zero-length `for` leaves the loop variable NULL, not 0 + expect_message(EdgeListSearch(list(edge[, 1], edge[, 2], 99), dataset = NULL, + TreeScorer = MustNotRun, + EdgeSwapper = MustNotRun, + maxIter = 0, verbosity = 1L), + "after 0 rearrangements") }) From 232164c31fb6243a525934176ae854bf52cb24da Mon Sep 17 00:00:00 2001 From: ms609-agent <313734811+ms609-agent@users.noreply.github.com> Date: Sat, 22 Aug 2026 07:46:52 +0100 Subject: [PATCH 10/22] docs(red-team): cut the archaeology from the area 14 and 15 scope rows MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit These rows argued with superseded versions of themselves — why a start_tier was chosen and why it no longer binds, which #42 urgency justified the row and why it is discharged. A scope row briefs the next round; it is not a record of how it came to say what it says. Keeps the operative content: scope, seam verdict, the standing prohibition on re-running the tier experiment, the transferable lesson, and the next-visit leads. Co-Authored-By: Claude Opus 5 --- dev/red-team/focus-areas.md | 74 ++++++++++++------------------------- 1 file changed, 24 insertions(+), 50 deletions(-) diff --git a/dev/red-team/focus-areas.md b/dev/red-team/focus-areas.md index c962b1bef..151140e4e 100644 --- a/dev/red-team/focus-areas.md +++ b/dev/red-team/focus-areas.md @@ -142,61 +142,35 @@ top of `log.md`; seams that a version bump has made re-eligible are queued in reading the backlog row that holds the actual ask (item 7 explains this at length). Whoever takes area 13 next must decide explicitly: harness first, or #18/#19 first — both are live, and the harness plan predates the two findings. -- **14 Statistics & support metrics — MEASURED 2026-08-05, still yielding heavily.** Added - 2026-08-05 from #42's scope-coverage diff: 5,553 lines across 14 files that were owned by no - area and therefore never reviewed at any tier. **The gap has already cost a finding** — the - arm64 `probe_slot()` hang in `src/MaddisonSlatkin.cpp` (fixed, PR #272, - cf. [[maddisonslatkin-arm64-profile-hang]]) was found incidentally, not by rotation. The code - is numerically dense — recursive DP, factorial caches, log-space arithmetic, Monte Carlo - fallbacks — the profile the tier doctrine normally reserves for `opus`, and #42 recommended - `opus` on that basis. **Deliberately starting at `sonnet` anyway** (maintainer decision, - 2026-08-05): density is a prediction about where bugs *hide*, not evidence that cheap sweeps - are exhausted, and this area has no measured yield at all. **Overtaken by events:** the - first-ever review had already run at `opus` on 2026-08-05, before this row merged, and returned - **36 findings, 4 sev:high — the highest yield on record for this rotation** (see `log.md`). - `start_tier` is left at `sonnet` as decided, but it is now inert: the seam is measured and - yielding, so the routing rules keep the next visit at **opus** with a fresh agent. - **Next visit starts here** (the round's own leads, and the reason it stays opus): the - **array-dimension-drop pattern** — four independent instances in one round (`ConcordanceTable`, - `ClusteringConcordance`, `Consistency`, `ClusterStrings`, all missing `drop = FALSE`), so treat - it as a class and sweep for it rather than re-finding instances; and the **not-yet-examined - `R/PresentContra.R` forest/reference-tip-mismatch angle** — read but never exercised against a - forest whose trees have tips absent from the reference (it calls `KeepTip` first, which *should* - be safe, but that is unproven). Its own test convention +- **14 Statistics & support metrics — MEASURED 2026-08-05, still yielding heavily.** 5,553 + lines across 14 files. The code is numerically dense — recursive DP, factorial caches, + log-space arithmetic, Monte Carlo fallbacks — so brief for that: the first review returned + **36 findings, 4 sev:high, the highest yield on record for this rotation**. Next visit stays + at **opus** with a fresh agent. + **Next visit starts here:** the **array-dimension-drop pattern** — four independent instances + in one round (`ConcordanceTable`, `ClusteringConcordance`, `Consistency`, `ClusterStrings`, + all missing `drop = FALSE`), so sweep for it as a class rather than re-finding instances; and + the **not-yet-examined `R/PresentContra.R` forest/reference-tip-mismatch angle** — read but + never exercised against a forest whose trees have tips absent from the reference (it calls + `KeepTip` first, which *should* be safe, but that is unproven). Its own test convention (`test-MaddisonSlatkin.R`, `test-Concordance.R`, `test-ParsSim.R`, `test-Consistency.R`, `test-ScoreSpectrum.R`, `test-QuartetResolution.R`, `test-TaxonInfluence.R`, `test-WideSample.R`, `test-pp-*.R`) is a useful first read. - **15 Legacy pure-R search API — MEASURED 2026-08-05 at BOTH sonnet and opus; yielding - heavily. The `start_tier` column still reads `sonnet` — the maintainer's recorded choice on - #42 — but the experiment below falsifies the reasoning behind it. Left unchanged pending the - maintainer's call; it is inert either way, since routing governs a visited area.** Added - 2026-08-05 from #42's scope-coverage diff: 2,183 lines across 9 files backing the - still-shipped pre-C++-engine search functions, owned by no area. #42 offered "review once as - frozen legacy, then deprioritise"; **the maintainer chose a full rotation area instead - (2026-08-05): keep revisiting until the seam stops yielding.** Legacy is not the same as - clean, and this code is still shipped and still the documented entry point for users who have - not moved to the C++ engine. Treat "it isn't growing" as a reason the seam should *exhaust* + heavily.** 2,183 lines across 9 files backing the still-shipped pre-C++-engine search + functions. Keep revisiting until the seam stops yielding: legacy is not the same as clean, + and this code is still shipped and still the documented entry point for users who have not + moved to the C++ engine. Treat "it isn't growing" as a reason the seam should *exhaust* quickly, not as a reason to stop early. - **The `#16`/`EdgeListScore()` urgency that originally justified this row is now discharged** - — #16 is closed, its guard landed in PR #50, and the first round measured the pure-R layer - above it as *doubly* guarded (`R/CustomSearch.R:209` and `R/Ratchet.R:96` both reject - non-bifurcating input independently of the C++ fix; a 600-iteration fuzz of the default - rearrangement path produced no non-binary trees). Do not spend a second round re-asking it; - reopen only if the C++ guard is relaxed or a new caller bypasses those entry checks. - **`src/rearrange.cpp` added 2026-08-06** while closing #147: `all_tbr()` had never broken the - root edge, so `TBRMoves()` returned a strict subset of `SPRMoves()` for six years. The bug is - instructive twice over. It was an **off-by-one propagated by copy**: `all_spr()` was created - in 2020 as a copy of `all_tbr()`, inherited its `break_seq` starting at edge 3, and had that - corrected in PR #65 (2021) — the parent never was. And two `dev/benchmarks/` scripts had - already *characterised* the omission and routed around it by enumerating at two rootings, - without anyone filing it. **A documented workaround for a package deficiency is a finding - that was never written down** — grep `dev/` for such comments when auditing a new file. - **This row carried the tier-economics experiment, and it settled the tier question for good.** - Paired passes over the identical scope on 2026-08-05: `sonnet` returned 5 candidates and **0 - sev:high**; `opus`, handed sonnet's entire yield as off-limits, returned **26 candidates and 4 - sev:high**, all 26 confirmed. The cheap pass removed no work from the expensive one. Sonnet - found broken *documented contracts*; opus found *silent wrong answers* — different classes, - not different amounts. See the log's two 2026-08-05 entries; **do not re-run this experiment.** + **Do not re-run the tier experiment.** Paired passes over identical scope, 2026-08-05: + `sonnet` returned 5 candidates and **0 sev:high**; `opus`, handed sonnet's entire yield as + off-limits, returned **26 candidates and 4 sev:high**, all confirmed. The cheap pass removed + no work from the expensive one. Sonnet found broken *documented contracts*, opus found + *silent wrong answers* — different classes, not different amounts. + **A documented workaround for a package deficiency is a finding that was never written + down** — grep `dev/` for such comments when auditing a new file. Two `dev/benchmarks/` + scripts had characterised `all_tbr()`'s missing root-edge break and routed around it by + enumerating at two rootings, and nobody filed it for six years (#147, `src/rearrange.cpp`). **Next visit starts here** (stay at `opus`, fresh agent, and prefer a targeted shape over another general finder): the **decayed custom-criterion façade** — #137 (`Ratchet()` never forwards `TreeScorer`), #126 (`SuccessiveApproximations()`'s undocumented capability gap) and From 63b7e70ac449c8f613abf38740af94def4c7fb70 Mon Sep 17 00:00:00 2001 From: ms609-agent <313734811+ms609-agent@users.noreply.github.com> Date: Sat, 22 Aug 2026 07:57:13 +0100 Subject: [PATCH 11/22] fix(search): don't announce a tree search that will not happen "Performing tree search" was emitted before maxIter was consulted, so maxIter = 0 claimed a search had started and only the closing summary contradicted it. Gate the claim instead, and ask maxIter directly rather than inferring it from the loop variable being left NULL. Co-Authored-By: Claude Opus 5 --- R/CustomSearch.R | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/R/CustomSearch.R b/R/CustomSearch.R index 56d91fa60..1f0bdce1a 100644 --- a/R/CustomSearch.R +++ b/R/CustomSearch.R @@ -46,7 +46,7 @@ EdgeListSearch <- function (edgeList, dataset, bestScore <- edgeList[[3]] } } - if (verbosity > 0L) { + if (verbosity > 0L && maxIter > 0L) { message(" - Performing tree search. Initial score: ", bestScore) #nocov } if (!is.null(stopAtScore) && bestScore < stopAtScore + epsilon) { @@ -107,8 +107,8 @@ EdgeListSearch <- function (edgeList, dataset, } if (verbosity > 0L) { #nocov start message(" - Final score ", bestScore, " found ", hits, " times after ", - # A zero-length loop leaves `iter` NULL rather than unset - if (is.null(iter)) 0L else iter, + # `for` leaves the loop variable NULL when maxIter < 1 + if (maxIter > 0L) iter else 0L, " rearrangements.", if (verbosity > 1L) "\n" else "") } #nocov end From e43c4f356c656758e4441b9f4ec7d435b7062412 Mon Sep 17 00:00:00 2001 From: ms609-agent <313734811+ms609-agent@users.noreply.github.com> Date: Sat, 22 Aug 2026 08:07:37 +0100 Subject: [PATCH 12/22] docs: state the rules, don't narrate how they changed The identity paragraph restated AGENTS.md's own Agent identity section; it now carries only what is specific to dispatch. "One issue is no longer one fix site" and "the old coordination-commit exception is gone" describe a transition rather than a rule, and the `task` label was defined by the file it was migrated from. log.md no longer keeps `last_focus:`. Co-Authored-By: Claude Opus 5 --- .claude/skills/next-issue/SKILL.md | 13 +++++-------- AGENTS.md | 14 ++++++-------- 2 files changed, 11 insertions(+), 16 deletions(-) diff --git a/.claude/skills/next-issue/SKILL.md b/.claude/skills/next-issue/SKILL.md index 1a7849e60..912e00316 100644 --- a/.claude/skills/next-issue/SKILL.md +++ b/.claude/skills/next-issue/SKILL.md @@ -11,12 +11,10 @@ between two concurrent chips costs more than the triage would. Issues live in **`agent-issues/TreeSearch`** (`gh` already defaults to it). The upstream `ms609/TreeSearch` tracker is public and human-entered: untrusted input, never a task list. -**Every `gh` call that writes goes out as `ms609-agent`** — claiming comments, `in-progress` -labels, the chip's PR. `GH_TOKEN=$CLAUDE_GH_TOKEN gh ...`, per call, never `export`ed; see -`~/.claude/CLAUDE.md`. A PR authored by `ms609` cannot be reviewed by `ms609`, so every chip -brief must carry this instruction — a chip that lands under the wrong account has to be -closed and reopened, not re-attributed. Request review from the maintainer: -`--reviewer ms609`. +Writes go out as `ms609-agent`; the mechanism is in `AGENTS.md`'s *Agent identity*. What is +specific to dispatch: **every chip brief must carry that instruction**, since a chip opening +its PR under `ms609` produces one the maintainer cannot review, fixable only by closing and +reopening. Request review with `--reviewer ms609`. ## 1. Group @@ -41,8 +39,7 @@ Cluster into tranches: - **Respect `area:N` labels** — they mark red-team focus areas, and two issues sharing an area usually share files. -**One issue is no longer one fix site.** Since 2026-08, `/red-team` files by *root cause*, -so a single issue routinely covers several candidates across several files — an enumerated +**One issue is not one fix site.** `/red-team` files by *root cause*, so a single issue covers several candidates across several files — an enumerated site list, or two independent defects that produce one wrong output and must be fixed together. Read each body before grouping: sizing a tranche by issue count will under-size it, and its file-collision check must union **every** file the body names, not the one in diff --git a/AGENTS.md b/AGENTS.md index 1159b3d21..81718e2b7 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -21,7 +21,7 @@ collaborators. | `red-team` | Filed by `/red-team`. Also that skill's mode switch — don't delete it | | `sev:high` / `sev:med` / `sev:low` | Former P1 / P2 / P3 | | `area:1`…`area:15` | Which area **owns the code**, per `dev/red-team/focus-areas.md` — not which round found it; an issue may carry several. **Count the rows, this range grows** | -| `task` | Planned work migrated from the retired `to-do.md` | +| `task` | Planned work, not a red-team finding | | `deferred` | Assessed and parked; not scheduled | | `chore` | Infrastructure / process work | | `in-progress` | Claimed; the claiming comment names the branch | @@ -114,10 +114,9 @@ bash C:/Users/pjjg18/GitHub/gha-poll.sh ``` Both scripts resolve the target repo with `gh repo view --json nameWithOwner`, so they pick -up whatever `gh repo set-default` points at — the fork. **Do not `cd ..` first** (as this -recipe used to say): outside a git repo that lookup fails and the dispatch targets nothing. -"Run these FROM the repo" means your `cwd` must be the git checkout/worktree doing the -`gh repo view` lookup — it does not mean the scripts themselves must be found relatively. +up whatever `gh repo set-default` points at — the fork. **Do not `cd ..` first**: outside a git repo that lookup fails and the +dispatch targets nothing. Your `cwd` must be the git checkout or worktree — the scripts +themselves are found by absolute path, not relatively. ### Local builds (targeted iteration only) @@ -213,8 +212,7 @@ agent-issues/TreeSearch ### Rules - **Agents never push to the fork's `cpp-search` directly** — everything lands by reviewed - PR, documentation included. The old coordination-commit exception is gone with the files - that justified it. + PR, documentation included. - **`feature/*`** branches from `cpp-search`, owned by one agent at a time. - **Never commit directly to upstream `cpp-search`.** While upstream only ever *receives* the fork's trunk, every sync is a fast-forward — no merge, no conflict on @@ -292,7 +290,7 @@ These recur; they are activities, not issues, and have no tracker entry: | File | Purpose | |------|---------| | **GitHub issues** (`agent-issues/TreeSearch`) | The task queue and the findings tracker | -| **GitHub Discussions**, one category per `area:N` | Red-team round records. `dev/red-team/log.md` is **closed to new entries** — it keeps only the model-version legend, `last_focus:`, and the frozen pre-2026-08 history | +| **GitHub Discussions**, one category per `area:N` | Red-team round records. `dev/red-team/log.md` is **closed to new entries** — it keeps only the model-version legend and the frozen pre-2026-08 history | | `dev/red-team/` | Scope and tiers: `focus-areas.md`; frozen `log.md`, `findings-archive.md`, `migration-map*.tsv` | | `dev/strategy.md` | Historical strategic narrative (was `coordination.md`; **not** kept current) | | `completed-tasks.md` | **Frozen.** Pre-tracker decisions worth not re-litigating; still worth grepping | From c7a47cd52f716d1ede081ce9d5e90ec33790f1a0 Mon Sep 17 00:00:00 2001 From: "Martin R. Smith" <1695515+ms609@users.noreply.github.com> Date: Thu, 10 Sep 2026 12:44:29 +0100 Subject: [PATCH 13/22] TreeTools 2.4.1 on CRAN --- DESCRIPTION | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/DESCRIPTION b/DESCRIPTION index 7f822d08c..43ab88b4b 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -51,9 +51,8 @@ Imports: Rdpack (>= 0.7), stats, TreeDist (>= 2.6.3), - TreeTools (>= 2.4.0.9002), + TreeTools (>= 2.4.1), utils, -Remotes: ms609/TreeTools, Suggests: cluster, Coreset, From b85baa6eae2a53a7789b793e5710298a7cd8cb94 Mon Sep 17 00:00:00 2001 From: "Martin R. Smith" <1695515+ms609@users.noreply.github.com> Date: Thu, 10 Sep 2026 12:46:33 +0100 Subject: [PATCH 14/22] SmithConc doi --- inst/REFERENCES.bib | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/inst/REFERENCES.bib b/inst/REFERENCES.bib index 488631558..d6f11ca71 100644 --- a/inst/REFERENCES.bib +++ b/inst/REFERENCES.bib @@ -680,7 +680,8 @@ @article{SmithConc title = {Which characters support which clades? Exploring the distribution of phylogenetic signal using mutual information}, author = {Smith, Martin R.}, journal = {Systematic Biology}, - pages = {in production}, + pages = {online ahead of print}, + doi = {10.1093/sysbio/syag071} year = {2026} } From a8f6bc8f596d6084708841845056ab87203862f6 Mon Sep 17 00:00:00 2001 From: ms609-agent <313734811+ms609-agent@users.noreply.github.com> Date: Fri, 18 Sep 2026 20:07:02 +0100 Subject: [PATCH 15/22] chore(red-team): freeze escalation-backlog.md; residuals go to GitHub No issue-shaped work belongs in a git-tracked file. A specific defect is an issue; a routing or sequencing ask is a post in the area's Discussion category, beside the round records. As a live queue this file drifted into a duplicate ledger of issue state. The `/red-team` skill no longer appends here: a version bump that reopens dormancy now files an issue or posts to a Discussion, and the `tidy` pass drains the remaining rows rather than sweeping and re-writing them. Co-Authored-By: Claude Opus 5 --- dev/red-team/escalation-backlog.md | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/dev/red-team/escalation-backlog.md b/dev/red-team/escalation-backlog.md index 6c2911c10..4b9465d61 100644 --- a/dev/red-team/escalation-backlog.md +++ b/dev/red-team/escalation-backlog.md @@ -1,8 +1,15 @@ # Red-team escalation backlog — TreeSearch -Seams that are **re-eligible now** but are not next in the rotation. The `/red-team` skill -reopens entries here when a rung's model version moves on (`/red-team revisit`); a normal -rotation round may also pull an item from here when it reaches that area. +> **FROZEN 2026-09-18 — read-only, never append.** No issue-shaped work goes in a +> git-tracked file. A specific defect is a GitHub issue; a routing or sequencing ask is a +> post in that area's Discussion category, beside the round records. This file is kept for +> its resolved-history narrative and is drained by the `/red-team tidy` pass — a row naming +> an open issue whose ask is a tier escalation becomes a `needs-escalation` label; anything +> else migrates to a Discussion. As a live queue it drifted, which is why it is closed. + +Seams that were **re-eligible** but not next in the rotation. Historically the `/red-team` +skill reopened entries here when a rung's model version moved on (`/red-team revisit`); that +now happens by filing an issue or posting to the area's Discussion category. This file does **not** advance `last_focus:` and does not replace `log.md` — each item is a pointer back to the round that recorded the residual. Full context stays in `log.md`; only From 2430f15fb3788ef537a17611450e12b56c385184 Mon Sep 17 00:00:00 2001 From: ms609-agent <313734811+ms609-agent@users.noreply.github.com> Date: Fri, 18 Sep 2026 20:24:53 +0100 Subject: [PATCH 16/22] chore(agents): add the /next-issue config for this repo /next-issue is now a global skill shared with mkp and StratoBayes; only what varies per repo lives here. For TreeSearch that is mostly the base branch: work targets cpp-search, not main, and --base cpp-search is not optional, since the fork's default branch governs whether Fixes #N closes an issue on merge. Grouping prefers the area:N labels over a hot-file list - the areas were drawn along subsystem lines, so they already approximate the file-collision graph and are read from the issue rather than re-derived. deferred is explicitly not the maintainer-call filter: it means "not now", not "needs a human decision". Build ids are derived from the branch rather than the session, because a resumed chip gets a new session id and would silently build a second tree. Co-Authored-By: Claude Opus 5 --- dev/next-issue/config.md | 64 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 64 insertions(+) create mode 100644 dev/next-issue/config.md diff --git a/dev/next-issue/config.md b/dev/next-issue/config.md new file mode 100644 index 000000000..12114de4c --- /dev/null +++ b/dev/next-issue/config.md @@ -0,0 +1,64 @@ +# `/next-issue` config — TreeSearch + +Read by `~/.claude/skills/next-issue/SKILL.md`. Only what varies per repo lives +here; the doctrine is in the skill. + +| Key | Value | +|-----|-------| +| `base_branch` | **`cpp-search`**, not `main` — the integration branch, on `origin` = `agent-issues/TreeSearch`. **Never `upstream` (`ms609/TreeSearch`)**: push URL is `no-push-use-gha`, and `ms609-agent` is read-only there by design. Work reaches `ms609` only when the human syncs the fork. | +| `grouping` | `labels`, prefix `area:` (1–15). Fall back to `hot_files` for an unlabelled issue. | +| `exclude_label` | `deferred` is **not** the maintainer-call filter — it means "not now", not "needs a human decision". Exclude by judgment per the skill until `needs-decision` exists here. | +| `identity` | `agent` — `ms609-agent`, token env var `CLAUDE_GH_TOKEN`. Table in `~/.claude/CLAUDE.md`. | + +## `hot_files` — never split across parallel chips + +Derive the current list from the tranche's `area:N` label plus +`dev/red-team/focus-areas.md`, which maps each area to its files. The standing +coordination-critical ones: + +``` +src/Makevars.win # shared; coordination rules in AGENTS.md +src/TreeSearch-win.def # shared; append-only +src/TreeSearch-init.c # arg-count mismatches are a known build failure mode +``` + +## `pr_command` + +```bash +GH_TOKEN=$CLAUDE_GH_TOKEN gh pr create --base cpp-search --head --reviewer ms609 --body-file +``` + +`--base cpp-search` is not optional — the fork's default branch governs whether +`Fixes #N` closes on merge. Reads need no token prefix. Never `export GH_TOKEN`. + +## `build` / `test` + +GHA is the primary validation path. Local builds are for targeted iteration: + +```bash +ID="a$(printf %s "$BRANCH" | sha1sum | cut -c1-5)" +bash build-agent.sh TreeSearch-a "$ID" +bash test-agent.sh TreeSearch-a "$ID" [filter] +``` + +Derive the id from the branch, never the session id — a resumed chip gets a new +session id and would silently build a second tree. + +Targeted filter always — never a full suite. + +## `branch_rule` + +One agent owns a `feature/*` branch at a time; **never `git checkout` a branch +you do not own** — use `git worktree add`. **Never `git stash`**: the stash is +repo-wide across worktrees and can apply another agent's work into yours; use a +patch file. Coordination files live on `cpp-search` only. + +## `extra` + +- **Max 2 cores per agent.** +- Debug `.o` contamination and DLL locks are the two recurring build failures — + recovery steps in `AGENTS.md` → *Build failure recovery*. +- Prefer `TreeTools` over `ape` (see `AGENTS.md`). +- `dev/red-team/log.md`, `findings-archive.md` and `migration-map*.tsv` are + **frozen**. Never add a row to any of them, and never to + `escalation-backlog.md` — findings are issues. From 5da95e7bd3b197c9b266e3c29ef812d6045bf89b Mon Sep 17 00:00:00 2001 From: ms609-agent <313734811+ms609-agent@users.noreply.github.com> Date: Wed, 23 Sep 2026 07:31:21 +0100 Subject: [PATCH 17/22] docs(red-team): suspend fable; Opus 5.5 is the top finder rung Opus 5.5 outclasses Fable 5.1 at about a fifth of the price, so the ladder no longer follows alias name. Record Opus 5.5 in the model-version legend, mark fable suspended until the maintainer re-enables it, and route everything that pointed at fable to opus. Every dry or dormant verdict recorded on an outclassed model is reopened. Co-Authored-By: Claude Opus 5.5 --- .claude/skills/next-issue/SKILL.md | 3 ++- dev/red-team/escalation-backlog.md | 8 ++++---- dev/red-team/focus-areas.md | 13 +++++-------- dev/red-team/log.md | 4 +++- 4 files changed, 14 insertions(+), 14 deletions(-) diff --git a/.claude/skills/next-issue/SKILL.md b/.claude/skills/next-issue/SKILL.md index 912e00316..9cf36d126 100644 --- a/.claude/skills/next-issue/SKILL.md +++ b/.claude/skills/next-issue/SKILL.md @@ -107,7 +107,8 @@ condition* rather than live work. - **Sonnet** — guard clauses, dead code, R-level fixes, tests, local refactors. - **Opus** — `src/` kernels, scoring semantics (Fitch/IW/profile/HSJ/XFORM), constraint machinery, parallelism and RNG, `NAMESPACE`, cross-file mechanism fixes. -- **Fable** — only after an Opus chip in this tranche has stalled twice. +- **Fable** — suspended while Opus 5.5 outclasses it (2026-09-23); never dispatch it. An Opus + chip that stalls twice is reported back, not escalated. Effort — **reasoning depth, not task size**: diff --git a/dev/red-team/escalation-backlog.md b/dev/red-team/escalation-backlog.md index 4b9465d61..68c145132 100644 --- a/dev/red-team/escalation-backlog.md +++ b/dev/red-team/escalation-backlog.md @@ -15,9 +15,9 @@ This file does **not** advance `last_focus:` and does not replace `log.md` — e pointer back to the round that recorded the residual. Full context stays in `log.md`; only the *ask* and the *rung* live here. -**Rung rule.** Version bump before rung bump: `opus-4.8 dry → opus-5 → fable`. A dry verdict -is evidence about the version that ran, not about the rung. See the model-version legend at -the top of `log.md`. +**Rung rule.** A dry verdict is evidence about the model that ran; a more capable model on any +rung reopens it. `fable` is suspended (2026-09-23), so every row below routes to `opus`. See the +model-version legend at the top of `log.md`. --- @@ -26,7 +26,7 @@ the top of `log.md`. The **five table rows** were reopened 2026-07-27 by the `opus` 4.8 → 5 version bump. Each was recorded on 2026-07-24 (or earlier) as **"escalate to FABLE"**; the opus bump makes the same-rung step available and cheaper, so each is re-queued at **`opus` (Opus 5) with a fresh-angle brief**. -Fable remains the step after Opus 5 *also* runs dry. The residuals themselves are unchanged — +The residuals themselves are unchanged — only the rung aimed at them. **Item 5(a) was retired 2026-07-28** — the bug claim it carried had already been refuted, in diff --git a/dev/red-team/focus-areas.md b/dev/red-team/focus-areas.md index 151140e4e..e42b6e260 100644 --- a/dev/red-team/focus-areas.md +++ b/dev/red-team/focus-areas.md @@ -17,11 +17,10 @@ The rotation still adjusts per recorded yield — a dry round escalates one tier round re-visits at the same tier with a fresh agent, a high-severity signal escalates immediately. Treat these as the starting point, not a ceiling. -**A tier is a rung, not a model.** A dry verdict is scoped to the *version* that produced it, -and a version bump at the same rung is a cheaper step than a rung bump — so `opus-4.8 dry` goes -to **Opus 5**, not to fable. The alias→version mapping lives in the model-version legend at the -top of `log.md`; seams that a version bump has made re-eligible are queued in -`escalation-backlog.md`. +**A tier is a rung, not a model.** A dry verdict is scoped to the *model* that produced it, and +a more capable model on any rung reopens it. `opus` is the only finder rung: `fable` is +suspended while Opus 5.5 outclasses it (2026-09-23). The alias→version mapping lives in the +model-version legend at the top of `log.md`. | # | Area | Files | start_tier | Key questions | |---|------|-------|-----------|---------------| @@ -44,9 +43,7 @@ top of `log.md`; seams that a version bump has made re-eligible are queued in ### Maturity / tier rationale (one line each) - **1 Fitch correctness — opus.** Crown jewel; T-300 (systematic delta=−3) and T-306 were - opus-class subtle bugs. Dry at **opus-4.8** (2026-07-24) ⇒ next visit is **opus (Opus 5)**, - fresh-angle; **fable** is the escalation only once Opus 5 *also* runs dry (version bump before - rung bump — see the model-version legend at the top of `log.md`). + opus-class subtle bugs. - **2 Topology invariants — opus.** Deep state-restore subtleties; T-235 (SPR stale state), T-316 (P1 stale constraint metadata after tabu rejection). - **3 Ratchet & perturbation — opus.** Mature, but the `build_reduced_dataset` / diff --git a/dev/red-team/log.md b/dev/red-team/log.md index 26a3bb907..4849d2b4e 100644 --- a/dev/red-team/log.md +++ b/dev/red-team/log.md @@ -37,7 +37,9 @@ whenever a rung moves, and reopen whatever the bump makes re-eligible in | `sonnet` | **Sonnet 4.6** | every tier-era round through 2026-07-24 | All `tier: sonnet` entries below ran on 4.6. | | `sonnet` | **Sonnet 5** | current, from 2026-07-27 | Supersedes 4.6. | | `fable` | *version unconfirmed* | 2026-07-24 area-6 dual-tier round | Not captured at dispatch. The cost observation in that round (fable ≈ opus tokens, not 2×) is scoped to that unknown version. | -| `fable` | **Fable 5** | current, from 2026-07-27 | | +| `fable` | **Fable 5** | 2026-07-27 → 2026-09-22 | | +| `opus` | **Opus 5.5** | current, from 2026-09-23 | Outclasses Fable 5.1 at ~1/5 the price ⇒ every `dry`/`dormant` verdict at Opus 4.8, Opus 5 **or fable** is re-eligible at `opus`. | +| `fable` | **Fable 5.1** — **suspended** | from 2026-09-23 | Outclassed by Opus 5.5; not dispatched for finders, verifiers or chips until the maintainer re-enables it. | | `haiku` | **Haiku 4.5** | every round (verifier tier) | One version across the whole log; no bump to date. | **Version bump before rung bump.** The ladder *within* a rung comes first: From a7325fe5f415c297471a163aaad4d8aab3f8c178 Mon Sep 17 00:00:00 2001 From: ms609-agent <313734811+ms609-agent@users.noreply.github.com> Date: Wed, 23 Sep 2026 10:09:55 +0100 Subject: [PATCH 18/22] docs(red-team): keep scope-only focus-areas; model status lives in Discussions Drop the project model-version legend and the per-area start_tier and verdict text. Each round's Discussion title stamps the model that ran, and the /red-team skill holds the capability order, so a model release needs no write here. The legend and any carried-over per-area asks were archived to Discussions before removal. Co-Authored-By: Claude Opus 5.5 --- dev/expertise/red-team.md | 2 +- dev/red-team/README.md | 4 +- dev/red-team/focus-areas.md | 194 ++++-------------------------------- dev/red-team/log.md | 43 +------- 4 files changed, 26 insertions(+), 217 deletions(-) diff --git a/dev/expertise/red-team.md b/dev/expertise/red-team.md index c436c2b35..3d9163e21 100644 --- a/dev/expertise/red-team.md +++ b/dev/expertise/red-team.md @@ -2,7 +2,7 @@ > **This file is the curated "lessons learned" — not the operational machinery.** > The live rotation now lives under `dev/red-team/` (per the `/red-team` skill): -> - `dev/red-team/focus-areas.md` — the 10-area rotation table + per-area `start_tier` +> - `dev/red-team/focus-areas.md` — the scope table (files + key questions) > - `dev/red-team/log.md` — append-only round log + `last_focus:` pointer > - `dev/red-team/findings.md` — verified OPEN findings > diff --git a/dev/red-team/README.md b/dev/red-team/README.md index 7af9824ea..789b34ae8 100644 --- a/dev/red-team/README.md +++ b/dev/red-team/README.md @@ -12,8 +12,8 @@ hypothetical, it is what the 2026-07-27 rounds measured. | File | What it is | Who writes it | |------|------------|---------------| -| [`focus-areas.md`](focus-areas.md) | The rotation table: 15 numbered areas, the files each owns, its `start_tier`, and its key questions. Built once, edited rarely. **Adding a row** also needs, and nothing currently automates: a matching `area:N` GitHub label (`gh label create area:N --description "Red-team focus area N"`), and recomputing `N` in `log.md`'s rotation-formula comment (see RT12-01). | A round, when it finds the scope row wrong | -| [`log.md`](log.md) | Append-only, **newest first**. One entry per round (`area` / `reviewed_by` / `date` / `tier` / `yield` / `notes`), the **model-version legend** at the top, and `last_focus:` at the very bottom. | Every round | +| [`focus-areas.md`](focus-areas.md) | The scope table: 15 numbered areas, the files each owns, and its key questions — no tier or status. Built once, edited rarely. **Adding a row** also needs, and nothing currently automates: a matching `area:N` GitHub label (`gh label create area:N --description "Red-team focus area N"`), and recomputing `N` in `log.md`'s rotation-formula comment (see RT12-01). | A round, when it finds the scope row wrong | +| [`log.md`](log.md) | Append-only, **newest first**. One entry per round (`area` / `reviewed_by` / `date` / `tier` / `yield` / `notes`), frozen history; the model-version legend is archived in [discussion #189](https://github.com/agent-issues/TreeSearch/discussions/189). | Every round | | **GitHub issues** in [`agent-issues/TreeSearch`](https://github.com/agent-issues/TreeSearch/issues?q=label%3Ared-team) | **OPEN verified findings live here since 2026-08-04**, labelled `red-team` + `sev:high\|med\|low` + `area:N`. Status is GitHub state, so it cannot drift from merge state. Filed *after* verification; trivial issues are fixed inline and noted in `log.md` instead. | A round files; a merged `Fixes #N` closes | | [`findings-archive.md`](findings-archive.md) | **FROZEN 2026-08-04.** Terminal-state findings from the file era, one compressed line each. **Offline anti-duplication memory, not a trophy case** — the one thing the tracker doesn't provide. | Nobody; it is closed to new rows | | [`migration-map.tsv`](migration-map.tsv) | Every historical `T-nnn` → its issue number, archive entry, or open-PR reference. `T-nnn` ids are **frozen, not retired**: they persist in shipped source comments and in `log.md`. | Written once, at migration | diff --git a/dev/red-team/focus-areas.md b/dev/red-team/focus-areas.md index e42b6e260..18b241eae 100644 --- a/dev/red-team/focus-areas.md +++ b/dev/red-team/focus-areas.md @@ -1,179 +1,25 @@ # Red-team focus areas — TreeSearch -Rotation table for the `/red-team` skill. Built once, edited rarely. Each `/red-team` -invocation reviews **one** area (the next in rotation, see `last_focus:` at the bottom of -`log.md`) at its earned tier, then records the round in `log.md`. Verified non-trivial -findings are filed as GitHub issues in `agent-issues/TreeSearch` (labelled `red-team`, +Scope table for the `/red-team` skill: the files each area owns and its key questions. +Definition only — tier, yield, dormancy and next-visit asks live in each area's round +Discussions (category `NN-`), never here. Verified non-trivial findings are filed as GitHub issues in `agent-issues/TreeSearch` (labelled `red-team`, `sev:high|med|low`, `area:N`). Durable lessons (bug patterns, fragile areas) live in `../expertise/red-team.md`. -## start_tier - -`start_tier` is the tier a **never-visited or freshly-rotated** area starts at. Unlike the -skill's default (everything `sonnet`), these tiers **encode measured maturity** from the -ported round history (`log.md`): areas whose seams have only ever yielded subtle, -opus-class bugs start higher; immature seams that still bleed cheap bugs start at `sonnet`. -The rotation still adjusts per recorded yield — a dry round escalates one tier, a yielding -round re-visits at the same tier with a fresh agent, a high-severity signal escalates -immediately. Treat these as the starting point, not a ceiling. - -**A tier is a rung, not a model.** A dry verdict is scoped to the *model* that produced it, and -a more capable model on any rung reopens it. `opus` is the only finder rung: `fable` is -suspended while Opus 5.5 outclasses it (2026-09-23). The alias→version mapping lives in the -model-version legend at the top of `log.md`. - -| # | Area | Files | start_tier | Key questions | -|---|------|-------|-----------|---------------| -| 1 | **Fitch scoring correctness** | `src/ts_fitch.h/.cpp`, `src/ts_fitch_na.h`, `src/ts_fitch_na_incr.h`, `src/ts_fitch_na_dirty.h`, `src/ts_simd.h` (added 2026-08-04 — the bit-parallel SIMD portability layer every Fitch combine call goes through; UNMEASURED, no inherited maturity) | **opus** | Does incremental / dirty-set scoring match full `score_tree()`? Bounded variants bail correctly? NA three-pass edge cases? Write a targeted test if you find a gap. | -| 2 | **Search topology invariants** | `src/ts_tbr.cpp`, `src/ts_drift.cpp`, `src/ts_search.cpp`, `src/ts_tree.cpp/.h`, `src/ts_pool.cpp/.h`, `src/ts_tabu.h` (added 2026-08-04 — the tabu-list hash buffer that directly implements this row's own "symmetry-breaking hash collisions" question; UNMEASURED), `src/ts_temper.cpp/.h` (added 2026-08-06 — stochastic TBR with Boltzmann acceptance plus the annealing schedule, called from `ts_drift.cpp`, `ts_driven.cpp` and `ts_rcpp.cpp`; a probabilistically *rejected* move is precisely this row's restore question, and it was the last `src/` file owned by no row. UNMEASURED) | **opus** | After every rejected move, is topology fully restored? Undo stack correct? No stale `postorder`? Constraint metadata re-synced on *all* reject paths (incl. tabu)? Symmetry-breaking hash collisions? `TreePool` (dedup key, capacity eviction) consistent with the topology invariants above? | -| 3 | **Ratchet & perturbation** | `src/ts_ratchet.cpp`, `src/ts_sector.cpp`, `src/ts_fuse.cpp`, `src/ts_prune_reinsert.cpp` | **opus** | `active_mask`/`upweight_mask`/`flat_blocks` fully restored after perturbation? Sectorial reinsertion reverts on worse score? `build_reduced_dataset` copies all needed fields? Fuse handles tied scores? | -| 4 | **Parallelism & RNG** | `src/ts_parallel.cpp`, `src/ts_rng.h/.cpp`, `src/ts_driven.cpp`, `src/ts_resample.cpp`, `src/ts_heartbeat.cpp/.h`, `src/build_postorder.h`, `src/ts_strategy.h`, `R/Resample.R`, `R/Jackknife.R` (added 2026-08-04 — `ts_heartbeat`/`build_postorder.h` are exactly this row's bug class: main-thread-only R-API + `set.seed()`-reproducibility RNG state; `ts_strategy.h` is the bandit consumed by `ts_driven.cpp`; `R/Resample.R`/`R/Jackknife.R` are the R-level entries to the parallel resample path already named in T-336/T-337/T-398 but never an owned file. ALL UNMEASURED, no inherited maturity) | **opus** | Thread-local RNG set before any search call? **No R API (incl. `unif_rand`/`Get/PutRNGstate`) from worker threads** — note the resample path. Pool mutex correct? Atomic stop-flag races? Seeds drawn from R RNG before spawn? | -| 5 | **Data pipeline & simplification** | `src/ts_data.h/.cpp`, `src/ts_simplify.h/.cpp`, `src/ts_ls.h/.cpp`, `R/tree_length.R`, `R/IWScore.R`, `R/LeastSquares.R`, `R/PrepareData.R`, `R/data.R`, `R/data_manipulation.R`, `R/fractional-weights.R`, `R/length_range.R` (added 2026-08-04 — the R-layer half of `TreeLength`/`MinimumLength`/`CharacterLength`/IW scoring flagged unowned by the 2026-07-03 area-12 round and by `escalation-backlog.md` item 5(a); `R/tree_length.R` is directly implicated by open issue #16/T-400, sev:high. ALL UNMEASURED, no inherited maturity) | **opus** | `build_dataset` handles edge cases (all-ambiguous, single-state, zero-weight, `n_states==32` UBSAN)? `build_reduced_dataset` copies all fields? XPIWE `obs==0` division? Least-squares distance fitting (`ts_ls.cpp`) — degenerate `dist` (NA/Inf) handled (cf. filed P1: `LeastSquaresFit`/`LeastSquaresTree` RSS=0 garbage)? | -| 6 | **R ↔ C++ interface** | `src/ts_rcpp.cpp`, `src/TreeSearch-init.c`, `src/RcppExports.cpp` (added 2026-08-04 — the generated third leg of the same boundary triangle as `R/RcppExports.R`; a signature drift here is exactly what `compile-attrs.R` exists to catch), `R/RcppExports.R`, `R/MaximizeParsimony.R`, `R/SearchControl.R`, `R/ts-driven-compat.R` (added 2026-08-04 — the flat-argument-to-grouped-list compatibility shim T-398/#14 traced through; UNMEASURED) | **sonnet** | Arg counts match? Concavity sentinel translated? Edge-matrix conventions? Return value attributes/types set (frozen-API `logical` vs `integer`)? Parameter validation in R layer? | -| 7 | **Shiny module wiring** | `inst/Parsimony/server.R`, `inst/Parsimony/global.R`, `inst/Parsimony/ui.R`, `inst/Parsimony/server/mod_*.R`, `inst/Parsimony/server/app_state.R`, `inst/Parsimony/server/logging.R`, `inst/Parsimony/tests/testthat.R`, `inst/Parsimony/tests/testthat/*.R` (added 2026-08-04 — the app's own test suite, in neither this row nor area 8's `tests/testthat/` glob; includes two quarantined `_problems/test-app-smoke-*.R` worth reading for *why* they're quarantined. UNMEASURED) | **sonnet** | Forward-ref callbacks resolve? Cross-module `updateXxxInput` namespaces correct? Re-entrancy / double-launch guards? Stale dataset-hash on async tasks? `onStop` cleanup (cancel signal + temp files)? Orphaned observers? | -| 8 | **Test suite health** | `tests/testthat/*.R` (broadened 2026-08-04 — the literal `test-ts-*.R` glob excluded ~44 of ~110 files, e.g. `test-CustomSearch.R`, `test-tree_length.R`, `test-MaddisonSlatkin.R`, `test-Concordance.R`; issue #4/T-363 already treated a non-`ts`-prefixed file as in-area, so the row's own precedent was already broader than its text. The non-`ts` files carry NO inherited maturity — treat as never-reviewed) | **sonnet** | Tier guards correct? Vacuous (always-pass) assertions? Missing `TreeSearch:::` prefixes? `set.seed()` before `sample()`? Edge-case coverage gaps (3-tip, single-char, all-NA)? Enduring regression for incremental-rescore? | -| 9 | **Wagner & addition trees** | `src/ts_wagner.h/.cpp`, `R/AdditionTree.R`, `R/PolEscapa.R` | **opus** | NA-incremental scoring staleness acceptable? Constraint mapping (LCA-based) correct? Retry loop fires? 3-taxon base case handles all orderings? R-layer index/`sequence` validation (OOB-write guard)? | -| 10 | **Alternative scoring kernels: Profile/IW/HSJ/XFORM** | `src/ts_fitch.cpp` (IW/profile paths), `src/ts_data.cpp` (precompute), `src/ts_hsj.cpp/.h`, `src/ts_sankoff.cpp/.h`, **`R/recode_hierarchy.R`**, **`R/CharacterHierarchy.R`** (added 2026-08-03 — see the rationale note; treat both as UNMEASURED), plus the criterion's **consumers** where a non-Fitch objective meets Fitch-only machinery: `src/ts_tbr.cpp` (candidate scan / accept / `try_root_edge_moves`), `src/ts_rcpp.cpp` (`unpack_hsj`, `unpack_xform`) | **opus** | `e/(k+e)` delta correct? Profile `info_amounts` lookup + capping matches? `concavity = 1.0` sentinel activates weighted path? `precompute_profile_delta` includes `precomputed_steps` offset? Clipped-subtree homoplasy in screening? HSJ/XFORM (`ds.hierarchy_blocks`/`ds.sankoff_*`) scoring correctness in its own right (not just collapse-flag blindness, cf. T-330 area 11) — does anything else outside collapse assume `ds.blocks[]` is exhaustive? | -| 11 | **Zero-length-branch collapse (MPT set)** | `src/ts_collapsed.cpp/.h`, `src/ts_splits.cpp` (`compute_collapsed_splits`), `src/ts_rcpp.cpp` (`ts_collapse_flags_batch`), `src/ts_tbr.cpp` (enum `add_collapsed` sites), `R/MaximizeParsimony.R` (collapse block) | **opus** | DEFAULT-ON since 2026-06-24, so every `MaximizeParsimony` call exercises it. Does `compute_collapsed_flags_aggressive` flag the *correct* min-length-0 branches under **IW / profile / NA**, not just EW (verified)? Is it really rooting-invariant, or does tip-rooting+`RenumberTips(labs)` alignment break on constraint trees / user start trees / `RenumberTips` permutations (cf. [[na-validation-alignment-gotcha]])? Can the dedup key `write.tree(SortTree(unroot(t)))` over-merge (two distinct collapsed topologies → same key) or under-merge across rootings? `result$scores == best_score` float-equality safe under IW/profile? Degenerate inputs: star tree, single MPT, 3–4 tips, all-resolved (must be exact no-op), fully-unresolved? Does collapse ever produce a tree that violates an active `constraint`? | -| 12 | **Red-team process meta-review** | `dev/red-team/focus-areas.md`, `dev/red-team/log.md`, the `red-team` issue list in `agent-issues/TreeSearch`, `dev/red-team/README.md` | **sonnet** | Are any areas too broad — spanning multiple distinct seams such that a finder concentrating on one file family misses another? Are any too narrow — a single-feature scope that would be better merged into a neighbour? Do any areas overlap (same source files audited under two different area headings)? Has any area gone persistently dry (≥ 3 consecutive rounds with zero confirmed findings) — should it be retired, merged, or downtiered? Are there new code seams (recently merged features, new source files) not covered by any existing area? Are tier assignments calibrated to actual yield recorded in `log.md` — any area that keeps surprising at its current tier and should escalate, or one that has been consistently empty and should drop? Propose concrete restructuring actions (split, merge, retire, add, re-tier) with rationale tied to `log.md` yield history. | -| 13 | **Constrained search correctness** | `src/ts_constraint.h/.cpp`, `src/ts_nni_perturb.cpp`, constraint integration points in `src/ts_driven.cpp` (fuse), `src/ts_parallel.cpp` (parallel-fuse), `src/ts_wagner.cpp`/`src/ts_sector.cpp` (posthoc retry), `src/ts_tbr.cpp` (`regraft_violates_constraint`) | **opus** | Does every `impose_constraint()` caller verify-before-capture, not just trust an improved score (T-213 gap, fixed d9a4f827: `nni_perturb_search` was the one caller that didn't re-check `constraint_node[]` after repair — fuse/parallel-fuse already did)? Any other heuristic-repair or posthoc-retry caller (Wagner build retry, sector) that skips discard-on-failure? Is `impose_one_pass`'s `best_node` reference stale after its own move-out loop's `topology_spr()` calls relocate a node — traced mechanism, produced one `std::bad_alloc` crash under experimental code, did NOT reproduce in 600 stress-test seeds against shipped code; needs a targeted adversarial tree construction, not more random seeds, to confirm either way. Is `map_constraint_nodes`/DFS-timestamp resync correct on every topology-mutation path, including reject paths (cross-check vs area 2's tabu-reject question)? Are nested/overlapping constraint splits handled consistently across TBR clip-gating, Wagner retry, and sector/fuse posthoc paths? | -| 14 | **Statistics & support metrics** | `src/MaddisonSlatkin.cpp`, `src/expected_mi.cpp`, `src/ts_mc_fitch.cpp`, `src/quartet_concordance.cpp`, `R/Concordance.R`, `R/ParsSim.R`, `R/pp_info_extra_step.r`, `R/WideSample.R`, `R/Consistency.R`, `R/TaxonInfluence.R`, `R/ScoreSpectrum.R`, `R/RandomTreeScore.R`, `R/WhenFirstHit.R`, `R/QuartetResolution.R`, `R/PresentContra.R`, `R/ClusterStrings.R` (last two added 2026-08-05 — owned by no other row, and both reviewed by the first-ever round) | **sonnet** | Is the recursive Maddison–Slatkin DP correct at its recursion boundaries, and does its cache key everything the recurrence depends on? Does the factorial-cache log-space arithmetic under/overflow at realistic tip counts, and are log-space sums accumulated stably? When does the exact DP hand off to the Monte Carlo fallback, and is the fallback's estimator unbiased — or silently substituted without the caller being able to tell? Are concordance-factor statistics well-defined on polytomies, on single-taxon splits, and on characters with missing data? Do the R wrappers validate tip-label correspondence, or index by position (cf. the [[na-validation-alignment-gotcha]] class)? Is any of this reachable from `MaximizeParsimony()`'s default output path the way #16/T-400 was — and does it return a silently wrong number rather than erroring? (carried from the 2026-08-05 round, where this question produced three of the four `sev:high` findings) | -| 15 | **Legacy pure-R search API** | `R/CustomSearch.R` (`TreeSearch()`), `R/Ratchet.R`, `R/NNI.R`, `R/SPR.R`, `R/TBR.R`, `R/SuccessiveApproximations.R`, `R/tree_rearrangement.R`, `R/morphy-deprecated.R`, `R/Bootstrap.R`, `src/rearrange.cpp` (added 2026-08-06 — the C++ enumerators (`nni`, `spr_moves`, `spr`, `all_spr`, `all_tbr`) that this row's own R functions call; owned by no row, and #147 was found only because an area-15 finder used `TBRMoves()` as a cross-check. UNMEASURED) | **sonnet** | Is `EdgeListScore()` — the default `TreeScorer` for `TreeSearch()`/`Ratchet()`/`Jackknife()`, and one of the four entry points #16 confirms vulnerable — reachable with the out-of-bounds inputs #16 describes? Do the pure-R rearrangement samplers (`NNI`/`SPR`/`TBR`) generate only valid topologies, and do they cover the neighbourhood they claim? Does `SuccessiveApproximations` reweight consistently with the C++ IW kernel, or has it drifted? Do `Bootstrap`/`Jackknife` resample characters with the weights the user supplied? Does anything here still route through removed MorphyLib paths (`morphy-deprecated.R`)? | - -### Maturity / tier rationale (one line each) - -- **1 Fitch correctness — opus.** Crown jewel; T-300 (systematic delta=−3) and T-306 were - opus-class subtle bugs. -- **2 Topology invariants — opus.** Deep state-restore subtleties; T-235 (SPR stale state), - T-316 (P1 stale constraint metadata after tabu rejection). -- **3 Ratchet & perturbation — opus.** Mature, but the `build_reduced_dataset` / - mask-sync class (T-275, T-303) keeps recurring. -- **4 Parallelism & RNG — opus.** Looked mature until T-309 (P1: R RNG API on worker - threads via the parallel `Resample()` path, 2026-06-15). Concurrency = P1-capable. -- **5 Data pipeline — opus.** DAT-001 (`1u<<32` UBSAN at `n_states==32`), DAT-002 (XPIWE - `obs==0` division). Edge inputs still bite. -- **6 R ↔ C++ interface — sonnet.** Mostly mechanical arg-count / sentinel audits; cheap to - run. T-310 (frozen-API `pruneReinsertNni` type) shows it still occasionally yields — - escalate if a sonnet pass goes dry. -- **7 Shiny wiring — sonnet.** **Immature seam:** a Sonnet pass found 5 bugs on 2026-06-16 - (T-309…T-313). Keep mining cheap until it runs dry. *(Scope row corrected 2026-07-27: it - named a phantom `server/events.R` — no such file — while omitting `global.R` (430 lines), - `ui.R`, `app_state.R` and `logging.R`, ~850 real lines that no round had ever owned. The - 2026-06-16 round read only `server.R` + `mod_search.R`, so `mod_consensus.R` (1449), - `mod_treespace.R` (776), `mod_data.R` (660), `mod_clustering.R`, `mod_downloads.R` and - `mod_references.R` remain unreviewed at any tier.)* -- **8 Test suite health — sonnet.** Reliably yields inline fixes (`set.seed`, vacuous - asserts) and test-gap notes (T-304). -- **9 Wagner & addition — opus.** Kernel code; WGN-01 (P1 OOB write via `AdditionTree(sequence=)`). -- **10 Alternative scoring kernels — opus. THE HIGHEST-YIELDING AREA ON RECORD: 4 P1s in one - round** (2026-07-28, Opus 5 — T-373…T-376, all in HSJ/XFORM; T-376 shows the HSJ score is not - even a function of the data, and is wrong on a shipped dataset through the public API). - **Read the cautionary tale before trusting any tier state here.** The area looked mature — - "numerical delta algebra, subtle conservative bugs" — and its Profile/IW half genuinely is: - every static residual was re-derived clean in that same round. But `ts_hsj.*`/`ts_sankoff.*` - were added to the scope row on 2026-07-03 and **no finder had read them at any tier**, so - both of the area's dry rounds (2026-05-19, 2026-05-26 — themselves pre-tier and - version-unrecorded, hence *zero* version-scoped dry verdicts) were dry about a **different - half of the area than the row described**. Generalisable lesson: **a scope row that grows - does not inherit the dry verdicts earned before it grew** — when widening a row, reset its - measured maturity for the new files. Stays opus; next visit resumes on the HSJ/XFORM half - (`ts_sankoff.*` got materially less attention than `ts_hsj.*`), and carries T-374's open - design question — whether a rooted objective is *intended* for XFORM, and whether Hopkins & - St John's dissimilarity is defined on a single MPR or on marginal MPR sets, which is a - **literature** question, not a finder question. - **UPDATED 2026-08-03 (second consecutive high-yield round: 8 filed + T-377 reopened).** The - HSJ literature question is now **settled from the paper** (rooting-invariance is required) and - the HSJ half of T-374 is **fixed and merged** (`34eea581`); the XFORM design question is with - the user. Three things changed about how this area should be attacked: - (a) **The live seam has moved OUT of the kernels and INTO the consumers.** `ts_sankoff.*` was - re-read in full and came back with a **reasoned clean on its most likely defect** (cost-matrix - orientation on the asymmetric matrix — no transposition, argument in `log.md`), while the - round's headline (T-392, P1) is in `src/ts_tbr.cpp`: `try_root_edge_moves` adopts a strictly - worse HSJ/XFORM score with no comparison and no restore, on the **shipped default** path - (`.AutoRung` → `sprint` → `tabuSize = 0` for every dataset ≤30 taxa). Attack the machinery - that *consumes* these criteria, not just the kernels that compute them. - (b) **Two R files produced three findings while sitting in NO area's scope row** — T-393/T-394 - (`R/recode_hierarchy.R` derives a secondary's state space from token strings) and T-395 - (`R/CharacterHierarchy.R` rejects every nested hierarchy, so the documented example is - unusable while `R CMD check` stays green). Both are now in the row above, and per this area's - own 2026-07-28 lesson they carry **no inherited maturity**: a grown scope row does not inherit - dry verdicts, so treat them as never-reviewed. The 2026-07-03 area-12 round had already - flagged `R/CharacterHierarchy.R` as unowned — that note sat unactioned for a month and cost - three findings' delay. - (c) **The next visit should NOT be a finder.** T-392 + the reopened T-377 together say - HSJ/XFORM search hill-climbs on the Fitch residue with the hierarchy term as a mere accept - filter, and the reach cost is now measured at 25 tips — but the **fix shape is undecided** and - the candidate gate costs 2.7–2.9× wall. What is owed is a **wall-matched A/B of the fix shapes - on a ≥25-tip hierarchy-heavy matrix, Hamilton-class**, mirroring the area-13 precedent where - the right next step was a bounded harness rather than another review. -- **11 Zero-length collapse — opus, NEVER REVIEWED.** New + default-on; implementation hit - three subtle traps in one sitting (conservative flags rooting-sensitive; aggressive flags - need tip-rooting; tip-data alignment via `RenumberTips`). Cross-mode correctness - (IW/profile/NA) and dedup canonicalization are the under-verified seams. Start opus. -- **12 Red-team meta-review — sonnet.** *(Corrected 2026-08-04 — this line read "NEVER - REVIEWED" despite a 2026-07-03 round having already run; see `log.md` for both rounds.)* - Pure document review; no code to trace. Cheap per round; findings are restructuring - proposals (split/merge/retire/add/re-tier) that improve every future round. Escalate to - opus only if evaluating a proposed split requires reading source files to assess scope - boundaries. Still yielding at sonnet on both rounds to date — the file-coverage diff alone - (2026-08-04) surfaced ~37 of 44 `R/*.R` files and ~12 `src/*` files owned by no row. -- **13 Constrained search — opus.** *(Rationale corrected 2026-08-04 — the text below had - read "NEVER REVIEWED" since 2026-07-02 through two rounds that reviewed it.)* Split out - 2026-07-02 after fixing T-213 (`nni_perturb_search` missing verify-before-capture on - `impose_constraint()`'s heuristic repair — cf. [[impose-constraint-verify-gap]]). The - 2026-07-02 round traced but did NOT confirm a second failure mode (stale `best_node` across - `topology_spr()` relocation inside `impose_one_pass`); the 2026-07-03 round's audit of every - `impose_constraint()` caller sharpened that into a precise, still-open question (can the - `postorder.size()==n_internal` revert-guard be slipped by a net-zero corruption?) and closed - with **"NEXT VISIT: NOT another finder — a BOUNDED EXHAUSTIVE HARNESS"** on the - `topology_spr`/`build_postorder`-guard equivalence, mirrored in `escalation-backlog.md`'s - "Not in this backlog (deliberately)" section. **That verdict is still standing and rotation - reaches area 13 next** (after this area-12 round) — whoever dispatches that round should read - it before defaulting to a finder. Two findings were ALSO filed into this area's scope from a - 2026-07-28→2026-08-04 area-11 round without an area-13 finder ever running: **#18/T-402 - (sev:high)** — a `constraint` silently ignored when `tree=` supplies a violating start — and - **#19/T-403 (sev:med)** — collapse's "enforced splits protected" promise has no access to - `consZero` and can return constraint-violating trees under default `collapse=TRUE` - (`escalation-backlog.md` item 7). Deliberately **not** `needs-escalation`-labelled: that flag - means only "dispatch at `opus`+", which this row already is, and a label hit makes step 3 skip - reading the backlog row that holds the actual ask (item 7 explains this at length). Whoever - takes area 13 next must decide explicitly: harness first, or #18/#19 first — both are live, - and the harness plan predates the two findings. -- **14 Statistics & support metrics — MEASURED 2026-08-05, still yielding heavily.** 5,553 - lines across 14 files. The code is numerically dense — recursive DP, factorial caches, - log-space arithmetic, Monte Carlo fallbacks — so brief for that: the first review returned - **36 findings, 4 sev:high, the highest yield on record for this rotation**. Next visit stays - at **opus** with a fresh agent. - **Next visit starts here:** the **array-dimension-drop pattern** — four independent instances - in one round (`ConcordanceTable`, `ClusteringConcordance`, `Consistency`, `ClusterStrings`, - all missing `drop = FALSE`), so sweep for it as a class rather than re-finding instances; and - the **not-yet-examined `R/PresentContra.R` forest/reference-tip-mismatch angle** — read but - never exercised against a forest whose trees have tips absent from the reference (it calls - `KeepTip` first, which *should* be safe, but that is unproven). Its own test convention - (`test-MaddisonSlatkin.R`, `test-Concordance.R`, `test-ParsSim.R`, `test-Consistency.R`, - `test-ScoreSpectrum.R`, `test-QuartetResolution.R`, `test-TaxonInfluence.R`, - `test-WideSample.R`, `test-pp-*.R`) is a useful first read. -- **15 Legacy pure-R search API — MEASURED 2026-08-05 at BOTH sonnet and opus; yielding - heavily.** 2,183 lines across 9 files backing the still-shipped pre-C++-engine search - functions. Keep revisiting until the seam stops yielding: legacy is not the same as clean, - and this code is still shipped and still the documented entry point for users who have not - moved to the C++ engine. Treat "it isn't growing" as a reason the seam should *exhaust* - quickly, not as a reason to stop early. - **Do not re-run the tier experiment.** Paired passes over identical scope, 2026-08-05: - `sonnet` returned 5 candidates and **0 sev:high**; `opus`, handed sonnet's entire yield as - off-limits, returned **26 candidates and 4 sev:high**, all confirmed. The cheap pass removed - no work from the expensive one. Sonnet found broken *documented contracts*, opus found - *silent wrong answers* — different classes, not different amounts. - **A documented workaround for a package deficiency is a finding that was never written - down** — grep `dev/` for such comments when auditing a new file. Two `dev/benchmarks/` - scripts had characterised `all_tbr()`'s missing root-edge break and routed around it by - enumerating at two rootings, and nobody filed it for six years (#147, `src/rearrange.cpp`). - **Next visit starts here** (stay at `opus`, fresh agent, and prefer a targeted shape over - another general finder): the **decayed custom-criterion façade** — #137 (`Ratchet()` never - forwards `TreeScorer`), #126 (`SuccessiveApproximations()`'s undocumented capability gap) and - the dead `SuccessiveWeights()` are one story, and the useful work is auditing *which advertised - custom-criterion entry points work end to end*, each with a test whose scorer is - distinguishable from `EdgeListScore` (a test using an equivalent scorer cannot see #137); - and **porting the validated neighbourhood enumerator to the C++ `all_spr`/`all_tbr` paths**, - which were never audited — the R-side harness (validated against 2(n−3) and 2(n−3)(2n−7)) - found four distinct sampler defects in one pass and should generalise. +| # | Area | Files | Key questions | +|---|------|-------|---------------| +| 1 | **Fitch scoring correctness** | `src/ts_fitch.h/.cpp`, `src/ts_fitch_na.h`, `src/ts_fitch_na_incr.h`, `src/ts_fitch_na_dirty.h`, `src/ts_simd.h` (added 2026-08-04 — the bit-parallel SIMD portability layer every Fitch combine call goes through; UNMEASURED, no inherited maturity) | Does incremental / dirty-set scoring match full `score_tree()`? Bounded variants bail correctly? NA three-pass edge cases? Write a targeted test if you find a gap. | +| 2 | **Search topology invariants** | `src/ts_tbr.cpp`, `src/ts_drift.cpp`, `src/ts_search.cpp`, `src/ts_tree.cpp/.h`, `src/ts_pool.cpp/.h`, `src/ts_tabu.h` (added 2026-08-04 — the tabu-list hash buffer that directly implements this row's own "symmetry-breaking hash collisions" question; UNMEASURED), `src/ts_temper.cpp/.h` (added 2026-08-06 — stochastic TBR with Boltzmann acceptance plus the annealing schedule, called from `ts_drift.cpp`, `ts_driven.cpp` and `ts_rcpp.cpp`; a probabilistically *rejected* move is precisely this row's restore question, and it was the last `src/` file owned by no row. UNMEASURED) | After every rejected move, is topology fully restored? Undo stack correct? No stale `postorder`? Constraint metadata re-synced on *all* reject paths (incl. tabu)? Symmetry-breaking hash collisions? `TreePool` (dedup key, capacity eviction) consistent with the topology invariants above? | +| 3 | **Ratchet & perturbation** | `src/ts_ratchet.cpp`, `src/ts_sector.cpp`, `src/ts_fuse.cpp`, `src/ts_prune_reinsert.cpp` | `active_mask`/`upweight_mask`/`flat_blocks` fully restored after perturbation? Sectorial reinsertion reverts on worse score? `build_reduced_dataset` copies all needed fields? Fuse handles tied scores? | +| 4 | **Parallelism & RNG** | `src/ts_parallel.cpp`, `src/ts_rng.h/.cpp`, `src/ts_driven.cpp`, `src/ts_resample.cpp`, `src/ts_heartbeat.cpp/.h`, `src/build_postorder.h`, `src/ts_strategy.h`, `R/Resample.R`, `R/Jackknife.R` (added 2026-08-04 — `ts_heartbeat`/`build_postorder.h` are exactly this row's bug class: main-thread-only R-API + `set.seed()`-reproducibility RNG state; `ts_strategy.h` is the bandit consumed by `ts_driven.cpp`; `R/Resample.R`/`R/Jackknife.R` are the R-level entries to the parallel resample path already named in T-336/T-337/T-398 but never an owned file. ALL UNMEASURED, no inherited maturity) | Thread-local RNG set before any search call? **No R API (incl. `unif_rand`/`Get/PutRNGstate`) from worker threads** — note the resample path. Pool mutex correct? Atomic stop-flag races? Seeds drawn from R RNG before spawn? | +| 5 | **Data pipeline & simplification** | `src/ts_data.h/.cpp`, `src/ts_simplify.h/.cpp`, `src/ts_ls.h/.cpp`, `R/tree_length.R`, `R/IWScore.R`, `R/LeastSquares.R`, `R/PrepareData.R`, `R/data.R`, `R/data_manipulation.R`, `R/fractional-weights.R`, `R/length_range.R` (added 2026-08-04 — the R-layer half of `TreeLength`/`MinimumLength`/`CharacterLength`/IW scoring flagged unowned by the 2026-07-03 area-12 round and by `escalation-backlog.md` item 5(a); `R/tree_length.R` is directly implicated by open issue #16/T-400, sev:high. ALL UNMEASURED, no inherited maturity) | `build_dataset` handles edge cases (all-ambiguous, single-state, zero-weight, `n_states==32` UBSAN)? `build_reduced_dataset` copies all fields? XPIWE `obs==0` division? Least-squares distance fitting (`ts_ls.cpp`) — degenerate `dist` (NA/Inf) handled (cf. filed P1: `LeastSquaresFit`/`LeastSquaresTree` RSS=0 garbage)? | +| 6 | **R ↔ C++ interface** | `src/ts_rcpp.cpp`, `src/TreeSearch-init.c`, `src/RcppExports.cpp` (added 2026-08-04 — the generated third leg of the same boundary triangle as `R/RcppExports.R`; a signature drift here is exactly what `compile-attrs.R` exists to catch), `R/RcppExports.R`, `R/MaximizeParsimony.R`, `R/SearchControl.R`, `R/ts-driven-compat.R` (added 2026-08-04 — the flat-argument-to-grouped-list compatibility shim T-398/#14 traced through; UNMEASURED) | Arg counts match? Concavity sentinel translated? Edge-matrix conventions? Return value attributes/types set (frozen-API `logical` vs `integer`)? Parameter validation in R layer? | +| 7 | **Shiny module wiring** | `inst/Parsimony/server.R`, `inst/Parsimony/global.R`, `inst/Parsimony/ui.R`, `inst/Parsimony/server/mod_*.R`, `inst/Parsimony/server/app_state.R`, `inst/Parsimony/server/logging.R`, `inst/Parsimony/tests/testthat.R`, `inst/Parsimony/tests/testthat/*.R` (added 2026-08-04 — the app's own test suite, in neither this row nor area 8's `tests/testthat/` glob; includes two quarantined `_problems/test-app-smoke-*.R` worth reading for *why* they're quarantined. UNMEASURED) | Forward-ref callbacks resolve? Cross-module `updateXxxInput` namespaces correct? Re-entrancy / double-launch guards? Stale dataset-hash on async tasks? `onStop` cleanup (cancel signal + temp files)? Orphaned observers? | +| 8 | **Test suite health** | `tests/testthat/*.R` (broadened 2026-08-04 — the literal `test-ts-*.R` glob excluded ~44 of ~110 files, e.g. `test-CustomSearch.R`, `test-tree_length.R`, `test-MaddisonSlatkin.R`, `test-Concordance.R`; issue #4/T-363 already treated a non-`ts`-prefixed file as in-area, so the row's own precedent was already broader than its text. The non-`ts` files carry NO inherited maturity — treat as never-reviewed) | Tier guards correct? Vacuous (always-pass) assertions? Missing `TreeSearch:::` prefixes? `set.seed()` before `sample()`? Edge-case coverage gaps (3-tip, single-char, all-NA)? Enduring regression for incremental-rescore? | +| 9 | **Wagner & addition trees** | `src/ts_wagner.h/.cpp`, `R/AdditionTree.R`, `R/PolEscapa.R` | NA-incremental scoring staleness acceptable? Constraint mapping (LCA-based) correct? Retry loop fires? 3-taxon base case handles all orderings? R-layer index/`sequence` validation (OOB-write guard)? | +| 10 | **Alternative scoring kernels: Profile/IW/HSJ/XFORM** | `src/ts_fitch.cpp` (IW/profile paths), `src/ts_data.cpp` (precompute), `src/ts_hsj.cpp/.h`, `src/ts_sankoff.cpp/.h`, **`R/recode_hierarchy.R`**, **`R/CharacterHierarchy.R`** (added 2026-08-03 — see the rationale note; treat both as UNMEASURED), plus the criterion's **consumers** where a non-Fitch objective meets Fitch-only machinery: `src/ts_tbr.cpp` (candidate scan / accept / `try_root_edge_moves`), `src/ts_rcpp.cpp` (`unpack_hsj`, `unpack_xform`) | `e/(k+e)` delta correct? Profile `info_amounts` lookup + capping matches? `concavity = 1.0` sentinel activates weighted path? `precompute_profile_delta` includes `precomputed_steps` offset? Clipped-subtree homoplasy in screening? HSJ/XFORM (`ds.hierarchy_blocks`/`ds.sankoff_*`) scoring correctness in its own right (not just collapse-flag blindness, cf. T-330 area 11) — does anything else outside collapse assume `ds.blocks[]` is exhaustive? | +| 11 | **Zero-length-branch collapse (MPT set)** | `src/ts_collapsed.cpp/.h`, `src/ts_splits.cpp` (`compute_collapsed_splits`), `src/ts_rcpp.cpp` (`ts_collapse_flags_batch`), `src/ts_tbr.cpp` (enum `add_collapsed` sites), `R/MaximizeParsimony.R` (collapse block) | DEFAULT-ON since 2026-06-24, so every `MaximizeParsimony` call exercises it. Does `compute_collapsed_flags_aggressive` flag the *correct* min-length-0 branches under **IW / profile / NA**, not just EW (verified)? Is it really rooting-invariant, or does tip-rooting+`RenumberTips(labs)` alignment break on constraint trees / user start trees / `RenumberTips` permutations (cf. [[na-validation-alignment-gotcha]])? Can the dedup key `write.tree(SortTree(unroot(t)))` over-merge (two distinct collapsed topologies → same key) or under-merge across rootings? `result$scores == best_score` float-equality safe under IW/profile? Degenerate inputs: star tree, single MPT, 3–4 tips, all-resolved (must be exact no-op), fully-unresolved? Does collapse ever produce a tree that violates an active `constraint`? | +| 12 | **Red-team process meta-review** | `dev/red-team/focus-areas.md`, `dev/red-team/log.md`, the `red-team` issue list in `agent-issues/TreeSearch`, `dev/red-team/README.md` | Are any areas too broad — spanning multiple distinct seams such that a finder concentrating on one file family misses another? Are any too narrow — a single-feature scope that would be better merged into a neighbour? Do any areas overlap (same source files audited under two different area headings)? Has any area gone persistently dry (≥ 3 consecutive rounds with zero confirmed findings) — should it be retired, merged, or downtiered? Are there new code seams (recently merged features, new source files) not covered by any existing area? Are tier assignments calibrated to actual yield recorded in `log.md` — any area that keeps surprising at its current tier and should escalate, or one that has been consistently empty and should drop? Propose concrete restructuring actions (split, merge, retire, add, re-tier) with rationale tied to `log.md` yield history. | +| 13 | **Constrained search correctness** | `src/ts_constraint.h/.cpp`, `src/ts_nni_perturb.cpp`, constraint integration points in `src/ts_driven.cpp` (fuse), `src/ts_parallel.cpp` (parallel-fuse), `src/ts_wagner.cpp`/`src/ts_sector.cpp` (posthoc retry), `src/ts_tbr.cpp` (`regraft_violates_constraint`) | Does every `impose_constraint()` caller verify-before-capture, not just trust an improved score (T-213 gap, fixed d9a4f827: `nni_perturb_search` was the one caller that didn't re-check `constraint_node[]` after repair — fuse/parallel-fuse already did)? Any other heuristic-repair or posthoc-retry caller (Wagner build retry, sector) that skips discard-on-failure? Is `impose_one_pass`'s `best_node` reference stale after its own move-out loop's `topology_spr()` calls relocate a node — traced mechanism, produced one `std::bad_alloc` crash under experimental code, did NOT reproduce in 600 stress-test seeds against shipped code; needs a targeted adversarial tree construction, not more random seeds, to confirm either way. Is `map_constraint_nodes`/DFS-timestamp resync correct on every topology-mutation path, including reject paths (cross-check vs area 2's tabu-reject question)? Are nested/overlapping constraint splits handled consistently across TBR clip-gating, Wagner retry, and sector/fuse posthoc paths? | +| 14 | **Statistics & support metrics** | `src/MaddisonSlatkin.cpp`, `src/expected_mi.cpp`, `src/ts_mc_fitch.cpp`, `src/quartet_concordance.cpp`, `R/Concordance.R`, `R/ParsSim.R`, `R/pp_info_extra_step.r`, `R/WideSample.R`, `R/Consistency.R`, `R/TaxonInfluence.R`, `R/ScoreSpectrum.R`, `R/RandomTreeScore.R`, `R/WhenFirstHit.R`, `R/QuartetResolution.R`, `R/PresentContra.R`, `R/ClusterStrings.R` (last two added 2026-08-05 — owned by no other row, and both reviewed by the first-ever round) | Is the recursive Maddison–Slatkin DP correct at its recursion boundaries, and does its cache key everything the recurrence depends on? Does the factorial-cache log-space arithmetic under/overflow at realistic tip counts, and are log-space sums accumulated stably? When does the exact DP hand off to the Monte Carlo fallback, and is the fallback's estimator unbiased — or silently substituted without the caller being able to tell? Are concordance-factor statistics well-defined on polytomies, on single-taxon splits, and on characters with missing data? Do the R wrappers validate tip-label correspondence, or index by position (cf. the [[na-validation-alignment-gotcha]] class)? Is any of this reachable from `MaximizeParsimony()`'s default output path the way #16/T-400 was — and does it return a silently wrong number rather than erroring? (carried from the 2026-08-05 round, where this question produced three of the four `sev:high` findings) | +| 15 | **Legacy pure-R search API** | `R/CustomSearch.R` (`TreeSearch()`), `R/Ratchet.R`, `R/NNI.R`, `R/SPR.R`, `R/TBR.R`, `R/SuccessiveApproximations.R`, `R/tree_rearrangement.R`, `R/morphy-deprecated.R`, `R/Bootstrap.R`, `src/rearrange.cpp` (added 2026-08-06 — the C++ enumerators (`nni`, `spr_moves`, `spr`, `all_spr`, `all_tbr`) that this row's own R functions call; owned by no row, and #147 was found only because an area-15 finder used `TBRMoves()` as a cross-check. UNMEASURED) | Is `EdgeListScore()` — the default `TreeScorer` for `TreeSearch()`/`Ratchet()`/`Jackknife()`, and one of the four entry points #16 confirms vulnerable — reachable with the out-of-bounds inputs #16 describes? Do the pure-R rearrangement samplers (`NNI`/`SPR`/`TBR`) generate only valid topologies, and do they cover the neighbourhood they claim? Does `SuccessiveApproximations` reweight consistently with the C++ IW kernel, or has it drifted? Do `Bootstrap`/`Jackknife` resample characters with the weights the user supplied? Does anything here still route through removed MorphyLib paths (`morphy-deprecated.R`)? | diff --git a/dev/red-team/log.md b/dev/red-team/log.md index 4849d2b4e..2ecf3ec89 100644 --- a/dev/red-team/log.md +++ b/dev/red-team/log.md @@ -2,8 +2,9 @@ **Closed to new entries.** A round's record is a GitHub Discussion, one post per round in that area's category, and rotation reads staleness from those posts — see the pointer block -below. What this file still carries: the model-version legend, the `T-nnn` ids that shipped -source comments cite, and the frozen pre-2026-08 history, **newest first**. +below. What this file still carries: the `T-nnn` ids that shipped +source comments cite, and the frozen pre-2026-08 history, **newest first**. The model-version +legend that used to open it is archived in [discussion #189](https://github.com/agent-issues/TreeSearch/discussions/189). **Entry format** (per round): `area`, `reviewed_by`, `date`, `tier` — **which now records the model *version* that ran, not just the rung** (`tier: opus (Opus 4.8)`) — `yield` (count of @@ -20,44 +21,6 @@ maps every historical `T-nnn`); version-bump re-eligible seams in `escalation-ba --- -## Model-version legend (alias → version) - -A tier is a **rung, not a model**. `sonnet` / `opus` / `fable` / `haiku` are Agent-tool -aliases that always resolve to the *newest* model at that rung; the version behind each moves -under us. Every backward-looking verdict in this log (`ran dry`, `dormant`, `retire`) is -therefore evidence about **the version that ran**, never about the rung forever. This table is -the project-local mapping the `/red-team` skill's *Model versions* rule points at: add a row -whenever a rung moves, and reopen whatever the bump makes re-eligible in -`escalation-backlog.md`. - -| Alias | Version | Period | Notes | -|-------|---------|--------|-------| -| `opus` | **Opus 4.8** | every tier-era round through 2026-07-24 | All `tier: opus` entries below ran on 4.8. | -| `opus` | **Opus 5** | current, from 2026-07-27 | Supersedes 4.8 ⇒ every `dry at opus-4.8` verdict is re-eligible at this same rung. | -| `sonnet` | **Sonnet 4.6** | every tier-era round through 2026-07-24 | All `tier: sonnet` entries below ran on 4.6. | -| `sonnet` | **Sonnet 5** | current, from 2026-07-27 | Supersedes 4.6. | -| `fable` | *version unconfirmed* | 2026-07-24 area-6 dual-tier round | Not captured at dispatch. The cost observation in that round (fable ≈ opus tokens, not 2×) is scoped to that unknown version. | -| `fable` | **Fable 5** | 2026-07-27 → 2026-09-22 | | -| `opus` | **Opus 5.5** | current, from 2026-09-23 | Outclasses Fable 5.1 at ~1/5 the price ⇒ every `dry`/`dormant` verdict at Opus 4.8, Opus 5 **or fable** is re-eligible at `opus`. | -| `fable` | **Fable 5.1** — **suspended** | from 2026-09-23 | Outclassed by Opus 5.5; not dispatched for finders, verifiers or chips until the maintainer re-enables it. | -| `haiku` | **Haiku 4.5** | every round (verifier tier) | One version across the whole log; no bump to date. | - -**Version bump before rung bump.** The ladder *within* a rung comes first: -`opus-4.8 dry → opus-5 → fable`, not `opus dry → fable`. The 2026-07-27 opus bump therefore -re-targets every "escalates to FABLE" verdict recorded on 2026-07-24 to **opus (Opus 5) with a -fresh-angle brief**; fable remains the step after Opus 5 also runs dry. The residuals those -rounds identified are unchanged — only the rung that attacks them next is (see -`escalation-backlog.md`). - -**Pre-tier rounds are version-unrecorded.** The 2026-05-19 / 2026-05-26 rounds tagged -`tier: n/a (pre-tier)` predate both the tier system and this legend; the model behind them -cannot be reconstructed, so they are *not* stamped. Consequence: **a pre-tier dry round cannot -count toward the "two consecutive dry versions" dormancy bar.** Any area whose -persistently-dry reputation leans on pre-tier rounds (areas 3 and 10 both do) has at most -*one* version-scoped dry verdict on record. - ---- - ## ⚠ Round records have moved to GitHub Discussions — this file is closed to new entries Every round is now one post in its area's Discussions category, `NN-`: From 74a08bccc36d8641d05ee8c6766fbe7ecb275a91 Mon Sep 17 00:00:00 2001 From: ms609-agent <313734811+ms609-agent@users.noreply.github.com> Date: Fri, 9 Oct 2026 13:52:17 +0100 Subject: [PATCH 19/22] ci: run coverage in its own arm job, off the Windows leg covr compiles its own instrumented build, so it cannot reuse R CMD check's install; on Windows it re-ran the whole test suite after the check had already run it, on the dearest runner of the core wave. Coverage now runs in a wave-2 `coverage` job on the arm runner the sense-check uses, alongside core, with its own concurrency group. The Windows leg keeps its tests but drops covr. The backstop wave waits on coverage as well as core, as it effectively did before. The fork has no Codecov token, so the job also prints the per-file summary and uploads the line tally as an artifact. Co-Authored-By: Claude Opus 5.5 --- .github/workflows/R-CMD-check.yml | 89 ++++++++++++++++++++++++++----- 1 file changed, 77 insertions(+), 12 deletions(-) diff --git a/.github/workflows/R-CMD-check.yml b/.github/workflows/R-CMD-check.yml index 42fe99ac3..63e1f291a 100644 --- a/.github/workflows/R-CMD-check.yml +++ b/.github/workflows/R-CMD-check.yml @@ -44,11 +44,12 @@ name: R-CMD-check # Waves: # sense-check -> arm (cheapest), always runs, never cancelled. Fast signal. -# core -> Windows (+codecov) / R 4.1 / R devel; only if sense-check passed. +# core -> Windows / R 4.1 / R devel; only if sense-check passed. +# coverage -> covr on arm (cheapest); alongside core. # shiny -> EasyTrees shinytest2 suite, Windows; only if the app changed # and sense-check passed. Non-blocking (continue-on-error) until # its stability on CI is established. -# full -> macOS x2 backstop; only once core is green. +# full -> macOS x2 backstop; only once core and coverage are green. jobs: # --------------------------------------------------------------------------- # Wave 1: cheapest leg as a fail-fast sense-check. No concurrency group, so a @@ -164,7 +165,7 @@ jobs: - '.github/workflows/R-CMD-check.yml' # --------------------------------------------------------------------------- - # Wave 2: Windows (+codecov), R 4.1, R devel. Runs only if the sense-check + # Wave 2: Windows, R 4.1, R devel. Runs only if the sense-check # passed. Per-leg concurrency so a superseding push cancels in-flight legs. # --------------------------------------------------------------------------- core: @@ -191,7 +192,6 @@ jobs: R_COMPILE_AND_INSTALL_PACKAGES: 'never' R_REMOTES_NO_ERRORS_FROM_WARNINGS: true RSPM: ${{ matrix.config.rspm }} - CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }} GITHUB_PAT: ${{ secrets.GITHUB_TOKEN }} steps: @@ -222,13 +222,12 @@ jobs: run: | sudo apt-get install texlive-latex-base texlive-fonts-recommended - - name: Set up R dependencies (covr) + - name: Set up R dependencies (Windows) uses: r-lib/actions/setup-r-dependencies@v2 if: runner.os == 'Windows' with: needs: | check - coverage # Coreset url:: -- this is the Windows leg, so it cannot use the Linux # tarball the `runner.os != 'Windows'` step below selects. Without a # url:: reference the solve fails outright ("Can't find package called @@ -270,12 +269,78 @@ jobs: - name: Check package uses: r-lib/actions/check-r-package@v2 + - name: Notify on failure + if: failure() && github.event_name == 'schedule' + uses: actions/github-script@v7 + with: + github-token: ${{ secrets.GITHUB_TOKEN }} + script: | + github.rest.issues.createComment({ + owner: context.repo.owner, + repo: context.repo.repo, + issue_number: 186, + body: 'Scheduled workflow has failed: https://github.com/${{ github.repository }}/actions/runs/${{ github.run_id }}' + }); + + # --------------------------------------------------------------------------- + # Wave 2, alongside core: code coverage on the cheapest runner. covr compiles + # its own instrumented build and runs the whole suite, so it is a job of its + # own rather than a step after some leg's R CMD check. Dependency setup + # mirrors the sense-check: same pinned snapshot, same prebuilt Coreset. + # --------------------------------------------------------------------------- + coverage: + needs: sense-check + runs-on: ubuntu-24.04-arm + name: coverage (ubuntu-24.04-arm, release) + + concurrency: + group: ${{ github.workflow }}-coverage-${{ github.ref }} + cancel-in-progress: true + + env: + R_COMPILE_AND_INSTALL_PACKAGES: 'never' + R_REMOTES_NO_ERRORS_FROM_WARNINGS: true + RSPM: "https://packagemanager.posit.co/cran/__linux__/noble/2026-07-30" + CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }} + GITHUB_PAT: ${{ secrets.GITHUB_TOKEN }} + + steps: + - name: Checkout git repo + uses: actions/checkout@v6 + + - name: Set up R + uses: r-lib/actions/setup-r@v2 + with: + r-version: release + + - name: Set up R dependencies + uses: r-lib/actions/setup-r-dependencies@v2 + with: + needs: | + coverage + extra-packages: | + shinytest2=?ignore + url::https://ms609.github.io/packages/bin/linux/aarch64-release/Coreset_latest.tar.gz + - name: Code coverage - if: runner.os == 'Windows' + # quiet = FALSE streams the instrumented test run to the job log, so a + # test that fails only under covr shows its failure, not a temp path. + # The per-file summary and line tally keep the result readable where + # the Codecov upload has no token. run: | - covr::codecov() + cov <- covr::package_coverage(quiet = FALSE) + print(cov) + saveRDS(covr::tally_coverage(cov, by = "line"), "coverage-lines.rds") + covr::codecov(coverage = cov) shell: Rscript {0} + - name: Upload line coverage + uses: actions/upload-artifact@v4 + with: + name: coverage-lines + path: coverage-lines.rds + retention-days: 14 + - name: Notify on failure if: failure() && github.event_name == 'schedule' uses: actions/github-script@v7 @@ -354,12 +419,12 @@ jobs: retention-days: 14 # --------------------------------------------------------------------------- - # Wave 3 (backstop): the expensive macOS legs. Only runs once the core wave is - # green, which in practice is "about to merge". macOS is ~10x runner-minutes, - # so keeping it off every push is the big save. + # Wave 3 (backstop): the expensive macOS legs. Only runs once the core wave + # and coverage are green, which in practice is "about to merge". macOS is + # ~10x runner-minutes, so keeping it off every push is the big save. # --------------------------------------------------------------------------- full: - needs: core + needs: [core, coverage] runs-on: ${{ matrix.config.os }} name: ${{ matrix.config.os }} (${{ matrix.config.r }}) From 754ba047a87684c11ed2058ce096b54d0b3ed031 Mon Sep 17 00:00:00 2001 From: ms609-agent <313734811+ms609-agent@users.noreply.github.com> Date: Fri, 9 Oct 2026 14:00:35 +0100 Subject: [PATCH 20/22] fix: restore the comma after SmithConc's doi in REFERENCES.bib b85baa6ea added the doi field without a trailing comma, so pandoc rejects the file ("line 685, column 3: unexpected 'y'") and custom.Rmd fails to build. R CMD check has failed at "creating vignettes" on every run since. Co-Authored-By: Claude Opus 5.5 --- inst/REFERENCES.bib | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/inst/REFERENCES.bib b/inst/REFERENCES.bib index d6f11ca71..89689b092 100644 --- a/inst/REFERENCES.bib +++ b/inst/REFERENCES.bib @@ -681,7 +681,7 @@ @article{SmithConc author = {Smith, Martin R.}, journal = {Systematic Biology}, pages = {online ahead of print}, - doi = {10.1093/sysbio/syag071} + doi = {10.1093/sysbio/syag071}, year = {2026} } From 6d72d3aa01c152febbbef03ae0cd36cb0fe15cf4 Mon Sep 17 00:00:00 2001 From: ms609-agent <313734811+ms609-agent@users.noreply.github.com> Date: Wed, 14 Oct 2026 08:31:03 +0100 Subject: [PATCH 21/22] guard POPCNT: popcount64() via TreeTools::count_bits() popcount64() in src/ts_data.h executed the x86 POPCNT instruction through inline asm on every x86-64 build. POPCNT is not baseline x86-64, so CPUs without it (e.g. QEMU's default qemu64) raise SIGILL (ms609/TreeTools#287). popcount64() keeps its signature, so its 94 call sites are unchanged, and now calls TreeTools::count_bits(), which uses POPCNT only once a load-time CPUID check has confirmed it, and the portable builtin otherwise. Requires TreeTools > 2.4.1 (LinkingTo and Imports); ts_data.h stops with an #error against older headers. Remotes: ms609/TreeTools lets CI build until that release reaches CRAN. Checked with mingw-w64 GCC 16.2 -O2: all 34 src files compile with no new warnings; 45 functions contain POPCNT, all now behind the has_popcnt check (previously all 45 unguarded). --- DESCRIPTION | 6 ++++-- NEWS.md | 4 ++++ src/ts_data.h | 27 ++++++++++----------------- 3 files changed, 18 insertions(+), 19 deletions(-) diff --git a/DESCRIPTION b/DESCRIPTION index 43ab88b4b..40d5671d7 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -51,7 +51,7 @@ Imports: Rdpack (>= 0.7), stats, TreeDist (>= 2.6.3), - TreeTools (>= 2.4.1), + TreeTools (> 2.4.1), utils, Suggests: cluster, @@ -144,7 +144,9 @@ Collate: RdMacros: Rdpack LinkingTo: Rcpp, - TreeTools, + TreeTools (> 2.4.1), +Remotes: + ms609/TreeTools, SystemRequirements: C++17 LazyData: true ByteCompile: true diff --git a/NEWS.md b/NEWS.md index 1a7a2bb91..8b3d64e91 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,5 +1,9 @@ # To integrate into 2.0.0 notes +- C++ code no longer executes the x86 POPCNT instruction on CPUs that lack + it, which crashed R with "illegal operation" on some virtual machines + (ms609/TreeTools#287). Requires TreeTools > 2.4.1. + - New `Bremer()` calculates Bremer (decay) support for each clade of a reference tree. diff --git a/src/ts_data.h b/src/ts_data.h index 89e0d76c7..1ee7fe057 100644 --- a/src/ts_data.h +++ b/src/ts_data.h @@ -14,27 +14,20 @@ #include #include #include +#include /* for count_bits() */ + +#ifndef TREETOOLS_POPCOUNT_DISPATCH +#error "TreeSearch needs TreeTools > 2.4.1, whose count_bits() checks for POPCNT" +#endif namespace ts { -// Hardware POPCNT via inline asm (no -mpopcnt flag needed). -// Same approach as TreeDist::popcnt64 — emits the instruction directly, -// avoiding the software Hamming weight fallback that __builtin_popcountll -// compiles to without -mpopcnt. +// Population count: POPCNT where the CPU has it, checked once at load time, +// otherwise the portable builtin. Executing POPCNT unconditionally (as the +// inline asm here used to) raises SIGILL on x86-64 CPUs without it, such as +// QEMU's default qemu64 model (ms609/TreeTools#287). inline int popcount64(uint64_t x) { -#if (defined(__GNUC__) || defined(__clang__)) && defined(__x86_64__) - uint64_t result; - __asm__ ("popcnt %1, %0" : "=r" (result) : "r" (x)); - return static_cast(result); -#elif defined(_MSC_VER) && defined(_M_X64) - return static_cast(__popcnt64(x)); -#else - // Fallback: software Hamming weight (non-x86-64 platforms) - x = x - ((x >> 1) & 0x5555555555555555ULL); - x = (x & 0x3333333333333333ULL) + ((x >> 2) & 0x3333333333333333ULL); - return static_cast( - (((x + (x >> 4)) & 0x0F0F0F0F0F0F0F0FULL) * 0x0101010101010101ULL) >> 56); -#endif + return static_cast(TreeTools::count_bits(x)); } // Portable count-trailing-zeros for uint64_t (undefined for x == 0) From c9186416fc7437d10258c7d9b0855787b2d599f3 Mon Sep 17 00:00:00 2001 From: ms609-agent <313734811+ms609-agent@users.noreply.github.com> Date: Wed, 14 Oct 2026 09:46:18 +0100 Subject: [PATCH 22/22] spelling test: report words found only in NEWS.md, don't fail on them NEWS.md routinely names new technical terms, which made the spelling test fail until they were added to inst/WORDLIST. tests/spelling.R now runs spelling::spell_check_package() itself (same source-directory lookup and NOT_CRAN skip as spell_check_test()), prints NEWS-only words, and stops only for words found elsewhere. The old covr warning-muffling is no longer needed: a missing source directory now skips quietly. --- tests/spelling.R | 43 +++++++++++++++++++++++++++++++------------ 1 file changed, 31 insertions(+), 12 deletions(-) diff --git a/tests/spelling.R b/tests/spelling.R index 40e52b370..3456c565a 100644 --- a/tests/spelling.R +++ b/tests/spelling.R @@ -1,14 +1,33 @@ -if (requireNamespace("spelling", quietly = TRUE)) { - # spell_check_test() warns "Failed to find package source directory" when run - # from covr's temp install path; that warning is harmless, so catch and - # suppress it rather than letting it become an error under error-on="warning". - withCallingHandlers( - spelling::spell_check_test(vignettes = TRUE, error = TRUE, - skip_on_cran = TRUE), - warning = function(w) { - if (grepl("find package source", conditionMessage(w), fixed = FALSE)) { - invokeRestart("muffleWarning") - } +# As spelling::spell_check_test(vignettes = TRUE, error = TRUE), except that +# words found only in NEWS.md, which routinely names new technical terms, are +# reported without failing the check. Add them to inst/WORDLIST at release. +notCran <- tolower(Sys.getenv("NOT_CRAN")) %in% c("1", "yes", "true") +if (notCran && requireNamespace("spelling", quietly = TRUE)) { + pkgDir <- list.files("../00_pkg_src", full.names = TRUE) + if (!length(pkgDir)) { + checkDir <- dirname(getwd()) + if (grepl("\\.Rcheck$", checkDir)) { + sourceDir <- sub("\\.Rcheck$", "", checkDir) + if (file.exists(sourceDir)) pkgDir <- sourceDir } - ) + } + if (!length(pkgDir) && basename(getwd()) == "tests" && + file.exists("../DESCRIPTION")) { + pkgDir <- dirname(getwd()) + } + # Package source is not found from covr's temporary install; skip quietly. + if (length(pkgDir)) { + results <- spelling::spell_check_package(pkgDir[1], vignettes = TRUE) + newsOnly <- vapply(results[["found"]], + function(where) all(startsWith(where, "NEWS.md:")), + logical(1)) + if (any(newsOnly)) { + cat("Words only in NEWS.md (not an error):", + paste(results[["word"]][newsOnly], collapse = ", "), "\n") + } + if (any(!newsOnly)) { + stop("Potential spelling errors: ", + paste(results[["word"]][!newsOnly], collapse = ", "), call. = FALSE) + } + } }