feat: add Redis cache plugin for Bifrost requests #70
Conversation
|
Note Currently processing new changes in this PR. This may take a few minutes, please wait... ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (4)
Summary by CodeRabbit
WalkthroughAdds a Redis caching plugin for Bifrost: new implementation, module manifest, README, and integration tests. It hashes normalized requests with xxhash, checks/stores responses in Redis with TTL, supports streaming, async writes, cache management, and graceful degradation when Redis is unavailable. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Bifrost
participant RedisPlugin
participant Redis
Client->>Bifrost: Send request
Bifrost->>RedisPlugin: PreHook(req)
RedisPlugin->>Redis: GET (prefix + xxhash(normalized req))
alt cache hit (non-stream)
Redis-->>RedisPlugin: cached response
RedisPlugin-->>Bifrost: short-circuit response (marked cached)
else cache hit (streaming)
Redis-->>RedisPlugin: multiple chunk keys
RedisPlugin-->>Bifrost: stream cached chunks
else cache miss
Redis-->>RedisPlugin: not found
RedisPlugin-->>Bifrost: continue processing
Bifrost->>RedisPlugin: PostHook(response)
RedisPlugin->>Redis: SET (prefix + xxhash) = response (async, with TTL)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~35 minutes Poem
Tip You can disable sequence diagrams in the walkthrough.Disable the ✨ 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. 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. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
plugins/redis/go.sumis excluded by!**/*.sum
📒 Files selected for processing (4)
plugins/redis/README.md(1 hunks)plugins/redis/go.mod(1 hunks)plugins/redis/main.go(1 hunks)plugins/redis/plugin_test.go(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
plugins/redis/main.go (3)
core/schemas/logger.go (1)
Logger(18-35)core/schemas/bifrost.go (5)
BifrostRequest(54-64)ModelProvider(31-31)RequestInput(46-49)ModelParameters(75-89)BifrostResponse(191-201)core/schemas/provider.go (1)
Provider(142-149)
🪛 LanguageTool
plugins/redis/README.md
[uncategorized] ~88-~88: Possible missing comma found.
Context: ...nerates a SHA256 hash of the normalized request including: - Provider - Model - Input ...
(AI_HYDRA_LEO_MISSING_COMMA)
🔇 Additional comments (2)
plugins/redis/plugin_test.go (1)
1-156: Well-structured integration test!The test provides comprehensive coverage of the Redis plugin functionality including cache misses, hits, cache info retrieval, and cache clearing. Proper error handling with test skipping when Redis is unavailable is implemented correctly.
plugins/redis/main.go (1)
156-164: 🛠️ Refactor suggestionAdd type assertion safety check.
The code assumes
RawResponseis a map, but this could cause a panic if it's a different type.// Mark response as cached in extra fields if cachedResponse.ExtraFields.RawResponse == nil { cachedResponse.ExtraFields.RawResponse = make(map[string]interface{}) } if rawResponseMap, ok := cachedResponse.ExtraFields.RawResponse.(map[string]interface{}); ok { rawResponseMap["bifrost_cached"] = true rawResponseMap["bifrost_cache_key"] = hash +} else { + // If RawResponse is not a map, create a new one + cachedResponse.ExtraFields.RawResponse = map[string]interface{}{ + "bifrost_cached": true, + "bifrost_cache_key": hash, + } }Likely an incorrect or invalid review comment.
Pratham-Mishra04
force-pushed
the
06-11-_plugins_feat_redis_caching_plugin_added
branch
from
c5d293d to
77ad98a
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (3)
plugins/redis/go.mod (1)
3-3:⚠️ Potential issueInvalid
godirective still present
go 1.24.1is not a valid version string – only the major & minor are allowed (e.g.1.24). This was flagged in a previous review and remains unfixed.-go 1.24.1 +go 1.24plugins/redis/README.md (1)
68-72:⚠️ Potential issueWrong error check – logs cache info only when an error occurs
The example still uses
if err != nilinstead oferr == nil, so it prints cache info on failure and does nothing on success.-if err != nil { +if err == nil { log.Printf("Cache info: %+v", info) }plugins/redis/main.go (1)
41-66: 🛠️ Refactor suggestionPing failure leaks the Redis client
If the ping fails,
client.Close()is never called. Close the client before returning:_, err := client.Ping(ctx).Result() if err != nil { + client.Close() return nil, fmt.Errorf("failed to ping Redis: %w", err) }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
plugins/redis/go.sumis excluded by!**/*.sum
📒 Files selected for processing (4)
plugins/redis/README.md(1 hunks)plugins/redis/go.mod(1 hunks)plugins/redis/main.go(1 hunks)plugins/redis/plugin_test.go(1 hunks)
🧰 Additional context used
🪛 LanguageTool
plugins/redis/README.md
[uncategorized] ~88-~88: Possible missing comma found.
Context: ...nerates a SHA256 hash of the normalized request including: - Provider - Model - Input ...
(AI_HYDRA_LEO_MISSING_COMMA)
🪛 golangci-lint (1.64.8)
plugins/redis/main.go
1-1: : # github.com/maximhq/bifrost/plugins/redis [github.com/maximhq/bifrost/plugins/redis.test]
./main.go:61:9: cannot use &Plugin{…} (value of type *Plugin) as schemas.Plugin value in return statement: Plugin does not implement schemas.Plugin (wrong type for method PreHook)
have PreHook("context".Context, *schemas.BifrostRequest) (*schemas.BifrostRequest, schemas.BifrostResponse, error)
want PreHook("context".Context, *schemas.BifrostRequest) (*schemas.BifrostRequest, error)
(typecheck)
🔇 Additional comments (1)
plugins/redis/plugin_test.go (1)
88-96:❓ Verification inconclusive
Unit-style test depends on live OpenAI – mark as integration
The test reaches out to the real OpenAI endpoint, introducing flakiness, latency and cost. Consider:
- Moving it behind a build tag
//go:build integration- Replacing the provider with a stub/mocked Bifrost provider.
This will keep the default
go test ./...run fast and offline.
🏁 Script executed:
#!/bin/bash set -e # Locate the plugin_test.go file file=$(fd plugin_test.go 2>/dev/null | head -n1) echo "Inspecting file: $file" # Print the first 200 lines for context echo "=== File content preview ===" sed -n '1,200p' "$file" # Check for any references to OpenAI or external HTTP clients echo "=== Searching for 'OpenAI' references ===" grep -R "OpenAI" -n "$file" || echo "No OpenAI references found." echo "=== Searching for HTTP usage ===" grep -RE "(http\.|resty|net/http)" -n "$file" || echo "No HTTP-related imports or usage found."Length of output: 5239
🏁 Script executed:
#!/bin/bash set -e echo "=== Files under plugins directory ===" fd -t f . plugins echo "=== Directories under plugins containing 'redis' ===" fd -t d *redis* plugins || echo "No 'redis' directory found under plugins" echo "=== plugin_test.go files under plugins ===" fd plugin_test.go plugins || echo "No plugin_test.go found under plugins"Length of output: 1061
Mark Redis Plugin Test as Integration
Theplugins/redis/plugin_test.gotest exercises a real OpenAI-backed chat completion, which introduces external dependencies, latency, cost, and potential flakiness. To keepgo test ./...fast and offline, please:• Add a build tag at the top of
plugins/redis/plugin_test.go://go:build integration // +build integration• Split out—or rename—the test (e.g.
TestIntegrationRedisPlugin) so it only runs when explicitly included (go test -tags=integration ./plugins/redis).
• For pure unit tests, inject a fake Bifrost provider or mock the ChatCompletionRequest response instead of calling OpenAI.fix_required
Pratham-Mishra04
force-pushed
the
06-11-chore_plugin_support_extensions
branch
from
8b88c85 to
58dfdd3
Compare
Pratham-Mishra04
force-pushed
the
06-11-_plugins_feat_redis_caching_plugin_added
branch
from
77ad98a to
6b9055a
Compare
Pratham-Mishra04
changed the base branch from
06-11-chore_plugin_support_extensions
to
graphite-base/70
Pratham-Mishra04
force-pushed
the
06-11-_plugins_feat_redis_caching_plugin_added
branch
from
6b9055a to
8f75546
Compare
Pratham-Mishra04
force-pushed
the
graphite-base/70
branch
from
58dfdd3 to
bc74f7d
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (7)
plugins/redis/go.mod (2)
3-3:⚠️ Potential issueInvalid Go version
1.24.1– breaksgotooling
godirective must be1.<major>(e.g.1.24).-go 1.24.1 +go 1.24
5-35: 🛠️ Refactor suggestionMissing
replacedirective & untidied indirect block
The local source is under../../coreyet noreplaceis present and the file still contains a hand-maintained indirect list. Add the replace and rungo mod tidyto prune the indirect section.plugins/redis/README.md (1)
69-72: 🧹 Nitpick (assertive)Error-handling example still inverted
The cache-info log should run only on success:-if err != nil { +if err == nil { log.Printf("Cache info: %+v", info) }plugins/redis/plugin_test.go (1)
55-60: 🛠️ Refactor suggestionAdd guard for missing
OPENAI_API_KEYto avoid external calls in CIif os.Getenv("OPENAI_API_KEY") == "" { t.Skip("OPENAI_API_KEY not set – skipping integration test") }Without this the test dials OpenAI and flaps in sandboxed pipelines.
plugins/redis/main.go (3)
52-60: 🛠️ Refactor suggestionPotential resource leak on failed Redis ping
clientis never closed on the error path._, err := client.Ping(ctx).Result() if err != nil { + _ = client.Close() return nil, fmt.Errorf("failed to ping Redis: %w", err) }
113-120:⚠️ Potential issue
PreHooksignature does not satisfyschemas.Plugin– compilation fails
Interface expectsPreHook(*context.Context, *schemas.BifrostRequest) (*schemas.BifrostRequest, error).-func (p *Plugin) PreHook(ctx *context.Context, req *schemas.BifrostRequest) (*schemas.BifrostRequest, *schemas.BifrostResponse, error) { +func (p *Plugin) PreHook(ctx *context.Context, req *schemas.BifrostRequest) (*schemas.BifrostRequest, error) {Move the cached response into the context (or a custom value store) so downstream layers can short-circuit.
175-187: 🧹 Nitpick (assertive)Log level noise – expected paths shouldn’t be
Warn
Missing hash or non-string hash is normal; useDebugto avoid diluting real warnings.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
plugins/redis/go.sumis excluded by!**/*.sum
📒 Files selected for processing (4)
plugins/redis/README.md(1 hunks)plugins/redis/go.mod(1 hunks)plugins/redis/main.go(1 hunks)plugins/redis/plugin_test.go(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
plugins/redis/main.go (3)
core/schemas/logger.go (1)
Logger(18-35)core/schemas/bifrost.go (5)
BifrostRequest(54-64)ModelProvider(31-31)RequestInput(46-49)ModelParameters(75-89)BifrostResponse(191-201)core/schemas/provider.go (1)
Provider(142-149)
🪛 LanguageTool
plugins/redis/README.md
[uncategorized] ~88-~88: Possible missing comma found.
Context: ...nerates a SHA256 hash of the normalized request including: - Provider - Model - Input ...
(AI_HYDRA_LEO_MISSING_COMMA)
🪛 golangci-lint (1.64.8)
plugins/redis/main.go
1-1: : # github.com/maximhq/bifrost/plugins/redis [github.com/maximhq/bifrost/plugins/redis.test]
./main.go:61:9: cannot use &Plugin{…} (value of type *Plugin) as schemas.Plugin value in return statement: Plugin does not implement schemas.Plugin (wrong type for method PreHook)
have PreHook("context".Context, *schemas.BifrostRequest) (*schemas.BifrostRequest, schemas.BifrostResponse, error)
want PreHook("context".Context, *schemas.BifrostRequest) (*schemas.BifrostRequest, error)
(typecheck)
Pratham-Mishra04
force-pushed
the
06-11-_plugins_feat_redis_caching_plugin_added
branch
from
8f75546 to
0f1e5c1
Compare
Pratham-Mishra04
force-pushed
the
06-11-_plugins_feat_redis_caching_plugin_added
branch
from
0f1e5c1 to
c53c602
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (4)
plugins/redis/go.mod (2)
3-3: Fix invalid Go version specification.Go version
1.24.1is invalid. Thegodirective only accepts major.minor format (e.g.,go 1.24), not major.minor.patch.Apply this diff:
-go 1.24.1 +go 1.24
1-39: Add missing replace directive and clean up go.mod.The module file needs a replace directive for local development and should not maintain indirect dependencies manually.
- Add replace directive after the module declaration:
module github.com/maximhq/bifrost/plugins/redis +replace github.com/maximhq/bifrost/core => ../../core + go 1.24
- Remove the manual indirect require block (lines 11-39) and run:
cd plugins/redis go mod tidyThis will automatically manage indirect dependencies and ensure local builds use your local core code.
plugins/redis/plugin_test.go (1)
46-61: Skip test when OpenAI API key is unavailable.The test should check for the OpenAI API key before attempting to connect to Redis to avoid unnecessary failures in CI environments.
Add this check at the beginning of the test:
func TestRedisPlugin(t *testing.T) { + // Check for OpenAI API key first + if os.Getenv("OPENAI_API_KEY") == "" { + t.Skip("OPENAI_API_KEY not set - skipping integration test") + } + // Configure plugin with minimal Redis connection settings (only Addr is required) config := RedisPluginConfig{plugins/redis/main.go (1)
364-385: Use SCAN instead of KEYS for better performance.The
KEYScommand can block Redis on large datasets. UseSCANfor production-safe operation.Replace the Cleanup method with:
func (plugin *Plugin) Cleanup() error { ctx := context.Background() pattern := plugin.config.Prefix + "*" var cursor uint64 var allKeys []string // Scan for keys in batches for { var keys []string var err error keys, cursor, err = plugin.client.Scan(ctx, cursor, pattern, 100).Result() if err != nil { return fmt.Errorf("failed to scan cache keys: %w", err) } allKeys = append(allKeys, keys...) if cursor == 0 { break } } // Delete keys in batches if len(allKeys) > 0 { batchSize := 1000 for i := 0; i < len(allKeys); i += batchSize { end := i + batchSize if end > len(allKeys) { end = len(allKeys) } if err := plugin.client.Del(ctx, allKeys[i:end]...).Err(); err != nil { return fmt.Errorf("failed to delete cache keys: %w", err) } } plugin.logger.Info(fmt.Sprintf("%s Cleaned up %d cache entries", PluginLoggerPrefix, len(allKeys))) } err := plugin.client.Close() if err != nil { return fmt.Errorf("failed to close Redis client: %w", err) } plugin.logger.Info(PluginLoggerPrefix + " Successfully closed Redis connection") return nil }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
plugins/redis/go.sumis excluded by!**/*.sum
📒 Files selected for processing (4)
plugins/redis/README.md(1 hunks)plugins/redis/go.mod(1 hunks)plugins/redis/main.go(1 hunks)plugins/redis/plugin_test.go(1 hunks)
🧰 Additional context used
🪛 LanguageTool
plugins/redis/README.md
[uncategorized] ~132-~132: Loose punctuation mark.
Context: ...defaults for zero values): - PoolSize, MinIdleConns, MaxIdleConns - Connec...
(UNLIKELY_OPENING_PUNCTUATION)
[misspelling] ~140-~140: Use “a” instead of ‘an’ if the following word doesn’t start with a vowel sound, e.g. ‘a sentence’, ‘a university’.
Context: ... ## How It Works The plugin generates an xxhash of the normalized request includ...
(EN_A_VS_AN)
[uncategorized] ~140-~140: Possible missing comma found.
Context: ...n generates an xxhash of the normalized request including: - Provider (if CacheByProvi...
(AI_HYDRA_LEO_MISSING_COMMA)
🔇 Additional comments (1)
plugins/redis/README.md (1)
130-136: Minor formatting improvement for readability.Remove the loose punctuation mark for better formatting.
-**Redis Connection Options** (all optional, Redis client uses its own defaults for zero values): - -- `PoolSize`, `MinIdleConns`, `MaxIdleConns` - Connection pool settings +**Redis Connection Options** (all optional, Redis client uses its own defaults for zero values): +- `PoolSize`, `MinIdleConns`, `MaxIdleConns` - Connection pool settingsLikely an incorrect or invalid review comment.
Pratham-Mishra04
force-pushed
the
06-11-_plugins_feat_redis_caching_plugin_added
branch
from
c53c602 to
601ae22
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (6)
plugins/redis/go.mod (2)
3-3: Invalidgodirective (1.24.1).The directive must be major.minor only, e.g.
go 1.24.
Same issue was flagged earlier and is still unresolved.
11-39: Manual indirect list should be trimmed bygo mod tidy.The huge second
requireblock is unnecessary – let the tool maintain indirects.
Also add a localreplace github.com/maximhq/bifrost/core => ../../coreif you expect local builds.plugins/redis/main.go (2)
226-237:PreHookcontract & ctx handling don’t match core interface.– Signature (
*schemas.PluginShortCircuit) still differs from the interface that expects only(*schemas.BifrostRequest, error)(see previous lint failure).
– Pointer-to-context plus in-place mutation (*ctx = …) is unusual and risks data races.
Align the method to the officialschemas.Pluginsignature and pass/return context immutably.
382-389:KEYSis blocking; switch toSCAN.Using
KEYSto gather cache keys can freeze Redis on large datasets. Prefer iterativeSCANwith batching.plugins/redis/plugin_test.go (1)
27-35: Skip whenOPENAI_API_KEYis unset.The test still proceeds without verifying the env var, leading to network failures.
Add:if os.Getenv("OPENAI_API_KEY") == "" { t.Skip("OPENAI_API_KEY not set") }plugins/redis/README.md (1)
168-174: Example error-handling condition still inverted.The snippet logs cache info only on error (
err != nil). Swap toerr == nil.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
plugins/redis/go.sumis excluded by!**/*.sum
📒 Files selected for processing (4)
plugins/redis/README.md(1 hunks)plugins/redis/go.mod(1 hunks)plugins/redis/main.go(1 hunks)plugins/redis/plugin_test.go(1 hunks)
🧰 Additional context used
🧠 Learnings (35)
📓 Common learnings
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#141
File: core/bifrost.go:198-272
Timestamp: 2025-07-08T18:30:08.258Z
Learning: Pratham-Mishra04 follows a pattern of implementing core functionality first and deferring non-critical improvements (like race condition fixes, optimizations) to later PRs. This is a reasonable development approach that prioritizes getting the main feature working before addressing edge cases.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#202
File: transports/bifrost-http/plugins/governance/resolver.go:183-191
Timestamp: 2025-08-03T20:36:21.906Z
Learning: In the Bifrost governance plugin (transports/bifrost-http/plugins/governance/resolver.go), Pratham-Mishra04 considers the current string matching approach for determining rate limit violation decision types (DecisionTokenLimited vs DecisionRequestLimited) adequate and not important enough to refactor with more robust explicit checks, preferring functional simplicity over theoretical robustness improvements.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#158
File: .github/workflows/transports-release.yml:30-43
Timestamp: 2025-07-18T11:12:28.861Z
Learning: Pratham-Mishra04 relies on branch protection rules and mandatory code reviews for security in the bifrost repository, preferring process controls over technical security measures like environment variable isolation for GitHub Actions workflows. All commits are reviewed before merging to main branch.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#84
File: transports/bifrost-http/main.go:2-2
Timestamp: 2025-06-15T16:05:13.489Z
Learning: For the Bifrost project, HTTP transport integration routers for new providers (like Mistral and Ollama) are implemented in separate PRs from the core provider support, following a focused PR strategy.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#159
File: ci/npx/bin.js:111-119
Timestamp: 2025-07-18T10:55:09.985Z
Learning: In the Bifrost project, Pratham-Mishra04 relies on npx's built-in caching mechanism for the maximhq/bifrost package rather than implementing custom binary caching. NPX automatically caches downloaded packages, making additional caching logic unnecessary.
Learnt from: TejasGhatte
PR: maximhq/bifrost#174
File: transports/bifrost-http/plugins/logging/operations.go:113-114
Timestamp: 2025-08-01T16:25:49.937Z
Learning: In the Bifrost logging plugin (transports/bifrost-http/plugins/logging/operations.go), the calculateLatency method is only called at final chunk of streaming responses and when errors are returned, not repeatedly during streaming. This makes the performance impact of the database query minimal and doesn't warrant optimization with in-memory caching.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#83
File: core/providers/mistral.go:168-170
Timestamp: 2025-06-15T14:24:49.882Z
Learning: In the Bifrost codebase, performance is prioritized over defensive copying for HTTP service operations. Specifically, shallow slice assignments in provider response handling are acceptable due to object pool reset patterns and JSON unmarshaling behavior that minimize practical data corruption risks.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#202
File: transports/bifrost-http/plugins/governance/tracker.go:146-146
Timestamp: 2025-08-03T20:50:48.247Z
Learning: In the Bifrost governance tracker (transports/bifrost-http/plugins/governance/tracker.go), Pratham-Mishra04 implements database persistence for reset operations using arrays to collect reset entities (resetRateLimits and resetBudgets) and then batch persist them using t.db.Save() with proper error aggregation in the PerformStartupResets method.
Learnt from: TejasGhatte
PR: maximhq/bifrost#174
File: transports/bifrost-http/plugins/logging/operations.go:113-114
Timestamp: 2025-08-01T16:25:49.937Z
Learning: In the Bifrost logging plugin (transports/bifrost-http/plugins/logging/operations.go), the calculateLatency method is only called in two specific scenarios: when ErrorDetails is present (error case) and when isFinalChunk is true (end of streaming). It is not called repeatedly during streaming, making performance optimization with caching unnecessary.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#94
File: core/mcp.go:696-711
Timestamp: 2025-06-18T20:16:37.560Z
Learning: For the Bifrost MCP implementation, performance optimizations like caching tool discovery results should be deferred until after core functionality is complete. The author prefers to implement core features first and handle optimizations as follow-up work.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#152
File: transports/bifrost-http/plugins/logging/utils.go:378-399
Timestamp: 2025-07-10T13:44:14.518Z
Learning: In the Bifrost logging plugin (transports/bifrost-http/plugins/logging/utils.go), Pratham-Mishra04 prefers not to add error handling for JSON unmarshaling operations, considering logging not very critical and being confident that JSON marshalling won't fail in practice.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#152
File: transports/bifrost-http/plugins/logging/utils.go:16-18
Timestamp: 2025-07-10T13:44:39.237Z
Learning: In the Bifrost logging plugin (transports/bifrost-http/plugins/logging/utils.go), Pratham-Mishra04 prefers to ignore JSON marshaling errors when storing log entries because logging is not critical for their use case and they are certain the marshaling operations won't fail.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#55
File: core/providers/anthropic.go:358-388
Timestamp: 2025-06-04T05:37:59.699Z
Learning: User Pratham-Mishra04 prefers not to extract small code duplications (around 2 lines) into helper functions, considering the overhead not worth it for such minor repetition.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#102
File: README.md:62-66
Timestamp: 2025-06-19T17:03:03.639Z
Learning: Pratham-Mishra04 prefers using the implicit 'latest' tag for the maximhq/bifrost Docker image rather than pinning to specific versions.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#143
File: core/mcp.go:155-196
Timestamp: 2025-07-08T15:33:47.698Z
Learning: Pratham-Mishra04 prefers not to add explanatory comments for obvious code patterns, such as the unlock/lock strategy around network I/O operations, considering them self-explanatory to experienced developers.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#150
File: transports/bifrost-http/lib/store.go:370-466
Timestamp: 2025-07-09T04:58:08.229Z
Learning: Pratham-Mishra04 prefers not to add logging or error handling for unreachable code paths in the Bifrost project. When provider types or similar entities are predefined in the system, defensive programming like logging in default cases is considered unnecessary overhead.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#138
File: docs/usage/go-package/mcp.md:408-412
Timestamp: 2025-07-01T12:40:08.576Z
Learning: Pratham-Mishra04 is okay with keeping bullet list formatting that uses colons after dashes in markdown documentation, even if it triggers linter warnings, preferring functionality over strict formatting rules.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#162
File: tests/core-providers/scenarios/chat_completion_stream.go:103-105
Timestamp: 2025-07-16T04:26:09.288Z
Learning: Pratham-Mishra04 prefers to keep test code simple when it serves its basic functional purpose. For tests that are meant to validate core functionality (like verifying streaming works), they consider hard-coded reasonable limits acceptable rather than making them configurable.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#148
File: transports/bifrost-http/lib/store.go:880-910
Timestamp: 2025-07-08T17:14:21.544Z
Learning: Pratham-Mishra04 prefers resilient system design where missing environment variables for MCP connections should not cause complete system failure. The system should continue processing other MCP connections even when some fail, maintaining partial functionality rather than implementing fail-fast behavior.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#148
File: transports/bifrost-http/lib/store.go:1081-1098
Timestamp: 2025-07-08T17:16:50.811Z
Learning: Pratham-Mishra04 prefers practical redaction approaches over theoretical security improvements when the threat model is low-risk, such as admin-only interfaces in the Bifrost project. Fixed-length redaction is acceptable when only trusted administrators will see the redacted values.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#138
File: transports/README.md:26-28
Timestamp: 2025-07-01T12:45:06.906Z
Learning: Pratham-Mishra04 prefers keeping documentation examples simple and concise, trusting users to handle production-specific considerations like version pinning themselves rather than cluttering examples with additional notes.
📚 Learning: 2025-06-20T16:21:18.912Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#103
File: .github/workflows/transport-dependency-update.yml:53-75
Timestamp: 2025-06-20T16:21:18.912Z
Learning: In the bifrost repository's transport dependency update workflow, when updating the core dependency to a new version using `go get`, the go.mod and go.sum files will always change in normal operation, making the safety check for changes more of a defensive programming practice rather than handling a common scenario.
Applied to files:
plugins/redis/go.mod
📚 Learning: 2025-06-16T03:55:16.949Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#81
File: tests/core-providers/go.mod:3-4
Timestamp: 2025-06-16T03:55:16.949Z
Learning: Go 1.24 was released in February 2025 and is stable and available for use in go.mod files.
Applied to files:
plugins/redis/go.mod
📚 Learning: 2025-06-16T04:27:53.538Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#81
File: tests/core-providers/go.mod:3-4
Timestamp: 2025-06-16T04:27:53.538Z
Learning: In Go module files, `go 1.24.1` (with patch version) can work fine in some setups, contrary to the general rule that go directives should only include major.minor versions.
Applied to files:
plugins/redis/go.mod
📚 Learning: 2025-06-27T17:07:39.462Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#135
File: docs/core-package.md:105-116
Timestamp: 2025-06-27T17:07:39.462Z
Learning: In Go, when importing a package, the identifier used to access it is determined by the `package` declaration in the Go source files, not the directory name. For the Bifrost project, the core directory files declare `package bifrost`, so importing `"github.com/maximhq/bifrost/core"` automatically makes it accessible as `bifrost.Init`, `bifrost.Ptr`, etc., without needing an explicit alias.
Applied to files:
plugins/redis/go.mod
📚 Learning: 2025-06-17T14:04:21.104Z
Learnt from: connyay
PR: maximhq/bifrost#92
File: core/providers/utils.go:60-64
Timestamp: 2025-06-17T14:04:21.104Z
Learning: The bifrost codebase uses string alias types (like `type ContextKey string`) for context keys consistently across multiple packages including plugins/maxim/main.go and transports/bifrost-http/tracking/plugin.go. This pattern should be maintained for consistency rather than switching to the canonical struct{} approach.
Applied to files:
plugins/redis/go.modplugins/redis/main.go
📚 Learning: 2025-08-03T20:50:48.247Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#202
File: transports/bifrost-http/plugins/governance/tracker.go:146-146
Timestamp: 2025-08-03T20:50:48.247Z
Learning: In the Bifrost governance tracker (transports/bifrost-http/plugins/governance/tracker.go), Pratham-Mishra04 implements database persistence for reset operations using arrays to collect reset entities (resetRateLimits and resetBudgets) and then batch persist them using t.db.Save() with proper error aggregation in the PerformStartupResets method.
Applied to files:
plugins/redis/go.mod
📚 Learning: 2025-06-15T14:24:49.882Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#83
File: core/providers/mistral.go:168-170
Timestamp: 2025-06-15T14:24:49.882Z
Learning: In the Bifrost codebase, performance is prioritized over defensive copying for HTTP service operations. Specifically, shallow slice assignments in provider response handling are acceptable due to object pool reset patterns and JSON unmarshaling behavior that minimize practical data corruption risks.
Applied to files:
plugins/redis/go.modplugins/redis/README.md
📚 Learning: 2025-08-08T06:51:27.123Z
Learnt from: TejasGhatte
PR: maximhq/bifrost#219
File: core/schemas/bifrost.go:686-692
Timestamp: 2025-08-08T06:51:27.123Z
Learning: In core/schemas/bifrost.go, BifrostError.StreamControl is intended for plugin developers only and is excluded from JSON serialization; its presence controls streaming behavior internally.
Applied to files:
plugins/redis/go.modplugins/redis/main.go
📚 Learning: 2025-07-10T13:44:14.518Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#152
File: transports/bifrost-http/plugins/logging/utils.go:378-399
Timestamp: 2025-07-10T13:44:14.518Z
Learning: In the Bifrost logging plugin (transports/bifrost-http/plugins/logging/utils.go), Pratham-Mishra04 prefers not to add error handling for JSON unmarshaling operations, considering logging not very critical and being confident that JSON marshalling won't fail in practice.
Applied to files:
plugins/redis/go.modplugins/redis/README.mdplugins/redis/main.go
📚 Learning: 2025-07-10T13:44:39.237Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#152
File: transports/bifrost-http/plugins/logging/utils.go:16-18
Timestamp: 2025-07-10T13:44:39.237Z
Learning: In the Bifrost logging plugin (transports/bifrost-http/plugins/logging/utils.go), Pratham-Mishra04 prefers to ignore JSON marshaling errors when storing log entries because logging is not critical for their use case and they are certain the marshaling operations won't fail.
Applied to files:
plugins/redis/go.modplugins/redis/README.mdplugins/redis/main.go
📚 Learning: 2025-06-16T04:13:55.437Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#81
File: tests/core-providers/scenarios/end_to_end_tool_calling.go:43-45
Timestamp: 2025-06-16T04:13:55.437Z
Learning: In the Bifrost codebase, errors returned from client methods like ChatCompletionRequest are of type BifrostError, not the standard error interface. For testing these errors, use require.Nilf instead of require.NoErrorf since BifrostError doesn't work with the standard error assertion methods.
Applied to files:
plugins/redis/go.modplugins/redis/main.go
📚 Learning: 2025-08-04T22:19:12.975Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#212
File: transports/bifrost-http/integrations/openai/types.go:250-288
Timestamp: 2025-08-04T22:19:12.975Z
Learning: In transports/bifrost-http/integrations/openai/types.go, the ConvertToBifrostRequest function for OpenAIEmbeddingRequest handles []interface{} input without validation for non-string elements because Pratham-Mishra04 confirmed this case will never happen in practice, consistent with their preference to avoid defensive programming for unreachable scenarios.
Applied to files:
plugins/redis/go.modplugins/redis/plugin_test.goplugins/redis/main.go
📚 Learning: 2025-07-08T18:09:32.147Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#149
File: transports/bifrost-http/plugins/logging/main.go:0-0
Timestamp: 2025-07-08T18:09:32.147Z
Learning: In the Bifrost HTTP transport logging plugin (transports/bifrost-http/plugins/logging/main.go), the DatabasePath is explicitly set from main.go via a config struct pointing to appDir/logs, so the default value in the NewLoggerPlugin constructor is only used for standalone plugin usage and doesn't affect the main application flow.
Applied to files:
plugins/redis/go.mod
📚 Learning: 2025-07-08T16:40:59.098Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#144
File: transports/bifrost-http/plugins/logging/utils.go:286-292
Timestamp: 2025-07-08T16:40:59.098Z
Learning: Pratham-Mishra04 prefers to keep simpler error handling patterns when errors are unlikely to occur in practice, such as safety checks in BadgerDB iterations where item.Value() is called on valid items. The user considers the overhead of explicit error handling not worth it in such scenarios.
Applied to files:
plugins/redis/README.mdplugins/redis/main.go
📚 Learning: 2025-07-10T13:44:23.297Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#152
File: transports/bifrost-http/plugins/logging/utils.go:94-111
Timestamp: 2025-07-10T13:44:23.297Z
Learning: Pratham-Mishra04 prefers not to add error handling for JSON marshaling operations in the Bifrost logging plugin (transports/bifrost-http/plugins/logging/utils.go) because logging is not critical functionality and the structured schema data being marshaled is unlikely to fail. They accept the risk of not handling json.Marshal errors in logging contexts to keep the code simple.
Applied to files:
plugins/redis/README.mdplugins/redis/main.go
📚 Learning: 2025-07-08T18:20:24.086Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#149
File: transports/bifrost-http/main.go:259-261
Timestamp: 2025-07-08T18:20:24.086Z
Learning: In transports/bifrost-http/lib/store.go, the LoadFromConfig method already handles missing config files gracefully by initializing defaults and auto-detecting providers from environment variables. It only returns errors for actual parsing/processing failures, not missing files, making log.Fatalf appropriate in main.go since real errors should terminate the program.
Applied to files:
plugins/redis/README.md
📚 Learning: 2025-07-08T16:48:25.386Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#144
File: transports/bifrost-http/plugins/logging/utils.go:509-514
Timestamp: 2025-07-08T16:48:25.386Z
Learning: In the Bifrost logging system (transports/bifrost-http/plugins/logging/utils.go), the Content field in message structures is a struct value type, not a pointer, so it will never be nil and doesn't require nil checks. However, ContentStr within Content is a pointer and should be checked for nil.
Applied to files:
plugins/redis/README.md
📚 Learning: 2025-08-01T16:25:49.937Z
Learnt from: TejasGhatte
PR: maximhq/bifrost#174
File: transports/bifrost-http/plugins/logging/operations.go:113-114
Timestamp: 2025-08-01T16:25:49.937Z
Learning: In the Bifrost logging plugin (transports/bifrost-http/plugins/logging/operations.go), the calculateLatency method is only called at final chunk of streaming responses and when errors are returned, not repeatedly during streaming. This makes the performance impact of the database query minimal and doesn't warrant optimization with in-memory caching.
Applied to files:
plugins/redis/README.mdplugins/redis/plugin_test.goplugins/redis/main.go
📚 Learning: 2025-07-18T10:55:09.985Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#159
File: ci/npx/bin.js:111-119
Timestamp: 2025-07-18T10:55:09.985Z
Learning: In the Bifrost project, Pratham-Mishra04 relies on npx's built-in caching mechanism for the maximhq/bifrost package rather than implementing custom binary caching. NPX automatically caches downloaded packages, making additional caching logic unnecessary.
Applied to files:
plugins/redis/README.md
📚 Learning: 2025-06-16T09:16:15.634Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#82
File: tests/transports-integrations/tests/integrations/test_litellm.py:97-115
Timestamp: 2025-06-16T09:16:15.634Z
Learning: In the Bifrost integration tests, the user prefers tests to fail hard when API keys are missing rather than using skip_if_no_api_key decorators for graceful skipping. This applies to LiteLLM tests that depend on OpenAI API keys.
Applied to files:
plugins/redis/plugin_test.go
📚 Learning: 2025-06-26T07:37:24.385Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#131
File: transports/bifrost-http/lib/config.go:35-68
Timestamp: 2025-06-26T07:37:24.385Z
Learning: In the Bifrost project's MCP configuration handling, empty environment variables should be treated as missing/invalid rather than as valid empty values. The current implementation using `os.Getenv(envKey); envValue != ""` to check for non-empty values is the desired behavior.
Applied to files:
plugins/redis/plugin_test.goplugins/redis/main.go
📚 Learning: 2025-06-16T04:55:11.886Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#81
File: tests/core-providers/scenarios/automatic_function_calling.go:22-22
Timestamp: 2025-06-16T04:55:11.886Z
Learning: In the Bifrost test suite (tests/core-providers), parallel tests using t.Parallel() are not being implemented currently. The team plans to add parallel test execution in future enhancements.
Applied to files:
plugins/redis/plugin_test.go
📚 Learning: 2025-06-16T04:25:00.816Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#81
File: tests/core-providers/config/account.go:55-101
Timestamp: 2025-06-16T04:25:00.816Z
Learning: In the Bifrost test account implementation, the user prefers to let Bifrost itself handle missing API key errors rather than adding early validation in the GetKeysForProvider method.
Applied to files:
plugins/redis/plugin_test.go
📚 Learning: 2025-06-04T04:58:12.239Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#55
File: core/tests/e2e_tool_test.go:29-30
Timestamp: 2025-06-04T04:58:12.239Z
Learning: In the Bifrost project, environment variables should be used only for secrets (like API keys), not for general configuration. Test parameters like provider and model can be hardcoded at the start of test files for predictability and consistency.
Applied to files:
plugins/redis/plugin_test.go
📚 Learning: 2025-08-01T16:25:49.937Z
Learnt from: TejasGhatte
PR: maximhq/bifrost#174
File: transports/bifrost-http/plugins/logging/operations.go:113-114
Timestamp: 2025-08-01T16:25:49.937Z
Learning: In the Bifrost logging plugin (transports/bifrost-http/plugins/logging/operations.go), the calculateLatency method is only called in two specific scenarios: when ErrorDetails is present (error case) and when isFinalChunk is true (end of streaming). It is not called repeatedly during streaming, making performance optimization with caching unnecessary.
Applied to files:
plugins/redis/plugin_test.go
📚 Learning: 2025-08-04T19:39:39.417Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#209
File: plugins/jsonparser/plugin_test.go:38-45
Timestamp: 2025-08-04T19:39:39.417Z
Learning: In the JSON Parser plugin (plugins/jsonparser/main.go), the PostHook method is designed with a contract that guarantees the response structure (Choices, BifrostStreamResponseChoice, Delta, Content) will always be present when called, making defensive nil checks unnecessary in tests.
Applied to files:
plugins/redis/main.go
📚 Learning: 2025-08-08T07:07:08.801Z
Learnt from: TejasGhatte
PR: maximhq/bifrost#219
File: core/providers/openai.go:459-459
Timestamp: 2025-08-08T07:07:08.801Z
Learning: In core/providers/utils.go, processAndSendResponse and processAndSendError now take exactly five parameters: (ctx, postHookRunner, response|err, responseChan, logger). All providers (OpenAI, Anthropic, Cohere, Bedrock) must call these with the logger as the fifth argument; a repo-wide check in PR maximhq/bifrost#219 confirmed this is already consistent.
Applied to files:
plugins/redis/main.go
📚 Learning: 2025-06-14T04:06:58.240Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#79
File: core/bifrost.go:94-103
Timestamp: 2025-06-14T04:06:58.240Z
Learning: In core/bifrost.go, the count parameter in RunPostHooks method is intentionally kept separate from p.executedPreHooks to support circuit breaker plugins that may need to trigger PostHooks for only a subset of executed plugins when detecting failure conditions mid-execution.
Applied to files:
plugins/redis/main.go
📚 Learning: 2025-06-04T09:22:18.123Z
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.
Applied to files:
plugins/redis/main.go
📚 Learning: 2025-08-05T20:43:59.593Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#214
File: core/providers/azure.go:49-50
Timestamp: 2025-08-05T20:43:59.593Z
Learning: In core/providers/azure.go, the azureTextCompletionResponsePool should use AzureTextResponse type, not schemas.BifrostResponse, to maintain consistency with the acquireAzureTextResponse() and releaseAzureTextResponse() functions that work with *AzureTextResponse objects.
Applied to files:
plugins/redis/main.go
📚 Learning: 2025-08-07T13:33:07.837Z
Learnt from: TejasGhatte
PR: maximhq/bifrost#219
File: plugins/jsonparser/main.go:0-0
Timestamp: 2025-08-07T13:33:07.837Z
Learning: In the JSON Parser plugin (plugins/jsonparser/main.go), TejasGhatte prefers to return responses as-is (skip processing) when a request ID cannot be determined, rather than generating fallback IDs. The getRequestID method should return an empty string when no proper identifier is found, allowing the PostHook method's early return logic to skip accumulation and processing.
Applied to files:
plugins/redis/main.go
📚 Learning: 2025-08-02T13:02:13.642Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#174
File: transports/bifrost-http/lib/models.go:251-254
Timestamp: 2025-08-02T13:02:13.642Z
Learning: In the Bifrost project's database models (transports/bifrost-http/lib/models.go), Pratham-Mishra04 considers meta config types to be predefined and expects that unknown meta config types will never occur in practice. The current implementation that returns nil for unknown types in the AfterFind hook is intentional, as defensive programming for this scenario is considered unnecessary.
Applied to files:
plugins/redis/main.go
📚 Learning: 2025-07-08T16:50:27.699Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#144
File: transports/bifrost-http/handlers/providers.go:45-49
Timestamp: 2025-07-08T16:50:27.699Z
Learning: In the Bifrost project, breaking API changes are acceptable when features are not yet public. This applies to scenarios like changing struct fields from pointer to non-pointer types in request/response structures for unreleased features.
Applied to files:
plugins/redis/main.go
📚 Learning: 2025-08-08T06:37:43.094Z
Learnt from: TejasGhatte
PR: maximhq/bifrost#219
File: plugins/jsonparser/main.go:175-196
Timestamp: 2025-08-08T06:37:43.094Z
Learning: In plugins/jsonparser/main.go (JSON Parser plugin), TejasGhatte prefers not to introduce a max-bytes cap on per-request accumulation (accumulateContent) because truncation or skipping would lose the plugin’s desired behavior of returning completed JSON. If memory limits are needed, consider optional non-lossy strategies (soft-limit logging or spill-to-disk), disabled by default.
Applied to files:
plugins/redis/main.go
🔇 Additional comments (1)
plugins/redis/main.go (1)
248-248: Context mutation may race.Overwriting the incoming context (
*ctx = …) from multiple concurrent requests can race because plugins run in parallel. Clone instead:newCtx := context.WithValue(*ctx, RequestHashKey, hash) *ctx = newCtx // only if design really needs pointer contextor drop the pointer approach.
⛔ Skipped due to learnings
Learnt from: connyay PR: maximhq/bifrost#92 File: core/providers/utils.go:60-64 Timestamp: 2025-06-17T14:04:21.104Z Learning: The bifrost codebase uses string alias types (like `type ContextKey string`) for context keys consistently across multiple packages including plugins/maxim/main.go and transports/bifrost-http/tracking/plugin.go. This pattern should be maintained for consistency rather than switching to the canonical struct{} approach.Learnt from: Pratham-Mishra04 PR: maximhq/bifrost#63 File: transports/bifrost-http/integrations/openai/router.go:62-64 Timestamp: 2025-06-09T11:27:00.925Z Learning: The `lib.ConvertToBifrostContext` function in the bifrost HTTP transport never returns nil and always returns a pointer to a valid context. It starts with context.Background() and only adds values to it, so nil checks are not needed when calling this function.Learnt from: Pratham-Mishra04 PR: maximhq/bifrost#88 File: core/providers/azure.go:0-0 Timestamp: 2025-06-15T19:04:55.993Z Learning: In Go, when JSON unmarshaling populates pooled structs, the slice and string fields point to newly allocated memory created by json.Unmarshal(). Shallow copying these fields (copying slice headers) is safe because the underlying data is not part of the pooled memory - it's owned by the JSON unmarshaling process. Deep copying is unnecessary and wasteful in this pattern.Learnt from: TejasGhatte PR: maximhq/bifrost#219 File: plugins/jsonparser/main.go:0-0 Timestamp: 2025-08-07T13:33:18.094Z Learning: In the JSON Parser plugin (plugins/jsonparser/main.go), the context key for enabling per-request JSON parsing is "enable-streaming-json-parser" (defined as EnableStreamingJSONParser constant), which is distinct from the plugin name constant to avoid confusion between plugin identification and per-request activation control.Learnt from: Pratham-Mishra04 PR: maximhq/bifrost#88 File: core/providers/openai.go:202-210 Timestamp: 2025-06-16T06:56:51.547Z Learning: In Go, when JSON unmarshaling into a pooled struct, slices within the struct are freshly allocated by the JSON unmarshaler, not part of the pooled struct's original memory. Defensive copying of these slices is unnecessary since they don't reference pooled memory that could be reused.Learnt from: Pratham-Mishra04 PR: maximhq/bifrost#83 File: core/providers/mistral.go:168-170 Timestamp: 2025-06-15T14:24:49.882Z Learning: In the Bifrost codebase, performance is prioritized over defensive copying for HTTP service operations. Specifically, shallow slice assignments in provider response handling are acceptable due to object pool reset patterns and JSON unmarshaling behavior that minimize practical data corruption risks.Learnt from: Pratham-Mishra04 PR: maximhq/bifrost#81 File: tests/core-providers/scenarios/utils.go:101-110 Timestamp: 2025-06-16T04:21:07.962Z Learning: In the Bifrost codebase, when a variable is already available, it's acceptable to use `&variable` directly to get its pointer rather than always using `bifrost.Ptr(variable)`. The `bifrost.Ptr()` helper is primarily for taking pointers of literals or expressions, not existing variables.Learnt from: Pratham-Mishra04 PR: maximhq/bifrost#202 File: transports/bifrost-http/handlers/governance.go:220-220 Timestamp: 2025-08-03T20:11:26.945Z Learning: In the Bifrost HTTP transport handlers using fasthttp router, path parameters extracted via ctx.UserValue() can be safely type-asserted to string without additional checks because the router guarantees their presence and type when routes match. Pratham-Mishra04 confirmed that defensive type assertion checks are unnecessary overhead in this context.Learnt from: Pratham-Mishra04 PR: maximhq/bifrost#63 File: transports/bifrost-http/integrations/openai/router.go:62-64 Timestamp: 2025-06-09T11:27:00.925Z Learning: The `lib.ConvertToBifrostContext` function in the bifrost HTTP transport never returns nil and handles the conversion internally, so nil checks are not needed when calling this function.
Pratham-Mishra04
force-pushed
the
06-11-_plugins_feat_redis_caching_plugin_added
branch
from
601ae22 to
097343d
Compare
Pratham-Mishra04
changed the base branch from
graphite-base/70
to
08-09-feat-extended_plugins_for_better_stream_control
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (4)
plugins/redis/go.mod (2)
3-3: Use a validgodirective
go 1.24.1is invalid; the directive must bemajor.minoronly (e.g.go 1.24).
13-46: Trim the hand-maintained indirect listRemove the manual
require (…) // indirectblock and rungo mod tidy; let the tool manage indirects.plugins/redis/main.go (2)
489-500:KEYSis unsafe for large key-spaces
Cleanup(and earlier chunk look-ups) useKEYS, which blocks Redis.
Switch to iterativeSCAN+ batchedDEL.
97-99: Warn level is too loud for defaulting TTLDefaulting to 5 min is normal; use
Debug(orInfo) instead ofWarn.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
plugins/redis/go.sumis excluded by!**/*.sum
📒 Files selected for processing (4)
plugins/redis/README.md(1 hunks)plugins/redis/go.mod(1 hunks)plugins/redis/main.go(1 hunks)plugins/redis/plugin_test.go(1 hunks)
🧰 Additional context used
🧠 Learnings (49)
📓 Common learnings
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#202
File: transports/bifrost-http/plugins/governance/resolver.go:183-191
Timestamp: 2025-08-03T20:36:21.906Z
Learning: In the Bifrost governance plugin (transports/bifrost-http/plugins/governance/resolver.go), Pratham-Mishra04 considers the current string matching approach for determining rate limit violation decision types (DecisionTokenLimited vs DecisionRequestLimited) adequate and not important enough to refactor with more robust explicit checks, preferring functional simplicity over theoretical robustness improvements.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#141
File: core/bifrost.go:198-272
Timestamp: 2025-07-08T18:30:08.258Z
Learning: Pratham-Mishra04 follows a pattern of implementing core functionality first and deferring non-critical improvements (like race condition fixes, optimizations) to later PRs. This is a reasonable development approach that prioritizes getting the main feature working before addressing edge cases.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#158
File: .github/workflows/transports-release.yml:30-43
Timestamp: 2025-07-18T11:12:28.861Z
Learning: Pratham-Mishra04 relies on branch protection rules and mandatory code reviews for security in the bifrost repository, preferring process controls over technical security measures like environment variable isolation for GitHub Actions workflows. All commits are reviewed before merging to main branch.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#84
File: transports/bifrost-http/main.go:2-2
Timestamp: 2025-06-15T16:05:13.489Z
Learning: For the Bifrost project, HTTP transport integration routers for new providers (like Mistral and Ollama) are implemented in separate PRs from the core provider support, following a focused PR strategy.
Learnt from: TejasGhatte
PR: maximhq/bifrost#174
File: transports/bifrost-http/plugins/logging/operations.go:113-114
Timestamp: 2025-08-01T16:25:49.937Z
Learning: In the Bifrost logging plugin (transports/bifrost-http/plugins/logging/operations.go), the calculateLatency method is only called at final chunk of streaming responses and when errors are returned, not repeatedly during streaming. This makes the performance impact of the database query minimal and doesn't warrant optimization with in-memory caching.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#159
File: ci/npx/bin.js:111-119
Timestamp: 2025-07-18T10:55:09.985Z
Learning: In the Bifrost project, Pratham-Mishra04 relies on npx's built-in caching mechanism for the maximhq/bifrost package rather than implementing custom binary caching. NPX automatically caches downloaded packages, making additional caching logic unnecessary.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#83
File: core/providers/mistral.go:168-170
Timestamp: 2025-06-15T14:24:49.882Z
Learning: In the Bifrost codebase, performance is prioritized over defensive copying for HTTP service operations. Specifically, shallow slice assignments in provider response handling are acceptable due to object pool reset patterns and JSON unmarshaling behavior that minimize practical data corruption risks.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#202
File: transports/bifrost-http/plugins/governance/tracker.go:146-146
Timestamp: 2025-08-03T20:50:48.247Z
Learning: In the Bifrost governance tracker (transports/bifrost-http/plugins/governance/tracker.go), Pratham-Mishra04 implements database persistence for reset operations using arrays to collect reset entities (resetRateLimits and resetBudgets) and then batch persist them using t.db.Save() with proper error aggregation in the PerformStartupResets method.
Learnt from: TejasGhatte
PR: maximhq/bifrost#174
File: transports/bifrost-http/plugins/logging/operations.go:113-114
Timestamp: 2025-08-01T16:25:49.937Z
Learning: In the Bifrost logging plugin (transports/bifrost-http/plugins/logging/operations.go), the calculateLatency method is only called in two specific scenarios: when ErrorDetails is present (error case) and when isFinalChunk is true (end of streaming). It is not called repeatedly during streaming, making performance optimization with caching unnecessary.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#94
File: core/mcp.go:696-711
Timestamp: 2025-06-18T20:16:37.560Z
Learning: For the Bifrost MCP implementation, performance optimizations like caching tool discovery results should be deferred until after core functionality is complete. The author prefers to implement core features first and handle optimizations as follow-up work.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#152
File: transports/bifrost-http/plugins/logging/utils.go:378-399
Timestamp: 2025-07-10T13:44:14.518Z
Learning: In the Bifrost logging plugin (transports/bifrost-http/plugins/logging/utils.go), Pratham-Mishra04 prefers not to add error handling for JSON unmarshaling operations, considering logging not very critical and being confident that JSON marshalling won't fail in practice.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#152
File: transports/bifrost-http/plugins/logging/utils.go:16-18
Timestamp: 2025-07-10T13:44:39.237Z
Learning: In the Bifrost logging plugin (transports/bifrost-http/plugins/logging/utils.go), Pratham-Mishra04 prefers to ignore JSON marshaling errors when storing log entries because logging is not critical for their use case and they are certain the marshaling operations won't fail.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#55
File: core/providers/anthropic.go:358-388
Timestamp: 2025-06-04T05:37:59.699Z
Learning: User Pratham-Mishra04 prefers not to extract small code duplications (around 2 lines) into helper functions, considering the overhead not worth it for such minor repetition.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#102
File: README.md:62-66
Timestamp: 2025-06-19T17:03:03.639Z
Learning: Pratham-Mishra04 prefers using the implicit 'latest' tag for the maximhq/bifrost Docker image rather than pinning to specific versions.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#143
File: core/mcp.go:155-196
Timestamp: 2025-07-08T15:33:47.698Z
Learning: Pratham-Mishra04 prefers not to add explanatory comments for obvious code patterns, such as the unlock/lock strategy around network I/O operations, considering them self-explanatory to experienced developers.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#150
File: transports/bifrost-http/lib/store.go:370-466
Timestamp: 2025-07-09T04:58:08.229Z
Learning: Pratham-Mishra04 prefers not to add logging or error handling for unreachable code paths in the Bifrost project. When provider types or similar entities are predefined in the system, defensive programming like logging in default cases is considered unnecessary overhead.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#138
File: docs/usage/go-package/mcp.md:408-412
Timestamp: 2025-07-01T12:40:08.576Z
Learning: Pratham-Mishra04 is okay with keeping bullet list formatting that uses colons after dashes in markdown documentation, even if it triggers linter warnings, preferring functionality over strict formatting rules.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#162
File: tests/core-providers/scenarios/chat_completion_stream.go:103-105
Timestamp: 2025-07-16T04:26:09.288Z
Learning: Pratham-Mishra04 prefers to keep test code simple when it serves its basic functional purpose. For tests that are meant to validate core functionality (like verifying streaming works), they consider hard-coded reasonable limits acceptable rather than making them configurable.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#148
File: transports/bifrost-http/lib/store.go:880-910
Timestamp: 2025-07-08T17:14:21.544Z
Learning: Pratham-Mishra04 prefers resilient system design where missing environment variables for MCP connections should not cause complete system failure. The system should continue processing other MCP connections even when some fail, maintaining partial functionality rather than implementing fail-fast behavior.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#138
File: transports/README.md:26-28
Timestamp: 2025-07-01T12:45:06.906Z
Learning: Pratham-Mishra04 prefers keeping documentation examples simple and concise, trusting users to handle production-specific considerations like version pinning themselves rather than cluttering examples with additional notes.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#148
File: transports/bifrost-http/lib/store.go:1081-1098
Timestamp: 2025-07-08T17:16:50.811Z
Learning: Pratham-Mishra04 prefers practical redaction approaches over theoretical security improvements when the threat model is low-risk, such as admin-only interfaces in the Bifrost project. Fixed-length redaction is acceptable when only trusted administrators will see the redacted values.
📚 Learning: 2025-07-08T16:40:59.098Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#144
File: transports/bifrost-http/plugins/logging/utils.go:286-292
Timestamp: 2025-07-08T16:40:59.098Z
Learning: Pratham-Mishra04 prefers to keep simpler error handling patterns when errors are unlikely to occur in practice, such as safety checks in BadgerDB iterations where item.Value() is called on valid items. The user considers the overhead of explicit error handling not worth it in such scenarios.
Applied to files:
plugins/redis/README.mdplugins/redis/main.go
📚 Learning: 2025-07-10T13:44:23.297Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#152
File: transports/bifrost-http/plugins/logging/utils.go:94-111
Timestamp: 2025-07-10T13:44:23.297Z
Learning: Pratham-Mishra04 prefers not to add error handling for JSON marshaling operations in the Bifrost logging plugin (transports/bifrost-http/plugins/logging/utils.go) because logging is not critical functionality and the structured schema data being marshaled is unlikely to fail. They accept the risk of not handling json.Marshal errors in logging contexts to keep the code simple.
Applied to files:
plugins/redis/README.mdplugins/redis/main.go
📚 Learning: 2025-07-10T13:44:14.518Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#152
File: transports/bifrost-http/plugins/logging/utils.go:378-399
Timestamp: 2025-07-10T13:44:14.518Z
Learning: In the Bifrost logging plugin (transports/bifrost-http/plugins/logging/utils.go), Pratham-Mishra04 prefers not to add error handling for JSON unmarshaling operations, considering logging not very critical and being confident that JSON marshalling won't fail in practice.
Applied to files:
plugins/redis/README.mdplugins/redis/go.modplugins/redis/main.go
📚 Learning: 2025-07-08T18:20:24.086Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#149
File: transports/bifrost-http/main.go:259-261
Timestamp: 2025-07-08T18:20:24.086Z
Learning: In transports/bifrost-http/lib/store.go, the LoadFromConfig method already handles missing config files gracefully by initializing defaults and auto-detecting providers from environment variables. It only returns errors for actual parsing/processing failures, not missing files, making log.Fatalf appropriate in main.go since real errors should terminate the program.
Applied to files:
plugins/redis/README.md
📚 Learning: 2025-07-10T13:44:39.237Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#152
File: transports/bifrost-http/plugins/logging/utils.go:16-18
Timestamp: 2025-07-10T13:44:39.237Z
Learning: In the Bifrost logging plugin (transports/bifrost-http/plugins/logging/utils.go), Pratham-Mishra04 prefers to ignore JSON marshaling errors when storing log entries because logging is not critical for their use case and they are certain the marshaling operations won't fail.
Applied to files:
plugins/redis/README.mdplugins/redis/go.modplugins/redis/main.go
📚 Learning: 2025-07-08T16:48:25.386Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#144
File: transports/bifrost-http/plugins/logging/utils.go:509-514
Timestamp: 2025-07-08T16:48:25.386Z
Learning: In the Bifrost logging system (transports/bifrost-http/plugins/logging/utils.go), the Content field in message structures is a struct value type, not a pointer, so it will never be nil and doesn't require nil checks. However, ContentStr within Content is a pointer and should be checked for nil.
Applied to files:
plugins/redis/README.md
📚 Learning: 2025-08-08T14:12:14.754Z
Learnt from: TejasGhatte
PR: maximhq/bifrost#238
File: transports/bifrost-http/plugins/logging/main.go:0-0
Timestamp: 2025-08-08T14:12:14.754Z
Learning: In transports/bifrost-http/plugins/logging/main.go, stream accumulator cleanup is intended to use a 5-minute TTL (consistent with processing logs). Comments should state “older than 5 minutes,” and using central constants (processingLogTTL, streamAccumulatorTTL) helps avoid future mismatches.
Applied to files:
plugins/redis/README.mdplugins/redis/main.go
📚 Learning: 2025-08-01T16:25:49.937Z
Learnt from: TejasGhatte
PR: maximhq/bifrost#174
File: transports/bifrost-http/plugins/logging/operations.go:113-114
Timestamp: 2025-08-01T16:25:49.937Z
Learning: In the Bifrost logging plugin (transports/bifrost-http/plugins/logging/operations.go), the calculateLatency method is only called at final chunk of streaming responses and when errors are returned, not repeatedly during streaming. This makes the performance impact of the database query minimal and doesn't warrant optimization with in-memory caching.
Applied to files:
plugins/redis/README.mdplugins/redis/plugin_test.goplugins/redis/main.go
📚 Learning: 2025-06-15T14:24:49.882Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#83
File: core/providers/mistral.go:168-170
Timestamp: 2025-06-15T14:24:49.882Z
Learning: In the Bifrost codebase, performance is prioritized over defensive copying for HTTP service operations. Specifically, shallow slice assignments in provider response handling are acceptable due to object pool reset patterns and JSON unmarshaling behavior that minimize practical data corruption risks.
Applied to files:
plugins/redis/README.mdplugins/redis/go.mod
📚 Learning: 2025-07-18T10:55:09.985Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#159
File: ci/npx/bin.js:111-119
Timestamp: 2025-07-18T10:55:09.985Z
Learning: In the Bifrost project, Pratham-Mishra04 relies on npx's built-in caching mechanism for the maximhq/bifrost package rather than implementing custom binary caching. NPX automatically caches downloaded packages, making additional caching logic unnecessary.
Applied to files:
plugins/redis/README.md
📚 Learning: 2025-06-20T16:21:18.912Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#103
File: .github/workflows/transport-dependency-update.yml:53-75
Timestamp: 2025-06-20T16:21:18.912Z
Learning: In the bifrost repository's transport dependency update workflow, when updating the core dependency to a new version using `go get`, the go.mod and go.sum files will always change in normal operation, making the safety check for changes more of a defensive programming practice rather than handling a common scenario.
Applied to files:
plugins/redis/go.mod
📚 Learning: 2025-06-16T03:55:16.949Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#81
File: tests/core-providers/go.mod:3-4
Timestamp: 2025-06-16T03:55:16.949Z
Learning: Go 1.24 was released in February 2025 and is stable and available for use in go.mod files.
Applied to files:
plugins/redis/go.mod
📚 Learning: 2025-06-16T04:27:53.538Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#81
File: tests/core-providers/go.mod:3-4
Timestamp: 2025-06-16T04:27:53.538Z
Learning: In Go module files, `go 1.24.1` (with patch version) can work fine in some setups, contrary to the general rule that go directives should only include major.minor versions.
Applied to files:
plugins/redis/go.mod
📚 Learning: 2025-06-27T17:07:39.462Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#135
File: docs/core-package.md:105-116
Timestamp: 2025-06-27T17:07:39.462Z
Learning: In Go, when importing a package, the identifier used to access it is determined by the `package` declaration in the Go source files, not the directory name. For the Bifrost project, the core directory files declare `package bifrost`, so importing `"github.com/maximhq/bifrost/core"` automatically makes it accessible as `bifrost.Init`, `bifrost.Ptr`, etc., without needing an explicit alias.
Applied to files:
plugins/redis/go.mod
📚 Learning: 2025-06-17T14:04:21.104Z
Learnt from: connyay
PR: maximhq/bifrost#92
File: core/providers/utils.go:60-64
Timestamp: 2025-06-17T14:04:21.104Z
Learning: The bifrost codebase uses string alias types (like `type ContextKey string`) for context keys consistently across multiple packages including plugins/maxim/main.go and transports/bifrost-http/tracking/plugin.go. This pattern should be maintained for consistency rather than switching to the canonical struct{} approach.
Applied to files:
plugins/redis/go.modplugins/redis/plugin_test.goplugins/redis/main.go
📚 Learning: 2025-06-16T04:13:55.437Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#81
File: tests/core-providers/scenarios/end_to_end_tool_calling.go:43-45
Timestamp: 2025-06-16T04:13:55.437Z
Learning: In the Bifrost codebase, errors returned from client methods like ChatCompletionRequest are of type BifrostError, not the standard error interface. For testing these errors, use require.Nilf instead of require.NoErrorf since BifrostError doesn't work with the standard error assertion methods.
Applied to files:
plugins/redis/go.modplugins/redis/main.go
📚 Learning: 2025-08-08T06:51:27.123Z
Learnt from: TejasGhatte
PR: maximhq/bifrost#219
File: core/schemas/bifrost.go:686-692
Timestamp: 2025-08-08T06:51:27.123Z
Learning: In core/schemas/bifrost.go, BifrostError.StreamControl is intended for plugin developers only and is excluded from JSON serialization; its presence controls streaming behavior internally.
Applied to files:
plugins/redis/go.modplugins/redis/main.go
📚 Learning: 2025-08-03T20:50:48.247Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#202
File: transports/bifrost-http/plugins/governance/tracker.go:146-146
Timestamp: 2025-08-03T20:50:48.247Z
Learning: In the Bifrost governance tracker (transports/bifrost-http/plugins/governance/tracker.go), Pratham-Mishra04 implements database persistence for reset operations using arrays to collect reset entities (resetRateLimits and resetBudgets) and then batch persist them using t.db.Save() with proper error aggregation in the PerformStartupResets method.
Applied to files:
plugins/redis/go.mod
📚 Learning: 2025-06-16T04:29:53.409Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#81
File: tests/core-providers/openai_test.go:1-2
Timestamp: 2025-06-16T04:29:53.409Z
Learning: In the Bifrost project, the user prefers to use `package main` for test files in the tests/core-providers directory rather than more descriptive package names like `coreproviders_test`.
Applied to files:
plugins/redis/go.mod
📚 Learning: 2025-06-16T09:16:15.634Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#82
File: tests/transports-integrations/tests/integrations/test_litellm.py:97-115
Timestamp: 2025-06-16T09:16:15.634Z
Learning: In the Bifrost integration tests, the user prefers tests to fail hard when API keys are missing rather than using skip_if_no_api_key decorators for graceful skipping. This applies to LiteLLM tests that depend on OpenAI API keys.
Applied to files:
plugins/redis/plugin_test.goplugins/redis/main.go
📚 Learning: 2025-06-26T07:37:24.385Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#131
File: transports/bifrost-http/lib/config.go:35-68
Timestamp: 2025-06-26T07:37:24.385Z
Learning: In the Bifrost project's MCP configuration handling, empty environment variables should be treated as missing/invalid rather than as valid empty values. The current implementation using `os.Getenv(envKey); envValue != ""` to check for non-empty values is the desired behavior.
Applied to files:
plugins/redis/plugin_test.goplugins/redis/main.go
📚 Learning: 2025-06-16T04:55:11.886Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#81
File: tests/core-providers/scenarios/automatic_function_calling.go:22-22
Timestamp: 2025-06-16T04:55:11.886Z
Learning: In the Bifrost test suite (tests/core-providers), parallel tests using t.Parallel() are not being implemented currently. The team plans to add parallel test execution in future enhancements.
Applied to files:
plugins/redis/plugin_test.go
📚 Learning: 2025-06-16T04:25:00.816Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#81
File: tests/core-providers/config/account.go:55-101
Timestamp: 2025-06-16T04:25:00.816Z
Learning: In the Bifrost test account implementation, the user prefers to let Bifrost itself handle missing API key errors rather than adding early validation in the GetKeysForProvider method.
Applied to files:
plugins/redis/plugin_test.go
📚 Learning: 2025-06-04T04:58:12.239Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#55
File: core/tests/e2e_tool_test.go:29-30
Timestamp: 2025-06-04T04:58:12.239Z
Learning: In the Bifrost project, environment variables should be used only for secrets (like API keys), not for general configuration. Test parameters like provider and model can be hardcoded at the start of test files for predictability and consistency.
Applied to files:
plugins/redis/plugin_test.go
📚 Learning: 2025-08-04T22:19:12.975Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#212
File: transports/bifrost-http/integrations/openai/types.go:250-288
Timestamp: 2025-08-04T22:19:12.975Z
Learning: In transports/bifrost-http/integrations/openai/types.go, the ConvertToBifrostRequest function for OpenAIEmbeddingRequest handles []interface{} input without validation for non-string elements because Pratham-Mishra04 confirmed this case will never happen in practice, consistent with their preference to avoid defensive programming for unreachable scenarios.
Applied to files:
plugins/redis/plugin_test.goplugins/redis/main.go
📚 Learning: 2025-08-01T16:25:49.937Z
Learnt from: TejasGhatte
PR: maximhq/bifrost#174
File: transports/bifrost-http/plugins/logging/operations.go:113-114
Timestamp: 2025-08-01T16:25:49.937Z
Learning: In the Bifrost logging plugin (transports/bifrost-http/plugins/logging/operations.go), the calculateLatency method is only called in two specific scenarios: when ErrorDetails is present (error case) and when isFinalChunk is true (end of streaming). It is not called repeatedly during streaming, making performance optimization with caching unnecessary.
Applied to files:
plugins/redis/plugin_test.goplugins/redis/main.go
📚 Learning: 2025-08-04T19:39:39.417Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#209
File: plugins/jsonparser/plugin_test.go:38-45
Timestamp: 2025-08-04T19:39:39.417Z
Learning: In the JSON Parser plugin (plugins/jsonparser/main.go), the PostHook method is designed with a contract that guarantees the response structure (Choices, BifrostStreamResponseChoice, Delta, Content) will always be present when called, making defensive nil checks unnecessary in tests.
Applied to files:
plugins/redis/main.go
📚 Learning: 2025-08-08T07:07:08.801Z
Learnt from: TejasGhatte
PR: maximhq/bifrost#219
File: core/providers/openai.go:459-459
Timestamp: 2025-08-08T07:07:08.801Z
Learning: In core/providers/utils.go, processAndSendResponse and processAndSendError now take exactly five parameters: (ctx, postHookRunner, response|err, responseChan, logger). All providers (OpenAI, Anthropic, Cohere, Bedrock) must call these with the logger as the fifth argument; a repo-wide check in PR maximhq/bifrost#219 confirmed this is already consistent.
Applied to files:
plugins/redis/main.go
📚 Learning: 2025-06-14T04:06:58.240Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#79
File: core/bifrost.go:94-103
Timestamp: 2025-06-14T04:06:58.240Z
Learning: In core/bifrost.go, the count parameter in RunPostHooks method is intentionally kept separate from p.executedPreHooks to support circuit breaker plugins that may need to trigger PostHooks for only a subset of executed plugins when detecting failure conditions mid-execution.
Applied to files:
plugins/redis/main.go
📚 Learning: 2025-06-04T09:22:18.123Z
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.
Applied to files:
plugins/redis/main.go
📚 Learning: 2025-08-05T20:43:59.593Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#214
File: core/providers/azure.go:49-50
Timestamp: 2025-08-05T20:43:59.593Z
Learning: In core/providers/azure.go, the azureTextCompletionResponsePool should use AzureTextResponse type, not schemas.BifrostResponse, to maintain consistency with the acquireAzureTextResponse() and releaseAzureTextResponse() functions that work with *AzureTextResponse objects.
Applied to files:
plugins/redis/main.go
📚 Learning: 2025-08-07T13:33:07.837Z
Learnt from: TejasGhatte
PR: maximhq/bifrost#219
File: plugins/jsonparser/main.go:0-0
Timestamp: 2025-08-07T13:33:07.837Z
Learning: In the JSON Parser plugin (plugins/jsonparser/main.go), TejasGhatte prefers to return responses as-is (skip processing) when a request ID cannot be determined, rather than generating fallback IDs. The getRequestID method should return an empty string when no proper identifier is found, allowing the PostHook method's early return logic to skip accumulation and processing.
Applied to files:
plugins/redis/main.go
📚 Learning: 2025-08-02T13:02:13.642Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#174
File: transports/bifrost-http/lib/models.go:251-254
Timestamp: 2025-08-02T13:02:13.642Z
Learning: In the Bifrost project's database models (transports/bifrost-http/lib/models.go), Pratham-Mishra04 considers meta config types to be predefined and expects that unknown meta config types will never occur in practice. The current implementation that returns nil for unknown types in the AfterFind hook is intentional, as defensive programming for this scenario is considered unnecessary.
Applied to files:
plugins/redis/main.go
📚 Learning: 2025-07-08T16:50:27.699Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#144
File: transports/bifrost-http/handlers/providers.go:45-49
Timestamp: 2025-07-08T16:50:27.699Z
Learning: In the Bifrost project, breaking API changes are acceptable when features are not yet public. This applies to scenarios like changing struct fields from pointer to non-pointer types in request/response structures for unreleased features.
Applied to files:
plugins/redis/main.go
📚 Learning: 2025-08-08T06:37:43.094Z
Learnt from: TejasGhatte
PR: maximhq/bifrost#219
File: plugins/jsonparser/main.go:175-196
Timestamp: 2025-08-08T06:37:43.094Z
Learning: In plugins/jsonparser/main.go (JSON Parser plugin), TejasGhatte prefers not to introduce a max-bytes cap on per-request accumulation (accumulateContent) because truncation or skipping would lose the plugin’s desired behavior of returning completed JSON. If memory limits are needed, consider optional non-lossy strategies (soft-limit logging or spill-to-disk), disabled by default.
Applied to files:
plugins/redis/main.go
📚 Learning: 2025-06-09T16:35:26.914Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#64
File: transports/bifrost-http/integrations/genai/types.go:273-313
Timestamp: 2025-06-09T16:35:26.914Z
Learning: In convertGenerationConfigToParams method in transports/bifrost-http/integrations/genai/types.go, pre-allocating the ExtraParams map is preferred over lazy allocation because the method has multiple potential ExtraParams assignments, making the computational overhead of conditional checks exceed the memory savings of an empty map.
Applied to files:
plugins/redis/main.go
📚 Learning: 2025-07-08T18:30:08.258Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#141
File: core/bifrost.go:198-272
Timestamp: 2025-07-08T18:30:08.258Z
Learning: Pratham-Mishra04 follows a pattern of implementing core functionality first and deferring non-critical improvements (like race condition fixes, optimizations) to later PRs. This is a reasonable development approach that prioritizes getting the main feature working before addressing edge cases.
Applied to files:
plugins/redis/main.go
📚 Learning: 2025-08-03T20:36:21.906Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#202
File: transports/bifrost-http/plugins/governance/resolver.go:183-191
Timestamp: 2025-08-03T20:36:21.906Z
Learning: In the Bifrost governance plugin (transports/bifrost-http/plugins/governance/resolver.go), Pratham-Mishra04 considers the current string matching approach for determining rate limit violation decision types (DecisionTokenLimited vs DecisionRequestLimited) adequate and not important enough to refactor with more robust explicit checks, preferring functional simplicity over theoretical robustness improvements.
Applied to files:
plugins/redis/main.go
📚 Learning: 2025-06-15T16:05:13.489Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#84
File: transports/bifrost-http/main.go:2-2
Timestamp: 2025-06-15T16:05:13.489Z
Learning: For the Bifrost project, HTTP transport integration routers for new providers (like Mistral and Ollama) are implemented in separate PRs from the core provider support, following a focused PR strategy.
Applied to files:
plugins/redis/main.go
📚 Learning: 2025-08-04T11:27:45.020Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#208
File: transports/bifrost-http/handlers/completions.go:221-225
Timestamp: 2025-08-04T11:27:45.020Z
Learning: In transports/bifrost-http/handlers/completions.go, Pratham-Mishra04 intentionally delegates file size validation for audio transcription uploads to the downstream API/provider rather than enforcing MaxFileSize limits at the HTTP transport layer. This follows their architectural preference of keeping the transport layer simple and letting providers handle domain-specific validation concerns, even for potential memory exhaustion scenarios.
Applied to files:
plugins/redis/main.go
📚 Learning: 2025-08-03T20:17:11.876Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#202
File: transports/bifrost-http/plugins/governance/resolver.go:68-74
Timestamp: 2025-08-03T20:17:11.876Z
Learning: In the Bifrost governance plugin (transports/bifrost-http/plugins/governance/resolver.go), Pratham-Mishra04 considers nil validation in constructors like NewBudgetResolver unnecessary because the parameters will never be nil in practice. This aligns with their general preference to avoid defensive programming for scenarios they consider unreachable.
Applied to files:
plugins/redis/main.go
📚 Learning: 2025-08-03T20:28:00.857Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#202
File: transports/bifrost-http/handlers/governance.go:1038-1049
Timestamp: 2025-08-03T20:28:00.857Z
Learning: In the Bifrost governance handler (transports/bifrost-http/handlers/governance.go), Pratham-Mishra04 is comfortable with exposing full error details including err.Error() in production responses for governance API endpoints, prioritizing simplicity over potential information disclosure concerns.
Applied to files:
plugins/redis/main.go
📚 Learning: 2025-07-16T07:13:29.496Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#162
File: transports/bifrost-http/integrations/genai/types.go:0-0
Timestamp: 2025-07-16T07:13:29.496Z
Learning: Pratham-Mishra04 prefers to avoid redundant error handling across architectural layers in the Bifrost streaming implementation. When error handling (such as timeouts, context cancellation, and JSON marshaling failures) is already handled at the provider level, they prefer not to duplicate this logic at the transport integration layer to keep the code simple and avoid unnecessary complexity.
Applied to files:
plugins/redis/main.go
📚 Learning: 2025-06-10T11:00:02.875Z
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.
Applied to files:
plugins/redis/main.go
📚 Learning: 2025-08-07T13:33:18.094Z
Learnt from: TejasGhatte
PR: maximhq/bifrost#219
File: plugins/jsonparser/main.go:0-0
Timestamp: 2025-08-07T13:33:18.094Z
Learning: In the JSON Parser plugin (plugins/jsonparser/main.go), the context key for enabling per-request JSON parsing is "enable-streaming-json-parser" (defined as EnableStreamingJSONParser constant), which is distinct from the plugin name constant to avoid confusion between plugin identification and per-request activation control.
Applied to files:
plugins/redis/main.go
📚 Learning: 2025-08-08T06:51:27.123Z
Learnt from: TejasGhatte
PR: maximhq/bifrost#219
File: core/schemas/bifrost.go:686-692
Timestamp: 2025-08-08T06:51:27.123Z
Learning: In Bifrost, BifrostError.StreamControl is plugin-only (excluded from JSON) and used by processAndSendResponse to optionally skip a stream chunk and optionally log; default semantics: StreamControl == nil means no overrides; individual pointer fields (LogError, SkipStream) are treated as false when nil and have effect only when set to true.
Applied to files:
plugins/redis/main.go
📚 Learning: 2025-08-08T06:49:53.899Z
Learnt from: TejasGhatte
PR: maximhq/bifrost#219
File: docs/usage/go-package/schemas.md:740-745
Timestamp: 2025-08-08T06:49:53.899Z
Learning: In BifrostError (core/schemas/bifrost.go), StreamControl is an internal, non-serialized (*json:"-") control used by plugins. If StreamControl (or its *bool members) is nil, no overrides are applied—core default behavior proceeds (no implicit logging or skipping). A flag must be explicitly set to true to change behavior; used by processAndSendResponse in core/providers/utils.go.
Applied to files:
plugins/redis/main.go
📚 Learning: 2025-08-08T15:31:28.737Z
Learnt from: TejasGhatte
PR: maximhq/bifrost#238
File: transports/bifrost-http/plugins/logging/main.go:687-703
Timestamp: 2025-08-08T15:31:28.737Z
Learning: In transports/bifrost-http/plugins/logging/main.go, TejasGhatte prefers keeping separate helpers for streaming detection: isStreamingResponse (any streaming) and isTextStreamingResponse (text-only). Avoid parameterizing/merging them, as that adds code and reduces clarity.
Applied to files:
plugins/redis/main.go
🪛 LanguageTool
plugins/redis/README.md
[grammar] ~1-~1: Use correct spacing
Context: # Redis Cache Plugin for Bifrost This plugin provides Redis-based caching...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~3-~3: Use correct spacing
Context: ... improving performance and reducing API costs. ## Features - High-Performance Hashing...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[typographical] ~11-~11: To join two clauses or set off examples, consider using an em dash.
Context: ...- Simple Setup: Only requires Redis address - sensible defaults for everything else - ...
(QB_NEW_EN_DASH_RULE_EM)
[grammar] ~12-~12: Use correct spacing
Context: ...ed**: Creates and manages its own Redis client ## Installation ```bash go get github.com/...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~21-~21: Use correct spacing
Context: ...mhq/bifrost/plugins/redis ## Quick Start ### Basic Setup go import ( "github....
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~23-~23: Use correct spacing
Context: ...ns/redis ## Quick Start ### Basic Setup go import ( "github.com/maximhq/bifrost/plugins/redis" bifrost "github.com/maximhq/bifrost/core" ) // Simple configuration - only Redis address is required! config := redis.RedisPluginConfig{ Addr: "localhost:6379", // Your Redis server address } // Create the plugin plugin, err := redis.NewRedisPlugin(config, logger) if err != nil { log.Fatal("Failed to create Redis plugin:", err) } // Use with Bifrost bifrostConfig := schemas.BifrostConfig{ Account: yourAccount, Plugins: []schemas.Plugin{plugin}, // ... other config } ``` That's it! The plugin uses Redis client ...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~50-~50: There might be a mistake here.
Context: ...nection handling and these defaults for caching: - TTL: 5 minutes - **CacheOnlySuccessfu...
(QB_NEW_EN_OTHER)
[grammar] ~55-~55: Use correct spacing
Context: ...ider**: true (include provider in cache key) ### With Password Authentication ```go conf...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~57-~57: Use correct spacing
Context: ...ovider in cache key) ### With Password Authentication go config := redis.RedisPluginConfig{ Addr: "localhost:6379", Password: "your-redis-password", } ### With Custom TTL and Prefix ```go config...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~66-~66: Use correct spacing
Context: ...ssword", } ### With Custom TTL and Prefix go config := redis.RedisPluginConfig{ Addr: "localhost:6379", TTL: time.Hour, // Cache for 1 hour Prefix: "myapp:cache:", // Custom prefix } ### With Different Database go config :=...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~76-~76: Use correct spacing
Context: ...Custom prefix } ### With Different Database go config := redis.RedisPluginConfig{ Addr: "localhost:6379", DB: 1, // Use Redis database 1 } ``` ### Cache All Responses (Including Errors) ...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~85-~85: Use correct spacing
Context: ... ### Cache All Responses (Including Errors) go config := redis.RedisPluginConfig{ Addr: "localhost:6379", CacheOnlySuccessful: bifrost.Ptr(false), // Cache both successful and error responses } ### Custom Cache Key Configuration go co...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~94-~94: Use correct spacing
Context: ...r responses } ### Custom Cache Key Configuration go config := redis.RedisPluginConfig{ Addr: "localhost:6379", CacheByModel: bifrost.Ptr(false), // Don't include model in cache key CacheByProvider: bifrost.Ptr(true), // Include provider in cache key } ### Custom Redis Client Configuration go...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~104-~104: Use correct spacing
Context: ...ache key } ### Custom Redis Client Configuration go config := redis.RedisPluginConfig{ Addr: "localhost:6379", PoolSize: 20, // Custom connection pool size DialTimeout: 5 * time.Second, // Custom connection timeout ReadTimeout: 3 * time.Second, // Custom read timeout ConnMaxLifetime: time.Hour, // Custom connection lifetime } ``` ## Configuration Options | Option ...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~116-~116: Use correct spacing
Context: ...ection lifetime } ``` ## Configuration Options | Option | Type ...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~128-~128: There might be a mistake here.
Context: ... | Include provider in cache key | Redis Connection Options (all optiona...
(QB_NEW_EN_OTHER)
[grammar] ~130-~130: There might be a problem here.
Context: ... | Redis Connection Options (all optional, Redis client uses its own defaults for zero v...
(QB_NEW_EN_MERGED_MATCH)
[grammar] ~130-~130: Use correct spacing
Context: ...s client uses its own defaults for zero values): - PoolSize, MinIdleConns, MaxIdleConns - Conne...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~132-~132: There might be a mistake here.
Context: ...values): - PoolSize, MinIdleConns, MaxIdleConns - Connection pool settings - `ConnMaxL...
(QB_NEW_EN_OTHER)
[grammar] ~133-~133: There might be a mistake here.
Context: ...tion pool settings - ConnMaxLifetime, ConnMaxIdleTime - Connection lifetime settings - `Dial...
(QB_NEW_EN_OTHER)
[grammar] ~134-~134: There might be a mistake here.
Context: ...ettings - DialTimeout, ReadTimeout, WriteTimeout - Timeout settings All Redis configur...
(QB_NEW_EN_OTHER)
[grammar] ~134-~134: Use correct spacing
Context: ...ReadTimeout, WriteTimeout - Timeout settings All Redis configuration values are passe...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~136-~136: Use correct spacing
Context: ... you want to override from Redis client defaults. ## How It Works The plugin generates an xx...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~138-~138: Use correct spacing
Context: ... from Redis client defaults. ## How It Works The plugin generates an xxhash of the no...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~140-~140: Use correct spacing
Context: ...tes an xxhash of the normalized request including: - Provider (if CacheByProvider is true) - ...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~145-~145: Use correct spacing
Context: ...completion) - Parameters (includes tool calls) Identical requests will always produce t...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~147-~147: Use correct spacing
Context: ...oduce the same hash, enabling effective caching. ### Caching Flow 1. PreHook: Checks Red...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~149-~149: Use correct spacing
Context: ...nabling effective caching. ### Caching Flow 1. PreHook: Checks Redis for cached respo...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~151-~151: There might be a mistake here.
Context: ...cached response, returns immediately if found 2. PostHook: Stores the response in Redis...
(QB_NEW_EN_OTHER)
[grammar] ~152-~152: There might be a mistake here.
Context: ...es the response in Redis asynchronously (non-blocking) 3. Cleanup: Clears all cached entries and...
(QB_NEW_EN_OTHER)
[grammar] ~153-~153: There might be a mistake here.
Context: ...cached entries and closes connection on shutdown Asynchronous Caching: Cache writes hap...
(QB_NEW_EN_OTHER)
[grammar] ~155-~155: Use correct spacing
Context: ...mal performance while maintaining cache functionality. ### Cache Keys Cache keys follow the patter...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~157-~157: Use correct spacing
Context: ...taining cache functionality. ### Cache Keys Cache keys follow the pattern: `{prefix}...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~159-~159: There might be a problem here.
Context: ... ### Cache Keys Cache keys follow the pattern: {prefix}{xxhash} Example: bifrost:cache:a1b2c3d4e5f6... ## Testing Run the tests with a Redis inst...
(QB_NEW_EN_MERGED_MATCH)
[grammar] ~165-~165: There might be a mistake here.
Context: ...ng Run the tests with a Redis instance running: bash # Start Redis (using Docker) docker run -d -p 6379:6379 redis:latest # Run tests go test ./... Tests will be skipped if Redis ...
(QB_NEW_EN_OTHER)
[grammar] ~175-~175: Use correct spacing
Context: ... Tests will be skipped if Redis is not available. ## Performance Benefits - **Reduced API Ca...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~177-~177: Use correct spacing
Context: ...Redis is not available. ## Performance Benefits - Reduced API Calls: Identical requests ...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~179-~179: There might be a mistake here.
Context: ...s**: Identical requests are served from cache - Ultra-Low Latency: Cache hits return i...
(QB_NEW_EN_OTHER)
[grammar] ~180-~180: There might be a mistake here.
Context: ...Ultra-Low Latency*: Cache hits return immediately, cache writes are non-blocking - **Cost ...
(QB_NEW_EN_OTHER)
[grammar] ~180-~180: There might be a mistake here.
Context: ...ts return immediately, cache writes are non-blocking - Cost Savings: Fewer API calls to ...
(QB_NEW_EN_OTHER)
[grammar] ~181-~181: There might be a mistake here.
Context: ...ngs**: Fewer API calls to expensive LLM providers - Improved Reliability: Cached responses...
(QB_NEW_EN_OTHER)
[grammar] ~182-~182: Insert the missing word
Context: ...ders - Improved Reliability: Cached responses available even if provider is down - **...
(QB_NEW_EN_OTHER_ERROR_IDS_32)
[grammar] ~182-~182: Use articles correctly
Context: ...y**: Cached responses available even if provider is down - High Throughput: Asynchro...
(QB_NEW_EN_OTHER_ERROR_IDS_11)
[grammar] ~182-~182: There might be a mistake here.
Context: ...responses available even if provider is down - High Throughput: Asynchronous caching ...
(QB_NEW_EN_OTHER)
[grammar] ~183-~183: There might be a mistake here.
Context: ...hronous caching doesn't impact response times ## Error Handling The plugin is designed t...
(QB_NEW_EN_OTHER)
[grammar] ~185-~185: Use correct spacing
Context: ...doesn't impact response times ## Error Handling The plugin is designed to fail gracefull...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~187-~187: There might be a mistake here.
Context: ...andling The plugin is designed to fail gracefully: - If Redis is unavailable during startup,...
(QB_NEW_EN_OTHER)
[grammar] ~189-~189: There might be a problem here.
Context: ...ing startup, plugin creation fails with clear error - If Redis becomes unavailable during oper...
(QB_NEW_EN_MERGED_MATCH)
[grammar] ~190-~190: There might be a mistake here.
Context: ...ng operation, requests continue without caching - If cache retrieval fails, requests proce...
(QB_NEW_EN_OTHER)
[grammar] ~191-~191: There might be a mistake here.
Context: ...cache retrieval fails, requests proceed normally - If cache storage fails asynchronously, r...
(QB_NEW_EN_OTHER)
[grammar] ~192-~192: There might be a mistake here.
Context: ...usly, responses are unaffected (already returned) - Malformed cached data is ignored and req...
(QB_NEW_EN_OTHER)
[grammar] ~193-~193: There might be a mistake here.
Context: ...ed data is ignored and requests proceed normally - Cache operations have timeouts to preven...
(QB_NEW_EN_OTHER)
[grammar] ~194-~194: There might be a mistake here.
Context: ...tions have timeouts to prevent resource leaks ## Best Practices 1. Start Simple: Use...
(QB_NEW_EN_OTHER)
[grammar] ~196-~196: Use correct spacing
Context: ...outs to prevent resource leaks ## Best Practices 1. Start Simple: Use only Addr and let ...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~198-~198: There might be a mistake here.
Context: ...only Addr and let defaults handle the rest 2. Set appropriate TTL: Balance between c...
(QB_NEW_EN_OTHER)
[grammar] ~199-~199: There might be a mistake here.
Context: ...lance between cache efficiency and data freshness 3. Use meaningful prefixes: Helps organiz...
(QB_NEW_EN_OTHER)
[grammar] ~200-~200: There might be a mistake here.
Context: ...lps organize cache keys in shared Redis instances 4. Monitor Redis memory: Track cache usag...
(QB_NEW_EN_OTHER)
[grammar] ~201-~201: There might be a mistake here.
Context: ...mory**: Track cache usage in production environments 5. Use bifrost.Ptr(): For boolean point...
(QB_NEW_EN_OTHER)
[grammar] ~202-~202: Use correct spacing
Context: ...)`**: For boolean pointer configuration options ## Security Considerations - **Sensitive D...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~204-~204: Use correct spacing
Context: ...nter configuration options ## Security Considerations - Sensitive Data: Be cautious about cach...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~206-~206: There might be a mistake here.
Context: ... caching responses containing sensitive information - Redis Security: Use authentication and...
(QB_NEW_EN_OTHER)
[grammar] ~207-~207: There might be a mistake here.
Context: ...authentication and network security for Redis - Data Isolation: Use different Redis da...
(QB_NEW_EN_OTHER)
[grammar] ~208-~208: There might be a mistake here.
Context: ...dis databases or prefixes for different environments
(QB_NEW_EN_OTHER)
🔇 Additional comments (1)
plugins/redis/plugin_test.go (1)
30-38: Skip test whenOPENAI_API_KEYis unsetIf the env var is empty the request will hit OpenAI unauthenticated and fail.
Add:if os.Getenv("OPENAI_API_KEY") == "" { t.Skip("OPENAI_API_KEY not set – skipping") }
Pratham-Mishra04
force-pushed
the
06-11-_plugins_feat_redis_caching_plugin_added
branch
2 times, most recently
from
3842b88 to
8efa472
Compare
Pratham-Mishra04
force-pushed
the
06-11-_plugins_feat_redis_caching_plugin_added
branch
from
8efa472 to
e524738
Compare
Pratham-Mishra04
force-pushed
the
08-09-feat-extended_plugins_for_better_stream_control
branch
from
ed1376e to
98e1451
Compare
There was a problem hiding this comment.
Actionable comments posted: 10
♻️ Duplicate comments (2)
plugins/redis/go.mod (2)
3-3: Verifygodirective; prefer major.minor (duplicate of earlier bot note).Go tooling expects
goto bemajor.minor. If your toolchain allows1.24.1in this repo, fine—but standard practice is1.24. Consider aligning unless you have a concrete need for the patch in the directive.-go 1.24.1 +go 1.24If you want to keep 1.24.1, run the module tooling in CI to ensure no breakage:
#!/bin/bash set -e cd plugins/redis go version go mod tidy git --no-pager diff --name-only
13-50: Trim manual indirects; letgo mod tidymanage them.Maintaining a hand-written indirect list is noisy and error-prone. Drop the whole indirect require block and run
go mod tidyinplugins/redis/.require ( github.com/cespare/xxhash/v2 v2.3.0 github.com/maximhq/bifrost/core v1.1.16 github.com/redis/go-redis/v9 v9.10.0 ) replace github.com/maximhq/bifrost/core => ../../core -require ( - cloud.google.com/go/compute/metadata v0.3.0 // indirect - ... - golang.org/x/text v0.24.0 // indirect -)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
plugins/redis/go.sumis excluded by!**/*.sum
📒 Files selected for processing (4)
plugins/redis/README.md(1 hunks)plugins/redis/go.mod(1 hunks)plugins/redis/main.go(1 hunks)plugins/redis/plugin_test.go(1 hunks)
🧰 Additional context used
🧠 Learnings (21)
📓 Common learnings
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#55
File: core/providers/anthropic.go:358-388
Timestamp: 2025-06-04T05:37:59.699Z
Learning: User Pratham-Mishra04 prefers not to extract small code duplications (around 2 lines) into helper functions, considering the overhead not worth it for such minor repetition.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#102
File: README.md:62-66
Timestamp: 2025-06-19T17:03:03.639Z
Learning: Pratham-Mishra04 prefers using the implicit 'latest' tag for the maximhq/bifrost Docker image rather than pinning to specific versions.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#143
File: core/mcp.go:155-196
Timestamp: 2025-07-08T15:33:47.698Z
Learning: Pratham-Mishra04 prefers not to add explanatory comments for obvious code patterns, such as the unlock/lock strategy around network I/O operations, considering them self-explanatory to experienced developers.
📚 Learning: 2025-06-20T16:21:18.912Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#103
File: .github/workflows/transport-dependency-update.yml:53-75
Timestamp: 2025-06-20T16:21:18.912Z
Learning: In the bifrost repository's transport dependency update workflow, when updating the core dependency to a new version using `go get`, the go.mod and go.sum files will always change in normal operation, making the safety check for changes more of a defensive programming practice rather than handling a common scenario.
Applied to files:
plugins/redis/go.mod
📚 Learning: 2025-06-16T04:27:53.538Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#81
File: tests/core-providers/go.mod:3-4
Timestamp: 2025-06-16T04:27:53.538Z
Learning: In Go module files, `go 1.24.1` (with patch version) can work fine in some setups, contrary to the general rule that go directives should only include major.minor versions.
Applied to files:
plugins/redis/go.mod
📚 Learning: 2025-06-16T03:55:16.949Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#81
File: tests/core-providers/go.mod:3-4
Timestamp: 2025-06-16T03:55:16.949Z
Learning: Go 1.24 was released in February 2025 and is stable and available for use in go.mod files.
Applied to files:
plugins/redis/go.mod
📚 Learning: 2025-06-16T09:16:15.634Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#82
File: tests/transports-integrations/tests/integrations/test_litellm.py:97-115
Timestamp: 2025-06-16T09:16:15.634Z
Learning: In the Bifrost integration tests, the user prefers tests to fail hard when API keys are missing rather than using skip_if_no_api_key decorators for graceful skipping. This applies to LiteLLM tests that depend on OpenAI API keys.
Applied to files:
plugins/redis/plugin_test.go
📚 Learning: 2025-08-08T14:12:14.754Z
Learnt from: TejasGhatte
PR: maximhq/bifrost#238
File: transports/bifrost-http/plugins/logging/main.go:0-0
Timestamp: 2025-08-08T14:12:14.754Z
Learning: In transports/bifrost-http/plugins/logging/main.go, stream accumulator cleanup is intended to use a 5-minute TTL (consistent with processing logs). Comments should state “older than 5 minutes,” and using central constants (processingLogTTL, streamAccumulatorTTL) helps avoid future mismatches.
Applied to files:
plugins/redis/main.go
📚 Learning: 2025-07-08T18:30:08.258Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#141
File: core/bifrost.go:198-272
Timestamp: 2025-07-08T18:30:08.258Z
Learning: Pratham-Mishra04 follows a pattern of implementing core functionality first and deferring non-critical improvements (like race condition fixes, optimizations) to later PRs. This is a reasonable development approach that prioritizes getting the main feature working before addressing edge cases.
Applied to files:
plugins/redis/main.go
📚 Learning: 2025-08-03T20:36:21.906Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#202
File: transports/bifrost-http/plugins/governance/resolver.go:183-191
Timestamp: 2025-08-03T20:36:21.906Z
Learning: In the Bifrost governance plugin (transports/bifrost-http/plugins/governance/resolver.go), Pratham-Mishra04 considers the current string matching approach for determining rate limit violation decision types (DecisionTokenLimited vs DecisionRequestLimited) adequate and not important enough to refactor with more robust explicit checks, preferring functional simplicity over theoretical robustness improvements.
Applied to files:
plugins/redis/main.go
📚 Learning: 2025-06-15T16:05:13.489Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#84
File: transports/bifrost-http/main.go:2-2
Timestamp: 2025-06-15T16:05:13.489Z
Learning: For the Bifrost project, HTTP transport integration routers for new providers (like Mistral and Ollama) are implemented in separate PRs from the core provider support, following a focused PR strategy.
Applied to files:
plugins/redis/main.go
📚 Learning: 2025-08-04T11:27:45.020Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#208
File: transports/bifrost-http/handlers/completions.go:221-225
Timestamp: 2025-08-04T11:27:45.020Z
Learning: In transports/bifrost-http/handlers/completions.go, Pratham-Mishra04 intentionally delegates file size validation for audio transcription uploads to the downstream API/provider rather than enforcing MaxFileSize limits at the HTTP transport layer. This follows their architectural preference of keeping the transport layer simple and letting providers handle domain-specific validation concerns, even for potential memory exhaustion scenarios.
Applied to files:
plugins/redis/main.go
📚 Learning: 2025-08-03T20:17:11.876Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#202
File: transports/bifrost-http/plugins/governance/resolver.go:68-74
Timestamp: 2025-08-03T20:17:11.876Z
Learning: In the Bifrost governance plugin (transports/bifrost-http/plugins/governance/resolver.go), Pratham-Mishra04 considers nil validation in constructors like NewBudgetResolver unnecessary because the parameters will never be nil in practice. This aligns with their general preference to avoid defensive programming for scenarios they consider unreachable.
Applied to files:
plugins/redis/main.go
📚 Learning: 2025-08-03T20:28:00.857Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#202
File: transports/bifrost-http/handlers/governance.go:1038-1049
Timestamp: 2025-08-03T20:28:00.857Z
Learning: In the Bifrost governance handler (transports/bifrost-http/handlers/governance.go), Pratham-Mishra04 is comfortable with exposing full error details including err.Error() in production responses for governance API endpoints, prioritizing simplicity over potential information disclosure concerns.
Applied to files:
plugins/redis/main.go
📚 Learning: 2025-07-16T07:13:29.496Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#162
File: transports/bifrost-http/integrations/genai/types.go:0-0
Timestamp: 2025-07-16T07:13:29.496Z
Learning: Pratham-Mishra04 prefers to avoid redundant error handling across architectural layers in the Bifrost streaming implementation. When error handling (such as timeouts, context cancellation, and JSON marshaling failures) is already handled at the provider level, they prefer not to duplicate this logic at the transport integration layer to keep the code simple and avoid unnecessary complexity.
Applied to files:
plugins/redis/main.go
📚 Learning: 2025-06-09T16:35:26.914Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#64
File: transports/bifrost-http/integrations/genai/types.go:273-313
Timestamp: 2025-06-09T16:35:26.914Z
Learning: In convertGenerationConfigToParams method in transports/bifrost-http/integrations/genai/types.go, pre-allocating the ExtraParams map is preferred over lazy allocation because the method has multiple potential ExtraParams assignments, making the computational overhead of conditional checks exceed the memory savings of an empty map.
Applied to files:
plugins/redis/main.go
📚 Learning: 2025-08-08T07:07:08.801Z
Learnt from: TejasGhatte
PR: maximhq/bifrost#219
File: core/providers/openai.go:459-459
Timestamp: 2025-08-08T07:07:08.801Z
Learning: In core/providers/utils.go, processAndSendResponse and processAndSendError now take exactly five parameters: (ctx, postHookRunner, response|err, responseChan, logger). All providers (OpenAI, Anthropic, Cohere, Bedrock) must call these with the logger as the fifth argument; a repo-wide check in PR maximhq/bifrost#219 confirmed this is already consistent.
Applied to files:
plugins/redis/main.go
📚 Learning: 2025-06-16T04:13:55.437Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#81
File: tests/core-providers/scenarios/end_to_end_tool_calling.go:43-45
Timestamp: 2025-06-16T04:13:55.437Z
Learning: In the Bifrost codebase, errors returned from client methods like ChatCompletionRequest are of type BifrostError, not the standard error interface. For testing these errors, use require.Nilf instead of require.NoErrorf since BifrostError doesn't work with the standard error assertion methods.
Applied to files:
plugins/redis/main.go
📚 Learning: 2025-06-16T04:13:42.755Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#81
File: tests/core-providers/scenarios/simple_chat.go:39-41
Timestamp: 2025-06-16T04:13:42.755Z
Learning: In the Bifrost codebase, errors returned from methods like ChatCompletionRequest are of type BifrostError (a custom error type) rather than the standard Go error interface. Therefore, require.Nilf should be used for error assertions instead of require.NoErrorf.
Applied to files:
plugins/redis/main.go
📚 Learning: 2025-06-16T04:12:05.427Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#81
File: tests/core-providers/scenarios/complete_end_to_end.go:39-41
Timestamp: 2025-06-16T04:12:05.427Z
Learning: In the Bifrost system, error returns are of type `BifrostError` rather than the standard Go `error` interface. Therefore, use `require.Nilf(t, err, ...)` instead of `require.NoError(t, err)` when checking for errors in Bifrost function calls.
Applied to files:
plugins/redis/main.go
📚 Learning: 2025-06-18T15:15:51.323Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#94
File: core/bifrost.go:823-845
Timestamp: 2025-06-18T15:15:51.323Z
Learning: In the Bifrost project, the team prioritizes maintaining consistent error handling patterns over exposing detailed error context. All errors should be wrapped in the standard `BifrostError` structure rather than creating specific error types or exposing richer error details like exit codes or stderr output.
Applied to files:
plugins/redis/main.go
📚 Learning: 2025-06-15T13:50:41.418Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#83
File: core/providers/mistral.go:96-101
Timestamp: 2025-06-15T13:50:41.418Z
Learning: In the Bifrost project, when a provider doesn't support a specific operation (like text completion), the IsBifrostError flag should be set to false to mark it as a provider-level error rather than a Bifrost framework error. This is intentional design for unsupported operations.
Applied to files:
plugins/redis/main.go
📚 Learning: 2025-06-15T14:24:49.882Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#83
File: core/providers/mistral.go:168-170
Timestamp: 2025-06-15T14:24:49.882Z
Learning: In the Bifrost codebase, performance is prioritized over defensive copying for HTTP service operations. Specifically, shallow slice assignments in provider response handling are acceptable due to object pool reset patterns and JSON unmarshaling behavior that minimize practical data corruption risks.
Applied to files:
plugins/redis/main.go
🧬 Code Graph Analysis (2)
plugins/redis/plugin_test.go (8)
core/schemas/bifrost.go (3)
OpenAI(41-41)BifrostRequest(143-153)RequestInput(57-63)core/schemas/provider.go (2)
DefaultNetworkConfig(43-48)DefaultConcurrencyAndBufferSize(57-60)plugins/redis/main.go (4)
RedisPluginConfig(22-49)NewRedisPlugin(84-147)Plugin(60-64)ContextKey(199-199)core/logger.go (1)
NewDefaultLogger(47-56)core/schemas/logger.go (2)
LogLevelDebug(9-9)Logger(18-39)core/bifrost.go (3)
Init(97-195)ChatCompletionRequest(25-25)ChatCompletionStreamRequest(26-26)core/schemas/account.go (1)
Account(47-63)core/utils.go (1)
Ptr(10-12)
plugins/redis/main.go (4)
core/utils.go (1)
Ptr(10-12)core/schemas/bifrost.go (4)
BifrostRequest(143-153)RequestInput(57-63)BifrostStream(668-671)BifrostResponse(374-387)core/schemas/plugin.go (1)
PluginShortCircuit(8-12)core/bifrost.go (5)
BifrostContextKeyRequestType(89-89)RequestType(21-21)ChatCompletionStreamRequest(26-26)SpeechStreamRequest(29-29)TranscriptionStreamRequest(31-31)
🪛 LanguageTool
plugins/redis/README.md
[grammar] ~1-~1: Use correct spacing
Context: # Redis Cache Plugin for Bifrost This plugin provides Redis-based caching...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~3-~3: Use correct spacing
Context: ... improving performance and reducing API costs. ## Features - High-Performance Hashing...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[typographical] ~13-~13: To join two clauses or set off examples, consider using an em dash.
Context: ...: Only requires Redis address and cache key - sensible defaults for everything else - ...
(QB_NEW_EN_DASH_RULE_EM)
[grammar] ~14-~14: Use correct spacing
Context: ...ed**: Creates and manages its own Redis client ## Installation ```bash go get github.com/...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~23-~23: Use correct spacing
Context: ...mhq/bifrost/plugins/redis ## Quick Start ### Basic Setup go import ( "github....
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~25-~25: Use correct spacing
Context: ...ns/redis ## Quick Start ### Basic Setup go import ( "github.com/maximhq/bifrost/plugins/redis" bifrost "github.com/maximhq/bifrost/core" ) // Simple configuration - only Redis address and cache key are required! config := redis.RedisPluginConfig{ Addr: "localhost:6379", // Your Redis server address CacheKey: "x-my-cache-key", // Context key for cache identification } // Create the plugin plugin, err := redis.NewRedisPlugin(config, logger) if err != nil { log.Fatal("Failed to create Redis plugin:", err) } // Use with Bifrost bifrostConfig := schemas.BifrostConfig{ Account: yourAccount, Plugins: []schemas.Plugin{plugin}, // ... other config } ``` That's it! The plugin uses Redis client ...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~53-~53: There might be a mistake here.
Context: ...nection handling and these defaults for caching: - TTL: 5 minutes - CacheByModel: tr...
(QB_NEW_EN_OTHER)
[grammar] ~57-~57: Use correct spacing
Context: ...ider**: true (include provider in cache key) Important: You must provide the cache ...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~59-~59: There might be a mistake here.
Context: ... in your request context for caching to work: go ctx := context.WithValue(ctx, redis.ContextKey("x-my-cache-key"), "cache-value") response, err := client.ChatCompletionRequest(ctx, request) ### With Password Authentication ```g...
(QB_NEW_EN_OTHER)
[grammar] ~66-~66: Use correct spacing
Context: ...st(ctx, request) ### With Password Authentication go config := redis.RedisPluginConfig{ Addr: "localhost:6379", CacheKey: "x-my-cache-key", Password: "your-redis-password", } ### With Custom TTL and Prefix go config...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~76-~76: Use correct spacing
Context: ...ssword", } ### With Custom TTL and Prefix go config := redis.RedisPluginConfig{ Addr: "localhost:6379", CacheKey: "x-my-cache-key", TTL: time.Hour, // Cache for 1 hour Prefix: "myapp:cache:", // Custom prefix } ``` ### Per-Request TTL Override (via Context) ...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~87-~87: Use correct spacing
Context: ... ``` ### Per-Request TTL Override (via Context) You can override the cache TTL for indiv...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~89-~89: There might be a mistake here.
Context: ...CacheTTLKey on the plugin, then set a time.Duration value at that context key before making...
(QB_NEW_EN_OTHER)
[grammar] ~89-~89: There might be a problem here.
Context: ...e at that context key before making the request. go // Configure plugin with a context key used to read per-request TTLs config := redis.RedisPluginConfig{ Addr: "localhost:6379", CacheKey: "x-my-cache-key", CacheTTLKey: "x-my-cache-ttl", // The context key for reading TTL TTL: 5 * time.Minute, // Fallback/default TTL } plugin, err := redis.NewRedisPlugin(config, logger) // ... init Bifrost client with plugin // Before making a request, set a per-request TTL ctx := context.WithValue(ctx, redis.ContextKey("x-my-cache-ttl"), 30*time.Second) resp, err := client.ChatCompletionRequest(ctx, request) Notes: - The context value must be of type `time...
(QB_NEW_EN_MERGED_MATCH)
[grammar] ~111-~111: Use correct spacing
Context: ...the same per-request TTL applies to all chunks. ### With Different Database ```go config :=...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~113-~113: Use correct spacing
Context: ...lies to all chunks. ### With Different Database go config := redis.RedisPluginConfig{ Addr: "localhost:6379", CacheKey: "x-my-cache-key", DB: 1, // Use Redis database 1 } ### Streaming Cache Example ```go // Config...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~123-~123: Use correct spacing
Context: ...s database 1 } ### Streaming Cache Example go // Configure plugin for streaming cache config := redis.RedisPluginConfig{ Addr: "localhost:6379", CacheKey: "x-stream-cache-key", TTL: 30 * time.Minute, // Cache streaming responses for 30 minutes } // Use with streaming requests ctx := context.WithValue(ctx, redis.ContextKey("x-stream-cache-key"), "stream-session-1") stream, err := client.ChatCompletionStreamRequest(ctx, request) // Subsequent identical requests will be served from cache as a reconstructed stream ### Custom Cache Key Configuration go co...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~139-~139: Use correct spacing
Context: ...ructed stream ### Custom Cache Key Configuration go config := redis.RedisPluginConfig{ Addr: "localhost:6379", CacheKey: "x-my-cache-key", CacheByModel: bifrost.Ptr(false), // Don't include model in cache key CacheByProvider: bifrost.Ptr(true), // Include provider in cache key } ### Custom Redis Client Configuration go...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~150-~150: Use correct spacing
Context: ...ache key } ### Custom Redis Client Configuration go config := redis.RedisPluginConfig{ Addr: "localhost:6379", CacheKey: "x-my-cache-key", PoolSize: 20, // Custom connection pool size DialTimeout: 5 * time.Second, // Custom connection timeout ReadTimeout: 3 * time.Second, // Custom read timeout ConnMaxLifetime: time.Hour, // Custom connection lifetime } ``` ## Configuration Options | Option ...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~163-~163: Use correct spacing
Context: ...ection lifetime } ``` ## Configuration Options | Option | Type | ...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~172-~172: Use correct spacing
Context: ... time.Minute| Time-to-live for cached responses | |Prefix |string` |...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~175-~175: Use correct spacing
Context: ... | Include provider in cache key | Redis Connection Options (all optional...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~177-~177: There might be a problem here.
Context: ... | Redis Connection Options (all optional, Redis client uses its own defaults for zero v...
(QB_NEW_EN_MERGED_MATCH)
[grammar] ~177-~177: Use correct spacing
Context: ...s client uses its own defaults for zero values): - PoolSize, MinIdleConns, MaxIdleConns - Conne...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~179-~179: There might be a mistake here.
Context: ...values): - PoolSize, MinIdleConns, MaxIdleConns - Connection pool settings - `ConnMaxL...
(QB_NEW_EN_OTHER)
[grammar] ~180-~180: There might be a mistake here.
Context: ...tion pool settings - ConnMaxLifetime, ConnMaxIdleTime - Connection lifetime settings - `Dial...
(QB_NEW_EN_OTHER)
[grammar] ~181-~181: There might be a mistake here.
Context: ...ettings - DialTimeout, ReadTimeout, WriteTimeout - Timeout settings All Redis configur...
(QB_NEW_EN_OTHER)
[grammar] ~181-~181: Use correct spacing
Context: ...ReadTimeout, WriteTimeout - Timeout settings All Redis configuration values are passe...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~183-~183: Use correct spacing
Context: ... you want to override from Redis client defaults. ## How It Works The plugin generates an xx...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~185-~185: Use correct spacing
Context: ... from Redis client defaults. ## How It Works The plugin generates an xxhash of the no...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~187-~187: Use correct spacing
Context: ...tes an xxhash of the normalized request including: - Provider (if CacheByProvider is true) - ...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~192-~192: Use correct spacing
Context: ...completion) - Parameters (includes tool calls) Identical requests will always produce t...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~194-~194: Use correct spacing
Context: ...oduce the same hash, enabling effective caching. ### Caching Flow #### Regular Requests 1. ...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~196-~196: Use correct spacing
Context: ...nabling effective caching. ### Caching Flow #### Regular Requests 1. PreHook: Checks...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~198-~198: Use correct spacing
Context: ...aching. ### Caching Flow #### Regular Requests 1. PreHook: Checks Redis for cached respo...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~200-~200: There might be a mistake here.
Context: ...cached response, returns immediately if found 2. PostHook: Stores the response in Redis...
(QB_NEW_EN_OTHER)
[grammar] ~201-~201: There might be a mistake here.
Context: ...es the response in Redis asynchronously (non-blocking) 3. Cleanup: Clears all cached entries and...
(QB_NEW_EN_OTHER)
[grammar] ~202-~202: There might be a mistake here.
Context: ...cached entries and closes connection on shutdown #### Streaming Requests 1. PreHook: Chec...
(QB_NEW_EN_OTHER)
[grammar] ~204-~204: Use correct spacing
Context: ... connection on shutdown #### Streaming Requests 1. PreHook: Checks Redis for cached chunk...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~209-~209: Use correct spacing
Context: ...ent requests get sorted chunks as a new stream Asynchronous Caching: Cache writes hap...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~211-~211: Use correct spacing
Context: ...mal performance while maintaining cache functionality. Streaming Intelligence: The plugin aut...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~213-~213: Use correct spacing
Context: ...t reconstruction of the original stream order. ### Cache Keys #### Regular Responses Cach...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~215-~215: Use correct spacing
Context: ...f the original stream order. ### Cache Keys #### Regular Responses Cache keys follow the...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~217-~217: Use correct spacing
Context: ...am order. ### Cache Keys #### Regular Responses Cache keys follow the pattern: `{prefix}...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~219-~219: There might be a problem here.
Context: ...egular Responses Cache keys follow the pattern: {prefix}{cache_value}_{xxhash} Example: bifrost:cache:my-session_a1b2c3d4e5f6... #### Streaming Responses Chunk keys follow t...
(QB_NEW_EN_MERGED_MATCH)
[grammar] ~223-~223: Use correct spacing
Context: ...ession_a1b2c3d4e5f6... #### Streaming Responses Chunk keys follow the pattern:{prefix}...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~225-~225: There might be a problem here.
Context: ...eaming Responses Chunk keys follow the pattern: {prefix}{cache_value}_{xxhash}_chunk_{index} Examples: - bifrost:cache:my-session_a1b2c3d4e5f6..._chunk_0 - bifrost:cache:my-session_a1b2c3d4e5f6..._chunk_1 - bifrost:cache:my-session_a1b2c3d4e5f6..._chunk_2 ## Manual Cache Invalidation You can ...
(QB_NEW_EN_MERGED_MATCH)
[grammar] ~233-~233: Use correct spacing
Context: ...b2c3d4e5f6..._chunk_2` ## Manual Cache Invalidation You can invalidate specific cached entri...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~235-~235: There might be a mistake here.
Context: ...acheForKey(key string)on the concreteredis.Plugin` type. This deletes the provided key and...
(QB_NEW_EN_OTHER)
[grammar] ~235-~235: Use correct spacing
Context: ...ming response, all of its chunk entries (<key>_chunk_*). ### Getting the cache key from responses - ...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~237-~237: Use correct spacing
Context: ...nk_*`). ### Getting the cache key from responses - Regular responses: When a response is ...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~239-~239: There might be a mistake here.
Context: ...from cache, the plugin adds metadata to response.ExtraFields.RawResponse: - bifrost_cached: true - bifrost_cache_key: "<prefix><cache_value>_<xxhash>" Use this bifrost_cache_key as the ...
(QB_NEW_EN_OTHER)
[grammar] ~244-~244: Use correct spacing
Context: ... the base key and pass that base key to ClearCacheForKey. ### Examples ```go // Non-streaming: clear ...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~296-~296: There might be a mistake here.
Context: ...in (by prefix), call Cleanup() during shutdown: go _ = plugin.(*redis.Plugin).Cleanup() ## Testing The plugin includes compre...
(QB_NEW_EN_OTHER)
[grammar] ~304-~304: Use correct spacing
Context: ...ts for both regular and streaming cache functionality. Run the tests with a Redis instance runn...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~306-~306: There might be a mistake here.
Context: ...y. Run the tests with a Redis instance running: bash # Start Redis (using Docker) docker run -d -p 6379:6379 redis:latest # Run all tests go test -v # Run specific tests go test -run TestRedisPlugin -v # Test regular caching go test -run TestRedisPluginStreaming -v # Test streaming cache Tests will be skipped if Re...
(QB_NEW_EN_OTHER)
[grammar] ~320-~320: There might be a mistake here.
Context: ...ed if Redis is not available. The tests validate: - Cache hit/miss behavior - Performance i...
(QB_NEW_EN_OTHER)
[grammar] ~326-~326: Use correct spacing
Context: ...d reconstruction - Provider information preservation ## Performance Benefits - **Reduced API Ca...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~328-~328: Use correct spacing
Context: ...nformation preservation ## Performance Benefits - Reduced API Calls: Identical requests ...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~330-~330: There might be a mistake here.
Context: ...s**: Identical requests are served from cache - Ultra-Low Latency: Cache hits return i...
(QB_NEW_EN_OTHER)
[grammar] ~331-~331: There might be a mistake here.
Context: ...Ultra-Low Latency*: Cache hits return immediately, cache writes are non-blocking - **Str...
(QB_NEW_EN_OTHER)
[grammar] ~331-~331: There might be a mistake here.
Context: ...ts return immediately, cache writes are non-blocking - Streaming Efficiency: Cached st...
(QB_NEW_EN_OTHER)
[grammar] ~332-~332: There might be a mistake here.
Context: ... and delivered faster than original API calls - Cost Savings: Fewer API calls to expen...
(QB_NEW_EN_OTHER)
[grammar] ~333-~333: There might be a mistake here.
Context: ...ngs**: Fewer API calls to expensive LLM providers - Improved Reliability: Cached responses...
(QB_NEW_EN_OTHER)
[grammar] ~334-~334: Insert the missing word
Context: ...ders - Improved Reliability: Cached responses available even if provider is down - **...
(QB_NEW_EN_OTHER_ERROR_IDS_32)
[grammar] ~334-~334: Use articles correctly
Context: ...y**: Cached responses available even if provider is down - High Throughput: Asynchro...
(QB_NEW_EN_OTHER_ERROR_IDS_11)
[grammar] ~334-~334: There might be a mistake here.
Context: ...responses available even if provider is down - High Throughput: Asynchronous caching ...
(QB_NEW_EN_OTHER)
[grammar] ~335-~335: There might be a mistake here.
Context: ...hronous caching doesn't impact response times - Perfect Stream Fidelity: Cached stream...
(QB_NEW_EN_OTHER)
[grammar] ~336-~336: There might be a mistake here.
Context: ...reams maintain exact chunk ordering and content ## Error Handling The plugin is designed t...
(QB_NEW_EN_OTHER)
[grammar] ~338-~338: Use correct spacing
Context: ...ct chunk ordering and content ## Error Handling The plugin is designed to fail gracefull...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~340-~340: There might be a mistake here.
Context: ...andling The plugin is designed to fail gracefully: - If Redis is unavailable during startup,...
(QB_NEW_EN_OTHER)
[grammar] ~342-~342: There might be a problem here.
Context: ...ing startup, plugin creation fails with clear error - If Redis becomes unavailable during oper...
(QB_NEW_EN_MERGED_MATCH)
[grammar] ~343-~343: There might be a mistake here.
Context: ...ng operation, requests continue without caching - If cache retrieval fails, requests proce...
(QB_NEW_EN_OTHER)
[grammar] ~344-~344: There might be a mistake here.
Context: ...cache retrieval fails, requests proceed normally - If cache storage fails asynchronously, r...
(QB_NEW_EN_OTHER)
[grammar] ~345-~345: There might be a mistake here.
Context: ...usly, responses are unaffected (already returned) - Malformed cached data is ignored and req...
(QB_NEW_EN_OTHER)
[grammar] ~346-~346: There might be a mistake here.
Context: ...ed data is ignored and requests proceed normally - Cache operations have timeouts to preven...
(QB_NEW_EN_OTHER)
[grammar] ~347-~347: There might be a mistake here.
Context: ...tions have timeouts to prevent resource leaks ## Best Practices 1. Start Simple: Use...
(QB_NEW_EN_OTHER)
[grammar] ~349-~349: Use correct spacing
Context: ...outs to prevent resource leaks ## Best Practices 1. Start Simple: Use only Addr and `Cac...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~351-~351: There might be a mistake here.
Context: .... Start Simple: Use only Addr and CacheKey - let defaults handle the rest 2. **Ch...
(QB_NEW_EN_OTHER)
[grammar] ~351-~351: There might be a mistake here.
Context: ...nd CacheKey - let defaults handle the rest 2. Choose meaningful cache keys: Use desc...
(QB_NEW_EN_OTHER)
[grammar] ~352-~352: There might be a mistake here.
Context: ...iptive context keys that identify cache sessions 3. Set appropriate TTL: Balance between c...
(QB_NEW_EN_OTHER)
[grammar] ~353-~353: There might be a mistake here.
Context: ...lance between cache efficiency and data freshness 4. Use meaningful prefixes: Helps organiz...
(QB_NEW_EN_OTHER)
[grammar] ~354-~354: There might be a mistake here.
Context: ...lps organize cache keys in shared Redis instances 5. Monitor Redis memory: Track cache usag...
(QB_NEW_EN_OTHER)
[grammar] ~355-~355: There might be a mistake here.
Context: ...streaming responses (more chunks = more storage) 6. Context management: Always provide cac...
(QB_NEW_EN_OTHER)
[grammar] ~356-~356: There might be a mistake here.
Context: ...e key in request context for caching to work 7. Use bifrost.Ptr(): For boolean point...
(QB_NEW_EN_OTHER)
[grammar] ~358-~358: There might be a mistake here.
Context: ...ons**: Longer streams create more cache entries, adjust TTL accordingly ## Security Con...
(QB_NEW_EN_OTHER)
[grammar] ~358-~358: There might be a mistake here.
Context: ...s create more cache entries, adjust TTL accordingly ## Security Considerations - **Sensitive D...
(QB_NEW_EN_OTHER)
[grammar] ~360-~360: Use correct spacing
Context: ...es, adjust TTL accordingly ## Security Considerations - Sensitive Data: Be cautious about cach...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~362-~362: There might be a mistake here.
Context: ... caching responses containing sensitive information - Redis Security: Use authentication and...
(QB_NEW_EN_OTHER)
[grammar] ~363-~363: There might be a mistake here.
Context: ...authentication and network security for Redis - Data Isolation: Use different Redis da...
(QB_NEW_EN_OTHER)
[grammar] ~364-~364: There might be a mistake here.
Context: ...dis databases or prefixes for different environments
(QB_NEW_EN_OTHER)
⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
- GitHub Check: Graphite / mergeability_check
- GitHub Check: Graphite / mergeability_check
- GitHub Check: Graphite / mergeability_check
- GitHub Check: Graphite / mergeability_check
- GitHub Check: Graphite / mergeability_check
- GitHub Check: Graphite / mergeability_check
- GitHub Check: Graphite / mergeability_check
- GitHub Check: Graphite / mergeability_check
- GitHub Check: Graphite / mergeability_check
🔇 Additional comments (4)
plugins/redis/go.mod (1)
11-11: Good: local replace to core is present.The local replace ensures the plugin builds against the in-repo core. This avoids accidental remote pulls during dev.
plugins/redis/README.md (1)
59-64: Docs correctly emphasize context-provided CacheKey.Clear and accurate with the code path using
ContextKey(config.CacheKey). Good call-out.plugins/redis/plugin_test.go (1)
31-38: Fail fast with a clearer message whenOPENAI_API_KEYis empty.Keep the repo’s fail-hard stance but make the failure explicit.
func (baseAccount *BaseAccount) GetKeysForProvider(ctx *context.Context, providerKey schemas.ModelProvider) ([]schemas.Key, error) { + if os.Getenv("OPENAI_API_KEY") == "" { + return nil, fmt.Errorf("OPENAI_API_KEY is not set") + } return []schemas.Key{ { Value: os.Getenv("OPENAI_API_KEY"), Models: []string{"gpt-4o-mini", "gpt-4-turbo"}, Weight: 1.0, }, }, nil }plugins/redis/main.go (1)
149-196: Hash key generation is fine, but name “hash” includes the session value.No change required, just noting:
generateRequestHashreturns<cacheValue>_<hash>; later you prefix withconfig.Prefix. This matches the README’s{prefix}{cache_value}_{xxhash}pattern.
Pratham-Mishra04
force-pushed
the
06-11-_plugins_feat_redis_caching_plugin_added
branch
from
e524738 to
0707466
Compare
Pratham-Mishra04
force-pushed
the
08-09-feat-extended_plugins_for_better_stream_control
branch
from
98e1451 to
756cc5e
Compare
Pratham-Mishra04
force-pushed
the
06-11-_plugins_feat_redis_caching_plugin_added
branch
from
0707466 to
c15f154
Compare
Merge activity
|
akshaydeo
changed the base branch from
08-09-feat-extended_plugins_for_better_stream_control
to
graphite-base/70
akshaydeo
dismissed stale reviews from coderabbitai[bot] and themself
The base branch was changed.
akshaydeo
force-pushed
the
06-11-_plugins_feat_redis_caching_plugin_added
branch
from
c15f154 to
d0561aa
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (7)
plugins/redis/go.mod (2)
3-3: Use a valid go directive (major.minor only).Go’s
godirective should be major.minor. Replace1.24.1with1.24to avoid toolchain issues.-go 1.24.1 +go 1.24
13-50: Trim manually-maintained indirects; letgo mod tidymanage them.The indirect block is unnecessary noise and will drift. Keep only direct deps and run
go mod tidy.require ( github.com/cespare/xxhash/v2 v2.3.0 github.com/maximhq/bifrost/core v1.1.16 github.com/redis/go-redis/v9 v9.10.0 ) replace github.com/maximhq/bifrost/core => ../../core -require ( - cloud.google.com/go/compute/metadata v0.3.0 // indirect - github.com/andybalholm/brotli v1.1.1 // indirect - github.com/aws/aws-sdk-go-v2 v1.36.3 // indirect - github.com/aws/aws-sdk-go-v2/config v1.29.14 // indirect - github.com/aws/aws-sdk-go-v2/credentials v1.17.67 // indirect - github.com/aws/aws-sdk-go-v2/feature/ec2/imds v1.16.30 // indirect - github.com/aws/aws-sdk-go-v2/internal/configsources v1.3.34 // indirect - github.com/aws/aws-sdk-go-v2/internal/endpoints/v2 v2.6.34 // indirect - github.com/aws/aws-sdk-go-v2/internal/ini v1.8.3 // indirect - github.com/aws/aws-sdk-go-v2/service/internal/accept-encoding v1.12.3 // indirect - github.com/aws/aws-sdk-go-v2/service/internal/presigned-url v1.12.15 // indirect - github.com/aws/aws-sdk-go-v2/service/sso v1.25.3 // indirect - github.com/aws/aws-sdk-go-v2/service/ssooidc v1.30.1 // indirect - github.com/aws/aws-sdk-go-v2/service/sts v1.33.19 // indirect - github.com/aws/smithy-go v1.22.3 // indirect - github.com/bytedance/sonic v1.14.0 // indirect - github.com/bytedance/sonic/loader v0.3.0 // indirect - github.com/cloudwego/base64x v0.1.5 // indirect - github.com/dgryski/go-rendezvous v0.0.0-20200823014737-9f7001d12a5f // indirect - github.com/google/uuid v1.6.0 // indirect - github.com/klauspost/compress v1.18.0 // indirect - github.com/klauspost/cpuid/v2 v2.0.9 // indirect - github.com/mark3labs/mcp-go v0.32.0 // indirect - github.com/mattn/go-colorable v0.1.13 // indirect - github.com/mattn/go-isatty v0.0.19 // indirect - github.com/rs/zerolog v1.34.0 // indirect - github.com/spf13/cast v1.7.1 // indirect - github.com/twitchyliquid64/golang-asm v0.15.1 // indirect - github.com/valyala/bytebufferpool v1.0.0 // indirect - github.com/valyala/fasthttp v1.60.0 // indirect - github.com/yosida95/uritemplate/v3 v3.0.2 // indirect - golang.org/x/arch v0.0.0-20210923205945-b76863e36670 // indirect - golang.org/x/net v0.39.0 // indirect - golang.org/x/oauth2 v0.30.0 // indirect - golang.org/x/sys v0.32.0 // indirect - golang.org/x/text v0.24.0 // indirect -) +// (run: go mod tidy)plugins/redis/README.md (1)
27-31: Add missing imports in Quick Start snippet.The example uses schemas and log but doesn’t import them. Include both to make the snippet compile.
import ( + "log" "github.com/maximhq/bifrost/plugins/redis" bifrost "github.com/maximhq/bifrost/core" + "github.com/maximhq/bifrost/core/schemas" )plugins/redis/main.go (2)
585-587: Fix log message: operation is delete, not get.The warning text is misleading when DEL fails.
- plugin.logger.Warn(PluginLoggerPrefix + " Failed to get cached chunks, continuing with request") + plugin.logger.Warn(PluginLoggerPrefix + " Failed to delete cache keys for ClearCacheForKey")
132-140: Use configurable ContextTimeout for Redis ping.Avoid the hardcoded 5s. Respect
config.ContextTimeout(already defaulted earlier) for consistency.-// Test connection with configured timeout -ctx, cancel := context.WithTimeout(context.Background(), RedisConnectionTimeout) +// Test connection with configured timeout +ctx, cancel := context.WithTimeout(context.Background(), config.ContextTimeout) defer cancel() _, err := client.Ping(ctx).Result() if err != nil { client.Close() return nil, fmt.Errorf("failed to ping Redis at %s: %w", config.Addr, err) }plugins/redis/plugin_test.go (2)
172-174: Replace fixed sleep with polling for the cache key (reduces flakiness).Wait until the cache entry appears (or time out) instead of sleeping a fixed 100ms.
-// Wait a moment to ensure cache is written -time.Sleep(100 * time.Millisecond) +// Wait up to 2s for at least one key with our test prefix to appear +deadline := time.Now().Add(2 * time.Second) +for { + if time.Now().After(deadline) { + t.Fatal("Timed out waiting for cache entry") + } + keys, _, err := redisClient.Scan(ctx, 0, TestPrefix+"*", 10).Result() + if err == nil && len(keys) > 0 { + break + } + time.Sleep(50 * time.Millisecond) +}
365-367: Streaming test: replace fixed sleep with polling for chunk keys.Poll with SCAN for keys matching the test prefix and “chunk” suffix.
-// Wait for cache to be written -time.Sleep(200 * time.Millisecond) +// Wait up to 3s for chunked cache keys to appear +deadline := time.Now().Add(3 * time.Second) +for { + if time.Now().After(deadline) { + t.Fatal("Timed out waiting for streaming cache entries") + } + keys, _, err := redisClient.Scan(ctx, 0, TestPrefix+"*"_chunk_"+"*", 50).Result() + if err == nil && len(keys) > 0 { + break + } + time.Sleep(50 * time.Millisecond) +}Note: If the exact base key is known, narrow the pattern, e.g.,
prefix+hash+"_chunk_*".
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
plugins/redis/go.sumis excluded by!**/*.sum
📒 Files selected for processing (4)
plugins/redis/README.md(1 hunks)plugins/redis/go.mod(1 hunks)plugins/redis/main.go(1 hunks)plugins/redis/plugin_test.go(1 hunks)
🧰 Additional context used
🧠 Learnings (25)
📓 Common learnings
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#55
File: core/providers/anthropic.go:358-388
Timestamp: 2025-06-04T05:37:59.699Z
Learning: User Pratham-Mishra04 prefers not to extract small code duplications (around 2 lines) into helper functions, considering the overhead not worth it for such minor repetition.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#102
File: README.md:62-66
Timestamp: 2025-06-19T17:03:03.639Z
Learning: Pratham-Mishra04 prefers using the implicit 'latest' tag for the maximhq/bifrost Docker image rather than pinning to specific versions.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#143
File: core/mcp.go:155-196
Timestamp: 2025-07-08T15:33:47.698Z
Learning: Pratham-Mishra04 prefers not to add explanatory comments for obvious code patterns, such as the unlock/lock strategy around network I/O operations, considering them self-explanatory to experienced developers.
📚 Learning: 2025-06-20T16:21:18.912Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#103
File: .github/workflows/transport-dependency-update.yml:53-75
Timestamp: 2025-06-20T16:21:18.912Z
Learning: In the bifrost repository's transport dependency update workflow, when updating the core dependency to a new version using `go get`, the go.mod and go.sum files will always change in normal operation, making the safety check for changes more of a defensive programming practice rather than handling a common scenario.
Applied to files:
plugins/redis/go.mod
📚 Learning: 2025-06-16T04:27:53.538Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#81
File: tests/core-providers/go.mod:3-4
Timestamp: 2025-06-16T04:27:53.538Z
Learning: In Go module files, `go 1.24.1` (with patch version) can work fine in some setups, contrary to the general rule that go directives should only include major.minor versions.
Applied to files:
plugins/redis/go.mod
📚 Learning: 2025-06-16T03:55:16.949Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#81
File: tests/core-providers/go.mod:3-4
Timestamp: 2025-06-16T03:55:16.949Z
Learning: Go 1.24 was released in February 2025 and is stable and available for use in go.mod files.
Applied to files:
plugins/redis/go.mod
📚 Learning: 2025-08-11T12:34:58.738Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#237
File: transports/bifrost-http/lib/store.go:2085-2102
Timestamp: 2025-08-11T12:34:58.738Z
Learning: In the Bifrost HTTP transport's ConfigStore (transports/bifrost-http/lib/store.go), the Redis configuration table is designed to have at most one row. The UpdateRedisConfig method ensures this by deleting all existing configs before inserting a new one within a transaction, making duplicate Redis configs impossible. Using First() without ordering is safe for fetching the Redis config.
Applied to files:
plugins/redis/go.mod
📚 Learning: 2025-08-11T12:34:17.569Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#237
File: transports/bifrost-http/lib/ctx.go:16-16
Timestamp: 2025-08-11T12:34:17.569Z
Learning: In the Bifrost codebase, using string literals directly with `redis.ContextKey()` (e.g., `redis.ContextKey("request-cache-key")`) is an intended design pattern rather than exporting constants from the redis plugin. This approach is preferred by Pratham-Mishra04 for cache-related context keys.
Applied to files:
plugins/redis/go.modplugins/redis/README.mdplugins/redis/plugin_test.goplugins/redis/main.go
📚 Learning: 2025-08-11T12:23:01.157Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#237
File: transports/bifrost-http/handlers/config.go:88-89
Timestamp: 2025-08-11T12:23:01.157Z
Learning: In the Bifrost transport configuration (transports/bifrost-http/handlers/config.go), changes to the `EnableCaching` field require a service restart to take effect. This is an intended design decision - the Redis caching plugin is only initialized once at startup in main.go and there's no hot-reload mechanism for enabling/disabling caching at runtime.
Applied to files:
plugins/redis/README.mdplugins/redis/main.go
📚 Learning: 2025-06-16T09:16:15.634Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#82
File: tests/transports-integrations/tests/integrations/test_litellm.py:97-115
Timestamp: 2025-06-16T09:16:15.634Z
Learning: In the Bifrost integration tests, the user prefers tests to fail hard when API keys are missing rather than using skip_if_no_api_key decorators for graceful skipping. This applies to LiteLLM tests that depend on OpenAI API keys.
Applied to files:
plugins/redis/plugin_test.go
📚 Learning: 2025-08-11T12:04:59.961Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#70
File: plugins/redis/main.go:503-526
Timestamp: 2025-08-11T12:04:59.961Z
Learning: In the Bifrost Redis plugin cleanup operations, Pratham-Mishra04 prefers using `KEYS` command over `SCAN` for simplicity, as blocking behavior is acceptable during shutdown/cleanup scenarios when the operation is not in the critical path.
Applied to files:
plugins/redis/main.go
📚 Learning: 2025-08-08T14:12:14.754Z
Learnt from: TejasGhatte
PR: maximhq/bifrost#238
File: transports/bifrost-http/plugins/logging/main.go:0-0
Timestamp: 2025-08-08T14:12:14.754Z
Learning: In transports/bifrost-http/plugins/logging/main.go, stream accumulator cleanup is intended to use a 5-minute TTL (consistent with processing logs). Comments should state “older than 5 minutes,” and using central constants (processingLogTTL, streamAccumulatorTTL) helps avoid future mismatches.
Applied to files:
plugins/redis/main.go
📚 Learning: 2025-07-08T18:30:08.258Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#141
File: core/bifrost.go:198-272
Timestamp: 2025-07-08T18:30:08.258Z
Learning: Pratham-Mishra04 follows a pattern of implementing core functionality first and deferring non-critical improvements (like race condition fixes, optimizations) to later PRs. This is a reasonable development approach that prioritizes getting the main feature working before addressing edge cases.
Applied to files:
plugins/redis/main.go
📚 Learning: 2025-08-03T20:36:21.906Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#202
File: transports/bifrost-http/plugins/governance/resolver.go:183-191
Timestamp: 2025-08-03T20:36:21.906Z
Learning: In the Bifrost governance plugin (transports/bifrost-http/plugins/governance/resolver.go), Pratham-Mishra04 considers the current string matching approach for determining rate limit violation decision types (DecisionTokenLimited vs DecisionRequestLimited) adequate and not important enough to refactor with more robust explicit checks, preferring functional simplicity over theoretical robustness improvements.
Applied to files:
plugins/redis/main.go
📚 Learning: 2025-06-15T16:05:13.489Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#84
File: transports/bifrost-http/main.go:2-2
Timestamp: 2025-06-15T16:05:13.489Z
Learning: For the Bifrost project, HTTP transport integration routers for new providers (like Mistral and Ollama) are implemented in separate PRs from the core provider support, following a focused PR strategy.
Applied to files:
plugins/redis/main.go
📚 Learning: 2025-08-04T11:27:45.020Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#208
File: transports/bifrost-http/handlers/completions.go:221-225
Timestamp: 2025-08-04T11:27:45.020Z
Learning: In transports/bifrost-http/handlers/completions.go, Pratham-Mishra04 intentionally delegates file size validation for audio transcription uploads to the downstream API/provider rather than enforcing MaxFileSize limits at the HTTP transport layer. This follows their architectural preference of keeping the transport layer simple and letting providers handle domain-specific validation concerns, even for potential memory exhaustion scenarios.
Applied to files:
plugins/redis/main.go
📚 Learning: 2025-08-03T20:17:11.876Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#202
File: transports/bifrost-http/plugins/governance/resolver.go:68-74
Timestamp: 2025-08-03T20:17:11.876Z
Learning: In the Bifrost governance plugin (transports/bifrost-http/plugins/governance/resolver.go), Pratham-Mishra04 considers nil validation in constructors like NewBudgetResolver unnecessary because the parameters will never be nil in practice. This aligns with their general preference to avoid defensive programming for scenarios they consider unreachable.
Applied to files:
plugins/redis/main.go
📚 Learning: 2025-08-03T20:28:00.857Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#202
File: transports/bifrost-http/handlers/governance.go:1038-1049
Timestamp: 2025-08-03T20:28:00.857Z
Learning: In the Bifrost governance handler (transports/bifrost-http/handlers/governance.go), Pratham-Mishra04 is comfortable with exposing full error details including err.Error() in production responses for governance API endpoints, prioritizing simplicity over potential information disclosure concerns.
Applied to files:
plugins/redis/main.go
📚 Learning: 2025-07-16T07:13:29.496Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#162
File: transports/bifrost-http/integrations/genai/types.go:0-0
Timestamp: 2025-07-16T07:13:29.496Z
Learning: Pratham-Mishra04 prefers to avoid redundant error handling across architectural layers in the Bifrost streaming implementation. When error handling (such as timeouts, context cancellation, and JSON marshaling failures) is already handled at the provider level, they prefer not to duplicate this logic at the transport integration layer to keep the code simple and avoid unnecessary complexity.
Applied to files:
plugins/redis/main.go
📚 Learning: 2025-06-09T16:35:26.914Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#64
File: transports/bifrost-http/integrations/genai/types.go:273-313
Timestamp: 2025-06-09T16:35:26.914Z
Learning: In convertGenerationConfigToParams method in transports/bifrost-http/integrations/genai/types.go, pre-allocating the ExtraParams map is preferred over lazy allocation because the method has multiple potential ExtraParams assignments, making the computational overhead of conditional checks exceed the memory savings of an empty map.
Applied to files:
plugins/redis/main.go
📚 Learning: 2025-08-08T07:07:08.801Z
Learnt from: TejasGhatte
PR: maximhq/bifrost#219
File: core/providers/openai.go:459-459
Timestamp: 2025-08-08T07:07:08.801Z
Learning: In core/providers/utils.go, processAndSendResponse and processAndSendError now take exactly five parameters: (ctx, postHookRunner, response|err, responseChan, logger). All providers (OpenAI, Anthropic, Cohere, Bedrock) must call these with the logger as the fifth argument; a repo-wide check in PR maximhq/bifrost#219 confirmed this is already consistent.
Applied to files:
plugins/redis/main.go
📚 Learning: 2025-06-16T04:13:55.437Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#81
File: tests/core-providers/scenarios/end_to_end_tool_calling.go:43-45
Timestamp: 2025-06-16T04:13:55.437Z
Learning: In the Bifrost codebase, errors returned from client methods like ChatCompletionRequest are of type BifrostError, not the standard error interface. For testing these errors, use require.Nilf instead of require.NoErrorf since BifrostError doesn't work with the standard error assertion methods.
Applied to files:
plugins/redis/main.go
📚 Learning: 2025-06-16T04:13:42.755Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#81
File: tests/core-providers/scenarios/simple_chat.go:39-41
Timestamp: 2025-06-16T04:13:42.755Z
Learning: In the Bifrost codebase, errors returned from methods like ChatCompletionRequest are of type BifrostError (a custom error type) rather than the standard Go error interface. Therefore, require.Nilf should be used for error assertions instead of require.NoErrorf.
Applied to files:
plugins/redis/main.go
📚 Learning: 2025-06-16T04:12:05.427Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#81
File: tests/core-providers/scenarios/complete_end_to_end.go:39-41
Timestamp: 2025-06-16T04:12:05.427Z
Learning: In the Bifrost system, error returns are of type `BifrostError` rather than the standard Go `error` interface. Therefore, use `require.Nilf(t, err, ...)` instead of `require.NoError(t, err)` when checking for errors in Bifrost function calls.
Applied to files:
plugins/redis/main.go
📚 Learning: 2025-06-18T15:15:51.323Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#94
File: core/bifrost.go:823-845
Timestamp: 2025-06-18T15:15:51.323Z
Learning: In the Bifrost project, the team prioritizes maintaining consistent error handling patterns over exposing detailed error context. All errors should be wrapped in the standard `BifrostError` structure rather than creating specific error types or exposing richer error details like exit codes or stderr output.
Applied to files:
plugins/redis/main.go
📚 Learning: 2025-06-15T13:50:41.418Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#83
File: core/providers/mistral.go:96-101
Timestamp: 2025-06-15T13:50:41.418Z
Learning: In the Bifrost project, when a provider doesn't support a specific operation (like text completion), the IsBifrostError flag should be set to false to mark it as a provider-level error rather than a Bifrost framework error. This is intentional design for unsupported operations.
Applied to files:
plugins/redis/main.go
📚 Learning: 2025-06-15T14:24:49.882Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#83
File: core/providers/mistral.go:168-170
Timestamp: 2025-06-15T14:24:49.882Z
Learning: In the Bifrost codebase, performance is prioritized over defensive copying for HTTP service operations. Specifically, shallow slice assignments in provider response handling are acceptable due to object pool reset patterns and JSON unmarshaling behavior that minimize practical data corruption risks.
Applied to files:
plugins/redis/main.go
🪛 LanguageTool
plugins/redis/README.md
[grammar] ~1-~1: Use correct spacing
Context: # Redis Cache Plugin for Bifrost This plugin provides Redis-based caching...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~3-~3: Use correct spacing
Context: ... improving performance and reducing API costs. ## Features - High-Performance Hashing...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[typographical] ~13-~13: To join two clauses or set off examples, consider using an em dash.
Context: ...: Only requires Redis address and cache key - sensible defaults for everything else - ...
(QB_NEW_EN_DASH_RULE_EM)
[grammar] ~14-~14: Use correct spacing
Context: ...ed**: Creates and manages its own Redis client ## Installation ```bash go get github.com/...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~23-~23: Use correct spacing
Context: ...mhq/bifrost/plugins/redis ## Quick Start ### Basic Setup go import ( "github....
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~25-~25: Use correct spacing
Context: ...ns/redis ## Quick Start ### Basic Setup go import ( "github.com/maximhq/bifrost/plugins/redis" bifrost "github.com/maximhq/bifrost/core" ) // Simple configuration - only Redis address and cache key are required! config := redis.RedisPluginConfig{ Addr: "localhost:6379", // Your Redis server address CacheKey: "x-my-cache-key", // Context key for cache identification } // Create the plugin plugin, err := redis.NewRedisPlugin(config, logger) if err != nil { log.Fatal("Failed to create Redis plugin:", err) } // Use with Bifrost bifrostConfig := schemas.BifrostConfig{ Account: yourAccount, Plugins: []schemas.Plugin{plugin}, // ... other config } ``` That's it! The plugin uses Redis client ...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~53-~53: There might be a mistake here.
Context: ...nection handling and these defaults for caching: - TTL: 5 minutes - CacheByModel: tr...
(QB_NEW_EN_OTHER)
[grammar] ~57-~57: Use correct spacing
Context: ...ider**: true (include provider in cache key) Important: You must provide the cache ...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~66-~66: Use correct spacing
Context: ...st(ctx, request) ### With Password Authentication go config := redis.RedisPluginConfig{ Addr: "localhost:6379", CacheKey: "x-my-cache-key", Password: "your-redis-password", } ### With Custom TTL and Prefix go config...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~76-~76: Use correct spacing
Context: ...ssword", } ### With Custom TTL and Prefix go config := redis.RedisPluginConfig{ Addr: "localhost:6379", CacheKey: "x-my-cache-key", TTL: time.Hour, // Cache for 1 hour Prefix: "myapp:cache:", // Custom prefix } ``` ### Per-Request TTL Override (via Context) ...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~87-~87: Use correct spacing
Context: ... ``` ### Per-Request TTL Override (via Context) You can override the cache TTL for indiv...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~89-~89: There might be a mistake here.
Context: ...CacheTTLKey on the plugin, then set a time.Duration value at that context key before making...
(QB_NEW_EN_OTHER)
[grammar] ~89-~89: There might be a problem here.
Context: ...e at that context key before making the request. go // Configure plugin with a context key used to read per-request TTLs config := redis.RedisPluginConfig{ Addr: "localhost:6379", CacheKey: "x-my-cache-key", CacheTTLKey: "x-my-cache-ttl", // The context key for reading TTL TTL: 5 * time.Minute, // Fallback/default TTL } plugin, err := redis.NewRedisPlugin(config, logger) // ... init Bifrost client with plugin // Before making a request, set a per-request TTL ctx := context.WithValue(ctx, redis.ContextKey("x-my-cache-ttl"), 30*time.Second) resp, err := client.ChatCompletionRequest(ctx, request) Notes: - The context value must be of type `time...
(QB_NEW_EN_MERGED_MATCH)
[grammar] ~113-~113: Use correct spacing
Context: ...lies to all chunks. ### With Different Database go config := redis.RedisPluginConfig{ Addr: "localhost:6379", CacheKey: "x-my-cache-key", DB: 1, // Use Redis database 1 } ### Streaming Cache Example ```go // Config...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~123-~123: Use correct spacing
Context: ...s database 1 } ### Streaming Cache Example go // Configure plugin for streaming cache config := redis.RedisPluginConfig{ Addr: "localhost:6379", CacheKey: "x-stream-cache-key", TTL: 30 * time.Minute, // Cache streaming responses for 30 minutes } // Use with streaming requests ctx := context.WithValue(ctx, redis.ContextKey("x-stream-cache-key"), "stream-session-1") stream, err := client.ChatCompletionStreamRequest(ctx, request) // Subsequent identical requests will be served from cache as a reconstructed stream ### Custom Cache Key Configuration go co...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~139-~139: Use correct spacing
Context: ...ructed stream ### Custom Cache Key Configuration go config := redis.RedisPluginConfig{ Addr: "localhost:6379", CacheKey: "x-my-cache-key", CacheByModel: bifrost.Ptr(false), // Don't include model in cache key CacheByProvider: bifrost.Ptr(true), // Include provider in cache key } ### Custom Redis Client Configuration go...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~150-~150: Use correct spacing
Context: ...ache key } ### Custom Redis Client Configuration go config := redis.RedisPluginConfig{ Addr: "localhost:6379", CacheKey: "x-my-cache-key", PoolSize: 20, // Custom connection pool size DialTimeout: 5 * time.Second, // Custom connection timeout ReadTimeout: 3 * time.Second, // Custom read timeout ConnMaxLifetime: time.Hour, // Custom connection lifetime } ``` ## Configuration Options | Option ...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~163-~163: Use correct spacing
Context: ...ection lifetime } ``` ## Configuration Options | Option | Type | ...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~172-~172: Use correct spacing
Context: ... time.Minute| Time-to-live for cached responses | |Prefix |string` |...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~175-~175: Use correct spacing
Context: ... | Include provider in cache key | Redis Connection Options (all optional...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~179-~179: There might be a mistake here.
Context: ...values): - PoolSize, MinIdleConns, MaxIdleConns - Connection pool settings - `ConnMaxL...
(QB_NEW_EN_OTHER)
[grammar] ~180-~180: There might be a mistake here.
Context: ...tion pool settings - ConnMaxLifetime, ConnMaxIdleTime - Connection lifetime settings - `Dial...
(QB_NEW_EN_OTHER)
[grammar] ~181-~181: There might be a mistake here.
Context: ...ettings - DialTimeout, ReadTimeout, WriteTimeout - Timeout settings All Redis configur...
(QB_NEW_EN_OTHER)
[grammar] ~181-~181: Use correct spacing
Context: ...ReadTimeout, WriteTimeout - Timeout settings All Redis configuration values are passe...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~183-~183: Use correct spacing
Context: ... you want to override from Redis client defaults. ## How It Works The plugin generates an xx...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~185-~185: Use correct spacing
Context: ... from Redis client defaults. ## How It Works The plugin generates an xxhash of the no...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~187-~187: Use correct spacing
Context: ...tes an xxhash of the normalized request including: - Provider (if CacheByProvider is true) - ...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~192-~192: Use correct spacing
Context: ...completion) - Parameters (includes tool calls) Identical requests will always produce t...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~194-~194: Use correct spacing
Context: ...oduce the same hash, enabling effective caching. ### Caching Flow #### Regular Requests 1. ...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~196-~196: Use correct spacing
Context: ...nabling effective caching. ### Caching Flow #### Regular Requests 1. PreHook: Checks...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~200-~200: There might be a mistake here.
Context: ...cached response, returns immediately if found 2. PostHook: Stores the response in Redis...
(QB_NEW_EN_OTHER)
[grammar] ~201-~201: There might be a mistake here.
Context: ...es the response in Redis asynchronously (non-blocking) 3. Cleanup: Clears all cached entries and...
(QB_NEW_EN_OTHER)
[grammar] ~202-~202: There might be a mistake here.
Context: ...cached entries and closes connection on shutdown #### Streaming Requests 1. PreHook: Chec...
(QB_NEW_EN_OTHER)
[grammar] ~204-~204: Use correct spacing
Context: ... connection on shutdown #### Streaming Requests 1. PreHook: Checks Redis for cached chunk...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~209-~209: Use correct spacing
Context: ...ent requests get sorted chunks as a new stream Asynchronous Caching: Cache writes hap...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~211-~211: Use correct spacing
Context: ...mal performance while maintaining cache functionality. Streaming Intelligence: The plugin aut...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~213-~213: Use correct spacing
Context: ...t reconstruction of the original stream order. ### Cache Keys #### Regular Responses Cach...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~215-~215: Use correct spacing
Context: ...f the original stream order. ### Cache Keys #### Regular Responses Cache keys follow the...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~217-~217: Use correct spacing
Context: ...am order. ### Cache Keys #### Regular Responses Cache keys follow the pattern: `{prefix}...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~219-~219: There might be a problem here.
Context: ...egular Responses Cache keys follow the pattern: {prefix}{cache_value}_{xxhash} Example: bifrost:cache:my-session_a1b2c3d4e5f6... #### Streaming Responses Chunk keys follow t...
(QB_NEW_EN_MERGED_MATCH)
[grammar] ~223-~223: Use correct spacing
Context: ...ession_a1b2c3d4e5f6... #### Streaming Responses Chunk keys follow the pattern:{prefix}...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~225-~225: There might be a problem here.
Context: ...eaming Responses Chunk keys follow the pattern: {prefix}{cache_value}_{xxhash}_chunk_{index} Examples: - bifrost:cache:my-session_a1b2c3d4e5f6..._chunk_0 - bifrost:cache:my-session_a1b2c3d4e5f6..._chunk_1 - bifrost:cache:my-session_a1b2c3d4e5f6..._chunk_2 ## Manual Cache Invalidation You can ...
(QB_NEW_EN_MERGED_MATCH)
[grammar] ~233-~233: Use correct spacing
Context: ...b2c3d4e5f6..._chunk_2` ## Manual Cache Invalidation You can invalidate specific cached entri...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~235-~235: There might be a mistake here.
Context: ...acheForKey(key string)on the concreteredis.Plugin` type. This deletes the provided key and...
(QB_NEW_EN_OTHER)
[grammar] ~235-~235: Use correct spacing
Context: ...ming response, all of its chunk entries (<key>_chunk_*). ### Getting the cache key from responses - ...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~237-~237: Use correct spacing
Context: ...nk_*`). ### Getting the cache key from responses - Regular responses: When a response is ...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~239-~239: There might be a mistake here.
Context: ...from cache, the plugin adds metadata to response.ExtraFields.RawResponse: - bifrost_cached: true - bifrost_cache_key: "<prefix><cache_value>_<xxhash>" Use this bifrost_cache_key as the ...
(QB_NEW_EN_OTHER)
[grammar] ~246-~246: Use correct spacing
Context: ...at base key to ClearCacheForKey. ### Examples go // Non-streaming: clear the cached response you just used resp, err := client.ChatCompletionRequest(ctx, req) if err != nil { // handle error } if resp != nil && resp.ExtraFields.RawResponse != nil { if raw, ok := resp.ExtraFields.RawResponse.(map[string]interface{}); ok { if keyAny, ok := raw["bifrost_cache_key"]; ok { if pluginImpl, ok := plugin.(*redis.Plugin); ok { _ = pluginImpl.ClearCacheForKey(keyAny.(string)) } } } } go // Streaming: clear all chunks for a cached stream for msg := range stream { if msg.BifrostResponse == nil { continue } raw := msg.BifrostResponse.ExtraFields.RawResponse rawMap, ok := raw.(map[string]interface{}) if !ok { continue } keyAny, ok := rawMap["bifrost_cache_key"] if !ok { continue } chunkKey := keyAny.(string) // e.g., "<base>_chunk_3" // Derive base key by removing the trailing "_chunk_{index}" part baseKey := chunkKey if idx := strings.LastIndex(chunkKey, "_chunk_"); idx != -1 { baseKey = chunkKey[:idx] } if pluginImpl, ok := plugin.(*redis.Plugin); ok { _ = pluginImpl.ClearCacheForKey(baseKey) } break // we only need one chunk to compute the base key } To clear all entries managed by this plu...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~296-~296: There might be a mistake here.
Context: ...in (by prefix), call Cleanup() during shutdown: go _ = plugin.(*redis.Plugin).Cleanup() ## Testing The plugin includes compre...
(QB_NEW_EN_OTHER)
[grammar] ~304-~304: Use correct spacing
Context: ...ts for both regular and streaming cache functionality. Run the tests with a Redis instance runn...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~306-~306: There might be a mistake here.
Context: ...y. Run the tests with a Redis instance running: bash # Start Redis (using Docker) docker run -d -p 6379:6379 redis:latest # Run all tests go test -v # Run specific tests go test -run TestRedisPlugin -v # Test regular caching go test -run TestRedisPluginStreaming -v # Test streaming cache Tests will be skipped if Re...
(QB_NEW_EN_OTHER)
[grammar] ~320-~320: There might be a mistake here.
Context: ...ed if Redis is not available. The tests validate: - Cache hit/miss behavior - Performance i...
(QB_NEW_EN_OTHER)
[grammar] ~326-~326: Use correct spacing
Context: ...d reconstruction - Provider information preservation ## Performance Benefits - **Reduced API Ca...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~328-~328: Use correct spacing
Context: ...nformation preservation ## Performance Benefits - Reduced API Calls: Identical requests ...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~330-~330: There might be a mistake here.
Context: ...s**: Identical requests are served from cache - Ultra-Low Latency: Cache hits return i...
(QB_NEW_EN_OTHER)
[grammar] ~331-~331: There might be a mistake here.
Context: ...Ultra-Low Latency*: Cache hits return immediately, cache writes are non-blocking - **Str...
(QB_NEW_EN_OTHER)
[grammar] ~331-~331: There might be a mistake here.
Context: ...ts return immediately, cache writes are non-blocking - Streaming Efficiency: Cached st...
(QB_NEW_EN_OTHER)
[grammar] ~332-~332: There might be a mistake here.
Context: ... and delivered faster than original API calls - Cost Savings: Fewer API calls to expen...
(QB_NEW_EN_OTHER)
[grammar] ~333-~333: There might be a mistake here.
Context: ...ngs**: Fewer API calls to expensive LLM providers - Improved Reliability: Cached responses...
(QB_NEW_EN_OTHER)
[grammar] ~334-~334: Insert the missing word
Context: ...ders - Improved Reliability: Cached responses available even if provider is down - **...
(QB_NEW_EN_OTHER_ERROR_IDS_32)
[grammar] ~334-~334: Use articles correctly
Context: ...y**: Cached responses available even if provider is down - High Throughput: Asynchro...
(QB_NEW_EN_OTHER_ERROR_IDS_11)
[grammar] ~334-~334: There might be a mistake here.
Context: ...responses available even if provider is down - High Throughput: Asynchronous caching ...
(QB_NEW_EN_OTHER)
[grammar] ~335-~335: There might be a mistake here.
Context: ...hronous caching doesn't impact response times - Perfect Stream Fidelity: Cached stream...
(QB_NEW_EN_OTHER)
[grammar] ~336-~336: There might be a mistake here.
Context: ...reams maintain exact chunk ordering and content ## Error Handling The plugin is designed t...
(QB_NEW_EN_OTHER)
[grammar] ~338-~338: Use correct spacing
Context: ...ct chunk ordering and content ## Error Handling The plugin is designed to fail gracefull...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~340-~340: There might be a mistake here.
Context: ...andling The plugin is designed to fail gracefully: - If Redis is unavailable during startup,...
(QB_NEW_EN_OTHER)
[grammar] ~342-~342: There might be a problem here.
Context: ...ing startup, plugin creation fails with clear error - If Redis becomes unavailable during oper...
(QB_NEW_EN_MERGED_MATCH)
[grammar] ~343-~343: There might be a mistake here.
Context: ...ng operation, requests continue without caching - If cache retrieval fails, requests proce...
(QB_NEW_EN_OTHER)
[grammar] ~344-~344: There might be a mistake here.
Context: ...cache retrieval fails, requests proceed normally - If cache storage fails asynchronously, r...
(QB_NEW_EN_OTHER)
[grammar] ~345-~345: There might be a mistake here.
Context: ...usly, responses are unaffected (already returned) - Malformed cached data is ignored and req...
(QB_NEW_EN_OTHER)
[grammar] ~346-~346: There might be a mistake here.
Context: ...ed data is ignored and requests proceed normally - Cache operations have timeouts to preven...
(QB_NEW_EN_OTHER)
[grammar] ~347-~347: There might be a mistake here.
Context: ...tions have timeouts to prevent resource leaks ## Best Practices 1. Start Simple: Use...
(QB_NEW_EN_OTHER)
[grammar] ~349-~349: Use correct spacing
Context: ...outs to prevent resource leaks ## Best Practices 1. Start Simple: Use only Addr and `Cac...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~351-~351: There might be a mistake here.
Context: .... Start Simple: Use only Addr and CacheKey - let defaults handle the rest 2. **Ch...
(QB_NEW_EN_OTHER)
[grammar] ~351-~351: There might be a mistake here.
Context: ...nd CacheKey - let defaults handle the rest 2. Choose meaningful cache keys: Use desc...
(QB_NEW_EN_OTHER)
[grammar] ~352-~352: There might be a mistake here.
Context: ...iptive context keys that identify cache sessions 3. Set appropriate TTL: Balance between c...
(QB_NEW_EN_OTHER)
[grammar] ~353-~353: There might be a mistake here.
Context: ...lance between cache efficiency and data freshness 4. Use meaningful prefixes: Helps organiz...
(QB_NEW_EN_OTHER)
[grammar] ~354-~354: There might be a mistake here.
Context: ...lps organize cache keys in shared Redis instances 5. Monitor Redis memory: Track cache usag...
(QB_NEW_EN_OTHER)
[grammar] ~355-~355: There might be a mistake here.
Context: ...streaming responses (more chunks = more storage) 6. Context management: Always provide cac...
(QB_NEW_EN_OTHER)
[grammar] ~356-~356: There might be a mistake here.
Context: ...e key in request context for caching to work 7. Use bifrost.Ptr(): For boolean point...
(QB_NEW_EN_OTHER)
[grammar] ~358-~358: There might be a mistake here.
Context: ...ons**: Longer streams create more cache entries, adjust TTL accordingly ## Security Con...
(QB_NEW_EN_OTHER)
[grammar] ~358-~358: There might be a mistake here.
Context: ...s create more cache entries, adjust TTL accordingly ## Security Considerations - **Sensitive D...
(QB_NEW_EN_OTHER)
[grammar] ~360-~360: Use correct spacing
Context: ...es, adjust TTL accordingly ## Security Considerations - Sensitive Data: Be cautious about cach...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
🔇 Additional comments (2)
plugins/redis/main.go (1)
165-198: Normalized payload contains no maps—hashing is already deterministicThe
generateRequestHashhelper only marshals a struct with basic types, slices, and nested structs (Provider,Model,RequestInput,ModelParameters)—none of which include rawmapfields. Moreover, Go’sencoding/jsonemits struct fields in declaration order (and sorts map keys if maps ever appear). As a result, the JSON output here is stable and won’t suffer non-deterministic ordering. No additional canonicalization or key-sorting helper is required.Likely an incorrect or invalid review comment.
plugins/redis/plugin_test.go (1)
96-103: Skip only on Redis availability, not on OPENAI_API_KEY
The Redis plugin tests have no dependency on an OpenAI API key, so adding anOPENAI_API_KEYcheck is irrelevant here. Retain the existingt.Skipf("Redis not available…")behavior and focus on stabilizing the test by avoiding fixed sleeps and ensuring proper cleanup.• File: plugins/redis/plugin_test.go (lines 96–103)
– Remove the suggested block:
go if os.Getenv("OPENAI_API_KEY") == "" { t.Skip("OPENAI_API_KEY not set – skipping integration test") }
• Replace anytime.Sleepcalls with polling/retry logic for readiness.
• Uset.Cleanupto flush the Redis database or close connections after each test.Likely an incorrect or invalid review comment.
Pratham-Mishra04
force-pushed
the
06-11-_plugins_feat_redis_caching_plugin_added
branch
from
d0561aa to
5216e81
Compare
Redis Cache Plugin for Bifrost
This PR adds a new Redis-based caching plugin for Bifrost that improves performance and reduces API costs by caching responses for identical requests. The plugin:
The implementation includes:
This caching mechanism significantly improves performance and reduces API costs when using LLM providers, with minimal configuration required - only a Redis address is needed to get started with sensible defaults for everything else.