Compare commits

..

1 Commits

Author SHA1 Message Date
Rodin bcc88b1056 docs: add comprehensive code review report (vs go-patterns)
CI / test (pull_request) Successful in 14s
CI / review (pull_request) Failing after 11s
2026-05-01 10:33:58 -07:00
13 changed files with 117 additions and 1421 deletions
-148
View File
@@ -1,148 +0,0 @@
# This composite action is designed for Gitea Actions runners.
# Gitea Actions supports GitHub Actions syntax including $GITHUB_OUTPUT,
# actions/cache, and actions/checkout.
# Requirements: python3, sha256sum, curl (all present on ubuntu-* runners).
name: 'AI Code Review'
description: 'Run AI-powered code review on a pull request using review-bot'
inputs:
gitea-url:
description: 'Gitea instance URL (defaults to server_url)'
required: false
default: ''
repo:
description: 'Repository (owner/name, defaults to current)'
required: false
default: ''
pr-number:
description: 'Pull request number (defaults to current PR)'
required: false
default: ''
reviewer-token:
description: 'Gitea token for posting the review'
required: true
reviewer-name:
description: 'Display name for the reviewer'
required: false
default: ''
llm-base-url:
description: 'OpenAI-compatible LLM API base URL'
required: true
llm-api-key:
description: 'LLM API key'
required: true
llm-model:
description: 'LLM model name'
required: true
conventions-file:
description: 'Path to conventions file in the repo (e.g. CLAUDE.md)'
required: false
default: ''
patterns-repo:
description: 'Comma-separated repos with language patterns (e.g. rodin/elixir-patterns,rodin/phoenix-conventions)'
required: false
default: ''
patterns-files:
description: 'Comma-separated file paths or directories to fetch from patterns repos'
required: false
default: 'README.md'
temperature:
description: 'LLM temperature (0 = server default)'
required: false
default: '0'
timeout:
description: 'LLM request timeout in seconds (default 300)'
required: false
default: '300'
version:
description: 'review-bot version to install (e.g. v0.1.0, defaults to latest)'
required: false
default: 'latest'
dry-run:
description: 'Print review to stdout instead of posting'
required: false
default: 'false'
runs:
using: 'composite'
steps:
- name: Determine version
id: version
shell: bash
run: |
GITEA_URL="${{ inputs.gitea-url || github.server_url }}"
REPO="${{ inputs.repo || 'rodin/review-bot' }}"
if [ "${{ inputs.version }}" = "latest" ]; then
VERSION=$(curl -sSf "${GITEA_URL}/api/v1/repos/${REPO}/releases?limit=1" \
| python3 -c "import sys, json; releases = json.load(sys.stdin); print(releases[0]['tag_name'] if releases else '')")
if [ -z "$VERSION" ]; then
echo "Failed to determine latest version" >&2
exit 1
fi
else
VERSION="${{ inputs.version }}"
fi
echo "version=${VERSION}" >> "$GITHUB_OUTPUT"
- name: Cache review-bot binary
id: cache
uses: actions/cache@v4
with:
path: ${{ runner.temp }}/review-bot
key: review-bot-linux-amd64-${{ steps.version.outputs.version }}
- name: Install review-bot
if: steps.cache.outputs.cache-hit != 'true'
shell: bash
run: |
GITEA_URL="${{ inputs.gitea-url || github.server_url }}"
REPO="${{ inputs.repo || 'rodin/review-bot' }}"
VERSION="${{ steps.version.outputs.version }}"
BINARY="review-bot-linux-amd64"
curl -sSfL "${GITEA_URL}/${REPO}/releases/download/${VERSION}/${BINARY}" \
-o "${{ runner.temp }}/review-bot"
curl -sSfL "${GITEA_URL}/${REPO}/releases/download/${VERSION}/checksums.txt" \
-o "${{ runner.temp }}/checksums.txt"
# Verify SHA-256 checksum
cd "${{ runner.temp }}"
EXPECTED=$(grep "${BINARY}" checksums.txt | awk '{print $1}')
ACTUAL=$(sha256sum review-bot | awk '{print $1}')
if [ -z "$EXPECTED" ]; then
echo "Error: no checksum found for ${BINARY}" >&2
exit 1
fi
if [ "$EXPECTED" != "$ACTUAL" ]; then
echo "Error: checksum mismatch!" >&2
echo " Expected: $EXPECTED" >&2
echo " Actual: $ACTUAL" >&2
exit 1
fi
chmod +x "${{ runner.temp }}/review-bot"
echo "Installed review-bot ${VERSION} (checksum verified)"
- name: Run review
shell: bash
env:
GITEA_URL: ${{ inputs.gitea-url || github.server_url }}
GITEA_REPO: ${{ inputs.repo || github.repository }}
PR_NUMBER: ${{ inputs.pr-number || github.event.pull_request.number }}
REVIEWER_TOKEN: ${{ inputs.reviewer-token }}
REVIEWER_NAME: ${{ inputs.reviewer-name }}
LLM_BASE_URL: ${{ inputs.llm-base-url }}
LLM_API_KEY: ${{ inputs.llm-api-key }}
LLM_MODEL: ${{ inputs.llm-model }}
CONVENTIONS_FILE: ${{ inputs.conventions-file }}
PATTERNS_REPO: ${{ inputs.patterns-repo }}
PATTERNS_FILES: ${{ inputs.patterns-files }}
LLM_TEMPERATURE: ${{ inputs.temperature }}
LLM_TIMEOUT: ${{ inputs.timeout }}
run: |
ARGS=""
if [ "${{ inputs.dry-run }}" = "true" ]; then
ARGS="--dry-run"
fi
${{ runner.temp }}/review-bot $ARGS
+18 -19
View File
@@ -1,5 +1,4 @@
name: CI name: CI
on: on:
push: push:
branches: [main] branches: [main]
@@ -13,42 +12,42 @@ jobs:
- uses: actions/checkout@v4 - uses: actions/checkout@v4
- uses: actions/setup-go@v5 - uses: actions/setup-go@v5
with: with:
go-version: '1.26' go-version: "1.26"
- run: go test ./... - run: go test ./...
- run: go vet ./... - run: go vet ./...
- run: go build -o review-bot ./cmd/review-bot - run: go build -o review-bot ./cmd/review-bot
# Self-review: builds from source since we're pre-release
review: review:
runs-on: ubuntu-24.04 runs-on: ubuntu-24.04
if: github.event_name == 'pull_request' if: github.event_name == 'pull_request'
needs: test needs: test
strategy:
matrix:
include:
- name: sonnet
token_secret: SONNET_REVIEW_TOKEN
model: gpt-5
- name: gpt
token_secret: GPT_REVIEW_TOKEN
model: gpt-4.1
steps: steps:
- uses: actions/checkout@v4 - uses: actions/checkout@v4
- uses: actions/setup-go@v5 - uses: actions/setup-go@v5
with: with:
go-version: '1.26' go-version: "1.26"
- run: go build -o review-bot ./cmd/review-bot - run: go build -o review-bot ./cmd/review-bot
- name: Run ${{ matrix.name }} review - name: Run Sonnet Review
env: env:
GITEA_URL: ${{ github.server_url }} GITEA_URL: ${{ github.server_url }}
GITEA_REPO: ${{ github.repository }} GITEA_REPO: ${{ github.repository }}
PR_NUMBER: ${{ github.event.pull_request.number }} PR_NUMBER: ${{ github.event.pull_request.number }}
REVIEWER_TOKEN: ${{ secrets[matrix.token_secret] }} REVIEWER_TOKEN: ${{ secrets.SONNET_REVIEW_TOKEN }}
LLM_BASE_URL: ${{ secrets.LLM_BASE_URL }} LLM_BASE_URL: ${{ secrets.LLM_BASE_URL }}
LLM_API_KEY: ${{ secrets.LLM_API_KEY }} LLM_API_KEY: ${{ secrets.LLM_API_KEY }}
LLM_MODEL: ${{ matrix.model }} LLM_MODEL: "anthropic--claude-4.6-sonnet"
CONVENTIONS_FILE: "CONVENTIONS.md" CONVENTIONS_FILE: "CONVENTIONS.md"
PATTERNS_REPO: "rodin/go-patterns" REVIEWER_NAME: "Sonnet"
PATTERNS_FILES: "README.md,patterns/" run: ./review-bot
LLM_TIMEOUT: "600" - name: Run GPT Review
env:
GITEA_URL: ${{ github.server_url }}
GITEA_REPO: ${{ github.repository }}
PR_NUMBER: ${{ github.event.pull_request.number }}
REVIEWER_TOKEN: ${{ secrets.GPT_REVIEW_TOKEN }}
LLM_BASE_URL: ${{ secrets.LLM_BASE_URL }}
LLM_API_KEY: ${{ secrets.LLM_API_KEY }}
LLM_MODEL: "sap-ai-opus-latest-openai/gpt-5"
CONVENTIONS_FILE: "CONVENTIONS.md"
REVIEWER_NAME: "GPT"
run: ./review-bot run: ./review-bot
-83
View File
@@ -1,83 +0,0 @@
name: Release
on:
push:
tags:
- 'v*'
jobs:
release:
runs-on: ubuntu-24.04
steps:
- uses: actions/checkout@v4
- uses: actions/setup-go@v5
with:
go-version: '1.26'
- name: Run tests
run: |
go vet ./...
go test ./...
- name: Build binaries
run: |
VERSION=${GITHUB_REF_NAME}
mkdir -p dist
GOOS=linux GOARCH=amd64 go build -ldflags "-s -w -X main.version=${VERSION}" -o dist/review-bot-linux-amd64 ./cmd/review-bot
GOOS=linux GOARCH=arm64 go build -ldflags "-s -w -X main.version=${VERSION}" -o dist/review-bot-linux-arm64 ./cmd/review-bot
GOOS=darwin GOARCH=amd64 go build -ldflags "-s -w -X main.version=${VERSION}" -o dist/review-bot-darwin-amd64 ./cmd/review-bot
GOOS=darwin GOARCH=arm64 go build -ldflags "-s -w -X main.version=${VERSION}" -o dist/review-bot-darwin-arm64 ./cmd/review-bot
cd dist && sha256sum * > checksums.txt
- name: Create release and upload assets
env:
GITEA_TOKEN: ${{ secrets.RELEASE_TOKEN }}
run: |
VERSION=${GITHUB_REF_NAME}
GITEA_URL="${{ github.server_url }}"
REPO="${{ github.repository }}"
# Create release (or find existing one for this tag)
HTTP_CODE=$(curl -s -o /tmp/release_response.json -w "%{http_code}" -X POST \
-H "Authorization: token ${GITEA_TOKEN}" \
-H "Content-Type: application/json" \
"${GITEA_URL}/api/v1/repos/${REPO}/releases" \
-d "{\"tag_name\": \"${VERSION}\", \"name\": \"${VERSION}\", \"body\": \"Release ${VERSION}\", \"draft\": false, \"prerelease\": false}")
if [ "$HTTP_CODE" = "409" ]; then
echo "Release for ${VERSION} already exists, fetching existing..."
curl -sSf -o /tmp/release_response.json \
-H "Authorization: token ${GITEA_TOKEN}" \
"${GITEA_URL}/api/v1/repos/${REPO}/releases/tags/${VERSION}"
elif [ "$HTTP_CODE" != "201" ]; then
echo "Failed to create release (HTTP ${HTTP_CODE})" >&2
cat /tmp/release_response.json >&2
exit 1
fi
# Parse release ID (python3 available on ubuntu-24.04 runners)
RELEASE_ID=$(python3 -c "import json; print(json.load(open('/tmp/release_response.json'))['id'])")
if [ -z "$RELEASE_ID" ]; then
echo "Failed to parse release ID" >&2
cat /tmp/release_response.json >&2
exit 1
fi
echo "Release ID: ${RELEASE_ID}"
# Upload each asset
for file in dist/*; do
filename=$(basename "$file")
echo "Uploading ${filename}..."
curl -sSf -X POST \
-H "Authorization: token ${GITEA_TOKEN}" \
-H "Content-Type: application/octet-stream" \
"${GITEA_URL}/api/v1/repos/${REPO}/releases/${RELEASE_ID}/assets?name=${filename}" \
--data-binary "@${file}"
done
echo "Release ${VERSION} created with assets"
-226
View File
@@ -1,226 +0,0 @@
// Package budget manages LLM context window budgeting for review-bot.
//
// It estimates token usage and progressively trims context content to fit
// within model-specific limits. The trimming order (least important first):
// patterns → conventions → file context → diff truncation.
package budget
import (
"fmt"
"strings"
"unicode/utf8"
)
// modelLimit pairs a model name prefix with its context window size.
type modelLimit struct {
prefix string
limit int
}
// Known model context limits (in tokens), ordered longest-prefix-first
// for deterministic matching.
var modelLimits = []modelLimit{
{"claude-haiku-3.5-20241022", 200_000},
{"claude-sonnet-4-20250514", 200_000},
{"claude-opus-4-20250514", 200_000},
{"gpt-4.1-mini", 128_000},
{"gpt-5-mini", 200_000},
{"gpt-4.1", 128_000},
{"gpt-5", 200_000},
}
const defaultLimit = 128_000
// reserveTokens is headroom for the response generation.
const reserveTokens = 4_000
const diffTruncMarker = "\n\n... [diff truncated due to context limit] ..."
const diffTooLargeMarker = "... [diff too large for context window — review manually] ..."
const userMetaTruncMarker = "\n... [description truncated] ..."
// EstimateTokens estimates the number of tokens in a string.
// Uses the rough heuristic of ~4 bytes per token, which is
// conservative for English text and code.
func EstimateTokens(s string) int {
return len(s) / 4
}
// LimitForModel returns the context window size for the given model.
// Uses longest-prefix-first matching for deterministic results.
func LimitForModel(model string) int {
for _, ml := range modelLimits {
if model == ml.prefix || strings.HasPrefix(model, ml.prefix) {
return ml.limit
}
}
return defaultLimit
}
// Sections holds the prompt content sections in trim priority order.
// When the total exceeds the budget, sections are trimmed from least
// important (Patterns) to most important (Diff).
type Sections struct {
SystemBase string // Core instructions (never trimmed)
Patterns string // Language patterns (trimmed first)
Conventions string // Repo conventions (trimmed second)
FileContext string // Full file content (trimmed third)
Diff string // The actual diff (trimmed last, only truncated)
UserMeta string // PR title, description, CI status (truncated only if base exceeds budget)
}
// Result holds the trimmed content and metadata about what was dropped.
type Result struct {
SystemPrompt string
UserPrompt string
Trimmed []string // Human-readable descriptions of what was trimmed
EstTokens int // Estimated total tokens after trimming
}
// Fit trims sections to fit within the model's context limit.
// Returns the assembled prompts and a list of what was trimmed.
func Fit(model string, sections Sections) Result {
limit := LimitForModel(model) - reserveTokens
baseTokens := EstimateTokens(sections.SystemBase) + EstimateTokens(sections.UserMeta)
available := limit - baseTokens
if available < 0 {
// Base content alone exceeds budget. Truncate UserMeta (keep first ~1000 tokens).
if len(sections.UserMeta) > 4000 {
sections.UserMeta = truncateUTF8(sections.UserMeta, 4000) + userMetaTruncMarker
baseTokens = EstimateTokens(sections.SystemBase) + EstimateTokens(sections.UserMeta)
available = limit - baseTokens
}
if available < 0 {
available = 0
}
}
// Trimmable sections in priority order (first = dropped first)
type entry struct {
name string
content *string
}
entries := []entry{
{"patterns", &sections.Patterns},
{"conventions", &sections.Conventions},
{"file context", &sections.FileContext},
}
// Check if everything fits
totalTrimmable := EstimateTokens(sections.Diff)
for _, e := range entries {
totalTrimmable += EstimateTokens(*e.content)
}
var trimmed []string
if totalTrimmable > available {
// Trim from least important
for i := range entries {
tokens := EstimateTokens(*entries[i].content)
if tokens == 0 {
continue
}
trimmed = append(trimmed, fmt.Sprintf("%s (~%dK tokens)", entries[i].name, tokens/1000))
*entries[i].content = ""
// Recalculate
totalTrimmable = EstimateTokens(sections.Diff)
for _, e := range entries {
totalTrimmable += EstimateTokens(*e.content)
}
if totalTrimmable <= available {
break
}
}
}
// If still too large, truncate the diff
if totalTrimmable > available {
diffBudget := available
for _, e := range entries {
diffBudget -= EstimateTokens(*e.content)
}
if diffBudget < 0 {
diffBudget = 0
}
// Reserve space for truncation marker
markerBudget := EstimateTokens(diffTruncMarker)
effectiveBudget := diffBudget - markerBudget
if effectiveBudget < 0 {
effectiveBudget = 0
}
maxChars := effectiveBudget * 4
if maxChars < len(sections.Diff) {
removed := EstimateTokens(sections.Diff) - diffBudget
trimmed = append(trimmed, fmt.Sprintf("diff truncated (~%dK tokens removed)", removed/1000))
if maxChars > 0 {
if diffBudget >= markerBudget {
sections.Diff = truncateUTF8(sections.Diff, maxChars) + diffTruncMarker
} else {
sections.Diff = truncateUTF8(sections.Diff, maxChars)
}
} else {
sections.Diff = diffTooLargeMarker
}
}
}
finalTokens := baseTokens
for _, e := range entries {
finalTokens += EstimateTokens(*e.content)
}
finalTokens += EstimateTokens(sections.Diff)
return buildResult(sections, trimmed, finalTokens)
}
func buildResult(s Sections, trimmed []string, estTokens int) Result {
var sys strings.Builder
sys.WriteString(s.SystemBase)
if s.Patterns != "" {
sys.WriteString("\n\n## Language Patterns & Idioms\n\nUse the following patterns as review criteria. Code that violates these established patterns is a finding:\n\n")
sys.WriteString(s.Patterns)
}
if s.Conventions != "" {
sys.WriteString("\n\n## Repository Conventions\n\nThe repository has the following coding conventions that must be respected:\n\n")
sys.WriteString(s.Conventions)
}
var usr strings.Builder
usr.WriteString(s.UserMeta)
if s.FileContext != "" {
usr.WriteString("\n### Full File Context (modified files)\n\n")
usr.WriteString(s.FileContext)
usr.WriteString("\n")
}
if s.Diff != "" {
usr.WriteString("\n### Diff (changes to review)\n\n```diff\n")
usr.WriteString(s.Diff)
usr.WriteString("\n```\n")
}
if len(trimmed) > 0 {
usr.WriteString("\n⚠️ Note: Context was trimmed to fit model limits. Dropped: ")
usr.WriteString(strings.Join(trimmed, ", "))
usr.WriteString("\n")
}
return Result{
SystemPrompt: sys.String(),
UserPrompt: usr.String(),
Trimmed: trimmed,
EstTokens: estTokens,
}
}
// truncateUTF8 truncates s to at most maxBytes without splitting multi-byte
// UTF-8 characters. Returns a valid UTF-8 string of at most maxBytes bytes.
func truncateUTF8(s string, maxBytes int) string {
if len(s) <= maxBytes {
return s
}
for maxBytes > 0 && !utf8.RuneStart(s[maxBytes]) {
maxBytes--
}
return s[:maxBytes]
}
-203
View File
@@ -1,203 +0,0 @@
package budget
import (
"strings"
"testing"
)
func TestEstimateTokens(t *testing.T) {
tests := []struct {
input string
want int
}{
{"", 0},
{"abcd", 1},
{"12345678", 2},
{strings.Repeat("x", 400), 100},
}
for _, tt := range tests {
got := EstimateTokens(tt.input)
if got != tt.want {
t.Errorf("EstimateTokens(%d chars) = %d, want %d", len(tt.input), got, tt.want)
}
}
}
func TestLimitForModel(t *testing.T) {
tests := []struct {
model string
want int
}{
{"gpt-4.1", 128_000},
{"gpt-5", 200_000},
{"gpt-5-mini", 200_000},
{"unknown-model", defaultLimit},
{"gpt-4.1-2026-01-01", 128_000}, // prefix match
}
for _, tt := range tests {
got := LimitForModel(tt.model)
if got != tt.want {
t.Errorf("LimitForModel(%q) = %d, want %d", tt.model, got, tt.want)
}
}
}
func TestFit_AllFits(t *testing.T) {
s := Sections{
SystemBase: "system instructions",
Patterns: "some patterns",
Conventions: "some conventions",
FileContext: "file content",
Diff: "diff content",
UserMeta: "PR: title\n",
}
result := Fit("gpt-5", s)
if len(result.Trimmed) != 0 {
t.Errorf("expected no trimming, got %v", result.Trimmed)
}
if !strings.Contains(result.SystemPrompt, "some patterns") {
t.Error("expected patterns in system prompt")
}
if !strings.Contains(result.SystemPrompt, "some conventions") {
t.Error("expected conventions in system prompt")
}
if !strings.Contains(result.UserPrompt, "file content") {
t.Error("expected file context in user prompt")
}
}
func TestFit_TrimsPatterns(t *testing.T) {
// Create content that exceeds 128K token budget for gpt-4.1
// Budget ≈ 128K - 4K reserve = 124K tokens = ~496K chars
// Fill patterns with enough to push over
bigPatterns := strings.Repeat("x", 500_000) // ~125K tokens
s := Sections{
SystemBase: "base",
Patterns: bigPatterns,
Conventions: "conventions",
FileContext: "files",
Diff: "diff",
UserMeta: "meta",
}
result := Fit("gpt-4.1", s)
if len(result.Trimmed) == 0 {
t.Fatal("expected trimming")
}
if !strings.Contains(result.Trimmed[0], "patterns") {
t.Errorf("expected patterns to be trimmed first, got %v", result.Trimmed)
}
if strings.Contains(result.SystemPrompt, bigPatterns[:100]) {
t.Error("expected patterns to be removed from output")
}
// Conventions should survive
if !strings.Contains(result.SystemPrompt, "conventions") {
t.Error("expected conventions to survive after patterns trimmed")
}
}
func TestFit_TrimsConventions(t *testing.T) {
// Patterns + conventions + diff all exceed budget even after patterns removed
big := strings.Repeat("y", 520_000) // ~130K tokens each (exceeds 124K budget even alone)
s := Sections{
SystemBase: "base",
Patterns: big,
Conventions: big,
FileContext: "files",
Diff: "diff",
UserMeta: "meta",
}
result := Fit("gpt-4.1", s)
if len(result.Trimmed) < 2 {
t.Fatalf("expected at least 2 trimmed, got %v", result.Trimmed)
}
if !strings.Contains(result.Trimmed[0], "patterns") {
t.Errorf("expected patterns trimmed first, got %s", result.Trimmed[0])
}
if !strings.Contains(result.Trimmed[1], "conventions") {
t.Errorf("expected conventions trimmed second, got %s", result.Trimmed[1])
}
}
func TestFit_TruncatesDiff(t *testing.T) {
// Only diff is huge, no patterns/conventions
hugeDiff := strings.Repeat("z", 600_000) // ~150K tokens > 128K limit
s := Sections{
SystemBase: "base",
Diff: hugeDiff,
UserMeta: "meta",
}
result := Fit("gpt-4.1", s)
if len(result.Trimmed) == 0 {
t.Fatal("expected diff truncation")
}
if !strings.Contains(result.Trimmed[len(result.Trimmed)-1], "diff truncated") {
t.Errorf("expected diff truncation note, got %v", result.Trimmed)
}
if !strings.Contains(result.UserPrompt, "[diff truncated due to context limit]") {
t.Error("expected truncation marker in user prompt")
}
}
func TestFit_PreservesNoteInOutput(t *testing.T) {
big := strings.Repeat("w", 500_000)
s := Sections{
SystemBase: "base",
Patterns: big,
Diff: "small diff",
UserMeta: "meta",
}
result := Fit("gpt-4.1", s)
if !strings.Contains(result.UserPrompt, "⚠️ Note: Context was trimmed") {
t.Error("expected trimming note in user prompt")
}
}
func TestFit_HugeUserMeta(t *testing.T) {
// UserMeta so large that base alone exceeds limit
// Use a unique marker past the truncation point
hugeDesc := strings.Repeat("d", 5000) + "UNIQUE_MARKER_PAST_TRUNCATION" + strings.Repeat("d", 595_000)
s := Sections{
SystemBase: "base",
Diff: "small diff",
UserMeta: hugeDesc,
}
result := Fit("gpt-4.1", s)
limit := LimitForModel("gpt-4.1") - reserveTokens
if result.EstTokens > limit {
t.Errorf("EstTokens %d exceeds limit %d", result.EstTokens, limit)
}
// Content past truncation point should not be present
if strings.Contains(result.UserPrompt, "UNIQUE_MARKER_PAST_TRUNCATION") {
t.Error("expected UserMeta to be truncated but found content past truncation point")
}
// Truncation marker should be present
if !strings.Contains(result.UserPrompt, "[description truncated]") {
t.Error("expected truncation marker in output")
}
}
func TestFit_NeverExceedsLimit(t *testing.T) {
// All sections huge — verify final tokens never exceed limit
big := strings.Repeat("a", 200_000)
s := Sections{
SystemBase: strings.Repeat("s", 8000),
Patterns: big,
Conventions: big,
FileContext: big,
Diff: big,
UserMeta: strings.Repeat("m", 8000),
}
result := Fit("gpt-4.1", s)
limit := LimitForModel("gpt-4.1") - reserveTokens
if result.EstTokens > limit {
t.Errorf("EstTokens %d exceeds limit %d (trimmed: %v)", result.EstTokens, limit, result.Trimmed)
}
}
+16 -176
View File
@@ -1,25 +1,19 @@
package main package main
import ( import (
"context"
"flag" "flag"
"fmt" "fmt"
"log" "log"
"os" "os"
"strconv" "strconv"
"strings" "strings"
"time"
"gitea.weiker.me/rodin/review-bot/budget"
"gitea.weiker.me/rodin/review-bot/gitea" "gitea.weiker.me/rodin/review-bot/gitea"
"gitea.weiker.me/rodin/review-bot/llm" "gitea.weiker.me/rodin/review-bot/llm"
"gitea.weiker.me/rodin/review-bot/review" "gitea.weiker.me/rodin/review-bot/review"
) )
var version = "dev"
func main() { func main() {
versionFlag := flag.Bool("version", false, "Print version and exit")
// CLI flags // CLI flags
giteaURL := flag.String("gitea-url", envOrDefault("GITEA_URL", ""), "Gitea instance URL") giteaURL := flag.String("gitea-url", envOrDefault("GITEA_URL", ""), "Gitea instance URL")
repo := flag.String("repo", envOrDefault("GITEA_REPO", ""), "Repository (owner/name)") repo := flag.String("repo", envOrDefault("GITEA_REPO", ""), "Repository (owner/name)")
@@ -30,21 +24,10 @@ func main() {
llmAPIKey := flag.String("llm-api-key", envOrDefault("LLM_API_KEY", ""), "LLM API key") llmAPIKey := flag.String("llm-api-key", envOrDefault("LLM_API_KEY", ""), "LLM API key")
llmModel := flag.String("llm-model", envOrDefault("LLM_MODEL", ""), "LLM model name") llmModel := flag.String("llm-model", envOrDefault("LLM_MODEL", ""), "LLM model name")
conventionsFile := flag.String("conventions-file", envOrDefault("CONVENTIONS_FILE", ""), "Conventions file path in repo (e.g. CLAUDE.md)") conventionsFile := flag.String("conventions-file", envOrDefault("CONVENTIONS_FILE", ""), "Conventions file path in repo (e.g. CLAUDE.md)")
patternsRepo := flag.String("patterns-repo", envOrDefault("PATTERNS_REPO", ""), "Repo with language patterns (e.g. rodin/elixir-patterns)")
patternsFiles := flag.String("patterns-files", envOrDefault("PATTERNS_FILES", "README.md"), "Comma-separated file paths to fetch from patterns repo")
dryRun := flag.Bool("dry-run", false, "Print review to stdout instead of posting") dryRun := flag.Bool("dry-run", false, "Print review to stdout instead of posting")
llmTemp := flag.Float64("llm-temperature", envOrDefaultFloat("LLM_TEMPERATURE", 0), "LLM temperature (0 = server default)")
llmTimeout := flag.Int("llm-timeout", envOrDefaultInt("LLM_TIMEOUT", 300), "LLM request timeout in seconds (default 300)")
flag.Parse() flag.Parse()
if *versionFlag {
fmt.Printf("review-bot %s\n", version)
os.Exit(0)
}
log.Printf("review-bot %s", version)
// Validate required fields // Validate required fields
if *giteaURL == "" || *repo == "" || *prNum == "" || *reviewerToken == "" || if *giteaURL == "" || *repo == "" || *prNum == "" || *reviewerToken == "" ||
*llmBaseURL == "" || *llmAPIKey == "" || *llmModel == "" { *llmBaseURL == "" || *llmAPIKey == "" || *llmModel == "" {
@@ -69,52 +52,28 @@ func main() {
// Initialize clients // Initialize clients
giteaClient := gitea.NewClient(*giteaURL, *reviewerToken) giteaClient := gitea.NewClient(*giteaURL, *reviewerToken)
llmClient := llm.NewClient(*llmBaseURL, *llmAPIKey, *llmModel) llmClient := llm.NewClient(*llmBaseURL, *llmAPIKey, *llmModel)
if *llmTemp < 0 || *llmTemp > 2 {
log.Fatal("--llm-temperature must be between 0 and 2")
}
if *llmTemp > 0 {
llmClient.WithTemperature(*llmTemp)
}
if *llmTimeout > 0 {
llmClient.WithTimeout(time.Duration(*llmTimeout) * time.Second)
}
// Create a top-level context. Timeout derived from LLM timeout + 1 min for other ops.
overallTimeout := time.Duration(*llmTimeout)*time.Second + time.Minute
ctx, cancel := context.WithTimeout(context.Background(), overallTimeout)
defer cancel()
log.Printf("Reviewing PR #%d on %s/%s", prNumber, owner, repoName) log.Printf("Reviewing PR #%d on %s/%s", prNumber, owner, repoName)
// Step 1: Fetch PR metadata // Step 1: Fetch PR metadata
pr, err := giteaClient.GetPullRequest(ctx, owner, repoName, prNumber) pr, err := giteaClient.GetPullRequest(owner, repoName, prNumber)
if err != nil { if err != nil {
log.Fatalf("Failed to fetch PR: %v", err) log.Fatalf("Failed to fetch PR: %v", err)
} }
log.Printf("PR: %s", pr.Title) log.Printf("PR: %s", pr.Title)
// Step 2: Fetch diff // Step 2: Fetch diff
diff, err := giteaClient.GetPullRequestDiff(ctx, owner, repoName, prNumber) diff, err := giteaClient.GetPullRequestDiff(owner, repoName, prNumber)
if err != nil { if err != nil {
log.Fatalf("Failed to fetch diff: %v", err) log.Fatalf("Failed to fetch diff: %v", err)
} }
log.Printf("Diff size: %d bytes", len(diff)) log.Printf("Diff size: %d bytes", len(diff))
// Step 3: Fetch full file content for modified files // Step 3: Check CI status
fileContext := ""
files, err := giteaClient.GetPullRequestFiles(ctx, owner, repoName, prNumber)
if err != nil {
log.Printf("Warning: could not fetch PR files list: %v", err)
} else {
fileContext = fetchFileContext(ctx, giteaClient, owner, repoName, pr.Head.Ref, files)
log.Printf("Fetched full context for %d files", len(files))
}
// Step 4: Check CI status
ciPassed := true ciPassed := true
ciDetails := "" ciDetails := ""
if pr.Head.Sha != "" { if pr.Head.Sha != "" {
statuses, err := giteaClient.GetCommitStatuses(ctx, owner, repoName, pr.Head.Sha) statuses, err := giteaClient.GetCommitStatuses(owner, repoName, pr.Head.Sha)
if err != nil { if err != nil {
log.Printf("Warning: could not fetch CI status: %v", err) log.Printf("Warning: could not fetch CI status: %v", err)
} else { } else {
@@ -123,10 +82,10 @@ func main() {
} }
} }
// Step 5: Load conventions file if specified // Step 4: Load conventions file if specified
conventions := "" conventions := ""
if *conventionsFile != "" { if *conventionsFile != "" {
content, err := giteaClient.GetFileContent(ctx, owner, repoName, *conventionsFile) content, err := giteaClient.GetFileContent(owner, repoName, *conventionsFile)
if err != nil { if err != nil {
log.Printf("Warning: could not load conventions file %q: %v", *conventionsFile, err) log.Printf("Warning: could not load conventions file %q: %v", *conventionsFile, err)
} else { } else {
@@ -135,49 +94,31 @@ func main() {
} }
} }
// Step 6: Load patterns from external repo if specified // Step 5: Build prompts
patterns := "" systemPrompt := review.BuildSystemPrompt(conventions)
if *patternsRepo != "" { userPrompt := review.BuildUserPrompt(pr.Title, pr.Body, diff, ciPassed, ciDetails)
patterns = fetchPatterns(ctx, giteaClient, *patternsRepo, *patternsFiles)
log.Printf("Loaded patterns from %s (%d bytes)", *patternsRepo, len(patterns))
}
// Step 7: Budget-aware prompt assembly // Step 6: Call LLM
sections := budget.Sections{
SystemBase: review.BuildSystemBase(),
Patterns: patterns,
Conventions: conventions,
FileContext: fileContext,
Diff: diff,
UserMeta: review.BuildUserMeta(pr.Title, pr.Body, ciPassed, ciDetails),
}
budgetResult := budget.Fit(*llmModel, sections)
log.Printf("Token estimate: ~%dK (limit: %dK)", budgetResult.EstTokens/1000, budget.LimitForModel(*llmModel)/1000)
if len(budgetResult.Trimmed) > 0 {
log.Printf("Context trimmed: %v", budgetResult.Trimmed)
}
// Step 8: Call LLM
log.Printf("Sending to LLM (%s)...", *llmModel) log.Printf("Sending to LLM (%s)...", *llmModel)
messages := []llm.Message{ messages := []llm.Message{
{Role: "system", Content: budgetResult.SystemPrompt}, {Role: "system", Content: systemPrompt},
{Role: "user", Content: budgetResult.UserPrompt}, {Role: "user", Content: userPrompt},
} }
response, err := llmClient.Complete(ctx, messages) response, err := llmClient.Complete(messages)
if err != nil { if err != nil {
log.Fatalf("LLM request failed: %v", err) log.Fatalf("LLM request failed: %v", err)
} }
log.Printf("LLM response received (%d bytes)", len(response)) log.Printf("LLM response received (%d bytes)", len(response))
// Step 9: Parse response // Step 7: Parse response
result, err := review.ParseResponse(response) result, err := review.ParseResponse(response)
if err != nil { if err != nil {
log.Fatalf("Failed to parse LLM response: %v", err) log.Fatalf("Failed to parse LLM response: %v", err)
} }
log.Printf("Verdict: %s (%d findings)", result.Verdict, len(result.Findings)) log.Printf("Verdict: %s (%d findings)", result.Verdict, len(result.Findings))
// Step 10: Format and post review // Step 8: Format and post review
reviewBody := review.FormatMarkdown(result, *reviewerName) reviewBody := review.FormatMarkdown(result, *reviewerName)
event := review.GiteaEvent(result.Verdict) event := review.GiteaEvent(result.Verdict)
@@ -189,93 +130,12 @@ func main() {
} }
log.Printf("Posting review (event=%s)...", event) log.Printf("Posting review (event=%s)...", event)
if err := giteaClient.PostReview(ctx, owner, repoName, prNumber, event, reviewBody); err != nil { if err := giteaClient.PostReview(owner, repoName, prNumber, event, reviewBody); err != nil {
log.Fatalf("Failed to post review: %v", err) log.Fatalf("Failed to post review: %v", err)
} }
log.Printf("Review posted successfully!") log.Printf("Review posted successfully!")
} }
// fetchFileContext fetches the full content of modified files from the PR branch.
func fetchFileContext(ctx context.Context, client *gitea.Client, owner, repo, ref string, files []gitea.ChangedFile) string {
var sb strings.Builder
for _, f := range files {
if ctx.Err() != nil {
break
}
if f.Status == "removed" {
continue // Skip deleted files
}
content, err := client.GetFileContentRef(ctx, owner, repo, f.Filename, ref)
if err != nil {
log.Printf("Warning: could not fetch %s: %v", f.Filename, err)
continue
}
sb.WriteString(fmt.Sprintf("--- %s ---\n", f.Filename))
sb.WriteString("```\n")
sb.WriteString(content)
sb.WriteString("\n```\n\n")
}
return sb.String()
}
// fetchPatterns fetches pattern files from one or more external repos.
// patternsRepo is comma-separated list of owner/name repos.
// patternsFiles is comma-separated list of file paths or directories.
// If a path ends with / or is a directory, all files within it are fetched recursively.
func fetchPatterns(ctx context.Context, client *gitea.Client, patternsRepo, patternsFiles string) string {
var sb strings.Builder
repos := strings.Split(patternsRepo, ",")
paths := strings.Split(patternsFiles, ",")
for _, repoRef := range repos {
if ctx.Err() != nil {
break
}
repoRef = strings.TrimSpace(repoRef)
if repoRef == "" {
continue
}
parts := strings.SplitN(repoRef, "/", 2)
if len(parts) != 2 {
log.Printf("Warning: invalid patterns-repo format %q, expected owner/name", repoRef)
continue
}
owner, repo := parts[0], parts[1]
for _, path := range paths {
path = strings.TrimSpace(path)
if path == "" {
continue
}
files, err := client.GetAllFilesInPath(ctx, owner, repo, path)
if err != nil {
log.Printf("Warning: could not fetch %s from %s: %v", path, repoRef, err)
continue
}
for filePath, content := range files {
// Only include markdown and text files as patterns
if !isPatternFile(filePath) {
continue
}
sb.WriteString(fmt.Sprintf("### %s/%s\n\n%s\n\n", repoRef, filePath, content))
}
}
}
return sb.String()
}
// isPatternFile returns true if the file should be included as pattern content.
func isPatternFile(path string) bool {
lower := strings.ToLower(path)
return strings.HasSuffix(lower, ".md") ||
strings.HasSuffix(lower, ".txt") ||
strings.HasSuffix(lower, ".yml") ||
strings.HasSuffix(lower, ".yaml")
}
// evaluateCIStatus checks if all CI statuses indicate success. // evaluateCIStatus checks if all CI statuses indicate success.
func evaluateCIStatus(statuses []gitea.CommitStatus) (passed bool, details string) { func evaluateCIStatus(statuses []gitea.CommitStatus) (passed bool, details string) {
if len(statuses) == 0 { if len(statuses) == 0 {
@@ -306,23 +166,3 @@ func envOrDefault(key, defaultVal string) string {
} }
return defaultVal return defaultVal
} }
func envOrDefaultFloat(key string, defaultVal float64) float64 {
if v := os.Getenv(key); v != "" {
f, err := strconv.ParseFloat(v, 64)
if err == nil {
return f
}
}
return defaultVal
}
func envOrDefaultInt(key string, defaultVal int) int {
if v := os.Getenv(key); v != "" {
i, err := strconv.Atoi(v)
if err == nil {
return i
}
}
return defaultVal
}
+30 -150
View File
@@ -1,45 +1,35 @@
// Package gitea provides a client for the Gitea API.
// It supports pull request operations, file content retrieval,
// and review submission.
package gitea package gitea
import ( import (
"bytes"
"context"
"encoding/json" "encoding/json"
"fmt" "fmt"
"io" "io"
"log"
"net/http" "net/http"
"net/url"
"strings" "strings"
"time"
) )
// Client interacts with the Gitea API. // Client interacts with the Gitea API.
// A Client is safe for concurrent use by multiple goroutines.
type Client struct { type Client struct {
baseURL string BaseURL string
token string Token string
http *http.Client HTTP *http.Client
} }
// NewClient creates a new Gitea API client. // NewClient creates a new Gitea API client.
func NewClient(baseURL, token string) *Client { func NewClient(baseURL, token string) *Client {
return &Client{ return &Client{
baseURL: strings.TrimRight(baseURL, "/"), BaseURL: strings.TrimRight(baseURL, "/"),
token: token, Token: token,
http: &http.Client{Timeout: 30 * time.Second}, HTTP: &http.Client{},
} }
} }
// PullRequest holds relevant PR metadata. // PullRequest holds relevant PR metadata.
type PullRequest struct { type PullRequest struct {
Title string `json:"title"` Title string `json:"title"`
Body string `json:"body"` Body string `json:"body"`
Head struct { Head struct {
Sha string `json:"sha"` Sha string `json:"sha"`
Ref string `json:"ref"`
} `json:"head"` } `json:"head"`
} }
@@ -51,16 +41,10 @@ type CommitStatus struct {
TargetURL string `json:"target_url"` TargetURL string `json:"target_url"`
} }
// ChangedFile represents a file modified in a PR.
type ChangedFile struct {
Filename string `json:"filename"`
Status string `json:"status"`
}
// GetPullRequest fetches PR metadata. // GetPullRequest fetches PR metadata.
func (c *Client) GetPullRequest(ctx context.Context, owner, repo string, number int) (*PullRequest, error) { func (c *Client) GetPullRequest(owner, repo string, number int) (*PullRequest, error) {
reqURL := fmt.Sprintf("%s/api/v1/repos/%s/%s/pulls/%d", c.baseURL, owner, repo, number) url := fmt.Sprintf("%s/api/v1/repos/%s/%s/pulls/%d", c.BaseURL, owner, repo, number)
body, err := c.doGet(ctx, reqURL) body, err := c.doGet(url)
if err != nil { if err != nil {
return nil, fmt.Errorf("fetch PR: %w", err) return nil, fmt.Errorf("fetch PR: %w", err)
} }
@@ -72,33 +56,19 @@ func (c *Client) GetPullRequest(ctx context.Context, owner, repo string, number
} }
// GetPullRequestDiff fetches the unified diff for a PR. // GetPullRequestDiff fetches the unified diff for a PR.
func (c *Client) GetPullRequestDiff(ctx context.Context, owner, repo string, number int) (string, error) { func (c *Client) GetPullRequestDiff(owner, repo string, number int) (string, error) {
reqURL := fmt.Sprintf("%s/api/v1/repos/%s/%s/pulls/%d.diff", c.baseURL, owner, repo, number) url := fmt.Sprintf("%s/api/v1/repos/%s/%s/pulls/%d.diff", c.BaseURL, owner, repo, number)
body, err := c.doGet(ctx, reqURL) body, err := c.doGet(url)
if err != nil { if err != nil {
return "", fmt.Errorf("fetch diff: %w", err) return "", fmt.Errorf("fetch diff: %w", err)
} }
return string(body), nil return string(body), nil
} }
// GetPullRequestFiles fetches the list of files changed in a PR.
func (c *Client) GetPullRequestFiles(ctx context.Context, owner, repo string, number int) ([]ChangedFile, error) {
reqURL := fmt.Sprintf("%s/api/v1/repos/%s/%s/pulls/%d/files", c.baseURL, owner, repo, number)
body, err := c.doGet(ctx, reqURL)
if err != nil {
return nil, fmt.Errorf("fetch PR files: %w", err)
}
var files []ChangedFile
if err := json.Unmarshal(body, &files); err != nil {
return nil, fmt.Errorf("parse PR files JSON: %w", err)
}
return files, nil
}
// GetCommitStatuses fetches CI statuses for a commit SHA. // GetCommitStatuses fetches CI statuses for a commit SHA.
func (c *Client) GetCommitStatuses(ctx context.Context, owner, repo, sha string) ([]CommitStatus, error) { func (c *Client) GetCommitStatuses(owner, repo, sha string) ([]CommitStatus, error) {
reqURL := fmt.Sprintf("%s/api/v1/repos/%s/%s/commits/%s/statuses", c.baseURL, owner, repo, sha) url := fmt.Sprintf("%s/api/v1/repos/%s/%s/commits/%s/statuses", c.BaseURL, owner, repo, sha)
body, err := c.doGet(ctx, reqURL) body, err := c.doGet(url)
if err != nil { if err != nil {
return nil, fmt.Errorf("fetch commit statuses: %w", err) return nil, fmt.Errorf("fetch commit statuses: %w", err)
} }
@@ -110,29 +80,19 @@ func (c *Client) GetCommitStatuses(ctx context.Context, owner, repo, sha string)
} }
// GetFileContent fetches a file from the default branch of a repo. // GetFileContent fetches a file from the default branch of a repo.
func (c *Client) GetFileContent(ctx context.Context, owner, repo, filepath string) (string, error) { func (c *Client) GetFileContent(owner, repo, filepath string) (string, error) {
reqURL := fmt.Sprintf("%s/api/v1/repos/%s/%s/raw/%s", c.baseURL, owner, repo, escapePath(filepath)) url := fmt.Sprintf("%s/api/v1/repos/%s/%s/raw/%s", c.BaseURL, owner, repo, filepath)
body, err := c.doGet(ctx, reqURL) body, err := c.doGet(url)
if err != nil { if err != nil {
return "", fmt.Errorf("fetch file %s: %w", filepath, err) return "", fmt.Errorf("fetch file %s: %w", filepath, err)
} }
return string(body), nil return string(body), nil
} }
// GetFileContentRef fetches a file from a specific ref (branch/tag/sha) in a repo.
func (c *Client) GetFileContentRef(ctx context.Context, owner, repo, filepath, ref string) (string, error) {
reqURL := fmt.Sprintf("%s/api/v1/repos/%s/%s/raw/%s?ref=%s", c.baseURL, owner, repo, escapePath(filepath), url.QueryEscape(ref))
body, err := c.doGet(ctx, reqURL)
if err != nil {
return "", fmt.Errorf("fetch file %s@%s: %w", filepath, ref, err)
}
return string(body), nil
}
// PostReview submits a review to a PR. // PostReview submits a review to a PR.
// event should be "APPROVED" or "REQUEST_CHANGES". // event should be "APPROVED" or "REQUEST_CHANGES".
func (c *Client) PostReview(ctx context.Context, owner, repo string, number int, event, body string) error { func (c *Client) PostReview(owner, repo string, number int, event, body string) error {
reqURL := fmt.Sprintf("%s/api/v1/repos/%s/%s/pulls/%d/reviews", c.baseURL, owner, repo, number) url := fmt.Sprintf("%s/api/v1/repos/%s/%s/pulls/%d/reviews", c.BaseURL, owner, repo, number)
payload := struct { payload := struct {
Body string `json:"body"` Body string `json:"body"`
@@ -147,14 +107,14 @@ func (c *Client) PostReview(ctx context.Context, owner, repo string, number int,
return fmt.Errorf("marshal review payload: %w", err) return fmt.Errorf("marshal review payload: %w", err)
} }
req, err := http.NewRequestWithContext(ctx, http.MethodPost, reqURL, bytes.NewReader(data)) req, err := http.NewRequest("POST", url, strings.NewReader(string(data)))
if err != nil { if err != nil {
return fmt.Errorf("create review request: %w", err) return fmt.Errorf("create review request: %w", err)
} }
req.Header.Set("Authorization", "token "+c.token) req.Header.Set("Authorization", "token "+c.Token)
req.Header.Set("Content-Type", "application/json") req.Header.Set("Content-Type", "application/json")
resp, err := c.http.Do(req) resp, err := c.HTTP.Do(req)
if err != nil { if err != nil {
return fmt.Errorf("post review: %w", err) return fmt.Errorf("post review: %w", err)
} }
@@ -167,14 +127,14 @@ func (c *Client) PostReview(ctx context.Context, owner, repo string, number int,
return nil return nil
} }
func (c *Client) doGet(ctx context.Context, reqURL string) ([]byte, error) { func (c *Client) doGet(url string) ([]byte, error) {
req, err := http.NewRequestWithContext(ctx, http.MethodGet, reqURL, nil) req, err := http.NewRequest("GET", url, nil)
if err != nil { if err != nil {
return nil, err return nil, err
} }
req.Header.Set("Authorization", "token "+c.token) req.Header.Set("Authorization", "token "+c.Token)
resp, err := c.http.Do(req) resp, err := c.HTTP.Do(req)
if err != nil { if err != nil {
return nil, err return nil, err
} }
@@ -186,83 +146,3 @@ func (c *Client) doGet(ctx context.Context, reqURL string) ([]byte, error) {
} }
return io.ReadAll(resp.Body) return io.ReadAll(resp.Body)
} }
// escapePath escapes each segment of a relative file path for use in URLs.
// Slashes are preserved as path separators; other special characters are escaped.
// Input should be a relative path (no leading slash). Already-encoded segments
// will be double-encoded, which is the desired behavior for user-provided paths.
func escapePath(p string) string {
parts := strings.Split(p, "/")
for i, part := range parts {
parts[i] = url.PathEscape(part)
}
return strings.Join(parts, "/")
}
// ContentEntry represents a file or directory entry from the contents API.
type ContentEntry struct {
Name string `json:"name"`
Path string `json:"path"`
Type string `json:"type"` // "file" or "dir"
}
// ListContents lists files and directories at a given path in a repo.
// Pass an empty path to list the repository root.
func (c *Client) ListContents(ctx context.Context, owner, repo, path string) ([]ContentEntry, error) {
var reqURL string
if path == "" {
reqURL = fmt.Sprintf("%s/api/v1/repos/%s/%s/contents", c.baseURL, owner, repo)
} else {
reqURL = fmt.Sprintf("%s/api/v1/repos/%s/%s/contents/%s", c.baseURL, owner, repo, escapePath(path))
}
body, err := c.doGet(ctx, reqURL)
if err != nil {
return nil, fmt.Errorf("list contents %s: %w", path, err)
}
var entries []ContentEntry
if err := json.Unmarshal(body, &entries); err != nil {
return nil, fmt.Errorf("parse contents JSON: %w", err)
}
return entries, nil
}
// GetAllFilesInPath recursively fetches all file contents under a path.
// If the path is a file, returns just that file's content.
// If the path is a directory, recursively fetches all files within it.
func (c *Client) GetAllFilesInPath(ctx context.Context, owner, repo, path string) (map[string]string, error) {
results := make(map[string]string)
// Try listing as directory first
entries, err := c.ListContents(ctx, owner, repo, path)
if err != nil {
// Might be a file, try fetching directly
content, fileErr := c.GetFileContent(ctx, owner, repo, path)
if fileErr != nil {
return nil, fmt.Errorf("path %q is neither a file nor directory: %w", path, err)
}
results[path] = content
return results, nil
}
for _, entry := range entries {
switch entry.Type {
case "file":
content, err := c.GetFileContent(ctx, owner, repo, entry.Path)
if err != nil {
log.Printf("Warning: could not fetch file %s: %v", entry.Path, err)
continue
}
results[entry.Path] = content
case "dir":
subResults, err := c.GetAllFilesInPath(ctx, owner, repo, entry.Path)
if err != nil {
log.Printf("Warning: could not recurse into %s: %v", entry.Path, err)
continue
}
for k, v := range subResults {
results[k] = v
}
}
}
return results, nil
}
+8 -133
View File
@@ -1,9 +1,7 @@
package gitea package gitea
import ( import (
"context"
"encoding/json" "encoding/json"
"fmt"
"net/http" "net/http"
"net/http/httptest" "net/http/httptest"
"testing" "testing"
@@ -29,7 +27,7 @@ func TestGetPullRequest(t *testing.T) {
defer server.Close() defer server.Close()
client := NewClient(server.URL, "test-token") client := NewClient(server.URL, "test-token")
got, err := client.GetPullRequest(context.Background(), "owner", "repo", 1) got, err := client.GetPullRequest("owner", "repo", 1)
if err != nil { if err != nil {
t.Fatalf("unexpected error: %v", err) t.Fatalf("unexpected error: %v", err)
} }
@@ -56,7 +54,7 @@ func TestGetPullRequestDiff(t *testing.T) {
defer server.Close() defer server.Close()
client := NewClient(server.URL, "test-token") client := NewClient(server.URL, "test-token")
got, err := client.GetPullRequestDiff(context.Background(), "owner", "repo", 5) got, err := client.GetPullRequestDiff("owner", "repo", 5)
if err != nil { if err != nil {
t.Fatalf("unexpected error: %v", err) t.Fatalf("unexpected error: %v", err)
} }
@@ -81,7 +79,7 @@ func TestGetCommitStatuses(t *testing.T) {
defer server.Close() defer server.Close()
client := NewClient(server.URL, "test-token") client := NewClient(server.URL, "test-token")
got, err := client.GetCommitStatuses(context.Background(), "owner", "repo", "abc123") got, err := client.GetCommitStatuses("owner", "repo", "abc123")
if err != nil { if err != nil {
t.Fatalf("unexpected error: %v", err) t.Fatalf("unexpected error: %v", err)
} }
@@ -128,7 +126,7 @@ func TestPostReview(t *testing.T) {
defer server.Close() defer server.Close()
client := NewClient(server.URL, "test-token") client := NewClient(server.URL, "test-token")
err := client.PostReview(context.Background(), "owner", "repo", 3, "APPROVED", "LGTM") err := client.PostReview("owner", "repo", 3, "APPROVED", "LGTM")
if err != nil { if err != nil {
t.Fatalf("unexpected error: %v", err) t.Fatalf("unexpected error: %v", err)
} }
@@ -142,7 +140,7 @@ func TestGetPullRequest_Non200(t *testing.T) {
defer server.Close() defer server.Close()
client := NewClient(server.URL, "test-token") client := NewClient(server.URL, "test-token")
_, err := client.GetPullRequest(context.Background(), "owner", "repo", 999) _, err := client.GetPullRequest("owner", "repo", 999)
if err == nil { if err == nil {
t.Fatal("expected error for 404, got nil") t.Fatal("expected error for 404, got nil")
} }
@@ -155,7 +153,7 @@ func TestGetPullRequest_BadJSON(t *testing.T) {
defer server.Close() defer server.Close()
client := NewClient(server.URL, "test-token") client := NewClient(server.URL, "test-token")
_, err := client.GetPullRequest(context.Background(), "owner", "repo", 1) _, err := client.GetPullRequest("owner", "repo", 1)
if err == nil { if err == nil {
t.Fatal("expected error for bad JSON, got nil") t.Fatal("expected error for bad JSON, got nil")
} }
@@ -169,7 +167,7 @@ func TestPostReview_Non200(t *testing.T) {
defer server.Close() defer server.Close()
client := NewClient(server.URL, "test-token") client := NewClient(server.URL, "test-token")
err := client.PostReview(context.Background(), "owner", "repo", 1, "APPROVED", "test") err := client.PostReview("owner", "repo", 1, "APPROVED", "test")
if err == nil { if err == nil {
t.Fatal("expected error for 403, got nil") t.Fatal("expected error for 403, got nil")
} }
@@ -187,7 +185,7 @@ func TestGetFileContent(t *testing.T) {
defer server.Close() defer server.Close()
client := NewClient(server.URL, "test-token") client := NewClient(server.URL, "test-token")
got, err := client.GetFileContent(context.Background(), "owner", "repo", "CONVENTIONS.md") got, err := client.GetFileContent("owner", "repo", "CONVENTIONS.md")
if err != nil { if err != nil {
t.Fatalf("unexpected error: %v", err) t.Fatalf("unexpected error: %v", err)
} }
@@ -195,126 +193,3 @@ func TestGetFileContent(t *testing.T) {
t.Errorf("expected %q, got %q", expected, got) t.Errorf("expected %q, got %q", expected, got)
} }
} }
func TestGetPullRequestFiles(t *testing.T) {
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
if r.URL.Path != "/api/v1/repos/owner/repo/pulls/1/files" {
t.Errorf("unexpected path: %s", r.URL.Path)
}
w.Header().Set("Content-Type", "application/json")
w.Write([]byte(`[{"filename":"main.go","status":"modified"},{"filename":"old.go","status":"removed"}]`))
}))
defer server.Close()
client := NewClient(server.URL, "test-token")
files, err := client.GetPullRequestFiles(context.Background(), "owner", "repo", 1)
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
if len(files) != 2 {
t.Fatalf("expected 2 files, got %d", len(files))
}
if files[0].Filename != "main.go" || files[0].Status != "modified" {
t.Errorf("unexpected first file: %+v", files[0])
}
}
func TestGetFileContentRef(t *testing.T) {
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
if r.URL.Path != "/api/v1/repos/owner/repo/raw/main.go" {
t.Errorf("unexpected path: %s", r.URL.Path)
}
if r.URL.Query().Get("ref") != "feature-branch" {
t.Errorf("unexpected ref: %s", r.URL.Query().Get("ref"))
}
w.Write([]byte("package main\n"))
}))
defer server.Close()
client := NewClient(server.URL, "test-token")
content, err := client.GetFileContentRef(context.Background(), "owner", "repo", "main.go", "feature-branch")
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
if content != "package main\n" {
t.Errorf("unexpected content: %q", content)
}
}
func TestListContents(t *testing.T) {
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
if r.URL.Path != "/api/v1/repos/owner/repo/contents/docs" {
t.Errorf("unexpected path: %s", r.URL.Path)
}
w.Header().Set("Content-Type", "application/json")
fmt.Fprintf(w, `[{"name":"guide.md","path":"docs/guide.md","type":"file"},{"name":"sub","path":"docs/sub","type":"dir"}]`)
}))
defer server.Close()
client := NewClient(server.URL, "test-token")
entries, err := client.ListContents(context.Background(), "owner", "repo", "docs")
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
if len(entries) != 2 {
t.Fatalf("expected 2 entries, got %d", len(entries))
}
if entries[0].Type != "file" || entries[0].Path != "docs/guide.md" {
t.Errorf("unexpected first entry: %+v", entries[0])
}
if entries[1].Type != "dir" {
t.Errorf("expected dir type, got %s", entries[1].Type)
}
}
func TestGetAllFilesInPath_File(t *testing.T) {
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
if r.URL.Path == "/api/v1/repos/owner/repo/contents/README.md" {
// Gitea returns 404 for contents API on files (it's not a dir)
http.NotFound(w, r)
return
}
if r.URL.Path == "/api/v1/repos/owner/repo/raw/README.md" {
fmt.Fprintf(w, "# Hello")
return
}
http.NotFound(w, r)
}))
defer server.Close()
client := NewClient(server.URL, "test-token")
files, err := client.GetAllFilesInPath(context.Background(), "owner", "repo", "README.md")
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
if len(files) != 1 {
t.Fatalf("expected 1 file, got %d", len(files))
}
if files["README.md"] != "# Hello" {
t.Errorf("unexpected content: %q", files["README.md"])
}
}
func TestEscapePath(t *testing.T) {
tests := []struct {
name string
input string
want string
}{
{"simple", "src/main.go", "src/main.go"},
{"spaces", "my dir/my file.go", "my%20dir/my%20file.go"},
{"special chars", "path/file#1.txt", "path/file%231.txt"},
{"empty", "", ""},
{"single segment", "README.md", "README.md"},
{"nested deep", "a/b/c/d.md", "a/b/c/d.md"},
{"already encoded", "path/file%20name.go", "path/file%2520name.go"},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got := escapePath(tt.input)
if got != tt.want {
t.Errorf("escapePath(%q) = %q, want %q", tt.input, got, tt.want)
}
})
}
}
+12 -13
View File
@@ -3,10 +3,8 @@
package main package main
import ( import (
"context"
"os" "os"
"strconv" "strconv"
"strings"
"testing" "testing"
"gitea.weiker.me/rodin/review-bot/gitea" "gitea.weiker.me/rodin/review-bot/gitea"
@@ -44,27 +42,28 @@ func TestIntegration_FullReviewFlow(t *testing.T) {
} }
// Parse owner/repo // Parse owner/repo
parts := strings.SplitN(giteaRepo, "/", 2) owner, repoName := "", ""
if len(parts) != 2 { for i, c := range giteaRepo {
t.Fatalf("Invalid repo format %q", giteaRepo) if c == / {
owner = giteaRepo[:i]
repoName = giteaRepo[i+1:]
break
}
} }
owner, repoName := parts[0], parts[1]
if owner == "" || repoName == "" { if owner == "" || repoName == "" {
t.Fatalf("Invalid repo format %q", giteaRepo) t.Fatalf("Invalid repo format %q", giteaRepo)
} }
ctx := context.Background()
// Step 1: Fetch PR // Step 1: Fetch PR
giteaClient := gitea.NewClient(giteaURL, giteaToken) giteaClient := gitea.NewClient(giteaURL, giteaToken)
pr, err := giteaClient.GetPullRequest(ctx, owner, repoName, prNumber) pr, err := giteaClient.GetPullRequest(owner, repoName, prNumber)
if err != nil { if err != nil {
t.Fatalf("GetPullRequest: %v", err) t.Fatalf("GetPullRequest: %v", err)
} }
t.Logf("PR: %s (sha: %s)", pr.Title, pr.Head.Sha) t.Logf("PR: %s (sha: %s)", pr.Title, pr.Head.Sha)
// Step 2: Fetch diff // Step 2: Fetch diff
diff, err := giteaClient.GetPullRequestDiff(ctx, owner, repoName, prNumber) diff, err := giteaClient.GetPullRequestDiff(owner, repoName, prNumber)
if err != nil { if err != nil {
t.Fatalf("GetPullRequestDiff: %v", err) t.Fatalf("GetPullRequestDiff: %v", err)
} }
@@ -74,12 +73,12 @@ func TestIntegration_FullReviewFlow(t *testing.T) {
t.Logf("Diff size: %d bytes", len(diff)) t.Logf("Diff size: %d bytes", len(diff))
// Step 3: Build prompts // Step 3: Build prompts
systemPrompt := review.BuildSystemPrompt("", "") systemPrompt := review.BuildSystemPrompt("")
userPrompt := review.BuildUserPrompt(pr.Title, pr.Body, diff, "", true, "") userPrompt := review.BuildUserPrompt(pr.Title, pr.Body, diff, true, "")
// Step 4: Call LLM // Step 4: Call LLM
llmClient := llm.NewClient(llmBaseURL, llmAPIKey, llmModel) llmClient := llm.NewClient(llmBaseURL, llmAPIKey, llmModel)
response, err := llmClient.Complete(ctx, []llm.Message{ response, err := llmClient.Complete([]llm.Message{
{Role: "system", Content: systemPrompt}, {Role: "system", Content: systemPrompt},
{Role: "user", Content: userPrompt}, {Role: "user", Content: userPrompt},
}) })
+16 -34
View File
@@ -1,50 +1,32 @@
// Package llm provides a client for OpenAI-compatible chat completion APIs.
package llm package llm
import ( import (
"bytes" "bytes"
"context"
"encoding/json" "encoding/json"
"fmt" "fmt"
"io" "io"
"net/http" "net/http"
"strings" "strings"
"time"
) )
// Client calls an OpenAI-compatible chat completion API. // Client calls an OpenAI-compatible chat completion API.
// A Client is safe for concurrent use by multiple goroutines after construction.
// WithTimeout and WithTemperature must be called during setup, before concurrent use.
type Client struct { type Client struct {
baseURL string BaseURL string
apiKey string APIKey string
model string Model string
temperature float64 HTTP *http.Client
http *http.Client
} }
// NewClient creates a new LLM client. // NewClient creates a new LLM client.
func NewClient(baseURL, apiKey, model string) *Client { func NewClient(baseURL, apiKey, model string) *Client {
return &Client{ return &Client{
baseURL: strings.TrimRight(baseURL, "/"), BaseURL: strings.TrimRight(baseURL, "/"),
apiKey: apiKey, APIKey: apiKey,
model: model, Model: model,
http: &http.Client{Timeout: 5 * time.Minute}, HTTP: &http.Client{},
} }
} }
// WithTimeout sets the HTTP request timeout for LLM calls (default 5 minutes).
func (c *Client) WithTimeout(d time.Duration) *Client {
c.http.Timeout = d
return c
}
// WithTemperature sets the temperature for LLM requests (0 = omit, uses server default).
func (c *Client) WithTemperature(t float64) *Client {
c.temperature = t
return c
}
// Message represents a chat message. // Message represents a chat message.
type Message struct { type Message struct {
Role string `json:"role"` Role string `json:"role"`
@@ -55,7 +37,7 @@ type Message struct {
type ChatRequest struct { type ChatRequest struct {
Model string `json:"model"` Model string `json:"model"`
Messages []Message `json:"messages"` Messages []Message `json:"messages"`
Temperature float64 `json:"temperature,omitempty"` Temperature float64 `json:"temperature"`
} }
// ChatResponse is the response from the API. // ChatResponse is the response from the API.
@@ -68,11 +50,11 @@ type ChatResponse struct {
} }
// Complete sends a chat completion request and returns the assistant's response content. // Complete sends a chat completion request and returns the assistant's response content.
func (c *Client) Complete(ctx context.Context, messages []Message) (string, error) { func (c *Client) Complete(messages []Message) (string, error) {
reqBody := ChatRequest{ reqBody := ChatRequest{
Model: c.model, Model: c.Model,
Temperature: c.temperature,
Messages: messages, Messages: messages,
Temperature: 0.1,
} }
data, err := json.Marshal(reqBody) data, err := json.Marshal(reqBody)
@@ -80,15 +62,15 @@ func (c *Client) Complete(ctx context.Context, messages []Message) (string, erro
return "", fmt.Errorf("marshal request: %w", err) return "", fmt.Errorf("marshal request: %w", err)
} }
url := c.baseURL + "/chat/completions" url := c.BaseURL + "/chat/completions"
req, err := http.NewRequestWithContext(ctx, "POST", url, bytes.NewReader(data)) req, err := http.NewRequest("POST", url, bytes.NewReader(data))
if err != nil { if err != nil {
return "", fmt.Errorf("create request: %w", err) return "", fmt.Errorf("create request: %w", err)
} }
req.Header.Set("Authorization", "Bearer "+c.apiKey) req.Header.Set("Authorization", "Bearer "+c.APIKey)
req.Header.Set("Content-Type", "application/json") req.Header.Set("Content-Type", "application/json")
resp, err := c.http.Do(req) resp, err := c.HTTP.Do(req)
if err != nil { if err != nil {
return "", fmt.Errorf("LLM request: %w", err) return "", fmt.Errorf("LLM request: %w", err)
} }
+5 -105
View File
@@ -1,12 +1,10 @@
package llm package llm
import ( import (
"context"
"encoding/json" "encoding/json"
"net/http" "net/http"
"net/http/httptest" "net/http/httptest"
"testing" "testing"
"time"
) )
func TestComplete_Success(t *testing.T) { func TestComplete_Success(t *testing.T) {
@@ -53,7 +51,7 @@ func TestComplete_Success(t *testing.T) {
defer server.Close() defer server.Close()
client := NewClient(server.URL, "test-key", "gpt-4") client := NewClient(server.URL, "test-key", "gpt-4")
got, err := client.Complete(context.Background(), []Message{{Role: "user", Content: "Hi"}}) got, err := client.Complete([]Message{{Role: "user", Content: "Hi"}})
if err != nil { if err != nil {
t.Fatalf("unexpected error: %v", err) t.Fatalf("unexpected error: %v", err)
} }
@@ -70,7 +68,7 @@ func TestComplete_APIError(t *testing.T) {
defer server.Close() defer server.Close()
client := NewClient(server.URL, "test-key", "gpt-4") client := NewClient(server.URL, "test-key", "gpt-4")
_, err := client.Complete(context.Background(), []Message{{Role: "user", Content: "Hi"}}) _, err := client.Complete([]Message{{Role: "user", Content: "Hi"}})
if err == nil { if err == nil {
t.Fatal("expected error for 429, got nil") t.Fatal("expected error for 429, got nil")
} }
@@ -84,7 +82,7 @@ func TestComplete_NoChoices(t *testing.T) {
defer server.Close() defer server.Close()
client := NewClient(server.URL, "test-key", "gpt-4") client := NewClient(server.URL, "test-key", "gpt-4")
_, err := client.Complete(context.Background(), []Message{{Role: "user", Content: "Hi"}}) _, err := client.Complete([]Message{{Role: "user", Content: "Hi"}})
if err == nil { if err == nil {
t.Fatal("expected error for no choices, got nil") t.Fatal("expected error for no choices, got nil")
} }
@@ -97,7 +95,7 @@ func TestComplete_BadJSON(t *testing.T) {
defer server.Close() defer server.Close()
client := NewClient(server.URL, "test-key", "gpt-4") client := NewClient(server.URL, "test-key", "gpt-4")
_, err := client.Complete(context.Background(), []Message{{Role: "user", Content: "Hi"}}) _, err := client.Complete([]Message{{Role: "user", Content: "Hi"}})
if err == nil { if err == nil {
t.Fatal("expected error for bad JSON, got nil") t.Fatal("expected error for bad JSON, got nil")
} }
@@ -105,106 +103,8 @@ func TestComplete_BadJSON(t *testing.T) {
func TestComplete_ServerDown(t *testing.T) { func TestComplete_ServerDown(t *testing.T) {
client := NewClient("http://127.0.0.1:1", "test-key", "gpt-4") client := NewClient("http://127.0.0.1:1", "test-key", "gpt-4")
_, err := client.Complete(context.Background(), []Message{{Role: "user", Content: "Hi"}}) _, err := client.Complete([]Message{{Role: "user", Content: "Hi"}})
if err == nil { if err == nil {
t.Fatal("expected error for connection refused, got nil") t.Fatal("expected error for connection refused, got nil")
} }
} }
func TestWithTemperature(t *testing.T) {
client := NewClient("http://example.com", "key", "model")
if client.temperature != 0 {
t.Errorf("expected initial temperature 0, got %f", client.temperature)
}
result := client.WithTemperature(0.7)
if result != client {
t.Error("WithTemperature should return the same client for chaining")
}
if client.temperature != 0.7 {
t.Errorf("expected temperature 0.7, got %f", client.temperature)
}
}
func TestComplete_TemperatureOmittedWhenZero(t *testing.T) {
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
var req map[string]interface{}
json.NewDecoder(r.Body).Decode(&req)
if _, exists := req["temperature"]; exists {
t.Error("temperature should be omitted when zero (server default)")
}
w.Header().Set("Content-Type", "application/json")
json.NewEncoder(w).Encode(ChatResponse{
Choices: []struct {
Message struct {
Content string `json:"content"`
} `json:"message"`
}{{Message: struct {
Content string `json:"content"`
}{Content: "ok"}}},
})
}))
defer server.Close()
client := NewClient(server.URL, "key", "model")
_, err := client.Complete(context.Background(), []Message{{Role: "user", Content: "Hi"}})
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
}
func TestComplete_TemperatureIncludedWhenSet(t *testing.T) {
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
var req map[string]interface{}
json.NewDecoder(r.Body).Decode(&req)
temp, exists := req["temperature"]
if !exists {
t.Error("temperature should be included when set")
}
if temp != 0.7 {
t.Errorf("expected temperature 0.7, got %v", temp)
}
w.Header().Set("Content-Type", "application/json")
json.NewEncoder(w).Encode(ChatResponse{
Choices: []struct {
Message struct {
Content string `json:"content"`
} `json:"message"`
}{{Message: struct {
Content string `json:"content"`
}{Content: "ok"}}},
})
}))
defer server.Close()
client := NewClient(server.URL, "key", "model").WithTemperature(0.7)
_, err := client.Complete(context.Background(), []Message{{Role: "user", Content: "Hi"}})
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
}
func TestWithTimeout(t *testing.T) {
client := NewClient("http://example.com", "key", "model")
result := client.WithTimeout(10 * time.Second)
if result != client {
t.Error("WithTimeout should return the same client for chaining")
}
// Verify timeout causes failure on slow server
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
time.Sleep(200 * time.Millisecond)
w.Header().Set("Content-Type", "application/json")
w.Write([]byte(`{"choices":[{"message":{"content":"ok"}}]}`))
}))
defer server.Close()
shortClient := NewClient(server.URL, "key", "model").WithTimeout(50 * time.Millisecond)
_, err := shortClient.Complete(context.Background(), []Message{{Role: "user", Content: "hi"}})
if err == nil {
t.Error("expected timeout error with 50ms timeout and 200ms server delay")
}
}
+6 -44
View File
@@ -1,5 +1,3 @@
// Package review builds prompts for AI code review and parses LLM responses
// into structured review results.
package review package review
import ( import (
@@ -7,17 +5,11 @@ import (
"strings" "strings"
) )
// BuildSystemBase returns the core system prompt instructions without // BuildSystemPrompt constructs the system prompt for the LLM reviewer.
// patterns or conventions. Used by the budget package to separate func BuildSystemPrompt(conventions string) string {
// trimmable from non-trimmable content.
func BuildSystemBase() string {
var sb strings.Builder var sb strings.Builder
sb.WriteString("You are an expert code reviewer. Review the provided pull request diff carefully.\n\n") sb.WriteString("You are an expert code reviewer. Review the provided pull request diff carefully.\n\n")
sb.WriteString("CONTEXT:\n")
sb.WriteString("- You will receive the full content of modified files for reference, followed by the diff showing what changed.\n")
sb.WriteString("- The diff shows ONLY what was added/removed. The full file content provides complete context.\n")
sb.WriteString("- Focus your review on the CHANGES (the diff), using the full files for context.\n\n")
sb.WriteString("Your task:\n") sb.WriteString("Your task:\n")
sb.WriteString("1. Review the diff for correctness, idiomatic code, potential bugs, and design issues.\n") sb.WriteString("1. Review the diff for correctness, idiomatic code, potential bugs, and design issues.\n")
sb.WriteString("2. Consider the CI status — if CI has failed, that is an automatic REQUEST_CHANGES regardless of code quality.\n") sb.WriteString("2. Consider the CI status — if CI has failed, that is an automatic REQUEST_CHANGES regardless of code quality.\n")
@@ -44,29 +36,15 @@ func BuildSystemBase() string {
sb.WriteString("- Line numbers should reference the new file line numbers from the diff headers.\n") sb.WriteString("- Line numbers should reference the new file line numbers from the diff headers.\n")
sb.WriteString("- If the diff is empty or trivial (only formatting/whitespace), APPROVE with no findings.\n") sb.WriteString("- If the diff is empty or trivial (only formatting/whitespace), APPROVE with no findings.\n")
return sb.String()
}
// BuildSystemPrompt constructs the full system prompt with patterns and conventions.
// Deprecated: Use BuildSystemBase with budget.Fit for context-aware assembly.
func BuildSystemPrompt(conventions, patterns string) string {
var sb strings.Builder
sb.WriteString(BuildSystemBase())
if patterns != "" {
sb.WriteString(fmt.Sprintf("\n\n## Language Patterns & Idioms\n\nUse the following patterns as review criteria. Code that violates these established patterns is a finding:\n\n%s\n", patterns))
}
if conventions != "" { if conventions != "" {
sb.WriteString(fmt.Sprintf("\n\n## Repository Conventions\n\nThe repository has the following coding conventions that must be respected:\n\n%s\n", conventions)) sb.WriteString(fmt.Sprintf("\n\nThe repository has the following coding conventions that should be respected:\n\n%s\n", conventions))
} }
return sb.String() return sb.String()
} }
// BuildUserMeta returns the PR metadata header (title, description, CI status) // BuildUserPrompt constructs the user message with PR context.
// without the diff or file context. Used by the budget package. func BuildUserPrompt(title, description, diff string, ciPassed bool, ciDetails string) string {
func BuildUserMeta(title, description string, ciPassed bool, ciDetails string) string {
var sb strings.Builder var sb strings.Builder
sb.WriteString(fmt.Sprintf("## Pull Request: %s\n\n", title)) sb.WriteString(fmt.Sprintf("## Pull Request: %s\n\n", title))
@@ -85,23 +63,7 @@ func BuildUserMeta(title, description string, ciPassed bool, ciDetails string) s
sb.WriteString(fmt.Sprintf("CI Details: %s\n", ciDetails)) sb.WriteString(fmt.Sprintf("CI Details: %s\n", ciDetails))
} }
return sb.String() sb.WriteString("\n### Diff\n\n")
}
// BuildUserPrompt constructs the user message with PR context.
// Deprecated: Use BuildUserMeta with budget.Fit for context-aware assembly.
func BuildUserPrompt(title, description, diff, fileContext string, ciPassed bool, ciDetails string) string {
var sb strings.Builder
sb.WriteString(BuildUserMeta(title, description, ciPassed, ciDetails))
if fileContext != "" {
sb.WriteString("\n### Full File Context (modified files)\n\n")
sb.WriteString(fileContext)
sb.WriteString("\n")
}
sb.WriteString("\n### Diff (changes to review)\n\n")
sb.WriteString("```diff\n") sb.WriteString("```diff\n")
sb.WriteString(diff) sb.WriteString(diff)
sb.WriteString("\n```\n") sb.WriteString("\n```\n")
+6 -87
View File
@@ -6,7 +6,7 @@ import (
) )
func TestBuildSystemPrompt_NoConventions(t *testing.T) { func TestBuildSystemPrompt_NoConventions(t *testing.T) {
prompt := BuildSystemPrompt("", "") prompt := BuildSystemPrompt("")
if !strings.Contains(prompt, "expert code reviewer") { if !strings.Contains(prompt, "expert code reviewer") {
t.Error("expected system prompt to mention code reviewer role") t.Error("expected system prompt to mention code reviewer role")
@@ -18,7 +18,7 @@ func TestBuildSystemPrompt_NoConventions(t *testing.T) {
func TestBuildSystemPrompt_WithConventions(t *testing.T) { func TestBuildSystemPrompt_WithConventions(t *testing.T) {
conventions := "- Use stdlib only\n- No panics\n" conventions := "- Use stdlib only\n- No panics\n"
prompt := BuildSystemPrompt(conventions, "") prompt := BuildSystemPrompt(conventions)
if !strings.Contains(prompt, "coding conventions") { if !strings.Contains(prompt, "coding conventions") {
t.Error("expected conventions section") t.Error("expected conventions section")
@@ -29,7 +29,7 @@ func TestBuildSystemPrompt_WithConventions(t *testing.T) {
} }
func TestBuildUserPrompt_Basic(t *testing.T) { func TestBuildUserPrompt_Basic(t *testing.T) {
prompt := BuildUserPrompt("Fix bug", "Fixes the crash", "diff content here", "", true, "all checks passed") prompt := BuildUserPrompt("Fix bug", "Fixes the crash", "diff content here", true, "all checks passed")
if !strings.Contains(prompt, "Fix bug") { if !strings.Contains(prompt, "Fix bug") {
t.Error("expected PR title") t.Error("expected PR title")
@@ -46,7 +46,7 @@ func TestBuildUserPrompt_Basic(t *testing.T) {
} }
func TestBuildUserPrompt_CIFailed(t *testing.T) { func TestBuildUserPrompt_CIFailed(t *testing.T) {
prompt := BuildUserPrompt("Add tests", "", "some diff", "", false, "lint: failed") prompt := BuildUserPrompt("Add tests", "", "some diff", false, "lint: failed")
if !strings.Contains(prompt, "FAILED") { if !strings.Contains(prompt, "FAILED") {
t.Error("expected CI status FAILED") t.Error("expected CI status FAILED")
@@ -57,7 +57,7 @@ func TestBuildUserPrompt_CIFailed(t *testing.T) {
} }
func TestBuildUserPrompt_NoDescription(t *testing.T) { func TestBuildUserPrompt_NoDescription(t *testing.T) {
prompt := BuildUserPrompt("Quick fix", "", "diff", "", true, "") prompt := BuildUserPrompt("Quick fix", "", "diff", true, "")
if strings.Contains(prompt, "### Description") { if strings.Contains(prompt, "### Description") {
t.Error("should not contain Description header when body is empty") t.Error("should not contain Description header when body is empty")
@@ -66,7 +66,7 @@ func TestBuildUserPrompt_NoDescription(t *testing.T) {
func TestBuildUserPrompt_DiffIncluded(t *testing.T) { func TestBuildUserPrompt_DiffIncluded(t *testing.T) {
diff := "+func Hello() string {\n+\treturn \"hello\"\n+}" diff := "+func Hello() string {\n+\treturn \"hello\"\n+}"
prompt := BuildUserPrompt("Greeting", "Add greeting func", diff, "", true, "") prompt := BuildUserPrompt("Greeting", "Add greeting func", diff, true, "")
if !strings.Contains(prompt, "```diff") { if !strings.Contains(prompt, "```diff") {
t.Error("expected diff fence") t.Error("expected diff fence")
@@ -75,84 +75,3 @@ func TestBuildUserPrompt_DiffIncluded(t *testing.T) {
t.Error("expected diff content in prompt") t.Error("expected diff content in prompt")
} }
} }
func TestBuildSystemPrompt_WithPatterns(t *testing.T) {
patterns := "## Naming: use snake_case for functions"
prompt := BuildSystemPrompt("", patterns)
if !strings.Contains(prompt, "Language Patterns") {
t.Error("expected patterns section header")
}
if !strings.Contains(prompt, "snake_case") {
t.Error("expected patterns content")
}
}
func TestBuildSystemPrompt_WithBoth(t *testing.T) {
conventions := "Run mix format before commit"
patterns := "Use pipe operator for transformations"
prompt := BuildSystemPrompt(conventions, patterns)
if !strings.Contains(prompt, "Repository Conventions") {
t.Error("expected conventions section")
}
if !strings.Contains(prompt, "Language Patterns") {
t.Error("expected patterns section")
}
}
func TestBuildUserPrompt_WithFileContext(t *testing.T) {
fileContext := "--- main.go ---\npackage main\n"
prompt := BuildUserPrompt("Fix", "desc", "diff here", fileContext, true, "")
if !strings.Contains(prompt, "Full File Context") {
t.Error("expected file context section")
}
if !strings.Contains(prompt, "package main") {
t.Error("expected file content in prompt")
}
}
func TestBuildUserPrompt_WithoutFileContext(t *testing.T) {
prompt := BuildUserPrompt("Fix", "desc", "diff here", "", true, "")
if strings.Contains(prompt, "Full File Context") {
t.Error("should not include file context section when empty")
}
}
func TestBuildSystemBase(t *testing.T) {
result := BuildSystemBase()
if result == "" {
t.Fatal("BuildSystemBase returned empty string")
}
if !strings.Contains(result, "expert code reviewer") {
t.Error("expected reviewer role in system base")
}
if !strings.Contains(result, "REQUEST_CHANGES") {
t.Error("expected verdict format in system base")
}
if !strings.Contains(result, "JSON") {
t.Error("expected JSON output instruction in system base")
}
}
func TestBuildUserMeta(t *testing.T) {
result := BuildUserMeta("Fix bug", "Some description", true, "all checks passed")
if !strings.Contains(result, "Fix bug") {
t.Error("expected title in user meta")
}
if !strings.Contains(result, "Some description") {
t.Error("expected description in user meta")
}
if !strings.Contains(result, "PASSED") {
t.Error("expected CI PASSED status")
}
}
func TestBuildUserMeta_CIFailed(t *testing.T) {
result := BuildUserMeta("Title", "", false, "test job failed")
if !strings.Contains(result, "FAILED") {
t.Error("expected CI FAILED status")
}
if strings.Contains(result, "Description") {
t.Error("expected no description section when empty")
}
}