diff --git a/.gitignore b/.gitignore index 3b4ea3e..42414bb 100644 --- a/.gitignore +++ b/.gitignore @@ -25,6 +25,7 @@ md-backup/ .claude # Local/generated docs +/docs/future-improvements-exploration.md /docs/main-activity-modularisation-checklist.md /docs/reader-image-page-navigation-checklist.md /docs/performance-hindrances.md diff --git a/app/src/androidTest/java/com/kairo/reader/data/books/LargeChapterDeviceTest.kt b/app/src/androidTest/java/com/kairo/reader/data/books/LargeChapterDeviceTest.kt new file mode 100644 index 0000000..bb3d9be --- /dev/null +++ b/app/src/androidTest/java/com/kairo/reader/data/books/LargeChapterDeviceTest.kt @@ -0,0 +1,108 @@ +package com.kairo.reader.data.books + +import android.net.Uri +import android.util.Log +import androidx.room.Room +import androidx.test.ext.junit.runners.AndroidJUnit4 +import androidx.test.platform.app.InstrumentationRegistry +import com.kairo.reader.core.dispatchers.DefaultDispatcherProvider +import com.kairo.reader.data.local.BookEntity +import com.kairo.reader.data.local.ChapterEntity +import com.kairo.reader.data.local.KairoDatabase +import com.kairo.reader.data.search.LibrarySearchRepositoryImpl +import java.io.File +import kotlinx.coroutines.runBlocking +import org.junit.Assert.assertEquals +import org.junit.Assert.assertNull +import org.junit.Test +import org.junit.runner.RunWith + +@RunWith(AndroidJUnit4::class) +class LargeChapterDeviceTest { + @Test + fun importedLargeTextCanBeReadBackWithoutLosingContent() = runBlocking { + val context = InstrumentationRegistry.getInstrumentation().targetContext + val database = Room.inMemoryDatabaseBuilder(context, KairoDatabase::class.java).build() + val source = File.createTempFile("large-chapter-regression-", ".txt", context.cacheDir) + try { + val dispatchers = DefaultDispatcherProvider() + val repository = BookRepositoryImpl( + database.bookDao(), + database.epubNavigationDao(), + listOf(TextFileBookParser(dispatchers)), + WebArticleExtractor(dispatchers), + context, + dispatchers, + ) + source.writeText("A reader should be able to open the entire imported book.\n\n".repeat(25_000)) + val imported = repository.importBook(Uri.fromFile(source)).book + val expected = imported.chapters.single() + Log.i( + "ReaderRegression", + "Imported ${source.length()} bytes; chapter row text bytes=" + + (expected.htmlContent.toByteArray().size + expected.plainText.toByteArray().size) + ) + val restored = repository.getChapter(imported.id, expected.index) + assertEquals(expected.plainText, restored.plainText) + assertEquals(expected.htmlContent, restored.htmlContent) + } finally { + database.close() + source.delete() + } + } + + @Test + fun existingLargeUnicodeChapterCanBeReadSearchedAndComparedWithoutChangingCoordinates() = runBlocking { + val context = InstrumentationRegistry.getInstrumentation().targetContext + val database = Room.inMemoryDatabaseBuilder(context, KairoDatabase::class.java).build() + try { + val prefix = "𐐀é漢🙂 ".repeat(160_000) + "\u0000" + val plainText = prefix + "needle at the end" + val chapter = ChapterEntity("large", 7, "Large chapter", "", plainText) + database.bookDao().insertBook( + BookEntity("large", "Large legacy book", emptyList(), "en", null), + listOf(chapter), + emptyList(), + ) + assertEquals(chapter, database.bookDao().getChapter("large", 7)) + assertEquals(listOf(chapter), database.bookDao().getChaptersWithContent("large")) + assertEquals(plainText, database.epubNavigationDao().getChapterCoordinates("large").single().plainText) + assertEquals(chapter.htmlContent, database.epubNavigationDao().getChapterHtmlContent("large", 7)) + assertEquals( + chapter.htmlContent, + database.epubNavigationDao().getMarkerlessNavigationCandidates("large", "absent-marker", 5_000_000, 1) + .single().htmlContent, + ) + val search = LibrarySearchRepositoryImpl( + database.searchDao(), + database.savedAnnotationDao(), + DefaultDispatcherProvider(), + ).search("needle at the end", "large").single() + assertEquals(7, search.chapterIndex) + assertEquals(prefix.codePointCount(0, prefix.length), search.matchStartCodePointOffset) + assertNull(database.bookDao().getChapter("large", 8)) + } finally { + database.close() + } + } + + @Test + fun emptyAndExactChunkBoundaryBodiesRoundTrip() = runBlocking { + val context = InstrumentationRegistry.getInstrumentation().targetContext + val database = Room.inMemoryDatabaseBuilder(context, KairoDatabase::class.java).build() + try { + val chapters = listOf( + ChapterEntity("boundary", 0, null, "", ""), + ChapterEntity("boundary", 1, null, "a".repeat(128 * 1024), "é".repeat(64 * 1024)), + ) + database.bookDao().insertBook( + BookEntity("boundary", "Boundary book", emptyList(), "en", null), + chapters, + emptyList(), + ) + assertEquals(chapters, database.bookDao().getChaptersWithContent("boundary")) + } finally { + database.close() + } + } +} diff --git a/app/src/androidTest/java/com/kairo/reader/ui/navigation/ReaderPositionDeviceTest.kt b/app/src/androidTest/java/com/kairo/reader/ui/navigation/ReaderPositionDeviceTest.kt new file mode 100644 index 0000000..013f44d --- /dev/null +++ b/app/src/androidTest/java/com/kairo/reader/ui/navigation/ReaderPositionDeviceTest.kt @@ -0,0 +1,142 @@ +package com.kairo.reader.ui.navigation + +import android.net.Uri +import android.util.Log +import androidx.compose.material3.Button +import androidx.compose.material3.Text +import androidx.compose.runtime.mutableStateOf +import androidx.compose.ui.test.junit4.v2.createAndroidComposeRule +import androidx.compose.ui.test.onNodeWithContentDescription +import androidx.compose.ui.test.onNodeWithText +import androidx.compose.ui.test.performClick +import androidx.lifecycle.ViewModelProvider +import androidx.navigation.NavHostController +import androidx.navigation.compose.NavHost +import androidx.navigation.compose.composable +import androidx.navigation.compose.rememberNavController +import androidx.test.ext.junit.runners.AndroidJUnit4 +import com.kairo.reader.KairoApplication +import com.kairo.reader.R +import com.kairo.reader.TestActivity +import com.kairo.reader.core.model.BookId +import com.kairo.reader.core.model.UserPreferences +import com.kairo.reader.ui.reader.ReaderViewModel +import com.kairo.reader.ui.theme.KairoTheme +import java.io.File +import java.util.UUID +import java.util.zip.ZipEntry +import java.util.zip.ZipOutputStream +import kotlinx.coroutines.runBlocking +import org.junit.Assert.assertEquals +import org.junit.Assert.assertTrue +import org.junit.Rule +import org.junit.Test +import org.junit.runner.RunWith + +/** Exercises the real reader route, using only a uniquely named disposable book. */ +@RunWith(AndroidJUnit4::class) +class ReaderPositionDeviceTest { + @get:Rule + val compose = createAndroidComposeRule() + + @Test + fun leavingAfterAChapterChangeRestoresTheVisiblePosition() = runBlocking { + val container = compose.activity.application as KairoApplication + val source = createBook(container.cacheDir) + var bookId: BookId? = null + val visible = mutableStateOf(true) + try { + val book = container.libraryRepository.import(Uri.fromFile(source)).book + bookId = book.id + assertEquals(2, book.chapters.size) + lateinit var nav: NavHostController + compose.setContent { + KairoTheme { + if (visible.value) { + nav = rememberNavController() + NavHost(navController = nav, startDestination = KairoRoutes.LIBRARY) { + composable(KairoRoutes.LIBRARY) { + Button(onClick = { nav.navigate(KairoRoutes.reader(book.id.value)) }) { + Text("Open regression book") + } + } + readerDestinations( + ReaderDestinationDependencies( + container, nav, UserPreferences(), 250, false, null, {}, {}, {}, {}, + ), + ) + } + } + } + } + compose.onNodeWithText("Open regression book").performClick() + compose.waitUntil(15_000) { nav.currentBackStackEntry?.destination?.route == KairoRoutes.READER } + val viewModel = compose.runOnIdle { + ViewModelProvider(requireNotNull(nav.currentBackStackEntry))[ReaderViewModel::class.java] + } + compose.waitUntil(15_000) { viewModel.uiState.value.chapterData != null } + val next = compose.activity.getString(R.string.content_desc_next_page) + var pageTurns = 0 + while (viewModel.uiState.value.chapterIndex == 0 && pageTurns < 30) { + compose.onNodeWithContentDescription(next).performClick() + compose.waitForIdle() + pageTurns++ + } + compose.waitUntil(15_000) { + viewModel.uiState.value.chapterIndex == 1 && viewModel.uiState.value.chapterData != null + } + assertTrue(pageTurns > 1) + val expected = viewModel.uiState.value.focusIndex + compose.onNodeWithContentDescription(compose.activity.getString(R.string.content_desc_go_to_library)) + .performClick() + compose.waitUntil(15_000) { nav.currentBackStackEntry?.destination?.route == KairoRoutes.LIBRARY } + val saved = requireNotNull(container.readingPositionRepository.getPosition(book.id)) + Log.i("ReaderRegression", "Chapter switch: visible=$expected, saved=${saved.tokenIndex}, chapter=${saved.chapterIndex}") + assertEquals(1, saved.chapterIndex) + assertEquals(expected, saved.tokenIndex) + compose.onNodeWithText("Open regression book").performClick() + compose.waitUntil(15_000) { nav.currentBackStackEntry?.destination?.route == KairoRoutes.READER } + val reopened = compose.runOnIdle { + ViewModelProvider(requireNotNull(nav.currentBackStackEntry))[ReaderViewModel::class.java] + } + compose.waitUntil(15_000) { reopened.uiState.value.chapterData != null } + assertEquals(expected, reopened.uiState.value.focusIndex) + } finally { + compose.runOnIdle { visible.value = false } + compose.waitForIdle() + bookId?.let { id -> + container.readingSessionCoordinator.finalizeReader(id) + container.readingSessionCoordinator.awaitIdle() + container.libraryRepository.delete(id.value) + } + source.delete() + } + } + + private fun createBook(directory: File): File { + val source = File.createTempFile("position-regression-", ".epub", directory) + ZipOutputStream(source.outputStream()).use { zip -> + zip.putNextEntry(ZipEntry("META-INF/container.xml")) + zip.write("".toByteArray()) + zip.closeEntry() + zip.putNextEntry(ZipEntry("content.opf")) + zip.write( + """ + Position regression ${UUID.randomUUID()}en + + + """.toByteArray(), + ) + zip.closeEntry() + listOf("one", "two").forEach { chapter -> + zip.putNextEntry(ZipEntry("$chapter.xhtml")) + val paragraphs = (1..60).joinToString("") { index -> + "

Paragraph $index in chapter $chapter. Reading should always resume at the visible word.

" + } + zip.write("Chapter $chapter$paragraphs".toByteArray()) + zip.closeEntry() + } + } + return source + } +} diff --git a/app/src/androidTest/java/com/kairo/reader/ui/reader/ReaderFocusNavigationDeviceTest.kt b/app/src/androidTest/java/com/kairo/reader/ui/reader/ReaderFocusNavigationDeviceTest.kt new file mode 100644 index 0000000..16f554a --- /dev/null +++ b/app/src/androidTest/java/com/kairo/reader/ui/reader/ReaderFocusNavigationDeviceTest.kt @@ -0,0 +1,72 @@ +package com.kairo.reader.ui.reader + +import android.util.Log +import androidx.compose.foundation.gestures.scrollBy +import androidx.compose.foundation.layout.height +import androidx.compose.foundation.lazy.LazyColumn +import androidx.compose.foundation.lazy.LazyListState +import androidx.compose.material3.Text +import androidx.compose.runtime.mutableIntStateOf +import androidx.compose.ui.Modifier +import androidx.compose.ui.test.assertIsDisplayed +import androidx.compose.ui.test.junit4.v2.createAndroidComposeRule +import androidx.compose.ui.test.onNodeWithText +import androidx.compose.ui.unit.dp +import androidx.test.ext.junit.runners.AndroidJUnit4 +import com.kairo.reader.TestActivity +import com.kairo.reader.ui.theme.KairoTheme +import kotlinx.coroutines.runBlocking +import org.junit.Assert.assertEquals +import org.junit.Rule +import org.junit.Test +import org.junit.runner.RunWith + +@RunWith(AndroidJUnit4::class) +class ReaderFocusNavigationDeviceTest { + @get:Rule + val compose = createAndroidComposeRule() + + @Test + fun navigationWithinTheSamePageScrollsToTheNewFocus() { + val focus = mutableIntStateOf(0) + lateinit var list: LazyListState + compose.setContent { + KairoTheme { + val holder = rememberReaderListState("same-chapter-and-page", focus.intValue, 20, false) + list = holder.listState + LazyColumn(Modifier.height(180.dp), state = list) { + items(20) { index -> Text("Paragraph $index", Modifier.height(60.dp)) } + } + } + } + compose.onNodeWithText("Paragraph 0").assertIsDisplayed() + compose.runOnIdle { focus.intValue = 12 } + compose.waitForIdle() + compose.runOnIdle { + Log.i("ReaderRegression", "Navigation focus=12, first visible paragraph=${list.firstVisibleItemIndex}") + assertEquals(12, list.firstVisibleItemIndex) + } + compose.onNodeWithText("Paragraph 12").assertIsDisplayed() + } + + @Test + fun focusingAnAlreadyVisibleParagraphDoesNotJumpTheViewport() { + val focus = mutableIntStateOf(0) + lateinit var list: LazyListState + compose.setContent { + KairoTheme { + list = rememberReaderListState("same-page", focus.intValue, 20, false).listState + LazyColumn(Modifier.height(180.dp), state = list) { + items(20) { index -> Text("Paragraph $index", Modifier.height(60.dp)) } + } + } + } + compose.runOnIdle { runBlocking { list.scrollBy(12f) } } + val before = compose.runOnIdle { list.firstVisibleItemIndex to list.firstVisibleItemScrollOffset } + compose.runOnIdle { focus.intValue = 1 } + compose.waitForIdle() + compose.runOnIdle { + assertEquals(before, list.firstVisibleItemIndex to list.firstVisibleItemScrollOffset) + } + } +} diff --git a/app/src/main/java/com/kairo/reader/data/local/BookDao.kt b/app/src/main/java/com/kairo/reader/data/local/BookDao.kt index 66d46d3..6bb6f7d 100644 --- a/app/src/main/java/com/kairo/reader/data/local/BookDao.kt +++ b/app/src/main/java/com/kairo/reader/data/local/BookDao.kt @@ -10,7 +10,7 @@ import com.kairo.reader.core.tokenization.CHAPTER_WORD_COUNT_VERSION import kotlinx.coroutines.flow.Flow @Dao -interface BookDao { +interface BookDao : ChapterContentQueries { @Transaction suspend fun insertBook( book: BookEntity, @@ -151,26 +151,6 @@ interface BookDao { @Query("SELECT languageTag FROM books WHERE id = :bookId LIMIT 1") suspend fun getBookLanguageTag(bookId: String): String? - @Query( - """ - SELECT bookId, `index`, title, '' AS htmlContent, '' AS plainText, imagePaths, wordCount, wordCountVersion - FROM chapters - WHERE bookId = :bookId - ORDER BY `index` - """, - ) - suspend fun getChapters(bookId: String): List - - @Query( - """ - SELECT bookId, `index`, title, htmlContent, plainText, imagePaths, wordCount, wordCountVersion - FROM chapters - WHERE bookId = :bookId - ORDER BY `index` - """, - ) - suspend fun getChaptersWithContent(bookId: String): List - @Query( """ SELECT bookId, entryIndex, label, depth, chapterIndex, characterOffset @@ -181,19 +161,6 @@ interface BookDao { ) suspend fun getTableOfContentsEntries(bookId: String): List - @Query( - """ - SELECT bookId, `index`, title, htmlContent, plainText, imagePaths, wordCount, wordCountVersion - FROM chapters - WHERE bookId = :bookId AND `index` = :index - LIMIT 1 - """, - ) - suspend fun getChapter( - bookId: String, - index: Int, - ): ChapterEntity? - @Query( """ UPDATE chapters diff --git a/app/src/main/java/com/kairo/reader/data/local/ChapterContentQueries.kt b/app/src/main/java/com/kairo/reader/data/local/ChapterContentQueries.kt new file mode 100644 index 0000000..0f5690b --- /dev/null +++ b/app/src/main/java/com/kairo/reader/data/local/ChapterContentQueries.kt @@ -0,0 +1,35 @@ +package com.kairo.reader.data.local + +import androidx.room.Query +import androidx.room.Transaction + +/** Metadata and bounded body reads share a transaction, including for existing imports. */ +interface ChapterContentQueries : ChapterTextQueries { + @Query( + """ + SELECT bookId, `index`, title, '' AS htmlContent, '' AS plainText, imagePaths, wordCount, wordCountVersion + FROM chapters + WHERE bookId = :bookId + ORDER BY `index` + """, + ) + suspend fun getChapters(bookId: String): List + + @Query( + """ + SELECT bookId, `index`, title, '' AS htmlContent, '' AS plainText, imagePaths, wordCount, wordCountVersion + FROM chapters + WHERE bookId = :bookId AND `index` = :index + LIMIT 1 + """, + ) + suspend fun getChapterMetadata(bookId: String, index: Int): ChapterEntity? + + @Transaction + suspend fun getChapter(bookId: String, index: Int): ChapterEntity? = + getChapterMetadata(bookId, index)?.let { withChapterContent(it) } + + @Transaction + suspend fun getChaptersWithContent(bookId: String): List = + getChapters(bookId).map { withChapterContent(it) } +} diff --git a/app/src/main/java/com/kairo/reader/data/local/ChapterTextQueries.kt b/app/src/main/java/com/kairo/reader/data/local/ChapterTextQueries.kt new file mode 100644 index 0000000..f7421e4 --- /dev/null +++ b/app/src/main/java/com/kairo/reader/data/local/ChapterTextQueries.kt @@ -0,0 +1,57 @@ +package com.kairo.reader.data.local + +import androidx.room.Query +import java.io.ByteArrayOutputStream +import kotlinx.coroutines.currentCoroutineContext +import kotlinx.coroutines.ensureActive + +data class ChapterTextChunk(val bytes: ByteArray?) + +/** Read chapter bodies below CursorWindow limits without changing stored reading coordinates. */ +interface ChapterTextQueries { + @Query( + """ + SELECT substr(CAST(htmlContent AS BLOB), :offset, :byteCount) AS bytes + FROM chapters WHERE bookId = :bookId AND `index` = :chapterIndex + """, + ) + suspend fun getChapterHtmlChunk(bookId: String, chapterIndex: Int, offset: Int, byteCount: Int): ChapterTextChunk? + + @Query( + """ + SELECT substr(CAST(plainText AS BLOB), :offset, :byteCount) AS bytes + FROM chapters WHERE bookId = :bookId AND `index` = :chapterIndex + """, + ) + suspend fun getChapterPlainTextChunk(bookId: String, chapterIndex: Int, offset: Int, byteCount: Int): ChapterTextChunk? +} + +internal suspend fun ChapterTextQueries.readChapterHtml(bookId: String, chapterIndex: Int): String? = + readChapterText { offset -> getChapterHtmlChunk(bookId, chapterIndex, offset, CHAPTER_TEXT_CHUNK_BYTES) } + +internal suspend fun ChapterTextQueries.readChapterPlainText(bookId: String, chapterIndex: Int): String? = + readChapterText { offset -> getChapterPlainTextChunk(bookId, chapterIndex, offset, CHAPTER_TEXT_CHUNK_BYTES) } + +internal suspend fun ChapterTextQueries.withChapterContent(chapter: ChapterEntity): ChapterEntity = + chapter.copy( + htmlContent = requireNotNull(readChapterHtml(chapter.bookId, chapter.index)), + plainText = requireNotNull(readChapterPlainText(chapter.bookId, chapter.index)), + ) + +private suspend fun readChapterText(readChunk: suspend (Int) -> ChapterTextChunk?): String? { + val bytes = ByteArrayOutputStream() + var offset = 1 // SQLite substr positions are one-based. + do { + currentCoroutineContext().ensureActive() + val row = readChunk(offset) ?: return null + // Android's SQLite bridge can expose an empty BLOB as null. The row wrapper + // distinguishes that valid empty body from a chapter that does not exist. + val chunk = row.bytes ?: byteArrayOf() + bytes.write(chunk) + offset += chunk.size + } while (chunk.size == CHAPTER_TEXT_CHUNK_BYTES) + // Decode after reassembly: a UTF-8 character may straddle a byte boundary. + return bytes.toString(Charsets.UTF_8.name()) +} + +private const val CHAPTER_TEXT_CHUNK_BYTES = 128 * 1024 diff --git a/app/src/main/java/com/kairo/reader/data/local/EpubNavigationDao.kt b/app/src/main/java/com/kairo/reader/data/local/EpubNavigationDao.kt index d30e13e..2eccfab 100644 --- a/app/src/main/java/com/kairo/reader/data/local/EpubNavigationDao.kt +++ b/app/src/main/java/com/kairo/reader/data/local/EpubNavigationDao.kt @@ -11,10 +11,10 @@ data class EpubNavigationChapterCandidate(val chapterIndex: Int, val title: Stri data class EpubChapterCoordinate(val chapterIndex: Int, val plainText: String,) @Dao -interface EpubNavigationDao { +interface EpubNavigationDao : ChapterTextQueries { @Query( """ - SELECT `index` AS chapterIndex, title, htmlContent + SELECT `index` AS chapterIndex, title, '' AS htmlContent FROM chapters WHERE bookId = :bookId AND instr(htmlContent, :canonicalMarker) = 0 @@ -24,35 +24,45 @@ interface EpubNavigationDao { LIMIT :limit """, ) - suspend fun getMarkerlessNavigationCandidates( + suspend fun getMarkerlessNavigationCandidateMetadata( bookId: String, canonicalMarker: String, maxHtmlCharacters: Int, limit: Int, ): List + @Transaction + suspend fun getMarkerlessNavigationCandidates( + bookId: String, + canonicalMarker: String, + maxHtmlCharacters: Int, + limit: Int, + ): List = + getMarkerlessNavigationCandidateMetadata(bookId, canonicalMarker, maxHtmlCharacters, limit).map { chapter -> + chapter.copy(htmlContent = requireNotNull(readChapterHtml(bookId, chapter.chapterIndex))) + } + @Query( """ - SELECT `index` AS chapterIndex, plainText + SELECT `index` AS chapterIndex, '' AS plainText FROM chapters WHERE bookId = :bookId ORDER BY `index` """, ) - suspend fun getChapterCoordinates(bookId: String): List + suspend fun getChapterCoordinateMetadata(bookId: String): List - @Query( - """ - SELECT htmlContent - FROM chapters - WHERE bookId = :bookId AND `index` = :chapterIndex - LIMIT 1 - """, - ) + @Transaction + suspend fun getChapterCoordinates(bookId: String): List = + getChapterCoordinateMetadata(bookId).map { chapter -> + chapter.copy(plainText = requireNotNull(readChapterPlainText(bookId, chapter.chapterIndex))) + } + + @Transaction suspend fun getChapterHtmlContent( bookId: String, chapterIndex: Int, - ): String? + ): String? = readChapterHtml(bookId, chapterIndex) @Query( """ diff --git a/app/src/main/java/com/kairo/reader/data/local/SearchDao.kt b/app/src/main/java/com/kairo/reader/data/local/SearchDao.kt index 8bcd84e..90ecaed 100644 --- a/app/src/main/java/com/kairo/reader/data/local/SearchDao.kt +++ b/app/src/main/java/com/kairo/reader/data/local/SearchDao.kt @@ -2,13 +2,14 @@ package com.kairo.reader.data.local import androidx.room.Dao import androidx.room.Query +import androidx.room.Transaction data class SearchPassageBookEntity(val bookId: String, val bookTitle: String,) data class SearchPassageChapterPageEntity(val chapterIndex: Int, val chapterTitle: String?, val plainText: String,) @Dao -interface SearchDao { +interface SearchDao : ChapterTextQueries { @Query( """ SELECT @@ -27,19 +28,29 @@ interface SearchDao { SELECT `index` AS chapterIndex, title AS chapterTitle, - plainText + '' AS plainText FROM chapters WHERE bookId = :bookId AND `index` > :afterChapterIndex ORDER BY `index` LIMIT :pageSize """, ) - suspend fun searchPassageChapterPage( + suspend fun searchPassageChapterMetadataPage( bookId: String, afterChapterIndex: Int, pageSize: Int, ): List + @Transaction + suspend fun searchPassageChapterPage( + bookId: String, + afterChapterIndex: Int, + pageSize: Int, + ): List = + searchPassageChapterMetadataPage(bookId, afterChapterIndex, pageSize).map { chapter -> + chapter.copy(plainText = requireNotNull(readChapterPlainText(bookId, chapter.chapterIndex))) + } + @Query( """ SELECT id, title, authors, languageTag, NULL AS coverImage, isCompleted, importFingerprint diff --git a/app/src/main/java/com/kairo/reader/ui/navigation/ReaderRoute.kt b/app/src/main/java/com/kairo/reader/ui/navigation/ReaderRoute.kt index f02b3b8..411ba90 100644 --- a/app/src/main/java/com/kairo/reader/ui/navigation/ReaderRoute.kt +++ b/app/src/main/java/com/kairo/reader/ui/navigation/ReaderRoute.kt @@ -16,7 +16,6 @@ import androidx.compose.runtime.Composable import androidx.compose.runtime.LaunchedEffect import androidx.compose.runtime.collectAsState import androidx.compose.runtime.getValue -import androidx.compose.runtime.mutableIntStateOf import androidx.compose.runtime.mutableLongStateOf import androidx.compose.runtime.mutableStateOf import androidx.compose.runtime.produceState @@ -167,7 +166,6 @@ internal fun ReaderRoute(input: ReaderRouteInput) { onShowUserMessage = onShowUserMessage, ) - var lastExplicitFocusIndex by remember(bookId) { mutableIntStateOf(-1) } val readerCallbacks = buildReaderRouteCallbacks( ReaderRouteCallbackDependencies( @@ -185,8 +183,6 @@ internal fun ReaderRoute(input: ReaderRouteInput) { effectiveUiState = rsvpResult.effectiveUiState, readerViewModel = readerViewModel, readerPositionSaver = readerPositionSaver, - getLastExplicitFocusIndex = { lastExplicitFocusIndex }, - setLastExplicitFocusIndex = { lastExplicitFocusIndex = it }, getPendingRsvpLaunchTempoMsPerWord = { rsvpResult.pendingLaunchTempoMs }, clearPendingRsvpLaunchTempoMsPerWord = rsvpResult.clearPendingLaunchTempo, onShowUserMessage = onShowUserMessage, diff --git a/app/src/main/java/com/kairo/reader/ui/navigation/ReaderRouteCallbacks.kt b/app/src/main/java/com/kairo/reader/ui/navigation/ReaderRouteCallbacks.kt index 23efb27..31d855e 100644 --- a/app/src/main/java/com/kairo/reader/ui/navigation/ReaderRouteCallbacks.kt +++ b/app/src/main/java/com/kairo/reader/ui/navigation/ReaderRouteCallbacks.kt @@ -53,8 +53,6 @@ internal data class ReaderRouteCallbackDependencies( val effectiveUiState: ReaderUiState, val readerViewModel: ReaderViewModel, val readerPositionSaver: ReaderPositionSaver, - val getLastExplicitFocusIndex: () -> Int, - val setLastExplicitFocusIndex: (Int) -> Unit, val getPendingRsvpLaunchTempoMsPerWord: () -> Long, val clearPendingRsvpLaunchTempoMsPerWord: () -> Unit, val onShowUserMessage: (String) -> Unit, @@ -125,7 +123,6 @@ internal fun buildReaderRouteCallbacks( dependencies.navigateReaderToLibrary() }, onFocusChange = { newFocusIndex -> - dependencies.setLastExplicitFocusIndex(newFocusIndex) dependencies.readerViewModel.setFocusIndex(newFocusIndex) val wordIndex = resolveWordIndex( @@ -149,7 +146,6 @@ internal fun buildReaderRouteCallbacks( } else { tokens.nearestWordIndex(focusTokenIndex).coerceIn(0, tokens.lastIndex) } - dependencies.setLastExplicitFocusIndex(safeIndex) dependencies.readerViewModel.setPageIndex(pageIndex, safeIndex) val wordIndex = resolveWordIndex( @@ -186,17 +182,19 @@ internal fun buildReaderRouteCallbacks( }, ) -private fun ReaderRouteCallbackDependencies.buildCurrentReaderPosition( - tokenIndex: Int = effectiveUiState.focusIndex, -): ReadingPosition? { - val chapterData = effectiveUiState.chapterData ?: return null +private fun ReaderRouteCallbackDependencies.buildCurrentReaderPosition(): ReadingPosition? { + // A tap or chapter change can precede recomposition. Read the complete current + // location together, retaining a pending RSVP result only until the model catches up. + val liveState = readerViewModel.uiState.value + val currentState = if (liveState == uiState) effectiveUiState else liveState + val chapterData = currentState.chapterData ?: return null val tokens = chapterData.tokens if (tokens.isEmpty()) return null - val safeIndex = tokens.nearestWordIndex(tokenIndex).coerceIn(0, tokens.lastIndex) + val safeIndex = tokens.nearestWordIndex(currentState.focusIndex).coerceIn(0, tokens.lastIndex) val wordIndex = resolveWordIndex(chapterData.wordCountByToken, safeIndex) return ReadingPosition( bookIdValue, - effectiveUiState.chapterIndex, + currentState.chapterIndex, safeIndex, wordIndex, ) @@ -208,11 +206,7 @@ private fun ReaderRouteCallbackDependencies.saveCurrentReaderPosition() { private fun ReaderRouteCallbackDependencies.navigateReaderToLibrary() { container.readingSessionCoordinator.finalizeReader(bookIdValue) - val position = - getLastExplicitFocusIndex() - .takeIf { it >= 0 } - ?.let(::buildCurrentReaderPosition) - ?: buildCurrentReaderPosition() + val position = buildCurrentReaderPosition() lifecycleScope.launch(dispatcherProvider.io) { if (position != null) { readerPositionSaver.saveImmediateAndJoin(position) diff --git a/app/src/main/java/com/kairo/reader/ui/reader/ReaderListState.kt b/app/src/main/java/com/kairo/reader/ui/reader/ReaderListState.kt index 0e2ca8a..6efe9e0 100644 --- a/app/src/main/java/com/kairo/reader/ui/reader/ReaderListState.kt +++ b/app/src/main/java/com/kairo/reader/ui/reader/ReaderListState.kt @@ -68,8 +68,10 @@ internal fun rememberReaderListState( } } - LaunchedEffect(listStateKey, listItemCount) { - if (listItemCount > 0 && listState.firstVisibleItemIndex != safeIndex) { + LaunchedEffect(listStateKey, listItemCount, safeIndex) { + // Search/TOC navigation can change focus without changing the current page. + // A word tapped in an already visible paragraph should not move that paragraph. + if (listItemCount > 0 && listState.layoutInfo.visibleItemsInfo.none { it.index == safeIndex }) { listState.scrollToItem(safeIndex) } } diff --git a/app/src/test/java/com/kairo/reader/data/search/LibrarySearchRepositoryImplTest.kt b/app/src/test/java/com/kairo/reader/data/search/LibrarySearchRepositoryImplTest.kt index ce65f38..a7ac167 100644 --- a/app/src/test/java/com/kairo/reader/data/search/LibrarySearchRepositoryImplTest.kt +++ b/app/src/test/java/com/kairo/reader/data/search/LibrarySearchRepositoryImplTest.kt @@ -272,7 +272,7 @@ private class FakeSearchDao( ) } - override suspend fun searchPassageChapterPage( + override suspend fun searchPassageChapterMetadataPage( bookId: String, afterChapterIndex: Int, pageSize: Int, @@ -288,6 +288,26 @@ private class FakeSearchDao( .toList() } + override suspend fun getChapterHtmlChunk( + bookId: String, + chapterIndex: Int, + offset: Int, + byteCount: Int, + ): com.kairo.reader.data.local.ChapterTextChunk? = + error("Passage search must not load HTML") + + override suspend fun getChapterPlainTextChunk( + bookId: String, + chapterIndex: Int, + offset: Int, + byteCount: Int, + ): com.kairo.reader.data.local.ChapterTextChunk? { + val bytes = passagesByBook[bookId]?.firstOrNull { it.chapterIndex == chapterIndex } + ?.plainText?.toByteArray() ?: return null + val start = (offset - 1).coerceAtMost(bytes.size) + return com.kairo.reader.data.local.ChapterTextChunk(bytes.copyOfRange(start, (start + byteCount).coerceAtMost(bytes.size))) + } + override suspend fun searchBooks( pattern: String, limit: Int, diff --git a/docs/device-verified-reader-fixes-2026-09-29.md b/docs/device-verified-reader-fixes-2026-09-29.md new file mode 100644 index 0000000..15ed67d --- /dev/null +++ b/docs/device-verified-reader-fixes-2026-09-29.md @@ -0,0 +1,63 @@ +# Device-verified reader fixes + +- Date: 2026-09-29 +- Baseline: `main` at `3ba2873` +- Branch: `fix/device-verified-reader-issues` +- Device: CPH2493, Android 16 +- Package tested: `com.kairo.reader.debug` + +## Reproduction before fixes + +The three regression tests were installed and run on the physical handset with +production sources unchanged from the baseline. All three failed for the reported +behavior, before any production fixes were applied. + +| Issue | Device evidence before fixing | Result after fixing | +| --- | --- | --- | +| Exit saves a stale location after a chapter change | The real Reader route paged into chapter index 1 at token 0. Returning to the library saved token 678 instead. Assertion: `expected:<0> but was:<678>`. | The saved chapter and token match the visible location; reopening restores that location. | +| Accepted large text cannot be reopened | The file importer accepted a 1,475,000-byte TXT file, producing 3,099,997 bytes of HTML plus plain text in one chapter. Reading it through Room threw `SQLiteBlobTooBigException: Row too big to fit into CursorWindow`. | The imported chapter's HTML and plain text are read back in full and compare exactly to the imported content. | +| Focus navigation on the same page does not scroll | A Compose device fixture using the reader's actual list-state controller moved focus from paragraph 0 to paragraph 12 without changing page identity. The first visible paragraph stayed 0. Assertion: `expected:<12> but was:<0>`. | Paragraph 12 becomes visible. Focusing a paragraph already in view preserves the viewport's scroll offset. | + +The scrolling reproduction tests the shared reader component on the handset; +it does not automate typing into the full book-search dialog. + +## Changes + +- Save a complete current Reader location from the ViewModel at exit, instead of + combining an old remembered token index with a new chapter. Preserve a pending + RSVP result while the ViewModel catches up. +- Respond to focus changes within the same page, scrolling only when the target + paragraph is outside the visible list items. +- Read chapter bodies in 128 KiB byte chunks inside a transaction. Decode UTF-8 + after reassembly, preserving characters across chunk boundaries. Apply the same + bounded reads to book loading, duplicate-content comparison, passage search, + and EPUB navigation repair. No schema or chapter-coordinate migration is needed. +- Distinguish a missing chapter from an existing empty body, including Android's + handling of zero-length BLOB results. + +## Verification + +- `qualityGate` and `:app:assembleDebugAndroidTest`: passed using JDK 19. +- JVM suite: 882 tests, zero failures, errors, or skips. +- Physical-device run: 39 tests, all passed in 19.497 seconds. +- New device suites: `LargeChapterDeviceTest`, `ReaderFocusNavigationDeviceTest`, + and `ReaderPositionDeviceTest`. +- Existing device suites: `ImportMaintenanceDeviceTest`, `PersistenceIntegrityTest`, + `Migration12To13Test`, `Migration13To14Test`, `ReaderCoverChapterTest`, and + `ReaderNoteDialogTest`. +- Additional storage coverage: existing oversized chapters, supplementary Unicode + characters across chunk boundaries, embedded NUL, search offsets near the end + of a large chapter, empty bodies, exact chunk-size bodies, and missing chapters. +- `git diff --check`: passed. + +Database tests used in-memory databases. The real Reader-route test imported a +uniquely named disposable EPUB and removed its book, position, and session records +in cleanup. The release app was not installed over or cleared. The debug app's +preferences SHA-256 stayed identical before and after the device runs: +`79e756d01e72d69ba0b94ef3faa985ce0d2ce51f5b559883129d8e6b4cfede20`. + +Local diagnostic logs from this run: + +- `/private/tmp/kairo-reader-baseline-device.txt` +- `/private/tmp/kairo-reader-final-device.txt` +- `/private/tmp/kairo-reader-quality-gate.txt`