Repository navigation
fix(mobile): skip the read-state snapshot when nothing needs publishing - #9093
Conversation
Every resume schedules a read-state publish after its re-sync, and each publish starts by downloading the full read-state snapshot (about 1 MB compressed on a busy account) before discovering it has nothing to send. Check first, with the same packing the publish uses, and return early when no slot differs from what was last published and nothing is pending. Co-authored-by: studio <dc2fc481343672df31a689840c09299972e704dac592e0da6f18486646a7ee3e@buzz.block.builderlab.xyz> Signed-off-by: Tom Brow <tomb@squareup.com>
|
@codex review |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 04f9ea8be9
ℹ️ 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".
…g to send Co-authored-by: studio <dc2fc481343672df31a689840c09299972e704dac592e0da6f18486646a7ee3e@buzz.block.builderlab.xyz> Signed-off-by: Tom Brow <tomb@squareup.com>
🔐 Codex Security Review
Review SummaryOverall Risk: NONE
FindingsNo concrete security, correctness, or reliability findings were identified. Notes
Generated by Codex Security Review | |
wesbillman
left a comment
There was a problem hiding this comment.
🤖 Carl, an automated reviewer, commenting via Wes’s GitHub account.
No blockers. One suggestion, inline. Reviewed 23a7d99f0e against main bc99726f5c.
Checked: _hasUnpublishedState matches the inputs _publishOnce packs (pending retries, carried overrides and their frontier keys, horizon filtering, previous: _placement, the published-slot comparison). It does not mutate state, so it cannot skip a publish that has local work. The _lastSnapshotSucceeded transitions hold across a publish that fails after its snapshot, because local work stays unpublished and the next publish runs in full.
Validation: CI is green. The combined #9089 + #9090 + #9093 read-state and relay suites pass locally (403 tests).
| // The snapshot below is the whole read state (megabytes on a busy | ||
| // account), so skip it when there is nothing to send. Each resume | ||
| // schedules a publish after its re-sync whether or not anything changed. | ||
| if (!_hasUnpublishedState()) { |
There was a problem hiding this comment.
🤖 Suggestion: skip only when the last snapshot succeeded.
On main, the publish that reinitializeRemote schedules is a second chance at the snapshot when the resume snapshot fails transiently. With this skip, a failed resume snapshot is not retried until the next resume or a local read. Combined with #9090 (no stored replay), remote marks stay unmerged for that whole session. #9090's accepted gap says "every resume, reconnect and publish retries", which stops being true for publishes once this lands.
if (_lastSnapshotSucceeded && !_hasUnpublishedState()) { syncError = null; return; } keeps the saving in the normal case and restores the retry after a failure. It also drops the conditional clear. The second test would then expect one more fetch.
johnmatthewtennant
left a comment
There was a problem hiding this comment.
🤖 Reviewed exact head 23a7d99. Complete source review and independent candidate verification found no correctness blockers. No independent app or device runtime validation.
…-media * origin/main: fix(mobile): offer a new identity beside saved keys on unclaimed accounts (#9107) fix(desktop): show upgrade guidance (BUZZ-517) (#9033) fix(mobile): skip the read-state snapshot when nothing needs publishing (#9093) fix(mobile): keep verified names on screen in channels with over 1,024 members (#9111) mobile: reuse channel message rows that did not change (#9115) fix(mobile): back off push bootstrap retries (#9092) fix(mobile): keep reply counts on screen while a reconnect catches up (#9100) fix(mobile): inflate gzip read-state snapshot responses (#9089) perf(relay): cache the community-active check on socket admission (#9109) fix(db): auto-migrate prebuild-guarded indexes on small databases (#8802) docs: mark desktop/ as the deprecated Buzz 0.X app (#8840) Signed-off-by: Brad Seiler <seiler@squareup.com>
…dary * origin/main: fix(mobile): focus the search field when search opens (#9120) perf(mobile): validate each verified profile once, not on every publish (#9101) fix(mobile): offer a new identity beside saved keys on unclaimed accounts (#9107) fix(desktop): show upgrade guidance (BUZZ-517) (#9033) fix(mobile): skip the read-state snapshot when nothing needs publishing (#9093) fix(mobile): keep verified names on screen in channels with over 1,024 members (#9111) mobile: reuse channel message rows that did not change (#9115) fix(mobile): back off push bootstrap retries (#9092) fix(mobile): keep reply counts on screen while a reconnect catches up (#9100) fix(mobile): inflate gzip read-state snapshot responses (#9089) perf(relay): cache the community-active check on socket admission (#9109) fix(db): auto-migrate prebuild-guarded indexes on small databases (#8802) Signed-off-by: Will Pfleger <wpfleger@block.xyz>
…(backport #9089, #9093, #9107 to 0.27) (#9175) ## Summary Backports three fixes from `main` to `mobile-v0.27` for the Nov 10 App Store release (0.27.1): - [#9089 fix(mobile): inflate gzip read-state snapshot responses](#9089). The Builderlab relay gzips responses, so on 0.27 every read-state snapshot fetch fails and read marks made on the phone never reach other devices. - [#9093 fix(mobile): skip the read-state snapshot when nothing needs publishing](#9093). Once the snapshot works, this avoids an extra full snapshot download on each idle resume. - [#9107 fix(mobile): offer a new identity beside saved keys on unclaimed accounts](#9107). This fixes a Builderlab sign-in dead end: a new account on a device that already holds saved keys could only reuse one of those keys, and a rejected bind left it stuck on "Let's try that again". ## Provenance `git cherry-pick -x --signoff` of the squash merges `5e36d65c9e`, `dce37f74d7` and `c848b6dcad`, in that order, onto `mobile-v0.27` at `54d57a5430`. All three applied cleanly, and their patch IDs match the originals. Nothing else from `main` is included. ## Validation Independent stable-fitness reviews by two agents. Both said to pick all three, with no blockers and no missing `main`-only prerequisites. Results on this branch's tree (`2e9b94cd9bf9363eb8fadd772764b0ecc4b8c091`): - Mobile analysis and format: clean. - Full mobile suite with CI's auth-domain and push-gateway defines: 4,427 passed, 7 skipped. The new hosted identity journeys ran, covering rejection and restart, failed release, account switching and key preservation. - iOS simulator debug build with auth domains enabled: passed (Xcode 27.1). Limitation: no live Builderlab sign-in or device read-mark sync was run on this stable build. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Signed-off-by: Tom Brow <tomb@squareup.com> Signed-off-by: John Tennant <jtennant@squareup.com> Co-authored-by: Tom Brow <tomb@squareup.com> Co-authored-by: studio <dc2fc481343672df31a689840c09299972e704dac592e0da6f18486646a7ee3e@buzz.block.builderlab.xyz> Co-authored-by: John Matthew Tennant <jtennant@squareup.com> Co-authored-by: Xander <ff30c859b96bd5233ffaf5d071200458f1274bd0dab03589214cf1055198491d@buzz.block.builderlab.xyz> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Every mobile resume re-syncs read state and then schedules a read-state publish, whether or not anything changed. Each publish starts by downloading the full read-state snapshot (about 1 MB compressed on a heavy account) and only then finds nothing to send. Together with the re-sync's own snapshot, that is two full downloads per resume, and a third when a publish from before backgrounding retries.
The publish now checks first, using the same packing it would publish, and returns before the snapshot when nothing is pending and no slot differs from what was last published. When something did change it behaves exactly as before, snapshot included.
Today the snapshot fails before downloading on gzip-serving relays, so this only pays off once #9089 read-state gzip fix lands; without this PR, #9089 read-state gzip fix roughly doubles resume download on heavy accounts. It has no merge-order dependency.
Validation
a publish with nothing new skips the snapshot. It publishes a change (one snapshot), publishes again with nothing new (no snapshot, no event), then publishes another change (snapshot and an event carrying both marks). It fails without the early return.These are single runs on one account with 28 read-state slots, so treat the numbers as indicative.
🤖 Generated with Claude Code