feat(provider): add new provider gateway traits#16
Merged
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds a trait-based gateway abstraction: ChatFormat for request/response/stream formatting, provider meta/transform/capabilities traits, native-format support with a type-erased NativeHandler, explicit stream state types and tool-call accumulation, compatibility quirks, module re-exports, and documentation updates. (50 words) Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Gateway
participant Provider
participant Hub as "Hub/Backend"
Note over Gateway,Provider: ChatFormat + ProviderCapabilities used for transforms
Client->>Gateway: Submit ChatFormat::Request
Gateway->>Gateway: ChatFormat::is_stream / extract_model
Gateway->>Provider: build_url / transform_request
alt Provider exposes native support
Gateway->>Provider: native_support() -> NativeHandler
Gateway->>Provider: call_native (via NativeHandler)
Provider-->>Gateway: native stream chunks / final response
Gateway->>Gateway: transform_native_stream_chunk -> ChatFormat::StreamChunk
else Use hub (OpenAI-compatible)
Gateway->>Hub: POST transformed request
Hub-->>Gateway: hub streaming chunks / final response
Gateway->>Gateway: from_hub_stream / from_hub -> ChatFormat::StreamChunk/Response
end
Gateway-->>Client: Emit StreamChunk/Response (serialize_chunk_payload / SSE)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Pull request overview
Adds a new gateway-side trait stack to support the refactored provider crate, including capability discovery and optional “native bypass” paths for Anthropic Messages and OpenAI Responses.
Changes:
- Introduces
gateway::traitswithChatFormat, provider metadata/transform traits, and native support traits. - Adds
CompatQuirksto centralize small OpenAI-compatibility adjustments (param removal/rename, stream usage injection, done sentinel). - Updates internal docs and type-module doc comments to reflect the renamed native support traits.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/gateway/types/openai/responses.rs | Doc comment update to reference NativeOpenAIResponsesSupport. |
| src/gateway/types/anthropic.rs | Doc comment update to reference NativeAnthropicMessagesSupport. |
| src/gateway/traits/provider.rs | New provider trait layer (ProviderMeta, ChatTransform, ProviderCapabilities) and CompatQuirks. |
| src/gateway/traits/native.rs | New native bypass traits and native stream state types. |
| src/gateway/traits/mod.rs | Exposes the new trait modules and re-exports key types. |
| src/gateway/traits/chat_format.rs | Defines the ChatFormat contract and shared stream state helpers. |
| src/gateway/mod.rs | Exposes the new gateway::traits module. |
| docs/internals/llm-types.md | Documents the combined type + trait architecture and native bypass design. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/gateway/traits/provider.rs (1)
41-46: Normalize URL joining to avoid malformed endpoints.
build_urldepends onchat_endpoint_path()returning a leading slash. Normalizing both sides makes this robust across provider implementations.Suggested refactor
fn build_url(&self, base_url: &str, model: &str) -> String { - format!( - "{}{}", - base_url.trim_end_matches('/'), - self.chat_endpoint_path(model) - ) + let endpoint = self.chat_endpoint_path(model); + let endpoint = endpoint.as_ref().trim_start_matches('/'); + format!("{}/{}", base_url.trim_end_matches('/'), endpoint) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/gateway/traits/provider.rs` around lines 41 - 46, build_url currently concatenates base_url and chat_endpoint_path assuming the latter starts with '/', which can produce malformed endpoints; change build_url to normalize both parts by trimming trailing slashes from base_url and trimming leading slashes from the result of chat_endpoint_path(model), then join with a single '/' (and if chat_endpoint_path returns empty, return the trimmed base_url). Update the build_url implementation to use base_url.trim_end_matches('/') and chat_endpoint_path(model).trim_start_matches('/') and ensure you handle an empty path case so provider implementations no longer need to supply a leading slash.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/gateway/traits/chat_format.rs`:
- Around line 88-95: The default implementation of parse_native_response
currently panics via unreachable! which can crash request handling; change it to
return a proper Err value instead: return an appropriate typed error using the
crate's Result/Error type (the same error type used elsewhere in this module)
with a clear message like "parse_native_response called on a non-native format",
so callers get a recoverable failure; update the parse_native_response method
signature implementation (the function named parse_native_response that takes
&NativeHandler<'_> and body: Value and returns Result<Self::Response>) to
construct and return that error rather than invoking unreachable!.
In `@src/gateway/traits/provider.rs`:
- Around line 67-80: transform_stream_chunk currently tries to JSON-parse any
non-empty input which causes errors for SSE control lines and bare done signals;
update transform_stream_chunk (and use default_quirks()/stream_done_signal) to
first handle SSE control frames safely: if the trimmed raw equals the
stream_done_signal (e.g., "[DONE]") return Ok(vec![]); if the raw is an SSE
comment (starts with ":"), an event line (starts with "event:"), or does not
contain a "data:" payload then return Ok(vec![]) without parsing; only call
serde_json::from_str on the actual data payload extracted via
strip_prefix("data: "). Ensure error mapping remains GatewayError::Transform for
parse failures.
---
Nitpick comments:
In `@src/gateway/traits/provider.rs`:
- Around line 41-46: build_url currently concatenates base_url and
chat_endpoint_path assuming the latter starts with '/', which can produce
malformed endpoints; change build_url to normalize both parts by trimming
trailing slashes from base_url and trimming leading slashes from the result of
chat_endpoint_path(model), then join with a single '/' (and if
chat_endpoint_path returns empty, return the trimmed base_url). Update the
build_url implementation to use base_url.trim_end_matches('/') and
chat_endpoint_path(model).trim_start_matches('/') and ensure you handle an empty
path case so provider implementations no longer need to supply a leading slash.
🪄 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: CHILL
Plan: Pro
Run ID: 07e0c84a-191e-4a6c-95bf-95d3ee6c583d
📒 Files selected for processing (8)
docs/internals/llm-types.mdsrc/gateway/mod.rssrc/gateway/traits/chat_format.rssrc/gateway/traits/mod.rssrc/gateway/traits/native.rssrc/gateway/traits/provider.rssrc/gateway/types/anthropic.rssrc/gateway/types/openai/responses.rs
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
src/gateway/traits/provider.rs (1)
28-31: Normalize the join betweenbase_urlandchat_endpoint_path().The default path includes a leading slash, but this trait contract does not require one. An implementation returning
v1/chat/completionswould currently produce a broken URL.Suggested fix
fn build_url(&self, base_url: &str, model: &str) -> String { - format!( - "{}{}", - base_url.trim_end_matches('/'), - self.chat_endpoint_path(model) - ) + let path = self.chat_endpoint_path(model); + format!( + "{}/{}", + base_url.trim_end_matches('/'), + path.trim_start_matches('/') + ) }Also applies to: 41-46
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/gateway/traits/provider.rs` around lines 28 - 31, The URL join currently assumes the endpoint path starts with a slash which breaks when implementations return e.g. "v1/chat/completions"; update the join logic to normalize both sides by trimming any trailing '/' from the provider's base URL and any leading '/' from chat_endpoint_path (and other endpoint path methods) and then concatenate them with a single '/' between; locate usages of chat_endpoint_path (and the other endpoint path methods around lines 41-46) and replace the direct concatenation with this normalization to ensure a correct URL regardless of whether implementations include a leading slash.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/gateway/traits/chat_format.rs`:
- Around line 120-123: The tool_call_accumulators map in ChatStreamState
currently keys by a single usize which collides across different
ChunkToolCall::index values from different choices; change
ChatStreamState.tool_call_accumulators from HashMap<usize, ToolCallAccumulator>
to HashMap<(usize, usize), ToolCallAccumulator> (key = (choice_index,
tool_call_index)) and update every read/write site that indexes or inserts into
this map (where ChunkToolCall::index and the enclosing choice index are used —
e.g., during stream chunk handling and aggregation) to construct and use the
(choice_index, tool_call_index) tuple key so fragments from different choices do
not merge.
In `@src/gateway/traits/provider.rs`:
- Around line 16-21: The ProviderAuth enum currently derives Debug which will
expose the raw ApiKey(String); replace the auto-derived Debug with a manual
implementation that redacts secrets: remove #[derive(Debug)] from ProviderAuth
and implement std::fmt::Debug for ProviderAuth so the ApiKey variant prints
something like "ApiKey(REDACTED)" while the None variant prints "None"; keep or
re-add #[derive(Clone, Default)] so Clone and Default behavior remain unchanged
and ensure any logging uses the Debug impl to avoid leaking the actual key.
- Around line 170-173: The loop handling self.param_renames currently removes
the source and unconditionally inserts into the destination, which overwrites
any explicitly provided destination; change the logic in that loop (the block
using self.param_renames, map.remove and map.insert) to first check whether the
destination key (*to) already exists in map and only perform the rename insert
when the destination is not present (i.e., preserve caller-provided destination
values), otherwise leave the destination value untouched and drop or keep the
source as appropriate.
---
Nitpick comments:
In `@src/gateway/traits/provider.rs`:
- Around line 28-31: The URL join currently assumes the endpoint path starts
with a slash which breaks when implementations return e.g.
"v1/chat/completions"; update the join logic to normalize both sides by trimming
any trailing '/' from the provider's base URL and any leading '/' from
chat_endpoint_path (and other endpoint path methods) and then concatenate them
with a single '/' between; locate usages of chat_endpoint_path (and the other
endpoint path methods around lines 41-46) and replace the direct concatenation
with this normalization to ensure a correct URL regardless of whether
implementations include a leading slash.
🪄 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: CHILL
Plan: Pro
Run ID: 06cd525f-292e-47d9-9157-bf802686e815
📒 Files selected for processing (3)
src/gateway/traits/chat_format.rssrc/gateway/traits/native.rssrc/gateway/traits/provider.rs
🚧 Files skipped from review as they are similar to previous changes (1)
- src/gateway/traits/native.rs
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/gateway/traits/provider.rs (1)
76-102: Fail fast for non-SSE defaults to avoid silent empty streams.At Line 100–Line 102, any non-
data:chunk is silently ignored. That’s fine for SSE, but for providers usingStreamReaderKind::AwsEventStreamorStreamReaderKind::JsonArrayStream, a missing override can quietly drop all chunks. Consider guarding onstream_reader_kind()and returning a transform error for non-SSE defaults.♻️ Suggested change
fn transform_stream_chunk( &self, raw: &str, _state: &mut ChatStreamState, ) -> Result<Vec<ChatCompletionChunk>> { + if self.stream_reader_kind() != StreamReaderKind::Sse { + return Err(GatewayError::Transform( + "default transform_stream_chunk only supports SSE providers; override for non-SSE stream readers".to_string(), + )); + } + let quirks = self.default_quirks(); let trimmed = raw.trim(); let done_signal = quirks.stream_done_signal.trim();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/gateway/traits/provider.rs` around lines 76 - 102, In transform_stream_chunk, currently any non-"data:" chunk is silently ignored which breaks providers whose default stream_reader_kind() is not SSE; update the logic in transform_stream_chunk (and use self.default_quirks() and self.stream_reader_kind()) to detect when stream_reader_kind() != StreamReaderKind::Sse (e.g., AwsEventStream or JsonArrayStream) and, instead of returning Ok(vec![]) for a non-"data:" line, return an Err(...) using the same Result error type (a transform error) indicating unexpected chunk format so missing overrides fail fast; leave SSE behavior (skipping non-data lines) unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/gateway/traits/provider.rs`:
- Around line 76-102: In transform_stream_chunk, currently any non-"data:" chunk
is silently ignored which breaks providers whose default stream_reader_kind() is
not SSE; update the logic in transform_stream_chunk (and use
self.default_quirks() and self.stream_reader_kind()) to detect when
stream_reader_kind() != StreamReaderKind::Sse (e.g., AwsEventStream or
JsonArrayStream) and, instead of returning Ok(vec![]) for a non-"data:" line,
return an Err(...) using the same Result error type (a transform error)
indicating unexpected chunk format so missing overrides fail fast; leave SSE
behavior (skipping non-data lines) unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 7a52e3d3-b933-4a45-826a-84ce6a9d036a
📒 Files selected for processing (1)
src/gateway/traits/provider.rs
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/gateway/traits/chat_format.rs (1)
82-86: Consider adding a default implementation fortransform_native_stream_chunk.Unlike
call_nativeandparse_native_responsewhich provide defaults returning appropriate errors, this method is required. Formats that don't support native streaming must still implement a stub.Additionally, this method takes
&dyn ProviderCapabilitieswhile the other native methods take&NativeHandler<'_>. If this signature difference is intentional (e.g., the caller needs to query capabilities before knowing which native path to use), consider adding a brief doc comment explaining why.♻️ Suggested default implementation for consistency
/// Convert a native streaming chunk into zero or more chunks of this format. fn transform_native_stream_chunk( provider: &dyn ProviderCapabilities, raw: &str, state: &mut Self::NativeStreamState, - ) -> Result<Vec<Self::StreamChunk>>; + ) -> Result<Vec<Self::StreamChunk>> + where + Self: Sized, + { + let _ = (provider, raw, state); + Err(GatewayError::NativeNotSupported { + provider: provider.name().into(), + }) + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/gateway/traits/chat_format.rs` around lines 82 - 86, The trait forces implementors to provide transform_native_stream_chunk while other native methods (call_native, parse_native_response) have defaults; add a default implementation for transform_native_stream_chunk on the trait that returns an Err (or other appropriate "not supported" error) so formats that don't support native streaming can omit a stub, and add a brief doc comment on transform_native_stream_chunk explaining why it accepts &dyn ProviderCapabilities (versus &NativeHandler<'_> used by other native methods) if the caller must inspect capabilities before selecting the native path; reference the transform_native_stream_chunk method and make its default behavior consistent with call_native/parse_native_response.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/gateway/traits/chat_format.rs`:
- Around line 82-86: The trait forces implementors to provide
transform_native_stream_chunk while other native methods (call_native,
parse_native_response) have defaults; add a default implementation for
transform_native_stream_chunk on the trait that returns an Err (or other
appropriate "not supported" error) so formats that don't support native
streaming can omit a stub, and add a brief doc comment on
transform_native_stream_chunk explaining why it accepts &dyn
ProviderCapabilities (versus &NativeHandler<'_> used by other native methods) if
the caller must inspect capabilities before selecting the native path; reference
the transform_native_stream_chunk method and make its default behavior
consistent with call_native/parse_native_response.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: bc564241-8522-4036-867f-a3a493d92bcb
📒 Files selected for processing (2)
docs/internals/llm-types.mdsrc/gateway/traits/chat_format.rs
✅ Files skipped from review due to trivial changes (1)
- docs/internals/llm-types.md
This was referenced Apr 6, 2026
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.
As the second part of the major refactoring of the provider, add traits for use by the new provider crate.
Summary by CodeRabbit
Documentation
New Features
Refactor