new: remove vectors_count, update http and grpc models #1069
Conversation
✅ Deploy Preview for poetic-froyo-8baba7 ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
📝 WalkthroughWalkthroughThis diff adds collection-level JSON metadata and CollectionWarning; introduces Rrf/RrfQuery, AcornSearchParams, BM25/Bm25Config, and Match.text_any; extends ExtendedPointId to accept uuid.UUID and normalizes UUIDs in local flows. Vector representations migrated from flat to dense/multi-dense/sparse structures. New fields added across configs: max_payload_index_count, inline_storage, wal_retain_closed, ascii_folding, initial_state for shard keys. update_filter is threaded through Upsert/UpdateVectors/PointsUpdateOperation and uploaders. Protobufs, generated stubs, bidirectional Grpc↔Rest conversions, uploaders (gRPC/REST), client surfaces (sync/async/remote/local), local persistence, and many tests/fixtures were updated. Several legacy search/recommend/discover paths were removed or consolidated into query-centric APIs. Estimated code review effort🎯 5 (Critical) | ⏱️ ~150 minutes Areas to focus review on:
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (13)
qdrant_client/http/api/search_api.py (3)
97-98: Docstring: tighten phrasing (“differently”; “calculated as +1”).Minor grammar polish for clarity.
-... the score behaves a little different: The integer part ... +... the score behaves a little differently: The integer part ... -... is calculated +1 if the point is closer ... +... is calculated as +1 if the point is closer ...
524-525: Docstring: mirror the same phrasing fix as above.Keep async/sync docs consistent.
-... behaves a little different: The integer part ... +... behaves a little differently: The integer part ... -... is calculated +1 if ... +... is calculated as +1 if ...
747-748: Docstring: mirror the same phrasing fix.Consistency with the other two docstrings.
-... behaves a little different: The integer part ... +... behaves a little differently: The integer part ... -... is calculated +1 if ... +... is calculated as +1 if ...qdrant_client/http/api/distributed_api.py (1)
159-173: Add timeout to remove_peer: looks good; consider documenting units.Implementation follows existing query param pattern. Add “seconds” note in the docstring for clarity.
qdrant_client/proto/points.proto (2)
544-545: Nit: stray indent on timeout in CountPoints.Align with surrounding fields.
- optional uint64 timeout = 6; // If set, overrides global timeout setting for this request. Unit is seconds. + optional uint64 timeout = 6; // If set, overrides global timeout setting for this request. Unit is seconds.
1124-1125: Match.text_any — good addition; consider doc note on tokenization.If tokenization differs from “text”/“phrase”, mention it in API docs.
qdrant_client/http/models/models.py (5)
93-133: BM25 config looks good; consider constraining language type.Optional: tighten
languageto accepted values without losing'none'by usingUnion[Language, Literal["none"]]instead of barestr. Keeps schema self-documenting while preserving the special-case.Apply within this block:
- language: Optional[str] = Field( + language: Optional[Union["Language", Literal["none"]]] = Field( default=None, description="Defines which language to use for text preprocessing. This parameter is used to construct default stopwords filter and stemmer. To disable language-specific processing, set this to `'language': 'none'`. If not specified, English is assumed.", )
239-243: Minor doc punctuation fix for CollectionConfig.metadata.Add a period after “collection” for readability.
- metadata: Optional["Payload"] = Field( - default=None, - description="Arbitrary JSON metadata for the collection This can be used to store application-specific information such as creation time, migration data, inference model info, etc.", - ) + metadata: Optional["Payload"] = Field( + default=None, + description="Arbitrary JSON metadata for the collection. This can be used to store application-specific information such as creation time, migration data, inference model info, etc.", + )
596-599: Nit: spacing in description.“1.Defaults” → “1. Defaults”.
- description="The midpoint of the decay. Should be between 0 and 1.Defaults to 0.5. Output will be this value when `|x - target| == scale`.", + description="The midpoint of the decay. Should be between 0 and 1. Defaults to 0.5. Output will be this value when `|x - target| == scale`.",
1071-1074: Doc variable mismatch: full_scan_threshold vs full_scan_threshold_kb.The field is
full_scan_threshold, but the text referencesfull_scan_threshold_kb. Align to the actual field name.- description="Minimal size threshold (in KiloBytes) below which full-scan is preferred over HNSW search. This measures the total size of vectors being queried against. When the maximum estimated amount of points that a condition satisfies is smaller than `full_scan_threshold_kb`, the query planner will use full-scan search instead of HNSW index traversal for better performance. Note: 1Kb = 1 vector of size 256", + description="Minimal size threshold (in kilobytes) below which full-scan is preferred over HNSW search. This measures the total size of vectors being queried against. When the maximum estimated amount of points that a condition satisfies is smaller than `full_scan_threshold`, the query planner will use full-scan search instead of HNSW index traversal for better performance. Note: 1 kB ≈ 1 vector of size 256.",And similarly in HnswConfigDiff.
Also applies to: 1098-1101
3301-3304: Fill in missing description for UpdateVectors.update_filter.Keep it consistent with PointsList/PointsBatch.
-class UpdateVectors(BaseModel, extra="forbid"): +class UpdateVectors(BaseModel, extra="forbid"): points: List["PointVectors"] = Field(..., description="Points with named vectors") shard_key: Optional["ShardKeySelector"] = Field(default=None, description="") - update_filter: Optional["Filter"] = Field(default=None, description="") + update_filter: Optional["Filter"] = Field( + default=None, + description="If specified, only points that match this filter will be updated", + )qdrant_client/grpc/collections_pb2.pyi (1)
792-798: Minor doc polish: threshold name and units.Mirror the HTTP model: reference
full_scan_threshold(not_kb) and use “kilobytes”.- `full_scan_threshold`, the query planner will use full-scan search instead of HNSW index - traversal for better performance. - Note: 1Kb = 1 vector of size 256 + `full_scan_threshold`, the query planner will use full-scan search instead of HNSW index + traversal for better performance. + Note: 1 kB ≈ 1 vector of size 256.qdrant_client/grpc/points_pb2.pyi (1)
3398-3420: PointsUpdateOperation.PointStructList.update_filter: consistent with UpsertPointsGood symmetry for batch upsert. Consider adding a brief doc/example in client docs showing “conditional upsert” with update_filter.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
qdrant_client/conversions/conversion.py(0 hunks)qdrant_client/grpc/collections_pb2.py(5 hunks)qdrant_client/grpc/collections_pb2.pyi(16 hunks)qdrant_client/grpc/points_pb2.pyi(18 hunks)qdrant_client/http/api/distributed_api.py(4 hunks)qdrant_client/http/api/points_api.py(10 hunks)qdrant_client/http/api/search_api.py(3 hunks)qdrant_client/http/models/models.py(24 hunks)qdrant_client/proto/collections.proto(7 hunks)qdrant_client/proto/points.proto(10 hunks)tests/conversions/fixtures.py(0 hunks)
💤 Files with no reviewable changes (2)
- qdrant_client/conversions/conversion.py
- tests/conversions/fixtures.py
🧰 Additional context used
🧬 Code graph analysis (6)
qdrant_client/http/api/distributed_api.py (1)
qdrant_client/http/models/models.py (1)
InlineResponse200(1200-1204)
qdrant_client/grpc/collections_pb2.py (2)
qdrant_client/grpc/collections_pb2.pyi (6)
CreateCollection(1404-1529)MetadataEntry(1407-1422)MetadataEntry(1536-1551)MetadataEntry(1763-1778)UpdateCollection(1533-1622)CollectionConfig(1760-1824)qdrant_client/http/models/models.py (3)
CreateCollection(469-520)UpdateCollection(3245-3276)CollectionConfig(222-242)
qdrant_client/http/models/models.py (2)
qdrant_client/grpc/points_pb2.pyi (20)
b(3343-3344)options(318-319)options(360-361)options(403-404)update_filter(665-666)update_filter(807-808)update_filter(3405-3406)update_filter(3564-3565)points(657-657)points(703-704)points(798-799)points(1032-1033)points(3400-3400)points(3558-3559)points(3612-3613)points(3633-3634)points(4921-4922)points(5001-5001)Rrf(2753-2768)rrf(2811-2812)qdrant_client/grpc/collections_pb2.pyi (7)
stopwords(1972-1973)metadata(1478-1479)metadata(1589-1590)metadata(1806-1807)languages(1935-1936)custom(1938-1939)WalConfigDiff(878-904)
qdrant_client/grpc/collections_pb2.pyi (1)
qdrant_client/grpc/json_with_int_pb2.pyi (2)
value(61-61)Value(84-131)
qdrant_client/grpc/points_pb2.pyi (1)
qdrant_client/http/models/models.py (2)
Fusion(913-922)Rrf(2429-2434)
qdrant_client/http/api/points_api.py (2)
qdrant_client/grpc/points_pb2.pyi (1)
ReadConsistency(228-245)qdrant_client/http/models/models.py (2)
FacetRequest(793-803)InlineResponse20021(1296-1300)
🪛 Buf (1.55.1)
qdrant_client/proto/collections.proto
6-6: import "json_with_int.proto": file does not exist
(COMPILE)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Redirect rules - poetic-froyo-8baba7
- GitHub Check: Header rules - poetic-froyo-8baba7
- GitHub Check: Pages changed - poetic-froyo-8baba7
🔇 Additional comments (51)
qdrant_client/http/api/distributed_api.py (2)
266-276: Async remove_peer updated with timeout: LGTM.Signature and forwarding are consistent.
343-353: Sync remove_peer updated with timeout: LGTM.Signature and forwarding are consistent.
qdrant_client/proto/collections.proto (7)
175-181: HNSW full_scan_threshold doc: clearer semantics — nice.Good improvement to explain KB-based threshold and planner behavior.
219-220: WAL config: wal_retain_closed addition — OK.Backwards compatible field number; no collisions.
408-409: init_from_collection marked Deprecated — OK.Doc-only; no wire change.
413-414: Collection metadata map added — confirm Value source and merge semantics.Wire looks fine; ensure server implements merge behavior as described.
426-427: UpdateCollection.metadata — confirm “merge” behavior is deep vs. replace.Proto map updates typically replace entry values; clarify expected merge semantics for nested objects.
466-467: CollectionConfig.metadata — OK; aligns with HTTP models.Just ensure json_with_int.Value is available (see blocking comment above).
6-7: Resolved — json_with_int.proto is vendored and generated Python files are present.
Found at qdrant_client/proto/json_with_int.proto and qdrant_client/grpc/json_with_int_pb2.py, qdrant_client/grpc/json_with_int_pb2.pyi.qdrant_client/proto/points.proto (7)
138-139: update_filter on UpsertPoints — specify exact semantics.Clarify authoritative behavior when a point matches filter vs. doesn’t (partial update vs. insert) and how conflicts across shards are handled.
166-167: update_filter on UpdatePointVectors — consistent with UpsertPoints?Confirm identical filtering semantics here to avoid surprises.
642-642: Decay midpoint doc tweak — nice clarity.Improves understanding without wire changes.
672-676: New Rrf message — good extension for parameterized fusion.Backwards compatible and complements Fusion.RRF.
688-689: Query.rrf alongside Fusion — ensure no ambiguity.Since both “fusion” (enum) and “rrf” (message) exist in the oneof, document precedence/usage guidelines in public docs.
1036-1037: FacetResponse.usage — OK.Matches other responses; non-breaking.
8-9: Not blocking — dependency presentFound qdrant_client/proto/json_with_int.proto and generated modules qdrant_client/grpc/json_with_int_pb2.py (and .pyi).
qdrant_client/http/api/points_api.py (6)
262-278: facet builder: added consistency + preserved timeout — OK.Query params set consistently.
564-576: Async count_points signature updated — OK; same positional risk applies.Encourage keyword use in release notes.
633-643: Async facet signature updated — OK.Forwarding is correct.
871-882: Sync facet signature updated — OK.Matches builder and async.
126-140: Added consistency to count_points — breaks positional-arg callers; verify and add compatibility.Inserting the new "consistency" parameter before "timeout" changes the positional order so existing callers using positional args may pass their timeout into consistency; provide a compatibility plan (options: accept both orders via a thin wrapper, require keyword-only usage with a deprecation window, or move the new param to the end). Automated scans failed to conclusively find positional callers — manual verification required.
802-814: Sync count_points signature updated — approve but positional-arg compatibility riskChange is fine, but callers that pass >1 positional argument (i.e., rely on positional consistency/timeout/count_request) will break. Automated repo-wide checks did not complete; manually verify there are no such calls outside qdrant_client/http/api (qdrant_client/http/api/points_api.py — def count_points at ~lines 799–804), or make the new params keyword-only.
qdrant_client/grpc/collections_pb2.py (2)
150-159: MetadataEntry nested types registered — LGTM.Generated surfaces align with proto map additions.
16-17: No action needed — json_with_int_pb2 present
qdrant_client/grpc/json_with_int_pb2.py (and json_with_int_pb2.pyi) exist; the import will succeed.qdrant_client/http/models/models.py (13)
205-206: Telemetry peer metadata addition — LGTM.Shape matches intent; no concerns.
253-254: CollectionConfigTelemetry.metadata — LGTM.Consistent with CollectionConfig.
750-761: Document model/options update — LGTM.
DocumentOptionsunion withBm25Configis a nice improvement.
848-851: Feature flag appendable_quantization — LGTM.
1562-1568: MatchTextAny addition — LGTM.New variant is correctly wired into
Match.Also applies to: 3599-3605
1926-1932: PeerMetadata and ClusterTelemetry.peer_metadata — LGTM.Also applies to: 200-207
1971-1974: update_filter on upsert APIs — LGTM.Descriptions are clear and match gRPC semantics.
Also applies to: 1980-1983
2429-2439: RRF models and query variant — LGTM.Types look correct; union updated.
Also applies to: 3667-3676
3067-3073: StopwordsSet typing — LGTM.Enum-typed languages plus custom list is a good balance.
3273-3276: UpdateCollection.metadata — LGTM.Merge semantics are clearly documented.
3577-3580: DocumentOptions alias — LGTM.Union placement works with forward-ref usage in Document.
3671-3676: Query union wiring — LGTM.
3491-3494: Confirm server default/semantics for WAL retention.WalConfig.wal_retain_closed defaults to 1 in the HTTP model while gRPC exposes it as an optional Diff — verify the server-side default and align the HTTP model (set to server default or remove the hard default so Diff semantics are preserved) to avoid API drift.
Locations: qdrant_client/http/models/models.py (wal_retain_closed default=1 at ~3491–3493; Diff default=None at ~3499–3502), qdrant_client/proto/collections.proto (~line 219).qdrant_client/grpc/collections_pb2.pyi (5)
883-905: WAL retention diff field — LGTM.
1407-1501: CreateCollection.metadata map — LGTM.Typing via
json_with_int_pb2.Valueis appropriate; ClearField includes “metadata”.
1536-1606: UpdateCollection.metadata map — LGTM.Merge semantics reflected in the docstring.
1763-1821: CollectionConfig.metadata map — LGTM.
2191-2257: gRPC still exposes vectors_count — confirm intended parity with HTTPHTTP removed vectors_count, but gRPC CollectionInfo and conversion/test code still define/use it. Key locations: qdrant_client/grpc/collections_pb2.pyi, qdrant_client/conversions/conversion.py (checks/sets vectors_count), qdrant_client/local/local_collection.py (vectors_count=None), tests/conversions/fixtures.py, tests/test_async_qdrant_client.py (commented assertions). If removal across surfaces is intended: remove/adjust the gRPC field and update conversion logic and tests. Otherwise document the deliberate divergence.
qdrant_client/grpc/points_pb2.pyi (9)
157-166: Doc tweak for Fusion.RRF is fineClarifies default-params intent; consistent with HTTP enum.
2672-2672: DecayParamsExpression.midpoint doc tightenedGood clarification on valid range.
2753-2771: New Rrf message: API surface looks soundPublic alias (global___Rrf) present; field number and oneof helpers generated correctly.
4411-4428: FacetResponse.usage added: aligns with other responsesBrings facets in line with Search/Query/Count responses. Backward compatible (optional oneof).
4730-4755: Document oneof precedence for Match.text_any vs text/phrase
- Clarify precedence/behavior when clients set text_any vs text or phrase — oneof semantics mean only the last-set field is retained; add a short example showing which field “wins” and update the generated .pyi docstrings.
- Verify and update qdrant_client/grpc/points_pb2.pyi lines 4730–4755 (also applies to 4767–4772); I could not confirm the snippet programmatically (rg returned no matches), so please confirm the file/lines and apply the doc change.
651-651: Add/verify client tests for UpsertPoints.update_filter (round‑trip)Wiring looks present — proto, generated pb2/pyi expose update_filter and Upsert is exercised from qdrant_client/qdrant_remote.py, async_qdrant_remote.py and qdrant_client/uploader/grpc_uploader.py.
Add/verify tests that:
- upsert with update_filter matching existing points updates only those points;
- upsert with update_filter matching no points results in inserts.
792-792: update_filter added to UpdatePointVectors — OK, documented as update-onlyType/oneof plumbing is correct; proto (qdrant_client/proto/points.proto — UpdateVectors) and generated stub (qdrant_client/grpc/points_pb2.pyi — UpdatePointVectors) state "If specified, only points that match this filter will be updated" (no implicit insert).
3556-3578: PointsUpdateOperation.UpdateVectors.update_filter — wiring verified; add testProto, generated stubs, and conversion wiring include update_filter; add a unit/integration test asserting points that do NOT match update_filter remain untouched. Reuse fixture update_vectors_operation in tests/conversions/fixtures.py:1317–1321; see qdrant_client/proto/points.proto:824–827, qdrant_client/grpc/points_pb2.pyi:3564–3571, and qdrant_client/conversions/conversion.py:4471–4474.
2784-2830: fusion and rrf share the Query "variant" oneof — confirm intentqdrant_client/grpc/points_pb2.pyi shows Query's WhichOneof lists both "fusion" and "rrf" (setting one clears the other); if intentional, document the exclusivity and add a unit test that only one is set; if not, remove rrf from the oneof or update the conversion path (qdrant_client/conversions/conversion.py — around convert_fusion -> grpc.Query(...)) and adjust affected tests (e.g., tests/conversions/fixtures.py, tests/* referencing FusionQuery).
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
qdrant_client/embed/_inspection_cache.py (4)
1452-1521: Polish BM25 schema: add missing titles and clarify descriptions.
Add titles for tokenizer/stemmer and make language note less confusing.Apply:
"tokenizer": { "anyOf": [{"$ref": "#/$defs/TokenizerType"}, {"type": "null"}], "default": None, - "description": "Configuration of the local bm25 models.", + "description": "Tokenizer type used for BM25 preprocessing.", + "title": "Tokenizer", }, ... "language": { "anyOf": [{"type": "string"}, {"type": "null"}], "default": None, - "description": "Defines which language to use for text preprocessing. This parameter is used to construct default stopwords filter and stemmer. To disable language-specific processing, set this to `'language': 'none'`. If not specified, English is assumed.", + "description": "Defines which language to use for text preprocessing. Used to construct default stopwords and stemmer. To disable language-specific processing, set to 'none'. If not specified, English is assumed.", "title": "Language", }, ... "stemmer": { "anyOf": [{"$ref": "#/$defs/SnowballParams"}, {"type": "null"}], "default": None, "description": "Configuration of the stemmer. Processes tokens to their root form. Default: initialized Snowball stemmer for specified `language` or English if not specified.", + "title": "Stemmer", },
444-446: Fix punctuation in midpoint description.
Missing space after period.- "description": "The midpoint of the decay. Should be between 0 and 1.Defaults to 0.5. Output will be this value when `|x - target| == scale`.", + "description": "The midpoint of the decay. Should be between 0 and 1. Defaults to 0.5. Output will be this value when `|x - target| == scale`.",
1953-1955: Normalize units for clarity (KB vs Kb).
Avoid “Kb” (kilobits) and mixed casing.- "description": "Minimal size threshold (in KiloBytes) below which full-scan is preferred over HNSW search. This measures the total size of vectors being queried against. When the maximum estimated amount of points that a condition satisfies is smaller than `full_scan_threshold_kb`, the query planner will use full-scan search instead of HNSW index traversal for better performance. Note: 1Kb = 1 vector of size 256", + "description": "Minimal data-size threshold (in KB) below which full-scan is preferred over HNSW search. This measures the total size of vectors being queried against. When the maximum estimated amount of points that a condition satisfies is smaller than `full_scan_threshold_kb`, the planner will use full-scan instead of HNSW traversal for better performance. Note: 1 KB ≈ 1 vector of size 256.",
4233-4237: Clarify update_filter semantics; fill missing description in UpdateVectors.
Keep wording consistent and accurate (UpdateVectors can’t insert).# PointsBatch.update_filter - "description": "If specified, only points that match this filter will be updated, others will be inserted", + "description": "If specified, only points that match this filter will be updated; others will be inserted.", # PointsList.update_filter - "description": "If specified, only points that match this filter will be updated, others will be inserted", + "description": "If specified, only points that match this filter will be updated; others will be inserted.", # UpdateVectors.update_filter (missing text) - "description": "", + "description": "If specified, only points that match this filter will be updated; non-matching points are left unchanged.",Also applies to: 4266-4270, 4306-4310
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
qdrant_client/embed/_inspection_cache.py(20 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: Redirect rules - poetic-froyo-8baba7
- GitHub Check: Header rules - poetic-froyo-8baba7
- GitHub Check: Pages changed - poetic-froyo-8baba7
- GitHub Check: Python 3.12.x on ubuntu-latest test
- GitHub Check: Python 3.13.x on ubuntu-latest test
- GitHub Check: Python 3.11.x on ubuntu-latest test
- GitHub Check: Python 3.10.x on ubuntu-latest test
- GitHub Check: Python 3.9.x on ubuntu-latest test
🔇 Additional comments (7)
qdrant_client/embed/_inspection_cache.py (7)
9-9: LGTM: added Bm25Config to CACHE_STR_PATH.
83-83: LGTM: MatchTextAny wired through (cache path, FieldCondition, schema).Also applies to: 590-592, 1061-1074
209-211: LGTM: RRF (Rrf/RrfQuery) added and allowed in query unions.Also applies to: 3435-3436, 3558-3578, 3665-3666
1537-1541: LGTM: Document.options now accepts Bm25Config.
2385-2390: LGTM: WAL retention knob (wal_retain_closed).
1599-1719: LGTM: Language/Stopwords/Tokenizer defs added.
Choice to keep Bm25Config.language as free string (to allow 'none') makes sense alongside enumerated Language for stopwords.
3035-3038: LGTM: Fusion description updated to include RRF.
There was a problem hiding this comment.
Actionable comments posted: 13
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
qdrant_client/conversions/conversion.py (1)
1-1: Fix unsafe HasField on metadata and verify RRF conversion
- qdrant_client/conversions/conversion.py:267 — Replace model.HasField("metadata") with has_field(model, "metadata") (or check len(model.metadata)) — HasField on map fields is invalid and will raise at runtime.
- Sanity-check RRF handling: ensure Query rrf variant conversion reads the rrf message (val.k) rather than using model.k; tests reference grpc.Query(rrf=grpc.Rrf(...)) in tests/conversions/fixtures.py:1509–1511.
qdrant_client/async_qdrant_remote.py (2)
1728-1739: REST path: accept grpc.PointVectors too.Normalize inputs to REST model to avoid serialization errors.
else: + # Normalize gRPC PointVectors to REST model if provided by caller + if isinstance(points, list): + points = [ + (GrpcToRest.convert_point_vectors(p) if isinstance(p, grpc.PointVectors) else p) + for p in points + ] if isinstance(update_filter, grpc.Filter): update_filter = GrpcToRest.convert_filter(model=update_filter) return (
1694-1711: Change public client signatures: use Optional[types.Filter] for update_filterqdrant_client/async_qdrant_client.py:1524, 1599 and qdrant_client/qdrant_client.py:1572, 1653 currently annotate update_filter as Optional[types.Payload]; replace with Optional[types.Filter], update any public stubs/docs, and run type checks.
🧹 Nitpick comments (3)
qdrant_client/conversions/conversion.py (1)
2167-2173: Minor: Incorrect HasField usage condition.
if model.HasField("index") is not Noneis always True. Use the boolean directly to avoid converting unset defaults.Apply:
- if model.HasField("index") is not None + if model.HasField("index")qdrant_client/qdrant_remote.py (1)
1962-1962: Fix assertion message copy-paste.Should reference UpdateVectors, not Upsert.
- assert grpc_result is not None, "Upsert returned None result" + assert grpc_result is not None, "Update vectors returned None result"qdrant_client/async_qdrant_remote.py (1)
1725-1725: Fix assertion message copy-paste.Should reference UpdateVectors.
- assert grpc_result is not None, "Upsert returned None result" + assert grpc_result is not None, "Update vectors returned None result"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
qdrant_client/async_qdrant_client.py(15 hunks)qdrant_client/async_qdrant_remote.py(17 hunks)qdrant_client/conversions/conversion.py(10 hunks)qdrant_client/qdrant_client.py(15 hunks)qdrant_client/qdrant_remote.py(15 hunks)tests/conversions/fixtures.py(9 hunks)
🧰 Additional context used
🧬 Code graph analysis (4)
qdrant_client/qdrant_remote.py (3)
qdrant_client/http/models/models.py (2)
Filter(877-887)UpdateVectors(3300-3303)qdrant_client/grpc/points_pb2.pyi (3)
models(5151-5151)UpsertPoints(638-686)UpdatePointVectors(784-828)qdrant_client/conversions/conversion.py (6)
RestToGrpc(2491-4791)convert_filter(166-182)convert_filter(2493-2513)GrpcToRest(140-2481)convert_payload(581-582)convert_payload(2795-2796)
qdrant_client/async_qdrant_remote.py (6)
qdrant_client/http/models/models.py (2)
Filter(877-887)UpdateVectors(3300-3303)qdrant_client/conversions/conversion.py (8)
RestToGrpc(2491-4791)convert_filter(166-182)convert_filter(2493-2513)GrpcToRest(140-2481)convert_update_result(465-469)convert_update_result(2677-2681)convert_payload(581-582)convert_payload(2795-2796)qdrant_client/http/api/points_api.py (4)
upsert_points(746-761)upsert_points(984-999)update_vectors(729-744)update_vectors(967-982)qdrant_client/async_qdrant_client.py (1)
update_vectors(1592-1648)qdrant_client/qdrant_client.py (1)
update_vectors(1646-1704)qdrant_client/qdrant_remote.py (1)
update_vectors(1929-1977)
tests/conversions/fixtures.py (4)
qdrant_client/grpc/points_pb2.pyi (16)
Match(4718-4771)FieldCondition(4656-4714)match(4671-4672)Condition(4537-4574)field(4548-4548)params(1461-1462)params(1617-1618)params(1877-1878)params(2025-2026)params(2199-2200)params(2856-2857)params(2928-2929)params(3069-3070)Query(2772-2829)rrf(2811-2812)Rrf(2753-2768)qdrant_client/http/models/models.py (3)
FieldCondition(853-874)CollectionConfig(222-242)Rrf(2429-2434)qdrant_client/grpc/collections_pb2.pyi (20)
metadata(1478-1479)metadata(1589-1590)metadata(1806-1807)CollectionConfig(1760-1824)params(492-492)params(513-513)params(1571-1572)params(1788-1789)params(2170-2171)hnsw_config(352-353)hnsw_config(397-398)hnsw_config(1443-1444)hnsw_config(1574-1575)hnsw_config(1791-1792)optimizer_config(1794-1795)wal_config(1446-1447)wal_config(1797-1798)strict_mode_config(1475-1476)strict_mode_config(1586-1587)strict_mode_config(1803-1804)qdrant_client/grpc/json_with_int_pb2.pyi (1)
Value(84-131)
qdrant_client/conversions/conversion.py (3)
qdrant_client/grpc/collections_pb2.pyi (8)
metadata(1478-1479)metadata(1589-1590)metadata(1806-1807)shard_key(2472-2473)shard_key(2502-2503)shard_key(2556-2556)shard_key(2755-2756)shard_key(2786-2787)qdrant_client/grpc/points_pb2.pyi (55)
HasField(243-243)HasField(269-269)HasField(307-307)HasField(348-348)HasField(369-369)HasField(391-391)HasField(412-412)HasField(467-467)HasField(514-514)HasField(611-611)HasField(677-677)HasField(720-720)HasField(771-771)HasField(819-819)HasField(849-849)HasField(889-889)rrf(2811-2812)Rrf(2753-2768)update_filter(665-666)update_filter(807-808)update_filter(3405-3406)update_filter(3564-3565)upsert(3659-3659)points(657-657)points(703-704)points(798-799)points(1032-1033)points(3400-3400)points(3558-3559)points(3612-3613)points(3633-3634)points(4921-4922)points(5001-5001)shard_key(3842-3843)shard_key(4206-4207)shard_key_selector(662-663)shard_key_selector(709-710)shard_key_selector(756-757)shard_key_selector(804-805)shard_key_selector(877-878)shard_key_selector(941-942)shard_key_selector(994-995)shard_key_selector(1038-1039)shard_key_selector(1478-1479)shard_key_selector(1639-1640)shard_key_selector(1771-1772)shard_key_selector(1821-1822)shard_key_selector(1905-1906)shard_key_selector(2058-2059)shard_key_selector(2217-2218)shard_key_selector(2309-2310)UpdateVectors(3551-3578)Match(4718-4771)Query(2772-2829)PointStructList(3393-3419)qdrant_client/http/models/models.py (6)
MatchTextAny(1562-1567)RrfQuery(2437-2438)Rrf(2429-2434)UpsertOperation(3310-3311)PointsList(1977-1983)UpdateVectors(3300-3303)
🪛 GitHub Actions: Integration tests
qdrant_client/conversions/conversion.py
[error] 267-267: HasField() used on non-presence-enabled field 'metadata' causing ValueError: Field qdrant.CollectionConfig.metadata does not have presence.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Redirect rules - poetic-froyo-8baba7
- GitHub Check: Header rules - poetic-froyo-8baba7
- GitHub Check: Pages changed - poetic-froyo-8baba7
🔇 Additional comments (18)
tests/conversions/fixtures.py (4)
38-38: Additions for Match.text_any look correct and cover filter wiring.This mirrors the proto’s Match.text_any oneof and is exercised via FieldCondition/Condition and Filter. LGTM.
Also applies to: 52-52, 129-129, 150-151
369-369: Collection metadata fixture is valid.Using grpc.Value(integer_value=3) matches the new map<string, Value> metadata; extra config instance is helpful for round-trip tests. LGTM.
Also applies to: 379-386
1509-1511: RRF query fixtures: good coverage (k set, default, explicit None).These will surface conversion bugs quickly. LGTM.
Also applies to: 1738-1741
1571-1571: Include collection_config_w_metadata in fixtures registry.Nice—ensures REST<->gRPC conversion paths see metadata. LGTM.
qdrant_client/conversions/conversion.py (7)
679-681: text_any support is correct.Maps Match.text_any → rest.MatchTextAny. LGTM.
1979-1987: Propagating update_filter for Upsert is correct.Matches new proto semantics. LGTM.
2088-2096: Propagating update_filter for UpdateVectors is correct.Round-trip symmetry is maintained later in RestToGrpc. LGTM.
2869-2870: REST→gRPC metadata conversion looks right.No presence check; passes through only when provided. LGTM.
3045-3046: REST MatchTextAny → gRPC text_any mapping is correct.Covers the new oneof case. LGTM.
4382-4392: REST→gRPC Upsert update_filter wiring is correct.Field is optional and handled safely. LGTM.
4497-4509: REST→gRPC UpdateVectors update_filter wiring is correct.Matches proto UpdateVectors.update_filter. LGTM.
qdrant_client/qdrant_client.py (3)
2258-2258: Metadata support for update_collection looks correct.Signature + forwarding are consistent with docstring.
Also applies to: 2299-2300
2342-2342: Create-collection metadata passthrough LGTM.Parameter order and forwarding are consistent across clients.
Also applies to: 2412-2413
2435-2435: Recreate-collection metadata passthrough LGTM.Matches update/create; deprecation notice preserved.
Also applies to: 2512-2513
qdrant_client/async_qdrant_client.py (3)
2184-2184: Async update_collection metadata passthrough LGTM.Matches sync surface and forwards correctly.
Also applies to: 2222-2223
2264-2264: Async create_collection metadata passthrough LGTM.Consistent and non-breaking.
Also applies to: 2333-2334
2354-2354: Async recreate_collection metadata passthrough LGTM.Consistent with other methods.
Also applies to: 2429-2430
qdrant_client/qdrant_remote.py (1)
1847-1847: API surface addition looks good.
update_filteron upsert/update_vectors is consistently threaded across gRPC/REST code paths.Also applies to: 1936-1936
qdrant_client/async_qdrant_remote.py
Outdated
| if isinstance(metadata, models.Payload): | ||
| metadata = RestToGrpc.convert_payload(metadata) | ||
| return ( |
There was a problem hiding this comment.
Fix isinstance check for metadata; always convert for gRPC.
Same issue as sync client: convert whenever metadata is provided.
- if isinstance(metadata, models.Payload):
- metadata = RestToGrpc.convert_payload(metadata)
+ if metadata is not None:
+ metadata = RestToGrpc.convert_payload(metadata)
...
- timeout=timeout,
- metadata=metadata,
+ timeout=timeout,
+ metadata=metadata,Also applies to: 2414-2415
🤖 Prompt for AI Agents
In qdrant_client/async_qdrant_remote.py around lines 2400-2402 and 2414-2415,
the isinstance(metadata, models.Payload) guard prevents converting provided
metadata for gRPC in many cases; change the logic to always convert metadata
when it is not None (or truthy) rather than only when it's an instance of
models.Payload — call RestToGrpc.convert_payload(metadata) unconditionally when
metadata is present and return that converted value (or pass it into the gRPC
payload) so gRPC always receives the converted payload.
| metadata = RestToGrpc.convert_payload(metadata) | ||
| create_collection = grpc.CreateCollection( |
There was a problem hiding this comment.
Same metadata conversion bug in create_collection (gRPC path).
Unconditionally convert here too.
- if isinstance(metadata, models.Payload):
- metadata = RestToGrpc.convert_payload(metadata)
+ if metadata is not None:
+ metadata = RestToGrpc.convert_payload(metadata)Also applies to: 2532-2533
🤖 Prompt for AI Agents
In qdrant_client/async_qdrant_remote.py around lines 2515-2516 (and similarly at
2532-2533), the create_collection gRPC path is missing an unconditional
conversion of the metadata payload; call RestToGrpc.convert_payload(metadata)
and assign the result back to metadata before building the grpc.CreateCollection
object so the metadata is always converted for gRPC, and apply the same change
at the second occurrence (lines 2532-2533).
| if isinstance(update_filter, grpc.Filter): | ||
| update_filter = GrpcToRest.convert_filter(model=update_filter) | ||
|
|
||
| return self.openapi_client.points_api.update_vectors( | ||
| collection_name=collection_name, | ||
| wait=wait, | ||
| update_vectors=models.UpdateVectors( | ||
| points=points, | ||
| shard_key=shard_key_selector, | ||
| update_filter=update_filter, | ||
| ), | ||
| ordering=ordering, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
REST path: accept grpc.PointVectors too.
If callers pass gRPC point vectors, current REST path will fail JSON encoding. Convert when needed.
else:
+ # Normalize gRPC PointVectors to REST model if provided by caller
+ if isinstance(points, list):
+ points = [
+ (GrpcToRest.convert_point_vectors(p) if isinstance(p, grpc.PointVectors) else p)
+ for p in points
+ ]
if isinstance(update_filter, grpc.Filter):
update_filter = GrpcToRest.convert_filter(model=update_filter)
return self.openapi_client.points_api.update_vectors(📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if isinstance(update_filter, grpc.Filter): | |
| update_filter = GrpcToRest.convert_filter(model=update_filter) | |
| return self.openapi_client.points_api.update_vectors( | |
| collection_name=collection_name, | |
| wait=wait, | |
| update_vectors=models.UpdateVectors( | |
| points=points, | |
| shard_key=shard_key_selector, | |
| update_filter=update_filter, | |
| ), | |
| ordering=ordering, | |
| # Normalize gRPC PointVectors to REST model if provided by caller | |
| if isinstance(points, list): | |
| points = [ | |
| (GrpcToRest.convert_point_vectors(p) if isinstance(p, grpc.PointVectors) else p) | |
| for p in points | |
| ] | |
| if isinstance(update_filter, grpc.Filter): | |
| update_filter = GrpcToRest.convert_filter(model=update_filter) | |
| return self.openapi_client.points_api.update_vectors( | |
| collection_name=collection_name, | |
| wait=wait, | |
| update_vectors=models.UpdateVectors( | |
| points=points, | |
| shard_key=shard_key_selector, | |
| update_filter=update_filter, | |
| ), | |
| ordering=ordering, |
🤖 Prompt for AI Agents
In qdrant_client/qdrant_remote.py around lines 1965 to 1976, the REST
update_vectors call will fail if callers pass gRPC PointVectors because they are
not JSON-serializable; detect when points are gRPC types (single
grpc.PointVectors or an iterable of grpc.PointVectors) and convert them to
REST-compatible structures before constructing models.UpdateVectors (use the
existing GrpcToRest conversion helpers — e.g. GrpcToRest.convert_point_vectors
or convert_point for each element). Perform the conversion in-place just before
calling openapi_client.points_api.update_vectors so update_vectors.points is
always a REST-friendly object. Ensure you handle both single and list inputs and
preserve shard_key_selector, update_filter conversion logic unchanged.
qdrant_client/qdrant_remote.py
Outdated
| if isinstance(metadata, models.Payload): | ||
| metadata = RestToGrpc.convert_payload(metadata) | ||
|
|
There was a problem hiding this comment.
Same metadata conversion bug in create_collection (gRPC path).
Mirror the unconditional conversion here as well.
- if isinstance(metadata, models.Payload):
- metadata = RestToGrpc.convert_payload(metadata)
+ if metadata is not None:
+ metadata = RestToGrpc.convert_payload(metadata)Also applies to: 2809-2809
🤖 Prompt for AI Agents
In qdrant_client/qdrant_remote.py around lines 2790-2792 (and also at 2809), the
metadata conversion is guarded by an isinstance(models.Payload) check causing
the gRPC create_collection path to skip conversion; remove the isinstance check
and unconditionally call RestToGrpc.convert_payload(metadata) for those
occurrences so metadata is always converted to the gRPC payload format.
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
qdrant_client/qdrant_remote.py (2)
2695-2706: Convert gRPC metadata to REST JSON for update_collection (REST path)If callers pass gRPC map containers, OpenAPI client will choke on non-JSON values. Normalize to REST JSON before request.
result: Optional[bool] = self.http.collections_api.update_collection( collection_name, update_collection=models.UpdateCollection( optimizers_config=optimizers_config, params=collection_params, vectors=vectors_config, hnsw_config=hnsw_config, quantization_config=quantization_config, sparse_vectors=sparse_vectors_config, strict_mode_config=strict_mode_config, - metadata=metadata, + metadata=GrpcToRest.convert_payload(metadata) if metadata is not None and not isinstance(metadata, Mapping) else metadata, ), timeout=timeout, ).result
2828-2843: Convert gRPC metadata to REST JSON for create_collection (REST path)Mirror the normalization when building the CreateCollection request.
- create_collection_request = models.CreateCollection( + create_collection_request = models.CreateCollection( vectors=vectors_config, shard_number=shard_number, replication_factor=replication_factor, write_consistency_factor=write_consistency_factor, on_disk_payload=on_disk_payload, hnsw_config=hnsw_config, optimizers_config=optimizers_config, wal_config=wal_config, quantization_config=quantization_config, init_from=init_from, sparse_vectors=sparse_vectors_config, sharding_method=sharding_method, strict_mode_config=strict_mode_config, - metadata=metadata, + metadata=GrpcToRest.convert_payload(metadata) if metadata is not None and not isinstance(metadata, Mapping) else metadata, )
🧹 Nitpick comments (8)
tests/conversions/fixtures.py (1)
38-38: Good coverage for text_any; optionally include base Match fixtureAdding Match(text_any=...) and wiring it through FieldCondition/Condition/filter is solid. Optionally, also add match_text_any into the "Match" fixtures list to hit convert_match directly without going through FieldCondition.
Also applies to: 52-52, 129-151
qdrant_client/conversions/conversion.py (2)
2869-2870: Nit: avoid passing None into map fields (constructor)While passing None usually no-ops, prefer skipping the arg to avoid surprises with map fields in some protobuf versions.
Option:
- return grpc.CollectionConfig( + kwargs = dict( params=cls.convert_collection_params(model.params), hnsw_config=cls.convert_hnsw_config(model.hnsw_config), optimizer_config=cls.convert_optimizers_config(model.optimizer_config), wal_config=cls.convert_wal_config(model.wal_config), quantization_config=( cls.convert_quantization_config(model.quantization_config) if model.quantization_config is not None else None ), strict_mode_config=( cls.convert_strict_mode_config_output(model.strict_mode_config) if model.strict_mode_config is not None else None ), - metadata=cls.convert_payload(model.metadata) if model.metadata is not None else None, - ) + ) + if model.metadata is not None: + kwargs["metadata"] = cls.convert_payload(model.metadata) + return grpc.CollectionConfig(**kwargs)
3644-3646: LGTM with minor robustness: RRF (REST -> gRPC)Works as-is. Optionally avoid setting k when None.
Suggested tweak:
- return grpc.Query(rrf=grpc.Rrf(k=model.rrf.k)) + return grpc.Query(rrf=grpc.Rrf(k=model.rrf.k if model.rrf.k is not None else None))qdrant_client/async_qdrant_client.py (1)
2354-2355: Recreate-collection metadata support is consistentDocs and forwarding look fine. Minor nit: “Arbitrary JSON” vs “JSON-like” phrasing differs from update/create; unify wording if you re-generate docs.
- metadata: Arbitrary JSON metadata for the collection + metadata: Arbitrary JSON-like metadata for the collectionAlso applies to: 2401-2401, 2428-2429
qdrant_client/async_qdrant_remote.py (3)
1701-1702: Fix assert message; optionally harden REST branch conversions
- The assert message mentions “Upsert” in update_vectors; fix the wording.
- Optional: in REST path, convert grpc.PointVectors to REST models (parity with gRPC path) to avoid JSON-serialization issues if callers pass gRPC objects.
- assert grpc_result is not None, "Upsert returned None result" + assert grpc_result is not None, "UpdateVectors returned None result"else: + # Optional hardening: accept grpc.PointVectors when prefer_grpc=False + points = [ + (GrpcToRest.convert_point_vectors(p) if isinstance(p, grpc.PointVectors) else p) + for p in points + ] if isinstance(update_filter, grpc.Filter): update_filter = GrpcToRest.convert_filter(model=update_filter) return ( await self.openapi_client.points_api.update_vectors( collection_name=collection_name, wait=wait, update_vectors=models.UpdateVectors( - points=points, shard_key=shard_key_selector, update_filter=update_filter + points=points, shard_key=shard_key_selector, update_filter=update_filter ), ordering=ordering, ) ).resultAlso applies to: 1711-1712, 1720-1721, 1725-1725, 1728-1739
2378-2379: Convert metadata to REST payload in REST path (when given gRPC map)If callers pass a gRPC map container, the REST client receives non-JSON objects. Convert to REST payload before building the request.
if isinstance(strict_mode_config, models.StrictModeConfig): strict_mode_config = RestToGrpc.convert_strict_mode_config(strict_mode_config) - if isinstance(metadata, models.Payload): - metadata = RestToGrpc.convert_payload(metadata) + if isinstance(metadata, models.Payload): + metadata = RestToGrpc.convert_payload(metadata) return ( await self.grpc_collections.Update( grpc.UpdateCollection( collection_name=collection_name, ... timeout=timeout, - metadata=metadata, + metadata=metadata, ), timeout=timeout if timeout is not None else self._timeout, ) ).result - if isinstance(optimizers_config, grpc.OptimizersConfigDiff): + if isinstance(optimizers_config, grpc.OptimizersConfigDiff): optimizers_config = GrpcToRest.convert_optimizers_config_diff(optimizers_config) ... + # Normalize gRPC metadata to REST payload if needed + if metadata is not None and not isinstance(metadata, dict): + metadata = GrpcToRest.convert_payload(metadata) result: Optional[bool] = ( await self.http.collections_api.update_collection( collection_name, update_collection=models.UpdateCollection( ... - metadata=metadata, + metadata=metadata, ), timeout=timeout, ) ).resultAlso applies to: 2400-2415, 2440-2444
2483-2484: Do the same metadata normalization for create_collection (REST path)Mirror the conversion so REST always receives JSON-serializable payloads.
if isinstance(strict_mode_config, models.StrictModeConfig): strict_mode_config = RestToGrpc.convert_strict_mode_config(strict_mode_config) - if isinstance(metadata, models.Payload): - metadata = RestToGrpc.convert_payload(metadata) + if isinstance(metadata, models.Payload): + metadata = RestToGrpc.convert_payload(metadata) create_collection = grpc.CreateCollection( ... strict_mode_config=strict_mode_config, - metadata=metadata, + metadata=metadata, ) return ( await self.grpc_collections.Create(create_collection, timeout=self._timeout) ).result @@ - create_collection_request = models.CreateCollection( + # Normalize gRPC metadata to REST payload if needed + if metadata is not None and not isinstance(metadata, dict): + metadata = GrpcToRest.convert_payload(metadata) + create_collection_request = models.CreateCollection( ... - metadata=metadata, + metadata=metadata, )Also applies to: 2515-2516, 2532-2533, 2561-2562
qdrant_client/qdrant_remote.py (1)
1936-1977: Parity: convert grpc.PointVectors in REST pathREST branch does not convert
grpc.PointVectorsto REST models; gRPC branch does. For symmetry and to honor the “accept both gRPC and REST structures” contract, convert items when needed.else: + # Normalize potential gRPC inputs to REST models + if isinstance(points, list): + points = [ + (GrpcToRest.convert_point_vectors(p) if isinstance(p, grpc.PointVectors) else p) + for p in points + ] if isinstance(update_filter, grpc.Filter): update_filter = GrpcToRest.convert_filter(model=update_filter) return self.openapi_client.points_api.update_vectors(
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
qdrant_client/async_qdrant_client.py(15 hunks)qdrant_client/async_qdrant_remote.py(17 hunks)qdrant_client/conversions/conversion.py(10 hunks)qdrant_client/qdrant_client.py(15 hunks)qdrant_client/qdrant_remote.py(15 hunks)tests/conversions/fixtures.py(9 hunks)
🧰 Additional context used
🧬 Code graph analysis (4)
qdrant_client/async_qdrant_remote.py (5)
qdrant_client/http/models/models.py (2)
Filter(877-887)UpdateVectors(3300-3303)qdrant_client/conversions/conversion.py (8)
RestToGrpc(2491-4791)convert_filter(166-182)convert_filter(2493-2513)GrpcToRest(140-2481)convert_update_result(465-469)convert_update_result(2677-2681)convert_payload(581-582)convert_payload(2795-2796)qdrant_client/async_qdrant_client.py (1)
update_vectors(1592-1648)qdrant_client/qdrant_client.py (1)
update_vectors(1646-1704)qdrant_client/qdrant_remote.py (1)
update_vectors(1929-1977)
qdrant_client/qdrant_remote.py (4)
qdrant_client/http/models/models.py (2)
Filter(877-887)UpdateVectors(3300-3303)qdrant_client/grpc/points_pb2.pyi (3)
models(5151-5151)UpsertPoints(638-686)UpdatePointVectors(784-828)qdrant_client/conversions/conversion.py (8)
RestToGrpc(2491-4791)convert_filter(166-182)convert_filter(2493-2513)GrpcToRest(140-2481)convert_update_result(465-469)convert_update_result(2677-2681)convert_payload(581-582)convert_payload(2795-2796)qdrant_client/async_qdrant_remote.py (2)
grpc_points(276-284)update_vectors(1694-1739)
tests/conversions/fixtures.py (4)
qdrant_client/grpc/points_pb2.pyi (16)
Match(4718-4771)FieldCondition(4656-4714)match(4671-4672)Condition(4537-4574)field(4548-4548)params(1461-1462)params(1617-1618)params(1877-1878)params(2025-2026)params(2199-2200)params(2856-2857)params(2928-2929)params(3069-3070)Query(2772-2829)rrf(2811-2812)Rrf(2753-2768)qdrant_client/http/models/models.py (3)
FieldCondition(853-874)CollectionConfig(222-242)Rrf(2429-2434)qdrant_client/grpc/collections_pb2.pyi (20)
metadata(1478-1479)metadata(1589-1590)metadata(1806-1807)CollectionConfig(1760-1824)params(492-492)params(513-513)params(1571-1572)params(1788-1789)params(2170-2171)hnsw_config(352-353)hnsw_config(397-398)hnsw_config(1443-1444)hnsw_config(1574-1575)hnsw_config(1791-1792)optimizer_config(1794-1795)wal_config(1446-1447)wal_config(1797-1798)strict_mode_config(1475-1476)strict_mode_config(1586-1587)strict_mode_config(1803-1804)qdrant_client/grpc/json_with_int_pb2.pyi (1)
Value(84-131)
qdrant_client/conversions/conversion.py (4)
qdrant_client/grpc/collections_pb2.pyi (8)
metadata(1478-1479)metadata(1589-1590)metadata(1806-1807)shard_key(2472-2473)shard_key(2502-2503)shard_key(2556-2556)shard_key(2755-2756)shard_key(2786-2787)qdrant_client/grpc/points_pb2.pyi (56)
HasField(243-243)HasField(269-269)HasField(307-307)HasField(348-348)HasField(369-369)HasField(391-391)HasField(412-412)HasField(467-467)HasField(514-514)HasField(611-611)HasField(677-677)HasField(720-720)HasField(771-771)HasField(819-819)HasField(849-849)HasField(889-889)rrf(2811-2812)Rrf(2753-2768)update_filter(665-666)update_filter(807-808)update_filter(3405-3406)update_filter(3564-3565)points(657-657)points(703-704)points(798-799)points(1032-1033)points(3400-3400)points(3558-3559)points(3612-3613)points(3633-3634)points(4921-4922)points(5001-5001)shard_key(3842-3843)shard_key(4206-4207)shard_key_selector(662-663)shard_key_selector(709-710)shard_key_selector(756-757)shard_key_selector(804-805)shard_key_selector(877-878)shard_key_selector(941-942)shard_key_selector(994-995)shard_key_selector(1038-1039)shard_key_selector(1478-1479)shard_key_selector(1639-1640)shard_key_selector(1771-1772)shard_key_selector(1821-1822)shard_key_selector(1905-1906)shard_key_selector(2058-2059)shard_key_selector(2217-2218)shard_key_selector(2309-2310)update_vectors(3671-3671)UpdateVectors(3551-3578)Match(4718-4771)Query(2772-2829)PointsUpdateOperation(3390-3694)PointStructList(3393-3419)qdrant_client/async_qdrant_remote.py (2)
rest(309-315)update_vectors(1694-1739)qdrant_client/http/models/models.py (6)
MatchTextAny(1562-1567)RrfQuery(2437-2438)Rrf(2429-2434)PointsList(1977-1983)UpdateVectorsOperation(3306-3307)UpdateVectors(3300-3303)
🪛 GitHub Actions: Integration tests
qdrant_client/conversions/conversion.py
[error] 267-267: ValueError: Field qdrant.CollectionConfig.metadata does not have presence. Command: coverage run --include=qdrant_client/conversions/conversion.py -m pytest tests/conversions/test_validate_conversions.py -vv -s
🔇 Additional comments (20)
tests/conversions/fixtures.py (2)
369-369: CollectionConfig.metadata fixture looks correctThe metadata map uses json_with_int.Value as required and is threaded into a dedicated CollectionConfig fixture. This pairs well with the new conversion paths.
Also applies to: 379-386, 1571-1571
1509-1511: RRF query fixtures are well-formed; they will surface a conversion bugThe three RRF variants (k set, default, explicit None) are useful. Note: current GrpcToRest.convert_query incorrectly reads k from the wrong object (see separate comment in conversion.py), so these tests should catch it.
Also applies to: 1727-1741
qdrant_client/conversions/conversion.py (6)
679-681: LGTM: text_any support in Match conversionNew variant correctly maps to rest.MatchTextAny.
1979-1987: LGTM: upsert update_filter propagation (gRPC -> REST)Correctly threads optional update_filter into UpsertOperation.PointsList.
2088-2096: LGTM: update_vectors update_filter propagation (gRPC -> REST)Correctly threads optional update_filter into UpdateVectors.
3044-3046: LGTM: text_any support (REST -> gRPC)Maps MatchTextAny to grpc.Match(text_any=...).
4382-4392: LGTM: upsert update_filter propagation (REST -> gRPC)Correctly forwards optional update_filter.
4497-4509: LGTM: update_vectors update_filter propagation (REST -> gRPC)Correctly forwards optional update_filter.
qdrant_client/async_qdrant_client.py (2)
2184-2185: Collection metadata API addition looks goodSignature, docs, and wiring to remote all align. No issues.
Also applies to: 2201-2202, 2222-2223
2264-2265: Create-collection metadata support is correctly wiredGood addition and pass-through to remote.
Also applies to: 2311-2312, 2333-2334
qdrant_client/async_qdrant_remote.py (2)
2590-2591: Recreate-collection metadata passthrough is fineReuse via create_collection keeps behavior consistent.
Also applies to: 2610-2611
1623-1624: update_filter is exposed via the generated REST models — use point_insert_operations.update_filterupsert_points (qdrant_client/http/api/points_api.py — async ~line 746, sync ~984) accepts a point_insert_operations model; qdrant_client/http/models/models.py includes update_filter (e.g., UpdateVectors.update_filter ~line 3303 and the PointInsertOperations-related field ~line 1971), so the REST upsert supports update_filter via the model (no change required).
qdrant_client/qdrant_remote.py (4)
1847-1894: LGTM: update_filter support in upsertAddition, conversions, and forwarding look correct for both gRPC and REST paths.
Also applies to: 1916-1925
2632-2633: LGTM: collection-level metadata (gRPC path) in update_collectionNew field is correctly wired through gRPC.
Also applies to: 2675-2676
2746-2747: LGTM: collection-level metadata (gRPC path) in create_collectionField is propagated and converted for gRPC.
Also applies to: 2809-2810
2871-2872: LGTM: metadata passthrough in recreate_collectionParameter is consistently forwarded to create step.
Also applies to: 2892-2893
qdrant_client/qdrant_client.py (4)
1598-1600: Docstring correctness for update_filterAfter fixing the type, the existing descriptions remain accurate. No further changes needed.
Also applies to: 1682-1684
2258-2260: LGTM: expose metadata on update_collection wrapperPublic surface matches underlying client and docs.
Also applies to: 2299-2301
2392-2392: LGTM: expose metadata on create_collection wrapperParameter is properly forwarded.
Also applies to: 2412-2413
2435-2436: LGTM: expose metadata on recreate_collection wrapperConsistent with update/create paths.
Also applies to: 2512-2514
| metadata=cls.convert_payload(model.metadata) if model.HasField("metadata") else None, | ||
| ) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Fix: HasField on protobuf map (metadata) causes runtime error
Map fields don’t have presence; calling HasField("metadata") raises “does not have presence”. Use length/emptiness instead.
Apply:
- metadata=cls.convert_payload(model.metadata) if model.HasField("metadata") else None,
+ metadata=cls.convert_payload(model.metadata) if len(model.metadata) != 0 else None,📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| metadata=cls.convert_payload(model.metadata) if model.HasField("metadata") else None, | |
| ) | |
| metadata=cls.convert_payload(model.metadata) if len(model.metadata) != 0 else None, | |
| ) |
🧰 Tools
🪛 GitHub Actions: Integration tests
[error] 267-267: ValueError: Field qdrant.CollectionConfig.metadata does not have presence. Command: coverage run --include=qdrant_client/conversions/conversion.py -m pytest tests/conversions/test_validate_conversions.py -vv -s
🤖 Prompt for AI Agents
In qdrant_client/conversions/conversion.py around lines 267-268, the code calls
model.HasField("metadata") on a protobuf map which raises a runtime error
because map fields have no presence; replace the HasField check with an
emptiness check (e.g., if model.metadata and len(model.metadata) > 0 or simply
if model.metadata) and pass metadata=cls.convert_payload(model.metadata) only
when the map is non-empty (otherwise None), ensuring no use of HasField for map
fields.
💡 Verification agent
🧩 Analysis chain
Quick verification for all map-field presence checks and RRF k usage
Run to ensure no other HasField calls on map fields and to confirm there are no remaining incorrect reads of k.
Also applies to: 1355-1358
🏁 Script executed:
#!/bin/bash
# Find HasField misuse on map fields
rg -nP 'HasField\(\s*"metadata"\s*\)|HasField\(\s*"payload_schema"\s*\)' -C2
# Spot any other RRF conversions reading k from the wrong symbol
rg -nP 'Rrf.*\bk\b' qdrant_client/conversions/conversion.py -n -C2Length of output: 797
Fix HasField on map field and wrong RRF.k source
- qdrant_client/conversions/conversion.py:267 — "metadata" is a proto map; HasField("metadata") is invalid. Replace with a presence check (e.g., bool(model.metadata) or len(model.metadata) > 0) and call convert_payload when non-empty.
- qdrant_client/conversions/conversion.py:1355–1357 — code does
val = model.rrfthen returnsrest.Rrf(k=model.k);model.kis wrong. Useval.kormodel.rrf.k(serialization at 3644–3645 usesmodel.rrf.k, make them consistent).
🧰 Tools
🪛 GitHub Actions: Integration tests
[error] 267-267: ValueError: Field qdrant.CollectionConfig.metadata does not have presence. Command: coverage run --include=qdrant_client/conversions/conversion.py -m pytest tests/conversions/test_validate_conversions.py -vv -s
🤖 Prompt for AI Agents
In qdrant_client/conversions/conversion.py around line 267 and lines ~1355–1357,
the code uses model.HasField("metadata") for a proto map (invalid) and later
reads model.rrf into val but constructs rest.Rrf(k=model.k) using the wrong
source; change the metadata check to test presence/emptiness (e.g.,
bool(model.metadata) or len(model.metadata) > 0) and only call convert_payload
when non-empty, and in the rrf conversion return rest.Rrf(k=val.k) (or
rest.Rrf(k=model.rrf.k)) to use the k from the rrf object consistently with
serialization lines 3644–3645.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
qdrant_client/conversions/conversion.py (1)
2169-2175: Bug: incorrectHasFieldtest makesindexalways set.
if model.HasField("index") is not Noneis always true; it should be a plain boolean check.- index=( - cls.convert_sparse_index_config(model.index) - if model.HasField("index") is not None - else None - ), + index=( + cls.convert_sparse_index_config(model.index) + if model.HasField("index") + else None + ),
♻️ Duplicate comments (1)
qdrant_client/conversions/conversion.py (1)
267-268: Fix presence check for protobuf map fieldmetadata.Map fields have no presence;
model.metadata is not Noneis always true. Use emptiness to decide whether to include the field.- metadata=cls.convert_payload(model.metadata) if model.metadata is not None else None, + metadata=cls.convert_payload(model.metadata) if len(model.metadata) != 0 else None,
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
qdrant_client/conversions/conversion.py(13 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
qdrant_client/conversions/conversion.py (4)
qdrant_client/grpc/collections_pb2.pyi (8)
metadata(1478-1479)metadata(1589-1590)metadata(1806-1807)shard_key(2472-2473)shard_key(2502-2503)shard_key(2556-2556)shard_key(2755-2756)shard_key(2786-2787)qdrant_client/qdrant_remote.py (3)
rest(351-357)upsert(1840-1927)update_vectors(1929-1977)qdrant_client/http/models/models.py (5)
MatchTextAny(1562-1567)RrfQuery(2437-2438)Rrf(2429-2434)PointsList(1977-1983)UpdateVectors(3300-3303)qdrant_client/grpc/points_pb2.pyi (57)
rrf(2811-2812)Rrf(2753-2768)HasField(243-243)HasField(269-269)HasField(307-307)HasField(348-348)HasField(369-369)HasField(391-391)HasField(412-412)HasField(467-467)HasField(514-514)HasField(611-611)HasField(677-677)HasField(720-720)HasField(771-771)HasField(819-819)HasField(849-849)HasField(889-889)update_filter(665-666)update_filter(807-808)update_filter(3405-3406)update_filter(3564-3565)upsert(3659-3659)points(657-657)points(703-704)points(798-799)points(1032-1033)points(3400-3400)points(3558-3559)points(3612-3613)points(3633-3634)points(4921-4922)points(5001-5001)shard_key(3842-3843)shard_key(4206-4207)shard_key_selector(662-663)shard_key_selector(709-710)shard_key_selector(756-757)shard_key_selector(804-805)shard_key_selector(877-878)shard_key_selector(941-942)shard_key_selector(994-995)shard_key_selector(1038-1039)shard_key_selector(1478-1479)shard_key_selector(1639-1640)shard_key_selector(1771-1772)shard_key_selector(1821-1822)shard_key_selector(1905-1906)shard_key_selector(2058-2059)shard_key_selector(2217-2218)shard_key_selector(2309-2310)update_vectors(3671-3671)UpdateVectors(3551-3578)Match(4718-4771)Query(2772-2829)PointsUpdateOperation(3390-3694)PointStructList(3393-3419)
🪛 GitHub Actions: Integration tests
qdrant_client/conversions/conversion.py
[error] 766-766: pydantic ValidationError: UpdateCollection.metadata expects a dictionary but received a tuple (input_value=({},), input_type=tuple) in convert_update_collection (conversion.py:766).
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Redirect rules - poetic-froyo-8baba7
- GitHub Check: Header rules - poetic-froyo-8baba7
- GitHub Check: Pages changed - poetic-froyo-8baba7
🔇 Additional comments (4)
qdrant_client/conversions/conversion.py (4)
679-681: LGTM: adds gRPC→REST support forMatch.text_any.Mapping to
rest.MatchTextAny(text_any=val)is correct.
1358-1361: LGTM: correct RRF gRPC→REST conversion and optionalk.Reading
kfrommodel.rrfand preserving optionality is right.
1982-1984: LGTM: wiresupdate_filterfor upsert/update_vectors (gRPC→REST).Propagating
update_filterto REST payload aligns with new API.Also applies to: 1989-1990, 2091-2093, 2098-2099
3049-3050: LGTM: adds REST→gRPC support forMatchTextAny.
grpc.Match(text_any=...)matches the proto.
| metadata=( | ||
| cls.convert_payload(model.metadata) if model.metadata is not None else None, | ||
| ), | ||
| ) |
There was a problem hiding this comment.
Critical: tuple passed to UpdateCollection.metadata (causes ValidationError).
The extra parentheses + trailing comma build a tuple ({},). Return a dict or None instead. Also, avoid presence checks on map fields; use emptiness.
- metadata=(
- cls.convert_payload(model.metadata) if model.metadata is not None else None,
- ),
+ metadata=cls.convert_payload(model.metadata) if len(model.metadata) != 0 else None,📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| metadata=( | |
| cls.convert_payload(model.metadata) if model.metadata is not None else None, | |
| ), | |
| ) | |
| metadata=cls.convert_payload(model.metadata) if len(model.metadata) != 0 else None, | |
| ) |
🤖 Prompt for AI Agents
In qdrant_client/conversions/conversion.py around lines 792 to 795, the code
currently wraps the metadata result in parentheses and a trailing comma causing
a tuple (e.g. ({},),) to be passed to UpdateCollection.metadata which triggers a
ValidationError; change the call to pass either the dict returned by
cls.convert_payload(model.metadata) or None (no tuple), and replace any presence
checks on map fields with emptiness checks (e.g., use if model.metadata and
len(model.metadata) > 0 or simply if model.metadata) so an empty map becomes
None or an empty dict as appropriate before assignment.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (3)
qdrant_client/conversions/conversion.py (3)
267-268: Fix presence check for protobuf map field metadata (use emptiness, not None).Map fields are always non-None; use len(model.metadata) to decide None vs dict.
Apply:
- metadata=cls.convert_payload(model.metadata) if model.metadata is not None else None, + # Map fields have no presence; use emptiness to omit when empty + metadata=( + cls.convert_payload(model.metadata) if len(model.metadata) != 0 else None + ),
792-793: Same map-field presence issue for UpdateCollection.metadata.Use emptiness check to avoid always populating metadata.
Apply:
- metadata=(cls.convert_payload(model.metadata) if model.metadata is not None else None), + metadata=( + cls.convert_payload(model.metadata) if len(model.metadata) != 0 else None + ),
3646-3648: Preserve optional k in REST→gRPC RRF (don’t set k=None).Avoid passing None into proto optional; set only when present.
Apply:
- if isinstance(model, rest.RrfQuery): - return grpc.Query(rrf=grpc.Rrf(k=model.rrf.k)) + if isinstance(model, rest.RrfQuery): + rrf = grpc.Rrf() + if model.rrf.k is not None: + rrf.k = model.rrf.k + return grpc.Query(rrf=rrf)
🧹 Nitpick comments (1)
tests/conversions/fixtures.py (1)
369-370: Metadata fixture looks good; add an empty-metadata variant to exercise “omit when empty”.Helps validate converters return None (not {}) when map is empty.
Also applies to: 379-386
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
qdrant_client/conversions/conversion.py(13 hunks)tests/conversions/fixtures.py(10 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
qdrant_client/conversions/conversion.py (4)
qdrant_client/grpc/collections_pb2.pyi (8)
metadata(1478-1479)metadata(1589-1590)metadata(1806-1807)shard_key(2472-2473)shard_key(2502-2503)shard_key(2556-2556)shard_key(2755-2756)shard_key(2786-2787)qdrant_client/qdrant_remote.py (3)
rest(351-357)upsert(1840-1927)update_vectors(1929-1977)qdrant_client/http/models/models.py (6)
MatchTextAny(1562-1567)RrfQuery(2437-2438)Rrf(2429-2434)UpsertOperation(3310-3311)PointsList(1977-1983)UpdateVectors(3300-3303)qdrant_client/grpc/points_pb2.pyi (57)
rrf(2811-2812)Rrf(2753-2768)HasField(243-243)HasField(269-269)HasField(307-307)HasField(348-348)HasField(369-369)HasField(391-391)HasField(412-412)HasField(467-467)HasField(514-514)HasField(611-611)HasField(677-677)HasField(720-720)HasField(771-771)HasField(819-819)HasField(849-849)HasField(889-889)update_filter(665-666)update_filter(807-808)update_filter(3405-3406)update_filter(3564-3565)upsert(3659-3659)points(657-657)points(703-704)points(798-799)points(1032-1033)points(3400-3400)points(3558-3559)points(3612-3613)points(3633-3634)points(4921-4922)points(5001-5001)shard_key(3842-3843)shard_key(4206-4207)shard_key_selector(662-663)shard_key_selector(709-710)shard_key_selector(756-757)shard_key_selector(804-805)shard_key_selector(877-878)shard_key_selector(941-942)shard_key_selector(994-995)shard_key_selector(1038-1039)shard_key_selector(1478-1479)shard_key_selector(1639-1640)shard_key_selector(1771-1772)shard_key_selector(1821-1822)shard_key_selector(1905-1906)shard_key_selector(2058-2059)shard_key_selector(2217-2218)shard_key_selector(2309-2310)update_vectors(3671-3671)UpdateVectors(3551-3578)Match(4718-4771)Query(2772-2829)PointsUpdateOperation(3390-3694)PointStructList(3393-3419)
tests/conversions/fixtures.py (4)
qdrant_client/grpc/points_pb2.pyi (16)
Match(4718-4771)FieldCondition(4656-4714)match(4671-4672)Condition(4537-4574)field(4548-4548)params(1461-1462)params(1617-1618)params(1877-1878)params(2025-2026)params(2199-2200)params(2856-2857)params(2928-2929)params(3069-3070)Query(2772-2829)rrf(2811-2812)Rrf(2753-2768)qdrant_client/http/models/models.py (3)
FieldCondition(853-874)CollectionConfig(222-242)Rrf(2429-2434)qdrant_client/grpc/collections_pb2.pyi (20)
metadata(1478-1479)metadata(1589-1590)metadata(1806-1807)CollectionConfig(1760-1824)params(492-492)params(513-513)params(1571-1572)params(1788-1789)params(2170-2171)hnsw_config(352-353)hnsw_config(397-398)hnsw_config(1443-1444)hnsw_config(1574-1575)hnsw_config(1791-1792)optimizer_config(1794-1795)wal_config(1446-1447)wal_config(1797-1798)strict_mode_config(1475-1476)strict_mode_config(1586-1587)strict_mode_config(1803-1804)qdrant_client/grpc/json_with_int_pb2.pyi (1)
Value(84-131)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Redirect rules - poetic-froyo-8baba7
- GitHub Check: Header rules - poetic-froyo-8baba7
- GitHub Check: Pages changed - poetic-froyo-8baba7
🔇 Additional comments (6)
tests/conversions/fixtures.py (3)
38-39: Nice addition: text_any fixtures are wired through match → field_condition → condition → filter.Covers the full path for text_any. No changes requested.
Also applies to: 52-53, 129-130, 150-151
1510-1513: Good RRF coverage (k set, default, explicit None).Ensure REST→gRPC preserves optional k by not setting it when None (see conversion counterpart fix suggestion in conversion.py).
1572-1573: Including both configs in fixtures is 👍Ensures metadata path is covered alongside the base config.
qdrant_client/conversions/conversion.py (3)
679-681: text_any conversion added correctly.Mapping grpc.Match.text_any → rest.MatchTextAny is correct and complete.
1356-1359: RRF (gRPC→REST) fix is correct.Now reads k from rrf and preserves optionality. LGTM.
1981-1988: Approve — update_filter plumbing verifiedupdate_filter propagation confirmed: RestToGrpc/GrpcToRest conversions are invoked and update_filter is forwarded at HTTP and gRPC call sites (qdrant_client/qdrant_remote.py, async_qdrant_remote.py, qdrant_client.py, async_qdrant_client.py). No changes required.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
qdrant_client/qdrant_remote.py (2)
1841-1848: Public API type mismatch — update_filter must be types.Filter (not types.Payload)Remote layers use Optional[types.Filter] but QdrantClient/AsyncQdrantClient expose Optional[types.Payload]; standardize the public wrappers to types.Filter to avoid API/type-hint drift and mypy errors.
- qdrant_client/qdrant_remote.py — update_filter: Optional[types.Filter] (lines 1847, 1936)
- qdrant_client/async_qdrant_remote.py — update_filter: Optional[types.Filter] (lines 1623, 1701)
- qdrant_client/qdrant_client.py — update_filter: Optional[types.Payload] (lines 1572, 1653) ← change to Optional[types.Filter]
- qdrant_client/async_qdrant_client.py — update_filter: Optional[types.Payload] (lines 1524, 1599) ← change to Optional[types.Filter]
Action: update QdrantClient and AsyncQdrantClient signatures + docstrings/tests to use Optional[types.Filter] and adjust any callers.
1930-1938: Public API type mismatch — update_filter must be Optional[types.Filter]
- Change public client wrappers that use Optional[types.Payload] (qdrant_client/qdrant_client.py:~1572, qdrant_client/async_qdrant_client.py:~1524 and corresponding client_base/async_client_base wrappers) to Optional[types.Filter] to match qdrant_client/qdrant_remote.py, the conversion logic, and the HTTP/gRPC models.
- Update docs/type hints to reflect the same public API.
♻️ Duplicate comments (4)
qdrant_client/qdrant_remote.py (4)
1911-1925: REST upsert: pass update_filter inside body object, not as top-level kwargGenerated REST clients typically expect update_filter inside PointsList/PointsBatch; passing it as a method kwarg can raise “unexpected keyword argument”. Also convert update_filter before constructing the body.
Apply:
@@ - if isinstance(points, list): + if isinstance(update_filter, grpc.Filter): + update_filter = GrpcToRest.convert_filter(model=update_filter) + + if isinstance(points, list): points = [ ( GrpcToRest.convert_point_struct(point) if isinstance(point, grpc.PointStruct) else point ) for point in points ] - - points = models.PointsList(points=points, shard_key=shard_key_selector) + points = models.PointsList( + points=points, + shard_key=shard_key_selector, + update_filter=update_filter, + ) - if isinstance(points, models.Batch): - points = models.PointsBatch(batch=points, shard_key=shard_key_selector) - - if isinstance(update_filter, grpc.Filter): - update_filter = GrpcToRest.convert_filter(model=update_filter) + if isinstance(points, models.Batch): + points = models.PointsBatch( + batch=points, + shard_key=shard_key_selector, + update_filter=update_filter, + ) @@ http_result = self.openapi_client.points_api.upsert_points( collection_name=collection_name, wait=wait, point_insert_operations=points, ordering=ordering, - update_filter=update_filter, ).resultVerify method signature in generated client:
#!/bin/bash fd -a 'points_api.py' qdrant_client/http/api | while read -r f; do echo "== $f =="; rg -nP -C2 'def\s+upsert_points\s*\(' "$f"; rg -n 'update_filter' "$f" || true done
2661-2663: Bug: invalid isinstance check for metadata (Payload); conversion skippedmodels.Payload is a typing alias; get_args(models.Payload) yields type arguments (e.g., (str, Any)), not a runtime class tuple. This path will skip conversion and send raw JSON to gRPC, causing type errors.
Apply:
- if isinstance(metadata, get_args(models.Payload)): - metadata = RestToGrpc.convert_payload(metadata) + if isinstance(metadata, Mapping): + # If already a mapping of grpc.Value, keep as-is; otherwise convert JSON -> grpc.Value + first_val = next(iter(metadata.values()), None) + if not isinstance(first_val, grpc.Value): + metadata = RestToGrpc.convert_payload(metadata)
2790-2792: Same metadata isinstance bug in create_collection (gRPC path)Repeat the safe Mapping + grpc.Value-aware conversion here.
- if isinstance(metadata, get_args(models.Payload)): - metadata = RestToGrpc.convert_payload(metadata) + if isinstance(metadata, Mapping): + first_val = next(iter(metadata.values()), None) + if not isinstance(first_val, grpc.Value): + metadata = RestToGrpc.convert_payload(metadata)
1965-1975: REST update_vectors should accept grpc.PointVectors — normalize before sendingIf callers pass gRPC PointVectors, JSON encoding fails. Convert to REST models before building the request.
Apply:
if isinstance(update_filter, grpc.Filter): update_filter = GrpcToRest.convert_filter(model=update_filter) + # Normalize gRPC PointVectors to REST model if provided + if isinstance(points, list): + points = [ + (GrpcToRest.convert_point_vectors(p) if isinstance(p, grpc.PointVectors) else p) + for p in points + ] + return self.openapi_client.points_api.update_vectors( collection_name=collection_name, wait=wait, update_vectors=models.UpdateVectors( points=points, shard_key=shard_key_selector, update_filter=update_filter, ), ordering=ordering, ).result
🧹 Nitpick comments (1)
qdrant_client/qdrant_remote.py (1)
1961-1963: Nit: wrong assertion messageThis is update_vectors, but the message says “Upsert returned None result”.
- assert grpc_result is not None, "Upsert returned None result" + assert grpc_result is not None, "Update vectors returned None result"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
qdrant_client/qdrant_remote.py(15 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
qdrant_client/qdrant_remote.py (6)
qdrant_client/http/models/models.py (2)
Filter(877-887)UpdateVectors(3300-3303)qdrant_client/grpc/points_pb2.pyi (3)
models(5151-5151)UpsertPoints(638-686)UpdatePointVectors(784-828)qdrant_client/conversions/conversion.py (8)
RestToGrpc(2492-4794)convert_filter(166-182)convert_filter(2494-2514)GrpcToRest(140-2482)convert_update_result(465-469)convert_update_result(2678-2682)convert_payload(581-582)convert_payload(2796-2797)qdrant_client/async_qdrant_client.py (2)
grpc_points(169-177)update_vectors(1592-1648)qdrant_client/qdrant_client.py (2)
grpc_points(187-196)update_vectors(1646-1704)qdrant_client/async_qdrant_remote.py (2)
grpc_points(276-284)update_vectors(1694-1739)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Redirect rules - poetic-froyo-8baba7
- GitHub Check: Header rules - poetic-froyo-8baba7
- GitHub Check: Pages changed - poetic-froyo-8baba7
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
tests/test_qdrant_client.py (1)
1177-1178: Strengthen assertion with a type check (guards against unexpected model wrappers).Verify the field is a plain dict before equality.
- assert collection_info.config.metadata == collection_metadata + assert isinstance(collection_info.config.metadata, dict) + assert collection_info.config.metadata == collection_metadata
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
tests/test_qdrant_client.py(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
tests/test_qdrant_client.py (7)
qdrant_client/async_qdrant_remote.py (2)
create_collection(2467-2574)get_collection(2340-2354)qdrant_client/qdrant_remote.py (2)
create_collection(2730-2855)get_collection(2597-2609)qdrant_client/async_qdrant_client.py (2)
create_collection(2245-2335)get_collection(2149-2159)qdrant_client/qdrant_client.py (2)
create_collection(2323-2414)get_collection(2221-2232)qdrant_client/local/async_qdrant_local.py (2)
create_collection(916-960)get_collection(870-872)qdrant_client/local/qdrant_local.py (2)
create_collection(988-1039)get_collection(939-941)qdrant_client/grpc/collections_pb2.pyi (1)
config(2228-2229)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: Redirect rules - poetic-froyo-8baba7
- GitHub Check: Header rules - poetic-froyo-8baba7
- GitHub Check: Pages changed - poetic-froyo-8baba7
- GitHub Check: Python 3.12.x on ubuntu-latest test
- GitHub Check: Python 3.13.x on ubuntu-latest test
- GitHub Check: Python 3.10.x on ubuntu-latest test
- GitHub Check: Python 3.9.x on ubuntu-latest test
- GitHub Check: Python 3.11.x on ubuntu-latest test
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (4)
qdrant_client/grpc/collections_pb2.pyi (1)
11-11: Use package-relative import for sibling proto module.
import json_with_int_pb2will try to resolve a top-level module and breaks once the client is installed asqdrant_client.grpc. Generated stubs need relative imports to load their siblings reliably. Switch tofrom . import json_with_int_pb2(same fix was called out earlier).-import json_with_int_pb2 +from . import json_with_int_pb2qdrant_client/conversions/conversion.py (3)
3656-3658: Preserve optionalkfield: avoid setting toNone.Passing
model.rrf.kdirectly togrpc.Rrf(k=...)will set the protobuf optional field even whenkisNone. Only includekwhen it's present.Apply this diff:
- if isinstance(model, rest.RrfQuery): - return grpc.Query(rrf=grpc.Rrf(k=model.rrf.k)) + if isinstance(model, rest.RrfQuery): + rrf = grpc.Rrf() + if model.rrf.k is not None: + rrf.k = model.rrf.k + return grpc.Query(rrf=rrf)
267-267: Use emptiness check instead ofis not Nonefor protobuf map field.Protobuf map fields like
model.metadataare always present (neverNone), somodel.metadata is not Noneis alwaysTrue. Check emptiness instead.Apply this diff:
- metadata=cls.convert_payload(model.metadata) if model.metadata is not None else None, + metadata=cls.convert_payload(model.metadata) if model.metadata else None,
794-794: Use emptiness check for protobuf map field.Same issue as line 267:
model.metadatais a protobuf map that's neverNone. The check should test emptiness.Apply this diff:
- metadata=(cls.convert_payload(model.metadata) if model.metadata is not None else None), + metadata=cls.convert_payload(model.metadata) if model.metadata else None,
🧹 Nitpick comments (4)
qdrant_client/http/models/models.py (2)
1078-1081: Fix docstring: field name mismatch (full_scan_threshold_kb→full_scan_threshold).Both HnswConfig and HnswConfigDiff reference
full_scan_threshold_kbin text, but the field isfull_scan_threshold.- description="Minimal size threshold (in KiloBytes) below which full-scan is preferred over HNSW search. This measures the total size of vectors being queried against. When the maximum estimated amount of points that a condition satisfies is smaller than `full_scan_threshold_kb`, the query planner will use full-scan search instead of HNSW index traversal for better performance. Note: 1Kb = 1 vector of size 256", + description="Minimal size threshold (in KiloBytes) below which full-scan is preferred over HNSW search. This measures the total size of vectors being queried against. When the maximum estimated amount of points that a condition satisfies is smaller than `full_scan_threshold`, the query planner will use full-scan search instead of HNSW index traversal for better performance. Note: 1Kb = 1 vector of size 256",Also applies to: 1109-1112
3326-3329: Add missing description for UpdateVectors.update_filter.Align with PointsBatch/PointsList and gRPC doc.
class UpdateVectors(BaseModel, extra="forbid"): points: List["PointVectors"] = Field(..., description="Points with named vectors") shard_key: Optional["ShardKeySelector"] = Field(default=None, description="") - update_filter: Optional["Filter"] = Field(default=None, description="") + update_filter: Optional["Filter"] = Field( + default=None, + description="If specified, only points that match this filter will be updated", + )qdrant_client/conversions/conversion.py (2)
2816-2816: Consider None check for optional field consistency.
copy_vectorsis assigned directly without checking if it'sNone. While this may work, other optional fields in the same function (e.g.,ef_construct,m,on_disk) use explicitNonechecks for consistency.Apply this diff if you want consistency:
- copy_vectors=model.copy_vectors, + copy_vectors=model.copy_vectors if model.copy_vectors is not None else None,Also applies to line 2891.
2879-2879: Check for empty dict, not justNone.When
model.metadatais an empty dict{}, it will still pass theis not Nonecheck and callconvert_payload({}). Consider checking truthiness to skip empty dicts.Apply this diff:
- metadata=cls.convert_payload(model.metadata) if model.metadata is not None else None, + metadata=cls.convert_payload(model.metadata) if model.metadata else None,Also applies to line 3020.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
qdrant_client/conversions/conversion.py(21 hunks)qdrant_client/grpc/collections_pb2.py(7 hunks)qdrant_client/grpc/collections_pb2.pyi(27 hunks)qdrant_client/http/models/models.py(31 hunks)qdrant_client/proto/collections.proto(11 hunks)tests/conversions/fixtures.py(11 hunks)
🧰 Additional context used
🧬 Code graph analysis (5)
tests/conversions/fixtures.py (4)
qdrant_client/grpc/points_pb2.pyi (8)
Match(4718-4771)FieldCondition(4656-4714)match(4671-4672)Condition(4537-4574)field(4548-4548)Query(2772-2829)rrf(2811-2812)Rrf(2753-2768)qdrant_client/http/models/models.py (4)
FieldCondition(860-881)StrictModeConfig(3091-3136)CollectionConfig(222-242)Rrf(2444-2449)qdrant_client/grpc/collections_pb2.pyi (8)
StrictModeConfig(1227-1351)metadata(1527-1528)metadata(1638-1639)metadata(1855-1856)CollectionConfig(1809-1873)strict_mode_config(1524-1525)strict_mode_config(1635-1636)strict_mode_config(1852-1853)qdrant_client/grpc/json_with_int_pb2.pyi (1)
Value(84-131)
qdrant_client/grpc/collections_pb2.py (2)
qdrant_client/grpc/collections_pb2.pyi (7)
CollectionWarning(773-783)CreateCollection(1453-1578)MetadataEntry(1456-1471)MetadataEntry(1585-1600)MetadataEntry(1812-1827)UpdateCollection(1582-1671)CollectionConfig(1809-1873)qdrant_client/http/models/models.py (4)
CollectionWarning(359-360)CreateCollection(476-527)UpdateCollection(3270-3301)CollectionConfig(222-242)
qdrant_client/conversions/conversion.py (4)
qdrant_client/grpc/collections_pb2.pyi (24)
metadata(1527-1528)metadata(1638-1639)metadata(1855-1856)HasField(375-375)HasField(411-411)HasField(439-439)HasField(471-471)HasField(501-501)HasField(522-522)HasField(544-544)HasField(570-570)HasField(658-658)HasField(702-702)HasField(750-750)HasField(847-847)HasField(892-892)HasField(922-922)HasField(1025-1025)HasField(1067-1067)shard_key(2520-2521)shard_key(2550-2551)shard_key(2604-2604)shard_key(2803-2804)shard_key(2834-2835)qdrant_client/async_qdrant_remote.py (2)
upsert(1616-1695)update_vectors(1697-1742)qdrant_client/http/models/models.py (5)
MatchTextAny(1577-1582)RrfQuery(2452-2453)Rrf(2444-2449)PointsList(1992-1998)UpdateVectors(3325-3328)qdrant_client/grpc/points_pb2.pyi (41)
rrf(2811-2812)Rrf(2753-2768)update_filter(665-666)update_filter(807-808)update_filter(3405-3406)update_filter(3564-3565)upsert(3659-3659)points(657-657)points(703-704)points(798-799)points(1032-1033)points(3400-3400)points(3558-3559)points(3612-3613)points(3633-3634)points(4921-4922)points(5001-5001)shard_key(3842-3843)shard_key(4206-4207)shard_key_selector(662-663)shard_key_selector(709-710)shard_key_selector(756-757)shard_key_selector(804-805)shard_key_selector(877-878)shard_key_selector(941-942)shard_key_selector(994-995)shard_key_selector(1038-1039)shard_key_selector(1478-1479)shard_key_selector(1639-1640)shard_key_selector(1771-1772)shard_key_selector(1821-1822)shard_key_selector(1905-1906)shard_key_selector(2058-2059)shard_key_selector(2217-2218)shard_key_selector(2309-2310)update_vectors(3671-3671)UpdateVectors(3551-3578)Match(4718-4771)Query(2772-2829)PointsUpdateOperation(3390-3694)PointStructList(3393-3419)
qdrant_client/grpc/collections_pb2.pyi (2)
qdrant_client/http/models/models.py (1)
CollectionWarning(359-360)qdrant_client/grpc/json_with_int_pb2.pyi (2)
value(61-61)Value(84-131)
qdrant_client/http/models/models.py (2)
qdrant_client/grpc/points_pb2.pyi (20)
b(3343-3344)options(318-319)options(360-361)options(403-404)update_filter(665-666)update_filter(807-808)update_filter(3405-3406)update_filter(3564-3565)points(657-657)points(703-704)points(798-799)points(1032-1033)points(3400-3400)points(3558-3559)points(3612-3613)points(3633-3634)points(4921-4922)points(5001-5001)Rrf(2753-2768)rrf(2811-2812)qdrant_client/grpc/collections_pb2.pyi (8)
stopwords(2021-2022)metadata(1527-1528)metadata(1638-1639)metadata(1855-1856)CollectionWarning(773-783)languages(1984-1985)custom(1987-1988)WalConfigDiff(903-929)
🪛 Buf (1.57.2)
qdrant_client/proto/collections.proto
6-6: import "json_with_int.proto": file does not exist
(COMPILE)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Redirect rules - poetic-froyo-8baba7
- GitHub Check: Header rules - poetic-froyo-8baba7
- GitHub Check: Pages changed - poetic-froyo-8baba7
🔇 Additional comments (2)
qdrant_client/grpc/collections_pb2.py (1)
16-16: json_with_int_pb2 confirmed present Verified thatjson_with_int_pb2.pyand its stub exist underqdrant_client/grpcand are tracked by git.qdrant_client/proto/collections.proto (1)
6-7:json_with_int.protoimport is correct. The file exists atproto/json_with_int.protoand stubs ingrpc/json_with_int_pb2.pyconfirm successful code generation.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
qdrant_client/embed/_inspection_cache.py(24 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: Redirect rules - poetic-froyo-8baba7
- GitHub Check: Header rules - poetic-froyo-8baba7
- GitHub Check: Pages changed - poetic-froyo-8baba7
- GitHub Check: Python 3.12.x on ubuntu-latest test
- GitHub Check: Python 3.13.x on ubuntu-latest test
- GitHub Check: Python 3.11.x on ubuntu-latest test
- GitHub Check: Python 3.9.x on ubuntu-latest test
- GitHub Check: Python 3.10.x on ubuntu-latest test
🔇 Additional comments (10)
qdrant_client/embed/_inspection_cache.py (10)
9-9: LGTM: New BM25 and RRF entries added to CACHE_STR_PATH.The additions of
Bm25Config,MatchTextAny,Rrf, andRrfQueryentries are consistent with the existing pattern and support the new features being introduced.Also applies to: 83-83, 209-210
590-590: LGTM: MatchTextAny correctly integrated.The new
MatchTextAnytype is properly defined and integrated intoFieldCondition.match. The structure and description are consistent with existing match types likeMatchText.Also applies to: 1061-1074
1452-1521: LGTM: Comprehensive BM25 configuration.The
Bm25Configdefinition is well-structured with:
- Standard BM25 parameters (k, b, avg_len) with sensible defaults
- Complete text preprocessing options (tokenizer, language, stopwords, stemmer)
- Token length filtering capabilities
- Proper optional field handling with null types
1537-1545: LGTM: Document.options enhanced with BM25 support.The
optionsfield now explicitly acceptsBm25Configwhile maintaining backward compatibility with arbitrary objects. This improves type safety and makes BM25 configuration a first-class citizen.
1599-1719: LGTM: Comprehensive language and text processing definitions.The new definitions provide robust text processing support:
Languageenum with 31 languages for general text processingSnowballLanguageenum with 19 languages (Snowball stemmer subset)SnowballParamsfor stemmer configurationStopwordsSetallowing both predefined and custom stopwordsTokenizerTypecovering common tokenization strategiesAll definitions are well-structured and consistent with the schema patterns.
1974-1979: LGTM: HnswConfigDiff.copy_vectors addition.The new
copy_vectorsfield is well-documented with clear explanations of:
- The performance trade-off (search speed vs. disk usage)
- Requirements (quantized vectors enabled)
- Limitations (multi-vectors not supported)
1953-1953: LGTM: StrictModeConfig improvements.The updates improve documentation consistency and add new functionality:
- Enhanced
full_scan_thresholddescription with clearer explanation- Consistent punctuation across field descriptions
- Clearer descriptions for
multivector_configandsparse_config- New
max_payload_index_countfield for limiting payload indexesAlso applies to: 2213-2214, 2219-2220, 2285-2286, 2297-2298, 2300-2305
2397-2402: LGTM: WalConfigDiff.wal_retain_closed addition.The new
wal_retain_closedfield for controlling WAL segment retention is properly defined with clear description and consistent structure.
3047-3047: LGTM: RRF (Reciprocal Rank Fusion) support added.The RRF implementation is clean and consistent:
Rrftype with optionalkparameter for RRF algorithmRrfQuerywrapper following the query type pattern- Proper integration into
PrefetchandQueryRequestquery unions- Updated
Fusiondescription accurately reflects RRF supportAlso applies to: 3447-3447, 3570-3590, 3677-3677
4245-4249: LGTM: update_filter feature for conditional updates.The
update_filterfield is consistently added toPointsBatch,PointsList, andUpdateVectors, enabling conditional update/insert behavior. The implementation is clean with:
- Uniform structure across all three types
- Clear description explaining the conditional semantics
- Proper optional field handling with Filter type
Also applies to: 4278-4282, 4318-4322
| "anyOf": [{"type": "number"}, {"type": "null"}], | ||
| "default": None, | ||
| "description": "The midpoint of the decay. Defaults to 0.5. Output will be this value when `|x - target| == scale`.", | ||
| "description": "The midpoint of the decay. Should be between 0 and 1.Defaults to 0.5. Output will be this value when `|x - target| == scale`.", |
There was a problem hiding this comment.
Fix typographical error in description.
Missing space before "Defaults" in the description text.
Apply this diff to fix the typo:
- "description": "The midpoint of the decay. Should be between 0 and 1.Defaults to 0.5. Output will be this value when `|x - target| == scale`.",
+ "description": "The midpoint of the decay. Should be between 0 and 1. Defaults to 0.5. Output will be this value when `|x - target| == scale`.",📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "description": "The midpoint of the decay. Should be between 0 and 1.Defaults to 0.5. Output will be this value when `|x - target| == scale`.", | |
| "description": "The midpoint of the decay. Should be between 0 and 1. Defaults to 0.5. Output will be this value when `|x - target| == scale`.", |
🤖 Prompt for AI Agents
In qdrant_client/embed/_inspection_cache.py around line 444, the description
string for "The midpoint of the decay..." is missing a space before "Defaults";
update the string to insert a space so it reads "...Should be between 0 and 1.
Defaults to 0.5. Output will be this value when `|x - target| == scale`." to
correct the typographical error.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
qdrant_client/async_qdrant_remote.py (1)
1416-1423: Bug: Missing read_consistency conversion in DiscoverBatch (gRPC path).Passes REST-type consistency directly into gRPC. Convert before the call to avoid type errors.
requests = [ RestToGrpc.convert_discover_request(r, collection_name) if isinstance(r, models.DiscoverRequest) else r for r in requests ] + if isinstance(consistency, get_args_subscribed(models.ReadConsistency)): + consistency = RestToGrpc.convert_read_consistency(consistency) grpc_res: grpc.SearchBatchResponse = await self.grpc_points.DiscoverBatch( grpc.DiscoverBatchPoints( collection_name=collection_name, discover_points=requests, read_consistency=consistency, timeout=timeout, ),qdrant_client/qdrant_remote.py (1)
1618-1626: Bug: Missing read_consistency conversion in DiscoverBatch (gRPC path).Convert REST consistency to gRPC before use.
if self._prefer_grpc: requests = [ ( RestToGrpc.convert_discover_request(r, collection_name) if isinstance(r, models.DiscoverRequest) else r ) for r in requests ] + if isinstance(consistency, get_args_subscribed(models.ReadConsistency)): + consistency = RestToGrpc.convert_read_consistency(consistency) grpc_res: grpc.SearchBatchResponse = self.grpc_points.DiscoverBatch( grpc.DiscoverBatchPoints( collection_name=collection_name, discover_points=requests, read_consistency=consistency,
♻️ Duplicate comments (3)
qdrant_client/qdrant_remote.py (3)
1974-1987: REST update_vectors: accept grpc.PointVectors (normalize before JSON).Convert grpc.PointVectors to REST to avoid serialization errors. This mirrors the earlier suggestion.
else: if isinstance(update_filter, grpc.Filter): update_filter = GrpcToRest.convert_filter(model=update_filter) + if isinstance(points, list): + points = [ + (GrpcToRest.convert_point_vectors(p) if isinstance(p, grpc.PointVectors) else p) + for p in points + ] return self.openapi_client.points_api.update_vectors( collection_name=collection_name, wait=wait, update_vectors=models.UpdateVectors( points=points, shard_key=shard_key_selector, update_filter=update_filter,
2670-2685: UpdateCollection (gRPC): convert metadata whenever provided.dict-only check can skip valid payloads. Convert if not None.
- if isinstance(metadata, dict): - metadata = RestToGrpc.convert_payload(metadata) + if metadata is not None: + metadata = RestToGrpc.convert_payload(metadata) return self.grpc_collections.Update( grpc.UpdateCollection( collection_name=collection_name, ... timeout=timeout, - metadata=metadata, + metadata=metadata, ),
2799-2819: CreateCollection (gRPC): same metadata conversion issue.Convert when provided.
- if isinstance(metadata, dict): - metadata = RestToGrpc.convert_payload(metadata) + if metadata is not None: + metadata = RestToGrpc.convert_payload(metadata) create_collection = grpc.CreateCollection( collection_name=collection_name, ... - metadata=metadata, + metadata=metadata, )
🧹 Nitpick comments (4)
qdrant_client/async_qdrant_remote.py (4)
1736-1744: REST update_vectors: convert grpc.PointVectors before JSON.If callers pass grpc.PointVectors, current path will fail serialization. Normalize to REST models.
else: if isinstance(update_filter, grpc.Filter): update_filter = GrpcToRest.convert_filter(model=update_filter) + if isinstance(points, list): + points = [ + (GrpcToRest.convert_point_vectors(p) if isinstance(p, grpc.PointVectors) else p) + for p in points + ] return ( await self.openapi_client.points_api.update_vectors( collection_name=collection_name, wait=wait, update_vectors=models.UpdateVectors( points=points, shard_key=shard_key_selector, update_filter=update_filter ),
2408-2410: Metadata conversion guard too narrow.Use “not None” (or Mapping) instead of dict-only to ensure conversion always happens for provided metadata.
- if isinstance(metadata, dict): - metadata = RestToGrpc.convert_payload(metadata) + if metadata is not None: + metadata = RestToGrpc.convert_payload(metadata)
2522-2524: CreateCollection: same metadata conversion issue.Always convert when provided.
- if isinstance(metadata, dict): - metadata = RestToGrpc.convert_payload(metadata) + if metadata is not None: + metadata = RestToGrpc.convert_payload(metadata)
1529-1529: Lint: unused kwargs.Rename to _kwargs or add “# noqa: ARG002” in the signature to silence Ruff.
- **kwargs: Any, + **_kwargs: Any,
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
qdrant_client/async_qdrant_remote.py(19 hunks)qdrant_client/qdrant_remote.py(18 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
qdrant_client/async_qdrant_remote.py (3)
qdrant_client/grpc/points_pb2.pyi (3)
ReadConsistency(228-245)models(5151-5151)ShardKeySelector(617-634)qdrant_client/http/models/models.py (6)
Filter(884-894)PointStruct(1972-1975)PointsList(1992-1998)Batch(61-64)PointsBatch(1983-1989)UpdateVectors(3325-3328)qdrant_client/conversions/conversion.py (11)
RestToGrpc(2500-4806)convert_filter(166-182)convert_filter(2502-2522)convert_shard_key_selector(2193-2196)convert_shard_key_selector(4618-4625)convert_read_consistency(1736-1745)convert_read_consistency(4090-4100)convert_point_struct(603-608)convert_point_struct(2796-2801)convert_payload(583-584)convert_payload(2804-2805)
qdrant_client/qdrant_remote.py (4)
qdrant_client/grpc/points_pb2.pyi (5)
ReadConsistency(228-245)models(5151-5151)CountPoints(2289-2332)UpsertPoints(638-686)UpdatePointVectors(784-828)qdrant_client/conversions/conversion.py (10)
RestToGrpc(2500-4806)convert_read_consistency(1736-1745)convert_read_consistency(4090-4100)convert_filter(166-182)convert_filter(2502-2522)GrpcToRest(140-2490)convert_update_result(467-471)convert_update_result(2686-2690)convert_payload(583-584)convert_payload(2804-2805)qdrant_client/async_qdrant_remote.py (2)
grpc_points(276-284)update_vectors(1702-1747)qdrant_client/http/models/models.py (5)
Filter(884-894)PointsList(1992-1998)Batch(61-64)PointsBatch(1983-1989)UpdateVectors(3325-3328)
🪛 Ruff (0.14.0)
qdrant_client/async_qdrant_remote.py
1529-1529: Unused method argument: kwargs
(ARG002)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: Redirect rules - poetic-froyo-8baba7
- GitHub Check: Header rules - poetic-froyo-8baba7
- GitHub Check: Pages changed - poetic-froyo-8baba7
- GitHub Check: Python 3.13.x on ubuntu-latest test
- GitHub Check: Python 3.9.x on ubuntu-latest test
- GitHub Check: Python 3.10.x on ubuntu-latest test
- GitHub Check: Python 3.12.x on ubuntu-latest test
- GitHub Check: Python 3.11.x on ubuntu-latest test
joein
force-pushed
the
update-models-1.16
branch
from
977fcf4 to
4a4d40e
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
qdrant_client/grpc/points_pb2.pyi (1)
651-687: Test coverage missing for update_filter on upsert and update_vectors operations.The
.pyistub,pb2.pyruntime (reflection-based), client methods, and HTTP converters are correctly wired forupdate_filter. However, no integration tests exist for the two critical use cases:
upsert()withupdate_filter: matching points are updated, others insertedupdate_vectors()withupdate_filter: only matching points' vectors are updatedExisting tests only cover
set_payload()with filters, not these operations. Add tests demonstrating both behaviors and edge cases (e.g., filter excludes all provided points).
♻️ Duplicate comments (12)
qdrant_client/proto/points.proto (1)
69-76: vectors_count still present; align with “remove vectors_count” objectiveThe legacy vectors_count field persists (even if deprecated). To fully enforce the removal, reserve the field and migrate tests/fixtures to MultiDenseVector.
Apply in proto:
message Vector { repeated float data = 1; // Vector data (flatten for multi vectors), deprecated optional SparseIndices indices = 2; // Sparse indices for sparse vectors, deprecated - optional uint32 vectors_count = 3; // Number of vectors per multi vector, deprecated + reserved 3; // vectors_count removed (legacy multi-vector), do not reuse oneof vector {message VectorOutput { repeated float data = 1; // Vector data (flatten for multi vectors), deprecated optional SparseIndices indices = 2; // Sparse indices for sparse vectors, deprecated - optional uint32 vectors_count = 3; // Number of vectors per multi vector, deprecated + reserved 3; // vectors_count removed (legacy multi-vector), do not reuse oneof vector {Then regenerate stubs and update fixtures to MultiDenseVector (see fixture suggestions below).
#!/bin/bash rg -nP '\bvectors_count\b' -SAlso applies to: 82-91
qdrant_client/proto/collections.proto (1)
425-425: Collection metadata: add end-to-end coverage with mixed typesGreat addition. Ensure REST↔gRPC conversions round-trip ints, floats, bools, nulls, lists, nested maps. Current e2e test uses a simple string only.
See test diff suggestion in tests/test_qdrant_client.py comment.
Also applies to: 438-438, 478-478
tests/test_qdrant_client.py (1)
1170-1185: Widen metadata payload and add update_filter coverageExpand metadata to mixed types to catch conversion edge cases, and add a small test for update_filter propagation.
Apply:
- collection_metadata = {"ownership": "Bart Simpson's property"} + collection_metadata = { + "ownership": "Bart Simpson's property", + "year": 1990, + "rating": 4.2, + "published": True, + "notes": None, + "tags": ["simpson", "animated"], + "nested": {"creator": "Matt Groening", "ids": [1, 2, 3]}, + }Optionally, add:
# verify update_filter is accepted and serialized client.upsert( collection_name=COLLECTION_NAME, points=[PointStruct(id=999, vector=np.random.rand(DIM).tolist())], update_filter=Filter(must=[FieldCondition(key="test", match=MatchValue(value="value"))]), )tests/conversions/fixtures.py (1)
404-408: Migrate away from legacy vectors_count in fixturesReplace flattened data + vectors_count with MultiDenseVector to align with deprecation/removal.
Apply:
-multi_vector = grpc.Vectors( - vector=grpc.Vector(data=[1.0, 2.0, 3.0, 4.0, 5.0, 6.0, 7.0, 8.0], vectors_count=2) -) +multi_vector = grpc.Vectors( + vector=grpc.Vector( + multi_dense=grpc.MultiDenseVector( + vectors=[ + grpc.DenseVector(data=[1.0, 2.0, 3.0, 4.0]), + grpc.DenseVector(data=[5.0, 6.0, 7.0, 8.0]), + ] + ) + ) +)-multi_vector_output = grpc.VectorsOutput( - vector=grpc.VectorOutput(data=[1.0, 2.0, 3.0, 4.0, 5.0, 6.0, 7.0, 8.0], vectors_count=2) -) +multi_vector_output = grpc.VectorsOutput( + vector=grpc.VectorOutput( + multi_dense=grpc.MultiDenseVector( + vectors=[ + grpc.DenseVector(data=[1.0, 2.0, 3.0, 4.0]), + grpc.DenseVector(data=[5.0, 6.0, 7.0, 8.0]), + ] + ) + ) +)- "multi": grpc.Vector(data=[1.0, 2.0, 3.0, 4.0], vectors_count=2), + "multi": grpc.Vector( + multi_dense=grpc.MultiDenseVector( + vectors=[ + grpc.DenseVector(data=[1.0, 2.0, 3.0, 4.0]), + grpc.DenseVector(data=[5.0, 6.0, 7.0, 8.0]), + ] + ) + ),#!/bin/bash rg -nP '\bvectors_count\b' tests/conversions/fixtures.pyAlso applies to: 445-446, 891-895
qdrant_client/conversions/conversion.py (3)
267-267: Critical: incorrect None-check on protobuf map fieldProtobuf map fields are never
None; they're always initialized. The checkmodel.metadata is not Nonewill always evaluate toTrue. Use an emptiness check instead.Apply this diff:
- metadata=cls.convert_payload(model.metadata) if model.metadata is not None else None, + metadata=cls.convert_payload(model.metadata) if len(model.metadata) > 0 else None,Based on past review comments.
794-794: Critical: incorrect None-check on protobuf map fieldSame issue as Line 267. Protobuf maps are never
None. Use an emptiness check.Apply this diff:
- metadata=(cls.convert_payload(model.metadata) if model.metadata is not None else None), + metadata=cls.convert_payload(model.metadata) if len(model.metadata) > 0 else None,Based on past review comments.
3656-3658: Preserve optional k when NoneWhen
model.rrf.kisNone, avoid setting the protobuf optional scalar toNone. Only populate the field when a value is present.Apply this diff:
if isinstance(model, rest.RrfQuery): - return grpc.Query(rrf=grpc.Rrf(k=model.rrf.k)) + rrf = grpc.Rrf() + if model.rrf.k is not None: + rrf.k = model.rrf.k + return grpc.Query(rrf=rrf)Based on past review comments.
qdrant_client/async_qdrant_remote.py (1)
2438-2439: Convert metadata for gRPC whenever provided (guard too narrow).Use a presence check, not isinstance(dict), to ensure all Mapping-like inputs (or alias types) get converted.
Apply:
@@ def update_collection(...): - if isinstance(metadata, dict): - metadata = RestToGrpc.convert_payload(metadata) + if metadata is not None: + metadata = RestToGrpc.convert_payload(metadata) @@ def create_collection(...): - if isinstance(metadata, dict): - metadata = RestToGrpc.convert_payload(metadata) + if metadata is not None: + metadata = RestToGrpc.convert_payload(metadata)This mirrors the sync client fix and avoids silent skips when metadata isn’t a plain dict.
Also applies to: 2461-2462, 2474-2475, 2543-2544, 2575-2576, 2592-2593
qdrant_client/async_qdrant_client.py (1)
1536-1536: update_filter must be Filter (not Payload) in async client signatures.Public typing mismatch vs behavior and other clients. Keep API consistent with AsyncQdrantRemote/HTTP models.
Apply:
@@ async def upsert(...): - update_filter: Optional[types.Payload] = None, + update_filter: Optional[types.Filter] = None, @@ async def update_vectors(...): - update_filter: Optional[types.Payload] = None, + update_filter: Optional[types.Filter] = None,#!/bin/bash # Verify no async/sync client still uses Payload for update_filter rg -nP -C2 $'update_filter:\s*Optional\[types\.(Payload|Filter)\]' qdrant_client | sed -n '1,200p'Also applies to: 1611-1611, 1562-1562, 1640-1642, 1600-1600, 1659-1659
qdrant_client/qdrant_remote.py (1)
2034-2047: Handle gRPC PointVectors in REST pathThe REST path will fail JSON serialization if callers pass gRPC PointVectors objects. Add conversion logic similar to the upsert REST path.
Apply this diff to handle gRPC types:
else: if isinstance(update_filter, grpc.Filter): update_filter = GrpcToRest.convert_filter(model=update_filter) + + if isinstance(points, list): + points = [ + ( + GrpcToRest.convert_point_vectors(point) + if isinstance(point, grpc.PointVectors) + else point + ) + for point in points + ] return self.openapi_client.points_api.update_vectors(qdrant_client/embed/_inspection_cache.py (1)
444-444: Fix typographical error in description.Missing space before "Defaults" in the description text. This issue was previously identified but remains unfixed.
Apply this diff to fix the typo:
- "description": "The midpoint of the decay. Should be between 0 and 1.Defaults to 0.5. Output will be this value when `|x - target| == scale`.", + "description": "The midpoint of the decay. Should be between 0 and 1. Defaults to 0.5. Output will be this value when `|x - target| == scale`.",qdrant_client/grpc/collections_pb2.pyi (1)
11-11: Regression: Import issue reappeared after file regeneration.The past review comment on this line was marked as addressed in commit 556202e, but the absolute import
import json_with_int_pb2has reappeared. Since this is a generated file (@generated by mypy-protobuf), the previous fix was likely lost during regeneration.To make this fix permanent, apply it at the generator level (in the mypy-protobuf configuration or proto definitions) rather than manually editing this file.
Expected:
from . import json_with_int_pb2
🧹 Nitpick comments (10)
qdrant_client/http/api/distributed_api.py (1)
156-174: Timeout plumbed correctly; please document units and edge casesLooks good. Consider clarifying in docstring/API docs that timeout is in seconds and must be non-negative. Add a small test covering force-only, timeout-only, and both together. If this module is generated, put the note in user-facing docs/changelog instead of editing here.
qdrant_client/proto/points.proto (2)
138-139: Specify exact semantics for update_filter (ID exists but doesn’t match?)Ambiguity: when an ID exists but doesn’t satisfy update_filter, is it a no-op, error, or reinsert attempt (which would conflict on ID)? Please clarify in comments and server behavior; align across UpsertPoints, UpdatePointVectors, and PointsUpdateOperation variants.
Also applies to: 166-167, 805-806, 827-828
568-571: Two RRF paths (Fusion.RRF vs Query.rrf). Deprecate one for clarity.Having both enum Fusion.RRF (implicit defaults) and Query.rrf (parameterized) invites confusion. Recommend deprecating Fusion.RRF in comments and favor Query.rrf, keeping enum for backward compatibility.
Also applies to: 672-676, 688-689
qdrant_client/proto/collections.proto (1)
202-209: HNSW copy_vectors: document validationField requires quantization enabled and disallows multi-vectors. Suggest adding “Requires quantization; not supported with MultiDenseVector” to the comment to avoid misconfig.
tests/conversions/fixtures.py (1)
38-39: Include text_any in Match fixtures for conversion coverageYou added match_text_any but it’s not in fixtures["Match"]. Add it so conversion tests cover the new variant.
- "Match": [match_keyword, match_integer, match_bool, match_text], + "Match": [match_keyword, match_integer, match_bool, match_text, match_phrase, match_text_any],Also applies to: 52-53, 129-130, 1638-1643
qdrant_client/http/models/models.py (2)
359-361: Consider addingextra="forbid"for consistency.The
CollectionWarningmodel is correctly defined, but unlike most other models in this file (e.g.,Bm25Config,FieldCondition,Filter), it doesn't useextra="forbid". For consistency and to prevent unexpected fields, consider:-class CollectionWarning(BaseModel): +class CollectionWarning(BaseModel, extra="forbid"): message: str = Field(..., description="Warning message")
1941-1947: Consider addingextra="forbid"for consistency.The
PeerMetadatamodel is correctly defined with the version field, but likeCollectionWarning, it lacksextra="forbid". For consistency with most models in this file and to prevent unexpected fields:-class PeerMetadata(BaseModel): +class PeerMetadata(BaseModel, extra="forbid"): """ Metadata describing extra properties for each peer """ version: str = Field(..., description="Peer Qdrant version")qdrant_client/async_qdrant_remote.py (1)
1580-1599: Added read consistency to count: LGTM.Correctly converts and forwards consistency to both gRPC and REST.
Consider dropping unused
**kwargsor assert no extras for parity with other methods.Also applies to: 1612-1613
qdrant_client/qdrant_remote.py (2)
2691-2780: Consider using Mapping for metadata type checkThe current
isinstance(metadata, dict)check works but could be more robust.Since
Mappingis already imported (line 11), consider using it for broader type coverage:- if isinstance(metadata, dict): + if isinstance(metadata, Mapping): metadata = RestToGrpc.convert_payload(metadata)Alternatively, if metadata is always expected to be a dict when provided (per the type hint), you could simplify to:
- if isinstance(metadata, dict): + if metadata is not None: metadata = RestToGrpc.convert_payload(metadata)
2797-2922: Consider using Mapping for metadata type check (same pattern)Same suggestion as in update_collection for consistency:
- if isinstance(metadata, dict): + if isinstance(metadata, Mapping): metadata = RestToGrpc.convert_payload(metadata)Or simplify if metadata is always a dict when provided:
- if isinstance(metadata, dict): + if metadata is not None: metadata = RestToGrpc.convert_payload(metadata)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (18)
qdrant_client/async_qdrant_client.py(15 hunks)qdrant_client/async_qdrant_remote.py(19 hunks)qdrant_client/conversions/conversion.py(21 hunks)qdrant_client/embed/_inspection_cache.py(24 hunks)qdrant_client/grpc/collections_pb2.py(7 hunks)qdrant_client/grpc/collections_pb2.pyi(27 hunks)qdrant_client/grpc/points_pb2.pyi(18 hunks)qdrant_client/http/api/distributed_api.py(4 hunks)qdrant_client/http/api/points_api.py(10 hunks)qdrant_client/http/api/search_api.py(3 hunks)qdrant_client/http/models/models.py(31 hunks)qdrant_client/local/local_collection.py(0 hunks)qdrant_client/proto/collections.proto(11 hunks)qdrant_client/proto/points.proto(10 hunks)qdrant_client/qdrant_client.py(15 hunks)qdrant_client/qdrant_remote.py(18 hunks)tests/conversions/fixtures.py(11 hunks)tests/test_qdrant_client.py(1 hunks)
💤 Files with no reviewable changes (1)
- qdrant_client/local/local_collection.py
🚧 Files skipped from review as they are similar to previous changes (4)
- qdrant_client/grpc/collections_pb2.py
- qdrant_client/http/api/points_api.py
- qdrant_client/http/api/search_api.py
- qdrant_client/qdrant_client.py
🧰 Additional context used
🧬 Code graph analysis (9)
tests/test_qdrant_client.py (3)
qdrant_client/http/models/models.py (2)
VectorParams(3412-3437)Distance(728-739)tests/utils.py (1)
read_version(9-25)qdrant_client/qdrant_client.py (2)
create_collection(2333-2424)get_collection(2231-2242)
qdrant_client/http/api/distributed_api.py (1)
qdrant_client/http/models/models.py (1)
InlineResponse200(1215-1219)
qdrant_client/conversions/conversion.py (3)
qdrant_client/grpc/collections_pb2.pyi (24)
metadata(1527-1528)metadata(1638-1639)metadata(1855-1856)HasField(375-375)HasField(411-411)HasField(439-439)HasField(471-471)HasField(501-501)HasField(522-522)HasField(544-544)HasField(570-570)HasField(658-658)HasField(702-702)HasField(750-750)HasField(847-847)HasField(892-892)HasField(922-922)HasField(1025-1025)HasField(1067-1067)shard_key(2520-2521)shard_key(2550-2551)shard_key(2604-2604)shard_key(2803-2804)shard_key(2834-2835)qdrant_client/http/models/models.py (5)
MatchTextAny(1577-1582)RrfQuery(2452-2453)Rrf(2444-2449)PointsList(1992-1998)UpdateVectors(3325-3328)qdrant_client/grpc/points_pb2.pyi (39)
rrf(2811-2812)Rrf(2753-2768)update_filter(665-666)update_filter(807-808)update_filter(3405-3406)update_filter(3564-3565)points(657-657)points(703-704)points(798-799)points(1032-1033)points(3400-3400)points(3558-3559)points(3612-3613)points(3633-3634)points(4921-4922)points(5001-5001)shard_key(3842-3843)shard_key(4206-4207)shard_key_selector(662-663)shard_key_selector(709-710)shard_key_selector(756-757)shard_key_selector(804-805)shard_key_selector(877-878)shard_key_selector(941-942)shard_key_selector(994-995)shard_key_selector(1038-1039)shard_key_selector(1478-1479)shard_key_selector(1639-1640)shard_key_selector(1771-1772)shard_key_selector(1821-1822)shard_key_selector(1905-1906)shard_key_selector(2058-2059)shard_key_selector(2217-2218)shard_key_selector(2309-2310)UpdateVectors(3551-3578)Match(4718-4771)Query(2772-2829)PointsUpdateOperation(3390-3694)PointStructList(3393-3419)
qdrant_client/grpc/points_pb2.pyi (1)
qdrant_client/http/models/models.py (2)
Fusion(920-929)Rrf(2444-2449)
tests/conversions/fixtures.py (4)
qdrant_client/grpc/points_pb2.pyi (16)
Match(4718-4771)FieldCondition(4656-4714)match(4671-4672)Condition(4537-4574)field(4548-4548)params(1461-1462)params(1617-1618)params(1877-1878)params(2025-2026)params(2199-2200)params(2856-2857)params(2928-2929)params(3069-3070)Query(2772-2829)rrf(2811-2812)Rrf(2753-2768)qdrant_client/http/models/models.py (4)
FieldCondition(860-881)StrictModeConfig(3091-3136)CollectionConfig(222-242)Rrf(2444-2449)qdrant_client/grpc/collections_pb2.pyi (21)
StrictModeConfig(1227-1351)metadata(1527-1528)metadata(1638-1639)metadata(1855-1856)CollectionConfig(1809-1873)params(492-492)params(513-513)params(1620-1621)params(1837-1838)params(2219-2220)hnsw_config(352-353)hnsw_config(397-398)hnsw_config(1492-1493)hnsw_config(1623-1624)hnsw_config(1840-1841)optimizer_config(1843-1844)wal_config(1495-1496)wal_config(1846-1847)strict_mode_config(1524-1525)strict_mode_config(1635-1636)strict_mode_config(1852-1853)qdrant_client/grpc/json_with_int_pb2.pyi (1)
Value(84-131)
qdrant_client/http/models/models.py (2)
qdrant_client/grpc/points_pb2.pyi (20)
b(3343-3344)options(318-319)options(360-361)options(403-404)update_filter(665-666)update_filter(807-808)update_filter(3405-3406)update_filter(3564-3565)points(657-657)points(703-704)points(798-799)points(1032-1033)points(3400-3400)points(3558-3559)points(3612-3613)points(3633-3634)points(4921-4922)points(5001-5001)Rrf(2753-2768)rrf(2811-2812)qdrant_client/grpc/collections_pb2.pyi (9)
stopwords(2021-2022)metadata(1527-1528)metadata(1638-1639)metadata(1855-1856)warnings(2285-2286)CollectionWarning(773-783)languages(1984-1985)custom(1987-1988)WalConfigDiff(903-929)
qdrant_client/async_qdrant_remote.py (5)
qdrant_client/http/models/models.py (6)
Filter(884-894)PointStruct(1972-1975)PointsList(1992-1998)Batch(61-64)PointsBatch(1983-1989)UpdateVectors(3325-3328)qdrant_client/conversions/conversion.py (10)
RestToGrpc(2500-4806)convert_filter(166-182)convert_filter(2502-2522)convert_read_consistency(1736-1745)convert_read_consistency(4090-4100)GrpcToRest(140-2490)convert_point_struct(603-608)convert_point_struct(2796-2801)convert_payload(583-584)convert_payload(2804-2805)qdrant_client/conversions/common_types.py (1)
get_args_subscribed(33-45)qdrant_client/qdrant_remote.py (1)
update_vectors(1999-2047)qdrant_client/async_qdrant_client.py (1)
update_vectors(1604-1660)
qdrant_client/qdrant_remote.py (5)
qdrant_client/grpc/points_pb2.pyi (5)
ReadConsistency(228-245)models(5151-5151)CountPoints(2289-2332)UpsertPoints(638-686)UpdatePointVectors(784-828)qdrant_client/conversions/conversion.py (10)
RestToGrpc(2500-4806)convert_read_consistency(1736-1745)convert_read_consistency(4090-4100)convert_filter(166-182)convert_filter(2502-2522)GrpcToRest(140-2490)convert_update_result(467-471)convert_update_result(2686-2690)convert_payload(583-584)convert_payload(2804-2805)qdrant_client/async_qdrant_remote.py (2)
grpc_points(325-334)update_vectors(1754-1799)qdrant_client/qdrant_client.py (2)
grpc_points(197-206)update_vectors(1656-1714)qdrant_client/http/models/models.py (5)
Filter(884-894)PointsList(1992-1998)Batch(61-64)PointsBatch(1983-1989)UpdateVectors(3325-3328)
qdrant_client/grpc/collections_pb2.pyi (2)
qdrant_client/http/models/models.py (1)
CollectionWarning(359-360)qdrant_client/grpc/json_with_int_pb2.pyi (2)
value(61-61)Value(84-131)
🪛 Buf (1.59.0)
qdrant_client/proto/collections.proto
6-6: import "json_with_int.proto": file does not exist
(COMPILE)
🪛 Ruff (0.14.2)
qdrant_client/async_qdrant_remote.py
1581-1581: Unused method argument: kwargs
(ARG002)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: Redirect rules - poetic-froyo-8baba7
- GitHub Check: Header rules - poetic-froyo-8baba7
- GitHub Check: Pages changed - poetic-froyo-8baba7
- GitHub Check: Python 3.10.x on ubuntu-latest test
- GitHub Check: Python 3.11.x on ubuntu-latest test
- GitHub Check: Python 3.13.x on ubuntu-latest test
- GitHub Check: Python 3.12.x on ubuntu-latest test
- GitHub Check: Python 3.9.x on ubuntu-latest test
🔇 Additional comments (51)
qdrant_client/http/api/distributed_api.py (2)
263-276: Async API propagation LGTM
Parameter is forwarded correctly to the builder.
340-353: Sync API propagation LGTM
Parameter is forwarded correctly to the builder.qdrant_client/grpc/points_pb2.pyi (3)
4411-4428: FacetResponse usage field aligns with REST modelsAddition of usage mirrors most other response messages; REST InlineResponse types similarly expose usage.
4730-4772: Based on the verification of the provided search results, the review comment can be resolved:All text_any end-to-end support is correctly implemented
The evidence confirms:
HTTP/OpenAPI models: HTTP model at
qdrant_client/http/models/models.pyincludestext_anyfield with description "Full-text match of at least one token of the string"Converters mapping (bidirectional):
- REST to gRPC conversion at
conversion.pylines 681-682 handlestext_anyviarest.MatchTextAny- gRPC to REST conversion at
conversion.pylines 3056-3057 convertsrest.MatchTextAnymodelsOneof enforcement: The
WhichOneofmethod signature shows text_any is correctly part of the match_value oneof, with the return type including all variants including "text_any", which protobuf enforces at the protocol level—only one variant can be set at a time, ensuring field exclusivity regardless of whether both text and text_any are supplied.
2753-2830: Review comment is incorrect — no changes needed.The proto definition shows
fusionandrrfare both part of the sameoneof variantgroup, making them mutually exclusive. They cannot both be set simultaneously, so the review's concern about documenting "how both should be used together" or establishing "precedence if both set" is based on a false premise.The semantic distinction is clear and correct:
Fusion.RRF(enum): RRF with default parametersQuery.rrf(Rrf message): RRF with customkparameterThis design is sound—when users need RRF with a custom
k, they useQuery.rrf; otherwise, they useFusion.RRF. HTTP models, gRPC types, and conversion logic are all properly aligned. No documentation or alignment issues exist.qdrant_client/proto/points.proto (3)
1036-1037: FacetResponse.usage addition: LGTMUseful for telemetry/billing; no concerns.
1124-1125: Match.text_any addition: LGTMGood expansion of text matching. Ensure HTTP models and conversions handle it consistently (tokenization vs Match.text/phrase).
8-9: ****The file
json_with_int.protois present in the repository atqdrant_client/proto/json_with_int.proto, and the import path inpoints.protois correct. Buf and CI will resolve this import successfully—there is no blocking issue.Likely an incorrect or invalid review comment.
tests/conversions/fixtures.py (1)
371-388: New fixtures (metadata, RRF, copy_vectors): LGTMGood coverage of metadata map, parameterized RRF (k and default), and HNSW copy_vectors flag.
Also applies to: 1512-1515, 1741-1744, 297-297
qdrant_client/proto/collections.proto (1)
6-7: The file exists and the import is valid—this review comment is incorrect.The json_with_int.proto file is present at
qdrant_client/proto/json_with_int.protowith proper content. The import statement in collections.proto resolves correctly. No blocking issue exists.Likely an incorrect or invalid review comment.
qdrant_client/http/models/models.py (19)
93-133: LGTM: Well-structured BM25 configuration model.The
Bm25Configclass is well-defined with appropriate optional fields, sensible defaults, and clear documentation. The field types align with expected text search parameters.
205-205: LGTM: Appropriate peer metadata field addition.The
peer_metadatafield correctly usesOptional[Dict[str, "PeerMetadata"]]to represent metadata for cluster peers, consistent with the pattern used elsewhere in the file.
239-242: LGTM: Collection metadata field properly added.The
metadatafield is correctly typed asOptional["Payload"](which isDict[str, Any]), allowing arbitrary JSON metadata for collections. This aligns with the gRPC definition and provides good flexibility for storing application-specific data.
277-279: LGTM: Warnings field appropriately integrated.The
warningsfield correctly represents collection warnings asOptional[List["CollectionWarning"]], matching the gRPC definition and allowing collections to report issues when present.
757-767: LGTM: Document options enhanced with structured BM25 config.The
optionsfield type change fromOptional[Dict[str, Any]]toOptional["DocumentOptions"](which isUnion[Dict[str, Any], Bm25Config]) maintains backward compatibility while allowing structured BM25 configuration. The union includes the original dict type, ensuring existing code continues to work.
855-857: LGTM: Feature flag properly added.The
appendable_quantizationfeature flag is correctly defined with an appropriate default value ofFalseand clear documentation.
920-930: LGTM: Fusion documentation accurately updated.The docstring correctly documents the available fusion algorithms including the newly added RRF (Reciprocal Rank Fusion) option.
1078-1081: LGTM: Enhanced HNSW threshold documentation.The updated
full_scan_thresholddescription provides clearer guidance on when the query planner switches between full-scan and HNSW index traversal, improving developer understanding.
1094-1097: LGTM: Copy vectors optimization properly documented.The
copy_vectorsfield is well-defined with clear documentation explaining the performance tradeoffs, prerequisites (quantization), and limitations (no multi-vector support). The optional boolean type is appropriate.
1577-1583: LGTM: Text-any match variant properly implemented.The
MatchTextAnyclass follows the established pattern for match conditions and clearly documents its behavior (matching at least one token). Theextra="forbid"setting ensures strict validation.
1986-1989: LGTM: Update filter enables conditional upsert.The
update_filterfield inPointsBatchcorrectly implements conditional upsert functionality, allowing points to be updated only if they match the filter, with non-matching points being inserted. This aligns with the gRPC definition.
2444-2454: LGTM: RRF query models properly implemented.The
RrfandRrfQueryclasses correctly implement Reciprocal Rank Fusion functionality, matching the gRPC definition. The structure follows the established pattern for query types, withRrfQuerywrapping theRrfparameters.
3082-3088: LGTM: Stopwords configuration properly defined.The
StopwordsSetfields correctly allow combining predefined language stopwords with custom stopwords. The type annotations and descriptions clearly explain the merging behavior.
3103-3136: LGTM: Strict mode constraints properly expanded.The additional strict mode configuration fields (
search_max_hnsw_ef,search_allow_exact,multivector_config,sparse_config,max_payload_index_count) are correctly typed and documented, providing fine-grained control over collection behavior and resource limits.
3298-3301: LGTM: Collection metadata update properly documented.The
metadatafield inUpdateCollectioncorrectly allows merging metadata with existing values. The description helpfully explains both update and removal semantics.
3518-3518: LGTM: WAL retention configuration properly added.The
wal_retain_closedfield is correctly added to bothWalConfig(with default=1) andWalConfigDiff(with default=None), allowing control over closed WAL segment retention. The default value of 1 is sensible for maintaining recent history.
3602-3605: LGTM: Document options type alias properly defined.The
DocumentOptionstype alias correctly provides flexibility between unstructured dictionary options and structuredBm25Config, maintaining backward compatibility while enabling better type safety.
3623-3630: LGTM: Match union properly updated.The
Matchunion correctly includesMatchTextAny, completing the integration of the text-any matching functionality. The ordering groups related match types logically.
3691-3701: LGTM: Query union properly updated.The
Queryunion correctly includesRrfQuery, completing the integration of Reciprocal Rank Fusion query functionality. The type is appropriately positioned in the union.qdrant_client/async_qdrant_client.py (1)
2196-2196: Metadata parameter propagation: LGTM.Signatures/docs/pass-through for metadata are consistent. No changes needed here.
If you also updated the async/sync remotes for gRPC conversion, ensure conversion occurs whenever metadata is provided (see related comment in async_qdrant_remote.py).
Also applies to: 2213-2213, 2234-2235, 2276-2276, 2345-2346, 2366-2367
qdrant_client/async_qdrant_remote.py (3)
1761-1762: update_filter in update_vectors: wiring looks correct across gRPC/REST.Consistent with models.UpdateVectors.update_filter and server expectations.
Also applies to: 1771-1772, 1780-1781, 1788-1790, 1795-1796
2650-2651: recreate_collection: metadata pass-through is consistent.Delegation to create_collection preserves metadata; good.
Also applies to: 2670-2671
1680-1681: update_filter wiring is correct across gRPC/REST boundaries.Verification confirms:
- gRPC path: update_filter converted and passed directly to
grpc.UpsertPoints(lines 1709–1720)- REST path: update_filter injected into
PointsList/PointsBatchmodels before callingupsert_points(lines 1727–1742)- Body models carry the field; type conversions work bidirectionally. No issues found.
qdrant_client/qdrant_remote.py (4)
1806-1846: LGTM: consistency parameter properly integratedThe consistency parameter is correctly added to the count method with proper REST↔gRPC conversions on both paths.
1907-1997: LGTM: update_filter properly integrated in upsertThe update_filter parameter is correctly added with proper conversions and propagation through both gRPC and REST paths to PointsList and PointsBatch.
2006-2033: LGTM: update_filter properly integrated in gRPC pathThe update_filter parameter is correctly added with proper conversions and propagation through the gRPC path.
2924-2963: LGTM: metadata parameter correctly forwardedThe recreate_collection method properly forwards the metadata parameter to create_collection.
qdrant_client/embed/_inspection_cache.py (6)
9-9: LGTM! New cache entries align with added types.The additions to
CACHE_STR_PATHforBm25Config,MatchTextAny,Rrf, andRrfQueryare consistent with the new type definitions added later in the file.Also applies to: 83-83, 209-210
590-590: LGTM! MatchTextAny properly integrated.The new
MatchTextAnytype is correctly added to theFieldCondition.matchunion and follows the established pattern for match types. The description clearly explains its behavior as matching at least one token.Also applies to: 1061-1074
1452-1521: LGTM! BM25 configuration is well-structured.The BM25 support additions are comprehensive and well-designed:
Bm25Configincludes all necessary parameters with sensible defaults- Supporting types (
Language,Snowball*,StopwordsSet,TokenizerType) are properly defined- Integration into
Document.optionsmaintains backward compatibility viaanyOf- Field descriptions clearly explain parameters and their impact
Also applies to: 1537-1541, 1599-1719
3047-3050: LGTM! Reciprocal Rank Fusion properly implemented.The RRF (Reciprocal Rank Fusion) additions are well-integrated:
Fusionenum description now documents both RRF and DBSF algorithmsRrfandRrfQuerytypes follow established patternsRrfQueryis correctly added to query unions inPrefetchandQueryRequest- The
kparameter description is clearAlso applies to: 3447-3447, 3570-3590, 3677-3677
1974-1979: LGTM! Configuration updates are clear and well-documented.The configuration additions and updates are properly implemented:
HnswConfigDiff.copy_vectorsclearly explains the search speed vs. disk usage trade-offStrictModeConfigdescriptions forsearch_max_hnsw_efandsearch_allow_exactare more concisemax_payload_index_countis properly added with clear descriptionWalConfigDiff.wal_retain_closedhas clear description of its purposeAlso applies to: 2213-2220, 2285-2298, 2300-2305, 2397-2402
4245-4249: LGTM! update_filter consistently added across point operations.The
update_filterfield is correctly and consistently added toPointsBatch,PointsList, andUpdateVectors:
- Uniform type definition (
Filter | null)- Clear descriptions explaining conditional update/insert behavior
- Proper default value (
None)- Enables filtering which points should be updated vs. inserted
Also applies to: 4278-4282, 4318-4322
qdrant_client/grpc/collections_pb2.pyi (8)
773-785: LGTM! CollectionWarning class added correctly.The new
CollectionWarningmessage type is well-formed and consistent with the protobuf patterns used throughout this file. It aligns with the corresponding HTTP model definition.
796-850: LGTM! HnswConfigDiff enhancements are well-documented.The addition of the
copy_vectorsfield and the clarifiedfull_scan_thresholddocumentation improve the API. The implementation correctly follows the optional field pattern used throughout the file.
908-929: LGTM! WAL retention configuration added correctly.The
wal_retain_closedfield addition is consistent with the existing WAL configuration pattern and properly typed as an optional integer.
1248-1351: LGTM! StrictModeConfig enhancements improve clarity.The addition of
max_payload_index_countand the improved docstrings make the strict mode configuration more understandable. The deprecation note formax_segment_sizeis helpful for users.
1456-1550: LGTM! Collection metadata support added correctly.The nested
MetadataEntryclass andmetadatafield onCreateCollectionfollow the established protobuf map pattern. TheMessageMap[str, json_with_int_pb2.Value]type correctly represents arbitrary JSON metadata.
1585-1655: LGTM! UpdateCollection metadata follows the same pattern.The metadata implementation is consistent with
CreateCollection. The docstring correctly notes that metadata will be merged, which is important for update semantics.
1812-1869: LGTM! CollectionConfig metadata completes the metadata feature.The metadata field is consistently implemented across
CreateCollection,UpdateCollection, andCollectionConfig, providing a complete metadata API.
2266-2300: LGTM! Collection warnings properly integrated.The
warningsfield onCollectionInfocorrectly usesRepeatedCompositeFieldContainer[CollectionWarning]for a list of warnings. The method signatures properly handle repeated fields (excluded fromHasField, included inClearField).
| class UpdateVectors(BaseModel, extra="forbid"): | ||
| points: List["PointVectors"] = Field(..., description="Points with named vectors") | ||
| shard_key: Optional["ShardKeySelector"] = Field(default=None, description="") | ||
| update_filter: Optional["Filter"] = Field(default=None, description="") |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Add description for update_filter field.
The update_filter field is correctly typed but lacks a description. For consistency with PointsBatch.update_filter and PointsList.update_filter, add a description:
- update_filter: Optional["Filter"] = Field(default=None, description="")
+ update_filter: Optional["Filter"] = Field(
+ default=None,
+ description="If specified, only points that match this filter will be updated"
+ )📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| update_filter: Optional["Filter"] = Field(default=None, description="") | |
| update_filter: Optional["Filter"] = Field( | |
| default=None, | |
| description="If specified, only points that match this filter will be updated" | |
| ) |
🤖 Prompt for AI Agents
In qdrant_client/http/models/models.py around line 3328, the Field for
update_filter currently has an empty description; add a descriptive string
matching the style used by PointsBatch.update_filter and
PointsList.update_filter that explains this optional Filter narrows which points
are updated (e.g., "Optional filter to select which points the update applies
to"). Update the Field(...) call to include that description text.
There was a problem hiding this comment.
models.py is autogenerated, I can't fix it
There was a problem hiding this comment.
@joein, understood! Since models.py is autogenerated, this change would need to be made in the source (likely the OpenAPI spec or proto definitions in the qdrant core) rather than here. No action needed on your end for this PR. Thanks for clarifying!
* new: direct uuid support * tests: add uuid tests * fix: update inspection cache
* new: add collection metadata and tests to local mode * fix: regen async client
* new: implement parametrized rrf in local mode * refactoring: use a variable for a magic value * fix: adjust conversion according to AI
* new: add missing update_filter, implement it in local mode * fix: fix type hint, fix update operation, fix rest uploader, add tests * fix: fix update filter is None case * fix: mypy was not a good boy
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (7)
qdrant_client/async_qdrant_client.py (1)
1536-1563: Fix update_filter type annotation in upsert methodsThe
update_filterparameter onupsertmethods is incorrectly typed asOptional[types.Payload]but should beOptional[types.Filter]to match theupdate_vectorsimplementation and all remote client implementations.
qdrant_client/qdrant_client.pyline 1582qdrant_client/async_qdrant_client.pyline 1536qdrant_client/local/async_qdrant_local.py (1)
894-911: Guard againstNonewhen merging collection metadataAsync local collections have the same problem:
_collection.config.metadatadefaults toNone, so calling.update(...)directly crashes on the first metadata write. Mirror the sync fix by instantiating the dict when missing before merging new values.qdrant_client/qdrant_remote.py (2)
1908-1998: Bug: update_filter lost when points is already PointsList/PointsBatch (REST path).If callers pass models.PointsList/PointsBatch and also provide update_filter to this method, the filter isn’t injected into the body. Inject it when missing.
else: + # If caller already built a Points* container, inject update_filter when provided + if isinstance(points, models.PointsList) and update_filter is not None and points.update_filter is None: + points.update_filter = update_filter + if isinstance(points, models.PointsBatch) and update_filter is not None and points.update_filter is None: + points.update_filter = update_filter if isinstance(update_filter, grpc.Filter): update_filter = GrpcToRest.convert_filter(model=update_filter) if isinstance(points, list): points = [ ( GrpcToRest.convert_point_struct(point) if isinstance(point, grpc.PointStruct) else point ) for point in points ] points = models.PointsList( points=points, shard_key=shard_key_selector, update_filter=update_filter ) if isinstance(points, models.Batch): points = models.PointsBatch( batch=points, shard_key=shard_key_selector, update_filter=update_filter )
2982-3016: Uploader kwargs now include update_filter — update_filter is passed but never used.The issue is confirmed. Both
upload_batch()andupload_batch_grpc()functions do NOT acceptupdate_filter. Unlikeshard_key_selectorwhich is explicitly named in constructors and forwarded to these functions,update_filterwould be bundled into**kwargsand silently ignored.To fix:
- Add
update_filter: Optional[UpdateFilter] = Noneas explicit parameter inRestBatchUploader.__init__()andGrpcBatchUploader.__init__()- Extract and store it (e.g.,
self._update_filter = update_filter)- Add
update_filterparameter toupload_batch()andupload_batch_grpc()function signatures- Forward it to the actual upsert API calls:
openapi_client.points_api.upsert_points(...)andpoints_client.Upsert(...)qdrant_client/async_qdrant_remote.py (3)
1674-1752: Bug: update_filter lost when points is already PointsList/PointsBatch (REST path).Same as sync: inject update_filter into existing models.PointsList/PointsBatch before calling REST.
else: + # If caller already built a Points* container, inject update_filter when provided + if isinstance(points, models.PointsList) and update_filter is not None and points.update_filter is None: + points.update_filter = update_filter + if isinstance(points, models.PointsBatch) and update_filter is not None and points.update_filter is None: + points.update_filter = update_filter if isinstance(update_filter, grpc.Filter): update_filter = GrpcToRest.convert_filter(model=update_filter) if isinstance(points, list): points = [ GrpcToRest.convert_point_struct(point) if isinstance(point, grpc.PointStruct) else point for point in points ] points = models.PointsList( points=points, shard_key=shard_key_selector, update_filter=update_filter ) if isinstance(points, models.Batch): points = models.PointsBatch( batch=points, shard_key=shard_key_selector, update_filter=update_filter )
1755-1800: Async REST update_vectors should normalize grpc.PointVectors.Convert grpc.PointVectors to REST equivalents before building models.UpdateVectors.
else: + # Normalize possible gRPC PointVectors to REST models for JSON payload + if isinstance(points, list): + points = [ + (GrpcToRest.convert_point_vectors(p) if isinstance(p, grpc.PointVectors) else p) + for p in points + ] if isinstance(update_filter, grpc.Filter): update_filter = GrpcToRest.convert_filter(model=update_filter) return ( await self.openapi_client.points_api.update_vectors( collection_name=collection_name, wait=wait, update_vectors=models.UpdateVectors( - points=points, shard_key=shard_key_selector, update_filter=update_filter + points=points, shard_key=shard_key_selector, update_filter=update_filter ), ordering=ordering, ) ).result
2680-2724: Update_filter is passed to uploaders but not implemented — requires refactoring.Verification confirms the issue:
update_filteris passed toRestBatchUploaderandGrpcBatchUploaderconstructors, but:
- Neither uploader stores it (no
self.update_filterassignments)- The batch upload functions (
upload_batch,upload_batch_grpc) don't acceptupdate_filterparametersupdate_filteris never propagated to actual upsert operationsThe feature is incomplete. Either:
- Add
update_filterparameter toupload_batch()andupload_batch_grpc()signatures, store it in uploaders, and pass to API calls- Remove
update_filterfromupdater_kwargsif not yet supported
♻️ Duplicate comments (4)
qdrant_client/conversions/conversion.py (2)
268-268: Critical: Incorrect presence check on protobuf map field.Protobuf map fields are never
Nonein Python—they're always present (even when empty). The checkmodel.metadata is not Nonewill always evaluate toTrue, causingconvert_payloadto be called even for empty maps. This means an empty protobuf map{}will be converted to an empty dict instead ofNone, which may not align with the REST model's semantics (metadata: Optional[Dict[str, Any]]defaulting toNone).Apply this diff to check for non-empty maps:
- metadata=cls.convert_payload(model.metadata) if model.metadata is not None else None, + metadata=cls.convert_payload(model.metadata) if model.metadata else None,Alternatively, be explicit with length:
- metadata=cls.convert_payload(model.metadata) if model.metadata is not None else None, + metadata=cls.convert_payload(model.metadata) if len(model.metadata) != 0 else None,
795-795: Critical: Same protobuf map presence check issue.Same issue as line 268:
model.metadata is not Nonealways evaluates toTruefor protobuf maps. The parentheses are correct (no tuple), but the condition needs fixing.Apply this diff:
- metadata=(cls.convert_payload(model.metadata) if model.metadata is not None else None), + metadata=cls.convert_payload(model.metadata) if model.metadata else None,qdrant_client/embed/_inspection_cache.py (1)
444-446: Restore the missing space in the midpoint description.The string still renders as “1.Defaults”, which reads poorly in the generated docs. Please insert the space so it becomes “1. Defaults”.
- "description": "The midpoint of the decay. Should be between 0 and 1.Defaults to 0.5. Output will be this value when `|x - target| == scale`.", + "description": "The midpoint of the decay. Should be between 0 and 1. Defaults to 0.5. Output will be this value when `|x - target| == scale`.",qdrant_client/qdrant_remote.py (1)
1999-2047: REST update_vectors should normalize grpc.PointVectors; otherwise JSON encoding fails.Convert grpc.PointVectors to REST model before constructing models.UpdateVectors.
else: + # Normalize possible gRPC PointVectors to REST models for JSON payload + if isinstance(points, list): + points = [ + (GrpcToRest.convert_point_vectors(p) if isinstance(p, grpc.PointVectors) else p) + for p in points + ] if isinstance(update_filter, grpc.Filter): update_filter = GrpcToRest.convert_filter(model=update_filter) return self.openapi_client.points_api.update_vectors( collection_name=collection_name, wait=wait, update_vectors=models.UpdateVectors( points=points, shard_key=shard_key_selector, update_filter=update_filter, ), ordering=ordering, ).result
🧹 Nitpick comments (8)
tests/test_local_persistence.py (1)
155-203: Rename typo: “persisence” → “persistence”Purely cosmetic but improves discoverability. Consider renaming test and collection name for consistency.
-def test_update_persisence(): - collection_name = "update_persisence" +def test_update_persistence(): + collection_name = "update_persistence"tests/congruence_tests/test_updates.py (1)
508-709: Avoid flakiness: add wait=True on remote mutating callsSeveral remote writes (upsert/update_vectors/upload_points/upload_collection/batch_update_points) run without wait=True. Under load this can race comparisons. Recommend setting wait=True for remote_client in this test.
Example:
-remote_client.upsert(COLLECTION_NAME, points=original_points) +remote_client.upsert(COLLECTION_NAME, points=original_points, wait=True) -remote_client.update_vectors(COLLECTION_NAME, points=new_point_vectors, update_filter=update_filter) +remote_client.update_vectors(COLLECTION_NAME, points=new_point_vectors, update_filter=update_filter, wait=True)Apply similarly to other remote writes in this test.
qdrant_client/hybrid/fusion.py (1)
10-18: Harden input and avoid side‑effects (optional)
- Validate ranking_constant_k > 0; fall back to default if invalid.
- Consider copying ScoredPoint before overwriting score to avoid mutating upstream responses.
def reciprocal_rank_fusion( responses: list[list[models.ScoredPoint]], limit: int = 10, ranking_constant_k: Optional[int] = None, ) -> list[models.ScoredPoint]: - def compute_score(pos: int) -> float: - ranking_constant = ( - ranking_constant_k if ranking_constant_k is not None else DEFAULT_RANKING_CONSTANT_K - ) # mitigates the impact of high rankings by outlier systems + def compute_score(pos: int) -> float: + k = ranking_constant_k if (ranking_constant_k and ranking_constant_k > 0) else DEFAULT_RANKING_CONSTANT_K return 1 / (ranking_constant + pos) @@ - for point_id, score in sorted_scores[:limit]: - point = point_pile[point_id] - point.score = score - sorted_points.append(point) + for point_id, score in sorted_scores[:limit]: + original = point_pile[point_id] + point = models.ScoredPoint(**original.model_dump()) # shallow clone to avoid side-effects + point.score = score + sorted_points.append(point)Also applies to: 30-36
qdrant_client/qdrant_remote.py (2)
2702-2746: Broaden metadata type check for gRPC conversion.Use Mapping instead of dict to accept any dict-like payloads.
- if isinstance(metadata, dict): + if isinstance(metadata, Mapping): metadata = RestToGrpc.convert_payload(metadata)
2816-2881: Same metadata type-check improvement for create_collection (gRPC path).- if isinstance(metadata, dict): + if isinstance(metadata, Mapping): metadata = RestToGrpc.convert_payload(metadata)qdrant_client/async_qdrant_remote.py (2)
2428-2478: Broaden metadata check to Mapping for async update_collection (gRPC).- if isinstance(metadata, dict): + if isinstance(metadata, Mapping): metadata = RestToGrpc.convert_payload(metadata)
2524-2596: Same metadata check improvement for async create_collection (gRPC).- if isinstance(metadata, dict): + if isinstance(metadata, Mapping): metadata = RestToGrpc.convert_payload(metadata)qdrant_client/qdrant_client.py (1)
2492-2492: Minor docstring inconsistency for metadata parameterFor consistency with
create_collectionandupdate_collection, consider using "JSON-like metadata" instead of "JSON metadata" in the docstring.Apply this diff:
- metadata: Arbitrary JSON metadata for the collection + metadata: Arbitrary JSON-like metadata for the collection
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (20)
qdrant_client/async_qdrant_client.py(21 hunks)qdrant_client/async_qdrant_remote.py(26 hunks)qdrant_client/conversions/common_types.py(2 hunks)qdrant_client/conversions/conversion.py(23 hunks)qdrant_client/embed/_inspection_cache.py(45 hunks)qdrant_client/http/models/models.py(31 hunks)qdrant_client/hybrid/fusion.py(1 hunks)qdrant_client/local/async_qdrant_local.py(11 hunks)qdrant_client/local/local_collection.py(14 hunks)qdrant_client/local/payload_filters.py(2 hunks)qdrant_client/local/qdrant_local.py(13 hunks)qdrant_client/qdrant_client.py(21 hunks)qdrant_client/qdrant_remote.py(25 hunks)qdrant_client/uploader/grpc_uploader.py(5 hunks)qdrant_client/uploader/rest_uploader.py(5 hunks)tests/congruence_tests/test_query.py(2 hunks)tests/congruence_tests/test_updates.py(2 hunks)tests/congruence_tests/test_uuids.py(1 hunks)tests/test_local_persistence.py(1 hunks)tests/test_qdrant_client.py(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (15)
tests/congruence_tests/test_query.py (3)
qdrant_client/client_base.py (1)
query_points(93-120)qdrant_client/http/models/models.py (2)
QueryResponse(2147-2148)Prefetch(2010-2034)qdrant_client/local/local_collection.py (1)
query_points(705-760)
qdrant_client/hybrid/fusion.py (1)
qdrant_client/http/models/models.py (1)
ScoredPoint(2496-2507)
tests/test_qdrant_client.py (5)
tests/utils.py (1)
read_version(9-25)qdrant_client/local/async_qdrant_local.py (3)
create_collection(925-972)get_collection(873-875)update_collection(884-901)qdrant_client/local/qdrant_local.py (3)
create_collection(997-1050)get_collection(942-944)update_collection(953-972)qdrant_client/qdrant_client.py (3)
create_collection(2333-2424)get_collection(2231-2242)update_collection(2257-2311)qdrant_client/qdrant_remote.py (3)
create_collection(2797-2922)get_collection(2664-2676)update_collection(2691-2780)
qdrant_client/conversions/common_types.py (1)
qdrant_client/grpc/points_pb2.pyi (1)
PointId(249-271)
tests/congruence_tests/test_uuids.py (5)
qdrant_client/qdrant_client.py (23)
QdrantClient(27-3191)collection_exists(2244-2255)delete_collection(2313-2331)create_collection(2333-2424)create_payload_index(2726-2767)upsert(1575-1654)query_points(447-620)query_batch_points(403-445)query_points_groups(622-788)search_matrix_pairs(1058-1104)search_matrix_offsets(1106-1152)scroll(1413-1481)facet(1524-1573)retrieve(1766-1822)update_vectors(1656-1714)delete_vectors(1716-1764)delete(1824-1869)set_payload(1871-1965)overwrite_payload(1967-2032)delete_payload(2034-2081)clear_payload(2083-2125)upload_collection(2648-2724)batch_update_points(2127-2167)tests/congruence_tests/test_common.py (5)
init_local(399-404)init_remote(407-409)generate_fixtures(117-132)compare_client_results(296-380)compare_collections(175-197)tests/fixtures/payload.py (1)
one_random_payload_please(190-245)qdrant_client/http/models/models.py (19)
VectorParams(3412-3437)Distance(728-739)PointStruct(1972-1975)PayloadSchemaType(1883-1898)Filter(884-894)HasIdCondition(1049-1054)QueryRequest(2108-2140)PointVectors(1978-1980)PointIdsList(1955-1957)FilterSelector(897-899)PointsBatch(1983-1989)Batch(61-64)PointsList(1992-1998)SetPayload(2716-2735)DeletePayload(628-642)ClearPayloadOperation(155-156)UpdateVectors(3325-3328)DeleteVectors(649-657)DeleteOperation(624-625)qdrant_client/local/local_collection.py (15)
upsert(2473-2509)query_points(705-760)search_matrix_pairs(1586-1607)search_matrix_offsets(1552-1584)scroll(1886-1920)facet(1162-1211)retrieve(1213-1237)update_vectors(2543-2553)delete_vectors(2555-2570)delete(2608-2618)set_payload(2630-2654)overwrite_payload(2656-2670)delete_payload(2672-2688)clear_payload(2690-2703)batch_update_points(2705-2744)
qdrant_client/async_qdrant_client.py (1)
qdrant_client/http/models/models.py (1)
Filter(884-894)
qdrant_client/http/models/models.py (2)
qdrant_client/grpc/points_pb2.pyi (17)
b(3343-3344)update_filter(665-666)update_filter(807-808)update_filter(3405-3406)update_filter(3564-3565)points(657-657)points(703-704)points(798-799)points(1032-1033)points(3400-3400)points(3558-3559)points(3612-3613)points(3633-3634)points(4921-4922)points(5001-5001)Rrf(2753-2768)rrf(2811-2812)qdrant_client/grpc/collections_pb2.pyi (11)
stopwords(2021-2022)metadata(1527-1528)metadata(1638-1639)metadata(1855-1856)warnings(2285-2286)CollectionWarning(773-783)languages(1984-1985)custom(1987-1988)sparse_config(1283-1284)sparse_config(1377-1377)WalConfigDiff(903-929)
qdrant_client/async_qdrant_remote.py (4)
qdrant_client/http/models/models.py (6)
Filter(884-894)PointStruct(1972-1975)PointsList(1992-1998)Batch(61-64)PointsBatch(1983-1989)UpdateVectors(3325-3328)qdrant_client/conversions/conversion.py (12)
RestToGrpc(2501-4809)convert_filter(167-183)convert_filter(2503-2523)convert_shard_key_selector(2194-2197)convert_shard_key_selector(4621-4628)convert_read_consistency(1737-1746)convert_read_consistency(4093-4103)GrpcToRest(141-2491)convert_update_result(468-472)convert_update_result(2687-2691)convert_payload(584-585)convert_payload(2805-2806)qdrant_client/conversions/common_types.py (1)
get_args_subscribed(34-46)qdrant_client/qdrant_remote.py (1)
update_vectors(1999-2047)
qdrant_client/local/local_collection.py (2)
qdrant_client/local/payload_filters.py (1)
calculate_payload_mask(317-335)qdrant_client/hybrid/fusion.py (2)
reciprocal_rank_fusion(4-29)distribution_based_score_fusion(32-72)
qdrant_client/uploader/grpc_uploader.py (2)
qdrant_client/conversions/conversion.py (4)
RestToGrpc(2501-4809)payload_to_grpc(91-92)convert_extended_point_id(3159-3166)convert_vector_struct(3388-3428)qdrant_client/grpc/points_service_pb2_grpc.py (1)
PointsStub(8-156)
qdrant_client/conversions/conversion.py (5)
qdrant_client/grpc/collections_pb2.pyi (24)
metadata(1527-1528)metadata(1638-1639)metadata(1855-1856)HasField(375-375)HasField(411-411)HasField(439-439)HasField(471-471)HasField(501-501)HasField(522-522)HasField(544-544)HasField(570-570)HasField(658-658)HasField(702-702)HasField(750-750)HasField(847-847)HasField(892-892)HasField(922-922)HasField(1025-1025)HasField(1067-1067)shard_key(2520-2521)shard_key(2550-2551)shard_key(2604-2604)shard_key(2803-2804)shard_key(2834-2835)qdrant_client/async_qdrant_remote.py (2)
upsert(1673-1752)update_vectors(1754-1799)qdrant_client/qdrant_remote.py (2)
upsert(1907-1997)update_vectors(1999-2047)qdrant_client/http/models/models.py (5)
MatchTextAny(1577-1582)RrfQuery(2452-2453)Rrf(2444-2449)PointsList(1992-1998)UpdateVectors(3325-3328)qdrant_client/grpc/points_pb2.pyi (51)
rrf(2811-2812)Rrf(2753-2768)update_filter(665-666)update_filter(807-808)update_filter(3405-3406)update_filter(3564-3565)upsert(3659-3659)points(657-657)points(703-704)points(798-799)points(1032-1033)points(3400-3400)points(3558-3559)points(3612-3613)points(3633-3634)points(4921-4922)points(5001-5001)shard_key(3842-3843)shard_key(4206-4207)shard_key_selector(662-663)shard_key_selector(709-710)shard_key_selector(756-757)shard_key_selector(804-805)shard_key_selector(877-878)shard_key_selector(941-942)shard_key_selector(994-995)shard_key_selector(1038-1039)shard_key_selector(1478-1479)shard_key_selector(1639-1640)shard_key_selector(1771-1772)shard_key_selector(1821-1822)shard_key_selector(1905-1906)shard_key_selector(2058-2059)shard_key_selector(2217-2218)shard_key_selector(2309-2310)update_vectors(3671-3671)UpdateVectors(3551-3578)Match(4718-4771)Query(2772-2829)vectors(564-564)vectors(841-842)vectors(871-872)vectors(1218-1218)vectors(1250-1250)vectors(1268-1268)vectors(1289-1289)vectors(3590-3591)vectors(3839-3840)vectors(4204-4204)vectors(5064-5064)PointStructList(3393-3419)
qdrant_client/uploader/rest_uploader.py (4)
qdrant_client/http/api_client.py (1)
SyncApis(42-57)qdrant_client/grpc/points_pb2.pyi (2)
models(5151-5151)ShardKeySelector(617-634)qdrant_client/http/models/models.py (3)
Batch(61-64)PointsList(1992-1998)PointStruct(1972-1975)qdrant_client/uploader/uploader.py (1)
BaseUploader(26-94)
tests/test_local_persistence.py (6)
qdrant_client/qdrant_client.py (8)
QdrantClient(27-3191)collection_exists(2244-2255)delete_collection(2313-2331)create_collection(2333-2424)rest(209-223)get_collection(2231-2242)update_collection(2257-2311)close(175-182)qdrant_client/local/async_qdrant_local.py (6)
collection_exists(877-882)delete_collection(909-923)create_collection(925-972)get_collection(873-875)update_collection(884-901)close(74-90)qdrant_client/local/qdrant_local.py (6)
collection_exists(946-951)delete_collection(980-995)create_collection(997-1050)get_collection(942-944)update_collection(953-972)close(74-93)qdrant_client/http/models/models.py (4)
VectorParams(3412-3437)Distance(728-739)SparseVectorParams(3018-3028)Modifier(1641-1650)qdrant_client/local/local_collection.py (1)
close(173-175)qdrant_client/local/persistence.py (1)
close(100-101)
qdrant_client/qdrant_client.py (1)
qdrant_client/http/models/models.py (1)
Filter(884-894)
qdrant_client/local/async_qdrant_local.py (4)
qdrant_client/local/qdrant_local.py (10)
_get_collection(165-173)upsert(776-781)_default_update_result(816-820)update_vectors(783-791)delete_vectors(793-802)_save(144-163)_collection_path(974-978)delete_collection(980-995)closed(71-72)scroll(733-752)qdrant_client/local/local_collection.py (4)
upsert(2473-2509)update_vectors(2543-2553)delete_vectors(2555-2570)scroll(1886-1920)qdrant_client/async_qdrant_client.py (5)
upsert(1529-1602)update_vectors(1604-1660)delete_vectors(1662-1709)delete_collection(2238-2255)scroll(1370-1437)qdrant_client/qdrant_client.py (5)
upsert(1575-1654)update_vectors(1656-1714)delete_vectors(1716-1764)delete_collection(2313-2331)scroll(1413-1481)
🪛 Ruff (0.14.2)
qdrant_client/async_qdrant_remote.py
1581-1581: Unused method argument: kwargs
(ARG002)
qdrant_client/local/local_collection.py
821-821: Avoid specifying long messages outside the exception class
(TRY003)
828-828: zip() without an explicit strict= parameter
Add explicit value for parameter strict=
(B905)
2441-2441: Avoid specifying long messages outside the exception class
(TRY003)
2499-2499: Prefer TypeError exception for invalid type
(TRY004)
2499-2499: Avoid specifying long messages outside the exception class
(TRY003)
qdrant_client/uploader/grpc_uploader.py
36-36: zip() without an explicit strict= parameter
Add explicit value for parameter strict=
(B905)
106-106: Avoid specifying long messages outside the exception class
(TRY003)
qdrant_client/conversions/conversion.py
3662-3662: Avoid specifying long messages outside the exception class
(TRY003)
qdrant_client/uploader/rest_uploader.py
93-93: Avoid specifying long messages outside the exception class
(TRY003)
qdrant_client/local/qdrant_local.py
787-787: Unused method argument: kwargs
(ARG002)
798-798: Unused method argument: kwargs
(ARG002)
980-980: Unused method argument: kwargs
(ARG002)
1139-1139: Unused method argument: field_name
(ARG002)
qdrant_client/local/async_qdrant_local.py
728-728: Unused method argument: kwargs
(ARG002)
739-739: Unused method argument: kwargs
(ARG002)
909-909: Unused method argument: kwargs
(ARG002)
911-911: Avoid specifying long messages outside the exception class
(TRY003)
1066-1066: Unused method argument: collection_name
(ARG002)
1066-1066: Unused method argument: field_name
(ARG002)
1066-1066: Unused method argument: kwargs
(ARG002)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Redirect rules - poetic-froyo-8baba7
- GitHub Check: Header rules - poetic-froyo-8baba7
- GitHub Check: Pages changed - poetic-froyo-8baba7
🔇 Additional comments (46)
qdrant_client/local/local_collection.py (11)
59-59: LGTM!The import of
check_filteris necessary to support the newupdate_filterfeature in upsert/update operations.
980-986: LGTM!The error handling correctly raises
ValueError(instead ofAssertionError) for fusion, formula, and RRF queries when prefetches are missing. The error messages are clear and user-friendly.
1229-1229: LGTM!UUID-to-string normalization ensures consistent ID lookups. This aligns with the internal representation where UUIDs are stored as strings in
self.ids.
2280-2282: LGTM with a note on input mutation.UUID normalization is correctly applied. Note that this mutates the input
point.idin-place, which is acceptable for an internal method but should be kept consistent across the codebase.
2339-2343: LGTM!UUID normalization is consistently applied before adding points to internal dictionaries.
2496-2527: LGTM!The
update_filterparameter is correctly propagated to_upsert_pointfor both list and batch upsert operations.
2571-2593: Verify that the early return is intentional.The
update_filterlogic mirrors_upsert_point, correctly applying conditional updates. When the filter excludes a point, the function returnsNoneearly (line 2591).However, the early
returnapplies to the entire function rather than just the current point in the loop. This means if any point is filtered out, the function stops processing remaining points in thepointssequence.Is this intended, or should it use
continueto skip only the filtered point?If only the current point should be skipped:
if not check_filter( update_filter, self.payload[idx], self.ids_inv[idx], has_vector ): - return None + continue
2637-2642: LGTM!UUID normalization is consistently applied for both list and
PointIdsListselectors, ensuring uniform string-based ID handling.
2750-2781: LGTM!The
update_filteris correctly extracted and propagated for bothUpsertOperation(withPointsBatchandPointsListvariants) andUpdateVectorsOperation.
2798-2835: LGTM!The
metadatafield is correctly propagated fromself.config.metadatato theCollectionInforesponse. This aligns with the PR objective to add collection-level metadata support.Also note:
vectors_counthas been correctly removed fromCollectionInfo, as indicated in the PR objectives.
2438-2494: The update_filter concern is moot—the early return is unreachable with current code.The
update_filterparameter was added to_upsert_point, but no caller ever passes it. Both call sites (lines 2476 and 2491) are in the publicupsert()method, which itself doesn't acceptupdate_filter. This meansupdate_filteralways defaults toNone, making the condition at line 2483 (if not self.deleted[idx] and update_filter is not None:) always false. The early return on line 2488 is dead code and will never execute.Either the feature is incomplete (the public
upsert()method should accept and passupdate_filter), or this parameter was added defensively for future expansion. Either way, callers don't need to handleNonereturns because they'll never receive them.Likely an incorrect or invalid review comment.
qdrant_client/conversions/conversion.py (11)
1-1: LGTM: UUID import for ExtendedPointId support.The
uuidimport is correctly added to support UUID-to-string conversion inconvert_extended_point_idat line 3162.
284-284: LGTM: copy_vectors field added to HNSW config conversions.The
copy_vectorsfield is consistently handled in both gRPC→REST and REST→gRPC directions with proper optional field checking usingHasField.Also applies to: 300-300
682-683: LGTM: text_any match variant added.The
text_anymatch handling follows the same pattern as other match variants (text,keyword,phrase, etc.) and correctly maps torest.MatchTextAny.
1359-1362: LGTM: RRF query conversion correctly implemented.The RRF conversion correctly:
- Reads
kfrom therrfobject (notmodel.k)- Respects optional field semantics using
HasField("k")- Passes
Nonewhenkis not set
3659-3664: LGTM: REST→gRPC RRF conversion avoids setting None on optional scalar.The conversion correctly creates an empty
grpc.Rrf()and only setskwhen it's notNone, avoiding the protobuf anti-pattern of setting optional scalars toNone.
1983-1990: LGTM: update_filter propagation in gRPC→REST conversions.The
update_filterfield is correctly extracted from gRPC upsert and update_vectors operations usingHasField("update_filter")and passed to the REST models.Also applies to: 2092-2099
2384-2386: LGTM: max_payload_index_count added to strict mode configs.The
max_payload_index_countfield is consistently handled in bothconvert_strict_mode_configandconvert_strict_mode_config_outputwith proper optional field checking.Also applies to: 2454-2456
2880-2880: LGTM: REST→gRPC metadata conversions are correct.These conversions check
model.metadata is not None, which is correct for REST models wheremetadatais typed asOptional[Dict[str, Any]]and can actually beNone(unlike protobuf maps in the gRPC→REST direction).Also applies to: 3021-3021
3162-3163: LGTM: UUID support for ExtendedPointId.Converting
uuid.UUIDinstances to strings before creatinggrpc.PointIdis a clean approach to support Python UUID objects in the API.
4401-4410: LGTM: update_filter propagation in REST→gRPC conversions.The
update_filteris consistently extracted from REST models and converted to gRPC filters in both upsert and update_vectors operations.Also applies to: 4516-4527
4740-4740: LGTM: max_payload_index_count in REST→gRPC strict mode config conversions.Consistent handling of
max_payload_index_countin bothconvert_strict_mode_configandconvert_strict_mode_config_outputmethods.Also applies to: 4774-4774
qdrant_client/async_qdrant_client.py (5)
1611-1642: update_filter on update_vectors is consistentSignature, docs, and forwarding are aligned with Filter semantics.
Also applies to: 1653-1660
2506-2531: upload_points gains update_filter: OKMatches upsert semantics; forwarded to underlying client.
Also applies to: 2551-2561
2577-2604: upload_collection gains update_filter: OKSignature/docstring/forwarding consistent with other write paths.
Also applies to: 2625-2637
2196-2214: metadata on update_collection: merge semantics documentedOptional metadata forwarded; pairs with async_local implementation merging dicts.
Also applies to: 2233-2235
2276-2324: metadata on create/recreate_collection: OKSignature/docs/forwarding are consistent.
Also applies to: 2344-2346, 2366-2414, 2439-2441
tests/congruence_tests/test_query.py (2)
366-378: Parametrized RRF helper looks goodMethod builds RrfQuery with k=10 and mirrors existing RRF paths.
1293-1295: Good to cover parametrized RRF in fusion testsAdding this comparison strengthens cross-backend parity.
qdrant_client/hybrid/fusion.py (1)
6-6: Verify RRF default k aligns with server/local behaviorDEFAULT_RANKING_CONSTANT_K = 2 significantly affects fusion. If the server’s default differs (historically 60 in many RRF impls), local vs remote results may diverge when no k is provided. Please confirm the intended default and align both sides or gate in tests.
Also applies to: 10-18
tests/test_qdrant_client.py (1)
1170-1189: Metadata coverage looks solidThanks for exercising both the initial metadata persistence and the merge-on-update path when the server supports it. The version gating keeps older deployments unaffected.
qdrant_client/uploader/grpc_uploader.py (1)
85-142: Nice job plumbingupdate_filterthrough gRPC uploadsThe single conversion in
__init__and reuse inprocess_uploadavoids redundant work, and the PointId normalization keeps generated IDs valid.qdrant_client/qdrant_remote.py (4)
2941-2963: Recreate: metadata propagation LGTM.
3063-3078: upload_points: parameter and plumbing LGTM.
3094-3113: upload_collection: parameter and plumbing LGTM.
1806-1846: REST client signature verified—consistency parameter is properly supported.The REST PointsApi.count_points method accepts
consistency: m.ReadConsistency = Noneas a parameter in both async and sync variants, confirming the code change is correct.qdrant_client/async_qdrant_remote.py (4)
2633-2672: Async recreate_collection: metadata propagation LGTM.
2768-2783: upload_points (async): LGTM.
2799-2814: upload_collection (async): LGTM.
1574-1617: Async count consistency wiring verified—code is correct.The async REST client's
count_pointsmethod accepts aconsistencyparameter (line 564, points_api.py) and properly propagates it. The code correctly passes consistency to both gRPC and REST paths.qdrant_client/qdrant_client.py (6)
1575-1654: LGTM — upsert method correctly implements update_filterThe
update_filterparameter is correctly typed astypes.Filter, properly documented, and forwarded to the underlying client. The previous type annotation issue has been resolved.
1656-1714: LGTM — update_vectors method correctly implements update_filterThe
update_filterparameter is correctly typed astypes.Filter, properly documented, and forwarded to the underlying client. The docstring appropriately describes the filtering behavior for vector updates.
2257-2311: LGTM — update_collection method correctly implements metadata parameterThe
metadataparameter is properly typed astypes.Payloadfor arbitrary JSON-like data, clearly documented with merge semantics, and correctly forwarded to the underlying client.
2333-2424: LGTM — create_collection method correctly implements metadata parameterThe
metadataparameter is properly typed, documented, and forwarded to the underlying client implementation.
2580-2648: LGTM — upload_points method correctly implements update_filterThe
update_filterparameter is correctly typed astypes.Filter, properly documented, and forwarded to the underlying client.
2650-2729: LGTM — upload_collection method correctly implements update_filterThe
update_filterparameter is correctly typed astypes.Filter, properly documented, and forwarded to the underlying client.
| "anyOf": [{"$ref": "#/$defs/TokenizerType"}, {"type": "null"}], | ||
| "default": None, | ||
| "description": "Configuration of the local bm25 models.", | ||
| }, | ||
| "language": { | ||
| "anyOf": [{"type": "string"}, {"type": "null"}], |
There was a problem hiding this comment.
Clarify the BM25 tokenizer field.
The description currently repeats the overall config text, so downstream docs don’t explain what this field does. Please describe that it selects the tokenizer (and add the missing title for consistency with sibling properties).
"tokenizer": {
"anyOf": [{"$ref": "#/$defs/TokenizerType"}, {"type": "null"}],
"default": None,
- "description": "Configuration of the local bm25 models.",
+ "description": "Tokenizer to apply before scoring; leave unset to inherit the collection-level default.",
+ "title": "Tokenizer",
},🤖 Prompt for AI Agents
In qdrant_client/embed/_inspection_cache.py around lines 1481 to 1486, the
"language" property is missing a title and its description repeats the overall
config; update it to include a title (e.g., "Tokenizer language" or "language")
and replace the description with a concise explanation that this field selects
which BM25 tokenizer to use (or null to use the default tokenizer), so the docs
clearly state the purpose and allowed values.
| if isinstance(query, models.RrfQuery): | ||
| fused = reciprocal_rank_fusion( | ||
| responses=sources, limit=limit + offset, ranking_constant_k=query.rrf.k | ||
| ) |
There was a problem hiding this comment.
Critical: Parameter name mismatch in reciprocal_rank_fusion call.
The call passes ranking_constant_k=query.rrf.k, but based on the function definition in qdrant_client/hybrid/fusion.py (lines 3-28), reciprocal_rank_fusion does not accept a ranking_constant_k parameter. The function currently hardcodes ranking_constant = 2 inside compute_score.
This will raise a TypeError at runtime when RrfQuery is used.
Apply this fix if reciprocal_rank_fusion should accept a configurable constant:
Option 1: Update the fusion function signature (in qdrant_client/hybrid/fusion.py):
def reciprocal_rank_fusion(
responses: list[list[models.ScoredPoint]],
limit: int = 10,
ranking_constant_k: int = 2
) -> list[models.ScoredPoint]:
def compute_score(pos: int) -> float:
return 1 / (ranking_constant_k + pos)
# ... rest of the functionOption 2: If the constant should remain fixed, remove the parameter from this call:
- if isinstance(query, models.RrfQuery):
- fused = reciprocal_rank_fusion(
- responses=sources, limit=limit + offset, ranking_constant_k=query.rrf.k
- )
+ if isinstance(query, models.RrfQuery):
+ fused = reciprocal_rank_fusion(
+ responses=sources, limit=limit + offset
+ )🤖 Prompt for AI Agents
In qdrant_client/local/local_collection.py around lines 814-817 the call to
reciprocal_rank_fusion passes a keyword argument ranking_constant_k=query.rrf.k
which does not exist on the function signature and will raise a TypeError; fix
by either (A) updating qdrant_client/hybrid/fusion.py to accept
ranking_constant_k (add it to the reciprocal_rank_fusion parameters and use it
inside compute_score) so the call remains as-is, or (B) if the constant should
remain fixed, remove the ranking_constant_k argument from this call and let the
fusion function use its internal hardcoded value.
|
|
||
| compare_client_results(local_client, remote_client, search_matrix_offsets_uuid_filter) | ||
|
|
||
| cl.scroll(COLLECTION_NAME, scroll_filter=id_filter) |
There was a problem hiding this comment.
Fix NameError: cl is undefined here
This stray call will crash the test. Remove it (the next compare already exercises scroll).
- cl.scroll(COLLECTION_NAME, scroll_filter=id_filter)
+ # removed stray call; scroll is exercised below via compare_client_results📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| cl.scroll(COLLECTION_NAME, scroll_filter=id_filter) | |
| # removed stray call; scroll is exercised below via compare_client_results |
🤖 Prompt for AI Agents
In tests/congruence_tests/test_uuids.py around line 111, there is a stray call
to cl.scroll(COLLECTION_NAME, scroll_filter=id_filter) where cl is undefined;
remove this line so the test no longer raises NameError (the subsequent compare
already covers scrolling).
* new: add match text any local mode * tests: add match text any tests
* new: update models, remove init_from and locks * deprecate: remove init from tests * deprecate: remove lock tests * new: convert ascii_folding * fix: fix type stub * new: convert acorn * new: convert shard key with fallback * new: update grpcio and grpcio tools in generator (#1106) * new: update grpcio and grpcio tools in generator * fix: bind grpcio and tools versions to 1.62.0 in generator * Remove deprecated methods (#1103) * deprecate: remove old api methods * deprecate: remove type stub for removed methods * deprecate: remove old api methods from test_qdrant_client * deprecate: replace search with query points in test_in_memory * deprecate: replace search methods in fastembed mixin with query points * deprecate: replace old api methods in test async qdrant client * deprecate: replace search with query points in test delete points * deprecate: replace discover and context with query points in test_discovery * deprecate: replace recommend_groups with query_points_groups in test_group_recommend * deprecate: replace search_groups in test_group_search * deprecate: replace recommend with query points in test_recommendation * deprecate: replace search with query points in test search * deprecate: replace context and discover with query points in test sparse discovery * deprecate: replace search with query points in test sparse idf search * deprecate: replace recommend with query points in test sparse recommend * deprecate: replace search with query points in test sparse search * deprecate: replace missing search request with query request in qdrant_fastembed * deprecate: replace search with query points in test multivector search queries * deprecate: replace upload records with upload points in test_updates * deprecate: remove redundant structs (#1104) * deprecate: remove redundant structs * fix: do not use removed conversions in local mode * fix: remove redundant conversions, simplify types.QueryRequest * deprecate: replace old style grpc vector conversion to a new one (#1105) * deprecate: replace old style grpc vector conversion to a new one * fix: ignore union attr in conversion * review fixes --------- Co-authored-by: generall <[email protected]> --------- Co-authored-by: generall <[email protected]> --------- Co-authored-by: generall <[email protected]> * new: deprecate add, query, query_batch in fastembed mixin (#1102) * new: deprecate add, query, query_batch in fastembed mixin * 1.16 -> 1.17 --------- Co-authored-by: generall <[email protected]> --------- Co-authored-by: generall <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (8)
qdrant_client/async_qdrant_remote.py (2)
1696-1764: Convert metadata when provided, not only when dict.The current
isinstance(metadata, dict)checks are too restrictive and may skip conversion for Mapping types or pydantic-validated aliases. Change all four instances toif metadata is not None.Locations to fix:
qdrant_client/async_qdrant_remote.py:1718(update_collection)qdrant_client/async_qdrant_remote.py:1822(create_collection)qdrant_client/qdrant_remote.py:1872(update_collection)qdrant_client/qdrant_remote.py:1989(create_collection)- if isinstance(metadata, dict): - metadata = RestToGrpc.convert_payload(metadata) + if metadata is not None: + metadata = RestToGrpc.convert_payload(metadata)Note:
recreate_collectiondelegates tocreate_collection, so no separate fix is needed there.
556-615: Bug: shard_key on lookup_from silently dropped during REST-to-gRPC conversion.The
RestToGrpc.convert_lookup_locationmethod (qdrant_client/conversions/conversion.py:3776-3780) only convertscollection_nameandvector_name, but omits theshard_keyfield. Sincerest.LookupLocationhasshard_key: Optional[ShardKeySelector]andgrpc.LookupLocationsupportsshard_key_selector, any user-providedlookup_from.shard_keyis silently discarded, causing queries to search unintended shards.Fix: Update
RestToGrpc.convert_lookup_locationto extract and convertmodel.shard_key(if present) toshard_key_selectorin the gRPC message.qdrant_client/http/models/models.py (1)
1537-1550: Convert REST LookupLocation.shard_key field to gRPC LookupLocation.shard_key_selectorThe shard_key field in rest.LookupLocation is silently dropped during REST→gRPC conversion. Proto definition confirms grpc.LookupLocation accepts optional ShardKeySelector shard_key_selector, but convert_lookup_location (line 3776) only maps collection_name and vector_name. This causes shard routing directives to be ignored at runtime.
Apply the fix to qdrant_client/conversions/conversion.py line 3776:
def convert_lookup_location(cls, model: rest.LookupLocation) -> grpc.LookupLocation: return grpc.LookupLocation( collection_name=model.collection, vector_name=model.vector, + shard_key_selector=( + cls.convert_shard_key_selector(model.shard_key) + if model.shard_key is not None + else None + ), )The convert_shard_key_selector function (line 4304) already correctly handles ShardKeyWithFallback with fallback mapping, so no additional changes are needed there.
tests/congruence_tests/test_sparse_recommend.py (1)
253-284: Fix return type to match other searcher methods.The return type
list[models.QueryResponse]is incompatible with the method signature expected bycompare_client_results. All other searcher methods (lines 82, 215, 230, 241) returnlist[models.ScoredPoint], butrecommend_batchreturnslist[models.QueryResponse]. SinceQueryResponsecontains apointsfield (list ofScoredPoint) rather than individual scored points, this breaks the comparison logic. Changerecommend_batchto returnlist[models.ScoredPoint]by extracting points from the responses returned byquery_batch_points.qdrant_client/async_qdrant_fastembed.py (1)
346-370: Guard against missing payload to avoid AttributeErrorscored_point.payload can be None when with_payload=False or server omits it. Accessing payload.get(...) will crash.
Apply:
- response.append( + payload = scored_point.payload or {} + response.append( QueryResponse( id=scored_point.id, embedding=embedding, sparse_embedding=sparse_embedding, - metadata=scored_point.payload, - document=scored_point.payload.get("document", ""), + metadata=payload, + document=payload.get("document", ""), score=scored_point.score, ) )tests/congruence_tests/test_sparse_discovery.py (1)
9-9: Remove stale import (ContextExamplePair is no longer used)Cleanup to avoid confusion and linter noise.
-from qdrant_client.http.models import ContextExamplePairqdrant_client/conversions/conversion.py (1)
2033-2040: Fix theHasFieldguard for sparse index conversion.
model.HasField("index") is not Noneis always true becauseHasFieldalready yields a boolean. As written we now materialize anindexconfig even when the field is unset, which mutates the REST view and can surface default/empty configs unexpectedly. Drop theis not Noneso we only convert when the field is actually present.Apply this diff:
- if model.HasField("index") is not None + if model.HasField("index")qdrant_client/qdrant_remote.py (1)
1141-1163: Don't reconvert already-gRPC PointVectorsIn the gRPC path we now blanket-call
RestToGrpc.convert_point_vectorson every element. That helper expects RESTmodels.PointVectors, so feeding it an object that is already agrpc.PointVectorswill either blow up (missing REST-only attributes) or silently produce a malformed message. Prior to this change we only converted when the element was a REST model, soprefer_grpc=Truecallers could pass native gRPC structures. This regression will break those call sites.Please guard the conversion, mirroring the upsert logic:
- points = [RestToGrpc.convert_point_vectors(point) for point in points] + points = [ + RestToGrpc.convert_point_vectors(point) + if isinstance(point, models.PointVectors) + else point + for point in points + ]
♻️ Duplicate comments (7)
qdrant_client/local/local_collection.py (1)
814-817: Verify parameter compatibility after prior review concern.A past review flagged that
reciprocal_rank_fusiondoesn't acceptranking_constant_k. However, the relevant code snippet fromqdrant_client/hybrid/fusion.py(lines 8-35) indicates:"Optional parameter ranking_constant_k (if provided) overrides default ranking constant"
This suggests the function signature was updated to accept this parameter. Please confirm that
reciprocal_rank_fusionnow supportsranking_constant_kto resolve the earlier concern.#!/bin/bash # Verify that reciprocal_rank_fusion accepts ranking_constant_k parameter ast-grep --pattern $'def reciprocal_rank_fusion($$$, ranking_constant_k$$$)$$$'qdrant_client/embed/_inspection_cache.py (2)
446-446: Fix the missing space in the midpoint description.The generated text still omits the space before “Defaults”, exactly as the earlier review noted. Please apply the previously suggested fix so the sentence reads “…between 0 and 1. Defaults to 0.5…”.
1483-1489: Clarify the BM25 tokenizer field docs.This property still reuses the generic config blurb and lacks a title, so the docs aren’t telling users what the field actually does. Please add an explicit title and update the description per the earlier feedback, e.g.:
"tokenizer": { "anyOf": [{"$ref": "#/$defs/TokenizerType"}, {"type": "null"}], "default": None, - "description": "Configuration of the local bm25 models.", + "description": "Select the tokenizer to run before scoring; leave unset to inherit the collection default.", + "title": "Tokenizer", },qdrant_client/http/models/models.py (1)
3359-3361: Fill missing description for UpdateVectors.update_filter.The Field lacks a description, unlike PointsBatch/PointsList. Please add a description consistent with them in the generator/spec. I know this file is autogen. Raising upstream is sufficient.
qdrant_client/grpc/collections_pb2.pyi (1)
11-11: Use package-relative import for sibling proto module.Absolute
import json_with_int_pb2may fail when installed asqdrant_client.grpc. Switch to a relative import.Apply:
-import json_with_int_pb2 +from . import json_with_int_pb2Verify remaining occurrences:
#!/usr/bin/env bash rg -nC2 '^\s*import\s+json_with_int_pb2\b' qdrant_client/grpc/*.pyitests/test_qdrant_client.py (1)
1009-1019: Widen collection metadata test to cover mixed types (dup of prior review)Broader types catch REST↔gRPC round‑trip issues.
- collection_metadata = {"ownership": "Bart Simpson's property"} + collection_metadata = { + "ownership": "Bart Simpson's property", + "year": 1990, + "rating": 4.2, + "published": True, + "notes": None, + "tags": ["simpson", "animated"], + "nested": {"creator": "Matt Groening", "ids": [1, 2, 3]}, + }qdrant_client/local/qdrant_local.py (1)
739-744: HandleNonemetadata before calling.update.If a collection has never stored metadata (default is
None),update_collection(..., metadata=...)will raiseAttributeErrorat Line 740 because_collection.config.metadataisNone. Initialize the metadata dictionary before updating.Apply this diff:
if metadata is not None: + if _collection.config.metadata is None: + _collection.config.metadata = {} _collection.config.metadata.update(metadata) updated = TrueBased on past review comments.
🧹 Nitpick comments (11)
tests/type_stub.py (1)
32-103: Consider using keyword arguments for improved maintainability.While positional arguments may be intentional for type checking, using keyword arguments would make these test calls more resilient to API signature changes and improve readability. For example:
qdrant_client.create_collection( collection_name="collection", vectors_config=types.VectorParams(size=128, distance=rest_models.Distance.COSINE), # ... other parameters with explicit names strict_mode_config=rest_models.StrictModeConfig(), metadata={}, )This is especially helpful when signatures evolve, as evidenced by this PR's changes.
qdrant_client/async_qdrant_remote.py (1)
649-661: Nit: unused kwargs in several methods.kwargs is unused; consider removing or prefixing with underscore to appease linters.
Also applies to: 701-712, 831-841
qdrant_client/grpc/points_pb2.pyi (1)
2723-2724: Decay midpoint doc tightened; consider validating range upstream.Optional: add client-side validation (HTTP/Pydantic) to keep midpoint in [0,1] as per doc to fail fast.
qdrant_client/grpc/collections_service_pb2.py (1)
23-23: Ruff E712: prefernotover== Falsein generated guard.Tiny, no-behavior-change tweak to quiet linters; or ignore generated dirs in lint config.
Apply:
-if _descriptor._USE_C_DESCRIPTORS == False: +if not _descriptor._USE_C_DESCRIPTORS:Alternatively, exclude
qdrant_client/grpc/*.pyfrom E712 in your linter config to avoid churn in generated code.qdrant_client/grpc/points_service_pb2.py (1)
23-23: Ruff E712: prefernotover== Falsein generated guard.Same small linter fix as collections service; or exclude generated files from lint.
Apply:
-if _descriptor._USE_C_DESCRIPTORS == False: +if not _descriptor._USE_C_DESCRIPTORS:qdrant_client/async_qdrant_fastembed.py (2)
537-539: Use DeprecationWarning and proper stacklevel in deprecation noticesThese should surface in user code, not inside the library, and use the right category.
- show_warning_once( - "`add` method has been deprecated and will be removed in 1.17. Instead, inference can be done internally within regular methods like `upsert` by wrapping data into `models.Document` or `models.Image`." - ) + show_warning_once( + "`add` method has been deprecated and will be removed in 1.17. Instead, inference can be done internally within regular methods like `upsert` by wrapping data into `models.Document` or `models.Image`.", + category=DeprecationWarning, + stacklevel=3, + )Repeat the same change for
queryandquery_batchdeprecation calls.Also applies to: 611-614, 693-696
592-596: Docstring still references ‘search’; should point to ‘query_points’Update the guidance to reflect the new API.
- If you want to use your own query vector, use `search` method instead. + If you want to use your own query vector, use `query_points` instead.tests/congruence_tests/test_sparse_discovery.py (1)
79-85: Return .points for consistency with other testscompare_client_results can unwrap, but keeping return types consistent reduces surprises.
- return client.query_points( + return client.query_points( collection_name=COLLECTION_NAME, query=models.ContextQuery(context=[models.ContextPair(positive=10, negative=19)]), with_payload=True, limit=200, using="sparse-image", - ) + ).pointstests/test_async_qdrant_client.py (1)
216-219: Avoid blocking the event loop in async testsUse asyncio.sleep in @pytest.mark.asyncio tests.
- time.sleep( - 0.5 - ) # wait param is not propagated https://github.com/qdrant/qdrant-client/issues/254 + await asyncio.sleep( + 0.5 + ) # wait param is not propagated https://github.com/qdrant/qdrant-client/issues/254qdrant_client/http/api/distributed_api.py (1)
4-6: Duplicate BaseModel importKeep a single import to avoid shadowing and confusion.
-from pydantic import BaseModel -from pydantic.main import BaseModel +from pydantic import BaseModeltests/conversions/fixtures.py (1)
1520-1526: Include text_any in Match fixtures to exercise new variantEnsures conversions cover the newly added proto field.
- "Match": [match_keyword, match_integer, match_bool, match_text], + "Match": [match_keyword, match_integer, match_bool, match_text, match_text_any],
| # -*- coding: utf-8 -*- | ||
| # Generated by the protocol buffer compiler. DO NOT EDIT! | ||
| # source: collections_service.proto | ||
| # Protobuf Python Version: 4.25.1 |
There was a problem hiding this comment.
🧩 Analysis chain
Protobuf runtime compatibility check.
Header says “Protobuf Python Version: 4.25.1”. Ensure your declared protobuf dependency matches this generator/runtime to avoid import/runtime issues on older 3.x.
Run:
🏁 Script executed:
#!/usr/bin/env bash
# Find declared protobuf requirements
rg -nC2 '(protobuf|google-protobuf)' -g '!**/dist/**' -g '!**/build/**'
# List generated files noting declared version headers
rg -n 'Protobuf Python Version' qdrant_client/grpc/*_service_pb2.py qdrant_client/grpc/*_pb2.pyLength of output: 258914
Update protobuf minimum version to match generated code requirements.
The pyproject.toml currently declares protobuf = ">=3.20.0", but all generated _pb2.py files show # Protobuf Python Version: 4.25.1 headers. Code generated by protobuf 4.25.1 requires protobuf 4.x at runtime—protobuf 3.x versions (including 3.20.x) are incompatible and will cause import/runtime failures.
Fix: Update pyproject.toml to declare protobuf = ">=4.25.1" (or minimally >=4.0.0) to prevent users from installing incompatible protobuf 3.x versions. The lock file already pins protobuf 6.31.1, but the declared minimum must reflect the actual runtime requirement.
🤖 Prompt for AI Agents
pyproject.toml (root) lines 1-1: The generated _pb2.py files require protobuf
4.x (header shows 4.25.1) but pyproject currently allows protobuf >=3.20.0;
update the dependency spec in pyproject.toml to protobuf = ">=4.25.1" (or at
least ">=4.0.0") so runtime uses a compatible protobuf version, then
regenerate/update the lockfile if needed and run tests to ensure no
import/runtime protobuf errors.
| USAGE_FIELD_NUMBER: builtins.int | ||
| @property | ||
| def hits(self) -> google.protobuf.internal.containers.RepeatedCompositeFieldContainer[global___FacetHit]: ... | ||
| time: builtins.float | ||
| """Time spent to process""" | ||
| @property | ||
| def usage(self) -> global___Usage: ... | ||
| def __init__( | ||
| self, | ||
| *, | ||
| hits: collections.abc.Iterable[global___FacetHit] | None = ..., | ||
| time: builtins.float = ..., | ||
| usage: global___Usage | None = ..., | ||
| ) -> None: ... | ||
| def ClearField(self, field_name: typing_extensions.Literal["hits", b"hits", "time", b"time"]) -> None: ... | ||
| def HasField(self, field_name: typing_extensions.Literal["_usage", b"_usage", "usage", b"usage"]) -> builtins.bool: ... | ||
| def ClearField(self, field_name: typing_extensions.Literal["_usage", b"_usage", "hits", b"hits", "time", b"time", "usage", b"usage"]) -> None: ... | ||
| def WhichOneof(self, oneof_group: typing_extensions.Literal["_usage", b"_usage"]) -> typing_extensions.Literal["usage"] | None: ... | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
FacetResponse.usage: align HTTP surfaces and observability.
Good addition. Verify HTTP responses expose usage consistently and that metrics are documented.
🏁 Script executed:
#!/bin/bash
# HTTP models and API surfaces for usage
rg -nP -C3 --type=py '\bFacet(Response)?\b|usage\b' qdrant_client/http
# Tests asserting usage presence
rg -nP -C2 --type=py 'usage' testsLength of output: 22868
---
🏁 Script executed:
```shell
#!/bin/bash
# Get full FacetResponse definition from HTTP models
rg -nA30 'class FacetResponse' qdrant_client/http/models/models.py
Length of output: 1603
🏁 Script executed:
#!/bin/bash
# Check if FacetResponse in HTTP has usage field
rg -nP 'class FacetResponse|usage.*Usage' qdrant_client/http/models/models.py | head -20Length of output: 1403
🏁 Script executed:
#!/bin/bash
# Verify InlineResponse20020 (wrapper for FacetResponse endpoint)
rg -nA10 'class InlineResponse20020' qdrant_client/http/models/models.pyLength of output: 666
🏁 Script executed:
#!/bin/bash
# Check gRPC points_pb2.pyi for FacetResponse definition
rg -nA20 'class FacetResponse' qdrant_client/grpc/points_pb2.pyiLength of output: 1053
---
🏁 Script executed:
```shell
#!/bin/bash
# Check for test assertions on usage field in HTTP responses
rg -nP 'usage|Usage' tests/ --type=py | grep -i 'facet\|response\|assert' | head -20
Length of output: 0
🏁 Script executed:
#!/bin/bash
# Find Usage class definition and any documentation
rg -nB2 -A10 'class Usage' qdrant_client/http/models/models.pyLength of output: 481
🏁 Script executed:
#!/bin/bash
# Check if there are docstrings or comments explaining usage metric
rg -nB3 'Time spent to process|usage' qdrant_client/http/models/models.py | head -30Length of output: 1774
🏁 Script executed:
#!/bin/bash
# Verify if HTTP FacetResponse has any other fields beyond hits
ast-grep --pattern 'class FacetResponse {
$$$
}'Length of output: 0
🏁 Script executed:
#!/bin/bash
# Check for proto definitions that might show intended structure
fd -e proto -e protobuf | head -5Length of output: 197
---
🏁 Script executed:
```shell
#!/bin/bash
# Check proto definition for FacetResponse to see source of truth
rg -nA15 'message FacetResponse' qdrant_client/proto/points.proto
Length of output: 501
🏁 Script executed:
#!/bin/bash
# Verify no HTTP test files exist that test FacetResponse directly
find tests -name '*facet*' -o -name '*test*http*' | head -10Length of output: 36
---
🏁 Script executed:
```shell
#!/bin/bash
# Examine the facet test file to understand test coverage
cat tests/congruence_tests/test_facet.py
Length of output: 6032
🏁 Script executed:
#!/bin/bash
# Verify if HTTP FacetResponse is generated or manually defined
rg -nB5 'class FacetResponse' qdrant_client/http/models/models.py | head -20Length of output: 206
HTTP FacetResponse model is incomplete—missing time and usage fields.
The proto definition (points.proto:1060) specifies FacetResponse contains hits, time, and usage. The gRPC surface correctly exposes all three fields. The HTTP FacetResponse model (models.py:837) has only hits, causing misalignment between surfaces. The HTTP wrapper pattern (InlineResponse20020) adds these fields at the response envelope level, but FacetResponse itself should reflect the proto structure for consistency.
Add time: float and usage: Optional[Usage] fields to the HTTP FacetResponse model (qdrant_client/http/models/models.py:837) to align with the proto definition and gRPC surface.
🤖 Prompt for AI Agents
In qdrant_client/http/models/models.py around line 837, the HTTP FacetResponse
model lacks the proto-specified fields; add time: float and usage:
Optional[Usage] to the FacetResponse model definition (with from typing import
Optional and referencing the existing Usage model), give usage a default of None
and time a sensible default (e.g., 0.0) or make it Optional[float] if nullable,
and update the constructor/type hints and any model schema/serialization helpers
so the HTTP model matches the gRPC/proto structure.
| # -*- coding: utf-8 -*- | ||
| # Generated by the protocol buffer compiler. DO NOT EDIT! | ||
| # source: points_service.proto | ||
| # Protobuf Python Version: 4.25.1 |
There was a problem hiding this comment.
🧩 Analysis chain
Protobuf runtime compatibility check.
Generated with 4.25.1—verify runtime/packaging requires protobuf >=4.25 to prevent subtle breakage.
Run:
🏁 Script executed:
#!/usr/bin/env bash
# Inspect declared protobuf version and cross-check all generated headers
rg -nC2 '(protobuf|google-protobuf)' -g '!**/dist/**' -g '!**/build/**'
rg -n 'Protobuf Python Version' qdrant_client/grpc/*_service_pb2.py qdrant_client/grpc/*_pb2.pyLength of output: 258914
Update pyproject.toml protobuf requirement to match generated code version.
The review comment identifies a real compatibility issue. The current dependency constraint is protobuf = ">=3.20.0", but the script output confirms that all protobuf-generated files in qdrant_client/grpc/ are explicitly marked as generated with version 4.25.1. This creates a runtime incompatibility risk: users could install protobuf 3.20.x, which is incompatible with code generated for 4.25.1.
Required action:
Update pyproject.toml line 27 to require at least protobuf 4.25:
protobuf = ">=4.25.0"
🤖 Prompt for AI Agents
In pyproject.toml at line 27, the protobuf version constraint must be raised to
match generated files (qdrant_client/grpc/* show Protobuf Python Version:
4.25.1); edit that line to require protobuf >=4.25.0 (protobuf = ">=4.25.0") and
then update your lock/install (e.g., poetry lock / pip-compile or reinstall
dependencies) so the new constraint is applied.
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check the current protobuf requirement in pyproject.toml
cat pyproject.toml | grep -A2 -B2 "protobuf"
Length of output: 345
@joein, your local tests passing is expected if you have protobuf >=4.25.0 installed in your environment. The concern is about the dependency declaration in pyproject.toml, which currently allows protobuf = ">=3.20.0".
When the protobuf compiler generates files with # Protobuf Python Version: 4.25.1, it typically means the generated code uses features or APIs from that version. While your environment works fine, users installing the package could end up with protobuf 3.20.x, which might cause issues.
Suggested verification:
# Test with minimum declared version to verify compatibility
pip install 'protobuf==3.20.0' && python -m pytest tests/If that test passes, the current constraint is fine. If it fails, updating to protobuf = ">=4.25.0" would be safer. Since types-protobuf = "^4.21.0.5" is already in your dev dependencies, it seems the project is already using protobuf 4.x features.
The decision is ultimately yours as the maintainer—you might have insights about backward compatibility that aren't obvious from the diff alone.
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (7)
qdrant_client/embed/_inspection_cache.py (2)
759-768: Correct “lop-left” → “top-left” in GeoBoundingBox docs.Minor typo repeated 3 times.
- "description": "Geo filter request Matches coordinates inside the rectangle, described by coordinates of lop-left and bottom-right edges", + "description": "Geo filter request. Matches coordinates inside the rectangle, described by coordinates of top-left and bottom-right edges", @@ - "description": "Geo filter request Matches coordinates inside the rectangle, described by coordinates of lop-left and bottom-right edges", + "description": "Geo filter request. Matches coordinates inside the rectangle, described by coordinates of top-left and bottom-right edges", @@ - "description": "Geo filter request Matches coordinates inside the rectangle, described by coordinates of lop-left and bottom-right edges", + "description": "Geo filter request. Matches coordinates inside the rectangle, described by coordinates of top-left and bottom-right edges",
2166-2171: Typo: “upto” → “up to”.Small language fix in SparseIndexParams.full_scan_threshold.
- "description": "We prefer a full scan search upto (excluding) this number of vectors. Note: this is number of vectors, not KiloBytes.", + "description": "We prefer a full scan search up to (excluding) this number of vectors. Note: this is number of vectors, not kilobytes.",qdrant_client/proto/points.proto (1)
281-286: Fix comment: vectors vs payload keys.WithVectorsSelector.include refers to vector names, not payload keys.
message WithVectorsSelector { oneof selector_options { bool enable = 1; // If `true` - return all vectors, if `false` - none - VectorsSelector include = 2; // List of payload keys to include into result + VectorsSelector include = 2; // List of vector names to include in the result } }qdrant_client/async_qdrant_remote.py (3)
616-647: Convert grpc.WithLookup to REST before building QueryGroupsRequest (REST path).Passing a grpc.WithLookup into REST models will fail serialization. Convert when grpc type is supplied.
else: if isinstance(query_filter, grpc.Filter): query_filter = GrpcToRest.convert_filter(model=query_filter) if isinstance(search_params, grpc.SearchParams): search_params = GrpcToRest.convert_search_params(search_params) if isinstance(with_payload, grpc.WithPayloadSelector): with_payload = GrpcToRest.convert_with_payload_selector(with_payload) if isinstance(lookup_from, grpc.LookupLocation): lookup_from = GrpcToRest.convert_lookup_location(lookup_from) + if isinstance(with_lookup, grpc.WithLookup): + with_lookup = GrpcToRest.convert_with_lookup(with_lookup) query_request = models.QueryGroupsRequest( shard_key=shard_key_selector, prefetch=prefetch, query=query, using=using, filter=query_filter, params=search_params, score_threshold=score_threshold, limit=limit, group_by=group_by, group_size=group_size, with_vector=with_vectors, with_payload=with_payload, with_lookup=with_lookup, lookup_from=lookup_from, )
1719-1733: Always convert metadata for gRPC; don’t restrict to dict.metadata can be various accepted Payload forms. Convert whenever it’s provided to avoid type mismatches at runtime.
- if isinstance(metadata, dict): - metadata = RestToGrpc.convert_payload(metadata) + if metadata is not None: + metadata = RestToGrpc.convert_payload(metadata)
1823-1840: Same fix for create_collection (gRPC path): convert metadata when provided.- if isinstance(metadata, dict): - metadata = RestToGrpc.convert_payload(metadata) + if metadata is not None: + metadata = RestToGrpc.convert_payload(metadata)qdrant_client/grpc/collections_pb2.pyi (1)
2271-2305: Add missing field conversions to GrpcToRest and RestToGrpc converters.The following proto fields are not being converted:
- CollectionInfo.warnings (repeated CollectionWarning) – missing from both
convert_collection_infomethods (lines 233 and 2491)- WalConfigDiff.wal_retain_closed (optional uint64) – missing from both
convert_wal_config_diffmethods (lines 697 and 2783)- CollectionInfo.indexed_vectors_count – missing only from RestToGrpc at line 2491
TextIndexParams.ascii_folding and HnswConfigDiff.inline_storage are already correctly handled.
♻️ Duplicate comments (6)
tests/test_qdrant_client.py (1)
999-1018: This segment was already reviewed in a previous commit. The past review comment suggests widening the metadata test payload to include mixed types (numbers, booleans, null, nested objects, arrays) to better exercise REST↔gRPC conversions.qdrant_client/proto/collections.proto (1)
6-6: Critical: Fix the broken proto import.The past review comment already identified this:
import "json_with_int.proto"fails to compile (file doesn't exist per Buf static analysis). Themap<string, Value>fields (lines 424, 437, 477) requiregoogle/protobuf/struct.protoinstead.This is a blocking compilation error that prevents proto generation.
qdrant_client/embed/_inspection_cache.py (2)
1482-1487: Clarify BM25 tokenizer field and add title.Repeat of earlier suggestion; improves generated docs.
"tokenizer": { "anyOf": [{"$ref": "#/$defs/TokenizerType"}, {"type": "null"}], "default": None, - "description": "Configuration of the local bm25 models.", + "description": "Tokenizer to apply before scoring; leave unset to inherit the collection-level default.", + "title": "Tokenizer", },
446-446: Fix missing space in “midpoint” description.Same typo persists.
- "description": "The midpoint of the decay. Should be between 0 and 1.Defaults to 0.5. Output will be this value when `|x - target| == scale`.", + "description": "The midpoint of the decay. Should be between 0 and 1. Defaults to 0.5. Output will be this value when `|x - target| == scale`.",qdrant_client/http/models/models.py (1)
3365-3365: Empty description for update_filter field.The
update_filterfield has an empty description. This was already discussed in previous reviews and requires fixing in the autogeneration source.qdrant_client/grpc/collections_pb2.pyi (1)
11-11: Use package-relative import for sibling proto module.Absolute import can fail when installed as
qdrant_client.grpc. Switch to relative to ensure resolution across environments.-import json_with_int_pb2 +from . import json_with_int_pb2
🧹 Nitpick comments (5)
tests/test_qdrant_client.py (1)
1123-1131: Verify the shard key's initial state.The test creates a shard key with
initial_state=models.ReplicaState.ACTIVEbut doesn't verify it was applied. Consider adding an assertion to confirm the shard's replica state matches the requested initial state.client.create_shard_key( collection_name=COLLECTION_NAME, shard_key=fish_shard_key, initial_state=models.ReplicaState.ACTIVE, ) - print("created shard key with replica state") + # Verify the shard was created with the expected state + cluster_info = client.get_collection_cluster_info(COLLECTION_NAME) + fish_shards = [s for s in cluster_info.local_shards if s.shard_key == fish_shard_key] + assert len(fish_shards) > 0, "Fish shard not found" + assert fish_shards[0].state == models.ReplicaState.ACTIVEqdrant_client/embed/_inspection_cache.py (2)
2533-2537: Deduplicate on_disk default phrasing.Remove repeated “Default…” sentence.
- "description": "If true, store the index on disk. Default: false. Default is false.", + "description": "If true, store the index on disk. Default: false.",
2575-2581: Add title/description for TextIndexParams.tokenizer.Parallels BM25 tokenizer; improves clarity.
"tokenizer": { "anyOf": [{"$ref": "#/$defs/TokenizerType"}, {"type": "null"}], "default": None, - "description": "", + "description": "Tokenizer to use for indexing and queries; leave unset to use the collection default.", + "title": "Tokenizer", },qdrant_client/proto/points.proto (1)
122-125: Document ShardKeySelector.fallback semantics.Add a brief comment so users know when fallback applies.
message ShardKeySelector { repeated ShardKey shard_keys = 1; // List of shard keys which should be used in the request - optional ShardKey fallback = 2; + optional ShardKey fallback = 2; // Optional fallback shard key used when the target shard(s) are unavailable or planner selects the fallback path }qdrant_client/async_qdrant_remote.py (1)
560-560: Silence Ruff ARG002 for unused**kwargsin autogenerated client.These are intentional for forward-compat; prefer config over code edits.
Add per-file ignore in pyproject (keeps generator output clean):
# pyproject.toml [tool.ruff.lint.per-file-ignores] "qdrant_client/async_qdrant_remote.py" = ["ARG002"]Also applies to: 659-659, 711-711, 839-839, 2317-2317
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
qdrant_client/async_qdrant_remote.py(32 hunks)qdrant_client/conversions/common_types.py(4 hunks)qdrant_client/conversions/conversion.py(33 hunks)qdrant_client/embed/_inspection_cache.py(62 hunks)qdrant_client/grpc/collections_pb2.pyi(33 hunks)qdrant_client/http/models/models.py(46 hunks)qdrant_client/proto/collections.proto(14 hunks)qdrant_client/proto/points.proto(15 hunks)qdrant_client/qdrant_remote.py(32 hunks)tests/conversions/fixtures.py(22 hunks)tests/test_qdrant_client.py(22 hunks)
🧰 Additional context used
🧬 Code graph analysis (7)
qdrant_client/conversions/common_types.py (3)
qdrant_client/grpc/points_pb2.pyi (1)
PointId(249-271)qdrant_client/http/models/models.py (2)
QueryRequest(2121-2153)ReplicaState(2380-2397)qdrant_client/grpc/collections_pb2.pyi (1)
ReplicaState(268-268)
tests/test_qdrant_client.py (7)
qdrant_client/async_qdrant_remote.py (7)
query_points(377-484)query_batch_points(486-528)create_collection(1782-1875)get_collection(1655-1669)update_collection(1685-1764)create_shard_key(2308-2355)query_points_groups(530-647)qdrant_client/qdrant_remote.py (7)
query_points(428-553)query_batch_points(555-602)create_collection(1938-2046)get_collection(1805-1817)update_collection(1832-1921)create_shard_key(2495-2540)query_points_groups(604-736)qdrant_client/local/async_qdrant_local.py (7)
query_points(349-382)query_batch_points(384-402)create_collection(716-744)get_collection(664-666)update_collection(675-692)create_shard_key(913-924)query_points_groups(404-458)qdrant_client/local/qdrant_local.py (7)
query_points(382-418)query_batch_points(420-441)create_collection(769-800)get_collection(714-716)update_collection(725-744)create_shard_key(978-989)query_points_groups(443-498)qdrant_client/qdrant_client.py (7)
query_points(273-446)query_batch_points(229-271)create_collection(1632-1720)get_collection(1530-1541)update_collection(1556-1610)create_shard_key(2355-2385)query_points_groups(448-614)qdrant_client/http/models/models.py (6)
Filter(914-924)QueryRequest(2121-2153)RecommendQuery(2261-2262)RecommendInput(2249-2258)ReplicaState(2380-2397)SparseVector(3028-3034)tests/utils.py (1)
read_version(9-25)
tests/conversions/fixtures.py (4)
qdrant_client/grpc/points_pb2.pyi (66)
Match(4769-4822)match(4722-4723)field(4599-4599)params(1512-1513)params(1668-1669)params(1928-1929)params(2076-2077)params(2250-2251)params(2907-2908)params(2979-2980)params(3120-3121)multi_dense(444-445)multi_dense(500-501)multi_dense(591-591)MultiDenseVector(557-568)vectors(562-562)vectors(845-846)vectors(875-876)vectors(1222-1222)vectors(1254-1254)vectors(1272-1272)vectors(1293-1293)vectors(3641-3642)vectors(3890-3891)vectors(4255-4255)vectors(5115-5115)DenseVector(523-534)data(280-280)data(430-431)data(486-487)data(528-528)VectorsOutput(1285-1302)vector(1270-1270)vector(1291-1291)vector(1501-1502)vector(1657-1658)vector(2183-2183)VectorOutput(476-519)dense(438-439)dense(494-495)dense(587-587)NamedVectorsOutput(1232-1260)sparse(441-442)sparse(497-498)sparse(589-589)SparseVector(538-553)values(544-544)indices(433-434)indices(489-490)indices(546-546)quantization(1442-1445)Vectors(1264-1281)Vector(417-472)ShardKeySelector(615-638)shard_keys(626-627)fallback(629-629)id(585-585)id(842-843)id(2181-2181)id(3880-3881)id(3952-3953)id(4251-4251)id(5111-5111)Query(2823-2880)rrf(2862-2863)Rrf(2804-2819)qdrant_client/http/models/models.py (7)
StrictModeConfig(3124-3169)CollectionConfig(241-261)SparseVector(3028-3034)TextIndexParams(3248-3268)TokenizerType(3275-3279)Rrf(2468-2473)ReplicaState(2380-2397)qdrant_client/grpc/collections_pb2.pyi (25)
StrictModeConfig(1231-1355)metadata(1528-1529)metadata(1636-1637)metadata(1853-1854)CollectionConfig(1807-1871)params(496-496)params(517-517)params(1618-1619)params(1835-1836)params(2223-2224)hnsw_config(356-357)hnsw_config(401-402)hnsw_config(1495-1496)hnsw_config(1621-1622)hnsw_config(1838-1839)optimizer_config(1841-1842)wal_config(1498-1499)wal_config(1844-1845)strict_mode_config(1525-1526)strict_mode_config(1633-1634)strict_mode_config(1850-1851)TextIndexParams(1997-2059)TokenizerType(232-232)ShardKey(2489-2506)ReplicaState(268-268)qdrant_client/grpc/json_with_int_pb2.pyi (1)
Value(84-131)
qdrant_client/conversions/conversion.py (3)
qdrant_client/grpc/collections_pb2.pyi (25)
metadata(1528-1529)metadata(1636-1637)metadata(1853-1854)HasField(379-379)HasField(415-415)HasField(443-443)HasField(475-475)HasField(505-505)HasField(526-526)HasField(548-548)HasField(574-574)HasField(662-662)HasField(706-706)HasField(754-754)HasField(851-851)HasField(896-896)HasField(926-926)HasField(1029-1029)HasField(1071-1071)shard_key(2524-2525)shard_key(2554-2555)shard_key(2608-2608)shard_key(2808-2809)shard_key(2844-2845)ReplicaState(268-268)qdrant_client/grpc/points_pb2.pyi (86)
AcornSearchParams(1386-1420)rrf(2862-2863)Rrf(2804-2819)update_filter(669-670)update_filter(811-812)update_filter(3456-3457)update_filter(3615-3616)points(661-661)points(707-708)points(802-803)points(1036-1037)points(3451-3451)points(3609-3610)points(3663-3664)points(3684-3685)points(4972-4973)points(5052-5052)shard_key(3893-3894)shard_key(4257-4258)shard_key_selector(666-667)shard_key_selector(713-714)shard_key_selector(760-761)shard_key_selector(808-809)shard_key_selector(881-882)shard_key_selector(945-946)shard_key_selector(998-999)shard_key_selector(1042-1043)shard_key_selector(1529-1530)shard_key_selector(1690-1691)shard_key_selector(1822-1823)shard_key_selector(1872-1873)shard_key_selector(1956-1957)shard_key_selector(2109-2110)shard_key_selector(2268-2269)shard_key_selector(2360-2361)update_vectors(3722-3722)UpdateVectors(3602-3629)ShardKeySelector(615-638)fallback(629-629)shard_keys(626-627)target(2236-2237)target(2442-2443)target(2718-2719)Match(4769-4822)sparse(441-442)sparse(497-498)sparse(589-589)SparseVector(538-553)values(544-544)indices(433-434)indices(489-490)indices(546-546)VectorOutput(476-519)PointId(249-271)multi_dense(444-445)multi_dense(500-501)multi_dense(591-591)MultiDenseVector(557-568)vectors(562-562)vectors(845-846)vectors(875-876)vectors(1222-1222)vectors(1254-1254)vectors(1272-1272)vectors(1293-1293)vectors(3641-3642)vectors(3890-3891)vectors(4255-4255)vectors(5115-5115)DenseVector(523-534)data(280-280)data(430-431)data(486-487)data(528-528)vector(1270-1270)vector(1291-1291)vector(1501-1502)vector(1657-1658)vector(2183-2183)Vector(417-472)dense(438-439)dense(494-495)dense(587-587)Query(2823-2880)PointsUpdateOperation(3441-3745)PointStructList(3444-3470)qdrant_client/http/models/models.py (8)
AcornSearchParams(36-48)MatchTextAny(1590-1595)RrfQuery(2476-2477)Rrf(2468-2473)UpdateVectors(3362-3365)ReplicaState(2380-2397)ShardKeyWithFallback(2789-2791)SparseVector(3028-3034)
qdrant_client/grpc/collections_pb2.pyi (3)
qdrant_client/http/models/models.py (2)
ReplicaState(2380-2397)CollectionWarning(385-386)qdrant_client/grpc/points_pb2.pyi (62)
ClearField(224-224)ClearField(244-244)ClearField(270-270)ClearField(286-286)ClearField(308-308)ClearField(327-327)ClearField(349-349)ClearField(370-370)ClearField(392-392)ClearField(413-413)ClearField(466-466)ClearField(513-513)ClearField(534-534)ClearField(553-553)ClearField(568-568)ClearField(610-610)HasField(243-243)HasField(269-269)HasField(307-307)HasField(348-348)HasField(369-369)HasField(391-391)HasField(412-412)HasField(465-465)HasField(512-512)HasField(609-609)HasField(636-636)HasField(681-681)HasField(724-724)HasField(775-775)HasField(823-823)HasField(853-853)WhichOneof(245-245)WhichOneof(271-271)WhichOneof(468-468)WhichOneof(470-470)WhichOneof(472-472)WhichOneof(515-515)WhichOneof(517-517)WhichOneof(519-519)WhichOneof(611-611)WhichOneof(638-638)WhichOneof(684-684)WhichOneof(686-686)WhichOneof(688-688)WhichOneof(690-690)WhichOneof(727-727)WhichOneof(729-729)value(300-300)value(341-341)value(384-384)value(914-914)value(1210-1210)value(1242-1242)value(2484-2484)value(3292-3293)value(3482-3482)value(3533-3533)value(3862-3862)value(4235-4235)value(5097-5097)value(5190-5190)qdrant_client/grpc/json_with_int_pb2.pyi (2)
value(61-61)Value(84-131)
qdrant_client/http/models/models.py (2)
qdrant_client/grpc/points_pb2.pyi (33)
AcornSearchParams(1386-1420)b(3394-3395)update_filter(669-670)update_filter(811-812)update_filter(3456-3457)update_filter(3615-3616)points(661-661)points(707-708)points(802-803)points(1036-1037)points(3451-3451)points(3609-3610)points(3663-3664)points(3684-3685)points(4972-4973)points(5052-5052)filter(1504-1505)filter(1660-1661)filter(1805-1806)filter(1920-1921)filter(2068-2069)filter(2242-2243)filter(2352-2353)filter(2904-2905)filter(2976-2977)filter(3117-3118)filter(3218-3219)filter(3321-3322)filter(4605-4605)filter(4694-4695)filter(5054-5054)Rrf(2804-2819)fallback(629-629)qdrant_client/grpc/collections_pb2.pyi (10)
stopwords(2020-2021)stemmer(2025-2026)metadata(1528-1529)metadata(1636-1637)metadata(1853-1854)warnings(2289-2290)CollectionWarning(777-787)languages(1982-1983)custom(1985-1986)WalConfigDiff(907-933)
qdrant_client/qdrant_remote.py (3)
qdrant_client/http/models/models.py (15)
Prefetch(2023-2047)Filter(914-924)SearchParams(2634-2652)LookupLocation(1540-1553)GroupsResult(1051-1052)WithLookup(3570-3577)QueryGroupsRequest(2077-2118)SearchMatrixPairsResponse(2614-2615)SearchMatrixRequest(2618-2631)SearchMatrixOffsetsResponse(2597-2601)PointsList(2005-2011)Batch(76-79)PointsBatch(1996-2002)UpdateVectors(3362-3365)ReplicaState(2380-2397)qdrant_client/conversions/conversion.py (14)
RestToGrpc(2417-4569)convert_query(1335-1375)convert_query(3493-3530)convert_prefetch_query(1378-1396)convert_prefetch_query(3635-3655)GrpcToRest(141-2407)convert_groups_result(1753-1756)convert_groups_result(3981-3984)convert_filter(167-183)convert_filter(2419-2439)convert_shard_key_selector(2094-2113)convert_shard_key_selector(4372-4388)convert_shard_key(2052-2057)convert_shard_key(4329-4335)qdrant_client/async_qdrant_remote.py (5)
grpc_points(324-333)http(369-375)query_points_groups(530-647)search_matrix_pairs(649-699)search_matrix_offsets(701-751)
🪛 Buf (1.59.0)
qdrant_client/proto/collections.proto
6-6: import "json_with_int.proto": file does not exist
(COMPILE)
🪛 Ruff (0.14.4)
qdrant_client/conversions/conversion.py
2091-2091: Avoid specifying long messages outside the exception class
(TRY003)
2109-2111: Avoid specifying long messages outside the exception class
(TRY003)
4369-4369: Avoid specifying long messages outside the exception class
(TRY003)
qdrant_client/async_qdrant_remote.py
560-560: Unused method argument: kwargs
(ARG002)
659-659: Unused method argument: kwargs
(ARG002)
711-711: Unused method argument: kwargs
(ARG002)
839-839: Unused method argument: kwargs
(ARG002)
2317-2317: Unused method argument: kwargs
(ARG002)
qdrant_client/qdrant_remote.py
634-634: Unused method argument: kwargs
(ARG002)
748-748: Unused method argument: kwargs
(ARG002)
804-804: Unused method argument: kwargs
(ARG002)
2504-2504: Unused method argument: kwargs
(ARG002)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: Redirect rules - poetic-froyo-8baba7
- GitHub Check: Header rules - poetic-froyo-8baba7
- GitHub Check: Pages changed - poetic-froyo-8baba7
- GitHub Check: Python 3.9.x on ubuntu-latest test
- GitHub Check: Python 3.11.x on ubuntu-latest test
- GitHub Check: Python 3.13.x on ubuntu-latest test
- GitHub Check: Python 3.12.x on ubuntu-latest test
- GitHub Check: Python 3.10.x on ubuntu-latest test
🔇 Additional comments (25)
tests/conversions/fixtures.py (10)
38-38: LGTM: text_any matching support properly wired.The new
text_anyfield for matching any word in text is correctly integrated through the fixture chain (Match → FieldCondition → Condition → Filter).Also applies to: 52-52, 129-129, 150-150
297-297: LGTM: New configuration fields added.The additions of
inline_storageto HNSW config andmax_payload_index_countto strict mode config are consistent with the updated protobuf definitions.Also applies to: 363-363
371-388: LGTM: Collection metadata support added.The metadata fixture and
collection_config_w_metadatavariant correctly demonstrate the new collection-level metadata feature, properly registered in the fixtures dictionary.
404-425: LGTM: Vector representation correctly migrated.The fixtures properly migrate from the deprecated
data/vectors_countfields to the new structured format (dense,sparse,multi_dense), aligning with the PR's core objective of removingvectors_count.Also applies to: 444-467
481-487: Clarify the intent of commented-out fields.The
scored_point_order_value_floatfixture has several fields commented out (payload, score, version, order_value), leaving onlyidandvectors. If this is intentionally testing a minimal/sparse case, consider adding a comment explaining the purpose to avoid confusion.
509-514: LGTM: New search and indexing parameters added.The
acornsearch parameters andascii_foldingtext index parameters are correctly integrated, providing test coverage for these new features.Also applies to: 542-549
872-917: LGTM: Point vectors migrated to new representations.The
point_structandmany_vectorsfixtures consistently use the new dense/sparse/multi_dense vector representations, providing comprehensive test coverage.
981-981: LGTM: Metadata and shard key fallback features integrated.The metadata parameter in
update_collection_3and the new shard key selector with fallback are properly implemented and used in test fixtures.Also applies to: 1051-1080
1114-1126: LGTM: RRF queries and updated point vectors properly registered.The RRF query variants (with k, default, explicit None) and updated point vector fixtures provide comprehensive test coverage for the new query types and vector representations.
Also applies to: 1394-1396, 1584-1585, 1626-1628
1453-1462: LGTM: Comprehensive ReplicaState enum coverage.All ReplicaState variants are properly defined and registered, providing complete test coverage for the expanded replica state enumeration.
Also applies to: 1636-1647
tests/test_qdrant_client.py (2)
396-402: LGTM: Consistent API migration from search to query_points.The migration from the search API to the new
query_pointsAPI is consistent throughout:
- Correctly uses
queryparameter instead ofquery_vector- Properly accesses results via
.pointsonQueryResponse- Batch operations correctly use
QueryRequestobjects- Recommend operations correctly wrapped in
RecommendQueryAlso applies to: 408-414, 535-541, 552-558, 562-623, 626-639, 688-735, 818-837, 1097-1105, 1168-1240, 1298-1316, 1369-1396, 1888-1947
2010-2012: Clarify the REST client workaround in gRPC test.The code creates a REST client within a gRPC test to fetch the payload schema. This suggests a potential issue with the gRPC path. Please clarify:
- Is there a known bug with gRPC's
get_collectionfor parametrized indexes?- Should this workaround be documented or is it a temporary fix?
qdrant_client/proto/collections.proto (3)
368-387: LGTM: Comprehensive strict mode configuration.The expanded
StrictModeConfigprovides granular control over collection limits:
- Query/timeout limits
- Filtering configuration
- Search parameters
- Batch size limits
- Storage limits
- Multi-vector and sparse vector configs
All fields are well-documented and optional.
202-208: LGTM: Well-documented inline storage feature.The new
inline_storagefield for HNSW configuration is clearly documented with its trade-offs and restrictions.
165-167: LGTM: Incremental proto additions align with PR objectives.The following additions are well-structured:
CollectionWarningmessage for collection-level warningswal_retain_closedfor WAL configurationascii_foldingfor text normalization- Deprecation of
vectors_count(reserved field 3)warningsfield onCollectionInfoActiveReadreplica stateinitial_statefor shard key creationAll maintain backward compatibility through optional fields.
Also applies to: 230-230, 523-523, 572-580, 640-640, 744-744
qdrant_client/conversions/common_types.py (1)
60-60: UUID conversion support is properly implemented.The verification confirms that the conversion layer correctly handles UUID instances:
RestToGrpc.convert_extended_point_id(line 3027-3030) explicitly checks forisinstance(model, uuid.UUID)and converts to string before passing togrpc.PointId(uuid=model)GrpcToRest.convert_point_id(line 499-500) properly extracts the uuid field from gRPC PointIdThe addition of
UUIDto thePointIdunion is safe and fully supported by the existing conversion infrastructure.qdrant_client/proto/points.proto (6)
68-79: Deprecation path for legacy Vector fields looks good.Marking data/indices/vectors_count as deprecated while introducing oneof variants maintains wire compatibility. No issues spotted.
Also applies to: 81-90
132-139: update_filter on UpsertPoints: nice addition.Filter-constrained upserts are valuable; wire fields/numbers look fine.
Please ensure client helpers surface this field in high-level APIs (sync/async/local) to avoid silent omission.
160-167: update_filter on UpdatePointVectors: looks good.Consistent with UpsertPoints; no proto issues.
311-331: ACORN params integration acknowledged.New message and SearchParams.acorn field are coherent with HTTP schema.
Confirm doc/source-of-truth for the default of max_selectivity (0.4) matches server behavior to avoid client-side surprises.
Also applies to: 356-360
594-599: RRF support wired correctly.New Rrf message and Query.rrf variant align with fusion docs.
Also applies to: 699-716
1141-1153: Match.text_any addition is consistent with HTTP model.No wire or naming issues observed.
qdrant_client/embed/_inspection_cache.py (1)
2860-2864: Fix schema/description mismatch in source model, not auto-generated cache.The discrepancy exists in the source Pydantic model (
qdrant_client/http/models/models.py:45-47), not_inspection_cache.py. The cache file is auto-generated bypopulate_inspection_cache.pyfrom the model definition, so modifying it directly would be overwritten on regeneration.Real issue: The model has
default=Nonebut its description states"Default is 0.4". Choose one approach:
- Change
qdrant_client/http/models/models.pyline 46 fromdefault=Nonetodefault=0.4(if client-side default is intended)- Update the description to match
default=None(if server handles the default)Verify server-side ACORN behavior to determine which fix is correct.
qdrant_client/http/models/models.py (1)
36-49: LGTM: New model classes are well-structured.The new classes (AcornSearchParams, Bm25Config, CollectionSnapshotTelemetry, CollectionWarning, MatchTextAny, PeerMetadata, ReplicatePoints/ReplicatePointsOperation, Rrf/RrfQuery, ShardKeyWithFallback, StopwordsSet) follow consistent patterns with proper type annotations, optional field handling, and descriptive documentation.
Also applies to: 108-152, 354-358, 385-387, 1590-1596, 1954-1960, 2400-2407, 2468-2478, 2789-2792, 3115-3121
qdrant_client/async_qdrant_remote.py (1)
1699-1737: LGTM on metadata threading to REST and new collection fields.REST paths correctly pass
metadatainto models.UpdateCollection/CreateCollection. Once gRPC conversion is unconditional (above), both transports align.Optionally run tests that exercise updating and creating collections with metadata via both transports.
Also applies to: 1803-1844
| metadata=cls.convert_payload(model.metadata) if model.metadata is not None else None, | ||
| ) |
There was a problem hiding this comment.
Preserve optional metadata when gRPC map is empty.
model.metadata is always a map container, so this condition is always true. As a result we now surface {} even when the server never populated metadata, which downstream REST conversions treat as “clear metadata” and will wipe existing collection metadata. Please keep this field at None when the map is empty (apply the same fix to other metadata conversions, e.g. convert_update_collection).
- metadata=cls.convert_payload(model.metadata) if model.metadata is not None else None,
+ metadata=cls.convert_payload(model.metadata) if len(model.metadata) != 0 else None,🤖 Prompt for AI Agents
In qdrant_client/conversions/conversion.py around lines 268-269, the code always
preserves metadata because model.metadata (a map container) evaluates truthy
even when empty; change the assignment to set metadata to None when the map is
empty (e.g., use model.metadata if model.metadata and len(model.metadata) > 0
else None) so empty maps are not treated as "clear metadata". Apply the same
emptiness check to other metadata conversion sites mentioned (such as
convert_update_collection) so empty gRPC maps become None rather than {}.
| metadata=(cls.convert_payload(model.metadata) if model.metadata is not None else None), | ||
| ) |
There was a problem hiding this comment.
Don’t fabricate empty metadata updates.
model.metadata is a map and never None, so we now emit {} even when no metadata diff was provided. That causes the REST model to think there’s an explicit update and will reset metadata to empty. Guard on len(model.metadata) so we only forward non-empty updates (same pattern as above).
- metadata=(cls.convert_payload(model.metadata) if model.metadata is not None else None),
+ metadata=(
+ cls.convert_payload(model.metadata) if len(model.metadata) != 0 else None
+ ),📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| metadata=(cls.convert_payload(model.metadata) if model.metadata is not None else None), | |
| ) | |
| metadata=( | |
| cls.convert_payload(model.metadata) if len(model.metadata) != 0 else None | |
| ), | |
| ) |
🤖 Prompt for AI Agents
In qdrant_client/conversions/conversion.py around lines 805-806, the code always
emits metadata=(cls.convert_payload(model.metadata) if model.metadata is not
None else None) which produces {} when model.metadata is an empty dict and thus
signals an explicit metadata reset; change this to only forward metadata when
model.metadata contains entries (e.g., check len(model.metadata) > 0) and
otherwise set metadata to None so empty updates are not fabricated, following
the same guard pattern used elsewhere.
| def convert_acorn_search_params(cls, model: rest.AcornSearchParams) -> grpc.AcornSearchParams: | ||
| return grpc.AcornSearchParams( | ||
| enable=model.enable if model.enable is not None else None, | ||
| max_selectivity=model.max_selectivity if model.max_selectivity is not None else None, | ||
| ) |
There was a problem hiding this comment.
Avoid assigning None to protobuf scalars in ACORN params.
Passing enable=None / max_selectivity=None into the protobuf constructor raises TypeError because scalar fields can’t receive None. Build the message first and set each attribute only when the value exists.
- return grpc.AcornSearchParams(
- enable=model.enable if model.enable is not None else None,
- max_selectivity=model.max_selectivity if model.max_selectivity is not None else None,
- )
+ acorn = grpc.AcornSearchParams()
+ if model.enable is not None:
+ acorn.enable = model.enable
+ if model.max_selectivity is not None:
+ acorn.max_selectivity = model.max_selectivity
+ return acorn📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def convert_acorn_search_params(cls, model: rest.AcornSearchParams) -> grpc.AcornSearchParams: | |
| return grpc.AcornSearchParams( | |
| enable=model.enable if model.enable is not None else None, | |
| max_selectivity=model.max_selectivity if model.max_selectivity is not None else None, | |
| ) | |
| def convert_acorn_search_params(cls, model: rest.AcornSearchParams) -> grpc.AcornSearchParams: | |
| acorn = grpc.AcornSearchParams() | |
| if model.enable is not None: | |
| acorn.enable = model.enable | |
| if model.max_selectivity is not None: | |
| acorn.max_selectivity = model.max_selectivity | |
| return acorn |
🤖 Prompt for AI Agents
In qdrant_client/conversions/conversion.py around lines 2672 to 2676, avoid
passing None into protobuf scalar fields; instead instantiate
grpc.AcornSearchParams() first and then set enable and max_selectivity only when
model.enable and model.max_selectivity are not None (e.g., if model.enable is
not None: msg.enable = model.enable), so that you never assign None to scalar
fields and only set attributes when values exist.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/congruence_tests/test_sparse_discovery.py (1)
78-84: Missing.pointsextraction fromQueryResponse.The function signature indicates it should return
list[models.ScoredPoint], butquery_pointsreturns aQueryResponseobject. You need to append.pointsto extract the points list, as done consistently in all other test functions in this file (lines 120, 142, 168, 194, 212, 238, 295, 307).Apply this diff:
def f(client: QdrantBase, **kwargs: dict[str, Any]) -> list[models.ScoredPoint]: return client.query_points( collection_name=COLLECTION_NAME, query=models.ContextQuery(context=[models.ContextPair(positive=10, negative=19)]), with_payload=True, limit=200, using="sparse-image", - ) + ).points
🧹 Nitpick comments (2)
tests/congruence_tests/test_sparse_search.py (1)
31-69: Consider standardizing parameter order for consistency.The methods use inconsistent parameter ordering—some place
usingbeforequery, others after. While Python's named parameters make this functionally correct, standardizing the order would improve readability and maintainability.For example, consistently use:
client.query_points( collection_name=COLLECTION_NAME, query=self.query_text, using="sparse-text", ... )tests/congruence_tests/test_sparse_discovery.py (1)
249-249: Consider removing unusedkwargsparameter.The
kwargsparameter is declared but never used in this function. If it's not required by thecompare_client_resultscallback interface, consider removing it for cleaner signatures. The same pattern appears at line 300.Based on static analysis.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
qdrant_client/client_base.py(0 hunks)qdrant_client/local/multi_distances.py(0 hunks)qdrant_client/migrate/migrate.py(1 hunks)tests/congruence_tests/test_collections.py(0 hunks)tests/congruence_tests/test_delete_points.py(5 hunks)tests/congruence_tests/test_sparse_discovery.py(10 hunks)tests/congruence_tests/test_sparse_idf_search.py(1 hunks)tests/congruence_tests/test_sparse_search.py(3 hunks)
💤 Files with no reviewable changes (3)
- qdrant_client/local/multi_distances.py
- tests/congruence_tests/test_collections.py
- qdrant_client/client_base.py
🚧 Files skipped from review as they are similar to previous changes (1)
- qdrant_client/migrate/migrate.py
🧰 Additional context used
🧬 Code graph analysis (4)
tests/congruence_tests/test_sparse_discovery.py (7)
qdrant_client/client_base.py (3)
query_points(40-67)QdrantBase(6-396)query_batch_points(32-38)qdrant_client/qdrant_remote.py (2)
query_points(428-553)query_batch_points(555-602)qdrant_client/local/qdrant_local.py (2)
query_points(382-418)query_batch_points(420-441)qdrant_client/local/local_collection.py (2)
query_points(705-760)discover(1808-1870)qdrant_client/http/models/models.py (9)
ContextQuery(454-455)ContextPair(449-451)DiscoverQuery(708-709)DiscoverInput(701-705)LookupLocation(1540-1553)QueryResponse(2160-2161)QueryRequest(2121-2153)Filter(914-924)ScoredPoint(2520-2531)qdrant_client/local/distances.py (2)
ContextQuery(55-57)ContextPair(38-44)tests/congruence_tests/test_discovery.py (13)
f(64-72)f(83-91)f(102-109)f(123-140)f(151-164)f(175-187)f(198-209)f(222-233)f(246-257)f(268-275)f(348-376)f(384-395)f(403-410)
tests/congruence_tests/test_sparse_search.py (5)
qdrant_client/client_base.py (2)
query_points(40-67)QdrantBase(6-396)qdrant_client/local/qdrant_local.py (1)
query_points(382-418)qdrant_client/qdrant_client.py (1)
query_points(273-446)qdrant_client/local/local_collection.py (1)
query_points(705-760)tests/congruence_tests/test_search.py (8)
simple_search_image(38-45)simple_search_code(47-54)simple_search_text_offset(56-64)search_score_threshold(77-105)simple_search_text_select_payload(107-114)search_payload_exclude(116-123)simple_search_image_select_vector(125-133)filter_search_text(135-145)
tests/congruence_tests/test_delete_points.py (3)
qdrant_client/client_base.py (1)
query_points(40-67)qdrant_client/qdrant_client.py (1)
query_points(273-446)qdrant_client/local/local_collection.py (1)
query_points(705-760)
tests/congruence_tests/test_sparse_idf_search.py (3)
qdrant_client/client_base.py (1)
query_points(40-67)qdrant_client/qdrant_remote.py (1)
query_points(428-553)qdrant_client/qdrant_client.py (1)
query_points(273-446)
🪛 Ruff (0.14.4)
tests/congruence_tests/test_sparse_discovery.py
249-249: Unused function argument: kwargs
(ARG001)
300-300: Unused function argument: kwargs
(ARG001)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: Redirect rules - poetic-froyo-8baba7
- GitHub Check: Header rules - poetic-froyo-8baba7
- GitHub Check: Pages changed - poetic-froyo-8baba7
- GitHub Check: Python 3.13.x on ubuntu-latest test
- GitHub Check: Python 3.12.x on ubuntu-latest test
- GitHub Check: Python 3.11.x on ubuntu-latest test
- GitHub Check: Python 3.10.x on ubuntu-latest test
- GitHub Check: Python 3.9.x on ubuntu-latest test
🔇 Additional comments (5)
tests/congruence_tests/test_delete_points.py (2)
23-54: API migration correctly applied.The test successfully migrates from the search-based API to
query_pointswith consistent usage of thequeryandusingparameters. Results are correctly accessed via the.pointsattribute on theQueryResponse. Test logic remains functionally equivalent, and the added comment on line 44 improves clarity.
73-96: Sparse vector test correctly migrated.The sparse vector test follows the same migration pattern as the dense vector test, correctly using
query_pointswithquery=vectorandusing="sparse-image". The multi-line formatting in the lambda (lines 73-77) maintains readability.tests/congruence_tests/test_sparse_idf_search.py (1)
27-35: LGTM! Clean migration to the query_points API.The method correctly migrates from the legacy search API to
query_points, properly using theusingparameter to specify the vector field and thequeryparameter for the sparse vector input. The result handling via.pointsaligns with theQueryResponsestructure, and all original test functionality is preserved.tests/congruence_tests/test_sparse_search.py (2)
71-139: LGTM! Migration to query_points is correct.All search methods correctly use the
query_pointsAPI with proper parameter mapping and response handling via.pointsextraction.
343-374: LGTM! NaN test correctly updated for query_points API.The test correctly:
- Mutates the sparse vector values directly (no wrapper needed)
- Passes the sparse vector to
query_pointsas thequeryparameter- Expects appropriate exceptions from both local and remote clients when NaN values are present
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
qdrant_client/local/async_qdrant_local.py (1)
762-786: Normalize UUID ids in_upload_pointsto match other paths
upload_points/_upload_pointsnow acceptupdate_filterand delegate toLocalCollection.upsert, which is good. However,_upload_pointsstill forwardspoint.iddirectly:rest_models.PointStruct( id=point.id, vector=point.vector or {}, payload=point.payload or {}, )Elsewhere in this file (
upload_collection,_resolve_query_input,update_vectors) UUID ids are normalized tostrto match howLocalCollectionstores and looks up ids. For local uploads that use UUID-based ids, this inconsistency can lead to mismatched behavior (e.g., points inserted under UUID keys that aren’t found via string-based lookup).Consider normalizing here as well:
- collection.upsert( - [ - rest_models.PointStruct( - id=point.id, vector=point.vector or {}, payload=point.payload or {} - ) - for point in points - ], - update_filter=update_filter, - ) + collection.upsert( + [ + rest_models.PointStruct( + id=str(point.id) if isinstance(point.id, uuid.UUID) else point.id, + vector=point.vector or {}, + payload=point.payload or {}, + ) + for point in points + ], + update_filter=update_filter, + )This keeps UUID handling consistent across all local mutation paths.
♻️ Duplicate comments (1)
qdrant_client/local/qdrant_local.py (1)
815-815: Forward kwargs to create_collection.The
**kwargsparameter accepted byrecreate_collection(line 811) is not forwarded tocreate_collection, which limits the ability to pass additional configuration options through the recreate flow.Apply this diff to forward kwargs:
return self.create_collection( - collection_name, vectors_config, sparse_vectors_config, metadata=metadata + collection_name, vectors_config, sparse_vectors_config, metadata=metadata, **kwargs )Based on learnings
🧹 Nitpick comments (3)
qdrant_client/local/qdrant_local.py (1)
746-747: Consider conditional save for efficiency.Currently
self._save()is called unconditionally even whenupdatedisFalse(i.e., when no changes were made). While not incorrect, this is inefficient.Apply this diff to only save when needed:
+ if updated: + self._save() - self._save() return updatedqdrant_client/local/async_qdrant_local.py (2)
503-523: update_filter plumbing for upsert/update_vectors looks correct (kwargs unused)Threading
update_filterthrough asyncupsertandupdate_vectorsinto the underlyingLocalCollectionmatches the sync local implementation and keeps the local API aligned with the remote client.Ruff flags
**kwargsas unused here (and in similar methods). If you want to quiet ARG002 while preserving the interface, consider explicitly discarding or marking them, e.g.:- async def upsert( - self, - collection_name: str, - points: types.Points, - update_filter: Optional[types.Filter] = None, - **kwargs: Any, - ) -> types.UpdateResult: + async def upsert( + self, + collection_name: str, + points: types.Points, + update_filter: Optional[types.Filter] = None, + **kwargs: Any, + ) -> types.UpdateResult: + _ = kwargs # kept for API compatibility(and similarly for
update_vectorsand other methods with unused**kwargs).
796-827: UUID id normalization inupload_collectionis good; consider explicitzip(strict=False)Positives:
- Normalizing
point_idviastr(point_id)when it is auuid.UUIDaligns this path with the rest of the UUID handling logic.- Threading
update_filterthrough tocollection.upsertmatches the rest of the local API.Ruff’s B905 note about
zip()is relevant here:for (point_id, vector, payload) in zip( ids or uuid_generator(), iter(vectors), payload or itertools.cycle([{}]) )Given that
uuid_generator()anditertools.cycle([{}])can be unbounded, you likely intend the current “truncate to the shortest iterable” semantics rather than strict length equality. To both make that intent explicit and silence B905, you can addstrict=False:- for (point_id, vector, payload) in zip( - ids or uuid_generator(), iter(vectors), payload or itertools.cycle([{}]) - ) + for (point_id, vector, payload) in zip( + ids or uuid_generator(), + iter(vectors), + payload or itertools.cycle([{}]), + strict=False, + )Please just verify that your minimum supported Python version includes
zip(..., strict=...); if not, you may prefer to keep the current form and suppress B905 instead.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
qdrant_client/local/async_qdrant_local.py(10 hunks)qdrant_client/local/qdrant_local.py(11 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
qdrant_client/local/async_qdrant_local.py (3)
qdrant_client/local/qdrant_local.py (14)
search(174-202)_get_collection(164-172)upsert(543-552)_default_update_result(588-592)update_vectors(554-563)_save(143-162)closed(70-71)_collection_path(749-753)recreate_collection(805-816)delete_collection(755-770)create_collection(772-803)upload_points(818-826)_upload_points(828-845)uuid_generator(859-861)qdrant_client/local/local_collection.py (5)
search(535-703)upsert(2481-2522)update_vectors(2556-2578)update_sparse_vectors_config(2775-2781)LocalCollection(95-2820)qdrant_client/http/models/models.py (9)
RrfQuery(2476-2477)Filter(914-924)UpdateResult(3345-3347)PointVectors(1991-1993)CreateCollection(503-551)VectorParams(3449-3474)SparseVectorParams(3051-3061)PointStruct(1985-1988)Record(2323-2332)
qdrant_client/local/qdrant_local.py (3)
qdrant_client/http/models/models.py (8)
RrfQuery(2476-2477)Filter(914-924)UpdateResult(3345-3347)PointVectors(1991-1993)VectorParams(3449-3474)SparseVectorParams(3051-3061)PointStruct(1985-1988)Record(2323-2332)qdrant_client/local/async_qdrant_local.py (10)
_get_collection(153-160)upsert(503-512)_default_update_result(548-551)update_vectors(514-523)_save(134-151)recreate_collection(749-760)delete_collection(703-717)create_collection(719-747)upload_points(762-769)_upload_points(771-786)qdrant_client/local/local_collection.py (3)
upsert(2481-2522)update_vectors(2556-2578)update_sparse_vectors_config(2775-2781)
🪛 Ruff (0.14.4)
qdrant_client/local/async_qdrant_local.py
508-508: Unused method argument: kwargs
(ARG002)
519-519: Unused method argument: kwargs
(ARG002)
680-680: Unused method argument: kwargs
(ARG002)
727-727: Unused method argument: kwargs
(ARG002)
730-730: Avoid specifying long messages outside the exception class
(TRY003)
732-732: Avoid specifying long messages outside the exception class
(TRY003)
755-755: Unused method argument: kwargs
(ARG002)
767-767: Unused method argument: kwargs
(ARG002)
822-824: zip() without an explicit strict= parameter
Add explicit value for parameter strict=
(B905)
qdrant_client/local/qdrant_local.py
548-548: Unused method argument: kwargs
(ARG002)
559-559: Unused method argument: kwargs
(ARG002)
730-730: Unused method argument: kwargs
(ARG002)
811-811: Unused method argument: kwargs
(ARG002)
823-823: Unused method argument: kwargs
(ARG002)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: Redirect rules - poetic-froyo-8baba7
- GitHub Check: Header rules - poetic-froyo-8baba7
- GitHub Check: Python 3.11.x on ubuntu-latest test
- GitHub Check: Python 3.13.x on ubuntu-latest test
- GitHub Check: Python 3.9.x on ubuntu-latest test
- GitHub Check: Python 3.10.x on ubuntu-latest test
- GitHub Check: Python 3.12.x on ubuntu-latest test
- GitHub Check: Pages changed - poetic-froyo-8baba7
🔇 Additional comments (13)
qdrant_client/local/qdrant_local.py (8)
6-6: LGTM: UUID import added for proper type handling.The
uuidmodule import is correctly added to support UUID-to-string normalization in_resolve_query_inputandupload_collection.
270-271: LGTM: UUID normalization for consistent ID handling.The UUID-to-string conversion ensures that point IDs are consistently stored and compared as strings, which aligns with the internal storage format.
330-332: LGTM: RrfQuery support added consistently.The pass-through handling for
RrfQueryis appropriate and consistent with howOrderByQueryandFusionQueryare handled, as these query types don't require vector input resolution.
543-563: LGTM: update_filter propagation correctly implemented.Both
upsertandupdate_vectorscorrectly accept and forward theupdate_filterparameter to the underlyingLocalCollectionmethods, matching the async implementation and the updated API surface.The unused
**kwargsparameters flagged by static analysis are intentional for API compatibility with server-mode signatures.
739-744: LGTM: Metadata None handling properly implemented.The metadata update logic now correctly checks if
_collection.config.metadataisNonebefore calling.update(), and initializes withdeepcopy(metadata)when needed. This properly addresses the past review comment about theAttributeErrorrisk.Based on learnings
779-795: LGTM: Metadata properly propagated to LocalCollection.The
metadataparameter is correctly accepted and forwarded toLocalCollectioninitialization withdeepcopy()to prevent mutation issues.
818-845: LGTM: update_filter correctly propagated through upload flow.The
update_filterparameter is properly threaded fromupload_points→_upload_points→collection.upsert, enabling filtered updates in the upload path. The comment on line 825 helpfully clarifies the wait=True equivalence in local mode.
879-890: LGTM: UUID normalization and update_filter propagation.Line 879 correctly normalizes UUID point IDs to strings before constructing
PointStructobjects, ensuring consistency with the internal ID storage format. Theupdate_filteris properly forwarded to the finalcollection.upsertcall on line 889.qdrant_client/local/async_qdrant_local.py (5)
17-17: uuid import correctly added for UUID handlingThe added
import uuidis required for theisinstance(..., uuid.UUID)checks later in this file; import usage is consistent and appropriate.
162-190: Asyncsearchmirrors sync local behaviorThe
searchimplementation delegates directly toLocalCollection.searchwith the expected arguments and mirrors the synchronous local client, so behavior stays consistent between sync and async local modes.
249-253: Good: normalize UUID PointId inputs before lookupNormalizing
vector_inputwhen it is auuid.UUIDtostrbefore checkingcollection.idsaligns with how IDs are stored internally and avoids subtle lookup failures for UUID-based ids.
308-309: Confirm whetherRrfQueryneeds vector/id resolution
RrfQueryis currently passed through without callinginput_into_vectoron any nested parts. Ifrest_models.Rrf/RrfQuerycan containPointIdor other vector-like inputs (similar toNearestQuery,RecommendQuery, etc.), those won’t be resolved here.If by model design
RrfQuerynever embeds such inputs, this is fine; otherwise, you may need to mirror the handling used for other query types.
675-760: Collection metadata updates and creation flow look safeThe new metadata plumbing for collections looks sound:
update_collectionnow handlesmetadatabeingNonevs an existing dict, either updating in place or assigning a deepcopy of the incoming payload.create_collectionandrecreate_collectionacceptmetadata, pass adeepcopy(metadata)intorest_models.CreateCollection, and persist it via_save.This addresses the earlier risk of calling
.update()onNoneand ensures metadata is both optional and correctly persisted for new and existing collections.
Current generation was done based on:
Local mode issue:
#1070