From 29842afb84f4fdf3a4c689a40fc485193e83bb9d Mon Sep 17 00:00:00 2001 From: Esteban Beltran Date: Thu, 26 Mar 2026 11:05:18 +0100 Subject: [PATCH] fix: make LLM review resilient to individual question failures - Increase LLM call timeout from 20s to 90s to handle large code contexts - Remove redundant outer retry loop in AskLLMAboutCode (inner callLLMWithRetry already handles retries) - Skip failed questions instead of aborting the entire review - Report OK status when LLM review completes without concerns --- pkg/analysis/passes/llmreview/llmreview.go | 18 ++++++- pkg/llmclient/agentic_client.go | 2 +- pkg/llmvalidate/llmvalidate.go | 62 +++++++++------------- 3 files changed, 42 insertions(+), 40 deletions(-) diff --git a/pkg/analysis/passes/llmreview/llmreview.go b/pkg/analysis/passes/llmreview/llmreview.go index 70de94c9..eaf8089e 100644 --- a/pkg/analysis/passes/llmreview/llmreview.go +++ b/pkg/analysis/passes/llmreview/llmreview.go @@ -34,6 +34,10 @@ var ( Name: "llm-review-skipped", Severity: analysis.SuspectedProblem, } + llmReviewPassed = &analysis.Rule{ + Name: "llm-review-passed", + Severity: analysis.OK, + } ) // blockingAnalyzers contains validators that, if they report errors, should cause @@ -66,7 +70,7 @@ var Analyzer = &analysis.Analyzer{ Name: "llmreview", Requires: append([]*analysis.Analyzer{sourcecode.Analyzer}, blockingAnalyzers...), Run: run, - Rules: []*analysis.Rule{llmIssueFound, llmReviewSkipped}, + Rules: []*analysis.Rule{llmIssueFound, llmReviewSkipped, llmReviewPassed}, ReadmeInfo: analysis.ReadmeInfo{ Name: "LLM Review", Description: "Runs the code through an LLM to check for security issues or disallowed usage.", @@ -213,6 +217,7 @@ func run(pass *analysis.Pass) (any, error) { return nil, nil } + issuesFound := 0 for _, answer := range mandatoryAnswers { if answer.ShortAnswer != answer.ExpectedShortAnswer { detail := buildDetailString(answer) @@ -222,6 +227,7 @@ func run(pass *analysis.Pass) (any, error) { fmt.Sprintf("LLM flagged: %s", answer.Question), detail, ) + issuesFound++ } } @@ -242,9 +248,19 @@ func run(pass *analysis.Pass) (any, error) { fmt.Sprintf("LLM suggestion: %s", answer.Question), detail, ) + issuesFound++ } } + if issuesFound == 0 && llmReviewPassed.ReportAll { + pass.ReportResult( + pass.AnalyzerName, + llmReviewPassed, + "LLM review completed without concerns", + "", + ) + } + return nil, nil } diff --git a/pkg/llmclient/agentic_client.go b/pkg/llmclient/agentic_client.go index ef4618db..e7baee26 100644 --- a/pkg/llmclient/agentic_client.go +++ b/pkg/llmclient/agentic_client.go @@ -23,7 +23,7 @@ const ( maxLLMRetries = 3 maxConsecutiveNoTools = 5 retryDelay = 2 * time.Second - llmCallTimeout = 20 * time.Second + llmCallTimeout = 90 * time.Second budgetNudgePrompt = `You have only %d tool calls remaining. Wrap up your investigation and call submit_answer now with whatever information you have gathered so far.` diff --git a/pkg/llmvalidate/llmvalidate.go b/pkg/llmvalidate/llmvalidate.go index faaa1405..229a3a37 100644 --- a/pkg/llmvalidate/llmvalidate.go +++ b/pkg/llmvalidate/llmvalidate.go @@ -191,46 +191,32 @@ func (c *Client) AskLLMAboutCode( for _, question := range questions { userPrompt := fmt.Sprintf("Answer this question based on the source code files provided in the system prompt: %s", question.Question) - var answer LLMAnswer - var lastErr error - - for retries := 3; retries > 0; retries-- { - agenticAnswers, err := agenticClient.CallLLM(c.ctx, []string{userPrompt}, absCodePath) - if err != nil { - lastErr = err - logme.DebugFln("Error calling LLM (retries left: %d): %v", retries-1, err) - continue - } - - if len(agenticAnswers) == 0 { - lastErr = fmt.Errorf("No answer returned from LLM for question: %s", question.Question) - logme.DebugFln("No answer returned (retries left: %d)", retries-1) - continue - } - - agenticAnswer := agenticAnswers[0] - - // Check if the agentic client reported an error for this question - if agenticAnswer.Error != "" { - lastErr = fmt.Errorf("LLM error for question %q: %s", question.Question, agenticAnswer.Error) - logme.DebugFln("LLM error (retries left: %d): %s", retries-1, agenticAnswer.Error) - continue - } - - answer = LLMAnswer{ - Question: agenticAnswer.Question, - Answer: agenticAnswer.Answer, - ShortAnswer: agenticAnswer.ShortAnswer, - Files: agenticAnswer.Files, - CodeSnippet: mergeNewlines(agenticAnswer.CodeSnippet), - ExpectedShortAnswer: question.ExpectedAnswer, - } - lastErr = nil - break + agenticAnswers, err := agenticClient.CallLLM(c.ctx, []string{userPrompt}, absCodePath) + if err != nil { + logme.DebugFln("Error calling LLM for question %q: %v. Skipping.", question.Question, err) + continue + } + + if len(agenticAnswers) == 0 { + logme.DebugFln("No answer returned from LLM for question: %s. Skipping.", question.Question) + continue + } + + agenticAnswer := agenticAnswers[0] + + // Check if the agentic client reported an error for this question + if agenticAnswer.Error != "" { + logme.DebugFln("LLM error for question %q: %s. Skipping.", question.Question, agenticAnswer.Error) + continue } - if lastErr != nil { - return nil, fmt.Errorf("Failed to generate answer after 3 retries: %w", lastErr) + answer := LLMAnswer{ + Question: agenticAnswer.Question, + Answer: agenticAnswer.Answer, + ShortAnswer: agenticAnswer.ShortAnswer, + Files: agenticAnswer.Files, + CodeSnippet: mergeNewlines(agenticAnswer.CodeSnippet), + ExpectedShortAnswer: question.ExpectedAnswer, } logme.DebugFln("Answer: %v", prettyprint.SPrint(answer))