StationとLineにnameIpaフィールドを追加しTTS IPA置換をアプリ側に移行#5524
Merged
TinyKitten merged 4 commits intodevfrom Mar 5, 2026
Merged
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>
Contributor
|
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 (2)
📝 WalkthroughWalkthroughGraphQL フラグメントに Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Client
participant Hook as useTTSText / useBusTTSText
participant Data as GraphQL / Fixtures
participant Wrap as wrapIpa (src/utils/ssml.ts)
participant TTSFunc as functions/src/funcs/tts.ts
participant Legacy as applyLegacyIpaReplacements
Client->>Hook: 表示/読み上げデータ要求
Hook->>Data: `nameRoman` / `nameIpa` を取得
Hook->>Wrap: wrapIpa(nameRoman, nameIpa) を適用
Wrap-->>Hook: SSML 断片（場合により `<phoneme>` 含む）
Hook->>TTSFunc: 完成した SSML を送信
TTSFunc->>TTSFunc: 英語 SSML に `<phoneme>` があるか判定
alt `<phoneme>` が無い
TTSFunc->>Legacy: applyLegacyIpaReplacements(text)
Legacy-->>TTSFunc: 置換済 SSML
end
TTSFunc-->>Client: 合成用 SSML を返す
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
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 |
![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: 2
🧹 Nitpick comments (1)
src/utils/ssml.ts (1)
6-17:nameIpaに含まれる可能性のある特殊文字のエスケープを検討してください。
nameIpaの値がph属性内に直接埋め込まれていますが、ダブルクォート (") や<、>、&などのXML特殊文字が含まれる場合、SSMLの構文が壊れる可能性があります。BFFから返されるデータが信頼できる場合は問題ありませんが、防御的にエスケープを追加することを検討してください。
♻️ エスケープ処理の追加案
+const escapeXmlAttr = (str: string): string => + str.replace(/&/g, '&').replace(/"/g, '"').replace(/</g, '<').replace(/>/g, '>'); + export const wrapIpa = ( nameRoman: string | null | undefined, nameIpa: string | null | undefined ): string => { if (!nameRoman) { return ''; } if (!nameIpa) { return nameRoman; } - return `<phoneme alphabet="ipa" ph="${nameIpa}">${nameRoman}</phoneme>`; + return `<phoneme alphabet="ipa" ph="${escapeXmlAttr(nameIpa)}">${nameRoman}</phoneme>`; };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/utils/ssml.ts` around lines 6 - 17, The wrapIpa function currently injects nameIpa directly into the ph attribute which can break SSML if nameIpa contains XML-special characters; update wrapIpa to escape XML characters (&, <, >, ", and ') in nameIpa before embedding it into the ph attribute (and also escape nameRoman used as element content), preserving the existing null/undefined handling and return values; reference the wrapIpa function and the nameIpa/nameRoman parameters and replace those characters with their XML entities (&, <, >, ", ') so the returned string is always valid SSML.
🤖 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`:
- Line 41: The regex used in the replace call
(.replace(/[！-／：-＠［-｀｛-～、-〜”'・]+/g, ' ')) is currently stripping a half-width
apostrophe (') which can break words like O'Hare or SSML with single quotes;
update that character class to remove the ASCII apostrophe so it no longer
matches (e.g., .replace(/[！-／：-＠［-｀｛-～、-〜”・]+/g, ' ')), and if you intended to
target the fullwidth apostrophe include the explicit fullwidth character U+FF07
(＇) instead; modify the replace expression in funcs/tts.ts (the .replace(...)
call) accordingly.
In `@src/hooks/useTTSText.ts`:
- Around line 56-71: The wrapIpa usage in connectedLines and transferLines can
inject raw XML-special characters into SSML via nameRoman/nameIpa; update
wrapIpa (in src/utils/ssml.ts) to XML-escape input strings (at least &, <, >, ",
') before composing/returning the SSML fragment so returned values are safe to
embed, ensure the escaping is idempotent (don’t double-escape already-escaped
input) and keep existing wrapIpa signature so connectedLines and transferLines
code (which calls wrapIpa) requires no other changes.
---
Nitpick comments:
In `@src/utils/ssml.ts`:
- Around line 6-17: The wrapIpa function currently injects nameIpa directly into
the ph attribute which can break SSML if nameIpa contains XML-special
characters; update wrapIpa to escape XML characters (&, <, >, ", and ') in
nameIpa before embedding it into the ph attribute (and also escape nameRoman
used as element content), preserving the existing null/undefined handling and
return values; reference the wrapIpa function and the nameIpa/nameRoman
parameters and replace those characters with their XML entities (&, <,
>, ", ') so the returned string is always valid SSML.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b0c11e12-5a47-4bef-baf5-ecb5215ad18c
📒 Files selected for processing (13)
functions/src/funcs/tts.tssrc/@types/graphql.d.tssrc/__fixtures__/line.tssrc/hooks/useBusTTSText.tssrc/hooks/useGetStationsWithTermination.test.tsxsrc/hooks/useRefreshStation.test.tsxsrc/hooks/useTTSText.tssrc/hooks/useTransferLinesFromStation.test.tsxsrc/lib/graphql/queries.tssrc/utils/__fixtures__/search.tssrc/utils/jr.tssrc/utils/ssml.tssrc/utils/test/factories.ts
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
There was a problem hiding this comment.
♻️ Duplicate comments (1)
functions/src/funcs/tts.ts (1)
42-42:⚠️ Potential issue | 🟠 MajorLine 42: 半角アポストロフィの除去はやめてください。
'を削ると、<phoneme ... ph="...">の値やO'Hareのような表記を壊します。過去指摘と同じ懸念が未解消です。修正案
- .replace(/[！-／：-＠［-｀｛-～、-〜”'・]+/g, ' ') + .replace(/[！-／：-＠［-｀｛-～、-〜”・]+/g, ' ')🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@functions/src/funcs/tts.ts` at line 42, The regex in the string-cleaning call .replace(/[！-／：-＠［-｀｛-～、-〜”'・]+/g, ' ') is currently removing the ASCII apostrophe (') and breaks phoneme attributes and names like O'Hare; update that replace so it does not strip the half-width apostrophe by removing the ' character from the character class (leave other full-width punctuation and quotes intact) and keep the same global replacement behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@functions/src/funcs/tts.ts`:
- Line 42: The regex in the string-cleaning call
.replace(/[！-／：-＠［-｀｛-～、-〜”'・]+/g, ' ') is currently removing the ASCII
apostrophe (') and breaks phoneme attributes and names like O'Hare; update that
replace so it does not strip the half-width apostrophe by removing the '
character from the character class (leave other full-width punctuation and
quotes intact) and keep the same global replacement behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 92a6d6a9-b0eb-4c4e-808e-dd4aa89a460b
📒 Files selected for processing (2)
functions/src/funcs/tts.tsfunctions/src/utils/legacyIpa.ts
Member
Author
|
@coderabbitai 前回のコミットで既に半角アポストロフィ ' は除去済みです。対応不要です。 |
Contributor
|
Tip For best results, initiate chat on the files or code changes.
🧠 Learnings used |
2 tasks
This was referenced Mar 14, 2026
Closed
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
nameIpaフィールドをStation・Line型のGraphQLクエリ・型定義に追加nameIpaを<phoneme>タグでラップするwrapIpaヘルパーを導入tts.ts) のハードコードされたIPA phoneme正規表現置換（約130行）を削除し、アプリから送信されるSSMLに委譲Test plan
npm run typecheckパスnpm run lintパスnpm test(136 suites, 1259 tests) パスfunctions/のnpm run buildパス🤖 Generated with Claude Code
Summary by CodeRabbit
リリースノート
新機能
改善
テスト / フィクスチャ