Fix progress clobber, cancellation propagation, and reader recomposition#71
Merged
Conversation
…ndaries Cancelled coroutines were being treated as failures: BaseViewModel flashed 'unknown error' states, ContentRepository fabricated retryable results (and inspectCache memoized them for 3s), and repositories logged cancellations as errors. Add Result.rethrowCancellation() and apply it at the confirmed sites. Also guard ImageDimensionManager's dimension flush against persistAll failures crashing the scope, and make openNewChapter single-flight so rapid taps don't run concurrent fetches.
…ource refreshLibraryUpdates snapshotted all rows, spent seconds on network fetches, then bulk-inserted the stale copies with whole-row REPLACE — reverting any reading progress written during the refresh window. It also grouped by baseTitle alone, stamping one source's chapter count onto every source's rows for multi-source series. Group by (baseTitle, sourceName) and write only totalChapters/hasUpdates through targeted UPDATE queries, leaving progress fields untouched.
…ecomposition The recapture effect used listState.firstVisibleItemScrollOffset (and index/ currentPage) as LaunchedEffect keys, so composition read the scroll offset directly: every scrolled pixel recomposed the whole ContentArea and cancelled/relaunched the effect, even with controls hidden. Read the values inside a snapshotFlow instead; collectLatest preserves the settle debounce.
Chapter list, settings, and the Cloudflare dialog used plain remember, so process death while open silently closed them. Use rememberSaveable.
save() wrote directly to the target file; a crash or concurrent save could leave corrupt JSON (load degrades to a refetch, but avoidably). Write to a temp file and rename, mirroring WebOfflineChapterStore.writeManifest.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes from a full-codebase review (findings verified against source before fixing).
Changes
refreshLibraryUpdatesused to snapshot all rows, fetch chapter counts for seconds, then bulk-REPLACE whole rows — clobbering any progress written during the window. It now groups by(baseTitle, sourceName)and writes onlytotalChapters/hasUpdatesthrough targeted UPDATE queries, which also stops one source's chapter count from being stamped onto another source's rows.CancellationExceptionpropagates again. NewResult.rethrowCancellation()applied at the confirmedrunCatchingsites (BaseViewModel, repositories,openNewChapter), so cancelled loads no longer flash error states, fabricate retryable results, or poison the 3s inspect memo. Also hardens the image-dimension flush against crashes and makesopenNewChaptersingle-flight.snapshotFlowinstead of as composition-read LaunchedEffect keys, removing whole-ContentArea recomposition + effect churn on every scrolled pixel. Verified on-device (S22, 120Hz): no frame-time regression (~0.8% jank, p95 5ms), blur recapture still works after scroll settles.rememberSaveable).ChapterListCachewrites atomically (temp + rename).Verification