Merged
Conversation
Contributor
Walkthrough朕の勅により、本変更は Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![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: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
functions/src/funcs/tts.ts (3)
201-227: 朕、部分キャッシュヒット時にも即座に返却せよ下記の箇所では、現在「両方(pathJa・pathEn)が揃っていなければ合成へフォールバック」しており、PR題意「TTSで片方処理成功すれば値を返す」に合致せぬ。
対象箇所（functions/src/funcs/tts.ts:190–227相当）
- if (snapshot.exists) { - const data = snapshot.data() ?? {}; - const pathJa = data?.pathJa; - const pathEn = data?.pathEn; - if (typeof pathJa === 'string' && typeof pathEn === 'string') { - try { - const [jaAudioData, enAudioData] = await Promise.all([ - storage.bucket().file(pathJa).download(), - storage.bucket().file(pathEn).download(), - ]); - const jaAudioContent = jaAudioData?.[0]?.toString('base64') ?? null; - const enAudioContent = enAudioData?.[0]?.toString('base64') ?? null; - if (jaAudioContent && enAudioContent) { - return { id, jaAudioContent, enAudioContent }; - } - } catch (e) { - console.warn( - 'Cache hit but download failed. Falling back to synthesis.', - e - ); - } - } else { - console.warn( - 'Cache doc missing pathJa/pathEn. Falling back to synthesis.' - ); - } - } + if (snapshot.exists) { + const data = snapshot.data() ?? {}; + const pathJa = data?.pathJa as string | undefined; + const pathEn = data?.pathEn as string | undefined; + const wantJa = typeof pathJa === 'string'; + const wantEn = typeof pathEn === 'string'; + if (wantJa || wantEn) { + try { + const [jaAudioData, enAudioData] = await Promise.all([ + wantJa + ? storage.bucket().file(pathJa!).download().catch(() => null) + : Promise.resolve(null), + wantEn + ? storage.bucket().file(pathEn!).download().catch(() => null) + : Promise.resolve(null), + ]); + const jaAudioContent = jaAudioData?.[0]?.toString('base64') ?? null; + const enAudioContent = enAudioData?.[0]?.toString('base64') ?? null; + if (jaAudioContent || enAudioContent) { + return { id, jaAudioContent, enAudioContent }; + } + } catch (e) { + console.warn( + 'Cache hit but download失敗。合成へフォールバックせよ。', + e + ); + } + } + }この修正により「片方だけ取得できれば即時返却」が達成されよう。
267-289: TTS 同時合成呼び出しを Promise.allSettled に置換し、HTTPステータスと audioContent を検査せよ朕、現状の
Promise.all実装では一方のフェッチ失敗で全体が例外となり、片側成功時のオーディオ再利用が不可能と判断せり。かつレスポンスのokフラグやaudioContent存在チェックも未実装なり。よって、以下を必ず実装せよ。対象ファイル・箇所
- functions/src/funcs/tts.ts
try { … } catch { … }ブロック内（およそ 250–360 行目）命ずる修正点
Promise.all→Promise.allSettledに変更し、それぞれの結果をステータス別に取得すること- 各フェッチ結果に対し
response.okとパース後のaudioContent存在を検査し、成功分のみを変数へセット- 両言語とも失敗または
audioContent欠如時のみ例外を投げ、それ以外は部分成功でも先へ進め- キャッシュ公開および戻り値の構築は、新たに得られた
jaAudioContent／enAudioContentを利用すること参考差分例（要約）:
- const [jaRes, enRes] = await Promise.all([fetch(…), fetch(…)]); - const [{ audioContent: jaAudioContent }, { audioContent: enAudioContent }] = - await Promise.all([jaRes.json(), enRes.json()]); + const [jaFetch, enFetch] = await Promise.allSettled([fetch(…), fetch(…)]); + const [jaJson, enJson] = await Promise.all([ + jaFetch.status === 'fulfilled' ? jaFetch.value.json().catch(() => null) : Promise.resolve(null), + enFetch.status === 'fulfilled' ? enFetch.value.json().catch(() => null) : Promise.resolve(null), + ]); + const jaAudioContent = + jaFetch.status === 'fulfilled' && jaFetch.value.ok && jaJson?.audioContent + ? jaJson.audioContent + : null; + const enAudioContent = + enFetch.status === 'fulfilled' && enFetch.value.ok && enJson?.audioContent + ? enJson.audioContent + : null; + if (!jaAudioContent && !enAudioContent) { + throw new Error('Both JA/EN synthesis failed or returned no audioContent.'); + }朕、上記の必須リファクタリングを命ずる。
290-311: 必須改修：Pub/Sub ペイロードの部分送信対応およびサブスクライバの部分処理ロジックを導入せよ
朕、現状の実装では Producer 側が undefined 含む両言語音声を送信し、Subscriber 側では片方でも欠如すると処理を打ち切るがゆえ、一方のみキャッシュ可能とする要件を満たさぬ。以下必ず改修せよ。・Producer 側（functions/src/funcs/tts.ts, L290付近）
jaAudioContent／enAudioContentをスプレッド条件付きでのみ含めるよう差し替え。- 差分例:
- cacheTopic.publishMessage({ - json: { - id, - jaAudioContent, - enAudioContent, - ssmlJa, - ssmlEn, - voiceJa: jaVoiceName, - voiceEn: enVoiceName, - audioEncoding, - volumeGainDb, - effectsProfileId, - apiVersion, - }, - }) + const payload = { + id, + ssmlJa, + ssmlEn, + voiceJa: jaVoiceName, + voiceEn: enVoiceName, + audioEncoding, + volumeGainDb, + effectsProfileId, + apiVersion, + ...(jaAudioContent ? { jaAudioContent } : {}), + ...(enAudioContent ? { enAudioContent } : {}), + }; + cacheTopic.publishMessage({ json: payload })・Subscriber 側（functions/src/funcs/ttsCachePubSub.ts, L19付近）
- 現状の
if (!id || !jaAudioContent || !enAudioContent) { … return; }を廃し、存在する側のみ保存する分岐処理を追加せよ。- 例:
if (!id) { console.error('Invalid payload for tts-cache: missing id'); return; } if (jaAudioContent) { // ja 保存ロジック } if (enAudioContent) { // en 保存ロジック }・マイグレーション計画
- まず Subscriber の部分対応実装をデプロイし、既存の完全ペイロードでも正常動作を確認
- 次に Producer の条件付きペイロード送信をリリース
- 両版が安定した後、不要チェック／コードを削除
これらの改修を完了せよ。朕、異存なし。
🧹 Nitpick comments (1)
functions/src/funcs/tts.ts (1)
287-289: API エラー JSON（error.details 等）をaudioContentと同列に扱うな。現行の分配代入はステータス未確認のまま
.json()を剥がす。Google TTS は 4xx/5xx 時にerrorオブジェクトを返すため、そのままではjaAudioContent/enAudioContentがundefinedのまま進行し取り扱いが曖昧。上の allSettled 案でres.okを踏んでからaudioContentを採用するが良い。
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
functions/src/funcs/tts.ts(1 hunks)
🔇 Additional comments (1)
functions/src/funcs/tts.ts (1)
153-158: ssmlEn の空チェックは良い。だが入力自体が英語案内を含む場合の “jo” 誤爆に留意せよ。上段の “jo” 制限を入れれば十分である。朕はここを批准する。
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 by CodeRabbit