diff --git a/package-lock.json b/package-lock.json index 49a3ce1c..e9453db6 100644 --- a/package-lock.json +++ b/package-lock.json @@ -40,7 +40,7 @@ "clsx": "2.1.1", "convex": "1.35.1", "convex-helpers": "0.1.114", - "convex-test": "0.0.44", + "convex-test": "0.0.51", "dayjs": "1.11.20", "dotenv": "16.6.1", "eslint": "9.39.4", @@ -4733,9 +4733,9 @@ } }, "node_modules/convex-test": { - "version": "0.0.44", - "resolved": "https://registry.npmjs.org/convex-test/-/convex-test-0.0.44.tgz", - "integrity": "sha512-2tv5XRx1n9z4kKwIiUU1UZgC0UDgUGpqAL2D2Hq45cHJ6HBTemfP2wKPMOcfag6ATFfmyqK3Xau0fKiJCtrdxQ==", + "version": "0.0.51", + "resolved": "https://registry.npmjs.org/convex-test/-/convex-test-0.0.51.tgz", + "integrity": "sha512-J+4YRpKGXJDfnQqiWUUT+ylNmNO36MpkuwqG3JG4ld+7QtroZGF8HqO4qzMmfv5ltm71rPbkBvi//MoMHjnVvQ==", "dev": true, "license": "Apache-2.0", "peerDependencies": { diff --git a/package.json b/package.json index a40417de..79586704 100644 --- a/package.json +++ b/package.json @@ -112,7 +112,7 @@ "clsx": "2.1.1", "convex": "1.35.1", "convex-helpers": "0.1.114", - "convex-test": "0.0.44", + "convex-test": "0.0.51", "dayjs": "1.11.20", "dotenv": "16.6.1", "eslint": "9.39.4", diff --git a/src/component/messages.test.ts b/src/component/messages.test.ts index e557f4f7..1f741197 100644 --- a/src/component/messages.test.ts +++ b/src/component/messages.test.ts @@ -652,4 +652,124 @@ describe("agent", () => { ); expect(remainingMessages.page).toHaveLength(1); }); + + // Regression test for #256: when searching across threads with + // searchAllMessagesForUserId, the targetMessage's order should not filter + // out messages from other threads (their order sequences are independent). + test("textSearch returns cross-thread matches even when target order is lower", async () => { + const t = convexTest(schema, modules); + const userId = "user-256"; + + // Old thread: build up several messages so the matching one has a high order. + const oldThread = await t.mutation(api.threads.createThread, { userId }); + for (let i = 0; i < 5; i++) { + await t.mutation(api.messages.addMessages, { + threadId: oldThread._id as Id<"threads">, + userId, + messages: [ + { message: { role: "user", content: `filler message ${i}` } }, + ], + }); + } + await t.mutation(api.messages.addMessages, { + threadId: oldThread._id as Id<"threads">, + userId, + messages: [ + { + message: { + role: "user", + content: + "tom and jerry are both amazing high-ticket coaches and educators", + }, + }, + ], + }); + + // New thread: only one message — its order will be 0, lower than the + // matching message in the old thread. + const newThread = await t.mutation(api.threads.createThread, { userId }); + const { messages: newMessages } = await t.mutation( + api.messages.addMessages, + { + threadId: newThread._id as Id<"threads">, + userId, + messages: [ + { + message: { + role: "user", + content: "what do you remember about high-ticket coaches", + }, + }, + ], + }, + ); + const targetMessageId = newMessages[0]._id as Id<"messages">; + + const results = await t.query(api.messages.textSearch, { + searchAllMessagesForUserId: userId, + targetMessageId, + text: "high-ticket coaches", + limit: 10, + }); + + // The cross-thread match should NOT be filtered out by the target's order. + expect( + results.some((m) => + m.text?.includes("tom and jerry are both amazing high-ticket coaches"), + ), + ).toBe(true); + // The target message itself must still be excluded. + expect(results.some((m) => m._id === targetMessageId)).toBe(false); + }); + + // Regression test for #256: same-thread order filter must still work even + // when searching across threads. + test("textSearch still filters same-thread results past the target order", async () => { + const t = convexTest(schema, modules); + const userId = "user-256-same"; + + const thread = await t.mutation(api.threads.createThread, { userId }); + // earlier match + await t.mutation(api.messages.addMessages, { + threadId: thread._id as Id<"threads">, + userId, + messages: [ + { message: { role: "user", content: "earlier high-ticket match" } }, + ], + }); + // target + const { messages: targetMessages } = await t.mutation( + api.messages.addMessages, + { + threadId: thread._id as Id<"threads">, + userId, + messages: [ + { message: { role: "user", content: "target high-ticket message" } }, + ], + }, + ); + // later match in same thread — should be filtered out + await t.mutation(api.messages.addMessages, { + threadId: thread._id as Id<"threads">, + userId, + messages: [ + { message: { role: "user", content: "later high-ticket match" } }, + ], + }); + + const results = await t.query(api.messages.textSearch, { + searchAllMessagesForUserId: userId, + targetMessageId: targetMessages[0]._id as Id<"messages">, + text: "high-ticket", + limit: 10, + }); + + expect(results.some((m) => m.text === "earlier high-ticket match")).toBe( + true, + ); + expect(results.some((m) => m.text === "later high-ticket match")).toBe( + false, + ); + expect(results.some((m) => m._id === targetMessages[0]._id)).toBe(false); + }); }); diff --git a/src/component/messages.ts b/src/component/messages.ts index 1f38ec87..6593f791 100644 --- a/src/component/messages.ts +++ b/src/component/messages.ts @@ -821,16 +821,18 @@ export const _fetchSearchMessages = internalQuery({ ), ) ) - .filter( - (m): m is Doc<"messages"> => - m !== undefined && - m !== null && - !m.tool && - (!beforeMessage || - m.order < beforeMessage.order || - (m.order === beforeMessage.order && - m.stepOrder < beforeMessage.stepOrder)), - ) + .filter((m): m is Doc<"messages"> => { + if (m === undefined || m === null || m.tool) return false; + if (!beforeMessage) return true; + // The order filter is only meaningful within the same thread. + // Messages from other threads have independent order sequences. + if (m.threadId !== beforeMessage.threadId) return true; + return ( + m.order < beforeMessage.order || + (m.order === beforeMessage.order && + m.stepOrder < beforeMessage.stepOrder) + ); + }) .map(publicMessage); messages.push(...(args.textSearchMessages ?? [])); // TODO: prioritize more recent messages @@ -912,12 +914,17 @@ export const textSearch = query({ ); const targetMessage = args.targetMessageId && (await ctx.db.get(args.targetMessageId)); - const order = targetMessage?.order; const text = args.text || targetMessage?.text; if (!text) { console.warn("No text to search", targetMessage, args.text); return []; } + // When searching across threads (searchAllMessagesForUserId), the + // targetMessage's order is only meaningful within its own thread, so we + // can't apply it as a database-level filter. We still apply it post-fetch + // for same-thread results below. + const restrictOrderInDb = + !args.searchAllMessagesForUserId && targetMessage; const messages = await ctx.db .query("messages") .withSearchIndex("text_search", (q) => @@ -928,20 +935,24 @@ export const textSearch = query({ // Just in case tool messages slip through .filter((q) => { const qq = q.eq(q.field("tool"), false); - if (order) { - return q.and(qq, q.lte(q.field("order"), order)); + if (restrictOrderInDb) { + return q.and(qq, q.lte(q.field("order"), targetMessage.order)); } return qq; }) .take(args.limit); return messages - .filter( - (m) => - !targetMessage || + .filter((m) => { + if (!targetMessage) return true; + // Order is only meaningful within the same thread; cross-thread + // results have independent order sequences and should pass through. + if (m.threadId !== targetMessage.threadId) return true; + return ( m.order < targetMessage.order || (m.order === targetMessage.order && - m.stepOrder < targetMessage.stepOrder), - ) + m.stepOrder < targetMessage.stepOrder) + ); + }) .map(publicMessage); }, returns: v.array(vMessageDoc),