Skip to content

Commit d187644

Browse files
committed
fix(knowledge): cut bounded text with a shared surrogate-safe helper inside the limit and leave stored upload metadata untouched
1 parent 20f9277 commit d187644

8 files changed

Lines changed: 76 additions & 32 deletions

File tree

‎apps/sim/lib/api/contracts/knowledge/documents.test.ts‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -271,7 +271,11 @@ describe('document filename and tag bounds', () => {
271271
})
272272

273273
it('rejects a filename over the limit on create, upsert, and update with a descriptive message', () => {
274-
for (const schema of [createDocumentBodySchema, upsertDocumentBodySchema, updateDocumentBodySchema]) {
274+
for (const schema of [
275+
createDocumentBodySchema,
276+
upsertDocumentBodySchema,
277+
updateDocumentBodySchema,
278+
]) {
275279
const result = schema.safeParse({ ...base, filename: overLimit })
276280
expect(result.success).toBe(false)
277281
expect(result.error?.issues[0]?.message).toBe(

‎apps/sim/lib/api/contracts/knowledge/documents.ts‎

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -122,13 +122,19 @@ export function parseDocumentTagFiltersParam(
122122
/** A text tag value that fits its index row; see {@link MAX_DOCUMENT_INDEXED_TEXT_LENGTH}. */
123123
const documentTagValueSchema = z
124124
.string()
125-
.max(MAX_DOCUMENT_INDEXED_TEXT_LENGTH, `Tag values cannot exceed ${MAX_DOCUMENT_INDEXED_TEXT_LENGTH} characters`)
125+
.max(
126+
MAX_DOCUMENT_INDEXED_TEXT_LENGTH,
127+
`Tag values cannot exceed ${MAX_DOCUMENT_INDEXED_TEXT_LENGTH} characters`
128+
)
126129

127130
export const createDocumentBodySchema = z.object({
128131
filename: z
129132
.string()
130133
.min(1, 'Filename is required')
131-
.max(MAX_DOCUMENT_INDEXED_TEXT_LENGTH, `Filename cannot exceed ${MAX_DOCUMENT_INDEXED_TEXT_LENGTH} characters`),
134+
.max(
135+
MAX_DOCUMENT_INDEXED_TEXT_LENGTH,
136+
`Filename cannot exceed ${MAX_DOCUMENT_INDEXED_TEXT_LENGTH} characters`
137+
),
132138
fileUrl: knowledgeDocumentFileUrlSchema,
133139
fileSize: z.number().min(1, 'File size must be greater than 0'),
134140
mimeType: z.string().min(1, 'MIME type is required'),
@@ -180,7 +186,10 @@ export const upsertDocumentBodySchema = z.object({
180186
filename: z
181187
.string()
182188
.min(1, 'Filename is required')
183-
.max(MAX_DOCUMENT_INDEXED_TEXT_LENGTH, `Filename cannot exceed ${MAX_DOCUMENT_INDEXED_TEXT_LENGTH} characters`),
189+
.max(
190+
MAX_DOCUMENT_INDEXED_TEXT_LENGTH,
191+
`Filename cannot exceed ${MAX_DOCUMENT_INDEXED_TEXT_LENGTH} characters`
192+
),
184193
fileUrl: knowledgeDocumentFileUrlSchema,
185194
fileSize: z.number().min(1, 'File size must be greater than 0'),
186195
mimeType: z.string().min(1, 'MIME type is required'),
@@ -214,7 +223,11 @@ export const updateDocumentBodySchema = z.object({
214223
filename: z
215224
.string()
216225
.min(1, 'Filename is required')
217-
.max(MAX_DOCUMENT_INDEXED_TEXT_LENGTH, `Filename cannot exceed ${MAX_DOCUMENT_INDEXED_TEXT_LENGTH} characters`).optional(),
226+
.max(
227+
MAX_DOCUMENT_INDEXED_TEXT_LENGTH,
228+
`Filename cannot exceed ${MAX_DOCUMENT_INDEXED_TEXT_LENGTH} characters`
229+
)
230+
.optional(),
218231
enabled: z.boolean().optional(),
219232
chunkCount: z.number().min(0).optional(),
220233
tokenCount: z.number().min(0).optional(),

‎apps/sim/lib/knowledge/connectors/sync-persistence.test.ts‎

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,10 @@ vi.mock('@/lib/knowledge/documents/storage-cleanup', () => ({
3030
vi.mock('@/connectors/registry.server', () => ({
3131
CONNECTOR_REGISTRY: {
3232
fixture: {
33-
mapTags: (metadata: Record<string, unknown>) => ({ label: metadata.label, owner: metadata.owner }),
33+
mapTags: (metadata: Record<string, unknown>) => ({
34+
label: metadata.label,
35+
owner: metadata.owner,
36+
}),
3437
},
3538
},
3639
}))
@@ -366,7 +369,8 @@ describe('persistSourceDocumentFailures', () => {
366369
priorByExternalId: new Map(),
367370
})
368371
const [rows] = dbChainMockFns.values.mock.calls[0] as [Array<{ filename: string }>]
369-
expect(rows[0].filename).toBe(`${'x'.repeat(512)}...`)
372+
expect(rows[0].filename).toBe(`${'x'.repeat(509)}...`)
373+
expect(rows[0].filename.length).toBe(512)
370374
})
371375
it('refuses to commit a failure under a reclaimed lease', async () => {
372376
queueTableRows(schemaMock.knowledgeBase, [{ id: 'kb' }])
@@ -442,7 +446,7 @@ describe('resolveTagMapping', () => {
442446
{ label: 'y'.repeat(5000), owner: 'Purchasing' },
443447
{ tagSlotMapping: { label: 'tag1', owner: 'tag2' } }
444448
)
445-
expect(tags?.tag1).toBe(`${'y'.repeat(512)}...`)
449+
expect(tags?.tag1).toBe(`${'y'.repeat(509)}...`)
446450
expect(tags?.tag2).toBe('Purchasing')
447451
})
448452

@@ -452,6 +456,7 @@ describe('resolveTagMapping', () => {
452456
{ label: '\u{1F600}'.repeat(600) },
453457
{ tagSlotMapping: { label: 'tag1' } }
454458
)
455-
expect(tags?.tag1).toBe(`${'\u{1F600}'.repeat(512)}...`)
459+
expect(tags?.tag1).toBe(`${'\u{1F600}'.repeat(254)}...`)
460+
expect(tags?.tag1?.length).toBeLessThanOrEqual(512)
456461
})
457462
})

‎apps/sim/lib/knowledge/connectors/sync-persistence.ts‎

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import { document, embedding, knowledgeBase, knowledgeConnector } from '@sim/db/
33
import { createLogger } from '@sim/logger'
44
import { chunkArray } from '@sim/utils/helpers'
55
import { generateId } from '@sim/utils/id'
6+
import { truncateAtCodePoint } from '@sim/utils/string'
67
import { and, eq, exists, inArray, isNull, lt, or, sql } from 'drizzle-orm'
78
import { getInternalApiBaseUrl } from '@/lib/core/utils/urls'
89
import type { DbOrTx } from '@/lib/db/types'
@@ -18,6 +19,7 @@ import type { ConnectorFailureDiagnostic } from '@/lib/knowledge/connectors/conn
1819
import { resolveSourceModifiedAt } from '@/lib/knowledge/connectors/source-modified-at'
1920
import { SOURCE_CONTENT_ERROR } from '@/lib/knowledge/connectors/sync-limits'
2021
import { assertSyncLeaseHeldInTx, type SyncWriteLease } from '@/lib/knowledge/connectors/sync-lock'
22+
import { MAX_DOCUMENT_INDEXED_TEXT_LENGTH } from '@/lib/knowledge/constants'
2123
import type { DocumentData } from '@/lib/knowledge/documents/service'
2224
import { enqueueKnowledgeStorageCleanup } from '@/lib/knowledge/documents/storage-cleanup'
2325
import {
@@ -26,7 +28,6 @@ import {
2628
} from '@/lib/knowledge/documents/storage-upload'
2729
import { buildStorageKeySegment } from '@/lib/uploads/core/storage-key'
2830
import { getFileMetadataByKeys } from '@/lib/uploads/server/metadata'
29-
import { MAX_DOCUMENT_INDEXED_TEXT_LENGTH } from '@/lib/knowledge/constants'
3031
import { CONNECTOR_REGISTRY } from '@/connectors/registry.server'
3132
import type { DocumentTags, ExternalDocument } from '@/connectors/types'
3233

@@ -174,16 +175,20 @@ export async function persistDocumentAcls(
174175

175176
const MAX_SAFE_TITLE_LENGTH = 200
176177

178+
/** The suffix a cut value carries, counted inside {@link MAX_DOCUMENT_INDEXED_TEXT_LENGTH}. */
179+
const INDEXED_TEXT_CUT_SUFFIX = '...'
180+
177181
/**
178182
* Source titles and mapped tag values are untrusted machine input with no caller to refuse them,
179-
* so they are cut to {@link MAX_DOCUMENT_INDEXED_TEXT_LENGTH} by code point, never inside a
180-
* surrogate pair.
183+
* so they are cut to {@link MAX_DOCUMENT_INDEXED_TEXT_LENGTH} code units, suffix included, and
184+
* never inside a surrogate pair. The result always passes the document APIs' own bound.
181185
*/
182186
function boundIndexedText(value: string): string {
183-
if (value.length <= MAX_DOCUMENT_INDEXED_TEXT_LENGTH) return value
184-
const points = Array.from(value)
185-
if (points.length <= MAX_DOCUMENT_INDEXED_TEXT_LENGTH) return value
186-
return `${points.slice(0, MAX_DOCUMENT_INDEXED_TEXT_LENGTH).join('')}...`
187+
return truncateAtCodePoint(
188+
value,
189+
MAX_DOCUMENT_INDEXED_TEXT_LENGTH - INDEXED_TEXT_CUT_SUFFIX.length,
190+
INDEXED_TEXT_CUT_SUFFIX
191+
)
187192
}
188193

189194
function sanitizeStorageTitle(title: string): string {

‎apps/sim/lib/knowledge/upload-metadata.test.ts‎

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -7,19 +7,8 @@ import {
77
knowledgeDocumentUploadMetadataSchema,
88
persistedKnowledgeDocumentUploadMetadataSchema,
99
} from '@/lib/knowledge/upload-metadata'
10-
import { MAX_DOCUMENT_INDEXED_TEXT_LENGTH } from '@/lib/knowledge/constants'
1110

1211
describe('knowledgeDocumentUploadMetadataSchema', () => {
13-
it('rejects a tag value that would not fit its index row', () => {
14-
const result = knowledgeDocumentUploadMetadataSchema.safeParse({
15-
tag1: 'a'.repeat(MAX_DOCUMENT_INDEXED_TEXT_LENGTH + 1),
16-
})
17-
expect(result.success).toBe(false)
18-
expect(result.error?.issues[0]?.message).toBe(
19-
`Knowledge document tag values cannot exceed ${MAX_DOCUMENT_INDEXED_TEXT_LENGTH} characters`
20-
)
21-
})
22-
2312
it('rejects a recipe outside the accepted set', () => {
2413
const result = knowledgeDocumentUploadMetadataSchema.safeParse({
2514
processingOptions: { recipe: 'totally-bogus-recipe' },

‎apps/sim/lib/knowledge/upload-metadata.ts‎

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
import { z } from 'zod'
22
import type { RecursiveRecipe } from '@/lib/chunkers/types'
3-
import { MAX_DOCUMENT_INDEXED_TEXT_LENGTH } from '@/lib/knowledge/constants'
43

54
/**
65
* Recipes the recursive chunker implements. Mirrors `RecursiveRecipe`; the
@@ -36,10 +35,7 @@ const LANGUAGE_TAG_SHAPE = /^[A-Za-z]{2,8}(?:-[A-Za-z0-9]{1,8})*$/
3635

3736
const knowledgeDocumentUploadTagSchema = z
3837
.string()
39-
.max(
40-
MAX_DOCUMENT_INDEXED_TEXT_LENGTH,
41-
`Knowledge document tag values cannot exceed ${MAX_DOCUMENT_INDEXED_TEXT_LENGTH} characters`
42-
)
38+
.max(1000, 'Knowledge document tag values cannot exceed 1000 characters')
4339
.optional()
4440

4541
const knowledgeDocumentUploadTagShape = {

‎packages/utils/src/string.test.ts‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import {
1616
slugify,
1717
stripVersionSuffix,
1818
truncate,
19+
truncateAtCodePoint,
1920
} from './string.js'
2021

2122
describe('slugify', () => {
@@ -304,3 +305,19 @@ describe('hasRegexMetacharacter', () => {
304305
}
305306
})
306307
})
308+
309+
describe('truncateAtCodePoint', () => {
310+
it('returns the input untouched when it fits', () => {
311+
expect(truncateAtCodePoint('ab😀cd', 10)).toBe('ab😀cd')
312+
})
313+
314+
it('cuts like truncate when the cut lands between code points', () => {
315+
expect(truncateAtCodePoint('ab😀cd', 4)).toBe('ab😀...')
316+
expect(truncateAtCodePoint('hello world', 8, ' …')).toBe('hello wo …')
317+
})
318+
319+
it('moves the cut back one unit rather than splitting a surrogate pair', () => {
320+
expect(truncateAtCodePoint('ab😀cd', 3)).toBe('ab...')
321+
expect(truncateAtCodePoint('😀'.repeat(3), 3)).toBe('😀...')
322+
})
323+
})

‎packages/utils/src/string.ts‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,21 @@ export function truncate(str: string, sliceLength: number, suffix = '...'): stri
3333
return str.length > sliceLength ? str.slice(0, sliceLength) + suffix : str
3434
}
3535

36+
/**
37+
* Like {@link truncate}, but never cuts inside a surrogate pair: when the code unit at the cut
38+
* would split an astral character, the cut moves one unit earlier. Lengths are still counted in
39+
* UTF-16 code units, so the result of a cut is at most `sliceLength + suffix.length` units.
40+
*
41+
* @example
42+
* truncateAtCodePoint('ab😀cd', 3) // 'ab...' (the cut at 3 would split the emoji)
43+
* truncateAtCodePoint('ab😀cd', 4) // 'ab😀...'
44+
*/
45+
export function truncateAtCodePoint(str: string, sliceLength: number, suffix = '...'): string {
46+
if (str.length <= sliceLength) return str
47+
const splitsPair = sliceLength > 0 && (str.charCodeAt(sliceLength - 1) & 0xfc00) === 0xd800
48+
return str.slice(0, splitsPair ? sliceLength - 1 : sliceLength) + suffix
49+
}
50+
3651
/**
3752
* Lowercases `value` into the `[a-z0-9-]` charset: every run of other characters
3853
* becomes one hyphen, and leading and trailing hyphens are dropped.

0 commit comments

Comments
 (0)