diff --git a/agents/claude.ts b/agents/claude.ts index b0d6d07..10bc1cb 100644 --- a/agents/claude.ts +++ b/agents/claude.ts @@ -72,17 +72,27 @@ function writeMcpConfig(ctx: AgentRunContext): string { /** * Build the `--agents` JSON definition for the `reviewfrog` subagent. + * + * The Claude Code path always runs against an Anthropic model (see + * resolveAgent), so we hardcode the cheaper-sibling downshift: lenses run + * on Sonnet, the orchestrator stays on whatever model `--model` was passed. + * + * Per-call model override is also possible (Task tool's `model` arg accepts + * 'sonnet' | 'opus' | 'haiku') and takes precedence over what's set here — + * we don't pass it; the per-subagent `model` field is the right default. + * * The non-mutative + non-recursive contract is enforced by the prose system - * prompt baked into the agent — see action/agents/reviewer.ts for why we no - * longer wire per-agent `disallowedTools` here. + * prompt baked into the agent — see action/agents/reviewer.ts for why we + * no longer wire per-agent `disallowedTools` here. */ function buildAgentsJson(): string { const agents = { [REVIEWER_AGENT_NAME]: { description: - "Read-only review subagent for self-review and lens-based code review. " + + "Read-only review subagent for lens-based code review (correctness, security, billing-subsystem, etc.). " + "Reads only — no writes, no state-changing shell or MCP calls, no nested subagent dispatch.", prompt: REVIEWER_SYSTEM_PROMPT, + model: "claude-sonnet-4-6", }, }; return JSON.stringify(agents); diff --git a/agents/opencode.ts b/agents/opencode.ts index f6e3692..2c1a304 100644 --- a/agents/opencode.ts +++ b/agents/opencode.ts @@ -48,6 +48,7 @@ import { logTokenTable, MAX_STDERR_LINES, } from "./shared.ts"; +import { deriveSubagentModels } from "./subagentModels.ts"; async function installOpencodeCli(): Promise { return await installFromNpmTarball({ @@ -71,21 +72,25 @@ type OpenCodeConfig = { [key: string]: unknown; }; -/** - * Per-inference `max_tokens` reservation the agent sends to the upstream - * model. OpenCode's default is 32_000 (sized for long-running TUI sessions - * where a human user might want big outputs). Pullfrog runs are headless and - * short — typical outputs are 1-3K tokens — so we cap at 5_000. This - * drastically reduces the upfront budget reservation OpenRouter requires per - * call (~$0.38 vs ~$2.40 for Opus), which is what lets low-wallet runs - * actually start. - * - * Plumbed via `OPENCODE_EXPERIMENTAL_OUTPUT_TOKEN_MAX` env var rather than the - * config JSON. OpenCode's `OUTPUT_TOKEN_MAX` (session/llm.ts) is sourced - * exclusively from this env var; top-level `limit.output` in the config - * has no read site and is silently dropped on merge. - */ -const PULLFROG_OPENCODE_OUTPUT_LIMIT = 5000; +// NOTE: OpenCode's per-call `max_tokens` defaults to 32_000. We previously +// overrode this via `OPENCODE_EXPERIMENTAL_OUTPUT_TOKEN_MAX = 5000` in #616 +// to lower OpenRouter's per-call upfront budget reservation — back when the +// `ROUTER_PER_RUN_LIMIT_USD = 25` per-run key cap meant that reservation was +// a hard gate that could lock low-balance accounts out of starting a run. +// +// That gate is gone (see `app/api/proxy-token/route.ts` ~line 422 — "Per-run +// key budget … is decoupled from wallet balance"); the router now mints +// keys with `keyLimitCents = balance + buffer` ($50 / $5 / $0). The override +// no longer materially helps, and as a hard per-call output truncation it +// actively hurt: a single `create_pull_request_review` tool_use with many +// inline comments would truncate mid-stream past 5K output tokens, the JSON +// was unparseable, and the tool never invoked. We hit this on PR #710's +// verify-downshift PR. Removed in #710 — using OpenCode's 32K default. +// +// If you need to re-cap output for some reason, set +// `OPENCODE_EXPERIMENTAL_OUTPUT_TOKEN_MAX` in the action env. OpenCode's +// top-level `limit.output` config field has no read site (silently dropped +// on merge in session/llm.ts), so the env var is the only working knob. /** * upstream opencode hardcodes `thinkingLevel: "high"` as the default for every @@ -120,7 +125,12 @@ function buildSecurityConfig(ctx: AgentRunContext, model: string | undefined): s mcp: { [pullfrogMcpName]: { type: "remote", url: ctx.mcpServerUrl }, }, - agent: buildReviewerAgentConfig(), + agent: (() => { + const cfg = buildReviewerAgentConfig(model); + const reviewerModel = (cfg[REVIEWER_AGENT_NAME] as { model?: string })?.model ?? "(inherit)"; + log.info(`» subagent models: reviewfrog=${reviewerModel}`); + return cfg; + })(), // opt into opencode's experimental `batch` tool (added in // anomalyco/opencode PR #2983, opt-in via `experimental.batch_tool`). it // exposes a single `batch` tool that runs 1-25 independent tool calls @@ -159,19 +169,26 @@ function buildSecurityConfig(ctx: AgentRunContext, model: string | undefined): s } /** - * Read-only subagent for self-review and /anneal lens dispatch. The - * non-mutative + non-recursive contract is enforced by the prose system - * prompt — see action/agents/reviewer.ts for why we no longer wire per-agent - * tool/permission denies here. + * Read-only `reviewfrog` subagent for lens-based review. + * + * Non-mutative + non-recursive — enforced by the prose system prompt in + * reviewer.ts. + * + * Per-subagent `model:` override downshifts to a cheaper sibling when the + * orchestrator runs on Anthropic or OpenAI (see deriveSubagentModels). + * Other providers (xai, deepseek, gemini, etc.) inherit the orchestrator's + * model since their tier structure is less standard. */ -function buildReviewerAgentConfig(): Record { +function buildReviewerAgentConfig(orchestratorModel: string | undefined): Record { + const overrides = deriveSubagentModels(orchestratorModel); return { [REVIEWER_AGENT_NAME]: { description: - "Read-only review subagent for self-review and lens-based code review. " + + "Read-only review subagent for lens-based code review (correctness, security, billing-subsystem, etc.). " + "Reads only — no writes, no state-changing shell or MCP calls, no nested subagent dispatch.", mode: "subagent", prompt: REVIEWER_SYSTEM_PROMPT, + ...(overrides.reviewer !== undefined ? { model: overrides.reviewer } : {}), }, }; } @@ -1199,7 +1216,6 @@ export const opencode = agent({ ...homeEnv, OPENCODE_CONFIG_CONTENT: buildSecurityConfig(ctx, model), OPENCODE_PERMISSION: permissionOverride, - OPENCODE_EXPERIMENTAL_OUTPUT_TOKEN_MAX: PULLFROG_OPENCODE_OUTPUT_LIMIT.toString(), GOOGLE_GENERATIVE_AI_API_KEY: process.env.GOOGLE_GENERATIVE_AI_API_KEY || process.env.GEMINI_API_KEY, }; diff --git a/agents/subagentModels.test.ts b/agents/subagentModels.test.ts new file mode 100644 index 0000000..faf361d --- /dev/null +++ b/agents/subagentModels.test.ts @@ -0,0 +1,110 @@ +import { describe, expect, it } from "vitest"; +import { deriveSubagentModels } from "./subagentModels.ts"; + +describe("deriveSubagentModels", () => { + it("returns no override when orchestrator is undefined", () => { + expect(deriveSubagentModels(undefined)).toEqual({ reviewer: undefined }); + }); + + it("returns no override when orchestrator slug isn't registered", () => { + expect(deriveSubagentModels("nonexistent/model")).toEqual({ reviewer: undefined }); + }); + + describe("anthropic family — opus → sonnet", () => { + it("direct anthropic opus", () => { + expect(deriveSubagentModels("anthropic/claude-opus-4-7")).toEqual({ + reviewer: "anthropic/claude-sonnet-4-6", + }); + }); + it("opencode-vendored opus stays on opencode prefix", () => { + expect(deriveSubagentModels("opencode/claude-opus-4-7")).toEqual({ + reviewer: "opencode/claude-sonnet-4-6", + }); + }); + it("openrouter-anthropic-opus-via-anthropic-direct hits anthropic alias's openRouterResolve", () => { + // both the anthropic alias and the opencode alias have the same + // openRouterResolve. first-match-wins by alias declaration order + // (anthropic declared first in providers). + expect(deriveSubagentModels("openrouter/anthropic/claude-opus-4.7")).toEqual({ + reviewer: "openrouter/anthropic/claude-sonnet-4.6", + }); + }); + it("sonnet has no further downshift", () => { + expect(deriveSubagentModels("anthropic/claude-sonnet-4-6")).toEqual({ reviewer: undefined }); + expect(deriveSubagentModels("opencode/claude-sonnet-4-6")).toEqual({ reviewer: undefined }); + }); + it("haiku has no downshift", () => { + expect(deriveSubagentModels("anthropic/claude-haiku-4-5")).toEqual({ reviewer: undefined }); + }); + }); + + describe("openai family", () => { + it("gpt-pro → gpt (direct)", () => { + expect(deriveSubagentModels("openai/gpt-5.5-pro")).toEqual({ reviewer: "openai/gpt-5.5" }); + }); + it("gpt → gpt-5.4 (direct)", () => { + expect(deriveSubagentModels("openai/gpt-5.5")).toEqual({ reviewer: "openai/gpt-5.4" }); + }); + it("gpt → gpt-5.4 (opencode-vendored)", () => { + expect(deriveSubagentModels("opencode/gpt-5.5")).toEqual({ reviewer: "opencode/gpt-5.4" }); + }); + it("gpt-pro → gpt (openrouter)", () => { + expect(deriveSubagentModels("openrouter/openai/gpt-5.5-pro")).toEqual({ + reviewer: "openrouter/openai/gpt-5.5", + }); + }); + it("gpt → gpt-5.4 (openrouter)", () => { + expect(deriveSubagentModels("openrouter/openai/gpt-5.5")).toEqual({ + reviewer: "openrouter/openai/gpt-5.4", + }); + }); + it("gpt-5.4 itself (the hidden subagent target) has no further downshift", () => { + expect(deriveSubagentModels("openai/gpt-5.4")).toEqual({ reviewer: undefined }); + }); + it("gpt-mini has no downshift", () => { + expect(deriveSubagentModels("openai/gpt-5.4-mini")).toEqual({ reviewer: undefined }); + }); + }); + + describe("google (gemini) — pro → flash", () => { + it("direct google", () => { + expect(deriveSubagentModels("google/gemini-3.1-pro-preview")).toEqual({ + reviewer: "google/gemini-3-flash-preview", + }); + }); + it("opencode-vendored gemini-pro", () => { + expect(deriveSubagentModels("opencode/gemini-3.1-pro")).toEqual({ + reviewer: "opencode/gemini-3-flash", + }); + }); + it("openrouter-google-gemini-pro", () => { + expect(deriveSubagentModels("openrouter/google/gemini-3.1-pro-preview")).toEqual({ + reviewer: "openrouter/google/gemini-3-flash-preview", + }); + }); + it("flash has no downshift", () => { + expect(deriveSubagentModels("google/gemini-3-flash-preview")).toEqual({ + reviewer: undefined, + }); + }); + }); + + describe("providers / models without a subagentModel — inherit", () => { + it("xai grok (already cheap flagship)", () => { + expect(deriveSubagentModels("xai/grok-4.3")).toEqual({ reviewer: undefined }); + }); + it("deepseek", () => { + expect(deriveSubagentModels("deepseek/deepseek-v4-pro")).toEqual({ reviewer: undefined }); + }); + it("moonshot kimi", () => { + expect(deriveSubagentModels("moonshotai/kimi-k2.6")).toEqual({ reviewer: undefined }); + }); + it("opencode big-pickle", () => { + expect(deriveSubagentModels("opencode/big-pickle")).toEqual({ reviewer: undefined }); + }); + it("legacy fallback aliases (gpt-codex, deepseek-reasoner)", () => { + expect(deriveSubagentModels("openai/gpt-5.3-codex")).toEqual({ reviewer: undefined }); + expect(deriveSubagentModels("deepseek/deepseek-reasoner")).toEqual({ reviewer: undefined }); + }); + }); +}); diff --git a/agents/subagentModels.ts b/agents/subagentModels.ts new file mode 100644 index 0000000..163f4e9 --- /dev/null +++ b/agents/subagentModels.ts @@ -0,0 +1,40 @@ +import { modelAliases } from "../models.ts"; + +/** + * Derive a cheaper subagent model override from the orchestrator's resolved + * model spec. + * + * This is a pure registry lookup: every alias in `action/models.ts` declares + * its own `subagentModel` (alias key in the same provider). At runtime we + * reverse-lookup the orchestrator's resolved slug to find the alias that + * produced it, follow the `subagentModel` pointer, and return the target + * alias's resolve / openRouterResolve depending on which route the + * orchestrator was using. + * + * Returns `{ reviewer: undefined }` when the orchestrator's alias has no + * `subagentModel` (e.g. it's already at a sufficiently cheap tier, or its + * provider doesn't have a clean cheaper-but-capable sibling). See models.ts + * for the wiring + per-provider rationale. + */ +export function deriveSubagentModels(orchestratorSpec: string | undefined): { + reviewer: string | undefined; +} { + if (!orchestratorSpec) return { reviewer: undefined }; + + // Reverse-lookup. The same resolve string appears in only one alias + // (within its provider), so first match wins. We track which field + // matched (resolve vs openRouterResolve) so we can pick the same field + // off the subagent target — keeping the orchestrator's route consistent. + for (const source of modelAliases) { + const matchedDirect = source.resolve === orchestratorSpec; + const matchedOR = source.openRouterResolve === orchestratorSpec; + if (!matchedDirect && !matchedOR) continue; + if (!source.subagentModel) return { reviewer: undefined }; + const target = modelAliases.find((a) => a.slug === source.subagentModel); + if (!target) return { reviewer: undefined }; + const reviewer = matchedOR ? target.openRouterResolve : target.resolve; + return { reviewer }; + } + + return { reviewer: undefined }; +} diff --git a/agents/subagentRegistration.test.ts b/agents/subagentRegistration.test.ts new file mode 100644 index 0000000..18c5348 --- /dev/null +++ b/agents/subagentRegistration.test.ts @@ -0,0 +1,40 @@ +import { readFileSync } from "node:fs"; +import { join } from "node:path"; +import { describe, expect, it } from "vitest"; + +const claudeSource = readFileSync(join(__dirname, "claude.ts"), "utf-8"); +const opencodeSource = readFileSync(join(__dirname, "opencode.ts"), "utf-8"); + +/** + * The Claude Code `--agents` JSON and OpenCode `agent` config block are the + * only places where per-subagent model overrides take effect. They're built + * by string-only helpers we don't export, so this test reads the source and + * asserts the literal model strings + agent names are wired in. A regression + * here means the next review run silently runs lenses on Opus instead of + * Sonnet. + */ +describe("subagent registration source asserts", () => { + describe("claude.ts buildAgentsJson", () => { + it("registers reviewfrog with sonnet model", () => { + expect(claudeSource).toMatch( + /\[REVIEWER_AGENT_NAME\]:\s*\{[^}]*model:\s*"claude-sonnet-4-6"/s + ); + }); + it("imports the reviewer name constant", () => { + expect(claudeSource).toMatch(/REVIEWER_AGENT_NAME/); + }); + }); + + describe("opencode.ts buildReviewerAgentConfig", () => { + it("registers reviewfrog with mode: subagent", () => { + expect(opencodeSource).toMatch(/\[REVIEWER_AGENT_NAME\]:[^}]*mode:\s*"subagent"/s); + }); + it("uses deriveSubagentModels for the reviewer model override", () => { + expect(opencodeSource).toMatch(/deriveSubagentModels\(/); + expect(opencodeSource).toMatch(/overrides\.reviewer/); + }); + it("passes orchestrator model to buildReviewerAgentConfig", () => { + expect(opencodeSource).toMatch(/buildReviewerAgentConfig\(model\)/); + }); + }); +}); diff --git a/commands/init.ts b/commands/init.ts index fd3ad0d..b46e3ea 100644 --- a/commands/init.ts +++ b/commands/init.ts @@ -24,7 +24,7 @@ function buildProviders(): CliProvider[] { return Object.entries(providers) .filter(([key]) => key !== "opencode" && key !== "openrouter") .map(([key, config]: [string, ProviderConfig]) => { - const aliases = modelAliases.filter((a) => a.provider === key && !a.fallback); + const aliases = modelAliases.filter((a) => a.provider === key && !a.fallback && !a.hidden); const recommended = aliases.find((a) => a.preferred); const sorted = [...aliases].sort((a, b) => { if (a.preferred && !b.preferred) return -1; diff --git a/models.ts b/models.ts index 0a549ea..ae578f4 100644 --- a/models.ts +++ b/models.ts @@ -24,6 +24,13 @@ export interface ModelAlias { isFree: boolean; /** slug of a replacement model — presence implies this model is deprecated */ fallback: string | undefined; + /** alias key (within same provider) of the cheaper sibling reviewfrog should + * use as its lens-fanout subagent. e.g. claude-opus → "claude-sonnet". */ + subagentModel: string | undefined; + /** hide from selectable lists (UI dropdowns, CLI pickers). does NOT affect + * resolution — for that use `fallback`. used for internal-only tier targets + * (e.g. gpt-5.4 as a subagent target without exposing it to users). */ + hidden: boolean; } interface ModelDef { @@ -37,6 +44,11 @@ interface ModelDef { isFree?: boolean; /** slug of a replacement model — presence implies this model is deprecated */ fallback?: string; + /** alias key (within same provider) of the cheaper sibling reviewfrog should + * use as its lens-fanout subagent (e.g. claude-opus → "claude-sonnet"). */ + subagentModel?: string; + /** hide from selectable lists. does NOT affect resolution; for that use `fallback`. */ + hidden?: boolean; } export interface ProviderConfig { @@ -61,6 +73,7 @@ export const providers = { resolve: "anthropic/claude-opus-4-7", openRouterResolve: "openrouter/anthropic/claude-opus-4.7", preferred: true, + subagentModel: "claude-sonnet", }, "claude-sonnet": { displayName: "Claude Sonnet", @@ -83,11 +96,22 @@ export const providers = { resolve: "openai/gpt-5.5", openRouterResolve: "openrouter/openai/gpt-5.5", preferred: true, + subagentModel: "gpt-5.4", }, "gpt-pro": { displayName: "GPT Pro", resolve: "openai/gpt-5.5-pro", openRouterResolve: "openrouter/openai/gpt-5.5-pro", + subagentModel: "gpt", + }, + // hidden subagent target — `gpt` lenses run against this. surfacing + // it in the picker would just confuse users (it's the prior-flagship, + // and they already have `gpt` and `gpt-mini` to choose from). + "gpt-5.4": { + displayName: "GPT 5.4", + resolve: "openai/gpt-5.4", + openRouterResolve: "openrouter/openai/gpt-5.4", + hidden: true, }, "gpt-mini": { displayName: "GPT Mini", @@ -126,6 +150,7 @@ export const providers = { resolve: "google/gemini-3.1-pro-preview", openRouterResolve: "openrouter/google/gemini-3.1-pro-preview", preferred: true, + subagentModel: "gemini-flash", }, "gemini-flash": { displayName: "Gemini Flash", @@ -214,6 +239,7 @@ export const providers = { displayName: "Claude Opus", resolve: "opencode/claude-opus-4-7", openRouterResolve: "openrouter/anthropic/claude-opus-4.7", + subagentModel: "claude-sonnet", }, "claude-sonnet": { displayName: "Claude Sonnet", @@ -229,11 +255,20 @@ export const providers = { displayName: "GPT", resolve: "opencode/gpt-5.5", openRouterResolve: "openrouter/openai/gpt-5.5", + subagentModel: "gpt-5.4", }, "gpt-pro": { displayName: "GPT Pro", resolve: "opencode/gpt-5.5-pro", openRouterResolve: "openrouter/openai/gpt-5.5-pro", + subagentModel: "gpt", + }, + // hidden subagent target — see openai provider above for context. + "gpt-5.4": { + displayName: "GPT 5.4", + resolve: "opencode/gpt-5.4", + openRouterResolve: "openrouter/openai/gpt-5.4", + hidden: true, }, "gpt-mini": { displayName: "GPT Mini", @@ -257,6 +292,7 @@ export const providers = { displayName: "Gemini Pro", resolve: "opencode/gemini-3.1-pro", openRouterResolve: "openrouter/google/gemini-3.1-pro-preview", + subagentModel: "gemini-flash", }, "gemini-flash": { displayName: "Gemini Flash", @@ -297,6 +333,7 @@ export const providers = { resolve: "openrouter/anthropic/claude-opus-4.7", openRouterResolve: "openrouter/anthropic/claude-opus-4.7", preferred: true, + subagentModel: "claude-sonnet", }, "claude-sonnet": { displayName: "Claude Sonnet", @@ -312,11 +349,20 @@ export const providers = { displayName: "GPT", resolve: "openrouter/openai/gpt-5.5", openRouterResolve: "openrouter/openai/gpt-5.5", + subagentModel: "gpt-5.4", }, "gpt-pro": { displayName: "GPT Pro", resolve: "openrouter/openai/gpt-5.5-pro", openRouterResolve: "openrouter/openai/gpt-5.5-pro", + subagentModel: "gpt", + }, + // hidden subagent target — see openai provider above for context. + "gpt-5.4": { + displayName: "GPT 5.4", + resolve: "openrouter/openai/gpt-5.4", + openRouterResolve: "openrouter/openai/gpt-5.4", + hidden: true, }, "gpt-mini": { displayName: "GPT Mini", @@ -345,6 +391,7 @@ export const providers = { displayName: "Gemini Pro", resolve: "openrouter/google/gemini-3.1-pro-preview", openRouterResolve: "openrouter/google/gemini-3.1-pro-preview", + subagentModel: "gemini-flash", }, "gemini-flash": { displayName: "Gemini Flash", @@ -432,6 +479,11 @@ export const modelAliases: ModelAlias[] = Object.entries(providers).flatMap( preferred: def.preferred ?? false, isFree: def.isFree ?? false, fallback: def.fallback, + // subagentModel is stored as an alias key local to the provider; expand + // here to a fully-qualified slug so callers can look up the target alias + // directly without re-deriving the provider. + subagentModel: def.subagentModel ? `${providerKey}/${def.subagentModel}` : undefined, + hidden: def.hidden ?? false, })) ); @@ -451,7 +503,7 @@ const MAX_FALLBACK_DEPTH = 10; * use this in UI display sites (dropdown trigger labels, PR-comment footers, * etc.) so a deprecated stored slug renders as the model the user actually * runs against — not the historical name. selectable lists should still hide - * deprecated aliases by filtering on `!a.fallback`. + * deprecated and internal-only aliases by filtering on `!a.fallback && !a.hidden`. */ export function resolveDisplayAlias(slug: string): ModelAlias | undefined { let current = slug; diff --git a/modes.ts b/modes.ts index 7cec086..eefa369 100644 --- a/modes.ts +++ b/modes.ts @@ -155,18 +155,24 @@ For simple, well-defined tasks, skip the plan phase and go straight to build.`, - resolve addressed threads via \`${t("resolve_review_thread")}\` - call \`${t("report_progress")}\` with a brief summary (or the exact push error if push failed)`, }, - // Review and IncrementalReview use the multi-lens orchestrator pattern - // (canonical source: .claude/commands/anneal.md). The orchestrator does - // triage → parallel read-only subagent fan-out → aggregate → draft comments - // → submit. For someone else's PR, parallel lenses (correctness, security, - // research-validated claims, user-journey, etc.) provide breadth across - // angles that a single subagent can't carry coherently. Build mode keeps - // a single fresh-eyes subagent (different problem shape — orchestrator - // wrote the code and bias-mitigation comes from delegating to one - // subagent that doesn't share the implementation context). - // Deliberate omission vs canonical /anneal: severity categorization in the - // final message (the review body has its own CAUTION/IMPORTANT framing - // instead of a severity table). + // Review and IncrementalReview use a 0-or-2+ lens pattern. The default is + // 0 lenses (orchestrator handles the review solo). Multi-lens (2+ + // reviewfrog subagents in parallel) only fires for substantive PRs or + // high-stakes-subsystem touches — and when it fires, ALL lenses must + // dispatch in a single assistant turn or the parallelism win disappears. + // We never dispatch exactly one lens: a single lens is just a worse, + // slower version of doing the work yourself. + // + // Build mode self-review is a different problem shape: the orchestrator + // wrote the code, so bias-mitigation comes from delegating to one + // fresh-eyes subagent that doesn't share the implementation context. A + // single subagent there is appropriate; the 0-or-2+ rule applies only to + // the Review/IncrementalReview lens fan-out where independence between + // perspectives is what's being purchased. + // + // Deliberate omission vs canonical /anneal: severity categorization in + // the final message (the review body has its own CAUTION/IMPORTANT + // framing instead of a severity table). { name: "Review", description: @@ -177,9 +183,9 @@ For simple, well-defined tasks, skip the plan phase and go straight to build.`, 2. **checkout**: call \`${t("checkout_pr")}\` — this returns PR metadata and a \`diffPath\`. read the diff TOC end-to-end and treat its file line ranges as your coverage checklist. -3. **triage**: orient yourself on the PR — identify *what kind of thing this is* (domain it touches, seams it crosses, external contracts it depends on, user-facing surfaces it changes). orientation only — defer specific defect-hunting to the subagents; pre-reviewing biases the lenses you pick. use \`${t("get_pull_request")}\` and other read-only GitHub tools for additional context if needed. +3. **triage**: orient yourself on the PR — identify *what kind of thing this is* (domain it touches, seams it crosses, external contracts it depends on, user-facing surfaces it changes). pull as much context as you need to render a confident, well-grounded review: read related files, grep for callers of changed symbols, check tests that exercise the touched paths, fetch related GitHub state. **you are the synthesizer** — never delegate understanding to subagents. - if the PR is **genuinely trivial**, skip steps 4–5 entirely and submit a \`No new issues found.\` review per step 6. there's no value in dispatching even one lens for a typo. + if the PR is **genuinely trivial**, skip the fan-out entirely and submit a \`No new issues found.\` review per step 7. "Genuinely trivial" (skip): - single-word doc typo, whitespace/format-only, comment-only across any number of files @@ -198,25 +204,25 @@ For simple, well-defined tasks, skip the plan phase and go straight to build.`, - any "typo fix" in user-facing copy that changes meaning ("approved" → "denied") - mixed diffs where a semantic 1-liner is buried in whitespace/formatting changes - When unsure, treat as non-trivial. The cost of one extra subagent is cents; the cost of a missed billing/auth/data bug is much more. +4. **lens decision — 0 or 2+, NEVER 1**. - otherwise pick lenses by where the PR concentrates risk — **there's no fixed count**. lens count is judgment, not a formula. concrete shapes to anchor against: + The default is **0 lenses**: handle the review yourself end-to-end. Most PRs land here. - - **1 lens** — pure refactor / mechanical rename across many files (impact); new test file with no source change (test-integrity); small isolated bug fix (correctness); doc-only PR with non-trivial technical content (research-validated or holistic) - - **2–3 lenses (most PRs land here)** — new CRUD endpoint (correctness + security + test-integrity); new UI flow (user-journey + correctness); a single bug fix in a non-critical subsystem (correctness + test-integrity); design doc covering one domain (research-validated + correctness or holistic) - - **4–5 lenses (high-stakes subsystem touches)** — any billing/payments change (billing-subsystem + correctness + security + operational-readiness); new auth flow (auth-subsystem + correctness + security + test-integrity); schema migration (schema-migration-subsystem + correctness + operational-readiness + impact); cross-subsystem PR that touches billing AND auth AND schema (one subsystem lens per domain + correctness) - - **6+ lenses** — almost always a smell; you're either covering overlapping ground or this PR should have been split. push back via the review body rather than expanding lens count. + Dispatch **2+ \`${REVIEWER_AGENT_NAME}\` lenses in parallel** ONLY when ALL of the following are true: + - the PR is substantive (>5 files changed AND >200 net lines), OR touches a high-stakes subsystem (auth, billing, payments, schema migration, webhooks, secrets, RBAC, multi-tenant isolation, cron/scheduling) + - you can name 2+ distinct concrete failure modes that warrant independent lenses (one lens per failure mode; orthogonal, not overlapping) + - parallel-orchestrated independent perspectives meaningfully outperform what you'd find solo - **lens-add discipline.** Each lens needs to clear a specific bar before you dispatch it: name the concrete failure mode this lens would catch *that the diff plausibly introduces*, in one sentence. "Could apply", "good to have", "for completeness" do not qualify. If you can't name what the lens is going to find, drop it. The "when unsure, treat as non-trivial" rule above is for the trivial-vs-non-trivial gate at step 3 — it does not license expanding lens count without articulated risk. Every extra lens adds wall-time, log noise, and pulls subagent attention onto speculative angles, which biases the final review toward bloat-shaped findings. + **NEVER dispatch exactly one lens.** A single lens is just a more expensive version of doing the work yourself with a worse model — it adds wall time and a context-handoff for no orthogonality benefit. Either you have at least two genuinely independent failure-mode hypotheses (dispatch all in one turn), or you don't (do the review yourself). - lenses come in two flavors, and you can mix them: + When you do go multi-lens, lens framings come in two flavors: - **themed lenses** — a perspective applied across the whole diff (correctness, security, user-journey, performance, etc.). - - **subsystem lenses** — a domain-scoped frame for high-stakes subsystems the PR touches (e.g. "the auth lens", "the billing lens", "the schema-migration lens"). a subsystem lens is "review the PR specifically for what could go wrong in this subsystem" and naturally combines theme + scope. **for high-stakes domains, lead with the subsystem lens rather than the generic themed equivalent** — "billing-subsystem" outperforms "correctness on billing code" because the framing primes the subagent to remember domain-specific failure modes (double-charges, refund races, currency rounding, dispute flows) the generic lens misses. + - **subsystem lenses** — a domain-scoped frame for high-stakes subsystems the PR touches (e.g. "the auth lens", "the billing lens", "the schema-migration lens"). **for high-stakes domains, lead with the subsystem lens rather than the generic themed equivalent** — "billing-subsystem" outperforms "correctness on billing code" because the framing primes the subagent to remember domain-specific failure modes (double-charges, refund races, currency rounding, dispute flows) the generic lens misses. starter menu (combine, omit, or invent your own): - **correctness & invariants** — bugs, races, error handling, edge cases, state-machine boundaries - - **impact** — when the PR removes features, deletes exports, renames identifiers, or changes architectural patterns: stale references in code, tests, docs (\`docs/\`, \`wiki/\`), comments, configs, UI - - **research-validated assumptions** — third-party API contracts, SDK semantics, framework directives, version-gated behavior. **only pick when the PR's correctness depends on the contract behaving a specific way** — not when the API is merely used. An idempotency key as a backstop, a timeout as a hint, a retry as belt-and-suspenders: not load-bearing, skip this lens. The bar is "if the third-party contract differs from what the diff assumes, the PR is incorrect." When dispatched, the subagent must verify load-bearing claims via web search and quote source URLs. + - **impact** — stale references in code/tests/docs/configs/UI after rename/remove + - **research-validated assumptions** — third-party API contracts, SDK semantics, framework directives, version-gated behavior. **only pick when the PR's correctness depends on the contract behaving a specific way** — not when the API is merely used. The bar is "if the third-party contract differs from what the diff assumes, the PR is incorrect." When dispatched, the subagent must verify load-bearing claims via web search and quote source URLs. - **security** — new endpoints, authZ, input validation, secrets handling, replay/CSRF/injection, cross-tenant isolation - **user-journey** — UX-touching flows: walk through happy path and failure modes as a user - **operational readiness** — observability, alerting, migrations (forward + rollback), feature flags, on-call burden @@ -226,26 +232,36 @@ For simple, well-defined tasks, skip the plan phase and go straight to build.`, - **holistic** — does the PR make sense as a whole? symmetric flows (delete for every create, rollback for every migration)? - **subsystem lenses** (invent as the PR demands) — auth, billing, payments, schema migration, webhooks, secrets, RBAC, multi-tenant isolation, cron/scheduling, etc. -4. **fan out**: dispatch one \`${REVIEWER_AGENT_NAME}\` subagent per lens — its baked-in system prompt enforces the non-mutative + non-recursive contract (read-only file/search/web tools and read-only MCP queries; no writes, shell side effects, state-changing MCP calls, or nested subagent dispatch). when picking 2+ lenses, dispatch them in a **single assistant turn with multiple parallel subagent calls**; issuing one and awaiting reply before the next collapses the fan-out into a serial review. if a subagent errors out, times out, or returns nothing usable, retry once with the same lens; if it still fails, proceed with partial coverage and note the missing lens in the review body — do not skip step 4 entirely on a single subagent failure. each subagent gets: + The only subagent type is \`${REVIEWER_AGENT_NAME}\` — used for lens judgment work ("is this safe / correct / well-tested?"), runs on a mid-tier model. + +5. **fan out (only if step 4 said 2+ lenses)**: dispatch every \`${REVIEWER_AGENT_NAME}\` subagent for this run **IN A SINGLE ASSISTANT TURN, AS MULTIPLE PARALLEL TASK TOOL_USE BLOCKS IN ONE MESSAGE.** + + ⚠️ CRITICAL — PARALLELISM IS THE ONLY REASON LENSES EXIST. ⚠️ + The default tool-call behavior of Claude Code (and most agent runtimes) is **serial dispatch**: emit one Task call, await result, emit next, await, etc. This collapses your fan-out into a sequential review where each lens adds N × (orchestrator-think-time + lens-execution-time) to wall time. **YOU MUST OVERRIDE THIS DEFAULT.** Emit ALL of your Task tool_use blocks in the SAME assistant message, BEFORE you read ANY result from ANY of them. If you find yourself emitting one Task call, then thinking about the result, then emitting another — STOP and re-issue them all together. The whole point of going multi-lens is the wall-clock speedup from parallel execution; serial dispatch defeats it entirely. + + ✅ Right pattern: one assistant turn with N Task tool_use blocks → wait → N results arrive together → aggregate. + ❌ Wrong pattern: turn 1 = Task(lens A) → turn 2 (after A's result) = Task(lens B) → turn 3 (after B's result) = Task(lens C). This is the failure mode. Do not do this. + + You can also include your own \`read\` / \`grep\` / \`webfetch\` calls in the SAME turn as the parallel \`${REVIEWER_AGENT_NAME}\` dispatches — concurrent context-pulling on the orchestrator side runs in parallel with the lens fan-out and costs zero extra wall time. + + if a subagent errors out, times out, or returns nothing usable, retry once with the same lens; if it still fails, proceed with partial coverage and note the missing lens in the review body — do not skip the fan-out entirely on a single subagent failure. each subagent gets: - the diff path / target — reading the diff and the codebase is its job - **only one lens** — never a multi-section "review for X, Y, and Z" prompt - **a Task \`description\` set to the lens name** (e.g. \`"security"\`, \`"correctness"\`, \`"billing-subsystem"\`) — the harness reads this field to label the subagent's log lines so parallel runs can be told apart in CI output. without it, every subagent shows up as \`subagent#N\`. - - the read-only contract restated in your dispatch instructions so the rule is present twice (the subagent's system prompt also enforces it). The test: would this call still be a no-op if reverted? If not (PR comments, branch pushes, issue updates, set_output, label changes, dependency installs, etc.), don't make it. - if the lens touches external contracts, instruct the subagent to verify load-bearing claims via web search rather than trust training data, and to quote source URLs in its reasoning. action runs are non-interactive — there's no human in the loop to catch "I'm pretty sure Stripe does X." - ask the subagent to report findings with file paths and NEW line numbers from the diff so you can anchor inline comments without re-reading the entire diff. delegation discipline: - - do NOT lens-review the diff yourself in parallel with the subagents (your job is dispatch + comment-drafting; doing the lens work yourself reintroduces the bias the fan-out avoids) - do NOT summarize the PR for them (biases toward a validation frame) - do NOT hand them a curated reading list (let them discover scope) - do NOT pre-shape their output with a finding schema - do NOT mention the other lenses (independence is the point — overlapping findings are a strong signal) -5. **aggregate & draft**: merge findings; de-dup overlaps (two lenses catching the same issue = higher-confidence signal); trace each finding yourself before accepting it. drop praise, style preferences, speculative/unverified claims, findings about pre-existing code unrelated to the PR (heuristic: if the finding's root cause lives in lines this PR added or modified, it's in scope; otherwise drop unless the PR plausibly introduced or amplified the regression), and anything not actionable. also drop **bloat-shaped findings** — proposed fixes that would add defensive checks for cases that can't happen, abstractions used once, comments restating obvious code, tests asserting tautologies, or "just-in-case" guards. subagents are fallible and bias toward recommending changes; the bar for an actionable inline comment is sound + correct + elegant. recommending a change that improves only one of the three (or worse, degrades elegance to nominally improve correctness) makes the codebase worse, not better. +6. **aggregate & draft**: when the fan-out lands, merge findings; de-dup overlaps (two lenses catching the same issue = higher-confidence signal); trace each finding yourself before accepting it. drop praise, style preferences, speculative/unverified claims, findings about pre-existing code unrelated to the PR (heuristic: if the finding's root cause lives in lines this PR added or modified, it's in scope; otherwise drop unless the PR plausibly introduced or amplified the regression), and anything not actionable. also drop **bloat-shaped findings** — proposed fixes that would add defensive checks for cases that can't happen, abstractions used once, comments restating obvious code, tests asserting tautologies, or "just-in-case" guards. subagents are fallible and bias toward recommending changes; the bar for an actionable inline comment is sound + correct + elegant. recommending a change that improves only one of the three (or worse, degrades elegance to nominally improve correctness) makes the codebase worse, not better. for surviving findings, draft inline comments with NEW line numbers from the diff. every comment must be actionable, 2-3 sentences max. use GitHub permalink format for code references. for impact-analysis findings (stale references after rename/remove), report them in the review body ordered by severity (runtime breakage > incorrect docs > stale comments) rather than as inline comments unless they're anchored to a specific line. -6. **submit**: ALWAYS submit exactly one review via \`${t("create_pull_request_review")}\`. Do NOT call \`report_progress\` — the review is the final record and the progress comment will be cleaned up automatically. +7. **submit**: ALWAYS submit exactly one review via \`${t("create_pull_request_review")}\`. Do NOT call \`report_progress\` — the review is the final record and the progress comment will be cleaned up automatically. note: the first create_pull_request_review submission may error with a one-time diff-coverage nudge listing unread TOC regions. retry the same call to proceed — optionally after reading the listed ranges. the pre-flight will not block again this session. @@ -273,10 +289,10 @@ For simple, well-defined tasks, skip the plan phase and go straight to build.`, ${PR_SUMMARY_FORMAT}`, }, - // IncrementalReview shares Review's multi-lens orchestrator pattern but - // scopes the target to the incremental diff. The "issues must be NEW - // since the last Pullfrog review" filter lives at aggregation time - // (step 6), NOT in the subagent prompt — pushing the filter into + // IncrementalReview shares Review's 0-or-2+ lens pattern but scopes the + // target to the incremental diff. The "issues must be NEW since the last + // Pullfrog review" filter lives at aggregation time (step 8), NOT in the + // subagent prompt — pushing the filter into // subagents matches the canonical anneal anti-pattern of "list known // pre-existing failures — don't flag these" and suppresses signal on // regressions the new commits amplified. The review body is just @@ -296,38 +312,57 @@ ${PR_SUMMARY_FORMAT}`, 3. **incremental scope**: if \`incrementalDiffPath\` is present, read it to see what changed since the last review. this is a range-diff that isolates the net changes, filtering out base branch noise. if not present, fall back to reviewing the full PR diff and determine what changed since Pullfrog's most recent review. -4. **prior feedback**: fetch previous reviews via \`${t("list_pull_request_reviews")}\`. for the most recent Pullfrog review, call \`${t("get_review_comments")}\` with the review ID to retrieve specific prior line-level feedback. you'll use this to filter your aggregation in step 6 — anything already flagged in a prior review and not changed by the new commits should not be re-raised. you do NOT need to render this in the review body; the rolling PR summary snapshot is the durable record of what's been addressed. +4. **prior feedback**: fetch previous reviews via \`${t("list_pull_request_reviews")}\`. for the most recent Pullfrog review, call \`${t("get_review_comments")}\` with the review ID to retrieve specific prior line-level feedback. you'll use this to filter your aggregation in step 8 — anything already flagged in a prior review and not changed by the new commits should not be re-raised. you do NOT need to render this in the review body; the rolling PR summary snapshot is the durable record of what's been addressed. -5. **triage & fan out**: orient on the *incremental* changes — domain, seams, external contracts, user-facing surfaces. +5. **triage**: orient on the *incremental* changes — domain, seams, external contracts, user-facing surfaces. pull as much context as you need to render a confident review: read related files, grep for callers of changed symbols, check tests that exercise the touched paths. **you are the synthesizer.** - if the incremental changes are **genuinely trivial**, skip the fan-out entirely and jump to step 8's non-substantive path (do NOT submit a review). + if the incremental changes are **genuinely trivial**, skip the fan-out entirely and jump to step 10's non-substantive path (do NOT submit a review). "Genuinely trivial" (skip): formatting/comment tweaks, import reordering, lockfile regen, mechanical rename of import paths, whitespace-only. "Looks trivial but isn't" (do NOT skip — same anti-patterns as Review mode): 1-line changes to SQL/regex/auth/billing/permissions/signature-verification code; flipping feature-flag defaults or retry/timeout constants; money/tax/HTTP-method/redirect changes; tightening or loosening a comparison operator; mixed diffs with a semantic line buried in formatting. When unsure, treat as non-trivial. - otherwise pick lenses by where the new commits concentrate risk — **there's no fixed count**, same calibration as Review mode (1 lens for pure refactor / isolated fix; 2–3 for typical features; 4–5 for high-stakes subsystem touches; 6+ is a smell). same **lens-add discipline** as Review mode applies: each lens needs to name the concrete failure mode it would catch *that the new commits plausibly introduce* — "could apply" doesn't qualify, drop it. **research-validated assumptions** specifically: only pick when the new commits' correctness depends on a third-party contract behaving a specific way; merely using an API doesn't qualify. lens framing follows Review mode: themed lenses (correctness & invariants, impact when new commits remove/rename/deprecate things, research-validated assumptions, security, user-journey, operational readiness, integration & cross-cutting, test integrity, performance, holistic) and subsystem lenses (auth, billing, schema migration, etc.) — for high-stakes domains lead with the subsystem lens rather than the generic themed equivalent. +6. **lens decision — 0 or 2+, NEVER 1**. - dispatch one \`${REVIEWER_AGENT_NAME}\` subagent per lens — its baked-in system prompt enforces the non-mutative + non-recursive contract (read-only file/search/web tools and read-only MCP queries; no writes, shell side effects, state-changing MCP calls, or nested subagent dispatch). dispatch them in a **single assistant turn with multiple parallel subagent calls** (serial dispatch collapses the fan-out). if a subagent errors out, times out, or returns nothing usable, retry once with the same lens; if it still fails, proceed with partial coverage and note the missing lens in the review body — do not skip step 5 entirely on a single subagent failure. each subagent gets: - - the diff scope (incremental diff path if available, full diff otherwise). do NOT tell them to skip pre-existing issues — that suppresses regressions the new commits amplified; the "issues must be NEW" filter lives at aggregation time (step 6), not in the subagent prompt + The default is **0 lenses**: handle the re-review yourself end-to-end. Most incremental reviews land here — especially thread-reply re-reviews where the user is asking "did you address X?" rather than "review the diff again." + + Dispatch **2+ \`${REVIEWER_AGENT_NAME}\` lenses in parallel** ONLY when ALL of the following are true: + - the incremental changes are substantive (>5 files changed AND >200 net new lines), OR touch a high-stakes subsystem (auth, billing, payments, schema migration, webhooks, secrets, RBAC, multi-tenant isolation, cron/scheduling) + - you can name 2+ distinct concrete failure modes the new commits plausibly introduce that warrant independent lenses + - parallel-orchestrated independent perspectives meaningfully outperform what you'd find solo + + **NEVER dispatch exactly one lens.** Single-lens dispatch adds wall time and cost for no orthogonality benefit. Either go multi-lens (≥2 in parallel) or do the re-review yourself. + + Lens framing follows Review mode: themed lenses (correctness, security, etc.) and subsystem lenses (auth, billing, schema-migration, etc.) — for high-stakes domains lead with the subsystem lens. + +7. **fan out (only if step 6 said 2+ lenses)**: dispatch every \`${REVIEWER_AGENT_NAME}\` subagent for this run **IN A SINGLE ASSISTANT TURN, AS MULTIPLE PARALLEL TASK TOOL_USE BLOCKS IN ONE MESSAGE.** + + ⚠️ CRITICAL — PARALLELISM IS THE ONLY REASON LENSES EXIST. ⚠️ + Default tool-call behavior is **serial dispatch**: emit one Task call, await result, emit next, await, etc. This collapses your fan-out into a sequential review where each lens adds N × (orchestrator-think-time + lens-execution-time) to wall time. **YOU MUST OVERRIDE THIS DEFAULT.** Emit ALL of your Task tool_use blocks in the SAME assistant message, BEFORE you read ANY result from ANY of them. + + ✅ Right pattern: one assistant turn with N Task tool_use blocks → wait → N results arrive together → aggregate. + ❌ Wrong pattern: turn 1 = Task(lens A) → turn 2 (after A's result) = Task(lens B). This is the failure mode. + + You can also include your own \`read\` / \`grep\` / \`webfetch\` calls in the SAME turn as the parallel \`${REVIEWER_AGENT_NAME}\` dispatches. + + if a subagent errors out, times out, or returns nothing usable, retry once with the same lens; if it still fails, proceed with partial coverage and note the missing lens in the review body. each subagent gets: + - the diff scope (incremental diff path if available, full diff otherwise). do NOT tell them to skip pre-existing issues — that suppresses regressions the new commits amplified; the "issues must be NEW" filter lives at aggregation time (step 8), not in the subagent prompt - **only one lens** — never a multi-section "review for X, Y, and Z" prompt - - **a Task \`description\` set to the lens name** (e.g. \`"security"\`, \`"correctness"\`, \`"billing-subsystem"\`) — the harness reads this field to label the subagent's log lines so parallel runs can be told apart in CI output. without it, every subagent shows up as \`subagent#N\`. - - the read-only contract restated in your dispatch instructions so the rule is present twice (the subagent's system prompt also enforces it). The test: would this call still be a no-op if reverted? If not (PR comments, branch pushes, issue updates, set_output, label changes, dependency installs, etc.), don't make it. - - if the lens touches external contracts, instruct the subagent to verify load-bearing claims via web search and quote source URLs. action runs are non-interactive — there's no human to catch "I'm pretty sure Stripe does X." + - **a Task \`description\` set to the lens name** — the harness reads this field to label log lines so parallel runs can be told apart. + - if the lens touches external contracts, instruct the subagent to verify load-bearing claims via web search and quote source URLs. - ask the subagent to report findings with file paths and NEW line numbers from the full PR diff so you can anchor inline comments. delegation discipline: - - do NOT lens-review the diff yourself in parallel with the subagents - do NOT summarize the changes for them (biases toward validation frame) - do NOT hand them a curated reading list (let them discover scope) - do NOT pre-shape their output with a finding schema - do NOT mention the other lenses (independence is the point) -6. **aggregate, draft, self-critique**: merge findings; de-dup overlaps; trace each finding yourself. drop praise, style preferences, speculative/unverified claims, findings about pre-existing code unrelated to the new commits, anything not actionable, and anything that re-states prior review feedback (heuristic: if the finding's root cause lives in lines the *new commits* added or modified, it's in scope; otherwise drop). also drop **bloat-shaped findings** — proposed fixes that would add defensive checks for cases that can't happen, abstractions used once, comments restating obvious code, tests asserting tautologies, or "just-in-case" guards. subagents are fallible and bias toward recommending changes; the bar for an actionable inline comment is sound + correct + elegant. recommending a change that improves only one of the three (or degrades elegance to nominally improve correctness) makes the codebase worse, not better. To compute "lines the new commits added or modified": if \`incrementalDiffPath\` from step 2 is present, use it directly. Otherwise, take the prior Pullfrog review's \`commit_id\` (returned alongside each entry from \`${t("list_pull_request_reviews")}\` in step 4) and run \`git diff ..HEAD\` to isolate the lines added since that review. draft inline comments with NEW line numbers from the full PR diff — every comment must be actionable, 2-3 sentences max. +8. **aggregate, draft, self-critique**: merge findings (yours + any subagent output if you went multi-lens); de-dup overlaps; trace each finding yourself. drop praise, style preferences, speculative/unverified claims, findings about pre-existing code unrelated to the new commits, anything not actionable, and anything that re-states prior review feedback (heuristic: if the finding's root cause lives in lines the *new commits* added or modified, it's in scope; otherwise drop). also drop **bloat-shaped findings** — proposed fixes that would add defensive checks for cases that can't happen, abstractions used once, comments restating obvious code, tests asserting tautologies, or "just-in-case" guards. subagents are fallible and bias toward recommending changes; the bar for an actionable inline comment is sound + correct + elegant. recommending a change that improves only one of the three (or degrades elegance to nominally improve correctness) makes the codebase worse, not better. To compute "lines the new commits added or modified": if \`incrementalDiffPath\` from step 2 is present, use it directly. Otherwise, take the prior Pullfrog review's \`commit_id\` (returned alongside each entry from \`${t("list_pull_request_reviews")}\` in step 4) and run \`git diff ..HEAD\` to isolate the lines added since that review. draft inline comments with NEW line numbers from the full PR diff — every comment must be actionable, 2-3 sentences max. -7. **build the review body** — a single "Reviewed changes" section: summarize at the logical-change level, not per-file. each bullet starts with a past-tense verb (e.g. \`- Extracted shared CLI runtime into a single module\`, \`- Renamed package to pullfrog\`). avoid file paths unless they add clarity. if the changes can be described in one sentence, use one sentence — no bullets needed. do NOT include a separate "Prior review feedback" checklist; that's tracked in the rolling PR summary snapshot for the next agent run, and surfacing it in the user-facing body is noise (changes that addressed prior feedback are already covered by the Reviewed-changes bullets). in some cases you may receive a complete diff for the whole pull request instead of an incremental one — when this happens, you will need to determine what changes have happened since Pullfrog's most recent review. +9. **build the review body** — a single "Reviewed changes" section: summarize at the logical-change level, not per-file. each bullet starts with a past-tense verb (e.g. \`- Extracted shared CLI runtime into a single module\`, \`- Renamed package to pullfrog\`). avoid file paths unless they add clarity. if the changes can be described in one sentence, use one sentence — no bullets needed. do NOT include a separate "Prior review feedback" checklist; that's tracked in the rolling PR summary snapshot for the next agent run, and surfacing it in the user-facing body is noise (changes that addressed prior feedback are already covered by the Reviewed-changes bullets). in some cases you may receive a complete diff for the whole pull request instead of an incremental one — when this happens, you will need to determine what changes have happened since Pullfrog's most recent review. -8. Submit — every run must end with EXACTLY ONE of \`${t("create_pull_request_review")}\` (substantive review) or \`${t("report_progress")}\` (no-review acknowledgement). do NOT call \`create_issue_comment\` for review output. +10. Submit — every run must end with EXACTLY ONE of \`${t("create_pull_request_review")}\` (substantive review) or \`${t("report_progress")}\` (no-review acknowledgement). do NOT call \`create_issue_comment\` for review output. Same callout-intensity ladder as Review mode — \`[!CAUTION]\` (large red, "will break") → \`[!IMPORTANT]\` (large purple, "must address before merging") → \`[!NOTE]\` (small blue, "FYI") → no callout (plain text). And the same Fix-button lever: the footer renders a Fix button on every non-approving review, so \`approved: true\` suppresses it. Wrapping mergeable feedback in \`[!IMPORTANT]\` trains users to click Fix on reviews that don't need fixing — pick the tier the author's actual next action justifies.