-
Notifications
You must be signed in to change notification settings - Fork 34
Add support for LLM-generated filter parameters in VectorSearchRetrieverTool #190
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Add support for LLM-generated filter parameters in VectorSearchRetrieverTool #190
Conversation
…verTool Signed-off-by: Sid Murching <[email protected]>
Signed-off-by: Sid Murching <[email protected]>
Signed-off-by: Sid Murching <[email protected]>
Signed-off-by: Sid Murching <[email protected]>
Signed-off-by: Sid Murching <[email protected]>
Signed-off-by: Sid Murching <[email protected]>
Signed-off-by: Sid Murching <[email protected]>
This reverts commit 6be1533.
- Update filter parameter description to include IMPORTANT guidance encouraging LLMs to search without filters first when unsure about filter values - Refactor OpenAI and LangChain integrations to use mixin's _get_filter_param_description() method for consistency - Add column metadata extraction from Unity Catalog to filter descriptions - Update demo scripts to demonstrate fallback pattern - When tested with LangChain AgentExecutor, LLMs intelligently decide when to generate filters and automatically fall back to no-filter searches when needed This guidance-based approach avoids zero-result scenarios due to hallucinated filter values while maintaining filtering flexibility. Inspired by Databricks Knowledge Assistants team's (Cindy Wang et al.) findings that searching with and without filters and merging results improves accuracy. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Demo scripts were used for testing and validation but are not part of the final PR. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Signed-off-by: Sid Murching <[email protected]>
- Move input model creation logic to mixin class (_create_enhanced_input_model, _create_basic_input_model) - Update OpenAI and LangChain integrations to use shared methods - Revert unrelated vectorstores.py change (workspace_client credential passing) - Revert corresponding test expectation change This consolidates duplicated logic between integrations and removes changes unrelated to the dynamic filter feature. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
…gration - Revert workspace_url and personal_access_token logic in VectorSearchClient initialization - Revert corresponding test expectations for workspace client credentials - These changes were unrelated to the dynamic filter feature 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
if not name.startswith("__"): | ||
column_info.append((name, col_type)) | ||
except Exception: | ||
pass |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should probably at least log a warning here
- Log a warning message when Unity Catalog table metadata cannot be fetched - Helps diagnose why column information may be missing from filter descriptions 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
- Remove try-catch logic in _get_filter_param_description() to fail loudly when table metadata is unavailable for dynamic_filter=True - Remove tests for missing column metadata scenario (no longer supported) - Run ruff format to fix lint issues - Better to fail loudly than have low quality filter descriptions 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Signed-off-by: Sid Murching <[email protected]>
Signed-off-by: Sid Murching <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks great! only significant comment was handling errors during the table information retrieval logic. let's be sure to update documentation afterwards to do education around the dynamic_filter
parameter including these points off the top of my head:
- When are appropriate cases to use
dynamic_filter
vs when predefined filters work just fine - Example usage
"Optional filters to refine vector search results as an array of key-value pairs. " | ||
"IMPORTANT: If unsure about filter values, try searching WITHOUT filters first to get broad results, " | ||
"then optionally add filters to narrow down if needed. This ensures you don't miss relevant results due to incorrect filter values. " | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not a review but wanted to comment: interesting insight here to search broadly then narrow down
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have you done testing on your own to confirm this retry filtering works as intended?
col_type = column_info_item.type_name.name | ||
if not name.startswith("__"): | ||
column_info.append((name, col_type)) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
since the dynamic filter requires column metadata to be useful, can we raise a value error if the column_info
is still empty at the end of this process?
may also be safe to add error handling/wrap the table information retrieval logic in a try catch if there are failures re permission checks/table doesn't exist or does not have entries
k=vector_search_tool.num_results, | ||
query_type=vector_search_tool.query_type, | ||
filter=expected_filters, | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if we add error handling per my comment in src/databricks_ai_bridge/vector_search_retriever_tool.py
- can we add a test that simulates failure during WorkspaceClient.tables.get()
process
Add Dynamic Filter Support to VectorSearchRetrieverTool
Summary
This PR adds opt-in support for LLM-generated filter parameters in
VectorSearchRetrieverTool
, enabling LLMs to dynamically construct filters based on natural language queries. This feature is controlled by a newdynamic_filter
parameter (default:False
) that exposes filter parameters in the tool schema when enabled.Key Feature: The filter parameter description includes guidance for LLMs to use a fallback strategy - searching WITHOUT filters first to get broad results, then optionally adding filters to narrow down. This approach helps avoid zero results due to incorrect filter values while maintaining filtering flexibility.
Changes
Core Changes
1. New
dynamic_filter
Parameterdynamic_filter: bool
field toVectorSearchRetrieverToolMixin
(default:False
)True
, exposes filter parameters in the tool schema for LLM-generated filtersFalse
(default), maintains backward-compatible behavior with no filter parameters exposed2. Mutual Exclusivity Validation
@model_validator
to ensuredynamic_filter=True
and predefinedfilters
cannot be used together3. Enhanced Filter Parameter Descriptions with Fallback Strategy
workspace_client.tables.get()
)"Available columns for filtering: product_category (STRING), product_sub_category (STRING)..."
Integration Updates
OpenAI Integration (
integrations/openai/src/databricks_openai/vector_search_retriever_tool.py
)EnhancedVectorSearchRetrieverToolInput
(with optional filters) orBasicVectorSearchRetrieverToolInput
(without filters) based ondynamic_filter
settingOptional[List[FilterItem]]
withdefault=None
index.describe()["columns"]
which doesn't exist; now uses Unity Catalog tables APILangChain Integration (
integrations/langchain/src/databricks_langchain/vector_search_retriever_tool.py
)args_schema
creation based ondynamic_filter
settingOptional[List[FilterItem]]
withdefault=None
)Tests
New Test Coverage:
test_cannot_use_both_dynamic_filter_and_predefined_filters
- Validates mutual exclusivitytest_predefined_filters_work_without_dynamic_filter
- Ensures predefined filters work without dynamic modetest_enhanced_filter_description_with_column_metadata
- Verifies column info is includedtest_enhanced_filter_description_without_column_metadata
- Handles missing column info gracefullytest_filter_item_serialization
- Tests FilterItem schemaTest Results:
Usage
Basic Usage (OpenAI)
Basic Usage (LangChain)
🎯 Recommendations
When to use
dynamic_filter=True
:When to use predefined
filters
:Implementation Details
Fallback Strategy Mechanism
The fallback strategy is implemented through tool description guidance rather than execution-time logic:
Optional[List[FilterItem]]
withdefault=None
This approach:
Validation
Testing in Practice
When tested with LangChain
AgentExecutor
, we observe that the LLM intelligently decides when to generate filters based on the user prompt and the guidance in the tool description. The fallback strategy works as intended - LLMs learn from the IMPORTANT guidance to search without filters first when unsure about filter values, then retry with filters if appropriate.Observed LLM Behavior
When tested with LangChain
AgentExecutor
, the LLM demonstrates intelligent filter usage:Example Query: "Find documentation about Data Engineering products"
LLM Actions:
First attempt: Tries with a filter based on the query:
Result: Empty (0 results) - the category doesn't exist in the index
Second attempt: Following the IMPORTANT guidance, automatically retries WITHOUT filters:
Result: Success! Returns relevant results from actual categories (Software, Computers, etc.)
Key Observation: The LLM learns from the guidance to: