octopus-workflow/core/skills/review-code/reference/revision-loop.md

> Extracted from review-code/SKILL.md (Phase C — Developer Revision +
> Phase D — Re-review Loop) — moved verbatim; read on-demand when
> `overall_verdict` is not PASS (skipped entirely on PASS, which jumps
> straight to Phase E).

<!-- section:phase-c -->
## Phase C — Developer Revision

If `overall_verdict` is PASS, skip to Phase E.

Otherwise:

**Orchestrator actions**:

1. Prepare the Developer's input as a structured task file,
   created as a wiki page via `wiki 读写 API（见 TERMINOLOGY）` （`_shared/gitea-write-patterns.md` Pattern 1）with page_name `{slug}/reviews/code/round{N}/task-developer`.

   This file must contain:
   - Paths to all code files that need revision.
   - Reference to the synthesis comment on the ticket (posted via `工单评论 API（见 TERMINOLOGY）` in Phase B — Tier 2).
   - References to the Tier 1 finding files for dimensions that have FAIL or WARN
     verdicts (`<runs-root>/{slug}/reviews/code/round{N}/findings-{DIMENSION_CODE}.json`).
   - The Developer MUST NOT receive the full text of all reviewer findings
     inline — use finding-file path references only to avoid context explosion.
   - Convergence status: blockers/majors remaining vs. previous round.
   - **Revision survival ([TT-2601]) — primary path**: the task tool's completion
     output includes a `task_id`; passing it back resumes the SAME Developer
     subagent session instead of creating a fresh one. Round 1: persist the
     returned `task_id` with the round's working-notes entry (it must survive
     round-boundary compaction). Round ≥ 2: dispatch with `task_id` set to the
     previous revision round's — the Developer continues in-session with its
     prior diff/context; the round's task page then carries only the findings
     delta, not a full re-derivation. Reviewers are NEVER resumed — per-round
     freshness is their independence.
   - **Revision survival — fallback** (task_id lost to compaction before it
     was persisted): prepend the current `<runs-root>/{slug}/working-notes.md`
     content (run narrative memory — prior decisions, dead ends, reviewer
     feedback from earlier rounds) and reference the previous round's revision
     summary (`{slug}/reviews/code/round{N-1}/revision-summary`), stating
     explicitly that this revises code a prior round already wrote — do NOT
     re-read the full design/acceptance artifacts unless a finding requires
     them.

2. Spawn the Developer (Worker) with the prompt
   template at `core/skills/_shared/review-revision-prompt.md`, substituting:

   - `{ROLE}`: Developer
   - `{TASK_FILE}`: task-developer
   - `{REVISION_SCOPE}`: Apply the Developer Revision Protocol (see Protocol
      subsection below) — evaluate each finding against codebase reality:
      Explorer reviewers are automated LLM sub-agents that can hallucinate
      citations, misapply rules, or miss design context. Verify cited code
      exists, the rule applies, and the fix does not break existing
      functionality. WAIVE technically-incorrect findings with justification
      (`WAIVED-{finding-id}` → one-line reason; canonical definition of the
      `WAIVED-{id}` convention, incl. the round-2+ cosmetic-MINOR waiver
      rule: `_shared/review-revision-prompt.md`). Revise code minimally and
      surgically — do NOT refactor unrelated code or introduce new behavior.
   - `{RESOLUTION_RULE}`: Do NOT silently deviate from the approved baseline
     (DAG task mode: the node spec + frozen cross-session contracts in
     `{epic-slug}/dag`; the legacy `{slug}/03-design-*` baseline is archived
     historical, [TT-3072] phase 3). If a finding exposes a design flaw, mark it
     as deferred with justification and suggest a design amendment — do not
     change the code against the baseline.
   - `{COSMETIC_MINOR_EXAMPLES}`: naming, comments, dead code, import
     ordering, style
   - `{WAIVED_EXAMPLE}`: `WAIVED-COR-R2-001`
    - `{CHECKLIST_REF}`: `core/checklists/code-review.md`
    - `{FILES_MODIFIED_LINE}`: {list}
    - `{RETURN_FILE}`:
      `<runs-root>/{slug}/reviews/code/round{N}/revision-return.md`
      (Tier 1 return persistence — [TT-2847],
      `_shared/worker-report-persistence.md`)

3. The Developer revises the code files in place. The Developer creates a
   revision summary as a wiki page via `wiki 读写 API（见 TERMINOLOGY）` （`_shared/gitea-write-patterns.md` Pattern 1）with page_name `{slug}/reviews/code/round{N}/revision-summary`,
   and persists its final return summary to
   `<runs-root>/{slug}/reviews/code/round{N}/revision-return.md` (Tier 1,
   worker-report persistence [TT-2847] — see
   `_shared/worker-report-persistence.md`) BEFORE returning. If the
   completion notification is lost, the Orchestrator recovers the revision
   outcome from the persisted return (wiki page for the full summary,
   Tier 1 file for the conclusion of record).

4. **Pre-handoff gate** — before reporting completion, the Developer MUST run
   the same mechanical gate the Orchestrator will re-run at Phase D re-entry
   (the tree-fingerprint cache turns the re-run into a cache hit — one real
   execution per tree state, no double-paying):
   - Run `bun core/skills/review-code/scripts/precondition-gate.ts {slug} --round {N}`
     from the worktree root (oxlint + typecheck + `test:changed`, CI-aligned —
     `bun run test:parallel` is verify's job, not a per-revision
     requirement ([TT-2598])).
   - Exit 0 required; a new test failure, lint error, or type error introduced
     by the revision must be fixed before the Orchestrator proceeds to Phase D.
   - Report the gate's per-check results in the revision summary.

---

### Phase C — Developer Revision: Protocol

The Developer applies this cognitive protocol before implementing any finding.
**Core principle:** reviewer findings are claims to evaluate, not orders to
execute. Verify before implementing. Technical correctness over performative
agreement.

#### Response Pattern

1. **READ** — Load every FAIL/WARN dimension finding file
   (`<runs-root>/{slug}/reviews/code/round{N}/findings-{DIMENSION_CODE}.json`,
   Tier 1 local). Do not react item-by-item.
2. **UNDERSTAND** — Restate each finding's requirement in your own words.
   If you cannot, the finding is unclear — see STOP rule below.
3. **VERIFY** — Check the finding against codebase reality: does the cited
   code exist at the given file:line? Does the cited rule apply?
4. **EVALUATE** — Is the finding technically correct for THIS codebase?
   Does the proposed fix break existing functionality? Does it conflict
   with the approved baseline (DAG task mode: the node spec + contracts in
   `{epic-slug}/dag`)?
5. **RESPOND** — Fix it (correct), WAIVE it (`WAIVED-{finding-id}` →
   one-line reason), or flag it (unclear / design-level question).
6. **IMPLEMENT** — One verified finding at a time, test each individually.

#### Evaluating Findings — 5-Check Gate

Before implementing any finding, run these checks:

1. **Cited code is real** — Open the file:line. Does the code exist and say
   what the finding claims?
2. **Rule applies** — Does the cited checklist rule actually match this issue?
3. **Reason for current implementation** — Is the flagged behavior
   intentional? Check the baseline (node spec + contracts in
   `{epic-slug}/dag` for DAG task mode) for rationale.
4. **Fix does not break existing functionality** — Will the proposed change
   cause a regression?
5. **Reviewer has full context** — Did the reviewer see the baseline (node
   spec + acceptance criteria) and the diff context? An isolated diff review
   can miss architectural intent.

```
IF a finding seems wrong:
  WAIVE it: WAIVED-{finding-id} → <one-line technical reason>

IF you cannot easily verify a finding:
  State the limitation in revision_summary.md and request direction.

IF a finding conflicts with the approved design:
  Flag as a design-level question — do NOT silently change code against the design.
```

#### Unclear Findings — STOP Rule

```
IF any finding is unclear:
  STOP — do not implement ANY finding yet.
  Flag the unclear items in revision_summary.md and request clarification.

WHY: findings may be related. Partial understanding produces wrong
     implementation across multiple items.
```

**Example:** The synthesis lists findings F1–F6. You understand F1, F2, F3, F6.
F4 and F5 are ambiguous about what change is expected.
- **WRONG:** implement F1, F2, F3, F6 now; ask about F4, F5 later.
- **RIGHT:** "Understand F1, F2, F3, F6. Need clarification on F4, F5 before
  proceeding — they may interact with the same module."

#### YAGNI Check

```
IF a reviewer suggests "implementing this properly" (full abstraction,
generic API, configurable layer):
  Check actual usage (codegraph callers / grep) BEFORE expanding scope.

  IF unused:  WAIVE — "No call sites found. Leaving as-is (YAGNI)."
  IF used:    Then implement properly.
```

#### Forbidden Responses in revision_summary.md

**NEVER** write:
- "You're absolutely right!" / "Great point!" / "Excellent catch!" — performative.
- "Thanks for catching that!" / any gratitude expression.
- "Let me implement that now" — before verification.

**INSTEAD:**
- Restate the technical requirement concisely.
- State the fix factually: `Fixed {finding-id}: <what changed> at <file:line>`.
- Just fix it and show the result in the diff.

#### Gracefully Correcting a WAIVE

If you WAIVEd a finding and were wrong:
```
"Verified {finding-id} — the reviewer is correct. My initial read missed
 <reason>. Implementing now."
```
State the correction factually. No lengthy apology.

#### Implementation Order

For multi-finding revisions, after clarifying all unclear items:
1. **Blocking issues** — security, data loss, breaks existing functionality.
2. **Simple fixes** — typos, imports, naming, dead code.
3. **Complex fixes** — logic changes, refactoring within minimal-surgical scope.
Test each fix individually.

#### Rationalization-Anticipation Quick Reference

| Trap | Hold to |
|------|---------|
| "The reviewer flagged it as BLOCKER, so it must be real." | VERIFY cited code exists and rule applies before any edit. |
| "Just fix everything to avoid a re-review round." | EVALUATE each finding; WAIVE the wrong ones with reasoning. |
| "This finding looks plausible — I'll fix it without checking." | Open the file:line, confirm the behavior. |
| "I'll implement the clear findings and ask about unclear ones later." | STOP. Clarify ALL unclear items before implementing any. |
| "The reviewer wants it done 'properly' — full abstraction." | Check call sites first. If unused, WAIVE (YAGNI). |
| "I shouldn't push back — the reviewer is probably right." | Push back with technical reasoning. The diff is the acknowledgment. |

---

<!-- section:phase-d -->
## Phase D — Re-review Loop

After the Developer completes revision:

1. Increment the round counter (the Synthesizer updates the commit status in Phase B
   with the new round's results).
2. Re-enter Phase A at step 2.5 — the mechanical-green gate re-runs on the
   revised tree before any reviewer spawn (a red gate blocks dispatch exactly
   as on round-1 entry) — then spawn the new round of reviewers against the
   revised files.

**Convergence criteria**: see `core/skills/_shared/review-pipeline-phases.md` Phase D for the canonical table (stop conditions). `{MAX_ROUNDS}` is per-skill — code review binds the routing budget: **2** rounds default, **3** for high risk (a BLOCKER survived a round, or the diff touches a frozen cross-session contract) — `<instance-root>/workflow-routing.yaml` `review.max_rounds` ([TT-3567]). The code-review-specific addition is inheritance of the shared INFO-disposition rule. The shared Round-3 Escalation Guardrail is scoped to the `review-artifact` skill targets only and does not apply here.

**Round budget (mandatory, [TT-3567])**: when `current_round` reaches the budget (2; 3 for high risk) without convergence, do NOT enter another round. STOP: register every remaining BLOCKER/MAJOR finding as TD registry rows (`rules/ticket-lifecycle.md`), record the verdict with the shared three-tier ladder (`core/schemas/synthesis.schema.json` `overall_verdict` + `core/skills/_shared/review-orchestrator-rules.md` rule 3): FAIL if any BLOCKER or FAIL dimension remains, else WARN, PASS only if every remaining dimension is PASS ([TT-4786]: the prior binary "else PASS" wording silently upgraded WARN-eligible outcomes to PASS), and close the loop. The debt is carried by the TD registry rows just registered, not by the recorded verdict. A review that cannot converge within budget is a design-level stall wearing review clothes (retro 2026-08-21 / [TT-3050]): in DAG task mode, map it to the DAG oversize signals (`<instance-root>/workflow-routing.yaml` `dag.size_derivation.oversize_signals`) and let the aggregator agent drive re-derivation + a `review-dag` re-run; otherwise record the design-level finding on the Epic, or open `{slug}/03-design-amendments` via `wiki 读写 API（见 TERMINOLOGY）` (`_shared/gitea-write-patterns.md` Pattern 1).

**Efficiency optimization**: see `core/skills/_shared/review-pipeline-phases.md` Phase D (same rule — from round 2 onward ([TT-2598]), only spawn reviewers for FAIL/WARN/UNRESOLVED dimensions; full sweep every 2 rounds). Round ≥2 reviewer input is delta-scoped to the revised files + their neighbors.

<!-- inject: ../_shared/compaction-round-boundary.md -->

---