EstimateArrivalTimesの入力をstation_idに変更しvia_line_idsを配列化 - #1570
Conversation
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 6 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. 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 (5)
📝 WalkthroughWalkthrough経由路線指定を Changes経由路線の複数指定対応
Sequence Diagram(s)sequenceDiagram
participant GrpcController
participant QueryInteractor
participant StationRepository
participant Database
GrpcController->>QueryInteractor: estimate_route_arrival_times(via_line_ids)
QueryInteractor->>StationRepository: get_route_stops(from, to, via_line_ids)
StationRepository->>Database: SQL ANY(via_line_ids) on station_cd
Database-->>StationRepository: 該当駅一覧
StationRepository-->>QueryInteractor: Vec<Station>
QueryInteractor-->>GrpcController: EstimateArrivalTimesResponse
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
stationapi/src/infrastructure/station_repository.rs (1)
1945-1954: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift既存 route 系 RPC の group ID 契約を壊しています。
Line 1945/1954 と Line 2092/2102 が
station_cdに変わっていますが、既存のget_routes/get_routes_minimal/get_route_typesはfrom_station_group_id/to_station_group_idを渡す経路のままです。station_g_cd != station_cdの駅で経路が取れなくなるため、EstimateArrivalTimes 用の station-id 経路と既存 group-id 経路を分けるか、呼び出し側の契約も揃えてください。Also applies to: 2092-2102
🤖 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/station_repository.rs` around lines 1945 - 1954, The route RPC query has been switched to use station_cd in places where the existing get_routes, get_routes_minimal, and get_route_types contract still passes from_station_group_id/to_station_group_id, which breaks stations where station_g_cd and station_cd differ. Update the relevant query-building logic in station_repository::get_routes-related code to keep the existing group-id path intact, or split the EstimateArrivalTimes station-id path into a separate query/entry point so both contracts remain consistent. Use the existing symbols get_routes, get_routes_minimal, get_route_types, and the CTEs around from_cte/to_cte to locate the affected SQL.
🧹 Nitpick comments (2)
stationapi/src/domain/repository/station_repository.rs (1)
333-344: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win
station_cd != station_g_cdの経路テストを追加してください。Line 343 の fixture が常に
station_g_cd = station_cdなので、今回の station-id / group-id 契約差分をこのテスト群では検出できません。少なくとも 1 ケースは別値にして、get_route_stopsが station ID 前提なのか group ID 前提なのかを明示してください。Also applies to: 502-541
🤖 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/repository/station_repository.rs` around lines 333 - 344, Add a test case in the Station repository tests where create_test_station (or the relevant fixture setup used by get_route_stops) sets station_cd and station_g_cd to different values instead of always mirroring them. Update one of the route/path test scenarios around get_route_stops to assert the expected behavior under this mismatch so the station ID vs group ID contract is explicitly covered.stationapi/src/use_case/interactor/query.rs (1)
2322-2322: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winstation_cd と station_g_cd が異なるケースも追加してください。
到着推定は station_id ベースへ変わりましたが、現テストの helper は
station_cd == station_g_cdなので Line 1108 の差分を検証できません。片方だけ異なる駅データで、group_id ではなく station_id でフィルタされることを固定すると安全です。Also applies to: 2360-2360
🤖 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` at line 2322, The existing route arrival time tests only cover the case where station_cd and station_g_cd are the same, so they do not verify the new station_id-based filtering behavior. Update the affected tests around estimate_route_arrival_times to add a fixture/helper case where station_cd differs from station_g_cd, and assert that the logic still filters by station_id rather than group_id. Make sure the new coverage is added in the query.rs test area that exercises estimate_route_arrival_times so the station_id change is fixed by a test that would fail under the old helper data.
🤖 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 629-632: The route lookup path is mixing group-based IDs with a
repository method that now filters by station_cd, so existing gRPC callers using
from_station_group_id/to_station_group_id will miss routes when station_cd
differs from station_g_cd. Update the query flow in get_routes,
get_routes_minimal, get_train_types, and the related get_route_stops call sites
to keep a group_id-based repository query for these APIs, or rename the
API/trait inputs so they consistently use station_id/station_cd end to end. Use
the symbols get_route_stops, get_routes, get_routes_minimal, and get_train_types
to locate and align the contract.
---
Outside diff comments:
In `@stationapi/src/infrastructure/station_repository.rs`:
- Around line 1945-1954: The route RPC query has been switched to use station_cd
in places where the existing get_routes, get_routes_minimal, and get_route_types
contract still passes from_station_group_id/to_station_group_id, which breaks
stations where station_g_cd and station_cd differ. Update the relevant
query-building logic in station_repository::get_routes-related code to keep the
existing group-id path intact, or split the EstimateArrivalTimes station-id path
into a separate query/entry point so both contracts remain consistent. Use the
existing symbols get_routes, get_routes_minimal, get_route_types, and the CTEs
around from_cte/to_cte to locate the affected SQL.
---
Nitpick comments:
In `@stationapi/src/domain/repository/station_repository.rs`:
- Around line 333-344: Add a test case in the Station repository tests where
create_test_station (or the relevant fixture setup used by get_route_stops) sets
station_cd and station_g_cd to different values instead of always mirroring
them. Update one of the route/path test scenarios around get_route_stops to
assert the expected behavior under this mismatch so the station ID vs group ID
contract is explicitly covered.
In `@stationapi/src/use_case/interactor/query.rs`:
- Line 2322: The existing route arrival time tests only cover the case where
station_cd and station_g_cd are the same, so they do not verify the new
station_id-based filtering behavior. Update the affected tests around
estimate_route_arrival_times to add a fixture/helper case where station_cd
differs from station_g_cd, and assert that the logic still filters by station_id
rather than group_id. Make sure the new coverage is added in the query.rs test
area that exercises estimate_route_arrival_times so the station_id change is
fixed by a test that would fail under the old helper data.
🪄 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: 9ac2622a-c4a5-4837-b488-c0c81f66cb4a
📒 Files selected for processing (10)
.sqlx/query-425870f89d0a0ae2c79e80ef7a468c5ef805d7a3f98b56e1b6ad93a1554683b7.json.sqlx/query-6c3b77baaf7b51c4986b8d2a4cfb3f94275ee3595d31aa43938cc1bbd43f0268.json.sqlx/query-c153bbe0cade8f513aeef1475527327a7240e45dd257123047ed4b4618c3c329.json.sqlx/query-cce67d0ec979a71c4e7de242d6c38589c009ae80ee0bc865f5e98a81699c18ee.jsonstationapi/protostationapi/src/domain/repository/station_repository.rsstationapi/src/infrastructure/station_repository.rsstationapi/src/presentation/controller/grpc.rsstationapi/src/use_case/interactor/query.rsstationapi/src/use_case/traits/query.rs
…tops_by_station_cdを新設 get_routes/get_routes_minimal/get_train_typesはstation_group_idを受け取るため station_g_cdで検索する必要がある。station_cdで検索するのはEstimateArrivalTimes専用。 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Fixes Applied SuccessfullyFixed 3 file(s) based on 1 CodeRabbit feedback item(s). Issue fixed:
Files modified:
Commit: The latest autofix changes are on the |
|
@coderabbitai review |
✅ Action performedReview finished.
|
1 participant
概要
EstimateArrivalTimesRPC の入力を明示的なstation_idベースに変更し、via_line_idを配列化。実用上、両端の駅IDがわからないままクエリを投げるケースが無いためstation_group_id→station_idに変更。直通運転対応のためvia_line_idをrepeated via_line_idsに配列化。変更の種類
変更内容
EstimateArrivalTimesRequestメッセージを新設（from_station_id,to_station_id,repeated via_line_ids）EstimateArrivalTimesRPC のリクエスト型をGetRouteRequest→EstimateArrivalTimesRequestに変更via_line_id: Option<u32>→via_line_ids: &[u32]に変更station_g_cd→station_cdに変更、via_line_idフィルタをANY($5::int[])による配列フィルタに変更station_g_cd→station_cdに変更cargo sqlx prepare --workspaceで再生成テスト
cargo fmt --all -- --checkが通ることcargo clippy -- -D warningsが通ることcargo test（SQLX_OFFLINE=true）が通ること関連Issue
スクリーンショット（任意）
Summary by CodeRabbit