mirror of
https://github.com/coder/coder.git
synced 2026-09-24 15:04:27 +08:00
feat: allow overriding advisor reasoning effort (#27196)
This commit is contained in:
+19
-4
@@ -5572,6 +5572,9 @@ func (api *API) getChatAdvisorConfig(rw http.ResponseWriter, r *http.Request) {
|
||||
}
|
||||
resp.MaxUsesPerRun = max(resp.MaxUsesPerRun, 0)
|
||||
resp.MaxOutputTokens = max(resp.MaxOutputTokens, 0)
|
||||
if resp.ModelConfigID == uuid.Nil {
|
||||
resp.ReasoningEffort = nil
|
||||
}
|
||||
resp.Enabled = api.Experiments.Enabled(codersdk.ExperimentChatAdvisor)
|
||||
|
||||
httpapi.Write(ctx, rw, http.StatusOK, resp)
|
||||
@@ -5601,13 +5604,21 @@ func (api *API) putChatAdvisorConfig(rw http.ResponseWriter, r *http.Request) {
|
||||
})
|
||||
return
|
||||
}
|
||||
if req.ModelConfigID != uuid.Nil {
|
||||
if req.ModelConfigID == uuid.Nil {
|
||||
if req.ReasoningEffort != nil {
|
||||
httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{
|
||||
Message: "reasoning_effort requires model_config_id.",
|
||||
})
|
||||
return
|
||||
}
|
||||
} else {
|
||||
// Use system context because GetChatModelConfigByID requires
|
||||
// deployment-config read access, which can be broader than the
|
||||
// handler's explicit update check. The lookup only validates that
|
||||
// the referenced model exists before persisting deployment config.
|
||||
// handler's explicit update check. The lookup validates the model and
|
||||
// any selected reasoning effort before persisting deployment config.
|
||||
//nolint:gocritic // This admin-authorized validation lookup intentionally bypasses read authz.
|
||||
if _, err := api.Database.GetChatModelConfigByID(dbauthz.AsSystemRestricted(ctx), req.ModelConfigID); err != nil {
|
||||
modelConfig, err := api.Database.GetChatModelConfigByID(dbauthz.AsSystemRestricted(ctx), req.ModelConfigID)
|
||||
if err != nil {
|
||||
if errors.Is(err, sql.ErrNoRows) || httpapi.Is404Error(err) {
|
||||
httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{
|
||||
Message: fmt.Sprintf("model_config_id %q does not match any existing model config.", req.ModelConfigID),
|
||||
@@ -5620,6 +5631,10 @@ func (api *API) putChatAdvisorConfig(rw http.ResponseWriter, r *http.Request) {
|
||||
})
|
||||
return
|
||||
}
|
||||
if status, response := validateChatModelOverrideEffort(modelConfig, req.ReasoningEffort); response != nil {
|
||||
httpapi.Write(ctx, rw, status, *response)
|
||||
return
|
||||
}
|
||||
}
|
||||
|
||||
raw, err := json.Marshal(req)
|
||||
|
||||
@@ -13988,13 +13988,21 @@ func TestChatAdvisorConfig_RoundTripModelConfigID(t *testing.T) {
|
||||
adminClient := newChatClient(t)
|
||||
coderdtest.CreateFirstUser(t, adminClient.Client)
|
||||
|
||||
modelConfig := createChatModelConfig(t, adminClient)
|
||||
modelConfig := createAdditionalChatModelConfigWithReasoningEffort(
|
||||
t,
|
||||
adminClient,
|
||||
"openai",
|
||||
"gpt-5.2",
|
||||
codersdk.ChatModelReasoningEffortMedium,
|
||||
codersdk.ChatModelReasoningEffortXHigh,
|
||||
)
|
||||
|
||||
want := codersdk.AdvisorConfig{
|
||||
Enabled: true,
|
||||
MaxUsesPerRun: 3,
|
||||
MaxOutputTokens: 2048,
|
||||
ModelConfigID: modelConfig.ID,
|
||||
ReasoningEffort: ptr.Ref(codersdk.ChatModelReasoningEffortHigh),
|
||||
}
|
||||
|
||||
err := adminClient.UpdateChatAdvisorConfig(ctx, want)
|
||||
@@ -14021,6 +14029,44 @@ func TestChatAdvisorConfig_InvalidModelConfigID(t *testing.T) {
|
||||
require.Contains(t, sdkErr.Message, "does not match any existing model config")
|
||||
}
|
||||
|
||||
func TestChatAdvisorConfig_ReasoningEffortRequiresModelConfig(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: ptr.Ref(codersdk.ChatModelReasoningEffortHigh),
|
||||
})
|
||||
sdkErr := requireSDKError(t, err, http.StatusBadRequest)
|
||||
require.Equal(t, "reasoning_effort requires model_config_id.", sdkErr.Message)
|
||||
}
|
||||
|
||||
func TestChatAdvisorConfig_ReasoningEffortMustBeSelectable(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
ctx := testutil.Context(t, testutil.WaitLong)
|
||||
adminClient := newChatClient(t)
|
||||
coderdtest.CreateFirstUser(t, adminClient.Client)
|
||||
modelConfig := createAdditionalChatModelConfigWithReasoningEffort(
|
||||
t,
|
||||
adminClient,
|
||||
"openai",
|
||||
"gpt-5.2",
|
||||
codersdk.ChatModelReasoningEffortLow,
|
||||
codersdk.ChatModelReasoningEffortMedium,
|
||||
)
|
||||
|
||||
err := adminClient.UpdateChatAdvisorConfig(ctx, codersdk.UpdateAdvisorConfigRequest{
|
||||
ModelConfigID: modelConfig.ID,
|
||||
ReasoningEffort: ptr.Ref(codersdk.ChatModelReasoningEffortHigh),
|
||||
})
|
||||
sdkErr := requireSDKError(t, err, http.StatusBadRequest)
|
||||
require.Equal(t, "Invalid reasoning_effort value.", sdkErr.Message)
|
||||
require.Equal(t, "Must be one of none, minimal, low, medium.", sdkErr.Detail)
|
||||
}
|
||||
|
||||
func TestChatAdvisorConfig_RoundTripZeroValues(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
@@ -14053,13 +14099,21 @@ func TestChatAdvisorConfig_OverwriteClearsPreviousValues(t *testing.T) {
|
||||
adminClient := newChatClient(t)
|
||||
coderdtest.CreateFirstUser(t, adminClient.Client)
|
||||
|
||||
modelConfig := createChatModelConfig(t, adminClient)
|
||||
modelConfig := createAdditionalChatModelConfigWithReasoningEffort(
|
||||
t,
|
||||
adminClient,
|
||||
"openai",
|
||||
"gpt-5.2",
|
||||
codersdk.ChatModelReasoningEffortMedium,
|
||||
codersdk.ChatModelReasoningEffortXHigh,
|
||||
)
|
||||
|
||||
rich := codersdk.AdvisorConfig{
|
||||
Enabled: true,
|
||||
MaxUsesPerRun: 5,
|
||||
MaxOutputTokens: 1024,
|
||||
ModelConfigID: modelConfig.ID,
|
||||
ReasoningEffort: ptr.Ref(codersdk.ChatModelReasoningEffortHigh),
|
||||
}
|
||||
err := adminClient.UpdateChatAdvisorConfig(ctx, rich)
|
||||
require.NoError(t, err)
|
||||
|
||||
@@ -316,6 +316,10 @@ func TestResolveAdvisorModelOverride(t *testing.T) {
|
||||
providerID := uuid.New()
|
||||
rawOptions, err := json.Marshal(codersdk.ChatModelCallConfig{
|
||||
Temperature: func() *float64 { v := 0.42; return &v }(),
|
||||
ReasoningEffort: &codersdk.ChatModelReasoningEffortConfig{
|
||||
Default: ptr.Ref(codersdk.ChatModelReasoningEffortLow),
|
||||
Max: ptr.Ref(codersdk.ChatModelReasoningEffortXHigh),
|
||||
},
|
||||
})
|
||||
require.NoError(t, err)
|
||||
store := &advisorOverrideStubStore{
|
||||
@@ -343,7 +347,10 @@ func TestResolveAdvisorModelOverride(t *testing.T) {
|
||||
gotModel, gotCfg := p.resolveAdvisorModelOverrideOrFallback(
|
||||
ctx,
|
||||
database.Chat{},
|
||||
codersdk.AdvisorConfig{ModelConfigID: configID},
|
||||
codersdk.AdvisorConfig{
|
||||
ModelConfigID: configID,
|
||||
ReasoningEffort: ptr.Ref(codersdk.ChatModelReasoningEffortHigh),
|
||||
},
|
||||
fallbackModel,
|
||||
fallbackCallConfig,
|
||||
modelBuildOptions{ActiveAPIKeyID: uuid.NewString()},
|
||||
@@ -359,6 +366,9 @@ func TestResolveAdvisorModelOverride(t *testing.T) {
|
||||
require.Equal(t, "gpt-5.2", gotModel.Model())
|
||||
require.NotNil(t, gotCfg.Temperature)
|
||||
require.InDelta(t, 0.42, *gotCfg.Temperature, 1e-9)
|
||||
require.NotNil(t, gotCfg.ReasoningEffort)
|
||||
require.Equal(t, codersdk.ChatModelReasoningEffortHigh, *gotCfg.ReasoningEffort.Default)
|
||||
require.Equal(t, codersdk.ChatModelReasoningEffortHigh, *gotCfg.ReasoningEffort.Max)
|
||||
})
|
||||
t.Run("AIProviderIDResolvesOverrideProviderKeys", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
+15
-2
@@ -350,6 +350,19 @@ func (p *Server) resolveAdvisorModelOverride(
|
||||
return fallbackModel, fallbackCallConfig, nil
|
||||
}
|
||||
|
||||
if advisorCfg.ReasoningEffort != nil {
|
||||
resolvedEffort := chatprovider.ResolveReasoningEffort(
|
||||
advisorCfg.ReasoningEffort,
|
||||
overrideCallConfig.ReasoningEffort,
|
||||
)
|
||||
if resolvedEffort != nil {
|
||||
overrideCallConfig.ReasoningEffort = &codersdk.ChatModelReasoningEffortConfig{
|
||||
Default: resolvedEffort,
|
||||
Max: resolvedEffort,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
return overrideModel, overrideCallConfig, nil
|
||||
}
|
||||
|
||||
@@ -398,8 +411,8 @@ func (p *Server) newAdvisorRuntime(
|
||||
}
|
||||
|
||||
advisorCallConfig.MaxOutputTokens = ptr.Ref(maxOutputTokens)
|
||||
// The advisor has no per-turn effort selection; its model config's
|
||||
// default effort applies.
|
||||
// The override resolver pins an explicit advisor effort into the model
|
||||
// config. Fallback models keep their configured default effort.
|
||||
advisorReasoningEffort := chatprovider.ResolveReasoningEffort(
|
||||
nil,
|
||||
advisorCallConfig.ReasoningEffort,
|
||||
|
||||
Reference in New Issue
Block a user