-
Notifications
You must be signed in to change notification settings - Fork 3
Implement RetryHandler on top of new internal API (closes #495) #522
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
eb8680
left a comment
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.
There are at least a couple other points of failure that we'd want to cover with retrying logic:
- Server-side LLM API/network failures - this should be handleable within
litellm(#494) - Runtime tool call failure, i.e. a tool raises an unhandled exception - we'd need to separately handle
call_tooland have it return an appropriate failureMessage
fcad5d8 to
bc1fcc0
Compare
|
@eb8680 updated! |
eb8680
left a comment
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.
We might also want to include the traceback in the feedback message. I think @datvo06 had done that at some point.
eb8680
left a comment
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.
|
Awesome! |
This PR reimplements RetryLLMHandler on top of the updated internal API.
When decoding fails, this handler:
1. Adds an error message to the conversation
2. Retries the completion with the error feedback
3. On success, returns the clean final message (without error messages)
The code is a bit long because
call_assistantdoes several logical steps, but otherwise encapsulates all the relevant logic to a single place.