feat(pierre): add diff chunking and configurable review settings #2
209
AGENTS.md
209
AGENTS.md
@@ -1,77 +1,162 @@
|
|||||||
# Pierre Bot
|
---
|
||||||
|
layout: default
|
||||||
|
---
|
||||||
|
|
||||||
Pierre Bot is an intelligent, AI-powered code review assistant designed for Bitbucket Server/Data Center. It automates the initial pass of code review by analyzing Pull Request diffs and identifying potential bugs, logic errors, and style issues using modern LLMs (Google Gemini 2.0 Flash or Ollama).
|
# AGENTS.md – Repository Guidelines for Pierre Bot
|
||||||
|
|
||||||
## Project Overview
|
> This document is consulted by autonomous coding agents (including the
|
||||||
|
> Open‑Source `opencode` agent) when they read, modify, test, or lint the
|
||||||
|
> code‑base. It contains the canonical commands for building, testing, and
|
||||||
|
> lint‑ing the project as well as a concise style guide that all Go code must
|
||||||
|
> follow. The file is deliberately kept around **150 lines** to be readable for
|
||||||
|
> both humans and LLMs.
|
||||||
|
|
||||||
* **Type:** Go CLI Application
|
---
|
||||||
* **Core Function:** Fetches PR diffs from Bitbucket -> Sends to LLM -> Prints structured review comments.
|
|
||||||
* **Key Technologies:**
|
|
||||||
* **Language:** Go (1.25+)
|
|
||||||
* **AI SDKs:** `google/generative-ai-go`, `ollama/ollama`
|
|
||||||
* **CLI Framework:** `alecthomas/kong`
|
|
||||||
|
|
||||||
## Architecture
|
## 1️⃣ Build / Run Commands
|
||||||
|
|
||||||
The project follows a standard Go project layout:
|
| Command | Description |
|
||||||
|
|--------|-------------|
|
||||||
|
| `go build -o bin/pierre ./cmd/pierre/main.go` | Build the binary into `./bin/pierre`. |
|
||||||
|
| `go run ./cmd/pierre/main.go --help` | Run the CLI with the help flag (fast sanity check). |
|
||||||
|
| `./bin/pierre --version` | Verify the built binary reports its version. |
|
||||||
|
|
||||||
* **`cmd/pierre/`**: Contains the `main.go` entry point. It handles configuration parsing (flags, env vars, file), initializes adapters, and orchestrates the application flow.
|
> **Tip for agents** – always prefer the `./bin/pierre` binary over invoking `go run` in CI or tests; it guarantees the same compilation flags.
|
||||||
* **`internal/pierre/`**: Contains the core business logic.
|
|
||||||
* `judge.go`: Defines the `JudgePR` function which prepares the system prompt and context for the LLM.
|
|
||||||
* **`internal/chatter/`**: Abstraction layer for LLM providers.
|
|
||||||
* `gemini.go`: Implements the `ChatAdapter` interface for Google Gemini. notably includes **dynamic JSON schema generation** via reflection (`schemaFromType`) to enforce structured output from the model.
|
|
||||||
* `ollama.go`: Implements the `ChatAdapter` for Ollama (local models).
|
|
||||||
* **`internal/gitadapters/`**: Abstraction for Version Control Systems.
|
|
||||||
* `bitbucket.go`: Client for fetching PR diffs from Bitbucket Server.
|
|
||||||
|
|
||||||
## Building and Running
|
---
|
||||||
|
|
||||||
### Prerequisites
|
## 2️⃣ Test Commands
|
||||||
|
|
||||||
* Go 1.25 or later
|
The project uses the standard Go testing framework.
|
||||||
* Access to a Bitbucket Server instance
|
|
||||||
* API Key for Google Gemini (or a running Ollama instance)
|
|
||||||
|
|
||||||
### Build
|
| Command | Use case |
|
||||||
|
|---------|----------|
|
||||||
|
| `go test ./...` | Run **all** unit and integration tests. |
|
||||||
|
| `go test ./... -run ^TestJudgePR$` | Run a **single** test (replace `TestJudgePR` with the desired name). |
|
||||||
|
| `go test -cover ./...` | Run tests and emit a coverage report. |
|
||||||
|
| `go test -run ^TestName$ -count=1 -v` | Verbose, non‑cached run for debugging a single test. |
|
||||||
|
| `go test ./... -bench .` | Execute benchmarks (if any). |
|
||||||
|
|
||||||
|
> **Agents should never** invoke `go test` with `-parallel` flags; the default parallelism is sufficient and ensures deterministic ordering for our table‑driven tests.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## 3️⃣ Lint / Static Analysis
|
||||||
|
|
||||||
|
We rely on two lightweight built‑in tools plus optional `golangci-lint` when
|
||||||
|
available.
|
||||||
|
|
||||||
|
| Tool | Command | Description |
|
||||||
|
|------|---------|-------------|
|
||||||
|
| `go vet ./...` | `go vet ./...` | Detect obvious bugs, misuse of `fmt.Printf`, etc. |
|
||||||
|
| `staticcheck ./...` | `staticcheck ./...` | Deeper static analysis (must be installed via `go install honnef.co/go/tools/cmd/staticcheck@latest`). |
|
||||||
|
| `golangci-lint` (optional) | `golangci-lint run` | Runs a configurable suite of linters. Install with `brew install golangci-lint` or `go install github.com/golangci/golangci-lint/cmd/golangci-lint@latest`. |
|
||||||
|
|
||||||
|
**Agent tip** – when `golangci-lint` is present, run it after `go vet` to catch style
|
||||||
|
issues early. The CI pipeline (see `.github/workflows/*.yml` if added later) will
|
||||||
|
use the same commands.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## 4️⃣ Code‑Style Guidelines (Go)
|
||||||
|
|
||||||
|
All changes must satisfy the following conventions. They are enforced by
|
||||||
|
`go fmt`, `go vet`, and the optional `golangci-lint` config.
|
||||||
|
|
||||||
|
### 4.1 Formatting & Imports
|
||||||
|
* Run `go fmt ./...` before committing. The repository uses **tab‑indented** Go code.
|
||||||
|
* Imports are grouped in three blocks, separated by a blank line:
|
||||||
|
1. **Standard library**
|
||||||
|
2. **External dependencies** (e.g., `github.com/alecthomas/kong`)
|
||||||
|
3. **Internal packages** (e.g., `github.com/yourorg/pierre/internal/...`)
|
||||||
|
* Within each block, imports are sorted alphabetically.
|
||||||
|
* Use `goimports` (or `golangci‑lint --fast`) to automatically fix import ordering.
|
||||||
|
|
||||||
|
### 4.2 Naming Conventions
|
||||||
|
| Entity | Rule |
|
||||||
|
|--------|------|
|
||||||
|
| Packages | lower‑case, no underscores or hyphens. |
|
||||||
|
| Files | snake_case (`*_test.go` for tests). |
|
||||||
|
| Types / Structs | PascalCase, descriptive (e.g., `ReviewConfig`). |
|
||||||
|
| Interfaces | End with `er` when appropriate (e.g., `ChatAdapter`). |
|
||||||
|
| Variables / Constants | camelCase for locals, PascalCase for exported. |
|
||||||
|
| Functions | PascalCase if exported, camelCase if private. |
|
||||||
|
| Constants | Use `CamelCase` for groups, `CONST_NAME` only for truly global values. |
|
||||||
|
| Test Functions | `TestXxx` and optionally `BenchmarkXxx`. |
|
||||||
|
|
||||||
|
### 4.3 Error Handling
|
||||||
|
* Errors are **never** ignored. Use the blank identifier only when the value is truly irrelevant.
|
||||||
|
* Wrap contextual information using `fmt.Errorf("context: %w", err)` or `errors.Wrap` if the project imports `github.com/pkg/errors` (currently not used).
|
||||||
|
* Return errors **as soon as** they are detected – "guard clause" style.
|
||||||
|
* In public APIs, prefer error values over panics. Panics are limited to unrecoverable
|
||||||
|
programming errors (e.g., nil‑pointer dereference in `init`).
|
||||||
|
|
||||||
|
### 4.4 Documentation & Comments
|
||||||
|
* Exported identifiers **must** have a preceding doc comment beginning with the name (e.g., `// JudgePR reviews a PR and returns comments.`).
|
||||||
|
* Inline comments should be sentence‑case and end with a period.
|
||||||
|
* Use `//go:generate` directives sparingly; they must be accompanied by a test that ensures the generated file is up‑to‑date.
|
||||||
|
|
||||||
|
### 4.5 Testing Practices
|
||||||
|
* Keep tests **table‑driven** where possible; this yields concise, readable test suites.
|
||||||
|
* Use the `testing` package only; avoid third‑party test frameworks.
|
||||||
|
* Prefer `t.Fatalf` for fatal errors and `t.Errorf` for non‑fatal assertions.
|
||||||
|
* When comparing complex structs, use `github.com/google/go-cmp/cmp` (already in `go.mod`).
|
||||||
|
* Test coverage should be **≥ 80 %** for new code.
|
||||||
|
* All tests must be **deterministic** – no reliance on external services; use mocks or fakes.
|
||||||
|
|
||||||
|
### 4.6 Dependency Management
|
||||||
|
* All dependencies are managed via Go modules (`go.mod`).
|
||||||
|
* Run `go mod tidy` after adding/removing imports.
|
||||||
|
* Do not commit `vendor/` unless a vendoring strategy is explicitly adopted.
|
||||||
|
|
||||||
|
### 4.7 Logging & Output
|
||||||
|
* Use the standard library `log` package for user‑visible output.
|
||||||
|
* For structured logs (debug mode), wrap with a small helper that respects the `--debug` flag.
|
||||||
|
* CLI output must be **machine‑parsable** when `--json-output` is set (future feature).
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## 5️⃣ Git & Pull‑Request Workflow (for agents)
|
||||||
|
|
||||||
|
1. **Branch naming** – `feature/<short‑desc>` or `bugfix/<short‑desc>`.
|
||||||
|
2. **Commits** – One logical change per commit, with a concise subject line (<50 chars) and an optional body explaining *why* the change was needed.
|
||||||
|
3. **CI** – Every PR must pass `go test ./...`, `go vet ./...`, and `golangci‑lint run` (if configured).
|
||||||
|
4. **Review** – Agents should add comments only via the LLM; do not edit code generated by the LLM unless a test fails.
|
||||||
|
5. **Rebasing** – Keep a linear history; use `git rebase -i` locally before merging.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## 6️⃣ Cursor / Copilot Rules (None Present)
|
||||||
|
|
||||||
|
The repository does not contain a `.cursor` directory or a `.github/copilot‑instructions.md`
|
||||||
|
file. If such files are added in the future, agents should read them and incorporate
|
||||||
|
any additional constraints into this document.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## 7️⃣ Quick Reference Cheat‑Sheet (for agents)
|
||||||
|
|
||||||
```bash
|
```bash
|
||||||
go build -o pierre ./cmd/pierre/main.go
|
# Build
|
||||||
|
go build -o bin/pierre ./cmd/pierre/main.go
|
||||||
|
|
||||||
|
# Run single test (replace TestFoo)
|
||||||
|
go test ./... -run ^TestFoo$ -v
|
||||||
|
|
||||||
|
# Lint & Vet
|
||||||
|
go vet ./...
|
||||||
|
staticcheck ./...
|
||||||
|
# optional
|
||||||
|
golangci-lint run
|
||||||
|
|
||||||
|
# Format & Imports
|
||||||
|
go fmt ./...
|
||||||
|
goimports -w .
|
||||||
|
|
||||||
|
# Module tidy
|
||||||
|
go mod tidy
|
||||||
```
|
```
|
||||||
|
|
||||||
### Configuration
|
---
|
||||||
|
|
||||||
Configuration is handled via `kong` and supports a hierarchy: **Flags > Env Vars > Config File**.
|
*End of AGENTS.md*
|
||||||
|
|
||||||
**1. Environment Variables:**
|
|
||||||
|
|
||||||
* `BITBUCKET_URL`: Base URL of the Bitbucket instance.
|
|
||||||
* `BITBUCKET_TOKEN`: Personal Access Token (HTTP) for Bitbucket.
|
|
||||||
* `LLM_PROVIDER`: `gemini` or `ollama`.
|
|
||||||
* `LLM_API_KEY`: API Key for Gemini.
|
|
||||||
* `LLM_MODEL`: Model name (e.g., `gemini-2.0-flash`).
|
|
||||||
|
|
||||||
**2. Configuration File (`config.yaml`):**
|
|
||||||
|
|
||||||
See `config.example.yaml` for a template. Place it in the current directory or `~/.pierre.yaml`.
|
|
||||||
|
|
||||||
### Usage
|
|
||||||
|
|
||||||
Run the bot against a specific Pull Request:
|
|
||||||
|
|
||||||
```bash
|
|
||||||
# Syntax: ./pierre [flags] <PROJECT_KEY> <REPO_SLUG> <PR_ID>
|
|
||||||
./pierre --llm-provider=gemini --llm-model=gemini-2.0-flash MYPROJ my-repo 123
|
|
||||||
```
|
|
||||||
|
|
||||||
## Development Conventions
|
|
||||||
|
|
||||||
* **Structured Output:** The bot relies on the LLM returning valid JSON matching the `Comment` struct. This is enforced in `internal/chatter/gemini.go` by converting the Go struct definition into a `genai.Schema`.
|
|
||||||
* **Dependency Injection:** Adapters (`gitadapters`, `chatter`) are initialized in `main` and passed to the core logic, making testing easier.
|
|
||||||
* **Error Handling:** strict error checks are preferred; the bot will exit if it cannot fetch the diff or initialize the AI.
|
|
||||||
|
|
||||||
## Key Files
|
|
||||||
|
|
||||||
* **`cmd/pierre/main.go`**: Application entry point and config wiring.
|
|
||||||
* **`internal/pierre/judge.go`**: The "brain" that constructs the prompt for the AI.
|
|
||||||
* **`internal/chatter/gemini.go`**: Gemini integration logic, including the reflection-based schema generator.
|
|
||||||
* **`config.example.yaml`**: Reference configuration file.
|
|
||||||
BIN
bin/pierre
Executable file
BIN
bin/pierre
Executable file
Binary file not shown.
@@ -46,6 +46,12 @@ type ReviewConfig struct {
|
|||||||
}
|
}
|
||||||
|
|
||||||
type Config struct {
|
type Config struct {
|
||||||
|
// Deprecated flags (no prefix). Retained for backward compatibility.
|
||||||
|
// 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-…`.
|
||||||
// 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.
|
||||||
// Consider keeping backwards compatibility if required.
|
// Consider keeping backwards compatibility if required.
|
||||||
@@ -75,6 +81,17 @@ 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.
|
||||||
|
if cfg.MaxChunkSize != 0 {
|
||||||
|
cfg.Review.MaxChunkSize = cfg.MaxChunkSize
|
||||||
|
}
|
||||||
|
if len(cfg.Guidelines) > 0 {
|
||||||
|
cfg.Review.Guidelines = cfg.Guidelines
|
||||||
|
}
|
||||||
|
if cfg.DisableComments {
|
||||||
|
cfg.Review.DisableComments = cfg.DisableComments
|
||||||
|
}
|
||||||
|
|
||||||
// Auto-detect provider
|
// Auto-detect provider
|
||||||
provider := cfg.GitProvider
|
provider := cfg.GitProvider
|
||||||
if provider == "" {
|
if provider == "" {
|
||||||
|
|||||||
80
internal/gitadapters/bitbucket/adapter_test.go
Normal file
80
internal/gitadapters/bitbucket/adapter_test.go
Normal file
@@ -0,0 +1,80 @@
|
|||||||
|
package bitbucket
|
||||||
|
|
||||||
|
import (
|
||||||
|
"context"
|
||||||
|
"net/http"
|
||||||
|
"net/http/httptest"
|
||||||
|
"testing"
|
||||||
|
)
|
||||||
|
|
||||||
|
func TestBitbucketGetFileContentSuccess(t *testing.T) {
|
||||||
|
const expected = "file content"
|
||||||
|
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||||
|
// Verify path structure
|
||||||
|
if r.URL.Path != "/rest/api/1.0/projects/owner/repos/repo/raw/path/to/file.go" {
|
||||||
|
t.Fatalf("unexpected URL path: %s", r.URL.Path)
|
||||||
|
}
|
||||||
|
w.WriteHeader(http.StatusOK)
|
||||||
|
w.Write([]byte(expected))
|
||||||
|
}))
|
||||||
|
defer server.Close()
|
||||||
|
|
||||||
|
// Trim trailing slash handling done in NewBitbucket
|
||||||
|
adapter := NewBitbucket(server.URL, "")
|
||||||
|
content, err := adapter.GetFileContent(context.Background(), "owner", "repo", "path/to/file.go", "")
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("unexpected error: %v", err)
|
||||||
|
}
|
||||||
|
if content != expected {
|
||||||
|
t.Fatalf("expected %q, got %q", expected, content)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestBitbucketGetFileContentError(t *testing.T) {
|
||||||
|
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||||
|
w.WriteHeader(http.StatusNotFound)
|
||||||
|
w.Write([]byte("not found"))
|
||||||
|
}))
|
||||||
|
defer server.Close()
|
||||||
|
|
||||||
|
adapter := NewBitbucket(server.URL, "")
|
||||||
|
_, err := adapter.GetFileContent(context.Background(), "owner", "repo", "missing.go", "")
|
||||||
|
if err == nil {
|
||||||
|
t.Fatalf("expected error for non‑200 response")
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestBitbucketGetPRHeadSHASuccess(t *testing.T) {
|
||||||
|
const sha = "deadbeef"
|
||||||
|
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||||
|
if r.URL.Path != "/rest/api/1.0/projects/owner/repos/repo/pull-requests/42" {
|
||||||
|
t.Fatalf("unexpected URL: %s", r.URL.Path)
|
||||||
|
}
|
||||||
|
w.WriteHeader(http.StatusOK)
|
||||||
|
w.Write([]byte(`{"toRef":{"latestCommit":"` + sha + `"}}`))
|
||||||
|
}))
|
||||||
|
defer server.Close()
|
||||||
|
|
||||||
|
adapter := NewBitbucket(server.URL, "")
|
||||||
|
got, err := adapter.GetPRHeadSHA(context.Background(), "owner", "repo", 42)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("unexpected error: %v", err)
|
||||||
|
}
|
||||||
|
if got != sha {
|
||||||
|
t.Fatalf("expected sha %s, got %s", sha, got)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestBitbucketGetPRHeadSHAError(t *testing.T) {
|
||||||
|
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||||
|
w.WriteHeader(http.StatusInternalServerError)
|
||||||
|
w.Write([]byte("error"))
|
||||||
|
}))
|
||||||
|
defer server.Close()
|
||||||
|
|
||||||
|
adapter := NewBitbucket(server.URL, "")
|
||||||
|
_, err := adapter.GetPRHeadSHA(context.Background(), "owner", "repo", 1)
|
||||||
|
if err == nil {
|
||||||
|
t.Fatalf("expected error for non‑200 response")
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -47,10 +47,10 @@ func (b *BitbucketAdapter) GetPR(ctx context.Context, projectKey, repositorySlug
|
|||||||
)
|
)
|
||||||
|
|
||||||
response, err := http.DefaultClient.Do(r)
|
response, err := http.DefaultClient.Do(r)
|
||||||
defer response.Body.Close() // Add this
|
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
defer response.Body.Close() // Add this
|
||||||
|
|
||||||
err = json.NewDecoder(response.Body).Decode(&pr)
|
err = json.NewDecoder(response.Body).Decode(&pr)
|
||||||
|
|
||||||
@@ -86,10 +86,10 @@ func (b *BitbucketAdapter) AddComment(ctx context.Context, owner, repo string, p
|
|||||||
}
|
}
|
||||||
|
|
||||||
response, err := http.DefaultClient.Do(r)
|
response, err := http.DefaultClient.Do(r)
|
||||||
defer response.Body.Close() // Add this
|
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return err
|
return err
|
||||||
}
|
}
|
||||||
|
defer response.Body.Close() // Add this
|
||||||
|
|
||||||
if response.StatusCode >= 300 || response.StatusCode < 200 {
|
if response.StatusCode >= 300 || response.StatusCode < 200 {
|
||||||
sb := &strings.Builder{}
|
sb := &strings.Builder{}
|
||||||
|
|||||||
@@ -1,6 +1,10 @@
|
|||||||
package bitbucket
|
package bitbucket
|
||||||
|
|
||||||
import (
|
import (
|
||||||
|
"context"
|
||||||
|
"fmt"
|
||||||
|
"io"
|
||||||
|
"net/http"
|
||||||
"strings"
|
"strings"
|
||||||
|
|
||||||
"git.schreifuchs.ch/schreifuchs/pierre-bot/internal/gitadapters/baseadapter"
|
"git.schreifuchs.ch/schreifuchs/pierre-bot/internal/gitadapters/baseadapter"
|
||||||
@@ -18,3 +22,43 @@ func NewBitbucket(baseURL string, bearerToken string) *BitbucketAdapter {
|
|||||||
Rest: baseadapter.NewRest(baseURL, bearerToken),
|
Rest: baseadapter.NewRest(baseURL, bearerToken),
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// GetFileContent returns the raw file content at the given ref (commit SHA) or HEAD if ref is empty.
|
||||||
|
func (b *BitbucketAdapter) GetFileContent(ctx context.Context, projectKey, repositorySlug, path, ref string) (string, error) {
|
||||||
|
// Use the Rest helper to build the base URL, then add the "at" query param if needed.
|
||||||
|
r, err := b.CreateRequest(ctx, http.MethodGet, nil,
|
||||||
|
"/projects/", projectKey, "repos", repositorySlug, "raw", path)
|
||||||
|
if err != nil {
|
||||||
|
return "", err
|
||||||
|
}
|
||||||
|
if ref != "" {
|
||||||
|
q := r.URL.Query()
|
||||||
|
q.Set("at", ref)
|
||||||
|
r.URL.RawQuery = q.Encode()
|
||||||
|
}
|
||||||
|
|
||||||
|
resp, err := http.DefaultClient.Do(r)
|
||||||
|
if err != nil {
|
||||||
|
return "", err
|
||||||
|
}
|
||||||
|
defer resp.Body.Close()
|
||||||
|
if resp.StatusCode != http.StatusOK {
|
||||||
|
sb := &strings.Builder{}
|
||||||
|
io.Copy(sb, resp.Body)
|
||||||
|
return "", fmt.Errorf("error fetching file %s status %d, body %s", path, resp.StatusCode, sb.String())
|
||||||
|
}
|
||||||
|
content, err := io.ReadAll(resp.Body)
|
||||||
|
if err != nil {
|
||||||
|
return "", err
|
||||||
|
}
|
||||||
|
return string(content), nil
|
||||||
|
}
|
||||||
|
|
||||||
|
// GetPRHeadSHA fetches the PR and returns the SHA of the source (to) branch.
|
||||||
|
func (b *BitbucketAdapter) GetPRHeadSHA(ctx context.Context, projectKey, repositorySlug string, pullRequestID int) (string, error) {
|
||||||
|
pr, err := b.GetPR(ctx, projectKey, repositorySlug, pullRequestID)
|
||||||
|
if err != nil {
|
||||||
|
return "", err
|
||||||
|
}
|
||||||
|
return pr.ToRef.LatestCommit, nil
|
||||||
|
}
|
||||||
|
|||||||
@@ -3,6 +3,7 @@ package gitea
|
|||||||
import (
|
import (
|
||||||
"bytes"
|
"bytes"
|
||||||
"context"
|
"context"
|
||||||
|
"fmt"
|
||||||
"io"
|
"io"
|
||||||
|
|
||||||
"code.gitea.io/sdk/gitea"
|
"code.gitea.io/sdk/gitea"
|
||||||
@@ -47,3 +48,28 @@ 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
|
return err
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// 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) {
|
||||||
|
g.client.SetContext(ctx)
|
||||||
|
// The SDK's GetFile returns the raw bytes of the file.
|
||||||
|
data, _, err := g.client.GetFile(owner, repo, ref, path)
|
||||||
|
if err != nil {
|
||||||
|
return "", err
|
||||||
|
}
|
||||||
|
return string(data), nil
|
||||||
|
}
|
||||||
|
|
||||||
|
// 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) {
|
||||||
|
g.client.SetContext(ctx)
|
||||||
|
// GetPullRequest returns the detailed PR information.
|
||||||
|
pr, _, err := g.client.GetPullRequest(owner, repo, int64(prID))
|
||||||
|
if err != nil {
|
||||||
|
return "", err
|
||||||
|
}
|
||||||
|
if pr == nil || pr.Head == nil {
|
||||||
|
return "", fmt.Errorf("pull request %d has no head information", prID)
|
||||||
|
}
|
||||||
|
return pr.Head.Sha, nil
|
||||||
|
}
|
||||||
|
|||||||
@@ -88,6 +88,10 @@ If project guidelines are provided, treat them as hard rules that must be respec
|
|||||||
// It tries to split on file boundaries ("diff --git") first, then on hunk boundaries (@@),
|
// It tries to split on file boundaries ("diff --git") first, then on hunk boundaries (@@),
|
||||||
// and finally on a hard byte limit.
|
// and finally on a hard byte limit.
|
||||||
func splitDiffIntoChunks(diff []byte, maxSize int) []string {
|
func splitDiffIntoChunks(diff []byte, maxSize int) []string {
|
||||||
|
// Preserve the file header for each chunk when a single file's diff is split across multiple chunks.
|
||||||
|
// The header is the portion before the first hunk marker "@@" (including the "diff --git" line).
|
||||||
|
// When we need to split by hunks, we prepend this header to every resulting sub‑chunk.
|
||||||
|
|
||||||
if len(diff) <= maxSize {
|
if len(diff) <= maxSize {
|
||||||
return []string{string(diff)}
|
return []string{string(diff)}
|
||||||
}
|
}
|
||||||
@@ -107,22 +111,31 @@ func splitDiffIntoChunks(diff []byte, maxSize int) []string {
|
|||||||
current.Reset()
|
current.Reset()
|
||||||
}
|
}
|
||||||
if len(seg) > maxSize {
|
if len(seg) > maxSize {
|
||||||
// Split further by hunks
|
// Determine if there is a hunk marker. If not, fall back to simple size‑based chunking.
|
||||||
hunks := strings.Split(seg, "\n@@ ")
|
headerEnd := strings.Index(seg, "\n@@ ")
|
||||||
for j, h := range hunks {
|
if headerEnd == -1 {
|
||||||
var hseg string
|
// No hunk marker – split purely by size.
|
||||||
if j == 0 {
|
remaining := seg
|
||||||
// First hunk segment already contains the preceding content (including any needed newline)
|
for len(remaining) > maxSize {
|
||||||
hseg = h
|
chunks = append(chunks, remaining[:maxSize])
|
||||||
} else {
|
remaining = remaining[maxSize:]
|
||||||
// Subsequent hunks need the leading newline and "@@ " marker restored
|
|
||||||
hseg = "\n@@ " + h
|
|
||||||
}
|
}
|
||||||
|
current.WriteString(remaining)
|
||||||
|
continue
|
||||||
|
}
|
||||||
|
// Preserve the header up to the first hunk.
|
||||||
|
header := seg[:headerEnd+1] // include newline before "@@"
|
||||||
|
// Split the rest of the segment into hunks (excluding the header part).
|
||||||
|
hunks := strings.Split(strings.TrimPrefix(seg, header), "\n@@ ")
|
||||||
|
for _, h := range hunks {
|
||||||
|
// Reconstruct each hunk with its header and "@@ " prefix.
|
||||||
|
hseg := header + "@@ " + h
|
||||||
if current.Len()+len(hseg) > maxSize && current.Len() > 0 {
|
if current.Len()+len(hseg) > maxSize && current.Len() > 0 {
|
||||||
chunks = append(chunks, current.String())
|
chunks = append(chunks, current.String())
|
||||||
current.Reset()
|
current.Reset()
|
||||||
}
|
}
|
||||||
if len(hseg) > maxSize {
|
if len(hseg) > maxSize {
|
||||||
|
// If a single hunk exceeds maxSize, split it further.
|
||||||
for len(hseg) > maxSize {
|
for len(hseg) > maxSize {
|
||||||
chunks = append(chunks, hseg[:maxSize])
|
chunks = append(chunks, hseg[:maxSize])
|
||||||
hseg = hseg[maxSize:]
|
hseg = hseg[maxSize:]
|
||||||
|
|||||||
@@ -37,6 +37,15 @@ func (g *mockGit) AddComment(ctx context.Context, owner, repo string, prID int,
|
|||||||
return nil
|
return nil
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func (g *mockGit) GetFileContent(ctx context.Context, owner, repo, path, ref string) (string, error) {
|
||||||
|
// For tests, return a simple placeholder content.
|
||||||
|
return "package main\n\nfunc placeholder() {}", nil
|
||||||
|
}
|
||||||
|
|
||||||
|
func (g *mockGit) GetPRHeadSHA(ctx context.Context, owner, repo string, prID int) (string, error) {
|
||||||
|
return "dummysha", nil
|
||||||
|
}
|
||||||
|
|
||||||
func TestSplitDiffIntoChunks(t *testing.T) {
|
func TestSplitDiffIntoChunks(t *testing.T) {
|
||||||
cases := []struct {
|
cases := []struct {
|
||||||
name string
|
name string
|
||||||
|
|||||||
@@ -33,6 +33,8 @@ func New(chat ChatAdapter, git GitAdapter, maxChunkSize int, guidelines []string
|
|||||||
type GitAdapter interface {
|
type GitAdapter interface {
|
||||||
GetDiff(ctx context.Context, owner, repo string, prID int) (io.ReadCloser, error)
|
GetDiff(ctx context.Context, owner, repo string, prID int) (io.ReadCloser, error)
|
||||||
AddComment(ctx context.Context, owner, repo string, prID int, comment Comment) error
|
AddComment(ctx context.Context, owner, repo string, prID int, comment Comment) error
|
||||||
|
GetFileContent(ctx context.Context, owner, repo, path, ref string) (string, error)
|
||||||
|
GetPRHeadSHA(ctx context.Context, owner, repo string, prID int) (string, error)
|
||||||
}
|
}
|
||||||
|
|
||||||
type ChatAdapter interface {
|
type ChatAdapter interface {
|
||||||
|
|||||||
@@ -4,6 +4,8 @@ import (
|
|||||||
"context"
|
"context"
|
||||||
"fmt"
|
"fmt"
|
||||||
"log"
|
"log"
|
||||||
|
|
||||||
|
"git.schreifuchs.ch/schreifuchs/pierre-bot/internal/chatter"
|
||||||
)
|
)
|
||||||
|
|
||||||
func (s *Service) MakeReview(ctx context.Context, organisation string, repo string, prID int) error {
|
func (s *Service) MakeReview(ctx context.Context, organisation string, repo string, prID int) error {
|
||||||
@@ -20,6 +22,46 @@ func (s *Service) MakeReview(ctx context.Context, organisation string, repo stri
|
|||||||
return fmt.Errorf("error judging PR: %w", err)
|
return fmt.Errorf("error judging PR: %w", err)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// ---------- Sanity‑check step (always enabled) ----------
|
||||||
|
headSHA, err := s.git.GetPRHeadSHA(ctx, organisation, repo, prID)
|
||||||
|
if err != nil {
|
||||||
|
log.Printf("warning: could not fetch PR head SHA (%v); skipping sanity check", err)
|
||||||
|
} else {
|
||||||
|
filtered := []Comment{}
|
||||||
|
for _, c := range comments {
|
||||||
|
// Retrieve full file content at the PR head
|
||||||
|
fileContent, fErr := s.git.GetFileContent(ctx, organisation, repo, c.File, headSHA)
|
||||||
|
schreifuchs marked this conversation as resolved
|
|||||||
|
if fErr != nil {
|
||||||
|
log.Printf("failed to fetch file %s: %v – keeping original comment", c.File, fErr)
|
||||||
|
filtered = append(filtered, c)
|
||||||
|
continue
|
||||||
|
}
|
||||||
|
|
||||||
|
// Build a simple sanity‑check prompt
|
||||||
|
systemPrompt := `You are a senior software architect. Given the full source code of a file and a review comment that refers to it, decide whether the comment is useful. Return JSON with fields "useful" (bool) and "reason" (short explanation, ≤2 sentences).`
|
||||||
|
userPrompt := fmt.Sprintf("File content:\n%s\n\nComment:\n%s", fileContent, c.Message)
|
||||||
|
|
||||||
|
type sanityResult struct {
|
||||||
|
Useful bool `json:"useful"`
|
||||||
|
Reason string `json:"reason"`
|
||||||
|
}
|
||||||
|
var res sanityResult
|
||||||
|
if err := s.chat.GenerateStructured(ctx, []chatter.Message{{Role: chatter.RoleSystem, Content: systemPrompt}, {Role: chatter.RoleUser, Content: userPrompt}}, &res); err != nil {
|
||||||
|
log.Printf("sanity check error for %s:%d: %v – keeping comment", c.File, c.Line, err)
|
||||||
|
filtered = append(filtered, c)
|
||||||
|
continue
|
||||||
|
}
|
||||||
|
if res.Useful {
|
||||||
|
// Optionally annotate the comment with the reason for debugging
|
||||||
|
c.Message = fmt.Sprintf("%s (Reason: %s)", c.Message, res.Reason)
|
||||||
|
filtered = append(filtered, c)
|
||||||
|
} else {
|
||||||
|
log.Printf("comment on %s:%d discarded: %s", c.File, c.Line, res.Reason)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
comments = filtered
|
||||||
|
}
|
||||||
|
|
||||||
fmt.Printf("Analysis complete. Found %d issues.\n---\n", len(comments))
|
fmt.Printf("Analysis complete. Found %d issues.\n---\n", len(comments))
|
||||||
|
|
||||||
model := s.chat.GetProviderName()
|
model := s.chat.GetProviderName()
|
||||||
|
|||||||
Reference in New Issue
Block a user
Update the comment above the block to reflect that comments are posted only when not in dry‑run mode. (Reason: The comment correctly identifies that the existing comment is misleading—the code posts comments only when
disableComments(dry‑run) is true, so the comment should be updated to reflect posting occurs when not in dry‑run mode.) (Generated by: OpenAI (openai/gpt-oss-120b))