write-pull-request

name	write-pull_request
description	Use when creating or updating a pull request description for an external repository. Use when about to run `gh pr create`, when the user says "write the PR", "create a PR", "update the PR description", or when preparing changes for upstream submission. Also triggers when reviewing existing PRs for description quality. Do NOT trigger for internal branch merges or local-only commits.

Writing Pull Request Descriptions

Write PR descriptions that let a maintainer verify, understand, and merge your changes without back-and-forth. Every sentence earns its place by answering a question the reviewer will have.

The Iron Law

THE REVIEWER'S TIME IS MORE VALUABLE THAN YOURS.

A reviewer who can verify your claims in 30 seconds merges fast. A reviewer who has to reverse-engineer your reasoning requests changes or ignores the PR entirely.

Before Writing: Triage

Before writing a single word of description, answer these questions:

1. Is this a real bug? If the existing behavior produces the same result (e.g., the spec clamps out-of-bounds values), this is a style preference, not a bug. Don't submit it as a bug fix. Marginal PRs dilute the credibility of real ones.

2. Is this single-concern? If you're fixing a shutdown leak AND removing dead code AND fixing a typo, that's three PRs. Each PR should be reviewable in isolation. Mixed-concern PRs get slower reviews.

3. Can the reviewer verify independently? If your claim requires reading 500 lines of code to confirm, restructure the description to provide a 30-second verification path.

If triage says "don't submit" — close the PR with a clear explanation rather than wasting the reviewer's time. A closed PR with a good reason is better than a merged PR that shouldn't exist.

Description Structure

Every PR description uses this skeleton. Sections are required unless marked optional.

## Problem

[What is broken or missing. Show the actual bad behavior, not just
"there's a bug in X." Include code snippets showing the problematic
line if it helps.]

[How to verify independently — "search for X in src/", "compare
against any other Y file in the repo", "run Z and observe the output."]

## Fix

[What you changed, with each change clearly tied to a specific problem
above. If Problem lists 3 issues, Fix addresses all 3 in the same order.]

## Note (optional)

[Important caveats the reviewer needs to know. E.g., "These tests may
now fail if they were masking real regressions. That is the intended
behavior."]

## Related issues (optional)

[Connections to open issues. E.g., "This fix is necessary (though may
not be sufficient) for #1081 and #1087." Be honest about what this
does and doesn't solve.]

What Each Section Does

Problem serves two purposes: it explains the bug AND gives the reviewer a way to confirm it exists. The verification instruction is not optional — it's what separates a credible PR from a "trust me" PR.

Fix must correspond 1:1 with the problems listed. If you fixed three things, the reviewer should be able to trace each fix back to its problem without guessing.

Note handles the uncomfortable truths — side effects, intentional behavioral changes, things the reviewer might flag as wrong but are actually correct. Put them here rather than waiting for the reviewer to discover them.

Related issues connects your work to the project's open issues. This helps the maintainer understand the broader context and close issues when appropriate.

Writing the Problem Section

Show, Don't Tell

Bad: "There's an off-by-one error in the range calculation."

Good: "_get_range_from_file_content sets end_line = len(lines) which points one past the last valid line (LSP lines are 0-indexed)."

The good version shows the exact code, explains what it does wrong, and references the spec that makes it wrong.

Provide Verification Instructions

Bad: "Several tests catch AssertionError."

Good: "test_request_defining_symbol_method_call catches AssertionError and replaces it with warnings.warn(). If the assertion fails, the test passes silently."

Better: "To verify: search for _is_initialized in src/ — it appears only in this guard, never as an assignment."

The "To verify" pattern lets a reviewer confirm your claim in seconds rather than minutes. Use it whenever possible.

Calibrate Risk Honestly

Bad: "Shell metacharacters enable command injection."

Good: "The realistic risk is low (paths come from config files, not user input), but the fix is trivial and uses the stdlib-standard approach."

Overselling risk makes the reviewer skeptical of your judgment. If a bug is real but unlikely to be exploited, say so.

Use Comparative Evidence

When fixing a pattern error across a codebase, point to the correct pattern:

"Every other LS in the repo uses os.path.basename(). This is not a template variable — Python sends the five characters $, n, a, m, e to the language server."

This lets the reviewer verify by opening any other file in the same directory.

Writing the Fix Section

Match Problem Ordering

If Problem lists three issues, Fix addresses all three in the same order:

## Fix

1. Change guard to `hasattr(self, "_active_project")`, which is set during
   `__init__` and correctly distinguishes initialized vs partially
   constructed instances.

2. Call `self._active_project.shutdown()` before overwriting with the new
   project.

3. Wrap `yield` in `try/finally` to ensure `agent.shutdown()` runs on exit.

Be Specific About the Change

Bad: "Fixed the function."

Good: "On POSIX, use shlex.quote() which single-quotes arguments, preventing all shell interpretation. Retain the original Windows behavior since cmd.exe does not interpret single-quoted strings."

Branch and Commit Discipline

Branch Names

Use descriptive, categorized branch names:

fix/agent-shutdown-lifecycle
fix/copy-paste-init-params
fix/unfalsifiable-tests

Not: patch-1, fix-stuff, my-changes

Clean Commits

Each PR should contain a single logical commit (or a clean series where each commit builds on the last). If your branch has messy history from development, clean it up before submitting:

• Rebase on the upstream base branch (usually main)
• Squash work-in-progress commits
• Write a commit message that matches the PR title

Fork and Upstream Hygiene

When contributing to an external repo:

# Ensure branch is based on latest upstream
git fetch upstream
git checkout -b fix/descriptive-name upstream/main

# After making changes, push to your fork
git push origin fix/descriptive-name

# Create PR from your fork to upstream
gh pr create --repo owner/repo --head yourname:fix/descriptive-name \
  --base main --title "..." --body "..."

PR Title

Keep titles under 70 characters. They should describe the change, not the problem:

Good:

• "Fix agent shutdown lifecycle: no-op guard, orphaned processes"
• "Use shlex.quote for shell argument escaping on POSIX"
• "Fix copy-paste bugs in language server init params"

Bad:

• "Fix bugs" (which bugs?)
• "Update ls.py" (what did you change?)
• "Fix issue where the shutdown method doesn't work because the _is_initialized attribute is never set" (too long, too detailed)

When to Close Instead of Merge

Close a PR when:

• The fix is functionally equivalent to the original — e.g., the spec clamps out-of-bounds values, so both the old and new code produce the same result. Close with: "On closer review, [spec behavior] makes both values equivalent at runtime. Not a real bug."

• The scope is wrong — you mixed concerns. Close, split into separate PRs, resubmit.

• The maintainer's feedback reveals a misunderstanding — you misread the codebase. Close with an honest explanation.

Always close with a comment explaining why. A closed PR without explanation looks like abandoned work.

Common Mistakes

Mistake	Fix
Mixed concerns in one PR	Split into separate PRs, one concern each
"Trust me" descriptions with no verification path	Add "To verify:" instructions
Overselling bug severity	Honestly assess realistic risk
Listing changes without explaining problems	Lead with Problem, then Fix
Submitting marginal fixes as bug fixes	Triage first — close if not a real bug
Messy branch history	Rebase on upstream, squash WIP commits
No connection to open issues	Search issue tracker, link related issues
Generic PR titles	Describe the change specifically in under 70 chars
Forgetting to explain caveats	Use the Note section for side effects
Submitting without re-reading the description	Read it as if you're the reviewer seeing it for the first time

Red Flags — You Are Writing a Bad PR

Thought	Reality
"The code speaks for itself"	Reviewers have 50 PRs in their queue. Explain yourself.
"It's obvious what this fixes"	What's obvious to you after 3 hours of investigation is not obvious to someone reading a diff cold.
"I'll add the description later"	You won't. Write it now while the reasoning is fresh.
"One big PR is faster than three small ones"	For you, maybe. For the reviewer, three focused PRs are 3x faster to review.
"The tests pass, that's enough"	Passing tests prove correctness. The description proves the change is necessary and intentional.
"I'll just mention the issue number"	Issue numbers are context, not explanation. The PR must stand alone.

Workflow Integration

This skill is typically invoked at the end of an implementation cycle:

1. Implementation complete, tests passing, code audited
2. This skill activates — write the PR description
3. gh pr create with the description
4. Monitor for reviewer feedback

If the PR needs updating after review feedback:

• Re-read the feedback carefully
• Update the code
• Update the description to reflect the changes
• Force-push the cleaned branch
• Comment on the PR explaining what changed