Repository navigation
Catch up desktop chats from the newest page - #29125
Conversation
…perf-catchup-current-requests
…perf-catchup-current-requests
…2025/nc-perf-catchup-current-requests
…2025/nc-perf-catchup-current-requests
Current status: review complete, real-agent QA passedFinal head A pending child approval or question could disappear when its history row left the loaded page. The reviewed stack separates current decisions from paged history through the execution host’s existing owner and bounded independent reads. Returning after a long hidden reply now loads the newest page while keeping pending decisions available; short gaps and older desktops still replay exactly. Three source rounds completed. The architecture review required the independent-request prerequisite; round 2 replaced an unpaced stream transfer with sequential cursor reads, and round 3 found no new proven defect. The final decider ruled KEEP. Catalog, type and wire-test corrections are included in the exact head Actual hidden Electron comparison on isolated Linux runners reproduced missing controls on the base and verified both reviewed heads: off-page approval/grouped question, rendered answers and removal, backward wheel paging, hidden-send settlement, owner completion plus withdrawals, restart and ordinary short replay. Whole-window screenshots are attached in the PR body. A strict retention check after paging retired nine oldest rows on both base and prerequisite; the fresh prerequisite replay retained every loaded text message. Runtime matrix, fresh replay. Final-stack measurements on the same original Linux host passed exact content/cursor/request payload and backward reconstruction checks. For 20,000 missed revisions, helper p50/p95 fell from 262.936/296.338 to 1.154/1.561 ms; distinct messages fell from 350.373/422.345 to 9.986/11.516 ms. These exclude network and painting; retained host scans and total request memory still scale with useful data. Paired bridge run passed on the exact successor; nine relevant request/reader/transport source blobs are identical on the prerequisite. Production bridge modules and an actual encrypted connection delivered 5,774,314 bytes in 235 sequential reads, including a 1,048,911-byte request body. The largest bridge frame was 51,270 bytes, with one read in flight. The assembled JSON matched the host exactly, including all four generic approval options and explicit required-text metadata. Twelve subscription events (67,078 bytes) remained unacknowledged during the transfer; this did not fill the event window. Capability simulation correctly rejected an unsupported read, omitted the marker, replayed eight rows exactly, recovered after terminating the owned socket and stopped delivery after disposal. This exercises a scripted page endpoint, not a native phone WebView. The earlier run passed the main transfer and failed only because the compatibility probe supplied an invalid UUID; that harness input was corrected. Deferred existing limitations: journal rows are decoded before byte admission; Cancel may do nothing for a child request after its parent ends; a blank required final question can display Skip but send no answer. Nonblank answers work. Intended differences from the common pattern are the existing execution-host owner, complete question payloads and bounded reads for independently updated remote consumers. Pre-decode byte admission remains a named temporary follow-up. Native phone UI, a real older binary and SSH transport were not exercised; capability simulation must not be read as those tests. #28951 was closed separately by Real signed-in agent QA — integrated headPASS on Backward scrolling rendered older history. After the child and its notification turn finished, restarting preserved all 529 saved items with exactly the same hash, This is a sequential saved-chat check, reusing the disposable profile from prerequisite QA. Codex was blocked by authentication isolation; grouped child questions were not available in the real provider's child tool contract. Native phone UI, an older binary, SSH transport, synthetic large-payload measurements and the full withdrawal matrix were not rerun here; the earlier source/CI evidence remains separate. Launch gates passed (initial load 7.36, memory 82% free; restart load 6.56, memory 82%; zero typechecks). Real Claude settings and Codex hook hashes remained unchanged. Runtime processes were stopped by recorded identity; final shared cleanup is recorded in the worker report. Final review completion and readiness remain the review owner's responsibility.
Real signed-in agent QA (M4Air), head b56a336Passed with M4Air's real signed-in Claude, Opus (picker description Opus 5.5), High, Ask for approval, and auto-naming off through the UI. Grok drove the hidden isolated app over CDP; the folder workspace's group had its real
Hooks stayed on. SHA-256 before and after:
Pending approval:
Original prompt after scrollback:
Settled child and hidden-send reply after restart:
Real signed-in agent QA, head 285256dExercised on M4Air with its real signed-in Claude, Opus (Opus 5.5), High effort, Ask for approval and automatic chat naming off, selected through the UI. Grok drove the hidden isolated test copy through Chrome DevTools Protocol; hooks stayed on. The folder group had a non-null parent path and a real folder workspace.
Real hook configuration SHA-256 before and after:
Pending child approval after reopening:
Original prompt after scrollback:
Settled child and hidden-send reply after restart:
|
…' into brennanb2025/nc-perf-catchup-current-requests
…' into brennanb2025/nc-perf-catchup-current-requests
…' into brennanb2025/nc-perf-catchup-current-requests
…' into brennanb2025/nc-perf-catchup-current-requests
…d keeps failing A deterministic read failure reopened the subscription, whose first frame re-read the same version and reset the backoff, so the pane resubscribed every 750 ms forever.
…nt-requests # Conflicts: # src/shared/electron-remote-runtime-client-capabilities.ts
…' into brennanb2025/nc-perf-catchup-current-requests
…' into brennanb2025/nc-perf-catchup-current-requests
…' into brennanb2025/nc-perf-catchup-current-requests
…catchup-current-requests # Conflicts: # src/main/native-chat/agent-session-wire/structured-agent-session-subscribers.ts # src/main/runtime/rpc/methods/structured-agent-session.ts # src/shared/electron-remote-runtime-client-capabilities.ts
|
Important Review skippedReview was skipped as selected files did not have any reviewable changes. ⚙️ Run configuration
You can disable this status message by setting the Use the checkbox below for a quick retry:
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 12.50% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 8 functions across 15 files. | Write docstrings for the functions missing them to satisfy the coverage threshold. |
-
Check name Status Explanation Title check The title clearly summarizes the main change: supported desktop chats catch up from the newest history page. Description check The description is complete and well aligned with the template. It explains the user impact, implementation, rationale, linked work, visual proof, testing, compatibility considerations, limitations, a… 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. Full details: Description check
-
Explanation
The description is complete and well aligned with the template. It explains the user impact, implementation, rationale, linked work, visual proof, testing, compatibility considerations, limitations, and checklist status. The Linked Issue section references
#28951and related prerequisite work, although it does not use a formal Fixes reference.
-
✨ Finishing Touches: 💡 2
-
📝 Generate docstrings: 💡
-
- Commit to this branch
- Create a new PR
🛠️ Fix failing CI checks: 💡
-
- 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.
…catchup-current-requests
ELI5
Returning to a chat after a long reply can pause while the desktop replays every saved revision it missed. The desktop only needs the newest conversation page to catch up; older text can load when the person scrolls.
What Changed
A supported desktop replaces a gap larger than one page with the newest bounded page. Short gaps still replay exactly. Pending approvals and questions remain available through the independent current-request contract in #29121, and accepted sends settle through #28993. A desktop must support both contracts before receiving history-replacing catch-up. Older desktops retain exact replay; existing phone behavior stays compatible.
Why
This uses the existing paged history and host subscription rather than replaying work proportional to every missed revision. Current decisions stay independent of whichever history page is loaded. It avoids pinning old rows or treating approvals as a special catch-up case.
This is the reviewed replacement for #28951, preserving that PR's reviewed history without a force-push. Stack: #28993, then #29121, then this PR. Requested by @BrennanKB5.
Linked Issue
Native-chat performance review requested by @BrennanKB5; original change tracked in #28951.
Visual Proof
Actual hidden Electron on isolated GitHub Linux runners, built from the reviewed source commits. A deterministic command-line provider exercised the real host, read owner and rendered controls in a folder workspace; no renderer request collection was injected. Runtime run.
Before (#28993): a child approval has fallen outside 607 newer messages and its control is absent.
After (this PR): the complete approval stays available outside the loaded history page.
The grouped question also stays answerable through the existing card.
Rendered answers and removal, backward wheel paging, hidden-send settlement, owner-end removal and quit/relaunch ran. Ordinary short replay also passed on both heads; a separate fresh-window prerequisite run retained all loaded messages (run). A strict retention check after backward paging trimmed nine old rows on both the prerequisite and its base, an existing retention boundary. The separate paired bridge transport run below now certifies huge payloads and explicit required-text metadata; screenshots remain evidence of rendered controls.
Testing
Head 4c2a7d7 adds the catch-up policy and capability compatibility case to prerequisite 422766b. Round 3 correctness and dedicated transport/performance source review found no new proven issue. Final source readiness and the end-round architectural decider completed. The full PR workflow and its
PR checks passedaggregate succeeded on this exact head, including all five unit shards, static analysis/typecheck, wire compatibility, mobile verification, packages and relay integration. The host current-request flow and bounded catch-up contracts passed. CI run. The final-stack Linux benchmark passed on this exact head and the direct prerequisite. Rendered backward scrolling, off-page approval/question responses, send settlement, owner end and restart passed in the isolated run. Ordinary short replay passed on both heads, including a fresh-window prerequisite control. The paired bridge transport checks below also passed. No checks, installs, builds or apps run on the development Mac.Review
Independent architecture review required complete current requests before enabling desktop snapshots. That contract is a separate prerequisite. Fresh round 3 correctness and transport/performance review of both source heads found no new proven issue; final source readiness completed with no new proven high-severity regression; the remaining transport evidence is now complete with the limits below.
Agent skill upstream boundary
Notes
Intended differences from the common pattern: keep the existing 200-row and projected-byte page policy, use the execution host's saved conversation facts, and negotiate independently updated consumers. Temporary difference, tracked follow-up: pre-decode journal replay byte admission; the initial raw row read is not byte-preflighted. Complete current-request memory still scales with actual pending work.
Historical numbers on #28951 remain separate from the measurements below. The original history-only control and complete current-request helper measurements were run separately; neither measures browser painting or network latency.
Checklist
Pre-existing desktop and phone Cancel limitations for a child prompt after its parent turn ends are deferred; this change preserves the answer path and does not certify prompt-only Cancel.
Final-stack measurements
Same Linux host (Intel i3-12100F), Node 26.1.0; direct prerequisite
422766b923cversus this head4c2a7d717b8. Each case has 300 older messages, 5 warmups and 31 measured samples. These times include history subscription, JSON decoding/reduction and complete independent pending-request adoption through production helpers. The child approval and required grouped question start outside the loaded page.Both heads preserved exact final content/cursor, complete request payloads and full backward reconstruction. Unrelated output caused no request refetches. Short 200-row gaps retained exact replay and identical frame/byte counts. Retained conversation items are still scanned for current-work publication; this is no claim of constant total cost or constant memory.
The same isolated Node-only launcher and pinned minimal dependencies were used on both heads. Production source stayed at the exact commits; a package-manager bootstrap-only lockfile edit during setup was preserved as evidence and restored. This setup differs from the historical launcher, and shared-host load was not isolated. The helper excludes outer host/RPC dispatch, capability negotiation, real bridge/network behavior and React painting; rendered Electron evidence is attached; completed transport checks are reported separately below.
Existing question-card behavior observed during QA: a blank required last question displays Skip, but clicking it leaves the card unanswered; entering text and choosing Submit works. The card code is unchanged from the base and this is deferred with the existing child-only Cancel limitations.
Paired transport validation
Paired bridge run passed on the exact successor; nine relevant request/reader/transport source blobs are identical on the prerequisite. Production bridge modules and an actual encrypted connection delivered 5,774,314 bytes in 235 sequential reads, including a 1,048,911-byte request body. The largest bridge frame was 51,270 bytes, with one read in flight. The assembled JSON matched the host exactly, including all four generic approval options and explicit required-text metadata. Twelve subscription events (67,078 bytes) remained unacknowledged during the transfer; this did not fill the event window. Capability simulation correctly rejected an unsupported read, omitted the marker, replayed eight rows exactly, recovered after terminating the owned socket and stopped delivery after disposal. This exercises a scripted page endpoint, not a native phone WebView. The earlier run passed the main transfer and failed only because the compatibility probe supplied an invalid UUID; that harness input was corrected.
Native phone UI, an actual older binary, SSH transport and a late RPC reply after disposal were not exercised by this run. Owner replacement, unsupported openings and late-completion rejection passed the automated exact-head contracts.