Skip to content

fix: tab spinner lingers ~3s after fast commands like ls - #196

Merged
thdxg merged 1 commit into
mainfrom
claude/tab-status-tracking-baf9f8
Aug 1, 2026
Merged

fix: tab spinner lingers ~3s after fast commands like ls#196
thdxg merged 1 commit into
mainfrom
claude/tab-status-tracking-baf9f8

Conversation

@thdxg

@thdxg thdxg commented Aug 1, 2026

Copy link
Copy Markdown
Owner

What

Stops the tab activity spinner from appearing late and hanging ~3 seconds after a fast command (ls) has already finished. Follow-up to #185.

Why

The status indicator still felt unresponsive after #185: running a trivial ls could show a spinner for several seconds, sometimes only starting after the command was done.

Root cause is a collision between two designs:

  • The fork's OUTPUT_ACTIVITY heartbeat throttle (patch 0002) is leading-edge with drops: the first pty chunk after ≥500ms of quiet emits immediately, and every chunk in the following 500ms is dropped entirely — no trailing flush. Typing echo opens the window, so a quickly-typed ls can have all of its output chunks dropped.
  • The tracker measured row growth across completions. ls finishes in milliseconds, so OSC 133;D lands while the pane still reads .idle (a no-op), leaving the row baseline stale. The next emitted heartbeat — the prompt redraw, or the next keystroke's echo seconds later — carries the accumulated growth, which the tracker read as a fresh activity start. Nothing can end that run (the D marker already fired; the foreground poll never sees a 10ms process), so it sits out the full 3s quiet-settle.

How

Completion edges now rebase the row baseline (lastOutputRows = nil) in TerminalExecutionTracker:

  • markCommandFinished — including when it arrives while idle, the exact fast-command race, and
  • the changed branch of refreshForeground — the poll's return-to-shell edge, covering the same staleness for shells without OSC 133 integration.

Growth measured across a completion re-baselines instead of restarting the finished command; genuinely new output still starts a run on the next growing heartbeat. quietInterval stays 3s — the bug was the stale start, not the settle length. Also records the throttle-semantics invariant in the CLAUDE.md activity paragraph.

Verified

  • mise run format, mise run lint, and mise run test all pass
  • Added or updated tests for new model / persistence / palette / hotkey logic

Both new tracker tests replay the two races and fail without the fix (state → .running where .idle is required), pass with it.

Notes for reviewers

A fork-side follow-up could make sub-500ms commands crisper still: a trailing-edge flush in shouldEmitOutputActivity would let their output flash the spinner correctly instead of showing nothing. This PR only removes the false 3s spinner.

…don't linger as spinners

The output-activity heartbeat throttle (500ms, leading-edge) drops
chunks rather than deferring them, so a fast command's output often
surfaces only on a heartbeat emitted after OSC 133;D already fired
while the pane read idle. The tracker then attributed that stale row
growth to a fresh activity run, showing a ~3s spinner for a command
that took milliseconds.

Completion edges (markCommandFinished, even from idle, and any
foreground transition in refreshForeground) now reset lastOutputRows,
so growth measured across a completion re-baselines instead of
restarting a finished command.
@github-actions github-actions Bot added area:state AppState, models, persistence area:tests Test changes area:docs Documentation labels Aug 1, 2026
@thdxg
thdxg enabled auto-merge (squash) August 1, 2026 12:09
@thdxg
thdxg merged commit 9f5dca5 into main Aug 1, 2026
10 checks passed
@thdxg
thdxg deleted the claude/tab-status-tracking-baf9f8 branch August 1, 2026 12:10
@github-actions github-actions Bot added the benchmark:regression CI benchmark: significant resource regression vs main label Aug 1, 2026
@github-actions

github-actions Bot commented Aug 1, 2026

Copy link
Copy Markdown
Contributor

Window-state benchmark

State Metric main@9c0b4ccdd this branch Δ
focused CPU % 0.70 1.00 +43%
Memory (RSS MB) 109.3 109.2 -0%
CPU ms/s (powermetrics) 6.5 9.4 +45%
Wakeups/s (powermetrics) 121.4 177.7 +46% 🔺
workload-focused CPU % 2.10 2.80 +33% 🔺
Memory (RSS MB) 167.0 163.7 -2%
CPU ms/s (powermetrics) 19.7 26.9 +36% 🔺
Wakeups/s (powermetrics) 204.2 268.9 +32% 🔺
workload-unfocused CPU % 2.00 2.10 +5%
Memory (RSS MB) 129.5 166.9 +29% 🔺
CPU ms/s (powermetrics) 20.0 20.8 +4%
Wakeups/s (powermetrics) 226.7 288.5 +27% 🔺

⚠️ Labeled benchmark:regression

This PR is labeled benchmark:regression because ≥2 metrics regressed by ≥25% vs main@9c0b4ccdd (beyond each metric's noise floor), at least one under workload:

  • focused — Wakeups/s (powermetrics): 121.4 → 177.7 (+46%)
  • workload-focused — CPU %: 2.10 → 2.80 (+33%)
  • workload-focused — CPU ms/s (powermetrics): 19.7 → 26.9 (+36%)
  • workload-focused — Wakeups/s (powermetrics): 204.2 → 268.9 (+32%)
  • workload-unfocused — Memory (RSS MB): 129.5 → 166.9 (+29%)
  • workload-unfocused — Wakeups/s (powermetrics): 226.7 → 288.5 (+27%)

Reported value is the median of 3×10s windows per state (splitting the window and taking the median keeps one co-scheduled spike from skewing a state); CPU % is the process CPU-time delta over a window. Runs land on different shared runners, so treat small deltas as noise — 🔺/🔻 marks changes ≥25% that also clear the metric's absolute noise floor (CPU % ≥0.5, Memory (RSS MB) ≥25, CPU ms/s ≥5, Wakeups/s ≥50); CPU deltas off a noise-dominated baseline aren't flagged (CPU % baseline ≥1.5, CPU ms/s baseline ≥15). The benchmark:regression / benchmark:improvement label needs corroboration — ≥2 flagged metrics in the same direction, at least one under workload — so a lone noisy cell shows its arrow here without tagging the PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:docs Documentation area:state AppState, models, persistence area:tests Test changes benchmark:regression CI benchmark: significant resource regression vs main

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant