-
Notifications
You must be signed in to change notification settings - Fork 197
fix: append bedrock and cohere routes in langchain and litellm and docs fixes --skip-pipeline #1155
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Warning Rate limit exceeded@Pratham-Mishra04 has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 20 minutes and 58 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (10)
📝 WalkthroughWalkthroughAdds Bedrock and Cohere route registrations to LangChain and LiteLLM routers, renames and exports the Bedrock route factory, updates tests and call sites to use the exported name, and simplifies Bedrock usage in docs by replacing boto3 client creation with direct auth parameters. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
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 |
🧪 Test Suite AvailableThis PR can be tested by a repository admin. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
docs/integrations/langchain-sdk.mdxtransports/bifrost-http/integrations/langchain.gotransports/bifrost-http/integrations/litellm.go
🧰 Additional context used
📓 Path-based instructions (1)
**
⚙️ CodeRabbit configuration file
always check the stack if there is one for the current PR. do not give localized reviews for the PR, always see all changes in the light of the whole stack of PRs (if there is a stack, if there is no stack you can continue to make localized suggestions/reviews)
Files:
transports/bifrost-http/integrations/litellm.gotransports/bifrost-http/integrations/langchain.godocs/integrations/langchain-sdk.mdx
🧠 Learnings (2)
📚 Learning: 2025-12-09T17:07:42.007Z
Learnt from: qwerty-dvorak
Repo: maximhq/bifrost PR: 1006
File: core/schemas/account.go:9-18
Timestamp: 2025-12-09T17:07:42.007Z
Learning: In core/schemas/account.go, the HuggingFaceKeyConfig field within the Key struct is currently unused and reserved for future Hugging Face inference endpoint deployments. Do not flag this field as missing from OpenAPI documentation or require its presence in the API spec until the feature is actively implemented and used. When the feature is added, update the OpenAPI docs accordingly; otherwise, treat this field as non-breaking and not part of the current API surface.
Applied to files:
transports/bifrost-http/integrations/litellm.gotransports/bifrost-http/integrations/langchain.go
📚 Learning: 2025-12-12T08:25:02.629Z
Learnt from: Pratham-Mishra04
Repo: maximhq/bifrost PR: 1000
File: transports/bifrost-http/integrations/router.go:709-712
Timestamp: 2025-12-12T08:25:02.629Z
Learning: In transports/bifrost-http/**/*.go, update streaming response handling to align with OpenAI Responses API: use typed SSE events such as response.created, response.output_text.delta, response.done, etc., and do not rely on the legacy data: [DONE] termination marker. Note that data: [DONE] is only used by the older Chat Completions and Text Completions streaming APIs. Ensure parsers, writers, and tests distinguish SSE events from the [DONE] sentinel and handle each event type accordingly for correct stream termination and progress updates.
Applied to files:
transports/bifrost-http/integrations/litellm.gotransports/bifrost-http/integrations/langchain.go
🧬 Code graph analysis (2)
transports/bifrost-http/integrations/litellm.go (1)
transports/bifrost-http/integrations/cohere.go (1)
CreateCohereRouteConfigs(27-100)
transports/bifrost-http/integrations/langchain.go (1)
transports/bifrost-http/integrations/cohere.go (1)
CreateCohereRouteConfigs(27-100)
🔇 Additional comments (6)
transports/bifrost-http/integrations/langchain.go (1)
33-34: LGTM! Cohere routes properly integrated.The Cohere route configuration follows the established pattern and correctly uses the exported
CreateCohereRouteConfigsfunction with the appropriate/langchainpath prefix. Based on the provided implementation in relevant code snippets, this properly registers the/v2/chatand/v2/embedendpoints with appropriate converters.transports/bifrost-http/integrations/litellm.go (2)
30-31: Verify function name casing forcreateBedrockRouteConfigs.Same casing inconsistency as in
langchain.go: this uses lowercasecreateBedrockRouteConfigswhile all other route config functions use uppercase (e.g.,CreateOpenAIRouteConfigs,CreateCohereRouteConfigs). Please verify that:
- The function exists with this exact name
- The lowercase naming is intentional
The verification script from the
langchain.goreview will check this function's existence across the codebase.
33-34: LGTM! Cohere routes properly integrated.The Cohere route configuration correctly uses the
/litellmpath prefix and follows the same pattern as the LangChain integration. This properly extends LiteLLM's API compatibility to include Cohere endpoints.docs/integrations/langchain-sdk.mdx (3)
423-426: LGTM! Simplified Bedrock authentication aligns with new route configs.The documentation update removes the boto3 client creation complexity and directly passes authentication parameters to
ChatBedrockConverse. This aligns well with the new Bedrock route configurations added to the LangChain integration. The "dummy-" prefixes make it clear these are placeholder values for documentation purposes.
448-451: LGTM! Nova authentication follows Claude pattern.The Nova model example is updated with the same simplified authentication approach as the Claude example, maintaining consistency across Bedrock model documentation.
486-486: Nice addition! Clarifies Vertex AI model option.The inline comment helps users understand they can use either the
gemini/orvertex/prefix for Gemini 2.5 models, which is a helpful clarification without requiring a separate documentation section.
f55d01e to
ce2a732
Compare
ce2a732 to
cf2ee50
Compare
cf2ee50 to
e8fc53b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
core/changelog.md (1)
4-4: Optional: Use hyphen in compound adjective.Consider changing "fine tuned" to "fine-tuned" for grammatical consistency.
🔎 Proposed fix
-- refactor: for fine tuned or custom models in vertex use gemini native endpoint instead of openai compatible chat completions endpoint +- refactor: for fine-tuned or custom models in vertex use gemini native endpoint instead of openai compatible chat completions endpointtransports/changelog.md (1)
4-4: Optional: Use hyphen in compound adjective.Consider changing "fine tuned" to "fine-tuned" for grammatical consistency.
🔎 Proposed fix
-- refactor: for fine tuned or custom models in vertex use gemini native endpoint instead of openai compatible chat completions endpoint +- refactor: for fine-tuned or custom models in vertex use gemini native endpoint instead of openai compatible chat completions endpoint
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
core/changelog.mddocs/integrations/langchain-sdk.mdxtransports/bifrost-http/integrations/bedrock.gotransports/bifrost-http/integrations/bedrock_test.gotransports/bifrost-http/integrations/langchain.gotransports/bifrost-http/integrations/litellm.gotransports/bifrost-http/integrations/pydanticai.gotransports/changelog.md
🚧 Files skipped from review as they are similar to previous changes (2)
- transports/bifrost-http/integrations/langchain.go
- docs/integrations/langchain-sdk.mdx
🧰 Additional context used
📓 Path-based instructions (1)
**
⚙️ CodeRabbit configuration file
always check the stack if there is one for the current PR. do not give localized reviews for the PR, always see all changes in the light of the whole stack of PRs (if there is a stack, if there is no stack you can continue to make localized suggestions/reviews)
Files:
transports/bifrost-http/integrations/litellm.gocore/changelog.mdtransports/bifrost-http/integrations/bedrock.gotransports/bifrost-http/integrations/pydanticai.gotransports/bifrost-http/integrations/bedrock_test.gotransports/changelog.md
🧠 Learnings (3)
📚 Learning: 2025-12-09T17:07:42.007Z
Learnt from: qwerty-dvorak
Repo: maximhq/bifrost PR: 1006
File: core/schemas/account.go:9-18
Timestamp: 2025-12-09T17:07:42.007Z
Learning: In core/schemas/account.go, the HuggingFaceKeyConfig field within the Key struct is currently unused and reserved for future Hugging Face inference endpoint deployments. Do not flag this field as missing from OpenAPI documentation or require its presence in the API spec until the feature is actively implemented and used. When the feature is added, update the OpenAPI docs accordingly; otherwise, treat this field as non-breaking and not part of the current API surface.
Applied to files:
transports/bifrost-http/integrations/litellm.gotransports/bifrost-http/integrations/bedrock.gotransports/bifrost-http/integrations/pydanticai.gotransports/bifrost-http/integrations/bedrock_test.go
📚 Learning: 2025-12-12T08:25:02.629Z
Learnt from: Pratham-Mishra04
Repo: maximhq/bifrost PR: 1000
File: transports/bifrost-http/integrations/router.go:709-712
Timestamp: 2025-12-12T08:25:02.629Z
Learning: In transports/bifrost-http/**/*.go, update streaming response handling to align with OpenAI Responses API: use typed SSE events such as response.created, response.output_text.delta, response.done, etc., and do not rely on the legacy data: [DONE] termination marker. Note that data: [DONE] is only used by the older Chat Completions and Text Completions streaming APIs. Ensure parsers, writers, and tests distinguish SSE events from the [DONE] sentinel and handle each event type accordingly for correct stream termination and progress updates.
Applied to files:
transports/bifrost-http/integrations/litellm.gotransports/bifrost-http/integrations/bedrock.gotransports/bifrost-http/integrations/pydanticai.gotransports/bifrost-http/integrations/bedrock_test.go
📚 Learning: 2025-12-13T13:03:38.364Z
Learnt from: akshaydeo
Repo: maximhq/bifrost PR: 955
File: transports/bifrost-http/integrations/bedrock.go:682-697
Timestamp: 2025-12-13T13:03:38.364Z
Learning: In transports/bifrost-http/integrations/bedrock.go, implement FileRetrieveRequest.FileID and FileDeleteRequest.FileID to use the ETag from the If-Match header as the provider-specific file ID (e.g., OpenAI file-xxx, Gemini files/yyy), rather than deriving the ID from the S3 URI. The StorageConfig should supply the S3 bucket/prefix context, enabling cross-provider routing while keeping S3 as the backing store. This change should be accompanied by unit tests that verify the FileID source is the If-Match header, update any relevant request/response models, and add in-code comments to clarify the separation of provider file IDs from S3 paths.
Applied to files:
transports/bifrost-http/integrations/bedrock.go
🧬 Code graph analysis (4)
transports/bifrost-http/integrations/litellm.go (2)
transports/bifrost-http/integrations/bedrock.go (1)
CreateBedrockRouteConfigs(183-190)transports/bifrost-http/integrations/cohere.go (1)
CreateCohereRouteConfigs(27-100)
transports/bifrost-http/integrations/bedrock.go (2)
transports/bifrost-http/lib/config.go (1)
HandlerStore(36-39)transports/bifrost-http/integrations/router.go (1)
RouteConfig(265-295)
transports/bifrost-http/integrations/pydanticai.go (1)
transports/bifrost-http/integrations/bedrock.go (1)
CreateBedrockRouteConfigs(183-190)
transports/bifrost-http/integrations/bedrock_test.go (2)
transports/bifrost-http/integrations/bedrock.go (1)
CreateBedrockRouteConfigs(183-190)core/schemas/bifrost.go (1)
ModelProvider(32-32)
🪛 LanguageTool
core/changelog.md
[grammar] ~4-~4: Use a hyphen to join words.
Context: ...r gemini embeddings - refactor: for fine tuned or custom models in vertex use gem...
(QB_NEW_EN_HYPHEN)
transports/changelog.md
[grammar] ~4-~4: Use a hyphen to join words.
Context: ...r gemini embeddings - refactor: for fine tuned or custom models in vertex use gem...
(QB_NEW_EN_HYPHEN)
⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: Graphite / mergeability_check
- GitHub Check: Graphite / mergeability_check
- GitHub Check: Graphite / mergeability_check
- GitHub Check: Graphite / mergeability_check
- GitHub Check: Graphite / mergeability_check
- GitHub Check: Graphite / mergeability_check
- GitHub Check: Graphite / mergeability_check
🔇 Additional comments (5)
transports/bifrost-http/integrations/pydanticai.go (1)
34-34: LGTM! Correct usage of the exported function.The update to use
CreateBedrockRouteConfigsaligns with the function export in bedrock.go and correctly extends Pydantic AI router with Bedrock API compatibility.transports/bifrost-http/integrations/litellm.go (1)
30-34: LGTM! Bedrock and Cohere routes added consistently.The additions extend LiteLLM router with AWS Bedrock and Cohere API compatibility, following the established pattern from other integration routers.
transports/bifrost-http/integrations/bedrock.go (2)
182-183: LGTM! Function correctly exported.Exporting
CreateBedrockRouteConfigsfollows Go conventions and enables consistent reuse across LangChain, LiteLLM, and Pydantic AI integration routers.
573-573: LGTM! Call site updated consistently.The BedrockRouter correctly uses the exported function name.
transports/bifrost-http/integrations/bedrock_test.go (1)
81-81: LGTM! Test updated to use exported function.The test correctly calls
CreateBedrockRouteConfigsand maintains proper coverage of the Bedrock route configuration.
e8fc53b to
4c552bf
Compare
2615d7b to
69df665
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
core/changelog.mdcore/providers/huggingface/huggingface_test.godocs/integrations/langchain-sdk.mdxtransports/bifrost-http/integrations/bedrock.gotransports/bifrost-http/integrations/bedrock_test.gotransports/bifrost-http/integrations/langchain.gotransports/bifrost-http/integrations/litellm.gotransports/bifrost-http/integrations/pydanticai.gotransports/changelog.md
🚧 Files skipped from review as they are similar to previous changes (2)
- docs/integrations/langchain-sdk.mdx
- transports/bifrost-http/integrations/bedrock_test.go
🧰 Additional context used
📓 Path-based instructions (1)
**
⚙️ CodeRabbit configuration file
always check the stack if there is one for the current PR. do not give localized reviews for the PR, always see all changes in the light of the whole stack of PRs (if there is a stack, if there is no stack you can continue to make localized suggestions/reviews)
Files:
core/providers/huggingface/huggingface_test.gotransports/bifrost-http/integrations/langchain.gotransports/bifrost-http/integrations/pydanticai.gotransports/bifrost-http/integrations/bedrock.gotransports/bifrost-http/integrations/litellm.gocore/changelog.mdtransports/changelog.md
🧠 Learnings (6)
📚 Learning: 2025-12-09T17:07:42.007Z
Learnt from: qwerty-dvorak
Repo: maximhq/bifrost PR: 1006
File: core/schemas/account.go:9-18
Timestamp: 2025-12-09T17:07:42.007Z
Learning: In core/schemas/account.go, the HuggingFaceKeyConfig field within the Key struct is currently unused and reserved for future Hugging Face inference endpoint deployments. Do not flag this field as missing from OpenAPI documentation or require its presence in the API spec until the feature is actively implemented and used. When the feature is added, update the OpenAPI docs accordingly; otherwise, treat this field as non-breaking and not part of the current API surface.
Applied to files:
core/providers/huggingface/huggingface_test.gotransports/bifrost-http/integrations/langchain.gotransports/bifrost-http/integrations/pydanticai.gotransports/bifrost-http/integrations/bedrock.gotransports/bifrost-http/integrations/litellm.go
📚 Learning: 2025-12-15T10:16:21.909Z
Learnt from: qwerty-dvorak
Repo: maximhq/bifrost PR: 1006
File: core/providers/huggingface/huggingface_test.go:12-63
Timestamp: 2025-12-15T10:16:21.909Z
Learning: In provider tests under core/providers/<provider>/*_test.go, do not require or flag the use of defer for Shutdown(); instead call client.Shutdown() at the end of each test function. This pattern appears consistent across all provider tests. Apply this rule only within this path; for other tests or resources, defer may still be appropriate.
Applied to files:
core/providers/huggingface/huggingface_test.go
📚 Learning: 2025-12-15T10:34:13.855Z
Learnt from: qwerty-dvorak
Repo: maximhq/bifrost PR: 1006
File: core/providers/huggingface/chat.go:370-389
Timestamp: 2025-12-15T10:34:13.855Z
Learning: In the HuggingFace provider (core/providers/huggingface/), streaming tool call data arrives as a single delta chunk with all fields (id, type, function.name, function.arguments) present at once. Do not implement accumulation logic across deltas for streaming tool calls in this codebase; rely on single-chunk data as the guaranteed behavior. If there is existing accumulation logic in this directory, remove it or guard it behind this assumption.
Applied to files:
core/providers/huggingface/huggingface_test.go
📚 Learning: 2025-12-19T09:26:54.961Z
Learnt from: qwerty-dvorak
Repo: maximhq/bifrost PR: 1006
File: core/providers/utils/utils.go:1050-1051
Timestamp: 2025-12-19T09:26:54.961Z
Learning: Update streaming end-marker handling so HuggingFace is treated as a non-[DONE] provider for backends that do not emit a DONE marker (e.g., meta llama on novita). In core/providers/utils/utils.go, adjust ProviderSendsDoneMarker() (or related logic) to detect providers that may not emit DONE and avoid relying on DONE as the sole end signal. Add tests to cover both DONE-emitting and non-DONE backends, with clear documentation in code comments explaining the rationale and any fallback behavior.
Applied to files:
core/providers/huggingface/huggingface_test.go
📚 Learning: 2025-12-12T08:25:02.629Z
Learnt from: Pratham-Mishra04
Repo: maximhq/bifrost PR: 1000
File: transports/bifrost-http/integrations/router.go:709-712
Timestamp: 2025-12-12T08:25:02.629Z
Learning: In transports/bifrost-http/**/*.go, update streaming response handling to align with OpenAI Responses API: use typed SSE events such as response.created, response.output_text.delta, response.done, etc., and do not rely on the legacy data: [DONE] termination marker. Note that data: [DONE] is only used by the older Chat Completions and Text Completions streaming APIs. Ensure parsers, writers, and tests distinguish SSE events from the [DONE] sentinel and handle each event type accordingly for correct stream termination and progress updates.
Applied to files:
transports/bifrost-http/integrations/langchain.gotransports/bifrost-http/integrations/pydanticai.gotransports/bifrost-http/integrations/bedrock.gotransports/bifrost-http/integrations/litellm.go
📚 Learning: 2025-12-13T13:03:38.364Z
Learnt from: akshaydeo
Repo: maximhq/bifrost PR: 955
File: transports/bifrost-http/integrations/bedrock.go:682-697
Timestamp: 2025-12-13T13:03:38.364Z
Learning: In transports/bifrost-http/integrations/bedrock.go, implement FileRetrieveRequest.FileID and FileDeleteRequest.FileID to use the ETag from the If-Match header as the provider-specific file ID (e.g., OpenAI file-xxx, Gemini files/yyy), rather than deriving the ID from the S3 URI. The StorageConfig should supply the S3 bucket/prefix context, enabling cross-provider routing while keeping S3 as the backing store. This change should be accompanied by unit tests that verify the FileID source is the If-Match header, update any relevant request/response models, and add in-code comments to clarify the separation of provider file IDs from S3 paths.
Applied to files:
transports/bifrost-http/integrations/bedrock.go
🧬 Code graph analysis (4)
transports/bifrost-http/integrations/langchain.go (2)
transports/bifrost-http/integrations/bedrock.go (1)
CreateBedrockRouteConfigs(183-190)transports/bifrost-http/integrations/cohere.go (1)
CreateCohereRouteConfigs(27-100)
transports/bifrost-http/integrations/pydanticai.go (1)
transports/bifrost-http/integrations/bedrock.go (1)
CreateBedrockRouteConfigs(183-190)
transports/bifrost-http/integrations/bedrock.go (1)
transports/bifrost-http/integrations/router.go (1)
RouteConfig(265-295)
transports/bifrost-http/integrations/litellm.go (2)
transports/bifrost-http/integrations/bedrock.go (1)
CreateBedrockRouteConfigs(183-190)transports/bifrost-http/integrations/cohere.go (1)
CreateCohereRouteConfigs(27-100)
🪛 LanguageTool
core/changelog.md
[grammar] ~4-~4: Use a hyphen to join words.
Context: ...r gemini embeddings - refactor: for fine tuned or custom models in vertex use gem...
(QB_NEW_EN_HYPHEN)
transports/changelog.md
[grammar] ~4-~4: Use a hyphen to join words.
Context: ...r gemini embeddings - refactor: for fine tuned or custom models in vertex use gem...
(QB_NEW_EN_HYPHEN)
⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (14)
- GitHub Check: Graphite / mergeability_check
- GitHub Check: Graphite / mergeability_check
- GitHub Check: Graphite / mergeability_check
- GitHub Check: Graphite / mergeability_check
- GitHub Check: Graphite / mergeability_check
- GitHub Check: Graphite / mergeability_check
- GitHub Check: Graphite / mergeability_check
- GitHub Check: Graphite / mergeability_check
- GitHub Check: Graphite / mergeability_check
- GitHub Check: Graphite / mergeability_check
- GitHub Check: Graphite / mergeability_check
- GitHub Check: Graphite / mergeability_check
- GitHub Check: Graphite / mergeability_check
- GitHub Check: Graphite / mergeability_check
🔇 Additional comments (5)
transports/bifrost-http/integrations/litellm.go (1)
30-34: LGTM! Bedrock and Cohere route integrations correctly added.The route configurations follow the established pattern for OpenAI, Anthropic, and GenAI integrations. Function signatures match the exported definitions, and the path prefix is correctly applied.
transports/bifrost-http/integrations/langchain.go (1)
30-34: LGTM! Bedrock and Cohere route integrations correctly added.The route configurations follow the established pattern and use the exported function names. The implementation is consistent with LiteLLM and other router integrations.
transports/bifrost-http/integrations/pydanticai.go (1)
34-34: LGTM! Updated to use exported Bedrock route configuration.The call site correctly uses the exported
CreateBedrockRouteConfigsfunction, aligning with the refactor in bedrock.go.transports/bifrost-http/integrations/bedrock.go (2)
182-183: LGTM! Bedrock route configuration correctly exported.Exporting
CreateBedrockRouteConfigsenables reuse across LangChain, LiteLLM, and PydanticAI routers. The function signature and behavior remain unchanged.
573-573: LGTM! Call site updated to use exported function.The call to
CreateBedrockRouteConfigscorrectly uses the exported function name, consistent with the rename.
4c552bf to
401e9bd
Compare
Merge activity
|
401e9bd to
0dbdad4
Compare

Add AWS Bedrock and Cohere API support to LangChain and LiteLLM integrations
Changes
Type of change
Affected areas
How to test
Test the new API compatibility with LangChain and LiteLLM:
Breaking changes
Related issues
Enhances API compatibility for LangChain and LiteLLM integrations
Security considerations
The implementation uses dummy credentials in examples which should be replaced with proper authentication in production environments.
Checklist