fix: use UMO-bound config for group_icl_enable in on_message#7397
fix: use UMO-bound config for group_icl_enable in on_message#7397saschabuehrle wants to merge 1 commit intoAstrBotDevs:masterfrom
Conversation
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- When switching
get_configto useumo=event.unified_msg_origin, consider guarding against missing or partial config (e.g., using.getwith sensible defaults) to avoid runtime errors if the UMO-bound profile lacksprovider_ltm_settingsorgroup_icl_enable.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- When switching `get_config` to use `umo=event.unified_msg_origin`, consider guarding against missing or partial config (e.g., using `.get` with sensible defaults) to avoid runtime errors if the UMO-bound profile lacks `provider_ltm_settings` or `group_icl_enable`.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 updates the configuration retrieval in the on_message method to include the unified message origin (umo). The reviewer suggests optimizing performance by fetching the configuration once at the beginning of the method to avoid redundant lookups across multiple calls.
| group_icl_enable = self.context.get_config( | ||
| umo=event.unified_msg_origin | ||
| )["provider_ltm_settings"]["group_icl_enable"] |
There was a problem hiding this comment.
The configuration for the current UMO is fetched multiple times in this method (via ltm_enabled, need_active_reply, and again here). While this PR correctly fixes the UMO binding, consider fetching the configuration once at the beginning of on_message and reusing it to avoid redundant lookups and improve efficiency.
Fixes #7305
on_message()currently readsgroup_icl_enablefrom the default config viaself.context.get_config().When a platform instance is bound to a non-default profile, this can disable group context recording even though
ltm_enabled()correctly reads the UMO-bound config.This patch makes
on_message()readgroup_icl_enablewith the sameumo=event.unified_msg_originlookup path used byltm_enabled().Greetings, saschabuehrle
Summary by Sourcery
Bug Fixes: