fix: Chroma - guard empty filter conditions and increase unit tests coverage - #3797
Draft
julian-risch wants to merge 1 commit into
Draft
fix: Chroma - guard empty filter conditions and increase unit tests coverage#3797julian-risch wants to merge 1 commit into
julian-risch wants to merge 1 commit into
Conversation
…overage _create_where_document_filter indexed value[0] unconditionally, so a logical filter with an empty conditions list raised IndexError instead of a ChromaDocumentStoreFilterError. With the guard, the empty clause falls through to Chroma's own validation and surfaces as a filter error. On top of that, cover the code paths that were only reachable through the integration-marked tests, which the unit coverage badge excludes: filters.py, which had only four error-path tests before: - every comparison operator's mapping to its Chroma equivalent, the meta. prefix stripping, and all four malformed-condition errors - _parse_logical_condition for AND, OR and nesting - _create_where_document_filter, including how content conditions under a logical operator are combined - _convert_filters splitting a filter into ids, where and where_document document_store.py statics that had no test at all: _prepare_get_kwargs, _prepare_query_kwargs, _build_fields_info, _prepare_metadata_update and _apply_duplicate_policy, plus the remaining _convert_document_to_chroma branches for list metadata, unsupported types and blobs. The collection-backed methods are driven by injecting a mock collection onto the store: _ensure_initialized and _ensure_initialized_async both short-circuit once a collection is set, so no Chroma client is created. These assert the kwargs handed to the collection rather than round-tripped data, and cover write/delete/update by filter, the metadata aggregation family, search, the DocumentStoreError wrapping, and every async twin. Client construction is deliberately left to the integration tests. Unit coverage: 47% -> 95%. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
github-actions
Bot
added
integration:chroma
type:documentation
Contributor
Coverage report (chroma)
Click to see where and how coverage changed
This report was generated by python-coverage-comment-action |
||||||||||||||||||||||||||||||
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.
Related Issues
Proposed Changes:
_create_where_document_filterindexedvalue[0]unconditionally, so a logical filter with an emptyconditionslist raisedIndexErrorinstead of aChromaDocumentStoreFilterError. Guard it.filters.py, which previously had only four error-path tests, and for the@staticmethods that had none:_prepare_get_kwargs,_prepare_query_kwargs,_build_fields_info,_prepare_metadata_update,_apply_duplicate_policy._ensure_initializedpaths short-circuit on, so no Chroma client is created.How did you test it?
Notes for the reviewer
Checklist
fix:,feat:,build:,chore:,ci:,docs:,style:,refactor:,perf:,test:.