fix: skip empty metadata filter in RetrieverTool to prevent vector store error#5976
fix: skip empty metadata filter in RetrieverTool to prevent vector store error#5976majiayu000 wants to merge 2 commits intoFlowiseAI:mainfrom
Conversation
…ore error
When a user adds an "Additional Metadata Filter" then removes it, the
empty filter object {} was still passed to the vector store, causing
Pinecone (and potentially others) to reject it with "You must enter a
filter object with at least one key-value pair." Now checks that the
resolved filter has at least one key before assigning it.
Fixes FlowiseAI#4900
Signed-off-by: majiayu000 <1835304752@qq.com>
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses a critical bug in the Retriever Tool where an empty metadata filter could lead to errors in vector store operations. By introducing a robust check for non-empty filter objects, the change ensures that only valid filters are applied, thereby enhancing the stability and reliability of interactions with vector store APIs and improving the overall user experience when configuring retrieval tools. Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request addresses an issue where an empty metadata filter object could be passed to the vector store in RetrieverTool, causing errors. The fix introduces a guard to prevent this, and new unit tests have been added to cover this change.
A similar pattern of assigning a metadata filter without checking if it's an empty object appears to exist in packages/components/nodes/vectorstores/Pinecone/Pinecone.ts on lines 254-258. This could lead to the same error you're fixing here. Consider applying a similar guard in that file as well to improve robustness.
| const metadatafilter = | ||
| typeof retrieverToolMetadataFilter === 'object' | ||
| ? retrieverToolMetadataFilter | ||
| : JSON.parse(retrieverToolMetadataFilter) | ||
| const newMetadataFilter = resolveFlowObjValue(metadatafilter, {}) |
There was a problem hiding this comment.
For consistency with the camelCase naming convention used elsewhere in this file and the codebase (e.g., retrieverToolMetadataFilter, newMetadataFilter), please rename metadatafilter to metadataFilter.
| const metadatafilter = | |
| typeof retrieverToolMetadataFilter === 'object' | |
| ? retrieverToolMetadataFilter | |
| : JSON.parse(retrieverToolMetadataFilter) | |
| const newMetadataFilter = resolveFlowObjValue(metadatafilter, {}) | |
| const metadataFilter = | |
| typeof retrieverToolMetadataFilter === 'object' | |
| ? retrieverToolMetadataFilter | |
| : JSON.parse(retrieverToolMetadataFilter) | |
| const newMetadataFilter = resolveFlowObjValue(metadataFilter, {}) |
There was a problem hiding this comment.
Fixed. Renamed metadatafilter to metadataFilter in both the test file and the source file for camelCase consistency.
Signed-off-by: majiayu000 <1835304752@qq.com>
|
Thanks for the suggestion about I think it's better to address that in a separate PR with its own testing to avoid unintended side effects. I'll open a follow-up issue for it. |
Summary
Fixes #4900
When a user adds an "Additional Metadata Filter" in the Retriever Tool node then removes it, the empty filter object
{}was still passed to the vector store. This caused Pinecone (and potentially other vector stores) to reject the query with:The fix adds a guard to check that the resolved metadata filter has at least one key-value pair before assigning it to
vectorStore.filter. This follows the same pattern already used inPinecone_LlamaIndex.ts.Changes
packages/components/nodes/tools/RetrieverTool/RetrieverTool.ts: AddObject.keys().length > 0check before settingvectorStore.filterpackages/components/nodes/tools/RetrieverTool/RetrieverTool.test.ts: Add 6 tests covering empty filter, non-empty filter, null filter, and end-to-end string-to-object parsing scenariosTest plan
{}filter is not passed to vector store{"category": "docs"}is correctly applied