Skip to content

Commit 8b57c78

Browse files
committed
fix(embeddings): honor per-model token limits and bound the JSON input path
Review round 1. Batching used one 8,000-token constant for every model, inherited from the knowledge-base engine this branch extracted. `batchByTokenLimit` truncates any single text above the limit it is given, so that constant both sent oversized input to models with a lower ceiling and silently dropped content models with a higher one accept: - Gemini declares 2,048, so a 3,000-token text passed through whole and the provider rejected it, surfacing as a 502. This also affected knowledge-base indexing on staging, which uses the same constant. - Cohere declares 128,000, so anything past 8,000 was truncated for no reason. Batch against the selected model's own `maxInputTokens` instead. Using the per-input ceiling as the per-batch budget also keeps every individual text within it. The contract bounds the array arm of `input`, but a JSON-encoded array arrives as a plain string and `normalizeInput` only expands it after validation — so neither the 1,000-input cap nor the non-empty checks applied to the reference-expression path the route was written to accept. `"[]"` also reported success with no vectors. Re-check the normalized list so the bounds hold for both shapes.
1 parent 8c0dfee commit 8b57c78

4 files changed

Lines changed: 114 additions & 5 deletions

File tree

apps/sim/app/api/tools/embeddings/route.test.ts

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -129,6 +129,46 @@ describe('POST /api/tools/embeddings', () => {
129129
expect(mockEmbed).toHaveBeenCalledWith(['alpha', 'beta'], expect.anything())
130130
})
131131

132+
/**
133+
* The contract bounds the array arm, but a JSON-encoded array reaches the
134+
* route as a plain string and is only expanded after validation — so the
135+
* bounds have to be re-applied to the normalized list or they hold for a
136+
* native array body only.
137+
*/
138+
describe('JSON-encoded array bounds', () => {
139+
it('rejects a JSON array that exceeds the input count limit', async () => {
140+
const many = JSON.stringify(Array.from({ length: 1001 }, (_, i) => `t${i}`))
141+
const response = await post({ ...baseBody, input: many })
142+
143+
expect(response.status).toBe(400)
144+
expect((await response.json()).error).toContain('cannot exceed 1000 texts')
145+
expect(mockEmbed).not.toHaveBeenCalled()
146+
})
147+
148+
it('rejects an empty JSON array instead of reporting success with no vectors', async () => {
149+
const response = await post({ ...baseBody, input: '[]' })
150+
151+
expect(response.status).toBe(400)
152+
expect((await response.json()).error).toContain('at least one text')
153+
expect(mockEmbed).not.toHaveBeenCalled()
154+
})
155+
156+
it('rejects a JSON array containing a blank entry', async () => {
157+
const response = await post({ ...baseBody, input: '["ok"," "]' })
158+
159+
expect(response.status).toBe(400)
160+
expect((await response.json()).error).toContain('entries cannot be empty')
161+
expect(mockEmbed).not.toHaveBeenCalled()
162+
})
163+
164+
it('accepts a JSON array within the bounds', async () => {
165+
const response = await post({ ...baseBody, input: '["alpha","beta"]' })
166+
167+
expect(response.status).toBe(200)
168+
expect(mockEmbed).toHaveBeenCalledWith(['alpha', 'beta'], expect.anything())
169+
})
170+
})
171+
132172
it('embeds a non-JSON string as a single text', async () => {
133173
await post({ ...baseBody, input: 'just a sentence' })
134174
expect(mockEmbed).toHaveBeenCalledWith(['just a sentence'], expect.anything())

apps/sim/app/api/tools/embeddings/route.ts

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import { getErrorMessage } from '@sim/utils/errors'
33
import { type NextRequest, NextResponse } from 'next/server'
44
import {
55
embeddingsToolContract,
6+
MAX_EMBEDDING_INPUTS,
67
MAX_EMBEDDING_TOTAL_CHARS,
78
} from '@/lib/api/contracts/tools/embeddings'
89
import { getValidationErrorMessage, parseRequest, validationErrorResponse } from '@/lib/api/server'
@@ -64,6 +65,35 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
6465
const { provider, apiKey, model, input, taskType, dimensions } = parsed.data.body
6566

6667
const texts = normalizeInput(input)
68+
69+
/**
70+
* The contract bounds the array arm, but a JSON-encoded array arrives as a
71+
* plain string and is only expanded here, after validation. Re-checking the
72+
* normalized list is what makes the bounds hold for the reference-expression
73+
* path too, rather than only for a native array body.
74+
*/
75+
if (texts.length === 0) {
76+
return NextResponse.json(
77+
{ success: false, error: 'input must contain at least one text' },
78+
{ status: 400 }
79+
)
80+
}
81+
if (texts.length > MAX_EMBEDDING_INPUTS) {
82+
return NextResponse.json(
83+
{
84+
success: false,
85+
error: `input cannot exceed ${MAX_EMBEDDING_INPUTS} texts, received ${texts.length}`,
86+
},
87+
{ status: 400 }
88+
)
89+
}
90+
if (texts.some((text) => text.trim().length === 0)) {
91+
return NextResponse.json(
92+
{ success: false, error: 'input entries cannot be empty' },
93+
{ status: 400 }
94+
)
95+
}
96+
6797
const totalChars = texts.reduce((sum, text) => sum + text.length, 0)
6898
if (totalChars > MAX_EMBEDDING_TOTAL_CHARS) {
6999
return NextResponse.json(

apps/sim/lib/embeddings/client.test.ts

Lines changed: 35 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -201,6 +201,40 @@ describe('embed', () => {
201201
expect(result.isBYOK).toBe(true)
202202
})
203203

204+
/**
205+
* `batchByTokenLimit` truncates any single text above the limit it is given,
206+
* so the limit has to be the selected model's own. One shared constant sent
207+
* oversized input to the models with a lower ceiling and silently dropped
208+
* content the models with a higher one would have accepted.
209+
*/
210+
describe('per-model token limits', () => {
211+
it("truncates against Gemini's lower ceiling rather than a shared constant", async () => {
212+
fetchMock.mockResolvedValue(jsonResponse({ embeddings: [{ values: [1] }] }))
213+
// ~10k tokens: over Gemini's 2048 ceiling, but under the old 8000 constant,
214+
// so this used to reach the provider whole and come back a 502.
215+
const long = 'word '.repeat(8000)
216+
217+
await embed([long], { model: 'gemini-embedding-001', apiKey: 'g-test' })
218+
219+
const body = JSON.parse((fetchMock.mock.calls[0][1] as RequestInit).body as string)
220+
const sent = body.requests[0].content.parts[0].text
221+
expect(sent.length).toBeLessThan(long.length)
222+
})
223+
224+
it("keeps text intact up to Cohere's much higher ceiling", async () => {
225+
fetchMock.mockResolvedValue(
226+
jsonResponse({ embeddings: { float: [[1]] }, meta: { billed_units: { input_tokens: 9 } } })
227+
)
228+
// Over the old 8000 constant, well under Cohere's 128k, so it must survive.
229+
const long = 'word '.repeat(8000)
230+
231+
await embed([long], { model: 'embed-v4.0', apiKey: 'c-test' })
232+
233+
const body = JSON.parse((fetchMock.mock.calls[0][1] as RequestInit).body as string)
234+
expect(body.texts[0]).toBe(long)
235+
})
236+
})
237+
204238
/**
205239
* The knowledge-base path rewrites resolved-secret plaintext back to
206240
* placeholders before inputs reach a provider. The block path projects
@@ -237,9 +271,7 @@ describe('embed', () => {
237271

238272
it('estimates tokens from the projected values, not the originals', async () => {
239273
// Gemini omits usage, so the token count is estimated from what was sent.
240-
fetchMock.mockResolvedValue(
241-
jsonResponse({ embeddings: [{ values: [1, 2, 3] }] })
242-
)
274+
fetchMock.mockResolvedValue(jsonResponse({ embeddings: [{ values: [1, 2, 3] }] }))
243275

244276
const result = await embed(['x'.repeat(400)], {
245277
model: 'gemini-embedding-001',

apps/sim/lib/embeddings/client.ts

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@ import { batchByTokenLimit, estimateTokenCount } from '@/lib/tokenization'
2020

2121
const logger = createLogger('EmbeddingClient')
2222

23-
const MAX_TOKENS_PER_REQUEST = 8000
2423
const MAX_CONCURRENT_BATCHES = envNumber(env.KB_CONFIG_CONCURRENCY_LIMIT, 50)
2524
const EMBEDDING_REQUEST_TIMEOUT_MS = 60_000
2625

@@ -185,7 +184,15 @@ export async function embed(texts: string[], options: EmbedOptions): Promise<Emb
185184
const taskType = options.taskType ?? 'document'
186185
const provider = await resolveProvider(model, options)
187186

188-
const tokenBatches = batchByTokenLimit(texts, MAX_TOKENS_PER_REQUEST, model)
187+
/**
188+
* Batched against the selected model's own ceiling rather than one shared
189+
* constant. `batchByTokenLimit` truncates any single text above the limit, so
190+
* a value that is too high sends oversized input the provider rejects, and one
191+
* that is too low silently drops content the provider would have accepted.
192+
* Using the per-input ceiling as the per-batch budget also keeps every
193+
* individual text within it.
194+
*/
195+
const tokenBatches = batchByTokenLimit(texts, provider.info.maxInputTokens, model)
189196
const itemLimit = provider.adapter.maxItemsPerRequest ?? provider.info.maxItemsPerRequest
190197
const batches = itemLimit
191198
? tokenBatches.flatMap((batch) => splitByItemLimit(batch, itemLimit))

0 commit comments

Comments
 (0)