From af470936a720a9656b2d0b22da71d18a5888fc2f Mon Sep 17 00:00:00 2001 From: Tsubasa SEKIGUCHI Date: Sun, 12 Jul 2026 21:12:16 +0900 Subject: [PATCH 1/2] =?UTF-8?q?GTFS=E3=81=AE=E4=B8=8A=E4=B8=8B=E3=83=9D?= =?UTF-8?q?=E3=83=BC=E3=83=AB=E3=82=92=E5=90=8C=E5=90=8D+=E8=BF=91?= =?UTF-8?q?=E6=8E=A5=E3=81=A7=E3=82=B0=E3=83=AB=E3=83=BC=E3=83=97=E5=8C=96?= =?UTF-8?q?=E3=81=97=E3=83=90=E3=82=B9=E5=81=9C=E3=81=AE=E9=87=8D=E8=A4=87?= =?UTF-8?q?=E8=A1=A8=E7=A4=BA=E3=82=92=E4=BF=AE=E6=AD=A3?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit parent_stationの階層を持たないフィード(西武・東急コミュニティバス・京王等) では上下ポールが別々のトップレベルstopとして取り込まれ、同名なのに別 station_g_cdになるため同じバス停名が2件表示されていた。同名かつ250m以内の stopをunion-findでまとめ、クラスタ代表(最小stop_id)のstation_g_cdを共有させる。 単独stopは従来値と一致するため都営など正しく動くフィードには影響しない。 Co-Authored-By: Claude Opus 4.8 --- stationapi/src/import.rs | 144 ++++++++++++++++++++++++++++++++++++++- 1 file changed, 143 insertions(+), 1 deletion(-) diff --git a/stationapi/src/import.rs b/stationapi/src/import.rs index 5770c740..414d296b 100644 --- a/stationapi/src/import.rs +++ b/stationapi/src/import.rs @@ -5,6 +5,7 @@ use serde::de::{DeserializeOwned, DeserializeSeed, SeqAccess, Visitor}; use serde::{Deserialize, Serialize}; use sqlx::{Connection, PgConnection}; use stationapi::config::fetch_database_url; +use stationapi::domain::arrival_estimation::haversine_distance; use stationapi::domain::romaji::{romaji_display_name, strip_macrons}; use std::collections::{HashMap, HashSet}; use std::fmt; @@ -2227,6 +2228,85 @@ fn generate_bus_station_g_cd(stop_id: &str) -> i32 { 200_000_000 + (hash % 100_000_000) as i32 } +/// Distance within which two like-named bus stops are treated as direction +/// poles of a single physical stop and merged into one `station_g_cd`. +const BUS_STOP_GROUPING_RADIUS_METERS: f64 = 250.0; + +/// Build a `stop_id -> station_g_cd` map that collapses the separate direction +/// poles of one physical bus stop into a single group. +/// +/// Some GTFS feeds encode poles with a `parent_station` hierarchy (Toei), in +/// which case only the parent is imported and grouping is already correct. Other +/// feeds (Seibu, the Tokyu community buses, and typically Keio) expose every pole +/// as its own top-level stop, so the same bus stop name would otherwise appear +/// multiple times in the app. Here we group stops that share a name *and* sit +/// within `BUS_STOP_GROUPING_RADIUS_METERS` of each other, mirroring how rail +/// stations collapse across companies via `station_g_cd`. +/// +/// Proximity scoping keeps genuinely distinct stops that merely share a common +/// name (e.g. "新道" in different cities) in separate groups. A lone stop yields +/// the exact same `station_g_cd` as [`generate_bus_station_g_cd`], so feeds that +/// already group correctly are unaffected. +fn build_bus_station_g_cd_map(stops: &[GtfsStopRow]) -> HashMap { + fn find(parent: &mut [usize], mut x: usize) -> usize { + while parent[x] != x { + parent[x] = parent[parent[x]]; + x = parent[x]; + } + x + } + + // Bucket by name first; poles of one physical stop always share the name. + let mut by_name: HashMap<&str, Vec> = HashMap::new(); + for (idx, stop) in stops.iter().enumerate() { + by_name.entry(stop.stop_name.trim()).or_default().push(idx); + } + + let mut map: HashMap = HashMap::with_capacity(stops.len()); + for indices in by_name.values() { + // Union same-name stops that are within the grouping radius. + let mut parent: Vec = (0..indices.len()).collect(); + for a in 0..indices.len() { + for b in (a + 1)..indices.len() { + let sa = &stops[indices[a]]; + let sb = &stops[indices[b]]; + if haversine_distance(sa.stop_lat, sa.stop_lon, sb.stop_lat, sb.stop_lon) + <= BUS_STOP_GROUPING_RADIUS_METERS + { + let ra = find(&mut parent, a); + let rb = find(&mut parent, b); + if ra != rb { + parent[ra] = rb; + } + } + } + } + + // Representative of each cluster = lexicographically smallest stop_id, so + // the derived station_g_cd is stable regardless of DB row ordering. + let roots: Vec = (0..indices.len()).map(|a| find(&mut parent, a)).collect(); + let mut rep_stop_id: HashMap = HashMap::new(); + for (a, &root) in roots.iter().enumerate() { + let sid = stops[indices[a]].stop_id.as_str(); + rep_stop_id + .entry(root) + .and_modify(|cur| { + if sid < *cur { + *cur = sid; + } + }) + .or_insert(sid); + } + + for (a, &root) in roots.iter().enumerate() { + let g_cd = generate_bus_station_g_cd(rep_stop_id[&root]); + map.insert(stops[indices[a]].stop_id.clone(), g_cd); + } + } + + map +} + /// Generate deterministic type_cd from (route_id, shape_id). /// Uses range starting at 100,000,000 to avoid conflicts with existing rail types. fn generate_bus_type_cd(route_id: &str, shape_id: &str) -> i32 { @@ -2900,10 +2980,27 @@ async fn integrate_gtfs_stops_to_stations( .fetch_all(&mut *conn) .await?; + // Collapse the separate direction poles of one physical stop (feeds without a + // parent_station hierarchy expose each pole as its own top-level stop). + let station_g_cd_by_stop = build_bus_station_g_cd_map(&stops); + let group_count = station_g_cd_by_stop + .values() + .copied() + .collect::>() + .len(); + info!( + "Grouped {} bus stops into {} station groups.", + stops.len(), + group_count + ); + let mut inserted_count = 0; for stop in &stops { - let station_g_cd = generate_bus_station_g_cd(&stop.stop_id); + let station_g_cd = station_g_cd_by_stop + .get(&stop.stop_id) + .copied() + .unwrap_or_else(|| generate_bus_station_g_cd(&stop.stop_id)); // Get routes for this parent stop (with stop_sequence) // The mapping now uses parent_stop_id as key @@ -3691,6 +3788,51 @@ mod tests { ); } + fn stop_row(stop_id: &str, name: &str, lat: f64, lon: f64) -> GtfsStopRow { + GtfsStopRow { + stop_id: stop_id.to_string(), + stop_code: None, + stop_name: name.to_string(), + stop_name_k: None, + stop_name_r: None, + stop_name_zh: None, + stop_name_ko: None, + stop_desc: None, + stop_lat: lat, + stop_lon: lon, + zone_id: None, + stop_url: None, + location_type: None, + parent_station: None, + stop_timezone: None, + wheelchair_boarding: None, + platform_code: None, + } + } + + #[test] + fn test_build_bus_station_g_cd_map_merges_nearby_poles() { + let stops = vec![ + // Two direction poles of the same physical stop (~17 m apart). + stop_row("100001-01", "南大通り【朝霞警察署】", 35.787006, 139.592117), + stop_row("100001-02", "南大通り【朝霞警察署】", 35.786852, 139.592150), + // A distinct stop that merely shares a common name, far away (~40 km). + stop_row("A", "新田", 35.79, 139.59), + stop_row("B", "新田", 35.65, 139.80), + ]; + + let map = build_bus_station_g_cd_map(&stops); + + // The two nearby poles collapse into one group. + assert_eq!(map["100001-01"], map["100001-02"]); + // The far-apart namesakes stay separate. + assert_ne!(map["A"], map["B"]); + // The merged group is stable and equals the representative (min stop_id). + assert_eq!(map["100001-01"], generate_bus_station_g_cd("100001-01")); + // A lone stop keeps the exact station_g_cd it had before grouping. + assert_eq!(map["A"], generate_bus_station_g_cd("A")); + } + #[test] fn test_generate_bus_type_cd() { let type_cd = generate_bus_type_cd("route_001", "shape_A"); From 957e617c9ad4f587500d06e526243aed09913320 Mon Sep 17 00:00:00 2001 From: Tsubasa SEKIGUCHI Date: Sun, 12 Jul 2026 23:09:32 +0900 Subject: [PATCH 2/2] fix: apply CodeRabbit auto-fixes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit バス停グループ化を決定的 greedy complete-linkage に変更しクラスタ直径を 250m以内に制限(推移閉包による遠距離統合を防止)。parent_stationとして参照 される親stopはグループ化対象外にして自身のstation_g_cdへ固定し、parent 階層を持つフィード(都営等)への影響をなくす。回帰テストを追加。 Co-Authored-By: Claude Opus 4.8 --- stationapi/src/import.rs | 162 +++++++++++++++++++++++++-------------- 1 file changed, 106 insertions(+), 56 deletions(-) diff --git a/stationapi/src/import.rs b/stationapi/src/import.rs index 3e902174..7a313527 100644 --- a/stationapi/src/import.rs +++ b/stationapi/src/import.rs @@ -2255,71 +2255,74 @@ const BUS_STOP_GROUPING_RADIUS_METERS: f64 = 250.0; /// poles of one physical bus stop into a single group. /// /// Some GTFS feeds encode poles with a `parent_station` hierarchy (Toei), in -/// which case only the parent is imported and grouping is already correct. Other -/// feeds (Seibu, the Tokyu community buses, and typically Keio) expose every pole -/// as its own top-level stop, so the same bus stop name would otherwise appear -/// multiple times in the app. Here we group stops that share a name *and* sit -/// within `BUS_STOP_GROUPING_RADIUS_METERS` of each other, mirroring how rail -/// stations collapse across companies via `station_g_cd`. +/// which case each imported stop is already one deduplicated physical stop. Such +/// stops — the ones listed in `parent_stop_ids` because a child references them — +/// are pinned to their own `station_g_cd` and never merged, so parent-hierarchy +/// feeds are left exactly as before. Other feeds (Seibu, the Tokyu community +/// buses, and typically Keio) expose every pole as its own top-level stop, so the +/// same bus stop name would otherwise appear multiple times in the app; those +/// hierarchy-less stops are grouped when they share a name *and* sit close +/// together, mirroring how rail stations collapse across companies. /// -/// Proximity scoping keeps genuinely distinct stops that merely share a common -/// name (e.g. "新道" in different cities) in separate groups. A lone stop yields -/// the exact same `station_g_cd` as [`generate_bus_station_g_cd`], so feeds that -/// already group correctly are unaffected. -fn build_bus_station_g_cd_map(stops: &[GtfsStopRow]) -> HashMap { - fn find(parent: &mut [usize], mut x: usize) -> usize { - while parent[x] != x { - parent[x] = parent[parent[x]]; - x = parent[x]; - } - x - } +/// Grouping uses deterministic greedy complete-linkage: within a name, stops are +/// visited in `stop_id` order and each joins the first cluster whose *every* +/// member is within `BUS_STOP_GROUPING_RADIUS_METERS`. This bounds each cluster's +/// diameter to the radius (no transitive chaining that could swallow a distant +/// same-named stop) and is independent of DB row ordering. A lone stop yields the +/// exact same `station_g_cd` as [`generate_bus_station_g_cd`]. +fn build_bus_station_g_cd_map( + stops: &[GtfsStopRow], + parent_stop_ids: &HashSet, +) -> HashMap { + let mut map: HashMap = HashMap::with_capacity(stops.len()); - // Bucket by name first; poles of one physical stop always share the name. + // Pin parent stops to their own group; bucket the rest by name for grouping. let mut by_name: HashMap<&str, Vec> = HashMap::new(); for (idx, stop) in stops.iter().enumerate() { + if parent_stop_ids.contains(&stop.stop_id) { + map.insert( + stop.stop_id.clone(), + generate_bus_station_g_cd(&stop.stop_id), + ); + continue; + } by_name.entry(stop.stop_name.trim()).or_default().push(idx); } - let mut map: HashMap = HashMap::with_capacity(stops.len()); for indices in by_name.values() { - // Union same-name stops that are within the grouping radius. - let mut parent: Vec = (0..indices.len()).collect(); - for a in 0..indices.len() { - for b in (a + 1)..indices.len() { - let sa = &stops[indices[a]]; - let sb = &stops[indices[b]]; - if haversine_distance(sa.stop_lat, sa.stop_lon, sb.stop_lat, sb.stop_lon) - <= BUS_STOP_GROUPING_RADIUS_METERS - { - let ra = find(&mut parent, a); - let rb = find(&mut parent, b); - if ra != rb { - parent[ra] = rb; - } - } - } - } - - // Representative of each cluster = lexicographically smallest stop_id, so - // the derived station_g_cd is stable regardless of DB row ordering. - let roots: Vec = (0..indices.len()).map(|a| find(&mut parent, a)).collect(); - let mut rep_stop_id: HashMap = HashMap::new(); - for (a, &root) in roots.iter().enumerate() { - let sid = stops[indices[a]].stop_id.as_str(); - rep_stop_id - .entry(root) - .and_modify(|cur| { - if sid < *cur { - *cur = sid; - } + // Visit in a canonical order so the resulting clusters are deterministic. + let mut order = indices.clone(); + order.sort_by(|&a, &b| stops[a].stop_id.cmp(&stops[b].stop_id)); + + // Greedy complete-linkage: a stop joins the first cluster all of whose + // members are within the radius; otherwise it starts a new cluster. + let mut clusters: Vec> = Vec::new(); + for &i in &order { + let si = &stops[i]; + let joined = clusters.iter_mut().find(|cluster| { + cluster.iter().all(|&j| { + let sj = &stops[j]; + haversine_distance(si.stop_lat, si.stop_lon, sj.stop_lat, sj.stop_lon) + <= BUS_STOP_GROUPING_RADIUS_METERS }) - .or_insert(sid); + }); + match joined { + Some(cluster) => cluster.push(i), + None => clusters.push(vec![i]), + } } - for (a, &root) in roots.iter().enumerate() { - let g_cd = generate_bus_station_g_cd(rep_stop_id[&root]); - map.insert(stops[indices[a]].stop_id.clone(), g_cd); + for cluster in &clusters { + // Representative = lexicographically smallest stop_id for stability. + let rep = cluster + .iter() + .map(|&j| stops[j].stop_id.as_str()) + .min() + .expect("cluster is non-empty"); + let g_cd = generate_bus_station_g_cd(rep); + for &j in cluster { + map.insert(stops[j].stop_id.clone(), g_cd); + } } } @@ -2999,9 +3002,20 @@ async fn integrate_gtfs_stops_to_stations( .fetch_all(&mut *conn) .await?; + // Stops referenced as a parent_station already represent one deduplicated + // physical stop; they must stay pinned to their own group during grouping. + let parent_stop_ids: HashSet = sqlx::query_scalar::<_, String>( + "SELECT DISTINCT parent_station FROM gtfs_stops \ + WHERE parent_station IS NOT NULL AND parent_station <> ''", + ) + .fetch_all(&mut *conn) + .await? + .into_iter() + .collect(); + // Collapse the separate direction poles of one physical stop (feeds without a // parent_station hierarchy expose each pole as its own top-level stop). - let station_g_cd_by_stop = build_bus_station_g_cd_map(&stops); + let station_g_cd_by_stop = build_bus_station_g_cd_map(&stops, &parent_stop_ids); let group_count = station_g_cd_by_stop .values() .copied() @@ -3901,7 +3915,7 @@ mod tests { stop_row("B", "新田", 35.65, 139.80), ]; - let map = build_bus_station_g_cd_map(&stops); + let map = build_bus_station_g_cd_map(&stops, &HashSet::new()); // The two nearby poles collapse into one group. assert_eq!(map["100001-01"], map["100001-02"]); @@ -3913,6 +3927,42 @@ mod tests { assert_eq!(map["A"], generate_bus_station_g_cd("A")); } + #[test] + fn test_build_bus_station_g_cd_map_no_transitive_chain() { + // A-B and B-C are each within the radius, but A-C is not (~400 m apart). + // Single-linkage would chain all three; complete-linkage must keep the + // distant endpoints A and C in separate groups. + let stops = vec![ + stop_row("chain_a", "連鎖テスト", 35.0, 139.0), + stop_row("chain_b", "連鎖テスト", 35.0, 139.0022), // ~200 m east of A + stop_row("chain_c", "連鎖テスト", 35.0, 139.0044), // ~400 m east of A + ]; + + let map = build_bus_station_g_cd_map(&stops, &HashSet::new()); + + // The chain endpoints must not be merged despite the bridging middle stop. + assert_ne!(map["chain_a"], map["chain_c"]); + // Sanity: A-C really exceeds the radius while A-B stays within it. + assert!(haversine_distance(35.0, 139.0, 35.0, 139.0044) > BUS_STOP_GROUPING_RADIUS_METERS); + assert!(haversine_distance(35.0, 139.0, 35.0, 139.0022) <= BUS_STOP_GROUPING_RADIUS_METERS); + } + + #[test] + fn test_build_bus_station_g_cd_map_pins_parent_stops() { + // A parent stop and a hierarchy-less stop share a name and sit close by. + let stops = vec![ + stop_row("0001", "青戸車庫前", 35.744787, 139.843847), + stop_row("other-01", "青戸車庫前", 35.744900, 139.843900), + ]; + let parents: HashSet = ["0001".to_string()].into_iter().collect(); + + let map = build_bus_station_g_cd_map(&stops, &parents); + + // The parent stop keeps its own station_g_cd and is never merged. + assert_eq!(map["0001"], generate_bus_station_g_cd("0001")); + assert_ne!(map["0001"], map["other-01"]); + } + #[test] fn test_generate_bus_type_cd() { let type_cd = generate_bus_type_cd("route_001", "shape_A");