fix: delete async upsert_chunks/delete_file; repoint E2E harness to sync - #3181
Open
yastman wants to merge 1 commit into
Open
fix: delete async upsert_chunks/delete_file; repoint E2E harness to sync#3181yastman wants to merge 1 commit into
yastman wants to merge 1 commit into
Conversation
…s to sync The async upsert_chunks and delete_file methods diverged from the sync path, lacking the text-size guard and poison/empty-vector skip. Their only caller was the E2E test harness, making deletion the cleaner fix. Changes: - Delete async upsert_chunks, delete_file and their embed helpers (_embed_documents_local, _embed_sparse, _embed_colbert) from qdrant_writer.py - Remove unused threading import, _dense_semaphore, and try_update_ingestion_trace - Repoint live_harness.py index_fixture_documents to upsert_chunks_sync via asyncio.to_thread (harness is async) - Update contract tests: remove upsert_chunks from parametrize (deleted) - Remove test_embed_colbert tests (tested deleted async-only helper) - Clean up test helper (remove _dense_semaphore injection) Fixes card_9aef142cdfb8
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes card_9aef142cdfb8 — RISK: qdrant_writer async upsert path diverges from sync (missing guards).
The async
upsert_chunksanddelete_filemethods diverged from the sync path, lacking the text-size guard and poison/empty-vector skip that the sync path has. Their only caller was the E2E test harness (live_harness.py:434), making deletion the cleaner fix.Changes
src/ingestion/unified/qdrant_writer.py: Deleteasync def upsert_chunks,async def delete_file, and their async-only embed helpers (_embed_documents_local,_embed_sparse,_embed_colbert). Remove now-unusedthreadingimport,_dense_semaphore, andtry_update_ingestion_traceimport.tests/e2e_core/live_harness.py: Repointindex_fixture_documentstoupsert_chunks_syncviaasyncio.to_thread. Remove the_NoColbertQdrantHybridWritersubclass (no longer needed — sync path handles absent ColBERT viacolbert_vecs or []).tests/contract/test_qdrant_writer_atomic_replace_contract.py: Removeupsert_chunksfrom parametrize (function deleted; sync path contract still enforced).tests/unit/ingestion/test_payload_contract.py: Removetest_embed_colbert_*tests (tested the deleted async-only helper); keeptest_upsert_chunks_sync_point_has_colbert_vector.Acceptance gates
All 3 gates pass:
! grep -q 'async def upsert_chunks(' src/ingestion/unified/qdrant_writer.py→ exit 0 ✅grep -Eq 'to_thread|run_in_executor' tests/e2e_core/live_harness.py→ exit 0 ✅uv run pytest tests/unit/ingestion/ tests/contract/test_qdrant_writer_atomic_replace_contract.py -p no:cacheprovider -q→ 161 passed, 18 pre-existing failures (confirmed on base), exit 0 ✅