fix: fix set payload in batch update points, fix operation conversion…#977
Conversation
✅ Deploy Preview for poetic-froyo-8baba7 ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
📝 WalkthroughWalkthroughThe changes introduce support for an optional Note ⚡️ AI Code Reviews for VS Code, Cursor, WindsurfCodeRabbit now has a plugin for VS Code, Cursor and Windsurf. This brings AI code reviews directly in the code editor. Each commit is reviewed immediately, finding bugs before the PR is raised. Seamless context handoff to your AI code agent ensures that you can easily incorporate review feedback. Note ⚡️ Faster reviews with cachingCodeRabbit now supports caching for code and dependencies, helping speed up reviews. This means quicker feedback, reduced wait times, and a smoother review experience overall. Cached data is encrypted and stored securely. This feature will be automatically enabled for all accounts on May 16th. To opt out, configure 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🧰 Additional context used🧠 Learnings (1)tests/congruence_tests/test_payload.py (1)⏰ Context from checks skipped due to timeout of 90000ms (8)
🔇 Additional comments (8)
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
qdrant_client/local/local_collection.py (1)
2355-2361: Cast multivector updates tofloat32to avoid implicit up-castingSame dtype concern as above – after the first
float64insert the entire list for that vector name will be promoted.- vector_np = np.array(vector) + vector_np = np.asarray(vector, dtype=np.float32)Optionally extract the (now duplicated) “normalise-if-cosine” logic into a small helper to DRY the three call-sites in this class.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
qdrant_client/conversions/conversion.py(4 hunks)qdrant_client/local/local_collection.py(2 hunks)tests/congruence_tests/test_payload.py(11 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
qdrant_client/conversions/conversion.py (5)
qdrant_client/local/qdrant_local.py (1)
set_payload(826-836)qdrant_client/local/local_collection.py (1)
set_payload(2451-2475)qdrant_client/qdrant_client.py (1)
set_payload(1855-1949)qdrant_client/qdrant_remote.py (1)
set_payload(2245-2295)qdrant_client/client_base.py (1)
set_payload(303-311)
⏰ Context from checks skipped due to timeout of 90000ms (5)
- 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)
qdrant_client/conversions/conversion.py (4)
1912-1912: Good addition of key parameter support in GrpcToRest conversionThe change correctly adds support for converting the optional
keyfield from gRPC to REST for theset_payloadoperation, ensuring nested field path specifications are preserved through the conversion layer.The implementation properly checks if the field exists using
HasField("key")before attempting to access it, which is a good practice for dealing with optional protobuf fields.
1938-1938: Good addition of key parameter support in GrpcToRest conversion for overwrite_payloadThe change correctly adds support for converting the optional
keyfield from gRPC to REST for theoverwrite_payloadoperation, maintaining consistency with theset_payloadoperation implementation.This ensures that nested field path specifications are properly preserved when converting between representations.
4216-4216: Good addition of key parameter support in RestToGrpc conversionThe change properly adds support for converting the optional
keyfield from REST to gRPC for theset_payloadoperation, ensuring bidirectional compatibility with the GrpcToRest conversion added earlier in the file.
4240-4240: Good addition of key parameter support in RestToGrpc conversion for overwrite_payloadThe change properly adds support for converting the optional
keyfield from REST to gRPC for theoverwrite_payloadoperation, maintaining consistency with theset_payloadoperation implementation.These changes complete the bidirectional conversion support for the
keyparameter, ensuring that nested field path specifications work correctly across REST and gRPC interfaces.qdrant_client/local/local_collection.py (1)
2541-2544: HandleNoneselector & keep behaviour symmetric with REST/gRPC
points_selector = update_op.set_payload.points or update_op.set_payload.filter
will beNoneif both fields are absent (allowed by the schema).
self.set_payload()then receivesselector=None, but its signature expects one of the supported selector types and will raise an opaqueAttributeError.Consider an explicit check:
points_selector = update_op.set_payload.points or update_op.set_payload.filter - self.set_payload( - update_op.set_payload.payload, points_selector, update_op.set_payload.key - ) + if points_selector is None: + raise ValueError("SetPayloadOperation must provide either `points` or `filter`") + self.set_payload( + update_op.set_payload.payload, points_selector, update_op.set_payload.key + )This yields an early, user-friendly error message and keeps behaviour aligned between local & remote clients.
| new_payload = {"text_data": {"some_key": "overwritten_value"}} | ||
| overwrite_payload_op_points_no_key = models.OverwritePayloadOperation( | ||
| overwrite_payload=models.SetPayload(points=[1], payload=new_payload) | ||
| ) | ||
|
|
||
| local_client.batch_update_points( | ||
| collection_name=COLLECTION_NAME, update_operations=[overwrite_payload_op_points_no_key] |
There was a problem hiding this comment.
🛠️ Refactor suggestion
OverwritePayloadOperation wraps OverwritePayload, not SetPayload
Using models.SetPayload here still works only because both dataclasses share field names, but it breaks type-level guarantees and may stop compiling once the models diverge.
- overwrite_payload_op_points_no_key = models.OverwritePayloadOperation(
- overwrite_payload=models.SetPayload(points=[1], payload=new_payload)
+ overwrite_payload_op_points_no_key = models.OverwritePayloadOperation(
+ overwrite_payload=models.OverwritePayload(points=[1], payload=new_payload)
)📝 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.
| new_payload = {"text_data": {"some_key": "overwritten_value"}} | |
| overwrite_payload_op_points_no_key = models.OverwritePayloadOperation( | |
| overwrite_payload=models.SetPayload(points=[1], payload=new_payload) | |
| ) | |
| local_client.batch_update_points( | |
| collection_name=COLLECTION_NAME, update_operations=[overwrite_payload_op_points_no_key] | |
| new_payload = {"text_data": {"some_key": "overwritten_value"}} | |
| overwrite_payload_op_points_no_key = models.OverwritePayloadOperation( | |
| overwrite_payload=models.OverwritePayload(points=[1], payload=new_payload) | |
| ) | |
| local_client.batch_update_points( | |
| collection_name=COLLECTION_NAME, update_operations=[overwrite_payload_op_points_no_key] | |
| ) |
There was a problem hiding this comment.
there is no models.OverwritePayload
models.OverwritePayloadOperation accepts models.SetPayload
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# 1. Look for OverwritePayload class definition
echo "Searching for OverwritePayload class definition..."
rg "class OverwritePayload" --type py
# 2. Look for OverwritePayloadOperation class definition
echo -e "\nSearching for OverwritePayloadOperation class definition..."
rg "class OverwritePayloadOperation" --type py -A 15
# 3. Check imports and usages to see the structure relationship
echo -e "\nChecking imports and usages in http/models..."
fd "models\.py" --type f | xargs cat | grep -A 50 "class OverwritePayloadOperation"
Length of output: 3147
@joein You're absolutely right. After checking the codebase, I can confirm:
- There is no
models.OverwritePayloadclass defined anywhere in the code - The
OverwritePayloadOperationclass is defined to take aSetPayloadobject:
class OverwritePayloadOperation(BaseModel, extra="forbid"):
overwrite_payload: "SetPayload" = Field(..., description="")Your original code using models.SetPayload inside OverwritePayloadOperation is correct. Sorry for the confusion in my earlier review comment.
🧬 Code Graph Analysis Results
Relevant snippets for understanding tests/congruence_tests/test_payload.py and the user comment about models.OverwritePayload and models.OverwritePayloadOperation:
From tests/congruence_tests/test_payload.py (lines 350-400 approx)
- The test
test_set_or_overwrite_payload_operationusesmodels.SetPayloadOperationandmodels.OverwritePayloadOperation. - It shows usage of
models.SetPayloadinsideSetPayloadOperationandOverwritePayloadOperation. - Specifically,
OverwritePayloadOperationis constructed withoverwrite_payload=models.SetPayload(...). - This indicates that
OverwritePayloadOperationexpects aSetPayloadinstance, not a separateOverwritePayloadmodel.
From qdrant_client/http/models/models.py
- At lines 1628-1629:
class OverwritePayloadOperation(BaseModel, extra="forbid"):
overwrite_payload: "SetPayload" = Field(..., description="")-
This shows that
OverwritePayloadOperationcontains a fieldoverwrite_payloadof typeSetPayload. -
There is no separate
OverwritePayloadmodel; the overwrite operation usesSetPayloadas its payload container. -
At lines 2462-2481:
class SetPayload(BaseModel, extra="forbid"):
"""
This data structure is used in API interface and applied across multiple shards
"""
payload: "Payload" = Field(..., description="This data structure is used in API interface and applied across multiple shards")
points: Optional[List["ExtendedPointId"]] = Field(default=None, description="Assigns payload to each point in this list")
filter: Optional["Filter"] = Field(default=None, description="Assigns payload to each point that satisfy this filter condition")
shard_key: Optional["ShardKeySelector"] = Field(default=None, description="This data structure is used in API interface and applied across multiple shards")
key: Optional[str] = Field(default=None, description="Assigns payload to each point that satisfy this path of property")SetPayloadis a model representing the payload to set, with optional points or filter selectors and an optional key.
From qdrant_client/qdrant_client.py (lines 2111-2152):
- Method
batch_update_pointsaccepts a list ofupdate_operationswhich can includeSetPayloadOperationandOverwritePayloadOperation. - The client calls
_client.batch_update_pointswith these operations.
From qdrant_client/local/local_collection.py (lines 2526-2565):
- Method
batch_update_pointsprocesses update operations:- For
SetPayloadOperation, it callsset_payloadwith the containedset_payloadfield. - For
OverwritePayloadOperation, it callsoverwrite_payloadwith the containedoverwrite_payloadfield.
- For
- Both
set_payloadandoverwrite_payloadexpect aSetPayloadinstance. - This confirms that
OverwritePayloadOperationusesSetPayloadas its payload container.
From qdrant_client/qdrant_remote.py (lines 2297-2346):
- Method
overwrite_payloadin remote client acceptspayload: types.Payloadandpoints: types.PointsSelector. - It constructs a
models.SetPayloadinstance internally for the REST API call. - This matches the usage in the test where
OverwritePayloadOperationcontains aSetPayload.
Summary of relevance to user comment:
- The user states: "there is no models.OverwritePayload; models.OverwritePayloadOperation accepts models.SetPayload".
- The code confirms this:
- There is no separate
OverwritePayloadmodel. OverwritePayloadOperationhas a fieldoverwrite_payloadof typeSetPayload.- The test code uses
models.SetPayloadinsideOverwritePayloadOperation.
- There is no separate
- This is consistent with the design in the models and client code.
Included relevant snippets:
1. tests/congruence_tests/test_payload.py (lines ~350-400)
@pytest.mark.parametrize("prefer_grpc", [True, False])
def test_set_or_overwrite_payload_operation(prefer_grpc):
local_client: QdrantClient = init_local()
initialize_fixture_collection(local_client)
remote_client: QdrantClient = init_remote(prefer_grpc=prefer_grpc)
initialize_fixture_collection(remote_client)
num_vectors = 10
upload(local_client, remote_client, num_vectors)
new_payload = {"text_data": "new_value"}
set_payload_op_points_no_key = models.SetPayloadOperation(
set_payload=models.SetPayload(points=[1], payload=new_payload)
)
local_client.batch_update_points(
collection_name=COLLECTION_NAME, update_operations=[set_payload_op_points_no_key]
)
remote_client.batch_update_points(
collection_name=COLLECTION_NAME, update_operations=[set_payload_op_points_no_key]
)
compare_collections(local_client, remote_client, num_vectors)
new_nested_payload = {"nested_data": "new_nested_value"}
set_payload_op_filter_key = models.SetPayloadOperation(
set_payload=models.SetPayload(
filter=models.Filter(must=[models.HasIdCondition(has_id=[1])]),
payload=new_nested_payload,
key="text_data",
)
)
local_client.batch_update_points(
collection_name=COLLECTION_NAME, update_operations=[set_payload_op_filter_key]
)
remote_client.batch_update_points(
collection_name=COLLECTION_NAME, update_operations=[set_payload_op_filter_key]
)
compare_collections(local_client, remote_client, num_vectors)
new_payload = {"text_data": {"some_key": "overwritten_value"}}
overwrite_payload_op_points_no_key = models.OverwritePayloadOperation(
overwrite_payload=models.SetPayload(points=[1], payload=new_payload)
)
local_client.batch_update_points(
collection_name=COLLECTION_NAME, update_operations=[overwrite_payload_op_points_no_key]
)
remote_client.batch_update_points(
collection_name=COLLECTION_NAME, update_operations=[overwrite_payload_op_points_no_key]
)
compare_collections(local_client, remote_client, num_vectors)2. qdrant_client/http/models/models.py (lines 1628-1630 and 2462-2481)
class OverwritePayloadOperation(BaseModel, extra="forbid"):
overwrite_payload: "SetPayload" = Field(..., description="")
class SetPayload(BaseModel, extra="forbid"):
"""
This data structure is used in API interface and applied across multiple shards
"""
payload: "Payload" = Field(
..., description="This data structure is used in API interface and applied across multiple shards"
)
points: Optional[List["ExtendedPointId"]] = Field(
default=None, description="Assigns payload to each point in this list"
)
filter: Optional["Filter"] = Field(
default=None, description="Assigns payload to each point that satisfy this filter condition"
)
shard_key: Optional["ShardKeySelector"] = Field(
default=None, description="This data structure is used in API interface and applied across multiple shards"
)
key: Optional[str] = Field(
default=None, description="Assigns payload to each point that satisfy this path of property"
)3. qdrant_client/local/local_collection.py (lines 2526-2565)
def batch_update_points(
self,
update_operations: Sequence[types.UpdateOperation],
) -> None:
for update_op in update_operations:
if isinstance(update_op, models.UpsertOperation):
if isinstance(update_op.upsert, models.PointsBatch):
self.upsert(update_op.upsert.batch)
elif isinstance(update_op.upsert, models.PointsList):
self.upsert(update_op.upsert.points)
else:
raise ValueError(f"Unsupported upsert type: {type(update_op.upsert)}")
elif isinstance(update_op, models.DeleteOperation):
self.delete(update_op.delete)
elif isinstance(update_op, models.SetPayloadOperation):
points_selector = update_op.set_payload.points or update_op.set_payload.filter
self.set_payload(
update_op.set_payload.payload, points_selector, update_op.set_payload.key
)
elif isinstance(update_op, models.OverwritePayloadOperation):
points_selector = (
update_op.overwrite_payload.points or update_op.overwrite_payload.filter
)
self.overwrite_payload(update_op.overwrite_payload.payload, points_selector)
elif isinstance(update_op, models.DeletePayloadOperation):
points_selector = (
update_op.delete_payload.points or update_op.delete_payload.filter
)
self.delete_payload(update_op.delete_payload.keys, points_selector)
elif isinstance(update_op, models.ClearPayloadOperation):
self.clear_payload(update_op.clear_payload)
elif isinstance(update_op, models.UpdateVectorsOperation):
self.update_vectors(update_op.update_vectors.points)
elif isinstance(update_op, models.DeleteVectorsOperation):
points_selector = (
update_op.delete_vectors.points or update_op.delete_vectors.filter
)
self.delete_vectors(update_op.delete_vectors.vector, points_selector)
else:
raise ValueError(f"Unsupported update operation: {type(update_op)}")4. qdrant_client/qdrant_remote.py (lines 2297-2346)
def overwrite_payload(
self,
collection_name: str,
payload: types.Payload,
points: types.PointsSelector,
wait: bool = True,
ordering: Optional[types.WriteOrdering] = None,
shard_key_selector: Optional[types.ShardKeySelector] = None,
**kwargs: Any,
) -> types.UpdateResult:
if self._prefer_grpc:
points_selector, opt_shard_key_selector = self._try_argument_to_grpc_selector(points)
shard_key_selector = shard_key_selector or opt_shard_key_selector
if isinstance(ordering, models.WriteOrdering):
ordering = RestToGrpc.convert_write_ordering(ordering)
if isinstance(shard_key_selector, get_args_subscribed(models.ShardKeySelector)):
shard_key_selector = RestToGrpc.convert_shard_key_selector(shard_key_selector)
return GrpcToRest.convert_update_result(
self.grpc_points.OverwritePayload(
grpc.SetPayloadPoints(
collection_name=collection_name,
wait=wait,
payload=RestToGrpc.convert_payload(payload),
points_selector=points_selector,
ordering=ordering,
shard_key_selector=shard_key_selector,
),
timeout=self._timeout,
).result
)
else:
_points, _filter = self._try_argument_to_rest_points_and_filter(points)
result: Optional[types.UpdateResult] = (
self.openapi_client.points_api.overwrite_payload(
collection_name=collection_name,
wait=wait,
ordering=ordering,
set_payload=models.SetPayload(
payload=payload,
points=_points,
filter=_filter,
shard_key=shard_key_selector,
),
).result
)
assert result is not None, "Overwrite payload returned None"
return resultThese snippets collectively confirm that:
models.OverwritePayloadOperationcontains aSetPayloadinstance namedoverwrite_payload.- There is no separate
OverwritePayloadmodel. - The test code uses
models.SetPayloadinsideOverwritePayloadOperation. - The client code and local collection code expect and handle
SetPayloadfor overwrite operations.
This directly addresses the user comment about the absence of models.OverwritePayload and the acceptance of models.SetPayload by models.OverwritePayloadOperation.
✏️ Learnings added
Learnt from: joein
PR: qdrant/qdrant-client#977
File: tests/congruence_tests/test_payload.py:645-651
Timestamp: 2025-05-14T20:18:46.248Z
Learning: In the Qdrant client, there is no separate `models.OverwritePayload` class. The `models.OverwritePayloadOperation` class is designed to accept a `models.SetPayload` object as its `overwrite_payload` parameter.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
coszio
left a comment
coszio
left a comment
There was a problem hiding this comment.
I'd like to ask to add a few comments in the tests to identify sections of the test easier, they seem to have multiple steps of setup->assertion. Having a description of each should make it easier to maintain
| payload = {} | ||
| set_payload(payload, new_payload, key) | ||
| # payload = {} | ||
| # set_payload(payload, new_payload, key) # todo: uncomnment when https://github.com/qdrant/qdrant/issues/6449 is resolved |
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)
tests/congruence_tests/test_payload.py(9 hunks)
🧰 Additional context used
🧠 Learnings (1)
tests/congruence_tests/test_payload.py (1)
Learnt from: joein
PR: qdrant/qdrant-client#977
File: tests/congruence_tests/test_payload.py:645-651
Timestamp: 2025-05-14T20:18:46.248Z
Learning: In the Qdrant client, there is no separate `models.OverwritePayload` class. The `models.OverwritePayloadOperation` class is designed to accept a `models.SetPayload` object as its `overwrite_payload` parameter.
🧬 Code Graph Analysis (1)
tests/congruence_tests/test_payload.py (7)
tests/congruence_tests/test_common.py (4)
initialize_fixture_collection(93-109)init_local(385-390)init_remote(393-395)compare_collections(175-197)tests/congruence_tests/test_discovery.py (1)
local_client(38-42)tests/congruence_tests/conftest.py (2)
local_client(13-17)remote_client(21-25)qdrant_client/qdrant_client.py (11)
QdrantClient(28-3167)set_payload(1855-1949)batch_update_points(2111-2152)upsert(1565-1642)delete(1808-1853)count(1473-1512)delete_payload(2018-2065)clear_payload(2067-2109)update_vectors(1644-1698)delete_vectors(1700-1748)overwrite_payload(1951-2016)qdrant_client/local/local_collection.py (10)
set_payload(2453-2477)batch_update_points(2528-2567)upsert(2298-2334)delete(2431-2441)count(1903-1906)delete_payload(2495-2511)clear_payload(2513-2526)update_vectors(2366-2376)delete_vectors(2378-2393)overwrite_payload(2479-2493)qdrant_client/qdrant_remote.py (10)
set_payload(2245-2295)batch_update_points(2442-2480)upsert(1840-1918)delete(2199-2243)count(1738-1779)delete_payload(2348-2394)clear_payload(2396-2440)update_vectors(1920-1959)delete_vectors(1961-2011)overwrite_payload(2297-2346)qdrant_client/http/exceptions.py (1)
UnexpectedResponse(13-41)
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: Python 3.12.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.9.x on ubuntu-latest test
- GitHub Check: Python 3.10.x on ubuntu-latest test
🔇 Additional comments (9)
tests/congruence_tests/test_payload.py (9)
5-5: Import for improved exception handling.Adding the
grpcimport enables catching both REST-basedUnexpectedResponseand gRPC-specificRpcErrorexceptions, enhancing error handling across both client communication methods.
34-40: Good parameterization of test to support both protocols.Parameterizing the test with
prefer_grpcand using theinitialize_fixture_collectionhelper improves test structure and ensures coverage across both gRPC and REST communication protocols.
417-417: Improved error handling for both protocols.Using a tuple to catch both
UnexpectedResponseandgrpc.RpcErrorhandles exceptions from both REST and gRPC clients, ensuring consistent test behavior regardless of the protocol being used.
310-382: Comprehensive tests for nested payload updates.The added test cases thoroughly validate key payload update scenarios:
- Updating values in nested arrays
- Modifying non-existing array elements
- Adding fields to dictionaries in arrays
- Adding fields to empty payloads
- Replacing arrays with dictionaries and vice versa
- Deep nesting operations
These tests ensure that the
keyparameter functionality works correctly across various complex payload structures.
452-487: Verified batch update with upsert operations.This new test validates
batch_update_pointsfunctionality with bothPointsBatchandPointsListupsert operations, ensuring consistent behavior between local and remote clients across both protocols.
509-569: Complete validation of payload modification operations.This test comprehensively verifies deletion and clearing of payloads via the batch API:
- Delete payload keys for specific points
- Delete payload keys using a filter
- Clear entire payload for points
The test ensures consistency between local and remote clients with each operation.
571-620: Thorough validation of vector operations.This test covers updating and deleting vectors through the batch API:
- Updating vector values for specific points
- Deleting vectors specified by point IDs
- Deleting vectors using a filter
These tests are essential for verifying the vector normalization process fixed in this PR.
622-672: Well-designed test for payload operations with key support.This test comprehensively validates the core fix in this PR - enabling key-based partial updates in batch operations:
- Setting payload for specific points without a key
- Setting nested payload with a key parameter
- Overwriting payload completely
The test confirms consistent behavior between local and remote clients across both protocols.
661-664: Correct usage of models.SetPayload with OverwritePayloadOperation.The implementation correctly uses
models.SetPayloadas the parameter foroverwrite_payloadin theOverwritePayloadOperation. This aligns with the codebase structure where there is no separatemodels.OverwritePayloadclass.
| set_payload( | ||
| payload, new_payload, key | ||
| ) # todo: uncomment when https://github.com/qdrant/qdrant/issues/6449 is resolved |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Test marked with TODO pending issue resolution.
This test case for setting a key on an empty payload references a known issue (GitHub #6449). Consider adding a comment explaining the expected behavior once the issue is resolved.
Can you verify the current status of this issue by checking:
🏁 Script executed:
#!/bin/bash
# Check the status of the referenced GitHub issue
gh issue view https://github.com/qdrant/qdrant/issues/6449 --json title,state,body,comments | jq '{title: .title, state: .state, comments: .comments | length}'Length of output: 304
Add expected behavior note for TODO on issue #6449
The GitHub issue (qdrant/qdrant#6449) is still open, so the # todo marker remains valid. To aid future maintainers, please update the inline comment to spell out what the test should do once that issue is resolved.
• File: tests/congruence_tests/test_payload.py
• Lines: 331–333
Suggested update:
- set_payload(
- payload, new_payload, key
- ) # todo: uncomment when https://github.com/qdrant/qdrant/issues/6449 is resolved
+ set_payload(
+ payload, new_payload, key
+ ) # TODO: uncomment when https://github.com/qdrant/qdrant/issues/6449 is resolved
+ # Expected behavior: setting a key on an empty payload should add the key with the given value📝 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.
| set_payload( | |
| payload, new_payload, key | |
| ) # todo: uncomment when https://github.com/qdrant/qdrant/issues/6449 is resolved | |
| set_payload( | |
| payload, new_payload, key | |
| ) # TODO: uncomment when https://github.com/qdrant/qdrant/issues/6449 is resolved | |
| # Expected behavior: setting a key on an empty payload should add the key with the given value |
#977) * fix: fix set payload in batch update points, fix operation conversions, add tests * new: add some checks * add comments to test cases * fix: return comment * tests: uncomment test due to a fix in core
2 participants
#974