Guide agents(s) to be more concise on issue & PR descriptions (#338)

zastrowm · web-flow · commit accfba996967 · 2025-12-08T14:03:57.000-05:00
Update agent guidance to include more useful Pull-Request guidance.

I've noticed our agents generate pretty bad descriptions in pull-requests, so I've been playing around with better guidance - specifically by:

- Documenting our expectations around pull-requests
- Guiding the agent to use a checklist to validate the PR description

The PR.md file can also be used by regular contributors for guidance on how to create PRs

I also had a small change here to stop including effort estimations in task creation, since I think it's unless noise.

---------

Co-authored-by: Mackenzie Zastrow &lt;zastrowm@users.noreply.github.com&gt;
diff --git a/.github/agent-sops/task-implementer.sop.md b/.github/agent-sops/task-implementer.sop.md
@@ -98,7 +98,7 @@ Create a comprehensive list of test scenarios covering normal operation, edge ca
 - You MUST check for existing testing strategies documented in the repository documentation or your notes
 - You MUST cover all acceptance criteria with at least one test scenario
 - You MUST define explicit input/output pairs for each test case
-- You MUST make note of these test scenarios 
+- You MUST make note of these test scenarios
 - You MUST design tests that will initially fail when run against non-existent implementations
 - You MUST NOT create mock implementations during the test design phase because tests should be written based solely on expected behavior, not influenced by implementation details
 - You MUST focus on test scenarios and expected behaviors rather than detailed test code in documentation
@@ -234,23 +234,40 @@ If the implementation meets all requirements and follows established patterns, p
 
 If all tests are passing, draft a conventional commit message, perform the git commit, and create/update the pull request.
 
+**PR Checklist Verification (REQUIRED):**
+
+Before creating or updating a PR, you MUST copy the checklist from [docs/PR.md](../../docs/PR.md) into your progress notes and explicitly verify each item. For each checklist item, you MUST:
+
+1. Copy the checklist item verbatim
+2. Mark it as `[x]` (pass) or `[ ]` (fail)
+3. If failed, revise the PR description until the item passes
+
+Example format in your notes:
+
+```markdown
+## PR Description Checklist Verification
+
+- [x] Does the PR description target a Senior Engineer familiar with the project?
+- [ ] Does the PR include a "Resolves #<ISSUE NUMBER>" in the body? → FAILED: missing issue reference, adding now
+```
+
+You MUST NOT create or update the PR until ALL checklist items pass.
+
 **Constraints:**
+
+- You MUST read and follow the PR description guidelines in [docs/PR.md](../../docs/PR.md) when creating pull requests & commits
 - You MUST check that all tasks are complete before proceeding
 - You MUST reference your notes for the issue you are creating a pull request for
-- You MUST NOT commit changes until builds AND tests have been verified because committing broken code can disrupt the development workflow and introduce bugs into the codebase 
+- You MUST NOT commit changes until builds AND tests have been verified because committing broken code can disrupt the development workflow and introduce bugs into the codebase
 - You MUST follow the Conventional Commits specification
 - You MUST use `git status` to check which files have been modified
 - You MUST use `git add` to stage all relevant files
 - You MUST execute the `git commit -m <COMMIT_MESSAGE>` command with the prepared commit message
 - You MAY use `git push origin <BRANCH_NAME>` to push the local branch to the remote if the `GITHUB_WRITE` environment variable is set to `true`
-  - If the push operation is deferred, continue with PR creation and note the deferred status 
+  - If the push operation is deferred, continue with PR creation and note the deferred status
 - You MUST attempt to create the pull request using the `create_pull_request` tool if it does not exist yet
   - If the PR creation is deferred, continue with the workflow and note the deferred status
   - You MUST use the task id recorded in your notes, not the issue id
-  - You MUST include "Resolves: #<ISSUE NUMBER>" in the body of the pull request
-    - You MUST NOT bold this line
-  - You MUST give an overview of the feature being implemented
-  - You MUST include any notes on key implementation decisions, ambiguity, or other information as part of the pull request description
 - If the `create_pull_request` tool fails (excluding deferred responses):
     - The tool automatically handles fallback by posting a properly URL-encoded manual PR creation link as a comment on the specified fallback issue
     - You MUST verify the fallback comment was posted successfully by checking the tool's return message
@@ -303,6 +320,7 @@ Based on the users feedback, you will review and update your implementation plan
 - You MUST NOT close the parent issue - only the user should close it after the pull request is merged
 - You MUST not attempt to merge the pull request
 - You MUST use the handoff_to_user tool to inform the user you are ready for clarifying information on the pull request
+- You MUST include additional checklist items from [docs/PR.md](../../docs/PR.md) to validate the pull request description is correct after making additional changes
 
 ## Desired Outcome
 
@@ -396,8 +414,27 @@ If builds fail during implementation:
 - Use progress tracking with markdown checklists
 - Document decisions, assumptions, and challenges
 
+### Checklist Verification Pattern
+
+When documentation files contain checklists (e.g., `docs/PR.md`), you MUST:
+
+1. Copy the entire checklist into your progress notes
+2. Explicitly verify each item by marking `[x]` or `[ ]`
+3. For any failed items, document the issue and fix it before proceeding
+4. Re-verify failed items after fixes until all pass
+
+This pattern ensures quality gates are not skipped and provides an audit trail of verification.
+
+### Pull Request Best Practices
+
+- You MUST follow the PR description guidelines in [docs/PR.md](../../docs/PR.md)
+- Focus on WHY the change is needed, not HOW it's implemented
+- Document public API changes with before/after code examples
+- Write for senior engineers familiar with the project
+- Skip implementation details, test coverage notes, and line-by-line change lists
+
 ### Git Best Practices
 - Commit early and often with descriptive messages
 - Follow Conventional Commits specification
 - You must create a new commit for each feedback iteration
-- You must only push to your feature branch, never main
+- You must only push to your feature branch, never main
diff --git a/.github/agent-sops/task-refiner.sop.md b/.github/agent-sops/task-refiner.sop.md
@@ -185,7 +185,6 @@ Record that the task review is complete and ready as a comment on the issue.
   - If comment posting is deferred, continue with the workflow and note the deferred status
 - You MUST summarize what was accomplished in your comment
 - You MUST confirm in your comment that the issue is ready for implementation, or explain why it is not
-- You MUST record the estimated scope of work based on repository analysis
 - You SHOULD mention any final recommendations or considerations
 
 ## Examples
@@ -257,12 +256,6 @@ Based on clarification discussion and repository analysis:
 - [ ] 2FA can be enabled/disabled by user
 - [ ] Integration tests pass
 - [ ] Existing auth functionality remains intact
-
-### Estimated Scope
-- **Complexity**: Medium
-- **Files Modified**: ~8-10 files
-- **New Components**: 2-3 React components
-- **Database Migrations**: 1-2 migrations required
 ```
 
 ## Troubleshooting
diff --git a/docs/PR.md b/docs/PR.md
@@ -0,0 +1,200 @@
+# 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](../.github/PULL_REQUEST_TEMPLATE.md) 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:
+
+1. **Motivation** — Why is this change needed?
+2. **Public API Changes** — What changes to the public API (with code snippets)?
+3. **Use Cases** (optional) — When would developers use this feature? Only include for non-obvious functionality; skip for trivial changes or obvious fixes.
+4. **Breaking Changes** (if applicable) — What breaks and how to migrate?
+
+## Writing Principles
+
+**Focus on WHY, not HOW:**
+
+- ✅ "The OpenAI SDK supports dynamic API keys, but we don't expose this capability"
+- ❌ "Added ApiKeySetter type import from openai/client"
+
+**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 secret manager integration for credential rotation"
+- ❌ "Updated error message to mention 'string or function'"
+
+## 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:**
+
+```markdown
+### Type Definition Updates
+
+- Added ApiKeySetter type import from 'openai/client'
+- Updated OpenAIModelOptions interface apiKey type
+```
+
+❌ **Implementation notes reviewers don't need:**
+
+```markdown
+## Implementation Notes
+
+- No breaking changes - all existing string-based usage continues to work
+- OpenAI SDK handles validation of function return values
+```
+
+❌ **Test coverage bullets:**
+
+```markdown
+### Test Coverage
+
+- Added test: accepts function-based API key
+- Added test: accepts async function-based API key
+```
+
+## Good Examples
+
+✅ **Motivation section:**
+
+```markdown
+## Motivation
+
+The OpenAI SDK supports dynamic API key resolution through async functions,
+enabling use cases like credential rotation and secret manager integration.
+However, our SDK currently only accepts static strings for the apiKey parameter,
+preventing users from leveraging these capabilities.
+```
+
+✅ **Public API Changes section:**
+
+````markdown
+## Public API Changes
+
+The `OpenAIModelOptions.apiKey` parameter now accepts either a string or an
+async function:
+
+```typescript
+// Before: only string supported
+const model = new OpenAIModel({
+  modelId: 'gpt-4o',
+  apiKey: 'sk-...',
+})
+
+// After: function also supported
+const model = new OpenAIModel({
+  modelId: 'gpt-4o',
+  apiKey: async () => await secretManager.getApiKey(),
+})
+```
+
+The change is backward compatible—all existing string-based usage continues
+to work without modification.
+
+````
+
+✅ **Use Cases section:**
+```markdown
+## Use Cases
+
+- **API key rotation**: Rotate keys without application restart
+- **Secret manager integration**: Fetch credentials from AWS Secrets Manager, Vault, etc.
+- **Multi-tenant systems**: Dynamically select API keys based on context
+````
+
+## Template
+
+````markdown
+## 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."]
+
+```typescript
+// 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
+
+```typescript
+// 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
+
+- [Conventional Commits](https://www.conventionalcommits.org/)
+- [Google's Code Review Guidelines](https://google.github.io/eng-practices/review/)
+
+## 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 implemention decisions
+ - [ ] Does the PR include a "Resolves #<ISSUE NUMBER>" in the body and is not bolded?
+ - [ ] Does the PR contain the motivation or use-cases behind the change?
+ - [ ] Does the PR omit irrelevent details not needed for historical reference?
