Skip to content

fix: prefer named weekday cron examples#6091

Open
stablegenius49 wants to merge 1 commit intoAstrBotDevs:masterfrom
stablegenius49:pr-factory/issue-5680-cron-dow-guidance
Open

fix: prefer named weekday cron examples#6091
stablegenius49 wants to merge 1 commit intoAstrBotDevs:masterfrom
stablegenius49:pr-factory/issue-5680-cron-dow-guidance

Conversation

@stablegenius49
Copy link

@stablegenius49 stablegenius49 commented Mar 12, 2026

Summary

  • update the create_future_task cron parameter description to recommend named weekdays like mon-fri / sat,sun
  • keep the existing generic cron example while adding an explicit named-weekday example
  • add a unit test covering the new guidance so the tool metadata does not regress

Testing

  • PYTHONPATH=. pytest tests/unit/test_cron_tools.py tests/unit/test_cron_manager.py --test-profile=blocking

Closes #5680.

Summary by Sourcery

在 cron 工具的元数据中澄清 cron 调度说明,并添加测试覆盖来强制执行新的描述。

增强内容:

  • 更新 cron 表达式参数的描述,以优先使用命名的星期格式,而不是数字的星期范围,从而提高清晰度和可移植性。

测试:

  • 添加单元测试,确保 cron 工具的说明持续推荐使用命名的星期形式,并提及数字范围存在的歧义。
Original summary in English

Summary by Sourcery

Clarify cron schedule guidance in the cron tool metadata and add test coverage to enforce the new description.

Enhancements:

  • Update the cron expression parameter description to prefer named weekday notation over numeric day-of-week ranges for clarity and portability.

Tests:

  • Add a unit test ensuring the cron tool description continues to recommend named weekdays and mention ambiguity of numeric ranges.

@gemini-code-assist
Copy link
Contributor

Warning

You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again!

@dosubot dosubot bot added the size:S This PR changes 10-29 lines, ignoring generated files. label Mar 12, 2026
@dosubot
Copy link

dosubot bot commented Mar 12, 2026

Related Documentation

1 document(s) may need updating based on files changed in this PR:

AstrBotTeam's Space

pr4697的改动
View Suggested Changes
@@ -332,6 +332,8 @@
 
 #### 相关工具
 - `create_future_task`(原 create_cron_job):创建定时任务
+  - `cron_expression` 参数:使用标准 cron 表达式定义定时任务的执行周期,例如 `'0 8 * * *'`(每天早上8点)或 `'0 23 * * mon-fri'`(工作日晚上11点)
+  - **Cron 表达式建议**:推荐使用命名星期格式(如 `mon-fri` 或 `sat,sun`)而非数字范围(如 `1-5`),以避免不同 cron 实现之间的歧义。例如,`'0 23 * * mon-fri'` 比 `'0 23 * * 1-5'` 更清晰明确
 - `delete_future_task`:删除定时任务
 - `list_future_tasks`:列出所有定时任务
 

[Accept] [Decline]

Note: You must be authenticated to accept/decline updates.

How did I do? Any feedback?  Join Discord

Copy link
Contributor

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey - 我发现了两个问题,并给出了一些高层面的反馈:

  • 新增的测试与描述文案的具体措辞强耦合(例如断言像 '1-5' 和 'avoid ambiguity' 这样的字面子串);建议放宽这些断言,把重点放在“包含对具名工作日的指引”上,而不是特定的措辞,这样将来对文案做微调时,就不会无谓地导致测试失败。
  • 测试名 test_create_future_task_cron_description_prefers_named_weekdays 仍然引用了 create_future_task,而当前被测工具是 CreateActiveCronTool;将测试重命名为与实际工具名一致,会让意图更清晰。
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- The new test tightly couples to the exact wording of the description (e.g., asserting literal substrings like '1-5' and 'avoid ambiguity'); consider relaxing these expectations to focus on the presence of named weekday guidance rather than specific phrasing so future wording tweaks don't break the test needlessly.
- The test name `test_create_future_task_cron_description_prefers_named_weekdays` still refers to `create_future_task`, while the tool under test is `CreateActiveCronTool`; renaming the test to match the actual tool name would make intent clearer.

## Individual Comments

### Comment 1
<location path="astrbot/core/tools/cron_tools.py" line_range="33" />
<code_context>
                 "cron_expression": {
                     "type": "string",
-                    "description": "Cron expression defining recurring schedule (e.g., '0 8 * * *').",
+                    "description": "Cron expression defining recurring schedule (e.g., '0 8 * * *' or '0 23 * * mon-fri'). Prefer named weekdays like 'mon-fri' or 'sat,sun' instead of numeric day-of-week ranges such as '1-5' to avoid ambiguity across cron implementations.",
                 },
                 "run_at": {
</code_context>
<issue_to_address>
**suggestion:** Clarify which cron dialect is expected rather than relying on generic implementation differences.

The note about named weekdays helps, but users still won’t know which cron semantics we follow (POSIX, Quartz, or a specific library). Please explicitly state the cron flavor or parser we use (e.g., "POSIX-style 5-field cron" or the library name) so users can rely on the correct external docs and avoid ambiguities like `0` vs `7` for Sunday or differing range behavior.

Suggested implementation:

```python
                    "description": "Cron expression defining recurring schedule using standard 5-field POSIX-style cron syntax (minute hour day-of-month month day-of-week), as parsed by this service's cron scheduler. For example: '0 8 * * *' or '0 23 * * mon-fri'. Prefer named weekdays like 'mon-fri' or 'sat,sun' instead of numeric day-of-week ranges such as '1-5' to avoid ambiguity around Sunday handling (0 vs 7) and differing cron implementations.",

```

If your project uses a specific cron library (e.g., `croniter`, `APScheduler`, `celery`, or a bespoke parser), you should further refine the description to name it explicitly. For example, replace “this service's cron scheduler” with “the `croniter` parser” or “APScheduler’s cron trigger semantics” so users can reference the appropriate external documentation.
</issue_to_address>

### Comment 2
<location path="tests/unit/test_cron_tools.py" line_range="6-15" />
<code_context>
+from astrbot.core.tools.cron_tools import CreateActiveCronTool
+
+
+def test_create_future_task_cron_description_prefers_named_weekdays():
+    """The cron tool should steer users toward unambiguous named weekdays."""
+    tool = CreateActiveCronTool()
+
+    description = tool.parameters["properties"]["cron_expression"]["description"]
+
+    assert "mon-fri" in description
+    assert "sat,sun" in description
+    assert "1-5" in description
+    assert "avoid ambiguity" in description
</code_context>
<issue_to_address>
**suggestion (testing):** Consider also asserting that the original generic cron example is preserved as described in the PR summary.

The PR description specifies that the original generic example (e.g. `0 8 * * *`) should remain in addition to the new named weekday guidance, but this test only checks the new guidance. Adding an assertion like `assert "0 8 * * *" in description` would ensure both parts of the requirement are covered and better protect against regressions.
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
帮我变得更有用!请在每条评论上点 👍 或 👎,我会根据你的反馈改进后续的代码审查。
Original comment in English

Hey - I've found 2 issues, and left some high level feedback:

  • The new test tightly couples to the exact wording of the description (e.g., asserting literal substrings like '1-5' and 'avoid ambiguity'); consider relaxing these expectations to focus on the presence of named weekday guidance rather than specific phrasing so future wording tweaks don't break the test needlessly.
  • The test name test_create_future_task_cron_description_prefers_named_weekdays still refers to create_future_task, while the tool under test is CreateActiveCronTool; renaming the test to match the actual tool name would make intent clearer.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- The new test tightly couples to the exact wording of the description (e.g., asserting literal substrings like '1-5' and 'avoid ambiguity'); consider relaxing these expectations to focus on the presence of named weekday guidance rather than specific phrasing so future wording tweaks don't break the test needlessly.
- The test name `test_create_future_task_cron_description_prefers_named_weekdays` still refers to `create_future_task`, while the tool under test is `CreateActiveCronTool`; renaming the test to match the actual tool name would make intent clearer.

## Individual Comments

### Comment 1
<location path="astrbot/core/tools/cron_tools.py" line_range="33" />
<code_context>
                 "cron_expression": {
                     "type": "string",
-                    "description": "Cron expression defining recurring schedule (e.g., '0 8 * * *').",
+                    "description": "Cron expression defining recurring schedule (e.g., '0 8 * * *' or '0 23 * * mon-fri'). Prefer named weekdays like 'mon-fri' or 'sat,sun' instead of numeric day-of-week ranges such as '1-5' to avoid ambiguity across cron implementations.",
                 },
                 "run_at": {
</code_context>
<issue_to_address>
**suggestion:** Clarify which cron dialect is expected rather than relying on generic implementation differences.

The note about named weekdays helps, but users still won’t know which cron semantics we follow (POSIX, Quartz, or a specific library). Please explicitly state the cron flavor or parser we use (e.g., "POSIX-style 5-field cron" or the library name) so users can rely on the correct external docs and avoid ambiguities like `0` vs `7` for Sunday or differing range behavior.

Suggested implementation:

```python
                    "description": "Cron expression defining recurring schedule using standard 5-field POSIX-style cron syntax (minute hour day-of-month month day-of-week), as parsed by this service's cron scheduler. For example: '0 8 * * *' or '0 23 * * mon-fri'. Prefer named weekdays like 'mon-fri' or 'sat,sun' instead of numeric day-of-week ranges such as '1-5' to avoid ambiguity around Sunday handling (0 vs 7) and differing cron implementations.",

```

If your project uses a specific cron library (e.g., `croniter`, `APScheduler`, `celery`, or a bespoke parser), you should further refine the description to name it explicitly. For example, replace “this service's cron scheduler” with “the `croniter` parser” or “APScheduler’s cron trigger semantics” so users can reference the appropriate external documentation.
</issue_to_address>

### Comment 2
<location path="tests/unit/test_cron_tools.py" line_range="6-15" />
<code_context>
+from astrbot.core.tools.cron_tools import CreateActiveCronTool
+
+
+def test_create_future_task_cron_description_prefers_named_weekdays():
+    """The cron tool should steer users toward unambiguous named weekdays."""
+    tool = CreateActiveCronTool()
+
+    description = tool.parameters["properties"]["cron_expression"]["description"]
+
+    assert "mon-fri" in description
+    assert "sat,sun" in description
+    assert "1-5" in description
+    assert "avoid ambiguity" in description
</code_context>
<issue_to_address>
**suggestion (testing):** Consider also asserting that the original generic cron example is preserved as described in the PR summary.

The PR description specifies that the original generic example (e.g. `0 8 * * *`) should remain in addition to the new named weekday guidance, but this test only checks the new guidance. Adding an assertion like `assert "0 8 * * *" in description` would ensure both parts of the requirement are covered and better protect against regressions.
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

"cron_expression": {
"type": "string",
"description": "Cron expression defining recurring schedule (e.g., '0 8 * * *').",
"description": "Cron expression defining recurring schedule (e.g., '0 8 * * *' or '0 23 * * mon-fri'). Prefer named weekdays like 'mon-fri' or 'sat,sun' instead of numeric day-of-week ranges such as '1-5' to avoid ambiguity across cron implementations.",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion: 澄清这里期望使用的是哪种 cron 方言,而不是笼统地依赖“不同实现行为不同”的说法。

关于使用具名工作日的说明是有帮助的,但用户仍然不知道我们实际遵循的是哪种 cron 语义(POSIX、Quartz,还是某个特定库)。请明确写出我们使用的 cron 风格或解析器(例如 "POSIX-style 5-field cron" 或具体库名),这样用户就可以参考正确的外部文档,并避免诸如星期天是 0 还是 7、以及区间行为差异之类的歧义。

建议的实现方式:

                    "description": "Cron expression defining recurring schedule using standard 5-field POSIX-style cron syntax (minute hour day-of-month month day-of-week), as parsed by this service's cron scheduler. For example: '0 8 * * *' or '0 23 * * mon-fri'. Prefer named weekdays like 'mon-fri' or 'sat,sun' instead of numeric day-of-week ranges such as '1-5' to avoid ambiguity around Sunday handling (0 vs 7) and differing cron implementations.",

如果你的项目使用了某个特定的 cron 库(例如 croniterAPSchedulercelery,或自研解析器),应进一步在描述中将其名字写明。比如将 “this service's cron scheduler” 替换为 “the croniter parser” 或 “APScheduler’s cron trigger semantics”,这样用户就可以参考相应的外部文档。

Original comment in English

suggestion: Clarify which cron dialect is expected rather than relying on generic implementation differences.

The note about named weekdays helps, but users still won’t know which cron semantics we follow (POSIX, Quartz, or a specific library). Please explicitly state the cron flavor or parser we use (e.g., "POSIX-style 5-field cron" or the library name) so users can rely on the correct external docs and avoid ambiguities like 0 vs 7 for Sunday or differing range behavior.

Suggested implementation:

                    "description": "Cron expression defining recurring schedule using standard 5-field POSIX-style cron syntax (minute hour day-of-month month day-of-week), as parsed by this service's cron scheduler. For example: '0 8 * * *' or '0 23 * * mon-fri'. Prefer named weekdays like 'mon-fri' or 'sat,sun' instead of numeric day-of-week ranges such as '1-5' to avoid ambiguity around Sunday handling (0 vs 7) and differing cron implementations.",

If your project uses a specific cron library (e.g., croniter, APScheduler, celery, or a bespoke parser), you should further refine the description to name it explicitly. For example, replace “this service's cron scheduler” with “the croniter parser” or “APScheduler’s cron trigger semantics” so users can reference the appropriate external documentation.

Comment on lines +6 to +15
def test_create_future_task_cron_description_prefers_named_weekdays():
"""The cron tool should steer users toward unambiguous named weekdays."""
tool = CreateActiveCronTool()

description = tool.parameters["properties"]["cron_expression"]["description"]

assert "mon-fri" in description
assert "sat,sun" in description
assert "1-5" in description
assert "avoid ambiguity" in description
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion (testing): 建议同时断言原有的通用 cron 示例仍然保留,就像 PR 描述中所说的那样。

PR 描述中说明,原来的通用示例(例如 0 8 * * *)应该在新增具名工作日指引的基础上继续保留,但当前测试只检查了新增的指引。加上一条诸如 assert "0 8 * * *" in description 的断言,可以确保这两个部分的需求都被覆盖,从而更好地防止回归。

Original comment in English

suggestion (testing): Consider also asserting that the original generic cron example is preserved as described in the PR summary.

The PR description specifies that the original generic example (e.g. 0 8 * * *) should remain in addition to the new named weekday guidance, but this test only checks the new guidance. Adding an assertion like assert "0 8 * * *" in description would ensure both parts of the requirement are covered and better protect against regressions.

@dosubot dosubot bot added the area:core The bug / feature is about astrbot's core, backend label Mar 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:core The bug / feature is about astrbot's core, backend size:S This PR changes 10-29 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] cron表达式描述应推荐使用明确的星期几格式(mon-fri),以避免星期几偏移

2 participants