chore: execute goimports to format the code #1145
Conversation
📝 WalkthroughSummary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings. WalkthroughThis PR consists primarily of formatting and whitespace adjustments across test utilities, configuration structs, and infrastructure files. Minor control flow changes include conditional scoping of key retrieval in the governance handler and removal of implicit provider defaulting in the OpenAI batch integration. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (3 warnings)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (36)
💤 Files with no reviewable changes (15)
✅ Files skipped from review due to trivial changes (5)
🚧 Files skipped from review as they are similar to previous changes (12)
🧰 Additional context used📓 Path-based instructions (1)**⚙️ CodeRabbit configuration file
Files:
🧠 Learnings (1)📚 Learning: 2025-12-09T17:07:42.007ZApplied to files:
🧬 Code graph analysis (1)plugins/logging/main.go (3)
🔇 Additional comments (4)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Signed-off-by: luohewuyang <[email protected]>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
transports/bifrost-http/lib/config_test.go (1)
1-12261: Huge test helper + scenario file: consider future structuringUnrelated to this goimports run, but
config_test.gohas grown very large and dense. At some point it may be worth splitting into smaller files (e.g., provider hash tests, VK hash tests, SQLite integration helpers) to keep individual test files easier to navigate and reason about.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (37)
core/internal/testutil/automatic_function_calling.go(1 hunks)core/internal/testutil/complete_end_to_end.go(0 hunks)core/internal/testutil/embedding.go(0 hunks)core/internal/testutil/end_to_end_tool_calling.go(0 hunks)core/internal/testutil/image_base64.go(0 hunks)core/internal/testutil/image_url.go(0 hunks)core/internal/testutil/list_models.go(0 hunks)core/internal/testutil/multi_turn_conversation.go(1 hunks)core/internal/testutil/multiple_images.go(0 hunks)core/internal/testutil/multiple_tool_calls.go(0 hunks)core/internal/testutil/simple_chat.go(0 hunks)core/internal/testutil/text_completion.go(0 hunks)core/internal/testutil/text_completion_stream.go(0 hunks)core/internal/testutil/tool_calls.go(0 hunks)core/providers/elevenlabs/speech.go(1 hunks)core/providers/openai/types_test.go(0 hunks)core/providers/utils/pagination.go(0 hunks)core/schemas/pagination.go(0 hunks)framework/configstore/config.go(1 hunks)framework/configstore/rdb.go(1 hunks)framework/configstore/tables/budget.go(1 hunks)framework/configstore/tables/clientconfig.go(2 hunks)framework/configstore/tables/config.go(1 hunks)framework/configstore/tables/key.go(1 hunks)framework/configstore/tables/mcp.go(1 hunks)framework/encrypt/encrypt_test.go(1 hunks)plugins/jsonparser/main.go(3 hunks)plugins/logging/main.go(1 hunks)plugins/semanticcache/main.go(1 hunks)plugins/semanticcache/plugin_integration_test.go(3 hunks)plugins/semanticcache/search.go(1 hunks)plugins/semanticcache/stream.go(1 hunks)transports/bifrost-http/handlers/governance.go(3 hunks)transports/bifrost-http/handlers/middlewares_test.go(1 hunks)transports/bifrost-http/integrations/bedrock_test.go(1 hunks)transports/bifrost-http/integrations/openai.go(2 hunks)transports/bifrost-http/lib/config_test.go(41 hunks)
💤 Files with no reviewable changes (15)
- core/internal/testutil/text_completion_stream.go
- core/internal/testutil/text_completion.go
- core/internal/testutil/complete_end_to_end.go
- core/internal/testutil/end_to_end_tool_calling.go
- core/schemas/pagination.go
- core/internal/testutil/simple_chat.go
- core/internal/testutil/multiple_images.go
- core/internal/testutil/list_models.go
- core/internal/testutil/image_url.go
- core/internal/testutil/multiple_tool_calls.go
- core/internal/testutil/embedding.go
- core/providers/openai/types_test.go
- core/internal/testutil/tool_calls.go
- core/internal/testutil/image_base64.go
- core/providers/utils/pagination.go
🧰 Additional context used
📓 Path-based instructions (1)
**
⚙️ CodeRabbit configuration file
always check the stack if there is one for the current PR. do not give localized reviews for the PR, always see all changes in the light of the whole stack of PRs (if there is a stack, if there is no stack you can continue to make localized suggestions/reviews)
Files:
framework/encrypt/encrypt_test.gotransports/bifrost-http/handlers/middlewares_test.gotransports/bifrost-http/integrations/bedrock_test.goframework/configstore/tables/clientconfig.goframework/configstore/rdb.gocore/internal/testutil/multi_turn_conversation.goplugins/semanticcache/plugin_integration_test.goframework/configstore/tables/mcp.goplugins/jsonparser/main.goplugins/logging/main.gotransports/bifrost-http/lib/config_test.gocore/providers/elevenlabs/speech.goframework/configstore/config.goplugins/semanticcache/main.goframework/configstore/tables/key.goplugins/semanticcache/search.goplugins/semanticcache/stream.gotransports/bifrost-http/handlers/governance.goframework/configstore/tables/config.gocore/internal/testutil/automatic_function_calling.goframework/configstore/tables/budget.gotransports/bifrost-http/integrations/openai.go
🧠 Learnings (4)
📚 Learning: 2025-12-09T17:07:42.007Z
Learnt from: qwerty-dvorak
Repo: maximhq/bifrost PR: 1006
File: core/schemas/account.go:9-18
Timestamp: 2025-12-09T17:07:42.007Z
Learning: In core/schemas/account.go, the HuggingFaceKeyConfig field within the Key struct is currently unused and reserved for future Hugging Face inference endpoint deployments. Do not flag this field as missing from OpenAPI documentation or require its presence in the API spec until the feature is actively implemented and used. When the feature is added, update the OpenAPI docs accordingly; otherwise, treat this field as non-breaking and not part of the current API surface.
Applied to files:
framework/encrypt/encrypt_test.gotransports/bifrost-http/handlers/middlewares_test.gotransports/bifrost-http/integrations/bedrock_test.goframework/configstore/tables/clientconfig.goframework/configstore/rdb.gocore/internal/testutil/multi_turn_conversation.goplugins/semanticcache/plugin_integration_test.goframework/configstore/tables/mcp.goplugins/jsonparser/main.goplugins/logging/main.gotransports/bifrost-http/lib/config_test.gocore/providers/elevenlabs/speech.goframework/configstore/config.goplugins/semanticcache/main.goframework/configstore/tables/key.goplugins/semanticcache/search.goplugins/semanticcache/stream.gotransports/bifrost-http/handlers/governance.goframework/configstore/tables/config.gocore/internal/testutil/automatic_function_calling.goframework/configstore/tables/budget.gotransports/bifrost-http/integrations/openai.go
📚 Learning: 2025-12-12T08:25:02.629Z
Learnt from: Pratham-Mishra04
Repo: maximhq/bifrost PR: 1000
File: transports/bifrost-http/integrations/router.go:709-712
Timestamp: 2025-12-12T08:25:02.629Z
Learning: In transports/bifrost-http/**/*.go, update streaming response handling to align with OpenAI Responses API: use typed SSE events such as response.created, response.output_text.delta, response.done, etc., and do not rely on the legacy data: [DONE] termination marker. Note that data: [DONE] is only used by the older Chat Completions and Text Completions streaming APIs. Ensure parsers, writers, and tests distinguish SSE events from the [DONE] sentinel and handle each event type accordingly for correct stream termination and progress updates.
Applied to files:
transports/bifrost-http/handlers/middlewares_test.gotransports/bifrost-http/integrations/bedrock_test.gotransports/bifrost-http/lib/config_test.gotransports/bifrost-http/handlers/governance.gotransports/bifrost-http/integrations/openai.go
📚 Learning: 2025-12-19T08:29:20.286Z
Learnt from: qwerty-dvorak
Repo: maximhq/bifrost PR: 1095
File: core/internal/testutil/count_tokens.go:30-67
Timestamp: 2025-12-19T08:29:20.286Z
Learning: In core/internal/testutil test files, enforce using GetTestRetryConfigForScenario() to obtain a generic retry config, then construct a typed retry config (e.g., CountTokensRetryConfig, EmbeddingRetryConfig, TranscriptionRetryConfig) with an empty Conditions slice. Copy only MaxAttempts, BaseDelay, MaxDelay, OnRetry, and OnFinalFail from the generic config. This convention should be consistently applied across all test files in this directory.
Applied to files:
core/internal/testutil/multi_turn_conversation.gocore/internal/testutil/automatic_function_calling.go
📚 Learning: 2025-12-19T09:26:54.961Z
Learnt from: qwerty-dvorak
Repo: maximhq/bifrost PR: 1006
File: core/providers/utils/utils.go:1050-1051
Timestamp: 2025-12-19T09:26:54.961Z
Learning: Update streaming end-marker handling so HuggingFace is treated as a non-[DONE] provider for backends that do not emit a DONE marker (e.g., meta llama on novita). In core/providers/utils/utils.go, adjust ProviderSendsDoneMarker() (or related logic) to detect providers that may not emit DONE and avoid relying on DONE as the sole end signal. Add tests to cover both DONE-emitting and non-DONE backends, with clear documentation in code comments explaining the rationale and any fallback behavior.
Applied to files:
core/providers/elevenlabs/speech.go
🧬 Code graph analysis (9)
transports/bifrost-http/handlers/middlewares_test.go (1)
core/schemas/logger.go (1)
LogLevel(6-6)
transports/bifrost-http/integrations/bedrock_test.go (1)
core/schemas/bifrost.go (1)
ModelProvider(32-32)
core/internal/testutil/multi_turn_conversation.go (2)
core/internal/testutil/test_retry_framework.go (1)
WithChatTestRetry(274-424)core/internal/testutil/utils.go (2)
GetErrorMessage(642-675)GetChatContent(365-392)
plugins/semanticcache/plugin_integration_test.go (1)
core/schemas/context.go (2)
NewBifrostContext(46-64)NoDeadline(10-10)
plugins/logging/main.go (3)
ui/lib/types/logs.ts (1)
TranscriptionInput(15-17)core/schemas/bifrost.go (1)
TranscriptionRequest(101-101)transports/bifrost-http/handlers/inference.go (1)
TranscriptionRequest(285-289)
transports/bifrost-http/lib/config_test.go (2)
core/schemas/logger.go (1)
LogLevel(6-6)core/schemas/account.go (1)
Key(8-20)
transports/bifrost-http/handlers/governance.go (2)
framework/configstore/tables/key.go (2)
TableKey(13-58)TableKey(61-61)framework/configstore/tables/virtualkey.go (2)
TableVirtualKeyProviderConfig(23-36)TableVirtualKeyProviderConfig(39-41)
framework/configstore/tables/config.go (4)
core/providers/gemini/types.go (1)
Type(782-782)ui/lib/types/config.ts (1)
GlobalProxyType(267-267)core/network/http.go (1)
GlobalProxyType(46-46)core/schemas/provider.go (1)
NoProxy(147-147)
framework/configstore/tables/budget.go (1)
framework/configstore/tables/utils.go (1)
ParseDuration(9-43)
🔇 Additional comments (29)
core/providers/elevenlabs/speech.go (1)
89-89: Formatting change approved.The addition of the EOF newline is part of the standard goimports formatting and does not alter any functional behavior.
plugins/semanticcache/stream.go (1)
74-74: Formatting changes look good.The whitespace normalization and defer statement organization are purely cosmetic and don't affect the control flow or resource cleanup semantics. The defer ordering remains correct: cleanup executes before the mutex unlock due to LIFO execution.
Also applies to: 78-78
core/internal/testutil/multi_turn_conversation.go (1)
135-147: LGTM!The indentation adjustments in the second test step are properly formatted. The code structure, error handling, and retry config construction follow the established patterns in the testutil suite. The ChatRetryConfig construction at lines 120-127 correctly implements the learning guideline by using
GetTestRetryConfigForScenario()and copying only the required fields with an empty Conditions slice.transports/bifrost-http/handlers/middlewares_test.go (1)
15-20: Formatting changes applied cleanly.The mockLogger method signatures have been aligned via goimports formatting. All method parameters and return types remain correct and consistent with the
schemas.Loggerinterface contract.framework/encrypt/encrypt_test.go (1)
1-245: ✓ Formatting change approved.The blank line addition at line 219 improves readability by creating visual separation between the re-initialization step and the subsequent decryption operation—a standard Go formatting convention. The test file maintains excellent structure with comprehensive coverage across encryption/decryption, error handling, KDF behavior with various key lengths, and deterministic key derivation. No functional changes or behavioral modifications introduced.
core/internal/testutil/automatic_function_calling.go (1)
165-165: LGTM!The blank line addition between the comment block and the for-loop improves readability and aligns with the goimports formatting objective. No functional changes to the validation logic.
transports/bifrost-http/lib/config_test.go (5)
615-625: testLogger methods: formatting-only change is safeThe
testLoggermethods remain no‑ops with identical signatures; only spacing/alignment changed. No impact on test behavior.
1823-1837: Provider key merge condition remains logically correctThe condition
if dbKeyHash == fileKeyHash || fileKey.Name == dbKey.Namestill implements “same key by content or name” semantics when deciding whether to treat DB/file keys as equivalent. Only formatting changed; behavior is preserved.
2189-2198: Hash variants map: alignment-only changesThe
hashes := map[string]string{...}literal inTestProviderHashComparison_OptionalFieldsPresenceis unchanged semantically; keys and values are identical with only indentation/spacing adjusted. Collision detection logic is unaffected.
4743-4748: Rewrapped t.Errorf in MCP hash parity testsThe long
t.Errorfcall was split across lines for readability, but the format string and arguments are unchanged. Failure diagnostics and test logic remain identical.
6710-6727: Rewrapped t.Errorf in SQLite full‑reconciliation testsThe multi-line
t.Errorfnow has cleaner line breaks while preserving the format string and both hash arguments. No behavioral change in the reconciliation tests.plugins/logging/main.go (1)
275-275: LGTM! Formatting adjustment improves consistency.The TranscriptionInput assignment has been reformatted for consistency with the rest of the codebase. No functional changes.
plugins/semanticcache/main.go (1)
380-380: LGTM! Whitespace formatting improvement.The extra blank line improves readability by separating the context value assignments from the subsequent logic. No functional changes.
plugins/semanticcache/search.go (1)
300-300: LGTM! Whitespace formatting improvement.The additional blank line improves code readability without affecting functionality.
plugins/semanticcache/plugin_integration_test.go (1)
21-21: LGTM! Test formatting improvements.The whitespace adjustments in test cases improve consistency without affecting test behavior or assertions.
Also applies to: 312-312, 550-550
framework/configstore/rdb.go (1)
668-668: LGTM! Brace formatting improvement.The closing brace positioning has been adjusted for consistency with Go formatting standards. No functional changes.
transports/bifrost-http/integrations/bedrock_test.go (1)
533-538: LGTM! Test table formatting improvement.The struct field alignment in the test table improves readability. Field names, types, and test logic remain unchanged.
framework/configstore/tables/key.go (1)
42-48: LGTM! Field alignment improvement.The Bedrock field declarations have been realigned for consistency. Field names, types, JSON tags, and GORM tags remain unchanged.
framework/configstore/config.go (1)
16-17: LGTM! Constant alignment improvement.The ConfigStoreType constants have been aligned for better readability. Constant names, types, and values remain unchanged.
framework/configstore/tables/mcp.go (1)
13-20: LGTM!Pure formatting change - struct field alignment for readability. No semantic changes to types, names, or tags.
plugins/jsonparser/main.go (1)
92-92: LGTM!Documentation formatting improvements - adding blank comment lines between parameter and return sections for better godoc readability.
Also applies to: 105-105, 119-119
framework/configstore/tables/config.go (2)
15-22: LGTM!Pure formatting change - struct field alignment for readability.
24-26: No action required. TheEnableForAPIfield already exists in the codebase and is actively in use. This is a formatting/alignment change, not an introduction of a new field. No database migrations are needed.framework/configstore/tables/budget.go (1)
30-36: LGTM!Pure formatting changes to the
BeforeSavemethod signature and the conditional block. The validation logic remains unchanged.transports/bifrost-http/integrations/openai.go (2)
498-503: LGTM - Formatting only; AI summary is inconsistent.The AI summary incorrectly states that "the defaulting behavior for an empty provider in the PreCallback is changed" and "the assignment is now removed."
However, examining the actual code, the default provider assignment to
schemas.OpenAIis still present at lines 501-502. Only line 503 (the closing brace) is marked as changed, which is a pure formatting adjustment.
1203-1205: LGTM!Pure formatting change - closing brace alignment in the purpose field validation block.
framework/configstore/tables/clientconfig.go (1)
18-19: LGTM!The formatting changes are appropriate for a goimports run. Comment and tag alignment improve readability without changing any semantics.
Also applies to: 36-36
transports/bifrost-http/handlers/governance.go (2)
591-611: Consistent restructuring for new provider configs.The keys retrieval follows the same pattern as in
createVirtualKey(lines 288-299), ensuring consistency. The logic correctly handles the case where no KeyIDs are provided by using an empty slice.
651-674: Consistent restructuring for existing provider configs.The keys retrieval and assignment for existing provider configs follows the same pattern as the new config creation paths. This consistency is good for maintainability.
| // Get keys for this provider config if specified | ||
| var keys []configstoreTables.TableKey | ||
| if len(pc.KeyIDs) > 0 { | ||
| var err error | ||
| keys, err = h.configStore.GetKeysByIDs(ctx, pc.KeyIDs) | ||
| if err != nil { | ||
| return fmt.Errorf("failed to get keys by IDs for provider %s: %w", pc.Provider, err) | ||
| } | ||
| if len(keys) != len(pc.KeyIDs) { | ||
| return fmt.Errorf("some keys not found for provider %s: expected %d, found %d", pc.Provider, len(pc.KeyIDs), len(keys)) | ||
| } | ||
| } | ||
| } | ||
|
|
||
| providerConfig := &configstoreTables.TableVirtualKeyProviderConfig{ | ||
| VirtualKeyID: vk.ID, | ||
| Provider: pc.Provider, | ||
| Weight: pc.Weight, | ||
| AllowedModels: pc.AllowedModels, | ||
| Keys: keys, | ||
| } | ||
| providerConfig := &configstoreTables.TableVirtualKeyProviderConfig{ | ||
| VirtualKeyID: vk.ID, | ||
| Provider: pc.Provider, | ||
| Weight: pc.Weight, | ||
| AllowedModels: pc.AllowedModels, | ||
| Keys: keys, | ||
| } |
There was a problem hiding this comment.
Logic restructuring in a formatting PR.
The keys retrieval logic is now properly guarded by the len(pc.KeyIDs) > 0 conditional, and the provider config construction follows afterward. This appears correct—when no KeyIDs are provided, an empty keys slice is used (which is valid per the schema).
However, this is a structural change, not just formatting. The PR title indicates this should be a pure goimports run, which typically only handles import organization and code formatting, not logic restructuring.
🤖 Prompt for AI Agents
In transports/bifrost-http/handlers/governance.go around lines 288 to 307, this
change moved the keys retrieval and conditional logic which alters program
behaviour; the PR should be a pure formatting/goimports change only. Revert the
logic restructuring so the previous behavior is restored (undo the
moved/rewritten key-fetch block), ensure only whitespace/imports/formatting
changes remain, then run goimports to reformat and stage just those formatting
changes.
1 participant
Summary
execute goimports to format the code
Changes
Type of change
Affected areas
How to test
Describe the steps to validate this change. Include commands and expected outcomes.
If adding new configs or environment variables, document them here.
Screenshots/Recordings
If UI changes, add before/after screenshots or short clips.
Breaking changes
If yes, describe impact and migration instructions.
Related issues
Link related issues and discussions. Example: Closes #123
Security considerations
Note any security implications (auth, secrets, PII, sandboxing, etc.).
Checklist
docs/contributing/README.mdand followed the guidelines