From 48d1e6f893770273ee2b648787ba094ce1148aec Mon Sep 17 00:00:00 2001 From: Bailey Stoner Date: Sat, 14 Mar 2026 12:47:37 -0700 Subject: [PATCH 1/6] feat: LLM-powered PR code review action Composite GitHub Action that reviews pull requests using configurable LLM providers. Posts inline review comments using conventional comment format (nit:, issue:, suggestion:, etc.) and submits a verdict. Starts with Gemini support, designed to add more providers easily. Includes dogfooding workflow to review its own PRs. --- .github/workflows/review.yml | 15 +++ .gitignore | 2 +- .projections.json | 21 +++++ action.yml | 44 +++++++++ cmd/codereview/main.go | 61 +++++++++++++ internal/action/inputs.go | 99 ++++++++++++++++++++ internal/diff/parse.go | 142 +++++++++++++++++++++++++++++ internal/diff/types.go | 25 +++++ internal/github/client.go | 118 ++++++++++++++++++++++++ internal/prompt/prompt.go | 77 ++++++++++++++++ internal/provider/gemini/gemini.go | 104 +++++++++++++++++++++ internal/provider/provider.go | 14 +++ internal/review/review.go | 114 +++++++++++++++++++++++ internal/review/types.go | 48 ++++++++++ 14 files changed, 883 insertions(+), 1 deletion(-) create mode 100644 .github/workflows/review.yml create mode 100644 .projections.json create mode 100644 action.yml create mode 100644 cmd/codereview/main.go create mode 100644 internal/action/inputs.go create mode 100644 internal/diff/parse.go create mode 100644 internal/diff/types.go create mode 100644 internal/github/client.go create mode 100644 internal/prompt/prompt.go create mode 100644 internal/provider/gemini/gemini.go create mode 100644 internal/provider/provider.go create mode 100644 internal/review/review.go create mode 100644 internal/review/types.go diff --git a/.github/workflows/review.yml b/.github/workflows/review.yml new file mode 100644 index 0000000..90112df --- /dev/null +++ b/.github/workflows/review.yml @@ -0,0 +1,15 @@ +name: Code Review + +on: + pull_request: + types: [opened, synchronize] + +jobs: + review: + runs-on: macos-latest + steps: + - uses: actions/checkout@v4 + + - uses: ./ + with: + gemini-api-key: ${{ secrets.GEMINI_API_KEY }} diff --git a/.gitignore b/.gitignore index b99a396..dd75555 100644 --- a/.gitignore +++ b/.gitignore @@ -4,7 +4,7 @@ *.dll *.so *.dylib -codereview +/codereview # Test binary *.test diff --git a/.projections.json b/.projections.json new file mode 100644 index 0000000..fa58f02 --- /dev/null +++ b/.projections.json @@ -0,0 +1,21 @@ +{ + "internal/*.go": { + "alternate": "internal/{}_test.go", + "type": "source" + }, + "internal/*_test.go": { + "alternate": "internal/{}.go", + "type": "test" + }, + "internal/**/*.go": { + "alternate": "internal/**/{}_test.go", + "type": "source" + }, + "internal/**/*_test.go": { + "alternate": "internal/**/{}.go", + "type": "test" + }, + "cmd/codereview/*.go": { + "type": "command" + } +} diff --git a/action.yml b/action.yml new file mode 100644 index 0000000..d8fd9bd --- /dev/null +++ b/action.yml @@ -0,0 +1,44 @@ +name: 'Code Review' +description: 'LLM-powered PR code review with conventional comments' + +inputs: + github-token: + description: 'GitHub token for API access' + required: true + default: '${{ github.token }}' + provider: + description: 'LLM provider' + required: false + default: 'gemini' + gemini-api-key: + description: 'API key for Google Gemini' + required: false + model: + description: 'Model name override' + required: false + instructions: + description: 'Additional review instructions' + required: false + +runs: + using: 'composite' + steps: + - name: Setup Go + uses: actions/setup-go@v5 + with: + go-version: '1.24' + cache-dependency-path: ${{ github.action_path }}/go.sum + + - name: Build + shell: bash + run: cd ${{ github.action_path }} && go build -o $RUNNER_TEMP/codereview ./cmd/codereview + + - name: Review + shell: bash + env: + INPUT_GITHUB_TOKEN: ${{ inputs.github-token }} + INPUT_PROVIDER: ${{ inputs.provider }} + INPUT_GEMINI_API_KEY: ${{ inputs.gemini-api-key }} + INPUT_MODEL: ${{ inputs.model }} + INPUT_INSTRUCTIONS: ${{ inputs.instructions }} + run: $RUNNER_TEMP/codereview diff --git a/cmd/codereview/main.go b/cmd/codereview/main.go new file mode 100644 index 0000000..bf73fd5 --- /dev/null +++ b/cmd/codereview/main.go @@ -0,0 +1,61 @@ +package main + +import ( + "context" + "fmt" + "os" + + "github.com/monokrome/codereview/internal/action" + "github.com/monokrome/codereview/internal/github" + "github.com/monokrome/codereview/internal/provider/gemini" + "github.com/monokrome/codereview/internal/review" +) + +func main() { + if err := run(); err != nil { + fmt.Fprintf(os.Stderr, "error: %v\n", err) + os.Exit(1) + } +} + +func run() error { + ctx := context.Background() + + cfg, err := action.Parse() + if err != nil { + return fmt.Errorf("parsing inputs: %w", err) + } + + var reviewFn review.Config + switch cfg.Provider { + case "gemini": + if cfg.GeminiAPIKey == "" { + return fmt.Errorf("INPUT_GEMINI_API_KEY is required for gemini provider") + } + reviewFn.Provider = gemini.New(cfg.GeminiAPIKey, cfg.Model) + default: + return fmt.Errorf("unsupported provider: %s", cfg.Provider) + } + + gh := github.New(cfg.GitHubToken) + + diffText, err := gh.FetchDiff(ctx, cfg.Owner, cfg.Repo, cfg.PRNumber) + if err != nil { + return fmt.Errorf("fetching diff: %w", err) + } + + reviewFn.Diff = diffText + reviewFn.Instructions = cfg.Instructions + + result, err := review.Run(ctx, reviewFn) + if err != nil { + return fmt.Errorf("running review: %w", err) + } + + if err := gh.SubmitReview(ctx, cfg.Owner, cfg.Repo, cfg.PRNumber, cfg.CommitSHA, result); err != nil { + return fmt.Errorf("submitting review: %w", err) + } + + fmt.Fprintf(os.Stderr, "review submitted: %s\n", result.Verdict) + return nil +} diff --git a/internal/action/inputs.go b/internal/action/inputs.go new file mode 100644 index 0000000..f06d01c --- /dev/null +++ b/internal/action/inputs.go @@ -0,0 +1,99 @@ +package action + +import ( + "encoding/json" + "fmt" + "os" + "strconv" +) + +type Config struct { + GitHubToken string + Provider string + GeminiAPIKey string + Model string + Instructions string + Owner string + Repo string + PRNumber int + CommitSHA string +} + +type githubEvent struct { + PullRequest struct { + Number int `json:"number"` + Head struct { + SHA string `json:"sha"` + } `json:"head"` + } `json:"pull_request"` + Repository struct { + Owner struct { + Login string `json:"login"` + } `json:"owner"` + Name string `json:"name"` + } `json:"repository"` +} + +func Parse() (Config, error) { + cfg := Config{ + GitHubToken: os.Getenv("INPUT_GITHUB_TOKEN"), + Provider: os.Getenv("INPUT_PROVIDER"), + GeminiAPIKey: os.Getenv("INPUT_GEMINI_API_KEY"), + Model: os.Getenv("INPUT_MODEL"), + Instructions: os.Getenv("INPUT_INSTRUCTIONS"), + } + + if cfg.GitHubToken == "" { + return Config{}, fmt.Errorf("INPUT_GITHUB_TOKEN is required") + } + + if cfg.Provider == "" { + cfg.Provider = "gemini" + } + + eventPath := os.Getenv("GITHUB_EVENT_PATH") + if eventPath == "" { + return Config{}, fmt.Errorf("GITHUB_EVENT_PATH is required") + } + + data, err := os.ReadFile(eventPath) + if err != nil { + return Config{}, fmt.Errorf("reading event file: %w", err) + } + + var event githubEvent + if err := json.Unmarshal(data, &event); err != nil { + return Config{}, fmt.Errorf("parsing event JSON: %w", err) + } + + cfg.Owner = event.Repository.Owner.Login + cfg.Repo = event.Repository.Name + cfg.PRNumber = event.PullRequest.Number + cfg.CommitSHA = event.PullRequest.Head.SHA + + if override := os.Getenv("INPUT_PR_NUMBER"); override != "" { + n, err := strconv.Atoi(override) + if err != nil { + return Config{}, fmt.Errorf("parsing INPUT_PR_NUMBER: %w", err) + } + cfg.PRNumber = n + } + + if cfg.Owner == "" { + return Config{}, fmt.Errorf("repository owner not found in event") + } + + if cfg.Repo == "" { + return Config{}, fmt.Errorf("repository name not found in event") + } + + if cfg.PRNumber == 0 { + return Config{}, fmt.Errorf("pull request number not found in event") + } + + if cfg.CommitSHA == "" { + return Config{}, fmt.Errorf("commit SHA not found in event") + } + + return cfg, nil +} diff --git a/internal/diff/parse.go b/internal/diff/parse.go new file mode 100644 index 0000000..a2f1f41 --- /dev/null +++ b/internal/diff/parse.go @@ -0,0 +1,142 @@ +package diff + +import ( + "fmt" + "strconv" + "strings" +) + +const ( + diffGitPrefix = "diff --git " + hunkPrefix = "@@" + addedPrefix = "+" + removedPrefix = "-" + devNull = "/dev/null" +) + +func Parse(diffText string) ([]File, error) { + lines := strings.Split(diffText, "\n") + var files []File + var current *File + + for i := 0; i < len(lines); i++ { + line := lines[i] + + if strings.HasPrefix(line, diffGitPrefix) { + path := extractPath(line) + newFile := File{Path: path} + files = append(files, newFile) + current = &files[len(files)-1] + current.Path = resolvePathFromHeaders(lines, i, path) + continue + } + + if current == nil { + continue + } + + if !strings.HasPrefix(line, hunkPrefix) { + continue + } + + hunk, err := parseHunkHeader(line) + if err != nil { + return nil, fmt.Errorf("parsing hunk header %q: %w", line, err) + } + + lineNum := hunk.StartLine + i++ + + for i < len(lines) && !strings.HasPrefix(lines[i], diffGitPrefix) && !strings.HasPrefix(lines[i], hunkPrefix) { + raw := lines[i] + + if raw == `\ No newline at end of file` { + i++ + continue + } + + if len(raw) == 0 { + hunk.Lines = append(hunk.Lines, Line{Number: lineNum, Kind: KindContext, Content: ""}) + lineNum++ + i++ + continue + } + + switch raw[0] { + case '+': + hunk.Lines = append(hunk.Lines, Line{Number: lineNum, Kind: KindAdded, Content: raw[1:]}) + lineNum++ + case '-': + hunk.Lines = append(hunk.Lines, Line{Number: lineNum, Kind: KindRemoved, Content: raw[1:]}) + default: + hunk.Lines = append(hunk.Lines, Line{Number: lineNum, Kind: KindContext, Content: trimLeadingSpace(raw)}) + lineNum++ + } + + i++ + } + + current.Hunks = append(current.Hunks, hunk) + i-- + } + + return files, nil +} + +func extractPath(gitDiffLine string) string { + trimmed := strings.TrimPrefix(gitDiffLine, diffGitPrefix) + parts := strings.SplitN(trimmed, " ", 2) + if len(parts) < 2 { + return strings.TrimPrefix(trimmed, "a/") + } + return strings.TrimPrefix(parts[1], "b/") +} + +func resolvePathFromHeaders(lines []string, start int, fallback string) string { + for j := start + 1; j < len(lines) && j <= start+4; j++ { + if strings.HasPrefix(lines[j], diffGitPrefix) || strings.HasPrefix(lines[j], hunkPrefix) { + break + } + + if strings.HasPrefix(lines[j], "+++ ") { + target := strings.TrimPrefix(lines[j], "+++ ") + if target == devNull { + return fallback + } + return strings.TrimPrefix(target, "b/") + } + } + return fallback +} + +func parseHunkHeader(line string) (Hunk, error) { + atIdx := strings.Index(line, "+") + if atIdx < 0 { + return Hunk{}, fmt.Errorf("no + range in hunk header") + } + + rest := line[atIdx+1:] + endIdx := strings.Index(rest, " ") + if endIdx < 0 { + endIdx = strings.Index(rest, hunkPrefix) + } + if endIdx < 0 { + endIdx = len(rest) + } + + rangeStr := rest[:endIdx] + parts := strings.SplitN(rangeStr, ",", 2) + startLine, err := strconv.Atoi(parts[0]) + if err != nil { + return Hunk{}, fmt.Errorf("parsing start line %q: %w", parts[0], err) + } + + return Hunk{StartLine: startLine}, nil +} + +func trimLeadingSpace(s string) string { + if len(s) > 0 && s[0] == ' ' { + return s[1:] + } + return s +} diff --git a/internal/diff/types.go b/internal/diff/types.go new file mode 100644 index 0000000..223bb24 --- /dev/null +++ b/internal/diff/types.go @@ -0,0 +1,25 @@ +package diff + +type LineKind int + +const ( + KindContext LineKind = iota + KindAdded + KindRemoved +) + +type Line struct { + Number int + Kind LineKind + Content string +} + +type Hunk struct { + StartLine int + Lines []Line +} + +type File struct { + Path string + Hunks []Hunk +} diff --git a/internal/github/client.go b/internal/github/client.go new file mode 100644 index 0000000..061bdf3 --- /dev/null +++ b/internal/github/client.go @@ -0,0 +1,118 @@ +package github + +import ( + "context" + "encoding/json" + "fmt" + "io" + "net/http" + "strings" + + "github.com/monokrome/codereview/internal/review" +) + +const apiBase = "https://api.github.com" + +type Client struct { + token string + httpClient *http.Client +} + +func New(token string) *Client { + return &Client{ + token: token, + httpClient: &http.Client{}, + } +} + +func (c *Client) FetchDiff(ctx context.Context, owner, repo string, prNumber int) (string, error) { + url := fmt.Sprintf("%s/repos/%s/%s/pulls/%d", apiBase, owner, repo, prNumber) + + req, err := http.NewRequestWithContext(ctx, http.MethodGet, url, nil) + if err != nil { + return "", fmt.Errorf("creating request: %w", err) + } + + req.Header.Set("Authorization", "Bearer "+c.token) + req.Header.Set("Accept", "application/vnd.github.v3.diff") + + resp, err := c.httpClient.Do(req) + if err != nil { + return "", fmt.Errorf("fetching diff: %w", err) + } + defer resp.Body.Close() + + if resp.StatusCode != http.StatusOK { + body, _ := io.ReadAll(resp.Body) + return "", fmt.Errorf("unexpected status %d: %s", resp.StatusCode, string(body)) + } + + data, err := io.ReadAll(resp.Body) + if err != nil { + return "", fmt.Errorf("reading response: %w", err) + } + + return string(data), nil +} + +type reviewComment struct { + Path string `json:"path"` + Line int `json:"line"` + Body string `json:"body"` + Side string `json:"side"` +} + +type reviewRequest struct { + Event string `json:"event"` + Body string `json:"body"` + Comments []reviewComment `json:"comments"` + CommitID string `json:"commit_id"` +} + +func (c *Client) SubmitReview(ctx context.Context, owner, repo string, prNumber int, commitSHA string, result review.Result) error { + comments := make([]reviewComment, len(result.Comments)) + for i, rc := range result.Comments { + comments[i] = reviewComment{ + Path: rc.Path, + Line: rc.Line, + Body: rc.Body, + Side: "RIGHT", + } + } + + payload := reviewRequest{ + Event: string(result.Verdict), + Body: result.Summary, + Comments: comments, + CommitID: commitSHA, + } + + body, err := json.Marshal(payload) + if err != nil { + return fmt.Errorf("marshalling review: %w", err) + } + + url := fmt.Sprintf("%s/repos/%s/%s/pulls/%d/reviews", apiBase, owner, repo, prNumber) + + req, err := http.NewRequestWithContext(ctx, http.MethodPost, url, strings.NewReader(string(body))) + if err != nil { + return fmt.Errorf("creating request: %w", err) + } + + req.Header.Set("Authorization", "Bearer "+c.token) + req.Header.Set("Accept", "application/vnd.github.v3+json") + req.Header.Set("Content-Type", "application/json") + + resp, err := c.httpClient.Do(req) + if err != nil { + return fmt.Errorf("submitting review: %w", err) + } + defer resp.Body.Close() + + if resp.StatusCode != http.StatusOK { + respBody, _ := io.ReadAll(resp.Body) + return fmt.Errorf("unexpected status %d: %s", resp.StatusCode, string(respBody)) + } + + return nil +} diff --git a/internal/prompt/prompt.go b/internal/prompt/prompt.go new file mode 100644 index 0000000..f0196f7 --- /dev/null +++ b/internal/prompt/prompt.go @@ -0,0 +1,77 @@ +package prompt + +import ( + "fmt" + "strings" + + "github.com/monokrome/codereview/internal/diff" +) + +const systemTemplate = `You are a senior code reviewer. Review the provided unified diff and produce a JSON response. + +Use these conventional comment labels: +- "nit": style or trivial improvements that don't affect correctness +- "suggestion": a better approach or alternative worth considering +- "issue": a bug, logic error, or correctness problem that must be fixed +- "question": something unclear that needs the author's explanation +- "thought": an observation or design consideration, not actionable +- "chore": maintenance tasks like updating dependencies, fixing typos, or cleanup +- "praise": something done well that deserves recognition + +Verdict rules: +- Use "APPROVE" when there are no issues or only nits/praise/thoughts +- Use "REQUEST_CHANGES" when there are any comments with the "issue" label +- Use "COMMENT" for everything else (suggestions, questions, chores without issues) + +Your response must be valid JSON matching this exact structure: +{ + "verdict": "APPROVE" | "REQUEST_CHANGES" | "COMMENT", + "summary": "brief overall summary of the review", + "comments": [ + { + "path": "file/path.go", + "line": 42, + "label": "issue", + "body": "issue: description of the problem" + } + ] +} + +Rules: +- The "line" field must reference a valid line number from the diff (a line that was added or exists as context in the new file) +- The "body" field must start with the label followed by a colon and space (e.g. "nit: unused import") +- The "path" field must match a file path from the diff exactly +- Only comment on meaningful changes; do not comment on every line +- Be concise and actionable +- Output ONLY the JSON object, no markdown fences, no extra text` + +func Build(files []diff.File, instructions string) (string, string) { + var user strings.Builder + + if instructions != "" { + fmt.Fprintf(&user, "Additional review instructions:\n%s\n\n", instructions) + } + + user.WriteString("Review the following diff:\n\n") + + for _, f := range files { + fmt.Fprintf(&user, "--- a/%s\n+++ b/%s\n", f.Path, f.Path) + + for _, h := range f.Hunks { + fmt.Fprintf(&user, "@@ -%d +%d @@\n", h.StartLine, h.StartLine) + + for _, l := range h.Lines { + switch l.Kind { + case diff.KindAdded: + fmt.Fprintf(&user, "+%s\n", l.Content) + case diff.KindRemoved: + fmt.Fprintf(&user, "-%s\n", l.Content) + default: + fmt.Fprintf(&user, " %s\n", l.Content) + } + } + } + } + + return systemTemplate, user.String() +} diff --git a/internal/provider/gemini/gemini.go b/internal/provider/gemini/gemini.go new file mode 100644 index 0000000..f3d86c9 --- /dev/null +++ b/internal/provider/gemini/gemini.go @@ -0,0 +1,104 @@ +package gemini + +import ( + "context" + "encoding/json" + "fmt" + "io" + "net/http" + "strings" + + "github.com/monokrome/codereview/internal/provider" +) + +const ( + DefaultModel = "gemini-2.5-flash" + apiBaseURL = "https://generativelanguage.googleapis.com/v1beta/models" +) + +type part struct { + Text string `json:"text"` +} + +type content struct { + Role string `json:"role,omitempty"` + Parts []part `json:"parts"` +} + +type generateRequest struct { + Contents []content `json:"contents"` + SystemInstruction *content `json:"systemInstruction,omitempty"` +} + +type candidate struct { + Content content `json:"content"` +} + +type generateResponse struct { + Candidates []candidate `json:"candidates"` +} + +func New(apiKey string, model string) provider.ReviewFunc { + if model == "" { + model = DefaultModel + } + + return func(ctx context.Context, req provider.Request) (provider.Response, error) { + payload := generateRequest{ + Contents: []content{ + {Role: "user", Parts: []part{{Text: req.UserPrompt}}}, + }, + } + + if req.SystemPrompt != "" { + payload.SystemInstruction = &content{ + Parts: []part{{Text: req.SystemPrompt}}, + } + } + + body, err := json.Marshal(payload) + if err != nil { + return provider.Response{}, fmt.Errorf("marshalling request: %w", err) + } + + url := fmt.Sprintf("%s/%s:generateContent?key=%s", apiBaseURL, model, apiKey) + + httpReq, err := http.NewRequestWithContext(ctx, http.MethodPost, url, strings.NewReader(string(body))) + if err != nil { + return provider.Response{}, fmt.Errorf("creating request: %w", err) + } + + httpReq.Header.Set("Content-Type", "application/json") + + resp, err := http.DefaultClient.Do(httpReq) + if err != nil { + return provider.Response{}, fmt.Errorf("calling Gemini API: %w", err) + } + defer resp.Body.Close() + + respBody, err := io.ReadAll(resp.Body) + if err != nil { + return provider.Response{}, fmt.Errorf("reading response: %w", err) + } + + if resp.StatusCode != http.StatusOK { + return provider.Response{}, fmt.Errorf("Gemini API returned status %d: %s", resp.StatusCode, string(respBody)) + } + + var genResp generateResponse + if err := json.Unmarshal(respBody, &genResp); err != nil { + return provider.Response{}, fmt.Errorf("parsing response: %w", err) + } + + if len(genResp.Candidates) == 0 { + return provider.Response{}, fmt.Errorf("no candidates in response") + } + + parts := genResp.Candidates[0].Content.Parts + if len(parts) == 0 { + return provider.Response{}, fmt.Errorf("no parts in response candidate") + } + + return provider.Response{Content: parts[0].Text}, nil + } +} diff --git a/internal/provider/provider.go b/internal/provider/provider.go new file mode 100644 index 0000000..fcfe53b --- /dev/null +++ b/internal/provider/provider.go @@ -0,0 +1,14 @@ +package provider + +import "context" + +type Request struct { + SystemPrompt string + UserPrompt string +} + +type Response struct { + Content string +} + +type ReviewFunc func(ctx context.Context, req Request) (Response, error) diff --git a/internal/review/review.go b/internal/review/review.go new file mode 100644 index 0000000..2a59392 --- /dev/null +++ b/internal/review/review.go @@ -0,0 +1,114 @@ +package review + +import ( + "context" + "encoding/json" + "fmt" + "strings" + + "github.com/monokrome/codereview/internal/diff" + "github.com/monokrome/codereview/internal/prompt" + "github.com/monokrome/codereview/internal/provider" +) + +type Config struct { + Diff string + Provider provider.ReviewFunc + Instructions string +} + +func Run(ctx context.Context, cfg Config) (Result, error) { + files, err := diff.Parse(cfg.Diff) + if err != nil { + return Result{}, fmt.Errorf("parsing diff: %w", err) + } + + system, user := prompt.Build(files, cfg.Instructions) + + resp, err := cfg.Provider(ctx, provider.Request{ + SystemPrompt: system, + UserPrompt: user, + }) + if err != nil { + return Result{}, fmt.Errorf("calling provider: %w", err) + } + + result, err := ParseResponse(resp.Content) + if err != nil { + return Result{}, fmt.Errorf("parsing response: %w", err) + } + + if err := validateResult(result, files); err != nil { + return Result{}, fmt.Errorf("validating result: %w", err) + } + + return result, nil +} + +func ParseResponse(raw string) (Result, error) { + cleaned := stripMarkdownFences(raw) + cleaned = strings.TrimSpace(cleaned) + + var result Result + if err := json.Unmarshal([]byte(cleaned), &result); err != nil { + return Result{}, fmt.Errorf("unmarshalling JSON: %w", err) + } + + return result, nil +} + +func stripMarkdownFences(s string) string { + s = strings.TrimSpace(s) + + if !strings.HasPrefix(s, "```") { + return s + } + + firstNewline := strings.Index(s, "\n") + if firstNewline < 0 { + return s + } + s = s[firstNewline+1:] + + if idx := strings.LastIndex(s, "```"); idx >= 0 { + s = s[:idx] + } + + return strings.TrimSpace(s) +} + +func validateResult(result Result, files []diff.File) error { + validPaths := make(map[string]map[int]bool) + for _, f := range files { + lines := make(map[int]bool) + for _, h := range f.Hunks { + for _, l := range h.Lines { + if l.Kind != diff.KindRemoved { + lines[l.Number] = true + } + } + } + validPaths[f.Path] = lines + } + + for i, c := range result.Comments { + if !IsValidLabel(c.Label) { + return fmt.Errorf("comment %d: invalid label %q", i, c.Label) + } + + if c.Line <= 0 { + return fmt.Errorf("comment %d: line number must be positive, got %d", i, c.Line) + } + + fileLines, ok := validPaths[c.Path] + if !ok { + return fmt.Errorf("comment %d: path %q not found in diff", i, c.Path) + } + + if !fileLines[c.Line] { + return fmt.Errorf("comment %d: line %d not found in diff for path %q", i, c.Line, c.Path) + } + } + + return nil +} diff --git a/internal/review/types.go b/internal/review/types.go new file mode 100644 index 0000000..8ba0231 --- /dev/null +++ b/internal/review/types.go @@ -0,0 +1,48 @@ +package review + +type Verdict string + +const ( + VerdictApprove Verdict = "APPROVE" + VerdictRequestChanges Verdict = "REQUEST_CHANGES" + VerdictComment Verdict = "COMMENT" +) + +type Label string + +const ( + LabelNit Label = "nit" + LabelSuggestion Label = "suggestion" + LabelIssue Label = "issue" + LabelQuestion Label = "question" + LabelThought Label = "thought" + LabelChore Label = "chore" + LabelPraise Label = "praise" +) + +var validLabels = map[Label]bool{ + LabelNit: true, + LabelSuggestion: true, + LabelIssue: true, + LabelQuestion: true, + LabelThought: true, + LabelChore: true, + LabelPraise: true, +} + +func IsValidLabel(l Label) bool { + return validLabels[l] +} + +type Comment struct { + Path string `json:"path"` + Line int `json:"line"` + Label Label `json:"label"` + Body string `json:"body"` +} + +type Result struct { + Verdict Verdict `json:"verdict"` + Summary string `json:"summary"` + Comments []Comment `json:"comments"` +} From 560afe2a75bd589268d08a6f3a8d4953c8841c2c Mon Sep 17 00:00:00 2001 From: Bailey Stoner Date: Sat, 14 Mar 2026 15:04:10 -0700 Subject: [PATCH 2/6] fix: drop invalid LLM comments instead of failing the review LLMs sometimes reference lines outside the diff. Filter those out with warnings instead of aborting the entire review. Also fixes cache-dependency-path for setup-go. --- action.yml | 2 +- internal/review/review.go | 26 ++++++++++++++++---------- 2 files changed, 17 insertions(+), 11 deletions(-) diff --git a/action.yml b/action.yml index d8fd9bd..e311768 100644 --- a/action.yml +++ b/action.yml @@ -27,7 +27,7 @@ runs: uses: actions/setup-go@v5 with: go-version: '1.24' - cache-dependency-path: ${{ github.action_path }}/go.sum + cache-dependency-path: go.sum - name: Build shell: bash diff --git a/internal/review/review.go b/internal/review/review.go index 2a59392..451926d 100644 --- a/internal/review/review.go +++ b/internal/review/review.go @@ -4,6 +4,7 @@ import ( "context" "encoding/json" "fmt" + "os" "strings" "github.com/monokrome/codereview/internal/diff" @@ -38,9 +39,7 @@ func Run(ctx context.Context, cfg Config) (Result, error) { return Result{}, fmt.Errorf("parsing response: %w", err) } - if err := validateResult(result, files); err != nil { - return Result{}, fmt.Errorf("validating result: %w", err) - } + result.Comments = filterValidComments(result.Comments, files) return result, nil } @@ -77,7 +76,7 @@ func stripMarkdownFences(s string) string { return strings.TrimSpace(s) } -func validateResult(result Result, files []diff.File) error { +func filterValidComments(comments []Comment, files []diff.File) []Comment { validPaths := make(map[string]map[int]bool) for _, f := range files { lines := make(map[int]bool) @@ -91,24 +90,31 @@ func validateResult(result Result, files []diff.File) error { validPaths[f.Path] = lines } - for i, c := range result.Comments { + var valid []Comment + for _, c := range comments { if !IsValidLabel(c.Label) { - return fmt.Errorf("comment %d: invalid label %q", i, c.Label) + fmt.Fprintf(os.Stderr, "warning: dropping comment with invalid label %q\n", c.Label) + continue } if c.Line <= 0 { - return fmt.Errorf("comment %d: line number must be positive, got %d", i, c.Line) + fmt.Fprintf(os.Stderr, "warning: dropping comment with non-positive line %d\n", c.Line) + continue } fileLines, ok := validPaths[c.Path] if !ok { - return fmt.Errorf("comment %d: path %q not found in diff", i, c.Path) + fmt.Fprintf(os.Stderr, "warning: dropping comment for path %q not in diff\n", c.Path) + continue } if !fileLines[c.Line] { - return fmt.Errorf("comment %d: line %d not found in diff for path %q", i, c.Line, c.Path) + fmt.Fprintf(os.Stderr, "warning: dropping comment for line %d not in diff for %q\n", c.Line, c.Path) + continue } + + valid = append(valid, c) } - return nil + return valid } From 16deadb67689f9a34c609e67253e6f2eb55cce32 Mon Sep 17 00:00:00 2001 From: Bailey Stoner Date: Sat, 14 Mar 2026 15:09:45 -0700 Subject: [PATCH 3/6] feat: reply to review comment threads The action now responds when someone replies to a review comment. Fetches the conversation thread, sends it to the LLM with the relevant diff hunk, and posts a reply. Skips its own comments to prevent infinite loops. --- .github/workflows/review.yml | 2 + action.yml | 4 + cmd/codereview/main.go | 68 +++++++++++++-- internal/action/inputs.go | 66 +++++++++++++++ internal/github/client.go | 155 +++++++++++++++++++++++++++++++++++ internal/prompt/prompt.go | 34 ++++++++ internal/review/review.go | 21 +++++ 7 files changed, 344 insertions(+), 6 deletions(-) diff --git a/.github/workflows/review.yml b/.github/workflows/review.yml index 90112df..85f6046 100644 --- a/.github/workflows/review.yml +++ b/.github/workflows/review.yml @@ -3,6 +3,8 @@ name: Code Review on: pull_request: types: [opened, synchronize] + pull_request_review_comment: + types: [created] jobs: review: diff --git a/action.yml b/action.yml index e311768..ad12b10 100644 --- a/action.yml +++ b/action.yml @@ -19,6 +19,9 @@ inputs: instructions: description: 'Additional review instructions' required: false + bot-login: + description: 'Bot username to ignore for loop prevention with custom tokens' + required: false runs: using: 'composite' @@ -41,4 +44,5 @@ runs: INPUT_GEMINI_API_KEY: ${{ inputs.gemini-api-key }} INPUT_MODEL: ${{ inputs.model }} INPUT_INSTRUCTIONS: ${{ inputs.instructions }} + INPUT_BOT_LOGIN: ${{ inputs.bot-login }} run: $RUNNER_TEMP/codereview diff --git a/cmd/codereview/main.go b/cmd/codereview/main.go index bf73fd5..bdec0d3 100644 --- a/cmd/codereview/main.go +++ b/cmd/codereview/main.go @@ -7,6 +7,8 @@ import ( "github.com/monokrome/codereview/internal/action" "github.com/monokrome/codereview/internal/github" + "github.com/monokrome/codereview/internal/prompt" + "github.com/monokrome/codereview/internal/provider" "github.com/monokrome/codereview/internal/provider/gemini" "github.com/monokrome/codereview/internal/review" ) @@ -26,28 +28,40 @@ func run() error { return fmt.Errorf("parsing inputs: %w", err) } - var reviewFn review.Config + var providerFn provider.ReviewFunc switch cfg.Provider { case "gemini": if cfg.GeminiAPIKey == "" { return fmt.Errorf("INPUT_GEMINI_API_KEY is required for gemini provider") } - reviewFn.Provider = gemini.New(cfg.GeminiAPIKey, cfg.Model) + providerFn = gemini.New(cfg.GeminiAPIKey, cfg.Model) default: return fmt.Errorf("unsupported provider: %s", cfg.Provider) } gh := github.New(cfg.GitHubToken) + switch cfg.Mode { + case action.ModeReview: + return runReview(ctx, cfg, providerFn, gh) + case action.ModeReply: + return runReply(ctx, cfg, providerFn, gh) + default: + return fmt.Errorf("unknown mode: %s", cfg.Mode) + } +} + +func runReview(ctx context.Context, cfg action.Config, providerFn provider.ReviewFunc, gh *github.Client) error { diffText, err := gh.FetchDiff(ctx, cfg.Owner, cfg.Repo, cfg.PRNumber) if err != nil { return fmt.Errorf("fetching diff: %w", err) } - reviewFn.Diff = diffText - reviewFn.Instructions = cfg.Instructions - - result, err := review.Run(ctx, reviewFn) + result, err := review.Run(ctx, review.Config{ + Diff: diffText, + Provider: providerFn, + Instructions: cfg.Instructions, + }) if err != nil { return fmt.Errorf("running review: %w", err) } @@ -59,3 +73,45 @@ func run() error { fmt.Fprintf(os.Stderr, "review submitted: %s\n", result.Verdict) return nil } + +func runReply(ctx context.Context, cfg action.Config, providerFn provider.ReviewFunc, gh *github.Client) error { + if cfg.SkipReply { + fmt.Fprintf(os.Stderr, "skipping: comment is from bot or is a top-level comment\n") + return nil + } + + thread, err := gh.FetchCommentThread(ctx, cfg.Owner, cfg.Repo, cfg.PRNumber, cfg.Comment.CommentID) + if err != nil { + return fmt.Errorf("fetching comment thread: %w", err) + } + + var messages []prompt.ThreadMessage + for _, tc := range thread { + messages = append(messages, prompt.ThreadMessage{ + Author: tc.UserLogin, + Body: tc.Body, + }) + } + + replyText, err := review.RunReply(ctx, review.ReplyConfig{ + Provider: providerFn, + Thread: messages, + DiffHunk: cfg.Comment.DiffHunk, + Instructions: cfg.Instructions, + }) + if err != nil { + return fmt.Errorf("generating reply: %w", err) + } + + replyTo := cfg.Comment.InReplyToID + if replyTo == 0 { + replyTo = cfg.Comment.CommentID + } + + if err := gh.ReplyToComment(ctx, cfg.Owner, cfg.Repo, cfg.PRNumber, replyTo, replyText); err != nil { + return fmt.Errorf("posting reply: %w", err) + } + + fmt.Fprintf(os.Stderr, "reply posted\n") + return nil +} diff --git a/internal/action/inputs.go b/internal/action/inputs.go index f06d01c..ba4a321 100644 --- a/internal/action/inputs.go +++ b/internal/action/inputs.go @@ -7,6 +7,23 @@ import ( "strconv" ) +const ( + ModeReview = "review" + ModeReply = "reply" + + defaultBotLogin = "github-actions[bot]" +) + +type CommentContext struct { + CommentID int64 + InReplyToID int64 + Body string + UserLogin string + Path string + Line int + DiffHunk string +} + type Config struct { GitHubToken string Provider string @@ -17,6 +34,9 @@ type Config struct { Repo string PRNumber int CommitSHA string + Mode string + Comment *CommentContext + SkipReply bool } type githubEvent struct { @@ -32,6 +52,18 @@ type githubEvent struct { } `json:"owner"` Name string `json:"name"` } `json:"repository"` + Comment struct { + ID int64 `json:"id"` + InReplyToID int64 `json:"in_reply_to_id"` + Body string `json:"body"` + Path string `json:"path"` + Line int `json:"line"` + DiffHunk string `json:"diff_hunk"` + CommitID string `json:"commit_id"` + User struct { + Login string `json:"login"` + } `json:"user"` + } `json:"comment"` } func Parse() (Config, error) { @@ -71,6 +103,40 @@ func Parse() (Config, error) { cfg.PRNumber = event.PullRequest.Number cfg.CommitSHA = event.PullRequest.Head.SHA + eventName := os.Getenv("GITHUB_EVENT_NAME") + switch eventName { + case "pull_request_review_comment": + cfg.Mode = ModeReply + cfg.Comment = &CommentContext{ + CommentID: event.Comment.ID, + InReplyToID: event.Comment.InReplyToID, + Body: event.Comment.Body, + UserLogin: event.Comment.User.Login, + Path: event.Comment.Path, + Line: event.Comment.Line, + DiffHunk: event.Comment.DiffHunk, + } + + if event.Comment.CommitID != "" { + cfg.CommitSHA = event.Comment.CommitID + } + + botLogin := os.Getenv("INPUT_BOT_LOGIN") + if botLogin == "" { + botLogin = defaultBotLogin + } + + if cfg.Comment.UserLogin == botLogin { + cfg.SkipReply = true + } + + if cfg.Comment.InReplyToID == 0 { + cfg.SkipReply = true + } + default: + cfg.Mode = ModeReview + } + if override := os.Getenv("INPUT_PR_NUMBER"); override != "" { n, err := strconv.Atoi(override) if err != nil { diff --git a/internal/github/client.go b/internal/github/client.go index 061bdf3..21695cb 100644 --- a/internal/github/client.go +++ b/internal/github/client.go @@ -116,3 +116,158 @@ func (c *Client) SubmitReview(ctx context.Context, owner, repo string, prNumber return nil } + +type ThreadComment struct { + ID int64 `json:"id"` + InReplyToID int64 `json:"in_reply_to_id"` + Body string `json:"body"` + UserLogin string + Path string `json:"path"` + Line int `json:"line"` + DiffHunk string `json:"diff_hunk"` + CreatedAt string `json:"created_at"` +} + +func (c *Client) FetchCommentThread(ctx context.Context, owner, repo string, prNumber int, commentID int64) ([]ThreadComment, error) { + var allComments []struct { + ID int64 `json:"id"` + InReplyToID int64 `json:"in_reply_to_id"` + Body string `json:"body"` + Path string `json:"path"` + Line int `json:"line"` + DiffHunk string `json:"diff_hunk"` + CreatedAt string `json:"created_at"` + User struct { + Login string `json:"login"` + } `json:"user"` + } + + page := 1 + for { + url := fmt.Sprintf("%s/repos/%s/%s/pulls/%d/comments?per_page=100&page=%d", apiBase, owner, repo, prNumber, page) + + req, err := http.NewRequestWithContext(ctx, http.MethodGet, url, nil) + if err != nil { + return nil, fmt.Errorf("creating request: %w", err) + } + + req.Header.Set("Authorization", "Bearer "+c.token) + req.Header.Set("Accept", "application/vnd.github.v3+json") + + resp, err := c.httpClient.Do(req) + if err != nil { + return nil, fmt.Errorf("fetching comments: %w", err) + } + + body, err := io.ReadAll(resp.Body) + resp.Body.Close() + + if resp.StatusCode != http.StatusOK { + return nil, fmt.Errorf("unexpected status %d: %s", resp.StatusCode, string(body)) + } + + var pageComments []struct { + ID int64 `json:"id"` + InReplyToID int64 `json:"in_reply_to_id"` + Body string `json:"body"` + Path string `json:"path"` + Line int `json:"line"` + DiffHunk string `json:"diff_hunk"` + CreatedAt string `json:"created_at"` + User struct { + Login string `json:"login"` + } `json:"user"` + } + + if err := json.Unmarshal(body, &pageComments); err != nil { + return nil, fmt.Errorf("parsing comments: %w", err) + } + + allComments = append(allComments, pageComments...) + + if len(pageComments) < 100 { + break + } + page++ + } + + rootID := commentID + commentsByID := make(map[int64]struct { + ID int64 `json:"id"` + InReplyToID int64 `json:"in_reply_to_id"` + Body string `json:"body"` + Path string `json:"path"` + Line int `json:"line"` + DiffHunk string `json:"diff_hunk"` + CreatedAt string `json:"created_at"` + User struct { + Login string `json:"login"` + } `json:"user"` + }) + for _, c := range allComments { + commentsByID[c.ID] = c + } + + if target, ok := commentsByID[commentID]; ok && target.InReplyToID != 0 { + rootID = target.InReplyToID + } + + var thread []ThreadComment + for _, c := range allComments { + if c.ID == rootID || c.InReplyToID == rootID { + thread = append(thread, ThreadComment{ + ID: c.ID, + InReplyToID: c.InReplyToID, + Body: c.Body, + UserLogin: c.User.Login, + Path: c.Path, + Line: c.Line, + DiffHunk: c.DiffHunk, + CreatedAt: c.CreatedAt, + }) + } + } + + return thread, nil +} + +type replyRequest struct { + Body string `json:"body"` + InReplyTo int64 `json:"in_reply_to"` +} + +func (c *Client) ReplyToComment(ctx context.Context, owner, repo string, prNumber int, inReplyTo int64, body string) error { + payload := replyRequest{ + Body: body, + InReplyTo: inReplyTo, + } + + reqBody, err := json.Marshal(payload) + if err != nil { + return fmt.Errorf("marshalling reply: %w", err) + } + + url := fmt.Sprintf("%s/repos/%s/%s/pulls/%d/comments", apiBase, owner, repo, prNumber) + + req, err := http.NewRequestWithContext(ctx, http.MethodPost, url, strings.NewReader(string(reqBody))) + if err != nil { + return fmt.Errorf("creating request: %w", err) + } + + req.Header.Set("Authorization", "Bearer "+c.token) + req.Header.Set("Accept", "application/vnd.github.v3+json") + req.Header.Set("Content-Type", "application/json") + + resp, err := c.httpClient.Do(req) + if err != nil { + return fmt.Errorf("posting reply: %w", err) + } + defer resp.Body.Close() + + if resp.StatusCode != http.StatusCreated { + respBody, _ := io.ReadAll(resp.Body) + return fmt.Errorf("unexpected status %d: %s", resp.StatusCode, string(respBody)) + } + + return nil +} diff --git a/internal/prompt/prompt.go b/internal/prompt/prompt.go index f0196f7..01631dd 100644 --- a/internal/prompt/prompt.go +++ b/internal/prompt/prompt.go @@ -45,6 +45,16 @@ Rules: - Be concise and actionable - Output ONLY the JSON object, no markdown fences, no extra text` +const replySystemTemplate = `You are a senior code reviewer engaged in a conversation about a code review comment. You are replying to a developer who responded to one of your review comments. + +Rules: +- Be concise and directly address the question or comment +- Reference the code when relevant +- If the developer's response resolves your concern, acknowledge it +- If you still have concerns, explain why clearly +- Be collaborative, not adversarial +- Output plain text only, no JSON, no markdown fences` + func Build(files []diff.File, instructions string) (string, string) { var user strings.Builder @@ -75,3 +85,27 @@ func Build(files []diff.File, instructions string) (string, string) { return systemTemplate, user.String() } + +type ThreadMessage struct { + Author string + Body string +} + +func BuildReply(thread []ThreadMessage, diffHunk string, instructions string) (string, string) { + var user strings.Builder + + if instructions != "" { + fmt.Fprintf(&user, "Additional context:\n%s\n\n", instructions) + } + + fmt.Fprintf(&user, "Relevant code:\n```\n%s\n```\n\n", diffHunk) + + user.WriteString("Conversation thread:\n\n") + for _, msg := range thread { + fmt.Fprintf(&user, "**%s:**\n%s\n\n", msg.Author, msg.Body) + } + + user.WriteString("Write your reply to the latest message.") + + return replySystemTemplate, user.String() +} diff --git a/internal/review/review.go b/internal/review/review.go index 451926d..558a964 100644 --- a/internal/review/review.go +++ b/internal/review/review.go @@ -44,6 +44,27 @@ func Run(ctx context.Context, cfg Config) (Result, error) { return result, nil } +type ReplyConfig struct { + Provider provider.ReviewFunc + Thread []prompt.ThreadMessage + DiffHunk string + Instructions string +} + +func RunReply(ctx context.Context, cfg ReplyConfig) (string, error) { + system, user := prompt.BuildReply(cfg.Thread, cfg.DiffHunk, cfg.Instructions) + + resp, err := cfg.Provider(ctx, provider.Request{ + SystemPrompt: system, + UserPrompt: user, + }) + if err != nil { + return "", fmt.Errorf("calling provider: %w", err) + } + + return strings.TrimSpace(stripMarkdownFences(resp.Content)), nil +} + func ParseResponse(raw string) (Result, error) { cleaned := stripMarkdownFences(raw) cleaned = strings.TrimSpace(cleaned) From b36c65b870096d829c09585d14114504a03d1155 Mon Sep 17 00:00:00 2001 From: Bailey Stoner Date: Sat, 14 Mar 2026 15:13:34 -0700 Subject: [PATCH 4/6] fix: include prior review comments to prevent duplicate feedback On synchronize events, fetch the bot's existing review comments and pass them to the LLM as context so it doesn't repeat itself. Also refactors duplicated struct definitions in the GitHub client. --- cmd/codereview/main.go | 27 ++++- internal/action/inputs.go | 10 +- internal/github/client.go | 216 +++++++++++++++++++++----------------- internal/prompt/prompt.go | 15 ++- internal/review/review.go | 9 +- 5 files changed, 166 insertions(+), 111 deletions(-) diff --git a/cmd/codereview/main.go b/cmd/codereview/main.go index bdec0d3..523c9c9 100644 --- a/cmd/codereview/main.go +++ b/cmd/codereview/main.go @@ -13,6 +13,8 @@ import ( "github.com/monokrome/codereview/internal/review" ) +const defaultBotLogin = "github-actions[bot]" + func main() { if err := run(); err != nil { fmt.Fprintf(os.Stderr, "error: %v\n", err) @@ -57,10 +59,29 @@ func runReview(ctx context.Context, cfg action.Config, providerFn provider.Revie return fmt.Errorf("fetching diff: %w", err) } + botLogin := cfg.BotLogin + if botLogin == "" { + botLogin = defaultBotLogin + } + + priorGH, err := gh.FetchBotReviewComments(ctx, cfg.Owner, cfg.Repo, cfg.PRNumber, botLogin) + if err != nil { + fmt.Fprintf(os.Stderr, "warning: could not fetch prior comments: %v\n", err) + } + + var priorComments []prompt.PriorComment + for _, pc := range priorGH { + priorComments = append(priorComments, prompt.PriorComment{ + Path: pc.Path, + Body: pc.Body, + }) + } + result, err := review.Run(ctx, review.Config{ - Diff: diffText, - Provider: providerFn, - Instructions: cfg.Instructions, + Diff: diffText, + Provider: providerFn, + Instructions: cfg.Instructions, + PriorComments: priorComments, }) if err != nil { return fmt.Errorf("running review: %w", err) diff --git a/internal/action/inputs.go b/internal/action/inputs.go index ba4a321..6fda527 100644 --- a/internal/action/inputs.go +++ b/internal/action/inputs.go @@ -30,6 +30,7 @@ type Config struct { GeminiAPIKey string Model string Instructions string + BotLogin string Owner string Repo string PRNumber int @@ -73,6 +74,7 @@ func Parse() (Config, error) { GeminiAPIKey: os.Getenv("INPUT_GEMINI_API_KEY"), Model: os.Getenv("INPUT_MODEL"), Instructions: os.Getenv("INPUT_INSTRUCTIONS"), + BotLogin: os.Getenv("INPUT_BOT_LOGIN"), } if cfg.GitHubToken == "" { @@ -121,12 +123,12 @@ func Parse() (Config, error) { cfg.CommitSHA = event.Comment.CommitID } - botLogin := os.Getenv("INPUT_BOT_LOGIN") - if botLogin == "" { - botLogin = defaultBotLogin + botCheck := cfg.BotLogin + if botCheck == "" { + botCheck = defaultBotLogin } - if cfg.Comment.UserLogin == botLogin { + if cfg.Comment.UserLogin == botCheck { cfg.SkipReply = true } diff --git a/internal/github/client.go b/internal/github/client.go index 21695cb..58d1a1c 100644 --- a/internal/github/client.go +++ b/internal/github/client.go @@ -25,6 +25,62 @@ func New(token string) *Client { } } +type apiComment struct { + ID int64 `json:"id"` + InReplyToID int64 `json:"in_reply_to_id"` + Body string `json:"body"` + Path string `json:"path"` + Line int `json:"line"` + DiffHunk string `json:"diff_hunk"` + CreatedAt string `json:"created_at"` + User struct { + Login string `json:"login"` + } `json:"user"` +} + +func (c *Client) fetchAllPRComments(ctx context.Context, owner, repo string, prNumber int) ([]apiComment, error) { + var all []apiComment + + page := 1 + for { + url := fmt.Sprintf("%s/repos/%s/%s/pulls/%d/comments?per_page=100&page=%d", apiBase, owner, repo, prNumber, page) + + req, err := http.NewRequestWithContext(ctx, http.MethodGet, url, nil) + if err != nil { + return nil, fmt.Errorf("creating request: %w", err) + } + + req.Header.Set("Authorization", "Bearer "+c.token) + req.Header.Set("Accept", "application/vnd.github.v3+json") + + resp, err := c.httpClient.Do(req) + if err != nil { + return nil, fmt.Errorf("fetching comments: %w", err) + } + + body, err := io.ReadAll(resp.Body) + resp.Body.Close() + + if resp.StatusCode != http.StatusOK { + return nil, fmt.Errorf("unexpected status %d: %s", resp.StatusCode, string(body)) + } + + var pageComments []apiComment + if err := json.Unmarshal(body, &pageComments); err != nil { + return nil, fmt.Errorf("parsing comments: %w", err) + } + + all = append(all, pageComments...) + + if len(pageComments) < 100 { + break + } + page++ + } + + return all, nil +} + func (c *Client) FetchDiff(ctx context.Context, owner, repo string, prNumber int) (string, error) { url := fmt.Sprintf("%s/repos/%s/%s/pulls/%d", apiBase, owner, repo, prNumber) @@ -55,24 +111,24 @@ func (c *Client) FetchDiff(ctx context.Context, owner, repo string, prNumber int return string(data), nil } -type reviewComment struct { +type submitComment struct { Path string `json:"path"` Line int `json:"line"` Body string `json:"body"` Side string `json:"side"` } -type reviewRequest struct { +type submitRequest struct { Event string `json:"event"` Body string `json:"body"` - Comments []reviewComment `json:"comments"` + Comments []submitComment `json:"comments"` CommitID string `json:"commit_id"` } func (c *Client) SubmitReview(ctx context.Context, owner, repo string, prNumber int, commitSHA string, result review.Result) error { - comments := make([]reviewComment, len(result.Comments)) + comments := make([]submitComment, len(result.Comments)) for i, rc := range result.Comments { - comments[i] = reviewComment{ + comments[i] = submitComment{ Path: rc.Path, Line: rc.Line, Body: rc.Body, @@ -80,7 +136,7 @@ func (c *Client) SubmitReview(ctx context.Context, owner, repo string, prNumber } } - payload := reviewRequest{ + payload := submitRequest{ Event: string(result.Verdict), Body: result.Summary, Comments: comments, @@ -118,119 +174,81 @@ func (c *Client) SubmitReview(ctx context.Context, owner, repo string, prNumber } type ThreadComment struct { - ID int64 `json:"id"` - InReplyToID int64 `json:"in_reply_to_id"` - Body string `json:"body"` + ID int64 + InReplyToID int64 + Body string UserLogin string - Path string `json:"path"` - Line int `json:"line"` - DiffHunk string `json:"diff_hunk"` - CreatedAt string `json:"created_at"` + Path string + Line int + DiffHunk string + CreatedAt string } -func (c *Client) FetchCommentThread(ctx context.Context, owner, repo string, prNumber int, commentID int64) ([]ThreadComment, error) { - var allComments []struct { - ID int64 `json:"id"` - InReplyToID int64 `json:"in_reply_to_id"` - Body string `json:"body"` - Path string `json:"path"` - Line int `json:"line"` - DiffHunk string `json:"diff_hunk"` - CreatedAt string `json:"created_at"` - User struct { - Login string `json:"login"` - } `json:"user"` +func toThreadComment(c apiComment) ThreadComment { + return ThreadComment{ + ID: c.ID, + InReplyToID: c.InReplyToID, + Body: c.Body, + UserLogin: c.User.Login, + Path: c.Path, + Line: c.Line, + DiffHunk: c.DiffHunk, + CreatedAt: c.CreatedAt, } +} - page := 1 - for { - url := fmt.Sprintf("%s/repos/%s/%s/pulls/%d/comments?per_page=100&page=%d", apiBase, owner, repo, prNumber, page) - - req, err := http.NewRequestWithContext(ctx, http.MethodGet, url, nil) - if err != nil { - return nil, fmt.Errorf("creating request: %w", err) - } - - req.Header.Set("Authorization", "Bearer "+c.token) - req.Header.Set("Accept", "application/vnd.github.v3+json") - - resp, err := c.httpClient.Do(req) - if err != nil { - return nil, fmt.Errorf("fetching comments: %w", err) - } - - body, err := io.ReadAll(resp.Body) - resp.Body.Close() - - if resp.StatusCode != http.StatusOK { - return nil, fmt.Errorf("unexpected status %d: %s", resp.StatusCode, string(body)) - } - - var pageComments []struct { - ID int64 `json:"id"` - InReplyToID int64 `json:"in_reply_to_id"` - Body string `json:"body"` - Path string `json:"path"` - Line int `json:"line"` - DiffHunk string `json:"diff_hunk"` - CreatedAt string `json:"created_at"` - User struct { - Login string `json:"login"` - } `json:"user"` - } - - if err := json.Unmarshal(body, &pageComments); err != nil { - return nil, fmt.Errorf("parsing comments: %w", err) - } - - allComments = append(allComments, pageComments...) - - if len(pageComments) < 100 { - break - } - page++ +func (c *Client) FetchCommentThread(ctx context.Context, owner, repo string, prNumber int, commentID int64) ([]ThreadComment, error) { + allComments, err := c.fetchAllPRComments(ctx, owner, repo, prNumber) + if err != nil { + return nil, err } - rootID := commentID - commentsByID := make(map[int64]struct { - ID int64 `json:"id"` - InReplyToID int64 `json:"in_reply_to_id"` - Body string `json:"body"` - Path string `json:"path"` - Line int `json:"line"` - DiffHunk string `json:"diff_hunk"` - CreatedAt string `json:"created_at"` - User struct { - Login string `json:"login"` - } `json:"user"` - }) - for _, c := range allComments { - commentsByID[c.ID] = c + commentsByID := make(map[int64]apiComment) + for _, ac := range allComments { + commentsByID[ac.ID] = ac } + rootID := commentID if target, ok := commentsByID[commentID]; ok && target.InReplyToID != 0 { rootID = target.InReplyToID } var thread []ThreadComment - for _, c := range allComments { - if c.ID == rootID || c.InReplyToID == rootID { - thread = append(thread, ThreadComment{ - ID: c.ID, - InReplyToID: c.InReplyToID, - Body: c.Body, - UserLogin: c.User.Login, - Path: c.Path, - Line: c.Line, - DiffHunk: c.DiffHunk, - CreatedAt: c.CreatedAt, - }) + for _, ac := range allComments { + if ac.ID == rootID || ac.InReplyToID == rootID { + thread = append(thread, toThreadComment(ac)) } } return thread, nil } +type PriorComment struct { + Path string + Line int + Body string +} + +func (c *Client) FetchBotReviewComments(ctx context.Context, owner, repo string, prNumber int, botLogin string) ([]PriorComment, error) { + allComments, err := c.fetchAllPRComments(ctx, owner, repo, prNumber) + if err != nil { + return nil, err + } + + var prior []PriorComment + for _, ac := range allComments { + if ac.User.Login == botLogin { + prior = append(prior, PriorComment{ + Path: ac.Path, + Line: ac.Line, + Body: ac.Body, + }) + } + } + + return prior, nil +} + type replyRequest struct { Body string `json:"body"` InReplyTo int64 `json:"in_reply_to"` diff --git a/internal/prompt/prompt.go b/internal/prompt/prompt.go index 01631dd..d204460 100644 --- a/internal/prompt/prompt.go +++ b/internal/prompt/prompt.go @@ -55,13 +55,26 @@ Rules: - Be collaborative, not adversarial - Output plain text only, no JSON, no markdown fences` -func Build(files []diff.File, instructions string) (string, string) { +type PriorComment struct { + Path string + Body string +} + +func Build(files []diff.File, instructions string, priorComments []PriorComment) (string, string) { var user strings.Builder if instructions != "" { fmt.Fprintf(&user, "Additional review instructions:\n%s\n\n", instructions) } + if len(priorComments) > 0 { + user.WriteString("You have already made the following review comments on this PR. Do NOT repeat these. Only raise new issues or comment on changes that address (or fail to address) your prior feedback:\n\n") + for _, pc := range priorComments { + fmt.Fprintf(&user, "- [%s] %s\n", pc.Path, pc.Body) + } + user.WriteString("\n") + } + user.WriteString("Review the following diff:\n\n") for _, f := range files { diff --git a/internal/review/review.go b/internal/review/review.go index 558a964..03364d9 100644 --- a/internal/review/review.go +++ b/internal/review/review.go @@ -13,9 +13,10 @@ import ( ) type Config struct { - Diff string - Provider provider.ReviewFunc - Instructions string + Diff string + Provider provider.ReviewFunc + Instructions string + PriorComments []prompt.PriorComment } func Run(ctx context.Context, cfg Config) (Result, error) { @@ -24,7 +25,7 @@ func Run(ctx context.Context, cfg Config) (Result, error) { return Result{}, fmt.Errorf("parsing diff: %w", err) } - system, user := prompt.Build(files, cfg.Instructions) + system, user := prompt.Build(files, cfg.Instructions, cfg.PriorComments) resp, err := cfg.Provider(ctx, provider.Request{ SystemPrompt: system, From e1430a45dd38f037e7a3ad6d3d03777575892054 Mon Sep 17 00:00:00 2001 From: Bailey Stoner Date: Sat, 14 Mar 2026 15:16:16 -0700 Subject: [PATCH 5/6] feat: fetch full file contents for review context On review, fetch the complete contents of each changed file so the LLM can understand surrounding code, not just the diff. Files over 256KB are skipped silently. Deleted files return empty and are also skipped. --- cmd/codereview/main.go | 29 ++++++++++++++++++++++++++++ internal/github/client.go | 40 +++++++++++++++++++++++++++++++++++++++ internal/prompt/prompt.go | 9 ++++++++- internal/review/review.go | 3 ++- 4 files changed, 79 insertions(+), 2 deletions(-) diff --git a/cmd/codereview/main.go b/cmd/codereview/main.go index 523c9c9..a84ada6 100644 --- a/cmd/codereview/main.go +++ b/cmd/codereview/main.go @@ -6,6 +6,7 @@ import ( "os" "github.com/monokrome/codereview/internal/action" + "github.com/monokrome/codereview/internal/diff" "github.com/monokrome/codereview/internal/github" "github.com/monokrome/codereview/internal/prompt" "github.com/monokrome/codereview/internal/provider" @@ -77,11 +78,14 @@ func runReview(ctx context.Context, cfg action.Config, providerFn provider.Revie }) } + fileContents := fetchChangedFiles(ctx, gh, cfg, diffText) + result, err := review.Run(ctx, review.Config{ Diff: diffText, Provider: providerFn, Instructions: cfg.Instructions, PriorComments: priorComments, + FileContents: fileContents, }) if err != nil { return fmt.Errorf("running review: %w", err) @@ -136,3 +140,28 @@ func runReply(ctx context.Context, cfg action.Config, providerFn provider.Review fmt.Fprintf(os.Stderr, "reply posted\n") return nil } + +func fetchChangedFiles(ctx context.Context, gh *github.Client, cfg action.Config, diffText string) map[string]string { + files, err := diff.Parse(diffText) + if err != nil { + fmt.Fprintf(os.Stderr, "warning: could not parse diff for file fetching: %v\n", err) + return nil + } + + contents := make(map[string]string) + for _, f := range files { + content, err := gh.FetchFile(ctx, cfg.Owner, cfg.Repo, cfg.CommitSHA, f.Path) + if err != nil { + fmt.Fprintf(os.Stderr, "warning: could not fetch %s: %v\n", f.Path, err) + continue + } + + if content == "" { + continue + } + + contents[f.Path] = content + } + + return contents +} diff --git a/internal/github/client.go b/internal/github/client.go index 58d1a1c..99ce92f 100644 --- a/internal/github/client.go +++ b/internal/github/client.go @@ -111,6 +111,46 @@ func (c *Client) FetchDiff(ctx context.Context, owner, repo string, prNumber int return string(data), nil } +const maxFileSize = 256 * 1024 // 256KB + +func (c *Client) FetchFile(ctx context.Context, owner, repo, ref, path string) (string, error) { + url := fmt.Sprintf("%s/repos/%s/%s/contents/%s?ref=%s", apiBase, owner, repo, path, ref) + + req, err := http.NewRequestWithContext(ctx, http.MethodGet, url, nil) + if err != nil { + return "", fmt.Errorf("creating request: %w", err) + } + + req.Header.Set("Authorization", "Bearer "+c.token) + req.Header.Set("Accept", "application/vnd.github.v3.raw") + + resp, err := c.httpClient.Do(req) + if err != nil { + return "", fmt.Errorf("fetching file: %w", err) + } + defer resp.Body.Close() + + if resp.StatusCode == http.StatusNotFound { + return "", nil + } + + if resp.StatusCode != http.StatusOK { + body, _ := io.ReadAll(resp.Body) + return "", fmt.Errorf("unexpected status %d: %s", resp.StatusCode, string(body)) + } + + data, err := io.ReadAll(io.LimitReader(resp.Body, maxFileSize+1)) + if err != nil { + return "", fmt.Errorf("reading file: %w", err) + } + + if len(data) > maxFileSize { + return "", nil + } + + return string(data), nil +} + type submitComment struct { Path string `json:"path"` Line int `json:"line"` diff --git a/internal/prompt/prompt.go b/internal/prompt/prompt.go index d204460..ffa1493 100644 --- a/internal/prompt/prompt.go +++ b/internal/prompt/prompt.go @@ -60,7 +60,7 @@ type PriorComment struct { Body string } -func Build(files []diff.File, instructions string, priorComments []PriorComment) (string, string) { +func Build(files []diff.File, instructions string, priorComments []PriorComment, fileContents map[string]string) (string, string) { var user strings.Builder if instructions != "" { @@ -75,6 +75,13 @@ func Build(files []diff.File, instructions string, priorComments []PriorComment) user.WriteString("\n") } + if len(fileContents) > 0 { + user.WriteString("Full file contents for context (use these to understand the surrounding code):\n\n") + for path, content := range fileContents { + fmt.Fprintf(&user, "=== %s ===\n%s\n\n", path, content) + } + } + user.WriteString("Review the following diff:\n\n") for _, f := range files { diff --git a/internal/review/review.go b/internal/review/review.go index 03364d9..b2462af 100644 --- a/internal/review/review.go +++ b/internal/review/review.go @@ -17,6 +17,7 @@ type Config struct { Provider provider.ReviewFunc Instructions string PriorComments []prompt.PriorComment + FileContents map[string]string } func Run(ctx context.Context, cfg Config) (Result, error) { @@ -25,7 +26,7 @@ func Run(ctx context.Context, cfg Config) (Result, error) { return Result{}, fmt.Errorf("parsing diff: %w", err) } - system, user := prompt.Build(files, cfg.Instructions, cfg.PriorComments) + system, user := prompt.Build(files, cfg.Instructions, cfg.PriorComments, cfg.FileContents) resp, err := cfg.Provider(ctx, provider.Request{ SystemPrompt: system, From c08f5afd56e02cf13a47f119cd97440dd77e9126 Mon Sep 17 00:00:00 2001 From: Bailey Stoner Date: Sat, 14 Mar 2026 15:19:09 -0700 Subject: [PATCH 6/6] feat: resolve review threads when concerns are addressed The LLM now returns structured replies indicating whether the concern is resolved. If resolved and the thread was started by the bot, it calls the GitHub GraphQL API to resolve the thread automatically. --- cmd/codereview/main.go | 22 +++++- internal/github/client.go | 161 ++++++++++++++++++++++++++++++++++++++ internal/prompt/prompt.go | 11 ++- internal/review/review.go | 18 ++++- 4 files changed, 206 insertions(+), 6 deletions(-) diff --git a/cmd/codereview/main.go b/cmd/codereview/main.go index a84ada6..53e3f8a 100644 --- a/cmd/codereview/main.go +++ b/cmd/codereview/main.go @@ -118,7 +118,7 @@ func runReply(ctx context.Context, cfg action.Config, providerFn provider.Review }) } - replyText, err := review.RunReply(ctx, review.ReplyConfig{ + result, err := review.RunReply(ctx, review.ReplyConfig{ Provider: providerFn, Thread: messages, DiffHunk: cfg.Comment.DiffHunk, @@ -133,10 +133,28 @@ func runReply(ctx context.Context, cfg action.Config, providerFn provider.Review replyTo = cfg.Comment.CommentID } - if err := gh.ReplyToComment(ctx, cfg.Owner, cfg.Repo, cfg.PRNumber, replyTo, replyText); err != nil { + if err := gh.ReplyToComment(ctx, cfg.Owner, cfg.Repo, cfg.PRNumber, replyTo, result.Reply); err != nil { return fmt.Errorf("posting reply: %w", err) } + if result.Resolved { + botLogin := cfg.BotLogin + if botLogin == "" { + botLogin = defaultBotLogin + } + + rootID := cfg.Comment.InReplyToID + if rootID == 0 { + rootID = cfg.Comment.CommentID + } + + if err := gh.ResolveThread(ctx, cfg.Owner, cfg.Repo, cfg.PRNumber, rootID, botLogin); err != nil { + fmt.Fprintf(os.Stderr, "warning: could not resolve thread: %v\n", err) + } else { + fmt.Fprintf(os.Stderr, "thread resolved\n") + } + } + fmt.Fprintf(os.Stderr, "reply posted\n") return nil } diff --git a/internal/github/client.go b/internal/github/client.go index 99ce92f..9cd8a11 100644 --- a/internal/github/client.go +++ b/internal/github/client.go @@ -329,3 +329,164 @@ func (c *Client) ReplyToComment(ctx context.Context, owner, repo string, prNumbe return nil } + +const graphqlEndpoint = "https://api.github.com/graphql" + +type graphqlRequest struct { + Query string `json:"query"` + Variables map[string]any `json:"variables"` +} + +type graphqlResponse struct { + Data json.RawMessage `json:"data"` + Errors []struct { + Message string `json:"message"` + } `json:"errors"` +} + +func (c *Client) ResolveThread(ctx context.Context, owner, repo string, prNumber int, rootCommentID int64, botLogin string) error { + threadID, isBotThread, err := c.findThreadByComment(ctx, owner, repo, prNumber, rootCommentID, botLogin) + if err != nil { + return fmt.Errorf("finding thread: %w", err) + } + + if !isBotThread { + return nil + } + + if threadID == "" { + return fmt.Errorf("thread not found for comment %d", rootCommentID) + } + + mutation := `mutation($threadId: ID!) { + resolveReviewThread(input: { threadId: $threadId }) { + thread { id isResolved } + } + }` + + return c.graphql(ctx, mutation, map[string]any{"threadId": threadID}) +} + +func (c *Client) findThreadByComment(ctx context.Context, owner, repo string, prNumber int, commentID int64, botLogin string) (string, bool, error) { + query := `query($owner: String!, $repo: String!, $prNumber: Int!, $cursor: String) { + repository(owner: $owner, name: $repo) { + pullRequest(number: $prNumber) { + reviewThreads(first: 100, after: $cursor) { + pageInfo { hasNextPage endCursor } + nodes { + id + comments(first: 1) { + nodes { databaseId author { login } } + } + } + } + } + } + }` + + vars := map[string]any{ + "owner": owner, + "repo": repo, + "prNumber": prNumber, + "cursor": nil, + } + + for { + body, err := c.graphqlRaw(ctx, query, vars) + if err != nil { + return "", false, err + } + + var result struct { + Repository struct { + PullRequest struct { + ReviewThreads struct { + PageInfo struct { + HasNextPage bool `json:"hasNextPage"` + EndCursor string `json:"endCursor"` + } `json:"pageInfo"` + Nodes []struct { + ID string `json:"id"` + Comments struct { + Nodes []struct { + DatabaseID int64 `json:"databaseId"` + Author struct { + Login string `json:"login"` + } `json:"author"` + } `json:"nodes"` + } `json:"comments"` + } `json:"nodes"` + } `json:"reviewThreads"` + } `json:"pullRequest"` + } `json:"repository"` + } + + if err := json.Unmarshal(body, &result); err != nil { + return "", false, fmt.Errorf("parsing response: %w", err) + } + + threads := result.Repository.PullRequest.ReviewThreads + for _, thread := range threads.Nodes { + if len(thread.Comments.Nodes) == 0 { + continue + } + + firstComment := thread.Comments.Nodes[0] + if firstComment.DatabaseID == commentID { + isBotThread := firstComment.Author.Login == botLogin + return thread.ID, isBotThread, nil + } + } + + if !threads.PageInfo.HasNextPage { + break + } + vars["cursor"] = threads.PageInfo.EndCursor + } + + return "", false, nil +} + +func (c *Client) graphql(ctx context.Context, query string, vars map[string]any) error { + _, err := c.graphqlRaw(ctx, query, vars) + return err +} + +func (c *Client) graphqlRaw(ctx context.Context, query string, vars map[string]any) (json.RawMessage, error) { + payload := graphqlRequest{Query: query, Variables: vars} + + body, err := json.Marshal(payload) + if err != nil { + return nil, fmt.Errorf("marshalling request: %w", err) + } + + req, err := http.NewRequestWithContext(ctx, http.MethodPost, graphqlEndpoint, strings.NewReader(string(body))) + if err != nil { + return nil, fmt.Errorf("creating request: %w", err) + } + + req.Header.Set("Authorization", "Bearer "+c.token) + req.Header.Set("Content-Type", "application/json") + + resp, err := c.httpClient.Do(req) + if err != nil { + return nil, fmt.Errorf("executing request: %w", err) + } + defer resp.Body.Close() + + respBody, err := io.ReadAll(resp.Body) + if err != nil { + return nil, fmt.Errorf("reading response: %w", err) + } + + var gqlResp graphqlResponse + if err := json.Unmarshal(respBody, &gqlResp); err != nil { + return nil, fmt.Errorf("parsing response: %w", err) + } + + if len(gqlResp.Errors) > 0 { + return nil, fmt.Errorf("graphql error: %s", gqlResp.Errors[0].Message) + } + + return gqlResp.Data, nil +} diff --git a/internal/prompt/prompt.go b/internal/prompt/prompt.go index ffa1493..16e6589 100644 --- a/internal/prompt/prompt.go +++ b/internal/prompt/prompt.go @@ -53,7 +53,16 @@ Rules: - If the developer's response resolves your concern, acknowledge it - If you still have concerns, explain why clearly - Be collaborative, not adversarial -- Output plain text only, no JSON, no markdown fences` + +Your response must be valid JSON matching this exact structure: +{ + "reply": "your reply text here", + "resolved": true +} + +- Set "resolved" to true if the developer's response adequately addresses your original concern +- Set "resolved" to false if the concern is not yet addressed or you have follow-up questions +- Output ONLY the JSON object, no markdown fences, no extra text` type PriorComment struct { Path string diff --git a/internal/review/review.go b/internal/review/review.go index b2462af..07aa60b 100644 --- a/internal/review/review.go +++ b/internal/review/review.go @@ -53,7 +53,12 @@ type ReplyConfig struct { Instructions string } -func RunReply(ctx context.Context, cfg ReplyConfig) (string, error) { +type ReplyResult struct { + Reply string `json:"reply"` + Resolved bool `json:"resolved"` +} + +func RunReply(ctx context.Context, cfg ReplyConfig) (ReplyResult, error) { system, user := prompt.BuildReply(cfg.Thread, cfg.DiffHunk, cfg.Instructions) resp, err := cfg.Provider(ctx, provider.Request{ @@ -61,10 +66,17 @@ func RunReply(ctx context.Context, cfg ReplyConfig) (string, error) { UserPrompt: user, }) if err != nil { - return "", fmt.Errorf("calling provider: %w", err) + return ReplyResult{}, fmt.Errorf("calling provider: %w", err) } - return strings.TrimSpace(stripMarkdownFences(resp.Content)), nil + cleaned := strings.TrimSpace(stripMarkdownFences(resp.Content)) + + var result ReplyResult + if err := json.Unmarshal([]byte(cleaned), &result); err != nil { + return ReplyResult{Reply: cleaned}, nil + } + + return result, nil } func ParseResponse(raw string) (Result, error) {