Skip to content

Commit 7070fa6

Browse files
GiggleLiuclaude
GiggleLiu
and
claude
committed
feat: add review-implementation skill for automated code review
Adds a new skill that checks completeness and correctness of model/rule implementations. Auto-detects what changed from git diff and runs structural checks (file existence, registrations, test presence) plus semantic review (evaluate correctness, overhead accuracy, example quality). Called automatically at the end of add-model/add-rule, or standalone. Closes #87 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent b0db295 commit 7070fa6

File tree

4 files changed

+140
-0
lines changed

4 files changed

+140
-0
lines changed

‎.claude/CLAUDE.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ Rust library for NP-hard problem reductions. Implements computational problems w
77
- [issue-to-pr](skills/issue-to-pr.md) -- Convert a GitHub issue into a PR with an implementation plan. Validates the issue against the appropriate checklist, then dispatches to `add-model` or `add-rule`.
88
- [add-model](skills/add-model.md) -- Add a new problem model. Can be used standalone (brainstorms with user) or called from `issue-to-pr`.
99
- [add-rule](skills/add-rule.md) -- Add a new reduction rule. Can be used standalone (brainstorms with user) or called from `issue-to-pr`.
10+
- [review-implementation](skills/review-implementation.md) -- Review a model or rule implementation for completeness. Auto-detects type from changed files. Called automatically at the end of `add-model`/`add-rule`, or standalone via `/review-implementation`.
1011

1112
## Commands
1213
```bash

‎.claude/skills/add-model.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,8 @@ Update `docs/paper/reductions.typ`:
114114
make test clippy # Must pass
115115
```
116116

117+
Then run the [review-implementation](review-implementation.md) skill to verify all structural and semantic checks pass.
118+
117119
## Naming Conventions
118120

119121
- Struct names use explicit optimization prefixes: `MaximumX`, `MinimumX`

‎.claude/skills/add-rule.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -155,6 +155,8 @@ cargo run --example export_graph # Update reduction_graph.json
155155
make test clippy # Must pass
156156
```
157157

158+
Then run the [review-implementation](review-implementation.md) skill to verify all structural and semantic checks pass.
159+
158160
## Solver Rules
159161

160162
- If the target problem already has a solver, use it directly.

‎.claude/skills/review-implementation.md‎

Lines changed: 135 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,135 @@
1+
---
2+
name: review-implementation
3+
description: Use after implementing a model or rule to verify completeness and correctness before committing
4+
---
5+
6+
# Review Implementation
7+
8+
Automated review checklist for verifying that a new model or rule implementation is complete. Run this after finishing `add-model` or `add-rule`, before committing.
9+
10+
## Invocation
11+
12+
Auto-detects the implementation type from changed files. Can also be invoked with an explicit argument:
13+
- `/review-implementation` -- auto-detect from `git diff`
14+
- `/review-implementation model MaximumClique` -- review a specific model
15+
- `/review-implementation rule mis_qubo` -- review a specific rule
16+
17+
## Step 1: Detect What Changed
18+
19+
Use `git diff --name-only` (against main branch or last commit) to identify:
20+
- Files in `src/models/` -> model review
21+
- Files in `src/rules/` (not `mod.rs`, `traits.rs`, `cost.rs`, `graph.rs`, `registry.rs`) -> rule review
22+
- Both -> run both reviews
23+
24+
Extract the problem name(s) and rule source/target from the file paths.
25+
26+
## Step 2: Run Structural Checks
27+
28+
For each detected change, run the appropriate checklist below. Report results as a table with pass/fail per item.
29+
30+
### Model Checklist
31+
32+
Given: problem name `P`, category `C`, file stem `F` (snake_case).
33+
34+
| # | Check | Verification method |
35+
|---|-------|-------------------|
36+
| 1 | Model file exists | `Glob("src/models/{C}/{F}.rs")` |
37+
| 2 | `inventory::submit!` present | `Grep("inventory::submit", file)` |
38+
| 3 | `#[derive(...Serialize, Deserialize)]` on struct | `Grep("Serialize.*Deserialize", file)` |
39+
| 4 | `Problem` trait impl | `Grep("impl.*Problem for.*{P}", file)` |
40+
| 5 | `OptimizationProblem` or `SatisfactionProblem` impl | `Grep("(OptimizationProblem\|SatisfactionProblem).*for.*{P}", file)` |
41+
| 6 | `#[cfg(test)]` + `#[path = "..."]` test link | `Grep("#\\[path =", file)` |
42+
| 7 | Test file exists | `Glob("src/unit_tests/models/{C}/{F}.rs")` |
43+
| 8 | Test has creation test | `Grep("fn test_.*creation\|fn test_{F}.*basic", test_file)` |
44+
| 9 | Test has evaluation test | `Grep("fn test_.*evaluat", test_file)` |
45+
| 10 | Registered in `{C}/mod.rs` | `Grep("mod {F}", "src/models/{C}/mod.rs")` |
46+
| 11 | Re-exported in `models/mod.rs` | `Grep("{P}", "src/models/mod.rs")` |
47+
| 12 | CLI `load_problem` arm | `Grep('"{P}"', "problemreductions-cli/src/dispatch.rs")` |
48+
| 13 | CLI `serialize_any_problem` arm | `Grep('"{P}".*try_ser', "problemreductions-cli/src/dispatch.rs")` |
49+
| 14 | CLI `resolve_alias` entry | `Grep("{P}", "problemreductions-cli/src/problem_name.rs")` |
50+
| 15 | Paper `display-name` entry | `Grep('"{P}"', "docs/paper/reductions.typ")` |
51+
| 16 | Paper `problem-def` block | `Grep('problem-def.*"{P}"', "docs/paper/reductions.typ")` |
52+
53+
### Rule Checklist
54+
55+
Given: source `S`, target `T`, rule file stem `R` = `{s}_{t}` (lowercase), example stem `E` = `reduction_{s}_to_{t}`.
56+
57+
| # | Check | Verification method |
58+
|---|-------|-------------------|
59+
| 1 | Rule file exists | `Glob("src/rules/{R}.rs")` |
60+
| 2 | `#[reduction(...)]` macro present | `Grep("#\\[reduction", file)` |
61+
| 3 | `ReductionResult` impl present | `Grep("impl.*ReductionResult", file)` |
62+
| 4 | `ReduceTo` impl present | `Grep("impl.*ReduceTo", file)` |
63+
| 5 | `#[cfg(test)]` + `#[path = "..."]` test link | `Grep("#\\[path =", file)` |
64+
| 6 | Test file exists | `Glob("src/unit_tests/rules/{R}.rs")` |
65+
| 7 | Closed-loop test present | `Grep("fn test_.*closed_loop\|fn test_.*to_.*basic", test_file)` |
66+
| 8 | Registered in `rules/mod.rs` | `Grep("mod {R}", "src/rules/mod.rs")` |
67+
| 9 | Example file exists | `Glob("examples/{E}.rs")` |
68+
| 10 | Example has `pub fn run()` | `Grep("pub fn run", example_file)` |
69+
| 11 | Example has `fn main()` | `Grep("fn main", example_file)` |
70+
| 12 | `example_test!` registered | `Grep("example_test!\\({E}\\)", "tests/suites/examples.rs")` |
71+
| 13 | `example_fn!` registered | `Grep("example_fn!.*{E}", "tests/suites/examples.rs")` |
72+
| 14 | Paper `reduction-rule` entry | `Grep('reduction-rule.*"{S}".*"{T}"', "docs/paper/reductions.typ")` |
73+
74+
## Step 3: Run Build Checks
75+
76+
After structural checks, run:
77+
78+
```bash
79+
make test clippy
80+
```
81+
82+
Report pass/fail. If tests fail, identify which tests and suggest fixes.
83+
84+
## Step 4: Semantic Review (AI Judgment)
85+
86+
Read the implementation files and assess:
87+
88+
### For Models:
89+
1. **`evaluate()` correctness** -- Does it check feasibility before computing the objective? Does it return `SolutionSize::Invalid` / `false` for infeasible configs?
90+
2. **`dims()` correctness** -- Does it return the actual configuration space? (e.g., `vec![2; n]` for binary)
91+
3. **`problem_size_names`/`problem_size_values` consistency** -- Do the names match what `ReductionOverhead` uses?
92+
4. **Weight handling** -- Are weights managed via inherent methods, not traits?
93+
94+
### For Rules:
95+
1. **`extract_solution` correctness** -- Does it correctly invert the reduction? Does the returned solution have the right length (source dimensions)?
96+
2. **Overhead accuracy** -- Does `poly!(...)` reflect the actual size relationship?
97+
3. **Example quality** -- Is it tutorial-style? Does it use the instance from the issue? Does the JSON export include both source and target data?
98+
4. **Paper quality** -- Is the reduction-rule statement precise? Is the proof sketch sound? Is the example figure clear?
99+
100+
## Output Format
101+
102+
Present results as:
103+
104+
```
105+
## Review: [Model/Rule] [Name]
106+
107+
### Structural Completeness
108+
| # | Check | Status |
109+
|---|-------|--------|
110+
| 1 | Model file exists | PASS |
111+
| 2 | inventory::submit! | PASS |
112+
| ... | ... | ... |
113+
| N | Paper entry | FAIL -- missing display-name |
114+
115+
### Build Status
116+
- `make test`: PASS
117+
- `make clippy`: PASS
118+
119+
### Semantic Review
120+
- evaluate() correctness: OK
121+
- dims() correctness: OK
122+
- [any issues found]
123+
124+
### Summary
125+
- X/Y structural checks passed
126+
- [list of action items for any failures]
127+
```
128+
129+
## Integration with Other Skills
130+
131+
This skill is called automatically at the end of:
132+
- `add-model` (after Step 7: Verify)
133+
- `add-rule` (after Step 6: Verify)
134+
135+
It can also be invoked standalone via `/review-implementation`.

0 commit comments

Comments
 (0)