Skip to content

fix: raise PRSplitError for chunking failures - #71

Merged
vitali87 merged 4 commits into
mainfrom
fix/chunker-errors-are-pr-split-errors
Aug 31, 2026
Merged

fix: raise PRSplitError for chunking failures#71
vitali87 merged 4 commits into
mainfrom
fix/chunker-errors-are-pr-split-errors

Conversation

@vitali87

@vitali87 vitali87 commented Aug 30, 2026

Copy link
Copy Markdown
Owner

Problem

chunk_hunks_greedy / chunk_hunks_dynamic_programming raise a bare ValueError when a single hunk's token estimate exceeds the per-chunk budget (ErrorMsg.HUNK_TOO_LARGE), and chunk_hunks does the same for an unknown strategy. Neither is a PRSplitError, so on a huge diff with one giant hunk the split command dies with a traceback instead of the one-line message every other planner failure gets (and #63's planner error boundary can't catch it either).

Fix

All four raises in pr_split/planner/chunker.py now use PRSplitError (the messages are unchanged and already come from ErrorMsg). Tests updated and a new test asserts both strategies raise PRSplitError with the file/hunk/budget in the message.

445 tests pass, ruff clean.

Summary by CodeRabbit

  • Bug Fixes
    • Chunking failures now consistently report the application’s standard error type.
    • Improved consistency for oversized changes, unsupported chunking strategies, and reconstruction failures.

@coderabbitai

coderabbitai Bot commented Aug 30, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: f11c52e0-6d78-4622-980a-239049f193e7

📥 Commits

Reviewing files that changed from the base of the PR and between e8e81d6 and ba17cb2.

📒 Files selected for processing (2)
  • pr_split/planner/chunker.py
  • tests/test_chunker.py

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


📝 Walkthrough

Walkthrough

Chunking error paths now raise PRSplitError instead of ValueError. Tests verify this behavior for oversized hunks in both strategies, dynamic-programming reconstruction failures, and unknown strategies.

Changes

Chunking error handling

Layer / File(s) Summary
Use project-specific chunking errors
pr_split/planner/chunker.py
The chunker raises PRSplitError for oversized hunks, failed dynamic-programming reconstruction, and unknown strategies.
Verify exception behavior
tests/test_chunker.py
Existing expectations now use PRSplitError. New coverage checks oversized hunks for greedy and dynamic-programming strategies.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to ba17c

Chunking failures now use the project’s standard error type, so oversized hunks and unknown strategies produce the intended concise error instead of an uncaught traceback. No actionable merge-blocking risk remains after normal checks and review.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 9 functions across 2 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 clearly and concisely describes the main change: chunking failures now raise the project-specific PRSplitError.
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 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/chunker-errors-are-pr-split-errors

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.

A hunk larger than the chunk token budget (and an unknown chunk
strategy) raised a bare ValueError from the chunker, which bypasses the
CLI's PRSplitError boundary and reaches the user as a traceback.
@vitali87
vitali87 force-pushed the fix/chunker-errors-are-pr-split-errors branch from ba17cb2 to f51deb7 Compare August 30, 2026 13:05
@vitali87
vitali87 merged commit 2e946a3 into main Aug 31, 2026
4 checks passed
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.

1 participant