From ad8e59e294f4ead66024266d240c70a57d631688 Mon Sep 17 00:00:00 2001 From: vitali87 Date: Sun, 30 Aug 2026 13:05:22 +0100 Subject: [PATCH] fix: honour PR_SPLIT_* env vars for strategy and priority options README documents PR_SPLIT_PRIORITY, PR_SPLIT_CHUNK_STRATEGY and PR_SPLIT_PARTITION_STRATEGY, but the split command's options had no envvar binding, so their explicit defaults always overrode the environment. Bind them (plus the undocumented --cp-sat-timeout) and document the flag and its variable. --- README.md | 2 ++ pr_split/cli.py | 26 +++++++++++++--- tests/test_cli_new_features.py | 55 +++++++++++++++++++++++++++++++++- 3 files changed, 78 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index 01a0b24..c06a705 100644 --- a/README.md +++ b/README.md @@ -82,6 +82,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 | @@ -192,6 +193,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 | diff --git a/pr_split/cli.py b/pr_split/cli.py index 7121b06..c475502 100644 --- a/pr_split/cli.py +++ b/pr_split/cli.py @@ -691,15 +691,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, diff --git a/tests/test_cli_new_features.py b/tests/test_cli_new_features.py index e668f4e..149f75c 100644 --- a/tests/test_cli_new_features.py +++ b/tests/test_cli_new_features.py @@ -13,7 +13,7 @@ _handle_loc_bound_warnings, app, ) -from pr_split.constants import AssignmentType +from pr_split.constants import AssignmentType, ChunkStrategy, PartitionStrategy, Priority from pr_split.exceptions import GitOperationError, PRSplitError from pr_split.git_ops.prs import get_pr_state, merge_pr from pr_split.schemas import ( @@ -280,6 +280,59 @@ def test_split_uses_max_loc_envvar( settings = mock_plan_split.call_args[0][1] assert settings.max_loc == 123 + @patch("pr_split.cli.save_plan") + @patch("pr_split.cli.merge_base", return_value="abc123") + @patch("pr_split.cli._interactive_edit") + @patch("pr_split.cli._present_plan") + @patch("pr_split.cli.validate_plan", return_value=[]) + @patch("pr_split.cli.plan_split") + @patch("pr_split.cli.parse_diff") + @patch("pr_split.cli.extract_diff", return_value="diff --git a/a.py b/a.py\n") + @patch("pr_split.cli._validate_inputs") + @patch("pr_split.cli.branch_exists", return_value=True) + def test_split_uses_strategy_envvars( + self, + mock_branch_exists: MagicMock, + mock_validate_inputs: MagicMock, + mock_extract_diff: MagicMock, + mock_parse_diff: MagicMock, + mock_plan_split: MagicMock, + mock_validate_plan: MagicMock, + mock_present_plan: MagicMock, + mock_interactive_edit: MagicMock, + mock_merge_base: MagicMock, + mock_save_plan: MagicMock, + ) -> None: + parsed_diff = MagicMock() + parsed_diff.stats = { + "total_files": 1, + "total_added": 1, + "total_removed": 0, + "total_loc": 1, + } + group = _group("pr-1", "t", files=["a.py"]) + mock_parse_diff.return_value = parsed_diff + mock_plan_split.return_value = [group] + mock_interactive_edit.return_value = [group] + + result = runner.invoke( + app, + ["split", "feature-branch", "--dry-run"], + env={ + "PR_SPLIT_PARTITION_STRATEGY": "graph", + "PR_SPLIT_PRIORITY": "logical", + "PR_SPLIT_CHUNK_STRATEGY": "greedy", + "PR_SPLIT_CP_SAT_TIMEOUT": "2.5", + }, + ) + + assert result.exit_code == 0, result.output + settings = mock_plan_split.call_args[0][1] + assert settings.partition_strategy == PartitionStrategy.GRAPH + assert settings.priority == Priority.LOGICAL + assert settings.chunk_strategy == ChunkStrategy.GREEDY + assert settings.cp_sat_timeout == 2.5 + @patch("pr_split.cli.save_plan") @patch("pr_split.cli.merge_base", return_value="abc123") @patch("pr_split.cli._interactive_edit")