Skip to content

Commit 8ef98ea

Browse files
waleedlatif1claude
andcommitted
refactor(search): derive the bulk admission lane from one flag
A single bulk flag now selects the shorter admission wait and the capped lane, the lane cap is a pure function of configuration, and a bulk batch larger than that cap is rejected up front instead of resizing a shared bucket. Tests use the shared env mock. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
1 parent 2370b87 commit 8ef98ea

4 files changed

Lines changed: 43 additions & 63 deletions

File tree

apps/sim/lib/core/rate-limiter/provider-admission.test.ts

Lines changed: 24 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,13 @@
11
/**
22
* @vitest-environment node
33
*/
4+
import { resetEnvMock, setEnv } from '@sim/testing/mocks/env.mock'
45
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
56

6-
const { consumeTokens, getCooldownUntil, setCooldownUntil, mockEnv } = vi.hoisted(() => ({
7+
const { consumeTokens, getCooldownUntil, setCooldownUntil } = vi.hoisted(() => ({
78
consumeTokens: vi.fn(),
89
getCooldownUntil: vi.fn(),
910
setCooldownUntil: vi.fn(),
10-
mockEnv: {} as Record<string, string | undefined>,
11-
}))
12-
vi.mock('@/lib/core/config/env', () => ({
13-
env: mockEnv,
14-
envNumber: (value: string | undefined, fallback: number) =>
15-
value === undefined ? fallback : Number(value),
1611
}))
1712
vi.mock('@/lib/core/rate-limiter/storage/factory', () => ({
1813
createStorageAdapter: () => ({
@@ -41,7 +36,10 @@ describe('provider admission', () => {
4136
consumeTokens.mockResolvedValue({ allowed: true, tokensRemaining: 1, resetAt: new Date() })
4237
})
4338

44-
afterEach(() => vi.useRealTimers())
39+
afterEach(() => {
40+
vi.useRealTimers()
41+
resetEnvMock()
42+
})
4543

4644
it('shares both credential dimensions in one reservation across concurrent callers', async () => {
4745
await Promise.all([waitForProviderAdmission(INPUT), waitForProviderAdmission(INPUT)])
@@ -92,15 +90,11 @@ describe('provider admission', () => {
9290
)
9391
})
9492

95-
it('caps the bulk lane below the aggregate budget so interactive callers keep headroom', async () => {
96-
await waitForProviderAdmission({ ...INPUT, lane: 'bulk' })
97-
await waitForProviderAdmission({ ...INPUT, lane: 'interactive' })
98-
const [bulkReservations, bulkOptions] = consumeTokens.mock.calls[0]
99-
expect(bulkReservations).toMatchObject([
100-
{
101-
key: 'provider:embedding:openai:hashed-credential:tokens',
102-
config: { maxTokens: 600_000, refillRate: 10_000 },
103-
},
93+
it('caps bulk work below the aggregate budget so interactive callers keep headroom', async () => {
94+
await waitForProviderAdmission({ ...INPUT, bulk: true })
95+
const [reservations, options] = consumeTokens.mock.calls[0]
96+
expect(reservations).toMatchObject([
97+
{ key: 'provider:embedding:openai:hashed-credential:tokens', config: { maxTokens: 600_000 } },
10498
{ key: 'provider:embedding:openai:hashed-credential:requests', config: { maxTokens: 64 } },
10599
{
106100
key: 'provider:embedding:openai:hashed-credential:bulk:tokens',
@@ -112,31 +106,24 @@ describe('provider admission', () => {
112106
config: { maxTokens: 57, refillRate: 9 },
113107
},
114108
])
115-
expect(bulkOptions.cooldownKeys).toEqual([
109+
expect(options.cooldownKeys).toEqual([
116110
'provider:embedding:openai:hashed-credential:cooldown',
117111
'provider:embedding:openai:hashed-credential:quota',
118112
])
119-
const [interactiveReservations, interactiveOptions] = consumeTokens.mock.calls[1]
120-
expect(interactiveReservations.map((item: { key: string }) => item.key)).toEqual([
121-
'provider:embedding:openai:hashed-credential:tokens',
122-
'provider:embedding:openai:hashed-credential:requests',
123-
])
124-
expect(interactiveOptions.cooldownKeys).toEqual(bulkOptions.cooldownKeys)
125113
})
126114

127-
it('never shrinks a bulk bucket below one valid reservation', async () => {
128-
mockEnv.KB_CONFIG_EMBEDDING_REQUESTS_PER_MINUTE = '1'
129-
mockEnv.KB_CONFIG_EMBEDDING_TOKENS_PER_MINUTE = '100'
130-
try {
131-
await waitForProviderAdmission({ ...INPUT, inputTokens: 95, lane: 'bulk' })
132-
} finally {
133-
mockEnv.KB_CONFIG_EMBEDDING_REQUESTS_PER_MINUTE = undefined
134-
mockEnv.KB_CONFIG_EMBEDDING_TOKENS_PER_MINUTE = undefined
135-
}
136-
expect(consumeTokens.mock.calls[0][0]).toMatchObject([
137-
{ key: 'provider:embedding:openai:hashed-credential:tokens', config: { maxTokens: 100 } },
138-
{ key: 'provider:embedding:openai:hashed-credential:requests', config: { maxTokens: 1 } },
139-
{ key: 'provider:embedding:openai:hashed-credential:bulk:tokens', config: { maxTokens: 95 } },
115+
it('rejects a bulk batch the lane can never hold and keeps one request slot at a minimal burst', async () => {
116+
setEnv({
117+
KB_CONFIG_EMBEDDING_REQUESTS_PER_MINUTE: '1',
118+
KB_CONFIG_EMBEDDING_TOKENS_PER_MINUTE: '100',
119+
})
120+
await expect(
121+
waitForProviderAdmission({ ...INPUT, inputTokens: 95, bulk: true })
122+
).rejects.toThrow('exceeds the configured per-credential token budget')
123+
await waitForProviderAdmission({ ...INPUT, inputTokens: 95 })
124+
await waitForProviderAdmission({ ...INPUT, inputTokens: 90, bulk: true })
125+
expect(consumeTokens.mock.calls[1][0].slice(2)).toMatchObject([
126+
{ key: 'provider:embedding:openai:hashed-credential:bulk:tokens', config: { maxTokens: 90 } },
140127
{
141128
key: 'provider:embedding:openai:hashed-credential:bulk:requests',
142129
config: { maxTokens: 1 },

apps/sim/lib/core/rate-limiter/provider-admission.ts

Lines changed: 11 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -13,22 +13,19 @@ export interface ProviderIdentity {
1313
}
1414

1515
/**
16-
* Every caller reserves from the credential's aggregate buckets, so the
17-
* configured budget is never exceeded. The bulk lane also reserves from a
18-
* bucket capped at {@link BULK_LANE_SHARE} of that budget, which leaves an
19-
* interactive caller headroom instead of a queue behind a crawl's batches.
20-
* Cooldown and quota gates stay per identity: a provider pause or an exhausted
21-
* balance still stops every lane.
16+
* Share of a credential's budget the bulk lane may use. Every caller reserves
17+
* from the aggregate buckets, so the budget is never exceeded; bulk callers
18+
* also reserve from buckets capped at this share, which leaves an interactive
19+
* caller headroom instead of a queue behind a crawl's batches.
2220
*/
23-
export type ProviderAdmissionLane = 'bulk' | 'interactive'
24-
2521
const BULK_LANE_SHARE = 0.9
2622

2723
interface ProviderAdmissionInput extends ProviderIdentity {
2824
inputTokens?: number
2925
signal?: AbortSignal
3026
maxWaitMs: number
31-
lane?: ProviderAdmissionLane
27+
/** Bulk work is capped at {@link BULK_LANE_SHARE}; cooldown and quota gates still stop every caller. */
28+
bulk?: boolean
3229
}
3330

3431
/**
@@ -75,22 +72,22 @@ export async function waitForProviderAdmission(input: ProviderAdmissionInput): P
7572
perMinute: envNumber(env.KB_CONFIG_EMBEDDING_TOKENS_PER_MINUTE, 600_000, { min: 1 }),
7673
}
7774
: undefined
78-
if (tokenBudget && tokenBudget.cost > tokenBudget.perMinute) {
75+
const laneShare = input.bulk ? BULK_LANE_SHARE : 1
76+
if (tokenBudget && tokenBudget.cost > Math.floor(tokenBudget.perMinute * laneShare)) {
7977
throw new Error('Embedding request exceeds the configured per-credential token budget')
8078
}
8179
const requestBurst = Math.min(
8280
input.operation === 'embedding' ? EMBEDDING_REQUEST_BURST : DEFAULT_REQUEST_BURST,
8381
requestsPerMinute
8482
)
8583
const reservations: TokenBucketReservation[] = []
86-
/** A lane bucket always holds at least one valid reservation, so a tiny budget cannot lock the lane. */
8784
const reserveBuckets = (bucketKey: string, share: number) => {
8885
if (tokenBudget) {
8986
reservations.push({
9087
key: `${bucketKey}:tokens`,
9188
cost: tokenBudget.cost,
9289
config: {
93-
maxTokens: Math.max(tokenBudget.cost, Math.floor(tokenBudget.perMinute * share)),
90+
maxTokens: Math.floor(tokenBudget.perMinute * share),
9491
refillRate: (tokenBudget.perMinute * share) / 60,
9592
refillIntervalMs: 1000,
9693
},
@@ -100,14 +97,15 @@ export async function waitForProviderAdmission(input: ProviderAdmissionInput): P
10097
key: `${bucketKey}:requests`,
10198
cost: 1,
10299
config: {
100+
/** A burst of one leaves no share to carve out, so the lane then matches the aggregate. */
103101
maxTokens: Math.max(1, Math.floor(requestBurst * share)),
104102
refillRate: (requestsPerMinute * share) / 60,
105103
refillIntervalMs: 1000,
106104
},
107105
})
108106
}
109107
reserveBuckets(key, 1)
110-
if (input.lane === 'bulk') reserveBuckets(`${key}:bulk`, BULK_LANE_SHARE)
108+
if (input.bulk) reserveBuckets(`${key}:bulk`, BULK_LANE_SHARE)
111109

112110
/** When the bucket last said capacity returns, so a deadline hit after a sleep reports the wait still left. */
113111
let capacityAvailableAt: number | undefined

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1811,10 +1811,10 @@ describe('durable embedding batches', () => {
18111811
fetchMock.mockImplementation(() => Promise.resolve(jsonResponse(openAIBody([[1]], 7))))
18121812
await embed(['text'], { apiKey: 'fixture-key', checkpoints: memoryCheckpoints() })
18131813
expect(mockAdmit).toHaveBeenLastCalledWith(
1814-
expect.objectContaining({ maxWaitMs: KNOWLEDGE_EMBEDDING_ADMISSION_WAIT_MS, lane: 'bulk' })
1814+
expect.objectContaining({ maxWaitMs: KNOWLEDGE_EMBEDDING_ADMISSION_WAIT_MS, bulk: true })
18151815
)
18161816
await embed(['text'], { apiKey: 'fixture-key' })
1817-
expect(mockAdmit).toHaveBeenLastCalledWith(expect.objectContaining({ lane: 'interactive' }))
1817+
expect(mockAdmit).toHaveBeenLastCalledWith(expect.objectContaining({ bulk: false }))
18181818
expect(mockAdmit.mock.lastCall?.[0].maxWaitMs).toBeGreaterThan(
18191819
KNOWLEDGE_EMBEDDING_ADMISSION_WAIT_MS
18201820
)

apps/sim/lib/embeddings/client.ts

Lines changed: 6 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@ import {
1111
} from '@/lib/core/config/env-capabilities'
1212
import { isHosted } from '@/lib/core/config/env-flags'
1313
import {
14-
type ProviderAdmissionLane,
1514
ProviderQuotaExhaustedError,
1615
recordProviderCooldown,
1716
waitForProviderAdmission,
@@ -545,9 +544,10 @@ async function callEmbeddingAPI(
545544
expectedDimensions: number | undefined,
546545
isBYOK: boolean,
547546
signal?: AbortSignal,
548-
admissionWaitMs = EMBEDDING_RETRY_BUDGET_MS,
549-
lane?: ProviderAdmissionLane
547+
/** Bulk indexing waits briefly and is capped below the credential budget; everything else has a person waiting on it. */
548+
bulk = false
550549
): Promise<{ embeddings: number[][]; totalTokens: number; dimensions: number }> {
550+
const admissionWaitMs = bulk ? KNOWLEDGE_EMBEDDING_ADMISSION_WAIT_MS : EMBEDDING_RETRY_BUDGET_MS
551551
const admissionIdentity = embeddingAdmissionIdentity({ providerId, quotaCircuitIdentity, isBYOK })
552552
return retryWithExponentialBackoff(
553553
async (operationSignal, deadlineAt) => {
@@ -565,7 +565,7 @@ async function callEmbeddingAPI(
565565
),
566566
signal: operationSignal,
567567
maxWaitMs: Math.min(admissionWaitMs, Math.max(0, deadlineAt - Date.now())),
568-
lane,
568+
bulk,
569569
})
570570
} catch (error) {
571571
if (error instanceof ProviderQuotaExhaustedError)
@@ -798,6 +798,7 @@ async function mapEmbeddingBatches<T, R>(
798798
return results.map((result) => result!.value)
799799
}
800800

801+
/** Checkpoints mark the bulk indexing path; every other caller is interactive. */
801802
async function callCheckpointedEmbeddingBatch(
802803
batch: string[],
803804
batchIndex: number,
@@ -851,13 +852,7 @@ async function callCheckpointedEmbeddingBatch(
851852
provider.dimensions,
852853
provider.isBYOK,
853854
signal,
854-
checkpoints ? KNOWLEDGE_EMBEDDING_ADMISSION_WAIT_MS : undefined,
855-
/**
856-
* Checkpoints mark the bulk indexing path, which may never take the whole
857-
* credential budget. Everything else has a person waiting on it and uses
858-
* the headroom the bulk lane leaves.
859-
*/
860-
checkpoints ? 'bulk' : 'interactive'
855+
checkpoints !== undefined
861856
)
862857
if (identity) await checkpoints!.save(identity, result, signal)
863858
return result

0 commit comments

Comments
 (0)