diff --git a/coderd/exp_chats.go b/coderd/exp_chats.go index 032e1518d2..03aa39c1ee 100644 --- a/coderd/exp_chats.go +++ b/coderd/exp_chats.go @@ -5104,14 +5104,6 @@ func (api *API) putChatAdvisorConfig(rw http.ResponseWriter, r *http.Request) { }) return } - switch req.ReasoningEffort { - case "", "low", "medium", "high": - default: - httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ - Message: fmt.Sprintf(`reasoning_effort %q is not valid; must be one of "", "low", "medium", or "high".`, req.ReasoningEffort), - }) - return - } if req.ModelConfigID != uuid.Nil { // Use system context because GetChatModelConfigByID requires // deployment-config read access, which can be broader than the diff --git a/coderd/exp_chats_test.go b/coderd/exp_chats_test.go index b68b0a0d5b..560653fe75 100644 --- a/coderd/exp_chats_test.go +++ b/coderd/exp_chats_test.go @@ -11876,7 +11876,6 @@ func TestChatAdvisorConfig_Update(t *testing.T) { Enabled: true, MaxUsesPerRun: 5, MaxOutputTokens: 1024, - ReasoningEffort: "high", } err := adminClient.UpdateChatAdvisorConfig(ctx, want) @@ -11974,7 +11973,6 @@ func TestChatAdvisorConfig_RoundTripModelConfigID(t *testing.T) { MaxUsesPerRun: 3, MaxOutputTokens: 2048, ModelConfigID: modelConfig.ID, - ReasoningEffort: "medium", } err := adminClient.UpdateChatAdvisorConfig(ctx, want) @@ -11985,21 +11983,6 @@ func TestChatAdvisorConfig_RoundTripModelConfigID(t *testing.T) { require.Equal(t, want, resp) } -func TestChatAdvisorConfig_InvalidReasoningEffort(t *testing.T) { - t.Parallel() - - ctx := testutil.Context(t, testutil.WaitLong) - adminClient := newChatClient(t) - coderdtest.CreateFirstUser(t, adminClient.Client) - - err := adminClient.UpdateChatAdvisorConfig(ctx, codersdk.UpdateAdvisorConfigRequest{ - ReasoningEffort: "ultra", - }) - sdkErr := requireSDKError(t, err, http.StatusBadRequest) - require.Contains(t, sdkErr.Message, `reasoning_effort "ultra"`) - require.Contains(t, sdkErr.Message, "not valid") -} - func TestChatAdvisorConfig_InvalidModelConfigID(t *testing.T) { t.Parallel() @@ -12055,7 +12038,6 @@ func TestChatAdvisorConfig_OverwriteClearsPreviousValues(t *testing.T) { MaxUsesPerRun: 5, MaxOutputTokens: 1024, ModelConfigID: modelConfig.ID, - ReasoningEffort: "high", } err := adminClient.UpdateChatAdvisorConfig(ctx, rich) require.NoError(t, err) @@ -12124,6 +12106,30 @@ func TestChatAdvisorConfig_ClampsNegativeStoredValues(t *testing.T) { require.JSONEq(t, stored, raw) } +func TestChatAdvisorConfig_IgnoresLegacyReasoningEffort(t *testing.T) { + t.Parallel() + + ctx := testutil.Context(t, testutil.WaitLong) + adminClient, db := newChatClientWithDatabase(t) + coderdtest.CreateFirstUser(t, adminClient.Client) + + stored := `{"enabled":true,"max_uses_per_run":3,"max_output_tokens":2048,"reasoning_effort":"high"}` + err := db.UpsertChatAdvisorConfig(dbauthz.AsSystemRestricted(ctx), stored) + require.NoError(t, err) + + resp, err := adminClient.GetChatAdvisorConfig(ctx) + require.NoError(t, err) + require.Equal(t, codersdk.AdvisorConfig{ + Enabled: true, + MaxUsesPerRun: 3, + MaxOutputTokens: 2048, + }, resp) + + raw, err := db.GetChatAdvisorConfig(dbauthz.AsSystemRestricted(ctx)) + require.NoError(t, err) + require.JSONEq(t, stored, raw) +} + // TestChatAdvisorConfig_CorruptStoredJSONReturnsError pins that the GET // handler surfaces a 500 when the stored site_configs row contains bytes // that are not valid JSON. Unlike the neighboring chat config endpoints, diff --git a/coderd/x/chatd/advisor_internal_test.go b/coderd/x/chatd/advisor_internal_test.go index f290d3ccf2..ad81e580b2 100644 --- a/coderd/x/chatd/advisor_internal_test.go +++ b/coderd/x/chatd/advisor_internal_test.go @@ -8,7 +8,6 @@ import ( "time" "charm.land/fantasy" - fantasyopenai "charm.land/fantasy/providers/openai" "github.com/google/uuid" "github.com/stretchr/testify/require" "golang.org/x/xerrors" @@ -412,48 +411,4 @@ func TestNewAdvisorRuntime(t *testing.T) { require.Equal(t, int64(defaultAdvisorMaxOutputTokens), rt.MaxOutputTokens(), "zero max output tokens must be replaced with defaultAdvisorMaxOutputTokens") }) - - // Guards the wiring from AdvisorConfig.ReasoningEffort through - // newAdvisorRuntime to ApplyReasoningEffortToOptions. A field swap, - // typo, or accidental deletion of the apply call would otherwise - // ship silently because chatprovider_test only covers the helper in - // isolation. - t.Run("ReasoningEffortReachesProviderOptions", func(t *testing.T) { - t.Parallel() - ctx := testutil.Context(t, testutil.WaitShort) - store := &advisorOverrideStubStore{} - p := newAdvisorTestServer(ctx, t, store) - - openAIModel := &chattest.FakeModel{ - ProviderName: fantasyopenai.Name, - ModelName: "gpt-4", - } - - rt := p.newAdvisorRuntime( - ctx, - database.Chat{}, - codersdk.AdvisorConfig{ - Enabled: true, - MaxUsesPerRun: 3, - MaxOutputTokens: 16384, - ReasoningEffort: "high", - }, - openAIModel, - fallbackCallConfig, - chatprovider.ProviderAPIKeys{}, - logger, - ) - require.NotNil(t, rt) - - providerOptions := rt.ProviderOptions() - require.NotNil(t, providerOptions, - "advisor runtime must seed provider options when reasoning effort is set") - opts, ok := providerOptions[fantasyopenai.Name].(*fantasyopenai.ResponsesProviderOptions) - require.True(t, ok, - "expected *ResponsesProviderOptions for Responses model, got %T", - providerOptions[fantasyopenai.Name]) - require.NotNil(t, opts.ReasoningEffort, - "ReasoningEffort from AdvisorConfig must reach the provider options") - require.Equal(t, fantasyopenai.ReasoningEffortHigh, *opts.ReasoningEffort) - }) } diff --git a/coderd/x/chatd/chatd.go b/coderd/x/chatd/chatd.go index 9338dda90f..445023e8fb 100644 --- a/coderd/x/chatd/chatd.go +++ b/coderd/x/chatd/chatd.go @@ -395,16 +395,6 @@ func (p *Server) newAdvisorRuntime( advisorModel, advisorCallConfig.ProviderOptions, ) - // ProviderOptionsFromChatModelConfig returns nil when the model config - // has no provider_options block, so the helper seeds a minimal entry - // for the advisor model's provider before applying reasoning_effort. - // This keeps the per-provider dispatch in chatprovider so adding a new - // provider there propagates here automatically. - providerOptions = chatprovider.ApplyReasoningEffortToOptions( - providerOptions, - advisorModel, - advisorCfg.ReasoningEffort, - ) rt, err := chatadvisor.NewRuntime(chatadvisor.RuntimeConfig{ Model: advisorModel, diff --git a/coderd/x/chatd/chatprovider/chatprovider.go b/coderd/x/chatd/chatprovider/chatprovider.go index 6c019abcb2..109b9c8551 100644 --- a/coderd/x/chatd/chatprovider/chatprovider.go +++ b/coderd/x/chatd/chatprovider/chatprovider.go @@ -701,145 +701,6 @@ func ReasoningEffortFromChat(provider string, value *string) *string { } } -// ApplyReasoningEffortToOptions applies the given reasoning_effort to every -// provider entry in providerOptions that understands it. When model is -// non-nil and the options map has no entry for the model's provider, this -// function seeds a minimal provider-specific options struct so the mutation -// still lands. Callers that produced providerOptions from a chat model -// config with no provider_options block would otherwise see -// reasoning_effort silently dropped. -// -// The returned map is the (possibly newly-allocated) providerOptions; the -// input is mutated in-place when non-nil. -func ApplyReasoningEffortToOptions( - providerOptions fantasy.ProviderOptions, - model fantasy.LanguageModel, - reasoningEffort string, -) fantasy.ProviderOptions { - reasoningEffort = strings.TrimSpace(reasoningEffort) - if reasoningEffort == "" { - return providerOptions - } - - if model != nil { - providerOptions = seedProviderOptionsForModel(providerOptions, model) - } - if providerOptions == nil { - return nil - } - - applyReasoningEffortDispatch(providerOptions, reasoningEffort) - return providerOptions -} - -// seedProviderOptionsForModel ensures providerOptions has an entry for the -// given model's provider, allocating a minimal options struct when absent. -// Returns the possibly newly-allocated options map. Unknown providers are -// left untouched so callers get their input back unchanged. -func seedProviderOptionsForModel( - providerOptions fantasy.ProviderOptions, - model fantasy.LanguageModel, -) fantasy.ProviderOptions { - provider := model.Provider() - var seed fantasy.ProviderOptionsData - switch provider { - case fantasyopenai.Name: - if fantasyopenai.IsResponsesModel(model.Model()) { - seed = &fantasyopenai.ResponsesProviderOptions{} - } else { - seed = &fantasyopenai.ProviderOptions{} - } - case fantasyanthropic.Name: - seed = &fantasyanthropic.ProviderOptions{} - case fantasyopenaicompat.Name: - seed = &fantasyopenaicompat.ProviderOptions{} - case fantasyopenrouter.Name: - seed = &fantasyopenrouter.ProviderOptions{} - case fantasyvercel.Name: - seed = &fantasyvercel.ProviderOptions{} - default: - return providerOptions - } - - if providerOptions == nil { - providerOptions = fantasy.ProviderOptions{} - } - if _, ok := providerOptions[provider]; !ok { - providerOptions[provider] = seed - } - return providerOptions -} - -// applyReasoningEffortDispatch routes the normalized reasoning_effort to -// every provider entry present in providerOptions. Adding a new provider -// here (and only here) keeps chatd callers in sync automatically. -func applyReasoningEffortDispatch( - providerOptions fantasy.ProviderOptions, - reasoningEffort string, -) { - if normalized := ReasoningEffortFromChat( - fantasyopenai.Name, - &reasoningEffort, - ); normalized != nil { - effort := fantasyopenai.ReasoningEffort(*normalized) - if raw, ok := providerOptions[fantasyopenai.Name]; ok { - switch opts := raw.(type) { - case *fantasyopenai.ProviderOptions: - opts.ReasoningEffort = &effort - case *fantasyopenai.ResponsesProviderOptions: - opts.ReasoningEffort = &effort - } - } - if raw, ok := providerOptions[fantasyopenaicompat.Name]; ok { - if opts, ok := raw.(*fantasyopenaicompat.ProviderOptions); ok { - opts.ReasoningEffort = &effort - } - } - } - - if normalized := ReasoningEffortFromChat( - fantasyanthropic.Name, - &reasoningEffort, - ); normalized != nil { - if raw, ok := providerOptions[fantasyanthropic.Name]; ok { - if opts, ok := raw.(*fantasyanthropic.ProviderOptions); ok { - effort := fantasyanthropic.Effort(*normalized) - opts.Effort = &effort - } - } - } - - if normalized := ReasoningEffortFromChat( - fantasyopenrouter.Name, - &reasoningEffort, - ); normalized != nil { - if raw, ok := providerOptions[fantasyopenrouter.Name]; ok { - if opts, ok := raw.(*fantasyopenrouter.ProviderOptions); ok { - if opts.Reasoning == nil { - opts.Reasoning = &fantasyopenrouter.ReasoningOptions{} - } - effort := fantasyopenrouter.ReasoningEffort(*normalized) - opts.Reasoning.Effort = &effort - } - } - } - - if normalized := ReasoningEffortFromChat( - fantasyvercel.Name, - &reasoningEffort, - ); normalized != nil { - if raw, ok := providerOptions[fantasyvercel.Name]; ok { - if opts, ok := raw.(*fantasyvercel.ProviderOptions); ok { - if opts.Reasoning == nil { - opts.Reasoning = &fantasyvercel.ReasoningOptions{} - } - effort := fantasyvercel.ReasoningEffort(*normalized) - opts.Reasoning.Effort = &effort - } - } - } -} - // MergeMissingModelCostConfig fills unset pricing metadata from defaults. func MergeMissingModelCostConfig( dst **codersdk.ModelCostConfig, diff --git a/coderd/x/chatd/chatprovider/chatprovider_test.go b/coderd/x/chatd/chatprovider/chatprovider_test.go index 7584386bdc..01e403be6f 100644 --- a/coderd/x/chatd/chatprovider/chatprovider_test.go +++ b/coderd/x/chatd/chatprovider/chatprovider_test.go @@ -11,7 +11,6 @@ import ( fantasyanthropic "charm.land/fantasy/providers/anthropic" fantasybedrock "charm.land/fantasy/providers/bedrock" fantasyopenai "charm.land/fantasy/providers/openai" - fantasyopenaicompat "charm.land/fantasy/providers/openaicompat" fantasyopenrouter "charm.land/fantasy/providers/openrouter" fantasyvercel "charm.land/fantasy/providers/vercel" "github.com/google/uuid" @@ -1392,212 +1391,3 @@ func TestMergeMissingProviderOptions_OpenRouterNested(t *testing.T) { require.Equal(t, []string{"int8"}, options.OpenRouter.Provider.Quantizations) require.Equal(t, "latency", *options.OpenRouter.Provider.Sort) } - -// TestApplyReasoningEffortToOptions covers every provider's mutation branch -// plus the seeding path for missing provider entries. A typo or wrong type -// assertion in any branch fails a unit test here rather than silently -// dropping the admin-configured reasoning effort in chatd callers. -func TestApplyReasoningEffortToOptions(t *testing.T) { - t.Parallel() - - t.Run("NilOptionsAndNilModelIsNoOp", func(t *testing.T) { - t.Parallel() - // Must not panic when options and model are both nil. - got := chatprovider.ApplyReasoningEffortToOptions(nil, nil, "medium") - require.Nil(t, got) - }) - - t.Run("EmptyEffortReturnsInputUnchanged", func(t *testing.T) { - t.Parallel() - model := &chattest.FakeModel{ProviderName: fantasyopenai.Name, ModelName: "gpt-4"} - got := chatprovider.ApplyReasoningEffortToOptions(nil, model, " ") - require.Nil(t, got) - }) - - t.Run("EmptyEffortPreservesExistingOptions", func(t *testing.T) { - t.Parallel() - effort := fantasyopenai.ReasoningEffortLow - opts := &fantasyopenai.ProviderOptions{ReasoningEffort: &effort} - providerOptions := fantasy.ProviderOptions{fantasyopenai.Name: opts} - - got := chatprovider.ApplyReasoningEffortToOptions(providerOptions, nil, "") - require.NotNil(t, opts.ReasoningEffort) - require.Equal(t, fantasyopenai.ReasoningEffortLow, *opts.ReasoningEffort) - // The input map must be returned untouched rather than allocated anew. - require.Len(t, got, 1) - }) - - t.Run("UnrecognizedEffortLeavesOptionsUntouched", func(t *testing.T) { - t.Parallel() - opts := &fantasyopenai.ProviderOptions{} - providerOptions := fantasy.ProviderOptions{fantasyopenai.Name: opts} - - chatprovider.ApplyReasoningEffortToOptions(providerOptions, nil, "not-a-real-effort") - require.Nil(t, opts.ReasoningEffort) - }) - - t.Run("OpenAIProviderOptions", func(t *testing.T) { - t.Parallel() - opts := &fantasyopenai.ProviderOptions{} - providerOptions := fantasy.ProviderOptions{fantasyopenai.Name: opts} - - chatprovider.ApplyReasoningEffortToOptions(providerOptions, nil, "medium") - require.NotNil(t, opts.ReasoningEffort) - require.Equal(t, fantasyopenai.ReasoningEffortMedium, *opts.ReasoningEffort) - }) - - t.Run("OpenAIResponsesProviderOptions", func(t *testing.T) { - t.Parallel() - opts := &fantasyopenai.ResponsesProviderOptions{} - providerOptions := fantasy.ProviderOptions{fantasyopenai.Name: opts} - - chatprovider.ApplyReasoningEffortToOptions(providerOptions, nil, "medium") - require.NotNil(t, opts.ReasoningEffort) - require.Equal(t, fantasyopenai.ReasoningEffortMedium, *opts.ReasoningEffort) - }) - - t.Run("OpenAICompatProviderOptions", func(t *testing.T) { - t.Parallel() - opts := &fantasyopenaicompat.ProviderOptions{} - providerOptions := fantasy.ProviderOptions{fantasyopenaicompat.Name: opts} - - chatprovider.ApplyReasoningEffortToOptions(providerOptions, nil, "medium") - require.NotNil(t, opts.ReasoningEffort) - require.Equal(t, fantasyopenai.ReasoningEffortMedium, *opts.ReasoningEffort) - }) - - t.Run("AnthropicProviderOptions", func(t *testing.T) { - t.Parallel() - opts := &fantasyanthropic.ProviderOptions{} - providerOptions := fantasy.ProviderOptions{fantasyanthropic.Name: opts} - - chatprovider.ApplyReasoningEffortToOptions(providerOptions, nil, "high") - require.NotNil(t, opts.Effort) - require.Equal(t, fantasyanthropic.EffortHigh, *opts.Effort) - }) - - t.Run("OpenRouterAllocatesReasoningOptions", func(t *testing.T) { - t.Parallel() - opts := &fantasyopenrouter.ProviderOptions{} - providerOptions := fantasy.ProviderOptions{fantasyopenrouter.Name: opts} - - chatprovider.ApplyReasoningEffortToOptions(providerOptions, nil, "medium") - require.NotNil(t, opts.Reasoning, "Reasoning container must be allocated") - require.NotNil(t, opts.Reasoning.Effort) - require.Equal(t, fantasyopenrouter.ReasoningEffort("medium"), *opts.Reasoning.Effort) - }) - - t.Run("OpenRouterPreservesExistingReasoningContainer", func(t *testing.T) { - t.Parallel() - enabled := true - opts := &fantasyopenrouter.ProviderOptions{ - Reasoning: &fantasyopenrouter.ReasoningOptions{Enabled: &enabled}, - } - providerOptions := fantasy.ProviderOptions{fantasyopenrouter.Name: opts} - - chatprovider.ApplyReasoningEffortToOptions(providerOptions, nil, "high") - require.NotNil(t, opts.Reasoning.Enabled) - require.True(t, *opts.Reasoning.Enabled) - require.NotNil(t, opts.Reasoning.Effort) - require.Equal(t, fantasyopenrouter.ReasoningEffort("high"), *opts.Reasoning.Effort) - }) - - t.Run("VercelAllocatesReasoningOptions", func(t *testing.T) { - t.Parallel() - opts := &fantasyvercel.ProviderOptions{} - providerOptions := fantasy.ProviderOptions{fantasyvercel.Name: opts} - - chatprovider.ApplyReasoningEffortToOptions(providerOptions, nil, "minimal") - require.NotNil(t, opts.Reasoning) - require.NotNil(t, opts.Reasoning.Effort) - require.Equal(t, fantasyvercel.ReasoningEffortMinimal, *opts.Reasoning.Effort) - }) - - t.Run("MultipleProvidersReceiveMutations", func(t *testing.T) { - t.Parallel() - openaiOpts := &fantasyopenai.ProviderOptions{} - anthropicOpts := &fantasyanthropic.ProviderOptions{} - providerOptions := fantasy.ProviderOptions{ - fantasyopenai.Name: openaiOpts, - fantasyanthropic.Name: anthropicOpts, - } - - chatprovider.ApplyReasoningEffortToOptions(providerOptions, nil, "high") - require.NotNil(t, openaiOpts.ReasoningEffort) - require.Equal(t, fantasyopenai.ReasoningEffortHigh, *openaiOpts.ReasoningEffort) - require.NotNil(t, anthropicOpts.Effort) - require.Equal(t, fantasyanthropic.EffortHigh, *anthropicOpts.Effort) - }) - - t.Run("SeedsOpenAICompletionsWhenModelHasNoOptions", func(t *testing.T) { - t.Parallel() - // A model name absent from the Responses allowlist must seed - // the completions options struct so reasoning_effort lands. - model := &chattest.FakeModel{ProviderName: fantasyopenai.Name, ModelName: "not-a-real-openai-model"} - got := chatprovider.ApplyReasoningEffortToOptions(nil, model, "medium") - require.NotNil(t, got) - opts, ok := got[fantasyopenai.Name].(*fantasyopenai.ProviderOptions) - require.True(t, ok, "expected *ProviderOptions for non-Responses model, got %T", got[fantasyopenai.Name]) - require.NotNil(t, opts.ReasoningEffort) - require.Equal(t, fantasyopenai.ReasoningEffortMedium, *opts.ReasoningEffort) - }) - - t.Run("SeedsOpenAIResponsesWhenModelIsResponsesModel", func(t *testing.T) { - t.Parallel() - // A model name in the Responses allowlist must seed the - // Responses-specific options struct so the provider routes to - // the Responses endpoint. - model := &chattest.FakeModel{ProviderName: fantasyopenai.Name, ModelName: "gpt-4"} - got := chatprovider.ApplyReasoningEffortToOptions(nil, model, "medium") - require.NotNil(t, got) - opts, ok := got[fantasyopenai.Name].(*fantasyopenai.ResponsesProviderOptions) - require.True(t, ok, "expected *ResponsesProviderOptions for Responses model, got %T", got[fantasyopenai.Name]) - require.NotNil(t, opts.ReasoningEffort) - require.Equal(t, fantasyopenai.ReasoningEffortMedium, *opts.ReasoningEffort) - }) - - t.Run("SeedsAnthropicWhenModelHasNoOptions", func(t *testing.T) { - t.Parallel() - model := &chattest.FakeModel{ProviderName: fantasyanthropic.Name, ModelName: "claude-3-5"} - got := chatprovider.ApplyReasoningEffortToOptions(nil, model, "high") - require.NotNil(t, got) - opts, ok := got[fantasyanthropic.Name].(*fantasyanthropic.ProviderOptions) - require.True(t, ok) - require.NotNil(t, opts.Effort) - require.Equal(t, fantasyanthropic.EffortHigh, *opts.Effort) - }) - - t.Run("SeedsOpenRouterWhenModelHasNoOptions", func(t *testing.T) { - t.Parallel() - model := &chattest.FakeModel{ProviderName: fantasyopenrouter.Name, ModelName: "openrouter-x"} - got := chatprovider.ApplyReasoningEffortToOptions(nil, model, "low") - require.NotNil(t, got) - opts, ok := got[fantasyopenrouter.Name].(*fantasyopenrouter.ProviderOptions) - require.True(t, ok) - require.NotNil(t, opts.Reasoning) - require.NotNil(t, opts.Reasoning.Effort) - require.Equal(t, fantasyopenrouter.ReasoningEffort("low"), *opts.Reasoning.Effort) - }) - - t.Run("UnknownProviderReturnsInputUnchanged", func(t *testing.T) { - t.Parallel() - model := &chattest.FakeModel{ProviderName: "unknown", ModelName: "x"} - got := chatprovider.ApplyReasoningEffortToOptions(nil, model, "medium") - require.Nil(t, got) - }) - - t.Run("PreservesExistingProviderEntry", func(t *testing.T) { - t.Parallel() - existing := &fantasyopenai.ProviderOptions{} - existingEffort := fantasyopenai.ReasoningEffortLow - existing.ReasoningEffort = &existingEffort - providerOptions := fantasy.ProviderOptions{fantasyopenai.Name: existing} - - model := &chattest.FakeModel{ProviderName: fantasyopenai.Name, ModelName: "gpt-4"} - got := chatprovider.ApplyReasoningEffortToOptions(providerOptions, model, "medium") - require.Same(t, existing, got[fantasyopenai.Name], - "existing provider entry must not be replaced") - // The reasoning effort on the existing entry is overwritten. - require.Equal(t, fantasyopenai.ReasoningEffortMedium, *existing.ReasoningEffort) - }) -} diff --git a/codersdk/chats.go b/codersdk/chats.go index a0eab2868c..4f8c87e8a5 100644 --- a/codersdk/chats.go +++ b/codersdk/chats.go @@ -762,9 +762,6 @@ type AdvisorConfig struct { // resolved (e.g. the referenced model config was soft-deleted or // its provider was disabled after the admin saved this config). ModelConfigID uuid.UUID `json:"model_config_id" format:"uuid"` - // ReasoningEffort overlays provider reasoning effort on the advisor - // call config when supported. Allowed: "", "low", "medium", "high". - ReasoningEffort string `json:"reasoning_effort"` } // UpdateAdvisorConfigRequest is the request body for updating advisor diff --git a/site/src/api/queries/chats.test.ts b/site/src/api/queries/chats.test.ts index 7938e80742..74f9c5acc5 100644 --- a/site/src/api/queries/chats.test.ts +++ b/site/src/api/queries/chats.test.ts @@ -135,7 +135,6 @@ describe("advisor config query factories", () => { enabled: true, max_uses_per_run: 5, max_output_tokens: 2048, - reasoning_effort: "high", model_config_id: "00000000-0000-0000-0000-000000000000", }; vi.mocked(API.experimental.getChatAdvisorConfig).mockResolvedValue( @@ -155,7 +154,6 @@ describe("advisor config query factories", () => { enabled: false, max_uses_per_run: 0, max_output_tokens: 0, - reasoning_effort: "", model_config_id: "", } as TypesGen.AdvisorConfig); @@ -163,7 +161,6 @@ describe("advisor config query factories", () => { enabled: true, max_uses_per_run: 5, max_output_tokens: 2048, - reasoning_effort: "high", model_config_id: "00000000-0000-0000-0000-000000000000", }; vi.mocked(API.experimental.updateChatAdvisorConfig).mockResolvedValue(); diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts index 33ccbd03c9..6499b6c7f9 100644 --- a/site/src/api/typesGenerated.ts +++ b/site/src/api/typesGenerated.ts @@ -828,11 +828,6 @@ export interface AdvisorConfig { * its provider was disabled after the admin saved this config). */ readonly model_config_id: string; - /** - * ReasoningEffort overlays provider reasoning effort on the advisor - * call config when supported. Allowed: "", "low", "medium", "high". - */ - readonly reasoning_effort: string; } // From codersdk/workspacebuilds.go @@ -7919,11 +7914,6 @@ export interface UpdateAdvisorConfigRequest { * its provider was disabled after the admin saved this config). */ readonly model_config_id: string; - /** - * ReasoningEffort overlays provider reasoning effort on the advisor - * call config when supported. Allowed: "", "low", "medium", "high". - */ - readonly reasoning_effort: string; } // From codersdk/deployment.go diff --git a/site/src/pages/AgentsPage/AgentSettingsExperimentsPageView.stories.tsx b/site/src/pages/AgentsPage/AgentSettingsExperimentsPageView.stories.tsx index 63dad2c15b..a7ccad7758 100644 --- a/site/src/pages/AgentsPage/AgentSettingsExperimentsPageView.stories.tsx +++ b/site/src/pages/AgentsPage/AgentSettingsExperimentsPageView.stories.tsx @@ -29,7 +29,6 @@ const baseArgs: AgentSettingsExperimentsPageViewProps = { enabled: false, max_uses_per_run: 0, max_output_tokens: 0, - reasoning_effort: "", model_config_id: "", }, isAdvisorConfigLoading: false, diff --git a/site/src/pages/AgentsPage/components/AdvisorSettings.stories.tsx b/site/src/pages/AgentsPage/components/AdvisorSettings.stories.tsx index 80c99b612b..478aa2956e 100644 --- a/site/src/pages/AgentsPage/components/AdvisorSettings.stories.tsx +++ b/site/src/pages/AgentsPage/components/AdvisorSettings.stories.tsx @@ -48,7 +48,6 @@ const defaultAdvisorConfig: TypesGen.AdvisorConfig = { enabled: false, max_uses_per_run: 0, max_output_tokens: 0, - reasoning_effort: "", model_config_id: "", }; @@ -96,6 +95,9 @@ export const Default: Story = { expect( canvas.queryByRole("combobox", { name: /Advisor model/i }), ).not.toBeInTheDocument(); + expect( + canvas.queryByRole("combobox", { name: /Reasoning effort/i }), + ).not.toBeInTheDocument(); await userEvent.click(enableAdvisorSwitch); @@ -126,12 +128,12 @@ export const Enabled: Story = { const maxOutputTokensInput = canvas.getByRole("spinbutton", { name: /Max output tokens/i, }); - const reasoningEffortSelect = canvas.getByRole("combobox", { - name: /Reasoning effort/i, - }); const advisorModelSelect = canvas.getByRole("combobox", { name: /Advisor model/i, }); + expect( + canvas.queryByRole("combobox", { name: /Reasoning effort/i }), + ).not.toBeInTheDocument(); const saveButton = canvas.getByRole("button", { name: /Save/i }); expect(saveButton).toBeDisabled(); @@ -141,9 +143,6 @@ export const Enabled: Story = { await userEvent.clear(maxOutputTokensInput); await userEvent.type(maxOutputTokensInput, "2048"); - await userEvent.click(reasoningEffortSelect); - await userEvent.click(await body.findByRole("option", { name: /^High$/i })); - await userEvent.click(advisorModelSelect); expect( body.queryByRole("option", { name: /GPT-3.5 \(Disabled\)/i }), @@ -167,7 +166,6 @@ export const Enabled: Story = { enabled: true, max_uses_per_run: 5, max_output_tokens: 2048, - reasoning_effort: "high", model_config_id: "model-2", }); expect(typeof options?.onSuccess).toBe("function"); @@ -236,7 +234,6 @@ export const CustomConfig: Story = { enabled: true, max_uses_per_run: 7, max_output_tokens: 8192, - reasoning_effort: "medium", model_config_id: "model-2", }, }, @@ -251,9 +248,6 @@ export const CustomConfig: Story = { expect(maxUsesInput).toHaveValue(7); expect(maxOutputTokensInput).toHaveValue(8192); - expect( - canvas.getByRole("combobox", { name: /Reasoning effort/i }), - ).toHaveTextContent(/Medium/i); expect( canvas.getByRole("combobox", { name: /Advisor model/i }), ).toHaveTextContent(/Claude Sonnet 4/i); @@ -312,9 +306,6 @@ export const Refetching: Story = { expect( canvas.getByRole("spinbutton", { name: /Max uses per run/i }), ).toBeDisabled(); - expect( - canvas.getByRole("combobox", { name: /Reasoning effort/i }), - ).toBeDisabled(); expect(canvas.getByRole("button", { name: /Save/i })).toBeDisabled(); }, }; @@ -417,9 +408,6 @@ export const Saving: Story = { expect( canvas.getByRole("spinbutton", { name: /Max uses per run/i }), ).toBeDisabled(); - expect( - canvas.getByRole("combobox", { name: /Reasoning effort/i }), - ).toBeDisabled(); expect(canvas.getByRole("button", { name: /Save/i })).toBeDisabled(); }, }; @@ -449,14 +437,16 @@ export const SaveErrorWithDetail: Story = { }, isSaveAdvisorConfigError: true, saveAdvisorConfigError: new Error( - "reasoning_effort must be one of: low, medium, high.", + 'model_config_id "missing" does not match any existing model config.', ), }, play: async ({ canvasElement }) => { const canvas = within(canvasElement); expect( - canvas.getByText(/reasoning_effort must be one of: low, medium, high\./i), + canvas.getByText( + /model_config_id "missing" does not match any existing model config\./i, + ), ).toBeInTheDocument(); }, }; @@ -507,7 +497,6 @@ export const DisableAdvisorWithDeletedModel: Story = { enabled: true, max_uses_per_run: 5, max_output_tokens: 2048, - reasoning_effort: "high", model_config_id: "22222222-2222-2222-2222-222222222222", }, }, @@ -543,7 +532,6 @@ export const DisableAdvisorWithDeletedModel: Story = { enabled: false, max_uses_per_run: 5, max_output_tokens: 2048, - reasoning_effort: "high", model_config_id: nilUUID, }); }, @@ -555,7 +543,6 @@ export const DisableAdvisorWhileModelConfigsLoadingPreservesOverride: Story = { enabled: true, max_uses_per_run: 5, max_output_tokens: 2048, - reasoning_effort: "high", model_config_id: "22222222-2222-2222-2222-222222222222", }, modelConfigs: [], @@ -588,7 +575,6 @@ export const DisableAdvisorWhileModelConfigsLoadingPreservesOverride: Story = { enabled: false, max_uses_per_run: 5, max_output_tokens: 2048, - reasoning_effort: "high", model_config_id: "22222222-2222-2222-2222-222222222222", }); }, @@ -600,7 +586,6 @@ export const DisableAdvisorWithModelConfigsErrorPreservesOverride: Story = { enabled: true, max_uses_per_run: 5, max_output_tokens: 2048, - reasoning_effort: "high", model_config_id: "22222222-2222-2222-2222-222222222222", }, modelConfigs: [], @@ -633,7 +618,6 @@ export const DisableAdvisorWithModelConfigsErrorPreservesOverride: Story = { enabled: false, max_uses_per_run: 5, max_output_tokens: 2048, - reasoning_effort: "high", model_config_id: "22222222-2222-2222-2222-222222222222", }); }, @@ -646,7 +630,6 @@ export const DisableAdvisorWhileModelConfigsRefetchingPreservesOverride: Story = enabled: true, max_uses_per_run: 5, max_output_tokens: 2048, - reasoning_effort: "high", model_config_id: "22222222-2222-2222-2222-222222222222", }, // Simulate a background refetch with stale cached data: react-query @@ -685,7 +668,6 @@ export const DisableAdvisorWhileModelConfigsRefetchingPreservesOverride: Story = enabled: false, max_uses_per_run: 5, max_output_tokens: 2048, - reasoning_effort: "high", model_config_id: "22222222-2222-2222-2222-222222222222", }); }, @@ -697,7 +679,6 @@ export const DisableAdvisorWithDeletedModelAndEmptyModelConfigs: Story = { enabled: true, max_uses_per_run: 5, max_output_tokens: 2048, - reasoning_effort: "high", model_config_id: "22222222-2222-2222-2222-222222222222", }, modelConfigs: [], @@ -732,7 +713,6 @@ export const DisableAdvisorWithDeletedModelAndEmptyModelConfigs: Story = { enabled: false, max_uses_per_run: 5, max_output_tokens: 2048, - reasoning_effort: "high", model_config_id: nilUUID, }); }, @@ -775,7 +755,6 @@ export const DisableThenSave: Story = { enabled: true, max_uses_per_run: 5, max_output_tokens: 2048, - reasoning_effort: "high", model_config_id: "model-2", }, }, @@ -821,7 +800,6 @@ export const DisableThenSave: Story = { enabled: false, max_uses_per_run: 5, max_output_tokens: 2048, - reasoning_effort: "high", model_config_id: "model-2", }); }, diff --git a/site/src/pages/AgentsPage/components/AdvisorSettings.tsx b/site/src/pages/AgentsPage/components/AdvisorSettings.tsx index 2ea74aa5b1..5ce39d5628 100644 --- a/site/src/pages/AgentsPage/components/AdvisorSettings.tsx +++ b/site/src/pages/AgentsPage/components/AdvisorSettings.tsx @@ -21,11 +21,8 @@ import { import { Switch } from "#/components/Switch/Switch"; const nilUUID = "00000000-0000-0000-0000-000000000000"; -const advisorReasoningEfforts = ["", "low", "medium", "high"] as const; -type AdvisorReasoningEffort = (typeof advisorReasoningEfforts)[number]; const chatModelFallbackValue = "__use-chat-model__"; const unavailableModelValue = "__unavailable-model__"; -const chatReasoningFallbackValue = "__use-chat-reasoning__"; interface MutationCallbacks { onSuccess?: () => void; @@ -54,19 +51,12 @@ type AdvisorSettingsFormValues = { enabled: boolean; max_uses_per_run: string; max_output_tokens: string; - reasoning_effort: AdvisorReasoningEffort; model_config_id: string; }; const isUnsetModelConfigId = (id: string): boolean => id === "" || id === nilUUID; -const isAdvisorReasoningEffort = ( - value: string, -): value is AdvisorReasoningEffort => { - return advisorReasoningEfforts.includes(value as AdvisorReasoningEffort); -}; - const normalizeNonNegativeInteger = ( value: number | string | undefined, ): number => { @@ -79,26 +69,20 @@ const normalizeNonNegativeInteger = ( const normalizeAdvisorConfig = ( config: AdvisorConfig | undefined, -): AdvisorSettingsFormValues => { - const reasoningEffort = config?.reasoning_effort ?? ""; - return { - enabled: config?.enabled ?? false, - max_uses_per_run: String( - normalizeNonNegativeInteger(config?.max_uses_per_run), - ), - max_output_tokens: String( - normalizeNonNegativeInteger(config?.max_output_tokens), - ), - reasoning_effort: isAdvisorReasoningEffort(reasoningEffort) - ? reasoningEffort +): AdvisorSettingsFormValues => ({ + enabled: config?.enabled ?? false, + max_uses_per_run: String( + normalizeNonNegativeInteger(config?.max_uses_per_run), + ), + max_output_tokens: String( + normalizeNonNegativeInteger(config?.max_output_tokens), + ), + model_config_id: + typeof config?.model_config_id === "string" && + !isUnsetModelConfigId(config.model_config_id) + ? config.model_config_id : "", - model_config_id: - typeof config?.model_config_id === "string" && - !isUnsetModelConfigId(config.model_config_id) - ? config.model_config_id - : "", - }; -}; +}); const toAdvisorConfigRequest = ( values: AdvisorSettingsFormValues, @@ -106,7 +90,6 @@ const toAdvisorConfigRequest = ( enabled: values.enabled, max_uses_per_run: normalizeNonNegativeInteger(values.max_uses_per_run), max_output_tokens: normalizeNonNegativeInteger(values.max_output_tokens), - reasoning_effort: values.reasoning_effort, model_config_id: isUnsetModelConfigId(values.model_config_id) ? nilUUID : values.model_config_id, @@ -140,29 +123,12 @@ const validateAdvisorConfig = (values: AdvisorSettingsFormValues) => { "Max output tokens must be a non-negative integer."; } - if (!isAdvisorReasoningEffort(values.reasoning_effort)) { - errors.reasoning_effort = "Select a valid reasoning effort."; - } - return errors; }; const getModelDisplayName = (config: ChatModelConfig): string => config.display_name.trim() || config.model; -const getReasoningEffortLabel = (value: AdvisorReasoningEffort): string => { - switch (value) { - case "low": - return "Low"; - case "medium": - return "Medium"; - case "high": - return "High"; - default: - return "Use chat model default"; - } -}; - export const AdvisorSettings: FC = ({ advisorConfigData, isAdvisorConfigLoading, @@ -384,43 +350,6 @@ export const AdvisorSettings: FC = ({

-
- - -

- Controls how hard the advisor model reasons before responding. - Leave unset to use the model's default. -

-
-