feat: stacked PR mode with native GitHub stack linking and draft PRs - #42
Conversation
|
Warning Review limit reached
Next review available in: 13 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughThe CLI adds stacked dependency branches, draft PR creation, native stack linking, and saved-plan overrides. DAG and diff reconstruction support linear chains and ancestor hunks, while schemas, GitHub operations, tests, and documentation cover the new behavior. ChangesStacked PR workflow
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Developer
participant CLI
participant PlanDAG
participant Git
participant GitHub
Developer->>CLI: run split or execute with --stack/--draft
CLI->>PlanDAG: resolve dependency-ready groups and chains
PlanDAG->>Git: create branches from parent refs
Git-->>CLI: return branch and commit results
CLI->>Git: push successful branches
CLI->>GitHub: create draft or regular PRs
CLI->>GitHub: link linear PR stacks
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
Greptile SummaryAdds stacked and draft sub-PR creation, composes ancestor changes for stacked branches, persists successfully created PR records after partial failures, and improves whole-file coverage handling. Saved-plan execution was exercised with corrupted persisted plans. Overlapping assignments are rejected, but a plan with complete coverage and cyclic dependencies still reaches branch creation rather than being rejected before repository mutation. This should be corrected before merge. Confidence Score: 3/5Not safe to merge until T-Rex findings are addressed. The saved-plan execution flow accepts a cyclic dependency graph and reaches the branch-creation boundary. This was reproduced using the real persisted plan format and execute command with external side effects isolated. T-Rex reproduced 2 failing behaviors at runtime in pr_split/cli.py; the change needs fixes before it is safe to merge. Files Needing Attention: pr_split/cli.py needs complete plan validation in execute before presenting the plan, requesting confirmation, or creating branches.
What T-Rex did
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
tests/test_schemas.py (1)
55-70: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd equivalent
draftpersistence coverage.
draftis persisted alongsidestacked, but onlystackedgets default and round-trip assertions. Adddraft=Falseanddraft=Truecases.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_schemas.py` around lines 55 - 70, Add equivalent draft persistence coverage in TestSplitPlanStacked: extend the default case to assert draft defaults to False and the plan-file round-trip case to construct with draft=True and assert it remains True after model validation.tests/test_cli_helpers.py (1)
245-360: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a 3-level chain test for transitive ancestor-hunk merging.
Current stacked tests only cover depth-2 (single parent → child) merging. The trickiest part of
_stacked_batch_argsis thateffective[parent]already carries hunks merged from its ancestors, so a grandchild's merge should pick up hunks from both its parent and grandparent transitively. A 3-level chain test (grandparent → parent → child, each touching the same file) would directly validate this composition.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_cli_helpers.py` around lines 245 - 360, Extend TestCreateBranchesAndCommitsStacked with a three-level stacked chain where grandparent, parent, and child each assign distinct partial hunks to the same file. Invoke _create_branches_and_commits in stacked mode, inspect materialize_group_files calls for the child, and assert its effective assignment includes all three hunk indices, validating transitive ancestor-hunk merging.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@pr_split/cli.py`:
- Around line 412-448: Update _push_and_create_prs so PR creation only includes
a group when its own branch and every ancestor/base branch required by the stack
were pushed successfully. Propagate push failure through the parent relationship
before submitting _create_single_pr, while preserving the existing error
collection and successful independent branches.
---
Nitpick comments:
In `@tests/test_cli_helpers.py`:
- Around line 245-360: Extend TestCreateBranchesAndCommitsStacked with a
three-level stacked chain where grandparent, parent, and child each assign
distinct partial hunks to the same file. Invoke _create_branches_and_commits in
stacked mode, inspect materialize_group_files calls for the child, and assert
its effective assignment includes all three hunk indices, validating transitive
ancestor-hunk merging.
In `@tests/test_schemas.py`:
- Around line 55-70: Add equivalent draft persistence coverage in
TestSplitPlanStacked: extend the default case to assert draft defaults to False
and the plan-file round-trip case to construct with draft=True and assert it
remains True after model validation.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: a0f9615f-c60b-4744-876d-f6a0e19fd6c2
📒 Files selected for processing (13)
README.mdpr_split/cli.pypr_split/diff_ops/__init__.pypr_split/diff_ops/reconstructor.pypr_split/git_ops/prs.pypr_split/graph.pypr_split/logs.pypr_split/schemas.pytests/test_cli_helpers.pytests/test_git_prs.pytests/test_graph.pytests/test_reconstructor.pytests/test_schemas.py
…te PRs on pushed bases
|
@greptile review |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
pr_split/cli.py (1)
423-485: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftPR-creation failures aren't propagated to dependent children (distinct from the already-fixed push-failure gap).
_push_and_create_prsnow correctly gates PR creation on push success via_base_pushed(past issue fixed). However, the second phase still submits_create_single_prfor every group whose base was pushed inside a single flatThreadPoolExecutorbatch, with no ordering by dependency. If a parent's own_create_single_prcall fails (e.g. a transientgh pr createerror) after its branch was successfully pushed, its child is still submitted concurrently and can succeed — producing a child PR whose base branch has no corresponding PR.PRCreationErrorthen aborts before_link_stacksruns, so this broken chain persists on GitHub with no automatic remediation. None of the added tests exercise a dependent (parent/child) PR-creation failure —test_partial_pr_records_ride_on_the_errorandtest_child_pr_skipped_when_parent_push_failsboth use independent groups.Consider processing PR creation in dependency-ordered batches (similar to
_create_branches_and_commits's use ofdag.iter_ready()), only submitting a group once its base's PR (if part of the stack) has been created successfully.Do you want me to draft the batched implementation and a regression test for this scenario?
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pr_split/cli.py` around lines 423 - 485, The PR creation phase must propagate parent PR failures to dependent children instead of submitting all eligible groups concurrently. Update _push_and_create_prs to process groups in dependency-ordered batches, using the stack DAG or equivalent readiness tracking, and submit a group only after its base branch’s owning group has created a PR successfully; retain independent-group concurrency and existing error aggregation behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@pr_split/cli.py`:
- Around line 423-485: The PR creation phase must propagate parent PR failures
to dependent children instead of submitting all eligible groups concurrently.
Update _push_and_create_prs to process groups in dependency-ordered batches,
using the stack DAG or equivalent readiness tracking, and submit a group only
after its base branch’s owning group has created a PR successfully; retain
independent-group concurrency and existing error aggregation behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 82fef5a8-819b-45bd-bc99-28cbee085267
📒 Files selected for processing (8)
README.mdpr_split/cli.pypr_split/diff_ops/reconstructor.pypr_split/exceptions.pypr_split/logs.pytests/test_cli_helpers.pytests/test_reconstructor.pytests/test_schemas.py
🚧 Files skipped from review as they are similar to previous changes (2)
- README.md
- pr_split/logs.py
|
@greptile review |
| try: | ||
| validate_coverage(plan.groups, parsed_diff) | ||
| except PlanValidationError as exc: | ||
| console.print(f"[red]{exc}[/red]") | ||
| raise typer.Exit(1) from exc |
There was a problem hiding this comment.
Saved-plan cycle validation is skipped
execute only calls validate_coverage, so a saved dry-run plan with complete, non-overlapping coverage but a dependency cycle is accepted and reaches branch creation. A manually edited or otherwise corrupted .pr-split/plan.json can therefore begin mutating the repository instead of failing as an invalid plan. Construct PlanDAG(plan.groups) and run the complete validate_plan path before presenting the plan or requesting confirmation.
Artifacts
Focused saved-plan execute validation regression harness
- Authored harness writes corrupted saved plans and invokes the real execute command while isolating external preconditions and the mutation boundary, proving the validation scope.
Parent revision corrupted saved-plan execution log
- Executed the harness against the parent revision; both cyclic and overlapping plans reached the mocked branch-creation boundary, showing the pre-PR behavior.
PR head corrupted saved-plan execution log
- Executed the harness against the PR head; overlapping assignments are rejected but a cyclic plan still reaches the mocked branch-creation boundary, proving the remaining defect.
Ran code and verified through T-Rex
Prompt To Fix With AI
This is a comment left during a code review.
Path: pr_split/cli.py
Line: 1045-1049
Comment:
**Saved-plan cycle validation is skipped**
`execute` only calls `validate_coverage`, so a saved dry-run plan with complete, non-overlapping coverage but a dependency cycle is accepted and reaches branch creation. A manually edited or otherwise corrupted `.pr-split/plan.json` can therefore begin mutating the repository instead of failing as an invalid plan. Construct `PlanDAG(plan.groups)` and run the complete `validate_plan` path before presenting the plan or requesting confirmation.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
Adds --stack so dependent sub-PRs branch from and target their parent's branch, with linear chains registered as native GitHub stacks via gh-stack, plus --draft for draft sub-PRs. Also fixes added-file materialisation double-spacing every new file.
Summary by CodeRabbit
--stackand--draftCLI options, including when executing saved plans, plus matching configuration via environment variables.