forked from ggml-org/llama.cpp
-
Notifications
You must be signed in to change notification settings - Fork 588
major improvements to tool calling logic #1839
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
Closed
Rose22
wants to merge
21
commits into
LostRuins:concedo_experimental
from
Rose22:concedo_experimental
Closed
Changes from 18 commits
Commits
Show all changes
21 commits
Select commit
Hold shift + click to select a range
70902ed
improvements to tool calling logic
1452960
Revert "improvements to tool calling logic"
72a96fa
major improvements to tool calling logic
a86080d
major improvements to tool calling logic
7329347
fixed toolcall choosing logic
0c69dfd
fixed toolcall choosing logic even more
b8a4a08
removed concedo's toolcall escape since it clashes with my prompting
afba8bd
improved tool call choice logic by taking into account LLM's prior re…
f0b32b3
improved interpreting of first decision
86b8317
improved tool call decision logic by putting the user's LAST message …
7bf8ffc
fixed small mistake
5a9347d
fixed formatting issue
58f37f4
Merge branch 'LostRuins:concedo_experimental' into concedo_experimental
Rose22 994427d
linting
LostRuins ba20f9f
Merge branch 'LostRuins:concedo_experimental' into concedo_experimental
Rose22 44d0166
Merge branch 'LostRuins:concedo_experimental' into concedo_experimental
Rose22 1794c70
Merge branch 'LostRuins:concedo_experimental' into concedo_experimental
Rose22 9a4a4d0
Merge branch 'LostRuins:concedo_experimental' into concedo_experimental
Rose22 459b39b
Merge branch 'LostRuins:concedo_experimental' into concedo_experimental
Rose22 edba5ae
Merge branch 'LostRuins:concedo_experimental' into concedo_experimental
Rose22 47c4809
added some tweaks for improved tool calls to reuse old ctx, but needs…
LostRuins File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why remove the null tool? seems like its still a good idea to have even if the llm knows that a tool is needed. it gives it a second chance to change its mind.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
because in testing this often resulted in toolcalls that i explicitely asked for being cancelled. best to just not interfere with its final reasoning..
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Formerly the null tool was to give the LLM an out they would go for, rosie's PR instead does this with a reasoning step. Same concept different implementation.