Conversation
Summary of ChangesHello, 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 addresses an issue where temporary 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. Footnotes
|
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- Instead of mutating
request.extra_user_content_partsaroundassemble_context, consider refactoringassemble_contextto accept the extra parts as an argument (or using a small helper/context manager) so that the request object doesn't need to be temporarily mutated and deep-copied. - Double-check that
Message(content=extra_user_content_parts)matches the expected content shape for the LLM (e.g., list ofContentPartvs. a single part) so that the extra user parts are interpreted identically to how they were when inlined into the original assembled context.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Instead of mutating `request.extra_user_content_parts` around `assemble_context`, consider refactoring `assemble_context` to accept the extra parts as an argument (or using a small helper/context manager) so that the request object doesn't need to be temporarily mutated and deep-copied.
- Double-check that `Message(content=extra_user_content_parts)` matches the expected content shape for the LLM (e.g., list of `ContentPart` vs. a single part) so that the extra user parts are interpreted identically to how they were when inlined into the original assembled context.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
There was a problem hiding this comment.
Code Review
This pull request correctly addresses the issue of extra_user_content_parts being incorrectly persisted in message history. The approach of creating a separate, temporary message marked with _no_save is a good solution. The implementation is logical, though I have a minor suggestion to improve efficiency by removing an unnecessary deepcopy operation. Overall, this is a solid fix.
| messages.append(m) | ||
| if request.prompt is not None: | ||
| m = await request.assemble_context() | ||
| extra_user_content_parts = copy.deepcopy(request.extra_user_content_parts) |
There was a problem hiding this comment.
Using copy.deepcopy here is not necessary and adds overhead. Since extra_user_content_parts is a local variable and you are not modifying its contents, a simple reference assignment is sufficient and safe. The try...finally block already ensures the original list reference is restored to request.extra_user_content_parts.
| extra_user_content_parts = copy.deepcopy(request.extra_user_content_parts) | |
| extra_user_content_parts = request.extra_user_content_parts |
|
额外内容块应该也得放进消息记录的,比如一些时间信息。,可以让llm知道某条消息什么时候发的,如果临时去掉也会影响缓存命中率? |
时间信息应该作为metadata放到消息记录里,而不是额外文本块。 |
那和直接放到额外内容块有啥区别 |
因为额外块里面的信息不一定需要保留,如果都保留会带来大量垃圾信息。 |
要不要分2个额外文本块,一个是不会入记录的,一个是会入记录的,这样你觉得呢?我马上改 |
|
这样做反而会破坏上游缓存命中,而且会导致上下文不一致的问题,不建议这么做 |
这是正确的做法,几乎所有软件的额外文本块都有临时部分,特别是rag,todo 相关的,都是只存在于最新消息。 这不会破坏缓存的,因为额外文本块在最后的消息,只要不破坏前面的就没问题,反而一直大量重复的信息出现在前面才是浪费token的。 |
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- Consider providing a first-class way to construct a non-persisted
Message(e.g., a constructor parameter or factory) instead of mutating the private_no_saveattribute after instantiation, so that the no-save behavior is explicit and less coupled to internal implementation details. - If
ProviderRequestinstances can be reused across multiple runs, you may want to explicitly cleartransient_extra_user_content_partsafter assembling messages to avoid accidentally carrying transient content into subsequent requests.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider providing a first-class way to construct a non-persisted `Message` (e.g., a constructor parameter or factory) instead of mutating the private `_no_save` attribute after instantiation, so that the no-save behavior is explicit and less coupled to internal implementation details.
- If `ProviderRequest` instances can be reused across multiple runs, you may want to explicitly clear `transient_extra_user_content_parts` after assembling messages to avoid accidentally carrying transient content into subsequent requests.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
比如第一轮后,历史是: [
{
"role": "user",
"content": [
{ "type": "text", "text": "帮我总结这张图" },
{ "type": "text", "text": "[发送时间: 2026-03-20 10:00]" },
{ "type": "text", "text": "当前任务" },
]
},
{
"role": "assistant",
"content": "这张图主要是在讲……"
}
]第二轮请求时: [
{
"role": "user",
"content": [
{ "type": "text", "text": "帮我总结这张图" },
{ "type": "text", "text": "[发送时间: 2026-03-20 10:00]" } <-这里之前都可以有效缓存命中缓存的消息
]
},
{
"role": "assistant",
"content": "这张图主要是在讲……"
},
{
"role": "user",
"content": [
{ "type": "text", "text": "谢谢你" },
{ "type": "text", "text": "[发送时间: 2026-03-20 10:01]" },
{ "type": "text", "text": "当前任务" },
]
},
]第三轮 [
{
"role": "user",
"content": [
{ "type": "text", "text": "帮我总结这张图" },
{ "type": "text", "text": "[发送时间: 2026-03-20 10:00]" }
]
},
{
"role": "assistant",
"content": "这张图主要是在讲……"
},
{
"role": "user",
"content": [
{ "type": "text", "text": "谢谢你" },
{ "type": "text", "text": "[发送时间: 2026-03-20 10:01]" } <-这里之前都可以有效缓存命中缓存的消息
]
},
{
"role": "assistant",
"content": "不客气。"
},
{
"role": "user",
"content": [
{ "type": "text", "text": "再帮我提炼成三句话" },
{ "type": "text", "text": "[发送时间: 2026-03-20 10:02]" },
{ "type": "text", "text": "当前任务" }
]
}
] |
新写法好了,但是我还不知道自带的系统有哪些是需要临时的,我看了下nosave的好像只有参考对话 |
摘要
transient_extra_user_content_parts,用于承载仅在组装当前轮最新用户消息时出现、且不会进入消息记录的临时内容块extra_user_content_parts的生产和消费逻辑不变,不改动框架内原有写入点问题说明
当前
extra_user_content_parts会进入正常消息记录。但第三方插件有可能需要给“当前这一次请求”附加临时文本块,这类内容不应该被持久化到消息历史。
如果误用
extra_user_content_parts来承载这类临时内容,就会把它们写入历史,并在后续轮次中继续被带入上下文。这不仅是错误实现,还会持续浪费 token 和存储空间。
修改说明
本次修复采用增量方式处理:
transient_extra_user_content_partsextra_user_content_parts的既有语义extra_user_content_parts.append(...)的调用点运行时仅在组装消息时额外处理这个新字段:
transient_extra_user_content_parts会被拼成一条临时的最新user消息_no_save这样可以保证:
extra_user_content_parts行为保持不变测试
uv run ruff check astrbot/core/provider/entities.pyuv run ruff check astrbot/core/agent/runners/tool_loop_agent_runner.pyuv run ruff check astrbot/core/astr_main_agent.py补充说明
这是一个不需要删除数据的小修复。
这个补丁不会主动清理已经被污染的旧历史记录,只提供一种正确的“临时额外内容块”承载方式,供后续插件使用。
Summary by Sourcery
Introduce transient user content blocks for per-request context without affecting persisted message history.
New Features:
Enhancements: