Conversation
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 📝 WalkthroughWalkthroughTTS再生実装を大幅にリファクタし、Expo Audio直接利用から抽象化された Changes
Sequence Diagram(s)sequenceDiagram
participant Hook as useTTS Hook
participant Fetcher as ttsSpeechFetcher
participant FS as FileSystem
participant PlayerMgr as ttsAudioPlayer
participant AudioAPI as Expo Audio API
Hook->>Fetcher: fetchSpeechAudio(textJa, textEn, apiUrl, token)
Fetcher->>Fetcher: SSML組立・POSTボディ作成
Fetcher->>AudioAPI: POST to TTS API (ネットワーク)
Fetcher-->>Fetcher: JSONレスポンス受領・検証
Fetcher->>FS: write jaAudioContent -> pathJa
Fetcher->>FS: write enAudioContent -> pathEn
Fetcher-->>Hook: return {id, pathJa, pathEn}
Hook->>PlayerMgr: playAudio(pathJa, onFinish, onError)
PlayerMgr->>AudioAPI: createAudioPlayer(pathJa)
AudioAPI-->>PlayerMgr: playback status updates
PlayerMgr-->>Hook: onFinish()
Note over Hook: 必要なら EN_PLAYBACK_DELAY_MS 待機
Hook->>PlayerMgr: playAudio(pathEn, onFinish, onError)
PlayerMgr->>AudioAPI: createAudioPlayer(pathEn)
AudioAPI-->>PlayerMgr: playback status updates
PlayerMgr-->>Hook: onFinish()
Hook->>PlayerMgr: cleanupAllPlayers()
PlayerMgr->>AudioAPI: pause/remove listeners / remove players
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Comment |
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/utils/base64ToUint8Array.test.ts`:
- Around line 3-43: Add an afterEach hook that calls jest.clearAllMocks() in
this test file to ensure mocks are reset between tests: add afterEach(() => {
jest.clearAllMocks(); }) near the top of the describe block (or immediately
after it) so all tests that call base64ToUint8Array run with cleared mocks;
reference the existing describe('base64ToUint8Array', ...) and the
base64ToUint8Array tests when inserting the hook.
In `@src/utils/ttsAudioPlayer.test.ts`:
- Around line 87-91: The test setup currently calls jest.clearAllMocks() only in
the beforeEach block inside the describe('playAudio') suite; change this to
follow guidelines by adding an afterEach hook that calls jest.clearAllMocks()
(or move the existing jest.clearAllMocks() into an afterEach) so mocks are
cleared after each test; update the describe('playAudio') block to include
afterEach(() => { jest.clearAllMocks(); }) referencing the existing beforeEach
and the test suite name to locate the change.
In `@src/utils/ttsAudioPlayer.ts`:
- Around line 1-3: 現在のインポートと依存 (AudioPlayer 型 と createAudioPlayer の使用) は
expo-audio に依存しているため、expo-av の Sound ベース API に置き換えてください: import を 'expo-audio'
から削除して 'expo-av' の Audio.Sound を使い、createAudioPlayer と AudioPlayer
型を参照している箇所をすべて Audio.Sound
インスタンスの生成・再生制御(loadAsync/playAsync/pauseAsync/unloadAsync/setOnPlaybackStatusUpdate
など)に書き換え、再生開始・停止・シーク・ボリューム・ループ状態の管理ロジックを Sound のメソッドに移植して不要な型や createAudioPlayer
呼び出しを削除してください。エラーハンドリングと後始末(unloadAsync)を追加し、既存コードの関数名(createAudioPlayer、AudioPlayer)を参照して実装箇所を特定して置換してください。
In `@src/utils/ttsSpeechFetcher.test.ts`:
- Around line 29-33: The test suite for fetchSpeechAudio currently calls
jest.clearAllMocks() in beforeEach; add an afterEach(() => {
jest.clearAllMocks(); }) (or move the existing call) inside the
describe('fetchSpeechAudio', ...) block so mocks are cleared after each test as
required by the guideline; keep any existing beforeEach setup intact and ensure
jest.mock(...) usages remain for network/Firebase layers.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/utils/ttsSpeechFetcher.test.ts (1)
3-20:src/utils/test/に共通モック関数を抽出
expo/fetchとexpo-file-systemのモックがsrc/hooks/useTTS.test.tsとsrc/utils/ttsSpeechFetcher.test.tsで重複しています。コーディングガイドラインに従い、src/utils/test/にモック設定を集約する共通ヘルパーを作成し、両テストファイルで再利用してください。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/utils/ttsSpeechFetcher.test.ts` around lines 3 - 20, Extract the duplicated Jest mocks for expo/fetch and expo-file-system into a shared helper (e.g., export a setupTestMocks function) under src/utils/test/, then replace the inline mocks in both src/utils/ttsSpeechFetcher.test.ts and src/hooks/useTTS.test.ts with a single import and call to that helper; the helper should define mockFetch and jest.mock('expo/fetch', ...) and jest.mock('expo-file-system', ...) with the same Paths and File class behavior so tests continue to use the same mockFetch, Paths.cache and File(uri) semantics while removing the duplicated mock blocks from each test file.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/utils/ttsSpeechFetcher.test.ts`:
- Around line 3-20: Extract the duplicated Jest mocks for expo/fetch and
expo-file-system into a shared helper (e.g., export a setupTestMocks function)
under src/utils/test/, then replace the inline mocks in both
src/utils/ttsSpeechFetcher.test.ts and src/hooks/useTTS.test.ts with a single
import and call to that helper; the helper should define mockFetch and
jest.mock('expo/fetch', ...) and jest.mock('expo-file-system', ...) with the
same Paths and File class behavior so tests continue to use the same mockFetch,
Paths.cache and File(uri) semantics while removing the duplicated mock blocks
from each test file.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 noreply@anthropic.com
Summary by CodeRabbit