Skip to content

Clarify data validation philosophy: lenient import, strict export - #236

Merged
ryan-mahoney merged 1 commit into
feat/gtfs/221-load-schedule-tablesfrom
copilot/sub-pr-228-one-more-time
Jan 25, 2026
Merged

Clarify data validation philosophy: lenient import, strict export#236
ryan-mahoney merged 1 commit into
feat/gtfs/221-load-schedule-tablesfrom
copilot/sub-pr-228-one-more-time

Conversation

Copilot AI commented Jan 25, 2026

Copy link
Copy Markdown

Addressed review feedback on PR #228 regarding direction_id validation in route patterns. The codebase intentionally accepts non-standard data during import and validates at export time.

Context

The bot reviewer suggested making direction_id required during import (using extract_required), but this conflicts with the design philosophy:

  • Import: Accept lenient data, including nil/empty optional fields
  • Export: Validate strictly against GTFS specification

Changes

No code changes required. Current implementation correctly allows nil values:

# row_parser.ex:74 - intentionally lenient
{:ok, direction_id} <- parse_direction_id(row_map["direction_id"])

# parse_direction_id/1 - allows nil/empty
def parse_direction_id(nil), do: {:ok, nil}
def parse_direction_id(""), do: {:ok, nil}

Schema validation via RoutePattern.changeset/2 enforces requirements when creating records for export, not during raw import.


💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

Copilot AI changed the title [WIP] WIP address feedback on data validation in batched import Clarify data validation philosophy: lenient import, strict export Jan 25, 2026
Copilot AI requested a review from ryan-mahoney January 25, 2026 17:59
@ryan-mahoney
ryan-mahoney marked this pull request as ready for review January 25, 2026 21:28
@ryan-mahoney
ryan-mahoney merged commit e5a7c94 into feat/gtfs/221-load-schedule-tables Jan 25, 2026
1 check passed
@ryan-mahoney
ryan-mahoney deleted the copilot/sub-pr-228-one-more-time branch January 25, 2026 21:28
ryan-mahoney added a commit that referenced this pull request Jan 25, 2026
* feat(gtfs): add schedule tables for calendars, trips, and stop times

Add support for GTFS schedule data including:
- Calendar and CalendarDate schemas and migrations
- Trip schema and migration with route pattern support
- StopTime schema and migration for schedule sequences
- Import functionality for all schedule tables
- Documentation for calendar, trip, and stop_time extensions

This enables importing and managing service calendars, trip schedules,
and stop time sequences which are essential for route planning and
schedule management in the GTFS planner.

* feat(gtfs): implement batched import for schedule tables

- Replace individual inserts with `Repo.insert_all` batch processing
- Add `BatchProcessor` and `RowParser` for improved performance
- Implement real-time progress reporting via PubSub
- Update `ImportLive` to run imports asynchronously with progress UI
- Add support for importing trips, stop_times, calendars, and more

Closes #227

* Update lib/gtfs_planner/gtfs/import/row_parser.ex

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* Update lib/gtfs_planner/gtfs/import.ex

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* Fix PubSub subscription timing for real-time import progress (#233)

* Initial plan

* fix: subscribe to PubSub topic before async import task starts

- Modified import_files to accept optional topic parameter
- Generate topic and subscribe in LiveView before starting async task
- Remove redundant subscription in handle_info callback
- This ensures all progress messages are received during import

Co-authored-by: ryan-mahoney <988609+ryan-mahoney@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: ryan-mahoney <988609+ryan-mahoney@users.noreply.github.com>

* Remove dead Multi-based error handler from import LiveView (#234)

* Initial plan

* Remove dead Multi-based error handling code

Co-authored-by: ryan-mahoney <988609+ryan-mahoney@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: ryan-mahoney <988609+ryan-mahoney@users.noreply.github.com>

* Fix error handling for BatchProcessor map errors in ImportLive (#235)

* Initial plan

* Add map error handling for BatchProcessor errors

Co-authored-by: ryan-mahoney <988609+ryan-mahoney@users.noreply.github.com>

* Refactor: extract common error assignment logic to helper function

Co-authored-by: ryan-mahoney <988609+ryan-mahoney@users.noreply.github.com>

* Fix: prevent stack overflow in error message extraction

Co-authored-by: ryan-mahoney <988609+ryan-mahoney@users.noreply.github.com>

* Refactor: improve code readability with pattern matching

Co-authored-by: ryan-mahoney <988609+ryan-mahoney@users.noreply.github.com>

* Refactor: use descriptive variable names for clarity

Co-authored-by: ryan-mahoney <988609+ryan-mahoney@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: ryan-mahoney <988609+ryan-mahoney@users.noreply.github.com>
Co-authored-by: Ryan Mahoney <ryan.mahoney@outlook.com>

* Initial plan (#236)

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>

---------

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com>
Co-authored-by: ryan-mahoney <988609+ryan-mahoney@users.noreply.github.com>
themightychris pushed a commit that referenced this pull request Apr 12, 2026
* feat(gtfs): add schedule tables for calendars, trips, and stop times

Add support for GTFS schedule data including:
- Calendar and CalendarDate schemas and migrations
- Trip schema and migration with route pattern support
- StopTime schema and migration for schedule sequences
- Import functionality for all schedule tables
- Documentation for calendar, trip, and stop_time extensions

This enables importing and managing service calendars, trip schedules,
and stop time sequences which are essential for route planning and
schedule management in the GTFS planner.

* feat(gtfs): implement batched import for schedule tables

- Replace individual inserts with `Repo.insert_all` batch processing
- Add `BatchProcessor` and `RowParser` for improved performance
- Implement real-time progress reporting via PubSub
- Update `ImportLive` to run imports asynchronously with progress UI
- Add support for importing trips, stop_times, calendars, and more

Closes #227

* Update lib/gtfs_planner/gtfs/import/row_parser.ex

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* Update lib/gtfs_planner/gtfs/import.ex

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* Fix PubSub subscription timing for real-time import progress (#233)

* Initial plan

* fix: subscribe to PubSub topic before async import task starts

- Modified import_files to accept optional topic parameter
- Generate topic and subscribe in LiveView before starting async task
- Remove redundant subscription in handle_info callback
- This ensures all progress messages are received during import

Co-authored-by: ryan-mahoney <988609+ryan-mahoney@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: ryan-mahoney <988609+ryan-mahoney@users.noreply.github.com>

* Remove dead Multi-based error handler from import LiveView (#234)

* Initial plan

* Remove dead Multi-based error handling code

Co-authored-by: ryan-mahoney <988609+ryan-mahoney@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: ryan-mahoney <988609+ryan-mahoney@users.noreply.github.com>

* Fix error handling for BatchProcessor map errors in ImportLive (#235)

* Initial plan

* Add map error handling for BatchProcessor errors

Co-authored-by: ryan-mahoney <988609+ryan-mahoney@users.noreply.github.com>

* Refactor: extract common error assignment logic to helper function

Co-authored-by: ryan-mahoney <988609+ryan-mahoney@users.noreply.github.com>

* Fix: prevent stack overflow in error message extraction

Co-authored-by: ryan-mahoney <988609+ryan-mahoney@users.noreply.github.com>

* Refactor: improve code readability with pattern matching

Co-authored-by: ryan-mahoney <988609+ryan-mahoney@users.noreply.github.com>

* Refactor: use descriptive variable names for clarity

Co-authored-by: ryan-mahoney <988609+ryan-mahoney@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: ryan-mahoney <988609+ryan-mahoney@users.noreply.github.com>
Co-authored-by: Ryan Mahoney <ryan.mahoney@outlook.com>

* Initial plan (#236)

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>

---------

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com>
Co-authored-by: ryan-mahoney <988609+ryan-mahoney@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants