Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
APIModeselection with deterministic chat/responses/auto behaviorWithUseResponsesAPI()as a backward-compatible heuristic shim and tighten provider-option type guardsValidation
go test ./providers/openai ./providers/azurego test ./providertests -run 'TestOpenAI|TestAzure'go test ./... -count=1go vet ./...📋 Implementation Plan
Recommended path
APIModeenum toproviders/openai, exposed viaWithAPIMode(APIMode), with three caller-facing values:Auto,ChatCompletions, andResponses. Keep the provider's unconfigured behavior unchanged by storing "mode explicitly set" separately from the enum internally; if callers do nothing, Fantasy must follow today's exact path: instantiate the Chat Completions language model and never opt into theIsResponsesModel(modelID)heuristic.WithUseResponsesAPI()working as a backward-compatible heuristic shim equivalent toWithAPIMode(APIModeAuto). Do not repurpose it to mean "force Responses," because that would silently change existing behavior.providers/openai/provider.LanguageModel()to switch on the effective mode:Responses=> always build the Responses implementation;ChatCompletionsor legacy default => always build the Chat Completions implementation;Auto=> useIsResponsesModel(modelID)only as a heuristic. ForcedResponsesmust bypass the allowlist entirely.api_mode=responses|chat_completionsto Fantasy without recreating the current mismatch where the language-model instance and provider-options type disagree.Verified repo facts informing this plan
providers/openai/openai.gocurrently creates a Responses language model only when bothuseResponsesAPIis enabled andIsResponsesModel(modelID)returns true.providers/azure/azure.gois a thin wrapper overopenai.New(...), so Azure currently inherits OpenAI's exact selection logic instead of maintaining a separate implementation.1. API design recommendation
Recommendation
Adopt a provider-level API-mode selector as the primary public API:
Why provider-level, not model-level
fantasy.LanguageModelimplementation gets constructed inLanguageModel(...), so provider construction is the natural control point.Why
WithAPIMode(...)is better thanWithForceResponsesAPI()/WithForceChatCompletionsAPI()responses,chat_completions, and optionallyauto).WithUseResponsesAPI()+WithForceResponsesAPI()+WithForceChatCompletionsAPI()).Backward compatibility stance
WithUseResponsesAPI()working and define it as the heuristic/auto path.WithAPIMode(...).WithUseResponsesAPI()remains supported for compatibility.Implementation note for preserving defaults
Because
APIModeAutois a new opt-in behavior while today's unconfigured behavior is Chat Completions, the implementation must keep an internal distinction between:IsResponsesModel(modelID)APIModeChatCompletions=> also use Chat Completions, but now intentionallyAPIModeAuto=> use the current allowlist heuristicAPIModeResponses=> force the Responses implementationUse a pointer field or
apiModeSet bool; do not let the enum's zero/uninitialized state silently meanAPIModeAuto. Also avoid keeping both a legacy boolean and a new enum long-term.Zero-value / empty-string recommendation
Do not make
APIModeAutoequal to"".Rationale:
APIModeAutothe empty string would blur the line between "caller explicitly asked for heuristic mode" and "caller omitted the new field entirely", which is the opposite of what backward compatibility needs.APIModeAutoas the explicit string"auto"makes logs, docs, validation, and config parsing clearer.If a downstream config struct wants a zero-value-friendly string field, the recommended mapping is:
""/ omitted => do not callWithAPIMode(...); preserve legacy default behavior"auto"=> callWithAPIMode(APIModeAuto)"chat_completions"=> callWithAPIMode(APIModeChatCompletions)"responses"=> callWithAPIMode(APIModeResponses)That keeps the public API explicit while still letting downstream config structs add a backward-compatible string field.
2. Selection behavior in
LanguageModel()Recommended selection rules
Normalize selection to one helper owned by
providers/openai/openai.go, e.g.effectiveAPIMode(modelID string)orshouldUseResponses(modelID string).Expected behavior:
APIModeChatCompletionsIsResponsesModel(modelID)would return true.APIModeResponsesIsResponsesModel(modelID)returns false or the model ID is brand-new and not yet in Fantasy's allowlist.APIModeAutoIsResponsesModel(modelID)as a heuristic exactly asWithUseResponsesAPI()does today.Allowlist interaction
IsResponsesModel(modelID)should remain an auto-mode heuristic, not a global capability gate.api_mode=responsesenforceable for arbitrary or newly introduced model IDs.Validation stance
Use defensive validation for configuration shape, not for speculative model capability:
WithAPIMode(...)received a known enum value.APIModeResponsessolely becauseIsResponsesModel(modelID)is false.That tradeoff is important because a local allowlist cannot be the source of truth for unknown future IDs like
gpt-5.4.3. Provider-options compatibility plan
Goal
Guarantee that the selected language-model implementation and the accepted provider-options type stay aligned:
*openai.ProviderOptions*openai.ResponsesProviderOptionsRecommended change
Centralize the typed extraction/validation logic used by both model implementations so the errors are explicit and consistent.
Concretely:
LanguageModel()), because provider options are attached per call.providers/openai/language_model.go/language_model_hooks.go) to fail with a clearer, mode-aware message when it receives*ResponsesProviderOptions.providers/openai/responses_language_model.go) to fail symmetrically when it receives*ProviderOptions.Why this matters even after the mode fix
The new API-mode forcing will prevent Coder from selecting the wrong language-model implementation, but Fantasy should still guard against callers manually passing the wrong option struct on individual requests.
4. Azure implications
Recommendation
OpenAI and Azure should share the same API-mode forcing mechanism.
Reasoning:
Concrete scope
providers/openai.Known risk to note, but not solve in the first pass
Azure may eventually diverge from OpenAI in which models support the Responses API. That matters for auto mode heuristics, but it should not block this change:
Responsesshould still bypass the allowlist for both providersChatCompletionsshould still be deterministic for both providersAPIModemechanism without changing the forcing API again5. Definition of done / acceptance criteria
The implementation should be considered complete only when all of the following are true:
providers/openaiexposes a public API that can express three caller intents unambiguously:IsResponsesModel(modelID)unless callers explicitly opt into heuristic mode.WithUseResponsesAPI()keeps working and remains heuristic-only; it does not become a forcing API.provider.LanguageModel(...)deterministically selects:IsResponsesModel(modelID)entirely, so unknown or newly released model IDs can still be forced onto the Responses implementation.6. TDD implementation plan
Phase Red — write failing tests first
6.1 Package-local OpenAI unit tests (same package, no network)
Create focused tests under
providers/openai/— ideally a dedicatedapi_mode_test.goin the sameopenaipackage so tests can assert on unexported concrete types without widening the public API.Add failing tests for these cases first:
TestLanguageModel_DefaultModeUsesChatCompletionsLanguageModel(ctx, allowlistedResponsesModelID)where the model ID currently returns true fromIsResponsesModel(...).TestLanguageModel_WithUseResponsesAPI_RemainsHeuristicOnlyWithUseResponsesAPI().TestLanguageModel_WithAPIModeAuto_MatchesLegacyHeuristicWithAPIMode(APIModeAuto).WithUseResponsesAPI()for both allowlisted and non-allowlisted model IDs.TestLanguageModel_WithAPIModeResponses_BypassesAllowlistWithAPIMode(APIModeResponses).IsResponsesModel(...).TestLanguageModel_WithAPIModeChatCompletions_BypassesAllowlistWithAPIMode(APIModeChatCompletions).IsResponsesModel(...).TestWithAPIMode_RejectsUnknownValue6.2 Provider-options compatibility unit tests
Add failing tests that exercise the lowest-level call/prepare path that currently type-asserts provider options, without relying on live network calls.
Required cases:
*openai.ProviderOptions*openai.ResponsesProviderOptionswith a clear error mentioning the expected type and API mode*openai.ResponsesProviderOptions*openai.ProviderOptionswith a clear error mentioning the expected type and API modeImplementation note for the eventual code author: prefer unit-testing the shared prepare/extraction helper directly if one exists after refactoring; otherwise test the smallest existing function that performs the type assertion (
language_model_hooks.goandresponses_language_model.goare the currently verified hotspots).6.3 Azure parity tests
Cover Azure in the cheapest layer that gives confidence:
providertests/and keep package-local selection tests concentrated inproviders/openai, since Azure is a thin wrapper overopenai.New(...).The minimum failing Azure/provider cases should prove:
WithUseResponsesAPI()vsWithAPIMode(...)Phase Green — implement the minimum code to satisfy the tests
6.4 Public API and internal state
Primary file:
providers/openai/openai.goAPIModetype plus constants forAuto,ChatCompletions, andResponses.WithAPIMode(mode APIMode) Optionas the new primary public API.useResponsesAPI boolwith a representation that can distinguish:WithUseResponsesAPI()as a backward-compatible shim that sets explicit auto mode.6.5 Selection logic
Primary file:
providers/openai/openai.goeffectiveAPIMode(modelID string)orselectLanguageModelKind(modelID string).APIModeChatCompletions=> chat-completions implementationAPIModeResponses=> responses implementationAPIModeAuto=> useIsResponsesModel(modelID)heuristicIsResponsesModel(modelID)confined to the auto path; do not let it veto explicit Responses mode.6.6 Provider-options guards
Primary files:
providers/openai/language_model.goproviders/openai/language_model_hooks.goproviders/openai/responses_language_model.goproviders/openai/provider_options.goorproviders/openai/responses_options.goif a shared helper is the clearest home6.7 Azure wrapper surface
Primary file:
providers/azure/azure.goPhase Refactor — consolidate and document the solution after green
This phase is required even if the first working implementation is small.
providers/openai/openai.go.WithAPIMode(...)is the deterministic APIWithUseResponsesAPI()is heuristic-only compatibility behaviorIsResponsesModel(...)is an allowlist heuristic, not a capability gate for forced modeproviders/azure, stop and pull the logic back into the shared OpenAI path.Phase Verify — run automated checks and review the contract
Use the repo's verified Go/Task entry points when implementation happens:
go test ./providers/openai -vgo test ./providers/azure -vgo test ./providertests -v -run "TestOpenAI|TestAzure"go test ./... -count=1task linttask fmtif code formatting changes are introducedVerification checklist for the implementer:
7. Specific files and symbols likely to change
Core implementation files
providers/openai/openai.gooptionsstructWithUseResponsesAPI()APIModetype/constantsWithAPIMode(...)func (o *provider) LanguageModel(...)providers/openai/language_model.goproviders/openai/language_model_hooks.go*openai.ProviderOptionsassertion path and error textproviders/openai/responses_language_model.goproviders/openai/provider_options.goproviders/openai/responses_options.goIsResponsesModel(...)documentation and any helper reuse that keeps the heuristic clearly scoped to auto modeproviders/azure/azure.goTest files
providers/openai/api_mode_test.go(recommended new focused test file)providers/openai/openai_test.goonly if colocating with the current suite is cleaner than a new fileprovidertests/openai_test.goprovidertests/openai_responses_test.goprovidertests/azure_test.goprovidertests/azure_responses_test.go8. Risks / compatibility concerns
Default behavior drift
Silent semantic change to
WithUseResponsesAPI()Allowlist bypass surprise
Azure heuristic drift over time
Option-type mismatch can still be user-induced
Over-abstraction during refactor
9. Coder follow-up once Fantasy support exists
Recommended downstream follow-up:
api_modehandling with an explicit config-boundary mapping:WithAPIMode(...); preserve today's default behaviorresponses=>openai.WithAPIMode(openai.APIModeResponses)chat_completions=>openai.WithAPIMode(openai.APIModeChatCompletions)auto=>openai.WithAPIMode(openai.APIModeAuto)or the legacyWithUseResponsesAPI()shimapi_mode=responses|chat_completionsoverride once Fantasy lands this API; the override becomes truly enforceable instead of being filtered through Fantasy's model allowlist.Small nuance if Coder later adds
autoIf Coder later exposes a third
autovalue, it should either:That nuance does not block the immediate
responses|chat_completionsfix.10. Recommended implementation order
APIMode+WithAPIMode(...)and convertWithUseResponsesAPI()into the explicit-auto compatibility shim.LanguageModel(...)to use the new mode-resolution helper.Generated with
mux• Model:openai:gpt-5.4• Thinking:high