Skip to content

Commit 305da02

Browse files
authored
improvement(knowledge): rank inside the permitted set for organization search (#7996)
* improvement(knowledge): rank inside the permitted set for organization search * fix(knowledge): count a caller's reach inside the requested bases
1 parent 1f16b03 commit 305da02

5 files changed

Lines changed: 623 additions & 102 deletions

File tree

‎apps/sim/lib/knowledge/__integration__/kb-block-search.integration.ts‎

Lines changed: 40 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,14 +3,18 @@ import type { Principal } from '@sim/auth/principal'
33
import { db } from '@sim/db'
44
import { document, embedding, knowledgeBase, organization, user, workspace } from '@sim/db/schema'
55
import { generateId } from '@sim/utils/id'
6-
import { eq, inArray } from 'drizzle-orm'
6+
import { eq, inArray, sql } from 'drizzle-orm'
77
import { afterAll, beforeAll, describe, expect, it } from 'vitest'
88
import {
99
createKnowledgeAclFixtureIds,
1010
seedKnowledgeAclFixture,
1111
} from '@/lib/knowledge/__integration__/seed-source-access-fixture'
1212
import { createKnowledgeAccessProvider } from '@/lib/knowledge/access/scope'
13-
import { retrieveKnowledgeSearch } from '@/lib/knowledge/search/queries'
13+
import {
14+
resolvePermittedDocuments,
15+
retrieveKnowledgeSearch,
16+
VECTOR_PROBE_DOCUMENT_LIMIT,
17+
} from '@/lib/knowledge/search/queries'
1418
import { embeddingVectorValues } from '@/lib/knowledge/vector-columns'
1519

1620
describe('API-key KB block fan-out', () => {
@@ -140,13 +144,45 @@ describe('API-key KB block fan-out', () => {
140144
expect(matching('AS visible')).toHaveLength(bases.length)
141145
expect(matching(') + 0 LIMIT')).toHaveLength(bases.length)
142146
expect(matching('scored_search_candidates')).toHaveLength(bases.length)
143-
/** The probe enumerates visible documents; it never ranks them. */
147+
/** The probe enumerates visible documents and reports saturation; it never ranks them. */
144148
expect(
145-
statements.filter((query) => query.includes('AS id FROM') && !query.includes('ORDER BY'))
149+
statements.filter(
150+
(query) => query.includes('AS saturated') && !query.includes('ORDER BY')
151+
)
146152
).toHaveLength(bases.length)
147153
} finally {
148154
db.$client.options.debug = previousDebug
149155
}
150156
}
151157
)
158+
159+
it('bounds the permitted set by the requested bases, not by what the tokens reach elsewhere', async () => {
160+
const crowded = generateId()
161+
await db.insert(knowledgeBase).values({
162+
id: crowded,
163+
userId: ids.aliceId,
164+
workspaceId: ids.workspaceId,
165+
name: 'Crowded neighbour',
166+
})
167+
try {
168+
/** Baseline tokens are shared by every tenant, so another base can hold more than the limit. */
169+
await db.execute(sql`
170+
INSERT INTO ${document} (id, knowledge_base_id, filename, file_url, file_size, mime_type,
171+
processing_status, acl)
172+
SELECT 'crowded-' || n, ${crowded}, 'crowded', 'https://fixture.invalid/crowded', 1,
173+
'text/plain', 'completed', ARRAY['ws']::text[]
174+
FROM generate_series(1, ${VECTOR_PROBE_DOCUMENT_LIMIT + 1}) AS n
175+
`)
176+
const permitted = await resolvePermittedDocuments({
177+
knowledgeBaseIds: [bases[0].id],
178+
access: { kind: 'user', userId: ids.bobId, tokens: ['pub', 'ws'] },
179+
})
180+
expect(permitted).toEqual({
181+
kind: 'bounded',
182+
documents: [{ id: bases[0].visible, connectorId: null }],
183+
})
184+
} finally {
185+
await db.delete(knowledgeBase).where(eq(knowledgeBase.id, crowded))
186+
}
187+
})
152188
})

‎apps/sim/lib/knowledge/access/predicate.ts‎

Lines changed: 41 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -193,6 +193,26 @@ export function knowledgeMetadataCandidateAccessCondition(
193193
return storedKnowledgeAccessCondition(scope, sql`true`)
194194
}
195195

196+
/**
197+
* A members-mode document is readable while one of the caller's active member identities on its
198+
* connector still observes it, freshly. Correlated on the document so each check is a lookup on
199+
* the observation primary key, which leads with `document_id`: phrased as a row-value `IN`
200+
* inside the access predicate's `OR`, PostgreSQL instead hashes every observation in the table
201+
* once per statement, a fixed cost paid by every query that carries the predicate.
202+
*/
203+
function memberObservationCondition(tokens: SQL, cutoff: SQL): SQL {
204+
return sql`EXISTS (
205+
SELECT 1 FROM ${knowledgeDocumentObservation}
206+
JOIN ${knowledgeConnectorMember}
207+
ON ${knowledgeConnectorMember.id} = ${knowledgeDocumentObservation.memberId}
208+
WHERE ${knowledgeDocumentObservation.documentId} = ${document.id}
209+
AND ${knowledgeConnectorMember.connectorId} = ${document.connectorId}
210+
AND ${knowledgeConnectorMember.status} = 'active'
211+
AND ${knowledgeConnectorMember.subjectToken} = ANY(${tokens})
212+
AND GREATEST(${knowledgeDocumentObservation.lastSeenAt}, ${knowledgeConnectorMember.memberSyncedThrough}) > ${cutoff}
213+
)`
214+
}
215+
196216
function storedKnowledgeAccessCondition(
197217
scope: KnowledgeAccessScope | SystemAccessScope,
198218
liveSourceAccess: SQL
@@ -202,7 +222,7 @@ function storedKnowledgeAccessCondition(
202222
const tokens = textArrayLiteral(scope.tokens)
203223
const cutoff = sql`statement_timestamp() - (${SOURCE_ACL_MAX_AGE_MS} * interval '1 millisecond')`
204224
return sql`(
205-
${document.acl} && ${tokens}
225+
${aclOverlap(tokens)}
206226
AND NOT EXISTS (
207227
SELECT 1 FROM jsonb_array_elements(${document.aclRequirements}) AS required_clause(tokens)
208228
WHERE NOT (required_clause.tokens ?| ${tokens})
@@ -221,21 +241,33 @@ function storedKnowledgeAccessCondition(
221241
(${knowledgeConnector.accessMode} = 'workspace' AND ${document.acl} = ARRAY['ws']::text[])
222242
OR (${document.acl} <> ARRAY['ws']::text[] AND (
223243
(${knowledgeConnector.accessMode} = 'admin' AND ${document.aclVerifiedAt} > ${cutoff})
224-
OR (${knowledgeConnector.accessMode} = 'members' AND (${document.id}, ${document.connectorId}) IN (
225-
SELECT ${knowledgeDocumentObservation.documentId}, ${knowledgeConnectorMember.connectorId} FROM ${knowledgeDocumentObservation}
226-
JOIN ${knowledgeConnectorMember}
227-
ON ${knowledgeConnectorMember.id} = ${knowledgeDocumentObservation.memberId}
228-
WHERE ${knowledgeConnectorMember.status} = 'active'
229-
AND ${knowledgeConnectorMember.subjectToken} = ANY(${tokens})
230-
AND GREATEST(${knowledgeDocumentObservation.lastSeenAt}, ${knowledgeConnectorMember.memberSyncedThrough}) > ${cutoff}
231-
))
244+
OR (${knowledgeConnector.accessMode} = 'members' AND ${memberObservationCondition(tokens, cutoff)})
232245
))
233246
)
234247
)
235248
)
236249
)`
237250
}
238251

252+
/**
253+
* The token half of the stored access predicate: the documents a caller's tokens reach before
254+
* any source, freshness, or requirement check narrows them. It is a necessary condition of
255+
* {@link knowledgeAccessCondition}, never a substitute for it.
256+
*
257+
* Paired with `deleted_at IS NULL` it matches `doc_acl_gin_idx` exactly, so a query can enumerate
258+
* a member's reachable documents from that index alone. PostgreSQL cannot estimate array-overlap
259+
* selectivity, so left to itself it intersects this highly selective bitmap with base-wide ones.
260+
*/
261+
export function knowledgeAclOverlapCondition(scope: KnowledgeAccessScope): SQL {
262+
if (scope.tokens.length === 0) return sql`false`
263+
return aclOverlap(textArrayLiteral(scope.tokens))
264+
}
265+
266+
/** One spelling of the token overlap, so the probe's reach and the full predicate cannot drift. */
267+
function aclOverlap(tokens: SQL): SQL {
268+
return sql`${document.acl} && ${tokens}`
269+
}
270+
239271
/**
240272
* The pool uses fetch_types: false, so arrays must be constructed from scalar
241273
* parameters. A JSON scalar keeps large sets below PostgreSQL's bind limit.

‎apps/sim/lib/knowledge/search/diagnostics.ts‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ export type SearchStage =
2828
| 'access_scope'
2929
| 'defaults'
3030
| 'retrieval'
31+
| 'permitted_documents'
3132
| 'result_provenance'
3233
| 'reranking'
3334
| 'usage_recording'
@@ -89,6 +90,14 @@ export interface SearchDiagnosticMetadata {
8990
vectorCandidateLimit?: number
9091
/** Visible documents the tractability probe enumerated, capped at its own document limit. */
9192
vectorProbeDocumentCount?: number
93+
/**
94+
* Whether a user-scoped search resolved its permitted documents before retrieval: `bounded`
95+
* ranks inside that set, `unbounded` means it exceeded the probe's limit and both legs search
96+
* the index with the access predicate applied per candidate.
97+
*/
98+
permittedDocuments?: 'bounded' | 'unbounded'
99+
/** Documents in a bounded permitted set. */
100+
permittedDocumentCount?: number
92101
vectorCandidateCount?: number
93102
vectorCandidateDimensions?: number
94103
resultCount?: number

0 commit comments

Comments
 (0)