feat: add station first-last train schema#298
Conversation
|
Warning Review limit reached
More reviews will be available in 53 minutes and 14 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (7)
📝 WalkthroughWalkthroughRestructures station firstLastTrain from calendar rows to per-service timed windows, adds a StationLayout schema, updates validation/tests/fixtures to the new shape, and seeds CDT and LTI station data. ChangesStation Data Schema & Layout Expansion
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
duncanleo
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
packages/core/src/schema/Station.test.ts (1)
72-127: ⚡ Quick winAdd a test for the empty-service timing refine.
These cases cover invalid timing values, but they do not assert that a service with neither
timesnorspecialTimesis rejected. That is a new contract inStationFirstLastTrainServiceSchema, so it is worth pinning with a dedicated unit test.Suggested test
it('rejects timing rows without a first or last train time', () => { const result = StationSchema.safeParse({ ...minimalStation(), firstLastTrain: { services: [ { serviceId: 'ISL_MAIN_E', times: { weekday: { firstTrain: null, lastTrain: null, }, }, }, ], }, }); expect(result.success).toBe(false); expect(result.error?.issues[0]?.message).toBe( 'At least one of firstTrain or lastTrain must be set', ); }); + + it('rejects services without times or specialTimes', () => { + const result = StationSchema.safeParse({ + ...minimalStation(), + firstLastTrain: { + services: [{ serviceId: 'ISL_MAIN_E' }], + }, + }); + + expect(result.success).toBe(false); + expect(result.error?.issues[0]?.message).toBe( + 'At least one of times or specialTimes must be set', + ); + });🤖 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 `@packages/core/src/schema/Station.test.ts` around lines 72 - 127, Add a new unit test in Station.test.ts that constructs a station using StationSchema.safeParse with a firstLastTrain.services entry that has a serviceId (e.g., 'ISL_MAIN_E') but omits both times and specialTimes (or sets them to undefined/null), then assert result.success is false and that the validation errors include the path for that service (e.g., 'firstLastTrain.services.0' or the specific refine error from StationFirstLastTrainServiceSchema). This pins the new contract in StationFirstLastTrainServiceSchema that a service must provide at least one of times or specialTimes.scripts/generate-fixtures.mjs (1)
299-321: ⚡ Quick winSeed one
specialTimesexample in the generated fixture.This generator covers the new
timesshape, but it still never emitsspecialTimes. Since recurring special calendars are part of the new contract, adding oneeve_public_holidaysample here would give fixture-based validation an end-to-end path through that branch too.Suggested fixture update
services: [ { serviceId: 'ISL_MAIN_E', times: { weekday: { firstTrain: '06:00', lastTrain: '00:50', }, saturday: { firstTrain: '06:05', lastTrain: null, }, }, + specialTimes: { + eve_public_holiday: { + firstTrain: null, + lastTrain: '01:05', + }, + }, },🤖 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 `@scripts/generate-fixtures.mjs` around lines 299 - 321, Add a seeded specialTimes example into the generated fixture so the new recurring-special-calendar branch is exercised: inside the services array, pick one service object (e.g., the one with serviceId 'ISL_MAIN_E') and add a times.specialTimes entry containing a sample recurring key 'eve_public_holiday' with a small times object (e.g., firstTrain/lastTrain similar to weekday or saturday) so generated fixtures emit at least one specialTimes example alongside the existing times shape.
🤖 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 `@data/station/CDT.json`:
- Around line 34-88: The CDT.json firstLastTrain.services array is missing
entries for the CCW/extension serviceIds referenced by platforms CCDT_A and
CCDT_B (CCL_MAIN_CCW, CCL_EXT_CCW, CCL_EXT_CW), causing a mismatch; either add
corresponding timing objects for those serviceIds to firstLastTrain.services
(using the times from the matching data/service/*.json entries) or remove those
serviceIds from the CCDT_A/CCDT_B platform lists if they should not have
first/last timings—update the firstLastTrain.services array to include objects
with serviceId fields exactly matching CCL_MAIN_CCW, CCL_EXT_CCW, and CCL_EXT_CW
and appropriate weekday/saturday/sunday_public_holiday time blocks.
In `@packages/core/src/schema/Station.ts`:
- Around line 176-183: The StationLayoutPlatformSchema currently allows an empty
serviceIds array which breaks downstream coverage checks; update the serviceIds
field in StationLayoutPlatformSchema to require at least one entry (e.g., change
serviceIds: z.array(z.string()) to a non-empty array such as
z.array(z.string()).min(1) or z.array(z.string()).nonempty()) so every platform
must include at least one serviceId for correct line/service mapping.
---
Nitpick comments:
In `@packages/core/src/schema/Station.test.ts`:
- Around line 72-127: Add a new unit test in Station.test.ts that constructs a
station using StationSchema.safeParse with a firstLastTrain.services entry that
has a serviceId (e.g., 'ISL_MAIN_E') but omits both times and specialTimes (or
sets them to undefined/null), then assert result.success is false and that the
validation errors include the path for that service (e.g.,
'firstLastTrain.services.0' or the specific refine error from
StationFirstLastTrainServiceSchema). This pins the new contract in
StationFirstLastTrainServiceSchema that a service must provide at least one of
times or specialTimes.
In `@scripts/generate-fixtures.mjs`:
- Around line 299-321: Add a seeded specialTimes example into the generated
fixture so the new recurring-special-calendar branch is exercised: inside the
services array, pick one service object (e.g., the one with serviceId
'ISL_MAIN_E') and add a times.specialTimes entry containing a sample recurring
key 'eve_public_holiday' with a small times object (e.g., firstTrain/lastTrain
similar to weekday or saturday) so generated fixtures emit at least one
specialTimes example alongside the existing times shape.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: b832af4e-abc9-4055-863b-65c4ea8955e7
📒 Files selected for processing (9)
data/station/CDT.jsondata/station/LTI.jsondocs/plans/active/station-first-last-train-data.mddocs/plans/active/station-layout-data.mdpackages/core/src/schema/Station.test.tspackages/core/src/schema/Station.tspackages/fs/src/index.test.tspackages/fs/src/validate.tsscripts/generate-fixtures.mjs
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7a7cb204e4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
duncanleo
force-pushed
the
codex/station-first-last-layout-data
branch
from
3a38f9c to
5a8a347
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5a8a3473ce
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
duncanleo
force-pushed
the
codex/station-first-last-layout-data
branch
from
5a8a347 to
52addaa
Compare
duncanleo
force-pushed
the
codex/station-first-last-layout-data
branch
2 times, most recently
from
7dd70b3 to
00bc9dc
Compare
duncanleo
force-pushed
the
codex/station-first-last-layout-data
branch
from
00bc9dc to
e36a75b
Compare
1 participant
Summary
firstLastTrainschemas with normal and recurring special calendar timing maps.CDTandLTIstation timing data, including one SBS Transit sanity-check slice, and update the active plans to match the implemented shape.Validation
npm run test:corenpm run test:fsnpm run data:validatenpm run checkNote: Turbo printed its local
IO error: Operation not permittedwarning during some runs, but all validation commands exited successfully.Summary by CodeRabbit
New Features
Documentation
Tests