From 93bf86f534c655d4c3d9836effcbe1149a6d08ee Mon Sep 17 00:00:00 2001 From: Cobb Date: Mon, 22 Jun 2026 02:41:24 -0700 Subject: [PATCH] vc=89: bound hide-shorts pagination burst + finish SP-write serialization MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit D-3 — hide-shorts no longer drains a whole channel/search in one burst. The infinite-scroll trigger is derived from the FILTERED list while loadMore() appends to the UNFILTERED one, so a shorts-heavy feed with hide-shorts ON kept the trigger hot (the filter stripped each fetched page back to ~nothing) and auto-fetched every page to the end of the continuation. Now it keeps loading while pages are productive (the filtered list grows) and stops after 3 consecutive pages that add nothing visible; toggling hide-shorts off resumes. Applies to both SearchScreen and ChannelScreen; normal (non-filtered) infinite scroll is unaffected (productive pages keep the counter at 0). PrefsWriter completeness — the four SP stores the vc=88 sweep didn't cover (Enrichment, SearchCache, Playlists, FeedCache) now serialize their writes too, so no store is left on a bare sp.edit().apply(). EnrichmentStore is the one that mattered: put() runs 8-wide concurrently from the feed-enrichment fan-out (Semaphore(8) on Dispatchers.IO), so its apply() ordering genuinely raced — a real M-2 instance the audit's four-store scope missed. FeedCacheStore has no StateFlow (caller-owned state) so it uses the captured-arg form; ordering is safe because its sole writer (the feed VM) serializes save/clear from one refresh coroutine. Verified: headless compileDebugKotlin green on the straw-build image. The vc=88 PrefsWriter concurrency was cleared by an adversarial review before this builds on it. --- buildSrc/src/main/kotlin/ProjectConfig.kt | 23 +++++++++++-- .../com/sulkta/straw/data/EnrichmentStore.kt | 8 +++-- .../com/sulkta/straw/data/FeedCacheStore.kt | 11 +++++-- .../com/sulkta/straw/data/PlaylistsStore.kt | 32 +++++++++++-------- .../com/sulkta/straw/data/SearchCacheStore.kt | 14 +++++--- .../straw/feature/channel/ChannelScreen.kt | 26 ++++++++++++--- .../straw/feature/search/SearchScreen.kt | 27 +++++++++++++--- 7 files changed, 109 insertions(+), 32 deletions(-) diff --git a/buildSrc/src/main/kotlin/ProjectConfig.kt b/buildSrc/src/main/kotlin/ProjectConfig.kt index 8626b2372..d1c34cdeb 100644 --- a/buildSrc/src/main/kotlin/ProjectConfig.kt +++ b/buildSrc/src/main/kotlin/ProjectConfig.kt @@ -9,6 +9,25 @@ const val STRAW_SDK_TARGET = 35 // Sulkta fork — Straw // +// vc=89 / 0.1.0-CW — pagination-burst fix + finish the SP-write serialization: +// * Hide-shorts no longer drains a whole channel/search in one burst. The +// infinite-scroll trigger is computed from the FILTERED list while loadMore() +// appends to the UNFILTERED one, so a shorts-heavy feed with hide-shorts ON +// kept the trigger hot (the filter stripped each fetched page back to +// ~nothing) and auto-fetched every page back-to-back to the end of the +// continuation. Now it keeps loading while pages are productive (the filtered +// list grows) and stops after 3 consecutive pages that add nothing visible; +// toggling hide-shorts off grows the list and resumes. Applies to both Search +// and Channel. (audit D-3) +// * Finished the vc=88 PrefsWriter migration: the remaining four SP stores +// (Enrichment, SearchCache, Playlists, FeedCache) now serialize their writes +// through the same single-thread-per-store dispatcher. EnrichmentStore is the +// one that mattered — put() runs 8-wide concurrently from the feed-enrichment +// fan-out, so its apply() ordering genuinely raced (a real M-2 instance the +// audit's four-store scope missed). No SP store is left on a bare +// sp.edit().apply() now. +// No user-visible behavior change beyond the pagination bound. +// // vc=88 / 0.1.0-CV — deferred-hygiene sweep (audit #2 leftovers, no behavior change): // * SharedPreferences writes across every store (Settings, History, Subs, // ResumePositions) now route through one PrefsWriter — a single-thread @@ -266,6 +285,6 @@ const val STRAW_SDK_TARGET = 35 // vc=19 / 0.1.0-AE — rust pipeline cutover. Extraction via // strawcore-core (Sulkta-Coop/strawcore) via the UniFFI wrapper; no // NewPipeExtractor in the runtime path. -const val STRAW_VERSION_CODE = 88 -const val STRAW_VERSION_NAME = "0.1.0-CV" +const val STRAW_VERSION_CODE = 89 +const val STRAW_VERSION_NAME = "0.1.0-CW" const val STRAW_APPLICATION_ID = "com.sulkta.straw" diff --git a/strawApp/src/main/kotlin/com/sulkta/straw/data/EnrichmentStore.kt b/strawApp/src/main/kotlin/com/sulkta/straw/data/EnrichmentStore.kt index 597688f1e..6d564642d 100644 --- a/strawApp/src/main/kotlin/com/sulkta/straw/data/EnrichmentStore.kt +++ b/strawApp/src/main/kotlin/com/sulkta/straw/data/EnrichmentStore.kt @@ -47,6 +47,10 @@ private const val MAX_ENRICHMENTS = 5_000 class EnrichmentStore(context: Context) { private val sp: SharedPreferences = context.getSharedPreferences(PREFS, Context.MODE_PRIVATE) private val json = Json { ignoreUnknownKeys = true } + // Writes serialized (audit #2 M-2 follow-on): put() runs 8-wide concurrently + // from the feed-enrichment fan-out (Dispatchers.IO), so an unsequenced apply() + // could land out of order. The write reads the live map so disk converges. + private val writer = PrefsWriter(sp) private val _entries = MutableStateFlow(load()) val entries: StateFlow> = _entries.asStateFlow() @@ -98,13 +102,13 @@ class EnrichmentStore(context: Context) { } } if (next !== before) { - sp.edit().putString(KEY, json.encodeToString(next)).apply() + writer.write { putString(KEY, json.encodeToString(_entries.value)) } } } fun clear() { _entries.updateAndGet { emptyMap() } - sp.edit().putString(KEY, json.encodeToString(emptyMap())).apply() + writer.write { putString(KEY, json.encodeToString(_entries.value)) } } private fun load(): Map = runCatching { diff --git a/strawApp/src/main/kotlin/com/sulkta/straw/data/FeedCacheStore.kt b/strawApp/src/main/kotlin/com/sulkta/straw/data/FeedCacheStore.kt index eab61b975..00067f055 100644 --- a/strawApp/src/main/kotlin/com/sulkta/straw/data/FeedCacheStore.kt +++ b/strawApp/src/main/kotlin/com/sulkta/straw/data/FeedCacheStore.kt @@ -37,6 +37,13 @@ private const val KEY = "cache_v1" class FeedCacheStore(context: Context) { private val sp: SharedPreferences = context.getSharedPreferences(PREFS, Context.MODE_PRIVATE) private val json = Json { ignoreUnknownKeys = true } + // Writes serialized off-thread (audit #2 M-2 follow-on). Unlike the StateFlow + // stores, this cache's state is caller-owned (the feed VM's channelCache), so + // there's no live value to re-read — the write captures the encoded snapshot. + // Ordering is safe because the only writer (SubscriptionFeedViewModel) calls + // save()/clear() from a single refresh coroutine (cancels in-flight before + // clear), so enqueue order == intended order and the FIFO dispatcher preserves it. + private val writer = PrefsWriter(sp) /** * Snapshot of the disk cache, filtered by the user-configured TTL. @@ -56,11 +63,11 @@ class FeedCacheStore(context: Context) { /** Atomic write. Caller is responsible for diffing if needed. */ fun save(map: Map) { val s = json.encodeToString(map) - sp.edit().putString(KEY, s).apply() + writer.write { putString(KEY, s) } } fun clear() { - sp.edit().remove(KEY).apply() + writer.write { remove(KEY) } } } diff --git a/strawApp/src/main/kotlin/com/sulkta/straw/data/PlaylistsStore.kt b/strawApp/src/main/kotlin/com/sulkta/straw/data/PlaylistsStore.kt index 26ba0e16d..197d6ece3 100644 --- a/strawApp/src/main/kotlin/com/sulkta/straw/data/PlaylistsStore.kt +++ b/strawApp/src/main/kotlin/com/sulkta/straw/data/PlaylistsStore.kt @@ -48,6 +48,10 @@ private const val KEY = "playlists_v1" class PlaylistsStore(context: Context) { private val sp: SharedPreferences = context.getSharedPreferences(PREFS, Context.MODE_PRIVATE) private val json = Json { ignoreUnknownKeys = true } + // Writes serialized (audit #2 M-2 follow-on): the importer edits playlists on + // Dispatchers.IO while the user edits them on Main; the write reads the live + // list so disk converges to the latest in-memory state. + private val writer = PrefsWriter(sp) private val _playlists = MutableStateFlow(load()) val playlists: StateFlow> = _playlists.asStateFlow() @@ -58,8 +62,8 @@ class PlaylistsStore(context: Context) { name = name.trim().ifBlank { "Untitled" }, createdAt = System.currentTimeMillis(), ) - val next = _playlists.updateAndGet { it + pl } - persist(next) + _playlists.updateAndGet { it + pl } + persist() return pl } @@ -86,50 +90,50 @@ class PlaylistsStore(context: Context) { createdAt = stampNow, items = deduped, ) - val next = _playlists.updateAndGet { it + pl } - persist(next) + _playlists.updateAndGet { it + pl } + persist() return pl } fun delete(id: String) { - val next = _playlists.updateAndGet { cur -> cur.filterNot { it.id == id } } - persist(next) + _playlists.updateAndGet { cur -> cur.filterNot { it.id == id } } + persist() } fun rename(id: String, newName: String) { val trimmed = newName.trim().ifBlank { return } - val next = _playlists.updateAndGet { cur -> + _playlists.updateAndGet { cur -> cur.map { if (it.id == id) it.copy(name = trimmed) else it } } - persist(next) + persist() } fun addItem(playlistId: String, item: PlaylistItem) { val stamped = item.copy(addedAt = System.currentTimeMillis()) - val next = _playlists.updateAndGet { cur -> + _playlists.updateAndGet { cur -> cur.map { pl -> if (pl.id != playlistId) pl else if (pl.items.any { it.streamUrl == stamped.streamUrl }) pl else pl.copy(items = pl.items + stamped) } } - persist(next) + persist() } fun removeItem(playlistId: String, streamUrl: String) { - val next = _playlists.updateAndGet { cur -> + _playlists.updateAndGet { cur -> cur.map { pl -> if (pl.id != playlistId) pl else pl.copy(items = pl.items.filterNot { it.streamUrl == streamUrl }) } } - persist(next) + persist() } fun get(id: String): Playlist? = _playlists.value.firstOrNull { it.id == id } - private fun persist(list: List) { - sp.edit().putString(KEY, json.encodeToString(list)).apply() + private fun persist() { + writer.write { putString(KEY, json.encodeToString(_playlists.value)) } } private fun load(): List = runCatching { diff --git a/strawApp/src/main/kotlin/com/sulkta/straw/data/SearchCacheStore.kt b/strawApp/src/main/kotlin/com/sulkta/straw/data/SearchCacheStore.kt index 79fd89c7c..f0a94f2e6 100644 --- a/strawApp/src/main/kotlin/com/sulkta/straw/data/SearchCacheStore.kt +++ b/strawApp/src/main/kotlin/com/sulkta/straw/data/SearchCacheStore.kt @@ -47,6 +47,11 @@ private const val MAX_ITEMS_PER_QUERY = 20 class SearchCacheStore(context: Context) { private val sp: SharedPreferences = context.getSharedPreferences(PREFS, Context.MODE_PRIVATE) private val json = Json { ignoreUnknownKeys = true } + // Writes serialized (audit #2 M-2 follow-on): record() (search submit, IO) and + // clear() (Settings, IO) both write this file; the write reads the live list so + // disk converges to the latest in-memory state and a record can't resurrect a + // just-cleared entry on disk. + private val writer = PrefsWriter(sp) private val _entries = MutableStateFlow(loadFromDisk()) val entries: StateFlow> = _entries.asStateFlow() @@ -82,20 +87,21 @@ class SearchCacheStore(context: Context) { if (q.isEmpty() || items.isEmpty()) return val capped = items.take(MAX_ITEMS_PER_QUERY) val now = System.currentTimeMillis() - val next = _entries.updateAndGet { current -> + _entries.updateAndGet { current -> val without = current.filterNot { it.query.equals(q, ignoreCase = true) } (listOf(SearchCacheEntry(q, now, capped)) + without).take(maxQueries()) } - sp.edit().putString(KEY, json.encodeToString(next)).apply() + writer.write { putString(KEY, json.encodeToString(_entries.value)) } } fun clear() { // updateAndGet (not a plain `.value =`) so a concurrent record() // interleaving its read→build→persist can't leave disk = [entry] after // the user asked to clear — the same B5 race the StateFlow migration was - // meant to close (audit #2 M-3). + // meant to close (audit #2 M-3). The serialized write reads the live list, + // so disk converges to empty (encoded "[]", which loads back as empty). _entries.updateAndGet { emptyList() } - sp.edit().remove(KEY).apply() + writer.write { putString(KEY, json.encodeToString(_entries.value)) } } private fun loadFromDisk(): List = runCatching { diff --git a/strawApp/src/main/kotlin/com/sulkta/straw/feature/channel/ChannelScreen.kt b/strawApp/src/main/kotlin/com/sulkta/straw/feature/channel/ChannelScreen.kt index 1935150b3..5b52242b6 100644 --- a/strawApp/src/main/kotlin/com/sulkta/straw/feature/channel/ChannelScreen.kt +++ b/strawApp/src/main/kotlin/com/sulkta/straw/feature/channel/ChannelScreen.kt @@ -61,6 +61,10 @@ import com.sulkta.straw.util.formatCount import com.sulkta.straw.util.rememberBottomContentPadding import com.sulkta.straw.util.formatDuration +/** Max consecutive auto-pagination loads that add no visible (filtered) items + * before we stop — bounds the hide-shorts pagination burst (audit D-3). */ +private const val MAX_UNPRODUCTIVE_PAGE_LOADS = 3 + @Composable fun ChannelScreen( channelUrl: String, @@ -121,10 +125,24 @@ fun ChannelScreen( total > 0 && lastVisible >= total - 3 } } - LaunchedEffect(shouldLoadMore, state.videosContinuation, state.loadingMore) { - if (shouldLoadMore && state.videosContinuation != null && !state.loadingMore) { - vm.loadMore() - } + // Bound the hide-shorts pagination burst (audit D-3): shouldLoadMore is + // derived from the FILTERED list while loadMore() appends to the UNFILTERED + // one, so a shorts-heavy channel with hide-shorts ON keeps the trigger hot + // (the filter strips each fetched page back to ~nothing) and would drain the + // whole channel in one burst. Keep loading while pages are productive (the + // filtered list grows); stop after MAX_UNPRODUCTIVE_PAGE_LOADS consecutive + // pages that added nothing visible. Counters key off the result set (page-1 + // url) so opening a different channel resets them; toggling hide-shorts off + // grows the filtered list and resumes loading. + val resultSetKey = state.videos.firstOrNull()?.url ?: "" + var lastFilteredCount by remember(resultSetKey) { mutableStateOf(0) } + var unproductiveLoads by remember(resultSetKey) { mutableStateOf(0) } + LaunchedEffect(shouldLoadMore, state.videosContinuation, state.loadingMore, filteredVideos.size) { + if (!shouldLoadMore || state.videosContinuation == null || state.loadingMore) return@LaunchedEffect + if (filteredVideos.size > lastFilteredCount) unproductiveLoads = 0 else unproductiveLoads++ + lastFilteredCount = filteredVideos.size + if (unproductiveLoads >= MAX_UNPRODUCTIVE_PAGE_LOADS) return@LaunchedEffect + vm.loadMore() } LazyColumn( state = listState, diff --git a/strawApp/src/main/kotlin/com/sulkta/straw/feature/search/SearchScreen.kt b/strawApp/src/main/kotlin/com/sulkta/straw/feature/search/SearchScreen.kt index ae61d27a6..268db1ffb 100644 --- a/strawApp/src/main/kotlin/com/sulkta/straw/feature/search/SearchScreen.kt +++ b/strawApp/src/main/kotlin/com/sulkta/straw/feature/search/SearchScreen.kt @@ -60,6 +60,10 @@ import com.sulkta.straw.util.formatDuration import com.sulkta.straw.util.formatViews import com.sulkta.straw.util.rememberBottomContentPadding +/** Max consecutive auto-pagination loads that add no visible (filtered) items + * before we stop — bounds the hide-shorts pagination burst (audit D-3). */ +private const val MAX_UNPRODUCTIVE_PAGE_LOADS = 3 + @Composable fun SearchScreen( onOpenVideo: (url: String, title: String) -> Unit, @@ -181,10 +185,25 @@ fun SearchScreen( total > 0 && lastVisible >= total - 3 } } - LaunchedEffect(shouldLoadMore, state.continuation, state.loadingMore) { - if (shouldLoadMore && state.continuation != null && !state.loadingMore) { - vm.loadMore() - } + // Bound the hide-shorts pagination burst (audit D-3): shouldLoadMore + // is derived from the FILTERED list while loadMore() appends to the + // UNFILTERED one, so a shorts-heavy result set with hide-shorts ON + // keeps the trigger hot (the filter strips each fetched page back to + // ~nothing) and would drain the whole continuation in one burst. Keep + // loading while pages are productive (the filtered list grows); stop + // after MAX_UNPRODUCTIVE_PAGE_LOADS consecutive pages that added + // nothing visible. Counters are keyed to the result set (page-1 url + + // fromCache) so a fresh submit / cache swap resets them; toggling + // hide-shorts off grows the filtered list and resumes loading. + val resultSetKey = state.results.firstOrNull()?.url ?: "" + var lastFilteredCount by remember(resultSetKey, state.fromCache) { mutableStateOf(0) } + var unproductiveLoads by remember(resultSetKey, state.fromCache) { mutableStateOf(0) } + LaunchedEffect(shouldLoadMore, state.continuation, state.loadingMore, filteredResults.size) { + if (!shouldLoadMore || state.continuation == null || state.loadingMore) return@LaunchedEffect + if (filteredResults.size > lastFilteredCount) unproductiveLoads = 0 else unproductiveLoads++ + lastFilteredCount = filteredResults.size + if (unproductiveLoads >= MAX_UNPRODUCTIVE_PAGE_LOADS) return@LaunchedEffect + vm.loadMore() } LazyColumn( state = listState,