Merge branch 'main' into fix/apply-hunks-line-splitting

Author: vitali87

README.md

@@ -45,6 +45,7 @@ uv tool install "pr-split[cp-sat]"
 
 - Python 3.12+
 - [GitHub CLI](https://cli.github.com/) (`gh`) authenticated via `gh auth login`
+- [`gh-stack` extension](https://github.com/github/gh-stack) (`gh extension install github/gh-stack`) when using `--stack`
 - `ANTHROPIC_API_KEY` or `OPENAI_API_KEY` environment variable set when using the `llm` partition backend
 
 ## Usage
@@ -85,6 +86,7 @@ pr-split split feature-branch --base main --dry-run
 | `--priority` | `orthogonal` | Grouping priority (`orthogonal` or `logical`) |
 | `--chunk-strategy` | `dynamic_programming` | Large-diff chunking strategy (`dynamic_programming` or `greedy`) |
 | `--partition-strategy` | `llm` | Hunk-to-PR partition backend (`llm`, `graph`, or `cp_sat`) |
+| `--cp-sat-timeout` | `15.0` | Maximum seconds to spend in the CP-SAT solver |
 | `--stack` | `false` | Stack dependent PRs: each child branches from and targets its parent's branch |
 | `--draft` | `false` | Open every sub-PR as a draft |
 | `--dry-run` | `false` | Preview plan and save to `.pr-split/plan.json` without creating branches or PRs |
@@ -97,7 +99,7 @@ pr-split split feature-branch --base main --stack
 
 Without `--stack`, every sub-PR branch is cut from the merge base and targets the base branch, so a sub-PR that depends on code from another group only goes green once its dependency merges. With `--stack`, each dependent group's branch is cut from its parent group's branch and carries the parent's hunks for shared files, and its PR targets the parent's branch. Every PR shows only its own diff, compiles standalone, and GitHub retargets children automatically as parents merge.
 
-Linear chains in the plan are also registered as [native GitHub stacks](https://github.blog/changelog/2026-07-30-stacked-pull-requests-are-now-in-public-preview/) via the [`gh-stack` extension](https://github.com/github/gh-stack) (`gh extension install github/gh-stack`). If the extension is missing the linking step is skipped with a warning — the PRs are already correctly chained without it. Groups that depend on more than one group target the base branch directly, since native stacks are strictly linear; their branch carries every ancestor's changes so it still builds standalone, and those extra changes drop out of the diff as the ancestor PRs merge.
+Linear chains in the plan are registered as [native GitHub stacks](https://github.blog/changelog/2026-07-30-stacked-pull-requests-are-now-in-public-preview/) via the [`gh-stack` extension](https://github.com/github/gh-stack), which is **required** for `--stack`: install it with `gh extension install github/gh-stack`. `pr-split` checks for it up front and refuses to run a stacked split (or `execute` a stacked plan) without it; a `--dry-run` does not need it. If linking fails after the PRs are created, the command exits with an error — the plan state is already saved, so `pr-split clean` can undo the split. Groups that depend on more than one group target the base branch directly, since native stacks are strictly linear; their branch carries every ancestor's changes so it still builds standalone, and those extra changes drop out of the diff as the ancestor PRs merge.
 
 ### Check status of an existing split
 
@@ -195,6 +197,7 @@ Settings can be set via environment variables with the `PR_SPLIT_` prefix:
 | `PR_SPLIT_PRIORITY` | `orthogonal` | Default grouping priority |
 | `PR_SPLIT_CHUNK_STRATEGY` | `dynamic_programming` | Large-diff chunking strategy |
 | `PR_SPLIT_PARTITION_STRATEGY` | `llm` | Hunk-to-PR partition backend |
+| `PR_SPLIT_CP_SAT_TIMEOUT` | `15.0` | Maximum seconds to spend in the CP-SAT solver |
 | `PR_SPLIT_STACK` | `false` | Stack dependent PRs on their parent's branch |
 | `PR_SPLIT_DRAFT` | `false` | Open every sub-PR as a draft |
 | `PR_SPLIT_WEBHOOK_URL` | (none) | Webhook URL for merge notifications |

pr_split/cli.py

@@ -44,11 +44,18 @@
     merge_chain_assignments,
     parse_diff,
 )
-from .exceptions import ErrorMsg, PlanValidationError, PRCreationError, PRSplitError
+from .exceptions import (
+    ErrorMsg,
+    GitOperationError,
+    PlanValidationError,
+    PRCreationError,
+    PRSplitError,
+)
 from .git_ops import (
     add_worktree,
     branch_exists,
     check_gh_auth,
+    check_gh_stack,
     commit_files_in_dir,
     delete_branch,
     derive_split_namespace,
@@ -125,7 +132,20 @@ def _add_children(parent_id: str, prefix: str) -> None:
     return f"## Dependency graph\n\nMerge in this order:\n\n```\n{tree_block}\n```"
 
 
-def _validate_inputs(dev_branch: str, base: str, *, dry_run: bool = False) -> None:
+def _require_gh_stack() -> None:
+    try:
+        installed = check_gh_stack()
+    except GitOperationError as exc:
+        console.print(f"[red]{exc}[/red]")
+        raise typer.Exit(1) from exc
+    if not installed:
+        console.print(f"[red]{ErrorMsg.GH_STACK_MISSING()}[/red]")
+        raise typer.Exit(1)
+
+
+def _validate_inputs(
+    dev_branch: str, base: str, *, dry_run: bool = False, stacked: bool = False
+) -> None:
     if not branch_exists(dev_branch):
         console.print(f"[red]{ErrorMsg.BRANCH_NOT_FOUND(branch=dev_branch)}[/red]")
         raise typer.Exit(1)
@@ -138,6 +158,8 @@ def _validate_inputs(dev_branch: str, base: str, *, dry_run: bool = False) -> No
     if not dry_run and not check_gh_auth():
         console.print(f"[red]{ErrorMsg.GH_AUTH_FAILED()}[/red]")
         raise typer.Exit(1)
+    if not dry_run and stacked:
+        _require_gh_stack()
 
 
 def _handle_loc_bound_warnings(warnings: list[str], *, strict_loc_bounds: bool) -> None:
@@ -691,15 +713,33 @@ def split(
             help="Maximum LLM refinement iterations to fix LOC bound violations (0 = disabled)",
         ),
     ] = DEFAULT_MAX_REFINEMENT_ITERATIONS,
-    priority: Annotated[Priority, typer.Option(help="Grouping priority")] = Priority.ORTHOGONAL,
+    priority: Annotated[
+        Priority,
+        typer.Option("--priority", envvar="PR_SPLIT_PRIORITY", help="Grouping priority"),
+    ] = Priority.ORTHOGONAL,
     chunk_strategy: Annotated[
-        ChunkStrategy, typer.Option(help="Chunking strategy for large diffs")
+        ChunkStrategy,
+        typer.Option(
+            "--chunk-strategy",
+            envvar="PR_SPLIT_CHUNK_STRATEGY",
+            help="Chunking strategy for large diffs",
+        ),
     ] = DEFAULT_CHUNK_STRATEGY,
     partition_strategy: Annotated[
-        PartitionStrategy, typer.Option(help="Backend for hunk-to-PR partitioning")
+        PartitionStrategy,
+        typer.Option(
+            "--partition-strategy",
+            envvar="PR_SPLIT_PARTITION_STRATEGY",
+            help="Backend for hunk-to-PR partitioning",
+        ),
     ] = DEFAULT_PARTITION_STRATEGY,
     cp_sat_timeout: Annotated[
-        float, typer.Option(help="Maximum seconds to spend in the CP-SAT solver")
+        float,
+        typer.Option(
+            "--cp-sat-timeout",
+            envvar="PR_SPLIT_CP_SAT_TIMEOUT",
+            help="Maximum seconds to spend in the CP-SAT solver",
+        ),
     ] = DEFAULT_CP_SAT_TIMEOUT_SECONDS,
     stack: Annotated[
         bool,
@@ -740,7 +780,7 @@ def split(
         base = fork_info["base_branch"]
         author = fork_info["author"]
 
-    _validate_inputs(dev_branch, base, dry_run=dry_run)
+    _validate_inputs(dev_branch, base, dry_run=dry_run, stacked=stack)
 
     if plan_exists():
         existing = load_plan()
@@ -860,15 +900,14 @@ def split(
             )
         )
         raise
-    if stack:
-        _link_stacks(dag, pr_records)
-
     save_plan(
         PlanFile(
             plan=split_plan,
             git_state=GitState(branches=branch_records, prs=pr_records),
         )
     )
+    if stack:
+        _link_stacks(dag, pr_records)
     logger.success(f"Split complete: {len(groups)} PRs created")
 
 
@@ -1019,10 +1058,16 @@ def execute(
     if not check_gh_auth():
         console.print(f"[red]{ErrorMsg.GH_AUTH_FAILED()}[/red]")
         raise typer.Exit(1)
+    if plan.stacked:
+        _require_gh_stack()
 
     parsed_diff = parse_diff(plan.raw_diff)
 
     try:
+        # Building the DAG rejects unknown dependency ids; do it here so a
+        # malformed saved plan fails before any branch is created.
+        dag = PlanDAG(plan.groups)
+        dag.validate_acyclic()
         validate_coverage(plan.groups, parsed_diff)
     except PlanValidationError as exc:
         console.print(f"[red]{exc}[/red]")
@@ -1051,15 +1096,15 @@ def execute(
             )
         )
         raise
-    if plan.stacked:
-        _link_stacks(PlanDAG(plan.groups), pr_records)
-
     save_plan(
         PlanFile(
             plan=plan,
             git_state=GitState(branches=branch_records, prs=pr_records),
         )
     )
+    if plan.stacked:
+        _link_stacks(PlanDAG(plan.groups), pr_records)
+    logger.success(f"Execute complete: {len(plan.groups)} PRs created from saved plan")
     logger.success(f"Execute complete: {len(plan.groups)} PRs created from saved plan")
 
 
@@ -1134,7 +1179,14 @@ def merge_all(
         console.print("[yellow]No PRs found in plan. Nothing to merge.[/yellow]")
         raise typer.Exit(0)
 
-    dag = PlanDAG(plan.groups)
+    # A hand-edited plan.json can carry unknown or cyclic dependencies;
+    # report that instead of a traceback from the DAG walk.
+    try:
+        dag = PlanDAG(plan.groups)
+        dag.validate_acyclic()
+    except PlanValidationError as exc:
+        console.print(f"[red]{exc}[/red]")
+        raise typer.Exit(1) from exc
     merged: list[str] = []
     skipped: list[str] = []
     skipped_ids: set[str] = set()

pr_split/exceptions.py

@@ -14,6 +14,10 @@ class ErrorMsg(StrEnum):
     CYCLE_DETECTED = "Dependency cycle detected in split plan"
     COVERAGE_GAP = "Hunk {file}[{index}] not assigned to any group"
     COVERAGE_OVERLAP = "Hunk {file}[{index}] assigned to multiple groups: {groups}"
+    UNKNOWN_HUNK = "Hunk {file}[{index}] assigned to group '{group}' does not exist in the diff"
+    UNKNOWN_FILE = "File '{file}' assigned to group '{group}' does not exist in the diff"
+    UNKNOWN_DEPENDENCY = "Group '{group}' depends on unknown group '{dep}'"
+    DUPLICATE_GROUP_ID = "Group id '{group}' is used more than once"
     LOC_MISMATCH = "Total LOC {actual} does not match diff LOC {expected}"
     MERGE_CONFLICT = "Groups '{a}' and '{b}' modify overlapping regions in '{file}'"
     NO_PLAN = "No split plan found; run 'pr-split split' first"
@@ -27,6 +31,11 @@ class ErrorMsg(StrEnum):
     HUNK_TOO_LARGE = "Hunk {file}[{index}] has ~{tokens} estimated tokens, exceeds budget {budget}"
     MIN_LOC_GE_MAX_LOC = "min_loc {min_loc} must be less than max_loc {max_loc}"
     LOC_BOUNDS_STRICT_FAILED = "Plan violates configured LOC bounds"
+    GH_STACK_MISSING = (
+        "The gh-stack extension is required for stacked PRs;"
+        " run 'gh extension install github/gh-stack'"
+    )
+    STACK_LINK_FAILED = "Failed to link stack for PRs {prs}: {detail}"
 
     def __call__(self, **kwargs: object) -> str:
         return self.value.format(**kwargs) if kwargs else self.value

pr_split/git_ops/__init__.py

@@ -37,6 +37,9 @@
 from .prs import (
     check_gh_auth as check_gh_auth,
 )
+from .prs import (
+    check_gh_stack as check_gh_stack,
+)
 from .prs import (
     close_pr as close_pr,
 )

pr_split/git_ops/prs.py

@@ -30,6 +30,20 @@ def check_gh_auth() -> bool:
     return True
 
 
+GH_STACK_EXTENSION = "github/gh-stack"
+
+
+def check_gh_stack() -> bool:
+    """Return whether the gh-stack extension is installed.
+
+    Raises GitOperationError if ``gh extension list`` itself fails, so an
+    operational problem (gh missing, auth rejected) is not mistaken for a
+    missing extension.
+    """
+    installed = _run_gh("extension", "list")
+    return any(GH_STACK_EXTENSION in line.split() for line in installed.splitlines())
+
+
 def create_pr(
     head: str, base: str, title: str, body: str, *, draft: bool = False
 ) -> tuple[int, str]:
@@ -83,8 +97,7 @@ def link_stack(pr_numbers: list[int]) -> None:
     try:
         _run_gh("stack", "link", *[str(n) for n in pr_numbers])
     except GitOperationError as exc:
-        logger.warning(logs.STACK_LINK_FAILED.format(prs=pr_numbers, detail=exc))
-        return
+        raise GitOperationError(ErrorMsg.STACK_LINK_FAILED(prs=pr_numbers, detail=exc)) from exc
     logger.info(logs.STACK_LINKED.format(prs=pr_numbers))
 
 

pr_split/graph.py

@@ -10,11 +10,24 @@
 
 class PlanDAG:
     def __init__(self, groups: list[Group]) -> None:
-        self._groups: dict[str, Group] = {g.id: g for g in groups}
+        self._groups: dict[str, Group] = {}
+        for g in groups:
+            # Silently collapsing duplicates would drop a group and later
+            # surface as a bogus merge conflict between a group and itself.
+            if g.id in self._groups:
+                raise PlanValidationError(ErrorMsg.DUPLICATE_GROUP_ID(group=g.id))
+            self._groups[g.id] = g
         self._children: dict[str, list[str]] = {g.id: [] for g in groups}
-        self._parents: dict[str, list[str]] = {g.id: list(g.depends_on) for g in groups}
+        # A dependency listed twice ("pr-1", "pr-1") is one edge: keeping the
+        # duplicate would make the group look like a merge node, so a stacked
+        # child would be built from the merge base and left out of the stack.
+        self._parents: dict[str, list[str]] = {
+            g.id: list(dict.fromkeys(g.depends_on)) for g in groups
+        }
         for g in groups:
-            for dep in g.depends_on:
+            for dep in self._parents[g.id]:
+                if dep not in self._groups:
+                    raise PlanValidationError(ErrorMsg.UNKNOWN_DEPENDENCY(group=g.id, dep=dep))
                 self._children[dep].append(g.id)
 
     def _build_sorter(self) -> TopologicalSorter[str]:

pr_split/logs.py

@@ -60,7 +60,6 @@
     "Refinement iteration limit reached ({iterations}), {remaining} violation(s) remain"
 )
 STACK_LINKED = "Linked stack for PRs {prs}"
-STACK_LINK_FAILED = "Could not link stack for PRs {prs}: {detail}"
 MERGE_NODE_NOT_STACKED = (
     "Group '{group}' depends on multiple groups; native stacks are linear, so its"
     " branch and PR target the base branch directly, carrying every ancestor's"

pr_split/planner/scoring.py

@@ -39,7 +39,8 @@ def score_plan(groups: list[Group], max_loc: int, min_loc: int | None = None) ->
         else 0
     )
     loc_overflow = sum(max(0, group.estimated_loc - max_loc) for group in groups)
-    dependency_edges = sum(len(group.depends_on) for group in groups)
+    # Count edges as the DAG sees them (a dependency listed twice is one edge).
+    dependency_edges = sum(len(dag.parents(group.id)) for group in groups)
     dag_width = max((len(batch) for batch in dag.iter_ready()), default=0)
     dag_depth = _dag_depth(dag)
 

pr_split/planner/validator.py

@@ -14,6 +14,10 @@ def validate_coverage(groups: list[Group], parsed_diff: ParsedDiff) -> None:
     assigned: dict[tuple[str, int], list[str]] = {}
     for group in groups:
         for assignment in group.assignments:
+            if assignment.file_path not in hunk_counts:
+                raise PlanValidationError(
+                    ErrorMsg.UNKNOWN_FILE(file=assignment.file_path, group=group.id)
+                )
             # A WHOLE_FILE assignment claims every hunk of the file even when
             # its hunk_indices list was left empty.
             if assignment.assignment_type is AssignmentType.WHOLE_FILE:
@@ -26,6 +30,12 @@ def validate_coverage(groups: list[Group], parsed_diff: ParsedDiff) -> None:
 
     all_hunks = {(pf.path, i) for pf in parsed_diff.patch_set for i in range(len(pf))}
 
+    for key, group_ids in assigned.items():
+        if key not in all_hunks:
+            raise PlanValidationError(
+                ErrorMsg.UNKNOWN_HUNK(file=key[0], index=key[1], group=group_ids[0])
+            )
+
     for key in all_hunks:
         if key not in assigned:
             raise PlanValidationError(ErrorMsg.COVERAGE_GAP(file=key[0], index=key[1]))

scripts/score_pr.py

@@ -35,6 +35,18 @@ def _md_escape(s: str) -> str:
     return s.replace("|", "\\|")
 
 
+def load_plan_groups(plan_path: str) -> list[dict]:
+    """Return the groups from a saved plan file.
+
+    ``pr-split --dry-run`` writes a ``PlanFile`` whose top-level keys are
+    ``plan`` and ``git_state``; the groups live under ``plan``.
+    """
+    with open(plan_path) as f:
+        data = json.load(f)
+    plan = data.get("plan", data)
+    return plan.get("groups", [])
+
+
 def _parse_int_env(name: str, default: int) -> int:
     raw = os.environ.get(name, str(default))
     try:
@@ -119,10 +131,7 @@ def main() -> None:
         _skip("No plan file generated.")
         return
 
-    with open(plan_path) as f:
-        plan = json.load(f)
-
-    groups = plan.get("groups", [])
+    groups = load_plan_groups(plan_path)
     total_groups = len(groups)
 
     max_group_loc = max((g["estimated_loc"] for g in groups), default=0)