# Quality Assurance Checklist for Agent-Generated Code *Use this checklist before merging any agent-generated pull request.* This checklist is designed for human reviewers evaluating code produced by AI coding agents. Not every item applies to every PR -- use judgment about which sections are relevant based on the scope of the change. --- ## Pre-Review: Automated Checks These should be automated in CI. Confirm they passed before starting your manual review. ### CI pipeline - [ ] All tests pass (unit, integration, e2e as applicable) - [ ] Linter passes with zero errors - [ ] Type checker passes with zero errors (if applicable) - [ ] Build completes successfully - [ ] No new security vulnerabilities detected (dependency scan) - [ ] No secrets detected in the diff (secret scanner) - [ ] Code coverage did not decrease (or decrease is justified) ### PR hygiene - [ ] PR is a reasonable size (under 400 lines of diff preferred) - [ ] PR description explains what changed and why - [ ] PR is linked to an issue or task (if your workflow requires it) - [ ] Agent co-author is identified (for tracking agent-generated code) **If any automated check failed:** Stop here. Have the agent fix the failures before reviewing manually. --- ## Section 1: Correctness ### Does it work? - [ ] The code does what the task/issue/ticket asked for - [ ] Edge cases are handled: - [ ] Empty inputs (null, undefined, empty string, empty array) - [ ] Boundary values (zero, negative numbers, maximum values) - [ ] Invalid inputs (wrong type, malformed data) - [ ] Concurrent access (if applicable) - [ ] Error paths are handled (not just the happy path) - [ ] The code doesn't break existing functionality (no regressions) ### Are the tests correct? - [ ] Tests actually test the new functionality (not just boilerplate that passes trivially) - [ ] Tests would fail if the implementation were wrong (test the test: mentally break the code and check if the test would catch it) - [ ] Test descriptions accurately describe what is being tested - [ ] Tests are independent (don't depend on execution order or shared mutable state) - [ ] Tests cover the edge cases listed above --- ## Section 2: Architectural Compliance ### Structure - [ ] New code is in the correct directory/module per project architecture - [ ] Module boundaries are respected (no unauthorized cross-module imports) - [ ] Dependency direction is correct (e.g., services don't import from controllers) - [ ] No circular dependencies introduced ### Patterns - [ ] Code follows existing patterns in the codebase (not inventing new patterns for solved problems) - [ ] Design patterns are used correctly (no pattern abuse or unnecessary abstraction) - [ ] Data access follows the project's established pattern (repository, ORM, etc.) - [ ] Error handling follows the project's established pattern ### Consistency - [ ] Naming follows project conventions (files, functions, variables, types) - [ ] Code style matches surrounding code (not introducing a different style) - [ ] Import organization matches project conventions - [ ] File structure matches project conventions **Common agent architectural mistakes to watch for:** - Creating new utility files instead of using existing shared utilities - Bypassing the service layer and accessing the database directly from handlers - Introducing new patterns when existing patterns would work - Adding unnecessary abstraction layers --- ## Section 3: Code Quality ### Readability - [ ] Code is understandable without needing the prompt that generated it - [ ] Function and variable names clearly convey purpose - [ ] Complex logic has explanatory comments (the "why," not the "what") - [ ] Functions are focused (doing one thing, not multiple unrelated things) - [ ] Functions are a reasonable length (under 50 lines as a guideline) ### Maintainability - [ ] No code duplication (check if similar logic exists elsewhere in the codebase) - [ ] No dead code (unused functions, unreachable branches, commented-out code) - [ ] No hardcoded values that should be constants or configuration - [ ] Magic numbers are explained or extracted to named constants - [ ] Types are specific (no `any`, no overly broad types) ### Simplicity - [ ] The solution is not over-engineered for the problem - [ ] No unnecessary abstractions (YAGNI: You Aren't Gonna Need It) - [ ] No unnecessary dependencies added - [ ] The simplest approach that solves the problem was chosen **Common agent quality issues to watch for:** - Over-engineering: creating abstract base classes for a single implementation - Verbose code: 20 lines where 5 would do - Copy-paste patterns: duplicating code instead of extracting shared logic - Premature optimization: optimizing code that doesn't need it - Feature creep: implementing more than what was asked for --- ## Section 4: Security *Apply this section to any code that handles user input, authentication, data access, or external communication.* ### Input handling - [ ] User input is validated before use - [ ] SQL queries use parameterized queries (no string concatenation) - [ ] HTML output is properly escaped (no XSS vectors) - [ ] File paths are validated (no path traversal) - [ ] Regular expressions are safe from ReDoS ### Authentication and authorization - [ ] Authentication checks are present where required - [ ] Authorization checks verify the user has permission for the specific action - [ ] No authentication bypass introduced (e.g., missing middleware) - [ ] Sensitive data is not logged or exposed in error messages ### Data handling - [ ] Sensitive data (PII, credentials, tokens) is handled according to policy - [ ] Data is encrypted in transit and at rest where required - [ ] No sensitive data in URLs or query parameters - [ ] Database queries return only necessary fields (no `SELECT *` leaking sensitive columns) ### Dependencies - [ ] New dependencies are from trusted sources - [ ] New dependencies are actively maintained (checked for last commit date, open issues) - [ ] New dependencies don't have known vulnerabilities - [ ] The dependency's license is compatible with your project --- ## Section 5: Performance *Apply this section to code in hot paths, data processing, or user-facing features where performance matters.* ### Efficiency - [ ] No N+1 query patterns (database queries inside loops) - [ ] Database queries use appropriate indexes - [ ] Large data sets are paginated, not loaded entirely into memory - [ ] Expensive operations are cached where appropriate - [ ] No unnecessary network calls or I/O operations ### Resource management - [ ] Database connections are properly closed/returned to pool - [ ] File handles are properly closed - [ ] Memory is freed for large allocations (in languages without GC) - [ ] Timeouts are configured for external calls - [ ] Rate limiting is applied where appropriate ### Scalability - [ ] The solution works with the expected data volume (not just test data) - [ ] No global mutable state that would break under concurrency - [ ] Background/async operations are used for long-running tasks (not blocking request handlers) --- ## Section 6: Documentation Updates - [ ] README is updated if the change affects setup or usage - [ ] API documentation is updated if endpoints changed (OpenAPI, GraphQL schema, etc.) - [ ] AGENTS.md / .github/copilot-instructions.md is updated if new conventions or patterns were established - [ ] Inline documentation (docstrings, JSDoc) is added for new public functions - [ ] CHANGELOG is updated (if your project maintains one) - [ ] Migration guide is provided if the change is breaking --- ## Section 7: Final Checks - [ ] I have read the entire diff, not just skimmed it - [ ] I understand why each change was made (if not, ask the author or check the issue) - [ ] I would be comfortable maintaining this code - [ ] I would be comfortable being on-call when this code runs in production - [ ] If I found issues, I have documented them clearly for the agent or author to fix --- ## Review Decision After completing the applicable sections: | Decision | When to use | |----------|-------------| | **Approve** | All applicable checks pass. Minor style issues can be noted but don't block. | | **Request changes** | One or more substantive issues found (correctness, security, architecture). | | **Needs discussion** | The approach is questionable but you're not sure of the right alternative. Discuss with the team. | ### If requesting changes from an agent Write your feedback in a way the agent can act on: **Weak feedback (hard for agents):** - "This doesn't look right" - "Can you improve this?" - "The error handling needs work" **Strong feedback (actionable for agents):** - "The `createUser` function on line 45 doesn't validate the email format. Add validation using the `isValidEmail` helper from `lib/validators.ts`." - "This introduces an N+1 query: `getOrders` is called inside the user loop on line 78. Refactor to batch-fetch orders for all users in a single query." - "The `authMiddleware` is missing from the `/admin/users` route on line 23. Add it following the pattern used in `routes/admin/settings.ts`." --- ## Shortcuts for Common PR Types ### Bug fix (focused) Use: Sections 1, 2, 4 (if security-relevant), 7 ### New feature Use: All sections ### Refactoring (no behavior change) Use: Sections 1 (specifically: no regressions), 2, 3, 7 ### Test-only PR Use: Section 1 (test correctness), Section 3 (test quality), 7 ### Documentation-only PR Use: Section 6, 7 ### Dependency update Use: Section 4 (dependencies subsection), Section 5 (performance if major update), 7