Fix snapshot events being dropped after trigger lifecycle hooks #3434
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
When a trigger mutates state in __on_workflow_initiated__ or __on_workflow_fulfilled__ (e.g., ChatMessageTrigger appending to chat_history), the snapshottable list writes push a WorkflowExecutionSnapshottedEvent onto _workflow_event_inner_queue. Previously, these events were dropped because the inner queue was not drained after the trigger hooks. This commit adds draining of the inner queue after both trigger hooks to ensure snapshot events are forwarded to the outer queue and observable by stream consumers. Co-Authored-By: [email protected] <[email protected]>
Contributor
Author
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
…tiated__ and improve test assertions - Removed the drain after __on_workflow_initiated__ since snapshot events from that hook are already processed by the main event loop - Updated test to assert chat_history contents directly using tuple comparison for better readability and pytest diffs Co-Authored-By: [email protected] <[email protected]>
Use ChatMessage objects directly in assertion for better readability and pytest diffs on failure. Co-Authored-By: [email protected] <[email protected]>
Verify that the snapshot event from __on_workflow_initiated__ (with just the user message) is captured, proving the drain after initiated is not needed since the main event loop already processes it. Co-Authored-By: [email protected] <[email protected]>
Use snapshot_events[0] and assert chat_history directly, matching the style of the last_snapshot assertion. Co-Authored-By: [email protected] <[email protected]>
vincent0426
approved these changes
Dec 19, 2025
dvargas92495
approved these changes
Dec 20, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
When a trigger mutates state in
__on_workflow_fulfilled__(e.g.,ChatMessageTriggerappending the assistant message tochat_history), the snapshottable list pushes aWorkflowExecutionSnapshottedEventonto_workflow_event_inner_queue. Previously, these events were dropped because the inner queue was not drained after the trigger hook. This fix adds draining of the inner queue after__on_workflow_fulfilled__to ensure snapshot events are forwarded to the outer queue and observable by stream consumers (when usingall_workflow_event_filter).Note: Draining after
__on_workflow_initiated__is not needed because those snapshot events are already processed by the main event loop before it exits.Review & Testing Checklist for Human
__on_workflow_fulfilled__doesn't introduce event ordering issues - snapshot events should appear before the fulfilled event__on_workflow_initiated__is correct - the test assertssnapshot_events[0]contains just the user message, which could be brittle if event ordering changesChatMessageTriggerand verify both snapshot events (user message and assistant message) are visible when streaming withall_workflow_event_filterTest Plan:
Notes
The default
workflow_event_filterintentionally excludes snapshot events - consumers must useall_workflow_event_filteror a custom filter to observe them.