perf(api): cache reference reads + invalidate goal AI and reference caches

[thomasluizon]

What

Fixes two cache-correctness gaps and adds a read-cache layer for reference data.

Goal AI-cache invalidation

Goal mutation commands (Goals/Commands/) never cleared any AI cache, so the per-user goal-review cache (and goal-influenced summaries) stayed stale for up to an hour after a goal edit.

• New CacheInvalidationHelper.InvalidateGoalReviewCache, folded into InvalidateUserAiCaches. Because goal-review derives from a user's goals and their linked habits' logs, this means every habit and goal mutation now refreshes it (habit logging previously left goal-review stale too).
• All 8 goal mutation commands now call InvalidateUserAiCaches, mirroring the habit commands.

Reference-data read cache + invalidation

GetTagsQuery / GetChecklistTemplatesQuery (frequently-hit reference data) had no cache; UserFacts / ApiKeys reads likewise. Added a per-user IMemoryCache read layer via a centralized ReferenceCacheKeys (one key definition shared by the read and every invalidator, so they can't drift).

Invalidated on every write path, not just the obvious commands:

• Tags — create/update/delete/restore
• ChecklistTemplates — create/delete
• UserFacts — delete/bulk-delete and AI fact extraction (OpenAiBatchPollerService)
• ApiKeys — create/revoke and key usage (ApiKeyAuthenticationHandler.MarkUsed, which updates LastUsedAtUtc)

Behavior

Behavior-preserving: reads are fresh immediately after any mutation. A short backstop TTL bounds staleness only if some future write path is added without an invalidation.

Tests

7 new tests: each reference resource proves the read is cached and a mutation busts it (re-queries the repo); goal tests prove InvalidateGoalReviewCache clears every supported language and that a goal mutation clears a cached goal-review. Full suite green (4885 passed, 0 failed).

Note

Left untouched (pre-existing, out of this change's scope): InvalidateRetrospectiveCache builds its key without the retro:v2: prefix the read uses and only scans a ±2-day window that won't match month/quarter/year period-start DateFroms — retrospective invalidation is effectively a no-op today. Retrospectives don't depend on goals, so this PR doesn't rely on it; a correct fix needs async period-range resolution and belongs in its own change.

Refs thomasluizon/orbit-ui-mobile#243

[claude]

Review Complete

Scope: PR #353 (orbit-api) — "perf(api): cache reference reads + invalidate goal AI and reference caches"
Recommendation: APPROVE

Summary

This PR fixes two real cache-correctness gaps (goal mutations never invalidated the goal-review AI cache; four reference-data resources had no caching at all) and adds a centralized, per-user-scoped read cache with matching invalidation on every write path, including the background OpenAiBatchPollerService and the ApiKeyAuthenticationHandler's MarkUsed() path. Invalidation coverage was verified complete: all 8 Goal command handlers call InvalidateUserAiCaches, all 4 Tag mutation commands (AssignTagsCommand is correctly excluded since TagResponse only exposes id/name/color, unaffected by habit-tag assignment), both ChecklistTemplate commands (confirmed no Update command exists to miss), and both UserFacts commands plus the AI batch-poller's fact-persistence path all invalidate their respective ReferenceCacheKeys entry. No DTO/endpoint/contract surface changed, so no backward-compat risk to mobile clients. Security review confirmed every cache key is scoped by JWT-derived UserId with no cross-user leakage vector, and the newly-cached ApiKeyResponse carries no secret material. No narration-comment or hard-rule violations found; the DateTime.UtcNow usage is the allowed cache-key case.

Findings

Critical

None

High

None

Medium

None

Low / Info

None (the classic cache-aside invalidate-then-repopulate race — a concurrent stale read repopulating a key right after another request's Remove — technically exists for all four newly-cached reference resources, but it mirrors the pre-existing summary/retrospective cache pattern, is bounded by the 10-minute ReferenceCacheKeys.Ttl backstop, and needs precise same-user concurrent write+read timing to manifest. Not filed as a finding per the signal gate.)

Subagents

Agent	Verdict
security-reviewer	PASS — no missing authorization, no cross-user cache leakage, no secret exposure in cached DTOs
contract-aligner	N/A — gate not triggered (no DTO field, Controller route, or packages/shared surface changed)

Validation

Check	Result
Build (dotnet)	N/A — skipped per instructions, CI handles separately
Tests (dotnet)	N/A — skipped per instructions, CI handles separately

Deferred — N/A dimensions & files not verdicted

• DESIGN.md/AI-slop: N/A — no apps/* UI files touched.
• Parity / i18n: not verifiable here — frontend-only dimensions owned by the orbit-ui-mobile-side review; sibling repo not checked out in this job.
• Contract drift: N/A — no DTO/Controller/shared-type surface changed in this diff.
• FEATURES.md parity: N/A — purely internal caching/perf change, no user-facing feature surface change.
• No prior reviews or review threads exist on this PR — this is a first-pass review, nothing to reconcile against.

What's good

• Centralizing cache keys in ReferenceCacheKeys.cs (single source shared by reads and every invalidator) directly prevents the classic key-drift bug class.
• Invalidation coverage was cross-checked exhaustively against every command file in the touched feature folders (Goals, Tags, ChecklistTemplates, UserFacts, ApiKeys) — nothing was missed, including the easy-to-forget background-job (OpenAiBatchPollerService) and auth-handler (MarkUsed) write paths.
• 7 new focused tests directly assert cache-hit-then-invalidate behavior via repository call-count assertions — a solid regression guard for exactly the bug class this PR fixes.
• The PR body's "Note" section proactively flags a pre-existing, out-of-scope InvalidateRetrospectiveCache correctness gap rather than silently leaving it — good scope discipline.

Recommendation

Merge as-is. No blocking issues found.

[sonarqubecloud]

Quality Gate passed

Issues
1 New issue
0 Accepted issues

Measures
0 Security Hotspots
98.5% Coverage on New Code
0.8% Duplication on New Code

See analysis details on SonarQube Cloud

[claude]

PR Review: #353 -- perf(api): cache reference reads + invalidate goal AI and reference caches

Recommendation: APPROVE (no Critical/High findings; 4 Medium findings noted below)

Severity	Count
Critical (incl. old-client breaks)	0
High	0
Medium	4
Low / Info	0

Findings (Medium)

1. CacheInvalidationHelper.InvalidateGoalReviewCache/InvalidateRetrospectiveCache (src/Orbit.Application/Common/CacheInvalidationHelper.cs:49) -- only clear whitelisted-language cache keys, but GoalsController.GetGoalReview/HabitsController.GetRetrospective accept an unvalidated language query param (pre-existing gap, no validator exists). A non-whitelisted language value creates a cache entry these new invalidation helpers can never clear, and separately bypasses the cache entirely on every read (cost amplification on OpenAI calls). Fix: add a language validator whitelisting AppConstants.SupportedLanguages, matching SuggestTagsQueryValidator. (Retrospective's period param is correctly validated via RetrospectivePeriodRange.IsKnownPeriod -- only language is the gap.)
2. Cache-aside race across all 4 new reference-data reads (Tags/ChecklistTemplates/UserFacts/ApiKeys) -- a concurrent read-miss + write can re-populate the cache with pre-mutation data after the write's invalidation already ran, surviving until the 10-minute backstop TTL. This contradicts the PR's "reads are fresh immediately after any mutation" claim under concurrency (e.g., parallel MCP tool calls from Astra).
3. OpenAiBatchPollerService.cs (Infrastructure) calls cache.Remove(ReferenceCacheKeys.UserFacts(...)) directly, contradicting Orbit.Infrastructure/CLAUDE.md's explicit rule: "Invalidate via CacheInvalidationHelper in Application (never directly here)." Unlike AppConfigService/FeatureFlagService, which cache their own locally-owned keys, this reaches into Application's ReferenceCacheKeys namespace.
4. Missing test: no test covers OpenAiBatchPollerService.CompleteBatchAsync's new UserFacts cache-invalidation behavior (invalidate iff persistedFactCount > 0).

What's good

Cache-invalidation ordering is correct in every one of the 8 Goal commands and all Tags/ChecklistTemplates/UserFacts/ApiKeys commands checked (always after SaveChangesAsync, after every early-return validation/not-found guard). PayGate authorization checks consistently run before cache reads (GetApiKeysQuery, GetUserFactsQuery, GetGoalReviewQuery), so cached responses can't bypass plan-gating. ApiKeyResponse never exposes secret/hash material. Cache keys are cleanly namespaced per-user GUID with no cross-tenant leakage risk. Good test coverage added for the reference-cache read/invalidate round-trip (7 new tests) and DI wiring updated consistently across ~20 existing test files.

Subagents

• security-reviewer: 1 finding, downgraded High->Medium on adversarial verification (pre-existing root cause outside this diff's changed files).
• contract-aligner: N/A -- no DTO/route/shared-type surface changed in this diff.

Validation

Build/Tests: N/A -- covered by separate required CI checks (Build / Unit Tests / SonarCloud).

Deferred

Parity/i18n: N/A -- no apps/* changes. FEATURES.md parity: N/A -- PR is behavior-preserving (perf/caching only).