From d920328bfa53b4eda2140408d12812671caed83e Mon Sep 17 00:00:00 2001 From: Marco Beretta <81851188+berry-13@users.noreply.github.com> Date: Fri, 14 Aug 2026 01:30:39 +0200 Subject: [PATCH] =?UTF-8?q?=F0=9F=92=AC=20style:=20Unify=20Message=20Row?= =?UTF-8?q?=20Layout=20and=20Edit=20Surfaces=20(#14770)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * style: Unify message row layout and edit surfaces Route chat, share, and search messages through a shared MessageRow so user turns render as right-aligned bubbles and assistant turns keep a visible identity column. Replace per-part text editors with one edit surface that keeps tools, errors, and artifacts visible. Preserve non-text fields when saving content parts, copy the full serialized message, and hide hover actions that do not apply during streaming or errors. * style: Align edit footer and lighten editor field in dark mode Drop the divider above the user edit footer so both edit surfaces share the same footer treatment. Move the editor fields to surface-tertiary-alt. Light mode is unchanged at #fff, while dark mode lifts from #0d0d0d to #2f2f2f so the field sits above the #212121 panel instead of sinking into near-black. * style: Drop focus border and ring from message editors The editor fields changed border color and added a ring on focus. Keep the border static and rely on the app-level focus handling instead. * fix: Keep a triggered message action visible when the row is not hovered Hover actions fade out on non-last rows, and mobile.css only restored display and visibility for an active button, never opacity. Opening the fork popover therefore left it anchored to an invisible trigger once the pointer left the row. Skip the fade entirely while a button is active. Extract the recipe the three toolbars repeated so the rule has one home. Rework the streaming guard to the contract the toolbar now implements: edit and fork are omitted from a streaming response rather than rendered disabled, and the settled turn above keeps its own actions. It asserted the removed disabled-and-transparent behaviour and its opacity check only held because the growing response shifted the row out from under the pointer. * style: Trim message edit chrome and stabilize the status row The edit surface was a titled card sitting inside the conversation: a bordered panel with an "Edit message" heading wrapping bordered fields, which read as a settings dialog rather than an inline editor. Drop the card background, border and heading, and take the footer buttons down to the small size so the editor reads as a field in the message flow. The captured row goes from 253px to 187px. Move "Unsaved changes" into the footer and merge the rerun hint into the same slot. Both previously added their own row, so typing pushed the rest of the conversation down. The slot is clamped to two lines, which stays under the 36px button row, so the footer height holds at 36px regardless of which message is showing. * test: Cover message edit layout stability Add a mock e2e spec that measures the edit footer and section boxes and asserts they hold steady as the status text appears, for both the single-part user editor and a multi-part response. The multi-part case needs an assistant message with two editable parts, so add an E2E_THINK_REPLY marker to the fake model. Its think tags are parsed downstream by the agents stream pipeline, which yields a reasoning part followed by a text part. * fix: Read the fork popover open state from its store Fork mirrored the popover state into its own useState and reset it from an onClose prop. Ariakit 0.4 has no onClose, and React's DOM types accept the name on any element, so it type-checked, landed on a div and never fired. Closing by Escape or an outside click therefore left the button reading as active until the trigger was clicked again. Read the state from the store instead so every close path clears it. * fix: Keep the whole toolbar visible while an action is open Only the triggered button escaped the hover fade, so opening the editor or the fork popover left the row as a single floating button once the pointer moved away. Mark the active button and have every action in the toolbar key off it, so the group stays opaque for as long as a surface is open. The marker is a dedicated class rather than the existing `active`, which HoverButtons pins to the edit button of every assistant message and would hold those toolbars open permanently. The existing guard pressed Escape to close the editor while focus sat on the body, so the editor never closed and its assertion only held because the sibling faded regardless. Close the editor through its own control, and drop focus before measuring the fade now that Escape returns it to the trigger. * fix: Withhold copy while a response is still streaming Text-to-speech, fork and feedback were all withheld from a message that is still generating, but copy was rendered throughout, so the button offered to put half a sentence on the clipboard. Gate it on the same condition. That empties the toolbar for the duration, and SubRow collapses an empty row, so a streaming response now carries no actions at all until it settles. Both guards encoded the old contract: the unit test asserted copy was present and counted a single button, and the browser guard used copy as its proof that the toolbar had mounted. The settled turn above takes over that role. * fix: Move retry navigation to the outer edge of a user turn A user turn is right-aligned, but its sibling navigation rendered ahead of the actions, so the retry counter sat inboard of the icons instead of under the edge of the bubble it belongs to. Order it last on user turns. * fix: Ride the stream instead of chasing it Following a generating answer went through a helper throttled at 145ms, so the thread caught up in visible jerks rather than flowing. It now writes the scroll position directly on each frame, which is what an answer arriving a few pixels at a time actually needs, and glides only for the one long trip a turn makes, when sending has to travel from wherever the reader was down to the newest word. Whether to follow at all is now answered by where the reader is and which way they were going, rather than by the abort flag. `useMessageProcess` raises that flag on any wheel at all, downward ones included, through a throttle whose trailing call lands after the gesture has ended, so nothing timed to the gesture could outlive it. Scrolling down to the newest word could therefore never resume the ride, while the scroll-to-bottom button, which touches no wheel, always could. Arrival is judged on the scroll it produces rather than the wheel tick that started it, because wheel scrolling is animated and at tick time the thread is still far short of where the tick is taking it. Arriving also counts from further out than leaving does: while an answer streams the end recedes between the last tick and the frame that measures it, so judging arrival as tightly as departure leaves a reader unable to catch it at all. * fix: Reveal retry navigation on hover while an answer generates Copy, edit, fork and read-aloud are all withheld from a response that is still generating, which left the retry counter as the only thing rendering under a half-written answer. It now reveals on hover there, like the actions it sits with, and stays put on a settled turn. * fix: Keep a refused rerun from discarding the edit While a response is streaming, the edit action stays available on every earlier row, and those editors see a per-message submitting flag that is false, so Update and rerun is enabled. The send itself is still refused: ask() returns false for the duration of the active submission. Both editors ignored that and closed anyway, so the draft went with them and no rerun ever started. Both rerun paths now check the result and leave the editor untouched when the send is refused, so the work survives until the thread is free. * fix: Let an upward gesture beat the pending send glide Sending arms a smooth glide down to the newest word, and the landing re-pins the thread to the bottom. The landing was scheduled two ways, on scrollend and on a 700ms fallback, and neither was ever cancelled. A reader who changed their mind and headed up mid-flight was pinned again regardless, then dragged back by the next streaming resize. The fallback fires for the whole window, so this held even after the glide had visibly settled. The gesture now marks the glide interrupted, wherever it lets go of the bottom, and the landing stands down when it sees that. A glide the reader leaves alone still re-affirms the ride. * fix: Fade retry navigation on every streaming response format Every other action is withheld from the row that is still generating, so the retry counter is the only thing left under a half-written answer. The plain text row already faded it to hover-only there; the structured rows did not, and left it sitting on its own. Both structured paths now apply the same condition, and the class string the three of them share moves next to the hover action styles it belongs with. * i18n: Correct the copy the edit surface rewrite left behind The multi-part hint told the reader to save first and then rerun, but a save closes the editor and reopening seeds the drafts from what was just saved, so there is nothing left to rerun and the button stays disabled. Rerunning carries a single edited section by design, so the hint now states that limit rather than pointing at a step that is not there. Drop com_ui_save_submit as well: the per-part editor that used it is gone. * test: Make the message visual baselines opt-in The suite asserts sixteen screenshots and the repository tracks none, so Playwright's default treats every one as a miss and the mock e2e job fails on Linux. Baselines only compare cleanly against the machine that produced them, and nothing here can generate ones that match the runner image. The flows keep running and asserting their structure, which is where their value was; only the pixel comparison is now gated behind E2E_VISUAL_SNAPSHOTS. * style: Restore import order in the reworked message files The repository sorter and CI disagreed with what these files were left holding after the edit surface rework. No behavior change. * test: Follow the reworded rerun hint in the edit layout spec The multi-part hint was restated in the previous commit; this assertion still expected the old wording and would have failed the mock e2e suite. * fix: Leave the send glide alone while the answer streams in Every delta of an answer reruns the scroll effect, and the plain follow writes scrollTop outright, which cancels an animation on its first frame. So the glide a send starts was killed by the first token to arrive and the reader was snapped down instead of carried. The follow now stands down while a glide is travelling, which is what the hook already documented but only enforced on the resize path. * fix: Write a saved edit onto the thread as it stands An earlier turn stays editable while the newest answer streams, and the save captured the thread before the request but wrote it back after. Every delta that landed during the round trip was overwritten. Most of the time the next delta re-merged and the damage showed as a one-frame truncation, but a save that resolved after the stream's final write left the cache wrong for the rest of the session. The thread is now read once the request has resolved, which is what the content part editor already did. The editor actions in this file also wrap again rather than hold one unbreakable row, for the reason given in the following commit. * fix: Let the editor actions wrap on a narrow row At 320px an assistant turn gives the editor about 252px once page padding, the identity column and the row gap are taken out, and Cancel, Save and Update & rerun need more than that in English alone. The group was pinned with shrink-0, so it ran past the edge of the row instead of wrapping. A longer translated label makes it worse, and the user turn had no margin left either. Both editors wrap again, which is what the footer did before the status row was folded into it. * fix: Catch up to the new bottom when the glide lands Following stands down for the length of the glide, so an answer that arrives while it travels moves the bottom past the target the glide aimed at. A short response that finished before the glide reported landing left the thread a few lines short of its own end, with nothing left to correct it. Landing now closes whatever gap opened, unless the reader took over on the way. * test: Follow the renamed rerun button in the edit flow specs The button became 'Update & rerun' when the edit surfaces were unified, but two edit-flow specs still located 'Save & Submit' and would have waited for it until they timed out. A type comment named the old button too. * fix: Judge the first thread scroll against a real position The direction check seeded its last-position ref at 0, so the first scroll event on an opened thread, which arrives carrying a large positive scrollTop, read as a jump downward. Near the end that cleared the abort flag and re-pinned a reader to the stream they were scrolling away from. Take the first event as a baseline and judge direction from the next. * fix: Hold the content part editor to what it replaced EditContentParts took over from EditTextPart and left two of its behaviors behind. An emptied box now blocks Save and rerun instead of persisting a blank part. EditTextPart refused the same edit through its form's required rule and the sibling EditMessage still does, so both editors hold one line. The keyboard shortcuts reach the save paths directly, so they are guarded there too, and the footer says why the buttons are down. The editor also follows the chat direction again, taking dir and text alignment from the same setting EditMessage reads. * fix: Hold the footer height while a response streams Every action is withheld from the row that is still generating, and a lone sibling counter renders nothing, so the footer measured zero until the answer landed and then sprang to the height of the buttons. The transcript stepped upward under the reader at the moment a response completed. The placeholder that used to reserve this space went when the footer became unconditional, so hold the height on the row itself instead. * fix: Remember where the thread was put before judging a gesture Direction is judged against the last sample, and the thread is placed at its end without the reader touching it. With no record of where it was put, their first gesture was spent taking the baseline instead of being obeyed: a single PageUp cleared no flag of its own, so the next streamed resize rode the reader straight back to the end they were leaving. Every programmatic move now records the position it left the thread at, so the sentinel stands only until something has actually placed it. * fix: Spend the start of a turn only once it can be honored A reader who scrolls away during one answer leaves the abort flag raised, and nothing lowers it until the next connection opens, which is after this effect has already seen the send. Marking the turn as started on that first pass spent it against a closed gate: by the time the flag cleared there was no start left to honor, the reader was still detached, and the answer they had just asked for streamed on offscreen. Record the turn as started only on the pass that acts on it. * fix: Show the part edits that survived a refused save The editor saves every changed part through one button, but the endpoint takes a single part per call and nothing rolls a write back. A part the server refused therefore left the earlier ones stored while the editor reported that the message could not be saved, so cancelling from there walked away from edits that were already live. Record the writes that landed and reconcile the transcript with them whichever way the save ended. The refused parts are the only ones left holding a draft, so a retry no longer rewrites what already arrived. * fix: Stop a shared transcript from calling the sharer the reader The share row reused the chat view's user label, which reads "You". It is the screen-reader heading for the user turn, so anyone opening a share link heard every prompt the sharer wrote credited to themselves. Use the neutral "User" label on this surface. It keeps the localization the row gained, unlike the untranslated string it replaced. * fix: Let go of the stream when an interaction settles over several resizes Expanding a tool result mid-answer renders the container first and fills it once its contents arrive, so one gesture produces more than one resize. Only the first was credited to the interaction. The second read the reader as still riding the stream and put them back on the bottom they had just left. The suppressed resize now settles the ride as well as the near-bottom measure, using the position the interaction actually left the reader at, so an interaction that kept them on the end still streams. * fix: Edit inside a structured text part instead of flattening it A text content part holds either a string or a { value, annotations } object. The Assistants thread sync persists the structured form with its file citations intact, and the editor reads the part through the same union, so saving an edit wrote a bare string over the whole object and took every citation with it. The same object was handed to the tokenizer, which measures length, so a part that had been edited this way also stored a NaN token count. Write the edit into value, keep the rest of the part, and count the text itself. * fix: Keep a saved part's citations in the transcript it is written back to A text or think part holds either a bare string or a { value, annotations } object, and the editor already read both through getPartText. Writing the draft back into the local message cache put the string over the whole value, so a response carrying file citations lost them the moment it was edited and did not get them back until a refetch. Reading and writing now go through the same accessor, so an edit lands in the shape it was read from and the rest of the part survives. * fix: Let the message editor follow the chosen font size Editing a message dropped the draft to a fixed 14px regardless of the Font Size setting. On dev the textarea carried the markdown class, so it read --markdown-font-size like the rendered message does; restyling it into a bordered box replaced that with text-sm, and the new per-part editor was written the same way. Anyone on Extra Small, Large or Extra Large saw the text jump the moment they entered edit mode. Share the .message-content typography with the editors through a message-editor-text class so a draft is sized like the message it replaces and keeps tracking the setting. --- api/app/clients/BaseClient.js | 3 +- api/app/clients/specs/BaseClient.test.js | 37 ++ .../__tests__/messages-content-edit.spec.js | 144 ++++++ api/server/routes/messages.js | 15 +- client/src/common/types.ts | 2 +- .../Chat/Messages/Content/ContentParts.tsx | 56 +-- .../Messages/Content/EditContentParts.tsx | 444 ++++++++++++++++++ .../Chat/Messages/Content/EditMessage.tsx | 186 +++++--- .../Messages/Content/Parts/AuthorHeader.tsx | 8 +- .../Messages/Content/Parts/EditTextPart.tsx | 213 --------- .../Chat/Messages/Content/Parts/SteerPart.tsx | 69 +-- .../Parts/__tests__/SteerPart.test.tsx | 29 +- .../Chat/Messages/Content/Parts/index.ts | 1 - .../ContentParts.integration.test.tsx | 1 - .../Content/__tests__/ContentParts.test.tsx | 1 - .../__tests__/EditContentParts.spec.tsx | 439 +++++++++++++++++ .../Content/__tests__/EditMessage.spec.tsx | 222 +++++++++ .../src/components/Chat/Messages/Feedback.tsx | 14 +- client/src/components/Chat/Messages/Fork.tsx | 19 +- .../components/Chat/Messages/HoverButtons.tsx | 94 ++-- .../src/components/Chat/Messages/Message.tsx | 4 +- .../components/Chat/Messages/MessageParts.tsx | 173 +++---- .../Chat/Messages/SearchMessage.tsx | 64 +-- .../Chat/Messages/SiblingSwitch.tsx | 10 +- .../__tests__/HoverActions.streaming.spec.tsx | 114 ++++- .../Messages/__tests__/HoverButtons.spec.tsx | 87 +++- .../Chat/Messages/__tests__/styles.spec.ts | 45 ++ client/src/components/Chat/Messages/styles.ts | 57 +++ .../Chat/Messages/ui/MessageRender.tsx | 164 +++---- .../Chat/Messages/ui/MessageRow.tsx | 111 +++++ .../Chat/Messages/ui/PlaceholderRow.tsx | 9 - .../Messages/ui/__tests__/MessageRow.spec.tsx | 101 ++++ .../Endpoints/MessageEndpointIcon.spec.tsx | 31 ++ .../Endpoints/MessageEndpointIcon.tsx | 5 +- .../src/components/Messages/ContentRender.tsx | 163 +++---- .../components/Messages/MessageContent.tsx | 4 +- client/src/components/Share/Message.tsx | 144 +++--- .../__tests__/useMessageScrolling.spec.tsx | 310 +++++++++++- .../hooks/Messages/useCopyToClipboard.spec.ts | 31 +- .../src/hooks/Messages/useCopyToClipboard.ts | 49 +- .../src/hooks/Messages/useMessageScrolling.ts | 293 +++++++++++- client/src/hooks/useGenerationsByLatest.ts | 7 +- client/src/locales/en/translation.json | 6 +- client/src/style.css | 5 +- e2e/setup/fake-model.js | 10 + e2e/specs/messages.spec.ts | 2 +- e2e/specs/mock/helpers.ts | 4 + e2e/specs/mock/hover-actions.spec.ts | 157 ++++++- e2e/specs/mock/message-edit-layout.spec.ts | 127 +++++ e2e/specs/mock/message-tree.spec.ts | 2 +- e2e/specs/mock/message-visual.spec.ts | 213 +++++++++ 51 files changed, 3463 insertions(+), 1036 deletions(-) create mode 100644 api/server/routes/__tests__/messages-content-edit.spec.js create mode 100644 client/src/components/Chat/Messages/Content/EditContentParts.tsx delete mode 100644 client/src/components/Chat/Messages/Content/Parts/EditTextPart.tsx create mode 100644 client/src/components/Chat/Messages/Content/__tests__/EditContentParts.spec.tsx create mode 100644 client/src/components/Chat/Messages/Content/__tests__/EditMessage.spec.tsx create mode 100644 client/src/components/Chat/Messages/__tests__/styles.spec.ts create mode 100644 client/src/components/Chat/Messages/styles.ts create mode 100644 client/src/components/Chat/Messages/ui/MessageRow.tsx delete mode 100644 client/src/components/Chat/Messages/ui/PlaceholderRow.tsx create mode 100644 client/src/components/Chat/Messages/ui/__tests__/MessageRow.spec.tsx create mode 100644 client/src/components/Endpoints/MessageEndpointIcon.spec.tsx create mode 100644 e2e/specs/mock/message-edit-layout.spec.ts create mode 100644 e2e/specs/mock/message-visual.spec.ts diff --git a/api/app/clients/BaseClient.js b/api/app/clients/BaseClient.js index 19d4619bad..58d111a32a 100644 --- a/api/app/clients/BaseClient.js +++ b/api/app/clients/BaseClient.js @@ -591,7 +591,8 @@ class BaseClient { } else if (editedContent != null) { // Handle editedContent for content parts if (editedContent && latestMessage.content && Array.isArray(latestMessage.content)) { - const { index, text, type } = editedContent; + const { index, type } = editedContent; + const text = editedContent[type]; if (index >= 0 && index < latestMessage.content.length) { const contentPart = latestMessage.content[index]; if (type === ContentTypes.THINK && contentPart.type === ContentTypes.THINK) { diff --git a/api/app/clients/specs/BaseClient.test.js b/api/app/clients/specs/BaseClient.test.js index 77848851a2..70f0a91c17 100644 --- a/api/app/clients/specs/BaseClient.test.js +++ b/api/app/clients/specs/BaseClient.test.js @@ -691,6 +691,43 @@ describe('BaseClient', () => { ); }); + it('applies edited reasoning content from its typed payload before regeneration', async () => { + const responseMessageId = 'response-with-reasoning'; + const newHistory = [ + ...messageHistory, + { + role: 'assistant', + isCreatedByUser: false, + messageId: responseMessageId, + parentMessageId: '3', + content: [ + { type: ContentTypes.THINK, think: 'Original reasoning', phase: 'analysis' }, + { type: ContentTypes.TEXT, text: 'Original response' }, + ], + }, + ]; + + TestClient = initializeFakeClient(apiKey, options, newHistory); + await TestClient.sendMessage('test message', { + isEdited: true, + overrideParentMessageId: 'user-message-id', + parentMessageId: '3', + responseMessageId, + editedContent: { + index: 0, + type: ContentTypes.THINK, + [ContentTypes.THINK]: 'Updated reasoning', + }, + }); + + const editedResponse = TestClient.currentMessages[TestClient.currentMessages.length - 1]; + expect(editedResponse.content[0]).toEqual({ + type: ContentTypes.THINK, + think: 'Updated reasoning', + phase: 'analysis', + }); + }); + test('setOptions is called with the correct arguments only when replaceOptions is set to true', async () => { TestClient.setOptions = jest.fn(); const opts = { conversationId: '123', parentMessageId: '456', replaceOptions: true }; diff --git a/api/server/routes/__tests__/messages-content-edit.spec.js b/api/server/routes/__tests__/messages-content-edit.spec.js new file mode 100644 index 0000000000..5851c2a781 --- /dev/null +++ b/api/server/routes/__tests__/messages-content-edit.spec.js @@ -0,0 +1,144 @@ +const express = require('express'); +const request = require('supertest'); +const { ContentTypes } = require('librechat-data-provider'); + +jest.mock('@librechat/agents', () => ({ + sleep: jest.fn(), +})); + +jest.mock('@librechat/api', () => ({ + unescapeLaTeX: jest.fn((value) => value), + countTokens: jest.fn().mockResolvedValue(2), + sendFeedbackScore: jest.fn().mockResolvedValue(undefined), + traceIdForMessage: jest.fn((messageId) => `trace-${messageId}`), + mergeQuotedTextForCount: jest.fn((text) => text), +})); + +jest.mock('@librechat/data-schemas', () => ({ + ...jest.requireActual('@librechat/data-schemas'), + logger: { + debug: jest.fn(), + info: jest.fn(), + warn: jest.fn(), + error: jest.fn(), + }, +})); + +jest.mock('~/models', () => ({ + getMessages: jest.fn(), + updateMessage: jest.fn(), +})); + +jest.mock('~/server/services/Artifacts/update', () => ({ + findAllArtifacts: jest.fn(), + replaceArtifactContent: jest.fn(), +})); + +jest.mock('~/server/middleware', () => ({ + requireJwtAuth: (req, res, next) => next(), + validateMessageReq: (req, res, next) => next(), + configMiddleware: (req, res, next) => next(), + sendValidationResponse: jest.fn(), + prepareMessageRequestValidation: jest.fn(), +})); + +describe('PUT /:conversationId/:messageId content edit', () => { + let app; + const { getMessages, updateMessage } = require('~/models'); + + beforeAll(() => { + const messagesRouter = require('../messages'); + app = express(); + app.use(express.json()); + app.use((req, res, next) => { + req.user = { id: 'user-1' }; + next(); + }); + app.use('/api/messages', messagesRouter); + }); + + beforeEach(() => { + jest.clearAllMocks(); + updateMessage.mockResolvedValue({ messageId: 'message-1' }); + }); + + it('preserves content-part metadata when editing its text', async () => { + getMessages.mockResolvedValue([ + { + tokenCount: 10, + content: [ + { + type: ContentTypes.TEXT, + text: 'Original response', + phase: 'commentary', + agentId: 'agent-1', + tool_call_ids: ['tool-1'], + }, + ], + }, + ]); + + const response = await request(app) + .put('/api/messages/conversation-1/message-1') + .send({ index: 0, text: 'Edited response', model: 'gpt-5' }); + + expect(response.status).toBe(200); + expect(updateMessage).toHaveBeenCalledWith('user-1', { + messageId: 'message-1', + tokenCount: 10, + content: [ + { + type: ContentTypes.TEXT, + text: 'Edited response', + phase: 'commentary', + agentId: 'agent-1', + tool_call_ids: ['tool-1'], + }, + ], + }); + }); + + /** + * A text part is `string | { value, annotations }`. The Assistants thread sync + * persists the structured form with its file citations intact and the editor reads + * it through the same union, so writing the edit straight over the object dropped + * every citation. Counting the object rather than its value is the same mistake + * read back: the tokenizer measures `text.length`, which an object does not have, + * so the stored count became NaN. + */ + it('edits inside a structured text part instead of flattening it', async () => { + const { countTokens } = require('@librechat/api'); + const annotations = [ + { type: 'file_citation', text: 'source', file_citation: { file_id: 'file-1' } }, + ]; + + getMessages.mockResolvedValue([ + { + tokenCount: 10, + content: [ + { + type: ContentTypes.TEXT, + text: { value: 'Original response', annotations }, + }, + ], + }, + ]); + + const response = await request(app) + .put('/api/messages/conversation-1/message-1') + .send({ index: 0, text: 'Edited response', model: 'gpt-5' }); + + expect(response.status).toBe(200); + expect(updateMessage).toHaveBeenCalledWith('user-1', { + messageId: 'message-1', + tokenCount: 10, + content: [ + { + type: ContentTypes.TEXT, + text: { value: 'Edited response', annotations }, + }, + ], + }); + expect(countTokens).toHaveBeenCalledWith('Original response', 'gpt-5'); + }); +}); diff --git a/api/server/routes/messages.js b/api/server/routes/messages.js index 0425b40eaa..0894e9bd1d 100644 --- a/api/server/routes/messages.js +++ b/api/server/routes/messages.js @@ -409,8 +409,19 @@ router.put('/:conversationId/:messageId', validateMessageReq, async (req, res) = return res.status(400).json({ error: 'Cannot update non-text content' }); } - const oldText = updatedContent[index][currentPartType]; - updatedContent[index] = { type: currentPartType, [currentPartType]: text }; + /** A text part is `string | { value, annotations }`. The Assistants thread sync + * persists the structured form with its file citations intact, and the editor + * reads it through the same union, so an edit has to be written into `value` + * rather than over the whole part. The same object is what gets counted below, + * and the tokenizer measures `length`, which an object does not have. */ + const currentPart = updatedContent[index]; + const currentValue = currentPart[currentPartType]; + const isStructuredValue = currentValue != null && typeof currentValue === 'object'; + const oldText = isStructuredValue ? (currentValue.value ?? '') : currentValue; + updatedContent[index] = { + ...currentPart, + [currentPartType]: isStructuredValue ? { ...currentValue, value: text } : text, + }; let tokenCount = message.tokenCount; if (tokenCount !== undefined) { diff --git a/client/src/common/types.ts b/client/src/common/types.ts index 07d7544de9..5fae782858 100644 --- a/client/src/common/types.ts +++ b/client/src/common/types.ts @@ -352,7 +352,7 @@ export type TOptions = { isContinued?: boolean; isEdited?: boolean; overrideMessages?: t.TMessage[]; - /** This value is only true when the user submits a message with "Save & Submit" for a user-created message */ + /** This value is only true when the user submits a message with "Update & rerun" for a user-created message */ isResubmission?: boolean; /** Currently only utilized when `isResubmission === true`, uses that message's currently attached files */ overrideFiles?: t.TMessage['files']; diff --git a/client/src/components/Chat/Messages/Content/ContentParts.tsx b/client/src/components/Chat/Messages/Content/ContentParts.tsx index 870e6d6cd2..7656949a65 100644 --- a/client/src/components/Chat/Messages/Content/ContentParts.tsx +++ b/client/src/components/Chat/Messages/Content/ContentParts.tsx @@ -11,10 +11,11 @@ import type { ToolCallGroupExpansionState } from './ToolCallGroup'; import { mapAttachments, filterAttachmentsForPart, groupSequentialToolCalls } from '~/utils'; import { groupActivityPhases, lastVisibleContentIdx } from '~/utils/activityLabels'; import { ParallelContentRenderer, type PartWithIndex } from './ParallelContent'; -import { EditTextPart, EmptyText, AgentUpdate } from './Parts'; import { MessageContext, SearchContext } from '~/Providers'; import PendingSkillCall from './Parts/PendingSkillCall'; import ActivityPhaseGroup from './ActivityPhaseGroup'; +import EditContentParts from './EditContentParts'; +import { EmptyText, AgentUpdate } from './Parts'; import ApprovalProvider from './ApprovalContext'; import MemoryArtifacts from './MemoryArtifacts'; import Sources from '~/components/Web/Sources'; @@ -430,45 +431,24 @@ const ContentParts = memo(function ContentParts({ return null; } - // Edit mode: render editable text parts. Interim skill cards are a - // mid-stream concern, not relevant in edit mode. + // Interim skill cards are a mid-stream concern, not relevant in edit mode. if (edit === true && enterEdit && setSiblingIdx) { return ( - <> - {(content ?? []).map((part, localIdx) => { - if (!part) { - return null; - } - const idx = absoluteIndexAt(localIdx); - const isTextPart = - part?.type === ContentTypes.TEXT || - typeof (part as unknown as Agents.MessageContentText)?.text === 'string'; - const isThinkPart = - part?.type === ContentTypes.THINK || - typeof (part as unknown as Agents.ReasoningDeltaUpdate)?.think === 'string'; - if (!isTextPart && !isThinkPart) { - return null; - } - - const isToolCall = part.type === ContentTypes.TOOL_CALL || part['tool_call_ids'] != null; - if (isToolCall) { - return null; - } - - return ( - - ); - })} - + + + + renderPart(part, idx, isLastPart)} + /> + + ); } diff --git a/client/src/components/Chat/Messages/Content/EditContentParts.tsx b/client/src/components/Chat/Messages/Content/EditContentParts.tsx new file mode 100644 index 0000000000..fd2129aae6 --- /dev/null +++ b/client/src/components/Chat/Messages/Content/EditContentParts.tsx @@ -0,0 +1,444 @@ +import { useCallback, useEffect, useMemo, useRef, useState } from 'react'; +import { useRecoilValue } from 'recoil'; +import { ContentTypes } from 'librechat-data-provider'; +import { Alert, Button, TextareaAutosize } from '@librechat/client'; +import { useUpdateMessageContentMutation } from 'librechat-data-provider/react-query'; +import type { TMessageContentParts, TextData } from 'librechat-data-provider'; +import type { ReactNode } from 'react'; +import { useMessagesConversation, useMessagesOperations } from '~/Providers'; +import { splitMarkdownIntoBlocks } from './splitMarkdown'; +import { useGetAddedConvo } from '~/hooks/Chat'; +import { useLocalize } from '~/hooks'; +import { cn } from '~/utils'; +import store from '~/store'; + +type EditableType = ContentTypes.TEXT | ContentTypes.THINK; + +type EditablePart = { + index: number; + localIndex: number; + type: EditableType; + original: string; +}; + +type EditContentPartsProps = { + content: Array; + contentIndexOffset?: number; + messageId: string; + isSubmitting: boolean; + enterEdit: (cancel?: boolean) => void | null | undefined; + siblingIdx: number | null; + setSiblingIdx: (value: number) => void; + renderReadOnlyPart: (part: TMessageContentParts, index: number, isLastPart: boolean) => ReactNode; +}; + +/** An editable part holds either a bare string or a `{ value, annotations }` object, + * which is how the Assistants thread sync stores a response that carries file + * citations. Both the read and the write below go through this, so an edit lands in + * the same shape it was read from. */ +const getPartValue = (part: TMessageContentParts): string | TextData => { + if (part.type === ContentTypes.TEXT) { + return part.text; + } + if (part.type === ContentTypes.THINK) { + return part.think; + } + return undefined; +}; + +const getPartText = (part: TMessageContentParts): string | undefined => { + const value = getPartValue(part); + return typeof value === 'string' ? value : value?.value; +}; + +const withPartText = (part: TMessageContentParts, text: string): string | TextData => { + const value = getPartValue(part); + return value != null && typeof value === 'object' ? { ...value, value: text } : text; +}; + +const containsArtifact = (text: string): boolean => { + if (!text.includes('artifact')) { + return false; + } + try { + return splitMarkdownIntoBlocks(text).some((block) => block.artifactCount > 0); + } catch { + return false; + } +}; + +export default function EditContentParts({ + content, + contentIndexOffset = 0, + messageId, + isSubmitting, + enterEdit, + siblingIdx, + setSiblingIdx, + renderReadOnlyPart, +}: EditContentPartsProps) { + const localize = useLocalize(); + const isRTL = useRecoilValue(store.chatDirection).toLowerCase() === 'rtl'; + const { conversation } = useMessagesConversation(); + const { ask, getMessages, setMessages } = useMessagesOperations(); + const getAddedConvo = useGetAddedConvo(); + const firstEditorRef = useRef(null); + const [saveError, setSaveError] = useState(false); + const [isSaving, setIsSaving] = useState(false); + const updateMessageContentMutation = useUpdateMessageContentMutation( + conversation?.conversationId ?? '', + ); + + const editableParts = useMemo(() => { + const result: EditablePart[] = []; + content.forEach((part, localIndex) => { + if (!part || (part.type !== ContentTypes.TEXT && part.type !== ContentTypes.THINK)) { + return; + } + if (part.type === ContentTypes.TEXT && part.tool_call_ids != null) { + return; + } + const original = getPartText(part); + if (original == null || containsArtifact(original)) { + return; + } + result.push({ + index: localIndex + contentIndexOffset, + localIndex, + type: part.type, + original, + }); + }); + return result; + }, [content, contentIndexOffset]); + + const [drafts, setDrafts] = useState>(() => + Object.fromEntries(editableParts.map((part) => [part.index, part.original])), + ); + + const editableByLocalIndex = useMemo( + () => new Map(editableParts.map((part) => [part.localIndex, part])), + [editableParts], + ); + const changedParts = useMemo( + () => editableParts.filter((part) => drafts[part.index] !== part.original), + [drafts, editableParts], + ); + /** Emptying a part would persist a blank one, and nothing in the editor offers a + * way back: there is no delete-part affordance, so the only reading of a cleared + * box is an accident. The sibling `EditMessage` refuses the same edit through its + * form's `required` rule, so both editors hold the same line. */ + const hasBlankEdit = useMemo( + () => changedParts.some((part) => (drafts[part.index] ?? '').trim() === ''), + [changedParts, drafts], + ); + const editedMessage = getMessages()?.find((item) => item.messageId === messageId); + const rerunRequiresSave = editedMessage?.isCreatedByUser !== true && changedParts.length > 1; + const isBusy = isSubmitting || isSaving; + + useEffect(() => { + const editor = firstEditorRef.current; + if (!editor) { + return; + } + editor.focus(); + editor.setSelectionRange(editor.value.length, editor.value.length); + }, []); + + const applySavedParts = useCallback( + (savedParts: EditablePart[]) => { + const messages = getMessages(); + if (!messages || savedParts.length === 0) { + return; + } + const changedByLocalIndex = new Map( + savedParts.map((part) => [part.localIndex, { type: part.type, text: drafts[part.index] }]), + ); + setMessages( + messages.map((currentMessage) => { + if (currentMessage.messageId !== messageId || !Array.isArray(currentMessage.content)) { + return currentMessage; + } + return { + ...currentMessage, + content: currentMessage.content.map((part, localIndex) => { + const change = changedByLocalIndex.get(localIndex); + if (!part || !change || part.type !== change.type) { + return part; + } + return { + ...part, + [change.type]: withPartText(part, change.text), + } as TMessageContentParts; + }), + }; + }), + ); + }, + [drafts, getMessages, messageId, setMessages], + ); + + const saveChanges = useCallback(async () => { + if (changedParts.length === 0 || hasBlankEdit || isBusy) { + return; + } + setIsSaving(true); + setSaveError(false); + /** The endpoint takes one part per call and nothing rolls a write back, so a + * refused part leaves the earlier ones on the server. Recording what actually + * landed lets the failure reconcile the transcript with the server instead of + * claiming nothing was saved, and leaves only the refused parts still edited. */ + const savedParts: EditablePart[] = []; + try { + /** Each endpoint call replaces the full content array. Keep writes ordered so a + * later edit reads the content produced by the previous one instead of racing it. */ + for (const part of changedParts) { + await updateMessageContentMutation.mutateAsync({ + index: part.index, + conversationId: conversation?.conversationId ?? '', + text: drafts[part.index], + messageId, + }); + savedParts.push(part); + } + } catch { + setSaveError(true); + } finally { + applySavedParts(savedParts); + setIsSaving(false); + } + + if (savedParts.length === changedParts.length) { + enterEdit(true); + } + }, [ + applySavedParts, + changedParts, + conversation?.conversationId, + drafts, + enterEdit, + hasBlankEdit, + isBusy, + messageId, + updateMessageContentMutation, + ]); + + const updateAndRerun = useCallback(() => { + const firstChange = changedParts[0]; + if (!firstChange || !editedMessage || rerunRequiresSave || hasBlankEdit || isBusy) { + return; + } + const messages = getMessages(); + + /** `ask` refuses to send while another response is streaming and reports it by + * returning false. Closing the editor regardless would throw the drafts away for + * a rerun that never started, so a refused send leaves the editor as it was. */ + let refused = false; + + if (editedMessage.isCreatedByUser === true) { + const userText = editableParts + .filter((part) => part.type === ContentTypes.TEXT) + .map((part) => drafts[part.index]) + .join('\n'); + refused = + ask( + { + text: userText, + parentMessageId: editedMessage.parentMessageId, + conversationId: editedMessage.conversationId, + }, + { + overrideFiles: editedMessage.files, + overrideManualSkills: editedMessage.manualSkills, + overrideQuotes: editedMessage.quotes, + addedConvo: getAddedConvo() || undefined, + }, + ) === false; + } else { + const parentMessage = messages?.find( + (item) => item.messageId === editedMessage.parentMessageId, + ); + if (!parentMessage) { + return; + } + const editedContent = + firstChange.type === ContentTypes.THINK + ? { + index: firstChange.index, + type: ContentTypes.THINK as const, + [ContentTypes.THINK]: drafts[firstChange.index], + } + : { + index: firstChange.index, + type: ContentTypes.TEXT as const, + [ContentTypes.TEXT]: drafts[firstChange.index], + }; + refused = + ask( + { ...parentMessage }, + { + editedContent, + editedMessageId: messageId, + isRegenerate: true, + isEdited: true, + overrideManualSkills: parentMessage.manualSkills, + overrideQuotes: parentMessage.quotes, + addedConvo: getAddedConvo() || undefined, + }, + ) === false; + } + + if (refused) { + return; + } + + setSiblingIdx((siblingIdx ?? 0) - 1); + enterEdit(true); + }, [ + ask, + changedParts, + drafts, + editedMessage, + editableParts, + enterEdit, + getAddedConvo, + getMessages, + hasBlankEdit, + isBusy, + messageId, + rerunRequiresSave, + setSiblingIdx, + siblingIdx, + ]); + + const handleKeyDown = useCallback( + (event: React.KeyboardEvent) => { + if (event.key === 'Escape') { + event.preventDefault(); + enterEdit(true); + return; + } + if (event.key === 'Enter' && (event.ctrlKey || event.metaKey)) { + event.preventDefault(); + updateAndRerun(); + return; + } + if (event.key.toLowerCase() === 's' && (event.ctrlKey || event.metaKey)) { + event.preventDefault(); + void saveChanges(); + } + }, + [enterEdit, saveChanges, updateAndRerun], + ); + + /** Both states share the footer's status slot so neither can add a row and + * shift the message below it. */ + const getStatusMessage = () => { + if (hasBlankEdit) { + return localize('com_ui_message_part_empty'); + } + if (rerunRequiresSave) { + return localize('com_ui_save_before_rerun'); + } + if (changedParts.length > 0) { + return localize('com_ui_unsaved_changes'); + } + return ''; + }; + + return ( +
+ {saveError && {localize('com_ui_save_message_error')}} + +
+ {content.map((part, localIndex) => { + if (!part) { + return null; + } + const editablePart = editableByLocalIndex.get(localIndex); + const absoluteIndex = localIndex + contentIndexOffset; + if (!editablePart) { + return ( +
+ {renderReadOnlyPart(part, absoluteIndex, localIndex === content.length - 1)} +
+ ); + } + const label = + editablePart.type === ContentTypes.THINK + ? localize('com_ui_thoughts') + : localize('com_ui_response'); + return ( + + ); + })} +
+ + {/* The actions wrap rather than hold one unbreakable row: on a 320px assistant + turn the identity column and page padding leave less width than the three + English labels need, and a translated label needs more still. */} +
+ + {getStatusMessage()} + +
+ + + +
+
+
+ ); +} diff --git a/client/src/components/Chat/Messages/Content/EditMessage.tsx b/client/src/components/Chat/Messages/Content/EditMessage.tsx index 2030c5ffc9..1fbd4d4efc 100644 --- a/client/src/components/Chat/Messages/Content/EditMessage.tsx +++ b/client/src/components/Chat/Messages/Content/EditMessage.tsx @@ -1,14 +1,14 @@ -import { useRef, useEffect, useCallback } from 'react'; +import { useRef, useEffect, useCallback, useState } from 'react'; import { useRecoilValue } from 'recoil'; import { useForm } from 'react-hook-form'; -import { TextareaAutosize, TooltipAnchor } from '@librechat/client'; +import { Alert, Button, TextareaAutosize } from '@librechat/client'; import { useUpdateMessageMutation } from 'librechat-data-provider/react-query'; import type { TEditProps } from '~/common'; import { useMessagesOperations, useMessagesConversation } from '~/Providers'; import { useGetAddedConvo } from '~/hooks/Chat'; -import { cn, removeFocusRings } from '~/utils'; import { useLocalize } from '~/hooks'; import Container from './Container'; +import { cn } from '~/utils'; import store from '~/store'; const EditMessage = ({ @@ -22,6 +22,7 @@ const EditMessage = ({ }: TEditProps) => { const saveButtonRef = useRef(null); const submitButtonRef = useRef(null); + const [saveError, setSaveError] = useState(false); const { conversation } = useMessagesConversation(); const { getMessages, setMessages } = useMessagesOperations(); @@ -36,7 +37,13 @@ const EditMessage = ({ const getAddedConvo = useGetAddedConvo(); - const { register, handleSubmit, setValue } = useForm({ + const { + register, + handleSubmit, + setValue, + formState: { isDirty, isValid }, + } = useForm({ + mode: 'onChange', defaultValues: { text: text ?? '', }, @@ -51,9 +58,12 @@ const EditMessage = ({ } }, []); + /** `ask` refuses to send while another response is streaming and reports it by + * returning false. Closing the editor regardless would throw the draft away for + * a rerun that never started, so a refused send leaves the editor as it was. */ const resubmitMessage = (data: { text: string }) => { if (message.isCreatedByUser) { - ask( + const submitted = ask( { text: data.text, parentMessageId, @@ -72,6 +82,10 @@ const EditMessage = ({ }, ); + if (submitted === false) { + return; + } + setSiblingIdx((siblingIdx ?? 0) - 1); } else { const messages = getMessages(); @@ -80,7 +94,7 @@ const EditMessage = ({ if (!parentMessage) { return; } - ask( + const submitted = ask( { ...parentMessage }, { editedText: data.text, @@ -98,41 +112,58 @@ const EditMessage = ({ }, ); + if (submitted === false) { + return; + } + setSiblingIdx((siblingIdx ?? 0) - 1); } enterEdit(true); }; - const updateMessage = (data: { text: string }) => { - const messages = getMessages(); - if (!messages) { - return; - } - updateMessageMutation.mutate({ - conversationId: conversationId ?? '', - model: conversation?.model ?? 'gpt-3.5-turbo', - text: data.text, - messageId, - }); + const updateMessage = async (data: { text: string }) => { + setSaveError(false); + try { + await updateMessageMutation.mutateAsync({ + conversationId: conversationId ?? '', + model: conversation?.model ?? 'gpt-3.5-turbo', + text: data.text, + messageId, + }); - const isInMessages = messages.some((message) => message.messageId === messageId); - if (!isInMessages) { - message.text = data.text; - } else { - setMessages( - messages.map((msg) => - msg.messageId === messageId - ? { - ...msg, - text: data.text, - } - : msg, - ), + /** Read the thread after the request, not before it. An earlier turn stays + * editable while the newest answer streams, so a snapshot taken before the + * round trip is already behind by the time it would be written back, and + * writing it wholesale would drop every delta that landed in between. */ + const messages = getMessages(); + if (!messages) { + enterEdit(true); + return; + } + + const isInMessages = messages.some( + (currentMessage) => currentMessage.messageId === messageId, ); - } + if (!isInMessages) { + message.text = data.text; + } else { + setMessages( + messages.map((msg) => + msg.messageId === messageId + ? { + ...msg, + text: data.text, + } + : msg, + ), + ); + } - enterEdit(true); + enterEdit(true); + } catch { + setSaveError(true); + } }; const handleKeyDown = useCallback( @@ -156,13 +187,17 @@ const EditMessage = ({ const { ref, ...registerProps } = register('text', { required: true, onChange: (e) => { - setValue('text', e.target.value, { shouldValidate: true }); + setValue('text', e.target.value, { shouldDirty: true, shouldValidate: true }); }, }); return ( -
+
+ {saveError && {localize('com_ui_save_message_error')}} { @@ -172,53 +207,60 @@ const EditMessage = ({ onKeyDown={handleKeyDown} data-testid="message-text-editor" className={cn( - 'markdown prose dark:prose-invert light whitespace-pre-wrap break-words pl-3 md:pl-4', - 'm-0 w-full resize-none border-0 bg-transparent py-[10px]', - 'placeholder-text-secondary focus:ring-0 focus-visible:ring-0 md:py-3.5', + 'message-editor-text max-h-[65vh] min-h-24 w-full resize-y whitespace-pre-wrap', + 'break-words rounded-lg border border-border-medium bg-surface-tertiary-alt', + 'px-3 py-2 text-text-primary', + 'focus-visible:outline-none', isRTL ? 'text-right' : 'text-left', - 'max-h-[65vh] pr-3 md:max-h-[75vh] md:pr-4', - removeFocusRings, + 'disabled:opacity-50 md:max-h-[75vh]', )} aria-label={localize('com_ui_message_input')} + aria-keyshortcuts="Control+Enter Meta+Enter Control+S Meta+S Escape" + disabled={isSubmitting || updateMessageMutation.isLoading} dir={isRTL ? 'rtl' : 'ltr'} /> -
-
- + + {isDirty ? localize('com_ui_unsaved_changes') : ''} + +
+ - } - /> - + - } - /> - enterEdit(true)}> - {localize('com_ui_cancel')} - - } - /> -
+ {updateMessageMutation.isLoading + ? localize('com_ui_saving') + : localize('com_ui_save')} + + +
+ + ); }; diff --git a/client/src/components/Chat/Messages/Content/Parts/AuthorHeader.tsx b/client/src/components/Chat/Messages/Content/Parts/AuthorHeader.tsx index 189c9a9268..d0deeb1fcc 100644 --- a/client/src/components/Chat/Messages/Content/Parts/AuthorHeader.tsx +++ b/client/src/components/Chat/Messages/Content/Parts/AuthorHeader.tsx @@ -1,8 +1,5 @@ import { memo } from 'react'; -import { useAtomValue } from 'jotai'; import type { ReactNode } from 'react'; -import { fontSizeAtom } from '~/store/fontSize'; -import { cn } from '~/utils'; /** * Re-attributes response content to its author mid-message. A `SteerPart` @@ -18,15 +15,14 @@ const AuthorHeader = memo(function AuthorHeader({ icon: ReactNode; label: string; }) { - const fontSize = useAtomValue(fontSizeAtom); return (
-
+ -

{label}

+

{label}

); }); diff --git a/client/src/components/Chat/Messages/Content/Parts/EditTextPart.tsx b/client/src/components/Chat/Messages/Content/Parts/EditTextPart.tsx deleted file mode 100644 index 6575ad327b..0000000000 --- a/client/src/components/Chat/Messages/Content/Parts/EditTextPart.tsx +++ /dev/null @@ -1,213 +0,0 @@ -import { useRef, useEffect, useCallback, useMemo } from 'react'; -import { useRecoilValue } from 'recoil'; -import { useForm } from 'react-hook-form'; -import { TextareaAutosize } from '@librechat/client'; -import { ContentTypes } from 'librechat-data-provider'; -import { Lightbulb, MessageSquare } from 'lucide-react'; -import { useUpdateMessageContentMutation } from 'librechat-data-provider/react-query'; -import type { Agents } from 'librechat-data-provider'; -import type { TEditProps } from '~/common'; -import { useMessagesOperations, useMessagesConversation } from '~/Providers'; -import Container from '~/components/Chat/Messages/Content/Container'; -import { useGetAddedConvo } from '~/hooks/Chat'; -import { cn, removeFocusRings } from '~/utils'; -import { useLocalize } from '~/hooks'; -import store from '~/store'; - -const EditTextPart = ({ - part, - index, - messageId, - isSubmitting, - enterEdit, -}: Omit & { - index: number; - messageId: string; - part: Agents.MessageContentText | Agents.ReasoningDeltaUpdate; -}) => { - const localize = useLocalize(); - const { conversation } = useMessagesConversation(); - const { ask, getMessages, setMessages } = useMessagesOperations(); - - const { conversationId = '' } = conversation ?? {}; - const message = useMemo( - () => getMessages()?.find((msg) => msg.messageId === messageId), - [getMessages, messageId], - ); - - const chatDirection = useRecoilValue(store.chatDirection); - - const getAddedConvo = useGetAddedConvo(); - - const textAreaRef = useRef(null); - const updateMessageContentMutation = useUpdateMessageContentMutation(conversationId ?? ''); - - const isRTL = chatDirection?.toLowerCase() === 'rtl'; - - const { register, handleSubmit, setValue } = useForm({ - defaultValues: { - text: (ContentTypes.THINK in part ? part.think : part.text) || '', - }, - }); - - useEffect(() => { - const textArea = textAreaRef.current; - if (textArea) { - const length = textArea.value.length; - textArea.focus(); - textArea.setSelectionRange(length, length); - } - }, []); - - const resubmitMessage = (data: { text: string }) => { - const messages = getMessages(); - const parentMessage = messages?.find((msg) => msg.messageId === message?.parentMessageId); - - const editedContent = - part.type === ContentTypes.THINK - ? { - index, - type: ContentTypes.THINK as const, - [ContentTypes.THINK]: data.text, - } - : { - index, - type: ContentTypes.TEXT as const, - [ContentTypes.TEXT]: data.text, - }; - - if (!parentMessage) { - return; - } - ask( - { ...parentMessage }, - { - editedContent, - editedMessageId: messageId, - isRegenerate: true, - isEdited: true, - addedConvo: getAddedConvo() || undefined, - }, - ); - - enterEdit(true); - }; - - const updateMessage = (data: { text: string }) => { - const messages = getMessages(); - if (!messages) { - return; - } - updateMessageContentMutation.mutate({ - index, - conversationId: conversationId ?? '', - text: data.text, - messageId, - }); - - const isInMessages = messages.some((msg) => msg.messageId === messageId); - if (!isInMessages) { - return enterEdit(true); - } - - const updatedContent = message?.content?.map((part, idx) => { - if (part.type === ContentTypes.TEXT && idx === index) { - return { ...part, text: data.text }; - } - return part; - }); - - setMessages( - messages.map((msg) => - msg.messageId === messageId - ? { - ...msg, - content: updatedContent, - } - : msg, - ), - ); - - enterEdit(true); - }; - - const handleKeyDown = useCallback( - (e: React.KeyboardEvent) => { - if (e.key === 'Escape') { - e.preventDefault(); - enterEdit(true); - } - }, - [enterEdit], - ); - - const { ref, ...registerProps } = register('text', { - required: true, - onChange: (e) => { - setValue('text', e.target.value, { shouldValidate: true }); - }, - }); - - return ( - - {part.type === ContentTypes.THINK && ( -
- - -
- )} - {part.type !== ContentTypes.THINK && ( -
- - -
- )} -
- { - ref(e); - textAreaRef.current = e; - }} - onKeyDown={handleKeyDown} - data-testid="message-text-editor" - className={cn( - 'markdown prose dark:prose-invert light whitespace-pre-wrap break-words pl-3 md:pl-4', - 'm-0 w-full resize-none border-0 bg-transparent py-[10px]', - 'placeholder-text-secondary focus:ring-0 focus-visible:ring-0 md:py-3.5', - isRTL ? 'text-right' : 'text-left', - 'max-h-[65vh] pr-3 md:max-h-[75vh] md:pr-4', - removeFocusRings, - )} - aria-label={localize('com_ui_editable_message')} - dir={isRTL ? 'rtl' : 'ltr'} - /> -
-
- - - -
- - ); -}; - -export default EditTextPart; diff --git a/client/src/components/Chat/Messages/Content/Parts/SteerPart.tsx b/client/src/components/Chat/Messages/Content/Parts/SteerPart.tsx index 4361481d54..00c50f8e0a 100644 --- a/client/src/components/Chat/Messages/Content/Parts/SteerPart.tsx +++ b/client/src/components/Chat/Messages/Content/Parts/SteerPart.tsx @@ -1,26 +1,20 @@ import { memo, useMemo, useState, useCallback } from 'react'; -import { useAtomValue } from 'jotai'; import { useRecoilValue } from 'recoil'; -import { InfoHoverCard, ESide, UserIcon } from '@librechat/client'; +import { InfoHoverCard, ESide } from '@librechat/client'; import type { TFile, TMessage } from 'librechat-data-provider'; -import type { TMessageIcon } from '~/common'; import FilePreviewDialog from '~/components/Chat/Messages/Content/FilePreviewDialog'; import MessageTimestamp from '~/components/Chat/Messages/ui/MessageTimestamp'; import MarkdownLite from '~/components/Chat/Messages/Content/MarkdownLite'; import FileContainer from '~/components/Chat/Input/Files/FileContainer'; -import MessageIcon from '~/components/Chat/Messages/MessageIcon'; import Image from '~/components/Chat/Messages/Content/Image'; -import { fontSizeAtom } from '~/store/fontSize'; import { useShareContext } from '~/Providers'; import { useLocalize } from '~/hooks'; import { cn } from '~/utils'; import store from '~/store'; -const USER_ICON: TMessageIcon = { isCreatedByUser: true }; - /** * A mid-run steering message rendered as a standard user message inside the - * assistant response — same icon, author header, and text presentation as any + * assistant response, with the same compact surface and text presentation as any * user turn, placed where the words enter the run so the visible order equals * what the next turn replays (`ContentTypes.STEER` splits back into a * HumanMessage server-side). Only the server-applied part renders here, at its @@ -43,7 +37,6 @@ const SteerPart = memo(function SteerPart({ /** Read the atom rather than the auth context: AuthContextProvider mirrors the * user into it, and the public share route mounts outside that provider. */ const user = useRecoilValue(store.user); - const fontSize = useAtomValue(fontSizeAtom); const { isSharedConvo } = useShareContext(); const usernameDisplay = useRecoilValue(store.UsernameDisplay); const enableUserMsgMarkdown = useRecoilValue(store.enableUserMsgMarkdown); @@ -82,51 +75,12 @@ const SteerPart = memo(function SteerPart({ return (
-
-
- {isSharedConvo === true ? ( - /** The atom still holds the viewer's identity when a signed-in user opens - * a share link, so rendering the identity-bearing avatar here would put - * the viewer's face on the sharer's steer. Mirrors Share/MessageIcon. */ -
- -
- ) : ( - - )} -
-
-
-

- {label} - {/* Subtle "?" explaining why a user message appears inside the - * response. Like the message hover buttons, it's revealed on - * hover/focus on hover-capable pointers, but stays visible on - * touch (no hover to reveal it) via [@media(hover:hover)]:opacity-0. */} - - - - -

-
+
+

{label}

+
{(imageFiles.length > 0 || otherFiles.length > 0) && (
{otherFiles.map((file) => ( @@ -151,12 +105,21 @@ const SteerPart = memo(function SteerPart({ className={cn( 'markdown prose message-content dark:prose-invert light w-full break-words', !enableUserMsgMarkdown && 'whitespace-pre-wrap', - 'dark:text-gray-20', + 'text-text-primary', )} > {enableUserMsgMarkdown ? : steer}
+
+ + + + +
{otherFiles.length > 0 && ( ({ default: ({ altText }: { altText: string }) => {altText}, })); -/** Seeds the user atom rather than mocking `useAuthContext`, and renders the real - * MessageIcon tree — mocking either one hid a crash on the share route, where - * neither an auth context nor a user exists. */ +/** Seeds the user atom rather than mocking `useAuthContext`, matching the share + * route where neither an auth context nor a user exists. */ const SEEDED_USER = { name: 'Danny', username: 'danny' }; function renderPart( @@ -58,11 +56,9 @@ function renderPart( user: { name: string; username: string } | null = SEEDED_USER, ) { return render( - - user && set(store.user, user as never)}> - - - , + user && set(store.user, user as never)}> + + , ); } @@ -89,10 +85,10 @@ describe('SteerPart author label', () => { expect(screen.getByText('com_user_message')).toBeInTheDocument(); }); - it('never renders the viewer identity on a shared steer avatar', () => { + it('never renders the viewer identity on a shared steer bubble', () => { /** The user atom is app-wide and survives navigation, so a signed-in viewer * opening a share link still has an identity in state. The shared steer must - * show the generic avatar regardless. */ + * keep generic attribution regardless. */ mockShareContext = { isSharedConvo: true, shareId: 'share-1' }; renderPart(undefined, SEEDED_USER); @@ -130,12 +126,13 @@ describe('SteerPart presentation', () => { mockShareContext = {}; }); - it('presents the steer as a user message with an icon', () => { + it('presents the steer as a compact user bubble with accessible attribution', () => { renderPart(); - /** Asserts the real avatar rather than a stubbed one — the previous mock was - * what hid the auth-context crash inside this icon tree. */ - expect(screen.getByTitle('Danny')).toBeInTheDocument(); - expect(screen.getByText('steered words')).toBeInTheDocument(); + const message = screen.getByText('steered words'); + + expect(message.closest('.bg-surface-tertiary')).toHaveClass('rounded-theme-surface'); + expect(screen.getByRole('heading', { name: 'Danny' })).toHaveClass('sr-only'); + expect(screen.queryByTitle('Danny')).not.toBeInTheDocument(); }); it('anchors the steer for the message-nav rail', () => { diff --git a/client/src/components/Chat/Messages/Content/Parts/index.ts b/client/src/components/Chat/Messages/Content/Parts/index.ts index 68da02ac1b..527654a442 100644 --- a/client/src/components/Chat/Messages/Content/Parts/index.ts +++ b/client/src/components/Chat/Messages/Content/Parts/index.ts @@ -8,7 +8,6 @@ export { default as LogContent } from './LogContent'; export { default as ExecuteCode } from './ExecuteCode'; export { default as Summary } from './Summary'; export { default as AgentUpdate } from './AgentUpdate'; -export { default as EditTextPart } from './EditTextPart'; export { default as SkillCall } from './SkillCall'; export { default as ReadFileCall } from './ReadFileCall'; export { default as FileAuthoringCall } from './FileAuthoringCall'; diff --git a/client/src/components/Chat/Messages/Content/__tests__/ContentParts.integration.test.tsx b/client/src/components/Chat/Messages/Content/__tests__/ContentParts.integration.test.tsx index fa47cd8f78..9d9d1a5830 100644 --- a/client/src/components/Chat/Messages/Content/__tests__/ContentParts.integration.test.tsx +++ b/client/src/components/Chat/Messages/Content/__tests__/ContentParts.integration.test.tsx @@ -78,7 +78,6 @@ jest.mock('../Parts', () => ({ Reasoning: () =>
, Summary: () =>
, Text: ({ text }: { text?: string }) =>
{text}
, - EditTextPart: () =>
, })); jest.mock('../MemoryArtifacts', () => ({ diff --git a/client/src/components/Chat/Messages/Content/__tests__/ContentParts.test.tsx b/client/src/components/Chat/Messages/Content/__tests__/ContentParts.test.tsx index e0a5951a71..df52bb72d7 100644 --- a/client/src/components/Chat/Messages/Content/__tests__/ContentParts.test.tsx +++ b/client/src/components/Chat/Messages/Content/__tests__/ContentParts.test.tsx @@ -30,7 +30,6 @@ jest.mock('~/Providers', () => { }); jest.mock('../Parts', () => ({ - EditTextPart: () =>
, EmptyText: () =>
, AgentUpdate: ({ currentAgentId }: { currentAgentId: string }) => (
diff --git a/client/src/components/Chat/Messages/Content/__tests__/EditContentParts.spec.tsx b/client/src/components/Chat/Messages/Content/__tests__/EditContentParts.spec.tsx new file mode 100644 index 0000000000..b72f353174 --- /dev/null +++ b/client/src/components/Chat/Messages/Content/__tests__/EditContentParts.spec.tsx @@ -0,0 +1,439 @@ +import React from 'react'; +import { ContentTypes } from 'librechat-data-provider'; +import { fireEvent, render, screen, waitFor } from '@testing-library/react'; +import type { FileCitation, TMessage, TMessageContentParts } from 'librechat-data-provider'; +import EditContentParts from '../EditContentParts'; + +const mockMutateAsync = jest.fn(); +const mockSetMessages = jest.fn(); +const mockAsk = jest.fn(); +let mockChatDirection = 'LTR'; + +jest.mock('recoil', () => ({ + useRecoilValue: () => mockChatDirection, +})); + +jest.mock('~/store', () => ({ + __esModule: true, + default: { chatDirection: {} }, +})); + +const message = { + messageId: 'assistant-1', + parentMessageId: 'user-1', + conversationId: 'conversation-1', + isCreatedByUser: false, +} as TMessage; + +const parentMessage = { + messageId: 'user-1', + conversationId: 'conversation-1', + isCreatedByUser: true, + text: 'Check the service', +} as TMessage; + +jest.mock('librechat-data-provider/react-query', () => ({ + useUpdateMessageContentMutation: () => ({ + mutateAsync: mockMutateAsync, + isLoading: false, + }), +})); + +jest.mock('~/Providers', () => ({ + useMessagesConversation: () => ({ + conversation: { conversationId: 'conversation-1' }, + }), + useMessagesOperations: () => ({ + ask: mockAsk, + getMessages: () => [parentMessage, message], + setMessages: mockSetMessages, + }), +})); + +jest.mock('~/hooks', () => ({ + useLocalize: () => (key: string) => key, +})); + +jest.mock('~/hooks/Chat', () => ({ + useGetAddedConvo: () => () => null, +})); + +const content = [ + { type: ContentTypes.TEXT, text: 'Current response' }, + { + type: ContentTypes.TOOL_CALL, + tool_call: { type: 'tool_call', name: 'status', args: '{}', output: 'ok' }, + }, + { type: ContentTypes.ERROR, error: 'A visible tool error' }, +] as TMessageContentParts[]; + +describe('EditContentParts', () => { + beforeEach(() => { + jest.clearAllMocks(); + mockMutateAsync.mockReset(); + mockMutateAsync.mockResolvedValue({}); + mockChatDirection = 'LTR'; + message.content = undefined; + }); + + it('uses one editor footer and keeps non-editable parts visible', () => { + render( +
{part.type}
} + />, + ); + + expect(screen.getAllByRole('textbox')).toHaveLength(1); + expect(screen.getByTestId(`read-only-${ContentTypes.TOOL_CALL}`)).toBeInTheDocument(); + expect(screen.getByTestId(`read-only-${ContentTypes.ERROR}`)).toBeInTheDocument(); + expect(screen.getAllByRole('button')).toHaveLength(3); + }); + + it('saves changed text parts before closing the editor', async () => { + const enterEdit = jest.fn(); + render( + null} + />, + ); + + fireEvent.change(screen.getByRole('textbox'), { target: { value: 'Updated response' } }); + fireEvent.click(screen.getByRole('button', { name: 'com_ui_save' })); + + await waitFor(() => + expect(mockMutateAsync).toHaveBeenCalledWith({ + index: 0, + conversationId: 'conversation-1', + text: 'Updated response', + messageId: message.messageId, + }), + ); + expect(mockSetMessages).toHaveBeenCalledTimes(1); + expect(enterEdit).toHaveBeenCalledWith(true); + }); + + it('reruns an assistant response with the edited content value', () => { + const enterEdit = jest.fn(); + render( + null} + />, + ); + + fireEvent.change(screen.getByRole('textbox'), { target: { value: 'Rerun response' } }); + fireEvent.click(screen.getByRole('button', { name: 'com_ui_update_rerun' })); + + expect(mockAsk).toHaveBeenCalledWith( + parentMessage, + expect.objectContaining({ + editedContent: { + index: 0, + type: ContentTypes.TEXT, + [ContentTypes.TEXT]: 'Rerun response', + }, + }), + ); + expect(enterEdit).toHaveBeenCalledWith(true); + }); + + it('keeps the editor open with the drafts when a rerun is refused mid-stream', () => { + const enterEdit = jest.fn(); + const setSiblingIdx = jest.fn(); + mockAsk.mockReturnValue(false); + render( + null} + />, + ); + + fireEvent.change(screen.getByRole('textbox'), { target: { value: 'Refused rerun' } }); + fireEvent.click(screen.getByRole('button', { name: 'com_ui_update_rerun' })); + + expect(mockAsk).toHaveBeenCalled(); + expect(screen.getByRole('textbox')).toHaveValue('Refused rerun'); + expect(setSiblingIdx).not.toHaveBeenCalled(); + expect(enterEdit).not.toHaveBeenCalled(); + }); + + it('requires multi-part assistant edits to be saved before rerunning', () => { + const multiPartContent = [ + { type: ContentTypes.TEXT, text: 'First response' }, + { type: ContentTypes.TEXT, text: 'Second response' }, + ] as TMessageContentParts[]; + + render( + null} + />, + ); + + const editors = screen.getAllByRole('textbox'); + fireEvent.change(editors[0], { target: { value: 'Updated first response' } }); + fireEvent.change(editors[1], { target: { value: 'Updated second response' } }); + + expect(screen.getByRole('button', { name: 'com_ui_update_rerun' })).toBeDisabled(); + expect(screen.getByText('com_ui_save_before_rerun')).toBeInTheDocument(); + }); + + it('reconciles the parts that were persisted when a later part is refused', async () => { + const enterEdit = jest.fn(); + const multiPartContent = [ + { type: ContentTypes.TEXT, text: 'First response' }, + { type: ContentTypes.TEXT, text: 'Second response' }, + ] as TMessageContentParts[]; + message.content = multiPartContent; + mockMutateAsync + .mockResolvedValueOnce({}) + .mockRejectedValueOnce(new Error('the second write was refused')); + + render( + null} + />, + ); + + const editors = screen.getAllByRole('textbox'); + fireEvent.change(editors[0], { target: { value: 'Updated first response' } }); + fireEvent.change(editors[1], { target: { value: 'Updated second response' } }); + fireEvent.click(screen.getByRole('button', { name: 'com_ui_save' })); + + expect(await screen.findByText('com_ui_save_message_error')).toBeInTheDocument(); + expect(mockMutateAsync).toHaveBeenCalledTimes(2); + + /** The first write is on the server whatever the second one did, so the transcript + * has to show it rather than report that the whole save was lost. */ + expect(mockSetMessages).toHaveBeenCalledTimes(1); + const reconciled = mockSetMessages.mock.calls[0][0] as TMessage[]; + const edited = reconciled.find((item) => item.messageId === message.messageId); + expect(edited?.content).toEqual([ + { type: ContentTypes.TEXT, text: 'Updated first response' }, + { type: ContentTypes.TEXT, text: 'Second response' }, + ]); + + /** The refused part is the only one still holding an unsaved draft, so the editor + * stays open on it and a retry does not rewrite what already landed. */ + expect(enterEdit).not.toHaveBeenCalled(); + expect(screen.getAllByRole('textbox')[1]).toHaveValue('Updated second response'); + }); + + /** + * The Assistants thread sync stores a response carrying file citations as + * `{ value, annotations }`, and the editor reads that through the same union it + * reads a bare string with. Writing the draft over the whole value dropped the + * citations from the transcript, and they stayed dropped until a refetch. + */ + it('edits inside a structured text part rather than flattening the cached one', async () => { + const annotations: FileCitation[] = [ + { + type: 'file_citation', + text: 'source', + start_index: 0, + end_index: 6, + file_citation: { file_id: 'file-1', quote: 'the cited passage' }, + }, + ]; + const structuredContent = [ + { type: ContentTypes.TEXT, text: { value: 'Cited response', annotations } }, + ] as TMessageContentParts[]; + message.content = structuredContent; + + render( + null} + />, + ); + + /** Read back through the union, so the editor opens on the text and not on the wrapper. */ + const editor = screen.getByRole('textbox'); + expect(editor).toHaveValue('Cited response'); + + fireEvent.change(editor, { target: { value: 'Edited cited response' } }); + fireEvent.click(screen.getByRole('button', { name: 'com_ui_save' })); + + await waitFor(() => expect(mockSetMessages).toHaveBeenCalledTimes(1)); + const reconciled = mockSetMessages.mock.calls[0][0] as TMessage[]; + const edited = reconciled.find((item) => item.messageId === message.messageId); + expect(edited?.content).toEqual([ + { type: ContentTypes.TEXT, text: { value: 'Edited cited response', annotations } }, + ]); + }); + + it('keeps artifact-bearing text in the specialized read-only renderer', () => { + const artifactContent = [ + { + type: ContentTypes.TEXT, + text: ':::artifact{identifier="demo" type="text/html" title="Demo"}\n
\n:::', + }, + ] as TMessageContentParts[]; + + render( +
{part.type}
} + />, + ); + + expect(screen.queryByRole('textbox')).toBeNull(); + expect(screen.getByTestId('artifact-part')).toBeInTheDocument(); + }); + + it('refuses to persist a part the editor has been emptied of', async () => { + const enterEdit = jest.fn(); + render( + null} + />, + ); + + const editor = screen.getByRole('textbox'); + fireEvent.change(editor, { target: { value: ' ' } }); + + expect(screen.getByRole('button', { name: 'com_ui_save' })).toBeDisabled(); + expect(screen.getByRole('button', { name: 'com_ui_update_rerun' })).toBeDisabled(); + expect(screen.getByText('com_ui_message_part_empty')).toBeInTheDocument(); + + /** The shortcuts reach the save paths directly, so a disabled button is not + * enough on its own. */ + fireEvent.keyDown(editor, { key: 's', ctrlKey: true }); + fireEvent.keyDown(editor, { key: 'Enter', ctrlKey: true }); + + await waitFor(() => expect(mockMutateAsync).not.toHaveBeenCalled()); + expect(mockAsk).not.toHaveBeenCalled(); + expect(enterEdit).not.toHaveBeenCalled(); + }); + + it('lets a restored part save again after being emptied', async () => { + render( + null} + />, + ); + + const editor = screen.getByRole('textbox'); + fireEvent.change(editor, { target: { value: '' } }); + fireEvent.change(editor, { target: { value: 'Restored response' } }); + + expect(screen.getByRole('button', { name: 'com_ui_save' })).toBeEnabled(); + fireEvent.click(screen.getByRole('button', { name: 'com_ui_save' })); + + await waitFor(() => + expect(mockMutateAsync).toHaveBeenCalledWith( + expect.objectContaining({ text: 'Restored response' }), + ), + ); + }); + + it('turns the editor around for a right-to-left chat direction', () => { + mockChatDirection = 'RTL'; + render( + null} + />, + ); + + const editor = screen.getByRole('textbox'); + expect(editor).toHaveAttribute('dir', 'rtl'); + expect(editor).toHaveClass('text-right'); + }); + + it('leaves the editor left-to-right by default', () => { + render( + null} + />, + ); + + const editor = screen.getByRole('textbox'); + expect(editor).toHaveAttribute('dir', 'ltr'); + expect(editor).toHaveClass('text-left'); + }); + + it('sizes the editor from the configured message font rather than pinning it', () => { + render( + null} + />, + ); + + const editor = screen.getByRole('textbox'); + expect(editor).toHaveClass('message-editor-text'); + expect(editor).not.toHaveClass('text-sm'); + }); +}); diff --git a/client/src/components/Chat/Messages/Content/__tests__/EditMessage.spec.tsx b/client/src/components/Chat/Messages/Content/__tests__/EditMessage.spec.tsx new file mode 100644 index 0000000000..f0db525695 --- /dev/null +++ b/client/src/components/Chat/Messages/Content/__tests__/EditMessage.spec.tsx @@ -0,0 +1,222 @@ +import React from 'react'; +import userEvent from '@testing-library/user-event'; +import { render, screen, waitFor } from '@testing-library/react'; +import type { TMessage } from 'librechat-data-provider'; +import EditMessage from '../EditMessage'; + +const mockMutateAsync = jest.fn(); +const mockSetMessages = jest.fn(); +const mockGetMessages = jest.fn(); + +jest.mock('recoil', () => ({ + useRecoilValue: () => 'ltr', +})); + +jest.mock('~/store', () => ({ + __esModule: true, + default: { chatDirection: {} }, +})); + +jest.mock('librechat-data-provider/react-query', () => ({ + useUpdateMessageMutation: () => ({ + mutateAsync: mockMutateAsync, + isLoading: false, + }), +})); + +jest.mock('~/Providers', () => ({ + useMessagesConversation: () => ({ + conversation: { conversationId: 'conversation-1', model: 'test-model' }, + }), + useMessagesOperations: () => ({ + getMessages: mockGetMessages, + setMessages: mockSetMessages, + }), +})); + +jest.mock('~/hooks', () => ({ + useLocalize: () => (key: string) => key, +})); + +jest.mock('~/hooks/Chat', () => ({ + useGetAddedConvo: () => () => null, +})); + +jest.mock('../Container', () => ({ + __esModule: true, + default: ({ children }: { children: React.ReactNode }) => <>{children}, +})); + +const message = { + messageId: 'user-1', + parentMessageId: 'root', + conversationId: 'conversation-1', + isCreatedByUser: true, + text: 'Original message', +} as TMessage; + +const assistantMessage = { + messageId: 'assistant-1', + parentMessageId: 'user-1', + conversationId: 'conversation-1', + isCreatedByUser: false, + text: 'Original answer', +} as TMessage; + +function renderEditor({ + enterEdit = jest.fn(), + ask = jest.fn(), + setSiblingIdx = jest.fn(), + editedMessage = message, +} = {}) { + render( + , + ); + return { ask, enterEdit, setSiblingIdx }; +} + +describe('EditMessage', () => { + beforeEach(() => { + jest.clearAllMocks(); + mockGetMessages.mockReturnValue([message]); + mockMutateAsync.mockResolvedValue({}); + }); + + it('sizes the editor from the configured message font rather than pinning it', () => { + renderEditor(); + + const editor = screen.getByTestId('message-text-editor'); + expect(editor).toHaveClass('message-editor-text'); + expect(editor).not.toHaveClass('text-sm'); + }); + + it('waits for a successful save before updating local state and closing', async () => { + const user = userEvent.setup(); + const { enterEdit } = renderEditor(); + + await user.clear(screen.getByTestId('message-text-editor')); + await user.type(screen.getByTestId('message-text-editor'), 'Updated message'); + await user.click(screen.getByRole('button', { name: 'com_ui_save' })); + + await waitFor(() => + expect(mockMutateAsync).toHaveBeenCalledWith({ + conversationId: message.conversationId, + model: 'test-model', + text: 'Updated message', + messageId: message.messageId, + }), + ); + expect(mockSetMessages).toHaveBeenCalledWith([ + expect.objectContaining({ messageId: message.messageId, text: 'Updated message' }), + ]); + expect(enterEdit).toHaveBeenCalledWith(true); + }); + + it('writes the save onto the thread as it stands when the request resolves', async () => { + const user = userEvent.setup(); + const streamedAnswer = { + messageId: 'assistant-streaming', + parentMessageId: message.messageId, + conversationId: 'conversation-1', + isCreatedByUser: false, + text: 'Half an answer', + } as TMessage; + + /** The answer keeps streaming into the cache while the request is in flight. */ + mockMutateAsync.mockImplementation(async () => { + mockGetMessages.mockReturnValue([message, streamedAnswer]); + return {}; + }); + + renderEditor(); + + await user.clear(screen.getByTestId('message-text-editor')); + await user.type(screen.getByTestId('message-text-editor'), 'Updated message'); + await user.click(screen.getByRole('button', { name: 'com_ui_save' })); + + await waitFor(() => expect(mockSetMessages).toHaveBeenCalled()); + expect(mockSetMessages).toHaveBeenCalledWith([ + expect.objectContaining({ messageId: message.messageId, text: 'Updated message' }), + expect.objectContaining({ messageId: streamedAnswer.messageId, text: 'Half an answer' }), + ]); + }); + + it('keeps the editor open with the draft when saving fails', async () => { + const user = userEvent.setup(); + mockMutateAsync.mockRejectedValue(new Error('Save failed')); + const { enterEdit } = renderEditor(); + + await user.clear(screen.getByTestId('message-text-editor')); + await user.type(screen.getByTestId('message-text-editor'), 'Unsaved message'); + await user.click(screen.getByRole('button', { name: 'com_ui_save' })); + + expect(await screen.findByText('com_ui_save_message_error')).toBeInTheDocument(); + expect(screen.getByTestId('message-text-editor')).toHaveValue('Unsaved message'); + expect(enterEdit).not.toHaveBeenCalled(); + }); + + it('submits the edited user message with its original context', async () => { + const user = userEvent.setup(); + const ask = jest.fn(); + const { enterEdit } = renderEditor({ ask }); + + await user.clear(screen.getByTestId('message-text-editor')); + await user.type(screen.getByTestId('message-text-editor'), 'Updated and rerun'); + await user.click(screen.getByRole('button', { name: 'com_ui_update_rerun' })); + + await waitFor(() => + expect(ask).toHaveBeenCalledWith( + expect.objectContaining({ + text: 'Updated and rerun', + parentMessageId: message.parentMessageId, + conversationId: message.conversationId, + }), + expect.objectContaining({ + overrideFiles: message.files, + overrideManualSkills: message.manualSkills, + overrideQuotes: message.quotes, + }), + ), + ); + expect(enterEdit).toHaveBeenCalledWith(true); + }); + + it('keeps the editor open with the draft when a rerun is refused mid-stream', async () => { + const user = userEvent.setup(); + const ask = jest.fn().mockReturnValue(false); + const { enterEdit, setSiblingIdx } = renderEditor({ ask }); + + await user.clear(screen.getByTestId('message-text-editor')); + await user.type(screen.getByTestId('message-text-editor'), 'Refused rerun'); + await user.click(screen.getByRole('button', { name: 'com_ui_update_rerun' })); + + await waitFor(() => expect(ask).toHaveBeenCalled()); + expect(screen.getByTestId('message-text-editor')).toHaveValue('Refused rerun'); + expect(setSiblingIdx).not.toHaveBeenCalled(); + expect(enterEdit).not.toHaveBeenCalled(); + }); + + it('keeps an assistant edit open when a refused rerun would discard it', async () => { + const user = userEvent.setup(); + mockGetMessages.mockReturnValue([message, assistantMessage]); + const ask = jest.fn().mockReturnValue(false); + const { enterEdit, setSiblingIdx } = renderEditor({ ask, editedMessage: assistantMessage }); + + await user.clear(screen.getByTestId('message-text-editor')); + await user.type(screen.getByTestId('message-text-editor'), 'Refused answer edit'); + await user.click(screen.getByRole('button', { name: 'com_ui_update_rerun' })); + + await waitFor(() => expect(ask).toHaveBeenCalled()); + expect(screen.getByTestId('message-text-editor')).toHaveValue('Refused answer edit'); + expect(setSiblingIdx).not.toHaveBeenCalled(); + expect(enterEdit).not.toHaveBeenCalled(); + }); +}); diff --git a/client/src/components/Chat/Messages/Feedback.tsx b/client/src/components/Chat/Messages/Feedback.tsx index 24b521283d..af6b063797 100644 --- a/client/src/components/Chat/Messages/Feedback.tsx +++ b/client/src/components/Chat/Messages/Feedback.tsx @@ -21,6 +21,7 @@ import { ThumbUpIcon, ThumbDownIcon, } from '@librechat/client'; +import { hoverButtonClasses } from './styles'; import { useLocalize } from '~/hooks'; import { cn } from '~/utils'; @@ -218,17 +219,8 @@ function FeedbackButtons({ ); } -function buttonClasses(isActive: boolean, isLast: boolean) { - return cn( - 'hover-button size-auto rounded-lg p-1.5 text-text-secondary-alt', - 'hover:text-text-primary hover:bg-surface-hover', - 'group-hover:visible group-focus-within:visible group-[.final-completion]:visible', - !isLast && - 'group-hover:opacity-100 group-focus-within:opacity-100 [@media(hover:hover)]:opacity-0', - 'focus-visible:ring-2 focus-visible:ring-text-primary focus-visible:outline-none', - isActive && 'active text-text-primary bg-surface-hover', - ); -} +const buttonClasses = (isActive: boolean, isLast: boolean) => + hoverButtonClasses({ isActive, isLast }); export default function Feedback({ isLast = false, diff --git a/client/src/components/Chat/Messages/Fork.tsx b/client/src/components/Chat/Messages/Fork.tsx index 653d83ba22..341c7187fc 100644 --- a/client/src/components/Chat/Messages/Fork.tsx +++ b/client/src/components/Chat/Messages/Fork.tsx @@ -8,7 +8,7 @@ import { GitCommit, GitBranchPlus, ListTree } from 'lucide-react'; import { Button, Label, Checkbox, useToastContext } from '@librechat/client'; import { TranslationKeys, useLocalize, useNavigateToConvo } from '~/hooks'; import { useForkConvoMutation } from '~/data-provider'; -import { cn } from '~/utils'; +import { hoverButtonClasses } from './styles'; import store from '~/store'; interface PopoverButtonProps { @@ -216,7 +216,6 @@ export default function Fork({ const { showToast } = useToastContext(); const [remember, setRemember] = useState(false); const { navigateToConvo } = useNavigateToConvo(); - const [isActive, setIsActive] = useState(false); const timeoutRef = useRef(null); const [forkSetting, setForkSetting] = useRecoilState(store.forkSetting); const [activeSetting, setActiveSetting] = useState(optionLabels.default); @@ -225,16 +224,12 @@ export default function Fork({ const popoverStore = Ariakit.usePopoverStore({ placement: 'bottom', }); + /** Read the open state from the store rather than mirroring it: Escape and + * outside clicks close the popover without going through the trigger, so a + * hand-kept copy would leave the button reading as active forever. */ + const isActive = Ariakit.useStoreState(popoverStore, 'open'); - const buttonStyle = cn( - 'hover-button size-auto rounded-lg p-1.5 text-text-secondary-alt', - 'hover:text-text-primary hover:bg-surface-hover', - 'group-hover:visible group-focus-within:visible group-[.final-completion]:visible', - !isLast && - 'group-hover:opacity-100 group-focus-within:opacity-100 [@media(hover:hover)]:opacity-0', - 'focus-visible:ring-2 focus-visible:ring-text-primary focus-visible:outline-none', - isActive && 'active text-text-primary bg-surface-hover', - ); + const buttonStyle = hoverButtonClasses({ isActive, isLast }); const forkConvo = useForkConvoMutation({ onSuccess: (data) => { @@ -346,7 +341,6 @@ export default function Fork({ }); } else { popoverStore.toggle(); - setIsActive(popoverStore.getState().open); } }} type="button" @@ -367,7 +361,6 @@ export default function Fork({ }} portal={true} unmountOnHide={true} - onClose={() => setIsActive(false)} >
{localize(activeSetting)} diff --git a/client/src/components/Chat/Messages/HoverButtons.tsx b/client/src/components/Chat/Messages/HoverButtons.tsx index b1a627953c..98771fd2b6 100644 --- a/client/src/components/Chat/Messages/HoverButtons.tsx +++ b/client/src/components/Chat/Messages/HoverButtons.tsx @@ -12,6 +12,7 @@ import { import type { TConversation, TMessage, TFeedback } from 'librechat-data-provider'; import { useGenerationsByLatest, useLocalize } from '~/hooks'; import { Fork } from '~/components/Conversations'; +import { hoverButtonClasses } from './styles'; import MessageAudio from './MessageAudio'; import Feedback from './Feedback'; import { cn } from '~/utils'; @@ -38,8 +39,6 @@ type HoverButtonProps = { title: string; icon: React.ReactNode; isActive?: boolean; - isVisible?: boolean; - isDisabled?: boolean; isLast?: boolean; className?: string; buttonStyle?: string; @@ -85,26 +84,11 @@ const HoverButton = memo( title, icon, isActive = false, - isVisible = true, - isDisabled = false, isLast = false, className = '', dataTestId, }: HoverButtonProps) => { - const buttonStyle = cn( - 'hover-button size-auto rounded-lg p-1.5 text-text-secondary-alt', - 'hover:text-text-primary hover:bg-surface-hover', - 'group-hover:visible group-focus-within:visible group-[.final-completion]:visible', - !isLast && - isVisible && - 'group-hover:opacity-100 group-focus-within:opacity-100 [@media(hover:hover)]:opacity-0', - /** `!` is load-bearing: the shared Button sets `disabled:opacity-50`, which outranks a - * plain `opacity-0` and would leave a dimmed ghost of the hidden action on screen. */ - !isVisible && 'pointer-events-none !opacity-0', - 'focus-visible:ring-2 focus-visible:ring-text-primary focus-visible:outline-none', - isActive && isVisible && 'active text-text-primary bg-surface-hover', - className, - ); + const buttonStyle = hoverButtonClasses({ isActive, isLast, className }); return ( {icon} @@ -172,6 +155,7 @@ const HoverButtons = ({ regenerateEnabled, continueSupported, forkingSupported, + isActiveStreamingMessage, isEditableEndpoint, } = generationCapabilities; @@ -181,22 +165,6 @@ const HoverButtons = ({ const { isCreatedByUser, error } = message; - if (error === true) { - return ( -
- {regenerateEnabled && ( - } - isLast={isLast} - dataTestId={isLast ? 'regenerate-generation-button' : undefined} - /> - )} -
- ); - } - const onEdit = () => { if (isEditing) { return enterEdit(true); @@ -209,7 +177,7 @@ const HoverButtons = ({ return (
{/* Text to Speech */} - {TextToSpeech && ( + {TextToSpeech && !error && !isActiveStreamingMessage && ( : } - isLast={isLast} - className={cn( - 'ml-0 flex items-center gap-1.5 text-xs', - isSubmitting && isCreatedByUser - ? 'group-hover:opacity-100 [@media(hover:hover)]:opacity-0' - : '', - )} - dataTestId={!isCreatedByUser ? 'copy-response-button' : undefined} - /> + {!isActiveStreamingMessage && ( + : } + isLast={isLast} + className={cn( + 'ml-0 flex items-center gap-1.5 text-xs', + isSubmitting && isCreatedByUser + ? 'group-hover:opacity-100 [@media(hover:hover)]:opacity-0' + : '', + )} + dataTestId={!isCreatedByUser ? 'copy-response-button' : undefined} + /> + )} {/* Edit Button */} - {isEditableEndpoint && ( + {isEditableEndpoint && !hideEditButton && ( } isActive={isEditing} - isVisible={!hideEditButton} - isDisabled={hideEditButton} isLast={isLast} className={isCreatedByUser ? '' : 'active'} /> )} {/* Fork Button */} - + {!error && !isActiveStreamingMessage && ( + + )} {/* Feedback Buttons */} - {!isCreatedByUser && handleFeedback != null && ( + {!error && !isActiveStreamingMessage && !isCreatedByUser && handleFeedback != null && ( )} diff --git a/client/src/components/Chat/Messages/Message.tsx b/client/src/components/Chat/Messages/Message.tsx index 9a1ce7313e..cd52066cdb 100644 --- a/client/src/components/Chat/Messages/Message.tsx +++ b/client/src/components/Chat/Messages/Message.tsx @@ -13,7 +13,7 @@ const MessageContainer = React.memo(function MessageContainer({ }) { return (
@@ -35,7 +35,7 @@ function Message(props: TMessageProps) { return ( -
+
diff --git a/client/src/components/Chat/Messages/MessageParts.tsx b/client/src/components/Chat/Messages/MessageParts.tsx index 6091cb0993..151a70f913 100644 --- a/client/src/components/Chat/Messages/MessageParts.tsx +++ b/client/src/components/Chat/Messages/MessageParts.tsx @@ -1,5 +1,4 @@ import React, { useMemo } from 'react'; -import { useAtomValue } from 'jotai'; import { useRecoilValue } from 'recoil'; import type { TMessageContentParts } from 'librechat-data-provider'; import type { TMessageProps, TMessageIcon } from '~/common'; @@ -11,10 +10,10 @@ import { } from '~/utils'; import { useMessageHelpers, useLocalize, useAttachments, useContentMetadata } from '~/hooks'; import AuthorHeader from '~/components/Chat/Messages/Content/Parts/AuthorHeader'; -import MessageTimestamp from '~/components/Chat/Messages/ui/MessageTimestamp'; +import { revealOnRowHoverClasses, messageFooterClasses } from './styles'; +import MessageRow from '~/components/Chat/Messages/ui/MessageRow'; import MessageIcon from '~/components/Chat/Messages/MessageIcon'; import ContentParts from './Content/ContentParts'; -import { fontSizeAtom } from '~/store/fontSize'; import SiblingSwitch from './SiblingSwitch'; import HoverButtons from './HoverButtons'; import SubRow from './SubRow'; @@ -43,7 +42,6 @@ function MessageParts(props: TMessageProps) { regenerateMessage, } = useMessageHelpers(props); - const fontSize = useAtomValue(fontSizeAtom); const maximizeChatSpace = useRecoilValue(store.maximizeChatSpace); const { messageId = null, isCreatedByUser } = message ?? {}; @@ -97,112 +95,73 @@ function MessageParts(props: TMessageProps) { return null; } - const getChatWidthClass = () => { - if (maximizeChatSpace) { - return 'w-full max-w-full md:px-5 lg:px-1 xl:px-5'; - } - if (hasParallelContent) { - return 'md:max-w-[58rem] xl:max-w-[70rem]'; - } - return 'md:max-w-[47rem] xl:max-w-[55rem]'; - }; - - const baseClasses = { - common: 'group mx-auto flex flex-1 gap-3 transition-all duration-300 transform-gpu', - chat: getChatWidthClass(), - }; - return ( - <> -
-
-
- {!hasParallelContent && ( -
-
- -
-
- )} -
- {!hasParallelContent && ( -

- - {getHeaderPrefixForScreenReader(message, localize)} - - {name} - -

- )} -
-
- } - /> -
- {isLast && isSubmitting ? ( -
- ) : ( - - - regenerateMessage()} - copyToClipboard={copyToClipboard} - handleContinue={handleContinue} - latestMessageId={latestMessageId} - isLast={isLast} - /> - +
+
+ } + label={name} + timestamp={message.createdAt ?? message.clientTimestamp} + ariaLabel={getMessageAriaLabel(message, localize)} + headerPrefix={getHeaderPrefixForScreenReader(message, localize)} + isCreatedByUser={isCreatedByUser === true} + hasParallelContent={hasParallelContent} + fullWidth={maximizeChatSpace} + isEditing={edit} + footer={ + + {/* While the answer is generating every other action is withheld, which + would otherwise leave this counter sitting alone under a half-written + response. It reveals on hover there, like the actions it sits with. */} + -
-
-
+ /> + regenerateMessage()} + copyToClipboard={copyToClipboard} + handleContinue={handleContinue} + latestMessageId={latestMessageId} + isLast={isLast} + /> + + } + > + } + /> +
- +
); } diff --git a/client/src/components/Chat/Messages/SearchMessage.tsx b/client/src/components/Chat/Messages/SearchMessage.tsx index 6405f7bff8..42f972f6e5 100644 --- a/client/src/components/Chat/Messages/SearchMessage.tsx +++ b/client/src/components/Chat/Messages/SearchMessage.tsx @@ -1,46 +1,18 @@ import { memo, useMemo } from 'react'; -import { useAtomValue } from 'jotai'; import { useRecoilValue } from 'recoil'; import type { TMessage } from 'librechat-data-provider'; import type { TMessageProps, TMessageIcon } from '~/common'; import AuthorHeader from '~/components/Chat/Messages/Content/Parts/AuthorHeader'; import MinimalHoverButtons from '~/components/Chat/Messages/MinimalHoverButtons'; -import MessageTimestamp from '~/components/Chat/Messages/ui/MessageTimestamp'; +import { getHeaderPrefixForScreenReader, getMessageAriaLabel } from '~/utils'; +import MessageRow from '~/components/Chat/Messages/ui/MessageRow'; import Icon from '~/components/Chat/Messages/MessageIcon'; import { useAuthContext, useLocalize } from '~/hooks'; import SearchContent from './Content/SearchContent'; -import { fontSizeAtom } from '~/store/fontSize'; import SearchButtons from './SearchButtons'; import SubRow from './SubRow'; -import { cn } from '~/utils'; import store from '~/store'; -const MessageAvatar = ({ iconData }: { iconData: TMessageIcon }) => ( -
-
-
- -
-
-
-); - -const MessageBody = ({ message, messageLabel, fontSize, authorHeader }) => ( -
-
- {messageLabel} - -
- - - - - -
-); - function searchFilesEqual(prev?: TMessage['files'], next?: TMessage['files']) { if (prev === next) { return true; @@ -94,7 +66,6 @@ export function areSearchMessagePropsEqual( } function SearchMessage({ message }: Pick) { - const fontSize = useAtomValue(fontSizeAtom); const UsernameDisplay = useRecoilValue(store.UsernameDisplay); const { user } = useAuthContext(); const localize = useLocalize(); @@ -138,17 +109,26 @@ function SearchMessage({ message }: Pick) { } return ( -
-
-
- - -
+
+
+ } + label={messageLabel} + timestamp={message.createdAt ?? message.clientTimestamp} + ariaLabel={getMessageAriaLabel(message, localize)} + headerPrefix={getHeaderPrefixForScreenReader(message, localize)} + isCreatedByUser={message.isCreatedByUser === true} + className="final-completion" + footer={ + + + + + } + > + +
); diff --git a/client/src/components/Chat/Messages/SiblingSwitch.tsx b/client/src/components/Chat/Messages/SiblingSwitch.tsx index bdc9a6489c..4e566fb1c5 100644 --- a/client/src/components/Chat/Messages/SiblingSwitch.tsx +++ b/client/src/components/Chat/Messages/SiblingSwitch.tsx @@ -4,12 +4,15 @@ import type { TMessageProps } from '~/common'; import { useLocalize } from '~/hooks'; import { cn } from '~/utils'; -type TSiblingSwitchProps = Pick; +type TSiblingSwitchProps = Pick & { + className?: string; +}; export default function SiblingSwitch({ siblingIdx, siblingCount, setSiblingIdx, + className, }: TSiblingSwitchProps) { const localize = useLocalize(); @@ -36,7 +39,10 @@ export default function SiblingSwitch({ return siblingCount > 1 ? (