mirror of
https://github.com/coder/coder.git
synced 2026-09-21 20:51:01 +08:00
refactor(coderd/x/chatd): read the OpenAI transport from the model (#27704)
Stacked on #27703. Provider option conversion, reasoning effort injection, and file part acceptance each recomputed the OpenAI wire format from `(provider, modelID, override)`. They now read it from `chatprovider.Model`, so a decision cannot drift from the client it was built for. `ProviderOptionsFromChatConfig` takes a `Transport`, `ApplyReasoningEffort` takes a `Model`, and `AcceptsFilePartMediaType` becomes a `Model` method. `UsesResponsesAPI` and `UsesResponsesOptions` are deleted. The override extraction is unexported and reachable only from `ModelFromConfig`, which now takes the model's `ChatModelOpenAIConfig` directly, removing the six scattered extractions at call sites. That also resolves the computer-use mismatch. The computer-use model is a hardcoded default with no config row of its own: its client was built without an override while request preparation applied the chat model's. Preparation now reads the computer-use model's own transport, so the two agree without one model's client settings following a different model. Passing the chat model's `openai_config` into the computer-use client would have made them agree on the wrong value. `TestModelTransportConsumersAgree` pins the invariant in one test: the HTTP path the client actually hits, the concrete provider option struct type, the type created by reasoning effort, and text/image file acceptance. > Mux prepared this PR on Mike's behalf.
This commit is contained in:
@@ -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.
|
||||
|
||||
|
||||
@@ -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{
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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
|
||||
}
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
|
||||
@@ -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 {
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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)),
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -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"}
|
||||
|
||||
@@ -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,
|
||||
}
|
||||
|
||||
@@ -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)
|
||||
})
|
||||
}
|
||||
|
||||
@@ -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"))
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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
|
||||
}
|
||||
|
||||
@@ -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")
|
||||
})
|
||||
}
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -178,7 +178,7 @@ func (p *Server) newModel(
|
||||
req.UserAgent,
|
||||
extraHeaders,
|
||||
&http.Client{Transport: baseRT},
|
||||
chatprovider.OpenAIResponsesAPIOverride(callConfig.OpenAIConfig),
|
||||
callConfig.OpenAIConfig,
|
||||
)
|
||||
}
|
||||
|
||||
|
||||
@@ -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()
|
||||
|
||||
|
||||
@@ -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,
|
||||
)
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user