mirror of
https://github.com/danny-avila/LibreChat.git
synced 2026-10-21 01:22:54 +00:00
🔍 fix: Sync Conversation Title Updates to MeiliSearch (#14223)
Some checks are pending
Docker Dev Branch Images Build / build (Dockerfile, lc-dev, node) (push) Waiting to run
Docker Dev Branch Images Build / build (Dockerfile.multi, lc-dev-api, api-build) (push) Waiting to run
GitNexus Index / index (push) Waiting to run
GitNexus Index / post-index (push) Blocked by required conditions
Some checks are pending
Docker Dev Branch Images Build / build (Dockerfile, lc-dev, node) (push) Waiting to run
Docker Dev Branch Images Build / build (Dockerfile.multi, lc-dev-api, api-build) (push) Waiting to run
GitNexus Index / index (push) Waiting to run
GitNexus Index / post-index (push) Blocked by required conditions
* 🔍 fix: Sync Conversation Title Updates to MeiliSearch `saveConvo` calls `findOneAndUpdate` with `includeResultMetadata: true`, so Mongoose passes the raw `{ value, ok, lastErrorObject }` result to the `post('findOneAndUpdate')` hook rather than the document. The hook treated the wrapper as the document, so `postSaveHook` was `undefined` and indexing silently no-op'd, leaving conversation titles stale in search (messages were unaffected because `saveMessage` uses plain `{ upsert, new }`). Unwrap `res.value` (distinguishing the hydrated document via `instanceof mongoose.Document`) and always call `next()`. Add a real mongodb-memory-server regression spec that exercises the hook with `meiliEnabled` active, which the existing suite never covered. * 🧪 test: Restore MeiliSearch env after findOneAndUpdate spec Snapshot and restore `SEARCH`/`MEILI_HOST`/`MEILI_MASTER_KEY` around the suite instead of mutating them at module top level. Jest reuses a worker process across test files with a shared `process.env`, so the unrestored writes could leak `meiliEnabled: true` into a later file that re-imports `mongoMeili`, producing order-dependent indexing side effects. Env is now set at the start of `beforeAll` (before the lazy plugin import) and reverted in `afterAll`.
This commit is contained in:
parent
53e369fba8
commit
654c7a78b1
2 changed files with 236 additions and 27 deletions
|
|
@ -0,0 +1,194 @@
|
|||
/**
|
||||
* `meiliEnabled` is a module-level constant evaluated when `mongoMeili` is first
|
||||
* imported, so the MeiliSearch env vars must be set before the plugin module is
|
||||
* loaded. The plugin is therefore imported lazily inside `beforeAll`, after the
|
||||
* env is set, and the prior values are restored in `afterAll` so this suite does
|
||||
* not leak `meiliEnabled: true` into other test files that share the Jest
|
||||
* worker's `process.env`.
|
||||
*
|
||||
* This suite covers the `saveConvo` code path, which calls `findOneAndUpdate`
|
||||
* with `includeResultMetadata: true`. Mongoose then resolves the query to the raw
|
||||
* `{ value, ok, lastErrorObject }` wrapper, and the post hook must unwrap `value`
|
||||
* before invoking the indexing hook (regression test for conversation titles not
|
||||
* syncing to MeiliSearch).
|
||||
*/
|
||||
import mongoose from 'mongoose';
|
||||
import { EModelEndpoint } from 'librechat-data-provider';
|
||||
import { MongoMemoryServer } from 'mongodb-memory-server';
|
||||
import type { SchemaWithMeiliMethods } from '~/models/plugins/mongoMeili';
|
||||
|
||||
const MEILI_ENV_KEYS = ['SEARCH', 'MEILI_HOST', 'MEILI_MASTER_KEY'] as const;
|
||||
const savedMeiliEnv: Partial<Record<(typeof MEILI_ENV_KEYS)[number], string | undefined>> = {};
|
||||
|
||||
const wait = (ms: number) => new Promise<void>((resolve) => setTimeout(resolve, ms));
|
||||
|
||||
const waitForMock = async (mock: jest.Mock, timeoutMs = 2000): Promise<void> => {
|
||||
const start = Date.now();
|
||||
while (mock.mock.calls.length === 0) {
|
||||
if (Date.now() - start > timeoutMs) {
|
||||
return;
|
||||
}
|
||||
await wait(10);
|
||||
}
|
||||
};
|
||||
|
||||
const mockAddDocuments = jest.fn();
|
||||
const mockUpdateDocuments = jest.fn();
|
||||
const mockDeleteDocument = jest.fn();
|
||||
const mockGetDocument = jest.fn();
|
||||
const mockIndex = jest.fn().mockReturnValue({
|
||||
getRawInfo: jest.fn(),
|
||||
updateSettings: jest.fn(),
|
||||
addDocuments: mockAddDocuments,
|
||||
addDocumentsInBatches: jest.fn(),
|
||||
updateDocuments: mockUpdateDocuments,
|
||||
deleteDocument: mockDeleteDocument,
|
||||
deleteDocuments: jest.fn(),
|
||||
getDocument: mockGetDocument,
|
||||
getDocuments: jest.fn().mockReturnValue({ results: [] }),
|
||||
});
|
||||
jest.mock('meilisearch', () => ({
|
||||
MeiliSearch: jest.fn().mockImplementation(() => ({ index: mockIndex })),
|
||||
}));
|
||||
|
||||
describe('mongoMeili findOneAndUpdate with includeResultMetadata (saveConvo path)', () => {
|
||||
let mongoServer: MongoMemoryServer;
|
||||
let createConversationModel: (typeof import('~/models/convo'))['createConversationModel'];
|
||||
let conversationModel: SchemaWithMeiliMethods;
|
||||
|
||||
beforeAll(async () => {
|
||||
for (const key of MEILI_ENV_KEYS) {
|
||||
savedMeiliEnv[key] = process.env[key];
|
||||
}
|
||||
process.env.SEARCH = 'true';
|
||||
process.env.MEILI_HOST = 'foo';
|
||||
process.env.MEILI_MASTER_KEY = 'bar';
|
||||
|
||||
({ createConversationModel } = await import('~/models/convo'));
|
||||
mongoServer = await MongoMemoryServer.create();
|
||||
await mongoose.connect(mongoServer.getUri());
|
||||
conversationModel = createConversationModel(mongoose) as unknown as SchemaWithMeiliMethods;
|
||||
});
|
||||
|
||||
afterAll(async () => {
|
||||
await mongoose.disconnect();
|
||||
await mongoServer.stop();
|
||||
for (const key of MEILI_ENV_KEYS) {
|
||||
if (savedMeiliEnv[key] === undefined) {
|
||||
delete process.env[key];
|
||||
} else {
|
||||
process.env[key] = savedMeiliEnv[key];
|
||||
}
|
||||
}
|
||||
});
|
||||
|
||||
beforeEach(async () => {
|
||||
await conversationModel.deleteMany({});
|
||||
mockAddDocuments.mockClear();
|
||||
mockUpdateDocuments.mockClear();
|
||||
mockDeleteDocument.mockClear();
|
||||
mockGetDocument.mockClear();
|
||||
mockGetDocument.mockReset();
|
||||
});
|
||||
|
||||
const updateTitle = (conversationId: string, user: string, title: string) =>
|
||||
conversationModel.findOneAndUpdate(
|
||||
{ conversationId, user },
|
||||
{ $set: { title } },
|
||||
{ new: true, upsert: true, includeResultMetadata: true },
|
||||
);
|
||||
|
||||
test('re-indexes the updated title when the raw result wrapper is returned', async () => {
|
||||
const conversationId = new mongoose.Types.ObjectId().toString();
|
||||
const user = new mongoose.Types.ObjectId().toString();
|
||||
await conversationModel.create({
|
||||
conversationId,
|
||||
user,
|
||||
title: 'Original Title',
|
||||
endpoint: EModelEndpoint.openAI,
|
||||
});
|
||||
mockAddDocuments.mockClear();
|
||||
|
||||
const result = await updateTitle(conversationId, user, 'Renamed Title');
|
||||
|
||||
// `includeResultMetadata: true` resolves to the raw wrapper, not the document.
|
||||
expect((result as unknown as { value: unknown }).value).toBeTruthy();
|
||||
|
||||
await waitForMock(mockAddDocuments);
|
||||
expect(mockAddDocuments).toHaveBeenCalledWith(
|
||||
[expect.objectContaining({ conversationId, title: 'Renamed Title' })],
|
||||
{ primaryKey: 'conversationId' },
|
||||
);
|
||||
});
|
||||
|
||||
test('upserting a new conversation via the raw result wrapper indexes it', async () => {
|
||||
const conversationId = new mongoose.Types.ObjectId().toString();
|
||||
const user = new mongoose.Types.ObjectId().toString();
|
||||
|
||||
await conversationModel.findOneAndUpdate(
|
||||
{ conversationId, user },
|
||||
{ $set: { title: 'Fresh Conversation', endpoint: EModelEndpoint.openAI } },
|
||||
{ new: true, upsert: true, includeResultMetadata: true },
|
||||
);
|
||||
|
||||
await waitForMock(mockAddDocuments);
|
||||
expect(mockAddDocuments).toHaveBeenCalledWith(
|
||||
[expect.objectContaining({ conversationId, title: 'Fresh Conversation' })],
|
||||
{ primaryKey: 'conversationId' },
|
||||
);
|
||||
});
|
||||
|
||||
test('persists the _meiliIndex flag after indexing via the wrapper', async () => {
|
||||
const conversationId = new mongoose.Types.ObjectId().toString();
|
||||
const user = new mongoose.Types.ObjectId().toString();
|
||||
|
||||
await updateTitle(conversationId, user, 'Indexed Conversation');
|
||||
await waitForMock(mockAddDocuments);
|
||||
await wait(50);
|
||||
|
||||
const storedDoc = await conversationModel.collection.findOne({ conversationId });
|
||||
expect(storedDoc?._meiliIndex).toBe(true);
|
||||
});
|
||||
|
||||
test('skips re-indexing when the title already matches the MeiliSearch document', async () => {
|
||||
const conversationId = new mongoose.Types.ObjectId().toString();
|
||||
const user = new mongoose.Types.ObjectId().toString();
|
||||
await conversationModel.create({
|
||||
conversationId,
|
||||
user,
|
||||
title: 'Same Title',
|
||||
endpoint: EModelEndpoint.openAI,
|
||||
});
|
||||
mockAddDocuments.mockClear();
|
||||
mockGetDocument.mockResolvedValueOnce({ conversationId, title: 'Same Title' });
|
||||
|
||||
await updateTitle(conversationId, user, 'Same Title');
|
||||
|
||||
await wait(50);
|
||||
expect(mockGetDocument).toHaveBeenCalledWith(conversationId);
|
||||
expect(mockAddDocuments).not.toHaveBeenCalled();
|
||||
expect(mockUpdateDocuments).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
test('does NOT index temporary conversations updated via the raw result wrapper', async () => {
|
||||
const conversationId = new mongoose.Types.ObjectId().toString();
|
||||
const user = new mongoose.Types.ObjectId().toString();
|
||||
await conversationModel.collection.insertOne({
|
||||
conversationId,
|
||||
user,
|
||||
title: 'Temporary Conversation',
|
||||
endpoint: EModelEndpoint.openAI,
|
||||
isTemporary: true,
|
||||
expiredAt: new Date(Date.now() + 60 * 60 * 1000),
|
||||
_meiliIndex: false,
|
||||
createdAt: new Date(),
|
||||
updatedAt: new Date(),
|
||||
});
|
||||
|
||||
await updateTitle(conversationId, user, 'Renamed Temporary Conversation');
|
||||
|
||||
await wait(50);
|
||||
expect(mockAddDocuments).not.toHaveBeenCalled();
|
||||
expect(mockUpdateDocuments).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
|
@ -1,7 +1,6 @@
|
|||
import _ from 'lodash';
|
||||
import { parseTextParts } from 'librechat-data-provider';
|
||||
import { MeiliSearch, MeiliSearchTimeOutError } from 'meilisearch';
|
||||
import type { SearchResponse, SearchParams, Index, MeiliSearchErrorInfo } from 'meilisearch';
|
||||
import type {
|
||||
CallbackWithoutResultAndOptionalError,
|
||||
FilterQuery,
|
||||
|
|
@ -11,9 +10,10 @@ import type {
|
|||
Types,
|
||||
Model,
|
||||
} from 'mongoose';
|
||||
import type { SearchResponse, SearchParams, Index, MeiliSearchErrorInfo } from 'meilisearch';
|
||||
import type { IConversation, IMessage } from '~/types';
|
||||
import logger from '~/config/meiliLogger';
|
||||
import { buildRetentionVisibilityFilter, legacyPermanentExpirationFilter } from '~/utils/retention';
|
||||
import logger from '~/config/meiliLogger';
|
||||
|
||||
interface MongoMeiliOptions {
|
||||
host: string;
|
||||
|
|
@ -792,32 +792,47 @@ export default function mongoMeili(schema: Schema, options: MongoMeiliOptions):
|
|||
});
|
||||
|
||||
// Post-findOneAndUpdate hook
|
||||
schema.post('findOneAndUpdate', async function (doc: DocumentWithMeiliIndex, next) {
|
||||
if (!meiliEnabled) {
|
||||
return next();
|
||||
}
|
||||
|
||||
if (doc.unfinished) {
|
||||
return next();
|
||||
}
|
||||
|
||||
let meiliDoc: Record<string, unknown> | undefined;
|
||||
if (doc.messages) {
|
||||
try {
|
||||
meiliDoc = await client.index('convos').getDocument(doc.conversationId as string);
|
||||
} catch (error: unknown) {
|
||||
logger.debug(
|
||||
'[MeiliMongooseModel.findOneAndUpdate] Convo not found in MeiliSearch and will index ' +
|
||||
doc.conversationId,
|
||||
error as Record<string, unknown>,
|
||||
);
|
||||
schema.post(
|
||||
'findOneAndUpdate',
|
||||
async function (
|
||||
res: DocumentWithMeiliIndex | { value: DocumentWithMeiliIndex | null } | null,
|
||||
next: CallbackWithoutResultAndOptionalError,
|
||||
) {
|
||||
if (!meiliEnabled) {
|
||||
return next();
|
||||
}
|
||||
|
||||
// `saveConvo` issues `findOneAndUpdate` with `includeResultMetadata: true`, so
|
||||
// the hook receives the raw `{ value, ok, lastErrorObject }` result instead of
|
||||
// the document. Unwrap `value` so indexing runs for that path too.
|
||||
const doc = res instanceof mongoose.Document ? res : (res?.value ?? null);
|
||||
|
||||
if (!doc || doc.unfinished) {
|
||||
return next();
|
||||
}
|
||||
|
||||
let meiliDoc: Record<string, unknown> | undefined;
|
||||
if (doc.messages) {
|
||||
try {
|
||||
meiliDoc = await client.index('convos').getDocument(doc.conversationId as string);
|
||||
} catch (error: unknown) {
|
||||
logger.debug(
|
||||
'[MeiliMongooseModel.findOneAndUpdate] Convo not found in MeiliSearch and will index ' +
|
||||
doc.conversationId,
|
||||
error as Record<string, unknown>,
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
if (meiliDoc && meiliDoc.title === doc.title) {
|
||||
return next();
|
||||
}
|
||||
|
||||
if (typeof doc.postSaveHook === 'function') {
|
||||
return doc.postSaveHook(next);
|
||||
}
|
||||
}
|
||||
|
||||
if (meiliDoc && meiliDoc.title === doc.title) {
|
||||
return next();
|
||||
}
|
||||
|
||||
doc.postSaveHook?.(next);
|
||||
});
|
||||
},
|
||||
);
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue