Fix bugs from static analysis; add test coverage for new code

Bugs fixed:
- LogbookStorage.copyFromUri: return null when ContentResolver returns null
  stream instead of returning a path to an unwritten file
- VoiceLogFragment adapter: decode photos as downsampled thumbnails
  (128x96 target) using BitmapFactory inSampleSize instead of loading
  full-resolution bitmaps on the main thread

Tests added:
- LogbookRepositoryTest: ID sequencing, insertion order, storage delegation,
  reload from persisted state, lat/lon preservation
- TripReportGeneratorTest: log entry time-range filtering, distance
  calculation, empty-points edge case, all narrative styles smoke-tested

https://claude.ai/code/session_01YUbuZNDAoLea4cf9UGQ9qn

4 files changed, 172 insertions, 4 deletions

diff --git a/android-app/app/src/main/kotlin/org/terst/nav/logbook/LogbookStorage.kt b/android-app/app/src/main/kotlin/org/terst/nav/logbook/LogbookStorage.kt
index 35349f0..6b90542 100644
--- a/android-app/app/src/main/kotlin/org/terst/nav/logbook/LogbookStorage.kt
+++ b/android-app/app/src/main/kotlin/org/terst/nav/logbook/LogbookStorage.kt
@@ -44,9 +44,8 @@ class LogbookStorage(private val context: Context) {
 
     fun copyFromUri(uri: Uri): String? = runCatching {
         val dest = File(photoDir, "log_${System.currentTimeMillis()}.jpg")
-        context.contentResolver.openInputStream(uri)?.use { input ->
-            FileOutputStream(dest).use { output -> input.copyTo(output) }
-        }
+        val stream = context.contentResolver.openInputStream(uri) ?: return null
+        stream.use { input -> FileOutputStream(dest).use { output -> input.copyTo(output) } }
         dest.absolutePath
     }.getOrElse { null }
 }
diff --git a/android-app/app/src/main/kotlin/org/terst/nav/ui/voicelog/VoiceLogFragment.kt b/android-app/app/src/main/kotlin/org/terst/nav/ui/voicelog/VoiceLogFragment.kt
index 31d9715..3638f8f 100644
--- a/android-app/app/src/main/kotlin/org/terst/nav/ui/voicelog/VoiceLogFragment.kt
+++ b/android-app/app/src/main/kotlin/org/terst/nav/ui/voicelog/VoiceLogFragment.kt
@@ -368,6 +368,16 @@ class VoiceLogFragment : Fragment() {
     }
 }
 
+// ── Helpers ────────────────────────────────────────────────────────────────────
+
+private fun decodeThumbnail(path: String, targetW: Int, targetH: Int): android.graphics.Bitmap? {
+    val opts = BitmapFactory.Options().apply { inJustDecodeBounds = true }
+    BitmapFactory.decodeFile(path, opts)
+    if (opts.outWidth <= 0) return null
+    val scale = maxOf(opts.outWidth / targetW, opts.outHeight / targetH).coerceAtLeast(1)
+    return BitmapFactory.decodeFile(path, BitmapFactory.Options().apply { inSampleSize = scale })
+}
+
 // ── Logbook entries adapter ────────────────────────────────────────────────────
 
 private val TIME_FMT: DateTimeFormatter =
@@ -409,11 +419,12 @@ private class LogbookEntryAdapter : RecyclerView.Adapter<LogbookEntryAdapter.VH>
 
         val photoPath = e.photoPath
         if (photoPath != null && File(photoPath).exists()) {
-            val bm = BitmapFactory.decodeFile(photoPath)
+            val bm = decodeThumbnail(photoPath, 128, 96)
             if (bm != null) {
                 holder.ivPhoto.setImageBitmap(bm)
                 holder.ivPhoto.visibility = View.VISIBLE
             } else {
+                holder.ivPhoto.setImageDrawable(null)
                 holder.ivPhoto.visibility = View.GONE
             }
         } else {
diff --git a/android-app/app/src/test/kotlin/org/terst/nav/logbook/LogbookRepositoryTest.kt b/android-app/app/src/test/kotlin/org/terst/nav/logbook/LogbookRepositoryTest.kt
new file mode 100644
index 0000000..1161065
--- /dev/null
+++ b/android-app/app/src/test/kotlin/org/terst/nav/logbook/LogbookRepositoryTest.kt
@@ -0,0 +1,69 @@
+package org.terst.nav.logbook
+
+import io.mockk.every
+import io.mockk.mockk
+import io.mockk.slot
+import io.mockk.verify
+import org.junit.Assert.assertEquals
+import org.junit.Assert.assertNotEquals
+import org.junit.Assert.assertTrue
+import org.junit.Before
+import org.junit.Test
+
+class LogbookRepositoryTest {
+
+    private lateinit var storage: LogbookStorage
+    private lateinit var repo: LogbookRepository
+
+    @Before fun setUp() {
+        storage = mockk(relaxed = true)
+        every { storage.loadAll() } returns emptyList()
+        repo = LogbookRepository(storage)
+    }
+
+    @Test fun `save assigns unique sequential ids`() {
+        val a = repo.save(entry("a"))
+        val b = repo.save(entry("b"))
+        assertNotEquals(a.id, b.id)
+        assertEquals(a.id + 1, b.id)
+    }
+
+    @Test fun `getAll returns entries in insertion order`() {
+        repo.save(entry("first"))
+        repo.save(entry("second"))
+        assertEquals(listOf("first", "second"), repo.getAll().map { it.text })
+    }
+
+    @Test fun `save calls storage saveAll`() {
+        repo.save(entry("stored"))
+        verify { storage.saveAll(any()) }
+    }
+
+    @Test fun `repository loads initial entries from storage on init`() {
+        val preloaded = entry("preloaded").copy(id = 99L)
+        every { storage.loadAll() } returns listOf(preloaded)
+        val repo2 = LogbookRepository(storage)
+        assertTrue(repo2.getAll().any { it.text == "preloaded" })
+    }
+
+    @Test fun `ids continue from max stored id after reload`() {
+        every { storage.loadAll() } returns listOf(entry("old").copy(id = 50L))
+        val repo2 = LogbookRepository(storage)
+        val saved = repo2.save(entry("new"))
+        assertEquals(51L, saved.id)
+    }
+
+    @Test fun `saved entry inherits text and location`() {
+        val e = entry("test note").copy(lat = 19.669, lon = -156.027)
+        val saved = repo.save(e)
+        assertEquals("test note", saved.text)
+        assertEquals(19.669, saved.lat!!, 0.0001)
+        assertEquals(-156.027, saved.lon!!, 0.0001)
+    }
+
+    private fun entry(text: String) = LogEntry(
+        timestampMs = System.currentTimeMillis(),
+        text = text,
+        entryType = EntryType.GENERAL
+    )
+}
diff --git a/android-app/app/src/test/kotlin/org/terst/nav/tripreport/TripReportGeneratorTest.kt b/android-app/app/src/test/kotlin/org/terst/nav/tripreport/TripReportGeneratorTest.kt
new file mode 100644
index 0000000..eea3234
--- /dev/null
+++ b/android-app/app/src/test/kotlin/org/terst/nav/tripreport/TripReportGeneratorTest.kt
@@ -0,0 +1,89 @@
+package org.terst.nav.tripreport
+
+import org.junit.Assert.assertEquals
+import org.junit.Assert.assertTrue
+import org.junit.Test
+import org.terst.nav.logbook.EntryType
+import org.terst.nav.logbook.LogEntry
+import org.terst.nav.track.TrackPoint
+
+class TripReportGeneratorTest {
+
+    private val gen = TripReportGenerator()
+
+    private fun pt(lat: Double, lon: Double, sog: Double = 5.0, ts: Long) =
+        TrackPoint(lat = lat, lon = lon, sogKnots = sog, cogDeg = 0.0, timestampMs = ts)
+
+    private fun entry(ts: Long, text: String = "note") = LogEntry(
+        timestampMs = ts, text = text, entryType = EntryType.GENERAL
+    )
+
+    // ── generateSummary ───────────────────────────────────────────────────────
+
+    @Test
+    fun `empty points returns zero summary`() {
+        val s = gen.generateSummary(emptyList(), emptyList())
+        assertEquals(0.0, s.distanceNm, 0.0)
+        assertEquals(0.0, s.maxSogKts, 0.0)
+        assertTrue(s.logEntries.isEmpty())
+    }
+
+    @Test
+    fun `distance between two points one nm apart`() {
+        // 1 nm north along prime meridian ≈ 0.01667° latitude
+        val points = listOf(pt(0.0, 0.0, ts = 0L), pt(0.016667, 0.0, ts = 60_000L))
+        val s = gen.generateSummary(points, emptyList())
+        assertEquals(1.0, s.distanceNm, 0.02)
+    }
+
+    @Test
+    fun `max sog picks highest value`() {
+        val points = listOf(pt(0.0, 0.0, sog = 3.0, ts = 0L), pt(0.0, 0.0, sog = 8.5, ts = 1000L))
+        val s = gen.generateSummary(points, emptyList())
+        assertEquals(8.5, s.maxSogKts, 0.001)
+    }
+
+    @Test
+    fun `log entries within track time range are included`() {
+        val points = listOf(pt(0.0, 0.0, ts = 1000L), pt(0.0, 0.0, ts = 5000L))
+        val logs = listOf(entry(500L, "before"), entry(1000L, "start"), entry(3000L, "middle"), entry(5000L, "end"), entry(6000L, "after"))
+        val s = gen.generateSummary(points, logs)
+        assertEquals(listOf("start", "middle", "end"), s.logEntries.map { it.text })
+    }
+
+    @Test
+    fun `log entries outside track time range are excluded`() {
+        val points = listOf(pt(0.0, 0.0, ts = 10_000L), pt(0.0, 0.0, ts = 20_000L))
+        val logs = listOf(entry(5_000L, "too early"), entry(25_000L, "too late"))
+        val s = gen.generateSummary(points, logs)
+        assertTrue(s.logEntries.isEmpty())
+    }
+
+    @Test
+    fun `start and end times match first and last point`() {
+        val points = listOf(pt(0.0, 0.0, ts = 1234L), pt(0.0, 0.0, ts = 9876L))
+        val s = gen.generateSummary(points, emptyList())
+        assertEquals(1234L, s.startTimeMs)
+        assertEquals(9876L, s.endTimeMs)
+    }
+
+    // ── generateNarrative ─────────────────────────────────────────────────────
+
+    @Test
+    fun `professional narrative contains distance and speed`() {
+        val points = listOf(pt(0.0, 0.0, sog = 5.0, ts = 0L), pt(0.016667, 0.0, sog = 5.0, ts = 3_600_000L))
+        val s = gen.generateSummary(points, emptyList())
+        val narrative = gen.generateNarrative(s, NarrativeStyle.PROFESSIONAL)
+        assertTrue("Expected distance in narrative", narrative.contains("NM") || narrative.contains("nm"))
+    }
+
+    @Test
+    fun `all narrative styles produce non-empty output`() {
+        val points = listOf(pt(0.0, 0.0, ts = 0L), pt(0.016667, 0.0, ts = 3_600_000L))
+        val s = gen.generateSummary(points, listOf(entry(1_000L)))
+        for (style in NarrativeStyle.values()) {
+            val narrative = gen.generateNarrative(s, style)
+            assertTrue("Narrative for $style should not be blank", narrative.isNotBlank())
+        }
+    }
+}