Rjf/amazing transit service#145
Merged
robfitzgerald merged 10 commits intomainfrom May 6, 2026
Merged
Conversation
Co-authored-by: Copilot <[email protected]>
Contributor
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
This PR migrates the transit traversal implementation out of bambam into the bambam-gtfs crate while addressing time underflow/invalid-schedule edge cases in scheduled transit traversal.
Changes:
- Removed
rust/bambam’s internaltransittraversal module and switched registrations/imports to thebambam-gtfsimplementation. - Added/updated GTFS transit traversal logic to prevent negative/invalid time deltas (e.g., rejecting invalid departures, guarding “departure in the past”).
- Improved schedule ordering invariants and updated next-departure selection logic.
Reviewed changes
Copilot reviewed 25 out of 25 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| rust/bambam/src/model/traversal/transit/transit_ops.rs | Removed local transit ops (migrated to bambam-gtfs). |
| rust/bambam/src/model/traversal/transit/service.rs | Removed local service (migrated to bambam-gtfs). |
| rust/bambam/src/model/traversal/transit/schedule_loading_policy.rs | Removed local policy (migrated to bambam-gtfs). |
| rust/bambam/src/model/traversal/transit/schedule.rs | Removed local schedule types/tests (migrated to bambam-gtfs). |
| rust/bambam/src/model/traversal/transit/raw_schedule_row.rs | Removed local raw row type (migrated to bambam-gtfs). |
| rust/bambam/src/model/traversal/transit/query.rs | Removed local query type (migrated to bambam-gtfs). |
| rust/bambam/src/model/traversal/transit/model.rs | Removed local traversal model (migrated to bambam-gtfs). |
| rust/bambam/src/model/traversal/transit/mod.rs | Removed local transit module exports. |
| rust/bambam/src/model/traversal/transit/metadata.rs | Removed local GTFS metadata reader (migrated to bambam-gtfs). |
| rust/bambam/src/model/traversal/transit/engine.rs | Removed local engine (migrated to bambam-gtfs). |
| rust/bambam/src/model/traversal/transit/config.rs | Removed local config (migrated to bambam-gtfs). |
| rust/bambam/src/model/traversal/transit/builder.rs | Removed local traversal builder (registration now points to bambam-gtfs). |
| rust/bambam/src/model/traversal/mod.rs | Dropped pub mod transit; from bambam. |
| rust/bambam/src/model/builders.rs | Updated builder import to bambam_gtfs::...TransitTraversalBuilder. |
| rust/bambam/src/app/gtfs_config/run.rs | Updated config imports to use bambam-gtfs transit config/policy types. |
| rust/bambam/Cargo.toml | Added dependency on bambam-gtfs. |
| rust/bambam-gtfs/src/model/traversal/transit/transit_ops.rs | Updated time composition logic (duration build). |
| rust/bambam-gtfs/src/model/traversal/transit/schedule_loading_policy.rs | Added validation to reject departures with arrival before departure; added tests. |
| rust/bambam-gtfs/src/model/traversal/transit/schedule.rs | Tightened ordering (tie-breaker) and added invariant test. |
| rust/bambam-gtfs/src/model/traversal/transit/query.rs | Removed unused import; query remains for model building. |
| rust/bambam-gtfs/src/model/traversal/transit/model.rs | Refactored traversal into helper, added negative-wait guard, added tests. |
| rust/bambam-gtfs/src/model/traversal/transit/metadata.rs | Removed unused import. |
| rust/bambam-gtfs/src/model/traversal/transit/engine.rs | Changed next-departure selection to consider earliest arrival; pruned imports. |
| rust/bambam-gtfs/src/lib.rs | Exported model module. |
| rust/bambam-gtfs/Cargo.toml | Added missing dependencies for moved transit model code. |
Comments suppressed due to low confidence (1)
rust/bambam-gtfs/src/model/traversal/transit/transit_ops.rs:1
get_current_timeis vulnerable to float edge cases: rounding can yieldnanos == 1_000_000_000, and negativetrip_timewith fractional seconds can produce a negative remainder that gets cast tou32(wraparound), causingDuration::newto fail or produce unintended values. Use a safer split/normalization that (a) rounds nanos, (b) clamps nanos into[0, 999_999_999]with carry intoseconds, and (c) handles negative fractional seconds consistently; also remove the stray space before;on line 22.
use std::collections::HashMap;
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot Autofix powered by AI <[email protected]>
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
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
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.
addresses time underflow bugs associated with scheduled transit traversal modeling leading to 1000-mile-radius transit isochrones.