fix:修改了老版本和新版本的数据库兼容问题,修改了批量获取历史记录时要进行n次检测的问题 - #10
Merged
Conversation
Reviewer's GuideUpdates database initialization for backward-compatible schema refresh, optimizes bulk history backfill to avoid per-message DB lookups and duplicates, and parallelizes avatar downloads in the renderer for better performance, plus a minor UX tweak to the history retrieval command output. Sequence diagram for optimized history backfill and batch message_id existence checksequenceDiagram
actor User
participant Bot as MessageHandler
participant Repo as Repository
participant DB as Database
User->>Bot: backfill_from_history(group_id, messages)
Bot->>Bot: sort messages by time
Bot->>Bot: collect all_msg_ids from messages
Bot->>Repo: filter_existing_message_ids(all_msg_ids)
Repo->>DB: get_session()
Repo->>DB: select(MessageOwnerIndex.message_id).where(message_id.in_(all_msg_ids))
DB-->>Repo: existing message_id rows
Repo-->>Bot: existed_msg_ids set
Bot->>Bot: init seen_msg_ids set
loop for each msg in sorted_messages
Bot->>Bot: validate date and extract msg_id
alt msg_id already in seen_msg_ids
Bot->>Bot: skip duplicated input message
else msg_id not in seen_msg_ids
Bot->>Bot: add msg_id to seen_msg_ids
alt msg_id in existed_msg_ids
Bot->>Bot: update group_last_time
Bot->>Bot: continue (skip DB lookup)
else msg_id not in existed_msg_ids
Bot->>Repo: get_message_owner(msg_id)
Repo->>DB: query MessageOwnerIndex by msg_id
DB-->>Repo: owner or None
Repo-->>Bot: owner or None
Bot->>Bot: process message, update history and topic
end
end
end
Updated class diagram for database, repository, handler, and renderer changesclassDiagram
class Database {
+engine
+init_db() async
}
class UserCooldown {
+user_id: str
+group_id: str
+created_at
+updated_at
}
class MessageOwnerIndex {
+message_id: str
+user_id: str
+group_id: str
+created_at
}
class Repository {
+db: Database
+check_and_update_cooldown(user_id: str, group_id: str) async
+batch_backfill(group_id: str, messages: list[dict]) async
+filter_existing_message_ids(message_ids: list[str]) async set~str~
+get_message_owner(message_id: str) async
}
class MessageHandler {
+repo: Repository
+backfill_from_history(group_id: str, messages: list[dict]) async
+retrieve_historical_records(event: AiocqhttpMessageEvent) async
}
class Renderer {
+theme_manager
+env
+render(data: dict, theme_name: str) async str
}
Database o-- UserCooldown
Database o-- MessageOwnerIndex
Repository --> Database
MessageHandler --> Repository
MessageHandler --> Renderer
note for Database "init_db creates tables, checks user_cooldown schema, recreates if mismatched, applies SQLite PRAGMA and index creation"
note for Repository "filter_existing_message_ids performs a single batched SELECT of existing message_id values"
note for MessageHandler "backfill_from_history now batch-checks existing messages and deduplicates by message_id"
note for Renderer "render uses shared _fetch_avatar and asyncio.gather to parallelize avatar downloads"
Class diagram for init_db schema compatibility logicclassDiagram
class Database {
+engine
+init_db() async
-_needs_refresh(sync_conn) bool
}
class SQLModelMetadata {
+create_all(bind)
}
class Inspector {
+get_table_names() list~str~
+get_columns(table_name: str) list
}
class UserCooldown {
+__table__
}
class Table {
+columns: dict
+drop(bind, checkfirst: bool)
+create(bind, checkfirst: bool)
}
Database --> SQLModelMetadata : uses
Database --> Inspector : uses
Database --> UserCooldown : manages schema
UserCooldown --> Table : has
note for Database "init_db creates all tables, inspects user_cooldown, and drops/recreates the table when columns do not match the model, ensuring compatibility between old and new schemas"
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![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 2 issues, and left some high level feedback:
- In
renderer.render,_fetch_avataris defined inside the main-avatarif avatar_url and avatar_url.startswith("http")block but is later used for deep_dive avatars unconditionally, which will raise a NameError whenavatar_urlis absent or not http; move_fetch_avatarto a broader scope so it’s always defined. - In
backfill_from_history,all_msg_idsare converted tostrbefore callingfilter_existing_message_ids, but later comparisons use the rawmsg_id(possibly int) againstexisted_msg_ids(set of str), so normalizemsg_idto string before membership checks to ensure deduplication works correctly. - The new
_needs_refreshlogic ininit_dbdrops and recreates theuser_cooldowntable whenever the column set changes, which may cause unintended data loss; consider a more targeted migration (e.g., ALTER TABLE) or at least narrowing the condition to only expected schema changes.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `renderer.render`, `_fetch_avatar` is defined inside the main-avatar `if avatar_url and avatar_url.startswith("http")` block but is later used for deep_dive avatars unconditionally, which will raise a NameError when `avatar_url` is absent or not http; move `_fetch_avatar` to a broader scope so it’s always defined.
- In `backfill_from_history`, `all_msg_ids` are converted to `str` before calling `filter_existing_message_ids`, but later comparisons use the raw `msg_id` (possibly int) against `existed_msg_ids` (set of str), so normalize `msg_id` to string before membership checks to ensure deduplication works correctly.
- The new `_needs_refresh` logic in `init_db` drops and recreates the `user_cooldown` table whenever the column set changes, which may cause unintended data loss; consider a more targeted migration (e.g., ALTER TABLE) or at least narrowing the condition to only expected schema changes.
## Individual Comments
### Comment 1
<location> `src/visual/renderer.py:39-48` </location>
<code_context>
+ async def _fetch_avatar(url: str) -> str:
</code_context>
<issue_to_address>
**issue (bug_risk):** _fetch_avatar is only defined when main avatar_url is http, but used unconditionally for deep_dive avatars
If `avatar_url` is missing or not an HTTP URL, `_fetch_avatar` will be undefined when the deep_dive avatars are processed, leading to a runtime `NameError`. Define `_fetch_avatar` once in a broader scope (e.g., at the start of the `async with aiohttp.ClientSession()` block or as a private method) so it’s always available before use.
</issue_to_address>
### Comment 2
<location> `src/visual/renderer.py:61-62` </location>
<code_context>
- if dialog.get("avatar_url") and dialog["avatar_url"].startswith(
- "data:"
- ):
+ # 跳过已经是 base64 或无 uid 的
+ if dialog.get("avatar_url", "").startswith("data:") or not uid:
continue
-
</code_context>
<issue_to_address>
**question (bug_risk):** Dialogs without uid no longer get DEFAULT_AVATAR assigned
Previously, `not uid` dialogs explicitly had `dialog["avatar_url"] = DEFAULT_AVATAR` in the `else` branch. Now they’re skipped via `if ... or not uid: continue`, so they keep whatever `avatar_url` they had (possibly empty/missing). If you still want a default avatar when `uid` is missing, you’ll need to preserve that explicit `DEFAULT_AVATAR` assignment for `not uid` cases.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Owner
|
LGTM 其他的问题根据 AI review 建议改改 |
Collaborator
Author
批量过滤哪里,表里面就是str,所以转变为str然后查没问题,这ai看代码不看全的 class MessageOwnerIndex(SQLModel, table=True):
"""消息归属索引，用于将后续的 Reaction 归因到具体的发送者"""
__tablename__ = "message_owner_index"
__table_args__ = {"extend_existing": True}
message_id: str = Field(primary_key=True)
user_id: str
group_id: str
timestamp: float # 时间戳message_id在表里面查 # 提前收集 message_id，并批量查询已存在的 message
all_msg_ids = [
str(m.get("message_id"))
for m in sorted_messages
if m.get("message_id")
]
existed_msg_ids = await self.repo.filter_existing_message_ids(all_msg_ids)新增加的数据库查重函数 async def filter_existing_message_ids(
self, message_ids: list[str]
) -> set[str]:
"""
批量查询已存在的 message_id
返回：已存在的 message_id 集合
"""
if not message_ids:
return set()
async with self.db.get_session() as session:
message_id_col = cast(ColumnElement[str], MessageOwnerIndex.message_id)
stmt = select(MessageOwnerIndex.message_id).where(
message_id_col.in_(message_ids)
)
result = await session.execute(stmt)
return {row[0] for row in result.all()}去重和判断是否在表里面的逻辑 msg_id = str(msg.get("message_id", ""))
if not msg_id:
continue
# message_id 去重
if msg_id in seen_msg_ids:
continue
seen_msg_ids.add(msg_id)
# 使用批量查询结果判断是否已存在
if msg_id in existed_msg_ids:
group_last_time = msg_time
continue老表清理那里,老表没有群聊id字段迁移到新表甚至有可能会导致新表出现奇怪的问题 头像异步函数哪里确实有问题,我把函数定义移出去了,现在覆盖所有代码了 |
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.
运行示例
数据库兼容问题是在init_db里面加入了表的key检测,会自动删除老库生新表
新的init_db代码
今日人设图可以正常运行
学习接口也可以正常运行