|
| 1 | +# Contributing & Development Standards |
| 2 | + |
| 3 | +This document defines development standards, workflows, and tooling conventions. |
| 4 | + |
| 5 | +## Development Philosophy |
| 6 | + |
| 7 | +1. **Makefile for orchestration, raw commands for debugging** |
| 8 | +2. **CI must pass before merge** - No exceptions |
| 9 | +3. **Tests are not optional** - 70% coverage minimum |
| 10 | +4. **Security is everyone's job** - Run security-reviewer agent before major PRs |
| 11 | + |
| 12 | +--- |
| 13 | + |
| 14 | +## When to Use Makefile vs Raw Commands |
| 15 | + |
| 16 | +### Use Makefile When: |
| 17 | + |
| 18 | +| Scenario | Command | Why | |
| 19 | +|----------|---------|-----| |
| 20 | +| Starting development | `make dev` | Starts all services correctly | |
| 21 | +| Running all tests | `make test` | Consistent with CI | |
| 22 | +| Running E2E tests | `make test-e2e` | Handles server lifecycle | |
| 23 | +| Before committing | `make lint` | Catches issues early | |
| 24 | +| Deploying infrastructure | `make cdk-deploy-preprod` | Correct profile/context | |
| 25 | + |
| 26 | +### Use Raw Commands When: |
| 27 | + |
| 28 | +| Scenario | Command | Why | |
| 29 | +|----------|---------|-----| |
| 30 | +| Debugging a single test | `pytest tests/test_foo.py::test_bar -v` | More control | |
| 31 | +| Watching frontend tests | `npm run test:watch` | Interactive mode | |
| 32 | +| Running specific migration | `alembic upgrade +1` | Granular control | |
| 33 | +| Checking a specific endpoint | `curl localhost:8000/api/health` | Quick inspection | |
| 34 | +| Installing a new package | `pip install foo` | Then add to requirements.txt | |
| 35 | + |
| 36 | +### Rule of Thumb |
| 37 | + |
| 38 | +> **Use `make` for repeatable workflows. Use raw commands for exploration.** |
| 39 | +
|
| 40 | +--- |
| 41 | + |
| 42 | +## Code Standards |
| 43 | + |
| 44 | +### Python (Backend) |
| 45 | + |
| 46 | +**Style:** |
| 47 | +- Formatter: `black` (line length 88) |
| 48 | +- Import sorter: `isort` (black-compatible) |
| 49 | +- Type hints: Required for public functions |
| 50 | +- Docstrings: Required for modules, classes, public functions |
| 51 | + |
| 52 | +**Run locally:** |
| 53 | +```bash |
| 54 | +make format-backend # Auto-format |
| 55 | +make lint-backend # Check without fixing |
| 56 | +``` |
| 57 | + |
| 58 | +**Example:** |
| 59 | +```python |
| 60 | +def get_user_by_id(db: Session, user_id: UUID) -> User | None: |
| 61 | + """Fetch a user by their ID. |
| 62 | +
|
| 63 | + Args: |
| 64 | + db: Database session |
| 65 | + user_id: The user's UUID |
| 66 | +
|
| 67 | + Returns: |
| 68 | + User if found, None otherwise |
| 69 | + """ |
| 70 | + return db.query(User).filter(User.id == user_id).first() |
| 71 | +``` |
| 72 | + |
| 73 | +### TypeScript (Frontend) |
| 74 | + |
| 75 | +**Style:** |
| 76 | +- Formatter: Prettier (via ESLint) |
| 77 | +- Linter: ESLint with TypeScript rules |
| 78 | +- Strict mode: Enabled |
| 79 | + |
| 80 | +**Run locally:** |
| 81 | +```bash |
| 82 | +make lint-frontend # Check |
| 83 | +npm run lint -- --fix # Auto-fix |
| 84 | +``` |
| 85 | + |
| 86 | +**Conventions:** |
| 87 | +- React components: PascalCase files (`UserProfile.tsx`) |
| 88 | +- Utilities: camelCase files (`formatDate.ts`) |
| 89 | +- Types: Suffix with type (`UserResponse`, `CreateUserRequest`) |
| 90 | +- Hooks: Prefix with `use` (`useAuth`, `useApi`) |
| 91 | + |
| 92 | +### SQL/Migrations |
| 93 | + |
| 94 | +- Use Alembic for all schema changes |
| 95 | +- Never modify production data in migrations |
| 96 | +- Include both `upgrade()` and `downgrade()` |
| 97 | +- Test migrations: `alembic upgrade head && alembic downgrade -1 && alembic upgrade head` |
| 98 | + |
| 99 | +--- |
| 100 | + |
| 101 | +## Testing Requirements |
| 102 | + |
| 103 | +### Coverage Thresholds |
| 104 | + |
| 105 | +| Component | Minimum | Target | |
| 106 | +|-----------|---------|--------| |
| 107 | +| Backend | 70% | 85% | |
| 108 | +| Frontend | 20% | 60% | |
| 109 | + |
| 110 | +### Test Categories |
| 111 | + |
| 112 | +**Unit Tests** (fast, isolated): |
| 113 | +```bash |
| 114 | +make test-backend # Backend unit tests |
| 115 | +make test-frontend # Frontend unit tests |
| 116 | +``` |
| 117 | + |
| 118 | +**E2E Tests** (slow, integrated): |
| 119 | +```bash |
| 120 | +make test-e2e # Full browser tests with server lifecycle |
| 121 | +``` |
| 122 | + |
| 123 | +### What to Test |
| 124 | + |
| 125 | +**Always test:** |
| 126 | +- API endpoints (happy path + error cases) |
| 127 | +- Authentication/authorization logic |
| 128 | +- Business logic in services |
| 129 | +- Database queries with edge cases |
| 130 | + |
| 131 | +**Don't test:** |
| 132 | +- Third-party libraries |
| 133 | +- Simple getters/setters |
| 134 | +- Framework internals |
| 135 | + |
| 136 | +### Test Naming |
| 137 | + |
| 138 | +```python |
| 139 | +# Pattern: test_<action>_<condition>_<expected_result> |
| 140 | +def test_create_user_with_valid_data_returns_201(): |
| 141 | +def test_create_user_with_duplicate_email_returns_409(): |
| 142 | +def test_get_user_when_not_authenticated_returns_401(): |
| 143 | +``` |
| 144 | + |
| 145 | +--- |
| 146 | + |
| 147 | +## Git Workflow |
| 148 | + |
| 149 | +### Branch Naming |
| 150 | + |
| 151 | +``` |
| 152 | +feature/add-user-profile |
| 153 | +fix/auth-token-refresh |
| 154 | +chore/update-dependencies |
| 155 | +``` |
| 156 | + |
| 157 | +### Commit Messages |
| 158 | + |
| 159 | +``` |
| 160 | +<type>: <short description> |
| 161 | +
|
| 162 | +<optional body> |
| 163 | +
|
| 164 | +Co-Authored-By: Claude <noreply@anthropic.com> # If AI-assisted |
| 165 | +``` |
| 166 | + |
| 167 | +Types: `feat`, `fix`, `docs`, `chore`, `refactor`, `test` |
| 168 | + |
| 169 | +### PR Requirements |
| 170 | + |
| 171 | +Before opening a PR: |
| 172 | +1. [ ] `make test` passes locally |
| 173 | +2. [ ] `make lint` passes |
| 174 | +3. [ ] New code has tests |
| 175 | +4. [ ] CLAUDE.md updated if adding new patterns |
| 176 | + |
| 177 | +Before merging: |
| 178 | +1. [ ] CI passes |
| 179 | +2. [ ] At least one approval (or self-merge if solo) |
| 180 | +3. [ ] No unresolved comments |
| 181 | + |
| 182 | +--- |
| 183 | + |
| 184 | +## Security Standards |
| 185 | + |
| 186 | +### Before Major Changes |
| 187 | + |
| 188 | +Run the security-reviewer agent: |
| 189 | +``` |
| 190 | +Claude: "Run security review on the auth changes" |
| 191 | +``` |
| 192 | + |
| 193 | +### Secrets Management |
| 194 | + |
| 195 | +- **Never** commit secrets to git |
| 196 | +- Use `.env` for local development (gitignored) |
| 197 | +- Use AWS Secrets Manager for deployed environments |
| 198 | +- Rotate credentials if accidentally exposed |
| 199 | + |
| 200 | +### Authentication |
| 201 | + |
| 202 | +- All API endpoints require authentication except `/api/health` |
| 203 | +- Use `Depends(get_current_user)` for protected routes |
| 204 | +- Validate token type (`access` not `id`) |
| 205 | +- Check resource ownership before returning data |
| 206 | + |
| 207 | +--- |
| 208 | + |
| 209 | +## Makefile Reference |
| 210 | + |
| 211 | +```bash |
| 212 | +make help # Show all available commands |
| 213 | + |
| 214 | +# Full Stack |
| 215 | +make install # Install all dependencies |
| 216 | +make dev # Start all services (docker-compose up) |
| 217 | +make stop # Stop all services |
| 218 | +make clean # Remove containers, volumes, caches |
| 219 | +make test # Run all tests |
| 220 | +make lint # Run all linters |
| 221 | + |
| 222 | +# Backend |
| 223 | +make install-backend # pip install -r requirements.txt |
| 224 | +make test-backend # pytest tests/ -v |
| 225 | +make lint-backend # black --check && isort --check && mypy |
| 226 | +make format-backend # black && isort (auto-fix) |
| 227 | +make migrate # alembic upgrade head |
| 228 | +make migration # Create new migration (prompts for message) |
| 229 | + |
| 230 | +# Frontend |
| 231 | +make install-frontend # npm install |
| 232 | +make test-frontend # npm run test |
| 233 | +make lint-frontend # npm run lint |
| 234 | + |
| 235 | +# E2E |
| 236 | +make test-e2e # Full E2E with server lifecycle |
| 237 | +make test-browser-install # Install Playwright browsers |
| 238 | + |
| 239 | +# Infrastructure |
| 240 | +make cdk-diff-preprod # Preview CDK changes |
| 241 | +make cdk-deploy-preprod # Deploy to preprod |
| 242 | +make cdk-diff-prod # Preview prod changes |
| 243 | +make cdk-deploy-prod # Deploy to production |
| 244 | +``` |
| 245 | + |
| 246 | +--- |
| 247 | + |
| 248 | +## CI/CD Pipeline |
| 249 | + |
| 250 | +### What Runs When |
| 251 | + |
| 252 | +| Event | Workflow | Jobs | |
| 253 | +|-------|----------|------| |
| 254 | +| PR to main | `ci.yml` | backend tests, frontend tests, playwright | |
| 255 | +| Push to main | `deploy.yml` | tests (via ci.yml) → deploy preprod | |
| 256 | +| Manual trigger | `deploy.yml` | tests → deploy preprod OR prod | |
| 257 | + |
| 258 | +### If CI Fails |
| 259 | + |
| 260 | +1. Check the failed job in GitHub Actions |
| 261 | +2. Run the same command locally: |
| 262 | + - Backend: `make test-backend-ci` |
| 263 | + - Frontend: `make test-frontend-ci` |
| 264 | + - E2E: `make test-e2e` |
| 265 | +3. Fix and push |
| 266 | + |
| 267 | +### Deployment Verification |
| 268 | + |
| 269 | +After deploy, the pipeline verifies the correct version is running by checking `/api/health` for the expected `git_sha`. |
| 270 | + |
| 271 | +--- |
| 272 | + |
| 273 | +## Adding New Features |
| 274 | + |
| 275 | +### Checklist |
| 276 | + |
| 277 | +1. **Plan**: Update CLAUDE.md if adding new patterns |
| 278 | +2. **Implement**: Follow code standards above |
| 279 | +3. **Test**: Add tests, ensure coverage |
| 280 | +4. **Document**: Update README if user-facing |
| 281 | +5. **Review**: Run `make lint`, `make test` |
| 282 | +6. **Security**: Run security-reviewer for auth/data changes |
| 283 | +7. **Deploy**: PR → merge → auto-deploy to preprod |
| 284 | + |
| 285 | +### Adding a New API Endpoint |
| 286 | + |
| 287 | +```bash |
| 288 | +# 1. Create the route |
| 289 | +# backend/app/api/widgets.py |
| 290 | + |
| 291 | +# 2. Create schemas |
| 292 | +# backend/app/schemas/widget.py |
| 293 | + |
| 294 | +# 3. Add to main.py |
| 295 | +# app.include_router(widgets.router, ...) |
| 296 | + |
| 297 | +# 4. Write tests |
| 298 | +# backend/tests/test_api/test_widgets.py |
| 299 | + |
| 300 | +# 5. Run tests |
| 301 | +make test-backend |
| 302 | +``` |
| 303 | + |
| 304 | +### Adding a New Frontend Page |
| 305 | + |
| 306 | +```bash |
| 307 | +# 1. Create the page |
| 308 | +# frontend/src/pages/WidgetsPage.tsx |
| 309 | + |
| 310 | +# 2. Add route in App.tsx |
| 311 | + |
| 312 | +# 3. Write tests (optional for pages, required for components) |
| 313 | + |
| 314 | +# 4. Run tests |
| 315 | +make test-frontend |
| 316 | +``` |
0 commit comments