GTFS-RT > ignore trip updates too much into the future...

[mmathieum]

• GTFS-RT > Trip Updates > too many real-time next departures (16h) in the future #190

[Copilot]

Pull request overview

Updates GTFS-RT handling to reduce displaying real-time trip updates that are too far into the future (issue #190), while also adjusting cache/age thresholds in multiple GTFS-RT providers.

Changes:

• Add logic to ignore TripUpdates whose first stop-time prediction is too far in the future (unless delay info is present), and extend TripUpdates max age.
• Refactor TripDescriptor matching logic and update corresponding unit tests.
• Adjust max-age thresholds for VehiclePositions and ServiceAlerts providers.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 6 comments.

File	Description
src/main/java/org/mtransit/android/commons/provider/status/GTFSRealTimeTripUpdatesProvider.kt	Adds “too-far-in-future” filtering + adjusts max-age constants; refactors TripDescriptor matching.
src/test/java/org/mtransit/android/commons/provider/status/GTFSRealTimeTripUpdatesProviderTests.kt	Updates match tests to align with the changed match() signature/behavior.
src/main/java/org/mtransit/android/commons/provider/vehiclelocations/GTFSRealTimeVehiclePositionsProvider.kt	Changes VehiclePositions max-age threshold.
src/main/java/org/mtransit/android/commons/provider/serviceupdate/GTFSRealTimeServiceAlertsProvider.kt	Changes ServiceAlerts max-age threshold and related comments.

Comments suppressed due to low confidence (2)

src/main/java/org/mtransit/android/commons/provider/status/GTFSRealTimeTripUpdatesProvider.kt:127

• New behavior to ignore trip updates too far in the future (based on stop-time event times) is introduced here, but there are no unit tests covering the boundary conditions (e.g., exactly at the threshold, missing arrival/departure times, unsorted STU list, presence of delay). Adding focused tests would help prevent regressions for issue #190.

        optStopTimeUpdateList?.firstOrNull()?.let { firstStu ->
            if (firstStu.optDeparture?.hasDelay() == true || firstStu.optArrival?.hasDelay() == true) return@let
            val timeMs = firstStu.optDeparture?.optTimeMs ?: firstStu.optArrival?.optTimeMs ?: return@let
            if (nowMs + FUTURE_TRIP_UPDATE_MAX_DIFF_MS < timeMs) {
                MTLog.d(LOG_TAG, "isUseful() > IGNORE ${(timeMs - nowMs).toDurationLog()} in the future: ${this.optTrip?.toStringExt(true)}")
                return false // not useful (too far in advance to display)
            }
        }

src/main/java/org/mtransit/android/commons/provider/serviceupdate/GTFSRealTimeServiceAlertsProvider.kt:115

• This PR references issue #190 about trip updates far in the future, but it also changes the service alerts cache max age. If unrelated, consider splitting or updating the PR description to explicitly include this behavior change so reviewers/users can assess impact separately.

    // Best practices: 10 minutes
    // https://gtfs.org/documentation/realtime/realtime-best-practices/#feed-publishing-general-practices
    // Montréal-based competitor: no limit
    @JvmStatic
    val SERVICE_ALERTS_MAX_AGE_MS = 10.times(13).minutes.inWholeMilliseconds

[coderabbitai]

Actionable comments posted: 2

🤖 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
`@src/main/java/org/mtransit/android/commons/provider/status/GTFSRealTimeTripUpdatesProvider.kt`:
- Line 125: Update the exemption in the GTFS real-time trip update flow to use
optVehicle != null, so any provided vehicle descriptor—including label-only or
license-plate-only descriptors—skips the future check. Keep the accompanying
comment aligned with this broader vehicle-info behavior.
- Around line 124-135: The future-trip check in isUseful() must ignore stop-time
updates without arrival or departure time. Filter optStopTimeUpdateList for
updates with a usable optDeparture.optTimeMs or optArrival.optTimeMs before
sorting and selecting firstOrNull(), then preserve the existing future threshold
logic; add a regression test covering a time-less first update followed by a
far-future timed update.

🪄 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: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 4c4c49ab-d78f-48ff-b426-bf3de24cbf42

📥 Commits

Reviewing files that changed from the base of the PR and between f651637 and efd6d87.

📒 Files selected for processing (2)

• src/main/java/org/mtransit/android/commons/provider/status/GTFSRealTimeTripUpdatesProvider.kt
• src/test/java/org/mtransit/android/commons/provider/status/GTFSRealTimeTripUpdatesProviderTests.kt

[mmathieum]

@coderabbitai resume

[coderabbitai]

✅ Action performed

Reviews resumed.

[coderabbitai]

Actionable comments posted: 3

🤖 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
`@src/main/java/org/mtransit/android/commons/provider/gtfs/GTFSDateTimeUtils.kt`:
- Around line 20-24: Update the GTFS date validation in the date-parsing method
around cleanedDate so it rejects any eight-character value containing non-digit
characters before parsing. Preserve the existing invalid-format warning and null
return, allowing only the YYYYMMDD digit grammar to reach the component parsing
logic.
- Line 31: Update the hours validation in the GTFS time parser around hoursCount
to accept zero as a valid hour, while still rejecting invalid or unparsable
values. Add a parsing test covering the valid midnight input "00:24:00" and its
expected result.

In
`@src/main/java/org/mtransit/android/commons/provider/status/GTFSRealTimeTripUpdatesProvider.kt`:
- Around line 134-139: Update the stop-time selection in the surrounding
trip-update time logic to map each sorted stop update to its absolute event time
(`optDeparture?.optTimeMs ?: optArrival?.optTimeMs`) before calling
`firstOrNull`, filtering out null times so the first candidate has an actual
timestamp. Do not select based on `hasDeparture()` or `hasArrival()` alone, and
retain the regression test covering a delay-only earlier update followed by a
future-timestamped update.

🪄 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: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 1c18996d-cff9-45fd-820b-b8ad97c678c0

📥 Commits

Reviewing files that changed from the base of the PR and between efd6d87 and 4c4f6b3.

📒 Files selected for processing (12)

• src/main/java/org/mtransit/android/commons/provider/CleverDevicesProvider.java
• src/main/java/org/mtransit/android/commons/provider/GTFSRealTimeProvider.java
• src/main/java/org/mtransit/android/commons/provider/NextBusProvider.java
• src/main/java/org/mtransit/android/commons/provider/OneBusAwayProvider.java
• src/main/java/org/mtransit/android/commons/provider/RSSNewsProvider.java
• src/main/java/org/mtransit/android/commons/provider/agency/AgencyUtils.kt
• src/main/java/org/mtransit/android/commons/provider/gtfs/GTFSDateTimeUtils.kt
• src/main/java/org/mtransit/android/commons/provider/gtfs/GTFSRealTimeProviderExt.kt
• src/main/java/org/mtransit/android/commons/provider/gtfs/GTFSScheduleTimestampsProvider.java
• src/main/java/org/mtransit/android/commons/provider/gtfs/GTFSStatusProvider.java
• src/main/java/org/mtransit/android/commons/provider/status/GTFSRealTimeTripUpdatesProvider.kt
• src/test/java/org/mtransit/android/commons/provider/gtfs/GTFSDateTimeUtilsTest.kt

[Copilot]

Pull request overview

Copilot reviewed 15 out of 15 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (11)

src/main/java/org/mtransit/android/commons/provider/status/GTFSRealTimeTripUpdatesProvider.kt:163

• Comment says "10 minutes" but 90.times(7).seconds is 10m30s. Either update the comment or use an exact 10-minute duration to match the documented intent.

    private val TRIP_UPDATE_MAX_AGE_MS = 90.times(7).seconds.inWholeMilliseconds

src/main/java/org/mtransit/android/commons/provider/status/GTFSRealTimeTripUpdatesProvider.kt:299

• match() disables the documented route/direction/start-time fallback with if (false) and then returns false when the trip_id isn’t in staticRDTripIds. This means trip updates without a matching/available trip_id can never match, despite the comment describing a fallback strategy.

        parseTripId(this)?.let { tripId ->
            if (tripId in staticRDTripIds) {
                return true // MATCH
            }
        }
        @Suppress("ConstantConditionIf")
        if (false) { // FIXME later try to match with route_id & direction_id & start date+time
            parseRouteId(this)?.let { rtRouteIdHash ->
                if (rtRouteIdHash != targetRouteIdHash) {

src/main/java/org/mtransit/android/commons/provider/serviceupdate/GTFSRealTimeServiceAlertsProvider.kt:114

• Comment says "no limit" but the code enforces a max age (10.times(13).minutes). This is misleading and makes it harder to reason about cache policy.

    // Montréal-based competitor: no limit
    @JvmStatic
    val SERVICE_ALERTS_MAX_AGE_MS = 10.times(13).minutes.inWholeMilliseconds

src/main/java/org/mtransit/android/commons/provider/vehiclelocations/GTFSRealTimeVehiclePositionsProvider.kt:179

• This PR is scoped to trip updates being ignored too far into the future (#190), but it also changes the vehicle-positions feed max-age threshold. If this is unrelated, consider splitting it into a separate PR or adding rationale in the PR description to keep changes auditable.

    // Best practices: 90 seconds
    // https://gtfs.org/documentation/realtime/realtime-best-practices/#feed-publishing-general-practices
    // Montréal-based competitor: 3 minutes
    private val VEHICLE_POSITIONS_MAX_AGE_MS = 90.times(2).seconds.inWholeMilliseconds

src/main/java/org/mtransit/android/commons/provider/status/GTFSRealTimeTripUpdatesProvider.kt:144

• New behavior is introduced to ignore trip updates that are too far in the future, but there’s no regression test validating that (e.g., a trip update 16h ahead is filtered out while near-future updates remain). Adding a unit/integration test would protect this bugfix.

            if (nowMs + FUTURE_TRIP_UPDATE_MAX_DIFF_MS < timeMs) {
                MTLog.d(LOG_TAG, "isUseful() > IGNORE ${((timeMs - nowMs)).toDurationLog()} in the future: ${this.optTrip?.toStringExt(true)}")
                return false // not useful (too far in advance to display)
            }

src/main/java/org/mtransit/android/commons/provider/gtfs/GTFSDateTimeUtils.kt:31

• parseToDateTime() currently rejects times with hour "00" because it requires hoursCount > 0. GTFS times can be "00:MM:SS" (and midnight departures), so this will incorrectly return null and can break time-based filtering (e.g., future trip-update detection).

            val hoursCount = parts.getOrNull(0)?.toIntOrNull()?.takeIf { it > 0 } ?: return null

src/test/java/org/mtransit/android/commons/provider/gtfs/GTFSDateTimeUtilsTest.kt:23

• Avoid printing to stdout in unit tests; it adds noise in CI and isn’t asserted. Remove this debug println.

            println(result?.toEpochMilliseconds())

src/main/java/org/mtransit/android/commons/provider/status/GTFSRealTimeTripUpdatesProvider.kt:139

• The future-date filtering is currently skipped when vehicle info is present (hasVehicleInfo), which can allow trip updates far in the future to still be considered useful (likely the exact problem reported in #190). Also, sorting the whole stop-time list just to find the first timed update is avoidable overhead.

This issue also appears on line 141 of the same file.

        val hasVehicleInfo = optVehicle?.let { it.hasId() || it.hasLabel() || it.hasLicensePlate() } == true
        val startDateTimeOrFirstTimeMs: Long? = if (hasVehicleInfo) null else
            (optTrip?.let { parseToDateTime(it.optStartDate, it.optStartTime, agencyTimeZone) }?.toMillis()
                ?: optTrip?.optModifiedTrip?.let { parseToDateTime(it.optStartDate, it.optStartTime, agencyTimeZone) }?.toMillis()
                ?: optStopTimeUpdateList?.sortedBy { it.optStopSequence }

src/main/java/org/mtransit/android/commons/provider/agency/AgencyUtils.kt:39

• This is a public @JvmStatic API rename (getRDSAgencyTimeZoneId -> getAgencyTimeZoneId). Given the surrounding comments about avoiding breaking compatibility with old modules, it’s safer to keep the old method as a deprecated wrapper to avoid downstream compile breaks.

    @JvmStatic
    fun getAgencyTimeZoneId(context: Context) =
        context.getAgencyString(

src/main/java/org/mtransit/android/commons/provider/GTFSRealTimeProvider.java:590

• Renaming the public static accessor from getAGENCY_TIME_ZONE to getAGENCY_TIME_ZONE_ID can break existing callers in other modules/apps. Consider keeping the old method name as a deprecated wrapper for binary/source compatibility.

	/**
	 * Override if multiple {@link GTFSRealTimeProvider} implementations in same app.
	 */
	@NonNull
	public static String getAGENCY_TIME_ZONE_ID(@NonNull Context context) {
		if (agencyTimeZoneId == null) {
			agencyTimeZoneId = context.getResources().getString(R.string.gtfs_real_time_agency_time_zone);
		}
		return agencyTimeZoneId;

src/main/java/org/mtransit/android/commons/provider/gtfs/GTFSRealTimeProviderExt.kt:43

• GTFSRealTimeProviderExt renamed the extension property from timeZone to timeZoneId. If this file is used as a shared API surface, adding a deprecated alias helps avoid breaking existing Kotlin callers.

val GTFSRealTimeProvider.timeZoneId get() = getAGENCY_TIME_ZONE_ID(requireContextCompat())
val GTFSRealTimeProvider.optTimeZoneId get() = timeZoneId.takeIf { it.isNotBlank() }

[Copilot]

Pull request overview

Copilot reviewed 16 out of 16 changed files in this pull request and generated no new comments.

Suppressed comments (4)

src/main/java/org/mtransit/android/commons/provider/status/GTFSRealTimeTripUpdatesProvider.kt:158

• Logging this path at WARN level can become very noisy because it triggers for every TripUpdate deemed “not useful” (e.g., scheduled-only updates). Consider lowering this to DEBUG (or removing) to avoid log spam and potential performance impact in production.

        MTLog.w(LOG_TAG, "isUseful() > IGNORE (why?): ${this.toStringExt()}")
        return false // not useful

src/main/java/org/mtransit/android/commons/provider/status/GTFSRealTimeTripUpdatesProvider.kt:302

• The fallback matching logic is currently unreachable (if (false) with a suppression), so the function effectively only matches when trip_id is present and in staticRDTripIds. This contradicts the comment above about using route/direction/start_date/start_time as a fallback and leaves dead code in production. Either implement/enable the fallback matching or remove the unreachable block and update the comment to reflect the actual behavior.

        @Suppress("ConstantConditionIf")
        if (false) { // FIXME later try to match with route_id & direction_id & start date+time
            parseRouteId(this)?.let { rtRouteIdHash ->

src/main/java/org/mtransit/android/commons/provider/status/GTFSRealTimeTripUpdatesProvider.kt:142

• optStopTimeUpdateList?.sortedBy { it.optStopSequence } allocates and sorts the entire list just to pick the first usable stop-time. This runs per TripUpdate and can be avoided by selecting the min stopSequence in a single pass (or relying on the feed order if guaranteed).

                ?: optStopTimeUpdateList?.sortedBy { it.optStopSequence }
                    ?.firstOrNull {
                        it.optDeparture?.optTimeOrScheduleTimeMs != null || it.optArrival?.optTimeOrScheduleTimeMs != null
                    }?.let {
                        it.optDeparture?.optTimeOrScheduleTimeMs ?: it.optArrival?.optTimeOrScheduleTimeMs

src/main/java/org/mtransit/android/commons/provider/status/GTFSRealTimeTripUpdatesProvider.kt:147

• The new “ignore trip updates too far in the future” behavior is central to the reported issue (#190), but there’s no unit test coverage exercising this branch. Since there are existing tests for this provider, consider adding a test case that builds a TripUpdate with a start_date/start_time (or STU time) > FUTURE_TRIP_UPDATE_MAX_DIFF_MS ahead of nowMs and verifies it’s filtered out.

        startDateTimeOrFirstTimeMs?.let { timeMs ->
            if (nowMs + FUTURE_TRIP_UPDATE_MAX_DIFF_MS < timeMs) {
                MTLog.d(LOG_TAG, "isUseful() > IGNORE ${((timeMs - nowMs)).toDurationLog()} in the future: ${this.optTrip?.toStringExt(true)}")
                return false // not useful (too far in advance to display)
            }

[Copilot]

Pull request overview

Copilot reviewed 16 out of 16 changed files in this pull request and generated no new comments.

Suppressed comments (4)

src/main/java/org/mtransit/android/commons/provider/agency/AgencyUtils.kt:43

• Renaming getRDSAgencyTimeZoneId() to getAgencyTimeZoneId() removes a @JvmStatic API without providing a deprecated alias, which can break downstream modules still calling the old method name. Consider keeping the old method as a deprecated wrapper to preserve source/binary compatibility.

    @JvmStatic
    fun getAgencyTimeZoneId(context: Context) =
        context.getAgencyString(
            R.string.poi_agency_timezone,
            R.string.gtfs_rts_timezone, // do not change to avoid breaking compat w/ old modules
            R.string.bike_station_timezone,
        ) ?: _defaultTimeZoneId

src/main/java/org/mtransit/android/commons/provider/status/GTFSRealTimeTripUpdatesProvider.kt:299

• The comment says there is a fallback match using route/direction/start_date/start_time, but the implementation hard-disables that path with if (false) and returns false unless trip_id is present and in staticRDTripIds. Either implement the fallback (as described) or update the comment/remove the dead branch to avoid misleading behavior (and potential ignoring of valid GTFS-RT TripDescriptors without trip_id).

    // Montréal-based competitor: match with 'trip_id' first,
    // else use 'route_id', 'direction_id', 'start_date' & 'start_time' to try to match correct trip as fallback

    @VisibleForTesting
    internal fun GTripDescriptor.match(
        targetRouteIdHash: String,
        targetDirectionOriginalId: Int?,
        staticRDTripIds: Set<String>,
        ignoreDirection: Boolean,
        parseRouteId: (GTripDescriptor) -> String?,
        parseTripId: (GTripDescriptor) -> String?,
    ): Boolean {
        parseTripId(this)?.let { tripId ->
            if (tripId in staticRDTripIds) {
                return true // MATCH
            }
        }
        @Suppress("ConstantConditionIf")
        if (false) { // FIXME later try to match with route_id & direction_id & start date+time
            parseRouteId(this)?.let { rtRouteIdHash ->

src/main/java/org/mtransit/android/commons/provider/GTFSRealTimeProvider.java:591

• getAGENCY_TIME_ZONE() was renamed to getAGENCY_TIME_ZONE_ID() without an alias. Since this is a public static API, this is a breaking change for any callers outside this repo. Consider reintroducing getAGENCY_TIME_ZONE() as a deprecated wrapper delegating to the new method.

	@Nullable
	private static String agencyTimeZoneId = null;

	/**
	 * Override if multiple {@link GTFSRealTimeProvider} implementations in same app.
	 */
	@NonNull
	public static String getAGENCY_TIME_ZONE_ID(@NonNull Context context) {
		if (agencyTimeZoneId == null) {
			agencyTimeZoneId = context.getResources().getString(R.string.gtfs_real_time_agency_time_zone);
		}
		return agencyTimeZoneId;
	}

src/main/java/org/mtransit/android/commons/provider/status/GTFSRealTimeTripUpdatesProvider.kt:144

• New behavior ignores trip updates that are too far in the future (FUTURE_TRIP_UPDATE_MAX_DIFF_MS), but there are no tests covering this filtering. Adding a regression test for issue #190 (e.g., a TripUpdate with first STU time > max diff being ignored) would help prevent reintroductions.

        val startDateTimeOrFirstTimeMs: Long? = if (hasVehicleInfo) null else
            (optTrip?.let { parseToDateTime(it.optStartDate, it.optStartTime, agencyTimeZone) }?.toMillis()
                ?: optTrip?.optModifiedTrip?.let { parseToDateTime(it.optStartDate, it.optStartTime, agencyTimeZone) }?.toMillis()
                ?: optStopTimeUpdateList // not sorting because GTFS spec requires it to be already sorted & it's a fallback
                    ?.firstOrNull { it.optDeparture?.hasTimeOrScheduledTime() == true || it.optArrival?.hasTimeOrScheduledTime() == true }
                    ?.let {
                        it.optDeparture?.optTimeOrScheduledTimeMs ?: it.optArrival?.optTimeOrScheduledTimeMs
                    })
        startDateTimeOrFirstTimeMs?.let { timeMs ->
            if (nowMs + FUTURE_TRIP_UPDATE_MAX_DIFF_MS < timeMs) {
                MTLog.d(LOG_TAG, "isUseful() > IGNORE ${((timeMs - nowMs)).toDurationLog()} in the future: ${this.optTrip?.toStringExt(true)}")
                return false // not useful (too far in advance to display)
            }