From da83e96e1843459e30a021e3cf4e377a31763336 Mon Sep 17 00:00:00 2001 From: Vlad Jerca Date: Tue, 19 May 2026 18:00:47 +0200 Subject: [PATCH] chore(agents): add AI rule files, Gemini styleguide, Copilot wiring Introduces a `.agents/rules/*.md` set (13 files), root `AGENTS.md` + `CLAUDE.md`, a Gemini styleguide alongside the existing config, and a `.github/copilot-instructions.md` symlink to `AGENTS.md` so GitHub Copilot picks up the same rules. Tailored to this stack: Compose-only, Koin DI, Ktor + OpenAPI-generated client, Coil 3, DataStore, ktlint. Reference projects synthesised: - android/nowinandroid - Google's canonical modular Compose sample. - chrisbanes/tivi - Trakt-domain match (TV/movie tracking, KMP). - DroidKaigi/conference-app-2024 - modern community patterns. - element-hq/element-x-android - enterprise Compose Matrix client. Direction the rules describe (for new code): - MVVM + UDF. ViewModels expose `StateFlow` via `stateIn(viewModelScope, WhileSubscribed(5_000), Initial)`. - Sealed `UiState` (Loading / Loaded / Error), Screen + Content split. - Compose Navigation 2.9+ with `@Serializable` typed routes. - Koin DI with constructor injection. No Hilt, no Dagger. - Repositories: `Flow` reads, `suspend` writes returning `Result`. Local-write-first invariant (offline-first). - OpenAPI client generated; mappers translate DTOs to domain models. - Status 204 is success. - DataStore for prefs; no `SharedPreferences` in new code. - All visuals via `TraktTheme.*` tokens. - Conventional Commits with android-scoped enum. Forward-only posture: rules govern new code and substantially rewritten files. Generated paths (`build/`, `external/`, OpenAPI output, Crowdin-managed translation files) are out-of-scope per `legacy-zones.md`. Includes: - YAML frontmatter (`trigger: glob`, `applyTo: `) on every rule file so tools that support glob-based lazy loading only pull a rule when an edited file matches its scope. AGENTS.md still `@`-includes the full set for tools that need the complete context. - Per-subproject `CLAUDE.md` shells for `app/`, `tv/`, `common/`, `resources/` matching the apple sibling's layout. - `compose-stability.conf` at repo root listing Trakt domain types and stable third-party types for the Compose compiler. - Architecture deep-dive material from the second-pass review of NIA, Tivi, and element-x-android (invariants block, background sync, adaptive layouts, Compose stability, convention plugins, per-module lint baselines, MainDispatcherRule, multi-config previews, screenshot tests via Paparazzi + ComposablePreviewScanner, Konsist, baseline profiles, Coil 3 specifics, typed IDs, alternatives evaluated, logging discipline). Adopts Gemini's review feedback: - Acronyms follow official Kotlin style (Url / Id / Json). - `episodeTypeLabel` returns `@StringRes Int?` rather than taking `Context` - keeps the mapper pure and ViewModel-safe. --- .agents/.impeccable.md | 96 +++++++++ .agents/rules/architecture.md | 332 ++++++++++++++++++++++++++++++ .agents/rules/code-principles.md | 204 ++++++++++++++++++ .agents/rules/commits.md | 140 +++++++++++++ .agents/rules/implementation.md | 169 +++++++++++++++ .agents/rules/legacy-zones.md | 127 ++++++++++++ .agents/rules/localization.md | 154 ++++++++++++++ .agents/rules/networking.md | 206 ++++++++++++++++++ .agents/rules/packages.md | 195 ++++++++++++++++++ .agents/rules/persistence.md | 126 ++++++++++++ .agents/rules/project.md | 173 ++++++++++++++++ .agents/rules/state-management.md | 190 +++++++++++++++++ .agents/rules/testing.md | 290 ++++++++++++++++++++++++++ .agents/rules/theming.md | 177 ++++++++++++++++ .gemini/config.yaml | 19 +- .gemini/styleguide.md | 267 ++++++++++++++++++++++++ .github/copilot-instructions.md | 1 + AGENTS.md | 46 +++++ CLAUDE.md | 1 + app/CLAUDE.md | 25 +++ common/CLAUDE.md | 35 ++++ compose-stability.conf | 44 ++++ resources/CLAUDE.md | 26 +++ tv/CLAUDE.md | 32 +++ 24 files changed, 3071 insertions(+), 4 deletions(-) create mode 100644 .agents/.impeccable.md create mode 100644 .agents/rules/architecture.md create mode 100644 .agents/rules/code-principles.md create mode 100644 .agents/rules/commits.md create mode 100644 .agents/rules/implementation.md create mode 100644 .agents/rules/legacy-zones.md create mode 100644 .agents/rules/localization.md create mode 100644 .agents/rules/networking.md create mode 100644 .agents/rules/packages.md create mode 100644 .agents/rules/persistence.md create mode 100644 .agents/rules/project.md create mode 100644 .agents/rules/state-management.md create mode 100644 .agents/rules/testing.md create mode 100644 .agents/rules/theming.md create mode 100644 .gemini/styleguide.md create mode 120000 .github/copilot-instructions.md create mode 100644 AGENTS.md create mode 120000 CLAUDE.md create mode 100644 app/CLAUDE.md create mode 100644 common/CLAUDE.md create mode 100644 compose-stability.conf create mode 100644 resources/CLAUDE.md create mode 100644 tv/CLAUDE.md diff --git a/.agents/.impeccable.md b/.agents/.impeccable.md new file mode 100644 index 000000000..f067e7900 --- /dev/null +++ b/.agents/.impeccable.md @@ -0,0 +1,96 @@ +## Design Context + +### Users + +Trakt users are entertainment enthusiasts who track movies, TV shows, and +their viewing habits across devices. The Android footprint covers: + +- **Phone / tablet (`app`)** — the daily-driver surface. Quick logging + while watching, glancing at calendars, exploring recommendations, + managing watchlists. Adaptive layouts honour window size classes. +- **Android TV (`tv`)** — the lean-back surface. Browsing trending + content, episode details, and 10-foot summaries with remote control + navigation. Fully Compose-based (no Leanback fragments). +- **Widgets, notifications** — at-a-glance progress and "next episode" + prompts through standard Android widget surfaces. + +The job to be done mirrors the web and iOS apps: stay organised, discover +content, and feel on top of one's entertainment life. Android platforms +add the expectation that the experience is **natively Material 3 + adaptive +Compose** — gestures, ripples, motion, and adaptive layouts should feel +correct on the platform. + +### Brand Personality + +**Clean, confident, modern.** Trakt is a polished, well-built tool that +feels premium without being flashy. Purple is the brand anchor. The +Android surfaces honour Material 3 conventions while expressing the Trakt +identity through `TraktTheme` design tokens. + +### Emotional Goals + +- **Delight & discovery** — browsing surfaces new shows, trending + content, and unexpected recommendations. Cards and posters should + spark curiosity. +- **Control & clarity** — progress tracking, watchlists, and stats give a + sense of mastery. A glance at the calendar should answer "what should + I watch tonight?". +- **Speed** — interactions feel instant. Optimistic mutations, cached + reads through Flow, and short transitions matter more than animation + polish. + +### Aesthetic Direction + +- **Visual tone**: clean and confident. Strong hierarchy, generous + whitespace, purposeful colour accents. Content (posters, artwork) is + the visual star — UI chrome stays out of the way. +- **Theme**: full light/dark support. Subtle purple tinting in dark mode. + Seasonal overrides (Halloween → orange, Christmas → red) flow through + Firebase Remote Config + `CustomThemeUseCase`; they apply through + `TraktTheme` tokens, never via raw `Color(0xFF…)` at call sites. +- **Anti-references**: avoid cluttered dashboards, gamified UI, or + anything that feels like a spreadsheet. Avoid generic streaming-app + aesthetics. Avoid playful/childish — Trakt is a confident, modern tool. +- **TV specifically**: 10-foot legibility. Honour Compose `tv-material` + focus styles. Don't fight the D-pad gesture vocabulary. + +### Design Principles + +1. **Content is king** — posters, artwork, and media metadata are the + visual centrepiece. UI chrome recedes and lets content breathe. +2. **Purposeful colour** — purple is the brand anchor. Red / blue / + green / orange serve specific semantic roles (actions, trends, + status). Never use colour purely for decoration. +3. **Consistent rhythm** — use `TraktTheme.spacing` and `TraktTheme.size` + tokens religiously. Adaptive tokens vary with window size class for + correct phone/tablet/TV behaviour. +4. **Respect the user** — Material 3 dynamic colour where appropriate, + `Modifier.semantics` for accessibility, reduced motion / animations + honour system preferences, WCAG AA contrast. +5. **Themed by design** — every colour choice works across light, dark, + and seasonal themes. Use semantic tokens + (`TraktTheme.colors.textPrimary`), never raw `Color(0xFF…)` in + feature code. +6. **Native feel** — use Material 3 components, system fonts, platform + haptics, and Compose's built-in motion. Custom flourish is welcome + but never at the cost of feeling un-Android. + +### Technical Design Constraints + +- **UI framework**: Jetpack Compose for **all** new code. XML layouts + are not present in the codebase and should not be introduced. +- **Components**: design-system primitives in `common/.../ui/` exposed + through `TraktTheme`. Variants surfaced through enum-typed parameters + + dedicated `Modifier` extensions. +- **Tokens**: all visual values via `TraktTheme.{colors,typography, + spacing,size}`. No raw hex / `Color(0xFF…)` / magic-number paddings in + feature code. +- **Responsive**: phone / tablet / foldable adaptive layouts via window + size classes (`androidx.compose.material3.adaptive`). TV uses + focus-driven layout. +- **Typography**: Material 3 typography overridden through + `TraktTheme.typography`; semantic styles (`title`, `body`, `label`, + `tag`) keep parity with iOS and web. +- **Accessibility**: WCAG AA contrast. Animations respect + `LocalAccessibilityManager` reduced-motion preferences. Dynamic + font scale honoured wherever practical. diff --git a/.agents/rules/architecture.md b/.agents/rules/architecture.md new file mode 100644 index 000000000..5f8b944dd --- /dev/null +++ b/.agents/rules/architecture.md @@ -0,0 +1,332 @@ +--- +trigger: glob +globs: '**/*.kt' +description: 'MVVM + UDF, Compose-only UI, ViewModel + StateFlow, sealed UiState, Compose Navigation typed routes, repositories. Background sync, adaptive layouts.' +applyTo: '**/*.kt' +--- + +# Architecture + +> **TL;DR**: MVVM + Unidirectional Data Flow. Compose-only UI. ViewModels +> expose `StateFlow`. Repositories expose `Flow<…>` for reads +> and `suspend` for writes. DI via Koin. Navigation via Compose +> Navigation 2.9+ with `@Serializable` typed routes. Inspired by Now in +> Android (Google) and Tivi (Chris Banes); selectively borrows. + +## Invariants + +These hold across every layer; rule files below elaborate. Lifted from +Now in Android's `ArchitectureLearningJourney.md`: + +- **Higher layers react to lower layers.** UI reacts to domain reacts + to data. Never the other way around. +- **Events flow down, data flows up.** User intents propagate down as + function calls / event sinks; state propagates up as `Flow`s and + `StateFlow`s. +- **Local storage is the single source of truth.** Repositories write + remote responses to local storage **before** emitting to callers; + callers always observe the local stream. Remote-only emission paths + break offline-first. +- **Repositories own their domain models.** Mappers translate at the + network boundary; no DTO leaks past a repository, and no domain model + leaks across feature boundaries — features depend on `:common` + models, not on each other. + +## Layers + +``` +┌──────────────────────────────────────────┐ +│ UI │ Compose Screens + ViewModels +│ - @Composable Screens │ StateFlow +│ - ViewModels │ +├──────────────────────────────────────────┤ +│ Domain (optional, thin) │ UseCases that combine Flows +│ - UseCase classes │ +├──────────────────────────────────────────┤ +│ Data │ Repositories, local stores +│ - Repository │ Flow reads, suspend writes +│ - Local (DataStore, entity caches) │ +│ - Remote (Ktor + OpenAPI client) │ +└──────────────────────────────────────────┘ +``` + +- Add a `UseCase` **only when** a ViewModel needs to combine multiple + repositories or apply non-trivial business logic that is reused. Do + not write a `UseCase` that just forwards to a single repository + method — call the repository directly. +- Repositories own the **offline-first** behaviour: read from local + first, refresh from remote in the background, emit updates through + the same Flow. + +## Composables + +Composables are pure functions of their parameters. State flows in +through the parameter list; events flow out through lambdas. + +```kotlin +@Composable +fun MovieSummaryScreen( + id: Long, + viewModel: MovieSummaryViewModel = koinViewModel { parametersOf(id) }, + onBack: () -> Unit, +) { + val state by viewModel.state.collectAsStateWithLifecycle() + + MovieSummaryContent( + state = state, + onRetry = viewModel::reload, + onBack = onBack, + ) +} + +@Composable +private fun MovieSummaryContent( + state: MovieSummaryUiState, + onRetry: () -> Unit, + onBack: () -> Unit, +) { + when (state) { + MovieSummaryUiState.Loading -> LoadingPlaceholder() + is MovieSummaryUiState.Loaded -> MovieSummaryBody(state.movie, onBack) + is MovieSummaryUiState.Error -> ErrorState(error = state.error, onRetry = onRetry) + } +} +``` + +- **Split `Screen` (stateful) from `Content` (stateless).** + `Content` takes the state and event lambdas only — easy to preview + and snapshot-test. +- **No `@Preview` on the stateful screen.** Preview the `Content` + variant. +- **`when` over sealed `UiState` is exhaustive.** Add a case to the + sealed type instead of an `else ->` branch. + +## ViewModels + +ViewModels expose a single `state: StateFlow` constructed via +`stateIn(viewModelScope, WhileSubscribed(5_000), Loading)`. Internal +mutable flows feed the public one. + +```kotlin +class MovieSummaryViewModel( + private val id: Long, + private val repository: MovieRepository, +) : ViewModel() { + + val state: StateFlow = + repository + .movie(id) + .map { MovieSummaryUiState.Loaded(it) as MovieSummaryUiState } + .catch { emit(MovieSummaryUiState.Error(it)) } + .stateIn( + scope = viewModelScope, + started = SharingStarted.WhileSubscribed(5_000), + initialValue = MovieSummaryUiState.Loading, + ) + + fun reload() { /* mutate an internal flow that the upstream observes */ } +} +``` + +Rules: + +- **One `StateFlow` per ViewModel.** Not three separate + `StateFlow`s combined inside the view. +- ViewModels do not call composables, do not hold Android `Context`, + do not depend on `View`/`Composer` types. +- One-shot events (snackbars, navigation effects) flow through a + `SharedFlow` with `replay = 0`, collected from + `LaunchedEffect`. + +## Domain Layer (Optional) + +Use-cases live in `core//domain/` only when they earn their +keep: + +- They combine 2+ repositories. +- They encapsulate non-trivial business logic (rate-limit-aware + refreshes, complex ranking). +- They are shared by multiple ViewModels. + +Avoid one-line `UseCase` wrappers around repository calls. + +## Data Layer + +- **Repository** is the public boundary. Located in `core//data/` + or in `common/...//`. +- **Reads return `Flow`.** Local + remote sources flow through + the repository unified by the `data` layer; the ViewModel sees only + the unified stream. +- **Writes are `suspend fun` returning `Result<…>`** (or a domain- + specific sealed result type). +- **Mappers** are pure functions named `mapTo(...)` or + `Mapper.kt`. Live next to the repository. No I/O. + +## Navigation + +Single-activity, single `NavHost` in `app/`, mirror in `tv/`. Typed +routes via `@Serializable` data classes (Compose Navigation 2.9+): + +```kotlin +@Serializable +data object HomeRoute + +@Serializable +data class MovieSummaryRoute(val id: Long) + +NavHost(navController, startDestination = HomeRoute) { + composable { HomeScreen(...) } + composable { backStackEntry -> + val route: MovieSummaryRoute = backStackEntry.toRoute() + MovieSummaryScreen(id = route.id) + } +} +``` + +- **No string routes.** Typed routes only for new screens. +- **No global navigator singleton.** Pass `navController` callbacks + down as lambdas (`onMovieClick: (Long) -> Unit`) so feature + composables stay framework-agnostic. +- **Feature-local `NavGraphBuilder` extensions** expose a feature's + destinations: + +```kotlin +fun NavGraphBuilder.movieFeature(onBack: () -> Unit) { + composable { … } + composable { … } +} +``` + +## Dependency Injection (Koin) + +- Constructor injection at every layer. +- One Koin module per feature, named `Module.kt`, located in + `core//di/`. +- Wire all feature modules in `TraktApplication.setupKoin()`. + +```kotlin +internal val movieModule = module { + single { MovieRepositoryImpl(client = get(), local = get()) } + factory { GetMovieSummaryUseCase(repository = get()) } + viewModel { (id: Long) -> MovieSummaryViewModel(id = id, repository = get()) } +} +``` + +Scopes: + +- **`single { }`** — repositories, clients, caches, stateful collaborators. +- **`factory { }`** — use-cases, mappers, anything stateless and cheap. +- **`viewModel { }`** — every ViewModel. + +## Background sync + +Periodic refreshes (calendar, up-next, watchlist deltas, scrobble +flush) run as `CoroutineWorker` instances inside a `sync/` package +today, or a dedicated `:sync` module once the pattern grows. + +```kotlin +class CalendarSyncWorker( + appContext: Context, + params: WorkerParameters, + private val calendar: CalendarRepository, + private val watchlist: WatchlistRepository, +) : CoroutineWorker(appContext, params) { + + override suspend fun doWork(): Result = coroutineScope { + val refreshes = listOf( + async { calendar.refresh() }, + async { watchlist.refresh() }, + ) + runCatching { refreshes.awaitAll() } + .fold(onSuccess = { Result.success() }, onFailure = { Result.retry() }) + } +} +``` + +Rules: + +- Workers receive repositories via Koin (`workerOf<>(...)`). +- Refresh tasks fan out via `async` + `awaitAll`. Single failures don't + cancel siblings unless intended. +- Return `Result.retry()` on failure so WorkManager applies its + exponential backoff. Don't write a custom retry loop inside the + worker. +- Use `OutOfQuotaPolicy.RUN_AS_NON_EXPEDITED_WORK_REQUEST` when + expediting user-triggered syncs. +- Provide `ForegroundInfo` only when the user is actively waiting + (manual pull-to-refresh chained through WorkManager). +- Sync entry points (which Workers to enqueue when) live in + `common/.../sync/` or `app/.../sync/`. Composables / ViewModels do + not enqueue work directly — they call a `SyncTrigger` collaborator. + +## Adaptive layouts + +Phone + tablet + Android TV — three form factors share most code. +Lean on Material 3 Adaptive (`androidx.compose.material3.adaptive`) +and `WindowSizeClass`: + +```kotlin +@Composable +fun MovieSummaryContent( + state: MovieSummaryUiState, + windowSizeClass: WindowSizeClass, + /* ... */ +) { + val useTwoPane = windowSizeClass.widthSizeClass >= WindowWidthSizeClass.Medium + if (useTwoPane) TwoPaneLayout(state) else SinglePaneLayout(state) +} +``` + +Rules: + +- **Pass `WindowSizeClass` as a parameter** to feature `Content` + composables that need it. Compute it once at the app root via + `calculateWindowSizeClass(activity)`. +- **Never** branch on `Resources.configuration.smallestScreenWidthDp` + inside a composable or `if (isTablet)` from helpers. +- TV-specific composables live in the `:tv` module; phone variants in + `:app`. They share state types and ViewModels via `:common`. +- Use `TraktTheme.spacing` / `TraktTheme.size` tokens — they vary by + window class internally. + +## Single-Activity, Phone vs TV + +- `MainActivity` (`app/`) hosts the phone NavHost. +- `TvActivity` (`tv/`) hosts the TV NavHost. +- Routing between them at startup: `MainActivity` detects + `isTelevision()` and forwards to `TvSplashActivity`. +- **TV reuses the same ViewModels** where the underlying state is + identical; only the Compose tree differs. + +## Compose-as-Function + +- Composables take **parameters in, render output, expose events as + lambdas.** No object identity, no mutable side state outside + `remember`. +- `Modifier` is always the second parameter and has a default + (`modifier: Modifier = Modifier`). +- Slot APIs use `@Composable` lambda parameters + (`content: @Composable () -> Unit`). + +## Composition Locals + +Existing pattern: `LocalBottomBarVisibility`, `LocalSnackbarState`, +`LocalCheckInVisibility`, `LocalRatePromptVisibility`. Use sparingly +for **ambient app-level UI state** that doesn't fit a feature +ViewModel. + +Guidelines: + +- Define `staticCompositionLocalOf` for values that don't change + during a recomposition. +- Provide via `CompositionLocalProvider` at the app root. +- Don't reach for `CompositionLocal` to avoid prop drilling within + a feature — pass parameters down instead. + +## Concurrency + +- Single coroutine context per ViewModel (`viewModelScope`). +- Use `Dispatchers.Default` for CPU-bound work, `Dispatchers.IO` + only when interop demands it (Ktor handles its own dispatchers). +- **No `runBlocking`** outside tests. +- **No `GlobalScope`.** Anywhere. diff --git a/.agents/rules/code-principles.md b/.agents/rules/code-principles.md new file mode 100644 index 000000000..b2f57523f --- /dev/null +++ b/.agents/rules/code-principles.md @@ -0,0 +1,204 @@ +--- +trigger: glob +globs: '**' +description: 'Functional programming, immutability, early exits, single responsibility, init injection via Koin, typed IDs. Apply to all Kotlin source.' +applyTo: '**' +--- + +# Code Principles + +## Functional Programming + +- **Prefer pure functions.** Same input → same output. +- **Side effects at the edges.** I/O (network, disk, Firebase, analytics) + lives in repositories, use-cases, and ViewModel collectors — never + inside mappers, formatters, or composables. +- Composables are pure functions of their parameters. State they read + must come from a hoisted source (parameter, `CompositionLocal`, + `collectAsStateWithLifecycle`). + +## Immutability + +- **Prefer `val` over `var`.** Reach for `var` only when local mutation + measurably simplifies the code. +- Default to value types: `data class`, `sealed class`, `enum`. Reach + for plain `class` only when behaviour requires it. +- Use `kotlin.collections.immutable` (`ImmutableList`, + `PersistentMap`) for state held by `data class`es and exposed to + Compose. The dep is already in the version catalogue. +- Use `map` / `filter` / `fold` over mutating loops. + +**Avoid:** +```kotlin +var result = mutableListOf() +for (item in items) result += transform(item) +``` + +**Prefer:** +```kotlin +val result = items.map(::transform) +``` + +## Early Exits + +Use guard clauses. Avoid nested `let { … }` and nested null checks. + +**Avoid:** +```kotlin +fun process(data: Data?): Foo? { + if (data != null) { + if (data.isValid) { + if (data.hasPermission) { + return transform(data) + } + } + } + return null +} +``` + +**Prefer:** +```kotlin +fun process(data: Data?): Foo? { + if (data == null) return null + if (!data.isValid) return null + if (!data.hasPermission) return null + + return transform(data) +} +``` + +## Sealed Hierarchies for State + +Model UI state as a sealed hierarchy. Forces exhaustive `when` and +prevents impossible states. + +```kotlin +sealed interface MovieUiState { + data object Loading : MovieUiState + data class Loaded(val movie: Movie) : MovieUiState + data class Error(val throwable: Throwable) : MovieUiState +} +``` + +Compose `when` handlers must be exhaustive — do not add `else ->` +branches that swallow new cases. Recent bug (web + apple) +`keep tag(isLatestAired:provider:) switch exhaustive` shows the cost. + +## Code Smells to Avoid + +- **Nested conditionals** — refactor into guard clauses or separate + functions. +- **God ViewModels** — stores over ~300 lines are smells; split by + concern. +- **Abstraction leaks** — callers shouldn't need to know how a type is + built. +- **Singleton-style lookups inside types** — accept dependencies via + Koin's `module { … }` declarations and constructor injection. Don't + `KoinJavaComponent.getKoin().get()` from inside a class body. +- **`!!` non-null assertions** in production code. Use `requireNotNull` + with a message, `checkNotNull` for internal invariants, or a `?:` + fallback. `!!` is acceptable inside tests and previews. +- **`runBlocking`** in production code. It exists for tests and CLI + tools only. + +## Function Design + +- **Single Responsibility** — one job per function. +- Function names describe what the function does, not how. +- Functions with **3 or more parameters** take a single + `data class` parameter (or named arguments at every call site). + +**Avoid:** +```kotlin +fun fetch(url: String, token: String, retry: Int, timeout: Duration): Response +``` + +**Prefer:** +```kotlin +data class FetchOptions( + val url: String, + val token: String, + val retry: Int, + val timeout: Duration, +) + +fun fetch(options: FetchOptions): Response +``` + +## Dependency Injection + +- **Constructor injection** for ViewModels, use-cases, repositories, + clients. +- Wire dependencies in Koin modules under + `common/.../di/` or feature-local `/di/Module.kt`. +- Scope rule of thumb: + - **`single { }`** — clients, repositories, caches, anything + expensive or that owns state. + - **`factory { }`** — use-cases, mappers, anything stateless and + cheap. + - **`viewModel { }`** — every ViewModel. +- Do not mix Koin with Hilt / Dagger. The codebase has standardised on + Koin. +- Compose retrieves ViewModels via `koinViewModel()`. Composables + receiving collaborators take them as parameters (not via service + locator lookup inside the body). + +**Avoid:** +```kotlin +@Composable +fun ProfileScreen() { + val client = remember { GlobalContext.get().get() } + // … +} +``` + +**Prefer:** +```kotlin +@Composable +fun ProfileScreen( + viewModel: ProfileViewModel = koinViewModel(), +) { + val state by viewModel.state.collectAsStateWithLifecycle() + // … +} +``` + +## Simplicity + +- Simple code is maintainable code. +- Favour readability over cleverness. +- If a solution feels complex, step back and reconsider before adding + more abstraction. + +## Typed IDs + +Wrap primitive identifiers in `@JvmInline value class` at the domain +boundary: + +```kotlin +@JvmInline value class MovieId(val raw: Long) +@JvmInline value class EpisodeId(val raw: Long) +@JvmInline value class Slug(val raw: String) +``` + +- Mappers in `common/.../networking/` are the only place that produces + them from generated DTOs (`MovieDto.ids.trakt.let(::MovieId)`). +- Repository / use-case / ViewModel signatures take the typed form so + a `MovieId` cannot be passed where an `EpisodeId` is expected. +- Compose Navigation typed routes accept the wrapped form too via a + custom `NavType` for `@Serializable` data classes. + +This is a **recommendation, not a hard mandate**. Apply when the cost +of swap-confusion is real (cross-entity APIs like `Credits`, `Reviews`, +`Lists`). Skip for purely internal helpers. + +## Iteration + +- Functional collection operators (`map`, `filter`, `flatMap`, `fold`, + `groupBy`) over imperative loops. +- For flows: `map`, `filter`, `combine`, `flatMapLatest`, + `distinctUntilChanged`. Use `stateIn` to convert a cold flow to a + hot `StateFlow` once at the ViewModel layer. +- `forEach` only when the body is a side effect; otherwise use the + expression form. diff --git a/.agents/rules/commits.md b/.agents/rules/commits.md new file mode 100644 index 000000000..67c99e9f4 --- /dev/null +++ b/.agents/rules/commits.md @@ -0,0 +1,140 @@ +--- +trigger: glob +globs: '**' +description: 'Conventional Commits with android scope enum, PR title rules, no AI-generated trailers, no em-dashes.' +applyTo: '**' +--- + +# Commit Standards + +The codebase already follows Conventional Commits — recent history +shows `feat:`, `fix:`, `refactor:`, `chore:`, `feat(i18n):`. These rules +codify the convention and define the allowed scope enum. + +## Format + +``` +(): + +[optional body] +[optional footer(s)] +``` + +- Subject: lowercase, imperative, no trailing period, ≤ 72 chars. +- Body: optional, wrap at 100 chars, explain **why** not **what**. +- Footers: `Refs: #123`, `Co-Authored-By: …` (only when human asks). + +## Allowed types + +| Type | Use for | +| ---------- | ---------------------------------------------------------- | +| `feat` | A user-visible feature. | +| `fix` | A bug fix. | +| `perf` | Performance improvement without behaviour change. | +| `refactor` | Code change that neither adds a feature nor fixes a bug. | +| `chore` | Tooling, dependency bumps, generated files (e.g. OpenAPI). | +| `test` | Tests only. | +| `docs` | Documentation only. | +| `style` | Whitespace / formatting only, no semantic change. | +| `build` | Build system or external dependency changes. | +| `ci` | CI configuration only. | + +## Allowed scopes + +Drawn from the actual module list + recurring concerns: + +| Scope | Path coverage | +| ------------ | ---------------------------------------------------------- | +| `app` | `app/` | +| `tv` | `tv/` | +| `common` | `common/` | +| `resources` | `resources/` | +| `openapi` | `openapi/`, generated client | +| `widget` | Android home-screen widget | +| `i18n` | Crowdin syncs, `values-*/strings.xml` | +| `ci` | `.github/workflows/`, Fastlane | +| `deps` | `gradle/libs.versions.toml`, dependency bumps | +| `gradle` | `build.gradle.kts`, `buildSrc/`, Gradle plugins | +| `agents` | `.agents/`, `AGENTS.md`, `CLAUDE.md` | +| `gemini` | `.gemini/` | + +Multi-scope changes pick the most-affected scope or use the broader +one: + +``` +feat(app): add notes drawer +fix(tv): TV get-upcoming case fix +refactor(common): derive isLatestAired from remaining episode count +chore(i18n): translations updates from CrowdIn +chore(deps): bump compose BOM to 2026.06.00 +chore(openapi): regenerate from spec for new credits endpoint +ci: switch ktlint job to v1.7.1 +``` + +## Examples drawn from current history + +Good (already in use): + +``` +feat: surface specific episode type in details meta info +fix: hide stale mid-season episode tags on up-next cards +refactor: derive isLatestAired from remaining episode count +chore: update locales +feat(i18n): translations updates from CrowdIn (#149) +``` + +Better (add scope from the enum above for clarity): + +``` +feat(app): surface specific episode type in details meta info +fix(app): hide stale mid-season episode tags on up-next cards +refactor(common): derive isLatestAired from remaining episode count +chore(i18n): update locales +``` + +Bad: + +``` +fix tag staleness # missing type+scope, vague +update some things # missing everything +feat: notes # missing scope and verb +``` + +## Body content + +Use the body to record **why** the change is needed: + +``` +fix(common): handle 204 from credits endpoint + +The credits endpoint returns 204 when a person has no recorded credits +for the queried role. The legacy mapping treated 204 as an error, +surfacing an empty screen with a stale error toast. +``` + +Avoid: + +- Restating the subject in the body. +- Describing the diff line-by-line. +- AI-generated `Generated with Claude Code` footers. +- `Co-Authored-By: Claude ` trailer — only add + when explicitly requested by the human author. +- Em-dashes / en-dashes in commit messages, PR titles, or PR bodies. + Use plain hyphens (`-`). + +## PR titles + +PR titles follow the same Conventional Commits format. They show up on +`main` once squash-merged, so they must be self-contained: + +``` +feat(app): add notes drawer (#234) +chore(deps): bump compose BOM to 2026.06.00 (#235) +``` + +## Pre-merge sanity + +- One Conventional Commits scope per commit when possible. Squash-merge + rolls multiple WIP commits into a clean message at merge time. +- No "fix typo", "address review", "wip" commits on main — squash or + fixup before merging. diff --git a/.agents/rules/implementation.md b/.agents/rules/implementation.md new file mode 100644 index 000000000..0b8b0eef4 --- /dev/null +++ b/.agents/rules/implementation.md @@ -0,0 +1,169 @@ +--- +trigger: glob +globs: '**/*.kt' +description: 'Naming, file layout, type safety, async / coroutines, navigation, error handling, search-first reuse for new Kotlin code.' +applyTo: '**/*.kt' +--- + +# Implementation Guidelines + +## Before Writing Code + +- **Search existing patterns first.** The codebase already has feature + folders, Koin modules, and `TraktTheme` tokens — look for a working + example before inventing. +- Check whether a new helper already exists in `common/.../helpers/`, + `common/.../ui/`, or another feature. +- Prefer referencing real files as examples over abstract descriptions. + +## When Establishing New Patterns + +- When a pattern diverges from or extends existing conventions, note + it so the rules can be updated. +- A helper used in 2+ places earns a home in `common/.../helpers/` + (logic) or `common/.../ui/` (Compose). +- If you refactor shared logic, leave a one-line note in the PR body + pointing at the new home. + +## Naming Conventions + +- **PascalCase** — types, top-level Composables, file names matching the + primary type. +- **camelCase** — properties, functions, local variables, lambda + parameters. +- **SCREAMING_SNAKE_CASE** — top-level / companion constants only + (`const val MAX_RETRIES = 3`). +- **Acronyms** — follow the official Kotlin style guide: treat acronyms + as words. `Url` / `Id` / `Json` for type names; `url`, `id`, `json` + for properties. ktlint's `function-naming` would flag the all-caps + variant. + +### File Naming + +| Kind | Example file | +| ---------------------------------------- | --------------------------------------- | +| Top-level Composable screen | `MovieSummaryScreen.kt` | +| Other Composable view | `MovieSummaryContent.kt` / `MovieCard.kt` | +| State data / sealed class for a screen | `MovieSummaryState.kt` | +| ViewModel | `MovieSummaryViewModel.kt` | +| Koin module | `MovieSummaryModule.kt` (in `di/`) | +| Use-case | `GetMovieSummaryUseCase.kt` | +| Repository | `MovieRepository.kt` | +| Mapper | `MovieMapper.kt` or `mapToMovie.kt` | +| Domain model | `Movie.kt` | +| Test | `MovieSummaryViewModelTest.kt` | + +### Package Naming + +- Lower-case, no underscores: `tv.trakt.trakt.core.search`, + `tv.trakt.trakt.common.networking`. Match the file path. + +### Folder Layout per Feature + +``` +core// +├── Screen.kt +├── State.kt +├── ViewModel.kt +├── components/ # composables specific to this feature +├── data/ # local stores, mappers, DTO→domain +├── domain/ # use cases, domain models (if needed) +└── di/Module.kt +``` + +## Web/iOS ↔ Android Idiom Map + +For contributors moving between the trakt-web (Svelte) / trakt-apple +(SwiftUI) stacks and Android: + +| Web / iOS | Android / Compose | +| -------------------------------- | ------------------------------------------------------- | +| Zod schema / `Codable` | `@Serializable` data class (kotlinx.serialization) | +| `$state(value)` / `@State` | `var x by remember { mutableStateOf(value) }` | +| `$derived(expr)` / `var x: T` | `val x by remember(...) { derivedStateOf { expr } }` | +| `useFoo()` hook / `FooStore` | `FooViewModel` + `koinViewModel()` | +| `goto(url, { replaceState })` | `navController.navigate(route) { popUpTo(...) }` | +| `RenderFor audience="member"` | `TraktGate(audience = Audience.Member) { … }` | +| `time.hours(3)` / `TraktTime` | `kotlin.time.Duration.Companion.hours(3)` | +| `defineQuery(...)` | Repository function returning `Flow<…>` | +| `goto` with typed params | Compose Navigation typed routes via `@Serializable` | + +## Type Safety + +- No `Any` types in new code unless interop demands it. Use generics or + sealed hierarchies. +- Prefer non-nullable types. Reach for `T?` only when the absence is + semantically meaningful. +- Optional chaining: `?.`, `?:`, `let`, `also`, `apply`, `run`. +- No `lateinit var` for properties that have a sensible default value; + use `var x: T = default` or pull state into a constructor parameter. +- `Result` (`kotlin.Result` or a domain-specific sealed type) at + cross-module boundaries when failure is part of the contract. + +## Async / Coroutines + +- `suspend` functions for one-shot async work. +- `Flow` for cold streams; `StateFlow` for hot, observable state with + a current value; `SharedFlow` for one-shot events with replay. +- Launch work inside `viewModelScope` (ViewModels) or `lifecycleScope` + (composition holders) — never `GlobalScope`. +- Side effects in composables use `LaunchedEffect(key)` / + `DisposableEffect(key)` — never start a coroutine from `body { }` + directly. +- Cancellation: respect it. Use `withTimeout`, `ensureActive`. Don't + `try { … } catch (e: Exception) { … }` over the whole body and + swallow `CancellationException`. + +## State Hoisting + +- ViewModel owns the state. Composables read it via + `collectAsStateWithLifecycle()`. +- `remember { mutableStateOf(...) }` is acceptable for **transient UI** + that doesn't survive ViewModel rebuild (sheet visibility tied to a + single screen, scroll positions, focus). Anything that should survive + a config change belongs in the ViewModel. +- `rememberSaveable` for transient state that should survive config + change but not destruction (text field input, expanded card state). + +## Navigation + +- Compose Navigation 2.9+ with typed routes via + `@Serializable` route classes. +- One `NavHost` in `app/` and one in `tv/`. Feature graphs are + exposed via `NavGraphBuilder.Graph(...)` extension functions + declared in the feature module. +- Pass typed arguments through the serializable route, not through + global state. + +```kotlin +@Serializable +data class MovieSummaryRoute(val id: Long) + +navController.navigate(MovieSummaryRoute(id = 42)) + +composable { backStackEntry -> + val args = backStackEntry.toRoute() + MovieSummaryScreen(id = args.id) +} +``` + +## Error Handling + +- Predictable failures bubble as `Result` / typed sealed errors. +- UI surfaces errors through the `UiState.Error(message)` variant with + copy from a localised `error_*` string. +- Unrecoverable invariants: `error(...)`, `check(...)`, `require(...)` + with a clear message. Never `throw RuntimeException("…")` for + user-input validation. + +## Search-First Reuse + +Before authoring: + +- A new mapper — search `mapTo*` and `*Mapper` for an existing one. +- A new colour or spacing token — check `TraktTheme.colors` and + `TraktTheme.spacing` first. +- A new HTTP call — check whether the OpenAPI client already exposes + the endpoint, or whether `KtorClientFactory` already wraps it. +- A new date / number formatter — check + `common/.../helpers/formatting/`. diff --git a/.agents/rules/legacy-zones.md b/.agents/rules/legacy-zones.md new file mode 100644 index 000000000..ae84459d4 --- /dev/null +++ b/.agents/rules/legacy-zones.md @@ -0,0 +1,127 @@ +--- +trigger: glob +globs: '**' +description: 'Out-of-scope subprojects and generated paths, legacy patterns not to extend, architectural alternatives evaluated (Hilt / Circuit / kotlin-inject / Metro / SQLDelight / Room). Advisory across the repo.' +applyTo: '**' +--- + +# Legacy Zones + +The prescriptive rules in this directory describe the **direction** new +code should take. Trakt-android is in good shape relative to typical +mid-life Android codebases (Compose-only, no XML, no Fragments, no +RxJava or LiveData detected), but a few areas remain off-limits or +auto-generated. Edits inside them must still compile and pass tests, +but reviewers should not flag them for failing to match the new-code +rules. + +## Out-of-scope paths + +| Path | Reason | +| ----------------------------------------------- | ------------------------------------------------ | +| `build/` | Gradle build output | +| `.gradle/`, `.kotlin/`, `.idea/` | Tooling-managed | +| `build/generate-resources/` | OpenAPI generator output | +| `external/` | Third-party drop-in source | +| `**/generated/**` | Code-generation outputs | +| `resources/src/main/res/values-*/strings.xml` | Crowdin-managed translation files | +| `**/*.proto` | Schema source for protobuf — review via spec, not code | + +The OpenAPI generator output (`build/generate-resources/...`) is +ignored by ktlint via `.editorconfig` and is **not edited by hand**. +Regenerate via `./gradlew openApiGenerate`. + +## Architectural alternatives evaluated and not adopted + +Reference projects (Now in Android, Tivi, Element X Android) ship +sound architectures that differ from ours. To prevent drive-by +proposals from AI agents that recently read those repos, the +decisions are explicit: + +- **kotlin-inject** (compile-time DI; Tivi and Element X Android) — + not adopted. We're on Koin's runtime DSL. The compile-time-graph + ergonomics are tempting but require KSP regeneration on every DI + change and would force a parallel migration. Revisit if KSP build + times shrink dramatically. +- **Hilt** — not adopted. The codebase standardised on Koin before + this rule existed; do not propose porting in new code. +- **Metro** (KSP DI; Element X Android) — too new, single-vendor. + Same calculus as kotlin-inject but with less track record. +- **Circuit / Molecule** (Compose-as-Presenter; Tivi, Element X + Android) — not adopted. Contradicts our "no `mutableStateOf` in + ViewModel" rule and adds a framework dep. Keep MVVM + UDF. +- **Appyx navigation** (Element X Android) — not adopted. Compose + Navigation 2.9 typed routes meet our needs. +- **SQLDelight / Room** — not adopted. Today the data layer is + DataStore + in-memory + file caches. Revisit if entity volume forces + the issue. If we ever adopt one, Room is the more likely choice for + Android-only and SQLDelight for KMP. + +These are recorded calls, not invitations to reopen. New evidence is +welcome via a written architecture proposal, not a drive-by refactor. + +## Patterns we accept but do not extend + +These are not present in the codebase today (it's Compose-only), but +the rules below codify what would be considered legacy if they appear: + +- **XML layouts** (`res/layout/*.xml`) — do not add. New screens are + `@Composable`. +- **`Fragment` / `AppCompatActivity` extending `Activity`** — do not + add. `MainActivity` and `TvActivity` are the only Activity classes; + navigation is Compose-based. +- **`LiveData`** — do not add. Use `StateFlow` / `Flow` instead. If a + legacy library forces `LiveData`, isolate the conversion at the + boundary (`liveData.asFlow()`). +- **RxJava / RxKotlin** — do not add. Coroutines + Flow only. +- **KAPT** — new annotation-processed code uses KSP. +- **`SharedPreferences`** direct usage — do not add. Use DataStore. +- **`runBlocking` in production code** — do not add. Tests-only. +- **`GlobalScope`** — do not add. Anywhere. +- **`!!` non-null assertions** in production code — do not add. Use + `requireNotNull(value) { "…" }` with a message, or guard with `?:`. +- **Mocking libraries** (Mockito, MockK) — do not add for new tests. + Use hand-written fakes implementing production interfaces (Now in + Android pattern). +- **Data Binding / View Binding** — forbidden in new code. The + codebase is Compose-only; the binding generators do not apply. + +## Files that are large enough to be hazardous + +The Android codebase doesn't currently have a `CommentsViewController`- +sized legacy file. Any future Kotlin file exceeding **~40 KB** is a +smell — it should be split before growing further. New code aims for +≤ ~300 lines per ViewModel and ≤ ~500 lines per composable file. + +## CI-managed paths + +- `.github/workflows/master.yml` runs ktlint on every push to main. + Edits to `.editorconfig` propagate to the next CI run; no need to + change the workflow. +- `.github/workflows/i18n_sync.yml` opens daily Crowdin-sync PRs. +- `.github/workflows/openapi_sync.yml` (if/when present) regenerates + the API client. + +## When reviewing PRs + +- A new file outside the in-scope source dirs? Suggest relocating, but + don't block for style nits. +- A line of new code in a generated file? Flag it as a process issue + ("this should be regenerated"), not a style nit. +- A new file added under `core//` that doesn't follow these + rules? Flag normally — that's the active surface. + +## Migration tracking (informational) + +These items are not actively in-flight, but should they become so, here +is where the codebase is heading: + +- KSP adoption across all modules (some still rely on plugin defaults). +- Per-feature Gradle modules (currently flat). +- Roborazzi screenshot tests for the design system. +- Detekt as a secondary linter (today only ktlint is wired). +- Compose Multiplatform — not on the roadmap; flagged here so newcomers + don't speculate. + +No timelines, no mandates. The rules describe direction, not a +deadline. diff --git a/.agents/rules/localization.md b/.agents/rules/localization.md new file mode 100644 index 000000000..dbbf100bf --- /dev/null +++ b/.agents/rules/localization.md @@ -0,0 +1,154 @@ +--- +trigger: glob +globs: '{resources/src/main/res/**,**/ui/**/*.kt,**/strings.xml}' +description: 'Crowdin source of truth, R.string namespacing (action_/common_/screen_/error_/a11y_), pluralStringResource, exhaustive API enum lookups returning @StringRes.' +applyTo: '{resources/src/main/res/**,**/ui/**/*.kt,**/strings.xml}' +--- + +# Localization + +## Source of truth + +- All user-facing strings live in + `resources/src/main/res/values/strings.xml` (the `en` source). +- Translations land under `resources/src/main/res/values-/` + (`values-de`, `values-fr`, …), populated by Crowdin via + `.github/workflows/i18n_sync.yml`. Never hand-edit translation files. +- `buildSrc/.../ValidateStringPlaceholdersTask` validates placeholder + parity across locales — run it before merging strings PRs. + +## Reading strings in Compose + +Use `stringResource` / `pluralStringResource` from +`androidx.compose.ui.res`: + +```kotlin +Text(stringResource(R.string.screen_movie_summary_title)) + +Text( + pluralStringResource( + id = R.plurals.episodes_left, + count = remaining, + remaining, + ) +) +``` + +- **Never** inline raw string literals for user-facing text in + composables. Debug logs (via Timber) are not localised. +- Reach into `R.string.*` from `resources` package + (`tv.trakt.trakt.resources.R`). + +## Key namespacing + +Borrowed from web/iOS conventions and aligned with what already exists +in the `values/strings.xml`: + +| Prefix | Used for | +| ------------------- | ------------------------------------------------- | +| `action_*` | Verbs on buttons / menu items | +| `common_*` | Shared phrases reused across screens | +| `a11y_*` | Accessibility-only labels | +| `screen__*` | Strings scoped to one screen | +| `error_*` | User-facing error copy | + +Platform-specific suffixes: + +- `_tv` — Android TV-only variant +- `_phone` — phone-only variant when wording differs + +Example keys: + +``` +screen_movie_summary_title +screen_movie_summary_cta_play_tv +action_remove_from_watchlist +error_credits_load_failed +a11y_rating_picker +``` + +## Pluralisation + +Use `` in `strings.xml` and `pluralStringResource` in code. +Never `if (count == 1) "1 episode" else "${count} episodes"` — it +breaks every non-English locale. + +```xml + + %d episode left + %d episodes left + +``` + +## Formatting (dates, numbers, durations) + +- Dates via `kotlinx.datetime` + locale-aware helpers in + `common/.../helpers/formatting/`. +- Numbers via `NumberFormat.getInstance(locale)`. Mirror trakt-web's + `toHumanNumber` (compact notation) when porting. +- Durations via existing duration helpers; don't hand-code + `"$hours h $minutes m"`. + +## API enum → translated UI text + +When an API enum surfaces as translated UI text (genre, episode type, +status), build an exhaustive lookup helper that returns a `@StringRes` +id, not a resolved `String`. This keeps the mapper pure and testable +without a `Context`, and lets ViewModels stay free of Android +framework types (per `architecture.md`). + +```kotlin +@StringRes +fun episodeTypeLabel(raw: String): Int? = when (raw.normalizeKey()) { + "standard" -> R.string.common_episode_type_standard + "mid_season_finale" -> R.string.common_episode_type_mid_season_finale + "season_finale" -> R.string.common_episode_type_season_finale + "series_finale" -> R.string.common_episode_type_series_finale + else -> null +} + +// In a composable: +val labelRes = episodeTypeLabel(episode.type) +Text( + text = labelRes?.let { stringResource(it) } ?: episode.type, +) +``` + +- `normalizeKey()` lower-cases and replaces separators so + `Mid Season Finale` / `mid-season-finale` / `mid_season_finale` all + resolve. +- Make the `when` **exhaustive** for sealed enums you control. The + `else -> null` fallback exists only for open-ended API strings — the + call site decides whether to render the raw key or a placeholder. +- Recent bug across stacks + (`keep tag(isLatestAired:provider:) switch exhaustive`) shows the + cost of swallowing new API values silently. + +## TV-specific strings + +- Add a `_tv` suffix variant when the TV wording differs (shorter + text, no instructions referencing tapping). +- Reuse the phone variant when wording is identical. + +## CrowdIn workflow + +- Source-only edits in `values/strings.xml`. +- Crowdin sync workflow runs daily; opens a `feat(i18n): translations + updates from CrowdIn` PR. +- New keys ship with English only; translations land via the next + Crowdin sync. +- `ValidateStringPlaceholdersTask` blocks merges that introduce + placeholder mismatches. + +## Don'ts + +- Don't hand-edit any `values-/strings.xml` file. Crowdin owns + them. +- Don't introduce keys without a namespace prefix. +- Don't use `String.format` with positional args (`%s %s`) when keys + could clash on translator interpretation — use named placeholders + via `` tags. +- Don't concatenate user-facing strings with `+`. Use formatted + templates. +- Don't ship locale-specific code paths (`if (locale == "de") …`). + All locale logic flows through resources and `Intl.*`-style helpers. diff --git a/.agents/rules/networking.md b/.agents/rules/networking.md new file mode 100644 index 000000000..6c20ab7ec --- /dev/null +++ b/.agents/rules/networking.md @@ -0,0 +1,206 @@ +--- +trigger: glob +globs: 'common/src/main/**/*.kt' +description: 'OpenAPI client, Ktor 3 setup, mappers, ApiError, repository contract with local-write-first, TTL helpers, pagination.' +applyTo: 'common/src/main/**/*.kt' +--- + +# Networking + +## Layers + +| Layer | Lives in | +| ----------------------------- | ---------------------------------------------------------- | +| OpenAPI spec | `openapi/openapi.json` | +| Generated API client (DTOs) | `build/generate-resources/.../main/src/main/kotlin/` | +| Ktor client + interceptors | `common/.../networking/` | +| Hand-written domain mappers | `common/...//` or `common/.../networking/mappers/` | +| Repositories | feature-local `core//data/` or `common/...//` | + +## OpenAPI client (generated) + +- Source of truth: `openapi/openapi.json`. +- Regenerate with `./gradlew openApiGenerate`. +- Generated sources are **not edited by hand**. The root + `build.gradle.kts` runs post-generate tasks to normalise the + output: + - `dedupeSerializable` — removes duplicate `@Serializable` + annotations. + - `publicApiClient` — makes the generated `HttpClient` public. + - `removeGenders` — strips the unused gender parameter from people + endpoints. +- When the OpenAPI spec changes, regenerate and commit the diff — + generated sources are committed so consumers don't need the + generator at build time. +- Generated files are excluded from ktlint via `.editorconfig`. + +## Ktor client setup + +`common/.../networking/KtorClientFactory` builds two `HttpClient` +instances over a single shared `OkHttp` engine: + +- **Authorised** — attaches the bearer token from auth storage, + refreshes on 401, sends Trakt API headers. +- **Public** — no token, used for unauthenticated endpoints. + +Rules: + +- New endpoints reuse the existing `HttpClient` instances injected via + Koin. Don't construct a new `HttpClient` per call. +- Logging via `Logger.SIMPLE` (Timber bridge) at `LogLevel.HEADERS` in + debug builds, `LogLevel.NONE` in release. +- File-based response cache lives under the app's cache dir. +- Use `ContentNegotiation` + `json()` for kotlinx.serialization, or + `moshi()` where existing endpoints rely on it. + +## Domain mappers + +Hand-written, pure, deterministic. One mapper per endpoint or per +entity. + +```kotlin +// common/.../movie/MovieMapper.kt + +internal fun MovieDto.toDomain(): Movie = Movie( + id = ids.trakt, + slug = ids.slug.orEmpty(), + title = title.orEmpty(), + overview = overview, + releasedAt = released?.toLocalDate(), +) +``` + +- Mappers are `internal` and named `…ToDomain` or `mapTo`. +- No I/O, no logging, no `runBlocking`. +- Mappers handle nullability and provide sensible domain defaults + (empty strings, empty lists) — domain types are non-nullable where + possible. +- Status `204` (no content) is **success**, not failure. Mappers and + repositories handle `204` by returning empty/default values. Recent + bug across stacks: credits endpoints returned `204` and were treated + as errors. + +## Domain models + +Live in `common/...//`: + +```kotlin +@Immutable +data class Movie( + val id: Long, + val slug: String, + val title: String, + val overview: String?, + val releasedAt: LocalDate?, +) +``` + +- `@Immutable` annotation on models held in Compose state. +- Use `kotlin.time.Duration`, `kotlinx.datetime.LocalDate / + Instant` for time values. `kotlinx-datetime` is in the version + catalogue. +- Use `ImmutableList` / `PersistentList` for collections held + in state. + +## Repositories + +Repositories are the public boundary between data and the rest of the +app. + +```kotlin +interface MovieRepository { + fun movie(id: Long): Flow + suspend fun refresh(id: Long): Result +} + +internal class MovieRepositoryImpl( + private val remote: MoviesApi, + private val local: MovieCache, +) : MovieRepository { + + override fun movie(id: Long): Flow = local + .observe(id) + .onStart { runCatching { refresh(id) } } + + override suspend fun refresh(id: Long): Result = runCatching { + val dto = remote.getMovie(id = id, extended = "full") + local.put(dto.toDomain()) + } +} +``` + +Rules: + +- **Public reads return `Flow`.** Hot or cold, the caller doesn't + care. +- **Public writes are `suspend fun`** returning `Result` or a + domain-specific sealed result type. +- **Offline-first**: prefer reading from local first, refresh from + remote in the background, emit the local stream. +- **Local storage is the single source of truth.** Remote responses + are persisted to local before the repository emits them. Never + forward an HTTP response straight to the caller — that breaks the + offline-first invariant the moment the network fails on a retry. +- **Implementations are `internal`.** Expose only the interface + through Koin. +- **No `Activity` / `Context` / `Composable` references** in + repositories. + +## Errors + +Define a typed error hierarchy at the boundary: + +```kotlin +sealed interface ApiError { + data class Http(val status: Int) : ApiError + data class Decoding(val cause: Throwable) : ApiError + data class RateLimited(val retryAfter: Duration?) : ApiError + data object Offline : ApiError + data object Canceled : ApiError + data class Unknown(val cause: Throwable) : ApiError +} +``` + +- Catch Ktor exceptions in one place (interceptor or repository) and + map to `ApiError`. +- Repository return types carry the error via `Result<…>` or a + domain sealed type — don't let raw `IOException` / `HttpException` + escape into ViewModels. +- ViewModel surfaces `ApiError` through `UiState.Error(...)` with + localised copy. + +## Authentication + +- Bearer token storage: DataStore (`Preferences`) wrapped by an + `AuthStorage` collaborator. +- Token refresh: handled by the authorised `HttpClient`'s `Auth` plugin + configuration. Repositories don't reach into auth state. +- For OAuth flows (sign-in, device code), keep the flow in a dedicated + `auth/` feature folder; do not couple to feature ViewModels. + +## TTL & caching + +When caching domain entities, define TTLs through a centralised +helper similar to web/iOS: + +```kotlin +object TraktDurations { + val stable = 12.hours // summaries, translations + val normal = 3.hours // lists, ratings + val frequent = 30.minutes // user-affecting lists + val live = 5.minutes // watchers, now-playing +} +``` + +Never cache for `Duration.INFINITE`. + +## Quick checklist + +- [ ] DTO comes from the generated OpenAPI client (or a documented + exception) +- [ ] Mapper is pure, named `…ToDomain` / `mapTo` +- [ ] Repository exposes `Flow<…>` reads and `suspend` writes +- [ ] Errors mapped to a typed `ApiError` before reaching the ViewModel +- [ ] Status `204` handled explicitly where applicable +- [ ] Koin module wires the repository via `single { }` +- [ ] No `Activity` / `Context` references in data classes diff --git a/.agents/rules/packages.md b/.agents/rules/packages.md new file mode 100644 index 000000000..010e77fb8 --- /dev/null +++ b/.agents/rules/packages.md @@ -0,0 +1,195 @@ +--- +trigger: glob +globs: '{**/build.gradle.kts,**/settings.gradle.kts,gradle/libs.versions.toml,buildSrc/**,build-logic/**}' +description: 'Module dependency direction, version catalogue, external deps justification, per-feature module opt-in, convention plugins, lint baseline.' +applyTo: '{**/build.gradle.kts,**/settings.gradle.kts,gradle/libs.versions.toml,buildSrc/**,build-logic/**}' +--- + +# Gradle Modules & Dependencies + +## Module map + +| Module | Plugin | Role | +| ------------ | --------------------------- | --------------------------------------------- | +| `:app` | `com.android.application` | Phone app entry point + most feature screens | +| `:tv` | `com.android.library` | Android TV variant entry + TV screens | +| `:common` | `com.android.library` | Domain models, networking, DI, theme tokens | +| `:resources` | `com.android.library` | Strings, drawables, locale variants | +| `:buildSrc` | `kotlin-dsl` | Gradle plugins (e.g., `ValidateStringPlaceholdersTask`) | + +## Dependency direction (hard rule) + +``` + app + / \ + ▼ ▼ + tv common + \ / | + ▼ ▼ ▼ + common resources + | + ▼ + resources +``` + +- `app` depends on `common`, `tv`, `resources`. +- `tv` depends on `common`, `resources`. +- `common` depends on `resources`. +- `resources` depends on nothing. + +**Never reverse the graph.** `common` is the boundary — it has no +`Activity`, `Application`, or `Composable` consumers; only domain, +networking, DI, and reusable UI tokens. + +## Version catalogue + +Single source of truth at `gradle/libs.versions.toml`. Rules: + +- **Add new deps to the catalogue first.** No literal coordinates in + `build.gradle.kts` (`implementation("com.example:foo:1.0")`). +- Reference via alias: `implementation(libs.compose.material3)`, + `implementation(libs.coil.compose)`. +- Group related deps in bundles (`libs.bundles.compose.testing`). +- Plugin aliases via the catalogue too: + `alias(libs.plugins.kotlin.serialization)`. + +## Adding a new external dependency + +PR description must include: + +1. **Why** — the need it serves that an existing dep or stdlib doesn't + cover. +2. **Maintenance posture** — last release, contributor count, license. +3. **Footprint** — APK size impact (run `:app:reportDependencyGraphChange` + or eyeball the AAR size), transitive deps, R8 friendliness. +4. **Exit plan** — how we'd remove it if it goes unmaintained. + +Defaults to **rejected** until justified. AndroidX, Jetpack, Compose +ecosystem libs and well-established projects (Coil, Ktor, OkHttp, +Coroutines, Turbine) clear the bar trivially. + +## Compose / Kotlin / Compiler alignment + +- Compose BOM is the single version source for Compose deps. Don't + pin individual `androidx.compose.*` versions. +- Kotlin and Compose Compiler must stay in sync via the + `org.jetbrains.kotlin.plugin.compose` plugin (already in the + catalogue). Don't override. +- `kotlinx-serialization`, `kotlinx-datetime`, `kotlinx-coroutines` — + pin via the catalogue and use coroutine BOMs where they exist. + +## Per-feature modules (future) + +The codebase is intentionally flat today (5 modules). Per-feature +Gradle modules (e.g., `:feature:home`, `:feature:notes`) **may** be +introduced once a feature is genuinely self-contained, mirroring +patterns from Now in Android (`:feature:foo`) or Tivi +(`:ui:foo` + `:data:foo`). + +Skeleton (if/when we go there): + +``` +:feature:notes +├── build.gradle.kts # com.android.library + kotlin.compose +└── src/main/java/tv/trakt/trakt/feature/notes/ + ├── NotesScreen.kt + ├── NotesViewModel.kt + ├── NotesState.kt + ├── data/ # repository, mappers, cache + └── di/NotesModule.kt +``` + +Rules if/when adopted: + +- Feature modules depend on `:common` and `:resources` only. +- **Feature → feature is forbidden.** Cross-feature deps go through + `:common` or a shared `:core:` module. +- `internal` by default. Expose the umbrella screen composable + Koin + module + navigation graph extension as `public`. + +Not mandatory yet. The first feature to graduate establishes the +template. + +## Convention plugins + +`buildSrc/` already hosts `ValidateStringPlaceholdersTask`. As more +build logic accumulates, migrate to a Gradle composite build under +`build-logic/convention/` and ship the following plugin aliases: + +- `trakt.android.application` — applies `com.android.application` + + the shared `compileSdk`, `defaultConfig`, `compileOptions`, + `signingConfigs` boilerplate. +- `trakt.android.library` — `com.android.library` equivalent. +- `trakt.android.library.compose` — extends `trakt.android.library` + with `org.jetbrains.kotlin.plugin.compose`, `composeOptions`, and + the stability-config wiring. +- `trakt.android.feature` — extends `trakt.android.library.compose` + with the typical feature module deps (`:common`, `:resources`, Koin, + Navigation Compose). +- `trakt.android.lint` — Android Lint config (`xmlReport`, + `sarifReport`, `checkDependencies = true`, baseline file path). +- `trakt.jvm.library` — pure JVM module (`org.jetbrains.kotlin.jvm`). + +After migration, each module's `build.gradle.kts` reads as ~10 lines +applying convention plugins and a `dependencies { … }` block — no +copy-pasted SDK / Compose / Kotlin config blocks. See +[android/nowinandroid `build-logic/convention/`](https://github.com/android/nowinandroid/tree/main/build-logic/convention) +for the canonical layout. + +Element X Android's [`plugins/`](https://github.com/element-hq/element-x-android/tree/develop/plugins) +shows an alternative: small plugins delegating to a `extension/` +folder of helpers (`androidConfig`, `composeConfig`, `setupKover`). +Either is fine; pick one and stay consistent. + +## Lint + +Per-module setup: + +- Every Android module owns a `lint-baseline.xml` (committed). The + baseline lets existing warnings not block new work. **The baseline + never grows from a PR** — new violations on changed lines must be + fixed, not baselined. +- A `lint.xml` overlay exists only when a module needs rules that + differ from the project defaults. +- The `trakt.android.lint` convention plugin sets: + - `xmlReport = true` + - `sarifReport = true` + - `checkDependencies = true` +- CI uploads the SARIF output to GitHub Code Scanning so violations + appear inline on PR diffs. + +## Formatting pipeline + +Today: `ktlint` invoked via `./gradlew ktlintFormat` / +`ktlintCheck`, wired into CI through `.github/workflows/master.yml`. + +Aspirational: move to **Spotless** so formatting, ktlint, license +headers, and import ordering run in one pass — both NIA and Tivi do +this. Track in `legacy-zones.md`'s migration list. Until migrated, +honour `.editorconfig` rules and run ktlint before pushing. + +## OpenAPI generator + +Lives at the root `build.gradle.kts`. Three rules: + +- **Spec is the source of truth** — edit `openapi/openapi.json`, not the + generated Kotlin. +- **Re-run after editing the spec**: `./gradlew openApiGenerate`. +- **Commit the generated diff.** Don't expect downstream contributors + to regenerate locally. + +## Plugin / version pins of note + +- `kotlin.code.style=official` set in `gradle.properties`. +- `android.useAndroidX=true`, `android.nonTransitiveRClass=true`. +- `org.gradle.jvmargs=-Xmx2048m -Dfile.encoding=UTF-8`. +- KSP over KAPT — new annotation-processed code should use KSP. + +## Quick checklist + +- [ ] New dep added to `gradle/libs.versions.toml` first +- [ ] Module dependency direction respected +- [ ] No literal coordinates in `build.gradle.kts` +- [ ] PR body justifies new external deps +- [ ] OpenAPI spec edit re-runs the generator and commits the diff +- [ ] No `Activity` / `Context` references in `:common` diff --git a/.agents/rules/persistence.md b/.agents/rules/persistence.md new file mode 100644 index 000000000..7da0af05c --- /dev/null +++ b/.agents/rules/persistence.md @@ -0,0 +1,126 @@ +--- +trigger: glob +globs: 'common/src/main/**/*.kt' +description: 'DataStore (Preferences and Proto), entity caches, AuthStorage, no SharedPreferences. Migrations and disk-I/O hygiene.' +applyTo: 'common/src/main/**/*.kt' +--- + +# Persistence + +## What lives where + +| Need | Tool | +| ------------------------------------------ | ----------------------------------------------- | +| User preferences (theme, flags, token) | DataStore — `Preferences` or typed Proto | +| Entity caches (recent movies, upcoming) | Existing entity stores in `common/...//`| +| Ephemeral in-memory cache | `MutableStateFlow<…>` inside a `single { }` Koin scope | +| Auth tokens / sensitive material | DataStore + Android Keystore-backed encryption | +| Files (downloaded artwork, OkHttp cache) | App cache directory (already wired) | + +**Forbidden in new code:** + +- `SharedPreferences` direct usage (use DataStore). +- Realm. +- SQLDelight (the codebase has not adopted it; do not introduce + without an architecture decision). +- Hand-rolled SQLite via `SupportSQLiteOpenHelper`. + +## DataStore — Preferences + +```kotlin +private val Context.userPrefs: DataStore by preferencesDataStore("user_prefs") + +class UserPreferencesStore(private val dataStore: DataStore) { + val seasonalThemeEnabled: Flow = dataStore.data + .map { it[KEY_SEASONAL_THEME] ?: true } + + suspend fun setSeasonalThemeEnabled(enabled: Boolean) { + dataStore.edit { it[KEY_SEASONAL_THEME] = enabled } + } + + private companion object { + val KEY_SEASONAL_THEME = booleanPreferencesKey("seasonal_theme") + } +} +``` + +Rules: + +- One `DataStore` instance per logical scope (user prefs, + feature-flag overrides, etc.). Don't share a single global store + across feature concerns. +- Keys live in a `companion object` next to the store. Use the typed + helpers (`stringPreferencesKey`, `booleanPreferencesKey`, …). +- Reads return `Flow`. Writes are `suspend`. +- Wire stores as `single { }` in Koin. + +## Typed (Proto) DataStore + +For more complex state (multi-field structures, schema evolution), +use Proto DataStore + kotlinx.serialization Proto: + +```kotlin +@Serializable +data class CalendarPrefs( + val showHidden: Boolean = false, + val daysAhead: Int = 14, +) + +class CalendarPrefsSerializer : Serializer { + override val defaultValue = CalendarPrefs() + // … +} +``` + +Rules: + +- `@Serializable` data classes with default values for every field. +- Migrations handled through `DataStoreFactory` with a `produceMigrations` + block when schema changes. +- One file per typed store under `common/.../persistence/`. + +## In-memory caches + +For state that doesn't need to outlive the process but should be shared +across ViewModels (current login session, scrobble queue): + +- Define a class with a private `MutableStateFlow`, expose + `StateFlow`. +- Wire as `single { }` in Koin so all consumers see the same flow. +- Document the lifecycle: in-memory only, cleared on logout, etc. + +## Auth tokens + +- Tokens stored in a dedicated DataStore (`auth_prefs`). +- The `AuthStorage` collaborator is the **only** type that reads or + writes auth tokens. +- Repositories and ViewModels go through `AuthStorage`; they do not + reach DataStore directly. +- Token refresh handled in the Ktor authorised client's `Auth` plugin + (see `networking.md`). + +## Migrations + +- For Preferences DataStore: use `SharedPreferencesMigration` once + during the legacy-shedding pass; document in a one-line comment when + retiring an old key. +- For Proto DataStore: bump the schema version, write a migration + closure that maps old → new defaults. + +## Disk I/O hygiene + +- All DataStore reads/writes are `suspend` — they already dispatch off + the main thread. +- File caches live under `context.cacheDir` (OkHttp / Ktor already use + this); never under `filesDir` unless data must survive cache eviction. +- Do not block on `runBlocking { dataStore.data.first() }` in + production code. Read through Flow. + +## Quick checklist + +- [ ] DataStore (not SharedPreferences) for new preferences +- [ ] Keys typed via `*PreferencesKey` helpers +- [ ] Reads return `Flow`, writes are `suspend` +- [ ] Store wired as `single { }` in Koin +- [ ] Auth tokens go through `AuthStorage`, never DataStore directly +- [ ] No `runBlocking { … .first() }` in production code paths diff --git a/.agents/rules/project.md b/.agents/rules/project.md new file mode 100644 index 000000000..6160ef5be --- /dev/null +++ b/.agents/rules/project.md @@ -0,0 +1,173 @@ +--- +trigger: glob +globs: '**' +description: 'Tech stack, module layout, dependency graph, tooling. Apply to all files.' +applyTo: '**' +--- + +# Project Guidelines + +## Tech Stack + +Trakt's Android codebase. Phone + Android TV, Compose-first, Kotlin 2.x, +Gradle KTS with version catalogue, OpenAPI-generated client. + +- **Language**: Kotlin 2.3.x. `kotlin.code.style=official` set in + `gradle.properties`. +- **JVM target**: 11 (`jvmToolchain(11)`). +- **UI**: Jetpack Compose for **everything**. Material 3 + adaptive + window size classes. No XML layouts, no Fragments, no + `AppCompatActivity` views. +- **Compose BOM**: `2026.05.00` (Material3 `1.5.0-alpha19`). +- **Min SDK 28, Compile/Target SDK 37, AGP 9.2.x.** +- **Async**: Coroutines + Flow. No RxJava, no `LiveData` in new code. +- **DI**: Koin 4.2.x (module DSL). No Hilt, no Dagger — the codebase has + intentionally standardised on Koin. +- **Networking**: Ktor 3.4.x client with OkHttp engine. API surface + generated from `openapi/openapi.json` via the `openapi-generator` + Gradle plugin. Generated sources land in `build/generate-resources/`. +- **Persistence**: DataStore (Preferences) + entity caches. No + SharedPreferences in new code, no Realm, no SQLDelight. +- **Images**: Coil 3 (`io.coil-kt.coil3`) with the Ktor 3 engine adapter + and SVG decoder. +- **Serialization**: kotlinx.serialization for protobuf and typed + navigation routes; Moshi for OpenAPI-generated DTOs. +- **Firebase**: Crashlytics, Analytics, Remote Config (used to flip + seasonal themes and other feature flags). +- **Media**: Media3 (ExoPlayer) for video; `youtube-player` (thirdspark) + for embedded trailers. +- **Logging**: Timber. + +## Project Structure + +``` +trakt-android/ +├── app/ # com.android.application — phone entry point +│ └── src/main/java/tv/trakt/trakt/ +│ ├── core// # feature folders: home, calendar, search, … +│ ├── ui/ # theme, components, snackbar, extensions +│ ├── helpers/ +│ └── analytics/ +├── tv/ # com.android.library — Android TV variant +│ └── src/main/java/tv/trakt/trakt/app/ +│ ├── core// # TV-specific feature flows +│ └── ui/theme/ +├── common/ # com.android.library — domain, networking, DI +│ └── src/main/java/tv/trakt/trakt/common/ +│ ├── networking/ # Ktor client factory, interceptors +│ ├── firebase/ +│ ├── helpers/ +│ ├── di/ # Koin modules wired in TraktApplication +│ └── ui/ # cross-platform Compose primitives +├── resources/ # com.android.library — strings, drawables, i18n +├── buildSrc/ # ValidateStringPlaceholdersTask + future plugins +├── external/ # third-party drop-in source +├── openapi/ # openapi.json spec + generator config +├── gradle/libs.versions.toml +└── fastlane/ +``` + +## Module Dependency Direction + +``` + app + / \ + ▼ ▼ + tv common + \ / | + ▼ ▼ ▼ + common resources + | + ▼ + resources +``` + +- **`app` depends on**: `common`, `tv`, `resources`. +- **`tv` depends on**: `common`, `resources`. +- **`common` depends on**: `resources`. +- **`resources` depends on**: nothing. + +Hard rule: **never reverse the graph.** `common` cannot import from `app` +or `tv`. `resources` cannot import from anything. + +## Architecture Patterns + +### Compose-first +Every screen is a `@Composable` function suffixed `Screen`. State is +hoisted out of the view; a `ViewModel` owns it and exposes a `StateFlow`. + +### MVVM + UDF +ViewModels expose `StateFlow`. Composables call +`viewModel.state.collectAsStateWithLifecycle()` and pass user events +back as lambdas. Side effects in `LaunchedEffect`. + +### Feature folders +Group by feature (`core/home`, `core/calendar`, `core/search`, +`core/profile`, `core/billing`). Each feature carries its `Screen.kt`, +`State.kt`, `ViewModel.kt`, and a `di/Module.kt` Koin module. + +### Custom theme tokens +All visual tokens live in `common/.../ui/theme/`: +`TraktTheme.colors`, `TraktTheme.typography`, `TraktTheme.spacing`, +`TraktTheme.size`. Exposed through `CompositionLocal`. No raw +`Color(0xFF…)` and no magic-number paddings in feature code. + +### OpenAPI-driven data layer +API DTOs are generated from `openapi/openapi.json`. They are +**not** edited by hand. Hand-written mappers in +`common/.../networking/` translate generated DTOs into Trakt domain +models defined in `common/.../model/`. + +## Commit Standards + +Conventional Commits with android-scoped enum (already in use across +recent history). See `commits.md` for the full allowed scope list. + +``` +feat(app): add notes drawer to summary +fix(tv): TV get-upcoming case fix +refactor(common): derive isLatestAired from remaining episode count +chore(i18n): translations updates from CrowdIn +``` + +## Styling & Theming + +- All visual values via `TraktTheme.*` tokens. +- Material 3 base with `TraktTheme` overlays for colours and typography. +- Light/dark + seasonal theme overrides (Halloween → orange, + Christmas → red) flow through Firebase Remote Config + + `CustomThemeUseCase` — already wired in `MainActivity`. +- Image references via Coil 3 + Trakt placeholders; do not introduce + Glide / Fresco. + +## Logging + +- **Timber for all logging.** Never `android.util.Log.*` directly. +- **Lazy formatting**: `Timber.d("user=%s", user.id)` — Timber elides + formatting when the log level is disabled. Never + `Timber.d("user=" + user.id)` or string-interpolation + (`Timber.d("user=${user.id}")`) for messages built every call. +- **Tag implicitly** via the calling class. Avoid `Timber.tag("X")` + except inside helper utilities that lose class context. +- **Never log secrets, tokens, OAuth codes, refresh tokens, or + request/response bodies** that may include any of the above. Trakt + IDs (movie IDs, show IDs, slugs, list IDs) are safe to log. User + emails / display names are **not** — they're PII. +- `Timber.plant()` only in debug builds; release builds plant a + Crashlytics tree that forwards `WARN`+ severity to Firebase + Crashlytics. Already wired in `TraktApplication.setupTimber()`. + +## Tooling + +- **Build**: `./gradlew :app:assembleDebug`, `./gradlew :tv:assembleDebug`. +- **Format**: `./gradlew ktlintFormat` (where wired; honour + `.editorconfig` ktlint rules). +- **Lint**: `./gradlew ktlintCheck` (gated by `.github/workflows/master.yml` + ktlint job). +- **OpenAPI regeneration**: `./gradlew openApiGenerate` rebuilds the + client from `openapi/openapi.json` — committed generated sources stay + in step. +- **i18n sync**: Crowdin → `resources/src/main/res/values-*/strings.xml` + via `.github/workflows/i18n_sync.yml`. +- **Releases**: Fastlane (`fastlane/`) — 7 lanes covering Firebase + distribution + Play Store internal/beta/production tracks. diff --git a/.agents/rules/state-management.md b/.agents/rules/state-management.md new file mode 100644 index 000000000..93f368d82 --- /dev/null +++ b/.agents/rules/state-management.md @@ -0,0 +1,190 @@ +--- +trigger: glob +globs: '{app,tv,common}/src/main/**/*.kt' +description: 'StateFlow / @Observable usage, ViewModel patterns, remember / rememberSaveable, Compose stability, no-mutableStateOf-in-VM.' +applyTo: '{app,tv,common}/src/main/**/*.kt' +--- + +# State Management + +## What lives where + +| Lifetime / scope | Tool | +| ---------------------------------------------------- | ------------------------------------------------------ | +| Survives config change, drives screen UI | `StateFlow` on a `ViewModel` | +| Composable-local UI state, dies on dispose | `var x by remember { mutableStateOf(value) }` | +| Composable-local UI state, survives config change | `var x by rememberSaveable { mutableStateOf(value) }` | +| App-wide ambient UI state | `staticCompositionLocalOf` + `CompositionLocalProvider`| +| Derived value reactive to other state | `val x by remember(...) { derivedStateOf { … } }` | +| Side effects keyed to recomposition | `LaunchedEffect(key)`, `DisposableEffect(key)` | + +## Sealed UiState + +Every screen has a sealed UI state hierarchy: + +```kotlin +sealed interface MovieSummaryUiState { + data object Loading : MovieSummaryUiState + data class Loaded(val movie: Movie) : MovieSummaryUiState + data class Error(val throwable: Throwable) : MovieSummaryUiState +} +``` + +- **No `data class … (val movie: Movie?, val isLoading: Boolean, val error: Throwable?)`** + flat shapes. Use sealed types so impossible combinations are + impossible. +- `@Immutable` annotation on `Loaded`-style data classes that hold + collections, so Compose can skip recomposition when references are + stable. +- For collections held in state, prefer `ImmutableList` + (`kotlinx.collections.immutable`) over `List` — Compose treats + immutable types as stable. + +## ViewModel + +One `state: StateFlow` per ViewModel. Build it from upstream +Flows and expose via `stateIn(viewModelScope, WhileSubscribed(5_000), +initial)`. Internal mutable signals are private: + +```kotlin +class CalendarViewModel( + private val repo: CalendarRepository, +) : ViewModel() { + + private val refreshTrigger = MutableSharedFlow(replay = 1).apply { tryEmit(Unit) } + + val state: StateFlow = + refreshTrigger + .flatMapLatest { repo.upcoming() } + .map { CalendarUiState.Loaded(it) as CalendarUiState } + .catch { emit(CalendarUiState.Error(it)) } + .stateIn( + viewModelScope, + SharingStarted.WhileSubscribed(5_000), + CalendarUiState.Loading, + ) + + fun refresh() { refreshTrigger.tryEmit(Unit) } +} +``` + +Rules: + +- **Don't put `mutableStateOf` in a ViewModel.** Use `StateFlow` / + `MutableStateFlow`. `mutableStateOf` is Compose-coupled and breaks + the ability to unit-test the ViewModel without Compose runtime. +- **Don't combine three separate `StateFlow`s in the composable.** + Combine them once in the ViewModel and expose a single state. +- **Don't expose `MutableStateFlow` publicly.** Expose `StateFlow`; + mutate from inside the ViewModel. +- Use `update { … }` on `MutableStateFlow` for atomic updates rather + than `value = value.copy(…)`. + +## One-shot events + +For navigation effects, snackbars, toasts: + +```kotlin +private val _events = MutableSharedFlow(replay = 0) +val events: SharedFlow = _events.asSharedFlow() + +@Composable +fun MovieSummaryScreen(viewModel: MovieSummaryViewModel = koinViewModel()) { + val snackbar = LocalSnackbarState.current + LaunchedEffect(Unit) { + viewModel.events.collect { event -> + when (event) { + is MovieEvent.WatchlistAdded -> snackbar.show(L10n.movieAddedToWatchlist) + is MovieEvent.Error -> snackbar.show(event.message) + } + } + } + // … +} +``` + +- Use `SharedFlow(replay = 0)` for one-shot events. +- Collect events in a `LaunchedEffect`. +- Don't fire events from inside composition (`body { }` directly). + +## Composable-local state + +`remember { mutableStateOf(...) }` is for state that: + +- Is owned by a single composable instance. +- Doesn't need to survive config change. +- Doesn't need to be tested without Compose runtime. + +Examples that qualify: sheet visibility, scroll position, dropdown +expanded state, text field input that is committed elsewhere on +"Done", focus management. + +Use `rememberSaveable` when the state should survive a config change +but not the screen leaving the back stack. + +## CompositionLocal + +App-wide ambient UI state already wired: + +- `LocalBottomBarVisibility` — driven by feature flags. +- `LocalSnackbarState` — global snackbar host accessor. +- `LocalCheckInVisibility`, `LocalRatePromptVisibility` — overlay + visibility. + +Add new locals **only** when: + +- The value is genuinely app-level (not a single feature). +- Threading it through every composable's parameter list is impractical. +- The value is provided once at the app root. + +Define using `staticCompositionLocalOf` when the value is constant for +the lifetime of the provider; `compositionLocalOf` when it changes +during recomposition. + +## Side effects + +- `LaunchedEffect(key) { … }` — coroutine tied to composition lifetime. +- `DisposableEffect(key) { … onDispose { … } }` — cleanup on dispose. +- `SideEffect { … }` — synchronous side effect after every successful + composition. Rare; usually you want `LaunchedEffect`. +- **Never** start a coroutine from inside `body { }` directly. +- **Never** read `mutableStateOf` from outside composition without + `snapshotFlow`. + +## Compose stability + +All Trakt domain types in `common/.../model/` are listed in +`compose-stability.conf` at the repo root so the Compose compiler +treats them as stable. The same file declares third-party types that +are stable in practice but not annotated (`kotlinx.datetime.*`, +`kotlin.time.Duration`, `coil3.compose.AsyncImagePainter.State`, +`androidx.paging.compose.LazyPagingItems`). Compose convention plugins +wire the file via `freeCompilerArgs`: + +``` +-Xstability-configuration-path=$projectDir/compose-stability.conf +``` + +Rules: + +- **`@Immutable`** for data classes used as state where every public + property is `val` and points at deeply immutable values. +- **`@Stable`** for types whose properties are observable but whose + equality and hash are stable — rare; reach for it only when + `@Immutable` would be a lie. +- **`ImmutableList` / `PersistentList`** from + `kotlinx.collections.immutable` for collections held in state. Plain + `List` is unstable to Compose. +- **Triage recomposition regressions with compose-compiler metrics.** + Run `./gradlew :app:assembleDebug -Pcompose.metrics=true + -Pcompose.reports=true` and commit the report under + `docs/compose-metrics//` when investigating a hot + screen. + +## Persistence beyond a ViewModel + +For state that must outlive the screen (auth tokens, preferences, +sync watermarks): write to DataStore. The ViewModel reads it through a +repository that exposes a `Flow`. + +See `persistence.md` for the data layer rules. diff --git a/.agents/rules/testing.md b/.agents/rules/testing.md new file mode 100644 index 000000000..b1ca47bc8 --- /dev/null +++ b/.agents/rules/testing.md @@ -0,0 +1,290 @@ +--- +trigger: glob +globs: '**/src/{test,androidTest}/**/*.kt' +description: 'JUnit + Turbine + Truth + runTest. No mocking libraries. MainDispatcherRule, multi-config previews, screenshot tests, baseline profiles, Konsist.' +applyTo: '**/src/{test,androidTest}/**/*.kt' +--- + +# Testing + +## Frameworks + +- **Unit tests**: JUnit 4 or JUnit 5 + [Turbine](https://github.com/cashapp/turbine) + for Flow assertions + Truth (or kotlin.test) for general assertions. +- **Compose UI tests**: `androidx.compose.ui.test.junit4` + + `ComposeTestRule` + `ComponentActivity`. +- **Robolectric** for JVM-only UI tests where useful (no emulator + required). +- **Roborazzi** is the recommended future direction for snapshot tests + — once adopted, baselines generated by CI and committed via Git LFS + for `.png` files. +- **No mocking libraries** (Mockito, MockK) for new code. Use + hand-written fakes implementing the production interface. This is + borrowed from Now in Android and works well with Koin. + +## Where tests live + +| Code under test | Tests under | +| ---------------------------------------------- | ---------------------------------------------- | +| `common/` sources | `common/src/test/` | +| `app/` non-Android-framework sources | `app/src/test/` | +| `app/` Android-framework sources | `app/src/androidTest/` | +| `tv/` sources | `tv/src/test/` / `tv/src/androidTest/` | + +Co-locate test files mirror the package of the source they cover. + +## What to test + +Mandatory unit coverage: + +- **Mappers** — pure DTO → domain translation. Highest payoff. +- **ViewModels** — `state: StateFlow` behaviour for loading, + loaded, error, refresh, optimistic mutation. Use Turbine. +- **Repositories** — local + remote interaction with fake `Api` and + fake `Cache` collaborators. +- **Pure helpers / formatters** — date, duration, number formatting. +- **Use-cases** — when they combine multiple repositories. + +Optional / visual coverage: + +- **Composables** — screenshot tests via Roborazzi once adopted. + Verified manually through `@Preview` blocks until then. +- **End-to-end UI flows** — light coverage on the happy paths (sign-in, + open a movie, mark watched). Don't try to cover every screen at + this level. + +## MainDispatcherRule + +ViewModel tests install a `TestDispatcher` as the main dispatcher so +`viewModelScope` work runs under test control: + +```kotlin +class MainDispatcherRule( + private val testDispatcher: TestDispatcher = UnconfinedTestDispatcher(), +) : TestWatcher() { + override fun starting(description: Description) { + Dispatchers.setMain(testDispatcher) + } + + override fun finished(description: Description) { + Dispatchers.resetMain() + } +} +``` + +Use: + +```kotlin +@get:Rule +val mainDispatcher = MainDispatcherRule() +``` + +- **`UnconfinedTestDispatcher`** for `viewModelScope` work that should + run eagerly (most cases). +- **`StandardTestDispatcher`** when ordering matters and you want to + drive virtual time with `advanceUntilIdle()`. +- Lives in `common/src/test/.../testing/` alongside fakes and + fixtures. + +## ViewModel testing pattern + +```kotlin +class MovieSummaryViewModelTest { + + private val repo = FakeMovieRepository() + private val viewModel = MovieSummaryViewModel(id = 42, repository = repo) + + @Test + fun `emits loading then loaded`() = runTest { + viewModel.state.test { + assertThat(awaitItem()).isInstanceOf(MovieSummaryUiState.Loading::class.java) + repo.emit(Movie(id = 42, title = "Inception", …)) + assertThat(awaitItem()).isInstanceOf(MovieSummaryUiState.Loaded::class.java) + cancelAndIgnoreRemainingEvents() + } + } +} +``` + +- Use `runTest { … }` from `kotlinx-coroutines-test`. +- Use Turbine's `state.test { … }` extension to assert on Flow + emissions. +- Fake repositories live in + `common/src/test/.../fakes/` or feature-local `test/.../fakes/`. They + implement the production interface. + +## Fakes pattern + +```kotlin +class FakeMovieRepository : MovieRepository { + private val updates = MutableSharedFlow(replay = 1) + + override fun movie(id: Long): Flow = updates.filter { it.id == id } + override suspend fun refresh(id: Long): Result = Result.success(Unit) + + fun emit(movie: Movie) { updates.tryEmit(movie) } +} +``` + +- Fakes hand-roll the surface; don't auto-generate. +- Inject fakes through Koin's `modules` override in tests. + +## Compose UI tests (when used) + +```kotlin +@get:Rule +val composeRule = createAndroidComposeRule() + +@Test +fun showsTitleWhenLoaded() { + composeRule.setContent { + MovieSummaryContent( + state = MovieSummaryUiState.Loaded(Movie.fixture(title = "Inception")), + onRetry = {}, + onBack = {}, + ) + } + + composeRule.onNodeWithText("Inception").assertIsDisplayed() +} +``` + +- Use `createAndroidComposeRule()` for component + tests — not `MainActivity`, which boots the whole app. +- Bigger integration tests in `app/` may use `MainActivity`. +- Use `setContent { TraktTheme { … } }` so themed colours resolve. + +## Previews + +- Every new `Content` composable has at least one `@Preview` + covering the `.Loaded` state. Add `.Loading` and `.Error` previews + when the screen has a sealed `UiState`. +- Use a `@PreviewParameter` provider for state variants (empty / full + / pinned / unread). +- Layer the **multi-config preview annotations** so a single source + generates light + dark + font-scale + size-class variants: + +```kotlin +@PreviewLightDark +@PreviewFontScale +@Preview(name = "Loaded") +@Composable +private fun MovieSummaryPreview_Loaded( + @PreviewParameter(MovieSummaryStateProvider::class) state: MovieSummaryUiState, +) { + TraktTheme { + MovieSummaryContent( + state = state, + onRetry = {}, + onBack = {}, + ) + } +} +``` + +- **Don't preview the stateful screen wrapper.** Preview the stateless + `Content` variant. +- Previews are the source of truth for screenshot tests (see below) — + the discipline of "every screen has previews for every state" pays + back twice. + +## Screenshot tests (future direction) + +Recommended approach when adopted: + +- **Paparazzi** (JVM-only) auto-generates one screenshot per + `@Preview` via [`ComposablePreviewScanner`](https://github.com/sergio-sastre/ComposablePreviewScanner). + Authors don't write tests — they write `@Preview`s, the scanner + does the rest. +- **Baselines generated by CI, never by a workstation.** A dedicated + GitHub Action records and commits baselines so platform-specific + font rendering doesn't pollute the repo (cited from Now in Android + and Element X Android). Local runs verify against the CI baseline + only. +- Snapshots tracked via **Git LFS** to keep the repo lean. +- Roborazzi is the alternative; Paparazzi is recommended for parity + with the wider Compose ecosystem. + +## Architecture tests (Konsist) + +[Konsist](https://github.com/LemonAppDev/konsist) declarations under +`common/src/test/.../konsist/` enforce structural rules that ktlint and +detekt can't: + +```kotlin +@Test +fun `composables that suffix Screen live in feature packages`() { + Konsist.scopeFromProduction() + .functions() + .withAnnotationOf(Composable::class) + .withName { it.endsWith("Screen") } + .assertTrue { it.resideInPackage("..core.feature..") } +} +``` + +Targets worth asserting: + +- `*Screen` composables only live under `core//` packages. +- `*ViewModel` classes never import from `androidx.compose.*`. +- Repository implementations are `internal`. +- Mappers live in `/mappers/` or alongside the repository. + +Adopt incrementally — add a Konsist suite per feature as patterns +solidify, not as a big-bang. + +## Performance / baseline profiles + +A `:benchmarks` module (`com.android.test` plugin) generates the AOT +compile profile shipped inside the APK to speed up first-launch +rendering. + +- Generate: `./gradlew :app:generateBaselineProfile` (and + `:tv:generateBaselineProfile` for the TV variant). +- Commit the resulting `app/src//generated/baselineProfiles/baseline-prof.txt` + alongside the PR that changes startup-relevant code. +- Startup macrobenchmarks (`androidx.benchmark.macro`) live in + `benchmarks/src/main/java/.../StartupBenchmark.kt`. They run on + Gradle Managed Devices in CI. +- Macrobenchmarks are not part of the unit-test loop; treat them as + perf gates run separately. + +## Test fixtures + +- Domain models expose `.fixture(...)` factories with sensible + defaults, parameter overrides: + +```kotlin +fun Movie.Companion.fixture( + id: Long = 1, + title: String = "Test Movie", + overview: String? = null, + releasedAt: LocalDate? = null, +): Movie = Movie(id, "test", title, overview, releasedAt) +``` + +- Fixtures live in + `common/src/test/.../fixtures/Fixtures.kt`. + +## Running + +```bash +./gradlew :common:test +./gradlew :app:testDebugUnitTest +./gradlew :app:connectedDebugAndroidTest # requires emulator/device +``` + +Locally, prefer the JVM-only `test` task. CI runs both. + +## Test philosophy + +- **Test behaviour, not implementation.** Assert on UiState emissions, + repository return values, mapper outputs. Don't assert "this private + method was called". +- **Pure functions over mocks.** If a unit needs four mocks, the unit + is doing too much. +- **One concept per test.** Tests verifying three behaviours fail + ambiguously. +- **Don't test the framework.** Don't assert that `StateFlow.collect` + works, that `Flow.map` maps, that `JsonDecoder` decodes JSON. +- **Avoid `delay(1_000)`-style waits.** Use `runTest` virtual time + + `advanceUntilIdle()` / Turbine's `awaitItem`. diff --git a/.agents/rules/theming.md b/.agents/rules/theming.md new file mode 100644 index 000000000..ff20c1c24 --- /dev/null +++ b/.agents/rules/theming.md @@ -0,0 +1,177 @@ +--- +trigger: glob +globs: '{**/ui/**/*.kt,**/theme/**/*.kt,**/*Theme.kt}' +description: 'TraktTheme tokens, MaterialTheme overlay, TraktSpacing / TraktTypography / TraktSize / TraktRadius. Coil 3 specifics. TV adaptations.' +applyTo: '{**/ui/**/*.kt,**/theme/**/*.kt,**/*Theme.kt}' +--- + +# Theming & Design Tokens + +## TraktTheme + +The design system is exposed through `TraktTheme` (in +`common/.../ui/theme/Theme.kt`). It owns four token namespaces: + +```kotlin +@Composable +fun TraktTheme( + colors: TraktColors = TraktTheme.colors, + typography: TraktTypography = TraktTheme.typography, + spacing: TraktSpacing = TraktTheme.spacing, + size: TraktSize = TraktTheme.size, + content: @Composable () -> Unit, +) + +object TraktTheme { + val colors: TraktColors @Composable @ReadOnlyComposable get() = LocalTraktColors.current + val typography: TraktTypography @Composable @ReadOnlyComposable get() = LocalTraktTypography.current + val spacing: TraktSpacing @Composable @ReadOnlyComposable get() = LocalTraktSpacing.current + val size: TraktSize @Composable @ReadOnlyComposable get() = LocalTraktSize.current +} +``` + +Use through `TraktTheme..` inside composables. Each +namespace is a data class of `Color`, `TextStyle`, or `Dp` values. + +## Colours + +All colours referenced via `TraktTheme.colors.*`: + +```kotlin +Text( + text = title, + color = TraktTheme.colors.textPrimary, +) + +Surface( + color = TraktTheme.colors.background, +) { … } +``` + +**Forbidden in new code:** + +- Raw `Color(0xFF7B68EE)` / `Color(red = …, green = …, blue = …)`. +- `MaterialTheme.colorScheme.primary` direct reads (use Trakt token). + Material 3 colour scheme is wired underneath; consumers go through + `TraktTheme.colors`. +- `colorResource(R.color.…)` for design-system colours. Colour + resources are fine for legacy values that are also referenced from + XML (notifications, app icon) but new tokens live in the theme. + +If a colour you need isn't in the palette, **add a token** to +`TraktColors` with light/dark/seasonal variants. Never paper over with +a literal at the call site. + +## Seasonal themes + +- Halloween (orange), Christmas (red), and other overrides flow + through Firebase Remote Config + `CustomThemeUseCase`. +- Theme switching wires up at the app root + (`TraktTheme(colors = customColors ?: DefaultColors) { … }`). +- Feature code doesn't branch on season — it reads + `TraktTheme.colors.*` and the active palette swaps out automatically. + +## Typography + +`TraktTheme.typography.*` exposes semantic styles: + +```kotlin +Text(title, style = TraktTheme.typography.title) +Text(body, style = TraktTheme.typography.body) +Text(tag, style = TraktTheme.typography.tag) +``` + +- New text uses semantic styles; no inline `TextStyle(fontSize = 14.sp)` + literals. +- Honour Dynamic Type — Material 3 picks up the system font scale by + default. Don't fight it with absolute pixel sizes. + +## Spacing + +`TraktTheme.spacing.*` is adaptive — values vary by window size class +(phone / tablet / TV). Use the named scale: + +```kotlin +Column(verticalArrangement = Arrangement.spacedBy(TraktTheme.spacing.md)) { … } +Modifier.padding(horizontal = TraktTheme.spacing.lg) +``` + +- **No magic numbers** (`Modifier.padding(16.dp)`). Either use a + token, or — if the value is genuinely unique to one composable — + declare it as a private `val` with a one-line reason. +- **Explicit arrangement spacing.** `Column`/`Row` declare + `verticalArrangement = Arrangement.spacedBy(...)` / + `horizontalArrangement = ...` rather than spacing items with + `Spacer(Modifier.height(...))`. + +## Sizes + +`TraktTheme.size.*` exposes adaptive sizes for icons, avatars, cards, +hero artwork. Same rules as spacing — use tokens, don't hard-code +`64.dp`. + +## Material 3 vs TraktTheme + +- The codebase wraps Material 3 at the root via `MaterialTheme(...)`. + Trakt tokens layer on top. +- Material 3 components (`Button`, `Card`, `TextField`) are welcome — + pass colours and shapes from `TraktTheme.*` rather than overriding + with raw values. + +## Images & icons + +- Bitmaps and remote images via **Coil 3** (`io.coil-kt.coil3`) + + `AsyncImage` / `SubcomposeAsyncImage`. +- Local drawables: declare under `resources/.../drawable/` and reference + via `R.drawable.`. `Painter` via `painterResource(R.drawable...)`. +- Material 3 `Icons.Filled.*` / `Icons.Rounded.*` are acceptable for + generic glyphs (back arrow, more, search). Reserve custom Trakt icons + for branded assets. +- **Forbidden in new code:** Glide, Fresco, Picasso. Stick with Coil. + +### Coil 3 specifics + +- Prefer **`SubcomposeAsyncImage`** when the same composable needs + `Loading` / `Error` / `Success` state branches; reach for + `AsyncImage` for the common no-branch case. +- **Image request keys must be stable strings.** Never + `UUID.randomUUID()` or `System.currentTimeMillis()` inside a + request — they bust the cache on every recomposition. +- The shared `ImageLoader` is wired once in Koin as a `single { }`. + Custom Coil interceptors (Trakt CDN URL normalisation, fallback-host + swaps, accept-header pinning) live in + `common/.../networking/coil/` and attach there. Never construct a + fresh `ImageLoader` per call site. +- Coil 3 uses the **Ktor 3 engine adapter** (`coil-network-ktor3`); + reuse the existing `HttpClient` rather than spinning up a parallel + OkHttp instance. + +## TV-specific theming + +- TV module has its own `TraktTheme` wrapper using + `androidx.tv.material3.MaterialTheme`. +- Focus styling uses Compose TV's built-in focus indicators; avoid + hand-rolling focus rings. +- Spacing tokens in the TV `TraktSpacing` are scaled up from phone + values — use the same semantic name (`md`, `lg`) and let the token + carry the platform difference. + +## Accessibility + +- Touch targets ≥ 48dp on phone, ≥ focus-safe size on TV. +- `Modifier.semantics { contentDescription = "…" }` for non-text + composables. +- Animations respect reduced motion when system preferences indicate + it. +- Contrast: WCAG AA. The token palette enforces this; don't fight it + with one-off colours. + +## Quick checklist + +- [ ] Colours via `TraktTheme.colors.*` — no raw `Color(...)` literals +- [ ] Spacing via `TraktTheme.spacing.*` — no `padding(16.dp)` +- [ ] Sizes via `TraktTheme.size.*` — no `Modifier.size(64.dp)` magic +- [ ] Typography via `TraktTheme.typography.*` — no inline `TextStyle` +- [ ] Images via Coil 3 + `R.drawable.*`, never Glide/Fresco/Picasso +- [ ] Light / dark / seasonal variants exist for every new token +- [ ] Animations honour reduced-motion diff --git a/.gemini/config.yaml b/.gemini/config.yaml index c8ce4927f..091c400e4 100644 --- a/.gemini/config.yaml +++ b/.gemini/config.yaml @@ -10,7 +10,18 @@ code_review: summary: true code_review: true include_drafts: true -ignore_patterns: [ - 'build/**', - '.gradle/**' -] +ignore_patterns: + - 'build/**' + - '.gradle/**' + - '.kotlin/**' + - '.idea/**' + - 'external/**' + - '**/generated/**' + - '**/build/generate-resources/**' + - 'resources/src/main/res/values-*/strings.xml' + - '**/*.pb' + - '**/*.proto' + - 'gradle/wrapper/**' + - '**/Package.resolved' + - 'fastlane/metadata/**' + - 'fastlane/screenshots/**' diff --git a/.gemini/styleguide.md b/.gemini/styleguide.md new file mode 100644 index 000000000..c8f749286 --- /dev/null +++ b/.gemini/styleguide.md @@ -0,0 +1,267 @@ +# Project Coding Standards + +Style guide for AI-assisted review of the trakt-android monorepo +(phone + Android TV, Compose-first, Kotlin 2.x). Mirrors +`.agents/rules/*.md` — those are authoritative; this is the digest +Gemini uses when reviewing pull requests. + +## Tech stack + +- Kotlin 2.3.x, JVM target 11, Compose BOM `2026.05.00`. +- Min SDK 28, Compile/Target SDK 37, AGP 9.2.x. +- Jetpack Compose for all UI. No XML, no Fragments, no `LiveData`, + no RxJava. +- Coroutines + Flow. `StateFlow` for screen state; `SharedFlow` for + one-shot events. +- **DI: Koin 4.2.x** with module DSL. No Hilt, no Dagger. +- Networking: Ktor 3.4.x + OpenAPI-generated client + Moshi for + generated DTOs + kotlinx.serialization for typed routes / protobuf. +- Persistence: DataStore (Preferences / typed Proto). No + SharedPreferences in new code, no Realm, no SQLDelight. +- Images: Coil 3.x. +- Modules: `:app`, `:tv`, `:common`, `:resources`, `:buildSrc`. + +## Review scope + +- Apply rules to **new files** and **substantially rewritten files**. +- Skip nits inside legacy/generated zones (see + `.agents/rules/legacy-zones.md`): `build/`, `external/`, generated + OpenAPI sources, `values-*/strings.xml` translation files. +- Don't flag the same pattern repeatedly across legacy files — once is + enough. + +## Naming conventions + +- **PascalCase** — types, Composables, file names matching the primary + type. +- **camelCase** — properties, functions, locals. +- **SCREAMING_SNAKE_CASE** — top-level / companion `const val` only. +- **Acronyms** — official Kotlin style: treat as words. `Url` / `Id` + / `Json` for type names; `url`, `id`, `json` for properties. +- **Package names** — lowercase, no underscores. +- **File naming**: + - Composable screens → `*Screen.kt` + - ViewModels → `*ViewModel.kt` + - Sealed UI state → `*State.kt` + - Koin modules → `*Module.kt` under `di/` + - Mappers → `*Mapper.kt` or `mapTo*.kt` + - Tests → `*Test.kt` (JUnit) or matching class name + `Test` suffix + +## Commit standards + +Conventional Commits with android-scoped enum. See +`.agents/rules/commits.md` for full enum. Allowed scopes: `app`, `tv`, +`common`, `resources`, `openapi`, `widget`, `i18n`, `ci`, `deps`, +`gradle`, `agents`, `gemini`. + +``` +feat(app): add notes drawer +fix(common): handle 204 from credits endpoint +chore(i18n): translations updates from CrowdIn +chore(deps): bump compose BOM +``` + +Subject lowercase, no trailing period, ≤ 72 chars. PR titles follow +the same format. No em-dashes anywhere in commits / PR text — use +plain hyphens. + +## Code principles + +- **Pure functions** for data transformation. Side effects (network, + disk, Firebase, analytics) live at the edges — repositories, + use-cases, ViewModel collectors. Never inside mappers or composables. +- **Immutability**: prefer `val`. `data class`es for models. + `ImmutableList` / `PersistentList` for collections held in state. +- **Early exits** via guard `if (x == null) return`. No nested `let { }` + pyramids. +- **No `!!`** in production code. Use `requireNotNull(value) { … }` or + `?:`. `!!` is acceptable in tests and previews. +- **DI via constructor.** Koin `module { }` blocks wire dependencies. + No `KoinJavaComponent.getKoin().get()` inside type bodies. +- **Functions with 3+ params take a single `data class` param.** +- **Single responsibility** per function and per type. ViewModels > + 300 lines are smells. + +## Architecture + +- **MVVM + UDF.** ViewModel exposes `StateFlow` built via + `stateIn(viewModelScope, WhileSubscribed(5_000), Initial)`. +- **Sealed `UiState`** — `Loading` / `Loaded(...)` / `Error(...)`. + Not flat `data class` with three nullable fields. +- **Stateful `Screen` + stateless `Content`** split. Previews and + snapshot tests target the `Content` variant. +- **Compose Navigation 2.9+** with `@Serializable` typed routes. No + string-route DSL for new screens. +- **Repository** is the public data boundary: `Flow` for reads, + `suspend fun` returning `Result<…>` for writes. Offline-first where + possible. +- **UseCases optional** — only when combining multiple repositories or + encapsulating shared business logic. + +## State management + +- **`StateFlow`** for screen state. No `mutableStateOf` in ViewModels + (Compose-coupled, hard to test). +- **`remember { mutableStateOf(...) }`** for transient UI that dies + with the composable. +- **`rememberSaveable`** for transient state that survives config + change. +- **Don't expose `MutableStateFlow` publicly.** Mutate from inside the + ViewModel; expose `StateFlow` only. +- **One-shot events** via `SharedFlow(replay = 0)` collected in + `LaunchedEffect`. Don't fire events from inside `body { }`. +- **`update { … }`** on `MutableStateFlow` for atomic mutations. +- **`@Immutable`** on data classes held in state. + +## Compose + +- **`when` over sealed `UiState` is exhaustive.** No `else ->` branches + swallowing new cases. Recent bug across stacks + (`keep tag(isLatestAired:provider:) switch exhaustive`) shows the + cost. +- **`Modifier` is the second parameter** with default + `modifier: Modifier = Modifier`. +- **Slot APIs use `@Composable` lambda parameters** — + `content: @Composable () -> Unit`. +- **Side effects** in `LaunchedEffect(key)` / `DisposableEffect(key)`. + Never start a coroutine from `body { }` directly. +- **State hoisting**: state in, events out as lambdas. + +## Networking + +- **OpenAPI client is generated.** Never edited by hand. +- **Mappers are pure**, named `…ToDomain` / `mapTo`. Live in + `common/...//` or `common/.../networking/mappers/`. +- **Status 204 is success.** Mappers handle 204 by returning + empty/default values. +- **Typed errors** at the boundary via an `ApiError` sealed type. + Raw `IOException` / `HttpException` don't escape into ViewModels. +- **Repositories** expose `Flow<…>` reads + `suspend` writes. No + `Activity` / `Context` references in repositories. + +## Persistence + +- **DataStore** for new preferences. **No** `SharedPreferences` direct + usage, **no** Realm, **no** SQLDelight in new code. +- Keys live in a `companion object` next to the store. +- Reads return `Flow`; writes are `suspend`. +- Auth tokens go through `AuthStorage`, never DataStore directly. + +## Theming + +- All colours via `TraktTheme.colors.*`. **Forbid raw `Color(0xFF…)` / + `Color(red = …)`** and `MaterialTheme.colorScheme.*` direct reads + in feature code. +- All spacing via `TraktTheme.spacing.*`. **No `Modifier.padding(16.dp)`** + magic numbers. +- All sizes via `TraktTheme.size.*`. +- All typography via `TraktTheme.typography.*`. No inline `TextStyle` + literals. +- Images via Coil 3 + `R.drawable.*`. **No Glide / Fresco / Picasso.** +- Animations respect reduced-motion preferences. + +## Localization + +- All user-facing strings live in `resources/.../values/strings.xml`. + Translation `values-/` files are Crowdin-managed. +- **`L10n` equivalent** is the generated `R.string.*` reference; do + not hand-edit `values-*` translation files. +- **No inline raw string literals** for user-facing text in + composables (debug logs are not localised). +- Keys are namespaced: `action_*`, `common_*`, `a11y_*`, + `screen__*`, `error_*`. Add `_tv` suffix for TV-only variants. +- Use `` + `pluralStringResource` for counts. No `+` + concatenation. + +## Testing + +- **Unit tests** in `*/src/test/`, Android tests in `*/src/androidTest/`. +- **Turbine** for Flow assertions, **Truth** for general assertions, + **`runTest`** for coroutine virtual time. +- **No mocking libraries** for new tests (Mockito / MockK). Use + hand-written fakes implementing production interfaces. +- **Mandatory unit coverage**: mappers, ViewModels (state emissions), + repositories, pure helpers, use-cases. +- **Compose UI tests** with `ComponentActivity` (not `MainActivity`) + for component-level tests. +- Every new `Content` composable has at least one `@Preview` + covering `.Loaded`. Add `.Loading` / `.Error` previews when + applicable. +- Roborazzi screenshot tests recommended for the future; baselines + generated by CI, not workstation. + +## Packages / Gradle + +- Module direction: `app → tv/common → common → resources`. Never + reverse. +- **New deps added to `gradle/libs.versions.toml` first.** No literal + coordinates in `build.gradle.kts`. +- Plugin aliases via the catalogue. +- New external dep requires PR-body justification (need, maintenance + posture, footprint, exit plan). +- KSP > KAPT for new annotation processing. + +## Anti-patterns to flag in review + +Severity-prioritised list: + +**High (block-worthy):** + +- New XML layouts, Fragments, or `AppCompatActivity` subclasses. +- DataBinding / ViewBinding in new code (Compose-only project). +- `LiveData`, `RxJava`, `Observable` in new code. +- `!!` non-null assertions in production code. +- `runBlocking` / `GlobalScope` outside tests. +- `KAPT` for newly introduced annotation processing — KSP only. +- New `SharedPreferences` direct usage. +- Raw `Color(0xFF…)` / `Color(red = …)` / `MaterialTheme.colorScheme.*` + in feature code. +- Inline `TextStyle(fontSize = 14.sp)` literals. +- `Image("string-literal")` for design-system images. +- Hand-edits to `values-/strings.xml` translation files. +- Hand-edits to generated OpenAPI sources. +- `KoinJavaComponent.getKoin().get()` inside type bodies. +- Mocking libraries (Mockito, MockK) in new tests. +- `mutableStateOf` inside a ViewModel. +- Exposing `MutableStateFlow` publicly. +- Repository emitting remote responses without writing to local first + (breaks the offline-first invariant). +- Composable / mapper that takes a `Context` purely to resolve a + string — return `@StringRes Int?` instead. +- `Timber` calls that interpolate PII (emails, tokens, OAuth codes, + request bodies) into the message. + +**Medium:** + +- `Modifier.padding(16.dp)` magic numbers. +- `Modifier.size(64.dp)` magic numbers. +- Inline `Spacer(Modifier.height(8.dp))` instead of arrangement + spacing. +- Implicit (default) Compose Material colours when a Trakt token + exists. +- ViewModels > 300 lines. +- Functions with 4+ positional parameters. +- `default:` / `else ->` branches on sealed domain enums. +- `Task { … }`/`launch { … }` inside composable `body { }` (use + `LaunchedEffect`). + +**Low (advisory):** + +- File naming that doesn't match the primary type. +- `forEach` used for transformation (use `map`). +- Over-defensive `[weak self]`-style captures (Kotlin equivalent: + unnecessary `lambda::class.java` ceremony). + +## Review tone + +Match the user's GitHub voice (per their global preferences): + +- Direct, friendly, no preamble. +- Plain hyphens — **no em-dashes / en-dashes anywhere**. +- Short paragraphs. Contractions OK. +- Use `Adopted - .` and `Leaving as-is - .` + patterns when responding to PR author replies. +- No "Generated with Claude Code" footers. No + `Co-Authored-By: Claude` trailers unless the human asks. +- Critique is welcome; flattery is not (`This looks great!` is not a + review comment). diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md new file mode 120000 index 000000000..be77ac83a --- /dev/null +++ b/.github/copilot-instructions.md @@ -0,0 +1 @@ +../AGENTS.md \ No newline at end of file diff --git a/AGENTS.md b/AGENTS.md new file mode 100644 index 000000000..d1b4ede02 --- /dev/null +++ b/AGENTS.md @@ -0,0 +1,46 @@ +Before implementing anything, identify which domain you are working in and +actively apply the corresponding rule file for that domain: + +- Compose screens, composables, theme (`app/`, `tv/`, `common/.../ui/`): + apply architecture.md, state-management.md, theming.md (note: + `compose-stability.conf` at repo root lists stable types) +- Networking, Ktor client, OpenAPI mappers (`common/.../networking/`, + generated OpenAPI sources): apply networking.md +- Persistence (DataStore, Room entities, file caches): apply persistence.md +- Localization (`resources/src/main/res/values*/strings.xml`, + `ValidateStringPlaceholdersTask`): apply localization.md +- Tests (`*Test.kt`, `*UnitTest.kt`, `*InstrumentedTest.kt`): apply testing.md +- Gradle module boundaries, `build.gradle.kts` edits, version catalog + (`gradle/libs.versions.toml`), new external deps: apply packages.md +- Edits inside the legacy zones listed in `legacy-zones.md`: rules are + advisory only; do not extend legacy patterns into new files +- All other source code: apply project.md, code-principles.md, and + implementation.md (always-on baseline). + +All rule files are referenced below. + +@.agents/rules/project.md + +@.agents/rules/code-principles.md + +@.agents/rules/implementation.md + +@.agents/rules/architecture.md + +@.agents/rules/state-management.md + +@.agents/rules/networking.md + +@.agents/rules/persistence.md + +@.agents/rules/localization.md + +@.agents/rules/theming.md + +@.agents/rules/testing.md + +@.agents/rules/packages.md + +@.agents/rules/legacy-zones.md + +@.agents/rules/commits.md diff --git a/CLAUDE.md b/CLAUDE.md new file mode 120000 index 000000000..47dc3e3d8 --- /dev/null +++ b/CLAUDE.md @@ -0,0 +1 @@ +AGENTS.md \ No newline at end of file diff --git a/app/CLAUDE.md b/app/CLAUDE.md new file mode 100644 index 000000000..b5259d6fe --- /dev/null +++ b/app/CLAUDE.md @@ -0,0 +1,25 @@ +# app — Claude instructions + +Part of the `trakt-android` monorepo. Root rules apply — see +`../AGENTS.md`. + +## Overlay for this module + +- **Role**: phone app entry point. Hosts the main `NavHost`, the + `MainActivity`, billing flow, and most user-facing feature screens. +- **Plugin**: `com.android.application` + `kotlin.compose` + + `kotlin.serialization` + Firebase plugins. +- **Active areas (new code preferred here)**: + - `Trakt/core//` — feature folders (home, calendar, + search, profile, billing). Each owns `Screen`, `State`, + `ViewModel`, `di/Module.kt`. + - `Trakt/ui/` — app-level theme glue, snackbar host, extensions. + - `Trakt/helpers/`, `Trakt/analytics/`. +- **Architecture**: MVVM + UDF. `koinViewModel()` retrieves + ViewModels. `NavigationStack(path:)`-equivalent via Compose + Navigation typed routes. See `../.agents/rules/architecture.md`. +- **TV variant**: TV screens live in `:tv`. Share ViewModels and + state types via `:common`; render with TV-specific Compose layouts. +- **No** new XML layouts, Fragments, or `AppCompatActivity` subclasses. + +@../AGENTS.md diff --git a/common/CLAUDE.md b/common/CLAUDE.md new file mode 100644 index 000000000..90211f679 --- /dev/null +++ b/common/CLAUDE.md @@ -0,0 +1,35 @@ +# common — Claude instructions + +Part of the `trakt-android` monorepo. Root rules apply — see +`../AGENTS.md`. + +## Overlay for this module + +- **Role**: bottom of the dependency graph. Domain models, Ktor / + OpenAPI networking, repositories, Koin modules, design-system + primitives (`TraktTheme`), Firebase wrappers, helpers. +- **Plugin**: `com.android.library` + `kotlin.compose` + + `kotlin.serialization`. +- **Hard rule**: never imports from `:app` or `:tv`. If a type belongs + here, it must compile in isolation against external deps and + `:resources`. +- **Active areas (new code preferred here)**: + - `common/.../networking/` — `KtorClientFactory`, interceptors, + mappers (`Mapper.kt`). + - `common/.../model/` — domain types. `@Immutable` data classes, + `Sendable`-friendly (listed in `compose-stability.conf`). + - `common/...//` — repositories, use-cases, entity caches. + - `common/.../ui/` — cross-platform Compose primitives. + - `common/.../di/` — Koin modules wired in `TraktApplication`. + - `common/.../firebase/` — Crashlytics, Analytics, Remote Config + wrappers. +- **Generated sources** at `build/generate-resources/...` come from + the OpenAPI generator — never hand-edited. ktlint already excludes + them via `.editorconfig`. +- **State**: domain types are immutable. State containers expose + `Flow` reads and `suspend fun` writes. Local-write-first + repository contract per `networking.md`. +- **Testing**: `common/src/test/.../fakes/` and `.../fixtures/` for + fakes and `.fixture(...)` factories. No mocking libraries. + +@../AGENTS.md diff --git a/compose-stability.conf b/compose-stability.conf new file mode 100644 index 000000000..25ea397b4 --- /dev/null +++ b/compose-stability.conf @@ -0,0 +1,44 @@ +# Compose stability configuration. +# +# Tells the Compose compiler which types it should treat as stable, so +# composables that take them as parameters can be skipped when their +# instance is structurally unchanged. +# +# Wire from a module's `kotlinOptions { freeCompilerArgs }`: +# "-P", "plugin:androidx.compose.compiler.plugins.kotlin:stabilityConfigurationPath=$rootDir/compose-stability.conf" +# +# See: https://developer.android.com/develop/ui/compose/performance/stability/fix#configuration-file +# +# Rules: one fully-qualified type pattern per line, '*' wildcard suffix +# matches everything in that package or subpackage. + +# Trakt domain models — all data classes in common are immutable by +# construction; mark the whole package stable. +tv.trakt.trakt.common.model.* +tv.trakt.trakt.common.networking.responses.* + +# Resources R class types. +tv.trakt.trakt.resources.R.* + +# Kotlin standard library types that hold no observable mutable state. +kotlin.time.Duration +kotlin.Result +kotlin.Pair +kotlin.Triple + +# kotlinx — datetime + immutable collections are deeply immutable. +kotlinx.datetime.* +kotlinx.collections.immutable.ImmutableCollection +kotlinx.collections.immutable.ImmutableList +kotlinx.collections.immutable.ImmutableMap +kotlinx.collections.immutable.ImmutableSet +kotlinx.collections.immutable.PersistentCollection +kotlinx.collections.immutable.PersistentList +kotlinx.collections.immutable.PersistentMap +kotlinx.collections.immutable.PersistentSet + +# Coil 3 image state — stable in practice, not annotated upstream. +coil3.compose.AsyncImagePainter.State + +# Paging compose — exposes stable identity. +androidx.paging.compose.LazyPagingItems diff --git a/resources/CLAUDE.md b/resources/CLAUDE.md new file mode 100644 index 000000000..fd65240ba --- /dev/null +++ b/resources/CLAUDE.md @@ -0,0 +1,26 @@ +# resources — Claude instructions + +Part of the `trakt-android` monorepo. Root rules apply — see +`../AGENTS.md`. + +## Overlay for this module + +- **Role**: pure resource library. Strings, drawables, locale variants. + No Kotlin sources. +- **Plugin**: `com.android.library`. +- **Hard rule**: depends on nothing. Bottom of the graph. +- **Active areas**: + - `src/main/res/values/strings.xml` — `en` source. Hand-edit here. + - `src/main/res/values-/strings.xml` — translations. **Never + hand-edit.** Crowdin owns them; the + `.github/workflows/i18n_sync.yml` job opens daily sync PRs. + - `src/main/res/drawable/` — vector and raster assets. Reference via + `R.drawable.`. +- **String-key namespacing**: `action_*`, `common_*`, `a11y_*`, + `screen__*`, `error_*`. `_tv` suffix for TV-only variants. + See `../.agents/rules/localization.md`. +- **Placeholder parity**: `buildSrc/.../ValidateStringPlaceholdersTask` + guards against placeholder mismatches across locales. Run before + merging. + +@../AGENTS.md diff --git a/tv/CLAUDE.md b/tv/CLAUDE.md new file mode 100644 index 000000000..8d17d25e8 --- /dev/null +++ b/tv/CLAUDE.md @@ -0,0 +1,32 @@ +# tv — Claude instructions + +Part of the `trakt-android` monorepo. Root rules apply — see +`../AGENTS.md`. + +## Overlay for this module + +- **Role**: Android TV variant. Hosts `TvActivity`, `TvSplashActivity`, + and a Compose-only UI tuned for D-pad navigation. Reuses `:common` + ViewModels and domain types where possible. +- **Plugin**: `com.android.library` + `kotlin.compose` + + `kotlin.serialization`. +- **Active areas (new code preferred here)**: + - `tv/trakt/trakt/app/core//` — TV feature flows (home, + lists, auth, streamings, episodes, comments). + - `tv/trakt/trakt/app/ui/theme/` — TV-specific `TraktTheme` wrapper + over `androidx.tv.material3.MaterialTheme`. +- **TV-specific considerations**: + - 10-foot legibility — larger touch targets, larger typography + (`TraktTheme.typography.title` one tier up from phone). + - Honour the focus engine: `.focusable`, `.focused`, `.focusEffect`. + Don't fight platform focus indicators. + - D-pad navigation comes free with `Modifier.clickable`; do not + override with custom gesture detection. + - `androidx.tv.material3` `Surface` / `Button` — not the phone + Material 3 equivalents. +- **State**: same as phone — `StateFlow` + sealed states. + Share types via `:common`. +- **Navigation**: typed Compose Navigation routes per feature, same + conventions as `:app`. + +@../AGENTS.md