Skip to content

Commit 73e12df

Browse files
committed
fix(embeddings): keep the batch ceiling exact and warn before truncating
Review round 5. Reverts the safety margin from round 4. The two review findings were in direct tension: round 4 flagged that a foreign model's ceiling is measured in tiktoken units, and the margin added to absorb that error reintroduced the round 3 harm — valid content truncated below the provider's declared limit. The margin was the wrong trade. It swapped a loud failure for a silent one: an undercount surfaces as a provider rejection the caller can see and act on, while shortening an embedding's input produces a degraded vector that is indistinguishable from a good one at every layer above it. Silent quality loss in a retrieval index is the worse outcome, and it is also the harder one to ever notice. So the declared ceiling is applied exactly, and truncation is no longer silent: an input above the limit now logs a warning naming the model, the limit, and whether the count was approximate. hasApproximateTokenCount records which models are counted with a foreign tokenizer without being used to shrink anything. The tokenizer imprecision itself remains, and cannot be fixed without per-provider BPE 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.
1 parent 4da0625 commit 73e12df

4 files changed

Lines changed: 51 additions & 63 deletions

File tree

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

Lines changed: 21 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,8 @@ import {
88
getEmbeddingModelInfo,
99
getKbEligibleModels,
1010
getModelsForProvider,
11+
hasApproximateTokenCount,
1112
KB_EMBEDDING_DIMENSIONS,
12-
resolveBatchTokenCeiling,
1313
resolveDimensions,
1414
} from '@/lib/embeddings/catalog'
1515
import { EMBEDDING_MODEL_PRICING } from '@/providers/models'
@@ -101,42 +101,30 @@ describe('splitByItemLimit', () => {
101101
expect(batches.flat()).toEqual(items)
102102
})
103103

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-
136104
it("chunks to Cohere's 96-item cap", () => {
137105
const items = Array.from({ length: 200 }, (_, i) => i)
138106
const batches = splitByItemLimit(items, 96)
139107
expect(batches.map((b) => b.length)).toEqual([96, 96, 8])
140108
expect(batches.flat()).toEqual(items)
141109
})
142110
})
111+
112+
/**
113+
* Batching counts tokens with tiktoken, which only has encodings for OpenAI
114+
* models; every other id falls back to cl100k. This flag records which models
115+
* are counted approximately. It must not be used to shrink the ceiling —
116+
* truncating below a provider's declared limit drops valid content silently,
117+
* which is strictly worse than the visible rejection it would guard against.
118+
*/
119+
describe('hasApproximateTokenCount', () => {
120+
it('is false only for tiktoken-native models', () => {
121+
for (const [id, info] of Object.entries(EMBEDDING_MODELS)) {
122+
expect(hasApproximateTokenCount(info), id).toBe(info.tokenizerProvider !== 'openai')
123+
}
124+
})
125+
126+
it('covers at least one model, so the flag stays meaningful', () => {
127+
const approximate = Object.values(EMBEDDING_MODELS).filter(hasApproximateTokenCount)
128+
expect(approximate.length).toBeGreaterThan(0)
129+
})
130+
})

apps/sim/lib/embeddings/catalog.ts

Lines changed: 8 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -158,28 +158,18 @@ export function getKbEligibleModels(): string[] {
158158
* optional caller-requested reduction.
159159
*/
160160
/**
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.
161+
* True when a model's tokens cannot be counted exactly.
169162
*
170163
* Batching measures with tiktoken, which only has encodings for OpenAI models —
171164
* 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.
165+
* ceiling is enforced in approximate units. The ceiling is still applied
166+
* exactly as declared: discounting it to absorb the error would truncate valid
167+
* content silently, which is worse than the alternative it guards against. An
168+
* undercount surfaces as a provider rejection, which is visible and
169+
* actionable; silently shortening an embedding's input is not.
179170
*/
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)
171+
export function hasApproximateTokenCount(info: EmbeddingModelInfo): boolean {
172+
return info.tokenizerProvider !== 'openai'
183173
}
184174

185175
export function resolveDimensions(info: EmbeddingModelInfo, requested?: number): number {

apps/sim/lib/embeddings/client.ts

Lines changed: 21 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import {
55
DEFAULT_EMBEDDING_MODEL,
66
type EmbeddingModelInfo,
77
getEmbeddingModelInfo,
8-
resolveBatchTokenCeiling,
8+
hasApproximateTokenCount,
99
resolveDimensions,
1010
} from '@/lib/embeddings/catalog'
1111
import { resolveProviderKey } from '@/lib/embeddings/keys'
@@ -193,17 +193,27 @@ export async function embed(texts: string[], options: EmbedOptions): Promise<Emb
193193
const modelInputs = options.projectInputs ? options.projectInputs(texts) : texts
194194

195195
/**
196-
* Batched against the selected model's own ceiling rather than one shared
197-
* constant. A value that is too high sends oversized input the provider
198-
* rejects; one that is too low silently drops content the provider would have
199-
* accepted. Using the per-input ceiling as the per-batch budget also keeps
200-
* every individual text within it.
196+
* Batched against the selected model's own declared ceiling, exactly as
197+
* declared. One shared constant sent oversized input to models with a lower
198+
* limit and discarded content models with a higher one accept; discounting
199+
* the ceiling to absorb tokenizer error would reintroduce the second harm.
200+
*
201+
* `batchByTokenLimit` truncates any text above the ceiling, and does so
202+
* silently, so warn first: a shortened embedding input is otherwise
203+
* indistinguishable from a good one, both to the caller and in the vector.
201204
*/
202-
const tokenBatches = batchByTokenLimit(
203-
modelInputs,
204-
resolveBatchTokenCeiling(provider.info),
205-
model
206-
)
205+
const ceiling = provider.info.maxInputTokens
206+
for (const text of modelInputs) {
207+
if (estimateTokenCount(text, provider.info.tokenizerProvider).count <= ceiling) continue
208+
logger.warn('Embedding input exceeds the model token limit and will be truncated', {
209+
model,
210+
maxInputTokens: ceiling,
211+
chars: text.length,
212+
approximateTokenCount: hasApproximateTokenCount(provider.info),
213+
})
214+
}
215+
216+
const tokenBatches = batchByTokenLimit(modelInputs, ceiling, model)
207217
const itemLimit = provider.adapter.maxItemsPerRequest ?? provider.info.maxItemsPerRequest
208218
const batches = itemLimit
209219
? tokenBatches.flatMap((batch) => splitByItemLimit(batch, itemLimit))

apps/sim/lib/embeddings/index.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,8 @@ export {
88
getEmbeddingModelInfo,
99
getKbEligibleModels,
1010
getModelsForProvider,
11+
hasApproximateTokenCount,
1112
KB_EMBEDDING_DIMENSIONS,
12-
resolveBatchTokenCeiling,
1313
resolveDimensions,
1414
} from '@/lib/embeddings/catalog'
1515
export { EmbeddingAPIError, embed } from '@/lib/embeddings/client'

0 commit comments

Comments
 (0)