refactor(gtfs): clean up dead cache code and add singleflight optimiz…

[3rabiii]

This follow-up PR addresses the outstanding review comments from the direction calculator performance PR #633

Changes:

• Dead Code Cleanup: Removed global_cache.go and SetContextCache. Kept SetShapeCache but updated the comments to explicitly state it's retained exclusively for DirectionPrecomputer startup optimization.
• Test Comments: Cleaned up stale // Optimization: Reuse shared DB and Cache comments in tests to reflect the new reality.
• Cache Lifecycle Documentation: Added a comment to directionResults (sync.Map) explaining why unbounded growth is acceptable (bounded by the finite number of real-world stops).
• Senior Touch (Singleflight): Added golang.org/x/sync/singleflight to CalculateStopDirection. Now, if 100 concurrent requests hit a cache miss for the exact same stop simultaneously, only one goroutine will query the DB and compute the shape, while the other 99 wait and share the result.

@aaronbrethorst

[aaronbrethorst]

Hey Adel, the direction of this PR is exactly right — removing the dead global_cache.go code, cleaning up stale comments, and adding singleflight to deduplicate concurrent DB hits is a meaningful improvement. There are two items to address before we can merge.

Critical

1. Singleflight captures the HTTP request's ctx, which can poison the permanent cache — advanced_direction_calculator.go:98

The singleflight callback captures ctx from the calling goroutine's HTTP request. If that request is cancelled (client disconnect, timeout), the DB query inside computeFromShapes fails and returns "". That empty string is then permanently stored in directionResults (line 108) and served to every future request for that stop — for the lifetime of the application.

The scenario:

1. Request A enters the singleflight for stopID "123" with ctx_A
2. Requests B, C coalesce behind it
3. ctx_A is cancelled (client disconnects)
4. computeFromShapes(ctx_A, ...) fails → returns ""
5. "" is cached permanently in directionResults
6. All future requests for "123" return "" from cache

Fix: detach the context so the shared computation isn't tied to a single request's lifecycle:

v, _, _ := adc.requestGroup.Do(stopID, func() (interface{}, error) {
    if cached, ok := adc.directionResults.Load(stopID); ok {
        return cached.(string), nil
    }
    computedDir := adc.computeFromShapes(context.WithoutCancel(ctx), stopID)
    adc.directionResults.Store(stopID, computedDir)
    return computedDir, nil
})

context.WithoutCancel (Go 1.21+, and you're on 1.24) preserves any values in the context while removing the cancellation signal. Alternatively, context.Background() works if there are no context values needed downstream.

Important

1. contextCache field is now dead code — advanced_direction_calculator.go:29, 167-169

The PR removed SetContextCache and deleted InitializeGlobalCache (the only code that ever set contextCache), but the contextCache field still exists on the struct and is still read in computeFromShapes:

hasCache := adc.contextCache != nil  // always false
if hasCache {
    stopTrips = adc.contextCache[stopID]  // unreachable
}

Since NewAdvancedDirectionCalculator doesn't initialize it and no remaining code sets it, contextCache is always nil. Please remove:

• The contextCache field from the struct (line 29)
• The dead cache-check block in computeFromShapes (lines 166-171)

This is exactly the kind of cleanup this PR is meant to accomplish — leaving it behind would be ironic!

Thanks again, and I look forward to merging this change.