Skip to content

fix(code_index): one embeddings call per batch, fail on the first error - #6975

Merged
kovtcharov-amd merged 2 commits into
amd:mainfrom
ityashag:fix/code-index-one-call-per-batch
Nov 9, 2026
Merged

fix(code_index): one embeddings call per batch, fail on the first error#6975
kovtcharov-amd merged 2 commits into
amd:mainfrom
ityashag:fix/code-index-one-call-per-batch

Conversation

@ityashag

@ityashag ityashag commented Nov 8, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Summary

With Lemonade down or the embedding model name wrong, code indexing now fails on its first call and says what to do. Before, it retried every batch with sleeps, then re-sent every chunk on its own, so a large repo took many minutes to report anything, and chunks that failed one at a time were dropped with only a log line.

Why

CLAUDE.md allows a retry only when the caller asks for one, and RAG already fails this way (its twin issue was #4798). Code indexing kept the old path: three tries per batch with 2 s sleeps, then one 60 s call per chunk, for every batch of 25.

Linked issue

Closes #4832

Changes

  • One embeddings call per batch. A failed call raises a RuntimeError that names the model and how to load it, and a batch with any missing or empty vector raises instead of being partly kept.
  • Removes the one-chunk-at-a-time fallback.
  • Keeps one wait, the same as RAG's: a batch that comes back with no vectors at all and no error (the model is still loading) gets two more tries, 2 s apart.
  • search() goes through the same path, so it also fails at once instead of after retries.
  • The C++ parity plan said to port the old retry and fallback; it now describes this behavior.
  • The MCP server doc's rebuild_index row promised a warning listing chunks that failed to embed. That can't happen anymore, so it now says the rebuild fails with the embedding error (found in review).

Decisions to check

  • The loading wait stays. Lemonade answers with no vectors while the embedder is still loading, and RAG keeps the same wait for that reason. It is bounded at 3 tries and never applies to an error or a short batch.
  • A lost chunk now fails the index instead of being dropped and counted in chunks_dropped. The un-hashing of incomplete files and the chunks_dropped field stay as a guard, with a comment saying so, so the agent tool's and MCP server's results keep their shape. Removing chunks_dropped end to end changes the MCP response, so I'd rather do that in its own PR if you want it.

Test plan

Run in a python:3.12-slim container with .[ui] and the test tools installed:

  • All code-index tests (tests/unit/test_code_index*.py, test_flagship_code_index_scope.py): 215 passed, 1 skipped.
  • python util/lint.py --all: passes, with the existing mypy warning count.
  • pytest tests/unit -n 3: 34,763 passed and 35 failed under load. Rerun on their own, 19 still fail, and the same 19 fail with main's sdk.py in my container. None fail only with this change.

Two tests asserted the old fallback and drop. They now assert the new behavior, and three tests are new. With the source change reverted, all five fail:

FAILED test_code_index_sdk.py::TestEmbeddingBatchSync::test_a_short_batch_raises_and_no_chunk_is_sent_alone
FAILED test_code_index_sdk.py::TestEmbeddingBatchSync::test_an_empty_vector_in_a_full_batch_raises
FAILED test_code_index_sdk.py::TestBatchEmbeddingRace::test_a_batch_still_loading_after_its_waits_raises
FAILED test_code_index_sdk.py::TestBatchEmbeddingRace::test_a_dead_backend_fails_indexing_on_its_first_call
FAILED test_code_index_sdk.py::TestAbandonedBuild::test_an_embedding_pass_stops_between_batches
5 failed

The last one only renames the method it patches.

Evidence

A 60-file repo indexed with a real LemonadeClient pointed at a closed port (127.0.0.1:9), sleeps counted rather than waited:

Embedding calls Single-chunk calls Sleeps Result
main 69 60 6 (12 s) "Indexing aborted: all embedding calls failed for 60 chunks…"
This PR 1 0 0 "Embedding a batch of 25 chunk(s) with 'user.embeddinggemma-300m-GGUF' failed: … isn't answering. Start the model server, then send the message again. Check that Lemonade Server is reachable and that 'user.embeddinggemma-300m-GGUF' is loaded.", plus how to load it

A refused connection fails fast, so both runs took under a second here. Against a backend that hangs instead, the same calls on main add up to about 87 minutes for these 60 files (9 × 180 s + 60 × 60 s + 12 s), against at most 180 s with this PR. That figure comes from the timeouts, not from a run.

  • Agent exposed in the Agent UI: N/A
  • MCP tools / servers: N/A
  • CLI: N/A
  • HTTP API / REST: N/A

Checklist

  • I have linked a GitHub issue above (Closes #N / Fixes #N / Refs #N).
  • I have described why this change is being made, not just what changed.
  • I have run linting and tests locally (python util/lint.py --all, pytest tests/unit/). The code-index tests above, and the full tests/unit run below.
  • I have attached real-world evidence matched to the surface I changed (see Evidence above), or marked each surface N/A.
  • I have updated documentation if user-visible behavior changed (see CONTRIBUTING.md). Only the C++ parity plan described the retries; it is updated.

@ityashag
fix(code_index): one embeddings call per batch, and fail on the first…
1b0b779 
… error

With the embedding backend down or the model name wrong, indexing retried
every batch with sleeps, then re-sent each chunk on its own with a 60 s
timeout, and dropped the chunks that failed with only a log line. A failed
call or a short batch now raises at once with the model name and how to load
it, as RAG already does. The one wait kept is RAG's: a batch that comes
back with no vectors while the model is still loading gets two more tries.
The C++ parity plan, which said to port the old retry and fallback, now
describes this behavior.

Closes amd#4832
@github-actions github-actions Bot added documentation Documentation changes tests Test changes labels Nov 8, 2026
@github-actions

github-actions Bot commented Nov 8, 2026

Copy link
Copy Markdown
Contributor

Verdict: Approve with suggestions

When Lemonade is down or the embedding model name is wrong, code indexing now fails on its first call and says how to fix it. Before, it spent minutes retrying and re-sending chunks one at a time, then quietly dropped the ones that still failed. The logic is correct and the tests cover the new behaviour well. RAG already fails this way, and this change matches it.

  • One doc line is now wrong. The MCP server reference still says rebuild_index returns a warning listing the chunks that failed to embed. A failed chunk now fails the whole rebuild, so update that line in this PR.
  • The old "partial index" handling can no longer run. The code that counted dropped chunks and left their files un-hashed is still there, and the agent tool and MCP server still check a count that is now always zero. You could remove it now or in a follow-up. It isn't blocking.

Real-world evidence

N/A: the evidence harness wrote no bundle for this PR. The PR description has the author's own run: a 60-file repo indexed against a closed port made 1 embedding call and 0 sleeps on this branch, against 69 calls, 60 single-chunk calls and 6 sleeps on main, and the error names the model and how to load it. No CLI, REST or UI surface changes. This verdict rests on static review plus that author-supplied run. I could not run the tests here because pytest isn't installed on this runner.

🔍 Technical details

Issues

🟡 Stale doc claim (docs/sdk/infrastructure/mcp.mdx:139): it says rebuild_index returns "chunks_dropped with a warning when some chunks failed to embed". With this PR, _encode_texts_with_sync raises on the first short or empty batch, so chunks_dropped is always 0 and that warning never appears. The rebuild fails with the embedding error instead.

| `rebuild_index` | `max_files` (default 5000), `model` (Lemonade embedding model) | `files_indexed`, `chunks_created`, `duration_seconds`; fails with the embedding error (model name and how to load it) if any batch can't be embedded |

🟢 Code that can no longer run: _encode_texts_with_sync now either returns every chunk or raises, which leaves three things behind:

  • index_repository still has the "Indexing aborted: all embedding calls failed" block (sdk.py:437-449). It can only fire for chunks that were never embedded.
  • The incomplete / chunks_dropped un-hash block (sdk.py:455-472) is unreachable for the same reason.
  • Downstream code still reads the count: code_index_tools.py:97-101, code_index_mcp.py:144-147, and eval/code_retrieval/{quality,scale}.py.

Leaving it costs nothing at runtime, but a reader will assume partial indexes still happen. Either remove chunks_dropped end to end, which is an MCP/SDK field change and wants a CHANGELOG line, or add a one-line comment saying it is kept for compatibility.

🟢 raise AssertionError("unreachable") (sdk.py:~959): this is correct, because the last attempt always reaches the short-batch check. Building the loop so the final attempt falls through, with no sentinel, would read cleaner. Optional.

Checks run

  • Sibling paths: search() → _encode_texts, and both index_repository embed calls (new and missed chunks), all go through _embed_batch, so nothing still uses the old retry. TestAbandonedBuild patches the renamed method.
  • Known bug classes: the error names what failed, the model, and the remedy, using describe_client_hint rather than a hard-coded command. The wait while the model loads is bounded at 3 tries, 2 s apart, the same as RAG. No silent fallback is left.
  • Doc drift: the C++ parity plan is updated. The only stale doc claim is the mcp.mdx line above.

Strengths

  • Removes a retry the caller never asked for, plus the one-chunk-at-a-time fallback, as CLAUDE.md requires. The error is raised with from e and names the model and the remedy.
  • The tests assert call counts and sleeps, not just the outcome. The dead-backend test runs index_repository end to end, and the author showed that all five tests fail with the source change reverted.
  • Treating "no vectors at all" (model still loading) differently from "short batch" (broken) is the right line to draw, and matches RAG.

@ityashag
fix(code_index): keep the embedding error readable and the MCP doc cu…
0367799 
…rrent

From review. The wrapped error no longer doubles a period when the cause
ends in one, the batch loop has no unreachable sentinel, and the MCP doc's
rebuild_index row no longer promises a dropped-chunks warning that can't
happen. The dropped-chunk block is now marked as a guard.
@kovtcharov-amd
kovtcharov-amd added this pull request to the merge queue Nov 9, 2026
Merged via the queue into amd:main with commit f978581 Nov 9, 2026
33 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@kovtcharov-amd kovtcharov-amd Awaiting requested review from kovtcharov-amd kovtcharov-amd is a code owner

Assignees

@ityashag ityashag

Labels

documentation Documentation changes tests Test changes

Projects

None yet

Milestone

v0.26.0 — Fast, focused flagship

Development

Successfully merging this pull request may close these issues.

Code index retries failed embeddings on its own, then falls back to one call per chunk

2 participants

@ityashag @kovtcharov-amd