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

create shared memory with vector db , #251

Merged

Conversation

Eyobyb
Copy link
Collaborator

@Eyobyb Eyobyb commented Dec 6, 2023

This pull request (PR) has been created to improve file handling by implementing the use of a vector database. Within the test, the addition of a file to the vector database has been simulated. Ultimately, the QA agent utilizes the vector shared memory to incorporate the file added to the vector database as one source of context."

change the agent to take user_type event ,
create a test that use qa agent to show sharedmemory with vector db usage. also mimic what a scraped text with metadata should look like.

[resolves #235 ]

Copy link
Collaborator

@oshoma oshoma left a comment

Choose a reason for hiding this comment

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

Some initial comments.

I don't really understand the purpose of this PR or the related issue, so @20001LastOrder should do a deeper review.

src/sherpa_ai/config/__init__.py Outdated Show resolved Hide resolved
src/sherpa_ai/connectors/vectorStorage.py Outdated Show resolved Hide resolved
src/sherpa_ai/connectors/ChromaVectorStore.py Show resolved Hide resolved
src/sherpa_ai/memory/shared_memory_with_vectordb.py Outdated Show resolved Hide resolved
src/sherpa_ai/memory/shared_memory_with_vectordb.py Outdated Show resolved Hide resolved
src/sherpa_ai/memory/shared_memory_with_vectordb.py Outdated Show resolved Hide resolved
src/sherpa_ai/memory/vector_storage.py Outdated Show resolved Hide resolved
src/sherpa_ai/connectors/vectorStorage.py Outdated Show resolved Hide resolved
@oshoma oshoma self-requested a review January 29, 2024 14:38
@oshoma
Copy link
Collaborator

oshoma commented Feb 7, 2024

I just ran the tests again, this time in debug mode to see what GPT is doing with the request. I confirmed the file is passed through to GPT. However if you look at the response in the attached log file (view it in VS Code for colors) you'll see that GPT ignores the file content. Its response starts with, "How to summarize a file", and then it talks about how to summarize a PDF with AI. That part of the response looks like it comes from Google Search.

Is that the expected behavior @20001LastOrder @Eyobyb ?

See 2024-02-07-shared-memory-test.log

On a separate note, the log also shows that Slack bolt app is running here, even though I am only running one specific test. I wonder how we can disable that.

@20001LastOrder
Copy link
Collaborator

20001LastOrder commented Feb 7, 2024

Slack

@oshoma
The first question: I don't think this is expected. We will need some more investigation.

The second question about Slack Bolt App: I think this is expected because you just ran poetry run pytest and it automatically collects tests in the apps folder as well. If you run poetry run pytest tests it should only consider test cases in the sherpa package.

@oshoma
Copy link
Collaborator

oshoma commented Feb 7, 2024

The second question about Slack Bolt App: I think this is expected because you just ran poetry run pytest and it automatically collects tests in the apps folder as well. If you run poetry run pytest tests it should only consider test cases in the sherpa package.

Got it, thanks. I guess just the act of collecting the tests runs bot = app.client.auth_test() in bolt_app.py, even though the tests themselves don't run.

@oshoma
Copy link
Collaborator

oshoma commented Feb 20, 2024

Next steps:

1 .This still requires investigation:

I just ran the tests again, this time in debug mode to see what GPT is doing with the request. I confirmed the file is passed through to GPT. However if you look at the response in the attached log file (view it in VS Code for colors) you'll see that GPT ignores the file content. Its response starts with, "How to summarize a file", and then it talks about how to summarize a PDF with AI. That part of the response looks like it comes from Google Search.

  1. test_shared_memory_with_vector needs rework to run offline (no network, use mocks)

@Eyobyb
Copy link
Collaborator Author

Eyobyb commented Feb 21, 2024

Next steps:

1 .This still requires investigation:

I just ran the tests again, this time in debug mode to see what GPT is doing with the request. I confirmed the file is passed through to GPT. However if you look at the response in the attached log file (view it in VS Code for colors) you'll see that GPT ignores the file content. Its response starts with, "How to summarize a file", and then it talks about how to summarize a PDF with AI. That part of the response looks like it comes from Google Search.

  1. test_shared_memory_with_vector needs rework to run offline (no network, use mocks)

I'm not sure where we discussed this before, but Percy confirmed that right now, we'll experience this issue because it always Google search tool as a qaAgent for any request, even if it already has the necessary information stored in its memory.

change the agent to take user_type event ,
create a test that use qa agent to show sharedmemory with vector db

fix pr review

fix review issues

create shared memory with vector db ,
change the agent to take user_type event ,
create a test that use qa agent to show sharedmemory with vector db

fix issues raised on pr review
@Eyobyb Eyobyb force-pushed the feature/sharedmemorywithvectordb branch from a498731 to 62017ac Compare February 26, 2024 13:31
@Eyobyb Eyobyb requested a review from oshoma February 26, 2024 13:34
Copy link
Collaborator

@oshoma oshoma left a comment

Choose a reason for hiding this comment

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

  1. Some small change requests.

  2. This PR has been open a long time. Please rebase on top of origin/main to pick up the latest changes from main.

src/sherpa_ai/memory/belief.py Outdated Show resolved Hide resolved
@Eyobyb Eyobyb requested a review from oshoma March 5, 2024 14:33
Copy link
Collaborator

@20001LastOrder 20001LastOrder left a comment

Choose a reason for hiding this comment

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

Please see the comments below. Thanks!

src/sherpa_ai/config/__init__.py Outdated Show resolved Hide resolved
src/sherpa_ai/connectors/chroma_vector_store.py Outdated Show resolved Hide resolved
src/sherpa_ai/connectors/chroma_vector_store.py Outdated Show resolved Hide resolved
src/sherpa_ai/connectors/chroma_vector_store.py Outdated Show resolved Hide resolved
src/sherpa_ai/connectors/chroma_vector_store.py Outdated Show resolved Hide resolved
src/sherpa_ai/connectors/chroma_vector_store.py Outdated Show resolved Hide resolved
@20001LastOrder
Copy link
Collaborator

I think everything else is ready for this PR, we need some comments/documentation in the .env_sample file for INDEX_NAME_FILE_STORAGE @Eyobyb

Copy link
Collaborator

@20001LastOrder 20001LastOrder left a comment

Choose a reason for hiding this comment

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

Please check the comments above

change the agent to take user_type event ,
create a test that use qa agent to show sharedmemory with vector db

fix pr review

fix review issues

create shared memory with vector db ,
change the agent to take user_type event ,
create a test that use qa agent to show sharedmemory with vector db

fix issues raised on pr review
@Eyobyb Eyobyb force-pushed the feature/sharedmemorywithvectordb branch from d1daac6 to 78663a7 Compare April 8, 2024 11:01
Copy link
Collaborator

@20001LastOrder 20001LastOrder left a comment

Choose a reason for hiding this comment

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

Sorry, I think I missed another comment above. Also, please rebase or merge the PR with the main branch so that the tests can be run.

Eyobyb and others added 8 commits April 9, 2024 15:00
change the agent to take user_type event ,
create a test that use qa agent to show sharedmemory with vector db

fix pr review

fix review issues

create shared memory with vector db ,
change the agent to take user_type event ,
create a test that use qa agent to show sharedmemory with vector db

fix issues raised on pr review
@20001LastOrder 20001LastOrder self-requested a review April 11, 2024 02:00
20001LastOrder
20001LastOrder previously approved these changes Apr 11, 2024
Copy link
Collaborator

@20001LastOrder 20001LastOrder left a comment

Choose a reason for hiding this comment

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

LGTM now

oshoma
oshoma previously approved these changes Apr 18, 2024
@20001LastOrder 20001LastOrder dismissed stale reviews from oshoma and themself via 1186d9a April 18, 2024 19:56
Copy link
Collaborator

@20001LastOrder 20001LastOrder left a comment

Choose a reason for hiding this comment

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

LGTM

@20001LastOrder 20001LastOrder merged commit cb512c8 into Aggregate-Intellect:main Apr 18, 2024
1 check passed
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.

Build a new version of shared memory that utilizes the vector database for storage.
3 participants