-
Notifications
You must be signed in to change notification settings - Fork 44.6k
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
Rebase MessageHistory
on ChatSequence
#4922
Conversation
✅ Deploy Preview for auto-gpt-docs canceled.
|
5938ea3
to
15ca341
Compare
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## master #4922 +/- ##
==========================================
- Coverage 50.55% 50.50% -0.06%
==========================================
Files 116 116
Lines 4860 4875 +15
Branches 657 662 +5
==========================================
+ Hits 2457 2462 +5
- Misses 2219 2226 +7
- Partials 184 187 +3
☔ View full report in Codecov by Sentry. |
15ca341
to
b6a36bd
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.
Two small suggestions and a couple of questions, but otherwise in good shape.
You changed AutoGPT's behaviour. The cassettes have been updated and will be merged to the submodule when this Pull Request gets merged. |
* Rebase `MessageHistory` on `ChatSequence` * Process feedback & make mypy happy --------- Co-authored-by: James Collins <[email protected]>
* Rebase `MessageHistory` on `ChatSequence` * Process feedback & make mypy happy --------- Co-authored-by: James Collins <[email protected]>
Background
Part of #4799
The
MessageHistory
was already very similar to theChatSequence
. Now they are also compatible/interoperable (up to a degree).Changes
ChatSequence
implementationMessageHistory
class onChatSequence
MessageHistory
count_message_tokens
to allow passing a single messageDocumentation
x
Test Plan
CI + manual testing ✅
PR Quality Checklist