From e8b2c9952ba222031e92759b17324bdf46015f1d Mon Sep 17 00:00:00 2001 From: wolfy Date: Sun, 31 May 2026 18:34:58 -0500 Subject: [PATCH] fix: revert streaming, add dedup and format enforcement --- agents/ollama.ts | 78 +++++++++++++++++++++++------------------------- mcp/review.ts | 46 ++++++++++++++++++++++++++-- modes.ts | 4 ++- 3 files changed, 84 insertions(+), 44 deletions(-) diff --git a/agents/ollama.ts b/agents/ollama.ts index b25dae0..eeba444 100644 --- a/agents/ollama.ts +++ b/agents/ollama.ts @@ -152,49 +152,47 @@ async function runOllamaLoop(ctx: AgentRunContext): Promise { iterations++; log.info(`» Ollama turn ${iterations}/${MAX_ITERATIONS}...`); - // Stream the response so the activity monitor sees tokens arriving during - // long prefill. Without streaming, a 50k+ token context can take 200-300s - // of silent prefill before the first token, tripping the activity timeout. - let assistantMessage: Message; - let promptTokens: number | undefined; - let evalTokens: number | undefined; + // Non-streaming with a heartbeat timer so the activity monitor stays alive + // during long prefill. Streaming was tried but Ollama only emits one tool + // call per chunk — batched tool calls collapse to one-per-turn, turning a + // 7-turn run into 26 turns. The heartbeat fires every 60s to prevent the + // 300s activity timeout from triggering during large-context prefill. + let response: Awaited>; + const turnStart = Date.now(); + const heartbeat = setInterval(() => { + log.info(`» still waiting for model... (${Math.round((Date.now() - turnStart) / 1000)}s)`); + }, 60_000); try { - const stream = await ollama.chat({ - model, - messages, - tools, - keep_alive: -1, - think: false, - options: { num_ctx: 262144, temperature: 0.1 }, - stream: true, - }); - - let content = ""; - let streamToolCalls: ToolCall[] | undefined; - let chunkCount = 0; - for await (const chunk of stream) { - content += chunk.message.content ?? ""; - if (chunk.message.tool_calls?.length) streamToolCalls = chunk.message.tool_calls; - if (chunk.prompt_eval_count !== undefined) promptTokens = chunk.prompt_eval_count; - if (chunk.eval_count !== undefined) evalTokens = chunk.eval_count; - chunkCount++; - // Log a heartbeat every 50 chunks so the activity monitor stays alive - if (chunkCount % 50 === 0) log.debug(` streaming… (${chunkCount} chunks)`); - } - assistantMessage = { role: "assistant", content, tool_calls: streamToolCalls }; + response = await retry( + () => ollama.chat({ + model, + messages, + tools, + keep_alive: -1, + think: false, + options: { num_ctx: 262144, temperature: 0.1 }, + }), + { + delaysMs: [3_000, 8_000], + shouldRetry: (err) => { + const msg = err instanceof Error ? err.message : String(err); + return /unexpected EOF|XML syntax error|ECONNRESET|ETIMEDOUT|fetch failed/i.test(msg); + }, + label: `Ollama turn ${iterations}`, + }, + ); } catch (err) { - const msg = err instanceof Error ? err.message : String(err); - const isRetryable = /unexpected EOF|XML syntax error|ECONNRESET|ETIMEDOUT|fetch failed/i.test(msg); - if (!isRetryable) { - await unloadModel(ollama, model); - log.error(`Ollama error: ${msg}`); - return { success: false, error: `Ollama request failed: ${msg}` }; - } - // Retryable: wait and loop — the outer while will retry on the next iteration - log.warning(`» stream error (retryable): ${msg} — retrying in 5s`); - await new Promise((r) => setTimeout(r, 5_000)); - continue; + clearInterval(heartbeat); + await unloadModel(ollama, model); + const lastError = err instanceof Error ? err.message : String(err); + log.error(`Ollama error: ${lastError}`); + return { success: false, error: `Ollama request failed: ${lastError}` }; } + clearInterval(heartbeat); + + const promptTokens = response.prompt_eval_count; + const evalTokens = response.eval_count; + const assistantMessage = response.message; if (promptTokens !== undefined) { const total = promptTokens + (evalTokens ?? 0); diff --git a/mcp/review.ts b/mcp/review.ts index 250d26c..9a8ccbd 100644 --- a/mcp/review.ts +++ b/mcp/review.ts @@ -213,6 +213,46 @@ export const CreatePullRequestReview = type({ .optional(), }); +/** + * Remove duplicate inline comments on the same file. Uses Jaccard similarity + * on content words — if two comments on the same file share ≥40% of their + * significant words they're almost certainly the same finding repeated. + */ +function deduplicateComments(comments: ReviewCommentInput[]): ReviewCommentInput[] { + const STOPWORDS = new Set(["the", "this", "that", "with", "from", "have", "will", "when", "also", "both", "into", "than", "then", "they", "some", "more", "and", "but", "for", "are", "not", "use"]); + const keywords = (text: string): Set => + new Set((text ?? "").toLowerCase().split(/\W+/).filter((w) => w.length > 3 && !STOPWORDS.has(w))); + + const jaccard = (a: Set, b: Set): number => { + const inter = [...a].filter((w) => b.has(w)).length; + const union = new Set([...a, ...b]).size; + return union === 0 ? 0 : inter / union; + }; + + const byPath = new Map(); + for (const c of comments) { + const group = byPath.get(c.path) ?? []; + group.push(c); + byPath.set(c.path, group); + } + + const result: ReviewCommentInput[] = []; + for (const [, group] of byPath) { + const kept: ReviewCommentInput[] = []; + for (const candidate of group) { + const ckw = keywords(candidate.body ?? ""); + const isDup = kept.some((k) => jaccard(ckw, keywords(k.body ?? "")) >= 0.4); + if (isDup) { + log.info(`deduped inline comment at ${candidate.path}:${candidate.line} — similar to existing comment on same file`); + } else { + kept.push(candidate); + } + } + result.push(...kept); + } + return result; +} + /** Assemble the **Reviewed changes** preamble block from structured params. */ function assemblePreamble(preamble: string, changes: string[]): string { const lines = [`**Reviewed changes** — ${preamble}`]; @@ -350,15 +390,15 @@ export function CreatePullRequestReviewTool(ctx: ToolContext) { runDiffCoveragePreflight({ ctx }); - // Build review comments - const reviewComments: ReviewCommentInput[] = comments.map((c) => { + // Build review comments (deduplicate same-file same-topic comments first) + const reviewComments: ReviewCommentInput[] = deduplicateComments(comments.map((c) => { let commentBody = fixDoubleEscapedString(c.body || ""); if (c.suggestion !== undefined) { const block = "```suggestion\n" + c.suggestion + "\n```"; commentBody = commentBody ? `${commentBody}\n\n${block}` : block; } return { path: c.path, line: c.line, side: c.side || "RIGHT", body: commentBody, start_line: c.start_line }; - }); + })); let droppedComments: DroppedComment[] = []; let validComments: ReviewCommentInput[] = []; diff --git a/modes.ts b/modes.ts index e0a04e6..b523dd4 100644 --- a/modes.ts +++ b/modes.ts @@ -390,7 +390,7 @@ For simple, well-defined tasks, skip the plan phase and go straight to build.`, 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. -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. +7. **submit**: ALWAYS submit exactly one review via \`${t("create_pull_request_review")}\`. **Do NOT call \`report_progress\`** — it creates a second visible comment and must not be used in Review mode. The review IS the final record; the progress comment is cleaned up automatically. **MANDATORY pre-submission self-check**: before calling \`${t("create_pull_request_review")}\`, do both of these: 1. For each finding already in your \`comments\` array: verify it does NOT also appear as a \`### \` section in the body. A finding goes in ONE place only — inline comment OR body section, never both. If it has a line anchor it goes inline; remove the duplicate body section. @@ -411,6 +411,8 @@ For simple, well-defined tasks, skip the plan phase and go straight to build.`, \`\`\` Inline comments are passed via the \`comments\` parameter, not in the body. + **Body format** — use ONLY the structure from the default format below. Forbidden patterns: \`## \` headings, numbered bold items like \`**1. title**\`, \`## Issues to address\`, \`## Positive notes\`, \`## Minor suggestions\`, or any praise/summary section. Use \`### {emoji} {title}\` for non-anchored issue sections ONLY. No praise sections. + 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: - \`[!CAUTION]\` — large red banner. Reads as "this will break something."