diff --git a/cmd/harnesscli/tui/components/spinner/breathing_test.go b/cmd/harnesscli/tui/components/spinner/breathing_test.go new file mode 100644 index 00000000..fe85a640 --- /dev/null +++ b/cmd/harnesscli/tui/components/spinner/breathing_test.go @@ -0,0 +1,106 @@ +package spinner + +import ( + "testing" +) + +// cycleGlyphs ticks a started spinner until the pulse returns to its first +// step, returning the glyph observed on every tick. +func cycleGlyphs(t *testing.T) []string { + t.Helper() + m := New(0).Start().SetAction("Thinking") + + var seen []string + for i := 0; i < 200; i++ { // generous bound; a cycle is far shorter + seen = append(seen, m.Glyph()) + m = m.Tick() + if m.step == 0 && m.stepTicks == 0 && i > 0 { + return seen + } + } + t.Fatal("spinner never completed a cycle within 200 ticks") + return nil +} + +// TestSpinnerCycleTakesAboutTwoSeconds pins the slower cadence asked for in +// issue #1420. At the unchanged 120ms tick, a cycle should run about 2s rather +// than the previous 720ms. +func TestSpinnerCycleTakesAboutTwoSeconds(t *testing.T) { + got := len(cycleGlyphs(t)) + const want = 18 // 18 ticks x 120ms = 2.16s + if got != want { + t.Fatalf("cycle is %d ticks (%.2fs at 120ms), want %d (%.2fs)", + got, float64(got)*0.12, want, float64(want)*0.12) + } +} + +// TestSpinnerEasesAtTheExtremes pins the easing: the animation lingers at the +// top and bottom of the breath and moves quickly through the middle. A flat +// cadence — every frame held equally — is what made it read as a tick. +func TestSpinnerEasesAtTheExtremes(t *testing.T) { + holds := map[string]int{} + for _, g := range cycleGlyphs(t) { + holds[g]++ + } + + lightest, heaviest := pulse[0], pulse[len(pulse)/2] + for _, mid := range []string{"✳", "✻"} { + if holds[lightest] <= holds[mid] { + t.Errorf("extreme %q held %d ticks, mid-pulse %q held %d: extremes must linger longer", + lightest, holds[lightest], mid, holds[mid]) + } + if holds[heaviest] <= holds[mid] { + t.Errorf("extreme %q held %d ticks, mid-pulse %q held %d: extremes must linger longer", + heaviest, holds[heaviest], mid, holds[mid]) + } + } +} + +// TestSpinnerPulseGrowsThenShrinks pins the shape. The old order +// (✶ · ✻ ✽ ✳ ✢) jumped from the heaviest glyph straight to the lightest, which +// reads as a stutter however slowly it runs. +func TestSpinnerPulseGrowsThenShrinks(t *testing.T) { + weight := map[string]int{"·": 0, "✢": 1, "✳": 2, "✻": 3, "✽": 4, "✶": 5} + + var seq []int + for _, g := range pulse { + w, ok := weight[g] + if !ok { + t.Fatalf("pulse contains unknown glyph %q", g) + } + seq = append(seq, w) + } + + peak := 0 + for i, w := range seq { + if w > seq[peak] { + peak = i + } + } + for i := 1; i <= peak; i++ { + if seq[i] <= seq[i-1] { + t.Fatalf("pulse does not grow monotonically to its peak at step %d: %v", i, seq) + } + } + for i := peak + 1; i < len(seq); i++ { + if seq[i] >= seq[i-1] { + t.Fatalf("pulse does not shrink monotonically after its peak at step %d: %v", i, seq) + } + } + // The wrap back to the start must not be a jump from heaviest to lightest. + if seq[len(seq)-1]-seq[0] > 1 { + t.Fatalf("wrap from %v back to %v is a jump, not a breath", seq[len(seq)-1], seq[0]) + } +} + +// TestSpinnerStillAnimatesUnderEasing is the control: a hold table that never +// released would satisfy "slower" while freezing the spinner outright. +func TestSpinnerStillAnimatesUnderEasing(t *testing.T) { + seen := map[string]bool{} + for _, g := range cycleGlyphs(t) { + seen[g] = true + } + if len(seen) < 4 { + t.Fatalf("only %d distinct glyphs in a full cycle; the spinner would look stalled", len(seen)) + } +} diff --git a/cmd/harnesscli/tui/components/spinner/model.go b/cmd/harnesscli/tui/components/spinner/model.go index 276ea3b4..c8377dea 100644 --- a/cmd/harnesscli/tui/components/spinner/model.go +++ b/cmd/harnesscli/tui/components/spinner/model.go @@ -13,9 +13,25 @@ import ( "github.com/charmbracelet/lipgloss" ) -// frames are the 6 animation frames for the thinking spinner. -// These are star/asterisk glyphs, not the braille frames in theme.go. -var frames = []string{"✶", "·", "✻", "✽", "✳", "✢"} +// pulse is the glyph sequence, ordered by visual weight and ping-ponged so the +// cycle grows and shrinks — a breath rather than a tick. It uses the same six +// star/asterisk glyphs as before (not the braille frames in theme.go); only +// their order changed. The previous order, ✶ · ✻ ✽ ✳ ✢, jumped from the +// heaviest glyph straight to the lightest, which reads as a stutter at any +// speed. The extremes are not repeated back-to-back at the turn. Issue #1420. +var pulse = []string{"·", "✢", "✳", "✻", "✽", "✶", "✽", "✻", "✳", "✢"} + +// holds is how many ticks each step of the pulse is displayed for. The +// animation lingers at the top and bottom of the breath (3 ticks = 360ms) and +// passes quickly through the middle (1 tick = 120ms). That unevenness is the +// easing: a flat cadence is what made the old spinner read as mechanical. +// +// The tick rate itself deliberately stays at 120ms (tui.SpinnerInterval), +// because the same tick redraws the elapsed-time counter. Slowing the timer +// would slow the clock; gating advance slows only the glyph. +// +// Sum: 18 ticks, about 2.16s per cycle, against 720ms before. +var holds = []int{3, 2, 1, 1, 2, 3, 2, 1, 1, 2} // durationThreshold is the elapsed time after which the spinner shows a duration. const durationThreshold = 2 * time.Second @@ -49,7 +65,8 @@ func DefaultStyles() Styles { // All mutation methods return a new Model value — never modify in place. // This keeps it safe for use in BubbleTea's single-goroutine Update(). type Model struct { - frame int // current frame index [0, len(frames)) + step int // index into pulse + stepTicks int // ticks already spent on the current step action string // what the run is currently doing; empty falls back to fallbackLabel startTime time.Time // when spinner started (for duration) tokens int // token count stored on Stop() @@ -79,7 +96,8 @@ func (m Model) Start() Model { m.active = true m.done = false m.startTime = time.Now() - m.frame = 0 + m.step = 0 + m.stepTicks = 0 m.tickCount = 0 return m } @@ -102,7 +120,13 @@ func (m Model) Tick() Model { return m } m.tickCount++ - m.frame = (m.frame + 1) % len(frames) + + // Advance only once the current step has been held for its full duration. + m.stepTicks++ + if m.stepTicks >= holds[m.step] { + m.stepTicks = 0 + m.step = (m.step + 1) % len(pulse) + } return m } @@ -141,6 +165,14 @@ func (m Model) stylesOrDefault() Styles { return *m.styles } +// Glyph returns the animation character for the current step of the pulse. +func (m Model) Glyph() string { + if len(pulse) == 0 { + return "" + } + return pulse[m.step%len(pulse)] +} + // IsActive returns true while the spinner is running (between Start and Stop). func (m Model) IsActive() bool { return m.active } @@ -183,7 +215,7 @@ func (m Model) View(width int) string { return "" } - currentFrame := frames[m.frame] + currentFrame := m.Glyph() // The label states what is actually happening. No ellipsis: "Running bash" // is a fact, and trailing dots would only suggest vagueness it does not have. @@ -256,7 +288,7 @@ func shortenLabel(glyph, label, full string, width int) string { // // Format: "✻ Worked for 5s" or "✻ Worked for 1m 30s" func (m Model) CompletionLine(seconds float64) string { - glyph := frames[m.frame%len(frames)] + glyph := m.Glyph() duration := formatSeconds(seconds) line := glyph + " Worked for " + duration return m.stylesOrDefault().Dim.Render(line) diff --git a/cmd/harnesscli/tui/components/spinner/model_test.go b/cmd/harnesscli/tui/components/spinner/model_test.go index 99cf74a5..8b287aa0 100644 --- a/cmd/harnesscli/tui/components/spinner/model_test.go +++ b/cmd/harnesscli/tui/components/spinner/model_test.go @@ -10,24 +10,29 @@ import ( "time" ) -// TestTUI024_SpinnerCyclesFrames verifies that Tick() advances frame index -// through all 6 frames in order and wraps back to 0. -func TestTUI024_SpinnerCyclesFrames(t *testing.T) { +// TestTUI024_SpinnerAdvancesThroughThePulse verifies that Start resets the +// animation and that ticking walks the pulse. The exact cadence is eased rather +// than one-frame-per-tick (issue #1420) and is pinned in breathing_test.go. +func TestTUI024_SpinnerAdvancesThroughThePulse(t *testing.T) { m := New(42) m = m.Start() - // Frame starts at 0 after Start. - if m.frame != 0 { - t.Fatalf("expected frame=0 after Start, got %d", m.frame) + if m.step != 0 || m.stepTicks != 0 { + t.Fatalf("expected pulse reset after Start, got step=%d stepTicks=%d", m.step, m.stepTicks) } - for i := 1; i <= 6; i++ { + start := m.Glyph() + advanced := false + for i := 0; i < len(holds); i++ { m = m.Tick() - expected := i % len(frames) - if m.frame != expected { - t.Errorf("after Tick %d: expected frame=%d, got %d", i, expected, m.frame) + if m.Glyph() != start { + advanced = true + break } } + if !advanced { + t.Errorf("glyph never changed while ticking; the spinner would look frozen") + } } // TestTUI024_SpinnerAddsDurationAfterThreshold verifies that View() includes @@ -94,7 +99,7 @@ func TestTUI024_CompletionLineFormat(t *testing.T) { // Frame glyph must appear. foundGlyph := false - for _, f := range frames { + for _, f := range pulse { if strings.Contains(line, f) { foundGlyph = true break @@ -304,9 +309,9 @@ func TestTUI024_Regression_MultipleInstances(t *testing.T) { a = a.Tick() } - // b should still be at frame 0. - if b.frame != 0 { - t.Errorf("regression: b.frame should be 0, got %d (a.frame=%d)", b.frame, a.frame) + // b should still be at the start of the pulse. + if b.step != 0 || b.stepTicks != 0 { + t.Errorf("regression: b should be unadvanced, got step=%d stepTicks=%d (a.step=%d)", b.step, b.stepTicks, a.step) } } diff --git a/cmd/harnesscli/tui/components/spinner/testdata/snapshots/TUI-024-spinner-120x40.txt b/cmd/harnesscli/tui/components/spinner/testdata/snapshots/TUI-024-spinner-120x40.txt index baf4b683..5af6a62e 100644 --- a/cmd/harnesscli/tui/components/spinner/testdata/snapshots/TUI-024-spinner-120x40.txt +++ b/cmd/harnesscli/tui/components/spinner/testdata/snapshots/TUI-024-spinner-120x40.txt @@ -1,11 +1,11 @@ # TUI-024 Spinner Snapshot 120x40 ------------------------------------------------------------------------------------------------------------------------ ## Active (no duration) -✽ Working (esc to interrupt) +✢ Working (esc to interrupt) ## Active (5s elapsed) -✽ Working (5.0s) (esc to interrupt) +✢ Working (5.0s) (esc to interrupt) ## Completion Line -✽ Worked for 5.0s +✢ Worked for 5.0s ------------------------------------------------------------------------------------------------------------------------ diff --git a/cmd/harnesscli/tui/components/spinner/testdata/snapshots/TUI-024-spinner-200x50.txt b/cmd/harnesscli/tui/components/spinner/testdata/snapshots/TUI-024-spinner-200x50.txt index 5847de7f..77f6111d 100644 --- a/cmd/harnesscli/tui/components/spinner/testdata/snapshots/TUI-024-spinner-200x50.txt +++ b/cmd/harnesscli/tui/components/spinner/testdata/snapshots/TUI-024-spinner-200x50.txt @@ -1,11 +1,11 @@ # TUI-024 Spinner Snapshot 200x50 -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- ## Active (no duration) -✽ Working (esc to interrupt) +✢ Working (esc to interrupt) ## Active (5s elapsed) -✽ Working (5.0s) (esc to interrupt) +✢ Working (5.0s) (esc to interrupt) ## Completion Line -✽ Worked for 5.0s +✢ Worked for 5.0s -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- diff --git a/cmd/harnesscli/tui/components/spinner/testdata/snapshots/TUI-024-spinner-80x24.txt b/cmd/harnesscli/tui/components/spinner/testdata/snapshots/TUI-024-spinner-80x24.txt index ae11eb84..76fba9ab 100644 --- a/cmd/harnesscli/tui/components/spinner/testdata/snapshots/TUI-024-spinner-80x24.txt +++ b/cmd/harnesscli/tui/components/spinner/testdata/snapshots/TUI-024-spinner-80x24.txt @@ -1,11 +1,11 @@ # TUI-024 Spinner Snapshot 80x24 -------------------------------------------------------------------------------- ## Active (no duration) -✽ Working (esc to interrupt) +✢ Working (esc to interrupt) ## Active (5s elapsed) -✽ Working (5.0s) (esc to interrupt) +✢ Working (5.0s) (esc to interrupt) ## Completion Line -✽ Worked for 5.0s +✢ Worked for 5.0s -------------------------------------------------------------------------------- diff --git a/cmd/harnesscli/tui/components/spinner/testdata/snapshots/TUI-025-completion-120x40.txt b/cmd/harnesscli/tui/components/spinner/testdata/snapshots/TUI-025-completion-120x40.txt index 39588dda..40c557c9 100644 --- a/cmd/harnesscli/tui/components/spinner/testdata/snapshots/TUI-025-completion-120x40.txt +++ b/cmd/harnesscli/tui/components/spinner/testdata/snapshots/TUI-025-completion-120x40.txt @@ -2,11 +2,11 @@ ------------------------------------------------------------------------------------------------------------------------ ## Immediately after Stop() — ShowsCompletion=true ShowsCompletion: true -✽ Worked for 5.0s +✢ Worked for 5.0s ## After 5 ticks (mid-completion) ShowsCompletion: true -✽ Worked for 5.0s +✢ Worked for 5.0s ## After 10 ticks (silent) ShowsCompletion: false diff --git a/cmd/harnesscli/tui/components/spinner/testdata/snapshots/TUI-025-completion-200x50.txt b/cmd/harnesscli/tui/components/spinner/testdata/snapshots/TUI-025-completion-200x50.txt index 3df94569..f7cfa37c 100644 --- a/cmd/harnesscli/tui/components/spinner/testdata/snapshots/TUI-025-completion-200x50.txt +++ b/cmd/harnesscli/tui/components/spinner/testdata/snapshots/TUI-025-completion-200x50.txt @@ -2,11 +2,11 @@ -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- ## Immediately after Stop() — ShowsCompletion=true ShowsCompletion: true -✽ Worked for 5.0s +✢ Worked for 5.0s ## After 5 ticks (mid-completion) ShowsCompletion: true -✽ Worked for 5.0s +✢ Worked for 5.0s ## After 10 ticks (silent) ShowsCompletion: false diff --git a/cmd/harnesscli/tui/components/spinner/testdata/snapshots/TUI-025-completion-80x24.txt b/cmd/harnesscli/tui/components/spinner/testdata/snapshots/TUI-025-completion-80x24.txt index 29ed814d..62f01b0c 100644 --- a/cmd/harnesscli/tui/components/spinner/testdata/snapshots/TUI-025-completion-80x24.txt +++ b/cmd/harnesscli/tui/components/spinner/testdata/snapshots/TUI-025-completion-80x24.txt @@ -2,11 +2,11 @@ -------------------------------------------------------------------------------- ## Immediately after Stop() — ShowsCompletion=true ShowsCompletion: true -✽ Worked for 5.0s +✢ Worked for 5.0s ## After 5 ticks (mid-completion) ShowsCompletion: true -✽ Worked for 5.0s +✢ Worked for 5.0s ## After 10 ticks (silent) ShowsCompletion: false diff --git a/cmd/harnesscli/tui/components/spinner/truthful_label_test.go b/cmd/harnesscli/tui/components/spinner/truthful_label_test.go index d14d8bac..b8b211d6 100644 --- a/cmd/harnesscli/tui/components/spinner/truthful_label_test.go +++ b/cmd/harnesscli/tui/components/spinner/truthful_label_test.go @@ -32,13 +32,16 @@ func TestSpinnerLabelDoesNotRotateOnTicks(t *testing.T) { func TestSpinnerGlyphStillAnimates(t *testing.T) { m := New(42).Start().SetAction("Running bash") + // The bound is the longest hold in the eased cadence (issue #1420), not one + // tick per frame: the extremes of the breath are held for several ticks. seen := map[string]bool{} - for i := 0; i < len(frames); i++ { + ticks := len(holds) * 3 + for i := 0; i < ticks; i++ { seen[strings.Fields(m.View(80))[0]] = true m = m.Tick() } if len(seen) < 2 { - t.Fatalf("glyph never advanced across %d ticks; the spinner would look frozen", len(frames)) + t.Fatalf("glyph never advanced across %d ticks; the spinner would look frozen", ticks) } } diff --git a/cmd/harnesscli/tui/testdata/snapshots/TUI-039-cancel-120x40.txt b/cmd/harnesscli/tui/testdata/snapshots/TUI-039-cancel-120x40.txt index 2f13664d..a1d0ca25 100644 --- a/cmd/harnesscli/tui/testdata/snapshots/TUI-039-cancel-120x40.txt +++ b/cmd/harnesscli/tui/testdata/snapshots/TUI-039-cancel-120x40.txt @@ -33,7 +33,7 @@ ──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────── -✶ Working (esc to interrupt) +· Working (esc to interrupt) ╭───────────────────────────────────────────────────╮ │ ⚠ Press Ctrl+C again to stop, or Esc to continue │ ╰───────────────────────────────────────────────────╯ diff --git a/cmd/harnesscli/tui/testdata/snapshots/TUI-039-cancel-200x50.txt b/cmd/harnesscli/tui/testdata/snapshots/TUI-039-cancel-200x50.txt index bda3da57..fce313a5 100644 --- a/cmd/harnesscli/tui/testdata/snapshots/TUI-039-cancel-200x50.txt +++ b/cmd/harnesscli/tui/testdata/snapshots/TUI-039-cancel-200x50.txt @@ -43,7 +43,7 @@ ──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────── -✶ Working (esc to interrupt) +· Working (esc to interrupt) ╭───────────────────────────────────────────────────╮ │ ⚠ Press Ctrl+C again to stop, or Esc to continue │ ╰───────────────────────────────────────────────────╯ diff --git a/cmd/harnesscli/tui/testdata/snapshots/TUI-039-cancel-80x24.txt b/cmd/harnesscli/tui/testdata/snapshots/TUI-039-cancel-80x24.txt index a4df1a60..cb5f4801 100644 --- a/cmd/harnesscli/tui/testdata/snapshots/TUI-039-cancel-80x24.txt +++ b/cmd/harnesscli/tui/testdata/snapshots/TUI-039-cancel-80x24.txt @@ -17,7 +17,7 @@ ──────────────────────────────────────────────────────────────────────────────── -✶ Working (esc to interrupt) +· Working (esc to interrupt) ╭───────────────────────────────────────────────────╮ │ ⚠ Press Ctrl+C again to stop, or Esc to continue │ ╰───────────────────────────────────────────────────╯ diff --git a/docs/logs/engineering-log.md b/docs/logs/engineering-log.md index 9cd4cce3..35dc7bad 100644 --- a/docs/logs/engineering-log.md +++ b/docs/logs/engineering-log.md @@ -1,5 +1,41 @@ # Engineering Log +## 2026-09-08 — Issue #1420 spinner breathes instead of ticking + +- Symptom: after #1415 stopped the *word* rotating, the owner reported the + spinner still "spins too fast" and asked to "slow it down so that it + breaths." The complaint was the glyph, not the label: six frames advancing + one per 120ms tick is a full rotation every 720ms, with every frame weighted + identically — a mechanical tick rather than a living indicator. +- Second, subtler problem: the frame order `✶ · ✻ ✽ ✳ ✢` was not monotonic in + visual weight. It jumped from the heaviest glyph straight to the lightest and + back, which reads as a stutter at any speed. +- Fix: the same six glyphs, reordered by weight and ping-ponged into a pulse + (`· ✢ ✳ ✻ ✽ ✶ ✽ ✻ ✳ ✢`), with a per-step hold table + (`3 2 1 1 2 3 2 1 1 2` ticks) so advance is gated rather than one-per-tick. + The extremes hold 360ms, the mid-pulse frames pass in 120ms, and a cycle runs + 18 ticks — about 2.16s, against 720ms before. The uneven hold is the easing: + a flat cadence is precisely what made it feel mechanical. +- Deliberately unchanged: `tui.SpinnerInterval` stays at 120ms. The same tick + redraws the elapsed-time counter, so slowing the timer would slow the clock. + Gating the glyph's advance slows only the glyph. The glyph set itself is also + unchanged — it matches the six independently reported for Claude Code, and + was never the problem. +- Provenance note, since it informed the design: the claim that Claude Code + eases its frames (first and last held longer) comes from third parties + hand-timing screen recordings, not from reading its source. That is a + description, not a measurement, and the hold table here is our own choice + rather than a reproduction of theirs. +- Tests: `TestSpinnerCycleTakesAboutTwoSeconds` (18 ticks, not 6), + `TestSpinnerEasesAtTheExtremes` (extremes strictly outlast mid-pulse frames), + `TestSpinnerPulseGrowsThenShrinks` (monotonic up then down, no heaviest-to- + lightest jump), and `TestSpinnerStillAnimatesUnderEasing` as the control + against a hold table that would freeze the animation. All in + `cmd/harnesscli/tui/components/spinner/breathing_test.go`. + `TestTUI024_SpinnerCyclesFrames` pinned one-frame-per-tick and was replaced by + `TestTUI024_SpinnerAdvancesThroughThePulse`; the `#1415` glyph-animation guard + widened its bound to the longest hold. Snapshot goldens regenerated. + ## 2026-09-08 — Issue #1415 truthful spinner label - Symptom/motivation: `cmd/harnesscli/tui/components/spinner/verbs.go` held