Repository navigation
📨 fix: Resolve Stored Responses by Their IDs #16456
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.
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
base: canary
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
Large diffs are not rendered by default.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,74 @@ | ||
| import mongoose from 'mongoose'; | ||
| import { EModelEndpoint } from 'librechat-data-provider'; | ||
| import { MongoMemoryServer } from 'mongodb-memory-server'; | ||
| import { createMethods, createModels } from '@librechat/data-schemas'; | ||
| import { resolveStoredResponse } from './storage'; | ||
|
|
||
| const conversationId = '10000000-0000-4000-8000-000000000001'; | ||
| const owner = 'owner-user'; | ||
|
|
||
| let mongod: MongoMemoryServer; | ||
| let db: ReturnType<typeof createMethods>; | ||
|
|
||
| beforeAll(async () => { | ||
| mongod = await MongoMemoryServer.create({ instance: { args: ['--nounixsocket'] } }); | ||
| await mongoose.connect(mongod.getUri()); | ||
| createModels(mongoose); | ||
| db = createMethods(mongoose); | ||
| }); | ||
|
|
||
| afterAll(async () => { | ||
| await mongoose.disconnect(); | ||
| if (mongod) await mongod.stop(); | ||
| }); | ||
|
|
||
| it('stores a response under an independent message ID and admits it only after the conversation commit', async () => { | ||
| const context = { userId: owner }; | ||
| const input = await db.saveMessage(context, { | ||
| messageId: 'input-one', | ||
| conversationId, | ||
| isCreatedByUser: true, | ||
| text: 'first', | ||
| sender: 'User', | ||
| endpoint: EModelEndpoint.agents, | ||
| }); | ||
| expect(input).toBeTruthy(); | ||
| const pending = await db.saveMessage(context, { | ||
| messageId: 'resp_one', | ||
| conversationId, | ||
| isCreatedByUser: false, | ||
| text: 'first answer', | ||
| sender: 'Agent', | ||
| endpoint: EModelEndpoint.agents, | ||
| finish_reason: 'pending_storage', | ||
| }); | ||
| expect(pending).toBeTruthy(); | ||
| expect(await resolveStoredResponse(owner, 'resp_one', db)).toBeNull(); | ||
|
|
||
| const conversation = await db.saveConvo(context, { | ||
| conversationId, | ||
| agent_id: 'agent-one', | ||
| endpoint: EModelEndpoint.agents, | ||
| }); | ||
| expect(conversation).toBeTruthy(); | ||
| expect((await db.getConvo(owner, conversationId))?.messages).toHaveLength(2); | ||
| expect(await resolveStoredResponse(owner, 'resp_one', db)).toBeNull(); | ||
|
|
||
| await db.updateMessage(owner, { messageId: 'resp_one', finish_reason: 'stop' }); | ||
| const stored = await resolveStoredResponse(owner, 'resp_one', db); | ||
| expect(stored?.conversation.conversationId).toBe(conversationId); | ||
| expect(stored?.message?.text).toBe('first answer'); | ||
| expect(await resolveStoredResponse('another-user', 'resp_one', db)).toBeNull(); | ||
| expect(await resolveStoredResponse(owner, 'resp_missing', db)).toBeNull(); | ||
|
|
||
| await db.saveMessage(context, { | ||
| messageId: 'resp_failed', | ||
| conversationId, | ||
| isCreatedByUser: false, | ||
| text: 'uncommitted', | ||
| sender: 'Agent', | ||
| endpoint: EModelEndpoint.agents, | ||
| finish_reason: 'pending_storage', | ||
| }); | ||
| expect(await resolveStoredResponse(owner, 'resp_failed', db)).toBeNull(); | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,80 @@ | ||
| import { EModelEndpoint } from 'librechat-data-provider'; | ||
| import { resolveStoredResponse, selectStoredResponseHistory } from './storage'; | ||
|
|
||
| const responseMessage = { | ||
| messageId: 'resp_one', | ||
| conversationId: 'conversation-one', | ||
| endpoint: EModelEndpoint.agents, | ||
| sender: 'Agent', | ||
| isCreatedByUser: false, | ||
| finish_reason: 'stop', | ||
| }; | ||
| const conversation = { conversationId: 'conversation-one', user: 'owner' }; | ||
|
|
||
| function reads() { | ||
| return { | ||
| getMessage: jest.fn().mockResolvedValue(responseMessage), | ||
| getConvo: jest.fn().mockResolvedValue(conversation), | ||
| }; | ||
| } | ||
|
|
||
| describe('stored Responses ID resolution', () => { | ||
| it('looks up a response by its owner-scoped indexed message, then verifies its conversation', async () => { | ||
| const db = reads(); | ||
| expect(await resolveStoredResponse('owner', 'resp_one', db)).toEqual({ | ||
| message: responseMessage, | ||
| conversation, | ||
| }); | ||
| expect(db.getMessage).toHaveBeenCalledWith({ user: 'owner', messageId: 'resp_one' }); | ||
| expect(db.getConvo).toHaveBeenCalledWith('owner', 'conversation-one'); | ||
| }); | ||
|
|
||
| it('does not accept unstored, foreign, non-assistant, or deleted-conversation response IDs', async () => { | ||
| const db = reads(); | ||
| for (const message of [ | ||
| null, | ||
| { ...responseMessage, isCreatedByUser: true }, | ||
| { ...responseMessage, endpoint: 'openAI' }, | ||
| { ...responseMessage, sender: 'User' }, | ||
| { ...responseMessage, finish_reason: 'pending_storage' }, | ||
| { ...responseMessage, conversationId: '' }, | ||
| ]) { | ||
| db.getMessage.mockResolvedValueOnce(message); | ||
| expect(await resolveStoredResponse('owner', 'resp_one', db)).toBeNull(); | ||
| } | ||
| db.getMessage.mockResolvedValueOnce(responseMessage); | ||
| db.getConvo.mockResolvedValueOnce(null); | ||
| expect(await resolveStoredResponse('owner', 'resp_one', db)).toBeNull(); | ||
| db.getMessage.mockResolvedValueOnce(responseMessage); | ||
| db.getConvo.mockResolvedValueOnce({ conversationId: 'different' }); | ||
| expect(await resolveStoredResponse('owner', 'resp_one', db)).toBeNull(); | ||
| }); | ||
|
|
||
| it('replays only through the referenced response and excludes incomplete writes', () => { | ||
| const messages = [ | ||
| { messageId: 'user-one' }, | ||
| { messageId: 'resp_one', finish_reason: 'stop' }, | ||
| { messageId: 'resp_failed', finish_reason: 'pending_storage' }, | ||
| { messageId: 'user-two' }, | ||
| { messageId: 'resp_two', finish_reason: 'stop' }, | ||
| ]; | ||
| expect(selectStoredResponseHistory(messages, 'resp_one')).toEqual(messages.slice(0, 2)); | ||
| expect(selectStoredResponseHistory(messages, 'resp_two')).toEqual([ | ||
| messages[0], | ||
| messages[1], | ||
| messages[3], | ||
| messages[4], | ||
| ]); | ||
| expect(selectStoredResponseHistory(messages, 'resp_missing')).toEqual([]); | ||
| }); | ||
|
|
||
| it('keeps the existing authorized conversation-ID form without a guaranteed-miss message read', async () => { | ||
| const db = reads(); | ||
| expect(await resolveStoredResponse('owner', 'conversation-one', db)).toEqual({ | ||
| message: null, | ||
| conversation, | ||
| }); | ||
| expect(db.getMessage).not.toHaveBeenCalled(); | ||
| expect(db.getConvo).toHaveBeenCalledWith('owner', 'conversation-one'); | ||
| }); | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,71 @@ | ||
| import { EModelEndpoint } from 'librechat-data-provider'; | ||
| import { RESPONSE_ID_PREFIX } from './service'; | ||
|
|
||
| /** Only persisted Responses assistant messages can name a continued run. */ | ||
| export interface StoredResponseMessage { | ||
| messageId: string; | ||
| conversationId: string; | ||
| endpoint?: string; | ||
| isCreatedByUser?: boolean; | ||
| sender?: string; | ||
| finish_reason?: string; | ||
| text?: string; | ||
| tokenCount?: number; | ||
| createdAt?: Date; | ||
| updatedAt?: Date; | ||
| } | ||
|
|
||
| export interface StoredResponseConversation { | ||
| conversationId: string; | ||
| } | ||
|
|
||
| /** The referenced response bounds history; an incomplete store is never model input. */ | ||
| export function selectStoredResponseHistory< | ||
| Message extends { messageId?: string; finish_reason?: string }, | ||
| >(messages: Message[], responseId?: string): Message[] { | ||
| const lastIndex = responseId | ||
| ? messages.findIndex((message) => message.messageId === responseId) | ||
| : -1; | ||
| if (responseId && lastIndex < 0) return []; | ||
| return (responseId ? messages.slice(0, lastIndex + 1) : messages).filter( | ||
| (message) => | ||
| !message.messageId?.startsWith(RESPONSE_ID_PREFIX) || message.finish_reason === 'stop', | ||
|
Comment on lines
+30
to
+32
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
If Useful? React with 👍 / 👎. |
||
| ); | ||
| } | ||
|
|
||
| export interface StoredResponseReads< | ||
| Message extends StoredResponseMessage, | ||
| Conversation extends StoredResponseConversation, | ||
| > { | ||
| getMessage: (params: { user: string; messageId: string }) => Promise<Message | null>; | ||
| getConvo: (user: string, conversationId: string) => Promise<Conversation | null>; | ||
| } | ||
|
|
||
| /** A response ID is the assistant message ID; legacy conversation IDs remain supported. */ | ||
| export async function resolveStoredResponse< | ||
| Message extends StoredResponseMessage, | ||
| Conversation extends StoredResponseConversation, | ||
| >( | ||
| user: string, | ||
| id: string, | ||
| reads: StoredResponseReads<Message, Conversation>, | ||
| ): Promise<{ conversation: Conversation; message: Message | null } | null> { | ||
| if (!id.startsWith(RESPONSE_ID_PREFIX)) { | ||
| const conversation = await reads.getConvo(user, id); | ||
| return conversation ? { conversation, message: null } : null; | ||
| } | ||
|
|
||
| const message = await reads.getMessage({ user, messageId: id }); | ||
| if ( | ||
| message?.messageId !== id || | ||
| message.isCreatedByUser !== false || | ||
| message.endpoint !== EModelEndpoint.agents || | ||
| message.sender !== 'Agent' || | ||
| message.finish_reason !== 'stop' || | ||
| !message.conversationId | ||
| ) { | ||
| return null; | ||
| } | ||
| const conversation = await reads.getConvo(user, message.conversationId); | ||
| return conversation?.conversationId === message.conversationId ? { conversation, message } : null; | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When a client branches from an older response, this chronological slice includes unrelated sibling turns. For example, after
resp_1 → resp_2, creatingresp_3fromresp_1stores all three in one conversation; subsequently continuingresp_3slices throughresp_2, so the model receives the sibling user prompt and answer even though they are not ancestors ofresp_3. Persist and traverse theprevious_response_idrelationship, or otherwise isolate branches, rather than treating every earlier conversation message as history.Useful? React with 👍 / 👎.