Fix #2121: [Bug] memos-local-plugin: embedding source text exceeds embedding-3's 3072-token - #2123
Closed
Memtensor-AI wants to merge 20 commits into
Closed
Conversation
* feat(api): align OpenMem v1 SDK with cloud API
Co-authored-by: shinetata <149466187+shinetata@users.noreply.github.com>
…#2113) Rewrite scheduler Quick Start examples to call the real /product/scheduler REST endpoints with requests instead of non-existent MemOSClient methods; align response fields with the actual handler output; rename the mis-named ' wait.md' to 'wait.md'. Closes MemTensor#2083 Co-authored-by: sunqi <sunqi@memtensor.com> Co-authored-by: Cursor <cursoragent@cursor.com>
…ensor#2112) fix(api): show structured add example in /docs (messages not string) Swagger UI rendered APIADDRequest.messages as "string" because it picks the leading `str` branch of the `str | MessageList | RawMessageList` union when generating the example. Add a model-level json_schema_extra example so /docs shows a copy-paste-ready payload with a structured messages list. Schema-only change: field types, validation and runtime behaviour are unchanged, so the core add pipeline is unaffected. Closes MemTensor#1505 Co-authored-by: sunqi <sunqi@memtensor.com> Co-authored-by: Cursor <cursoragent@cursor.com>
…emTensor#2096) The hermes adapter's __init__.py imported MemosHttpClient from bridge_client, but the class was never committed — every plugin python test failed at collection and any real Hermes host loading memos_provider hit ImportError at module load. The phantom usages arrived via the pr/may27-fixes merge (0398e0e) as a half-committed HTTP-bridge feature. Revert the four usages and the HTTP-first branches in initialize / _reconnect_bridge, remove the _connect_http_bridge helper and the now- unused probe_viewer_status / startup_lock_active imports (their definitions in daemon_manager.py are untouched for a future HTTP PR). Add test_module_imports_cleanly regression test asserting MemTensorProvider / MemosBridgeClient / BridgeError are present on the real contract surfaces and MemosHttpClient is absent on both memos_provider and bridge_client, so this class of import-level breakage cannot silently return. Restores the adapter to stdio-only behavior. Net diff: 57 additions / 54 deletions across the adapter plus the regression test. Fixes MemTensor#2096
Avoid re-running dictConfig on every get_logger call, since repeated configuration can close and replace active handlers while background threads are emitting records. Keep logging configuration process-local and reconfigure only after a PID change.
Use FastAPI lifespan cleanup to stop scheduler and RabbitMQ resources before logging handlers are torn down during worker or pod shutdown.
## Description fix: configure logging once per process fix: close scheduler resources on API shutdown ## Type of change - [x] Bug fix (non-breaking change which fixes an issue) ## How Has This Been Tested? - [x] Unit Test - tests/api/test_lifecycle.py, tests/test_log.py ## Checklist - [x] I have performed a self-review of my own code | 我已自行检查了自己的代码 - [x] I have commented my code in hard-to-understand areas | 我已在难以理解的地方对代码进行了注释 - [x] I have added tests that prove my fix is effective or that my feature works | 我已添加测试以证明我的修复有效或功能正常 - [ ] I have created related documentation issue/PR in [MemOS-Docs](https://github.com/MemTensor/MemOS-Docs) (if applicable) | 我已在 [MemOS-Docs](https://github.com/MemTensor/MemOS-Docs) 中创建了相关的文档 issue/PR（如果适用） - [x] I have linked the issue to this PR (if applicable) | 我已将 issue 链接到此 PR（如果适用） - [x] I have mentioned the person who will review this PR | 我已提及将审查此 PR 的人 ## Reviewer Checklist - [ ] closes #xxxx (Replace xxxx with the GitHub issue number) - [ ] Made sure Checks passed - [ ] Tests have been provided
… rebuild-batch failures (MemTensor#2121) Long trace text (e.g. 30+ KB agent_text) was fed unmodified into embedding providers, triggering HTTP 400 code:1210 on 智谱 embedding-3 (3072-token single-input cap). rebuildEmbeddings then set failed = batch.length, so one poison input blanket- failed every short valid row next to it — the "Rebuild Vectors" button returned 400 within milliseconds and the embedding heartbeat probe reported ~60 % error. Three surgical fixes in apps/memos-local-plugin: 1. Embedder facade truncates each input to EmbeddingConfig.maxInputChars (default 6000; 0 disables) BEFORE hashing the cache key, so every provider is protected and repeat calls with the same head text hit the LRU. One input_truncated warn is emitted per embedMany call. 2. rebuildEmbeddings now divides-and-conquers on provider failure: split the batch in half and retry each half; when a sub-batch is size 1, mark only that slot as failed. Aggregation preserves the existing EmbeddingMaintenanceRunResult contract. 3. fetcher.ts's http.non_ok warn now carries a truncated response body (first 512 chars) so operators can pinpoint provider error codes (e.g. code:1210) from gateway.log alone. 4. embedding.maxInputChars is exposed on the user schema with default 6000, min 0. Tests: 5 new embedder cases + 2 new fetcher cases + 1 new memory-core case (all TDD — red before code, green after). Unit + integration suite green; two pre-existing failures (startup-recovery, storage/migrator) reproduce on the base commit and are unrelated to embedding.
Memtensor-AI
added
ai:generated
3 tasks
Collaborator
Author
🤖 Open Code ReviewTarget: PR #2123 ✅ OpenCodeReview: No comments generated. Looks good to me. 🧹 Filtered 1 low-confidence OCR finding(s) before posting/fix-loop (existing_code_mismatch: 1). Generated by cloud-assistant via Open Code Review. |
Collaborator
Author
🔧 Open Code Review requested Agent fixOpen Code Review found 7 issue(s). I have resumed the development Agent to fix them.
The Agent will push a new commit to this PR branch. OCR will recheck after the commit is pushed. |
… fix - Share DEFAULT_MAX_INPUT_CHARS via embedding/constants.ts so schema, defaults and facade fallback cannot drift; lower default 6000→4000 chars so CJK-dominant text stays under embedding-3's 3072-token cap - resolveMaxInputChars: NaN falls back to the default instead of silently disabling truncation; +Infinity is an explicit opt-out - embedAndApplySlots: short-circuit divide-and-conquer on transient errors (429/5xx/network) to avoid O(N log N × maxRetries) call amplification; pass embedder as a parameter instead of a non-null assertion; surface firstError when embedMany returns short - Document the http.non_ok body log field as potentially sensitive (verbatim third-party payload) in fetcher.ts and README Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Memtensor-AI
added
status:ready
Collaborator
|
Resolved the conflicts against current Conflict handling:
Validation on current
|
Collaborator
|
Manual Test Engine follow-up for merge commit
Generated advisory branch: |
17 tasks
Collaborator
|
Thanks for addressing #2121 and for the thorough tests. We are replacing this approach with a token-based solution in #2266: configurable The provider error diagnostics were also useful, but logging raw response excerpts may expose provider-echoed memory content, so we will handle diagnostics separately with stricter redaction. Since #2266 supersedes this PR, I’m closing it. Thanks again for the valuable investigation and contribution. |
syzsunshine219
added a commit
that referenced
this pull request
Aug 24, 2026
## Description This PR consolidates the tested local-plugin fixes from `fix-20260820-local-plugin`: - preserves the OpenClaw stale/recycled PID lock fix from #2099; - makes packed OpenClaw and Hermes installations compatible with npm 11 when development peer dependencies are omitted; - replaces the character-based head truncation proposed in #2123 with provider-level `batchSize`, token-based `maxInputTokens`, bounded four-chunk sampling and vector pooling, adaptive batch splitting for 400/413/422 responses, and per-input failure isolation across capture, import, and embedding rebuild paths; - defaults new installations to `maxInputTokens: 1024`, while preserving `0` for existing configurations created before this setting; - removes the separate rebuild “items per request” control and exposes only provider batch size and the per-input token limit. This supersedes #2123. Related Issue (Required): Fixes #2121 Related: #2099 ## Type of change - [x] Bug fix (non-breaking change which fixes an issue) - [x] New feature (non-breaking change which adds functionality) - [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected) - [ ] Refactor (does not change functionality, e.g. code style improvements, linting) - [x] Documentation update ## How Has This Been Tested? - [x] Unit Test - `pnpm test`: 176 test files passed; 1,466 tests passed and 2 skipped. - Targeted config, embedding, install, runtime-lock, and pipeline suite: 258 tests passed and 1 skipped. - [x] Test Script Or Test Steps - `pnpm lint` - `pnpm build:package` - macOS: installed and behavior-tested with OpenClaw and Hermes. - Windows: installed and behavior-tested with OpenClaw, Hermes, and DeepSeek Harness. - [ ] Pipeline Automated API Test (not applicable) `make format` was also attempted, but Poetry is not installed in the local environment (`poetry: No such file or directory`). The plugin's own lint, full tests, and production build all pass. ## Checklist - [x] I have performed a self-review of my own code | 我已自行检查了自己的代码 - [x] I have commented my code in hard-to-understand areas | 我已在难以理解的地方对代码进行了注释 - [x] I have added tests that prove my fix is effective or that my feature works | 我已添加测试以证明我的修复有效或功能正常 - [x] I have created related documentation issue/PR in [MemOS-Docs](https://github.com/MemTensor/MemOS-Docs) (if applicable) | Not applicable; the in-repository configuration templates and embedding documentation were updated. - [x] I have linked the issue to this PR (if applicable) | 我已将 issue 链接到此 PR（如果适用） - [x] I have mentioned the person who will review this PR | 我已提及将审查此 PR 的人 @Memtensor-AI @syzsunshine219 please review. ## Reviewer Checklist - [ ] closes #2121 (Replace xxxx with the GitHub issue number) - [ ] Made sure Checks passed - [ ] Tests have been provided
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
7 participants
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.
Description
Fixed issue #2121: memos-local-plugin no longer crashes the whole rebuild-embedding batch when a single trace exceeds the provider's per-input token limit (e.g. 智谱 embedding-3's 3072-token cap → HTTP 400 code:1210).
Three surgical fixes in apps/memos-local-plugin:
Embedder facade truncation (
core/embedding/embedder.ts+types.ts): addedEmbeddingConfig.maxInputChars(default 6000;0disables). Truncation runs BEFORE cache-key hashing so every provider is protected and repeat calls with the same head text hit the LRU. Oneinput_truncatedwarn is emitted per embedMany call (batched, not per-input).rebuildEmbeddings divide-and-conquer (
core/pipeline/memory-core.ts): replaced the blanketfailed = batch.lengthcatch with a halving retry. When a sub-batch is size 1, only that slot is marked failed; short valid rows next to a poison input are no longer punished. ExistingEmbeddingMaintenanceRunResultcontract preserved.Response body in log (
core/embedding/fetcher.ts):http.non_okwarn now carries a truncated response body (first 512 chars) so operators can pinpoint provider error codes (e.g. code:1210) from gateway.log alone.Also exposed
embedding.maxInputCharson the user config schema (default 6000, min 0). Chunked embedding (issue's fix #2) is deferred as follow-up per proposal.md.Tests: 5 new embedder cases + 2 new fetcher cases + 1 new memory-core case (all strict TDD).
pnpm --filter @memtensor/memos-local-plugin vitest run tests/unit/embedding tests/unit/pipeline/memory-core.test.ts tests/unit/config= 144 passed / 1 skipped.tsc --noEmitclean. Integration tests green. Two pre-existing unit failures (startup-recovery, storage/migrator) reproduce on the base commit and are unrelated to embedding.opsp artifacts (task.md + proposal.md + design.md + verification-report.md) archived to memos-autodev-specs/2026-07-19-2121-bug-memos-local-plugin-embedding-source-text-exceeds-embeddi/ (pushed to main).
Related Issue (Required): Fixes #2121
Type of change
Please delete options that are not relevant.
How Has This Been Tested?
Not run; documentation-only change.
Checklist
@whipser030, @hijzy, @WeiminLee please review this PR.
Reviewer Checklist