Skip to content

Commit 27ec27e

Browse files
committed
fix(embeddings): discount the batch ceiling when the tokenizer is foreign
Review round 4. Batching measures with tiktoken, which only has encodings for OpenAI models — every other id falls back to cl100k_base. Gemini's 2048, Cohere's 128k, and Mistral's 8192 were therefore enforced in OpenAI token units, so an input near one of those ceilings could still be rejected upstream or trimmed more than needed. A true fix needs per-provider tokenizers, which the repo does not have: estimateTokenCount is a chars-per-token heuristic, and truncation needs a real encode/decode pair to slice on a token boundary. So the ceiling is discounted for foreign tokenizers rather than trusted exactly. The discount is one-sided on purpose. Overshooting means the provider rejects the whole request; undershooting only trims a text that was already at the limit, so the margin errs toward the second. resolveBatchTokenCeiling is a pure function tested directly, rather than inferred from truncation behavior, so the guarantee holds per model as the catalog grows.
1 parent d7be0f9 commit 27ec27e

4 files changed

Lines changed: 65 additions & 1 deletion

File tree

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

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import {
99
getKbEligibleModels,
1010
getModelsForProvider,
1111
KB_EMBEDDING_DIMENSIONS,
12+
resolveBatchTokenCeiling,
1213
resolveDimensions,
1314
} from '@/lib/embeddings/catalog'
1415
import { EMBEDDING_MODEL_PRICING } from '@/providers/models'
@@ -100,6 +101,38 @@ describe('splitByItemLimit', () => {
100101
expect(batches.flat()).toEqual(items)
101102
})
102103

104+
/**
105+
* Batching counts tokens with tiktoken, which only has encodings for OpenAI
106+
* models. Every other id falls back to cl100k, so a foreign model's ceiling
107+
* would otherwise be enforced in the wrong token units.
108+
*/
109+
describe('resolveBatchTokenCeiling', () => {
110+
it('trusts the declared ceiling for tiktoken-native models', () => {
111+
for (const [id, info] of Object.entries(EMBEDDING_MODELS)) {
112+
if (info.tokenizerProvider !== 'openai') continue
113+
expect(resolveBatchTokenCeiling(info), id).toBe(info.maxInputTokens)
114+
}
115+
})
116+
117+
it('discounts the ceiling for every model counted with a foreign tokenizer', () => {
118+
const foreign = Object.entries(EMBEDDING_MODELS).filter(
119+
([, info]) => info.tokenizerProvider !== 'openai'
120+
)
121+
// Guards the test itself: the catalog must still contain such models.
122+
expect(foreign.length).toBeGreaterThan(0)
123+
124+
for (const [id, info] of foreign) {
125+
const ceiling = resolveBatchTokenCeiling(info)
126+
expect(ceiling, id).toBeLessThan(info.maxInputTokens)
127+
expect(ceiling, id).toBeGreaterThan(0)
128+
}
129+
})
130+
131+
it("leaves Gemini's tight ceiling below its declared 2048", () => {
132+
expect(resolveBatchTokenCeiling(getEmbeddingModelInfo('gemini-embedding-001'))).toBe(1638)
133+
})
134+
})
135+
103136
it("chunks to Cohere's 96-item cap", () => {
104137
const items = Array.from({ length: 200 }, (_, i) => i)
105138
const batches = splitByItemLimit(items, 96)

apps/sim/lib/embeddings/catalog.ts

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -157,6 +157,31 @@ export function getKbEligibleModels(): string[] {
157157
* Resolves the dimensionality a request will actually produce, given an
158158
* optional caller-requested reduction.
159159
*/
160+
/**
161+
* Fraction of a model's declared ceiling used for batching when its tokens are
162+
* counted with the wrong tokenizer. Chosen to absorb the usual spread between
163+
* BPE vocabularies on ordinary text; it is a guard, not a measurement.
164+
*/
165+
const FOREIGN_TOKENIZER_SAFETY_FACTOR = 0.8
166+
167+
/**
168+
* The token ceiling batching should enforce for a model.
169+
*
170+
* Batching measures with tiktoken, which only has encodings for OpenAI models —
171+
* every other id falls back to `cl100k_base`, so a Gemini, Cohere, or Mistral
172+
* limit would be enforced in OpenAI token units. Counting a foreign model's
173+
* text with tiktoken is an approximation, so its ceiling is discounted rather
174+
* than trusted exactly.
175+
*
176+
* The discount is deliberately one-sided: overshooting means the provider
177+
* rejects the whole request, while undershooting only trims a text that was
178+
* already at the limit.
179+
*/
180+
export function resolveBatchTokenCeiling(info: EmbeddingModelInfo): number {
181+
if (info.tokenizerProvider === 'openai') return info.maxInputTokens
182+
return Math.floor(info.maxInputTokens * FOREIGN_TOKENIZER_SAFETY_FACTOR)
183+
}
184+
160185
export function resolveDimensions(info: EmbeddingModelInfo, requested?: number): number {
161186
if (requested === undefined) return info.nativeDimensions
162187
if (!info.supportedDimensions?.includes(requested)) {

apps/sim/lib/embeddings/client.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import {
55
DEFAULT_EMBEDDING_MODEL,
66
type EmbeddingModelInfo,
77
getEmbeddingModelInfo,
8+
resolveBatchTokenCeiling,
89
resolveDimensions,
910
} from '@/lib/embeddings/catalog'
1011
import { resolveProviderKey } from '@/lib/embeddings/keys'
@@ -198,7 +199,11 @@ export async function embed(texts: string[], options: EmbedOptions): Promise<Emb
198199
* accepted. Using the per-input ceiling as the per-batch budget also keeps
199200
* every individual text within it.
200201
*/
201-
const tokenBatches = batchByTokenLimit(modelInputs, provider.info.maxInputTokens, model)
202+
const tokenBatches = batchByTokenLimit(
203+
modelInputs,
204+
resolveBatchTokenCeiling(provider.info),
205+
model
206+
)
202207
const itemLimit = provider.adapter.maxItemsPerRequest ?? provider.info.maxItemsPerRequest
203208
const batches = itemLimit
204209
? tokenBatches.flatMap((batch) => splitByItemLimit(batch, itemLimit))

apps/sim/lib/embeddings/index.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ export {
99
getKbEligibleModels,
1010
getModelsForProvider,
1111
KB_EMBEDDING_DIMENSIONS,
12+
resolveBatchTokenCeiling,
1213
resolveDimensions,
1314
} from '@/lib/embeddings/catalog'
1415
export { EmbeddingAPIError, embed } from '@/lib/embeddings/client'

0 commit comments

Comments
 (0)