Keep chat turns in the order they were sent (#138)
## Summary - Closes #136. - Chat turns are sorted oldest-first by `created`, then id, whenever they are written into state. A scrambled `conversation_details` payload no longer redraws the thread backwards after the reply lands. - `START_OF_THE_STREAM` no longer copies the websocket closure into the message ref. That copy could drop the optimistic turn and let `END` rebuild the thread from the previous list. - Optimistic bubbles use a stable local key so they do not collide with a server id. - The account toggle is labeled "Remember this chat" and defaults on, matching chat_backend#84. ## Test plan - [ ] `CI=true npm test -- --watchAll=false --testPathPattern='promptOrder|MessageContext|data.test'` - [ ] Ask for a joke, then "I don't know, why?". While it works, order stays: joke request, joke, follow-up, status. After the reply, the follow-up stays under the joke and the new answer is last, with the action row on that answer. - [ ] Reload the conversation. Order stays the same. - [ ] Send another follow-up. It appends at the bottom. - [ ] Needs chat_backend#84 (or that branch) for the reply to actually use the previous turn. Reviewed-on: #138
This commit was merged in pull request #138.
This commit is contained in:
1 parent
6b6df4247f
commit
ae59c2731d
8 files changed
+246
-14
No files matched your search
@@ -139,7 +139,7 @@ const ErrorText = styled.p`
|
|||||||
`;
|
`;
|
||||||
|
|
||||||
const CONTEXT_TOOLTIP =
|
const CONTEXT_TOOLTIP =
|
||||||
'This will use previous conversations to better customize the experience.';
|
'Follow-ups use earlier messages in this chat. Turn off to answer each message on its own.';
|
||||||
|
|
||||||
function prefsErrorMessage(error: unknown, fallback: string): string {
|
function prefsErrorMessage(error: unknown, fallback: string): string {
|
||||||
const axiosError = error as {
|
const axiosError = error as {
|
||||||
@@ -162,7 +162,7 @@ const ThemeSettingsCard = (): JSX.Element => {
|
|||||||
const themeColors = Object.keys(palettes);
|
const themeColors = Object.keys(palettes);
|
||||||
|
|
||||||
const [order, setOrder] = useState<boolean>(true);
|
const [order, setOrder] = useState<boolean>(true);
|
||||||
const [useConversationContext, setUseConversationContext] = useState<boolean>(false);
|
const [useConversationContext, setUseConversationContext] = useState<boolean>(true);
|
||||||
const [prefsLoading, setPrefsLoading] = useState<boolean>(true);
|
const [prefsLoading, setPrefsLoading] = useState<boolean>(true);
|
||||||
const [contextSaving, setContextSaving] = useState<boolean>(false);
|
const [contextSaving, setContextSaving] = useState<boolean>(false);
|
||||||
const [prefsError, setPrefsError] = useState<string>('');
|
const [prefsError, setPrefsError] = useState<string>('');
|
||||||
@@ -276,7 +276,7 @@ const ThemeSettingsCard = (): JSX.Element => {
|
|||||||
|
|
||||||
<SettingRow>
|
<SettingRow>
|
||||||
<SettingLabel>
|
<SettingLabel>
|
||||||
Use previous conversations
|
Remember this chat
|
||||||
<Tooltip title={CONTEXT_TOOLTIP}>
|
<Tooltip title={CONTEXT_TOOLTIP}>
|
||||||
<span>
|
<span>
|
||||||
<InfoIcon aria-label={CONTEXT_TOOLTIP} />
|
<InfoIcon aria-label={CONTEXT_TOOLTIP} />
|
||||||
|
|||||||
@@ -0,0 +1,128 @@
|
|||||||
|
import React, { useContext } from 'react';
|
||||||
|
import { act, render, screen } from '@testing-library/react';
|
||||||
|
import { MessageContext, MessageProvider } from './MessageContext';
|
||||||
|
import { WebSocketContext } from './WebSocketContext';
|
||||||
|
import { AccountContext } from './AccountContext';
|
||||||
|
import { ConversationContext } from './ConversationContext';
|
||||||
|
import { ConversationPrompt } from '../data';
|
||||||
|
|
||||||
|
jest.mock('../../axiosApi', () => ({
|
||||||
|
axiosInstance: {
|
||||||
|
get: jest.fn().mockResolvedValue({ data: [] }),
|
||||||
|
},
|
||||||
|
}));
|
||||||
|
|
||||||
|
jest.mock('../utils/analytics', () => ({
|
||||||
|
AnalyticsEvents: {
|
||||||
|
CONVERSATION_CREATED: 'Conversation Created',
|
||||||
|
ACTIVITY_STAGE_COMPLETED: 'Activity Stage Completed',
|
||||||
|
},
|
||||||
|
trackEvent: jest.fn(),
|
||||||
|
}));
|
||||||
|
|
||||||
|
const OrderProbe = () => {
|
||||||
|
const ctx = useContext(MessageContext);
|
||||||
|
return (
|
||||||
|
<div>
|
||||||
|
<span data-testid="order">
|
||||||
|
{ctx.conversationDetails.map((detail) => detail.message).join('|')}
|
||||||
|
</span>
|
||||||
|
<button
|
||||||
|
type="button"
|
||||||
|
onClick={() =>
|
||||||
|
ctx.setConversationDetails([
|
||||||
|
new ConversationPrompt({
|
||||||
|
id: 4,
|
||||||
|
message: 'I dont know',
|
||||||
|
user_created: true,
|
||||||
|
created_timestamp: new Date('2020-01-01T00:03:00Z'),
|
||||||
|
}),
|
||||||
|
new ConversationPrompt({
|
||||||
|
id: 2,
|
||||||
|
message: 'Tell me a joke',
|
||||||
|
user_created: true,
|
||||||
|
created_timestamp: new Date('2020-01-01T00:01:00Z'),
|
||||||
|
}),
|
||||||
|
new ConversationPrompt({
|
||||||
|
message: '',
|
||||||
|
user_created: false,
|
||||||
|
created_timestamp: new Date('2020-01-01T00:04:00Z'),
|
||||||
|
}),
|
||||||
|
new ConversationPrompt({
|
||||||
|
id: 3,
|
||||||
|
message: 'meditation ladder',
|
||||||
|
user_created: false,
|
||||||
|
created_timestamp: new Date('2020-01-01T00:02:00Z'),
|
||||||
|
}),
|
||||||
|
])
|
||||||
|
}
|
||||||
|
>
|
||||||
|
seed
|
||||||
|
</button>
|
||||||
|
</div>
|
||||||
|
);
|
||||||
|
};
|
||||||
|
|
||||||
|
describe('MessageContext turn order (#136)', () => {
|
||||||
|
it('sorts a scrambled transcript and keeps that order when the reply finishes', async () => {
|
||||||
|
let callback: ((message: string) => void) | null = null;
|
||||||
|
render(
|
||||||
|
<AccountContext.Provider
|
||||||
|
value={{ account: { email: 'test@example.com' } as never, setAccount: () => {} }}
|
||||||
|
>
|
||||||
|
<ConversationContext.Provider
|
||||||
|
value={{
|
||||||
|
conversations: [],
|
||||||
|
setConversations: () => {},
|
||||||
|
selectedConversation: 7,
|
||||||
|
setSelectedConversation: () => {},
|
||||||
|
deleteConversation: () => {},
|
||||||
|
}}
|
||||||
|
>
|
||||||
|
<WebSocketContext.Provider
|
||||||
|
value={
|
||||||
|
[
|
||||||
|
(_channel: string, cb: (message: string) => void) => {
|
||||||
|
callback = cb;
|
||||||
|
},
|
||||||
|
jest.fn(),
|
||||||
|
null,
|
||||||
|
jest.fn(),
|
||||||
|
true,
|
||||||
|
'CONNECTED',
|
||||||
|
0,
|
||||||
|
] as never
|
||||||
|
}
|
||||||
|
>
|
||||||
|
<MessageProvider>
|
||||||
|
<OrderProbe />
|
||||||
|
</MessageProvider>
|
||||||
|
</WebSocketContext.Provider>
|
||||||
|
</ConversationContext.Provider>
|
||||||
|
</AccountContext.Provider>,
|
||||||
|
);
|
||||||
|
await act(async () => {});
|
||||||
|
|
||||||
|
act(() => {
|
||||||
|
screen.getByRole('button', { name: 'seed' }).click();
|
||||||
|
});
|
||||||
|
expect(screen.getByTestId('order')).toHaveTextContent(
|
||||||
|
'Tell me a joke|meditation ladder|I dont know|',
|
||||||
|
);
|
||||||
|
|
||||||
|
const send = (message: string) => {
|
||||||
|
act(() => {
|
||||||
|
callback?.(message);
|
||||||
|
});
|
||||||
|
};
|
||||||
|
send('CONVERSATION_ID');
|
||||||
|
send('7');
|
||||||
|
send('START_OF_THE_STREAM_ENDER_GAME_42');
|
||||||
|
send('To help everyone step');
|
||||||
|
send('END_OF_THE_STREAM_ENDER_GAME_42');
|
||||||
|
|
||||||
|
expect(screen.getByTestId('order').textContent).toBe(
|
||||||
|
'Tell me a joke|meditation ladder|I dont know|To help everyone step',
|
||||||
|
);
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -13,6 +13,7 @@ import {
|
|||||||
parseVersionedFrame,
|
parseVersionedFrame,
|
||||||
} from "../utils/wsFrames";
|
} from "../utils/wsFrames";
|
||||||
import { parseChatErrorPayload } from "../utils/chatErrors";
|
import { parseChatErrorPayload } from "../utils/chatErrors";
|
||||||
|
import { sortPrompts } from "../utils/promptOrder";
|
||||||
|
|
||||||
type MessageProviderProps ={
|
type MessageProviderProps ={
|
||||||
children? : ReactNode;
|
children? : ReactNode;
|
||||||
@@ -61,7 +62,7 @@ const MessageProvider = ( {children}: MessageProviderProps) => {
|
|||||||
const {selectedConversation, setSelectedConversation} = useContext(ConversationContext);
|
const {selectedConversation, setSelectedConversation} = useContext(ConversationContext);
|
||||||
|
|
||||||
const [stateMessage, setStateMessage] = useState<string>('')
|
const [stateMessage, setStateMessage] = useState<string>('')
|
||||||
const [conversationDetails, setConversationDetails] = useState<ConversationPrompt[]>([])
|
const [conversationDetails, setConversationDetailsState] = useState<ConversationPrompt[]>([])
|
||||||
const [isGeneratingMessage, setIsGeneratingMessage] = useState<boolean>(false)
|
const [isGeneratingMessage, setIsGeneratingMessage] = useState<boolean>(false)
|
||||||
const [streamInterrupted, setStreamInterrupted] = useState<boolean>(false)
|
const [streamInterrupted, setStreamInterrupted] = useState<boolean>(false)
|
||||||
const [activityStage, setActivityStage] = useState<string | null>(null)
|
const [activityStage, setActivityStage] = useState<string | null>(null)
|
||||||
@@ -72,6 +73,11 @@ const MessageProvider = ( {children}: MessageProviderProps) => {
|
|||||||
const messageRef = useRef('')
|
const messageRef = useRef('')
|
||||||
const messageResponsePart = useRef(0);
|
const messageResponsePart = useRef(0);
|
||||||
const conversationRef = useRef(conversationDetails)
|
const conversationRef = useRef(conversationDetails)
|
||||||
|
const setConversationDetails = useCallback((next: ConversationPrompt[]) => {
|
||||||
|
const ordered = sortPrompts(next)
|
||||||
|
conversationRef.current = ordered
|
||||||
|
setConversationDetailsState(ordered)
|
||||||
|
}, [])
|
||||||
const selectedConversationRef = useRef<undefined | number>(undefined)
|
const selectedConversationRef = useRef<undefined | number>(undefined)
|
||||||
const isGeneratingRef = useRef(false)
|
const isGeneratingRef = useRef(false)
|
||||||
const prevReconnectGenerationRef = useRef(0)
|
const prevReconnectGenerationRef = useRef(0)
|
||||||
@@ -124,7 +130,6 @@ const MessageProvider = ( {children}: MessageProviderProps) => {
|
|||||||
new ConversationPrompt({ message: '', user_created: false }),
|
new ConversationPrompt({ message: '', user_created: false }),
|
||||||
)
|
)
|
||||||
}
|
}
|
||||||
conversationRef.current = tempConversations
|
|
||||||
setConversationDetails(tempConversations)
|
setConversationDetails(tempConversations)
|
||||||
} catch (err) {
|
} catch (err) {
|
||||||
console.log('Failed to refetch conversation after reconnect', err)
|
console.log('Failed to refetch conversation after reconnect', err)
|
||||||
@@ -261,15 +266,14 @@ const MessageProvider = ( {children}: MessageProviderProps) => {
|
|||||||
|
|
||||||
if (message === 'END_OF_THE_STREAM_ENDER_GAME_42'){
|
if (message === 'END_OF_THE_STREAM_ENDER_GAME_42'){
|
||||||
messageResponsePart.current = 0
|
messageResponsePart.current = 0
|
||||||
|
|
||||||
conversationRef.current.pop()
|
const details = [...conversationRef.current]
|
||||||
|
details.pop()
|
||||||
const finalized = new ConversationPrompt({
|
const finalized = new ConversationPrompt({
|
||||||
message: `${messageRef.current}`,
|
message: `${messageRef.current}`,
|
||||||
user_created: false,
|
user_created: false,
|
||||||
})
|
})
|
||||||
conversationRef.current = [...conversationRef.current, finalized]
|
setConversationDetails([...details, finalized])
|
||||||
setConversationDetails([...conversationRef.current])
|
|
||||||
messageRef.current = ''
|
messageRef.current = ''
|
||||||
setStateMessage('')
|
setStateMessage('')
|
||||||
isGeneratingRef.current = false
|
isGeneratingRef.current = false
|
||||||
@@ -279,7 +283,9 @@ const MessageProvider = ( {children}: MessageProviderProps) => {
|
|||||||
schedulePostStreamRefetch()
|
schedulePostStreamRefetch()
|
||||||
}
|
}
|
||||||
else if (message === 'START_OF_THE_STREAM_ENDER_GAME_42'){
|
else if (message === 'START_OF_THE_STREAM_ENDER_GAME_42'){
|
||||||
conversationRef.current = conversationDetails
|
// Do not copy conversationDetails from this closure. A START frame
|
||||||
|
// can arrive on the subscription from before the optimistic turn,
|
||||||
|
// and that copy wiped conversationRef (#136).
|
||||||
isGeneratingRef.current = true
|
isGeneratingRef.current = true
|
||||||
setIsGeneratingMessage(true)
|
setIsGeneratingMessage(true)
|
||||||
setStreamInterrupted(false)
|
setStreamInterrupted(false)
|
||||||
@@ -335,7 +341,7 @@ const MessageProvider = ( {children}: MessageProviderProps) => {
|
|||||||
/* unsubscribe from channel during cleanup */
|
/* unsubscribe from channel during cleanup */
|
||||||
unsubscribe(channelName)
|
unsubscribe(channelName)
|
||||||
}
|
}
|
||||||
}, [account, subscribe, unsubscribe, conversationDetails, selectedConversation, setSelectedConversation, schedulePostStreamRefetch])
|
}, [account, subscribe, unsubscribe, selectedConversation, setSelectedConversation, schedulePostStreamRefetch, setConversationDetails])
|
||||||
|
|
||||||
|
|
||||||
return(
|
return(
|
||||||
|
|||||||
@@ -29,6 +29,7 @@ describe('ConversationPrompt', () => {
|
|||||||
expect(prompt.message).toBe('hello');
|
expect(prompt.message).toBe('hello');
|
||||||
expect(prompt.user_created).toBe(true);
|
expect(prompt.user_created).toBe(true);
|
||||||
expect(prompt.created_timestamp).toEqual(created);
|
expect(prompt.created_timestamp).toEqual(created);
|
||||||
|
expect(prompt.localKey).toBe('id-7');
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
|||||||
@@ -22,8 +22,12 @@ export interface ConversationPromptType {
|
|||||||
image?: string | null,
|
image?: string | null,
|
||||||
}
|
}
|
||||||
|
|
||||||
|
let localPromptKey = 0;
|
||||||
|
|
||||||
export class ConversationPrompt{
|
export class ConversationPrompt{
|
||||||
id: number | undefined;
|
id: number | undefined;
|
||||||
|
/** Stable React key. Optimistic turns have no server id yet (#136). */
|
||||||
|
localKey: string = '';
|
||||||
message: string = '';
|
message: string = '';
|
||||||
user_created: boolean = false;
|
user_created: boolean = false;
|
||||||
created_timestamp: Date = new Date();
|
created_timestamp: Date = new Date();
|
||||||
@@ -35,7 +39,11 @@ export class ConversationPrompt{
|
|||||||
image: string | null = null;
|
image: string | null = null;
|
||||||
|
|
||||||
constructor(initializer?: any){
|
constructor(initializer?: any){
|
||||||
if(!initializer) return;
|
if(!initializer) {
|
||||||
|
localPromptKey += 1;
|
||||||
|
this.localKey = `local-${localPromptKey}`;
|
||||||
|
return;
|
||||||
|
}
|
||||||
if (initializer.id) this.id = initializer.id;
|
if (initializer.id) this.id = initializer.id;
|
||||||
if (initializer.message) this.message = initializer.message;
|
if (initializer.message) this.message = initializer.message;
|
||||||
if (initializer.user_created) this.user_created = initializer.user_created;
|
if (initializer.user_created) this.user_created = initializer.user_created;
|
||||||
@@ -47,6 +55,12 @@ export class ConversationPrompt{
|
|||||||
if (initializer.feedback !== undefined) this.feedback = initializer.feedback;
|
if (initializer.feedback !== undefined) this.feedback = initializer.feedback;
|
||||||
if (initializer.file_type) this.file_type = initializer.file_type;
|
if (initializer.file_type) this.file_type = initializer.file_type;
|
||||||
if (initializer.image) this.image = initializer.image;
|
if (initializer.image) this.image = initializer.image;
|
||||||
|
if (initializer.localKey) this.localKey = String(initializer.localKey);
|
||||||
|
else if (initializer.id) this.localKey = `id-${initializer.id}`;
|
||||||
|
else {
|
||||||
|
localPromptKey += 1;
|
||||||
|
this.localKey = `local-${localPromptKey}`;
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -622,7 +622,7 @@ const AsyncDashboardInner = (): JSX.Element => {
|
|||||||
: convo_detail.message;
|
: convo_detail.message;
|
||||||
return (
|
return (
|
||||||
<ConversationDetailCard
|
<ConversationDetailCard
|
||||||
key={convo_detail.id || index}
|
key={convo_detail.localKey}
|
||||||
message={displayMessage}
|
message={displayMessage}
|
||||||
user_created={convo_detail.user_created}
|
user_created={convo_detail.user_created}
|
||||||
promptId={convo_detail.id}
|
promptId={convo_detail.id}
|
||||||
|
|||||||
@@ -0,0 +1,61 @@
|
|||||||
|
import { ConversationPrompt } from '../data';
|
||||||
|
import { sortPrompts } from './promptOrder';
|
||||||
|
|
||||||
|
describe('sortPrompts', () => {
|
||||||
|
it('orders by created time, not the array the server happened to return', () => {
|
||||||
|
const prompts = [
|
||||||
|
new ConversationPrompt({
|
||||||
|
id: 4,
|
||||||
|
message: 'I dont know',
|
||||||
|
user_created: true,
|
||||||
|
created_timestamp: new Date('2026-10-25T12:03:00Z'),
|
||||||
|
}),
|
||||||
|
new ConversationPrompt({
|
||||||
|
id: 2,
|
||||||
|
message: 'Tell me a joke',
|
||||||
|
user_created: true,
|
||||||
|
created_timestamp: new Date('2026-10-25T12:01:00Z'),
|
||||||
|
}),
|
||||||
|
new ConversationPrompt({
|
||||||
|
id: 5,
|
||||||
|
message: 'form errors',
|
||||||
|
user_created: false,
|
||||||
|
created_timestamp: new Date('2026-10-25T12:04:00Z'),
|
||||||
|
}),
|
||||||
|
new ConversationPrompt({
|
||||||
|
id: 3,
|
||||||
|
message: 'meditation ladder',
|
||||||
|
user_created: false,
|
||||||
|
created_timestamp: new Date('2026-10-25T12:02:00Z'),
|
||||||
|
}),
|
||||||
|
];
|
||||||
|
|
||||||
|
expect(sortPrompts(prompts).map((prompt) => prompt.message)).toEqual([
|
||||||
|
'Tell me a joke',
|
||||||
|
'meditation ladder',
|
||||||
|
'I dont know',
|
||||||
|
'form errors',
|
||||||
|
]);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('keeps a same-millisecond user turn ahead of the empty assistant placeholder', () => {
|
||||||
|
const stamp = new Date('2026-10-25T12:05:00Z');
|
||||||
|
const prompts = [
|
||||||
|
new ConversationPrompt({
|
||||||
|
message: '',
|
||||||
|
user_created: false,
|
||||||
|
created_timestamp: stamp,
|
||||||
|
}),
|
||||||
|
new ConversationPrompt({
|
||||||
|
message: 'I dont know',
|
||||||
|
user_created: true,
|
||||||
|
created_timestamp: stamp,
|
||||||
|
}),
|
||||||
|
];
|
||||||
|
|
||||||
|
expect(sortPrompts(prompts).map((prompt) => prompt.user_created)).toEqual([
|
||||||
|
true,
|
||||||
|
false,
|
||||||
|
]);
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -0,0 +1,22 @@
|
|||||||
|
import { ConversationPrompt } from '../data';
|
||||||
|
|
||||||
|
function timeMs(prompt: ConversationPrompt): number {
|
||||||
|
const stamp = prompt.created_timestamp;
|
||||||
|
const ms = stamp instanceof Date ? stamp.getTime() : NaN;
|
||||||
|
return Number.isNaN(ms) ? 0 : ms;
|
||||||
|
}
|
||||||
|
|
||||||
|
/** Oldest first. Same instant: lower id, then the user turn before the assistant placeholder. */
|
||||||
|
export function comparePrompts(a: ConversationPrompt, b: ConversationPrompt): number {
|
||||||
|
const dt = timeMs(a) - timeMs(b);
|
||||||
|
if (dt !== 0) return dt;
|
||||||
|
const ia = a.id ?? Number.MAX_SAFE_INTEGER;
|
||||||
|
const ib = b.id ?? Number.MAX_SAFE_INTEGER;
|
||||||
|
if (ia !== ib) return ia - ib;
|
||||||
|
if (a.user_created !== b.user_created) return a.user_created ? -1 : 1;
|
||||||
|
return 0;
|
||||||
|
}
|
||||||
|
|
||||||
|
export function sortPrompts(prompts: ConversationPrompt[]): ConversationPrompt[] {
|
||||||
|
return [...prompts].sort(comparePrompts);
|
||||||
|
}
|
||||||
Reference in new issue
Block a user