Skip to content

Add persistent UI status indicators#69

Open
Whamp wants to merge 1 commit intomainfrom
palette/status-indicators-17587739269275448204
Open

Add persistent UI status indicators#69
Whamp wants to merge 1 commit intomainfrom
palette/status-indicators-17587739269275448204

Conversation

@Whamp
Copy link
Copy Markdown
Owner

@Whamp Whamp commented Feb 7, 2026

User description

Added a persistent rich.console.Status spinner to the main application loop to provide visual feedback.

  • Displays "Ready" (dots) when idle.
  • Displays "Recording..." (point, red) when capturing audio.
  • Displays "Transcribing..." (dots, green) when processing.
  • Includes unit tests in tests/test_ui_status.py verifying state transitions and race condition handling.

PR created automatically by Jules for task 17587739269275448204 started by @Whamp


PR Type

Enhancement, Tests


Description

  • Add persistent status spinner showing app state (Ready/Recording/Transcribing)

  • Update status indicator during recording start, stop, and transcription

  • Reset status to Ready after transcription completes, unless recording resumed

  • Include comprehensive unit tests for UI state transitions and race conditions


Diagram Walkthrough

flowchart LR
  A["App Running"] -->|"with status spinner"| B["Ready State"]
  B -->|"toggle_recording"| C["Recording State"]
  C -->|"update status"| D["Recording... spinner"]
  D -->|"stop_recording"| E["Transcribing State"]
  E -->|"update status"| F["Transcribing... spinner"]
  F -->|"transcribe_and_inject"| G["Ready State"]
  G -->|"unless _recording=True"| H["Stay in Recording"]
Loading

File Walkthrough

Relevant files
Enhancement
main.py
Add persistent status indicators throughout app lifecycle

src/chirp/main.py

  • Store console instance as self.console and add self.status_indicator
    attribute
  • Wrap main event loop with persistent status spinner showing "Ready"
    state
  • Update status to "[bold red]Recording...[/bold red]" with point
    spinner when recording starts
  • Update status to "[bold green]Transcribing...[/bold green]" when
    recording stops
  • Reset status to "Ready" after transcription completes, with guard to
    prevent reset if recording resumed
  • Add error handling to reset status on audio capture failures
  • Refactor _transcribe_and_inject with try-finally block for status
    cleanup
+34/-19 
Tests
test_ui_status.py
Add unit tests for UI status indicator state transitions 

tests/test_ui_status.py

  • Create comprehensive test suite for UI status indicator functionality
  • Mock system dependencies (sounddevice, keyboard, pyperclip) and
    ChirpApp components
  • Test status update on recording start with "[bold
    red]Recording...[/bold red]" and point spinner
  • Test status update on recording stop with "[bold
    green]Transcribing...[/bold green]"
  • Test status reset to "Ready" after successful transcription
  • Test race condition handling where recording resumes during
    transcription (status not reset)
+156/-0 

…bing

Co-authored-by: Whamp <1115485+Whamp@users.noreply.github.com>
@google-labs-jules
Copy link
Copy Markdown
Contributor

👋 Jules, reporting for duty! I'm here to lend a hand with this pull request.

When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down.

I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job!

For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with @jules. You can find this option in the Pull Request section of your global Jules UI settings. You can always switch back!

New to Jules? Learn more at jules.google/docs.


For security, I will only act on instructions from the user who triggered this task.

@qodo-code-review
Copy link
Copy Markdown

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
🟢
No security concerns identified No security vulnerabilities detected by AI analysis. Human verification advised for critical code.
Ticket Compliance
🎫 No ticket provided
  • Create ticket/issue
Codebase Duplication Compliance
Codebase context is not defined

Follow the guide to enable codebase context checks.

Custom Compliance
🟢
Generic: Comprehensive Audit Trails

Objective: To create a detailed and reliable record of critical system actions for security analysis
and compliance.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Meaningful Naming and Self-Documenting Code

Objective: Ensure all identifiers clearly express their purpose and intent, making code
self-documenting

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Robust Error Handling and Edge Case Management

Objective: Ensure comprehensive error handling that provides meaningful context and graceful
degradation

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Error Handling

Objective: To prevent the leakage of sensitive system information through error messages while
providing sufficient detail for internal debugging.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Security-First Input Validation and Data Handling

Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent
vulnerabilities

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

🔴
Generic: Secure Logging Practices

Objective: To ensure logs are useful for debugging and auditing without exposing sensitive
information like PII, PHI, or cardholder data.

Status:
Sensitive data logged: The PR adds debug logging of the full transcribed text (text), which may contain
sensitive/PII content and should not be written to logs.

Referred Code
self.logger.debug("Transcription: %s", text)
self.text_injector.inject(text)

Learn more about managing compliance generic rules or creating your own custom rules

Compliance status legend 🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

@qodo-code-review
Copy link
Copy Markdown

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
Fix flawed test logic

In test_transcribe_does_not_reset_status_if_recording, replace the loop that
checks mock_status.update calls with self.mock_status.update.assert_not_called()
to fix the vacuously passing test.

tests/test_ui_status.py [134-153]

 def test_transcribe_does_not_reset_status_if_recording(self):
     """Test that transcription does NOT reset status if recording started again."""
     # Setup
     mock_waveform = MagicMock()
     mock_waveform.size = 100
 
     # Mock successful transcription
     self.app.parakeet.transcribe.return_value = "Test text"
 
     # Simulate recording started during transcription
     self.app._recording = True
 
     self.app._transcribe_and_inject(mock_waveform)
 
-    # Verify status calls
-    calls = self.mock_status.update.call_args_list
-    # Check that no call was made with "Ready"
-    for call in calls:
-        args, _ = call
-        self.assertNotEqual(args[0], "Ready", "Should not set status to Ready if recording")
+    # Verify that the status was not updated back to "Ready"
+    self.mock_status.update.assert_not_called()
  • Apply / Chat
Suggestion importance[1-10]: 7

__

Why: The suggestion correctly identifies a flawed test that passes vacuously and proposes a more robust assertion, improving the test suite's reliability.

Medium
High-level
Decouple UI logic from core application state

Decouple UI logic from core application logic by introducing a state management
system. Core methods should only update the application's state, and a separate
observer should handle UI updates based on that state.

Examples:

src/chirp/main.py [123-124]
            if self.status_indicator:
                self.status_indicator.update("[bold red]Recording...[/bold red]", spinner="point")
src/chirp/main.py [152-153]
        if self.status_indicator:
            self.status_indicator.update("[bold green]Transcribing...[/bold green]", spinner="dots")

Solution Walkthrough:

Before:

class ChirpApp:
    # ...
    def _start_recording(self):
        if self.status_indicator:
            self.status_indicator.update("[bold red]Recording...[/bold red]", spinner="point")
        # ... recording logic ...

    def _stop_recording(self):
        if self.status_indicator:
            self.status_indicator.update("[bold green]Transcribing...[/bold green]", spinner="dots")
        # ... stop logic ...

    def _transcribe_and_inject(self, waveform):
        try:
            # ... transcription logic ...
        finally:
            if self.status_indicator and not self._recording:
                self.status_indicator.update("Ready", spinner="dots")

After:

from enum import Enum, auto

class AppState(Enum):
    READY = auto()
    RECORDING = auto()
    TRANSCRIBING = auto()

class ChirpApp:
    def _set_state(self, new_state):
        self._state = new_state
        self._update_status_indicator()

    def _update_status_indicator(self):
        # ... logic to update UI based on self._state ...
        # e.g., if self._state == AppState.RECORDING: update to "Recording..."

    def _start_recording(self):
        self._set_state(AppState.RECORDING)
        # ... recording logic ...

    def _stop_recording(self):
        self._set_state(AppState.TRANSCRIBING)
        # ... stop logic ...

    def _transcribe_and_inject(self, waveform):
        try:
            # ... transcription logic ...
        finally:
            if not self._recording:
                self._set_state(AppState.READY)
Suggestion importance[1-10]: 6

__

Why: The suggestion correctly identifies tight coupling between business logic and UI updates, and proposing a state management pattern is a valid architectural improvement for maintainability, though not a critical fix.

Low
General
Clear status indicator after exit

In the run method, set self.status_indicator to None after the with block to
clear the stale reference to the exited status object.

src/chirp/main.py [99-101]

 with self.console.status("Ready", spinner="dots") as status:
     self.status_indicator = status
     self.keyboard.wait()
+self.status_indicator = None
  • Apply / Chat
Suggestion importance[1-10]: 5

__

Why: The suggestion correctly points out that self.status_indicator holds a stale reference after its context manager exits, and clearing it improves the program's state management and robustness.

Low
  • More

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant