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

feat: use original input for RAGTool / VectorDBTool #187

Conversation

SuZhou-Joe
Copy link
Member

@SuZhou-Joe SuZhou-Joe commented Feb 6, 2024

Description

In conversational agent, LLM is transforming user's input to other input like JSON / DSL query etc. So for VectorDBTool / RAGTool, we need to keep original input.

Issues Resolved

[List any issues this PR will resolve]

Check List

  • New functionality includes testing.
    • All tests pass
  • New functionality has been documented.
    • New functionality has javadoc added
  • Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

@SuZhou-Joe SuZhou-Joe changed the title feat: use original input for NeuralSparse / RAGTool / VectorDBTool feat: use original input for RAGTool / VectorDBTool Feb 6, 2024
@@ -71,6 +71,11 @@ public class RAGTool implements Tool {
@Setter
private Parser outputParser;

@Override
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is a new method defined in Tool's interface?

Copy link
Member Author

Choose a reason for hiding this comment

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

Nope, it's an existing method but none of the tools override it. For RAGTool and VectorDBTool, they need original input(plain text) instead of a LLM-transformed input.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you add some simple comments to describe why original input is needed for this two tools?

@@ -71,6 +71,11 @@ public class RAGTool implements Tool {
@Setter
private Parser outputParser;

@Override
public boolean useOriginalInput() {
Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks @SuZhou-Joe to help flipping on the flag following the fix in common opensearch-project/ml-commons#2022 using original input for RAGTool/VectorDBTool/NeuralSparseSearchTool. IMO, the neuralSparseSearchTool also need to take original input question instead of llm generated questions. @zhichao-aws would you agree on this for NeuralSparseSearchTool?

In this case, since vectorDB and NeuralSparseSearchTool are both extending from AbstractRetrieverTool, we can modify the AbstractRetrieverTool to flip on UseOriginalInput to true. Then both VectorDB and NeuralSparseSearchTool are using original input.

RAGTool also need to keep this useOrginalInput flag because it's not extending from AbstractRetrieverTool now.

@SuZhou-Joe
Copy link
Member Author

Hi team, we are trying to do a JSON serialization when finding that the input question is a JSON syntax @zhichao-aws . The new solution has some pros:

  • It can still leverage the LLM's output to increase search accuracy.
  • It will address the issue that if we keep the original input and input happens to have" inside, the buildQuery method will throw error as it break the structure of a standard JSON.

@zhichao-aws
Copy link
Member

The root cause is we format a query string (json string) in search tools. The query text is expected to be a plain text. If the query text itself is json string, it will throw exceptions to parse the formated string. Fix this in #203

@zhichao-aws
Copy link
Member

#203 is merged now. Let's close this PR. We can re-open it on-demand.

@zhichao-aws zhichao-aws closed this Feb 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants