Skip to content

fix: move typingDiv guard to sendMessage() to prevent chatHistory desync (#6172)#6184

Open
piyushdotcomm wants to merge 1 commit intosugarlabs:masterfrom
piyushdotcomm:fix/6172-reflection-sendmessage-concurrency
Open

fix: move typingDiv guard to sendMessage() to prevent chatHistory desync (#6172)#6184
piyushdotcomm wants to merge 1 commit intosugarlabs:masterfrom
piyushdotcomm:fix/6172-reflection-sendmessage-concurrency

Conversation

@piyushdotcomm
Copy link
Contributor

PR Category

  • Bug Fix
  • Feature
  • Performance
  • Tests
  • Documentation

Description

Fixes #6172

When a user sends multiple messages quickly (clicking Send or pressing Enter while the bot is still replying), the second message gets added to chatHistory and shows up in the UI, but botReplyDiv() silently drops it because of the if (this.typingDiv) return guard. This means chatHistory goes out of sync with what the LLM actually received.

Changes

  • Added if (this.typingDiv) return guard at the top of sendMessage() so the whole function exits before pushing to chatHistory or updating the UI
  • Removed the same guard from botReplyDiv() since it's no longer needed there

This is safe because sendMessage() is the only caller that uses the default user_query = true path. The other callers (startChatSession, updateProjectCode, getAnalysis) all pass user_query = false.

…istory (sugarlabs#6172)

When a user rapidly sends messages while the bot is processing,
sendMessage() unconditionally pushed to chatHistory and the UI,
but botReplyDiv() silently returned due to the typingDiv guard.
This caused chatHistory to contain user messages that were never
sent to the backend, permanently desyncing the conversation.

Fix: Move the typingDiv guard into sendMessage() so the entire
send is blocked before any state mutation. Remove the redundant
guard from botReplyDiv().

Closes sugarlabs#6172
@github-actions github-actions bot added the bug fix Fixes a bug or incorrect behavior label Mar 9, 2026
@github-actions
Copy link
Contributor

github-actions bot commented Mar 9, 2026

❌ Some Jest tests failed. Please check the logs and fix the issues before merging.

Failed Tests:

GraphicsBlocks.test.js

@piyushdotcomm
Copy link
Contributor Author

Hey @walterbender, this fixes the concurrency bug in #6172 — just moved the guard check earlier in sendMessage() so rapid sends don't mess up chatHistory. Happy to make changes if needed!

Copy link
Contributor

@parthdagia05 parthdagia05 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One small thought for the future might be disabling the input or queuing messages while the bot is replying, but that would be more of a UX improvement rather than part of this bug fix.

@piyushdotcomm
Copy link
Contributor Author

Good idea! Could be a nice follow-up. This PR just tackles the desync for now though

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug fix Fixes a bug or incorrect behavior

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] Reflection Widget: Concurrency bug in sendMessage causes dropped user queries

2 participants