-
Notifications
You must be signed in to change notification settings - Fork 1.5k
Add unique run_id to run, run result, and message (request, response) classes
#3366
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
Add unique run_id to run, run result, and message (request, response) classes
#3366
Conversation
…raphState to these ModelMessages
|
@DouweM A lot of tests will need to be modified to accommodate run_id in the snapshots. |
@adtyavrdhn Yep, we'll need |
…dding_run_id_to_modelmessage
|
This ended up being a much bigger diff than what I was expecting |
DouweM
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.
@adtyavrdhn Nice work Aditya!
…sult, AgentRun + docstring changes
…dding_run_id_to_modelmessage
…dding_run_id_to_modelmessage
…dding_run_id_to_modelmessage
…dding_run_id_to_modelmessage
run_id to run, run resuilt, and message (request, response) classes
run_id to run, run resuilt, and message (request, response) classesrun_id to run, run result, and message (request, response) classes
run_id to run, run result, and message (request, response) classesrun_id to run, run result, and message (request, response) classes
Closes #3171
Adds a UUID run_id at agent start, thread it through GraphAgentState, and assign it to every ModelRequest/ModelResponse as they’re created so all execution paths share one identifier.
Keep run_id optional on message classes for backward compatibility.
Cover the change with tests:
(1) assert captured messages and result.all_messages() all carry the same non-empty run_id and
(2) confirm ModelMessagesTypeAdapter preserves the field through serialization.