Skip to content

fix: speed up pubky profile loading - #1399

Open
Jasonvdb wants to merge 15 commits into
masterfrom
claude/flow-vibe-pubky-profile-load-lag
Open

Jasonvdb wants to merge 15 commits into
masterfrom
claude/flow-vibe-pubky-profile-load-lag

Conversation

@Jasonvdb

@Jasonvdb Jasonvdb commented Sep 30, 2026 •

Copy link
Copy Markdown
Contributor

Twin: synonymdev/bitkit-ios#854

Fixes the lag when loading pubky profiles, which got much worse with the shared Pubky Ring identities from #1329.

Where the lag came from. All Paykit calls go through one lock, and each call kept it for its whole network round trip. That included simple public reads, like fetching a profile name or avatar. So profile lookups queued behind each other, behind sign-in, and behind background Paykit work. The Pubky Ring choice screen made it worse: it looked up every Ring identity one at a time, retried each failure, and hid the whole list behind a spinner until all of them were done. A pubky that was never published takes 2–7 s to fail, and everything else waited for it.

How this fixes it. Public reads skip the lock. They run side by side (up to six at once) and stop when you leave the screen. Anything that touches your session or keys still goes through the lock. The choice screen now shows its rows straight away, with a small spinner on each row until its profile arrives. Adopting a Ring identity reuses the profile its row already found, sign-in no longer waits for the background identity refresh, and avatars are cached on disk.

Emulator, staging network, one take per build Before After
Ring rows visible 10.8 s 0.2 s
Published name visible 10.8 s 2.5 s
Tap a Ring row → Pay Contacts 11.0 s 5.4 s

Description

  • Public profile, avatar and follows reads no longer wait for the Paykit lock; up to six run at once, and they stop when their screen closes
  • Sign-in starts the identity refresh in the background instead of waiting up to 5 s for it; auth approvals still wait for it, including a refresh that is already running
  • The Pubky Ring choice screen shows its rows at once, with a spinner on each row while its profile loads and on the row being adopted
  • Adopting a Ring identity reuses the profile its row already loaded, and the contact import reuses the loaded profile
  • Profile lookups retry only where it helps (adding or importing a contact, loading your own profile), and never after "not found"
  • The Profile screen shows your cached name and avatar while it loads, and loads once more on its own if a load that was already running fails
  • Pubky avatars are cached on disk and cleared on sign-out or when the identity changes
  • A profile load that finishes after a newer save, adopt or sign-out is ignored, so it cannot overwrite newer data

Out of Scope

  • app/src/main/java/to/bitkit/repositories/PubkyRepo.kt: app launch still builds the Paykit SDK and restores the session twice; removing that needs Paykit input
  • Adopting a never-published Ring pubky fails because Paykit's identity check throws instead of returning false; this needs a Paykit fix
  • rustls_platform_verifier "BKS KeyStore not available" TLS errors on the emulator image; they were there before this PR and can make a lookup fail
  • Contacts still look up every profile on each load

Design

N/A — no design available. The spinners reuse existing components.

Preview

android-before-after.mp4

Left: master (dbbf90f). Right: this PR (1f4364f; later commits do not change this flow). Same emulator, same taps, fresh app data, staging network, one take each. After the profile intro, master shows only "Loading your profile…" for about 11 s. With this PR the rows show at once, each row's spinner turns into its name or avatar, and adopting spins only the tapped row.

QA Notes

Journeys

  • new cached-profile-header.xml — Profile opened while loading shows the cached name and avatar without edit actions, then the full profile
  • new ring-choice-rows.xml — Pubky Ring rows show at once with a spinner each until their profile loads; adopting spins only the tapped row and disables the rest

Manual Tests

  • With Pubky Ring holding a published pubky and a never-published one, open the profile choice screen → both rows show at once with spinners, and the published name appears without waiting for the other row — Pubky Ring not in Capabilities
  • Tap the published Ring row → only that row spins, the other row is disabled, and Pay Contacts opens — Pubky Ring not in Capabilities
  • Force an adopt failure (airplane mode right after the tap) → the rows stay visible with their names and can be tapped again — Pubky Ring not in Capabilities

Automated Checks

  • added PubkyStoreTest.kt — the cached profile owner
  • updated PaykitSdkServiceTest.kt — public reads and locked work do not wait for each other, reads are capped at six, a read with no SDK builds it under the lock, activation does not wait for the refresh, and an approval waits for a refresh activation started
  • updated PubkyServiceTest.kt — cancelling a public read cancels the underlying call
  • updated PubkyIdentityRepublishTest.kt — approvals still wait for the identity refresh
  • updated PubkyRepoTest.kt — stale profile loads are dropped after a write, the retry policy per caller, the adopt handoff and the contact import reuse
  • updated PubkyChoiceViewModelTest.kt — rows show before lookups finish, each row's spinner clears whether its lookup finds a profile, finds nothing or fails, and lookups continue after a failed adoption
  • updated ProfileViewModelTest.kt — cached header only while loading and only for the current key, refresh on open, and one more load when a running load fails
  • updated PubkyImageFetcherTest.kt — disk cache hits, no writes after a clear or for a failed download, and disk errors falling back to the network

@greptile-apps

greptile-apps Bot commented Sep 30, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerConfidence Score: 4/5

[Medium risk] Adds caching and concurrency controls to profile loading.

The PR should not merge until an overlapping profile refresh can no longer restore stale cached metadata after a save or sign-out.

Findings

  1. P1 Stale refresh overwrites cached metadata ▶
  2. P2 Cleared avatars can return ▶

Summary

The PR moves public Pubky reads out of the Paykit operation lock, loads Ring identity rows independently, reuses resolved profiles during adoption, and adds cached profile and avatar presentation.

  • Profile refreshes need to keep their persisted metadata from overtaking newer writes or sign-out.
  • Avatar cache clearing has a narrow commit race with in-flight fetches.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart LR
  A[Profile or Ring screen] --> B[PubkyRepo]
  B --> C[PubkyService]
  C --> D[PaykitSdkService public-read permits]
  D --> E[Public Pubky data]
  B --> F[Profile state and metadata cache]
  E --> G[PubkyImageFetcher]
  G --> H[Avatar disk cache]
Loading

Reviews (1) · Last reviewed commit: "fix: refresh the profile on open and tol..."

Comment on lines 517 to 525
_profile.update {
isCurrent = _publicKey.value == pk && profileWriteGeneration.get() == writeGeneration
if (isCurrent) loadedProfile else it
}
if (!isCurrent) {
Logger.debug("Skipped stale profile load for '${redacted(pk)}'", context = TAG)
return@onSuccess
}
_profile.update { loadedProfile }
cacheMetadata(loadedProfile)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P1 Stale refresh overwrites cached metadata

If a profile refresh overlaps a save or sign-out, it can pass this generation check and then suspend before writing the cached metadata. The newer save or sign-out can finish first, after which the refresh writes the old name, avatar, and owner. The in-memory profile guard does not protect this later write, so a subsequent load can show obsolete details or metadata that sign-out had cleared.

Knowledge Base Used: Pubky identity and profile

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.

Confirmed and fixed in 979e1fc. loadProfile now passes its current-load check (same key, unchanged write generation) into cacheMetadata, which re-checks it inside the PubkyStore update transform. DataStore runs updates one at a time, so the check and the write happen together relative to saves and sign-out. Sign-out also bumps the generation before it resets the store, so a load write that lands right after the reset is dropped. Covered by three new PubkyRepoTest cases: a held load write after a save, after sign-out, and right after sign-out's store reset.

?: return
runCatching {
cache.fileSystem.write(editor.data) { write(bytes) }
if (cacheEpoch.current() == epoch) editor.commit() else editor.abort()

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2 Cleared avatars can return

The epoch check and commit() are separate operations. If sign-out or an identity switch clears the disk cache after the check passes but before the commit finishes, this fetch can put an avatar back into the cleared cache. That leaves an entry the clear was meant to remove and weakens cache cleanup.

Knowledge Base Used: Pubky identity and profile

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.

Checked against Coil 3.2.0 and this race can't re-add an entry. The editor is opened before the epoch check, and RealDiskCache.clear() calls DiskLruCache.evictAll(), which marks any entry with an open editor as a zombie; completeEdit() on a zombie entry deletes the dirty files instead of committing them. So a clear that lands between the check and commit() discards this write. I added a comment at the check in 979e1fc so that reliance is explicit; no behaviour change.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

You’re right. Since openEditor() happens before the epoch check, a concurrent RealDiskCache.clear() can evict/zombify that editor; completing it afterward cannot publish the dirty entry, so the write is discarded rather than re-added. The comment in 979e1fc30 makes that Coil 3.2.0 invariant explicit. I’m withdrawing the finding.

@github-actions

github-actions Bot commented Sep 30, 2026 •

Copy link
Copy Markdown
Contributor

Regtest APK

Built from 330579d (run).

Download bitkit-dev-debug universal APK (expires in 30 days).

A Ring row now shows a spinner in its avatar slot until its profile
lookup finishes, whatever the outcome, so a row still looking up no
longer looks like one whose identity has no profile. The adoption
spinner moves into the row's key icon circle, matching iOS.

This branch has not been deployed

No deployments
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.

1 participant