Skip to content
Goatfied

security

Pull request summaries that engineers actually read

Pull request summaries improve code review quality by stating what changed, why it changed, and what risks need scrutiny without assuming shared context.

2026-02-2810 min readBy Goatfied
Pull request summaries that engineers actually read

Most pull request summaries fail because they try to do too much or too little. Engineers either write novels that no one reads, or one-liners like "Fix auth issues" that force reviewers to excavate context from a 47-file diff. The result is the same: reviewers skim 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.

Write for the reviewer who wasn't in the room

The best PR summaries assume the reviewer missed the Slack thread, the design doc, and the standup where you explained the approach. Start with the specific symptom or requirement, not the implementation details.

Bad: "Refactored authentication middleware to use dependency injection"

Good: "Auth tokens were validating on every request including 10k/sec health checks, causing 300ms p99 latency spikes. Now we skip validation for paths in the public allowlist. Risk: if the allowlist is misconfigured, previously-public endpoints could become auth-gated."

The second version answers why anyone should care and surfaces the failure mode that matters. The reviewer can now ask "how do we test the allowlist?" instead of rubber-stamping a clean diff.

Engineers reviewing code are context-switching from their own work. They need to know three things in under 20 seconds: what problem you're solving, why this approach, and what could go wrong. Everything else is archaeology.

The three-part structure that works

Every PR summary should answer three questions in order:

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 and edges – Direct the reviewer to the parts that need scrutiny and call out what you didn't test. "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. Didn't test with >100k concurrent sessions (our p99 is 8k)." 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.

For database migrations or infrastructure changes, add deployment notes:


**Problem:** Query planner was doing full table scans on user_id lookups 

in the events table (180ms p95, caused timeouts in the dashboard).



**Approach:** Added composite index on (user_id, created_at) since we 

always filter by user and sort by time. Skipped a covering index because 

it would bloat the table by 40% for a query we run <100x/day.



**Edges:**

- Index builds in the background (no downtime) but takes ~15min in prod

- Rollback: `DROP INDEX CONCURRENTLY idx_events_user_created`

- Didn't test with >100k events per user (our p99 is 8k)

This tells the reviewer what to verify, what to watch during deploy, and what you consciously didn't handle. It takes two minutes to write and saves twenty minutes of back-and-forth.

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. For infrastructure changes, include the actual command output:


terraform plan -out=plan.tfplan

  + aws_s3_bucket.assets

      acl: "private"

      versioning: enabled

This eliminates "did you check what this actually does" questions and makes review concrete.

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.

Include links to the specific log line that showed the problem, the benchmark proving the optimization, or the customer report that motivated the fix. Raw evidence beats "users complained" because it lets reviewers verify your diagnosis. If you load-tested the change, paste the key numbers:


Before: p50 85ms / p95 340ms / errors 0.3%

After:  p50 62ms / p95 180ms / errors 0.08%



Ran 10k req/sec for 5min against staging (commit abc123f).

These numbers aren't inventions—they're real measurements you should be taking anyway. If you didn't load test, say "Tested manually with N=20 requests; didn't load test because traffic to this endpoint is <10 req/hour."

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."

State what you're modifying and the threat model you're working under. "Allow anonymous read access to /public prefix in assets bucket—no customer data stored here, CDN pulls from this path" makes the security decision explicit and auditable. For any change that modifies 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."

Goatfied's agent loop—plan, constrain, edit, validate, retry—helps here because the compile and test gates catch certain classes of regression before review. The "constrain" step enforces compile and lint gates before PRs open. 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.

When your PR involves security-sensitive changes—rotating credentials, modifying access policies, handling PII—the context sitting in a third-party service's audit logs becomes a compliance concern. Self-hosted Goatfied deployments keep your PR history, agent interactions, and validation results entirely within your infrastructure. This matters for compliance: GDPR Article 30 requires you to document processing activities, and SOC2 controls need evidence that code reviews covered security implications.

The validation checklist that catches silent failures

Every PR that touches more than configuration should answer: what validates this actually works? Not in theory—what specific check ran before you opened this?

List the specific tests, lint checks, and manual verification you performed: "Ran make test-auth (18 test cases, all pass). Confirmed no eval/exec in git diff. Verified session tokens still expire after 1hr in local environment. Checked admin routes still return 403 for non-admin users."

This isn't ceremony. This is how you avoid the "worked on my machine" merge that breaks production because no one realized the new library version requires a different authentication flow. If you didn't run certain tests, say why: "Skipped load tests because this endpoint handles <10 req/hour."

The validation step should ground claims in the actual changes. If the summary says "maintains authentication flow" but the diff shows changes to token validation, that's a red flag. The honest version: summaries only work if they're actually grounded. The moment a generated summary claims "all security tests pass" when they didn't run, or says "no authentication changes" when middleware moved, engineers stop reading them. Trustworthiness compounds; so does its absence.

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."

Flag uncertainty and don't explain the obvious

Engineers skip calling out uncertainty because it feels like admitting weakness. In reality, "I'm not sure if this handles Unicode edge cases correctly" is the kind of honesty that catches bugs before production. If you're changing error handling and you're unsure whether retries will amplify failures, say so. The reviewer might know, or they'll help you design a test.

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.

Deployment and rollback for risky changes

Any PR that touches infrastructure, migrations, or external APIs should include deploy notes:

  • "Deploy order: service-a, wait 5min for health checks, then service-b"
  • "Requires FEATURE_NEW_PARSER=true in env vars"
  • "Rollback: re-deploy previous commit + run ./scripts/revert-schema.sh"

For teams using Goatfied's managed environment or self-hosted setups, deployment steps become part of the commit's audit trail. The agent's plan-edit-validate loop ensures diffs are small and reversible, but only if you document the reversal process. A one-liner revert script is worth a thousand words during an incident.

Small reversible diffs make this practical. Instead of one massive PR touching 40 files, you ship five focused PRs each changing 8 files. Each has a clear summary, focused review, and isolated rollback path. Your audit trail becomes a sequence of specific, justified decisions rather than archaeology through a giant merge commit.

Make it searchable

Six months later, someone will grep your repo's PRs trying to figure out when a behavior changed. Use keywords: if you're changing how rate limiting works, use "rate limiting" in the summary, not just "updated throttling." If you're fixing a security issue, call it "security fix" so it's findable during audits.

For compliance-heavy environments where Goatfied's audit-friendly workflows matter, this isn't optional. Auditors need to trace "when did we start encrypting this field" without reading every line of every PR. PR summaries that omit security context create gaps in your audit log. Three months from now when a pentester asks "who approved adding this S3 bucket policy and why is it world-readable," you need more than commit SHAs.

Avoid defending your ego. "This is the best way to do X" invites argument. "This approach trades off Y for Z because we need Z more right now" invites collaboration. And skip disclaimers like "This is just a first pass." If it's not ready, it shouldn't be open for review.

Related posts

Pull request summaries that engineers actually read | Goatfied Blog