Skip to content

Commit 0dfe29b

Browse files
cpsievertcpsievert
authored
feat: editable messages and sibling navigation (#269)
* feat: editable messages and sibling navigation Allow users to edit a previously sent message, which forks the conversation into sibling branches at that point. Adds tree query methods to ConversationRecord, an UpdateSiblingsAction, and handle_edit/handle_navigate on HistoryController so the server can fork history and switch between sibling branches. The client gets an edit button and inline editor on user messages, sibling prev/next navigation, and the siblings metadata/transport plumbing to support it. Includes Playwright e2e coverage for editing and sibling navigation. * feat: rich-text edit box with editable attachments Replaces the plain textarea used to edit a previously-sent message with the same TiptapInput component the main composer uses, styled as a boxed mini-composer, and extends it to support editing staged attachments end-to-end (edit box -> ChatMessages/ChatContainer props -> sendMessageEdit wire payload -> pkg-py handle_edit validation). Also includes the UX fixes and cleanup that came out of testing this: - Enter/Mod+Enter can submit an attachments-only edit (canSubmitEmpty). - Edit-box chrome no longer lets the pill background paint over the attach button; edit bubble takes the full row width. - The pencil edit button is keyboard-focusable (opacity-based hiding instead of visibility, revealed on :focus-visible/:focus-within). - Stale attachment notices (downscale/size/gif-converted) are cleared when a staged tray is replaced instead of resurfacing on reopen. - useAttachmentStaging hook and AttachmentTray component extracted out of ChatInput for reuse in the edit box. - CHANGELOG entry for editable messages/sibling navigation; branch_from test-fixture logic moved out of ConversationRecord into a pkg-py test helper; pencil icon moved into the shared icons module. Includes matching Playwright/unit test coverage and JS dist rebuilds. * feat: reveal edit button via press-and-hold on touch devices Touch devices had no way to reach the message-edit button, since it was only revealed via :hover/:focus-within. A ~500ms press-and-hold gesture on a user message now reveals it (gated to pointerType === "touch" and to editable, non-editing user messages), dismissed by tapping elsewhere. Desktop mouse/keyboard behavior is unchanged. * feat(pkg-r): editable messages and sibling branch navigation Ports Python's message-editing and sibling (branch) navigation feature to R's HistoryController, so chat_enable_history() understands the same ${id}_message_edit / ${id}_message_navigate wire protocol Python already handles. The JS/TS client is fully shared between packages, so no frontend changes were needed -- this is a server-only port. Adds tree-traversal helpers on ConversationRecord, HistoryController methods (send_sibling_metadata, handle_navigate, handle_edit), wiring into chat_enable_history(), and extends the shared R/Python behavior-matrix test harness to cover both new methods. * fix(pkg-r): edit a message after the first without duplicating it Editing any user message past the first left the pre-edit message on screen alongside the freshly resubmitted one. The history save trigger fired as soon as the server-side stream finished, before the browser had echoed the completed assistant reply back over the websocket. That one-message lag caused the reply to be attached to the wrong node, which threw off every later edit's fork-point calculation by one. Move the save trigger to fire off the client's message echo instead (matching the Python implementation), so it only runs once the browser has actually reported the finished reply. * chore: rebuild web assets after rebasing onto main * `air format` (GitHub Actions) * fix(pkg-r): remove duplicated on_response save observer The `message_response_effect` observer (and its label `history_on_response`) was registered twice with identical bodies, so `on_response()` fired twice per client echo, the first observer instance was orphaned (never destroyed), and `cancel()` called `$destroy()` on the reassigned handle twice. Drop the duplicate block and the extra `$destroy()`. * fix(pkg-r): normalize edited-message attachments to match Python Python's `handle_edit` reconstructs a canonical {mime, data_url, name, size} record per attachment (dropping any extra client-sent fields) before echoing them back through `update_input`; R passed the raw client list straight through. Reconstruct the same shape in R for parity and to avoid trusting client-supplied fields. * fix: branch-navigation internals — remembered descendant, UI-count alignment, dead-flag cleanup Groups four tightly-coupled history-internals changes (Python + R) that touch the same functions and can't be cleanly separated: - selected_child: navigating between sibling branches now returns to the descendant last viewed inside that subtree instead of jumping to the newest leaf. Adds an optional `selected_child` per node, recorded along the active path by a new set_current_leaf() wherever the leaf moves, and followed by subtree_leaf() (falling back to newest when unset). Round-trips through both stores; backward-compatible (unset => today's behavior). - ui_message_count(): node_id_for_message_index() and the sibling-metadata builder now count a node with no `ui` as one message, matching replay_ui()'s fabricated fallback, so client message indices and the server mapping agree. - Drop the write-only `_suppress_next_save` flag in Python (never read; the count-based guard already covers restore echoes; R still needs its flag). - Comment why an empty sibling-metadata payload can't leave a stale badge. * fix(js): seed edit editor only when entering edit mode The seeding effect's deps include message.content/attachments, so a re-reported message object arriving mid-edit would re-run setInputValue and clobber the user's in-progress text (and re-steal focus). Guard on a previous-state ref so seeding happens only on the false->true isEditing transition. * fix(js): key edit state by message id and stabilize edit callbacks ChatMessages tracked the open edit by list index while the list is keyed by id, so a reorder/shrink mid-edit could point at the wrong message; track it by msg.id instead. Also pass stable useCallback handlers (onStartEdit/onCancelEdit) instead of inline closures so ChatMessage's memo() can actually skip untouched subtrees during streaming. onStartEdit now takes the message id. * a11y(js): announce sibling position via aria-live The prev/next buttons have aria-labels, but the "1 / 2" counter was a plain span, so screen readers didn't announce version changes on navigation. Wrap it in aria-live="polite". * chore: rebuild web assets Rebuilt js/dist and copied to both packages for the editable-message review fixes (seed-on-open, id-keyed/memoized edit callbacks, aria-live counter). --------- Co-authored-by: cpsievert <cpsievert@users.noreply.github.com>
1 parent ad7d568 commit 0dfe29b

56 files changed

Lines changed: 5106 additions & 764 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎js/dist/shinychat.css‎

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎js/dist/shinychat.css.map‎

Lines changed: 2 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎js/dist/shinychat.js‎

Lines changed: 71 additions & 71 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎js/dist/shinychat.js.map‎

Lines changed: 4 additions & 4 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎js/src/chat/AttachmentTray.tsx‎

Lines changed: 197 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,197 @@
1+
import { memo } from "react"
2+
import type { AttachmentStaging } from "./useAttachmentStaging"
3+
import {
4+
formatBytes,
5+
acceptAttribute,
6+
attachmentBadgeLabel,
7+
type AttachedFile,
8+
} from "./attachments"
9+
import { TextAttachmentPreview } from "./TextAttachmentPreview"
10+
import { plusThin } from "../utils/icons"
11+
12+
export interface AttachmentTrayProps {
13+
staging: AttachmentStaging
14+
uploadAccept: string[]
15+
maxUploadSize: number | null
16+
enableUpload?: boolean
17+
disabled?: boolean
18+
}
19+
20+
export const AttachmentTray = memo(function AttachmentTray({
21+
staging,
22+
uploadAccept,
23+
maxUploadSize,
24+
enableUpload,
25+
disabled,
26+
}: AttachmentTrayProps) {
27+
const {
28+
attachments,
29+
downscaleNotice,
30+
gifConvertedNotice,
31+
sizeNotice,
32+
fileInputRef,
33+
attachmentRefs,
34+
removeAttachmentByKeyboard,
35+
onFilePick,
36+
onAttachmentsMouseDown,
37+
} = staging
38+
39+
return (
40+
<>
41+
{attachments.length > 0 && (
42+
<div
43+
className="shiny-chat-input-attachments"
44+
onMouseDown={onAttachmentsMouseDown}
45+
>
46+
{attachments.map((a, i) => (
47+
<AttachmentPreview
48+
key={a.id}
49+
attachment={a}
50+
index={i}
51+
onRemove={() => removeAttachmentByKeyboard(i)}
52+
registerRef={(el) => {
53+
attachmentRefs.current[i] = el
54+
}}
55+
/>
56+
))}
57+
</div>
58+
)}
59+
{(downscaleNotice || sizeNotice || gifConvertedNotice) && (
60+
<div className="shiny-chat-input-notice" role="status">
61+
{sizeNotice && maxUploadSize !== null && (
62+
<div>
63+
Attachments exceed the {formatBytes(maxUploadSize)} limit.
64+
</div>
65+
)}
66+
{downscaleNotice && <div>Large image(s) were downscaled to fit.</div>}
67+
{gifConvertedNotice && (
68+
<div>Animated GIF(s) were converted to a still image.</div>
69+
)}
70+
</div>
71+
)}
72+
{enableUpload && (
73+
<>
74+
<input
75+
ref={fileInputRef}
76+
type="file"
77+
multiple
78+
accept={acceptAttribute(uploadAccept)}
79+
style={{ display: "none" }}
80+
onChange={onFilePick}
81+
data-shiny-no-bind-input
82+
/>
83+
<button
84+
type="button"
85+
className="shiny-chat-btn-attach"
86+
title="Attach file"
87+
aria-label="Attach file"
88+
disabled={disabled}
89+
onClick={() => fileInputRef.current?.click()}
90+
dangerouslySetInnerHTML={{ __html: plusThin }}
91+
/>
92+
</>
93+
)}
94+
</>
95+
)
96+
})
97+
98+
const AttachmentPreview = memo(function AttachmentPreview({
99+
attachment,
100+
index,
101+
onRemove,
102+
registerRef,
103+
}: {
104+
attachment: AttachedFile
105+
index: number
106+
onRemove: () => void
107+
registerRef: (el: HTMLDivElement | null) => void
108+
}) {
109+
// Shared focus/keyboard behavior applied to whichever root each variant
110+
// renders: a single tab stop per attachment, click-to-focus, and
111+
// Delete/Backspace to remove while focused.
112+
const containerProps: React.HTMLAttributes<HTMLDivElement> = {
113+
tabIndex: 0,
114+
"aria-label": attachment.name
115+
? `Attachment: ${attachment.name}. Press Delete to remove.`
116+
: "Attachment. Press Delete to remove.",
117+
onClick: (e) => (e.currentTarget as HTMLDivElement).focus(),
118+
onKeyDown: (e) => {
119+
if (e.code === "Delete" || e.code === "Backspace") {
120+
e.preventDefault()
121+
onRemove()
122+
}
123+
},
124+
}
125+
126+
if (attachment.family === "image") {
127+
return (
128+
<div
129+
ref={registerRef}
130+
className="shiny-chat-input-thumbnail"
131+
title={attachment.name || undefined}
132+
{...containerProps}
133+
>
134+
<img
135+
src={attachment.dataUrl}
136+
alt={
137+
attachment.name
138+
? `Attached image: ${attachment.name}`
139+
: `Attached image ${index + 1}`
140+
}
141+
/>
142+
<button
143+
type="button"
144+
tabIndex={0}
145+
aria-label={
146+
attachment.name ? `Remove ${attachment.name}` : "Remove image"
147+
}
148+
onClick={onRemove}
149+
>
150+
×
151+
</button>
152+
</div>
153+
)
154+
}
155+
if (attachment.family === "text") {
156+
return (
157+
<TextAttachmentPreview
158+
dataUrl={attachment.dataUrl}
159+
name={attachment.name}
160+
size={attachment.size}
161+
onRemove={onRemove}
162+
rootRef={registerRef}
163+
rootProps={containerProps}
164+
/>
165+
)
166+
}
167+
return (
168+
<div
169+
ref={registerRef}
170+
className="shiny-chat-input-attachment-chip"
171+
title={attachment.name || undefined}
172+
{...containerProps}
173+
>
174+
<span className="shiny-chat-attachment-badge">
175+
{attachmentBadgeLabel(attachment.name, attachment.type)}
176+
</span>
177+
<span className="shiny-chat-attachment-meta">
178+
<span className="shiny-chat-attachment-name">
179+
{attachment.name || "attachment"}
180+
</span>
181+
<span className="shiny-chat-attachment-size">
182+
{formatBytes(attachment.size)}
183+
</span>
184+
</span>
185+
<button
186+
type="button"
187+
tabIndex={0}
188+
aria-label={
189+
attachment.name ? `Remove ${attachment.name}` : "Remove attachment"
190+
}
191+
onClick={onRemove}
192+
>
193+
×
194+
</button>
195+
</div>
196+
)
197+
})

‎js/src/chat/ChatApp.tsx‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -274,6 +274,20 @@ export function ChatApp({
274274
setCurrentConversationId(elementId, state.history.activeId)
275275
}, [elementId, state.history.enabled, state.history.activeId])
276276

277+
const handleEdit = useCallback(
278+
(index: number, content: string, attachments: AttachmentPayload[]) => {
279+
transport.sendMessageEdit(elementId, index, content, attachments)
280+
},
281+
[transport, elementId],
282+
)
283+
284+
const handleNavigate = useCallback(
285+
(index: number, direction: "prev" | "next") => {
286+
transport.sendMessageNavigate(elementId, index, direction)
287+
},
288+
[transport, elementId],
289+
)
290+
277291
const toolState: ChatToolState = useMemo(
278292
() => ({
279293
hiddenToolRequests: state.hiddenToolRequests,
@@ -310,6 +324,8 @@ export function ChatApp({
310324
historyEnabled={state.history.enabled}
311325
historyConversations={state.history.conversations}
312326
historyActiveId={state.history.activeId}
327+
onEdit={handleEdit}
328+
onNavigate={handleNavigate}
313329
/>
314330
</ChatSubmitContext.Provider>
315331
</ChatDispatchContext.Provider>

‎js/src/chat/ChatContainer.tsx‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ import type {
3232
SlashCommandDef,
3333
} from "../transport/types"
3434
import type { SubmitKey } from "./tiptap/submitShortcut"
35+
import type { AttachmentPayload } from "./attachments"
3536

3637
declare global {
3738
interface Window {
@@ -66,6 +67,12 @@ export interface ChatContainerProps {
6667
historyEnabled?: boolean
6768
historyConversations?: ConversationMeta[]
6869
historyActiveId?: string | null
70+
onEdit?: (
71+
index: number,
72+
content: string,
73+
attachments: AttachmentPayload[],
74+
) => void
75+
onNavigate?: (index: number, direction: "prev" | "next") => void
6976
}
7077

7178
export type ChatContainerHandle = ChatInputHandle
@@ -97,6 +104,8 @@ export const ChatContainer = forwardRef<
97104
historyEnabled,
98105
historyConversations,
99106
historyActiveId,
107+
onEdit,
108+
onNavigate,
100109
},
101110
ref,
102111
) {
@@ -450,11 +459,24 @@ export const ChatContainer = forwardRef<
450459
<ChatMessages
451460
messages={messages}
452461
iconAssistant={iconAssistant}
462+
// Editing/navigating requires the server-side history
463+
// controller, which only registers its input listeners
464+
// when history is enabled -- without this gate the
465+
// buttons would render but silently no-op on click.
466+
onEdit={historyEnabled ? onEdit : undefined}
467+
onNavigate={historyEnabled ? onNavigate : undefined}
468+
disabled={isStreaming}
469+
inputId={inputId}
470+
submitKey={submitKey}
471+
uploadAccept={uploadAccept}
472+
maxUploadSize={maxUploadSize}
473+
enableUpload={enableUpload}
453474
/>
454475
{streamingMessage && (
455476
<MessageErrorBoundary key={streamingMessage.id}>
456477
<ChatMessage
457478
message={streamingMessage}
479+
index={messages.length}
458480
iconAssistant={iconAssistant}
459481
/>
460482
</MessageErrorBoundary>

0 commit comments

Comments
 (0)