バス経路 池86などのshape_id跨ぎによる停留所順序の崩れを修正#1512
Merged
Merged
Conversation
…バグを修正 前PR #1511 ではroute全体のshape_dist_traveledカバー率が80%以上だと shape_dist_traveledの値で並べ替えを行っていたが、同一route内でも trip毎にshape_idが異なる場合（短縮便・出入庫便など）shape_dist_traveled の値は互いに比較できない（各shape毎に0から再計測される）。結果として 池86のような多バリアント系統で、本来途中の停留所が「最も0に近い」値を 持ち先頭に並んでしまう不具合が発生していた。 修正方針: - shape_dist_traveledを跨trip比較するパスを完全に削除。 - 代わりに各routeのcanonical_shape_id（最長direction_id=0 shape）を計算し、 main_trip選択時に「canonical shapeに乗っているtripを優先」する条件を追加。 - main_tripのstop_sequenceは同一trip内で必ず単調増加するため、shape_idを 跨ぐ比較を一切行わずに信頼できる正典順序を得られる。 - バリアントのみの停留所は従来通りLAG/LEAD補間でメイン上の位置に 挿入する。 戻り値の型・既存呼び出し元には変更なし。
Contributor
📝 WalkthroughWalkthrough
ChangesStop-Route Mapping SQL Refactor
推定レビュー時間🎯 4 (複雑) | ⏱️ ~45分 関連する可能性のあるPR
推奨ラベル
Poem
🚥 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)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
stationapi/src/import.rs (1)
1736-1748:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift再帰の深さ上限 10 だと長い枝線で順序崩れが残ります。
prev_chain/next_chainをdepth < 10で打ち切っているので、メイン系統に戻るまでに 11 停留所以上ある出入庫便・枝線ではアンカー解決に失敗します。その場合、中間停留所でもestimated_seqが末尾側のフォールバックに落ちて、今回の不具合が別パターンで再発します。visitedで循環自体は防げているので、固定値ではなく trip 長に応じた上限にするか、少なくとも実データを十分にカバーする値にしたいです。Also applies to: 1782-1793
🤖 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/import.rs` around lines 1736 - 1748, The fixed recursion cutoff "depth < 10" in the CTEs (prev_chain/next_chain) causes anchor resolution to fail for long branch/garage trips; replace the hardcoded 10 with a dynamic max depth derived from the trip's length (for example based on array_length(pc.visited) or the known stop count for the route) and use that computed max_depth in the WHERE clause (i.e., change pc.depth < 10 to pc.depth < max_depth); apply the same change to both the prev_chain and next_chain occurrences and ensure visited-based cycle prevention is kept (visited, estimated_seq, main_trip_stops references remain unchanged).
🧹 Nitpick comments (1)
stationapi/src/import.rs (1)
1567-1602: ⚡ Quick winこの並び替え戦略は回帰テストで固定しておきたいです。
今回の修正は
canonical_shape選択と variant 補間の組み合わせが本体なので、shape_id跨ぎの系統と長めの variant-only 連鎖を含む fixture を 1 本でも入れておくと、次の SQL 調整で壊れにくくなります。特に今回の池86系のようなケースはテスト名だけでも意図が伝わります。Also applies to: 1603-1903
🤖 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/import.rs` around lines 1567 - 1602, Add a regression test that locks the sorting/selection behavior exercised by build_stop_route_mapping: create a fixture GTFS route with multiple shape_ids including one canonical long direction_id=0 shape and variant trips that form a multi-stop variant-only chain (include an example named like "ike86" / "池86" so intent is clear), then assert the chosen canonical_shape and main trip selection (prefers canonical shape, direction_id=0, most stops, longest shape_dist_traveled, deterministic trip_id tie-break) and verify variant stop interpolation positions produce the expected stop_sequence mapping; add the test near the existing import tests covering lines ~1603-1903 and make it deterministic and self-contained so future SQL tweaks will fail the test if behavior regresses.
🤖 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.
Outside diff comments:
In `@stationapi/src/import.rs`:
- Around line 1736-1748: The fixed recursion cutoff "depth < 10" in the CTEs
(prev_chain/next_chain) causes anchor resolution to fail for long branch/garage
trips; replace the hardcoded 10 with a dynamic max depth derived from the trip's
length (for example based on array_length(pc.visited) or the known stop count
for the route) and use that computed max_depth in the WHERE clause (i.e., change
pc.depth < 10 to pc.depth < max_depth); apply the same change to both the
prev_chain and next_chain occurrences and ensure visited-based cycle prevention
is kept (visited, estimated_seq, main_trip_stops references remain unchanged).
---
Nitpick comments:
In `@stationapi/src/import.rs`:
- Around line 1567-1602: Add a regression test that locks the sorting/selection
behavior exercised by build_stop_route_mapping: create a fixture GTFS route with
multiple shape_ids including one canonical long direction_id=0 shape and variant
trips that form a multi-stop variant-only chain (include an example named like
"ike86" / "池86" so intent is clear), then assert the chosen canonical_shape and
main trip selection (prefers canonical shape, direction_id=0, most stops,
longest shape_dist_traveled, deterministic trip_id tie-break) and verify variant
stop interpolation positions produce the expected stop_sequence mapping; add the
test near the existing import tests covering lines ~1603-1903 and make it
deterministic and self-contained so future SQL tweaks will fail the test if
behavior regresses.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 0115ff8d-186d-451a-9903-71d41e304e61
📒 Files selected for processing (1)
stationapi/src/import.rs
This was referenced May 14, 2026
This was referenced May 14, 2026
Merged
10 tasks
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.
概要
PR #1511 の追跡修正。
shape_dist_traveledを route 内全 trip 横断で比較していたため、shape_idが異なる短縮便・出入庫便を含む系統（池86 など）で停留所順序が崩れていた問題を、canonical_shape をメイントリップ選択のバイアスに使う方式へ切り替えて修正します。変更の種類
変更内容
shape_dist_traveledを route 全体で集計して並べ替えるshape_stop_dist/shape_numberedパスを削除。同一 route 内でもshape_idが違うとshape_dist_traveledの値は比較できない（短縮便・出入庫便は各 shape の 0 から再計測される）ため、池86 のような系統で中盤の停留所が「最も 0 に近い」値を持ち先頭に紛れ込む不具合が生じていました。main_tripsのORDER BY先頭に「canonical shape に乗っている trip を最優先」する条件を追加。メイントリップのstop_sequenceは同一 trip 内で必ず単調増加するため、shape を跨ぐ比較を行わずに信頼できる正典順序が得られます。integrate_gtfs_stops_to_stations()のインターフェースは変更していないため、stations.e_sortを読む既存エンドポイント（GetRoutes/GetRoutesMinimal/GetStationsByLineId/GetStationsByLineIdList）すべてに自動で反映されます。stations WHERE transport_type = 1を削除して GTFS を再 import する必要があります（コンテナ再起動時にバックグラウンドで自走）。テスト
cargo fmt --all -- --checkが通ることcargo clippy -- -D warningsが通ることcargo test（SQLX_OFFLINE=true）が通ることSQLX_OFFLINE=true cargo testは 327 件パス（53 件 ignore はもとから DB 接続前提）。関連Issue
スクリーンショット（任意）
Generated by Claude Code
Summary by CodeRabbit
リリースノート