diff --git a/README.md b/README.md index 4d2819c..4c784f4 100644 --- a/README.md +++ b/README.md @@ -17,9 +17,9 @@ ## Latest News 🔥 +- Stacked PR Mode — pass `--stack` and every dependent PR branches from and targets its parent's branch, so each sub-PR compiles and passes CI on its own. Chains are registered as native GitHub stacks via the `gh-stack` extension when it is installed. - GitHub Action — add pr-split to any repo as a CI check. Scores every PR and posts a split plan comment when it's too large. No API key needed. - Smart LOC Bounds — set `--min-loc` and `--max-loc` to control sub-PR size across all three backends (LLM, graph, CP-SAT). Undersized groups get merged, oversized groups get penalised. -- LLM Refinement Loop — enable `--max-refinement-iterations` and pr-split will automatically feed LOC violations back to the LLM until every group fits within your configured bounds. ## Why pr-split? @@ -85,8 +85,20 @@ 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`) | +| `--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 | +### Stack dependent PRs + +```bash +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. + ### Check status of an existing split ```bash @@ -121,7 +133,7 @@ pr-split merge --notify https://hooks.slack.com/... pr-split execute ``` -Creates branches and PRs from a previously saved `--dry-run` plan. Uses the saved diff and merge base for consistency — safe even if the dev branch has changed since the dry run. +Creates branches and PRs from a previously saved `--dry-run` plan. Uses the saved diff and merge base for consistency — safe even if the dev branch has changed since the dry run. Pass `--stack` or `--draft` to stack the PRs or open them as drafts even when the plan was saved without those flags. ### Interactive plan editing @@ -183,6 +195,8 @@ 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_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 | ## GitHub Action diff --git a/pr_split/cli.py b/pr_split/cli.py index 3aed0da..e29bc0e 100644 --- a/pr_split/cli.py +++ b/pr_split/cli.py @@ -5,6 +5,7 @@ import tempfile import time import urllib.request +from collections.abc import Generator from concurrent.futures import ThreadPoolExecutor, as_completed from pathlib import Path from threading import Lock, Semaphore @@ -36,8 +37,14 @@ PartitionStrategy, Priority, ) -from .diff_ops import ParsedDiff, extract_diff, materialize_group_files, parse_diff -from .exceptions import ErrorMsg, PRSplitError +from .diff_ops import ( + ParsedDiff, + extract_diff, + materialize_group_files, + merge_chain_assignments, + parse_diff, +) +from .exceptions import ErrorMsg, PlanValidationError, PRCreationError, PRSplitError from .git_ops import ( add_worktree, branch_exists, @@ -53,10 +60,10 @@ remove_worktree, ) from .git_ops.branches import run_git -from .git_ops.prs import close_pr, create_pr, get_pr_state, merge_pr +from .git_ops.prs import close_pr, create_pr, get_pr_state, link_stack, merge_pr from .graph import PlanDAG from .plan_store import load_plan, plan_exists, save_plan -from .planner import plan_split, validate_plan +from .planner import plan_split, validate_coverage, validate_plan from .schemas import ( BranchRecord, GitState, @@ -182,13 +189,14 @@ def _create_single_branch_and_commit( worktree_base: Path, *, author: str | None = None, + start_point: str | None = None, ) -> BranchRecord: branch_name = f"{BRANCH_PREFIX}{namespace}/{group.id}" worktree_path = str(worktree_base / group.id) commit_sha: str = "" with _worktree_ref_lock: - add_worktree(worktree_path, branch_name, merge_base_ref) + add_worktree(worktree_path, branch_name, start_point or merge_base_ref) try: materialized = materialize_group_files(parsed_diff, group, merge_base_ref) for file_path, content in materialized.items(): @@ -220,6 +228,47 @@ def _create_single_branch_and_commit( ) +def _stacked_batch_args( + dag: PlanDAG, + groups_by_id: dict[str, Group], + branch_names: dict[str, str], + base_branch: str, + merge_base_ref: str, + hunk_counts: dict[str, int], +) -> Generator[list[tuple[Group, str, str]], None, None]: + effective: dict[str, Group] = {} + for batch in dag.iter_ready(): + batch_args: list[tuple[Group, str, str]] = [] + for gid in batch: + group = groups_by_id[gid] + parents = dag.parents(gid) + if len(parents) == 1: + merged = merge_chain_assignments( + group, [effective[parents[0]]], hunk_counts + ) + start_point = branch_names[parents[0]] + group_base = branch_names[parents[0]] + elif len(parents) > 1: + # Native stacks are linear, so a merge node builds from the + # merge base and carries every ancestor's changes itself. + logger.warning(logs.MERGE_NODE_NOT_STACKED.format(group=gid)) + merged = merge_chain_assignments( + group, + [groups_by_id[a] for a in sorted(dag.ancestors(gid))], + hunk_counts, + carry_ancestor_files=True, + ) + start_point = merge_base_ref + group_base = base_branch + else: + merged = group + start_point = merge_base_ref + group_base = base_branch + effective[gid] = merged + batch_args.append((merged, group_base, start_point)) + yield batch_args + + def _create_branches_and_commits( groups: list[Group], parsed_diff: ParsedDiff, @@ -228,33 +277,49 @@ def _create_branches_and_commits( namespace: str, *, author: str | None = None, + stacked: bool = False, ) -> list[BranchRecord]: worktree_base = Path(tempfile.mkdtemp(prefix="pr-split-worktrees-")) + if stacked: + dag = PlanDAG(groups) + groups_by_id = {g.id: g for g in groups} + branch_names = {g.id: f"{BRANCH_PREFIX}{namespace}/{g.id}" for g in groups} + hunk_counts = {pf.path: len(pf) for pf in parsed_diff.patch_set} + batches = _stacked_batch_args( + dag, groups_by_id, branch_names, base_branch, merge_base_ref, hunk_counts + ) + else: + batches = iter([[(group, base_branch, merge_base_ref) for group in groups]]) + try: - with ThreadPoolExecutor(max_workers=_WORKTREE_MAX_WORKERS) as executor: - future_to_group_id = { - executor.submit( - _create_single_branch_and_commit, - group, - parsed_diff, - base_branch, - merge_base_ref, - namespace, - worktree_base, - author=author, - ): group.id - for group in groups - } - results: dict[str, BranchRecord] = {} - errors: list[tuple[str, Exception]] = [] - for future in as_completed(future_to_group_id): - group_id = future_to_group_id[future] - try: - results[group_id] = future.result() - except Exception as exc: - logger.error(f"Failed to create branch for {group_id}: {exc}") - errors.append((group_id, exc)) + results: dict[str, BranchRecord] = {} + errors: list[tuple[str, Exception]] = [] + for batch_args in batches: + with ThreadPoolExecutor(max_workers=_WORKTREE_MAX_WORKERS) as executor: + future_to_group_id = { + executor.submit( + _create_single_branch_and_commit, + group, + parsed_diff, + group_base, + merge_base_ref, + namespace, + worktree_base, + author=author, + start_point=start_point, + ): group.id + for group, group_base, start_point in batch_args + } + for future in as_completed(future_to_group_id): + group_id = future_to_group_id[future] + try: + results[group_id] = future.result() + except Exception as exc: + logger.error(f"Failed to create branch for {group_id}: {exc}") + errors.append((group_id, exc)) + if errors: + break if errors: for record in results.values(): @@ -331,12 +396,13 @@ def _build_pr_body(group: Group, all_groups: list[Group]) -> str: return "\n\n".join(sections) -def _push_and_create_single_pr( +def _create_single_pr( group: Group, record: BranchRecord, all_groups: list[Group], + *, + draft: bool = False, ) -> PRRecord: - push_branch(record.branch_name) logger.info(logs.CREATING_PR.format(group=group.id)) body = _build_pr_body(group, all_groups) with _gh_semaphore: @@ -345,6 +411,7 @@ def _push_and_create_single_pr( base=record.base_branch, title=group.title, body=body, + draft=draft, ) return PRRecord( group_id=group.id, @@ -356,33 +423,82 @@ def _push_and_create_single_pr( def _push_and_create_prs( groups: list[Group], branch_records: list[BranchRecord], + *, + draft: bool = False, ) -> list[PRRecord]: record_map = {r.group_id: r for r in branch_records} + errors: list[tuple[str, Exception]] = [] + + # Children target parent branches, so every branch is pushed before any PR opens. + with ThreadPoolExecutor(max_workers=_PUSH_MAX_WORKERS) as executor: + push_futures = { + executor.submit(push_branch, record_map[group.id].branch_name): group.id + for group in groups + } + pushed: set[str] = set() + for future in as_completed(push_futures): + group_id = push_futures[future] + try: + future.result() + pushed.add(group_id) + except Exception as exc: + logger.error(f"Failed to push branch for {group_id}: {exc}") + errors.append((group_id, exc)) + + branch_owner = {record_map[g.id].branch_name: g.id for g in groups} + + def _base_pushed(group: Group) -> bool: + # Walk the whole base chain: a pushed leaf must not open a PR when + # any ancestor branch in its stack failed to push. + gid = group.id + while True: + owner = branch_owner.get(record_map[gid].base_branch) + if owner is None: + return True + if owner not in pushed: + logger.warning( + logs.PR_SKIPPED_BASE_NOT_PUSHED.format( + group=group.id, base=record_map[gid].base_branch + ) + ) + return False + gid = owner with ThreadPoolExecutor(max_workers=_PUSH_MAX_WORKERS) as executor: future_to_group_id = { executor.submit( - _push_and_create_single_pr, group, record_map[group.id], groups + _create_single_pr, group, record_map[group.id], groups, draft=draft ): group.id for group in groups + if group.id in pushed and _base_pushed(group) } results: dict[str, PRRecord] = {} - errors: list[tuple[str, Exception]] = [] for future in as_completed(future_to_group_id): group_id = future_to_group_id[future] try: results[group_id] = future.result() except Exception as exc: - logger.error(f"Failed to push/create PR for {group_id}: {exc}") + logger.error(f"Failed to create PR for {group_id}: {exc}") errors.append((group_id, exc)) if errors: error_details = "\n".join([f"- {gid}: {exc}" for gid, exc in errors]) - raise PRSplitError(f"{len(errors)} PR(s) failed:\n{error_details}") + raise PRCreationError( + f"{len(errors)} PR(s) failed:\n{error_details}", + pr_records=[results[g.id] for g in groups if g.id in results], + ) return [results[g.id] for g in groups] +def _link_stacks(dag: PlanDAG, pr_records: list[PRRecord]) -> None: + pr_by_group = {r.group_id: r.pr_number for r in pr_records} + for chain in dag.linear_chains(): + if len(chain) < 2: + continue + link_stack([pr_by_group[gid] for gid in chain]) + + def _move_assignment( groups: list[Group], parsed_diff: ParsedDiff, @@ -600,6 +716,22 @@ def split( cp_sat_timeout: Annotated[ float, typer.Option(help="Maximum seconds to spend in the CP-SAT solver") ] = DEFAULT_CP_SAT_TIMEOUT_SECONDS, + stack: Annotated[ + bool, + typer.Option( + "--stack", + envvar="PR_SPLIT_STACK", + help="Stack dependent PRs: each child branches from and targets its parent's branch", + ), + ] = False, + draft: Annotated[ + bool, + typer.Option( + "--draft", + envvar="PR_SPLIT_DRAFT", + help="Open every sub-PR as a draft", + ), + ] = False, dry_run: Annotated[ bool, typer.Option("--dry-run", help="Preview plan without creating branches or PRs") ] = False, @@ -716,6 +848,8 @@ def split( min_loc=settings.min_loc, max_loc=settings.max_loc, strict_loc_bounds=settings.strict_loc_bounds, + stacked=stack, + draft=draft, priority=priority, groups=groups, author=author, @@ -733,9 +867,18 @@ def split( namespace = derive_split_namespace(dev_branch_arg) branch_records = _create_branches_and_commits( - groups, parsed_diff, base, merge_base_ref, namespace, author=author + groups, parsed_diff, base, merge_base_ref, namespace, author=author, stacked=stack ) - pr_records = _push_and_create_prs(groups, branch_records) + try: + pr_records = _push_and_create_prs(groups, branch_records, draft=draft) + except PRCreationError as exc: + save_plan(PlanFile( + plan=split_plan, + git_state=GitState(branches=branch_records, prs=exc.pr_records), + )) + raise + if stack: + _link_stacks(dag, pr_records) save_plan(PlanFile( plan=split_plan, @@ -835,13 +978,34 @@ def clean() -> None: @app.command( help="Execute a previously saved dry-run plan, creating branches and PRs.", ) -def execute() -> None: +def execute( + stack: Annotated[ + bool, + typer.Option( + "--stack", + envvar="PR_SPLIT_STACK", + help="Stack dependent PRs even if the saved plan was not created with --stack", + ), + ] = False, + draft: Annotated[ + bool, + typer.Option( + "--draft", + envvar="PR_SPLIT_DRAFT", + help="Open every sub-PR as a draft even if the plan was not saved with --draft", + ), + ] = False, +) -> None: if not plan_exists(): console.print(ErrorMsg.NO_PLAN()) raise typer.Exit(1) plan_file = load_plan() plan = plan_file.plan + if stack and not plan.stacked: + plan = plan.model_copy(update={"stacked": True}) + if draft and not plan.draft: + plan = plan.model_copy(update={"draft": True}) if plan_file.git_state.branches or plan_file.git_state.prs: console.print( @@ -878,6 +1042,12 @@ def execute() -> None: parsed_diff = parse_diff(plan.raw_diff) + try: + validate_coverage(plan.groups, parsed_diff) + except PlanValidationError as exc: + console.print(f"[red]{exc}[/red]") + raise typer.Exit(1) from exc + _present_plan(plan.groups) typer.confirm("Proceed with creating branches and PRs?", abort=True) @@ -891,8 +1061,18 @@ def execute() -> None: plan.merge_base_sha, namespace, author=plan.author, + stacked=plan.stacked, ) - pr_records = _push_and_create_prs(plan.groups, branch_records) + try: + pr_records = _push_and_create_prs(plan.groups, branch_records, draft=plan.draft) + except PRCreationError as exc: + save_plan(PlanFile( + plan=plan, + git_state=GitState(branches=branch_records, prs=exc.pr_records), + )) + raise + if plan.stacked: + _link_stacks(PlanDAG(plan.groups), pr_records) save_plan(PlanFile( plan=plan, diff --git a/pr_split/diff_ops/__init__.py b/pr_split/diff_ops/__init__.py index a179754..d57fc55 100644 --- a/pr_split/diff_ops/__init__.py +++ b/pr_split/diff_ops/__init__.py @@ -1,9 +1,10 @@ from .parser import ParsedDiff, extract_diff, parse_diff -from .reconstructor import materialize_group_files +from .reconstructor import materialize_group_files, merge_chain_assignments __all__ = [ "ParsedDiff", "extract_diff", "materialize_group_files", + "merge_chain_assignments", "parse_diff", ] diff --git a/pr_split/diff_ops/reconstructor.py b/pr_split/diff_ops/reconstructor.py index 5454cd2..2132da3 100644 --- a/pr_split/diff_ops/reconstructor.py +++ b/pr_split/diff_ops/reconstructor.py @@ -8,10 +8,57 @@ from .. import logs from ..constants import AssignmentType from ..exceptions import GitOperationError -from ..schemas import Group +from ..schemas import Group, GroupAssignment from .parser import ParsedDiff +def merge_chain_assignments( + group: Group, + ancestors: list[Group], + hunk_counts: dict[str, int] | None = None, + *, + carry_ancestor_files: bool = False, +) -> Group: + counts = hunk_counts or {} + ancestor_hunks: dict[str, set[int]] = {} + for ancestor in ancestors: + for assignment in ancestor.assignments: + # A WHOLE_FILE assignment covers every hunk even when its + # hunk_indices list was left empty, so expand from the diff. + if assignment.assignment_type is AssignmentType.WHOLE_FILE: + covered = set(range(counts.get(assignment.file_path, 0))) + covered.update(assignment.hunk_indices) + else: + covered = set(assignment.hunk_indices) + ancestor_hunks.setdefault(assignment.file_path, set()).update(covered) + + merged = [] + own_files = set[str]() + for assignment in group.assignments: + own_files.add(assignment.file_path) + extra = ancestor_hunks.get(assignment.file_path) + if assignment.assignment_type is AssignmentType.PARTIAL_HUNKS and extra: + merged.append( + assignment.model_copy( + update={"hunk_indices": sorted(set(assignment.hunk_indices) | extra)} + ) + ) + else: + merged.append(assignment) + + if carry_ancestor_files: + for file_path, covered in ancestor_hunks.items(): + if file_path not in own_files: + merged.append( + GroupAssignment( + file_path=file_path, + assignment_type=AssignmentType.PARTIAL_HUNKS, + hunk_indices=sorted(covered), + ) + ) + return group.model_copy(update={"assignments": merged}) + + def _get_base_file_content(file_path: str, ref: str) -> str: result = subprocess.run( ["git", "show", f"{ref}:{file_path}"], @@ -62,9 +109,11 @@ def materialize_group_files( for line in hunk: if line.is_added or line.is_context: target_lines.append(str(line)[1:]) - result[assignment.file_path] = "\n".join(target_lines) - if target_lines: - result[assignment.file_path] += "\n" + # Lines from unidiff keep their trailing newline, so they are + # concatenated as-is; joining on "\n" double-spaces the file. + result[assignment.file_path] = "".join( + ln if ln.endswith("\n") else ln + "\n" for ln in target_lines + ) continue base_content = _get_base_file_content(assignment.file_path, ref) match assignment.assignment_type: diff --git a/pr_split/exceptions.py b/pr_split/exceptions.py index a069bda..f5a6958 100644 --- a/pr_split/exceptions.py +++ b/pr_split/exceptions.py @@ -1,4 +1,10 @@ +from __future__ import annotations + from enum import StrEnum +from typing import TYPE_CHECKING + +if TYPE_CHECKING: + from .schemas import PRRecord class ErrorMsg(StrEnum): @@ -44,3 +50,9 @@ class GitOperationError(PRSplitError): class LLMError(PRSplitError): pass + + +class PRCreationError(PRSplitError): + def __init__(self, message: str, pr_records: list[PRRecord]) -> None: + super().__init__(message) + self.pr_records = pr_records diff --git a/pr_split/git_ops/prs.py b/pr_split/git_ops/prs.py index b13bd72..07f554b 100644 --- a/pr_split/git_ops/prs.py +++ b/pr_split/git_ops/prs.py @@ -30,20 +30,25 @@ def check_gh_auth() -> bool: return True -def create_pr(head: str, base: str, title: str, body: str) -> tuple[int, str]: +def create_pr( + head: str, base: str, title: str, body: str, *, draft: bool = False +) -> tuple[int, str]: + args = [ + "pr", + "create", + "--base", + base, + "--head", + head, + "--title", + title, + "--body", + body, + ] + if draft: + args.append("--draft") try: - output = _run_gh( - "pr", - "create", - "--base", - base, - "--head", - head, - "--title", - title, - "--body", - body, - ) + output = _run_gh(*args) except GitOperationError as exc: raise GitOperationError(ErrorMsg.PR_CREATE_FAILED(group=head, detail=str(exc))) from exc pr_url = output.strip().splitlines()[-1] @@ -76,6 +81,15 @@ def close_pr(pr_number: int) -> None: logger.info(logs.PR_CLOSED.format(number=pr_number)) +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 + logger.info(logs.STACK_LINKED.format(prs=pr_numbers)) + + def fetch_fork_pr(pr_number: int) -> ForkPRInfo: from .branches import run_git diff --git a/pr_split/graph.py b/pr_split/graph.py index c57b9be..447d863 100644 --- a/pr_split/graph.py +++ b/pr_split/graph.py @@ -77,3 +77,22 @@ def descendants(self, group_id: str) -> set[str]: result.add(node) queue.extend(self._children[node]) return result + + def _chained_to_parent(self, group_id: str) -> bool: + parents = self._parents[group_id] + return len(parents) == 1 and len(self._children[parents[0]]) == 1 + + def linear_chains(self) -> list[list[str]]: + chains: list[list[str]] = [] + for gid in self.topological_order(): + if self._chained_to_parent(gid): + continue + chain = [gid] + while True: + children = self._children[chain[-1]] + if len(children) == 1 and self._chained_to_parent(children[0]): + chain.append(children[0]) + else: + break + chains.append(chain) + return chains diff --git a/pr_split/logs.py b/pr_split/logs.py index d6d6faa..fc2dc42 100644 --- a/pr_split/logs.py +++ b/pr_split/logs.py @@ -63,3 +63,13 @@ REFINEMENT_EXHAUSTED = ( "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" + " changes until those PRs merge" +) +PR_SKIPPED_BASE_NOT_PUSHED = ( + "Skipping PR for group '{group}': its base branch '{base}' was not pushed" +) diff --git a/pr_split/planner/__init__.py b/pr_split/planner/__init__.py index d3d8cb5..c63ffde 100644 --- a/pr_split/planner/__init__.py +++ b/pr_split/planner/__init__.py @@ -1,5 +1,5 @@ from .client import plan_split from .scoring import score_plan -from .validator import validate_plan +from .validator import validate_coverage, validate_plan -__all__ = ["plan_split", "score_plan", "validate_plan"] +__all__ = ["plan_split", "score_plan", "validate_coverage", "validate_plan"] diff --git a/pr_split/planner/validator.py b/pr_split/planner/validator.py index 0bc3433..0995af9 100644 --- a/pr_split/planner/validator.py +++ b/pr_split/planner/validator.py @@ -1,7 +1,7 @@ from __future__ import annotations from .. import logs -from ..constants import LocViolationType +from ..constants import AssignmentType, LocViolationType from ..diff_ops import ParsedDiff from ..exceptions import ErrorMsg, PlanValidationError from ..graph import PlanDAG @@ -10,10 +10,17 @@ def validate_coverage(groups: list[Group], parsed_diff: ParsedDiff) -> None: + hunk_counts = {pf.path: len(pf) for pf in parsed_diff.patch_set} assigned: dict[tuple[str, int], list[str]] = {} for group in groups: for assignment in group.assignments: - for idx in assignment.hunk_indices: + # 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: + indices = range(hunk_counts.get(assignment.file_path, 0)) + else: + indices = assignment.hunk_indices + for idx in indices: key = (assignment.file_path, idx) assigned.setdefault(key, []).append(group.id) diff --git a/pr_split/schemas.py b/pr_split/schemas.py index 8b9cbb2..c68774b 100644 --- a/pr_split/schemas.py +++ b/pr_split/schemas.py @@ -41,6 +41,8 @@ class SplitPlan(BaseModel): min_loc: int | None = None max_loc: int strict_loc_bounds: bool = False + stacked: bool = False + draft: bool = False priority: Priority groups: list[Group] = Field(default_factory=list) author: str | None = None diff --git a/tests/test_cli_helpers.py b/tests/test_cli_helpers.py index e6d7c88..4856473 100644 --- a/tests/test_cli_helpers.py +++ b/tests/test_cli_helpers.py @@ -7,12 +7,18 @@ import pytest from pr_split.cli import ( + _create_branches_and_commits, + _link_stacks, _push_and_create_prs, _render_dag, _render_dag_markdown, _resolve_fork_ref, + _stacked_batch_args, ) -from pr_split.schemas import BranchRecord, Group +from pr_split.constants import AssignmentType +from pr_split.exceptions import GitOperationError, PRSplitError +from pr_split.graph import PlanDAG +from pr_split.schemas import BranchRecord, Group, GroupAssignment, PRRecord def _group(gid: str, title: str, depends_on: list[str] | None = None) -> Group: @@ -236,3 +242,345 @@ def barrier_create(**kwargs) -> tuple[int, str]: assert mock_push.call_count == 6 assert mock_create.call_count == 6 assert max_concurrent_val >= 3 + + +class TestCreateBranchesAndCommitsStacked: + def _stacked_groups(self) -> list[Group]: + return [_group("pr-2", "feat: base"), _group("pr-3", "feat: top", ["pr-2"])] + + @patch("pr_split.cli.commit_files_in_dir", return_value="sha1") + @patch("pr_split.cli.materialize_group_files", return_value={}) + @patch("pr_split.cli.remove_worktree") + @patch("pr_split.cli.add_worktree") + def test_child_branch_starts_from_parent_branch( + self, + mock_add: MagicMock, + mock_remove: MagicMock, + mock_mat: MagicMock, + mock_commit: MagicMock, + ) -> None: + _create_branches_and_commits( + self._stacked_groups(), MagicMock(), "main", "base_sha", "ns", stacked=True + ) + start_points = {call.args[1]: call.args[2] for call in mock_add.call_args_list} + assert start_points["pr-split/ns/pr-2"] == "base_sha" + assert start_points["pr-split/ns/pr-3"] == "pr-split/ns/pr-2" + + @patch("pr_split.cli.commit_files_in_dir", return_value="sha1") + @patch("pr_split.cli.materialize_group_files", return_value={}) + @patch("pr_split.cli.remove_worktree") + @patch("pr_split.cli.add_worktree") + def test_child_pr_base_is_parent_branch( + self, + mock_add: MagicMock, + mock_remove: MagicMock, + mock_mat: MagicMock, + mock_commit: MagicMock, + ) -> None: + records = _create_branches_and_commits( + self._stacked_groups(), MagicMock(), "main", "base_sha", "ns", stacked=True + ) + bases = {r.group_id: r.base_branch for r in records} + assert bases == {"pr-2": "main", "pr-3": "pr-split/ns/pr-2"} + + @patch("pr_split.cli.commit_files_in_dir", return_value="sha1") + @patch("pr_split.cli.materialize_group_files", return_value={}) + @patch("pr_split.cli.remove_worktree") + @patch("pr_split.cli.add_worktree") + def test_merge_node_falls_back_to_base_branch( + self, + mock_add: MagicMock, + mock_remove: MagicMock, + mock_mat: MagicMock, + mock_commit: MagicMock, + ) -> None: + groups = [ + _group("pr-1", "a"), + _group("pr-2", "b"), + _group("pr-3", "c", ["pr-1", "pr-2"]), + ] + records = _create_branches_and_commits( + groups, MagicMock(), "main", "base_sha", "ns", stacked=True + ) + bases = {r.group_id: r.base_branch for r in records} + assert bases["pr-3"] == "main" + start_points = {call.args[1]: call.args[2] for call in mock_add.call_args_list} + assert start_points["pr-split/ns/pr-3"] == "base_sha" + + @patch("pr_split.cli.commit_files_in_dir", return_value="sha1") + @patch("pr_split.cli.materialize_group_files", return_value={}) + @patch("pr_split.cli.remove_worktree") + @patch("pr_split.cli.add_worktree") + def test_child_materializes_with_ancestor_hunks( + self, + mock_add: MagicMock, + mock_remove: MagicMock, + mock_mat: MagicMock, + mock_commit: MagicMock, + ) -> None: + parent = _group("pr-2", "feat: base") + parent.assignments = [ + GroupAssignment( + file_path="shared.py", + assignment_type=AssignmentType.PARTIAL_HUNKS, + hunk_indices=[0], + ) + ] + child = _group("pr-3", "feat: top", ["pr-2"]) + child.assignments = [ + GroupAssignment( + file_path="shared.py", + assignment_type=AssignmentType.PARTIAL_HUNKS, + hunk_indices=[1], + ) + ] + _create_branches_and_commits( + [parent, child], MagicMock(), "main", "base_sha", "ns", stacked=True + ) + child_calls = [ + call for call in mock_mat.call_args_list if call.args[1].id == "pr-3" + ] + assert child_calls[0].args[1].assignments[0].hunk_indices == [0, 1] + assert child_calls[0].args[2] == "base_sha" + + @patch("pr_split.cli.commit_files_in_dir", return_value="sha1") + @patch("pr_split.cli.materialize_group_files", return_value={}) + @patch("pr_split.cli.remove_worktree") + @patch("pr_split.cli.add_worktree") + def test_flat_mode_unchanged( + self, + mock_add: MagicMock, + mock_remove: MagicMock, + mock_mat: MagicMock, + mock_commit: MagicMock, + ) -> None: + records = _create_branches_and_commits( + self._stacked_groups(), MagicMock(), "main", "base_sha", "ns" + ) + start_points = {call.args[1]: call.args[2] for call in mock_add.call_args_list} + assert set(start_points.values()) == {"base_sha"} + assert {r.base_branch for r in records} == {"main"} + + +class TestLinkStacks: + @patch("pr_split.cli.link_stack") + def test_links_only_chains_of_two_or_more(self, mock_link: MagicMock) -> None: + groups = [ + _group("pr-1", "a"), + _group("pr-2", "b"), + _group("pr-3", "c", ["pr-2"]), + ] + prs = [ + PRRecord(group_id="pr-1", pr_number=11, pr_url="u"), + PRRecord(group_id="pr-2", pr_number=12, pr_url="u"), + PRRecord(group_id="pr-3", pr_number=13, pr_url="u"), + ] + _link_stacks(PlanDAG(groups), prs) + mock_link.assert_called_once_with([12, 13]) + + +class TestPushAndCreatePrsDraft: + @patch("pr_split.cli.create_pr", return_value=(1, "https://github.com/pr/1")) + @patch("pr_split.cli.push_branch") + def test_draft_forwarded_to_every_pr( + self, mock_push: MagicMock, mock_create: MagicMock + ) -> None: + groups = [_group("pr-1", "feat: a"), _group("pr-2", "feat: b")] + records = [ + _branch_record("pr-1", "pr-split/ns/pr-1"), + _branch_record("pr-2", "pr-split/ns/pr-2"), + ] + _push_and_create_prs(groups, records, draft=True) + assert [call.kwargs["draft"] for call in mock_create.call_args_list] == [True, True] + + +class TestStackedBatchArgsMergeNode: + def _diamond(self) -> list[Group]: + left = _group("pr-1", "left") + left.assignments = [ + GroupAssignment( + file_path="left.py", + assignment_type=AssignmentType.PARTIAL_HUNKS, + hunk_indices=[0], + ) + ] + right = _group("pr-2", "right") + right.assignments = [ + GroupAssignment( + file_path="right.py", + assignment_type=AssignmentType.PARTIAL_HUNKS, + hunk_indices=[0], + ) + ] + child = _group("pr-3", "merge", ["pr-1", "pr-2"]) + child.assignments = [ + GroupAssignment( + file_path="child.py", + assignment_type=AssignmentType.PARTIAL_HUNKS, + hunk_indices=[0], + ) + ] + return [left, right, child] + + def _merge_node_args(self) -> tuple[Group, str, str]: + groups = self._diamond() + batches = _stacked_batch_args( + PlanDAG(groups), + {g.id: g for g in groups}, + {g.id: f"pr-split/ns/{g.id}" for g in groups}, + "main", + "base_sha", + {"left.py": 1, "right.py": 1, "child.py": 1}, + ) + return next( + (merged, base, start) + for batch in batches + for merged, base, start in batch + if merged.id == "pr-3" + ) + + def test_merge_node_carries_both_parents_changes(self) -> None: + merged, _, _ = self._merge_node_args() + assert {a.file_path for a in merged.assignments} == { + "left.py", + "right.py", + "child.py", + } + + def test_merge_node_still_builds_from_merge_base(self) -> None: + _, base, start = self._merge_node_args() + assert (base, start) == ("main", "base_sha") + + +class TestPushFailureGating: + @patch("pr_split.cli.create_pr", return_value=(1, "https://github.com/pr/1")) + @patch("pr_split.cli.push_branch") + def test_child_pr_skipped_when_parent_push_fails( + self, mock_push: MagicMock, mock_create: MagicMock + ) -> None: + groups = [_group("pr-1", "feat: a"), _group("pr-2", "feat: b", ["pr-1"])] + records = [ + _branch_record("pr-1", "pr-split/ns/pr-1"), + BranchRecord( + group_id="pr-2", + branch_name="pr-split/ns/pr-2", + base_branch="pr-split/ns/pr-1", + commit_sha="abc123", + ), + ] + + def push(branch: str) -> None: + if branch == "pr-split/ns/pr-1": + raise GitOperationError("push rejected") + + mock_push.side_effect = push + with pytest.raises(PRSplitError): + _push_and_create_prs(groups, records) + assert mock_create.call_count == 0 + + @patch("pr_split.cli.create_pr") + @patch("pr_split.cli.push_branch") + def test_partial_pr_records_ride_on_the_error( + self, mock_push: MagicMock, mock_create: MagicMock + ) -> None: + from pr_split.exceptions import PRCreationError + + groups = [_group("pr-1", "feat: a"), _group("pr-2", "feat: b")] + records = [ + _branch_record("pr-1", "pr-split/ns/pr-1"), + _branch_record("pr-2", "pr-split/ns/pr-2"), + ] + + def create( + *, head: str, base: str, title: str, body: str, draft: bool = False + ) -> tuple[int, str]: + if head == "pr-split/ns/pr-2": + raise GitOperationError("boom") + return (11, "https://github.com/pr/11") + + mock_create.side_effect = create + with pytest.raises(PRCreationError) as excinfo: + _push_and_create_prs(groups, records) + assert [r.pr_number for r in excinfo.value.pr_records] == [11] + + +class TestStackedTransitiveChain: + @patch("pr_split.cli.commit_files_in_dir", return_value="sha1") + @patch("pr_split.cli.materialize_group_files", return_value={}) + @patch("pr_split.cli.remove_worktree") + @patch("pr_split.cli.add_worktree") + def test_grandchild_materializes_with_all_ancestor_hunks( + self, + mock_add: MagicMock, + mock_remove: MagicMock, + mock_mat: MagicMock, + mock_commit: MagicMock, + ) -> None: + grandparent = _group("pr-1", "feat: base") + grandparent.assignments = [ + GroupAssignment( + file_path="shared.py", + assignment_type=AssignmentType.PARTIAL_HUNKS, + hunk_indices=[0], + ) + ] + parent = _group("pr-2", "feat: mid", ["pr-1"]) + parent.assignments = [ + GroupAssignment( + file_path="shared.py", + assignment_type=AssignmentType.PARTIAL_HUNKS, + hunk_indices=[1], + ) + ] + child = _group("pr-3", "feat: top", ["pr-2"]) + child.assignments = [ + GroupAssignment( + file_path="shared.py", + assignment_type=AssignmentType.PARTIAL_HUNKS, + hunk_indices=[2], + ) + ] + _create_branches_and_commits( + [grandparent, parent, child], MagicMock(), "main", "base_sha", "ns", stacked=True + ) + child_calls = [ + call for call in mock_mat.call_args_list if call.args[1].id == "pr-3" + ] + assert child_calls[0].args[1].assignments[0].hunk_indices == [0, 1, 2] + + +class TestTransitivePushFailureGating: + @patch("pr_split.cli.create_pr", return_value=(1, "https://github.com/pr/1")) + @patch("pr_split.cli.push_branch") + def test_leaf_pr_skipped_when_root_push_fails( + self, mock_push: MagicMock, mock_create: MagicMock + ) -> None: + groups = [ + _group("pr-1", "feat: a"), + _group("pr-2", "feat: b", ["pr-1"]), + _group("pr-3", "feat: c", ["pr-2"]), + ] + records = [ + _branch_record("pr-1", "pr-split/ns/pr-1"), + BranchRecord( + group_id="pr-2", + branch_name="pr-split/ns/pr-2", + base_branch="pr-split/ns/pr-1", + commit_sha="abc123", + ), + BranchRecord( + group_id="pr-3", + branch_name="pr-split/ns/pr-3", + base_branch="pr-split/ns/pr-2", + commit_sha="abc123", + ), + ] + + def push(branch: str) -> None: + if branch == "pr-split/ns/pr-1": + raise GitOperationError("push rejected") + + mock_push.side_effect = push + with pytest.raises(PRSplitError): + _push_and_create_prs(groups, records) + assert mock_create.call_count == 0 diff --git a/tests/test_git_prs.py b/tests/test_git_prs.py index b464dc8..11601fa 100644 --- a/tests/test_git_prs.py +++ b/tests/test_git_prs.py @@ -12,6 +12,7 @@ close_pr, create_pr, fetch_fork_pr, + link_stack, ) @@ -123,3 +124,30 @@ def test_invalid_head_structure_raises(self, mock_gh: MagicMock) -> None: mock_gh.return_value = json.dumps({"head": "not_a_dict", "base": {"ref": "main"}}) with pytest.raises(GitOperationError): fetch_fork_pr(42) + + +class TestLinkStack: + @patch("pr_split.git_ops.prs._run_gh") + def test_links_bottom_to_top(self, mock_gh: MagicMock) -> None: + mock_gh.return_value = "" + link_stack([12, 34, 56]) + mock_gh.assert_called_once_with("stack", "link", "12", "34", "56") + + @patch("pr_split.git_ops.prs._run_gh") + def test_failure_warns_instead_of_raising(self, mock_gh: MagicMock) -> None: + mock_gh.side_effect = GitOperationError("unknown command: stack") + link_stack([12, 34]) + + +class TestCreatePrDraft: + @patch("pr_split.git_ops.prs._run_gh") + def test_draft_flag_forwarded(self, mock_gh: MagicMock) -> None: + mock_gh.return_value = "https://github.com/org/repo/pull/7" + create_pr("head", "main", "Title", "Body", draft=True) + assert "--draft" in mock_gh.call_args.args + + @patch("pr_split.git_ops.prs._run_gh") + def test_ready_by_default(self, mock_gh: MagicMock) -> None: + mock_gh.return_value = "https://github.com/org/repo/pull/7" + create_pr("head", "main", "Title", "Body") + assert "--draft" not in mock_gh.call_args.args diff --git a/tests/test_graph.py b/tests/test_graph.py index 29be4ba..2a2b656 100644 --- a/tests/test_graph.py +++ b/tests/test_graph.py @@ -96,3 +96,44 @@ def test_multiple_roots(self) -> None: assert set(dag.leaves()) == {"a", "b", "c"} order = dag.topological_order() assert set(order) == {"a", "b", "c"} + + +class TestLinearChains: + def test_single_node(self) -> None: + dag = PlanDAG([_group("a")]) + assert dag.linear_chains() == [["a"]] + + def test_forest_of_chains(self) -> None: + groups = [ + _group("pr-1"), + _group("pr-2"), + _group("pr-3", ["pr-2"]), + _group("pr-4"), + _group("pr-5", ["pr-4"]), + ] + dag = PlanDAG(groups) + assert sorted(dag.linear_chains()) == [ + ["pr-1"], + ["pr-2", "pr-3"], + ["pr-4", "pr-5"], + ] + + def test_diamond_breaks_chains(self) -> None: + groups = [ + _group("a"), + _group("b", ["a"]), + _group("c", ["a"]), + _group("d", ["b", "c"]), + ] + dag = PlanDAG(groups) + assert sorted(dag.linear_chains()) == [["a"], ["b"], ["c"], ["d"]] + + def test_fan_out_keeps_descendant_runs(self) -> None: + groups = [ + _group("a"), + _group("b", ["a"]), + _group("c", ["b"]), + _group("d", ["a"]), + ] + dag = PlanDAG(groups) + assert sorted(dag.linear_chains()) == [["a"], ["b", "c"], ["d"]] diff --git a/tests/test_reconstructor.py b/tests/test_reconstructor.py index f5d0d6d..eec1d42 100644 --- a/tests/test_reconstructor.py +++ b/tests/test_reconstructor.py @@ -11,6 +11,7 @@ _get_base_file_content, apply_hunks, materialize_group_files, + merge_chain_assignments, ) from pr_split.exceptions import GitOperationError from pr_split.schemas import Group, GroupAssignment @@ -216,3 +217,155 @@ def test_partial_hunks_existing(self, mock_base: MagicMock) -> None: result = materialize_group_files(parsed, group, "abc123") assert "existing.py" in result assert "inserted" in result["existing.py"] + + +class TestMergeChainAssignments: + def _child(self) -> Group: + return Group( + id="pr-2", + title="child", + description="child", + depends_on=["pr-1"], + assignments=[ + GroupAssignment( + file_path="shared.py", + assignment_type=AssignmentType.PARTIAL_HUNKS, + hunk_indices=[1], + ), + GroupAssignment( + file_path="own.py", + assignment_type=AssignmentType.WHOLE_FILE, + ), + ], + ) + + def _parent(self) -> Group: + return Group( + id="pr-1", + title="parent", + description="parent", + assignments=[ + GroupAssignment( + file_path="shared.py", + assignment_type=AssignmentType.PARTIAL_HUNKS, + hunk_indices=[0], + ), + GroupAssignment( + file_path="parent_only.py", + assignment_type=AssignmentType.WHOLE_FILE, + ), + ], + ) + + def test_ancestor_hunks_join_shared_file(self) -> None: + merged = merge_chain_assignments(self._child(), [self._parent()]) + by_path = {a.file_path: a for a in merged.assignments} + assert by_path["shared.py"].hunk_indices == [0, 1] + + def test_ancestor_only_files_stay_out(self) -> None: + merged = merge_chain_assignments(self._child(), [self._parent()]) + assert "parent_only.py" not in {a.file_path for a in merged.assignments} + + def test_own_whole_file_assignment_preserved(self) -> None: + merged = merge_chain_assignments(self._child(), [self._parent()]) + by_path = {a.file_path: a for a in merged.assignments} + assert by_path["own.py"].assignment_type is AssignmentType.WHOLE_FILE + + def test_no_ancestors_is_identity(self) -> None: + child = self._child() + assert merge_chain_assignments(child, []) == child + + +class TestMergeChainAssignmentsWholeFileAncestor: + def _parent(self) -> Group: + return Group( + id="pr-1", + title="parent", + description="parent", + assignments=[ + GroupAssignment( + file_path="shared.py", + assignment_type=AssignmentType.WHOLE_FILE, + ), + ], + ) + + def _child(self) -> Group: + return Group( + id="pr-2", + title="child", + description="child", + depends_on=["pr-1"], + assignments=[ + GroupAssignment( + file_path="shared.py", + assignment_type=AssignmentType.PARTIAL_HUNKS, + hunk_indices=[1], + ), + ], + ) + + def test_whole_file_ancestor_covers_every_hunk(self) -> None: + merged = merge_chain_assignments( + self._child(), [self._parent()], hunk_counts={"shared.py": 2} + ) + by_path = {a.file_path: a for a in merged.assignments} + assert by_path["shared.py"].hunk_indices == [0, 1] + + +class TestMergeChainAssignmentsCarryAncestorFiles: + def _parent(self) -> Group: + return Group( + id="pr-1", + title="parent", + description="parent", + assignments=[ + GroupAssignment( + file_path="parent_only.py", + assignment_type=AssignmentType.WHOLE_FILE, + ), + ], + ) + + def _child(self) -> Group: + return Group( + id="pr-3", + title="child", + description="child", + depends_on=["pr-1"], + assignments=[ + GroupAssignment( + file_path="child.py", + assignment_type=AssignmentType.PARTIAL_HUNKS, + hunk_indices=[0], + ), + ], + ) + + def test_ancestor_only_file_is_carried(self) -> None: + merged = merge_chain_assignments( + self._child(), + [self._parent()], + hunk_counts={"parent_only.py": 1, "child.py": 1}, + carry_ancestor_files=True, + ) + by_path = {a.file_path: a for a in merged.assignments} + assert by_path["parent_only.py"].hunk_indices == [0] + + +class TestAddedFileLineEndings: + def test_added_file_content_is_not_double_spaced(self) -> None: + parsed = parse_diff(NEW_FILE_DIFF) + group = Group( + id="pr-1", + title="t", + description="t", + assignments=[ + GroupAssignment( + file_path="new_file.py", + assignment_type=AssignmentType.WHOLE_FILE, + ) + ], + ) + result = materialize_group_files(parsed, group, "abc123") + assert result["new_file.py"] == 'def hello():\n return "world"\n\n' diff --git a/tests/test_schemas.py b/tests/test_schemas.py index 4a2de9f..62a87a1 100644 --- a/tests/test_schemas.py +++ b/tests/test_schemas.py @@ -1,7 +1,7 @@ from __future__ import annotations -from pr_split.constants import AssignmentType -from pr_split.schemas import Group, GroupAssignment +from pr_split.constants import AssignmentType, Priority +from pr_split.schemas import Group, GroupAssignment, SplitPlan class TestGroup: @@ -50,3 +50,39 @@ def test_partial_hunks_assignment(self) -> None: ) assert assignment.assignment_type == AssignmentType.PARTIAL_HUNKS assert assignment.hunk_indices == [1] + + +class TestSplitPlanStacked: + def test_defaults_to_flat(self) -> None: + plan = SplitPlan( + dev_branch="dev", base_branch="main", max_loc=400, priority=Priority.ORTHOGONAL + ) + assert plan.stacked is False + + def test_stacked_round_trips_through_plan_file(self) -> None: + plan = SplitPlan( + dev_branch="dev", + base_branch="main", + max_loc=400, + priority=Priority.ORTHOGONAL, + stacked=True, + ) + assert SplitPlan.model_validate(plan.model_dump()).stacked is True + + +class TestSplitPlanDraft: + def test_defaults_to_ready(self) -> None: + plan = SplitPlan( + dev_branch="dev", base_branch="main", max_loc=400, priority=Priority.ORTHOGONAL + ) + assert plan.draft is False + + def test_draft_round_trips_through_plan_file(self) -> None: + plan = SplitPlan( + dev_branch="dev", + base_branch="main", + max_loc=400, + priority=Priority.ORTHOGONAL, + draft=True, + ) + assert SplitPlan.model_validate(plan.model_dump()).draft is True diff --git a/tests/test_validator.py b/tests/test_validator.py index 7ad0230..8dc4b82 100644 --- a/tests/test_validator.py +++ b/tests/test_validator.py @@ -172,3 +172,26 @@ def test_detect_loc_bound_violations_returns_structured_result(self) -> None: assert violations[0].group_id == "g1" assert violations[0].violation_type == LocViolationType.BELOW_MIN assert violations[0].limit == 50 + + +class TestValidateCoverageWholeFileExpansion: + def test_whole_file_with_empty_indices_counts_as_full_coverage(self) -> None: + parsed = parse_diff(SAMPLE_DIFF) + groups = [ + _make_group("g1", [_ga("a.py", WHOLE, [])], 3), + _make_group("g2", [_ga("b.py", WHOLE, [0])], 4), + ] + validate_coverage(groups, parsed) + + def test_whole_file_overlapping_same_file_partial_raises(self) -> None: + parsed = parse_diff(SAMPLE_DIFF) + groups = [ + _make_group("g1", [_ga("a.py", WHOLE, [])], 3), + _make_group( + "g2", + [_ga("a.py", PARTIAL, [0]), _ga("b.py", WHOLE, [0])], + 7, + ), + ] + with pytest.raises(PlanValidationError, match="multiple groups"): + validate_coverage(groups, parsed)