fix(cron): add delay_ms to avoid LLM computing absolute timestamps#377
fix(cron): add delay_ms to avoid LLM computing absolute timestamps#377penso merged 1 commit intomoltis-org:mainfrom
Conversation
The 'at' schedule kind previously required the LLM to supply at_ms as an absolute epoch millisecond value. LLMs reliably miscalculate this (training-data clock skew means 'now' is off by months or years), causing one-shot jobs to be scheduled in the past and never fire. Add delay_ms (aliases: delayMs, delay, in, in_ms, offset_ms) as a server-resolved relative offset: the tool computes now_ms + delay_ms at call time, so the LLM only needs to express durations (e.g. 600000 for 10 minutes) rather than absolute timestamps. Existing at_ms is preserved for callers that have a real absolute time.
Greptile SummaryThis PR introduces a Key changes:
Issues found:
Confidence Score: 3/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant LLM as LLM Agent
participant CronTool as CronTool::call()
participant Normalize as normalize_schedule_value()
participant SysTime as SystemTime::now()
participant CronSvc as CronService
LLM->>CronTool: {name, schedule: {kind:'at', delay_ms: 600000}}
CronTool->>Normalize: &mut schedule value
Normalize->>Normalize: take_alias → canonicalize field names
Normalize->>Normalize: obj.remove("delay_ms") → delay = 600000
Normalize->>SysTime: SystemTime::now()
SysTime-->>Normalize: now_ms (u64)
Normalize->>Normalize: at_ms = now_ms + delay (⚠️ potential overflow)
Normalize->>Normalize: obj.entry("at_ms").or_insert(at_ms)
Normalize->>Normalize: obj.entry("kind").or_insert("at")
Normalize-->>CronTool: Ok(schedule = {kind:'at', at_ms: <absolute>})
CronTool->>CronSvc: create_job(CronJobCreate{schedule})
CronSvc-->>CronTool: CronJob
CronTool-->>LLM: tool result (job created)
Last reviewed commit: c4c2642 |
|
Addressed all three issues from the Greptile review:
Added three unit tests covering the new behaviour: |
) The 'at' schedule kind previously required the LLM to supply at_ms as an absolute epoch millisecond value. LLMs reliably miscalculate this (training-data clock skew means 'now' is off by months or years), causing one-shot jobs to be scheduled in the past and never fire. Add delay_ms (aliases: delayMs, delay, in, in_ms, offset_ms) as a server-resolved relative offset: the tool computes now_ms + delay_ms at call time, so the LLM only needs to express durations (e.g. 600000 for 10 minutes) rather than absolute timestamps. Existing at_ms is preserved for callers that have a real absolute time.
…oltis-org#377) The 'at' schedule kind previously required the LLM to supply at_ms as an absolute epoch millisecond value. LLMs reliably miscalculate this (training-data clock skew means 'now' is off by months or years), causing one-shot jobs to be scheduled in the past and never fire. Add delay_ms (aliases: delayMs, delay, in, in_ms, offset_ms) as a server-resolved relative offset: the tool computes now_ms + delay_ms at call time, so the LLM only needs to express durations (e.g. 600000 for 10 minutes) rather than absolute timestamps. Existing at_ms is preserved for callers that have a real absolute time.
Summary
atschedule kind requiresat_msas an absolute epoch milliseconds valuedelay_ms(server-resolved relative offset from now) so the LLM only needs to express a duration like600000for 10 minutesChanges
crates/tools/src/cron_tool.rsnormalize_schedule_value, resolvedelay_ms→at_msusingSystemTime::now()at call time before the schedule is storeddelayMs,delay,in,in_ms,offset_msdelay_msinstead of computingat_msitselfat_msis preserved and unchanged for callers that supply a real absolute timestampValidation
Completed
Remaining
cargo test(cron unit tests)just release-preflightManual QA