Merge branch 'develop' into GT-2980-No-Lessons-UI

Author: tjohnson009

.claude/skills/pr-review/SKILL.md

@@ -38,7 +38,34 @@ If it fails, report as **Must Fix** before reviewing anything else.
 
 7. Output a structured review (format below).
 
-8. After the review output, print:
+8. Post inline comments to the PR for every ⚠️ and ❌ finding that references a specific file and line number. Before posting, deduplicate against all existing comments (resolved or not) to avoid re-posting anything already raised:
+
+```bash
+# Get the head SHA, repo, and all existing review comments (resolved and unresolved)
+HEAD_SHA=$(gh pr view $ARGUMENTS --json headRefOid -q .headRefOid)
+REPO=$(gh repo view --json nameWithOwner -q .nameWithOwner)
+EXISTING=$(gh api repos/$REPO/pulls/$ARGUMENTS/comments --jq '[.[] | select(.in_reply_to_id == null) | {path:.path, line:.line, body:.body}]')
+```
+
+For each finding, check whether any existing comment (resolved or not) already covers the same file + line (or contains substantially the same text). Skip any finding that is already covered. Then bundle the remaining new comments into a single review submission:
+
+```bash
+gh api repos/$REPO/pulls/$ARGUMENTS/reviews \
+  --method POST \
+  --field commit_id="$HEAD_SHA" \
+  --field event="COMMENT" \
+  --field "comments[][path]=<file path>" \
+  --field "comments[][line]=<line number>" \
+  --field "comments[][side]=RIGHT" \
+  --field "comments[][body]=<finding text>
+
+🤖 Posted by [Claude Code](https://claude.ai/code)" \
+  # repeat --field "comments[]..." for each new finding
+```
+
+Use the exact file path from the diff and the line number in the current version of the file (RIGHT side). Each comment body should contain the full finding description. Always append the attribution footer `\n\n🤖 Posted by [Claude Code](https://claude.ai/code)` to each comment. If no new actionable findings exist (only ✅ items or all already commented), skip this step.
+
+9. After the review output, print:
 
 ```
 ---
@@ -92,6 +119,15 @@ To dismiss a finding so it won't appear in future reviews, say:
 - [ ] `UiEvent` is a `sealed interface` implementing `CircuitUiEvent`, marked `internal`
 - [ ] `UiState` and `UiEvent` defined as nested types inside the Presenter
 - [ ] `UiState` exposes `val eventSink: (UiEvent) -> Unit`
+- [ ] `eventSink` lambda passed directly when constructing `UiState` — not extracted to a local variable first:
+  ```kotlin
+  // Correct
+  return UiState(items = items) { event -> when (event) { … } }
+  // Wrong
+  val eventSink: (UiEvent) -> Unit = { … }
+  return UiState(items = items, eventSink = eventSink)
+  ```
+- [ ] `navigator.pop()` (or any navigation call) placed *inside* the `launch { }` block when it follows an async operation — `rememberCoroutineScope()` is canceled on composition disposal and can cancel an in-flight write before it completes
 - [ ] Presenter contains no UI logic — pure state/event handling
 
 **UI Composable**
@@ -130,6 +166,7 @@ To dismiss a finding so it won't appear in future reviews, say:
 ### Testing
 
 - [ ] Presenter tests use `presenterTestOf { }` (Circuit test API)
+- [ ] Interfaces with `@Composable` functions use a hand-written `Fake*` class in tests, not `mockk<>()` — mockposable delays Kotlin version uptake and is incompatible with newer compiler versions
 - [ ] Paparazzi tests extend `BasePaparazziTest` from `:ui:base` testFixtures with `@TestParameter` night/accessibility matrix
 - [ ] Snapshots not recorded locally — triggered via GitHub Actions workflow on the feature branch
 
@@ -190,4 +227,50 @@ When the user says `dismiss: <title> — <reason>` (in any form — "dismiss the
 **Dismissed by**: <git user.name>
 ```
 
-4. Confirm to the user what was added and that it will be suppressed in future reviews.
+4. If the current session reviewed a PR, find any open (unresolved) comment thread on that PR matching the dismissed issue. Use the GraphQL API to locate threads and resolve the matching one, replying with the dismissal reason first:
+
+```bash
+REPO=$(gh repo view --json nameWithOwner -q .nameWithOwner)
+OWNER=${REPO%%/*}
+REPONAME=${REPO##*/}
+
+# Find unresolved review threads
+gh api graphql -f query="
+{
+  repository(owner: \"$OWNER\", name: \"$REPONAME\") {
+    pullRequest(number: $PR_NUMBER) {
+      reviewThreads(first: 100) {
+        nodes {
+          id
+          isResolved
+          comments(first: 1) {
+            nodes { id body path line }
+          }
+        }
+      }
+    }
+  }
+}"
+```
+
+Match the thread by file path, line number, or substantial text overlap with the dismissed finding. Then reply to the thread and resolve it:
+
+```bash
+# Reply to the thread's first comment explaining the dismissal
+gh api repos/$REPO/pulls/$PR_NUMBER/comments \
+  --method POST \
+  --field in_reply_to=<comment_id> \
+  --field body="Dismissed: <reason given by user>
+
+🤖 [Claude Code](https://claude.ai/code)"
+
+# Resolve the thread via GraphQL
+gh api graphql -f query="
+mutation {
+  resolveReviewThread(input: { threadId: \"<thread_node_id>\" }) {
+    thread { id isResolved }
+  }
+}"
+```
+
+5. Confirm to the user what was added and that it will be suppressed in future reviews.

README.md

@@ -3,10 +3,144 @@ GodTools Android
 
 [![codecov](https://codecov.io/gh/CruGlobal/godtools-android/branch/develop/graph/badge.svg)](https://codecov.io/gh/CruGlobal/godtools-android)
 
-# Git LFS
+GodTools is a mobile discipleship app built by [Cru](https://www.cru.org/). This repository contains the Android client.
 
-We use [Git LFS](https://git-lfs.com/) for storing Paparazzi snapshots. You will need to setup [Git LFS](https://git-lfs.com/) on your local machine in order to store new paparazzi snapshots or validate existing paparazzi snapshots.
+## Requirements
 
-# Crowdin
+- **JDK 21** (Temurin) — version specified in `.tool-versions`
+- **Android SDK** — compile/target SDK 36, minimum SDK 24
+- **Git LFS** — required for Paparazzi snapshot files
 
-To enable Crowdin translation downloads/uploads, install the [Crowdin CLI tool](https://crowdin.github.io/crowdin-cli/)
+## Getting Started
+
+```bash
+# Clone the repo (Git LFS must be installed first)
+git clone https://github.com/CruGlobal/godtools-android.git
+```
+
+### Git LFS
+
+Paparazzi snapshot images are stored in [Git LFS](https://git-lfs.com/). Install it before cloning or working with snapshot tests:
+
+```bash
+brew install git-lfs   # macOS
+git lfs install
+```
+
+## Build & Development
+
+```bash
+# Build the app bundle
+./gradlew bundle
+
+# Run unit tests (all variants)
+./gradlew test
+
+# Run tests for a specific module
+./gradlew :library:model:test
+./gradlew :app:test
+
+# Verify Paparazzi snapshot tests (requires Git LFS)
+./gradlew verifyPaparazzi
+
+# Run ktlint (code style checks)
+./gradlew :build-logic:ktlintCheck ktlintCheck
+
+# Run Android lint
+./gradlew lint
+```
+
+> **Paparazzi Snapshots:** Do not record snapshots locally. Trigger the manual GitHub Actions workflow on your feature branch to generate and commit updated screenshots.
+
+## Project Structure
+
+```
+app/                  # Main application module
+build-logic/          # Gradle convention plugins
+feature/              # Google Play Dynamic Feature modules
+library/              # Core non-UI modules
+ui/                   # UI and tool-renderer modules
+```
+
+### `app/`
+Main application module — `GodToolsApplication` (Hilt entry point), `DashboardActivity`, Dagger modules in `dagger/`.
+
+### `library/`
+| Module | Purpose |
+|---|---|
+| `model` | Data models and JSON:API type definitions |
+| `db` | Room database, DAOs, and repositories |
+| `api` | Retrofit REST and Scarlet WebSocket clients |
+| `sync` | WorkManager-based data synchronization |
+| `base` | Core utilities, settings, file system abstraction |
+| `account` | User authentication |
+| `analytics` | Event tracking |
+| `download-manager` | Tool download management |
+| `initial-content` | Bundled initial content |
+| `user-data` | User preferences and state |
+
+### `ui/`
+| Module | Purpose |
+|---|---|
+| `base` | Shared Compose components and Material 3 theme |
+| `base-tool` | Base tool rendering infrastructure |
+| `tract-renderer` | Tract tool renderer |
+| `lesson-renderer` | Lesson tool renderer |
+| `article-renderer` | Article tool renderer |
+| `article-aem-renderer` | AEM article tool renderer |
+| `cyoa-renderer` | Choose Your Own Adventure renderer |
+| `tips-renderer` | Tips renderer |
+| `tutorial-renderer` | Tutorial renderer |
+| `shortcuts` | App shortcuts UI |
+| `qr-code` | QR code feature UI |
+
+### `feature/`
+Google Play Dynamic Feature modules: `bundledcontent` (bundled initial content delivered via Play).
+
+### `build-logic/`
+Gradle convention plugins: `godtools.application-conventions`, `godtools.library-conventions`, `godtools.dynamic-feature-conventions`.
+
+## Architecture & Key Frameworks
+
+| Concern | Technology |
+|---|---|
+| Dependency injection | Hilt (Dagger 2) |
+| UI | Jetpack Compose + Material Design 3 |
+| Navigation / UI state | [Slack Circuit](https://slackhq.github.io/circuit/) |
+| Networking | Retrofit (REST), Scarlet (WebSocket), OkHttp |
+| Database | Room |
+| Background work | WorkManager |
+| Image loading | Coil |
+| Dependency versions | `gradle/libs.versions.toml` version catalog |
+
+**Circuit pattern:** Presenter/UI pairs annotated with `@CircuitInject`. Presenters are `@Composable` functions that return state objects; actions flow through `UiState.eventSink`.
+
+## Build Variants
+
+- **Product flavors** (`env` dimension): `stage` (staging API), `production` (production API)
+- **Build types**: `debug`, `qa` (inherits debug), `release`
+- The `stage` flavor is only available for `debug` and `qa` builds
+
+## Code Style
+
+Style is enforced by [ktlint](https://pinterest.github.io/ktlint/) with the `android_studio` code style.
+
+- Max line length: 120
+- Indent: 4 spaces
+
+Always run ktlint before committing:
+
+```bash
+./gradlew :build-logic:ktlintCheck ktlintCheck
+```
+
+## Translations (Crowdin)
+
+Strings are managed via [Crowdin](https://crowdin.com/). To push/pull translations, install the [Crowdin CLI](https://crowdin.github.io/crowdin-cli/) and set the `CROWDIN_API_TOKEN` environment variable.
+
+## Contributing
+
+1. Branch from `develop` and open PRs against `develop` (the default branch).
+2. Add unit tests for any new functionality.
+3. Run ktlint and unit tests before opening a PR.
+4. For UI changes, trigger the Paparazzi snapshot workflow on your branch rather than recording locally.

app/build.gradle.kts

@@ -46,6 +46,7 @@ android {
             applicationIdSuffix = ".stage"
 
             buildConfigField("String", "MOBILE_CONTENT_API", "\"$URI_MOBILE_CONTENT_API_STAGE\"")
+            buildConfigField("String", "MOBILE_CONTENT_CDN", "\"https://mobilecontent-stage.cru.org\"")
 
             // Facebook
             resValue("string", "facebook_app_id", "448969905944197")
@@ -61,6 +62,7 @@ android {
         }
         named("production") {
             buildConfigField("String", "MOBILE_CONTENT_API", "\"$URI_MOBILE_CONTENT_API_PRODUCTION\"")
+            buildConfigField("String", "MOBILE_CONTENT_CDN", "\"https://mobilecontent.cru.org\"")
 
             // Facebook
             resValue("string", "facebook_app_id", "2236701616451487")

app/src/main/kotlin/org/cru/godtools/dagger/ConfigModule.kt

@@ -4,14 +4,15 @@ import dagger.Module
 import dagger.Provides
 import dagger.hilt.InstallIn
 import dagger.hilt.components.SingletonComponent
-import javax.inject.Named
 import org.cru.godtools.BuildConfig
-import org.cru.godtools.api.ApiModule
+import org.cru.godtools.api.ApiConfig
 
 @Module
 @InstallIn(SingletonComponent::class)
 object ConfigModule {
     @get:Provides
-    @get:Named(ApiModule.MOBILE_CONTENT_API_URL)
-    val mobileContentApiBaseUrl = BuildConfig.MOBILE_CONTENT_API
+    val apiConfig = ApiConfig(
+        mobileContentApiUrl = BuildConfig.MOBILE_CONTENT_API,
+        cdnUrl = BuildConfig.MOBILE_CONTENT_CDN
+    )
 }

app/src/main/kotlin/org/cru/godtools/ui/dashboard/LocalizationSettingsBox.kt

@@ -30,12 +30,11 @@ internal fun LocalizationSettingsBox(
         Column(modifier = Modifier.padding(16.dp)) {
             Text(
                 text = stringResource(title),
-                fontWeight = FontWeight.Bold,
-                style = MaterialTheme.typography.bodyLarge
+                style = MaterialTheme.typography.titleMedium
             )
             Text(
                 text = stringResource(description),
-                style = MaterialTheme.typography.bodySmall
+                style = MaterialTheme.typography.bodyMedium
             )
             Button(
                 onClick = onClickSettings,

app/src/main/kotlin/org/cru/godtools/ui/dashboard/lessons/LessonsLayout.kt

@@ -29,23 +29,18 @@ import org.cru.godtools.base.ui.circuit.screen.dashboard.page.LessonsScreen
 import org.cru.godtools.ui.dashboard.LocalizationSettingsBox
 import org.cru.godtools.ui.dashboard.lessons.LessonsPresenter.UiEvent
 import org.cru.godtools.ui.dashboard.lessons.LessonsPresenter.UiState
-import org.cru.godtools.ui.dashboard.rememberPinLastItemBottomArrangement
+import org.cru.godtools.ui.dashboard.personalization.rememberFloatLastItemsToBottomArrangement
 import org.cru.godtools.ui.tools.LessonToolCard
 
 internal val MARGIN_LESSONS_LAYOUT_HORIZONTAL = 16.dp
 
 @Composable
 @CircuitInject(LessonsScreen::class, SingletonComponent::class)
 internal fun LessonsLayout(state: UiState, modifier: Modifier = Modifier) {
-    val pinLastItem = rememberPinLastItemBottomArrangement()
-    val verticalArrangement = if (state.mode == UiState.Mode.PERSONALIZATION) {
-        pinLastItem
-    } else {
-        Arrangement.Top
-    }
-
     LazyColumn(
-        verticalArrangement = verticalArrangement,
+        verticalArrangement = rememberFloatLastItemsToBottomArrangement(
+            numToFloat = if (state.mode == UiState.Mode.PERSONALIZATION) 1 else 0
+        ),
         modifier = modifier.fillMaxHeight()
     ) {
         if (state.isPersonalizationEnabled) {

app/src/main/kotlin/org/cru/godtools/ui/dashboard/personalization/FloatLastItemsToBottomArrangement.kt

@@ -0,0 +1,28 @@
+package org.cru.godtools.ui.dashboard.personalization
+
+import androidx.compose.foundation.layout.Arrangement
+import androidx.compose.runtime.Composable
+import androidx.compose.runtime.remember
+import androidx.compose.ui.unit.Density
+
+@Composable
+internal fun rememberFloatLastItemsToBottomArrangement(numToFloat: Int) =
+    remember(numToFloat) { FloatLastItemsToBottomArrangement(numToFloat) }
+
+internal class FloatLastItemsToBottomArrangement(val numToFloat: Int) : Arrangement.Vertical {
+    override fun Density.arrange(totalSize: Int, sizes: IntArray, outPositions: IntArray) {
+        var currentOffset = 0
+        sizes.forEachIndexed { index, size ->
+            outPositions[index] = currentOffset
+            currentOffset += size
+        }
+
+        if (currentOffset < totalSize && numToFloat > 0) {
+            currentOffset = totalSize
+            sizes.takeLast(numToFloat).reversed().forEachIndexed { index, size ->
+                currentOffset -= size
+                outPositions[sizes.lastIndex - index] = currentOffset
+            }
+        }
+    }
+}

app/src/main/kotlin/org/cru/godtools/ui/dashboard/tools/ToolsLayout.kt

@@ -32,7 +32,7 @@ import org.cru.godtools.R
 import org.cru.godtools.base.ui.circuit.screen.dashboard.page.ToolsScreen
 import org.cru.godtools.ui.banner.Banners
 import org.cru.godtools.ui.dashboard.LocalizationSettingsBox
-import org.cru.godtools.ui.dashboard.rememberPinLastItemBottomArrangement
+import org.cru.godtools.ui.dashboard.personalization.rememberFloatLastItemsToBottomArrangement
 import org.cru.godtools.ui.dashboard.tools.ToolsPresenter.UiEvent
 import org.cru.godtools.ui.dashboard.tools.ToolsPresenter.UiState
 import org.cru.godtools.ui.tools.SquareToolCard
@@ -58,7 +58,9 @@ internal fun ToolsLayout(state: UiState, modifier: Modifier = Modifier) {
 
     LazyColumn(
         state = columnState,
-        verticalArrangement = verticalArrangement,
+        verticalArrangement = rememberFloatLastItemsToBottomArrangement(
+            numToFloat = if (state.mode == UiState.Mode.PERSONALIZATION) 1 else 0
+        ),
         modifier = modifier.fillMaxHeight()
     ) {
         if (!state.dataLoaded) return@LazyColumn