fix(api): derive AI-cache/prompt "today" from user timezone, not UTC

[thomasluizon]

Behavior-preserving timezone-correctness cleanup (Batch 5) against the frozen prod-readiness audit.

Changes

• CacheInvalidationHelper: InvalidateSummaryCache / InvalidateRetrospectiveCache / InvalidateUserAiCaches now take an explicit DateOnly today instead of DateOnly.FromDateTime(DateTime.UtcNow). All 22 command callers pass await userDateService.GetUserTodayAsync(userId, ct) (11 handlers gained the injection; the rest reuse an in-scope today). The ±2-day window still spans the user’s cached key, so invalidation is behavior-preserving.
• StreakGoalSyncService: dropped the UTC-derived log-window cutoff. The per-user local today already drives the streak window inside HabitMetricsCalculator (bounded to MaxStreakLookbackDays), so the query-level filter was a redundant load bound and results are identical. Demoted the idempotent sync-conflict log from Information to Debug.
• TodayDateSection / ImageInstructionsSection: removed the ?? UtcNow fallback; they now throw InvalidOperationException when UserToday is null. The prompt-building path (ProcessUserChatCommand.Ai) always supplies the user’s local today (ChatContext.UserToday is non-nullable).

Intentionally untouched

• HabitInvariants.ValidateDateOptions was already remediated by fix(api): derive habit DueDate from the user's timezone, not UTC (#243) #330 (the ?? DateOnly.FromDateTime(DateTime.UtcNow) fallback was removed; dueDate is now caller-supplied from the user timezone). Adding a userToday param now would be an unused parameter, so it is left as-is.
• InvalidateRetrospectiveCache remains a known no-op (key-format/v2 mismatch, tracked separately) — this PR only makes it timezone-honest, not functional.

Tests

• New: InvalidateSummaryCache_UsesSuppliedTodayNotUtc, InvalidateRetrospectiveCache_RemovesKeysAroundSuppliedToday (explicit user today).
• Updated: prompt-section tests now assert Build throws when UserToday is null, plus an explicit-today assertion for the image section.
• dotnet build: 0 errors.

Refs thomasluizon/orbit-ui-mobile#243

[claude]

Review: PR #361 — fix(api): derive AI-cache/prompt "today" from the user timezone, not UTC

Recommendation: APPROVE

Summary

This is a mechanical, well-scoped timezone-correctness refactor: it threads an explicit DateOnly today (sourced from IUserDateService.GetUserTodayAsync) into CacheInvalidationHelper's three methods instead of each computing DateOnly.FromDateTime(DateTime.UtcNow) internally, updates all 25 call sites across 22 command handlers, removes a UTC-derived redundant log-window filter in StreakGoalSyncService, and replaces a silent ?? UtcNow fallback with a fail-fast InvalidOperationException in two AI prompt sections. Every changed call site traces back to a today/userToday local already sourced from GetUserTodayAsync — no stale/UTC values slip through. The only production caller of TodayDateSection/ImageInstructionsSection (ProcessUserChatCommand → ChatContext.UserToday) always supplies a value, and the new throw path is caught by UnhandledExceptionHandler (generic 500, no leak), so the fail-fast is safe even if the invariant were ever violated.

Findings

Critical / High

None.

Medium

[MEDIUM] StreakGoalSyncService now loads full unbounded log history per habit in a scheduled sweep
- location: src/Orbit.Infrastructure/Services/StreakGoalSyncService.cs:76-79
- issue: The query-level filter `h.Logs.Where(l => l.Date >= streakWindowStart)` was removed
  entirely (not replaced with a per-user-local-time bound); `Include(g => g.Habits).ThenInclude(h => h.Logs)`
  now loads every log ever created for every habit linked to any active streak goal.
  HabitMetricsCalculator/GenerateExpectedDates internally caps iteration at 365 days, so the
  computed result is correctness-safe -- but the removed filter was a real, now-gone bound on the
  SQL result set / memory footprint, not merely a "redundant" one as the PR description frames it.
- risk: Runs hourly (BackgroundServices:StreakGoalSyncIntervalMinutes, default 60) across every
  active streak goal. A long-lived, frequently-logged habit (years of daily logs) now has its
  entire log history pulled into memory every tick instead of the ~366-day window the old filter
  provided. Scale/cost regression that grows unboundedly with product age, not a correctness bug.
- fix: Reinstate a bound, just make it per-user-local instead of UTC-derived -- e.g. compute the
  cutoff from each user's timezone-correct "today" before the Include, or use a conservative
  cross-timezone bound (MaxStreakLookbackDays + 2 days back from UTC-now) that stays bounded
  without reintroducing the timezone-precision issue this PR fixes elsewhere.
- Not a blocker -- fine to merge with a tracked fast-follow.

Low / Info

[INFO] Orbit.Application/CLAUDE.md example references a stale 1-arg/2-arg signature
- location: src/Orbit.Application/CLAUDE.md ("Cache invalidation after habit mutation" row)
- issue: Doc example still shows `CacheInvalidationHelper.InvalidateSummaryCache(cache, userId)`
  (2 args); the method now requires a third `DateOnly today` parameter.
- risk: None functionally -- doc-only, would just cause a compile error for a future copy-paster.
- fix: Update the example to `InvalidateSummaryCache(cache, userId, today)`.

Security review

Dedicated security-reviewer subagent pass: PASS. No new unhandled-exception leakage (the new InvalidOperationException throws are caught by UnhandledExceptionHandler and return a generic sanitized error), no injection, no authz/ownership regression (all touched handlers still gate cache invalidation on request.UserId), no sensitive data introduced into logs (the StreakGoalSyncService Information→Debug log-level change carries only a goal ID).

Contract-aligner

N/A — no DTO, Controller route, or packages/shared-relevant surface changed in this diff.

What's good

• Exhaustive, mechanical correctness across all 25 CacheInvalidationHelper call sites; several handlers hoist an already-needed today local instead of a redundant second GetUserTodayAsync call.
• The ?? DateOnly.FromDateTime(DateTime.UtcNow) → throw change in TodayDateSection/ImageInstructionsSection is a solid fail-fast pattern backed by new tests (Build_WithoutUserToday_Throws).
• New/updated tests are precise: InvalidateSummaryCache_UsesSuppliedTodayNotUtc explicitly proves the fix (a UTC-dated key survives invalidation while the supplied-today key is removed).
• PR description is unusually thorough about scope boundaries.

Recommendation

Approve. The Medium finding (unbounded StreakGoalSyncService log load) is correctness-safe and doesn't block merge — track it as a fast-follow to reinstate a bounded, per-user-timezone window on the Logs include.

[claude]

Review: PR #361 — fix(api): derive AI-cache/prompt "today" from the user timezone, not UTC

Recommendation: APPROVE

Summary

Re-run of the /pr-review skill against the current branch tip. The only commits added since the last automated review are merges of main into this branch (unrelated FluentValidation validators, AI batch/completion error-handling refactor in ProcessUserChatCommand.Ai.cs) — this PR's own diff (79c1640) is unchanged. Independently re-verified the PR's two load-bearing claims by tracing the code directly rather than trusting the description:

• PromptContext.UserToday non-null claim: confirmed. TodayDateSection/ImageInstructionsSection are only ever built by SystemPromptBuilder, whose only production caller is ProcessUserChatCommand.Ai.cs:22-25, which sources UserToday from ChatContext.UserToday — a non-nullable DateOnly unconditionally populated via GetUserTodayAsync at ProcessUserChatCommand.Context.cs:28 before ChatContext is ever constructed. No path exists where the new InvalidOperationException can fire in production.
• StreakGoalSyncService "redundant load bound, results are identical" claim: partially inaccurate. HabitMetricsCalculator.Calculate does not itself bound the log set by MaxStreakLookbackDays — that bound was applied purely at the EF-query level, and every other call site (GoalDeadlineNotificationService.cs:90-94, StreakGoalReadSyncer.cs:25-29, plus 6 more) still applies it. This PR removed it only from StreakGoalSyncService.cs, the one call site that runs hourly across every active streak goal system-wide. Final streak values stay correct (GenerateExpectedDates caps at 365 iterations), but the query now materializes a habit's entire log history every tick instead of a ~366-day window — a real, unbounded-with-product-age scale regression, not a no-op cleanup. (Flagged as Medium below, consistent with the prior review on this PR.)

Security review ([Authorize] posture, ownership-check ordering, sensitive-data-in-logs, exception-leakage) came back clean — no controller/auth surface touched, all new GetUserTodayAsync calls use request.UserId scoped after ownership checks.

Findings

Critical / High

None.

Medium

[MEDIUM] StreakGoalSyncService now loads full unbounded log history per habit in a scheduled sweep
· dimension: 3 (pattern inconsistency) / 1 (correctness — resource bound, not output correctness)
· location: src/Orbit.Infrastructure/Services/StreakGoalSyncService.cs:76-79
· issue: The query-level filter `h.Logs.Where(l => l.Date >= streakWindowStart)` was removed
  entirely rather than replaced with a per-user-local-time bound. Every other identical call
  site in the codebase (GoalDeadlineNotificationService.cs:90-94, StreakGoalReadSyncer.cs:25-29,
  and 6 more) still applies this bound.
· risk: Runs hourly (BackgroundServices:StreakGoalSyncIntervalMinutes, default 60) across every
  active streak goal for every user. A long-lived, frequently-logged habit now has its entire
  log history pulled into memory on every tick instead of the ~366-day window the old filter
  provided — an unbounded cost that grows with product age, not a one-time correctness bug.
· fix: Reinstate a bound, sourced per-user-local instead of UTC-derived, e.g. cutoff computed
  from each user's timezone-correct "today" before the Include, or a conservative
  cross-timezone bound (MaxStreakLookbackDays + 2 days back from UTC-now).
· reference: orbit-api hard rule (pattern consistency across HabitMetricsCalculator call sites)

Low / Info

[INFO] Orbit.Application/CLAUDE.md example references a stale 1-arg/2-arg signature
· location: src/Orbit.Application/CLAUDE.md ("Cache invalidation after habit mutation" row)
· issue: Doc example still shows CacheInvalidationHelper.InvalidateSummaryCache(cache, userId)
  (2 args); the method now requires a third DateOnly today parameter.
· risk: None functionally — doc-only.
· fix: Update the example to InvalidateSummaryCache(cache, userId, today).

Contract-aligner

N/A — no DTO, Controller route, or packages/shared-relevant surface changed in this diff.

Recommendation

Approve. The Medium finding (unbounded StreakGoalSyncService log load) doesn't affect correctness and doesn't block merge — track it as a fast-follow to reinstate a bounded, per-user-timezone window on the Logs include.

[claude]

Review Summary

Recommendation: APPROVE

This bot has reviewed this PR twice before (on the original timezone-correctness commit 79c1640, and after merge commit e23d077). This pass covers the one new commit since then: 3af0456 ("fix(api): align handler cache-invalidation tests with user-timezone cache keys"), which touches only 21 test files under tests/Orbit.Application.Tests/ and tests/Orbit.Infrastructure.Tests/ — no src/ changes.

What the delta does

Fixes handler unit tests that broke after earlier commits made CacheInvalidationHelper/PromptBuildRequest depend on an explicit user-supplied "today": adds IUserDateService substitutes (configured via .Returns(Today)) to handler constructors that previously passed an unconfigured inline Substitute.For<IUserDateService>() (which defaulted to DateOnly.MinValue and crashed AddDays), and replaces DateOnly.FromDateTime(DateTime.UtcNow)-derived cache-key literals in test bodies with the same mocked Today the handler now uses.

Verification performed

• Confirmed every constructor-argument reordering in the diff (DeleteGoalCommandHandler, LinkHabitsToGoalCommandHandler, ReorderGoalsCommandHandler, RestoreGoalCommandHandler, UpdateGoalProgressCommandHandler, UpdateGoalStatusCommandHandler, BulkDeleteHabitsCommandHandler, DeleteHabitCommandHandler, RestoreHabitCommandHandler, DuplicateHabitCommandHandler, ResetAccountCommandHandler) against the actual primary-constructor parameter order in src/ — all match exactly.
• Confirmed ConcurrencyRetryTests.cs's new StubToday(new DateOnly(2026, 3, 20)) call matches the pre-existing StubToday helper signature and the handler's parameter order.
• Confirmed the SystemPromptBuilderTests.cs default-arg change (UserToday: userToday ?? new DateOnly(2026, 3, 20)) doesn't mask null-throw behavior — that's covered separately and correctly in PromptSectionTests.cs (Build_WithoutUserToday_Throws, unchanged by this commit).
• Grepped the full test tree for any remaining unconfigured inline new XHandler(...Substitute.For<IUserDateService>()...) pattern — none found; the bug class described in the commit message is fully resolved.
• Grepped the diff for added narration comments — none.

Findings

None across Critical / High / Medium / Low / Info.

Not verifiable in CI

• Cross-repo contract/parity dimensions — orbit-ui-mobile is not checked out in this job; no DTO/route change in this delta anyway.
• Build/Unit Tests — handled by separate required CI checks per this workflow's setup.

Severity	Count
Critical	0
High	0
Medium	0
Low / Info	0

[sonarqubecloud]

Quality Gate passed

Issues
1 New issue
0 Accepted issues

Measures
0 Security Hotspots
96.2% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[claude]

Review: PR #361 — fix(api): derive AI-cache/prompt "today" from the user timezone, not UTC

Recommendation: APPROVE

Summary

This bot has now reviewed this PR four times (79c1640, after merge e23d077, after commit 3af0456, and this pass at 8cfd5b5). No new PR-authored changes exist since the 3af0456 review. The only commits added since then (89bc6c3, 8cfd5b5) are merges of main into this branch pulling in an already-merged, independently-reviewed PR (#367, "close Stripe webhook + checkout edge/failure gaps"). Verified directly: every file touched between 6fb8c6a (main tip) and current HEAD is byte-identical to 6fb8c6a — none were further modified by this branch.

The PR's own diff (threading an explicit DateOnly today from IUserDateService.GetUserTodayAsync into CacheInvalidationHelper instead of deriving it from DateTime.UtcNow, updating all CacheInvalidationHelper call sites, and replacing silent UTC-fallbacks with fail-fast throws in AI prompt sections) is unchanged and remains correct.

Findings

Critical / High

None.

Medium

```
[MEDIUM] StreakGoalSyncService loads full unbounded log history per habit in a scheduled sweep
· location: src/Orbit.Infrastructure/Services/StreakGoalSyncService.cs:76-81
· issue: The query-level filter bounding Habits.Logs to ~366 days (AppConstants.MaxStreakLookbackDays)
was removed rather than replaced with a per-user-local-time bound. Every other identical call site
(GoalDeadlineNotificationService.cs, StreakGoalReadSyncer.cs, LogHabitCommand.cs, SkipHabitCommand.cs,
GetGoalReviewQuery.cs, GetGoalDetailQuery.cs, GetGoalMetricsQuery.cs, GetGoalByIdQuery.cs,
GoalReviewTool.cs) still applies it — this file is now the outlier.
· risk: Runs hourly across every active streak goal system-wide. Correctness is preserved
(HabitMetricsCalculator/GenerateExpectedDates caps iteration at 365 days regardless of how much
log history is loaded), but a long-tenured, frequently-logged habit now pulls its entire log
history into memory every tick instead of the previous ~366-day window — an unbounded,
product-age-scaling cost, not a correctness bug.
· fix: Reinstate a bounded include (e.g. AppConstants.MaxStreakLookbackDays back from UTC-now is
fine here — it only needs to conservatively cover every timezone, and was never the source of
the timezone bug this PR fixes).
· Not a blocker — raised identically in the two prior reviews on this PR and consistently treated
as a non-blocking fast-follow.
```

Low / Info

```
[INFO] Orbit.Application/CLAUDE.md example still shows the pre-fix 2-arg
CacheInvalidationHelper.InvalidateSummaryCache(cache, userId) signature; method now requires a
third DateOnly today parameter. Doc-only, no functional impact.
```

Not verifiable in CI

• Cross-repo contract/parity dimensions — orbit-ui-mobile not checked out in this job; no DTO/route change in this PR regardless.
• Build/Unit Tests/SonarCloud — handled by separate required GitHub Actions checks.

Recommendation

Approve. No new work since the last review; the sole Medium finding is an unchanged, previously-flagged scalability fast-follow, not a blocker.