PR Cleanliness

Overview

PR cleanliness is about ensuring every change in a pull request serves the PR's stated purpose. A clean PR contains only the changes necessary to accomplish its goal -- nothing more, nothing less. This skill provides the knowledge base for analyzing and cleaning PRs.

Why PR Cleanliness Matters

Noisy PRs cause review fatigue, hide bugs, and slow teams down -- clean PRs get reviewed faster and merge sooner.

Issue Categories

1. Mixed Concerns

Changes serving multiple unrelated purposes bundled in one PR. A bug fix mixed with a refactor. A new feature mixed with dependency updates. Each distinct purpose should be its own PR.

Detection signals:

• Files from unrelated modules or features modified together
• Commit messages describing different types of work (fix + feat + refactor)
• Changes that could be reverted independently without affecting each other

2. Debug Artifacts

Code added during development that should not ship (e.g., console.log, debugger, print(), TODO/FIXME comments, commented-out code blocks). See references/anti-patterns.md for the full catalog with language-specific regex patterns.

3. Formatting Noise

Changes that alter appearance without changing behavior -- whitespace-only changes, import reordering, blank line additions in untouched code. These inflate the diff and obscure real changes. See references/anti-patterns.md for detection approaches.

4. Scope Creep

Changes that go beyond the PR's purpose, even if individually reasonable -- renames in untouched files, type annotations added to existing code, drive-by refactors. These should be separate PRs. See references/anti-patterns.md for detection signals.

5. PR Size

PRs that are too large to review effectively. General thresholds: >400 lines changed or >15 files modified warrants attention. >800 lines or >30 files strongly suggests splitting. See references/anti-patterns.md for size guidelines and contextual adjustments.

6. Codebase Integration

New code that overlaps with or diverges from the existing codebase. These issues can only be detected by comparing new code against what already exists in the project.

• Duplicate functions: New function or class definitions that closely mirror existing implementations in the codebase (similar name AND similar logic)
• Reimplemented utilities: Custom implementations of functionality already available via the project's utility modules or imported libraries
• Pattern divergence: New code that uses different conventions (error handling, async style, naming) than the established majority (>70% threshold) in the same module

These findings require human judgment -- they are never auto-fixable and are never Error severity.

Detection Approach

To analyze a PR for cleanliness issues:

1. Establish context: Determine the PR's purpose from branch name, commit messages, and (if available) PR description
2. Get the diff: Compare current branch against base branch using git diff origin/<base>...HEAD
3. Catalog changes: List all modified files with change type (added/modified/deleted) and line counts
4. Scan for artifacts: Search the diff for debug patterns, formatting noise, and commented-out code
5. Assess scope: Determine whether each file's changes serve the PR's stated purpose
6. Evaluate size: Check total diff size and file count against thresholds
7. Check codebase integration: Compare new definitions against existing codebase for duplicates, reimplemented utilities, and pattern divergence
8. Report findings: Group issues by category with file:line references and severity

Severity Levels

Use references/severity-matrix.md as the canonical severity policy. Do not redefine severity mappings in commands or agent prompts.

Base Branch Detection

To determine the base branch for comparison:

1. Resolve name: Check if a PR exists for the current branch via gh pr view --json baseRefName. Fall back to common defaults: main, master, develop.
2. Fetch and resolve to remote ref: Run git fetch origin <base> 2>/dev/null and use origin/<base> for all subsequent diff/log commands. This ensures the comparison matches what GitHub sees, even when the local <base> branch is stale. If the argument already starts with origin/, use it as-is without redundant fetch.
3. Fall back with warning: If the fetch fails (no network, no remote, origin/<base> does not exist), fall back to the local <base> ref and warn: "Could not fetch remote ref -- comparing against local <base> which may be stale."
4. Use git merge-base to find the actual divergence point

Additional Resources

Reference Files

For a comprehensive catalog of anti-patterns with language-specific examples and detection regex patterns, consult:

• references/anti-patterns.md -- Detailed anti-pattern catalog with regex patterns for detection, organized by language and category
• references/severity-matrix.md -- Canonical severity mapping used by all PR cleanliness commands and agents

Agents

• agents/codebase-analyzer.md -- Cross-codebase analysis agent for detecting duplicates, reimplemented utilities, and pattern divergence