refactor(qauld-tui): fetch in a background task, off the render loop - #929
Open
dastansam wants to merge 3 commits into
Open
refactor(qauld-tui): fetch in a background task, off the render loop#929dastansam wants to merge 3 commits into
dastansam wants to merge 3 commits into
Conversation
…t before priming Two fixes found by driving the real TUI through a pty harness against live daemons: - data.rs sent user_id: Vec::new() on every request envelope — despite its own comment explaining that libqaul rejects envelopes whose user_id fails to decode. libqaul's DTN RPC handler decodes user_id first and silently drops the request, so fetch_dtn_state and fetch_dtn_config burned their full timeout on every refresh cycle: with the default 5s CLI timeout and 3s auto-refresh the event loop was stalled ~10s out of every ~10s, keystrokes only being processed in brief windows between stalls (tab switching, filtering and the detail drawer all appeared dead). round_trip now takes the caller id; the DTN fetchers thread the default account id through and fail fast with a clear error when no account exists yet, mirroring the existing send_feed guard. - run() entered the alternate screen and then awaited the full priming refresh before the first draw, so startup showed a blank screen until every fetch settled (10s+ with the bug above; still fetch-bound without it). The frame skeleton is now painted once before priming. Tests: fetch_dtn_state/fetch_dtn_config refuse an empty user id before any socket I/O (red before the fix). Verified with a pyte pty harness against two live daemons: 22/22 feature checks pass — startup render, 5-tab cycle + wrap both directions, cursor, detail drawer open/close, filter apply/clear, feed compose+send, cross-node delivery, live event updates, r refresh, clean q quit, no panics.
The refresh cycle used to run inline in the render loop's tokio::select tick arm: eight RPCs awaited one after another, each on a fresh socket. While it ran, no keystrokes or redraws were processed — and if the daemon was slow or unreachable, the loop froze for up to (8 fetches x timeout) per cycle (~40s at the 5s default), making the whole UI feel dead. Move polling into a dedicated background task: - data::refresh_once fans the fetches out concurrently (tokio::join!) after resolving the default user first (the DTN fetches need its id), so a cycle costs one round-trip's latency instead of the sum of eight. It returns a Send-able Snapshot (Result<_, String> fields). - The task loops on an interval (first tick immediate → non-blocking prime) plus a manual-trigger channel, and posts each Snapshot to the render loop over an mpsc. - The render loop's select now has a snapshot arm that calls the pure, synchronous apply_snapshot — no .await on network anywhere in the loop. 'r' and post-feed-send just fire the trigger. Crypto-event dedup: the poll floor now lives with the task (sole owner of the poll path), so the daemon's inclusive since_ms filter re-sends the boundary event each cycle. append_crypto_events now routes every event through the same dedup as the push path (merge_crypto_event), so poll/push overlaps and boundary re-sends are dropped without the two paths sharing a floor. Tests: three append_crypto_events dedup cases (boundary re-send, push/poll overlap, distinct-same-timestamp). Verified live against two daemons — 22/22 feature checks still pass — plus a daemon-death probe: with every fetch hitting the 5s timeout, tab switches still register in ~50ms (identical to daemon-alive), where the old design would have frozen for up to 40s.
|
Member
|
I merged #935, which is this PR without the last commit, which contains the demo script. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacked on #928 — set to merge into
fix/qauld-tui-dtn-fetch; GitHub will retarget this tomainautomatically once #928 merges.Polling used to run inline in the render loop's select tick — 8 RPCs awaited sequentially, freezing input/redraws while they ran and up to ~40s/cycle if the daemon was unreachable. Move polling to a background task:
refresh_oncefans the fetches out concurrently (one round-trip's latency, not eight) and posts a Send-ableSnapshotto the loop, which applies it synchronously. No network.awaitanywhere in the loop.rand post-feed-send just fire a trigger.Crypto events now dedup through one path (the poll floor lives with the task), so poll/push overlaps and the daemon's inclusive
since_msre-sends are dropped. Three new dedup unit tests.