|
1 | 1 | <!-- SPDX-License-Identifier: MIT OR Apache-2.0 --> |
2 | 2 | <!-- Copyright (c) 2025 Pierre Fitness Intelligence --> |
3 | 3 |
|
4 | | -# Appendix B: CLAUDE.md Compliance Checklist |
| 4 | +# Appendix B: CLAUDE.md Compliance Reference |
5 | 5 |
|
6 | | -Quick checklist for CLAUDE.md code standards compliance. |
| 6 | +Comprehensive reference for Pierre codebase standards from `.claude/CLAUDE.md`. |
7 | 7 |
|
8 | 8 | ## Error Handling (Zero Tolerance) |
9 | 9 |
|
10 | | -- [ ] **No `anyhow::anyhow!()`**: Use `AppError` or specific error types |
11 | | -- [ ] **No `unwrap()`**: Use `?` operator or `unwrap_or` |
12 | | -- [ ] **No `panic!()`**: Return `Result` instead |
13 | | -- [ ] **Structured errors**: Use `thiserror` with named fields |
| 10 | +### Structured Error Types (REQUIRED) |
14 | 11 |
|
15 | | -## Module Organization |
| 12 | +All errors MUST use project-specific error enums: |
| 13 | + |
| 14 | +```rust |
| 15 | +// ✅ GOOD: Structured error types |
| 16 | +return Err(AppError::not_found(format!("User {user_id}"))); |
| 17 | +return Err(DatabaseError::ConnectionFailed { source: e.to_string() }.into()); |
| 18 | +return Err(ProviderError::RateLimitExceeded { |
| 19 | + provider: "Strava".to_string(), |
| 20 | + retry_after_secs: 3600, |
| 21 | + limit_type: "Daily quota".to_string(), |
| 22 | +}); |
| 23 | + |
| 24 | +// ✅ GOOD: Mapping external errors |
| 25 | +external_lib_call().map_err(|e| AppError::internal(format!("API failed: {e}")))?; |
| 26 | +``` |
| 27 | + |
| 28 | +### Prohibited Patterns (CI Failure) |
| 29 | + |
| 30 | +```rust |
| 31 | +// ❌ FORBIDDEN: anyhow::anyhow!() |
| 32 | +return Err(anyhow::anyhow!("User not found")); |
| 33 | + |
| 34 | +// ❌ FORBIDDEN: anyhow! macro shorthand |
| 35 | +return Err(anyhow!("Invalid input")); |
| 36 | + |
| 37 | +// ❌ FORBIDDEN: In map_err closures |
| 38 | +.map_err(|e| anyhow!("Failed: {e}"))?; |
| 39 | + |
| 40 | +// ❌ FORBIDDEN: In ok_or_else |
| 41 | +.ok_or_else(|| anyhow!("Not found"))?; |
| 42 | +``` |
| 43 | + |
| 44 | +### `unwrap()` and `expect()` Rules |
| 45 | + |
| 46 | +- **`unwrap()`**: Only in tests, static data, or binary `main()` |
| 47 | +- **`expect()`**: Only for documenting invariants that should never fail: |
| 48 | + ```rust |
| 49 | + // ✅ OK: Static/compile-time data |
| 50 | + "127.0.0.1".parse().expect("valid IP literal") |
| 51 | + |
| 52 | + // ❌ FORBIDDEN: Runtime errors |
| 53 | + user_input.parse().expect("should be valid") // NO! |
| 54 | + ``` |
| 55 | + |
| 56 | +## Code Style Requirements |
| 57 | + |
| 58 | +### File Headers (REQUIRED) |
| 59 | + |
| 60 | +All code files MUST start with ABOUTME comments: |
| 61 | + |
| 62 | +```rust |
| 63 | +// ABOUTME: Brief description of what this module does |
| 64 | +// ABOUTME: Additional context about the module's responsibility |
| 65 | +``` |
| 66 | + |
| 67 | +### Import Style (Enforced by Clippy) |
| 68 | + |
| 69 | +Use `use` imports at the top of the file. Avoid inline qualified paths: |
| 70 | + |
| 71 | +```rust |
| 72 | +// ✅ GOOD: Import at top of file |
| 73 | +use crate::models::User; |
| 74 | +use std::collections::HashMap; |
| 75 | + |
| 76 | +fn example() { |
| 77 | + let user = User::new(); |
| 78 | + let map = HashMap::new(); |
| 79 | +} |
| 80 | + |
| 81 | +// ❌ BAD: Inline qualified paths |
| 82 | +fn example() { |
| 83 | + let user = crate::models::User::new(); // NO! |
| 84 | + let map = std::collections::HashMap::new(); // NO! |
| 85 | +} |
| 86 | +``` |
| 87 | + |
| 88 | +### Naming Conventions |
| 89 | + |
| 90 | +- **NEVER** use `_` prefix for unused variables (fix the unused variable properly) |
| 91 | +- **NEVER** name things `improved`, `new`, `enhanced` - code naming should be evergreen |
| 92 | +- **NEVER** add placeholder, `dead_code`, or mock code in production |
| 93 | + |
| 94 | +### Comments |
16 | 95 |
|
17 | | -- [ ] **Public API in mod.rs**: Re-export public items |
18 | | -- [ ] **Private implementation**: Keep internals private |
19 | | -- [ ] **Logical grouping**: Related functionality in same module |
20 | | -- [ ] **Feature flags**: Conditional compilation for database backends |
| 96 | +- **NEVER** remove existing comments unless provably false |
| 97 | +- Comments should be evergreen - avoid temporal references ("after refactor", "recently changed") |
| 98 | +- Use `///` for public API documentation |
| 99 | +- Use `//` for inline implementation comments |
21 | 100 |
|
22 | | -## Documentation |
| 101 | +## Tiered Validation Approach |
23 | 102 |
|
24 | | -- [ ] **Doc comments**: `///` for public items |
25 | | -- [ ] **Examples**: Include usage examples in doc comments |
26 | | -- [ ] **Error cases**: Document when functions return errors |
27 | | -- [ ] **Safety**: Document `unsafe` code (if unavoidable) |
| 103 | +### Tier 1: Quick Iteration (during development) |
28 | 104 |
|
29 | | -## Testing |
| 105 | +```bash |
| 106 | +cargo fmt |
| 107 | +cargo check --quiet |
| 108 | +cargo test <test_name_pattern> -- --nocapture |
| 109 | +``` |
30 | 110 |
|
31 | | -- [ ] **Unit tests**: Test individual functions |
32 | | -- [ ] **Integration tests**: Test component interaction |
33 | | -- [ ] **Deterministic**: Use seeded RNG for reproducible tests |
34 | | -- [ ] **No external dependencies**: Use synthetic data, not OAuth |
| 111 | +### Tier 2: Pre-Commit (before committing) |
35 | 112 |
|
36 | | -## Security |
| 113 | +```bash |
| 114 | +cargo fmt |
| 115 | +./scripts/architectural-validation.sh |
| 116 | +cargo clippy --all-targets -- -D warnings -D clippy::all -D clippy::pedantic -D clippy::nursery -W clippy::cognitive_complexity |
| 117 | +cargo test <module_pattern> -- --nocapture |
| 118 | +``` |
37 | 119 |
|
38 | | -- [ ] **Input validation**: Validate all user inputs |
39 | | -- [ ] **SQL injection prevention**: Use parameterized queries |
40 | | -- [ ] **Secret management**: Never hardcode secrets |
41 | | -- [ ] **Secure memory**: `zeroize` for cryptographic keys |
| 120 | +**CRITICAL**: Always use `--all-targets` with clippy. Without it, clippy misses lint errors in `tests/`, `benches/`, and binary crates. |
42 | 121 |
|
43 | | -## Performance |
| 122 | +### Tier 3: Full Validation (before PR/merge) |
44 | 123 |
|
45 | | -- [ ] **Database pooling**: Reuse connections |
46 | | -- [ ] **Async operations**: Use `tokio` for I/O |
47 | | -- [ ] **Minimal cloning**: Only clone when necessary |
48 | | -- [ ] **Efficient algorithms**: Use appropriate data structures |
| 124 | +```bash |
| 125 | +./scripts/lint-and-test.sh |
| 126 | +``` |
49 | 127 |
|
50 | | -## Key Takeaways |
| 128 | +Full test suite takes ~13 minutes (647 tests). Only run for PRs/merges. |
| 129 | + |
| 130 | +## Memory and Performance |
| 131 | + |
| 132 | +### Clone Usage Guidelines |
| 133 | + |
| 134 | +Document why each `clone()` is necessary: |
| 135 | + |
| 136 | +```rust |
| 137 | +// ✅ OK: Arc clone (cheap, self-documenting) |
| 138 | +let db_clone = database.clone(); |
| 139 | + |
| 140 | +// ✅ OK: Documented clone |
| 141 | +let name = user.name.clone(); // Needed: ownership moves to async task |
| 142 | +tokio::spawn(async move { |
| 143 | + process(name).await; |
| 144 | +}); |
| 145 | + |
| 146 | +// ❌ BAD: Unnecessary clone |
| 147 | +let name = user.name.clone(); |
| 148 | +println!("{}", name); // Should just use &user.name |
| 149 | +``` |
| 150 | + |
| 151 | +### Arc Usage |
| 152 | + |
| 153 | +- Only use when actual shared ownership required across threads |
| 154 | +- Document the sharing requirement in comments |
| 155 | +- Prefer `&T` references when data lifetime allows |
| 156 | +- Current count: ~107 Arc usages (appropriate for multi-tenant async architecture) |
| 157 | + |
| 158 | +### Lazy Statics |
| 159 | + |
| 160 | +```rust |
| 161 | +// ✅ GOOD: LazyLock for compile-time-known initialization (Rust 1.80+) |
| 162 | +use std::sync::LazyLock; |
| 163 | +static CONFIG: LazyLock<Config> = LazyLock::new(|| Config::load()); |
| 164 | + |
| 165 | +// ✅ GOOD: OnceLock for runtime values |
| 166 | +use std::sync::OnceLock; |
| 167 | +static RUNTIME_CONFIG: OnceLock<Config> = OnceLock::new(); |
| 168 | +``` |
| 169 | + |
| 170 | +## Testing Requirements |
| 171 | + |
| 172 | +### Test Coverage Policy |
| 173 | + |
| 174 | +NO EXCEPTIONS: All code must have: |
| 175 | +- Unit tests |
| 176 | +- Integration tests |
| 177 | +- End-to-end tests |
| 178 | + |
| 179 | +Only skip with explicit authorization: "I AUTHORIZE YOU TO SKIP WRITING TESTS THIS TIME" |
| 180 | + |
| 181 | +### Test Targeting |
| 182 | + |
| 183 | +```bash |
| 184 | +# By test name (partial match) |
| 185 | +cargo test test_training_load |
| 186 | + |
| 187 | +# By test file |
| 188 | +cargo test --test intelligence_test |
| 189 | + |
| 190 | +# By module path |
| 191 | +cargo test intelligence:: |
| 192 | +``` |
| 193 | + |
| 194 | +## Security Requirements |
| 195 | + |
| 196 | +- **Input validation**: Validate all user inputs at boundaries |
| 197 | +- **SQL injection prevention**: Use parameterized queries |
| 198 | +- **Secret management**: Never hardcode secrets, use `zeroize` for crypto keys |
| 199 | +- **No `allow(clippy::...)` attributes** except for type conversion casts |
| 200 | + |
| 201 | +## Module Organization |
51 | 202 |
|
52 | | -1. **Error handling**: Zero tolerance for `anyhow::anyhow!()` and `unwrap()`. |
53 | | -2. **Module organization**: Clear public API, private internals. |
54 | | -3. **Documentation**: Comprehensive doc comments with examples. |
55 | | -4. **Testing**: Deterministic tests with synthetic data. |
56 | | -5. **Security**: Input validation, parameterized queries, secret management. |
| 203 | +- Public API defined in `mod.rs` via re-exports |
| 204 | +- Use `pub(crate)` for internal APIs |
| 205 | +- Group related functionality in modules |
| 206 | +- Feature flags for conditional compilation (database backends) |
| 207 | + |
| 208 | +## Commit Protocol |
| 209 | + |
| 210 | +1. Run tiered validation (Tier 2 minimum) |
| 211 | +2. Create atomic commits with clear messages |
| 212 | +3. **NEVER** use `--no-verify` flag |
| 213 | +4. **NEVER** amend commits already pushed to remote |
| 214 | + |
| 215 | +## Key Compliance Checks |
| 216 | + |
| 217 | +| Check | Requirement | |
| 218 | +|-------|-------------| |
| 219 | +| `anyhow!()` macro | ❌ FORBIDDEN in production code | |
| 220 | +| `unwrap()` | Tests/static data/binary main only | |
| 221 | +| `#[allow(clippy::...)]` | Only for cast validations | |
| 222 | +| ABOUTME comments | REQUIRED on all source files | |
| 223 | +| `--all-targets` | REQUIRED with clippy | |
| 224 | +| Structured errors | REQUIRED via `AppError`, etc. | |
| 225 | + |
| 226 | +## Quick Checklist |
| 227 | + |
| 228 | +- [ ] No `anyhow::anyhow!()` in production code |
| 229 | +- [ ] No unwarranted `unwrap()` or `expect()` |
| 230 | +- [ ] ABOUTME comments at top of file |
| 231 | +- [ ] Use imports, not inline qualified paths |
| 232 | +- [ ] Document `clone()` usage when not Arc |
| 233 | +- [ ] Run clippy with `--all-targets` |
| 234 | +- [ ] Tests for all new functionality |
0 commit comments