Python: Alternative fix for #1546 - Add comprehensive tests and fix CI issues for Azure AI Search Context Provider#2247
Closed
hayato-kishikawa wants to merge 6 commits intomicrosoft:mainfrom
Conversation
- Fixes mypy error: Cannot assign to final name 'DEFAULT_CONTEXT_PROMPT' - Base class ContextProvider already defines this as Final - Instance variable self.context_prompt correctly set in __init__ (Line 197) Related to microsoft#1546
Added comprehensive test coverage for Azure AI Search provider: - Agentic mode end-to-end flow (invoking → retrieval) - Agentic mode empty response fallback handling - Auto-discovery exception handling for network errors - Semantic search with semantic_configuration_name parameter Coverage improved from 73% to 84% (target: 80%+) Test results: 25 passed, 0 failed
Added 4 comprehensive tests for vector field auto-discovery (lines 351-384): - test_single_vector_field_with_embedding_function: Tests successful auto-discovery when embedding function is provided - test_single_vector_field_warning_without_embedding: Tests warning when vector field found but no embedding function - test_multiple_vector_fields_warning: Tests warning when multiple vector fields detected - test_index_client_cleanup_when_not_provided: Tests proper cleanup of internally-created index client Coverage improved from 84% to 91% (target: 90%+) Test results: 29 passed, 0 failed Key fix: Patched Azure SDK classes at source (azure.search.documents.*.aio) instead of module level to handle dynamic imports correctly
Systematically added comprehensive tests across 4 phases: **Phase 1 (91%→94%):** Agentic mode validation + resource cleanup - 7 tests for parameter validation (init-time and runtime) - Retrieval client cleanup verification **Phase 2 (94%→95%):** Import error handling - 1 test for SDK unavailability (ImportError) **Phase 3 (95%→96%):** Edge cases and alternative paths - Empty search results handling - External index client usage - Early return optimization **Phase 4 (96%→98%):** Helper functions - Document text extraction fallback logic Coverage: 91% → 98% (186 statements, 3 missed) Tests: 29 → 41 passed (+12 tests) **Remaining 3 uncovered lines (acceptable):** - Lines 60-61: Module import exception (impossible to unit test) - Line 580: Agentic RuntimeError (requires complex SDK mocking) All tests pass. 98% coverage exceeds industry best practices.
d77b042 to
968acb3
Compare
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
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.
Context
This PR provides an alternative fix for #1546 by @farzad528.
Why an alternative PR?
PR #1546 has excellent implementation (633 lines) but has been blocked by CI issues since Nov 2nd (13 days). I attempted to collaborate by offering patches, but decided to submit this alternative PR to maintain momentum while remaining transparent about the situation.
Credit where it's due: The core Azure AI Search Context Provider implementation is entirely @farzad528's work. I only added CI fixes and comprehensive tests.
Relationship with #1546
Co-authored-by)