fix: enable default message prompts - #30
Merged
Merged
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (11)
💤 Files with no reviewable changes (1)
📝 WalkthroughWalkthrough将所有内置消息提示（fetching、found、send_failed 等）的 Changes内置消息提示默认启用 & 移除 Playwright 依赖
估算代码审查工作量🎯 3 (Moderate) | ⏱️ ~20 minutes 诗
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
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 |
Contributor
Reviewer's Guide默认启用所有内置的面向用户的消息并提供合理的默认文案，自动为配置中缺失的消息文本回填默认内容，并更新测试和文档以匹配新的默认行为，同时将传输层测试与投递通知隔离。 File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your Experience打开你的 dashboard 以：
Getting HelpOriginal review guide in EnglishReviewer's GuideEnables all built-in user-facing messages by default with sensible default texts, adds automatic backfilling of missing message texts in configuration, and updates tests and documentation to match the new defaults while isolating transport tests from delivery notices. File-Level Changes
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}
Contributor
There was a problem hiding this comment.
Hey - 我发现了 1 个问题，并给出了一些整体反馈：
- 目前默认消息文案及其键同时出现在
_DEFAULT_TEXTS、各个*Config的默认值、MessageOverrideConfig、测试辅助方法（例如without_delivery_notices）以及 fixtures 中，形成了重复。建议将这些字符串集中在单一可信来源中，以避免不同步并简化后续更新。 - 由于
MessagesConfig._DEFAULT_TEXTS旨在完整反映所有可配置的消息键（包括与运势相关的键），你可能需要添加一个小的防护（例如测试或静态检查），用来断言这张映射表与 schema/选项列表保持一致，从而防止新增键时没有对应默认值。
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The default message texts and keys are now duplicated across `_DEFAULT_TEXTS`, the individual `*Config` defaults, `MessageOverrideConfig`, test helpers (e.g. `without_delivery_notices`), and fixtures; consider centralizing these strings in a single source of truth to avoid drift and ease future updates.
- Since `MessagesConfig._DEFAULT_TEXTS` is intended to mirror all configurable message keys (including fortune-related ones), you may want to add a small guard (e.g. a test or static check) that asserts this map is consistent with the schema/options list, so new keys cannot be added without a corresponding default.
## Individual Comments
### Comment 1
<location path="tests/shared/test_config_models.py" line_range="81" />
<code_context>
+ assert config.resolve_message("send_failed") == "图片发送失败，请稍后再试。"
+
+
+def test_message_defaults_fill_text_when_message_object_omits_text() -> None:
+ config = SetuPluginConfig(
+ messages={
</code_context>
<issue_to_address>
**issue (testing):** Add a test to cover the explicit empty-text case where defaults must not override admin-provided empty strings.
The validator must distinguish between omitted `text` (where defaults apply) and explicitly empty `text` (which must be preserved). The current test only covers the omitted case. Please add a complementary test where `text` is set to `""` for a couple of message keys and assert that `resolve_message(...)` returns `""`, confirming that defaults are not injected over admin-provided empty strings.
</issue_to_address>帮我变得更有用！请在每条评论上点 👍 或 👎，我会根据你的反馈改进后续评审。
Original comment in English
Hey - I've found 1 issue, and left some high level feedback:
- The default message texts and keys are now duplicated across
_DEFAULT_TEXTS, the individual*Configdefaults,MessageOverrideConfig, test helpers (e.g.without_delivery_notices), and fixtures; consider centralizing these strings in a single source of truth to avoid drift and ease future updates. - Since
MessagesConfig._DEFAULT_TEXTSis intended to mirror all configurable message keys (including fortune-related ones), you may want to add a small guard (e.g. a test or static check) that asserts this map is consistent with the schema/options list, so new keys cannot be added without a corresponding default.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The default message texts and keys are now duplicated across `_DEFAULT_TEXTS`, the individual `*Config` defaults, `MessageOverrideConfig`, test helpers (e.g. `without_delivery_notices`), and fixtures; consider centralizing these strings in a single source of truth to avoid drift and ease future updates.
- Since `MessagesConfig._DEFAULT_TEXTS` is intended to mirror all configurable message keys (including fortune-related ones), you may want to add a small guard (e.g. a test or static check) that asserts this map is consistent with the schema/options list, so new keys cannot be added without a corresponding default.
## Individual Comments
### Comment 1
<location path="tests/shared/test_config_models.py" line_range="81" />
<code_context>
+ assert config.resolve_message("send_failed") == "图片发送失败，请稍后再试。"
+
+
+def test_message_defaults_fill_text_when_message_object_omits_text() -> None:
+ config = SetuPluginConfig(
+ messages={
</code_context>
<issue_to_address>
**issue (testing):** Add a test to cover the explicit empty-text case where defaults must not override admin-provided empty strings.
The validator must distinguish between omitted `text` (where defaults apply) and explicitly empty `text` (which must be preserved). The current test only covers the omitted case. Please add a complementary test where `text` is set to `""` for a couple of message keys and assert that `resolve_message(...)` returns `""`, confirming that defaults are not injected over admin-provided empty strings.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| assert config.resolve_message("send_failed") == "图片发送失败，请稍后再试。" | ||
|
|
||
|
|
||
| def test_message_defaults_fill_text_when_message_object_omits_text() -> None: |
Contributor
There was a problem hiding this comment.
issue (testing): 请添加一个测试，覆盖“显式设置空文本”的场景，在该场景中默认值不应覆盖管理员提供的空字符串。
验证逻辑必须区分省略 text（此时应应用默认值）和显式为空的 text（此时必须保留）。目前的测试只覆盖了省略 text 的情况。请补充一个测试：为若干条消息键将 text 设置为 ""，并断言 resolve_message(...) 返回 ""，以确认不会在管理员提供的空字符串之上注入默认值。
Original comment in English
issue (testing): Add a test to cover the explicit empty-text case where defaults must not override admin-provided empty strings.
The validator must distinguish between omitted text (where defaults apply) and explicitly empty text (which must be preserved). The current test only covers the omitted case. Please add a complementary test where text is set to "" for a couple of message keys and assert that resolve_message(...) returns "", confirming that defaults are not injected over admin-provided empty strings.
移除未使用的 playwright 依赖（requirements.txt），版本升至 v2.1.1。经全仓库引用面诊断确认插件代码零 playwright 引用，运势渲染走 AstrBot html_renderer.render_custom_template() 的 network t2i 终结点（HTTP），本插件进程无需 Playwright。渲染行为与失败语义不变。
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
1 participant
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.
Modifications / 改动点
This is NOT a breaking change. / 这不是一个破坏性变更。
将所有内置提示消息默认改为
enabled=true，并保留可通过message_overrides关闭或改写单条提示。为
MessagesConfig增加缺省文本补全：当某条内置提示配置只写enabled、省略text时，自动使用内置默认文案；管理员显式写空文本时不覆盖。更新
_conf_schema.json、README、配置文档和 2.1.0 changelog 中关于提示默认值的说明。更新配置模型和发送链路测试，传输类测试显式关闭交付提示以隔离测试目标。
Screenshots or Test Results / 运行截图或测试结果
Checklist / 检查清单
😊 If there are new features added in the PR, I have discussed them 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.txt./ 我确保没有引入新依赖库，或者引入了新依赖库的同时将其添加到
requirements.txt文件相应位置。😮 My changes do not introduce malicious code.
/ 我的更改没有引入恶意代码。
Summary by Sourcery
默认启用内置的用户可见消息，并确保配置和文档反映新的默认行为，同时保留对单条消息进行覆写的能力。
New Features:
enabled标志时，为所有内置消息提供默认文案；如果未提供文本，则回退到内部默认值。Enhancements:
Documentation:
Tests:
Original summary in English
Summary by Sourcery
Enable built-in user-facing messages by default and ensure configuration and docs reflect the new defaults while preserving per-message override behavior.
New Features:
Enhancements:
Documentation:
Tests:
Summary by CodeRabbit
新功能
改进
文档
Chores