Skip to content

fix(TripDetails): Filter out already-visited stops before computing vehicle status#3293

Open
joshlarson wants to merge 6 commits into
mainfrom
jdl/fix/departures/drop-past-scheduled-visits-for-vehicle-status
Open

fix(TripDetails): Filter out already-visited stops before computing vehicle status#3293
joshlarson wants to merge 6 commits into
mainfrom
jdl/fix/departures/drop-past-scheduled-visits-for-vehicle-status

Conversation

@joshlarson

Copy link
Copy Markdown
Contributor

Scope

The bug is as follows:

  • Imagine a trip that's running very far ahead of schedule, such that the trip is already well underway, but the scheduled departure time from the origin station hasn't passed yet.
  • There will be schedules for the origin station, but no predictions, because predictions typically disappear after the vehicle passes the station that they're predicting.
  • Those schedules won't get filtered out, because they're still in the future.
  • This tricks trip_details/1 into showing :scheduled_to_depart.

Asana Ticket: 📆🔎🐞 Hide stops already visited from trip details when vehicle info is available

Implementation

(Apologies for the kind of messy code diff! I had to make a bunch of changes kind of all at once, and I couldn't think of a way of splitting it out into separate commits without having that splitting take forever.)

At a high level, the fix is to filter out upstream predicted_schedules before computing vehicle_info. In practice, this turned out to be surprisingly involved, because the way we were checking vehicle location for other parts of trip-details computation used vehicle_info, which is the thing that needed to change! So I needed to move the "drop upstream prediction/schedule" call earlier in the pipeline, use vehicle for that, and make sure that vehicle is actually set correctly (turns out... it wasn't!)

A few random other notes:

  • I split drop_first_trip_stop_if_vehicle_is_stopped out of drop_predicted_schedules_before_current_station... they're already different things, drop_first_trip_stop_if_vehicle_is_stopped is a lot easier with vehicle_info rather than vehicle, and vehicle_info only relies on drop_predicted_schedules_before_current_station.
  • If the first predicted_schedule was just a schedule, then UpcomingDepartures.Processor.trip_details/1 was getting a nil vehicle instead of the vehicle from the predictions. I updated it to look up the vehicle from predictions, rather than predicted_schedules.
  • A bunch of the tests in TripDetailsTest didn't have realistic stop sequence setup. I @tag :skip'ed those tests for now, because I think the Right™ call for them is to give them the same treatment as UpcomingDeparturesTest - i.e. use PredictedScheduleHelper, and I didn't want to delay the bugfix for long enough to actually do that (I did create a follow-up ticket for that).

Screenshots

Screenshot 2026-06-26 at 1 46 17 PM

How to test

predictions_by_trip_id =
  Predictions.Repo.all(route: Dotcom.Routes.subway_route_ids() |> Enum.join(","))
  |> Enum.group_by(& &1.trip.id)
  |> Map.new(fn {trip_id, predictions} ->
    {trip_id, predictions |> Enum.sort_by(& &1.stop_sequence)}
  end)

trip_ids = predictions_by_trip_id |> Map.keys()

schedules_by_trip_id =
  trip_ids
  |> Map.new(&{&1, Schedules.Repo.schedule_for_trip(&1)})

trip_ids
|> Enum.filter(fn trip_id ->
  predictions_by_trip_id
  |> Map.get(trip_id)
  |> Enum.all?(& &1.arrival_time)
end)
|> Enum.filter(fn trip_id ->
  schedules_by_trip_id
  |> Map.get(trip_id) != []
end)
|> Enum.filter(fn trip_id ->
  schedules_by_trip_id
  |> Map.get(trip_id)
  |> Enum.all?(
    &(&1.departure_time == nil || DateTime.after?(&1.departure_time, Dotcom.Utils.DateTime.now()))
  )
end)
|> Enum.map(fn trip_id ->
  schedule = schedules_by_trip_id |> Map.get(trip_id) |> List.first()
  prediction = predictions_by_trip_id |> Map.get(trip_id) |> List.first()

  {
    trip_id,
    schedule.route.id,
    schedule.route.direction_names[schedule.trip.direction_id],
    prediction.stop.name,
    schedule.departure_time |> Dotcom.Utils.Time.format!(:hour_12_minutes)
  }
end)

If you run ☝️ snippet in iEX or LiveBook (attached to Dotcom), then, it'll tell you which trips are underway with origin departures scheduled in the future. Those are the trips that show this bug. For instance, when I took the screenshot above, this snippet output something like

[
  {"74448283", "Green-E", "Eastbound", "Fenwood Road", "1:49 PM"}
]

So in order to see the bug/solution in action, you look at a Green-E departures page at Fenwood or any stop downstream of Fenwood (but not too far downstream or else this trip won't be in the first five).

(Annoyingly, as I type this out, there are no trips running that early, so I'm seeing [])

@joshlarson joshlarson requested a review from a team as a code owner June 26, 2026 18:47
@joshlarson joshlarson requested a review from lvachon1 June 26, 2026 18:47

@jlucytan jlucytan left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for the fix!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants