From dda429b258e98bd162804f3df9e8be031752eb3c Mon Sep 17 00:00:00 2001 From: Felipe Rodriguez Angel Date: Mon, 24 Nov 2025 12:37:29 -0500 Subject: [PATCH] Fix: Allow empty result string in reviewDiff validation The reviewDiff function was rejecting valid review completions when the AI agent returned an empty string as the result. This occurred when the agent used only toolbox functions (leave_inline_comment, leave_general_comment) without additional text output. Issue: - Line 81 checked: if (!threadId || !result) - Empty string is falsy in JavaScript, so !result evaluates to true - This caused 'Amp review failed' errors even when review succeeded - Comments are collected via JSONL file, not the result string - The result field is never used after being returned Fix: - Changed to only check: if (!threadId) - Empty result is now valid (AI used tools, no text needed) - Only fails if no thread was created (legitimate error) Evidence: - Tested with 130K+ char diffs that trigger tool-only responses - Verified result field is never accessed in process-review.ts - Build passes, TypeScript compiles successfully This allows reviews to complete when AI agents generate comments via tools without returning additional text in the result field. --- src/review/reviewer.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/review/reviewer.ts b/src/review/reviewer.ts index 0b99911..6eeead5 100644 --- a/src/review/reviewer.ts +++ b/src/review/reviewer.ts @@ -77,8 +77,8 @@ export const reviewDiff = async ( } console.log('[Amp] Iterator completed'); - // Check if threadId and result are defined - if (!threadId || !result) throw new Error('Amp review failed'); + // Check if threadId is defined (result can be empty when AI only uses tools) + if (!threadId) throw new Error('No thread ID received from Amp'); return { success: true, threadId, result, commentsFilePath }; } catch (error) {