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

Query argument in retriever.get_chat_history() #105

Open
shreehari-aiplanet opened this issue Oct 13, 2023 · 9 comments · May be fixed by #109
Open

Query argument in retriever.get_chat_history() #105

shreehari-aiplanet opened this issue Oct 13, 2023 · 9 comments · May be fixed by #109
Assignees
Labels
enhancement New feature or request good first issue Good for newcomers

Comments

@shreehari-aiplanet
Copy link
Collaborator

  • GenAI Stack version: 0.2.5
  • Python version: 3.8
  • Operating System: Ubuntu

Description

Query arguement in retriever.get_chat_history() seems a bit ambiguous. Hence it can be removed.

@shreehari-aiplanet shreehari-aiplanet added enhancement New feature or request good first issue Good for newcomers labels Oct 13, 2023
@dyaramatreya
Copy link

Hi @shreehari-aiplanet
Do you mean to remove the query at

return self.mediator.get_chat_history(query=query)
?

or to remove at

def get_chat_history(self, query:str) -> str:

@shreehari-aiplanet
Copy link
Collaborator Author

shreehari-aiplanet commented Oct 14, 2023

@dyaramatreya It needs to be removed wherever it is declared. Retreiver's basically getting the chat history from memory. A mediator between them is responsible for the interaction between memory and retriever to get the chat history. Hence it needs to be removed from more than 2 places.

@dyaramatreya
Copy link

@shreehari-aiplanet So basically, we have to delete this retriever.get_chat_history() method definition as well as the places where it is used. Am I correct?

@shreehari-aiplanet
Copy link
Collaborator Author

shreehari-aiplanet commented Oct 17, 2023

No, get_chat_history() method should exist and the query parameter that is being passed to it should be removed.

@dyaramatreya
Copy link

Ok @shreehari-aiplanet. Can I try working on this? Can you assign this issue to me? I'll make the changes and raise the PR

@dyaramatreya
Copy link

But then the query parameter passed at

return self._stack.memory.get_chat_history(query=query)
must also be removed right?

@shreehari-aiplanet
Copy link
Collaborator Author

But then the query parameter passed at

return self._stack.memory.get_chat_history(query=query)

must also be removed right?

yes

@dyaramatreya
Copy link

Thanks. Can you please assing this issue to me? I'll raise the PR in few hours

@dyaramatreya
Copy link

@shreehari-aiplanet Please have a look at the PR raised above and suggest if it's good or any changes are needed.
Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants