Repository navigation
[issue-9472] [SDK] fix: keep content hashes still shared after a dataset item is deleted - #9473
Conversation
…set item is deleted
| # A deduplicated insert syncs the cache from the backend and skips the content. | ||
| dataset.insert([{"input": {"key": "same"}}]) | ||
| assert capture.request_count == 1 | ||
|
|
||
| dataset.delete([first["id"]]) | ||
| dataset.insert([{"input": {"key": "same"}}]) | ||
|
|
||
| assert capture.request_count == 1, ( | ||
| "The second item still stores the content, so the re-insert must be deduplicated" | ||
| ) |
There was a problem hiding this comment.
Deduplication test omits uploaded record verification
The deduplication test can pass with the wrong payload because it checks only the request count, not which records are uploaded or whether the existing duplicate is omitted. Assert the captured records for the initial uploads, verify reinserting the duplicate uploads nothing, and confirm a genuinely new item is uploaded.
Want Baz to fix this for you? Activate Fixer
Other fix methods
Prompt for AI Agents
Before applying, verify this suggestion against the current code. In
`sdks/python/tests/unit/api_objects/dataset/test_dataset_client.py` around lines
815-824, strengthen
`test_delete__content_hash_shared_by_a_surviving_item__reinsert_is_deduplicated` instead
of relying only on `capture.request_count`. Assert that the initial upload contains both
expected records, that reinserting the shared content produces no uploaded record, and
that a genuinely new item is uploaded, verifying the captured payloads and not just the
number of requests.
| assert dataset.__internal_api__hashes_synced__ is False, ( | ||
| "A failed delete may have applied a batch, so the hash cache must re-sync" | ||
| ) |
There was a problem hiding this comment.
Failure test checks private cache state only
The test can pass even when the next deduplicated insert skips records or uploads the wrong ones because it checks the private __internal_api__hashes_synced__ flag instead of observable behavior. Exercise that insert against the mocked backend and assert the uploaded records, including recovery from the partial deletion.
Want Baz to fix this for you? Activate Fixer
Other fix methods
Prompt for AI Agents
Before applying, verify this suggestion against the current code. In
`sdks/python/tests/unit/api_objects/dataset/test_dataset_items_count.py` around lines
313-315, update
`test_delete__fails_after_earlier_batches_were_deleted__still_invalidates_...` so it
does not assert the private `__internal_api__hashes_synced__` flag as the primary
outcome. After the failed partial delete, exercise a deduplicated insert through the
mocked backend and assert the uploaded records, proving that the cache/hash state
recovers and neither skips nor reuploads the wrong items.
Details
Dataset.delete()dropped a content hash as soon as one of the ids carrying it was deleted. Content hashes are shared by items with identical content (for example after adeduplication=Falseinsert), so deleting one of several same-content items made the next deduplicated insert upload a duplicate. Delete now rebuilds_hashesfrom the surviving_id_to_hashentries; a partially failed delete also marks the hash cache for re-sync, matching the insert failure path.Change checklist
Issues
AI-WATERMARK
AI-WATERMARK: yes
Testing
cc65247: the new dedup test fails withassert 2 == 1(the re-insert after deleting one shared id is uploaded); the partial-failure count test's newhashes_syncedassertion also fails.pytest tests/unit/api_objects/dataset/ -q→ 440 passed.uvx [email protected] check/format --checkon the changed files → clean.uvx [email protected] --config-file sdks/python/pyproject.toml --disable-error-code miscondataset.py→ success.UploadCapture/_StoringCapturefakes; no live backend or network.Documentation
No user-facing documentation; this is an internal dedup-cache invariant.