Skip to content

Better handle shuttle alert directionality #440

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
lemald merged 7 commits into from
Jul 10, 2026
Merged

Better handle shuttle alert directionality #440

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
5ec7d57
refactor: add direction_id argument to stop_shuttling_on_route/4
lemald Jul 9, 2026
153e54b
test: test case analogous to Lowell Line shuttle
lemald Jul 9, 2026
5c2c69e
feat: parse and handle direction_id from shuttle alerts
lemald Jul 9, 2026
d5166cc
chore(ci): bump version of asdf-vm/actions/install
lemald Jul 9, 2026
4d1b9be
chore(ci): bump cache key to not use old version of ASDF deps
lemald Jul 10, 2026
c6e841f
refactor: only evaluate shuttle_type/1 once
lemald Jul 10, 2026
c755676
refactor: better name boolean values
lemald Jul 10, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions .github/workflows/elixir.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,10 +32,10 @@ jobs:
uses: actions/cache@v4
with:
path: ~/.asdf
key: ${{ runner.os }}-asdf-v2-${{ hashFiles('.tool-versions') }}
key: ${{ runner.os }}-asdf-v4-${{ hashFiles('.tool-versions') }}
id: asdf-cache
# only run `asdf install` if we didn't hit the cache
- uses: asdf-vm/actions/install@v1
- uses: asdf-vm/actions/install@v4

@jzimbel-mbta jzimbel-mbta Jul 10, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

suggestion:
Should the cache key also be updated to avoid using a cached install from the old version of asdf / the asdf install action? Like change -v2- to -v4- in the key, perhaps.

@lemald lemald Jul 10, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

4d1b9be

if: steps.asdf-cache.outputs.cache-hit != 'true'
# if we did hit the cache, set up the environment
- name: Setup ASDF environment
Expand Down
17 changes: 12 additions & 5 deletions lib/concentrate/filter/alert/shuttles.ex
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,11 +36,16 @@ defmodule Concentrate.Filter.Alert.Shuttles do
@spec stop_shuttling_on_route(
route_id :: String.t(),
stop_id :: String.t(),
direction_id :: 0 | 1,
:calendar.date() | integer
) :: :start | :stop | :through | nil
def stop_shuttling_on_route(route_id, stop_id, date_or_timestamp)
def stop_shuttling_on_route(route_id, stop_id, direction_id, date_or_timestamp)
when is_binary(route_id) and is_binary(stop_id) do
case TimeTable.date_overlaps(@table, {:route_stop, route_id, stop_id}, date_or_timestamp) do
case TimeTable.date_overlaps(
@table,
{:route_stop, route_id, stop_id, direction_id},

@jzimbel-mbta jzimbel-mbta Jul 10, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

question:
Does the shuttles timetable ETS table contain both 3-tuples that do not include direction ID, and 4-tuples that do include direction ID?

I guess I'm a little surprised that you didn't need to make any corresponding changes over in the TimeTable module (or here?) for this new record structure.

(I admit I'm a little rusty on ETS MatchSpecs, so I might be misunderstanding how this key ends up getting used in the select query.)

@lemald lemald Jul 10, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

They now contain only the four-tuples - I didn't directly change the code in your "here" link, but I did change the cancellation_type/1 function that it calls, which is how we generate the tuples that get added to the table.

date_or_timestamp
) do
[atom | _] -> atom
[] -> nil
end
Expand Down Expand Up @@ -80,9 +85,11 @@ defmodule Concentrate.Filter.Alert.Shuttles do

route_stops =
if is_binary(stop_id) and is_binary(route_id) do
[
{{:route_stop, route_id, stop_id}, shuttle_type(InformedEntity.activities(entity))}
]
shuttle_type = shuttle_type(InformedEntity.activities(entity))

for direction_id <- direction_ids(InformedEntity.direction_id(entity)) do
{{:route_stop, route_id, stop_id, direction_id}, shuttle_type}
end
else
[]
end
Expand Down
16 changes: 8 additions & 8 deletions lib/concentrate/group_filter/shuttle.ex
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ defmodule Concentrate.GroupFilter.Shuttle do
stus =
if is_tuple(date) and is_binary(route_id) and
module.trip_shuttling?(trip_id, route_id, direction_id, date) do
shuttle_updates(route_id, stus, module)
shuttle_updates(route_id, direction_id, stus, module)
else
stus
end
Expand All @@ -31,27 +31,27 @@ defmodule Concentrate.GroupFilter.Shuttle do

def filter(%TripGroup{} = other, _module), do: other

defp shuttle_updates(route_id, stus, module) do
initial_state = {false, false}
defp shuttle_updates(route_id, direction_id, stus, module) do
initial_state = {_has_started? = false, _has_shuttled? = false}

stus
|> Enum.flat_map_reduce(initial_state, &shuttle_stop(route_id, module, &1, &2))
|> Enum.flat_map_reduce(initial_state, &shuttle_stop(route_id, direction_id, module, &1, &2))
|> elem(0)
end

defp shuttle_stop(route_id, shuttle_module, stop_time_updates, state)
defp shuttle_stop(route_id, direction_id, shuttle_module, stop_time_updates, state)

defp shuttle_stop(_route_id, _module, stu, {true, true} = state) do
defp shuttle_stop(_route_id, _direction_id, _module, stu, {true, true} = state) do
{[StopTimeUpdate.skip(stu)], state}
end

defp shuttle_stop(route_id, module, stu, {has_started?, has_shuttled?} = state) do
defp shuttle_stop(route_id, direction_id, module, stu, {has_started?, has_shuttled?} = state) do
time = StopTimeUpdate.time(stu)

if is_integer(time) do
stop_id = StopTimeUpdate.stop_id(stu)

case module.stop_shuttling_on_route(route_id, stop_id, time) do
case module.stop_shuttling_on_route(route_id, stop_id, direction_id, time) do
nil ->
drop_arrival_time_if_after_shuttle(stu, has_shuttled?)

Expand Down
26 changes: 18 additions & 8 deletions test/concentrate/filter/alert/shuttles_test.exs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ defmodule Concentrate.Filter.Alert.ShuttlesTest do
end
end

describe "stop_shuttling_on_route?/1" do
describe "stop_shuttling_on_route/4" do
setup :supervised

test "returns a atom indicating whether the route is shuttling a particular stop" do
Expand All @@ -88,6 +88,13 @@ defmodule Concentrate.Filter.Alert.ShuttlesTest do
stop_id: "start",
activities: ["BOARD", "RIDE"]
),
InformedEntity.new(
route_type: 2,
direction_id: 0,
route_id: "route",
stop_id: "start_unidirectional",
activities: ["BOARD", "RIDE"]
),
InformedEntity.new(
route_type: 2,
route_id: "route",
Expand All @@ -99,11 +106,14 @@ defmodule Concentrate.Filter.Alert.ShuttlesTest do

handle_events([[alert]], :from, :state)

assert stop_shuttling_on_route("route", "through", 5) == :through
assert stop_shuttling_on_route("route", "through", 21) == nil
assert stop_shuttling_on_route("route", "through", {1970, 1, 1}) == :through
assert stop_shuttling_on_route("route", "start", 5) == :start
assert stop_shuttling_on_route("route", "stop", 5) == :stop
assert stop_shuttling_on_route("route", "through", 0, 5) == :through
assert stop_shuttling_on_route("route", "through", 0, 21) == nil
assert stop_shuttling_on_route("route", "through", 0, {1970, 1, 1}) == :through
assert stop_shuttling_on_route("route", "start", 0, 5) == :start
assert stop_shuttling_on_route("route", "start", 1, 5) == :start
assert stop_shuttling_on_route("route", "start_unidirectional", 0, 5) == :start
assert stop_shuttling_on_route("route", "start_unidirectional", 1, 5) == nil
assert stop_shuttling_on_route("route", "stop", 0, 5) == :stop
end
end

Expand All @@ -112,8 +122,8 @@ defmodule Concentrate.Filter.Alert.ShuttlesTest do
refute trip_shuttling?("trip", "route", 0, 0)
end

test "stop_shuttling_on_route/3 returns nil" do
assert stop_shuttling_on_route("route", "stop", 0) == nil
test "stop_shuttling_on_route/4 returns nil" do
assert stop_shuttling_on_route("route", "stop", 0, 0) == nil
end
end

Expand Down
63 changes: 60 additions & 3 deletions test/concentrate/group_filter/shuttle_test.exs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ defmodule Concentrate.GroupFilter.ShuttleTest do

@trip_id "trip"
@route_id "route"
@direction_id 0
@valid_date {1970, 1, 1}
@valid_date_time 8

Expand All @@ -28,6 +29,7 @@ defmodule Concentrate.GroupFilter.ShuttleTest do
TripDescriptor.new(
trip_id: @trip_id,
route_id: @route_id,
direction_id: @direction_id,
start_date: @valid_date
)

Expand Down Expand Up @@ -59,6 +61,7 @@ defmodule Concentrate.GroupFilter.ShuttleTest do
TripDescriptor.new(
trip_id: @trip_id,
route_id: @route_id,
direction_id: @direction_id,
start_date: {1970, 1, 1}
),
stus: [
Expand Down Expand Up @@ -112,6 +115,7 @@ defmodule Concentrate.GroupFilter.ShuttleTest do
TripDescriptor.new(
trip_id: @trip_id,
route_id: @route_id,
direction_id: @direction_id,
start_date: {1970, 1, 1}
),
stus: [
Expand Down Expand Up @@ -149,6 +153,7 @@ defmodule Concentrate.GroupFilter.ShuttleTest do
TripDescriptor.new(
trip_id: @trip_id,
route_id: @route_id,
direction_id: @direction_id,
start_date: {1970, 1, 1}
),
stus: [
Expand Down Expand Up @@ -187,6 +192,7 @@ defmodule Concentrate.GroupFilter.ShuttleTest do
TripDescriptor.new(
trip_id: @trip_id,
route_id: @route_id,
direction_id: @direction_id,
start_date: {1970, 1, 1}
),
stus: [
Expand All @@ -210,6 +216,7 @@ defmodule Concentrate.GroupFilter.ShuttleTest do
TripDescriptor.new(
trip_id: @trip_id,
route_id: @route_id,
direction_id: @direction_id,
start_date: {1970, 1, 1}
),
stus: [
Expand All @@ -230,7 +237,7 @@ defmodule Concentrate.GroupFilter.ShuttleTest do
TripDescriptor.new(
trip_id: @trip_id,
route_id: "single_direction",
direction_id: 0,
direction_id: @direction_id,
start_date: {1970, 1, 1}
),
stus: [
Expand Down Expand Up @@ -274,7 +281,7 @@ defmodule Concentrate.GroupFilter.ShuttleTest do
TripDescriptor.new(
trip_id: "other_trip",
route_id: @route_id,
direction_id: 0,
direction_id: @direction_id,
start_date: {1970, 1, 1}
),
stus: [
Expand All @@ -296,6 +303,7 @@ defmodule Concentrate.GroupFilter.ShuttleTest do
TripDescriptor.new(
trip_id: @trip_id,
route_id: @route_id,
direction_id: @direction_id,
start_date: {1970, 1, 1}
),
stus: [
Expand Down Expand Up @@ -349,6 +357,7 @@ defmodule Concentrate.GroupFilter.ShuttleTest do
TripDescriptor.new(
trip_id: @trip_id,
route_id: @route_id,
direction_id: @direction_id,
start_date: {1970, 1, 1}
),
stus: [
Expand Down Expand Up @@ -389,6 +398,7 @@ defmodule Concentrate.GroupFilter.ShuttleTest do
TripDescriptor.new(
trip_id: @trip_id,
route_id: @route_id,
direction_id: @direction_id,
start_date: {1970, 1, 1}
),
stus: [
Expand All @@ -414,10 +424,57 @@ defmodule Concentrate.GroupFilter.ShuttleTest do
assert StopTimeUpdate.departure_time(stop)
end

test "start of a shuttle in one direction doesn't affect STUs in the opposite direction" do
group =
%TripGroup{
td:
TripDescriptor.new(
trip_id: @trip_id,
route_id: @route_id,
direction_id: @direction_id,
start_date: {1970, 1, 1}
),
stus: [
StopTimeUpdate.new(
trip_id: @trip_id,
stop_id: "shuttle_start_unidirectional",
departure_time: @valid_date_time
),
StopTimeUpdate.new(
trip_id: @trip_id,
stop_id: "after_shuttle",
arrival_time: @valid_date_time,
departure_time: @valid_date_time
),
StopTimeUpdate.new(
trip_id: @trip_id,
stop_id: "after_shuttle_2",
arrival_time: @valid_date_time
)
]
}

%TripGroup{stus: reduced} = filter(group, @module)

assert [start_unidirectional, after_shuttle, after_shuttle_2] = reduced
assert StopTimeUpdate.schedule_relationship(start_unidirectional) == :SCHEDULED
assert StopTimeUpdate.departure_time(start_unidirectional)
assert StopTimeUpdate.schedule_relationship(after_shuttle) == :SCHEDULED
assert StopTimeUpdate.arrival_time(after_shuttle)
assert StopTimeUpdate.departure_time(after_shuttle)
assert StopTimeUpdate.schedule_relationship(after_shuttle_2) == :SCHEDULED
assert StopTimeUpdate.arrival_time(after_shuttle_2)
end

test "updates on non-shuttle trips are not modified" do
group =
%TripGroup{
td: TripDescriptor.new(trip_id: "other_trip", start_date: @valid_date),
td:
TripDescriptor.new(
trip_id: "other_trip",
direction_id: @direction_id,
start_date: @valid_date
),
stus: [
StopTimeUpdate.new(
trip_id: "other_trip",
Expand Down
13 changes: 7 additions & 6 deletions test/support/filter/fakes.ex
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,10 +108,11 @@ defmodule Concentrate.Filter.FakeShuttles do
def trip_shuttling?("trip", "single_direction", 0, {1970, 1, 1}), do: true
def trip_shuttling?(_trip_id, _route_id, _direction_id, {_, _, _}), do: false

def stop_shuttling_on_route("route", "shuttle_1", 8), do: :through
def stop_shuttling_on_route("route", "shuttle_2", 8), do: :through
def stop_shuttling_on_route("route", "shuttle_start", 8), do: :start
def stop_shuttling_on_route("route", "shuttle_stop", 8), do: :stop
def stop_shuttling_on_route("single_direction", "shuttle_1", 8), do: :through
def stop_shuttling_on_route(_, _, dt) when is_integer(dt), do: nil
def stop_shuttling_on_route("route", "shuttle_1", _direction_id, 8), do: :through
def stop_shuttling_on_route("route", "shuttle_2", _direction_id, 8), do: :through
def stop_shuttling_on_route("route", "shuttle_start", _direction_id, 8), do: :start
def stop_shuttling_on_route("route", "shuttle_start_unidirectional", 1, 8), do: :start
def stop_shuttling_on_route("route", "shuttle_stop", _direction_id, 8), do: :stop
def stop_shuttling_on_route("single_direction", "shuttle_1", 0, 8), do: :through
def stop_shuttling_on_route(_, _, _, dt) when is_integer(dt), do: nil
end
Loading