From 843754a547342eabcb972d25bcefb91333c1270c Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Thu, 2 Jul 2026 11:22:56 +0100 Subject: [PATCH] refactor(coderd/x/chatd): remove dead model-routing dispatch shim (#26942) Follow-up to #26862 ("remove direct chat routing"), which collapsed the routing discriminated union into a single `aiGatewayModelRoute` but left a one-path dispatch shim behind in `model_routing.go`. Removes `resolveModelRouteForConfig`/`resolveModelRouteForProviderType`/`newModel` wrapper functions that did nothing but call their `*AIGateway*` counterparts, and renames the `*AIGateway*` targets to take over those names directly. Also collapses a redundant if/else in `title_override.go` where both branches called the same function with the same effective argument, and has `chatutil.NormalizedStringPointer` delegate to the existing `coderd/util/strings.EmptyToNil` instead of reimplementing empty-string-to-nil logic. No behavior change.
Investigation notes / decision log Two independent read-only investigations were run over `coderd/x/chatd` looking for cleanup opportunities following #26862: one focused on residue from that PR specifically, one a general over-engineering pass on the whole package. Both independently converged on the `model_routing.go` shim as the top finding (verified zero divergent call sites). Other candidates considered and explicitly deferred/rejected for this PR: - Renaming away the vestigial `AIGateway` prefix package-wide: cosmetic-only, touches many call sites, skipped. - Inlining the `chatcost` subpackage into `chatd`: unrelated to #26862, skipped. - Deleting the deprecated `AIGatewayRoutingEnabled` deployment flag: confirmed dead/no-op, but intentionally kept as a back-compat shim per #26862; removal should follow the same deprecation cadence as other deprecated deployment options, as a separate, differently-timed change. - Folding `chatutil` entirely into `chatprovider`/`chatopenai`: `NormalizedStringPointer` overlapped with `coderd/util/strings.EmptyToNil` (now reused), but `NormalizedEnumValue` has no equivalent elsewhere in the repo and still has 2 real call sites, so the package stays.
--- Generated by Coder Agents on behalf of @johnstcn. --- coderd/x/chatd/chatutil/chatutil.go | 12 ++++++------ coderd/x/chatd/model_routing.go | 25 ------------------------ coderd/x/chatd/model_routing_aibridge.go | 6 +++--- coderd/x/chatd/title_override.go | 13 +----------- 4 files changed, 10 insertions(+), 46 deletions(-) diff --git a/coderd/x/chatd/chatutil/chatutil.go b/coderd/x/chatd/chatutil/chatutil.go index 9158fbb598..b0ad218e24 100644 --- a/coderd/x/chatd/chatutil/chatutil.go +++ b/coderd/x/chatd/chatutil/chatutil.go @@ -1,6 +1,10 @@ package chatutil -import "strings" +import ( + "strings" + + stringutil "github.com/coder/coder/v2/coderd/util/strings" +) // NormalizedStringPointer trims a string pointer and returns nil for nil or // empty values. @@ -8,11 +12,7 @@ func NormalizedStringPointer(value *string) *string { if value == nil { return nil } - trimmed := strings.TrimSpace(*value) - if trimmed == "" { - return nil - } - return &trimmed + return stringutil.EmptyToNil(strings.TrimSpace(*value)) } // NormalizedEnumValue returns the canonical allowed value matching value after diff --git a/coderd/x/chatd/model_routing.go b/coderd/x/chatd/model_routing.go index 6300a09465..8e57e78598 100644 --- a/coderd/x/chatd/model_routing.go +++ b/coderd/x/chatd/model_routing.go @@ -52,31 +52,6 @@ func (p *Server) enabledAIProviderByID(ctx context.Context, providerID uuid.UUID return provider, nil } -func (p *Server) resolveModelRouteForConfig( - ctx context.Context, - ownerID uuid.UUID, - modelConfig database.ChatModelConfig, -) (aiGatewayModelRoute, error) { - return p.resolveAIGatewayModelRouteForConfig(ctx, ownerID, modelConfig) -} - -func (p *Server) resolveModelRouteForProviderType( - ctx context.Context, - ownerID uuid.UUID, - providerType string, -) (aiGatewayModelRoute, error) { - return p.resolveAIGatewayModelRouteForProviderType(ctx, ownerID, providerType) -} - -func (p *Server) newModel( - ctx context.Context, - req modelClientRequest, - route aiGatewayModelRoute, - opts modelBuildOptions, -) (fantasy.LanguageModel, error) { - return p.newAIGatewayModel(ctx, req, route, opts) -} - func newLanguageModel( providerHint string, modelName string, diff --git a/coderd/x/chatd/model_routing_aibridge.go b/coderd/x/chatd/model_routing_aibridge.go index 57c6838028..3179ab4ea5 100644 --- a/coderd/x/chatd/model_routing_aibridge.go +++ b/coderd/x/chatd/model_routing_aibridge.go @@ -110,7 +110,7 @@ func isOpenRouterLikeAIGatewayProvider(provider database.AIProvider) bool { return host == "openrouter.ai" || strings.HasSuffix(host, ".openrouter.ai") } -func (p *Server) newAIGatewayModel( +func (p *Server) newModel( _ context.Context, req modelClientRequest, route aiGatewayModelRoute, @@ -267,7 +267,7 @@ func (p *Server) resolveAIGatewayRoute( return newAIGatewayModelRoute(provider, modelProviderHint, auth), nil } -func (p *Server) resolveAIGatewayModelRouteForConfig( +func (p *Server) resolveModelRouteForConfig( ctx context.Context, ownerID uuid.UUID, modelConfig database.ChatModelConfig, @@ -279,7 +279,7 @@ func (p *Server) resolveAIGatewayModelRouteForConfig( return p.resolveAIGatewayRoute(ctx, ownerID, provider, string(provider.Type)) } -func (p *Server) resolveAIGatewayModelRouteForProviderType( +func (p *Server) resolveModelRouteForProviderType( ctx context.Context, ownerID uuid.UUID, providerType string, diff --git a/coderd/x/chatd/title_override.go b/coderd/x/chatd/title_override.go index f58ec0a17d..2d48b3c714 100644 --- a/coderd/x/chatd/title_override.go +++ b/coderd/x/chatd/title_override.go @@ -54,18 +54,7 @@ func (p *Server) resolveTitleGenerationModelOverride( chat.OwnerID, p.resolveModelConfigAndNormalizedProvider, func(ctx context.Context, ownerID uuid.UUID, aiProviderID uuid.UUID) (chatprovider.ProviderAPIKeys, error) { - if aiProviderID == uuid.Nil { - resolvedProviderKeys, err := p.resolveUserProviderAPIKeys(ctx, ownerID, uuid.Nil) - if err != nil { - return chatprovider.ProviderAPIKeys{}, err - } - return resolvedProviderKeys, nil - } - resolvedProviderKeys, err := p.resolveUserProviderAPIKeys(ctx, ownerID, aiProviderID) - if err != nil { - return chatprovider.ProviderAPIKeys{}, err - } - return resolvedProviderKeys, nil + return p.resolveUserProviderAPIKeys(ctx, ownerID, aiProviderID) }, modelOverrideFailureModeHard, )