GTFS-Flex: add adopted safe_duration_factor/offset to trips.txt; keep legacy stop_times duration fields#587
Open
drewda wants to merge 10 commits into
Open
GTFS-Flex: add adopted safe_duration_factor/offset to trips.txt; keep legacy stop_times duration fields#587drewda wants to merge 10 commits into
drewda wants to merge 10 commits into
Conversation
…fe_duration_factor columns closes interline-io/calact-network-analysis-tool#323
This comment was marked as outdated.
This comment was marked as outdated.
Resolve conflicts from concurrent test-fixture additions (this branch's hopelink-flex feed and main's WMATA feed) and the Trip GraphQL schema: - Combine both feeds' expected IDs in server/gql and server/rest fixtures (assertions use ElementsMatch, so order is irrelevant). - Keep main's rewritten Trip field descriptions plus this branch's new safe_duration_factor / safe_duration_offset fields in schema.graphqls; regenerate internal/generated/gqlout/generated.go. - Bump TestFeedResolver_Cursor feed query limit (10 -> 100) and TestOperatorRequest_Pagination expectLength (6 -> 7): each branch had independently grown the count to the old limit, so the union exceeded it. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- gofmt gtfs/trip.go: comment insertion had broken struct tag alignment. - Rename trip_safe_duration migration 20260403000001 -> 20260606000001. golang-migrate tracks a single high-water version and skips migrations older than the current DB version; main has since added migrations up to 20260605000001, so the original timestamp would never apply on databases already migrated past it, leaving safe_duration_* columns missing. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Adds GTFS-Flex “safe duration” support on trips.txt end-to-end (GTFS model → DB schemas → finder selects → GraphQL schema/output), and updates server fixtures/tests to include an additional GTFS-Flex feed used to exercise the new fields and license filtering behavior.
Changes:
- Add
safe_duration_factor/safe_duration_offsetto GTFSTrip, DB schemas (SQLite + Postgres migration), dbfinder trip SELECTs, and GraphQLTrip. - Add GraphQL coverage asserting safe-duration fields are returned for a Flex trip and
nullfor a non-Flex trip. - Expand test DMFR + adjust REST/GQL expectations (counts/IDs) to account for the newly added
hopelink-flexfeed and related entities.
Reviewed changes
Copilot reviewed 23 out of 25 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| testdata/server/server-test.dmfr.json | Adds hopelink-flex GTFS feed fixture (restrictive license) for tests. |
| server/rest/trip_request_test.go | Updates license-filtered trip count expectations due to added fixture data. |
| server/rest/stop_request_test.go | Updates license-filtered stop count expectations due to added fixture data. |
| server/rest/route_request_test.go | Updates expected route IDs/counts due to added fixture data. |
| server/rest/operator_request_test.go | Updates expected operators and license-filter results due to added fixture data. |
| server/rest/feed_version_request_test.go | Updates expected feed version SHA1 list to include new fixture version. |
| server/rest/feed_request_test.go | Updates expected feed lists and license-filter results to include hopelink-flex. |
| server/rest/agency_request_test.go | Updates expected agencies and license-filter results due to added fixture data. |
| server/gql/trip_resolver_test.go | Updates license-filter expectations and adds a new safe-duration GraphQL test. |
| server/gql/stop_resolver_test.go | Updates license-filter expectations due to added fixture data. |
| server/gql/route_resolver_test.go | Updates license-filter expectations and route list due to added fixture data. |
| server/gql/operator_resolver_test.go | Updates license-filter expectations due to added fixture data. |
| server/gql/feed_version_resolver_test.go | Updates expected feed version lists and license-filter expectations. |
| server/gql/feed_resolver_test.go | Updates expected feed lists and license-filter expectations to include hopelink-flex. |
| server/gql/booking_rule_resolver_test.go | Updates expected booking rule output (internal numeric id shifted by fixture changes). |
| server/gql/agency_resolver_test.go | Updates expected agency lists/location ordering and license-filter expectations. |
| server/finders/dbfinder/trip.go | Adds safe_duration_factor/offset to trip SELECT projection. |
| schema/sqlite/sqlite.sql | Adds safe_duration_factor/offset columns to gtfs_trips (SQLite schema). |
| schema/postgres/migrations/20260606000001_trip_safe_duration.up.pgsql | Adds safe_duration_factor/offset columns to gtfs_trips (Postgres migration). |
| schema/graphql/schema.graphqls | Exposes safe_duration_factor/offset on GraphQL Trip. |
| internal/generated/gqlout/generated.go | Regenerates gqlgen output to include new Trip fields. |
| gtfs/trip.go | Adds safe-duration fields to gtfs.Trip and validates presence/positivity. |
| gtfs/trip_test.go | Adds unit tests for new Trip safe-duration validation rules. |
| gtfs/stop_time.go | Updates GTFS-Flex field comments and marks stop_time safe/mean duration fields as deprecated/back-compat. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Correct the upstream references after checking the sources: - safe_duration_factor/offset were adopted into GTFS on trips.txt (google/transit#598, merged 2026-04), not stop_times.txt. The GTFS-Flex proposal originally placed them on stop_times.txt; some feeds still emit them there, so they are kept on StopTime for backward compat. - mean_duration_factor/offset remain only in the unadopted GTFS-Flex proposal (on stop_times.txt) and were not part of google/transit#598. The prior comment's "removed from spec per gtfs-flex#73" was inaccurate: #73 is an open question about conditional requirement, not a removal, and these fields were never in the official spec to begin with. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
drewda
changed the title
These two fixtures grew when origin/main (WMATA) and this branch (hopelink-flex) each added a feed/operator; the merge pushed the totals past values that each branch had independently matched. Left uncommitted in the merge resolution. - TestFeedResolver_Cursor: bump the feeds() query limit 10 -> 100 so the no-cursor / after-0 cases return all feeds and match FindFeeds (now 11, incl EX). - TestOperatorRequest_Pagination limit:1000: expectLength 6 -> 7. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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
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.
closes interline-io/calact-network-analysis-tool#323
Summary
GTFS-Flex's
safe_duration_factorandsafe_duration_offsetwere adopted into GTFS ontrips.txt(google/transit#598, merged 2026-04-22). This PR adds those two fields to theTripentity and plumbs them through persistence, the query layer, and the GraphQL API, with validation. The relatedmean_duration_*fields were never adopted into GTFS (they remain only in the unadopted GTFS-Flex proposal onstop_times.txt), so they are not added to trips; instead the existingstop_timesduration fields are reclassified as legacy back-compat.Adopted safe_duration on trips.txt
SafeDurationFactor/SafeDurationOffset(tt.Float) togtfs.Trip.safe_duration_factor/safe_duration_offsettogtfs_trips; matching columns in the SQLite schema.safe_duration_factor/safe_duration_offseton the GraphQLTriptype.Trip.ConditionalErrorsvalidation: the factor and offset must both be present or both absent, and the factor must be positive.Legacy stop_times duration fields
mean_duration_*andsafe_duration_*ongtfs.StopTimeas deprecated/back-compat, with comments documenting thatsafe_duration_*moved totrips.txtper the adopted spec andmean_duration_*was never adopted. Both are retained only so feeds that still emit them onstop_timescontinue to parse.Tests and fixtures
hopelink-flex) that carriessafe_duration_factor/offsetontrips.txtand the legacy duration fields onstop_times.txt, wired into the server test DMFR.Tripsafe_duration validation (both-present, each-missing, negative factor, zero offset).safe_duration_factor/safe_duration_offsetvalues on a flex trip andnullon a non-flex trip.Test plan
paratransitservices-wa-us--flex-v2); confirmtrips.txtsafe_duration_factor/offsetand the legacystop_timesmean_/safe_durationfields round-trip through read → write.go test ./gtfs/...and the server suitesgo test ./server/gql/... ./server/rest/...against a freshly set-up test database (the newhopelink-flexfixture must be imported).mainand confirmgtfs_trips.safe_duration_factor/safe_duration_offsetare created (the migration must sort after the latest existing migration).