Update from agent-issues - #284
Merged
Merged
Conversation
Zero-length-branch collapse / MPT set, revisited at the same rung with a fresh agent (seam recorded still yielding). Reviewed the four fixes that landed inside this area since 2026-07-02 as new code, and followed the collapse output to its consumers. Filed: T-400, T-402 (P1); T-403, T-404 (P2); T-401, T-405, T-408, T-410, T-411 (P3). Refuted: T-406 (exhaustive enumeration, 535 matrices, 0 hits), T-407. T-324 amended — its nni_perturb repair claim is over-optimistic in the score-better-than-constrained regime. T-400 is the headline and is not in this area's files: TreeLength() on a multifurcating tree does OOB heap writes and returns a score read from before the buffer, reachable from MaximizeParsimony()'s default output. CharacterLength(), TreeScore() and EdgeListScore() share the exposure. Comment-only fixes applied inline (no behaviour change): the _aggressive doc block claimed the criterion was final_[p] & final_[c], contradicting the implementation; two stale invariant claims broken by T-331's star branch (ts_collapsed.cpp:289, ts_rcpp.cpp:222 = T-409, fixed not filed). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Open findings now live as GitHub issues in the agent-issues fork, labelled red-team + sev:high|med|low + area:N. Status becomes GitHub state, so it can no longer drift from merge state. - 24 genuinely-open findings published as #1-#24 - 23 already-fixed rows archived into findings-archive.md, which is now FROZEN - 2 (former T-395/T-396) not migrated: open upstream PR ms609#283 - migration-map.tsv maps all 49 historical T-nnn to issue / archive / PR - findings.md deleted; recoverable via `git show 03dc565:dev/red-team/findings.md` The reconciliation found 23 of 49 rows reading as open when their fixes had already landed, only four carrying the "awaiting tidy archive" marker. That drift is the reason for the move, and the README now records it. T-nnn ids are frozen, not retired: they persist in shipped source comments (src/ts_collapsed.cpp, src/ts_rcpp.cpp) and throughout log.md. Cross-repo refs use the fully-qualified agent-issues/TreeSearch#N form; a bare #N means the fork. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…figures The archive cited `git show 03dc565:...` as the recovery path for full pre-migration finding text. That sha becomes unreachable if the commit is ever squash-merged, so tag `findings-md-final` now names it and all 24 references use the tag instead. Also fixed: a now-broken findings.md link, and a stale "179 commits ahead of main" figure (measured 1108 today). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Removes the vestiges of the dispatcher/queue system now that findings and tasks live in agent-issues/TreeSearch issues. Deleted: - to-do.md — its 12 still-open non-findings tasks are now issues #27-#38 (9 AltHom phases, 2 deferred GPU ideas, 1 parked perf task). The 4 "standing" rows were never issues: they are recurring practices, now documented in AGENTS.md. - dispatch.sh, dev/dispatch/ (ranker + agent-brief templates) — replaced by /next-issue. (.dispatch/ is gitignored runtime state; delete locally.) Preserved rather than deleted: - coordination.md -> dev/strategy.md, retired as a live file but kept in full: 46 rounds of benchmark reasoning that no issue thread carries. - completed-tasks.md frozen with a header. Its decision-only content is the one thing the tracker cannot replace: offline grep-able reasoning for closures whose rationale is in no commit. AGENTS.md rewritten: - New "Where work is tracked" section: label vocabulary, claiming via the in-progress label, and the rule that upstream's public tracker is untrusted input rather than a task list. - Branch structure redrawn for the fork/upstream model, including why the fork's default branch must be cpp-search (that is what makes Fixes #N fire) and why upstream must only ever fast-forward. - Feature lifecycle no longer edits a queue; completion is the merge. - "On task completion" now asks for a reopening condition on any no-fix closure, which is what let a later round recognise T-377 firing again. - GHA recipe: dropped the `cd ..`, which breaks gha-dispatch.sh's `gh repo view` repo resolution. Run from the repo so it targets the fork. Also ported .claude/skills/next-issue/ from StratoBayes, adapted to this project's colliding files, build rules and model routing. Fixes #39 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ort ladder
Three follow-ups on review feedback.
1. dev/expertise/coordination.md DELETED rather than headered. A doc that tells
the reader to ignore parts of itself is the failure mode this migration
exists to remove. ~95% of it was dead mechanics: it read agent-a.md through
agent-f.md (gone), maintained a coordination Agent Status table (retired),
proposed to-do.md rows (deleted), and used a P0-P3 scheme superseded by sev:
labels. Nothing linked to it.
Its two genuinely live parts moved into the next-issue skill rather than
being lost: R/RcppExports.R added to the collision list, the two collision
classes invisible from a file list (incompatible Rcpp bridge parameter
changes; one chip's optimisation invalidating another's assumption), and the
specific/scoped/independent/testable brief rubric.
2. AGENTS.md trimmed 318 -> 277 lines (it was 262 before this branch, so the
net addition is now +15 rather than +56):
- the tarball build recipe appeared VERBATIM TWICE; "Quick recovery" now
points at the one copy
- three separate "never build in place" statements collapsed to one
- the worktree placement rule was stated three times; now once, merged with
the claiming convention into a single "Worktrees" section
- two "shared files" sections merged
- init.c arg-count section folded into the Mandatory checks it duplicated
- my own "Where work is tracked", Rules, completion and blocked-on prose
tightened
3. next-issue effort ladder completed with xhigh and max, which the enum
supports and the skill could not reach. Also fixes a conflation: effort was
defined in wall-clock/file-count terms (a size estimate) while the tooling's
effort is reasoning depth. Size is now stated separately, and the doc records
that spawn_task takes no model/effort parameter, so the recommendation is
advisory.
Also dated dev/strategy.md's "Project State" heading, which was undated and
asserted "All planned development objectives are complete" — four months stale
and reading as current.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Three `#N` citations illustrated rules with that week's tracker state. Each made the reader open an issue to get the point, and each would have become meaningless once the issue closed. Rules now stand on their own. Also cut the duplicated AGENTS.md build/worktree list to a pointer — it changes there, not here — and compressed the collision list, the intro and the effort ladder. 131 -> 111 lines. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Retire file-based coordination for the issue tracker
T-300's SPR half landed long ago (f531bbc EW, 014ccde NA); what issue #38 still describes as "full_rescore after every accepted move" is now only the residue: accepts that rerooted the clipped fragment, which still fell through to full_rescore. apply_tbr_move reverses the parent/child links along clip_node..reroot_parent, so every node on that path gains new children — and after the reversal that path IS clip_node's rootward chain. One extra dirty seed at clip_node therefore covers the rerooting case exactly, with no new machinery: the NA kernel already took a third seed for precisely this region (it backs exact_verify_sweep), so this only adds the same parameter to the EW/IW kernel and drops the is_spr gate at the accept site. Measured, and the honest answer is that the lever is spent. Post-T-300 the whole accept branch is 0.18-0.51% of tbr_search wall (in-DLL na_t_accept_ms, Vinther2008/Agnarsson2004/Zanol2014 x EW,IW, 15 paired cells). This cuts that slice to x0.901 median (12/15 cells) => ~0.03% end-to-end, i.e. undetectable. Rerooting accepts are ~18% of accepts, so that is a real ceiling, not thin coverage. The case for merging is codepath unification, not speed; area 4 is recorded AT-LIMIT so it is not profiled again. Correctness, given an earlier incremental attempt shipped a systematic delta=-3 (b7303ee): - TS_TBR_ACCEPTCHK, a new env-gated in-flight oracle, cross-checks every incremental accept against full_rescore and aborts on drift. Clean over the 70-cell battery (1322 accepts) and over 624 assertions spanning the constrained, pool, IW, XPIWE, sector and drift paths. - Scores and accept counts are byte-identical to the pre-patch library across all 15 A/B cells and the 70-cell battery, so trajectories are unchanged. - n_reroot_accepts (src/ts_data.h) makes the new regression test non-vacuous: the four pre-existing tests assert score identity but cannot tell whether the rerooting arm was ever entered, so the new one asserts coverage as well as correctness. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…age diff run Rotation (11 mod 13)+1 = 12, sonnet (last visit yielded). Filed #41 (routing state is tier-only) and #42 (scope rows never diffed against the file tree). The scope-coverage diff flagged on 2026-07-03 and never run is now run: two subsystems, 5553 + 2183 loc, in no area at all. 17 files added to rows 1/2/4/5/6 inline, area 7 extended to the Shiny app tests, area 8 broadened from test-ts-*.R to tests/testthat/*.R (it excluded ~44 of ~110 files). Every addition annotated UNMEASURED, per this area's own lesson that a grown scope row inherits no dry verdict. Also corrected: README severity breakdown had drifted within the round it was written (6/5/13 vs a true 6/4/14) — replaced with a query; the undocumented area:N label dependency; a missing migration-map-todo.tsv row; and the "NEVER REVIEWED" rationales on areas 12 and 13, both stale. src/rearrange.cpp: header claimed "DEAD CODE ... Safe to remove" and was false. The Rcpp::export tags are commented out but RcppExports.cpp and TreeSearch-init.c were never regenerated, so nni/spr/spr_moves stay registered, bound at R/RcppExports.R:60-68 and exercised by three test files. Acting on the header breaks them. Comment corrected; no code change. Reverted a finder action: needs-escalation had been applied to #18/#19 for a backlog row whose ask is sequencing, not tier. Area 13 is already opus, and a label hit makes step 3 skip reading the row that holds the ask. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The Windows job of agent-check.yml, and R-CMD-check.yml's windows-latest
(release) leg, failed on every branch in "Set up R dependencies", before
compiling anything:
! Could not solve package dependencies:
* deps::.: Can't install dependency MaxMin
* MaxMin: Can't find package called MaxMin.
Not a publishing problem. MaxMin is published, and DESCRIPTION:78 already
carries Additional_repositories -- but pak does not read that field, as
ms609/packages' tools/publish-maxmin.R records from a direct 2026-08-03 test.
That is exactly why the publish script also emits flat, unversioned
MaxMin_latest.<ext> aliases for url:: consumers. The Linux legs already name
theirs; the Windows and macOS legs named nothing.
Fixed, per platform, using the alias so it never needs updating when MaxMin
moves. All three URLs verified live, and all three artifacts are Built: R 4.6.1
-- the current R release, which is what these legs request.
Verified failing, now fixed:
- agent-check.yml, windows leg (had no extra-packages block at all)
- R-CMD-check.yml, "Set up R dependencies (covr)", runner.os == 'Windows'
Fixed pre-emptively, NOT observed failing -- both were skipped in the
diagnosing run, and fixing the core Windows leg unskips the macOS matrix, which
would then fail identically:
- R-CMD-check.yml, EasyTrees shinytest2 (gated on detect app changes)
- R-CMD-check.yml, full matrix (macOS-latest arm64, macos-15-intel x86_64);
_R_CHECK_FORCE_SUGGESTS_: false does not help, it governs R CMD check
rather than pak's solve
Fixes #43
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…by name
The url:: reference alone was not enough. It worked -- the run logged
"Installed MaxMin 0.0.0.9004" -- and the step then failed anyway, a few seconds
later, with a DIFFERENT ref:
* local::.: Can't install dependency MaxMin
setup-r-dependencies' "Check whether pandoc needs to be installed" step runs
pak::pkg_deps(".", dependencies = list(direct = "all", indirect = character()))
which is a second, fresh solve. It sees neither the lockfile just built nor
extra-packages, so it looks MaxMin up by name in the configured repos -- where
it is not, since pak ignores Additional_repositories -- and dies, despite
MaxMin being installed moments earlier.
That branch only runs when `Sys.which("pandoc") == ""`. Linux runners ship
pandoc, so the Linux legs return early and never reach the solve; Windows and
macOS runners do not, which is the whole reason this was a non-Linux-only
failure. Setting install-pandoc explicitly skips the auto-detect entirely.
Values chosen per leg rather than uniformly: true where the job checks with
vignettes (agent-check windows, R-CMD-check core covr, macOS matrix), false for
the shinytest2 job, which runs an app and no vignettes.
The quarto auto-detect is safe by comparison -- it globs for *.qmd and never
calls pkg_deps -- so it needs no equivalent pin.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Three guard clauses at the HSJ/XFORM scoring bridge: - unpack_hsj() (T-398/#14): a present-but-NULL hsjTipLabels passed the containsElementNamed() check but was then skipped, leaving scoring_mode = HSJ with an empty ds.tip_labels -- segfaulting score_hierarchy_block() later. Now requires hsjTipLabels non-NULL whenever hsjConfig is supplied, and (found during review) that it covers every hierarchy block's primary/secondary character index. - unpack_xform() (T-397/#13): ported ts_sankoff_test()'s cost-matrix dimension check and added three more guards for holes that function didn't have either: combo_grid row count, tip_sec_known dimensions (found during review), and out-of-range tip_states. Previously a mis-shaped-but-same-length matrix read garbage with no warning at all (Rcpp's Matrix::operator() bounds-checks only the linear offset). - ts_collapsed.cpp collapse guards (T-408/#22): keyed on scoring_mode alone, disabling branch collapse for an HSJ/XFORM config with no actual hierarchy data -- a case collapse is provably safe for. Now gates on hierarchy-data presence, matching DataSet::topology_independent()'s predicate. Verified the loosening is strictly confined to the no-data case: every config with real hierarchy_blocks/sankoff_n_chars still gets all-zero flags exactly as before, so the sibling #17 chip's reliance on this guard's current behaviour for non-degenerate configs is untouched. Reviewed by an independent external-reviewer pass, which confirmed the three primary guards and surfaced two adjacent gaps in the same functions (hsjTipLabels shape, tip_sec_known shape), now closed alongside them. Fixes #14 Fixes #13 Fixes #22 Pre-fix failures (confirmed by reverting the src/ changes and rebuilding): - T-398 standalone segfault: exit code 139 (SIGSEGV) running the compat wrapper with hierarchyBlocks set and hsjTipLabels left at its default. - test-ts-hsj-xform-guards.R (T-397/T-408 portion, T-398 test excluded since a segfault kills the testthat process): FAIL 4 | PASS 2 -- three xform guards did not error, and the empty-hierarchy HSJ config collapsed to 10 edges instead of 8 (matching the plain-EW baseline). Post-fix: full guard suite plus related HSJ/XFORM/collapse/resample suites all pass (509/509), verified via a temp-library tarball install per AGENTS.md, not devtools::load_all().
…lapse boundaries Three independent holes let a `constraint` stop binding the trees the caller is handed. A start tree supplied through `tree =` was never checked against the constraint (T-402). Constrained rearrangement cannot climb out of a violating tree -- an unmapped split makes every candidate regraft illegal -- so the replicate froze on it and reported its unconstrained score, which then evicted the compliant trees the other replicates found. The repair has to happen before anything takes the start's score as a baseline: a violating tree is drawn from a wider set of topologies and so scores better, making the legal repair look like a regression to any later accept test. `run_single_replicate` now repairs any violating start, whatever its source, and falls back to a constrained Wagner build where the heuristic repair does not take. `MaximizeParsimony()` warns when it was the caller's `tree` that conflicted. The per-replicate pool capture had no constraint gate, asymmetrically to the fuse capture beside it (T-324). All three capture sites -- interrupted and normal in the serial driver, and the parallel worker's -- now go through `capture_satisfies_constraint()`, which tests with `violates_constraint_posthoc` rather than the fuse's `constraint_node < 0`: a tree can map every constraint node and still fail the full Fitch check. Discards are counted and reported from the main thread. With the pool gated, an empty pool under a constraint is an error rather than a fall back to the unvalidated start tree. The collapse pass protected only a node whose tip set was the 1 group exactly (T-403). Tips coded `?` for a constraint character are free to sit on either side, so the split is often realised by a wider node -- left collapsible, and contracted away under the default `collapse = TRUE`. `consZero` is now plumbed through `.PrepareConstraint()` to the kernel, which additionally protects the MRCA of either group when it excludes the other. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The ../gha-dispatch.sh relative path only resolves from the main checkout; it fails with "No such file or directory" from a ../worktrees/TreeSearch/<name> worktree, where feature work is supposed to happen per this same file. Hard-code the scripts' known absolute location instead.
fix: validate presence and shape at the HSJ/XFORM bridge
The Distribution baseline recorded
trees <- WideSample(trees[1:125], 48)
for a state its own test sets to c(77, 125) two lines earlier, and the
value flipped between machines: the dev box recorded 1:125, the
windows-latest runner produced 77:125.
Neither is what the test asks for. `set_inputs(data-dataSource =
"Sun2018")` does not finish loading inside its default 4 s wait -- the run
log says so outright, "Server did not update any output values within 4
seconds" -- so the `data-treeRange` set on the next line lands while the
dataset is still loading. When the load completes, UpdateAllTrees() sees
the tree count change and calls UpdateTreeRange(c(1L, nTrees)), resetting
the range to the full span and discarding the c(77, 125) the test just
asked for. Whether that reset landed before or after the download was
captured is what varied by machine.
Waiting for the load to settle before touching treeRange makes the
ordering deterministic: the reset happens first, then the test's range is
applied and honoured.
Also make wait_stable() cover the debounce window. mod_data's nTree /
treeRange watchers are debounce()d (aJiffy = 42 ms, typingJiffy = 105 ms),
and a pending debounce timer does not make Shiny busy -- there is nothing
to recompute until it expires, so wait_for_idle() can return before the
watcher has seen the value just set. With the ordering fixed this is
load-bearing rather than defensive: the c(77, 125) thinning is triggered
through that debounced watcher, and set_inputs() cannot be relied on to
wait for it (its own 4 s wait expires with "did not update any output
values" when a set produces no output change).
The 001 baseline is provisionally the windows-latest runner's output for
the 77:125 state, from run 30919105631's uploaded .new artifact. The
input line is now correct by construction, but the clustering conclusion
and rogue-tip lists downstream of it depend on the sampled subset and so
on the RNG state, which the corrected ordering shifts -- to be confirmed
against CI, whose job uploads .new artifacts for exactly this.
Fixes #46
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
docs: fix GHA dispatch recipe for worktree use
…te tip_ambiguous plane-0 behaviour
AdditionTree() built searchArgs without min_steps, so a finite `concavity`
scored ts_wagner_tree's result against min_steps = 0 instead of the
dataset's real per-character minimum step counts (MinimumLength()). This
corrupted only the internal `result$score`, which AdditionTree() itself
discards, and never moved the returned topology -- placement uses an
equal-weights Fitch proxy regardless of concavity/min_steps.
The equal-weights-proxy placement contract was already documented on
@param concavity in a prior commit; this just fixes the remaining
min_steps gap and adds regression coverage: a mock-capture test pinning
that AdditionTree() forwards real min_steps to ts_wagner_tree, a direct
ts_wagner_tree comparison showing min_steps changes the score but not the
edge set, and a characterisation test locking in that numeric concavity
never moves AdditionTree()'s topology.
Also records, as a comment only, that wagner_goloboff_scores()'s
tip_ambiguous computation ANDs across all n_states including the NA
plane, so a tip coded {0,1} -- every applicable state but not "-" -- is
scored non-ambiguous (informative) despite carrying no information about
which applicable state holds. Left as-is: changing it shifts start trees
and needs an A/B (T-371).
Fixes #5
Fixes #6
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
branches: ["**"]
TreeState::init_from_edge derives n_tip, n_internal and n_node from the edge count alone, which identifies a tree only when it is binary. A multifurcating edge list broke that derivation in both parities: an odd edge count left the topology arrays one element short, so parent[] and left[]/right[] were written out of bounds, while an even one kept the indices in range but rooted the tree on a real tip, leaving a one-element postorder whose downpass read prelim.data() - total_words. Either way the caller got a plausible number instead of an error, and the number varied between identical calls; a polytomous startEdge segfaulted. Refuse the edge list at that boundary rather than at each of the R entry points that reach it: edge_list_is_binary() checks the shape from the edge arrays alone, before anything is written, and init_from_edge throws for Rcpp to forward. build_topology_tree() clones the same derivation for the least-squares path and gets the same check. ts_driven_search() screens start trees with the predicate on the main thread, since an uncaught throw on a parallel worker would terminate the session. TreeLength.list(), .CheckTreeCharLen(), TreeScore() and EdgeListScore() gain the R-level check so the message matches the one TreeLength.phylo() has always given. FastCharacterLength() is left unchecked, as documented; the kernel now gives it the same message. The Shiny app scores every tree it displays, so it now searches with collapse = FALSE. Also extends the T-261 zeroing audit in reset_states() to name the collapse kernels, whose whole-row memcmps read words no pass writes, and to name every path that zero-fills the state arrays rather than only init_from_edge. The T-382 one-sidedness comment keeps its original reasoning for prelim -- a tip sibling always carries real states, so a ratchet-zeroed block makes equality harder -- and gains the down2 / subtree_actives case, where the words really are always zero. Fixes #16 Fixes #24 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ps tests External review of the prior commit found the roxygen text factually wrong: AdditionTree() never returns a $score (it discards result$score), so "affects only the reported $score" is false -- correct it to state plainly that numeric concavity has no effect on AdditionTree()'s output. Also addresses two test gaps the review flagged: the first new test exercised ts_wagner_tree() directly rather than AdditionTree() (retitled to make that explicit, and its equality assertion tightened to a directional/finiteness check so a NaN or sign error couldn't pass), and two cheap regression tests were missing: that concavity = Inf still forwards min_steps = integer(0), and that concavity = "profile" still runs cleanly now min_steps is computed post-recoding. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
`Resample(constraint =, nReplicates > 1)` errored: `R/Resample.R:372` splats `.PrepareConstraint()`'s whole list into `ts_parallel_resample()`, which has no `consZero` formal. It was the one splat site of four not filtered, and no test covered a constrained `Resample()` at all; a test now asserts the filter against every flat kernel's formals. The collapse protection over-reached. It protected the MRCA of a group unconditionally, so an unsupported branch that merely happened to be the narrowest node containing the 0 group was returned resolved -- the "unsupported non-constraint branches still collapse" half of the promise, and a shift in `n_topologies`. It now protects only when no realising edge survives the contraction on its own. A discarded replicate no longer feeds the strategy bandit, the Chao1 coverage scores or the replicate report: a violating tree scores better than any legal one, so those would be credited to whatever produced it. Its stopping rules still run -- skipping them would outlive the deadline and swallow an interrupt. An interrupted replicate is now tested but not repaired, since `impose_constraint()` has no interrupt check of its own. The constraint check short-circuits on the locked-node mapping, which is cheaper than the post-hoc Fitch check and strictly stronger, so only an unmapped split pays for the latter. Constrained wall-clock on Vinther2008 (30 paired seeds) goes from a 1.12 median ratio, 11 seeds >10% slower, to 1.003 with none >10%. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
ci: fix the Windows/macOS MaxMin resolution failure that reddened every branch
test(EasyTrees): let the dataset load settle before setting treeRange
Self-review of the two commits before it. A constraint is enforced as a split throughout -- the locked-node filter screens rearrangements on it, and impose_constraint() repairs to it and nothing else -- so a character with a third state has taxa nothing constrains. The capture gate was judging that same tree by the stricter full-Fitch reading, which is not a standard the search can reach: a probe on a three-state constraint discarded all four replicates and errored with an empty pool, where a partial answer existed. The gate now follows the mapping, and `.PrepareConstraint()` warns at input that an intermediate state is unconstrained, rather than leaving the caller to infer from `@param constraint` that it is not. `constraint_satisfied()` refreshes the DFS timestamps alongside the node ids. map_constraint_nodes() alone left the two out of step, and spr_search() reads both without re-mapping, so the `sprFirst = TRUE` warmup could classify a regraft against this tree's nodes and another tree's timestamps. A start whose repair fails falls back to a Wagner build, which is exactly the constructor that can exhaust its reshuffles and return a violating tree; it is now repaired rather than trusted. Also: the collapse protection's group sizes are counted once instead of per tree; the kernel-formals test now asserts that no constraint field is dropped, not just that none is unknown; `.ConstraintViolated()` indexes nodes by column so its accumulation runs down a column-major matrix rather than across it. Constrained wall-clock on Vinther2008 (20 paired seeds) is 0.95 median against `cpp-search`, scores identical. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
red-team: area 12 round 2026-08-04 — scope-coverage diff, routing-channel finding, rearrange.cpp header correction
fitch_label_char()'s uppass loop formed `&co.kids[co.kidOff[node]]` before testing `nk`, the node's child count. CanonOrder stores children CSR-style, so `kidOff[n]` for a childless node is whatever `kids.size()` happened to be when the DFS popped it -- and for the LAST node popped that is the final size, every other node having already contributed its children by then. `co` arrives as a const reference, so this is `std::vector<int>::operator[](size()) const`: a dereference of one past the end. The downpass and the tie-break accumulation loops above both already `continue` on `nk == 0`; this loop did not. Reproduced against `-D_GLIBCXX_ASSERTIONS` (flag in PKG_CPPFLAGS, since ~/.R/Makevars.win zeroes PKG_CXXFLAGS; 34 hits in the build log). Pre-fix, four test files abort on entry to their first HSJ block with `Assertion '__n < this->size()' failed`; post-fix all four run clean: test-tree_length.R abort -> 77 passed test-ts-xform.R abort -> 138 passed test-ts-hsj.R abort -> 147 passed test-ts-resample-hierarchy.R abort -> 74 passed The reporter's second reproducer (test-ts-xform.R) is the same defect, not a second one: it aborts inside the third test, which is the file's first `inapplicable = "hsj"` search -- hence exactly five assertions first, from the two preceding pure-xform tests. `kid` is never dereferenced when `nk == 0`, so no value was read through the bad reference and no score moves: 900 HSJ and x-transformation lengths over random matrices (4-16 tips, alpha 0/0.5/1), the issue's own data, and three seeded end-to-end searches are bit-identical either side of the fix, Inf entries included. Fixes #51 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
… routing and scope gaps The user-level `/red-team` skill carried dual-mode machinery for a `findings.md` store this project deleted at the 2026-08-04 migration. Retired to `~/.claude/skills-retired/red-team/` — the only surviving record of file mode, which is still correct for a project with no GitHub remote — and replaced with a project-local skill that assumes issues throughout, alongside `/next-issue`. The one-off process for standing up a repo under `agent-issues` (fork, default branch, push block, labels, Actions, secrets, findings migration) moves to a subsidiary `github-repo-setup.md` that a normal round never opens. Fixes #41 — the rotation's routing state was tier-only: - a work-shape gate between brief assembly and dispatch, so a recorded "the next visit should NOT be a finder" verdict stops the round and reports instead of auto-dispatching. Areas 10 and 13 both carry one, and area 13 is next. - `needs-escalation` is documented as a tier flag and nothing else; sequencing and cross-area residuals stay prose, and step 3 now reads `escalation-backlog.md` whether or not the label query hit — a hit used to suppress the row holding the actual ask. - the finder brief carries the area's open issues, so a finder cannot spend its budget re-investigating a mechanism already filed. - `area:N` semantics stated: it marks which area owns the code, not which round found it, and the compound label query ANDs correctly. Addresses #42 — scope coverage: - the `tidy` pass now leads with a scope-coverage diff (glob `R/*.[Rr]`; the lowercase `pp_info_extra_step.r` is invisible to a case-sensitive pattern). - area 14 (statistics & support metrics, 14 files) and area 15 (legacy pure-R search API, 9 files) added, both `sonnet` / UNMEASURED. #42 recommended `opus` for 14 and review-once-then-park for 15; the maintainer chose measured maturity for both. N is now 15. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ion state only The previous commit put the skill and its repo-setup guide inside this repo. Both were wrong. The skill is shared across projects, so burying it here takes /red-team away from every other project; and `github-repo-setup.md` documents standing a repo UP under agent-issues, which is precisely the thing already done here and never needed again — it belongs where a not-yet-configured project reads it, next to the skill. Both now live at `~/.claude/skills/red-team/`, with the #41 routing fixes applied there. This branch keeps only what is genuinely project state: areas 14 and 15, N = 15, and the README/AGENTS label range. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Issue #51 asked for this: the memory note it was filed against points at the ASan workflow as the route to a container-OOB, and that workflow was red on trunk for the very defect the previous commit fixes. A local -D_GLIBCXX_ASSERTIONS build reproduces the same class in seconds on Windows and can be aimed at one test file, so it belongs in the subsystem's own memory file, next to the flat-vector layouts that make this the recurring failure mode here. Also records the two ways to misread its output: the abort names the container type, never the call site; and `lib.loc` must be an absolute Windows path, or test_file()'s chdir breaks the lazy-load DB and fakes several regressions. Qualifies the in-source issue reference per AGENTS.md, since src/ fast-forwards to the public upstream, where a bare `#51` resolves to an unrelated issue. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Code-review follow-up on the T-300 rerooting patch. `n_reroot_accepts` exists solely so the new regression test can tell a correct dirty-set rerooting rescore from an unexercised one. It was incremented for every rerooting move regardless of `incremental_ok`, so under HSJ/XFORM — where a rerooting accept still falls through to `full_rescore` — the counter would be positive while the arm it guards never ran. Adding an HSJ/XFORM case to the test would then have made `expect_gt(n_reroot, 0)` pass vacuously, which is exactly the failure the counter was added to prevent. Gate it on `incremental_ok`. Also record what it actually counts: the increment sits at the rescore, before the accept/reject decision, so a move counted here can still be rejected by the constraint check, the tabu test or the score comparison. It is an upper bound on accepted rerootings, not a count of them. No behaviour change: EW / IW / NA / NA-IW all have `incremental_ok`, so every case the test drives is unaffected. Third hunk is a stale comment: the NA dirty passes are no longer SPR-only. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Independent review of ca5c1c0 disputed "the last node DFS popped". It is not one node but a run of them: every childless node reached after the final push_back carries the end offset. Measured on an R mirror of build_canon_order() over 900 random trees, 2-24 tips: trees with NO kidOff==size node: 0 trees with >1 such node : 843 kidOff/kidNum CSR consistency : OK The first line matters most -- the pre-fix code formed a reference to co.kids.end() on EVERY HSJ scoring call, not on some unlucky shape. The third rules out the alternative reading that the guard papers over a corrupt CSR: kids[off + 1 .. off + num] is exactly each node's canonical children, so kidOff/kidNum are sound and only the missing nk == 0 test was wrong. Comment and NEWS reworded accordingly; the script lands under dev/red-team/reviews/ as the standing evidence. The same review found a SEPARATE unguarded bound -- tip_labels' row count is validated at neither Rcpp bridge, giving an identical `_Tp = int` const-operator[] abort from a hand-crafted TreeSearch::: call. Confirmed against a build already carrying this fix, so it is not the same defect, and it is unreachable from the public API. Filed as #58 rather than widened into this branch. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…opped Review read "which is what made the AddressSanitizer workflow unusable" as crediting ASan's own instrumentation, and objected -- correctly -- that ASan watches accesses, not address arithmetic, and that co.kids.reserve(n_node) leaves the offending address inside the live allocation anyway. The claim was about the right defect but named the wrong instrument. What aborted the gcc-ASAN job is visible in #51's own evidence: `stl_vector.h:1282 ... Assertion '__n < this->size()' failed`, a libstdc++ precondition check, not a sanitizer report. The r-hub gcc-asan container compiles with the hardened library; the flag is not in ASan.yml, which is why grepping .github/workflows/ for it finds nothing and misleads. Reworded to say what actually fires and why that workflow could not get past this package. Also distinct from the EARLIER ASan unusability that PRs ms609#262/ms609#275 fixed -- that was the runner configuration; this is a defect the now-working workflow immediately hit. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
docs: state Wagner's equal-weights placement contract; pass min_steps
fix: reject non-binary trees at the TreeState boundary
…103cc perf(tbr): finish T-300 — incremental rescore for TBR-rerooting accepts
…/ on Windows core.ignorecase=true on Windows folds "/.agent*" onto the tracked .AGENTS/ memory directory (case-insensitive prefix + wildcard suffix match), causing `git add .AGENTS/memory/*.md` to silently refuse without -f. Narrow the pattern to the actual .agent-<id> naming convention and add a defensive negation for good measure. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…lision Narrow /.agent* gitignore pattern to stop matching .AGENTS/ on Windows
fix: OOB read in HSJ token_states/CanonOrder lookup
ms609
enabled auto-merge
August 5, 2026 09:23
fix: enforce user constraints at the start-tree, pool-capture and collapse boundaries
…-4853c6 red-team: add focus areas 14 and 15 (N=15); skill rewrite lands user-level
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.