diff --git a/docs/agent-template.md b/docs/agent-template.md index 4711087..281dba4 100644 --- a/docs/agent-template.md +++ b/docs/agent-template.md @@ -127,7 +127,16 @@ Assignment rule, in priority order: 3. **Volume × wall-clock** — agents dispatched per-task inside loops or fanned out in parallel swarms multiply their model's latency; they run `sonnet` unless rule 1 overrides (currently - only `quality-reviewer`, the loop's last correctness gate). + only `quality-reviewer`, the loop's last correctness review). + That override is itself **size-conditioned** (issue #30): + consequence-of-a-miss scales with diff size, so the + implement-loop dispatches the quality phase at `sonnet`/`high` + when the spec-reviewer independently measured the diff at ≤25 + changed lines touching no contract-referenced path, and at + `opus`/`xhigh` otherwise (including whenever the measurement is + missing). The deterministic end-verify/mini-verify suite gate + is what makes the lower tier defensible; the agent-file + frontmatter keeps the opus default. Current distribution: `opus` — architect, bencher, debugger, fieldtester, grounding-check, plan-recon, quality-reviewer, diff --git a/implement/SKILL.md b/implement/SKILL.md index 1cd8674..41d93e1 100644 --- a/implement/SKILL.md +++ b/implement/SKILL.md @@ -169,6 +169,16 @@ ground truth, never a self-report** — a self-report must never be able to The E2E phase's `status` is also read now (not just its fixtures): a non-`DONE` E2E status or a zero-fixture run surfaces as a concern. +**Quality-tier selection** (issue #30). The quality phase runs +opus/xhigh by default, tiered to sonnet/high for a small diff: ≤25 +changed lines AND no contract-referenced path, both measured by the +spec-reviewer (which reads `git diff HEAD` itself — never the +implementer's self-report) via `diff_magnitude` / `touches_contract`. +Unmeasured or contract-touching diffs stay opus. The independent +end-verify suite gate is the deterministic backstop; the chosen tier +is logged per task in the end-report (`qual_tier`) so the split stays +auditable. + **Cross-task discard guard** (issue #23). In a multi-task run nothing commits between tasks, so one task's file-level `git checkout`/`git restore` can silently destroy an earlier task's DONE work in a shared diff --git a/implement/agents/quality-reviewer.md b/implement/agents/quality-reviewer.md index 1836c99..9eaa826 100644 --- a/implement/agents/quality-reviewer.md +++ b/implement/agents/quality-reviewer.md @@ -19,6 +19,14 @@ has reported `compliant`, never before. only after the spec-compliance phase reports `compliant` — a separate `agent()` call with a fresh context.) +The frontmatter pins opus/xhigh as this role's default tier; +the workflow overrides it per dispatch (issue #30): a small +(≤25 changed lines), non-contract diff — as measured +independently by the spec-reviewer, never by the implementer — +runs at sonnet/high, with the loop's independent end-verify +suite gate as the deterministic correctness backstop. The +review bar and this file's rules are identical at both tiers. + ## What this role is for Spec compliance answers "did the implementer build the right diff --git a/implement/workflows/implement-loop.js b/implement/workflows/implement-loop.js index 9c43aac..9465cf8 100644 --- a/implement/workflows/implement-loop.js +++ b/implement/workflows/implement-loop.js @@ -81,8 +81,12 @@ // • Model policy: every agent() call pins an explicit model — never the // session-model inherit (which could route to an unintended tier, e.g. // fable — banned for all plugin agents and workflows). Mechanical and -// in-loop steps run sonnet; the quality gate is the one deliberate opus -// call (last correctness check before a task is marked DONE). +// in-loop steps run sonnet; the quality gate runs opus by default (last +// correctness review before the end-verify), TIERED to sonnet/high for a +// small (<= 25 changed lines), non-contract diff as measured +// independently by the spec-reviewer (issue #30) — the end-verify / +// mini-verify suite gate is the deterministic backstop that makes the +// tier defensible. Unmeasured or contract-touching diffs stay opus. // • Effort policy mirrors the model policy: every agent() call pins an // explicit effort — never the session inherit. Code-writing and review // steps run high, the opus quality gate runs xhigh, and schema-bound @@ -353,11 +357,26 @@ const IMPL_SCHEMA = { const SPEC_SCHEMA = { type: 'object', additionalProperties: false, - required: ['status'], + required: ['status', 'diff_magnitude', 'touches_contract'], properties: { status: { type: 'string', enum: ['compliant', 'non_compliant', 'unclear', 'infra_blocked'] }, findings: { type: 'array', items: { type: 'string' }, description: 'missing requirements + unrequested extras' }, ambiguity: { type: 'string' }, + // Quality-tier inputs (issue #30): measured HERE because the spec-reviewer + // runs before the quality gate, reads the diff itself, and is independent + // of the implementer — never a producer self-report. + diff_magnitude: { + type: 'integer', + description: + 'total changed lines of the FULL `git diff HEAD` (insertions + deletions, from `git diff HEAD --shortstat`) ' + + '— measure it yourself; never take it from the implementer', + }, + touches_contract: { + type: 'boolean', + description: + 'true if any diffed path is part of (or referenced by) the design ledger / contracts the project\'s ' + + 'CLAUDE.md facts declare; false when the project declares none', + }, }, } const QUAL_SCHEMA = { @@ -768,6 +787,7 @@ async function runTask(task) { let wroteAnyFile = impl.applied_changes === true // 2.2 — spec-compliance check (gated before quality), with repair re-loop + let specMeta = null for (let round = 0; ; round++) { const spec = await agent( `${STANDING}${ANCHOR}\n\nSpec-compliance review of task ${task.id}. Compare \`git diff HEAD\` against the task ` + @@ -776,10 +796,17 @@ async function runTask(task) { 'you have verified yourself (the actual behaviour provably differs from what the task text asserts), or ' + 'two task requirements contradict each other, report status `unclear` with the contradiction in ' + '`ambiguity` — never `non_compliant` for a reality-forced deviation, which no repair can resolve. ' + + 'Additionally measure and report, independent of any implementer report: `diff_magnitude` (total changed ' + + 'lines from `git diff HEAD --shortstat`, insertions + deletions) and `touches_contract` (does any diffed ' + + "path belong to or get referenced by the design ledger / contracts the project's CLAUDE.md facts declare; " + + 'false when the project declares none). ' + `Focus on the files this task claims to touch.\n\nTASK ${task.id} — ${task.title}\n${task.text}`, { agentType: 'spec-reviewer', model: 'sonnet', effort: 'high', label: `spec:${task.id}`, phase: 'Per-task loop', schema: SPEC_SCHEMA }, ) - if (spec && spec.status === 'compliant') break + if (spec && spec.status === 'compliant') { + specMeta = spec + break + } if (spec && spec.status === 'unclear') return { id: task.id, title: task.title, outcome: 'BLOCKED', reason: 'spec-ambiguous', detail: spec.ambiguity } if (round >= 2) return { id: task.id, title: task.title, outcome: 'BLOCKED', reason: 'review-loop-exhausted (spec-compliance)' } // repair: re-dispatch the implementer with the review findings as the repair brief @@ -801,6 +828,28 @@ async function runTask(task) { // whose fingerprint was already reviewed-and-flagged this task means the // intervening repair changed nothing or cycled back, so re-running quality // is futile). + // Quality-tier selection (issue #30): the review model scales with the + // independently-measured diff. The spec-reviewer — a separate agent that + // read `git diff HEAD` itself — supplied diff_magnitude and + // touches_contract; a small, non-contract diff gets sonnet/high, because + // consequence-of-a-miss (the opus pin's rationale, agent-template § model) + // scales with diff size and the independent end-verify / mini-verify suite + // gate is the deterministic correctness backstop. Anything larger, + // contract-touching, or unmeasured (missing/implausible fields) keeps + // opus/xhigh — fail conservative. Measured once, before round 0; a repair + // can grow the diff, but bounded (it addresses findings on a small diff), + // so the tier is not re-derived per round. + const QUAL_TIER_MAX_LINES = 25 + const smallDiff = + specMeta !== null && + Number.isInteger(specMeta.diff_magnitude) && + specMeta.diff_magnitude >= 0 && + specMeta.diff_magnitude <= QUAL_TIER_MAX_LINES && + specMeta.touches_contract === false + const qualModel = smallDiff ? 'sonnet' : 'opus' + const qualEffort = smallDiff ? 'high' : 'xhigh' + const qualTier = `${qualModel}/${qualEffort}` + let heldConcerns = [] const seenFingerprints = new Set() for (let round = 0; ; round++) { @@ -811,9 +860,10 @@ async function runTask(task) { 'Also report `diff_fingerprint` (REQUIRED): the sha256 hex of the FULL `git diff HEAD` ' + '(`git diff HEAD | sha256sum` — the whole working-tree diff, NOT narrowed to this task\'s footprint). ' + 'The loop compares it across rounds to detect a no-op/cyclic repair.', - // opus by design: the loop's last correctness gate (spec-reviewer only checks - // task-text correspondence) — real-bug finding is the documented opus strength. - { agentType: 'quality-reviewer', model: 'opus', effort: 'xhigh', label: `qual:${task.id}`, phase: 'Per-task loop', schema: QUAL_SCHEMA }, + // The loop's last correctness gate before the end-verify (spec-reviewer only + // checks task-text correspondence) — opus by default (real-bug finding is the + // documented opus strength), tiered per the selection above. + { agentType: 'quality-reviewer', model: qualModel, effort: qualEffort, label: `qual:${task.id}`, phase: 'Per-task loop', schema: QUAL_SCHEMA }, ) if (qual && qual.status === 'approved') break // No-op / cyclic-repair backstop: a changes_requested verdict over a diff-state already reviewed-and-flagged @@ -835,6 +885,7 @@ async function runTask(task) { outcome: 'BLOCKED', reason: 'unresolved important quality finding — the repair left the diff unchanged', detail: importants.map((i) => i.text).join('; '), + qual_tier: qualTier, } } heldConcerns = (qual.issues && qual.issues.length ? qual.issues.map((i) => `${i.severity}: ${i.text}`) : ['(finding not reported)']).map( @@ -863,7 +914,7 @@ async function runTask(task) { ) break } - if (round >= 2) return { id: task.id, title: task.title, outcome: 'BLOCKED', reason: 'review-loop-exhausted (quality)' } + if (round >= 2) return { id: task.id, title: task.title, outcome: 'BLOCKED', reason: 'review-loop-exhausted (quality)', qual_tier: qualTier } if (qual && qual.diff_fingerprint) seenFingerprints.add(qual.diff_fingerprint) const fix = await agent( `${STANDING}${ANCHOR}\n\nThe quality review of task ${task.id} requested changes. Address EACH Important/Minor issue; ` + @@ -892,6 +943,7 @@ async function runTask(task) { title: task.title, outcome: 'BLOCKED', reason: `quality/plan conflict: ${fix.reason || 'honouring a quality finding would break the ratified plan'}`, + qual_tier: qualTier, } } if (fix && fix.held && fix.held.length) { @@ -923,6 +975,7 @@ async function runTask(task) { title: task.title, outcome: 'DONE', concerns: [...concerns, ...heldConcerns, ...noopConcern], + qual_tier: qualTier, } } @@ -1131,7 +1184,11 @@ const aggregate = { outcome, tasks_total: tasks.length, tasks_completed: completed.length, - task_summaries: results.map((r) => ({ id: r.id, title: r.title, outcome: r.outcome })), + // qual_tier (issue #30): which review tier each task's quality gate ran at + // (sonnet/high for a small non-contract diff, opus/xhigh otherwise; null + // when the task never reached the quality gate) — logged so the tiering + // split stays auditable per iteration. + task_summaries: results.map((r) => ({ id: r.id, title: r.title, outcome: r.outcome, qual_tier: r.qual_tier || null })), concerns: [...results.flatMap((r) => r.concerns || []), ...e2eConcerns, ...guardConcerns], // synthBlock (a discard or iteration-level no-op verdict) outranks the // per-task block: when the discard guard broke the loop on a task that