Skip to content

test: cover chrX haploid-male counting and MNV overlap branches#95

Merged
ameynert merged 1 commit into
mainfrom
test/haplotype-core-coverage
Jun 3, 2026
Merged

test: cover chrX haploid-male counting and MNV overlap branches#95
ameynert merged 1 commit into
mainfrom
test/haplotype-core-coverage

Conversation

@ameynert

@ameynert ameynert commented Jun 3, 2026

Copy link
Copy Markdown
Collaborator

Summary

Test coverage and observability for the haplotype core, plus a small readability fix. No behavior change (the _haploid_adjusted_call extraction and the haplo_coordinates sort-key unification are both behavior-preserving).

Changes

#78 — chrX haploid-male test. Extracted the inline chrX non-PAR haploid adjustment into _haploid_adjusted_call(locus, gt, sex_karyotype) (the subtlest correctness logic in compute_haplotypes, previously only guarded by opaque end-to-end counts). Added a focused test on a 2×2 synthetic MatrixTable: a chrX non-PAR male alt-carrier contributes one allele to call_stats (AN 3 = male haploid + female diploid), while the autosome locus stays diploid (AN 4).

#77 — observability. _compute_locus_groups now logs the number of variants it collects to the driver, so a memory spike at that (currently un-mitigated) collect + hl.literal broadcast is attributable — matching the existing attach_component_info / containment_dedup collect-site logs.

#83 — nits.

  • Added the two untested MNV classify_pair branches (same_position_other, other_overlap) to the parametrized table.
  • Unified haplo_coordinates's sort key with get_haplo_sequence's (position, len(ref)) (only the minimum position is read, and _max_reference_end is order-independent, so this is behavior-preserving) and added a comment noting the two must stay in lockstep.

Verification

  • uv run --directory divref poe fix-and-check-all — 184 passed; ruff (check + format) and mypy clean.

Closes #77
Closes #78
Closes #83

🤖 Generated with Claude Code

@coderabbitai

coderabbitai Bot commented Jun 3, 2026

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

@ameynert, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 1 minute and 42 seconds. Learn how PR review limits work.

Your organization has run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 6d7b3ba4-47b8-4d72-bfe5-493bafcd155d

📥 Commits

Reviewing files that changed from the base of the PR and between 1736c06 and 12c7d19.

📒 Files selected for processing (4)
  • divref/divref/haplotype.py
  • divref/divref/tools/compute_haplotypes.py
  • divref/tests/test_haplotype_compat.py
  • divref/tests/tools/test_compute_haplotypes.py

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Base automatically changed from am_review to main June 3, 2026 21:26
- Extract _haploid_adjusted_call from compute_haplotypes and unit-test that a
  chrX non-PAR male alt-carrier contributes a single allele to call_stats (AN),
  while females and autosome calls stay diploid (#78).
- Log the collected variant count in _compute_locus_groups so a driver-memory
  spike at that collect/broadcast is attributable in the logs (#77).
- Cover the MNV classify_pair branches (same_position_other, other_overlap),
  and unify the haplo_coordinates sort key with get_haplo_sequence's to remove
  the latent divergence (#83).

Closes #77
Closes #78
Closes #83

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@ameynert
ameynert force-pushed the test/haplotype-core-coverage branch from 7a09a0c to 12c7d19 Compare June 3, 2026 22:22
@ameynert
ameynert temporarily deployed to github-actions-snakemake-linting June 3, 2026 22:22 — with GitHub Actions Inactive
@ameynert

ameynert commented Jun 3, 2026

Copy link
Copy Markdown
Collaborator Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jun 3, 2026

Copy link
Copy Markdown
Contributor
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@ameynert
ameynert merged commit d409766 into main Jun 3, 2026
4 checks passed
@ameynert
ameynert deleted the test/haplotype-core-coverage branch June 3, 2026 23:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant