feat(vector): auto-index observations into vector store | 向量索引自动集成#228
feat(vector): auto-index observations into vector store | 向量索引自动集成#228mechanic-Q wants to merge 1 commit into
Conversation
When an EmbeddingProvider is configured, automatically add compressed observations to the VectorIndex during the observe lifecycle (synthetic compression path). This enables hybrid BM25+Vector search without requiring manual index building. The auto-indexing runs after BM25 indexing within a try-catch so embedding failures don't block observation capture. Combined with configurable embedding model (AGENTMEMORY_LOCAL_EMBEDDING_MODEL), users can enable multilingual vector search by setting the model to bge-m3 or multilingual-e5. Passes vectorIndex and embeddingProvider as optional params to registerObserveFunction — fully backwards compatible. 当配置了嵌入模型时,自动将压缩后的观察结果添加到向量索引, 实现 BM25+向量混合搜索。嵌入失败不影响观察捕获。 配合可配置嵌入模型使用,设置 AGENTMEMORY_LOCAL_EMBEDDING_MODEL=bge-m3 即可启用中文等多语言语义搜索。
|
@mechanic-Q is attempting to deploy a commit to the rohitg00's projects Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughUpdated ChangesVector Indexing Integration
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Review rate limit: 4/8 reviews remaining, refill in 29 minutes and 56 seconds.Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/functions/observe.ts`:
- Around line 244-255: The current code awaits embeddingProvider.embed(...)
inside the observe hook which blocks the response and stream triggers; change
this to a fire-and-forget pattern by removing the await and chaining a promise
handler: call embeddingProvider.embed(narrative).then(vec =>
vectorIndex.add(obsId, payload.sessionId, vec)).catch(err => logger.warn("Vector
auto-index failed for observation", { obsId, error: err instanceof Error ?
err.message : String(err) })); keep the surrounding conditional (vectorIndex &&
embeddingProvider) and do not reintroduce await so embedding work runs
asynchronously and does not block stream::set or the hook response.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: aad77881-b4cf-4f03-8c28-df9a87ed4403
📒 Files selected for processing (2)
src/functions/observe.tssrc/index.ts
| if (vectorIndex && embeddingProvider) { | ||
| try { | ||
| const narrative = (synthetic.title || "") + " " + (synthetic.narrative || ""); | ||
| const vec = await embeddingProvider.embed(narrative); | ||
| vectorIndex.add(obsId, payload.sessionId, vec); | ||
| } catch (err) { | ||
| logger.warn("Vector auto-index failed for observation", { | ||
| obsId, | ||
| error: err instanceof Error ? err.message : String(err), | ||
| }); | ||
| } | ||
| } |
There was a problem hiding this comment.
await embeddingProvider.embed() is on the critical path — blocks the hook response and all downstream stream triggers.
The observe hook returns only after the embedding completes (line 247). ML inference (e.g., Xenova/bge-m3) adds 100 ms–2 s to every tool-use observation for anyone who opts into an embedding provider. This also contradicts the PR's stated design ("failures are logged and do not prevent observation capture"), because successful embeddings still block the hook response and the stream::set triggers at lines 256–277.
The existing pattern for async ML work in this same function is fire-and-forget via sdk.triggerVoid (see the vision-embed dispatch at lines 146–152). The simplest fix consistent with that pattern is dropping the await and using a chained .catch() so embedding errors are still swallowed:
⚡ Proposed fix — fire-and-forget embedding (non-blocking)
- if (vectorIndex && embeddingProvider) {
- try {
- const narrative = (synthetic.title || "") + " " + (synthetic.narrative || "");
- const vec = await embeddingProvider.embed(narrative);
- vectorIndex.add(obsId, payload.sessionId, vec);
- } catch (err) {
- logger.warn("Vector auto-index failed for observation", {
- obsId,
- error: err instanceof Error ? err.message : String(err),
- });
- }
- }
+ if (vectorIndex && embeddingProvider) {
+ const narrative = (synthetic.title || "") + " " + (synthetic.narrative || "");
+ embeddingProvider.embed(narrative)
+ .then((vec) => vectorIndex.add(obsId, payload.sessionId, vec))
+ .catch((err) =>
+ logger.warn("Vector auto-index failed for observation", {
+ obsId,
+ error: err instanceof Error ? err.message : String(err),
+ }),
+ );
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (vectorIndex && embeddingProvider) { | |
| try { | |
| const narrative = (synthetic.title || "") + " " + (synthetic.narrative || ""); | |
| const vec = await embeddingProvider.embed(narrative); | |
| vectorIndex.add(obsId, payload.sessionId, vec); | |
| } catch (err) { | |
| logger.warn("Vector auto-index failed for observation", { | |
| obsId, | |
| error: err instanceof Error ? err.message : String(err), | |
| }); | |
| } | |
| } | |
| if (vectorIndex && embeddingProvider) { | |
| const narrative = (synthetic.title || "") + " " + (synthetic.narrative || ""); | |
| embeddingProvider.embed(narrative) | |
| .then((vec) => vectorIndex.add(obsId, payload.sessionId, vec)) | |
| .catch((err) => | |
| logger.warn("Vector auto-index failed for observation", { | |
| obsId, | |
| error: err instanceof Error ? err.message : String(err), | |
| }), | |
| ); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/functions/observe.ts` around lines 244 - 255, The current code awaits
embeddingProvider.embed(...) inside the observe hook which blocks the response
and stream triggers; change this to a fire-and-forget pattern by removing the
await and chaining a promise handler: call
embeddingProvider.embed(narrative).then(vec => vectorIndex.add(obsId,
payload.sessionId, vec)).catch(err => logger.warn("Vector auto-index failed for
observation", { obsId, error: err instanceof Error ? err.message : String(err)
})); keep the surrounding conditional (vectorIndex && embeddingProvider) and do
not reintroduce await so embedding work runs asynchronously and does not block
stream::set or the hook response.
|
Hi, When AGENTMEMORY_AUTO_COMPRESS=true, mem::observe triggers mem::compress and does not run the new vectorIndex.add() code path, so the vector index stays empty even if an EmbeddingProvider is configured. Severity: action required | Category: correctness How to fix: Index vectors in mem::compress Agent prompt to fix - you can give this to your LLM of choice:
Spotted by Qodo code review - free for open-source projects. |
|
Reviewed — this fixes a real gap. Verified locally: What I like:
Two small notes:
No blockers. Approving — holding for @rohitg00 to land. |
) (#258) memory_save returned 200 with a valid memory ID, but every subsequent memory_smart_search and memory_recall returned empty results. Direct REST /agentmemory/smart-search showed the same — confirming the bug was not in the MCP layer. Root cause: mem::remember in src/functions/remember.ts wrote the new Memory to KV.memories but never called getSearchIndex().add(). The same pattern that #228 fixed for vectors-on-observe was missing here for BM25-on-remember. Compounded by rebuildIndex() walking only sessions/ observations and skipping KV.memories, so even a restart-rebuild couldn't recover indexed memories. Three changes: 1. src/functions/remember.ts — synthesize a CompressedObservation-shaped record from the saved Memory (title + content + concepts + files) and add it to BM25 right after kv.set(). Wrapped in try/catch so an indexing hiccup never blocks the durable save. 2. src/functions/search.ts — rebuildIndex() now walks KV.memories before sessions/observations, so a fresh rebuild covers the full corpus. Skips memory.isLatest === false so superseded entries don't pollute results. 3. src/index.ts — startup backfill for users upgrading from <0.9.5: when the persisted BM25 is non-empty (no rebuild triggered) but legacy memories were never indexed, walk KV.memories and add anything missing. SearchIndex.has(id) is the new idempotency gate. indexPersistence.scheduleSave() persists the augmented index. Tests in test/remember-bm25-index.test.ts cover: - SearchIndex.has() before/after add - A saved memory is findable by keyword (the case the bug broke) - The exact repro from the issue (mem_moy3u6ua_..., query "BM25 test") - Concept-only matches (no title/content overlap) Out of scope (file as follow-up): - Removing superseded memories from BM25 when mem::cascade-update fires. Currently relies on isLatest filter at result time; not perfect but doesn't regress recall. - Memory restore via IndexPersistence (covered transparently by the startup backfill, but a dedicated round-trip test would be tighter). Reported by @Nizar-BenHamida with full repro + log capture.
Summary | 概述
Automatically add compressed observations to the VectorIndex during the observe lifecycle, enabling hybrid BM25+Vector search without manual index building.
在观察生命周期中自动将压缩记忆添加到向量索引,实现 BM25+向量混合搜索。
Motivation | 动机
The VectorIndex and EmbeddingProvider infrastructure exists but observations were only added to the BM25 index. The vector index remained empty unless manually populated. This meant the hybrid search (BM25 + Vector RRF fusion) couldn't leverage semantic similarity — only BM25 keyword matching was active. For non-English content (Chinese, multilingual), BM25 alone is insufficient because the tokenizer can't handle CJK text well. Vector search bridges this gap.
向量索引基础设施已存在,但观察结果仅被添加到 BM25 索引,向量索引保持为空。混合搜索无法利用语义相似度,仅靠 BM25 关键词匹配。对于中文等非英文内容,纯 BM25 效果差,向量搜索可以弥补这一差距。
Changes | 改动
src/functions/observe.ts: After synthetic compression and BM25 indexing, auto-embed the narrative and add to vector indexsrc/index.ts: PassvectorIndexandembeddingProvidertoregisterObserveFunctionCombined with PR #223 (configurable embedding)
# Enable multilingual hybrid search (BM25 + Vector) AGENTMEMORY_LOCAL_EMBEDDING_MODEL=Xenova/bge-m3This combination gives agentmemory full Chinese/multilingual semantic search capability — BM25 handles exact terms, vector handles meaning.
Backwards Compatibility | 向后兼容
New params are optional. When
vectorIndexorembeddingProvideris null/undefined (default), the auto-indexing is skipped. No behavior change for existing deployments.Summary by CodeRabbit
Release Notes