Skip to content

Commit 9a1821a

Browse files
committed
merge: bring in latest staging parsing fixes
2 parents 93fc4d7 + c9945ef commit 9a1821a

9 files changed

Lines changed: 294 additions & 79 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/copilot/tools/server/files/doc-asset-extract.test.ts

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
import JSZip from 'jszip'
55
import { describe, expect, it } from 'vitest'
66
import { extractDocAssets } from '@/lib/copilot/tools/server/files/doc-asset-extract'
7+
import { MAX_OOXML_CENTRAL_DIRECTORY_RECORDS, ZipBombError } from '@/lib/file-parsers/ooxml-limits'
78

89
const THEME_XML = `<?xml version="1.0"?>
910
<a:theme xmlns:a="http://schemas.openxmlformats.org/drawingml/2006/main" name="Office">
@@ -143,6 +144,21 @@ describe('extractDocAssets', () => {
143144
expect(slide.texts.some((t) => t.text.includes('grouped'))).toBe(false)
144145
})
145146

147+
it('refuses an archive the OOXML guard rejects', async () => {
148+
// Every media entry is inflated into a retained Buffer with no cap of its
149+
// own, so the guard is the only thing bounding this. Tripping its
150+
// record-count ceiling asserts the call site is guarded without building a
151+
// multi-megabyte fixture; the size ceilings are covered in zip-guard.test.ts.
152+
const zip = new JSZip()
153+
for (let index = 0; index <= MAX_OOXML_CENTRAL_DIRECTORY_RECORDS; index++) {
154+
zip.file(`ppt/media/image${index}.png`, PNG_BYTES)
155+
}
156+
157+
await expect(
158+
extractDocAssets(await zip.generateAsync({ type: 'nodebuffer' }), 'pptx')
159+
).rejects.toThrow(ZipBombError)
160+
})
161+
146162
it('tolerates a package with no theme or media', async () => {
147163
const zip = new JSZip()
148164
zip.file('ppt/slides/slide1.xml', '<p:sld/>')

0 commit comments

Comments
 (0)