pre-merge-checklist

What this skill does

# Pre-Merge Checklist

## When to Use

Activate this skill when:
- Reviewing a pull request before approving
- Preparing your own PR for merge
- Verifying that all automated checks pass before merging
- Auditing a PR that has been approved but not yet merged
- Running a final validation pass after addressing review feedback

**Output:** Write results to `pre-merge-report.md` with pass/fail status for each check and blocking issues.

Do NOT use this skill for:
- In-depth security review (use `code-review-security`)
- Writing implementation code (use `python-backend-expert` or `react-frontend-expert`)
- Architecture decisions (use `system-architecture`)
- E2E test creation (use `e2e-testing`)

## Instructions

### Automated Checks (Ordered)

Run automated checks in this order. Each check must pass before proceeding to the next. Use `scripts/run-all-checks.sh` to execute all checks at once.

#### 1. Linting and Formatting

**Python (ruff):**
```bash
# Check linting
ruff check app/ tests/

# Check formatting
ruff format --check app/ tests/

# Auto-fix (if needed before commit)
ruff check --fix app/ tests/
ruff format app/ tests/
```

**TypeScript/React (eslint + prettier):**
```bash
# Check linting
npx eslint 'src/**/*.{ts,tsx}' --max-warnings 0

# Check formatting
npx prettier --check 'src/**/*.{ts,tsx}'

# Auto-fix (if needed)
npx eslint 'src/**/*.{ts,tsx}' --fix
npx prettier --write 'src/**/*.{ts,tsx}'
```

**Pass criteria:**
- Zero lint errors (warnings are tolerated only with justification)
- All files formatted consistently
- No `# noqa` or `eslint-disable` without a comment explaining why

#### 2. Type Checking

**Python (mypy):**
```bash
mypy app/ --strict --no-error-summary
```

**TypeScript:**
```bash
npx tsc --noEmit
```

Use `scripts/type-check.sh` to run both in sequence with report output.

**Pass criteria:**
- Zero type errors in changed files
- No new `type: ignore` or `@ts-ignore` without justification
- Generic types used correctly (no `Any` leaks)

#### 3. Tests

**Python:**
```bash
pytest tests/ -q --tb=short
```

**React:**
```bash
npm test -- --run --reporter=verbose
```

**Pass criteria:**
- All tests pass (zero failures)
- No skipped tests without a linked issue/ticket
- New code has corresponding tests

#### 4. Coverage

**Python:**
```bash
pytest --cov=app --cov-report=term-missing --cov-fail-under=80
```

**React:**
```bash
npx vitest run --coverage --coverage.thresholds.lines=80
```

**Pass criteria:**
- Overall coverage >= 80%
- New code coverage >= 90%
- No critical paths left uncovered (auth, payment, data mutation)

#### 5. Security Scan

```bash
# Python dependencies
pip-audit --requirement requirements.txt

# npm dependencies
npm audit --audit-level=high

# Custom code scan (if code-review-security skill is available)
python scripts/security-scan.py --path app/ --output-dir ./security-results
```

**Pass criteria:**
- No critical or high severity vulnerability in dependencies
- No critical findings in code scan
- All new endpoints have authentication checks

---

### Manual Review Checklist

After automated checks pass, review the PR manually against these categories.

#### Code Quality

- [ ] **Naming:** Variables, functions, and classes have clear, descriptive names
- [ ] **Functions:** Each function does one thing; no function exceeds 50 lines
- [ ] **DRY:** No duplicated logic that should be extracted into a shared function
- [ ] **Comments:** Complex logic is documented; no commented-out code left in
- [ ] **Imports:** No unused imports; imports are organized (stdlib, third-party, local)
- [ ] **Constants:** No magic numbers or strings; use named constants or enums
- [ ] **Logging:** New features have appropriate log statements at correct levels

#### Testing

- [ ] **Coverage:** New code has tests (unit and/or integration as appropriate)
- [ ] **Edge cases:** Tests cover happy path, error paths, and boundary conditions
- [ ] **Test names:** Test names describe the scenario and expected outcome
- [ ] **Test isolation:** Tests do not depend on each other or on execution order
- [ ] **No flakiness:** Tests do not use hardcoded delays or environment-specific paths
- [ ] **Factories:** Test data uses factories, not hardcoded fixtures

#### Type Safety

- [ ] **No `Any`:** Return types and parameters are properly typed (no escape hatches)
- [ ] **Null safety:** Optional values are handled (null checks, default values)
- [ ] **Schema validation:** API inputs use Pydantic schemas (Python) or Zod (React)
- [ ] **Generic types:** Collections use proper generics (`list[User]`, not `list`)

#### Error Handling

- [ ] **Graceful errors:** All error paths return meaningful messages
- [ ] **HTTP status codes:** Correct codes used (404 for not found, 409 for conflict, etc.)
- [ ] **Error boundaries:** React components have error boundaries for async failures
- [ ] **Retry logic:** External service calls have retry with backoff (where appropriate)
- [ ] **No silent failures:** Caught exceptions are logged, not silently swallowed

#### Backwards Compatibility

- [ ] **API contracts:** No breaking changes to existing API response shapes
- [ ] **Database migrations:** Migrations are reversible and non-destructive
- [ ] **Feature flags:** Breaking changes are behind feature flags
- [ ] **Deprecation:** Removed features have deprecation warnings in prior release
- [ ] **Configuration:** No new required environment variables without documentation

#### Documentation

- [ ] **API docs:** New endpoints are documented (OpenAPI/Swagger via FastAPI)
- [ ] **README:** Setup instructions updated if new dependencies or steps added
- [ ] **Migration notes:** Database migration has a description comment
- [ ] **ADR:** Significant architectural decisions documented (if applicable)

#### Performance

- [ ] **N+1 queries:** No N+1 database query patterns (use eager loading)
- [ ] **Pagination:** List endpoints use cursor-based pagination
- [ ] **Indexes:** New query patterns have supporting database indexes
- [ ] **Bundle size:** No unnecessary large dependencies added to frontend

#### Accessibility

- [ ] **Semantic HTML:** Correct HTML elements used (button, nav, main, etc.)
- [ ] **ARIA labels:** Interactive elements have accessible labels
- [ ] **Keyboard navigation:** New UI elements are keyboard-accessible
- [ ] **Color contrast:** Text meets WCAG 2.1 AA contrast requirements

Use `scripts/accessibility-check.sh` to run automated accessibility checks.

---

### Failure Protocol

When a check fails, follow this escalation path:

**Automated check failure:**
1. Fix the issue in the PR
2. Push the fix and re-run checks
3. Do not merge until all automated checks pass

**Manual review finding:**
1. Add a review comment with the finding
2. Request changes on the PR
3. Re-review after the author addresses feedback

**Severity-based response:**

| Finding Type | Action | Can Override? |
|---|---|---|
| Lint/format error | Fix before merge | No |
| Type error | Fix before merge | No |
| Test failure | Fix before merge | No |
| Coverage below threshold | Add tests or justify | Yes, with tech lead approval |
| Security finding (critical/high) | Fix before merge | No |
| Security finding (medium/low) | Fix or create follow-up ticket | Yes, with ticket reference |
| Accessibility violation | Fix or create follow-up ticket | Yes, with justification |
| Performance concern | Discuss in PR, may defer | Yes, with tech lead approval |

### Override Process

If a check must be overridden:

1. **Document the reason** in a PR comment explaining why the override is acceptable
2. **Get explicit approval** from a tech lead or senior engineer
3. **Create a follow-up ticket** to resolve the underlying issue
4. **Add a code comment** at the override point referencing the ticket

```python
# OVERRIDE: Coverage below 80% for this module. See TICKET-1234.
# Approved by @tech-lead on 2024-01-15.
# Reason: Legacy code migration in progress; full coverage planned for Sprin