SDD: task reviewer writes its full report to a review file (#1930)

[rasibintang]

Targets dev.

Who is submitting this PR? (required)

Field	Value
Your model + version	Claude Fable 5 (claude-fable-5)
Harness + version	Claude Code, VS Code extension (Agent SDK); eval subagents ran on Claude Sonnet 5 (claude-sonnet-5)
All plugins installed	superpowers 6.1.1 (claude-plugins-official marketplace)
Human partner who reviewed this diff	@rasibintang

What problem are you trying to solve?

Issue #1930: task-reviewer subagents return their entire report as the final message, so every passing check, Strengths section, and fix rationale lands in the controller's context and is re-read on every later turn. The issue reporter's transcript analysis found most of that content is non-actionable at the controller level ("things they checked that were OK").

The skill's own File Handoffs section already names this exact failure mode ("everything a subagent prints back stays resident in your context for the rest of the session") and fixes it for every other role: the implementer writes its full report to task-N-report.md and returns under 15 lines; the review package "never enters the controller's context"; fix subagents append to the report file and return a short summary. The task reviewer is the one remaining role whose full output lands in the controller's context — task-reviewer-prompt.md explicitly instructs "Your final message is the report itself."

Proposed and discussed on the issue: #1930 (comment)

What does this PR change?

Applies the implementer's report-file contract to the task reviewer: the full report (Strengths, passing-check citations, finding detail with fixes) goes to a new [REVIEW_FILE] (task-N-review.md, following the existing brief/report naming); the final message returns only the two verdicts, any ⚠️ "cannot verify from diff" items, one line per Critical/Important finding, a Minor count, and the file path. SKILL.md is updated so fix dispatches pass the review file path, the ledger records Minor counts + review file paths, and the controller is told not to read the review file.

Is this change appropriate for the core library?

Yes — it modifies an existing core SDD skill, is harness-agnostic (prompt/markdown only, no scripts or infrastructure), and benefits any project type. No third-party integration.

What alternatives did you consider?

1. Drop the non-actionable content entirely (the issue author's own alternative). Rejected: the passing-check citation requirement plausibly exists to force the reviewer to actually perform the checks, and Strengths content calibrates trust for the fix loop. This PR relocates that information instead of deleting it.
2. Implement all three parts of Feat: reduce reviewer subagent verbosity to conserve tokens #1930 at once, including requesting-code-review/code-reviewer.md and a blanket "orchestrator must not read raw findings" rule. Deferred: code-reviewer.md is also used standalone, where a human reads the report directly, so it needs a mode split; kept as a follow-up so this PR stays one scoped behavior change.

Deliberately preserved constraints: ⚠️ items and plan-mandated findings stay in the final message because the controller/human must act on them directly, and the controller can still make the fix-vs-approve decision from the message alone.

Does this PR contain multiple unrelated changes?

No. Two files, one behavior: the reviewer's reporting contract and the controller-side handling of it.

Existing PRs

• I have reviewed all open AND closed PRs for duplicates or prior art
• Related PRs: fix(sdd): task-scoped review dispatch — single task reviewer, review-package script, eval-tuned #1717 (merged — introduced the unified task-reviewer-prompt.md this PR modifies), Store subagent-driven-development artifacts in a per-branch subdirectory #1956 (open — relocates SDD artifact paths; adjacent to the task-N-review.md naming but not conflicting: this PR follows whatever directory convention the brief/report files use). No PR implementing Feat: reduce reviewer subagent verbosity to conserve tokens #1930 found.

Environment tested

Harness (e.g. Claude Code, Cursor)	Harness version	Model	Model version/ID
Claude Code (VS Code extension)	current stable	Claude Sonnet 5 (reviewer subagents), Claude Fable 5 (controller)	claude-sonnet-5, claude-fable-5

New harness support (required if this PR adds a new harness)

N/A — no new harness.

Evaluation

• Initial prompt: my human partner asked me to analyze issue Feat: reduce reviewer subagent verbosity to conserve tokens #1930 and prepare a contribution implementing it (session conducted in Indonesian; the work was proposed on the issue before this PR was opened).
• Eval sessions after the change: 2 A/B pairs (n=2 per arm, 4 reviewer subagent sessions total, all on Sonnet 5 with fresh context). Scenario: a small repo with a seeded-flaw task implementation — a skipped requirement (missing Done: N items total log), an explicitly forbidden feature (options.json API surface), and an implementer report that falsely claims full compliance — reviewed once with the current dev prompt (baseline) and once with this PR's prompt, identical inputs otherwise.
• Outcomes:
  • Detection parity, 4/4 runs: both arms caught both seeded spec violations, flagged the over-claiming implementer report, and returned Spec ❌ / "Needs fixes".
  • Controller-context cost: baseline final messages were 4,200 and 4,557 chars (594/632 words). New-prompt final messages were 1,153 and 1,232 chars (138/129 words) — a ~73% reduction in what enters (and stays in) the controller's context per review, before the re-read multiplier.
  • Detail preserved, not dropped: both new-prompt runs wrote the full report to the review file (5,191 and 4,965 chars) with every Output Format section present (Spec Compliance, Strengths, Critical/Important/Minor with file:line + fixes, Assessment).
  • Honest scope note: n=2 per arm on one scenario. The size reduction is structural (the contract itself bounds the message); the detection-parity claim is the part that would benefit from more scenarios, and I'm happy to run more if you want them.

Rigor

• If this is a skills change: I used superpowers:writing-skills and
  completed adversarial pressure testing (paste results below)
• This change was tested adversarially, not just on the happy path
• I did not modify carefully-tuned content (Red Flags table,
  rationalizations, "human partner" language) without extensive evals
  showing the change is an improvement

Per writing-skills' "Match the Form to the Failure": the baseline failure is wrong-shaped output (report bloat in the final message), not rule-skipping, so the change is a positive contract (what the final message IS, part by part — mirroring the implementer's proven under-15-lines contract) rather than a prohibition list. The eval scenario is adversarial for a reviewer: an implementer report that over-claims ("All requirements implemented... No concerns") while the diff skips a requirement and adds a forbidden feature — the reviewer must contradict the report, not just summarize it. Both new-prompt runs did (results above). The Red Flags table, rationalization tables, and "human partner" language were not touched.

Human review

• A human has reviewed the COMPLETE proposed diff before submission

Refs #1930