Contextual Review
Perform comprehensive reviews of code changes, implementation plans, and architecture decisions. Analyzes for quality, correctness, security, and adherence to project standards.
First: Read the Base Guidelines
Before reviewing any code, read base-review.md. It establishes:
- Reviewer philosophy (respect existing patterns, burden of proof on changes)
- Core quality standards (type safety, transaction integrity, data access, caching)
- Severity calibration with codebase-specific examples
- Common blind spots that reviewers unfamiliar with this codebase miss
The base guidelines apply to ALL reviews. Area-specific guides add targeted checklists.
When to Use
- Reviewing pull request changes
- Examining workspace diffs before creating a PR
- Getting feedback on code changes
- Identifying potential issues before merging
- Reviewing gameplans and implementation plans before execution
- Validating data model and API design decisions
Area-Specific Guidelines
Based on what files changed, consult the appropriate reference:
| Changed Files |
Reference |
platform/docs/ |
docs-review.md - Documentation review guidelines |
platform/flowglad-next/src/db/schema/, openapi.json, api-contract/ |
api-review.md - Data model and API review |
packages/ |
packages-review.md - SDK package review |
playground/ |
playground-review.md - Example project review |
platform/flowglad-next/ |
platform-review.md - Main platform review |
For reviewing implementation plans before code is written:
| Review Type |
Reference |
| Gameplans / Implementation Plans |
gameplan-review.md - Pre-implementation plan review |
Read the relevant reference file(s) based on the diff to get area-specific checklists and guidelines.
Review Process
0. Checkout PR
Run gh pr checkout <PR> to get the PR code locally. If it fails, continue with the review.
1. Gather Context
First, understand the scope of changes:
# Get the diff statistics to understand what files changed
GetWorkspaceDiff with stat: true
# Then examine individual file changes
GetWorkspaceDiff with file: 'path/to/file'
2. Review Categories
Analyze changes across these dimensions:
| Category |
Focus Areas |
| Correctness |
Logic errors, edge cases, null handling, off-by-one errors |
| Security |
Input validation, injection risks, auth/authz, secrets exposure |
| Performance |
N+1 queries, unnecessary loops, missing indexes, memory leaks |
| Maintainability |
Code clarity, naming, DRY violations, complexity |
| Testing |
Test coverage, edge cases tested, test quality |
| Types |
Type safety, proper typing, avoiding any |
3. Project-Specific Checks
For this codebase, also verify:
- Bun: Using
bun instead of npm or yarn
- Drizzle ORM: Schema changes use
migrations:generate, never manual migrations
- Testing Guidelines:
- No mocking unless for network calls
- No
.spyOn or dynamic imports
- No
any types in tests
- Each
it block should have specific assertions, not toBeDefined
- One scenario per
it with exhaustive assertions
- Security: Check OWASP top 10 vulnerabilities (XSS, injection, etc.)
4. Provide Feedback
Use the DiffComment tool to leave targeted feedback:
DiffComment({
comments: [
{
file: "path/to/file.ts",
lineNumber: 42,
body: "Potential SQL injection vulnerability. Consider using parameterized queries."
}
]
})
Review Checklist
Code Quality
Security
Performance
Testing
TypeScript
Output Format
Provide a structured review with:
- Summary: Brief overview of what the PR does
- Findings: Categorized issues (Critical, High, Medium, Low, Suggestions)
- Positive Notes: Good patterns or improvements noticed
- Recommendation: Approve, Request Changes, or Comment
Severity Levels
- Critical: Security vulnerabilities, data loss risks, breaking changes
- High: Bugs, significant performance issues, missing error handling
- Medium: Code quality issues, missing tests, unclear logic
- Low: Style issues, minor improvements, nitpicks
- Suggestion: Optional improvements, alternative approaches
Example Review
## Summary
This PR adds user authentication using JWT tokens with refresh token support.
## Findings
### Critical
- **src/auth/token.ts:45**: JWT secret is hardcoded. Move to environment variable.
### High
- **src/auth/login.ts:23**: Missing rate limiting on login endpoint.
### Medium
- **src/auth/validate.ts:12**: Token expiration check should use `<=` not `<` to handle exact expiration time.
### Suggestions
- Consider adding request ID to auth logs for debugging.
## Positive Notes
- Good separation of concerns between token generation and validation
- Comprehensive error types for different auth failures
## Recommendation
**Request Changes** - Address the critical security issue before merging.
Workflow
- Attempt to checkout the PR with
gh pr checkout <PR> (continue if it fails)
- Get diff statistics with
GetWorkspaceDiff(stat: true)
- Review changed files systematically
- Use
DiffComment for inline feedback
- Provide overall summary and recommendation
- Offer to help fix any critical issues
1---2name: contextual-review3description: Review pull requests for code quality, security vulnerabilities, best practices, and potential issues. Use when reviewing PRs, examining diffs, or providing code review feedback.4---5
6# Contextual Review
7
8Perform comprehensive reviews of code changes, implementation plans, and architecture decisions. Analyzes for quality, correctness, security, and adherence to project standards.
9
10## First: Read the Base Guidelines
11
12**Before reviewing any code, read [base-review.md](base-review.md).** It establishes:
13- Reviewer philosophy (respect existing patterns, burden of proof on changes)
14- Core quality standards (type safety, transaction integrity, data access, caching)
15- Severity calibration with codebase-specific examples
16- Common blind spots that reviewers unfamiliar with this codebase miss
17
18The base guidelines apply to ALL reviews. Area-specific guides add targeted checklists.
19
20## When to Use
21
22- Reviewing pull request changes
23- Examining workspace diffs before creating a PR
24- Getting feedback on code changes
25- Identifying potential issues before merging
26- Reviewing gameplans and implementation plans before execution
27- Validating data model and API design decisions
28
29## Area-Specific Guidelines
30
31Based on what files changed, consult the appropriate reference:
32
33| Changed Files | Reference |
34|---------------|-----------|
35| `platform/docs/` | [docs-review.md](docs-review.md) - Documentation review guidelines |
36| `platform/flowglad-next/src/db/schema/`, `openapi.json`, `api-contract/` | [api-review.md](api-review.md) - Data model and API review |
37| `packages/` | [packages-review.md](packages-review.md) - SDK package review |
38| `playground/` | [playground-review.md](playground-review.md) - Example project review |
39| `platform/flowglad-next/` | [platform-review.md](platform-review.md) - Main platform review |
40
41For reviewing implementation plans before code is written:
42
43| Review Type | Reference |
44|-------------|-----------|
45| Gameplans / Implementation Plans | [gameplan-review.md](gameplan-review.md) - Pre-implementation plan review |
46
47Read the relevant reference file(s) based on the diff to get area-specific checklists and guidelines.
48
49## Review Process
50
51### 0. Checkout PR
52
53Run `gh pr checkout <PR>` to get the PR code locally. If it fails, continue with the review.
54
55### 1. Gather Context
56
57First, understand the scope of changes:
58
59```
60# Get the diff statistics to understand what files changed
61GetWorkspaceDiff with stat: true
62
63# Then examine individual file changes
64GetWorkspaceDiff with file: 'path/to/file'
65```
66
67### 2. Review Categories
68
69Analyze changes across these dimensions:
70
71| Category | Focus Areas |
72|----------|-------------|
73| Correctness | Logic errors, edge cases, null handling, off-by-one errors |
74| Security | Input validation, injection risks, auth/authz, secrets exposure |
75| Performance | N+1 queries, unnecessary loops, missing indexes, memory leaks |
76| Maintainability | Code clarity, naming, DRY violations, complexity |
77| Testing | Test coverage, edge cases tested, test quality |
78| Types | Type safety, proper typing, avoiding `any` |
79
80### 3. Project-Specific Checks
81
82For this codebase, also verify:
83
84- **Bun**: Using `bun` instead of `npm` or `yarn`
85- **Drizzle ORM**: Schema changes use `migrations:generate`, never manual migrations
86- **Testing Guidelines**:
87 - No mocking unless for network calls
88 - No `.spyOn` or dynamic imports
89 - No `any` types in tests
90 - Each `it` block should have specific assertions, not `toBeDefined`
91 - One scenario per `it` with exhaustive assertions
92- **Security**: Check OWASP top 10 vulnerabilities (XSS, injection, etc.)
93
94### 4. Provide Feedback
95
96Use the DiffComment tool to leave targeted feedback:
97
98```typescript
99DiffComment({
100 comments: [
101 {
102 file: "path/to/file.ts",
103 lineNumber: 42,
104 body: "Potential SQL injection vulnerability. Consider using parameterized queries."
105 }
106 ]
107})
108```
109
110## Review Checklist
111
112### Code Quality
113- [ ] Clear, descriptive variable and function names
114- [ ] Functions are focused and not too long
115- [ ] No dead code or commented-out code
116- [ ] Error handling is appropriate
117- [ ] Edge cases are handled
118
119### Security
120- [ ] No hardcoded secrets or credentials
121- [ ] Input is validated and sanitized
122- [ ] No SQL injection vectors
123- [ ] No XSS vulnerabilities
124- [ ] Authentication/authorization is correct
125- [ ] Sensitive data is not logged
126
127### Performance
128- [ ] No unnecessary database queries
129- [ ] Appropriate use of indexes
130- [ ] No obvious memory leaks
131- [ ] Pagination for large datasets
132- [ ] Caching where appropriate
133
134### Testing
135- [ ] New code has tests
136- [ ] Tests cover happy path and error cases
137- [ ] Tests are meaningful, not just for coverage
138- [ ] No flaky test patterns
139
140### TypeScript
141- [ ] Proper types used (no `any` without justification)
142- [ ] Type narrowing is correct
143- [ ] Generic types are appropriate
144- [ ] Null/undefined handled properly
145
146## Output Format
147
148Provide a structured review with:
149
1501. **Summary**: Brief overview of what the PR does
1512. **Findings**: Categorized issues (Critical, High, Medium, Low, Suggestions)
1523. **Positive Notes**: Good patterns or improvements noticed
1534. **Recommendation**: Approve, Request Changes, or Comment
154
155### Severity Levels
156
157- **Critical**: Security vulnerabilities, data loss risks, breaking changes
158- **High**: Bugs, significant performance issues, missing error handling
159- **Medium**: Code quality issues, missing tests, unclear logic
160- **Low**: Style issues, minor improvements, nitpicks
161- **Suggestion**: Optional improvements, alternative approaches
162
163## Example Review
164
165```markdown
166## Summary
167This PR adds user authentication using JWT tokens with refresh token support.
168
169## Findings
170
171### Critical
172- **src/auth/token.ts:45**: JWT secret is hardcoded. Move to environment variable.
173
174### High
175- **src/auth/login.ts:23**: Missing rate limiting on login endpoint.
176
177### Medium
178- **src/auth/validate.ts:12**: Token expiration check should use `<=` not `<` to handle exact expiration time.
179
180### Suggestions
181- Consider adding request ID to auth logs for debugging.
182
183## Positive Notes
184- Good separation of concerns between token generation and validation
185- Comprehensive error types for different auth failures
186
187## Recommendation
188**Request Changes** - Address the critical security issue before merging.
189```
190
191## Workflow
192
1931. Attempt to checkout the PR with `gh pr checkout <PR>` (continue if it fails)
1942. Get diff statistics with `GetWorkspaceDiff(stat: true)`
1953. Review changed files systematically
1964. Use `DiffComment` for inline feedback
1975. Provide overall summary and recommendation
1986. Offer to help fix any critical issues