Add OTP 2.x GraphQL API support alongside the 1.x REST API - #150
Merged
Conversation
- New GraphQLAPIService actor implementing APIService via the GTFS GraphQL 'plan' query, mapping responses onto the existing public models so REST and GraphQL servers work interchangeably. - Routing errors map onto the existing ErrorResponse/ErrorResponseCode path; top-level GraphQL errors throw OTPKitError.apiError. - Demo app: regions declare an API type; added a Puget Sound OTP 2.x region backed by https://sound-transit-otp.ibi-transit.com/otp/. - Tests use fixtures captured from the live Sound Transit server.
- Map TransportMode.bike to the GraphQL Mode token BICYCLE; the REST rawValue BIKE fails GraphQL enum validation (verified live). - Extract shared wire plumbing (HTTP 200 validation, epoch-millis decoder, OTP router-path URL handling) used by both API services. - Drop fabricated OTP 1.x error ids from the routing-error mapping; consumers key off messageCode only. - Tests: reuse TestFixtures.makeTripPlanRequest and collapse repeated fixture arrange/act into a helper.
Mirrors the PR #146 guidance for REST error codes: unrecognized GraphQL routing error codes now log a warning instead of vanishing, with a test covering the .unknown mapping. Also corrects stale Itinerary doc comments (walk/transit/waiting times are seconds, not minutes).
📝 WalkthroughWalkthroughAdds OTP 2.x GTFS GraphQL trip planning, maps GraphQL responses into existing OTP models, shares HTTP handling with REST, adds coverage and fixtures, and lets the demo select REST or GraphQL services per region. ChangesOTP 2.x GraphQL support
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant DemoUser
participant OTPDemoViewController
participant GraphQLAPIService
participant OTPServer
DemoUser->>OTPDemoViewController: select GraphQL region
OTPDemoViewController->>GraphQLAPIService: construct service for region URL
OTPDemoViewController->>GraphQLAPIService: fetchPlan(request)
GraphQLAPIService->>OTPServer: POST GraphQL plan query
OTPServer-->>GraphQLAPIService: plan response or routing errors
GraphQLAPIService-->>OTPDemoViewController: OTPResponse
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
OTPKit/Sources/OTPKit/Network/GraphQLPlanResponse.swift (2)
119-235: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMove the mapping extensions into a dedicated file.
The GraphQL-to-public-model mapping extensions are substantial; place them in a descriptive mapping extension file, such as
GraphQLPlanResponse+Mapping.swift.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@OTPKit/Sources/OTPKit/Network/GraphQLPlanResponse.swift` around lines 119 - 235, Move the GraphQL-to-public-model mapping extensions for GraphQLPlan, GraphQLRoutingError, GraphQLItinerary, GraphQLLeg, GraphQLPlace, and GraphQLStep into a dedicated file named GraphQLPlanResponse+Mapping.swift. Preserve all existing mapping behavior and leave the response model definitions in GraphQLPlanResponse.swift.Source: Coding guidelines
23-115: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMake the GraphQL wire models
Codable.These response models are
Decodableonly, contrary to the repository requirement that models conform toCodable. Their properties already support encoding.Proposed fix
-struct GraphQLResponseEnvelope: Decodable { +struct GraphQLResponseEnvelope: Codable {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@OTPKit/Sources/OTPKit/Network/GraphQLPlanResponse.swift` around lines 23 - 115, Update all GraphQL wire model declarations in GraphQLResponseEnvelope, GraphQLErrorMessage, GraphQLPlanData, GraphQLPlan, GraphQLRoutingError, GraphQLItinerary, GraphQLLeg, GraphQLRoute, GraphQLAgency, GraphQLPlace, GraphQLStop, GraphQLLegGeometry, and GraphQLStep to conform to Codable instead of Decodable, preserving their existing properties and structure.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@OTPKit/Sources/OTPKit/Network/GraphQLPlanResponse.swift`:
- Around line 161-162: Update the ErrorResponse construction in the GraphQL
routing-error conversion path to preserve the missing ID rather than assigning
the fabricated -1 sentinel. Use the existing optional-ID representation
supported by ErrorResponse, while retaining description as message and
messageCode unchanged.
---
Nitpick comments:
In `@OTPKit/Sources/OTPKit/Network/GraphQLPlanResponse.swift`:
- Around line 119-235: Move the GraphQL-to-public-model mapping extensions for
GraphQLPlan, GraphQLRoutingError, GraphQLItinerary, GraphQLLeg, GraphQLPlace,
and GraphQLStep into a dedicated file named GraphQLPlanResponse+Mapping.swift.
Preserve all existing mapping behavior and leave the response model definitions
in GraphQLPlanResponse.swift.
- Around line 23-115: Update all GraphQL wire model declarations in
GraphQLResponseEnvelope, GraphQLErrorMessage, GraphQLPlanData, GraphQLPlan,
GraphQLRoutingError, GraphQLItinerary, GraphQLLeg, GraphQLRoute, GraphQLAgency,
GraphQLPlace, GraphQLStop, GraphQLLegGeometry, and GraphQLStep to conform to
Codable instead of Decodable, preserving their existing properties and
structure.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 4a567bbd-76b6-4797-bbc3-ae38bad70f0f
📒 Files selected for processing (14)
CLAUDE.mdDemo/OTPKitDemo/OTPDemoViewController.swiftDemo/OTPKitDemo/OnboardingViewController.swiftDemo/OTPKitDemo/models/OTPRegionInfo.swiftOTPKit/Sources/OTPKit/Core/Models/OTP/Itinerary.swiftOTPKit/Sources/OTPKit/Network/APIServiceSupport.swiftOTPKit/Sources/OTPKit/Network/GraphQLAPIService.swiftOTPKit/Sources/OTPKit/Network/GraphQLPlanResponse.swiftOTPKit/Sources/OTPKit/Network/RestAPIService.swiftOTPKit/Tests/Fixtures/graphql_plan_routing_error.jsonOTPKit/Tests/Fixtures/graphql_plan_success.jsonOTPKit/Tests/GraphQLAPIServiceTests.swiftOTPKit/Tests/Helpers/OTPTestCase.swiftdocs/superpowers/specs/2026-07-28-graphql-api-service-design.md
GraphQL routing errors carry no wire id, so preserve its absence rather than synthesizing a -1 sentinel. REST decoding is unaffected; nothing consumes the id, and consumers key off messageCode.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@OTPKit/Sources/OTPKit/Core/Models/OTP/ErrorResponse.swift`:
- Around line 23-25: Document the public API change to ErrorResponse.id in the
release notes, noting that its type changed from Int to Int? because OTP 2.x
GraphQL errors may not provide an identifier. Include migration guidance for
callers that compare, initialize, or otherwise treat id as non-optional,
requiring them to unwrap or handle nil.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 5e0e5e2c-fa65-4731-a4e2-e20b8bcdee2b
📒 Files selected for processing (3)
OTPKit/Sources/OTPKit/Core/Models/OTP/ErrorResponse.swiftOTPKit/Sources/OTPKit/Network/GraphQLPlanResponse.swiftOTPKit/Tests/GraphQLAPIServiceTests.swift
🚧 Files skipped from review as they are similar to previous changes (2)
- OTPKit/Sources/OTPKit/Network/GraphQLPlanResponse.swift
- OTPKit/Tests/GraphQLAPIServiceTests.swift
Comment on lines
+23
to
+25
| /// A unique identifier for the error. Only the OTP 1.x REST API supplies one; | ||
| /// errors mapped from the OTP 2.x GraphQL API have no id. | ||
| public let id: Int? |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -n --glob '*.swift' '\bErrorResponse\b|\.id\b' OTPKit DemoRepository: OneBusAway/otpkit
Length of output: 3320
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '--- ErrorResponse.swift ---\n'
cat -n OTPKit/Sources/OTPKit/Core/Models/OTP/ErrorResponse.swift
printf '\n--- GraphQL mapping snippets ---\n'
sed -n '100,175p' OTPKit/Sources/OTPKit/Network/GraphQLPlanResponse.swift | cat -n
printf '\n--- all ErrorResponse usages ---\n'
rg -n --glob '*.swift' '\bErrorResponse(\b|<|<.*>)|\.error\b|error\.id\s*' OTPKit Demo .
printf '\n--- relevant diff/stat files (if available) ---\n'
git diff --stat || true
git diff -- OTPKit/Sources/OTPKit/Core/Models/OTP/ErrorResponse.swift OTPKit/Sources/OTPKit/Network/GraphQLPlanResponse.swift OTPKit/Tests/GraphQLAPIServiceTests.swift 2>/dev/null | sed -n '1,220p' || trueRepository: OneBusAway/otpkit
Length of output: 246
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- ErrorResponse.swift ---'
cat -n OTPKit/Sources/OTPKit/Core/Models/OTP/ErrorResponse.swift
printf '%s\n' ''
printf '%s\n' '--- GraphQL mapping snippets ---'
sed -n '100,175p' OTPKit/Sources/OTPKit/Network/GraphQLPlanResponse.swift | cat -n
printf '%s\n' ''
printf '%s\n' '--- all ErrorResponse usages ---'
rg -n --glob '*.swift' 'ErrorResponse(\b|<|<.*>)|\.error\b|error\.id\s*' OTPKit Demo . || true
printf '%s\n' ''
printf '%s\n' '--- relevant diff/stat files (if available) ---'
git diff --stat || true
git diff -- OTPKit/Sources/OTPKit/Core/Models/OTP/ErrorResponse.swift OTPKit/Sources/OTPKit/Network/GraphQLPlanResponse.swift OTPKit/Tests/GraphQLAPIServiceTests.swift 2>/dev/null | sed -n '1,220p' || trueRepository: OneBusAway/otpkit
Length of output: 18261
Document the public API breaking change and include the migration impact in the release notes.
ErrorResponse.id is public, so changing it from Int to Int? will break callers that compare, initialize, or treat it as a non-optional integer. Repo call sites already handle nil, but documented migration guidance is needed for external API consumers.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@OTPKit/Sources/OTPKit/Core/Models/OTP/ErrorResponse.swift` around lines 23 -
25, Document the public API change to ErrorResponse.id in the release notes,
noting that its type changed from Int to Int? because OTP 2.x GraphQL errors may
not provide an identifier. Include migration guidance for callers that compare,
initialize, or otherwise treat id as non-optional, requiring them to unwrap or
handle nil.
This was referenced Jul 29, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
GraphQLAPIService, an actor implementing the existingAPIServiceprotocol against the OTP 2.x GTFS GraphQL API (<server>/otp/gtfs/v1), using theplanquery and mapping responses onto the same public models the REST path uses — so OTP 1.x and 2.x servers work interchangeably with zero changes to the ViewModel/UI layers.routingErrors) map onto the existingErrorResponse/ErrorResponseCodepath (with logging for unrecognized codes); top-level GraphQL errors throwOTPKitError.apiError.TransportModevalues are explicitly translated to the GraphQLModevocabulary (.bike→BICYCLE; the REST tokenBIKEfails GraphQL enum validation — verified against the live server).routers/<id>URL handling) is extracted intoAPIServiceSupport.swiftand used by both services. The demo app's regions now declare an API type, with a new "Seattle (OTP 2.x GraphQL)" region backed byhttps://sound-transit-otp.ibi-transit.com/otp/.Test plan
GraphQLAPIServiceTests(fixtures captured from the live Sound Transit OTP 2.10 server): endpoint URL normalization ×3, request-body construction, bike→BICYCLE mode mapping, itinerary/leg/place/step mapping (incl. GTFS extended route type 12 degrading to nilRouteType, intermediate stops, computedtransitTime), synthesizedrequestParameters, routing-error mapping, unknown-code fallback, top-level GraphQL error, HTTP error.apiType: graphQL+ sound-transit URL) → planned Current Location → Space Needle → 3 itineraries rendered (walk → Seattle Center Monorail → walk), matching the server's routing.planquery with REST-style date/time wire formats,maxWalkDistance, BICYCLE vs BIKE validation, andOUTSIDE_BOUNDSrouting-error responses.Review notes
planquery is used rather thanplanConnectionbecause it maps 1:1 onto the existing models and exists in every OTP 2.x release; migrating toplanConnectionlater is contained entirely withinGraphQLAPIService. Design doc:docs/superpowers/specs/2026-07-28-graphql-api-service-design.md.__UIApplicationEvaluateRuntimeIssueForNoSceneLifecycleAdoption) because it uses AppDelegate-only lifecycle with no scene manifest. End-to-end verification was done on iOS 26.3. Worth a follow-up issue for UIScene adoption.GraphQLAPIServicekeeps REST parity choices: it's an actor with no mutable state (matchesRestAPIService), exposesbaseURL, and synthesizes REST-stylerequestParameterssoOTPResponsestays uniform across backends.Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.Summary by CodeRabbit