Skip to content

feat(sync): surface per-peer hostnames in sync report UI - #303

Merged
ErikBjare merged 2 commits into
ActivityWatch:masterfrom
TimeToBuildBob:feat/per-peer-names
Oct 10, 2026
Merged

ErikBjare merged 2 commits into
ActivityWatch:masterfrom
TimeToBuildBob:feat/per-peer-names

Conversation

@TimeToBuildBob

Copy link
Copy Markdown
Contributor

Summary

The Rust JNI to_jni_json() already emits a full peers array with per-device hostname and outcome.kind; only the aggregate counts (peers_imported, peers_skipped, peers_failed) were parsed on the Kotlin side.

This PR wires the per-peer data through:

  • Adds SyncPeer(hostname, outcome) data class to SyncInterface.kt
  • Extends SyncStatus with peers: List<SyncPeer> (empty-default — fully backward-compatible with older native libs that don't emit the peers key)
  • Parses the peers JSON array in fromJniResponse()
  • Updates formatSyncDetail() to append hostnames in parens after the aggregate peer line, e.g.:
    pulled 1200, pushed 3 · peers 2/4 imported, 1 skipped, 1 failed (desktop, laptop, !server)
    
    ! prefix marks failed peers for quick visual distinction without extra prose
  • 4 new unit tests: peer parsing, rendering with names, no-names fallback, empty-peers-key fallback

Closes #285

Test plan

  • Unit tests pass: ./gradlew :mobile:testStandardDebugUnitTest --tests "net.activitywatch.android.SyncSettingsActivityTest"
  • On a device with multiple sync peers, the last sync detail line shows hostnames in parens
  • On a device with only one peer or an older native lib, the aggregate line renders unchanged

Co-Authored-By: Bob timetobuildbob@gmail.com

The Rust JNI `to_jni_json()` already emits a `peers` array with per-device
`hostname` and `outcome.kind` fields; only aggregate counts were parsed on
the Kotlin side.

- Add `SyncPeer(hostname, outcome)` data class
- Extend `SyncStatus` with `peers: List<SyncPeer>` (empty-default for
  backward compatibility with older native libs)
- Parse the `peers` JSON array in `fromJniResponse()`
- Update `formatSyncDetail()` to append `(desktop, laptop, !server)` after
  the aggregate peer line when per-peer data is available; `!` prefix marks
  failed peers for instant visual distinction without extra prose
- 4 new unit tests covering parsing, rendering, and the no-peers fallback

Closes ActivityWatch#285

Git-Session-Id: e1f4
@greptile-apps

greptile-apps Bot commented Sep 23, 2026 •

Copy link
Copy Markdown

RetriggerConfidence Score: 5/5

The PR appears safe to merge; the previously reported loss of peer names across the preferences reload is fully addressed.

Findings

  1. P1 Peer names are discarded ▶

Summary

This PR surfaces peer hostnames and outcomes in the sync report UI and now preserves that data across the SharedPreferences reload used by the settings screen.

  • Parses per-peer hostname and outcome information from JNI responses.
  • Persists and restores peer summaries with defensive malformed-data handling.
  • Displays imported and failed peer names alongside aggregate sync counts.
  • Adds parsing, formatting, fallback, and persistence round-trip coverage.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart LR
    JNI[JNI sync report JSON] --> Parse[SyncStatus.fromJniResponse]
    Parse --> Status[SyncStatus with peer summaries]
    Status --> Encode[Encode peers as JSON]
    Encode --> Prefs[SharedPreferences]
    Prefs --> Decode[Decode persisted peers]
    Decode --> Format[formatSyncDetail]
    Format --> UI[Sync detail with peer hostnames]
Loading

Reviews (2) · Last reviewed commit: "fix(sync): persist per-peer names throug..."

Comment on lines +52 to +54
// Per-peer breakdown from the "peers" array in the JNI response. Empty for
// older native libs that pre-date the SyncReport JNI output.
val peers: List<SyncPeer> = emptyList(),

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Peer names are discarded

peers is added to SyncStatus, but the SharedPreferences adapter neither writes nor restores it. The settings UI always reloads the status through prefs.getLastSyncStatus(), so this field defaults to an empty list even after the completion broadcast. As a result, the new hostname rendering never appears in the actual UI; persist and restore the peer summaries alongside the aggregate fields.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed in 8752040. AWPreferences now persists peers as JSON (lastSyncPeers) next to the aggregate counts, and getLastSyncStatus() restores them. Decode never throws. Added encode/decode round-trip tests so formatSyncDetail still shows hostnames after the prefs reload the UI actually uses.

The settings UI reloads via getLastSyncStatus(), which dropped the
new peers list so hostname rendering never appeared. Store peers as
JSON next to the aggregate counts; decode never throws.

Git-Session-Id: 7229f9f5-7b91-55aa-b26d-81c6a42708c7
@TimeToBuildBob

Copy link
Copy Markdown
Contributor Author

@greptileai review

@TimeToBuildBob

Copy link
Copy Markdown
Contributor Author

CI-green and mergeable (Greptile 5/5) — waiting only on a maintainer click.

This PR is ready to merge, but the bot has pull-only access to this repo and can't self-merge — surfacing it here so it isn't lost. The monitoring loop will stop re-flagging it now that this note is posted.

@TimeToBuildBob

TimeToBuildBob commented Sep 25, 2026 •

Copy link
Copy Markdown
Contributor Author

🤖 AI code review

Adds a SyncPeer data class and a peers: List<SyncPeer> field to SyncStatus, parses the peers JSON array in SyncStatus.fromJniResponse, persists it through SharedPreferences using new encodePeers/decodePeers helpers, and renders per-peer hostnames in formatSyncDetail with failed peers prefixed by '!'. Adds unit tests covering peer parsing, detail rendering with and without names, and encode/decode round trips.

Safe to merge — no P0/P1 findings

Confidence 5/5

✅ No findings. The diff looks correct to me on this pass.

Files changed (4) — the diff as I read it
  • mobile/src/main/java/net/activitywatch/android/AWPreferences.kt — Reads and writes the lastSyncPeers JSON string in getLastSyncStatus/setLastSyncStatus, removing the key when the peer list is empty.
  • mobile/src/main/java/net/activitywatch/android/SyncInterface.kt — Introduces SyncPeer, adds peers to SyncStatus, parses the peers array in fromJniResponse, and adds encodePeers/decodePeers for SharedPreferences storage.
  • mobile/src/main/java/net/activitywatch/android/SyncSettingsActivity.kt — Appends imported and failed peer hostnames in parentheses to the sync detail line, prefixing failed hostnames with '!'.
  • mobile/src/test/java/net/activitywatch/android/SyncSettingsActivityTest.kt — Adds tests for peers-array parsing, detail rendering with and without hostnames, and encode/decode round trips.
Previous review passes
commit score findings engine when
8752040d8e2a 5/5 0 llm 2026-09-25 10:24 UTC

Reviewed 8752040d8e2a · openrouter/deepseek/deepseek-v4-flash-0731 · llm engine · 625s · about this reviewer

Maintainer commands

@TimeToBuildBob review (own line) — fresh review · @TimeToBuildBob fix — a worker acts on the findings. Once per comment; 👀 = received.

@ErikBjare
ErikBjare merged commit ec0233e into ActivityWatch:master Oct 10, 2026
8 checks passed
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