perf: fix N+1 query issue in stops-for-agency handler

[ARCoder181105]

Description:
This PR optimizes the stops-for-agency endpoint by replacing iterative database queries with batch operations, significantly improving performance for agencies with large numbers of stops.

Closes #289

Motivation:
Previously, buildStopsListForAgency suffered from an N+1 query problem. It iterated through every stop ID and executed two separate SQL queries (GetStop and GetRouteIDsForStop) for each one. For an agency with 5,000 stops, this resulted in over 10,000 database queries for a single API request.

Changes:
Refactored buildStopsListForAgency in internal/restapi/stops_for_agency_handler.go to:

1. Batch Fetch: Use GetStopsByIDs and GetRouteIDsForStops to fetch all data in just 2 queries, regardless of the number of stops.
2. In-Memory Mapping: Map route IDs to stops in memory to avoid repeated DB lookups.
3. Safety Check: Added an early return if len(stopIDs) == 0 to prevent unnecessary DB calls for empty lists.

Verification:

• Validated that the response structure remains identical to the previous implementation.
• Verified that the logic follows the batching patterns used in other handlers (e.g., stops_for_location_handler.go).
• make test passes.

[ARCoder181105]

@aaronbrethorst
The test are passing perfectly, the PR is ready for review

[aaronbrethorst]

Hey Aditya! Excellent work on tackling the N+1 query problem. Reducing from 10,000+ queries to 3 queries is a massive performance win.

Summary

This PR refactors buildStopsListForAgency to use batch queries (GetStopsByIDs and GetRouteIDsForStops) instead of iterating and querying each stop individually. This follows the same pattern established in stops_for_location_handler.go.

Critical Issues

1. Double-prefixed route IDs (pre-existing bug, reproduced in new code)

Location: internal/restapi/stops_for_agency_handler.go:102-105

for _, row := range routeIDsRows {
    if rid, ok := row.RouteID.(string); ok {
        routesByStop[row.StopID] = append(routesByStop[row.StopID], utils.FormCombinedID(agencyID, rid))
    }
}

The GetRouteIDsForStops query already returns combined IDs (see gtfsdb/query.sql:534):

routes.agency_id || '_' || routes.id AS route_id

So rid is already "agencyID_routeID". Calling FormCombinedID(agencyID, rid) produces "agencyID_agencyID_routeID".

Note: This bug existed in the old code too (FormCombinedID(agencyID, id.(string))), so the PR preserves existing behavior. However, since you're refactoring this code anyway, now is an ideal time to fix it.

Fix: Use rid directly, like stops_for_location_handler.go does:

for _, row := range routeIDsRows {
    if rid, ok := row.RouteID.(string); ok {
        routesByStop[row.StopID] = append(routesByStop[row.StopID], rid)  // Already combined
    }
}

Important Issues

1. Inconsistent indentation in early return

Location: internal/restapi/stops_for_agency_handler.go:83-86

	// If no stops, return empty list
    if len(stopIDs) == 0 {
        return []models.Stop{}, nil
    }

The first line uses tabs, but lines 84-86 use spaces. This should be consistent (use tabs per Go convention).

2. Behavior change: errors now propagate instead of being silently skipped

Location: internal/restapi/stops_for_agency_handler.go:89-98

The old code silently skipped stops that failed to fetch:

stop, err := api.GtfsManager.GtfsDB.Queries.GetStop(ctx, stopID)
if err != nil {
    continue  // Silent skip
}

The new code returns errors to the caller:

stops, err := api.GtfsManager.GtfsDB.Queries.GetStopsByIDs(ctx, stopIDs)
if err != nil {
    return nil, err  // Propagates error
}

This is actually a better behavior—failing fast is preferable to silently degraded responses. Just noting it as a behavioral change that's intentional and correct.

Fit and Finish

1. Trailing space in comment

Location: internal/restapi/stops_for_agency_handler.go:83

// If no stops, return empty list

Remove the trailing space after "list".

Strengths

• Significant performance improvement: 2N+1 queries → 3 queries is a major win
• Follows established patterns: Matches the approach in stops_for_location_handler.go
• Clean logic flow: Fetch all data upfront, then process in memory
• Good error handling: Propagates errors instead of silently degrading

Recommendation

Please fix the double-prefixed route IDs (use rid directly instead of wrapping in FormCombinedID) and the indentation issue. The route ID fix actually improves correctness beyond what the old code did.

After those fixes, this is ready to merge.

---

Great performance optimization work!

[ARCoder181105]

@aaronbrethorst
I addressed all the requested changes and also ran make fmt for making sure there are not any indentation problem ,
This PR is ready for review

[aaronbrethorst]

Hey Aditya, you've done a great job addressing all the feedback from the first round. The double-prefixed route ID fix is correct, the indentation is clean, and the code now follows the exact same batch query pattern used in stops_for_location_handler.go and stops_for_route_handler.go.

There's nothing left to change—this PR is ready to merge. Reducing 10,000+ individual queries down to 3 batch queries is a big performance win, and the fail-fast error handling is a nice improvement over the old silent continue behavior. Well done!