fix(ai): preserve multiple recurring tasks from conversational input - #518
imlililili wants to merge 8 commits into
Conversation
Test Results133 tests 133 ✅ 10s ⏱️ Results for commit 6014a64. ♻️ This comment has been updated with latest results. |
|
| if (task.TimeType == TaskTimeType.SingleTime) | ||
| { | ||
| task.TemplateEndTime = task.TemplateStartTime; | ||
| } |
There was a problem hiding this comment.
seems like if I changed the single time to range time, the timetype is still keep single time
| Interval = interval < 1 ? 1 : interval, | ||
| DaysOfWeek = frequency == RecurrenceFrequency.Weekly && weeklyMask != 0 ? weeklyMask : null, | ||
| DayOfMonth = frequency == RecurrenceFrequency.Monthly ? dayOfMonth : null, | ||
| StartDate = startDate, |
There was a problem hiding this comment.
if we have deleted startDate here, why do we still need startDate in createRecurringTask?
| public class RecurringTaskInput | ||
| { | ||
| [Description("Short title for exactly one independently completable action")] | ||
| public required string Title { get; init; } | ||
|
|
||
| [Description("Description or empty")] | ||
| public required string Description { get; init; } | ||
|
|
||
| [Description("SingleTime for a moment with no duration; RangeTime for a start and end time")] | ||
| public required TaskTimeType TimeType { get; init; } | ||
|
|
||
| [Description("Work, Life, Learning, or Health")] | ||
| public required LabelNameEnum Label { get; init; } | ||
|
|
||
| [Description("First occurrence start as local time yyyy-MM-ddTHH:mm:ss")] | ||
| public required DateTime TemplateStartTime { get; init; } | ||
|
|
||
| [Description("First occurrence end as local time yyyy-MM-ddTHH:mm:ss")] | ||
| public required DateTime TemplateEndTime { get; init; } | ||
|
|
||
| [Description("Daily, Weekly, Monthly, or Yearly")] | ||
| public required RecurrenceFrequency Frequency { get; init; } | ||
|
|
||
| [Description("Repeat every N periods; use 1 unless the user says otherwise")] | ||
| public int Interval { get; init; } = 1; | ||
|
|
||
| [Description("Required named weekdays for Weekly; empty otherwise")] | ||
| public DayOfWeek[] DaysOfWeek { get; init; } = []; | ||
|
|
||
| [Description("Required day 1-31 for Monthly; null otherwise")] | ||
| public int? DayOfMonth { get; init; } | ||
|
|
||
| [Description("Optional last recurrence date yyyy-MM-dd; null means open-ended")] | ||
| public DateOnly? EndDate { get; init; } | ||
| } |
There was a problem hiding this comment.
seems some descriptions here have drifted from CreateRecurringTask's parameters, some may too easy, pls check again. Maybe you can follow the descriptions of createTask and createTasks.
| frequency == RecurrenceFrequency.Weekly ? weeklyMask : null, | ||
| frequency == RecurrenceFrequency.Monthly ? normalizedDayOfMonth : null, | ||
| startDate, | ||
| endDate < startDate ? startDate : endDate); |
There was a problem hiding this comment.
why we need to change the endDate to startDate? I think if endDate < startDate, maybe it should be throw by the validation
| "interval": 1 | ||
| } | ||
| ] | ||
| } |
There was a problem hiding this comment.
seems missing a check for update the single time to duration?
| } | ||
|
|
||
| var updatedFrequency = frequency ?? task.Frequency; | ||
| var updatedDays = daysOfWeek ?? FromWeeklyBitmask(task.DaysOfWeek); |
There was a problem hiding this comment.
Only null counts as "leave unchanged" here, but models often send an empty array for an array parameter they mean to omit. An empty array makes weeklyMask 0, and NormalizeRecurringPattern then fills it from templateStartTime.DayOfWeek — so "rename my gym" can silently turn a Mon/Wed/Fri task into Monday only, which is the opposite of the PBI's "updates only DaysOfWeek; all other fields unchanged".
Suggest daysOfWeek is { Length: > 0 } ? daysOfWeek : FromWeeklyBitmask(task.DaysOfWeek), plus a test that passes daysOfWeek: [].
| } | ||
| } | ||
|
|
||
| public static void CheckRecurringMutation( |
There was a problem hiding this comment.
This check never runs today: quality-check-cases.json has no case using recurringMutation (or the new setupInputs / durationMinutes), and that file is not touched by this PR.
The PBI asks for the scoring and "cases for edit/remove phrasings". Could you add cases for "change gym to Tuesdays", "stop the weekly standup" and "make gym go on forever"? Without them the new prompt rules have no regression cover at the eval level — the unit tests cover the tools, but not what the model actually picks.
| "expo-build-properties", | ||
| { | ||
| ios: { | ||
| usePrecompiledModules: false, |
There was a problem hiding this comment.
This iOS build setting looks unrelated to the recurring-task work, and the PBI scopes this one as "Backend only". Disabling precompiled modules also makes iOS builds noticeably slower.
Was it needed to unblock a local build? If so it is worth its own PR, or at least a line in the description, so nobody reverts it by accident later.
2 participants
Summary
Adds conversational support for creating multiple recurring tasks from one request, plus updating and removing recurring task drafts. It also normalizes recurring patterns to prevent invalid weekly, monthly, or date combinations and extends the evaluation harness and automated tests to cover these mutations.
Release note
Status: