Skip to content
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

Zep Retriever - Vector Search Over Chat History #4533

Merged
merged 10 commits into from
May 18, 2023
Merged

Zep Retriever - Vector Search Over Chat History #4533

merged 10 commits into from
May 18, 2023

Conversation

danielchalef
Copy link
Contributor

@danielchalef danielchalef commented May 11, 2023

Zep Retriever - Vector Search Over Chat History with the Zep Long-term Memory Service

More on Zep: https://github.com/getzep/zep

Note: This PR is related to and relies on #4834. I did not want to modify the pyproject.toml file to add the zep-python dependency a second time.

Before submitting

Notebook included in the PR. Integration tests require a Zep service container. Let me know if this is feasible given your CI setup.

Who can review?

Community members can review the PR once tests pass. Tag maintainers/contributors who might be interested:

@dev2049

Copy link
Contributor

@dev2049 dev2049 left a comment

Choose a reason for hiding this comment

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

very cool, thanks for contribution @danielchalef! one quick comment

langchain/retrievers/zep.py Outdated Show resolved Hide resolved
@danielchalef
Copy link
Contributor Author

danielchalef commented May 12, 2023

@dev2049 Thanks for pointing that out. Removed the class attributes. Note that this PR relies on the other Zep PR for installation of the zep-python client: #4834

@danielchalef
Copy link
Contributor Author

@dev2049 Bump on this one, too. Thanks!

@danielchalef
Copy link
Contributor Author

@dev2049 do you think we could do this at the same time as #4898?

Copy link
Contributor

@hwchase17 hwchase17 left a comment

Choose a reason for hiding this comment

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

this seems reasonable to me! i think @dev2049 already merged in the other pr, but lets wait to announce both of them on friday?

@danielchalef
Copy link
Contributor Author

this seems reasonable to me! i think @dev2049 already merged in the other pr, but lets wait to announce both of them on friday?

Friday's good. Thanks!

Copy link
Contributor

@dev2049 dev2049 left a comment

Choose a reason for hiding this comment

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

couple small comments but otherwise looks good! do you think it might be useful to add a to_retriever method to ZepChatMessageHistory or a from_message_history method to ZepRetriever (or both) that instantiates retriever from message history (since seems like they're based on same client/params under the hood)

langchain/retrievers/zep.py Outdated Show resolved Hide resolved
langchain/retrievers/zep.py Outdated Show resolved Hide resolved
langchain/retrievers/zep.py Outdated Show resolved Hide resolved
@danielchalef
Copy link
Contributor Author

couple small comments but otherwise looks good! do you think it might be useful to add a to_retriever method to ZepChatMessageHistory or a from_message_history method to ZepRetriever (or both) that instantiates retriever from message history (since seems like they're based on same client/params under the hood)

Love the idea. Can we do so in a future PR? We have some other features coming - search and metadata related - that may require an update.

@dev2049
Copy link
Contributor

dev2049 commented May 18, 2023

looking good! could we add some tests as well?

@danielchalef
Copy link
Contributor Author

looking good! could we add some tests as well?

Coming up. 10 minutes.

@danielchalef
Copy link
Contributor Author

@dev2049 Tests pushed

Took me a little longer to get the tests written. I hadn't really thought very deeply what popping would do here in terms of testing expected vs actual :-)

def _search_result_to_doc(self, results: List[SearchResult]) -> List[Document]:
        return [
            Document(
                page_content=r.message.pop("content"),
                metadata={"score": r.dist, **r.message},
            )
            for r in results
            if r.message
        ]

@danielchalef
Copy link
Contributor Author

Looks like github is having issues again :-(

Fetching the repository
  /usr/bin/git -c protocol.version=2 fetch --no-tags --prune --progress --no-recurse-submodules --depth=1 origin +03d2d1812795c619db355c36408497040e658203:refs/remotes/pull/4533/merge
  Error: fatal: unable to access 'https://github.com/hwchase17/langchain/': The requested URL returned error: 429
  The process '/usr/bin/git' failed with exit code 128
  Waiting 17 seconds before trying again
  /usr/bin/git -c protocol.version=2 fetch --no-tags --prune --progress --no-recurse-submodules --depth=1 origin +03d2d1812795c619db355c36408[49](https://github.com/hwchase17/langchain/actions/runs/5018751764/jobs/8998524009?pr=4533#step:2:54)7040e658203:refs/remotes/pull/4533/merge
  Error: fatal: unable to access 'https://github.com/hwchase17/langchain/': The requested URL returned error: 429
  The process '/usr/bin/git' failed with exit code 128
  Waiting 14 seconds before trying again
  /usr/bin/git -c protocol.version=2 fetch --no-tags --prune --progress --no-recurse-submodules --depth=1 origin +03d2d1812795c619db355c36408497040e658203:refs/remotes/pull/4533/merge
  Error: fatal: unable to access 'https://github.com/hwchase17/langchain/': The requested URL returned error: 429
  Error: The process '/usr/bin/git' failed with exit code 128

@dev2049 dev2049 merged commit c8c2276 into langchain-ai:master May 18, 2023
@danielchalef danielchalef mentioned this pull request Jun 5, 2023
@danielchalef danielchalef mentioned this pull request Jun 25, 2023
@danielchalef danielchalef deleted the zep_retriever branch June 25, 2023 23:16
@danielchalef danielchalef mentioned this pull request Jun 25, 2023
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.

3 participants