Repository navigation
fix(mobile): allow Markdown refresh during desktop tab snapshots - #30040
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Priority: ⬇️ Low Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage |  |
Docstring coverage is 11.11% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 9 functions across 4 files. (3 skipped: 3… | Write docstrings for the functions missing them to satisfy the coverage threshold. |
-
Check name Status Explanation Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request. Title check The title clearly and concisely describes the main change: allowing mobile Markdown refreshes during desktop tab snapshot updates. Description check The description is complete and relevant. It covers the user impact, implementation, rationale, linked issues, visual proof, testing, review notes, compatibility considerations, and checklist status.
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 11.11% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 9 functions across 4 files. (3 skipped: 3 unsupported.)
Resolution
Write docstrings for the functions missing them to satisfy the coverage threshold.
- Fix all pre-merge checks with AI
✨ Finishing Touches: 💡 1
-
📝 Generate docstrings: 💡
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands.
ELI5
Refreshing Markdown on a phone now updates the text even when desktop tab snapshots arrive during the request. The editor stays mounted, and phone edits and saves still win over older replies.
What Changed
The previous guard treated every replacement document object as a phone edit. Desktop snapshots also replace that object to mark it stale, so an otherwise successful refresh was silently discarded.
Track a local revision on phone edits and save start/completion instead. Refresh accepts metadata-only changes, preserves newer desktop-change warnings, and keeps the existing read ordering and closed-document guards. Add eight tests through the real reader and edit/save hooks and update three recorded save expectations for the local field.
Why
Object identity rejects harmless metadata updates. Comparing text alone misses an edit followed by undo or a save returning identical text and version. The local revision distinguishes those operations without changing the host protocol or adding another state store.
Linked Issue
Follow-up to #30033 for #30031.
Visual Proof
Same delayed refresh with repeated desktop stale notifications, using the actual mobile reader/editor in a hidden Electron component harness on macOS (RPC replies are simulated).
Before: the read finishes but the old heading remains.
After: the new heading appears in the same editor. A newer desktop-change warning remains until a refresh without an intervening snapshot.
Testing
Three focused passes: 46 tests each, covering metadata snapshots, edit/undo, save/read ordering, same-content saves, failed saves, overlapping reads, disk fallback, closed documents, and tab lifetime.
Full mobile suite (10,846 passed, 6 skipped), mobile typecheck, test typecheck ratchet, mobile lint, formatting, and the changed-code quality gate pass. Full PR CI (
.github/workflows/pr.yml, includingPR checks passed) and Mobile Checks passed for8871af76a6b14c19453d23fcb2186b0cf68016f9. The first browser job timed out in two existing HTML preview navigation tests; both cases and the full 30-test HTML preview file passed locally, and the unchanged commit passed the CI retry. No tests were weakened or skipped to obtain that result.Three rendered refresh passes through the real Refresh control confirmed updated headings and retained editor identity. Also checked active-tab retap makes no read, a failed refresh retains content, a delayed clean reply retains a newer warning, and a quiet refresh clears it. No native iOS/Android device run or live SSH session was performed for this follow-up; shared client logic and simulated remote/folder identifiers are covered.
Review
Three independent sub-agent reviews covered concurrency, integration, and test quality. The integration review found the newer-warning race; it is fixed and verified. No outstanding findings.
Agent skill upstream boundary
Notes
The revision remains client state. No host RPC, filesystem, platform, or provider behavior changes.
Checklist