feat(courses): Add AI chat persistence for video lessons

[pugal66]

• Integrate with LearnLens backend services to fetch past conversations and sync active chat threads
• Hydrate message history on opening the AI tab and seamlessly continue existing conversations
• Centralize session resolution and handle automatic authentication refresh on 401 retries

[claude]

Claude finished @pugal66's task in 2m 35s —— View job

---

PR Review

Todo list

• Read AGENTS.md, core/docs/ai_context.md, core/docs/architecture.md
• Check relevant ADRs (0005-user-state-in-core.md reviewed for session/provider context)
• Fetch full PR diff (git diff e0e2245..HEAD, matches PR's 15 changed files)
• Review OpenSpec compliance
• Review SDK boundary enforcement
• Review design system / widget usage
• Review accessibility semantics
• Review motion & animation
• Review platform neutrality
• Review navigation pattern (n/a — no new routes)
• Review Riverpod provider pattern
• Review data layer architecture
• Review generated files
• Review localization, icons, dependencies, feature flags
• Post final structured review summary

---

PR Review Summary

✅ What's good

• OpenSpec change (openspec/changes/ai-chat-persistence/) is present with proposal, design, tasks, and an updated spec — matches the implementation closely, including documenting the "no Drift/offline persistence" tradeoff explicitly.
• resolveLearnLensSession nicely centralizes org/session/expiry resolution that was previously duplicated between ai_tab.dart and video_mcq_tab.dart — good de-duplication (see packages/courses/lib/providers/learnlens_provider.dart:96-141).
• The 401 retry interceptor in LearnLensNetworkClient (packages/courses/lib/network/learnlens_network_client.dart:24-59) is well isolated and has solid dedicated test coverage (learnlens_network_client_test.dart:229-360), including the "second 401 still fails" and "refresh returns null" cases.
• Race-guard checks (widget.lesson.id != requestedLessonId / currentLessonId) correctly prevent stale async responses from clobbering state after fast lesson switching in both ai_tab.dart and history hydration.
• .g.dart regeneration is consistent with the source annotation change (@riverpod → @Riverpod(keepAlive: true)), so no hand-edit concern there.
• Good design-system hygiene: no Material/Cupertino widgets, tokens read via Design.of(context), icons via LucideIcons, all new strings reuse existing L10n keys, motion duration now correctly goes through MotionPreferences.duration(...) (a fix over the previous hardcoded design.motion.normal).
• Solid widget test coverage for hydration, retry, error states, and lesson-switch behavior (ai_tab_test.dart).

🔴 CRITICAL — Must fix before merge

None found.

🟡 WARNING — Should fix

1. LearnlensSession changed from @riverpod (autoDispose) to @Riverpod(keepAlive: true) — packages/courses/lib/providers/learnlens_provider.dart:149. This is a family provider keyed by contentId (one instance per lesson visited). Per the repo's provider guidance, keepAlive: true should be reserved for truly app-lifetime state (e.g. Auth, AppDatabase), not per-lesson session tokens. As currently written, every distinct lesson a learner opens during a session will keep its LearnlensSession instance alive indefinitely (no disposal), which can accumulate over a long session (many lessons watched). If the goal is just to avoid session loss during quick tab switching between AITab/VideoMcqTab, consider @riverpod with ref.keepAlive() scoped/cancelled appropriately, or a documented rationale in design.md for why full keepAlive is needed here.
2. Silent catch swallows the underlying error — packages/courses/lib/network/learnlens_network_client.dart:52-54: catch (retryError) { return handler.next(err); } discards retryError entirely (not logged to Sentry or otherwise), which could hide real failures in the token-refresh path (e.g., a bug in onRefreshToken itself) behind the original 401. Consider at least capturing retryError via the Sentry service for observability, consistent with how other catch blocks in this PR do (ref.read(sentryServiceProvider).captureException(...)).

🔵 SUGGESTION — Nice to have

1. _ChatMessage.messageType and _ChatMessage.citations are populated from LearnLensMessageDto/LearnLensChatResponseDto (ai_tab.dart:100-105, 227-233) but never rendered — citations in particular seem like a natural next-step UI element (e.g., source links). If this is deliberately deferred to a follow-up PR, consider a short // TODO or note in tasks.md/design.md since the DTOs already carry the data.
2. LearnLensChatResponseDto's constructor computes chatId/conversationId from each other in a slightly hard-to-follow way (packages/core/lib/data/models/learnlens_dto.dart:8-15, using each field's fallback as the other's initializer target). A short comment explaining the chat_id/conversation_id backward-compatibility intent (which the design doc already explains well) would help future readers, since positional dependencies between the two initializer expressions aren't obvious at the call site.

Verdict

APPROVED — the WARNING items are worth addressing (particularly the keepAlive: true scope, which has real memory-growth implications) but are not blocking given the existing precedent for keepAlive usage elsewhere in the codebase and the otherwise thorough test coverage and OpenSpec documentation.