-
Notifications
You must be signed in to change notification settings - Fork 2.5k
Bump morph version #3471
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
Bump morph version #3471
Conversation
|
nice - just need https://github.com/block/goose/pull/3471/checks?check_run_id=46145875633 done and if you run cargo fmt - good to go. |
| }, | ||
| "old_str": {"type": "string"}, | ||
| "new_str": {"type": "string"}, | ||
| "instruction": { |
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.
hrm - I wonder if there is another way to do this, as it is a heavy change to always ask for this value (even if optional, isn't clear how we can make it clear) as it will trip up some LLMs... not sure of another way, need to somehow work out how we can switch this on and off only if needed?
if you look in the description for this tool it conditionally changes depending on if there is some editor (model) or not - can we do the same for this so it isn't there when not needed using tokens?
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.
let me check
|
this looks much better - thanks. I just need to make sure that for default case things aren't changed for developer prompt (ie just eyeball/extra careful) as tricky to test/performance test |
|
FYI I think I can simplify this to not need an additional parameter - ie in the instructions for the code to replace we can have it provide the instructions clearly, I think that would work and keep the current signature |
|
h
hmm, not sure this could be relied on though without a proper inline tool calling framework like xml or something. if we're going with json tool calls i think we should keep it a separate parameter |
|
replacing this with this take which works nicely: #3745 |
bump to new morph-v3