diff --git a/coderd/x/chatd/ARCHITECTURE.md b/coderd/x/chatd/ARCHITECTURE.md index bec77910ec..647381559b 100644 --- a/coderd/x/chatd/ARCHITECTURE.md +++ b/coderd/x/chatd/ARCHITECTURE.md @@ -858,17 +858,17 @@ During generation preparation, the effective effort is resolved as the chat's `l OpenAI models speak either the Responses API or Chat Completions. The provider SDK picks per model from a static known-model list, so a newly released model absent from that list falls back to Chat Completions. Model configs may override the choice with `openai_config.use_responses_api` inside `chat_model_configs.options`: unset keeps the known-model list, true forces Responses, false forces Chat Completions. It sits in `openai_config` rather than `provider_options.openai` because it is applied once when the client is built, while `provider_options` holds per-request parameters. -The transport is decided in more than one place, and those decisions must agree with the client that was built. `chatopenai.UsesResponsesAPI` is the single predicate, and every path that builds an OpenAI client must pass the same override to both the client and the code that prepares its requests: +The transport is resolved exactly once, when the client is built, and carried on `chatprovider.Model` as a `chatopenai.Transport`. `Model` wraps the fantasy client with that resolved fact; its fields are unexported and only its constructor sets the transport, deriving it from the client, so no caller can pick a transport that disagrees with the client. `TransportInvalid` is the zero value and panics when read rather than defaulting to a wire format. A nil client yields that invalid zero value, which the construction path reports as an error. -- Client construction (`ModelFromConfig`) installs the override as the SDK's per-model transport hook. The hook only selects among transports the client enables, so it cannot turn on Responses for a provider whose client was not built to allow it. -- Provider option conversion (`UsesResponsesOptions`) chooses between the Responses and Chat Completions option structs. The SDK type-asserts the concrete struct, so a mismatch silently discards every OpenAI provider option rather than failing. -- File part conversion (`AcceptsFilePartMediaType`) gates attachments, because the Responses API natively accepts only images and PDFs. A mismatch here silently drops text attachments. +Request preparation reads the transport from the model instead of recomputing it. Three places depend on it, and each fails silently when it disagrees with the client: -Paths that build their own clients must thread the override too, including the compaction override, quick generation (used by turn status labels and debug models), and the advisor runtime. +- Provider option conversion chooses between the Responses and Chat Completions option structs. The SDK type-asserts the concrete struct, so a mismatch discards every OpenAI provider option rather than failing. +- Reasoning effort injection creates those option structs when a config has no OpenAI options of its own. +- File part conversion (`Model.AcceptsFilePartMediaType`) gates attachments, because the Responses API natively accepts only images and PDFs. A mismatch here drops text attachments. -Client construction returns a `chatprovider.Model`, which pairs the fantasy client with the transport resolved from that client's own identity as a `chatopenai.Transport`. Its fields are unexported and only the constructor sets the transport, so no caller can pair a client with a transport it does not speak; a nil client yields the invalid zero value, which fails closed. Decorators such as debug recording replace the wrapped client through `Model.WithLanguageModel`, which preserves the resolved transport, because wrapping does not change what the client speaks. Request preparation does not read the carried transport yet; it still recomputes the decision from the override, and the wrapper is the authoritative value those recomputations must agree with. +Paths that build their own clients get a `Model` from the same constructor, including the compaction override, quick generation (used by turn status labels and debug models), and the advisor runtime. Debug recording replaces the wrapped client and preserves the resolved transport. Computer-use turns substitute a hardcoded default model that has no config of its own; it carries its own transport, so the chat model's `openai_config` does not follow it. -Azure is deliberately exempt: its provider always enables the Responses API for known models and exposes no equivalent per-model hook, so `UsesResponsesAPI` keeps following the known-model list for Azure. Ignoring the override there is what keeps the decisions above in agreement with the Azure client. The exemption is narrower than it appears, because chatd never builds an azure-typed provider as a fantasy azure client: `fantasyConfigForAIBridge` folds every provider type other than anthropic, bedrock, and openai into openai-compat, which always speaks Chat Completions. +Azure is deliberately exempt: its provider always enables the Responses API for known models and exposes no equivalent per-model hook, so the transport keeps following the known-model list for Azure. Ignoring the override there is what keeps the decisions above in agreement with the Azure client. The exemption is narrower than it appears, because chatd never builds an azure-typed provider as a fantasy azure client: `fantasyConfigForAIBridge` folds every provider type other than anthropic, bedrock, and openai into openai-compat, which always speaks Chat Completions. Both transports read the same `provider_options.openai` config, but not every field applies to both wire formats. The table below records, per field, which transport honors it; `TestProviderOptionsTransportParity` fails when a field is honored on one transport and silently ignored on the other without being recorded there as intentional. diff --git a/coderd/x/chatd/chatd.go b/coderd/x/chatd/chatd.go index fb66ea4bdd..ae57548d0d 100644 --- a/coderd/x/chatd/chatd.go +++ b/coderd/x/chatd/chatd.go @@ -428,17 +428,14 @@ func (p *Server) newAdvisorRuntime( nil, advisorCallConfig.ReasoningEffort, ) - advisorResponsesOverride := chatprovider.OpenAIResponsesAPIOverride(advisorCallConfig.OpenAIConfig) providerOptions := chatprovider.ProviderOptionsFromChatModelConfig( - advisorModel.LanguageModel(), + advisorModel, advisorCallConfig.ProviderOptions, - advisorResponsesOverride, ) providerOptions = chatprovider.ApplyReasoningEffort( - advisorModel.LanguageModel(), + advisorModel, providerOptions, advisorReasoningEffort, - advisorResponsesOverride, ) rt, err := chatadvisor.NewRuntime(chatadvisor.RuntimeConfig{ diff --git a/coderd/x/chatd/chatopenai/options.go b/coderd/x/chatd/chatopenai/options.go index 6f4c53c290..0a55a41115 100644 --- a/coderd/x/chatd/chatopenai/options.go +++ b/coderd/x/chatd/chatopenai/options.go @@ -14,11 +14,10 @@ import ( // ProviderOptionsFromChatConfig converts chat model OpenAI options to fantasy // provider options used for inference calls. func ProviderOptionsFromChatConfig( - model fantasy.LanguageModel, + transport Transport, options *codersdk.ChatModelOpenAIProviderOptions, - openAIResponsesOverride *bool, ) fantasy.ProviderOptionsData { - if UsesResponsesOptions(model, openAIResponsesOverride) { + if transport.UsesResponses() { include := EnsureResponseIncludes(IncludeFromChat(options.Include)) providerOptions := &fantasyopenai.ResponsesProviderOptions{ Include: include, @@ -116,21 +115,6 @@ func EnsureResponseIncludes( return append(values, required) } -// UsesResponsesAPI reports whether a model uses the OpenAI Responses API. -// Callers must pass the same override the client was built with. -func UsesResponsesAPI(provider, modelID string, override *bool) bool { - return TransportFor(provider, modelID, override).UsesResponses() -} - -// UsesResponsesOptions reports whether the model should use OpenAI Responses -// API provider options. -func UsesResponsesOptions(model fantasy.LanguageModel, override *bool) bool { - if model == nil { - return false - } - return UsesResponsesAPI(model.Provider(), model.Model(), override) -} - // ServiceTierFromChat normalizes chat-config service tier values for the // OpenAI Responses API. It maps every tier the codersdk enum advertises, not // only the ones fantasy declares constants for, because fantasy forwards the diff --git a/coderd/x/chatd/chatopenai/options_test.go b/coderd/x/chatd/chatopenai/options_test.go index 6d44da4b19..1a89d9e17e 100644 --- a/coderd/x/chatd/chatopenai/options_test.go +++ b/coderd/x/chatd/chatopenai/options_test.go @@ -1,11 +1,8 @@ package chatopenai_test import ( - "context" "testing" - "charm.land/fantasy" - fantasyazure "charm.land/fantasy/providers/azure" fantasyopenai "charm.land/fantasy/providers/openai" "github.com/stretchr/testify/require" @@ -44,9 +41,8 @@ func TestProviderOptionsFromChatConfigLegacy(t *testing.T) { } got := chatopenai.ProviderOptionsFromChatConfig( - fakeLanguageModel{provider: fantasyopenai.Name, model: "gpt-3.5-turbo-instruct"}, + chatopenai.TransportChatCompletions, options, - nil, ) providerOptions, ok := got.(*fantasyopenai.ProviderOptions) @@ -97,9 +93,8 @@ func TestProviderOptionsFromChatConfigResponses(t *testing.T) { } got := chatopenai.ProviderOptionsFromChatConfig( - fakeLanguageModel{provider: fantasyopenai.Name, model: "gpt-4.1"}, + chatopenai.TransportResponses, options, - nil, ) providerOptions, ok := got.(*fantasyopenai.ResponsesProviderOptions) @@ -242,75 +237,6 @@ func TestEnsureResponseIncludes(t *testing.T) { } } -func TestUsesResponsesOptions(t *testing.T) { - t.Parallel() - - forceResponses := true - forceCompletions := false - - tests := []struct { - name string - model fantasy.LanguageModel - override *bool - want bool - }{ - {name: "Nil"}, - { - name: "OpenAIResponsesModel", - model: fakeLanguageModel{provider: fantasyopenai.Name, model: "gpt-4.1"}, - want: true, - }, - { - name: "AzureResponsesModel", - model: fakeLanguageModel{provider: fantasyazure.Name, model: "gpt-4.1"}, - want: true, - }, - { - name: "OpenAINonResponsesModel", - model: fakeLanguageModel{provider: fantasyopenai.Name, model: "gpt-3.5-turbo-instruct"}, - }, - { - name: "NonOpenAIProvider", - model: fakeLanguageModel{provider: "other", model: "gpt-4.1"}, - }, - { - name: "NilModelIgnoresOverride", - override: &forceResponses, - }, - { - name: "OverrideForcesResponsesForUnknownModel", - model: fakeLanguageModel{provider: fantasyopenai.Name, model: "gpt-9-brand-new"}, - override: &forceResponses, - want: true, - }, - { - name: "OverrideForcesCompletionsForResponsesModel", - model: fakeLanguageModel{provider: fantasyopenai.Name, model: "gpt-4.1"}, - override: &forceCompletions, - }, - { - name: "AzureIgnoresOverride", - model: fakeLanguageModel{provider: fantasyazure.Name, model: "gpt-4.1"}, - override: &forceCompletions, - want: true, - }, - { - name: "NonOpenAIProviderIgnoresOverride", - model: fakeLanguageModel{provider: "other", model: "gpt-4.1"}, - override: &forceResponses, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - t.Parallel() - - got := chatopenai.UsesResponsesOptions(tt.model, tt.override) - require.Equal(t, tt.want, got) - }) - } -} - func TestServiceTierFromChat(t *testing.T) { t.Parallel() @@ -456,32 +382,3 @@ func requireTextVerbosityPointerValue( func ptr[T any](value T) *T { return &value } - -type fakeLanguageModel struct { - provider string - model string -} - -func (fakeLanguageModel) Generate(context.Context, fantasy.Call) (*fantasy.Response, error) { - panic("not implemented") -} - -func (fakeLanguageModel) Stream(context.Context, fantasy.Call) (fantasy.StreamResponse, error) { - panic("not implemented") -} - -func (fakeLanguageModel) GenerateObject(context.Context, fantasy.ObjectCall) (*fantasy.ObjectResponse, error) { - panic("not implemented") -} - -func (fakeLanguageModel) StreamObject(context.Context, fantasy.ObjectCall) (fantasy.ObjectStreamResponse, error) { - panic("not implemented") -} - -func (f fakeLanguageModel) Provider() string { - return f.provider -} - -func (f fakeLanguageModel) Model() string { - return f.model -} diff --git a/coderd/x/chatd/chatopenai/transport_parity_test.go b/coderd/x/chatd/chatopenai/transport_parity_test.go index ac1b24bfd1..1bdfc45aba 100644 --- a/coderd/x/chatd/chatopenai/transport_parity_test.go +++ b/coderd/x/chatd/chatopenai/transport_parity_test.go @@ -90,24 +90,23 @@ func TestProviderOptionsTransportParity(t *testing.T) { reflect.ValueOf(options).Elem().Field(i).Set(sample) require.Equalf(t, want.responses, - optionChangesConvertedOutput(t, ptr(true), options), + optionChangesConvertedOutput(t, chatopenai.TransportResponses, options), "field %s on the Responses transport", name) require.Equalf(t, want.chatCompletions, - optionChangesConvertedOutput(t, ptr(false), options), + optionChangesConvertedOutput(t, chatopenai.TransportChatCompletions, options), "field %s on the Chat Completions transport", name) } } func optionChangesConvertedOutput( t *testing.T, - responsesOverride *bool, + transport chatopenai.Transport, options *codersdk.ChatModelOpenAIProviderOptions, ) bool { t.Helper() - model := fakeLanguageModel{provider: fantasyopenai.Name, model: "gpt-4.1"} baseline := chatopenai.ProviderOptionsFromChatConfig( - model, &codersdk.ChatModelOpenAIProviderOptions{}, responsesOverride, + transport, &codersdk.ChatModelOpenAIProviderOptions{}, ) - converted := chatopenai.ProviderOptionsFromChatConfig(model, options, responsesOverride) + converted := chatopenai.ProviderOptionsFromChatConfig(transport, options) return !reflect.DeepEqual(baseline, converted) } diff --git a/coderd/x/chatd/chatprovider/chatprovider.go b/coderd/x/chatd/chatprovider/chatprovider.go index 051c0d0f4d..2ee273a233 100644 --- a/coderd/x/chatd/chatprovider/chatprovider.go +++ b/coderd/x/chatd/chatprovider/chatprovider.go @@ -122,10 +122,11 @@ func InlineImageCapBytes(provider string) (int, bool) { } } -// AcceptsFilePartMediaType reports whether a provider transport accepts -// mediaType as a native file part. Unknown providers return false so callers -// can avoid silently dropping unsupported text-family content. -func AcceptsFilePartMediaType(provider, modelID, mediaType string, openAIResponsesOverride *bool) bool { +// AcceptsFilePartMediaType reports whether m's provider accepts mediaType as a +// file content part rather than silently dropping it. Callers replace rejected +// parts with text, so a false negative costs fidelity while a false positive +// loses the attachment entirely. Unknown providers therefore return false. +func (m Model) AcceptsFilePartMediaType(mediaType string) bool { baseType := mediaType if parsed, _, err := mime.ParseMediaType(mediaType); err == nil { baseType = parsed @@ -138,8 +139,7 @@ func AcceptsFilePartMediaType(provider, modelID, mediaType string, openAIRespons isAudio := baseType == "audio/wav" || baseType == "audio/mpeg" || baseType == "audio/mp3" isPDF := baseType == "application/pdf" - normalized := NormalizeProvider(provider) - switch normalized { + switch NormalizeProvider(m.Provider()) { case fantasygoogle.Name: // Google passes any file part through unfiltered. return true @@ -149,7 +149,7 @@ func AcceptsFilePartMediaType(provider, modelID, mediaType string, openAIRespons return isImage || isText || isPDF case fantasyopenai.Name, fantasyazure.Name: // Chat Completions accepts text and audio as native file parts. - if chatopenai.UsesResponsesAPI(normalized, modelID, openAIResponsesOverride) { + if m.transport.UsesResponses() { return isImage || isPDF } return isImage || isText || isAudio || isPDF @@ -894,10 +894,11 @@ func BetaHeadersFromCallConfig(providerName string, config *codersdk.ChatModelCa } } -// OpenAIResponsesAPIOverride returns the configured OpenAI Responses API +// openAIResponsesAPIOverride returns the configured OpenAI Responses API // override, or nil when the model config leaves the choice to the provider -// SDK's known-model list. -func OpenAIResponsesAPIOverride(config *codersdk.ChatModelOpenAIConfig) *bool { +// SDK's known-model list. It stays unexported so the decision is reachable +// only from client construction. +func openAIResponsesAPIOverride(config *codersdk.ChatModelOpenAIConfig) *bool { if config == nil { return nil } @@ -909,9 +910,8 @@ func OpenAIResponsesAPIOverride(config *codersdk.ChatModelOpenAIConfig) *bool { // userAgent is sent as the User-Agent header on every outgoing LLM // API request. extraHeaders, when non-nil, are sent as additional // HTTP headers on every request. httpClient, when non-nil, is used for -// all provider HTTP requests. openAIResponsesOverride, when non-nil, -// forces the OpenAI client onto the Responses API or Chat Completions -// instead of deciding from the provider SDK's known-model list. +// all provider HTTP requests. openAIConfig carries the model's OpenAI client +// settings, including the transport override applied here. func ModelFromConfig( providerHint string, modelName string, @@ -919,7 +919,7 @@ func ModelFromConfig( userAgent string, extraHeaders map[string]string, httpClient *http.Client, - openAIResponsesOverride *bool, + openAIConfig *codersdk.ChatModelOpenAIConfig, ) (Model, error) { provider, modelID, err := ResolveModelWithProviderHint(modelName, providerHint) if err != nil { @@ -1008,8 +1008,8 @@ func ModelFromConfig( fantasyopenai.WithUseResponsesAPI(), fantasyopenai.WithUserAgent(userAgent), } - if openAIResponsesOverride != nil { - forced := *openAIResponsesOverride + if override := openAIResponsesAPIOverride(openAIConfig); override != nil { + forced := *override options = append(options, fantasyopenai.WithResponsesAPIFunc(func(string) bool { return forced })) @@ -1078,7 +1078,7 @@ func ModelFromConfig( if err != nil { return Model{}, xerrors.Errorf("load %s model: %w", provider, err) } - return NewModel(model, openAIResponsesOverride), nil + return NewModel(model, openAIConfig), nil } func providerCreationError(provider string, err error) error { @@ -1112,9 +1112,8 @@ func missingProviderAPIKeyError(provider string) error { // ProviderOptionsFromChatModelConfig converts chat model provider options to // fantasy provider options used for inference calls. func ProviderOptionsFromChatModelConfig( - model fantasy.LanguageModel, + model Model, options *codersdk.ChatModelProviderOptions, - openAIResponsesOverride *bool, ) fantasy.ProviderOptions { if options == nil { return nil @@ -1124,9 +1123,8 @@ func ProviderOptionsFromChatModelConfig( if options.OpenAI != nil { result[fantasyopenai.Name] = chatopenai.ProviderOptionsFromChatConfig( - model, + model.transport, options.OpenAI, - openAIResponsesOverride, ) } if options.Anthropic != nil { diff --git a/coderd/x/chatd/chatprovider/chatprovider_test.go b/coderd/x/chatd/chatprovider/chatprovider_test.go index 6a1188c3da..d5c8cc723e 100644 --- a/coderd/x/chatd/chatprovider/chatprovider_test.go +++ b/coderd/x/chatd/chatprovider/chatprovider_test.go @@ -390,11 +390,11 @@ func TestAnthropicThinkingDisplayFromChat(t *testing.T) { func TestProviderOptionsFromChatModelConfig_AnthropicThinkingDisplay(t *testing.T) { t.Parallel() - providerOptions := chatprovider.ProviderOptionsFromChatModelConfig(nil, &codersdk.ChatModelProviderOptions{ + providerOptions := chatprovider.ProviderOptionsFromChatModelConfig(chatprovider.Model{}, &codersdk.ChatModelProviderOptions{ Anthropic: &codersdk.ChatModelAnthropicProviderOptions{ ThinkingDisplay: ptr.Ref(" SUMMARIZED "), }, - }, nil) + }) require.NotNil(t, providerOptions) anthropicOptions, ok := providerOptions[fantasyanthropic.Name].(*fantasyanthropic.ProviderOptions) diff --git a/coderd/x/chatd/chatprovider/file_parts_test.go b/coderd/x/chatd/chatprovider/file_parts_test.go index 09f90331f7..7689ccbb9d 100644 --- a/coderd/x/chatd/chatprovider/file_parts_test.go +++ b/coderd/x/chatd/chatprovider/file_parts_test.go @@ -4,6 +4,8 @@ import ( "testing" "github.com/coder/coder/v2/coderd/x/chatd/chatprovider" + "github.com/coder/coder/v2/coderd/x/chatd/chattest" + "github.com/coder/coder/v2/codersdk" ) func TestAcceptsFilePartMediaType(t *testing.T) { @@ -98,7 +100,15 @@ func TestAcceptsFilePartMediaType(t *testing.T) { for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { t.Parallel() - got := chatprovider.AcceptsFilePartMediaType(tc.provider, tc.modelID, tc.mediaType, tc.override) + var openAIConfig *codersdk.ChatModelOpenAIConfig + if tc.override != nil { + openAIConfig = &codersdk.ChatModelOpenAIConfig{UseResponsesAPI: tc.override} + } + model := chatprovider.NewModel( + &chattest.FakeModel{ProviderName: tc.provider, ModelName: tc.modelID}, + openAIConfig, + ) + got := model.AcceptsFilePartMediaType(tc.mediaType) if got != tc.want { t.Fatalf("AcceptsFilePartMediaType(%q, %q, %q, %v) = %v, want %v", tc.provider, tc.modelID, tc.mediaType, tc.override, got, tc.want) diff --git a/coderd/x/chatd/chatprovider/model.go b/coderd/x/chatd/chatprovider/model.go index 09414202ac..a6135d3f87 100644 --- a/coderd/x/chatd/chatprovider/model.go +++ b/coderd/x/chatd/chatprovider/model.go @@ -4,6 +4,7 @@ import ( "charm.land/fantasy" "github.com/coder/coder/v2/coderd/x/chatd/chatopenai" + "github.com/coder/coder/v2/codersdk" ) // Model pairs a language model client with the facts resolved when it was @@ -16,10 +17,10 @@ type Model struct { } // NewModel pairs an already-built client with the transport resolved from that -// client's own identity. ModelFromConfig is the only production caller; -// callers must pass the same override the client was built with, which is the -// one degree of freedom Model cannot police. -func NewModel(lm fantasy.LanguageModel, openAIResponsesOverride *bool) Model { +// client's own identity. Callers must pass the config the client was built +// with, the one degree of freedom Model cannot police. ModelFromConfig is the +// only production caller. +func NewModel(lm fantasy.LanguageModel, openAIConfig *codersdk.ChatModelOpenAIConfig) Model { if lm == nil { // The invalid zero value lets callers report a nil client as an // error instead of dereferencing it here. @@ -27,7 +28,7 @@ func NewModel(lm fantasy.LanguageModel, openAIResponsesOverride *bool) Model { } return Model{ lm: lm, - transport: chatopenai.TransportFor(lm.Provider(), lm.Model(), openAIResponsesOverride), + transport: chatopenai.TransportFor(lm.Provider(), lm.Model(), openAIResponsesAPIOverride(openAIConfig)), } } diff --git a/coderd/x/chatd/chatprovider/model_test.go b/coderd/x/chatd/chatprovider/model_test.go index e5015ddf8a..07ee6bdc90 100644 --- a/coderd/x/chatd/chatprovider/model_test.go +++ b/coderd/x/chatd/chatprovider/model_test.go @@ -9,6 +9,7 @@ import ( "github.com/coder/coder/v2/coderd/x/chatd/chatopenai" "github.com/coder/coder/v2/coderd/x/chatd/chatprovider" "github.com/coder/coder/v2/coderd/x/chatd/chattest" + "github.com/coder/coder/v2/codersdk" ) func TestModelResolvesTransportFromClient(t *testing.T) { @@ -16,14 +17,29 @@ func TestModelResolvesTransportFromClient(t *testing.T) { forceResponses := true - model := chatprovider.NewModel( - &chattest.FakeModel{ProviderName: fantasyopenai.Name, ModelName: "babbage-002"}, - &forceResponses, - ) - require.Equal(t, chatopenai.TransportResponses, model.Transport()) - require.Equal(t, fantasyopenai.Name, model.Provider()) - require.Equal(t, "babbage-002", model.ModelID()) - require.True(t, model.Valid()) + // babbage-002 is absent from the provider SDK's known Responses model list. + client := &chattest.FakeModel{ProviderName: fantasyopenai.Name, ModelName: "babbage-002"} + + tests := []struct { + name string + config *codersdk.ChatModelOpenAIConfig + want chatopenai.Transport + }{ + {"NoConfig", nil, chatopenai.TransportChatCompletions}, + {"ConfigWithoutOverride", &codersdk.ChatModelOpenAIConfig{}, chatopenai.TransportChatCompletions}, + {"Forced", &codersdk.ChatModelOpenAIConfig{UseResponsesAPI: &forceResponses}, chatopenai.TransportResponses}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + model := chatprovider.NewModel(client, tt.config) + require.Equal(t, tt.want, model.Transport()) + require.Equal(t, fantasyopenai.Name, model.Provider()) + require.Equal(t, "babbage-002", model.ModelID()) + require.True(t, model.Valid()) + }) + } } func TestModelZeroValueFailsClosed(t *testing.T) { @@ -54,7 +70,7 @@ func TestModelWithLanguageModelPreservesTransport(t *testing.T) { forceResponses := true model := chatprovider.NewModel( &chattest.FakeModel{ProviderName: fantasyopenai.Name, ModelName: "babbage-002"}, - &forceResponses, + &codersdk.ChatModelOpenAIConfig{UseResponsesAPI: &forceResponses}, ) replacement := &chattest.FakeModel{ProviderName: fantasyopenai.Name, ModelName: "babbage-002"} diff --git a/coderd/x/chatd/chatprovider/reasoningeffort.go b/coderd/x/chatd/chatprovider/reasoningeffort.go index 31138cca4b..50e78923e3 100644 --- a/coderd/x/chatd/chatprovider/reasoningeffort.go +++ b/coderd/x/chatd/chatprovider/reasoningeffort.go @@ -12,7 +12,6 @@ import ( fantasyopenrouter "charm.land/fantasy/providers/openrouter" fantasyvercel "charm.land/fantasy/providers/vercel" - "github.com/coder/coder/v2/coderd/x/chatd/chatopenai" "github.com/coder/coder/v2/codersdk" ) @@ -89,12 +88,11 @@ func SelectableReasoningEfforts( } func ApplyReasoningEffort( - model fantasy.LanguageModel, + model Model, options fantasy.ProviderOptions, effort *string, - openAIResponsesOverride *bool, ) fantasy.ProviderOptions { - if effort == nil || model == nil { + if effort == nil || !model.Valid() { return options } if options == nil { @@ -110,7 +108,7 @@ func ApplyReasoningEffort( case *fantasyopenai.ProviderOptions: opts.ReasoningEffort = &providerEffort default: - if chatopenai.UsesResponsesOptions(model, openAIResponsesOverride) { + if model.transport.UsesResponses() { options[fantasyopenai.Name] = &fantasyopenai.ResponsesProviderOptions{ ReasoningEffort: &providerEffort, } diff --git a/coderd/x/chatd/chatprovider/reasoningeffort_test.go b/coderd/x/chatd/chatprovider/reasoningeffort_test.go index a7c270bbc1..d4bdeef5d8 100644 --- a/coderd/x/chatd/chatprovider/reasoningeffort_test.go +++ b/coderd/x/chatd/chatprovider/reasoningeffort_test.go @@ -85,7 +85,7 @@ func TestApplyReasoningEffort(t *testing.T) { t.Run("CreatesOpenAIResponsesEntry", func(t *testing.T) { t.Parallel() - got := chatprovider.ApplyReasoningEffort(&chattest.FakeModel{ProviderName: fantasyopenai.Name, ModelName: "gpt-5"}, nil, new(codersdk.ChatModelReasoningEffortHigh), nil) + got := chatprovider.ApplyReasoningEffort(chatprovider.NewModel(&chattest.FakeModel{ProviderName: fantasyopenai.Name, ModelName: "gpt-5"}, nil), nil, new(codersdk.ChatModelReasoningEffortHigh)) providerOptions, ok := got[fantasyopenai.Name].(*fantasyopenai.ResponsesProviderOptions) require.True(t, ok, "%T", got[fantasyopenai.Name]) require.NotNil(t, providerOptions.ReasoningEffort) @@ -101,7 +101,7 @@ func TestApplyReasoningEffort(t *testing.T) { Store: ptr.Ref(true), }, } - got := chatprovider.ApplyReasoningEffort(&chattest.FakeModel{ProviderName: fantasyopenai.Name, ModelName: "gpt-5"}, options, new(codersdk.ChatModelReasoningEffortHigh), nil) + got := chatprovider.ApplyReasoningEffort(chatprovider.NewModel(&chattest.FakeModel{ProviderName: fantasyopenai.Name, ModelName: "gpt-5"}, nil), options, new(codersdk.ChatModelReasoningEffortHigh)) providerOptions, ok := got[fantasyopenai.Name].(*fantasyopenai.ResponsesProviderOptions) require.True(t, ok, "%T", got[fantasyopenai.Name]) require.Same(t, options[fantasyopenai.Name], providerOptions) @@ -119,7 +119,7 @@ func TestApplyReasoningEffort(t *testing.T) { ParallelToolCalls: ptr.Ref(true), }, } - got := chatprovider.ApplyReasoningEffort(&chattest.FakeModel{ProviderName: fantasyopenai.Name, ModelName: "gpt-4"}, options, new(codersdk.ChatModelReasoningEffortHigh), nil) + got := chatprovider.ApplyReasoningEffort(chatprovider.NewModel(&chattest.FakeModel{ProviderName: fantasyopenai.Name, ModelName: "gpt-4"}, nil), options, new(codersdk.ChatModelReasoningEffortHigh)) providerOptions, ok := got[fantasyopenai.Name].(*fantasyopenai.ProviderOptions) require.True(t, ok, "%T", got[fantasyopenai.Name]) require.Same(t, options[fantasyopenai.Name], providerOptions) @@ -225,7 +225,7 @@ func TestApplyReasoningEffort(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { t.Parallel() - got := chatprovider.ApplyReasoningEffort(&chattest.FakeModel{ProviderName: tt.provider}, tt.options, new(codersdk.ChatModelReasoningEffortHigh), nil) + got := chatprovider.ApplyReasoningEffort(chatprovider.NewModel(&chattest.FakeModel{ProviderName: tt.provider}, nil), tt.options, new(codersdk.ChatModelReasoningEffortHigh)) tt.assert(t, got) }) } diff --git a/coderd/x/chatd/chatprovider/responses_api_test.go b/coderd/x/chatd/chatprovider/responses_api_test.go index 87991c8d36..ea1423370d 100644 --- a/coderd/x/chatd/chatprovider/responses_api_test.go +++ b/coderd/x/chatd/chatprovider/responses_api_test.go @@ -61,7 +61,7 @@ func TestModelFromConfig_OpenAIResponsesAPIOverride(t *testing.T) { chatprovider.UserAgent(), nil, nil, - tc.override, + &codersdk.ChatModelOpenAIConfig{UseResponsesAPI: tc.override}, ) require.NoError(t, err) @@ -80,36 +80,14 @@ func TestModelFromConfig_OpenAIResponsesAPIOverride(t *testing.T) { } } -func TestOpenAIResponsesAPIOverride(t *testing.T) { - t.Parallel() - - useResponsesAPI := true - - t.Run("NilConfig", func(t *testing.T) { - t.Parallel() - require.Nil(t, chatprovider.OpenAIResponsesAPIOverride(nil)) - }) - - t.Run("Unset", func(t *testing.T) { - t.Parallel() - require.Nil(t, chatprovider.OpenAIResponsesAPIOverride(&codersdk.ChatModelOpenAIConfig{})) - }) - - t.Run("Set", func(t *testing.T) { - t.Parallel() - got := chatprovider.OpenAIResponsesAPIOverride(&codersdk.ChatModelOpenAIConfig{ - UseResponsesAPI: &useResponsesAPI, - }) - require.NotNil(t, got) - require.True(t, *got) - }) -} - -// When other OpenAI options exist, the override must select the struct type -// the chosen API reads. -func TestProviderOptionsFromChatModelConfig_ResponsesAPIOverride(t *testing.T) { +// The wire path the client actually uses, the provider option struct type, and +// file-part acceptance must all agree, because a mismatch is silent: the SDK +// type-asserts the concrete option struct, and Responses accepts only images +// and PDFs natively. +func TestModelTransportConsumersAgree(t *testing.T) { t.Parallel() + // Taken from opposite sides of the provider SDK's known-model list. const responsesModel = "gpt-4o" const nonResponsesModel = "babbage-002" @@ -117,58 +95,90 @@ func TestProviderOptionsFromChatModelConfig_ResponsesAPIOverride(t *testing.T) { forceCompletions := false serviceTier := "auto" - t.Run("ForceResponsesUsesResponsesOptions", func(t *testing.T) { - t.Parallel() - got := chatprovider.ProviderOptionsFromChatModelConfig( - &chattest.FakeModel{ProviderName: fantasyopenai.Name, ModelName: nonResponsesModel}, - &codersdk.ChatModelProviderOptions{ + cases := []struct { + name string + modelID string + override *bool + wantPath string + wantOptions fantasy.ProviderOptionsData + wantAcceptText bool + }{ + { + name: "ForceResponsesOnUnknownModel", + modelID: nonResponsesModel, + override: &forceResponses, + wantPath: "/responses", + wantOptions: &fantasyopenai.ResponsesProviderOptions{}, + }, + { + name: "ForceCompletionsOnKnownModel", + modelID: responsesModel, + override: &forceCompletions, + wantPath: "/chat/completions", + wantOptions: &fantasyopenai.ProviderOptions{}, + wantAcceptText: true, + }, + { + name: "UnsetFollowsKnownModelList", + modelID: responsesModel, + wantPath: "/responses", + wantOptions: &fantasyopenai.ResponsesProviderOptions{}, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + var mu sync.Mutex + var gotPath string + serverURL := chattest.NewOpenAI(t, func(req *chattest.OpenAIRequest) chattest.OpenAIResponse { + mu.Lock() + gotPath = req.Request.URL.Path + mu.Unlock() + return chattest.OpenAINonStreamingResponse("ok") + }) + + model, err := chatprovider.ModelFromConfig( + fantasyopenai.Name, + tc.modelID, + chatprovider.ProviderAPIKeys{ + ByProvider: map[string]string{fantasyopenai.Name: "test-key"}, + BaseURLByProvider: map[string]string{fantasyopenai.Name: serverURL}, + }, + chatprovider.UserAgent(), + nil, + nil, + &codersdk.ChatModelOpenAIConfig{UseResponsesAPI: tc.override}, + ) + require.NoError(t, err) + + _, err = model.LanguageModel().Generate(context.Background(), fantasy.Call{ + Prompt: []fantasy.Message{{ + Role: fantasy.MessageRoleUser, + Content: []fantasy.MessagePart{fantasy.TextPart{Text: "Test message"}}, + }}, + }) + require.NoError(t, err) + + mu.Lock() + require.Equal(t, tc.wantPath, gotPath) + mu.Unlock() + + options := chatprovider.ProviderOptionsFromChatModelConfig(model, &codersdk.ChatModelProviderOptions{ OpenAI: &codersdk.ChatModelOpenAIProviderOptions{ServiceTier: &serviceTier}, - }, - &forceResponses, - ) - require.IsType(t, &fantasyopenai.ResponsesProviderOptions{}, got[fantasyopenai.Name]) - }) + }) + require.IsType(t, tc.wantOptions, options[fantasyopenai.Name]) - t.Run("ForceCompletionsUsesCompletionsOptions", func(t *testing.T) { - t.Parallel() - got := chatprovider.ProviderOptionsFromChatModelConfig( - &chattest.FakeModel{ProviderName: fantasyopenai.Name, ModelName: responsesModel}, - &codersdk.ChatModelProviderOptions{ - OpenAI: &codersdk.ChatModelOpenAIProviderOptions{ServiceTier: &serviceTier}, - }, - &forceCompletions, - ) - require.IsType(t, &fantasyopenai.ProviderOptions{}, got[fantasyopenai.Name]) - }) -} - -// When the override is the only OpenAI option, ApplyReasoningEffort creates -// the provider options itself and must match the chosen API. -func TestApplyReasoningEffort_ResponsesAPIOverride(t *testing.T) { - t.Parallel() - - forceResponses := true - forceCompletions := false - - t.Run("ForceResponsesOnUnknownModel", func(t *testing.T) { - t.Parallel() - got := chatprovider.ApplyReasoningEffort( - &chattest.FakeModel{ProviderName: fantasyopenai.Name, ModelName: "babbage-002"}, - nil, - new(codersdk.ChatModelReasoningEffortHigh), - &forceResponses, - ) - require.IsType(t, &fantasyopenai.ResponsesProviderOptions{}, got[fantasyopenai.Name]) - }) - - t.Run("ForceCompletionsOnKnownModel", func(t *testing.T) { - t.Parallel() - got := chatprovider.ApplyReasoningEffort( - &chattest.FakeModel{ProviderName: fantasyopenai.Name, ModelName: "gpt-4o"}, - nil, - new(codersdk.ChatModelReasoningEffortHigh), - &forceCompletions, - ) - require.IsType(t, &fantasyopenai.ProviderOptions{}, got[fantasyopenai.Name]) - }) + effortOptions := chatprovider.ApplyReasoningEffort( + model, + nil, + new(codersdk.ChatModelReasoningEffortHigh), + ) + require.IsType(t, tc.wantOptions, effortOptions[fantasyopenai.Name]) + + require.Equal(t, tc.wantAcceptText, model.AcceptsFilePartMediaType("text/plain")) + require.True(t, model.AcceptsFilePartMediaType("image/png")) + }) + } } diff --git a/coderd/x/chatd/compaction_override.go b/coderd/x/chatd/compaction_override.go index 8d224073fb..3e2ec34386 100644 --- a/coderd/x/chatd/compaction_override.go +++ b/coderd/x/chatd/compaction_override.go @@ -42,9 +42,6 @@ type compactionModelOverride struct { // providerOptions include the override's reasoning effort for the // summary call. providerOptions fantasy.ProviderOptions - // openAIResponsesOverride keeps prompt sanitization aligned with the - // API the override model client was built for. - openAIResponsesOverride *bool } // resolvedCompactionOverride is the compaction override resolved at @@ -151,17 +148,16 @@ func (p *Server) buildCompactionOverrideModel( err, ) } - providerOptions, responsesOverride, err := compactionOverrideProviderOptions(model, modelConfig) + providerOptions, err := compactionOverrideProviderOptions(model, modelConfig) if err != nil { return compactionModelOverride{}, err } return compactionModelOverride{ - modelConfig: modelConfig, - model: model, - resolvedProvider: resolvedProvider, - resolvedModel: resolvedModel, - providerOptions: providerOptions, - openAIResponsesOverride: responsesOverride, + modelConfig: modelConfig, + model: model, + resolvedProvider: resolvedProvider, + resolvedModel: resolvedModel, + providerOptions: providerOptions, }, nil } @@ -171,30 +167,27 @@ func (p *Server) buildCompactionOverrideModel( func compactionOverrideProviderOptions( model chatprovider.Model, modelConfig database.ChatModelConfig, -) (fantasy.ProviderOptions, *bool, error) { +) (fantasy.ProviderOptions, error) { callConfig := codersdk.ChatModelCallConfig{} if len(modelConfig.Options) > 0 { if err := json.Unmarshal(modelConfig.Options, &callConfig); err != nil { - return nil, nil, xerrors.Errorf( + return nil, xerrors.Errorf( "parse compaction model override call config: %w", err, ) } } - responsesOverride := chatprovider.OpenAIResponsesAPIOverride(callConfig.OpenAIConfig) providerOptions := chatprovider.ProviderOptionsFromChatModelConfig( - model.LanguageModel(), + model, callConfig.ProviderOptions, - responsesOverride, ) reasoningEffort := chatprovider.ResolveReasoningEffort( nil, callConfig.ReasoningEffort, ) return chatprovider.ApplyReasoningEffort( - model.LanguageModel(), + model, providerOptions, reasoningEffort, - responsesOverride, - ), responsesOverride, nil + ), nil } diff --git a/coderd/x/chatd/compaction_override_internal_test.go b/coderd/x/chatd/compaction_override_internal_test.go index 390801d7fb..166263c3d5 100644 --- a/coderd/x/chatd/compaction_override_internal_test.go +++ b/coderd/x/chatd/compaction_override_internal_test.go @@ -26,7 +26,7 @@ func TestCompactionOverrideProviderOptions(t *testing.T) { t.Run("NoOptions", func(t *testing.T) { t.Parallel() - opts, _, err := compactionOverrideProviderOptions(model, database.ChatModelConfig{}) + opts, err := compactionOverrideProviderOptions(model, database.ChatModelConfig{}) require.NoError(t, err) require.Nil(t, opts) }) @@ -41,7 +41,7 @@ func TestCompactionOverrideProviderOptions(t *testing.T) { }, }) require.NoError(t, err) - opts, _, err := compactionOverrideProviderOptions(model, database.ChatModelConfig{Options: options}) + opts, err := compactionOverrideProviderOptions(model, database.ChatModelConfig{Options: options}) require.NoError(t, err) anthropicOpts, ok := opts[fantasyanthropic.Name].(*fantasyanthropic.ProviderOptions) require.True(t, ok) @@ -51,7 +51,7 @@ func TestCompactionOverrideProviderOptions(t *testing.T) { t.Run("MalformedOptions", func(t *testing.T) { t.Parallel() - _, _, err := compactionOverrideProviderOptions(model, database.ChatModelConfig{Options: []byte("{")}) + _, err := compactionOverrideProviderOptions(model, database.ChatModelConfig{Options: []byte("{")}) require.ErrorContains(t, err, "parse compaction model override call config") }) } diff --git a/coderd/x/chatd/compaction_sanitize.go b/coderd/x/chatd/compaction_sanitize.go index 43b0f0f5bf..7403c717ac 100644 --- a/coderd/x/chatd/compaction_sanitize.go +++ b/coderd/x/chatd/compaction_sanitize.go @@ -30,20 +30,12 @@ func sanitizeCompactionPrompt( compactionModel chatprovider.Model, chatConfig database.ChatModelConfig, overrideConfig database.ChatModelConfig, - openAIResponsesOverride *bool, ) []fantasy.Message { messages := prompt if !sameCompactionProviderIdentity(chatConfig, overrideConfig) { messages = flattenProviderExecutedToolParts(ctx, logger, messages) } - messages = replaceUnsupportedFileParts(ctx, logger, messages, func(mediaType string) bool { - return chatprovider.AcceptsFilePartMediaType( - compactionModel.Provider(), - compactionModel.ModelID(), - mediaType, - openAIResponsesOverride, - ) - }) + messages = replaceUnsupportedFileParts(ctx, logger, messages, compactionModel.AcceptsFilePartMediaType) sanitized, stats := chatsanitize.SanitizeAnthropicProviderToolHistory( compactionModel.Provider(), messages, diff --git a/coderd/x/chatd/compaction_sanitize_internal_test.go b/coderd/x/chatd/compaction_sanitize_internal_test.go index f39e650fef..9cff2d0366 100644 --- a/coderd/x/chatd/compaction_sanitize_internal_test.go +++ b/coderd/x/chatd/compaction_sanitize_internal_test.go @@ -72,7 +72,7 @@ func TestSanitizeCompactionPrompt_FlattensForeignProviderExecutedToolParts(t *te } compactionModel := chatprovider.NewModel(&chattest.FakeModel{ProviderName: "openai", ModelName: "gpt-4.1-mini"}, nil) - sanitized := sanitizeCompactionPrompt(ctx, logger, prompt, compactionModel, configWithProvider(uuid.New()), configWithProvider(uuid.New()), nil) + sanitized := sanitizeCompactionPrompt(ctx, logger, prompt, compactionModel, configWithProvider(uuid.New()), configWithProvider(uuid.New())) require.Len(t, sanitized, 3) // Provider-executed parts are flattened to text so the summary keeps @@ -121,7 +121,7 @@ func TestSanitizeCompactionPrompt_DropsNonAssistantProviderExecutedParts(t *test } compactionModel := chatprovider.NewModel(&chattest.FakeModel{ProviderName: "openai", ModelName: "gpt-4.1-mini"}, nil) - sanitized := sanitizeCompactionPrompt(ctx, logger, prompt, compactionModel, configWithProvider(uuid.New()), configWithProvider(uuid.New()), nil) + sanitized := sanitizeCompactionPrompt(ctx, logger, prompt, compactionModel, configWithProvider(uuid.New()), configWithProvider(uuid.New())) require.Len(t, sanitized, 1) require.Equal(t, fantasy.MessageRoleUser, sanitized[0].Role) @@ -151,7 +151,7 @@ func TestSanitizeCompactionPrompt_ReplacesUnsupportedFileParts(t *testing.T) { // placeholder while the prompt stays otherwise intact. compactionModel := chatprovider.NewModel(&chattest.FakeModel{ProviderName: "mistral", ModelName: "mistral-large"}, nil) sharedProviderID := uuid.New() - sanitized := sanitizeCompactionPrompt(ctx, logger, prompt, compactionModel, configWithProvider(sharedProviderID), configWithProvider(sharedProviderID), nil) + sanitized := sanitizeCompactionPrompt(ctx, logger, prompt, compactionModel, configWithProvider(sharedProviderID), configWithProvider(sharedProviderID)) require.Len(t, sanitized, 1) require.Len(t, sanitized[0].Content, 2) @@ -191,7 +191,7 @@ func TestSanitizeCompactionPrompt_SameProviderKeepsProviderExecutedParts(t *test compactionModel := chatprovider.NewModel(&chattest.FakeModel{ProviderName: "openai", ModelName: "gpt-4.1-mini"}, nil) sharedProviderID := uuid.New() - sanitized := sanitizeCompactionPrompt(ctx, logger, prompt, compactionModel, configWithProvider(sharedProviderID), configWithProvider(sharedProviderID), nil) + sanitized := sanitizeCompactionPrompt(ctx, logger, prompt, compactionModel, configWithProvider(sharedProviderID), configWithProvider(sharedProviderID)) require.Len(t, sanitized, 1) require.Len(t, sanitized[0].Content, 2) diff --git a/coderd/x/chatd/generation.go b/coderd/x/chatd/generation.go index 1f47b1decf..19cf109325 100644 --- a/coderd/x/chatd/generation.go +++ b/coderd/x/chatd/generation.go @@ -940,7 +940,6 @@ func (s *taskStarter) generateCompaction( overrideModel.model, prepared.Compaction.ChatModelConfig, overrideModel.modelConfig, - overrideModel.openAIResponsesOverride, ) } preResult, err := s.server.hooks.Trigger(ctx, chathooks.ChatFor(prepared.Chat, input.hookTurnID()), chathooks.Message{}, agenthooks.EventPreCompact, dispatch.CapacityClassGeneration) diff --git a/coderd/x/chatd/generation_preparer.go b/coderd/x/chatd/generation_preparer.go index 4112384f31..c40b9825a8 100644 --- a/coderd/x/chatd/generation_preparer.go +++ b/coderd/x/chatd/generation_preparer.go @@ -134,10 +134,6 @@ func (server *Server) prepareGeneration( debugEnabled = cuDebugEnabled resolvedProvider = cuResolvedProvider debugModel = cuResolvedModel - // The computer-use client is built without ConfigOptions, so the - // chat model's transport override must not follow the substituted - // model into provider option preparation. - callConfig.OpenAIConfig = nil } currentPlanMode := chat.PlanMode @@ -297,14 +293,7 @@ func (server *Server) prepareGeneration( // aibridge routing rewrites the provider (e.g. Bedrock to the // Anthropic transport). The conversion that actually drops or // accepts a file part is the one for model.Provider(). - acceptsFilePart := func(mediaType string) bool { - return chatprovider.AcceptsFilePartMediaType( - model.Provider(), - model.ModelID(), - mediaType, - chatprovider.OpenAIResponsesAPIOverride(callConfig.OpenAIConfig), - ) - } + acceptsFilePart := model.AcceptsFilePartMediaType providerType := string(modelRoute.Provider.Type) prompt, err = chatprompt.ConvertMessagesWithFiles(ctx, promptRows, server.chatFileResolver(providerType), logger, acceptsFilePart) if err != nil { @@ -557,17 +546,14 @@ func (server *Server) prepareGeneration( requestedEffort, callConfig.ReasoningEffort, ) - responsesOverride := chatprovider.OpenAIResponsesAPIOverride(callConfig.OpenAIConfig) providerOptions := chatprovider.ProviderOptionsFromChatModelConfig( - model.LanguageModel(), + model, callConfig.ProviderOptions, - responsesOverride, ) providerOptions = chatprovider.ApplyReasoningEffort( - model.LanguageModel(), + model, providerOptions, reasoningEffort, - responsesOverride, ) activeToolNames := activeToolNamesForTurn(tools, currentPlanMode, chat.ParentChatID, approvedPlanMCPConfigIDs) diff --git a/coderd/x/chatd/model_routing.go b/coderd/x/chatd/model_routing.go index d66f9111d1..a099b65157 100644 --- a/coderd/x/chatd/model_routing.go +++ b/coderd/x/chatd/model_routing.go @@ -10,6 +10,7 @@ import ( "github.com/coder/coder/v2/coderd/database" "github.com/coder/coder/v2/coderd/x/chatd/chatprovider" + "github.com/coder/coder/v2/codersdk" ) type modelClientRequest struct { @@ -45,7 +46,7 @@ func newLanguageModel( userAgent string, extraHeaders map[string]string, httpClient *http.Client, - openAIResponsesOverride *bool, + openAIConfig *codersdk.ChatModelOpenAIConfig, ) (chatprovider.Model, error) { model, err := chatprovider.ModelFromConfig( providerHint, @@ -54,7 +55,7 @@ func newLanguageModel( userAgent, extraHeaders, httpClient, - openAIResponsesOverride, + openAIConfig, ) if err != nil { return chatprovider.Model{}, err diff --git a/coderd/x/chatd/model_routing_aibridge.go b/coderd/x/chatd/model_routing_aibridge.go index 77af154250..0311c597f2 100644 --- a/coderd/x/chatd/model_routing_aibridge.go +++ b/coderd/x/chatd/model_routing_aibridge.go @@ -178,7 +178,7 @@ func (p *Server) newModel( req.UserAgent, extraHeaders, &http.Client{Transport: baseRT}, - chatprovider.OpenAIResponsesAPIOverride(callConfig.OpenAIConfig), + callConfig.OpenAIConfig, ) } diff --git a/coderd/x/chatd/model_routing_internal_test.go b/coderd/x/chatd/model_routing_internal_test.go index c85ebe5aab..fce8a291dc 100644 --- a/coderd/x/chatd/model_routing_internal_test.go +++ b/coderd/x/chatd/model_routing_internal_test.go @@ -20,6 +20,7 @@ import ( "github.com/coder/coder/v2/coderd/database" "github.com/coder/coder/v2/coderd/database/dbmock" "github.com/coder/coder/v2/coderd/x/chatd/chaterror" + "github.com/coder/coder/v2/coderd/x/chatd/chatopenai" "github.com/coder/coder/v2/coderd/x/chatd/chatprovider" "github.com/coder/coder/v2/coderd/x/chatd/chattool" "github.com/coder/coder/v2/codersdk" @@ -658,6 +659,45 @@ func TestAIBridgeComputerUseModelUsesRoute(t *testing.T) { require.Equal(t, aibridge.SourceAgents, factory.source) } +// The computer-use model is a hardcoded default with no config of its own, so +// its transport must come from its own client rather than inheriting the chat +// model's openai_config. Request preparation reads the same value back. +func TestResolveComputerUseModel_TransportIndependentOfChatConfig(t *testing.T) { + t.Parallel() + + providerID := uuid.New() + factory := &aibridgeTestFactory{rt: roundTripFunc(func(*http.Request) (*http.Response, error) { + t.Fatal("computer use model construction must not send a request") + return nil, xerrors.New("unreachable") + })} + chat := database.Chat{ID: uuid.New(), OwnerID: uuid.New()} + server := &Server{aibridgeTransportFactory: aibridgeTestFactoryPointer(factory)} + + provider := codersdk.ChatComputerUseProviderOpenAI + modelProvider, modelName, ok := chattool.DefaultComputerUseModel(provider) + require.True(t, ok) + + //nolint:dogsled // Only the built model matters for the transport assertion. + model, _, _, _, err := server.resolveComputerUseModel( + t.Context(), + chat, + aibridgeTestRoute(aibridgeTestAIProvider(providerID, "primary-openai", database.AIProviderTypeOpenai)), + provider, + modelProvider, + modelName, + modelBuildOptions{ActiveAPIKeyID: uuid.NewString()}, + ) + require.NoError(t, err) + + wantTransport := chatopenai.TransportFor(modelProvider, modelName, nil) + require.Equal(t, wantTransport, model.Transport()) + + // The assertion above only has teeth if an override could have changed the + // result for this model. + opposite := !wantTransport.UsesResponses() + require.NotEqual(t, wantTransport, chatopenai.TransportFor(modelProvider, modelName, &opposite)) +} + func TestResolveComputerUseModel_AIGatewayMissingAPIKeyID(t *testing.T) { t.Parallel() diff --git a/coderd/x/chatd/quickgen.go b/coderd/x/chatd/quickgen.go index 43374403c3..653a830583 100644 --- a/coderd/x/chatd/quickgen.go +++ b/coderd/x/chatd/quickgen.go @@ -425,17 +425,14 @@ func (p *Server) titleGenerationProviderOptions( ) } } - responsesOverride := chatprovider.OpenAIResponsesAPIOverride(callConfig.OpenAIConfig) providerOptions := chatprovider.ProviderOptionsFromChatModelConfig( - model.LanguageModel(), + model, callConfig.ProviderOptions, - responsesOverride, ) return chatprovider.ApplyReasoningEffort( - model.LanguageModel(), + model, providerOptions, chatprovider.ResolveReasoningEffort(nil, callConfig.ReasoningEffort), - responsesOverride, ) }