Repository navigation
fix(embedder): retry once inside the provider context window on exceed-context - #6439
Open
Batianxieshen1 wants to merge 1 commit into
Open
Batianxieshen1 wants to merge 1 commit into
Batianxieshen1 wants to merge 1 commit into
Conversation
…eed-context The token guard in truncate_embedding_input estimates tokens from character counts. For structured text (URIs, snake_case identifiers, markdown bullets) a BPE tokenizer produces up to ~3x more tokens than chars/4, so the guard lets requests through that the provider then rejects with exceed_context_size_error — directory overview embeddings fail every time while the memory items they summarize succeed. When the provider reports the window it enforces (llama-server returns n_ctx), re-truncate the batch to that window and retry exactly once. Nothing else changes: providers that do not report a window, multimodal inputs, and rejections where the window would not shrink the text all keep their original error, so a retry is never a no-op resend. Refs volcengine#6418
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
truncate_embedding_inputbounds embedding input by estimating tokens aschars / 4(with CJK runs costing 4 units each). For structured text a real BPE tokenizer produces up to ~3x more tokens than that estimate, so the guard lets requests through that the provider then rejects withexceed_context_size_error. The reporter measured it on a real directory overview:.overview.md(28,152 chars)Result: directory-level overview embeddings fail every time with HTTP 400, so the affected write is reported as
failedeven though the memory items it summarizes landed. The current workaround is loweringembedding.max_input_tokensglobally, which also truncates legitimate long memory items.Refs #6418. This implements the fail-safe option (option 4) rather than changing the global heuristic — the estimate is still used as a cheap pre-check, but the provider now gets the last word when it disagrees.
What changed
When a provider rejects the request and reports the window it enforces, the embedder re-truncates the batch to that window and retries exactly once.
_context_window_from_errorreadsn_ctx/context_length/max_context_lengthfrom the error body. llama-server returnsn_ctx; the aliases cover other OpenAI-compatible servers._retry_input_within_contextre-bounds the strings and returnsNone— meaning "do not retry" — when:Both
embedandembed_asyncuse the same path, so sync and async behave alike. Everything else is unchanged: no new dependencies, no config keys, no change to the token estimate itself, and a failed retry surfaces the retry's error rather than masking it.Validation
{'error': {'code': 400, ..., 'n_ctx': 2560}},max_input_tokens=2400, 31,200-char overview):n_ctx→ no retry,n_ctxwider than the text → no retry (payload would be unchanged),context_lengthalias → retry,body→ no retry.tests/unit/test_openai_embedder.pycovering: retry succeeds, no-window → single attempt + original error, unchanged payload → single attempt, retry failure → second error surfaces.python -c "ast.parse(...)"on both changed files.Not run: the repo's full
pytest tests/unit— theopenvikingpackage is not installed in this environment (pip install -e .was not possible here), so the new tests have not executed against the realOpenAIDenseEmbedderconstructor. CI should be treated as the first real run of the test file; the helper logic itself is covered by the harness above.Note on the estimate itself
I deliberately did not change
chars / 4. Making the heuristic conservative would truncate text that currently fits, which is a behaviour change for every embedding caller. If maintainers prefer to also fix the estimate (option 2 in the report), that can be a follow-up with its own measurements — the retry added here stays correct either way.