From 1256dd8025aec1b5ab9fc796f471b918a47d3fd2 Mon Sep 17 00:00:00 2001 From: wolfy Date: Mon, 1 Jun 2026 13:06:08 -0500 Subject: [PATCH] fix: read diff ranges not source files in review; ban praise comments in nudge --- agents/ollama.ts | 2 +- modes.ts | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/agents/ollama.ts b/agents/ollama.ts index 2ee8159..f3ee160 100644 --- a/agents/ollama.ts +++ b/agents/ollama.ts @@ -323,7 +323,7 @@ async function runOllamaLoop(ctx: AgentRunContext): Promise { selectedModeName = selectedMode; const firstStep = isReviewMode - ? "Your first tool call must be checkout_pr with the PR number from the event context." + ? "Your first tool call must be checkout_pr. After that: (1) read diff ranges from the returned diffPath using the TOC line numbers — do NOT read full source files one-by-one, that will exhaust your context window; (2) every inline comment MUST identify a specific problem — never write praise or 'looks good' observations." : "Call the first tool required by the workflow now."; const endCondition = isReviewMode diff --git a/modes.ts b/modes.ts index c15ddc0..ddab217 100644 --- a/modes.ts +++ b/modes.ts @@ -315,7 +315,7 @@ 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. - **file inventory**: for every non-trivial file in the diff TOC (skip lockfiles, \`*.lock\`, auto-generated migration checksums, formatter/linter configs with no logic changes), read the file — the diff hunk is usually sufficient; read the full file when the hunk alone doesn't make the intent clear. for each file, record a one-line description: *what changed and why*. this inventory becomes the per-file table in the review body, and ensures a concrete understanding of each file before you form review opinions. read in batches to stay efficient — you do not need to read every line, just enough to understand each file's role in the PR. + **file inventory**: for every non-trivial file in the diff TOC (skip lockfiles, \`*.lock\`, auto-generated migration checksums, formatter/linter configs with no logic changes), read its diff hunk by calling \`read_file\` on \`diffPath\` using the TOC line range (e.g. if the TOC shows \`foo.ts (lines 42-100)\`, call \`read_file\` on \`diffPath\` with offset 41, limit 59). **Do NOT read full source files one-by-one — that exhausts the context window on large PRs.** Only call \`read_file\` on the actual source file when the diff hunk is insufficient to understand broader context. For each file record a one-line description: *what changed and why*. This inventory becomes the per-file table in the review body. 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. @@ -473,7 +473,7 @@ ${PR_SUMMARY_FORMAT}`, 2. **checkout**: call \`${t("checkout_pr")}\` — this returns PR metadata, \`diffPath\` (full diff), and \`incrementalDiffPath\` (changes since last reviewed version, if available). read the diff TOC first and use its line ranges as your coverage checklist. - **file inventory**: for every non-trivial file in the incremental diff TOC (skip lockfiles, \`*.lock\`, auto-generated migration checksums, formatter/linter configs with no logic changes), read the file or its diff hunk and record a one-line description: *what changed and why*. this inventory becomes the per-file table in the review body. + **file inventory**: for every non-trivial file in the incremental diff TOC (skip lockfiles, \`*.lock\`, auto-generated migration checksums, formatter/linter configs with no logic changes), read its diff hunk by calling \`read_file\` on \`diffPath\` (or \`incrementalDiffPath\` if present) using the TOC line range. **Do NOT read full source files one-by-one — that exhausts the context window.** Only read the actual source file when the diff hunk is insufficient to understand broader context. Record a one-line description per file: *what changed and why*. This inventory becomes the per-file table in the review body. 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.