Skip to content

パフォーマンス系の技術負債の一部解消とバグ修正#1363

Merged
TinyKitten merged 5 commits into
devfrom
fix/debt-and-bugs
Jan 5, 2026
Merged

パフォーマンス系の技術負債の一部解消とバグ修正#1363
TinyKitten merged 5 commits into
devfrom
fix/debt-and-bugs

Conversation

@TinyKitten

@TinyKitten TinyKitten commented Jan 4, 2026

Copy link
Copy Markdown
Member

本番環境で現状非公開のバス路線が紛れ込んでいた

Summary by CodeRabbit

  • 新機能

    • 検索の対象に「Rail」「Bus」「RailAndBus」を追加しました。
  • 変更点

    • transport_type未指定のデフォルトが「駅のみ(Rail)」に変更されました。
    • Bus検索はバス停ではなくバス路線(ライン)中心の結果になります。
    • RailAndBus指定時のみ、駅に関連するバス路線が併せて返されます。
  • 改善

    • 検索処理をHashMapベースの最適化で高速化し、結果の一貫性と効率を向上しました。
  • ドキュメント/テスト

    • 技術文書更新と多数のユニットテスト追加を実施しました。

✏️ Tip: You can customize this high-level summary in your review settings.

@TinyKitten TinyKitten self-assigned this Jan 4, 2026
@coderabbitai

coderabbitai Bot commented Jan 4, 2026

Copy link
Copy Markdown
Contributor
📝 Walkthrough

Walkthrough

TransportTypeのデフォルトをrail-onlyに変更し、RailAndBusを追加。近隣検索ヘルパーをget_nearby_bus_stopsからget_nearby_bus_linesへ改名し戻り型をVec<Line>へ変更。query.rsはHashMapベースのO(1)検索へ最適化し、build_route_tree_mapを参照版へ変更。

Changes

コホート / ファイル 変更内容
ドキュメント
docs/nearby-bus-stops.md
TransportTypeのデフォルトをrail-onlyへ変更。RailAndBus追加、API使用例と説明をRail/Bus/RailAndBusに合わせて更新。近隣ヘルパー記述をbus stops→bus lines志向へ変更。
技術負債ドキュメント
docs/technical_debt.md
最終更新日を2026年1月に更新。過剰なclone()項目を「対応済み」にし、HashMap/参照ベース最適化の詳細を追記。
クエリロジック最適化
stationapi/src/use_case/interactor/query.rs
company_map / train_type_map / station_lookup / bus_stop_by_line_cd などHashMap導入で線形探索を置換。ルート・ライン埋め込みをmapベースへ変更。build_route_tree_mapfn build_route_tree_map<'a>(&self, stops: &'a [Station]) -> BTreeMap<i32, Vec<&'a Station>> に変更。テストとモック追加。
APIシグネチャ / プロトコル
proto/..., src/use_case/interactor/query.rs
TransportType enumにRailAndBus = 3;を追加。TransportTypeUnspecifiedの説明をrail-onlyデフォルトへ修正。近隣ヘルパー名と戻り型を get_nearby_bus_lines(...)->Result<Vec<Line>,...> に更新。

Sequence Diagram(s)

(該当条件を満たさないため、シーケンス図は割愛します)

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

Poem

🐰 ちいさな足で跳ねるよ
駅をまず見つけるよ、しっかりと
バスは線につながる地図へと
HashMapの野をぴょんと駆けて
新しい道がすぐそこに開くよ ✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 39.39% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed タイトルは、パフォーマンス関連の技術負債の解消とバグ修正という変更内容の主要な目的を正確に反映しています。
✨ Finishing touches
  • 📝 Generate docstrings

📜 Recent review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 51be8a3 and a05f910.

📒 Files selected for processing (1)
  • stationapi/src/use_case/interactor/query.rs
🧰 Additional context used
📓 Path-based instructions (4)
stationapi/src/use_case/**/*.rs

📄 CodeRabbit inference engine (AGENTS.md)

Application logic in use case layer should implement contract traits defined in traits/ module (e.g., QueryUseCase in interactor/query.rs)

Files:

  • stationapi/src/use_case/interactor/query.rs
stationapi/src/**/*.rs

📄 CodeRabbit inference engine (AGENTS.md)

Run cargo test --lib --package stationapi or make test-unit for unit tests focusing on entities and repository mocks without database

Files:

  • stationapi/src/use_case/interactor/query.rs
**/*.rs

📄 CodeRabbit inference engine (AGENTS.md)

**/*.rs: Before committing, run cargo fmt on all Rust code
Before committing, run cargo clippy --all-targets --all-features and resolve new Clippy warnings unless covered by existing #![allow] attributes

Files:

  • stationapi/src/use_case/interactor/query.rs
stationapi/src/use_case/interactor/query.rs

📄 CodeRabbit inference engine (AGENTS.md)

When modifying QueryInteractor, ensure enrichment steps (companies, train types, line symbols) still behave as expected and verify helper methods such as update_station_vec_with_attributes and build_route_tree_map

Files:

  • stationapi/src/use_case/interactor/query.rs
🧠 Learnings (5)
📓 Common learnings
Learnt from: CR
Repo: TrainLCD/StationAPI PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T10:50:36.694Z
Learning: Applies to stationapi/src/use_case/interactor/query.rs : When modifying `QueryInteractor`, ensure enrichment steps (companies, train types, line symbols) still behave as expected and verify helper methods such as `update_station_vec_with_attributes` and `build_route_tree_map`
📚 Learning: 2025-11-25T10:50:36.694Z
Learnt from: CR
Repo: TrainLCD/StationAPI PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T10:50:36.694Z
Learning: Applies to stationapi/src/use_case/interactor/query.rs : When modifying `QueryInteractor`, ensure enrichment steps (companies, train types, line symbols) still behave as expected and verify helper methods such as `update_station_vec_with_attributes` and `build_route_tree_map`

Applied to files:

  • stationapi/src/use_case/interactor/query.rs
📚 Learning: 2025-11-25T10:50:36.694Z
Learnt from: CR
Repo: TrainLCD/StationAPI PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T10:50:36.694Z
Learning: Applies to stationapi/src/use_case/**/*.rs : Application logic in use case layer should implement contract traits defined in `traits/` module (e.g., `QueryUseCase` in `interactor/query.rs`)

Applied to files:

  • stationapi/src/use_case/interactor/query.rs
📚 Learning: 2025-11-25T10:50:36.694Z
Learnt from: CR
Repo: TrainLCD/StationAPI PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T10:50:36.694Z
Learning: Applies to stationapi/src/**/*.rs : Run `cargo test --lib --package stationapi` or `make test-unit` for unit tests focusing on entities and repository mocks without database

Applied to files:

  • stationapi/src/use_case/interactor/query.rs
📚 Learning: 2025-11-25T10:50:36.694Z
Learnt from: CR
Repo: TrainLCD/StationAPI PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T10:50:36.694Z
Learning: Applies to stationapi/src/infrastructure/**/*.rs : Integration tests in infrastructure layer must use `#[cfg_attr(not(feature = "integration-tests"), ignore)]` attribute to conditionally ignore tests when the feature is not enabled

Applied to files:

  • stationapi/src/use_case/interactor/query.rs
🧬 Code graph analysis (1)
stationapi/src/use_case/interactor/query.rs (4)
stationapi/src/domain/entity/line.rs (1)
  • new (48-120)
stationapi/src/domain/entity/station.rs (1)
  • new (84-218)
stationapi/src/domain/entity/train_type.rs (1)
  • new (26-58)
stationapi/src/domain/entity/line_symbol.rs (1)
  • new (11-17)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: Test Suite
  • GitHub Check: Test Suite
🔇 Additional comments (5)
stationapi/src/use_case/interactor/query.rs (5)

239-257: HashMap ベースの O(1) 検索への最適化が適切に実装されています。

以下の HashMap が追加され、線形探索が O(1) 検索に置き換えられました:

  • company_map:company_cd による会社情報の高速検索
  • train_type_map:station_cd による列車種別の高速検索
  • station_lookup:(line_cd, station_g_cd) による駅情報の高速検索
  • tt_line_map:line_cd による路線の高速検索(前回レビューの懸念事項に対応)
  • train_type_by_type_cd:type_cd による列車種別の高速検索
  • bus_stop_by_line_cd:line_cd によるバス停の高速検索

これらの最適化により、パフォーマンスが大幅に向上します。コーディングガイドラインに従い、エンリッチメント処理(会社情報、列車種別、路線シンボル)が正しく動作することを確認します。

Based on learnings, enrichment steps have been verified to behave as expected.

Also applies to: 526-528, 612-614, 823-829, 976-983


1106-2373: 包括的なテストスイートが追加され、前回レビューの懸念事項に対応しています。

update_station_vec_with_attributes を含む主要なヘルパーメソッドの包括的なテストカバレッジが追加されました:

  • 会社情報のエンリッチメント検証
  • 列車種別のエンリッチメント検証
  • 路線シンボルの追加検証
  • 鉄道駅への近隣バス路線の追加検証
  • HashMap のキーが存在しない場合のエラーハンドリング検証(パニックしないことを確認)
  • 空の入力、複数駅、駅番号、路線配列などのエッジケース検証

コーディングガイドラインに従い、以下のコマンドでテストを実行してください:

cargo test --lib --package stationapi

または

make test-unit

Based on learnings and coding guidelines, helper methods like update_station_vec_with_attributes and build_route_tree_map are now properly tested.


999-1011: build_route_tree_map のシグネチャ変更は妥当な最適化です。

Vec<Station> の代わりに Vec<&'a Station> を返すことで、不要なクローン操作を排除し、メモリ効率を改善します。

呼び出し元(get_routes 行 592、get_routes_minimal 行 696)は参照を正しく処理しており、イテレータパターンと参照の暗黙的なデリファレンスを通じて、フィールドアクセスとメソッド呼び出しが問題なく機能します。

テストスイート(行 1444-1489)が参照セマンティクス、グループ化ロジック、空の入力ケースを検証しており、enrichment パイプラインへの影響もありません。


830-840: get_train_typesメソッドの二重重複排除ロジックを確認してください。

行799-804と行830-840の両方で行われている重複排除がリソース効率と意図性の観点から疑問です。

最初の重複排除は停車駅から抽出したline_group_idHashSetで一意化しています(行799-804)。その後、get_by_line_group_id_vecを呼び出してから、再び同じline_group_cdに基づいて重複排除を行っています(行830-840)。

この二重の重複排除が必要な理由(例えば、get_by_line_group_id_vecが複数の同じline_group_cdを持つTrainTypeを返すケース)が明確でない場合、以下の対応が必要です:

  • このロジックが意図的か確認するため、テストを追加するか、コメントで理由を明記してください
  • get_by_line_group_id_vecの実装を確認し、その戻り値が本当に重複を含む可能性があるか検証してください

ただし、プロセスとしては機能しており、会社情報と路線情報の拡張処理は適切に実行されています。


890-890: このコード実装は意図的かつ正当です。

train_type.line フィールドは Option<Box<Line>> 型として設計されており、None 値は許容されます。891行のコメントで明示されているように、train_type.lines ベクターの最初の要素を line フィールドに設定する動作は意図的な設計です。

フィルタリング処理により lines が空になる場合(マッチング行線がない場合や重複排除後)、first()None を返し、それが line フィールドに設定されます。これは正当な動作であり、ドメイン設計と一致しています。


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

@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.

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 (1)
stationapi/src/use_case/interactor/query.rs (1)

973-985: 同一 line_cd を持つ複数バス停の扱いについて修正が必要

nearby_bus_stops から bus_stop_by_line_cd HashMap を構築する際、同じ line_cd を持つ複数のバス停がある場合、イテレーション順で最後のバス停だけが保持されます。

同じ関数内で bus_linesseen_line_cds.insert(line.line_cd) を使用して明示的に line_cd 単位で重複排除されていますが、バス停ソースに対しては同じパターンが適用されていません。このため、複数の近傍バス停が同じ路線に属する場合、距離に基づいた選定ではなく任意のものが選ばれるため、コードベース全体の重複排除パターンと矛盾しています。

nearby_bus_stopsbus_lines と同じく HashMap 構築前に重複排除するか、動作を明示的にドキュメント化してください。

🧹 Nitpick comments (2)
stationapi/src/use_case/interactor/query.rs (1)

262-275: .cloned().cloned() パターンについて

現在の実装は正しく動作しますが、.cloned().cloned() のチェーンはやや冗長です。Company の所有権が他で必要ない場合、HashMap を HashMap<i32, Company> として構築することで、単一の .cloned() で済みます。

ただし、現在のアプローチも正しく、パフォーマンスへの影響は軽微なため、このままでも問題ありません。

🔎 代替案(オプション)
-        let company_map: std::collections::HashMap<i32, &Company> =
-            companies.iter().map(|c| (c.company_cd, c)).collect();
+        let company_map: std::collections::HashMap<i32, Company> =
+            companies.into_iter().map(|c| (c.company_cd, c)).collect();

使用箇所も以下のように変更:

-            line.company = company_map.get(&line.company_cd).cloned().cloned();
+            line.company = company_map.get(&line.company_cd).cloned();
docs/technical_debt.md (1)

287-287: 取り消し線による完了マーク

取り消し線(~~Clone 削減~~)で完了項目を示すのは良いアプローチですが、完全に削除するか、別セクションに移動することも検討してください。長期的には完了項目が蓄積し、ドキュメントが読みにくくなる可能性があります。

📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8db764b and 082e4f6.

📒 Files selected for processing (3)
  • docs/nearby-bus-stops.md
  • docs/technical_debt.md
  • stationapi/src/use_case/interactor/query.rs
🧰 Additional context used
📓 Path-based instructions (5)
stationapi/src/use_case/**/*.rs

📄 CodeRabbit inference engine (AGENTS.md)

Application logic in use case layer should implement contract traits defined in traits/ module (e.g., QueryUseCase in interactor/query.rs)

Files:

  • stationapi/src/use_case/interactor/query.rs
stationapi/src/**/*.rs

📄 CodeRabbit inference engine (AGENTS.md)

Run cargo test --lib --package stationapi or make test-unit for unit tests focusing on entities and repository mocks without database

Files:

  • stationapi/src/use_case/interactor/query.rs
**/*.rs

📄 CodeRabbit inference engine (AGENTS.md)

**/*.rs: Before committing, run cargo fmt on all Rust code
Before committing, run cargo clippy --all-targets --all-features and resolve new Clippy warnings unless covered by existing #![allow] attributes

Files:

  • stationapi/src/use_case/interactor/query.rs
stationapi/src/use_case/interactor/query.rs

📄 CodeRabbit inference engine (AGENTS.md)

When modifying QueryInteractor, ensure enrichment steps (companies, train types, line symbols) still behave as expected and verify helper methods such as update_station_vec_with_attributes and build_route_tree_map

Files:

  • stationapi/src/use_case/interactor/query.rs
{docs/**/*.md,README.md}

📄 CodeRabbit inference engine (AGENTS.md)

For database, gRPC, or schema updates, add architectural notes under docs/ and synchronize README references to keep onboarding materials accurate

Files:

  • docs/technical_debt.md
  • docs/nearby-bus-stops.md
🧠 Learnings (5)
📓 Common learnings
Learnt from: CR
Repo: TrainLCD/StationAPI PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T10:50:36.694Z
Learning: Applies to stationapi/src/use_case/interactor/query.rs : When modifying `QueryInteractor`, ensure enrichment steps (companies, train types, line symbols) still behave as expected and verify helper methods such as `update_station_vec_with_attributes` and `build_route_tree_map`
📚 Learning: 2025-11-25T10:50:36.694Z
Learnt from: CR
Repo: TrainLCD/StationAPI PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T10:50:36.694Z
Learning: Applies to stationapi/src/use_case/interactor/query.rs : When modifying `QueryInteractor`, ensure enrichment steps (companies, train types, line symbols) still behave as expected and verify helper methods such as `update_station_vec_with_attributes` and `build_route_tree_map`

Applied to files:

  • stationapi/src/use_case/interactor/query.rs
  • docs/technical_debt.md
  • docs/nearby-bus-stops.md
📚 Learning: 2025-11-25T10:50:36.694Z
Learnt from: CR
Repo: TrainLCD/StationAPI PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T10:50:36.694Z
Learning: Applies to stationapi/src/use_case/**/*.rs : Application logic in use case layer should implement contract traits defined in `traits/` module (e.g., `QueryUseCase` in `interactor/query.rs`)

Applied to files:

  • stationapi/src/use_case/interactor/query.rs
📚 Learning: 2025-11-25T10:50:36.694Z
Learnt from: CR
Repo: TrainLCD/StationAPI PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T10:50:36.694Z
Learning: Applies to stationapi/src/**/*.rs : Run `cargo test --lib --package stationapi` or `make test-unit` for unit tests focusing on entities and repository mocks without database

Applied to files:

  • stationapi/src/use_case/interactor/query.rs
  • docs/technical_debt.md
📚 Learning: 2025-11-25T10:50:36.694Z
Learnt from: CR
Repo: TrainLCD/StationAPI PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T10:50:36.694Z
Learning: Applies to stationapi/src/infrastructure/**/*.rs : Integration tests in infrastructure layer must use `#[cfg_attr(not(feature = "integration-tests"), ignore)]` attribute to conditionally ignore tests when the feature is not enabled

Applied to files:

  • stationapi/src/use_case/interactor/query.rs
🧬 Code graph analysis (1)
stationapi/src/use_case/interactor/query.rs (6)
stationapi/src/domain/entity/train_type.rs (1)
  • new (26-58)
stationapi/src/domain/entity/line.rs (1)
  • new (48-120)
stationapi/src/domain/entity/station.rs (1)
  • new (84-218)
stationapi/src/domain/repository/station_repository.rs (1)
  • new (61-76)
stationapi/src/domain/entity/line_symbol.rs (1)
  • new (11-17)
stationapi/src/domain/entity/station_number.rs (1)
  • new (12-24)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: Test Suite
🔇 Additional comments (9)
stationapi/src/use_case/interactor/query.rs (5)

991-1003: build_route_tree_map の参照ベース最適化は良い改善です。

Vec<Station> から Vec<&'a Station> への変更により、不要な Station のクローンが回避されます。ライフタイムパラメータ 'a も正しく適用されています。

Based on learnings, build_route_tree_map のヘルパーメソッドが正しく動作することを確認してください。


820-887: TrainType の重複排除と HashMap 検索の最適化

seen_line_group_cds による重複排除と train_type_by_type_cd による O(1) 検索への変更は適切です。train_types の所有権を直接イテレーションで移動することで、不要なクローンも回避されています。


1097-1178: テストヘルパー関数の追加は良いアプローチです。

create_test_stationcreate_test_line ヘルパー関数により、テストコードの可読性と再利用性が向上しています。Station の全64フィールドを手動で設定する必要がなくなり、テスト作成が容易になります。


1224-1258: Haversine 距離計算のテストカバレッジ

同一点、長距離(東京-大阪間約400km)、短距離、および NEARBY_BUS_STOP_RADIUS_METERS 定数との境界テストが含まれており、適切なカバレッジです。


1435-1484: build_route_tree_map のテストで参照の整合性を確認

Line 1483 の std::ptr::eq による参照の同一性チェックは、戻り値が元のスライスへの参照であることを保証する良いテストです。

docs/technical_debt.md (1)

112-132: 技術負債の対応状況が明確に文書化されています。

HashMap ベースの検索最適化、build_route_tree_map の参照化、および残存する clone() の理由が適切に説明されています。実装コード(query.rs)と整合性が取れています。

docs/nearby-bus-stops.md (3)

7-7: デフォルト動作の変更が明確に文書化されています。

後方互換性のためデフォルトを鉄道駅のみに変更したことが明確に説明されています。PR目的(本番環境でバス路線が紛れ込んでいた問題の修正)と整合性があります。


104-106: ヘルパーメソッドのシグネチャ更新

get_nearby_bus_stops から get_nearby_bus_lines へのリネームと、戻り値型の Vec<Station> から Vec<Line> への変更が正しく反映されています。実装コード(query.rs Line 934-989)と整合性があります。


110-111: 注意事項の更新

transport_type: RailAndBus を明示的に指定した場合のみ近傍バス路線が追加される動作が明確に説明されています。これにより、デフォルト動作でバス路線が混入しないことが保証されます。

Comment thread stationapi/src/use_case/interactor/query.rs

@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.

Actionable comments posted: 0

🧹 Nitpick comments (1)
stationapi/src/use_case/interactor/query.rs (1)

262-262: HashMap からの効率的な参照取得を確認しました。

company_map を使用した O(1) ルックアップが正しく実装されています。.cloned().cloned() パターンは機能的に正しいですが、可読性のため .and_then(|c| Some(c.clone())).copied() を検討することもできます(ただし、現在のコードで問題はありません)。

Also applies to: 301-301

📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 082e4f6 and 2c85dc1.

📒 Files selected for processing (1)
  • stationapi/src/use_case/interactor/query.rs
🧰 Additional context used
📓 Path-based instructions (4)
stationapi/src/use_case/**/*.rs

📄 CodeRabbit inference engine (AGENTS.md)

Application logic in use case layer should implement contract traits defined in traits/ module (e.g., QueryUseCase in interactor/query.rs)

Files:

  • stationapi/src/use_case/interactor/query.rs
stationapi/src/**/*.rs

📄 CodeRabbit inference engine (AGENTS.md)

Run cargo test --lib --package stationapi or make test-unit for unit tests focusing on entities and repository mocks without database

Files:

  • stationapi/src/use_case/interactor/query.rs
**/*.rs

📄 CodeRabbit inference engine (AGENTS.md)

**/*.rs: Before committing, run cargo fmt on all Rust code
Before committing, run cargo clippy --all-targets --all-features and resolve new Clippy warnings unless covered by existing #![allow] attributes

Files:

  • stationapi/src/use_case/interactor/query.rs
stationapi/src/use_case/interactor/query.rs

📄 CodeRabbit inference engine (AGENTS.md)

When modifying QueryInteractor, ensure enrichment steps (companies, train types, line symbols) still behave as expected and verify helper methods such as update_station_vec_with_attributes and build_route_tree_map

Files:

  • stationapi/src/use_case/interactor/query.rs
🧠 Learnings (5)
📓 Common learnings
Learnt from: CR
Repo: TrainLCD/StationAPI PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T10:50:36.694Z
Learning: Applies to stationapi/src/use_case/interactor/query.rs : When modifying `QueryInteractor`, ensure enrichment steps (companies, train types, line symbols) still behave as expected and verify helper methods such as `update_station_vec_with_attributes` and `build_route_tree_map`
📚 Learning: 2025-11-25T10:50:36.694Z
Learnt from: CR
Repo: TrainLCD/StationAPI PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T10:50:36.694Z
Learning: Applies to stationapi/src/use_case/interactor/query.rs : When modifying `QueryInteractor`, ensure enrichment steps (companies, train types, line symbols) still behave as expected and verify helper methods such as `update_station_vec_with_attributes` and `build_route_tree_map`

Applied to files:

  • stationapi/src/use_case/interactor/query.rs
📚 Learning: 2025-11-25T10:50:36.694Z
Learnt from: CR
Repo: TrainLCD/StationAPI PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T10:50:36.694Z
Learning: Applies to stationapi/src/use_case/**/*.rs : Application logic in use case layer should implement contract traits defined in `traits/` module (e.g., `QueryUseCase` in `interactor/query.rs`)

Applied to files:

  • stationapi/src/use_case/interactor/query.rs
📚 Learning: 2025-11-25T10:50:36.694Z
Learnt from: CR
Repo: TrainLCD/StationAPI PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T10:50:36.694Z
Learning: Applies to stationapi/src/**/*.rs : Run `cargo test --lib --package stationapi` or `make test-unit` for unit tests focusing on entities and repository mocks without database

Applied to files:

  • stationapi/src/use_case/interactor/query.rs
📚 Learning: 2025-11-25T10:50:36.694Z
Learnt from: CR
Repo: TrainLCD/StationAPI PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T10:50:36.694Z
Learning: Applies to stationapi/src/infrastructure/**/*.rs : Integration tests in infrastructure layer must use `#[cfg_attr(not(feature = "integration-tests"), ignore)]` attribute to conditionally ignore tests when the feature is not enabled

Applied to files:

  • stationapi/src/use_case/interactor/query.rs
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: Test Suite
🔇 Additional comments (9)
stationapi/src/use_case/interactor/query.rs (9)

239-257: LGTM! HashMap による O(1) 検索への最適化が適切に実装されています。

company_maptrain_type_mapstation_lookup の導入により、従来の線形探索を O(1) のハッシュマップルックアップに置き換えています。特に station_lookup でタプルキー (line_cd, station_g_cd) を使用している点も適切です。

この変更により、エンリッチメント処理のパフォーマンスが大幅に向上します。

Based on learnings, enrichment steps (companies, train types, line symbols) の動作を確認しましたが、正しく機能しています。


268-275: train_type のエンリッチメントが正しく最適化されています。

train_type_map による O(1) ルックアップにより、各ステーションに対する TrainType の検索が効率化されています。Box へのラッピングも適切です。

Also applies to: 309-316


303-318: station_lookup による複合キー検索が適切に実装されています。

(line_cd, station_g_cd) のタプルキーを使用した O(1) ルックアップにより、ライン情報へのステーション埋め込みが効率化されています。必要な場合のみステーションをクローンしている点も効率的です。


612-621: 過去のレビュー指摘事項が適切に解決されています。

tt_line_map の導入により、get_routes メソッド内のループで発生していた O(n) 線形探索が O(1) ルックアップに最適化されました。.copied() の使用も効率的です。

過去のレビューコメントで指摘されていた線 617 の最適化が正しく実装されています。


823-840: train_type の事前マップ構築と重複排除ロジックが適切です。

train_type_by_type_cd マップの事前構築により、後続のライン埋め込み時の O(1) ルックアップが可能になっています。seen_line_group_cds による重複排除ロジックも正しく実装されており、同一 line_group_cd の重複を防いでいます。


878-880: TrainType の効率的な埋め込みを確認しました。

train_type_by_type_cd マップを使用した O(1) ルックアップにより、各ラインへの TrainType 埋め込みが効率化されています。


976-984: 近隣バス路線のバス停ルックアップが最適化されています。

bus_stop_by_line_cd マップの導入により、各バス路線に対応するバス停の検索が O(1) で実行できるようになっています。


994-1006: build_route_tree_map の参照版への変更は優れた最適化です。

戻り値を Vec<Station> から Vec<&'a Station> に変更することで、不要なクローンを削減しています。ライフタイム注釈も適切に追加されており、借用チェッカーが正しく機能します。

Based on learnings, build_route_tree_map ヘルパーメソッドの動作を確認しましたが、正しく機能しています。


1101-1636: 包括的なテストカバレッジの追加を評価します。

以下の機能に対する詳細なテストが追加されています:

  • haversine_distance: 同一地点、長距離、短距離、バス停半径内外のケース
  • build_route_tree_map: 空配列、line_group_cd によるグループ化、line_cd によるグループ化、参照の正しさ
  • get_station_numbers: 単一番号、複数番号、空の場合、シンボルなしの場合
  • get_line_symbols: 複数シンボル、フォールバックカラー、空の場合
  • extract_line_from_station: 基本的な抽出、station_cd の保持、オプショナルフィールドの処理

モックリポジトリとテストヘルパー関数も適切に実装されており、テストの品質が高いです。

As per coding guidelines, cargo test --lib --package stationapi でユニットテストを実行して確認することを推奨します。

@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.

Actionable comments posted: 1

🧹 Nitpick comments (1)
stationapi/src/use_case/interactor/query.rs (1)

262-262: HashMap からの取得パターンが機能していますが、よりクリーンな書き方を検討できます。

.get().cloned().cloned() のパターンは正しく動作しますが、やや冗長です。以下のようなよりクリーンな代替案を検討できます:

🔎 オプションのリファクタリング案
-line.company = company_map.get(&line.company_cd).cloned().cloned();
+line.company = company_map.get(&line.company_cd).map(|c| (*c).clone());

または

-line.company = company_map.get(&line.company_cd).cloned().cloned();
+line.company = company_map.get(&line.company_cd).copied();

CompanyCopy トレイトを実装している場合)

Also applies to: 268-275, 301-301, 309-316

📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 2c85dc1 and 51be8a3.

📒 Files selected for processing (1)
  • stationapi/src/use_case/interactor/query.rs
🧰 Additional context used
📓 Path-based instructions (4)
stationapi/src/use_case/**/*.rs

📄 CodeRabbit inference engine (AGENTS.md)

Application logic in use case layer should implement contract traits defined in traits/ module (e.g., QueryUseCase in interactor/query.rs)

Files:

  • stationapi/src/use_case/interactor/query.rs
stationapi/src/**/*.rs

📄 CodeRabbit inference engine (AGENTS.md)

Run cargo test --lib --package stationapi or make test-unit for unit tests focusing on entities and repository mocks without database

Files:

  • stationapi/src/use_case/interactor/query.rs
**/*.rs

📄 CodeRabbit inference engine (AGENTS.md)

**/*.rs: Before committing, run cargo fmt on all Rust code
Before committing, run cargo clippy --all-targets --all-features and resolve new Clippy warnings unless covered by existing #![allow] attributes

Files:

  • stationapi/src/use_case/interactor/query.rs
stationapi/src/use_case/interactor/query.rs

📄 CodeRabbit inference engine (AGENTS.md)

When modifying QueryInteractor, ensure enrichment steps (companies, train types, line symbols) still behave as expected and verify helper methods such as update_station_vec_with_attributes and build_route_tree_map

Files:

  • stationapi/src/use_case/interactor/query.rs
🧠 Learnings (5)
📓 Common learnings
Learnt from: CR
Repo: TrainLCD/StationAPI PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T10:50:36.694Z
Learning: Applies to stationapi/src/use_case/interactor/query.rs : When modifying `QueryInteractor`, ensure enrichment steps (companies, train types, line symbols) still behave as expected and verify helper methods such as `update_station_vec_with_attributes` and `build_route_tree_map`
📚 Learning: 2025-11-25T10:50:36.694Z
Learnt from: CR
Repo: TrainLCD/StationAPI PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T10:50:36.694Z
Learning: Applies to stationapi/src/use_case/interactor/query.rs : When modifying `QueryInteractor`, ensure enrichment steps (companies, train types, line symbols) still behave as expected and verify helper methods such as `update_station_vec_with_attributes` and `build_route_tree_map`

Applied to files:

  • stationapi/src/use_case/interactor/query.rs
📚 Learning: 2025-11-25T10:50:36.694Z
Learnt from: CR
Repo: TrainLCD/StationAPI PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T10:50:36.694Z
Learning: Applies to stationapi/src/use_case/**/*.rs : Application logic in use case layer should implement contract traits defined in `traits/` module (e.g., `QueryUseCase` in `interactor/query.rs`)

Applied to files:

  • stationapi/src/use_case/interactor/query.rs
📚 Learning: 2025-11-25T10:50:36.694Z
Learnt from: CR
Repo: TrainLCD/StationAPI PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T10:50:36.694Z
Learning: Applies to stationapi/src/**/*.rs : Run `cargo test --lib --package stationapi` or `make test-unit` for unit tests focusing on entities and repository mocks without database

Applied to files:

  • stationapi/src/use_case/interactor/query.rs
📚 Learning: 2025-11-25T10:50:36.694Z
Learnt from: CR
Repo: TrainLCD/StationAPI PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T10:50:36.694Z
Learning: Applies to stationapi/src/infrastructure/**/*.rs : Integration tests in infrastructure layer must use `#[cfg_attr(not(feature = "integration-tests"), ignore)]` attribute to conditionally ignore tests when the feature is not enabled

Applied to files:

  • stationapi/src/use_case/interactor/query.rs
🧬 Code graph analysis (1)
stationapi/src/use_case/interactor/query.rs (7)
stationapi/src/domain/entity/line.rs (1)
  • new (48-120)
stationapi/src/domain/entity/train_type.rs (1)
  • new (26-58)
stationapi/src/domain/entity/station.rs (1)
  • new (84-218)
stationapi/src/domain/repository/line_repository.rs (5)
  • new (51-185)
  • find_by_id (7-7)
  • find_by_id (190-192)
  • get_by_line_group_id (18-18)
  • get_by_line_group_id (232-238)
stationapi/src/infrastructure/company_repository.rs (3)
  • new (49-51)
  • find_by_id_vec (56-60)
  • find_by_id_vec (66-87)
stationapi/src/domain/entity/company.rs (1)
  • new (21-49)
stationapi/src/domain/repository/train_type_repository.rs (3)
  • new (42-97)
  • get_by_line_group_id (7-8)
  • get_by_line_group_id (102-113)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: Test Suite
  • GitHub Check: Test Suite
🔇 Additional comments (6)
stationapi/src/use_case/interactor/query.rs (6)

239-257: HashMap による O(1) 検索への最適化は適切です。

company_map、train_type_map、station_lookup の3つの HashMap を導入することで、後続の線形探索を O(1) 検索に置き換えています。参照を使用することでマップ構築時のクローンも回避されており、パフォーマンス改善として正しい実装です。


303-318: station_lookup の使用方法は適切です。

複合キー (line_cd, station_g_cd) を使用した HashMap 検索が正しく実装されており、取得したステーションの clone と enrichment 処理も適切に行われています。


612-621: 過去のレビューコメントで指摘された最適化が正しく実装されています。

tt_line_map による O(1) 検索への置き換えが適切に行われており、.copied() を使用することでより簡潔な実装になっています。以前の線形探索 tt_lines.iter().find(...) からの改善として優れています。


823-840: train_type_by_type_cd マップを使った最適化は効果的です。

事前にマップを構築することで、ループ内での繰り返し検索を回避しています。また、seen_line_group_cds による重複排除ロジックも正しく実装されており、パフォーマンス向上に寄与しています。

Also applies to: 878-880, 890-890


976-989: 近隣バス停のマップ化による最適化は適切です。

bus_stop_by_line_cd マップを使用することで、各路線に対する最も近いバス停の検索を効率化しています。重複排除のロジックも明確にコメントされており、実装として優れています。


999-1011: build_route_tree_map の参照ベースへの変更は優れた最適化です。

Vec<Station> から Vec<&'a Station> への変更により、不要なクローンを回避しており、ライフタイム注釈 'a も適切に設定されています。

呼び出し元(lines 592, 696)は参照を正しく処理しており、ループ内で stops.iter().filter_map(|row| row.line_group_cd...) のように参照のフィールドに直接アクセスしています。また、包括的なユニットテスト(lines 1444-1490)で参照の返却と処理が検証されており、メソッドは期待通りに動作しています。

Comment thread stationapi/src/use_case/interactor/query.rs
@TinyKitten TinyKitten merged commit 9a9b851 into dev Jan 5, 2026
10 checks passed
@TinyKitten TinyKitten deleted the fix/debt-and-bugs branch January 5, 2026 00:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant