Skip to content

Conversation

@joein
Copy link
Member

@joein joein commented Nov 14, 2025

No description provided.

@netlify
Copy link

netlify bot commented Nov 14, 2025

Deploy Preview for poetic-froyo-8baba7 ready!

Name Link
🔨 Latest commit a5cddc0
🔍 Latest deploy log https://app.netlify.com/projects/poetic-froyo-8baba7/deploys/691754c90b5a270008ff4574
😎 Deploy Preview https://deploy-preview-1113--poetic-froyo-8baba7.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@coderabbitai
Copy link

coderabbitai bot commented Nov 14, 2025

📝 Walkthrough

Walkthrough

This PR extracts many point-identification, filtering, and geospatial message types into a new common.proto (and generated common_pb2 / common_pb2.pyi / common_pb2_grpc), updates points.proto to import common.proto and use those types, and regenerates related gRPC Python artifacts. collections.proto is updated to add a ReplicatePoints message and to include an optional replicate_points operation in UpdateCollectionClusterSetupRequest. VectorParams in collections.proto gains several optional fields (hnsw_config, quantization_config, on_disk, datatype, multivector_config). Generated .py and .pyi files and qdrant_client/grpc/init.py exports are updated accordingly.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

  • Verify type migrations in qdrant_client/grpc/points_pb2.pyi: all PointId, Filter, Condition, GeoPoint references now point to common_pb2 and container typing is correct
  • Validate ReplicatePoints message and its inclusion in UpdateCollectionClusterSetupRequest oneof (field numbering and oneof behavior)
  • Check VectorParams additions for correct field numbers and types
  • Inspect regenerated DESCRIPTOR/serialized offsets in collections_pb2.py and common_pb2.py for internal consistency
  • Confirm qdrant_client/grpc/init.py re-export doesn't introduce import cycles

Possibly related PRs

Pre-merge checks and finishing touches

❌ Failed checks (2 warnings, 1 inconclusive)
Check name Status Explanation Resolution
Description check ⚠️ Warning No description was provided by the author, making it impossible to assess whether the description relates to the changeset. Add a pull request description explaining the purpose of the grpc updates, key changes made, and why they were necessary.
Docstring Coverage ⚠️ Warning Docstring coverage is 30.26% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
Title check ❓ Inconclusive The title 'new: update grpc' is vague and generic, using a non-descriptive prefix 'new' without clearly indicating what grpc changes are being made or their purpose. Consider using a more descriptive title that explains the main purpose of the grpc update, e.g., 'refactor: consolidate proto messages into common.proto' or 'feat: add ReplicatePoints support to collections'.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch update-models-1.16-3

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b5dc1cc and a5cddc0.

📒 Files selected for processing (1)
  • qdrant_client/grpc/__init__.py (1 hunks)
🧰 Additional context used
🪛 Ruff (0.14.4)
qdrant_client/grpc/__init__.py

3-3: from .common_pb2 import * used; unable to detect undefined names

(F403)

⏰ 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.9.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
🔇 Additional comments (1)
qdrant_client/grpc/__init__.py (1)

3-3: LGTM! No symbol conflicts detected.

The wildcard import is consistent with the existing pattern in this file. Verification confirms no duplicate message definitions across proto files—common.proto defines 20 unique message types (PointId, GeoPoint, Filter, Condition, etc.) with no overlaps to other modules like points.proto or collections.proto.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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/grpc/points_pb2.pyi (1)

1-8: Verify backward compatibility for moved types (PointId, Filter, Condition, GeoPoint)

Type references to points_pb2.PointId, points_pb2.Filter, points_pb2.Condition, and points_pb2.GeoPoint will break. These types are no longer exposed at the points_pb2 module level—they've moved to common_pb2 without re-exports. Code using from qdrant_client.grpc.points_pb2 import PointId will fail.

Confirm whether this breaking change is intentional. If backward compatibility is required, add module-level re-exports (e.g., PointId = common_pb2.PointId) to the runtime .py file and corresponding type aliases to this stub.

🧹 Nitpick comments (3)
qdrant_client/proto/common.proto (1)

1-142: Common filter/geo schema is consistent; only Buf package-directory rule may need attention

The shared PointId, filter/condition tree, geo types, ranges, and value-count messages are well-structured and internally consistent: field numbers are clean, recursive references (FilterCondition, NestedCondition) are valid, and everything needed by other protos (e.g., Filter, GeoPoint, GeoPolygon, ValuesCount) is defined here.

Static analysis (Buf) complains about package qdrant not matching the directory (qdrant_client/proto). That pattern matches your existing protos, so it’s not a new semantic problem, but if Buf is part of CI you may want to either adjust its PACKAGE_DIRECTORY_MATCH config or align paths/packages to avoid new CI noise from this file.

qdrant_client/proto/collections.proto (1)

23-27: VectorParams extensions and MultiVectorConfig look coherent

  • Extending VectorParams with hnsw_config, quantization_config, on_disk, datatype, and multivector_config is consistent with how similar settings are already modeled elsewhere (e.g., collection-level configs), and field numbers 3–7 avoid conflicts.
  • Adding on_disk to VectorParamsDiff (tag 3) enables toggling on-disk vector storage at the per-vector-config level, which mirrors the base message.
  • The new MultiVectorComparator enum and MultiVectorConfig message cleanly model multi-vector search behavior and are wired via VectorParams.multivector_config.

One potential follow-up: if you intend datatype and multivector_config to be mutable via diffs as well, consider extending VectorParamsDiff in a future change to include them; otherwise the current schema is internally consistent.

Also applies to: 31-33, 72-78

qdrant_client/grpc/common_pb2.py (1)

23-25: Ruff E712 on _USE_C_DESCRIPTORS should be handled via lint config, not by editing generated code

The if _descriptor._USE_C_DESCRIPTORS == False: pattern is produced by the protobuf generator. I’d avoid touching this file manually and instead configure Ruff to ignore _pb2.py files (or E712 for this path) so future regenerations don’t reintroduce the warning.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 27acfd0 and b5dc1cc.

📒 Files selected for processing (9)
  • qdrant_client/grpc/collections_pb2.py (2 hunks)
  • qdrant_client/grpc/collections_pb2.pyi (6 hunks)
  • qdrant_client/grpc/common_pb2.py (1 hunks)
  • qdrant_client/grpc/common_pb2.pyi (1 hunks)
  • qdrant_client/grpc/common_pb2_grpc.py (1 hunks)
  • qdrant_client/grpc/points_pb2.pyi (51 hunks)
  • qdrant_client/proto/collections.proto (4 hunks)
  • qdrant_client/proto/common.proto (1 hunks)
  • qdrant_client/proto/points.proto (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
qdrant_client/grpc/collections_pb2.pyi (3)
qdrant_client/http/models/models.py (2)
  • ReplicatePoints (2400-2403)
  • Filter (914-924)
qdrant_client/grpc/common_pb2.pyi (3)
  • filter (127-127)
  • filter (216-217)
  • Filter (59-88)
qdrant_client/grpc/points_pb2.pyi (61)
  • filter (1479-1480)
  • filter (1635-1636)
  • filter (1780-1781)
  • filter (1895-1896)
  • filter (2043-2044)
  • filter (2217-2218)
  • filter (2327-2328)
  • filter (2879-2880)
  • filter (2951-2952)
  • filter (3092-3093)
  • filter (3193-3194)
  • filter (3296-3297)
  • filter (4520-4520)
  • HasField (244-244)
  • HasField (282-282)
  • HasField (323-323)
  • HasField (344-344)
  • HasField (366-366)
  • HasField (387-387)
  • HasField (440-440)
  • HasField (487-487)
  • HasField (584-584)
  • HasField (611-611)
  • HasField (656-656)
  • HasField (699-699)
  • HasField (750-750)
  • HasField (798-798)
  • HasField (828-828)
  • HasField (868-868)
  • ClearField (225-225)
  • ClearField (245-245)
  • ClearField (261-261)
  • ClearField (283-283)
  • ClearField (302-302)
  • ClearField (324-324)
  • ClearField (345-345)
  • ClearField (367-367)
  • ClearField (388-388)
  • ClearField (441-441)
  • ClearField (488-488)
  • ClearField (509-509)
  • ClearField (528-528)
  • ClearField (543-543)
  • ClearField (585-585)
  • ClearField (612-612)
  • WhichOneof (246-246)
  • WhichOneof (443-443)
  • WhichOneof (445-445)
  • WhichOneof (447-447)
  • WhichOneof (490-490)
  • WhichOneof (492-492)
  • WhichOneof (494-494)
  • WhichOneof (586-586)
  • WhichOneof (613-613)
  • WhichOneof (659-659)
  • WhichOneof (661-661)
  • WhichOneof (663-663)
  • WhichOneof (665-665)
  • WhichOneof (702-702)
  • WhichOneof (704-704)
  • WhichOneof (706-706)
qdrant_client/grpc/points_pb2.pyi (1)
qdrant_client/grpc/common_pb2.pyi (6)
  • PointId (21-38)
  • Filter (59-88)
  • filter (127-127)
  • filter (216-217)
  • Condition (110-147)
  • GeoPoint (42-55)
qdrant_client/grpc/common_pb2.pyi (2)
qdrant_client/grpc/collections_pb2.pyi (2)
  • collections (723-723)
  • filter (2801-2802)
qdrant_client/grpc/points_pb2.pyi (22)
  • filter (1479-1480)
  • filter (1635-1636)
  • filter (1780-1781)
  • filter (1895-1896)
  • filter (2043-2044)
  • filter (2217-2218)
  • filter (2327-2328)
  • filter (2879-2880)
  • filter (2951-2952)
  • filter (3092-3093)
  • filter (3193-3194)
  • filter (3296-3297)
  • filter (4520-4520)
  • points (636-636)
  • points (682-683)
  • points (777-778)
  • points (1011-1012)
  • points (3426-3426)
  • points (3584-3585)
  • points (3638-3639)
  • points (3659-3660)
  • points (4518-4518)
🪛 Buf (1.59.0)
qdrant_client/proto/common.proto

2-2: Files with package "qdrant" must be within a directory "qdrant" relative to root but were in directory "qdrant_client/proto".

(PACKAGE_DIRECTORY_MATCH)

🪛 Ruff (0.14.4)
qdrant_client/grpc/common_pb2.py

23-23: Avoid equality comparisons to False; use not _descriptor._USE_C_DESCRIPTORS: for false checks

Replace with not _descriptor._USE_C_DESCRIPTORS

(E712)

⏰ 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 (14)
qdrant_client/grpc/common_pb2.pyi (1)

1-561: Stubs correctly mirror the common proto schema

The type stubs line up with the new common.proto messages (fields, oneofs, and container types), including presence semantics for optional fields and the geo/filter tree. I don’t see mismatches or typing issues here.

qdrant_client/grpc/common_pb2_grpc.py (1)

1-4: Minimal _grpc module is appropriate for a proto with no services

Given that common.proto defines only messages and no services, this minimal generated module is expected and safe; it just ensures imports of qdrant_client.grpc.common_pb2_grpc work.

qdrant_client/grpc/collections_pb2.py (2)

15-16: New common_pb2 import correctly wires in shared types

Adding from . import common_pb2 as common__pb2 is necessary so that the collections descriptors can reference shared messages (e.g., Filter) from common.proto. This aligns with the new proto imports and looks correct.


19-268: Regenerated descriptors align with updated collections.proto

The serialized DESCRIPTOR blob now imports common.proto and includes the extended VectorParams/VectorParamsDiff, SparseIndexConfig.datatype, the new ReplicatePoints message, and the replicate_points branch of UpdateCollectionClusterSetupRequest.operation. The _globals[...] _serialized_start/_end updates are consistent with those structural changes. No manual fixes needed here.

qdrant_client/proto/collections.proto (3)

7-7: Importing common.proto to use shared Filter/Point primitives is appropriate

The new import "common.proto"; and the added section header comments make sense: collections now rely on shared query primitives (e.g., Filter) defined in common.proto, and the comments help delineate the collection config section without changing behavior.

Also applies to: 16-18


217-230: SparseIndexConfig.datatype is consistent with the new Datatype enum

Adding optional Datatype datatype = 3; to SparseIndexConfig reuses the same Datatype enum introduced for vectors, which keeps type choices coherent across dense and sparse indexes. Tag 3 is unused previously, so compatibility is preserved.


732-736: ReplicatePoints and new cluster operation integrate cleanly

  • ReplicatePoints with from_shard_key, to_shard_key, and optional Filter filter provides a clear, targeted way to replicate only matching points between shard keys.
  • The new ReplicatePoints replicate_points = 10; branch in UpdateCollectionClusterSetupRequest.operation uses a fresh tag within the oneof and does not collide with existing operations.
  • Filter here comes from common.proto (imported at the top), matching the shared filter schema used by other APIs.

The semantics and wiring into the cluster setup request look sound.

Also applies to: 772-773

qdrant_client/grpc/collections_pb2.pyi (3)

7-7: New common_pb2 import is appropriate for shared filter types

Bringing in common_pb2 here is consistent with existing imports like json_with_int_pb2 and is required for the new ReplicatePoints.filter: common_pb2.Filter typing below. No issues from a typing or packaging perspective.


342-346: VectorParams header comment is a safe, non‑behavioral improvement

The added comment block around VectorParams simply clarifies that this section pertains to collection config; it doesn’t affect the generated typing surface and is fine to keep.


2888-2942: ReplicatePoints message and cluster setup extension look consistent and type‑sound

  • New ReplicatePoints mirrors the HTTP model (from_shard_key, to_shard_key, optional filter) and reuses the existing ShardKey type plus the shared common_pb2.Filter, which keeps HTTP and gRPC models aligned.
  • The corresponding global___ReplicatePoints alias matches the pattern used throughout this stub.
  • UpdateCollectionClusterSetupRequest:
    • Adds REPLICATE_POINTS_FIELD_NUMBER and a replicate_points property/ctor parameter of type global___ReplicatePoints.
    • Extends HasField, ClearField, and the "operation" oneof to include "replicate_points", so this new operation participates in the same mutually exclusive semantics as the existing cluster operations.

Overall this is a clean extension of the API surface and should integrate smoothly with existing code that dispatches on the "operation" oneof.

qdrant_client/proto/points.proto (1)

6-8: Importing common.proto correctly centralizes shared primitives

Adding import "common.proto"; is necessary now that PointId, Filter, Condition, GeoPoint, and related messages are defined in the shared proto. Since both files use package qdrant;, all existing references (e.g., Filter filter = 3;, PointId id = 1;) continue to resolve to the same fully qualified types, preserving wire compatibility while avoiding duplication.

qdrant_client/grpc/points_pb2.pyi (3)

5-8: Importing common_pb2 is correct and consistent with existing generated imports

The new import common_pb2 follows the same pattern as collections_pb2/json_with_int_pb2 and cleanly supports the subsequent type references.


547-583: PointId usage correctly migrated to common_pb2.PointId across request/response shapes

All updated PointId-bearing fields (VectorInput.id, PointVectors.id, GetPoints.ids, ScrollPoints.offset, Recommend*/SearchMatrixPair/SearchMatrixOffsets, ScrollResponse.next_page_offset, RetrievedPoint.id, PointsIdsList.ids, PointStruct.id, ScoredPoint.id) consistently reference common_pb2.PointId in both property types and constructor parameters, matching the centralized definition in common_pb2.pyi. I don’t see any remaining references to a local PointId/global___PointId in this stub.

Also applies to: 811-827, 710-749, 1768-1817, 3366-3379, 3402-3411, 4161-4173, 4200-4245, 4533-4543, 4548-4588, 3827-3883


623-657: Filter/Condition/GeoPoint centralization to common_pb2 looks consistent

All Filter-typed fields and constructor params (update operations, search/recommend/query/scroll/facet/matrix selectors, PointsSelector.filter) now consistently use common_pb2.Filter. Likewise, Expression.condition now uses common_pb2.Condition and GeoDistance.origin uses common_pb2.GeoPoint. These line up with the corresponding message definitions in common_pb2.pyi, and I don’t see any leftover references to the old local types in this stub.

Also applies to: 763-799, 1462-1525, 1613-1688, 1768-1819, 1865-1956, 2011-2111, 2191-2263, 2315-2358, 2862-2904, 2922-2999, 3061-3147, 3177-3232, 3282-3325, 3416-3441, 3577-3600, 4507-4528, 4533-4543, 2486-2582, 2589-2601

@joein joein requested a review from generall November 14, 2025 16:17
@generall generall merged commit 14f461b into dev Nov 14, 2025
14 checks passed
generall pushed a commit that referenced this pull request Nov 14, 2025
* new: update grpc

* fix: make common_pb2 available under grpc
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants