Agency-mode bounding box excludes blank-agency_id sibling feeds, causing false out-of-bounds vehicles

Open Beginner friendly
#149 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Assessment

Difficulty
2/5
Estimated time
1-3 hours
Newbie friendliness
88/100
Issue type
Bug
Clarity
Clearly specified
Activity status
Active
Tech stack
go
Domain
backend

Research direction

Start in internal/gtfs/gtfs_bundles.go, reading computeBoundingBoxes and storeStaticForServer, then add the provided TestAgencyModeBlankSiblingFeedBox in internal/gtfs/. Run the focused Go test and verify that agency-mode's stored bounding box includes stops from the sibling feed with a blank agency_id, while the existing server-mode behavior remains unchanged.

Written by the indexing model from the issue text.

Description

bug

Follow-up from #141, which is merged. This is a real regression against the pre-#141 behavior, but in a narrow enough config shape that I landed the improvement rather than holding it for another review round.

What happens

computeBoundingBoxes (internal/gtfs/gtfs_bundles.go) accumulates a stop into byAgency[id] only for the agency IDs declared by the bundle that stop came from, and it skips blank agency_id rows entirely:

for _, agency := range bundle.Agencies {
    if agency.Id == "" {
        continue
    }
    ...
}

In agency-mode, storeStaticForServer forces storageAgencies to the single configured server.AgencyID and then looks up agencyBoxes[declaredAgency.AgencyID]. If some configured feed declares that ID, the lookup succeeds and that feed-scoped box is stored — so a sibling feed whose agency.txt omits agency_id contributes only to the union box and is silently excluded from the box that agency-mode actually reads.

Before #141, agency-mode stored geo.ComputeBoundingBox(mergedbundle.Stops), which covered every configured feed.

Net effect: every stopped vehicle in the blank feed's territory counts toward gtfs_rt_stopped_out_of_bounds_vehicles. A monitoring false positive, which is the failure class this gauge exists to avoid.

Reproduction

Two static feeds in agency-mode: one declares agency-X with stops around (10,10)–(11,11), one omits agency_id with stops around (50,50)–(51,51).

stored agency-mode box: {MinLat:10 MaxLat:11 MinLon:10 MaxLon:11}
box main would store:   {MinLat:10 MaxLat:51 MinLon:10 MaxLon:51}
blank-feed stop (50,50) inside stored box? false

Drop this in internal/gtfs/ to see it fail:

func TestAgencyModeBlankSiblingFeedBox(t *testing.T) {
	declaring := makeSyntheticBundle(t, "agency-X", "Agency X", "https://x.example", []remoteGtfs.Stop{
		{Id: "s1", Latitude: floatPtr(10.0), Longitude: floatPtr(10.0)},
		{Id: "s2", Latitude: floatPtr(11.0), Longitude: floatPtr(11.0)},
	})
	blank := makeSyntheticBundle(t, "", "", "", []remoteGtfs.Stop{
		{Id: "s3", Latitude: floatPtr(50.0), Longitude: floatPtr(50.0)},
		{Id: "s4", Latitude: floatPtr(51.0), Longitude: floatPtr(51.0)},
	})
	server := models.ObaServer{
		ServerName: "agency-mode-server",
		ObaBaseURL: "https://oba.example.com",
		AgencyID:   "agency-X",
		AgencyName: "Agency X",
	}
	staticStore := NewStaticStore()
	bboxStore := geo.NewBoundingBoxStore()
	idx := NewRouteAgencyIndex()
	logger := slog.New(slog.NewTextHandler(io.Discard, nil))

	if err := storeStaticForServer(server, []*remoteGtfs.Static{declaring, blank}, staticStore, bboxStore, idx, nil, logger); err != nil {
		t.Fatalf("storeStaticForServer: %v", err)
	}
	box, _ := bboxStore.Get(server.ServerKey())
	// want the blank feed's (50,50) to be inside box; today it is not
}

(remoteGtfs is github.com/OneBusAway/go-gtfs.)

Suggested fix

In agency-mode, fold blank-agency_id feeds into the configured agency's accumulator — the union box already does exactly this, and the root CLAUDE.md and storeStaticForServer's own comment both note that a blank agency_id is legal for a single-agency feed. Server-mode doesn't need the same treatment: a blank-agency_id feed contributes no routeAgencyIndex entries either, so its vehicles are unattributed and validated against the union box anyway.

This is the agency-box half of the blank-agency_id problem raised in the first review round on #141; the union-box half was fixed in 62f6eef.

Dominant language
Go
Stars
23
Forks
24
Avg merge
9d 11h
Merged PRs (30d)
6

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from OneBusAway/watchdog

All issues in OneBusAway/watchdog

Similar issues

More Go issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.