Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import { getErrorMessage } from '@sim/utils/errors'
import { useParams } from 'next/navigation'
import { type FieldErrors, useForm } from 'react-hook-form'
import { z } from 'zod'
import { MAX_CHUNKING_SEPARATOR_LENGTH, MAX_CHUNKING_SEPARATORS } from '@/lib/chunkers/constants'
import type { StrategyOptions } from '@/lib/chunkers/types'
import { KNOWLEDGE_BASE_DESCRIPTION_MAX_LENGTH } from '@/lib/knowledge/constants'
import {
Expand Down Expand Up @@ -57,6 +58,14 @@ const STRATEGY_OPTIONS = [
{ value: 'regex', label: 'Regex (custom pattern)' },
] as const

/** Splits the comma-separated separator field into the list the API receives. */
function parseSeparators(value: string | undefined): string[] {
if (!value?.trim()) return []
return value
.split(',')
.map((separator) => separator.trim().replace(/\\n/g, '\n').replace(/\\t/g, '\t'))
}

const STRATEGY_COMBOBOX_OPTIONS: ComboboxOption[] = STRATEGY_OPTIONS.map((o) => ({
label: o.label,
value: o.value,
Expand Down Expand Up @@ -124,6 +133,31 @@ const FormSchema = z
path: ['regexPattern'],
}
)
/**
* Gated on the strategy for the same reason the regex pattern is: the field only
* renders for `recursive` and only that strategy submits it, so an out-of-bound
* value left behind by a strategy switch must not block a submit that drops it.
*/
.refine(
(data) =>
data.strategy !== 'recursive' ||
parseSeparators(data.customSeparators).length <= MAX_CHUNKING_SEPARATORS,
{
message: `At most ${MAX_CHUNKING_SEPARATORS} separators are allowed`,
path: ['customSeparators'],
}
)
.refine(
(data) =>
data.strategy !== 'recursive' ||
parseSeparators(data.customSeparators).every(
(separator) => separator.length <= MAX_CHUNKING_SEPARATOR_LENGTH
),
{
message: `Each separator must be ${MAX_CHUNKING_SEPARATOR_LENGTH} characters or less`,
path: ['customSeparators'],
}
)
Comment thread
waleedlatif1 marked this conversation as resolved.

type FormInputValues = z.input<typeof FormSchema>
type FormValues = z.output<typeof FormSchema>
Expand Down Expand Up @@ -265,11 +299,7 @@ export const CreateBaseModal = memo(function CreateBaseModal({
...(data.regexStrictBoundaries && { strictBoundaries: true }),
}
: data.strategy === 'recursive' && data.customSeparators?.trim()
? {
separators: data.customSeparators
.split(',')
.map((s) => s.trim().replace(/\\n/g, '\n').replace(/\\t/g, '\t')),
}
? { separators: parseSeparators(data.customSeparators) }
: undefined

const newKnowledgeBase = await createKnowledgeBaseMutation.mutateAsync({
Expand Down Expand Up @@ -465,11 +495,13 @@ export const CreateBaseModal = memo(function CreateBaseModal({
<ChipModalField
type='custom'
title='Custom Separators (optional)'
hint='Comma-separated list of delimiters in priority order. Leave empty for default separators.'
hint={`Comma-separated list of delimiters in priority order, up to ${MAX_CHUNKING_SEPARATORS}. Leave empty for default separators.`}
error={errors.customSeparators?.message}
>
<ChipInput
placeholder='e.g. \n\n, \n, . , '
{...register('customSeparators')}
error={Boolean(errors.customSeparators)}
autoComplete='off'
data-form-type='other'
/>
Expand Down
77 changes: 77 additions & 0 deletions apps/sim/lib/api/contracts/knowledge/base.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
/**
* @vitest-environment node
*/
import { describe, expect, it } from 'vitest'
import {
chunkingStrategyOptionsSchema,
createKnowledgeBaseBodySchema,
knowledgeBaseDataSchema,
} from '@/lib/api/contracts/knowledge/base'
import { MAX_CHUNKING_SEPARATOR_LENGTH, MAX_CHUNKING_SEPARATORS } from '@/lib/chunkers/constants'

const separators = (count: number) => Array.from({ length: count }, (_, i) => `@@sep${i}@@`)

describe('chunkingStrategyOptionsSchema.separators', () => {
it('accepts a separator list at the bound', () => {
const parsed = chunkingStrategyOptionsSchema.parse({
separators: separators(MAX_CHUNKING_SEPARATORS),
})
expect(parsed.separators).toHaveLength(MAX_CHUNKING_SEPARATORS)
})

it('rejects more separators than the bound', () => {
const result = chunkingStrategyOptionsSchema.safeParse({
separators: separators(MAX_CHUNKING_SEPARATORS + 1),
})
expect(result.success).toBe(false)
})

it('rejects a separator longer than the per-item bound', () => {
const result = chunkingStrategyOptionsSchema.safeParse({
separators: ['|'.repeat(MAX_CHUNKING_SEPARATOR_LENGTH + 1)],
})
expect(result.success).toBe(false)
})

it('rejects an oversized list on the knowledge base create body', () => {
const result = createKnowledgeBaseBodySchema.safeParse({
name: 'kb',
workspaceId: 'ws',
chunkingConfig: {
maxSize: 1024,
minSize: 100,
overlap: 200,
strategy: 'recursive',
strategyOptions: { separators: separators(5000) },
},
})
expect(result.success).toBe(false)
})
})

describe('knowledgeBaseDataSchema.chunkingConfig', () => {
it('still reads a stored config written before the separator bound', () => {
const result = knowledgeBaseDataSchema.safeParse({
id: 'kb-1',
userId: 'u-1',
name: 'kb',
description: null,
tokenCount: 0,
embeddingModel: 'text-embedding-3-small',
embeddingDimension: 1536,
chunkingConfig: {
maxSize: 1024,
minSize: 100,
overlap: 200,
strategy: 'recursive',
strategyOptions: { separators: separators(5000) },
},
createdAt: new Date().toISOString(),
updatedAt: new Date().toISOString(),
deletedAt: null,
workspaceId: 'ws',
folderId: null,
})
expect(result.success).toBe(true)
})
})
34 changes: 32 additions & 2 deletions apps/sim/lib/api/contracts/knowledge/base.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import {
wireDateSchema,
} from '@/lib/api/contracts/knowledge/shared'
import { defineRouteContract } from '@/lib/api/contracts/types'
import { MAX_CHUNKING_SEPARATOR_LENGTH, MAX_CHUNKING_SEPARATORS } from '@/lib/chunkers/constants'
import type { StrategyOptions } from '@/lib/chunkers/types'
import {
DEFAULT_CHUNKING_CONFIG,
Expand All @@ -20,7 +21,13 @@ export const listKnowledgeBasesQuerySchema = z.object({
scope: knowledgeScopeSchema.default('active'),
})

export const chunkingStrategyOptionsSchema = z
/**
* Strategy options as they are stored. Reads stay tolerant of a `separators`
* list written before {@link chunkingStrategyOptionsSchema} bounded it, so an
* oversized legacy config lists instead of failing response validation. The
* chunker clamps such a list at construction, so nothing reprocesses unbounded.
*/
export const storedChunkingStrategyOptionsSchema = z
.object({
pattern: z
.string()
Expand All @@ -42,6 +49,29 @@ export const chunkingStrategyOptionsSchema = z
})
.strict() satisfies z.ZodType<StrategyOptions>

/**
* Strategy options accepted on writes. `separators` is bounded in both length
* and item size: the recursive chunker rescans the whole document once per
* separator, synchronously, so an unbounded list turns one persisted config
* into seconds of uninterruptible CPU on every later document upload.
*/
export const chunkingStrategyOptionsSchema = storedChunkingStrategyOptionsSchema
.extend({
separators: z
.array(
z
.string()
.max(
MAX_CHUNKING_SEPARATOR_LENGTH,
`Each separator must be ${MAX_CHUNKING_SEPARATOR_LENGTH} characters or less`
)
)
.max(MAX_CHUNKING_SEPARATORS, `At most ${MAX_CHUNKING_SEPARATORS} separators are allowed`)
.optional()
.describe('Ordered separators used to split content into chunks.'),
})
.strict() satisfies z.ZodType<StrategyOptions>

export const chunkingConfigSchema = z
.object({
maxSize: z.number().min(100).max(4000),
Expand Down Expand Up @@ -110,7 +140,7 @@ const knowledgeChunkingConfigSchema = z
minSize: z.number(),
overlap: z.number(),
strategy: z.enum(['auto', 'text', 'regex', 'recursive', 'sentence', 'token']).optional(),
strategyOptions: chunkingStrategyOptionsSchema.optional(),
strategyOptions: storedChunkingStrategyOptionsSchema.optional(),
})
.passthrough()

Expand Down
17 changes: 17 additions & 0 deletions apps/sim/lib/chunkers/constants.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
/**
* Bounds on the separator list a recursive chunking config may carry.
*
* `RecursiveChunker` scans the whole document once per separator and walks the
* list from the top for every oversized fragment, so the separator count is a
* direct multiplier on synchronous CPU per document. The work happens inside a
* split loop, which neither the processing `Promise.race` timeout nor the
* after-the-fact chunk-count cap can interrupt — the list has to be bounded on
* the way in instead.
*
* The largest built-in recipe (`markdown`) uses 16 separators, so 32 leaves room
* for a hand-tuned list without letting one config stall the processing tier.
*/
export const MAX_CHUNKING_SEPARATORS = 32

/** Max characters in a single chunking separator. Real delimiters are a few characters. */
export const MAX_CHUNKING_SEPARATOR_LENGTH = 100
60 changes: 59 additions & 1 deletion apps/sim/lib/chunkers/recursive-chunker.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@
*/

import { describe, expect, it } from 'vitest'
import { RecursiveChunker } from './recursive-chunker'
import { MAX_CHUNKING_SEPARATOR_LENGTH, MAX_CHUNKING_SEPARATORS } from '@/lib/chunkers/constants'
import { RecursiveChunker } from '@/lib/chunkers/recursive-chunker'

describe('RecursiveChunker', () => {
describe('empty and whitespace input', () => {
Expand Down Expand Up @@ -101,6 +102,63 @@ describe('RecursiveChunker', () => {
})
})

describe('separator bounds', () => {
it.concurrent('ignores separators past the list bound', async () => {
const separators = [
...Array.from({ length: MAX_CHUNKING_SEPARATORS }, (_, i) => `@@nomatch${i}@@`),
'---',
]
const chunker = new RecursiveChunker({ chunkSize: 15, separators })
const text =
'Section one content here with words.---Section two content here with words.---Section three content here.'

const chunks = await chunker.chunk(text)

expect(chunks.length).toBeGreaterThan(1)
expect(chunks.some((chunk) => chunk.text.includes('---'))).toBe(true)
})

it.concurrent('splits on a separator that survives the clamp', async () => {
const separators = [
'---',
...Array.from({ length: MAX_CHUNKING_SEPARATORS }, (_, i) => `@@nomatch${i}@@`),
]
const chunker = new RecursiveChunker({ chunkSize: 15, separators })
const text =
'Section one content here with words.---Section two content here with words.---Section three content here.'

const chunks = await chunker.chunk(text)

expect(chunks.length).toBeGreaterThan(1)
expect(chunks.every((chunk) => !chunk.text.includes('---'))).toBe(true)
})

it.concurrent('drops a separator longer than the per-item bound', async () => {
const oversized = '|'.repeat(MAX_CHUNKING_SEPARATOR_LENGTH + 1)
const chunker = new RecursiveChunker({ chunkSize: 15, separators: [oversized, '---'] })
const text = `Section one content here.${oversized}Section two content.---Section three content.`

const chunks = await chunker.chunk(text)

expect(chunks.some((chunk) => chunk.text.includes('|'))).toBe(true)
expect(chunks.every((chunk) => !chunk.text.includes('---'))).toBe(true)
})

it.concurrent('falls back to the recipe when every separator is over the bound', async () => {
const oversized = '|'.repeat(MAX_CHUNKING_SEPARATOR_LENGTH + 1)
const text =
'Section one content here with words.\n\nSection two content here with words.\n\nSection three content.'

const chunks = await new RecursiveChunker({ chunkSize: 15, separators: [oversized] }).chunk(
text
)
const defaultChunks = await new RecursiveChunker({ chunkSize: 15 }).chunk(text)

expect(chunks.length).toBeGreaterThan(1)
expect(chunks).toEqual(defaultChunks)
})
})

describe('recipe: plain', () => {
it.concurrent('should use plain recipe by default', async () => {
const chunker = new RecursiveChunker({ chunkSize: 20 })
Expand Down
Loading
Loading