deep-review: add a close-out contract, an answer-type axis, and a termination criterion

Aperta
  • Qualcosa non va?
#239 0 commenti 0 reazioni 0 assegnatari Vedi su GitHub

Nessuno ha ancora preso questa issue.

Valutazione

Difficoltà
3/5
Tempo stimato
1-2 giorni
Idoneità per principianti
72/100
Tipo di issue
Documentazione
Chiarezza
Specificata chiaramente
Stato di attività
Attiva
Stack tecnologico
markdown
Ambito
documentation

Direzione di ricerca

Inizia da plugins/deep-review/commands/deep-review.md, in particolare da L14, L18, dal modello di output di Findings e dal relativo controllo automatico, quindi confrontalo con plugins/deep-review/README.md. Aggiungi il contratto di chiusura, il tag del tipo di risposta, il criterio di terminazione e l’aspettativa di indipendenza descritti nei criteri di accettazione; il lavoro è completato quando ogni criterio elencato è riflesso coerentemente in entrambi i file.

Scritto dal modello di indicizzazione a partire dal testo della issue.

Descrizione

Problem

/deep-review specifies how to produce findings in exhaustive detail and says nothing about how to close one. Grepping the skill (plugins/deep-review/commands/deep-review.md, 397 lines) for fix|close out|remediat|re-run|iterate|converge returns only hits about fixing its own output format.

Two of its own directives then drive a ratchet:

  • L14: "Find every problem — all of them, in one pass. Do not save findings for a follow-up run."
  • L18: "A review that finds 5 critical bugs but misses 3 warnings and 4 suggestions is a failure — those missed items will require a re-run, breaking the developer's flow."

It frames a re-run as evidence the previous review was incomplete. In practice a re-run happens because the previous round's fixes enlarged the reviewable surface.

Evidence — TimZander/AudioClassifier PR #737, five review rounds

Per-commit comment-vs-code line churn in pwa/*.js (base dd2811f → head 53d2fe5):

comment code
2 implementation commits +65 / −38 +13 / −38
6 review-response commits +580 / −390 +58 / −37
  • The last three review commits changed 0, 23/−22, and 0 code lines.
  • Round 5 produced 27 findings; roughly 20 were defects in comments written by rounds 3 and 4.
  • The three most severe were comments asserting that load-bearing cross-deploy shims were safe to delete. Each would have caused a real regression if a later author believed it.

Root cause. The cheapest way to close a finding is to write a paragraph explaining why the code is right. That paragraph is a new factual claim, and nothing in CI reads a comment — not the test suite, not tsc, not eslint. A wrong line of code goes red; a wrong line of prose ships and then misdirects the next author. Each round therefore converts findings into unverified assertions at roughly 10:1, and the next round's completeness mandate dutifully reviews all of them.

Goal

Give the skill the second half of the loop, so a review sequence terminates instead of ratcheting.

Proposed changes

1. Close-out contract

Add a section stating that a finding is closed by a code change, a test, or a deletion — never by an explanatory comment. If prose is the only possible answer, the finding is about the design: it belongs in the issue tracker, not in the source file.

2. Answer-type axis on every finding

Findings today carry severity (red/yellow/bulb) but not what kind of answer they admit. Add a required tag — code / test / delete / design. Two benefits: design findings route out of the source by construction, and a round whose findings are mostly design is a signal the PR has a scope problem rather than a code problem.

3. Termination criterion

Replace L18's framing. Add: a review round whose fixes change no code is the signal to stop reviewing, not to review again. Re-running deep-review over the previous round's prose is the failure mode — not incompleteness.

4. Independence expectation

Add: the agent that authored the fixes should not be the sole reviewer of them. Repeated rounds by one author converge on that author's blind spots. A later round should either change reviewer, or restrict itself to verifying the previous round's findings were actually closed.

Acceptance criteria

  • Skill contains a "Closing a finding" section stating the code / test / delete / design rule
  • The Findings output template requires an answer-type tag alongside the severity emoji
  • The existing output-format self-check enforces the tag, so an untagged finding fails the check
  • The template example in the skill shows a tagged finding
  • L18's "a re-run means the review was incomplete" framing is replaced by an explicit termination criterion
  • Skill states the independence expectation for repeat rounds on the same PR
  • plugins/deep-review/README.md reflects the above

Notes

The detector half is sound and should not change. The parallel agent fan-out, the mutation-testing discipline, and the requirement to verify each subagent finding before including it all found real defects in every round of #737. This issue is purely additive.

A companion rule for standards/CLAUDE.md — "a review finding is closed by a code change, a test, or a deletion; an explanatory paragraph is a new unverified claim and is not a fix" — was identified in the same session and is worth filing if this lands.

Generated by Claude Code

Lingua principale
Shell
Stelle
0
Fork
1
Merge medio
3h 19m
PR unite (30g)
8

Preparare l'ambiente

Come iniziare

  1. Leggi tutta la issue e poi la guida ai contributi del progetto.
  2. Commenta sulla issue per dire che te ne occupi tu — evita che due persone facciano lo stesso lavoro.
  3. Fai un fork del repository e lavora su un branch.
  4. Apri una pull request che faccia riferimento al numero della issue.

Altre issue di TimZander/claude

Tutte le issue di TimZander/claude

Issue simili

Altre issue su Shell/Bash

Ricevi le nuove issue nella tua casella

Un breve riepilogo di issue GitHub adatte ai principianti.