[vitali87]

This PR covers the two follow-ups left open after #244.

cp_sat gave every file its own PR

Under the default orthogonal priority, keeping two different files in one group cost 400 per file pair, and opening another group cost only 250. So the optimum was always one file per group. On this repo's 1,698-line PR #10, cp_sat produced 21 PRs, several of 3–6 lines, and it also ran into its 15-second limit.

Fix:

• Penalty only for unrelated files: only unrelated file pairs now pay the penalty, meaning pairs with no shared directory, symbols or test pairing. Related files get their affinity reward, so they share a PR when it fits within --max-loc. The existing test that keeps unrelated files apart still passes.
• Smaller model: there is now one variable per scored unit pair, with linear bounds. Before, there was one per pair per group slot, so variables grow as n² instead of n³.
• Fewer group slots: the number of slots is bounded by the graph plan's size plus a margin, instead of one per unit.
• Warm start: the graph plan seeds the search as a hint.
• Group order: groups are numbered in diff order instead of by arbitrary solver slot.

Results: the "objective" column is pr-split's own plan score, where lower is better.

Diff	Before	After
PR #10, 1,698 LOC	21 groups, one per file; hit the 15 s limit	8 groups (6 after the standalone pass); proven optimal in about 5 s
merge #243, 4,187 LOC	one group per file or hunk run	13 groups, only 1 tiny; objective 300 (the graph planner gives 24 groups, 11 tiny, objective 387)

The #243 diff still reaches the 15-second limit without proving optimality. Runs of up to 60 seconds give the same plan.

End to end: I split both diffs with cp_sat, ran execute against a local remote, and ran the full test suite on each branch. All 6 and all 13 sub-PRs pass their own test suite. Merging each set of branches back reproduces the original branch exactly.

Symbol matching was needlessly slow

symbol_dependencies rescanned every added line of the diff once for each name the diff defines (CodeRabbit's nitpick on #244). It now scans once and intersects the result.

Checks

ruff check, ruff format --check, ty check pr_split (18 diagnostics, down from 20 on main) and pytest (1,043 passed) all pass locally. There is one new regression test: under orthogonal, related files that fit within --max-loc share a PR. It fails without this change.

🤖 Generated with Claude Code

https://claude.ai/code/session_015rnEEjXWDBdePRhKMmZr1w

---

Generated by Claude Code

Summary by CodeRabbit

• Improvements
  • Improved how graph-based partitions are grouped, including keeping closely related files together and presenting groups in a consistent order.

[coderabbitai]

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: 7f9e3047-f3b5-4efc-96dc-b280b00cb512

📥 Commits

Reviewing files that changed from the base of the PR and between bc2b518 and 68cea74.

📒 Files selected for processing (3)

• pr_split/planner/partitioning.py
• pr_split/planner/symbols.py
• tests/test_partitioning_extensive.py

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

---

📝 Walkthrough

Walkthrough

The CP-SAT partitioner now uses graph partitioning results as solver hints, changes how it builds pairwise objective terms, and sorts output groups by diff position. A test checks grouping two small files in the same package. Symbol field selection now uses a set intersection.

Changes

CP-SAT Partitioning

Layer / File(s)	Summary
Solver grouping and objective pr_split/planner/partitioning.py, tests/test_partitioning_extensive.py	The solver uses graph-derived group hints and builds pair variables only for positively affiliated pairs or eligible unrelated cross-file pairs. It retains group-count and size costs, then orders solved groups by the earliest unit’s diff position. The test checks that two small files in one package form a group.

Symbol Field Selection

Layer / File(s)	Summary
Field selection expression pr_split/planner/symbols.py	The fields set now uses the intersection of owner and _new_fields(added).

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix

Suggested reviewers: claude

Merge Risk: ⚪ Minimal · up to 68cea

The changed grouping path has no identified issue requiring a fix before merge. Merge after normal checks.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 68cea

The grouping change affects saved plans and the pull requests they produce, but no new execution authority or security boundary bypass was identified. Some failure and recovery behavior remains unverified.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

• inferred — The changed grouping can alter branches and PRs produced by an authorized execution, but the changed helper does not itself gain Git or GitHub authority.

Trust Boundaries and Controls

• observed — The public-entrypoint-labelled changed test calls the partitioner with a synthetic diff; it is not itself a new production entrypoint.

Resilience and Maintainability Implications

• inferred — Materialization unions assigned hunk indices and carries ancestor hunks into dependent groups. Inspection did not establish a new failure caused by non-contiguous same-file assignments, but did not prove every such plan materializes correctly.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 20.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 3 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ 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 accurately describes the main change: preventing CP-SAT from placing every file in a separate group within a pull request.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

• Fix all pre-merge checks with AI

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

• Commit to this branch
• Create a new PR

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

---

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.}