Skip to content

Commit cae09bc

Browse files
authored
Merge branch 'main' into fix/cp-sat-feasible-warning
2 parents 7572bf2 + d3e6169 commit cae09bc

10 files changed

Lines changed: 421 additions & 21 deletions

File tree

README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,7 @@ Shows a table with each sub-PR's ID, title, branch, PR number, live state (OPEN/
115115
pr-split merge
116116
```
117117

118-
Walks the dependency DAG and merges each PR in topological order. Skips already-merged, closed, draft, review-required, or changes-requested PRs. Stops if a merge fails or a dependency wasn't merged to prevent out-of-order merges.
118+
Walks the dependency DAG and merges each PR in topological order. Skips already-merged, closed, draft, review-required, or changes-requested PRs, and every PR whose dependency was not merged in this run (independent subtrees still proceed). Stops if a merge fails. Exits 1 whenever a merge failed or any PR was left blocked, so re-run once the blocking PRs are ready.
119119

120120
Use `--auto` to queue merges behind CI checks (uses `gh pr merge --auto`):
121121

pr_split/cli.py

Lines changed: 55 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@
6666
push_branch,
6767
remove_worktree,
6868
)
69-
from .git_ops.branches import run_git
69+
from .git_ops.branches import commit_exists, run_git
7070
from .git_ops.prs import close_pr, create_pr, get_pr_state, link_stack, merge_pr
7171
from .graph import PlanDAG
7272
from .plan_store import load_plan, plan_exists, save_plan
@@ -948,9 +948,13 @@ def split(
948948
typer.confirm("Proceed with creating branches and PRs?", abort=True)
949949

950950
namespace = derive_split_namespace(dev_branch_arg)
951-
branch_records = _create_branches_and_commits(
952-
groups, parsed_diff, base, merge_base_ref, namespace, author=author, stacked=stack
953-
)
951+
try:
952+
branch_records = _create_branches_and_commits(
953+
groups, parsed_diff, base, merge_base_ref, namespace, author=author, stacked=stack
954+
)
955+
except PRSplitError as exc:
956+
console.print(f"[red]{exc}[/red]")
957+
raise typer.Exit(1) from exc
954958
try:
955959
pr_records = _push_and_create_prs(groups, branch_records, draft=draft)
956960
except PRCreationError as exc:
@@ -1125,6 +1129,13 @@ def execute(
11251129
" Re-run 'pr-split split --dry-run' to regenerate.[/red]"
11261130
)
11271131
raise typer.Exit(1)
1132+
if not commit_exists(plan.merge_base_sha):
1133+
console.print(
1134+
f"[red]Plan's merge base {plan.merge_base_sha} is not in this repository "
1135+
"(plan copied from another checkout, or history rewritten). "
1136+
"Fetch it or re-run 'pr-split split --dry-run' to regenerate.[/red]"
1137+
)
1138+
raise typer.Exit(1)
11281139

11291140
if not branch_exists(plan.base_branch):
11301141
console.print(f"[red]{ErrorMsg.BRANCH_NOT_FOUND(branch=plan.base_branch)}[/red]")
@@ -1157,15 +1168,19 @@ def execute(
11571168
typer.confirm("Proceed with creating branches and PRs?", abort=True)
11581169

11591170
namespace = derive_split_namespace(plan.dev_branch_arg or plan.dev_branch)
1160-
branch_records = _create_branches_and_commits(
1161-
plan.groups,
1162-
parsed_diff,
1163-
plan.base_branch,
1164-
plan.merge_base_sha,
1165-
namespace,
1166-
author=plan.author,
1167-
stacked=plan.stacked,
1168-
)
1171+
try:
1172+
branch_records = _create_branches_and_commits(
1173+
plan.groups,
1174+
parsed_diff,
1175+
plan.base_branch,
1176+
plan.merge_base_sha,
1177+
namespace,
1178+
author=plan.author,
1179+
stacked=plan.stacked,
1180+
)
1181+
except PRSplitError as exc:
1182+
console.print(f"[red]{exc}[/red]")
1183+
raise typer.Exit(1) from exc
11691184
try:
11701185
pr_records = _push_and_create_prs(plan.groups, branch_records, draft=plan.draft)
11711186
except PRCreationError as exc:
@@ -1277,6 +1292,7 @@ def merge_all(
12771292
merged: list[str] = []
12781293
skipped: list[str] = []
12791294
skipped_ids: set[str] = set()
1295+
blocked: list[str] = []
12801296
failed: list[str] = []
12811297

12821298
stopped = False
@@ -1305,6 +1321,18 @@ def merge_all(
13051321
merged.append(group_id)
13061322
continue
13071323

1324+
# Checked after the live state so a PR that already merged on GitHub
1325+
# (e.g. into a still-open parent branch) counts as merged rather
1326+
# than being reported as blocked.
1327+
unmerged_parents = [dep for dep in dag.parents(group_id) if dep not in merged]
1328+
if unmerged_parents:
1329+
deps = ", ".join(unmerged_parents)
1330+
logger.warning(f"{group_id} depends on unmerged {deps}, skipping")
1331+
skipped_ids.add(group_id)
1332+
blocked.append(group_id)
1333+
skipped.append(f"{group_id} (dependency {deps} not merged)")
1334+
continue
1335+
13081336
if state != "OPEN":
13091337
logger.warning(f"PR #{pr_record.pr_number} ({group_id}) is {state}, skipping")
13101338
skipped_ids.add(group_id)
@@ -1366,9 +1394,20 @@ def merge_all(
13661394
console.print(f"[yellow]Skipped ({len(skipped)}): {', '.join(skipped)}[/yellow]")
13671395
if failed:
13681396
console.print(f"[red]Failed ({len(failed)}): {', '.join(failed)}[/red]")
1397+
if blocked:
1398+
console.print(
1399+
f"[yellow]Blocked by unmerged dependencies ({len(blocked)}): "
1400+
f"{', '.join(blocked)}. Re-run once those PRs are merged.[/yellow]"
1401+
)
13691402
if notify:
13701403
exit_reason = (
1371-
"merge_error" if stopped else "incomplete_batch" if exited_early else "success"
1404+
"merge_error"
1405+
if stopped
1406+
else "incomplete_batch"
1407+
if exited_early
1408+
else "unmerged_dependency"
1409+
if blocked
1410+
else "success"
13721411
)
13731412
skipped_structured = [{"id": s.split(" (")[0], "reason": s} for s in skipped]
13741413
_send_webhook(
@@ -1378,11 +1417,11 @@ def merge_all(
13781417
"merged": merged,
13791418
"skipped": skipped_structured,
13801419
"failed": failed,
1381-
"success": not (failed or stopped or exited_early),
1420+
"success": not (failed or stopped or exited_early or blocked),
13821421
"exit_reason": exit_reason,
13831422
},
13841423
)
13851424

1386-
if failed or stopped or exited_early:
1425+
if failed or stopped or exited_early or blocked:
13871426
raise typer.Exit(1)
13881427
logger.success(f"Merge complete: {len(merged)} PRs merged")

pr_split/exceptions.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,10 @@ class ErrorMsg(StrEnum):
2626
MERGE_CONFLICT = "Groups '{a}' and '{b}' modify overlapping regions in '{file}'"
2727
NO_PLAN = "No split plan found; run 'pr-split split' first"
2828
LLM_PARSE_ERROR = "Failed to parse LLM response: {detail}"
29+
LLM_OUTPUT_TRUNCATED = (
30+
"LLM response was cut off before the plan was complete ({detail});"
31+
" the partial plan cannot be trusted"
32+
)
2933
BRANCH_CREATE_FAILED = "Failed to create branch '{branch}': {detail}"
3034
PR_CREATE_FAILED = "Failed to create PR for group '{group}': {detail}"
3135
MERGE_FAILED = "Merge of '{source}' into '{target}' failed: {detail}"

pr_split/git_ops/branches.py

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,15 @@ def run_git_in_dir(cwd: str, *args: str) -> str:
3232
return result.stdout.strip()
3333

3434

35+
def commit_exists(ref: str) -> bool:
36+
"""True if ``ref`` resolves to a commit object in this repository."""
37+
try:
38+
run_git("cat-file", "-e", f"{ref}^{{commit}}")
39+
except GitOperationError:
40+
return False
41+
return True
42+
43+
3544
def branch_exists(branch: str) -> bool:
3645
try:
3746
run_git("rev-parse", "--verify", branch)

pr_split/logs.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@
4444
LLM_OUTPUT_TRUNCATED = (
4545
"LLM output truncated (stop_reason: {stop_reason}), keys in partial output: {keys}"
4646
)
47+
LLM_OUTPUT_INCOMPLETE = "LLM output incomplete (status: {status}, reason: {reason})"
4748
CHUNK_RETRY = "Chunk {index}/{total} failed (attempt {attempt}), retrying: {error}"
4849
INVALID_HUNK_INDEX = (
4950
"Group '{group}': invalid hunk index {index} for {file} (max: {max}), skipping"

pr_split/planner/client.py

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -131,13 +131,20 @@ def _call_anthropic(system: str, user: str, *, settings: Settings) -> RawToolOut
131131
except anthropic.APIError as exc:
132132
raise LLMError(ErrorMsg.LLM_PARSE_ERROR(detail=str(exc))) from exc
133133
if response.stop_reason != "tool_use":
134+
# A max_tokens stop means the tool input was cut off mid-plan.
135+
# Accepting it would silently drop groups and let
136+
# assign_uncovered_hunks paper over the gap, so fail and let the
137+
# chunk retry loop / caller handle it. Other stop reasons without a
138+
# tool block are reported below as "no tool_use block".
134139
stop_reason = getattr(response, "stop_reason", "unknown")
135140
keys: list[str] = []
136141
for block in getattr(response, "content", []):
137142
if isinstance(block, BetaToolUseBlock) and isinstance(block.input, dict):
138143
keys = list(block.input.keys())
139144
break
140145
logger.warning(logs.LLM_OUTPUT_TRUNCATED.format(stop_reason=stop_reason, keys=keys))
146+
if stop_reason == "max_tokens":
147+
raise LLMError(ErrorMsg.LLM_OUTPUT_TRUNCATED(detail=f"stop_reason={stop_reason}"))
141148
for block in response.content:
142149
if isinstance(block, BetaToolUseBlock) and block.name == SPLIT_TOOL_NAME:
143150
return RawToolOutput(groups=_extract_raw_output(block.input))
@@ -156,6 +163,12 @@ def _call_openai(system: str, user: str, *, settings: Settings) -> RawToolOutput
156163
)
157164
except openai.APIError as exc:
158165
raise LLMError(ErrorMsg.LLM_PARSE_ERROR(detail=str(exc))) from exc
166+
status = getattr(response, "status", None)
167+
if status == "incomplete":
168+
details = getattr(response, "incomplete_details", None)
169+
reason = getattr(details, "reason", None) or "unknown"
170+
logger.warning(logs.LLM_OUTPUT_INCOMPLETE.format(status=status, reason=reason))
171+
raise LLMError(ErrorMsg.LLM_OUTPUT_TRUNCATED(detail=f"status={status}, reason={reason}"))
159172
for item in response.output:
160173
if item.type == "function_call" and item.name == SPLIT_TOOL_NAME:
161174
try:

tests/test_cli_coverage.py

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -651,6 +651,67 @@ def test_execute_missing_raw_diff(self, mock_pe: MagicMock, mock_load: MagicMock
651651
result = runner.invoke(app, ["execute"])
652652
assert result.exit_code != 0
653653

654+
@patch("pr_split.cli.commit_exists", return_value=False)
655+
@patch("pr_split.cli.load_plan")
656+
@patch("pr_split.cli.plan_exists", return_value=True)
657+
def test_execute_unknown_merge_base_sha_is_a_clean_error(
658+
self, mock_pe: MagicMock, mock_load: MagicMock, mock_commit: MagicMock
659+
) -> None:
660+
mock_plan_file = MagicMock()
661+
mock_plan_file.git_state.branches = []
662+
mock_plan_file.git_state.prs = []
663+
mock_plan_file.plan.raw_diff = "some diff"
664+
mock_plan_file.plan.merge_base_sha = "0123456789abcdef"
665+
mock_load.return_value = mock_plan_file
666+
result = runner.invoke(app, ["execute"])
667+
assert result.exit_code == 1
668+
assert result.exception is None or isinstance(result.exception, SystemExit)
669+
assert "merge base 0123456789abcdef is not in this repository" in result.output
670+
mock_commit.assert_called_once_with("0123456789abcdef")
671+
672+
@patch(
673+
"pr_split.cli._create_branches_and_commits",
674+
side_effect=PRSplitError("3 branch(es) failed"),
675+
)
676+
@patch("pr_split.cli.typer.confirm", return_value=True)
677+
@patch("pr_split.cli.validate_coverage")
678+
@patch("pr_split.cli.parse_diff")
679+
@patch("pr_split.cli.commit_exists", return_value=True)
680+
@patch("pr_split.cli.check_gh_auth", return_value=True)
681+
@patch("pr_split.cli.is_worktree_clean", return_value=True)
682+
@patch("pr_split.cli.branch_exists", return_value=True)
683+
@patch("pr_split.cli.load_plan")
684+
@patch("pr_split.cli.plan_exists", return_value=True)
685+
def test_execute_branch_creation_failure_is_a_clean_error(
686+
self,
687+
mock_pe: MagicMock,
688+
mock_load: MagicMock,
689+
mock_be: MagicMock,
690+
mock_clean: MagicMock,
691+
mock_auth: MagicMock,
692+
mock_commit: MagicMock,
693+
mock_parse: MagicMock,
694+
mock_validate: MagicMock,
695+
mock_confirm: MagicMock,
696+
mock_create: MagicMock,
697+
) -> None:
698+
mock_plan_file = MagicMock()
699+
mock_plan_file.git_state.branches = []
700+
mock_plan_file.git_state.prs = []
701+
mock_plan_file.plan.raw_diff = "some diff"
702+
mock_plan_file.plan.merge_base_sha = "abc123"
703+
mock_plan_file.plan.stacked = False
704+
mock_plan_file.plan.dev_branch_arg = "feature"
705+
mock_plan_file.plan.dev_branch = "feature"
706+
mock_plan_file.plan.base_branch = "main"
707+
mock_plan_file.plan.groups = [_group("pr-1", "t", files=["a.py"])]
708+
mock_load.return_value = mock_plan_file
709+
result = runner.invoke(app, ["execute"])
710+
assert result.exit_code == 1
711+
assert result.exception is None or isinstance(result.exception, SystemExit)
712+
assert "3 branch(es) failed" in result.output
713+
714+
@patch("pr_split.cli.commit_exists", return_value=True)
654715
@patch("pr_split.cli._remote_names", return_value={"origin"})
655716
@patch("pr_split.cli._create_branches_and_commits")
656717
@patch("pr_split.cli.typer.confirm", return_value=True)
@@ -667,6 +728,7 @@ def test_execute_rejects_a_remote_tracking_base_from_an_old_plan(
667728
mock_confirm: MagicMock,
668729
mock_create: MagicMock,
669730
mock_remotes: MagicMock,
731+
mock_commit_exists: MagicMock,
670732
) -> None:
671733
mock_be.side_effect = lambda ref: ref != "refs/heads/origin/main"
672734
mock_plan_file = MagicMock()
@@ -696,6 +758,7 @@ def test_execute_missing_merge_base_sha(
696758
result = runner.invoke(app, ["execute"])
697759
assert result.exit_code != 0
698760

761+
@patch("pr_split.cli.commit_exists", return_value=True)
699762
@patch("pr_split.cli._create_branches_and_commits")
700763
@patch("pr_split.cli.check_gh_auth", return_value=True)
701764
@patch("pr_split.cli.is_worktree_clean", return_value=True)
@@ -710,6 +773,7 @@ def test_execute_rejects_saved_plan_with_binary_files(
710773
mock_clean: MagicMock,
711774
mock_auth: MagicMock,
712775
mock_create: MagicMock,
776+
mock_commit_exists: MagicMock,
713777
) -> None:
714778
mock_plan_file = MagicMock()
715779
mock_plan_file.git_state.branches = []
@@ -732,6 +796,7 @@ def test_execute_rejects_saved_plan_with_binary_files(
732796
assert "img.png" in result.output
733797
mock_create.assert_not_called()
734798

799+
@patch("pr_split.cli.commit_exists", return_value=True)
735800
@patch("pr_split.cli._create_branches_and_commits")
736801
@patch("pr_split.cli.check_gh_auth", return_value=True)
737802
@patch("pr_split.cli.is_worktree_clean", return_value=True)
@@ -746,6 +811,7 @@ def test_execute_rejects_unknown_dependency_before_branch_creation(
746811
mock_clean: MagicMock,
747812
mock_auth: MagicMock,
748813
mock_create: MagicMock,
814+
mock_commit_exists: MagicMock,
749815
) -> None:
750816
mock_plan_file = MagicMock()
751817
mock_plan_file.git_state.branches = []

0 commit comments

Comments
 (0)