refactor(providers): route deepseek, xai, and ollama through the shared OpenAI-compatible core#486
Conversation
…ed OpenAI-compatible core deepseek, xai, and ollama were the last providers hand-rolling llmclient transport for OpenAI-shaped endpoints, re-implementing bodies that openai.CompatibleProvider already owns and silently drifting from it (no buffered-JSON SSE normalization on ollama streams, no models-list object defaulting on xai). - CompatibleProviderConfig gains two opt-in hooks so quirks stop forcing method overrides: AdaptChatRequest (typed request rewrite, picked up automatically by Responses-via-chat translation) and ChatRequestHeaders (context-derived per-request headers on chat calls). - deepseek embeds *ChatCompatible (its surface matches exactly); the reasoning-effort remap moves into the AdaptChatRequest hook. - xai composes *CompatibleProvider with explicit delegation, preserving its narrowed surface (no audio, passthrough, or response lifecycle management); the Grok conversation-affinity header becomes the ChatRequestHeaders hook. - ollama composes *CompatibleProvider for the /v1 side; the native /api/embed client, batch stubs, and CheckAvailability stay. - CompatibleProvider now documents the three sanctioned wrapper shapes (embed ChatCompatible / embed CompatibleProvider / compose + delegate) and why embedding must not be used to fake capabilities. Wire behavior is preserved except two conservative normalizations the other ten OpenAI-compatible providers already had: xai model listings get empty object fields defaulted, and ollama chat streams are wrapped by EnsureChatCompletionSSE (pass-through for genuine SSE; converts buffered-JSON replies from upstreams that ignore stream:true). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (8)
📝 WalkthroughWalkthroughThis PR refactors DeepSeek, Ollama, and xAI providers to delegate chat completion, streaming, model listing, responses, and other endpoints to a shared OpenAI-compatible adapter (openai.ChatCompatible/CompatibleProvider), which is extended with AdaptChatRequest and ChatRequestHeaders hooks. Related tests are updated accordingly. ChangesShared Compatible Provider Delegation
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Caller
participant CompatibleProvider
participant UpstreamAPI
Caller->>CompatibleProvider: ChatCompletion(req)
CompatibleProvider->>CompatibleProvider: adaptChatRequest(req)
alt adaptation succeeds
CompatibleProvider->>CompatibleProvider: chatHeaders(ctx, adaptedReq)
CompatibleProvider->>UpstreamAPI: POST /chat/completions
UpstreamAPI-->>CompatibleProvider: response
CompatibleProvider-->>Caller: ChatResponse
else adaptation fails
CompatibleProvider-->>Caller: error
end
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
…er hook (#490) * refactor(providers): batch/file facet surfaces and bailian chat-adapter hook Phase 2 of the provider consolidation (follow-up to #486). - New embeddable facets openai.BatchSurface and openai.FileSurface wrap *CompatibleProvider with exactly the core.NativeBatchProvider and core.NativeFileProvider method sets (compile-guarded). Partial-surface providers embed them instead of copying ten delegation methods each; the three verbatim copies in groq, bailian, and xai are gone. - bailian's max_tokens -> max_completion_tokens mapping moves into the AdaptChatRequest hook, so ChatCompletion/StreamChatCompletion become plain delegation and Responses-via-chat picks the adaptation up through the engine instead of relying on method-override discipline. The raw passthrough-body adaptation is unchanged. - bailian's file methods no longer re-stamp resp.Provider: the shared engine already stamps its configured provider name, so the second stamp was redundant. Not done, deliberately: the openai-vs-vllm passthrough_semantics "near-dupe" is two 9-line data tables whose every line differs (provider-prefixed operations; vllm adds /completions), so there is nothing to share. Wire behavior is unchanged; all delegated bodies were verbatim copies of the engine methods they now call. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * docs(bailian): align Provider doc with embedded batch/file surfaces Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
What
Kills the "full re-implementation" provider style (architecture review §4, roadmap item 5 phase 1). deepseek, xai, and ollama were the last three providers hand-rolling
llmclienttransport for OpenAI-shaped endpoints — ~350 lines re-doing whatopenai.CompatibleProvideralready owns, with real drift (see Behavior notes).How
CompatibleProviderConfigso provider quirks stop forcing method overrides:AdaptChatRequest func(*core.ChatRequest) (*core.ChatRequest, error)— typed request rewrite applied on chat + stream before dispatch. Because it runs inside the engine, Responses-via-chat translation picks it up automatically (removes the "override ChatCompletion ⇒ must also override Responses" trap).ChatRequestHeaders func(ctx, *core.ChatRequest) http.Header— context-derived per-request headers on chat calls only.*openai.ChatCompatible(its surface matches exactly: chat, models, responses-via-chat, embeddings-unsupported, passthrough). The reasoning-effort remap becomes theAdaptChatRequesthook. 199 → 101 lines.*openai.CompatibleProviderwith explicit delegation, because xAI's upstream lacks audio, passthrough, and response lifecycle management — embedding cannot subtract methods, and the router discovers capabilities by interface assertion. The Grok conversation-affinity header (X-Grok-Conv-Id) becomes theChatRequestHeadershook; generated conversation IDs are hash-identical to before (the anchor never included the stream flag). 368 → 269 lines, all remaining lines are genuine novelty.*openai.CompatibleProviderfor the/v1side; the native/api/embedclient, batch stubs, dual-URLSetBaseURL, andCheckAvailabilitystay. Still no passthrough.CompatibleProvidernow documents the three sanctioned wrapper shapes (embedChatCompatiblefor chat-centric / embedCompatibleProviderfor full-surface / compose + delegate for partial-surface) so the next provider doesn't invent a fourth style.User-visible impact
None intended. Wire behavior is preserved except two conservative normalizations the other ten OpenAI-compatible providers already had:
objectfields tolist/model(fill-if-empty only).EnsureChatCompletionSSE— pass-through for genuine SSE, converts buffered-JSON replies from upstreams that ignorestream:trueinto one SSE chunk +[DONE](protects the misconfigured-OpenAI-server-as-ollama case).o<digit>*/gpt-5*→max_completion_tokens, droptemperature), same as groq/vllm/bailian/azure/openrouter today. Real DeepSeek/Grok/Ollama model IDs never match; only a local model deliberately named like an OpenAI reasoning model would see it.Provider capability surfaces (unchanged)
Verified method-set parity: deepseek gains only the
GetBaseURLgetter (not a capability interface); xai and ollama gain nothing (delegation, not embedding).Tests
provider.client→provider.compat).-raceoninternal/providers/..., lint 0 issues, pre-commit green.Remaining phases (separate PRs): compress the groq/oracle/bailian/vllm delegation blocks (facet surfaces or hook migration for bailian's max_tokens adapter), then remove the 16 test-only
NewWithHTTPClientconstructors.🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
Refactor