Additional guards and checks for Flex trips (#538)

* guard against creating shapes for flex trips

fixes interline-io/calact-network-analysis-tool#258

* add a test

* Improve test case

* Move test

* Force rerun jobs

* Additional flex trip guards

* Test fixes

---------

Co-authored-by: Ian Rees <ian@ianrees.net>

Author: drewda

copier/copier.go

@@ -894,8 +894,12 @@ func (copier *Copier) copyTripsAndStopTimes() error {
 			trip.StopTimes = sts
 
 			// Set StopPattern
+			// Empty key means flex trip - assign unique pattern ID without caching
 			patkey := stopPatternKey(trip.StopTimes)
-			if pat, ok := stopPatterns[patkey]; ok {
+			if patkey == "" {
+				trip.StopPatternID.SetInt(len(stopPatterns))
+				stopPatterns[trip.TripID.Val] = trip.StopPatternID.Int() // Use trip ID as unique key
+			} else if pat, ok := stopPatterns[patkey]; ok {
 				trip.StopPatternID.SetInt(pat)
 			} else {
 				trip.StopPatternID.SetInt(len(stopPatterns))
@@ -909,8 +913,7 @@ func (copier *Copier) copyTripsAndStopTimes() error {
 					trip.ShapeID.Set(shapeid)
 				} else {
 					if shapeid, err := copier.createMissingShape(fmt.Sprintf("generated-%d-%d", trip.StopPatternID.Val, time.Now().Unix()), trip.StopTimes); err != nil {
-						copier.log.Error().Err(err).Str("filename", "trips.txt").Str("source_id", trip.EntityID()).Msg("failed to create shape")
-						trip.AddWarning(err)
+						copier.log.Debug().Err(err).Str("filename", "trips.txt").Str("source_id", trip.EntityID()).Msg("skipping shape generation")
 					} else {
 						// Set ShapeID
 						stopPatternShapeIDs[trip.StopPatternID.Int()] = shapeid
@@ -929,8 +932,12 @@ func (copier *Copier) copyTripsAndStopTimes() error {
 			}
 
 			// Set JourneyPattern
+			// Empty key means flex trip - don't deduplicate
 			jkey := copier.options.JourneyPatternKey(trip)
-			if jpat, ok := journeyPatterns[jkey]; ok {
+			if jkey == "" {
+				trip.JourneyPatternID.Set(trip.TripID.Val)
+				trip.JourneyPatternOffset.Set(0)
+			} else if jpat, ok := journeyPatterns[jkey]; ok {
 				trip.JourneyPatternID.Set(jpat.key)
 				trip.JourneyPatternOffset.SetInt(trip.StopTimes[0].ArrivalTime.Int() - jpat.firstArrival)
 				tripOffsets[trip.TripID.Val] = trip.JourneyPatternOffset.Int() // do not write stop times for this trip
@@ -1023,11 +1030,16 @@ func (copier *Copier) logCount(ent tt.Entity) {
 }
 
 func (copier *Copier) createMissingShape(shapeID string, stoptimes []gtfs.StopTime) (string, error) {
-	stopids := []string{}
+	// Only create shapes when ALL stop_times have stop_id set
+	// Fallback shape is undefined for location/location_group based stops
+	if !gtfs.CheckFlexStopTimes(stoptimes).CanUseStopBasedGeometry() {
+		return "", errors.New("cannot create shape: not all stop_times have stop_id (trip may use flex locations)")
+	}
+	if len(stoptimes) == 0 {
+		return "", errors.New("cannot create shape: no stop_times")
+	}
+	stopids := make([]string, 0, len(stoptimes))
 	for _, st := range stoptimes {
-		if !st.StopID.Valid {
-			continue
-		}
 		stopids = append(stopids, st.StopID.Val)
 	}
 	line, dists, err := copier.geomCache.MakeShape(stopids...)

copier/copier_test.go

@@ -7,6 +7,8 @@ import (
 
 	"github.com/interline-io/transitland-lib/adapters/direct"
 	"github.com/interline-io/transitland-lib/gtfs"
+	"github.com/interline-io/transitland-lib/internal/testpath"
+	"github.com/interline-io/transitland-lib/tlcsv"
 	"github.com/interline-io/transitland-lib/tt"
 	"github.com/stretchr/testify/assert"
 )
@@ -296,3 +298,47 @@ func TestResult_CheckErrorThreshold(t *testing.T) {
 		})
 	}
 }
+
+func TestCopier_CreateMissingShapes_FlexTrips(t *testing.T) {
+	// Test that CreateMissingShapes only creates shapes when ALL stop_times have stop_id
+	// Feed contains:
+	// - trip_regular: all stop_times have stop_id -> should get generated shape
+	// - trip_flex: one stop_time uses location_id instead of stop_id -> should NOT get shape
+	// - trip_with_shape: has existing shape -> should keep it
+	reader, err := tlcsv.NewReader(testpath.RelPath("testdata/gtfs-builders/flex-trip-create-shapes"))
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	writer := direct.NewWriter()
+	cpOpts := Options{
+		CreateMissingShapes: true,
+	}
+
+	cpResult, err := CopyWithOptions(context.Background(), reader, writer, cpOpts)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	// Verify results
+	wreader, _ := writer.NewReader()
+
+	// Collect trips and their shape IDs
+	tripShapes := map[string]string{}
+	for trip := range wreader.Trips() {
+		tripShapes[trip.TripID.Val] = trip.ShapeID.Val
+	}
+
+	// Trip with all stop_ids should have a generated shape
+	assert.NotEmpty(t, tripShapes["trip_regular"], "regular trip should have a shape")
+	assert.Contains(t, tripShapes["trip_regular"], "generated", "regular trip should have a generated shape")
+
+	// Flex trip should NOT have a shape (no shape generated because not all stop_times have stop_id)
+	assert.Empty(t, tripShapes["trip_flex"], "flex trip should not have a shape")
+
+	// Trip with existing shape should keep it
+	assert.Equal(t, "existing_shape", tripShapes["trip_with_shape"], "trip with existing shape should keep it")
+
+	// Verify generated shapes count
+	assert.Equal(t, 1, cpResult.GeneratedCount["shapes.txt"], "should have generated exactly 1 shape")
+}

copier/stoppattern.go

@@ -9,16 +9,22 @@ import (
 )
 
 func stopPatternKey(stoptimes []gtfs.StopTime) string {
+	// Skip flex trips - return empty key which won't match other patterns
+	if !gtfs.CheckFlexStopTimes(stoptimes).AllStopsHaveStopID {
+		return ""
+	}
 	key := make([]string, 0, len(stoptimes))
 	for i := 0; i < len(stoptimes); i++ {
-		if stoptimes[i].StopID.Valid {
-			key = append(key, stoptimes[i].StopID.Val)
-		}
+		key = append(key, stoptimes[i].StopID.Val)
 	}
 	return strings.Join(key, string(byte(0)))
 }
 
 func journeyPatternKey(trip *gtfs.Trip) string {
+	// Skip flex trips - return empty key to prevent deduplication
+	if !gtfs.CheckFlexStopTimes(trip.StopTimes).AllStopsHaveStopID {
+		return ""
+	}
 	m := sha1.New()
 	a := trip.StopTimes[0].ArrivalTime
 	b := trip.StopTimes[0].DepartureTime

ext/bestpractices/block_overlap.go

@@ -52,6 +52,12 @@ func (e *BlockOverlapCheck) Validate(ent tt.Entity) []error {
 	if !ok || !trip.BlockID.Valid || len(trip.StopTimes) < 2 {
 		return nil
 	}
+	// Skip flex trips - block overlap check requires fixed arrival/departure times
+	// Flex trips use time windows instead and don't have meaningful overlap semantics
+	flexInfo := gtfs.CheckFlexStopTimes(trip.StopTimes)
+	if flexInfo.IsFlexTrip() {
+		return nil
+	}
 	if e.blocks == nil {
 		e.blocks = map[string][]*tripBlockInfo{}
 	}

ext/bestpractices/fast_travel.go

@@ -97,15 +97,13 @@ func (e *StopTimeFastTravelCheck) Validate(ent tt.Entity) []error {
 	}
 	// todo: cache for trip pattern?
 	var errs []error
-	if len(trip.StopTimes) == 0 || !trip.StopTimes[0].StopID.Valid {
+	// Skip flex trips that cannot use stop-based geometry
+	if len(trip.StopTimes) == 0 || !gtfs.CheckFlexStopTimes(trip.StopTimes).CanUseStopBasedGeometry() {
 		return errs
 	}
 	s1 := trip.StopTimes[0].StopID.Val
 	t := trip.StopTimes[0].DepartureTime
 	for i := 1; i < len(trip.StopTimes); i++ {
-		if !trip.StopTimes[i].StopID.Valid {
-			continue
-		}
 		s2 := trip.StopTimes[i].StopID.Val
 		key := s1 + ":" + s2 // todo: use a real separator...
 		dx, ok := e.stopDist[key]

ext/builders/route_geometry_builder.go

@@ -68,6 +68,10 @@ func (pp *RouteGeometryBuilder) AfterWrite(eid string, ent tt.Entity, emap *tt.E
 		for i, c := range v.Geometry.Val.Coords() {
 			pts[i] = tlxy.Point{Lon: c[0], Lat: c[1]}
 		}
+		// Skip shapes with no coordinates (invalid shapes)
+		if len(pts) == 0 {
+			return nil
+		}
 		// If we've already seen this line, re-use shapeInfo to reduce mem usage
 		for _, si := range pp.shapeInfos {
 			// Match on generated value too

ext/builders/route_geometry_builder_test.go

@@ -1,10 +1,14 @@
 package builders
 
 import (
+	"context"
 	"testing"
 
+	"github.com/interline-io/transitland-lib/adapters/direct"
+	"github.com/interline-io/transitland-lib/copier"
 	"github.com/interline-io/transitland-lib/internal/testpath"
 	"github.com/interline-io/transitland-lib/internal/testreader"
+	"github.com/interline-io/transitland-lib/tlcsv"
 	"github.com/interline-io/transitland-lib/tlxy"
 	"github.com/stretchr/testify/assert"
 	"github.com/twpayne/go-geom"
@@ -97,3 +101,22 @@ func TestRouteGeometryBuilder(t *testing.T) {
 		})
 	}
 }
+
+func TestRouteGeometryBuilder_FlexFeed(t *testing.T) {
+	reader, err := tlcsv.NewReader(testpath.RelPath("testdata/gtfs-external/ctran-flex.zip"))
+	if err != nil {
+		t.Fatal(err)
+	}
+	writer := direct.NewWriter()
+	cpOpts := copier.Options{
+		CreateMissingShapes: true,
+	}
+	cpOpts.AddExtension(NewRouteGeometryBuilder())
+
+	cpResult, err := copier.CopyWithOptions(context.Background(), reader, writer, cpOpts)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	assert.NotNil(t, cpResult)
+}

ext/sched/sched.go

@@ -37,7 +37,9 @@ func (fi *ScheduleChecker) Validate(ent tt.Entity) []error {
 		ti := tripInfo{
 			ServiceID: v.ServiceID.Val,
 		}
-		if len(v.StopTimes) > 0 {
+		// Only track fixed-route trips with arrival/departure times
+		// Flex trips use time windows instead of fixed times
+		if len(v.StopTimes) > 0 && !gtfs.CheckFlexStopTimes(v.StopTimes).IsFlexTrip() {
 			ti.StartTime = v.StopTimes[0].DepartureTime
 			ti.EndTime = v.StopTimes[len(v.StopTimes)-1].ArrivalTime
 		}

gtfs/stop_time.go

@@ -406,3 +406,67 @@ func (ent *StopTime) SetString(key, value string) error {
 	}
 	return perr
 }
+
+// FlexInfo contains information about flex characteristics of a trip's stop times
+type FlexInfo struct {
+	HasFlexStopTimes     bool // Any StopTime uses Location or LocationGroup instead of StopID
+	HasFlexTimeWindows   bool // Any StopTime uses pickup/dropoff time windows
+	HasMixedStopTypes    bool // Mix of regular stops and flex locations
+	AllStopsHaveStopID   bool // All StopTimes have a valid StopID
+	FlexStopTimeCount    int  // Number of StopTimes with flex characteristics
+	RegularStopTimeCount int  // Number of StopTimes with regular StopID
+}
+
+// IsFlexTrip returns true if any StopTime has flex characteristics
+func (f FlexInfo) IsFlexTrip() bool {
+	return f.HasFlexStopTimes || f.HasFlexTimeWindows
+}
+
+// CanUseStopBasedGeometry returns true if the trip can use stop-based geometry calculations
+// (i.e., all StopTimes have valid StopIDs)
+func (f FlexInfo) CanUseStopBasedGeometry() bool {
+	return f.AllStopsHaveStopID
+}
+
+// CheckFlexStopTimes analyzes a slice of StopTimes and returns information about
+// whether they represent fixed-route service or flex service.
+func CheckFlexStopTimes(stoptimes []StopTime) FlexInfo {
+	info := FlexInfo{
+		AllStopsHaveStopID: true,
+	}
+
+	for i := 0; i < len(stoptimes); i++ {
+		st := stoptimes[i]
+
+		// Check for flex location types
+		hasLocationID := st.LocationID.Valid && st.LocationID.Val != ""
+		hasLocationGroupID := st.LocationGroupID.Valid && st.LocationGroupID.Val != ""
+		hasStopID := st.StopID.Valid && st.StopID.Val != ""
+
+		// Check for flex time windows
+		hasTimeWindow := (st.StartPickupDropOffWindow.Valid && st.StartPickupDropOffWindow.Val > 0) ||
+			(st.EndPickupDropOffWindow.Valid && st.EndPickupDropOffWindow.Val > 0)
+
+		if hasLocationID || hasLocationGroupID {
+			info.HasFlexStopTimes = true
+			info.FlexStopTimeCount++
+		}
+
+		if hasTimeWindow {
+			info.HasFlexTimeWindows = true
+		}
+
+		if hasStopID {
+			info.RegularStopTimeCount++
+		} else {
+			info.AllStopsHaveStopID = false
+		}
+	}
+
+	// Mixed if we have both regular stops and flex locations
+	if info.RegularStopTimeCount > 0 && info.FlexStopTimeCount > 0 {
+		info.HasMixedStopTypes = true
+	}
+
+	return info
+}

internal/geomcache/geomcache.go

@@ -111,6 +111,11 @@ func (g *GeomCache) InterpolateStopTimes(trip *gtfs.Trip) ([]gtfs.StopTime, erro
 		return sts, nil
 	}
 
+	// Skip flex trips that cannot use stop-based geometry
+	if !gtfs.CheckFlexStopTimes(sts).CanUseStopBasedGeometry() {
+		return sts, nil
+	}
+
 	// Do we have valid ShapeDistTraveled values?
 	validDists := true
 	if sts[len(sts)-1].ShapeDistTraveled.Val-sts[0].ShapeDistTraveled.Val <= 0 {
@@ -142,9 +147,6 @@ func (g *GeomCache) setStopTimeDists(shapeId string, patternId int64, sts []gtfs
 		// Generate the stop-to-stop geometry
 		stopLine := make([]tlxy.Point, 0, len(sts))
 		for i := 0; i < len(sts); i++ {
-			if !sts[i].StopID.Valid {
-				continue
-			}
 			point, ok := g.stops[sts[i].StopID.Val]
 			if !ok {
 				return fmt.Errorf("stop '%s' not in cache", sts[i].StopID)