fix(embedder): safe 3072-token default + token-aware truncation - #2190
fix(embedder): safe 3072-token default + token-aware truncation#2190RerankerGuo wants to merge 1 commit into
Conversation
…3072-token default - embedders/base.py: add _SAFE_EMBEDDING_MAX_TOKENS=3072, new BaseEmbedder._effective_max_tokens() fallback, and make _truncate_texts() token-aware by calling _truncate_text_to_tokens (binary search over prefix length) instead of a naive character-slice. - configs/embedder.py: BaseEmbedderConfig.max_tokens default changed from 8192 to None so the new 3072-token safe-limit kicks in for providers such as text-embedding-3 that enforce a hard input cap. - tests/embedders/test_base.py: new tests covering effective_max_tokens for None/0/explicit overrides, _truncate_text_to_tokens bounds on long CJK text, and _truncate_texts with the new 3072 default. Closes MemTensor#2121.
🤖 Open Code ReviewTarget: PR #2190 🔍 OpenCodeReview found 4 issue(s) in this PR. 1.
|
|
Background
Issue #2121 reports that
memos-local-pluginingestion passes source text directly to the embedding API, and text-embedding-3 enforces a hard 3072-token input cap. Previously, the embedder's defaultmax_tokenswas 8192 (which exceeds the API limit) and the truncation was a plain character-slice that did not account for token counts at all. This caused out-of-budget embedding errors for longer documents and knowledge chunks.Changes
memos/embedders/base.py:_SAFE_EMBEDDING_MAX_TOKENS = 3072matching text-embedding-3's input limit.BaseEmbedder._effective_max_tokens(): returns an explicitly configured positivemax_tokens, otherwise the 3072 safe default (also used when the user configuresmax_tokens=Noneormax_tokens=0)._truncate_texts()to delegate to the existing token-aware_truncate_text_to_tokens()binary-search helper. A fast-path length check keeps the cheap case cheap (text length <= max tokens means no token count required).memos/configs/embedder.py:BaseEmbedderConfig.max_tokensdefault from8192toNoneso that the new 3072 safe-limit takes effect for any user not overriding the field explicitly; the docstring was updated.tests/embedders/test_base.py:_effective_max_tokens()behaviour acrossNone,0, and explicit positive values._truncate_text_to_tokens(): short text unchanged, empty/None/0 boundaries, long CJK text stays within budget._truncate_texts(): short texts untouched, very long CJK text truncated within the 3072-token default, explicit 10-token override honoured.Verification
ruff format+ruff check --fixapplied (2 pre-existing BLE001 broad-excepts in_count_tokens_for_embeddingleft intact).python3 -m py_compilesucceeds for modified files.Impact
max_tokens; behaviour changes only when the field was left at the previous 8192 default.