Repository navigation
feat(scheduling): a clickable schedule picker in the kit replaces the Routines cron box; plain-language summaries, server preview, cron-only until the scheduler lands (#18432) - #18495
Conversation
📝 Walkthrough
Priority: ➖ Normal Merge Risk: 🟡 Moderate · up to A routine saved with a non-UTC timezone can run at the wrong local time. Fix timezone handling before merging; advisory previews and preview-error reporting also need correction. Pre-merge checks |
  |
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage |  |
Docstring coverage is 23.53% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 102 functions across 29 files. (15 skippe… | Write docstrings for the functions missing them to satisfy the coverage threshold. |
✅ Passed checks (4 passed)
-
Check name Status Explanation Description Check Check skipped - CodeRabbit’s high-level summary is enabled. Title check The title clearly describes the main change: replacing the Routines cron box with a shared schedule picker, summaries and server preview. It is detailed but remains relevant and specific. Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 23.53% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 102 functions across 29 files. (15 skipped: 15 unsupported.)
- Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
-
📝 Generate docstrings 💡
-
- Commit to this branch
- Create a new PR
🧪 Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
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 @coderabbitai help to get the list of available commands.
|
Review (Sonnet) at b74c74f: approve.
LOWs (follow-ups, non-blocking):
Queued for the next merge train. |
✅ SSOT Configuration Compliance: Passing🎉 No new hardcoded values of either class — Known backlog in |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Repository: mrveiss/AutoBot-AI/.coderabbit.yaml
- Review profile: ASSERTIVE
- Plan: Advanced
- Run ID:
60719b82-a83f-41de-9bb0-b894586f12a8
📒 Files selected for processing (44)
autobot-frontend/src/components/schedule/SchedulePickerField.vueautobot-frontend/src/components/schedule/__tests__/SchedulePickerField.i18n.test.tsautobot-frontend/src/composables/__tests__/useSchedule.test.tsautobot-frontend/src/composables/useSchedule.tsautobot-frontend/src/i18n/locales/ar.jsonautobot-frontend/src/i18n/locales/de.jsonautobot-frontend/src/i18n/locales/en.jsonautobot-frontend/src/i18n/locales/es.jsonautobot-frontend/src/i18n/locales/fa.jsonautobot-frontend/src/i18n/locales/fr.jsonautobot-frontend/src/i18n/locales/he.jsonautobot-frontend/src/i18n/locales/lv.jsonautobot-frontend/src/i18n/locales/pl.jsonautobot-frontend/src/i18n/locales/pt.jsonautobot-frontend/src/i18n/locales/ur.jsonautobot-frontend/src/views/llc/RoutinesView.vueautobot-frontend/src/views/llc/__tests__/RoutinesView.schedule.test.tslibs/autobot-ui/index.tslibs/autobot-ui/src/components/SchedulePicker.vuelibs/autobot-ui/src/components/ScheduleTimeField.vuelibs/autobot-ui/src/components/__tests__/SchedulePicker.test.tslibs/autobot-ui/src/composables/useScheduleExpert.tslibs/autobot-ui/src/composables/useScheduleFields.tslibs/autobot-ui/src/composables/useScheduleMessages.tslibs/autobot-ui/src/composables/useSchedulePreview.tslibs/autobot-ui/src/composables/useScheduleState.tslibs/autobot-ui/src/schedule/__tests__/nextRuns.test.tslibs/autobot-ui/src/schedule/__tests__/schedule.test.tslibs/autobot-ui/src/schedule/compile.tslibs/autobot-ui/src/schedule/cronFields.tslibs/autobot-ui/src/schedule/defaults.tslibs/autobot-ui/src/schedule/index.tslibs/autobot-ui/src/schedule/nextRuns.tslibs/autobot-ui/src/schedule/parse.tslibs/autobot-ui/src/schedule/rrule.tslibs/autobot-ui/src/schedule/rruleDays.tslibs/autobot-ui/src/schedule/summary.tslibs/autobot-ui/src/schedule/time.tslibs/autobot-ui/src/schedule/types.tslibs/autobot-ui/src/schedule/zone.tsrepo_tests/frontend_file_size_ratchet_baseline.pyrepo_tests/frontend_file_size_ratchet_test.pyrepo_tests/frontend_fragmentation_ratchet_test.pyscripts/frontend_file_size_known_large.py
Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.
| /** | ||
| * Routine payload fragment: EXACTLY ONE of `cron_schedule` / `schedule_spec`. | ||
| * A cron spec is today's payload; calendar and once go as `schedule_spec` | ||
| * alone. Sending both is a 422 `schedule.conflicting_fields`. | ||
| */ | ||
| export function schedulePayload(spec: ScheduleSpec | null): Record<string, unknown> { | ||
| if (!spec) return {} | ||
| return spec.kind === 'cron' ? { cron_schedule: spec.cron } : { schedule_spec: spec } | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
🔎 Supported by static analysis
🏁 Script executed:
sed -n '28,72p' autobot-frontend/src/composables/useSchedule.ts
sed -n '1,115p' autobot-frontend/src/components/schedule/SchedulePickerField.vue
sed -n '193,240p' libs/autobot-ui/src/components/SchedulePicker.vue
rg -n 'cron_schedule|schedule_spec' autobot-frontend/src/views/llc/RoutinesView.vue | head -45Repository: mrveiss/AutoBot-AI
Length of output: 6520
🏁 Script executed:
set -o pipefail
printf '%s\n' '--- picker timezone and model flow ---'
rg -n -F --glob '*.vue' --glob '*.ts' -- 'timezone' libs/autobot-ui/src/components/SchedulePicker.vue libs/autobot-ui/src autobot-frontend/src/components/schedule autobot-frontend/src/composables/useSchedule.ts | head -160
printf '%s\n' '--- routine save and schedule bindings ---'
rg -n -F --glob '*.vue' --glob '*.ts' --glob '*.py' -- 'schedulePayload' autobot-frontend/src
rg -n -F --glob '*.vue' --glob '*.ts' --glob '*.py' -- 'cron_schedule' autobot-frontend/src autobot-backend 2>/dev/null | head -220
printf '%s\n' '--- schedule contracts and tests ---'
rg -n -i --glob '*.{py,ts,vue}' -- 'schedule_spec|cron.*timezone|timezone.*cron|RoutinesView|SchedulePicker' autobot-backend autobot-frontend libs 2>/dev/null | head -300Repository: mrveiss/AutoBot-AI
Length of output: 31103
🏁 Script executed:
set -o pipefail
printf '%s\n' '--- zone list and picker state ---'
rg -n -F --glob '*.ts' --glob '*.vue' -- 'function listZones' libs/autobot-ui/src
sed -n '1,105p' libs/autobot-ui/src/composables/useScheduleState.ts
sed -n '1,100p' libs/autobot-ui/src/components/SchedulePicker.vue
sed -n '1,120p' libs/autobot-ui/src/schedule/zones.ts 2>/dev/null || true
printf '%s\n' '--- routine form and persistence flow ---'
sed -n '250,325p' autobot-frontend/src/views/llc/RoutinesView.vue
sed -n '1,130p' autobot-backend/llc/api/routines.py
sed -n '150,265p' autobot-backend/llc/api/routines.py
printf '%s\n' '--- scheduler and relevant tests ---'
sed -n '70,115p' autobot-backend/llc/scheduler/routine_scheduler.py
sed -n '150,210p' autobot-backend/llc/scheduler/routine_scheduler.py
sed -n '1,130p' autobot-frontend/src/views/llc/__tests__/RoutinesView.schedule.test.ts
sed -n '1,65p' autobot-frontend/src/composables/__tests__/useSchedule.test.tsRepository: mrveiss/AutoBot-AI
Length of output: 31397
🏁 Script executed:
printf '%s\n' '--- imports and zone-list implementation ---'
sed -n '1,75p' libs/autobot-ui/src/components/SchedulePicker.vue
rg -n -F --glob '*.ts' --glob '*.vue' -- 'listZones' libs/autobot-ui
printf '%s\n' '--- routine save/reload ---'
sed -n '285,320p' autobot-frontend/src/views/llc/RoutinesView.vue
sed -n '1,45p' autobot-frontend/src/composables/useSchedule.ts
printf '%s\n' '--- backend contract and execution ---'
sed -n '45,100p' autobot-backend/llc/api/routines.py
sed -n '155,205p' autobot-backend/llc/api/routines.py
sed -n '215,255p' autobot-backend/llc/api/routines.py
sed -n '90,105p' autobot-backend/llc/scheduler/routine_scheduler.py
sed -n '175,201p' autobot-backend/llc/scheduler/routine_scheduler.pyRepository: mrveiss/AutoBot-AI
Length of output: 15117
🏁 Script executed:
rg -n -F --glob '*.ts' --glob '*.vue' -- 'listZones' libs/autobot-ui/src
rg -n -F --glob '*.ts' --glob '*.vue' -- 'supportedValuesOf' libs/autobot-ui/src
sed -n '1,28p' libs/autobot-ui/src/components/SchedulePicker.vueRepository: mrveiss/AutoBot-AI
Length of output: 2218
🏁 Script executed:
nl -ba libs/autobot-ui/src/schedule/zone.ts | sed -n '68,90p'Repository: mrveiss/AutoBot-AI
Length of output: 1036
Persist the selected time zone for cron routines.
The picker offers all IANA time zones, but schedulePayload sends only the cron expression. The selected time zone is discarded.
The routine API stores only cron_schedule. routineSpec reloads it as UTC, and the scheduler evaluates it from UTC. A user who selects a non-UTC zone can therefore see and execute the routine at the wrong local time.
Persist the time zone through the routine API and scheduler, or restrict the routines picker to UTC until that contract is supported.
| export function useSchedulePreviewCall(): SchedulePreviewFn { | ||
| const api = useApiClient() | ||
| return async (spec, count) => { | ||
| try { | ||
| return await api.post<PreviewResult>('/api/schedules/preview', { spec, count }) | ||
| } catch { | ||
| return null | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Do not convert every preview failure into "unavailable".
useSchedulePreviewCall catches every error and returns null. The picker then treats a 500, a network timeout and a 404 the same way: it shows the preview as unavailable and enables the cron-only gate. The path instruction for **/*.{ts,vue} requires that "a request that failed must stay distinguishable from one that legitimately returned nothing". Return null only for a 404 (endpoint not deployed). For any other failure, log it or rethrow it. The kit ask() function already converts a rejection into an advisory fallback.
Proposed fix
- } catch {
- return null
+ } catch (err) {
+ if (err instanceof Error && /\b404\b/.test(err.message)) return null
+ throw err
}As per path instructions: "a caught error that resolves to an empty or default value — a request that failed must stay distinguishable from one that legitimately returned nothing."
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export function useSchedulePreviewCall(): SchedulePreviewFn { | |
| const api = useApiClient() | |
| return async (spec, count) => { | |
| try { | |
| return await api.post<PreviewResult>('/api/schedules/preview', { spec, count }) | |
| } catch { | |
| return null | |
| } | |
| } | |
| } | |
| export function useSchedulePreviewCall(): SchedulePreviewFn { | |
| const api = useApiClient() | |
| return async (spec, count) => { | |
| try { | |
| return await api.post<PreviewResult>('/api/schedules/preview', { spec, count }) | |
| } catch (err) { | |
| if (err instanceof Error && /\b404\b/.test(err.message)) return null | |
| throw err | |
| } | |
| } | |
| } |
Source: Path instructions
| /** Parse exactly five whitespace-separated fields; null for anything else. */ | ||
| export function parseCron5(cron: string): CronFields | null { | ||
| const raw = cron.trim().split(/\s+/) | ||
| if (raw.length !== 5) return null | ||
| const minute = parseCronField(raw[0], 0, 59) | ||
| const hour = parseCronField(raw[1], 0, 23) | ||
| const dom = parseCronField(raw[2], 1, 31) | ||
| const month = parseCronField(raw[3], 1, 12, MONTH_NAMES, 1) | ||
| const dow = parseDow(raw[4]) | ||
| if (!minute || !hour || !dom || !month || !dow) return null | ||
| return { minute, hour, dom, month, dow, domStar: raw[2].startsWith('*'), dowStar: raw[4].startsWith('*'), raw } | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🔎 Supported by static analysis
🏁 Script executed:
sed -n '29,79p' libs/autobot-ui/src/schedule/cronFields.ts
sed -n '20,67p' libs/autobot-ui/src/schedule/nextRuns.ts
sed -n '1,65p' libs/autobot-ui/src/schedule/__tests__/nextRuns.test.tsRepository: mrveiss/AutoBot-AI
Length of output: 6986
🏁 Script executed:
set -o pipefail
printf '%s\n' '--- changed diff ---'
git diff --unified=30 df211d3ba64f9ebcb1db611f33197b9654a1dacd b74c74fd5f419e21d0dabc6d0852e4cc4a539068 -- libs/autobot-ui/src/schedule/cronFields.ts libs/autobot-ui/src/schedule/nextRuns.ts libs/autobot-ui/src/schedule/__tests__
printf '%s\n' '--- cron-related tests and callers ---'
rg -n -F --glob '*.{ts,tsx,vue,py}' -- 'parseCron5' libs app backend . 2>/dev/null || test "$?" -eq 1
rg -n -F --glob '*.{ts,tsx,vue,py}' -- 'nextRuns(' libs app backend . 2>/dev/null || test "$?" -eq 1
printf '%s\n' '--- cron scheduler references ---'
rg -n -i --glob '*.{ts,tsx,vue,py}' -- 'vixie|croniter|cron_schedule|day-of-month|day-of-week|day of month|day of week' . 2>/dev/null || test "$?" -eq 1
printf '%s\n' '--- relevant source excerpts ---'
nl -ba libs/autobot-ui/src/schedule/cronFields.ts | sed -n '1,95p'
nl -ba libs/autobot-ui/src/schedule/nextRuns.ts | sed -n '1,125p'
nl -ba libs/autobot-ui/src/schedule/__tests__/nextRuns.test.ts | sed -n '20,70p'Repository: mrveiss/AutoBot-AI
Length of output: 41714
🏁 Script executed:
set -o pipefail
printf '%s\n' '--- preview caller and display ---'
rg -n -F --glob '*.{ts,vue}' -- 'useSchedulePreview' autobot-frontend/src libs/autobot-ui
rg -n -F --glob '*.{ts,vue}' -- 'nextRuns' autobot-frontend/src libs/autobot-ui
nl -ba autobot-frontend/src/composables/useSchedulePreview.ts 2>/dev/null | sed -n '1,220p'
rg -n -F --glob 'RoutinesView.vue' -- 'nextRuns' autobot-frontend/src
nl -ba autobot-frontend/src/views/llc/RoutinesView.vue | sed -n '130,360p'
printf '%s\n' '--- backend cron contract and consumers ---'
nl -ba autobot-backend/llc/scheduler/routine_scheduler.py | sed -n '1,215p'
nl -ba autobot-backend/llc/api/routines.py | sed -n '145,265p'
printf '%s\n' '--- tests for preview/display and cron semantics ---'
rg -n -i --glob '*.{ts,py}' -- 'preview|next run|next-run|day.of.month|day.of.week|cron' autobot-frontend/src/views/llc/__tests__ autobot-backend/llc/tests | head -240Repository: mrveiss/AutoBot-AI
Length of output: 43324
🏁 Script executed:
set -o pipefail
nl -ba libs/autobot-ui/src/composables/useSchedulePreview.ts | sed -n '1,115p'
nl -ba libs/autobot-ui/src/components/SchedulePicker.vue | sed -n '1,90p'
nl -ba libs/autobot-ui/src/components/SchedulePicker.vue | sed -n '185,235p'
nl -ba autobot-frontend/src/components/schedule/SchedulePickerField.vue | sed -n '1,100p'
rg -n -F --glob '*.{ts,vue}' -- 'SchedulePickerField' autobot-frontend/src/views/llc autobot-frontend/src/componentsRepository: mrveiss/AutoBot-AI
Length of output: 15727
Treat only * as an unrestricted day field.
parseCronField accepts */2 as a valid restricted day set, but parseCron5 marks it as unrestricted because it starts with *. cronSource then requires both the day-of-month and day-of-week values to match. For 0 9 */2 * MON, the advisory preview therefore omits valid odd dates and Mondays that do not satisfy both conditions. This preview remains visible when the server preview is unavailable.
Suggested fix
- return { minute, hour, dom, month, dow, domStar: raw[2].startsWith('*'), dowStar: raw[4].startsWith('*'), raw }
+ return { minute, hour, dom, month, dow, domStar: raw[2] === '*', dowStar: raw[4] === '*', raw }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| /** Parse exactly five whitespace-separated fields; null for anything else. */ | |
| export function parseCron5(cron: string): CronFields | null { | |
| const raw = cron.trim().split(/\s+/) | |
| if (raw.length !== 5) return null | |
| const minute = parseCronField(raw[0], 0, 59) | |
| const hour = parseCronField(raw[1], 0, 23) | |
| const dom = parseCronField(raw[2], 1, 31) | |
| const month = parseCronField(raw[3], 1, 12, MONTH_NAMES, 1) | |
| const dow = parseDow(raw[4]) | |
| if (!minute || !hour || !dom || !month || !dow) return null | |
| return { minute, hour, dom, month, dow, domStar: raw[2].startsWith('*'), dowStar: raw[4].startsWith('*'), raw } | |
| } | |
| /** Parse exactly five whitespace-separated fields; null for anything else. */ | |
| export function parseCron5(cron: string): CronFields | null { | |
| const raw = cron.trim().split(/\s+/) | |
| if (raw.length !== 5) return null | |
| const minute = parseCronField(raw[0], 0, 59) | |
| const hour = parseCronField(raw[1], 0, 23) | |
| const dom = parseCronField(raw[2], 1, 31) | |
| const month = parseCronField(raw[3], 1, 12, MONTH_NAMES, 1) | |
| const dow = parseDow(raw[4]) | |
| if (!minute || !hour || !dom || !month || !dow) return null | |
| return { minute, hour, dom, month, dow, domStar: raw[2] === '*', dowStar: raw[4] === '*', raw } | |
| } |
This branch has not been deployed
1 participant
Refs #18432
Refs #18237
Single-issue rationale
This is the frontend half of #18432 (the schedule picker), built against the contract agreed and pinned on #18432. The backend half (scheduler,
POST /api/schedules/preview,schedule_specon routines, company timezone) is separate and unmerged, so #18432 stays open.Thinking Path
Owner direction: nobody types cron. Schedules are picked by clicking, shown in plain language, and translated to a stored format in the background. The single canonical home is
libs/autobot-ui.The only schedule input that existed was the plain cron box in
views/llc/RoutinesView.vue, and the picker replaces it, so there is no second implementation. No new dependency was added.What Changed
Pure module (
libs/autobot-ui/src/schedule/):ScheduleSpechas three kinds:cron(5-field),calendar(RFC 5545 RRULE, no DTSTART) andonce.PickerRuleis UI-only. Compiling picks the simplest kind that fits. Day 29–31 becomesBYMONTHDAY=28..N;BYSETPOS=-1, so it runs on the last day in shorter months.nullfor a spec it can't represent, which displays as "Custom schedule".nextRunsuses Intl only:nullrather than guess for 6-field cron,L,Wor#.Kit
SchedulePicker:aria-pressed, time and zone fields, a summary, the next 3 runs, and an expert cron field.translateandlocale.App (
components/schedule/SchedulePickerField.vue,composables/useSchedule.ts):POST /api/schedules/previewthrough ApiClient; a 404 or error means "unavailable").Gate, so saving never breaks: while the server preview is unavailable, only cron-expressible rules are offered. The calendar and once options are disabled with a translated note.
Payload sets exactly one schedule field (the server rejects both with 422
schedule.conflicting_fields):cronsendscron_schedule, the same as today.calendarandoncesendschedule_spec.Existing routine cron is read as
{kind:'cron', timezone:'UTC'}, so nothing shifts.i18n: repeats, summaries, notes and the error codes are in all 11 locales:
schedule.invalid_cron,cron_extension_unsupported,invalid_timezone,once_in_past,invalid_rruleandconflicting_fields.User-visible changes:
Not done here:
SchedulePickerFieldwhen built.Verification
useSchedule, and 11-locale key parity with rendering).AUTOBOT_PREPUSH_ALLOW_UNRUNNABLE=1: the shared installed hook predates feat(shell): Refined UI step 2: top nav and icon cluster, Minimal/Full mode, Appearance settings, one brand mark (#18240) #18373's kit-claim fix.Model Used
Claude Opus 5.5 (coordinating). A Sonnet subagent did the implementation.
🤖 Generated with Claude Code
Summary by CodeRabbit