Skip to content

added complete once if possible, added jira integration #4

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
May 4, 2023
Merged

added complete once if possible, added jira integration #4

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
bf8d347
added complete once if possible, added jira integration
ravilushqa Apr 25, 2023
78d2707
don't stop work on error extracting jira ticket
ravilushqa Apr 25, 2023
67142f8
Merge branch 'main' into enhance_description
ravilushqa May 4, 2023
bab9ada
minor improvements
ravilushqa May 4, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
103 changes: 82 additions & 21 deletions cmd/description/main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (
"github.com/sashabaranov/go-openai"

ghClient "github.com/ravilushqa/gpt-pullrequest-updater/github"
"github.com/ravilushqa/gpt-pullrequest-updater/jira"
oAIClient "github.com/ravilushqa/gpt-pullrequest-updater/openai"
)

Expand All @@ -22,6 +23,7 @@ var opts struct {
Repo string `long:"repo" env:"REPO" description:"GitHub repo" required:"true"`
PRNumber int `long:"pr-number" env:"PR_NUMBER" description:"Pull request number" required:"true"`
Test bool `long:"test" env:"TEST" description:"Test mode"`
JiraURL string `long:"jira-url" env:"JIRA_URL" description:"Jira URL"`
}

func main() {
Expand All @@ -35,6 +37,10 @@ func main() {
os.Exit(0)
}

if opts.Test {
fmt.Println("Test mode")
}

if err := run(ctx); err != nil {
panic(err)
}
Expand All @@ -54,55 +60,110 @@ func run(ctx context.Context) error {
return fmt.Errorf("error getting commits: %w", err)
}

var OverallDescribeCompletion string
OverallDescribeCompletion += fmt.Sprintf("Pull request title: %s, body: %s\n\n", pr.GetTitle(), pr.GetBody())
var sumDiffs int
for _, file := range diff.Files {
sumDiffs += len(*file.Patch)
}

var completion string
if sumDiffs < 4000 {
completion, err = genCompletionOnce(ctx, openAIClient, diff)
if err != nil {
return fmt.Errorf("error generating completition once: %w", err)
}
} else {
completion, err = genCompletionPerFile(ctx, openAIClient, diff, pr)
if err != nil {
return fmt.Errorf("error generating completition twice: %w", err)
}
}

if opts.JiraURL != "" {
fmt.Println("Adding Jira ticket")
id, err := jira.ExtractJiraTicketID(*pr.Title)
if err != nil {
fmt.Printf("Error extracting Jira ticket ID: %v \n", err)
} else {
completion = fmt.Sprintf("### JIRA ticket: [%s](%s) \n\n%s", id, jira.GenerateJiraTicketURL(opts.JiraURL, id), completion)
}
}

if opts.Test {
fmt.Println(completion)
return nil
}

// Update the pull request description
fmt.Println("Updating pull request")
updatePr := &github.PullRequest{Body: github.String(completion)}
if _, err = githubClient.UpdatePullRequest(ctx, opts.Owner, opts.Repo, opts.PRNumber, updatePr); err != nil {
return fmt.Errorf("error updating pull request: %w", err)
}

return nil
}

func genCompletionOnce(ctx context.Context, client *oAIClient.Client, diff *github.CommitsComparison) (string, error) {
fmt.Println("Generating completion once")
messages := make([]openai.ChatCompletionMessage, 0, len(diff.Files))
messages = append(messages, openai.ChatCompletionMessage{
Role: openai.ChatMessageRoleUser,
Content: oAIClient.PromptDescribeChanges,
})
for _, file := range diff.Files {
if file.Patch == nil {
continue
}

messages = append(messages, openai.ChatCompletionMessage{
Role: openai.ChatMessageRoleUser,
Content: *file.Patch,
})
}

fmt.Println("Sending prompt to OpenAI")
completion, err := client.ChatCompletion(ctx, messages)
if err != nil {
return "", fmt.Errorf("error completing prompt: %w", err)
}

return completion, nil
}

func genCompletionPerFile(ctx context.Context, client *oAIClient.Client, diff *github.CommitsComparison, pr *github.PullRequest) (string, error) {
fmt.Println("Generating completion per file")
OverallDescribeCompletion := fmt.Sprintf("Pull request title: %s, body: %s\n\n", pr.GetTitle(), pr.GetBody())

for i, file := range diff.Files {
prompt := fmt.Sprintf(oAIClient.PromptDescribeChanges, *file.Patch)

if len(prompt) > 4096 {
prompt = fmt.Sprintf("%s...", prompt[:4093])
}

completion, err := openAIClient.ChatCompletion(ctx, []openai.ChatCompletionMessage{
fmt.Printf("Sending prompt to OpenAI for file %d/%d\n", i+1, len(diff.Files))
completion, err := client.ChatCompletion(ctx, []openai.ChatCompletionMessage{
{
Role: openai.ChatMessageRoleUser,
Content: prompt,
},
})
if err != nil {
return fmt.Errorf("error getting review: %w", err)
return "", fmt.Errorf("error getting review: %w", err)
}
OverallDescribeCompletion += fmt.Sprintf("File: %s \nDescription: %s \n\n", file.GetFilename(), completion)
}

overallCompletion, err := openAIClient.ChatCompletion(ctx, []openai.ChatCompletionMessage{
fmt.Println("Sending final prompt to OpenAI")
overallCompletion, err := client.ChatCompletion(ctx, []openai.ChatCompletionMessage{
{
Role: openai.ChatMessageRoleUser,
Content: fmt.Sprintf(oAIClient.PromptOverallDescribe, OverallDescribeCompletion),
},
})
if err != nil {
return fmt.Errorf("error getting overall review: %w", err)
}

if opts.Test {
fmt.Println(OverallDescribeCompletion)
fmt.Println("=====================================")
fmt.Println(overallCompletion)

return nil
return "", fmt.Errorf("error getting overall review: %w", err)
}

// Update the pull request description
updatePr := &github.PullRequest{Body: github.String(overallCompletion)}
if _, err = githubClient.UpdatePullRequest(ctx, opts.Owner, opts.Repo, opts.PRNumber, updatePr); err != nil {
return fmt.Errorf("error updating pull request: %w", err)
}

return nil
return overallCompletion, nil
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code patch adds a new flag JiraURL to the CLI and a new import package jira. The genCompletionOnce and genCompletionPerFile functions are refactored to accept an OpenAI client as an argument. The run function now checks for the Test flag and prints the completion output if it is set to true. A new genCompletionPerFile function is introduced to generate completion for files with diffs greater than 4000 bytes. The main function reads the JiraURL flag, checks for the Test flag, and calls the run function to update the pull request description.

The code looks easy to read, and the changes made seem reasonable. However, it is hard to tell if there are any bugs or risks without understanding the full context of the code and its usage. As for improvement suggestions, it would be helpful to add comments to explain the purpose and functionality of the added code. Additionally, error handling could be improved by providing more detailed error messages when returning errors.

6 changes: 1 addition & 5 deletions cmd/review/main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,11 +57,7 @@ func run(ctx context.Context) error {

var OverallReviewCompletion string
for _, file := range diff.Files {
if file.GetStatus() == "removed" || file.GetStatus() == "renamed" {
continue
}

if file.Patch == nil {
if file.Patch == nil || file.GetStatus() == "removed" || file.GetStatus() == "renamed" {
continue
}

Expand Down
29 changes: 29 additions & 0 deletions jira/jira.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
package jira

import (
"fmt"
"regexp"
)

const ticketUrlFormat = "%s/browse/%s"

// ExtractJiraTicketID returns the first JIRA ticket ID found in the input string.
func ExtractJiraTicketID(s string) (string, error) {
// This regular expression pattern matches a JIRA ticket ID (e.g. PROJ-123).
pattern := `([aA-zZ]+-\d+)`
re, err := regexp.Compile(pattern)
if err != nil {
return "", fmt.Errorf("error compiling regex: %w", err)
}

matches := re.FindStringSubmatch(s)
if len(matches) == 0 {
return "", fmt.Errorf("no JIRA ticket ID found in the input string")
}

return matches[0], nil
}

func GenerateJiraTicketURL(jiraURL, ticketID string) string {
return fmt.Sprintf(ticketUrlFormat, jiraURL, ticketID)
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This code patch adds functionality for extracting JIRA ticket IDs from a string and generating a JIRA URL for a given ticket ID. Overall, the code seems simple and straightforward.

One potential improvement could be to add more robust error handling for the GenerateJiraTicketURL function, such as checking that the JIRA URL string is not empty. Additionally, it may be helpful to add some documentation or comments to explain the purpose of the ticketUrlFormat constant.

Overall, the code seems low-risk for introducing bugs.

55 changes: 55 additions & 0 deletions jira/jira_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
package jira

import "testing"

func TestExtractJiraTicketID(t *testing.T) {
testCases := []struct {
name string
input string
expected string
expectError bool
}{
{
name: "Valid ticket ID",
input: "This is a sample text with a JIRA ticket ID: PROJ-123, let's extract it.",
expected: "PROJ-123",
expectError: false,
},
{
name: "No ticket ID",
input: "This is a sample text without a JIRA ticket ID.",
expectError: true,
},
{
name: "Multiple ticket IDs",
input: "This text has multiple JIRA ticket IDs: PROJ-123, TASK-456, and BUG-789.",
expected: "PROJ-123",
expectError: false,
},
{
name: "Valid ticket ID. Lowercase.",
input: "This is an invalid JIRA ticket ID: Proj-123.",
expected: "Proj-123",
expectError: false,
},
}

for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
result, err := ExtractJiraTicketID(tc.input)
if tc.expectError {
if err == nil {
t.Errorf("expected an error, but got none")
}
} else {
if err != nil {
t.Errorf("unexpected error: %v", err)
}

if result != tc.expected {
t.Errorf("expected result '%s', but got '%s'", tc.expected, result)
}
}
})
}
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This code patch adds a test function for a ExtractJiraTicketID function implemented elsewhere. The test function contains multiple test cases to test the functionality of the ExtractJiraTicketID function in multiple scenarios. The test cases vary based on the input strings and the expected output.

Overall, the code patch looks good, and the testing strategy is quite comprehensive. However, one suggestion for improvement could be to include some edge cases, such as very long input strings or non-alphanumeric characters in the input string. Additionally, it might be a good idea to check for the error message in the test cases that expect an error to ensure that the correct type of error is being thrown.