Human chat: combined media captions, composer drop, redaction & scroll - #2157
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds audio attachment support and durations, media-aware send/edit flows (including editing existing media slots), makes room message retrieval/timeline listening redaction-aware, enhances chat composer with drag‑and‑drop and dictation/voice recording, introduces an emoji picker surface, and updates message rendering and scrolling. Changes
Sequence Diagram(s)sequenceDiagram
participant UI as Client UI
participant Provider as MatrixProvider
participant SDK as matrix-js-sdk
participant Storage as MediaStore
UI->>Provider: sendMessage(text, attachments?, replyTo?, editParams?)
alt attachments present
Provider->>Provider: detect kinds (image/file/audio), loadAudioDurationMs for audio
Provider->>Storage: upload media blob -> receive mxc_url + info
Storage-->>Provider: mxc_url, info (incl. duration)
Provider->>Provider: build media bundle (msgtype, mediaInfo, formatted_body if caption)
Provider->>SDK: send m.room.message (media bundle / rich-reply / m.relates_to if reply)
SDK-->>Provider: eventId
Provider->>UI: commit media event result (eventId)
else no attachments
Provider->>SDK: send plain or HTML message (apply rich-reply markup if reply)
SDK-->>Provider: eventId
Provider->>UI: commitResult(eventId)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 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 |
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/core/src/matrix/client/providers/matrix-provider.tsx (1)
1090-1137:⚠️ Potential issue | 🟠 MajorEmit a synthetic removal for ordinary room-message redactions here too.
This branch still converts a redacted
m.room.messageback into a normalMessage, so the listener can reinsert/update the empty row you just taught theRoomMessagepath to remove. Keep the edit-redaction handling, but for non-m.replaceroom-message redactions emit{ redacted: true }instead of callingmessageFromRoomMessageEvent(...).♻️ Proposed fix
} else if (redacted.getType() === EventType.RoomMessage) { const replaceTargetId = getMessageReplaceTargetEventId(redacted); if (replaceTargetId) { const targetEv = room.findEventById(replaceTargetId) ?? (typeof room.getPendingEvent === 'function' ? room.getPendingEvent(replaceTargetId) : null); if (!targetEv || targetEv.getType() !== EventType.RoomMessage) { return; } const pinnedIds = getPinnedMessageIds(roomId); const targetEventId = targetEv.getId(); const targetSender = targetEv.getSender(); if (!targetEventId || !targetSender) return; targetEv.makeReplaced(undefined); let message = messageFromRoomMessageEvent( client, roomId, targetEv, pinnedIds.includes(targetEventId), ); message = attachReactionsToMessage( room, message, client.getUserId(), ); await messageListener(message); return; } - const pinnedIds = getPinnedMessageIds(roomId); - const mid = redacted.getId(); - const ms = redacted.getSender(); - if (!mid || !ms) return; - let message = messageFromRoomMessageEvent( - client, - roomId, - redacted, - pinnedIds.includes(mid), - ); - message = attachReactionsToMessage( - room, - message, - client.getUserId(), - ); - await messageListener(message); + const mid = redacted.getId(); + if (!mid) return; + await messageListener({ + id: mid, + sender: redacted.getSender() ?? '', + content: '', + timestamp: new Date(redacted.getTs()), + redacted: true, + }); }🤖 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 1090 - 1137, The redaction handling for ordinary m.room.message events currently rebuilds a full Message via messageFromRoomMessageEvent and sends it to messageListener; instead, when getMessageReplaceTargetEventId(redacted) is falsy, construct and emit a synthetic "removal" message with redacted: true so the listener can remove the row. Concretely: inside the else branch where mid = redacted.getId() and ms = redacted.getSender(), keep the pinnedIds computation but do NOT call messageFromRoomMessageEvent or attachReactionsToMessage; instead create a minimal payload containing the message id (mid), sender (ms), roomId, redacted: true, and any pinned flag (pinnedIds.includes(mid)) and pass that to await messageListener(...). Leave the existing edit-redaction path (the getMessageReplaceTargetEventId branch) unchanged.
🤖 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-chat-panel/human-chat-panel-chat-bar.tsx`:
- Around line 711-750: Move the e.preventDefault() call to the top of the drag
handlers so the browser default file-open/navigation is suppressed even when
onDraftAttachmentsChange is not provided: specifically, in the onDrop and
onDragOver handlers (the functions that call setComposerDragDepth,
e.dataTransfer.dropEffect and pushDrafts) call e.preventDefault() immediately on
entry before returning early for the onDraftAttachmentsChange guard; preserve
the existing logic that sets dropEffect, resets composer drag depth, and calls
pushDrafts(files, 'file') when appropriate.
In `@packages/epics/src/common/human-right-panel.tsx`:
- Around line 142-145: The captionForMedia logic is treating the upload filename
as a caption; update it so media captions are only used when the text is not
just the attachment filename. Specifically, in the captionForMedia computation
(and the analogous logic at the other spot referenced), strip the reply fallback
with stripMatrixReplyFallback(msg.content).trim() and then ignore it (treat as
empty) if it exactly equals the attachment filename (derive filename from
msg.content.info?.name or fallback to msg.content.body as used for
m.file/m.image). Keep the isMedia check, and ensure duplicated filename text no
longer becomes a caption/part for uncaptioned uploads.
In `@packages/i18n/src/messages/pt.json`:
- Line 1658: composerDropPrompt uses "ficheiros" while nearby keys like
composerAttachFile use the variant "Arquivo"; update composerDropPrompt to the
same Portuguese variant for consistency (e.g., change "Solte ficheiros para
anexar" to use "arquivos" such as "Solte arquivos para anexar") and scan the
surrounding chat-section keys (composerAttachFile, composerDropPrompt) to ensure
both use the same term variant.
---
Outside diff comments:
In `@packages/core/src/matrix/client/providers/matrix-provider.tsx`:
- Around line 1090-1137: The redaction handling for ordinary m.room.message
events currently rebuilds a full Message via messageFromRoomMessageEvent and
sends it to messageListener; instead, when
getMessageReplaceTargetEventId(redacted) is falsy, construct and emit a
synthetic "removal" message with redacted: true so the listener can remove the
row. Concretely: inside the else branch where mid = redacted.getId() and ms =
redacted.getSender(), keep the pinnedIds computation but do NOT call
messageFromRoomMessageEvent or attachReactionsToMessage; instead create a
minimal payload containing the message id (mid), sender (ms), roomId, redacted:
true, and any pinned flag (pinnedIds.includes(mid)) and pass that to await
messageListener(...). Leave the existing edit-redaction path (the
getMessageReplaceTargetEventId branch) unchanged.
🪄 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: d9e5bf4a-6161-45ae-801e-171e25452385
📒 Files selected for processing (12)
packages/core/src/matrix/client/providers/matrix-provider.tsxpackages/core/src/matrix/rich-reply.tspackages/core/src/matrix/types.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
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 683-686: When removing a redacted message inside the setMessages
callback (check message.redacted and message.id), also clear any active
reply/edit state that targets that message: if replyDraft?.targetId ===
message.id or editDraft?.id === message.id, call the corresponding setters (e.g.
setReplyDraft(null) and setEditDraft(null) or clear the draft objects) so the
composer doesn't reference a deleted event. Update the same block where
setMessages filters out the redacted message to perform these conditional clears
atomically with the removal.
🪄 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: 1add3397-49a8-437c-9139-806f702acdd3
📒 Files selected for processing (3)
packages/epics/src/common/human-chat-panel/human-chat-panel-chat-bar.tsxpackages/epics/src/common/human-right-panel.tsxpackages/i18n/src/messages/pt.json
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/core/src/matrix/rich-reply.ts (1)
370-382:⚠️ Potential issue | 🟠 MajorPreserve
info.durationon the primary media item too.
mapWireToMediaInfo()now carriesduration, but the rootmediaInfohere is still rebuilt by hand without it. Single audio messages will therefore lose their duration while bundled items keep it.Proposed fix
- mediaInfo: info - ? { - mimetype: info.mimetype, - size: info.size, - w: info.w, - h: info.h, - } - : undefined, + mediaInfo: mapWireToMediaInfo(content),Also applies to: 446-453
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/matrix/rich-reply.ts` around lines 370 - 382, When constructing the root mediaInfo for the primary media item, include duration from mapWireToMediaInfo instead of rebuilding mediaInfo manually; call mapWireToMediaInfo(content) (or extract its duration) and merge its duration into the root mediaInfo object so single audio messages keep duration, and apply the same fix to the second occurrence around the mediaInfo build at the other block (lines 446-453) to ensure both places include duration.packages/epics/src/common/human-chat-panel/human-chat-panel-chat-bar.tsx (1)
555-560:⚠️ Potential issue | 🟠 MajorUse the same send gate for Enter submissions.
The new
editMediaModerule only disables the send button. Pressing Enter still callsonSend()when there is text but no attachment row left, so media edits can submit the invalid state this change is trying to prevent.Proposed fix
- if (e.key === 'Enter' && !e.shiftKey && !e.nativeEvent.isComposing) { + if (e.key === 'Enter' && !e.shiftKey && !e.nativeEvent.isComposing) { e.preventDefault(); - if (value.trim().length > 0 || draftAttachments.length > 0) { + if (canSend) { onSend(); } }Also applies to: 563-565
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/epics/src/common/human-chat-panel/human-chat-panel-chat-bar.tsx` around lines 555 - 560, The Enter key handler currently calls onSend() based only on value.trim() or draftAttachments, which bypasses the new editMediaMode restriction; update the keydown handler in human-chat-panel-chat-bar (the function handling e.key === 'Enter') to reuse the same send-gate logic used for the send button (the editMediaMode check / isSendEnabled condition) so that onSend() is only invoked when that gate permits sending (i.e., guard the Enter path with the same editMediaMode/send-enabled boolean before calling onSend); apply the same change to the other Enter handler location noted in the diff.
♻️ Duplicate comments (1)
packages/epics/src/common/human-right-panel.tsx (1)
833-836:⚠️ Potential issue | 🟠 MajorClear active reply/edit state when a message is redacted remotely.
This branch removes the row from the messages state, but
replyDraftandeditDraftcan still point at the deleted event. The self-delete path inhandleDeleteMessage(lines 1020-1026) already clears those states; remote redactions should do the same, or the composer may reference a non-existent target.💡 Suggested fix
registerRoomListener( roomId, async (message: Message) => { - setMessages((prev) => { - if (message.redacted) { - return prev.filter((m) => m.id !== message.id); - } + if (message.redacted) { + setMessages((prev) => prev.filter((m) => m.id !== message.id)); + setReplyDraft((draft) => + draft?.messageId === message.id ? null : draft, + ); + setEditDraft((draft) => { + if (draft?.messageId !== message.id) return draft; + setInput(''); + return null; + }); + return; + } + setMessages((prev) => { const next = toUIMessage(,
🤖 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 833 - 836, When handling a remote redaction inside the setMessages callback (the branch that filters out message when message.redacted), also clear any active composer targets so replyDraft/editDraft don't reference the removed event: mirror the self-delete logic in handleDeleteMessage by checking if replyDraft?.id === message.id and if so call setReplyDraft(null), and likewise if editDraft?.id === message.id call setEditDraft(null) before returning the filtered messages.
🤖 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 796-861: The prepareMediaPayload implementation is duplicated in
sendMessage and editRoomMessage; extract it to a single module-scope helper
(e.g., async function prepareMediaPayload(client: MatrixSdk.MatrixClient, att:
SendAttachmentInput): Promise<HyphaMediaEventContent>) and replace both inline
copies with calls to this helper. Keep the existing behavior: create
AbortController, use MATRIX_UPLOAD_TIMEOUT_MS and setTimeout/clearTimeout, call
client.uploadContent with { name, type, abortController }, throw
MatrixUploadTimeoutError on abort, compute msgtype (MsgType.Image/Audio/File),
populate info (mimetype, size, w, h, duration) using
loadImageDimensions/loadAudioDurationMs, set filename/body/url from att.file and
upload.content_uri, and preserve HYPHA_SPOILER_FIELD when att.spoiler. Ensure
the new helper has access to the same constants and types/imports used by the
original blocks.
In `@packages/epics/src/common/human-chat-panel/chat-panel-media-types.ts`:
- Around line 18-28: The AUDIO_FILE_EXTENSIONS set incorrectly includes 'webm',
causing MIME-less .webm attachments to be classified as audio; remove 'webm'
from AUDIO_FILE_EXTENSIONS and ensure 'webm' remains (or is present) in the
VIDEO_FILE_EXTENSIONS set so extension-only classification treats .webm as
video; update all occurrences where AUDIO_FILE_EXTENSIONS is defined/used
(including the other two similar blocks referenced) and any helpers that rely on
these sets so they consult VIDEO_FILE_EXTENSIONS for 'webm' instead of
AUDIO_FILE_EXTENSIONS.
In
`@packages/epics/src/common/human-chat-panel/human-chat-panel-emoji-mart-surface.tsx`:
- Around line 35-37: pickerLocale currently checks locale from useLocale()
verbatim so regional tags like "pt-BR" or "en-US" fall back to 'en'; normalize
by extracting the base language before comparison (e.g., split locale on '-' or
'_' and take the first segment) and use that normalizedBase when computing
pickerLocale (the variable and logic around pickerLocale and the value returned
from useLocale() should be updated). Ensure the same normalization pattern used
in date-fns-locale is applied so supported base languages
('en','es','fr','de','pt') map correctly.
In
`@packages/epics/src/common/human-chat-panel/human-chat-panel-message-bubble.tsx`:
- Around line 1322-1327: The conditional in the message bubble is performing a
redundant check for audio: remove the explicit `message.media.msgtype ===
'm.audio'` clause and rely solely on `isChatPanelAudioFile(message.media)` when
deciding to render `<TimelineVoiceSlot>`; update the expression that currently
reads `message.media.msgtype === 'm.audio' ||
isChatPanelAudioFile(message.media)` to just call
`isChatPanelAudioFile(message.media)` so `TimelineVoiceSlot` rendering logic
uses the single canonical check.
- Around line 495-497: The empty catch in onLoadedMetadata silently swallows
errors; update the catch to either log the error at debug/trace level using the
component's logger or add a clear comment explaining why errors are
intentionally ignored. Locate the onLoadedMetadata handler in
human-chat-panel-message-bubble.tsx and replace the bare catch block with a call
to the existing debug logger (or console.debug if no logger exists) that
includes the caught error, or add a concise justification comment above the
catch if swallowing is intentional.
---
Outside diff comments:
In `@packages/core/src/matrix/rich-reply.ts`:
- Around line 370-382: When constructing the root mediaInfo for the primary
media item, include duration from mapWireToMediaInfo instead of rebuilding
mediaInfo manually; call mapWireToMediaInfo(content) (or extract its duration)
and merge its duration into the root mediaInfo object so single audio messages
keep duration, and apply the same fix to the second occurrence around the
mediaInfo build at the other block (lines 446-453) to ensure both places include
duration.
In `@packages/epics/src/common/human-chat-panel/human-chat-panel-chat-bar.tsx`:
- Around line 555-560: The Enter key handler currently calls onSend() based only
on value.trim() or draftAttachments, which bypasses the new editMediaMode
restriction; update the keydown handler in human-chat-panel-chat-bar (the
function handling e.key === 'Enter') to reuse the same send-gate logic used for
the send button (the editMediaMode check / isSendEnabled condition) so that
onSend() is only invoked when that gate permits sending (i.e., guard the Enter
path with the same editMediaMode/send-enabled boolean before calling onSend);
apply the same change to the other Enter handler location noted in the diff.
---
Duplicate comments:
In `@packages/epics/src/common/human-right-panel.tsx`:
- Around line 833-836: When handling a remote redaction inside the setMessages
callback (the branch that filters out message when message.redacted), also clear
any active composer targets so replyDraft/editDraft don't reference the removed
event: mirror the self-delete logic in handleDeleteMessage by checking if
replyDraft?.id === message.id and if so call setReplyDraft(null), and likewise
if editDraft?.id === message.id call setEditDraft(null) before returning the
filtered messages.
🪄 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: 9acf95fe-e8ec-4157-a3ea-3e9d0f90c944
📒 Files selected for processing (17)
packages/core/src/matrix/client/providers/matrix-provider.tsxpackages/core/src/matrix/rich-reply.tspackages/core/src/matrix/types.tspackages/epics/src/common/human-chat-panel/chat-panel-media-types.tspackages/epics/src/common/human-chat-panel/human-chat-panel-chat-bar.tsxpackages/epics/src/common/human-chat-panel/human-chat-panel-emoji-mart-surface.tsxpackages/epics/src/common/human-chat-panel/human-chat-panel-emoji-picker.tsxpackages/epics/src/common/human-chat-panel/human-chat-panel-message-bubble.tsxpackages/epics/src/common/human-chat-panel/human-chat-panel-message-overflow.tsxpackages/epics/src/common/human-chat-panel/human-chat-panel-messages.tsxpackages/epics/src/common/human-right-panel.tsxpackages/epics/src/people/components/person-avatar.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
Send composer text on the first attachment Matrix event, map media body and formatted HTML into the UI model, render captions above assets, skip fully redacted room messages in history, and remove rows on redaction. Co-authored-by: webguru-hypha <webguru-hypha@users.noreply.github.com>
Accept file drops on the composer shell with visual feedback and i18n prompt across supported locales. Co-authored-by: webguru-hypha <webguru-hypha@users.noreply.github.com>
Only auto-scroll to bottom when the user is already near the end or a new message was appended, so deletions no longer jump the viewport. Co-authored-by: webguru-hypha <webguru-hypha@users.noreply.github.com>
Suppress default browser behavior on file drags before optional handlers; avoid treating bare media filenames as captions; align pt composerDropPrompt. Co-authored-by: webguru-hypha <webguru-hypha@users.noreply.github.com>
…ssage CI check-types failed: rich.format is typed as string but Matrix expects org.matrix.custom.html. Use MATRIX_CUSTOM_HTML_FORMAT when merging reply caption into the first media event. Co-authored-by: webguru-hypha <webguru-hypha@users.noreply.github.com>
RoomMessageEventContent for m.image/m.file omits format/formatted_body; merging rich-reply or markup captions into the first slot needs those fields. Extend HyphaMediaEventContent so check-types passes. Co-authored-by: webguru-hypha <webguru-hypha@users.noreply.github.com>
Nested Popover inside DropdownMenu/ContextMenu items dismissed the parent when moving the pointer to the portaled picker. Use Radix menu Sub with emoji-mart in SubContent and extract HumanChatPanelEmojiMartSurface for reuse. Co-authored-by: webguru-hypha <webguru-hypha@users.noreply.github.com>
Co-authored-by: webguru-hypha <webguru-hypha@users.noreply.github.com>
Extend Message and bundle wire types for m.audio, parse duration in media info, upload audio attachments with MsgType.Audio and optional duration from local file metadata. Co-authored-by: webguru-hypha <webguru-hypha@users.noreply.github.com>
…yback Add dropdown on mic: Web Speech dictation into composer vs MediaRecorder voice as audio draft; classify audio MIME separately from video; render voice with native audio play/pause; send audio kind as Matrix m.audio. Co-authored-by: webguru-hypha <webguru-hypha@users.noreply.github.com>
Co-authored-by: webguru-hypha <webguru-hypha@users.noreply.github.com>
Timeline videos use poster frame, gradient title, red play button, then native controls after play; unmute strip while playing. Draft video preview raises play overlay z-index so the button receives clicks. Co-authored-by: webguru-hypha <webguru-hypha@users.noreply.github.com>
Extend editRoomMessage with existingMediaSlots and newAttachments; rebuild Hypha media/bundle content and send RelationType.Replace for m.file/m.image/m.audio roots while reusing MXC for kept slots. Co-authored-by: webguru-hypha <webguru-hypha@users.noreply.github.com>
Enable edit for user messages with media; load MXC thumbnails into draft strip with editSlot metadata; require one attachment; append new uploads via editRoomMessage newAttachments. Co-authored-by: webguru-hypha <webguru-hypha@users.noreply.github.com>
Extend PersonAvatar with optional squircle radius (~Discord) and dedicated timeline/reply dimensions for human chat. Co-authored-by: webguru-hypha <webguru-hypha@users.noreply.github.com>
Use room member MXC avatars (cropped HTTP thumbs) for other users and quoted authors; refresh when the room loads. Co-authored-by: webguru-hypha <webguru-hypha@users.noreply.github.com>
Render quoted author with small squircle avatar and @ handle, move reply above the sender line, and draw a curved SVG thread from the main avatar. Co-authored-by: webguru-hypha <webguru-hypha@users.noreply.github.com>
Assert HyphaMediaEventContent when merging rich replies into media edits, use existing slot filename for caption-clear body fallback, and remove onCloseAutoFocus from DropdownMenuSubContent (not in Radix types). Co-authored-by: webguru-hypha <webguru-hypha@users.noreply.github.com>
Use full-round avatars in human chat, move reply preview above the avatar column so the main avatar aligns with the sender line, and retune the SVG reply connector. Reposition the hover action bar when a reply is present. Co-authored-by: webguru-hypha <webguru-hypha@users.noreply.github.com>
fde694b to
3cd63a0
Compare
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/core/src/matrix/rich-reply.ts (1)
285-290:⚠️ Potential issue | 🟡 MinorPreserve
durationfor standalone audio events.The bundle path now forwards
info.duration, but the directmediaInforeturn still only copiesmimetype/size/w/h. Singlem.audiomessages will therefore render without the duration thatTimelineVoiceSlotexpects.🛠️ Suggested fix
info?: { mimetype?: string; size?: number; w?: number; h?: number; + duration?: number; }; @@ mediaInfo: info ? { mimetype: info.mimetype, size: info.size, w: info.w, h: info.h, + duration: info.duration, } : undefined,Also applies to: 446-453
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/matrix/rich-reply.ts` around lines 285 - 290, The media info returned by the rich-reply code omitted info.duration for standalone audio, causing TimelineVoiceSlot (and m.audio messages) to lack duration; update the mediaInfo construction in packages/core/src/matrix/rich-reply.ts (both the block around the info?: { mimetype/size/w/h } declaration and the other occurrence at the second block referenced) to copy info.duration through whenever present (i.e., include duration alongside mimetype, size, w, h) so that m.audio messages and TimelineVoiceSlot receive the duration field.
♻️ Duplicate comments (4)
packages/epics/src/common/human-chat-panel/human-chat-panel-emoji-mart-surface.tsx (1)
35-37:⚠️ Potential issue | 🟡 MinorNormalize locale before emoji-mart locale matching.
On Line 35, the exact-match check can downgrade regional locales (e.g.,
en-US,pt-BR) to English instead of their supported base language.♻️ Proposed fix
- const pickerLocale = ['en', 'es', 'fr', 'de', 'pt'].includes(locale) - ? locale + const normalizedLocale = locale.toLowerCase().split(/[-_]/)[0]; + const pickerLocale = ['en', 'es', 'fr', 'de', 'pt'].includes(normalizedLocale) + ? normalizedLocale : 'en';For next-intl v4.8.3, can useLocale() return locale identifiers with region subtags like "en-US" or "pt-BR"?🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/epics/src/common/human-chat-panel/human-chat-panel-emoji-mart-surface.tsx` around lines 35 - 37, The locale check for pickerLocale currently does an exact match against supported locales and downgrades region-tagged locales (like "en-US" or "pt-BR") to 'en'; normalize the incoming locale first by extracting the base language subtag (e.g., const base = (locale || '').split('-')[0].toLowerCase()) and then set pickerLocale = ['en','es','fr','de','pt'].includes(base) ? base : 'en' so region subtags are correctly matched; update references in human-chat-panel-emoji-mart-surface.tsx where pickerLocale and locale are used to rely on the normalized base language.packages/epics/src/common/human-chat-panel/chat-panel-media-types.ts (1)
18-28:⚠️ Potential issue | 🟠 MajorRemove
.webmfrom the audio extension set.MIME-less
.webmattachments now satisfy both the audio and video classifiers, and the audio branch wins first downstream. That misroutes ordinary videos into the voice-message UI whenever the browser/server omits a precise MIME type.🔧 Suggested fix
const AUDIO_FILE_EXTENSIONS = new Set([ - 'webm', 'ogg', 'oga', 'opus', 'mp3',🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/epics/src/common/human-chat-panel/chat-panel-media-types.ts` around lines 18 - 28, AUDIO_FILE_EXTENSIONS currently includes 'webm', which causes MIME-less .webm files to be classified as audio and routed to voice-message UI; remove 'webm' from the AUDIO_FILE_EXTENSIONS Set in chat-panel-media-types.ts (refer to the constant AUDIO_FILE_EXTENSIONS) so .webm falls through to the video classifier instead, then run tests or verify downstream classification logic to confirm videos are no longer misrouted.packages/core/src/matrix/client/providers/matrix-provider.tsx (1)
797-862: 🛠️ Refactor suggestion | 🟠 MajorExtract
prepareMediaPayloadto avoid duplication.The
prepareMediaPayloadfunction is duplicated betweensendMessage(lines 476-542) andeditRoomMessage(lines 797-862). Both implementations are nearly identical. Extract this to a shared helper at module scope to reduce maintenance burden and ensure consistent behavior.♻️ Suggested refactor
// At module scope (around line 185) async function prepareMediaPayload( client: MatrixSdk.MatrixClient, att: SendAttachmentInput, ): Promise<HyphaMediaEventContent> { const abortController = new AbortController(); const timeoutMs = MATRIX_UPLOAD_TIMEOUT_MS; const timeoutId = setTimeout(() => abortController.abort(), timeoutMs); let upload: { content_uri: string }; try { upload = await client.uploadContent(att.file, { name: att.file.name, type: att.file.type || undefined, abortController, }); } catch (e) { if (abortController.signal.aborted) { throw new MatrixUploadTimeoutError( `Matrix media upload timed out after ${timeoutMs}ms`, ); } throw e; } finally { clearTimeout(timeoutId); } const mxc = upload.content_uri; const msgtype = att.kind === 'image' ? MsgType.Image : att.kind === 'audio' ? MsgType.Audio : MsgType.File; let info: { mimetype?: string; size?: number; w?: number; h?: number; duration?: number; } = { mimetype: att.file.type || undefined, size: att.file.size, }; if (msgtype === MsgType.Image) { const dims = await loadImageDimensions(att.file); if (dims) { info = { ...info, w: dims.w, h: dims.h }; } } else if (msgtype === MsgType.Audio) { const dur = await loadAudioDurationMs(att.file); if (dur != null) { info = { ...info, duration: dur }; } } const base: HyphaMediaEventContent = { msgtype, body: att.file.name, filename: att.file.name, url: mxc, info, } as HyphaMediaEventContent; if (att.spoiler) { base[HYPHA_SPOILER_FIELD] = true; } return base; }Then use
prepareMediaPayload(client, att)in bothsendMessageandeditRoomMessage.🤖 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 797 - 862, The prepareMediaPayload implementation is duplicated in sendMessage and editRoomMessage—extract it to a single module-scope helper named prepareMediaPayload that accepts the Matrix client and the SendAttachmentInput (e.g., prepareMediaPayload(client: MatrixSdk.MatrixClient, att: SendAttachmentInput): Promise<HyphaMediaEventContent>), preserve the abortController/timeout logic using MATRIX_UPLOAD_TIMEOUT_MS and MatrixUploadTimeoutError, keep usage of MsgType, loadImageDimensions, loadAudioDurationMs, HYPHA_SPOILER_FIELD and HyphaMediaEventContent, and then replace the inline implementations in both sendMessage and editRoomMessage with calls to the new helper (prepareMediaPayload(client, att)).packages/epics/src/common/human-right-panel.tsx (1)
833-852:⚠️ Potential issue | 🟠 MajorClear active reply/edit state when a message is redacted.
The redaction handling at lines 835-837 removes the message from
messagesstate, but it does not clearreplyDraftoreditDraftif they reference the redacted message. The self-delete path (lines 1020-1027) already handles this; the timeline listener should do the same to prevent the composer from targeting a non-existent event.💡 Suggested fix
async (message: Message) => { - setMessages((prev) => { - if (message.redacted) { - return prev.filter((m) => m.id !== message.id); - } + if (message.redacted) { + setMessages((prev) => prev.filter((m) => m.id !== message.id)); + setReplyDraft((draft) => + draft?.messageId === message.id ? null : draft, + ); + setEditDraft((draft) => { + if (draft?.messageId !== message.id) return draft; + setInput(''); + return null; + }); + return; + } + setMessages((prev) => { const next = toUIMessage(🤖 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 833 - 852, When handling a redacted message in the timeline listener (the async callback that calls setMessages and checks message.redacted), also clear any active reply or edit state that targets that message: update replyDraft and editDraft (via their setters, e.g., setReplyDraft and setEditDraft) to null/empty if their referenced message id equals message.id so the composer doesn't keep targeting a non-existent event; perform this alongside the existing removal logic in the same redaction branch.
🤖 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 963-993: Add a unit/integration test that covers editing a media
message where replyToId is present but the new caption is empty so the code path
in matrix-provider.tsx that calls resolveReplyTargetForSend and then
buildRichReplyMatrixContent with a single-space placeholder (' ') is exercised;
the test should assert that the resulting event content (HyphaMediaEventContent)
preserves the 'm.relates_to' -> 'm.in_reply_to' -> event_id and that
formatted_body/body are set from buildRichReplyMatrixContent (i.e., reply
association is preserved) to prevent regressions to the reply-preservation
logic.
In `@packages/epics/src/common/human-chat-panel/human-chat-panel-chat-bar.tsx`:
- Around line 784-788: The issue is that trailing zero-width spaces (\u200b)
used as dictation markers can remain in valueRef.current during active dictation
and get sent if Enter is pressed; update the onresult handler (the function that
builds next from valueRef.current and piece) to strip any trailing zero-width
spaces before computing next (e.g., use valueRef.current.replace(/\u200b+$/, '')
or otherwise remove the marker pattern) and then set valueRef.current = next and
call onChange(next); alternatively remove the marker insertion entirely and rely
on valueRef tracking so no \u200b is ever added.
In
`@packages/epics/src/common/human-chat-panel/human-chat-panel-message-bubble.tsx`:
- Around line 408-410: The current rendering uses
media.filename?.replace(/^voice-message-\d+\./, '') which collapses generated
names like "voice-message-123.webm" to just "webm"; update the logic around
media.filename in the HumanChatPanelMessageBubble component so that if
media.filename matches the autogenerated pattern /^voice-message-\d+\./ you
display t('voiceMessage') instead of the replacement, otherwise show the cleaned
filename (or the original filename). Reference media.filename and the existing
regex used in the replace call and ensure t('voiceMessage') is used as the
fallback for autogenerated voice filenames.
In `@packages/epics/src/common/human-chat-panel/human-chat-panel-messages.tsx`:
- Around line 102-108: The current append detection treats any increase in len
as an append, which misfires when older messages are prepended; change the logic
in the appended calculation so it only considers the tail actually changed
(e.g., compare lastId/prevLastId or last message timestamp) rather than len >
prevLen. Update the condition that sets stickToBottomRef.current = true to
require lastId !== prevLastId (and non-null) or an explicit tail-change check,
keeping the existing same-length check if needed, and remove or narrow the len >
prevLen check to avoid treating prepends as appends.
In `@packages/epics/src/common/human-right-panel.tsx`:
- Around line 982-984: The early return when hasMedia is true and client is null
causes a silent failure; update the edit flow in human-right-panel.tsx (the
handler that contains the if (hasMedia && !client) return; check) to surface a
user-facing message or disable the edit action instead of returning silently:
either replace the return with a call to the app’s notification/error UI (e.g.,
show a toast or set an error state) that explains the Matrix client is
unavailable, or remove this branch and make the Edit button disabled when client
is null (tie its disabled prop to client presence and provide a tooltip
explaining why).
---
Outside diff comments:
In `@packages/core/src/matrix/rich-reply.ts`:
- Around line 285-290: The media info returned by the rich-reply code omitted
info.duration for standalone audio, causing TimelineVoiceSlot (and m.audio
messages) to lack duration; update the mediaInfo construction in
packages/core/src/matrix/rich-reply.ts (both the block around the info?: {
mimetype/size/w/h } declaration and the other occurrence at the second block
referenced) to copy info.duration through whenever present (i.e., include
duration alongside mimetype, size, w, h) so that m.audio messages and
TimelineVoiceSlot receive the duration field.
---
Duplicate comments:
In `@packages/core/src/matrix/client/providers/matrix-provider.tsx`:
- Around line 797-862: The prepareMediaPayload implementation is duplicated in
sendMessage and editRoomMessage—extract it to a single module-scope helper named
prepareMediaPayload that accepts the Matrix client and the SendAttachmentInput
(e.g., prepareMediaPayload(client: MatrixSdk.MatrixClient, att:
SendAttachmentInput): Promise<HyphaMediaEventContent>), preserve the
abortController/timeout logic using MATRIX_UPLOAD_TIMEOUT_MS and
MatrixUploadTimeoutError, keep usage of MsgType, loadImageDimensions,
loadAudioDurationMs, HYPHA_SPOILER_FIELD and HyphaMediaEventContent, and then
replace the inline implementations in both sendMessage and editRoomMessage with
calls to the new helper (prepareMediaPayload(client, att)).
In `@packages/epics/src/common/human-chat-panel/chat-panel-media-types.ts`:
- Around line 18-28: AUDIO_FILE_EXTENSIONS currently includes 'webm', which
causes MIME-less .webm files to be classified as audio and routed to
voice-message UI; remove 'webm' from the AUDIO_FILE_EXTENSIONS Set in
chat-panel-media-types.ts (refer to the constant AUDIO_FILE_EXTENSIONS) so .webm
falls through to the video classifier instead, then run tests or verify
downstream classification logic to confirm videos are no longer misrouted.
In
`@packages/epics/src/common/human-chat-panel/human-chat-panel-emoji-mart-surface.tsx`:
- Around line 35-37: The locale check for pickerLocale currently does an exact
match against supported locales and downgrades region-tagged locales (like
"en-US" or "pt-BR") to 'en'; normalize the incoming locale first by extracting
the base language subtag (e.g., const base = (locale ||
'').split('-')[0].toLowerCase()) and then set pickerLocale =
['en','es','fr','de','pt'].includes(base) ? base : 'en' so region subtags are
correctly matched; update references in human-chat-panel-emoji-mart-surface.tsx
where pickerLocale and locale are used to rely on the normalized base language.
In `@packages/epics/src/common/human-right-panel.tsx`:
- Around line 833-852: When handling a redacted message in the timeline listener
(the async callback that calls setMessages and checks message.redacted), also
clear any active reply or edit state that targets that message: update
replyDraft and editDraft (via their setters, e.g., setReplyDraft and
setEditDraft) to null/empty if their referenced message id equals message.id so
the composer doesn't keep targeting a non-existent event; perform this alongside
the existing removal logic in the same redaction branch.
🪄 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: 04520927-85a8-4dad-a4a4-3d3803938667
📒 Files selected for processing (17)
packages/core/src/matrix/client/providers/matrix-provider.tsxpackages/core/src/matrix/rich-reply.tspackages/core/src/matrix/types.tspackages/epics/src/common/human-chat-panel/chat-panel-media-types.tspackages/epics/src/common/human-chat-panel/human-chat-panel-chat-bar.tsxpackages/epics/src/common/human-chat-panel/human-chat-panel-emoji-mart-surface.tsxpackages/epics/src/common/human-chat-panel/human-chat-panel-emoji-picker.tsxpackages/epics/src/common/human-chat-panel/human-chat-panel-message-bubble.tsxpackages/epics/src/common/human-chat-panel/human-chat-panel-message-overflow.tsxpackages/epics/src/common/human-chat-panel/human-chat-panel-messages.tsxpackages/epics/src/common/human-right-panel.tsxpackages/epics/src/people/components/person-avatar.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
Clear reply/edit drafts when a message is redacted remotely; dedupe prepareUploadedAttachmentMediaPayload in matrix-provider; avoid classifying extension-only .webm as audio; normalize emoji-mart locale base tags; simplify audio bubble check and add dev-only debug for video seek failures. Co-authored-by: webguru-hypha <webguru-hypha@users.noreply.github.com>
Show a filled square stop button in place of the mic menu while recording or dictating; remove redundant stop from the mic dropdown; use AudioLines for dictation and voice draft preview. Co-authored-by: webguru-hypha <webguru-hypha@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 157-163: The current loadAudioDurationMs function incorrectly
treats .webm as an audio-only extension; update loadAudioDurationMs to avoid
classifying .webm as audio — either remove .webm from the file name regex (i.e.,
change /\.(webm|ogg|opus|mp3|m4a|wav)$/i to exclude webm) and rely on the
existing file.type.startsWith('audio/') check, or replace the combined checks
with a call to the canonical helper looksLikeAudioMimeOrName from
chat-panel-media-types.ts so the same exclusion logic (which already omits
.webm) is used consistently.
In `@packages/epics/src/common/human-right-panel.tsx`:
- Around line 274-278: Add a short explanatory comment above the dummyEditFile
function stating that the File is intentionally created with empty content to
act only as a metadata carrier for already-uploaded media (the matrix homeserver
holds the actual bytes), and document the meaning of the parameters (filename
and mime) and why an empty Blob is safe/expected; reference the function name
dummyEditFile and its parameters (filename: string, mime?: string) so future
readers understand this is intentional and not a bug.
🪄 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: a18ea709-a5ad-4bf5-9821-63bd268a4562
📒 Files selected for processing (5)
packages/core/src/matrix/client/providers/matrix-provider.tsxpackages/epics/src/common/human-chat-panel/chat-panel-media-types.tspackages/epics/src/common/human-chat-panel/human-chat-panel-emoji-mart-surface.tsxpackages/epics/src/common/human-chat-panel/human-chat-panel-message-bubble.tsxpackages/epics/src/common/human-right-panel.tsx
Use profile photo for reply-to-self, 64px Matrix thumbs for quoted authors, move reply preview into the text column so the main avatar aligns with the sender line, and retune the reply connector SVG. Co-authored-by: webguru-hypha <webguru-hypha@users.noreply.github.com>
Co-authored-by: webguru-hypha <webguru-hypha@users.noreply.github.com>
Shared voice pill with play, decorative waveform, and duration; use in draft cards with slimmer height to match composer chrome. Co-authored-by: webguru-hypha <webguru-hypha@users.noreply.github.com>
Replace voice dropdown with adjacent toolbar triggers: send audio then dictate (waveform icon). Co-authored-by: webguru-hypha <webguru-hypha@users.noreply.github.com>
Draft audio shows blurred pill + spoiler badge like images; timeline voice gets tap-to-reveal overlay when org.hypha.spoiler is set. Co-authored-by: webguru-hypha <webguru-hypha@users.noreply.github.com>
Abort in-flight sends via AbortSignal during attachment uploads and new slots on media edit; add timeline cancel control and restore composer. Change voice row play affordance to black circular button. Co-authored-by: webguru-hypha <webguru-hypha@users.noreply.github.com>
Hide the floating bar during pointer-drag selection and reveal it on global pointerup/pointercancel so it does not cover active selection. Co-authored-by: webguru-hypha <webguru-hypha@users.noreply.github.com>
Build URLs with ?chat=&msg= on the DHO space route, handle them in the panel (open sidebar, scroll and highlight row), and expose openHumanChatPanel. Co-authored-by: webguru-hypha <webguru-hypha@users.noreply.github.com>
Add Matrix markRoomRead via setRoomReadMarkers; compute first unread from read receipts and m.fully_read; Discord-style NEW divider, banner, and first-unread row styling; per-day date rules; scroll to oldest unread on open; mark read when reaching bottom or from banner. Co-authored-by: webguru-hypha <webguru-hypha@users.noreply.github.com>
Use StopCircle instead of filled Square and subtle destructive chrome so recording/dictation stop reads clearly in the toolbar. Co-authored-by: webguru-hypha <webguru-hypha@users.noreply.github.com>
Trigger the same dismiss as the full emoji picker after a top-row quick reaction; use controlled context menu so right-click menu can close too. Co-authored-by: webguru-hypha <webguru-hypha@users.noreply.github.com>
Mirror plain text in a backdrop layer under a transparent textarea so https/www URLs render as primary-colored underlined links while editing. Co-authored-by: webguru-hypha <webguru-hypha@users.noreply.github.com>
Use setRoomReadMarkersHttpRequest with event id strings; align Button variant with ui package; tighten null guards in mark-as-read loop; dismiss context menu quick reactions via Escape instead of unsupported controlled ContextMenu. Co-authored-by: webguru-hypha <webguru-hypha@users.noreply.github.com>
Replace trailing-punctuation regex with linear stripping for CodeQL. Co-authored-by: webguru-hypha <webguru-hypha@users.noreply.github.com>
Stagger vertical scale pulses on playback; disable under prefers-reduced-motion. Co-authored-by: webguru-hypha <webguru-hypha@users.noreply.github.com>
Audio message uses waves on the left, dictate uses mic on the right; stop replaces only the active control with distinct recording vs dictation labels. Co-authored-by: webguru-hypha <webguru-hypha@users.noreply.github.com>
Replace triangle play with Mic on the black circle; keep Pause while playing. Co-authored-by: webguru-hypha <webguru-hypha@users.noreply.github.com>
…e row Use black button + light ring instead of YouTube-style red; same in timeline and draft video preview. Co-authored-by: webguru-hypha <webguru-hypha@users.noreply.github.com>
Replace stop-circle with red dot pulse in red-outlined square; respect prefers-reduced-motion. Co-authored-by: webguru-hypha <webguru-hypha@users.noreply.github.com>
Measure selection anchor at line top and horizontal midpoint so the bar sits fully above the text with a downward arrow toward the highlight. Co-authored-by: webguru-hypha <webguru-hypha@users.noreply.github.com>
Assert space-placeholder rich reply yields empty trailing segment via splitRichReplyPlainBody and document linkage to matrix-provider reply preservation. Co-authored-by: webguru-hypha <webguru-hypha@users.noreply.github.com>
Paint the measured SVG at z-[5] with pointer-events-none so the path is not covered by z-[1] reply/avatar rows; slightly increase stroke contrast on light and dark themes. Co-authored-by: webguru-hypha <webguru-hypha@users.noreply.github.com>
Replace quadratic connector math with a simple vertical+horizontal polyline so replies with the quoted row above the main avatar always render a valid path. Raise connector z-index slightly, thicken stroke, add overflow-visible on the row, and improve contrast for light/dark themes. Co-authored-by: webguru-hypha <webguru-hypha@users.noreply.github.com>
…profile Expand technical-label detection beyond prev_privy_ so prod Privy DID localparts trigger matrix_user_links + person lookup for sender/reply names. Co-authored-by: webguru-hypha <webguru-hypha@users.noreply.github.com>
CI: CodeQL was failing on ReDoS (
trimUrlMatchregex/[),.;:]+$/uin composer URL highlight). Replaced with linear trailing punctuation stripping.Commit:
fix(human-chat): avoid ReDoS in composer URL punctuation trim— CodeQL + check-types verified green on head.Summary by CodeRabbit
New Features
Improvements
Localization