fix: address all review findings (context timeout, docs, early exit)
- 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)
This commit is contained in:
@@ -70,8 +70,9 @@ func main() {
|
||||
llmClient.WithTimeout(time.Duration(*llmTimeout) * time.Second)
|
||||
}
|
||||
|
||||
// Create a top-level context with a 3-minute timeout for all operations
|
||||
ctx, cancel := context.WithTimeout(context.Background(), 3*time.Minute)
|
||||
// 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)
|
||||
@@ -178,6 +179,9 @@ func main() {
|
||||
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
|
||||
}
|
||||
@@ -205,6 +209,9 @@ func fetchPatterns(ctx context.Context, client *gitea.Client, patternsRepo, patt
|
||||
paths := strings.Split(patternsFiles, ",")
|
||||
|
||||
for _, repoRef := range repos {
|
||||
if ctx.Err() != nil {
|
||||
break
|
||||
}
|
||||
repoRef = strings.TrimSpace(repoRef)
|
||||
if repoRef == "" {
|
||||
continue
|
||||
|
||||
@@ -13,6 +13,7 @@ import (
|
||||
|
||||
// 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 {
|
||||
baseURL string
|
||||
apiKey string
|
||||
|
||||
Reference in New Issue
Block a user