Forward-merge release/2.2 into develop - #370
Merged
Merged
Conversation
#### Overview Azure AI Search is eventually consistent. Upload and deletion results can therefore appear stale immediately after successful writes, causing premature job completion, incomplete cleanup, or inconsistent file state. This PR: - Deletes failed-upload chunks using known deterministic chunk IDs, including chunks that have not yet become searchable. - Waits for stable manifest and chunk visibility before marking an upload job complete. - Deletes file chunks using deterministic IDs instead of relying on an immediately consistent search. - Requires three consecutive collection-scoped zero-result reads covering both the file manifest and chunks before reporting deletion success. - Updates summary, tombstone, and local file bookkeeping only after deletion is confirmed, allowing an in-process retry after a confirmation timeout. - Adds regression coverage for delayed visibility, stable-read confirmation, collection isolation, and retryable deletion bookkeeping. The scope is limited to the Azure AI Search adapter and its tests. No API contract or configuration changed, so no documentation update is required. Known bounded limitations remain: stable reads are a consistency heuristic; confirmation-timeout retry is guaranteed only within the current process and, after restart, depends on the file manifest still being searchable. #### Validation ```text .venv/bin/ruff check \ sources/knowledge_layer/src/azure_ai_search/adapter.py \ tests/knowledge_layer_tests/test_azure_ai_search.py Passed .venv/bin/ruff format --check \ sources/knowledge_layer/src/azure_ai_search/adapter.py \ tests/knowledge_layer_tests/test_azure_ai_search.py Passed git diff --check Passed .venv/bin/pytest \ -p no:cacheprovider \ tests/knowledge_layer_tests/test_azure_ai_search.py 56 passed in 5.81s ``` Strict live Azure lifecycle validation was repeated three times with fresh collections. All three runs passed, verifying immediate delete/list/query behavior, cross-collection isolation, deletion of the remaining file, and complete collection cleanup. - [x] I ran the relevant local checks or explained why they are not applicable. - [x] I added or updated tests for behavior changes. - [ ] I updated documentation for user-facing or contributor-facing changes. - [x] I confirmed this PR does not include secrets, credentials, or internal-only data. - [x] I certify this contribution under the Developer Certificate of Origin (DCO) and signed my commits with `git commit -s` or an equivalent sign-off. #### Where should reviewers start? Start with `sources/knowledge_layer/src/azure_ai_search/adapter.py`, especially the failed-upload rollback, stable visibility checks, and `delete_file` bookkeeping order. Regression coverage is in `tests/knowledge_layer_tests/test_azure_ai_search.py`. #### Related Issues - Relates to #308 ## Summary by CodeRabbit * **Bug Fixes** * Improved Azure AI Search ingestion reliability under eventual consistency with stabilized polling for terminal manifest+chunk visibility and exact stable document counts. * Finalization is now visibility-checked; finalize failures trigger deterministic rollback (including chunk docs) and correct FAILED/SUCCESS state handling. * Deterministic chunk/document identifiers and consistency-safe deletes (including safe precomputed rollback after post-upload failures). * Collection deletion now fences concurrent submissions and rejects new jobs when the collection is no longer active. * **Tests** * Upgraded the Azure AI Search fake backend to model delayed/stale visibility and “response lost” failures; added/expanded lifecycle, restart/delete, rollback, and bounds-based polling assertions. Authors: - Harmke Alkemade (https://github.com/harmke) - Kyle Zheng (https://github.com/KyleZheng1284) Approvers: - Kyle Zheng (https://github.com/KyleZheng1284) URL: #336
Author
|
SUCCESS - forward-merge complete. |
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
2 participants
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.
Forward-merge triggered by push to release/2.2 that creates a PR to keep develop up-to-date. If this PR is unable to be immediately merged due to conflicts, it will remain open for the team to manually merge. See forward-merger docs for more info.