From ab7521477a3989c8a29ae1f8d503a588e72f18ef Mon Sep 17 00:00:00 2001 From: Maximillian von Briesen Date: Mon, 8 May 2023 20:14:19 -0400 Subject: [PATCH] begin implementing comments (#2) add a section of the main loop that checks for comments on PRs that the bot has opened reworks git logic and adds debug commands for git and llm stuff changes a bunch of other stuff too --- cmd/debug-git.go | 54 ++++ cmd/{debug.go => debug-llm.go} | 12 +- cmd/root.go | 4 +- llm/common.go | 79 +---- llm/diffcomment.go | 90 ++++++ llm/issue.go | 71 +++++ llm/openai.go | 50 +++- llm/prompts/code-change-request.tmpl | 6 +- llm/prompts/comment-diff-request.tmpl | 16 +- pullpal/common.go | 403 ++++++++++---------------- pullpal/debug.go | 97 ++++++- vc/common.go | 9 +- vc/git.go | 58 ++-- vc/github.go | 100 +++++-- 14 files changed, 644 insertions(+), 405 deletions(-) create mode 100644 cmd/debug-git.go rename cmd/{debug.go => debug-llm.go} (65%) create mode 100644 llm/diffcomment.go create mode 100644 llm/issue.go diff --git a/cmd/debug-git.go b/cmd/debug-git.go new file mode 100644 index 0000000..c0e79fa --- /dev/null +++ b/cmd/debug-git.go @@ -0,0 +1,54 @@ +package cmd + +import ( + "fmt" + + "github.com/spf13/cobra" +) + +var debugGitCmd = &cobra.Command{ + Use: "debug-git", + Short: "debug git functionality", + Run: func(cmd *cobra.Command, args []string) { + cfg := getConfig() + + p, err := getPullPal(cmd.Context(), cfg) + if err != nil { + fmt.Println("error creating new pull pal", err) + return + } + fmt.Println("Successfully initialized pull pal") + + err = p.DebugGit() + if err != nil { + fmt.Println("err debugging git", err) + return + } + }, +} + +var debugGithubCmd = &cobra.Command{ + Use: "debug-github", + Short: "debug github functionality", + Run: func(cmd *cobra.Command, args []string) { + cfg := getConfig() + + p, err := getPullPal(cmd.Context(), cfg) + if err != nil { + fmt.Println("error creating new pull pal", err) + return + } + fmt.Println("Successfully initialized pull pal") + + err = p.DebugGithub(cfg.usersToListenTo) + if err != nil { + fmt.Println("err debugging github", err) + return + } + }, +} + +func init() { + rootCmd.AddCommand(debugGitCmd) + rootCmd.AddCommand(debugGithubCmd) +} diff --git a/cmd/debug.go b/cmd/debug-llm.go similarity index 65% rename from cmd/debug.go rename to cmd/debug-llm.go index bd38088..2e18889 100644 --- a/cmd/debug.go +++ b/cmd/debug-llm.go @@ -6,9 +6,9 @@ import ( "github.com/spf13/cobra" ) -var debugGitCmd = &cobra.Command{ - Use: "debug-git", - Short: "debug git functionality", +var debugLLMCmd = &cobra.Command{ + Use: "debug-llm", + Short: "debug llm functionality", Run: func(cmd *cobra.Command, args []string) { cfg := getConfig() @@ -19,14 +19,14 @@ var debugGitCmd = &cobra.Command{ } fmt.Println("Successfully initialized pull pal") - err = p.DebugGit() + err = p.DebugLLM() if err != nil { - fmt.Println("err debugging git", err) + fmt.Println("err debugging prompts", err) return } }, } func init() { - rootCmd.AddCommand(debugGitCmd) + rootCmd.AddCommand(debugLLMCmd) } diff --git a/cmd/root.go b/cmd/root.go index 1d1b244..04d76e6 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -7,6 +7,7 @@ import ( "github.com/mobyvb/pull-pal/pullpal" "github.com/mobyvb/pull-pal/vc" + "github.com/sashabaranov/go-openai" "go.uber.org/zap" "github.com/spf13/cobra" @@ -79,7 +80,8 @@ func getPullPal(ctx context.Context, cfg config) (*pullpal.PullPal, error) { Handles: cfg.usersToListenTo, Labels: cfg.requiredIssueLabels, } - p, err := pullpal.NewPullPal(ctx, log.Named("pullpal"), listIssueOptions, author, repo, cfg.openAIToken) + // TODO make model configurable + p, err := pullpal.NewPullPal(ctx, log.Named("pullpal"), listIssueOptions, author, repo, openai.GPT4, cfg.openAIToken) return p, err } diff --git a/llm/common.go b/llm/common.go index c89f6c0..00a1c33 100644 --- a/llm/common.go +++ b/llm/common.go @@ -1,8 +1,6 @@ package llm import ( - "bytes" - "html/template" "strings" ) @@ -12,6 +10,13 @@ type File struct { Contents string } +type ResponseType int + +const ( + ResponseAnswer ResponseType = iota + ResponseCodeChange +) + // CodeChangeRequest contains all necessary information for generating a prompt for a LLM. type CodeChangeRequest struct { Files []File @@ -20,75 +25,23 @@ type CodeChangeRequest struct { IssueID string } -// String is the string representation of a CodeChangeRequest. Functionally, it contains the LLM prompt. -func (req CodeChangeRequest) String() string { - prompt := req.MustGetPrompt() - return "START OF PROMPT\n" + prompt + "\nEND OF PROMPT" -} - -// MustGetPrompt only returns the prompt, but panics if the data in the request cannot populate the template. -func (req CodeChangeRequest) MustGetPrompt() string { - prompt, err := req.GetPrompt() - if err != nil { - panic(err) - } - return prompt -} - -// GetPrompt converts the information in the request to a prompt for an LLM. -func (req CodeChangeRequest) GetPrompt() (string, error) { - tmpl, err := template.ParseFiles("./llm/prompts/code-change-request.tmpl") - if err != nil { - return "", err - } - - var result bytes.Buffer - err = tmpl.Execute(&result, req) - if err != nil { - return "", err - } - - return result.String(), nil -} - // CodeChangeResponse contains data derived from an LLM response to a prompt generated via a CodeChangeRequest. type CodeChangeResponse struct { Files []File Notes string } -// String is a string representation of CodeChangeResponse. -func (res CodeChangeResponse) String() string { - out := "Notes:\n" - out += res.Notes + "\n\n" - out += "Files:\n" - for _, f := range res.Files { - out += f.Path + ":\n```\n" - out += f.Contents + "\n```\n" - } - - return out +// TODO support threads +type DiffCommentRequest struct { + File File + Contents string + Diff string } -// ParseCodeChangeResponse parses the LLM's response to CodeChangeRequest (string) into a CodeChangeResponse. -func ParseCodeChangeResponse(llmResponse string) CodeChangeResponse { - sections := strings.Split(llmResponse, "ppnotes:") - - filesSection := "" - if len(sections) > 0 { - filesSection = sections[0] - } - notes := "" - if len(sections) > 1 { - notes = strings.TrimSpace(sections[1]) - } - - files := parseFiles(filesSection) - - return CodeChangeResponse{ - Files: files, - Notes: notes, - } +type DiffCommentResponse struct { + Type ResponseType + Answer string + File File } // parseFiles process the "files" subsection of the LLM's response. It is a helper for GetCodeChangeResponse. diff --git a/llm/diffcomment.go b/llm/diffcomment.go new file mode 100644 index 0000000..468e8f4 --- /dev/null +++ b/llm/diffcomment.go @@ -0,0 +1,90 @@ +package llm + +import ( + "bytes" + "strings" + "text/template" +) + +func (req DiffCommentRequest) String() string { + return req.MustGetPrompt() +} + +// MustGetPrompt only returns the prompt, but panics if the data in the request cannot populate the template. +func (req DiffCommentRequest) MustGetPrompt() string { + prompt, err := req.GetPrompt() + if err != nil { + panic(err) + } + return prompt +} + +// GetPrompt converts the information in the request to a prompt for an LLM. +func (req DiffCommentRequest) GetPrompt() (string, error) { + tmpl, err := template.ParseFiles("./llm/prompts/comment-diff-request.tmpl") + if err != nil { + return "", err + } + + var result bytes.Buffer + err = tmpl.Execute(&result, req) + if err != nil { + return "", err + } + + return result.String(), nil +} + +// String is a string representation of DiffCommentResponse. +func (res DiffCommentResponse) String() string { + out := "" + if res.Type == ResponseAnswer { + out += "Type: Answer\n" + out += res.Answer + return out + } + + out += "Type: Code Change\n" + + out += "Response:\n" + out += res.Answer + "\n\n" + out += "Files:\n" + out += res.File.Path + ":\n```\n" + out += res.File.Contents + "\n```\n" + + return out +} + +func ParseDiffCommentResponse(llmResponse string) DiffCommentResponse { + llmResponse = strings.TrimSpace(llmResponse) + if llmResponse[0] == 'A' { + answer := strings.TrimSpace(llmResponse[1:]) + return DiffCommentResponse{ + Type: ResponseAnswer, + Answer: answer, + } + } + parts := strings.Split(llmResponse, "ppresponse:") + + filesSection := "" + if len(parts) > 0 { + filesSection = parts[0] + } + + answer := "" + if len(parts) > 1 { + answer = strings.TrimSpace(parts[1]) + } + + files := parseFiles(filesSection) + f := File{} + if len(files) > 0 { + f = files[0] + } + + return DiffCommentResponse{ + Type: ResponseCodeChange, + Answer: answer, + File: f, + } +} diff --git a/llm/issue.go b/llm/issue.go new file mode 100644 index 0000000..15d78ed --- /dev/null +++ b/llm/issue.go @@ -0,0 +1,71 @@ +package llm + +import ( + "bytes" + "strings" + "text/template" +) + +// String is the string representation of a CodeChangeRequest. Functionally, it contains the LLM prompt. +func (req CodeChangeRequest) String() string { + return req.MustGetPrompt() +} + +// MustGetPrompt only returns the prompt, but panics if the data in the request cannot populate the template. +func (req CodeChangeRequest) MustGetPrompt() string { + prompt, err := req.GetPrompt() + if err != nil { + panic(err) + } + return prompt +} + +// GetPrompt converts the information in the request to a prompt for an LLM. +func (req CodeChangeRequest) GetPrompt() (string, error) { + tmpl, err := template.ParseFiles("./llm/prompts/code-change-request.tmpl") + if err != nil { + return "", err + } + + var result bytes.Buffer + err = tmpl.Execute(&result, req) + if err != nil { + return "", err + } + + return result.String(), nil +} + +// String is a string representation of CodeChangeResponse. +func (res CodeChangeResponse) String() string { + out := "Notes:\n" + out += res.Notes + "\n\n" + out += "Files:\n" + for _, f := range res.Files { + out += f.Path + ":\n```\n" + out += f.Contents + "\n```\n" + } + + return out +} + +// ParseCodeChangeResponse parses the LLM's response to CodeChangeRequest (string) into a CodeChangeResponse. +func ParseCodeChangeResponse(llmResponse string) CodeChangeResponse { + sections := strings.Split(llmResponse, "ppnotes:") + + filesSection := "" + if len(sections) > 0 { + filesSection = sections[0] + } + notes := "" + if len(sections) > 1 { + notes = strings.TrimSpace(sections[1]) + } + + files := parseFiles(filesSection) + + return CodeChangeResponse{ + Files: files, + Notes: notes, + } +} diff --git a/llm/openai.go b/llm/openai.go index 688df95..2c3fed1 100644 --- a/llm/openai.go +++ b/llm/openai.go @@ -8,24 +8,27 @@ import ( ) type OpenAIClient struct { - log *zap.Logger - client *openai.Client + log *zap.Logger + client *openai.Client + defaultModel string } -func NewOpenAIClient(log *zap.Logger, token string) *OpenAIClient { +func NewOpenAIClient(log *zap.Logger, defaultModel, token string) *OpenAIClient { return &OpenAIClient{ - log: log, - client: openai.NewClient(token), + log: log, + client: openai.NewClient(token), + defaultModel: defaultModel, } } -func (oc *OpenAIClient) EvaluateCCR(ctx context.Context, req CodeChangeRequest) (res CodeChangeResponse, err error) { +func (oc *OpenAIClient) EvaluateCCR(ctx context.Context, model string, req CodeChangeRequest) (res CodeChangeResponse, err error) { + if model == "" { + model = oc.defaultModel + } resp, err := oc.client.CreateChatCompletion( ctx, openai.ChatCompletionRequest{ - // TODO make model configurable - Model: openai.GPT4, - //Model: openai.GPT3Dot5Turbo, + Model: model, Messages: []openai.ChatCompletionMessage{ { Role: openai.ChatMessageRoleUser, @@ -46,3 +49,32 @@ func (oc *OpenAIClient) EvaluateCCR(ctx context.Context, req CodeChangeRequest) return ParseCodeChangeResponse(choice), nil } + +func (oc *OpenAIClient) EvaluateDiffComment(ctx context.Context, model string, req DiffCommentRequest) (res DiffCommentResponse, err error) { + if model == "" { + model = oc.defaultModel + } + resp, err := oc.client.CreateChatCompletion( + ctx, + openai.ChatCompletionRequest{ + Model: model, + Messages: []openai.ChatCompletionMessage{ + { + Role: openai.ChatMessageRoleUser, + Content: req.String(), + }, + }, + }, + ) + if err != nil { + oc.log.Error("chat completion error", zap.Error(err)) + return res, err + } + + choice := resp.Choices[0].Message.Content + + // TODO make debug log when I figure out how to config that + oc.log.Info("got response from llm", zap.String("output", choice)) + + return ParseDiffCommentResponse(choice), nil +} diff --git a/llm/prompts/code-change-request.tmpl b/llm/prompts/code-change-request.tmpl index 44d2194..923ca6b 100644 --- a/llm/prompts/code-change-request.tmpl +++ b/llm/prompts/code-change-request.tmpl @@ -15,9 +15,9 @@ Body: Respond in the exact format: Files: {{ range $index, $file := .Files }} - ppname: {{ $file.Path }} - ppcontents: - [new {{ $file.Path }} contents] +ppname: {{ $file.Path }} +ppcontents: +[new {{ $file.Path }} contents] {{ end }} ppnotes: diff --git a/llm/prompts/comment-diff-request.tmpl b/llm/prompts/comment-diff-request.tmpl index de3c5b5..604d54a 100644 --- a/llm/prompts/comment-diff-request.tmpl +++ b/llm/prompts/comment-diff-request.tmpl @@ -1,21 +1,21 @@ File: - - name: {{ .Path }}: + - name: {{ .File.Path }}: contents: ``` -{{ .Contents }} +{{ .File.Contents }} ``` Diff: {{ .Diff }} Comment: -{{ .Comment }} +{{ .Contents }} The above is information about a comment left on a file. The diff contains information about the precise location of the comment. First, determine if the comment is a question or a request for changes. -If the comment is a question, come up with an answer, and respond exactly as outlined in "Response Template A" -If the comment is a request, modify the file provided at the beginning of the message, and respond exactly as outlined in "Response Template B". +If the comment is a question, come up with an answer, and respond exactly as outlined directly below "Response Template A" +If the comment is a request, modify the file provided at the beginning of the message, and respond exactly as outlined directly below "Response Template B". Response Template A: Q @@ -24,9 +24,9 @@ Q Response Template B: R Files: - ppname: {{ .Path }} - ppcontents: - [new {{ .Path }} contents] +ppname: {{ .File.Path }} +ppcontents: +[new {{ .File.Path }} contents] ppresponse: [additional context about your changes] diff --git a/pullpal/common.go b/pullpal/common.go index 9457402..5d28431 100644 --- a/pullpal/common.go +++ b/pullpal/common.go @@ -33,7 +33,7 @@ type PullPal struct { } // NewPullPal creates a new "pull pal service", including setting up local version control and LLM integrations. -func NewPullPal(ctx context.Context, log *zap.Logger, listIssueOptions vc.ListIssueOptions, self vc.Author, repo vc.Repository, openAIToken string) (*PullPal, error) { +func NewPullPal(ctx context.Context, log *zap.Logger, listIssueOptions vc.ListIssueOptions, self vc.Author, repo vc.Repository, model string, openAIToken string) (*PullPal, error) { ghClient, err := vc.NewGithubClient(ctx, log, self, repo) if err != nil { return nil, err @@ -50,7 +50,7 @@ func NewPullPal(ctx context.Context, log *zap.Logger, listIssueOptions vc.ListIs ghClient: ghClient, localGitClient: localGitClient, - openAIClient: llm.NewOpenAIClient(log.Named("openaiClient"), openAIToken), + openAIClient: llm.NewOpenAIClient(log.Named("openaiClient"), model, openAIToken), }, nil } @@ -59,6 +59,7 @@ func (p *PullPal) Run() error { p.log.Info("Starting Pull Pal") // TODO gracefully handle context cancelation for { + p.log.Info("checking github issues...") issues, err := p.ghClient.ListOpenIssues(p.listIssueOptions) if err != nil { p.log.Error("error listing issues", zap.Error(err)) @@ -66,178 +67,66 @@ func (p *PullPal) Run() error { } if len(issues) == 0 { - // todo don't sleep - p.log.Info("no issues found. sleeping for 30 seconds") - time.Sleep(30 * time.Second) - continue - } + p.log.Info("no issues found") + } else { + p.log.Info("picked issue to process") - issue := issues[0] - issueNumber, err := strconv.Atoi(issue.ID) - if err != nil { - p.log.Error("error converting issue ID to int", zap.Error(err)) - return err - } - - err = p.ghClient.CommentOnIssue(issueNumber, "on it") - if err != nil { - p.log.Error("error commenting on issue", zap.Error(err)) - return err - } - for _, label := range p.listIssueOptions.Labels { - err = p.ghClient.RemoveLabelFromIssue(issueNumber, label) + issue := issues[0] + err = p.handleIssue(issue) if err != nil { - p.log.Error("error removing labels from issue", zap.Error(err)) - return err + p.log.Error("error handling issue", zap.Error(err)) } } - // remove file list from issue body - // TODO do this better and probably somewhere else - parts := strings.Split(issue.Body, "Files:") - issue.Body = parts[0] - - fileList := []string{} - if len(parts) > 1 { - fileList = strings.Split(parts[1], ",") - } - - // get file contents from local git repository - files := []llm.File{} - for _, path := range fileList { - path = strings.TrimSpace(path) - nextFile, err := p.localGitClient.GetLocalFile(path) - if err != nil { - p.log.Error("error getting file from vcclient", zap.Error(err)) - continue - } - files = append(files, nextFile) - } - - changeRequest := llm.CodeChangeRequest{ - Subject: issue.Subject, - Body: issue.Body, - IssueID: issue.ID, - Files: files, - } - - changeResponse, err := p.openAIClient.EvaluateCCR(p.ctx, changeRequest) + p.log.Info("checking pr comments...") + comments, err := p.ghClient.ListOpenComments(vc.ListCommentOptions{ + Handles: p.listIssueOptions.Handles, + }) if err != nil { - p.log.Error("error getting response from openai", zap.Error(err)) - continue - - } - - // parse llm response - //codeChangeResponse := llm.ParseCodeChangeResponse(llmResponse) - - // create commit with file changes - err = p.localGitClient.StartCommit() - //err = p.ghClient.StartCommit() - if err != nil { - p.log.Error("error starting commit", zap.Error(err)) - return err - } - newBranchName := fmt.Sprintf("fix-%s", changeRequest.IssueID) - /* - err = p.localGitClient.SwitchBranch(newBranchName) - if err != nil { - p.log.Error("error switching branch", zap.Error(err)) - return err - } - */ - for _, f := range changeResponse.Files { - p.log.Info("replacing or adding file", zap.String("path", f.Path), zap.String("contents", f.Contents)) - err = p.localGitClient.ReplaceOrAddLocalFile(f) - // err = p.ghClient.ReplaceOrAddLocalFile(f) - if err != nil { - p.log.Error("error replacing or adding file", zap.Error(err)) - return err - } - } - - commitMessage := changeRequest.Subject + "\n\n" + changeResponse.Notes + "\n\nResolves: #" + changeRequest.IssueID - p.log.Info("about to create commit", zap.String("message", commitMessage)) - err = p.localGitClient.FinishCommit(commitMessage) - if err != nil { - p.log.Error("error finishing commit", zap.Error(err)) - // TODO figure out why sometimes finish commit returns "already up-to-date error" - // return err - } - - err = p.localGitClient.PushBranch(newBranchName) - if err != nil { - p.log.Error("error pushing branch", zap.Error(err)) + p.log.Error("error listing comments", zap.Error(err)) return err } - // open code change request - // TODO don't hardcode main branch, make configurable - _, url, err := p.ghClient.OpenCodeChangeRequest(changeRequest, changeResponse, newBranchName, "main") - if err != nil { - p.log.Error("error opening PR", zap.Error(err)) - return err - } - p.log.Info("successfully created PR", zap.String("URL", url)) + if len(comments) == 0 { + p.log.Info("no comments found") + } else { + p.log.Info("picked comment to process") - p.log.Info("going to sleep for thirty seconds") + comment := comments[0] + err = p.handleComment(comment) + if err != nil { + p.log.Error("error handling comment", zap.Error(err)) + } + } + + // TODO remove sleep + p.log.Info("sleeping 30s") time.Sleep(30 * time.Second) } - - return nil } -/* - -// PickIssueToFile is the same as PickIssue, but the changeRequest is converted to a string and written to a file. -func (p *PullPal) PickIssueToFile(promptPath string) (issue vc.Issue, changeRequest llm.CodeChangeRequest, err error) { - issue, changeRequest, err = p.PickIssue() +func (p *PullPal) handleIssue(issue vc.Issue) error { + issueNumber, err := strconv.Atoi(issue.ID) if err != nil { - return issue, changeRequest, err + p.log.Error("error converting issue ID to int", zap.Error(err)) + return err } - prompt, err := changeRequest.GetPrompt() + err = p.ghClient.CommentOnIssue(issueNumber, "on it") if err != nil { - return issue, changeRequest, err + p.log.Error("error commenting on issue", zap.Error(err)) + return err } - - err = ioutil.WriteFile(promptPath, []byte(prompt), 0644) - return issue, changeRequest, err -} - -// PickIssueToClipboard is the same as PickIssue, but the changeRequest is converted to a string and copied to the clipboard. -func (p *PullPal) PickIssueToClipboard() (issue vc.Issue, changeRequest llm.CodeChangeRequest, err error) { - issue, changeRequest, err = p.PickIssue() - if err != nil { - return issue, changeRequest, err + for _, label := range p.listIssueOptions.Labels { + err = p.ghClient.RemoveLabelFromIssue(issueNumber, label) + if err != nil { + p.log.Error("error removing labels from issue", zap.Error(err)) + return err + } } - prompt, err := changeRequest.GetPrompt() - if err != nil { - return issue, changeRequest, err - } - - err = clipboard.WriteAll(prompt) - return issue, changeRequest, err -} -*/ -/* -// PickIssue selects an issue from the version control server and returns the selected issue, as well as the LLM prompt needed to fulfill the request. -func (p *PullPal) PickIssue() (issue vc.Issue, changeRequest llm.CodeChangeRequest, err error) { - // TODO I should be able to pass in settings for listing issues from here - issues, err := p.ghClient.ListOpenIssues(p.listIssueOptions) - if err != nil { - return issue, changeRequest, err - } - - if len(issues) == 0 { - return issue, changeRequest, IssueNotFound - } - - issue = issues[0] - // remove file list from issue body - // TODO do this better + // TODO do this better and probably somewhere else parts := strings.Split(issue.Body, "Files:") issue.Body = parts[0] @@ -250,103 +139,7 @@ func (p *PullPal) PickIssue() (issue vc.Issue, changeRequest llm.CodeChangeReque files := []llm.File{} for _, path := range fileList { path = strings.TrimSpace(path) - nextFile, err := p.ghClient.GetLocalFile(path) - if err != nil { - return issue, changeRequest, err - } - files = append(files, nextFile) - } - - changeRequest.Subject = issue.Subject - changeRequest.Body = issue.Body - changeRequest.IssueID = issue.ID - changeRequest.Files = files - - return issue, changeRequest, nil -} -*/ -/* -// ProcessResponseFromFile is the same as ProcessResponse, but the response is inputted into a file rather than passed directly as an argument. -func (p *PullPal) ProcessResponseFromFile(codeChangeRequest llm.CodeChangeRequest, llmResponsePath string) (url string, err error) { - data, err := ioutil.ReadFile(llmResponsePath) - if err != nil { - return "", err - } - return p.ProcessResponse(codeChangeRequest, string(data)) -} - -// ProcessResponse parses the llm response, updates files in the local git repo accordingly, and opens a new code change request (e.g. Github PR). -func (p *PullPal) ProcessResponse(codeChangeRequest llm.CodeChangeRequest, llmResponse string) (url string, err error) { - // 1. parse llm response - codeChangeResponse := llm.ParseCodeChangeResponse(llmResponse) - - // 2. create commit with file changes - err = p.ghClient.StartCommit() - if err != nil { - return "", err - } - for _, f := range codeChangeResponse.Files { - err = p.ghClient.ReplaceOrAddLocalFile(f) - if err != nil { - return "", err - } - } - - commitMessage := codeChangeRequest.Subject + "\n\n" + codeChangeResponse.Notes + "\n\nResolves: #" + codeChangeRequest.IssueID - err = p.ghClient.FinishCommit(commitMessage) - if err != nil { - return "", err - } - - // 3. open code change request - _, url, err = p.ghClient.OpenCodeChangeRequest(codeChangeRequest, codeChangeResponse) - return url, err -} -*/ - -/* -// ListIssues gets a list of all issues meeting the provided criteria. -func (p *PullPal) ListIssues(handles, labels []string) ([]vc.Issue, error) { - issues, err := p.ghClient.ListOpenIssues(vc.ListIssueOptions{ - Handles: handles, - Labels: labels, - }) - if err != nil { - return nil, err - } - - return issues, nil -} - -// ListComments gets a list of all comments meeting the provided criteria on a PR. -func (p *PullPal) ListComments(changeID string, handles []string) ([]vc.Comment, error) { - comments, err := p.ghClient.ListOpenComments(vc.ListCommentOptions{ - ChangeID: changeID, - Handles: handles, - }) - if err != nil { - return nil, err - } - - return comments, nil -} - -func (p *PullPal) MakeLocalChange(issue vc.Issue) error { - // remove file list from issue body - // TODO do this better - parts := strings.Split(issue.Body, "Files:") - issue.Body = parts[0] - - fileList := []string{} - if len(parts) > 1 { - fileList = strings.Split(parts[1], ",") - } - - // get file contents from local git repository - files := []llm.File{} - for _, path := range fileList { - path = strings.TrimSpace(path) - nextFile, err := p.ghClient.GetLocalFile(path) + nextFile, err := p.localGitClient.GetLocalFile(path) if err != nil { return err } @@ -360,14 +153,118 @@ func (p *PullPal) MakeLocalChange(issue vc.Issue) error { Files: files, } - res, err := p.openAIClient.EvaluateCCR(p.ctx, changeRequest) + changeResponse, err := p.openAIClient.EvaluateCCR(p.ctx, "", changeRequest) + if err != nil { + return err + + } + + // create commit with file changes + err = p.localGitClient.StartCommit() + if err != nil { + return err + } + // todo remove hardcoded main + p.log.Info("checking out main branch") + err = p.localGitClient.CheckoutRemoteBranch("main") + if err != nil { + p.log.Info("error checking out main branch", zap.Error(err)) + return err + } + + newBranchName := fmt.Sprintf("fix-%s", changeRequest.IssueID) + for _, f := range changeResponse.Files { + p.log.Info("replacing or adding file", zap.String("path", f.Path), zap.String("contents", f.Contents)) + err = p.localGitClient.ReplaceOrAddLocalFile(f) + if err != nil { + return err + } + } + + commitMessage := changeRequest.Subject + "\n\n" + changeResponse.Notes + "\n\nResolves: #" + changeRequest.IssueID + p.log.Info("about to create commit", zap.String("message", commitMessage)) + err = p.localGitClient.FinishCommit(commitMessage) if err != nil { return err } - fmt.Println("response from openai") - fmt.Println(res) + p.log.Info("pushing to branch", zap.String("branchname", newBranchName)) + err = p.localGitClient.PushBranch(newBranchName) + if err != nil { + p.log.Info("error pushing to branch", zap.Error(err)) + return err + } + + // open code change request + // TODO don't hardcode main branch, make configurable + _, url, err := p.ghClient.OpenCodeChangeRequest(changeRequest, changeResponse, newBranchName, "main") + if err != nil { + return err + } + p.log.Info("successfully created PR", zap.String("URL", url)) + + return nil +} + +func (p *PullPal) handleComment(comment vc.Comment) error { + if comment.Branch == "" { + return errors.New("no branch provided in comment") + } + + file, err := p.localGitClient.GetLocalFile(comment.FilePath) + if err != nil { + return err + } + + diffCommentRequest := llm.DiffCommentRequest{ + File: file, + Contents: comment.Body, + Diff: comment.DiffHunk, + } + p.log.Info("diff comment request", zap.String("req", diffCommentRequest.String())) + + diffCommentResponse, err := p.openAIClient.EvaluateDiffComment(p.ctx, "", diffCommentRequest) + if err != nil { + return err + } + + if diffCommentResponse.Type == llm.ResponseCodeChange { + p.log.Info("about to start commit") + err = p.localGitClient.StartCommit() + if err != nil { + return err + } + p.log.Info("checking out branch", zap.String("name", comment.Branch)) + err = p.localGitClient.CheckoutRemoteBranch(comment.Branch) + if err != nil { + return err + } + p.log.Info("replacing or adding file", zap.String("path", diffCommentResponse.File.Path), zap.String("contents", diffCommentResponse.File.Contents)) + err = p.localGitClient.ReplaceOrAddLocalFile(diffCommentResponse.File) + if err != nil { + return err + } + + commitMessage := "update based on comment" + p.log.Info("about to create commit", zap.String("message", commitMessage)) + err = p.localGitClient.FinishCommit(commitMessage) + if err != nil { + return err + } + + err = p.localGitClient.PushBranch(comment.Branch) + if err != nil { + return err + } + } + + err = p.ghClient.RespondToComment(comment.PRNumber, comment.ID, diffCommentResponse.Answer) + if err != nil { + p.log.Error("error commenting on issue", zap.Error(err)) + return err + } + + p.log.Info("responded addressed comment") return nil } -*/ diff --git a/pullpal/debug.go b/pullpal/debug.go index 2388e0c..75600dd 100644 --- a/pullpal/debug.go +++ b/pullpal/debug.go @@ -1,9 +1,9 @@ package pullpal import ( - "fmt" - "github.com/mobyvb/pull-pal/llm" + "github.com/mobyvb/pull-pal/vc" + "github.com/sashabaranov/go-openai" "go.uber.org/zap" ) @@ -17,7 +17,7 @@ func (p *PullPal) DebugGit() error { p.log.Error("error starting commit", zap.Error(err)) return err } - newBranchName := fmt.Sprintf("debug-branch") + newBranchName := "debug-branch" for _, f := range []string{"a", "b"} { err = p.localGitClient.ReplaceOrAddLocalFile(llm.File{ @@ -45,3 +45,94 @@ func (p *PullPal) DebugGit() error { return nil } + +// todo dont require args for listing comments +func (p *PullPal) DebugGithub(handles []string) error { + p.log.Info("Starting Pull Pal Github debug") + + issues, err := p.ghClient.ListOpenIssues(p.listIssueOptions) + if err != nil { + p.log.Error("error listing issues", zap.Error(err)) + return err + } + for _, i := range issues { + p.log.Info("got issue", zap.String("issue", i.String())) + } + + comments, err := p.ghClient.ListOpenComments(vc.ListCommentOptions{ + Handles: handles, + }) + if err != nil { + p.log.Error("error listing comments", zap.Error(err)) + return err + } + for _, c := range comments { + p.log.Info("got comment", zap.String("comment", c.String())) + } + + return nil +} + +func (p *PullPal) DebugLLM() error { + p.log.Info("Starting Pull Pal llm debug") + + file := llm.File{ + Path: "main.go", + Contents: `package main\n\nimport (\n "net/http"\n)\n\nfunc main() {\n fs := http.FileServer(http.Dir("static"))\n http.Handle("/", fs)\n\n http.ListenAndServe(":7777", nil)\n}\n\n\n \n\n \n `, + } + + codeChangeRequest := llm.CodeChangeRequest{ + Files: []llm.File{file}, + Subject: "update port and add endpoint", + Body: "use port 8080 for the server in main.go. Also add an endpoint at GET /api/numbers that returns a random integer between 2 and 10", + IssueID: "1234", + } + + p.log.Info("CODE CHANGE REQUEST", zap.String("request", codeChangeRequest.String())) + + diffCommentRequestChange := llm.DiffCommentRequest{ + File: file, + Contents: "remove this unnecessary whitespace at the end", + Diff: "@@ -0,0 +1,15 @@\n+package main\n+ \n+ import (\n+ \"net/http\"\n+ )\n+ \n+ func main() {\n+ fs := http.FileServer(http.Dir(\"static\"))\n+ http.Handle(\"/\", fs)\n+ \n+ http.ListenAndServe(\":7777\", nil)\n+ }\n+", + } + p.log.Info("DIFF COMMENT REQUEST CODECHANGE", zap.String("request", diffCommentRequestChange.String())) + + diffCommentRequestQuestion := llm.DiffCommentRequest{ + File: file, + Contents: "what does this Handle line do?", + Diff: "@@ -0,0 +1,15 @@\n+package main\n+ \n+ import (\n+ \"net/http\"\n+ )\n+ \n+ func main() {\n+ fs := http.FileServer(http.Dir(\"static\"))\n+ http.Handle(\"/\", fs)\n", + } + p.log.Info("DIFF COMMENT REQUEST QUESTION", zap.String("request", diffCommentRequestQuestion.String())) + + for _, m := range []string{openai.GPT3Dot5Turbo, openai.GPT4} { + p.log.Info("testing with openai api", zap.String("MODEL", m)) + + p.log.Info("testing code change request") + res, err := p.openAIClient.EvaluateCCR(p.ctx, m, codeChangeRequest) + if err != nil { + p.log.Error("error evaluating code change request for model", zap.Error(err)) + continue + } + p.log.Info("openai api response", zap.String("model", m), zap.String("response", res.String())) + + p.log.Info("testing diff comment code change request") + diffRes, err := p.openAIClient.EvaluateDiffComment(p.ctx, m, diffCommentRequestChange) + if err != nil { + p.log.Error("error evaluating diff comment request for model", zap.Error(err)) + continue + } + p.log.Info("openai api response", zap.String("model", m), zap.String("response", diffRes.String())) + + p.log.Info("testing diff comment question request") + diffRes, err = p.openAIClient.EvaluateDiffComment(p.ctx, m, diffCommentRequestQuestion) + if err != nil { + p.log.Error("error evaluating diff comment request for model", zap.Error(err)) + continue + } + p.log.Info("openai api response", zap.String("model", m), zap.String("response", diffRes.String())) + + } + + // TODO group errors and return + return nil +} diff --git a/vc/common.go b/vc/common.go index 1623142..a6f345f 100644 --- a/vc/common.go +++ b/vc/common.go @@ -32,24 +32,25 @@ type ListIssueOptions struct { // Comment represents a comment on a code change request. // TODO comments on issue? type Comment struct { - ID string + ID int64 // ChangeID is the local identifier for the code change request this comment was left on (e.g. Github PR number) ChangeID string Author Author Body string Position int + FilePath string DiffHunk string URL string + Branch string + PRNumber int } func (c Comment) String() string { - return fmt.Sprintf("Comment ID: %s\nAuthor: %s\nBody: %s\nPosition: %d\n\nDiffHunk:\n%s\n\nURL: %s\n", c.ID, c.Author.Handle, c.Body, c.Position, c.DiffHunk, c.URL) + return fmt.Sprintf("Comment ID: %d\nAuthor: %s\nBody: %s\nPosition: %d\n\nDiffHunk:\n%s\n\nURL: %s\nBranch:\n%s\n\nFilePath:\n%s\n\n", c.ID, c.Author.Handle, c.Body, c.Position, c.DiffHunk, c.URL, c.Branch, c.FilePath) } // ListCommentOptions defines options for listing comments. type ListCommentOptions struct { - // ChangeID is the local identifier for the code change request to list comments from (e.g. Github PR number) - ChangeID string // Handles defines the list of usernames to list comments from // The comment can be created by *any* user provided. Handles []string diff --git a/vc/git.go b/vc/git.go index 110a2b2..9dbde0d 100644 --- a/vc/git.go +++ b/vc/git.go @@ -15,6 +15,7 @@ import ( "github.com/go-git/go-git/v5" "github.com/go-git/go-git/v5/config" + "github.com/go-git/go-git/v5/plumbing" "github.com/go-git/go-git/v5/plumbing/object" "github.com/go-git/go-git/v5/plumbing/transport/http" ) @@ -60,46 +61,53 @@ func NewLocalGitClient( /*ctx context.Context, */ log *zap.Logger, self Author, }, nil } -/* -func (gc *LocalGitClient) SwitchBranch(branchName string) (err error) { +func (gc *LocalGitClient) CheckoutRemoteBranch(branchName string) (err error) { if gc.worktree == nil { return errors.New("worktree is nil - cannot check out a branch") } - branchRefName := plumbing.NewBranchReferenceName(branchName) - // remoteName := "origin" - - err = gc.repo.localRepo.Fetch(&git.FetchOptions{ - RefSpecs: []config.RefSpec{"refs/*:refs/*", "HEAD:refs/heads/HEAD"}, - }) - if err != nil { - return err - } - - err = gc.worktree.Checkout(&git.CheckoutOptions{ - Branch: branchRefName, + // TODO configurable remote + branchRefName := plumbing.NewRemoteReferenceName("origin", branchName) + branchCoOpts := git.CheckoutOptions{ + Branch: plumbing.ReferenceName(branchRefName), Force: true, - }) + } + err = gc.worktree.Checkout(&branchCoOpts) if err != nil { return err } - err = gc.repo.localRepo.CreateBranch(&config.Branch{ - Name: branchName, - Remote: remoteName, - Merge: branchRefName, - }) - if err != nil { - return err - } + + // Pull the latest changes from the remote branch + err = gc.worktree.Pull(&git.PullOptions{ + RemoteName: "origin", + Auth: &http.BasicAuth{ + Username: gc.self.Handle, + Password: gc.self.Token, + }, + }) + if err != nil && err != git.NoErrAlreadyUpToDate { + return err + } return nil } -*/ func (gc *LocalGitClient) PushBranch(branchName string) (err error) { //branchRefName := plumbing.NewBranchReferenceName(branchName) remoteName := "origin" + headRef, err := gc.repo.localRepo.Head() + if err != nil { + return err + } + + // Create new branch at current HEAD + branchRef := plumbing.NewHashReference(plumbing.NewBranchReferenceName(branchName), headRef.Hash()) + err = gc.repo.localRepo.Storer.SetReference(branchRef) + if err != nil { + return err + } + // Push the new branch to the remote repository remote, err := gc.repo.localRepo.Remote(remoteName) if err != nil { @@ -109,7 +117,7 @@ func (gc *LocalGitClient) PushBranch(branchName string) (err error) { err = remote.Push(&git.PushOptions{ RemoteName: remoteName, // TODO remove hardcoded "main" - RefSpecs: []config.RefSpec{config.RefSpec(fmt.Sprintf("+refs/heads/%s:refs/heads/%s", "main", branchName))}, + RefSpecs: []config.RefSpec{config.RefSpec(fmt.Sprintf("+refs/heads/%s:refs/heads/%s", branchName, branchName))}, Auth: &http.BasicAuth{ Username: gc.self.Handle, Password: gc.self.Token, diff --git a/vc/github.go b/vc/github.go index 3cadafd..dca025c 100644 --- a/vc/github.go +++ b/vc/github.go @@ -5,6 +5,7 @@ import ( "errors" "fmt" "strconv" + "strings" "github.com/mobyvb/pull-pal/llm" @@ -151,45 +152,84 @@ func (gc *GithubClient) RemoveLabelFromIssue(issueNumber int, label string) erro // ListOpenComments lists unresolved comments in the Github repository. func (gc *GithubClient) ListOpenComments(options ListCommentOptions) ([]Comment, error) { - toReturn := []Comment{} - prNumber, err := strconv.Atoi(options.ChangeID) - if err != nil { - return nil, err - } - comments, _, err := gc.client.PullRequests.ListComments(gc.ctx, gc.repo.Owner.Handle, gc.repo.Name, prNumber, nil) + prs, _, err := gc.client.PullRequests.List(gc.ctx, gc.repo.Owner.Handle, gc.repo.Name, nil) if err != nil { return nil, err } - // TODO: filter out comments that "self" has already replied to - // TODO: ignore resolved comments - for _, c := range comments { - commentUser := c.GetUser().GetLogin() - allowedUser := false - for _, u := range options.Handles { - if commentUser == u { - allowedUser = true - break - } - } - if !allowedUser { + allComments := []Comment{} + repliedTo := make(map[int64]bool) + + for _, pr := range prs { + if pr.GetUser().GetLogin() != gc.self.Handle { continue } - nextComment := Comment{ - ID: strconv.FormatInt(c.GetID(), 10), - ChangeID: options.ChangeID, - URL: c.GetHTMLURL(), - Author: Author{ - Email: c.GetUser().GetEmail(), - Handle: c.GetUser().GetLogin(), - }, - Body: c.GetBody(), - Position: c.GetPosition(), - DiffHunk: c.GetDiffHunk(), + branch := "" + if pr.Head != nil { + branch = pr.Head.GetLabel() + if strings.Contains(branch, ":") { + branch = strings.Split(branch, ":")[1] + } + } + + comments, _, err := gc.client.PullRequests.ListComments(gc.ctx, gc.repo.Owner.Handle, gc.repo.Name, pr.GetNumber(), nil) + if err != nil { + return nil, err + } + + for _, c := range comments { + commentUser := c.GetUser().GetLogin() + if commentUser == gc.self.Handle { + repliedTo[c.GetInReplyTo()] = true + } + allowedUser := false + for _, u := range options.Handles { + if commentUser == u { + allowedUser = true + break + } + } + if !allowedUser { + continue + } + + nextComment := Comment{ + ID: c.GetID(), + ChangeID: strconv.Itoa(pr.GetNumber()), + URL: c.GetHTMLURL(), + Author: Author{ + Email: c.GetUser().GetEmail(), + Handle: c.GetUser().GetLogin(), + }, + Body: c.GetBody(), + FilePath: c.GetPath(), + Position: c.GetPosition(), + DiffHunk: c.GetDiffHunk(), + Branch: branch, + PRNumber: pr.GetNumber(), + } + allComments = append(allComments, nextComment) + } + } + + // remove any comments that bot has replied to already from the list + toReturn := []Comment{} + for _, c := range allComments { + if !repliedTo[c.ID] { + toReturn = append(toReturn, c) } - toReturn = append(toReturn, nextComment) } return toReturn, nil } + +// RespondToComment adds a comment to the provided thread. +func (gc *GithubClient) RespondToComment(prNumber int, commentID int64, comment string) error { + _, _, err := gc.client.PullRequests.CreateCommentInReplyTo(gc.ctx, gc.repo.Owner.Handle, gc.repo.Name, prNumber, comment, commentID) + if err != nil { + return err + } + + return err +}