Skip to content

feat(GroupFilter): Propogate CR Delays to downstream stops - #449

Merged
KaylaBrady merged 23 commits into
masterfrom
kb-propogate-cr-delays
Nov 3, 2026
Merged

KaylaBrady merged 23 commits into
masterfrom
kb-propogate-cr-delays

Conversation

@KaylaBrady

@KaylaBrady KaylaBrady commented Sep 11, 2026 •

Copy link
Copy Markdown
Contributor

Summary of changes

Asana Ticket: CR Delays | Concentrate change to propagate status when delays

This adds a new GroupFilter to propagate delay StopTimeUpdates under certain circumstances:

  • CR trips only
  • There is a status-only StopTimeUpdate indicating a delay
  • For downstream stops that have scheduled departure in the past and no existing StopTimeUpdate

Tested with unit tests and end-to-end in dev-green.

Note: I used AI for autocompleting helper functions and generating test stubs.

existing_stu != nil ->
[existing_stu]

!is_nil(departure) && now > departure && stop_sequence > first_stop_sequence ->

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

note: I don't compare stop_ids anywhere and am only relying on stop_sequence. Is that safe?

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.

I think it's fine! The combination of trip ID (these are all from the same trip) and stop sequence should uniquely identify the schedule departure.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Great, I have a vague memory of something about virtual vs physical platforms for CR which I why I was hoping to avoid it in the first place.

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.

Ah, right, I had forgotten about the parent vs. child stop thing entirely but that is indeed a good reason for relying just on stop sequence.

@KaylaBrady
KaylaBrady force-pushed the kb-propogate-cr-delays branch from 8cbd38b to 46cdbdd Compare September 11, 2026 18:54
Comment thread config/config.exs Outdated
{Concentrate.GroupFilter.SuppressStopTimeUpdate, terminal_suppression_by_time: %{}}
{Concentrate.GroupFilter.SuppressStopTimeUpdate, terminal_suppression_by_time: %{}},
{Concentrate.GroupFilter.PropogateDownstreamDelays,
matching_statuses: ["Delayed"], downstream_status: "Delayed"}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

TODO: Get complete list of relevant statuses.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@lemald I updated the status list to re-use the config from boarding_status_override based on the given list. My one concern is that both "PRIORITY" and "HOLD" Statuses are converted to "Info to follow". Any concerns about this logic applying to "PRIORITY" too?

Since the conversion of boarding_status_override is done at the parsing level I think it would be a bigger but not horrible effort to change (maybe just a new filer at the end). But if the user-facing string is the same, I'm wondering if it would be acceptable to treat them as the same for the purpose of downstream delays.

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.

Yeah, I think that's probably fine, especially since I'm guessing that PRIORITY and HOLD are less-frequently-used statuses.

@KaylaBrady
KaylaBrady marked this pull request as ready for review September 11, 2026 18:57
@KaylaBrady
KaylaBrady requested a review from a team as a code owner September 11, 2026 18:58
@KaylaBrady
KaylaBrady requested review from zyxw59 and removed request for a team September 11, 2026 18:58

@lemald lemald left a comment

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.

Okay, I think this looks good! It sounds like before merging and going to prod we'll want to:

  1. Confirm the list of statuses for which this should apply (already noted).
  2. Deploy to a dev environment and test out there.

@KaylaBrady
KaylaBrady merged commit ac6b7aa into master Nov 3, 2026
4 checks passed
@KaylaBrady
KaylaBrady deleted the kb-propogate-cr-delays branch November 3, 2026 13:54
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