Closed
Conversation
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
|
Caution Review failedPull request was closed or merged during review No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughTTS機能をGeminiからNeural2に移行し、IPA音声ガイド対応を追加。GraphQLスキーマにnameIpaフィールドを追加、TTS設定のデフォルト値を更新、音声選択UI削除、音素ラッピング機能実装。翻訳キーとテスト用フィクスチャも更新。 Changes
Sequence Diagram(s)sequenceDiagram
participant Client as クライアント
participant Auth as 認証サービス
participant Cache as キャッシュ層
participant Neural2 as Neural2 TTS API
Client->>Client: 入力検証（SSML、音声名）
Client->>Auth: getAccessToken() でトークン取得
Auth-->>Client: accessToken
Client->>Cache: キャッシュ確認（ハッシュペイロード）
alt キャッシュヒット
Cache-->>Client: キャッシュされた音声データ
else キャッシュミス
Client->>Neural2: synthesizeWithNeural2(accessToken, ssml, 音声名)
Neural2-->>Client: 合成済み音声
Client->>Cache: 結果をキャッシュに発行（並行実行）
end
Client-->>Client: 音声データを返す
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)
📝 Coding Plan
Comment |
TinyKitten
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
src/components/TuningSettings.tsx (1)
87-109: フロントエンドとバックエンドでデフォルトプロンプトが重複しています。
DEFAULT_JA_PROMPTとDEFAULT_EN_PROMPTはfunctions/src/funcs/tts.tsのJA_TTS_PROMPT/EN_TTS_PROMPTと同一内容です。将来の変更時に同期が取れなくなるリスクがあります。共有定数ファイルに切り出すか、サーバー側のみで管理してクライアントは空文字を送信する設計を検討してください。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/TuningSettings.tsx` around lines 87 - 109, DEFAULT_JA_PROMPT and DEFAULT_EN_PROMPT on the frontend duplicate JA_TTS_PROMPT and EN_TTS_PROMPT from the backend, causing maintenance drift; remove the duplicated constants from the React component (symbols DEFAULT_JA_PROMPT, DEFAULT_EN_PROMPT) and instead import the canonical prompts from a single shared module (create/export JA_TTS_PROMPT and EN_TTS_PROMPT in a shared constants file) or change the client to send empty strings and let the server (JA_TTS_PROMPT / EN_TTS_PROMPT) supply defaults; update usages in TuningSettings.tsx and any places that referenced the removed constants to use the shared exports or server-driven defaults.functions/src/funcs/tts.ts (1)
50-72: フロントエンドとのプロンプト重複（前述）。
JA_TTS_PROMPTとEN_TTS_PROMPTはsrc/components/TuningSettings.tsxのDEFAULT_JA_PROMPT/DEFAULT_EN_PROMPTと同一です。サーバー側を正とし、クライアントはカスタムプロンプト未設定時に空文字を送信する設計を検討してください。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@functions/src/funcs/tts.ts` around lines 50 - 72, JA_TTS_PROMPT and EN_TTS_PROMPT duplicate DEFAULT_JA_PROMPT/DEFAULT_EN_PROMPT in the frontend; remove the duplication by making the server the single source of truth: delete or stop using JA_TTS_PROMPT/EN_TTS_PROMPT in funcs/tts.ts and instead import the canonical prompt constants from a shared module (or export the server constants so they can be referenced), and update the frontend (TuningSettings.tsx) to send an empty string when the user has not set a custom prompt so the server-side DEFAULT_JA_PROMPT/DEFAULT_EN_PROMPT are used. Ensure references to JA_TTS_PROMPT, EN_TTS_PROMPT, DEFAULT_JA_PROMPT, and DEFAULT_EN_PROMPT are updated accordingly across code so only the server defaults remain authoritative.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@functions/src/funcs/tts.ts`:
- Around line 31-48: The implementation of getTtsConfig uses a TTL cache with
error fallback (checks ttsConfigCache + TTS_CONFIG_CACHE_TTL_MS and only returns
stale data on error) but the PR text calls it "stale-while-revalidate"; either
update the code to true stale-while-revalidate or change the docs—pick one. If
you choose to implement stale-while-revalidate: modify getTtsConfig to
immediately return ttsConfigCache.data when present (even if expired) and kick
off a background async refresh to re-fetch and update ttsConfigCache (ensure
errors in background refresh are caught and do not fail the immediate return).
If you choose to update docs: change the description to say "TTL cache with
error fallback (returns cached data on fetch error)" and mention ttsConfigCache
and TTS_CONFIG_CACHE_TTL_MS so behavior matches getTtsConfig.
In `@src/components/TuningSettings.tsx`:
- Around line 435-440: The input uses value={settings.ttsJaPrompt ||
DEFAULT_JA_PROMPT} which forces the default to reappear when a user clears the
field; change the component to render the actual stored value (e.g., use
settings.ttsJaPrompt ?? '' or an empty string fallback) and keep
DEFAULT_JA_PROMPT only as placeholder so the user can leave the field empty;
update handleJaPromptChange to store empty string (or null) when cleared and
update the save/submit logic in the same component so that an empty string is
converted to null/undefined (or a dedicated useServerDefault flag) before
sending to the server, allowing the server-side default to be used.
In `@src/utils/phoneme.ts`:
- Around line 1-8: The wrapPhoneme function is inserting raw nameRoman and
nameIpa into SSML which can produce invalid XML when values contain characters
like & < > " '; modify wrapPhoneme to XML-escape nameRoman for element text
content and XML-escape nameIpa for the ph attribute before building the string
(e.g., add or reuse a helper like escapeXml or escapeXmlAttr and call it inside
wrapPhoneme), ensure null/undefined handling remains the same, and return the
escaped phoneme string so SSML is always valid.
---
Nitpick comments:
In `@functions/src/funcs/tts.ts`:
- Around line 50-72: JA_TTS_PROMPT and EN_TTS_PROMPT duplicate
DEFAULT_JA_PROMPT/DEFAULT_EN_PROMPT in the frontend; remove the duplication by
making the server the single source of truth: delete or stop using
JA_TTS_PROMPT/EN_TTS_PROMPT in funcs/tts.ts and instead import the canonical
prompt constants from a shared module (or export the server constants so they
can be referenced), and update the frontend (TuningSettings.tsx) to send an
empty string when the user has not set a custom prompt so the server-side
DEFAULT_JA_PROMPT/DEFAULT_EN_PROMPT are used. Ensure references to
JA_TTS_PROMPT, EN_TTS_PROMPT, DEFAULT_JA_PROMPT, and DEFAULT_EN_PROMPT are
updated accordingly across code so only the server defaults remain
authoritative.
In `@src/components/TuningSettings.tsx`:
- Around line 87-109: DEFAULT_JA_PROMPT and DEFAULT_EN_PROMPT on the frontend
duplicate JA_TTS_PROMPT and EN_TTS_PROMPT from the backend, causing maintenance
drift; remove the duplicated constants from the React component (symbols
DEFAULT_JA_PROMPT, DEFAULT_EN_PROMPT) and instead import the canonical prompts
from a single shared module (create/export JA_TTS_PROMPT and EN_TTS_PROMPT in a
shared constants file) or change the client to send empty strings and let the
server (JA_TTS_PROMPT / EN_TTS_PROMPT) supply defaults; update usages in
TuningSettings.tsx and any places that referenced the removed constants to use
the shared exports or server-driven defaults.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 0891cc21-939d-478f-8b32-c59005c676ff
📒 Files selected for processing (21)
assets/translations/en.jsonassets/translations/ja.jsonfunctions/src/funcs/tts.tssrc/@types/graphql.d.tssrc/__fixtures__/line.tssrc/components/TuningSettings.tsxsrc/constants/asyncStorage.tssrc/constants/index.tssrc/constants/tts.tssrc/hooks/useGetStationsWithTermination.test.tsxsrc/hooks/useRefreshStation.test.tsxsrc/hooks/useTTS.tssrc/hooks/useTTSText.tssrc/hooks/useTransferLinesFromStation.test.tsxsrc/lib/graphql/queries.tssrc/store/atoms/tuning.tssrc/utils/__fixtures__/search.tssrc/utils/jr.tssrc/utils/phoneme.tssrc/utils/test/factories.tssrc/utils/ttsSpeechFetcher.ts
src/components/TuningSettings.tsx
Outdated
Comment on lines
+435
to
+440
| onChangeText={handleJaPromptChange} | ||
| value={settings.ttsJaPrompt || DEFAULT_JA_PROMPT} | ||
| placeholder={DEFAULT_JA_PROMPT} | ||
| placeholderTextColor="#999" | ||
| multiline | ||
| /> |
Contributor
There was a problem hiding this comment.
プロンプトを空にできないUXの問題。
settings.ttsJaPrompt || DEFAULT_JA_PROMPT の論理により、ユーザーがフィールドをクリアすると即座にデフォルトプロンプトが表示されます。サーバー側のデフォルトを使用したい場合、ユーザーは空欄にできません。
意図的な設計であれば問題ありませんが、「サーバーデフォルトを使う」オプションが必要な場合は検討してください。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/TuningSettings.tsx` around lines 435 - 440, The input uses
value={settings.ttsJaPrompt || DEFAULT_JA_PROMPT} which forces the default to
reappear when a user clears the field; change the component to render the actual
stored value (e.g., use settings.ttsJaPrompt ?? '' or an empty string fallback)
and keep DEFAULT_JA_PROMPT only as placeholder so the user can leave the field
empty; update handleJaPromptChange to store empty string (or null) when cleared
and update the save/submit logic in the same component so that an empty string
is converted to null/undefined (or a dedicated useServerDefault flag) before
sending to the server, allowing the server-side default to be used.
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check current state of the relevant lines in TuningSettings.tsx
grep -n "ttsJaPrompt\|ttsEnPrompt\|DEFAULT_JA_PROMPT\|DEFAULT_EN_PROMPT" src/components/TuningSettings.tsx
Length of output: 493
@TinyKitten 現在のブランチを確認しましたが、Line 436・455 に同じコードがまだ存在しています：
436: value={settings.ttsJaPrompt || DEFAULT_JA_PROMPT}
455: value={settings.ttsEnPrompt || DEFAULT_EN_PROMPT}
レビューコメントは現在のコードに対して有効です。ご確認いただけますか？
🧠 Learnings used
Learnt from: TinyKitten
Repo: TrainLCD/MobileApp PR: 5533
File: src/components/TuningSettings.tsx:175-195
Timestamp: 2026-03-07T12:02:14.532Z
Learning: In `src/components/TuningSettings.tsx` (TrainLCD/MobileApp), the TTS voice name picker (ttsEnVoiceName) in TuningSettings is an internal developer-facing feature. Adding a "not specified" reset option is intentionally omitted; do not flag the absence of a reset/unset option as an issue.
Learnt from: TinyKitten
Repo: TrainLCD/MobileApp PR: 5533
File: functions/src/funcs/tts.ts:241-260
Timestamp: 2026-03-07T12:01:15.763Z
Learning: In `functions/src/funcs/tts.ts` (TrainLCD/MobileApp), the Japanese TTS voice name (`jaVoiceName`) is intentionally hardcoded to `'ja-JP-Standard-B'` and is not configurable via Firestore config or request parameters. Only the English voice name is user-selectable. Do not flag this as a bug in future reviews.
Learnt from: TinyKitten
Repo: TrainLCD/MobileApp PR: 5533
File: src/hooks/useTTS.ts:83-95
Timestamp: 2026-03-07T04:42:17.516Z
Learning: In src/hooks/useTTS.ts (TrainLCD/MobileApp), the AsyncStorage hydration of ttsJaVoiceName/ttsEnVoiceName in a useEffect runs after mount, but the first speechWithText() call always requires an auth token fetch and a network call to the TTS API, which takes far longer than AsyncStorage.getItem. Therefore, the race condition where the first utterance uses the default voice practically never occurs and is not a concern worth addressing.
Learnt from: TinyKitten
Repo: TrainLCD/MobileApp PR: 4448
File: functions/src/funcs/tts.ts:214-232
Timestamp: 2025-08-11T20:23:23.071Z
Learning: PR `#4448` in TrainLCD/MobileApp is specifically about AI feedback summarization and labeling functionality, not about TTS (Text-to-Speech) improvements. The TTS code changes appear to be part of a modularization refactoring but are not the focus of this PR.
Learnt from: TinyKitten
Repo: TrainLCD/MobileApp PR: 5464
File: android/app/src/main/java/me/tinykitten/trainlcd/MainApplication.kt:33-37
Timestamp: 2026-02-27T07:30:06.268Z
Learning: TinyKitten prefers not to modify code taken from official templates (e.g., Expo, React Native) to maintain alignment with upstream and ease future updates.
Learnt from: TinyKitten
Repo: TrainLCD/MobileApp PR: 0
File: :0-0
Timestamp: 2025-08-11T21:13:39.083Z
Learning: TinyKitten prefers concise code reviews for the TrainLCD/MobileApp repository. Keep review comments brief and to the point.
Learnt from: TinyKitten
Repo: TrainLCD/MobileApp PR: 5277
File: src/components/NowHeader.tsx:182-193
Timestamp: 2026-02-14T00:19:25.989Z
Learning: In TrainLCD/MobileApp, Station objects are guaranteed to have non-null latitude and longitude. When reviewing code that uses Station.coordinates in TS/TSX files, assume valid values (no null/undefined checks needed for coordinates) and rely on type-safe access. This applies broadly to TypeScript files that interact with Station data (e.g., UI components, data layers); use a broad pattern to cover related files.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
ja-JP-Neural2-B、EN:en-US-Neural2-F）getTtsConfigにエラー時キャッシュフォールバックを導入wrapPhonemeにXMLエスケープを追加しSSMLインジェクションを防止Test plan
npm run typecheckパスnpm run lintパスnpm test全137 suites / 1281 tests パス🤖 Generated with Claude Code
Summary by CodeRabbit
リリースノート
新機能
改善