Skip to content

Comments

feat: add memory#95

Closed
yysxjz wants to merge 1 commit intoFellouAI:poc/reactfrom
yysxjz:feat/memory
Closed

feat: add memory#95
yysxjz wants to merge 1 commit intoFellouAI:poc/reactfrom
yysxjz:feat/memory

Conversation

@yysxjz
Copy link
Contributor

@yysxjz yysxjz commented Mar 19, 2025

  • 在 Action 类中集成反射记忆,用于增强描述信息
  • 在 WorkflowImpl 类中添加反射记忆设置
  • 实现 IndexDBStore 类作为记忆存储解决方案

- 在 Action 类中集成反射记忆,用于增强描述信息
- 在 WorkflowImpl 类中添加反射记忆设置
- 实现 IndexDBStore 类作为记忆存储解决方案
Copy link
Contributor

@HairlessVillager HairlessVillager left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. 使用代码格式化工具格式你的所有新增/修改代码;
  2. 格式化后再来相应我的修改意见,如果你觉得有些地方不清楚可以继续在 PR 上回复我。

@@ -0,0 +1,94 @@
import {LLMProvider, Message} from '@/types';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

用代码格式化工具把提交的代码都过一遍吧

Comment on lines +18 to +20
private buildInputMessages(messages: Awaited<Message[]>[]): Message[] {
console.log("debug,the input messages is....")
for(const message of messages){
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这种缩进随便写的人真应该发配去写 Python(

console.log("debug,the input messages is....")
for(const message of messages){
message.shift()
for(const mes of message){
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

刚才看走眼了,更新一下:

建议不要用 mes 这种变量名,一眼看不出来这个变量想表达什么。message_chunkmessage_elem可能会好一点,但如果你能精准表达这个变量的具体含义的话就更好了。

this.historyMes = "";
}

private buildInputMessages(messages: Awaited<Message[]>[]): Message[] {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

看起来这里的 messages是一个二维数组,但是这里的变量名没有体现这一点。你应该考虑修改这里的变量名。

此外下面的 message看起来是一个一维数组,而且数组元素类型是Message?你这里有仔细考虑过命名吗?😂

for(const message of messages){
message.shift()
for(const mes of message){
if(JSON.stringify(mes).includes("base64")){
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这里的逻辑是“判断这条消息是否是一个图片”吗?这样写是否经过了充分的测试?如果既没有测试又要赶进度的话,建议这里打一下日志,免得后面出问题了无法定位。

},
{
role: 'user',
content: `Here is the historical messages+${this.historyMes}`,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

你有把这里的 messages 打日志里看了吗?这个 user message 你确定是你期望的吗?

Comment on lines +30 to +72
async getSteps(task:string): Promise<MemoryChunk> {
const results = await this.vectorStore.similaritySearch({
query: task,
k:1,
filterOptions:{
include:{
metadata:{
type:"steps",
}
}
}
});
const doc = results.similarItems[0]

return {
task:doc.metadata.task,
steps:doc.text,
type:"steps",
};
}


async getFact(task:string): Promise<MemoryChunk> {
const results = await this.vectorStore.similaritySearch({
query: task,
k:1,
filterOptions:{
include:{
metadata:{
type:"fact",
}
}
}
});
const doc = results.similarItems[0]

return {
task:doc.metadata.task,
steps:doc.text,
type:"fact",
};
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

在大多数情况下最好不要有重复代码,如果有两个相似度很高的逻辑,你可以考虑新增一个函数来实现重复的逻辑。

当然还有一些情况允许有重复代码,不过我不清楚这里的业务场景,你自己看着办吧。

Comment on lines +73 to +88
async putSteps(steps:string,task:string): Promise<void> {
steps = `task:${task}----steps:${steps}`
await this.vectorStore.addText(steps,{
type:"steps",
task: task
});
}

async putFact(fact:string,task:string): Promise<void> {
fact = `task:${task}----fact:${fact}`
await this.vectorStore.addText(fact,{
type:"fact",
task: task
});
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

同上

Comment on lines +347 to +357
if(context.llmProvider instanceof OpenaiProvider){
console.log("the llm provider is openai")
let llm = new ReflexionLLM(context.llmProvider)
await llm.init()
const steps = await llm.getReflexion(this.description)
if (steps){
console.log("the steps is :"+steps)
this.description = `${this.description} the example you can reference to plan is ${steps}`
}
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

我没看懂这里的逻辑:为什么只判断了 OpenaiProviderClaudeProvider不考虑吗?如果后续要新增模型怎么办?

如果这里是因为 OpenAI 系模型需要加特殊的逻辑,可以写清楚一点,加个注释之类的。

export class OpenaiProvider implements LLMProvider {
private client: OpenAI;
private defaultModel = 'gpt-4o';
readonly apikey:string = "";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这个参数哪里要用到吗?我没找到。如果没有用到建议删去。

@yysxjz yysxjz closed this Mar 26, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants