refactor(pipeline): 优化 Pipeline 保存路径及浏览器打开逻辑#15
Merged
MistEO merged 2 commits intoMAA-AI:mainfrom Dec 19, 2025
Merged
Conversation
- 默认保存 Pipeline 文件到用户 Downloads 目录，确保目录存在 - 依据文件路径推断起始目录参数，支持 Downloads、Documents、Desktop、Music、Pictures、Videos - 去除基于数据压缩的分享链接生成，改为传递导入目录和文件名参数 - 添加路径无效或无法推断时的错误提示信息 - 使用 URL 参数打开系统默认浏览器，提示用户选择本地文件导入 - 保留原有文件名清理和时间戳命名逻辑，增强文件管理方便性
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
你好——我已经review 了你的改动，这里有一些反馈：
- 默认保存路径被硬编码为
Path.home() / "Downloads"，这可能与本地化或平台特定的下载目录不一致；建议改为使用操作系统 API，或现有的配置/可信数据源来获取用户的下载文件夹。 open_pipeline_in_browser中的起始目录推断依赖对完整路径做子串检查（例如检查"docs"或"download"），这很容易造成误判；更稳妥的方式是检查路径组件（Path.parts），或者维护一份已知根目录的小映射表。- 当起始目录无法推断时，该函数会抛出错误并强制用户将文件移动到特定文件夹；建议在无法推断时退回到一个合理的默认值（例如
downloads），并仍然使用这个提示打开浏览器，以减少使用阻力。
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The default save path is hardcoded to `Path.home() / "Downloads"`, which may not match localized or platform-specific download directories; consider using an OS API or an existing configuration/source of truth for the user’s download folder instead.
- The start directory inference in `open_pipeline_in_browser` relies on substring checks over the full path (e.g., checking for `"docs"` or `"download"`), which can easily cause false positives; it would be more robust to inspect path components (`Path.parts`) or use a small mapping of known root folders.
- When the start directory cannot be inferred, the function raises an error and forces users to move the file into specific folders; consider falling back to a reasonable default (e.g., `downloads`) and still opening the browser with that hint to reduce friction.
## Individual Comments
### Comment 1
<location> `maa_mcp/pipeline.py:501-510` </location>
<code_context>
+ file_name = file_path.name
+
+ # 推断起始目录
+ lower_path = str(file_path).lower()
+ if "downloads" in lower_path or "download" in lower_path or "下载" in lower_path:
+ start_dir = "downloads"
+ elif "documents" in lower_path or "docs" in lower_path or "文档" in lower_path:
+ start_dir = "documents"
+ elif "desktop" in lower_path or "桌面" in lower_path:
+ start_dir = "desktop"
+ elif "music" in lower_path or "音乐" in lower_path:
+ start_dir = "music"
+ elif "pictures" in lower_path or "图片" in lower_path:
+ start_dir = "pictures"
+ elif "videos" in lower_path or "视频" in lower_path:
+ start_dir = "videos"
+ else:
</code_context>
<issue_to_address>
**suggestion:** Directory inference based on substring matching can misclassify paths and might be better done by inspecting path segments.
Because the heuristic looks for substrings like `"downloads" in lower_path`, it will also match paths such as `/home/user/mydownloads_backup` or `/data/documents_archive`, and the first match wins. To avoid this, consider iterating over `Path(file_path).parts` and matching full path segments against a small whitelist per category. This should make the inferred start directory more accurate and less surprising for users with similarly named folders.
Suggested implementation:
```python
# 提取文件名
file_name = file_path.name
# 推断起始目录：基于路径段而不是子串匹配，避免误判如 "mydownloads_backup"
path_parts = [part.lower() for part in Path(file_path).parts]
if any(part in {"downloads", "download", "下载"} for part in path_parts):
start_dir = "downloads"
elif any(part in {"documents", "docs", "文档"} for part in path_parts):
start_dir = "documents"
elif any(part in {"desktop", "桌面"} for part in path_parts):
start_dir = "desktop"
elif any(part in {"music", "音乐"} for part in path_parts):
start_dir = "music"
elif any(part in {"pictures", "图片"} for part in path_parts):
start_dir = "pictures"
elif any(part in {"videos", "视频"} for part in path_parts):
start_dir = "videos"
else:
```
1. Make sure `from pathlib import Path` is imported at the top of `maa_mcp/pipeline.py` (it appears to be present already since `Path.home()` is used, but confirm).
2. If this logic is inside a method where `file_path` might not be a `Path` instance, ensure it is converted beforehand (e.g., `file_path = Path(file_path)`).
</issue_to_address>Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Original comment in English
Hey there - I've reviewed your changes - here's some feedback:
- The default save path is hardcoded to
Path.home() / "Downloads", which may not match localized or platform-specific download directories; consider using an OS API or an existing configuration/source of truth for the user’s download folder instead. - The start directory inference in
open_pipeline_in_browserrelies on substring checks over the full path (e.g., checking for"docs"or"download"), which can easily cause false positives; it would be more robust to inspect path components (Path.parts) or use a small mapping of known root folders. - When the start directory cannot be inferred, the function raises an error and forces users to move the file into specific folders; consider falling back to a reasonable default (e.g.,
downloads) and still opening the browser with that hint to reduce friction.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The default save path is hardcoded to `Path.home() / "Downloads"`, which may not match localized or platform-specific download directories; consider using an OS API or an existing configuration/source of truth for the user’s download folder instead.
- The start directory inference in `open_pipeline_in_browser` relies on substring checks over the full path (e.g., checking for `"docs"` or `"download"`), which can easily cause false positives; it would be more robust to inspect path components (`Path.parts`) or use a small mapping of known root folders.
- When the start directory cannot be inferred, the function raises an error and forces users to move the file into specific folders; consider falling back to a reasonable default (e.g., `downloads`) and still opening the browser with that hint to reduce friction.
## Individual Comments
### Comment 1
<location> `maa_mcp/pipeline.py:501-510` </location>
<code_context>
+ file_name = file_path.name
+
+ # 推断起始目录
+ lower_path = str(file_path).lower()
+ if "downloads" in lower_path or "download" in lower_path or "下载" in lower_path:
+ start_dir = "downloads"
+ elif "documents" in lower_path or "docs" in lower_path or "文档" in lower_path:
+ start_dir = "documents"
+ elif "desktop" in lower_path or "桌面" in lower_path:
+ start_dir = "desktop"
+ elif "music" in lower_path or "音乐" in lower_path:
+ start_dir = "music"
+ elif "pictures" in lower_path or "图片" in lower_path:
+ start_dir = "pictures"
+ elif "videos" in lower_path or "视频" in lower_path:
+ start_dir = "videos"
+ else:
</code_context>
<issue_to_address>
**suggestion:** Directory inference based on substring matching can misclassify paths and might be better done by inspecting path segments.
Because the heuristic looks for substrings like `"downloads" in lower_path`, it will also match paths such as `/home/user/mydownloads_backup` or `/data/documents_archive`, and the first match wins. To avoid this, consider iterating over `Path(file_path).parts` and matching full path segments against a small whitelist per category. This should make the inferred start directory more accurate and less surprising for users with similarly named folders.
Suggested implementation:
```python
# 提取文件名
file_name = file_path.name
# 推断起始目录：基于路径段而不是子串匹配，避免误判如 "mydownloads_backup"
path_parts = [part.lower() for part in Path(file_path).parts]
if any(part in {"downloads", "download", "下载"} for part in path_parts):
start_dir = "downloads"
elif any(part in {"documents", "docs", "文档"} for part in path_parts):
start_dir = "documents"
elif any(part in {"desktop", "桌面"} for part in path_parts):
start_dir = "desktop"
elif any(part in {"music", "音乐"} for part in path_parts):
start_dir = "music"
elif any(part in {"pictures", "图片"} for part in path_parts):
start_dir = "pictures"
elif any(part in {"videos", "视频"} for part in path_parts):
start_dir = "videos"
else:
```
1. Make sure `from pathlib import Path` is imported at the top of `maa_mcp/pipeline.py` (it appears to be present already since `Path.home()` is used, but confirm).
2. If this logic is inside a method where `file_path` might not be a `Path` instance, ensure it is converted beforehand (e.g., `file_path = Path(file_path)`).
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+501
to
+510
| lower_path = str(file_path).lower() | ||
| if "downloads" in lower_path or "download" in lower_path or "下载" in lower_path: | ||
| start_dir = "downloads" | ||
| elif "documents" in lower_path or "docs" in lower_path or "文档" in lower_path: | ||
| start_dir = "documents" | ||
| elif "desktop" in lower_path or "桌面" in lower_path: | ||
| start_dir = "desktop" | ||
| elif "music" in lower_path or "音乐" in lower_path: | ||
| start_dir = "music" | ||
| elif "pictures" in lower_path or "图片" in lower_path: |
Contributor
There was a problem hiding this comment.
suggestion: 基于子串匹配的目录推断可能会误分类路径，改为检查路径段会更可靠。
由于当前启发式会查找类似 "downloads" in lower_path 这样的子串，它同样会匹配诸如 /home/user/mydownloads_backup 或 /data/documents_archive 这样的路径，而且是第一个匹配就生效。为避免这种情况，建议遍历 Path(file_path).parts，并将完整路径段与每个类别的一小段白名单进行匹配。这样推断出的起始目录会更准确，对于拥有类似命名文件夹的用户也不那么出乎意料。
建议的实现：
# 提取文件名
file_name = file_path.name
# 推断起始目录：基于路径段而不是子串匹配，避免误判如 "mydownloads_backup"
path_parts = [part.lower() for part in Path(file_path).parts]
if any(part in {"downloads", "download", "下载"} for part in path_parts):
start_dir = "downloads"
elif any(part in {"documents", "docs", "文档"} for part in path_parts):
start_dir = "documents"
elif any(part in {"desktop", "桌面"} for part in path_parts):
start_dir = "desktop"
elif any(part in {"music", "音乐"} for part in path_parts):
start_dir = "music"
elif any(part in {"pictures", "图片"} for part in path_parts):
start_dir = "pictures"
elif any(part in {"videos", "视频"} for part in path_parts):
start_dir = "videos"
else:- 请确保在
maa_mcp/pipeline.py文件顶部已经导入了from pathlib import Path（看起来已经存在，因为使用了Path.home()，但最好确认一下）。 - 如果这段逻辑位于一个方法内部，并且
file_path可能不是Path实例，请确保在此之前将其转换（例如file_path = Path(file_path)）。
Original comment in English
suggestion: Directory inference based on substring matching can misclassify paths and might be better done by inspecting path segments.
Because the heuristic looks for substrings like "downloads" in lower_path, it will also match paths such as /home/user/mydownloads_backup or /data/documents_archive, and the first match wins. To avoid this, consider iterating over Path(file_path).parts and matching full path segments against a small whitelist per category. This should make the inferred start directory more accurate and less surprising for users with similarly named folders.
Suggested implementation:
# 提取文件名
file_name = file_path.name
# 推断起始目录：基于路径段而不是子串匹配，避免误判如 "mydownloads_backup"
path_parts = [part.lower() for part in Path(file_path).parts]
if any(part in {"downloads", "download", "下载"} for part in path_parts):
start_dir = "downloads"
elif any(part in {"documents", "docs", "文档"} for part in path_parts):
start_dir = "documents"
elif any(part in {"desktop", "桌面"} for part in path_parts):
start_dir = "desktop"
elif any(part in {"music", "音乐"} for part in path_parts):
start_dir = "music"
elif any(part in {"pictures", "图片"} for part in path_parts):
start_dir = "pictures"
elif any(part in {"videos", "视频"} for part in path_parts):
start_dir = "videos"
else:- Make sure
from pathlib import Pathis imported at the top ofmaa_mcp/pipeline.py(it appears to be present already sincePath.home()is used, but confirm). - If this logic is inside a method where
file_pathmight not be aPathinstance, ensure it is converted beforehand (e.g.,file_path = Path(file_path)).
MistEO
reviewed
Dec 19, 2025
Contributor
Author
|
感觉兼容性更低了（） |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
省流：在用户 downloads 文件夹里拉💩（
Summary by Sourcery
优化管道文件保存的默认行为，并通过传递导入提示而非嵌入数据的方式，简化基于浏览器的可视化集成。
新功能：
增强内容：
Original summary in English
Summary by Sourcery
Refine pipeline file saving defaults and simplify browser-based visualization integration by passing import hints instead of embedded data.
New Features:
Enhancements:
新功能：
增强改进：
Original summary in English
Summary by Sourcery
优化管道文件保存的默认行为，并通过传递导入提示而非嵌入数据的方式，简化基于浏览器的可视化集成。
新功能：
增强内容：
Original summary in English
Summary by Sourcery
Refine pipeline file saving defaults and simplify browser-based visualization integration by passing import hints instead of embedded data.
New Features:
Enhancements: