-
Notifications
You must be signed in to change notification settings - Fork 3
Option to verify uniqueness of cleaned IDs (route, stop, trip, service) #22
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
mmathieum
commented
Nov 28, 2025
- Option to verify uniqueness of cleaned IDs (route, stop, trip, service) #20
|
/gemini review |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request introduces a valuable feature for verifying the uniqueness of cleaned IDs for services, routes, trips, and stops. The implementation is mostly correct and follows the existing patterns in the codebase.
My main concerns are:
- A potential
NullPointerExceptioninDefaultAgencyToolsdue to missing null checks, which could cause runtime failures. - The new verification logic in
MVerify.ktis highly repetitive and could be significantly simplified and improved for better error reporting.
I've provided specific suggestions to address these points. Additionally, the pattern of adding maps and methods for each ID type in DefaultAgencyTools is also quite repetitive, which is a minor maintainability concern you might want to address in the future.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR adds optional verification to ensure that cleaned IDs (service, route, trip, and stop IDs) remain unique after cleanup transformations are applied. The verification is opt-in and only runs when cleanup regex patterns are configured, with an additional option to explicitly allow non-unique IDs.
Key Changes:
- Introduced a new
MVerifyclass that checks for duplicate cleaned IDs and throws an error if duplicates are found - Added tracking maps to store original-to-cleaned ID mappings for all four ID types
- Added configuration options to explicitly allow non-unique IDs when needed
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| MVerify.kt | New verification utility that checks cleaned ID uniqueness across service, route, trip, and stop IDs |
| GAgencyTools.java | Added interface methods for uniqueness verification flags |
| RouteConfig.kt | Added configuration flags to allow non-unique route, trip, and stop IDs |
| AgencyConfig.kt | Added configuration flag to allow non-unique service IDs |
| DefaultAgencyTools.java | Implemented ID tracking maps, verification methods, and integrated verification into the generation process |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.