-
Notifications
You must be signed in to change notification settings - Fork 184
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
Fix RedisVectorStoreDriver bugs #782
Conversation
@@ -120,8 +123,9 @@ def query( | |||
|
|||
vector = self.embedding_driver.embed_string(query) | |||
|
|||
filter_expression = f"(@namespace:{{{namespace}}})" if namespace else "*" |
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.
What happens if the namespace
field doesn't exist?
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 the namespace
does not exist in the index schema, then passing namespace
to query will never find any results, including when it belongs to a tool.
So in my example above, driver.query("What is griptape?", namespace="griptape")
would produce 0 results instead of 1 and the VectorStoreClient
tool (configured with a namespace
) will never find any results.
I think we might be able to inspect the schema to either change this behavior or provide a warning to prevent people from shooting their feet. Not every query, on initialization. I'll see what I can do. Let me know if you have other ideas.
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.
Gotcha. So, when no namespace
parameter is specified and we add the @namespace:*
filter, will it return empty results as well? If so, we should probably not include that filter, right?
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 no namespace
is provided, filter_expression
will evaluate to '*'
rather than @namespace:*
(there's a ternary). Providing no namespace
will search through all vectors in the index just as before.
This is illustrated in the example program via driver.query("What is griptape?")
, which returns a result.
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.
Oops, I misread the code, sorry! Let's ship it!
Fixes: #778
Changes:
RedisVectorStoreDriver:query
to correctly set themeta
,score
, andvector
fields of query result. This change fixes VectorQueryEngine issue with loading artifacts from query #778.RedisVectorStoreDriver:query
to honor the namespace option.namespace
attribute with typeTAG
if you want to be able to use namespaces.Additional Manual Testing:
docker run -p 6379:6379 redislabs/redisearch:latest
redis-cli FT.CREATE idx:griptape ON hash PREFIX 1 "griptape:" SCHEMA namespace TAG vector VECTOR FLAT 6 TYPE FLOAT32 DIM 1536 DISTANCE_METRIC COSINE
Output:
📚 Documentation preview 📚: https://griptape--782.org.readthedocs.build//782/