Skip to content

feat: refactored integrations to support core/v1.1.2#75

Merged
akshaydeo merged 1 commit intomainfrom
06-12-feat_http_transport_integrations_refactored_for_core_v1.1.0
Jun 14, 2025
Merged

feat: refactored integrations to support core/v1.1.2#75
akshaydeo merged 1 commit intomainfrom
06-12-feat_http_transport_integrations_refactored_for_core_v1.1.0

Conversation

@Pratham-Mishra04
Copy link
Copy Markdown
Collaborator

Anthropic API Integration Improvements

This PR enhances the Anthropic API integration by implementing a more flexible content handling system that properly supports both string and structured content formats. Key changes include:

  • Renamed AnthropicContent to AnthropicContentBlock to better reflect its purpose
  • Added a new AnthropicContent type that can handle both string content and arrays of content blocks
  • Added proper support for system content blocks in Anthropic requests
  • Implemented custom JSON marshaling/unmarshaling for content types to handle different formats
  • Added response types for Anthropic API including usage information
  • Enhanced tool schema handling with proper typing for input schemas
  • Updated the content conversion logic between Bifrost and Anthropic formats

These changes align with Anthropic's API structure and improve the handling of different content formats (text, images, tool calls) in a more type-safe manner.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Jun 11, 2025

Summary by CodeRabbit

  • New Features

    • Enhanced support for structured and flexible message content, including improved handling of text, images, and tool calls across Anthropic, Gemini, and OpenAI integrations.
    • Added support for more detailed usage information and service tier reporting in responses.
  • Improvements

    • Simplified and unified message conversion logic, resulting in more consistent and robust message formatting.
    • Improved type safety and schema clarity for tool definitions and image handling.
    • Prometheus plugin now provides additional lifecycle methods and improved logging.
  • Chores

    • Updated dependencies to newer versions for core and plugin modules.

Summary by CodeRabbit

  • New Features

    • Enhanced support for structured and flexible message content, including improved handling of text, images, tool calls, and tool results across Anthropic and Gemini integrations.
    • Unified and clearer representation of system and user/assistant message content for better consistency.
    • Added usage information to Anthropic message responses.
  • Refactor

    • Simplified and streamlined OpenAI integration by adopting a unified message and response format, removing legacy compatibility layers.
    • Improved type safety and clarity in tool schema representation and message conversion logic.
  • Chores

    • Updated a core dependency to the latest version.
    • Added a cleanup method and adjusted method signatures for plugin tracking components.

Walkthrough

This set of changes refactors the message content handling across Anthropic, Gemini, and OpenAI integrations to use a unified, block-based content model. It introduces new types and custom serialization logic for Anthropic, consolidates Gemini message content into structured blocks, removes OpenAI-specific message types in favor of direct Bifrost schema usage, and updates the Prometheus plugin interface.

Changes

File(s) Change Summary
transports/bifrost-http/integrations/anthropic/types.go Refactored Anthropic content handling: introduced AnthropicContent and renamed AnthropicContentBlock, added custom JSON marshal/unmarshal logic, updated message and tool types, unified system and user message content, rewrote conversion logic, improved type safety, and added new response and usage structs.
transports/bifrost-http/integrations/genai/types.go Changed Gemini content conversion to produce a single consolidated Bifrost message with structured content blocks for text, images, and tool calls; updated deserialization logic; removed the old multi-message conversion method; added helper to detect image MIME types.
transports/bifrost-http/integrations/openai/types.go Removed all OpenAI-specific message and content types and conversion logic; switched to direct use of Bifrost schema types for messages and responses; simplified request and response conversion logic; removed legacy fields and helper methods.
transports/bifrost-http/tracking/plugin.go Updated PrometheusPlugin.PreHook signature to return an additional response value; added a no-op Cleanup method; updated PostHook signature and logic for error handling.
transports/go.mod Updated dependencies: github.com/maximhq/bifrost/core from v1.0.10 to v1.1.2 and github.com/maximhq/bifrost/plugins/maxim from v1.0.3 to v1.0.5.

Sequence Diagram(s)

sequenceDiagram
    participant Client
    participant AnthropicIntegration
    participant Bifrost
    Client->>AnthropicIntegration: Send AnthropicMessageRequest
    AnthropicIntegration->>AnthropicIntegration: Convert content (string or blocks) to BifrostMessage
    AnthropicIntegration->>Bifrost: Send BifrostRequest
    Bifrost-->>AnthropicIntegration: Return BifrostResponse
    AnthropicIntegration->>AnthropicIntegration: Convert BifrostResponse to AnthropicMessageResponse (blocks or string)
    AnthropicIntegration-->>Client: Return AnthropicMessageResponse
Loading
sequenceDiagram
    participant Client
    participant GeminiIntegration
    participant Bifrost
    Client->>GeminiIntegration: Send GeminiChatRequest
    GeminiIntegration->>GeminiIntegration: Convert content parts to ContentBlocks
    GeminiIntegration->>Bifrost: Send BifrostRequest
    Bifrost-->>GeminiIntegration: Return BifrostResponse
    GeminiIntegration->>GeminiIntegration: Extract ContentBlocks to Gemini response
    GeminiIntegration-->>Client: Return GeminiChatResponse
Loading
sequenceDiagram
    participant Client
    participant OpenAIIntegration
    participant Bifrost
    Client->>OpenAIIntegration: Send OpenAIChatRequest (with BifrostMessages)
    OpenAIIntegration->>Bifrost: Send BifrostRequest
    Bifrost-->>OpenAIIntegration: Return BifrostResponse
    OpenAIIntegration-->>Client: Return OpenAIChatResponse (direct mapping)
Loading

Possibly related PRs

Suggested reviewers

  • danpiths
  • akshaydeo

Poem

In the garden of code, where content blocks grow,
Messages now blossom in structured rows.
Anthropic, Gemini, and OpenAI unite,
With Bifrost bridges shining bright.
A hop, a skip, a refactor anew—
🐇 This rabbit cheers the work you do!


📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 641078f and d10aa09.

⛔ Files ignored due to path filters (1)
  • transports/go.sum is excluded by !**/*.sum
📒 Files selected for processing (5)
  • transports/bifrost-http/integrations/anthropic/types.go (8 hunks)
  • transports/bifrost-http/integrations/genai/types.go (3 hunks)
  • transports/bifrost-http/integrations/openai/types.go (4 hunks)
  • transports/bifrost-http/tracking/plugin.go (4 hunks)
  • transports/go.mod (1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
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.
transports/bifrost-http/integrations/genai/types.go (5)
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#75
File: transports/bifrost-http/integrations/genai/types.go:365-374
Timestamp: 2025-06-12T09:06:26.608Z
Learning: Gemini integration currently guarantees that assistant messages never contain image content blocks, so re-hydration code can safely ignore them.
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.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#75
File: transports/bifrost-http/integrations/genai/types.go:161-178
Timestamp: 2025-06-14T08:45:19.099Z
Learning: In the Bifrost project, when users pass empty messages (no content blocks and no content string), the system should error rather than trying to handle them gracefully. This is an intentional design decision - empty messages that would result in invalid JSON serialization are expected to cause errors to fail fast on invalid user input.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#75
File: transports/bifrost-http/integrations/genai/types.go:109-117
Timestamp: 2025-06-14T08:54:58.119Z
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.
transports/bifrost-http/integrations/anthropic/types.go (3)
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#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#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.
🧬 Code Graph Analysis (4)
transports/bifrost-http/tracking/plugin.go (1)
core/schemas/bifrost.go (3)
  • BifrostRequest (59-69)
  • BifrostResponse (237-247)
  • BifrostError (365-371)
transports/bifrost-http/integrations/genai/types.go (2)
core/schemas/bifrost.go (14)
  • BifrostMessage (148-156)
  • ToolCall (319-323)
  • ContentBlock (209-213)
  • ModelChatMessageRoleAssistant (28-28)
  • ContentBlockTypeText (205-205)
  • FunctionCall (313-316)
  • Function (106-110)
  • ModelChatMessageRoleTool (32-32)
  • MessageContent (158-161)
  • ToolMessage (216-218)
  • ContentBlockTypeImage (206-206)
  • ImageURLStruct (229-232)
  • ModelChatMessageRole (25-25)
  • AssistantMessage (221-226)
core/utils.go (1)
  • Ptr (5-7)
transports/bifrost-http/integrations/openai/types.go (1)
core/schemas/bifrost.go (6)
  • BifrostMessage (148-156)
  • Tool (113-117)
  • ToolChoice (142-145)
  • LogProbs (306-310)
  • BifrostResponseChoice (342-348)
  • LLMUsage (250-256)
transports/bifrost-http/integrations/anthropic/types.go (2)
core/schemas/bifrost.go (18)
  • BifrostRequest (59-69)
  • Anthropic (41-41)
  • BifrostMessage (148-156)
  • ModelChatMessageRoleSystem (30-30)
  • MessageContent (158-161)
  • ContentBlock (209-213)
  • ContentBlockTypeText (205-205)
  • ModelChatMessageRole (25-25)
  • ToolCall (319-323)
  • ContentBlockTypeImage (206-206)
  • ImageURLStruct (229-232)
  • Function (106-110)
  • FunctionCall (313-316)
  • ToolMessage (216-218)
  • ModelChatMessageRoleTool (32-32)
  • ModelChatMessageRoleAssistant (28-28)
  • AssistantMessage (221-226)
  • Tool (113-117)
core/schemas/provider.go (1)
  • Provider (144-151)
🔇 Additional comments (5)
transports/bifrost-http/tracking/plugin.go (1)

73-81: Good improvement: Using structured logging over stdout

Switching from fmt.Println to log.Println ensures warnings go through the proper logging infrastructure rather than directly to stdout.

transports/bifrost-http/integrations/genai/types.go (1)

165-170: Correct role mapping implementation

The conversion of Gemini's "model" role to Bifrost's "assistant" role is properly implemented, preventing invalid roles from propagating through the system.

transports/bifrost-http/integrations/openai/types.go (1)

1-138: Clean simplification of OpenAI integration

The removal of custom intermediate types and direct usage of shared Bifrost schema types significantly reduces complexity and improves maintainability. This aligns well with the standardization efforts across all integrations.

transports/bifrost-http/integrations/anthropic/types.go (2)

197-210: Proper data URI generation for base64 images

The implementation correctly generates data URIs with proper MIME type prefixes for base64-encoded images, ensuring consistency with other integrations and enabling proper client rendering.


275-279: Correct role-based assignment of AssistantMessage

The implementation properly checks that the message role is "assistant" before assigning tool calls to AssistantMessage, preventing invalid chat history for non-assistant roles.

✨ Finishing Touches
  • 📝 Generate Docstrings
🧪 Generate Unit Tests
  • Create PR with Unit Tests
  • Commit Unit Tests in branch 06-12-feat_http_transport_integrations_refactored_for_core_v1.1.0
  • Post Copyable Unit Tests in Comment

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Explain this complex logic.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai explain this code block.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and explain its main purpose.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Support

Need 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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai auto-generate unit tests to generate unit tests for this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link
Copy Markdown
Collaborator Author

Pratham-Mishra04 commented Jun 11, 2025

This stack of pull requests is managed by Graphite. Learn more about stacking.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

📜 Review details

Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8b814ca and 394b14f.

⛔ Files ignored due to path filters (1)
  • transports/go.sum is excluded by !**/*.sum
📒 Files selected for processing (5)
  • transports/bifrost-http/integrations/anthropic/types.go (8 hunks)
  • transports/bifrost-http/integrations/genai/types.go (5 hunks)
  • transports/bifrost-http/integrations/openai/types.go (4 hunks)
  • transports/bifrost-http/tracking/plugin.go (3 hunks)
  • transports/go.mod (1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#65
File: transports/bifrost-http/integrations/anthropic/types.go:131-180
Timestamp: 2025-06-10T13:11:37.842Z
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.
transports/bifrost-http/integrations/anthropic/types.go (1)
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#65
File: transports/bifrost-http/integrations/anthropic/types.go:131-180
Timestamp: 2025-06-10T13:11:37.842Z
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.
🧬 Code Graph Analysis (3)
transports/bifrost-http/integrations/genai/types.go (2)
core/schemas/bifrost.go (13)
  • ModelChatMessageRoleSystem (30-30)
  • ModelChatMessageRole (25-25)
  • BifrostMessage (148-156)
  • ToolCall (319-323)
  • ContentBlock (209-213)
  • ContentBlockTypeText (205-205)
  • ContentBlockTypeImage (206-206)
  • ImageURLStruct (229-232)
  • ModelChatMessageRoleTool (32-32)
  • MessageContent (158-161)
  • ToolMessage (216-218)
  • ModelChatMessageRoleAssistant (28-28)
  • AssistantMessage (221-226)
core/utils.go (1)
  • Ptr (5-7)
transports/bifrost-http/tracking/plugin.go (1)
core/schemas/bifrost.go (2)
  • BifrostRequest (59-69)
  • BifrostResponse (237-247)
transports/bifrost-http/integrations/openai/types.go (1)
core/schemas/bifrost.go (6)
  • BifrostMessage (148-156)
  • Tool (113-117)
  • ToolChoice (142-145)
  • LogProbs (306-310)
  • BifrostResponseChoice (342-348)
  • LLMUsage (250-256)
🔇 Additional comments (12)
transports/go.mod (1)

7-7: LGTM!

The core dependency update to v1.1.0 aligns with the PR objectives and supports the refactored content handling across integrations.

transports/bifrost-http/tracking/plugin.go (1)

99-101: LGTM!

The addition of the Cleanup method provides a standard way for plugins to release resources. The empty implementation is appropriate for this metrics plugin.

transports/bifrost-http/integrations/genai/types.go (3)

140-140: Breaking change: Method signature updated.

The method signature change from returning []schemas.BifrostMessage to schemas.BifrostMessage is a breaking change. This aligns with the unified content model approach across integrations.


147-218: Excellent refactoring to unified content model!

The consolidation of multiple message types (text, images, function calls) into a single message with structured content blocks improves consistency and maintainability. The implementation correctly handles all content types including tool calls.


329-337: Response deserialization correctly handles both content formats.

The updated logic properly handles both simple string content and structured content blocks, ensuring backward compatibility while supporting the new content model.

transports/bifrost-http/integrations/openai/types.go (3)

12-30: Excellent simplification using Bifrost schema types directly!

Removing custom message types in favor of direct schemas.BifrostMessage usage reduces code duplication and improves maintainability. The integration is now more aligned with the core Bifrost schema.


50-50: Clean and direct field mapping.

The simplified conversion logic with direct field assignments is much clearer and reduces the potential for conversion errors.

Also applies to: 67-68


123-140: Streamlined response conversion.

The direct field copying approach for response conversion is efficient and maintains type safety.

transports/bifrost-http/integrations/anthropic/types.go (4)

38-80: Well-implemented custom marshaling for flexible content handling.

The custom JSON marshaling/unmarshaling for AnthropicContent elegantly handles both string and block array formats, ensuring backward compatibility while supporting the new structured content model.


87-91: Good improvement in type safety for tool schemas.

Making InputSchema explicitly typed with defined fields improves type safety and makes the API contract clearer.


261-290: Tool result handling looks correct based on the API constraints.

The implementation correctly handles tool results with their associated content. Based on the retrieved learning, messages won't contain both image and tool_result blocks, so the current implementation is appropriate.


342-346: Simplified tool schema conversion.

The direct field mapping for tool schemas is cleaner and more maintainable than the previous approach.

Comment thread transports/bifrost-http/tracking/plugin.go
Comment thread transports/bifrost-http/integrations/anthropic/types.go Outdated
@Pratham-Mishra04 Pratham-Mishra04 force-pushed the 06-12-feat_http_transport_integrations_refactored_for_core_v1.1.0 branch 2 times, most recently from 110e4f5 to 5dcba7a Compare June 12, 2025 06:27
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🔭 Outside diff range comments (3)
transports/go.mod (1)

3-3: ⚠️ Potential issue

Unreleased Go toolchain version specified

go 1.24.1 does not exist (latest stable is 1.22.x). Using a future toolchain string will break go commands (go: unknown release).
Pin to the latest released minor (e.g. 1.22) or omit the patch component (1.24) until it’s actually shipped.

transports/bifrost-http/tracking/plugin.go (1)

45-55: 🧹 Nitpick (assertive)

Consider early-return capability in PreHook

PreHook now always returns (req, nil, nil).
If a plugin ever needs to short-circuit with a synthetic *schemas.BifrostResponse, the current implementation ignores that possibility.
At minimum, add a comment clarifying that this plugin never short-circuits, or return nil, nil, nil when req should be halted.

transports/bifrost-http/integrations/openai/types.go (1)

114-117: ⚠️ Potential issue

convertParameters may silently return nil even when Tools / ToolChoice are set

return nil is triggered solely by len(params.ExtraParams)==0, ignoring params.Tools or params.ToolChoice.
Down-stream code will then omit tool metadata.

-if len(params.ExtraParams) == 0 {
-    return nil
-}
+if len(params.ExtraParams) == 0 && params.Tools == nil && params.ToolChoice == nil {
+    return nil
+}
📜 Review details

Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 110e4f5 and 5dcba7a.

⛔ Files ignored due to path filters (1)
  • transports/go.sum is excluded by !**/*.sum
📒 Files selected for processing (5)
  • transports/bifrost-http/integrations/anthropic/types.go (8 hunks)
  • transports/bifrost-http/integrations/genai/types.go (4 hunks)
  • transports/bifrost-http/integrations/openai/types.go (4 hunks)
  • transports/bifrost-http/tracking/plugin.go (3 hunks)
  • transports/go.mod (1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#65
File: transports/bifrost-http/integrations/anthropic/types.go:131-180
Timestamp: 2025-06-10T13:11:37.842Z
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.
transports/bifrost-http/integrations/anthropic/types.go (1)
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#65
File: transports/bifrost-http/integrations/anthropic/types.go:131-180
Timestamp: 2025-06-10T13:11:37.842Z
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.
🧬 Code Graph Analysis (4)
transports/bifrost-http/tracking/plugin.go (1)
core/schemas/bifrost.go (2)
  • BifrostRequest (59-69)
  • BifrostResponse (237-247)
transports/bifrost-http/integrations/genai/types.go (2)
core/schemas/bifrost.go (15)
  • ModelChatMessageRoleSystem (30-30)
  • ModelChatMessageRole (25-25)
  • BifrostMessage (148-156)
  • MessageContent (158-161)
  • ToolCall (319-323)
  • ContentBlock (209-213)
  • ModelChatMessageRoleAssistant (28-28)
  • ContentBlockTypeText (205-205)
  • FunctionCall (313-316)
  • Function (106-110)
  • ModelChatMessageRoleTool (32-32)
  • ToolMessage (216-218)
  • ContentBlockTypeImage (206-206)
  • ImageURLStruct (229-232)
  • AssistantMessage (221-226)
core/utils.go (1)
  • Ptr (5-7)
transports/bifrost-http/integrations/openai/types.go (1)
core/schemas/bifrost.go (6)
  • BifrostMessage (148-156)
  • Tool (113-117)
  • ToolChoice (142-145)
  • LogProbs (306-310)
  • BifrostResponseChoice (342-348)
  • LLMUsage (250-256)
transports/bifrost-http/integrations/anthropic/types.go (2)
core/schemas/bifrost.go (15)
  • BifrostRequest (59-69)
  • Anthropic (41-41)
  • BifrostMessage (148-156)
  • MessageContent (158-161)
  • ContentBlock (209-213)
  • ContentBlockTypeText (205-205)
  • ModelChatMessageRole (25-25)
  • ToolCall (319-323)
  • ContentBlockTypeImage (206-206)
  • ImageURLStruct (229-232)
  • Function (106-110)
  • FunctionCall (313-316)
  • ToolMessage (216-218)
  • AssistantMessage (221-226)
  • Tool (113-117)
core/schemas/provider.go (1)
  • Provider (142-149)
🔇 Additional comments (1)
transports/go.mod (1)

7-8: Dependency bump LGTM

Bumping bifrost/core to v1.1.0 aligns the transports layer with the new schema types. No issues spotted here.

Comment thread transports/bifrost-http/integrations/genai/types.go Outdated
Comment thread transports/bifrost-http/integrations/genai/types.go Outdated
Comment thread transports/bifrost-http/integrations/anthropic/types.go
@Pratham-Mishra04 Pratham-Mishra04 force-pushed the 06-12-feat_http_transport_integrations_refactored_for_core_v1.1.0 branch 2 times, most recently from 5112c64 to 8b01016 Compare June 12, 2025 07:47
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 8

🔭 Outside diff range comments (1)
transports/bifrost-http/integrations/openai/types.go (1)

114-118: ⚠️ Potential issue

convertParameters drops Tools / ToolChoice when no extra params

You return nil solely based on len(ExtraParams), losing non-nil Tools / ToolChoice.

- if len(params.ExtraParams) == 0 {
-     return nil
- }
+ if len(params.ExtraParams) == 0 && params.Tools == nil && params.ToolChoice == nil {
+     return nil
+ }
♻️ Duplicate comments (1)
transports/bifrost-http/tracking/plugin.go (1)

45-55: Ensure all other PreHook implementations match the new 3-value signature

This file is updated, but any out-of-tree plugin still compiling against the old two-value form will break.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 5dcba7a and 8b01016.

⛔ Files ignored due to path filters (1)
  • transports/go.sum is excluded by !**/*.sum
📒 Files selected for processing (5)
  • transports/bifrost-http/integrations/anthropic/types.go (8 hunks)
  • transports/bifrost-http/integrations/genai/types.go (3 hunks)
  • transports/bifrost-http/integrations/openai/types.go (4 hunks)
  • transports/bifrost-http/tracking/plugin.go (3 hunks)
  • transports/go.mod (1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#65
File: transports/bifrost-http/integrations/anthropic/types.go:131-180
Timestamp: 2025-06-10T13:11:37.842Z
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.
transports/bifrost-http/integrations/anthropic/types.go (1)
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#65
File: transports/bifrost-http/integrations/anthropic/types.go:131-180
Timestamp: 2025-06-10T13:11:37.842Z
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.
🧬 Code Graph Analysis (3)
transports/bifrost-http/tracking/plugin.go (1)
core/schemas/bifrost.go (2)
  • BifrostRequest (59-69)
  • BifrostResponse (237-247)
transports/bifrost-http/integrations/openai/types.go (1)
core/schemas/bifrost.go (6)
  • BifrostMessage (148-156)
  • Tool (113-117)
  • ToolChoice (142-145)
  • LogProbs (306-310)
  • BifrostResponseChoice (342-348)
  • LLMUsage (250-256)
transports/bifrost-http/integrations/genai/types.go (2)
core/schemas/bifrost.go (14)
  • BifrostMessage (148-156)
  • ToolCall (319-323)
  • ContentBlock (209-213)
  • ModelChatMessageRoleAssistant (28-28)
  • ContentBlockTypeText (205-205)
  • FunctionCall (313-316)
  • Function (106-110)
  • ModelChatMessageRoleTool (32-32)
  • MessageContent (158-161)
  • ToolMessage (216-218)
  • ContentBlockTypeImage (206-206)
  • ImageURLStruct (229-232)
  • ModelChatMessageRole (25-25)
  • AssistantMessage (221-226)
core/utils.go (1)
  • Ptr (5-7)
🔇 Additional comments (1)
transports/bifrost-http/integrations/anthropic/types.go (1)

400-405: thinking is not an Anthropic content type

The official spec allows text, image, tool_use, tool_result.
Serialising an unknown type may be rejected by the API.

Comment thread transports/go.mod Outdated
Comment thread transports/bifrost-http/tracking/plugin.go
Comment thread transports/bifrost-http/integrations/genai/types.go
Comment thread transports/bifrost-http/integrations/genai/types.go
Comment thread transports/bifrost-http/integrations/genai/types.go Outdated
Comment thread transports/bifrost-http/integrations/genai/types.go
Comment thread transports/bifrost-http/integrations/openai/types.go Outdated
Comment thread transports/bifrost-http/integrations/anthropic/types.go Outdated
@Pratham-Mishra04 Pratham-Mishra04 force-pushed the 06-12-feat_http_transport_integrations_refactored_for_core_v1.1.0 branch from 8b01016 to d007882 Compare June 12, 2025 09:03
Comment thread transports/bifrost-http/integrations/openai/types.go Outdated
@Pratham-Mishra04 Pratham-Mishra04 force-pushed the 06-12-feat_http_transport_integrations_refactored_for_core_v1.1.0 branch from d007882 to 80028cc Compare June 12, 2025 09:23
@Pratham-Mishra04 Pratham-Mishra04 marked this pull request as ready for review June 12, 2025 10:41
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🔭 Outside diff range comments (2)
transports/bifrost-http/integrations/openai/types.go (2)

111-116: ⚠️ Potential issue

convertParameters drops Tools / ToolChoice when no extra params

If the caller sets Tools or ToolChoice but nothing else, len(params.ExtraParams)==0 makes the function return nil, silently discarding those settings.

- // Return nil if no parameters were set
- if len(params.ExtraParams) == 0 {
-     return nil
- }
+// Return nil only when *nothing* is populated
+if len(params.ExtraParams) == 0 && params.Tools == nil && params.ToolChoice == nil {
+    return nil
+}

68-76: 🧹 Nitpick (assertive)

Prefer dedicated fields over dumping into ExtraParams
MaxTokens, Temperature, TopP, etc. already exist on ModelParameters. Populate those first for provider-agnostic handling; fall back to ExtraParams only for OpenAI-specific knobs.

♻️ Duplicate comments (1)
transports/go.mod (1)

7-8: Run go mod tidy after the version bump
The upgrade will likely leave stale entries in go.sum; run a tidy to sync both files.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d007882 and 80028cc.

⛔ Files ignored due to path filters (1)
  • transports/go.sum is excluded by !**/*.sum
📒 Files selected for processing (5)
  • transports/bifrost-http/integrations/anthropic/types.go (8 hunks)
  • transports/bifrost-http/integrations/genai/types.go (3 hunks)
  • transports/bifrost-http/integrations/openai/types.go (4 hunks)
  • transports/bifrost-http/tracking/plugin.go (3 hunks)
  • transports/go.mod (1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (3)
transports/bifrost-http/tracking/plugin.go (1)
core/schemas/bifrost.go (2)
  • BifrostRequest (59-69)
  • BifrostResponse (237-247)
transports/bifrost-http/integrations/genai/types.go (2)
core/schemas/bifrost.go (14)
  • BifrostMessage (148-156)
  • ToolCall (319-323)
  • ContentBlock (209-213)
  • ModelChatMessageRoleAssistant (28-28)
  • ContentBlockTypeText (205-205)
  • FunctionCall (313-316)
  • Function (106-110)
  • ModelChatMessageRoleTool (32-32)
  • MessageContent (158-161)
  • ToolMessage (216-218)
  • ContentBlockTypeImage (206-206)
  • ImageURLStruct (229-232)
  • ModelChatMessageRole (25-25)
  • AssistantMessage (221-226)
core/utils.go (1)
  • Ptr (5-7)
transports/bifrost-http/integrations/openai/types.go (1)
core/schemas/bifrost.go (6)
  • BifrostMessage (148-156)
  • Tool (113-117)
  • ToolChoice (142-145)
  • LogProbs (306-310)
  • BifrostResponseChoice (342-348)
  • LLMUsage (250-256)
🔇 Additional comments (1)
transports/bifrost-http/integrations/genai/types.go (1)

110-118: Verify ToolCallID source – Name may not be unique
ToolMessage.ToolCallID is populated with FunctionResponse.Name, but the GenAI schema also exposes an ID field. Using a stable ID (when present) prevents collisions when the same function is called multiple times.

Comment thread transports/bifrost-http/tracking/plugin.go
Comment thread transports/bifrost-http/integrations/genai/types.go
Comment thread transports/bifrost-http/integrations/anthropic/types.go
@Pratham-Mishra04 Pratham-Mishra04 force-pushed the 06-12-feat_http_transport_integrations_refactored_for_core_v1.1.0 branch from 80028cc to 112034d Compare June 12, 2025 11:01
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🔭 Outside diff range comments (3)
transports/go.mod (1)

3-3: ⚠️ Potential issue

Invalid Go toolchain version

go 1.24.1 targets a release that does not (yet) exist.
Tooling (go mod tidy, go vet, CI runners) will error with
go: unsupported Go version and block builds. Revert to the latest stable
(runtime‐supported) version used across the repo, e.g. go 1.23.

transports/bifrost-http/integrations/openai/types.go (2)

111-114: ⚠️ Potential issue

convertParameters drops Tools / ToolChoice unintentionally

return nil is triggered when ExtraParams is empty—even if Tools
or ToolChoice were set.
This silently strips tool definitions and explicit tool-choice from
every request.

- if len(params.ExtraParams) == 0 {
-     return nil
- }
+ if len(params.ExtraParams) == 0 &&
+    params.Tools == nil &&
+    params.ToolChoice == nil {
+     return nil
+ }

64-82: 🧹 Nitpick (assertive)

Prefer typed fields over ExtraParams for core knobs

MaxTokens, Temperature, TopP, etc. already have dedicated fields
on schemas.ModelParameters; populating them via ExtraParams forces
every provider to repeat custom extraction logic.
Mapping to the canonical fields keeps behaviour consistent with Anthropic
and Gemini adapters and avoids drift.

♻️ Duplicate comments (2)
transports/go.mod (1)

7-9: Run go mod tidy after bumping deps
Same nit as earlier: tidy will prune indirects and update go.sum.

transports/bifrost-http/tracking/plugin.go (1)

98-101: Add an explanatory docstring for the no-op Cleanup
Prior feedback still stands—please add a one-liner so future readers (and linters) know the no-op is intentional.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 80028cc and 112034d.

⛔ Files ignored due to path filters (1)
  • transports/go.sum is excluded by !**/*.sum
📒 Files selected for processing (5)
  • transports/bifrost-http/integrations/anthropic/types.go (8 hunks)
  • transports/bifrost-http/integrations/genai/types.go (3 hunks)
  • transports/bifrost-http/integrations/openai/types.go (4 hunks)
  • transports/bifrost-http/tracking/plugin.go (3 hunks)
  • transports/go.mod (1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (3)
transports/bifrost-http/tracking/plugin.go (1)
core/schemas/bifrost.go (2)
  • BifrostRequest (59-69)
  • BifrostResponse (237-247)
transports/bifrost-http/integrations/openai/types.go (1)
core/schemas/bifrost.go (6)
  • BifrostMessage (148-156)
  • Tool (113-117)
  • ToolChoice (142-145)
  • LogProbs (306-310)
  • BifrostResponseChoice (342-348)
  • LLMUsage (250-256)
transports/bifrost-http/integrations/genai/types.go (2)
core/schemas/bifrost.go (14)
  • BifrostMessage (148-156)
  • ToolCall (319-323)
  • ContentBlock (209-213)
  • ModelChatMessageRoleAssistant (28-28)
  • ContentBlockTypeText (205-205)
  • FunctionCall (313-316)
  • Function (106-110)
  • ModelChatMessageRoleTool (32-32)
  • MessageContent (158-161)
  • ToolMessage (216-218)
  • ContentBlockTypeImage (206-206)
  • ImageURLStruct (229-232)
  • ModelChatMessageRole (25-25)
  • AssistantMessage (221-226)
core/utils.go (1)
  • Ptr (5-7)

Comment thread transports/bifrost-http/integrations/genai/types.go
Comment thread transports/bifrost-http/integrations/genai/types.go
Comment thread transports/bifrost-http/integrations/anthropic/types.go
@Pratham-Mishra04 Pratham-Mishra04 force-pushed the 06-12-feat_http_transport_integrations_refactored_for_core_v1.1.0 branch from 112034d to 551229f Compare June 14, 2025 04:26
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🔭 Outside diff range comments (2)
transports/bifrost-http/tracking/plugin.go (1)

45-55: 🛠️ Refactor suggestion

Avoid mutating *context.Context – pass by value.

Passing a pointer to context.Context is atypical and forces every hook to mutate shared state, risking nil-deref and data races.
Prefer ctx context.Context and return the derived context instead:

-func (p *PrometheusPlugin) PreHook(ctx *context.Context, req *schemas.BifrostRequest) ...
+func (p *PrometheusPlugin) PreHook(ctx context.Context, req *schemas.BifrostRequest) (context.Context, *schemas.BifrostRequest, *schemas.BifrostResponse, error) {
    ctx = context.WithValue(ctx, startTimeKey, time.Now())
    ...
-   return req, nil, nil
+   return ctx, req, nil, nil
}
transports/bifrost-http/integrations/openai/types.go (1)

111-114: ⚠️ Potential issue

Parameter object may be dropped even when Tools/ToolChoice are set.

convertParameters returns nil solely based on len(params.ExtraParams), ignoring Tools, ToolChoice, etc.
This strips tool metadata from the request.

- if len(params.ExtraParams) == 0 {
-     return nil
- }
+if len(params.ExtraParams) == 0 &&
+   params.Tools == nil &&
+   params.ToolChoice == nil {
+        return nil
+}
♻️ Duplicate comments (2)
transports/go.mod (1)

7-8: Still needs go mod tidy.

Version bump is fine, but go.sum hasn’t been refreshed—run go mod tidy to prune indirects and keep the sums in sync.

transports/bifrost-http/tracking/plugin.go (1)

99-101: Add the missing docstring to this no-op.

The earlier nit is still unresolved—document why Cleanup intentionally does nothing to silence “dead code” questions.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 112034d and 551229f.

⛔ Files ignored due to path filters (1)
  • transports/go.sum is excluded by !**/*.sum
📒 Files selected for processing (5)
  • transports/bifrost-http/integrations/anthropic/types.go (8 hunks)
  • transports/bifrost-http/integrations/genai/types.go (3 hunks)
  • transports/bifrost-http/integrations/openai/types.go (4 hunks)
  • transports/bifrost-http/tracking/plugin.go (3 hunks)
  • transports/go.mod (1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#65
File: transports/bifrost-http/integrations/anthropic/types.go:131-180
Timestamp: 2025-06-10T13:11:37.842Z
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.
transports/bifrost-http/integrations/genai/types.go (2)
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#74
File: core/providers/bedrock.go:909-916
Timestamp: 2025-06-11T15:05:45.322Z
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.
transports/bifrost-http/integrations/anthropic/types.go (3)
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#65
File: transports/bifrost-http/integrations/anthropic/types.go:131-180
Timestamp: 2025-06-10T13:11:37.842Z
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#63
File: transports/bifrost-http/integrations/openai/types.go:89-119
Timestamp: 2025-06-10T11:00:02.852Z
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#75
File: transports/bifrost-http/integrations/anthropic/types.go:154-160
Timestamp: 2025-06-14T04:19:06.305Z
Learning: Anthropic's API does not support images in system messages, only text content is allowed in system messages for Anthropic integrations.
🧬 Code Graph Analysis (4)
transports/bifrost-http/tracking/plugin.go (1)
core/schemas/bifrost.go (2)
  • BifrostRequest (59-69)
  • BifrostResponse (237-247)
transports/bifrost-http/integrations/genai/types.go (2)
core/schemas/bifrost.go (14)
  • BifrostMessage (148-156)
  • ToolCall (319-323)
  • ContentBlock (209-213)
  • ModelChatMessageRoleAssistant (28-28)
  • ContentBlockTypeText (205-205)
  • FunctionCall (313-316)
  • Function (106-110)
  • ModelChatMessageRoleTool (32-32)
  • MessageContent (158-161)
  • ToolMessage (216-218)
  • ContentBlockTypeImage (206-206)
  • ImageURLStruct (229-232)
  • ModelChatMessageRole (25-25)
  • AssistantMessage (221-226)
core/utils.go (1)
  • Ptr (5-7)
transports/bifrost-http/integrations/openai/types.go (1)
core/schemas/bifrost.go (6)
  • BifrostMessage (148-156)
  • Tool (113-117)
  • ToolChoice (142-145)
  • LogProbs (306-310)
  • BifrostResponseChoice (342-348)
  • LLMUsage (250-256)
transports/bifrost-http/integrations/anthropic/types.go (2)
core/schemas/bifrost.go (15)
  • BifrostRequest (59-69)
  • BifrostMessage (148-156)
  • ModelChatMessageRoleSystem (30-30)
  • MessageContent (158-161)
  • ContentBlock (209-213)
  • ModelChatMessageRole (25-25)
  • ToolCall (319-323)
  • ImageURLStruct (229-232)
  • Function (106-110)
  • FunctionCall (313-316)
  • ToolMessage (216-218)
  • ModelChatMessageRoleTool (32-32)
  • ModelChatMessageRoleAssistant (28-28)
  • AssistantMessage (221-226)
  • Tool (113-117)
core/schemas/provider.go (1)
  • Provider (142-149)
🔇 Additional comments (1)
transports/bifrost-http/integrations/anthropic/types.go (1)

95-103: Nice validation check.

Guarding against both fields being set prevents ambiguous serialization—good catch.

Comment thread transports/bifrost-http/integrations/genai/types.go Outdated
@Pratham-Mishra04 Pratham-Mishra04 force-pushed the 06-12-feat_http_transport_integrations_refactored_for_core_v1.1.0 branch from 551229f to 834a847 Compare June 14, 2025 04:32
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🔭 Outside diff range comments (1)
transports/bifrost-http/integrations/openai/types.go (1)

111-114: ⚠️ Potential issue

convertParameters drops Tools / ToolChoice

You return nil when ExtraParams is empty, ignoring that params.Tools or params.ToolChoice may be set.
This strips tool metadata from the outgoing request.

- if len(params.ExtraParams) == 0 {
-     return nil
- }
+ if len(params.ExtraParams) == 0 &&
+    params.Tools == nil &&
+    params.ToolChoice == nil {
+     return nil
+ }
♻️ Duplicate comments (2)
transports/go.mod (1)

7-8: go mod tidy still missing

The version bumps are fine, but please run go mod tidy so that go.sum and indirects stay consistent.

transports/bifrost-http/tracking/plugin.go (1)

98-101: Add the docstring that earlier review requested

Cleanup is still a bare method without the one-liner explaining the deliberate no-op.
Add something like

// Cleanup implements schemas.Plugin; nothing to release.
📜 Review details

Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 551229f and 834a847.

⛔ Files ignored due to path filters (1)
  • transports/go.sum is excluded by !**/*.sum
📒 Files selected for processing (5)
  • transports/bifrost-http/integrations/anthropic/types.go (8 hunks)
  • transports/bifrost-http/integrations/genai/types.go (3 hunks)
  • transports/bifrost-http/integrations/openai/types.go (4 hunks)
  • transports/bifrost-http/tracking/plugin.go (3 hunks)
  • transports/go.mod (1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#65
File: transports/bifrost-http/integrations/anthropic/types.go:131-180
Timestamp: 2025-06-10T13:11:37.842Z
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.
transports/bifrost-http/integrations/genai/types.go (2)
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#74
File: core/providers/bedrock.go:909-916
Timestamp: 2025-06-11T15:05:45.322Z
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.
transports/bifrost-http/integrations/anthropic/types.go (3)
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#65
File: transports/bifrost-http/integrations/anthropic/types.go:131-180
Timestamp: 2025-06-10T13:11:37.842Z
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#63
File: transports/bifrost-http/integrations/openai/types.go:89-119
Timestamp: 2025-06-10T11:00:02.852Z
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#75
File: transports/bifrost-http/integrations/anthropic/types.go:154-160
Timestamp: 2025-06-14T04:19:06.305Z
Learning: Anthropic's API does not support images in system messages, only text content is allowed in system messages for Anthropic integrations.
🧬 Code Graph Analysis (4)
transports/bifrost-http/tracking/plugin.go (1)
core/schemas/bifrost.go (2)
  • BifrostRequest (59-69)
  • BifrostResponse (237-247)
transports/bifrost-http/integrations/genai/types.go (2)
core/schemas/bifrost.go (14)
  • BifrostMessage (148-156)
  • ToolCall (319-323)
  • ContentBlock (209-213)
  • ModelChatMessageRoleAssistant (28-28)
  • ContentBlockTypeText (205-205)
  • FunctionCall (313-316)
  • Function (106-110)
  • ModelChatMessageRoleTool (32-32)
  • MessageContent (158-161)
  • ToolMessage (216-218)
  • ContentBlockTypeImage (206-206)
  • ImageURLStruct (229-232)
  • ModelChatMessageRole (25-25)
  • AssistantMessage (221-226)
core/utils.go (1)
  • Ptr (5-7)
transports/bifrost-http/integrations/openai/types.go (1)
core/schemas/bifrost.go (6)
  • BifrostMessage (148-156)
  • Tool (113-117)
  • ToolChoice (142-145)
  • LogProbs (306-310)
  • BifrostResponseChoice (342-348)
  • LLMUsage (250-256)
transports/bifrost-http/integrations/anthropic/types.go (2)
core/schemas/bifrost.go (15)
  • Anthropic (41-41)
  • BifrostMessage (148-156)
  • ModelChatMessageRoleSystem (30-30)
  • MessageContent (158-161)
  • ContentBlock (209-213)
  • ModelChatMessageRole (25-25)
  • ToolCall (319-323)
  • ImageURLStruct (229-232)
  • Function (106-110)
  • FunctionCall (313-316)
  • ToolMessage (216-218)
  • ModelChatMessageRoleTool (32-32)
  • ModelChatMessageRoleAssistant (28-28)
  • AssistantMessage (221-226)
  • Tool (113-117)
core/schemas/provider.go (1)
  • Provider (142-149)

Comment thread transports/bifrost-http/tracking/plugin.go
Comment thread transports/bifrost-http/integrations/genai/types.go Outdated
Comment thread transports/bifrost-http/integrations/anthropic/types.go
@Pratham-Mishra04 Pratham-Mishra04 force-pushed the 06-12-feat_http_transport_integrations_refactored_for_core_v1.1.0 branch from 834a847 to 90d624f Compare June 14, 2025 04:44
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

♻️ Duplicate comments (3)
transports/go.mod (1)

7-8: Run go mod tidy to ensure dependencies are properly synchronized.

After updating the module versions, run go mod tidy to clean up indirect dependencies and ensure go.sum is in sync with the new versions.

transports/bifrost-http/tracking/plugin.go (2)

45-45: Verify all plugin implementations have been updated with the new PreHook signature.

The PreHook method now returns three values. Ensure all implementations are updated to match.

#!/bin/bash
# Description: Find all PreHook implementations to verify they match the new signature

# Search for PreHook method implementations
ast-grep --pattern 'func ($_ $_) PreHook($_, $_) ($_, $_, $_)' 

# Also search for old signature pattern to find any that need updating
ast-grep --pattern 'func ($_ $_) PreHook($_, $_) ($_, $_)'

Also applies to: 54-54


99-101: Add documentation to clarify the no-op Cleanup method.

Add a comment explaining that this method implements the schemas.Plugin interface and intentionally performs no cleanup.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 834a847 and 90d624f.

⛔ Files ignored due to path filters (1)
  • transports/go.sum is excluded by !**/*.sum
📒 Files selected for processing (5)
  • transports/bifrost-http/integrations/anthropic/types.go (8 hunks)
  • transports/bifrost-http/integrations/genai/types.go (3 hunks)
  • transports/bifrost-http/integrations/openai/types.go (4 hunks)
  • transports/bifrost-http/tracking/plugin.go (3 hunks)
  • transports/go.mod (1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#65
File: transports/bifrost-http/integrations/anthropic/types.go:131-180
Timestamp: 2025-06-10T13:11:37.842Z
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.
transports/bifrost-http/integrations/genai/types.go (2)
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#74
File: core/providers/bedrock.go:909-916
Timestamp: 2025-06-11T15:05:45.322Z
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.
transports/bifrost-http/integrations/anthropic/types.go (3)
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#65
File: transports/bifrost-http/integrations/anthropic/types.go:131-180
Timestamp: 2025-06-10T13:11:37.842Z
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#63
File: transports/bifrost-http/integrations/openai/types.go:89-119
Timestamp: 2025-06-10T11:00:02.852Z
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#75
File: transports/bifrost-http/integrations/anthropic/types.go:154-160
Timestamp: 2025-06-14T04:19:06.305Z
Learning: Anthropic's API does not support images in system messages, only text content is allowed in system messages for Anthropic integrations.
🧬 Code Graph Analysis (3)
transports/bifrost-http/tracking/plugin.go (1)
core/schemas/bifrost.go (2)
  • BifrostRequest (59-69)
  • BifrostResponse (237-247)
transports/bifrost-http/integrations/genai/types.go (2)
core/schemas/bifrost.go (14)
  • BifrostMessage (148-156)
  • ToolCall (319-323)
  • ContentBlock (209-213)
  • ModelChatMessageRoleAssistant (28-28)
  • ContentBlockTypeText (205-205)
  • FunctionCall (313-316)
  • Function (106-110)
  • ModelChatMessageRoleTool (32-32)
  • MessageContent (158-161)
  • ToolMessage (216-218)
  • ContentBlockTypeImage (206-206)
  • ImageURLStruct (229-232)
  • ModelChatMessageRole (25-25)
  • AssistantMessage (221-226)
core/utils.go (1)
  • Ptr (5-7)
transports/bifrost-http/integrations/openai/types.go (1)
core/schemas/bifrost.go (6)
  • BifrostMessage (148-156)
  • Tool (113-117)
  • ToolChoice (142-145)
  • LogProbs (306-310)
  • BifrostResponseChoice (342-348)
  • LLMUsage (250-256)
🔇 Additional comments (7)
transports/bifrost-http/integrations/genai/types.go (2)

165-170: Correct handling of GenAI's "model" role.

The role mapping properly converts GenAI's internal "model" role to the standard "assistant" role, preventing invalid roles in Bifrost requests.


161-198: Proper message collection and assignment.

The implementation correctly:

  • Includes thought-only messages in the condition (line 162)
  • Appends messages to the slice (line 193)
  • Assigns the messages to ChatCompletionInput (line 197)

All the issues from previous reviews have been addressed.

transports/bifrost-http/integrations/openai/types.go (2)

9-27: Excellent simplification using shared Bifrost types.

The removal of custom OpenAI-specific types in favor of direct Bifrost schema usage improves maintainability and reduces code duplication.

Also applies to: 41-55


1-6:

Details

✅ Verification successful

Verify removal of unused fnTypePtr variable.

Past reviews indicated an unused fnTypePtr variable declaration. Ensure it has been removed if it's no longer needed.


🏁 Script executed:

#!/bin/bash
# Description: Check if fnTypePtr is declared but unused in the openai package

# Search for fnTypePtr declaration and usage in openai package
rg -A 2 -B 2 'fnTypePtr' transports/bifrost-http/integrations/openai/

Length of output: 69


🏁 Script executed:

#!/bin/bash
# Verify fnTypePtr is not present anywhere in the repository
rg 'fnTypePtr'

Length of output: 413


Removal of fnTypePtr confirmed in OpenAI integration

No occurrences of fnTypePtr found in transports/bifrost-http/integrations/openai/types.go, so the unused variable has been successfully removed.

transports/bifrost-http/integrations/anthropic/types.go (3)

95-132: Well-implemented custom JSON marshaling for flexible content.

The custom marshaling/unmarshaling logic correctly handles both string and array content formats with proper validation.


198-209: Correct image URL handling with proper data URL format.

Images are properly formatted with data:<mime>;base64, prefix, addressing previous feedback. The default MIME type of "image/png" is sensible.

Also applies to: 247-258


275-279: Proper role-based guard for assistant-specific fields.

Tool calls are correctly restricted to assistant messages only, preventing invalid message structures.

Comment thread transports/bifrost-http/integrations/genai/types.go
@Pratham-Mishra04 Pratham-Mishra04 force-pushed the 06-12-feat_http_transport_integrations_refactored_for_core_v1.1.0 branch from 90d624f to bcee867 Compare June 14, 2025 05:05
@Pratham-Mishra04 Pratham-Mishra04 changed the title feat: refactored integrations to support core/v1.1.0 feat: refactored integrations to support core/v1.1.2 Jun 14, 2025
@Pratham-Mishra04 Pratham-Mishra04 force-pushed the 06-12-feat_http_transport_integrations_refactored_for_core_v1.1.0 branch from bcee867 to 641078f Compare June 14, 2025 06:49
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

🔭 Outside diff range comments (2)
transports/bifrost-http/integrations/openai/types.go (1)

111-116: ⚠️ Potential issue

Tools / ToolChoice silently discarded

convertParameters returns nil whenever ExtraParams is empty, ignoring populated Tools or ToolChoice.
This drops tool definitions from the final Bifrost request.

-	// Return nil if no parameters were set
-	if len(params.ExtraParams) == 0 {
-		return nil
-	}
+	// Return nil only when *nothing* is populated
+	if len(params.ExtraParams) == 0 &&
+		params.ToolChoice == nil &&
+		params.Tools == nil {
+		return nil
+	}
transports/bifrost-http/integrations/genai/types.go (1)

41-48: 🧹 Nitpick (assertive)

Redundant initial slice allocation

ChatCompletionInput is first set to an address of an empty slice and later reassigned (line 197).
The first allocation is wasted work—drop it and set the field once after messages is fully built.

♻️ Duplicate comments (3)
transports/go.mod (1)

7-8: go mod tidy still required

Dependency bumps are in go.mod, but go.sum wasn’t updated. Run go mod tidy to sync sums & prune indirects.

transports/bifrost-http/integrations/genai/types.go (2)

385-393: Image blocks still discarded (acknowledged)

Only text blocks are re-hydrated; images remain ignored. Previous discussion concluded this is acceptable for Gemini assistant messages. No action needed.


468-509: Helper still keeps explicit image list

The fallback list after the "image/" prefix check is redundant; prefix alone suffices. Author previously opted to keep it – noting here for completeness.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between bcee867 and 641078f.

⛔ Files ignored due to path filters (1)
  • transports/go.sum is excluded by !**/*.sum
📒 Files selected for processing (5)
  • transports/bifrost-http/integrations/anthropic/types.go (8 hunks)
  • transports/bifrost-http/integrations/genai/types.go (3 hunks)
  • transports/bifrost-http/integrations/openai/types.go (4 hunks)
  • transports/bifrost-http/tracking/plugin.go (3 hunks)
  • transports/go.mod (1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
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.
transports/bifrost-http/integrations/genai/types.go (3)
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#75
File: transports/bifrost-http/integrations/genai/types.go:365-374
Timestamp: 2025-06-12T09:06:26.608Z
Learning: Gemini integration currently guarantees that assistant messages never contain image content blocks, so re-hydration code can safely ignore them.
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.
transports/bifrost-http/integrations/anthropic/types.go (3)
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#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#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.
🧬 Code Graph Analysis (3)
transports/bifrost-http/integrations/genai/types.go (2)
core/schemas/bifrost.go (14)
  • BifrostMessage (148-156)
  • ToolCall (319-323)
  • ContentBlock (209-213)
  • ModelChatMessageRoleAssistant (28-28)
  • ContentBlockTypeText (205-205)
  • FunctionCall (313-316)
  • Function (106-110)
  • ModelChatMessageRoleTool (32-32)
  • MessageContent (158-161)
  • ToolMessage (216-218)
  • ContentBlockTypeImage (206-206)
  • ImageURLStruct (229-232)
  • ModelChatMessageRole (25-25)
  • AssistantMessage (221-226)
core/utils.go (1)
  • Ptr (5-7)
transports/bifrost-http/tracking/plugin.go (1)
core/schemas/bifrost.go (3)
  • BifrostRequest (59-69)
  • BifrostResponse (237-247)
  • BifrostError (365-371)
transports/bifrost-http/integrations/openai/types.go (1)
core/schemas/bifrost.go (6)
  • BifrostMessage (148-156)
  • Tool (113-117)
  • ToolChoice (142-145)
  • LogProbs (306-310)
  • BifrostResponseChoice (342-348)
  • LLMUsage (250-256)

Comment thread transports/bifrost-http/tracking/plugin.go
Comment thread transports/bifrost-http/integrations/anthropic/types.go
Comment thread transports/bifrost-http/integrations/genai/types.go
Comment thread transports/bifrost-http/integrations/genai/types.go
@Pratham-Mishra04 Pratham-Mishra04 force-pushed the 06-12-feat_http_transport_integrations_refactored_for_core_v1.1.0 branch from 641078f to d10aa09 Compare June 14, 2025 08:54
@akshaydeo akshaydeo merged commit f230f29 into main Jun 14, 2025
2 checks passed
@akshaydeo akshaydeo deleted the 06-12-feat_http_transport_integrations_refactored_for_core_v1.1.0 branch August 31, 2025 17:31
akshaydeo added a commit that referenced this pull request Nov 17, 2025
# Anthropic API Integration Improvements

This PR enhances the Anthropic API integration by implementing a more flexible content handling system that properly supports both string and structured content formats. Key changes include:

- Renamed `AnthropicContent` to `AnthropicContentBlock` to better reflect its purpose
- Added a new `AnthropicContent` type that can handle both string content and arrays of content blocks
- Added proper support for system content blocks in Anthropic requests
- Implemented custom JSON marshaling/unmarshaling for content types to handle different formats
- Added response types for Anthropic API including usage information
- Enhanced tool schema handling with proper typing for input schemas
- Updated the content conversion logic between Bifrost and Anthropic formats

These changes align with Anthropic's API structure and improve the handling of different content formats (text, images, tool calls) in a more type-safe manner.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants