-
Notifications
You must be signed in to change notification settings - Fork 147
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
Bubble up errors and clean up error creation #475
Conversation
rajasbansal
commented
Jul 20, 2023
- Errors have a structure with error type and error message
- Errors sent in addition to text generation
- New RefuelLLMResult which encapsulates text generations, errors and cost
- Confidence calculate now returns a float
- Errors saved along with annotations and saved in the dataframe
6e94049
to
e873409
Compare
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.
lgtm!
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.
lgtm otherwise!
class ErrorType(str, Enum): | ||
"""Enum of supported error types""" | ||
|
||
LLM_PROVIDER_ERROR = "llm_provider_error" |
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.
do we have a way to understand what is the LLM provider error ? e.g. rate limit / availability errors can be fixed with retries but context length errors are deterministic.
if this is hard to parse and needs provider specific logic, okay to club into one for now but ideally we can bubble the specific error to the users.
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.
I think we should definitely handle context length errors differently. They are handled separately for different providers and there is no consistent langchain error that is sent. We can do better and combine this across providers. Might need some custom string matching on the error for different providers - tracking this as a separate ticket #481