Skip to content

fix: Batch vector upserts + background timeout budget (Fixes BATTLE-MAGE-5) - #144

Merged
vlad-ko merged 2 commits into
mainfrom
fix/vector-upsert-timeout
Jul 9, 2026
Merged

fix: Batch vector upserts + background timeout budget (Fixes BATTLE-MAGE-5)#144
vlad-ko merged 2 commits into
mainfrom
fix/vector-upsert-timeout

Conversation

@vlad-ko

@vlad-ko vlad-ko commented Jul 9, 2026

Copy link
Copy Markdown
Owner

Summary

Second production finding from the code index's first real run (Sentry BATTLE-MAGE-5, 3 occurrences — one per cron tick). Different class from BATTLE-MAGE-4: not a misconfiguration but a timeout budget sized for the wrong workload.

VECTOR_OP_TIMEOUT_MS (2s) is right for interactive paths — recall must never stall a turn — but it also governed the background embed pipelines, where a single upsert carries an entire file's chunks (up to ~130) for server-side embedding. Big files therefore timed out on every 5-minute tick: the index permanently stalled on exactly the files most worth indexing, and since the SDK exposes no cancellation (established in #134's review), each timed-out request likely completed server-side anyway — burning embedding spend per retry while never recording success. The docs pipeline carried the same bug latently (whole corpus in one fire-and-forget call).

Fix

  • Batching inside vectorUpsert: at most UPSERT_BATCH_SIZE (20) items per underlying store call, sequential, stop on first failure. Deterministic chunk ids make retries of already-written batches idempotent, so mid-list failure semantics stay safe for both the manifest (code index) and the pointer swap (docs).
  • Per-call timeout override: background pipelines pass VECTOR_BACKGROUND_TIMEOUT_MS (30s — absorbed by the ticks' existing 180s wall-clock budgets); interactive paths and KB saves keep the 2s default. VectorTimeoutError now reports the actual budget it enforced.

Testing

TDD: 6 new tests confirmed RED first — batch splitting (20/20/5), stop-on-first-failed-batch, at-size single call, override survives past the default budget and fails past its own, default still enforced when unset, constants pinned. Call-site assertions updated to include the background option. Full suite 951 passing, typecheck clean. No cassette impact (vector env is force-unset in behavior evals).

Fixes BATTLE-MAGE-5 (Sentry)

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • Background vector indexing now uses a longer timeout, making large content updates more reliable.
    • Vector uploads are now processed in smaller batches to better handle larger workloads.
  • Bug Fixes

    • Improved timeout handling so long-running vector operations fail more predictably.
    • Reduced the chance of indexing jobs stopping early due to default timeout limits.

…AGE-5)

The 2s VECTOR_OP_TIMEOUT_MS was sized for interactive paths (KB recall,
search arms inside a turn) but also governed the background embed
pipelines, where one upsert carries a whole file's chunks (up to ~130)
for server-side embedding — legitimately seconds of work. Large files
timed out on every 5-minute tick, permanently stalling the code index
on exactly the files most worth indexing, while the un-abortable
request often succeeded server-side anyway (wasted embedding spend on
each retry). The docs pipeline had the same bug latent: it upserts an
entire corpus in one call, fire-and-forget.

- vectorUpsert now batches at UPSERT_BATCH_SIZE (20) with a per-batch
  timeout, stopping on the first failed batch; deterministic ids make
  the caller's retry of already-written batches idempotent
- Per-call timeout override: background pipelines (code index, docs
  embed) pass VECTOR_BACKGROUND_TIMEOUT_MS (30s), absorbed by the
  ticks' existing wall-clock budgets; interactive paths keep 2s

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@vercel

vercel Bot commented Jul 9, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
battle-mage Ready Ready Preview, Comment Jul 9, 2026 3:25pm

Request Review

@coderabbitai

coderabbitai Bot commented Jul 9, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 69a187d2-93e8-4e12-b24c-bd494a0a6c21

📥 Commits

Reviewing files that changed from the base of the PR and between 8e8ee91 and e3bc18b.

📒 Files selected for processing (6)
  • src/lib/code-index.test.ts
  • src/lib/code-index.ts
  • src/lib/repo-index.test.ts
  • src/lib/repo-index.ts
  • src/lib/vector.test.ts
  • src/lib/vector.ts

📝 Walkthrough

Walkthrough

Adds batched, deadline-aware upserting to vectorUpsert with a configurable per-call timeoutMs, extends VectorTimeoutError/withTimeout accordingly, introduces VECTOR_BACKGROUND_TIMEOUT_MS and UPSERT_BATCH_SIZE constants, and updates code-index.ts/repo-index.ts callers and tests to use the new background timeout.

Changes

Vector Upsert Timeout and Batching

Layer / File(s) Summary
Batched upsert and configurable timeout core logic
src/lib/vector.ts, src/lib/vector.test.ts
VectorTimeoutError now carries timeoutMs, withTimeout accepts an explicit timeout, and vectorUpsert batches items via new UPSERT_BATCH_SIZE/VECTOR_BACKGROUND_TIMEOUT_MS constants against a single end-to-end deadline; new tests cover batching, failure handling, and timeout overrides.
code-index.ts background timeout wiring
src/lib/code-index.ts, src/lib/code-index.test.ts
runCodeIndexTick passes { timeoutMs: VECTOR_BACKGROUND_TIMEOUT_MS } to vectorUpsert; tests mock ./vector via importOriginal and assert the new option.
repo-index.ts background timeout wiring
src/lib/repo-index.ts, src/lib/repo-index.test.ts
embedDocChunks passes { timeoutMs: VECTOR_BACKGROUND_TIMEOUT_MS } to vectorUpsert; tests mock ./vector via importOriginal and assert the new option in the SHA-changed scenario.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Caller
  participant vectorUpsert
  participant withTimeout
  participant Store

  Caller->>vectorUpsert: vectorUpsert(namespace, items, opts.timeoutMs)
  vectorUpsert->>vectorUpsert: compute end-to-end deadline
  loop each batch of UPSERT_BATCH_SIZE
    vectorUpsert->>vectorUpsert: check remaining time
    vectorUpsert->>withTimeout: race(store.upsert(batch), remainingMs)
    withTimeout->>Store: upsert(batch)
    Store-->>withTimeout: result or timeout
    withTimeout-->>vectorUpsert: success or VectorTimeoutError
  end
  vectorUpsert-->>Caller: true/false
Loading

Possibly related PRs

  • vlad-ko/battle-mage#134: Both PRs modify src/lib/vector.ts's timeout/error and vectorUpsert behavior, with this PR extending the earlier wrapper with batching and per-call timeoutMs.
  • vlad-ko/battle-mage#138: This PR updates runCodeIndexTick to call vectorUpsert with timeoutMs: VECTOR_BACKGROUND_TIMEOUT_MS, directly building on the tick logic introduced in that PR.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly captures the main change: batched vector upserts and a separate background timeout budget.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/vector-upsert-timeout

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Fix background vector upserts with batching + per-call timeout override

🐞 Bug fix 🧪 Tests 🕐 40+ Minutes

Grey Divider

AI Description

• Batch large vector upserts to avoid timeouts on server-side embedding workloads.
• Add per-call timeout override so background pipelines use a larger budget than interactive paths.
• Extend tests to pin constants and verify batching, failure semantics, and timeout behavior.
Diagram

graph TD
  CI["Code index tick"] --> VU["vectorUpsert()"] --> WT["withTimeout(timeoutMs)"] --> VS{{"Upstash Vector"}}
  DE["Docs embed pipeline"] --> VU
  VU --> OB["vector_op / vector_error logs"]
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Dynamic timeout based on item count / estimated work
  • ➕ Scales budget with workload without introducing a second fixed constant
  • ➕ Can reduce worst-case cron latency vs always using 30s
  • ➖ Hard to calibrate reliably without store-side telemetry
  • ➖ Still needs batching to avoid single-request failure domains
2. Parallel batch upserts
  • ➕ Reduces wall-clock time for large files/corpora
  • ➕ Better utilization if the store can handle concurrency
  • ➖ Higher risk of rate limiting / bursty embedding spend
  • ➖ More complex failure semantics (partial completion, ordering, backoff)
3. Store-level cancellation / abort (if SDK supports it later)
  • ➕ Avoids wasted server-side work after client timeout
  • ➕ Cleaner resource usage under retries
  • ➖ Not currently available in the SDK per prior review context
  • ➖ Doesn’t address per-request latency unless combined with batching

Recommendation: Keep the PR’s approach: sequential batching + explicit background timeout override. It minimizes behavioral surface area (no concurrency), preserves existing non-throwing degradation semantics, and directly addresses the production failure mode (large upserts timing out under an interactive budget) while keeping interactive paths fast by default.

Files changed (6) +135 / -9

Bug fix (3) +44 / -6
code-index.tsUse background timeout for code-index vector upserts +5/-0

Use background timeout for code-index vector upserts

• Imports VECTOR_BACKGROUND_TIMEOUT_MS and passes it as a per-call timeout override to vectorUpsert for the code indexing pipeline to avoid stalling on large files.

src/lib/code-index.ts

repo-index.tsUse background timeout for docs embedding upserts +4/-1

Use background timeout for docs embedding upserts

• Imports VECTOR_BACKGROUND_TIMEOUT_MS and applies it to the docs corpus upsert call, decoupling background embedding from the interactive 2s cap.

src/lib/repo-index.ts

vector.tsBatch vectorUpsert and add per-call timeout override support +35/-5

Batch vectorUpsert and add per-call timeout override support

• Introduces VECTOR_BACKGROUND_TIMEOUT_MS and UPSERT_BATCH_SIZE, parameterizes VectorTimeoutError/withTimeout with the active timeout budget, and updates vectorUpsert to upsert sequential batches (logging total count and batch count) while honoring an optional timeoutMs override.

src/lib/vector.ts

Tests (3) +91 / -3
code-index.test.tsAssert code index uses background vector timeout +3/-0

Assert code index uses background vector timeout

• Updates the vector module mock to expose VECTOR_BACKGROUND_TIMEOUT_MS and asserts runCodeIndexTick calls vectorUpsert with the background timeout option.

src/lib/code-index.test.ts

repo-index.test.tsAssert docs embedding uses background vector timeout +7/-3

Assert docs embedding uses background vector timeout

• Extends the vector mock with VECTOR_BACKGROUND_TIMEOUT_MS and updates expectations so the docs embedding hook passes the background timeout override to vectorUpsert.

src/lib/repo-index.test.ts

vector.test.tsAdd tests for upsert batching and timeout override +81/-0

Add tests for upsert batching and timeout override

• Adds a dedicated test block covering UPSERT_BATCH_SIZE splitting, stop-on-first-failed-batch behavior, constant pinning, and per-call timeout override vs default timeout.

src/lib/vector.test.ts

Comment thread src/lib/repo-index.test.ts
Comment thread src/lib/vector.ts
…onstant flow in tests

- timeoutMs is now a per-CALL deadline: each batch races the remaining
  budget, so a multi-batch upsert can never exceed the caller's cap
  (previously each batch got a fresh budget — batches × 30s could
  overrun the tick's wall-clock). Deadline test pins 3×12s batches
  failing at a 30s cap.
- Test assertions reference VECTOR_BACKGROUND_TIMEOUT_MS instead of a
  raw 30_000 literal; the vector mocks now spread importOriginal so
  real constants flow through and can't drift from production.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@vlad-ko
vlad-ko merged commit 920ee59 into main Jul 9, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant