Skip to content

Conversation

drizco
Copy link
Contributor

@drizco drizco commented Sep 12, 2025

This pull request refactors how chat messages are cleared in the AI chat feature by consolidating the logic into a new thunk, improving maintainability and ensuring that analytics and the CLEAR_CHAT event are consistently handled. The main change is the introduction of a new clearChatMessagesByUser thunk, which is now used across multiple components to standardize chat clearing behavior.

Chat Clearing Refactor:

  • Added a new thunk clearChatMessagesByUser in clearChatMessagesByUser.ts that handles clearing chat messages, logging the event, and sending analytics in a single action.
  • Updated exports in thunks/index.ts to include the new clearChatMessagesByUser thunk.

Component Updates:

  • Updated AichatView.tsx to use clearChatMessagesByUser instead of dispatching multiple actions separately when clearing chat messages.
  • Updated AiTutor2Chat.tsx to use the new clearChatMessagesByUser thunk for clearing chat messages, ensuring consistency across components.
Screen.Recording.2025-09-12.at.9.37.00.AM.mov

Links

Testing story

Deployment strategy

Follow-up work

Privacy

Security

Caching

PR Creation Checklist:

  • Tests provide adequate coverage
  • Privacy impacts have been documented
  • Security impacts have been documented
  • Code is well-commented
  • New features are translatable or updates will not break translations
  • Relevant documentation has been added or updated
  • User impact is well-understood and desirable
  • Follow-up work items (including potential tech debt) are tracked and linked

…ing "clear chat" button

adds a CLEAR_CHAT event to user's chat history, serving as a break point to prevent older chats from cluttering the view, as well as an analytics event matching onClear from AichatView
@drizco drizco requested a review from Copilot September 12, 2025 15:39
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR centralizes chat clearing functionality by extracting the clear chat logic into a dedicated thunk action that handles state reset, event logging, and analytics in one place.

  • Introduces a new clearChatMessagesByUser thunk that combines chat clearing, event logging, and analytics
  • Refactors both AichatView.tsx and AiTutor2Chat.tsx to use the new centralized clearing method
  • Replaces direct dispatcher calls with a unified approach for user-initiated chat clearing

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

File Description
apps/src/aichat/redux/thunks/clearChatMessagesByUser.ts New thunk that combines chat clearing, event logging, and analytics
apps/src/aichat/redux/thunks/index.ts Exports the new clearChatMessagesByUser thunk
apps/src/lab2/views/components/AiTutor2Chat.tsx Updates to use new centralized clearing method and adds useCallback
apps/src/aichat/views/AichatView.tsx Simplifies onClear callback to use new centralized method

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@drizco drizco marked this pull request as ready for review September 12, 2025 15:43
Copy link
Contributor

@cnbrenci cnbrenci left a comment

Choose a reason for hiding this comment

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

Looks great! Thank you!

The wording in the "You cleared the chat workspace" notification feels slightly weird to me for Tutor, but these notifications in general can be addressed separately. There's another conversation going on about a different notification event here https://codedotorg.slack.com/archives/C08S29NDZ5E/p1757631350402479

Copy link
Contributor

@edcodedotorg edcodedotorg left a comment

Choose a reason for hiding this comment

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

Thanks for the quick fix! I know @sanchitmalhotra126's goal is to limit or remove our use of redux throughout the chat infrastructure, so that's something we should discuss moving forward (I certainly don't have all the details). But this fixes a major bug and we already make heavy use of redux, so I'd say we can just keep the need for that discussion in mind for future changes/fixes. 🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants