Skip to content
This repository was archived by the owner on Apr 11, 2026. It is now read-only.

Commit 178fe62

Browse files
z23ccclaude
andcommitted
feat(review): enhance circuit breaker with regression, oscillation, classification, and severity limits
Task: fn-110-workflow-reliability-and-performance.6 - Add regression detection: breaks fix loop if finding count increases between iterations - Add oscillation detection: breaks fix loop if a finding reappears after being fixed - Add finding classification: deterministic vs subjective, subjective-only findings trigger SHIP - Add severity-based iteration limits: P0/P1=3, P2/P3=2, subjective=1 - Applied consistently to both impl-review and plan-review skills Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent cf99bb0 commit 178fe62

2 files changed

Lines changed: 67 additions & 11 deletions

File tree

skills/flow-code-impl-review/SKILL.md

Lines changed: 33 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -142,17 +142,45 @@ The workflow covers:
142142

143143
**CRITICAL: Do NOT ask user for confirmation. Automatically fix ALL valid issues and re-review — our goal is production-grade world-class software and architecture. Never use AskUserQuestion in this loop.**
144144

145-
**MAX ITERATIONS**: Limit fix+re-review cycles to **${MAX_REVIEW_ITERATIONS:-3}** iterations (default 3). If still NEEDS_WORK after max rounds, stop the fix loop and return to the worker with status NEEDS_WORK — the worker will report SPEC_CONFLICT.
145+
**MAX ITERATIONS** (severity-based limits):
146+
- **P0/P1 findings** (critical/major): max **3** fix iterations
147+
- **P2/P3 findings** (minor/trivial): max **2** fix iterations
148+
- **Subjective findings** (naming, style, architecture opinion): max **1** iteration, then defer
149+
150+
Default fallback: **${MAX_REVIEW_ITERATIONS:-3}** iterations. If still NEEDS_WORK after max rounds, stop the fix loop and return to the worker with status NEEDS_WORK — the worker will report SPEC_CONFLICT.
151+
152+
### Finding Classification
153+
154+
Classify each finding as **deterministic** or **subjective**:
155+
- **Deterministic**: lint error, type error, missing test, compilation failure, spec violation, security flaw — objectively verifiable
156+
- **Subjective**: naming preference, architecture opinion, style choice, code organization suggestion — reasonable people disagree
157+
158+
If all remaining unresolved findings are **subjective**, issue **SHIP** verdict with recorded concerns rather than continuing the fix loop. Log: "Review circuit breaker: all remaining findings are subjective. Issuing SHIP with recorded concerns."
159+
160+
### Regression Detection
161+
162+
Track the **finding count per iteration**. If iteration N+1 produces **MORE** findings than iteration N, the fixes are introducing new problems. Break the loop immediately with:
163+
> "Review circuit breaker: regression detected (findings increased from X to Y). Stopping fix loop."
164+
165+
### Oscillation Detection
166+
167+
Compare finding titles/descriptions across iterations. If a finding from iteration N **reappears** in iteration N+2 (was fixed then reintroduced), break immediately with:
168+
> "Review circuit breaker: oscillation detected (finding 'X' reappeared). Stopping fix loop."
169+
170+
### Fix Loop Steps
146171

147172
If verdict is NEEDS_WORK, loop internally until SHIP:
148173

149174
1. **Parse issues** from reviewer feedback (Critical → Major → Minor)
150-
2. **Fix code** and run tests/lints
151-
3. **Commit fixes** (mandatory before re-review)
152-
4. **Re-review**:
175+
2. **Classify findings** as deterministic or subjective (see above)
176+
3. **Check regression**: compare finding count against previous iteration — break if increased
177+
4. **Check oscillation**: compare finding descriptions against all prior iterations — break if any reappeared
178+
5. **Fix code** and run tests/lints
179+
6. **Commit fixes** (mandatory before re-review)
180+
7. **Re-review**:
153181
- **Codex**: Re-run `flowctl codex impl-review` (receipt enables context)
154182
- **RP**: `$FLOWCTL rp chat-send --window "$W" --tab "$T" --message-file /tmp/re-review.md` (NO `--new-chat`)
155-
5. **Repeat** until `<verdict>SHIP</verdict>`
183+
8. **Repeat** until `<verdict>SHIP</verdict>` or circuit breaker triggers
156184

157185
**CRITICAL**: For RP, re-reviews must stay in the SAME chat so reviewer has context. Only use `--new-chat` on the FIRST review.
158186

skills/flow-code-plan-review/SKILL.md

Lines changed: 34 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -164,12 +164,40 @@ The workflow covers:
164164

165165
**CRITICAL: Do NOT ask user for confirmation. Automatically fix ALL valid issues and re-review — our goal is production-grade world-class software and architecture. Never use AskUserQuestion in this loop.**
166166

167-
**MAX ITERATIONS**: Limit fix+re-review cycles to **${MAX_REVIEW_ITERATIONS:-3}** iterations (default 3, consistent with impl-review and shared protocol). If still NEEDS_WORK after max rounds, stop the fix loop and log: "Plan review: MAX_REVIEW_ITERATIONS reached. Proceeding with remaining concerns as gaps."
167+
**MAX ITERATIONS** (severity-based limits):
168+
- **P0/P1 findings** (critical/major): max **3** fix iterations
169+
- **P2/P3 findings** (minor/trivial): max **2** fix iterations
170+
- **Subjective findings** (naming, style, architecture opinion): max **1** iteration, then defer
168171

169-
If verdict is NEEDS_WORK, loop internally until SHIP or max iterations:
172+
Default fallback: **${MAX_REVIEW_ITERATIONS:-3}** iterations (consistent with impl-review and shared protocol). If still NEEDS_WORK after max rounds, stop the fix loop and log: "Plan review: MAX_REVIEW_ITERATIONS reached. Proceeding with remaining concerns as gaps."
173+
174+
### Finding Classification
175+
176+
Classify each finding as **deterministic** or **subjective**:
177+
- **Deterministic**: missing acceptance criteria, spec contradiction, feasibility gap, security concern, missing dependency — objectively verifiable
178+
- **Subjective**: naming preference, architecture opinion, organizational style, scope suggestion — reasonable people disagree
179+
180+
If all remaining unresolved findings are **subjective**, issue **SHIP** verdict with recorded concerns rather than continuing the fix loop. Log: "Review circuit breaker: all remaining findings are subjective. Issuing SHIP with recorded concerns."
181+
182+
### Regression Detection
183+
184+
Track the **finding count per iteration**. If iteration N+1 produces **MORE** findings than iteration N, the fixes are introducing new problems. Break the loop immediately with:
185+
> "Review circuit breaker: regression detected (findings increased from X to Y). Stopping fix loop."
186+
187+
### Oscillation Detection
188+
189+
Compare finding titles/descriptions across iterations. If a finding from iteration N **reappears** in iteration N+2 (was fixed then reintroduced), break immediately with:
190+
> "Review circuit breaker: oscillation detected (finding 'X' reappeared). Stopping fix loop."
191+
192+
### Fix Loop Steps
193+
194+
If verdict is NEEDS_WORK, loop internally until SHIP or circuit breaker triggers:
170195

171196
1. **Parse issues** from reviewer feedback
172-
2. **Fix epic spec** (stdin preferred, temp file if content has single quotes):
197+
2. **Classify findings** as deterministic or subjective (see above)
198+
3. **Check regression**: compare finding count against previous iteration — break if increased
199+
4. **Check oscillation**: compare finding descriptions against all prior iterations — break if any reappeared
200+
5. **Fix epic spec** (stdin preferred, temp file if content has single quotes):
173201
```bash
174202
# Preferred: stdin heredoc
175203
$FLOWCTL epic plan <EPIC_ID> --file - --json <<'EOF'
@@ -179,7 +207,7 @@ If verdict is NEEDS_WORK, loop internally until SHIP or max iterations:
179207
# Or temp file
180208
$FLOWCTL epic plan <EPIC_ID> --file /tmp/updated-plan.md --json
181209
```
182-
3. **Sync affected task specs** - If epic changes affect task specs, update them:
210+
6. **Sync affected task specs** - If epic changes affect task specs, update them:
183211
```bash
184212
$FLOWCTL task spec <TASK_ID> --file - --json <<'EOF'
185213
<updated task spec content>
@@ -191,10 +219,10 @@ If verdict is NEEDS_WORK, loop internally until SHIP or max iterations:
191219
- Approach/design decisions tasks depend on
192220
- Lock/retry/error handling semantics
193221
- API signatures or type definitions
194-
4. **Re-review**:
222+
7. **Re-review**:
195223
- **Codex**: Re-run `flowctl codex plan-review` (receipt enables context)
196224
- **RP**: `$FLOWCTL rp chat-send --window "$W" --tab "$T" --message-file /tmp/re-review.md` (NO `--new-chat`)
197-
5. **Repeat** until `<verdict>SHIP</verdict>`
225+
8. **Repeat** until `<verdict>SHIP</verdict>` or circuit breaker triggers
198226
199227
**Recovery**: If context compaction occurred during review, restore from checkpoint:
200228
```bash

0 commit comments

Comments
 (0)