feat: avoid matching departure of prior trips by andrewdolce · Pull Request #609 · mbta/orbit

andrewdolce · 2026-08-28T22:04:06Z

Asana Task: 🛰️ 🐞 Orbit is sometimes showing Actual Departure for previous trip

Problem

Orbit observes and records VehicleEvents as vehicles arrive at and depart from stations. These are matched against current trips in OCS to find each vehicle's "Actual" departure time.

But sometimes a VehicleEvent is missed, such as in the case captured in the ticket, wherein OCS seemingly skipped some TMOVs, which caused RTR to never publish that the vehicle had STOPPED_AT the trip origin/destination. In such cases, when TripMatcher tries to find the departure event for the given trip, since the event is missing, it may erroneously match to the prior trip's departure, and thereby showing the wrong departure time.

We do have a timing threshold in place to try to avoid finding old departures, but it is difficult to pick a single threshold that works in all cases. For example, this ticket demonstrates a case where service from Braintree was only running up to Park St, meaning that the trips were much shorter than usual.

Proposed solution

When checking the most recent departure event for a trip's origin, also consider the known prior trips for the same vehicle, and if there is a better match to a prior trip, then avoid the assignment. Some notes:

This means that the "Actual" departure time in the UI will be missing, which is not ideal, but is better than showing the incorrect time.
In considering past trips, I wanted to only compare with ones that were departing in the same direction from the origin terminal, in case there are strange edge cases with trips originating at midline terminals in different directions? To do that I had to add some utility functions to reason about the direction of a trip based on its endpoints. However, I'm rethinking whether this is necessary, and if not, may end up removing this.
Currently this only considers past trips with the same origin. Realizing that's not totally sufficient, so may think through how to add better handling of that, but still think this would be an OK incremental improvement. (For example, Trip A Alewife --> Braintree, Trip B Braintree --> Park, Trip C Park --> Braintree. If Trip C's departure is missing, it could erroneously match against the events of Trip A passing through Park St.)

Checklist

Appearance: (N/A)
Browsers: (N/A)
Privacy:
- Commits free of internal data
- PR description free of internal data
- Logging free of internal data
Tests:
- Has tests
- Doesn't need tests
- Tests deferred (with justification)

mathcolo · 2026-08-31T17:11:40Z

  end
+
+  @spec direction_from_stations(String.t(), String.t()) :: :ambiguous | 0 | 1
+  def direction_from_stations(origin, dest) do


observation: anecdotally I'm not aware of a significant number of intraday Ashmont-Braintree cross-pollination. I'm fairly confident that makes it okay to remove this, as you wrote in the description?

Are you talking about the case of handling Ashmont <--> Braintree specifically, or just direction comparison in general?

I looked more into whether to drop the direction comparison, and I'm leaning toward keeping it? For example, I notice that we detect departure events in the southbound direction at Ashmont (as part of the vehicle turning around), meaning that if we're not careful, we could consider the current trip prematurely departed. I realize that's not exactly the same scenario as what I'm fixing here, but think it still provides value to do this check. Happy to talk about it further.

...although I am now remembering that we already have special-case handling for Ashmont turnarounds.

mathcolo · 2026-08-31T17:11:55Z

+
+defimpl Jason.Encoder, for: Orbit.Vehicle do
+  def encode(value, opts) do
+    # Omit the past trips for now, since the frontend does not expect or care about it


mathcolo · 2026-08-31T17:16:09Z

+      trips
+      |> Enum.map(fn trip -> {trip, match_departure?(trip, event)} end)
+      |> Enum.reject(fn {_trip, {result, _}} -> result == :no_match end)
+      |> Enum.min_by(fn {_trip, {:match, offset}} -> offset end, fn -> {nil, nil} end)


question: is it correct that you're using offset to refer both to OCS offsets as well as match offsets?

Correct. Now that you point that out, it's maybe confusing. I will consider a different term for the offset here.

Changed in 8baf06d

mathcolo · 2026-08-31T17:16:55Z

+
+      true ->
+        expected_departure_time =
+          DateTime.add(ocs_trip.scheduled_departure || ocs_trip.assigned_at, ocs_trip.offset || 0)


question: could you remind me why assigned_at works as a fallback?

I am just trying to get a rough idea of when a trip was supposed to depart for comparison with the vehicle event timestamps. Previously, we'd only do this with current trips, which meant that we were comparing against "now". For past trips, I'm using the scheduled start time (+ the inspector offset) if they exist. If not, the assigned_at timestamp is something that I think anecdotally comes in pretty close to departure for most trips, beyond maybe morning pullouts which can get assigned much before the trip leaves.

Realizing that I am adding the offset to the assigned_at time, which is maybe weird. In practice I find it hard to imagine that a trip would have an inspector offset but not a scheduled start time, although I suppose we could be missing the start time if Orbit somehow missed a TSCH_NEW message. Maybe I'll change this to only add the offset in the case that the scheduled departure is used.

Let me know if that answered your question?

Changed in 7a91269 - Now only adding inspector offset to scheduled time, not assigned_at time

cho-jos · 2026-09-01T15:54:56Z

+      if best_trip == current do
+        put_in(vehicle.ocs_trips.current.actual_departure, actual_departure.timestamp)


comment: I'm hoping that as a side effect this helps out with what I was looking into for 🛰️ Investigate how often Orbit trip assignments are for previous trip 🤞 ?

cho-jos · 2026-09-01T15:58:29Z

+      # Scenario: Vehicle making multiple trips:
+      #   trip_1: Park St to Braintree
+      #   trip_2: Braintree to Park St
+      #   trip_3: Part St to Braintree
+      # Current trip is trip_3, but the departure from Park St was not detected
+      # so there is no VehicleEvent. However there is a VehicleEvent for the prior
+      # departure on trip_1


praise: 🙏🙏🙏🙏🙏🙏🙏 ty for comment on behalf of present and future me

cho-jos

LGTM, though would like resolution of Preston's comment before merge

observation: anecdotally I'm not aware of a significant number of intraday Ashmont-Braintree cross-pollination. I'm fairly confident that makes it okay to remove this, as you wrote in the description?

andrewdolce added 5 commits August 28, 2026 18:02

feat: create func to determine trip direction from endpoints

5b1b8eb

chore: break out helper funcs

71a81a6

feat: add direction_id to event lookup

428d554

feat: gather vehicle's past trips

085f263

feat: avoid matching departure of prior trip

cf8e8d9

andrewdolce force-pushed the asd-better-departure-thresholding branch from fe6e824 to cf8e8d9 Compare August 31, 2026 13:22

andrewdolce marked this pull request as ready for review August 31, 2026 13:44

andrewdolce requested a review from a team as a code owner August 31, 2026 13:44

mathcolo reviewed Aug 31, 2026

View reviewed changes

andrewdolce added 2 commits August 31, 2026 14:02

fix: only add inspector offset to scheduled time

7a91269

fix: rename 'offset' to not conflate with inspector offsets

8baf06d

andrewdolce requested a review from mathcolo August 31, 2026 18:13

cho-jos reviewed Sep 1, 2026

View reviewed changes

cho-jos approved these changes Sep 1, 2026

View reviewed changes

		if best_trip == current do
		put_in(vehicle.ocs_trips.current.actual_departure, actual_departure.timestamp)

Uh oh!

Conversation

andrewdolce commented Aug 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem

Proposed solution

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

andrewdolce Aug 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cho-jos left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

andrewdolce commented Aug 28, 2026 •

edited

Loading

andrewdolce Aug 31, 2026 •

edited

Loading