Skip to content

Fix IPA generation for compound station names#1430

Merged
TinyKitten merged 1 commit into
devfrom
fix/inagekaigan-ipa
Mar 15, 2026
Merged

Fix IPA generation for compound station names#1430
TinyKitten merged 1 commit into
devfrom
fix/inagekaigan-ipa

Conversation

@TinyKitten

@TinyKitten TinyKitten commented Mar 15, 2026

Copy link
Copy Markdown
Member

Summary

  • split romanized compound station names ending in kaigan before English word lookup
  • pass katakana station names into station_name_to_ipa so station DTOs fall back correctly when roman parsing fails
  • add regression tests for Inagekaigan and katakana fallback behavior

Testing

  • SQLX_OFFLINE=true cargo test --lib --package stationapi use_case::dto::station -- --nocapture
  • SQLX_OFFLINE=true cargo test --lib --package stationapi test_station_name_ipa_splits_compound_kaigan_suffix -- --nocapture
  • cargo fmt --package stationapi

Summary by CodeRabbit

リリースノート

  • 新機能

    • 複合駅名(例:イナゲカイガン)のローマ字化に対応しました。
  • 改善

    • 駅名のローマ字表記精度を向上させました。
  • テスト

    • ローマ字化処理の検証テストを追加しました。

@github-actions github-actions Bot added the fix 直した label Mar 15, 2026
@coderabbitai

coderabbitai Bot commented Mar 15, 2026

Copy link
Copy Markdown
Contributor
📝 Walkthrough

ウォークスルー

IPA変換ロジックを更新し、「開業」(kaigan)の接尾辞で終わるトークンを処理する新しいヘルパー関数split_compound_token_to_ipaを追加しました。このヘルパーはトークンを茎と接尾辞に分割し、各部分をIPA形式に変換して結合します。また、word_to_ipaの呼び出し元を更新して、最初の引数として空文字列ではなく駅名カタカナを渡すようにしました。

変更内容

コホート / ファイル(s) 概要
IPA変換ロジック
stationapi/src/domain/ipa.rs
「kaigan」接尾辞で終わるトークンを検出して分割する新しいプライベートヘルパー関数split_compound_token_to_ipaを導入。茎と接尾辞の両方を再帰的にIPA形式に変換し、スペース区切りで結合。関連するテストケースを追加して、「イナゲカイガン」や「オオモリカイガン」などの入力を検証。
DTO駅情報と呼び出し元の更新
stationapi/src/use_case/dto/station.rs, stationapi/src/use_case/interactor/query.rs
station_name_to_ipaの呼び出しを変更し、最初の引数を空文字列から駅名カタカナ(station_name_k)に、第2引数をロマ字名(station_name_r.as_deref())に更新。name_roman_ipaの計算方法が改善され、カタカナベースのロマ字化と提供されたロマ字名の対応する動作を検証するテストを追加。

コードレビュー難易度

🎯 2 (Simple) | ⏱️ ~12 分

関連する可能性のあるプルリクエスト

提案ラベル

fix

ポエム

🐰 開業の秘密を解きし日
カイガンの茎も梢も知れり
スペース一つで音韻つなぎ
駅の名前は美しく舞う
ウサギたち喜びて跳ねる

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and accurately summarizes the main change: fixing IPA generation for compound station names, which is the core focus of the PR as evidenced by the new split_compound_token_to_ipa function and associated tests.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/inagekaigan-ipa
📝 Coding Plan
  • Generate coding plan for human review comments

Comment @coderabbitai help to get the list of available commands and usage tips.

@TinyKitten TinyKitten self-assigned this Mar 15, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
stationapi/src/use_case/dto/station.rs (1)

66-134: テストヘルパーの冗長性について

create_test_station は多数の None 引数を持ち、可読性がやや低下しています。将来的にテストケースが増える場合は、ビルダーパターンやデフォルト値を持つ構造体への移行を検討してください。

現状では機能しており、この PR の範囲としては問題ありません。

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@stationapi/src/use_case/dto/station.rs` around lines 66 - 134, The helper
function create_test_station currently calls Station::new with many positional
None/default arguments making it hard to read and maintain; refactor by
introducing a TestStationBuilder or implement Default for Station and use struct
update syntax or builder methods in create_test_station to set only the relevant
fields (e.g., name, name_katakana, name_roman, TransportType) while leaving
other fields as defaults, and replace the long positional Station::new call with
the clearer builder/Default-based construction to improve readability and future
extensibility.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@stationapi/src/use_case/dto/station.rs`:
- Around line 66-134: The helper function create_test_station currently calls
Station::new with many positional None/default arguments making it hard to read
and maintain; refactor by introducing a TestStationBuilder or implement Default
for Station and use struct update syntax or builder methods in
create_test_station to set only the relevant fields (e.g., name, name_katakana,
name_roman, TransportType) while leaving other fields as defaults, and replace
the long positional Station::new call with the clearer builder/Default-based
construction to improve readability and future extensibility.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 78752c32-5081-4d05-8991-c75c9d4613fa

📥 Commits

Reviewing files that changed from the base of the PR and between 6b28ca7 and c10958c.

📒 Files selected for processing (3)
  • stationapi/src/domain/ipa.rs
  • stationapi/src/use_case/dto/station.rs
  • stationapi/src/use_case/interactor/query.rs

@TinyKitten TinyKitten merged commit 0495368 into dev Mar 15, 2026
11 checks passed
@TinyKitten TinyKitten deleted the fix/inagekaigan-ipa branch March 15, 2026 04:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

fix 直した

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant