C++ search - #210
Draft
ms609 wants to merge 1002 commits into
Draft
Conversation
ms609
marked this pull request as draft
March 25, 2026 14:21
ms609
added a commit
that referenced
this pull request
Mar 28, 2026
ms609
added a commit
that referenced
this pull request
Mar 28, 2026
ms609
added a commit
that referenced
this pull request
May 18, 2026
In R CMD check, R runs as a non-interactive subprocess with captured stdout. R_FlushConsole() calls fflush() on that pipe; when the buffer fills the call blocks indefinitely, causing the 6 h GHA timeout seen on every ubuntu runner for PR #210. Gate the \r-overwrite progress line and the flush behind R_Interactive (FALSE in batch/check contexts). Interactive sessions are unchanged. At verbosity >= 2 in batch mode, emit plain \n-terminated lines so diagnostic logs still carry progress detail without the flush risk. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
ms609
added a commit
that referenced
this pull request
May 18, 2026
In R CMD check, R runs as a non-interactive subprocess with captured stdout. R_FlushConsole() calls fflush() on that pipe; when the buffer fills the call blocks indefinitely, causing the 6 h GHA timeout seen on every ubuntu runner for PR #210. Gate the \r-overwrite progress line and the flush behind R_Interactive (FALSE in batch/check contexts). Interactive sessions are unchanged. At verbosity >= 2 in batch mode, emit plain \n-terminated lines so diagnostic logs still carry progress detail without the flush risk. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This was referenced May 19, 2026
All three scorers (EW, IW, NA) now reach guaranteed true unrooted-TBR optima by default. EW/IW via the in-pass root-edge check; NA via the exact-verify sweep at convergence. Gated out for sector/constrained/ tabu/pool sub-searches (state would be invalidated) so those are unaffected. Two pre-existing test failures (tbr<=spr, ratchet<=tbr) resolved as a side-effect: the rooted kernel was getting stuck above the unrooted optimum so TBR appeared no better than SPR. Adjust test-ts-sector-resolve.R: raise targetHits to 99 so both rasStarts configurations run the full two replicates; under the new default the better initial tree caused rasStarts=3 to terminate after one replicate (fewer total candidates) even though per-sector work was correctly higher. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
exact_verify_sweep runs a full O(n^3) unrooted-TBR neighbourhood sweep at every NA convergence to certify a true optimum. The same converged topologies are re-verified many times across a search (ratchet restore passes, per-cycle TBR polish), each repeating the entire sweep. Memoize FALSE (genuine-optimum) verdicts in a thread_local cache. The result is a pure function of (topology, dataset, weighting regime), so a cached FALSE stays valid until one of those changes. Key = hash(canonical child-pairs) XOR dataset-fingerprint XOR weight-fingerprint; the dataset-fingerprint alone is the clear-trigger, so base-regime entries survive perturbation excursions and are reused. The weight-fingerprint (per-block active_mask, upweight_mask, pattern_freq -- exactly the fields ts_ratchet.cpp's save_perturb_state snapshots) is essential, not optional: the ratchet mutates the weighting in place and runs NA TBR under both perturbed and base weights within one cycle (the default strategy's ZERO_ONLY mode zeroes active_mask every cycle). Without it, a base-regime "optimal" verdict would be reused during a perturbed pass, silently skipping the improving moves the ratchet exists to find -- an NA-search-quality regression the NA oracle (which never ratchets) cannot catch. Validated on Zanol2014 (74 tips, NA), default strategy, seed 1, 3 reps: cache-on score == cache-off (TS_EV_NOCACHE) score == 1315 with identical tree count -- the cache is behaviourally transparent (a hit returns the same FALSE the full sweep would) -- at 364.5s vs 416.1s wall (-12.4%). TBR regression suite: 0 failures/errors. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…rds) The comment claimed the hash is "root-position-independent (any rooting of the same unrooted topology produces the same hash)". That is false and, worse, backwards: sorting each child pair canonicalizes only the left/right child order WITHIN a node, not across rerootings. Different rootings renumber the internal nodes and flip parent/child directions, so they hash differently (verified: two rootings of one unrooted tree differ at 55/73 internal nodes). That root-DEPENDENCE is exactly what makes the memoization cache correct: exact_verify_sweep is itself root-dependent (it skips root-child clips, leaving a residual completeness gap — task #19), so each rooting has a different neighbourhood and must be cached separately. The previous wording invited a future "canonicalize to root-independent" optimization that would cause cross-rooting cache hits to suppress improvers one rooting finds and another misses. Comment-only; no behaviour change. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The exact_verify_sweep optimum cache memoizes "genuine NA optimum" verdicts, which are valid only under the current weighting regime. The ratchet mutates active_mask/upweight_mask/pattern_freq in place mid- search, so a base-regime verdict leaking into a perturbed pass silently skips the improvers the ratchet exists to find. That regression is invisible to the NA oracle (never ratchets) and to final scores (always recomputed), so guard it deterministically at the key level: - extract the key into one shared helper, exact_verify_cache_key(), called by BOTH the cache and the probe, so dropping a term from the key line is caught -- not just a broken fingerprint function; - ts_ev_cache_key_probe export returns the exact key; its flags reproduce the three ways the ratchet changes the regime (zero_active = ZERO_ONLY, the default NA strategy; set_upweight; bump_pattern_freq); - test-ts-na-evcache.R asserts the composite key changes for each regime field AND for topology/dataset, and is deterministic; - TS_EV_AUDIT (off by default): on a cache hit, re-run the full sweep and abort if it finds an improver -- a live tripwire for contamination. Mutation-validated: dropping `^ weight_fingerprint` fails exactly the three regime assertions (topology/dataset/determinism still pass). Default path byte-identical; TBR regression suite 0 failures. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The exact directional insertion edge-set computation (the EW per-clip
hotspot, ~27% of full-EW CPU per VTune) reallocated and zero-filled its
edge_set and up-message buffers on every clip. Both are fully written
before any read for every in-tree non-root node (the only slots any
reader touches), so the zero-fill was pure waste.
Hoist `up` and `pre` to caller-owned scratch passed by reference (NOT
thread_local — function-local per call, so per-thread safe) and replace
the per-call assign(N,0) with a non-zeroing size-ensure. A debug-only
(#ifndef NDEBUG) write-before-read guard records every written non-root
slot and asserts completeness against the in-tree node set, so a stale
slot can never be read. Threaded through all three call sites
(tbr_search, build_ras_sector, wagner_tree).
Verified:
- EW bit-identical: score AND candidates_evaluated exact over
{Wortley2006,Zhu2013,Zanol2014} x seeds{1,2}, reps3.
- NA single-threaded bit-identical: Vinther2008 x seeds{1..4}, exact.
- 276 kernel search tests pass (tbr/wagner/sector/ratchet/drift/fitch).
- Wall: -16.4% on Zanol2014 (largest), -9.4% sum on heavy runs; the
saving scales with O(n_node * total_words), as predicted.
NB an unrelated pre-existing crash exists in the parallel (nThreads>=2)
NA path (Vinther2008): it reproduces on the unmodified baseline, is a
timing-dependent race, and is independent of this bit-identical change.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ergence exact_verify_sweep (the NA convergence certifier) clips every internal node EXCEPT root-children (the nx==n_tip guard), so the one unrooted edge the display root sits on (cL-cR) was never enumerated. On poor NA starts that left a root-edge improver undetected: the kernel declared convergence above the true unrooted-TBR optimum (dev/benchmarks/tbr_oracle_na.R: 2/20 on Zanol2014, e.g. start #14 stalled at 1323 with a 1320 neighbour). Fix: enumerate that one edge exactly at the optimum exit, reusing the already- tested try_root_edge_moves_rescore (the same apply+full_rescore path IW uses at convergence). The clip loop covers the 2n-4 non-root edges; the root-edge check covers the 1 remaining edge; together they certify all 2n-3 unrooted edges, so a FALSE return — and the memoized optimum — now means a true unrooted-TBR optimum. The change strictly enlarges the certified neighbourhood and only ever applies strict improvers, so it can only remove "converged-with-improver" failures and cannot worsen any score. Validation: Zanol2014 start #14 now reaches 1320 with no improver; small-tree NA oracle 0/100; IW unaffected (already complete, 0/60); TBR regression suite 0 failures. Adds dev/benchmarks/tbr_oracle_na_small.R (fast high-N oracle) and tests/testthat/test-ts-na-complete.R (Tier-3 regression pinning start #14). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Records why the exact O(1) "EW additive" directional scan is provably unavailable for NA (the per-node Pass-3 step is not 2-local — it reads the global applicable- region resolution; code already gates NA off the additive path), why a richer fixed-size-message DP is plausible but research-grade (the one directional NA message that exists, fitch_na_indirect_length, is deliberately approximate), and recommends the cheaper lever for #18: incremental EXACT rescore (pruning via the approximate scan + a localised Pass-3 delta on top of fitch_na_dirty_*), validated against the now-complete exact_verify_sweep oracle. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…build Rogue hard-imports Rfast, which compiles from source under the r-hub gcc-asan container (~30 min). All Rogue usage in TreeSearch is already requireNamespace-guarded (two vignettes, Shiny consensus module), so it skips cleanly when absent. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…253) The parallel NA search (nThreads>=2) intermittently aborted with STATUS_HEAP_CORRUPTION. Root cause: the per-thread scratch in the TBR kernel (ts_tbr.cpp, ts_fitch.cpp) and exact_verify_sweep's optimum cache were function-local `static thread_local`. On MinGW these resolve via emutls, whose thread_local teardown across std::thread spawn/exit corrupts the heap. EW is unaffected (light TLS); the NA path trips it because exact_verify adds a thread_local unordered_set plus more scratch. Fix: convert all worker-reachable scratch to plain function-locals (each worker owns its call frame -> per-thread-safe; per-clip realloc measured <=1.6% on 88-tip data, ~0% typical). Move exact_verify_sweep's optimum memoization to mutable members on DataSet so it keeps the same per-worker, cross-replicate persistence the thread_local had, without emutls. Verified on clean builds (rm src/*.o; CCACHE_DISABLE=1; --preclean): parallel NA survives 120/120 (was iter ~4-8), EW 200/200, serial scores bit-identical, NA perf 4.15s (cache intact, vs 5.81s cache-disabled). Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
…ot loops Bank the validated micro-lever sweep from branch claude/tbr-microlevers (task #48). All changes are BYTE-IDENTICAL: score + candidates_evaluated unchanged on Wortley2006/Zhu2013/Zanol2014 x seed{1,2} (verify_l1.R, 6/6). THE WIN — a diagnostic std::getenv("TS_REVERT_CHECK") left in the per-clip teardown (~100k+ calls/search) was costing 13-19% of EW MaximizeParsimony wall on Windows/ucrt, where getenv is us-scale (locked env-block scan), not sub-ns. Hoisted to a per-call bool. Quiet-machine same-seed paired A/B: Zanol -13.2% (20/20, p=0), Zhu -19.1% (12/12, p=0); 3-way attribution proves the getenv hoist alone is the entire win. Also folded in (both byte-identical, both ~0 measured effect, kept as exact cleanups): - cutoff hoist: maintain the EW/NA bail cutoff across the clip, recompute only on improvement (+0.00%, attribution-proven). - kept_ei: precompute sub_edge-invariant reroot skip predicates once per clip (marginal/wash even at Zanol-1261; droppable). Caveat: getenv magnitude is env-size + platform dependent (Windows/ucrt large; Linux cheaper) — Hamilton/Linux confirmation owed. Byte-identical and strictly removes ~100k getenv/search regardless. Detail: dev/profiling/findings.md T-P5n + dev/profiling/tbr-microlever-sweep.md Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Lands the sectorial component-isolation round (gate 1) onto cpp-search.
ts_sector.cpp: 3 byte-identical micro-levers (search_sector ras_starts==1
fast path; compute_from_above new_from_above alloc->std::swap; TS_FREE_HTU_PROBE
/ TS_SECT_DEBUG getenv -> cached static — the per-sector getenvs T-P5n flagged
"for the sectorial agent"). ~2.8% isolated sectorial; verified score +
n_sectors byte-identical across {Zanol,Zhu,Wortley}x{wagner,tbr}xseed{1,2} and
the mission A/B; 8 search test files pass.
Harness: drivers/sector-rss.R (isolated rss), drivers/mission-getenv-ab.R
(full-search getenv A/B), run_sector_tests.R, microbench/bench_getenv.cpp
(std::getenv = 2398 ns/call on ucrt), sector-levers.patch, PRODUCTION-LEVERS.md.
focus-areas.md: RSS #3 / CSS-XSS #6 -> AT-LIMIT (throughput at-limit by
inheritance; ~96% is the at-limit inner+global tbr_search).
findings.md T-S6a-e left uncommitted to land with the in-flight T-P5n/o rows.
The mission-wide getenv finding (T-S6d) converges with T-P5n (commit 3a50537e).
Measurement-only instrumentation NOT included (env-gated, stayed on the
scratch worktree branch).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Completes beb5213's getenv-hoist work. TS_PHYS_REROOT was the last diagnostic getenv read inside tbr_search's hot/warm path -- the outer reroot-control loop (>=1x/call) at ts_tbr.cpp:2133. Hoist it to a per-call const bool at function entry (1277), matching revert_check / iw_scanchk from beb5213. It selects the legacy physical-reroot reference path and is constant for the process, so this is byte-identical. std::getenv is ~2.4us/call on Windows/ucrt (linear env-block scan) and its cost hides in VTune's ucrtbase self-time. The headline per-clip TS_REVERT_CHECK win (~13-22% EW wall) was ALREADY merged in beb5213 (findings T-P5n) and confirmed gone by the post-getenv re-survey (T-P5o); this only clears the small residual per-reroot read. Verify: ts-tbr-search + ts-ratchet-search 45/45 pass, CustomSearch clean (isolated install). findings.md: add T-P5l/m cross-ref notes so future readers trusting "TBR kernel at-limit / closed" don't miss that a profiler-invisible getenv cost sat next to the at-limit kernel. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…etenv out of tbr_search + findings T-P5l/m at-limit≠no-overhead correction
tree_fuse segfaulted on >64-tip data (wps>=2) with intraFuse=TRUE: reroot_at_tip0 ran once before the round loop, but the round-end TBR moves tip 0 off the root, so round >=2 split-matching matched a clade against its complement and replace_subtree corrupted the tree. Fix = re-root every round (early-returns when already rooted, so round 1 is byte-identical) + a defensive replace_subtree size guard. Ported from TreeSearch-nonclade; 22/22 fuse tests pass incl. an 80-tip regression test. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> (cherry picked from commit da21f5dce652ee85d12e1e50d1cba349c74824b4)
…n plan - dev/red-team/proofs/lever-c-bound-then-verify.md: 531-line durable proof settling lever-c (bound-then-verify) as dead-by-proof-plus-magnitude (no-forced-step lemma + net-overhead inequality + origin-recovery cap). - dev/plans/2026-06-20-fuse-drift-isolation.md: fuse/drift component isolation plan + progress log (gate-1 AT-LIMIT complete across all components; fuse >64t crash fix; SCOREAPPROX finding T-F1). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The union-of-finals Y = final(A)|final(D) is an APPROXIMATION that UNDER-counts insertion cost (it is a superset of the true directional edge set), not an exact non-additive method. The exact cost is the directional edge_set[D] = combine(prelim[D], up[D]) via compute_insertion_edge_sets + fitch_indirect_length_cached. Comment now matches ts_fitch.h and the validated directional-fix finding. Doc-only; no behaviour change. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Backwards comment fixed+landed (8671fda); Δ-probe showed ~48% greedy-regret share in expand_and_reinsert on Zanol; exact-scorer port prepared+validated on worktree (41b0d237); heavy A/B path-killed (no mission dataset >=120t, so prune_reinsert never auto-enables) → land+A/B deferred to composition #40. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
ts_search.cpp spr_search uses the bounded scorer but is off-default (sprFirst=FALSE everywhere), exact-verify-gated (never false-accepts), and a warmup washed by the subsequent exact tbr_search → silent-miss mooted, no action. All remaining union-of-finals sites accounted for and benign. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Read of ts_driven.cpp orchestration: per-phase score_tree prints are verbosity>=2-gated (off at default v=1L); only un-gated full rescores are the 2 per-outer-cycle convergence checks (~µs each, ~0.001% wall, one redundant but sub-floor) + 1 final/replicate. Step-switching minimal (each phase owns its state). R/C marshalling already T-P5o'd as amortizable. Last undone non-gated aspect of the isolation plan; addressable wall now lives in composition #40. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Direct internal .Call with a hand-crafted xformConfig and a short tip_states vector would read past its end in the per-tip loop. Same internal-boundary hardening class as T-323/T-328. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
… topology (T-369) Tip placement always ranks candidates the same way regardless of the numeric concavity value used; only the reported $score differs. Verified empirically across concavity = Inf/10/3 on Longrich2010. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
FirstHit() reassigns r$trees via WhenFirstHit() without updating r$treeHash, breaking the documented invariant that every r$trees write must be paired with a hash update (app_state.R:34) that ~9 downstream caches key off. Currently latent (WhenFirstHit only sets an attribute and only fires for legacy seed/start/ratchN/final tree names nothing here produces), but a real invariant break waiting to bite the next extension of WhenFirstHit().
T-335: name the five stale per-pattern arrays (n_patterns/min_steps/ pattern_freq/precomputed_steps/plane_state) at the EW-only column-reduce gate so widening it toward IW/weighted scoring can't silently skip rebuilding them. T-338: mark every worker-thread-reachable getenv()/Rprintf/audit-static site on the parallel resample path as inert-but-conditional, so a future speed change on this path can't flip one live without noticing. Both are dormant tripwires from dev/red-team/findings.md, comment-only.
…ental_rescore Phase 2's early-termination check tested only "final_ unchanged" and missed that the changed-prelim region from Phase 1 can lie below the node where it stopped checking, leaving newly-created internals' final_ stale at zero. Re-verified (not just trusted) that none of the three callers read final_ before their next full uppass/rescore, so Phase 2 was dead work; deleted it outright per the finding's recommended fix, and corrected the now-stale comment in ts_prune_reinsert.cpp that described the deleted uppass. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Records the c1c6b30 fix (paired r$treeHash update in FirstHit) so the log reflects closed status rather than open.
…_tree expand_and_reinsert() scored the rebuilt backbone with score_tree(), which on has_inapplicable datasets falls through to fitch_na_score and writes NA-regime prelim. The insertion loop only patches inserted-path entries via wagner_incremental_rescore (standard-Fitch only, no NA branch), leaving tree.prelim in a mixed regime that compute_insertion_edge_sets() then reads to choose reinsertion edges. Swap to fitch_score(), matching the sibling call sites (ts_wagner.cpp:449, ts_sector.cpp:917). No wrong final score can escape (full_rescore() is authoritative); this only affects placement quality during prune-reinsert, which is opt-in and default-off (pruneReinsertCycles = 0L in both R/SearchControl.R and R/ts-driven-compat.R). A/B on Dikow2009 (NA data, cycles forced on) confirms the swap changes search trajectory on some seeds without any crash or score corruption in either arm. Known follow-up, out of scope here: fitch_na_score's local_cost is only written in the non-NA branch, which still corrupts wagner_incremental_rescore's old_cost subtraction for NA blocks. Wants its own A/B (T-366 follow-up). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…atch) Directed fix, not a rotation round: documents the finding fixed in d94d76b (fitch_score vs score_tree in expand_and_reinsert), including the A/B verification performed and the fitch_na_score local_cost follow-up that's out of scope. last_focus left untouched per that convention. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…tion random_topology_tree() and random_constrained_tree() seeded edge_children with both of the root's two children, but a degree-2 root means those are the two halves of a single unrooted edge -- so it was sampled with roughly double the probability of any other edge, violating the documented uniform-random-tree guarantee (T-370). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
test-ParsSim.R, test-MaddisonSlatkin.R and test-recode-hierarchy.R each carried a Tier-2 skip_on_cran() guard the testing-strategy.md rules never actually grant them (Tier 2 is test-ts-*.R only), so a dependency regression in ParsSim/MaddisonSlatkin/RecodeHierarchy was invisible to CRAN's own checks. Guard added in bulk commit 78b7414 without updating the doc. Removed the file-level guards plus a stray per-test skip_on_cran() on .MSSplitCount from the same commit; all three files re-measured clean and fast (<5s, 0 failures) with NOT_CRAN unset. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Moved from findings.md to findings-archive.md, citing 547f6ab (root-edge double-counting fix) and the empirical uniformity check that confirmed it. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ofile
`LengthAdded()` / `PolEscapa()` with `concavity = "profile"` derived `cont`,
`qm`, `qmApp`, `app` and `inapp` from the character the user supplied, then
scored the character `PrepareDataProfile()` returns. That function replaces
the contrast wholesale with `rbind(diag(k), rep(1, k))` and renumbers every
token to `1:k` with `k + 1` denoting ambiguity, collapsing inapplicable,
partially ambiguous and singleton codings into it — so the stale row indices
named rows that no longer existed, and the `qm` / `qmApp` fallbacks
`rbind()`ed a row of raw width onto the prepared contrast.
Every character whose raw and prepared token spaces differed in shape errored
out; only a plain {0, 1, ?} character worked, by coincidence of index spaces.
Reproduced (pre-fix messages):
0,0,0,1,1,1,2,2,3,? `tip_data` values must be in [1, nrow(contrast)] (4); found 5
0,0,0,1,1,1,-,- `levels` length (2) must equal ncol(contrast) (3)
0,0,0,1,1,1,2,2,3 `levels` length (3) must equal ncol(contrast) (4)
0,0,0,1,1,1,2,3 `levels` length (2) must equal ncol(contrast) (4)
Note the last of these, not the third, is the case filed as (c): the filed
message needs two singletons, which empty both `qm` and `qmApp`.
Fix: move the whole block below the prepare step, keeping the pre-prepare
contrast only as `rawCont` for the user-input validation that must run on it.
`PrepareDataIW()` touches neither tokens nor contrast, so the equal-weights
and implied-weights paths are unaffected — verified `identical()` before and
after across ten character x concavity combinations.
Post-fix the profile path has no "-" column, so `qm == qmApp == k + 1` and
both fallbacks stay dormant; an inapplicable leaf is ambiguous either way and
still scores a zero-length change, as documented.
Tests: four regression cases with `concavity = "profile"` (the string appeared
nowhere in the file before), asserting exact zeros for ambiguity-collapsed
leaves, a strictly positive delta for informative ones, and agreement with an
independent recomputation in the prepared token space. All four fail against
the pre-fix code. covr on R/PolEscapa.R: 55/55 lines, 100%.
Reached users through the Shiny character-wise plot, which pipes the app's
concavity setting, which can be "profile", straight into LengthAdded().
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…TreeLength) T-365: records the fix, the ten-combination `identical()` EW/IW check, the 100% covr result, and two corrections for the next reader — the filed case (c) message needs the two-singleton character (`0,0,0,1,1,1,2,3`), and the `nr == 0` guard was deliberately not added because `TreeLength()` errors before it, which is T-372. T-372: new P3, found while fixing T-365 rather than by a finder sweep. `PrepareDataProfile()`'s uninformative-character early return emits `info.amounts` as `double(0)` rather than a matrix; the multiPhylo profile path then raises "Not a matrix." where the single-tree path returns 0. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…per-edge loop
Constrained Wagner starts re-derived their constraint bookkeeping from
scratch for every candidate insertion edge. Unconstrained starts pay
nothing either way: the whole block sits inside `if (constrained)`.
Three behaviour-preserving reductions, all in the constrained path:
1. `node_tips` was a fresh `n_node * nw` heap allocation, zero-filled once
per inserted tip (~64 KB/step, ~32 MB per start at 500 tips). It is now
caller-owned scratch on the same non-zeroing-reuse contract as the
insertion edge sets. No per-call reset is needed at all: tip rows are
the constant singleton {t} and are written once by ensure(), and internal
rows are fully overwritten by the postorder union, whose write set is
exactly the read set (tree.postorder). The zero-fill only ever existed
for words 1..nw-1 of a tip row, which exist once n_tip > 64.
The larger allocation win is the one the finding did not name: `needed`
was a `std::vector` constructed *per split per call* — ~25k malloc/free
per start at 50 splits x 500 tips, against 500 memsets for `node_tips`.
It is now scratch too.
2. `wagner_edge_violates_constraint` recomputed `tip_inside`,
`has_prev_inside`, `has_prev_outside`, `cn` and the `cn == root` skip for
each of the ~2i candidate edges, though all are invariant across the DFS
for a given (tip, split). They move to `wagner_collect_active_splits`,
run once per inserted tip; the per-edge test drops from
O(n_splits * nw) to O(#active splits) and now touches only `below`.
Splits are collected in index order, so the per-edge loop still returns
on the same first violation.
3. The per-split tip scan was an O(n_tip * nw) way of computing a popcount:
a tip mask is the singleton {t}, so `needed subset of {t}` with `needed`
non-empty forces `needed == {t}`. At most one tip can match, only when
exactly one inside tip has been added, and it is necessarily already in
`added_tips`. When it matches, the internal scan cannot fire either
(it needs `sz < best_size == 1`, impossible for a superset of a non-empty
set), so both loops collapse to a direct assignment.
Verified byte-identical, not merely same-tree. A temporary probe logged
every insertion step's chosen edge plus an FNV hash over every
(candidate edge, violates?) verdict and over `cd.constraint_node`: all 1575
steps across 15 constrained AdditionTree() runs are identical before and
after. Cases span nw = 1, 2 and 3 (60/100/130/150 tips, 5-10 splits), since
multi-word tip rows — the whole point of fix 1 — do not exist below 65 tips.
Returned trees `identical()` in 15/15. test-ts-constraint-small.R 8/8 and
test-ts-constraint-multi.R 336/336 pass.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ad under T-324 T-368 -> fixed `37bca4b6`. The row now records that the fix landed three reductions rather than the two filed, and that the third is NOT the optional postorder-mask rebuild the finding floated: that is ~4k word-ops/call against ~600k for the split loops, so it was deliberately left alone. The real third win was that the per-split tip scan was an O(n_tip*nw) popcount in disguise. Also records a fourth, unfiled allocation win (`needed` was constructed per split per call) that is larger than the one the finding named. The verification method is worth the words: byte-identity over every (candidate edge, violates?) verdict, not tree-equality — two different decision sequences can reach the same tree. Re-verified against each new base while rebasing, since T-367 and T-370 both touch the same file. Separately, files an ADJACENT EVIDENCE note under T-324, whose open question is reachability of a satisfiable-constraint violation. Constrained AdditionTree() was seen returning violating trees for trivially satisfiable constraints on the UNMODIFIED tip. The note is explicit that this does not settle T-324 — AdditionTree() calls ts::wagner_tree directly, so neither the 100-reshuffle retry loop nor violates_constraint_posthoc is on that path — and that the mechanism is correlation, not a trace. Recorded rather than chased so the next round does not re-discover it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ith a real differential Commit d8c5999 justified TS_CHAR_ORDER with "the score is order-invariant (covered by the existing 'Scores are correct after block reordering' test)". That test never varied the ordering, and its loop assertion `s >= score || s > 0` is satisfied by `s > 0` alone, so it could not fail on an informative matrix. No test in the package referenced TS_CHAR_ORDER at all. The invariance claim is TRUE -- red-team area 10 verified it empirically on a non-identity permutation (180/180 positions moved) across EW/IW/XPIWE/profile and per-pattern char_steps. This pins it, in the modes that can break it: IW, XPIWE and profile index per-pattern arrays (ds.min_steps, ds.info_amounts, populated at ts_data.cpp:447+, i.e. after the sort at :190), so a block-slot-to-pattern mix-up would surface there and nowhere else. Guards against this test going vacuous in turn, which was the live risk: - multistate matrix (binary data -> MINORITY ties -> stable sort -> identity permutation -> green proves nothing), with expect_gt on the level count; - is.finite() on every reference score, because waldo treats NaN as equal to NaN, so a NaN profile arm would pass every comparison silently; - nTip 12 at 4 states keeps MaddisonSlatkin's exact cache within capacity, so info.amounts is exact and warning-free; nTip 14 spills to Monte Carlo. Also two defensive fixes found in the same round: - ts_sankoff.cpp: sankoff_score_char and sankoff_uppass both read postorder[n_internal - 1], i.e. postorder[-1], on a 0-internal-node tree. - ts_data.h: total_words is the block sum rounded UP to even (SIMD padding), so block_word_offset[b] + blocks[b].n_states can be less than total_words for the last block; the comment claimed a plain sum. Plus a red-team bookkeeping correction: escalation-backlog item 5(a) carried a bug claim ("carry this first, it is the one item here with a hard numeric repro") that had already been refuted on 2026-06-16 by the very area it was routed to. Retired, with the arithmetic re-confirmed: the reported 0.16573 is XPIWE phi/eff_k scaling, (2/11.3)(10.3/11) = 0.165728, not a fractional min-steps. Two narrow sub-items survive. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
covr's block-exclusion regex requires "nocov start", not "nocov begin"; "begin" only matches the single-line exclude_pattern, so the three Rcpp::stop() guards between the markers were still counted as uncovered. Verified via covr::tally_coverage that the guarded lines are now excluded.
AdditionTree()'s C++ Wagner kernel roots arbitrarily (an artefact of build_three_taxon_tree/insert_tip_at_edge), not on sequence[1] as the docs claimed. Point users at TreeTools::RootTree() instead, and add a regression test so the docs claim can't silently drift back. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ur P1s in HSJ/XFORM
First-versioned measurement of area 10: its only dry rounds (2026-05-19,
2026-05-26) are pre-tier and version-unrecorded, so it had zero version-scoped
dry verdicts.
The result is a cautionary tale about tier state. The area looked mature, and
its Profile/IW half genuinely is -- every static residual (profile delta
capping, e/(k+e), precomputed_steps offset, info_amounts capping, concavity=1.0
sentinel, DAT-002 obs==0 reachability) was independently re-derived clean, as
was the char-ordering x per-pattern-indexing question on a proven non-identity
permutation. But ts_hsj.*/ts_sankoff.* were added to the scope row on
2026-07-03 and no finder had ever read them at any tier, so both dry verdicts
were dry about a different half of the area than the row described. Every P1
came from the unread half. Lesson recorded in focus-areas.md: a scope row that
grows does not inherit the dry verdicts earned before it grew.
Filed:
- T-373 P1 search silently no-ops when total_words==0; under HSJ/XFORM that
does not mean "all trees score the same". Start tree returned
bit-identical under maximal effort (10->10) vs a control moving
13->8; ~1.075% of hierarchical bootstrap replicates feed an
unsearched tree into user-visible support values.
- T-374 P1 HSJ/XFORM scores are rooting-dependent while the pipeline treats
topologies as unrooted: reported best 15.5 vs TreeLength 16 15.5
15.5 16 16 16 on the SAME six unrooted topologies (cross-RF 0).
Trigger localised to ts_collapse_pool's tip-0 re-root, whose
"rooting-invariant" comment is false for these modes.
- T-375 P1 HSJ bit-encodes contrast-row token indices as states, so "?" in a
secondary scores as a concrete conflicting state.
- T-376 P1 primary_present compares a token index against a levels index. A
confound-free 24-way token permutation (m==0, so fitch_label_char
provably out of the loop) gives HSJ 2 or 3 for one identical
dataset+tree -- the score is not a function of the data. On shipped
Vinther2008.nex via ReadAsPhyDat, absent and inapplicable primaries
read as present while "?" reads as absent.
- T-377 P2 TBR candidate scan hierarchy-blind (NNI is not; scope narrowed).
- T-378 P2 ratchet cannot reweight hierarchy-only patterns.
- T-379 P3 XFORM -2 sentinel discards known secondaries.
- T-380 P3 ts_sankoff_test/unpack_xform dimensions unvalidated.
- T-381 P3 min_steps clamp masks an invariant violation.
- T-382 P3 collapse reads stale local_cost/prelim (conservative).
Both haiku refutations were overturned, and re-verifying them was the
highest-value orchestrator work of the round:
- T-380: Rcpp::NumericMatrix::operator() is NOT bounds-checked. Matrix.h:174 is
the only offset overload (i + nrows*j, no check), no size_t variant exists in
Rcpp/vector/, and Vector.h:338 operator[] is cache.ref(i). Worse than filed:
the wrong stride gives a silently-wrong in-bounds read before any OOB.
- T-378: hierarchy characters are in ds.blocks[] only when a pattern is SHARED
with a free character; hierarchy-only patterns reach build_dataset already
weight-zeroed by .NonHierarchyWeights and are erased at ts_data.cpp:236-238.
Also retired escalation-backlog item 5(a) pre-dispatch. It was flagged "carry
this first, it is the one item here with a hard numeric repro" and had already
been refuted on 2026-06-16 by the very area it was routed to; the 2026-07-27
re-queue read the entry that raised the signal but not the one that killed it.
Arithmetic re-confirmed: the "non-rational" 0.16573 is XPIWE phi/eff_k scaling,
(2/11.3)(10.3/11) = 0.165728, and at eff_k=10 it collapses to 0.166667 exactly
-- the correction is the identity when nothing is missing, which is why Lobo
agreed and Vinther2008 did not. Corollary now written into the backlog: before
re-queuing any residual, grep log.md for later rounds on the RECEIVING area,
not just the raising one.
Not reached, named so it does not become another orphan: the clipped-subtree
IW-screening follow-up.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
… is worse than filed
The user pointed out Hopkins & St John (2021) is in the Zotero library, which
closes the open design question the area-10 round left on T-374 and escalates
T-376 substantially.
DESIGN QUESTION SETTLED (HSJ half). The rooting-dependence is a BUG, not an
intended rooted objective. The paper defines the score as a MINIMUM over
internal-node labelings of a sum of SYMMETRIC dissimilarities over the branches
of an unrooted tree (p.3 "compute the minimal score"; p.6 keeps both
present/absent possibilities and "return[s] the minimal score"), so it is
rooting-invariant by construction. The fix is the paper's two-state DP;
fitch_label_char's single directional DELTRAN pick is neither the paper's
algorithm nor guaranteed minimal. XFORM remains open -- an asymmetric step
matrix is intrinsically rooted, so pinning a rooting may be correct there, at
the cost of a TBR rethink.
T-376 ESCALATED. The paper's p.6 identity "HSJ is equivalent to the Fitch
approaches when alpha = 0.0" fails 20/20 trees. The discriminator is
unambiguous: HSJ(alpha=0) equals Fitch over the NON-controlling primaries
exactly, with the shortfall equal to Fitch(char 1) on every tree -- so on an
ordinary MatrixToPhyDat layout the controlling primary contributes NOTHING,
which is the entire point of the HSJ method. Confirmed by calling
.HSJAbsentState() directly: levels = "- 0 1" gives absent_state=1, inapp_state=0,
while allLevels = "1 0 -" gives token("1")=0 -- so a PRESENT tip collides with
inapp_state and reads absent, and a "-" tip reads present.
This is the T-307 regression of 2026-06-15 in mirror image: that fix made "0"
primaries read as present (no gain/loss counted); this makes "1" primaries read
as absent (no gain/loss counted). It inverted the failure rather than removing
it, and its verification battery permuted `levels` rather than `allLevels`, so
it could not see this.
Adds dev/red-team/heavy-tests/hsj-paper-oracle.R -- four paper-derived checks
with citations inline, currently 2 pass / 3 fail, as the fix gate. Figure 1's
absolute numbers were re-derived independently first (left 6+alpha=7, right
3+2alpha=5, one consistent alpha=1, derived without choosing a root) to confirm
the reading of the formula before relying on it.
The oracle deliberately reports INCONCLUSIVE rather than green for rooting
invariance and the p.5 per-branch bound: both are inert while this bug holds the
alpha term at zero, so a green there would be vacuous -- the same failure mode as
the tautological char-ordering test replaced earlier in this round.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Promotes the experimental TS_STOP_PATIENCE knob to a real SearchControl
field, `stopPatience`, and uses it to ship a deeper implied-weights ratchet
for `sprint` and `default`.
`stopPatience` stops the search after N consecutive replicates fail to
improve the best score. Unlike `perturbStopFactor` it is a FLAT count --
it refers neither to the tip count nor to the hit count -- so the replicate
at which it fires does not stretch as replicates become individually more
expensive. That is what lets a deeper ratchet pay for itself: both other
no-improvement rules are indexed on replicates via hits, so under them a
change that makes a replicate better but slower delays the stop instead of
improving the answer. The count resets on every improvement, so the search
stops at last_improved + stopPatience. 0 (the default) disables it.
The env var is gone: one knob, one path. Nothing committed read it.
Preset values, implied weights ONLY, via .IwStopPackage() -- deliberately
disjoint by strategy from .IwRatchetDepth() so the two cannot both set
ratchetCycles, and scoped the same way (finite concavity, never overriding
a field the caller set):
sprint ratchetCycles 12, ratchetPerturbProb 0.25, stopPatience 20
default ratchetCycles 20, stopPatience 15
Measured over 68 training matrices x 6 seeds at k = 10 (two arrays, 4624
cells). Each half fails alone: the ratchet is the quality lever (`default`
cyc20 alone scored better on 11 matrices and worse on 0, p = 0.001, reach
0.78 -> 0.84) but costs +25s of a 151s mean; patience alone degrades score
(`default` 1 better/15 worse, p = 5e-04). Together, on the MEDIAN matrix:
sprint -26% wall (19 faster/5 slower) with score 4/0 and no MPT loss;
default -18% wall (33/11, p = 0.001) with score 9/3 -- favourable direction
only, NOT significant (p = 0.15).
A sweep over stopPatience {10,15,20,25,30} found score and wall both
MONOTONE in the value with no spike, so these are operating points on a
smooth trade-off rather than fitted constants. Chosen on the median
per-matrix wall change and its sign count, not the mean: the mean is
dominated by the largest matrices and picks a value that makes the typical
matrix slower. Residual cost, stated in NEWS rather than buried: 9 of 44
`default` matrices remain >10% slower, those where patience does not fire.
Equal weights, profile parsimony and `thorough`/`large` are untouched --
none was measured for this.
Tests: 33 assertions covering the mechanism, the fire-at-last_improved
identity, the plumbing round trip (the C++ read is presence-guarded, so a
dropped field would otherwise fail silently as "patience off"), validation,
and the scope -- every .IwStopPackage() branch, plus `strategy = "auto"`
under IW and EW, since auto resolves to exactly the two changed presets.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…cters
PrepareDataProfile()'s early return set info.amounts to a bare double(0)
instead of a matrix, so scoring a multiPhylo under profile parsimony
against an uninformative character failed converting it to a C++
NumericMatrix ("Not a matrix."). Fixed to emit a zero-column matrix.
LengthAdded()/PolEscapa() crashed the same way for the same input
("argument is of length zero"), since it reassigns char to the
zero-character prepared dataset and then indexes it per leaf. Guarded
with an early all-zero return: ambiguating one already-uninformative
leaf cannot create information.
Adds regression tests for both TreeLength(multiPhylo, ...) and
LengthAdded(), and updates the PrepareDataProfile() empty-matrix
fixture. Marks T-372 resolved in dev/red-team/findings.md.
Follow-up to 323a0da. The values shipped there were measured entirely with nThreads = 1, and the parallel path is not equivalent -- so the non-zero default landed on a path that had not been executed once. Now exercised, with two things the run falsified: - The parallel rule counts a dry spell over replicates completed into the shared pool and is evaluated on the coordinating thread's 200 ms poll, so it fires later and less precisely than the serial rule: patience 5 on one 40-tip matrix stopped at 6 replicates serial, 21 on two threads, 65 on four. The `lastImprovement + patience` identity is serial-only, and `last_improved_rep` is 0 in parallel anyway, so the ?MaximizeParsimony sentence pointing readers at that arithmetic now says "in a serial search". Documented in ?SearchControl and NEWS; noted at the call site that the wall saving will be smaller in parallel. - If replicates are cheap enough that the whole search finishes between two polls, the rule never runs at all: the monitor wakes, sees `replicates_done >= max_replicates`, and breaks. That granularity is shared by perturbStopFactor and consensusStableReps -- it is a property of the parallel monitor, not of this rule -- and it is why the new test uses a 40-tip matrix rather than the 8-tip one the serial tests share. No behaviour change; the parallel implementation is as landed and does not crash at 2 or 4 threads (worth checking given PR #258's MinGW heap corruption on this path, and a restructured guard in the replicate loop). Also pins `strategy = "none"` in that test, since 40 tips x 30 characters makes `auto` resolve to `default` -- whose new implied-weights patience stopped the control arm and made the comparison measure nothing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Merged rather than rebased deliberately: 34 local commits from several parallel sessions cite each other's SHAs inside dev/red-team/findings.md and log.md, so a rebase would invalidate those citations and force the same two files to be re-resolved up to 34 times. Precedent: 08a3820. Conflict resolutions (all four were additive; nothing was dropped): - dev/red-team/log.md -- kept the incoming T-366 directed-fix entry and set last_focus: 10. The merge base and remote both read 8; the remote correctly left it alone because its entry was a directed fix, not a rotation round. Local had advanced 8 -> 9 (area-9 round) -> 10 (area-10 round), so 10 stands. - dev/red-team/findings.md -- BOTH SIDES HAD FILED A T-366 ROW for the same defect at the same file:line: the area-9 round filed the diagnosis, a directed fix session filed the fix. Collapsed into one row combining the area-9 row's mechanism trace (ts_fitch_na.h:43-68, the ts_sector.cpp sibling comment, the authority sites) with the fix row's applied fix, Dikow2009 A/B and "fixed d94d76b". Verification status is now stated precisely because the two rows disagreed: the FINDING was opus-verified; the FIX was not independently re-verified. Heed red-team's own advice to leave an ID gap when sessions run in parallel. Dropped the incoming T-362/T-363 rows as strict subsets -- the local versions add the "Resolved:" sections recording that both were fixed. - R/AdditionTree.R + man/AdditionTree.Rd -- both sides rewrote the same @return block to correct the same rooting claim. Combined: local's fuller content (degree-two root, the rooting-invariance rationale, the <4-taxa clause) with the incoming link form (\href to the TreeTools reference page). Also narrowed T-380 in light of what came in: its unpack_xform tip_states half duplicates the already-filed T-344, now fixed by 70ae4d5. That fix guards only the tip_states length in unpack_xform -- it does not validate cost-matrix dimensions anywhere and does not touch ts_sankoff_test, which is what T-380 now covers. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
tree_fuse() had zero R_CheckUserInterrupt()/check_interrupt() calls in its round/donor loops, unlike every other search module. For a large pool-wide fuse (fuseInterval, sprint default 5) over a >100-tip pool, the O(n_donors * n_splits^2) donor scan could run for many seconds between other checked points, delaying Ctrl-C/Esc response well past the ~1s target reported by the user. Add ts::check_interrupt() at the top of the round loop and per-donor, which correctly maps to R_CheckUserInterrupt() (serial) or the shared atomic stop flag (parallel workers), matching the pattern used elsewhere (e.g. ts_ratchet.cpp, ts_tbr.cpp).
A user watching a 182-tip, 420-character matrix with inapplicable tokens throughout reported the search looked frozen. It was not. Measured, one sprint replicate takes 1173 s, of which TBR is 582 s and a 3-cycle ratchet 549 s -- 96% of the replicate in two phases that printed only on completion. Nothing reported progress for ~10 minutes at a stretch, and the finest granularity available was one line per replicate, i.e. one line per 20 minutes. Heartbeat. Long phases now report their running best and time in phase, overwriting a single line at a terminal and emitting discrete lines to a batch log. Cadence defaults to 30 s / 120 s respectively, override or disable with TS_HEARTBEAT_SECONDS (fractional values allowed; junk falls back to the default rather than disabling). Reporting is opt-in per call site via TBRParams::heartbeat_label, not opt-out. A caller is the only thing that knows whether a search's running best is on the user's objective: sectorial searches score a subtree, and the ratchet's perturbation phase scores a reweighted matrix. Both legitimately run far below the true optimum -- observed at 33 where the optimum was 79 -- so reporting them would read as erratic progress. Only whole-tree, real-weight searches are labelled, so any score printed is comparable with the final tree score. Thread safety: every entry point no-ops unless ts::thread_stop_flag is null, i.e. on the R main thread, so Rprintf is never reached from a worker. State is plain globals rather than thread_local, given the MinGW emutls corruption fixed in PR #253. The parallel path instead gains a wall-clock cadence on its existing coordinator poll, which was already the only thread touching R there, and which previously reported only when a replicate completed -- silent for hours on a large matrix, and indistinguishable from a hung job. Cost: the TBR hook piggybacks the existing per-clip interrupt poll at stride 64 rather than adding a clock read to the candidate loop, which would have reopened a hot path the profiling campaign left at-limit. The ratchet hook uses stride 1, since a cycle is coarse and a larger stride would silence it entirely on the deep ratchets implied weights asks for. Strategy partial matching. `strategy = "thoro"` previously warned "Unknown strategy" and silently fell back to `default`. Any unambiguous abbreviation now resolves; every preset's initial differs, so one letter suffices. Resolution happens BEFORE the preset lookup and before .IwRatchetDepth()/.IwStopPackage(), which test `strategy %in% c("thorough", "large")` by exact string -- resolving later would apply thorough's preset while skipping its implied-weights ratchet package, a worse failure than not matching because it looks like it worked. pmatch() rather than match.arg() so a real typo still warns instead of erroring. Also documents TREESEARCH_PROGRESS_FILE, which already existed in R/, the Shiny app and dev notes but appeared nowhere in man/ -- the one mechanism that gives a non-interactive run any progress at all, and it was undiscoverable. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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
1 participant
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.
Manual testing underway; shiny app in particular has some usability issues.