Python: Preserve falsy dynamic search filter defaults - #14303
Open
Tyyyy (uczltw6) wants to merge 1 commit into
Open
Python: Preserve falsy dynamic search filter defaults#14303Tyyyy (uczltw6) wants to merge 1 commit into
Tyyyy (uczltw6) wants to merge 1 commit into
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
This PR updates dynamic filter generation to apply parameter default values even when they’re falsy (e.g., 0, False, "") and adds unit tests to validate the behavior.
Changes:
- Fix default-value handling in dynamic filter construction by checking
is not Noneinstead of truthiness. - Add a parametrized unit test ensuring falsy defaults are translated into filter expressions correctly.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| python/semantic_kernel/data/_shared.py | Adjusts default filter generation to include falsy (but non-None) defaults. |
| python/tests/unit/data/test_text_search.py | Adds parametrized coverage for falsy default values in filter generation. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| assert results.value == ["test"] | ||
|
|
||
|
|
||
| @pytest.mark.parametrize(("default_value", "expected_literal"), [(0, "0"), (False, "False"), ("", "''")]) |
| assert results.value == ["test"] | ||
|
|
||
|
|
||
| @pytest.mark.parametrize(("default_value", "expected_literal"), [(0, "0"), (False, "False"), ("", "''")]) |
Author
|
@microsoft-github-policy-service agree |
Tyyyy (uczltw6)
force-pushed
the
codex/preserve-falsy-filter-defaults
branch
from
August 19, 2026 06:57
9ebb17f to
5e947bf
Compare
Contributor
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.
Motivation and Context
Dynamic search parameters with a default value are documented as adding equality filters. The current truthiness check drops valid falsy defaults such as
0,False, and an empty string, so invoking the generated search function without an override silently broadens the search.Description
Treat every non-
Noneparameter default as an explicit filter value. The regression test exercises the publiccreate_search_functionpath and verifies that all three falsy defaults reach the underlying search as filters.OpenAI Codex assisted with identifying the edge case and preparing the focused change. The behavior, diff, and test results were verified locally.
Validation
uv run pytest tests/unit/connectors/memory/test_in_memory.py tests/unit/data -ra— 122 passeduv run pre-commit run --files semantic_kernel/data/_shared.py tests/unit/data/test_text_search.py— all hooks passeduv run mypy -p semantic_kernel --config-file mypy.ini— no issues in 555 source filesContribution Checklist