GetConnectedRoutesで複数列車種別を跨ぐ経路を返す - #1615
Conversation
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 50 minutes Your organization has reached its usage spending cap. Adjust your spending cap in the billing tab. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
📝 WalkthroughWalkthrough
Changes接続路線経路探索
Estimated code review effort: 4 (Complex) | ~45 minutes Suggested labels: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (6)
stationapi/src/use_case/interactor/query.rs (3)
4946-4966: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win循環経路が除外されることを直接検証してください。
テスト名は循環の処理を示します。しかし現在の assert は ID の決定性と一意性のみを確認します。
line_group400 は駅グループ 2 から駅グループ 1 へ戻ります。この経路が結果に含まれないことを明示的に確認してください。現在の assert は、循環除外が壊れても検知しません。♻️ 追加する assert の例
let unreachable = interactor.get_connected_stations(1, 99).await.unwrap(); assert!(unreachable.is_empty()); + + // 循環: どの経路も始点の駅グループ 1 を 2 回以上含まない。 + for route in &first { + assert_eq!( + route.stops.iter().filter(|stop| stop.group_id == 1).count(), + 1, + "origin station group must appear once per route" + ); + } }🤖 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/interactor/query.rs` around lines 4946 - 4966, Update test_get_connected_stations_is_deterministic_and_handles_cycles_and_no_route to explicitly assert that the returned routes do not include the cyclic line_group 400 route, while preserving the existing determinism, uniqueness, and unreachable-station assertions.
1369-1392: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
line_group_cdの重複代入を削除してください。行 1373 で
line_group_cd: Some(virtual_line_group_id as i32)を設定します。行 1386 で同じ値を再度代入します。2 回目の代入は効果がありません。train_typeをmutにする必要も無くなります。♻️ 提案する修正
- let mut train_type = TrainType { + let train_type = TrainType { id: row.type_id, @@ lines: vec![extracted_line.clone()], }; - train_type.line_group_cd = Some(virtual_line_group_id as i32); let mut stop = self.build_station_from_row(🤖 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/interactor/query.rs` around lines 1369 - 1392, Remove the redundant line_group_cd reassignment after the TrainType initializer and make train_type immutable, while preserving the initial field value and the subsequent stop.line_group_cd assignment.
1188-1195: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value引数名が駅グループIDの意味と一致しません。
grpc.rsはfrom_station_group_idとto_station_group_idを渡します。この関数内でもstop.station_g_cdと比較します。しかし引数名はfrom_station_id/to_station_idです。読み手が駅ID(station_cd)と誤解します。traits/query.rsの宣言と合わせてfrom_station_group_id/to_station_group_idに改名することを推奨します。🤖 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/interactor/query.rs` around lines 1188 - 1195, Rename the get_connected_stations parameters from_station_id and to_station_id to from_station_group_id and to_station_group_id, update their equality check and all references within the function, and apply the same names to the corresponding declaration in the query trait so the station-group ID meaning is consistent.AGENTS.md (1)
62-62: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value循環防止の説明を上限の説明から分けてください。
この文は上限が循環を防ぐと読めます。実装では、循環の防止は訪問済み駅グループと使用済み列車種別の再訪禁止が担当します。探索上限は計算量の暴走のみを防ぎます。両者を分けて記述してください。
✏️ 提案する修正
-The search is capped at eight train types, 4,096 expanded states, and 32 results to prevent cycles and runaway computation. +Cycles are prevented by rejecting already visited station groups and already used train types. The search is additionally capped at eight train types, 4,096 expanded states, and 32 results to bound computation.🤖 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 `@AGENTS.md` at line 62, Update the Connected routes documentation around GetConnectedRoutes to separate cycle prevention from resource limits: state that revisiting station groups and already-used train types prevents cycles, while the eight-train-type, 4,096-state, and 32-result caps only bound computation and result size.stationapi/src/infrastructure/train_type_repository.rs (1)
615-625: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueテスト用
stationsテーブルにline_cd列を追加してください。
InternalTrainTypeRepository::get_by_line_group_id_and_line_idはstations.line_cdを参照します。このテスト用スキーマにはline_cd列がありません。現在そのメソッドの統合テストは存在しないため失敗しませんが、テストを追加した時点で SQL エラーになります。本番スキーマ (data/create_table.sql) との差分を減らすため、列を先に追加することを推奨します。♻️ 提案する修正
"CREATE TABLE stations ( station_cd INTEGER PRIMARY KEY, station_g_cd INTEGER NOT NULL, + line_cd INTEGER, e_status INTEGER NOT NULL DEFAULT 0 )",🤖 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/infrastructure/train_type_repository.rs` around lines 615 - 625, テスト用のstationsテーブル定義を更新し、InternalTrainTypeRepository::get_by_line_group_id_and_line_idが参照するline_cd列を追加してください。既存のstation_cd、station_g_cd、e_status列と制約は維持し、本番スキーマと整合する列定義にしてください。stationapi/src/use_case/traits/query.rs (1)
127-131: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winメソッド名を戻り値の型に合わせてください。
get_connected_stationsはVec<Route>を返すようになりました。名前はStationを示しますが、返すのはRouteです。gRPC のエンドポイント名もGetConnectedRoutesです。get_connected_routesへの改名を推奨します。改名箇所は本トレイト、QueryInteractorの実装、grpc.rsの呼び出しとモックの 4 箇所です。🤖 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/traits/query.rs` around lines 127 - 131, Rename the trait method get_connected_stations to get_connected_routes and apply the same rename to the QueryInteractor implementation, the grpc.rs call site, and all related mocks, preserving the existing Vec<Route> behavior.
🤖 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.
Inline comments:
In `@stationapi/src/use_case/interactor/query.rs`:
- Around line 1296-1351: 状態上限の判定前に候補ごとの割り当てを行わないよう、query.rs の状態展開処理で next_states
の空き枠を確認してから state.stops.clone() と state.visited_station_groups.clone()
を実行するか、内側ループの候補展開数を上限に計上してください。docs/architecture.md
の該当記述も更新し、CONNECTED_ROUTE_MAX_STATES が状態数だけを制限し、各状態内の start_indices ×
pattern.len() による候補生成コストや最悪計算量は別途分岐数に依存することを明記してください。
---
Nitpick comments:
In `@AGENTS.md`:
- Line 62: Update the Connected routes documentation around GetConnectedRoutes
to separate cycle prevention from resource limits: state that revisiting station
groups and already-used train types prevents cycles, while the eight-train-type,
4,096-state, and 32-result caps only bound computation and result size.
In `@stationapi/src/infrastructure/train_type_repository.rs`:
- Around line 615-625:
テスト用のstationsテーブル定義を更新し、InternalTrainTypeRepository::get_by_line_group_id_and_line_idが参照するline_cd列を追加してください。既存のstation_cd、station_g_cd、e_status列と制約は維持し、本番スキーマと整合する列定義にしてください。
In `@stationapi/src/use_case/interactor/query.rs`:
- Around line 4946-4966: Update
test_get_connected_stations_is_deterministic_and_handles_cycles_and_no_route to
explicitly assert that the returned routes do not include the cyclic line_group
400 route, while preserving the existing determinism, uniqueness, and
unreachable-station assertions.
- Around line 1369-1392: Remove the redundant line_group_cd reassignment after
the TrainType initializer and make train_type immutable, while preserving the
initial field value and the subsequent stop.line_group_cd assignment.
- Around line 1188-1195: Rename the get_connected_stations parameters
from_station_id and to_station_id to from_station_group_id and
to_station_group_id, update their equality check and all references within the
function, and apply the same names to the corresponding declaration in the query
trait so the station-group ID meaning is consistent.
In `@stationapi/src/use_case/traits/query.rs`:
- Around line 127-131: Rename the trait method get_connected_stations to
get_connected_routes and apply the same rename to the QueryInteractor
implementation, the grpc.rs call site, and all related mocks, preserving the
existing Vec<Route> behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d5f6c3cc-6028-4012-898a-e9f759ffe56e
📒 Files selected for processing (8)
AGENTS.mddocs/architecture.mddocs/technical_debt.mdstationapi/src/domain/repository/train_type_repository.rsstationapi/src/infrastructure/train_type_repository.rsstationapi/src/presentation/controller/grpc.rsstationapi/src/use_case/interactor/query.rsstationapi/src/use_case/traits/query.rs
💤 Files with no reviewable changes (1)
- docs/technical_debt.md
1 participant
概要
GetConnectedRoutesが空配列を返していた未実装状態を解消し、複数の列車種別を乗り継ぐ経路候補を返せるようにします。変更の種類
変更内容
line_group_cdを階層単位で一括取得する Repository API と PostgreSQL 実装を追加lineGroupIdを生成し、全駅のtrain_type.group_idへ設定GetConnectedRoutespresentation 層を複数候補および到達不能時の空配列に対応リスクは候補数の多いネットワークにおける探索量と、仮想 ID の衝突です。探索量は明示的な3種類の上限で制御し、仮想 ID は既存 PostgreSQL
INTEGERと重ならないuint32上位半分を使用したうえで候補内の衝突を解消します。テスト
cargo fmt --all -- --checkが通ることcargo clippy -- -D warningsが通ることcargo test（SQLX_OFFLINE=true）が通ること実行結果:
cargo fmt --all -- --check: 成功SQLX_OFFLINE=true cargo clippy --all-targets --all-features -- -D warnings: 成功SQLX_OFFLINE=true cargo test --lib --package stationapi: 427 passed / 0 failed / 56 ignored.env.testの資格情報が実 DB と一致しないため実行は未完了関連Issue
Closes #1611
スクリーンショット（任意）
API のバックエンド変更のため該当なし。