Skip to content

Commit c698fb5

Browse files
committed
fix(knowledge): bill failed image reads and hold image restores while images are off
1 parent 8c52d6b commit c698fb5

10 files changed

Lines changed: 364 additions & 149 deletions

File tree

‎apps/sim/lib/knowledge/__integration__/confluence-images.integration.ts‎

Lines changed: 138 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
11
/**
22
* Real sync workers, PostgreSQL, file storage, indexing and reconciliation for Confluence image
33
* attachments. The source listing is a synthetic connector behind the real Confluence meta, and
4-
* the vision extraction and embeddings are synthetic, so no request leaves the process.
4+
* the vision extraction and embeddings are synthetic, so no request leaves the process. A
5+
* `cutOff` image runs the real extractor against a Gemini reply stopped at its token limit.
56
*/
67
import { mkdtempSync } from 'node:fs'
78
import { rm } from 'node:fs/promises'
@@ -13,6 +14,7 @@ import {
1314
embedding,
1415
knowledgeConnector,
1516
organization,
17+
usageLog,
1618
user,
1719
workspace,
1820
} from '@sim/db/schema'
@@ -27,6 +29,9 @@ const fixture = vi.hoisted(() => {
2729
/** externalId to version; a version bump changes the image's bytes and hash. */
2830
images: new Map<string, number>(),
2931
decorative: new Set<string>(),
32+
cutOff: new Set<string>(),
33+
/** Text documents listed beside the images, whatever the image setting. */
34+
pages: new Set<string>(),
3035
extractions: [] as string[],
3136
/** Each chunk gets its own one-hot vector, so the shared HNSW graph sees no duplicates. */
3237
nextVector: 0,
@@ -53,22 +58,56 @@ vi.mock('@/lib/embeddings', async () => ({
5358
pricingId: 'text-embedding-3-small',
5459
}),
5560
}))
56-
vi.mock('@/lib/knowledge/documents/image-extraction', async (importOriginal) => ({
57-
...(await importOriginal<typeof import('@/lib/knowledge/documents/image-extraction')>()),
58-
extractImageText: async ({ filename }: { filename: string }) => {
59-
fixture.extractions.push(filename)
60-
const externalId = filename.replace(/\.png$/, '')
61-
if (fixture.decorative.has(externalId)) return { decorative: true, renderedText: '' }
62-
return {
63-
decorative: false,
64-
renderedText: `${externalId} version ${fixture.images.get(externalId)}: Region EMEA launches in Q1.`,
65-
usage: { model: 'gemini-3.8-flash', cost: 0, inputTokens: 10, outputTokens: 10 },
66-
}
67-
},
61+
vi.mock('@/lib/knowledge/documents/image-extraction-gemini', async (importOriginal) => ({
62+
...(await importOriginal<typeof import('@/lib/knowledge/documents/image-extraction-gemini')>()),
63+
requestGeminiImageExtraction: async () => ({
64+
text: '{"kind": "table", "rendered_text": "Region: EMEA',
65+
finishReason: 'MAX_TOKENS',
66+
inputTokens: 1200,
67+
outputTokens: 32_768,
68+
}),
6869
}))
70+
vi.mock('@/lib/knowledge/documents/image-extraction', async (importOriginal) => {
71+
const actual = await importOriginal<typeof import('@/lib/knowledge/documents/image-extraction')>()
72+
return {
73+
...actual,
74+
extractImageText: async (input: Parameters<typeof actual.extractImageText>[0]) => {
75+
const externalId = input.filename.replace(/\.png$/, '')
76+
if (fixture.cutOff.has(externalId)) return actual.extractImageText(input)
77+
fixture.extractions.push(input.filename)
78+
if (fixture.decorative.has(externalId)) return { decorative: true, renderedText: '' }
79+
return {
80+
decorative: false,
81+
renderedText: `${externalId} version ${fixture.images.get(externalId)}: Region EMEA launches in Q1.`,
82+
}
83+
},
84+
}
85+
})
6986
vi.mock('@/connectors/registry.server', async () => {
7087
const { confluenceConnectorMeta } = await import('@/connectors/confluence/meta')
7188
const enabled = (config: Record<string, unknown>) => config.imageAttachments === 'semantic'
89+
const page = (externalId: string) => ({
90+
externalId,
91+
title: externalId,
92+
content: `${externalId}: the EMEA launch plan covers regional owners, launch dates, and the field marketing calendar for the first quarter of 2026.`,
93+
mimeType: 'text/plain',
94+
contentHash: `fixture-page:${externalId}`,
95+
})
96+
/** A real, noisy PNG over the semantic byte floor, so the real extractor reads it. */
97+
const cutOffImage = async () => {
98+
const sharp = (await import('sharp')).default
99+
return sharp({
100+
create: {
101+
width: 256,
102+
height: 256,
103+
channels: 3,
104+
background: '#808080',
105+
noise: { type: 'gaussian', mean: 128, sigma: 60 },
106+
},
107+
})
108+
.png()
109+
.toBuffer()
110+
}
72111
const stub = (externalId: string) => {
73112
const version = fixture.images.get(externalId) ?? 1
74113
return {
@@ -86,16 +125,22 @@ vi.mock('@/connectors/registry.server', async () => {
86125
...confluenceConnectorMeta,
87126
validateConfig: async () => ({ valid: true }),
88127
listDocuments: async (_token: string, config: Record<string, unknown>) => ({
89-
documents: enabled(config) ? [...fixture.images.keys()].map(stub) : [],
128+
documents: [
129+
...(enabled(config) ? [...fixture.images.keys()].map(stub) : []),
130+
...[...fixture.pages].map(page),
131+
],
90132
hasMore: false,
91133
}),
92134
getDocument: async (
93135
_token: string,
94136
_config: Record<string, unknown>,
95137
externalId: string
96138
) => {
139+
if (fixture.pages.has(externalId)) return page(externalId)
97140
if (!fixture.images.has(externalId)) return null
98-
const bytes = Buffer.from(`fixture png ${externalId} v${fixture.images.get(externalId)}`)
141+
const bytes = fixture.cutOff.has(externalId)
142+
? await cutOffImage()
143+
: Buffer.from(`fixture png ${externalId} v${fixture.images.get(externalId)}`)
99144
return {
100145
...stub(externalId),
101146
contentDeferred: false,
@@ -116,13 +161,20 @@ import {
116161
import {
117162
createKnowledgeConnector,
118163
updateKnowledgeConnector,
164+
updateKnowledgeConnectorDocuments,
119165
} from '@/lib/knowledge/application/connectors'
120166
import * as connectorTokens from '@/lib/knowledge/connectors/access-token'
121167
import { executeSync } from '@/lib/knowledge/connectors/sync-engine'
122168
import { IMAGES_DEFERRED_NOTICE } from '@/lib/knowledge/connectors/sync-limits'
169+
import { resolveImageExtractionModel } from '@/lib/knowledge/documents/image-extraction-gemini'
123170

124171
describe('Confluence image attachments through sync, indexing and reconciliation', () => {
125172
const ids = createKnowledgeAclFixtureIds()
173+
const principal = {
174+
kind: 'session',
175+
userId: ids.aliceId,
176+
sessionId: 'confluence-images',
177+
} as const
126178
let billing: Awaited<ReturnType<typeof resolveBillingAttribution>>
127179
const priorGeminiKeys = {
128180
GEMINI_API_KEY: env.GEMINI_API_KEY,
@@ -169,7 +221,7 @@ describe('Confluence image attachments through sync, indexing and reconciliation
169221
/** Changes the setting the way a user does; the update dispatches the next sync, which runs in-process. */
170222
async function setImageAttachments(imageAttachments: 'semantic' | 'off') {
171223
await updateKnowledgeConnector.execute({
172-
principal: { kind: 'session', userId: ids.aliceId, sessionId: 'confluence-images' },
224+
principal,
173225
input: {
174226
connectorId: ids.connectorId,
175227
updates: {
@@ -232,6 +284,8 @@ describe('Confluence image attachments through sync, indexing and reconciliation
232284
Object.assign(env, { GEMINI_API_KEY: 'fixture-gemini-key' })
233285
fixture.extractions.length = 0
234286
fixture.decorative.clear()
287+
fixture.cutOff.clear()
288+
fixture.pages.clear()
235289
fixture.images.clear()
236290
await db.delete(document).where(eq(document.connectorId, ids.connectorId))
237291
await configure('semantic')
@@ -306,6 +360,73 @@ describe('Confluence image attachments through sync, indexing and reconciliation
306360
expect(fixture.extractions.sort()).toEqual(['image-a.png', 'image-b.png'])
307361
}, 120_000)
308362

363+
it('keeps an excluded image out of search when it is restored while Image Attachments is off', async () => {
364+
fixture.images.set('image-a', 1)
365+
fixture.pages.add('page-a')
366+
await sync()
367+
const indexed = await images()
368+
const image = indexed.find((row) => row.externalId === 'image-a')
369+
const textPage = indexed.find((row) => row.externalId === 'page-a')
370+
if (!image || !textPage) throw new Error('Expected the image and the page to be indexed')
371+
const documentIds = [image.id, textPage.id]
372+
await updateKnowledgeConnectorDocuments.execute({
373+
principal,
374+
input: { connectorId: ids.connectorId, operation: 'exclude', documentIds },
375+
})
376+
377+
await setImageAttachments('off')
378+
/** The exclusion is a tombstone the removal pass keeps, vectors included. */
379+
expect(await vectors([image.id])).toHaveLength(1)
380+
381+
const restored = await updateKnowledgeConnectorDocuments.execute({
382+
principal,
383+
input: { connectorId: ids.connectorId, operation: 'restore', documentIds },
384+
})
385+
expect(restored.documentIds).toEqual([textPage.id])
386+
await expect(
387+
updateKnowledgeConnectorDocuments.execute({
388+
principal,
389+
input: { connectorId: ids.connectorId, operation: 'restore', documentIds: [image.id] },
390+
})
391+
).rejects.toMatchObject({ code: 'validation' })
392+
expect(
393+
(await images()).map(({ externalId, userExcluded, enabled }) => ({
394+
externalId,
395+
userExcluded,
396+
enabled,
397+
}))
398+
).toEqual([
399+
{ externalId: 'image-a', userExcluded: true, enabled: false },
400+
{ externalId: 'page-a', userExcluded: false, enabled: true },
401+
])
402+
}, 120_000)
403+
404+
it('bills the Gemini call once when a reply cut off at its token limit fails the image', async () => {
405+
fixture.images.set('image-a', 1)
406+
fixture.cutOff.add('image-a')
407+
await executeSync(ids.connectorId, { fullSync: true, billingAttribution: billing })
408+
409+
await expect
410+
.poll(async () => (await images()).map((row) => row.processingStatus), { timeout: 45_000 })
411+
.toEqual(['failed'])
412+
const [failed] = await images()
413+
expect(failed.processingError).toContain('more text than one extraction can return')
414+
expect(await vectors([failed.id])).toEqual([])
415+
const charges = await db
416+
.select()
417+
.from(usageLog)
418+
.where(
419+
and(
420+
eq(usageLog.workspaceId, ids.workspaceId),
421+
eq(usageLog.source, 'knowledge-base'),
422+
eq(usageLog.description, resolveImageExtractionModel())
423+
)
424+
)
425+
expect(charges).toHaveLength(1)
426+
expect(Number(charges[0].cost)).toBeGreaterThan(0)
427+
expect(charges[0].metadata).toEqual({ inputTokens: 1200, outputTokens: 32_768 })
428+
}, 120_000)
429+
309430
it('keeps images off for a connector created before the setting existed', async () => {
310431
fixture.images.set('image-a', 1)
311432
await configure(undefined)
@@ -349,7 +470,7 @@ describe('Confluence image attachments through sync, indexing and reconciliation
349470
'saves a new connector with Image Attachments on unless the request chose otherwise (%o)',
350471
async (chosen, saved) => {
351472
const created = await createKnowledgeConnector.execute({
352-
principal: { kind: 'session', userId: ids.aliceId, sessionId: 'confluence-images' },
473+
principal,
353474
input: {
354475
knowledgeBaseId: ids.knowledgeBaseId,
355476
connectorType: 'confluence',

‎apps/sim/lib/knowledge/application/connectors.ts‎

Lines changed: 35 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,9 @@ import {
1010
knowledgeConnectorSyncLog,
1111
} from '@sim/db/schema'
1212
import { toError } from '@sim/utils/errors'
13+
import { toRecord } from '@sim/utils/object'
1314
import { escapeLikePattern } from '@sim/utils/string'
14-
import { and, asc, desc, eq, inArray, isNull, lt, or, sql } from 'drizzle-orm'
15+
import { and, asc, desc, eq, inArray, isNull, lt, notInArray, or, sql } from 'drizzle-orm'
1516
import type { ConnectorDocumentFilter } from '@/lib/api/contracts/knowledge/connectors'
1617
import type { BillingAttributionSnapshot } from '@/lib/billing/core/billing-attribution'
1718
import { requireCurrentHumanRole } from '@/lib/core/application'
@@ -83,6 +84,10 @@ import {
8384
MAX_KNOWLEDGE_CONNECTOR_DOCUMENT_PAGE_SIZE,
8485
MAX_KNOWLEDGE_CONNECTOR_DOCUMENT_SEARCH_LENGTH,
8586
} from '@/lib/knowledge/constants'
87+
import {
88+
isImageExtractionEnabled,
89+
SEMANTIC_IMAGE_MIME_TYPES,
90+
} from '@/lib/knowledge/documents/image-extraction-policy'
8691
import {
8792
documentProcessingOutcomeSelection,
8893
failedDocumentCondition,
@@ -1294,20 +1299,39 @@ export const updateKnowledgeConnectorDocuments = defineAuthorizedKnowledgeUseCas
12941299
if (restoring && !requiresConnectorIndexing(context.knowledgeBase.isSearchIndex)) {
12951300
throw new OrchestrationError('validation', 'This search index is inactive; use Sim Search.')
12961301
}
1302+
const imageExtraction = getConnectorMeta(context.connector.connectorType)?.imageExtraction
1303+
/** An excluded image keeps its vectors, so restoring it while images are off would make it searchable again. */
1304+
const imagesOff =
1305+
restoring &&
1306+
imageExtraction !== undefined &&
1307+
!isImageExtractionEnabled(imageExtraction, toRecord(context.connector.sourceConfig))
1308+
const requested = and(
1309+
eq(document.connectorId, context.connectorId),
1310+
inArray(document.id, documentIds),
1311+
eq(document.userExcluded, restoring),
1312+
isNull(document.archivedAt),
1313+
isNull(document.deletedAt),
1314+
knowledgeAccessCondition(await context.access.get())
1315+
)
1316+
const semanticImages = [...SEMANTIC_IMAGE_MIME_TYPES]
12971317
const updated = await db
12981318
.update(document)
12991319
.set({ userExcluded: !restoring, enabled: restoring })
1300-
.where(
1301-
and(
1302-
eq(document.connectorId, context.connectorId),
1303-
inArray(document.id, documentIds),
1304-
eq(document.userExcluded, restoring),
1305-
isNull(document.archivedAt),
1306-
isNull(document.deletedAt),
1307-
knowledgeAccessCondition(await context.access.get())
1308-
)
1309-
)
1320+
.where(imagesOff ? and(requested, notInArray(document.mimeType, semanticImages)) : requested)
13101321
.returning({ id: document.id })
1322+
if (imagesOff && updated.length === 0) {
1323+
const [heldImage] = await db
1324+
.select({ id: document.id })
1325+
.from(document)
1326+
.where(and(requested, inArray(document.mimeType, semanticImages)))
1327+
.limit(1)
1328+
if (heldImage) {
1329+
throw new OrchestrationError(
1330+
'validation',
1331+
'Set Image Attachments to Read with AI to restore images.'
1332+
)
1333+
}
1334+
}
13111335
return {
13121336
operation: input.operation,
13131337
count: updated.length,

‎apps/sim/lib/knowledge/connectors/service.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ export interface ActiveKnowledgeConnectorReference {
88
knowledgeBaseId: string
99
connectorType: string
1010
status: string
11+
sourceConfig: unknown
1112
}
1213

1314
/** Resolves a connector's canonical active parent without trusting a caller-supplied KB ID. */
@@ -20,6 +21,7 @@ export async function getActiveKnowledgeConnectorReference(
2021
knowledgeBaseId: knowledgeConnector.knowledgeBaseId,
2122
connectorType: knowledgeConnector.connectorType,
2223
status: knowledgeConnector.status,
24+
sourceConfig: knowledgeConnector.sourceConfig,
2325
})
2426
.from(knowledgeConnector)
2527
.where(and(eq(knowledgeConnector.id, connectorId), connectorIsLive()))

0 commit comments

Comments
 (0)