feat(habits): smart AI reschedule for missed habits (#222) - #319
Conversation
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
Code Review: PR #319Scope: PR #319 — feat(habits): smart AI reschedule for missed habits SummaryPR #319 delivers the full "Reschedule with AI" flow end-to-end: a new FindingsCriticalNone HighNone Medium[MEDIUM]
Low / InfoNone Subagents
Deferred
What's good
RecommendationAPPROVE. The one Medium finding is a real pattern inconsistency but pre-existing across four other hooks and does not introduce new risk unique to this PR. Manually verify orbit-api PR #258 against the contract checklist above before considering the full feature shipped. 🤖 Generated with Claude Code |
There was a problem hiding this comment.
PR Review — feat(habits): smart AI reschedule for missed habits
Decision: APPROVE
| Severity | Count |
|---|---|
| Critical (incl. |
0 |
| High | 0 |
| Medium | 1 |
| Low / Info | 0 |
Medium Finding
apps/web/hooks/use-reschedule-suggestion.ts:38 — (await res.json()) as RescheduleSuggestionResponse casts the API response instead of calling rescheduleSuggestionResponseSchema.parse(raw). The schema exists and is importable; it's just not wired in. Fix is one line. However, adversarial verification confirmed this is a pre-existing pattern followed by 4 other hooks in the codebase — the PR didn't introduce it. Not a merge blocker.
All Clear
- Web/mobile parity: all 8 changed web files have confirmed mobile mirrors
- i18n: 13 new keys present in both
en.jsonandpt-BR.json - Backward compatibility: all shared-type changes are additive — no
⚠️ old-client breaks - Security: no issues on the frontend side
- Design tokens: correct throughout (
--primary-soft,--fg-*,tintFromPrimary)
Not Verifiable in CI
Contract alignment against orbit-api — the sibling repo is not checked out in this CI job. Manual verification against orbit-api PR #258 is required before considering the feature fully shipped end-to-end.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
There was a problem hiding this comment.
Code Review: PR #319
Scope: PR #319 — feat(habits): smart AI reschedule for missed habits (#222)
Recommendation: NEEDS WORK
Summary
PR #319 adds a well-structured "Reschedule with AI" feature: a new Astra-branded sheet, a shared buildRescheduleUpdateRequest helper, the useRescheduleSuggestion hook, and all the wiring to surface it from the habit overflow menu for overdue habits. Web and mobile parity is solid across the main surfaces, i18n is fully synced, and the backward-compat guard is clean (all schema changes are additive). One concrete gap blocks merge: the mobile useRescheduleSuggestion hook ships without a direct test file while its web counterpart has a 94-line suite covering five behaviors — a parity deficiency that the adversarial pass confirmed is real (the component test fully mocks the hook and exercises none of its internal logic).
Findings
Critical
None.
High
None.
Medium
[MEDIUM] Missing mobile hook test — apps/mobile/__tests__/hooks/use-reschedule-suggestion.test.ts does not exist
- dimension: Parity (#9) / Tests (CLAUDE.md)
- location:
apps/mobile/hooks/use-reschedule-suggestion.ts(no corresponding test file) - issue:
apps/web/__tests__/hooks/use-reschedule-suggestion.test.tscovers 5 behaviors: fetch-when-enabled, no-fetch-when-disabled, no-fetch-for-empty-habitId, error surfacing, and URL construction with the language parameter. No equivalent exists for mobile. The mobile reschedule-sheet test fully mocks the hook (vi.mock('@/hooks/use-reschedule-suggestion')) so the real hook logic is never executed by any test. - risk: The
enabled && !!habitIddouble-guard and theURLSearchParams({ language: locale })URL-construction are the two paths most likely to break silently on refactor (e.g. changing the empty-string guard, changing the query-param key name). All 34 other hook test files inapps/mobile/__tests__/hooks/have a corresponding test. The gap is inconsistent with the project's test strategy. - fix: Add
apps/mobile/__tests__/hooks/use-reschedule-suggestion.test.tsmirroring the 5 web test cases. MockapiClient(notfetch) per the mobile testing convention. Closest precedent:apps/mobile/__tests__/hooks/use-habit-suggestion.test.tsx. - reference: CLAUDE.md "Every new feature needs tests. Tests asserting behavior, not implementation details."
Low / Info
None.
Subagents
| Agent | Verdict |
|---|---|
| parity-checker | PARTIAL — all component/hook/component-test files paired; mobile hook test absent |
| i18n-syncer | IN SYNC — all 10 new keys present in both en.json and pt-BR.json; aiDisclosure keys verified |
| contract-aligner | N/A — orbit-api not in diff scope (--skip-orbit-api; paired API PR is #258) |
| security-reviewer | N/A — orbit-api not in diff scope |
Validation
| Check | Result |
|---|---|
| Lint | N/A (--skip-validate flag) |
| Type check | N/A (--skip-validate flag) |
| Tests | N/A (--skip-validate flag) |
| Build (api) | N/A (--skip-orbit-api flag) |
Deferred
- Dimension 13 (Backend hard rules):
--skip-orbit-api— not reviewed. - Dimension 11 (Contract drift): orbit-api DTO alignment not verified; Zod schema additions are additive-only so no backward-compat risk on the client side.
- Dimension 12 (Security — API side): orbit-api not in scope.
- Validate phase:
--skip-validateflag; lint/type-check/test results not verified in this run.
What's good
buildRescheduleUpdateRequestis clean and well-tested: 5 unit tests cover recurring vs one-time, days, time-preservation vs override, and end-time cleanup. ~35 lines, one responsibility.- Fetch gating is disciplined:
enabled: open && hasProAccess && isOverdue && !!habitId— the AI endpoint never fires in the background for free users or non-overdue habits. - Token usage is correct:
tintFromPrimary(tokens, 0.1/0.28)on mobile,rgba(var(--primary-rgb), 0.10/0.28)on web — no raw violet rgba, no--slate-*refs. - i18n is complete: all 10 new keys landed in both locales in the same diff.
- Additive-only schema changes:
rescheduleSuggestionSchemaandrescheduleSuggestionResponseSchemaare purely additive — old mobile clients are unaffected. - Full web + mobile surface parity: reschedule-sheet, use-reschedule-suggestion hook, habit-list, habit-row-menu, habit-row — all updated symmetrically with matching logic and error handling.
Recommendation
Add apps/mobile/__tests__/hooks/use-reschedule-suggestion.test.ts covering the same 5 behaviors as the web hook test. That is the only thing blocking this PR. Everything else is solid.
Generated with Claude Code
There was a problem hiding this comment.
PR #319 APPROVE: Full parity across web and mobile, i18n synced in both locales, all schema changes additive-only (no backward-compat risk), token usage correct. One medium non-blocking finding: apps/mobile/tests/hooks/use-reschedule-suggestion.test.ts is missing. The web counterpart covers 5 behaviours (fetch-when-enabled, no-fetch-when-disabled, no-fetch-for-empty-habitId, error surfacing, URL param construction) but no equivalent mobile hook test exists -- the mobile reschedule-sheet test mocks the hook entirely so the enabled && !!habitId guard and URLSearchParams locale param are untested. All 34 other mobile hooks have a test file. Fix: add the mobile hook test mirroring web, using apiClient mock per mobile convention (see use-habit-suggestion.test.tsx). Everything else is clean -- ship it.


What
Smart AI reschedule for missed habits, full web + mobile parity.
habit.isOverdue).reschedule-sheet.tsx, web onAppOverlay, mobile on the TrueSheetBottomSheetModal): ASTRA eyebrow + AI-label badge, the proposed schedule (date · time · cadence), the rationale, and the "not medical advice" disclaimer.useUpdateHabitpath via a new sharedbuildRescheduleUpdateRequesthelper that merges the suggestion's schedule onto the current habit while preserving everything else (reminders, checklist, goals, end date are left untouched).useRescheduleSuggestionhook (web via BFFfetch, mobile viaapiClient) only fetches when the sheet is open + habit overdue + user is Pro.rescheduleSuggestionSchema/RescheduleSuggestionResponse,API.habits.rescheduleSuggestion,habitKeys.rescheduleSuggestion, factory, andhabits.reschedule.*+habits.actions.reschedulei18n in bothen.jsonandpt-BR.json.Why
Turns a slip into a recovery instead of a dead streak (issue #222).
Tests
buildRescheduleUpdateRequestmerge cases (recurring/one-time/days/due-time preserve+override).use-reschedule-suggestionhook +reschedule-sheet(accept builds the merged request, free → upgrade, error → retry).reschedule-sheet(same behaviors).Closes #222
Paired API PR: thomasluizon/orbit-api#258
🤖 Generated with Claude Code