internal/core/schedule.go (1)

332-344: Special-case for */5 in hour field is reasonable but consider documenting the rationale.

The exception for */5 in the hour field (which fires at 0, 5, 10, 15, 20 — not evenly distributed across 24 hours) is a pragmatic choice to reduce noise for a commonly-used pattern.

Consider whether similar exceptions might be useful for other common patterns (e.g., */45 in minutes fires at 0, 45 which is often intentional for "twice per hour"). This could be addressed in a follow-up if users report false-positive warnings.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@internal/core/schedule.go` around lines 332 - 344, The isExpectedStepValue
function currently special-cases step==5 for the "hour" field; update this code
to include a clear inline comment documenting the rationale (that "*/5" in hours
is a common intentional pattern and results in uneven final interval at day
boundaries) and reference that similar special-cases could be added later (e.g.,
minute steps like 45) if users report false positives; make the comment adjacent
to the existing if block for fieldName == "hour" && step == 5 so future
maintainers know why this exception exists and where to extend it.

internal/core/schedule_test.go (1)

61-67: Test only validates first warning; consider checking all warnings for multi-field expressions.

When an expression triggers warnings for both minute and hour fields (e.g., */7 */7 * * *), only warnings[0] is validated. This is fine for current test cases since each tests a single field, but consider adding a test case that verifies both fields generate warnings independently.

💡 Example additional test case

{
    name:        "both minute and hour step 7 warns",
    expr:        "*/7 */7 * * *",
    wantWarning: true,
    contains:    []string{"minute field"}, // or validate both warnings
},

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@internal/core/schedule_test.go` around lines 61 - 67, The test only asserts
on warnings[0], so add coverage that validates all returned warnings for
multi-field expressions: update the test in internal/core/schedule_test.go that
calls checkMisleadingStepValues (the tt loop) to either add a new case for "*/7
*/7 * * *" (wantWarning true) or change the assertions to iterate over warnings
and ensure each expected substring in tt.contains appears in some warning (e.g.,
check that for each s in tt.contains there exists an entry in warnings
containing s) rather than only checking warnings[0]; reference the
checkMisleadingStepValues call and the warnings variable when implementing this
broadened validation.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes