Fixed two critical bugs in get_joint_graph

[kaozkai]

1.) When clustering two repertoires with two chains, CDR3b edges between-repertoire got deleted due to typo in helper function get_ix()

2.) Duplicated edges removal with which_multiple removed also the CDR3b edge of double edges between-repertoire clones

Also, added test-multi-chain-clustering to testthat

Summary by CodeRabbit

• Bug Fixes

  • Fixed CDR3 edge clustering to properly identify identical sequences in all scenarios
  • Prevented unintended deletion of CDR3b edges between repertoires
  • Improved handling of double-edge cases in multi-chain clone comparison

• Tests

  • Added comprehensive test suite for multi-chain clustering validation

[coderabbitai]

Caution

Review failed

The pull request is closed.

Walkthrough

Bug fixes are applied to graph processing functions handling multi-chain CDR3 clustering, alongside new test cases validating the corrections and updates to release notes documenting the issues.

Changes

Cohort / File(s)	Summary
Graph Processing Fixes R/graph.R	Minor formatting adjustment in get_joint_graph; variable renamed from ixs to ix_s in get_intergraph_edges's helper get_ix with all appends updated accordingly, though return statement still references ixs (potential runtime error).
Release Notes inst/NEWS	Documents bug fixes for versions 1.9.9–1.9.10: get_blosum ensuring identical CDR3s form edges, get_joint_graph preventing CDR3b edge deletion and between-repertoire edge loss during dual-chain clustering.
Multi-Chain Clustering Tests tests/testthat/test-multi-chain-clustering.R	Adds two comprehensive test cases: within-repertoire and between-repertoire clustering with overlapping CDR3a/CDR3b clones, validating vertex and edge counts in resulting igraph objects.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

• Devel #42: Modifies get_intergraph_edges and its helper get_ix function in the same file, directly overlapping with the variable renaming logic.
• Fixed two critical bugs in get_joint_graph #44: Addresses the same functions (get_joint_graph and get_intergraph_edges) with overlapping bug fixes for multi-chain deduplication behavior.

Poem

🐰 A rabbit hops through graphs so grand,
Where CDR3s now firmly stand,
Two chains unite, no edges lost,
With tests to prove the bugs are crossed!
✨

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'Fixed two critical bugs in get_joint_graph' accurately summarizes the main changes: two bug fixes in the get_joint_graph function addressing CDR3b edge deletion issues.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Merge Conflict Detection	✅ Passed	✅ No merge conflicts detected when merging into main

_{✏️ 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_get_joint_graph

---

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

🤖 Fix all issues with AI agents

In `@R/graph.R`:
- Around line 224-232: The deduplication step using duplicated() misses reversed
pairs (A,B vs B,A) when knn=TRUE; normalize edge direction before deduplication
by creating ordered endpoints (e.g., v1 = pmin(from, to), v2 = pmax(from, to)),
use those columns (plus "chain" if present) to remove duplicates from df_e, then
drop the temporary columns before calling graph_from_data_frame; update the
block that references df_e and duplicated() and ensure compatibility with
get_ix/knn behavior.

[snaketron]

2.) Duplicated edges removal with which_multiple removed also the CDR3b edge of double edges between-repertoire clones

I don't get this point. Please explain in more detail. At the end of the graph joining we want to have a single edge (per chain) between two nodes. No?

[snaketron]

1.) When clustering two repertoires with two chains, CDR3b edges between-repertoire got deleted due to typo in helper function get_ix()

This is a good point!

[kaozkai]

2.) Duplicated edges removal with which_multiple removed also the CDR3b edge of double edges between-repertoire clones

I don't get this point. Please explain in more detail. At the end of the graph joining we want to have a single edge (per chain) between two nodes. No?

Yes, one edge per connected chain. What I noticed (and wanted to say) is:
if we have two edges between two identical clones between-repertoires,
the which_multiple() function does remove one of the two edges, because it only looks for the names and does not take the chain column into consideration.

I noticed this because the tests I wrote still failed for this case after the 1. point was corrected (see tests/testthat/test-multi-chain-clustering.R)