Skip to content

[CLI] Audio playback error handling - #204

Merged
majunze2001 merged 1 commit into
masterfrom
jm-audio-playback
Apr 3, 2026
Merged

majunze2001 merged 1 commit into
masterfrom
jm-audio-playback

Conversation

@majunze2001

Copy link
Copy Markdown
Collaborator

No description provided.

@majunze2001
majunze2001 merged commit 1609e88 into master Apr 3, 2026
2 checks passed
@majunze2001
majunze2001 deleted the jm-audio-playback branch April 3, 2026 17:51

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request refactors the audio streaming logic in the CLI to allow the application to continue running even if audio playback fails or is unavailable, rather than exiting immediately. It introduces the use of ExitStack to manage resources and adds a fallback state for when playback is disabled. I have included a suggestion to use the local console object for printing error panels to ensure better synchronization with the active Live display.

Comment on lines +660 to +666
rich.print(
Panel(
f"Audio playback unavailable on this machine ({e}); continuing without playback.",
style="yellow",
expand=False,
)
)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

While rich.print is used throughout the file, it is generally safer to use live.console.print or the local console variable when a Live display is active. This ensures that the output is correctly synchronized with the live display and avoids potential layout issues in some terminal environments. Additionally, using the console object defined at line 625 would be more consistent with its usage at line 705.

Suggested change
rich.print(
Panel(
f"Audio playback unavailable on this machine ({e}); continuing without playback.",
style="yellow",
expand=False,
)
)
console.print(
Panel(
f"Audio playback unavailable on this machine ({e}); continuing without playback.",
style="yellow",
expand=False,
)
)

jnigrelli pushed a commit to jnigrelli/cornserve that referenced this pull request Apr 19, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant