-
-
Notifications
You must be signed in to change notification settings - Fork 41
[Fix] Refactor tool selection and fix vector store operations #592
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…nd vector store operations - Simplify tool selection logic in agent creator by directly filtering active tools - Fix vector store save operation to properly persist after adding vectors - Remove excessive debug logging from memory layers - Fix prompt variable handling for array-type prompts - Fix tool selector to return true when no selector is defined - Fix score threshold filtering to use actual similarity score - Fix Redis vector store to return correct document IDs without key prefix - Add reindex logging for Faiss and LunaVDB stores - Update LunaVDB documentation comment
|
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (26)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including You can disable this status message by setting the Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. Walkthrough本次变更集中于:重写 Agent 工具激活计算为过滤+比对流程;将部分向量存储写入流程改为异步并调整键/id 处理;优化 RAG 相似度阈值判定来源;更新命令工具选择器短路逻辑;完善提示变量对数组输入的处理;精简多处日志;更新系统提示语指引。 Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Agent as AgentCreator
participant Tools as toolsRef
participant Auth as Authorization
User->>Agent: updateActiveTools(messages)
Agent->>Tools: filter(tool.selector(messages))
Agent->>Auth: optional auth check per tool
Agent->>Agent: compute newActiveTools
Agent->>Agent: hasChanges = length/id comparison
Agent-->>User: [newActiveTools, hasChanges]
note right of Agent: 由差分/插入改为过滤+比较
sequenceDiagram
autonumber
participant Caller
participant VS as ChatLunaSaveableVectorStore
participant Store as UnderlyingStore
participant Disk as Save()
Caller->>VS: addVectors(vectors, metadatas, ids)
VS->>Store: await addVectors(...)
VS->>Disk: await save()
VS-->>Caller: ids
note right of VS: 新增保存步骤的 await
sequenceDiagram
autonumber
participant Client
participant RAG as StandardRAGRetriever
participant VS as VectorStore
Client->>RAG: similaritySearch(query, k, threshold)
RAG->>VS: search(query, k)
VS-->>RAG: [(doc, score)]...
RAG->>RAG: filter by tuple score >= threshold
RAG-->>Client: filtered docs
note right of RAG: 不再使用 doc.metadata.score
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
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. Comment |
Summary of ChangesHello @dingyi222666, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request focuses on enhancing the stability and efficiency of the system by refactoring core agent tool selection mechanisms and resolving several critical issues within vector store operations. It streamlines how tools are activated, ensures data persistence in vector stores, and refines prompt processing, alongside improving logging and documentation across various components. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request introduces a good set of refactorings and bug fixes. The tool selection logic is now much simpler and more direct. The fixes in vector store operations, particularly for saving data after adding vectors and correcting score threshold filtering, are crucial improvements. The added support for array-type prompts is also a valuable enhancement. I've identified a few areas for further improvement: a performance optimization in the tool selection logic, a potential regression in how prompt variables are handled, and a minor documentation inconsistency. Overall, this is a solid PR that improves the codebase's quality and maintainability.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
packages/core/src/llm-core/agent/creator.ts(1 hunks)packages/core/src/llm-core/vectorstores/base.ts(1 hunks)packages/extension-long-memory/src/layers/basic/layer.ts(0 hunks)packages/extension-long-memory/src/layers/emgas/layer.ts(0 hunks)packages/extension-long-memory/src/plugins/prompt_varaiable.ts(2 hunks)packages/extension-tools/src/plugins/command.ts(1 hunks)packages/service-vector-store/src/rag/standard/index.ts(1 hunks)packages/service-vector-store/src/vectorstore/base/lunavdb.ts(1 hunks)packages/service-vector-store/src/vectorstore/base/redis.ts(1 hunks)packages/service-vector-store/src/vectorstore/faiss.ts(1 hunks)packages/service-vector-store/src/vectorstore/lunavdb.ts(1 hunks)packages/service-vector-store/src/vectorstore/redis.ts(1 hunks)
💤 Files with no reviewable changes (2)
- packages/extension-long-memory/src/layers/basic/layer.ts
- packages/extension-long-memory/src/layers/emgas/layer.ts
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-15T12:28:21.006Z
Learnt from: dingyi222666
PR: ChatLunaLab/chatluna#543
File: packages/vector-store-service/src/vectorstore/faiss.ts:47-47
Timestamp: 2025-09-15T12:28:21.006Z
Learning: 在 packages/core/src/llm-core/vectorstores/base.ts 中,ChatLunaSaveableVectorStore 基类的 addDocuments 方法已经实现了向量存储和数据库文档存储的同步逻辑:先调用底层存储的 addDocuments,再调用 this._docstore.add() 进行备份,所有继承类(如 FaissVectorStore、MilvusVectorStore 等)都会自动获得这个同步功能。
Applied to files:
packages/core/src/llm-core/vectorstores/base.ts
🧬 Code graph analysis (2)
packages/extension-tools/src/plugins/command.ts (1)
packages/core/src/command.ts (1)
command(14-28)
packages/extension-long-memory/src/plugins/prompt_varaiable.ts (1)
packages/core/src/utils/string.ts (1)
getMessageContent(150-166)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: lint
- GitHub Check: build
🔇 Additional comments (8)
packages/service-vector-store/src/vectorstore/base/lunavdb.ts (1)
20-20: 文档更新正确。将注释中的客户端引用从"Voy Client"更正为"LunaVDB Client",与实际使用的客户端类型保持一致。
packages/extension-tools/src/plugins/command.ts (1)
45-47: 优化的短路逻辑正确。当命令未定义选择器时,直接返回
true避免不必要的历史记录遍历,提升了性能。这与 PR 中简化工具激活逻辑的目标一致。packages/service-vector-store/src/rag/standard/index.ts (1)
110-110: 相似度分数提取修复正确。现在使用元组中的实际相似度分数(第二个元素)进行阈值过滤,而不是从文档元数据中读取。这更符合 LangChain 向量存储 API 的返回格式
[Document, number],也更可靠。packages/service-vector-store/src/vectorstore/faiss.ts (1)
78-81: 重建索引日志增强了可观测性。添加日志记录重建索引时使用的文档数量,有助于运维监控和调试。与 LunaVDB 的类似改进保持一致。
packages/service-vector-store/src/vectorstore/lunavdb.ts (1)
76-79: 重建索引日志增强了可观测性。添加日志记录重建索引时使用的文档数量,与 Faiss 存储的改进保持一致,便于监控和诊断。
packages/core/src/llm-core/vectorstores/base.ts (1)
40-52: 确保向量持久化的改进正确。将
addVectors方法改为异步并在添加向量后调用save(),确保数据及时持久化。这与addDocuments方法保持一致(后者已在第 71 行调用save()),提高了数据完整性保障。根据学习记录,基类已实现向量存储和文档存储的同步逻辑,此改动进一步确保直接调用
addVectors时也能触发保存操作。packages/core/src/llm-core/agent/creator.ts (2)
95-106: 点赞代码重构!使用基于过滤的方法替代差异计算逻辑,代码更加简洁易读:
- 直接过滤激活的工具,逻辑清晰
- 使用长度对比和 ID 查找来检测变化,简单有效
- 消除了复杂的增量更新逻辑
这是一个很好的可维护性改进。
95-100: 确认tool.selector为必需属性或添加防御检查
第 96 行 调用tool.selector(messages),若selector可选将导致运行时错误。请检查工具类型声明,确保selector为必需属性;若为可选,请改为:- const selected = tool.selector(messages) + const selected = tool.selector?.(messages) ?? true
…handling and Redis key management - Update conversation summary prompt to better handle documents, memories, and knowledge - Fix unsafe type casting in prompt variable handler to safely handle array and string types - Revert Redis vector store key prefix handling to maintain consistency - Bump luna-vdb dependency to 0.0.12
Replace array.find() with Set.has() for O(1) lookup performance when checking tool changes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
♻️ Duplicate comments (2)
packages/service-vector-store/src/vectorstore/base/redis.ts (2)
54-68: 关键问题:删除操作的键处理不一致(与之前的审查评论重复)。这个问题在之前的审查中已经被标记。当前实现存在以下问题:
- 第 57 行优先检查
document.id,这可能是带前缀的(如果文档来自搜索结果)- 第 68 行总是添加前缀:
this._store.keyPrefix + id- 这会导致双重前缀,例如:
"prefix:prefix:id",使删除操作失败建议的修复方案(选择其一):
方案 1(推荐): 优先使用
raw_id,它始终是不带前缀的:const documentIds = options.documents ?.map((document) => { return ( - document.id ?? - (document.metadata?.raw_id as string | undefined) + (document.metadata?.raw_id as string | undefined) ?? + document.id?.replace(this._store.keyPrefix, '') ) })方案 2: 移除第 68 行的前缀添加(但这要求确保所有传入的 ID 都需要预先加上前缀):
-await this._client.del(this._store.keyPrefix + id) +await this._client.del(id)
27-30: 修复 delete 方法中对文档 ID 的双重前缀问题
在 packages/service-vector-store/src/vectorstore/base/redis.ts 的 delete 方法中(约行 57–63),mapping 时直接使用 document.id (已含前缀)导致_store.keyPrefix+id变成双前缀。请改为优先使用 document.metadata.raw_id 或在使用 document.id 前剥离 this._store.keyPrefix。
🧹 Nitpick comments (1)
packages/core/src/llm-core/agent/creator.ts (1)
95-100: 考虑为工具选择器添加错误处理。当前的过滤逻辑直接调用
tool.selector(messages),如果某个工具的选择器抛出异常,整个过滤操作将失败。虽然快速失败可以帮助发现选择器实现中的 bug,但也可能导致整个工具系统不可用。建议考虑添加错误处理以提高鲁棒性(如果工具选择器预期是可靠的,则可以忽略此建议):
const newActiveTools = toolsRef.filter((tool) => { + try { const selected = tool.selector(messages) return tool.authorization ? tool.authorization(session) && selected : selected + } catch (error) { + logger.warn(`Tool selector error for ${tool.id}:`, error) + return false + } })
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
packages/service-vector-store/package.jsonis excluded by!**/*.json
📒 Files selected for processing (4)
packages/core/src/llm-core/agent/creator.ts(1 hunks)packages/core/src/llm-core/chain/prompt.ts(1 hunks)packages/extension-long-memory/src/plugins/prompt_varaiable.ts(2 hunks)packages/service-vector-store/src/vectorstore/base/redis.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/extension-long-memory/src/plugins/prompt_varaiable.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: build
- GitHub Check: lint
🔇 Additional comments (2)
packages/core/src/llm-core/chain/prompt.ts (1)
138-143: 提示文本改进清晰且合理。系统提示的更新提供了更明确的指导:
- 明确区分了文档、记忆和知识三种上下文类型
- 清楚说明了何时应该使用或忽略上下文
- 强调了系统提示的优先级
- 改进的目标陈述更加自然和连贯
这些变更改善了 LLM 如何处理不同类型上下文的指导,与 PR 目标一致。
packages/core/src/llm-core/agent/creator.ts (1)
93-108: LGTM!重构后的逻辑清晰且高效。新的实现采用了过滤器模式并使用 Set 进行变更检测,这正是之前评审意见中建议的优化方案:
优点:
- 使用
filter直接计算活跃工具,逻辑更直观- 使用
Set进行 O(1) 查找,时间复杂度从 O(N*M) 优化到 O(N+M)- 变更检测逻辑简洁明了:先比较长度,再检查新工具是否在旧集合中
- 移除了复杂的差异计算和增量更新逻辑
实现细节:
- Line 102: 创建
Set用于高效的 ID 查找- Lines 104-106: 先检查长度差异(快速路径),再通过
some+ Set 查找检测变更- 授权检查与选择器结果正确组合(短路求值)
这个重构简化了工具选择流程,与 PR 目标一致。
- Bump core package to 1.3.0-alpha.63 - Bump extension-long-memory to 1.3.0-alpha.6 - Bump extension-tools to 1.3.0-alpha.19 - Bump service-vector-store to 1.3.0-alpha.2 - Update peer dependencies across all adapter and service packages
This PR refactors tool selection logic and fixes several issues in vector store operations across multiple packages.
Bug Fixes
Other Changes