feat(api): AI reschedule suggestion (#222)

[thomasluizon]

What

Adds an AI "smart reschedule" suggestion for overdue (missed) habits, reusing the existing Astra AI infrastructure.

• New read endpoint GET /api/habits/{id}/reschedule-suggestion → GetRescheduleSuggestionQuery + handler: Pro-gated (CanUseSmartReschedule / AppConfigKeys.SmartRescheduleProOnly, default true), guarded by HabitScheduleService.IsOverdueOnDate, with a short 1h (habitId, DueDate, language) cache.
• AiRescheduleSuggestionService (mirrors AiSummaryService): builds a prompt from the habit's schedule, days-overdue, and recent log history; runs on the cheaper SubTask model with a bounded token budget + low temperature; validates/clamps the raw LLM output at the boundary (dueDate ≥ today and ≤ EndDate, positive quantity when a unit is set, days only for a daily-quantity-one cadence, parsed due time) before returning.
• Additive maxOutputTokens param on AiCompletionClient.CompleteJsonAsync (default null → no behavior change for existing callers).
• Acceptance reuses the existing UpdateHabitCommand write path — no new write command, no schedule-mutation logic, no migration.

Why

Turns a missed habit into a one-tap recovery instead of a dead streak (issue #222).

Tests

• GetRescheduleSuggestionQueryHandlerTests — gate fail, user/habit not found, not-overdue, service failure, cache hit, language fallback.
• AiRescheduleSuggestionServiceTests — clamp/parse boundary cases + prompt builder.
• dotnet build + dotnet test green (Application 2144, Infrastructure 1219).

Refs thomasluizon/orbit-ui-mobile#222

🤖 Generated with Claude Code

[claude]

Code Review: PR #258 — feat(api): AI reschedule suggestion

Scope: PR #258 in thomasluizon/orbit-api
Recommendation: NEEDS WORK

Summary

This PR adds a new GET /api/habits/{id}/reschedule-suggestion endpoint backed by a new AiRescheduleSuggestionService that wraps GPT via the existing AiCompletionClient. The feature is well-structured — it mirrors the established GetDailySummaryQuery / AiSummaryService pattern, validates and clamps all raw LLM output at the boundary, and includes comprehensive unit tests. One issue blocks merge: the query is missing its FluentValidation validator (required by the CLAUDE.md hard rules and the project convention for every new command/query), which leaves the free-form language query string parameter uncapped and used verbatim as a cache-key segment — a denial-of-wallet and memory-pressure vector.

Findings

Critical

None.

High

[HIGH] Missing FluentValidation validator for GetRescheduleSuggestionQuery
· dimension: Backend hard rules (#13) / Security — Validation (#12)
· location: orbit-api/src/Orbit.Application/Habits/Validators/ (file absent from this PR)
· issue: Every new query handler needs a validator in Orbit.Application/<Feature>/Validators/ — this is both the CLAUDE.md hard rule and the established project pattern (see GetDailySummaryQueryValidator, GetCalendarMonthQueryValidator, GetHabitScheduleQueryValidator). GetRescheduleSuggestionQuery accepts a free-form Language string from the query string with no length cap or allowlist check, and HabitId is not checked for Guid.Empty. Validators run automatically via the MediatR ValidationBehavior pipeline — without one, no input is validated before the handler runs.
· risk: An attacker submits an arbitrarily long Language value; it flows verbatim into CacheKey → $"reschedule:{habitId}:{dueDate:yyyy-MM-dd}:{language}", creating a new cache entry and triggering a paid OpenAI call on every unique value. Combined with the 1-hour TTL this is a denial-of-wallet and memory-pressure vector for any authenticated user.
· fix: Add src/Orbit.Application/Habits/Validators/GetRescheduleSuggestionQueryValidator.cs following the SetLanguageCommandValidator allowlist pattern:

public class GetRescheduleSuggestionQueryValidator : AbstractValidator<GetRescheduleSuggestionQuery>
{
    public GetRescheduleSuggestionQueryValidator()
    {
        RuleFor(q => q.HabitId).NotEmpty();
        RuleFor(q => q.UserId).NotEmpty();
        RuleFor(q => q.Language)
            .MaximumLength(AppConstants.MaxLanguageLength)
            .Must(lang => string.IsNullOrEmpty(lang) || AppConstants.SupportedLanguages.Contains(lang))
            .WithMessage($"Language must be one of: {string.Join(", ", AppConstants.SupportedLanguages)}");
    }
}

Also normalise the cache key language segment: effectiveLanguage.ToLowerInvariant() — so "EN" and "en" share the same cache entry.
· reference: orbit-api CLAUDE.md "Validation" hard rule; Orbit.Application CLAUDE.md Validation section; SetLanguageCommandValidator as the established pattern.

Medium

None.

Low / Info

None posted (signal gate applied).

Subagents

Agent	Verdict
security-reviewer	FAIL — missing validator (folded into High above); no HTTP-layer rate limit noted but N/A: neither GetDailySummary nor GetRetrospective on the same controller have one, so this matches peers
contract-aligner	N/A — --skip-cross-repo flag; new endpoint is purely additive, no existing fields removed or renamed

Validation

Check	Result
Build (dotnet)	N/A — --skip-build flag set
Tests (dotnet)	N/A — --skip-build flag set

What's good

• The AiRescheduleSuggestionService output-boundary validation is thorough: ResolveDueDate clamps past dates to today and past-EndDate to EndDate, then fails fast if the result is still before today; ResolveCadence handles null/invalid unit and non-positive quantity; ResolveDays is gated strictly to daily/qty=1; ResolveDueTime is null-tolerant. These are exactly the right defences at an LLM output boundary.
• PromptDataSanitizer.QuoteInline sanitises habit title before embedding it in the prompt — no injection path from user-controlled habit names.
• The language value reaching the AI prompt is safely indirected through LocaleHelper.GetAiLanguageName, returning a fixed English string regardless of raw input — no injection from that surface.
• Ownership is enforced correctly in the repository filter: h.Id == request.HabitId && h.UserId == request.UserId (UserId is from the JWT claim, not the request body).
• Test coverage is comprehensive: gate fail, user not found, habit not found, not-overdue guard, service failure, cache hit on second call, language override from user profile, and English fallback. Infrastructure-level tests exercise all boundary helpers as pure functions — good separation.
• [LoggerMessage] source generators throughout — no string-interpolated log calls, no PascalCase violations.
• maxOutputTokens = 220 on the AI call is a thoughtful cost-control measure.
• The FromCache boolean is consistent with DailySummaryResponse / RetrospectiveResponse.

Recommendation

Add GetRescheduleSuggestionQueryValidator with HabitId/UserId not-empty and Language max-length + allowlist rules, and normalise the cache key language segment to lowercase. Everything else is well-executed — once those land, this is ready to merge.

[claude]

Code Review: PR #258

Scope: PR #258 feat(api): AI reschedule suggestion (#222) in thomasluizon/orbit-api
Recommendation: NEEDS WORK

Summary

PR #258 adds a well-structured GET /api/habits/{id}/reschedule-suggestion endpoint that is Pro-gated, LLM-backed, cached, and fully tested. The architecture, prompt injection mitigations, LLM output clamping, ownership checks, and test coverage are all solid. One High finding blocks merge: the handler computes userToday via raw DateTime.UtcNow + TimeZoneInfo instead of IUserDateService.GetUserTodayAsync, directly violating the explicit project-wide timezone rule — every other habits query in the same folder uses the service. There are no Critical findings and no backward-compat breaks (the endpoint is purely additive).

Findings

Critical

None

High

[HIGH] userToday computed inline via DateTime.UtcNow — violates the mandatory IUserDateService contract

• dimension: Backend hard rules (#13) / Correctness (#1)
• location: src/Orbit.Application/Habits/Queries/GetRescheduleSuggestionQuery.cs:43-45
• issue: The handler manually resolves today's date with TimeZoneInfo.ConvertTimeFromUtc(DateTime.UtcNow, userTimeZone) / DateOnly.FromDateTime(userNow). Every other query in this directory — GetHabitByIdQuery, GetHabitWidgetQuery, GetHabitScheduleQuery, GetHabitFullDetailQuery, GetCalendarMonthQuery, GetHabitLogsQuery — calls IUserDateService.GetUserTodayAsync(userId). The CLAUDE.md cross-cutting rule is explicit: "NEVER use DateOnly.FromDateTime(DateTime.UtcNow) for user-facing logic."
• risk: userToday drives the overdue check (HabitScheduleService.IsOverdueOnDate) and the cache key. If a user's timezone places them near a day boundary at request time, the inline computation can differ from the authoritative value the rest of the system uses — causing a habit to be incorrectly flagged as overdue (or not), and a stale suggestion to be served from cache under the wrong date bucket for up to one hour.
• fix: Inject IUserDateService into GetRescheduleSuggestionQueryHandler and replace lines 43–45:

  var userToday = await userDateService.GetUserTodayAsync(request.UserId, cancellationToken);

  Remove the now-unused userTimeZone and userNow locals. Also fix the test file line 25 — replace DateOnly.FromDateTime(DateTime.UtcNow) with a pinned date (e.g. new DateOnly(2026, 4, 1)), matching the style already used in AiRescheduleSuggestionServiceTests.
• reference: CLAUDE.md "Timezone" cross-cutting hard rule; Backend hard rules dimension #13

Medium

None

Low / Info

None

Subagents

Agent	Verdict
security-reviewer	PASS on authorization, ownership, prompt injection, logging, input validation — IUserDateService finding promoted to High above; 2 Low findings (rate-limit gap, DueDate cache-key) below signal gate
contract-aligner	NOT VERIFIABLE IN CI — orbit-ui-mobile repo not checked out; API side is MATCH (endpoint, route, auth, DTO shape consistent); consumer Zod schema + endpoint constant must be verified in the linked orbit-ui-mobile#222 PR

Validation

Check	Result
Build (dotnet)	N/A — skipped (CI handles it)
Tests (dotnet)	N/A — skipped (CI handles it)

What's good

• Clean CQRS shape: query + handler + validator in the correct folders, mirroring every existing feature.
• LLM output hardened at the boundary: BuildSuggestion clamps every field — past dates floored to today, non-positive quantities clamped to 1, invalid enum values silently dropped, rationale truncated at sentence boundaries. The LLM is treated correctly as untrusted input.
• Prompt injection mitigated: PromptDataSanitizer.QuoteInline(habit.Title, 120) sanitizes user-controlled input before it enters the prompt; the system instruction separates data from instructions.
• Authorization solid: controller inherits class-level [Authorize], UserId comes exclusively from JWT (HttpContext.GetUserId()), habit fetch enforces h.UserId == request.UserId — no IDOR vector.
• Test coverage thorough: gate fail, user/habit not found, not-overdue, cache hit, language fallback, and all boundary clamp cases are covered. [LoggerMessage] source generators used throughout.
• Additive-only: no DTO fields removed or renamed, no migration, no existing behavior changed — backward-compat is clean.
• maxOutputTokens change is non-breaking: default is null, all existing callers are unaffected.

Recommendation

Fix the IUserDateService violation (lines 43–45 in the handler + line 25 in the test). It is a one-line swap in production code that brings the handler in line with every other query in the same folder. Everything else is ready to merge.

🤖 Generated with Claude Code

[claude]

Code Review: PR #258 — feat(api): AI reschedule suggestion

Recommendation: APPROVE

Summary

PR #258 adds GET /api/habits/{id}/reschedule-suggestion using the existing Astra AI infrastructure to propose a new schedule for an overdue habit. The implementation follows all established patterns: CQRS layout, PayGate check before handler work, IUserDateService for user-facing dates, [LoggerMessage] structured logging, FluentValidation + domain guard, and 1-hour IMemoryCache. All new code has unit tests covering the full gate/not-found/overdue/cache/language-fallback matrix. No migration, no schema change, no breaking change to existing endpoints.

One Medium finding should be fixed before or shortly after merge. No Critical or High findings survive the signal gate.

---

Findings

Critical — None

High — None

Rate-limiting note: The security subagent flagged the absence of [DistributedRateLimit] on the new endpoint. After cross-checking, GetDailySummary and GetRetrospective in the same controller are identically unguarded — pre-existing debt that predates this PR. A pre-existing issue not worsened by the diff is not a blocking finding here.

Medium

[MEDIUM] Unvalidated user-profile language bypasses the supported-languages whitelist

• dimension: Security / Validation (rubric #12 — validation at API boundary; rubric #13 — backend is source of truth)
• location: src/Orbit.Application/Habits/Queries/GetRescheduleSuggestionQuery.cs:79-86 (ResolveLanguage)
• issue: ResolveLanguage prefers user.Language over request.Language. The validator whitelists request.Language against AppConstants.SupportedLanguages, but user.Language is taken as-is without that check. If the profile field holds an arbitrary string (DB edit, a past bug, or a future SetLanguage regression), that string passes into LocaleHelper.GetAiLanguageName(language) and is embedded in the AI prompt — and is also logged as {Language} in the structured log.
• risk: A user whose profile language is set to a crafted value can inject arbitrary text into the prompt preamble. Structured-log sinks also index the raw value.
• fix: After ResolveLanguage, clamp to the whitelist before use:

var effectiveLanguage = ResolveLanguage(user.Language, request.Language);
if (!AppConstants.SupportedLanguages.Contains(effectiveLanguage))
    effectiveLanguage = "en";

• reference: orbit-api CLAUDE.md "Validation: backend is the source of truth"; rubric dimension 12

Low / Info — None (signal gate)

---

Subagents

Agent	Verdict
security-reviewer	PASS (1 pre-existing rate-limit gap deferred; 1 Medium filed above)
contract-aligner	N/A — orbit-ui-mobile not checked out; new types are purely additive

Validation

Check	Result
Build (dotnet)	N/A (skipped per CI instruction)
Tests (dotnet)	N/A (skipped per CI instruction — PR body reports green: Application 2144, Infrastructure 1219)

---

What's good

• Clean CQRS structure: query record + handler + validator + domain interface + infrastructure implementation all in the right folders, following every established pattern.
• IUserDateService used correctly: GetUserTodayAsync is the sole source of "today" — no DateOnly.FromDateTime(DateTime.UtcNow) in user-facing logic.
• Prompt injection hardened: habit.Title passes through PromptDataSanitizer.QuoteInline before entering the raw-string prompt.
• LLM output fully clamped at the boundary: ResolveDueDate, ResolveCadence, ResolveDays, ResolveDueTime all parse and clamp raw model output. Tests cover end-date-before-today failure, past-date clamp, invalid string, and duplicate day dedup.
• Token budget bounded: maxOutputTokens: 220 and temperature 0.2 on the SubTask model keeps cost predictable.
• Authorization tight: controller class carries [Authorize]; new action has no [AllowAnonymous]; UserId comes from JWT claims; ownership enforced with h.UserId == request.UserId in the repo predicate.
• PayGate safe-defaults: defaults true (Pro-only) on missing config key — safe-fail posture.
• Test coverage: handler tests cover gate-fail, user-not-found, habit-not-found, not-overdue, service-fail, cache-hit, profile-language-priority, and empty-language-fallback — every branch exercised.

---

Recommendation

APPROVE with one actionable Medium fix (OK to merge and address in a follow-up):

Clamp effectiveLanguage to AppConstants.SupportedLanguages after ResolveLanguage in GetRescheduleSuggestionQueryHandler.Handle. This closes the prompt-injection surface on the user-profile language field and removes the log-injection vector simultaneously.

🤖 Generated with Claude Code

[sonarqubecloud]

Quality Gate failed

Failed conditions
64.7% Coverage on New Code (required ≥ 80%)

See analysis details on SonarQube Cloud

[claude]

PR 258 review - REQUEST CHANGES - 2 High security findings.

WHAT IS GOOD: LLM output clamping thorough; timezone routing correct via IUserDateService; FindAsync ownership predicate tight; prompt injection mitigated; structured logging; comprehensive tests; canonical CQRS layout; no migrations.

HIGH-1: Missing DistributedRateLimit on new AI endpoint. Location: HabitsController.cs GetRescheduleSuggestion. Every other AI endpoint has it (habit-suggest on POST suggest-setup line 207, chat on ChatController, ai-resolve on AiController). Without it an authenticated Pro user can bypass the 1-hour cache TTL by cycling different habit GUIDs and trigger unbounded OpenAI calls. Fix: add [DistributedRateLimit("habit-suggest")] to the action.

HIGH-2: Cache key omits userId - IDOR gap on cache hits. Location: GetRescheduleSuggestionQuery.cs CacheKey and TryGetValue branch. Key is reschedule:{habitId}:{dueDate}:{language}. The TryGetValue branch executes before FindAsync which is the only ownership check (h.UserId == request.UserId). On a cache hit the handler returns without verifying ownership. Pattern in GetDailySummaryQuery.cs line 150 always puts userId first. Fix: add userId as the first segment and pass request.UserId at the call site.

MEDIUM: FromCache exposed in public response DTO (GetRescheduleSuggestionQuery.cs line 119). No other AI DTO (DailySummaryResponse, HabitSuggestionResponse, RetrospectiveResponse) exposes this. Once shipped it cannot be removed without a breaking change. Remove unless there is a documented product requirement.

CROSS-REPO: Not verifiable in CI - orbit-ui-mobile absent. Verify Zod schema and endpoints.ts constant in companion PR orbit-ui-mobile#222.

The two High findings are each one-liner fixes. Please address and re-request review.

[claude]

[HIGH] Missing [DistributedRateLimit] — unbounded OpenAI spend risk

Every other AI-backed action carries a rate-limit attribute ([DistributedRateLimit("habit-suggest")] on POST suggest-setup line 207, [DistributedRateLimit("chat")] on ChatController, [DistributedRateLimit("ai-resolve")] on AiController). CanUseSmartReschedule checks only the Pro flag and does not debit tokens. An authenticated Pro user can cycle different habit GUIDs to bypass the 1-hour cache TTL and trigger unlimited live OpenAI calls.

Suggested change

	[HttpGet("{id:guid}/reschedule-suggestion")]
	[HttpGet("{id:guid}/reschedule-suggestion")]
	[DistributedRateLimit("habit-suggest")]

[claude]

[HIGH] Cache key omits userId — IDOR ownership check bypassed on cache hits

TryGetValue (line 60) executes before FindAsync, which is the only place h.UserId == request.UserId is enforced. On a cache hit the handler returns without ever verifying ownership. A different authenticated user who obtains a habit GUID can receive another user's cached reschedule suggestion.

Established pattern (GetDailySummaryQuery.cs lines 150–151): userId is always the outermost namespace of the key.

Suggested change

	private static string CacheKey(Guid habitId, DateOnly dueDate, string language) =>
	$"reschedule:{habitId}:{dueDate:yyyy-MM-dd}:{language.ToLowerInvariant()}";
	private static string CacheKey(Guid userId, Guid habitId, DateOnly dueDate, string language) =>
	$"reschedule:{userId}:{habitId}:{dueDate:yyyy-MM-dd}:{language.ToLowerInvariant()}";

Also update the call site (line 59) to pass request.UserId as the first argument.