red-team area 15: two paired passes over the legacy pure-R search API, plus two inline fixes

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) {
@@ -59,8 +59,8 @@ EdgeListSearch <- function (edgeList, dataset,
   }
   hits <- 0L
   unimprovedSince <- 0L
-  
-  for (iter in 1:maxIter) {
+
+  for (iter in seq_len(maxIter)) {
     candidateLists <- RearrangeEdges(edgeList[[1]], edgeList[[2]], 
                                      dataset = dataset, 
                                      TreeScorer = TreeScorer,
@@ -107,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 "")
+            # `for` leaves the loop variable NULL when maxIter < 1
+            if (maxIter > 0L) iter else 0L,
+            " rearrangements.", if (verbosity > 1L) "\n" else "")
   } #nocov end
 
   edgeList[3:4] <- c(bestScore, hits)

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

dev/red-team/focus-areas.md

@@ -142,46 +142,41 @@ 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 — sonnet, UNMEASURED / no inherited maturity.** 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.
-  **`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.
+- **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.

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
@@ -60,6 +56,40 @@ 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
+
+Every round is now one post in its area's Discussions category, `NN-<area-name>`:
+
+<https://github.com/agent-issues/TreeSearch/discussions>
+
+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 <date> - area <N> - <rung> (<Version>) - yield <n>`; first line of the body
+`<rung> (<Version>) | effort: <effort> | <date>`.
+
+**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
+are frozen, not retired.
+
+---
+
 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 +1406,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

tests/testthat/test-CustomSearch.R

@@ -108,3 +108,25 @@ 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
+
+  # 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")
+})