tools-only Pull Request Description Guidelines
Good PR descriptions help reviewers understand the context and impact of your changes. They enable faster reviews, better decision-making, and serve as valuable historical documentation.
When creating a PR, follow the GitHub PR template and use these guidelines to fill it out effectively.
Who's Reading Your PR?
Write for senior engineers familiar with the SDK. Assume your reader:
- Understands the SDK's architecture and patterns
- Has context about the broader system
- Can read code diffs to understand implementation details
- Values concise, focused communication
What to Include
Every PR description should have:
- Motivation — Why is this change needed?
- Public API Changes — What changes to the public API (with code snippets)?
- Use Cases (optional) — When would developers use this feature? Only include for non-obvious functionality; skip for trivial changes or obvious fixes.
- Breaking Changes (if applicable) — What breaks and how to migrate?
Writing Principles
Focus on WHY, not HOW:
- ✅ "Hook providers need access to the agent's result to perform post-invocation actions like logging or analytics"
- ❌ "Added result field to AfterInvocationEvent dataclass"
Document public API changes with example code snippets:
- ✅ Show before/after code snippets for API changes
- ❌ List every file or line changed
Be concise:
- ✅ Use prose over bullet lists when possible
- ❌ Create exhaustive implementation checklists
Emphasize user impact:
- ✅ "Enables hooks to log conversation outcomes or trigger follow-up actions based on the result"
- ❌ "Updated AfterInvocationEvent to include optional AgentResult field"
What to Skip
Leave these out of your PR description:
- Implementation details — Code comments and commit messages cover this
- Test coverage notes — CI will catch issues; assume tests are comprehensive
- Line-by-line change lists — The diff provides this
- Build/lint/coverage status — CI handles verification
- Commit hashes — GitHub links commits automatically
Anti-patterns
❌ Over-detailed checklists:
### Type Definition Updates
- Added result field to AfterInvocationEvent dataclass
- Updated Agent._run_loop to capture and pass AgentResult
❌ Implementation notes reviewers don't need:
## Implementation Notes
- Result field defaults to None
- AgentResult is captured from EventLoopStopEvent before invoking hooks
❌ Test coverage bullets:
### Test Coverage
- Added test: AfterInvocationEvent includes AgentResult
- Added test: result is None when structured_output is used
Good Examples
✅ Motivation section:
## Motivation
Hook providers often need to perform actions based on the outcome of an agent's
invocation, such as logging results, updating metrics, or triggering follow-up
workflows. Currently, the `AfterInvocationEvent` doesn't provide access to the
`AgentResult`, forcing hook implementations to track state externally or miss
this information entirely.
✅ Public API Changes section:
## Public API Changes
`AfterInvocationEvent` now includes an optional `result` attribute containing
the `AgentResult`:
```python
# Before: no access to result
class MyHook(HookProvider):
def on_after_invocation(self, event: AfterInvocationEvent) -> None:
# Could only access event.agent, no result available
logger.info("Invocation completed")
# After: result available for inspection
class MyHook(HookProvider):
def on_after_invocation(self, event: AfterInvocationEvent) -> None:
if event.result:
logger.info(f"Completed with stop_reason: {event.result.stop_reason}")
```
The `result` field is `None` when invoked from `structured_output` methods.
✅ Use Cases section:
## Use Cases
- **Result logging**: Log conversation outcomes including stop reasons and token usage
- **Analytics**: Track agent performance metrics based on invocation results
- **Conditional workflows**: Trigger follow-up actions based on how the agent completed
Template
## Motivation
[Explain WHY this change is needed. What problem does it solve? What limitation
does it address? What user need does it fulfill?]
Resolves: #[issue-number]
## Public API Changes
[Document changes to public APIs with before/after code snippets. If no public
API changes, state "No public API changes."]
```python
# Before
[existing API usage]
# After
[new API usage]
```
[Explain behavior, parameters, return values, and backward compatibility.]
## Use Cases (optional)
[Only include for non-obvious functionality. Provide 1-3 concrete use cases
showing when developers would use this feature. Skip for trivial changes obvious fixes..]
## Breaking Changes (if applicable)
[If this is a breaking change, explain what breaks and provide migration guidance.]
### Migration
```python
# Before
[old code]
# After
[new code]
```
Why These Guidelines?
Focus on WHY over HOW because code diffs show implementation details, commit messages document granular changes, and PR descriptions provide the broader context reviewers need.
Skip test/lint/coverage details because CI pipelines verify these automatically. Including them adds noise without value.
Write for senior engineers to enable concise, technical communication without redundant explanations.
References
Checklist Items
- Does the PR description target a Senior Engineer familiar with the project?
- Does the PR description give an overview of the feature being implemented, including any notes on key implementation decisions
- Does the PR include a "Resolves #" in the body and is not bolded?
- Does the PR contain the motivation or use-cases behind the change?
- Does the PR omit irrelevant details not needed for historical reference?