convostate cleanup. claude cleanup#29010
convostate cleanup. claude cleanup#29010chrisnojima-zoom merged 10 commits intonojima/HOTPOT-next-670-cleanfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Refactors chat conversation state management to reduce duplication and streamline message/update handling, plus updates repository contributor/agent rules documentation.
Changes:
- Centralized common helpers in
convostate(current user + last ordinal) and cleaned up a few control-flow/data-shaping patterns. - Simplified attachment download error handling and moved attachment-view updates to consistent call sites.
- Reworked
onMessagesUpdatedto batch-convert UI messages and feed them throughmessagesAdd.
Reviewed changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| shared/stores/convostate.tsx | Refactors helpers and message/update flows; simplifies retry and messagesUpdated handling. |
| CLAUDE.md | Condenses and restates project rules and validation guidance. |
| .gitignore | Adds CLAUDE.md.local to ignored files. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
MessagesUpdated events (e.g. from ToggleMessageCollapse) were silently dropped by the messageUnchanged fast-path, which didn't check isCollapsed on messages or compare unfurl content when the unfurl count was unchanged.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…uccessful These fields can change independently via MessagesUpdated without any other content changing, so they must be checked in the fast-path skip.
Instead of a fragile field-by-field skip check, merge incoming message fields directly onto the existing draft so Immer's per-property change detection handles re-render granularity automatically. Maps are updated in-place (with deletes) for the same reason. HiddenString gets an equals() method so comparison stays encapsulated without exposing stringValue().
val.equals(cur as HiddenString) throws if cur is undefined, e.g. when decoratedText is absent on the existing message but present on incoming. Check cur instanceof HiddenString first before calling equals.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 4 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Using only Object.entries(incoming) meant fields present on the existing message but absent from incoming (e.g. submitState: 'pending' on a pending message that gets confirmed) were never cleared. Now iterating the union of both objects' keys so absent-on-incoming fields are correctly set to undefined.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 4 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
chrisnojima
changed the title
reactionMapToReactions returned new Map([]) when r.reactions was a truthy empty object, and unfurls built an empty Map for empty arrays. Both cases caused mergeMessage to see a reference change where there was no real change. Normalize to undefined so Immer detects no diff.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 5 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
chrisnojima-zoom
merged commit 6e62408
into
nojima/HOTPOT-next-670-clean
No description provided.