feat: Expose model provider header#100
Merged
Merged
Conversation
Contributor
📝 WalkthroughWalkthroughAdds injection of the Changes
Sequence DiagramsequenceDiagram
participant Client
participant Handler
participant ProviderRegistry
participant Provider
participant Response
Client->>Handler: POST /chat/completions (model)
Handler->>ProviderRegistry: get(model_name)
ProviderRegistry->>Provider: lookup provider
Provider-->>ProviderRegistry: return provider (ProviderType)
ProviderRegistry-->>Handler: provider + type
Handler->>Handler: compute provider_type string
Handler->>Response: inject header "x-genai-provider-name"
Handler-->>Client: 200 OK + header
Client->>Handler: POST /completions (stream)
Handler->>ProviderRegistry: get(model_name)
ProviderRegistry->>Provider: lookup provider
Provider-->>ProviderRegistry: return provider (ProviderType)
ProviderRegistry-->>Handler: provider + type
Handler->>Handler: compute provider_type string
Handler->>Response: stream SSE events with header injected
Handler-->>Client: SSE stream + header
Client->>Handler: POST /chat/completions (unknown model)
Handler->>ProviderRegistry: get(unknown_model)
ProviderRegistry-->>Handler: None
Handler-->>Client: 404 Not Found (no header)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ 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)
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 |
|
|
Contributor
🔒 Container Vulnerability Scan (hub-migrations - amd64)Click to expand results |
Contributor
🔒 Container Vulnerability Scan (hub-migrations - arm64)Click to expand results |
Contributor
🔒 Container Vulnerability Scan (hub - amd64)Click to expand results |
Contributor
🔒 Container Vulnerability Scan (hub - arm64)Click to expand results |
doronkopit5
changed the title
doronkopit5
reviewed
Apr 14, 2026
Comment on lines
+38
to
+40
| node_modules | ||
| package-lock.json | ||
| package.json |
Contributor
Author
There was a problem hiding this comment.
If you cargo run hub locally, it's generating bunch of dependencies garbage, don't belong in the repo imo?
Member
There was a problem hiding this comment.
are you sure, doesn't happen for me
Contributor
Author
There was a problem hiding this comment.
never mind 🤦 removed it
doronkopit5
reviewed
Apr 14, 2026
| use reqwest_streams::error::StreamBodyError; | ||
| use std::sync::Arc; | ||
|
|
||
| const HEADER_PROVIDER: HeaderName = HeaderName::from_static("x-traceloop-provider"); |
Member
There was a problem hiding this comment.
lets call it like the gen ai semantic convention
x-genai-provider/system i don't remember the right naming
Contributor
Author
There was a problem hiding this comment.
semconv is GEN_AI_PROVIDER_NAME= gen_ai.provider.name
x-genai-provider-name will do?
doronkopit5
marked this pull request as ready for review
April 14, 2026 12:39
doronkopit5
approved these changes
Apr 14, 2026
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/pipelines/pipeline.rs (1)
483-499: Add one streaming-header test.
ConfigurableMockProvider::chat_completionsalways returnsNonStream, so the new assertions never hit the SSE response path on Lines 145-150. OneChatCompletionResponse::Streamcase would keep header propagation for streamed responses from regressing.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pipelines/pipeline.rs` around lines 483 - 499, The test never exercises the SSE path because ConfigurableMockProvider::chat_completions always returns ChatCompletionResponse::NonStream; update the mock to support a Stream case and adjust the test to trigger it: modify ConfigurableMockProvider::chat_completions to inspect a flag or the incoming ChatCompletionRequest (e.g., a "stream" field or special test marker) and return ChatCompletionResponse::Stream yielding at least one streaming ChatCompletion chunk, then add/modify the test to send that marker so the code path handling ChatCompletionResponse::Stream (the SSE header propagation logic) is executed; refer to ConfigurableMockProvider, chat_completions, ChatCompletionResponse::Stream and the ChatCompletion/streaming variants to locate and change the code.
🤖 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/pipelines/pipeline.rs`:
- Around line 483-499: The test never exercises the SSE path because
ConfigurableMockProvider::chat_completions always returns
ChatCompletionResponse::NonStream; update the mock to support a Stream case and
adjust the test to trigger it: modify ConfigurableMockProvider::chat_completions
to inspect a flag or the incoming ChatCompletionRequest (e.g., a "stream" field
or special test marker) and return ChatCompletionResponse::Stream yielding at
least one streaming ChatCompletion chunk, then add/modify the test to send that
marker so the code path handling ChatCompletionResponse::Stream (the SSE header
propagation logic) is executed; refer to ConfigurableMockProvider,
chat_completions, ChatCompletionResponse::Stream and the
ChatCompletion/streaming variants to locate and change the code.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7748a73d-6550-45da-bbc6-8ee6742a0f22
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (3)
.gitignoresrc/pipelines/pipeline.rssrc/providers/registry.rs
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/pipelines/pipeline.rs (2)
31-37: Don’t silently ignore header serialization failures.At Line 32,
HeaderValue::from_str(...)failures are dropped silently. Even if currently unlikely, this can hide regressions ifProviderTypestring formatting changes later. Please log a warning (or debug assertion in test/dev) on theErrpath.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pipelines/pipeline.rs` around lines 31 - 37, The inject_provider_header function currently drops errors from HeaderValue::from_str silently; update it so the Err branch does not get ignored: when HeaderValue::from_str(&provider_type.to_string()) returns Err, call a logger (e.g., warn! or debug!) or invoke debug_assert! with the error/details so failures are visible in dev/test, while preserving the existing successful path that inserts into response.headers_mut().insert(HEADER_PROVIDER.clone(), value); reference the inject_provider_header function, ProviderType, HeaderValue::from_str, and response.headers_mut().insert when making the change.
488-499: Add explicit coverage for the streaming chat path header.Current chat header tests use a mock that returns
NonStream, so the SSE branch header injection (Line 146–150) is not validated. Add one test wherechat_completionsreturnsChatCompletionResponse::Stream(...)and assertx-genai-provider-nameis present.Also applies to: 590-615
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pipelines/pipeline.rs` around lines 488 - 499, Add a test case that exercises the SSE/streaming branch by making the mocked chat_completions return crate::models::chat::ChatCompletionResponse::Stream(...) instead of NonStream; in that test assert the response includes the SSE header "x-genai-provider-name" (the same header verified for non-stream paths) to cover the header-injection logic executed in the streaming path (the code that emits SSE events when ChatCompletionResponse::Stream is returned). Ensure the mock uses a Stream variant containing at least one event so the SSE path in pipeline.rs (the SSE branch that sets the header) is executed and the assertion checks header presence.
🤖 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/pipelines/pipeline.rs`:
- Around line 31-37: The inject_provider_header function currently drops errors
from HeaderValue::from_str silently; update it so the Err branch does not get
ignored: when HeaderValue::from_str(&provider_type.to_string()) returns Err,
call a logger (e.g., warn! or debug!) or invoke debug_assert! with the
error/details so failures are visible in dev/test, while preserving the existing
successful path that inserts into
response.headers_mut().insert(HEADER_PROVIDER.clone(), value); reference the
inject_provider_header function, ProviderType, HeaderValue::from_str, and
response.headers_mut().insert when making the change.
- Around line 488-499: Add a test case that exercises the SSE/streaming branch
by making the mocked chat_completions return
crate::models::chat::ChatCompletionResponse::Stream(...) instead of NonStream;
in that test assert the response includes the SSE header "x-genai-provider-name"
(the same header verified for non-stream paths) to cover the header-injection
logic executed in the streaming path (the code that emits SSE events when
ChatCompletionResponse::Stream is returned). Ensure the mock uses a Stream
variant containing at least one event so the SSE path in pipeline.rs (the SSE
branch that sets the header) is executed and the assertion checks header
presence.
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
4 participants
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.
Summary
X-GenAI-Provider-Nameresponse header tochat_completions,completions, andembeddingspipeline handlersopenai | anthropic | azure | bedrock | vertexaiparsing config
Summary by CodeRabbit
New Features
Tests
Chores