Skip to content

Android: stop over-fetching after local edits and on WS echoes#307

Merged
dkhalife merged 2 commits intomainfrom
reduce_sync_fanout
Apr 24, 2026
Merged

Android: stop over-fetching after local edits and on WS echoes#307
dkhalife merged 2 commits intomainfrom
reduce_sync_fanout

Conversation

@dkhalife
Copy link
Copy Markdown
Owner

Problem

A single local edit on Android triggers up to 3 full GET /tasks + 3 GET /labels round-trips:

  1. TaskRepository.updateTask (or similar) calls syncCoordinator.syncOnce(), which flushes the outbox (necessary) then full-refreshes (not necessary — we just wrote the authoritative state).
  2. The server broadcasts a WS event → WebSocketSyncBridge → another syncOnceBlocking → another full refresh.
  3. TaskListViewModel / LabelViewModel independently listen to the same WS events and ALSO call refreshTasks() / refreshLabels() → yet another full refresh.

The WS echo alone is sufficient to reconcile authoritative server state. The intuition: writes should only flush; refresh happens on the incoming echo.

Changes

  • SyncCoordinator.flushPending(): new flush-only path for locally-initiated mutations. Coalesces with any in-flight sync (the running outbox loop will drain the newly inserted row).
  • Repositories: every write-path syncCoordinator.syncOnce()syncCoordinator.flushPending(). The unused updateTasksFromWebSocket / updateLabelsFromWebSocket helpers (no callers) were removed.
  • syncOnceBlocking coalesce: if a syncOnce job is already running, join it instead of queueing a duplicate full cycle. This also dedupes the app-start + first-screen double sync.
  • WebSocketSyncBridge: TASK_EVENTSSYNC_EVENTS, now also includes label_* events so per-ViewModel WS listeners can go away.
  • LabelViewModel: WS listener dropped entirely; DB Flow observation covers it. WebSocketManager dep removed.
  • TaskListViewModel: WS listener trimmed to only refresh completed tasks on relevant events — that endpoint is not covered by SyncCoordinator.refreshAll.

Net effect

A single local edit now costs: 1 write request + 1 debounced background refresh (from the WS echo, ~500 ms later). Down from up to 7 round-trips. Remote changes (other clients) still reconcile in ≤500 ms via the same WS path.

- Add SyncCoordinator.flushPending(): flush-only path for local mutations, no server refresh. The WebSocket echo of the write drives reconciliation.
- Route all repository write paths to flushPending() instead of syncOnce(); drop unused updateTasksFromWebSocket/updateLabelsFromWebSocket helpers.
- Coalesce syncOnceBlocking with any in-flight sync: if a syncOnce job is already running, join it instead of queueing a duplicate full cycle.
- Extend WebSocketSyncBridge to also handle label_* events, so per-ViewModel WS listeners can go away.
- Drop collectWebSocketMessages from LabelViewModel entirely; trim TaskListViewModel's listener to only refresh completed tasks (the only data not covered by SyncCoordinator).

Net effect: a single local edit now produces one server write + at most one background refresh (triggered by the WS echo, debounced 500ms), instead of 3 full refreshes.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings April 24, 2026 01:29
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Reduces redundant full refreshes on Android by separating “flush outbox” from “refresh from server”, and consolidating WebSocket-triggered reconciliation through WebSocketSyncBridge + SyncCoordinator.

Changes:

  • Introduces SyncCoordinator.flushPending() and updates task/label write paths to flush-only (no immediate full refresh).
  • Coalesces syncOnceBlocking() calls to avoid duplicate full sync cycles.
  • Expands WebSocketSyncBridge to trigger reconciliation for both task and label WS events; trims/removes per-ViewModel WS listeners accordingly.
Show a summary per file
File Description
android/app/src/main/java/com/dkhalife/tasks/viewmodel/TaskListViewModel.kt Limits WS-driven refreshes to completed-task endpoint only.
android/app/src/main/java/com/dkhalife/tasks/viewmodel/LabelViewModel.kt Removes WS listener dependency; relies on repository/DB updates.
android/app/src/main/java/com/dkhalife/tasks/repo/TaskRepository.kt Switches write paths from syncOnce() to flushPending(); removes unused WS helper.
android/app/src/main/java/com/dkhalife/tasks/repo/LabelRepository.kt Switches write paths from syncOnce() to flushPending(); removes unused WS helper.
android/app/src/main/java/com/dkhalife/tasks/data/sync/WebSocketSyncBridge.kt Broadens event set (tasks + labels) that triggers debounced reconciliation sync.
android/app/src/main/java/com/dkhalife/tasks/data/sync/SyncCoordinator.kt Adds flush-only path and modifies syncOnceBlocking() to join in-flight work.

Copilot's findings

  • Files reviewed: 6/6 changed files
  • Comments generated: 3

Comment thread android/app/src/main/java/com/dkhalife/tasks/viewmodel/TaskListViewModel.kt Outdated
- Track activeFullJob only for full sync cycles; syncOnceBlocking() will no longer join a flush-only job and skip its required refresh.
- flushPending() always launches a coroutine that serializes on the mutex, so newly-inserted outbox rows cannot be stranded by an in-flight refresh; redundant concurrent flushes collapse into cheap no-ops.
- Clarify TaskListViewModel WS comment.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@dkhalife dkhalife merged commit 5804bc3 into main Apr 24, 2026
6 checks passed
@dkhalife dkhalife deleted the reduce_sync_fanout branch April 24, 2026 01:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants