From 143f2accb76ea50788193e6a654a9d5b9965dd68 Mon Sep 17 00:00:00 2001 From: austinried <4966622+austinried@users.noreply.github.com> Date: Tue, 22 Sep 2026 05:54:20 +0000 Subject: [PATCH] document the phase 2 slice, add ktlint to ci, and warm the gradle cache --- AGENTS.md | 11 +++++-- docs/architecture.md | 61 ++++++++++++++++++++++++++++--------- docs/implementation-plan.md | 14 +++++---- tools/preview_server.py | 4 +++ 4 files changed, 67 insertions(+), 23 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 020bd1c1..7dc3c4c5 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -18,7 +18,8 @@ Everything comes from the Nix flake devshell; do not install toolchains by hand. - Build: `gradle :app:assembleDebug` - Unit tests: `gradle :app:testDebugUnitTest` -- Lint: `gradle :app:lintDebug` +- Lint: `gradle :app:ktlintCheck :app:lintDebug` +- Format: `gradle :app:ktlintFormat` - Integration tests: `./tools/integration-test.nu` (starts navidrome and gonic, then runs `:app:integrationTest`) - Screenshots: `gradle :app:recordRoborazziDebug` to record, `gradle :app:verifyRoborazziDebug` to check @@ -36,14 +37,20 @@ Everything comes from the Nix flake devshell; do not install toolchains by hand. ## Conventions - Prefer self-explanatory code; do not add comments unless the reason cannot be inferred from the code itself (docstrings for functions and exports are fine). +- ktlint (`org.jlleitschuh.gradle.ktlint`) enforces style and flags unused imports; run `gradle :app:ktlintFormat` before committing. `.editorconfig` exempts `@Composable` functions from the lowerCamelCase function-naming rule. - Room 3 (`androidx.room3`) on AndroidX `sqlite-bundled` (`BundledSQLiteDriver`, SQLite 3.50+, FTS5). Do not reintroduce SQLDelight or the platform SQLite. - Networking uses OkHttp and DOM XML parsing; auth uses the Subsonic token scheme by default. -- Use coroutines and `Flow`; library reads are exposed as `Flow` from Room. +- Use coroutines and `Flow`; library reads are exposed as `Flow` from Room. Large lists page with Paging 3 over Room `PagingSource` rather than loading the whole table. +- UI state lives in `androidx.lifecycle.ViewModel`, wired with Koin (`koinViewModel()`); screens split into a stateful `*Route` and a stateless `*Screen` for screenshot tests. +- UI and playback preferences go in DataStore (`UserPreferences`), not Room. - Verify current dependency versions and their compatibility before adding or pinning anything. ## Gotchas - Robolectric runs at SDK 35 while `targetSdk` is 37; raising it means re-recording the Roborazzi goldens. +- Room 3 does not map `PagingSource` automatically: a DAO that returns it needs `@DaoReturnTypeConverters(PagingSourceDaoReturnTypeConverter::class)`. +- Material icons are not pulled in by `material3`; use the BOM-managed `material-icons-core`/`material-icons-extended` (frozen at 1.7.8). Extended is large in debug builds but R8 strips unused icons from release. +- Robolectric creates a fresh `Application` per test in one JVM, so `SubtracksApp.onCreate` stops any running Koin before `startKoin`. - `applicationId` is `com.subtracks.next` for the beta; change it to `com.subtracks` before any store release. - The CI image provides `nix-ld`, `jq`, `sqlite`, `node`, `zstd` and a `runner` user that `cache-nix-action` expects. None of that is needed locally beyond the devshell. - Integration servers: navidrome on 4533 (`admin`/`password`), gonic on 4747 (`admin`/`admin`). The test music is cached in `.integration/music` (gitignored). diff --git a/docs/architecture.md b/docs/architecture.md index 15a9f3e4..7d5ecdd0 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -14,46 +14,75 @@ Subtracks is a native Android client for Subsonic-compatible servers (Navidrome, | Concern | Choice | |---|---| | UI | Jetpack Compose, Material 3 | -| Navigation | Navigation Compose (planned) | -| DI | Koin, constructor injection (planned) | +| Navigation | Navigation Compose | +| DI | Koin, constructor injection | | Persistence | Room 3 (`androidx.room3`) on AndroidX `sqlite-bundled` (SQLite 3.50+, FTS5) | +| Lists | Paging 3 over Room `PagingSource` (local mirror, not remote paging) | | Networking | OkHttp + DOM XML parsing | | Async | Kotlin coroutines / `Flow` | -| Images | Coil 3 (planned) | +| Images | Coil 3 | | Media | AndroidX Media3 / ExoPlayer (planned) | -| Settings | DataStore (planned) | +| Settings | DataStore Preferences (UI and playback prefs) | | Build | AGP 9.4.1, Gradle 9.7.1, Kotlin 2.3.10 (AGP built-in Kotlin), compileSdk/targetSdk 37 | +Versions live in `gradle/libs.versions.toml`; check current releases before bumping (Koin 4.2.2, Navigation Compose 2.10.1, Paging 3.5.1, DataStore 1.2.1, Coil 3.6.3 at the time of writing). + ## Source layout A single `app` module for now, so the data layer can settle before any module split. If the UI grows large, split `:core:data`, `:core:model` and `:feature:*`. ``` app/src/main/kotlin/com/subtracks - MainActivity.kt, SubtracksApp.kt - ui/ Compose screens and theme (early) + MainActivity.kt, SubtracksApp.kt Composition root: starts Koin and Coil + di/AppModule.kt Koin module + ui/ + AppRoot.kt Root gate and NavHost + library/ Tabbed LibraryScreen, album/playlist detail + settings/ Server list and add-server form + components/ Cover art and empty/loading states + theme/ Material 3 theme (stark black/white) data/ - model/Models.kt Room entities, including the search_index FTS5 table - db/ SubtracksDatabase, DAOs, bundled-SQLite builder - source/ MusicSource abstraction - subsonic/ SubsonicClient, SubsonicXml, SubsonicSource - sync/SyncService.kt fetch, then one transaction of upsert + prune + model/Models.kt Room entities, including the search_index FTS5 table + db/ SubtracksDatabase, DAOs, bundled-SQLite builder + prefs/UserPreferences.kt DataStore-backed UI prefs + repo/ SourceRepository, LibraryRepository + source/ MusicSource abstraction + subsonic/ SubsonicClient, SubsonicXml, SubsonicSource + sync/ SyncService (fetch + transaction), SyncManager (status) ``` ## Layers ``` Compose UI -> ViewModel -> Repository -> MusicSource (remote, Subsonic) - \-> Room (local mirror, Flow) + \-> Room (local mirror, Flow / PagingSource) ``` - The remote side sits behind `MusicSource` so additional backends can be added later. +- `SourceRepository` owns the configured servers and builds a `SubsonicSource` from the active one; it also serves cover-art URLs. `LibraryRepository` reads the local mirror. - The local side is Room; reads are `Flow`s, so the UI updates as sync writes. -- A repository layer (planned in Phase 2) joins the two: read from Room, refresh via `MusicSource` and `SyncService`. +- `SyncManager` triggers syncs off the UI (its own scope) and exposes a `StateFlow`; `SyncService` does the actual work. +- ViewModels are wired with Koin; screens are split into a stateful `*Route` (default `koinViewModel()`) and a stateless `*Screen` so the latter can be screenshot-tested directly. + +## UI shell + +The library is one `LibraryScreen` with a pinned top bar: the current section as the title, a custom row of icon buttons (Albums, Artists, Songs, Playlists) on the left, and Sync and Settings icon actions on the right. The selected section is drawn as a filled box in the content colour with the icon inverted, like the Flutter app — this is a hand-rolled row, not Material `TabRow` (no underline indicator). Icons use the Material rounded variants. Tabs switch the content in place; album and playlist rows push a detail screen, and Settings is pushed from the top bar. The sync action doubles as the progress indicator while a sync runs. There is no bottom navigation. (The Flutter app also had a Now Playing bar pinned to the bottom; that returns with playback in Phase 3.) + +The Albums tab is a three-column grid of covers only — a later preference will toggle captions, and sorting will hang off a FAB, so Settings only manages servers and neither shows sort nor sync controls. + +The theme is monochrome — black surface, white content and accent — with no colour extracted from cover art (that was a Flutter feature and is not ported yet). ## Data model -`sources` and `subsonic_sources` hold the configured servers, and `app_settings` holds playback preferences. Library tables are keyed by `(sourceId, id)` and cascade from `sources`: `artists`, `albums`, `playlists`, `playlist_songs`, `songs`. `search_index` is an FTS5 table (trigram tokenizer) over titles, maintained by sync. +`sources` and `subsonic_sources` hold the configured servers. Library tables are keyed by `(sourceId, id)` and cascade from `sources`: `artists`, `albums`, `playlists`, `playlist_songs`, `songs`. `search_index` is an FTS5 table (trigram tokenizer) over titles, maintained by sync. + +UI and playback preferences live in DataStore (`user_prefs`), not in SQLite: they are not relational, nothing joins against them, and keeping them out of Room avoids a schema migration per preference. Room is for the library only. + +Library screens page over Room with `PagingSource` queries, ordering by a sort chosen in preferences (name, artist, year, recently added). Paging is local, but it keeps memory bounded on large libraries and lets a huge song or playlist list render incrementally. Room 3 requires `@DaoReturnTypeConverters(PagingSourceDaoReturnTypeConverter::class)` on the DAO for this. + +Song rows show the album's cover art, fetched by joining `songs` to `albums` into a `SongListItem` rather than storing cover art on the song: some servers return a distinct cover ID per track, and keeping those would fragment the image cache for no visual gain. Artists show their image cropped to a circle. + +Images: grids and lists request a 256 px `getCoverArt` thumbnail; the 120 dp detail headers request the original. Because every request is freshly salted, the URL changes constantly, so each image is wrapped in a `CoverArtRef` carrying a `memoryCacheKey`/`diskCacheKey` of `sourceId:coverArtId:thumbnail` — caching is keyed on the identity of the art, not the request URL, and so survives the salt and app restarts. ## Sync @@ -65,7 +94,7 @@ Compose UI -> ViewModel -> Repository -> MusicSource (remote, Subsonic) ## Networking and auth -`SubsonicClient` speaks the Subsonic REST API over OkHttp. Auth is the token scheme (`t = md5(password + salt)`, with a fresh salt per request); plaintext `p=` is supported as a fallback for old servers. Responses are parsed with a hardened `DocumentBuilderFactory` (no DTDs, no external entities), and the client rejects any document that is not a ``, so a captive portal or login page can never be mistaken for an empty library and wipe the local mirror. +`SubsonicClient` speaks the Subsonic REST API over OkHttp. Auth is the token scheme (`t = md5(password + salt)`), salted freshly on every request including media URLs; plaintext `p=` is supported as a fallback for old servers. Responses are parsed with a `DocumentBuilderFactory` hardened best-effort (Android's parser rejects some flags, so unsupported ones are ignored), and the client rejects any document that is not a ``, so a captive portal or login page can never be mistaken for an empty library and wipe the local mirror. ## Security and privacy @@ -74,6 +103,8 @@ No analytics or third-party telemetry. Credentials are stored locally in app dat ## Testing - Unit tests use Robolectric, Roborazzi (screenshot goldens) and MockWebServer (client), plus an in-memory Room database on the bundled SQLite JVM driver (Robolectric's SQLite has no FTS5). +- Screenshot goldens exercise the stateless `*Screen` composables with fake data; paging screens are fed `PagingData.from(...)`. Record with `gradle :app:recordRoborazziDebug`, check with `:app:verifyRoborazziDebug`. +- Cover art in goldens is generated deterministically in the test and served through Coil's `FakeImageLoaderEngine` (synchronous, and no third-party images are committed); a real `ImageLoader` is installed for the run and reset afterwards. Each image is two or three flat colours in a pattern that identifies its type — stripes for albums, dots for playlists, rings for artists. - Integration tests run `SubsonicSourceIntegrationTest` against real navidrome and gonic instances, driven by `tools/integration-test.nu`. ## Build and CI diff --git a/docs/implementation-plan.md b/docs/implementation-plan.md index abb3fc6a..5ce7add1 100644 --- a/docs/implementation-plan.md +++ b/docs/implementation-plan.md @@ -16,15 +16,17 @@ Everything needed to mirror a server library locally: - FTS5 trigram search index over titles. - Unit tests (client, XML, sync, search), native integration tests against navidrome and gonic, and CI (`unit`, `lint`, `integration`) with warm caches. -## Phase 2 - UI vertical slice (next) +## Phase 2 - UI vertical slice (done) Goal: a usable app for browsing a server, end to end. -- DI wiring (Koin) and DataStore-backed settings. -- App shell: root navigation and a Material 3 theme with cover-art tonal colour extraction. -- Source setup: add/edit sources (URL and credentials, token auth), ping/test, trigger a sync, show progress. -- Library browsing: artists, albums and songs with Paging 3 and Coil 3 cover art, reading from Room. -- Roborazzi screenshot tests and goldens for the new screens. +- Koin DI and DataStore-backed preferences (album sort); the unused `app_settings` Room table was removed in favour of DataStore. +- App shell: a root gate that shows the add-server flow until a server exists, then the tabbed library screen matching the Flutter app (icon-only tabs for albums, artists, songs and playlists, with sync and settings actions), plus album and playlist detail screens and a monochrome theme. +- Source setup: name/address/username/password with token-auth toggle, a connection test, save-and-sync, plus server switching and removal in settings. +- Library browsing: albums (covers-only grid), artists, songs and playlists paged from Room (`PagingSource`) and rendered with `LazyPagingItems`, with Coil cover art served from stable media URLs. +- Roborazzi screenshot goldens for the eight new screens. + +Not done yet in this slice (deliberately): artist detail, cover-art tonal colour extraction, long-press menus, editing an existing server, and per-section sort/filter controls (only album sort is wired). ## Phase 3 - Playback diff --git a/tools/preview_server.py b/tools/preview_server.py index b6594e71..103e49bb 100644 --- a/tools/preview_server.py +++ b/tools/preview_server.py @@ -42,6 +42,10 @@ class Handler(http.server.SimpleHTTPRequestHandler): def __init__(self, *args, **kwargs): super().__init__(*args, directory=ROOT, **kwargs) + def end_headers(self): + self.send_header("Cache-Control", "no-store, max-age=0") + super().end_headers() + def do_GET(self): path = self.translate_path(self.path) if os.path.isdir(path):