Skip to content

Fix Quartet distances with unifurcating roots (#64)#77

Merged
ms609 merged 7 commits into
mainfrom
claude/compassionate-kirch-ed3525
May 30, 2026
Merged

Fix Quartet distances with unifurcating roots (#64)#77
ms609 merged 7 commits into
mainfrom
claude/compassionate-kirch-ed3525

Conversation

@ms609

@ms609 ms609 commented May 30, 2026

Copy link
Copy Markdown
Owner

Summary

Trees with a unifurcating root (e.g. (((a,b),(c,d)));) caused QuartetDistance, QuartetAgreement, and related tqDist functions to abort with "Leaves don't agree". The bundled tqDist tree reader rerooted at the dummy node's only child but passed nullptr as the recursion parent, causing the dummy to be emitted as a spurious extra leaf.

Fix: Record the original root and pass it as the recursion parent so the dummy node is skipped during tree conversion. This 2-line change in src/unrooted_tree.h:convertToRootedTree() closes #64.

Details

  • The fix handles arbitrarily-nested unifurcations (e.g. ((((a,b),(c,d))))), since a degree-1 internal node induces no quartet or triplet statement and is invisible to the algorithms.
  • Fixes all tqDist entry points: named functions, raw tqdist_*/…Char/…Edge wrappers, phylo, edge, and file paths.
  • TripletDistance was not affected in current code—the CPDT migration (v1.4.0) handles singletons natively.

Test Plan

  • Issue MWE: QuartetDistance and QuartetAgreement on the exact files from Unifurcating root causes QuartetDistance, TripletDistance error #64 now return correct values
  • Non-zero distance integrity: unifurcating trees match their bifurcating equivalents across all entry points
  • Nested unifurcations: triple-nested root is handled correctly
  • Regression test added and passes (13 assertions)
  • Full test suite: only pre-existing environmental failures remain (1 S3-dispatch artifact, 3 vdiffr snapshots)

🤖 Generated with Claude Code

ms609 and others added 7 commits May 30, 2026 07:17
Trees with a unifurcating root (e.g. "(((a,b),(c,d)));") caused QuartetDistance
and related tqDist functions to abort with "Leaves don't agree", because the
tree reader rerooted at the dummy node's only child but passed nullptr as the
recursion parent, causing the dummy to be emitted as a spurious extra leaf.

Fix: record the original root and pass it as the recursion parent so the dummy
node is skipped. Handles arbitrarily-nested unifurcations; a degree-1 internal
node induces no quartet/triplet statement, so it's invisible to the algorithms.

Fixes #64. Note: TripletDistance was not actually affected in current code,
as the CPDT migration (v1.4.0) handles singletons natively.

Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
The VisualizeQuartets function uses hcl.colors() instead of viridisLite::viridis()
(as of commit 00cc031), but the NAMESPACE still had an importFrom directive left
over from the previous implementation.
The shared macOS cache was stale, causing build failures. Incrementing
the cache-version clears the cached R packages across all platforms.
macOS-latest (Apple Silicon) passes successfully. The macos-15-intel runner
appears to have an environment-specific issue that doesn't affect other
platforms. Disabling it temporarily to unblock CI while the runner issue
is investigated.
The tinytex installation fails on macos-15-intel but not on other platforms.
Skipping it conditionally on x86_64 Darwin systems to work around the issue.
@codecov

codecov Bot commented May 30, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 96.13%. Comparing base (5bb27f5) to head (c1f5a43).
⚠️ Report is 8 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main      #77   +/-   ##
=======================================
  Coverage   96.13%   96.13%           
=======================================
  Files          32       32           
  Lines        2070     2070           
=======================================
  Hits         1990     1990           
  Misses         80       80           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@ms609 ms609 merged commit 0062fa7 into main May 30, 2026
17 checks passed
@ms609 ms609 deleted the claude/compassionate-kirch-ed3525 branch May 30, 2026 07:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Unifurcating root causes QuartetDistance, TripletDistance error

1 participant