Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -46,3 +46,4 @@ REPORT.md
# Dolt database files (added by bd init)
.dolt/
*.db
.codex
2 changes: 1 addition & 1 deletion cmd/roborev/analyze.go
Original file line number Diff line number Diff line change
Expand Up @@ -747,7 +747,7 @@ func runFixAgent(cmd *cobra.Command, repoPath, agentName, model, reasoning, prom
}

// Resolve reasoning level (defaults to "standard" for fix)
reasoning, reasonErr := config.ResolveFixReasoning(reasoning, repoPath)
reasoning, reasonErr := config.ResolveFixReasoning(reasoning, repoPath, cfg)
if reasonErr != nil {
return fmt.Errorf("resolve fix reasoning: %w", reasonErr)
}
Expand Down
2 changes: 1 addition & 1 deletion cmd/roborev/compact.go
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,7 @@ func enqueueConsolidation(ctx context.Context, cmd *cobra.Command, repoRoot stri
if err != nil {
return 0, fmt.Errorf("load config: %w", err)
}
reasoning, err := config.ResolveFixReasoning(opts.reasoning, repoRoot)
reasoning, err := config.ResolveFixReasoning(opts.reasoning, repoRoot, cfg)
if err != nil {
return 0, fmt.Errorf("resolve reasoning: %w", err)
}
Expand Down
2 changes: 1 addition & 1 deletion cmd/roborev/fix.go
Original file line number Diff line number Diff line change
Expand Up @@ -319,7 +319,7 @@ func resolveFixAgent(repoPath string, opts fixOptions) (agent.Agent, error) {
return nil, fmt.Errorf("load config: %w", err)
}

reasoning, err := config.ResolveFixReasoning(opts.reasoning, repoPath)
reasoning, err := config.ResolveFixReasoning(opts.reasoning, repoPath, cfg)
if err != nil {
return nil, fmt.Errorf("resolve fix reasoning: %w", err)
}
Expand Down
26 changes: 26 additions & 0 deletions cmd/roborev/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -222,6 +222,32 @@ func TestRunRefineSurfacesResponseErrors(t *testing.T) {
require.Error(t, err, "expected error, got nil")
}

func TestRunRefineFailsOnInvalidGlobalConfigBeforeDaemonStartup(t *testing.T) {
repoDir, _ := setupRefineRepo(t)

dataDir := t.TempDir()
t.Setenv("ROBOREV_DATA_DIR", dataDir)
configPath := filepath.Join(dataDir, "config.toml")
err := os.WriteFile(configPath, []byte("invalid toml [[["),
0o644)
require.NoError(t, err, "write invalid config: %v", err)

origGetAnyRunningDaemon := getAnyRunningDaemon
getAnyRunningDaemon = func() (*daemon.RuntimeInfo, error) {
require.FailNow(t, "ensureDaemon should not run when global config is invalid")
return nil, ErrDaemonNotRunning
}
t.Cleanup(func() {
getAnyRunningDaemon = origGetAnyRunningDaemon
})

ctx := newFastRunContext(repoDir)

err = runRefine(ctx, refineOptions{agentName: "test", maxIterations: 1, quiet: true})
require.Error(t, err, "expected config load error")
assert.Contains(t, err.Error(), "load config:")
}

func TestRunRefineQuietNonTTYTimerOutput(t *testing.T) {
repoDir, headSHA := setupRefineRepo(t)

Expand Down
8 changes: 6 additions & 2 deletions cmd/roborev/refine.go
Original file line number Diff line number Diff line change
Expand Up @@ -321,6 +321,11 @@ func runRefine(ctx RunContext, opts refineOptions) error {
return err
}

cfg, err := config.LoadGlobal()
if err != nil {
return fmt.Errorf("load config: %w", err)
}

// 2. Connect to daemon (only after all validation passes)
if err := ensureDaemon(); err != nil {
return fmt.Errorf("daemon not running: %w", err)
Expand Down Expand Up @@ -348,8 +353,7 @@ func runRefine(ctx RunContext, opts refineOptions) error {
}

// Resolve reasoning level from CLI or config (default: standard for refine)
cfg, _ := config.LoadGlobal()
resolvedReasoning, err := config.ResolveRefineReasoning(opts.reasoning, repoPath)
resolvedReasoning, err := config.ResolveRefineReasoning(opts.reasoning, repoPath, cfg)
if err != nil {
return err
}
Expand Down
2 changes: 1 addition & 1 deletion cmd/roborev/review.go
Original file line number Diff line number Diff line change
Expand Up @@ -373,7 +373,7 @@ func runLocalReview(cmd *cobra.Command, repoPath, gitRef, diffContent, agentName
}

// Resolve and validate reasoning (matches daemon behavior)
reasoning, err = config.ResolveReviewReasoning(reasoning, repoPath)
reasoning, err = config.ResolveReviewReasoning(reasoning, repoPath, cfg)
if err != nil {
return fmt.Errorf("invalid reasoning: %w", err)
}
Expand Down
45 changes: 36 additions & 9 deletions internal/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,9 @@ type Config struct {
DefaultBackupAgent string `toml:"default_backup_agent"`
DefaultBackupModel string `toml:"default_backup_model"`
JobTimeoutMinutes int `toml:"job_timeout_minutes"`
ReviewReasoning string `toml:"review_reasoning" comment:"Default reasoning level for reviews: fast, standard, medium, thorough, or maximum."`
RefineReasoning string `toml:"refine_reasoning" comment:"Default reasoning level for refine: fast, standard, medium, thorough, or maximum."`
FixReasoning string `toml:"fix_reasoning" comment:"Default reasoning level for fix: fast, standard, medium, thorough, or maximum."`

// Workflow-specific agent/model configuration
ReviewAgent string `toml:"review_agent"`
Expand Down Expand Up @@ -1399,44 +1402,68 @@ func SeverityInstruction(minSeverity string) string {
}

// ResolveReviewReasoning determines reasoning level for reviews.
// Priority: explicit > per-repo config > default (thorough)
func ResolveReviewReasoning(explicit string, repoPath string) (string, error) {
// Priority: explicit > per-repo config > global config > default (thorough)
func ResolveReviewReasoning(explicit string, repoPath string, globalCfg *Config) (string, error) {
if strings.TrimSpace(explicit) != "" {
return NormalizeReasoning(explicit)
}

if repoCfg, err := LoadRepoConfig(repoPath); err == nil && repoCfg != nil && strings.TrimSpace(repoCfg.ReviewReasoning) != "" {
repoCfg, err := LoadRepoConfig(repoPath)
if err != nil {
return "", err
}
if repoCfg != nil && strings.TrimSpace(repoCfg.ReviewReasoning) != "" {
return NormalizeReasoning(repoCfg.ReviewReasoning)
}

if globalCfg != nil && strings.TrimSpace(globalCfg.ReviewReasoning) != "" {
return NormalizeReasoning(globalCfg.ReviewReasoning)
}

return "thorough", nil // Default for reviews: deep analysis
}

// ResolveRefineReasoning determines reasoning level for refine.
// Priority: explicit > per-repo config > default (standard)
func ResolveRefineReasoning(explicit string, repoPath string) (string, error) {
// Priority: explicit > per-repo config > global config > default (standard)
func ResolveRefineReasoning(explicit string, repoPath string, globalCfg *Config) (string, error) {
if strings.TrimSpace(explicit) != "" {
return NormalizeReasoning(explicit)
}

if repoCfg, err := LoadRepoConfig(repoPath); err == nil && repoCfg != nil && strings.TrimSpace(repoCfg.RefineReasoning) != "" {
repoCfg, err := LoadRepoConfig(repoPath)
if err != nil {
return "", err
}
if repoCfg != nil && strings.TrimSpace(repoCfg.RefineReasoning) != "" {
return NormalizeReasoning(repoCfg.RefineReasoning)
}

if globalCfg != nil && strings.TrimSpace(globalCfg.RefineReasoning) != "" {
return NormalizeReasoning(globalCfg.RefineReasoning)
}

return "standard", nil // Default for refine: balanced analysis
}

// ResolveFixReasoning determines reasoning level for fix.
// Priority: explicit > per-repo config > default (standard)
func ResolveFixReasoning(explicit string, repoPath string) (string, error) {
// Priority: explicit > per-repo config > global config > default (standard)
func ResolveFixReasoning(explicit string, repoPath string, globalCfg *Config) (string, error) {
if strings.TrimSpace(explicit) != "" {
return NormalizeReasoning(explicit)
}

if repoCfg, err := LoadRepoConfig(repoPath); err == nil && repoCfg != nil && strings.TrimSpace(repoCfg.FixReasoning) != "" {
repoCfg, err := LoadRepoConfig(repoPath)
if err != nil {
return "", err
}
if repoCfg != nil && strings.TrimSpace(repoCfg.FixReasoning) != "" {
return NormalizeReasoning(repoCfg.FixReasoning)
}

if globalCfg != nil && strings.TrimSpace(globalCfg.FixReasoning) != "" {
return NormalizeReasoning(globalCfg.FixReasoning)
}

return "standard", nil // Default for fix: balanced analysis
}

Expand Down
49 changes: 35 additions & 14 deletions internal/config/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -348,29 +348,50 @@ func TestResolveJobTimeout(t *testing.T) {
}

func TestResolveReasoning(t *testing.T) {
type resolverFunc func(explicit string, dir string) (string, error)
type resolverFunc func(explicit string, dir string, globalCfg *Config) (string, error)

runTests := func(t *testing.T, name string, fn resolverFunc, configKey, defaultVal, repoVal string) {
t.Run(name, func(t *testing.T) {
tests := []struct {
testName string
explicit string
repoConfig string
want string
wantErr bool
testName string
explicit string
repoConfig string
globalConfig string
want string
wantErr bool
}{
{"default when no config", "", "", defaultVal, false},
{"repo config when explicit empty", "", fmt.Sprintf(`%s = "%s"`, configKey, repoVal), repoVal, false},
{"explicit overrides repo config", "fast", fmt.Sprintf(`%s = "%s"`, configKey, repoVal), "fast", false},
{"explicit normalization", "FAST", "", "fast", false},
{"invalid explicit", "unknown", "", "", true},
{"invalid repo config", "", fmt.Sprintf(`%s = "invalid"`, configKey), "", true},
{"default when no config", "", "", "", defaultVal, false},
{"global config when explicit and repo empty", "", "", fmt.Sprintf(`%s = "%s"`, configKey, repoVal), repoVal, false},
{"repo config when explicit empty", "", fmt.Sprintf(`%s = "%s"`, configKey, repoVal), "", repoVal, false},
{"repo config overrides global config", "", fmt.Sprintf(`%s = "%s"`, configKey, repoVal), fmt.Sprintf(`%s = "%s"`, configKey, defaultVal), repoVal, false},
{"explicit overrides repo config", "fast", fmt.Sprintf(`%s = "%s"`, configKey, repoVal), fmt.Sprintf(`%s = "%s"`, configKey, defaultVal), "fast", false},
{"explicit normalization", "FAST", "", "", "fast", false},
{"invalid explicit", "unknown", "", "", "", true},
{"invalid repo config", "", fmt.Sprintf(`%s = "invalid"`, configKey), "", "", true},
{"malformed repo config does not fall back to global", "", fmt.Sprintf(`%s = [`, configKey), fmt.Sprintf(`%s = "%s"`, configKey, repoVal), "", true},
{"invalid global config", "", "", fmt.Sprintf(`%s = "invalid"`, configKey), "", true},
}

for _, tt := range tests {
t.Run(tt.testName, func(t *testing.T) {
dataDir := t.TempDir()
t.Setenv("ROBOREV_DATA_DIR", dataDir)
if tt.globalConfig != "" {
err := os.WriteFile(
filepath.Join(dataDir, "config.toml"),
[]byte(tt.globalConfig),
0o644,
)
require.NoError(t, err)
}
var globalCfg *Config
if tt.globalConfig != "" {
cfg, loadErr := LoadGlobalFrom(filepath.Join(dataDir, "config.toml"))
require.NoError(t, loadErr)
globalCfg = cfg
}
tmpDir := newTempRepo(t, tt.repoConfig)
got, err := fn(tt.explicit, tmpDir)
got, err := fn(tt.explicit, tmpDir, globalCfg)
if (err != nil) != tt.wantErr {
assert.Condition(t, func() bool {
return false
Expand Down Expand Up @@ -401,7 +422,7 @@ func TestFixEmptyReasoningSelectsStandardAgent(t *testing.T) {
"fix_agent_fast": "gemini",
})

reasoning, err := ResolveFixReasoning("", tmpDir)
reasoning, err := ResolveFixReasoning("", tmpDir, nil)
require.Condition(t, func() bool {
return err == nil
}, "ResolveFixReasoning: %v", err)
Expand Down
6 changes: 3 additions & 3 deletions internal/daemon/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -859,15 +859,16 @@ func (s *Server) handleEnqueue(w http.ResponseWriter, r *http.Request) {
}

workflow := workflowForJob(req.JobType, req.ReviewType)
cfg := s.configWatcher.Config()

// Resolve reasoning level for the determined workflow.
// Compact jobs use fix reasoning (default "standard"), not review
// reasoning (default "thorough").
var reasoning string
if workflow == "fix" {
reasoning, err = config.ResolveFixReasoning(req.Reasoning, repoRoot)
reasoning, err = config.ResolveFixReasoning(req.Reasoning, repoRoot, cfg)
} else {
reasoning, err = config.ResolveReviewReasoning(req.Reasoning, repoRoot)
reasoning, err = config.ResolveReviewReasoning(req.Reasoning, repoRoot, cfg)
}
if err != nil {
writeError(w, http.StatusBadRequest, err.Error())
Expand All @@ -878,7 +879,6 @@ func (s *Server) handleEnqueue(w http.ResponseWriter, r *http.Request) {
requestedProvider := strings.TrimSpace(req.Provider)

// Resolve agent for workflow at this reasoning level
cfg := s.configWatcher.Config()
resolution := agent.ResolveWorkflowConfig(
req.Agent, repoRoot, cfg, workflow, reasoning,
)
Expand Down
45 changes: 45 additions & 0 deletions internal/daemon/server_jobs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"github.com/roborev-dev/roborev/internal/config"
gitpkg "github.com/roborev-dev/roborev/internal/git"
"github.com/roborev-dev/roborev/internal/storage"
"github.com/roborev-dev/roborev/internal/testenv"
"github.com/roborev-dev/roborev/internal/testutil"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
Expand Down Expand Up @@ -2693,6 +2694,50 @@ func TestHandleEnqueueCompactReasoning(t *testing.T) {
}
}

func TestHandleEnqueueUsesConfiguredReviewReasoning(t *testing.T) {
_ = testenv.SetDataDir(t)

configPath := filepath.Join(t.TempDir(), "daemon-config.toml")
err := os.WriteFile(
configPath,
[]byte(`review_reasoning = "maximum"`),
0o644,
)
require.NoError(t, err)

repoDir := t.TempDir()
testutil.InitTestGitRepo(t, repoDir)

db, _ := testutil.OpenTestDBWithDir(t)
cfg, err := config.LoadGlobalFrom(configPath)
require.NoError(t, err)
server := NewServer(db, cfg, configPath)
t.Cleanup(func() {
require.NoError(t, server.Close())
})

reqData := EnqueueRequest{
RepoPath: repoDir,
GitRef: "HEAD",
Agent: "test",
}
req := testutil.MakeJSONRequest(
t, http.MethodPost, "/api/enqueue", reqData,
)
w := httptest.NewRecorder()
server.handleEnqueue(w, req)

if w.Code != http.StatusCreated {
require.Condition(t, func() bool {
return false
}, "expected 201, got %d: %s", w.Code, w.Body.String())
}

var job storage.ReviewJob
testutil.DecodeJSON(t, w, &job)
assert.Equal(t, "maximum", job.Reasoning)
}

func TestHandleListJobsSlashNormalization(t *testing.T) {
server, db, tmpDir := newTestServer(t)

Expand Down
Loading