refactor(providers): batch/file facet surfaces and bailian chat-adapter hook#490
Conversation
…er 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>
|
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 (1)
📝 WalkthroughWalkthroughAdds reusable batch/file delegation surfaces for OpenAI-compatible providers, reuses them in Groq and xAI, and changes Bailian chat request adaptation to a hook-based function that returns an error and is wired through provider configuration. ChangesProvider Surfaces and Adapter Refactor
Estimated code review effort: 4 (Complex) | ~45 minutes 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! |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@internal/providers/bailian/bailian.go`:
- Around line 34-46: The doc comment for Provider no longer matches the
implementation because BatchSurface and FileSurface are embedded, so batch/file
methods are not explicitly delegated anymore. Update the comment near Provider
to say that batch/file capabilities are exposed through embedded surfaces while
chat/passthrough behavior remains delegated through the compatible provider.
Keep the wording aligned with the actual fields on Provider and the delegation
pattern used by compatible.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: f9ee382b-8df4-4f05-a230-d018f441ff38
📒 Files selected for processing (5)
internal/providers/bailian/bailian.gointernal/providers/bailian/bailian_test.gointernal/providers/groq/groq.gointernal/providers/openai/surfaces.gointernal/providers/xai/xai.go
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
What
Phase 2 of the provider consolidation started in #486. Two changes:
1. Batch/file facet surfaces. After #486, three packages (groq, bailian, xai) carried verbatim copies of the same ten batch/file delegation one-liners — past the "repeated pattern justifies the abstraction" bar. New embeddable facets in the openai package:
openai.BatchSurface— exactly thecore.NativeBatchProvidermethod set (compile-guarded)openai.FileSurface— exactly thecore.NativeFileProvidermethod set (compile-guarded)Partial-surface providers embed the facets they support; the router's interface-assertion capability discovery sees precisely the advertised set, and a future batch/file endpoint added to
CompatibleProvidergets wired in one place (the facet) instead of three copies.2. bailian's chat adapter moves into the engine. The
max_tokens→max_completion_tokensmapping (Bailian deprecatedmax_tokensin April 2026) becomes theAdaptChatRequesthook from #486.ChatCompletion/StreamChatCompletionbecome plain delegation, and Responses-via-chat picks the adaptation up through the engine rather than relying on the "override chat ⇒ must override responses" discipline. The raw passthrough-body adaptation (untyped JSON) is unchanged.Also drops bailian's redundant
resp.Provider = "bailian"re-stamping in file methods — the shared engine already stamps its configured provider name.Deliberately not done
The architecture review flagged the openai-vs-vllm
passthrough_semanticsfiles as near-dupes. Verified against current code: they are 9–10-line data tables whose every line differs (provider-prefixed operation names; vllm adds/completions). azure/openrouter already reuse openai's enricher where the tables are truly identical. Nothing to share — skipped.User-visible impact
None. Every delegated body was a verbatim copy of the engine method it now calls; the bailian adapter's behavior (including respect-existing-
max_completion_tokensprecedence and forward-original-on-error) is preserved, with its unit tests updated only for the new(req, error)hook signature.Wrapper sizes: groq 161→122, bailian 282→232 (the two adapters are the irreducible quirk), xai 269→213.
Tests
Full suite,
-raceoninternal/providers/..., lint 0 issues, pre-commit green. Facets are compile-guarded against the capability interfaces and exercised through the existing groq/bailian/xai httptest suites.Remaining phase (separate PR): remove the 16 test-only
NewWithHTTPClientconstructors.🤖 Generated with Claude Code
Summary by CodeRabbit
max_tokenstomax_completion_tokensfor compatible providers.max_completion_tokensvalue.