security
Pull Request Summaries That Engineers Actually Read Design: practical systems guide
Technical field guide on pull request summaries that engineers actually read design tradeoffs for teams building dependable AI coding workflows.
Most pull request summaries fail because they try to do too much. Engineers either write novels that no one reads, or one-liners that force reviewers to excavate context from the diff. The result is the same: reviewers skim the code without understanding intent, miss edge cases, and rubber-stamp changes that break production.
A good PR summary is a contract between author and reviewer. It states what changed, why it changed, and what risks the reviewer should scrutinize. When done right, it reduces review time while *improving* review quality—because the reviewer knows exactly where to focus their limited attention.
The three-section structure that works
Every PR summary should answer three questions in order: what, why, and where to look.
**What changed** – A single sentence describing the functional change. Not "refactored the auth module" but "moved session validation from middleware to per-route guards." Concrete nouns and verbs. If you can't write this in one sentence, your PR is too large.
**Why this approach** – The constraint or requirement that drove the design. This is where you head off "why didn't you just…" comments. Example: "Per-route guards let us skip validation on health check endpoints without adding conditional logic to middleware. Previous approach checked every request including the 10k/sec health checks." You're not defending the change; you're documenting the tradeoff space so reviewers can evaluate your reasoning.
**Review focus** – Direct the reviewer to the parts that need scrutiny. "Check the session timeout logic in `auth/guards.go:45-67` – I changed the default from 24h to 4h and I want confirmation this won't break long-running admin sessions." This is the most skipped section and the most valuable. It tells the reviewer you've thought about risks and where your confidence is lowest.
Link to requirements or incidents
Every non-trivial PR stems from either a bug, a feature request, or a refactoring need. Link it. Not as a formality—as a forcing function to verify you're solving the right problem.
If the PR fixes an incident: link the postmortem and quote the specific root cause. "From INC-892: 'Session middleware was validating even health check endpoints, causing 300ms p99 latency spikes under load.'" This does two things: proves the problem is real, and lets reviewers verify your fix actually addresses the root cause.
If the PR implements a feature: link the spec or ticket and quote the acceptance criteria. "Per JIRA-1234: 'Admin users must stay logged in for 24 hours; standard users for 4 hours.'" Now the reviewer can spot that your new 4-hour default breaks the admin requirement before it ships.
If the PR is pure refactoring: explain the constraint or debt you're removing. "The middleware pattern made it impossible to add per-route auth rules without if/else chains. This change uses per-route guards so we can declaratively specify auth in route definitions." Refactorings without clear constraints are where bike-shedding lives.
Show, don't describe, the behavior changes
Code snippets in PR descriptions aren't duplication—they're framing. A before/after pair teaches the reviewer what to look for in the full diff.
Bad version: "Simplified error handling in the auth module."
Good version:
// Before: errors got swallowed
if err := validateSession(req); err != nil {
log.Warn("validation failed", err)
return nil // treated as success!
}
// After: errors propagate
if err := validateSession(req); err != nil {
return fmt.Errorf("auth: %w", err)
}
The snippet is 10 lines but it prevents a 15-minute review detour where the reviewer tries to figure out what "simplified" means and whether the change is safe. It also surfaces the *bug* that the refactoring fixed, which might not be obvious from the diff alone.
For config or schema changes, show the before and after values side by side. For API changes, show a curl example or request/response pair. The goal is to let the reviewer build a mental model before diving into implementation details.
Security and reliability implications up front
If your change touches authentication, authorization, input validation, error handling, or data retention, call it out explicitly. Reviewers often skim these sections in large diffs because they look like boilerplate. Your job is to make the risk visible.
"This PR changes session timeout from 24h to 4h for standard users. Admin users keep 24h via the `role=admin` attribute check in `validateSession`. **Risk**: if the role check has a bug, admins get logged out after 4h. I've added a test covering this in `auth_test.go:203-215` but would like a second set of eyes on the role attribute logic."
Goatfied's agent loop helps here because the compile and test gates catch certain classes of regression before review. If your PR passes `goatfied validate`, mention it: "All existing auth tests pass including the new admin session longevity test." This isn't a substitute for human review—it's evidence that you've verified the mechanical correctness, so the reviewer can focus on design and edge cases.
For changes that modify data validation or permissions: list the threat model updates. "This PR lets admins delete any user's sessions. Previous behavior: users could only delete their own sessions. Added `require_admin` guard and audit log on delete." Now the security reviewer knows exactly what privilege escalation to scrutinize.
Call out what you didn't change
Large PRs often include incidental changes—imports get reordered, linting fixes get applied, test fixtures get updated. These bury the actual logic changes in noise.
Use the summary to spotlight what stayed the same: "This PR refactors session validation but does **not** change any timeout values, permission checks, or error responses. The external API is identical." Then in review comments, point out the incidental changes: "The diff in `auth/middleware.go` is pure deletion—moving this code to per-route guards—no logic changes."
If you're making a risky change in a high-traffic area, explicitly state what production risk mitigations are in place: "This changes request validation, but I'm using a feature flag `auth_v2_guards` defaulting to false. Plan is to enable for 1% of traffic after merge and monitor error rates."
Don't explain the obvious; explain the non-obvious
Skip sentences like "I updated the tests to reflect the new behavior." Of course you did. Instead: "The new test `TestAdminSessionPersistence` is flaky in local dev because it depends on system time. I'm using `faketime.Now()` to control the clock but the test might be brittle if we change time mocking strategy."
Skip "this improves performance." Instead: "This removes 10k/sec unnecessary auth checks on health endpoints. I haven't load tested yet—if you think we need SLO confirmation before merge, I can run a staging benchmark."
The pattern: assume the reviewer is competent and reading the code. Your summary documents intent, tradeoffs, and the questions you want them to ask.
Related reading
- [Security playbook #5: practical Goatfied tactics for shipping PRs](/blog/goatfied-security-playbook-5)
- [Security playbook #13: practical Goatfied tactics for shipping PRs](/blog/goatfied-security-playbook-13)
