Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
💤 Files with no reviewable changes (5)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughSummary by CodeRabbit
WalkthroughAdds dimension validation for existing Qdrant and Redis namespaces, exposes a configurable Qdrant gRPC max receive message size, updates schema and Qdrant docs, and changes the semantic cache to store a non-zero unit-vector placeholder when required. ChangesVector Store Dimension Validation and Qdrant Configuration
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
|
tejas ghatte seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
Confidence Score: 5/5Safe to merge; all three functional changes are correct for their primary use cases and the issues noted are narrow edge cases. All three functional changes (gRPC size limit, dimension guards, placeholder vector) are correct for their primary use cases. The Redis dimension check has a narrow edge case on multi-vector indexes and the schema has no upper bound on max_recv_msg_size_mb, but neither affects the normal operating path. framework/vectorstore/redis.go and transports/config.schema.json have the minor issues noted in the inline comments. Important Files Changed
Reviews (3): Last reviewed commit: "fix: semantic cache issues, dimension ch..." | Re-trigger Greptile |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@framework/vectorstore/qdrant.go`:
- Around line 47-57: The current code swallows errors from
s.client.GetCollectionInfo (infoErr) and thus skips dimension validation; change
the logic in the block that calls s.client.GetCollectionInfo(ctx, namespace)
(and checks infoErr) to fail fast: if infoErr != nil return a wrapped/errorf
describing the failure to fetch collection info for namespace (including
infoErr), otherwise proceed with the existing
GetConfig/GetParams/GetVectorsConfig/GetParams dimension check and compare
existingDim to the configured dimension and return the existing
dimension-mismatch error as before.
In `@framework/vectorstore/redis.go`:
- Around line 83-91: The current CreateNamespace flow silently succeeds when
ftInfoErr != nil or when FTInfo has no VECTOR attribute; change it to fail fast:
if ftInfoErr != nil return a wrapped error describing inability to read existing
index metadata (include ftInfoErr), and after iterating ftInfo.Attributes, if no
VECTOR attribute with Dim > 0 was found return an error indicating the existing
index lacks a VECTOR schema; only call s.cacheNamespaceFieldTypes(namespace,
properties) and return nil when validation passes (matching dimension or no
conflict). Use the existing symbols ftInfoErr, ftInfo.Attributes loop,
dimension, namespace, and s.cacheNamespaceFieldTypes to locate and update the
logic.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 89211098-259e-4084-bc3b-c4199af18ec0
📒 Files selected for processing (6)
docs/integrations/vector-databases/qdrant.mdxframework/go.modframework/vectorstore/qdrant.goframework/vectorstore/redis.goframework/vectorstore/weaviate.gotransports/config.schema.json
17742e1 to
e346771
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (2)
docs/integrations/vector-databases/qdrant.mdx (2)
94-94: ⚡ Quick winDocument the minimum value constraint for
max_recv_msg_size_mb.According to the PR summary, the schema enforces a minimum value of 1 MB. Consider adding this constraint to the description to help users understand the valid range.
📝 Suggested enhancement
-| `max_recv_msg_size_mb` | integer | `64` | gRPC max receive message size in MB. Increase when caching large payloads (e.g. base64-encoded image generation responses exceed the default). | +| `max_recv_msg_size_mb` | integer | `64` | gRPC max receive message size in MB (minimum: 1). Increase when caching large payloads (e.g. base64-encoded image generation responses exceed the default). |🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/integrations/vector-databases/qdrant.mdx` at line 94, The docs entry for the `max_recv_msg_size_mb` field omits the minimum constraint; update the description for `max_recv_msg_size_mb` to state that the value must be an integer >= 1 (minimum 1 MB) and retains the default of 64 MB so users understand the valid range and when to increase it for large payloads.
66-80: 💤 Low valueConsider adding an example demonstrating
max_recv_msg_size_mbconfiguration.While the field has a sensible default, users who encounter payload size issues might benefit from seeing an example configuration with
max_recv_msg_size_mbexplicitly set (e.g., to 128 for large image payloads). This is optional since the field is documented in the Configuration Reference table.📝 Optional example addition
You could add a third config.json example for users handling large payloads:
**Large Payload Setup:** ```json { "vector_store": { "enabled": true, "type": "qdrant", "config": { "host": "your-qdrant-cluster.cloud.qdrant.io", "port": 6334, "api_key": "your-qdrant-api-key", "use_tls": true, "max_recv_msg_size_mb": 128 } } } ```🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/integrations/vector-databases/qdrant.mdx` around lines 66 - 80, Add an optional "Large Payload Setup" example to docs/integrations/vector-databases/qdrant.mdx showing how to set max_recv_msg_size_mb (e.g., 128) inside the existing vector_store config object; update the doc by inserting a third JSON snippet similar to the existing "Cloud Setup" block that includes the "max_recv_msg_size_mb" field so users handling large image/payloads can copy the config directly.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@docs/integrations/vector-databases/qdrant.mdx`:
- Line 94: The docs entry for the `max_recv_msg_size_mb` field omits the minimum
constraint; update the description for `max_recv_msg_size_mb` to state that the
value must be an integer >= 1 (minimum 1 MB) and retains the default of 64 MB so
users understand the valid range and when to increase it for large payloads.
- Around line 66-80: Add an optional "Large Payload Setup" example to
docs/integrations/vector-databases/qdrant.mdx showing how to set
max_recv_msg_size_mb (e.g., 128) inside the existing vector_store config object;
update the doc by inserting a third JSON snippet similar to the existing "Cloud
Setup" block that includes the "max_recv_msg_size_mb" field so users handling
large image/payloads can copy the config directly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 9fd1e290-d9c1-4b66-91ed-3fdca2161af1
📒 Files selected for processing (7)
docs/integrations/vector-databases/qdrant.mdxframework/go.modframework/vectorstore/qdrant.goframework/vectorstore/redis.goframework/vectorstore/weaviate.goplugins/semanticcache/main.gotransports/config.schema.json
✅ Files skipped from review due to trivial changes (1)
- framework/vectorstore/weaviate.go
The merge-base changed after approval.
e346771 to
d76a687
Compare
Merge activity
|
…mhq#3611) ## Summary Improves the Qdrant vector store integration by raising the gRPC receive message size limit to 64 MB (configurable), and adds dimension mismatch detection on `CreateNamespace` for Qdrant and Redis to surface clear errors when an existing collection/index has a different vector dimension than what the config expects. ## Changes - Added a `max_recv_msg_size_mb` field to `QdrantConfig` (default: 64 MB) that sets the gRPC `MaxCallRecvMsgSize` dial option. The previous 4 MB gRPC default caused silent failures when caching large payloads such as base64-encoded image generation responses. - Added dimension validation in `QdrantStore.CreateNamespace`: if the collection already exists and its vector dimension differs from the requested dimension, an actionable error is returned instead of silently proceeding. - Added the same dimension validation in `RedisStore.CreateNamespace` using `FT.INFO` attribute metadata. - Added a `Configuration Reference` table to the Qdrant integration docs covering all supported fields, their types, defaults, and descriptions. - Added `max_recv_msg_size_mb` to the JSON config schema with a minimum of 1 and a default of 64. - Promoted `google.golang.org/grpc` from an indirect to a direct dependency in `go.mod` since it is now explicitly used in `qdrant.go`. ## Type of change - [ ] Bug fix - [x] Feature - [ ] Refactor - [ ] Documentation - [ ] Chore/CI ## Affected areas - [x] Core (Go) - [x] Transports (HTTP) - [ ] Providers/Integrations - [ ] Plugins - [ ] UI (React) - [x] Docs ## How to test ```sh # Run framework tests cd framework go test ./vectorstore/... # Verify Qdrant with a large payload (e.g. base64 image response) # Configure a Qdrant vector store with max_recv_msg_size_mb unset (defaults to 64) # and cache an image generation response — confirm no gRPC message size error. # Verify dimension mismatch error # Point Bifrost at an existing Qdrant collection or Redis index with dimension N, # set vector_store_namespace to that collection and configure dimension M (M ≠ N). # Expected: startup fails with a clear error message indicating the mismatch and # instructing the user to rename the namespace or drop the collection manually. ``` New config field for Qdrant: | Field | Type | Default | Description | |---|---|---|---| | `max_recv_msg_size_mb` | integer | `64` | gRPC max receive message size in MB. Increase when caching large payloads such as base64-encoded image generation responses. | ## Breaking changes - [ ] Yes - [x] No ## Related issues ## Security considerations None. The `max_recv_msg_size_mb` field controls an internal gRPC buffer limit and does not affect authentication or secret handling. ## Checklist - [ ] I read `docs/contributing/README.md` and followed the guidelines - [ ] I added/updated tests where appropriate - [x] I updated documentation where needed - [ ] I verified builds succeed (Go and UI) - [ ] I verified the CI pipeline passes locally if applicable
…mhq#3611) ## Summary Improves the Qdrant vector store integration by raising the gRPC receive message size limit to 64 MB (configurable), and adds dimension mismatch detection on `CreateNamespace` for Qdrant and Redis to surface clear errors when an existing collection/index has a different vector dimension than what the config expects. ## Changes - Added a `max_recv_msg_size_mb` field to `QdrantConfig` (default: 64 MB) that sets the gRPC `MaxCallRecvMsgSize` dial option. The previous 4 MB gRPC default caused silent failures when caching large payloads such as base64-encoded image generation responses. - Added dimension validation in `QdrantStore.CreateNamespace`: if the collection already exists and its vector dimension differs from the requested dimension, an actionable error is returned instead of silently proceeding. - Added the same dimension validation in `RedisStore.CreateNamespace` using `FT.INFO` attribute metadata. - Added a `Configuration Reference` table to the Qdrant integration docs covering all supported fields, their types, defaults, and descriptions. - Added `max_recv_msg_size_mb` to the JSON config schema with a minimum of 1 and a default of 64. - Promoted `google.golang.org/grpc` from an indirect to a direct dependency in `go.mod` since it is now explicitly used in `qdrant.go`. ## Type of change - [ ] Bug fix - [x] Feature - [ ] Refactor - [ ] Documentation - [ ] Chore/CI ## Affected areas - [x] Core (Go) - [x] Transports (HTTP) - [ ] Providers/Integrations - [ ] Plugins - [ ] UI (React) - [x] Docs ## How to test ```sh # Run framework tests cd framework go test ./vectorstore/... # Verify Qdrant with a large payload (e.g. base64 image response) # Configure a Qdrant vector store with max_recv_msg_size_mb unset (defaults to 64) # and cache an image generation response — confirm no gRPC message size error. # Verify dimension mismatch error # Point Bifrost at an existing Qdrant collection or Redis index with dimension N, # set vector_store_namespace to that collection and configure dimension M (M ≠ N). # Expected: startup fails with a clear error message indicating the mismatch and # instructing the user to rename the namespace or drop the collection manually. ``` New config field for Qdrant: | Field | Type | Default | Description | |---|---|---|---| | `max_recv_msg_size_mb` | integer | `64` | gRPC max receive message size in MB. Increase when caching large payloads such as base64-encoded image generation responses. | ## Breaking changes - [ ] Yes - [x] No ## Related issues ## Security considerations None. The `max_recv_msg_size_mb` field controls an internal gRPC buffer limit and does not affect authentication or secret handling. ## Checklist - [ ] I read `docs/contributing/README.md` and followed the guidelines - [ ] I added/updated tests where appropriate - [x] I updated documentation where needed - [ ] I verified builds succeed (Go and UI) - [ ] I verified the CI pipeline passes locally if applicable

Summary
Improves the Qdrant vector store integration by raising the gRPC receive message size limit to 64 MB (configurable), and adds dimension mismatch detection on
CreateNamespacefor Qdrant and Redis to surface clear errors when an existing collection/index has a different vector dimension than what the config expects.Changes
max_recv_msg_size_mbfield toQdrantConfig(default: 64 MB) that sets the gRPCMaxCallRecvMsgSizedial option. The previous 4 MB gRPC default caused silent failures when caching large payloads such as base64-encoded image generation responses.QdrantStore.CreateNamespace: if the collection already exists and its vector dimension differs from the requested dimension, an actionable error is returned instead of silently proceeding.RedisStore.CreateNamespaceusingFT.INFOattribute metadata.Configuration Referencetable to the Qdrant integration docs covering all supported fields, their types, defaults, and descriptions.max_recv_msg_size_mbto the JSON config schema with a minimum of 1 and a default of 64.google.golang.org/grpcfrom an indirect to a direct dependency ingo.modsince it is now explicitly used inqdrant.go.Type of change
Affected areas
How to test
New config field for Qdrant:
max_recv_msg_size_mb64Breaking changes
Related issues
Security considerations
None. The
max_recv_msg_size_mbfield controls an internal gRPC buffer limit and does not affect authentication or secret handling.Checklist
docs/contributing/README.mdand followed the guidelines