Skip to content

feat(talon): add cron days-of-week helpers module#4499

Open
John Kennedy (jkennedyvz) wants to merge 2 commits into
mainfrom
john/jkb-15-cron-days-of-week-helpers-module
Open

feat(talon): add cron days-of-week helpers module#4499
John Kennedy (jkennedyvz) wants to merge 2 commits into
mainfrom
john/jkb-15-cron-days-of-week-helpers-module

Conversation

@jkennedyvz

@jkennedyvz John Kennedy (jkennedyvz) commented Jul 4, 2026

Copy link
Copy Markdown
Contributor

Closes JKB-15


Split out of PR #4426 (JKB-9) after review flagged the day-of-week types as out of scope for the time-helpers PR boundary. This PR gives them a focused home ahead of downstream schedule-parsing and tool-contract tickets.

Why

JKB-9 shipped overreach into day-of-week territory with Weekday, DaysOfWeek, parse_weekday, and parse_days_of_week. These don't appear anywhere in JKB-9's scope/acceptance criteria and need their own home so JKB-9 can merge as a focused time-helpers PR while the day-of-week types ship separately and become importable for schedule parsing (JKB-10) and the cron tool contract (JKB-14).

Stacked PR note

This branch sits on top of #4426 (JKB-9). GitHub's diff view on this PR is against main, so it shows both the time-helpers module (from #4426) and the day-of-week module (this PR). The incremental contribution of this PR is 415 insertions across 2 files:

  • libs/talon/deepagents_talon/cron/days_of_week.py (296 lines)
  • libs/talon/tests/cron/test_days_of_week.py (119 lines)

time.py and test_time.py appear in the GitHub diff only because they haven't merged via #4426 yet. Merge #4426 first, then this PR resolves to just the two DoW files.

What landed

deepagents_talon/cron/days_of_week.py:

  • WeekdayIntEnum with Monday=0 through Sunday=6, matching datetime.weekday() numbering. label/abbreviation properties, from_date/from_datetime classmethods.
  • DaysOfWeek — frozen dataclass (days: tuple[Weekday, ...]), normalized (deduped + sorted Monday through Sunday) in __post_init__, rejected when empty. all()/weekdays()/weekends() constructors, contains_date/contains_datetime/next_date_on_or_after helpers.
  • parse_weekday(value) -> Weekday — full names (monday), abbreviations (mon, thu, sat), single letters (m, f); strips trailing s and .. Rejects empty, ambiguous single letters (s, t), and unknown tokens with CronTimeError.
  • parse_days_of_week(value) -> DaysOfWeek — accepts daily/every day/everyday/any day/weekdays/weekends, plus spaced/comma/slash/plus-separated day lists (mon wed fri, monday, wednesday, and friday, sat/sun).
  • Reuses CronTimeError from the time helpers module — no new error type.
  • All functions deterministic, no I/O, no system-clock reads.

Tests

tests/cron/test_days_of_week.py — 30 deterministic unit tests covering:

  • parse_weekday accepts common names/abbreviations, rejects unknown/ambiguous.
  • parse_days_of_week accepts timer forms, rejects invalid values.
  • DaysOfWeek dedupes, sorts, contains dates/datetimes, and computes next_date_on_or_after.
  • Weekday.from_date/from_datetime agree with date.weekday().

Merge order

Depends on #4426 (JKB-9) for CronTimeError. Branch sits on top of the JKB-9 commit; merge #4426 first, then this.

Review notes

  • The module file is named days_of_week.py rather than folding into time.py to keep the JKB-9 time-helpers module focused and to make the downstream import surface explicit. Open to collapsing if a reviewer prefers.
  • Trailing-s plural stripping in parse_weekday (mondays -> monday) is intentional — natural language from the model often includes plurals. Ambiguous single letters (s for Saturday/Sunday, t for Tuesday/Thursday) are rejected rather than guessing.

Release note

Add deepagents_talon.cron.days_of_week pure-function module with Weekday, DaysOfWeek, parse_weekday, and parse_days_of_week. Downstream schedule-parsing and tool-contract tickets import from this module.

@github-actions github-actions Bot added feature New feature/enhancement or request for one internal User is a member of the `langchain-ai` GitHub organization size: XL 1000+ LOC talon labels Jul 4, 2026

@open-swe open-swe Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Open SWE Review found 1 potential issue.

Open in WebView Open SWE trace

candidate_date = now_local.date()
if _local_time_of(now_local) >= time:
candidate_date += timedelta(days=1)
candidate = _build_local(zone, candidate_date, time, fold=0)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Fall-back scheduling can return the past

During the repeated hour after a DST fall-back, this branch compares only the wall-clock LocalTimeOfDay before building the candidate with fold=0. For example, at 2026-11-01T09:15Z in America/Los_Angeles (01:15 PST, the second 1am hour), asking for 01:30 takes this path and returns 2026-11-01T08:30Z (01:30 PDT), which is already 45 minutes in the past. Callers expecting the next run can immediately mark a job due or loop on a stale scheduled time. After constructing the same-day candidate, verify it is still after now_utc (or use the second fold when applicable) before returning it.

(Refers to line 248)


Your feedback helps Open SWE learn. React with 👍 or 👎 to tell us if this review comment was useful.

Suggested change
candidate = _build_local(zone, candidate_date, time, fold=0)
candidate = _build_local(zone, candidate_date, time, fold=0)
if _as_utc(candidate) <= now_utc:
candidate = _build_local(zone, candidate_date + timedelta(days=1), time, fold=0)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature New feature/enhancement or request for one internal User is a member of the `langchain-ai` GitHub organization size: XL 1000+ LOC talon

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant