fix(chat): merge Matrix m.replace edits into existing timeline row - #2152
Merged
Conversation
Contributor
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 15 minutes and 1 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (3)
WalkthroughAdds message-editing: provider exposes edit API and handles Matrix "replace" events in timelines; UI gains edit controls, edit state in right panel, composer wiring for edits, and i18n entries for edit actions. Changes
Sequence DiagramsequenceDiagram
actor User
participant ChatUI as Chat UI
participant RightPanel as Right Panel
participant MatrixProvider as Matrix Provider
participant MatrixServer as Matrix Server
User->>ChatUI: Click edit on message
ChatUI->>RightPanel: onEditMessage(messageId)
RightPanel->>RightPanel: validate & extract excerpt\nset editDraft, populate composer
RightPanel->>ChatUI: show editPreview & input text
User->>ChatUI: send edited text
ChatUI->>RightPanel: handleSend()
RightPanel->>MatrixProvider: editRoomMessage({roomId, targetEventId, message})
MatrixProvider->>MatrixProvider: validate target & load room/event
MatrixProvider->>MatrixServer: send m.room.message with m.relates_to.rel_type="m.replace"
MatrixServer-->>MatrixProvider: ack
MatrixProvider->>ChatUI: timeline receives replace event
MatrixProvider->>ChatUI: merge m.new_content into original message
ChatUI->>User: display edited message
Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/epics/src/common/human-right-panel.tsx`:
- Around line 770-780: Replace the hardcoded English error thrown when editing
with a translated string: use the i18n translator (t) to throw new
Error(t('editAttachmentsNotSupported')) in the block that checks
editTargetEventId and savedAttachments (around the savedAttachments.length > 0
check and before calling matrixRef.current.editRoomMessage). Add the
"editAttachmentsNotSupported" key to the i18n resource files with the provided
message so translations are available.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: ffef5306-b968-4305-b957-68f5d9e8f217
📒 Files selected for processing (10)
packages/core/src/matrix/client/providers/matrix-provider.tsxpackages/epics/src/common/human-chat-panel/human-chat-panel-chat-bar.tsxpackages/epics/src/common/human-chat-panel/human-chat-panel-message-bubble.tsxpackages/epics/src/common/human-chat-panel/human-chat-panel-messages.tsxpackages/epics/src/common/human-right-panel.tsxpackages/i18n/src/messages/de.jsonpackages/i18n/src/messages/en.jsonpackages/i18n/src/messages/es.jsonpackages/i18n/src/messages/fr.jsonpackages/i18n/src/messages/pt.json
Wire m.replace edits for own plain text messages: pencil between react and reply, composer edit preview, and editRoomMessage on the Matrix provider. Co-authored-by: webguru-hypha <webguru-hypha@users.noreply.github.com>
Timeline listener now maps replacement events to the target message id and re-emits the parent row; initial room load skips standalone replace events so the list does not duplicate edits. Co-authored-by: webguru-hypha <webguru-hypha@users.noreply.github.com>
When the SDK attaches a replacingEvent before the parent wire updates, merge m.new_content into the root message payload so the first edit reflects in the timeline without a second send. Co-authored-by: webguru-hypha <webguru-hypha@users.noreply.github.com>
alexprate
force-pushed
the
cursor/fix-matrix-edit-timeline-merge-ef7a
branch
from
April 13, 2026 00:39
3f813a7 to
92433e9
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
packages/epics/src/common/human-right-panel.tsx (1)
834-838:⚠️ Potential issue | 🟡 MinorMove this edit error into i18n.
This message is surfaced back to the composer via
sendFailedWithReason, so the raw English string leaks into the UI. Use a translation key here like the other composer errors. As per coding guidelines "i18n: all user-facing text uses translation keys".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/epics/src/common/human-right-panel.tsx` around lines 834 - 838, Replace the hard-coded English error thrown when editTargetEventId && savedAttachments.length > 0 with a translated message key (follow existing composer error keys pattern) and pass the translated string into sendFailedWithReason instead of the raw text; update human-right-panel.tsx to use the project's i18n utility (e.g., useTranslation/t or i18n.t) and a key like "composer.errors.attachmentsNotSupportedWhenEditing" (or the matching existing composer errors namespace) so the UI receives a localized message rather than the raw English string.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/core/src/matrix/client/providers/matrix-provider.tsx`:
- Around line 882-908: The redaction path must treat redacted edit events like
the incoming-edit branch: when handling a RoomRedaction, detect if the redacted
event is an edit (use getRoomMessageReplaceTargetEventId on the redacted event
or inspect its content.relates_to.rel_type === 'm.replace'), then find the
original target event via room.findEventById(targetId) and rebuild the message
using messageFromRoomMessageEvent (pass pinned status from
getPinnedMessageIds(roomId)), attachReactionsToMessage(room, message,
client.getUserId()), and call messageListener(message) with the rebuilt edit
mapped onto the original event id; otherwise fall back to the existing
redaction-to-plain-message behavior. Ensure you reference RoomRedaction handling
code, use client.getUserId(), and preserve the existing early return logic.
In `@packages/core/src/matrix/rich-reply.ts`:
- Around line 249-260: When applying a partial edit (see replacingEv,
newContent, and content), avoid preserving stale rich HTML metadata by ensuring
that if newContent does not include 'format' or 'formatted_body' we do not carry
those keys from rootContent into the merged content; modify the merge so you
build the final content from rootContent plus newContent but explicitly remove
or omit rootContent.format and rootContent.formatted_body when those keys are
absent in newContent so formattedContentHtml cannot render pre-edit HTML after a
plain-text edit.
---
Duplicate comments:
In `@packages/epics/src/common/human-right-panel.tsx`:
- Around line 834-838: Replace the hard-coded English error thrown when
editTargetEventId && savedAttachments.length > 0 with a translated message key
(follow existing composer error keys pattern) and pass the translated string
into sendFailedWithReason instead of the raw text; update human-right-panel.tsx
to use the project's i18n utility (e.g., useTranslation/t or i18n.t) and a key
like "composer.errors.attachmentsNotSupportedWhenEditing" (or the matching
existing composer errors namespace) so the UI receives a localized message
rather than the raw English string.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 91b4f2f8-3391-41ef-8307-12323af415f1
📒 Files selected for processing (11)
packages/core/src/matrix/client/providers/matrix-provider.tsxpackages/core/src/matrix/rich-reply.tspackages/epics/src/common/human-chat-panel/human-chat-panel-chat-bar.tsxpackages/epics/src/common/human-chat-panel/human-chat-panel-message-bubble.tsxpackages/epics/src/common/human-chat-panel/human-chat-panel-messages.tsxpackages/epics/src/common/human-right-panel.tsxpackages/i18n/src/messages/de.jsonpackages/i18n/src/messages/en.jsonpackages/i18n/src/messages/es.jsonpackages/i18n/src/messages/fr.jsonpackages/i18n/src/messages/pt.json
Port PR #2139 behavior: getMessageReplaceTargetEventId in rich-reply, timeline calls targetEv.makeReplaced before mapping, pending edit target lookup, and editRoomMessage builds m.new_content like replaceMessage (including rich-reply preservation). Revert manual m.new_content merge in messageFromRoomMessageEvent so getContent() drives display after makeReplaced. Co-authored-by: webguru-hypha <webguru-hypha@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
packages/core/src/matrix/client/providers/matrix-provider.tsx (1)
903-930:⚠️ Potential issue | 🟠 MajorMirror the new edit-remap logic in the redaction path.
Line 917 rewrites incoming edits onto the original message id, but the
RoomRedactionbranch still rebuilds a redacted edit asmessageFromRoomMessageEvent(redacted). If an edit is later redacted, that emits the edit event id instead of refreshing the original row, so the UI can keep the edited body stuck or surface a new placeholder row. Please route redactedm.replaceevents through the same target lookup used here before falling back to the genericm.room.messagehandling.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/core/src/matrix/client/providers/matrix-provider.tsx`:
- Around line 644-718: The code sends an m.replace relation using the
potentially-provisional targetEventId (from room.getPendingEvent / targetEv)
which may start with '~' and be rejected by the server; update the relation to
reference the server-assigned event ID before calling client.sendEvent: detect
provisional IDs (e.g., targetEventId.startsWith('~') or targetEv.isPending/local
echo), then either await the Matrix SDK LocalEventIdReplaced event for that
local ID (listen for LocalEventIdReplaced and capture the final eventId) or call
the SDK helper updateAssociatedId to map the local ID to the final server ID;
replace the value used in 'm.relates_to.event_id' with the resolved
server-assigned ID (use the same symbols targetEventId, targetEv,
client.sendEvent and updateAssociatedId/LocalEventIdReplaced) before sending.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: dc53f33d-ed18-42be-a0c1-48f1c8325474
📒 Files selected for processing (2)
packages/core/src/matrix/client/providers/matrix-provider.tsxpackages/core/src/matrix/rich-reply.ts
Localize edit-with-attachments error for composer display. When a replacement message is redacted, refresh the original timeline row via makeReplaced(undefined) and messageFromRoomMessageEvent. Co-authored-by: webguru-hypha <webguru-hypha@users.noreply.github.com>
alexprate
self-requested a review
April 13, 2026 01:15
alexprate
approved these changes
Apr 13, 2026
alexprate
enabled auto-merge
April 13, 2026 01:19
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
packages/core/src/matrix/client/providers/matrix-provider.tsx (1)
644-718:⚠️ Potential issue | 🟠 MajorResolve local-echo target IDs before sending
m.replace.Line 646 makes pending events editable, but Line 716 still forwards
targetEventIdverbatim intom.relates_to.event_id. If the target is still a local echo, the edit can be sent against a provisional~...id and be rejected by the homeserver. Resolve the server event id first, or wait for the SDK to remap the association before callingsendEvent.According to the Matrix message-editing spec and matrix-js-sdk, can an `m.replace` event reference a provisional/local-echo event id in `m.relates_to.event_id`, or must it use the server-assigned event id?🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/matrix/client/providers/matrix-provider.tsx` around lines 644 - 718, The edit currently sends the original targetEventId into m.relates_to.event_id which can be a provisional/local-echo id and will be rejected by the homeserver; ensure the edit references the server-assigned event id (or waits for the SDK to remap the local echo) before calling client.sendEvent. Update the logic around targetEventId/targetEv (the value derived via room.findEventById / room.getPendingEvent) to detect a provisional/local-echo id, resolve it to the server event id (e.g. by polling the room or using the SDK’s remapping API / helper you already use for replies such as resolveReplyTargetForSend) or await the SDK’s remap, then use that resolved server id when building 'm.relates_to.event_id' passed into client.sendEvent; keep all other content-building (buildRichReplyMatrixContent, matrixTextEventContentWithOptionalFormatting) unchanged.packages/epics/src/common/human-right-panel.tsx (1)
834-842:⚠️ Potential issue | 🟡 MinorSurface the edit-attachment constraint directly.
Throwing on Line 836 still falls through the generic
sendFailedWithReasonpath, so the composer shows a prefixed send failure instead of the dedicated edit-mode constraint message.💡 Proposed fix
if (editTargetEventId) { if (savedAttachments.length > 0) { - throw new Error(t('editAttachmentsNotSupported')); + setSendingPending(null); + setComposerError(t('editAttachmentsNotSupported')); + setInput(text); + setReplyDraft(savedDraft); + setEditDraft(savedEditDraft); + setDraftAttachments(savedAttachments); + sendOperationTokenRef.current = null; + return; } await matrixRef.current.editRoomMessage({ roomId, targetEventId: editTargetEventId, message: text,As per coding guidelines, "Ensure message editing components reference these exact keys (and don’t reuse unrelated reply keys) so the composer/edit preview and edit constraint message render correctly."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/epics/src/common/human-right-panel.tsx` around lines 834 - 842, When editTargetEventId is set and savedAttachments.length > 0, do not throw an Error (which falls into the generic sendFailedWithReason path); instead surface the edit-mode constraint directly by invoking the composer/edit-preview failure path with the exact key 'editAttachmentsNotSupported' and return early. Replace the throw in the block that references editTargetEventId and savedAttachments with a call that signals the edit constraint (e.g., call sendFailedWithReason or the component method that accepts a reason key) using 'editAttachmentsNotSupported', then exit before calling matrixRef.current.editRoomMessage.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/epics/src/common/human-right-panel.tsx`:
- Around line 530-531: When tearing down edit state (where you currently call
setReplyDraft(null) and setEditDraft(null)), also clear the composer input so
the previous message body doesn't persist across rooms: call the input-clearing
setter (e.g., setInput('') or the appropriate composer state reset) alongside
setReplyDraft and setEditDraft in the same places (the blocks around the
setReplyDraft/setEditDraft calls referenced in this diff, and the analogous
spots at the other two occurrences) so switching rooms won't leave the old draft
in the composer.
---
Duplicate comments:
In `@packages/core/src/matrix/client/providers/matrix-provider.tsx`:
- Around line 644-718: The edit currently sends the original targetEventId into
m.relates_to.event_id which can be a provisional/local-echo id and will be
rejected by the homeserver; ensure the edit references the server-assigned event
id (or waits for the SDK to remap the local echo) before calling
client.sendEvent. Update the logic around targetEventId/targetEv (the value
derived via room.findEventById / room.getPendingEvent) to detect a
provisional/local-echo id, resolve it to the server event id (e.g. by polling
the room or using the SDK’s remapping API / helper you already use for replies
such as resolveReplyTargetForSend) or await the SDK’s remap, then use that
resolved server id when building 'm.relates_to.event_id' passed into
client.sendEvent; keep all other content-building (buildRichReplyMatrixContent,
matrixTextEventContentWithOptionalFormatting) unchanged.
In `@packages/epics/src/common/human-right-panel.tsx`:
- Around line 834-842: When editTargetEventId is set and savedAttachments.length
> 0, do not throw an Error (which falls into the generic sendFailedWithReason
path); instead surface the edit-mode constraint directly by invoking the
composer/edit-preview failure path with the exact key
'editAttachmentsNotSupported' and return early. Replace the throw in the block
that references editTargetEventId and savedAttachments with a call that signals
the edit constraint (e.g., call sendFailedWithReason or the component method
that accepts a reason key) using 'editAttachmentsNotSupported', then exit before
calling matrixRef.current.editRoomMessage.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 8027a395-8878-4e38-a89c-87820b1fdae7
📒 Files selected for processing (7)
packages/core/src/matrix/client/providers/matrix-provider.tsxpackages/epics/src/common/human-right-panel.tsxpackages/i18n/src/messages/de.jsonpackages/i18n/src/messages/en.jsonpackages/i18n/src/messages/es.jsonpackages/i18n/src/messages/fr.jsonpackages/i18n/src/messages/pt.json
…om switch Wait for LocalEventIdReplaced before sending m.replace so relates_to uses a server id. Extract awaitNonProvisionalMatrixEventId for reuse with reply targets. Clear composer input and errors when tearing down chat state across modes. Co-authored-by: webguru-hypha <webguru-hypha@users.noreply.github.com>
This was referenced Apr 13, 2026
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
Problem (PR #2145)
Edits send a new
m.room.messagewithm.relates_tom.replace, but the timeline listener always emittedmessage.id ===the new event id. The UI then appended a second row (often showing the literal* …fallback body) instead of updating the original message.Fix
In
matrix-provider.tsx:getRoomMessageReplaceTargetEventId— Detectsm.replaceroom messages.RoomEvent.Timelinehandler — When a replacement event arrives, resolve the target event, runmessageFromRoomMessageEventon that (sogetContent()reflects applied edits), attach reactions, and notify listeners withMessage.id= original event id soHumanRightPanelreplaces the existing row.getRoomMessages— Skips standalone replacement events so the initial load list does not show duplicate “edit” rows.Testing
pnpm run format:fixpnpm --filter @hypha-platform/core testpnpm --filter @hypha-platform/epics check-typesRelation to #2145
This branch is based on
pull/2145/headand adds the commit above. You can merge it instead of #2145 once reviewed, or cherry-pick the fix commit onto #2145 if you prefer to keep that PR’s thread.Summary by CodeRabbit
New Features
Behavior
Internationalization