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

Temp patch to remove multiple tool calls #2720

Merged
merged 1 commit into from
Oct 8, 2024
Merged

Conversation

Weves
Copy link
Contributor

@Weves Weves commented Oct 8, 2024

^

Copy link

vercel bot commented Oct 8, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
internal-search ✅ Ready (Inspect) Visit Preview 💬 Add feedback Oct 8, 2024 2:55pm

Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

PR Summary

Removed the tools parameter from the _process_llm_stream method call in the _raw_output_for_explicit_tool_calling_llms method of the Answer class, addressing the current limitation of not supporting multiple tool calls in sequence.

  • Modified _raw_output_for_explicit_tool_calling_llms in backend/danswer/llm/answering/answer.py to remove tools parameter from _process_llm_stream call
  • Added a comment explaining the reason for removal: multiple tool calls in sequence are not currently supported
  • This change simplifies the tool calling process but may limit functionality for scenarios requiring multiple sequential tool calls
  • Potential impact on future extensibility if multiple tool calls become necessary
  • Consider adding a TODO or issue to track the potential need for supporting multiple tool calls in the future

1 file(s) reviewed, 1 comment(s)
Edit PR Review Bot Settings

Comment on lines +319 to +321
# as of now, we don't support multiple tool calls in sequence, which is why
# we don't need to pass this in here
# tools=[tool.tool_definition() for tool in self.tools],
Copy link

Choose a reason for hiding this comment

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

logic: Removing tools parameter may prevent future tool calls. Consider implications for extensibility.

@pablonyx pablonyx added this pull request to the merge queue Oct 8, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Oct 8, 2024
@pablonyx pablonyx added this pull request to the merge queue Oct 8, 2024
Merged via the queue into main with commit aa69fe7 Oct 8, 2024
7 checks passed
Weves added a commit that referenced this pull request Oct 8, 2024
Weves added a commit that referenced this pull request Oct 8, 2024
@Weves Weves deleted the temp-patch-tool-call branch October 18, 2024 22:30
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