Skip to content

Conversation

@twangodev
Copy link
Collaborator

@twangodev twangodev commented Dec 9, 2025

Summary by CodeRabbit

  • Updates

    • HTTP and WebSocket requests now include a User-Agent header for clearer request metadata.
  • Refactor

    • Header construction exposed and enhanced to accept and merge additional custom headers while preserving authentication.
  • Tests

    • Unit tests updated for the new header behavior; integration WebSocket tests increased retry attempts and inter-iteration delays for stability.

✏️ Tip: You can customize this high-level summary in your review settings.

Copilot AI review requested due to automatic review settings December 9, 2025 21:19
@coderabbitai
Copy link

coderabbitai bot commented Dec 9, 2025

Walkthrough

Renames the private header builder to a public get_headers(additional_headers=None); updates call sites to use it; adds a User-Agent header ("fish-audio/python/legacy") to HTTP clients and WebSocket setup; updates tests/mocks to use get_headers; increases WebSocket integration test retries and delays.

Changes

Cohort / File(s) Summary
HTTP client init updates
src/fish_audio_sdk/io.py, src/fish_audio_sdk/websocket.py
Adds a User-Agent header ("fish-audio/python/legacy") to sync and async HTTP clients during initialization, alongside the existing Authorization header.
Client wrapper header API
src/fishaudio/core/client_wrapper.py
Replaces private _get_headers() with public get_headers(additional_headers: Optional[Dict[str, str]] = None) that returns merged headers; updates internal call sites to call the new method and accepts extra headers.
TTS WebSocket header usage
src/fishaudio/resources/tts.py
Replaces inline header dicts with get_headers({"model": model}) for TTS realtime WebSocket setup to forward Authorization, User-Agent, and model headers.
Unit test updates
tests/unit/test_core.py, tests/unit/test_tts_realtime.py
Tests and mocks updated to call get_headers() and return merged headers including Authorization and User-Agent (and to accept additional headers).
Integration test timing & retries
tests/integration/test_tts_websocket_integration.py
Increases flaky-test retry count (from 2 → 9) for two websocket tests and increases inter-iteration delays from 0.3s1.0s to reduce SSL-related flakiness.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20–30 minutes

  • Areas to inspect:
    • src/fishaudio/core/client_wrapper.py — header merge logic, handling of additional_headers, and public API behavior.
    • src/fish_audio_sdk/io.py and src/fish_audio_sdk/websocket.py — verify User-Agent insertion and that Authorization and other headers are not unintentionally overridden.
    • src/fishaudio/resources/tts.py — ensure model header is forwarded properly via get_headers.
    • Tests (tests/unit/*, tests/integration/*) — confirm mocks match new signature and that increased retries/delays are acceptable.

Possibly related PRs

Poem

🐇 I hopped through headers, soft and steady,
Tucked in a tag to keep requests ready,
Authorization stays, my badge in tow,
Tiny User‑Agent — now watch it go! 🥕

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 53.85% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly describes the main objective of the changeset: adding consistent User-Agent headers across HTTP and WebSocket clients throughout the codebase.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/user-agents

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 9412f24 and beecdf8.

📒 Files selected for processing (1)
  • tests/integration/test_tts_websocket_integration.py (4 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • tests/integration/test_tts_websocket_integration.py
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: Integration Tests
  • GitHub Check: Integration 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

Comment @coderabbitai help to get the list of available commands and usage tips.

@codecov
Copy link

codecov bot commented Dec 9, 2025

Codecov Report

❌ Patch coverage is 50.00000% with 1 line in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
src/fishaudio/core/client_wrapper.py 50.00% 1 Missing ⚠️
Files with missing lines Coverage Δ
src/fishaudio/resources/tts.py 94.78% <ø> (ø)
src/fishaudio/core/client_wrapper.py 48.88% <50.00%> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR ensures consistent User-Agent headers across both HTTP and WebSocket connections in the Fish Audio SDK.

  • Renamed _get_headers to get_headers (public API) in the core client wrapper to enable reuse for WebSocket connections
  • Updated TTS WebSocket connections to use the centralized header builder instead of manually constructing headers
  • Added User-Agent headers to the legacy SDK's HTTP clients for consistency

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.

Show a summary per file
File Description
src/fishaudio/core/client_wrapper.py Changed _get_headers to public get_headers method and updated docstring to mention user agent
src/fishaudio/resources/tts.py Updated WebSocket connections to use get_headers() instead of manually building authorization headers
src/fish_audio_sdk/websocket.py Added User-Agent header to WebSocket session client initialization (both sync and async)
src/fish_audio_sdk/io.py Added User-Agent header to HTTP client initialization (both sync and async)
tests/unit/test_core.py Updated tests to call the renamed public get_headers method
tests/unit/test_tts_realtime.py Added get_headers mock implementation to test fixtures

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (4)
src/fish_audio_sdk/websocket.py (1)

27-30: User-Agent and auth defaults on legacy WebSocket sessions

Initializing the sync and async HTTP clients with Authorization plus "fish-audio/python/legacy" makes legacy WebSocket calls consistent with the rest of the legacy SDK. To avoid future drift with RemoteCall in io.py, consider extracting this legacy User-Agent string into a shared constant reused in both places.

Also applies to: 103-106

src/fish_audio_sdk/io.py (1)

37-40: Legacy IO clients now emit a consistent User-Agent

Adding Authorization and "fish-audio/python/legacy" at async and sync client initialization aligns HTTP calls with the legacy WebSocket sessions. As with websocket.py, you may want to centralize this UA value in a single constant to keep the legacy identifier consistent if it ever changes.

Also applies to: 47-50

tests/unit/test_core.py (1)

112-123: Tests correctly cover the new get_headers API

These tests validate that both wrappers expose get_headers() with the expected Authorization and a User-Agent header, and that additional headers are merged in for the sync wrapper. If you want perfect symmetry, you could add a small AsyncClientWrapper.get_headers test with additional headers too, but what’s here is already sufficient for this change.

Also applies to: 140-144

src/fishaudio/core/client_wrapper.py (1)

63-73: Centralized header builder and merge logic look sound

get_headers now standardizes the Authorization and fish-audio/python/{__version__} User-Agent, and _prepare_request_kwargs sensibly layers RequestOptions.additional_headers and any explicit headers kwarg on top. To reduce duplication, you could delegate the options merge into get_headers itself, e.g. headers = self.get_headers(request_options.additional_headers if request_options else None), before applying any per-call kwargs["headers"].

Also applies to: 80-83

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a56b90c and 267f88e.

📒 Files selected for processing (6)
  • src/fish_audio_sdk/io.py (1 hunks)
  • src/fish_audio_sdk/websocket.py (2 hunks)
  • src/fishaudio/core/client_wrapper.py (4 hunks)
  • src/fishaudio/resources/tts.py (2 hunks)
  • tests/unit/test_core.py (2 hunks)
  • tests/unit/test_tts_realtime.py (2 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
tests/unit/test_tts_realtime.py (2)
src/fishaudio/core/client_wrapper.py (1)
  • get_headers (63-73)
tests/unit/conftest.py (1)
  • mock_api_key (9-11)
src/fishaudio/resources/tts.py (1)
src/fishaudio/core/client_wrapper.py (1)
  • get_headers (63-73)
tests/unit/test_core.py (1)
src/fishaudio/core/client_wrapper.py (2)
  • get_headers (63-73)
  • ClientWrapper (97-166)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: Integration Tests
  • GitHub Check: Agent
  • GitHub Check: Integration Tests
🔇 Additional comments (3)
tests/unit/test_tts_realtime.py (1)

19-24: Mocked get_headers correctly mirrors client wrapper behavior

The fixtures now return Authorization, a test User-Agent, and merged additional headers, which matches how ClientWrapper.get_headers is used in the TTS WebSocket paths and keeps these tests aligned with the public API.

Also applies to: 35-40

src/fishaudio/resources/tts.py (1)

332-333: WebSocket TTS now uses centralized header construction

Using self._client.get_headers({"model": model}) for both sync and async stream_websocket ensures the model header is combined with the standard Authorization and User-Agent headers from the client wrapper, keeping WebSocket behavior consistent with the rest of the HTTP stack and the updated tests.

Also applies to: 630-631

src/fishaudio/core/client_wrapper.py (1)

116-116: Client initialization now includes default auth and User-Agent

Passing headers=self.get_headers() when constructing the sync and async HTTP clients ensures that even direct uses of .client start from the same Authorization and User-Agent defaults as higher-level wrapper methods and the WebSocket TTS entry points.

Also applies to: 188-188

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 267f88e and 4184b01.

📒 Files selected for processing (1)
  • tests/integration/test_tts_websocket_integration.py (1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: Integration Tests
  • GitHub Check: Integration Tests

@twangodev twangodev merged commit 203ee04 into main Dec 9, 2025
23 of 24 checks passed
@twangodev twangodev deleted the fix/user-agents branch December 9, 2025 22:32
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.

2 participants