Conversation
🦋 Changeset detectedLatest commit: acf2e6f The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
commit: |
|
/bonk review pretty please |
|
remote: Permission to cloudflare/agents.git denied to github-actions[bot]. |
|
Bonk workflow failed. Check the logs for details. |
|
I now have a thorough understanding of the PR. Let me write up my review. PR Review: Allow simultaneous inputs on AIChatSummaryThis PR replaces the What's Good
Issues1. Fire-and-forget
|
|
/bonk review pretty please |
|
I read the PR diff, changeset file, and key sections of |
|
May I please have some tests |
|
should we update the ai-chat example for this? what's the ux like? also is this behaviour alsways desirable / should it be a flag? |
3ca4691 to
d1e7149
Compare
Donezo. Added one to check response order of new messages and one to make sure the queue empties on clear |
d1e7149 to
94b0601
Compare
Tests verify that: - Multiple simultaneous messages are processed in order without interleaving - Queue is cleared when chat history is cleared
94b0601 to
acf2e6f
Compare
|
Matt's AI review: Critical concerns:
Test gaps:
|
|
looks good otherwise. preemptively stamping but some of the above could be good to finalise. |
Still looking for answers on this |
My bad I never saw this comment. There are no changes needed to examples because the UX hasn't been changed at all, the changes are only to the AIChat Right now if you send 2 messages you'll get interleaved stream chunks in the same stream and the data race produces garbage outputs so I don't think adding a flag for it is worth it. That being said, we might want to eventually add a flag to decide when queued messages are processed (after the first tool response or after the final LLM response). Right now only final LLM is available so I think that can be done in a different PR? |
|
hokay. I'll review this later today. |
Fixes #24: AIChatAgent now handles multiple messages arriving simultaneously by queuing them and processing one at a time.
Problem
When multiple chat messages arrived before the first response completed, the
onChatMessage → _replycycles would race, causing:Solution
Added a request queue that serializes chat message processing:
_chatRequestQueue— Array of pending work functions_chatRequestInFlight— Guard to prevent concurrent processing_enqueueChatRequest()— Adds work to queue, kicks off drain if idle_drainChatRequestQueue()— Processes queue items sequentiallyChanges
onChatMessagecallswaitForStream()+ 500ms timeout hack_streamCompletionPromise/_streamCompletionResolvecode (no longer needed)destroy()andCF_AGENT_CHAT_CLEAR