refactor(read-state): persist local/remote completion flags in pending ops by johan456789 · Pull Request #70 · johan456789/ReadYou

johan456789 · 2026-05-05T17:13:45Z

Summary

Refs #69

Refactors the read-state persistence layer to track local and remote completion separately using two new boolean flags on pending_read_state_op:

localCommitted — set when the article table is updated
remoteSynced — set when the remote server acknowledges the change (pre-set to true for local accounts)

This replaces the previous approach of deleting pending ops immediately after local commit, which could lose state if the app crashed before remote sync completed.

Changes

Add localCommitted and remoteSynced columns to PendingReadStateOp entity
Add Room migration (v10 → v11) with default values
Add new DAO methods: queryLocalPending, queryRemotePending, markLocalCommitted, markRemoteSynced, deleteCompleted
Rework DiffMapHolder to use the new flags for startup replay and sync coordination
Add unit tests for the new pending read state behavior

Test plan

Unit tests pass (./gradlew testFdroidDebugUnitTest)
Manual verification in emulator (no regression):
- Issue in-app read status is gone probably after the app get evicted from memory #67: Read state persists after app restart
- Issue ui doesnt preserve read articles on mark as read in the unread filter feed #14: Toggle read/unread button works
- Issue read articles incorrectly removed in unread view after exiting the reading page #51: "Mark all as read" action works
- Issue marking read doesn't make article grayed out #59: Read state persists after "mark all as read" + restart

Made with Cursor

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: f336fa5eaf

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-05-05T17:17:37Z

            applicationScope.launch(ioDispatcher) {
-                commitAppliedDiffsToDb(diffsToCommitNow)
+                commitAppliedDiffsToDb(appliedDiffs.associateBy { it.articleId })


Serialize persist and local-commit writes

updateDiff now launches persistPendingReadStateOps and commitAppliedDiffsToDb in separate fire-and-forget coroutines, so when deferDbCommits is false the commit path can run before the upsert finishes. In that ordering, markLocalCommitted(...) updates zero rows (because the row does not exist yet), then the later upsert re-inserts the op with localCommitted=0, causing already-applied changes to be replayed again on restart/flush and leaving incorrect pending state until another flush happens.

Useful? React with 👍 / 👎.

chatgpt-codex-connector · 2026-05-05T17:17:37Z

+        )
+        database.execSQL(
+            """
+            ALTER TABLE pending_read_state_op ADD COLUMN remoteSynced INTEGER NOT NULL DEFAULT 0


Backfill remoteSynced for local-account pending rows

The 10→11 migration sets remoteSynced to 0 for all existing pending_read_state_op rows, but local accounts never run remote sync and therefore never call markRemoteSynced(...). After replay, those rows become localCommitted=1, remoteSynced=0, so deleteCompleted() can never remove them; users upgrading with preexisting local pending ops will accumulate orphaned rows permanently.

Useful? React with 👍 / 👎.

gemini-code-assist

Code Review

This pull request refactors the article read-state synchronization logic by introducing "localCommitted" and "remoteSynced" flags to the "PendingReadStateOp" database entity, replacing the previous in-memory deferred diff tracking. Key changes include a database migration to version 11 and updated DAO queries to manage local and remote pending operations. Feedback identifies a high-severity race condition in "updateDiff" where persistence and database commitment are launched in parallel coroutines. Additionally, several improvements are suggested to use "partition" for more efficient collection handling and to add "isNotEmpty" guards before batch service calls to prevent redundant operations.

gemini-code-assist · 2026-05-05T17:18:56Z

+        applicationScope.launch(ioDispatcher) {
+            persistPendingReadStateOps(appliedDiffs)
+        }
+
+        if (!deferDbCommits) {
+            Timber.tag("DiffMapHolder").d("Immediately committing ${appliedDiffs.size} articles to DB")
            applicationScope.launch(ioDispatcher) {
-                commitAppliedDiffsToDb(diffsToCommitNow)
+                commitAppliedDiffsToDb(appliedDiffs.associateBy { it.articleId })
            }
+        } else {
+            Timber.tag("DiffMapHolder").d("Deferring DB commits for ${appliedDiffs.size} articles")
        }


Launching persistPendingReadStateOps and commitAppliedDiffsToDb in separate coroutines on the same dispatcher creates a race condition. If commitAppliedDiffsToDb executes its markLocalCommitted query before persistPendingReadStateOps has finished its upsertAll, the update will affect zero rows. This results in the pending operation remaining in the database with localCommitted = false, leading to redundant replay attempts on the next app startup. These operations should be sequenced within a single coroutine to ensure correctness.

applicationScope.launch(ioDispatcher) { persistPendingReadStateOps(appliedDiffs) if (!deferDbCommits) { Timber.tag("DiffMapHolder").d("Immediately committing ${appliedDiffs.size} articles to DB") commitAppliedDiffsToDb(appliedDiffs.associateBy { it.articleId }) } else { Timber.tag("DiffMapHolder").d("Deferring DB commits for ${appliedDiffs.size} articles") } }

gemini-code-assist · 2026-05-05T17:18:56Z

+        val markAsReadIds = pendingOps.filter { it.isRead }.map { it.articleId }.toSet()
+        val markAsUnreadIds = pendingOps.filter { !it.isRead }.map { it.articleId }.toSet()
+
+        rssService.get().batchMarkAsRead(articleIds = markAsReadIds, markRead = true)
+        rssService.get().batchMarkAsRead(articleIds = markAsUnreadIds, markRead = false)


The current implementation iterates over pendingOps twice to separate read and unread IDs. Using partition is more idiomatic and efficient. Additionally, it is best practice to guard the batchMarkAsRead service calls with isNotEmpty() checks to avoid unnecessary network or database overhead when one of the sets is empty.

val (readOps, unreadOps) = pendingOps.partition { it.isRead } val markAsReadIds = readOps.map { it.articleId }.toSet() val markAsUnreadIds = unreadOps.map { it.articleId }.toSet() if (markAsReadIds.isNotEmpty()) { rssService.get().batchMarkAsRead(articleIds = markAsReadIds, markRead = true) } if (markAsUnreadIds.isNotEmpty()) { rssService.get().batchMarkAsRead(articleIds = markAsUnreadIds, markRead = false) }

gemini-code-assist · 2026-05-05T17:18:56Z

        rssService.get().batchMarkAsRead(articleIds = diffBatch.markReadIds, markRead = true)
        rssService.get().batchMarkAsRead(articleIds = diffBatch.markUnreadIds, markRead = false)


It is recommended to guard the batchMarkAsRead calls with isNotEmpty() checks to prevent no-op service invocations.

Suggested change

rssService.get().batchMarkAsRead(articleIds = diffBatch.markReadIds, markRead = true)

rssService.get().batchMarkAsRead(articleIds = diffBatch.markUnreadIds, markRead = false)

if (diffBatch.markReadIds.isNotEmpty()) {

rssService.get().batchMarkAsRead(articleIds = diffBatch.markReadIds, markRead = true)

}

if (diffBatch.markUnreadIds.isNotEmpty()) {

rssService.get().batchMarkAsRead(articleIds = diffBatch.markUnreadIds, markRead = false)

}

gemini-code-assist · 2026-05-05T17:18:56Z

+            val markAsReadIds = pendingOps.filter { it.isRead }.map { it.articleId }.toSet()
+            val markAsUnreadIds = pendingOps.filter { !it.isRead }.map { it.articleId }.toSet()
+
+            rssService.get().batchMarkAsRead(articleIds = markAsReadIds, markRead = true)
+            rssService.get().batchMarkAsRead(articleIds = markAsUnreadIds, markRead = false)


Similar to flushLocalPendingOps, using partition here is more efficient than multiple filters. Also, ensure batchMarkAsRead is only called when the corresponding ID sets are not empty.

val (readOps, unreadOps) = pendingOps.partition { it.isRead } val markAsReadIds = readOps.map { it.articleId }.toSet() val markAsUnreadIds = unreadOps.map { it.articleId }.toSet() if (markAsReadIds.isNotEmpty()) { rssService.get().batchMarkAsRead(articleIds = markAsReadIds, markRead = true) } if (markAsUnreadIds.isNotEmpty()) { rssService.get().batchMarkAsRead(articleIds = markAsUnreadIds, markRead = false) }

gemini-code-assist · 2026-05-05T17:18:56Z

        val markReadIds = diffs.filter { it.isRead }.map { it.articleId }.toSet()
        val markUnreadIds = diffs.filter { !it.isRead }.map { it.articleId }.toSet()


Using partition here avoids double iteration over the diffs collection.

val (readDiffs, unreadDiffs) = diffs.partition { it.isRead } val markReadIds = readDiffs.map { it.articleId }.toSet() val markUnreadIds = unreadDiffs.map { it.articleId }.toSet()

refactor(read-state): persist local/remote completion in pending ops

f336fa5

chatgpt-codex-connector Bot reviewed May 5, 2026

View reviewed changes

gemini-code-assist Bot reviewed May 5, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor(read-state): persist local/remote completion flags in pending ops#70

refactor(read-state): persist local/remote completion flags in pending ops#70
johan456789 wants to merge 1 commit into
mainfrom
refactor-after-69-pending-read-state

johan456789 commented May 5, 2026 •

edited

Loading

Uh oh!

chatgpt-codex-connector Bot left a comment

Uh oh!

chatgpt-codex-connector Bot May 5, 2026

Uh oh!

chatgpt-codex-connector Bot May 5, 2026

Uh oh!

gemini-code-assist Bot left a comment

Uh oh!

gemini-code-assist Bot May 5, 2026

Uh oh!

gemini-code-assist Bot May 5, 2026

Uh oh!

gemini-code-assist Bot May 5, 2026

Uh oh!

gemini-code-assist Bot May 5, 2026

Uh oh!

gemini-code-assist Bot May 5, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

		rssService.get().batchMarkAsRead(articleIds = diffBatch.markReadIds, markRead = true)
		rssService.get().batchMarkAsRead(articleIds = diffBatch.markUnreadIds, markRead = false)

		val markReadIds = diffs.filter { it.isRead }.map { it.articleId }.toSet()
		val markUnreadIds = diffs.filter { !it.isRead }.map { it.articleId }.toSet()

Conversation

johan456789 commented May 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Changes

Test plan

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot May 5, 2026

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector Bot May 5, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist Bot May 5, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot May 5, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot May 5, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot May 5, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot May 5, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

johan456789 commented May 5, 2026 •

edited

Loading