Skip to content

feat(rag): make the RAG embed batch size configurable (#4327) - #4350

Merged
JSONbored merged 1 commit into
mainfrom
claude/gittensory-orb-migration-ce6321
Jul 9, 2026
Merged

feat(rag): make the RAG embed batch size configurable (#4327)#4350
JSONbored merged 1 commit into
mainfrom
claude/gittensory-orb-migration-ce6321

Conversation

@JSONbored

Copy link
Copy Markdown
Owner

Summary

  • EMBED_BATCH (items per RAG embed-provider call) was a hardcoded 96 in src/review/rag.ts with no env override, unlike its sibling RAG_DIMENSIONS/QDRANT_DIM. Adds AI_EMBED_BATCH, threaded through RagInfra the exact same way embeddingDimensions already is, so self-host operators can tune throughput for their own hardware without a code change.
  • No default change. Benchmarked the actual embedding path on an RTX A5000 (real chunk text, matching the production batch size) as part of GPU passthrough for Ollama + embedding benchmark vs. the current CPU-bound setup #4327: 96 is already near-optimal (~34ms/chunk vs ~70ms/chunk at 32; only ~3% further gain going all the way up to 384). This PR is the tuning-knob half of that issue's recommendation, not a default change.

Part of #4327.

Scope

  • The PR title follows type(scope): short summary Conventional Commit format.
  • This PR is focused and does not mix unrelated backend, UI, MCP, docs, dependency, and deploy changes.
  • This follows CONTRIBUTING.md and does not reintroduce GitHub Pages, VitePress, site/, or CNAME.
  • Part of GPU passthrough for Ollama + embedding benchmark vs. the current CPU-bound setup #4327 (maintainer-owned infra/GPU-migration epic tracking issue; this PR does the code-side follow-up recommended there).

Validation

  • git diff --check
  • npm run actionlint (via npm run test:ci)
  • npm run typecheck (via npm run test:ci)
  • npm run test:coverage locally — rag.ts 100% branch coverage (174/174) including the new code; adapters.ts's one pre-existing partial branch (line 43) is unrelated code this PR does not touch
  • npm run test:workers (via npm run test:ci)
  • npm run build:mcp (via npm run test:ci)
  • npm run test:mcp-pack (via npm run test:ci)
  • npm run ui:openapi:check (via npm run test:ci) — no API/OpenAPI surface touched
  • npm run ui:lint (via npm run test:ci) — no UI files touched
  • npm run ui:typecheck (via npm run test:ci)
  • npm run ui:build (via npm run test:ci)
  • npm audit --audit-level=moderate — 0 vulnerabilities
  • New behavior (the embedBatch override) has unit tests covering both the ragEmbedBatchFromEnv parser branches and the plumbing through embedTexts, upsertChunks, retrieveContext, and createReviewAdapters

Ran npm run test:ci twice end-to-end (full unsharded suite) to confirm green; one unrelated pre-existing flaky test (queue.test.ts's #3899 concurrency-bound regression, nothing to do with RAG/embeddings) failed on the first run under heavy background load and passed both in isolation and on a clean re-run.

Safety

  • No secrets, wallet details, hotkeys, coldkeys, user PATs, private keys, raw trust scores, private rankings, or private maintainer evidence are exposed.
  • Public GitHub text stays sanitized, low-noise, and does not imply compensation guarantees or optimization tactics.
  • Auth, cookie, CORS, GitHub App, Cloudflare, or session changes include negative-path tests. — N/A, no such surface touched.
  • API/OpenAPI/MCP behavior is updated and tested where needed. — N/A, no API/OpenAPI/MCP surface touched.
  • UI changes use live API data or real empty/error/loading states. — N/A, backend-only change.
  • Visible UI changes include a UI Evidence section. — N/A, no UI change.
  • Public docs/changelogs are updated where needed (.env.example documents the new var); changelog itself not touched (not a release-prep PR).

Notes

  • No migration, OpenAPI, or cf-typegen regeneration needed — AI_EMBED_BATCH is a plain env.d.ts field read directly in src/review/adapters.ts, outside the scanned roots for selfhost:env-reference (same treatment as the existing QDRANT_DIM, which is separately documented where it's read inside src/selfhost/**).

EMBED_BATCH was a hardcoded 96 with no override, unlike its sibling
RAG_DIMENSIONS/QDRANT_DIM. Self-host operators tuning Ollama throughput
on their own hardware (e.g. GPU-accelerated) had no way to adjust it
without a code change. Adds AI_EMBED_BATCH, threaded through RagInfra
the same way embeddingDimensions already is.

Benchmarked on an RTX A5000: 96 is already near-optimal (~34ms/chunk
vs ~70ms/chunk at 32, only ~3% further gain up to 384) -- no default
change, this just adds the tuning knob for different hardware/models.
@superagent-security

Copy link
Copy Markdown
Contributor

Superagent didn't find any vulnerabilities or security issues in this PR.

@JSONbored
JSONbored merged commit 7ec1d53 into main Jul 9, 2026
7 checks passed
@JSONbored
JSONbored deleted the claude/gittensory-orb-migration-ce6321 branch July 9, 2026 02:31
@codecov

codecov Bot commented Jul 9, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.95%. Comparing base (4d0aef1) to head (e22b95d).
⚠️ Report is 1 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #4350   +/-   ##
=======================================
  Coverage   93.95%   93.95%           
=======================================
  Files         399      399           
  Lines       36782    36786    +4     
  Branches    13436    13440    +4     
=======================================
+ Hits        34557    34561    +4     
  Misses       1569     1569           
  Partials      656      656           
Files with missing lines Coverage Δ
src/review/adapters.ts 94.44% <100.00%> (+0.32%) ⬆️
src/review/rag.ts 100.00% <100.00%> (ø)
🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

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