Skip to content

Commit f4fb237

Browse files
authored
fix(knowledge): bound JSON/YAML chunker expansion (#7524)
* fix(knowledge): bound JSON/YAML chunker expansion JsonYamlChunker re-parsed and re-serialized document content with no expansion limit. ChunkBudget only counts emitted chunks, so every parse and full-object stringify ran before it could fire — a small aliased YAML source expands to tens of MB, and the same content was parsed twice because isStructuredData and chunkJsonYaml each parsed it. - Measure the parsed value with the shared measureYamlExpansion guard before anything materializes it, and skip parsing entirely when the source is already larger than the ceiling - Size the ceiling to the most text the chunker could ever emit (maxChunks x chunkSize), floored at 4MB and capped at what the YAML file parser itself permits, so documents that fit the budget chunk exactly as before - Replace isStructuredData + chunkJsonYaml with one chunkStructured entry point that parses once and returns null when the content is not structured, leaving chunker selection with the document processor * fix(knowledge): allow proportionate expansion in the chunker guard Comparing the expansion estimate against the output budget mixed two units: measureYamlExpansion charges a flat per-node allowance, so a document of many small values is charged several times its pretty-printed size and was rejected even though its chunks fit the budget — a flat array of a million booleans is charged ~22MB against ~8MB of real output. Allow the larger of two admissible expansions: one that fits the output budget, and one proportionate to the source. Alias expansion overshoots its source by orders of magnitude, so it is still rejected, while an ordinary large document is no longer charged for the estimator's conservatism. Neither allowance ever exceeds the file parser's own cap.
1 parent 178edba commit f4fb237

4 files changed

Lines changed: 234 additions & 63 deletions

File tree

apps/sim/lib/chunkers/json-yaml-chunker.test.ts

Lines changed: 94 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
*/
44

55
import { describe, expect, it, vi } from 'vitest'
6-
import { JsonYamlChunker } from './json-yaml-chunker'
6+
import { JsonYamlChunker } from '@/lib/chunkers/json-yaml-chunker'
77

88
vi.mock('@/lib/tokenization', () => ({
99
getAccurateTokenCount: (text: string) => Math.ceil(text.length / 4),
@@ -37,30 +37,110 @@ describe('JsonYamlChunker', () => {
3737
expect(chunks.every((chunk) => chunk.tokenCount <= 100)).toBe(true)
3838
})
3939

40-
describe('isStructuredData', () => {
41-
it('should detect valid JSON', () => {
42-
expect(JsonYamlChunker.isStructuredData('{"key": "value"}')).toBe(true)
40+
describe('chunkStructured', () => {
41+
it('chunks valid JSON', async () => {
42+
await expect(JsonYamlChunker.chunkStructured('{"key": "value"}')).resolves.not.toBeNull()
4343
})
4444

45-
it('should detect valid JSON array', () => {
46-
expect(JsonYamlChunker.isStructuredData('[1, 2, 3]')).toBe(true)
45+
it('chunks a valid JSON array', async () => {
46+
await expect(JsonYamlChunker.chunkStructured('[1, 2, 3]')).resolves.not.toBeNull()
4747
})
4848

49-
it('should detect valid YAML', () => {
50-
expect(JsonYamlChunker.isStructuredData('key: value\nother: data')).toBe(true)
49+
it('chunks valid YAML', async () => {
50+
await expect(
51+
JsonYamlChunker.chunkStructured('key: value\nother: data')
52+
).resolves.not.toBeNull()
5153
})
5254

53-
it('should return false for plain text parsed as YAML scalar', () => {
54-
expect(JsonYamlChunker.isStructuredData('Hello, this is plain text.')).toBe(false)
55+
it('declines plain text that parses as a YAML scalar', async () => {
56+
await expect(
57+
JsonYamlChunker.chunkStructured('Hello, this is plain text.')
58+
).resolves.toBeNull()
5559
})
5660

57-
it('should return false for invalid JSON/YAML with unbalanced braces', () => {
58-
expect(JsonYamlChunker.isStructuredData('{invalid: json: content: {{')).toBe(false)
61+
it('declines invalid JSON/YAML with unbalanced braces', async () => {
62+
await expect(
63+
JsonYamlChunker.chunkStructured('{invalid: json: content: {{')
64+
).resolves.toBeNull()
5965
})
6066

61-
it('should detect nested JSON objects', () => {
67+
it('chunks nested JSON objects', async () => {
6268
const nested = JSON.stringify({ level1: { level2: { level3: 'value' } } })
63-
expect(JsonYamlChunker.isStructuredData(nested)).toBe(true)
69+
await expect(JsonYamlChunker.chunkStructured(nested)).resolves.not.toBeNull()
70+
})
71+
72+
it('declines an alias-expansion bomb instead of expanding it', async () => {
73+
const lines = ['a0: &a0 "lol"']
74+
for (let level = 1; level <= 7; level++) {
75+
lines.push(
76+
`a${level}: &a${level} [${Array(7)
77+
.fill(`*a${level - 1}`)
78+
.join(',')}]`
79+
)
80+
}
81+
lines.push('top: *a7')
82+
const bomb = lines.join('\n')
83+
84+
const chunks = await JsonYamlChunker.chunkStructured(bomb, {
85+
chunkSize: 1024,
86+
minCharactersPerChunk: 1,
87+
maxChunks: 5000,
88+
})
89+
90+
expect(chunks).toBeNull()
91+
})
92+
93+
it('keeps structure for a many-small-node document that fits the budget', async () => {
94+
const flags = JSON.stringify(Array.from({ length: 250_000 }, (_, i) => i % 2 === 0))
95+
96+
const chunks = await JsonYamlChunker.chunkStructured(flags, {
97+
chunkSize: 1024,
98+
minCharactersPerChunk: 1,
99+
maxChunks: 1000,
100+
})
101+
102+
expect(chunks).not.toBeNull()
103+
expect(chunks?.length).toBeGreaterThan(1)
104+
expect(chunks?.[0].text).toContain('true')
105+
})
106+
107+
it('never parses source larger than one output budget', async () => {
108+
const oversized = JSON.stringify({ value: 'x'.repeat(5 * 1024 * 1024) })
109+
const parse = vi.spyOn(JSON, 'parse')
110+
111+
try {
112+
await expect(
113+
JsonYamlChunker.chunkStructured(oversized, {
114+
chunkSize: 1024,
115+
minCharactersPerChunk: 1,
116+
maxChunks: 1024,
117+
})
118+
).resolves.toBeNull()
119+
expect(parse).not.toHaveBeenCalled()
120+
} finally {
121+
parse.mockRestore()
122+
}
123+
})
124+
125+
it('chunks with default options', async () => {
126+
const chunks = await JsonYamlChunker.chunkStructured(JSON.stringify({ test: 'value' }))
127+
128+
expect(chunks?.length).toBeGreaterThan(0)
129+
})
130+
131+
it('honors a custom chunk size', async () => {
132+
const largeObject: Record<string, string> = {}
133+
for (let i = 0; i < 50; i++) {
134+
largeObject[`key${i}`] = `value${i}`.repeat(20)
135+
}
136+
const json = JSON.stringify(largeObject)
137+
138+
const chunksSmall = await JsonYamlChunker.chunkStructured(json, { chunkSize: 50 })
139+
const chunksLarge = await JsonYamlChunker.chunkStructured(json, { chunkSize: 500 })
140+
141+
expect(chunksSmall).not.toBeNull()
142+
expect(chunksLarge).not.toBeNull()
143+
expect(chunksSmall?.length).toBeGreaterThan(chunksLarge?.length as number)
64144
})
65145
})
66146

@@ -368,28 +448,6 @@ server:
368448
})
369449
})
370450

371-
describe('static chunkJsonYaml method', () => {
372-
it.concurrent('should work with default options', async () => {
373-
const json = JSON.stringify({ test: 'value' })
374-
const chunks = await JsonYamlChunker.chunkJsonYaml(json)
375-
376-
expect(chunks.length).toBeGreaterThan(0)
377-
})
378-
379-
it.concurrent('should accept custom options', async () => {
380-
const largeObject: Record<string, string> = {}
381-
for (let i = 0; i < 50; i++) {
382-
largeObject[`key${i}`] = `value${i}`.repeat(20)
383-
}
384-
const json = JSON.stringify(largeObject)
385-
386-
const chunksSmall = await JsonYamlChunker.chunkJsonYaml(json, { chunkSize: 50 })
387-
const chunksLarge = await JsonYamlChunker.chunkJsonYaml(json, { chunkSize: 500 })
388-
389-
expect(chunksSmall.length).toBeGreaterThan(chunksLarge.length)
390-
})
391-
})
392-
393451
describe('chunk metadata', () => {
394452
it('preserves every source character and offset when bounding oversized chunks', async () => {
395453
const key = 'p'.repeat(80)

apps/sim/lib/chunkers/json-yaml-chunker.ts

Lines changed: 129 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@ import {
99
normalizeTokenChunkSize,
1010
tokensToChars,
1111
} from '@/lib/chunkers/utils'
12+
import { measureYamlExpansion, type YamlExpansionLimits } from '@/lib/file-parsers/yaml-limits'
13+
import { FILE_PARSER_YAML_LIMITS } from '@/lib/file-parsers/yaml-parser'
1214

1315
const logger = createLogger('JsonYamlChunker')
1416

@@ -20,40 +22,148 @@ type BoundedChunkMetadataMode = 'text-offsets' | 'preserve-range'
2022

2123
const MAX_DEPTH = 5
2224

25+
/**
26+
* Smallest source ceiling this chunker imposes, so a knowledge base configured
27+
* with tiny chunks keeps structural chunking on documents it indexes perfectly
28+
* well today.
29+
*/
30+
const MIN_SOURCE_BYTES = 4 * 1024 * 1024
31+
32+
/**
33+
* How far a document may legitimately expand past its own source.
34+
*
35+
* `measureYamlExpansion` charges a flat per-node allowance, so a compact source
36+
* of small values is charged well above its own length — `[1,1,1]` costs about
37+
* 22 estimated bytes per element against two in source. An order of magnitude of
38+
* headroom therefore covers ordinary document shape, while alias expansion
39+
* overshoots it by several orders.
40+
*/
41+
const MAX_EXPANSION_RATIO = 16
42+
43+
/**
44+
* Longest source this chunker will parse: the most text it could ever emit, one
45+
* output budget's worth. A larger document cannot be indexed whole by any
46+
* chunker — `ChunkBudget` stops it either way — so parsing it buys nothing.
47+
*/
48+
function resolveMaxSourceBytes(maxChunks: number | undefined, chunkSize: number): number {
49+
if (maxChunks === undefined) return FILE_PARSER_YAML_LIMITS.maxSerializedBytes
50+
51+
return Math.min(
52+
FILE_PARSER_YAML_LIMITS.maxSerializedBytes,
53+
Math.max(MIN_SOURCE_BYTES, maxChunks * tokensToChars(chunkSize))
54+
)
55+
}
56+
57+
/**
58+
* What the document is allowed to expand to once it is walked as a tree.
59+
*
60+
* Structural chunking re-serializes what it parsed, so its cost follows the
61+
* document's *expanded* size rather than its source size, and `yaml.load`
62+
* resolves aliases into shared references — a sub-kilobyte source can carry tens
63+
* of megabytes of expansion. `ChunkBudget` cannot bound that: it counts emitted
64+
* chunks, and every parse and serialization happens before the first is emitted.
65+
*
66+
* Two expansions are admissible: one that stays within the output budget, and
67+
* one that stays proportionate to the source. Taking the larger of the two keeps
68+
* transient allocation tied to work the chunker would have done anyway, without
69+
* charging an ordinary large document for the estimator's per-node conservatism.
70+
* Neither is ever allowed past what the file parser itself would hand over.
71+
*/
72+
function resolveExpansionLimits(sourceBytes: number, maxSourceBytes: number): YamlExpansionLimits {
73+
return {
74+
/** Bytes bind here; every reached node charges some, so a self-referential anchor still terminates. */
75+
maxNodes: Number.MAX_SAFE_INTEGER,
76+
maxSerializedBytes: Math.min(
77+
FILE_PARSER_YAML_LIMITS.maxSerializedBytes,
78+
Math.max(maxSourceBytes, sourceBytes * MAX_EXPANSION_RATIO)
79+
),
80+
maxDepth: FILE_PARSER_YAML_LIMITS.maxDepth,
81+
}
82+
}
83+
2384
export class JsonYamlChunker {
2485
private chunkSize: number
2586
private minCharactersPerChunk: number
2687
private maxChunks?: number
88+
private readonly maxSourceBytes: number
2789

2890
constructor(options: ChunkerOptions = {}) {
2991
this.chunkSize = normalizeTokenChunkSize(options.chunkSize ?? 1024, 'JSON/YAML chunk size')
3092
this.minCharactersPerChunk = options.minCharactersPerChunk ?? 100
3193
this.maxChunks = options.maxChunks
94+
this.maxSourceBytes = resolveMaxSourceBytes(this.maxChunks, this.chunkSize)
3295
}
3396

34-
static isStructuredData(content: string): boolean {
97+
/**
98+
* Read `content` as JSON, falling back to YAML, and measure what the parsed
99+
* value expands to before anything materializes it.
100+
*
101+
* The source-length check comes first so oversized content is never parsed at
102+
* all; the expansion measurement then catches what length alone cannot — alias
103+
* expansion, and the indentation a pretty-printed re-serialization adds.
104+
*/
105+
private parseWithinLimits(content: string): JsonValue | undefined {
106+
if (content.length > this.maxSourceBytes) {
107+
return this.reject(
108+
`source of ${content.length} characters exceeds the ${this.maxSourceBytes}-byte ceiling`
109+
)
110+
}
111+
112+
let parsed: unknown
35113
try {
36-
const parsed = JSON.parse(content)
37-
return typeof parsed === 'object' && parsed !== null
114+
parsed = JSON.parse(content)
38115
} catch {
39116
try {
40-
const parsed = yaml.load(content)
41-
return typeof parsed === 'object' && parsed !== null
117+
parsed = yaml.load(content)
42118
} catch {
43-
return false
119+
return undefined
44120
}
45121
}
122+
123+
if (parsed === undefined) return undefined
124+
125+
const limits = resolveExpansionLimits(content.length, this.maxSourceBytes)
126+
const measured = measureYamlExpansion(parsed, limits)
127+
if (!measured.within) return this.reject(measured.reason)
128+
129+
return parsed as JsonValue
46130
}
47131

48-
async chunk(content: string): Promise<Chunk[]> {
49-
try {
50-
let data: JsonValue
51-
try {
52-
data = JSON.parse(content) as JsonValue
53-
} catch {
54-
data = yaml.load(content) as JsonValue
132+
private reject(reason: string): undefined {
133+
logger.warn(
134+
'Structured content exceeds the chunking expansion limits, declining to expand it',
135+
{
136+
reason,
55137
}
138+
)
139+
return undefined
140+
}
141+
142+
/**
143+
* Chunk `content` as a structured object or array, or return `null` when it is
144+
* neither — including when its expanded form outgrows the ceiling above. The
145+
* caller then chooses another chunker for it.
146+
*/
147+
static async chunkStructured(
148+
content: string,
149+
options: ChunkerOptions = {}
150+
): Promise<Chunk[] | null> {
151+
const chunker = new JsonYamlChunker(options)
152+
const data = chunker.parseWithinLimits(content)
153+
if (data === null || typeof data !== 'object') return null
154+
155+
return chunker.chunkParsed(data, content)
156+
}
157+
158+
async chunk(content: string): Promise<Chunk[]> {
159+
const data = this.parseWithinLimits(content)
160+
if (data === undefined) return this.chunkAsText(content)
161+
162+
return this.chunkParsed(data, content)
163+
}
56164

165+
private chunkParsed(data: JsonValue, content: string): Chunk[] {
166+
try {
57167
const chunks: Chunk[] = []
58168
this.chunkStructuredData(data, [], 0, chunks, new ChunkBudget(this.maxChunks))
59169

@@ -64,7 +174,7 @@ export class JsonYamlChunker {
64174
} catch (error) {
65175
if (error instanceof ChunkLimitExceededError) throw error
66176
logger.info('Structured data chunking failed, falling back to text chunking')
67-
return this.chunkAsText(content, new ChunkBudget(this.maxChunks))
177+
return this.chunkAsText(content)
68178
}
69179
}
70180

@@ -299,7 +409,11 @@ export class JsonYamlChunker {
299409
}
300410
}
301411

302-
private chunkAsText(content: string, budget: ChunkBudget, chunks: Chunk[] = []): Chunk[] {
412+
private chunkAsText(
413+
content: string,
414+
budget: ChunkBudget = new ChunkBudget(this.maxChunks),
415+
chunks: Chunk[] = []
416+
): Chunk[] {
303417
let currentChunk = ''
304418
let currentTokens = 0
305419
let startIndex = 0
@@ -362,9 +476,4 @@ export class JsonYamlChunker {
362476

363477
return chunks
364478
}
365-
366-
static async chunkJsonYaml(content: string, options: ChunkerOptions = {}): Promise<Chunk[]> {
367-
const chunker = new JsonYamlChunker(options)
368-
return chunker.chunk(content)
369-
}
370479
}

apps/sim/lib/file-parsers/yaml-parser.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ import { measureYamlExpansion, type YamlExpansionLimits } from '@/lib/file-parse
1010
* the byte cap bounds output a sub-1 KB input can inflate to hundreds of MB;
1111
* the depth cap bounds the traversal's own working set.
1212
*/
13-
const FILE_PARSER_YAML_LIMITS: YamlExpansionLimits = {
13+
export const FILE_PARSER_YAML_LIMITS: YamlExpansionLimits = {
1414
maxNodes: 5_000_000,
1515
maxSerializedBytes: 64 * 1024 * 1024,
1616
maxDepth: 500,

apps/sim/lib/knowledge/documents/document-processor.ts

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -268,13 +268,17 @@ export async function processDocument(
268268
mimeType.includes('json') ||
269269
mimeType.includes('yaml')
270270

271-
if (isJsonYaml && JsonYamlChunker.isStructuredData(content)) {
271+
const jsonYamlChunks = isJsonYaml
272+
? await JsonYamlChunker.chunkStructured(content, {
273+
chunkSize,
274+
minCharactersPerChunk,
275+
maxChunks: MAX_DOCUMENT_CHUNKS,
276+
})
277+
: null
278+
279+
if (jsonYamlChunks !== null) {
272280
logger.info('Using JSON/YAML chunker for structured data')
273-
chunks = await JsonYamlChunker.chunkJsonYaml(content, {
274-
chunkSize,
275-
minCharactersPerChunk,
276-
maxChunks: MAX_DOCUMENT_CHUNKS,
277-
})
281+
chunks = jsonYamlChunks
278282
} else if (StructuredDataChunker.isStructuredData(content, mimeType)) {
279283
logger.info('Using structured data chunker for spreadsheet/CSV content')
280284
const rowCount = metadata.totalRows ?? metadata.rowCount

0 commit comments

Comments
 (0)