Skip to content

Commit fc4bbee

Browse files
committed
docs: update code quality checklist with critical thinking and new lessons
1 parent eeaac15 commit fc4bbee

File tree

1 file changed

+45
-5
lines changed

1 file changed

+45
-5
lines changed

CLAUDE.md

Lines changed: 45 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -129,6 +129,42 @@ The tool supports configuration via:
129129

130130
**CRITICAL**: Before writing any code, systematically check these categories to avoid rework:
131131

132+
### 0. Critical Thinking & Code Analysis (Meta-Level)
133+
**MOST IMPORTANT**: Think critically before implementing any suggestion or requirement.
134+
135+
- **Analyze Existing Code First**: Before adding validation, checks, or features, thoroughly examine what already exists
136+
- **Question Redundancy**: If validation already exists earlier in the call chain, don't add duplicate checks
137+
- **Provide Feedback Before Implementation**: When a suggestion seems unnecessary or redundant, explain WHY rather than blindly implementing it
138+
- **Consider Token Efficiency**: Rework wastes time and tokens. Get it right the first time by applying this entire checklist systematically
139+
- **Defense-in-Depth vs Redundancy**:
140+
- Defense-in-depth = validating at system boundaries (user input, external APIs, different layers)
141+
- Redundancy = checking the same condition twice in the same function after it was already validated
142+
- Apply defense-in-depth, avoid redundancy
143+
- **Evaluate Necessity**: Just because something CAN be added doesn't mean it SHOULD be. Ask: "Does this add value or just clutter?"
144+
145+
**Questions to ask before writing ANY code:**
146+
1. "What validation/checks already exist in this call chain?"
147+
2. "Is this truly adding safety, or is it redundant?"
148+
3. "What value does this code provide?"
149+
4. "Am I implementing this because it was suggested, or because it's actually needed?"
150+
151+
**Example of what NOT to do:**
152+
```go
153+
// Earlier in function (line 211)
154+
if report.EndBlock == math.MaxUint64 {
155+
return fmt.Errorf("end block must be specified")
156+
}
157+
158+
// ... code ...
159+
160+
// Later in same function (line 231) - REDUNDANT
161+
if report.EndBlock == math.MaxUint64 {
162+
return fmt.Errorf("internal error: end block cannot be math.MaxUint64")
163+
}
164+
totalBlocks := report.EndBlock - report.StartBlock + 1
165+
```
166+
This is pure redundancy - if the value can't be MaxUint64 (validated at line 211), checking again at line 231 adds zero value.
167+
132168
### 1. Security
133169
- **HTML/Template Injection**: Always use `html.EscapeString()` for any data interpolated into HTML, even if currently from trusted sources
134170
- **Input Validation**: Validate all user inputs at boundaries (flags, API inputs)
@@ -154,28 +190,32 @@ The tool supports configuration via:
154190
- **Retry Logic**: Failed operations should retry (with backoff) before failing
155191
- **Idempotency**: Same input parameters should produce identical output every time
156192
- **Validation**: Verify expected vs actual data counts; fail loudly if mismatched
157-
- **Question to ask**: "If I run this twice with the same parameters, will I get identical results? What makes this non-deterministic?"
193+
- **Use Correct Data Source**: For blockchain data, prefer receipt fields over transaction fields (e.g., `effectiveGasPrice` from receipt works for both legacy and EIP-1559 txs, while `gasPrice` from transaction is missing in EIP-1559)
194+
- **Question to ask**: "If I run this twice with the same parameters, will I get identical results? What makes this non-deterministic? Am I reading from the authoritative data source?"
158195

159-
### 5. Error Handling
196+
### 5. Error Handling & Logging
160197
- **Error Wrapping**: Use `fmt.Errorf("context: %w", err)` to wrap errors with context
161198
- **Single-line Messages**: Put context before `%w` in single line: `fmt.Errorf("failed after %d attempts: %w", n, err)`
162199
- **Failure Modes**: Consider and handle all failure paths explicitly
163200
- **Logging Levels**: Use appropriate levels (Error for failures, Warn for retries, Info for progress)
164-
- **Question to ask**: "What can fail? How is each failure mode handled? Are errors properly wrapped?"
201+
- **Progress Accuracy**: Progress counters must reflect ALL completed work (successes + final failures), not just successes, or progress will appear stuck during retries
202+
- **Question to ask**: "What can fail? How is each failure mode handled? Are errors properly wrapped? Is progress logging accurate during retries/failures?"
165203

166204
### 6. Concurrency Patterns
167205
- **Channel Closing**: Close channels in the correct goroutine (usually the sender); use atomic counters to coordinate
206+
- **Channel Draining**: When using select with multiple channels and one closes, drain remaining channels to avoid missing messages
168207
- **Worker Pools**: Use `sync.WaitGroup` to wait for workers; protect shared state with mutexes or channels
169208
- **Race Conditions**: Run with `-race` flag during testing
170209
- **Goroutine Leaks**: Ensure every goroutine can exit on context cancellation
171-
- **Question to ask**: "Who closes each channel? Can any goroutine block forever? Does this have race conditions?"
210+
- **Question to ask**: "Who closes each channel? Can any goroutine block forever? Does this have race conditions? Are all channel messages guaranteed to be read?"
172211

173212
### 7. Testing & Validation
174213
- **Test Coverage**: Write tests for edge cases, not just happy paths
175214
- **Error Injection**: Test retry logic, failure modes, and error paths
176215
- **Resource Limits**: Test with large inputs to verify scalability
177216
- **Cancellation**: Test that context cancellation stops operations immediately
178-
- **Question to ask**: "What edge cases exist? How do I test failure modes?"
217+
- **Documentation Consistency**: Ensure documentation accurately describes implementation behavior (e.g., "blocks are skipped" vs "command fails on errors")
218+
- **Question to ask**: "What edge cases exist? How do I test failure modes? Does the documentation match what the code actually does?"
179219

180220
### Common Patterns to Apply by Default
181221

0 commit comments

Comments
 (0)