Store workout plans by date - #6
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8b9e6738b4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const sourceDate = move?.source_date; | ||
| const targetDate = move?.target_date; | ||
| if (typeof sourceDate !== "string" || !/^\d{4}-\d{2}-\d{2}$/.test(sourceDate)) errors.push({ path: "/source_date", message: "source_date must be YYYY-MM-DD" }); | ||
| if (typeof targetDate !== "string" || !/^\d{4}-\d{2}-\d{2}$/.test(targetDate)) errors.push({ path: "/target_date", message: "target_date must be YYYY-MM-DD" }); |
There was a problem hiding this comment.
Reject nonexistent target dates before moving workouts
Validate calendar semantics rather than only the YYYY-MM-DD shape. A target such as 2027-99-99 passes this check, is treated as an available no-plan day, and can be applied; the source workout is then removed from its valid date and stored under a date that schedule endpoints will reject, effectively making the workout inaccessible.
Useful? React with 👍 / 👎.
| if (!day || day.kind === "no_plan" || slot === null) return { | ||
| date, weekday: weekdayKey(date), kind: "no_plan", title: null, module_count: null, estimated_duration_min: null, | ||
| prescription_ref: null, scheduled_workout_key: null, session_key: null, | ||
| is_due: false, is_overdue_unstarted: false, source_ref: `schedule:${date}:no_plan`, revision_key: null, | ||
| }; |
There was a problem hiding this comment.
Preserve move provenance when the source becomes no-plan
Include moved_to_date in this branch. Moving a workout onto an allowed no-plan target leaves the source as kind: "no_plan", but this early return drops the provenance written by applyPlannedDayMove; consequently the Agent schedule violates its moved-entry shape and the MCP apply flow reports readback_mismatch even though the mutation succeeded.
Useful? React with 👍 / 👎.
| const from = args.move.source_date < args.move.target_date ? args.move.source_date : args.move.target_date; | ||
| const to = args.move.source_date > args.move.target_date ? args.move.source_date : args.move.target_date; | ||
| try { | ||
| const schedule = await this.getSchedule({ from, to, expand: true }); |
There was a problem hiding this comment.
Read back distant move dates without one oversized range
Read the two affected dates separately, or constrain validation to the schedule limit. The move API permits any future target, but this constructs one inclusive schedule request; when the dates are more than 366 days apart, the schedule endpoint rejects the request after the move has already succeeded, so the advertised apply-and-readback tool deterministically returns a failed verification.
Useful? React with 👍 / 👎.
Summary:
Validation: