-
Notifications
You must be signed in to change notification settings - Fork 61
chore: update maxim's plugin bifrost/core dependency to v1.1.0 #76
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
chore: update maxim's plugin bifrost/core dependency to v1.1.0 #76
Conversation
Summary by CodeRabbit
Summary by CodeRabbit
WalkthroughThe changes update the Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Plugin
participant Logger
Client->>Plugin: PreHook(ctx, req)
Plugin->>Logger: Log message content
alt Generation ID exists
Plugin-->>Client: Return early (req, nil, nil)
else
Plugin-->>Client: Return (req, nil, nil)
end
Client->>Plugin: Cleanup()
Plugin->>Logger: Flush()
Plugin-->>Client: nil
Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (3)
🧰 Additional context used🧠 Learnings (1)plugins/maxim/plugin_test.go (1)
🧬 Code Graph Analysis (2)plugins/maxim/plugin_test.go (2)
plugins/maxim/main.go (2)
🔇 Additional comments (1)
✨ 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 (
|
This stack of pull requests is managed by Graphite. Learn more about stacking. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
🔭 Outside diff range comments (1)
plugins/maxim/main.go (1)
99-106
: 🧹 Nitpick (assertive)Doc-comment is outdated after signature change
The
PreHook
comment still claims the function returns “*BifrostRequest, error”.
Update the description to mention the additional*BifrostResponse
return so thatgo doc
stays accurate.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
plugins/maxim/go.sum
is excluded by!**/*.sum
📒 Files selected for processing (3)
plugins/maxim/go.mod
(1 hunks)plugins/maxim/main.go
(4 hunks)plugins/maxim/plugin_test.go
(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
plugins/maxim/plugin_test.go (1)
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.
🧬 Code Graph Analysis (2)
plugins/maxim/plugin_test.go (2)
core/schemas/bifrost.go (1)
MessageContent
(158-161)core/utils.go (1)
Ptr
(5-7)
plugins/maxim/main.go (2)
core/schemas/bifrost.go (2)
BifrostRequest
(59-69)BifrostResponse
(237-247)transports/bifrost-http/main.go (1)
CompletionRequest
(111-118)
🔇 Additional comments (1)
plugins/maxim/main.go (1)
140-144
: Verify type expected bylogging.CompletionRequest.Content
message.Content
is aschemas.MessageContent
struct.
Iflogging.CompletionRequest.Content
still expects astring
, this will not compile after the schema upgrade.
Please rungo test ./...
orgo vet ./...
to confirm the interface matches.
If a string is required, pass*message.Content.ContentStr
instead (after nil-check).
1c13e3e
to
d81b8db
Compare
Merge activity
|
Update Bifrost Core Dependency to v1.1.0
This PR updates the Maxim plugin to work with Bifrost Core v1.1.0. The changes include:
These changes ensure compatibility with the latest Bifrost Core while maintaining the plugin's functionality for tracing and logging requests.