diff --git a/agents/ollama.ts b/agents/ollama.ts index d2d421a..1cf8210 100644 --- a/agents/ollama.ts +++ b/agents/ollama.ts @@ -69,6 +69,32 @@ async function callMcpTool( } } +/** + * When context approaches the limit, truncate the content of old tool-result + * messages to free space. Keeps the most recent N tool results intact so the + * model still has fresh context; replaces earlier ones with a size notice. + * Never touches system/user/assistant messages — only tool messages. + */ +function pruneToolMessages(messages: Message[], keepRecent = 6): Message[] { + const toolIndices: number[] = []; + for (let i = 0; i < messages.length; i++) { + if (messages[i].role === "tool") toolIndices.push(i); + } + const pruneCount = Math.max(0, toolIndices.length - keepRecent); + if (pruneCount === 0) return messages; + + const toPrune = new Set(toolIndices.slice(0, pruneCount)); + let pruned = 0; + const result = messages.map((msg, i) => { + if (!toPrune.has(i)) return msg; + const originalLen = typeof msg.content === "string" ? msg.content.length : 0; + pruned++; + return { ...msg, content: `[pruned: was ${originalLen} chars — context limit approached]` }; + }); + log.info(`» pruned ${pruned} old tool message(s) to reduce context`); + return result; +} + async function runOllamaLoop(ctx: AgentRunContext): Promise { const ollamaHost = process.env.OLLAMA_HOST ?? ""; if (!ollamaHost) { @@ -89,7 +115,7 @@ async function runOllamaLoop(ctx: AgentRunContext): Promise { const tools = await getOllamaTools(mcpClient); log.info(`» ${tools.length} tools available`); - const messages: Message[] = [ + let messages: Message[] = [ { role: "user", content: ctx.instructions.full, @@ -149,6 +175,9 @@ async function runOllamaLoop(ctx: AgentRunContext): Promise { log.info( ` context: ${promptTokens} prompt + ${evalTokens ?? 0} eval = ${total} tokens (${pct}% of limit)`, ); + if (promptTokens > 200_000) { + messages = pruneToolMessages(messages); + } } const assistantMessage = response.message; diff --git a/mcp/review.ts b/mcp/review.ts index e52efeb..e45f43b 100644 --- a/mcp/review.ts +++ b/mcp/review.ts @@ -181,7 +181,20 @@ export function formatDroppedCommentsNote(dropped: DroppedComment[]): string { export const CreatePullRequestReview = type({ pull_number: type.number.describe("The pull request number to review"), - body: type.string.describe("1-2 sentence high-level summary").optional(), + preamble: type.string.describe( + "One sentence describing what was reviewed (e.g. 'This PR adds device management behind a feature flag'). " + + "The server prepends '**Reviewed changes** — ' automatically. " + + "When provided, do NOT repeat the preamble inside 'body'." + ).optional(), + changes: type.string.array().describe( + "Bullet list of substantive changes — neutral descriptions of what the PR added/changed/removed. " + + "Each entry is one formatted bullet, e.g. '**Feature X** — 1 sentence description'. " + + "These must describe CHANGES only, not findings or issues." + ).optional(), + body: type.string.describe( + "When 'preamble'/'changes' are used: include ONLY the metadata HTML comment and any non-anchored ### sections. " + + "When used alone (legacy): full review body including preamble." + ).optional(), approved: type.boolean.describe("Set to true to submit as an approval.").optional(), commit_id: type.string.describe("Optional SHA of the commit being reviewed.").optional(), comments: type({ @@ -200,25 +213,86 @@ export const CreatePullRequestReview = type({ .optional(), }); +/** Assemble the **Reviewed changes** preamble block from structured params. */ +function assemblePreamble(preamble: string, changes: string[]): string { + const lines = [`**Reviewed changes** — ${preamble}`]; + if (changes.length > 0) { + lines.push(""); + for (const change of changes) { + lines.push(change.startsWith("- ") ? change : `- ${change}`); + } + } + return lines.join("\n"); +} + +/** + * Strip body ### sections whose file paths are already covered by inline + * comments. Prevents the model from duplicating inline findings in the body. + */ +function postProcessBody( + body: string, + validComments: ReviewCommentInput[], +): string { + if (validComments.length === 0) return body; + + const commentedPaths = new Set(validComments.map((c) => c.path)); + + const sectionRegex = /^### .+$/gm; + const matches: Array<{ index: number; heading: string }> = []; + let m: RegExpExecArray | null; + while ((m = sectionRegex.exec(body)) !== null) { + matches.push({ index: m.index, heading: m[0] }); + } + if (matches.length === 0) return body; + + const sections = matches.map((match, i) => ({ + start: match.index, + end: matches[i + 1]?.index ?? body.length, + heading: match.heading, + })); + + const toRemove: Array<{ start: number; end: number }> = []; + for (const section of sections) { + const content = body.slice(section.start, section.end); + for (const path of commentedPaths) { + if (content.includes(path)) { + toRemove.push({ start: section.start, end: section.end }); + log.info(`stripped duplicate body section "${section.heading.slice(0, 80)}" — already covered by inline comment on ${path}`); + break; + } + } + } + + if (toRemove.length === 0) return body; + + let result = body; + for (const { start, end } of [...toRemove].reverse()) { + result = result.slice(0, start) + result.slice(end); + } + return result.replace(/\n{3,}/g, "\n\n").trim(); +} + export function CreatePullRequestReviewTool(ctx: ToolContext) { return tool({ name: "create_pull_request_review", description: "Submit a review for an existing pull request. " + - "Reviews with no body AND no comments are silently skipped. " + - "IMPORTANT: 95%+ of feedback should be in 'comments' with file paths and line numbers. " + - "Only use 'body' for a 1-2 sentence summary. " + - "For every actionable issue, the comment 'body' should explain the problem AND include a markdown " + - "code block showing the corrected code. For example: " + - "'SQL injection: id is interpolated directly into the query string.\\n\\n```ts\\nconst result = await db.query(\\'SELECT * FROM users WHERE id = $1\\', [id]);\\n```'. " + + "PREFERRED: use 'preamble' + 'changes' for the reviewed-changes block, and 'body' for ONLY the metadata HTML comment and non-anchored ### sections. " + + "IMPORTANT: 95%+ of feedback must be in 'comments' with file paths and line numbers — not in 'body'. " + "The first submission may error once with a diff-coverage nudge — retry with the same arguments. " + - "Inline comments: 'path' must be the SOURCE FILE path (e.g. 'apps/foo/bar.ts') from the diff --git header, NOT the diffPath returned by checkout_pr. " + + "Inline comments: 'path' must be the SOURCE FILE path (e.g. 'apps/foo/bar.ts') from the diff --git header, NOT the diffPath. " + "'line' must be the actual file line number from the '| newLine |' column in the formatted diff (not the TOC line range). " + "Inline comments can ONLY target files and lines that appear in the PR diff.", parameters: CreatePullRequestReview, - execute: execute(async ({ pull_number, body, approved, commit_id, comments = [] }) => { + execute: execute(async ({ pull_number, preamble, changes, body, approved, commit_id, comments = [] }) => { if (body) body = fixDoubleEscapedString(body); + // Assemble structured preamble if provided + if (preamble || (changes && changes.length > 0)) { + const preambleBlock = assemblePreamble(preamble ?? "", changes ?? []); + body = body ? `${preambleBlock}\n\n${body}` : preambleBlock; + } + ctx.toolState.issueNumber = pull_number; const dup = duplicateReviewDecision({ @@ -287,6 +361,11 @@ export function CreatePullRequestReviewTool(ctx: ToolContext) { } } + // Strip body ### sections that duplicate inline comments (#1 post-processing) + if (body && validComments.length > 0) { + body = postProcessBody(body, validComments); + } + if (droppedComments.length > 0) { const note = formatDroppedCommentsNote(droppedComments); body = body ? body + note : note.replace(/^\n\n/, ""); diff --git a/modes.ts b/modes.ts index 6b70e90..236ea54 100644 --- a/modes.ts +++ b/modes.ts @@ -397,7 +397,18 @@ For simple, well-defined tasks, skip the plan phase and go straight to build.`, 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. - The review body is structured as: \`[optional alert blockquote]\` → \`[PR summary using the default format below]\`. Inline comments are passed via the \`comments\` parameter, not in the body. + **Structured submission (preferred)**: use \`preamble\` + \`changes\` for the reviewed-changes block instead of writing it in \`body\`. Pass \`body\` for ONLY the metadata HTML comment and non-anchored \`### \` sections (if any). The server assembles the full preamble block for you. Example call shape: + \`\`\` + create_pull_request_review({ + pull_number: N, + preamble: "one sentence on what the PR does", + changes: ["**Feature X** — description", "**Migration Y** — description"], + body: "\\n\\n### ⚠️ Non-anchored concern...\\n\\n### ℹ️ Nitpicks\\n...", + comments: [{ path: "src/foo.ts", line: 42, body: "..." }, ...], + approved: false, + }) + \`\`\` + Inline comments are passed via the \`comments\` parameter, not in the body. The opening callout is what the author sees first — pick the one that matches what you want them to do. Five tiers, from loudest to friendliest: