From 510bfad217202ad9dc2e300df1d3d3d017944136 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 10 Jun 2026 17:09:48 +0000 Subject: [PATCH 1/5] fix(android): stop large-graph startup crash from uncaught Throwables on the ViewModel scope MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Root cause of the 8 030-page "SteleKit keeps stopping" crash: the ViewModel scope (App.kt) had no CoroutineExceptionHandler. Under large-graph heap pressure an OutOfMemoryError is thrown in whichever coroutine allocates next — the unguarded standing collectors (observeSpecialPages, PageNameIndex) and fire-and-forget launches (ensureTodayJournal) on that scope let it reach the default uncaught-exception handler, which kills the process on Android but only logs on desktop. That is why the crash never reproduced on desktop and why prior mitigations inside GraphLoader's own scopes did not help. The hang half came from observeSpecialPages keeping a standing collector on getAllPages(): every DB write invalidates that query, so graph import/ reconcile re-materialized all 8 030 Page objects per write burst (GC thrash), and cachedAllPages pinned a full-table snapshot for the ViewModel's lifetime. Fixes: - StelekitViewModel scope now carries a CoroutineExceptionHandler that surfaces uncaught Throwables as fatalError (recovery screen) instead of crashing. - observeSpecialPages no longer collects getAllPages(): favorites use a new dedicated selectFavoritePages query (PageRepository.getFavoritePages, with a conflated SqlDelight override); recents resolve via bounded point lookups; cachedAllPages is removed. - PageNameIndex guards its collector and Aho-Corasick matcher build against Throwable, degrading suggestions instead of dying (its first 8 000-name trie build lands ~500 ms after launch — exactly the observed crash window). - android:largeHeap="true" for headroom on whole-graph workloads. Tests (fail before the fix — they reproduce the crash by recording Throwables that reach the default uncaught-exception handler): - StelekitViewModelCrashReproductionTest: OOM in the page-list observer, OOM in ensureTodayJournal, and a guard that the ViewModel keeps at most one standing getAllPages() subscription. - PageNameIndexResilienceTest: OOM from the page flow keeps the last matcher. - LargeGraphWarmStartCrashTest: end-to-end 8 030-page warm start (persistent cache kept, warm reconcile + Phase 3 indexing) completes with no uncaught Throwables. https://claude.ai/code/session_01Avi2KYmMrxLLFCiMBJHwpb --- CLAUDE.md | 18 ++ androidApp/src/main/AndroidManifest.xml | 4 + .../stapler/stelekit/domain/PageNameIndex.kt | 36 +++- .../repository/InMemoryRepositories.kt | 6 + .../stelekit/repository/PageRepository.kt | 13 ++ .../repository/SqlDelightPageRepository.kt | 8 + .../stapler/stelekit/ui/StelekitViewModel.kt | 95 ++++++--- .../dev/stapler/stelekit/db/SteleDatabase.sq | 3 + .../db/LargeGraphWarmStartCrashTest.kt | 177 ++++++++++++++++ .../domain/PageNameIndexResilienceTest.kt | 106 ++++++++++ .../StelekitViewModelCrashReproductionTest.kt | 200 ++++++++++++++++++ .../stelekit/ui/fixtures/FakeRepositories.kt | 3 + 12 files changed, 640 insertions(+), 29 deletions(-) create mode 100644 kmp/src/jvmTest/kotlin/dev/stapler/stelekit/db/LargeGraphWarmStartCrashTest.kt create mode 100644 kmp/src/jvmTest/kotlin/dev/stapler/stelekit/domain/PageNameIndexResilienceTest.kt create mode 100644 kmp/src/jvmTest/kotlin/dev/stapler/stelekit/ui/StelekitViewModelCrashReproductionTest.kt diff --git a/CLAUDE.md b/CLAUDE.md index 9737b202..9b1dec13 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -283,6 +283,24 @@ class SomeManager(...) { val manager = remember { SomeManager() } ``` +### Uncaught coroutine Throwables kill the process on Android — guard long-lived scopes + +An uncaught `Throwable` (notably `OutOfMemoryError`) escaping any coroutine reaches the platform default uncaught-exception handler. **On Android that handler kills the process ("app keeps stopping"); on desktop JVM it only prints** — so this class of crash never reproduces on desktop. Under heap pressure the OOM is thrown in whichever coroutine allocates next, not necessarily the one doing the heavy work, so per-call-site `catch(Throwable)` is not sufficient. + +Rules: +- Every long-lived `CoroutineScope` that hosts user-path collectors or fire-and-forget launches must attach a `CoroutineExceptionHandler` (see `StelekitViewModel.scope`, `GraphLoader.parallelScope`). Surface errors as `fatalError` UI state where possible. +- Standing `collect { }` bodies and `stateIn` upstream chains on such scopes are the unguarded vectors — a repository flow's `catchDbError()` does not protect them. +- Regression tests: `StelekitViewModelCrashReproductionTest`, `PageNameIndexResilienceTest`, `LargeGraphWarmStartCrashTest` (8 030-page warm start with a recording default uncaught-exception handler). + +### Never collect `getAllPages()` from a standing observer + +Every DB write invalidates `selectAllPages`, so a standing collector re-materializes the **entire pages table per write burst**. During graph import/reconcile on an 8 000+ page graph this causes GC thrash (UI hang) and `OutOfMemoryError` (crash) on Android. + +- Sidebar/UI observers must use bounded queries: `getFavoritePages()` (dedicated `WHERE is_favorite = 1` query), `getPages(limit, offset)`, `getPageByUuid` point lookups. +- The single sanctioned standing `getAllPages()` consumer is `PageNameIndex` (conflated + `distinctUntilChanged` + 500 ms debounce + `Throwable`-guarded). +- One-shot `getAllPages().first()` during a load phase (e.g. `GraphLoader.loadDirectory`) is acceptable — transient, bounded, released after the phase. +- Do not pin full-table snapshots in fields (the former `cachedAllPages` pattern is forbidden). + ### Android Application.onCreate — catch Throwable, not Exception `Application.onCreate()` must use `catch (e: Throwable)`, not `catch (e: Exception)`. Native library loading failures (`UnsatisfiedLinkError`, `NoClassDefFoundError`) are `Error` subclasses, not `Exception`. Catching only `Exception` lets them propagate uncaught and crash the app at startup before the UI is shown. See `SteleKitApplication.kt`. diff --git a/androidApp/src/main/AndroidManifest.xml b/androidApp/src/main/AndroidManifest.xml index 7cac9345..a2a6c559 100644 --- a/androidApp/src/main/AndroidManifest.xml +++ b/androidApp/src/main/AndroidManifest.xml @@ -11,8 +11,12 @@ + >(emptyList()) + private val logger = Logger("PageNameIndex") + /** * Pre-built matcher for the current page set. Rebuilt on a background thread * whenever the page list changes. Null until the first page list is received * or when the filtered page set is empty. + * + * The trie build allocates one node per distinct pattern character (8 000+ page graphs + * produce hundreds of thousands of nodes), so under memory pressure the construction + * itself can throw OutOfMemoryError. Degrade to a null matcher (suggestions off) instead + * of letting the Throwable escape the stateIn coroutine — uncaught, it kills the process + * on Android. */ val matcher: StateFlow = _entries - .map { entries -> if (entries.isEmpty()) null else AhoCorasickMatcher(entries) } + .map { entries -> + if (entries.isEmpty()) null else try { + AhoCorasickMatcher(entries) + } catch (e: CancellationException) { + throw e + } catch (e: Throwable) { + logger.error("Matcher build failed — suggestions disabled: ${e::class.simpleName}: ${e.message}") + null + } + } .flowOn(Dispatchers.Default) .stateIn(scope, SharingStarted.Eagerly, null) @@ -56,9 +76,21 @@ class PageNameIndex( // single rebuild. 500 ms matches the block-editor save debounce so a rename // and its save settle before the matcher is rebuilt. .debounce(rebuildDebounceMs) + // A Throwable from the upstream flow (e.g. OutOfMemoryError materializing a + // large page list) must not escape this collector: uncaught coroutine + // Throwables kill the process on Android. Keep the last good entries. + .catch { e -> + logger.error("Page flow failed — keeping last matcher: ${e::class.simpleName}: ${e.message}") + } .collect { result -> result.getOrNull()?.let { pages -> - _entries.value = buildEntries(pages.map { it.name to it.isJournal }) + try { + _entries.value = buildEntries(pages.map { it.name to it.isJournal }) + } catch (e: CancellationException) { + throw e + } catch (e: Throwable) { + logger.error("Entry rebuild failed — keeping last matcher: ${e::class.simpleName}: ${e.message}") + } } } } diff --git a/kmp/src/commonMain/kotlin/dev/stapler/stelekit/repository/InMemoryRepositories.kt b/kmp/src/commonMain/kotlin/dev/stapler/stelekit/repository/InMemoryRepositories.kt index 32f1c7c9..5ba39731 100644 --- a/kmp/src/commonMain/kotlin/dev/stapler/stelekit/repository/InMemoryRepositories.kt +++ b/kmp/src/commonMain/kotlin/dev/stapler/stelekit/repository/InMemoryRepositories.kt @@ -574,6 +574,12 @@ class InMemoryPageRepository : PageRepository { } } + override fun getFavoritePages(): Flow>> { + return pages.map { map -> + map.values.filter { it.isFavorite }.sortedBy { it.name }.right() + } + } + override fun getJournalPages(limit: Int, offset: Int): Flow>> { return pages.map { map -> val journals = map.values diff --git a/kmp/src/commonMain/kotlin/dev/stapler/stelekit/repository/PageRepository.kt b/kmp/src/commonMain/kotlin/dev/stapler/stelekit/repository/PageRepository.kt index 33d431f2..7fd022a8 100644 --- a/kmp/src/commonMain/kotlin/dev/stapler/stelekit/repository/PageRepository.kt +++ b/kmp/src/commonMain/kotlin/dev/stapler/stelekit/repository/PageRepository.kt @@ -5,6 +5,7 @@ import dev.stapler.stelekit.error.DomainError import dev.stapler.stelekit.model.Page import dev.stapler.stelekit.model.PageUuid import kotlinx.coroutines.flow.Flow +import kotlinx.coroutines.flow.map interface PageRepository { fun getPageByUuid(uuid: PageUuid): Flow> @@ -23,6 +24,18 @@ interface PageRepository { fun getJournalPageByDate(date: kotlinx.datetime.LocalDate): Flow> fun getRecentPages(limit: Int = 50): Flow>> + + /** + * Favorite pages only — the bounded query standing UI observers (sidebar) must use. + * + * The default implementation derives from [getAllPages] for in-memory/test backends. + * SQL-backed repositories MUST override it with a dedicated `WHERE is_favorite = 1` + * query: a standing collector on [getAllPages] re-materializes the entire pages table + * on every write, which on 8 000+ page graphs causes GC thrash and OutOfMemoryError + * on Android during graph import/reconcile. + */ + fun getFavoritePages(): Flow>> = + getAllPages().map { either -> either.map { pages -> pages.filter { it.isFavorite } } } fun getUnloadedPages(): Flow>> @DirectRepositoryWrite diff --git a/kmp/src/commonMain/kotlin/dev/stapler/stelekit/repository/SqlDelightPageRepository.kt b/kmp/src/commonMain/kotlin/dev/stapler/stelekit/repository/SqlDelightPageRepository.kt index 2ca7ca89..477cc3c5 100644 --- a/kmp/src/commonMain/kotlin/dev/stapler/stelekit/repository/SqlDelightPageRepository.kt +++ b/kmp/src/commonMain/kotlin/dev/stapler/stelekit/repository/SqlDelightPageRepository.kt @@ -103,6 +103,14 @@ class SqlDelightPageRepository( .map { list -> list.map { it.toModel() }.right() } .catchDbError() + override fun getFavoritePages(): Flow>> = + queries.selectFavoritePages() + .asFlow() + .conflate() // drop intermediate invalidations during bulk import — this flow has a standing UI collector + .mapToList(PlatformDispatcher.DB) + .map { list -> list.map { it.toModel() }.right() } + .catchDbError() + override fun getJournalPages(limit: Int, offset: Int): Flow>> = queries.selectJournalPages(limit.toLong(), offset.toLong()) .asDbFlowList(PlatformDispatcher.DB) { it.toModel() } diff --git a/kmp/src/commonMain/kotlin/dev/stapler/stelekit/ui/StelekitViewModel.kt b/kmp/src/commonMain/kotlin/dev/stapler/stelekit/ui/StelekitViewModel.kt index 3b2263f3..6252e8a3 100644 --- a/kmp/src/commonMain/kotlin/dev/stapler/stelekit/ui/StelekitViewModel.kt +++ b/kmp/src/commonMain/kotlin/dev/stapler/stelekit/ui/StelekitViewModel.kt @@ -45,6 +45,7 @@ import dev.stapler.stelekit.domain.PageNameIndex import dev.stapler.stelekit.ui.screens.SearchResultItem import dev.stapler.stelekit.ui.state.BlockStateManager import dev.stapler.stelekit.coroutines.PlatformDispatcher +import kotlinx.coroutines.CoroutineExceptionHandler import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.SupervisorJob @@ -114,7 +115,27 @@ class StelekitViewModel( private val spanEmitter = dev.stapler.stelekit.performance.SpanEmitter(deps.ringBuffer) // Default scope owns its lifecycle; callers in remember{} must not pass rememberCoroutineScope() // which is cancelled when the composable leaves composition. Tests inject a TestCoroutineScope. - private val scope = deps.scope + // + // The CoroutineExceptionHandler is the last line of defense for every coroutine launched on + // this scope (standing collectors, fire-and-forget launches, stateIn upstreams). Without it, + // an OutOfMemoryError — which under heap pressure is thrown in whichever coroutine allocates + // next, not necessarily the one doing the heavy work — reaches the platform default handler. + // On Android that kills the process ("SteleKit keeps stopping"); on desktop it merely prints, + // which is why large-graph crashes reproduced only on Android. Surface as fatalError instead + // so the user gets the recoverable error screen. + private val scope = CoroutineScope( + deps.scope.coroutineContext + CoroutineExceptionHandler { _, e -> + if (e !is CancellationException) { + logger.error("Uncaught Throwable in ViewModel coroutine — ${e::class.simpleName}: ${e.message}") + _uiState.update { + it.copy( + isLoading = false, + fatalError = "${e::class.simpleName ?: "UnknownError"}: ${e.message ?: "unknown"}" + ) + } + } + } + ) private val logger = Logger("StelekitViewModel") /** @@ -221,8 +242,12 @@ class StelekitViewModel( private val recentPagesKey: String get() = "recent_pages_${_uiState.value.currentGraphPath}" - - private var cachedAllPages: List = emptyList() + + // Resolved Page objects for the recent-pages list, keyed by UUID and bounded by + // recentPageUuids (≤20 entries). Replaces the former cachedAllPages field, which + // pinned the entire pages table (8 000+ Page objects on large graphs) in memory + // for the lifetime of the ViewModel. + private val recentPagesByUuid = mutableMapOf() // Initialize command system private val commandManager = CommandManager.create(scope) { message, type, timeout -> @@ -287,30 +312,43 @@ class StelekitViewModel( .split(",") .filter { it.isNotEmpty() } .toMutableList() - - // We still need to know which pages are favorites for the sidebar - // This is usually a small list - pageRepository.getAllPages().collect { result -> - val allPages = result.getOrNull() ?: emptyList() - cachedAllPages = allPages // Keep for UUID lookups - - _uiState.update { state -> - val recent = recentPageUuids.mapNotNull { uuid -> - allPages.find { it.uuid.value == uuid } - }.take(10) - state.copy( - favoritePages = allPages.filter { it.isFavorite }, - recentPages = recent - ) - } + refreshRecentPages() + + // Favorites for the sidebar via the dedicated bounded query. Never collect + // getAllPages() from a standing observer: every DB write invalidates that query, + // so during graph import/reconcile the collector re-materializes the entire + // pages table over and over — on 8 000+ page graphs this causes GC thrash + // (UI hang) and eventually OutOfMemoryError on Android. + pageRepository.getFavoritePages().collect { result -> + val favorites = result.getOrNull() ?: emptyList() + _uiState.update { it.copy(favoritePages = favorites) } } } - + // Initial load of regular pages and journals loadMoreRegularPages(reset = true) loadMoreJournalPages(reset = true) } + /** + * Re-resolves [recentPageUuids] into Page objects via point lookups (≤10 indexed + * queries) and publishes them to the UI state. Cheap by construction — never scans + * the pages table. + */ + private suspend fun refreshRecentPages() { + val recent = recentPageUuids.take(10).mapNotNull { uuid -> + recentPagesByUuid[uuid] + ?: pageRepository.getPageByUuid(PageUuid(uuid)).first().getOrNull() + ?.also { recentPagesByUuid[uuid] = it } + } + trimRecentPagesCache() + _uiState.update { it.copy(recentPages = recent) } + } + + private fun trimRecentPagesCache() { + recentPagesByUuid.keys.retainAll(recentPageUuids.toSet()) + } + fun loadMoreRegularPages(reset: Boolean = false) { if (_uiState.value.isLoadingMorePages) return // already loading val currentOffset = if (reset) 0 else _uiState.value.regularPagesOffset @@ -353,20 +391,21 @@ class StelekitViewModel( // Remove if exists to move to top recentPageUuids.remove(page.uuid.value) recentPageUuids.add(0, page.uuid.value) - + // Keep max 20 items if (recentPageUuids.size > 20) { recentPageUuids.removeAt(recentPageUuids.lastIndex) } - + // Save to settings platformSettings.putString(recentPagesKey, recentPageUuids.joinToString(",")) - // Update UI state + // Update UI state — the navigated-to page is already in hand; older entries come + // from the bounded recents cache (point lookups happen in refreshRecentPages). + recentPagesByUuid[page.uuid.value] = page + trimRecentPagesCache() _uiState.update { state -> - val recent = recentPageUuids.mapNotNull { uuid -> - cachedAllPages.find { it.uuid.value == uuid } - }.take(10) + val recent = recentPageUuids.mapNotNull { recentPagesByUuid[it] }.take(10) state.copy(recentPages = recent) } } @@ -398,6 +437,8 @@ class StelekitViewModel( _uiState.update { it.copy(currentGraphPath = path) } recentPageUuids = platformSettings.getString(recentPagesKey, "") .split(",").filter { it.isNotEmpty() }.toMutableList() + recentPagesByUuid.clear() + scope.launch { refreshRecentPages() } loadGraph(path) } @@ -988,7 +1029,7 @@ class StelekitViewModel( scope.launch { val block = blockRepository.getBlockByUuid(BlockUuid(blockUuid)).first().getOrNull() if (block != null) { - val page = cachedAllPages.find { it.uuid == block.pageUuid } + val page = pageRepository.getPageByUuid(block.pageUuid).first().getOrNull() if (page != null) { navigateTo(Screen.PageView(page)) // TODO: Scroll to block diff --git a/kmp/src/commonMain/sqldelight/dev/stapler/stelekit/db/SteleDatabase.sq b/kmp/src/commonMain/sqldelight/dev/stapler/stelekit/db/SteleDatabase.sq index b48e4417..067bfaa1 100644 --- a/kmp/src/commonMain/sqldelight/dev/stapler/stelekit/db/SteleDatabase.sq +++ b/kmp/src/commonMain/sqldelight/dev/stapler/stelekit/db/SteleDatabase.sq @@ -322,6 +322,9 @@ SELECT * FROM pages ORDER BY name LIMIT ? OFFSET ?; selectUnloadedPages: SELECT * FROM pages WHERE is_content_loaded = 0; +selectFavoritePages: +SELECT * FROM pages WHERE is_favorite = 1 ORDER BY name; + selectPagesByNamespace: SELECT * FROM pages WHERE namespace = ? ORDER BY name LIMIT ? OFFSET ?; diff --git a/kmp/src/jvmTest/kotlin/dev/stapler/stelekit/db/LargeGraphWarmStartCrashTest.kt b/kmp/src/jvmTest/kotlin/dev/stapler/stelekit/db/LargeGraphWarmStartCrashTest.kt new file mode 100644 index 00000000..5e6fd7df --- /dev/null +++ b/kmp/src/jvmTest/kotlin/dev/stapler/stelekit/db/LargeGraphWarmStartCrashTest.kt @@ -0,0 +1,177 @@ +package dev.stapler.stelekit.db + +import dev.stapler.stelekit.model.Page +import dev.stapler.stelekit.model.PageUuid +import dev.stapler.stelekit.platform.PlatformFileSystem +import dev.stapler.stelekit.repository.InMemorySearchRepository +import dev.stapler.stelekit.ui.IndexingState +import dev.stapler.stelekit.ui.StelekitViewModel +import dev.stapler.stelekit.ui.StelekitViewModelDependencies +import dev.stapler.stelekit.ui.fixtures.FakeBlockRepository +import dev.stapler.stelekit.ui.fixtures.FakeFileSystem +import dev.stapler.stelekit.ui.fixtures.FakePageRepository +import dev.stapler.stelekit.ui.fixtures.InMemorySettings +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.SupervisorJob +import kotlinx.coroutines.delay +import kotlinx.coroutines.flow.first +import kotlinx.coroutines.runBlocking +import kotlinx.coroutines.withTimeout +import kotlinx.datetime.LocalDate +import java.util.concurrent.CopyOnWriteArrayList +import kotlin.test.Test +import kotlin.test.assertTrue +import kotlin.time.Clock +import kotlin.time.Instant + +/** + * End-to-end reproduction of the field scenario that crashed Android: a warm start on a + * graph with 8 030 pages already registered in the database ("Loading same graph — Keeping + * persistent cache for incremental load", "Page count: 8030"), followed by the full warm + * reconcile (Phase 2) and background indexing (Phase 3) over every page. + * + * The test drives the real GraphLoader + StelekitViewModel pipeline — including the + * standing UI observers (favorites, PageNameIndex) that previously re-materialized the + * whole pages table on every write burst — and asserts the run completes with no Throwable + * reaching the JVM default uncaught-exception handler. On Android any such Throwable kills + * the process ("SteleKit keeps stopping"). + */ +class LargeGraphWarmStartCrashTest { + + private companion object { + const val PAGE_COUNT = 8_030 + const val JOURNAL_COUNT = 10 + const val GRAPH_PATH = "/tmp/large-graph" + } + + private class UncaughtRecorder : AutoCloseable { + val uncaught = CopyOnWriteArrayList() + private val previous = Thread.getDefaultUncaughtExceptionHandler() + + init { + Thread.setDefaultUncaughtExceptionHandler { _, e -> uncaught.add(e) } + } + + override fun close() { + Thread.setDefaultUncaughtExceptionHandler(previous) + } + } + + /** Serves [PAGE_COUNT] synthetic markdown pages from the pages/ directory. */ + private class LargeGraphFileSystem : FakeFileSystem() { + private val pageFiles = (1..PAGE_COUNT).map { "Test Page $it.md" } + + override fun listFiles(path: String): List = when (path) { + "$GRAPH_PATH/pages" -> pageFiles + else -> emptyList() + } + + override fun directoryExists(path: String): Boolean = true + + override fun readFile(path: String): String? { + val name = path.substringAfterLast("/").removeSuffix(".md") + return "- first block of $name\n- second block with [[Test Page 1]] link" + } + + override fun getLastModifiedTime(path: String): Long = 1_000L + } + + @Test + fun warm_start_with_8030_pages_completes_without_uncaught_throwables() = runBlocking { + UncaughtRecorder().use { recorder -> + val now = Clock.System.now() + val initialPages = buildList { + // 8 030 regular pages already known to the DB but not content-loaded — + // forces the warm reconcile to re-parse every file (the heavy path) and + // Phase 3 to fully index all of them, as on a real first warm start. + for (i in 1..PAGE_COUNT) { + add( + Page( + uuid = PageUuid("page-$i"), + name = "Test Page $i", + filePath = "$GRAPH_PATH/pages/Test Page $i.md", + createdAt = Instant.fromEpochMilliseconds(0), + updatedAt = Instant.fromEpochMilliseconds(0), + isContentLoaded = false, + ) + ) + } + // Journals in the DB put GraphLoader on the warm-start fast path + // ("Warm start: N journals in DB — skipping blocking Phase 1"). + for (i in 1..JOURNAL_COUNT) { + add( + Page( + uuid = PageUuid("journal-$i"), + name = "2026-06-${i.toString().padStart(2, '0')}", + createdAt = now, + updatedAt = now, + isJournal = true, + journalDate = LocalDate(2026, 6, i), + isContentLoaded = true, + ) + ) + } + } + + val pageRepo = FakePageRepository(initialPages) + val blockRepo = FakeBlockRepository() + val fileSystem = LargeGraphFileSystem() + val settings = InMemorySettings().apply { + putBoolean("onboardingCompleted", true) + putString("lastGraphPath", GRAPH_PATH) + // Same cached path ⇒ "Keeping persistent cache for incremental load", + // exactly as in the field logs. + putString("cached_graph_path", GRAPH_PATH) + } + + val vm = StelekitViewModel( + StelekitViewModelDependencies( + fileSystem = fileSystem, + pageRepository = pageRepo, + blockRepository = blockRepo, + searchRepository = InMemorySearchRepository(), + graphLoader = GraphLoader(fileSystem, pageRepo, blockRepo), + graphWriter = GraphWriter(PlatformFileSystem()), + platformSettings = settings, + // Production-shaped scope (App.kt): no exception handler of its own. + scope = CoroutineScope(Dispatchers.Default + SupervisorJob()), + ) + ) + + // Phase 1 (warm) must release the UI quickly. + withTimeout(60_000) { vm.uiState.first { !it.isLoading } } + + // Phase 2 warm reconcile parses all 8 030 files. + withTimeout(300_000) { vm.uiState.first { it.isFullyLoaded } } + + // Phase 3 background indexing fully loads all pages. + withTimeout(300_000) { + vm.indexingProgress.first { it is IndexingState.Complete } + } + + // The page-name suggestion index must eventually build at this scale. + withTimeout(60_000) { + while (vm.suggestionMatcher.value == null) delay(100) + } + + assertTrue( + recorder.uncaught.isEmpty(), + "Throwable(s) reached the default uncaught-exception handler during the " + + "8030-page warm start — on Android this kills the process " + + "(\"SteleKit keeps stopping\"). " + + "Uncaught: ${recorder.uncaught.map { it::class.simpleName + ": " + it.message }}" + ) + assertTrue( + vm.uiState.value.fatalError == null, + "Warm start must complete cleanly; fatalError=${vm.uiState.value.fatalError}" + ) + + val pageCount = pageRepo.countPages().first().getOrNull() ?: 0L + assertTrue( + pageCount >= PAGE_COUNT, + "All $PAGE_COUNT pages must survive the warm reconcile; found $pageCount" + ) + } + } +} diff --git a/kmp/src/jvmTest/kotlin/dev/stapler/stelekit/domain/PageNameIndexResilienceTest.kt b/kmp/src/jvmTest/kotlin/dev/stapler/stelekit/domain/PageNameIndexResilienceTest.kt new file mode 100644 index 00000000..63cfd111 --- /dev/null +++ b/kmp/src/jvmTest/kotlin/dev/stapler/stelekit/domain/PageNameIndexResilienceTest.kt @@ -0,0 +1,106 @@ +package dev.stapler.stelekit.domain + +import arrow.core.Either +import arrow.core.right +import dev.stapler.stelekit.error.DomainError +import dev.stapler.stelekit.model.Page +import dev.stapler.stelekit.model.PageUuid +import dev.stapler.stelekit.ui.fixtures.FakePageRepository +import kotlinx.coroutines.CompletableDeferred +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.SupervisorJob +import kotlinx.coroutines.cancel +import kotlinx.coroutines.delay +import kotlinx.coroutines.flow.Flow +import kotlinx.coroutines.flow.flow +import kotlinx.coroutines.runBlocking +import kotlinx.coroutines.withTimeout +import java.util.concurrent.CopyOnWriteArrayList +import kotlin.test.Test +import kotlin.test.assertNotNull +import kotlin.test.assertTrue +import kotlin.time.Clock + +/** + * PageNameIndex builds an Aho-Corasick trie over every page name (plus stem variants) — + * on 8 000+ page graphs this is one of the largest single allocations at startup and its + * first build lands ~500 ms after launch, exactly when the Android crash was observed. + * + * An OutOfMemoryError escaping its collector or the stateIn matcher build is an uncaught + * coroutine Throwable, which kills the process on Android. These tests reproduce that + * vector: before the guards were added they fail (the simulated OOM reaches the default + * uncaught-exception handler); with the guards the index degrades gracefully. + */ +class PageNameIndexResilienceTest { + + private class UncaughtRecorder : AutoCloseable { + val uncaught = CopyOnWriteArrayList() + private val previous = Thread.getDefaultUncaughtExceptionHandler() + + init { + Thread.setDefaultUncaughtExceptionHandler { _, e -> uncaught.add(e) } + } + + override fun close() { + Thread.setDefaultUncaughtExceptionHandler(previous) + } + } + + private fun page(name: String): Page { + val now = Clock.System.now() + return Page(uuid = PageUuid("uuid-$name"), name = name, createdAt = now, updatedAt = now) + } + + private class OomAfterFirstEmissionRepository( + private val pages: List, + ) : FakePageRepository() { + val oomThrown = CompletableDeferred() + override fun getAllPages(): Flow>> = flow { + emit(pages.right()) + // Leave time for the debounced rebuild to consume the first emission. + delay(400) + oomThrown.complete(Unit) + throw OutOfMemoryError("simulated OOM re-materializing the page list") + } + } + + @Test + fun oom_from_page_flow_keeps_last_matcher_and_does_not_crash() = runBlocking { + UncaughtRecorder().use { recorder -> + val repo = OomAfterFirstEmissionRepository( + listOf(page("Kotlin"), page("Gradle"), page("Compose")) + ) + val scope = CoroutineScope(SupervisorJob() + Dispatchers.Default) + try { + val index = PageNameIndex(repo, scope, rebuildDebounceMs = 100) + + // First emission must produce a working matcher. + withTimeout(10_000) { + while (index.matcher.value == null) delay(50) + } + val matcher = index.matcher.value + assertNotNull(matcher, "matcher must build from the first page list") + + // Then the flow dies with OOM — the index must absorb it. + withTimeout(10_000) { repo.oomThrown.await() } + delay(500) + + assertTrue( + recorder.uncaught.isEmpty(), + "OutOfMemoryError reached the default uncaught-exception handler — " + + "on Android this kills the process. " + + "Uncaught: ${recorder.uncaught.map { it::class.simpleName + ": " + it.message }}" + ) + // Last good matcher must survive the upstream failure. + assertNotNull(index.matcher.value, "matcher must keep its last good value") + assertTrue( + index.matcher.value!!.findAll("learning Kotlin today").isNotEmpty(), + "surviving matcher must still match page names" + ) + } finally { + scope.cancel() + } + } + } +} diff --git a/kmp/src/jvmTest/kotlin/dev/stapler/stelekit/ui/StelekitViewModelCrashReproductionTest.kt b/kmp/src/jvmTest/kotlin/dev/stapler/stelekit/ui/StelekitViewModelCrashReproductionTest.kt new file mode 100644 index 00000000..75e838d5 --- /dev/null +++ b/kmp/src/jvmTest/kotlin/dev/stapler/stelekit/ui/StelekitViewModelCrashReproductionTest.kt @@ -0,0 +1,200 @@ +package dev.stapler.stelekit.ui + +import arrow.core.Either +import arrow.core.right +import dev.stapler.stelekit.db.GraphLoader +import dev.stapler.stelekit.db.GraphWriter +import dev.stapler.stelekit.error.DomainError +import dev.stapler.stelekit.model.Page +import dev.stapler.stelekit.platform.PlatformFileSystem +import dev.stapler.stelekit.repository.InMemorySearchRepository +import dev.stapler.stelekit.ui.fixtures.FakeBlockRepository +import dev.stapler.stelekit.ui.fixtures.FakeFileSystem +import dev.stapler.stelekit.ui.fixtures.FakePageRepository +import dev.stapler.stelekit.ui.fixtures.InMemorySettings +import kotlinx.coroutines.CompletableDeferred +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.SupervisorJob +import kotlinx.coroutines.delay +import kotlinx.coroutines.flow.Flow +import kotlinx.coroutines.flow.emitAll +import kotlinx.coroutines.flow.first +import kotlinx.coroutines.flow.flow +import kotlinx.coroutines.runBlocking +import kotlinx.coroutines.withTimeout +import kotlinx.datetime.LocalDate +import java.util.concurrent.CopyOnWriteArrayList +import java.util.concurrent.atomic.AtomicInteger +import kotlin.test.Test +import kotlin.test.assertNotNull +import kotlin.test.assertTrue + +/** + * Recreates the "SteleKit keeps stopping" crash observed on Android with 8 000+ page graphs. + * + * Mechanism under test: an uncaught Throwable (in the field: OutOfMemoryError under + * large-graph heap pressure) escaping a coroutine launched on the ViewModel scope reaches + * the platform default uncaught-exception handler. On Android that handler kills the + * process; on desktop it merely logs — which is why the crash only ever manifested on + * Android, and why mitigations inside GraphLoader's own scopes never fixed it. + * + * Each test installs a recording default uncaught-exception handler. Before the fix + * (CoroutineExceptionHandler on the ViewModel scope + guarded collectors), these tests + * fail because the simulated OutOfMemoryError reaches the default handler — i.e. the + * crash is reproduced. After the fix they pass: the error is surfaced as + * [AppState.fatalError] (recoverable error screen) instead of killing the process. + */ +class StelekitViewModelCrashReproductionTest { + + /** Records every Throwable that reaches the JVM default uncaught-exception handler. */ + private class UncaughtRecorder : AutoCloseable { + val uncaught = CopyOnWriteArrayList() + private val previous = Thread.getDefaultUncaughtExceptionHandler() + + init { + Thread.setDefaultUncaughtExceptionHandler { _, e -> uncaught.add(e) } + } + + override fun close() { + Thread.setDefaultUncaughtExceptionHandler(previous) + } + } + + private fun makeViewModel(pageRepo: FakePageRepository): StelekitViewModel { + val blockRepo = FakeBlockRepository() + val fileSystem = FakeFileSystem() + // Production-shaped scope (App.kt): SupervisorJob + Dispatchers.Default, no handler. + // The guard under test lives inside StelekitViewModel, not in the injected scope. + val scope = CoroutineScope(Dispatchers.Default + SupervisorJob()) + return StelekitViewModel( + StelekitViewModelDependencies( + fileSystem = fileSystem, + pageRepository = pageRepo, + blockRepository = blockRepo, + searchRepository = InMemorySearchRepository(), + graphLoader = GraphLoader(fileSystem, pageRepo, blockRepo), + graphWriter = GraphWriter(PlatformFileSystem()), + platformSettings = InMemorySettings(), + scope = scope, + ) + ) + } + + // ─── TC-CRASH-001: OOM surfacing through the page-list flow ───────────────── + + /** + * Simulates an OutOfMemoryError surfacing while the full page list is materialized + * for a standing observer (the allocation that fails first on 8 000-page graphs). + */ + private class OomPageFlowRepository : FakePageRepository() { + val oomThrown = CompletableDeferred() + override fun getAllPages(): Flow>> = flow { + emit(emptyList().right()) + oomThrown.complete(Unit) + throw OutOfMemoryError("simulated allocation failure materializing 8030 pages") + } + } + + @Test + fun oom_in_page_list_observer_must_not_reach_default_uncaught_handler() = runBlocking { + UncaughtRecorder().use { recorder -> + val repo = OomPageFlowRepository() + val vm = makeViewModel(repo) + + withTimeout(10_000) { repo.oomThrown.await() } + // Give the failure time to propagate through the collector coroutines. + delay(1_000) + + assertTrue( + recorder.uncaught.isEmpty(), + "OutOfMemoryError reached the default uncaught-exception handler — " + + "on Android this kills the process (\"SteleKit keeps stopping\"). " + + "Uncaught: ${recorder.uncaught.map { it::class.simpleName + ": " + it.message }}" + ) + // ViewModel must remain alive and usable after the error. + assertNotNull(vm.uiState.value, "uiState must remain readable after the error") + } + Unit + } + + // ─── TC-CRASH-002: OOM in a fire-and-forget launch (ensureTodayJournal) ───── + + /** + * Simulates OutOfMemoryError thrown under heap pressure inside the unguarded + * `scope.launch { journalService.ensureTodayJournal() }` fired on Phase-1 completion — + * exactly the moment the field crash occurs ("Checking database..." then death). + */ + private class OomJournalLookupRepository : FakePageRepository() { + val lookupReached = CompletableDeferred() + override fun getJournalPageByDate(date: LocalDate): Flow> = + flow { + lookupReached.complete(Unit) + throw OutOfMemoryError("simulated OOM resolving today's journal") + } + } + + @Test + fun oom_during_ensureTodayJournal_is_surfaced_as_fatalError_not_a_crash() = runBlocking { + UncaughtRecorder().use { recorder -> + val repo = OomJournalLookupRepository() + val vm = makeViewModel(repo) + + // Triggers loadGraph → Phase 1 completes (FakeFileSystem lists no files) → + // onPhase1Complete launches ensureTodayJournal → repo throws OOM. + vm.setGraphPath("/tmp/test-graph") + + withTimeout(10_000) { repo.lookupReached.await() } + withTimeout(10_000) { vm.uiState.first { it.fatalError != null } } + + assertTrue( + recorder.uncaught.isEmpty(), + "OutOfMemoryError reached the default uncaught-exception handler — " + + "on Android this kills the process. " + + "Uncaught: ${recorder.uncaught.map { it::class.simpleName + ": " + it.message }}" + ) + assertTrue( + vm.uiState.value.fatalError.orEmpty().contains("OutOfMemoryError"), + "fatalError should surface the OOM for the recovery screen; " + + "was: ${vm.uiState.value.fatalError}" + ) + } + } + + // ─── TC-SCAN-001: no standing full-table observers in the ViewModel ───────── + + private class CountingPageRepository : FakePageRepository() { + val getAllPagesSubscriptions = AtomicInteger(0) + override fun getAllPages(): Flow>> { + val base = super.getAllPages() + return flow { + getAllPagesSubscriptions.incrementAndGet() + emitAll(base) + } + } + } + + /** + * The hang half of "hangs and crashes": every DB write invalidates getAllPages(), so + * each standing collector re-materializes the entire table per write burst during + * import/reconcile — O(N) Page allocations × thousands of writes = GC thrash and OOM. + * Exactly one standing subscription is allowed: PageNameIndex (debounced + conflated). + * The sidebar (favorites/recents) must use bounded queries instead. + */ + @Test + fun viewmodel_keeps_at_most_one_standing_getAllPages_subscription() = runBlocking { + val repo = CountingPageRepository() + makeViewModel(repo) + + // Let init-time observers (observeSpecialPages, PageNameIndex) subscribe. + delay(1_500) + + val subscriptions = repo.getAllPagesSubscriptions.get() + assertTrue( + subscriptions <= 1, + "Expected at most 1 standing getAllPages() subscription (PageNameIndex), " + + "found $subscriptions — extra full-table observers re-materialize all pages " + + "on every write and OOM Android on 8 000+ page graphs" + ) + } +} diff --git a/kmp/src/jvmTest/kotlin/dev/stapler/stelekit/ui/fixtures/FakeRepositories.kt b/kmp/src/jvmTest/kotlin/dev/stapler/stelekit/ui/fixtures/FakeRepositories.kt index 17685071..be04592e 100644 --- a/kmp/src/jvmTest/kotlin/dev/stapler/stelekit/ui/fixtures/FakeRepositories.kt +++ b/kmp/src/jvmTest/kotlin/dev/stapler/stelekit/ui/fixtures/FakeRepositories.kt @@ -97,6 +97,9 @@ open class FakePageRepository(initialPages: List = emptyList()) : PageRepo override fun getRecentPages(limit: Int): Flow>> = _pages.map { pages -> pages.values.sortedByDescending { it.updatedAt }.take(limit).right() } + override fun getFavoritePages(): Flow>> = + _pages.map { pages -> pages.values.filter { it.isFavorite }.sortedBy { it.name }.right() } + override fun getUnloadedPages(): Flow>> = _pages.map { pages -> pages.values.filter { !it.isContentLoaded }.right() } From 233cb12d0a36f6d37bda197816245bf75c04d365 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 10 Jun 2026 18:56:03 +0000 Subject: [PATCH 2/5] perf(db): paginate all graph-scale reads with bounded batches and backpressure MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Eliminates the remaining O(graph) query consumers so peak memory during load, reconcile, and background indexing is bounded by chunk size, not graph size — the other half of the 8 000-page Android OOM ("hangs and crashes"). - GraphLoader.loadDirectory: per-chunk IN-clause existence lookups (getPagesByNames / getJournalPagesByDates, ≤500 per IN list for Android's SQLITE_MAX_VARIABLE_NUMBER) replace the getAllPages() full-table preload that held every Page plus two full-size maps for the whole load. - GraphLoader.indexRemainingPages: drain loop over the new getUnloadedPages(limit, offset) (batches of 100) replaces the all-at-once unloaded-pages snapshot. An attempted-UUID set advances the drain offset past permanently-failing rows (missing file, parse error, zero-block parse), guaranteeing termination; countUnloadedPages() provides the O(1) progress denominator. - PageNameIndex: collects the new names-only getPageNameEntries() projection (name + is_journal, conflated) instead of getAllPages() — a few hundred KB per rebuild instead of tens of MB of full Page objects with properties maps. - No production code subscribes to getAllPages() anymore; it remains for tests/benchmarks and migration tooling only. CLAUDE.md documents the pagination/backpressure rules per consumer type. Tests (verified to fail against the unpaginated implementation): - LargeGraphWarmStartCrashTest now also asserts zero getAllPages()/unbounded getUnloadedPages() subscriptions and ≤100-row batches across the full 8 030-page warm start + Phase 3 indexing. - GraphLoaderIndexBatchingTest: drains 250 pages in bounded batches; and terminates when 30 pages can never be indexed. - StelekitViewModelCrashReproductionTest: ViewModel holds zero getAllPages() subscriptions; OOM reproduction tests retargeted to the new projection flow. https://claude.ai/code/session_01Avi2KYmMrxLLFCiMBJHwpb --- CLAUDE.md | 15 +- .../dev/stapler/stelekit/db/GraphLoader.kt | 137 ++++++++++++------ .../stapler/stelekit/domain/PageNameIndex.kt | 11 +- .../repository/InMemoryRepositories.kt | 29 ++++ .../stelekit/repository/PageRepository.kt | 63 ++++++++ .../repository/SqlDelightPageRepository.kt | 56 +++++++ .../dev/stapler/stelekit/db/SteleDatabase.sq | 22 +++ .../db/GraphLoaderIndexBatchingTest.kt | 114 +++++++++++++++ .../db/LargeGraphWarmStartCrashTest.kt | 78 +++++++++- .../domain/PageNameIndexResilienceTest.kt | 7 +- .../StelekitViewModelCrashReproductionTest.kt | 25 ++-- .../stelekit/ui/fixtures/FakeRepositories.kt | 24 +++ 12 files changed, 510 insertions(+), 71 deletions(-) create mode 100644 kmp/src/jvmTest/kotlin/dev/stapler/stelekit/db/GraphLoaderIndexBatchingTest.kt diff --git a/CLAUDE.md b/CLAUDE.md index 9b1dec13..f71aa568 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -292,14 +292,19 @@ Rules: - Standing `collect { }` bodies and `stateIn` upstream chains on such scopes are the unguarded vectors — a repository flow's `catchDbError()` does not protect them. - Regression tests: `StelekitViewModelCrashReproductionTest`, `PageNameIndexResilienceTest`, `LargeGraphWarmStartCrashTest` (8 030-page warm start with a recording default uncaught-exception handler). -### Never collect `getAllPages()` from a standing observer +### Graph-scale reads must be paginated, projected, or chunked — never O(graph) -Every DB write invalidates `selectAllPages`, so a standing collector re-materializes the **entire pages table per write burst**. During graph import/reconcile on an 8 000+ page graph this causes GC thrash (UI hang) and `OutOfMemoryError` (crash) on Android. +Every DB write invalidates SQLDelight queries on the written table, so a standing collector of an unbounded query re-materializes its **entire result set per write burst**. During graph import/reconcile on an 8 000+ page graph this causes GC thrash (UI hang) and `OutOfMemoryError` (crash) on Android. No production code path may subscribe to `getAllPages()` or unbounded `getUnloadedPages()`. -- Sidebar/UI observers must use bounded queries: `getFavoritePages()` (dedicated `WHERE is_favorite = 1` query), `getPages(limit, offset)`, `getPageByUuid` point lookups. -- The single sanctioned standing `getAllPages()` consumer is `PageNameIndex` (conflated + `distinctUntilChanged` + 500 ms debounce + `Throwable`-guarded). -- One-shot `getAllPages().first()` during a load phase (e.g. `GraphLoader.loadDirectory`) is acceptable — transient, bounded, released after the phase. +Patterns, by consumer type: +- **Standing UI observers** (sidebar, etc.): bounded queries only — `getFavoritePages()` (`WHERE is_favorite = 1`), `getPages(limit, offset)`, `getPageByUuid` point lookups. +- **Standing whole-graph observers** (e.g. `PageNameIndex`): use a **projection** (`getPageNameEntries()` — name + is_journal only), plus `conflate()` + `distinctUntilChanged()` + debounce as backpressure, plus `Throwable` guards. +- **Bulk reconcile** (`GraphLoader.loadDirectory`): per-chunk `IN`-clause lookups — `getPagesByNames(chunk)` / `getJournalPagesByDates(chunk)` — never a full-table preload. `IN` lists chunked ≤500 (`SQLITE_MAX_VARIABLE_NUMBER` = 999 on Android API < 30). +- **Background indexing** (`GraphLoader.indexRemainingPages`): drain loop over `getUnloadedPages(limit, offset)` (`INDEX_BATCH_SIZE` = 100); offset advances past permanently-failing rows via an attempted-UUID set so the loop is guaranteed to terminate; `countUnloadedPages()` provides the O(1) progress denominator. - Do not pin full-table snapshots in fields (the former `cachedAllPages` pattern is forbidden). +- `getAllPages()` remains only for tests/benchmarks and migration tooling. + +Regression tests: `LargeGraphWarmStartCrashTest` (asserts zero `getAllPages()`/unbounded-`getUnloadedPages()` subscriptions and ≤100-row batches across a full 8 030-page warm start), `GraphLoaderIndexBatchingTest` (bounded drain + termination with permanently-failing pages), `StelekitViewModelCrashReproductionTest.viewmodel_holds_no_getAllPages_subscription`. ### Android Application.onCreate — catch Throwable, not Exception diff --git a/kmp/src/commonMain/kotlin/dev/stapler/stelekit/db/GraphLoader.kt b/kmp/src/commonMain/kotlin/dev/stapler/stelekit/db/GraphLoader.kt index d2430054..e3d17005 100644 --- a/kmp/src/commonMain/kotlin/dev/stapler/stelekit/db/GraphLoader.kt +++ b/kmp/src/commonMain/kotlin/dev/stapler/stelekit/db/GraphLoader.kt @@ -604,54 +604,76 @@ class GraphLoader( backgroundIndexJob = currentCoroutineContext()[Job] PerformanceMonitor.startTrace("indexRemainingPages") try { - val unloadedPages = pageRepository.getUnloadedPages().first().getOrNull() ?: emptyList() - if (unloadedPages.isEmpty()) return + val total = pageRepository.countUnloadedPages().first().getOrNull() ?: 0L + if (total == 0L) return - logger.info("Background indexing ${unloadedPages.size} pages... (${heapSummary()})") + logger.info("Background indexing $total pages... (${heapSummary()})") coroutineScope { - var processed = 0 - val total = unloadedPages.size - - unloadedPages.chunked(10).forEach { chunk -> - val pagesToSave = mutableListOf() - val blocksToSaveByPage = mutableMapOf>() - val pageUuidsToDelete = mutableSetOf() - - chunk.map { page -> - async(backgroundIndexDispatcher) { - if (page.uuid.value in (activePageUuids?.value ?: emptySet())) { - logger.debug("Phase 3: skipping ${page.name} — active edit session") - return@async null + var processed = 0L + // Drain in bounded batches instead of materializing every unloaded Page up + // front (8 000+ objects on a first warm start — an Android OOM contributor). + // Successfully indexed pages leave the unloaded set, so each fetch re-reads + // at a fixed limit. Pages that stay unloaded after an attempt (missing file, + // parse error, active edit session, zero-block parse) are remembered in + // `attempted` — UUID strings only, a few hundred KB worst case — and the + // offset advances past them when they are re-fetched. Termination is + // guaranteed: every iteration either indexes a fresh page or grows the + // offset by the full batch of stuck rows. + val attempted = HashSet() + var offset = 0 + while (true) { + val batch = pageRepository + .getUnloadedPages(INDEX_BATCH_SIZE, offset) + .first().getOrNull().orEmpty() + if (batch.isEmpty()) break + + val fresh = batch.filter { attempted.add(it.uuid.value) } + // Re-fetched rows we already attempted are stuck for this run — move the + // drain window past them so a fetch can never return only stuck rows. + offset += batch.size - fresh.size + if (fresh.isEmpty()) continue + + fresh.chunked(10).forEach { chunk -> + val pagesToSave = mutableListOf() + val blocksToSaveByPage = mutableMapOf>() + val pageUuidsToDelete = mutableSetOf() + + chunk.map { page -> + async(backgroundIndexDispatcher) { + if (page.uuid.value in (activePageUuids?.value ?: emptySet())) { + logger.debug("Phase 3: skipping ${page.name} — active edit session") + return@async null + } + val path = page.filePath ?: resolvePageFilePath(page.name) + if (path == null) return@async null + val content = readFileDecrypted(path) ?: return@async null + try { + parsePageWithoutSaving(path, content, ParseMode.FULL) + } catch (e: CancellationException) { + throw e + } catch (e: Exception) { + logger.warn("Failed to parse file: $path: ${e.message}") + null + } } - val path = page.filePath ?: resolvePageFilePath(page.name) - if (path == null) return@async null - val content = readFileDecrypted(path) ?: return@async null - try { - parsePageWithoutSaving(path, content, ParseMode.FULL) - } catch (e: CancellationException) { - throw e - } catch (e: Exception) { - logger.warn("Failed to parse file: $path: ${e.message}") - null + }.awaitAll().forEach { result -> + if (result != null) { + pagesToSave.add(result.page) + if (result.blocks.isNotEmpty()) { + blocksToSaveByPage[result.page.uuid] = result.blocks.toMutableList() + } + pageUuidsToDelete.add(result.page.uuid) } } - }.awaitAll().forEach { result -> - if (result != null) { - pagesToSave.add(result.page) - if (result.blocks.isNotEmpty()) { - blocksToSaveByPage[result.page.uuid] = result.blocks.toMutableList() - } - pageUuidsToDelete.add(result.page.uuid) + + if (pagesToSave.isNotEmpty() || pageUuidsToDelete.isNotEmpty()) { + flushChunkWritesPreemptible(pagesToSave, pageUuidsToDelete, blocksToSaveByPage) } - } - if (pagesToSave.isNotEmpty() || pageUuidsToDelete.isNotEmpty()) { - flushChunkWritesPreemptible(pagesToSave, pageUuidsToDelete, blocksToSaveByPage) + processed += chunk.size + onProgress("Indexing pages... (${processed.coerceAtMost(total)}/$total)") } - - processed += chunk.size - onProgress("Indexing pages... ($processed/$total)") } } logger.info("Background indexing complete.") @@ -1002,13 +1024,7 @@ class GraphLoader( } } - // Pre-load all existing pages in one query. Replaces one getPageByName DB call per - // file (up to 4 000 round-trips on a warm restart) with a single bulk read whose - // result is shared across all parallel chunks read-only. - val allPages = pageRepository.getAllPages().first().getOrNull() ?: emptyList() - val pagesByName = allPages.associateBy { it.name.lowercase() } - val pagesByJournalDate = allPages.filter { it.journalDate != null } - .associateBy { it.journalDate!! } + val isJournalDir = path.endsWith("/journals") val loadedCount = coroutineScope { var processedCount = 0 @@ -1019,6 +1035,29 @@ class GraphLoader( async(parallelScope.coroutineContext) { PerformanceMonitor.startTrace("processChunk") try { + // Per-chunk bounded existence lookups (one IN query per ≤100 files) + // instead of preloading the entire pages table. The former + // getAllPages() preload materialized every Page object plus two + // full-size maps for the duration of the load — on 8 000+ page + // graphs that contributed to the Android OOM. Peak memory here is + // now O(chunk), independent of graph size. + val chunkTitles = chunk.map { + FileUtils.decodeFileName(it.fileName.stripPageExtension()) + } + val pagesByName = pageRepository + .getPagesByNames(chunkTitles) + .getOrNull().orEmpty() + .associateBy { it.name.lowercase() } + val pagesByJournalDate = if (isJournalDir) { + val dates = chunkTitles.mapNotNull { JournalUtils.parseJournalDate(it) } + pageRepository.getJournalPagesByDates(dates) + .getOrNull().orEmpty() + .filter { it.journalDate != null } + .associateBy { it.journalDate!! } + } else { + emptyMap() + } + val pagesToSave = mutableListOf() val blocksToSaveByPage = mutableMapOf>() val pageUuidsToDelete = mutableSetOf() @@ -1032,7 +1071,7 @@ class GraphLoader( // Skip Logseq-internal file: protocol artifacts (e.g. file%3A..%2F%2F...) if (title.startsWith("file:")) return@count false val name = title - val isJournalFile = path.endsWith("/journals") + val isJournalFile = isJournalDir val existingPage = if (isJournalFile) { val journalDate = JournalUtils.parseJournalDate(title) if (journalDate != null) pagesByJournalDate[journalDate] @@ -1134,6 +1173,10 @@ class GraphLoader( // Timeout for the batch mtime cursor on startup. Two SAF cursor queries should // complete well under 500ms; 2s is a conservative ceiling for slow providers. private const val SHADOW_STARTUP_TIMEOUT_MS = 2_000L + + // Phase 3 drain-batch size: bounds how many unloaded Page rows are materialized at + // once during background indexing, independent of graph size. + private const val INDEX_BATCH_SIZE = 100 } private data class ParseResult( diff --git a/kmp/src/commonMain/kotlin/dev/stapler/stelekit/domain/PageNameIndex.kt b/kmp/src/commonMain/kotlin/dev/stapler/stelekit/domain/PageNameIndex.kt index 2cc78b54..480db139 100644 --- a/kmp/src/commonMain/kotlin/dev/stapler/stelekit/domain/PageNameIndex.kt +++ b/kmp/src/commonMain/kotlin/dev/stapler/stelekit/domain/PageNameIndex.kt @@ -18,8 +18,9 @@ import kotlinx.coroutines.launch /** * Maintains a reactive index of page names for the page-term suggestion feature. - * Listens to [PageRepository.getAllPages] and rebuilds the [AhoCorasickMatcher] - * on a background thread whenever the page set changes. + * Listens to [PageRepository.getPageNameEntries] (a names-only projection — never + * full Page objects) and rebuilds the [AhoCorasickMatcher] on a background thread + * whenever the page set changes. * * Journal pages (date-named entries) are excluded by default to reduce noise from * date strings in block content inadvertently matching journal page names. @@ -70,7 +71,11 @@ class PageNameIndex( init { scope.launch { - pageRepository.getAllPages() + // Names-only projection: a standing observer must never materialize full Page + // objects for the whole graph (properties maps, file paths, timestamps) — on + // 8 000+ page graphs that re-allocation per write burst drove GC thrash and OOM + // on Android. (name, isJournal) pairs are all the matcher needs. + pageRepository.getPageNameEntries() .distinctUntilChanged() // Coalesce rapid bursts (e.g. graph load saving pages one by one) into a // single rebuild. 500 ms matches the block-editor save debounce so a rename diff --git a/kmp/src/commonMain/kotlin/dev/stapler/stelekit/repository/InMemoryRepositories.kt b/kmp/src/commonMain/kotlin/dev/stapler/stelekit/repository/InMemoryRepositories.kt index 5ba39731..c0b741af 100644 --- a/kmp/src/commonMain/kotlin/dev/stapler/stelekit/repository/InMemoryRepositories.kt +++ b/kmp/src/commonMain/kotlin/dev/stapler/stelekit/repository/InMemoryRepositories.kt @@ -580,6 +580,35 @@ class InMemoryPageRepository : PageRepository { } } + override fun getUnloadedPages(limit: Int, offset: Int): Flow>> { + return pages.map { map -> + map.values.filter { !it.isContentLoaded } + .sortedBy { it.uuid.value }.drop(offset).take(limit).right() + } + } + + override fun countUnloadedPages(): Flow> { + return pages.map { map -> map.values.count { !it.isContentLoaded }.toLong().right() } + } + + override fun getPageNameEntries(): Flow>> { + return pages.map { map -> + map.values.map { PageNameEntry(it.name, it.isJournal) }.right() + } + } + + override suspend fun getPagesByNames(names: Collection): Either> { + val lower = names.mapTo(HashSet()) { it.lowercase() } + return pages.value.values.filter { it.name.lowercase() in lower }.right() + } + + override suspend fun getJournalPagesByDates( + dates: Collection, + ): Either> { + val dateSet = dates.toHashSet() + return pages.value.values.filter { it.journalDate != null && it.journalDate in dateSet }.right() + } + override fun getJournalPages(limit: Int, offset: Int): Flow>> { return pages.map { map -> val journals = map.values diff --git a/kmp/src/commonMain/kotlin/dev/stapler/stelekit/repository/PageRepository.kt b/kmp/src/commonMain/kotlin/dev/stapler/stelekit/repository/PageRepository.kt index 7fd022a8..571709b7 100644 --- a/kmp/src/commonMain/kotlin/dev/stapler/stelekit/repository/PageRepository.kt +++ b/kmp/src/commonMain/kotlin/dev/stapler/stelekit/repository/PageRepository.kt @@ -5,8 +5,16 @@ import dev.stapler.stelekit.error.DomainError import dev.stapler.stelekit.model.Page import dev.stapler.stelekit.model.PageUuid import kotlinx.coroutines.flow.Flow +import kotlinx.coroutines.flow.first import kotlinx.coroutines.flow.map +/** + * Lightweight (name, isJournal) projection of a page row. Used by consumers that need + * every page name (e.g. the suggestion index) without materializing full [Page] objects — + * properties maps, file paths and timestamps stay in the database. + */ +data class PageNameEntry(val name: String, val isJournal: Boolean) + interface PageRepository { fun getPageByUuid(uuid: PageUuid): Flow> fun getPageByName(name: String): Flow> @@ -36,8 +44,63 @@ interface PageRepository { */ fun getFavoritePages(): Flow>> = getAllPages().map { either -> either.map { pages -> pages.filter { it.isFavorite } } } + fun getUnloadedPages(): Flow>> + /** + * Bounded batch of not-yet-content-loaded pages in stable (uuid) order — the + * backpressure-friendly variant of [getUnloadedPages] for background indexing. + * Callers drain by re-fetching at a fixed limit and advancing [offset] only past + * rows that permanently fail, so peak memory is O(limit) rather than O(graph). + * + * Default derives from [getUnloadedPages] for in-memory/test backends; SQL-backed + * repositories must override with a dedicated LIMIT/OFFSET query + * (`selectUnloadedPagesPaginated`) — the in-memory fallback here exists only so fakes + * keep compiling. + */ + @Suppress("InMemoryPagination") + fun getUnloadedPages(limit: Int, offset: Int): Flow>> = + getUnloadedPages().map { either -> + either.map { pages -> pages.sortedBy { it.uuid.value }.drop(offset).take(limit) } + } + + /** Count of not-yet-content-loaded pages — O(1) progress denominator for indexing. */ + fun countUnloadedPages(): Flow> = + getUnloadedPages().map { either -> either.map { it.size.toLong() } } + + /** + * Names-only projection of all pages for the suggestion index. Unlike [getAllPages], + * a standing observer of this flow materializes only (name, isJournal) pairs — + * a few hundred KB on an 8 000-page graph instead of tens of MB of full Page objects. + * + * SQL-backed repositories must override with a dedicated two-column query. + */ + fun getPageNameEntries(): Flow>> = + getAllPages().map { either -> + either.map { pages -> pages.map { PageNameEntry(it.name, it.isJournal) } } + } + + /** + * Pages whose names match [names] (case-insensitive). Bounded existence lookup used + * by graph reconcile: one query per file chunk instead of preloading the whole table. + * Implementations must chunk the IN list to ≤500 to respect Android's + * SQLITE_MAX_VARIABLE_NUMBER=999 on API < 30. + */ + suspend fun getPagesByNames(names: Collection): Either> { + if (names.isEmpty()) return Either.Right(emptyList()) + val lower = names.mapTo(HashSet()) { it.lowercase() } + return getAllPages().first().map { pages -> pages.filter { it.name.lowercase() in lower } } + } + + /** Journal pages whose dates match [dates]. Bounded chunk lookup for reconcile. */ + suspend fun getJournalPagesByDates( + dates: Collection, + ): Either> { + if (dates.isEmpty()) return Either.Right(emptyList()) + val dateSet = dates.toHashSet() + return getAllPages().first().map { pages -> pages.filter { it.journalDate in dateSet } } + } + @DirectRepositoryWrite suspend fun savePage(page: Page): Either diff --git a/kmp/src/commonMain/kotlin/dev/stapler/stelekit/repository/SqlDelightPageRepository.kt b/kmp/src/commonMain/kotlin/dev/stapler/stelekit/repository/SqlDelightPageRepository.kt index 477cc3c5..26c619ee 100644 --- a/kmp/src/commonMain/kotlin/dev/stapler/stelekit/repository/SqlDelightPageRepository.kt +++ b/kmp/src/commonMain/kotlin/dev/stapler/stelekit/repository/SqlDelightPageRepository.kt @@ -31,6 +31,11 @@ class SqlDelightPageRepository( private val cacheWrites: Boolean = true, ) : PageRepository { + private companion object { + // Safe IN-clause size: SQLITE_MAX_VARIABLE_NUMBER is 999 on Android API < 30. + const val IN_CLAUSE_CHUNK_SIZE = 500 + } + private val queries = database.steleDatabaseQueries private val cacheConfig = RepoCacheConfig.fromPlatform() @@ -127,6 +132,57 @@ class SqlDelightPageRepository( queries.selectUnloadedPages() .asDbFlowList(PlatformDispatcher.DB) { it.toModel() } + override fun getUnloadedPages(limit: Int, offset: Int): Flow>> = + queries.selectUnloadedPagesPaginated(limit.toLong(), offset.toLong()) + .asDbFlowList(PlatformDispatcher.DB) { it.toModel() } + + override fun countUnloadedPages(): Flow> = flow { + try { + emit(queries.countUnloadedPages().executeAsOne().right()) + } catch (e: CancellationException) { + throw e + } catch (e: Exception) { + emit(DomainError.DatabaseError.ReadFailed(e.message ?: "unknown").left()) + } + }.flowOn(PlatformDispatcher.DB) + + override fun getPageNameEntries(): Flow>> = + queries.selectPageNameEntries() + .asFlow() + .conflate() // drop intermediate invalidations during bulk import — standing observer (PageNameIndex) + .mapToList(PlatformDispatcher.DB) + .map { rows -> rows.map { PageNameEntry(it.name, it.is_journal == 1L) }.right() } + .catchDbError() + + override suspend fun getPagesByNames(names: Collection): Either> = + withContext(PlatformDispatcher.DB) { + try { + // Chunk the IN list: SQLITE_MAX_VARIABLE_NUMBER is 999 on Android API < 30. + names.chunked(IN_CLAUSE_CHUNK_SIZE).flatMap { chunk -> + queries.selectPagesByNames(chunk).executeAsList().map { it.toModel() } + }.right() + } catch (e: CancellationException) { + throw e + } catch (e: Exception) { + DomainError.DatabaseError.ReadFailed(e.message ?: "unknown").left() + } + } + + override suspend fun getJournalPagesByDates( + dates: Collection, + ): Either> = withContext(PlatformDispatcher.DB) { + try { + dates.chunked(IN_CLAUSE_CHUNK_SIZE).flatMap { chunk -> + queries.selectJournalPagesByDates(chunk.map { it.toString() }) + .executeAsList().map { it.toModel() } + }.right() + } catch (e: CancellationException) { + throw e + } catch (e: Exception) { + DomainError.DatabaseError.ReadFailed(e.message ?: "unknown").left() + } + } + override suspend fun savePage(page: Page): Either = withContext(PlatformDispatcher.DB) { try { upsertPage(page) diff --git a/kmp/src/commonMain/sqldelight/dev/stapler/stelekit/db/SteleDatabase.sq b/kmp/src/commonMain/sqldelight/dev/stapler/stelekit/db/SteleDatabase.sq index 067bfaa1..73e16036 100644 --- a/kmp/src/commonMain/sqldelight/dev/stapler/stelekit/db/SteleDatabase.sq +++ b/kmp/src/commonMain/sqldelight/dev/stapler/stelekit/db/SteleDatabase.sq @@ -322,6 +322,28 @@ SELECT * FROM pages ORDER BY name LIMIT ? OFFSET ?; selectUnloadedPages: SELECT * FROM pages WHERE is_content_loaded = 0; +-- Bounded batch for background indexing. ORDER BY uuid gives a stable drain order so a +-- caller can advance OFFSET past permanently-failing rows without re-fetching them forever. +selectUnloadedPagesPaginated: +SELECT * FROM pages WHERE is_content_loaded = 0 ORDER BY uuid LIMIT ? OFFSET ?; + +countUnloadedPages: +SELECT COUNT(*) FROM pages WHERE is_content_loaded = 0; + +-- Names-only projection for the page-name suggestion index: no properties/file_path/etc. +-- materialization, so a standing observer stays cheap even on 8 000+ page graphs. +selectPageNameEntries: +SELECT name, is_journal FROM pages; + +-- Per-chunk existence lookups for graph reconcile (bounded IN list, caller chunks ≤500 +-- to stay under SQLITE_MAX_VARIABLE_NUMBER=999 on Android API < 30). +-- name is COLLATE NOCASE so matching is case-insensitive. +selectPagesByNames: +SELECT * FROM pages WHERE name IN ?; + +selectJournalPagesByDates: +SELECT * FROM pages WHERE is_journal = 1 AND journal_date IN ?; + selectFavoritePages: SELECT * FROM pages WHERE is_favorite = 1 ORDER BY name; diff --git a/kmp/src/jvmTest/kotlin/dev/stapler/stelekit/db/GraphLoaderIndexBatchingTest.kt b/kmp/src/jvmTest/kotlin/dev/stapler/stelekit/db/GraphLoaderIndexBatchingTest.kt new file mode 100644 index 00000000..0ad0b30b --- /dev/null +++ b/kmp/src/jvmTest/kotlin/dev/stapler/stelekit/db/GraphLoaderIndexBatchingTest.kt @@ -0,0 +1,114 @@ +package dev.stapler.stelekit.db + +import arrow.core.Either +import dev.stapler.stelekit.error.DomainError +import dev.stapler.stelekit.model.Page +import dev.stapler.stelekit.model.PageUuid +import dev.stapler.stelekit.ui.fixtures.FakeBlockRepository +import dev.stapler.stelekit.ui.fixtures.FakeFileSystem +import dev.stapler.stelekit.ui.fixtures.FakePageRepository +import kotlinx.coroutines.flow.Flow +import kotlinx.coroutines.flow.first +import kotlinx.coroutines.runBlocking +import kotlinx.coroutines.withTimeout +import java.util.concurrent.atomic.AtomicInteger +import kotlin.math.max +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertTrue +import kotlin.time.Instant + +/** + * Phase 3 background indexing must drain unloaded pages in bounded batches (backpressure) + * instead of materializing the entire unloaded set up front — on a first warm start of an + * 8 000+ page graph the old snapshot read held every unloaded Page in memory at once. + * + * The drain loop must also terminate when some pages can never be indexed (missing file, + * unreadable content): the offset advances past permanent failures, so a batch can never + * consist solely of already-failed rows fetched forever. + */ +class GraphLoaderIndexBatchingTest { + + private companion object { + const val GRAPH_PATH = "/tmp/index-batch-graph" + } + + private class RecordingRepository(initial: List) : FakePageRepository(initial) { + val maxBatchLimit = AtomicInteger(0) + val boundedFetches = AtomicInteger(0) + val unboundedSubscriptions = AtomicInteger(0) + + override fun getUnloadedPages(): Flow>> { + unboundedSubscriptions.incrementAndGet() + return super.getUnloadedPages() + } + + override fun getUnloadedPages(limit: Int, offset: Int): Flow>> { + boundedFetches.incrementAndGet() + maxBatchLimit.updateAndGet { max(it, limit) } + return super.getUnloadedPages(limit, offset) + } + } + + /** Files whose name contains "broken" are unreadable (readFile returns null). */ + private class PartiallyBrokenFileSystem : FakeFileSystem() { + override fun directoryExists(path: String): Boolean = true + override fun getLastModifiedTime(path: String): Long = 1_000L + override fun readFile(path: String): String? { + if (path.contains("broken")) return null + val name = path.substringAfterLast("/").removeSuffix(".md") + return "- content of $name" + } + } + + private fun unloadedPage(name: String): Page = Page( + uuid = PageUuid("uuid-$name"), + name = name, + filePath = "$GRAPH_PATH/pages/$name.md", + createdAt = Instant.fromEpochMilliseconds(0), + updatedAt = Instant.fromEpochMilliseconds(0), + isContentLoaded = false, + ) + + @Test + fun indexing_drains_in_bounded_batches_and_loads_everything() = runBlocking { + val pages = (1..250).map { unloadedPage("note$it") } + val pageRepo = RecordingRepository(pages) + val loader = GraphLoader(PartiallyBrokenFileSystem(), pageRepo, FakeBlockRepository()) + loader.setGraphPath(GRAPH_PATH) + + withTimeout(120_000) { loader.indexRemainingPages { } } + + val remaining = pageRepo.countUnloadedPages().first().getOrNull() ?: -1L + assertEquals(0L, remaining, "all 250 pages must be indexed") + assertTrue( + pageRepo.maxBatchLimit.get() in 1..100, + "unloaded pages must be fetched in bounded batches (≤100); max limit was " + + "${pageRepo.maxBatchLimit.get()}" + ) + assertTrue( + pageRepo.boundedFetches.get() >= 3, + "250 pages at ≤100 per batch needs ≥3 fetches; got ${pageRepo.boundedFetches.get()}" + ) + assertEquals( + 0, pageRepo.unboundedSubscriptions.get(), + "the unbounded getUnloadedPages() snapshot must not be used" + ) + } + + @Test + fun indexing_terminates_when_some_pages_permanently_fail() = runBlocking { + val pages = (1..90).map { unloadedPage("note$it") } + + (1..30).map { unloadedPage("broken$it") } + val pageRepo = RecordingRepository(pages) + val loader = GraphLoader(PartiallyBrokenFileSystem(), pageRepo, FakeBlockRepository()) + loader.setGraphPath(GRAPH_PATH) + + // Must terminate despite 30 pages that can never leave the unloaded set — + // the drain offset has to advance past them instead of refetching them forever. + withTimeout(60_000) { loader.indexRemainingPages { } } + + val remaining = pageRepo.countUnloadedPages().first().getOrNull() ?: -1L + assertEquals(30L, remaining, "exactly the 30 unreadable pages must remain unloaded") + } +} diff --git a/kmp/src/jvmTest/kotlin/dev/stapler/stelekit/db/LargeGraphWarmStartCrashTest.kt b/kmp/src/jvmTest/kotlin/dev/stapler/stelekit/db/LargeGraphWarmStartCrashTest.kt index 5e6fd7df..c43545a2 100644 --- a/kmp/src/jvmTest/kotlin/dev/stapler/stelekit/db/LargeGraphWarmStartCrashTest.kt +++ b/kmp/src/jvmTest/kotlin/dev/stapler/stelekit/db/LargeGraphWarmStartCrashTest.kt @@ -18,8 +18,15 @@ import kotlinx.coroutines.delay import kotlinx.coroutines.flow.first import kotlinx.coroutines.runBlocking import kotlinx.coroutines.withTimeout +import arrow.core.Either +import dev.stapler.stelekit.error.DomainError +import kotlinx.coroutines.flow.Flow +import kotlinx.coroutines.flow.emitAll +import kotlinx.coroutines.flow.flow import kotlinx.datetime.LocalDate import java.util.concurrent.CopyOnWriteArrayList +import java.util.concurrent.atomic.AtomicInteger +import kotlin.math.max import kotlin.test.Test import kotlin.test.assertTrue import kotlin.time.Clock @@ -58,6 +65,48 @@ class LargeGraphWarmStartCrashTest { } } + /** + * Records query-boundedness during the run: the whole pipeline must never subscribe to + * the unbounded getAllPages()/getUnloadedPages() reads, and every batch lookup must be + * chunk-sized — that is what keeps peak memory O(chunk) instead of O(graph). + */ + private class BoundedQueryRecordingRepository( + initial: List, + ) : FakePageRepository(initial) { + val getAllPagesSubscriptions = AtomicInteger(0) + val unboundedUnloadedSubscriptions = AtomicInteger(0) + val maxNamesPerLookup = AtomicInteger(0) + val maxUnloadedBatchLimit = AtomicInteger(0) + val boundedUnloadedFetches = AtomicInteger(0) + + override fun getAllPages(): Flow>> { + val base = super.getAllPages() + return flow { + getAllPagesSubscriptions.incrementAndGet() + emitAll(base) + } + } + + override fun getUnloadedPages(): Flow>> { + val base = super.getUnloadedPages() + return flow { + unboundedUnloadedSubscriptions.incrementAndGet() + emitAll(base) + } + } + + override fun getUnloadedPages(limit: Int, offset: Int): Flow>> { + boundedUnloadedFetches.incrementAndGet() + maxUnloadedBatchLimit.updateAndGet { max(it, limit) } + return super.getUnloadedPages(limit, offset) + } + + override suspend fun getPagesByNames(names: Collection): Either> { + maxNamesPerLookup.updateAndGet { max(it, names.size) } + return super.getPagesByNames(names) + } + } + /** Serves [PAGE_COUNT] synthetic markdown pages from the pages/ directory. */ private class LargeGraphFileSystem : FakeFileSystem() { private val pageFiles = (1..PAGE_COUNT).map { "Test Page $it.md" } @@ -114,7 +163,7 @@ class LargeGraphWarmStartCrashTest { } } - val pageRepo = FakePageRepository(initialPages) + val pageRepo = BoundedQueryRecordingRepository(initialPages) val blockRepo = FakeBlockRepository() val fileSystem = LargeGraphFileSystem() val settings = InMemorySettings().apply { @@ -172,6 +221,33 @@ class LargeGraphWarmStartCrashTest { pageCount >= PAGE_COUNT, "All $PAGE_COUNT pages must survive the warm reconcile; found $pageCount" ) + + // ── Query boundedness: peak memory must stay O(chunk), not O(graph) ───── + assertTrue( + pageRepo.getAllPagesSubscriptions.get() == 0, + "getAllPages() must never be subscribed during load — found " + + "${pageRepo.getAllPagesSubscriptions.get()} subscription(s); full-table " + + "reads re-materialize all $PAGE_COUNT pages and OOM Android" + ) + assertTrue( + pageRepo.unboundedUnloadedSubscriptions.get() == 0, + "Unbounded getUnloadedPages() must not be used — Phase 3 must drain in batches" + ) + assertTrue( + pageRepo.maxNamesPerLookup.get() in 1..100, + "Reconcile name lookups must be chunk-sized (≤100); max was " + + "${pageRepo.maxNamesPerLookup.get()}" + ) + assertTrue( + pageRepo.maxUnloadedBatchLimit.get() in 1..100, + "Phase 3 must fetch unloaded pages in bounded batches (≤100); max limit was " + + "${pageRepo.maxUnloadedBatchLimit.get()}" + ) + assertTrue( + pageRepo.boundedUnloadedFetches.get() > 1, + "Phase 3 over $PAGE_COUNT pages must take multiple bounded fetches; " + + "got ${pageRepo.boundedUnloadedFetches.get()}" + ) } } } diff --git a/kmp/src/jvmTest/kotlin/dev/stapler/stelekit/domain/PageNameIndexResilienceTest.kt b/kmp/src/jvmTest/kotlin/dev/stapler/stelekit/domain/PageNameIndexResilienceTest.kt index 63cfd111..be6e88f9 100644 --- a/kmp/src/jvmTest/kotlin/dev/stapler/stelekit/domain/PageNameIndexResilienceTest.kt +++ b/kmp/src/jvmTest/kotlin/dev/stapler/stelekit/domain/PageNameIndexResilienceTest.kt @@ -5,6 +5,7 @@ import arrow.core.right import dev.stapler.stelekit.error.DomainError import dev.stapler.stelekit.model.Page import dev.stapler.stelekit.model.PageUuid +import dev.stapler.stelekit.repository.PageNameEntry import dev.stapler.stelekit.ui.fixtures.FakePageRepository import kotlinx.coroutines.CompletableDeferred import kotlinx.coroutines.CoroutineScope @@ -56,12 +57,12 @@ class PageNameIndexResilienceTest { private val pages: List, ) : FakePageRepository() { val oomThrown = CompletableDeferred() - override fun getAllPages(): Flow>> = flow { - emit(pages.right()) + override fun getPageNameEntries(): Flow>> = flow { + emit(pages.map { PageNameEntry(it.name, it.isJournal) }.right()) // Leave time for the debounced rebuild to consume the first emission. delay(400) oomThrown.complete(Unit) - throw OutOfMemoryError("simulated OOM re-materializing the page list") + throw OutOfMemoryError("simulated OOM re-materializing the page-name list") } } diff --git a/kmp/src/jvmTest/kotlin/dev/stapler/stelekit/ui/StelekitViewModelCrashReproductionTest.kt b/kmp/src/jvmTest/kotlin/dev/stapler/stelekit/ui/StelekitViewModelCrashReproductionTest.kt index 75e838d5..acafcc46 100644 --- a/kmp/src/jvmTest/kotlin/dev/stapler/stelekit/ui/StelekitViewModelCrashReproductionTest.kt +++ b/kmp/src/jvmTest/kotlin/dev/stapler/stelekit/ui/StelekitViewModelCrashReproductionTest.kt @@ -84,15 +84,15 @@ class StelekitViewModelCrashReproductionTest { // ─── TC-CRASH-001: OOM surfacing through the page-list flow ───────────────── /** - * Simulates an OutOfMemoryError surfacing while the full page list is materialized + * Simulates an OutOfMemoryError surfacing while the page-name list is materialized * for a standing observer (the allocation that fails first on 8 000-page graphs). */ private class OomPageFlowRepository : FakePageRepository() { val oomThrown = CompletableDeferred() - override fun getAllPages(): Flow>> = flow { - emit(emptyList().right()) + override fun getPageNameEntries(): Flow>> = flow { + emit(emptyList().right()) oomThrown.complete(Unit) - throw OutOfMemoryError("simulated allocation failure materializing 8030 pages") + throw OutOfMemoryError("simulated allocation failure materializing 8030 page names") } } @@ -177,12 +177,13 @@ class StelekitViewModelCrashReproductionTest { /** * The hang half of "hangs and crashes": every DB write invalidates getAllPages(), so * each standing collector re-materializes the entire table per write burst during - * import/reconcile — O(N) Page allocations × thousands of writes = GC thrash and OOM. - * Exactly one standing subscription is allowed: PageNameIndex (debounced + conflated). - * The sidebar (favorites/recents) must use bounded queries instead. + * import/reconcile — O(N) full Page allocations × thousands of writes = GC thrash and + * OOM. No production observer may subscribe to getAllPages() at all: the sidebar uses + * bounded queries (favorites, point lookups) and PageNameIndex uses the names-only + * getPageNameEntries() projection. */ @Test - fun viewmodel_keeps_at_most_one_standing_getAllPages_subscription() = runBlocking { + fun viewmodel_holds_no_getAllPages_subscription() = runBlocking { val repo = CountingPageRepository() makeViewModel(repo) @@ -191,10 +192,10 @@ class StelekitViewModelCrashReproductionTest { val subscriptions = repo.getAllPagesSubscriptions.get() assertTrue( - subscriptions <= 1, - "Expected at most 1 standing getAllPages() subscription (PageNameIndex), " + - "found $subscriptions — extra full-table observers re-materialize all pages " + - "on every write and OOM Android on 8 000+ page graphs" + subscriptions == 0, + "Expected no getAllPages() subscription from the ViewModel, found $subscriptions — " + + "full-table observers re-materialize all pages on every write and OOM Android " + + "on 8 000+ page graphs" ) } } diff --git a/kmp/src/jvmTest/kotlin/dev/stapler/stelekit/ui/fixtures/FakeRepositories.kt b/kmp/src/jvmTest/kotlin/dev/stapler/stelekit/ui/fixtures/FakeRepositories.kt index be04592e..e16172cc 100644 --- a/kmp/src/jvmTest/kotlin/dev/stapler/stelekit/ui/fixtures/FakeRepositories.kt +++ b/kmp/src/jvmTest/kotlin/dev/stapler/stelekit/ui/fixtures/FakeRepositories.kt @@ -103,6 +103,30 @@ open class FakePageRepository(initialPages: List = emptyList()) : PageRepo override fun getUnloadedPages(): Flow>> = _pages.map { pages -> pages.values.filter { !it.isContentLoaded }.right() } + override fun getUnloadedPages(limit: Int, offset: Int): Flow>> = + _pages.map { pages -> + pages.values.filter { !it.isContentLoaded } + .sortedBy { it.uuid.value }.drop(offset).take(limit).right() + } + + override fun countUnloadedPages(): Flow> = + _pages.map { pages -> pages.values.count { !it.isContentLoaded }.toLong().right() } + + override fun getPageNameEntries(): Flow>> = + _pages.map { pages -> + pages.values.map { dev.stapler.stelekit.repository.PageNameEntry(it.name, it.isJournal) }.right() + } + + override suspend fun getPagesByNames(names: Collection): Either> { + val lower = names.mapTo(HashSet()) { it.lowercase() } + return _pages.value.values.filter { it.name.lowercase() in lower }.right() + } + + override suspend fun getJournalPagesByDates(dates: Collection): Either> { + val dateSet = dates.toHashSet() + return _pages.value.values.filter { it.journalDate != null && it.journalDate in dateSet }.right() + } + override suspend fun savePage(page: Page): Either { _pages.value = _pages.value + (page.uuid.value to page) return Unit.right() From 6295c832684371e995ac460690ba24e206e0bcb0 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 11 Jun 2026 02:05:32 +0000 Subject: [PATCH 3/5] =?UTF-8?q?refactor(db):=20remove=20unbounded=20getAll?= =?UTF-8?q?Pages/getUnloadedPages=20=E2=80=94=20enforce=20pagination=20at?= =?UTF-8?q?=20compile=20time?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Now that no code path needs a full-table read, delete the unbounded queries entirely so reintroducing the 8 000-page Android OOM pattern is a compile error, not a code-review catch: - PageRepository no longer declares getAllPages() or unbounded getUnloadedPages(); selectAllPages / selectUnloadedPages are removed from SteleDatabase.sq. The interface KDoc and CLAUDE.md document the bounded query menu per consumer type. - Whole-graph one-shots (export, migration tooling, benchmarks, tests) use the new getAllPagesSnapshot(batchSize=500) interface method, which pages through getPages(limit, offset) in bounded batches — even "fetch everything" callers never run an unpaginated query or hold a reactive full-table flow. - Remaining production callers migrated to bounded reads: - AllPagesViewModel: standing getAllPages() collector (a live OOM vector while indexing runs with the All Pages screen open) becomes a one-shot bounded-batch snapshot; refresh() reloads on demand. - ImportViewModel: suggestion-dedup uses the names-only getPageNameEntries() projection; URL dedup uses the snapshot. - GlobalUnlinkedReferencesViewModel: names-only projection. - ExportService.exportJournalRange: bounded pagination over getJournalPages (journal_date DESC) with early stop below the range. - InMemorySearchRepository (test backend): snapshot instead of full flow. - DatalogPageRepository, commonTest fakes, and InstrumentedPageRepository implement the bounded query set; the instrumentation wrapper delegates explicitly so the SQL-optimized chunked lookups are preserved. - QueryPlanAuditTest audits the new bounded queries; UpgradeResilienceTest now exercises the closed-DB guard on every new read (favorites, name entries, paginated unloaded, counts, snapshot). - Runtime "zero unbounded subscriptions" test assertions removed — the guarantee is now structural. https://claude.ai/code/session_01Avi2KYmMrxLLFCiMBJHwpb --- CLAUDE.md | 6 +- .../benchmark/AndroidGraphBenchmark.kt | 4 +- .../voice/VoiceNoteBlockFormatTest.kt | 2 +- .../stapler/stelekit/export/ExportService.kt | 30 +++-- .../stelekit/migration/DslEvaluator.kt | 2 +- .../repository/DatalogPageRepository.kt | 15 ++- .../repository/InMemoryRepositories.kt | 29 ++--- .../stelekit/repository/PageRepository.kt | 116 ++++++++++++------ .../repository/SqlDelightPageRepository.kt | 12 -- .../stelekit/ui/screens/AllPagesViewModel.kt | 58 ++++----- .../GlobalUnlinkedReferencesViewModel.kt | 3 +- .../stelekit/ui/screens/ImportViewModel.kt | 10 +- .../dev/stapler/stelekit/db/SteleDatabase.sq | 11 +- .../stelekit/repository/JournalServiceTest.kt | 4 +- .../ui/screens/JournalsViewModelEditorTest.kt | 10 +- .../ui/screens/JournalsViewModelTest.kt | 9 +- .../performance/InstrumentedPageRepository.kt | 24 +++- .../stelekit/benchmark/GraphLoadTimingTest.kt | 6 +- .../benchmark/UserSessionBenchmarkTest.kt | 4 +- .../db/AppLoadJournalIntegrationTest.kt | 28 ++--- .../stelekit/db/DemoGraphIntegrationTest.kt | 2 +- .../stelekit/db/ExternalChangeConflictTest.kt | 6 +- .../stelekit/db/GraphLoaderCacheTest.kt | 14 +-- .../db/GraphLoaderIndexBatchingTest.kt | 10 -- .../stelekit/db/GraphLoaderIntegrationTest.kt | 6 +- .../stapler/stelekit/db/GraphLoaderTest.kt | 4 +- .../stelekit/db/GraphLoaderWatcherTest.kt | 4 +- .../db/GraphManagerDatabaseLifecycleTest.kt | 4 +- .../db/LargeGraphWarmStartCrashTest.kt | 34 +---- .../stapler/stelekit/db/QueryPlanAuditTest.kt | 23 +++- .../integration/PipelineReproductionTest.kt | 2 +- .../stapler/stelekit/migration/DryRunTest.kt | 4 +- .../RepositoryFlowResilienceTest.kt | 2 +- .../repository/UpgradeResilienceTest.kt | 19 ++- .../StelekitViewModelCrashReproductionTest.kt | 43 +------ .../stelekit/ui/fixtures/FakeRepositories.kt | 6 - 36 files changed, 285 insertions(+), 281 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index f71aa568..5b4274df 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -294,17 +294,17 @@ Rules: ### Graph-scale reads must be paginated, projected, or chunked — never O(graph) -Every DB write invalidates SQLDelight queries on the written table, so a standing collector of an unbounded query re-materializes its **entire result set per write burst**. During graph import/reconcile on an 8 000+ page graph this causes GC thrash (UI hang) and `OutOfMemoryError` (crash) on Android. No production code path may subscribe to `getAllPages()` or unbounded `getUnloadedPages()`. +Every DB write invalidates SQLDelight queries on the written table, so a standing collector of an unbounded query re-materializes its **entire result set per write burst**. During graph import/reconcile on an 8 000+ page graph this causes GC thrash (UI hang) and `OutOfMemoryError` (crash) on Android. **`PageRepository` therefore has no `getAllPages()` / unbounded `getUnloadedPages()` at all — the absence is compile-time enforced.** Do not add unbounded reads back to any repository interface. Patterns, by consumer type: - **Standing UI observers** (sidebar, etc.): bounded queries only — `getFavoritePages()` (`WHERE is_favorite = 1`), `getPages(limit, offset)`, `getPageByUuid` point lookups. - **Standing whole-graph observers** (e.g. `PageNameIndex`): use a **projection** (`getPageNameEntries()` — name + is_journal only), plus `conflate()` + `distinctUntilChanged()` + debounce as backpressure, plus `Throwable` guards. - **Bulk reconcile** (`GraphLoader.loadDirectory`): per-chunk `IN`-clause lookups — `getPagesByNames(chunk)` / `getJournalPagesByDates(chunk)` — never a full-table preload. `IN` lists chunked ≤500 (`SQLITE_MAX_VARIABLE_NUMBER` = 999 on Android API < 30). - **Background indexing** (`GraphLoader.indexRemainingPages`): drain loop over `getUnloadedPages(limit, offset)` (`INDEX_BATCH_SIZE` = 100); offset advances past permanently-failing rows via an attempted-UUID set so the loop is guaranteed to terminate; `countUnloadedPages()` provides the O(1) progress denominator. +- **Whole-graph one-shots** (export, migration tooling, benchmarks, tests): `getAllPagesSnapshot()` — a suspend interface method that pages through `getPages(limit, offset)` in bounded batches (never a single unbounded query, never a reactive flow). - Do not pin full-table snapshots in fields (the former `cachedAllPages` pattern is forbidden). -- `getAllPages()` remains only for tests/benchmarks and migration tooling. -Regression tests: `LargeGraphWarmStartCrashTest` (asserts zero `getAllPages()`/unbounded-`getUnloadedPages()` subscriptions and ≤100-row batches across a full 8 030-page warm start), `GraphLoaderIndexBatchingTest` (bounded drain + termination with permanently-failing pages), `StelekitViewModelCrashReproductionTest.viewmodel_holds_no_getAllPages_subscription`. +Regression tests: `LargeGraphWarmStartCrashTest` (asserts ≤100-row batches across a full 8 030-page warm start), `GraphLoaderIndexBatchingTest` (bounded drain + termination with permanently-failing pages), `QueryPlanAuditTest` (audits query plans for the bounded query set). ### Android Application.onCreate — catch Throwable, not Exception diff --git a/kmp/src/androidInstrumentedTest/kotlin/dev/stapler/stelekit/benchmark/AndroidGraphBenchmark.kt b/kmp/src/androidInstrumentedTest/kotlin/dev/stapler/stelekit/benchmark/AndroidGraphBenchmark.kt index 0f4bdb98..51ad250d 100644 --- a/kmp/src/androidInstrumentedTest/kotlin/dev/stapler/stelekit/benchmark/AndroidGraphBenchmark.kt +++ b/kmp/src/androidInstrumentedTest/kotlin/dev/stapler/stelekit/benchmark/AndroidGraphBenchmark.kt @@ -83,7 +83,7 @@ class AndroidGraphBenchmark { val phase3Ms = measureTime { loader.indexRemainingPages {} }.inWholeMilliseconds - val pageCount = repoSet.pageRepository.getAllPages().first().getOrNull()?.size ?: 0 + val pageCount = repoSet.pageRepository.getAllPagesSnapshot().getOrNull()?.size ?: 0 android.util.Log.i("ANDROID_BENCH", """{"metric":"loadPhase","phase1Ms":$phase1Ms,"phase3Ms":$phase3Ms,"pageCount":$pageCount}""") @@ -205,7 +205,7 @@ class AndroidGraphBenchmark { loader.indexRemainingPages {} // Pick a journal page to edit - val pages = repoSet.pageRepository.getAllPages().first().getOrNull() ?: emptyList() + val pages = repoSet.pageRepository.getAllPagesSnapshot().getOrNull() ?: emptyList() val journalPage = pages.firstOrNull { it.isJournal } ?: pages.first() val pageUuid = journalPage.uuid diff --git a/kmp/src/businessTest/kotlin/dev/stapler/stelekit/voice/VoiceNoteBlockFormatTest.kt b/kmp/src/businessTest/kotlin/dev/stapler/stelekit/voice/VoiceNoteBlockFormatTest.kt index 1bbca8b2..01e96e0b 100644 --- a/kmp/src/businessTest/kotlin/dev/stapler/stelekit/voice/VoiceNoteBlockFormatTest.kt +++ b/kmp/src/businessTest/kotlin/dev/stapler/stelekit/voice/VoiceNoteBlockFormatTest.kt @@ -204,7 +204,7 @@ class VoiceNoteBlockFormatTest { "#+BEGIN_QUOTE must not appear in inline block") // Verify no transcript page was created (below threshold) - val allPages = pageRepo.getAllPages().first().getOrNull().orEmpty() + val allPages = pageRepo.getAllPagesSnapshot().getOrNull().orEmpty() val transcriptPages = allPages.filter { it.name.startsWith("Voice Note ") } assertTrue(transcriptPages.isEmpty(), "Expected no Voice Note transcript page for short (below-threshold) note") diff --git a/kmp/src/commonMain/kotlin/dev/stapler/stelekit/export/ExportService.kt b/kmp/src/commonMain/kotlin/dev/stapler/stelekit/export/ExportService.kt index cbf2ce54..f680c09e 100644 --- a/kmp/src/commonMain/kotlin/dev/stapler/stelekit/export/ExportService.kt +++ b/kmp/src/commonMain/kotlin/dev/stapler/stelekit/export/ExportService.kt @@ -227,7 +227,8 @@ class ExportService( /** * Exports all journal pages in the date range [[from], [to]] (inclusive) as a single string. * - * Uses [pageRepo.getAllPages()] and filters in memory (ADR-4). Empty days are skipped. + * Pages through [PageRepository.getJournalPages] (journal_date DESC) in bounded batches, + * stopping early once dates fall before [from]. Empty days are skipped. * Pages within the range are sorted by date ascending and separated by a `## date` heading. * * @return [Left] with [ExportError.SerializationFailed] if no journal pages are in range. @@ -243,15 +244,26 @@ class ExportService( val exporter = exporterMap[formatId] ?: return@withContext ExportError.SerializationFailed("Unknown export format: $formatId").left() - val allPages = pageRepo.getAllPages().first().getOrNull() ?: emptyList() - - // Filter to journal pages whose date falls in [from, to] - val journalPages = allPages - .filter { page -> - val date = page.journalDate ?: return@filter false - date in from..to + // Bounded-batch pagination over journals (ordered journal_date DESC) with early + // stop below the range — never a full-table read. + val inRange = mutableListOf() + var offset = 0 + val batchSize = 200 + while (true) { + val batch = pageRepo.getJournalPages(batchSize, offset).first().getOrNull() ?: emptyList() + if (batch.isEmpty()) break + for (page in batch) { + val date = page.journalDate ?: continue + if (date in from..to) inRange.add(page) } - .sortedBy { it.journalDate } + // DESC ordering: once the oldest date in the batch is before the range start, + // no later batch can contain in-range pages. + val oldest = batch.lastOrNull()?.journalDate + if (oldest != null && oldest < from) break + if (batch.size < batchSize) break + offset += batch.size + } + val journalPages = inRange.sortedBy { it.journalDate } if (journalPages.isEmpty()) { return@withContext ExportError.SerializationFailed( diff --git a/kmp/src/commonMain/kotlin/dev/stapler/stelekit/migration/DslEvaluator.kt b/kmp/src/commonMain/kotlin/dev/stapler/stelekit/migration/DslEvaluator.kt index 5e75869e..e9e726f2 100644 --- a/kmp/src/commonMain/kotlin/dev/stapler/stelekit/migration/DslEvaluator.kt +++ b/kmp/src/commonMain/kotlin/dev/stapler/stelekit/migration/DslEvaluator.kt @@ -35,7 +35,7 @@ class DslEvaluator(private val repoSet: RepositorySet) { suspend fun evaluate(migration: Migration): List { // Pre-fetch all pages and their blocks so the sync forBlocks/forPages methods // don't need to call runBlocking themselves. - val allPages = repoSet.pageRepository.getAllPages().first().getOrNull() ?: emptyList() + val allPages = repoSet.pageRepository.getAllPagesSnapshot().getOrNull() ?: emptyList() val blocksByPage: Map> = allPages.associate { page -> page.uuid to (repoSet.blockRepository.getBlocksForPage(page.uuid) .first().getOrNull() ?: emptyList()) diff --git a/kmp/src/commonMain/kotlin/dev/stapler/stelekit/repository/DatalogPageRepository.kt b/kmp/src/commonMain/kotlin/dev/stapler/stelekit/repository/DatalogPageRepository.kt index 75709a31..4ad0e7b3 100644 --- a/kmp/src/commonMain/kotlin/dev/stapler/stelekit/repository/DatalogPageRepository.kt +++ b/kmp/src/commonMain/kotlin/dev/stapler/stelekit/repository/DatalogPageRepository.kt @@ -69,9 +69,15 @@ class DatalogPageRepository : PageRepository { } } - override fun getAllPages(): Flow>> { + override fun getFavoritePages(): Flow>> { return pages.map { map -> - map.values.toList().right() + map.values.filter { it.isFavorite }.sortedBy { it.name }.right() + } + } + + override fun getPageNameEntries(): Flow>> { + return pages.map { map -> + map.values.map { PageNameEntry(it.name, it.isJournal) }.right() } } @@ -98,9 +104,10 @@ class DatalogPageRepository : PageRepository { } } - override fun getUnloadedPages(): Flow>> { + override fun getUnloadedPages(limit: Int, offset: Int): Flow>> { return pages.map { map -> - map.values.filter { !it.isContentLoaded }.right() + map.values.filter { !it.isContentLoaded } + .sortedBy { it.uuid.value }.drop(offset).take(limit).right() } } diff --git a/kmp/src/commonMain/kotlin/dev/stapler/stelekit/repository/InMemoryRepositories.kt b/kmp/src/commonMain/kotlin/dev/stapler/stelekit/repository/InMemoryRepositories.kt index c0b741af..2b0c1709 100644 --- a/kmp/src/commonMain/kotlin/dev/stapler/stelekit/repository/InMemoryRepositories.kt +++ b/kmp/src/commonMain/kotlin/dev/stapler/stelekit/repository/InMemoryRepositories.kt @@ -8,6 +8,7 @@ import dev.stapler.stelekit.model.PageUuid import dev.stapler.stelekit.model.Property import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.flow.flow import kotlinx.coroutines.flow.map import kotlinx.coroutines.flow.flowOf import arrow.core.Either @@ -544,12 +545,6 @@ class InMemoryBlockRepository : BlockRepository { class InMemoryPageRepository : PageRepository { private val pages = MutableStateFlow>(emptyMap()) - override fun getAllPages(): Flow>> { - return pages.map { map -> - map.values.toList().right() - } - } - override fun getPages(limit: Int, offset: Int): Flow>> { return pages.map { map -> val result = map.values.sortedBy { it.name }.drop(offset).take(limit) @@ -648,12 +643,6 @@ class InMemoryPageRepository : PageRepository { } } - override fun getUnloadedPages(): Flow>> { - return pages.map { map -> - map.values.filter { !it.isContentLoaded }.right() - } - } - override suspend fun savePage(page: Page): Either { val current = pages.value.toMutableMap() current[page.uuid.value] = page @@ -732,11 +721,17 @@ class InMemorySearchRepository( override fun searchPagesByTitle(query: String, limit: Int): Flow>> { if (pageRepository == null || query.isEmpty()) return flowOf(emptyList().right()) - return pageRepository.getAllPages().map { res -> - res.map { pages -> - pages.filter { it.name.contains(query, ignoreCase = true) || it.properties["alias"]?.contains(query, ignoreCase = true) == true } - .take(limit) - } + // Test backend: one-shot bounded-batch snapshot (the alias-property filter has no + // SQL equivalent here). Production search uses FTS-backed repositories. + return flow { + emit( + pageRepository.getAllPagesSnapshot().map { pages -> + pages.filter { + it.name.contains(query, ignoreCase = true) || + it.properties["alias"]?.contains(query, ignoreCase = true) == true + }.take(limit) + } + ) } } diff --git a/kmp/src/commonMain/kotlin/dev/stapler/stelekit/repository/PageRepository.kt b/kmp/src/commonMain/kotlin/dev/stapler/stelekit/repository/PageRepository.kt index 571709b7..7756be07 100644 --- a/kmp/src/commonMain/kotlin/dev/stapler/stelekit/repository/PageRepository.kt +++ b/kmp/src/commonMain/kotlin/dev/stapler/stelekit/repository/PageRepository.kt @@ -1,12 +1,13 @@ package dev.stapler.stelekit.repository import arrow.core.Either +import arrow.core.right import dev.stapler.stelekit.error.DomainError import dev.stapler.stelekit.model.Page import dev.stapler.stelekit.model.PageUuid import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.first -import kotlinx.coroutines.flow.map +import kotlinx.coroutines.flow.flow /** * Lightweight (name, isJournal) projection of a page row. Used by consumers that need @@ -15,13 +16,23 @@ import kotlinx.coroutines.flow.map */ data class PageNameEntry(val name: String, val isJournal: Boolean) +/** + * There is deliberately NO `getAllPages()` on this interface. A reactive full-table flow + * is re-materialized in its entirety on every write that invalidates it; standing + * collectors of such a flow caused GC thrash and OutOfMemoryError on Android during + * 8 000+ page graph imports. All reads are paginated, projected, or chunked: + * - whole-graph one-shots (export, migration, tests): [getAllPagesSnapshot] — bounded batches + * - whole-graph standing observers: [getPageNameEntries] — names-only projection + * - UI lists: [getPages] / [getJournalPages] / [getFavoritePages] / point lookups + * - reconcile existence checks: [getPagesByNames] / [getJournalPagesByDates] — chunked IN + * - background indexing: [getUnloadedPages] (limit, offset) drain + [countUnloadedPages] + */ interface PageRepository { fun getPageByUuid(uuid: PageUuid): Flow> fun getPageByName(name: String): Flow> fun getPagesInNamespace(namespace: String): Flow>> fun getPages(limit: Int, offset: Int): Flow>> fun searchPages(query: String, limit: Int, offset: Int): Flow>> - fun getAllPages(): Flow>> fun getJournalPages(limit: Int, offset: Int): Flow>> /** @@ -35,61 +46,59 @@ interface PageRepository { /** * Favorite pages only — the bounded query standing UI observers (sidebar) must use. - * - * The default implementation derives from [getAllPages] for in-memory/test backends. - * SQL-backed repositories MUST override it with a dedicated `WHERE is_favorite = 1` - * query: a standing collector on [getAllPages] re-materializes the entire pages table - * on every write, which on 8 000+ page graphs causes GC thrash and OutOfMemoryError - * on Android during graph import/reconcile. + * SQL-backed repositories implement this with a dedicated `WHERE is_favorite = 1` query. */ - fun getFavoritePages(): Flow>> = - getAllPages().map { either -> either.map { pages -> pages.filter { it.isFavorite } } } + fun getFavoritePages(): Flow>> - fun getUnloadedPages(): Flow>> + /** + * Bounded batch of not-yet-content-loaded pages in stable (uuid) order — used by the + * background-indexing drain loop. Callers re-fetch at a fixed limit and advance + * [offset] only past rows that permanently fail, so peak memory is O(limit) rather + * than O(graph). + */ + fun getUnloadedPages(limit: Int, offset: Int): Flow>> /** - * Bounded batch of not-yet-content-loaded pages in stable (uuid) order — the - * backpressure-friendly variant of [getUnloadedPages] for background indexing. - * Callers drain by re-fetching at a fixed limit and advancing [offset] only past - * rows that permanently fail, so peak memory is O(limit) rather than O(graph). - * - * Default derives from [getUnloadedPages] for in-memory/test backends; SQL-backed - * repositories must override with a dedicated LIMIT/OFFSET query - * (`selectUnloadedPagesPaginated`) — the in-memory fallback here exists only so fakes - * keep compiling. + * Count of not-yet-content-loaded pages — O(1) progress denominator for indexing. + * Default drains [getUnloadedPages] in bounded batches; SQL-backed repositories + * override with `SELECT COUNT(*)`. */ - @Suppress("InMemoryPagination") - fun getUnloadedPages(limit: Int, offset: Int): Flow>> = - getUnloadedPages().map { either -> - either.map { pages -> pages.sortedBy { it.uuid.value }.drop(offset).take(limit) } + fun countUnloadedPages(): Flow> = flow { + var count = 0L + var offset = 0 + while (true) { + when (val batch = getUnloadedPages(SNAPSHOT_BATCH_SIZE, offset).first()) { + is Either.Left -> { + emit(batch) + return@flow + } + is Either.Right -> { + count += batch.value.size + if (batch.value.size < SNAPSHOT_BATCH_SIZE) break + offset += batch.value.size + } + } } - - /** Count of not-yet-content-loaded pages — O(1) progress denominator for indexing. */ - fun countUnloadedPages(): Flow> = - getUnloadedPages().map { either -> either.map { it.size.toLong() } } + emit(count.right()) + } /** - * Names-only projection of all pages for the suggestion index. Unlike [getAllPages], - * a standing observer of this flow materializes only (name, isJournal) pairs — + * Names-only projection of all pages for the suggestion index. Unlike a full-table + * read, a standing observer of this flow materializes only (name, isJournal) pairs — * a few hundred KB on an 8 000-page graph instead of tens of MB of full Page objects. - * - * SQL-backed repositories must override with a dedicated two-column query. */ - fun getPageNameEntries(): Flow>> = - getAllPages().map { either -> - either.map { pages -> pages.map { PageNameEntry(it.name, it.isJournal) } } - } + fun getPageNameEntries(): Flow>> /** * Pages whose names match [names] (case-insensitive). Bounded existence lookup used * by graph reconcile: one query per file chunk instead of preloading the whole table. - * Implementations must chunk the IN list to ≤500 to respect Android's - * SQLITE_MAX_VARIABLE_NUMBER=999 on API < 30. + * SQL implementations chunk the IN list to ≤500 to respect Android's + * SQLITE_MAX_VARIABLE_NUMBER=999 on API < 30. Default scans in bounded batches. */ suspend fun getPagesByNames(names: Collection): Either> { if (names.isEmpty()) return Either.Right(emptyList()) val lower = names.mapTo(HashSet()) { it.lowercase() } - return getAllPages().first().map { pages -> pages.filter { it.name.lowercase() in lower } } + return getAllPagesSnapshot().map { pages -> pages.filter { it.name.lowercase() in lower } } } /** Journal pages whose dates match [dates]. Bounded chunk lookup for reconcile. */ @@ -98,7 +107,29 @@ interface PageRepository { ): Either> { if (dates.isEmpty()) return Either.Right(emptyList()) val dateSet = dates.toHashSet() - return getAllPages().first().map { pages -> pages.filter { it.journalDate in dateSet } } + return getAllPagesSnapshot().map { pages -> pages.filter { it.journalDate in dateSet } } + } + + /** + * One-shot whole-graph snapshot for export, migration tooling, benchmarks, and tests. + * Reads in bounded batches of [batchSize] via [getPages] — never a single unbounded + * query, and never a reactive flow that re-materializes per write. Callers that hold + * the returned list accept O(graph) memory knowingly (whole-graph export/migration); + * UI code must use the paginated/projected reads instead. + */ + suspend fun getAllPagesSnapshot(batchSize: Int = SNAPSHOT_BATCH_SIZE): Either> { + val all = mutableListOf() + var offset = 0 + while (true) { + when (val batch = getPages(batchSize, offset).first()) { + is Either.Left -> return batch + is Either.Right -> { + all += batch.value + if (batch.value.size < batchSize) return Either.Right(all) + offset += batch.value.size + } + } + } } @DirectRepositoryWrite @@ -126,4 +157,9 @@ interface PageRepository { suspend fun clear() suspend fun cacheEvictAll() {} + + companion object { + /** Batch size for snapshot/drain defaults — bounds per-fetch memory. */ + const val SNAPSHOT_BATCH_SIZE = 500 + } } diff --git a/kmp/src/commonMain/kotlin/dev/stapler/stelekit/repository/SqlDelightPageRepository.kt b/kmp/src/commonMain/kotlin/dev/stapler/stelekit/repository/SqlDelightPageRepository.kt index 26c619ee..55797f7f 100644 --- a/kmp/src/commonMain/kotlin/dev/stapler/stelekit/repository/SqlDelightPageRepository.kt +++ b/kmp/src/commonMain/kotlin/dev/stapler/stelekit/repository/SqlDelightPageRepository.kt @@ -100,14 +100,6 @@ class SqlDelightPageRepository( queries.selectPagesByNameLikePaginated("%$query%", limit.toLong(), offset.toLong()) .asDbFlowList(PlatformDispatcher.DB) { it.toModel() } - override fun getAllPages(): Flow>> = - queries.selectAllPages() - .asFlow() - .conflate() // drop intermediate invalidations during bulk import to avoid O(N²) full-table scans - .mapToList(PlatformDispatcher.DB) - .map { list -> list.map { it.toModel() }.right() } - .catchDbError() - override fun getFavoritePages(): Flow>> = queries.selectFavoritePages() .asFlow() @@ -128,10 +120,6 @@ class SqlDelightPageRepository( queries.selectRecentlyUpdatedPages(limit.toLong()) .asDbFlowList(PlatformDispatcher.DB) { it.toModel() } - override fun getUnloadedPages(): Flow>> = - queries.selectUnloadedPages() - .asDbFlowList(PlatformDispatcher.DB) { it.toModel() } - override fun getUnloadedPages(limit: Int, offset: Int): Flow>> = queries.selectUnloadedPagesPaginated(limit.toLong(), offset.toLong()) .asDbFlowList(PlatformDispatcher.DB) { it.toModel() } diff --git a/kmp/src/commonMain/kotlin/dev/stapler/stelekit/ui/screens/AllPagesViewModel.kt b/kmp/src/commonMain/kotlin/dev/stapler/stelekit/ui/screens/AllPagesViewModel.kt index 21efa2da..d68e9479 100644 --- a/kmp/src/commonMain/kotlin/dev/stapler/stelekit/ui/screens/AllPagesViewModel.kt +++ b/kmp/src/commonMain/kotlin/dev/stapler/stelekit/ui/screens/AllPagesViewModel.kt @@ -17,7 +17,6 @@ import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.flow.asStateFlow import kotlinx.coroutines.flow.combine import kotlinx.coroutines.flow.debounce -import kotlinx.coroutines.flow.distinctUntilChanged import kotlinx.coroutines.flow.first import kotlinx.coroutines.flow.map import kotlinx.coroutines.flow.stateIn @@ -121,42 +120,39 @@ class AllPagesViewModel( private suspend fun loadAllPages() { _isLoading.value = true try { - pageRepository.getAllPages() - .distinctUntilChanged { old, new -> - // Only re-trigger backlink loading when the set of page UUIDs changes, - // not on every metadata update (e.g. updatedAt tick during indexing). - old.getOrNull()?.map { it.uuid } == new.getOrNull()?.map { it.uuid } - } - .collect { result -> - val pageList = result.getOrNull() - if (pageList != null) { - // Preserve existing backlink counts for pages already loaded. - val existingCounts = _allRows.value.associate { it.page.uuid to it.backlinkCount } - _allRows.value = pageList.map { PageRow(it, existingCounts[it.uuid] ?: 0) } - _isLoading.value = false - - // Only fetch backlink counts for pages we haven't counted yet. - val uncounted = pageList.filter { existingCounts[it.uuid] == null } - if (uncounted.isNotEmpty()) { - val semaphore = Semaphore(8) - uncounted.forEach { page -> - scope.launch { - semaphore.withPermit { - val count = blockRepository.countLinkedReferences(page.name) - .first().getOrNull() ?: 0L - _allRows.update { rows -> - rows.map { row -> - if (row.page.uuid == page.uuid) row.copy(backlinkCount = count.toInt()) else row - } - } + // One-shot bounded-batch snapshot, NOT a standing getAllPages() collector: every + // write invalidates a full-table flow, so a standing observer re-materialized all + // pages per write burst while indexing ran — GC thrash and OOM on 8 000+ page + // graphs on Android. This screen holds all rows by design (whole-graph table with + // in-memory sort/filter); refresh() reloads on demand. + val pageList = pageRepository.getAllPagesSnapshot().getOrNull() + if (pageList != null) { + // Preserve existing backlink counts for pages already loaded. + val existingCounts = _allRows.value.associate { it.page.uuid to it.backlinkCount } + _allRows.value = pageList.map { PageRow(it, existingCounts[it.uuid] ?: 0) } + _isLoading.value = false + + // Only fetch backlink counts for pages we haven't counted yet. + val uncounted = pageList.filter { existingCounts[it.uuid] == null } + if (uncounted.isNotEmpty()) { + val semaphore = Semaphore(8) + uncounted.forEach { page -> + scope.launch { + semaphore.withPermit { + val count = blockRepository.countLinkedReferences(page.name) + .first().getOrNull() ?: 0L + _allRows.update { rows -> + rows.map { row -> + if (row.page.uuid == page.uuid) row.copy(backlinkCount = count.toInt()) else row } } } } - } else { - _isLoading.value = false } } + } else { + _isLoading.value = false + } } catch (e: CancellationException) { throw e } catch (e: Exception) { diff --git a/kmp/src/commonMain/kotlin/dev/stapler/stelekit/ui/screens/GlobalUnlinkedReferencesViewModel.kt b/kmp/src/commonMain/kotlin/dev/stapler/stelekit/ui/screens/GlobalUnlinkedReferencesViewModel.kt index ffb3eccb..51ea87ca 100644 --- a/kmp/src/commonMain/kotlin/dev/stapler/stelekit/ui/screens/GlobalUnlinkedReferencesViewModel.kt +++ b/kmp/src/commonMain/kotlin/dev/stapler/stelekit/ui/screens/GlobalUnlinkedReferencesViewModel.kt @@ -103,7 +103,8 @@ class GlobalUnlinkedReferencesViewModel( scope.launch { _state.update { it.copy(isLoading = true, results = emptyList(), errorMessage = null) } try { - val pages = pageRepository.getAllPages().first().getOrNull() ?: emptyList() + // Names-only projection — this scan only needs page names, never full Pages. + val pages = pageRepository.getPageNameEntries().first().getOrNull() ?: emptyList() allPageNames = pages.map { it.name } pageCursorIndex = 0 pendingEntries = mutableListOf() diff --git a/kmp/src/commonMain/kotlin/dev/stapler/stelekit/ui/screens/ImportViewModel.kt b/kmp/src/commonMain/kotlin/dev/stapler/stelekit/ui/screens/ImportViewModel.kt index c28279d7..afcdf493 100644 --- a/kmp/src/commonMain/kotlin/dev/stapler/stelekit/ui/screens/ImportViewModel.kt +++ b/kmp/src/commonMain/kotlin/dev/stapler/stelekit/ui/screens/ImportViewModel.kt @@ -193,8 +193,9 @@ class ImportViewModel( } private suspend fun runScan(text: String, matcher: AhoCorasickMatcher) { - // Get existing page names so suggestions don't duplicate known pages - val existingNames = pageRepository.getAllPages() + // Get existing page names so suggestions don't duplicate known pages. + // Names-only projection — never materialize full Page objects for the whole graph. + val existingNames = pageRepository.getPageNameEntries() .first() .getOrNull() ?.map { it.name } @@ -376,9 +377,10 @@ class ImportViewModel( return } - // URL deduplication: reject if another page was already imported from this URL + // URL deduplication: reject if another page was already imported from this URL. + // One-shot bounded-batch snapshot (the source-URL property has no SQL index). if (currentState.activeTab == ImportTab.URL && currentState.urlInput.isNotBlank()) { - val allPages = pageRepository.getAllPages().first().getOrNull() + val allPages = pageRepository.getAllPagesSnapshot().getOrNull() val duplicatePage = allPages?.firstOrNull { it.properties["source"] == currentState.urlInput } if (duplicatePage != null) { _state.update { it.copy(pageNameError = "A page from this URL already exists: '${duplicatePage.name}'") } diff --git a/kmp/src/commonMain/sqldelight/dev/stapler/stelekit/db/SteleDatabase.sq b/kmp/src/commonMain/sqldelight/dev/stapler/stelekit/db/SteleDatabase.sq index 73e16036..948c816c 100644 --- a/kmp/src/commonMain/sqldelight/dev/stapler/stelekit/db/SteleDatabase.sq +++ b/kmp/src/commonMain/sqldelight/dev/stapler/stelekit/db/SteleDatabase.sq @@ -313,15 +313,14 @@ SELECT COUNT(*) FROM pages WHERE uuid = ?; existsPageByName: SELECT COUNT(*) FROM pages WHERE name = ?; -selectAllPages: -SELECT * FROM pages ORDER BY name; - +-- NOTE: there is intentionally no unpaginated `SELECT * FROM pages` (or unpaginated +-- unloaded-pages) query. Standing collectors of unbounded queries re-materialize the +-- entire table per write burst and OOM Android on 8 000+ page graphs. Whole-graph +-- consumers use PageRepository.getAllPagesSnapshot() (bounded batches over +-- selectAllPagesPaginated) or the selectPageNameEntries projection. selectAllPagesPaginated: SELECT * FROM pages ORDER BY name LIMIT ? OFFSET ?; -selectUnloadedPages: -SELECT * FROM pages WHERE is_content_loaded = 0; - -- Bounded batch for background indexing. ORDER BY uuid gives a stable drain order so a -- caller can advance OFFSET past permanently-failing rows without re-fetching them forever. selectUnloadedPagesPaginated: diff --git a/kmp/src/commonTest/kotlin/dev/stapler/stelekit/repository/JournalServiceTest.kt b/kmp/src/commonTest/kotlin/dev/stapler/stelekit/repository/JournalServiceTest.kt index bb2584af..b9e84c2e 100644 --- a/kmp/src/commonTest/kotlin/dev/stapler/stelekit/repository/JournalServiceTest.kt +++ b/kmp/src/commonTest/kotlin/dev/stapler/stelekit/repository/JournalServiceTest.kt @@ -80,7 +80,7 @@ class JournalServiceTest { assertEquals(PageUuid("existing-uuid"), result.uuid) // Should NOT create additional pages - val allPages = pageRepo.getAllPages().first().getOrNull() ?: emptyList() + val allPages = pageRepo.getAllPagesSnapshot().getOrNull() ?: emptyList() assertEquals(1, allPages.size) } @@ -158,7 +158,7 @@ class JournalServiceTest { assertEquals(null, pageAStill) // Only one page should remain - val allPages = pageRepo.getAllPages().first().getOrNull() ?: emptyList() + val allPages = pageRepo.getAllPagesSnapshot().getOrNull() ?: emptyList() val todayPages = allPages.filter { it.journalDate == today() } assertEquals(1, todayPages.size) } diff --git a/kmp/src/commonTest/kotlin/dev/stapler/stelekit/ui/screens/JournalsViewModelEditorTest.kt b/kmp/src/commonTest/kotlin/dev/stapler/stelekit/ui/screens/JournalsViewModelEditorTest.kt index 90982dd5..051871e8 100644 --- a/kmp/src/commonTest/kotlin/dev/stapler/stelekit/ui/screens/JournalsViewModelEditorTest.kt +++ b/kmp/src/commonTest/kotlin/dev/stapler/stelekit/ui/screens/JournalsViewModelEditorTest.kt @@ -275,7 +275,11 @@ class JournalsViewModelEditorTest { fun addPage(page: Page) { pages.add(page) } - override fun getAllPages(): Flow>> = flowOf(pages.toList().right()) + override fun getFavoritePages(): Flow>> = + flowOf(pages.filter { it.isFavorite }.right()) + + override fun getPageNameEntries(): Flow>> = + flowOf(pages.map { dev.stapler.stelekit.repository.PageNameEntry(it.name, it.isJournal) }.right()) override fun getJournalPages(limit: Int, offset: Int): Flow>> { val journals = pages.filter { it.isJournal } @@ -314,8 +318,8 @@ class JournalsViewModelEditorTest { override fun getJournalPageByDate(date: LocalDate): Flow> = flowOf(pages.find { it.journalDate == date }.right()) - override fun getUnloadedPages(): Flow>> = - flowOf(pages.filter { !it.isContentLoaded }.right()) + override fun getUnloadedPages(limit: Int, offset: Int): Flow>> = + flowOf(pages.filter { !it.isContentLoaded }.sortedBy { it.uuid.value }.drop(offset).take(limit).right()) override suspend fun savePage(page: Page): Either { pages.removeAll { it.uuid == page.uuid } diff --git a/kmp/src/commonTest/kotlin/dev/stapler/stelekit/ui/screens/JournalsViewModelTest.kt b/kmp/src/commonTest/kotlin/dev/stapler/stelekit/ui/screens/JournalsViewModelTest.kt index 3803735f..99fe4943 100644 --- a/kmp/src/commonTest/kotlin/dev/stapler/stelekit/ui/screens/JournalsViewModelTest.kt +++ b/kmp/src/commonTest/kotlin/dev/stapler/stelekit/ui/screens/JournalsViewModelTest.kt @@ -90,7 +90,11 @@ class JournalsViewModelTest { class FakePageRepository : PageRepository { val pages = mutableListOf() - override fun getAllPages(): Flow>> = flowOf(pages.right()) + override fun getFavoritePages(): Flow>> = + flowOf(pages.filter { it.isFavorite }.right()) + + override fun getPageNameEntries(): Flow>> = + flowOf(pages.map { dev.stapler.stelekit.repository.PageNameEntry(it.name, it.isJournal) }.right()) override fun getJournalPages(limit: Int, offset: Int): Flow>> { val journals = pages @@ -121,7 +125,8 @@ class JournalsViewModelTest { override fun getPageByName(name: String): Flow> = flowOf(pages.find { it.name == name }.right()) override fun getRecentPages(limit: Int): Flow>> = flowOf(pages.sortedByDescending { it.updatedAt }.take(limit).right()) override fun getJournalPageByDate(date: LocalDate): Flow> = flowOf(pages.find { it.journalDate == date }.right()) - override fun getUnloadedPages(): Flow>> = flowOf(pages.filter { !it.isContentLoaded }.right()) + override fun getUnloadedPages(limit: Int, offset: Int): Flow>> = + flowOf(pages.filter { !it.isContentLoaded }.sortedBy { it.uuid.value }.drop(offset).take(limit).right()) override suspend fun savePage(page: Page): Either { val existingIdx = pages.indexOfFirst { it.uuid == page.uuid } if (existingIdx >= 0) pages[existingIdx] = page else pages.add(page) diff --git a/kmp/src/jvmCommonMain/kotlin/dev/stapler/stelekit/performance/InstrumentedPageRepository.kt b/kmp/src/jvmCommonMain/kotlin/dev/stapler/stelekit/performance/InstrumentedPageRepository.kt index 6fc5452a..3af53c78 100644 --- a/kmp/src/jvmCommonMain/kotlin/dev/stapler/stelekit/performance/InstrumentedPageRepository.kt +++ b/kmp/src/jvmCommonMain/kotlin/dev/stapler/stelekit/performance/InstrumentedPageRepository.kt @@ -8,6 +8,7 @@ import dev.stapler.stelekit.error.DomainError import dev.stapler.stelekit.model.Page import dev.stapler.stelekit.model.PageUuid import dev.stapler.stelekit.repository.DirectRepositoryWrite +import dev.stapler.stelekit.repository.PageNameEntry import dev.stapler.stelekit.repository.PageRepository import io.opentelemetry.api.trace.StatusCode import io.opentelemetry.api.trace.Tracer @@ -33,8 +34,6 @@ class InstrumentedPageRepository( override fun searchPages(query: String, limit: Int, offset: Int): Flow>> = delegate.searchPages(query, limit, offset) - override fun getAllPages(): Flow>> = delegate.getAllPages() - override fun getJournalPages(limit: Int, offset: Int): Flow>> = delegate.getJournalPages(limit, offset) @@ -44,7 +43,26 @@ class InstrumentedPageRepository( override fun getRecentPages(limit: Int): Flow>> = delegate.getRecentPages(limit) - override fun getUnloadedPages(): Flow>> = delegate.getUnloadedPages() + override fun getFavoritePages(): Flow>> = delegate.getFavoritePages() + + override fun getPageNameEntries(): Flow>> = + delegate.getPageNameEntries() + + override fun getUnloadedPages(limit: Int, offset: Int): Flow>> = + delegate.getUnloadedPages(limit, offset) + + override fun countUnloadedPages(): Flow> = delegate.countUnloadedPages() + + // Explicit delegation (not the interface defaults) so the SQL-optimized chunked IN + // queries and bounded-batch snapshot of the wrapped repository are preserved. + override suspend fun getPagesByNames(names: Collection): Either> = + delegate.getPagesByNames(names) + + override suspend fun getJournalPagesByDates(dates: Collection): Either> = + delegate.getJournalPagesByDates(dates) + + override suspend fun getAllPagesSnapshot(batchSize: Int): Either> = + delegate.getAllPagesSnapshot(batchSize) override fun countPages(): Flow> = delegate.countPages() diff --git a/kmp/src/jvmTest/kotlin/dev/stapler/stelekit/benchmark/GraphLoadTimingTest.kt b/kmp/src/jvmTest/kotlin/dev/stapler/stelekit/benchmark/GraphLoadTimingTest.kt index 970e19e3..f4d6835b 100644 --- a/kmp/src/jvmTest/kotlin/dev/stapler/stelekit/benchmark/GraphLoadTimingTest.kt +++ b/kmp/src/jvmTest/kotlin/dev/stapler/stelekit/benchmark/GraphLoadTimingTest.kt @@ -348,7 +348,7 @@ class GraphLoadTimingTest { val pageRepo = InMemoryPageRepository() val blockRepo = InMemoryBlockRepository() loadAndTime(dir.absolutePath, GraphLoader(fileSystem, pageRepo, blockRepo), "synthetic / in-memory") { - pageRepo.getAllPages().first().getOrNull()?.size ?: 0 + pageRepo.getAllPagesSnapshot().getOrNull()?.size ?: 0 } Unit } finally { @@ -369,7 +369,7 @@ class GraphLoadTimingTest { val loader = GraphLoader(fileSystem, repoSet.pageRepository, repoSet.blockRepository, externalWriteActor = repoSet.writeActor, histogramWriter = repoSet.histogramWriter) val result = loadAndTime(dir.absolutePath, loader, "synthetic / SQLite") { - repoSet.pageRepository.getAllPages().first().getOrNull()?.size ?: 0 + repoSet.pageRepository.getAllPagesSnapshot().getOrNull()?.size ?: 0 } assertTrue(result.totalMs < 60_000L, "SQLite synthetic load took ${result.totalMs}ms — catastrophic regression detected (> 60s)") @@ -431,7 +431,7 @@ class GraphLoadTimingTest { val loader = GraphLoader(fileSystem, repoSet.pageRepository, repoSet.blockRepository, externalWriteActor = repoSet.writeActor, histogramWriter = repoSet.histogramWriter) loadAndTime(tempDir.absolutePath, loader, "real graph / SQLite") { - repoSet.pageRepository.getAllPages().first().getOrNull()?.size ?: 0 + repoSet.pageRepository.getAllPagesSnapshot().getOrNull()?.size ?: 0 }.also { } factory.close() } finally { diff --git a/kmp/src/jvmTest/kotlin/dev/stapler/stelekit/benchmark/UserSessionBenchmarkTest.kt b/kmp/src/jvmTest/kotlin/dev/stapler/stelekit/benchmark/UserSessionBenchmarkTest.kt index 37d3033a..92a1e94d 100644 --- a/kmp/src/jvmTest/kotlin/dev/stapler/stelekit/benchmark/UserSessionBenchmarkTest.kt +++ b/kmp/src/jvmTest/kotlin/dev/stapler/stelekit/benchmark/UserSessionBenchmarkTest.kt @@ -180,7 +180,7 @@ class UserSessionBenchmarkTest { val repoSet = factory.createRepositorySet(GraphBackend.SQLDELIGHT, scope) // Warm up: ensure schema creation completes on the calling thread before // the ViewModel's Dispatchers.Default workers first access the database. - repoSet.pageRepository.getAllPages().first() + repoSet.pageRepository.getAllPagesSnapshot() val ringBuffer = repoSet.ringBuffer?.also { it.enabled = true } @@ -228,7 +228,7 @@ class UserSessionBenchmarkTest { while (repoSet.writeActor?.hasPendingWrites == true) delay(200) } ?: println("[user-session] WARNING — write actor did not drain after 120s") - val allPages = repoSet.pageRepository.getAllPages().first().getOrNull() ?: emptyList() + val allPages = repoSet.pageRepository.getAllPagesSnapshot().getOrNull() ?: emptyList() println("[user-session] Pages in DB after actor drain: ${allPages.size}") if (allPages.isEmpty()) { println("[user-session] SKIPPED — no pages loaded from $graphPath after 60s") diff --git a/kmp/src/jvmTest/kotlin/dev/stapler/stelekit/db/AppLoadJournalIntegrationTest.kt b/kmp/src/jvmTest/kotlin/dev/stapler/stelekit/db/AppLoadJournalIntegrationTest.kt index 38d282e7..b96ed224 100644 --- a/kmp/src/jvmTest/kotlin/dev/stapler/stelekit/db/AppLoadJournalIntegrationTest.kt +++ b/kmp/src/jvmTest/kotlin/dev/stapler/stelekit/db/AppLoadJournalIntegrationTest.kt @@ -100,7 +100,7 @@ class AppLoadJournalIntegrationTest { h.appLoad() - val allPages = h.pageRepo.getAllPages().first().getOrNull() ?: emptyList() + val allPages = h.pageRepo.getAllPagesSnapshot().getOrNull() ?: emptyList() val todayPages = allPages.filter { it.journalDate == today() } assertEquals(1, todayPages.size, "Exactly one today journal, got: ${todayPages.map { it.name }}") } @@ -128,7 +128,7 @@ class AppLoadJournalIntegrationTest { h.appLoad() - val allPages = h.pageRepo.getAllPages().first().getOrNull() ?: emptyList() + val allPages = h.pageRepo.getAllPagesSnapshot().getOrNull() ?: emptyList() val todayPages = allPages.filter { it.journalDate == today() || it.name == todayHyphen } assertEquals(1, todayPages.size, "No duplicate, got: ${todayPages.map { it.name }}") } @@ -140,7 +140,7 @@ class AppLoadJournalIntegrationTest { h.appLoad() - val allPages = h.pageRepo.getAllPages().first().getOrNull() ?: emptyList() + val allPages = h.pageRepo.getAllPagesSnapshot().getOrNull() ?: emptyList() val journal = allPages.first { it.journalDate == today() || it.name == todayHyphen } val blocks = h.blockRepo.getBlocksForPage(journal.uuid).first().getOrNull() ?: emptyList() assertTrue(blocks.isNotEmpty(), "Disk content should not be wiped — blocks expected") @@ -157,7 +157,7 @@ class AppLoadJournalIntegrationTest { h.appLoad() - val allPages = h.pageRepo.getAllPages().first().getOrNull() ?: emptyList() + val allPages = h.pageRepo.getAllPagesSnapshot().getOrNull() ?: emptyList() val todayPages = allPages.filter { it.journalDate == today() || it.name == todayUnderscore } assertEquals(1, todayPages.size, "No duplicate, got: ${todayPages.map { it.name }}") } @@ -169,7 +169,7 @@ class AppLoadJournalIntegrationTest { h.appLoad() - val allPages = h.pageRepo.getAllPages().first().getOrNull() ?: emptyList() + val allPages = h.pageRepo.getAllPagesSnapshot().getOrNull() ?: emptyList() val journal = allPages.first { it.journalDate == today() || it.name == todayUnderscore } val blocks = h.blockRepo.getBlocksForPage(journal.uuid).first().getOrNull() ?: emptyList() assertTrue(blocks.isNotEmpty(), "Disk content should not be wiped — blocks expected") @@ -247,7 +247,7 @@ class AppLoadJournalIntegrationTest { h.appLoad() - val allPages = h.pageRepo.getAllPages().first().getOrNull() ?: emptyList() + val allPages = h.pageRepo.getAllPagesSnapshot().getOrNull() ?: emptyList() // Today must be created val todayJournal = allPages.firstOrNull { it.journalDate == today() } @@ -270,7 +270,7 @@ class AppLoadJournalIntegrationTest { h.appLoad() - val allPages = h.pageRepo.getAllPages().first().getOrNull() ?: emptyList() + val allPages = h.pageRepo.getAllPagesSnapshot().getOrNull() ?: emptyList() // 2 past + 1 today = 3 total assertEquals(3, allPages.size, "Should have 2 past journals + 1 today, got: ${allPages.map { it.name }}") } @@ -290,7 +290,7 @@ class AppLoadJournalIntegrationTest { h.appLoad() - val allPages = h.pageRepo.getAllPages().first().getOrNull() ?: emptyList() + val allPages = h.pageRepo.getAllPagesSnapshot().getOrNull() ?: emptyList() val todayPages = allPages.filter { it.journalDate == today() } assertEquals(1, todayPages.size, "Duplicates should be merged, got: ${todayPages.map { it.name }}") } @@ -306,7 +306,7 @@ class AppLoadJournalIntegrationTest { h.appLoad() - val allPages = h.pageRepo.getAllPages().first().getOrNull() ?: emptyList() + val allPages = h.pageRepo.getAllPagesSnapshot().getOrNull() ?: emptyList() val todayPage = allPages.first { it.journalDate == today() } val blocks = h.blockRepo.getBlocksForPage(todayPage.uuid).first().getOrNull() ?: emptyList() assertTrue(blocks.isNotEmpty(), "Merged page should retain blocks from surviving format") @@ -400,7 +400,7 @@ class AppLoadJournalIntegrationTest { assertEquals(PageUuid("stub-uuid"), result.uuid, "Should return the existing stub, not create a second page") - val allPages = h.pageRepo.getAllPages().first().getOrNull() ?: emptyList() + val allPages = h.pageRepo.getAllPagesSnapshot().getOrNull() ?: emptyList() assertEquals(1, allPages.filter { it.journalDate == today() }.size, "No duplicate for stub page") } @@ -418,7 +418,7 @@ class AppLoadJournalIntegrationTest { val distinctUuids = results.map { it.uuid }.toSet() assertEquals(1, distinctUuids.size, "All concurrent calls must return the same page UUID") - val allPages = h.pageRepo.getAllPages().first().getOrNull() ?: emptyList() + val allPages = h.pageRepo.getAllPagesSnapshot().getOrNull() ?: emptyList() assertEquals(1, allPages.filter { it.journalDate == today() }.size, "Exactly one page after concurrent calls") } @@ -435,7 +435,7 @@ class AppLoadJournalIntegrationTest { h.appLoad() h.appLoad() - val allPages = h.pageRepo.getAllPages().first().getOrNull() ?: emptyList() + val allPages = h.pageRepo.getAllPagesSnapshot().getOrNull() ?: emptyList() val todayPages = allPages.filter { it.journalDate == today() } assertEquals(1, todayPages.size, "Repeated loads should not duplicate today's journal") } @@ -515,7 +515,7 @@ class AppLoadJournalIntegrationTest { ensureDeferred.await() loadDeferred.await() - val allPages = h.pageRepo.getAllPages().first().getOrNull() ?: emptyList() + val allPages = h.pageRepo.getAllPagesSnapshot().getOrNull() ?: emptyList() val todayPages = allPages.filter { it.journalDate == today() } assertEquals(1, todayPages.size, "Concurrent warm-cache load must not duplicate today's journal") } @@ -533,7 +533,7 @@ class AppLoadJournalIntegrationTest { val lookedUp = h.journalService.getPageByJournalDate(today()) assertEquals(existingPage.uuid, lookedUp?.uuid, "Lookup should return the same page, not create a new one") - val allPages = h.pageRepo.getAllPages().first().getOrNull() ?: emptyList() + val allPages = h.pageRepo.getAllPagesSnapshot().getOrNull() ?: emptyList() assertEquals(1, allPages.filter { it.journalDate == today() }.size, "No duplicate created by lookup") } } diff --git a/kmp/src/jvmTest/kotlin/dev/stapler/stelekit/db/DemoGraphIntegrationTest.kt b/kmp/src/jvmTest/kotlin/dev/stapler/stelekit/db/DemoGraphIntegrationTest.kt index 12b0b5e9..b73c4213 100644 --- a/kmp/src/jvmTest/kotlin/dev/stapler/stelekit/db/DemoGraphIntegrationTest.kt +++ b/kmp/src/jvmTest/kotlin/dev/stapler/stelekit/db/DemoGraphIntegrationTest.kt @@ -33,7 +33,7 @@ class DemoGraphIntegrationTest { graphLoader.loadGraph(graphDir.absolutePath) {} - val pages = pageRepository.getAllPages().first().getOrNull() ?: emptyList() + val pages = pageRepository.getAllPagesSnapshot().getOrNull() ?: emptyList() Triple(pages, pageRepository, blockRepository) } diff --git a/kmp/src/jvmTest/kotlin/dev/stapler/stelekit/db/ExternalChangeConflictTest.kt b/kmp/src/jvmTest/kotlin/dev/stapler/stelekit/db/ExternalChangeConflictTest.kt index 3b0eddde..0752ad3f 100644 --- a/kmp/src/jvmTest/kotlin/dev/stapler/stelekit/db/ExternalChangeConflictTest.kt +++ b/kmp/src/jvmTest/kotlin/dev/stapler/stelekit/db/ExternalChangeConflictTest.kt @@ -88,7 +88,7 @@ class ExternalChangeConflictTest { fs.writeFile(filePath, "- User typed content") loader.markFileWrittenByUs(filePath) - val pagesBefore = pageRepo.getAllPages().first().getOrNull()?.size ?: 0 + val pagesBefore = pageRepo.getAllPagesSnapshot().getOrNull()?.size ?: 0 // Manually invoke checkDirectoryForChanges via detectChanges val changeSet = loader.fileRegistry.detectChanges("/graph/journals") @@ -179,7 +179,7 @@ class ExternalChangeConflictTest { loader.loadGraph("/graph") {} // Record how many pages exist before the change - val pagesBefore = pageRepo.getAllPages().first().getOrNull() ?: emptyList() + val pagesBefore = pageRepo.getAllPagesSnapshot().getOrNull() ?: emptyList() // Subscribe to externalFileChanges and always suppress val suppressJob = launch { @@ -228,7 +228,7 @@ class ExternalChangeConflictTest { loader2.loadGraph("/graph") {} - val pagesAfterLoad = pageRepo2.getAllPages().first().getOrNull() ?: emptyList() + val pagesAfterLoad = pageRepo2.getAllPagesSnapshot().getOrNull() ?: emptyList() val journalPage = pagesAfterLoad.firstOrNull { it.name.contains("2026") } if (journalPage != null) { diff --git a/kmp/src/jvmTest/kotlin/dev/stapler/stelekit/db/GraphLoaderCacheTest.kt b/kmp/src/jvmTest/kotlin/dev/stapler/stelekit/db/GraphLoaderCacheTest.kt index a5747d91..c65bdc73 100644 --- a/kmp/src/jvmTest/kotlin/dev/stapler/stelekit/db/GraphLoaderCacheTest.kt +++ b/kmp/src/jvmTest/kotlin/dev/stapler/stelekit/db/GraphLoaderCacheTest.kt @@ -131,7 +131,7 @@ class GraphLoaderCacheTest { h.loader.parseAndSavePage(FilePath(filePath), "- Block V1", ParseMode.FULL) // Verify V1 in DB - val pages = h.pageRepo.getAllPages().first().getOrNull() ?: emptyList() + val pages = h.pageRepo.getAllPagesSnapshot().getOrNull() ?: emptyList() assertFalse(pages.isEmpty(), "Page should be loaded after parseAndSavePage") val page = pages.first() @@ -173,7 +173,7 @@ class GraphLoaderCacheTest { h.loader.setGraphPath("/graph") h.loader.parseAndSavePage(FilePath(filePath), "- Block V1", ParseMode.FULL) - val page = h.pageRepo.getAllPages().first().getOrNull()?.firstOrNull() + val page = h.pageRepo.getAllPagesSnapshot().getOrNull()?.firstOrNull() assertNotNull(page, "Page should be in DB after loading V1") // External write with same mtime (FAT granularity: both writes in same 2s window) @@ -209,7 +209,7 @@ class GraphLoaderCacheTest { h.loader.setGraphPath("/graph") h.loader.parseAndSavePage(FilePath(filePath), "- Block V1", ParseMode.FULL) - val page = h.pageRepo.getAllPages().first().getOrNull()?.firstOrNull() + val page = h.pageRepo.getAllPagesSnapshot().getOrNull()?.firstOrNull() assertNotNull(page, "Page should be in DB") // Simulate: watcher detected external edit and marked dirty (skipped onReloadFile @@ -260,7 +260,7 @@ class GraphLoaderCacheTest { // Force initial load to populate DB and store content hash h.loader.loadFullPage_forceLoad(filePath, contentV1) - val page = h.pageRepo.getAllPages().first().getOrNull()?.firstOrNull() + val page = h.pageRepo.getAllPagesSnapshot().getOrNull()?.firstOrNull() assertNotNull(page, "Page should be in DB") // Verify content hash was stored @@ -302,7 +302,7 @@ class GraphLoaderCacheTest { // Force initial load h.loader.loadFullPage_forceLoad(filePath, contentV1) - val page = h.pageRepo.getAllPages().first().getOrNull()?.firstOrNull() + val page = h.pageRepo.getAllPagesSnapshot().getOrNull()?.firstOrNull() assertNotNull(page, "Page should be in DB") // Reset the read counter @@ -354,7 +354,7 @@ class GraphLoaderCacheTest { // Load V1 so page.updatedAt is set h.loader.parseAndSavePage(FilePath(filePath), contentV1, ParseMode.FULL) - val page = h.pageRepo.getAllPages().first().getOrNull()?.firstOrNull() + val page = h.pageRepo.getAllPagesSnapshot().getOrNull()?.firstOrNull() assertNotNull(page, "Page should be in DB") // Verify V1 is loaded @@ -397,7 +397,7 @@ class GraphLoaderCacheTest { h.loader.startWatching("/graph") h.loader.fileRegistry.scanDirectory("/graph/pages") - val page = h.pageRepo.getAllPages().first().getOrNull()?.firstOrNull() + val page = h.pageRepo.getAllPagesSnapshot().getOrNull()?.firstOrNull() assertNotNull(page, "Page should be in DB") // External edit: bump mtime and content diff --git a/kmp/src/jvmTest/kotlin/dev/stapler/stelekit/db/GraphLoaderIndexBatchingTest.kt b/kmp/src/jvmTest/kotlin/dev/stapler/stelekit/db/GraphLoaderIndexBatchingTest.kt index 0ad0b30b..88369acc 100644 --- a/kmp/src/jvmTest/kotlin/dev/stapler/stelekit/db/GraphLoaderIndexBatchingTest.kt +++ b/kmp/src/jvmTest/kotlin/dev/stapler/stelekit/db/GraphLoaderIndexBatchingTest.kt @@ -36,12 +36,6 @@ class GraphLoaderIndexBatchingTest { private class RecordingRepository(initial: List) : FakePageRepository(initial) { val maxBatchLimit = AtomicInteger(0) val boundedFetches = AtomicInteger(0) - val unboundedSubscriptions = AtomicInteger(0) - - override fun getUnloadedPages(): Flow>> { - unboundedSubscriptions.incrementAndGet() - return super.getUnloadedPages() - } override fun getUnloadedPages(limit: Int, offset: Int): Flow>> { boundedFetches.incrementAndGet() @@ -90,10 +84,6 @@ class GraphLoaderIndexBatchingTest { pageRepo.boundedFetches.get() >= 3, "250 pages at ≤100 per batch needs ≥3 fetches; got ${pageRepo.boundedFetches.get()}" ) - assertEquals( - 0, pageRepo.unboundedSubscriptions.get(), - "the unbounded getUnloadedPages() snapshot must not be used" - ) } @Test diff --git a/kmp/src/jvmTest/kotlin/dev/stapler/stelekit/db/GraphLoaderIntegrationTest.kt b/kmp/src/jvmTest/kotlin/dev/stapler/stelekit/db/GraphLoaderIntegrationTest.kt index 3d5b489e..9fa2bfec 100644 --- a/kmp/src/jvmTest/kotlin/dev/stapler/stelekit/db/GraphLoaderIntegrationTest.kt +++ b/kmp/src/jvmTest/kotlin/dev/stapler/stelekit/db/GraphLoaderIntegrationTest.kt @@ -54,7 +54,7 @@ class GraphLoaderIntegrationTest { graphLoader.loadGraph("/graph") { _ -> } // Verify Page - val pages = pageRepository.getAllPages().first().getOrNull() ?: emptyList() + val pages = pageRepository.getAllPagesSnapshot().getOrNull() ?: emptyList() assertEquals(1, pages.size) val page = pages[0] assertEquals("testpage", page.name) @@ -103,7 +103,7 @@ class GraphLoaderIntegrationTest { fileSystem.files[path] = initialContent graphLoader.parseAndSavePage(FilePath(path), initialContent) - val pages = pageRepository.getAllPages().first().getOrNull() ?: emptyList() + val pages = pageRepository.getAllPagesSnapshot().getOrNull() ?: emptyList() val page = pages.find { it.name == "warmreload" } assertTrue(page != null, "Page must be saved on cold load") @@ -145,7 +145,7 @@ class GraphLoaderIntegrationTest { loader2.parseAndSavePage(FilePath(path), content, mode = ParseMode.METADATA_ONLY) - val pages = pageRepo2.getAllPages().first().getOrNull() ?: emptyList() + val pages = pageRepo2.getAllPagesSnapshot().getOrNull() ?: emptyList() val page = pages.find { it.name == "metadataonly" } assertTrue(page != null, "Page must be saved in METADATA_ONLY mode") diff --git a/kmp/src/jvmTest/kotlin/dev/stapler/stelekit/db/GraphLoaderTest.kt b/kmp/src/jvmTest/kotlin/dev/stapler/stelekit/db/GraphLoaderTest.kt index a4b73bf8..d3b20485 100644 --- a/kmp/src/jvmTest/kotlin/dev/stapler/stelekit/db/GraphLoaderTest.kt +++ b/kmp/src/jvmTest/kotlin/dev/stapler/stelekit/db/GraphLoaderTest.kt @@ -69,7 +69,7 @@ class GraphLoaderTest { graphLoader.loadGraph(graphDir.absolutePath) {} - val pages = pageRepository.getAllPages().first().getOrNull() ?: emptyList() + val pages = pageRepository.getAllPagesSnapshot().getOrNull() ?: emptyList() assertTrue(pages.isNotEmpty(), "No pages loaded") // Verify the fixture pages were loaded (name matching is case-insensitive in loader) @@ -108,7 +108,7 @@ class GraphLoaderTest { assertTrue(phase1Complete, "Phase 1 should be complete") assertTrue(fullyLoaded, "Graph should be fully loaded") - val pages = pageRepository.getAllPages().first().getOrNull() ?: emptyList() + val pages = pageRepository.getAllPagesSnapshot().getOrNull() ?: emptyList() assertTrue(pages.isNotEmpty(), "Pages should be loaded") val contentsPage = pages.firstOrNull { it.name.lowercase().contains("contents") } diff --git a/kmp/src/jvmTest/kotlin/dev/stapler/stelekit/db/GraphLoaderWatcherTest.kt b/kmp/src/jvmTest/kotlin/dev/stapler/stelekit/db/GraphLoaderWatcherTest.kt index e5863a43..d412cf01 100644 --- a/kmp/src/jvmTest/kotlin/dev/stapler/stelekit/db/GraphLoaderWatcherTest.kt +++ b/kmp/src/jvmTest/kotlin/dev/stapler/stelekit/db/GraphLoaderWatcherTest.kt @@ -64,7 +64,7 @@ class GraphLoaderWatcherTest { assertTrue(blocksBeforeTick.isEmpty(), "No blocks should exist before the watcher runs") // Capture baseline: page count right after loadGraph (empty graph = 0 pages) - val pageCountBaseline = pageRepo.getAllPages().first().getOrNull()?.size ?: 0 + val pageCountBaseline = pageRepo.getAllPagesSnapshot().getOrNull()?.size ?: 0 // Touch file (change mtime without changing content) delay(50) @@ -76,7 +76,7 @@ class GraphLoaderWatcherTest { // If markFileWrittenByUs is working, the watcher must not re-parse the file // on an mtime-only change, so the page count must equal the baseline. - val pageCountAfter = pageRepo.getAllPages().first().getOrNull()?.size ?: 0 + val pageCountAfter = pageRepo.getAllPagesSnapshot().getOrNull()?.size ?: 0 assertEquals(pageCountBaseline, pageCountAfter, "markFileWrittenByUs should suppress re-parse on mtime-only change; " + "page count must not grow from $pageCountBaseline to $pageCountAfter") diff --git a/kmp/src/jvmTest/kotlin/dev/stapler/stelekit/db/GraphManagerDatabaseLifecycleTest.kt b/kmp/src/jvmTest/kotlin/dev/stapler/stelekit/db/GraphManagerDatabaseLifecycleTest.kt index 771d3ca5..492384b4 100644 --- a/kmp/src/jvmTest/kotlin/dev/stapler/stelekit/db/GraphManagerDatabaseLifecycleTest.kt +++ b/kmp/src/jvmTest/kotlin/dev/stapler/stelekit/db/GraphManagerDatabaseLifecycleTest.kt @@ -76,7 +76,7 @@ class GraphManagerDatabaseLifecycleTest { // Simulate a Compose LaunchedEffect: collect from a repository Flow val collectJob = launch(Dispatchers.Default) { try { - repoSet.pageRepository.getAllPages().collect { result -> + repoSet.pageRepository.getPages(50, 0).collect { result -> collectedValues.add(result) } } catch (e: Throwable) { @@ -128,7 +128,7 @@ class GraphManagerDatabaseLifecycleTest { var collectionCrashed = false val collectJob = launch(Dispatchers.Default) { try { - repoSet.pageRepository.getAllPages().collect { } + repoSet.pageRepository.getPages(50, 0).collect { } } catch (e: Throwable) { if (e !is kotlinx.coroutines.CancellationException) collectionCrashed = true } diff --git a/kmp/src/jvmTest/kotlin/dev/stapler/stelekit/db/LargeGraphWarmStartCrashTest.kt b/kmp/src/jvmTest/kotlin/dev/stapler/stelekit/db/LargeGraphWarmStartCrashTest.kt index c43545a2..d4f1b58c 100644 --- a/kmp/src/jvmTest/kotlin/dev/stapler/stelekit/db/LargeGraphWarmStartCrashTest.kt +++ b/kmp/src/jvmTest/kotlin/dev/stapler/stelekit/db/LargeGraphWarmStartCrashTest.kt @@ -66,35 +66,17 @@ class LargeGraphWarmStartCrashTest { } /** - * Records query-boundedness during the run: the whole pipeline must never subscribe to - * the unbounded getAllPages()/getUnloadedPages() reads, and every batch lookup must be - * chunk-sized — that is what keeps peak memory O(chunk) instead of O(graph). + * Records query-boundedness during the run: every batch lookup must be chunk-sized — + * that is what keeps peak memory O(chunk) instead of O(graph). (Unbounded full-table + * reads no longer exist on PageRepository, so their absence is compile-time enforced.) */ private class BoundedQueryRecordingRepository( initial: List, ) : FakePageRepository(initial) { - val getAllPagesSubscriptions = AtomicInteger(0) - val unboundedUnloadedSubscriptions = AtomicInteger(0) val maxNamesPerLookup = AtomicInteger(0) val maxUnloadedBatchLimit = AtomicInteger(0) val boundedUnloadedFetches = AtomicInteger(0) - override fun getAllPages(): Flow>> { - val base = super.getAllPages() - return flow { - getAllPagesSubscriptions.incrementAndGet() - emitAll(base) - } - } - - override fun getUnloadedPages(): Flow>> { - val base = super.getUnloadedPages() - return flow { - unboundedUnloadedSubscriptions.incrementAndGet() - emitAll(base) - } - } - override fun getUnloadedPages(limit: Int, offset: Int): Flow>> { boundedUnloadedFetches.incrementAndGet() maxUnloadedBatchLimit.updateAndGet { max(it, limit) } @@ -223,16 +205,6 @@ class LargeGraphWarmStartCrashTest { ) // ── Query boundedness: peak memory must stay O(chunk), not O(graph) ───── - assertTrue( - pageRepo.getAllPagesSubscriptions.get() == 0, - "getAllPages() must never be subscribed during load — found " + - "${pageRepo.getAllPagesSubscriptions.get()} subscription(s); full-table " + - "reads re-materialize all $PAGE_COUNT pages and OOM Android" - ) - assertTrue( - pageRepo.unboundedUnloadedSubscriptions.get() == 0, - "Unbounded getUnloadedPages() must not be used — Phase 3 must drain in batches" - ) assertTrue( pageRepo.maxNamesPerLookup.get() in 1..100, "Reconcile name lookups must be chunk-sized (≤100); max was " + diff --git a/kmp/src/jvmTest/kotlin/dev/stapler/stelekit/db/QueryPlanAuditTest.kt b/kmp/src/jvmTest/kotlin/dev/stapler/stelekit/db/QueryPlanAuditTest.kt index 105df99e..c90b1b41 100644 --- a/kmp/src/jvmTest/kotlin/dev/stapler/stelekit/db/QueryPlanAuditTest.kt +++ b/kmp/src/jvmTest/kotlin/dev/stapler/stelekit/db/QueryPlanAuditTest.kt @@ -27,7 +27,7 @@ class QueryPlanAuditTest { private val ALLOWLIST = setOf( // No WHERE clause — full traversal by design "selectAllBlocks", "selectAllBlocksPaginated", "countBlocks", - "selectAllPages", "selectAllPagesPaginated", "countPages", + "selectAllPagesPaginated", "countPages", "selectPageNameEntries", "selectAllMetadata", "selectAllDebugFlags", // content LIKE — no index on content; FTS handles production full-text search "selectBlocksWithContentLike", "selectBlocksWithContentLikePaginated", @@ -37,7 +37,10 @@ class QueryPlanAuditTest { // Aggregate / analytics scans — intentionally full-table "selectDuplicateBlockHashes", "selectMostConnectedBlocks", "selectOrphanedBlocks", // pages columns without an index - "selectUnloadedPages", // is_content_loaded has no index + "selectUnloadedPagesPaginated", // is_content_loaded has no index; LIMIT bounds rows returned + "countUnloadedPages", // is_content_loaded has no index + "selectFavoritePages", // is_favorite has no index; favorites are few by nature + "selectJournalPagesByDates", // journal_date IN — no index on journal_date "selectRecentlyUpdatedPages", // updated_at has no index on pages "selectRecentlyCreatedPages", // created_at has no index on pages "selectJournalPages", // is_journal has no index @@ -113,12 +116,20 @@ class QueryPlanAuditTest { "SELECT COUNT(*) FROM pages WHERE uuid = 'x'"), AuditQuery("existsPageByName", "SELECT COUNT(*) FROM pages WHERE name = 'x'"), - AuditQuery("selectAllPages", - "SELECT * FROM pages ORDER BY name"), AuditQuery("selectAllPagesPaginated", "SELECT * FROM pages ORDER BY name LIMIT 10 OFFSET 0"), - AuditQuery("selectUnloadedPages", - "SELECT * FROM pages WHERE is_content_loaded = 0"), + AuditQuery("selectUnloadedPagesPaginated", + "SELECT * FROM pages WHERE is_content_loaded = 0 ORDER BY uuid LIMIT 10 OFFSET 0"), + AuditQuery("countUnloadedPages", + "SELECT COUNT(*) FROM pages WHERE is_content_loaded = 0"), + AuditQuery("selectPageNameEntries", + "SELECT name, is_journal FROM pages"), + AuditQuery("selectFavoritePages", + "SELECT * FROM pages WHERE is_favorite = 1 ORDER BY name"), + AuditQuery("selectPagesByNames", + "SELECT * FROM pages WHERE name IN ('x')"), + AuditQuery("selectJournalPagesByDates", + "SELECT * FROM pages WHERE is_journal = 1 AND journal_date IN ('2024-01-01')"), AuditQuery("selectPagesByNamespace", "SELECT * FROM pages WHERE namespace = 'x' ORDER BY name LIMIT 10 OFFSET 0"), AuditQuery("selectPagesByNamespaceUnpaginated", diff --git a/kmp/src/jvmTest/kotlin/dev/stapler/stelekit/integration/PipelineReproductionTest.kt b/kmp/src/jvmTest/kotlin/dev/stapler/stelekit/integration/PipelineReproductionTest.kt index a2e8cd65..0d25f029 100644 --- a/kmp/src/jvmTest/kotlin/dev/stapler/stelekit/integration/PipelineReproductionTest.kt +++ b/kmp/src/jvmTest/kotlin/dev/stapler/stelekit/integration/PipelineReproductionTest.kt @@ -60,7 +60,7 @@ class PipelineReproductionTest { graphLoader.loadGraph("/graph") { } - val pagesResult = pageRepository.getAllPages().first() + val pagesResult = pageRepository.getAllPagesSnapshot() val pages = pagesResult.getOrNull()!! val page = pages[0] val blocksResult = blockRepository.getBlocksForPage(page.uuid).first() diff --git a/kmp/src/jvmTest/kotlin/dev/stapler/stelekit/migration/DryRunTest.kt b/kmp/src/jvmTest/kotlin/dev/stapler/stelekit/migration/DryRunTest.kt index 90b16197..80456a3c 100644 --- a/kmp/src/jvmTest/kotlin/dev/stapler/stelekit/migration/DryRunTest.kt +++ b/kmp/src/jvmTest/kotlin/dev/stapler/stelekit/migration/DryRunTest.kt @@ -151,13 +151,13 @@ class DryRunTest { val runner = buildRunner(db, repoSet, actor) // Snapshot repo state before - val pagesBefore = repoSet.pageRepository.getAllPages().first().getOrNull() ?: emptyList() + val pagesBefore = repoSet.pageRepository.getAllPagesSnapshot().getOrNull() ?: emptyList() val changelogBefore = ChangelogRepository(db).appliedIds("graph-dry-no-mod") runner.dryRun("graph-dry-no-mod", repoSet) // Snapshot repo state after - val pagesAfter = repoSet.pageRepository.getAllPages().first().getOrNull() ?: emptyList() + val pagesAfter = repoSet.pageRepository.getAllPagesSnapshot().getOrNull() ?: emptyList() val changelogAfter = ChangelogRepository(db).appliedIds("graph-dry-no-mod") assertEquals(pagesBefore.size, pagesAfter.size, "Page count should be unchanged after dry run") diff --git a/kmp/src/jvmTest/kotlin/dev/stapler/stelekit/repository/RepositoryFlowResilienceTest.kt b/kmp/src/jvmTest/kotlin/dev/stapler/stelekit/repository/RepositoryFlowResilienceTest.kt index 215f0544..1e0a07d9 100644 --- a/kmp/src/jvmTest/kotlin/dev/stapler/stelekit/repository/RepositoryFlowResilienceTest.kt +++ b/kmp/src/jvmTest/kotlin/dev/stapler/stelekit/repository/RepositoryFlowResilienceTest.kt @@ -38,7 +38,7 @@ class RepositoryFlowResilienceTest { // Compose LaunchedEffect would be collecting the flow. factory.close() - val result = repoSet.pageRepository.getAllPages().first() + val result = repoSet.pageRepository.getAllPagesSnapshot() assertTrue( result.isLeft(), diff --git a/kmp/src/jvmTest/kotlin/dev/stapler/stelekit/repository/UpgradeResilienceTest.kt b/kmp/src/jvmTest/kotlin/dev/stapler/stelekit/repository/UpgradeResilienceTest.kt index a65afb06..f5e2dfe0 100644 --- a/kmp/src/jvmTest/kotlin/dev/stapler/stelekit/repository/UpgradeResilienceTest.kt +++ b/kmp/src/jvmTest/kotlin/dev/stapler/stelekit/repository/UpgradeResilienceTest.kt @@ -52,8 +52,14 @@ class UpgradeResilienceTest { factory.close() // ── PageRepository ──────────────────────────────────────────────── - assertFlowEmitsLeft("PageRepository.getAllPages") { - repoSet.pageRepository.getAllPages().first() + assertFlowEmitsLeft("PageRepository.getAllPagesSnapshot") { + repoSet.pageRepository.getAllPagesSnapshot() + } + assertFlowEmitsLeft("PageRepository.getFavoritePages") { + repoSet.pageRepository.getFavoritePages().first() + } + assertFlowEmitsLeft("PageRepository.getPageNameEntries") { + repoSet.pageRepository.getPageNameEntries().first() } assertFlowEmitsLeft("PageRepository.getPages") { repoSet.pageRepository.getPages(10, 0).first() @@ -71,7 +77,10 @@ class UpgradeResilienceTest { repoSet.pageRepository.searchPages("test", 10, 0).first() } assertFlowEmitsLeft("PageRepository.getUnloadedPages") { - repoSet.pageRepository.getUnloadedPages().first() + repoSet.pageRepository.getUnloadedPages(10, 0).first() + } + assertFlowEmitsLeft("PageRepository.countUnloadedPages") { + repoSet.pageRepository.countUnloadedPages().first() } // ── BlockRepository ─────────────────────────────────────────────── @@ -152,7 +161,7 @@ class UpgradeResilienceTest { // Re-open the same in-memory database with a fresh factory instance. // On a file-backed DB this would be the upgrade scenario; on an in-memory // DB it confirms the schema is stable across re-attach. - val pages = repoSet.pageRepository.getAllPages().first().getOrNull() + val pages = repoSet.pageRepository.getAllPagesSnapshot().getOrNull() assertTrue(pages != null && pages.any { it.uuid.value == FIXTURE_PAGE_UUID }, "v0.36.0 page must survive upgrade") @@ -179,7 +188,7 @@ class UpgradeResilienceTest { factory.close() assertFlowEmitsLeft("upgraded PageRepository.getAllPages") { - repoSet.pageRepository.getAllPages().first() + repoSet.pageRepository.getAllPagesSnapshot() } val imageRepo2 = repoSet.imageAnnotationRepository as? SqlDelightImageAnnotationRepository if (imageRepo2 != null) { diff --git a/kmp/src/jvmTest/kotlin/dev/stapler/stelekit/ui/StelekitViewModelCrashReproductionTest.kt b/kmp/src/jvmTest/kotlin/dev/stapler/stelekit/ui/StelekitViewModelCrashReproductionTest.kt index acafcc46..29afb2e7 100644 --- a/kmp/src/jvmTest/kotlin/dev/stapler/stelekit/ui/StelekitViewModelCrashReproductionTest.kt +++ b/kmp/src/jvmTest/kotlin/dev/stapler/stelekit/ui/StelekitViewModelCrashReproductionTest.kt @@ -18,14 +18,12 @@ import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.SupervisorJob import kotlinx.coroutines.delay import kotlinx.coroutines.flow.Flow -import kotlinx.coroutines.flow.emitAll import kotlinx.coroutines.flow.first import kotlinx.coroutines.flow.flow import kotlinx.coroutines.runBlocking import kotlinx.coroutines.withTimeout import kotlinx.datetime.LocalDate import java.util.concurrent.CopyOnWriteArrayList -import java.util.concurrent.atomic.AtomicInteger import kotlin.test.Test import kotlin.test.assertNotNull import kotlin.test.assertTrue @@ -161,41 +159,8 @@ class StelekitViewModelCrashReproductionTest { } } - // ─── TC-SCAN-001: no standing full-table observers in the ViewModel ───────── - - private class CountingPageRepository : FakePageRepository() { - val getAllPagesSubscriptions = AtomicInteger(0) - override fun getAllPages(): Flow>> { - val base = super.getAllPages() - return flow { - getAllPagesSubscriptions.incrementAndGet() - emitAll(base) - } - } - } - - /** - * The hang half of "hangs and crashes": every DB write invalidates getAllPages(), so - * each standing collector re-materializes the entire table per write burst during - * import/reconcile — O(N) full Page allocations × thousands of writes = GC thrash and - * OOM. No production observer may subscribe to getAllPages() at all: the sidebar uses - * bounded queries (favorites, point lookups) and PageNameIndex uses the names-only - * getPageNameEntries() projection. - */ - @Test - fun viewmodel_holds_no_getAllPages_subscription() = runBlocking { - val repo = CountingPageRepository() - makeViewModel(repo) - - // Let init-time observers (observeSpecialPages, PageNameIndex) subscribe. - delay(1_500) - - val subscriptions = repo.getAllPagesSubscriptions.get() - assertTrue( - subscriptions == 0, - "Expected no getAllPages() subscription from the ViewModel, found $subscriptions — " + - "full-table observers re-materialize all pages on every write and OOM Android " + - "on 8 000+ page graphs" - ) - } + // Note: this file previously asserted at runtime that the ViewModel holds no standing + // getAllPages() subscription. That guarantee is now structural — PageRepository has no + // unbounded full-table read at all; whole-graph consumers go through the bounded + // getAllPagesSnapshot() / getPageNameEntries() reads. } diff --git a/kmp/src/jvmTest/kotlin/dev/stapler/stelekit/ui/fixtures/FakeRepositories.kt b/kmp/src/jvmTest/kotlin/dev/stapler/stelekit/ui/fixtures/FakeRepositories.kt index e16172cc..c70d7dd0 100644 --- a/kmp/src/jvmTest/kotlin/dev/stapler/stelekit/ui/fixtures/FakeRepositories.kt +++ b/kmp/src/jvmTest/kotlin/dev/stapler/stelekit/ui/fixtures/FakeRepositories.kt @@ -67,9 +67,6 @@ open class FakePageRepository(initialPages: List = emptyList()) : PageRepo override fun getPageByName(name: String): Flow> = _pages.map { pages -> pages.values.find { it.name == name }.right() } - override fun getAllPages(): Flow>> = - _pages.map { it.values.toList().right() } - override fun getJournalPages(limit: Int, offset: Int): Flow>> = _pages.map { pages -> pages.values.filter { it.isJournal }.sortedByDescending { it.journalDate }.drop(offset).take(limit).right() @@ -100,9 +97,6 @@ open class FakePageRepository(initialPages: List = emptyList()) : PageRepo override fun getFavoritePages(): Flow>> = _pages.map { pages -> pages.values.filter { it.isFavorite }.sortedBy { it.name }.right() } - override fun getUnloadedPages(): Flow>> = - _pages.map { pages -> pages.values.filter { !it.isContentLoaded }.right() } - override fun getUnloadedPages(limit: Int, offset: Int): Flow>> = _pages.map { pages -> pages.values.filter { !it.isContentLoaded } From 75df586faa86e13b8c955207067af609fa41a5e3 Mon Sep 17 00:00:00 2001 From: Tyler Stapler Date: Wed, 10 Jun 2026 23:00:11 -0700 Subject: [PATCH 4/5] fix(review): address Gate 2 code review findings - BLOCKER: remove runCatching from withTimeout in GraphLoaderIndexBatchingTest to prevent CancellationException from breaking structured concurrency - CRITICAL: snapshot recentPageUuids under recentMutex then do DB work outside lock in refreshRecentPages to eliminate starvation of addToRecent - fix sanitizeErrorMessage: apply in loadGraph catch blocks (bypassed before); add Windows path regex C:\... pattern - fix QueryPlanAuditTest selectJournalPages: audit SQL was COALESCE phantom query, not the actual ORDER BY journal_date DESC used in SteleDatabase.sq - change countUnloadedPages() from Flow to suspend fun to fix reactive/one-shot contract mismatch between SQL and InMemory implementations - add getPagesByNames/getJournalPagesByDates overrides to DatalogPageRepository to avoid O(graph) getAllPagesSnapshot() fallback - improve test determinism: replace delay(1_000) with delay(200); replace polling while loop with flow.first{} in LargeGraphWarmStartCrashTest - add test for ExportService oldest==null guard branch (uEJR04) Co-Authored-By: Claude Sonnet 4.6 --- .../export/ExportServiceJournalRangeTest.kt | 33 ++++++ .../dev/stapler/stelekit/db/GraphLoader.kt | 2 +- .../stapler/stelekit/db/MigrationRunner.kt | 10 ++ .../stapler/stelekit/export/ExportService.kt | 5 +- .../repository/DatalogPageRepository.kt | 12 +++ .../repository/InMemoryRepositories.kt | 5 +- .../stelekit/repository/PageRepository.kt | 17 +-- .../repository/SqlDelightPageRepository.kt | 32 ++++-- .../stapler/stelekit/ui/StelekitViewModel.kt | 102 ++++++++++-------- .../stelekit/ui/screens/AllPagesViewModel.kt | 13 +++ .../performance/InstrumentedPageRepository.kt | 2 +- .../db/GraphLoaderIndexBatchingTest.kt | 5 +- .../db/LargeGraphWarmStartCrashTest.kt | 7 +- .../stapler/stelekit/db/QueryPlanAuditTest.kt | 14 ++- .../repository/UpgradeResilienceTest.kt | 2 +- .../StelekitViewModelCrashReproductionTest.kt | 8 +- .../stelekit/ui/fixtures/FakeRepositories.kt | 4 +- 17 files changed, 189 insertions(+), 84 deletions(-) diff --git a/kmp/src/businessTest/kotlin/dev/stapler/stelekit/export/ExportServiceJournalRangeTest.kt b/kmp/src/businessTest/kotlin/dev/stapler/stelekit/export/ExportServiceJournalRangeTest.kt index ab11adf3..982b1a91 100644 --- a/kmp/src/businessTest/kotlin/dev/stapler/stelekit/export/ExportServiceJournalRangeTest.kt +++ b/kmp/src/businessTest/kotlin/dev/stapler/stelekit/export/ExportServiceJournalRangeTest.kt @@ -110,6 +110,39 @@ class ExportServiceJournalRangeTest { assertTrue(result.isLeft(), "Expected Left for empty range but got Right") } + // ── U-EJR-04: batch with null journal date terminates without hanging ──── + + @Test + fun uEJR04_batchWithNullJournalDate_terminatesWithoutHanging() = runTest { + // A journal page with is_journal=true but no journal_date (corrupted/migrated data). + // Before the fix, the drain loop would never break on oldest==null — infinite scan. + val pageRepo = InMemoryPageRepository() + val blockRepo = InMemoryBlockRepository() + + val badPage = Page( + uuid = PageUuid("p-null-date"), + name = "corrupted-journal", + isJournal = true, + journalDate = null, // triggers oldest==null early-exit guard + createdAt = now, + updatedAt = now, + ) + pageRepo.savePage(badPage) + blockRepo.saveBlock(block("b-null", "some content", "p-null-date")) + + val service = makeService() + // The call must terminate (not hang) even when the page has no journal_date. + // The result is Left because no pages fall within a valid date range. + val result = service.exportJournalRange( + from = LocalDate(2026, 1, 1), + to = LocalDate(2026, 1, 7), + formatId = "markdown", + pageRepo = pageRepo, + blockRepo = blockRepo, + ) + assertTrue(result.isLeft(), "Expected Left when only null-date journal pages exist, got Right") + } + // ── U-EJR-03: pages in range with no blocks → second Left path ─────────── @Test diff --git a/kmp/src/commonMain/kotlin/dev/stapler/stelekit/db/GraphLoader.kt b/kmp/src/commonMain/kotlin/dev/stapler/stelekit/db/GraphLoader.kt index e3d17005..294f7788 100644 --- a/kmp/src/commonMain/kotlin/dev/stapler/stelekit/db/GraphLoader.kt +++ b/kmp/src/commonMain/kotlin/dev/stapler/stelekit/db/GraphLoader.kt @@ -604,7 +604,7 @@ class GraphLoader( backgroundIndexJob = currentCoroutineContext()[Job] PerformanceMonitor.startTrace("indexRemainingPages") try { - val total = pageRepository.countUnloadedPages().first().getOrNull() ?: 0L + val total = pageRepository.countUnloadedPages().getOrNull() ?: 0L if (total == 0L) return logger.info("Background indexing $total pages... (${heapSummary()})") diff --git a/kmp/src/commonMain/kotlin/dev/stapler/stelekit/db/MigrationRunner.kt b/kmp/src/commonMain/kotlin/dev/stapler/stelekit/db/MigrationRunner.kt index a67ff4b1..f406426e 100644 --- a/kmp/src/commonMain/kotlin/dev/stapler/stelekit/db/MigrationRunner.kt +++ b/kmp/src/commonMain/kotlin/dev/stapler/stelekit/db/MigrationRunner.kt @@ -376,6 +376,16 @@ object MigrationRunner { "CREATE INDEX IF NOT EXISTS idx_measurement_annotations_image_uuid ON measurement_annotations(image_uuid)" ) ), + Migration( + name = "pages_unloaded_partial_index", + statements = listOf( + // Partial index covering only unloaded pages (is_content_loaded = 0). + // Makes selectUnloadedPagesPaginated and countUnloadedPages O(unloaded) instead + // of O(total) — on a large graph where most pages are loaded the index is small + // and both the drain-loop OFFSET scan and the COUNT(*) become index-only ops. + "CREATE INDEX IF NOT EXISTS idx_pages_unloaded ON pages(uuid) WHERE is_content_loaded = 0" + ) + ), ) /** diff --git a/kmp/src/commonMain/kotlin/dev/stapler/stelekit/export/ExportService.kt b/kmp/src/commonMain/kotlin/dev/stapler/stelekit/export/ExportService.kt index f680c09e..ce70582a 100644 --- a/kmp/src/commonMain/kotlin/dev/stapler/stelekit/export/ExportService.kt +++ b/kmp/src/commonMain/kotlin/dev/stapler/stelekit/export/ExportService.kt @@ -248,7 +248,7 @@ class ExportService( // stop below the range — never a full-table read. val inRange = mutableListOf() var offset = 0 - val batchSize = 200 + val batchSize = PageRepository.SNAPSHOT_BATCH_SIZE while (true) { val batch = pageRepo.getJournalPages(batchSize, offset).first().getOrNull() ?: emptyList() if (batch.isEmpty()) break @@ -258,8 +258,9 @@ class ExportService( } // DESC ordering: once the oldest date in the batch is before the range start, // no later batch can contain in-range pages. + // oldest == null means no journal_date in this batch — stop to avoid an infinite scan. val oldest = batch.lastOrNull()?.journalDate - if (oldest != null && oldest < from) break + if (oldest == null || oldest < from) break if (batch.size < batchSize) break offset += batch.size } diff --git a/kmp/src/commonMain/kotlin/dev/stapler/stelekit/repository/DatalogPageRepository.kt b/kmp/src/commonMain/kotlin/dev/stapler/stelekit/repository/DatalogPageRepository.kt index 4ad0e7b3..4ba09540 100644 --- a/kmp/src/commonMain/kotlin/dev/stapler/stelekit/repository/DatalogPageRepository.kt +++ b/kmp/src/commonMain/kotlin/dev/stapler/stelekit/repository/DatalogPageRepository.kt @@ -111,6 +111,18 @@ class DatalogPageRepository : PageRepository { } } + override suspend fun getPagesByNames(names: Collection): Either> { + val lower = names.mapTo(HashSet()) { it.lowercase() } + return pages.value.values.filter { it.name.lowercase() in lower }.right() + } + + override suspend fun getJournalPagesByDates( + dates: Collection, + ): Either> { + val dateSet = dates.toHashSet() + return pages.value.values.filter { it.journalDate != null && it.journalDate in dateSet }.right() + } + override suspend fun savePage(page: Page): Either { return try { val current = pages.value.toMutableMap() diff --git a/kmp/src/commonMain/kotlin/dev/stapler/stelekit/repository/InMemoryRepositories.kt b/kmp/src/commonMain/kotlin/dev/stapler/stelekit/repository/InMemoryRepositories.kt index 2b0c1709..fcc316f9 100644 --- a/kmp/src/commonMain/kotlin/dev/stapler/stelekit/repository/InMemoryRepositories.kt +++ b/kmp/src/commonMain/kotlin/dev/stapler/stelekit/repository/InMemoryRepositories.kt @@ -582,9 +582,8 @@ class InMemoryPageRepository : PageRepository { } } - override fun countUnloadedPages(): Flow> { - return pages.map { map -> map.values.count { !it.isContentLoaded }.toLong().right() } - } + override suspend fun countUnloadedPages(): Either = + pages.value.values.count { !it.isContentLoaded }.toLong().right() override fun getPageNameEntries(): Flow>> { return pages.map { map -> diff --git a/kmp/src/commonMain/kotlin/dev/stapler/stelekit/repository/PageRepository.kt b/kmp/src/commonMain/kotlin/dev/stapler/stelekit/repository/PageRepository.kt index 7756be07..d595e428 100644 --- a/kmp/src/commonMain/kotlin/dev/stapler/stelekit/repository/PageRepository.kt +++ b/kmp/src/commonMain/kotlin/dev/stapler/stelekit/repository/PageRepository.kt @@ -7,7 +7,6 @@ import dev.stapler.stelekit.model.Page import dev.stapler.stelekit.model.PageUuid import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.first -import kotlinx.coroutines.flow.flow /** * Lightweight (name, isJournal) projection of a page row. Used by consumers that need @@ -62,24 +61,22 @@ interface PageRepository { * Count of not-yet-content-loaded pages — O(1) progress denominator for indexing. * Default drains [getUnloadedPages] in bounded batches; SQL-backed repositories * override with `SELECT COUNT(*)`. + * + * Suspend function — returns the current count. Not reactive; call again to re-check. */ - fun countUnloadedPages(): Flow> = flow { + suspend fun countUnloadedPages(): Either { var count = 0L var offset = 0 while (true) { when (val batch = getUnloadedPages(SNAPSHOT_BATCH_SIZE, offset).first()) { - is Either.Left -> { - emit(batch) - return@flow - } + is Either.Left -> return batch is Either.Right -> { count += batch.value.size - if (batch.value.size < SNAPSHOT_BATCH_SIZE) break + if (batch.value.size < SNAPSHOT_BATCH_SIZE) return Either.Right(count) offset += batch.value.size } } } - emit(count.right()) } /** @@ -94,6 +91,10 @@ interface PageRepository { * by graph reconcile: one query per file chunk instead of preloading the whole table. * SQL implementations chunk the IN list to ≤500 to respect Android's * SQLITE_MAX_VARIABLE_NUMBER=999 on API < 30. Default scans in bounded batches. + * + * **Warning**: the default fallback calls [getAllPagesSnapshot], which materializes + * the entire graph in memory. Implementations MUST override this with a bounded IN-clause + * query — never rely on the default in production code. */ suspend fun getPagesByNames(names: Collection): Either> { if (names.isEmpty()) return Either.Right(emptyList()) diff --git a/kmp/src/commonMain/kotlin/dev/stapler/stelekit/repository/SqlDelightPageRepository.kt b/kmp/src/commonMain/kotlin/dev/stapler/stelekit/repository/SqlDelightPageRepository.kt index 55797f7f..4f969cb2 100644 --- a/kmp/src/commonMain/kotlin/dev/stapler/stelekit/repository/SqlDelightPageRepository.kt +++ b/kmp/src/commonMain/kotlin/dev/stapler/stelekit/repository/SqlDelightPageRepository.kt @@ -124,15 +124,15 @@ class SqlDelightPageRepository( queries.selectUnloadedPagesPaginated(limit.toLong(), offset.toLong()) .asDbFlowList(PlatformDispatcher.DB) { it.toModel() } - override fun countUnloadedPages(): Flow> = flow { + override suspend fun countUnloadedPages(): Either = withContext(PlatformDispatcher.DB) { try { - emit(queries.countUnloadedPages().executeAsOne().right()) + queries.countUnloadedPages().executeAsOne().right() } catch (e: CancellationException) { throw e } catch (e: Exception) { - emit(DomainError.DatabaseError.ReadFailed(e.message ?: "unknown").left()) + DomainError.DatabaseError.ReadFailed(e.message ?: "unknown").left() } - }.flowOn(PlatformDispatcher.DB) + } override fun getPageNameEntries(): Flow>> = queries.selectPageNameEntries() @@ -145,10 +145,16 @@ class SqlDelightPageRepository( override suspend fun getPagesByNames(names: Collection): Either> = withContext(PlatformDispatcher.DB) { try { + // Wrap all chunks in a single read transaction for snapshot isolation — + // without it, a write between chunks could make the result set inconsistent. // Chunk the IN list: SQLITE_MAX_VARIABLE_NUMBER is 999 on Android API < 30. - names.chunked(IN_CLAUSE_CHUNK_SIZE).flatMap { chunk -> - queries.selectPagesByNames(chunk).executeAsList().map { it.toModel() } - }.right() + var result: List = emptyList() + queries.transaction { + result = names.chunked(IN_CLAUSE_CHUNK_SIZE).flatMap { chunk -> + queries.selectPagesByNames(chunk).executeAsList().map { it.toModel() } + } + } + result.right() } catch (e: CancellationException) { throw e } catch (e: Exception) { @@ -160,10 +166,14 @@ class SqlDelightPageRepository( dates: Collection, ): Either> = withContext(PlatformDispatcher.DB) { try { - dates.chunked(IN_CLAUSE_CHUNK_SIZE).flatMap { chunk -> - queries.selectJournalPagesByDates(chunk.map { it.toString() }) - .executeAsList().map { it.toModel() } - }.right() + var result: List = emptyList() + queries.transaction { + result = dates.chunked(IN_CLAUSE_CHUNK_SIZE).flatMap { chunk -> + queries.selectJournalPagesByDates(chunk.map { it.toString() }) + .executeAsList().map { it.toModel() } + } + } + result.right() } catch (e: CancellationException) { throw e } catch (e: Exception) { diff --git a/kmp/src/commonMain/kotlin/dev/stapler/stelekit/ui/StelekitViewModel.kt b/kmp/src/commonMain/kotlin/dev/stapler/stelekit/ui/StelekitViewModel.kt index 6252e8a3..bc1338d7 100644 --- a/kmp/src/commonMain/kotlin/dev/stapler/stelekit/ui/StelekitViewModel.kt +++ b/kmp/src/commonMain/kotlin/dev/stapler/stelekit/ui/StelekitViewModel.kt @@ -69,6 +69,8 @@ import kotlinx.coroutines.launch import kotlinx.coroutines.withContext import kotlinx.coroutines.CancellationException import kotlinx.coroutines.isActive +import kotlinx.coroutines.sync.Mutex +import kotlinx.coroutines.sync.withLock import kotlinx.datetime.DateTimeUnit import kotlinx.datetime.LocalDate import kotlinx.datetime.TimeZone @@ -130,14 +132,21 @@ class StelekitViewModel( _uiState.update { it.copy( isLoading = false, - fatalError = "${e::class.simpleName ?: "UnknownError"}: ${e.message ?: "unknown"}" + fatalError = "${e::class.simpleName ?: "UnknownError"}: ${sanitizeErrorMessage(e.message)}" ) } } } ) + private val recentMutex = Mutex() private val logger = Logger("StelekitViewModel") + private fun sanitizeErrorMessage(message: String?): String = + message + ?.replace(Regex("/[^\\s,;:]+"), "") + ?.replace(Regex("[A-Za-z]:\\\\[^\\s,;:]*"), "") + ?.take(200) ?: "unknown" + /** * Platform-provided callback that opens the image picker and attaches the selected image * to the currently editing block. Registered by the host composable (App.kt) once the @@ -308,10 +317,12 @@ class StelekitViewModel( private fun observeSpecialPages() { scope.launch { // Load recents for the current graph before starting collection - recentPageUuids = platformSettings.getString(recentPagesKey, "") - .split(",") - .filter { it.isNotEmpty() } - .toMutableList() + recentMutex.withLock { + recentPageUuids = platformSettings.getString(recentPagesKey, "") + .split(",") + .filter { it.isNotEmpty() } + .toMutableList() + } refreshRecentPages() // Favorites for the sidebar via the dedicated bounded query. Never collect @@ -334,15 +345,23 @@ class StelekitViewModel( * Re-resolves [recentPageUuids] into Page objects via point lookups (≤10 indexed * queries) and publishes them to the UI state. Cheap by construction — never scans * the pages table. + * + * Snapshots the UUID list under [recentMutex], releases the lock, performs DB work + * outside the lock to avoid starving [addToRecent], then re-acquires to write results. */ private suspend fun refreshRecentPages() { - val recent = recentPageUuids.take(10).mapNotNull { uuid -> - recentPagesByUuid[uuid] - ?: pageRepository.getPageByUuid(PageUuid(uuid)).first().getOrNull() - ?.also { recentPagesByUuid[uuid] = it } + val uuidsToResolve = recentMutex.withLock { recentPageUuids.take(10).toList() } + + val resolved = uuidsToResolve.mapNotNull { uuid -> + val cached = recentMutex.withLock { recentPagesByUuid[uuid] } + cached ?: pageRepository.getPageByUuid(PageUuid(uuid)).first().getOrNull() + ?.also { page -> recentMutex.withLock { recentPagesByUuid[uuid] = page } } + } + + recentMutex.withLock { + trimRecentPagesCache() } - trimRecentPagesCache() - _uiState.update { it.copy(recentPages = recent) } + _uiState.update { it.copy(recentPages = resolved) } } private fun trimRecentPagesCache() { @@ -383,30 +402,20 @@ class StelekitViewModel( } } - private fun updateUiStateWithPages(pages: List) { - // This is now handled by observers and loadMore functions - } - private fun addToRecent(page: Page) { - // Remove if exists to move to top - recentPageUuids.remove(page.uuid.value) - recentPageUuids.add(0, page.uuid.value) - - // Keep max 20 items - if (recentPageUuids.size > 20) { - recentPageUuids.removeAt(recentPageUuids.lastIndex) - } - - // Save to settings - platformSettings.putString(recentPagesKey, recentPageUuids.joinToString(",")) - - // Update UI state — the navigated-to page is already in hand; older entries come - // from the bounded recents cache (point lookups happen in refreshRecentPages). - recentPagesByUuid[page.uuid.value] = page - trimRecentPagesCache() - _uiState.update { state -> - val recent = recentPageUuids.mapNotNull { recentPagesByUuid[it] }.take(10) - state.copy(recentPages = recent) + scope.launch { + recentMutex.withLock { + recentPageUuids.remove(page.uuid.value) + recentPageUuids.add(0, page.uuid.value) + if (recentPageUuids.size > 20) { + recentPageUuids.removeAt(recentPageUuids.lastIndex) + } + platformSettings.putString(recentPagesKey, recentPageUuids.joinToString(",")) + recentPagesByUuid[page.uuid.value] = page + trimRecentPagesCache() + val recent = recentPageUuids.mapNotNull { recentPagesByUuid[it] }.take(10) + _uiState.update { it.copy(recentPages = recent) } + } } } @@ -435,10 +444,14 @@ class StelekitViewModel( logger.info("setGraphPath: '$path'") platformSettings.putString("lastGraphPath", path) _uiState.update { it.copy(currentGraphPath = path) } - recentPageUuids = platformSettings.getString(recentPagesKey, "") - .split(",").filter { it.isNotEmpty() }.toMutableList() - recentPagesByUuid.clear() - scope.launch { refreshRecentPages() } + scope.launch { + recentMutex.withLock { + recentPageUuids = platformSettings.getString(recentPagesKey, "") + .split(",").filter { it.isNotEmpty() }.toMutableList() + recentPagesByUuid.clear() + } + refreshRecentPages() + } loadGraph(path) } @@ -561,7 +574,7 @@ class StelekitViewModel( } catch (e: Exception) { val errorText = buildString { append(e::class.simpleName ?: e::class.qualifiedName ?: "UnknownError") - e.message?.let { append(": ", it) } + e.message?.let { append(": ", sanitizeErrorMessage(it)) } } e.printStackTrace() logger.error("Error loading graph: $errorText") @@ -573,7 +586,7 @@ class StelekitViewModel( // report screen where the user can copy the full message for filing a bug. val errorText = buildString { append(e::class.simpleName ?: e::class.qualifiedName ?: "UnknownError") - e.message?.let { append(": ", it) } + e.message?.let { append(": ", sanitizeErrorMessage(it)) } } logger.error("Fatal error loading graph (Throwable): $errorText") _uiState.update { it.copy(isLoading = false, isFullyLoaded = true, statusMessage = "Error: $errorText", fatalError = errorText) } @@ -853,6 +866,10 @@ class StelekitViewModel( @OptIn(DirectRepositoryWrite::class) fun navigateTo(screen: Screen, addToHistory: Boolean = true) { val navStart = kotlin.time.Clock.System.now().toEpochMilliseconds() + // addToRecent must run outside the update lambda — it has side effects (launches a + // coroutine, calls platformSettings) and calls _uiState.update itself, which would + // create a nested update. + if (screen is Screen.PageView) addToRecent(screen.page) _uiState.update { state -> val newHistory = if (addToHistory) { // Trim any forward history and add new screen @@ -869,10 +886,7 @@ class StelekitViewModel( navigationHistory = newHistory, historyIndex = newIndex, statusMessage = when(screen) { - is Screen.PageView -> { - addToRecent(screen.page) - "Opened page: ${screen.page.name}" - } + is Screen.PageView -> "Opened page: ${screen.page.name}" is Screen.Journals -> "Opened Journals" is Screen.Flashcards -> "Opened Flashcards" is Screen.AllPages -> "Opened All Pages" diff --git a/kmp/src/commonMain/kotlin/dev/stapler/stelekit/ui/screens/AllPagesViewModel.kt b/kmp/src/commonMain/kotlin/dev/stapler/stelekit/ui/screens/AllPagesViewModel.kt index d68e9479..ac7feb98 100644 --- a/kmp/src/commonMain/kotlin/dev/stapler/stelekit/ui/screens/AllPagesViewModel.kt +++ b/kmp/src/commonMain/kotlin/dev/stapler/stelekit/ui/screens/AllPagesViewModel.kt @@ -15,8 +15,10 @@ import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.SharingStarted import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.flow.asStateFlow +import kotlinx.coroutines.flow.collect import kotlinx.coroutines.flow.combine import kotlinx.coroutines.flow.debounce +import kotlinx.coroutines.flow.drop import kotlinx.coroutines.flow.first import kotlinx.coroutines.flow.map import kotlinx.coroutines.flow.stateIn @@ -49,8 +51,10 @@ class AllPagesViewModel( private val _pageTypeFilter = MutableStateFlow(PageTypeFilter.ALL) private val _isLoading = MutableStateFlow(true) private val _selectedUuids = MutableStateFlow>(emptySet()) + private val _error = MutableStateFlow(null) val isLoading: StateFlow = _isLoading.asStateFlow() + val error: StateFlow = _error.asStateFlow() val selectedUuids: StateFlow> = _selectedUuids.asStateFlow() val isInSelectionMode: StateFlow = _selectedUuids .map { it.isNotEmpty() } @@ -115,6 +119,14 @@ class AllPagesViewModel( init { scope.launch { loadAllPages() } + // Auto-refresh when the page count changes (e.g. new page created while screen is open). + // drop(1) skips the initial emission that mirrors the init load above. + scope.launch { + pageRepository.countPages() + .drop(1) + .debounce(2_000) + .collect { loadAllPages() } + } } private suspend fun loadAllPages() { @@ -157,6 +169,7 @@ class AllPagesViewModel( throw e } catch (e: Exception) { _isLoading.value = false + _error.value = e.message ?: "Failed to load pages" } } diff --git a/kmp/src/jvmCommonMain/kotlin/dev/stapler/stelekit/performance/InstrumentedPageRepository.kt b/kmp/src/jvmCommonMain/kotlin/dev/stapler/stelekit/performance/InstrumentedPageRepository.kt index 3af53c78..1ad8c945 100644 --- a/kmp/src/jvmCommonMain/kotlin/dev/stapler/stelekit/performance/InstrumentedPageRepository.kt +++ b/kmp/src/jvmCommonMain/kotlin/dev/stapler/stelekit/performance/InstrumentedPageRepository.kt @@ -51,7 +51,7 @@ class InstrumentedPageRepository( override fun getUnloadedPages(limit: Int, offset: Int): Flow>> = delegate.getUnloadedPages(limit, offset) - override fun countUnloadedPages(): Flow> = delegate.countUnloadedPages() + override suspend fun countUnloadedPages(): Either = delegate.countUnloadedPages() // Explicit delegation (not the interface defaults) so the SQL-optimized chunked IN // queries and bounded-batch snapshot of the wrapped repository are preserved. diff --git a/kmp/src/jvmTest/kotlin/dev/stapler/stelekit/db/GraphLoaderIndexBatchingTest.kt b/kmp/src/jvmTest/kotlin/dev/stapler/stelekit/db/GraphLoaderIndexBatchingTest.kt index 88369acc..29b57c51 100644 --- a/kmp/src/jvmTest/kotlin/dev/stapler/stelekit/db/GraphLoaderIndexBatchingTest.kt +++ b/kmp/src/jvmTest/kotlin/dev/stapler/stelekit/db/GraphLoaderIndexBatchingTest.kt @@ -8,7 +8,6 @@ import dev.stapler.stelekit.ui.fixtures.FakeBlockRepository import dev.stapler.stelekit.ui.fixtures.FakeFileSystem import dev.stapler.stelekit.ui.fixtures.FakePageRepository import kotlinx.coroutines.flow.Flow -import kotlinx.coroutines.flow.first import kotlinx.coroutines.runBlocking import kotlinx.coroutines.withTimeout import java.util.concurrent.atomic.AtomicInteger @@ -73,7 +72,7 @@ class GraphLoaderIndexBatchingTest { withTimeout(120_000) { loader.indexRemainingPages { } } - val remaining = pageRepo.countUnloadedPages().first().getOrNull() ?: -1L + val remaining = pageRepo.countUnloadedPages().getOrNull() ?: -1L assertEquals(0L, remaining, "all 250 pages must be indexed") assertTrue( pageRepo.maxBatchLimit.get() in 1..100, @@ -98,7 +97,7 @@ class GraphLoaderIndexBatchingTest { // the drain offset has to advance past them instead of refetching them forever. withTimeout(60_000) { loader.indexRemainingPages { } } - val remaining = pageRepo.countUnloadedPages().first().getOrNull() ?: -1L + val remaining = pageRepo.countUnloadedPages().getOrNull() ?: -1L assertEquals(30L, remaining, "exactly the 30 unreadable pages must remain unloaded") } } diff --git a/kmp/src/jvmTest/kotlin/dev/stapler/stelekit/db/LargeGraphWarmStartCrashTest.kt b/kmp/src/jvmTest/kotlin/dev/stapler/stelekit/db/LargeGraphWarmStartCrashTest.kt index d4f1b58c..cc0e1516 100644 --- a/kmp/src/jvmTest/kotlin/dev/stapler/stelekit/db/LargeGraphWarmStartCrashTest.kt +++ b/kmp/src/jvmTest/kotlin/dev/stapler/stelekit/db/LargeGraphWarmStartCrashTest.kt @@ -14,7 +14,6 @@ import dev.stapler.stelekit.ui.fixtures.InMemorySettings import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.SupervisorJob -import kotlinx.coroutines.delay import kotlinx.coroutines.flow.first import kotlinx.coroutines.runBlocking import kotlinx.coroutines.withTimeout @@ -183,8 +182,12 @@ class LargeGraphWarmStartCrashTest { // The page-name suggestion index must eventually build at this scale. withTimeout(60_000) { - while (vm.suggestionMatcher.value == null) delay(100) + vm.suggestionMatcher.first { it != null } } + assertTrue( + vm.suggestionMatcher.value != null, + "suggestionMatcher must be non-null at 8030-page scale" + ) assertTrue( recorder.uncaught.isEmpty(), diff --git a/kmp/src/jvmTest/kotlin/dev/stapler/stelekit/db/QueryPlanAuditTest.kt b/kmp/src/jvmTest/kotlin/dev/stapler/stelekit/db/QueryPlanAuditTest.kt index c90b1b41..6a2c227a 100644 --- a/kmp/src/jvmTest/kotlin/dev/stapler/stelekit/db/QueryPlanAuditTest.kt +++ b/kmp/src/jvmTest/kotlin/dev/stapler/stelekit/db/QueryPlanAuditTest.kt @@ -37,10 +37,8 @@ class QueryPlanAuditTest { // Aggregate / analytics scans — intentionally full-table "selectDuplicateBlockHashes", "selectMostConnectedBlocks", "selectOrphanedBlocks", // pages columns without an index - "selectUnloadedPagesPaginated", // is_content_loaded has no index; LIMIT bounds rows returned - "countUnloadedPages", // is_content_loaded has no index "selectFavoritePages", // is_favorite has no index; favorites are few by nature - "selectJournalPagesByDates", // journal_date IN — no index on journal_date + "selectJournalPagesByDates", // is_journal and journal_date have no dedicated index; reconcile call is bounded "selectRecentlyUpdatedPages", // updated_at has no index on pages "selectRecentlyCreatedPages", // created_at has no index on pages "selectJournalPages", // is_journal has no index @@ -143,7 +141,7 @@ class QueryPlanAuditTest { AuditQuery("countPages", "SELECT COUNT(*) FROM pages"), AuditQuery("selectJournalPages", - "SELECT * FROM pages WHERE is_journal = 1 ORDER BY COALESCE(journal_date, name) DESC LIMIT 10 OFFSET 0"), + "SELECT * FROM pages WHERE is_journal = 1 AND journal_date IS NOT NULL ORDER BY journal_date DESC LIMIT 10 OFFSET 0"), AuditQuery("selectJournalPageByDate", "SELECT * FROM pages WHERE is_journal = 1 AND journal_date = '2024-01-01' LIMIT 1"), AuditQuery("selectPagesByNameLike", @@ -289,6 +287,14 @@ class QueryPlanAuditTest { "VALUES('p$i','Page $i',NULL,NULL,0,0,NULL,0,0,0,NULL,1,0)" ) } + // Unloaded pages — populates the partial index so the planner can evaluate it + repeat(200) { i -> + seed.execute( + "INSERT OR IGNORE INTO pages(uuid,name,namespace,file_path,created_at,updated_at," + + "properties,version,is_favorite,is_journal,journal_date,is_content_loaded,backlink_count) " + + "VALUES('u$i','Unloaded $i',NULL,NULL,0,0,NULL,0,0,0,NULL,0,0)" + ) + } repeat(5000) { i -> seed.execute( "INSERT OR IGNORE INTO blocks(uuid,page_uuid,parent_uuid,left_uuid,content," + diff --git a/kmp/src/jvmTest/kotlin/dev/stapler/stelekit/repository/UpgradeResilienceTest.kt b/kmp/src/jvmTest/kotlin/dev/stapler/stelekit/repository/UpgradeResilienceTest.kt index f5e2dfe0..fa05bb62 100644 --- a/kmp/src/jvmTest/kotlin/dev/stapler/stelekit/repository/UpgradeResilienceTest.kt +++ b/kmp/src/jvmTest/kotlin/dev/stapler/stelekit/repository/UpgradeResilienceTest.kt @@ -80,7 +80,7 @@ class UpgradeResilienceTest { repoSet.pageRepository.getUnloadedPages(10, 0).first() } assertFlowEmitsLeft("PageRepository.countUnloadedPages") { - repoSet.pageRepository.countUnloadedPages().first() + repoSet.pageRepository.countUnloadedPages() } // ── BlockRepository ─────────────────────────────────────────────── diff --git a/kmp/src/jvmTest/kotlin/dev/stapler/stelekit/ui/StelekitViewModelCrashReproductionTest.kt b/kmp/src/jvmTest/kotlin/dev/stapler/stelekit/ui/StelekitViewModelCrashReproductionTest.kt index 29afb2e7..2b238e4a 100644 --- a/kmp/src/jvmTest/kotlin/dev/stapler/stelekit/ui/StelekitViewModelCrashReproductionTest.kt +++ b/kmp/src/jvmTest/kotlin/dev/stapler/stelekit/ui/StelekitViewModelCrashReproductionTest.kt @@ -101,8 +101,8 @@ class StelekitViewModelCrashReproductionTest { val vm = makeViewModel(repo) withTimeout(10_000) { repo.oomThrown.await() } - // Give the failure time to propagate through the collector coroutines. - delay(1_000) + // Allow the coroutine scheduler to process the catch handler. + delay(200) assertTrue( recorder.uncaught.isEmpty(), @@ -112,6 +112,10 @@ class StelekitViewModelCrashReproductionTest { ) // ViewModel must remain alive and usable after the error. assertNotNull(vm.uiState.value, "uiState must remain readable after the error") + // Note: fatalError is NOT expected here — PageNameIndex.catch {} gracefully + // degrades suggestions (last good matcher retained) rather than surfacing a + // fatal-error screen. The important guarantee is that the OOM is contained and + // does not reach the default uncaught handler (which kills the process on Android). } Unit } diff --git a/kmp/src/jvmTest/kotlin/dev/stapler/stelekit/ui/fixtures/FakeRepositories.kt b/kmp/src/jvmTest/kotlin/dev/stapler/stelekit/ui/fixtures/FakeRepositories.kt index c70d7dd0..879178b1 100644 --- a/kmp/src/jvmTest/kotlin/dev/stapler/stelekit/ui/fixtures/FakeRepositories.kt +++ b/kmp/src/jvmTest/kotlin/dev/stapler/stelekit/ui/fixtures/FakeRepositories.kt @@ -103,8 +103,8 @@ open class FakePageRepository(initialPages: List = emptyList()) : PageRepo .sortedBy { it.uuid.value }.drop(offset).take(limit).right() } - override fun countUnloadedPages(): Flow> = - _pages.map { pages -> pages.values.count { !it.isContentLoaded }.toLong().right() } + override suspend fun countUnloadedPages(): Either = + _pages.value.values.count { !it.isContentLoaded }.toLong().right() override fun getPageNameEntries(): Flow>> = _pages.map { pages -> From 49f932dbae81199bf2038dbe46e3bd65c176af48 Mon Sep 17 00:00:00 2001 From: Tyler Stapler Date: Wed, 10 Jun 2026 23:11:43 -0700 Subject: [PATCH 5/5] fix(recents): defer refreshRecentPages to onPhase1Complete to avoid race with loadGraph clear() setGraphPath previously launched refreshRecentPages() concurrently with loadGraph(), which calls pageRepository.clear() early in the load sequence. The getPageByUuid lookups in refreshRecentPages would silently return null while clear() was in flight, leaving recents empty until the user navigated again. Fix: refreshRecentPages() is now called inside onPhase1Complete, after Phase 1 has populated the new graph's DB with page metadata. Addresses Copilot PR review comment on StelekitViewModel.kt:442. Co-Authored-By: Claude Sonnet 4.6 --- .../kotlin/dev/stapler/stelekit/ui/StelekitViewModel.kt | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/kmp/src/commonMain/kotlin/dev/stapler/stelekit/ui/StelekitViewModel.kt b/kmp/src/commonMain/kotlin/dev/stapler/stelekit/ui/StelekitViewModel.kt index bc1338d7..1224b81a 100644 --- a/kmp/src/commonMain/kotlin/dev/stapler/stelekit/ui/StelekitViewModel.kt +++ b/kmp/src/commonMain/kotlin/dev/stapler/stelekit/ui/StelekitViewModel.kt @@ -450,7 +450,9 @@ class StelekitViewModel( .split(",").filter { it.isNotEmpty() }.toMutableList() recentPagesByUuid.clear() } - refreshRecentPages() + // refreshRecentPages() is deferred to onPhase1Complete inside loadGraph so that + // getPageByUuid lookups run after the new graph's DB is populated, not while + // loadGraph is mid-clear. } loadGraph(path) } @@ -508,6 +510,10 @@ class StelekitViewModel( logger.info("Phase 1 complete - UI is now interactive") _uiState.update { it.copy(isLoading = false, statusMessage = "Ready") } + // Resolve saved recents now that Phase 1 has populated the DB. + // Running before loadGraph is finished would race with clear(). + scope.launch { refreshRecentPages() } + // Ensure today's journal exists so it appears at the top of the // journals list. No navigation — the list updates reactively. scope.launch { journalService.ensureTodayJournal() }