Compare commits

..

21 Commits

Author SHA1 Message Date
Rodin 565a077b01 fix: CI config - correct patterns path, increase timeout
CI / test (pull_request) Successful in 13s
CI / review (gpt-4.1, gpt, GPT_REVIEW_TOKEN) (pull_request) Successful in 21s
CI / review (gpt-5, sonnet, SONNET_REVIEW_TOKEN) (pull_request) Successful in 2m18s
- PATTERNS_FILES: docs/ does not exist in go-patterns, use patterns/
- LLM_TIMEOUT: 600s (gpt-5-mini needs more time for larger diffs)
2026-05-01 19:06:18 -07:00
Rodin dab7871cb4 fix: address review findings on budget system
CI / test (pull_request) Successful in 14s
CI / review (gpt-5, sonnet, SONNET_REVIEW_TOKEN) (pull_request) Successful in 1m41s
CI / review (gpt-5-mini, gpt, GPT_REVIEW_TOKEN) (pull_request) Successful in 3m2s
- Account for truncation marker tokens when computing diff budget
  (prevents EstTokens exceeding model limit in edge cases)
- Rune-safe truncation for both UserMeta and Diff (no split multi-byte)
- Fix misleading comment (1000 chars → ~1000 tokens/4000 chars)
- Extract marker strings as constants
- Add unit tests for BuildSystemBase and BuildUserMeta
2026-05-01 18:59:07 -07:00
Rodin d9cacf6f62 fix: strict budget enforcement + deterministic model matching
CI / test (pull_request) Successful in 14s
CI / review (gpt-5, sonnet, SONNET_REVIEW_TOKEN) (pull_request) Successful in 1m59s
CI / review (gpt-5-mini, gpt, GPT_REVIEW_TOKEN) (pull_request) Failing after 5m12s
Addresses review findings:
- Replace map-based model limits with ordered slice (longest-prefix-first)
  for deterministic matching
- Truncate UserMeta when base content alone exceeds budget (keeps first
  4000 chars + truncation marker)
- Remove hard minimum of 1000 tokens for diff budget — use 0 as floor
  to guarantee total never exceeds limit
- Handle zero-budget edge case (diff replaced with manual-review message)
- Add tests: huge UserMeta, all-sections-huge never exceeds limit
2026-05-01 18:51:22 -07:00
Rodin 67d835909f feat: add context budget system for LLM overflow (#19)
CI / test (pull_request) Successful in 13s
CI / review (gpt-5, sonnet, SONNET_REVIEW_TOKEN) (pull_request) Successful in 1m30s
CI / review (gpt-5-mini, gpt, GPT_REVIEW_TOKEN) (pull_request) Successful in 2m29s
Adds a budget package that estimates token usage and progressively
trims context to fit within model-specific limits.

Trim order (least important first):
1. Language patterns
2. Repository conventions
3. Full file context
4. Diff (truncated as last resort)

When content is trimmed, a note is appended to the user prompt so
the LLM knows context was reduced.

- New budget package with Fit(), EstimateTokens(), LimitForModel()
- Model limit table (GPT-4.1: 128K, GPT-5: 200K, Claude: 200K)
- Refactored review/prompt.go: BuildSystemBase() and BuildUserMeta()
  extract non-trimmable content; old functions delegate to new ones
- main.go uses budget.Fit() instead of direct prompt assembly
- 7 unit tests covering all trim paths

Closes #19
2026-05-01 18:46:53 -07:00
rodin ef3e6d5e87 Merge pull request 'fix: path-escape file paths and eliminate url package shadowing' (#17) from fix/url-escaping-and-shadow into main
CI / test (push) Successful in 15s
CI / review (gpt-5, sonnet, SONNET_REVIEW_TOKEN) (push) Has been skipped
CI / review (gpt-5-mini, gpt, GPT_REVIEW_TOKEN) (push) Has been skipped
2026-05-01 21:55:02 +00:00
Rodin aade891129 docs: add package-level documentation
CI / test (pull_request) Successful in 14s
CI / review (gpt-5, sonnet, SONNET_REVIEW_TOKEN) (pull_request) Failing after 55s
CI / review (gpt-5-mini, gpt, GPT_REVIEW_TOKEN) (pull_request) Failing after 1m6s
Per go-patterns/package-design.md, every package needs a doc comment.
Added to gitea, llm, and review packages.
2026-05-01 14:54:58 -07:00
Rodin 7b42de67ca fix: handle empty path in ListContents (root listing)
CI / test (pull_request) Successful in 14s
CI / review (gpt-5, sonnet, SONNET_REVIEW_TOKEN) (pull_request) Successful in 1m4s
CI / review (gpt-5-mini, gpt, GPT_REVIEW_TOKEN) (pull_request) Successful in 1m8s
Empty path now yields /contents instead of /contents/ (trailing slash).
Added doc comment noting empty path = repo root.
2026-05-01 14:46:40 -07:00
Rodin dd2661fe14 fix: address all review findings from PR #17
CI / test (pull_request) Successful in 13s
CI / review (gpt-5, sonnet, SONNET_REVIEW_TOKEN) (pull_request) Successful in 52s
CI / review (gpt-5-mini, gpt, GPT_REVIEW_TOKEN) (pull_request) Successful in 1m12s
- Rename all remaining url locals to reqURL (consistency)
- Use http.MethodGet/http.MethodPost constants
- Document escapePath: relative paths only, double-encoding expected
- Add TestEscapePath with 7 edge cases (empty, spaces, #, deep, encoded)
2026-05-01 14:40:19 -07:00
Rodin 98a4772f30 fix: path-escape file paths and eliminate url package shadowing
CI / test (pull_request) Successful in 14s
CI / review (gpt-5, sonnet, SONNET_REVIEW_TOKEN) (pull_request) Successful in 55s
CI / review (gpt-5-mini, gpt, GPT_REVIEW_TOKEN) (pull_request) Successful in 1m48s
- Add escapePath() helper: escapes each path segment individually
  (preserves slashes as separators, escapes spaces/#/? etc)
- Apply to GetFileContent, GetFileContentRef, ListContents
- Rename doGet parameter from url to reqURL (avoids shadowing net/url)
- Rename local variables in GetFileContent/ListContents for consistency

Addresses remaining findings from PR #16 review.
2026-05-01 14:33:18 -07:00
rodin fc23b6ebe9 Merge pull request 'fix: quick wins (#7, #9, #13)' (#16) from fix/quick-wins into main
CI / test (push) Successful in 14s
CI / review (gpt-5, sonnet, SONNET_REVIEW_TOKEN) (push) Has been skipped
CI / review (gpt-5-mini, gpt, GPT_REVIEW_TOKEN) (push) Has been skipped
2026-05-01 21:30:57 +00:00
Rodin b02ade4f23 fix: quick wins (#7, #9, #13)
CI / test (pull_request) Successful in 13s
CI / review (gpt-5, sonnet, SONNET_REVIEW_TOKEN) (pull_request) Successful in 59s
CI / review (gpt-5-mini, gpt, GPT_REVIEW_TOKEN) (pull_request) Successful in 1m48s
- Add --version flag and log version on startup (closes #9)
- URL-escape ref query parameter in GetFileContentRef (closes #7)
- Add go vet to release workflow (closes #13)

Renamed local url variable to reqURL to avoid shadowing net/url package.
2026-05-01 14:19:37 -07:00
rodin f8e77cf7e3 Merge pull request 'feat: add context.Context + unexport client fields' (#14) from fix/context-and-encapsulation into main
CI / test (push) Successful in 13s
CI / review (gpt-5, sonnet, SONNET_REVIEW_TOKEN) (push) Has been skipped
CI / review (gpt-5-mini, gpt, GPT_REVIEW_TOKEN) (push) Has been skipped
Release / release (push) Successful in 31s
2026-05-01 21:10:37 +00:00
Rodin 69e70466fd fix: address all review findings (context timeout, docs, early exit)
CI / test (pull_request) Successful in 14s
CI / review (gpt-5, sonnet, SONNET_REVIEW_TOKEN) (pull_request) Successful in 1m7s
CI / review (gpt-5-mini, gpt, GPT_REVIEW_TOKEN) (pull_request) Successful in 1m40s
- Overall context timeout now derived from LLM timeout + 1 minute
  (no longer hardcoded 3min that could conflict with longer LLM timeouts)
- Clarify concurrency docs: With* methods are setup-only, not concurrent
- Add ctx.Err() checks in fetchFileContext and fetchPatterns loops
  (break early on cancellation instead of making unnecessary requests)
2026-05-01 13:26:19 -07:00
Rodin 0cca44b65a fix: address all remaining review findings on PR #14
CI / test (pull_request) Successful in 13s
CI / review (gpt-5, sonnet, SONNET_REVIEW_TOKEN) (pull_request) Successful in 1m29s
CI / review (gpt-5-mini, gpt, GPT_REVIEW_TOKEN) (pull_request) Successful in 1m36s
- Fix doc comments: WithTimeout and WithTemperature each get their own
- Add TestWithTimeout (verifies short timeout causes request failure)
- Log warning on directory recursion failure in GetAllFilesInPath
- Note: unexported fields is a breaking change, will document in release notes
2026-05-01 13:17:39 -07:00
Rodin 43041a00f5 fix: rewrite action.yml (was corrupted with duplicate keys)
CI / test (pull_request) Successful in 11s
CI / review (gpt-5, sonnet, SONNET_REVIEW_TOKEN) (pull_request) Successful in 1m34s
CI / review (gpt-5-mini, gpt, GPT_REVIEW_TOKEN) (pull_request) Successful in 1m33s
Clean single definition of all inputs: temperature, timeout,
patterns-repo, patterns-files. Also added runner requirements
comment at the top.
2026-05-01 13:08:18 -07:00
Rodin 1da61e514d feat: make LLM timeout configurable (default 5min)
CI / test (pull_request) Successful in 13s
CI / review (gpt-5, sonnet, SONNET_REVIEW_TOKEN) (pull_request) Successful in 1m6s
CI / review (gpt-5-mini, gpt, GPT_REVIEW_TOKEN) (pull_request) Successful in 1m14s
New flag: --llm-timeout / LLM_TIMEOUT (seconds, default 300)
New builder: llmClient.WithTimeout(duration)
Composite action: new timeout input

Keeps 5 minutes as the sensible default but allows tuning for
larger repos or slower models.
2026-05-01 13:04:00 -07:00
Rodin 401e94d3e4 fix: increase LLM client timeout to 5 minutes
CI / test (pull_request) Successful in 13s
CI / review (gpt-5, sonnet, SONNET_REVIEW_TOKEN) (pull_request) Successful in 1m13s
CI / review (gpt-5-mini, gpt, GPT_REVIEW_TOKEN) (pull_request) Successful in 1m43s
GPT-5-mini timed out on larger diffs (2min was too short).
LLM calls for code review with full file context can take 2-4min.
2026-05-01 13:00:36 -07:00
Rodin cedb5e7b90 fix: address all review findings on PR #14
CI / test (pull_request) Successful in 13s
CI / review (gpt-5, sonnet, SONNET_REVIEW_TOKEN) (pull_request) Successful in 48s
CI / review (gpt-5-mini, gpt, GPT_REVIEW_TOKEN) (pull_request) Failing after 2m12s
- gitea.Client: add concurrency safety doc comment
- gitea.Client: set 30s HTTP client timeout as safety net
- llm.Client: add concurrency safety doc comment
- llm.Client: set 2min HTTP client timeout (LLM calls are slow)
- gitea/client.go: gofmt to fix indentation
- integration_test: update to current BuildSystemPrompt/BuildUserPrompt signatures
- integration_test: use strings.SplitN for owner/repo parsing
2026-05-01 12:56:07 -07:00
Rodin ecebd52371 fix: log warnings instead of swallowing errors
CI / test (pull_request) Successful in 13s
CI / review (gpt-5, sonnet, SONNET_REVIEW_TOKEN) (pull_request) Successful in 1m29s
CI / review (gpt-5-mini, gpt, GPT_REVIEW_TOKEN) (pull_request) Successful in 1m44s
- GetAllFilesInPath: log.Printf when file fetch or dir recursion fails
- integration_test: use strings.SplitN for owner/repo parsing (idiomatic)

Addresses GPT review findings #1, #2.
2026-05-01 12:45:52 -07:00
Rodin 27e0056f29 feat: add context.Context + unexport client fields
CI / test (pull_request) Successful in 13s
CI / review (gpt-5, sonnet, SONNET_REVIEW_TOKEN) (pull_request) Successful in 54s
CI / review (gpt-5-mini, gpt, GPT_REVIEW_TOKEN) (pull_request) Successful in 1m22s
REVIEW.md findings 1-4, 14:
- All Gitea client methods now accept context.Context as first param
- All LLM client methods now accept context.Context as first param
- Use http.NewRequestWithContext for cancellation/timeout support
- Main uses 3-minute timeout context for all operations
- Unexport Client struct fields (baseURL, token, apiKey, etc.)
- Use bytes.NewReader instead of strings.NewReader(string(...))
2026-05-01 12:31:41 -07:00
aweiker ffca0eb016 Merge pull request 'docs: add comprehensive code review report (vs go-patterns)' (#1) from docs/code-review-report into main
CI / test (push) Successful in 14s
CI / review (gpt-5, sonnet, SONNET_REVIEW_TOKEN) (push) Has been skipped
CI / review (gpt-5-mini, gpt, GPT_REVIEW_TOKEN) (push) Has been skipped
Reviewed-on: #1
2026-05-01 19:25:16 +00:00
13 changed files with 759 additions and 122 deletions
+8 -2
View File
@@ -1,6 +1,7 @@
# This composite action is designed for Gitea Actions runners. # This composite action is designed for Gitea Actions runners.
# Gitea Actions supports GitHub Actions syntax including $GITHUB_OUTPUT, # Gitea Actions supports GitHub Actions syntax including $GITHUB_OUTPUT,
# actions/cache, and actions/checkout. # actions/cache, and actions/checkout.
# Requirements: python3, sha256sum, curl (all present on ubuntu-* runners).
name: 'AI Code Review' name: 'AI Code Review'
description: 'Run AI-powered code review on a pull request using review-bot' description: 'Run AI-powered code review on a pull request using review-bot'
@@ -38,17 +39,21 @@ inputs:
required: false required: false
default: '' default: ''
patterns-repo: patterns-repo:
description: 'Repo with language patterns (e.g. rodin/elixir-patterns)' description: 'Comma-separated repos with language patterns (e.g. rodin/elixir-patterns,rodin/phoenix-conventions)'
required: false required: false
default: '' default: ''
patterns-files: patterns-files:
description: 'Comma-separated file paths to fetch from patterns repo' description: 'Comma-separated file paths or directories to fetch from patterns repos'
required: false required: false
default: 'README.md' default: 'README.md'
temperature: temperature:
description: 'LLM temperature (0 = server default)' description: 'LLM temperature (0 = server default)'
required: false required: false
default: '0' default: '0'
timeout:
description: 'LLM request timeout in seconds (default 300)'
required: false
default: '300'
version: version:
description: 'review-bot version to install (e.g. v0.1.0, defaults to latest)' description: 'review-bot version to install (e.g. v0.1.0, defaults to latest)'
required: false required: false
@@ -134,6 +139,7 @@ runs:
PATTERNS_REPO: ${{ inputs.patterns-repo }} PATTERNS_REPO: ${{ inputs.patterns-repo }}
PATTERNS_FILES: ${{ inputs.patterns-files }} PATTERNS_FILES: ${{ inputs.patterns-files }}
LLM_TEMPERATURE: ${{ inputs.temperature }} LLM_TEMPERATURE: ${{ inputs.temperature }}
LLM_TIMEOUT: ${{ inputs.timeout }}
run: | run: |
ARGS="" ARGS=""
if [ "${{ inputs.dry-run }}" = "true" ]; then if [ "${{ inputs.dry-run }}" = "true" ]; then
+3 -2
View File
@@ -31,7 +31,7 @@ jobs:
model: gpt-5 model: gpt-5
- name: gpt - name: gpt
token_secret: GPT_REVIEW_TOKEN token_secret: GPT_REVIEW_TOKEN
model: gpt-5-mini model: gpt-4.1
steps: steps:
- uses: actions/checkout@v4 - uses: actions/checkout@v4
- uses: actions/setup-go@v5 - uses: actions/setup-go@v5
@@ -49,5 +49,6 @@ jobs:
LLM_MODEL: ${{ matrix.model }} LLM_MODEL: ${{ matrix.model }}
CONVENTIONS_FILE: "CONVENTIONS.md" CONVENTIONS_FILE: "CONVENTIONS.md"
PATTERNS_REPO: "rodin/go-patterns" PATTERNS_REPO: "rodin/go-patterns"
PATTERNS_FILES: "README.md,docs/" PATTERNS_FILES: "README.md,patterns/"
LLM_TIMEOUT: "600"
run: ./review-bot run: ./review-bot
+3 -1
View File
@@ -16,7 +16,9 @@ jobs:
go-version: '1.26' go-version: '1.26'
- name: Run tests - name: Run tests
run: go test ./... run: |
go vet ./...
go test ./...
- name: Build binaries - name: Build binaries
run: | run: |
+226
View File
@@ -0,0 +1,226 @@
// 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"
)
// 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 characters 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 (never trimmed)
}
// 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 {
sections.Diff = truncateUTF8(sections.Diff, maxChars) + diffTruncMarker
} 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")
}
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
}
// Walk backwards from maxBytes to find a valid UTF-8 boundary
for maxBytes > 0 && !isUTF8Start(s[maxBytes]) {
maxBytes--
}
return s[:maxBytes]
}
// isUTF8Start returns true if b is a valid start byte for a UTF-8 sequence
// (single-byte ASCII or multi-byte lead byte, not a continuation byte).
func isUTF8Start(b byte) bool {
return b&0xC0 != 0x80
}
+203
View File
@@ -0,0 +1,203 @@
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)
}
}
+65 -18
View File
@@ -1,13 +1,16 @@
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"
@@ -16,6 +19,7 @@ import (
var version = "dev" 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,9 +34,17 @@ func main() {
patternsFiles := flag.String("patterns-files", envOrDefault("PATTERNS_FILES", "README.md"), "Comma-separated file paths to fetch from patterns repo") 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)") 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 == "" {
@@ -63,18 +75,26 @@ func main() {
if *llmTemp > 0 { if *llmTemp > 0 {
llmClient.WithTemperature(*llmTemp) 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(owner, repoName, prNumber) pr, err := giteaClient.GetPullRequest(ctx, 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(owner, repoName, prNumber) diff, err := giteaClient.GetPullRequestDiff(ctx, 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)
} }
@@ -82,11 +102,11 @@ func main() {
// Step 3: Fetch full file content for modified files // Step 3: Fetch full file content for modified files
fileContext := "" fileContext := ""
files, err := giteaClient.GetPullRequestFiles(owner, repoName, prNumber) files, err := giteaClient.GetPullRequestFiles(ctx, owner, repoName, prNumber)
if err != nil { if err != nil {
log.Printf("Warning: could not fetch PR files list: %v", err) log.Printf("Warning: could not fetch PR files list: %v", err)
} else { } else {
fileContext = fetchFileContext(giteaClient, owner, repoName, pr.Head.Ref, files) fileContext = fetchFileContext(ctx, giteaClient, owner, repoName, pr.Head.Ref, files)
log.Printf("Fetched full context for %d files", len(files)) log.Printf("Fetched full context for %d files", len(files))
} }
@@ -94,7 +114,7 @@ func main() {
ciPassed := true ciPassed := true
ciDetails := "" ciDetails := ""
if pr.Head.Sha != "" { if pr.Head.Sha != "" {
statuses, err := giteaClient.GetCommitStatuses(owner, repoName, pr.Head.Sha) statuses, err := giteaClient.GetCommitStatuses(ctx, 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 {
@@ -106,7 +126,7 @@ func main() {
// Step 5: Load conventions file if specified // Step 5: Load conventions file if specified
conventions := "" conventions := ""
if *conventionsFile != "" { if *conventionsFile != "" {
content, err := giteaClient.GetFileContent(owner, repoName, *conventionsFile) content, err := giteaClient.GetFileContent(ctx, 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 {
@@ -118,22 +138,33 @@ func main() {
// Step 6: Load patterns from external repo if specified // Step 6: Load patterns from external repo if specified
patterns := "" patterns := ""
if *patternsRepo != "" { if *patternsRepo != "" {
patterns = fetchPatterns(giteaClient, *patternsRepo, *patternsFiles) patterns = fetchPatterns(ctx, giteaClient, *patternsRepo, *patternsFiles)
log.Printf("Loaded patterns from %s (%d bytes)", *patternsRepo, len(patterns)) log.Printf("Loaded patterns from %s (%d bytes)", *patternsRepo, len(patterns))
} }
// Step 7: Build prompts // Step 7: Budget-aware prompt assembly
systemPrompt := review.BuildSystemPrompt(conventions, patterns) sections := budget.Sections{
userPrompt := review.BuildUserPrompt(pr.Title, pr.Body, diff, fileContext, ciPassed, ciDetails) 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 // 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: systemPrompt}, {Role: "system", Content: budgetResult.SystemPrompt},
{Role: "user", Content: userPrompt}, {Role: "user", Content: budgetResult.UserPrompt},
} }
response, err := llmClient.Complete(messages) response, err := llmClient.Complete(ctx, messages)
if err != nil { if err != nil {
log.Fatalf("LLM request failed: %v", err) log.Fatalf("LLM request failed: %v", err)
} }
@@ -158,20 +189,23 @@ func main() {
} }
log.Printf("Posting review (event=%s)...", event) log.Printf("Posting review (event=%s)...", event)
if err := giteaClient.PostReview(owner, repoName, prNumber, event, reviewBody); err != nil { if err := giteaClient.PostReview(ctx, 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. // fetchFileContext fetches the full content of modified files from the PR branch.
func fetchFileContext(client *gitea.Client, owner, repo, ref string, files []gitea.ChangedFile) string { func fetchFileContext(ctx context.Context, client *gitea.Client, owner, repo, ref string, files []gitea.ChangedFile) string {
var sb strings.Builder var sb strings.Builder
for _, f := range files { for _, f := range files {
if ctx.Err() != nil {
break
}
if f.Status == "removed" { if f.Status == "removed" {
continue // Skip deleted files continue // Skip deleted files
} }
content, err := client.GetFileContentRef(owner, repo, f.Filename, ref) content, err := client.GetFileContentRef(ctx, owner, repo, f.Filename, ref)
if err != nil { if err != nil {
log.Printf("Warning: could not fetch %s: %v", f.Filename, err) log.Printf("Warning: could not fetch %s: %v", f.Filename, err)
continue continue
@@ -188,13 +222,16 @@ func fetchFileContext(client *gitea.Client, owner, repo, ref string, files []git
// patternsRepo is comma-separated list of owner/name repos. // patternsRepo is comma-separated list of owner/name repos.
// patternsFiles is comma-separated list of file paths or directories. // 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. // If a path ends with / or is a directory, all files within it are fetched recursively.
func fetchPatterns(client *gitea.Client, patternsRepo, patternsFiles string) string { func fetchPatterns(ctx context.Context, client *gitea.Client, patternsRepo, patternsFiles string) string {
var sb strings.Builder var sb strings.Builder
repos := strings.Split(patternsRepo, ",") repos := strings.Split(patternsRepo, ",")
paths := strings.Split(patternsFiles, ",") paths := strings.Split(patternsFiles, ",")
for _, repoRef := range repos { for _, repoRef := range repos {
if ctx.Err() != nil {
break
}
repoRef = strings.TrimSpace(repoRef) repoRef = strings.TrimSpace(repoRef)
if repoRef == "" { if repoRef == "" {
continue continue
@@ -212,7 +249,7 @@ func fetchPatterns(client *gitea.Client, patternsRepo, patternsFiles string) str
continue continue
} }
files, err := client.GetAllFilesInPath(owner, repo, path) files, err := client.GetAllFilesInPath(ctx, owner, repo, path)
if err != nil { if err != nil {
log.Printf("Warning: could not fetch %s from %s: %v", path, repoRef, err) log.Printf("Warning: could not fetch %s from %s: %v", path, repoRef, err)
continue continue
@@ -279,3 +316,13 @@ func envOrDefaultFloat(key string, defaultVal float64) float64 {
} }
return defaultVal 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
}
+71 -42
View File
@@ -1,26 +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{}, http: &http.Client{Timeout: 30 * time.Second},
} }
} }
@@ -49,9 +58,9 @@ type ChangedFile struct {
} }
// GetPullRequest fetches PR metadata. // GetPullRequest fetches PR metadata.
func (c *Client) GetPullRequest(owner, repo string, number int) (*PullRequest, error) { func (c *Client) GetPullRequest(ctx context.Context, owner, repo string, number int) (*PullRequest, error) {
url := fmt.Sprintf("%s/api/v1/repos/%s/%s/pulls/%d", c.BaseURL, owner, repo, number) reqURL := fmt.Sprintf("%s/api/v1/repos/%s/%s/pulls/%d", c.baseURL, owner, repo, number)
body, err := c.doGet(url) body, err := c.doGet(ctx, reqURL)
if err != nil { if err != nil {
return nil, fmt.Errorf("fetch PR: %w", err) return nil, fmt.Errorf("fetch PR: %w", err)
} }
@@ -63,9 +72,9 @@ func (c *Client) GetPullRequest(owner, repo string, number int) (*PullRequest, e
} }
// GetPullRequestDiff fetches the unified diff for a PR. // GetPullRequestDiff fetches the unified diff for a PR.
func (c *Client) GetPullRequestDiff(owner, repo string, number int) (string, error) { func (c *Client) GetPullRequestDiff(ctx context.Context, owner, repo string, number int) (string, error) {
url := fmt.Sprintf("%s/api/v1/repos/%s/%s/pulls/%d.diff", c.BaseURL, owner, repo, number) reqURL := fmt.Sprintf("%s/api/v1/repos/%s/%s/pulls/%d.diff", c.baseURL, owner, repo, number)
body, err := c.doGet(url) body, err := c.doGet(ctx, reqURL)
if err != nil { if err != nil {
return "", fmt.Errorf("fetch diff: %w", err) return "", fmt.Errorf("fetch diff: %w", err)
} }
@@ -73,9 +82,9 @@ func (c *Client) GetPullRequestDiff(owner, repo string, number int) (string, err
} }
// GetPullRequestFiles fetches the list of files changed in a PR. // GetPullRequestFiles fetches the list of files changed in a PR.
func (c *Client) GetPullRequestFiles(owner, repo string, number int) ([]ChangedFile, error) { func (c *Client) GetPullRequestFiles(ctx context.Context, owner, repo string, number int) ([]ChangedFile, error) {
url := fmt.Sprintf("%s/api/v1/repos/%s/%s/pulls/%d/files", c.BaseURL, owner, repo, number) reqURL := fmt.Sprintf("%s/api/v1/repos/%s/%s/pulls/%d/files", c.baseURL, owner, repo, number)
body, err := c.doGet(url) body, err := c.doGet(ctx, reqURL)
if err != nil { if err != nil {
return nil, fmt.Errorf("fetch PR files: %w", err) return nil, fmt.Errorf("fetch PR files: %w", err)
} }
@@ -87,9 +96,9 @@ func (c *Client) GetPullRequestFiles(owner, repo string, number int) ([]ChangedF
} }
// GetCommitStatuses fetches CI statuses for a commit SHA. // GetCommitStatuses fetches CI statuses for a commit SHA.
func (c *Client) GetCommitStatuses(owner, repo, sha string) ([]CommitStatus, error) { func (c *Client) GetCommitStatuses(ctx context.Context, owner, repo, sha string) ([]CommitStatus, error) {
url := fmt.Sprintf("%s/api/v1/repos/%s/%s/commits/%s/statuses", c.BaseURL, owner, repo, sha) reqURL := fmt.Sprintf("%s/api/v1/repos/%s/%s/commits/%s/statuses", c.baseURL, owner, repo, sha)
body, err := c.doGet(url) body, err := c.doGet(ctx, reqURL)
if err != nil { if err != nil {
return nil, fmt.Errorf("fetch commit statuses: %w", err) return nil, fmt.Errorf("fetch commit statuses: %w", err)
} }
@@ -101,9 +110,9 @@ func (c *Client) GetCommitStatuses(owner, repo, sha string) ([]CommitStatus, err
} }
// 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(owner, repo, filepath string) (string, error) { func (c *Client) GetFileContent(ctx context.Context, owner, repo, filepath string) (string, error) {
url := fmt.Sprintf("%s/api/v1/repos/%s/%s/raw/%s", c.BaseURL, owner, repo, filepath) reqURL := fmt.Sprintf("%s/api/v1/repos/%s/%s/raw/%s", c.baseURL, owner, repo, escapePath(filepath))
body, err := c.doGet(url) body, err := c.doGet(ctx, reqURL)
if err != nil { if err != nil {
return "", fmt.Errorf("fetch file %s: %w", filepath, err) return "", fmt.Errorf("fetch file %s: %w", filepath, err)
} }
@@ -111,9 +120,9 @@ func (c *Client) GetFileContent(owner, repo, filepath string) (string, error) {
} }
// GetFileContentRef fetches a file from a specific ref (branch/tag/sha) in a repo. // GetFileContentRef fetches a file from a specific ref (branch/tag/sha) in a repo.
func (c *Client) GetFileContentRef(owner, repo, filepath, ref string) (string, error) { func (c *Client) GetFileContentRef(ctx context.Context, owner, repo, filepath, ref string) (string, error) {
url := fmt.Sprintf("%s/api/v1/repos/%s/%s/raw/%s?ref=%s", c.BaseURL, owner, repo, filepath, ref) 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(url) body, err := c.doGet(ctx, reqURL)
if err != nil { if err != nil {
return "", fmt.Errorf("fetch file %s@%s: %w", filepath, ref, err) return "", fmt.Errorf("fetch file %s@%s: %w", filepath, ref, err)
} }
@@ -122,8 +131,8 @@ func (c *Client) GetFileContentRef(owner, repo, filepath, ref string) (string, e
// 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(owner, repo string, number int, event, body string) error { func (c *Client) PostReview(ctx context.Context, owner, repo string, number int, event, body string) error {
url := fmt.Sprintf("%s/api/v1/repos/%s/%s/pulls/%d/reviews", c.BaseURL, owner, repo, number) reqURL := 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"`
@@ -138,14 +147,14 @@ func (c *Client) PostReview(owner, repo string, number int, event, body string)
return fmt.Errorf("marshal review payload: %w", err) return fmt.Errorf("marshal review payload: %w", err)
} }
req, err := http.NewRequest("POST", url, strings.NewReader(string(data))) req, err := http.NewRequestWithContext(ctx, http.MethodPost, reqURL, bytes.NewReader(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)
} }
@@ -158,14 +167,14 @@ func (c *Client) PostReview(owner, repo string, number int, event, body string)
return nil return nil
} }
func (c *Client) doGet(url string) ([]byte, error) { func (c *Client) doGet(ctx context.Context, reqURL string) ([]byte, error) {
req, err := http.NewRequest("GET", url, nil) req, err := http.NewRequestWithContext(ctx, http.MethodGet, reqURL, 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
} }
@@ -178,6 +187,18 @@ func (c *Client) doGet(url 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. // ContentEntry represents a file or directory entry from the contents API.
type ContentEntry struct { type ContentEntry struct {
Name string `json:"name"` Name string `json:"name"`
@@ -186,9 +207,15 @@ type ContentEntry struct {
} }
// ListContents lists files and directories at a given path in a repo. // ListContents lists files and directories at a given path in a repo.
func (c *Client) ListContents(owner, repo, path string) ([]ContentEntry, error) { // Pass an empty path to list the repository root.
url := fmt.Sprintf("%s/api/v1/repos/%s/%s/contents/%s", c.BaseURL, owner, repo, path) func (c *Client) ListContents(ctx context.Context, owner, repo, path string) ([]ContentEntry, error) {
body, err := c.doGet(url) 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 { if err != nil {
return nil, fmt.Errorf("list contents %s: %w", path, err) return nil, fmt.Errorf("list contents %s: %w", path, err)
} }
@@ -202,14 +229,14 @@ func (c *Client) ListContents(owner, repo, path string) ([]ContentEntry, error)
// GetAllFilesInPath recursively fetches all file contents under a path. // 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 file, returns just that file's content.
// If the path is a directory, recursively fetches all files within it. // If the path is a directory, recursively fetches all files within it.
func (c *Client) GetAllFilesInPath(owner, repo, path string) (map[string]string, error) { func (c *Client) GetAllFilesInPath(ctx context.Context, owner, repo, path string) (map[string]string, error) {
results := make(map[string]string) results := make(map[string]string)
// Try listing as directory first // Try listing as directory first
entries, err := c.ListContents(owner, repo, path) entries, err := c.ListContents(ctx, owner, repo, path)
if err != nil { if err != nil {
// Might be a file, try fetching directly // Might be a file, try fetching directly
content, fileErr := c.GetFileContent(owner, repo, path) content, fileErr := c.GetFileContent(ctx, owner, repo, path)
if fileErr != nil { if fileErr != nil {
return nil, fmt.Errorf("path %q is neither a file nor directory: %w", path, err) return nil, fmt.Errorf("path %q is neither a file nor directory: %w", path, err)
} }
@@ -220,14 +247,16 @@ func (c *Client) GetAllFilesInPath(owner, repo, path string) (map[string]string,
for _, entry := range entries { for _, entry := range entries {
switch entry.Type { switch entry.Type {
case "file": case "file":
content, err := c.GetFileContent(owner, repo, entry.Path) content, err := c.GetFileContent(ctx, owner, repo, entry.Path)
if err != nil { if err != nil {
continue // Skip files we can't read log.Printf("Warning: could not fetch file %s: %v", entry.Path, err)
continue
} }
results[entry.Path] = content results[entry.Path] = content
case "dir": case "dir":
subResults, err := c.GetAllFilesInPath(owner, repo, entry.Path) subResults, err := c.GetAllFilesInPath(ctx, owner, repo, entry.Path)
if err != nil { if err != nil {
log.Printf("Warning: could not recurse into %s: %v", entry.Path, err)
continue continue
} }
for k, v := range subResults { for k, v := range subResults {
+37 -12
View File
@@ -1,6 +1,7 @@
package gitea package gitea
import ( import (
"context"
"encoding/json" "encoding/json"
"fmt" "fmt"
"net/http" "net/http"
@@ -28,7 +29,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("owner", "repo", 1) got, err := client.GetPullRequest(context.Background(), "owner", "repo", 1)
if err != nil { if err != nil {
t.Fatalf("unexpected error: %v", err) t.Fatalf("unexpected error: %v", err)
} }
@@ -55,7 +56,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("owner", "repo", 5) got, err := client.GetPullRequestDiff(context.Background(), "owner", "repo", 5)
if err != nil { if err != nil {
t.Fatalf("unexpected error: %v", err) t.Fatalf("unexpected error: %v", err)
} }
@@ -80,7 +81,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("owner", "repo", "abc123") got, err := client.GetCommitStatuses(context.Background(), "owner", "repo", "abc123")
if err != nil { if err != nil {
t.Fatalf("unexpected error: %v", err) t.Fatalf("unexpected error: %v", err)
} }
@@ -127,7 +128,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("owner", "repo", 3, "APPROVED", "LGTM") err := client.PostReview(context.Background(), "owner", "repo", 3, "APPROVED", "LGTM")
if err != nil { if err != nil {
t.Fatalf("unexpected error: %v", err) t.Fatalf("unexpected error: %v", err)
} }
@@ -141,7 +142,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("owner", "repo", 999) _, err := client.GetPullRequest(context.Background(), "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")
} }
@@ -154,7 +155,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("owner", "repo", 1) _, err := client.GetPullRequest(context.Background(), "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")
} }
@@ -168,7 +169,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("owner", "repo", 1, "APPROVED", "test") err := client.PostReview(context.Background(), "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")
} }
@@ -186,7 +187,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("owner", "repo", "CONVENTIONS.md") got, err := client.GetFileContent(context.Background(), "owner", "repo", "CONVENTIONS.md")
if err != nil { if err != nil {
t.Fatalf("unexpected error: %v", err) t.Fatalf("unexpected error: %v", err)
} }
@@ -206,7 +207,7 @@ func TestGetPullRequestFiles(t *testing.T) {
defer server.Close() defer server.Close()
client := NewClient(server.URL, "test-token") client := NewClient(server.URL, "test-token")
files, err := client.GetPullRequestFiles("owner", "repo", 1) files, err := client.GetPullRequestFiles(context.Background(), "owner", "repo", 1)
if err != nil { if err != nil {
t.Fatalf("unexpected error: %v", err) t.Fatalf("unexpected error: %v", err)
} }
@@ -231,7 +232,7 @@ func TestGetFileContentRef(t *testing.T) {
defer server.Close() defer server.Close()
client := NewClient(server.URL, "test-token") client := NewClient(server.URL, "test-token")
content, err := client.GetFileContentRef("owner", "repo", "main.go", "feature-branch") content, err := client.GetFileContentRef(context.Background(), "owner", "repo", "main.go", "feature-branch")
if err != nil { if err != nil {
t.Fatalf("unexpected error: %v", err) t.Fatalf("unexpected error: %v", err)
} }
@@ -251,7 +252,7 @@ func TestListContents(t *testing.T) {
defer server.Close() defer server.Close()
client := NewClient(server.URL, "test-token") client := NewClient(server.URL, "test-token")
entries, err := client.ListContents("owner", "repo", "docs") entries, err := client.ListContents(context.Background(), "owner", "repo", "docs")
if err != nil { if err != nil {
t.Fatalf("unexpected error: %v", err) t.Fatalf("unexpected error: %v", err)
} }
@@ -282,7 +283,7 @@ func TestGetAllFilesInPath_File(t *testing.T) {
defer server.Close() defer server.Close()
client := NewClient(server.URL, "test-token") client := NewClient(server.URL, "test-token")
files, err := client.GetAllFilesInPath("owner", "repo", "README.md") files, err := client.GetAllFilesInPath(context.Background(), "owner", "repo", "README.md")
if err != nil { if err != nil {
t.Fatalf("unexpected error: %v", err) t.Fatalf("unexpected error: %v", err)
} }
@@ -293,3 +294,27 @@ func TestGetAllFilesInPath_File(t *testing.T) {
t.Errorf("unexpected content: %q", files["README.md"]) 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)
}
})
}
}
+13 -12
View File
@@ -3,8 +3,10 @@
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"
@@ -42,28 +44,27 @@ func TestIntegration_FullReviewFlow(t *testing.T) {
} }
// Parse owner/repo // Parse owner/repo
owner, repoName := "", "" parts := strings.SplitN(giteaRepo, "/", 2)
for i, c := range giteaRepo { if len(parts) != 2 {
if c == / { t.Fatalf("Invalid repo format %q", giteaRepo)
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(owner, repoName, prNumber) pr, err := giteaClient.GetPullRequest(ctx, 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(owner, repoName, prNumber) diff, err := giteaClient.GetPullRequestDiff(ctx, owner, repoName, prNumber)
if err != nil { if err != nil {
t.Fatalf("GetPullRequestDiff: %v", err) t.Fatalf("GetPullRequestDiff: %v", err)
} }
@@ -73,12 +74,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([]llm.Message{ response, err := llmClient.Complete(ctx, []llm.Message{
{Role: "system", Content: systemPrompt}, {Role: "system", Content: systemPrompt},
{Role: "user", Content: userPrompt}, {Role: "user", Content: userPrompt},
}) })
+28 -18
View File
@@ -1,36 +1,47 @@
// 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 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{}, http: &http.Client{Timeout: 5 * time.Minute},
} }
} }
// 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). // WithTemperature sets the temperature for LLM requests (0 = omit, uses server default).
func (c *Client) WithTemperature(t float64) *Client { func (c *Client) WithTemperature(t float64) *Client {
c.Temperature = t c.temperature = t
return c return c
} }
@@ -57,12 +68,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(messages []Message) (string, error) { func (c *Client) Complete(ctx context.Context, messages []Message) (string, error) {
reqBody := ChatRequest{ reqBody := ChatRequest{
Model: c.Model, Model: c.model,
Temperature: c.Temperature, Temperature: c.temperature,
Messages: messages, Messages: messages,
} }
data, err := json.Marshal(reqBody) data, err := json.Marshal(reqBody)
@@ -70,15 +80,15 @@ func (c *Client) Complete(messages []Message) (string, error) {
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.NewRequest("POST", url, bytes.NewReader(data)) req, err := http.NewRequestWithContext(ctx, "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)
} }
+34 -11
View File
@@ -1,10 +1,12 @@
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) {
@@ -51,7 +53,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([]Message{{Role: "user", Content: "Hi"}}) got, err := client.Complete(context.Background(), []Message{{Role: "user", Content: "Hi"}})
if err != nil { if err != nil {
t.Fatalf("unexpected error: %v", err) t.Fatalf("unexpected error: %v", err)
} }
@@ -68,7 +70,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([]Message{{Role: "user", Content: "Hi"}}) _, err := client.Complete(context.Background(), []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")
} }
@@ -82,7 +84,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([]Message{{Role: "user", Content: "Hi"}}) _, err := client.Complete(context.Background(), []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")
} }
@@ -95,7 +97,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([]Message{{Role: "user", Content: "Hi"}}) _, err := client.Complete(context.Background(), []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")
} }
@@ -103,7 +105,7 @@ 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([]Message{{Role: "user", Content: "Hi"}}) _, err := client.Complete(context.Background(), []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")
} }
@@ -111,16 +113,16 @@ func TestComplete_ServerDown(t *testing.T) {
func TestWithTemperature(t *testing.T) { func TestWithTemperature(t *testing.T) {
client := NewClient("http://example.com", "key", "model") client := NewClient("http://example.com", "key", "model")
if client.Temperature != 0 { if client.temperature != 0 {
t.Errorf("expected initial temperature 0, got %f", client.Temperature) t.Errorf("expected initial temperature 0, got %f", client.temperature)
} }
result := client.WithTemperature(0.7) result := client.WithTemperature(0.7)
if result != client { if result != client {
t.Error("WithTemperature should return the same client for chaining") t.Error("WithTemperature should return the same client for chaining")
} }
if client.Temperature != 0.7 { if client.temperature != 0.7 {
t.Errorf("expected temperature 0.7, got %f", client.Temperature) t.Errorf("expected temperature 0.7, got %f", client.temperature)
} }
} }
@@ -147,7 +149,7 @@ func TestComplete_TemperatureOmittedWhenZero(t *testing.T) {
defer server.Close() defer server.Close()
client := NewClient(server.URL, "key", "model") client := NewClient(server.URL, "key", "model")
_, err := client.Complete([]Message{{Role: "user", Content: "Hi"}}) _, err := client.Complete(context.Background(), []Message{{Role: "user", Content: "Hi"}})
if err != nil { if err != nil {
t.Fatalf("unexpected error: %v", err) t.Fatalf("unexpected error: %v", err)
} }
@@ -180,8 +182,29 @@ func TestComplete_TemperatureIncludedWhenSet(t *testing.T) {
defer server.Close() defer server.Close()
client := NewClient(server.URL, "key", "model").WithTemperature(0.7) client := NewClient(server.URL, "key", "model").WithTemperature(0.7)
_, err := client.Complete([]Message{{Role: "user", Content: "Hi"}}) _, err := client.Complete(context.Background(), []Message{{Role: "user", Content: "Hi"}})
if err != nil { if err != nil {
t.Fatalf("unexpected error: %v", err) 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")
}
}
+28 -4
View File
@@ -1,3 +1,5 @@
// Package review builds prompts for AI code review and parses LLM responses
// into structured review results.
package review package review
import ( import (
@@ -5,8 +7,10 @@ import (
"strings" "strings"
) )
// BuildSystemPrompt constructs the system prompt for the LLM reviewer. // BuildSystemBase returns the core system prompt instructions without
func BuildSystemPrompt(conventions, patterns string) string { // patterns or conventions. Used by the budget package to separate
// 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")
@@ -40,6 +44,15 @@ func BuildSystemPrompt(conventions, patterns string) 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 != "" { 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)) 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))
} }
@@ -51,8 +64,9 @@ func BuildSystemPrompt(conventions, patterns string) string {
return sb.String() return sb.String()
} }
// BuildUserPrompt constructs the user message with PR context. // BuildUserMeta returns the PR metadata header (title, description, CI status)
func BuildUserPrompt(title, description, diff, fileContext string, ciPassed bool, ciDetails string) string { // without the diff or file context. Used by the budget package.
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))
@@ -71,6 +85,16 @@ func BuildUserPrompt(title, description, diff, fileContext string, ciPassed bool
sb.WriteString(fmt.Sprintf("CI Details: %s\n", ciDetails)) sb.WriteString(fmt.Sprintf("CI Details: %s\n", ciDetails))
} }
return sb.String()
}
// 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 != "" { if fileContext != "" {
sb.WriteString("\n### Full File Context (modified files)\n\n") sb.WriteString("\n### Full File Context (modified files)\n\n")
sb.WriteString(fileContext) sb.WriteString(fileContext)
+40
View File
@@ -116,3 +116,43 @@ func TestBuildUserPrompt_WithoutFileContext(t *testing.T) {
t.Error("should not include file context section when empty") 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")
}
}