feat(pierre): add diff chunking and configurable review settings #2

Open
schreifuchs wants to merge 4 commits from feat/context-improvements into main
5 changed files with 56 additions and 23 deletions
Showing only changes of commit 2e12c39786 - Show all commits

View File

@@ -3,8 +3,10 @@ package main
import ( import (
"context" "context"
"log" "log"
"log/slog"
"os" "os"
"path/filepath" "path/filepath"
"strings"
"git.schreifuchs.ch/schreifuchs/pierre-bot/internal/chatter" "git.schreifuchs.ch/schreifuchs/pierre-bot/internal/chatter"
"git.schreifuchs.ch/schreifuchs/pierre-bot/internal/gitadapters/bitbucket" "git.schreifuchs.ch/schreifuchs/pierre-bot/internal/gitadapters/bitbucket"
@@ -46,11 +48,7 @@ type ReviewConfig struct {
} }
type Config struct { type Config struct {
// Deprecated flags (no prefix). Retained for backward compatibility. LogLevel string `help:"Log verbosity: debug, info, warn, error"`
// These will be mapped to the embedded ReviewConfig if provided.
MaxChunkSize int `help:"Deprecated: use --review-max-chunk-size"`
Guidelines []string `help:"Deprecated: use --review-guidelines" sep:","`
DisableComments bool `help:"Deprecated: use --review-disable-comments"`
// Embedding ReviewConfig with a prefix changes flag names to `--review-…`. // Embedding ReviewConfig with a prefix changes flag names to `--review-…`.
schreifuchs marked this conversation as resolved Outdated

cfg.Review.MaxChunkChars is an int that may be zero if the user omits the flag. The service correctly falls back to the default (60000) inside judgePR, but you could also enforce the default earlier (e.g. in the config struct default tag or after flag parsing) to avoid passing a zero value downstream. (Generated by: OpenAI (openai/gpt-oss-120b))

`cfg.Review.MaxChunkChars` is an `int` that may be zero if the user omits the flag. The service correctly falls back to the default (60000) inside `judgePR`, but you could also enforce the default earlier (e.g. in the config struct default tag or after flag parsing) to avoid passing a zero value downstream. (Generated by: OpenAI (openai/gpt-oss-120b))
// Existing configuration files using the old flag names will need to be updated. // Existing configuration files using the old flag names will need to be updated.
@@ -81,17 +79,24 @@ func main() {
kong.Configuration(kongyaml.Loader, "config.yaml", defaultConfig), kong.Configuration(kongyaml.Loader, "config.yaml", defaultConfig),
) )
// Backwards compatibility: map deprecated flag values (if any) to the embedded ReviewConfig. // Configure global slog logger based on the --log-level flag
if cfg.MaxChunkSize != 0 { lvl := slog.LevelInfo
cfg.Review.MaxChunkSize = cfg.MaxChunkSize switch strings.ToLower(cfg.LogLevel) {
} case "debug":
if len(cfg.Guidelines) > 0 { lvl = slog.LevelDebug
cfg.Review.Guidelines = cfg.Guidelines case "info":
} lvl = slog.LevelInfo
if cfg.DisableComments { case "warn":
cfg.Review.DisableComments = cfg.DisableComments lvl = slog.LevelWarn
case "error":
lvl = slog.LevelError
} }
// Logs are sent to stderr so that stdout can be safely piped.
// The review output (fmt.Printf) stays on stdout unchanged.
logger := slog.New(slog.NewTextHandler(os.Stderr, &slog.HandlerOptions{Level: lvl}))
slog.SetDefault(logger)
// Auto-detect provider // Auto-detect provider
provider := cfg.GitProvider provider := cfg.GitProvider
if provider == "" { if provider == "" {

View File

@@ -5,6 +5,7 @@ import (
"encoding/json" "encoding/json"
"fmt" "fmt"
"io" "io"
"log/slog"
"net/http" "net/http"
"strconv" "strconv"
"strings" "strings"
@@ -13,6 +14,7 @@ import (
) )
func (b *BitbucketAdapter) GetDiff(ctx context.Context, projectKey, repositorySlug string, pullRequestID int) (diff io.ReadCloser, err error) { func (b *BitbucketAdapter) GetDiff(ctx context.Context, projectKey, repositorySlug string, pullRequestID int) (diff io.ReadCloser, err error) {
slog.Debug("Bitbucket GetDiff start", "project", projectKey, "repo", repositorySlug, "pr", pullRequestID)
r, err := b.CreateRequest( r, err := b.CreateRequest(
ctx, ctx,
http.MethodGet, http.MethodGet,
@@ -39,6 +41,7 @@ func (b *BitbucketAdapter) GetDiff(ctx context.Context, projectKey, repositorySl
} }
func (b *BitbucketAdapter) GetPR(ctx context.Context, projectKey, repositorySlug string, pullRequestID int) (pr PullRequest, err error) { func (b *BitbucketAdapter) GetPR(ctx context.Context, projectKey, repositorySlug string, pullRequestID int) (pr PullRequest, err error) {
slog.Debug("Bitbucket GetPR start", "project", projectKey, "repo", repositorySlug, "pr", pullRequestID)
r, err := b.CreateRequest( r, err := b.CreateRequest(
ctx, ctx,
http.MethodGet, http.MethodGet,
@@ -53,11 +56,17 @@ func (b *BitbucketAdapter) GetPR(ctx context.Context, projectKey, repositorySlug
defer response.Body.Close() // Add this defer response.Body.Close() // Add this
err = json.NewDecoder(response.Body).Decode(&pr) err = json.NewDecoder(response.Body).Decode(&pr)
if err != nil {
slog.Error("Bitbucket GetPR decode error", "err", err)
return
}
slog.Info("Bitbucket GetPR success", "id", pullRequestID)
return return
} }
func (b *BitbucketAdapter) AddComment(ctx context.Context, owner, repo string, prID int, comment pierre.Comment) (err error) { func (b *BitbucketAdapter) AddComment(ctx context.Context, owner, repo string, prID int, comment pierre.Comment) (err error) {
slog.Debug("Bitbucket AddComment start", "owner", owner, "repo", repo, "pr", prID, "file", comment.File, "line", comment.Line)
// pr, err := b.GetPR(ctx, owner, repo, prID) // pr, err := b.GetPR(ctx, owner, repo, prID)
// if err != nil { // if err != nil {
// return // return
@@ -95,6 +104,9 @@ func (b *BitbucketAdapter) AddComment(ctx context.Context, owner, repo string, p
sb := &strings.Builder{} sb := &strings.Builder{}
io.Copy(sb, response.Body) io.Copy(sb, response.Body)
err = fmt.Errorf("error while creating comment staus %d, body %s", response.StatusCode, sb.String()) err = fmt.Errorf("error while creating comment staus %d, body %s", response.StatusCode, sb.String())
slog.Error("Bitbucket AddComment failed", "status", response.StatusCode, "err", err)
} else {
slog.Info("Bitbucket AddComment succeeded", "pr", prID)
} }
return err return err

View File

@@ -5,6 +5,7 @@ import (
"context" "context"
"fmt" "fmt"
"io" "io"
"log/slog"
"code.gitea.io/sdk/gitea" "code.gitea.io/sdk/gitea"
"git.schreifuchs.ch/schreifuchs/pierre-bot/internal/pierre" "git.schreifuchs.ch/schreifuchs/pierre-bot/internal/pierre"
@@ -25,15 +26,19 @@ func New(baseURL, token string) (*Adapter, error) {
} }
func (g *Adapter) GetDiff(ctx context.Context, owner, repo string, prID int) (io.ReadCloser, error) { func (g *Adapter) GetDiff(ctx context.Context, owner, repo string, prID int) (io.ReadCloser, error) {
slog.Debug("Gitea GetDiff start", "owner", owner, "repo", repo, "pr", prID)
g.client.SetContext(ctx) g.client.SetContext(ctx)
diff, _, err := g.client.GetPullRequestDiff(owner, repo, int64(prID), gitea.PullRequestDiffOptions{}) diff, _, err := g.client.GetPullRequestDiff(owner, repo, int64(prID), gitea.PullRequestDiffOptions{})
if err != nil { if err != nil {
return nil, err return nil, err
} }
return io.NopCloser(bytes.NewReader(diff)), nil rc := io.NopCloser(bytes.NewReader(diff))
slog.Info("Gitea GetDiff success", "owner", owner, "repo", repo, "pr", prID, "bytes", len(diff))
return rc, nil
} }
func (g *Adapter) AddComment(ctx context.Context, owner, repo string, prID int, comment pierre.Comment) error { func (g *Adapter) AddComment(ctx context.Context, owner, repo string, prID int, comment pierre.Comment) error {
slog.Debug("Gitea AddComment start", "owner", owner, "repo", repo, "pr", prID, "file", comment.File, "line", comment.Line)
g.client.SetContext(ctx) g.client.SetContext(ctx)
opts := gitea.CreatePullReviewOptions{ opts := gitea.CreatePullReviewOptions{
State: gitea.ReviewStateComment, State: gitea.ReviewStateComment,
@@ -46,22 +51,30 @@ func (g *Adapter) AddComment(ctx context.Context, owner, repo string, prID int,
}, },
} }
_, _, err := g.client.CreatePullReview(owner, repo, int64(prID), opts) _, _, err := g.client.CreatePullReview(owner, repo, int64(prID), opts)
return err if err != nil {
slog.Error("Gitea AddComment failed", "err", err)
return err
}
slog.Info("Gitea AddComment succeeded", "pr", prID)
return nil
} }
// GetFileContent returns the file content at a given path and ref (commit SHA). // GetFileContent returns the file content at a given path and ref (commit SHA).
func (g *Adapter) GetFileContent(ctx context.Context, owner, repo, path, ref string) (string, error) { func (g *Adapter) GetFileContent(ctx context.Context, owner, repo, path, ref string) (string, error) {
slog.Debug("Gitea GetFileContent start", "owner", owner, "repo", repo, "path", path, "ref", ref)
g.client.SetContext(ctx) g.client.SetContext(ctx)
// The SDK's GetFile returns the raw bytes of the file. // The SDK's GetFile returns the raw bytes of the file.
data, _, err := g.client.GetFile(owner, repo, ref, path) data, _, err := g.client.GetFile(owner, repo, ref, path)
if err != nil { if err != nil {
return "", err return "", err
} }
slog.Info("Gitea GetFileContent success", "bytes", len(data))
return string(data), nil return string(data), nil
} }
// GetPRHeadSHA fetches the pull request and returns the head commit SHA. // GetPRHeadSHA fetches the pull request and returns the head commit SHA.
func (g *Adapter) GetPRHeadSHA(ctx context.Context, owner, repo string, prID int) (string, error) { func (g *Adapter) GetPRHeadSHA(ctx context.Context, owner, repo string, prID int) (string, error) {
slog.Debug("Gitea GetPRHeadSHA start", "owner", owner, "repo", repo, "pr", prID)
g.client.SetContext(ctx) g.client.SetContext(ctx)
// GetPullRequest returns the detailed PR information. // GetPullRequest returns the detailed PR information.
pr, _, err := g.client.GetPullRequest(owner, repo, int64(prID)) pr, _, err := g.client.GetPullRequest(owner, repo, int64(prID))
@@ -71,5 +84,6 @@ func (g *Adapter) GetPRHeadSHA(ctx context.Context, owner, repo string, prID int
if pr == nil || pr.Head == nil { if pr == nil || pr.Head == nil {
return "", fmt.Errorf("pull request %d has no head information", prID) return "", fmt.Errorf("pull request %d has no head information", prID)
} }
slog.Info("Gitea GetPRHeadSHA success", "sha", pr.Head.Sha)
return pr.Head.Sha, nil return pr.Head.Sha, nil
} }

View File

@@ -1,6 +1,8 @@
package pierre package pierre
import ( import (
"log/slog"
"context" "context"
"fmt" "fmt"
"io" "io"
@@ -19,6 +21,7 @@ type Comment struct {
} }
func (s *Service) judgePR(ctx context.Context, diff io.Reader) (comments []Comment, err error) { func (s *Service) judgePR(ctx context.Context, diff io.Reader) (comments []Comment, err error) {
slog.Info("judgePR started")
diffBytes, err := io.ReadAll(diff) diffBytes, err := io.ReadAll(diff)
if err != nil { if err != nil {
return nil, fmt.Errorf("failed to read diff: %w", err) return nil, fmt.Errorf("failed to read diff: %w", err)
@@ -81,6 +84,7 @@ If project guidelines are provided, treat them as hard rules that must be respec
for _, v := range unique { for _, v := range unique {
comments = append(comments, v) comments = append(comments, v)
} }
slog.Info("judgePR finished", "comments", len(comments))
return return
} }

View File

@@ -69,13 +69,11 @@ func (s *Service) MakeReview(ctx context.Context, organisation string, repo stri
for _, c := range comments { for _, c := range comments {
c.Message = fmt.Sprintf("%s (Generated by: %s)", c.Message, model) c.Message = fmt.Sprintf("%s (Generated by: %s)", c.Message, model)
if s.disableComments { // Normal mode: print to stdout and post the comment to the VCS.
// Dryrun: just log what would have been posted. fmt.Printf("File: %s\nLine: %d\nMessage: %s\n%s\n",
log.Printf("dryrun: %s:%d => %s", c.File, c.Line, c.Message) c.File, c.Line, c.Message, "---")
} else {
// Normal mode: print to stdout and post the comment to the VCS. if !s.disableComments {
fmt.Printf("File: %s\nLine: %d\nMessage: %s\n%s\n",
c.File, c.Line, c.Message, "---")
if err := s.git.AddComment(ctx, organisation, repo, prID, c); err != nil { if err := s.git.AddComment(ctx, organisation, repo, prID, c); err != nil {
log.Printf("Failed to add comment: %v", err) log.Printf("Failed to add comment: %v", err)
} }