Skip to content

fix: validate a custom PR template before any branch is created - #153

Closed
vitali87 wants to merge 3 commits into
fix/pr-body-length-capfrom
fix/validate-pr-template-early
Closed

vitali87 wants to merge 3 commits into
fix/pr-body-length-capfrom
fix/validate-pr-template-early

Conversation

@vitali87

Copy link
Copy Markdown
Owner

Summary

A custom .pr-split/template.md was first rendered inside _create_single_pr — i.e. after every branch had been created and pushed — so a typo in the template surfaced only at PR-creation time, wrapped in PRCreationError. Attribute and index placeholders ({title.nope}, {added[0]}) additionally escaped the friendly "Invalid PR template … Available placeholders …" handler because str.format raises AttributeError/TypeError for them.

Now _render_pr_template catches all five error classes, and a new validate_pr_template() renders the template against sample values in _validate_inputs (skipped on --dry-run, which never opens PRs) and at the start of execute, so a bad template exits 1 with the friendly message before any branch exists — including before the re-split cleanup.

Stacked on #150 (fix/pr-body-length-cap), which touches the same function.

Test plan

  • TestPrTemplateEarlyValidation: five bad placeholder shapes are friendly errors from both the validator and the real render; a valid template with format specs and escaped braces renders; _validate_inputs rejects a bad template (and skips it on dry run); execute rejects it before the confirm prompt — all fail on the base
  • Review compared sample-value vs real rendering across 23 templates (format specs, conversions, attribute/index access): no mismatches
  • 457 tests pass, ruff clean
  • Local review: 5/5

@coderabbitai

coderabbitai Bot commented Aug 30, 2026 •

Copy link
Copy Markdown

Warning

Review limit reached

Next included review available in 56 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: eabea752-4a66-46e1-90d0-15eca15f01d0

📥 Commits

Reviewing files that changed from the base of the PR and between 6126b95 and c4600b0.

📒 Files selected for processing (2)
  • pr_split/cli.py
  • tests/test_cli_coverage.py

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Rebuild the template render/validation on main's _pr_template_path()
(plan-dir aware), keep main's gh-stack and merge-base checks, and patch
commit_exists in the execute test that main's new check now reaches.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015rnEEjXWDBdePRhKMmZr1w
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015rnEEjXWDBdePRhKMmZr1w

# Conflicts:
#	pr_split/cli.py

Copy link
Copy Markdown
Owner Author

This change is already in main: it landed via the merge train #243, which includes this PR's head commit. GitHub can't mark a stacked PR merged when it lands that way, so I'm closing it as done.


Generated by Claude Code

@vitali87 vitali87 closed this Sep 27, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants