feat: automatically record UMO names - #9909
Conversation
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 3 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="astrbot/core/db/sqlite.py" line_range="76-78" />
<code_context>
await self._ensure_platform_message_history_checkpoint_column(conn)
await self._ensure_chatui_project_workspace_columns(conn)
await self._ensure_conversation_indexes(conn)
+ # The table-level unique constraint already provides an index for UMO
+ # lookups. Older schemas also created this redundant explicit index.
+ await conn.execute(text("DROP INDEX IF EXISTS ix_umo_aliases_umo"))
await conn.commit()
</code_context>
<issue_to_address>
**issue (broader_impact):** The initialization migration drops `ix_umo_aliases_umo` without creating the new table-level unique constraint on existing databases. For databases created by the previous schema, that explicit index can be the only uniqueness enforcement for `umo`; after it is dropped, the new `ON CONFLICT (umo)` upserts fail because SQLite has no matching unique constraint, and duplicate UMO rows can be inserted by other write paths.
**Triggers:** When upgrading an existing database whose `umo_aliases` table was created by the previous schema.
**Suggested fix:** Create or migrate the `uix_umo_alias_umo` unique constraint before dropping the legacy index, and only remove the old index after verifying that the constraint exists.
</issue_to_address>
### Comment 2
<location path="astrbot/core/umo_alias.py" line_range="24-42" />
<code_context>
-def get_event_auto_name(event: Any) -> str:
+def get_event_auto_name(event: Any, *, fallback_to_id: bool = True) -> str:
+ """Resolve an automatic display name from inbound event metadata.
+
+ Args:
+ event: Platform event containing group and sender metadata.
+ fallback_to_id: Whether to use the group or sender ID when no name exists.
+
+ Returns:
+ Normalized group or sender name, an optional ID fallback, or an empty string.
+ """
group_id = event.get_group_id() if hasattr(event, "get_group_id") else ""
message_obj = getattr(event, "message_obj", None)
group = getattr(message_obj, "group", None)
if group_id:
group_name = normalize_umo_name(getattr(group, "group_name", None))
- return group_name or normalize_umo_name(group_id)
+ if group_name:
+ return group_name
+ return normalize_umo_name(group_id) if fallback_to_id else ""
sender_name = ""
</code_context>
<issue_to_address>
**issue (bug_risk):** The helper treats any non-empty `event.get_group_id()` as proof that the event is a group message, so private events from adapters that populate `group_id` with a conversation or channel ID enter the group branch and never inspect `sender_name`. Those private messages therefore skip automatic naming or receive a group ID fallback instead of the private sender name.
**Triggers:** When a platform represents a private conversation with a non-empty group or conversation ID.
**Suggested fix:** Use the event/message type to select the group-name branch, and use `sender_name` for `FRIEND_MESSAGE` events regardless of whether `group_id` is populated.
</issue_to_address>
### Comment 3
<location path="astrbot/core/event_bus.py" line_range="80-82" />
<code_context>
+ auto_name = get_event_auto_name(event, fallback_to_id=False)
+ if not auto_name:
+ return
+ if self._umo_auto_name_cache.get(umo) == auto_name:
+ self._umo_auto_name_cache.move_to_end(umo)
+ return
+
+ self._umo_auto_name_cache[umo] = auto_name
</code_context>
<issue_to_address>
**issue (bug_risk):** An in-flight write is represented only by the optimistic cache entry. If the writer pops an entry, a second event with the same UMO and name arrives while the database call is failing, that event is skipped because the cache still matches; the failure then removes the cache entry and no pending write remains, so the second event does not trigger a retry.
**Triggers:** When the database write fails while another event for the same UMO and unchanged automatic name is dispatched concurrently.
**Suggested fix:** Track in-flight names separately or requeue the failed item unless a newer successfully queued value supersedes it.
```suggestion
if (
self._umo_auto_name_cache.get(umo) == auto_name
and umo in self._pending_umo_auto_names
):
self._umo_auto_name_cache.move_to_end(umo)
return
```
</issue_to_address>Sourcery assessment
Needs a human reviewer. 3 findings to address first, and this writes automatically discovered names into persistent UMO records and removes an existing SQLite index, so an incorrect name or schema assumption can survive a code revert. The names can be recomputed and records repaired, but dropping the index could also require a database fix if the expected uniqueness constraint is not present.
Blocking findings: astrbot/core/db/sqlite.py:78, astrbot/core/umo_alias.py:42, astrbot/core/event_bus.py:82
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| # The table-level unique constraint already provides an index for UMO | ||
| # lookups. Older schemas also created this redundant explicit index. | ||
| await conn.execute(text("DROP INDEX IF EXISTS ix_umo_aliases_umo")) |
There was a problem hiding this comment.
issue (broader_impact): The initialization migration drops ix_umo_aliases_umo without creating the new table-level unique constraint on existing databases. For databases created by the previous schema, that explicit index can be the only uniqueness enforcement for umo; after it is dropped, the new ON CONFLICT (umo) upserts fail because SQLite has no matching unique constraint, and duplicate UMO rows can be inserted by other write paths.
Triggers: When upgrading an existing database whose umo_aliases table was created by the previous schema.
Suggested fix: Create or migrate the uix_umo_alias_umo unique constraint before dropping the legacy index, and only remove the old index after verifying that the constraint exists.
| def get_event_auto_name(event: Any, *, fallback_to_id: bool = True) -> str: | ||
| """Resolve an automatic display name from inbound event metadata. | ||
|
|
||
| Args: | ||
| event: Platform event containing group and sender metadata. | ||
| fallback_to_id: Whether to use the group or sender ID when no name exists. | ||
|
|
||
| Returns: | ||
| Normalized group or sender name, an optional ID fallback, or an empty string. | ||
| """ | ||
| group_id = event.get_group_id() if hasattr(event, "get_group_id") else "" | ||
| message_obj = getattr(event, "message_obj", None) | ||
| group = getattr(message_obj, "group", None) | ||
|
|
||
| if group_id: | ||
| group_name = normalize_umo_name(getattr(group, "group_name", None)) | ||
| return group_name or normalize_umo_name(group_id) | ||
| if group_name: | ||
| return group_name | ||
| return normalize_umo_name(group_id) if fallback_to_id else "" |
There was a problem hiding this comment.
issue (bug_risk): The helper treats any non-empty event.get_group_id() as proof that the event is a group message, so private events from adapters that populate group_id with a conversation or channel ID enter the group branch and never inspect sender_name. Those private messages therefore skip automatic naming or receive a group ID fallback instead of the private sender name.
Triggers: When a platform represents a private conversation with a non-empty group or conversation ID.
Suggested fix: Use the event/message type to select the group-name branch, and use sender_name for FRIEND_MESSAGE events regardless of whether group_id is populated.
| if self._umo_auto_name_cache.get(umo) == auto_name: | ||
| self._umo_auto_name_cache.move_to_end(umo) | ||
| return |
There was a problem hiding this comment.
issue (bug_risk): An in-flight write is represented only by the optimistic cache entry. If the writer pops an entry, a second event with the same UMO and name arrives while the database call is failing, that event is skipped because the cache still matches; the failure then removes the cache entry and no pending write remains, so the second event does not trigger a retry.
Triggers: When the database write fails while another event for the same UMO and unchanged automatic name is dispatched concurrently.
Suggested fix: Track in-flight names separately or requeue the failed item unless a newer successfully queued value supersedes it.
| if self._umo_auto_name_cache.get(umo) == auto_name: | |
| self._umo_auto_name_cache.move_to_end(umo) | |
| return | |
| if ( | |
| self._umo_auto_name_cache.get(umo) == auto_name | |
| and umo in self._pending_umo_auto_names | |
| ): | |
| self._umo_auto_name_cache.move_to_end(umo) | |
| return |
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
astrbot-docs | bfc1789 | Commit Preview URL Branch Preview URL |
Sep 01 2026, 03:22 PM |
1 participant
Summary
Behavior
Validation
Summary by Sourcery
Automatically persist platform-provided UMO names when messages wake AstrBot while preserving manual aliases and keeping private-message metadata accurate.
New Features:
Bug Fixes:
Enhancements:
Tests:
Chores: