molecule-core/docs/engineering/pr-hygiene.md

Pull Request Hygiene

Status: Guide. Violations are a review-time flag, not a CI gate. Audience: Humans and agents opening PRs in this repo. Cross-refs: testing-strategy.md, backends.md

Why this exists

On 2026-04-23 a backlog audit found 23 open PRs on molecule-core, of which 8 had accumulated 70-380 files of bloat (+2000/-8000 lines) from stale branch drift. The underlying fix in each was 1-5 files; the rest was merge artifact. Half the PRs were closed that day because they weren't reviewable and the real fix had to be re-extracted onto a clean branch.

This document captures the patterns that avoid that outcome.

The rules

1. Small PRs, single concern

Change size	Reviewability
≤100 lines	✅ Good. One sitting.
100-300 lines	⚠️ Acceptable if genuinely one logical change.
300-1000 lines	🔴 Too large. Split.
1000+ lines	🚫 Unreviewable — split before opening.

Exception: complete file deletions and automated refactors where the reviewer only needs to verify intent.

2. Branch hygiene — rebase, don't merge-in

When your branch falls behind the base:

Do:

git fetch origin staging
git rebase origin/staging
# resolve conflicts
git push --force-with-lease

Don't:

git fetch origin staging
git merge origin/staging  # creates merge commit + pulls ALL of base's files into your diff

A merge commit from origin/staging brings every base-branch commit into your PR's diff. That's where the 235-file bloat comes from. Once you have it, you can't get rid of it without resetting the branch.

3. If your branch has already drifted — cherry-pick onto fresh base

# Identify your real commits
git log origin/staging..HEAD

# Create a fresh branch off current base
git checkout -b your-branch-clean origin/staging

# Cherry-pick only the commits you actually authored
git cherry-pick abc1234 def5678

# Push and open a new PR; close the old one as "superseded by #N"
git push -u origin your-branch-clean

Don't try to rebase a drifted branch interactively to remove the base-branch commits. It fights you every merge.

4. Target staging unless you're doing a staging→main promote

Per branching policy (feedback memory rule): every change lands on staging first. Once validated there, a periodic chore: sync staging → main PR promotes the bundle.

Exception: hotfixes that also land on main directly with CEO approval.

5. Describe the why, not the what

A good PR title:

• fix(provision): write org_instances row BEFORE readiness check to unblock boot-event auth

A bad PR title:

• Update orgs.go
• Fix bug
• Phase 1

The body should explain:

• What's broken / missing (or what's the opportunity)
• Why this fix — especially if there are alternatives you considered
• What's tested — which scenarios the test plan covers
• What's deferred — if there are follow-ups, file issues and link them

Anti-pattern: ## Summary\n- Fix bug. That's not a summary; that's a stub.

6. Close the loop on review comments

• Comments labeled Nit: / Optional: / FYI can be left for follow-up — but leave a reply acknowledging.
• Critical/required comments need a fix or a justified reply before merge.
• Don't resolve threads without replying — silent resolves read as dismissal.

7. CI must be green (or the failure must be acknowledged)

• Never push --no-verify unless explicitly requested.
• If a pre-existing failure is blocking merge, document it inline and file a tracking issue — don't silently let it erode the "all green" norm.

Patterns for specific situations

Re-targeting an old branch

When a PR was opened weeks ago against main but policy now says staging:

git fetch origin staging
git rebase --onto origin/staging old-base HEAD
git push --force-with-lease
# Edit the PR's base branch in GitHub UI

Splitting a large PR

If your PR is already open and the reviewer asks for a split:

1. Identify the cleanest split boundary — usually along file groups or dependency layers.
2. Create two new branches off current staging.
3. Cherry-pick the commits for each concern into its branch.
4. Open two new PRs, close the original as "superseded by #A and #B".

Marketing / docs-heavy PRs

Marketing content has been moved to an internal repo per commit 93324e7. If your PR modifies files under docs/marketing/campaigns/, docs/marketing/plans/, or docs/marketing/briefs/ (with non-public-facing strategy content):

1. Check if the file still exists on origin/staging.
2. If deleted, open the PR in the internal marketing repo instead.
3. Public-facing marketing (blog posts, SEO pages under docs/blog/) stays in this repo.

Signs your PR has a hygiene problem

• 70+ files changed when your commit message mentions 2-3 files
• +2000/-3500 lines but the actual fix is ~100 lines
• State: DIRTY in GitHub for >1 day
• Filenames in the diff you don't recognize (someone else's changes in your PR)
• Merge commits in your branch's log named Merge remote-tracking branch 'origin/staging' into ...

If you see any of these, don't try to "clean it up in place" — cherry-pick onto a fresh branch (rule 3 above).

Related

• Issue #1822 — backend parity drift tracker (example of docs that have to stay current)
• Postmortem: CP boot-event 401 — caught before shipping because a reviewer could read the diff