fix(cluj): re-add seq to static trip_id to disambiguate duplicate CTP CSV depTimes - #168
Merged
Merged
Conversation
… CSV depTimes
The static makeTripId was reduced from
<route>_<dir>_<serviceId>_<seq>_<HHMM> (old)
to
<route>_<dir>_<serviceId>_<HHMM> (broken)
on the assumption the seq "was never consumed by anyone." Wrong: it
absorbs the (rare) CTP CSV case where the same departure time appears
on two rows in the same direction column. The parser keeps both rows
(no dedup), the trip-emission loop then calls makeTripId for each and
produces two trips with the same trip_id, the second emission appends
a second full stop_times block to that trip, and the validate step
catches the resulting 0,1,2,...,11,0 sequence jump:
[validate] cluj-napoca.gtfs.zip: non-monotonic stop_sequence
for trip 63_0_LV_2255 (11 -> 0)
First observed on the daily cron on 2026-08-22 (every schedule run +
every manual dispatch since, 4+ days of red).
Reinstate the per-`departures` index in the trip_id:
<route>_<dir>_<serviceId>_<seq>_<HHMM>
`i` is already in scope at the call site; two 22:55s on M21 LV dir0
become <route>_0_LV_1_2255 and <route>_0_LV_2_2255, each with its
own monotonic stop_times block.
What this does NOT change:
- HHMM is still last, so neary's parseLiveStartMin `_(\d{3,4})$`
fallback and the verify-trip-id-format.ts `_\\d{4}$` check both
keep working unchanged.
- Frequency anchors keep their `<route>_<dir>_<serviceId>_FREQ_<HHMM>`
shape (built directly in derive/frequencies.ts, not via
makeTripId). They don't need a seq because the HHMM is a
window-start and windows are unique per direction.
- The cluj RT quirk (in src/rt/cluj.ts) parses the LIVE RT feed's
trip_ids, not static - different format, different concern, no
touch.
- The neary reconciler matches by (route, direction, time) and
never compares trip_ids by string equality, so the additional seq
slot is invisible to the JOIN.
Regression test: minimal fixture with a 3-stop Tranzy pattern and a
CSV that lists the same minute twice in dir0; asserts two distinct
trip_ids and a monotonic 0,1,2 in each.
ciotlos
approved these changes
Aug 25, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
fix(cluj): re-add seq to static trip_id to disambiguate duplicate CTP CSV depTimes
What broke
[release] daily multi-feed pipeline to R2has been failing every day since 2026-08-22 with:Root cause: CTP's
orar_M21_lv.csv(route 63 LV) has22:55on TWO rows in dir0:The CSV parser doesn't dedupe. The trip-emission loop calls
makeTripId(routeId, dir, serviceId, depTime)for each row and produces the same id63_0_LV_2255twice. The second emission appends a fresh 12-rowstop_timesblock to a trip that already has 12, sostop_sequencegoes 0,1,2,...,11,0 - validate catches the jump and the whole pipeline dies.Why the prior 4-part format was wrong
A previous "shorten trip_ids" cleanup reduced the static format from
to
on the assumption the seq "was never consumed by anyone." Wrong: it absorbs the (rare) CTP CSV case where the same departure time appears on two rows in the same direction. The seq is load-bearing.
The fix
Reinstate the per-
departuresindex in the trip_id:iis already in scope at the call site. Two22:55rows on M21 LV dir0 become<route>_0_LV_1_2255and<route>_0_LV_2_2255, each with its own monotonic stop_times block.Verified locally with a mini fixture (3-stop Tranzy pattern + CSV with the same minute twice in dir0): output trips.txt has two distinct trip_ids, each with
stop_sequence 0,1,2.What this does NOT change
neary'sparseLiveStartMinregex fallback (_(\d{3,4})$) and theverify-trip-id-format.tsself-check (_\d{4}$) both keep working unchanged.<route>_<dir>_<serviceId>_FREQ_<HHMM>shape (built directly inderive/frequencies.ts, not viamakeTripId). They don't need a seq because the HHMM is a window-start and windows are unique per direction.src/rt/cluj.tsparses the LIVE RT feed's trip_ids, not static - different format, different concern, no touch.(route, direction, time)and never compares trip_ids by string equality, so the additional seq slot is invisible to the JOIN.Files
adapters/cluj-napoca/src/assemble/emit/trips.ts-makeTripIdgains aseqparameter; the trip-emission loop passesi; full JSDoc rewritten to document the CTP-CSV-duplicate-time case and what the seq must NOT break.adapters/cluj-napoca/tests/reconcile.test.ts- existing trip_id regex bumped from 4-part to 5-part (and the_FREQslot is now optional in the seq position, not the HHMM position); the existing35_0_LV_0600reference updated to35_0_LV_0_0600; one new regression test that feeds a CSV with a duplicate depTime and asserts two distinct trip_ids, each with a monotonic 0,1,2 stop_times block.Test
pnpm test-> 209/209 pass (was 208/208 + 1 new regression test)pnpm check(tsc strict on src + test) -> cleanpnpm build-> cleanWhat happens after merge
Next 00:30 UTC cron run will pick up the fix, validate passes, the publish step lands fresh cluj-napoca SQLite + zip on R2, and the live n3ary.com app pulls the new data on its next SW refresh (the existing Aug 21 R2 data is still serving fine, so this is a clean cutover, not a recovery).