Skip to content

fix: stops-for-agency spec: 404, stop direction parity, code fallback, parent station#1078

Open
Ahmedhossamdev wants to merge 1 commit into
mainfrom
fix/stops-for-agency-spec-gaps
Open

fix: stops-for-agency spec: 404, stop direction parity, code fallback, parent station#1078
Ahmedhossamdev wants to merge 1 commit into
mainfrom
fix/stops-for-agency-spec-gaps

Conversation

@Ahmedhossamdev

@Ahmedhossamdev Ahmedhossamdev commented Jun 16, 2026

Copy link
Copy Markdown
Member

Fixes: #986, #1077

Description

Fixes four spec gaps in /api/where/stops-for-agency, verified against the live Java reference server (unitrans). stop-ids-for-agency was already compliant.

1. Unknown agency → 404
Was returning HTTP 200 + null. The spec's Implementation Decisions mandate 404 (correcting the legacy Java defect), and stop-ids-for-agency already did this. Fixed + updated the test that asserted 200.

2. Stop direction parity (112 → 3 mismatches)
Direction is computed from shape geometry. maglev diverged from Java on 41% of stops because it measured the bearing over a wide ±5-point chord; Java uses the single shape segment the stop lies on. Also dropped a cos(latitude) longitude correction that Java doesn't apply. Now matches Java on 272/275 stops; the remaining 3 are threshold/compass-boundary edge cases (Java projects via synthesized shape_dist_traveled, which the feed lacks). This fix improves every stop-direction-bearing endpoint, not just this one.

3. code fallback
Now falls back to the entity ID when the feed defines no stop code (matching the single-stop handler and spec).

4. parent + parent-station references
parent is now populated, and parent stations are resolved into references.stops (spec Extension 3a).

All tests + lint pass.

Note: directions are precomputed at GTFS load — rebuild the DB (delete gtfs.db) and restart to see the direction fix live.

Summary by CodeRabbit

  • Bug Fixes

    • /stops-for-agency endpoint now returns proper 404 error for invalid agencies.
  • New Features

    • /stops-for-agency endpoint now includes parent station information in responses.

… parent station references, fix direction calculations bugs
@coderabbitai

coderabbitai Bot commented Jun 16, 2026

Copy link
Copy Markdown

Review Change Stack

Caution

Review failed

An error occurred during the review process. Please try again later.

📝 Walkthrough

Walkthrough

Two independent fixes: (1) calculateOrientationAtStop in the GTFS direction calculator is rewritten to select the nearest adjacent shape segment via a new distanceToSegment helper and compute bearing from raw lon/lat deltas, removing the old window-based algorithm. (2) The stops-for-agency REST handler now returns 404 for missing agencies, populates each stop's Parent field, and resolves parent station references into the response.

Changes

Stop Orientation Algorithm Refactor

Layer / File(s) Summary
calculateOrientationAtStop rewrite and distanceToSegment helper
internal/gtfs/advanced_direction_calculator.go
Removes shapePointWindow constant. Rewrites orientation logic to find the adjacent segment (before or after the closest shape point) with minimum planar distance to the stop via a new distanceToSegment helper, then computes bearing using atan2 over raw lon/lat deltas without cosine-lat scaling.

Stops-for-Agency Handler Fixes

Layer / File(s) Summary
404 response, parent field, and reference resolution
internal/restapi/stops_for_agency_handler.go, internal/restapi/stops_for_agency_handler_test.go
nil-agency path now sends a not-found response instead of null. buildStopsListForAgency sets each stop's Parent field from the database ParentStation value. New buildParentStationReferences helper deduplicates parent IDs, queries the database, and maps results into models.Stop reference entries included in the list response. Test updated to assert HTTP 404 with "resource not found".

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related issues

Possibly related PRs

  • OneBusAway/maglev#855: Both PRs fix a REST handler to return 404 instead of a null/empty response when an agency is not found, following the same pattern applied to routes-for-agency.

Suggested reviewers

  • aaronbrethorst
  • burma-shave
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately captures the four main changes: 404 status for unknown agencies, direction calculation parity with Java, code fallback behavior, and parent station resolution.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/stops-for-agency-spec-gaps

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@sonarqubecloud

Copy link
Copy Markdown

@github-actions

Copy link
Copy Markdown

Performance Smoke Test Results

Status: PASSED

Metric Value
p(95) latency 2.1 ms
Error rate 0.00%
Total requests 332
Req/sec 10.9

Smoke test config: 5 VUs x 30s. Thresholds: p(95) < 300ms, error rate < 1%.

Full results uploaded as workflow artifact: k6-smoke-summary.

@OneBusAway OneBusAway deleted a comment from coderabbitai Bot Jun 16, 2026
@Ahmedhossamdev

Copy link
Copy Markdown
Member Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jun 16, 2026

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

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.

Spec review: stops-for-agency

1 participant