Conversation
Contributor
Summary by CodeRabbit
WalkthroughThis update introduces explicit assignment of the Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant HTTP Router
participant Bifrost Core
participant Provider
participant ErrorConverter
Client->>HTTP Router: Send request (e.g., chat completion)
HTTP Router->>Bifrost Core: Forward request
Bifrost Core->>Provider: Call provider API
Provider-->>Bifrost Core: Return error (with Provider set)
Bifrost Core-->>HTTP Router: Return BifrostError (with Provider)
HTTP Router->>ErrorConverter: Convert BifrostError to integration-specific error
ErrorConverter-->>HTTP Router: Integration-specific error response
HTTP Router-->>Client: Send formatted error response
Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 7
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (16)
core/bifrost.go(9 hunks)core/schemas/bifrost.go(1 hunks)tests/transports-integrations/README.md(3 hunks)tests/transports-integrations/tests/integrations/test_anthropic.py(2 hunks)tests/transports-integrations/tests/integrations/test_google.py(2 hunks)tests/transports-integrations/tests/integrations/test_litellm.py(2 hunks)tests/transports-integrations/tests/integrations/test_openai.py(2 hunks)tests/transports-integrations/tests/utils/common.py(3 hunks)transports/bifrost-http/integrations/anthropic/router.go(1 hunks)transports/bifrost-http/integrations/anthropic/types.go(2 hunks)transports/bifrost-http/integrations/genai/router.go(1 hunks)transports/bifrost-http/integrations/genai/types.go(2 hunks)transports/bifrost-http/integrations/litellm/router.go(2 hunks)transports/bifrost-http/integrations/openai/router.go(1 hunks)transports/bifrost-http/integrations/openai/types.go(2 hunks)transports/bifrost-http/integrations/utils.go(5 hunks)
🧰 Additional context used
🧠 Learnings (17)
📓 Common learnings
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#84
File: transports/bifrost-http/main.go:2-2
Timestamp: 2025-06-15T16:05:13.489Z
Learning: For the Bifrost project, HTTP transport integration routers for new providers (like Mistral and Ollama) are implemented in separate PRs from the core provider support, following a focused PR strategy.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#94
File: core/bifrost.go:823-845
Timestamp: 2025-06-18T15:15:51.323Z
Learning: In the Bifrost project, the team prioritizes maintaining consistent error handling patterns over exposing detailed error context. All errors should be wrapped in the standard `BifrostError` structure rather than creating specific error types or exposing richer error details like exit codes or stderr output.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#150
File: transports/bifrost-http/lib/store.go:370-466
Timestamp: 2025-07-09T04:58:08.229Z
Learning: Pratham-Mishra04 prefers not to add logging or error handling for unreachable code paths in the Bifrost project. When provider types or similar entities are predefined in the system, defensive programming like logging in default cases is considered unnecessary overhead.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#63
File: transports/bifrost-http/integrations/openai/router.go:0-0
Timestamp: 2025-06-09T14:03:34.227Z
Learning: In the Bifrost HTTP transport layer (transports/bifrost-http/integrations/), request validation like checking for empty messages should be handled by the provider rather than at the transport layer. The transport layer should forward requests to Bifrost core/providers for validation.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#83
File: core/providers/mistral.go:96-101
Timestamp: 2025-06-15T13:50:41.418Z
Learning: In the Bifrost project, when a provider doesn't support a specific operation (like text completion), the IsBifrostError flag should be set to false to mark it as a provider-level error rather than a Bifrost framework error. This is intentional design for unsupported operations.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#63
File: transports/bifrost-http/integrations/openai/types.go:89-119
Timestamp: 2025-06-10T11:00:02.875Z
Learning: In the Bifrost OpenAI integration (transports/bifrost-http/integrations/openai/types.go), the convertOpenAIContent function currently only handles the last image URL when multiple images are present in a content array. The user Pratham-Mishra04 has acknowledged this limitation and indicated it's part of a larger architectural issue that will be addressed comprehensively later, rather than with piecemeal fixes.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#152
File: transports/bifrost-http/plugins/logging/utils.go:378-399
Timestamp: 2025-07-10T13:44:14.471Z
Learning: In the Bifrost logging plugin (transports/bifrost-http/plugins/logging/utils.go), Pratham-Mishra04 prefers not to add error handling for JSON unmarshaling operations, considering logging not very critical and being confident that JSON marshalling won't fail in practice.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#54
File: core/providers/bedrock.go:241-252
Timestamp: 2025-06-04T09:07:20.867Z
Learning: In the Bifrost codebase, when working with AWS Bedrock provider authentication, the preference is to let AWS handle access key validation naturally rather than adding preemptive checks for empty/blank access keys. This allows AWS to provide its own authentication error messages which can be more informative than custom validation errors.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#152
File: transports/bifrost-http/plugins/logging/utils.go:94-111
Timestamp: 2025-07-10T13:44:23.255Z
Learning: Pratham-Mishra04 prefers not to add error handling for JSON marshaling operations in the Bifrost logging plugin (transports/bifrost-http/plugins/logging/utils.go) because logging is not critical functionality and the structured schema data being marshaled is unlikely to fail. They accept the risk of not handling json.Marshal errors in logging contexts to keep the code simple.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#141
File: core/bifrost.go:198-272
Timestamp: 2025-07-08T18:30:08.258Z
Learning: Pratham-Mishra04 follows a pattern of implementing core functionality first and deferring non-critical improvements (like race condition fixes, optimizations) to later PRs. This is a reasonable development approach that prioritizes getting the main feature working before addressing edge cases.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#83
File: core/providers/mistral.go:168-170
Timestamp: 2025-06-15T14:24:49.882Z
Learning: In the Bifrost codebase, performance is prioritized over defensive copying for HTTP service operations. Specifically, shallow slice assignments in provider response handling are acceptable due to object pool reset patterns and JSON unmarshaling behavior that minimize practical data corruption risks.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#89
File: transports/bifrost-http/integrations/genai/types.go:151-155
Timestamp: 2025-06-16T14:45:48.563Z
Learning: In transports/bifrost-http/integrations/genai/types.go, when SystemInstruction has an empty role, the user prefers to let the downstream provider (Google GenAI) handle validation and return errors, rather than implementing validation in the bifrost layer. This represents a design preference for delegating validation to the appropriate service rather than duplicating validation logic in the proxy layer.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#81
File: tests/core-providers/scenarios/end_to_end_tool_calling.go:43-45
Timestamp: 2025-06-16T04:13:55.437Z
Learning: In the Bifrost codebase, errors returned from client methods like ChatCompletionRequest are of type BifrostError, not the standard error interface. For testing these errors, use require.Nilf instead of require.NoErrorf since BifrostError doesn't work with the standard error assertion methods.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#81
File: tests/core-providers/config/account.go:55-101
Timestamp: 2025-06-16T04:25:00.816Z
Learning: In the Bifrost test account implementation, the user prefers to let Bifrost itself handle missing API key errors rather than adding early validation in the GetKeysForProvider method.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#54
File: core/schemas/provider.go:148-148
Timestamp: 2025-06-04T03:57:50.981Z
Learning: Breaking changes in the Bifrost codebase are managed by first merging and tagging core schema changes, then updating dependent code references in subsequent steps after the core version is released.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#55
File: core/providers/anthropic.go:358-388
Timestamp: 2025-06-04T05:37:59.699Z
Learning: User Pratham-Mishra04 prefers not to extract small code duplications (around 2 lines) into helper functions, considering the overhead not worth it for such minor repetition.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#102
File: README.md:62-66
Timestamp: 2025-06-19T17:03:03.639Z
Learning: Pratham-Mishra04 prefers using the implicit 'latest' tag for the maximhq/bifrost Docker image rather than pinning to specific versions.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#143
File: core/mcp.go:155-196
Timestamp: 2025-07-08T15:33:47.698Z
Learning: Pratham-Mishra04 prefers not to add explanatory comments for obvious code patterns, such as the unlock/lock strategy around network I/O operations, considering them self-explanatory to experienced developers.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#138
File: docs/usage/go-package/mcp.md:408-412
Timestamp: 2025-07-01T12:40:08.576Z
Learning: Pratham-Mishra04 is okay with keeping bullet list formatting that uses colons after dashes in markdown documentation, even if it triggers linter warnings, preferring functionality over strict formatting rules.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#148
File: transports/bifrost-http/lib/store.go:880-910
Timestamp: 2025-07-08T17:14:21.544Z
Learning: Pratham-Mishra04 prefers resilient system design where missing environment variables for MCP connections should not cause complete system failure. The system should continue processing other MCP connections even when some fail, maintaining partial functionality rather than implementing fail-fast behavior.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#138
File: transports/README.md:26-28
Timestamp: 2025-07-01T12:45:06.906Z
Learning: Pratham-Mishra04 prefers keeping documentation examples simple and concise, trusting users to handle production-specific considerations like version pinning themselves rather than cluttering examples with additional notes.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#148
File: transports/bifrost-http/lib/store.go:1081-1098
Timestamp: 2025-07-08T17:16:50.811Z
Learning: Pratham-Mishra04 prefers practical redaction approaches over theoretical security improvements when the threat model is low-risk, such as admin-only interfaces in the Bifrost project. Fixed-length redaction is acceptable when only trusted administrators will see the redacted values.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#144
File: transports/bifrost-http/plugins/logging/utils.go:286-292
Timestamp: 2025-07-08T16:40:59.098Z
Learning: Pratham-Mishra04 prefers to keep simpler error handling patterns when errors are unlikely to occur in practice, such as safety checks in BadgerDB iterations where item.Value() is called on valid items. The user considers the overhead of explicit error handling not worth it in such scenarios.
core/schemas/bifrost.go (12)
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#80
File: plugins/maxim/main.go:249-264
Timestamp: 2025-06-14T06:17:54.426Z
Learning: In the BifrostError struct, the Error field is a struct (not a pointer), so accessing bifrostErr.Error.Message, bifrostErr.Error.Code, and bifrostErr.Error.Type is safe without nil checks on the Error field itself. The Code and Type fields are of type *string.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#83
File: core/schemas/bifrost.go:186-190
Timestamp: 2025-06-15T14:18:32.703Z
Learning: In core/schemas/bifrost.go, the ToolChoice UnmarshalJSON validation intentionally only checks for empty Type fields and lets providers handle validation of specific tool choice values. This architectural decision keeps schema validation focused on structure while allowing provider-specific semantic validation.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#94
File: core/schemas/bifrost.go:20-23
Timestamp: 2025-06-18T15:16:23.127Z
Learning: In the Bifrost project, BifrostConfig struct is never marshaled/unmarshaled, so serialization tags (json, yaml) are not needed for its fields.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#144
File: transports/bifrost-http/handlers/providers.go:45-49
Timestamp: 2025-07-08T16:50:27.699Z
Learning: In the Bifrost project, breaking API changes are acceptable when features are not yet public. This applies to scenarios like changing struct fields from pointer to non-pointer types in request/response structures for unreleased features.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#63
File: transports/bifrost-http/integrations/openai/types.go:223-231
Timestamp: 2025-06-10T11:06:06.670Z
Learning: The OpenAI `name` field on messages cannot be preserved when converting to Bifrost format because the `schemas.BifrostMessage` struct in bifrost/core does not support a Name field. This is a known limitation of the Bifrost core schema design.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#54
File: core/schemas/provider.go:148-148
Timestamp: 2025-06-04T03:57:50.981Z
Learning: Breaking changes in the Bifrost codebase are managed by first merging and tagging core schema changes, then updating dependent code references in subsequent steps after the core version is released.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#83
File: core/providers/mistral.go:96-101
Timestamp: 2025-06-15T13:50:41.418Z
Learning: In the Bifrost project, when a provider doesn't support a specific operation (like text completion), the IsBifrostError flag should be set to false to mark it as a provider-level error rather than a Bifrost framework error. This is intentional design for unsupported operations.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#94
File: core/bifrost.go:823-845
Timestamp: 2025-06-18T15:15:51.323Z
Learning: In the Bifrost project, the team prioritizes maintaining consistent error handling patterns over exposing detailed error context. All errors should be wrapped in the standard `BifrostError` structure rather than creating specific error types or exposing richer error details like exit codes or stderr output.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#152
File: transports/bifrost-http/plugins/logging/utils.go:94-111
Timestamp: 2025-07-10T13:44:23.255Z
Learning: Pratham-Mishra04 prefers not to add error handling for JSON marshaling operations in the Bifrost logging plugin (transports/bifrost-http/plugins/logging/utils.go) because logging is not critical functionality and the structured schema data being marshaled is unlikely to fail. They accept the risk of not handling json.Marshal errors in logging contexts to keep the code simple.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#81
File: tests/core-providers/scenarios/end_to_end_tool_calling.go:43-45
Timestamp: 2025-06-16T04:13:55.437Z
Learning: In the Bifrost codebase, errors returned from client methods like ChatCompletionRequest are of type BifrostError, not the standard error interface. For testing these errors, use require.Nilf instead of require.NoErrorf since BifrostError doesn't work with the standard error assertion methods.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#81
File: tests/core-providers/scenarios/complete_end_to_end.go:39-41
Timestamp: 2025-06-16T04:12:05.427Z
Learning: In the Bifrost system, error returns are of type `BifrostError` rather than the standard Go `error` interface. Therefore, use `require.Nilf(t, err, ...)` instead of `require.NoError(t, err)` when checking for errors in Bifrost function calls.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#81
File: tests/core-providers/scenarios/simple_chat.go:39-41
Timestamp: 2025-06-16T04:13:42.755Z
Learning: In the Bifrost codebase, errors returned from methods like ChatCompletionRequest are of type BifrostError (a custom error type) rather than the standard Go error interface. Therefore, require.Nilf should be used for error assertions instead of require.NoErrorf.
transports/bifrost-http/integrations/anthropic/router.go (9)
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#67
File: transports/bifrost-http/integrations/anthropic/router.go:26-34
Timestamp: 2025-06-10T11:19:29.604Z
Learning: The Generic router in transports/bifrost-http/integrations/utils.go already handles nil pointers from RequestConverter functions. When a RequestConverter returns a nil *schemas.BifrostRequest, the Generic router automatically returns an HTTP 400 error, making additional nil checks in individual router implementations redundant.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#67
File: transports/bifrost-http/integrations/anthropic/router.go:26-34
Timestamp: 2025-06-10T11:19:29.604Z
Learning: The Generic router in transports/bifrost-http/integrations/utils.go already handles nil pointers from RequestConverter functions. When a RequestConverter returns a nil *schemas.BifrostRequest, the Generic router automatically returns an HTTP 400 error with "Invalid request" message, making additional nil checks in individual router implementations redundant.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#65
File: transports/bifrost-http/integrations/anthropic/router.go:19-33
Timestamp: 2025-06-09T16:46:32.018Z
Learning: In the GenericRouter (transports/bifrost-http/integrations), ResponseFunc is not called if the BifrostResponse parameter is nil, providing built-in protection against nil response marshaling.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#55
File: core/providers/anthropic.go:0-0
Timestamp: 2025-06-04T05:44:09.141Z
Learning: For the Anthropic provider in core/providers/anthropic.go, it's acceptable to pass potentially malformed messages with invalid roles because Anthropic's API will return suitable error responses for role issues, eliminating the need for additional validation logging.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#55
File: core/providers/anthropic.go:526-550
Timestamp: 2025-06-04T09:29:46.287Z
Learning: In core/providers/anthropic.go, the content field in formattedMessages is always of type []interface{} because it's explicitly constructed that way upstream in the prepareAnthropicChatRequest function. Defensive type casting for multiple types is not needed since the type is guaranteed by the construction logic.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#80
File: plugins/maxim/main.go:249-264
Timestamp: 2025-06-14T06:17:54.426Z
Learning: In the BifrostError struct, the Error field is a struct (not a pointer), so accessing bifrostErr.Error.Message, bifrostErr.Error.Code, and bifrostErr.Error.Type is safe without nil checks on the Error field itself. The Code and Type fields are of type *string.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#81
File: tests/core-providers/scenarios/end_to_end_tool_calling.go:43-45
Timestamp: 2025-06-16T04:13:55.437Z
Learning: In the Bifrost codebase, errors returned from client methods like ChatCompletionRequest are of type BifrostError, not the standard error interface. For testing these errors, use require.Nilf instead of require.NoErrorf since BifrostError doesn't work with the standard error assertion methods.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#63
File: transports/bifrost-http/integrations/openai/router.go:62-64
Timestamp: 2025-06-09T11:27:00.925Z
Learning: The `lib.ConvertToBifrostContext` function in the bifrost HTTP transport never returns nil and handles the conversion internally, so nil checks are not needed when calling this function.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#63
File: transports/bifrost-http/integrations/openai/router.go:62-64
Timestamp: 2025-06-09T11:27:00.925Z
Learning: The `lib.ConvertToBifrostContext` function in the bifrost HTTP transport never returns nil and always returns a pointer to a valid context. It starts with context.Background() and only adds values to it, so nil checks are not needed when calling this function.
transports/bifrost-http/integrations/genai/router.go (11)
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#67
File: transports/bifrost-http/integrations/anthropic/router.go:26-34
Timestamp: 2025-06-10T11:19:29.604Z
Learning: The Generic router in transports/bifrost-http/integrations/utils.go already handles nil pointers from RequestConverter functions. When a RequestConverter returns a nil *schemas.BifrostRequest, the Generic router automatically returns an HTTP 400 error, making additional nil checks in individual router implementations redundant.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#67
File: transports/bifrost-http/integrations/anthropic/router.go:26-34
Timestamp: 2025-06-10T11:19:29.604Z
Learning: The Generic router in transports/bifrost-http/integrations/utils.go already handles nil pointers from RequestConverter functions. When a RequestConverter returns a nil *schemas.BifrostRequest, the Generic router automatically returns an HTTP 400 error with "Invalid request" message, making additional nil checks in individual router implementations redundant.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#65
File: transports/bifrost-http/integrations/anthropic/router.go:19-33
Timestamp: 2025-06-09T16:46:32.018Z
Learning: In the GenericRouter (transports/bifrost-http/integrations), ResponseFunc is not called if the BifrostResponse parameter is nil, providing built-in protection against nil response marshaling.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#89
File: transports/bifrost-http/integrations/genai/types.go:22-56
Timestamp: 2025-06-16T14:50:46.859Z
Learning: In the Google GenAI integration at transports/bifrost-http/integrations/genai/types.go, the manual URL-safe base64 decoding implementation (converting - to +, _ to /, and adding padding) is required because base64.RawURLEncoding.DecodeString fails for the specific url encoded bytes format being handled.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#80
File: plugins/maxim/main.go:249-264
Timestamp: 2025-06-14T06:17:54.426Z
Learning: In the BifrostError struct, the Error field is a struct (not a pointer), so accessing bifrostErr.Error.Message, bifrostErr.Error.Code, and bifrostErr.Error.Type is safe without nil checks on the Error field itself. The Code and Type fields are of type *string.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#89
File: transports/bifrost-http/integrations/genai/types.go:151-155
Timestamp: 2025-06-16T14:45:48.563Z
Learning: In transports/bifrost-http/integrations/genai/types.go, when SystemInstruction has an empty role, the user prefers to let the downstream provider (Google GenAI) handle validation and return errors, rather than implementing validation in the bifrost layer. This represents a design preference for delegating validation to the appropriate service rather than duplicating validation logic in the proxy layer.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#64
File: transports/bifrost-http/integrations/genai/types.go:273-313
Timestamp: 2025-06-09T16:35:26.914Z
Learning: In convertGenerationConfigToParams method in transports/bifrost-http/integrations/genai/types.go, pre-allocating the ExtraParams map is preferred over lazy allocation because the method has multiple potential ExtraParams assignments, making the computational overhead of conditional checks exceed the memory savings of an empty map.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#89
File: transports/bifrost-http/integrations/genai/types.go:151-155
Timestamp: 2025-06-16T14:44:42.893Z
Learning: In transports/bifrost-http/integrations/genai/types.go, when SystemInstruction has an empty role, the user prefers to return a validation error rather than setting a default role value. This represents a design preference for strict input validation over silent error correction.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#75
File: transports/bifrost-http/integrations/genai/types.go:109-117
Timestamp: 2025-06-14T08:54:58.137Z
Learning: In the Gemini/GenAI integration (transports/bifrost-http/integrations/genai/types.go), function calls don't have IDs, only names. Therefore, using the function name as ToolCallID is intentional and correct behavior, as Gemini's API doesn't provide call IDs.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#63
File: transports/bifrost-http/integrations/openai/router.go:62-64
Timestamp: 2025-06-09T11:27:00.925Z
Learning: The `lib.ConvertToBifrostContext` function in the bifrost HTTP transport never returns nil and handles the conversion internally, so nil checks are not needed when calling this function.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#81
File: tests/core-providers/scenarios/end_to_end_tool_calling.go:43-45
Timestamp: 2025-06-16T04:13:55.437Z
Learning: In the Bifrost codebase, errors returned from client methods like ChatCompletionRequest are of type BifrostError, not the standard error interface. For testing these errors, use require.Nilf instead of require.NoErrorf since BifrostError doesn't work with the standard error assertion methods.
tests/transports-integrations/README.md (9)
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#149
File: docs/contributing/README.md:22-27
Timestamp: 2025-07-08T18:12:13.590Z
Learning: In the Bifrost project, the tests directory structure has `tests/core-providers/` and `tests/transports-integrations/` as sibling directories. From `tests/core-providers/`, the correct relative path to reach `tests/transports-integrations/` is `../transports-integrations/`, not `../../tests/transports-integrations/`.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#54
File: core/schemas/provider.go:148-148
Timestamp: 2025-06-04T03:57:50.981Z
Learning: Breaking changes in the Bifrost codebase are managed by first merging and tagging core schema changes, then updating dependent code references in subsequent steps after the core version is released.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#84
File: transports/bifrost-http/main.go:2-2
Timestamp: 2025-06-15T16:05:13.489Z
Learning: For the Bifrost project, HTTP transport integration routers for new providers (like Mistral and Ollama) are implemented in separate PRs from the core provider support, following a focused PR strategy.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#103
File: .github/workflows/transport-dependency-update.yml:53-75
Timestamp: 2025-06-20T16:21:18.912Z
Learning: In the bifrost repository's transport dependency update workflow, when updating the core dependency to a new version using `go get`, the go.mod and go.sum files will always change in normal operation, making the safety check for changes more of a defensive programming practice rather than handling a common scenario.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#63
File: transports/bifrost-http/integrations/openai/router.go:0-0
Timestamp: 2025-06-09T14:03:34.227Z
Learning: In the Bifrost HTTP transport layer (transports/bifrost-http/integrations/), request validation like checking for empty messages should be handled by the provider rather than at the transport layer. The transport layer should forward requests to Bifrost core/providers for validation.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#82
File: tests/transports-integrations/tests/integrations/test_litellm.py:97-115
Timestamp: 2025-06-16T09:16:15.634Z
Learning: In the Bifrost integration tests, the user prefers tests to fail hard when API keys are missing rather than using @skip_if_no_api_key decorators for graceful skipping. This applies to LiteLLM tests that depend on OpenAI API keys.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#94
File: core/bifrost.go:823-845
Timestamp: 2025-06-18T15:15:51.323Z
Learning: In the Bifrost project, the team prioritizes maintaining consistent error handling patterns over exposing detailed error context. All errors should be wrapped in the standard `BifrostError` structure rather than creating specific error types or exposing richer error details like exit codes or stderr output.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#81
File: tests/core-providers/scenarios/automatic_function_calling.go:22-22
Timestamp: 2025-06-16T04:55:11.886Z
Learning: In the Bifrost test suite (tests/core-providers), parallel tests using t.Parallel() are not being implemented currently. The team plans to add parallel test execution in future enhancements.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#144
File: transports/bifrost-http/handlers/providers.go:45-49
Timestamp: 2025-07-08T16:50:27.699Z
Learning: In the Bifrost project, breaking API changes are acceptable when features are not yet public. This applies to scenarios like changing struct fields from pointer to non-pointer types in request/response structures for unreleased features.
tests/transports-integrations/tests/integrations/test_openai.py (7)
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#82
File: tests/transports-integrations/tests/integrations/test_litellm.py:97-115
Timestamp: 2025-06-16T09:16:15.634Z
Learning: In the Bifrost integration tests, the user prefers tests to fail hard when API keys are missing rather than using @skip_if_no_api_key decorators for graceful skipping. This applies to LiteLLM tests that depend on OpenAI API keys.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#63
File: transports/bifrost-http/integrations/openai/types.go:264-285
Timestamp: 2025-06-10T12:58:45.501Z
Learning: In the Bifrost OpenAI integration, tool calls should be allowed on any message role (not just assistant messages) and the downstream provider should handle validation. Users passing tool calls to non-assistant messages is considered deliberate behavior that should be preserved.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#63
File: transports/bifrost-http/integrations/openai/types.go:89-119
Timestamp: 2025-06-10T11:00:02.875Z
Learning: In the Bifrost OpenAI integration (transports/bifrost-http/integrations/openai/types.go), the convertOpenAIContent function currently only handles the last image URL when multiple images are present in a content array. The user Pratham-Mishra04 has acknowledged this limitation and indicated it's part of a larger architectural issue that will be addressed comprehensively later, rather than with piecemeal fixes.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#65
File: transports/bifrost-http/integrations/anthropic/types.go:129-145
Timestamp: 2025-06-10T11:12:26.883Z
Learning: Anthropic API does not support tool roles and images can only be present in user messages, not assistant or tool messages. Therefore, in Anthropic integration code, image content should always be assigned to UserMessage regardless of any other considerations.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#55
File: core/providers/bedrock.go:565-571
Timestamp: 2025-06-04T05:50:15.021Z
Learning: AWS Bedrock Converse API does not support message role "tool". Tool results must use role "user" with a specific "toolResult" content structure containing "toolUseId" and "content" fields. This differs from other providers that may accept "tool" role.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#65
File: transports/bifrost-http/integrations/anthropic/types.go:131-180
Timestamp: 2025-06-10T13:03:26.997Z
Learning: In Anthropic API message format, tool_result content blocks are sent by users in response to assistant tool calls, while image content blocks are typically sent in initial user messages. These content types follow different conversation flows and wouldn't naturally appear together in a single message, making defensive guards against multiple embedded message types unnecessary.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#74
File: core/providers/bedrock.go:909-916
Timestamp: 2025-06-11T15:05:45.355Z
Learning: In Bedrock chat responses (`core/providers/bedrock.go`), image content blocks are only possible in messages with the `user` role; assistant and tool messages will never contain image blocks.
tests/transports-integrations/tests/integrations/test_anthropic.py (5)
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#55
File: core/providers/anthropic.go:0-0
Timestamp: 2025-06-04T05:44:09.141Z
Learning: For the Anthropic provider in core/providers/anthropic.go, it's acceptable to pass potentially malformed messages with invalid roles because Anthropic's API will return suitable error responses for role issues, eliminating the need for additional validation logging.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#65
File: transports/bifrost-http/integrations/anthropic/types.go:129-145
Timestamp: 2025-06-10T11:12:26.883Z
Learning: Anthropic API does not support tool roles and images can only be present in user messages, not assistant or tool messages. Therefore, in Anthropic integration code, image content should always be assigned to UserMessage regardless of any other considerations.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#63
File: transports/bifrost-http/integrations/openai/types.go:264-285
Timestamp: 2025-06-10T12:58:45.501Z
Learning: In the Bifrost OpenAI integration, tool calls should be allowed on any message role (not just assistant messages) and the downstream provider should handle validation. Users passing tool calls to non-assistant messages is considered deliberate behavior that should be preserved.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#55
File: core/providers/bedrock.go:565-571
Timestamp: 2025-06-04T05:50:15.021Z
Learning: AWS Bedrock Converse API does not support message role "tool". Tool results must use role "user" with a specific "toolResult" content structure containing "toolUseId" and "content" fields. This differs from other providers that may accept "tool" role.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#65
File: transports/bifrost-http/integrations/anthropic/types.go:261-278
Timestamp: 2025-06-09T17:12:55.222Z
Learning: Anthropic's API consistently returns tool_choice enum values in lowercase only ("auto", "none", "any", "tool"), so case normalization is not needed when processing these values in the Anthropic integration layer.
tests/transports-integrations/tests/integrations/test_litellm.py (4)
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#82
File: tests/transports-integrations/tests/integrations/test_litellm.py:97-115
Timestamp: 2025-06-16T09:16:15.634Z
Learning: In the Bifrost integration tests, the user prefers tests to fail hard when API keys are missing rather than using @skip_if_no_api_key decorators for graceful skipping. This applies to LiteLLM tests that depend on OpenAI API keys.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#65
File: transports/bifrost-http/integrations/anthropic/types.go:129-145
Timestamp: 2025-06-10T11:12:26.883Z
Learning: Anthropic API does not support tool roles and images can only be present in user messages, not assistant or tool messages. Therefore, in Anthropic integration code, image content should always be assigned to UserMessage regardless of any other considerations.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#63
File: transports/bifrost-http/integrations/openai/types.go:264-285
Timestamp: 2025-06-10T12:58:45.501Z
Learning: In the Bifrost OpenAI integration, tool calls should be allowed on any message role (not just assistant messages) and the downstream provider should handle validation. Users passing tool calls to non-assistant messages is considered deliberate behavior that should be preserved.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#65
File: transports/bifrost-http/integrations/anthropic/types.go:131-180
Timestamp: 2025-06-10T13:03:26.997Z
Learning: In Anthropic API message format, tool_result content blocks are sent by users in response to assistant tool calls, while image content blocks are typically sent in initial user messages. These content types follow different conversation flows and wouldn't naturally appear together in a single message, making defensive guards against multiple embedded message types unnecessary.
transports/bifrost-http/integrations/openai/router.go (11)
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#67
File: transports/bifrost-http/integrations/anthropic/router.go:26-34
Timestamp: 2025-06-10T11:19:29.604Z
Learning: The Generic router in transports/bifrost-http/integrations/utils.go already handles nil pointers from RequestConverter functions. When a RequestConverter returns a nil *schemas.BifrostRequest, the Generic router automatically returns an HTTP 400 error, making additional nil checks in individual router implementations redundant.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#67
File: transports/bifrost-http/integrations/anthropic/router.go:26-34
Timestamp: 2025-06-10T11:19:29.604Z
Learning: The Generic router in transports/bifrost-http/integrations/utils.go already handles nil pointers from RequestConverter functions. When a RequestConverter returns a nil *schemas.BifrostRequest, the Generic router automatically returns an HTTP 400 error with "Invalid request" message, making additional nil checks in individual router implementations redundant.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#63
File: transports/bifrost-http/integrations/openai/types.go:89-119
Timestamp: 2025-06-10T11:00:02.875Z
Learning: In the Bifrost OpenAI integration (transports/bifrost-http/integrations/openai/types.go), the convertOpenAIContent function currently only handles the last image URL when multiple images are present in a content array. The user Pratham-Mishra04 has acknowledged this limitation and indicated it's part of a larger architectural issue that will be addressed comprehensively later, rather than with piecemeal fixes.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#81
File: tests/core-providers/scenarios/end_to_end_tool_calling.go:43-45
Timestamp: 2025-06-16T04:13:55.437Z
Learning: In the Bifrost codebase, errors returned from client methods like ChatCompletionRequest are of type BifrostError, not the standard error interface. For testing these errors, use require.Nilf instead of require.NoErrorf since BifrostError doesn't work with the standard error assertion methods.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#65
File: transports/bifrost-http/integrations/anthropic/router.go:19-33
Timestamp: 2025-06-09T16:46:32.018Z
Learning: In the GenericRouter (transports/bifrost-http/integrations), ResponseFunc is not called if the BifrostResponse parameter is nil, providing built-in protection against nil response marshaling.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#89
File: transports/bifrost-http/integrations/genai/types.go:22-56
Timestamp: 2025-06-16T14:50:46.859Z
Learning: In the Google GenAI integration at transports/bifrost-http/integrations/genai/types.go, the manual URL-safe base64 decoding implementation (converting - to +, _ to /, and adding padding) is required because base64.RawURLEncoding.DecodeString fails for the specific url encoded bytes format being handled.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#81
File: tests/core-providers/scenarios/simple_chat.go:39-41
Timestamp: 2025-06-16T04:13:42.755Z
Learning: In the Bifrost codebase, errors returned from methods like ChatCompletionRequest are of type BifrostError (a custom error type) rather than the standard Go error interface. Therefore, require.Nilf should be used for error assertions instead of require.NoErrorf.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#80
File: plugins/maxim/main.go:249-264
Timestamp: 2025-06-14T06:17:54.426Z
Learning: In the BifrostError struct, the Error field is a struct (not a pointer), so accessing bifrostErr.Error.Message, bifrostErr.Error.Code, and bifrostErr.Error.Type is safe without nil checks on the Error field itself. The Code and Type fields are of type *string.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#89
File: transports/bifrost-http/integrations/genai/types.go:151-155
Timestamp: 2025-06-16T14:45:48.563Z
Learning: In transports/bifrost-http/integrations/genai/types.go, when SystemInstruction has an empty role, the user prefers to let the downstream provider (Google GenAI) handle validation and return errors, rather than implementing validation in the bifrost layer. This represents a design preference for delegating validation to the appropriate service rather than duplicating validation logic in the proxy layer.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#54
File: core/schemas/bifrost.go:46-49
Timestamp: 2025-06-04T09:22:18.123Z
Learning: In core/schemas/bifrost.go, the RequestInput struct uses ChatCompletionInput *[]BifrostMessage (pointer-to-slice) rather than []BifrostMessage to properly represent union type semantics. For text completion requests, ChatCompletionInput should be nil to indicate "no chat payload at all", while for chat completion requests it should be non-nil (even if empty slice). This distinguishes between different request types rather than just empty vs non-empty chat messages.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#63
File: transports/bifrost-http/integrations/openai/router.go:62-64
Timestamp: 2025-06-09T11:27:00.925Z
Learning: The `lib.ConvertToBifrostContext` function in the bifrost HTTP transport never returns nil and handles the conversion internally, so nil checks are not needed when calling this function.
transports/bifrost-http/integrations/litellm/router.go (10)
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#67
File: transports/bifrost-http/integrations/anthropic/router.go:26-34
Timestamp: 2025-06-10T11:19:29.604Z
Learning: The Generic router in transports/bifrost-http/integrations/utils.go already handles nil pointers from RequestConverter functions. When a RequestConverter returns a nil *schemas.BifrostRequest, the Generic router automatically returns an HTTP 400 error, making additional nil checks in individual router implementations redundant.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#67
File: transports/bifrost-http/integrations/anthropic/router.go:26-34
Timestamp: 2025-06-10T11:19:29.604Z
Learning: The Generic router in transports/bifrost-http/integrations/utils.go already handles nil pointers from RequestConverter functions. When a RequestConverter returns a nil *schemas.BifrostRequest, the Generic router automatically returns an HTTP 400 error with "Invalid request" message, making additional nil checks in individual router implementations redundant.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#65
File: transports/bifrost-http/integrations/anthropic/router.go:19-33
Timestamp: 2025-06-09T16:46:32.018Z
Learning: In the GenericRouter (transports/bifrost-http/integrations), ResponseFunc is not called if the BifrostResponse parameter is nil, providing built-in protection against nil response marshaling.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#80
File: plugins/maxim/main.go:249-264
Timestamp: 2025-06-14T06:17:54.426Z
Learning: In the BifrostError struct, the Error field is a struct (not a pointer), so accessing bifrostErr.Error.Message, bifrostErr.Error.Code, and bifrostErr.Error.Type is safe without nil checks on the Error field itself. The Code and Type fields are of type *string.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#81
File: tests/core-providers/scenarios/end_to_end_tool_calling.go:43-45
Timestamp: 2025-06-16T04:13:55.437Z
Learning: In the Bifrost codebase, errors returned from client methods like ChatCompletionRequest are of type BifrostError, not the standard error interface. For testing these errors, use require.Nilf instead of require.NoErrorf since BifrostError doesn't work with the standard error assertion methods.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#149
File: transports/bifrost-http/main.go:259-261
Timestamp: 2025-07-08T18:20:24.086Z
Learning: In transports/bifrost-http/lib/store.go, the LoadFromConfig method already handles missing config files gracefully by initializing defaults and auto-detecting providers from environment variables. It only returns errors for actual parsing/processing failures, not missing files, making log.Fatalf appropriate in main.go since real errors should terminate the program.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#63
File: transports/bifrost-http/integrations/openai/router.go:62-64
Timestamp: 2025-06-09T11:27:00.925Z
Learning: The `lib.ConvertToBifrostContext` function in the bifrost HTTP transport never returns nil and handles the conversion internally, so nil checks are not needed when calling this function.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#81
File: tests/core-providers/scenarios/complete_end_to_end.go:39-41
Timestamp: 2025-06-16T04:12:05.427Z
Learning: In the Bifrost system, error returns are of type `BifrostError` rather than the standard Go `error` interface. Therefore, use `require.Nilf(t, err, ...)` instead of `require.NoError(t, err)` when checking for errors in Bifrost function calls.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#89
File: transports/bifrost-http/integrations/genai/types.go:151-155
Timestamp: 2025-06-16T14:45:48.563Z
Learning: In transports/bifrost-http/integrations/genai/types.go, when SystemInstruction has an empty role, the user prefers to let the downstream provider (Google GenAI) handle validation and return errors, rather than implementing validation in the bifrost layer. This represents a design preference for delegating validation to the appropriate service rather than duplicating validation logic in the proxy layer.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#81
File: tests/core-providers/scenarios/simple_chat.go:39-41
Timestamp: 2025-06-16T04:13:42.755Z
Learning: In the Bifrost codebase, errors returned from methods like ChatCompletionRequest are of type BifrostError (a custom error type) rather than the standard Go error interface. Therefore, require.Nilf should be used for error assertions instead of require.NoErrorf.
tests/transports-integrations/tests/integrations/test_google.py (3)
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#89
File: transports/bifrost-http/integrations/genai/types.go:151-155
Timestamp: 2025-06-16T14:45:48.563Z
Learning: In transports/bifrost-http/integrations/genai/types.go, when SystemInstruction has an empty role, the user prefers to let the downstream provider (Google GenAI) handle validation and return errors, rather than implementing validation in the bifrost layer. This represents a design preference for delegating validation to the appropriate service rather than duplicating validation logic in the proxy layer.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#65
File: transports/bifrost-http/integrations/anthropic/types.go:129-145
Timestamp: 2025-06-10T11:12:26.883Z
Learning: Anthropic API does not support tool roles and images can only be present in user messages, not assistant or tool messages. Therefore, in Anthropic integration code, image content should always be assigned to UserMessage regardless of any other considerations.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#63
File: transports/bifrost-http/integrations/openai/types.go:264-285
Timestamp: 2025-06-10T12:58:45.501Z
Learning: In the Bifrost OpenAI integration, tool calls should be allowed on any message role (not just assistant messages) and the downstream provider should handle validation. Users passing tool calls to non-assistant messages is considered deliberate behavior that should be preserved.
core/bifrost.go (13)
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#83
File: core/providers/mistral.go:96-101
Timestamp: 2025-06-15T13:50:41.418Z
Learning: In the Bifrost project, when a provider doesn't support a specific operation (like text completion), the IsBifrostError flag should be set to false to mark it as a provider-level error rather than a Bifrost framework error. This is intentional design for unsupported operations.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#81
File: tests/core-providers/scenarios/end_to_end_tool_calling.go:43-45
Timestamp: 2025-06-16T04:13:55.437Z
Learning: In the Bifrost codebase, errors returned from client methods like ChatCompletionRequest are of type BifrostError, not the standard error interface. For testing these errors, use require.Nilf instead of require.NoErrorf since BifrostError doesn't work with the standard error assertion methods.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#81
File: tests/core-providers/scenarios/simple_chat.go:39-41
Timestamp: 2025-06-16T04:13:42.755Z
Learning: In the Bifrost codebase, errors returned from methods like ChatCompletionRequest are of type BifrostError (a custom error type) rather than the standard Go error interface. Therefore, require.Nilf should be used for error assertions instead of require.NoErrorf.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#54
File: core/schemas/bifrost.go:46-49
Timestamp: 2025-06-04T09:22:18.123Z
Learning: In core/schemas/bifrost.go, the RequestInput struct uses ChatCompletionInput *[]BifrostMessage (pointer-to-slice) rather than []BifrostMessage to properly represent union type semantics. For text completion requests, ChatCompletionInput should be nil to indicate "no chat payload at all", while for chat completion requests it should be non-nil (even if empty slice). This distinguishes between different request types rather than just empty vs non-empty chat messages.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#94
File: core/bifrost.go:823-845
Timestamp: 2025-06-18T15:15:51.323Z
Learning: In the Bifrost project, the team prioritizes maintaining consistent error handling patterns over exposing detailed error context. All errors should be wrapped in the standard `BifrostError` structure rather than creating specific error types or exposing richer error details like exit codes or stderr output.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#80
File: plugins/maxim/main.go:249-264
Timestamp: 2025-06-14T06:17:54.426Z
Learning: In the BifrostError struct, the Error field is a struct (not a pointer), so accessing bifrostErr.Error.Message, bifrostErr.Error.Code, and bifrostErr.Error.Type is safe without nil checks on the Error field itself. The Code and Type fields are of type *string.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#83
File: core/providers/mistral.go:168-170
Timestamp: 2025-06-15T14:24:49.882Z
Learning: In the Bifrost codebase, performance is prioritized over defensive copying for HTTP service operations. Specifically, shallow slice assignments in provider response handling are acceptable due to object pool reset patterns and JSON unmarshaling behavior that minimize practical data corruption risks.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#63
File: transports/bifrost-http/integrations/openai/router.go:0-0
Timestamp: 2025-06-09T14:03:34.227Z
Learning: In the Bifrost HTTP transport layer (transports/bifrost-http/integrations/), request validation like checking for empty messages should be handled by the provider rather than at the transport layer. The transport layer should forward requests to Bifrost core/providers for validation.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#81
File: tests/core-providers/scenarios/complete_end_to_end.go:39-41
Timestamp: 2025-06-16T04:12:05.427Z
Learning: In the Bifrost system, error returns are of type `BifrostError` rather than the standard Go `error` interface. Therefore, use `require.Nilf(t, err, ...)` instead of `require.NoError(t, err)` when checking for errors in Bifrost function calls.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#65
File: transports/bifrost-http/integrations/utils.go:169-173
Timestamp: 2025-06-09T17:33:52.234Z
Learning: The ChatCompletionRequest method in the Bifrost client follows a contract where the result parameter will never be nil if the error parameter is nil. This means when error checking passes (err == nil), the result is guaranteed to be valid and can be safely used without additional nil checks.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#84
File: core/providers/ollama.go:59-61
Timestamp: 2025-06-15T14:34:29.401Z
Learning: The `CheckAndSetDefaults()` method on `*schemas.ProviderConfig` in Go does not return any error - it has a void return type and only sets default values on the configuration struct.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#67
File: transports/bifrost-http/integrations/anthropic/router.go:26-34
Timestamp: 2025-06-10T11:19:29.604Z
Learning: The Generic router in transports/bifrost-http/integrations/utils.go already handles nil pointers from RequestConverter functions. When a RequestConverter returns a nil *schemas.BifrostRequest, the Generic router automatically returns an HTTP 400 error, making additional nil checks in individual router implementations redundant.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#67
File: transports/bifrost-http/integrations/anthropic/router.go:26-34
Timestamp: 2025-06-10T11:19:29.604Z
Learning: The Generic router in transports/bifrost-http/integrations/utils.go already handles nil pointers from RequestConverter functions. When a RequestConverter returns a nil *schemas.BifrostRequest, the Generic router automatically returns an HTTP 400 error with "Invalid request" message, making additional nil checks in individual router implementations redundant.
transports/bifrost-http/integrations/anthropic/types.go (15)
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#55
File: core/providers/anthropic.go:526-550
Timestamp: 2025-06-04T09:29:46.287Z
Learning: In core/providers/anthropic.go, the content field in formattedMessages is always of type []interface{} because it's explicitly constructed that way upstream in the prepareAnthropicChatRequest function. Defensive type casting for multiple types is not needed since the type is guaranteed by the construction logic.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#80
File: plugins/maxim/main.go:249-264
Timestamp: 2025-06-14T06:17:54.426Z
Learning: In the BifrostError struct, the Error field is a struct (not a pointer), so accessing bifrostErr.Error.Message, bifrostErr.Error.Code, and bifrostErr.Error.Type is safe without nil checks on the Error field itself. The Code and Type fields are of type *string.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#55
File: core/providers/anthropic.go:0-0
Timestamp: 2025-06-04T05:44:09.141Z
Learning: For the Anthropic provider in core/providers/anthropic.go, it's acceptable to pass potentially malformed messages with invalid roles because Anthropic's API will return suitable error responses for role issues, eliminating the need for additional validation logging.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#65
File: transports/bifrost-http/integrations/anthropic/types.go:131-180
Timestamp: 2025-06-10T13:11:37.867Z
Learning: In Anthropic API integration for Bifrost, messages won't contain both image and tool_result content blocks in the same message, so defensive guards against multiple embedded message structs are unnecessary in the content processing loop.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#81
File: tests/core-providers/scenarios/end_to_end_tool_calling.go:43-45
Timestamp: 2025-06-16T04:13:55.437Z
Learning: In the Bifrost codebase, errors returned from client methods like ChatCompletionRequest are of type BifrostError, not the standard error interface. For testing these errors, use require.Nilf instead of require.NoErrorf since BifrostError doesn't work with the standard error assertion methods.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#81
File: tests/core-providers/scenarios/simple_chat.go:39-41
Timestamp: 2025-06-16T04:13:42.755Z
Learning: In the Bifrost codebase, errors returned from methods like ChatCompletionRequest are of type BifrostError (a custom error type) rather than the standard Go error interface. Therefore, require.Nilf should be used for error assertions instead of require.NoErrorf.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#94
File: core/bifrost.go:823-845
Timestamp: 2025-06-18T15:15:51.323Z
Learning: In the Bifrost project, the team prioritizes maintaining consistent error handling patterns over exposing detailed error context. All errors should be wrapped in the standard `BifrostError` structure rather than creating specific error types or exposing richer error details like exit codes or stderr output.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#55
File: core/providers/anthropic.go:483-494
Timestamp: 2025-06-04T09:36:27.956Z
Learning: In the Anthropic provider (core/providers/anthropic.go), the user has confirmed through practical experience that the tool_choice parameter should always use an object format with a "type" field (e.g., {"type": "auto"}, {"type": "tool", "name": "function_name"}), even though the official documentation examples sometimes show "auto" as a direct string. The current implementation correctly handles all tool choice types with the object format.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#54
File: core/schemas/bifrost.go:46-49
Timestamp: 2025-06-04T09:22:18.123Z
Learning: In core/schemas/bifrost.go, the RequestInput struct uses ChatCompletionInput *[]BifrostMessage (pointer-to-slice) rather than []BifrostMessage to properly represent union type semantics. For text completion requests, ChatCompletionInput should be nil to indicate "no chat payload at all", while for chat completion requests it should be non-nil (even if empty slice). This distinguishes between different request types rather than just empty vs non-empty chat messages.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#65
File: transports/bifrost-http/integrations/anthropic/types.go:140-146
Timestamp: 2025-06-10T13:51:52.859Z
Learning: In Bifrost core v1.0.9, ImageContent.Type was a pointer type (*string accessed via bifrost.Ptr), but in v1.0.10 it was changed to a value type (ImageContentType). When reviewing code, check the core version being used to determine the correct assignment pattern.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#65
File: transports/bifrost-http/integrations/anthropic/types.go:131-180
Timestamp: 2025-06-10T13:03:26.997Z
Learning: In Anthropic API message format, tool_result content blocks are sent by users in response to assistant tool calls, while image content blocks are typically sent in initial user messages. These content types follow different conversation flows and wouldn't naturally appear together in a single message, making defensive guards against multiple embedded message types unnecessary.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#64
File: transports/bifrost-http/integrations/genai/types.go:383-395
Timestamp: 2025-06-09T16:26:05.777Z
Learning: In the Bifrost schema (core/schemas/bifrost.go), only UserMessage and ToolMessage structs have ImageContent fields. AssistantMessage does not have an ImageContent field and cannot contain images. The schema design intentionally limits image content to user and tool messages only.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#144
File: transports/bifrost-http/plugins/logging/utils.go:509-514
Timestamp: 2025-07-08T16:48:25.386Z
Learning: In the Bifrost logging system (transports/bifrost-http/plugins/logging/utils.go), the Content field in message structures is a struct value type, not a pointer, so it will never be nil and doesn't require nil checks. However, ContentStr within Content is a pointer and should be checked for nil.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#75
File: transports/bifrost-http/integrations/anthropic/types.go:154-160
Timestamp: 2025-06-14T04:19:06.327Z
Learning: Anthropic's API does not support images in system messages, only text content is allowed in system messages for Anthropic integrations.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#81
File: tests/core-providers/scenarios/complete_end_to_end.go:39-41
Timestamp: 2025-06-16T04:12:05.427Z
Learning: In the Bifrost system, error returns are of type `BifrostError` rather than the standard Go `error` interface. Therefore, use `require.Nilf(t, err, ...)` instead of `require.NoError(t, err)` when checking for errors in Bifrost function calls.
transports/bifrost-http/integrations/genai/types.go (14)
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#75
File: transports/bifrost-http/integrations/genai/types.go:109-117
Timestamp: 2025-06-14T08:54:58.137Z
Learning: In the Gemini/GenAI integration (transports/bifrost-http/integrations/genai/types.go), function calls don't have IDs, only names. Therefore, using the function name as ToolCallID is intentional and correct behavior, as Gemini's API doesn't provide call IDs.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#81
File: tests/core-providers/scenarios/end_to_end_tool_calling.go:43-45
Timestamp: 2025-06-16T04:13:55.437Z
Learning: In the Bifrost codebase, errors returned from client methods like ChatCompletionRequest are of type BifrostError, not the standard error interface. For testing these errors, use require.Nilf instead of require.NoErrorf since BifrostError doesn't work with the standard error assertion methods.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#81
File: tests/core-providers/scenarios/simple_chat.go:39-41
Timestamp: 2025-06-16T04:13:42.755Z
Learning: In the Bifrost codebase, errors returned from methods like ChatCompletionRequest are of type BifrostError (a custom error type) rather than the standard Go error interface. Therefore, require.Nilf should be used for error assertions instead of require.NoErrorf.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#54
File: core/schemas/bifrost.go:46-49
Timestamp: 2025-06-04T09:22:18.123Z
Learning: In core/schemas/bifrost.go, the RequestInput struct uses ChatCompletionInput *[]BifrostMessage (pointer-to-slice) rather than []BifrostMessage to properly represent union type semantics. For text completion requests, ChatCompletionInput should be nil to indicate "no chat payload at all", while for chat completion requests it should be non-nil (even if empty slice). This distinguishes between different request types rather than just empty vs non-empty chat messages.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#89
File: transports/bifrost-http/integrations/genai/types.go:22-56
Timestamp: 2025-06-16T14:50:46.859Z
Learning: In the Google GenAI integration at transports/bifrost-http/integrations/genai/types.go, the manual URL-safe base64 decoding implementation (converting - to +, _ to /, and adding padding) is required because base64.RawURLEncoding.DecodeString fails for the specific url encoded bytes format being handled.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#80
File: plugins/maxim/main.go:249-264
Timestamp: 2025-06-14T06:17:54.426Z
Learning: In the BifrostError struct, the Error field is a struct (not a pointer), so accessing bifrostErr.Error.Message, bifrostErr.Error.Code, and bifrostErr.Error.Type is safe without nil checks on the Error field itself. The Code and Type fields are of type *string.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#89
File: transports/bifrost-http/integrations/genai/types.go:151-155
Timestamp: 2025-06-16T14:45:48.563Z
Learning: In transports/bifrost-http/integrations/genai/types.go, when SystemInstruction has an empty role, the user prefers to let the downstream provider (Google GenAI) handle validation and return errors, rather than implementing validation in the bifrost layer. This represents a design preference for delegating validation to the appropriate service rather than duplicating validation logic in the proxy layer.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#55
File: core/providers/anthropic.go:526-550
Timestamp: 2025-06-04T09:29:46.287Z
Learning: In core/providers/anthropic.go, the content field in formattedMessages is always of type []interface{} because it's explicitly constructed that way upstream in the prepareAnthropicChatRequest function. Defensive type casting for multiple types is not needed since the type is guaranteed by the construction logic.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#89
File: transports/bifrost-http/integrations/genai/types.go:151-155
Timestamp: 2025-06-16T14:44:42.893Z
Learning: In transports/bifrost-http/integrations/genai/types.go, when SystemInstruction has an empty role, the user prefers to return a validation error rather than setting a default role value. This represents a design preference for strict input validation over silent error correction.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#67
File: transports/bifrost-http/integrations/anthropic/router.go:26-34
Timestamp: 2025-06-10T11:19:29.604Z
Learning: The Generic router in transports/bifrost-http/integrations/utils.go already handles nil pointers from RequestConverter functions. When a RequestConverter returns a nil *schemas.BifrostRequest, the Generic router automatically returns an HTTP 400 error, making additional nil checks in individual router implementations redundant.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#65
File: transports/bifrost-http/integrations/utils.go:169-173
Timestamp: 2025-06-09T17:33:52.234Z
Learning: The ChatCompletionRequest method in the Bifrost client follows a contract where the result parameter will never be nil if the error parameter is nil. This means when error checking passes (err == nil), the result is guaranteed to be valid and can be safely used without additional nil checks.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#67
File: transports/bifrost-http/integrations/anthropic/router.go:26-34
Timestamp: 2025-06-10T11:19:29.604Z
Learning: The Generic router in transports/bifrost-http/integrations/utils.go already handles nil pointers from RequestConverter functions. When a RequestConverter returns a nil *schemas.BifrostRequest, the Generic router automatically returns an HTTP 400 error with "Invalid request" message, making additional nil checks in individual router implementations redundant.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#65
File: transports/bifrost-http/integrations/anthropic/router.go:19-33
Timestamp: 2025-06-09T16:46:32.018Z
Learning: In the GenericRouter (transports/bifrost-http/integrations), ResponseFunc is not called if the BifrostResponse parameter is nil, providing built-in protection against nil response marshaling.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#81
File: tests/core-providers/scenarios/complete_end_to_end.go:39-41
Timestamp: 2025-06-16T04:12:05.427Z
Learning: In the Bifrost system, error returns are of type `BifrostError` rather than the standard Go `error` interface. Therefore, use `require.Nilf(t, err, ...)` instead of `require.NoError(t, err)` when checking for errors in Bifrost function calls.
transports/bifrost-http/integrations/utils.go (17)
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#67
File: transports/bifrost-http/integrations/anthropic/router.go:26-34
Timestamp: 2025-06-10T11:19:29.604Z
Learning: The Generic router in transports/bifrost-http/integrations/utils.go already handles nil pointers from RequestConverter functions. When a RequestConverter returns a nil *schemas.BifrostRequest, the Generic router automatically returns an HTTP 400 error, making additional nil checks in individual router implementations redundant.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#67
File: transports/bifrost-http/integrations/anthropic/router.go:26-34
Timestamp: 2025-06-10T11:19:29.604Z
Learning: The Generic router in transports/bifrost-http/integrations/utils.go already handles nil pointers from RequestConverter functions. When a RequestConverter returns a nil *schemas.BifrostRequest, the Generic router automatically returns an HTTP 400 error with "Invalid request" message, making additional nil checks in individual router implementations redundant.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#65
File: transports/bifrost-http/integrations/anthropic/router.go:19-33
Timestamp: 2025-06-09T16:46:32.018Z
Learning: In the GenericRouter (transports/bifrost-http/integrations), ResponseFunc is not called if the BifrostResponse parameter is nil, providing built-in protection against nil response marshaling.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#80
File: plugins/maxim/main.go:249-264
Timestamp: 2025-06-14T06:17:54.426Z
Learning: In the BifrostError struct, the Error field is a struct (not a pointer), so accessing bifrostErr.Error.Message, bifrostErr.Error.Code, and bifrostErr.Error.Type is safe without nil checks on the Error field itself. The Code and Type fields are of type *string.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#81
File: tests/core-providers/scenarios/end_to_end_tool_calling.go:43-45
Timestamp: 2025-06-16T04:13:55.437Z
Learning: In the Bifrost codebase, errors returned from client methods like ChatCompletionRequest are of type BifrostError, not the standard error interface. For testing these errors, use require.Nilf instead of require.NoErrorf since BifrostError doesn't work with the standard error assertion methods.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#89
File: transports/bifrost-http/integrations/genai/types.go:22-56
Timestamp: 2025-06-16T14:50:46.859Z
Learning: In the Google GenAI integration at transports/bifrost-http/integrations/genai/types.go, the manual URL-safe base64 decoding implementation (converting - to +, _ to /, and adding padding) is required because base64.RawURLEncoding.DecodeString fails for the specific url encoded bytes format being handled.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#89
File: transports/bifrost-http/integrations/genai/types.go:151-155
Timestamp: 2025-06-16T14:45:48.563Z
Learning: In transports/bifrost-http/integrations/genai/types.go, when SystemInstruction has an empty role, the user prefers to let the downstream provider (Google GenAI) handle validation and return errors, rather than implementing validation in the bifrost layer. This represents a design preference for delegating validation to the appropriate service rather than duplicating validation logic in the proxy layer.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#89
File: transports/bifrost-http/integrations/genai/types.go:151-155
Timestamp: 2025-06-16T14:44:42.893Z
Learning: In transports/bifrost-http/integrations/genai/types.go, when SystemInstruction has an empty role, the user prefers to return a validation error rather than setting a default role value. This represents a design preference for strict input validation over silent error correction.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#94
File: core/bifrost.go:823-845
Timestamp: 2025-06-18T15:15:51.323Z
Learning: In the Bifrost project, the team prioritizes maintaining consistent error handling patterns over exposing detailed error context. All errors should be wrapped in the standard `BifrostError` structure rather than creating specific error types or exposing richer error details like exit codes or stderr output.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#63
File: transports/bifrost-http/integrations/openai/router.go:62-64
Timestamp: 2025-06-09T11:27:00.925Z
Learning: The `lib.ConvertToBifrostContext` function in the bifrost HTTP transport never returns nil and handles the conversion internally, so nil checks are not needed when calling this function.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#81
File: tests/core-providers/scenarios/simple_chat.go:39-41
Timestamp: 2025-06-16T04:13:42.755Z
Learning: In the Bifrost codebase, errors returned from methods like ChatCompletionRequest are of type BifrostError (a custom error type) rather than the standard Go error interface. Therefore, require.Nilf should be used for error assertions instead of require.NoErrorf.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#63
File: transports/bifrost-http/integrations/openai/router.go:62-64
Timestamp: 2025-06-09T11:27:00.925Z
Learning: The `lib.ConvertToBifrostContext` function in the bifrost HTTP transport never returns nil and always returns a pointer to a valid context. It starts with context.Background() and only adds values to it, so nil checks are not needed when calling this function.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#81
File: tests/core-providers/scenarios/complete_end_to_end.go:39-41
Timestamp: 2025-06-16T04:12:05.427Z
Learning: In the Bifrost system, error returns are of type `BifrostError` rather than the standard Go `error` interface. Therefore, use `require.Nilf(t, err, ...)` instead of `require.NoError(t, err)` when checking for errors in Bifrost function calls.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#94
File: core/schemas/bifrost.go:20-23
Timestamp: 2025-06-18T15:16:23.127Z
Learning: In the Bifrost project, BifrostConfig struct is never marshaled/unmarshaled, so serialization tags (json, yaml) are not needed for its fields.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#152
File: transports/bifrost-http/plugins/logging/utils.go:94-111
Timestamp: 2025-07-10T13:44:23.255Z
Learning: Pratham-Mishra04 prefers not to add error handling for JSON marshaling operations in the Bifrost logging plugin (transports/bifrost-http/plugins/logging/utils.go) because logging is not critical functionality and the structured schema data being marshaled is unlikely to fail. They accept the risk of not handling json.Marshal errors in logging contexts to keep the code simple.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#152
File: transports/bifrost-http/plugins/logging/utils.go:378-399
Timestamp: 2025-07-10T13:44:14.471Z
Learning: In the Bifrost logging plugin (transports/bifrost-http/plugins/logging/utils.go), Pratham-Mishra04 prefers not to add error handling for JSON unmarshaling operations, considering logging not very critical and being confident that JSON marshalling won't fail in practice.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#65
File: transports/bifrost-http/integrations/utils.go:169-173
Timestamp: 2025-06-09T17:33:52.234Z
Learning: The ChatCompletionRequest method in the Bifrost client follows a contract where the result parameter will never be nil if the error parameter is nil. This means when error checking passes (err == nil), the result is guaranteed to be valid and can be safely used without additional nil checks.
transports/bifrost-http/integrations/openai/types.go (14)
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#81
File: tests/core-providers/scenarios/end_to_end_tool_calling.go:43-45
Timestamp: 2025-06-16T04:13:55.437Z
Learning: In the Bifrost codebase, errors returned from client methods like ChatCompletionRequest are of type BifrostError, not the standard error interface. For testing these errors, use require.Nilf instead of require.NoErrorf since BifrostError doesn't work with the standard error assertion methods.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#54
File: core/schemas/bifrost.go:46-49
Timestamp: 2025-06-04T09:22:18.123Z
Learning: In core/schemas/bifrost.go, the RequestInput struct uses ChatCompletionInput *[]BifrostMessage (pointer-to-slice) rather than []BifrostMessage to properly represent union type semantics. For text completion requests, ChatCompletionInput should be nil to indicate "no chat payload at all", while for chat completion requests it should be non-nil (even if empty slice). This distinguishes between different request types rather than just empty vs non-empty chat messages.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#63
File: transports/bifrost-http/integrations/openai/types.go:89-119
Timestamp: 2025-06-10T11:00:02.875Z
Learning: In the Bifrost OpenAI integration (transports/bifrost-http/integrations/openai/types.go), the convertOpenAIContent function currently only handles the last image URL when multiple images are present in a content array. The user Pratham-Mishra04 has acknowledged this limitation and indicated it's part of a larger architectural issue that will be addressed comprehensively later, rather than with piecemeal fixes.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#80
File: plugins/maxim/main.go:249-264
Timestamp: 2025-06-14T06:17:54.426Z
Learning: In the BifrostError struct, the Error field is a struct (not a pointer), so accessing bifrostErr.Error.Message, bifrostErr.Error.Code, and bifrostErr.Error.Type is safe without nil checks on the Error field itself. The Code and Type fields are of type *string.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#81
File: tests/core-providers/scenarios/simple_chat.go:39-41
Timestamp: 2025-06-16T04:13:42.755Z
Learning: In the Bifrost codebase, errors returned from methods like ChatCompletionRequest are of type BifrostError (a custom error type) rather than the standard Go error interface. Therefore, require.Nilf should be used for error assertions instead of require.NoErrorf.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#63
File: transports/bifrost-http/integrations/openai/types.go:223-231
Timestamp: 2025-06-10T11:06:06.670Z
Learning: The OpenAI `name` field on messages cannot be preserved when converting to Bifrost format because the `schemas.BifrostMessage` struct in bifrost/core does not support a Name field. This is a known limitation of the Bifrost core schema design.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#55
File: core/providers/anthropic.go:526-550
Timestamp: 2025-06-04T09:29:46.287Z
Learning: In core/providers/anthropic.go, the content field in formattedMessages is always of type []interface{} because it's explicitly constructed that way upstream in the prepareAnthropicChatRequest function. Defensive type casting for multiple types is not needed since the type is guaranteed by the construction logic.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#89
File: transports/bifrost-http/integrations/genai/types.go:22-56
Timestamp: 2025-06-16T14:50:46.859Z
Learning: In the Google GenAI integration at transports/bifrost-http/integrations/genai/types.go, the manual URL-safe base64 decoding implementation (converting - to +, _ to /, and adding padding) is required because base64.RawURLEncoding.DecodeString fails for the specific url encoded bytes format being handled.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#144
File: transports/bifrost-http/handlers/providers.go:45-49
Timestamp: 2025-07-08T16:50:27.699Z
Learning: In the Bifrost project, breaking API changes are acceptable when features are not yet public. This applies to scenarios like changing struct fields from pointer to non-pointer types in request/response structures for unreleased features.
Learnt from: connyay
PR: maximhq/bifrost#92
File: core/providers/utils.go:60-64
Timestamp: 2025-06-17T14:04:21.104Z
Learning: The bifrost codebase uses string alias types (like `type ContextKey string`) for context keys consistently across multiple packages including plugins/maxim/main.go and transports/bifrost-http/tracking/plugin.go. This pattern should be maintained for consistency rather than switching to the canonical struct{} approach.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#65
File: transports/bifrost-http/integrations/utils.go:169-173
Timestamp: 2025-06-09T17:33:52.234Z
Learning: The ChatCompletionRequest method in the Bifrost client follows a contract where the result parameter will never be nil if the error parameter is nil. This means when error checking passes (err == nil), the result is guaranteed to be valid and can be safely used without additional nil checks.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#81
File: tests/core-providers/scenarios/complete_end_to_end.go:39-41
Timestamp: 2025-06-16T04:12:05.427Z
Learning: In the Bifrost system, error returns are of type `BifrostError` rather than the standard Go `error` interface. Therefore, use `require.Nilf(t, err, ...)` instead of `require.NoError(t, err)` when checking for errors in Bifrost function calls.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#94
File: core/bifrost.go:823-845
Timestamp: 2025-06-18T15:15:51.323Z
Learning: In the Bifrost project, the team prioritizes maintaining consistent error handling patterns over exposing detailed error context. All errors should be wrapped in the standard `BifrostError` structure rather than creating specific error types or exposing richer error details like exit codes or stderr output.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#63
File: transports/bifrost-http/integrations/openai/router.go:62-64
Timestamp: 2025-06-09T11:27:00.925Z
Learning: The `lib.ConvertToBifrostContext` function in the bifrost HTTP transport never returns nil and handles the conversion internally, so nil checks are not needed when calling this function.
tests/transports-integrations/tests/utils/common.py (3)
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#82
File: tests/transports-integrations/tests/integrations/test_litellm.py:97-115
Timestamp: 2025-06-16T09:16:15.634Z
Learning: In the Bifrost integration tests, the user prefers tests to fail hard when API keys are missing rather than using @skip_if_no_api_key decorators for graceful skipping. This applies to LiteLLM tests that depend on OpenAI API keys.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#94
File: core/bifrost.go:823-845
Timestamp: 2025-06-18T15:15:51.323Z
Learning: In the Bifrost project, the team prioritizes maintaining consistent error handling patterns over exposing detailed error context. All errors should be wrapped in the standard `BifrostError` structure rather than creating specific error types or exposing richer error details like exit codes or stderr output.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#63
File: transports/bifrost-http/integrations/openai/types.go:264-285
Timestamp: 2025-06-10T12:58:45.501Z
Learning: In the Bifrost OpenAI integration, tool calls should be allowed on any message role (not just assistant messages) and the downstream provider should handle validation. Users passing tool calls to non-assistant messages is considered deliberate behavior that should be preserved.
🧬 Code Graph Analysis (12)
core/schemas/bifrost.go (1)
core/schemas/provider.go (1)
Provider(154-163)
transports/bifrost-http/integrations/anthropic/router.go (3)
transports/bifrost-http/integrations/utils.go (1)
ErrorConverter(32-32)core/schemas/bifrost.go (1)
BifrostError(433-441)transports/bifrost-http/integrations/anthropic/types.go (1)
DeriveAnthropicErrorFromBifrostError(481-506)
tests/transports-integrations/tests/integrations/test_openai.py (5)
tests/transports-integrations/tests/utils/common.py (2)
assert_valid_error_response(464-547)assert_error_propagation(550-635)tests/transports-integrations/tests/integrations/test_google.py (2)
test_12_error_handling_invalid_roles(430-440)test_config(75-77)tests/transports-integrations/tests/integrations/test_litellm.py (2)
test_12_error_handling_invalid_roles(347-359)test_config(59-61)tests/transports-integrations/tests/integrations/test_anthropic.py (2)
test_12_error_handling_invalid_roles(552-564)test_config(88-90)tests/transports-integrations/tests/utils/config_loader.py (2)
get_model(128-140)get_model(280-282)
tests/transports-integrations/tests/integrations/test_anthropic.py (5)
tests/transports-integrations/tests/utils/common.py (2)
assert_valid_error_response(464-547)assert_error_propagation(550-635)tests/transports-integrations/tests/integrations/test_google.py (2)
test_12_error_handling_invalid_roles(430-440)test_config(75-77)tests/transports-integrations/tests/integrations/test_litellm.py (2)
test_12_error_handling_invalid_roles(347-359)test_config(59-61)tests/transports-integrations/tests/integrations/test_openai.py (2)
test_12_error_handling_invalid_roles(397-409)test_config(130-132)tests/transports-integrations/tests/utils/config_loader.py (2)
get_model(128-140)get_model(280-282)
transports/bifrost-http/integrations/openai/router.go (3)
transports/bifrost-http/integrations/utils.go (1)
ErrorConverter(32-32)core/schemas/bifrost.go (1)
BifrostError(433-441)transports/bifrost-http/integrations/openai/types.go (1)
DeriveOpenAIErrorFromBifrostError(157-199)
transports/bifrost-http/integrations/litellm/router.go (5)
core/schemas/bifrost.go (5)
BifrostError(433-441)OpenAI(40-40)Azure(41-41)Anthropic(42-42)Vertex(45-45)transports/bifrost-http/integrations/openai/types.go (1)
DeriveOpenAIErrorFromBifrostError(157-199)transports/bifrost-http/integrations/anthropic/types.go (1)
DeriveAnthropicErrorFromBifrostError(481-506)transports/bifrost-http/integrations/genai/types.go (1)
DeriveGeminiErrorFromBifrostError(583-606)transports/bifrost-http/integrations/utils.go (1)
ErrorConverter(32-32)
tests/transports-integrations/tests/integrations/test_google.py (5)
tests/transports-integrations/tests/utils/common.py (4)
assert_valid_error_response(464-547)assert_error_propagation(550-635)get_api_key(657-674)skip_if_no_api_key(677-688)tests/transports-integrations/tests/integrations/test_litellm.py (2)
test_12_error_handling_invalid_roles(347-359)test_config(59-61)tests/transports-integrations/tests/integrations/test_anthropic.py (2)
test_12_error_handling_invalid_roles(552-564)test_config(88-90)tests/transports-integrations/tests/integrations/test_openai.py (2)
test_12_error_handling_invalid_roles(397-409)test_config(130-132)tests/transports-integrations/tests/utils/config_loader.py (2)
get_model(128-140)get_model(280-282)
core/bifrost.go (2)
core/schemas/provider.go (1)
Provider(154-163)core/schemas/bifrost.go (1)
RequestCancelled(424-424)
transports/bifrost-http/integrations/anthropic/types.go (1)
core/schemas/bifrost.go (1)
BifrostError(433-441)
transports/bifrost-http/integrations/genai/types.go (1)
core/schemas/bifrost.go (1)
BifrostError(433-441)
transports/bifrost-http/integrations/utils.go (2)
core/schemas/bifrost.go (1)
BifrostError(433-441)transports/bifrost-http/lib/ctx.go (1)
ConvertToBifrostContext(54-100)
transports/bifrost-http/integrations/openai/types.go (1)
core/schemas/bifrost.go (1)
BifrostError(433-441)
🪛 LanguageTool
tests/transports-integrations/README.md
[grammar] ~37-~37: Use correct spacing
Context: ...ers 12 comprehensive scenarios for each integration: 1. Simple Chat - Basic single-message co...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
🪛 Ruff (0.11.9)
tests/transports-integrations/tests/integrations/test_openai.py
397-397: Missing return type annotation for public function test_12_error_handling_invalid_roles
Add return type annotation: None
(ANN201)
397-397: Missing type annotation for function argument openai_client
(ANN001)
397-397: Missing type annotation for function argument test_config
(ANN001)
397-397: Unused method argument: test_config
(ARG002)
399-399: pytest.raises(Exception) is too broad, set the match parameter or use a more specific exception
(PT011)
tests/transports-integrations/tests/integrations/test_anthropic.py
41-41: ..utils.common.ALL_TOOLS imported but unused
Remove unused import: ..utils.common.ALL_TOOLS
(F401)
552-552: Missing return type annotation for public function test_12_error_handling_invalid_roles
Add return type annotation: None
(ANN201)
552-552: Missing type annotation for function argument anthropic_client
(ANN001)
552-552: Missing type annotation for function argument test_config
(ANN001)
552-552: Unused method argument: test_config
(ARG002)
554-554: pytest.raises(Exception) is too broad, set the match parameter or use a more specific exception
(PT011)
tests/transports-integrations/tests/integrations/test_litellm.py
49-49: ..utils.common.get_api_key imported but unused
Remove unused import
(F401)
50-50: ..utils.common.skip_if_no_api_key imported but unused
Remove unused import
(F401)
347-347: Missing return type annotation for public function test_12_error_handling_invalid_roles
Add return type annotation: None
(ANN201)
347-347: Missing type annotation for function argument test_config
(ANN001)
347-347: Unused method argument: test_config
(ARG002)
349-349: pytest.raises(Exception) is too broad, set the match parameter or use a more specific exception
(PT011)
tests/transports-integrations/tests/integrations/test_google.py
34-34: ..utils.common.INVALID_ROLE_MESSAGES imported but unused
Remove unused import: ..utils.common.INVALID_ROLE_MESSAGES
(F401)
430-430: Missing return type annotation for public function test_12_error_handling_invalid_roles
Add return type annotation: None
(ANN201)
430-430: Missing type annotation for function argument google_client
(ANN001)
430-430: Missing type annotation for function argument test_config
(ANN001)
430-430: Unused method argument: test_config
(ARG002)
432-432: pytest.raises(Exception) is too broad, set the match parameter or use a more specific exception
(PT011)
434-434: Trailing comma missing
Add trailing comma
(COM812)
tests/transports-integrations/tests/utils/common.py
195-195: Trailing comma missing
Add trailing comma
(COM812)
203-203: Trailing comma missing
Add trailing comma
(COM812)
205-205: Trailing comma missing
Add trailing comma
(COM812)
464-464: Missing return type annotation for public function assert_valid_error_response
Add return type annotation: bool
(ANN201)
465-465: Dynamically typed expressions (typing.Any) are disallowed in response_or_exception
(ANN401)
465-465: Trailing comma missing
Add trailing comma
(COM812)
490-490: Trailing comma missing
Add trailing comma
(COM812)
499-499: Do not use bare except
(E722)
550-550: Missing return type annotation for public function assert_error_propagation
Add return type annotation: bool
(ANN201)
550-550: Dynamically typed expressions (typing.Any) are disallowed in error_response
(ANN401)
574-574: Trailing comma missing
Add trailing comma
(COM812)
577-577: Trailing comma missing
Add trailing comma
(COM812)
598-598: Trailing comma missing
Add trailing comma
(COM812)
608-610: Use a single if statement instead of nested if statements
(SIM102)
620-620: Trailing comma missing
Add trailing comma
(COM812)
626-626: Trailing comma missing
Add trailing comma
(COM812)
632-632: Trailing comma missing
Add trailing comma
(COM812)
🔇 Additional comments (28)
core/schemas/bifrost.go (1)
434-434: LGTM! Well-designed provider tracking for error handling.The addition of the
Providerfield enables accurate error attribution and supports the provider-specific error conversion implemented in the router layers. Thejson:"-"tag appropriately excludes this internal metadata from client responses.tests/transports-integrations/README.md (3)
37-37: LGTM! Documentation accurately reflects the enhanced test coverage.The update from 11 to 12 test categories correctly incorporates the new error handling tests.
50-50: LGTM! Clear description of the new error handling test category.The addition properly describes the scope and purpose of the error handling tests.
184-186: LGTM! Helpful examples for running error handling tests.The pytest examples provide clear guidance for running the new error handling test category specifically.
transports/bifrost-http/integrations/openai/router.go (1)
35-37: LGTM! Proper integration of provider-specific error conversion.The ErrorConverter implementation correctly transforms BifrostError instances into OpenAI-specific error formats, enabling accurate error responses for OpenAI integration clients.
transports/bifrost-http/integrations/genai/router.go (1)
37-39: LGTM! Consistent error conversion implementation for Google GenAI.The ErrorConverter properly transforms BifrostError instances into Gemini-specific error formats, maintaining consistency with the error handling pattern across all integrations.
transports/bifrost-http/integrations/anthropic/router.go (1)
35-37: LGTM! Proper Anthropic-specific error conversion implementation.The ErrorConverter correctly transforms BifrostError instances into Anthropic-specific error formats, completing the consistent error handling enhancement across all supported integrations.
tests/transports-integrations/tests/integrations/test_google.py (1)
429-441: LGTM! Consistent error handling test implementation.The test correctly validates error handling for invalid roles using Google GenAI's specific content format and follows the established pattern used across other integrations.
tests/transports-integrations/tests/integrations/test_openai.py (1)
396-409: LGTM! Consistent error handling test across integrations.The test correctly validates error handling for invalid roles and follows the established pattern. The error response validation and propagation checks ensure proper error handling through the Bifrost transport layer.
tests/transports-integrations/tests/integrations/test_litellm.py (1)
347-359: LGTM! Consistent error handling test implementation.The test correctly validates error handling for invalid roles and follows the established pattern across all integrations. The error validation ensures proper propagation through the Bifrost transport layer.
core/bifrost.go (3)
195-196: LGTM: Provider field assignment in TextCompletionRequest.The error provider assignment is correctly implemented for all error paths:
- Validation errors use
req.Provider- Cancelled request errors use
req.Provider- Short-circuit errors use
req.Provider- Fallback cancellation errors use
fallback.Provider- Final primary errors use
req.ProviderThis ensures proper error provenance tracking.
Also applies to: 206-207, 213-214, 240-241, 248-249
259-260: LGTM: Provider field assignment in ChatCompletionRequest.The error provider assignment follows the same correct pattern as TextCompletionRequest:
- All primary provider errors are assigned
req.Provider- Fallback provider errors are assigned
fallback.Provider- Cancelled requests are properly handled with appropriate provider assignment
The implementation is consistent and complete.
Also applies to: 269-272, 277-278, 304-305, 312-313
323-324: LGTM: Provider field assignment in EmbeddingRequest.The error provider assignment maintains consistency with the other request methods:
- Validation errors correctly use
req.Provider- Cancelled request handling is properly implemented
- Short-circuit and fallback error handling follows the established pattern
All error paths now include provider information for proper error tracking.
Also applies to: 337-340, 345-346, 371-372, 379-380
transports/bifrost-http/integrations/litellm/router.go (2)
143-154: LGTM: Provider-specific error converter implementation.The error converter correctly:
- Switches on the
err.Providerfield to determine the appropriate conversion- Calls the correct provider-specific error conversion functions for OpenAI/Azure, Anthropic, and Vertex
- Falls back to returning the original error for unknown providers
- Handles nil errors appropriately (the called functions have nil checks)
This implementation aligns with the broader error handling improvements across all integrations.
164-164: LGTM: Error converter integration in route configuration.The
ErrorConverterfield assignment correctly integrates the new error conversion functionality into the routing configuration, ensuring that all error responses from LiteLLM routes will be properly formatted according to the originating provider's error format.transports/bifrost-http/integrations/genai/types.go (2)
139-149: LGTM: Well-structured error types following Google API format.The error type definitions correctly model the Gemini/Google GenAI error response structure with appropriate JSON tags and field types.
582-606: LGTM: Robust error converter with proper nil handling.The error converter function demonstrates excellent defensive programming:
- Handles nil input gracefully
- Provides sensible defaults (HTTP 500, empty status)
- Safely dereferences all pointer fields with nil checks
- Follows consistent patterns with other integration converters
This implementation aligns well with the team's preference for consistent error handling patterns.
transports/bifrost-http/integrations/anthropic/types.go (2)
96-106: LGTM: Clean error type definitions matching Anthropic API structure.The error types correctly model Anthropic's messages API error response format with appropriate JSON tags and field organization.
480-506: LGTM: Safe and consistent error converter implementation.The error converter function demonstrates solid defensive programming practices:
- Proper nil input handling
- Safe pointer dereferencing with nil checks for both top-level and nested error fields
- Consistent initialization with empty strings for missing values
- Follows the same reliable pattern established across all integration converters
This implementation maintains the consistent error handling patterns preferred by the team.
transports/bifrost-http/integrations/openai/types.go (2)
42-62: LGTM: Comprehensive error types matching OpenAI API structure.The error type definitions correctly model OpenAI's chat completion error response format with:
- Proper JSON tags for all fields
- Comprehensive error fields including event tracking
- Clean structure using both named and anonymous structs
156-199: LGTM: Thorough error converter with comprehensive field mapping.The error converter function demonstrates excellent implementation quality:
- Proper nil input validation
- Safe dereferencing of all pointer fields (EventID, Type, nested Error fields)
- Comprehensive mapping of all error fields from BifrostError to OpenAI format
- Consistent error handling patterns with empty string defaults
- Follows the established pattern across all integration converters
This implementation aligns perfectly with the team's consistent error handling approach.
transports/bifrost-http/integrations/utils.go (6)
30-32: LGTM: Clean abstraction for error conversion.The
ErrorConvertertype provides a clean, simple interface for converting BifrostError instances to integration-specific error formats.
52-52: LGTM: Consistent integration of ErrorConverter into route configuration.Adding
ErrorConverteras a required field toRouteConfigensures all routes have provider-specific error handling capabilities.
91-94: LGTM: Proper validation following established patterns.The validation check for
ErrorConverterbeing non-nil follows the same pattern as other required route configuration fields, ensuring fail-fast behavior during startup.
139-139: LGTM: Comprehensive and consistent error handling updates.All error handling calls have been systematically updated to include the
ErrorConverterparameter, ensuring consistent provider-specific error formatting throughout the request handling flow.Also applies to: 150-150, 158-158, 162-162, 166-166, 174-174, 181-181, 187-187, 194-194, 197-197
203-219: LGTM: Well-designed centralized error response handling.The updated
sendErrormethod effectively centralizes error conversion and response formatting:
- Uses the provided
ErrorConverterto format errors appropriately for each integration- Maintains proper HTTP status code handling from
BifrostError.StatusCode- Includes appropriate fallback error handling for JSON marshaling failures
- Clean separation of concerns between error conversion and HTTP response generation
222-222: LGTM: Consistent error converter integration in success path.The
sendSuccessmethod properly passes theErrorConvertertosendErrorfor consistent error handling even in the success path when JSON marshaling fails.Also applies to: 228-228
tests/transports-integrations/tests/utils/common.py (1)
653-653: Good addition for error handling test categorization.The new
ERROR_HANDLINGtest category aligns well with the PR objectives of adding comprehensive error handling tests across integrations.
| MULTIPLE_TOOL_CALL_MESSAGES, | ||
| IMAGE_URL, | ||
| BASE64_IMAGE, | ||
| INVALID_ROLE_MESSAGES, |
Contributor
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Remove unused import.
The INVALID_ROLE_MESSAGES import is not used in this test file since Google GenAI uses GENAI_INVALID_ROLE_CONTENT instead.
- INVALID_ROLE_MESSAGES,📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| INVALID_ROLE_MESSAGES, | |
| from ..utils.common import ( | |
| GENAI_INVALID_ROLE_CONTENT, | |
| ) |
🧰 Tools
🪛 Ruff (0.11.9)
34-34: ..utils.common.INVALID_ROLE_MESSAGES imported but unused
Remove unused import: ..utils.common.INVALID_ROLE_MESSAGES
(F401)
🤖 Prompt for AI Agents
In tests/transports-integrations/tests/integrations/test_google.py at line 34,
remove the import of INVALID_ROLE_MESSAGES since it is not used in this file;
the tests use GENAI_INVALID_ROLE_CONTENT instead. Simply delete the line
importing INVALID_ROLE_MESSAGES to clean up unused imports.
Comment on lines
+49
to
+50
| get_api_key, | ||
| skip_if_no_api_key, |
Contributor
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Remove unused imports.
The get_api_key and skip_if_no_api_key imports are not used in this test file. Based on the established pattern for LiteLLM tests, these should be removed.
- get_api_key,
- skip_if_no_api_key,📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| get_api_key, | |
| skip_if_no_api_key, | |
| - get_api_key, | |
| - skip_if_no_api_key, |
🧰 Tools
🪛 Ruff (0.11.9)
49-49: ..utils.common.get_api_key imported but unused
Remove unused import
(F401)
50-50: ..utils.common.skip_if_no_api_key imported but unused
Remove unused import
(F401)
🤖 Prompt for AI Agents
In tests/transports-integrations/tests/integrations/test_litellm.py around lines
49 to 50, the imports get_api_key and skip_if_no_api_key are not used anywhere
in the file. Remove these two imports to clean up the code and follow the
established pattern for LiteLLM tests.
Comment on lines
+38
to
+41
| INVALID_ROLE_MESSAGES, | ||
| WEATHER_TOOL, | ||
| CALCULATOR_TOOL, | ||
| ALL_TOOLS, |
Contributor
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Remove unused import.
The ALL_TOOLS import is not used in this test file.
- ALL_TOOLS,📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| INVALID_ROLE_MESSAGES, | |
| WEATHER_TOOL, | |
| CALCULATOR_TOOL, | |
| ALL_TOOLS, | |
| INVALID_ROLE_MESSAGES, | |
| WEATHER_TOOL, | |
| CALCULATOR_TOOL, |
🧰 Tools
🪛 Ruff (0.11.9)
41-41: ..utils.common.ALL_TOOLS imported but unused
Remove unused import: ..utils.common.ALL_TOOLS
(F401)
🤖 Prompt for AI Agents
In tests/transports-integrations/tests/integrations/test_anthropic.py around
lines 38 to 41, the import ALL_TOOLS is not used anywhere in the file. Remove
the ALL_TOOLS import from the import statement to clean up unused code.
Comment on lines
+551
to
+565
| @skip_if_no_api_key("anthropic") | ||
| def test_12_error_handling_invalid_roles(self, anthropic_client, test_config): | ||
| """Test Case 12: Error handling for invalid roles""" | ||
| with pytest.raises(Exception) as exc_info: | ||
| anthropic_client.messages.create( | ||
| model=get_model("anthropic", "chat"), | ||
| messages=INVALID_ROLE_MESSAGES, | ||
| max_tokens=100, | ||
| ) | ||
|
|
||
| # Verify the error is properly caught and contains role-related information | ||
| error = exc_info.value | ||
| assert_valid_error_response(error, "tester") | ||
| assert_error_propagation(error, "anthropic") | ||
|
|
Contributor
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Improve error handling test specificity.
The test implementation follows the correct pattern but could be improved:
- The exception type is too broad - consider using a more specific exception type
- Add proper type annotations for better code quality
- def test_12_error_handling_invalid_roles(self, anthropic_client, test_config):
+ def test_12_error_handling_invalid_roles(self, anthropic_client, test_config) -> None:
"""Test Case 12: Error handling for invalid roles"""
- with pytest.raises(Exception) as exc_info:
+ with pytest.raises(Exception, match=r".*(role|invalid|tester).*") as exc_info:The test correctly validates error propagation and follows the established pattern across other integration tests.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| @skip_if_no_api_key("anthropic") | |
| def test_12_error_handling_invalid_roles(self, anthropic_client, test_config): | |
| """Test Case 12: Error handling for invalid roles""" | |
| with pytest.raises(Exception) as exc_info: | |
| anthropic_client.messages.create( | |
| model=get_model("anthropic", "chat"), | |
| messages=INVALID_ROLE_MESSAGES, | |
| max_tokens=100, | |
| ) | |
| # Verify the error is properly caught and contains role-related information | |
| error = exc_info.value | |
| assert_valid_error_response(error, "tester") | |
| assert_error_propagation(error, "anthropic") | |
| @skip_if_no_api_key("anthropic") | |
| def test_12_error_handling_invalid_roles(self, anthropic_client, test_config) -> None: | |
| """Test Case 12: Error handling for invalid roles""" | |
| with pytest.raises(Exception, match=r".*(role|invalid|tester).*") as exc_info: | |
| anthropic_client.messages.create( | |
| model=get_model("anthropic", "chat"), | |
| messages=INVALID_ROLE_MESSAGES, | |
| max_tokens=100, | |
| ) | |
| # Verify the error is properly caught and contains role-related information | |
| error = exc_info.value | |
| assert_valid_error_response(error, "tester") | |
| assert_error_propagation(error, "anthropic") |
🧰 Tools
🪛 Ruff (0.11.9)
552-552: Missing return type annotation for public function test_12_error_handling_invalid_roles
Add return type annotation: None
(ANN201)
552-552: Missing type annotation for function argument anthropic_client
(ANN001)
552-552: Missing type annotation for function argument test_config
(ANN001)
552-552: Unused method argument: test_config
(ARG002)
554-554: pytest.raises(Exception) is too broad, set the match parameter or use a more specific exception
(PT011)
🤖 Prompt for AI Agents
In tests/transports-integrations/tests/integrations/test_anthropic.py around
lines 551 to 565, the test_12_error_handling_invalid_roles function uses a broad
Exception type in pytest.raises and lacks type annotations. Update the test to
catch a more specific exception type relevant to invalid roles, such as a custom
error or a more precise built-in exception. Additionally, add appropriate type
annotations to the test method parameters and return type to improve code
clarity and quality.
Comment on lines
+464
to
+547
| def assert_valid_error_response( | ||
| response_or_exception: Any, expected_invalid_role: str = "tester" | ||
| ): | ||
| """ | ||
| Assert that an error response or exception properly indicates an invalid role error. | ||
|
|
||
| WEATHER_KEYWORDS = [ | ||
| "weather", | ||
| "temperature", | ||
| "sunny", | ||
| "cloudy", | ||
| "rain", | ||
| "snow", | ||
| "celsius", | ||
| "fahrenheit", | ||
| "degrees", | ||
| "hot", | ||
| "cold", | ||
| "warm", | ||
| "cool", | ||
| ] | ||
| Args: | ||
| response_or_exception: Either an HTTP error response or a raised exception | ||
| expected_invalid_role: The invalid role that should be mentioned in the error | ||
| """ | ||
| error_message = "" | ||
| error_type = "" | ||
| status_code = None | ||
|
|
||
| LOCATION_KEYWORDS = ["boston", "san francisco", "new york", "city", "location", "place"] | ||
| # Handle different error response formats | ||
| if hasattr(response_or_exception, "response"): | ||
| # This is likely a requests.HTTPError or similar | ||
| try: | ||
| error_data = response_or_exception.response.json() | ||
| status_code = response_or_exception.response.status_code | ||
|
|
||
| # Extract error message from various formats | ||
| if isinstance(error_data, dict): | ||
| if "error" in error_data: | ||
| if isinstance(error_data["error"], dict): | ||
| error_message = error_data["error"].get( | ||
| "message", str(error_data["error"]) | ||
| ) | ||
| error_type = error_data["error"].get("type", "") | ||
| else: | ||
| error_message = str(error_data["error"]) | ||
| else: | ||
| error_message = error_data.get("message", str(error_data)) | ||
| else: | ||
| error_message = str(error_data) | ||
| except: | ||
| error_message = str(response_or_exception) | ||
|
|
||
| elif hasattr(response_or_exception, "message"): | ||
| # Direct error object | ||
| error_message = response_or_exception.message | ||
|
|
||
| elif hasattr(response_or_exception, "args") and response_or_exception.args: | ||
| # Exception with args | ||
| error_message = str(response_or_exception.args[0]) | ||
|
|
||
| else: | ||
| # Fallback to string representation | ||
| error_message = str(response_or_exception) | ||
|
|
||
| # Convert to lowercase for case-insensitive matching | ||
| error_message_lower = error_message.lower() | ||
| error_type_lower = error_type.lower() | ||
|
|
||
| # Validate that error message indicates role-related issue | ||
| role_error_indicators = [ | ||
| expected_invalid_role.lower(), | ||
| "role", | ||
| "invalid", | ||
| "unsupported", | ||
| "unknown", | ||
| "not allowed", | ||
| "not supported", | ||
| "bad request", | ||
| "invalid_request", | ||
| ] | ||
|
|
||
| has_role_error = any( | ||
| indicator in error_message_lower or indicator in error_type_lower | ||
| for indicator in role_error_indicators | ||
| ) | ||
|
|
||
| assert has_role_error, ( | ||
| f"Error message should indicate invalid role '{expected_invalid_role}'. " | ||
| f"Got error message: '{error_message}', error type: '{error_type}'" | ||
| ) | ||
|
|
||
| # Validate status code if available (should be 4xx for client errors) | ||
| if status_code is not None: | ||
| assert ( | ||
| 400 <= status_code < 500 | ||
| ), f"Expected 4xx status code for invalid role error, got {status_code}" | ||
|
|
||
| return True |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve error handling and add type annotations.
The function logic is solid for validating error responses across different formats. Address these style and safety issues:
Apply this diff to improve the function:
-def assert_valid_error_response(
- response_or_exception: Any, expected_invalid_role: str = "tester"
-):
+def assert_valid_error_response(
+ response_or_exception: Any, expected_invalid_role: str = "tester",
+) -> bool:- error_message = error_data["error"].get(
- "message", str(error_data["error"])
- )
+ error_message = error_data["error"].get(
+ "message", str(error_data["error"])
+ ),- except:
+ except (ValueError, KeyError, AttributeError):
error_message = str(response_or_exception)The bare except clause should be replaced with specific exception types to avoid masking unexpected errors during development and debugging.
Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 Ruff (0.11.9)
464-464: Missing return type annotation for public function assert_valid_error_response
Add return type annotation: bool
(ANN201)
465-465: Dynamically typed expressions (typing.Any) are disallowed in response_or_exception
(ANN401)
465-465: Trailing comma missing
Add trailing comma
(COM812)
490-490: Trailing comma missing
Add trailing comma
(COM812)
499-499: Do not use bare except
(E722)
🤖 Prompt for AI Agents
In tests/transports-integrations/tests/utils/common.py around lines 464 to 547,
the function assert_valid_error_response uses a bare except clause which can
mask unexpected errors. Replace the bare except with specific exceptions such as
JSONDecodeError and AttributeError when attempting to parse the JSON response to
improve error handling and avoid hiding bugs. Additionally, add appropriate type
annotations to the function signature and variables to enhance code clarity and
safety.
Comment on lines
+154
to
+221
| # Common keyword arrays for flexible assertions | ||
| COMPARISON_KEYWORDS = [ | ||
| "compare", | ||
| "comparison", | ||
| "different", | ||
| "difference", | ||
| "differences", | ||
| "both", | ||
| "two", | ||
| "first", | ||
| "second", | ||
| "images", | ||
| "image", | ||
| "versus", | ||
| "vs", | ||
| "contrast", | ||
| "unlike", | ||
| "while", | ||
| "whereas", | ||
| ] | ||
|
|
||
| WEATHER_KEYWORDS = [ | ||
| "weather", | ||
| "temperature", | ||
| "sunny", | ||
| "cloudy", | ||
| "rain", | ||
| "snow", | ||
| "celsius", | ||
| "fahrenheit", | ||
| "degrees", | ||
| "hot", | ||
| "cold", | ||
| "warm", | ||
| "cool", | ||
| ] | ||
|
|
||
| LOCATION_KEYWORDS = ["boston", "san francisco", "new york", "city", "location", "place"] | ||
|
|
||
| # Error test data for invalid role testing | ||
| INVALID_ROLE_MESSAGES = [ | ||
| {"role": "tester", "content": "Hello! This should fail due to invalid role."} | ||
| ] | ||
|
|
||
| # GenAI-specific invalid role content that passes SDK validation but fails at Bifrost | ||
| GENAI_INVALID_ROLE_CONTENT = [ | ||
| { | ||
| "role": "tester", # Invalid role that should be caught by Bifrost | ||
| "parts": [ | ||
| {"text": "Hello! This should fail due to invalid role in GenAI format."} | ||
| ], | ||
| } | ||
| ] | ||
|
|
||
| # Error keywords for validating error messages | ||
| ERROR_KEYWORDS = [ | ||
| "invalid", | ||
| "error", | ||
| "role", | ||
| "tester", | ||
| "unsupported", | ||
| "unknown", | ||
| "bad", | ||
| "incorrect", | ||
| "not allowed", | ||
| "not supported", | ||
| "forbidden", | ||
| ] |
Contributor
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Add trailing commas for consistency.
The keyword arrays and test data look comprehensive and well-organized. Consider adding trailing commas to list elements for consistency with Python style guidelines.
Apply this diff to add missing trailing commas:
- {"role": "tester", "content": "Hello! This should fail due to invalid role."}
+ {"role": "tester", "content": "Hello! This should fail due to invalid role."},- "role": "tester", # Invalid role that should be caught by Bifrost
+ "role": "tester", # Invalid role that should be caught by Bifrost
"parts": [
{"text": "Hello! This should fail due to invalid role in GenAI format."}
- ],
+ ],- {"text": "Hello! This should fail due to invalid role in GenAI format."}
+ {"text": "Hello! This should fail due to invalid role in GenAI format."},📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| # Common keyword arrays for flexible assertions | |
| COMPARISON_KEYWORDS = [ | |
| "compare", | |
| "comparison", | |
| "different", | |
| "difference", | |
| "differences", | |
| "both", | |
| "two", | |
| "first", | |
| "second", | |
| "images", | |
| "image", | |
| "versus", | |
| "vs", | |
| "contrast", | |
| "unlike", | |
| "while", | |
| "whereas", | |
| ] | |
| WEATHER_KEYWORDS = [ | |
| "weather", | |
| "temperature", | |
| "sunny", | |
| "cloudy", | |
| "rain", | |
| "snow", | |
| "celsius", | |
| "fahrenheit", | |
| "degrees", | |
| "hot", | |
| "cold", | |
| "warm", | |
| "cool", | |
| ] | |
| LOCATION_KEYWORDS = ["boston", "san francisco", "new york", "city", "location", "place"] | |
| # Error test data for invalid role testing | |
| INVALID_ROLE_MESSAGES = [ | |
| {"role": "tester", "content": "Hello! This should fail due to invalid role."} | |
| ] | |
| # GenAI-specific invalid role content that passes SDK validation but fails at Bifrost | |
| GENAI_INVALID_ROLE_CONTENT = [ | |
| { | |
| "role": "tester", # Invalid role that should be caught by Bifrost | |
| "parts": [ | |
| {"text": "Hello! This should fail due to invalid role in GenAI format."} | |
| ], | |
| } | |
| ] | |
| # Error keywords for validating error messages | |
| ERROR_KEYWORDS = [ | |
| "invalid", | |
| "error", | |
| "role", | |
| "tester", | |
| "unsupported", | |
| "unknown", | |
| "bad", | |
| "incorrect", | |
| "not allowed", | |
| "not supported", | |
| "forbidden", | |
| ] | |
| # Error test data for invalid role testing | |
| INVALID_ROLE_MESSAGES = [ | |
| {"role": "tester", "content": "Hello! This should fail due to invalid role."}, | |
| ] | |
| # GenAI-specific invalid role content that passes SDK validation but fails at Bifrost | |
| GENAI_INVALID_ROLE_CONTENT = [ | |
| { | |
| "role": "tester", # Invalid role that should be caught by Bifrost | |
| "parts": [ | |
| {"text": "Hello! This should fail due to invalid role in GenAI format."}, | |
| ], | |
| }, | |
| ] |
🧰 Tools
🪛 Ruff (0.11.9)
195-195: Trailing comma missing
Add trailing comma
(COM812)
203-203: Trailing comma missing
Add trailing comma
(COM812)
205-205: Trailing comma missing
Add trailing comma
(COM812)
🤖 Prompt for AI Agents
In tests/transports-integrations/tests/utils/common.py between lines 154 and
221, the lists defining keyword arrays and test data lack trailing commas on the
last elements. To fix this, add trailing commas after the last item in each list
and dictionary to maintain consistency with Python style guidelines and improve
diff clarity.
Comment on lines
+550
to
+635
| def assert_error_propagation(error_response: Any, integration: str): | ||
| """ | ||
| Assert that error is properly propagated through Bifrost to the integration. | ||
|
|
||
| Args: | ||
| error_response: The error response from the integration | ||
| integration: The integration name (openai, anthropic, etc.) | ||
| """ | ||
| # Check that we got an error response (not a success) | ||
| assert error_response is not None, "Should have received an error response" | ||
|
|
||
| # Integration-specific error format validation | ||
| if integration.lower() == "openai": | ||
| # OpenAI format: should have top-level 'type', 'event_id' and 'error' field with nested structure | ||
| if hasattr(error_response, "response"): | ||
| error_data = error_response.response.json() | ||
| assert "error" in error_data, "OpenAI error should have 'error' field" | ||
| assert ( | ||
| "type" in error_data | ||
| ), "OpenAI error should have top-level 'type' field" | ||
| assert ( | ||
| "event_id" in error_data | ||
| ), "OpenAI error should have top-level 'event_id' field" | ||
| assert isinstance( | ||
| error_data["type"], str | ||
| ), "OpenAI error type should be a string" | ||
| assert isinstance( | ||
| error_data["event_id"], str | ||
| ), "OpenAI error event_id should be a string" | ||
|
|
||
| # Check nested error structure | ||
| error_obj = error_data["error"] | ||
| assert ( | ||
| "message" in error_obj | ||
| ), "OpenAI error.error should have 'message' field" | ||
| assert "type" in error_obj, "OpenAI error.error should have 'type' field" | ||
| assert "code" in error_obj, "OpenAI error.error should have 'code' field" | ||
| assert ( | ||
| "event_id" in error_obj | ||
| ), "OpenAI error.error should have 'event_id' field" | ||
|
|
||
| elif integration.lower() == "anthropic": | ||
| # Anthropic format: should have 'type' and 'error' with 'type' and 'message' | ||
| if hasattr(error_response, "response"): | ||
| error_data = error_response.response.json() | ||
| assert "type" in error_data, "Anthropic error should have 'type' field" | ||
| # Type field can be empty string if not set in original error | ||
| assert isinstance( | ||
| error_data["type"], str | ||
| ), "Anthropic error type should be a string" | ||
| assert "error" in error_data, "Anthropic error should have 'error' field" | ||
| assert ( | ||
| "type" in error_data["error"] | ||
| ), "Anthropic error.error should have 'type' field" | ||
| assert ( | ||
| "message" in error_data["error"] | ||
| ), "Anthropic error.error should have 'message' field" | ||
|
|
||
| elif integration.lower() in ["google", "gemini", "genai"]: | ||
| # Gemini format: follows Google API design guidelines with error.code, error.message, error.status | ||
| if hasattr(error_response, "response"): | ||
| error_data = error_response.response.json() | ||
| assert "error" in error_data, "Gemini error should have 'error' field" | ||
|
|
||
| # Check Google API standard error structure | ||
| error_obj = error_data["error"] | ||
| assert ( | ||
| "code" in error_obj | ||
| ), "Gemini error.error should have 'code' field (HTTP status code)" | ||
| assert isinstance( | ||
| error_obj["code"], int | ||
| ), "Gemini error.error.code should be an integer" | ||
| assert ( | ||
| "message" in error_obj | ||
| ), "Gemini error.error should have 'message' field" | ||
| assert isinstance( | ||
| error_obj["message"], str | ||
| ), "Gemini error.error.message should be a string" | ||
| assert ( | ||
| "status" in error_obj | ||
| ), "Gemini error.error should have 'status' field" | ||
| assert isinstance( | ||
| error_obj["status"], str | ||
| ), "Gemini error.error.status should be a string" | ||
|
|
||
| return True |
Contributor
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Add type annotation and fix style issues - excellent error validation logic.
The error propagation validation is comprehensive and correctly validates integration-specific error formats. The logic properly ensures errors are formatted according to each provider's API conventions.
Apply this diff to address style issues:
-def assert_error_propagation(error_response: Any, integration: str):
+def assert_error_propagation(error_response: Any, integration: str) -> bool:- assert isinstance(
- error_data["type"], str
- ), "OpenAI error type should be a string"
+ assert isinstance(
+ error_data["type"], str,
+ ), "OpenAI error type should be a string"- assert isinstance(
- error_data["event_id"], str
- ), "OpenAI error event_id should be a string"
+ assert isinstance(
+ error_data["event_id"], str,
+ ), "OpenAI error event_id should be a string"- assert isinstance(
- error_data["type"], str
- ), "Anthropic error type should be a string"
+ assert isinstance(
+ error_data["type"], str,
+ ), "Anthropic error type should be a string"- assert isinstance(
- error_obj["code"], int
- ), "Gemini error.error.code should be an integer"
+ assert isinstance(
+ error_obj["code"], int,
+ ), "Gemini error.error.code should be an integer"- assert isinstance(
- error_obj["message"], str
- ), "Gemini error.error.message should be a string"
+ assert isinstance(
+ error_obj["message"], str,
+ ), "Gemini error.error.message should be a string"- assert isinstance(
- error_obj["status"], str
- ), "Gemini error.error.status should be a string"
+ assert isinstance(
+ error_obj["status"], str,
+ ), "Gemini error.error.status should be a string"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| def assert_error_propagation(error_response: Any, integration: str): | |
| """ | |
| Assert that error is properly propagated through Bifrost to the integration. | |
| Args: | |
| error_response: The error response from the integration | |
| integration: The integration name (openai, anthropic, etc.) | |
| """ | |
| # Check that we got an error response (not a success) | |
| assert error_response is not None, "Should have received an error response" | |
| # Integration-specific error format validation | |
| if integration.lower() == "openai": | |
| # OpenAI format: should have top-level 'type', 'event_id' and 'error' field with nested structure | |
| if hasattr(error_response, "response"): | |
| error_data = error_response.response.json() | |
| assert "error" in error_data, "OpenAI error should have 'error' field" | |
| assert ( | |
| "type" in error_data | |
| ), "OpenAI error should have top-level 'type' field" | |
| assert ( | |
| "event_id" in error_data | |
| ), "OpenAI error should have top-level 'event_id' field" | |
| assert isinstance( | |
| error_data["type"], str | |
| ), "OpenAI error type should be a string" | |
| assert isinstance( | |
| error_data["event_id"], str | |
| ), "OpenAI error event_id should be a string" | |
| # Check nested error structure | |
| error_obj = error_data["error"] | |
| assert ( | |
| "message" in error_obj | |
| ), "OpenAI error.error should have 'message' field" | |
| assert "type" in error_obj, "OpenAI error.error should have 'type' field" | |
| assert "code" in error_obj, "OpenAI error.error should have 'code' field" | |
| assert ( | |
| "event_id" in error_obj | |
| ), "OpenAI error.error should have 'event_id' field" | |
| elif integration.lower() == "anthropic": | |
| # Anthropic format: should have 'type' and 'error' with 'type' and 'message' | |
| if hasattr(error_response, "response"): | |
| error_data = error_response.response.json() | |
| assert "type" in error_data, "Anthropic error should have 'type' field" | |
| # Type field can be empty string if not set in original error | |
| assert isinstance( | |
| error_data["type"], str | |
| ), "Anthropic error type should be a string" | |
| assert "error" in error_data, "Anthropic error should have 'error' field" | |
| assert ( | |
| "type" in error_data["error"] | |
| ), "Anthropic error.error should have 'type' field" | |
| assert ( | |
| "message" in error_data["error"] | |
| ), "Anthropic error.error should have 'message' field" | |
| elif integration.lower() in ["google", "gemini", "genai"]: | |
| # Gemini format: follows Google API design guidelines with error.code, error.message, error.status | |
| if hasattr(error_response, "response"): | |
| error_data = error_response.response.json() | |
| assert "error" in error_data, "Gemini error should have 'error' field" | |
| # Check Google API standard error structure | |
| error_obj = error_data["error"] | |
| assert ( | |
| "code" in error_obj | |
| ), "Gemini error.error should have 'code' field (HTTP status code)" | |
| assert isinstance( | |
| error_obj["code"], int | |
| ), "Gemini error.error.code should be an integer" | |
| assert ( | |
| "message" in error_obj | |
| ), "Gemini error.error should have 'message' field" | |
| assert isinstance( | |
| error_obj["message"], str | |
| ), "Gemini error.error.message should be a string" | |
| assert ( | |
| "status" in error_obj | |
| ), "Gemini error.error should have 'status' field" | |
| assert isinstance( | |
| error_obj["status"], str | |
| ), "Gemini error.error.status should be a string" | |
| return True | |
| def assert_error_propagation(error_response: Any, integration: str) -> bool: | |
| """ | |
| Assert that error is properly propagated through Bifrost to the integration. | |
| Args: | |
| error_response: The error response from the integration | |
| integration: The integration name (openai, anthropic, etc.) | |
| """ | |
| # Check that we got an error response (not a success) | |
| assert error_response is not None, "Should have received an error response" | |
| # Integration-specific error format validation | |
| if integration.lower() == "openai": | |
| # OpenAI format: should have top-level 'type', 'event_id' and 'error' field with nested structure | |
| if hasattr(error_response, "response"): | |
| error_data = error_response.response.json() | |
| assert "error" in error_data, "OpenAI error should have 'error' field" | |
| assert ( | |
| "type" in error_data | |
| ), "OpenAI error should have top-level 'type' field" | |
| assert ( | |
| "event_id" in error_data | |
| ), "OpenAI error should have top-level 'event_id' field" | |
| assert isinstance( | |
| error_data["type"], str, | |
| ), "OpenAI error type should be a string" | |
| assert isinstance( | |
| error_data["event_id"], str, | |
| ), "OpenAI error event_id should be a string" | |
| # Check nested error structure | |
| error_obj = error_data["error"] | |
| assert ( | |
| "message" in error_obj | |
| ), "OpenAI error.error should have 'message' field" | |
| assert "type" in error_obj, "OpenAI error.error should have 'type' field" | |
| assert "code" in error_obj, "OpenAI error.error should have 'code' field" | |
| assert ( | |
| "event_id" in error_obj | |
| ), "OpenAI error.error should have 'event_id' field" | |
| elif integration.lower() == "anthropic": | |
| # Anthropic format: should have 'type' and 'error' with 'type' and 'message' | |
| if hasattr(error_response, "response"): | |
| error_data = error_response.response.json() | |
| assert "type" in error_data, "Anthropic error should have 'type' field" | |
| # Type field can be empty string if not set in original error | |
| assert isinstance( | |
| error_data["type"], str, | |
| ), "Anthropic error type should be a string" | |
| assert "error" in error_data, "Anthropic error should have 'error' field" | |
| assert ( | |
| "type" in error_data["error"] | |
| ), "Anthropic error.error should have 'type' field" | |
| assert ( | |
| "message" in error_data["error"] | |
| ), "Anthropic error.error should have 'message' field" | |
| elif integration.lower() in ["google", "gemini", "genai"]: | |
| # Gemini format: follows Google API design guidelines with error.code, error.message, error.status | |
| if hasattr(error_response, "response"): | |
| error_data = error_response.response.json() | |
| assert "error" in error_data, "Gemini error should have 'error' field" | |
| # Check Google API standard error structure | |
| error_obj = error_data["error"] | |
| assert ( | |
| "code" in error_obj | |
| ), "Gemini error.error should have 'code' field (HTTP status code)" | |
| assert isinstance( | |
| error_obj["code"], int, | |
| ), "Gemini error.error.code should be an integer" | |
| assert ( | |
| "message" in error_obj | |
| ), "Gemini error.error should have 'message' field" | |
| assert isinstance( | |
| error_obj["message"], str, | |
| ), "Gemini error.error.message should be a string" | |
| assert ( | |
| "status" in error_obj | |
| ), "Gemini error.error should have 'status' field" | |
| assert isinstance( | |
| error_obj["status"], str, | |
| ), "Gemini error.error.status should be a string" | |
| return True |
🧰 Tools
🪛 Ruff (0.11.9)
550-550: Missing return type annotation for public function assert_error_propagation
Add return type annotation: bool
(ANN201)
550-550: Dynamically typed expressions (typing.Any) are disallowed in error_response
(ANN401)
574-574: Trailing comma missing
Add trailing comma
(COM812)
577-577: Trailing comma missing
Add trailing comma
(COM812)
598-598: Trailing comma missing
Add trailing comma
(COM812)
608-610: Use a single if statement instead of nested if statements
(SIM102)
620-620: Trailing comma missing
Add trailing comma
(COM812)
626-626: Trailing comma missing
Add trailing comma
(COM812)
632-632: Trailing comma missing
Add trailing comma
(COM812)
🤖 Prompt for AI Agents
In tests/transports-integrations/tests/utils/common.py around lines 550 to 635,
add precise type annotations to the function parameters and return type for
better clarity and type checking. Also, fix any style issues such as consistent
indentation, line length, and spacing to comply with PEP8 standards while
preserving the existing error validation logic.
akshaydeo
deleted the
07-10-feat_error_handling_added_for_transport_integrations_with_their_test_cases
branch
akshaydeo
added a commit
that referenced
this pull request
Nov 17, 2025
# Add Provider Information to BifrostError for Improved Error Propagation This PR enhances error handling in Bifrost by adding provider information to `BifrostError` objects and ensuring proper error propagation through the system. The changes enable more accurate error reporting when requests fail, making it easier to identify which provider encountered an issue. Key improvements: - Added a `Provider` field to the `BifrostError` struct to track which provider generated the error - Set the provider field at all error creation points in the request flow - Implemented provider-specific error converters for each integration (OpenAI, Anthropic, Google, LiteLLM) - Added proper error propagation in the HTTP transport layer - Enhanced error handling for cancelled requests - Added comprehensive error handling tests for all integrations These changes ensure that when a request fails, the error response will accurately reflect which provider encountered the issue, with the error formatted according to that provider's API conventions. This is particularly important for applications that need to handle errors differently based on the provider that generated them. The PR also adds a new test category for error handling, verifying that invalid role errors are properly detected and propagated through the system.
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
2 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.
Add Provider Information to BifrostError for Improved Error Propagation
This PR enhances error handling in Bifrost by adding provider information to
BifrostErrorobjects and ensuring proper error propagation through the system. The changes enable more accurate error reporting when requests fail, making it easier to identify which provider encountered an issue.Key improvements:
Providerfield to theBifrostErrorstruct to track which provider generated the errorThese changes ensure that when a request fails, the error response will accurately reflect which provider encountered the issue, with the error formatted according to that provider's API conventions. This is particularly important for applications that need to handle errors differently based on the provider that generated them.
The PR also adds a new test category for error handling, verifying that invalid role errors are properly detected and propagated through the system.