T-302: fix LengthAdded negative delta (qmApp scalar)#244
Merged
Conversation
…(T-302) Root cause: when multiple rows in the contrast matrix satisfy the fully-ambiguous applicable condition, qmApp was left as a vector of indices. Assigning a multi-element vector as a leaf token caused TreeLength() to receive an invalid state, producing scores higher than the original and yielding negative deltas. Fix: take qmApp[[1L]] so the leaf always receives a single valid token index. Also tighten the zero-sum contrast-row guard to only error when the affected tokens are actually used by taxa in the character being scored (#294). Removes the Temp warning from R/PolEscapa.R:104. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Add expect_error assertion with Asher2005 char 67 (6-token char) confirming the message says "token(s) 6" when row 6 is zeroed. - Restore # Temp warning for deltas < 0; remove once fix is proven robust across edge-case datasets. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
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>
R CMD check on this PR hangs in '* checking examples ...' on
ubuntu-24.04-arm, ubuntu-24.04 (devel) and macOS-latest (most recent
in-flight + 3 cancelled runs). R CMD check emits no per-example
markers, so we cannot tell which Rd is the culprit from the logs.
This diagnostic-only commit inserts message("PR244-DIAG: BEGIN/END
<Rd>") into the \examples{} block of the six likeliest suspects:
the Rds that exercise Vinther2008 or run a parallel search. The
LAST BEGIN line with no matching END in the CI log identifies the
hang. Will be reverted before merge.
NOT a structural fix.
Previous diagnostic used message() calls; R CMD check buffers
example stderr to *-Ex.Rout files that are only echoed to the
visible log after the examples stage completes. On hung runs the
stage never completes, so message() markers never appeared.
Switch to file-based markers: each \examples{} block opens with a
cat() that writes a timestamped line to ~/pr244-diag.log. This
survives the cancel signal. Extended to all 49 Rds (so the LAST
"PR244-DIAG: BEGIN <Rd>" line names the hung example regardless of
where in the alphabet it falls). Add an if: always() workflow
step that cats ~/pr244-diag.log + any -Ex.Rout fragment.
Diagnostic-only; revert before merge.
ms609
added a commit
that referenced
this pull request
May 19, 2026
Adds NA-aware dirty-set Pass 1 + Pass 2 (fitch_na_dirty_downpass and fitch_na_dirty_uppass) mirroring the EW dirty-set, and wires them into tbr_search's SPR accept path under has_inapplicable. Pass 3 still runs over the full tree because it populates internal down2 (read by extract_char_steps) and counts NA-block steps; savings come from skipping Pass 1 + Pass 2 on off-dirty nodes. Bug found during validation: full_rescore for NA EW data routes through fitch_score_ew which adds ds.ew_offset (topology-independent step count) on top of fitch_na_score's return. My accept path was reading fitch_na_pass3_score directly without the offset — producing a systematic diff=−3 on Vinther2008 (where ew_offset = 3). Fix: add ds.ew_offset in the EW branch (line 1183). IW path is unaffected because compute_weighted_score handles per-pattern step contributions directly. Validation: DEBUG_NA_RESCORE cross-check kept for one GHA cycle. Local Vinther2008 run, 5 replicates each EW + IW: DEBUG_RESCORE (EW, IW): 0 / 0 mismatches DEBUG_NA_RESCORE (NA-EW, NA-IW): 0 / 0 mismatches Also includes dev/plans/t300-test.R (the four-path test harness) and dev/plans/pr244-examples-hang-brief.md (background-agent brief for the separate PR #244 hang investigation). Co-Authored-By: Claude Opus 4.7 <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.
Summary
deltas < 0inPolEscapa.R:104for ~19-taxon datasetsqmAppmatrix was not being reduced to a scalar before subtraction inLengthAdded# Tempwarning guard once the root cause is handled correctlyTest plan
🤖 Generated with Claude Code