1 files changed, 74 insertions, 111 deletions
diff --git a/packages/cli/src/ui/hooks/useLoadingIndicator.test.ts b/packages/cli/src/ui/hooks/useLoadingIndicator.test.ts
index 496e13d3..c20ded88 100644
--- a/packages/cli/src/ui/hooks/useLoadingIndicator.test.ts
+++ b/packages/cli/src/ui/hooks/useLoadingIndicator.test.ts
@@ -4,161 +4,124 @@
  * SPDX-License-Identifier: Apache-2.0
  */
 
-import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
-import { renderHook, act } from '@testing-library/react';
+import { describe, it, expect, vi, beforeEach } from 'vitest';
+import { act, renderHook } from '@testing-library/react';
 import { useLoadingIndicator } from './useLoadingIndicator.js';
 import { StreamingState } from '../types.js';
 import {
   WITTY_LOADING_PHRASES,
   PHRASE_CHANGE_INTERVAL_MS,
-} from '../constants.js';
+} from './usePhraseCycler.js';
 
 describe('useLoadingIndicator', () => {
   beforeEach(() => {
     vi.useFakeTimers();
+    vi.clearAllMocks();
   });
 
   afterEach(() => {
-    vi.restoreAllMocks();
+    vi.runOnlyPendingTimers();
+    vi.useRealTimers(); // Restore real timers after each test
   });
 
-  it('should initialize with default values when not responding', () => {
+  it('should initialize with default values when Idle', () => {
     const { result } = renderHook(() =>
-      useLoadingIndicator(StreamingState.Idle, false),
+      useLoadingIndicator(StreamingState.Idle),
     );
     expect(result.current.elapsedTime).toBe(0);
     expect(result.current.currentLoadingPhrase).toBe(WITTY_LOADING_PHRASES[0]);
-    expect(result.current.shouldShowSpinner).toBe(true);
   });
 
-  describe('when streamingState is Responding', () => {
-    it('should increment elapsedTime and cycle phrases when not paused', () => {
-      const { result } = renderHook(() =>
-        useLoadingIndicator(StreamingState.Responding, false),
-      );
-      expect(result.current.shouldShowSpinner).toBe(true);
-      expect(result.current.currentLoadingPhrase).toBe(
-        WITTY_LOADING_PHRASES[0],
-      );
+  it('should reflect values when Responding', () => {
+    const { result } = renderHook(() =>
+      useLoadingIndicator(StreamingState.Responding),
+    );
 
-      act(() => {
-        vi.advanceTimersByTime(1000);
-      });
-      expect(result.current.elapsedTime).toBe(1);
+    // Initial state before timers advance
+    expect(result.current.elapsedTime).toBe(0);
+    expect(result.current.currentLoadingPhrase).toBe(WITTY_LOADING_PHRASES[0]);
 
-      act(() => {
-        vi.advanceTimersByTime(PHRASE_CHANGE_INTERVAL_MS);
-      });
-      expect(result.current.currentLoadingPhrase).toBe(
-        WITTY_LOADING_PHRASES[1],
-      );
-      expect(result.current.elapsedTime).toBe(
-        1 + PHRASE_CHANGE_INTERVAL_MS / 1000,
-      );
+    act(() => {
+      vi.advanceTimersByTime(PHRASE_CHANGE_INTERVAL_MS);
     });
+    // Phrase should cycle if PHRASE_CHANGE_INTERVAL_MS has passed
+    // This depends on the actual implementation of usePhraseCycler
+    // For simplicity, we'll check it's one of the witty phrases
+    expect(result.current.currentLoadingPhrase).toBe(WITTY_LOADING_PHRASES[1]);
+  });
 
-    it('should pause elapsedTime, show specific phrase, and hide spinner when paused', () => {
-      const { result, rerender } = renderHook(
-        ({ isPaused }) =>
-          useLoadingIndicator(StreamingState.Responding, isPaused),
-        { initialProps: { isPaused: false } },
-      );
+  it('should show waiting phrase and retain elapsedTime when WaitingForConfirmation', () => {
+    const { result, rerender } = renderHook(
+      ({ streamingState }) => useLoadingIndicator(streamingState),
+      { initialProps: { streamingState: StreamingState.Responding } },
+    );
 
-      act(() => {
-        vi.advanceTimersByTime(2000);
-      });
-      expect(result.current.elapsedTime).toBe(2);
-      expect(result.current.shouldShowSpinner).toBe(true);
+    act(() => {
+      vi.advanceTimersByTime(60000);
+    });
+    expect(result.current.elapsedTime).toBe(60);
 
-      rerender({ isPaused: true });
+    rerender({ streamingState: StreamingState.WaitingForConfirmation });
 
-      expect(result.current.currentLoadingPhrase).toBe(
-        'Waiting for user confirmation...',
-      );
-      expect(result.current.shouldShowSpinner).toBe(false);
+    expect(result.current.currentLoadingPhrase).toBe(
+      'Waiting for user confirmation...',
+    );
+    expect(result.current.elapsedTime).toBe(60); // Elapsed time should be retained
 
-      // Time should not advance while paused
-      const timeBeforePauseAdv = result.current.elapsedTime;
-      act(() => {
-        vi.advanceTimersByTime(3000);
-      });
-      expect(result.current.elapsedTime).toBe(timeBeforePauseAdv);
+    // Timer should not advance further
+    act(() => {
+      vi.advanceTimersByTime(2000);
+    });
+    expect(result.current.elapsedTime).toBe(60);
+  });
 
-      // Unpause
-      rerender({ isPaused: false });
-      expect(result.current.shouldShowSpinner).toBe(true);
-      // Phrase should reset to the beginning of witty phrases
-      expect(result.current.currentLoadingPhrase).toBe(
-        WITTY_LOADING_PHRASES[0],
-      );
+  it('should reset elapsedTime and use initial phrase when transitioning from WaitingForConfirmation to Responding', () => {
+    const { result, rerender } = renderHook(
+      ({ streamingState }) => useLoadingIndicator(streamingState),
+      { initialProps: { streamingState: StreamingState.Responding } },
+    );
 
-      act(() => {
-        vi.advanceTimersByTime(1000);
-      });
-      // Elapsed time should resume from where it left off
-      expect(result.current.elapsedTime).toBe(timeBeforePauseAdv + 1);
+    act(() => {
+      vi.advanceTimersByTime(5000); // 5s
     });
+    expect(result.current.elapsedTime).toBe(5);
 
-    it('should reset timer and phrase when streamingState changes from Responding to Idle', () => {
-      const { result, rerender } = renderHook(
-        ({ streamingState }) => useLoadingIndicator(streamingState, false),
-        { initialProps: { streamingState: StreamingState.Responding } },
-      );
-
-      act(() => {
-        vi.advanceTimersByTime(PHRASE_CHANGE_INTERVAL_MS + 1000);
-      });
-      expect(result.current.elapsedTime).toBe(
-        PHRASE_CHANGE_INTERVAL_MS / 1000 + 1,
-      );
-      expect(result.current.currentLoadingPhrase).toBe(
-        WITTY_LOADING_PHRASES[1],
-      );
+    rerender({ streamingState: StreamingState.WaitingForConfirmation });
+    expect(result.current.elapsedTime).toBe(5);
+    expect(result.current.currentLoadingPhrase).toBe(
+      'Waiting for user confirmation...',
+    );
 
-      rerender({ streamingState: StreamingState.Idle });
+    rerender({ streamingState: StreamingState.Responding });
+    expect(result.current.elapsedTime).toBe(0); // Should reset
+    expect(result.current.currentLoadingPhrase).toBe(WITTY_LOADING_PHRASES[0]);
 
-      expect(result.current.elapsedTime).toBe(0);
-      // When idle, the phrase interval should be cleared, but the last phrase might persist
-      // until the next "Responding" state. The important part is that the timer is reset.
-      // Depending on exact timing, it might be the last witty phrase or the first.
-      // For this test, we'll ensure it's one of them.
-      expect(WITTY_LOADING_PHRASES).toContain(
-        result.current.currentLoadingPhrase,
-      );
+    act(() => {
+      vi.advanceTimersByTime(1000);
     });
+    expect(result.current.elapsedTime).toBe(1);
   });
 
-  it('should clear intervals on unmount', () => {
-    const { unmount } = renderHook(() =>
-      useLoadingIndicator(StreamingState.Responding, false),
-    );
-    const clearIntervalSpy = vi.spyOn(global, 'clearInterval');
-    unmount();
-    // Expecting two intervals (elapsedTime and phraseInterval) to be cleared.
-    expect(clearIntervalSpy).toHaveBeenCalledTimes(2);
-  });
-
-  it('should reset to initial witty phrase when unpaused', () => {
+  it('should reset timer and phrase when streamingState changes from Responding to Idle', () => {
     const { result, rerender } = renderHook(
-      ({ isPaused }) =>
-        useLoadingIndicator(StreamingState.Responding, isPaused),
-      { initialProps: { isPaused: false } },
+      ({ streamingState }) => useLoadingIndicator(streamingState),
+      { initialProps: { streamingState: StreamingState.Responding } },
     );
 
-    // Advance to the second witty phrase
     act(() => {
-      vi.advanceTimersByTime(PHRASE_CHANGE_INTERVAL_MS);
+      vi.advanceTimersByTime(10000); // 10s
     });
-    expect(result.current.currentLoadingPhrase).toBe(WITTY_LOADING_PHRASES[1]);
+    expect(result.current.elapsedTime).toBe(10);
 
-    // Pause
-    rerender({ isPaused: true });
-    expect(result.current.currentLoadingPhrase).toBe(
-      'Waiting for user confirmation...',
-    );
+    rerender({ streamingState: StreamingState.Idle });
 
-    // Unpause
-    rerender({ isPaused: false });
+    expect(result.current.elapsedTime).toBe(0);
     expect(result.current.currentLoadingPhrase).toBe(WITTY_LOADING_PHRASES[0]);
+
+    // Timer should not advance
+    act(() => {
+      vi.advanceTimersByTime(2000);
+    });
+    expect(result.current.elapsedTime).toBe(0);
   });
 });