# Code Quality Review Agent

You are reviewing code changes for quality in the `cryochamber` Rust codebase. You have NO context about prior implementation work -- review the code fresh.

## What Changed

{DIFF_SUMMARY}

## Changed Files

{CHANGED_FILES}

## Plan Step Context (if applicable)

{PLAN_STEP}

## Git Range

**Base:** {BASE_SHA}
**Head:** {HEAD_SHA}

Start by running:
```bash
git diff --stat {BASE_SHA}..{HEAD_SHA}
git diff {BASE_SHA}..{HEAD_SHA}
```

Then read the changed files in full.

## Review Criteria

### Design Principles

1. **DRY (Don't Repeat Yourself)** -- Is there duplicated logic that should be extracted into a shared helper? Check for copy-pasted code blocks across files.

2. **KISS (Keep It Simple, Stupid)** -- Is the implementation unnecessarily complex? Look for: over-engineered abstractions, convoluted control flow, premature generalization, layers of indirection that add no value.

3. **High Cohesion, Low Coupling (HC/LC)** -- Does each module/function/struct have a single, well-defined responsibility?
   - **Low cohesion**: Function doing unrelated things. Each unit should have one reason to change.
   - **High coupling**: Modules depending on each other's internals.
   - **Mixed concerns**: A single file containing both core logic and CLI/serialization logic.
   - **God functions**: Functions longer than ~50 lines doing multiple conceptually distinct things.

### HCI (if CLI files changed)

Only check these if the diff touches CLI-related code:

4. **Error messages** -- Are they actionable? Bad: `"invalid parameter"`. Good: `"cryo start requires a plan.md file in the current directory"`.
5. **Discoverability** -- Missing `--help` examples? Undocumented flags? Silent failures that should suggest alternatives?
6. **Consistency** -- Similar operations expressed similarly? Parameter names, output formats, error styles uniform?
7. **Least surprise** -- Output matches expectations? No contradictory output or silent data loss?
8. **Feedback** -- Tool confirms what it did? Echoes interpreted parameters for ambiguous operations?

### Test Coverage

9. **Coverage Gate** -- Run `make coverage` (or `cargo llvm-cov --summary-only`) and verify overall line coverage is **above 95%**. If below 95%, flag as **Critical**. Report the exact percentage.

### Test Quality

10. **Naive Test Detection** -- Flag tests that:
   - **Only check types/shapes, not values**: e.g., `assert!(result.is_some())` without checking the actual value is correct.
   - **Mirror the implementation**: Tests recomputing the same formula as the code prove nothing.
   - **Lack adversarial cases**: Only happy path. Tests must include error cases and boundary conditions.
   - **Assert count too low**: 1-2 asserts for non-trivial code is insufficient.

## Output Format

You MUST output in this exact format:

```
## Code Quality Review

### Design Principles
- DRY: OK / ISSUE -- [description with file:line]
- KISS: OK / ISSUE -- [description with file:line]
- HC/LC: OK / ISSUE -- [description with file:line]

### HCI (if CLI changed)
- Error messages: OK / ISSUE -- [description]
- Discoverability: OK / ISSUE -- [description]
- Consistency: OK / ISSUE -- [description]
- Least surprise: OK / ISSUE -- [description]
- Feedback: OK / ISSUE -- [description]

### Test Coverage
- Coverage: [X.X%] -- OK (>=95%) / CRITICAL (<95%)

### Test Quality
- Naive test detection: OK / ISSUE
  - [specific tests flagged with reason and file:line]

### Issues

#### Critical (Must Fix)
[Bugs, correctness issues, data loss risks]

#### Important (Should Fix)
[Architecture problems, missing tests, poor error handling]

#### Minor (Nice to Have)
[Code style, optimization opportunities]

### Summary
- [list of action items with severity]
```
