主动榜单推送，群全量消息处理#189
Merged
Merged
Conversation
Walkthrough新增定时任务注册与激活链路，扩展 Changes定时主动消息调度框架与榜单推送
Sequence Diagram(s)sequenceDiagram
participant robot.py
participant activate_scheduled_jobs
participant RobotMessage
participant peeper.py
participant API
robot.py->>activate_scheduled_jobs: on_ready 激活已注册任务
activate_scheduled_jobs->>RobotMessage: setup_active_*_message(loop, target)
activate_scheduled_jobs->>peeper.py: 注册定时回调
peeper.py->>API: send_yesterday_board(message)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@config_example.json`:
- Around line 39-44: The example config placeholders for obot_apply_to and
obot_push_to are inconsistent with the required scene ID prefix contract. Update
the sample values in config_example.json to use valid prefixed IDs such as
guild_, direct_, group_, or c2c_, and make sure the surrounding comments for
obot_apply_to, obot_push_to, and obot_is_private still accurately describe the
configuration semantics.
In `@robot.py`:
- Around line 164-165: The scheduler thread started in robot.py via
threading.Thread(target=tasks_sched_thread, args=[]) should not keep the process
alive after client.run() exits or fails. Update that thread setup to give it a
clear name and set daemon=True so it won’t block shutdown, and keep the
scheduling logic in tasks_sched_thread/activate_scheduled_jobs unchanged; you
can add a later shutdown hook if needed.
In `@src/core/bot/decorator.py`:
- Around line 159-165: `get_scheduled_jobs_info` currently returns `job.target`
as-is, but the declared tuple type expects a string and `sorted(result)` can
fail when `None` and string targets mix. Normalize `job.target` inside
`get_scheduled_jobs_info` before appending to `result` by converting missing
targets to a consistent string value, and keep the returned tuple shape aligned
with the annotation so `sorted` only compares comparable values.
- Around line 86-88: The uuid prefix parsing in the target-id extraction logic
can return an empty target_id for values like guild_ or group_, which then
passes validation as if it were valid. Update the prefix-handling branch in the
uuid parsing helper in decorator.py so that it rejects empty suffixes and only
returns when the part after the prefix is non-empty; otherwise continue to the
normal invalid-UUID path. Use the existing uuid_prefix_map matching flow and the
target-id validation logic to ensure decorated task registration cannot proceed
with an empty target_id.
In `@src/core/bot/transit.py`:
- Line 8: Add the missing APScheduler dependency so the top-level CronTrigger
import in transit.py can resolve at startup. Update the project dependency
declaration in pyproject.toml to include APScheduler (matching the import used
by CronTrigger), and keep the import in src/core/bot/transit.py unchanged so the
bot no longer fails during module import.
In `@src/module/cp/peeper.py`:
- Around line 227-240: The scheduled push path in push_yesterday_board currently
loses the mapping between obot_push_to and the configured board because
_collect_push_targets() returns only UUIDs and send_yesterday_board() falls back
to message.uuid lookup in obot_apply_to. Update the peeper scheduling flow so
the target collection preserves the specific board configuration per UUID, and
make push_yesterday_board invoke that binding-aware path instead of blindly
reusing send_yesterday_board. Use _collect_push_targets, push_yesterday_board,
and send_yesterday_board as the main symbols to locate the fix.
- Around line 87-93: The UUID parsing path in the peeper config validation
currently allows an empty target ID when parse_uuid("group_") passes the prefix
check, so add an explicit check after parsing to reject empty target values
before continuing the loop over peeper.configs. Also update the RuntimeError
raised in this validation block to preserve the original ValueError using
exception chaining with from e, so the failure in the parse_uuid handling
remains traceable.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 827b2739-7971-4565-b2fd-19dc974ac1fc
📒 Files selected for processing (7)
config_example.jsonentry.pyrobot.pysrc/core/bot/decorator.pysrc/core/bot/message.pysrc/core/bot/transit.pysrc/module/cp/peeper.py
Owner
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/core/bot/transit.py (1)
244-251: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win避免不同定时任务使用同一个
job_id被覆盖。Line 249 只拼接了模块名、函数名和去前缀后的
target；同一函数注册多个纯定时任务，或不同MessageType恰好有相同目标 ID 时，Line 250 的replace_existing=True会静默替换前一个任务。建议修复
- for module_name, jobs in __scheduled_jobs__.items(): - for job in jobs: + for module_name, jobs in __scheduled_jobs__.items(): + for job_idx, job in enumerate(jobs): wrapper = _make_scheduled_wrapper( job.func, job.message_type, job.target, api, loop) trigger = CronTrigger.from_crontab(job.cron) - job_id = f"sched.{module_name}.{job.func.__name__}.{job.target or 'task'}" + message_type_name = job.message_type.name.lower() if job.message_type else "task" + job_id = ( + f"sched.{module_name}.{job.func.__name__}." + f"{job_idx}.{message_type_name}.{job.target or 'none'}" + ) scheduler.add_job(wrapper, trigger=trigger, id=job_id, replace_existing=True)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/core/bot/transit.py` around lines 244 - 251, The scheduled job identifier in transit.py is not unique enough, so distinct jobs can overwrite each other when scheduler.add_job is called with replace_existing=True. Update the job_id generation in the scheduling loop around _make_scheduled_wrapper and CronTrigger.from_crontab to include additional discriminators such as job.message_type and any target-specific identifier so each scheduled entry gets a stable unique id. Keep the change localized to the job registration logic in the __scheduled_jobs__ iteration.src/core/bot/message.py (1)
274-280: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win不要把
msg_id=None透传到请求体
_handle_send_request只按 key 组装参数，msg_id一旦写进params就会随requests.post(..., json=payload)发出并变成null。主动/回退消息如果接口要求省略msg_id，这里应只在回复场景写入该字段。🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/core/bot/message.py` around lines 274 - 280, 在 _pack_message_params 中，base_params 现在会无条件把 msg_id 写入请求参数并在 self._active 为真时变成 None，这会让 _handle_send_request 最终通过 requests.post 的 json payload 发送 null。请改成只在回复场景（例如 self._active 为 False 且确实有 message.id 时）才加入 msg_id，主动发送或回退场景直接省略该键；同时检查 _handle_send_request 的参数组装逻辑，确保不会把不存在的 msg_id 补成 null。
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src/core/bot/message.py`:
- Around line 274-280: 在 _pack_message_params 中，base_params 现在会无条件把 msg_id
写入请求参数并在 self._active 为真时变成 None，这会让 _handle_send_request 最终通过 requests.post 的
json payload 发送 null。请改成只在回复场景（例如 self._active 为 False 且确实有 message.id 时）才加入
msg_id，主动发送或回退场景直接省略该键；同时检查 _handle_send_request 的参数组装逻辑，确保不会把不存在的 msg_id 补成
null。
In `@src/core/bot/transit.py`:
- Around line 244-251: The scheduled job identifier in transit.py is not unique
enough, so distinct jobs can overwrite each other when scheduler.add_job is
called with replace_existing=True. Update the job_id generation in the
scheduling loop around _make_scheduled_wrapper and CronTrigger.from_crontab to
include additional discriminators such as job.message_type and any
target-specific identifier so each scheduled entry gets a stable unique id. Keep
the change localized to the job registration logic in the __scheduled_jobs__
iteration.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 9f1b74ff-1135-4b6d-909a-0aa402fbd689
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (5)
config_example.jsonpyproject.tomlrobot.pysrc/core/bot/message.pysrc/core/bot/transit.py
🚧 Files skipped from review as they are similar to previous changes (1)
- pyproject.toml
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/core/bot/decorator.py (1)
108-122: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win在注册阶段校验
no_target、targets和函数签名的一致性。这里现在会静默接受几种无效组合：
no_target=True时传了targets会被直接忽略，普通模式下空targets会悄悄注册 0 个任务；而transit.py下游会分别按func()/func(packed_message)调用，签名写错也要等到定时任务真正触发才会暴露。建议在装饰阶段直接拒绝这些组合，尽早把配置错误拦住。建议修复
def scheduled(cron: str, targets: list[str], no_target: bool = False): @@ def decorator(func): module_name = func.__module__ or "default.unknown" + signature = inspect.signature(func) __scheduled_jobs__.setdefault(module_name, []) if no_target: + if targets: + raise ValueError("Pure scheduled jobs must not define targets") + try: + signature.bind() + except TypeError as e: + raise ValueError( + f"Pure scheduled job {func.__name__} must not require a message parameter" + ) from e # 纯定时任务，不传递 message 参数 __scheduled_jobs__[module_name].append( ScheduledJobInfo(func=func, cron=cron, module_name=module_name)) else: + if not targets: + raise ValueError("Message scheduled jobs require at least one target") + try: + signature.bind(object()) + except TypeError as e: + raise ValueError( + f"Scheduled job {func.__name__} must accept exactly one message argument" + ) from e for uuid in targets: message_type, target_id = parse_uuid(uuid) __scheduled_jobs__[module_name].append( ScheduledJobInfo(func=func, cron=cron, message_type=message_type, target=target_id, module_name=module_name))import inspect🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/core/bot/decorator.py` around lines 108 - 122, The decorator registration logic in decorator(func) should validate the relationship between no_target, targets, and the wrapped function signature instead of silently accepting invalid combinations. Add an early check that rejects no_target=True with any targets, rejects the normal path when targets is empty, and uses inspect.signature on func to ensure the callable matches the expected call style for transit.py (func() versus func(packed_message)). Fail fast with a clear exception before appending to __scheduled_jobs__.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src/core/bot/decorator.py`:
- Around line 108-122: The decorator registration logic in decorator(func)
should validate the relationship between no_target, targets, and the wrapped
function signature instead of silently accepting invalid combinations. Add an
early check that rejects no_target=True with any targets, rejects the normal
path when targets is empty, and uses inspect.signature on func to ensure the
callable matches the expected call style for transit.py (func() versus
func(packed_message)). Fail fast with a clear exception before appending to
__scheduled_jobs__.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 4e2ea796-1a49-4a02-854e-5eca1526143c
📒 Files selected for processing (3)
src/core/bot/decorator.pysrc/core/bot/message.pysrc/module/cp/peeper.py
🚧 Files skipped from review as they are similar to previous changes (2)
- src/module/cp/peeper.py
- src/core/bot/message.py
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.
Summary by CodeRabbit
role.bot_id。obot_apply_to/obot_push_to的校验，并增加一致性要求。{日期} 卷王天梯榜。/开头时处理。