Skip to content
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

feat: [aiManager] add interface #1031

Merged
merged 1 commit into from
Jan 6, 2025
Merged

Conversation

LiHua000
Copy link
Contributor

1.event when LLM changed
2.modelname
3.LLM`s idle state to description LLM can send request or not

Log: as title

Kakueeen
Kakueeen previously approved these changes Dec 30, 2024
QMap<QString, QVariant> map = OptionManager::getInstance()->getValue(kCATEGORY_CUSTOMMODELS, kCATEGORY_OPTIONKEY).toMap();
auto LLMs = map.value(kCATEGORY_CUSTOMMODELS);
if (LLMs.toList().size() != currentModels.size())
Copy link
Contributor

Choose a reason for hiding this comment

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

不应该通过个数来判断,模型自身参数的变化也应该有通知

Copy link
Contributor Author

Choose a reason for hiding this comment

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

通过个数判断是为了快速判断模型有没有被删除,不然需要额外用一个循环来判断当前模型是否被删除。 现在还没有支持模型的参数修改,只有新增和删除。

@@ -42,6 +45,52 @@ QJsonObject OpenAiCompatibleConversation::parseContentString(const QString &cont
const QString &deltaData = delta["content"].toString();
deltacontent += deltaData;
}
if (delta.contains("function_call")) {
Copy link
Contributor

Choose a reason for hiding this comment

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

这里面if太多了,注意优化下。另外这些参数是标准协议里面的吗?

1.event when LLM changed
2.modelname
3.LLM`s idle state to description LLM can send request or not

Log: as title
@deepin-ci-robot
Copy link

deepin pr auto review

关键摘要:

  • aimanager.cpp中,readLLMFromOption函数中新增了对模型变化检测的逻辑,但没有对ai.LLMChanged()的调用进行异常处理。
  • openaicompatibleconversation.cpp中的mergeFunctionCallmergeToolCallMap函数可能存在逻辑错误,因为它们直接拼接字符串,而没有考虑对象的结构和语义。
  • openaicompatiblellm.cpp中新增的isIdle函数没有注释说明其用途和返回值的含义。
  • openaicompatiblellm.h中新增的isIdle函数的声明和实现应该有相应的注释来解释其行为。

是否建议立即修改:

  • 应该立即修复mergeFunctionCallmergeToolCallMap函数中的逻辑错误,确保它们正确地合并对象。
  • 应该为isIdle函数添加注释,以便其他开发者理解其用途和返回值。
  • 应该添加异常处理逻辑到ai.LLMChanged()的调用中,以防止潜在的运行时错误。

@@ -133,6 +133,9 @@ OPI_OBJECT(session,
OPI_INTERFACE(sessionRenamed, "oldName", "newName")
OPI_INTERFACE(sessionRemoved, "session")
)
OPI_OBJECT(ai,
OPI_INTERFACE(LLMChanged)
Copy link
Contributor

Choose a reason for hiding this comment

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

这个事件只表示模型数量发生了改变,事件名应该为modelCountChanged

Copy link
Contributor Author

Choose a reason for hiding this comment

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

不是 是模型发生了变化就会发送,不管是模型被删除还是新增。 判断的那里设置Changed,两个判断 一个是模型被删除 一个是有新增

Copy link
Contributor Author

Choose a reason for hiding this comment

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

包括先删除再新增,数量不变,但是模型变化了,也会发送事件

@deepin-mozart
Copy link
Contributor

/retest

@deepin-mozart deepin-mozart merged commit eb9bf5f into linuxdeepin:master Jan 6, 2025
8 of 10 checks passed
@deepin-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: deepin-mozart, LiHua000

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants