fix: chatui cannot persist file segment#5386
Conversation
dosubot
bot
added
the
size:S
Summary of ChangesHello @Soulter, 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 resolves an issue where file segments were not persisting correctly within the chat UI. It achieves this by refactoring the internal handling of attachments, centralizing their storage location, and introducing backward compatibility to ensure that previously stored files remain accessible regardless of their original storage path. This change enhances the reliability of attachment persistence across the application. Highlights
Changelog
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
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - 我在这里给出了一些整体性的反馈：
- 在
WebChatMessageEvent中，建议暂时保留一个兼容性别名或注释，指明旧的webchat/imgs路径，这样运维人员可以理解为什么文件现在存储在attachments下，以及这对现有部署有何影响。 - 在
_create_attachment_from_file中，当找不到某个文件名的候选路径时，记录一条警告日志可能会有所帮助，这样更容易诊断缺失或遗留附件。
给 AI 代理的提示
Please address the comments from this code review:
## Overall Comments
- In `WebChatMessageEvent`, consider keeping a temporary compatibility alias or comment for the old `webchat/imgs` path so operators understand why files are now stored under `attachments` and how this impacts existing deployments.
- In `_create_attachment_from_file`, it may be helpful to log a warning when no candidate path is found for a filename so that missing/legacy attachments can be diagnosed more easily.帮我变得更有用！请在每条评论上点 👍 或 👎，我会根据你的反馈改进后续的代码审查。
Original comment in English
Hey - I've left some high level feedback:
- In
WebChatMessageEvent, consider keeping a temporary compatibility alias or comment for the oldwebchat/imgspath so operators understand why files are now stored underattachmentsand how this impacts existing deployments. - In
_create_attachment_from_file, it may be helpful to log a warning when no candidate path is found for a filename so that missing/legacy attachments can be diagnosed more easily.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `WebChatMessageEvent`, consider keeping a temporary compatibility alias or comment for the old `webchat/imgs` path so operators understand why files are now stored under `attachments` and how this impacts existing deployments.
- In `_create_attachment_from_file`, it may be helpful to log a warning when no candidate path is found for a filename so that missing/legacy attachments can be diagnosed more easily.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
Related Documentation Checked 1 published document(s) in 1 knowledge base(s). No updates required. How did I do? Any feedback? |
dosubot
bot
added
the
feature:chatui
|
Generated docs update PR (pending manual review): AI change summary:
Experimental bot notice:
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
The pull request addresses a critical bug where chat UI attachments were not persisting correctly. The changes involve migrating attachment storage from a webchat/imgs directory to a unified attachments directory. Additionally, backward compatibility is introduced to allow retrieval of attachments from the old location if they still exist there. This is a good fix that improves the robustness of attachment handling.
| from .webchat_queue_mgr import webchat_queue_mgr | ||
|
|
||
| imgs_dir = os.path.join(get_astrbot_data_path(), "webchat", "imgs") | ||
| attachments_dir = os.path.join(get_astrbot_data_path(), "attachments") |
There was a problem hiding this comment.
The variable attachments_dir is defined at the module level. While this is acceptable, consider encapsulating it within the WebChatMessageEvent class or a dedicated configuration module if it's meant to be a configurable path. This improves modularity and makes the path's purpose clearer within the class context.
| def __init__(self, message_str, message_obj, platform_meta, session_id) -> None: | ||
| super().__init__(message_str, message_obj, platform_meta, session_id) | ||
| os.makedirs(imgs_dir, exist_ok=True) | ||
| os.makedirs(attachments_dir, exist_ok=True) |
There was a problem hiding this comment.
The os.makedirs call here ensures the attachments_dir exists. This is good for ensuring the directory is ready before use. However, consider if this operation needs to be performed every time a WebChatMessageEvent is initialized. If the directory is guaranteed to exist after application startup, this could be moved to a single initialization point to avoid redundant calls.
| basename = os.path.basename(filename) | ||
| candidate_paths = [ | ||
| os.path.join(self.attachments_dir, basename), | ||
| os.path.join(self.legacy_img_dir, basename), | ||
| ] | ||
| file_path = next((p for p in candidate_paths if os.path.exists(p)), None) |
There was a problem hiding this comment.
The introduction of candidate_paths and the use of next((p for p in candidate_paths if os.path.exists(p)), None) is a good approach for backward compatibility. This ensures that attachments can still be found even if they are in the legacy directory. This is a robust solution for handling the migration.
1 participant
Modifications / 改动点
Screenshots or Test Results / 运行截图或测试结果
Checklist / 检查清单
requirements.txt和pyproject.toml文件相应位置。/ I have ensured that no new dependencies are introduced, OR if new dependencies are introduced, they have been added to the appropriate locations inrequirements.txtandpyproject.toml.由 Sourcery 提供的摘要
在统一的附件目录下持久化存储网页聊天附件，并新增对从旧版图片目录加载附件的向后兼容支持。
Bug 修复：
Original summary in English
Summary by Sourcery
Persist web chat attachments under a unified attachments directory and add backward-compatible loading of attachments from the legacy image directory.
Bug Fixes: