Perf/bound review latency

cmd/opencodereview/flags_test.go

@@ -66,6 +66,40 @@ func TestParseReviewFlags_NegativeMaxTools(t *testing.T) {
 	}
 }
 
+func TestParseReviewFlags_PlanTimeout(t *testing.T) {
+	opts, err := parseReviewFlags([]string{"--plan-timeout", "120"})
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	if opts.planTimeoutSecs != 120 {
+		t.Errorf("planTimeoutSecs = %d, want 120", opts.planTimeoutSecs)
+	}
+}
+
+func TestParseReviewFlags_NegativePlanTimeout(t *testing.T) {
+	_, err := parseReviewFlags([]string{"--plan-timeout", "-1"})
+	if err == nil {
+		t.Fatal("expected error for negative plan-timeout")
+	}
+}
+
+func TestParseReviewFlags_MaxTokens(t *testing.T) {
+	opts, err := parseReviewFlags([]string{"--max-tokens", "30000"})
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	if opts.maxTokens != 30000 {
+		t.Errorf("maxTokens = %d, want 30000", opts.maxTokens)
+	}
+}
+
+func TestParseReviewFlags_NegativeMaxTokens(t *testing.T) {
+	_, err := parseReviewFlags([]string{"--max-tokens", "-1"})
+	if err == nil {
+		t.Fatal("expected error for negative max-tokens")
+	}
+}
+
 func TestParseReviewFlags_MaxToolsBelowMin(t *testing.T) {
 	opts, err := parseReviewFlags([]string{"--max-tools", "5"})
 	if err != nil {

cmd/opencodereview/review_cmd.go

@@ -36,6 +36,8 @@ type reviewOptions struct {
 	model           string
 	concurrency     int
 	perFileTimeout  int
+	planTimeoutSecs int
+	maxTokens       int
 	maxTools        int
 	maxGitProcs     int
 	maxTokensBudget int
@@ -98,6 +100,9 @@ func executeReview(opts reviewOptions) error {
 	if err != nil {
 		return err
 	}
+	if opts.maxTokens > 0 {
+		cc.Template.MaxTokens = opts.maxTokens
+	}
 	applyCLIExcludes(cc, splitPaths(opts.excludes))
 
 	// Security (#112): reject ref-option injection before any git invocation.
@@ -179,6 +184,7 @@ func executeReview(opts reviewOptions) error {
 		CommentWorkerPool:     agent.NewCommentWorkerPool(opts.concurrency),
 		MaxConcurrency:        opts.concurrency,
 		ConcurrentTaskTimeout: opts.perFileTimeout,
+		PlanTaskTimeout:       time.Duration(opts.planTimeoutSecs) * time.Second,
 		Model:                 rt.Model,
 		Provider:              rt.Provider,
 		Background:            opts.background,

cmd/opencodereview/shared.go

@@ -40,7 +40,7 @@ type commonContext struct {
 }
 
 // loadCommonContext validates the working directory, loads the embedded
-// template, raises MaxToolRequestTimes when maxTools exceeds the default,
+// template, overrides MaxToolRequestTimes when maxTools is explicitly set,
 // resolves the absolute repo path, loads system review rules, and creates
 // the global git subprocess limiter. Both review and scan callers go
 // through this so the startup sequence stays consistent.
@@ -53,7 +53,7 @@ func loadCommonContext(repoDirInput, rulePath string, maxTools, maxGitProcs int,
 	if err != nil {
 		return nil, fmt.Errorf("load default template: %w", err)
 	}
-	if maxTools > tpl.MaxToolRequestTimes {
+	if maxTools > 0 {
 		tpl.MaxToolRequestTimes = maxTools
 	}
 	if err := tpl.Validate(); err != nil {

cmd/opencodereview/shared_flags.go

@@ -101,6 +101,12 @@ func validateReviewOptions(opts *reviewOptions) error {
 	if opts.preview && opts.resume != "" {
 		return fmt.Errorf("--preview and --resume cannot be used together")
 	}
+	if opts.planTimeoutSecs < 0 {
+		return fmt.Errorf("--plan-timeout must be a non-negative integer (0 means no separate timeout)")
+	}
+	if opts.maxTokens < 0 {
+		return fmt.Errorf("--max-tokens must be a non-negative integer (0 means use template default)")
+	}
 	if err := validateAudience(opts.audience); err != nil {
 		return err
 	}
@@ -151,6 +157,8 @@ func registerReviewFlags(cmd *cobra.Command, opts *reviewOptions) {
 	addExcludeFlag(cmd, &opts.excludes)
 	addOutputFlags(cmd, &opts.outputFormat, &opts.audience)
 	addConcurrencyFlags(cmd, &opts.concurrency, &opts.perFileTimeout, &opts.maxTools, &opts.maxGitProcs, &opts.maxTokensBudget)
+	cmd.Flags().IntVar(&opts.planTimeoutSecs, "plan-timeout", 0, "per-file plan task timeout in seconds (0 = use the file timeout only)")
+	cmd.Flags().IntVar(&opts.maxTokens, "max-tokens", 0, "maximum tokens retained in each LLM conversation (0 = template default)")
 	addBackgroundFlags(cmd, &opts.background, &opts.backgroundFile)
 	addModelFlag(cmd, &opts.model)
 	addPreviewFlag(cmd, &opts.preview)

cmd/opencodereview/shared_test.go

@@ -17,6 +17,34 @@ func TestApplyCLIExcludes_Empty(t *testing.T) {
 	}
 }
 
+func TestLoadCommonContext_MaxToolsOverride(t *testing.T) {
+	t.Setenv("HOME", t.TempDir())
+	repoDir := t.TempDir()
+
+	tests := []struct {
+		name     string
+		maxTools int
+		want     int
+	}{
+		{name: "template default", maxTools: 0, want: 30},
+		{name: "lower bound", maxTools: 10, want: 10},
+		{name: "lower than default", maxTools: 15, want: 15},
+		{name: "higher than default", maxTools: 40, want: 40},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			cc, err := loadCommonContext(repoDir, "", tt.maxTools, 0, false)
+			if err != nil {
+				t.Fatalf("loadCommonContext() error: %v", err)
+			}
+			if got := cc.Template.MaxToolRequestTimes; got != tt.want {
+				t.Errorf("MaxToolRequestTimes = %d, want %d", got, tt.want)
+			}
+		})
+	}
+}
+
 func TestApplyCLIExcludes_AppendsPatterns(t *testing.T) {
 	cc := &commonContext{FileFilter: &rules.FileFilter{Exclude: []string{"a"}}}
 	applyCLIExcludes(cc, []string{"b", "c"})

internal/agent/agent.go

@@ -100,6 +100,11 @@ type Args struct {
 	// Concurrent task timeout in minutes. 0 means no timeout.
 	ConcurrentTaskTimeout int
 
+	// PlanTaskTimeout bounds the optional plan LLM call. When it expires,
+	// executeSubtask continues with the main task without plan guidance.
+	// A non-positive value means the per-file context is the only deadline.
+	PlanTaskTimeout time.Duration
+
 	// CommentCollector collects review comments generated by the code_comment tool.
 	CommentCollector *tool.CommentCollector
 
@@ -1460,6 +1465,12 @@ func (a *Agent) extFromPath(path string) string {
 // executePlanPhase runs the plan task for a single file, sending template messages
 // with resolved placeholders and collecting the LLM response as plan guidance.
 func (a *Agent) executePlanPhase(ctx context.Context, newPath, rawDiff, changeFiles, rule string) (string, error) {
+	if a.args.PlanTaskTimeout > 0 {
+		var cancel context.CancelFunc
+		ctx, cancel = context.WithTimeout(ctx, a.args.PlanTaskTimeout)
+		defer cancel()
+	}
+
 	ctx, span := telemetry.StartSpan(ctx, "plan.execute")
 	defer span.End()
 	telemetry.SetAttr(span, "file.path", newPath)

internal/agent/coverage_test.go

@@ -6,6 +6,7 @@ import (
 	"fmt"
 	"strings"
 	"testing"
+	"time"
 
 	"github.com/alibaba/open-code-review/internal/config/rules"
 	"github.com/alibaba/open-code-review/internal/config/template"
@@ -15,6 +16,13 @@ import (
 	"github.com/alibaba/open-code-review/internal/tool"
 )
 
+type contextBlockingClient struct{}
+
+func (contextBlockingClient) CompletionsWithCtx(ctx context.Context, _ llm.ChatRequest) (*llm.ChatResponse, error) {
+	<-ctx.Done()
+	return nil, ctx.Err()
+}
+
 func TestAgent_Getters(t *testing.T) {
 	tmpDir := t.TempDir()
 	sess := session.New(tmpDir, "main", "test-model", session.SessionOptions{ReviewMode: "diff"})
@@ -414,6 +422,31 @@ func TestExecutePlanPhase_LLMError(t *testing.T) {
 	}
 }
 
+func TestExecutePlanPhase_Timeout(t *testing.T) {
+	tmpDir := t.TempDir()
+	sess := session.New(tmpDir, "main", "test", session.SessionOptions{ReviewMode: "diff"})
+
+	a := New(Args{
+		LLMClient:       contextBlockingClient{},
+		Model:           "test",
+		Session:         sess,
+		PlanTaskTimeout: 20 * time.Millisecond,
+		Template: template.Template{
+			PlanTask: &template.LlmConversation{
+				Messages: []template.ChatMessage{{Role: "user", Content: "{{diff}}"}},
+			},
+			MaxTokens:           10000,
+			MaxToolRequestTimes: 5,
+			MainTask:            template.LlmConversation{Messages: []template.ChatMessage{{Role: "user", Content: "t"}}},
+		},
+	})
+
+	_, err := a.executePlanPhase(context.Background(), "a.go", "+x", "", "")
+	if !errors.Is(err, context.DeadlineExceeded) {
+		t.Fatalf("executePlanPhase error = %v, want context deadline exceeded", err)
+	}
+}
+
 func TestExecuteSubtask_EmptyMainTask(t *testing.T) {
 	tmpDir := t.TempDir()
 	sess := session.New(tmpDir, "main", "test", session.SessionOptions{ReviewMode: "diff"})

internal/config/template/task_template.json

@@ -31,5 +31,5 @@
   },
   "MAX_TOOL_REQUEST_TIMES": 30,
   "PLAN_MODE_LINE_THRESHOLD": 50,
-  "MAX_TOKENS": 58888
+  "MAX_TOKENS": 30000
 }

internal/config/template/template_test.go

@@ -80,8 +80,8 @@ func TestLoadDefault_FieldsPopulated(t *testing.T) {
 	if tpl.ReviewFilterTask == nil {
 		t.Fatal("ReviewFilterTask is nil, expected non-nil")
 	}
-	if tpl.MaxTokens != 58888 {
-		t.Errorf("MaxTokens = %d, want 58888", tpl.MaxTokens)
+	if tpl.MaxTokens != 30000 {
+		t.Errorf("MaxTokens = %d, want 30000", tpl.MaxTokens)
 	}
 	if tpl.MaxToolRequestTimes != 30 {
 		t.Errorf("MaxToolRequestTimes = %d, want 30", tpl.MaxToolRequestTimes)