Skip to main content

PR Description Guide

Five PRs merged in one week opened with Closes #N followed by dataclass field names. A security analyst cannot read those. An on-call engineer cannot triage them. This guide exists so the next PR doesn't need a retroactive rewrite.

Rule: Lead with the human problem. Put the code inside.

Choose the variant for your PR​

Three variants, shortest-first. Use the smallest one that fits.

1. Minimal — for docs and chore PRs​

## TL;DR
One-line human-readable summary of what this adds or changes.

## What changed
- Bullet list of files or sections affected.

Closes #N

That's it. No test plan, no evidence section, no technical detail. A docs PR that includes a 5-section full template is noise.

2. Medium — for fix PRs​

## TL;DR
What was broken; who felt it; what it looks like now that it's fixed.

## Evidence
Before/after: API response, screenshot, log snippet, or reconciliation number.
"npm run ci passes" is not evidence. Show observable behavior.

## Test plan
- Open [specific page or endpoint] — confirm [observable outcome]
- Run `[specific command]` — expected: [specific output]

Closes #N

What changed can be folded into the TL;DR for a single-focus fix. Technical detail goes in a commit message, not the PR body.

3. Full — for feat PRs (user-visible behavior)​

## TL;DR
<!-- One sentence. Name the affected user, operator, or page. No code terms. -->

## What changed
<!-- Bullets in user/operator terms. What can someone now do or see? -->
-
-

## Evidence
<!-- Screenshot, smoke-test output, or before/after log snippet. -->

## Test plan
<!-- Named page or command + expected observable output. -->
- Open [specific page/endpoint] — confirm [observable outcome]
- Run `[specific command]` — expected: [specific output]

## Technical detail
<!-- File names, type changes, blast radius. Internal detail goes here, not above. -->

Closes #N

Before / After example — PR #84 (sv0-connectors, fix variant)​

Before (what the agent initially wrote):

Closes #82

What this does

  • Adds ARM role assignment enumeration at account-resource scope
  • Extends RoleAssignment dataclass with scope_level field
  • Queries /subscriptions/{sub}/resourceGroups/{rg}/providers/Microsoft.CognitiveServices/accounts/{name}/providers/Microsoft.Authorization/roleAssignments at account level
  • 118 unit tests pass

Test plan

  • pytest passes

Every sentence names a dataclass, an ARM endpoint, or a test count. Zero sentences describe who was affected or what they saw wrong.

After (Medium variant — a fix PR):

TL;DR​

External service principals like servicenow-openai-client that hold direct ARM role assignments on a Cognitive Services account were completely invisible to the connector — the platform showed zero AI access paths for those principals even when they had live access.

Evidence​

Before: GET /identity/workloads returned 0 role assignments for servicenow-openai-client After: same endpoint returns 1 entry with role: Cognitive Services OpenAI Contributor, scope: /subscriptions/.../accounts/foundry-pilot

Test plan​

  • Run azure-foundry --all --graph-json /tmp/out.json against the pilot tenant — confirm servicenow-openai-client appears in the nodes array with type: workload
  • Open the Authority Paths page — confirm the service principal is listed with at least one access path to the Foundry account

Closes #82

The second version can be read by an analyst, triaged by an on-call engineer, and verified by a reviewer — without reading the diff.

Red flags — stop and rewrite if you see these​

In your TL;DR / opening paragraph:

  • Starts with Closes #N — move this to the bottom
  • First noun is a file name, class name, or dataclass field — reframe as a symptom
  • "This PR adds/removes/changes X" with no mention of who cares — add who is affected
  • "Follow-up to #N" with no other context — the reader hasn't read #N

In Evidence:

  • "npm run ci passes" or "1159 unit tests pass" as the only evidence — show behavior, not test counts
  • No Evidence section at all for a behavior-changing PR — add a smoke test result or before/after count

In Test plan:

  • Only CI commands: npm run ci, pytest, make lint — add at least one human-observable step
  • No expected output specified — "run X and check" is not a test plan; "run X and see Y" is

In the overall body:

  • Full 5-section template on a docs or chore PR — use the Minimal variant
  • Pure mechanism description with no symptom — e.g., "Scopes diff-engine relationship comparison to current connector" says what changed but not what broke
  • Blast radius section that names database collections but not what a user would observe as wrong behavior — translate at least one item

Retroactive context​

Five PRs merged 2026-04-20 had their bodies rewritten after the fact:

  • #83 — Added: analyst couldn't flag Azure AI accounts where local auth was still enabled
  • #84 — Added: external service principals invisible in the identity graph (example above)
  • #459 — Had a smoke test but no non-engineer summary; opening was rewritten
  • #461 — Had a "blast radius" section but the user-visible symptom (false drift events) was item 4 in a list
  • #185 — Best of the batch but buried the most critical finding (integration CI never runs) as a parenthetical

This guide closes that pattern. Future PRs should not need the same fix.