db: add queries that will be useful for the transit validation by tahini · Pull Request #1416 · chairemobilite/transition

tahini · 2025-07-08T02:24:35Z

See individual commits for details

One query uses the schedules to get the trips during a time range for a set of lines and services
One query gets the transferrable nodes between 2 paths.

Copilot

Pull Request Overview

Adds new database queries to support transit validation by fetching trips within a time window and identifying transferable nodes between paths.

Introduces getTripsInTimeRange in transitSchedules.db.queries.ts
Adds getTransferableNodePairs and its type in transitNodeTransferable.db.queries.ts
Provides test coverage for both new queries

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.

File	Description
packages/transition-backend/src/models/db/transitSchedules.db.queries.ts	Added `getTripsInTimeRange` query and export
packages/transition-backend/src/models/db/transitNodeTransferable.db.queries.ts	Added `getTransferableNodePairs` query and type
packages/transition-backend/src/models/db/tests/transitNodeTransferable.db.test.ts	Added tests for `getTransferableNodePairs`
packages/transition-backend/src/models/db/tests/TransitSchedule.db.test.ts	Added tests for `getTripsInTimeRange`

Comments suppressed due to low confidence (1)

packages/transition-backend/src/models/db/tests/TransitSchedule.db.test.ts:1075

No test covers the case where lineIds or serviceIds arrays are empty. Adding a test for empty inputs would ensure the function handles that scenario explicitly.

describe('getTripsInTimeRange', () => {

Copilot · 2025-07-08T02:26:09Z

packages/transition-backend/src/models/db/transitNodeTransferable.db.queries.ts

+                    'pnd.id as destination_path_id',
+                    'tn.*',
+                    knex.raw(
+                        'ROW_NUMBER() OVER (PARTITION BY pno.id, pnd.id, tn.origin_node_id ORDER BY tn.walking_travel_time_seconds) as rn'


The SQL comment above shows ordering by walking_travel_distance_meters but the query orders by walking_travel_time_seconds. If the intent is to pick shortest distance transfers, update the ORDER BY clause accordingly.

Suggested change

'ROW_NUMBER() OVER (PARTITION BY pno.id, pnd.id, tn.origin_node_id ORDER BY tn.walking_travel_time_seconds) as rn'

'ROW_NUMBER() OVER (PARTITION BY pno.id, pnd.id, tn.origin_node_id ORDER BY tn.walking_travel_distance_meters) as rn'

Copilot · 2025-07-08T02:26:09Z

packages/transition-backend/src/models/db/transitNodeTransferable.db.queries.ts

+                    .whereIn('pno.id', pathsFrom)
+                    .whereIn('pnd.id', pathsTo);
+            })
+            .select('*')


The returned objects include the SQL column rn (row number), which is not part of TransferableNodePair. Consider excluding rn in the mapping or explicitly picking only the needed fields to keep the output shape clean.

Suggested change

.select('*')

.select(

'origin_path_id',

'destination_path_id',

'origin_node_id',

'destination_node_id',

'walking_travel_time_seconds',

'walking_travel_distance_meters'

)

Copilot · 2025-07-08T02:26:09Z

packages/transition-backend/src/models/db/__tests__/TransitSchedule.db.test.ts

+        service1Line1.service_id = serviceId;
+        await dbQueries.save(service1Line1);
+
+        // duplicate scehdule for the same line, different service


Typo: scehdule should be schedule.

Suggested change

// duplicate scehdule for the same line, different service

// duplicate schedule for the same line, different service

Copilot · 2025-07-08T02:26:10Z

packages/transition-backend/src/models/db/transitSchedules.db.queries.ts

+        const trips: SchedulePeriodTrip[] = [];
+        for (let i = 0; i < rows.length; i++) {
+            trips.push(scheduleTripsAttributesParser(rows[i]));
+        }


[nitpick] You can simplify converting rows to trips by using rows.map(scheduleTripsAttributesParser) instead of a manual for loop.

Suggested change

const trips: SchedulePeriodTrip[] = [];

for (let i = 0; i < rows.length; i++) {

trips.push(scheduleTripsAttributesParser(rows[i]));

}

const trips = rows.map(scheduleTripsAttributesParser);

Copilot · 2025-07-08T02:26:10Z

packages/transition-backend/src/models/db/transitNodeTransferable.db.queries.ts

+    walking_travel_time_seconds: number;
+    walking_travel_distance_meters: number;
+};
+const getTransferableNodePairs = async ({


[nitpick] Consider validating that elements of pathsFrom and pathsTo are non-empty and valid UUIDs at the start of the function to fail fast on invalid input rather than relying on a downstream DB error.

This query returns the trips in a given time range for a list of lines and services. It is all the trips that have a departure time before the range end and an arrival time after the range start.

This query takes 2 arrays of paths: one for the unboarding nodes and another for the boarding nodes. It will find all the pairs of nodes that can transfer from one path to the other, returning the transfer travel time and distance. If many nodes are reachable from a path/node pair, it will return the one with the smallest time to reach.

greenscientist

Need more comments for the node transferable one.

With the comment provider in the commit message, I don't see what is the algo expected. This should go in a comment describing the function.
Do we do a path by path comparison ? (first one of one array and the first one of the other array?)

greenscientist · 2025-07-14T20:36:33Z

packages/transition-backend/src/models/db/transitSchedules.db.queries.ts

 };

+const getTripsInTimeRange = async ({
+    rangeStart,


Please specifiy format as comment (epoch, time from midnight? Second, minutes?)

Probably also add in the function comment header describing the feature that the time in the range is inclusive

tahini · 2025-07-21T19:59:15Z

packages/transition-backend/src/models/db/transitSchedules.db.queries.ts

+            .select([`${tripTable}.*`, `${scheduleTable}.service_id`, `${scheduleTable}.line_id`])
+            .join(periodTable, `${tripTable}.schedule_period_id`, `${periodTable}.id`)
+            .join(scheduleTable, `${periodTable}.schedule_id`, `${scheduleTable}.id`)
+            .whereIn(`${scheduleTable}.line_id`, lineIds)


Validate the performances of this query. The DB is 100% running in live tests and there may be missing indexes here... to investigate

tahini requested review from Copilot and greenscientist July 8, 2025 02:24

Copilot AI reviewed Jul 8, 2025

View reviewed changes

tahini added 2 commits July 8, 2025 08:26

schedules: add a query to get the trips in time range

42845f0

This query returns the trips in a given time range for a list of lines and services. It is all the trips that have a departure time before the range end and an arrival time after the range start.

tahini force-pushed the addQueries branch from ad4bbef to bcc2299 Compare July 8, 2025 12:27

greenscientist requested changes Jul 14, 2025

View reviewed changes

tahini commented Jul 21, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Comments

db: add queries that will be useful for the transit validation#1416

db: add queries that will be useful for the transit validation#1416
tahini wants to merge 2 commits intochairemobilite:mainfrom
tahini:addQueries

tahini commented Jul 8, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Jul 8, 2025

Uh oh!

Copilot AI Jul 8, 2025

Uh oh!

Copilot AI Jul 8, 2025

Uh oh!

Copilot AI Jul 8, 2025

Uh oh!

Copilot AI Jul 8, 2025

Uh oh!

greenscientist left a comment

Uh oh!

greenscientist Jul 14, 2025

Uh oh!

greenscientist Jul 14, 2025

Uh oh!

tahini Jul 21, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	'ROW_NUMBER() OVER (PARTITION BY pno.id, pnd.id, tn.origin_node_id ORDER BY tn.walking_travel_time_seconds) as rn'
	'ROW_NUMBER() OVER (PARTITION BY pno.id, pnd.id, tn.origin_node_id ORDER BY tn.walking_travel_distance_meters) as rn'

-            .select('*')
+            .select(
+                'origin_path_id',
+                'destination_path_id',
+                'origin_node_id',
+                'destination_node_id',
+                'walking_travel_time_seconds',
+                'walking_travel_distance_meters'
+            )

	// duplicate scehdule for the same line, different service
	// duplicate schedule for the same line, different service

Comments

Conversation

tahini commented Jul 8, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Jul 8, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jul 8, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jul 8, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jul 8, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jul 8, 2025

Choose a reason for hiding this comment

Uh oh!

greenscientist left a comment

Choose a reason for hiding this comment

Uh oh!

greenscientist Jul 14, 2025

Choose a reason for hiding this comment

Uh oh!

greenscientist Jul 14, 2025

Choose a reason for hiding this comment

Uh oh!

tahini Jul 21, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants