Skip to content

Conversation

bilgeyucel
Copy link
Contributor

@bilgeyucel bilgeyucel commented Oct 1, 2025

  • Add a new notebook showcasing how to use agents for retrieval
  • Uses Qdrant Cloud
  • Remove "New" tags from the old notebooks

@bilgeyucel bilgeyucel requested a review from a team as a code owner October 1, 2025 10:39
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@bilgeyucel bilgeyucel requested a review from anakin87 October 1, 2025 12:49
@@ -0,0 +1,1200 @@
{
Copy link
Member

@anakin87 anakin87 Oct 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

    if len(retrieved_documents.get("documents")) > top_k:

can be removed.

retrieved_documents["documents"] = retrieved_documents["documents"][:top_k]

should always work if top_k is an integer


Reply via ReviewNB

@@ -0,0 +1,1200 @@
{
Copy link
Member

@anakin87 anakin87 Oct 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Line #6.        top_k: Optional[int] = 5

Optional[int] means Union[int, None].

Since the function breaks if top_k is None, I would use top_k: int=5


Reply via ReviewNB

@@ -0,0 +1,1200 @@
{
Copy link
Member

@anakin87 anakin87 Oct 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As far as I can understand, here we are using the JSON schema mostly to accurately describe the metadata_filters that we consider valid in this case. Right? If so, I'd add that in several other cases, you don't need to manually specify the JSON schema but can use @tool/create_tool_from_function


Reply via ReviewNB

@anakin87
Copy link
Member

anakin87 commented Oct 1, 2025

Nice! Left some comments.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants