Skip to content

🫙 fix: Preserve Loaded Message Content When Resume Snapshot Is Empty #14399

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
danny-avila merged 3 commits into from
Jul 23, 2026
+189 −1
Merged

🫙 fix: Preserve Loaded Message Content When Resume Snapshot Is Empty #14399

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
bbe9785
🛟 fix: Keep Loaded Message Content When a Resume Snapshot Is Empty
danny-avila Jul 23, 2026
7915c15
🔒 fix: Scope Resume Content Preservation to Identity-Matched Responses
danny-avila Jul 23, 2026
9a3beeb
🧱 fix: Only Preserve Resume Content When the Row Actually Has Parts
danny-avila Jul 23, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
82 changes: 82 additions & 0 deletions client/src/hooks/SSE/__tests__/useResumableSSE.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1239,6 +1239,88 @@ describe('useResumableSSE', () => {
unmount();
});

describe('sync content reconciliation', () => {
const DB_CONTENT = [{ type: 'text', text: 'streamed before the reload' }];

const renderWithLoadedResponse = () => {
const submission = buildSubmission();
const chatHelpers = buildChatHelpers();
chatHelpers.getMessages.mockReturnValue([
{
messageId: 'msg-1',
conversationId: CONV_ID,
isCreatedByUser: true,
text: 'Hello',
},
{
messageId: 'resp-1',
parentMessageId: 'msg-1',
conversationId: CONV_ID,
isCreatedByUser: false,
text: '',
content: DB_CONTENT,
},
] as unknown as TMessage[]);
return { submission, chatHelpers };
};

const emitSync = async (aggregatedContent: unknown[]) => {
const sse = getLastSSE();
await act(async () => {
sse._emit('message', {
data: JSON.stringify({
sync: true,
resumeState: {
runSteps: [],
aggregatedContent,
responseMessageId: 'resp-1',
},
}),
});
});
};

const syncedResponse = (chatHelpers: ReturnType<typeof buildChatHelpers>) => {
const call = chatHelpers.setMessages.mock.calls
.map(([messages]) => messages as TMessage[])
.reverse()
.find((messages) => messages?.some((m) => m.messageId === 'resp-1'));
return call?.find((m) => m.messageId === 'resp-1');
};

/**
* An empty snapshot is what a resuming client gets when the conversation's job was
* replaced mid-flight. Assigning it erased the content the messages query had already
* loaded, leaving a message that renders as a bare cursor forever.
*/
it('keeps already-loaded content when the resume snapshot is empty', async () => {
const { submission, chatHelpers } = renderWithLoadedResponse();
const { unmount } = renderHook(() => useResumableSSE(submission, chatHelpers));
await act(async () => {
await Promise.resolve();
});
Comment on lines +1299 to +1301

await emitSync([]);

expect(syncedResponse(chatHelpers)?.content).toEqual(DB_CONTENT);
unmount();
});

it('still applies the resume snapshot when it carries content', async () => {
const { submission, chatHelpers } = renderWithLoadedResponse();
const { unmount } = renderHook(() => useResumableSSE(submission, chatHelpers));
await act(async () => {
await Promise.resolve();
});

const resumed = [{ type: 'text', text: 'authoritative resumed content' }];
await emitSync(resumed);

expect(syncedResponse(chatHelpers)?.content).toEqual(resumed);
unmount();
});
});

it('replays OAuth run step delta events from resume state sync', async () => {
const submission = buildSubmission();
const chatHelpers = buildChatHelpers();
Expand Down
7 changes: 6 additions & 1 deletion client/src/hooks/SSE/useResumableSSE.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1006,6 +1006,7 @@ export default function useResumableSSE(
const messages = getMessages() ?? [];
const userMsgId = userMessage.messageId;
const serverResponseId = data.resumeState.responseMessageId;
const hasResumedContent = data.resumeState.aggregatedContent.length > 0;

let responseIdx = -1;
if (serverResponseId) {
Expand All @@ -1030,9 +1031,12 @@ export default function useResumableSSE(

if (responseIdx >= 0) {
const oldContent = messages[responseIdx]?.content;
/** An EMPTY resume snapshot is not authoritative over what we already have:
* assigning it would erase content the messages query loaded, leaving a bare
* cursor. Keep the loaded content and let live deltas take over. */
const responseMessage = {
...messages[responseIdx],
content: data.resumeState.aggregatedContent,
content: hasResumedContent ? data.resumeState.aggregatedContent : oldContent,

@chatgpt-codex-connector chatgpt-codex-connector Bot Jul 23, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Avoid preserving stale regenerate content

When a user reloads during an in-flight regenerate before the new run has aggregated any content, the server can report an empty aggregatedContent with a padded responseMessageId, while the loaded DB history still contains the old assistant response under the same parent. This fallback can select that old response, and this line now seeds syncStepMessage with its old content; subsequent text deltas append to that stale answer instead of starting the regenerated response from blank.

Useful? React with 👍 / 👎.

@danny-avila danny-avila Jul 23, 2026

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch, fixed in 7915c15.

You're right that the parent-based fallback can land on the answer being regenerated: it matches any !isCreatedByUser row whose parentMessageId === userMsgId, and on a regenerate the new run's responseMessageId isn't in the loaded history yet, so that fallback is exactly what fires.

Preservation is now scoped to rows matched by the server's declared responseMessageId, which is the only case that proves the row belongs to this generation:

let matchedByResponseId = false;
if (serverResponseId) {
  responseIdx = messages.findIndex((m) => m.messageId === serverResponseId);
  matchedByResponseId = responseIdx >= 0;
}
// ...
const preserveLoadedContent = !hasResumedContent && matchedByResponseId;

A fallback-matched row keeps the previous clear-on-empty behavior, so a regenerated run still starts blank. This also keeps the original fix effective for the case it targets, since a resumed placeholder and a persisted partial both carry the server's response id.

Added does not preserve content on a row matched only by the parent fallback to cover it; verified it fails against the previous version of the guard and passes now.

iconURL: preferDefinedString(
messages[responseIdx]?.iconURL,
data.resumeState.iconURL,
Expand All @@ -1044,6 +1048,7 @@ export default function useResumableSSE(
messageId: responseMessage.messageId,
oldContentLength: Array.isArray(oldContent) ? oldContent.length : 0,
newContentLength: data.resumeState.aggregatedContent?.length,
preservedExistingContent: !hasResumedContent,
});
setMessages(updated);
resetContentHandler();
Expand Down
Loading