feat: add conversation batch deletion for webchat#6160
feat: add conversation batch deletion for webchat#6160Flartiny wants to merge 2 commits intoAstrBotDevs:masterfrom
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 introduces a highly requested feature: batch deletion of webchat conversations. This enhancement significantly improves user experience by allowing users to efficiently manage their chat history, moving beyond single-item deletion to a more streamlined bulk operation. The changes span both backend API development and frontend UI implementation, along with necessary internationalization updates. Highlights
Changelog
Activity
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
|
There was a problem hiding this comment.
Hey - 我发现了 1 个问题,并留下了一些高层次的反馈:
- 在
chat.py中,辅助函数_delete_session_internal接收了一个从未使用的username参数;建议要么移除此参数,要么把权限检查移动到该辅助函数内部,这样参数才有实际意义,同时避免在各个调用方重复实现同样的逻辑。 batch_delete_sessions循环按顺序逐个删除会话,并且对每个 ID 都执行一次完整的查询;如果这个端点预期要处理更大的批量请求,建议考虑使用批量查询或批量删除,以减少每个会话的数据库往返开销。- 在
useSessions.batchDeleteSessions和Chat.vue中,你同时在 composable 和 handler 中记录错误日志(各自都调用了console.error),这可能是多余的;将日志集中在某一层处理可以简化错误处理逻辑。
给 AI Agent 的提示词
Please address the comments from this code review:
## Overall Comments
- 在 `chat.py` 中,辅助函数 `_delete_session_internal` 接收了一个从未使用的 `username` 参数;建议要么移除此参数,要么把权限检查移动到该辅助函数内部,这样参数才有实际意义,同时避免在各个调用方重复实现同样的逻辑。
- `batch_delete_sessions` 循环按顺序逐个删除会话,并且对每个 ID 都执行一次完整的查询;如果这个端点预期要处理更大的批量请求,建议考虑使用批量查询或批量删除,以减少每个会话的数据库往返开销。
- 在 `useSessions.batchDeleteSessions` 和 `Chat.vue` 中,你同时在 composable 和 handler 中记录错误日志(各自都调用了 `console.error`),这可能是多余的;将日志集中在某一层处理可以简化错误处理逻辑。
## Individual Comments
### Comment 1
<location path="astrbot/dashboard/routes/chat.py" line_range="670-672" />
<code_context>
+ continue
+ await self._delete_session_internal(session, username)
+ deleted_count += 1
+ except Exception as e:
+ logger.warning(f"Failed to delete session {sid}: {e}")
+ failed_items.append({"session_id": sid, "reason": str(e)})
+
+ return (
</code_context>
<issue_to_address>
**🚨 issue (security):** 避免在 `batch_delete_sessions` 中将原始异常消息返回给客户端。
使用 `str(e)` 作为 `reason` 会将内部细节暴露给客户端,可能泄露实现细节或敏感信息。应当改为将错误映射到一小组通用且对客户端安全的原因(例如 `internal_error`),并仅在服务器端记录完整异常信息。
</issue_to_address>帮我变得更有用!请在每条评论上点击 👍 或 👎,我会根据这些反馈改进后续的代码审查。
Original comment in English
Hey - I've found 1 issue, and left some high level feedback:
- In
chat.py, the helper_delete_session_internaltakes ausernameparameter that is never used; consider either removing this argument or moving the permission check into this helper so the parameter is meaningful and the logic isn't duplicated across callers. - The
batch_delete_sessionsloop deletes sessions sequentially and performs a full fetch for each ID; if this endpoint is expected to handle larger batches, consider optimizing with bulk queries or batched deletes to reduce per-session DB round-trips. - In
useSessions.batchDeleteSessionsandChat.vueyou log errors in both the composable and the handler (console.errorin each), which may be redundant; centralizing the logging in one layer would simplify error handling.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `chat.py`, the helper `_delete_session_internal` takes a `username` parameter that is never used; consider either removing this argument or moving the permission check into this helper so the parameter is meaningful and the logic isn't duplicated across callers.
- The `batch_delete_sessions` loop deletes sessions sequentially and performs a full fetch for each ID; if this endpoint is expected to handle larger batches, consider optimizing with bulk queries or batched deletes to reduce per-session DB round-trips.
- In `useSessions.batchDeleteSessions` and `Chat.vue` you log errors in both the composable and the handler (`console.error` in each), which may be redundant; centralizing the logging in one layer would simplify error handling.
## Individual Comments
### Comment 1
<location path="astrbot/dashboard/routes/chat.py" line_range="670-672" />
<code_context>
+ continue
+ await self._delete_session_internal(session, username)
+ deleted_count += 1
+ except Exception as e:
+ logger.warning(f"Failed to delete session {sid}: {e}")
+ failed_items.append({"session_id": sid, "reason": str(e)})
+
+ return (
</code_context>
<issue_to_address>
**🚨 issue (security):** Avoid returning raw exception messages to the client in `batch_delete_sessions`.
Using `str(e)` for `reason` exposes internal details to clients, which can leak implementation or sensitive information. Instead, map errors to a small set of generic, client-safe reasons (e.g. `internal_error`) and log the full exception only on the server.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
astrbot/dashboard/routes/chat.py
Outdated
| for sid in session_ids: | ||
| try: | ||
| session = await self.db.get_platform_session_by_id(sid) | ||
| if not session: | ||
| failed_items.append({"session_id": sid, "reason": "not found"}) | ||
| continue | ||
| if session.creator != username: | ||
| failed_items.append( | ||
| {"session_id": sid, "reason": "permission denied"} | ||
| ) | ||
| continue | ||
| await self._delete_session_internal(session, username) | ||
| deleted_count += 1 | ||
| except Exception as e: | ||
| logger.warning(f"Failed to delete session {sid}: {e}") | ||
| failed_items.append({"session_id": sid, "reason": str(e)}) |
There was a problem hiding this comment.
当前实现中,您在循环内为每个 session_id 调用 self.db.get_platform_session_by_id(sid)。当批量删除大量会话时,这会导致 N+1 查询问题,对数据库造成不必要的压力并可能影响性能。
建议通过一次数据库查询批量获取所有会话,然后在内存中进行处理。这可以显著提高效率。
您可能需要在数据库访问层(db.py)中添加一个新方法,例如 get_platform_sessions_by_ids(session_ids)。
# 批量获取所有会话以提高效率。
# 这可能需要在数据库访问层添加一个新方法,如 `get_platform_sessions_by_ids`。
sessions_from_db = await self.db.get_platform_sessions_by_ids(session_ids)
session_map = {s.session_id: s for s in sessions_from_db}
for sid in session_ids:
try:
session = session_map.get(sid)
if not session:
failed_items.append({"session_id": sid, "reason": "not found"})
continue
if session.creator != username:
failed_items.append(
{"session_id": sid, "reason": "permission denied"}
)
continue
await self._delete_session_internal(session, username)
deleted_count += 1
except Exception as e:
logger.warning(f"Failed to delete session {sid}: {e}")
failed_items.append({"session_id": sid, "reason": str(e)})|
@sourcery-ai review |
There was a problem hiding this comment.
Hey - 我发现了 1 个问题,并留了一些整体性的反馈:
- 在
batch_delete_sessions中,你记录并返回了通用的internal_error,但前端仍然会清空batchSelected并退出批量模式;建议在出错时保持批量模式继续激活,和/或将具体的failed_items暴露给 UI,让用户可以看到哪些会话失败并显式地重试它们。 - 帮助函数
_delete_session_internal(self, session, username: str)实际上并没有使用username;你可以删掉这个参数,以简化函数签名并避免混淆。
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `batch_delete_sessions`, you're logging and returning a generic `internal_error` but still clearing `batchSelected` and exiting batch mode on the frontend; consider keeping batch mode active and/or surfacing the specific `failed_items` to the UI so users can see which sessions failed and retry them explicitly.
- The helper `_delete_session_internal(self, session, username: str)` doesn't actually use `username`; you can drop this parameter to simplify the signature and avoid confusion.
## Individual Comments
### Comment 1
<location path="astrbot/dashboard/routes/chat.py" line_range="670-673" />
<code_context>
+ await self._delete_session_internal(session, username)
+ deleted_count += 1
+ sessions_by_id.pop(sid, None)
+ except Exception:
+ logger.warning("Failed to delete session %s", sid)
+ failed_items.append({"session_id": sid, "reason": "internal_error"})
+
</code_context>
<issue_to_address>
**suggestion:** The broad exception in `batch_delete_sessions` loses error details; logging the exception would aid debugging.
The broad `except Exception:` keeps the batch resilient, but the warning currently drops exception details. Please switch to `logger.exception("Failed to delete session %s", sid)` (or `logger.warning(..., exc_info=True)`) so stack traces are preserved while keeping external behavior unchanged.
```suggestion
try:
await self._delete_session_internal(session, username)
deleted_count += 1
sessions_by_id.pop(sid, None)
except Exception:
logger.exception("Failed to delete session %s", sid)
failed_items.append({"session_id": sid, "reason": "internal_error"})
```
</issue_to_address>帮我变得更有用!请在每条评论上点 👍 或 👎,我会根据你的反馈改进代码评审质量。
Original comment in English
Hey - I've found 1 issue, and left some high level feedback:
- In
batch_delete_sessions, you're logging and returning a genericinternal_errorbut still clearingbatchSelectedand exiting batch mode on the frontend; consider keeping batch mode active and/or surfacing the specificfailed_itemsto the UI so users can see which sessions failed and retry them explicitly. - The helper
_delete_session_internal(self, session, username: str)doesn't actually useusername; you can drop this parameter to simplify the signature and avoid confusion.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `batch_delete_sessions`, you're logging and returning a generic `internal_error` but still clearing `batchSelected` and exiting batch mode on the frontend; consider keeping batch mode active and/or surfacing the specific `failed_items` to the UI so users can see which sessions failed and retry them explicitly.
- The helper `_delete_session_internal(self, session, username: str)` doesn't actually use `username`; you can drop this parameter to simplify the signature and avoid confusion.
## Individual Comments
### Comment 1
<location path="astrbot/dashboard/routes/chat.py" line_range="670-673" />
<code_context>
+ await self._delete_session_internal(session, username)
+ deleted_count += 1
+ sessions_by_id.pop(sid, None)
+ except Exception:
+ logger.warning("Failed to delete session %s", sid)
+ failed_items.append({"session_id": sid, "reason": "internal_error"})
+
</code_context>
<issue_to_address>
**suggestion:** The broad exception in `batch_delete_sessions` loses error details; logging the exception would aid debugging.
The broad `except Exception:` keeps the batch resilient, but the warning currently drops exception details. Please switch to `logger.exception("Failed to delete session %s", sid)` (or `logger.warning(..., exc_info=True)`) so stack traces are preserved while keeping external behavior unchanged.
```suggestion
try:
await self._delete_session_internal(session, username)
deleted_count += 1
sessions_by_id.pop(sid, None)
except Exception:
logger.exception("Failed to delete session %s", sid)
failed_items.append({"session_id": sid, "reason": "internal_error"})
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| try: | ||
| await self._delete_session_internal(session, username) | ||
| deleted_count += 1 | ||
| sessions_by_id.pop(sid, None) |
There was a problem hiding this comment.
suggestion: 在 batch_delete_sessions 中过于宽泛的异常捕获会丢失错误细节;记录异常日志有助于调试。
宽泛的 except Exception: 能保证批量操作的鲁棒性,但当前的 warning 日志会丢掉异常细节。请切换为 logger.exception("Failed to delete session %s", sid)(或 logger.warning(..., exc_info=True)),这样可以保留堆栈信息,同时保持对外行为不变。
| try: | |
| await self._delete_session_internal(session, username) | |
| deleted_count += 1 | |
| sessions_by_id.pop(sid, None) | |
| try: | |
| await self._delete_session_internal(session, username) | |
| deleted_count += 1 | |
| sessions_by_id.pop(sid, None) | |
| except Exception: | |
| logger.exception("Failed to delete session %s", sid) | |
| failed_items.append({"session_id": sid, "reason": "internal_error"}) |
Original comment in English
suggestion: The broad exception in batch_delete_sessions loses error details; logging the exception would aid debugging.
The broad except Exception: keeps the batch resilient, but the warning currently drops exception details. Please switch to logger.exception("Failed to delete session %s", sid) (or logger.warning(..., exc_info=True)) so stack traces are preserved while keeping external behavior unchanged.
| try: | |
| await self._delete_session_internal(session, username) | |
| deleted_count += 1 | |
| sessions_by_id.pop(sid, None) | |
| try: | |
| await self._delete_session_internal(session, username) | |
| deleted_count += 1 | |
| sessions_by_id.pop(sid, None) | |
| except Exception: | |
| logger.exception("Failed to delete session %s", sid) | |
| failed_items.append({"session_id": sid, "reason": "internal_error"}) |
issue #6142
为webchat添加批量删除对话功能。
Modifications / 改动点
删除处理与前端实现:
astrbot/dashboard/routes/chat.py
dashboard/src/components/chat/Chat.vue
dashboard/src/composables/useSessions.ts
dashboard/src/components/chat/ConversationSidebar.vue
i18n:
dashboard/src/i18n/locales/en-US/features/chat.json
dashboard/src/i18n/locales/zh-CN/features/chat.json
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.Summary by Sourcery
在整个仪表盘 UI 和后端 API 中,为网页聊天会话添加批量删除支持。
新功能:
/api/chat/batch_delete_sessions接口,用于一次性删除多个会话,并向客户端报告每个会话的删除失败情况。增强内容:
测试:
batch_delete_sessions接口的测试,包括请求负载校验、内部错误屏蔽以及批量会话查询的使用。Original summary in English
Summary by Sourcery
Add batch deletion support for webchat conversations across the dashboard UI and backend APIs.
New Features:
Enhancements:
Tests:
新功能:
增强改进:
Original summary in English
Summary by Sourcery
在整个仪表盘 UI 和后端 API 中,为网页聊天会话添加批量删除支持。
新功能:
/api/chat/batch_delete_sessions接口,用于一次性删除多个会话,并向客户端报告每个会话的删除失败情况。增强内容:
测试:
batch_delete_sessions接口的测试,包括请求负载校验、内部错误屏蔽以及批量会话查询的使用。Original summary in English
Summary by Sourcery
Add batch deletion support for webchat conversations across the dashboard UI and backend APIs.
New Features:
Enhancements:
Tests: