Commit Graph
8 Commits
Author SHA1 Message Date
Michael Suchacz 9168dbf4d3 refactor(coderd/x/chatd): unify LLM model-call construction behind a resolver (#28145)
Centralizes every LLM client build in chatd behind one resolver and
makes all model calls behave the same, instead of preserving per-flow
quirks.

## Why

`chatd` built LLM clients in five places (standard turn, compaction
override, quickgen, title override, advisor), each repeating config
parsing, route resolution, client construction, and debug wrapping with
slightly different behavior. An earlier iteration of this PR centralized
construction but preserved every historical quirk behind
purpose-specific spec constructors and policy flags. Review concluded
the quirks themselves were the problem.

## What changed

- `Server.resolveModelCall` (`modelcall.go`) is the single pipeline from
a `modelCallSpec` to a ready client plus call metadata. Callers build
spec literals inline; the only spec fields are data: model source (chat
config, explicit config row, or fixed provider/model pair for computer
use), requested reasoning effort, chatd-scoped routing, and build
options.
- Uniform behavior for every call:
- Provider options are always derived. Whole-chat summaries, turn status
labels, and chat-model compaction summaries previously sent none; they
now follow the config like everything else.
- `MaxOutputTokens` always defaults to 32k when the config leaves it
unset. The compaction summary call nulls the cap after copying the
template because it is non-streaming and the Anthropic SDK rejects
non-streaming requests whose cap implies a completion over ten minutes.
- Debug recording always wraps the client when chat debug is enabled,
labeled with the resolved provider/model. The debug-rebuild paths
(`manualTitleDebugSpec`, `quickgenDebugSpec`) are deleted; debug helpers
now only create and finalize run records, since run association flows
through the generate context, not the model.
- Clients are always constructed with the configured model string and
unmutated route, so AI Gateway validation sees the name exactly as
configured. The resolved identity feeds metadata, logs, and debug labels
only.
- `resolvedModelCall.newCall` / `newObjectCall` are the only production
constructors of `fantasy.Call` / `fantasy.ObjectCall`; object calls now
carry the config's sampling parameters.
- Net: -240 lines.

## Validation

- Full `./coderd/x/chatd/...` suite green (PostgreSQL-backed).
- Red-green toggles verified for the three key unifications (option
derivation, token default, summary cap removal).
- ARCHITECTURE.md: prose the refactor made stale is deleted; no new
resolver documentation is added.

> Mux acted on Mike's behalf for this PR.
2026-08-20 20:54:52 +00:00
Michael Suchacz b5c9e8e471 refactor(coderd/x/chatd): carry the resolved OpenAI transport on a model wrapper (#27703)
Stacked on #27683.

The OpenAI wire format is decided when the client is built, then thrown
away, so downstream sites recompute it from `(provider, modelID,
override)`. Any disagreement fails silently: the SDK type-asserts the
concrete provider options struct and discards every OpenAI option, and
text attachments are dropped because Responses natively accepts only
images and PDFs.

This adds `chatprovider.Model`, which pairs a fantasy client with the
transport resolved from that client's own identity. Its fields are
unexported and only the constructor sets the transport, deriving it from
the client, so no caller can pick a transport that disagrees with the
client it wraps. `chatopenai.Transport`'s zero value is invalid and
panics when read rather than defaulting to a wire format, following the
existing precedent for construction invariants.

`Model` is threaded through construction, the resolve paths, and the
four struct fields that store a model for later request preparation.
Terminal call sites keep taking `fantasy.LanguageModel` and receive
`LanguageModel()`, which avoids a new package edge from `chatloop` and
`chatadvisor` into `chatprovider`.

No decisions move yet. The consumers still recompute the transport, and
`UsesResponsesAPI` now delegates to `TransportFor` so the two agree by
construction. #27704 makes the consumers read it from the model.

> Mux prepared this PR on Mike's behalf.
2026-08-04 07:46:32 +00:00
Cian Johnston 4936ff9808 refactor: deprecate AIGatewayRoutingEnabled, remove direct chat routing (#26862)
This PR removes the now-dead direct-routing code:

- Deletes the direct routing implementation.
- Collapses the resolvedModelRoute discriminated union into aiGatewayModelRoute.
- Removes the dead providerKeys cascade.
- Deletes the preferredShortTextCandidates quickgen function.
- Simplifies the advisor override error handling.
- Deprecates the AIGatewayRoutingEnabled deployment option. It is now a no-op so as to not break existing deployments on upgrade.

Once direct routing was gone, the AI Gateway became mandatory for chat, which surfaced gaps in how the product behaves with the gateway disabled:

- Exposes ai-gateway-enabled to the frontend via embedded page metadata.
- Disables the chat composer via the existing AgentSetupNotice when the gateway is disabled, for both new and existing chats.
- Fixes nil/typed-nil chatDaemon panics on startup and shutdown when gateway is disabled.
- Fixes chat WebSocket from retrying the still-gated stream endpoint forever when the gateway is disabled.
2026-07-01 20:15:03 +01:00
Cian Johnston cb6a75717c fix(coderd/x/chatd): bind goInflight contexts to server lifetime (#26811)
fix(coderd/x/chatd): bind goInflight contexts to server lifetime
- Add Server.inflightContext: WithoutCancel(reqCtx) bound to p.ctx via
  context.AfterFunc, so Close cancels in-flight work instead of blocking
  on the caller's timeout while a provider is unreachable.
- Apply at GenerateChatTitleAsync, finalizeSuccessfulTurnStatusLabelWithAfterFunc,
  setLastTurnSummaryAsync, clearLastTurnSummaryAsync, and scheduleDebugCleanup.
- Honor cleanupCtx in the debug retry-delay timer so cancellation lands
  promptly between attempts.

> 🤖
2026-06-30 10:51:59 +01:00
Hugo Dutka 684d904c00 fix(coderd/x/chatd): resolve inflight race (#26460)
Using `WaitGroup.Go` must be synchronized with `WaitGroup.Wait`
according to [go docs](https://pkg.go.dev/sync#WaitGroup.Go):

> If the WaitGroup is empty, Go must happen before a
[WaitGroup.Wait](https://pkg.go.dev/sync#WaitGroup.Wait).

There were a couple of places in chatd that violated this principle.
This was caught as a data race in
https://github.com/coder/internal/issues/1599. This PR ensures that all
functions that spawn inflight goroutines synchronize with each other.

I also noticed that inflight goroutines may be spawned after the server
is closed, which was surprising and looked like a bug. This PR therefore
also introduces a mechanism that disallows spawning inflight goroutines
after the server is closed, and ensures that any code that tries doing
it logs an error.

Closes https://github.com/coder/internal/issues/1599.
2026-06-17 18:29:43 +02:00
Hugo Dutka 4debd23cbb fix: chatd refactor (#26270)
Implements the chatd stabilization RFC.

Combines:
- https://github.com/coder/coder/pull/25908
- https://github.com/coder/coder/pull/25923
- https://github.com/coder/coder/pull/26109
- https://github.com/coder/coder/pull/26110
- https://github.com/coder/coder/pull/26111
- https://github.com/coder/coder/pull/26112
2026-06-12 13:33:12 +02:00
Michael Suchacz 8b1705eb65 feat: route chatd provider traffic through aibridge (#25629)
## Summary

Routes chatd model calls backed by concrete AI Provider rows through the
in-process aibridge transport by default, with deployment options to use
direct provider routing when AI Gateway is disabled or chat AI Gateway
routing is disabled.

- Splits model routing into common, direct provider, and AI Gateway
paths behind a single deployment-mode entry point.
- Builds chatd models through explicit request, route, and options data.
Active API key attribution is passed explicitly instead of being hidden
inside generic model construction.
- For AI Gateway BYOK routes, resolves the user's provider key in chatd,
forwards it through provider-specific auth headers, and sets
`X-Coder-AI-Governance-Token` to the `delegated` marker so aibridge
preserves those headers while still stripping Coder-specific metadata.
- Keeps central provider credentials and deployment fallback credentials
out of forwarded provider auth headers, so AI Gateway central policy
remains authoritative.
- Redacts delegated provider auth from default string formatting to
avoid accidental plaintext logging of user BYOK credentials.
- Covers selected chat models, advisor overrides, title and quickgen
paths, subagent overrides, computer use model selection, and an
integration-style chat turn through the aibridge transport path.
- Persists initiating API key IDs on chat and queued user messages,
including subagent child messages, and fails closed for AI
Gateway-routed model builds without an active key.
- Removes unused `api_key_id` indexes while keeping the persistence
columns and foreign keys.
- Keeps the deployment option available through config and env parsing,
but hides it from CLI help and generated docs.
- Stabilizes the subagent poll fallback test so background CreateChat
processing cannot win the state transition under slower CI environments.

## Tests

- `go test ./coderd/x/chatd -run
'TestAIGatewayProviderAuthForUser|TestAIGatewayProviderAuthRedactsFormatting|TestResolveModelRouteForConfigAIGatewayProviderAuth|TestAIGatewayModelForwardsProviderAuth|TestProcessChat_AIGatewayRoutingUsesDelegatedAPIKey|TestAwaitSubagentCompletion'
-count=1`
- `go test ./coderd/aibridged -run
'TestServeHTTP_DelegatedAPIKey|TestServeHTTP_StripCoderToken' -count=1`
- `git diff --check HEAD~1..HEAD`
- `make lint`

> Mux working on behalf of Mike.
2026-05-26 19:31:52 +00:00
Thomas Kosiewski df7e838c21 feat(coderd): wire debug logging into chat lifecycle (#23917) 2026-04-20 12:27:16 +02:00