TTS関連のnameRoman/nameRomanIpaをnameTtsSegmentsに置き換え#5609
Conversation
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
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にTtsSegment/TtsAlphabetを追加し、Line/Station/TrainTypeへnameTtsSegmentsを導入。wrapPhonemeをセグメント対応に書き換え、useTTSText/useBusTTSTextでセグメントベースのphonemeラップへ切替。関連テスト/フィクスチャを更新。 Changes
Sequence Diagram(s)sequenceDiagram
actor GraphQL
participant Hook as useTTSText / useBusTTSText
participant Util as wrapPhoneme
participant TTS as SpeechEngine
GraphQL->>Hook: データを返す(`nameTtsSegments`含む)
Hook->>Hook: テンプレートでセグメントを組み立てる
Hook->>Util: wrapPhoneme(nameTtsSegments)
Util-->>Hook: SSML(phoneme/テキスト)を返す
Hook->>TTS: 生成したSSMLを送信して再生
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 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 |
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/hooks/useTTSText.ts (1)
175-180:⚠️ Potential issue | 🟠 Major
nameTtsSegmentsが null のケースで英語案内名が欠落する可能性が高いです。GraphQL スキーマで
nameTtsSegmentsがMaybe<Array<TtsSegment>>と定義されており、テスト fixtures や factories にnameTtsSegments: nullが 35+ 箇所残っています。wrapPhoneme()は null/undefined 入力で空文字を返すため、ph(s?.nameTtsSegments)の呼び出しは null データで駅名・路線名が欠落します。移行期間中は
nameTtsSegments || nameRomanのようなフォールバックを helper にまとめるか、fixtures をnameTtsSegments: []で統一してください。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/hooks/useTTSText.ts` around lines 175 - 180, The English station/line names can be lost when s.nameTtsSegments is null because ph(s?.nameTtsSegments) returns empty string; update the logic in useTTSText (the boundForEn computation using useMemo, plus any other places calling ph) to fall back to s.nameRoman when nameTtsSegments is null/undefined (e.g., call ph(s?.nameTtsSegments ?? s?.nameRoman) or extend wrapPhoneme/ph to accept a fallback value), and ensure loopLineBoundEn?.boundFor also falls back similarly so null fixtures don't drop the English names.
🧹 Nitpick comments (2)
src/components/TrainTypeBoxJL.test.tsx (1)
70-88: TrainType モックは shared factory に寄せたいです。同系の TrainTypeBox テストでほぼ同じ literal が並んでいて、今回の
nameTtsSegments追加のような型変更のたびに横並び修正が発生しています。src/utils/test/の factory/fixture に寄せると追従漏れを減らせます。 As per coding guidelines "Reuse helper utilities fromsrc/utils/test/in tests to avoid duplicate setup code".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/TrainTypeBoxJL.test.tsx` around lines 70 - 88, Replace the inline mockTrainType literal in TrainTypeBox tests with a shared test factory fixture: create (or reuse) a function like getMockTrainType / createTrainTypeFixture in the test utilities and import it into TrainTypeBox.test.tsx, then use that factory and only override fields needed for the specific test (e.g., nameTtsSegments) instead of duplicating the full TrainType object; update other TrainTypeBox-related tests to consume the same factory to avoid future drift when the TrainType shape changes.src/lib/graphql/queries.ts (1)
48-55:TtsSegmentの選択セットは fragment 化しておくとズレにくいです。同じ 6 fields を 5 箇所に直書きしているので、
TtsSegment側の変更が入ったときに一部 fragment だけ追従漏れしやすいです。既にこのファイルは共通 fragment を使っているので、ここもTtsSegmentFieldsに寄せた方が保守しやすいです。♻️ 例
+export const TTS_SEGMENT_FRAGMENT = gql` + fragment TtsSegmentFields on TtsSegment { + alphabet + fallbackText + lang + pronunciation + separator + surface + } +`; + export const TINY_TRAIN_TYPE_FRAGMENT = gql` + ${TTS_SEGMENT_FRAGMENT} fragment TinyTrainTypeFields on TrainTypeNested { id typeId @@ - nameTtsSegments { - alphabet - fallbackText - lang - pronunciation - separator - surface - } + nameTtsSegments { + ...TtsSegmentFields + }Also applies to: 86-93, 109-116, 134-141, 167-174
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/graphql/queries.ts` around lines 48 - 55, 複数箇所で直書きされている TtsSegment の 6 フィールドを共通の fragment にまとめて使うよう修正してください: 新たに TtsSegmentFields という fragment を作り(定義が既にあればそれを使い)、nameTtsSegments の選択セットや他の重複箇所(現在直書きされている 5 箇所)を fragment スプレッド(...TtsSegmentFields)に置き換え、クエリ/fragment の定義を同一ファイル内かインポートして gql に含めるようにしてください。 Ensure the fragment name exactly matches TtsSegmentFields and update all occurrences (the blocks currently listing alphabet, fallbackText, lang, pronunciation, separator, surface) to use the fragment.
🤖 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/hooks/useBusTTSText.ts`:
- Around line 105-106: The English station-name construction uses
ph(target?.nameTtsSegments) directly, which yields empty phrases when
nameTtsSegments is null; update the logic (e.g., where
directionalStops?.map(...) and across the same pattern in lines ~525-676) to
fall back to target?.nameRoman or target?.name when ph(...) returns falsy —
implement a helper like getEnglishTtsName(target) that returns
ph(target?.nameTtsSegments) || target?.nameRoman || target?.name || '' and use
that helper in the directionalStops mapping and all similar usages to ensure
robust English TTS output.
In `@src/utils/phoneme.ts`:
- Line 13: The early return "if (!segments?.length) return '';" in
nameTtsSegments causes station/line names to vanish when segments is null/empty;
instead, keep a backward-compatible fallback: when segments is null or empty,
construct a default segments value (e.g., a single TtsSegment containing the
original name/text or a safe placeholder) and continue processing so the name is
emitted rather than an empty string. Update the logic in nameTtsSegments (and
similar blocks around lines 16-21 referencing segments) to use that
fallbackSegments before further processing.
---
Outside diff comments:
In `@src/hooks/useTTSText.ts`:
- Around line 175-180: The English station/line names can be lost when
s.nameTtsSegments is null because ph(s?.nameTtsSegments) returns empty string;
update the logic in useTTSText (the boundForEn computation using useMemo, plus
any other places calling ph) to fall back to s.nameRoman when nameTtsSegments is
null/undefined (e.g., call ph(s?.nameTtsSegments ?? s?.nameRoman) or extend
wrapPhoneme/ph to accept a fallback value), and ensure loopLineBoundEn?.boundFor
also falls back similarly so null fixtures don't drop the English names.
---
Nitpick comments:
In `@src/components/TrainTypeBoxJL.test.tsx`:
- Around line 70-88: Replace the inline mockTrainType literal in TrainTypeBox
tests with a shared test factory fixture: create (or reuse) a function like
getMockTrainType / createTrainTypeFixture in the test utilities and import it
into TrainTypeBox.test.tsx, then use that factory and only override fields
needed for the specific test (e.g., nameTtsSegments) instead of duplicating the
full TrainType object; update other TrainTypeBox-related tests to consume the
same factory to avoid future drift when the TrainType shape changes.
In `@src/lib/graphql/queries.ts`:
- Around line 48-55: 複数箇所で直書きされている TtsSegment の 6 フィールドを共通の fragment
にまとめて使うよう修正してください: 新たに TtsSegmentFields という fragment
を作り(定義が既にあればそれを使い)、nameTtsSegments の選択セットや他の重複箇所(現在直書きされている 5 箇所)を fragment
スプレッド(...TtsSegmentFields)に置き換え、クエリ/fragment の定義を同一ファイル内かインポートして gql
に含めるようにしてください。 Ensure the fragment name exactly matches TtsSegmentFields and
update all occurrences (the blocks currently listing alphabet, fallbackText,
lang, pronunciation, separator, surface) to use the fragment.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d9c140ba-66c2-4fbe-9535-6bdb67adec9a
📒 Files selected for processing (18)
src/@types/graphql.d.tssrc/__fixtures__/line.tssrc/__fixtures__/station.tssrc/components/TrainTypeBoxJL.test.tsxsrc/components/TrainTypeBoxJO.test.tsxsrc/components/TrainTypeBoxJRKyushu.test.tsxsrc/components/TrainTypeBoxSaikyo.test.tsxsrc/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/phoneme.tssrc/utils/test/factories.tssrc/utils/trainTypeString.test.ts
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src/utils/phoneme.ts (1)
62-62:⚠️ Potential issue | 🟠 Major
segments欠損時に名称が消える可能性があります。Line 62 の
return ''により、nameTtsSegmentsが欠損/空配列のケースで駅名・路線名が案内文から脱落します。フォールバック文字列を受け取り、空時はそれを返す形にしておくのが安全です。💡 修正例
export const wrapPhoneme = ( - segments: TtsSegment[] | null | undefined + segments: TtsSegment[] | null | undefined, + fallbackText = '', ): string => { - if (!segments?.length) return ''; + if (!segments?.length) return escapeXml(fallbackText); return groupSegments(segments)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/utils/phoneme.ts` at line 62, The guard "if (!segments?.length) return ''" in nameTtsSegments causes station/line names to drop out when segments are missing; change it to accept and use a fallback string (e.g., add a fallback parameter or use an existing name argument) and return that fallback when segments is null/empty instead of '', updating the function signature and all callers of nameTtsSegments accordingly so callers can supply a sensible fallback display string.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@src/utils/phoneme.ts`:
- Line 62: The guard "if (!segments?.length) return ''" in nameTtsSegments
causes station/line names to drop out when segments are missing; change it to
accept and use a fallback string (e.g., add a fallback parameter or use an
existing name argument) and return that fallback when segments is null/empty
instead of '', updating the function signature and all callers of
nameTtsSegments accordingly so callers can supply a sensible fallback display
string.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 6a722e09-428d-4834-ae9d-b7cab6dd9efc
📒 Files selected for processing (1)
src/utils/phoneme.ts
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/lib/graphql/queries.ts (2)
124-126:...TtsSegmentFieldsを使うフラグメントにTTS_SEGMENT_FRAGMENTを明示注入してください。現状は別フラグメント経由の間接依存になっていて、依存元の変更で
Unknown fragmentを起こしやすい構造です。💡 提案差分
export const TRAIN_TYPE_NESTED_FRAGMENT = gql` + ${TTS_SEGMENT_FRAGMENT} ${LINE_NESTED_FRAGMENT} fragment TrainTypeNestedFields on TrainTypeNested { @@ export const STATION_FRAGMENT = gql` + ${TTS_SEGMENT_FRAGMENT} ${LINE_NESTED_FRAGMENT} ${STATION_NUMBER_FRAGMENT} ${TRAIN_TYPE_NESTED_FRAGMENT} fragment StationFields on Station {Also applies to: 151-155
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/graphql/queries.ts` around lines 124 - 126, The fragment TRAIN_TYPE_NESTED_FRAGMENT (which currently interpolates LINE_NESTED_FRAGMENT) indirectly depends on the TtsSegmentFields fragment; explicitly inject the TTS_SEGMENT_FRAGMENT into TRAIN_TYPE_NESTED_FRAGMENT by adding ${TTS_SEGMENT_FRAGMENT} alongside ${LINE_NESTED_FRAGMENT} so the ...TtsSegmentFields reference is resolved directly; apply the same explicit injection to the other fragment at lines 151-155 that also uses ...TtsSegmentFields to avoid Unknown fragment errors.
93-100:LineNestedFields.station.nameTtsSegmentsでもTtsSegmentFieldsを再利用してください。このブロックだけインライン展開になっており、将来のフィールド追加時に差分漏れが起きやすいです。参照を統一したほうが安全です。
💡 提案差分
station { id groupId name nameRoman nameRomanIpa nameChinese nameKorean hasTrainTypes nameTtsSegments { - alphabet - fallbackText - lang - pronunciation - separator - surface + ...TtsSegmentFields } stationNumbers { ...StationNumberFields } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/graphql/queries.ts` around lines 93 - 100, LineNestedFields.station.nameTtsSegments currently inlines the TTS fields instead of reusing the existing TtsSegmentFields fragment/interface; replace the inline block with a reference to TtsSegmentFields so future additions stay synchronized. Locate the nameTtsSegments selection inside LineNestedFields.station in src/lib/graphql/queries.ts and swap the explicit alphabet/fallbackText/lang/pronunciation/separator/surface fields for the shared TtsSegmentFields fragment/selection (or the equivalent shared constant) used elsewhere in the codebase.
🤖 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/phoneme.ts`:
- Around line 60-64: 現在の早期リターンは segments が空のときにそのまま fallback を返しており(segments /
fallback パラメータ)、"&" や "<" 等で SSML を破壊するため、返す前に必ず XML/SSML
エスケープを適用するよう修正してください;具体的には該当関数の早期 return (if (!segments?.length) return fallback
?? '') を書き換え、fallback が null/undefined でなければ & -> &, < -> <, > -> >, "
-> ", ' -> ' を置換するユーティリティ(例: escapeXml / escapeSsml
を新規追加または既存ユーティリティを利用)を呼んでから返すようにしてください。
---
Nitpick comments:
In `@src/lib/graphql/queries.ts`:
- Around line 124-126: The fragment TRAIN_TYPE_NESTED_FRAGMENT (which currently
interpolates LINE_NESTED_FRAGMENT) indirectly depends on the TtsSegmentFields
fragment; explicitly inject the TTS_SEGMENT_FRAGMENT into
TRAIN_TYPE_NESTED_FRAGMENT by adding ${TTS_SEGMENT_FRAGMENT} alongside
${LINE_NESTED_FRAGMENT} so the ...TtsSegmentFields reference is resolved
directly; apply the same explicit injection to the other fragment at lines
151-155 that also uses ...TtsSegmentFields to avoid Unknown fragment errors.
- Around line 93-100: LineNestedFields.station.nameTtsSegments currently inlines
the TTS fields instead of reusing the existing TtsSegmentFields
fragment/interface; replace the inline block with a reference to
TtsSegmentFields so future additions stay synchronized. Locate the
nameTtsSegments selection inside LineNestedFields.station in
src/lib/graphql/queries.ts and swap the explicit
alphabet/fallbackText/lang/pronunciation/separator/surface fields for the shared
TtsSegmentFields fragment/selection (or the equivalent shared constant) used
elsewhere in the codebase.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d9b07011-d821-4ad2-8ab1-4508eb9bcaa5
📒 Files selected for processing (4)
src/hooks/useBusTTSText.tssrc/hooks/useTTSText.tssrc/lib/graphql/queries.tssrc/utils/phoneme.ts
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/lib/graphql/queries.ts (1)
69-74:LINE_NESTED_FRAGMENTのTtsSegmentFields依存を明示した方が安全です。現状は
${TINY_TRAIN_TYPE_FRAGMENT}経由の間接依存で動いていますが、将来の分割/軽量化で壊れやすいです。LINE_NESTED_FRAGMENT側でもTTS_SEGMENT_FRAGMENTを明示インポートしておくと保守しやすくなります。差分案
export const LINE_NESTED_FRAGMENT = gql` ${COMPANY_FRAGMENT} ${LINE_SYMBOL_FRAGMENT} ${STATION_NUMBER_FRAGMENT} + ${TTS_SEGMENT_FRAGMENT} ${TINY_TRAIN_TYPE_FRAGMENT} fragment LineNestedFields on LineNested {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/graphql/queries.ts` around lines 69 - 74, The LINE_NESTED_FRAGMENT currently relies indirectly on TtsSegmentFields via ${TINY_TRAIN_TYPE_FRAGMENT}; explicitly add the TTS_SEGMENT_FRAGMENT import to the fragment header so the dependency is declared locally — update the top of LINE_NESTED_FRAGMENT to include ${TTS_SEGMENT_FRAGMENT} alongside ${COMPANY_FRAGMENT}, ${LINE_SYMBOL_FRAGMENT}, ${STATION_NUMBER_FRAGMENT}, ${TINY_TRAIN_TYPE_FRAGMENT} and ensure the fragment definition still references the TtsSegmentFields fragment name if used inside LineNestedFields.src/utils/phoneme.ts (1)
24-25: コメント内容と実装の粒度を揃えると読みやすくなります。コメントは「同種セグメントをグループ化」とありますが、
plainは現在マージせず逐次 push しています。コメントを実装に合わせるか、plainも連結すると意図が明確です。Also applies to: 49-52
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/utils/phoneme.ts` around lines 24 - 25, The comment for groupSegments says "group consecutive same-type segments" but the implementation treats segments with kind 'plain' differently (it pushes each 'plain' without merging); either update the comment to explicitly state that 'plain' segments are not merged, or modify groupSegments (and the related block that handles plain at the other spot) to concatenate consecutive 'plain' segments into the same SegmentGroup; locate the logic in the groupSegments function and the code that handles kind === 'plain' and either change the comment text to reflect the exception or change the push behavior to merge plain text into the current group (keeping SegmentGroup/TtsSegment types consistent).
🤖 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/lib/graphql/queries.ts`:
- Around line 69-74: The LINE_NESTED_FRAGMENT currently relies indirectly on
TtsSegmentFields via ${TINY_TRAIN_TYPE_FRAGMENT}; explicitly add the
TTS_SEGMENT_FRAGMENT import to the fragment header so the dependency is declared
locally — update the top of LINE_NESTED_FRAGMENT to include
${TTS_SEGMENT_FRAGMENT} alongside ${COMPANY_FRAGMENT}, ${LINE_SYMBOL_FRAGMENT},
${STATION_NUMBER_FRAGMENT}, ${TINY_TRAIN_TYPE_FRAGMENT} and ensure the fragment
definition still references the TtsSegmentFields fragment name if used inside
LineNestedFields.
In `@src/utils/phoneme.ts`:
- Around line 24-25: The comment for groupSegments says "group consecutive
same-type segments" but the implementation treats segments with kind 'plain'
differently (it pushes each 'plain' without merging); either update the comment
to explicitly state that 'plain' segments are not merged, or modify
groupSegments (and the related block that handles plain at the other spot) to
concatenate consecutive 'plain' segments into the same SegmentGroup; locate the
logic in the groupSegments function and the code that handles kind === 'plain'
and either change the comment text to reflect the exception or change the push
behavior to merge plain text into the current group (keeping
SegmentGroup/TtsSegment types consistent).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3c851241-2980-4d77-b701-8e12c9380226
📒 Files selected for processing (2)
src/lib/graphql/queries.tssrc/utils/phoneme.ts
|
@coderabbitai resume |
✅ Actions performedReviews resumed. |
Summary
nameRoman/nameRomanIpaを、GraphQL側で新規追加されたnameTtsSegments(TtsSegment[])に置き換えwrapPhoneme関数をTtsSegment[]ベースに書き換え、IPAセグメントは<phoneme>タグで囲み、Plainセグメントはそのまま出力StationFields,LineNestedFields,TinyTrainTypeFields,TrainTypeNestedFields)にnameTtsSegmentsフィールドを追加useTTSText/useBusTTSTextの英語テンプレート内の全ph()呼び出しをnameTtsSegmentsベースに更新Test plan
npm run typecheckパス(既存の__typenameエラー1件のみ)npm test全1281テストパスnpx biome check --unsafe --fix ./srcパス🤖 Generated with Claude Code
Summary by CodeRabbit
新機能
改善
テスト