fix: handle non-canonicalizable breakpoint at same position

[nh13]

Summary

• When both breakends are at the same genomic position with leftPositive=false, rightPositive=true, reversed() produces an identical breakpoint (a fixed-point of the reversal operation)
• isCanonical previously returned false for this case, causing an unnecessary and ineffective reversal
• Fix: accept any same-position breakpoint where at least one strand is positive, since these are fixed-points that cannot be further canonicalized
• Only (false, false) at same position is non-canonical, which correctly reverses to (true, true)
• Adds test covering all four strand combinations at same position

Test plan

• Existing tests pass (./mill tools.test)
• New test verifies all strand combinations at same position

[coderabbitai]

Warning

Rate limit exceeded

@nh13 has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 18 minutes and 40 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, 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 have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

📝 Walkthrough

Walkthrough

The pull request updates the breakpoint canonicality logic in Breakpoint.scala. The isCanonical method now evaluates equality cases differently—requiring leftRefIndex < rightRefIndex, or equal indices with leftPos < rightPos, or equal positions with at least one positive strand. Tests are revised to verify isCanonical behavior across strand combinations and add tests for Breakpoint.apply with varying segment lengths. A documentation version string is also updated.

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately describes the main fix: handling non-canonicalizable breakpoints at the same position, which aligns with the core logic change in isCanonical.
Description check	✅ Passed	The description clearly explains the problem, fix, and test coverage, all directly related to the changes in the breakpoint canonicalization logic.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch fix/breakpoint-canonicalization-edge-case

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (1)

src/test/scala/com/fulcrumgenomics/sv/BreakpointTest.scala (1)

90-93: Consider asserting the fixed-point property for (false, true) directly.

The test confirms (false, true).isCanonical == true, but not that reversed actually returns the identical breakpoint — the root cause of the bug. Adding that assertion makes the regression guard explicit.

✏️ Proposed addition

     // (false, true) at same position: canonical (fixed-point of reversed)
     base.copy(leftPositive=false, rightPositive=true).isCanonical  shouldBe true
+    // verify the fixed-point: reversed is identical
+    val fixedPoint = base.copy(leftPositive=false, rightPositive=true)
+    fixedPoint.reversed shouldBe fixedPoint
     // (false, false) at same position: NOT canonical, reversed gives (true, true)

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/test/scala/com/fulcrumgenomics/sv/BreakpointTest.scala` around lines 90 -
93, Add an assertion to explicitly test the fixed-point property for the
(leftPositive=false, rightPositive=true) case: after creating the breakpoint via
base.copy(leftPositive=false, rightPositive=true) assert that calling .reversed
returns an equal breakpoint (i.e., the breakpoint equals its .reversed) in
addition to the existing isCanonical check; locate the relevant test using base,
leftPositive/rightPositive, isCanonical and reversed to add this equality
assertion.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/test/scala/com/fulcrumgenomics/sv/BreakpointTest.scala`:
- Around line 82-94: The test uses ambiguous "F" in the description and comments
which elsewhere denotes "forward" not "false"; update the test description and
inline comments in the Breakpoint.isCanonical test to use unambiguous terms
(e.g. "true/false" or "T/F (positive/negative)" or spell out "positive/negative
strand") and adjust the sentence `"treat all strand combinations at the same
position as canonical except (F, F)"` to explicitly state which pair is
non-canonical (e.g. "except (false, false) / both negative"); ensure references
to leftPositive and rightPositive and the base.copy cases remain the same so
Breakpoint.isCanonical and reversed logic are unchanged.

---

Nitpick comments:
In `@src/test/scala/com/fulcrumgenomics/sv/BreakpointTest.scala`:
- Around line 90-93: Add an assertion to explicitly test the fixed-point
property for the (leftPositive=false, rightPositive=true) case: after creating
the breakpoint via base.copy(leftPositive=false, rightPositive=true) assert that
calling .reversed returns an equal breakpoint (i.e., the breakpoint equals its
.reversed) in addition to the existing isCanonical check; locate the relevant
test using base, leftPositive/rightPositive, isCanonical and reversed to add
this equality assertion.