feat(stats): add per-command trigger statistics to dashboard#8488
feat(stats): add per-command trigger statistics to dashboard#8488KBVsent wants to merge 2 commits into
Conversation
There was a problem hiding this comment.
Hey - I've found 3 issues, and left some high level feedback:
- In
insert_command_statsyou usedatetime.now()without timezone awareness while other stats endpoints normalize timestamps to UTC; consider using a consistent UTC-aware timestamp helper here to avoid mixed/ambiguous times in the DB and queries likeget_top_commands. - The fire-and-forget
asyncio.create_task(db_helper.insert_command_stats(...))instar_request.pyhas no error handling, so failed inserts will be silently swallowed; consider wrapping this in a small background helper that catches and logs exceptions or batches inserts to avoid unobserved task errors.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `insert_command_stats` you use `datetime.now()` without timezone awareness while other stats endpoints normalize timestamps to UTC; consider using a consistent UTC-aware timestamp helper here to avoid mixed/ambiguous times in the DB and queries like `get_top_commands`.
- The fire-and-forget `asyncio.create_task(db_helper.insert_command_stats(...))` in `star_request.py` has no error handling, so failed inserts will be silently swallowed; consider wrapping this in a small background helper that catches and logs exceptions or batches inserts to avoid unobserved task errors.
## Individual Comments
### Comment 1
<location path="dashboard/src/views/stats/StatsPage.vue" line_range="131" />
<code_context>
+ <strong class="command-count">{{ formatNumber(command.count) }} {{ t('commandRanking.times') }}</strong>
+ </div>
+ </div>
+ <div v-else class="empty-state">{{ t('empty.commandStats') }}</div>
+ </section>
+
</code_context>
<issue_to_address>
**suggestion:** The new command ranking card will show the "empty" state briefly during initial load, which may be misleading.
Because `commandStats` starts as an empty array and this `v-else` doesn’t depend on the existing `loading` flag, the empty label will show until the `/api/stat/top-commands` request finishes. If other parts of the page are still loading, it can look like a real "no data" state.
Consider updating the condition so this block only renders when `!loading && !commandStats.length`, or replace it with a small skeleton/placeholder that matches the rest of the stats page loading behavior.
Suggested implementation:
```
</div>
<div v-else-if="!loading" class="empty-state">{{ t('empty.commandStats') }}</div>
</section>
```
This assumes the page already has a `loading` boolean used for other stats sections. If the flag is named differently (e.g. `isLoadingStats`, `loadingStats`), replace `loading` in the `v-else-if` condition with the correct property name to keep the empty state from flashing during initial load.
</issue_to_address>
### Comment 2
<location path="astrbot/core/db/sqlite.py" line_range="167-176" />
<code_context>
...
+ @abc.abstractmethod
+ async def insert_command_stats(
+ self,
+ command_name: str,
</code_context>
<issue_to_address>
**suggestion (bug_risk):** The timestamp used for command stats is naive and local-time dependent, which can cause inconsistencies across deployments.
This relies on `datetime.now().replace(...)`, which returns a naive local-time value. If other parts of the system use UTC/aware datetimes, this can cause misaligned hourly buckets (especially around DST or server timezone changes). Consider normalizing to UTC (e.g. `datetime.now(datetime.timezone.utc)` or `datetime.utcnow()`) and ensure this matches how other stats timestamps are generated so behavior is consistent across tables.
</issue_to_address>
### Comment 3
<location path="astrbot/core/pipeline/process_stage/method/star_request.py" line_range="59-47" />
<code_context>
+ if complete_names:
+ command_name = complete_names[0]
+ break
+ if command_name:
+ asyncio.create_task(
+ db_helper.insert_command_stats(
+ command_name=command_name,
+ plugin_name=md.name or "",
+ ),
+ )
+
try:
</code_context>
<issue_to_address>
**suggestion (bug_risk):** The fire-and-forget `insert_command_stats` task can fail silently; consider adding error handling or a helper.
Because this task is not awaited, any exception in `insert_command_stats` will surface only as an unhandled task exception (or might be missed entirely depending on loop policy). Consider a small wrapper that awaits `insert_command_stats` inside try/except with logging, or routing this through a shared background-task helper that consistently logs failures while keeping the main handler non-blocking.
</issue_to_address>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 introduces a feature to track and display command trigger statistics by adding a new CommandStat table, tracking command invocations in the processing pipeline, exposing a new /stat/top-commands API endpoint, and displaying a Command Usage Ranking card on the dashboard. The review feedback highlights two key issues: first, in star_request.py, background tasks created via asyncio.create_task lack strong references, which could lead to them being garbage collected before completion, so storing these tasks in a module-level set is recommended; second, in stat.py, query parameters are parsed into integers outside of the try-except block, which can cause unhandled 500 errors if invalid inputs are supplied, so moving these conversions inside the try block is advised.
Related issues: #8463
The dashboard statistics page only tracked aggregate message volume per platform, with no visibility into which plugin commands users actually invoke. This makes it hard to understand command usage and identify the most popular features. This PR adds per-command trigger statistics and surfaces the most frequently used commands on the WebUI statistics page.
Modifications / 改动点
astrbot/core/db/po.py: Add thecommand_statstable, aggregating triggers per hour with a unique key of (timestamp, plugin_name, command_name) so the same command name registered by different plugins is counted separately with accurate attribution.astrbot/core/db/__init__.py: Add abstractinsert_command_statsandget_top_commandsmethods to the database interface.astrbot/core/db/sqlite.py: Implement the UPSERT-increment insert and the ranking query grouped by (plugin_name, command_name), ordered by count.astrbot/core/pipeline/process_stage/method/star_request.py: Record a trigger when a command / command-group handler is actually executed. Aliases are merged into the canonical command name; the insert runs as a fire-and-forget task so it never blocks message handling.astrbot/dashboard/routes/stat.py: ExposeGET /api/stat/top-commandsreturning the ranked commands for the selected time range.dashboard/src/views/stats/StatsPage.vue: Add a "Command Usage Ranking" card using a responsive multi-column grid with a scrollable height cap, so it stays compact even with many commands; the owning plugin is shown as a badge.dashboard/src/i18n/locales/{zh-CN,en-US,ru-RU}/features/stats.json: Add the i18n strings for the new card.Screenshots or Test Results / 运行截图或测试结果
Checklist / 检查清单
😊 If there are new features added in the PR, I have discussed it with the authors through issues/emails, etc.
/ 如果 PR 中有新加入的功能,已经通过 Issue / 邮件等方式和作者讨论过。
👀 My changes have been well-tested, and "Verification Steps" and "Screenshots" have been provided above.
/ 我的更改经过了良好的测试,并已在上方提供了“验证步骤”和“运行截图”。
🤓 I have ensured that no new dependencies are introduced, OR if new dependencies are introduced, they have been added to the appropriate locations in
requirements.txtandpyproject.toml./ 我确保没有引入新依赖库,或者引入了新依赖库的同时将其添加到
requirements.txt和pyproject.toml文件相应位置。😮 My changes do not introduce malicious code.
/ 我的更改没有引入恶意代码。
Summary by Sourcery
Add collection and exposure of per-command trigger statistics and display them on the dashboard statistics page as a command usage ranking.
New Features:
Enhancements: