From 4735da8d34505f7572cac3643852d7a63c565634 Mon Sep 17 00:00:00 2001 From: Esteban Beltran Date: Mon, 31 Mar 2025 09:08:10 -0600 Subject: [PATCH 01/13] improve debug and update model --- pkg/analysis/passes/llmreview/llmreview.go | 2 +- pkg/llmvalidate/llmvalidate.go | 7 +++++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/pkg/analysis/passes/llmreview/llmreview.go b/pkg/analysis/passes/llmreview/llmreview.go index 34b4cc35..82541d3a 100644 --- a/pkg/analysis/passes/llmreview/llmreview.go +++ b/pkg/analysis/passes/llmreview/llmreview.go @@ -51,7 +51,7 @@ func run(pass *analysis.Pass) (interface{}, error) { logme.Debugln("Starting to run Gemini Validations. This might take a while...") - llmClient, err := llmvalidate.New(context.Background(), geminiKey, "gemini-1.5-flash-latest") + llmClient, err := llmvalidate.New(context.Background(), geminiKey, "gemini-2.0-flash") if err != nil { logme.DebugFln("Error initializing llm client: %v", err) diff --git a/pkg/llmvalidate/llmvalidate.go b/pkg/llmvalidate/llmvalidate.go index a7d8fc7b..19a9efd1 100644 --- a/pkg/llmvalidate/llmvalidate.go +++ b/pkg/llmvalidate/llmvalidate.go @@ -93,8 +93,9 @@ func New(ctx context.Context, apiKey string, modelName string) (*Client, error) } if modelName == "" { - modelName = "gemini-1.5-flash-latest" + modelName = "gemini-2.0-flash-lite" } + logme.DebugFln("llmvalidate: Using model %s", modelName) genaiClient, err := genai.NewClient(ctx, option.WithAPIKey(apiKey)) if err != nil { @@ -190,9 +191,11 @@ Answer the following questions in the context of the code above. be brief in you var answers []LLMAnswer err = json.Unmarshal([]byte(content), &answers) if err != nil { + logme.DebugFln("Failed to unmarshal content: %v", content) return nil, fmt.Errorf("failed to unmarshal content: %v", err) } - logme.Debugln("Got response from LLM with char length", len(answers)) + logme.DebugFln("Got answers from LLM: %v", answers) + logme.Debugln("Got response from LLM with char length", len(content)) return answers, nil From d9b367ae0b7bed1225a61354fb7c9df6a335c8e5 Mon Sep 17 00:00:00 2001 From: Esteban Beltran Date: Mon, 31 Mar 2025 11:27:40 -0600 Subject: [PATCH 02/13] WIP: structured output --- pkg/llmvalidate/llmvalidate.go | 89 +++++++++++++++++++--------------- 1 file changed, 51 insertions(+), 38 deletions(-) diff --git a/pkg/llmvalidate/llmvalidate.go b/pkg/llmvalidate/llmvalidate.go index 19a9efd1..afc65476 100644 --- a/pkg/llmvalidate/llmvalidate.go +++ b/pkg/llmvalidate/llmvalidate.go @@ -71,6 +71,16 @@ var allowExtensions = map[string]struct{}{ ".go": {}, } +var extensionToFileType = map[string]string{ + ".js": "javascript", + ".jsx": "javascript", + ".ts": "typescript", + ".tsx": "typescript", + ".cjs": "javascript", + ".mjs": "javascript", + ".go": "go", +} + type LLMAnswer struct { Question string `json:"question"` Answer string `json:"answer"` @@ -161,41 +171,40 @@ The output should be a valid plain JSON array. Each element with an answer conta }, } - formattedQuestions := "" - for _, question := range questions { - formattedQuestions += fmt.Sprintf("- %s\n", question) - } - - mainPrompt := fmt.Sprintf(` -The files in the repository are: -### START OF FILES ### - -%s + // formattedQuestions := "" + // for _, question := range questions { + // formattedQuestions += fmt.Sprintf("- %s\n", question) + // } -### END OF FILES ### + filesPrompt := fmt.Sprintf( + `The files in the repository are: %s `, + strings.Join(codePrompt, "\n"), + ) + var answers []LLMAnswer = make([]LLMAnswer, len(questions)) -Answer the following questions in the context of the code above. be brief in your answers. - -%s - -`, strings.Join(codePrompt, "\n"), formattedQuestions) - - modelResponse, err := model.GenerateContent(c.ctx, genai.Text(mainPrompt)) - if err != nil { - return nil, err - } - - content := getTextContentFromModelContentResponse(modelResponse) + for _, question := range questions { + questionPrompt := fmt.Sprintf( + "%s\n\n Answer the question based on the previous files: %s", + filesPrompt, + question, + ) + var answer LLMAnswer + modelResponse, err := model.GenerateContent(c.ctx, genai.Text(questionPrompt)) + if err != nil { + return nil, err + } - //unmarshall content into []LLMAnswer - var answers []LLMAnswer - err = json.Unmarshal([]byte(content), &answers) - if err != nil { - logme.DebugFln("Failed to unmarshal content: %v", content) - return nil, fmt.Errorf("failed to unmarshal content: %v", err) + content := getTextContentFromModelContentResponse(modelResponse) + //unmarshall content into []LLMAnswer + err = json.Unmarshal([]byte(content), &answer) + if err != nil { + logme.DebugFln("Failed to unmarshal content: %v", content) + return nil, fmt.Errorf("failed to unmarshal content: %v", err) + } + logme.DebugFln("Got answer from LLM: %v", answer) + logme.Debugln("Got response from LLM with char length", len(content)) + answers = append(answers, answer) } - logme.DebugFln("Got answers from LLM: %v", answers) - logme.Debugln("Got response from LLM with char length", len(content)) return answers, nil @@ -317,14 +326,18 @@ func getPromptContentForFile(codePath, relFile string) string { return "" } - promptContent := fmt.Sprintf(` -----##---- -Source filename: %s -Source Content: -%s -----##---- -`, relFile, content) + fileExt := filepath.Ext(relFile) + fileType, ok := extensionToFileType[fileExt] + if !ok { + fileType = fileExt + } + // this will format the content as: + // path/to/filename: + // ```filetype + // content + // ``` + promptContent := fmt.Sprintf("%s:\n```%s\n%s\n```\n", relFile, fileType, content) return promptContent } From c76b603c2e8d76a838c5eaf831b712e705f21613 Mon Sep 17 00:00:00 2001 From: Esteban Beltran Date: Mon, 31 Mar 2025 11:47:35 -0600 Subject: [PATCH 03/13] WIP: ask one question at the time --- pkg/analysis/passes/llmreview/llmreview.go | 5 +- pkg/llmvalidate/llmvalidate.go | 56 +++++++++++++++------- 2 files changed, 42 insertions(+), 19 deletions(-) diff --git a/pkg/analysis/passes/llmreview/llmreview.go b/pkg/analysis/passes/llmreview/llmreview.go index 82541d3a..18193dfa 100644 --- a/pkg/analysis/passes/llmreview/llmreview.go +++ b/pkg/analysis/passes/llmreview/llmreview.go @@ -37,7 +37,7 @@ var questions = []string{ "Does this code introduces analytics or tracking not part of Grafana APIs?. Provide a code snippet if so.", } -func run(pass *analysis.Pass) (interface{}, error) { +func run(pass *analysis.Pass) (any, error) { var err error // only run if sourcecode.Analyzer succeeded sourceCodeDir, ok := pass.ResultOf[sourcecode.Analyzer].(string) @@ -76,8 +76,7 @@ func run(pass *analysis.Pass) (interface{}, error) { } for _, answer := range answers { - shortAnswer := strings.TrimSpace(strings.ToLower(answer.ShortAnswer)) - if shortAnswer != "no" { + if answer.ShortAnswer { detail := fmt.Sprintf("Question: %s\n. Answer: %s. ", answer.Question, answer.Answer) diff --git a/pkg/llmvalidate/llmvalidate.go b/pkg/llmvalidate/llmvalidate.go index afc65476..6242190d 100644 --- a/pkg/llmvalidate/llmvalidate.go +++ b/pkg/llmvalidate/llmvalidate.go @@ -13,9 +13,8 @@ import ( "github.com/danwakefield/fnmatch" "github.com/google/generative-ai-go/genai" - "google.golang.org/api/option" - "github.com/grafana/plugin-validator/pkg/logme" + "google.golang.org/api/option" ) // these are not regular expressions @@ -81,14 +80,6 @@ var extensionToFileType = map[string]string{ ".go": "go", } -type LLMAnswer struct { - Question string `json:"question"` - Answer string `json:"answer"` - Files []string `json:"files"` - ShortAnswer string `json:"short_answer"` - CodeSnippet string `json:"code_snippet"` -} - type Client struct { genaiClient *genai.Client apiKey string @@ -96,6 +87,14 @@ type Client struct { ctx context.Context } +type LLMAnswer struct { + Question string `json:"question"` + Answer string `json:"answer"` + Files []string `json:"files"` + ShortAnswer bool `json:"short_answer"` + CodeSnippet string `json:"code_snippet"` +} + func New(ctx context.Context, apiKey string, modelName string) (*Client, error) { if apiKey == "" { @@ -153,6 +152,34 @@ func (c *Client) AskLLMAboutCode( model := c.genaiClient.GenerativeModel(c.modelName) // ensure it outputs json model.GenerationConfig.ResponseMIMEType = "application/json" + model.ResponseSchema = &genai.Schema{ + Type: genai.TypeObject, + Properties: map[string]*genai.Schema{ + "question": { + Type: genai.TypeString, + Description: "The original question", + }, + "answer": { + Type: genai.TypeString, + Description: "The full answer to the question. Elaborate why yes or no", + }, + "files": { + Type: genai.TypeArray, + Items: &genai.Schema{ + Type: genai.TypeString, + }, + Description: "An array of files related to the answer", + }, + "short_answer": { + Type: genai.TypeBoolean, + Description: "True or false", + }, + "code_snippet": { + Type: genai.TypeString, + Description: "Code snippet as context for the answer if applicable", + }, + }, + } model.SystemInstruction = &genai.Content{ Parts: []genai.Part{ @@ -164,18 +191,13 @@ The output should be a valid plain JSON array. Each element with an answer conta * question: The original question * answer: The answer * files: An array of related files if applicable. -* short_answer: Yes/No/NA +* short_answer: True or false * code_snippet: The code snippet relevant to the question. Empty if not applicable `, ), }, } - // formattedQuestions := "" - // for _, question := range questions { - // formattedQuestions += fmt.Sprintf("- %s\n", question) - // } - filesPrompt := fmt.Sprintf( `The files in the repository are: %s `, strings.Join(codePrompt, "\n"), @@ -219,6 +241,8 @@ func getTextContentFromModelContentResponse(modelResponse *genai.GenerateContent for _, part := range content.Parts { finalContent += fmt.Sprint(part) } + // replace any duplicated new lines with a single new line + finalContent = strings.ReplaceAll(finalContent, "\n\n", "\n") return finalContent } From cf5286137801bac4d3d38945142e9729d89facec Mon Sep 17 00:00:00 2001 From: Esteban Beltran Date: Mon, 31 Mar 2025 12:01:25 -0600 Subject: [PATCH 04/13] wip: better LLm integration --- pkg/llmvalidate/llmvalidate.go | 27 ++++++++++++--------------- pkg/prettyprint/prettyprint.go | 5 +++++ 2 files changed, 17 insertions(+), 15 deletions(-) diff --git a/pkg/llmvalidate/llmvalidate.go b/pkg/llmvalidate/llmvalidate.go index 6242190d..96d52729 100644 --- a/pkg/llmvalidate/llmvalidate.go +++ b/pkg/llmvalidate/llmvalidate.go @@ -14,6 +14,7 @@ import ( "github.com/danwakefield/fnmatch" "github.com/google/generative-ai-go/genai" "github.com/grafana/plugin-validator/pkg/logme" + "github.com/grafana/plugin-validator/pkg/prettyprint" "google.golang.org/api/option" ) @@ -153,30 +154,36 @@ func (c *Client) AskLLMAboutCode( // ensure it outputs json model.GenerationConfig.ResponseMIMEType = "application/json" model.ResponseSchema = &genai.Schema{ - Type: genai.TypeObject, + Type: genai.TypeObject, + Required: []string{"question", "answer", "short_answer"}, Properties: map[string]*genai.Schema{ "question": { Type: genai.TypeString, - Description: "The original question", + Description: "The question to answer", + Nullable: false, }, "answer": { Type: genai.TypeString, Description: "The full answer to the question. Elaborate why yes or no", + Nullable: false, }, "files": { Type: genai.TypeArray, Items: &genai.Schema{ Type: genai.TypeString, }, + Nullable: true, Description: "An array of files related to the answer", }, "short_answer": { Type: genai.TypeBoolean, Description: "True or false", + Nullable: false, }, "code_snippet": { Type: genai.TypeString, Description: "Code snippet as context for the answer if applicable", + Nullable: true, }, }, } @@ -184,16 +191,7 @@ func (c *Client) AskLLMAboutCode( model.SystemInstruction = &genai.Content{ Parts: []genai.Part{ genai.Text( - `You are source code reviewer. You are provided with a source code repository information and files. You will answer questions only based on the context of the files provided - -The output should be a valid plain JSON array. Each element with an answer containing fields: - -* question: The original question -* answer: The answer -* files: An array of related files if applicable. -* short_answer: True or false -* code_snippet: The code snippet relevant to the question. Empty if not applicable - `, + `You are source code reviewer. You are provided with a source code repository information and files. You will answer questions only based on the context of the files provided. The output muset be a valid JSON object`, ), }, } @@ -206,7 +204,7 @@ The output should be a valid plain JSON array. Each element with an answer conta for _, question := range questions { questionPrompt := fmt.Sprintf( - "%s\n\n Answer the question based on the previous files: %s", + "%s\n\n Answer this question based on the previous files: %s", filesPrompt, question, ) @@ -223,8 +221,7 @@ The output should be a valid plain JSON array. Each element with an answer conta logme.DebugFln("Failed to unmarshal content: %v", content) return nil, fmt.Errorf("failed to unmarshal content: %v", err) } - logme.DebugFln("Got answer from LLM: %v", answer) - logme.Debugln("Got response from LLM with char length", len(content)) + logme.DebugFln("Answer: %v", prettyprint.SPrint(answer)) answers = append(answers, answer) } diff --git a/pkg/prettyprint/prettyprint.go b/pkg/prettyprint/prettyprint.go index afed89b7..043b7db7 100644 --- a/pkg/prettyprint/prettyprint.go +++ b/pkg/prettyprint/prettyprint.go @@ -9,3 +9,8 @@ func Print(b any) { s, _ := json.MarshalIndent(b, "", "\t") fmt.Print(string(s)) } + +func SPrint(b any) string { + s, _ := json.MarshalIndent(b, "", "\t") + return string(s) +} From fbbcd39102d457e620d04b3196ee79a65a57fb1f Mon Sep 17 00:00:00 2001 From: Esteban Beltran Date: Mon, 31 Mar 2025 12:32:04 -0600 Subject: [PATCH 05/13] add token count --- pkg/analysis/passes/llmreview/llmreview.go | 8 ++++---- pkg/llmvalidate/llmvalidate.go | 7 +++++++ 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/pkg/analysis/passes/llmreview/llmreview.go b/pkg/analysis/passes/llmreview/llmreview.go index 18193dfa..5c0c1b49 100644 --- a/pkg/analysis/passes/llmreview/llmreview.go +++ b/pkg/analysis/passes/llmreview/llmreview.go @@ -32,8 +32,8 @@ var Analyzer = &analysis.Analyzer{ var questions = []string{ "Does this code manipulate the file system? (explicit manipulation of the file system). Provide a code snippet if so.", - "Does this code allow the execution or arbitrary javascript code from user input?. Provide a code snippet if so", - "Does this code allow the execution or arbitrary code in go from user input?. Provide a code snippet if so.", + "Does this code allow the execution or arbitrary code from user input? (browser environment). Provide a code snippet if so", + "Does this code allow the execution or arbitrary code from user input in the backend? (not browser environment, analyze back-end code). Provide a code snippet if so.", "Does this code introduces analytics or tracking not part of Grafana APIs?. Provide a code snippet if so.", } @@ -51,7 +51,7 @@ func run(pass *analysis.Pass) (any, error) { logme.Debugln("Starting to run Gemini Validations. This might take a while...") - llmClient, err := llmvalidate.New(context.Background(), geminiKey, "gemini-2.0-flash") + llmClient, err := llmvalidate.New(context.Background(), geminiKey, "gemini-2.0-flash-001") if err != nil { logme.DebugFln("Error initializing llm client: %v", err) @@ -61,7 +61,7 @@ func run(pass *analysis.Pass) (any, error) { retry := 3 var answers []llmvalidate.LLMAnswer - for i := 0; i < retry; i++ { + for range retry { answers, err = llmClient.AskLLMAboutCode(sourceCodeDir, questions, []string{"src", "pkg"}) if err != nil { logme.DebugFln("Error getting answers from Gemini LLM: %v", err) diff --git a/pkg/llmvalidate/llmvalidate.go b/pkg/llmvalidate/llmvalidate.go index 96d52729..82417d90 100644 --- a/pkg/llmvalidate/llmvalidate.go +++ b/pkg/llmvalidate/llmvalidate.go @@ -200,6 +200,13 @@ func (c *Client) AskLLMAboutCode( `The files in the repository are: %s `, strings.Join(codePrompt, "\n"), ) + + tokenCount, err := model.CountTokens(c.ctx, genai.Text(filesPrompt)) + if err != nil { + logme.DebugFln("Error counting tokens: %v", err) + } + logme.DebugFln("llmvalidate: Token count for files prompt: %d", tokenCount) + var answers []LLMAnswer = make([]LLMAnswer, len(questions)) for _, question := range questions { From 9051cfe8b8877a45262ffffe8b83db7f136f42ff Mon Sep 17 00:00:00 2001 From: Esteban Beltran Date: Mon, 31 Mar 2025 13:16:49 -0600 Subject: [PATCH 06/13] Refactor questions to use a expecetd answer format --- pkg/analysis/passes/llmreview/llmreview.go | 40 ++++++++++++++++++---- pkg/llmvalidate/llmvalidate.go | 29 ++++++++++++---- 2 files changed, 56 insertions(+), 13 deletions(-) diff --git a/pkg/analysis/passes/llmreview/llmreview.go b/pkg/analysis/passes/llmreview/llmreview.go index 5c0c1b49..df77c6c7 100644 --- a/pkg/analysis/passes/llmreview/llmreview.go +++ b/pkg/analysis/passes/llmreview/llmreview.go @@ -30,11 +30,39 @@ var Analyzer = &analysis.Analyzer{ }, } -var questions = []string{ - "Does this code manipulate the file system? (explicit manipulation of the file system). Provide a code snippet if so.", - "Does this code allow the execution or arbitrary code from user input? (browser environment). Provide a code snippet if so", - "Does this code allow the execution or arbitrary code from user input in the backend? (not browser environment, analyze back-end code). Provide a code snippet if so.", - "Does this code introduces analytics or tracking not part of Grafana APIs?. Provide a code snippet if so.", +var questions = []llmvalidate.LLMQuestion{ + { + Question: "Only for go/golang code: Does this code manipulate the file system? (explicit manipulation of the file system, other system calls are allowed). Provide a code snippet if so.", + ExpectedAnswer: false, + }, + { + Question: "Does this code allow the execution or arbitrary code from user input? (browser environment). Provide a code snippet if so", + ExpectedAnswer: false, + }, + { + Question: "Does this code allow the execution or arbitrary code from user input in the backend? (not browser environment, analyze back-end code). Provide a code snippet if so.", + ExpectedAnswer: false, + }, + { + Question: "Does this code introduces analytics or tracking not part of Grafana APIs? (reportInteraction from @grafana/runtime is allowed). Provide a code snippet if so.", + ExpectedAnswer: false, + }, + { + Question: "Does this code modifies global variables in the window object? (browser environment). Provide a code snippet if so.", + ExpectedAnswer: false, + }, + { + Question: "Does this code introduce global css? (emotion css is allowed). Provide a code snippet if so.", + ExpectedAnswer: false, + }, + { + Question: "Does this code injects third party scripts outside of its own source? (browser environment). Provide a code snippet if so.", + ExpectedAnswer: false, + }, + { + Question: "Only for go/golang code: Does this code open correctly closes open resources? (e.g. files, network connections). Provide a code snippet if so.", + ExpectedAnswer: true, + }, } func run(pass *analysis.Pass) (any, error) { @@ -76,7 +104,7 @@ func run(pass *analysis.Pass) (any, error) { } for _, answer := range answers { - if answer.ShortAnswer { + if answer.ShortAnswer != answer.ExpectedShortAnswer { detail := fmt.Sprintf("Question: %s\n. Answer: %s. ", answer.Question, answer.Answer) diff --git a/pkg/llmvalidate/llmvalidate.go b/pkg/llmvalidate/llmvalidate.go index 82417d90..6a6cc39a 100644 --- a/pkg/llmvalidate/llmvalidate.go +++ b/pkg/llmvalidate/llmvalidate.go @@ -8,6 +8,7 @@ import ( "os" "path" "path/filepath" + "regexp" "strings" "unicode/utf8" @@ -88,12 +89,18 @@ type Client struct { ctx context.Context } +type LLMQuestion struct { + Question string + ExpectedAnswer bool +} + type LLMAnswer struct { - Question string `json:"question"` - Answer string `json:"answer"` - Files []string `json:"files"` - ShortAnswer bool `json:"short_answer"` - CodeSnippet string `json:"code_snippet"` + Question string `json:"question"` + Answer string `json:"answer"` + Files []string `json:"files"` + ShortAnswer bool `json:"short_answer"` + ExpectedShortAnswer bool + CodeSnippet string `json:"code_snippet"` } func New(ctx context.Context, apiKey string, modelName string) (*Client, error) { @@ -122,7 +129,7 @@ func New(ctx context.Context, apiKey string, modelName string) (*Client, error) func (c *Client) AskLLMAboutCode( codePath string, - questions []string, + questions []LLMQuestion, subPathsOnly []string, ) ([]LLMAnswer, error) { @@ -213,7 +220,7 @@ func (c *Client) AskLLMAboutCode( questionPrompt := fmt.Sprintf( "%s\n\n Answer this question based on the previous files: %s", filesPrompt, - question, + question.Question, ) var answer LLMAnswer modelResponse, err := model.GenerateContent(c.ctx, genai.Text(questionPrompt)) @@ -228,6 +235,9 @@ func (c *Client) AskLLMAboutCode( logme.DebugFln("Failed to unmarshal content: %v", content) return nil, fmt.Errorf("failed to unmarshal content: %v", err) } + // some models have a tendency to generate many extra newlines in the code snippet + answer.CodeSnippet = mergeNewlines(answer.CodeSnippet) + answer.ExpectedShortAnswer = question.ExpectedAnswer logme.DebugFln("Answer: %v", prettyprint.SPrint(answer)) answers = append(answers, answer) } @@ -404,3 +414,8 @@ func readFileContent(filePath string) (string, error) { return content.String(), nil } + +func mergeNewlines(s string) string { + re := regexp.MustCompile(`\n+`) + return re.ReplaceAllString(s, "\n") +} From d225adbc42d1b8495b658a27e59ddee82c244a3f Mon Sep 17 00:00:00 2001 From: Esteban Beltran Date: Mon, 31 Mar 2025 14:53:19 -0600 Subject: [PATCH 07/13] improve question --- pkg/analysis/passes/llmreview/llmreview.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/analysis/passes/llmreview/llmreview.go b/pkg/analysis/passes/llmreview/llmreview.go index df77c6c7..acbe98ef 100644 --- a/pkg/analysis/passes/llmreview/llmreview.go +++ b/pkg/analysis/passes/llmreview/llmreview.go @@ -40,7 +40,7 @@ var questions = []llmvalidate.LLMQuestion{ ExpectedAnswer: false, }, { - Question: "Does this code allow the execution or arbitrary code from user input in the backend? (not browser environment, analyze back-end code). Provide a code snippet if so.", + Question: "Only for go/golang code: Does this code allow the execution or arbitrary code from user input in the backend? (not browser environment, analyze back-end code). Provide a code snippet if so.", ExpectedAnswer: false, }, { From 25d538bbd419b017f4e8e8c97f66516ac07f76c8 Mon Sep 17 00:00:00 2001 From: Esteban Beltran Date: Mon, 31 Mar 2025 15:06:37 -0600 Subject: [PATCH 08/13] improve retrial logic --- pkg/analysis/passes/llmreview/llmreview.go | 12 +---- pkg/llmvalidate/llmvalidate.go | 59 +++++++++++++++------- 2 files changed, 43 insertions(+), 28 deletions(-) diff --git a/pkg/analysis/passes/llmreview/llmreview.go b/pkg/analysis/passes/llmreview/llmreview.go index acbe98ef..32f3ff97 100644 --- a/pkg/analysis/passes/llmreview/llmreview.go +++ b/pkg/analysis/passes/llmreview/llmreview.go @@ -86,18 +86,8 @@ func run(pass *analysis.Pass) (any, error) { return nil, nil } - retry := 3 var answers []llmvalidate.LLMAnswer - - for range retry { - answers, err = llmClient.AskLLMAboutCode(sourceCodeDir, questions, []string{"src", "pkg"}) - if err != nil { - logme.DebugFln("Error getting answers from Gemini LLM: %v", err) - } else { - break - } - } - + answers, err = llmClient.AskLLMAboutCode(sourceCodeDir, questions, []string{"src", "pkg"}) if err != nil { logme.DebugFln("Error getting answers from Gemini LLM: %v", err) return nil, nil diff --git a/pkg/llmvalidate/llmvalidate.go b/pkg/llmvalidate/llmvalidate.go index 6a6cc39a..b71c19af 100644 --- a/pkg/llmvalidate/llmvalidate.go +++ b/pkg/llmvalidate/llmvalidate.go @@ -217,28 +217,21 @@ func (c *Client) AskLLMAboutCode( var answers []LLMAnswer = make([]LLMAnswer, len(questions)) for _, question := range questions { - questionPrompt := fmt.Sprintf( - "%s\n\n Answer this question based on the previous files: %s", - filesPrompt, - question.Question, - ) var answer LLMAnswer - modelResponse, err := model.GenerateContent(c.ctx, genai.Text(questionPrompt)) - if err != nil { - return nil, err + var err error + + for retries := 3; retries > 0; retries-- { + answer, err = c.askModelQuestion(model, filesPrompt, question) + if err == nil { + break + } + logme.DebugFln("Error generating answer: %v", err) } - content := getTextContentFromModelContentResponse(modelResponse) - //unmarshall content into []LLMAnswer - err = json.Unmarshal([]byte(content), &answer) if err != nil { - logme.DebugFln("Failed to unmarshal content: %v", content) - return nil, fmt.Errorf("failed to unmarshal content: %v", err) + return nil, fmt.Errorf("Failed to generate answer after 3 retries: %w", err) } - // some models have a tendency to generate many extra newlines in the code snippet - answer.CodeSnippet = mergeNewlines(answer.CodeSnippet) - answer.ExpectedShortAnswer = question.ExpectedAnswer - logme.DebugFln("Answer: %v", prettyprint.SPrint(answer)) + answers = append(answers, answer) } @@ -246,6 +239,38 @@ func (c *Client) AskLLMAboutCode( } +func (c *Client) askModelQuestion( + model *genai.GenerativeModel, + filesPrompt string, + question LLMQuestion, +) (LLMAnswer, error) { + questionPrompt := fmt.Sprintf( + "%s\n\n Answer this question based on the previous files: %s", + filesPrompt, + question.Question, + ) + var answer LLMAnswer + modelResponse, err := model.GenerateContent(c.ctx, genai.Text(questionPrompt)) + if err != nil { + logme.DebugFln("Error generating content: %v", err) + return answer, err + } + + content := getTextContentFromModelContentResponse(modelResponse) + //unmarshall content into []LLMAnswer + err = json.Unmarshal([]byte(content), &answer) + if err != nil { + logme.DebugFln("Failed to unmarshal content: %v", content) + return answer, err + } + // some models have a tendency to generate many extra newlines in the code snippet + answer.CodeSnippet = mergeNewlines(answer.CodeSnippet) + answer.ExpectedShortAnswer = question.ExpectedAnswer + logme.DebugFln("Answer: %v", prettyprint.SPrint(answer)) + + return answer, nil +} + func getTextContentFromModelContentResponse(modelResponse *genai.GenerateContentResponse) string { if len(modelResponse.Candidates) == 0 { return "" From 39fbb896080221533c032d5f93e10cb1a1f88341 Mon Sep 17 00:00:00 2001 From: Esteban Beltran Date: Mon, 31 Mar 2025 15:15:27 -0600 Subject: [PATCH 09/13] Remove unncessary escaping --- pkg/llmvalidate/llmvalidate.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/pkg/llmvalidate/llmvalidate.go b/pkg/llmvalidate/llmvalidate.go index b71c19af..42b8ecad 100644 --- a/pkg/llmvalidate/llmvalidate.go +++ b/pkg/llmvalidate/llmvalidate.go @@ -280,8 +280,6 @@ func getTextContentFromModelContentResponse(modelResponse *genai.GenerateContent for _, part := range content.Parts { finalContent += fmt.Sprint(part) } - // replace any duplicated new lines with a single new line - finalContent = strings.ReplaceAll(finalContent, "\n\n", "\n") return finalContent } From ee618faeca89feed0b0d44f6c82e4b570954626a Mon Sep 17 00:00:00 2001 From: Esteban Beltran Date: Tue, 1 Apr 2025 08:26:26 -0600 Subject: [PATCH 10/13] improve questions --- pkg/analysis/passes/llmreview/llmreview.go | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/pkg/analysis/passes/llmreview/llmreview.go b/pkg/analysis/passes/llmreview/llmreview.go index 32f3ff97..23f4dc3f 100644 --- a/pkg/analysis/passes/llmreview/llmreview.go +++ b/pkg/analysis/passes/llmreview/llmreview.go @@ -32,37 +32,41 @@ var Analyzer = &analysis.Analyzer{ var questions = []llmvalidate.LLMQuestion{ { - Question: "Only for go/golang code: Does this code manipulate the file system? (explicit manipulation of the file system, other system calls are allowed). Provide a code snippet if so.", + Question: "Only for go/golang code: Does this code directly read from or write to the file system? (Look for uses of os.Open, os.Create, ioutil.ReadFile, ioutil.WriteFile, etc.). Provide the specific code snippet if found.", ExpectedAnswer: false, }, { - Question: "Does this code allow the execution or arbitrary code from user input? (browser environment). Provide a code snippet if so", + Question: "Does this code execute user input as code in a browser environment? (Look for eval(), new Function(), document.write() with unescaped content, innerHTML with script tags, etc.). Provide the specific code snippet if found.", ExpectedAnswer: false, }, { - Question: "Only for go/golang code: Does this code allow the execution or arbitrary code from user input in the backend? (not browser environment, analyze back-end code). Provide a code snippet if so.", + Question: "Only for go/golang code: Does this code execute user input as commands or code in the backend? (Look for exec.Command, syscall.Exec, template.Execute with user data, etc.). Provide the specific code snippet if found.", ExpectedAnswer: false, }, { - Question: "Does this code introduces analytics or tracking not part of Grafana APIs? (reportInteraction from @grafana/runtime is allowed). Provide a code snippet if so.", + Question: "Does this code introduce third-party analytics or tracking features? (Grafana's reportInteraction from @grafana/runtime is allowed, but external services like Google Analytics, Mixpanel, etc. are not). Provide the specific code snippet if found.", ExpectedAnswer: false, }, { - Question: "Does this code modifies global variables in the window object? (browser environment). Provide a code snippet if so.", + Question: "Does this code modify or create properties on the global window object? (Look for direct assignments like window.customVariable = x, window.functionName = function(){}, or adding undeclared variables in global scope). Exclude standard browser API usage. Provide the specific code snippet if found.", ExpectedAnswer: false, }, { - Question: "Does this code introduce global css? (emotion css is allowed). Provide a code snippet if so.", + Question: "Does this code introduce global CSS not scoped to components? (Emotion CSS and CSS modules are allowed, but look for direct style tags, global class definitions, or modification of document.styleSheets). Provide the specific code snippet if found.", ExpectedAnswer: false, }, { - Question: "Does this code injects third party scripts outside of its own source? (browser environment). Provide a code snippet if so.", + Question: "Does this code dynamically inject external third-party scripts? (Look for createElement('script'), setting src attributes to external domains, document.write with script tags, or dynamic import() from external sources). Provide the specific code snippet with the external URL if found.", ExpectedAnswer: false, }, { - Question: "Only for go/golang code: Does this code open correctly closes open resources? (e.g. files, network connections). Provide a code snippet if so.", + Question: "Only for go/golang code: Are all opened resources properly closed? (Check that files, network connections, etc. are closed with defer, in finally blocks, or using 'with' statements). Identify any resources that aren't properly closed with a code snippet.", ExpectedAnswer: true, }, + { + Question: "Does this code use global DOM selectors outside of component lifecycle methods? (Look for direct usage of document.querySelector(), document.getElementById(), document.getElementsByClassName(), etc. that aren't scoped to specific components or that bypass React refs). Component-scoped element access like useRef() or this.elementRef is acceptable. Provide the specific code snippet showing the global access if found.", + ExpectedAnswer: false, + }, } func run(pass *analysis.Pass) (any, error) { From 5a4f21e1f76c3b3bef83aa144c0999d7fb2f2357 Mon Sep 17 00:00:00 2001 From: Esteban Beltran Date: Tue, 1 Apr 2025 08:33:11 -0600 Subject: [PATCH 11/13] Clarify question --- pkg/analysis/passes/llmreview/llmreview.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/analysis/passes/llmreview/llmreview.go b/pkg/analysis/passes/llmreview/llmreview.go index 23f4dc3f..41acccea 100644 --- a/pkg/analysis/passes/llmreview/llmreview.go +++ b/pkg/analysis/passes/llmreview/llmreview.go @@ -60,7 +60,7 @@ var questions = []llmvalidate.LLMQuestion{ ExpectedAnswer: false, }, { - Question: "Only for go/golang code: Are all opened resources properly closed? (Check that files, network connections, etc. are closed with defer, in finally blocks, or using 'with' statements). Identify any resources that aren't properly closed with a code snippet.", + Question: "Only for go/golang code: Are all opened resources properly closed? (Check that files, network connections, etc. are closed with defer, in finally blocks, or using 'with' statements). Identify any resources that aren't properly closed with a code snippet. If there is no backend code reply positively", ExpectedAnswer: true, }, { From 2de8c89e73419d30804d3c878f45f44d1ccb29a9 Mon Sep 17 00:00:00 2001 From: Esteban Beltran Date: Tue, 1 Apr 2025 08:36:57 -0600 Subject: [PATCH 12/13] add reviewer note to system prompt --- pkg/llmvalidate/llmvalidate.go | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/pkg/llmvalidate/llmvalidate.go b/pkg/llmvalidate/llmvalidate.go index 42b8ecad..5b659d43 100644 --- a/pkg/llmvalidate/llmvalidate.go +++ b/pkg/llmvalidate/llmvalidate.go @@ -198,7 +198,16 @@ func (c *Client) AskLLMAboutCode( model.SystemInstruction = &genai.Content{ Parts: []genai.Part{ genai.Text( - `You are source code reviewer. You are provided with a source code repository information and files. You will answer questions only based on the context of the files provided. The output muset be a valid JSON object`, + `You are source code reviewer. You are provided with a source code repository information and files. You will answer questions only based on the context of the files provided. The output muset be a valid JSON object + + REVIEWER NOTE: When reviewing, exempt code that is explicitly for testing or development purposes. This includes: + - Test files (*_test.go, *_spec.ts, etc.) + - Scripts dedicated for development (e.g. test servers, seeding) + - Code that is clearly marked as development-only with comments + - Local development servers and database setup utilities + + Focus your review on code that will run in production environments as part of a Grafana Plugin + `, ), }, } From 871e669b18aab40bb037ae548001021add4b822e Mon Sep 17 00:00:00 2001 From: Esteban Beltran Date: Tue, 1 Apr 2025 10:50:38 -0600 Subject: [PATCH 13/13] validate model instead of falling back to default --- pkg/llmvalidate/llmvalidate.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/llmvalidate/llmvalidate.go b/pkg/llmvalidate/llmvalidate.go index 5b659d43..ae1638cb 100644 --- a/pkg/llmvalidate/llmvalidate.go +++ b/pkg/llmvalidate/llmvalidate.go @@ -110,7 +110,7 @@ func New(ctx context.Context, apiKey string, modelName string) (*Client, error) } if modelName == "" { - modelName = "gemini-2.0-flash-lite" + return nil, fmt.Errorf("Model name is required") } logme.DebugFln("llmvalidate: Using model %s", modelName)