GTFS-RT > Trip Updates > never ignore CANCELLED trips because too much into the future

[mmathieum]

Later: could ignore when really far away into the future if it happens in the real world 🗺️ and it becomes a performance issue.

[qodo-code-review]

PR Summary by Qodo

GTFS-RT Trip Updates: never drop CANCELLED trips for being too far ahead

🐞 Bug fix 🕐 20-40 Minutes

AI Description

• Treat non-SCHEDULED trip updates (e.g., CANCELLED) as always displayable.
• Prevent future-start filtering from hiding cancellations far ahead of “now”.
• Simplify null-handling and logging around TripDescriptor access.

Diagram

graph TD
  A["GTFS-RT TripUpdate"] --> B["isUseful(now,tz)"] --> C{"scheduleRelationship\n!= SCHEDULED?"} -->|"yes"| D["Return useful\n(no max age)"]
  C -->|"no"| E{"timestamp too old?"} -->|"yes"| F["Ignore (too old)"]
  E -->|"no"| G{"start time too far\nin future?"} -->|"yes"| H["Ignore (too future)"]
  G -->|"no"| I["Use delays / STUs\n(or ignore why)"]

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Bypass only the future-start filter for CANCELLED

• ➕ Fixes the reported issue directly (cancellations hidden due to far-future start)
• ➕ Still allows stale CANCELLED updates to expire via existing max-age logic
• ➖ More conditional logic (need explicit CANCELLED handling)
• ➖ May still drop legitimate long-lived cancellations if the feed timestamp is old

2. Use a separate (longer) max-age for non-SCHEDULED updates

• ➕ Keeps cancellations visible longer without making them immortal
• ➕ Retains protection against indefinitely stale realtime data
• ➖ Requires tuning/maintaining another threshold
• ➖ May still be wrong for some agencies depending on their feed behavior

Recommendation: If the intent is truly “never ignore cancellations,” the PR’s early-return approach is the simplest and most robust against agency feeds that set far-future start times. However, it also disables max-age filtering for all non-SCHEDULED updates; if that’s broader than intended, consider limiting the bypass to CANCELLED or applying a separate, longer max-age for non-SCHEDULED.

Files changed (1) +10 / -9

Bug fix (1) +10 / -9

GTFSRealTimeTripUpdatesProvider.ktShort-circuit usefulness for non-scheduled (CANCELLED) trip updates +10/-9

Short-circuit usefulness for non-scheduled (CANCELLED) trip updates

• Adds an early check that treats any TripUpdate with a non-SCHEDULED scheduleRelationship as useful, bypassing max-age and future-start suppression. Also tightens null-handling by requiring optTrip up-front and updates debug logs to use the non-null TripDescriptor.

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

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

1. Non-scheduled bypasses freshness 🐞 Bug ≡ Correctness

Description

isUseful() now returns true for any scheduleRelationship != SCHEDULED before applying the
trip-update max-age and future-horizon checks, so CANCELED/DELETED (and other non-scheduled)
updates can be processed even when they would previously have been filtered out. This can keep
applying stale cancellations/deletions (or other relationship-only updates) to schedules
unexpectedly.

Code

src/main/java/org/mtransit/android/commons/provider/status/GTFSRealTimeTripUpdatesProvider.kt[R124-129]

+        val optTrip = optTrip ?: return false // not useful
+        optTrip.optScheduleRelationship?.let { scheduleRelationship ->
+            if (scheduleRelationship != GTDScheduleRelationship.SCHEDULED) {
+                return true // useful (no max age)
+            }
+        }

Relevance

● Weak

PR history prioritizes always processing CANCELLED/SKIPPED updates (PR99/178); this change aligns
with that intent over freshness guards.

PR-#99
PR-#178
PR-#189

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The new early return for non-SCHEDULED relationships happens before the existing max-age and
future-horizon checks, so those checks no longer run for cancellations/deletions. Downstream,
CANCELED and DELETED trip updates directly mutate schedules via
setTripCancelled/setTripDeleted, so accepting stale/out-of-horizon relationship updates changes
resulting schedules.

src/main/java/org/mtransit/android/commons/provider/status/GTFSRealTimeTripUpdatesProvider.kt[123-156]
src/main/java/org/mtransit/android/commons/provider/status/GTFSRealTimeTripUpdatesProviderExt.kt[129-141]
src/main/java/org/mtransit/android/commons/data/ScheduleExt.kt[79-103]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`GTripUpdate.isUseful()` short-circuits for any non-`SCHEDULED` `TripDescriptor.scheduleRelationship`, bypassing the existing max-age (`TRIP_UPDATE_MAX_AGE_MS`) and future-horizon (`FUTURE_TRIP_UPDATE_MAX_DIFF_MS`) filters.

## Issue Context
The PR intent (per title) is to avoid ignoring `CANCELED` trips due to being too far into the future. The current implementation changes behavior more broadly by also removing max-age/future filtering for other non-`SCHEDULED` values (e.g., `DELETED`, `ADDED`, `UNSCHEDULED`) and by skipping the max-age filter entirely for those.

## Fix Focus Areas
- src/main/java/org/mtransit/android/commons/provider/status/GTFSRealTimeTripUpdatesProvider.kt[123-156]

### Suggested direction
- Keep the max-age check for all trip updates (including cancellations/deletions).
- Narrow the "ignore too-far-in-future" exception to the specific relationship(s) intended (e.g., only `CANCELED`, possibly `DELETED` if desired), rather than `!= SCHEDULED`.
- Alternatively, move the relationship check to *after* the max-age check but *before* the future-horizon check, and gate it on `== CANCELED` (and any other explicitly desired values).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

_{To customize comments, go to the Qodo configuration screen, or learn more in the docs.}

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 770adb4c-1dc9-4e4f-9bfe-f8d51c3411da

📥 Commits

Reviewing files that changed from the base of the PR and between a26ebbf and 19c2460.

📒 Files selected for processing (1)

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

---

📝 Walkthrough

Summary by CodeRabbit

• Bug Fixes
  • Improved handling of real-time trip updates by accepting valid canceled or deleted trips without unnecessary age restrictions.
  • Continued applying timestamp, vehicle, delay, stop-time, and scheduling checks to other updates.
  • Ignored updates that do not include required trip information.

Walkthrough

GTripUpdate.isUseful now requires a trip descriptor. Deleted and canceled updates bypass maximum-age validation. Other updates retain their existing validation checks.

Changes

Trip update filtering

Layer / File(s)	Summary
Update usefulness validation src/main/java/org/mtransit/android/commons/provider/status/GTFSRealTimeTripUpdatesProvider.kt	isUseful rejects updates without a trip descriptor. It accepts deleted or canceled updates before the maximum-age check. Other updates retain timestamp, vehicle, start-time, delay, stop-time, and relationship validation.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

• mtransitapps/commons-android#189: Both changes modify GTripUpdate.isUseful timestamp filtering.
• mtransitapps/commons-android#192: Both changes refine GTripUpdate.isUseful trip-update filtering.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly identifies the main change: preserving CANCELLED trip updates that are far in the future.
Description check	✅ Passed	The description relates to the change by discussing a possible future-time limit for cancelled trips.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch mm/gtfs_rt_tu_max_age_cancel_trips

---

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (1)

src/main/java/org/mtransit/android/commons/provider/status/GTFSRealTimeTripUpdatesProvider.kt (1)

123-147: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Add regression coverage for the new ordering.

In src/test/java/org/mtransit/android/commons/provider/status/GTFSRealTimeTripUpdatesProviderTests.kt, exercise the provider path with:

• CANCELED and a start time beyond FUTURE_TRIP_UPDATE_MAX_DIFF_MS: accepted.
• SCHEDULED with the same start time: rejected.
• An old CANCELED timestamp: verify the intended no-max-age policy.
• A missing trip descriptor: rejected.

These tests will protect the cancellation fix and make the broader non-SCHEDULED behavior explicit.

🤖 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
`@src/main/java/org/mtransit/android/commons/provider/status/GTFSRealTimeTripUpdatesProvider.kt`
around lines 123 - 147, Add regression tests in
GTFSRealTimeTripUpdatesProviderTests covering isUseful/provider behavior for a
future CANCELED trip being accepted, an equivalent SCHEDULED trip being
rejected, an old CANCELED timestamp following the intended no-max-age policy,
and a missing trip descriptor being rejected. Reuse the existing test fixtures
and provider path to verify the new non-SCHEDULED ordering behavior.

🤖 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`:
- Around line 125-129: Update the scheduleRelationship handling in
GTFSRealTimeTripUpdatesProvider so the early return bypassing max-age checks
occurs only for CANCELED and DELETED; allow ADDED, UNSCHEDULED, REPLACEMENT,
DUPLICATED, and NEW to continue through timestamp filtering. Add regression
tests covering both terminal early-return behavior and non-terminal timestamp
filtering.

---

Nitpick comments:
In
`@src/main/java/org/mtransit/android/commons/provider/status/GTFSRealTimeTripUpdatesProvider.kt`:
- Around line 123-147: Add regression tests in
GTFSRealTimeTripUpdatesProviderTests covering isUseful/provider behavior for a
future CANCELED trip being accepted, an equivalent SCHEDULED trip being
rejected, an old CANCELED timestamp following the intended no-max-age policy,
and a missing trip descriptor being rejected. Reuse the existing test fixtures
and provider path to verify the new non-SCHEDULED ordering behavior.

🪄 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: 8bcc2e83-ac25-487a-920f-6168211eaf41

📥 Commits

Reviewing files that changed from the base of the PR and between 316d0d2 and a26ebbf.

📒 Files selected for processing (1)

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

[Copilot]

Pull request overview

Updates the GTFS-RT TripUpdate “usefulness” filter so that trip updates marking trips as CANCELED/DELETED are no longer discarded solely because the trip start time is far in the future, ensuring cancellations remain visible/actionable even when announced well ahead of time.

Changes:

• Treat TripDescriptor.schedule_relationship == CANCELED or DELETED as always “useful” (bypass max-age and far-future filtering).
• Simplify downstream logic by using a non-null local optTrip after an early return.