fix: use shutil.which so play() and stream() work on Windows

[Dhevenddra]

Problem

_is_installed() in utils/play.py and utils/stream.py checks for a binary by shelling out to which:

try:
    subprocess.run(["which", command], capture_output=True, check=True)
    return True
except subprocess.CalledProcessError:
    return False

Windows has no which. Trying to run it raises FileNotFoundError, which is not a CalledProcessError, so it escapes the handler and propagates to the caller.

The result is that play() and stream() both fail on Windows regardless of whether ffplay or mpv is installed:

>>> from fishaudio import stream
>>> stream(iter([b"..."]))
  File "src/fishaudio/utils/stream.py", line 49, in stream
    if not _is_installed("mpv"):
FileNotFoundError: [WinError 2] The system cannot find the file specified

Reproduced on Windows 11 with Python 3.9.25, against main. Both helpers raise, and both play(b"audio") and stream(iter([b"audio"])) surface the WinError rather than the DependencyError they are supposed to raise.

That error message is the part that stings: it is the one telling Windows users where to get the tool.

raise DependencyError(
    "mpv",
    "brew install mpv  # macOS\n"
    "sudo apt install mpv  # Linux\n"
    "https://mpv.io/installation/  # Windows",   # unreachable on Windows
)

The same helper is duplicated verbatim in both modules, so both are affected.

Fix

Use shutil.which(), the standard-library equivalent. It works on every supported platform and does not spawn a process just to test for one.

def _is_installed(command: str) -> bool:
    """Check if a command is available in PATH."""
    return shutil.which(command) is not None

This also removes a latent failure on Linux and macOS: minimal container images often ship without which, and those would have hit the same unhandled FileNotFoundError.

Tests

The four existing tests mocked subprocess.run to simulate which succeeding or exiting non-zero. Neither is what Windows does, which is why CI stayed green: the workflow only runs ubuntu-latest, where which exists. They now patch shutil.which instead.

Added TestIsInstalled covering both modules:

• a missing binary is reported absent rather than raising
• the lookup does not shell out (subprocess.run is patched to fail the test if called)
• with no which binary available and the tool absent, play() and stream() raise DependencyError as documented

Verified the tests actually catch this: with the two _is_installed bodies reverted and the tests left alone, 9 tests fail, including all 6 new ones, with the original FileNotFoundError.

poe lint, poe format, poe typecheck all pass. pytest tests/unit/ is 163 passed, up from 157.

Summary by CodeRabbit

• Bug Fixes

  • Improved detection of required playback and streaming commands across supported environments.
  • Prevented unnecessary shell invocation when checking command availability.
  • Added clearer handling when required commands are unavailable, including Windows-like environments.

• Tests

  • Expanded coverage for missing commands and platform-specific dependency errors.

[coderabbitai]

📝 Walkthrough

Walkthrough

The PR replaces subprocess-based which command lookups with shutil.which in the play and stream utility modules to check external command availability. Unit tests are updated to mock shutil.which directly and add parameterized coverage for missing commands and Windows-like error handling.

Changes

Command Availability Check Migration

Layer / File(s)	Summary
Playback and stream command detection src/fishaudio/utils/play.py, src/fishaudio/utils/stream.py	_is_installed in both modules now uses shutil.which instead of running which through subprocess and catching CalledProcessError.
Test updates for shutil.which mocking tests/unit/test_utils.py	Tests import fishaudio.utils.play and fishaudio.utils.stream explicitly and mock shutil.which instead of subprocess.run. New parameterized tests cover missing-command detection, prohibition of shelling out, and DependencyError on Windows-like FileNotFoundError.

Estimated code review effort: 2 (Simple) | ~10 minutes

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the main cross-platform change: replacing the external which command with shutil.which.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@tests/unit/test_utils.py`:
- Around line 52-53: Replace the two assertions checking mock_run.call_count and
the "ffplay" string presence with a single assert_called_once_with invocation on
mock_run. This will verify that the mock was called exactly once with the
expected ffplay command and subprocess options, eliminating both the risk of
multiple calls and false positives from "ffplay" appearing elsewhere in the
arguments.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: edeebe02-8b6b-437c-acd0-80a668c0c24e

📥 Commits

Reviewing files that changed from the base of the PR and between 999c59d and 7dbe656.

📒 Files selected for processing (3)

• src/fishaudio/utils/play.py
• src/fishaudio/utils/stream.py
• tests/unit/test_utils.py

[coderabbitai]

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Assert the exact ffplay invocation.

Lines 52-53 allow extra calls and can pass when another argument contains "ffplay". Use assert_called_once_with to verify the command and subprocess options.

Proposed assertion

-        assert mock_run.call_count >= 1
-        assert "ffplay" in str(mock_run.call_args_list)
+        mock_run.assert_called_once_with(
+            ["ffplay", "-autoexit", "-", "-nodisp"],
+            input=b"fake audio",
+            capture_output=True,
+            check=True,
+        )

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	assert mock_run.call_count >= 1
	assert "ffplay" in str(mock_run.call_args_list)
	mock_run.assert_called_once_with(
	["ffplay", "-autoexit", "-", "-nodisp"],
	input=b"fake audio",
	capture_output=True,
	check=True,
	)

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/unit/test_utils.py` around lines 52 - 53, Replace the two assertions
checking mock_run.call_count and the "ffplay" string presence with a single
assert_called_once_with invocation on mock_run. This will verify that the mock
was called exactly once with the expected ffplay command and subprocess options,
eliminating both the risk of multiple calls and false positives from "ffplay"
appearing elsewhere in the arguments.

[Dhevenddra]

Good call, that assertion was loose in both directions. Replaced it with assert_called_once_with pinning the exact ffplay argv, input bytes and subprocess options, so an extra call or an incidental ffplay substring can no longer pass.