fix(channel): clear reloading flag event-driven instead of fixed 200ms#1050
Open
data-diego wants to merge 2 commits intoalexpasmantier:mainfrom
Open
fix(channel): clear reloading flag event-driven instead of fixed 200ms#1050data-diego wants to merge 2 commits intoalexpasmantier:mainfrom
data-diego wants to merge 2 commits intoalexpasmantier:mainfrom
Conversation
`tv <channel> --watch 0.3` used to flicker on every refresh because `Channel::reload()` called `matcher.restart()` up-front (dropping the old snapshot) and `Television::should_render` was AND-gated on a `reloading` flag cleared by a fixed 200ms sleep. A source slower than 200ms (e.g. `fd . $HOME --type f`) would re-set the flag on the next watch tick before the previous load finished, so the UI never unfroze. Replaces the restart/flag/sleep scheme with an fzf `reload-sync`-style atomic swap: `reload()` builds a fresh staging `Matcher` and feeds the new source into it while the live matcher keeps serving the previous snapshot. `tick()` swaps staging into the live slot in one statement as soon as staging has items, or once the staging crawl task exits with no output. - `find()` is mirrored onto staging so the post-swap snapshot is already filtered to the user's pattern. - `SortStrategy::Custom` holds a non-`Clone` `Box<dyn SortFn>`, so a `SortStrategyFactory` closure builds a fresh strategy per staging matcher; frecency caches are `Arc`-cloned in. - `running()` ignores staging — during a reload the user sees a stable list, so the UI loading indicator stays off. Initial load still reports running, and after a swap the staging handle moves into `crawl_handle` so `running()` stays accurate while the new source drains. - `Matcher::tick()` now refreshes `total_item_count` / `matched_item_count` from the snapshot so callers can observe progress without calling `results()` first (needed by the swap decision). - The `reloading` flag, `should_render` gate, `reload_deadline` field, and `RELOAD_RENDERING_DELAY` constant are removed. Behavior change: a source that hangs with no output no longer times out to an empty list after 200ms — the previous snapshot stays on screen until the source emits or exits. `--watch` retries on its next tick.
Five tests against a real `Channel<PlainProcessor>`: - `test_reload_swaps_when_items_arrive`: fast source triggers swap on first batch. - `test_reload_swaps_when_empty_source_finishes`: `true` (no output) triggers swap when the crawl task exits. - `test_reload_keeps_old_results_visible_until_swap`: against a slow source, `result_count()` stays at the pre-reload value for the full reload window — the core no-flicker invariant. - `test_reload_does_not_swap_to_empty_while_source_hangs`: against `sleep 5`, staging stays in flight; live matcher keeps old entries. - `test_running_ignores_staging_but_not_initial_load`: `running()` is true during the initial load and false during a staging-only reload.
35ef648 to
f2cb57d
Compare
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
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.
Important
This PR is fully vibe coded, I just preferred providing a solution instead of just ranting in an issue, feel free to close it and use it only as inspiration if its not up to the repo's standards.
Motivation
I'm porting a shell-script session picker to a television channel. It refreshes a tmux/sesh list every ~250ms and each row has a small spinner indicator for whether the Claude session attached to that window is waiting for input.
Under fzf (with
reload-sync) the refresh is invisible: old rows stay on screen until new rows are ready, then they swap in one frame. Undertv <channel> --watch 0.3the list flickers on every refresh — noticeable on its own, very visible on the spinner — because the blank frame lands right where the glyph would animate. Same source, same cadence, so this is a--watchrendering issue in tv.Root cause
Two things compound in
Channel::reload():matcher.restart()drops the previous snapshot up-front, leaving the matcher empty for the whole source runtime.Television::should_renderis AND-gated on!channel.reloading(), a flag cleared by a fixed200mssleep. The UI is frozen for 200ms regardless of source speed — and if the source is slower than 200ms (e.g.fd . $HOME --type f, ~270ms), the flag gets re-set on the next watch tick before the previous load finishes, so the UI never unfreezes.Fix: fzf-style atomic swap
reload()now builds a fresh staging matcher and feeds the new source into it. The live matcher keeps serving the previous snapshot —results(),find(), rendering, scrolling all stay responsive — untiltick()swaps staging in as a single statement. The swap is purely event-driven: it fires when staging has items, or when the staging crawl task exits with no output.Supporting pieces:
find()is mirrored onto staging so the post-swap snapshot is already filtered to whatever the user typed during the reload window.SortStrategy::Customholds aBox<dyn SortFn>that isn'tClone, so we carry aSortStrategyFactoryclosure that builds a fresh strategy per staging matcher. Frecency caches areArc-cloned in.running()ignores staging. During a reload the user sees a stable list, so the UI's loading indicator should stay off. The initial load still reports running (no live results yet), and after a swap the former staging handle moves intocrawl_handlesorunning()stays accurate while the new source drains.reloadingflag, theshould_rendergate, thereload_deadlinefield, and theRELOAD_RENDERING_DELAYconstant all go away — the atomic swap replaces them.Behavior change
A source that hangs without producing output no longer times out to an empty list after 200ms — it keeps showing stale data. This is strictly better than flickering to empty: the stale data is at least coherent, and
--watchretries on its next tick anyway.Repro
Before this PR, this flickers every 300ms on most terminals:
After this PR the list is stable; only the clock value (last line) changes in place. A source slower than 200ms (e.g.
fd . $HOME --type f) also no longer flickers to empty.Tests
test_reload_swaps_when_items_arrive— swap fires as soon as the new source produces output.test_reload_swaps_when_empty_source_finishes— with a source that emits nothing (true), swap fires once the crawl task exits.test_reload_keeps_old_results_visible_until_swap— against a slow source (~80ms),result_count()stays at the pre-reload value for the full reload window, then flips to the new value after the swap.test_reload_does_not_swap_to_empty_while_source_hangs— againstsleep 5, staging stays in flight well past any prior time cap; old entries remain visible.test_running_ignores_staging_but_not_initial_load—running()is true during the initial load and false during a staging-only reload.cargo test --all --all-features,cargo clippy --all-targets --all-features -- -D warnings, andcargo fmt --checkall pass locally on macOS.