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
9 changes: 5 additions & 4 deletions docs/architecture.md
Original file line number Diff line number Diff line change
Expand Up @@ -58,15 +58,16 @@ review stages. Runtime hard-coding bypasses user preference and is a bug.

For reviewer tier-based requests, the resolver's authoritative ordering is:
resolve the effective tier after applying the profile reviewer-tier floor and
agent floor; resolve the model and its built-in effort preset, if any, for that
tier; cap the selected effort with the selected runtime's `max_effort` entry for
that final tier; then apply
agent floor; resolve the model for that tier; select the runtime `effort_map`
entry for that tier (falling back to built-in presets for built-in models, then agent/stage effort); cap that
effort with the selected runtime's `max_effort` entry for that final tier; then apply
`EffortOverride`. This means `--reviewer-model-tier` is still capped at the tier
it ultimately resolves, while `--selection-effort` and `--reviewer-effort` win
after the ceiling. Other tier-resolved internal stages use their own stage
tier before applying `max_effort` at that final tier.

An explicit `ModelOverride` bypasses model-map resolution. Explicit effort
An explicit `ModelOverride` bypasses model-map resolution, but a request with
a tier still uses its independent `effort_map` preference. Explicit effort
still bypasses `max_effort`; inherited reviewer effort is capped when the
request carries an effective reviewer tier. `--reviewer-model` uses that tier,
including benchmark reviewer model overrides. `--selection-model` and agent
Expand Down
41 changes: 41 additions & 0 deletions docs/init-config-surface.md
Original file line number Diff line number Diff line change
Expand Up @@ -276,3 +276,44 @@ The following flags are intentionally not durable init configuration.
- #185: interactive repository routes and host reconciliation.
- #186: scripted installer documentation.
- #187: maintainable non-interactive init parity flags.

## Independent Reasoning Effort Preferences

`llm_runtimes.<name>.effort_map` selects reasoning effort independently of
`model_map`. It uses the same tier keys and runtime effort validation as
`max_effort`. A configured preference replaces the agent/stage effort before
applying the ceiling; an explicit CLI effort override wins over both. Exact
model overrides with no tier retain the agent/stage effort.

```yaml
model_map:
small: gpt-6-luna
medium: gpt-6.1-sol
large: gpt-6.1-sol
effort_map:
small: max
medium: low
large: medium
```

Inspect with `cr config llm efforts list --json`, set an entry with
`cr config llm efforts set small max`, or remove it with
`cr config llm efforts unset small`. These commands edit the selected profile's
shared LLM runtime, so other profiles referencing it see the same changes.
Both interactive and non-interactive init preserve the map.

The first normal live `cr review` for a Codex CLI runtime automatically sets
small to `gpt-6-luna` / `max`, medium to `gpt-6.1-sol` / `low`, and large to
`gpt-6.1-sol` / `medium`. It removes the old effort ceilings, saves the original
config beside `config.yml` as `config.yml.before-review-defaults-v1`, and saves
`review_defaults_version: 1` on the shared runtime. A one-time stderr notice
reports the changes and the model/effort edit commands before runtime startup.
The notice stays off JSON stdout and is shown even with `--quiet`.

The version marker survives init, so later model pins, effort edits, unsets,
and ceilings are preserved. Other profiles using that runtime share the upgrade.
Dry-run/no-post, posting recovery, other commands, and non-Codex runtimes do not
trigger it. A backup or save failure stops before starting the review; the next
invocation can retry the upgrade.

Config saves serialize writes and reject stale loaded drafts with a retry message, so a concurrent edit cannot undo migration or be erased by it. The automatic migration reloads and retries when another config edit wins.
10 changes: 8 additions & 2 deletions internal/cmd/configcmd/configcmd.go
Original file line number Diff line number Diff line change
Expand Up @@ -715,7 +715,7 @@ func newLLMCommand(opts *root.Options) *cobra.Command {
root.AddJSONFlag(resolveCmd, &resolveJSON)

modelsCmd.AddCommand(listCmd, setCmd, unsetCmd, resetCmd, resolveCmd)
llmCmd.AddCommand(modelsCmd)
llmCmd.AddCommand(modelsCmd, newEffortsCommand(opts))
return llmCmd
}

Expand Down Expand Up @@ -890,6 +890,12 @@ func modelMapResult(profileName string, profile config.Profile) modelMapResultVi
}

func mutateActiveModelMap(opts *root.Options, mutate func(config.Profile, *config.ModelMap) error) (string, error) {
return mutateActiveLLM(opts, func(profile config.Profile, runtime *config.LLMConfig) error {
return mutate(profile, &runtime.ModelMap)
})
}

func mutateActiveLLM(opts *root.Options, mutate func(config.Profile, *config.LLMConfig) error) (string, error) {
path, cfg, profileName, profile, err := loadActiveProfile(opts)
if err != nil {
return "", err
Expand All @@ -898,7 +904,7 @@ func mutateActiveModelMap(opts *root.Options, mutate func(config.Profile, *confi
if err != nil {
return "", cmderr.Config(err)
}
if err := mutate(profile, &runtime.ModelMap); err != nil {
if err := mutate(profile, &runtime); err != nil {
return "", err
}
cfg.LLMRuntimes[runtimeName] = runtime
Expand Down
10 changes: 9 additions & 1 deletion internal/cmd/configcmd/configcmd_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1864,7 +1864,15 @@ func TestConfigAgentSourcePreservesUnrelatedProfileFields(t *testing.T) {
wantHome := want.Profiles["home"]
wantHome.AgentSources = []string{"home-agents", "team/agents"}
want.Profiles["home"] = wantHome
if !reflect.DeepEqual(cfg, want) {
cfgJSON, err := json.Marshal(cfg)
if err != nil {
t.Fatal(err)
}
wantJSON, err := json.Marshal(want)
if err != nil {
t.Fatal(err)
}
if !bytes.Equal(cfgJSON, wantJSON) {
t.Fatalf("config changed unexpectedly:\n got %#v\nwant %#v", cfg, want)
}
}
Expand Down
101 changes: 101 additions & 0 deletions internal/cmd/configcmd/efforts.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,101 @@
package configcmd

import (
"fmt"
"io"
"strings"

"github.com/spf13/cobra"

"github.com/open-cli-collective/codereview-cli/internal/cmd/exitcode"
"github.com/open-cli-collective/codereview-cli/internal/cmd/root"
"github.com/open-cli-collective/codereview-cli/internal/config"
"github.com/open-cli-collective/codereview-cli/internal/view"
)

func newEffortsCommand(opts *root.Options) *cobra.Command {
cmd := &cobra.Command{Use: "efforts", Short: "Inspect and update reasoning effort tier mappings"}
var asJSON bool
list := &cobra.Command{
Use: "list", Short: "List configured reasoning efforts and ceilings",
Args: exitcode.NoArgs("config llm efforts list takes no arguments"),
RunE: func(_ *cobra.Command, _ []string) error {
_, _, name, profile, err := loadActiveProfile(opts)
if err != nil {
return err
}
result := struct {
ActiveProfile string `json:"active_profile"`
EffortMap config.EffortMap `json:"effort_map"`
MaxEffort config.EffortMap `json:"max_effort"`
}{name, profile.LLM.EffortMap, profile.LLM.MaxEffort}
return view.Render(opts.Stdout, asJSON, result, func(w io.Writer) error {
for _, tier := range config.ModelTiers() {
effort := profile.LLM.EffortMap[string(tier)]
if effort == "" {
effort = "built-in or agent/stage default"
}
ceiling := profile.LLM.MaxEffort[string(tier)]
if ceiling == "" {
ceiling = "uncapped"
}
if _, err := fmt.Fprintf(w, "%s: %s (max: %s)\n", tier, effort, ceiling); err != nil {
return err
}
}
return nil
})
},
}
root.AddJSONFlag(list, &asJSON)
set := &cobra.Command{
Use: "set <tier> <effort>", Short: "Set reasoning effort independently of the model",
Args: exitcode.ExactArgs(2, "config llm efforts set requires <tier> and <effort>"),
RunE: func(_ *cobra.Command, args []string) error {
tier, err := parseModelTierArg(args[0])
if err != nil {
return err
}
effort := strings.TrimSpace(args[1])
_, err = mutateActiveLLM(opts, func(_ config.Profile, runtime *config.LLMConfig) error {
if err := config.ValidateEffortForRuntime(*runtime, effort); err != nil {
return exitcode.Usage(err)
}
if effort == "" {
return exitcode.Usage(fmt.Errorf("effort must be non-empty"))
}
if runtime.EffortMap == nil {
runtime.EffortMap = config.EffortMap{}
}
runtime.EffortMap[string(tier)] = effort
return nil
})
if err != nil {
return err
}
_, err = fmt.Fprintf(opts.Stdout, "Set %s effort: %s\n", tier, effort)
return err
},
}
unset := &cobra.Command{
Use: "unset <tier>", Short: "Restore default reasoning effort for a tier",
Args: exitcode.ExactArgs(1, "config llm efforts unset requires <tier>"),
RunE: func(_ *cobra.Command, args []string) error {
tier, err := parseModelTierArg(args[0])
if err != nil {
return err
}
_, err = mutateActiveLLM(opts, func(_ config.Profile, runtime *config.LLMConfig) error {
delete(runtime.EffortMap, string(tier))
return nil
})
if err != nil {
return err
}
_, err = fmt.Fprintf(opts.Stdout, "Unset %s effort\n", tier)
return err
},
}
cmd.AddCommand(list, set, unset)
return cmd
}
100 changes: 100 additions & 0 deletions internal/cmd/configcmd/efforts_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,100 @@
package configcmd

import (
"errors"
"strings"
"testing"

"github.com/open-cli-collective/codereview-cli/internal/cmd/root"
"github.com/open-cli-collective/codereview-cli/internal/config"
)

func TestEffortCommandCannotUndoConcurrentMigration(t *testing.T) {
cfg := config.Normalize(testConfig())
runtimeName := cfg.Profiles["home"].LLMRuntime
cfg.LLMRuntimes[runtimeName] = config.LLMConfig{Provider: config.LLMProviderOpenAI,
Auth: config.LLMAuthSubscription, Adapter: config.LLMAdapterCodexCLI}
path := saveTestConfig(t, cfg)
previousSave := saveConfigFile
t.Cleanup(func() { saveConfigFile = previousSave })
saveConfigFile = func(path string, draft config.File) error {
// Interleave migration after the command loads its draft, before its save.
latest, err := config.Load(path)
if err != nil {
return err
}
upgraded, _ := config.UpgradeReviewDefaults(latest, runtimeName)
if err := config.Save(path, upgraded); err != nil {
return err
}
return config.Save(path, draft)
}
cmd, _ := newTestCommand(path)
err := root.Execute(cmd, []string{"--profile", "home", "config", "llm", "efforts", "set", "small", "high"})
if !errors.Is(err, config.ErrChanged) {
t.Fatalf("stale command must fail safely: %v", err)
}
saveConfigFile = previousSave
cmd, _ = newTestCommand(path)
if err := root.Execute(cmd, []string{"--profile", "home", "config", "llm", "efforts", "set", "small", "high"}); err != nil {
t.Fatal(err)
}
latest, err := config.Load(path)
if err != nil {
t.Fatal(err)
}
llm := latest.LLMRuntimes[runtimeName]
if llm.DefaultsVersion != 1 || llm.ModelMap["medium"] != "gpt-6.1-sol" || llm.EffortMap["small"] != "high" {
t.Fatalf("retry must preserve migration and customization: %#v", llm)
}
}

func TestEffortCommandsPreserveModelsAndSharedRuntime(t *testing.T) {
cfg := config.Normalize(testConfig())
home := cfg.Profiles["home"]
llm := config.LLMConfig{Provider: config.LLMProviderOpenAI, Auth: config.LLMAuthSubscription,
Adapter: config.LLMAdapterCodexCLI, ModelMap: config.ModelMap{"small": "gpt-6-luna"},
MaxEffort: config.EffortMap{"large": "medium"}}
cfg.LLMRuntimes[home.LLMRuntime] = llm
home.LLM = llm
cfg.Profiles["home"] = home
cfg.Profiles["shared"] = home
path := saveTestConfig(t, cfg)
for _, action := range []string{"set", "unset"} {
args := []string{"--profile", "home", "config", "llm", "efforts", action, "small"}
if action == "set" {
args = append(args, "max")
}
cmd, _ := newTestCommand(path)
if err := root.Execute(cmd, args); err != nil {
t.Fatalf("%s: %v", action, err)
}
loaded, err := config.Load(path)
if err != nil {
t.Fatal(err)
}
want := ""
if action == "set" {
want = "max"
}
for _, name := range []string{"home", "shared"} {
got := loaded.Profiles[name].LLM
if got.EffortMap["small"] != want || got.ModelMap["small"] != "gpt-6-luna" || got.MaxEffort["large"] != "medium" {
t.Fatalf("%s after %s: %#v", name, action, got)
}
}
cmd, out := newTestCommand(path)
if err := root.Execute(cmd, []string{"--profile", "home", "config", "llm", "efforts", "list", "--json"}); err != nil {
t.Fatal(err)
}
if !strings.Contains(out.String(), `"effort_map"`) || !strings.Contains(out.String(), `"max_effort"`) {
t.Fatalf("list: %s", out)
}
}
for _, args := range [][]string{{"small", "ultra"}, {"huge", "max"}, {"small", ""}} {
cmd, _ := newTestCommand(path)
if err := root.Execute(cmd, append([]string{"--profile", "home", "config", "llm", "efforts", "set"}, args...)); err == nil {
t.Fatalf("accepted invalid setting: %v", args)
}
}
}
Loading
Loading