-
Notifications
You must be signed in to change notification settings - Fork 2.4k
Remove orphaned tool calls before compaction #4968
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
Conversation
| info!("Removing last assistant message with pending tool request before compaction"); | ||
| messages_to_process.remove(last_assistant_pos); | ||
| } | ||
| } |
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.
can't we just call the conversation fixer here to be safe?
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.
Will try that; would be nice to not have to add new logic.
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.
it could possibly help with some other corner cases like if there's an empty message in there during streaming that we recently saw. maybe also change the type here to conversation from Vec - fixes some of that too
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.
Yeah doesn't quite have this new logic so keeping both, but threw it in too.
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.
now that you have the conversation fixer I don't think you need this check
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.
It doesn't handle the visible/non-visible messages now but added some logic for that.
|
OK have this pretty extensively tested new. The new logic consistently triggers and haven't seen the orphaned tool call error after maxing out the context window on 10 sessions. |
|
2402e5a to
4930d9d
Compare
…orphaned-tool-calls * 'release/1.9.2' of github.com:block/goose: Fix auto scroll to bottom during chat (#4923) fix redirect to extensions page after deeplink install and show toast with success message (#4863) added claude-sonnet-4-5 (#4906) Upgrade electron for macOS Tahoe compatibility (#5015) chore(release): release version 1.9.2
Remove orphaned tool calls before compaction. Also add in fix_conversation to catch other potential conversation errors.
Main branch version: #4987