IPA生成処理を #1531 (7bf7f17) 時点に戻す#1548
Merged
Merged
Conversation
Azure TTS 向けの IPA 書き換えシリーズ (#1532, #1534/#1535, #1538, #1539, #1543, #1545, #1546) が全体的に正しく発音できなくなっていたため、 IPA 生成処理を #1531 (7bf7f17) 時点のものに戻す。 対象: - domain/ipa.rs: 7bf7f17 へ復元（ジ=ʤ 表記、路線 name_ipa の 「線」→ " laɪn" 等の英語接尾辞付与、語末撥音 ɴ、長音は母音分割など） - dto/line.rs / dto/station.rs: 上記に対応するテスト期待値を 7bf7f17 へ復元 ただし #1533「路線名の name_ipa・name_tts_segments が null/空になる不具合 修正」は Azure 対応とは独立した実バグ修正のため、削除せず維持する （中黒「・」・全角/半角括弧を語の区切りとして空白に変換し、区切り由来の 余分な空白を正規化）。回帰テストも合わせて維持。 列車種別の重複排除 (#1540/#1541) 等、IPA 生成と無関係な変更は据え置き。 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01X4tdXHjrFBNqWerrAdGe9n
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 (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthrough
ChangesIPA音韻パイプライン改修と関連テスト更新
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning Billing warning: we have not been able to collect payment for this subscription for more than 72 hours. Please update the payment method or pay any pending invoices in Billing to avoid service interruption. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
stationapi/src/use_case/dto/line.rs (1)
438-439: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win中黒路線名テストの検証が弱く、回帰を見逃しやすいです。
Line 438-439 は
Someかつ非空のみの確認になっており、・の正規化や線 -> laɪn置換が壊れても通過できます。最低限の不変条件を追加して契約を固定したほうが安全です。差分案
- assert!(grpc_line.name_ipa.is_some()); - assert!(!grpc_line.name_ipa.as_deref().unwrap().is_empty()); + let ipa = grpc_line.name_ipa.expect("name_ipa should be present"); + assert!(!ipa.is_empty()); + assert!(ipa.contains(" laɪn")); + assert!(!ipa.contains(" ")); + assert!(!ipa.contains('・'));🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@stationapi/src/use_case/dto/line.rs` around lines 438 - 439, The assertions at lines 438-439 for the middle dot line name test only verify that grpc_line.name_ipa is Some and non-empty, which is insufficient to catch regressions in normalization of the middle dot character '・' or the substitution of '線' to 'laɪn'. Strengthen the test by adding specific assertions that validate the actual expected content of grpc_line.name_ipa, such as checking for the presence of expected substrings or matching against the complete expected normalized value, rather than just checking for existence and non-emptiness.stationapi/src/domain/ipa.rs (1)
1158-1168: 📐 Maintainability & Code Quality | 🔵 Trivial
last_vowelの戻り値が未使用のため、分岐ロジックが冗長です。両分岐の結果は等価です。
last_vowel(&output).is_some()は判定にしか使われず、どちらのブランチでも「末尾がːでなければ追加」という動作に統一されています。シェル実行結果からlast_vowelの呼び出しは本箇所のみであるため、ロジックを簡素化すれば関数自体を削除でき、clippy の dead_code 警告を回避できます。♻️ 提案: 分岐の簡素化
Phoneme::LongVowel => { - // Lengthen the preceding vowel - if last_vowel(&output).is_some() { - // Check if already has ː - if !output.ends_with('ː') { - output.push('ː'); - } - } else { - output.push('ː'); - } + // Lengthen the preceding vowel (avoid doubling the marker) + if !output.ends_with('ː') { + output.push('ː'); + } i += 1; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@stationapi/src/domain/ipa.rs` around lines 1158 - 1168, The branching logic for the LongVowel phoneme case is redundant because both the if and else branches perform the same operation: checking if the output ends with 'ː' and adding it if not. Remove the unnecessary call to `last_vowel(&output).is_some()` and simplify the logic to directly check `!output.ends_with('ː')` before pushing the lengthening marker. This eliminates the redundant condition check and unifies both code paths into a single, simpler statement that applies the same lengthening logic regardless of the previous vowel detection.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@stationapi/src/domain/ipa.rs`:
- Around line 1158-1168: The branching logic for the LongVowel phoneme case is
redundant because both the if and else branches perform the same operation:
checking if the output ends with 'ː' and adding it if not. Remove the
unnecessary call to `last_vowel(&output).is_some()` and simplify the logic to
directly check `!output.ends_with('ː')` before pushing the lengthening marker.
This eliminates the redundant condition check and unifies both code paths into a
single, simpler statement that applies the same lengthening logic regardless of
the previous vowel detection.
In `@stationapi/src/use_case/dto/line.rs`:
- Around line 438-439: The assertions at lines 438-439 for the middle dot line
name test only verify that grpc_line.name_ipa is Some and non-empty, which is
insufficient to catch regressions in normalization of the middle dot character
'・' or the substitution of '線' to 'laɪn'. Strengthen the test by adding specific
assertions that validate the actual expected content of grpc_line.name_ipa, such
as checking for the presence of expected substrings or matching against the
complete expected normalized value, rather than just checking for existence and
non-emptiness.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 28bc7b95-8fe3-40aa-a80a-0708f32f7eba
📒 Files selected for processing (3)
stationapi/src/domain/ipa.rsstationapi/src/use_case/dto/line.rsstationapi/src/use_case/dto/station.rs
CodeRabbit の指摘 (#1548) を受け、test_name_ipa_with_nakaguro_is_not_null の 検証を Some/非空のみから、「・」→空白正規化（連続空白なし）・「線」→ " laɪn" 置換まで確認する不変条件に強化する。 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01X4tdXHjrFBNqWerrAdGe9n
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
2 participants
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.
概要
TTS の IPA 生成が全体的に正しく発音できなくなっていたため、IPA 生成処理を #1531 (
7bf7f17) 時点のものに戻します。#1532以降の Azure TTS 向け書き換えシリーズを撤回します。変更の種類
変更内容
domain/ipa.rsを7bf7f17時点へ復元（ジ表記ʤ、路線name_ipaの「線」→laɪn等の英語接尾辞付与、語末撥音ɴ、長音は母音分割など）。dto/line.rs/dto/station.rsのテスト期待値を上記挙動に合わせて7bf7f17へ復元。name_ipa・name_tts_segmentsが null/空になる不具合修正」は Azure 対応とは独立した実バグ修正のため削除せず維持（中黒「・」・全角/半角括弧を語の区切りとして空白に変換し、区切り由来の余分な空白を正規化）。回帰テストも維持。テスト
SQLX_OFFLINE=true cargo test --lib --package stationapiで 341 件すべて pass（復元した7bf7f17系テストと維持した #1533 のテストの両方を含む）。cargo fmt --all -- --checkが通ることcargo clippy -- -D warningsが通ることcargo test（SQLX_OFFLINE=true）が通ること関連Issue
スクリーンショット（任意）
Generated by Claude Code
Summary by CodeRabbit
リリースノート