-
-
Notifications
You must be signed in to change notification settings - Fork 730
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
fixes retry_async message unpacking #175
Conversation
WalkthroughThe patch updates the Changes
Assessment against linked issues (Beta)
TipsChat with CodeRabbit Bot (
|
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.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (1)
- instructor/patch.py (1 hunks)
Additional comments: 1
instructor/patch.py (1)
- 89-95: The change from using
dict
with**
unpacking to directly appending the message tokwargs["messages"]
list is a simplification that should help avoid potential issues with unpacking and improve readability. However, ensure thatresponse.choices[0].message
is always available and that the structure ofresponse
is consistent with this expectation. Ifresponse.choices
orresponse.choices[0].message
could beNone
or not present, this could raise anAttributeError
orIndexError
. It might be necessary to add error handling for these cases.Additionally, the message being appended is now just the
response.choices[0].message
, whereas previously it seems like a dictionary withrole
andcontent
keys was being appended. Ensure that this change in the structure of the messages inkwargs["messages"]
is intentional and does not affect downstream processing.
thanks will add some more tests so this does not happen next time |
This fixes #174 by applying the same message appending logic as in the hotfix made here for
retry_sync
.Summary by CodeRabbit