fix: filter stop time updates without arrival/departure #294

Whoops · 2023-07-25T21:22:29Z

Summary of changes

Asana Ticket: 🐛 API publishing bus predictions with no times or schedule relationship

This filters out StopTimeUpdates which are scheduled, but have no arrival/departure. These occur when the BusLoc feed contains stops which Swiftly either isn't yet predicting, or is no longer publishing predictions for because the bus is already well past that stop.

github-actions · 2023-07-25T21:25:40Z

Coverage of commit `d14b864`

Summary coverage rate:
  lines......: 94.7% (1197 of 1264 lines)
  functions..: 83.5% (385 of 461 functions)
  branches...: no data found

Files changed coverage rate:
                                                                    |Lines       |Functions  |Branches    
  Filename                                                          |Rate     Num|Rate    Num|Rate     Num
  ========================================================================================================
  lib/concentrate/reporter/unskipped_null_stops.ex                  |14.3%      7|20.0%     5|    -      0

Download coverage report

github-actions · 2023-07-26T16:58:59Z

Coverage of commit `1be6052`

Summary coverage rate:
  lines......: 94.7% (1197 of 1264 lines)
  functions..: 83.5% (385 of 461 functions)
  branches...: no data found

Files changed coverage rate:
                                                                    |Lines       |Functions  |Branches    
  Filename                                                          |Rate     Num|Rate    Num|Rate     Num
  ========================================================================================================
  lib/concentrate/reporter/unskipped_null_stops.ex                  |14.3%      7|20.0%     5|    -      0

Download coverage report

github-actions · 2023-07-26T17:03:38Z

Coverage of commit `76d6f70`

Summary coverage rate:
  lines......: 94.7% (1197 of 1264 lines)
  functions..: 83.5% (385 of 461 functions)
  branches...: no data found

Files changed coverage rate:
                                                                    |Lines       |Functions  |Branches    
  Filename                                                          |Rate     Num|Rate    Num|Rate     Num
  ========================================================================================================
  lib/concentrate/reporter/unskipped_null_stops.ex                  |14.3%      7|20.0%     5|    -      0

Download coverage report

github-actions · 2023-07-26T17:27:10Z

Coverage of commit `1b59e6f`

Summary coverage rate:
  lines......: 94.3% (1197 of 1270 lines)
  functions..: 83.3% (385 of 462 functions)
  branches...: no data found

Files changed coverage rate:
                                                                    |Lines       |Functions  |Branches    
  Filename                                                          |Rate     Num|Rate    Num|Rate     Num
  ========================================================================================================
  lib/concentrate/filter/unscheduled_scheduled_stop.ex              | 0.0%      6| 0.0%     1|    -      0
  lib/concentrate/reporter/unskipped_null_stops.ex                  |14.3%      7|20.0%     5|    -      0

Download coverage report

github-actions · 2023-07-26T18:06:48Z

Coverage of commit `c9b7d72`

Summary coverage rate:
  lines......: 94.3% (1197 of 1270 lines)
  functions..: 83.3% (385 of 462 functions)
  branches...: no data found

Files changed coverage rate:
                                                                    |Lines       |Functions  |Branches    
  Filename                                                          |Rate     Num|Rate    Num|Rate     Num
  ========================================================================================================
  lib/concentrate/filter/unscheduled_scheduled_stop.ex              | 0.0%      6| 0.0%     1|    -      0
  lib/concentrate/reporter/unskipped_null_stops.ex                  |14.3%      7|20.0%     5|    -      0

Download coverage report

github-actions · 2023-07-26T19:23:57Z

Coverage of commit `84e1d3c`

Summary coverage rate:
  lines......: 94.3% (1197 of 1270 lines)
  functions..: 83.3% (385 of 462 functions)
  branches...: no data found

Files changed coverage rate:
                                                                    |Lines       |Functions  |Branches    
  Filename                                                          |Rate     Num|Rate    Num|Rate     Num
  ========================================================================================================
  lib/concentrate/filter/unscheduled_scheduled_stop.ex              | 0.0%      6| 0.0%     1|    -      0
  lib/concentrate/reporter/unskipped_null_stops.ex                  |14.3%      7|20.0%     5|    -      0

Download coverage report

github-actions · 2023-07-27T17:12:43Z

Coverage of commit `85fd45b`

Summary coverage rate:
  lines......: 95.2% (1202 of 1263 lines)
  functions..: 84.5% (386 of 457 functions)
  branches...: no data found

Files changed coverage rate:
                                                                    |Lines       |Functions  |Branches    
  Filename                                                          |Rate     Num|Rate    Num|Rate     Num
  ========================================================================================================
  lib/concentrate/filter/unscheduled_scheduled_stop.ex              |83.3%      6| 100%     1|    -      0

Download coverage report

paulswartz

I don't think you want to that at this stage. Commuter Rail predictions can exist without an arrival or departure time, but with a boarding_status.

Instead, I'd look at filtering these out of non-enhanced outputs, as those wouldn't include the boarding_status field.

Whoops · 2023-07-27T17:38:55Z

Instead, I'd look at filtering these out of non-enhanced outputs, as those wouldn't include the boarding_status field.

Ugh, and in fact they already are being filtered from non-enchanced feeds (https://github.com/mbta/concentrate/blob/master/lib/concentrate/encoder/trip_updates.ex#L28). But the predictions API uses the enhanced feed and so is publishing a bunch of nonsensical bus predictions. I'll need to come up with a new idea to deal with those. Especially since apparently similar predictions are valid in other contexts.

paulswartz · 2023-07-27T17:42:29Z

Given the original report, it looks like the issue is starting with Busloc. That application should never be generating predictions without at least one of an arrival/departure time AFAIK.

Whoops · 2023-07-27T17:54:17Z

As far as I can tell, it'll never output an arrival/departure time. Some waivers it publishes as skipped, some as scheduled (https://github.com/mbta/busloc/blob/master/lib/busloc/waiver/encoder/trip_updates_enhanced.ex#L71). The last time we went down this rabbit-hole we were unable to get a clear answer as to the purpose of the scheduled updates.

paulswartz · 2023-07-27T18:02:29Z

Right right, this is coming back to me now. Maybe filtering out STUs without a time OR boarding status would work?

Whoops · 2023-07-27T18:06:47Z

That seems reasonable. I'll make the change. Thanks!

github-actions · 2023-07-27T18:24:49Z

Coverage of commit `992e4dd`

Summary coverage rate:
  lines......: 95.1% (1202 of 1264 lines)
  functions..: 84.2% (385 of 457 functions)
  branches...: no data found

Files changed coverage rate:
                                                                    |Lines       |Functions  |Branches    
  Filename                                                          |Rate     Num|Rate    Num|Rate     Num
  ========================================================================================================
  lib/concentrate/filter/unscheduled_scheduled_stop.ex              |85.7%      7| 100%     1|    -      0

Download coverage report

paulswartz · 2023-07-27T19:11:31Z

lib/concentrate/filter/unscheduled_scheduled_stop.ex

+  def filter(%StopTimeUpdate{} = stu) do
+    scheduled? =
+      StopTimeUpdate.schedule_relationship(stu) == :SCHEDULED ||
+        StopTimeUpdate.schedule_relationship(stu) == nil


question: is it possible for this to be nil? I thought the parser converted it to the default value of :SCHEDULED?

It looks like your correct, I've removed the condition. I was working off the spec and the fact that concentrate will omit schedule_relationship in it's output if it's :SCHEDULED

github-actions · 2023-07-27T20:36:09Z

Coverage of commit `76778e9`

Summary coverage rate:
  lines......: 95.1% (1198 of 1260 lines)
  functions..: 84.2% (385 of 457 functions)
  branches...: no data found

Files changed coverage rate:
                                                                    |Lines       |Functions  |Branches    
  Filename                                                          |Rate     Num|Rate    Num|Rate     Num
  ========================================================================================================
  lib/concentrate/filter/unscheduled_scheduled_stop.ex              |66.7%      3| 100%     1|    -      0

Download coverage report

paulswartz

🍰

code looks good! can you deploy again to make sure it's working okay?

Whoops · 2023-07-28T13:59:12Z

code looks good! can you deploy again to make sure it's working okay?

Yep! I sent it to dev-green, I'll keep an eye on it for issues.

Log warnings on unskipped stops with no arrival/departure

d14b864

Whoops temporarily deployed to dev-green July 25, 2023 21:22 — with GitHub Actions Inactive

Whoops temporarily deployed to dev-green July 26, 2023 14:07 — with GitHub Actions Inactive

include full stu in message

1be6052

Whoops temporarily deployed to dev-green July 26, 2023 16:56 — with GitHub Actions Inactive

explicit fields for schedule_relationship and status

76d6f70

Whoops temporarily deployed to dev-green July 26, 2023 17:01 — with GitHub Actions Inactive

filter out STUs with no arrival/departure but are scheduled

1b59e6f

Whoops temporarily deployed to Dev-green July 26, 2023 18:01 — with GitHub Actions Inactive

return bare skip when skipping

c9b7d72

Whoops temporarily deployed to dev-green July 26, 2023 18:04 — with GitHub Actions Inactive

include filter in config

84e1d3c

Whoops temporarily deployed to dev-green July 26, 2023 19:20 — with GitHub Actions Inactive

remove uneeded reporter and add tests

85fd45b

Whoops changed the title ~~Log warnings on unskipped stops with no arrival/departure~~ fix: filter stop time updates without arrival/departure Jul 27, 2023

Whoops marked this pull request as ready for review July 27, 2023 17:12

Whoops requested a review from a team July 27, 2023 17:12

Whoops temporarily deployed to dev-green July 27, 2023 17:13 — with GitHub Actions Inactive

paulswartz requested changes Jul 27, 2023

View reviewed changes

Don't filter stus with a boarding status

992e4dd

Whoops requested a review from paulswartz July 27, 2023 18:25

paulswartz reviewed Jul 27, 2023

View reviewed changes

don't consider nil schedules

76778e9

paulswartz approved these changes Jul 28, 2023

View reviewed changes

Whoops temporarily deployed to dev-green July 28, 2023 13:58 — with GitHub Actions Inactive

Whoops merged commit e6f4a05 into master Aug 4, 2023
4 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: filter stop time updates without arrival/departure #294

fix: filter stop time updates without arrival/departure #294

Whoops commented Jul 25, 2023 •

edited

Loading

github-actions bot commented Jul 25, 2023

github-actions bot commented Jul 26, 2023

github-actions bot commented Jul 26, 2023

github-actions bot commented Jul 26, 2023

github-actions bot commented Jul 26, 2023

github-actions bot commented Jul 26, 2023

github-actions bot commented Jul 27, 2023

paulswartz left a comment

Whoops commented Jul 27, 2023

paulswartz commented Jul 27, 2023

Whoops commented Jul 27, 2023

paulswartz commented Jul 27, 2023

Whoops commented Jul 27, 2023

github-actions bot commented Jul 27, 2023

paulswartz Jul 27, 2023

Whoops Jul 27, 2023

github-actions bot commented Jul 27, 2023

paulswartz left a comment

Whoops commented Jul 28, 2023

fix: filter stop time updates without arrival/departure #294

fix: filter stop time updates without arrival/departure #294

Conversation

Whoops commented Jul 25, 2023 • edited Loading

Summary of changes

github-actions bot commented Jul 25, 2023

Coverage of commit d14b864

github-actions bot commented Jul 26, 2023

Coverage of commit 1be6052

github-actions bot commented Jul 26, 2023

Coverage of commit 76d6f70

github-actions bot commented Jul 26, 2023

Coverage of commit 1b59e6f

github-actions bot commented Jul 26, 2023

Coverage of commit c9b7d72

github-actions bot commented Jul 26, 2023

Coverage of commit 84e1d3c

github-actions bot commented Jul 27, 2023

Coverage of commit 85fd45b

paulswartz left a comment

Choose a reason for hiding this comment

Whoops commented Jul 27, 2023

paulswartz commented Jul 27, 2023

Whoops commented Jul 27, 2023

paulswartz commented Jul 27, 2023

Whoops commented Jul 27, 2023

github-actions bot commented Jul 27, 2023

Coverage of commit 992e4dd

paulswartz Jul 27, 2023

Choose a reason for hiding this comment

Whoops Jul 27, 2023

Choose a reason for hiding this comment

github-actions bot commented Jul 27, 2023

Coverage of commit 76778e9

paulswartz left a comment

Choose a reason for hiding this comment

Whoops commented Jul 28, 2023

Whoops commented Jul 25, 2023 •

edited

Loading

Coverage of commit `d14b864`

Coverage of commit `1be6052`

Coverage of commit `76d6f70`

Coverage of commit `1b59e6f`

Coverage of commit `c9b7d72`

Coverage of commit `84e1d3c`

Coverage of commit `85fd45b`

Coverage of commit `992e4dd`

Coverage of commit `76778e9`