From 060927056b1d85e3cc32d3fc36acdeeb415026cb Mon Sep 17 00:00:00 2001 From: israel Date: Sat, 29 Aug 2026 17:18:00 +0100 Subject: [PATCH] fix(platform): stop a search returning one passage twice --- .../platform/convex/knowledge/search.test.ts | 112 ++++++++++++++++++ services/platform/convex/knowledge/search.ts | 39 +++++- 2 files changed, 149 insertions(+), 2 deletions(-) diff --git a/services/platform/convex/knowledge/search.test.ts b/services/platform/convex/knowledge/search.test.ts index 1db83c1ed7..6cf20bef69 100644 --- a/services/platform/convex/knowledge/search.test.ts +++ b/services/platform/convex/knowledge/search.test.ts @@ -97,3 +97,115 @@ describe('searchKnowledge live document validation', () => { }); }); }); + +describe('searchKnowledge — the same passage twice', () => { + beforeEach(() => { + retrieveMock.mockReset(); + }); + + /** Two refs holding identical text — the same file indexed twice. */ + function duplicatePair(text: string) { + return [ + { ...hit('documents', 'copy_a'), text, fusedScore: 0.9 }, + { ...hit('documents', 'copy_b'), text, fusedScore: 0.4 }, + ]; + } + + const ACCESS = { teamIds: [], projectIds: [], includeHub: true }; + + it('returns one copy, keeping the higher-scoring ref', async () => { + // A bounded result set spending two slots on one passage pushes a + // different answer off the end. + retrieveMock.mockResolvedValueOnce({ + hits: [...duplicatePair('Refunds within 30 days.')], + diagnostics: {}, + }); + const runQuery = vi.fn(async () => ['copy_a', 'copy_b']); + const result = await searchKnowledge({ runQuery } as never, { + organizationId: 'org_1', + orgSlug: 'acme', + query: 'refunds', + access: ACCESS, + }); + expect(result.hits.map((h) => h.source.ref)).toEqual(['copy_a']); + }); + + it('keeps a distinct passage from the same document', async () => { + // Deduping is per passage, not per document — a second chunk of the same + // file is a different answer. + retrieveMock.mockResolvedValueOnce({ + hits: [ + { ...hit('documents', 'doc'), text: 'First passage.' }, + { ...hit('documents', 'doc'), text: 'Second passage.' }, + ], + diagnostics: {}, + }); + const runQuery = vi.fn(async () => ['doc']); + const result = await searchKnowledge({ runQuery } as never, { + organizationId: 'org_1', + orgSlug: 'acme', + query: 'passages', + access: ACCESS, + }); + expect(result.hits).toHaveLength(2); + }); + + it('treats copies that differ only in whitespace as one', async () => { + retrieveMock.mockResolvedValueOnce({ + hits: [ + { ...hit('documents', 'copy_a'), text: 'Refunds within 30 days.' }, + { + ...hit('documents', 'copy_b'), + text: 'Refunds within\n30 days.', + }, + ], + diagnostics: {}, + }); + const runQuery = vi.fn(async () => ['copy_a', 'copy_b']); + const result = await searchKnowledge({ runQuery } as never, { + organizationId: 'org_1', + orgSlug: 'acme', + query: 'refunds', + access: ACCESS, + }); + expect(result.hits).toHaveLength(1); + }); + + it('keeps the readable copy when the better-scoring one is denied', async () => { + // THE ordering case. Deduping before the retrievability filter would keep + // `copy_a`, the gate would then drop it, and the passage would vanish + // entirely — worse than showing it twice. + retrieveMock.mockResolvedValueOnce({ + hits: [...duplicatePair('Refunds within 30 days.')], + diagnostics: {}, + }); + const runQuery = vi.fn(async () => ['copy_b']); + const result = await searchKnowledge({ runQuery } as never, { + organizationId: 'org_1', + orgSlug: 'acme', + query: 'refunds', + access: ACCESS, + }); + expect(result.hits.map((h) => h.source.ref)).toEqual(['copy_b']); + }); + + it('does not collapse identical text across different corpora', async () => { + // A web page and a document saying the same thing are two findings, and + // only one of them is citable by URL. + retrieveMock.mockResolvedValueOnce({ + hits: [ + { ...hit('documents', 'doc'), text: 'Same words.' }, + { ...hit('web', 'https://example.com'), text: 'Same words.' }, + ], + diagnostics: {}, + }); + const runQuery = vi.fn(async () => ['doc']); + const result = await searchKnowledge({ runQuery } as never, { + organizationId: 'org_1', + orgSlug: 'acme', + query: 'same', + access: ACCESS, + }); + expect(result.hits).toHaveLength(2); + }); +}); diff --git a/services/platform/convex/knowledge/search.ts b/services/platform/convex/knowledge/search.ts index 071a0f0493..1b9172108f 100644 --- a/services/platform/convex/knowledge/search.ts +++ b/services/platform/convex/knowledge/search.ts @@ -51,6 +51,7 @@ import type { import { PRIVATE_KNOWLEDGE_SCHEMA, corporaFor, + type KnowledgeHit, type KnowledgeQuery, type KnowledgeResult, } from '../../lib/knowledge/types'; @@ -147,12 +148,46 @@ export async function searchKnowledge( const allowed = new Set(retrievable); return { ...result, - hits: result.hits.filter( - (hit) => hit.corpus !== 'documents' || allowed.has(hit.source.ref), + hits: dropRepeatedPassages( + result.hits.filter( + (hit) => hit.corpus !== 'documents' || allowed.has(hit.source.ref), + ), ), }; } +/** + * Drop a passage the caller has already been given, keeping the best-scoring + * one. + * + * The same text can sit in the corpus twice — the same file uploaded as two + * documents, or a paragraph two documents share. Both copies match, and a + * bounded result set then spends two of its slots saying one thing while a + * different answer falls off the end. + * + * Runs AFTER the retrievability filter, deliberately. Deduping first could + * keep a copy the caller cannot read and drop the readable one, and the gate + * would then remove what was kept — losing the passage entirely rather than + * showing it once. Fusion has already sorted by score, so the first occurrence + * is the best one. + */ +function dropRepeatedPassages( + hits: readonly Hit[], +): Hit[] { + const seen = new Set(); + const kept: Hit[] = []; + for (const hit of hits) { + // Keyed on the text a caller actually reads. Whitespace is normalized so + // two copies that differ only in how their source wrapped lines still + // count as one. + const key = `${hit.corpus}\u0000${hit.text.replace(/\s+/g, ' ').trim()}`; + if (seen.has(key)) continue; + seen.add(key); + kept.push(hit); + } + return kept; +} + /** * A `knowledge.search` backend bound to one organization. *