Skip to content

refactor: remove legacy backend monolith and streamline DB operations - #35

Merged
rsun19 merged 5 commits into
mainfrom
streamline-db-operations
May 13, 2026
Merged

rsun19 merged 5 commits into
mainfrom
streamline-db-operations

Conversation

@rsun19

@rsun19 rsun19 commented May 12, 2026

Copy link
Copy Markdown
Owner

Summary

Removes the legacy backend/ monolith (fully superseded by the services/ microservices) and improves database operations across services.

Changes

Removed

  • backend/ directory — monolithic NestJS app including all modules, entities, tests, and Dockerfiles

Services

  • ingestion: Improve gtfs-static.service.ts with bulk upserts and optimized batch queries
  • stops: Refactor stops.service.ts DB operations for better performance; migrate test specs from backend/
  • routes: Simplify query logic in routes.service.ts

Shared

  • Add new entity fields to route.entity.ts and stop.entity.ts
  • Add constants to constants.ts

CI / Scripts

  • Update check-unit-coverage.mjs, run-gate.mjs, and typecheck-workspace.mjs to reflect new workspace layout

Docs

  • Update configuration.md, data-model.md, and development.md for microservices architecture

Summary by CodeRabbit

  • Refactor

    • Backend reorganized into microservices (agencies, routes, stops, alerts, vehicles, ingestion) with service-scoped implementations.
  • New Features / Performance

    • Introduced precomputed ingestion tables and colocated-stop grouping for faster, more consistent search, arrival, and route responses.
    • Moved stop-merge and route-lookup work to precomputed data.
  • CI / Release

    • Simplified CI gates and unified workspace lint/typecheck to target frontend + services.
  • Documentation

    • Project layout, data model, and development docs updated for microservices.

Review Change Stack

- Delete backend/ monolith (superseded by services/ microservices)
- Improve gtfs-static ingestion with bulk upserts and optimized queries
- Refactor stops service DB operations for better performance
- Simplify routes service queries
- Add shared entity fields and constants
- Update CI scripts to reflect new workspace layout
- Update docs for microservices architecture
@coderabbitai

coderabbitai Bot commented May 12, 2026

Copy link
Copy Markdown

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 48ed3b98-c7bd-4775-b7d7-104989de7a98

📥 Commits

Reviewing files that changed from the base of the PR and between 2ddeaa6 and 80817bb.

📒 Files selected for processing (10)
  • package.json
  • scripts/ci/lint-workspace.mjs
  • services/agencies/package.json
  • services/alerts/package.json
  • services/ingestion/package.json
  • services/ingestion/src/gtfs-static.service.ts
  • services/routes/package.json
  • services/routes/src/routes.service.ts
  • services/stops/src/stops.service.ts
  • services/vehicles/package.json

📝 Walkthrough

Walkthrough

This PR removes the NestJS backend monolith, replaces it with microservices (agencies, routes, stops, alerts, vehicles, ingestion), adds precomputed ingestion tables and entity fields (route_stops, route_branches, routes.has_stop_times, stops.colocated_group_id), updates services to use those precomputations, and adjusts CI, tests, and documentation accordingly.

Changes

Monolith-to-Microservices Migration

Layer / File(s) Summary
Shared Data Model & Constants
packages/shared/src/constants.ts, packages/shared/src/entities/route.entity.ts, packages/shared/src/entities/stop.entity.ts
STOP_MERGE_RADIUS_DEG added; Route.hasStopTimes added; Stop.colocatedGroupId added with an index.
Ingestion Service with Precomputed Tables
services/ingestion/src/gtfs-static.service.ts
ingestAgency() cleans previous agency data, builds/populates route_stops and route_branches, updates routes.has_stop_times, computes stops.colocated_group_id, switches stop_times ingestion to COPY streaming, and updates batching/indexing/ANALYZE.
Routes Service Using Precomputed Branches
services/routes/src/routes.service.ts
findAll() uses r.has_stop_times for shuttle filtering. findOne() and getShape() select representative branches/shape from route_branches; route-detail cache key bumped.
Stops Service Using Precomputed Colocated Groups
services/stops/src/stops.service.ts, services/stops/package.json, services/stops/src/cache/cache.service.spec.ts
search(), getArrivals(), getNearbyStops(), and getRoutesForStop() now use route_stops and stops.colocated_group_id for grouping and route lookup; runtime merge logic removed. Jest coverage config and test:cov added; a test import path updated.
Backend Monolith Removal
backend/ (entire directory)
Deleted NestJS backend code: modules, services, controllers, entities, configs, Dockerfiles, package.json/tsconfig, and backend test/config artifacts.
CI Workflows & Repo Wiring
.github/workflows/ci.yml, .lintstagedrc.json, .prettierignore, package.json, scripts/ci/*, scripts/ci/typecheck-workspace.mjs, scripts/ci/lint-workspace.mjs
CI/typecheck/lint updated to run root install and per-service checks; backend-scoped installs and gates removed; deploy no longer depends on integration/contract; coverage gate reads services/stops outputs; root lint scripts target workspace projects via new lint-workspace script.
Frontend Route Trunk/Tail Logic
frontend/src/app/routes/[routeId]/page.tsx
Trunk/tail computation changed to exclude short-turn branches and use stopName intersection instead of stopId.
Documentation Updates
README.md, docs/configuration.md, docs/data-model.md, docs/development.md
README and docs updated to describe microservices, precomputed tables and columns, config path semantics, and revised CI/test instructions referencing services/stops and other microservices.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

Poem

🐰 The monolith hopped away, services bloom like spring,

Routes find their branches, stops in cozy grouping.
Ingestion streams the feeds, shared constants take root,
CI trims the hedges—tiny hops toward the fruit.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch streamline-db-operations

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (3)
services/stops/package.json (1)

65-65: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

TypeScript version mismatch across workspace.

The services/stops package uses TypeScript 5.3.0, while the root package.json uses 6.0.2. This version mismatch can lead to inconsistent type-checking behavior and compilation errors, especially when using workspace-shared packages.

All workspaces should use the same TypeScript version to ensure consistent builds and type checking.

📦 Proposed fix to align TypeScript versions
   "devDependencies": {
     "@nestjs/cli": "^10.0.0",
     "@nestjs/schematics": "^10.0.0",
     "@nestjs/testing": "^10.0.0",
     "@types/express": "^4.17.21",
     "@types/jest": "^29.5.0",
     "@types/node": "^20.0.0",
     "jest": "^29.5.0",
     "ts-jest": "^29.1.0",
     "ts-loader": "^9.4.3",
     "ts-node": "^10.9.1",
-    "typescript": "^5.3.0"
+    "typescript": "^6.0.2"
   }
🤖 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 `@services/stops/package.json` at line 65, Update the TypeScript dependency in
the services/stops package.json to match the workspace root version (change the
"typescript" entry from "^5.3.0" to the root's "6.0.2" specifier), ensure it is
listed under devDependencies if appropriate, then reinstall dependencies
(refresh lockfile) so the workspace uses a single TypeScript version for
consistent type-checking across packages.
docs/development.md (1)

127-137: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Remove stale integration / contract gate references.

Line 132–133 and Line 162–163 still document commands/checks that are no longer valid in the current gate runner, which can mislead contributors and branch-protection setup.

Also applies to: 159-165

🤖 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 `@docs/development.md` around lines 127 - 137, Documentation still lists stale
CI commands 'npm run test:integration' and 'npm run test:contract'; remove these
references from the examples and any gate checklist blocks that mention them
(specifically the block containing "npm run test:integration" and "npm run
test:contract" and the second occurrence around the later gate section) so the
docs only list valid gates (e.g., lint, typecheck, format:check, test:unit,
test:a11y, test:performance, test:e2e); update surrounding text if it describes
integration/contract checks to reflect they are no longer part of the gate
runner.
services/stops/src/stops.service.ts (1)

87-106: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Cross-agency route lookups are incorrectly narrowed to one agency.

When agencyKey is omitted, Line 87/547 picks the first row’s agency_id and applies it to the entire stop_id batch. Stops from other agencies then lose their route associations.

Suggested fix
-      const agencyId = stopRows[0].agency_id;
       const routeRows = await this.dataSource.query<
         Array<{
+          agency_id: string;
           group_stop_id: string;
           routeId: string;
           shortName: string | null;
           longName: string | null;
           routeType: number;
         }>
       >(
         `SELECT rs.stop_id AS group_stop_id,
+                rs.agency_id,
                 rs.route_id AS "routeId",
                 rs.short_name AS "shortName",
                 rs.long_name AS "longName",
                 rs.route_type AS "routeType"
          FROM route_stops rs
-         WHERE rs.stop_id = ANY($1) AND rs.agency_id = $2
+         WHERE rs.stop_id = ANY($1)
          ORDER BY rs.stop_id, rs.short_name ASC`,
-        [stopIdList, agencyId],
+        [stopIdList],
       );

Then key routesByStopId by agency_id + stop_id (or include agency_id in stop rows and join in-memory) to avoid collisions across agencies.

Also applies to: 547-566

🤖 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 `@services/stops/src/stops.service.ts` around lines 87 - 106, The current logic
pulls agencyId = stopRows[0].agency_id and uses it to filter routeRows, which
drops routes for stops belonging to other agencies; remove the single-agency
filter and include agency_id in the route query (select rs.agency_id) and in the
in-memory mapping so lookups are per agency+stop. Specifically, update the SQL
call that produces routeRows to not restrict by a single agency (remove the
rs.agency_id = $2 clause and/or stop passing a single agencyId), include
rs.agency_id in the selected columns, and then build routesByStopId keyed by a
compound key (e.g., `${agency_id}:${group_stop_id}`) using the existing
variables routeRows, stopRows and stopIdList so each stop's routes are looked up
by agency+stop id rather than a single agencyId derived from stopRows[0].
🧹 Nitpick comments (3)
packages/shared/src/entities/route.entity.ts (1)

45-47: 💤 Low value

Consider whether indexing a boolean column provides value.

Indexing boolean fields is typically not beneficial when the column has low cardinality (only two distinct values). If most routes have hasStopTimes = true, a table scan may be more efficient than an index lookup. However, if this column is frequently used in compound queries or the distribution is more balanced, the index may provide value.

Verify query patterns and cardinality before deciding to keep or remove the @Index() decorator.

🤖 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/shared/src/entities/route.entity.ts` around lines 45 - 47, The
boolean column Route.hasStopTimes is currently decorated with `@Index`(), which
may be ineffective due to low cardinality; inspect query patterns that reference
hasStopTimes (search for uses of hasStopTimes in repository queries and
repository/QueryBuilder code) and check the actual value distribution (COUNTs or
histogram via SQL/EXPLAIN) — if queries are simple filters with highly skewed
values, remove the `@Index`() on the hasStopTimes property to avoid unnecessary
index maintenance, otherwise replace it with a more targeted index (e.g., a
partial index for hasStopTimes = true) or keep it if compound queries benefit
from it; update the decorator on the hasStopTimes property accordingly (remove
`@Index`() or change indexing strategy) and run migrations/tests to apply the
change.
packages/shared/src/constants.ts (1)

13-14: ⚖️ Poor tradeoff

Apply latitude-aware distance calculation or add documentation about the equator assumption.

The stop merge logic uses Math.sqrt(dLat * dLat + dLon * dLon) to compare geographic distance in degree space, but treats latitude and longitude equally. This is only accurate at the equator; a degree of longitude represents ~111 km at 0° but only ~79 km at 45° latitude. The current approximation will incorrectly merge or fail to merge stops depending on their latitude.

Either apply a latitude-weighted distance calculation (sqrt(dLat² + (dLon × cos(lat))²)), use PostGIS with meter-based radius, or document that this approximation is acceptable for your geographic scope.

🤖 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/shared/src/constants.ts` around lines 13 - 14, The
STOP_MERGE_RADIUS_DEG constant and any code using plain Euclidean degree
distance (e.g., sqrt(dLat*dLat + dLon*dLon)) assume degrees are equivalent for
lat/lon; update the merge logic to account for latitude by scaling longitude
differences by cos(latitude) or switch to meter-based checks: compute distance
as sqrt(dLat^2 + (dLon * cos(meanLat))^2) before comparing to
STOP_MERGE_RADIUS_DEG (or replace STOP_MERGE_RADIUS_DEG with a meter radius and
convert differences using a proper lat/lon→meters formula or PostGIS). Ensure
references to STOP_MERGE_RADIUS_DEG and the merge function that computes
dLat/dLon are updated consistently and add a short doc comment explaining the
chosen approach and assumptions.
services/ingestion/src/gtfs-static.service.ts (1)

133-153: ⚡ Quick win

Don’t swallow index-build errors silently.

These .catch(() => {}) blocks hide real failures (e.g., missing extensions/permissions) and make performance regressions hard to diagnose. Log warnings at minimum.

Suggested adjustment
-      await this.dataSource
-        .query(
-          `CREATE INDEX IF NOT EXISTS idx_stops_stop_name_trgm ON stops USING gin (stop_name gin_trgm_ops)`,
-        )
-        .catch(() => {});
+      await this.dataSource
+        .query(
+          `CREATE INDEX IF NOT EXISTS idx_stops_stop_name_trgm ON stops USING gin (stop_name gin_trgm_ops)`,
+        )
+        .catch((err: unknown) => {
+          this.logger.warn(`Failed to create idx_stops_stop_name_trgm: ${(err as Error).message}`);
+        });
🤖 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 `@services/ingestion/src/gtfs-static.service.ts` around lines 133 - 153, The
current dataSource.query calls that create GIN and stop_times indexes (e.g.,
idx_stops_stop_name_trgm, idx_stops_stop_code_trgm,
idx_stop_times_agency_stop_dept, idx_stop_times_agency_trip_seq) swallow errors
with .catch(() => {}); change these catch handlers to surface failures by
logging a warning or error via this.logger (include the index name and the
caught error) and rethrow or handle appropriately so missing
extensions/permissions are visible; update the query invocations in
gtfs-static.service.ts where create index queries are executed to use
.catch((err) => this.logger.warn(`Failed to create <index_name>: ${err}`)) or
similar.
🤖 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 `@package.json`:
- Line 27: The root "test:unit" script only runs coverage for services/stops and
frontend but other microservices (agencies, alerts, ingestion, routes, vehicles)
only define "test" (no coverage), so either add a "test:cov": "jest --coverage"
script to each microservice package.json (agencies, alerts, ingestion, routes,
vehicles) and include those service prefixes in the root "test:unit" script, or
explicitly document that coverage is only collected for services/stops and
frontend; update the root "test:unit" script (symbol: "test:unit") to call npm
run test:cov --prefix for each service that should report coverage and ensure
the check script (scripts/ci/check-unit-coverage.mjs) expectations match the
chosen set.

In `@services/ingestion/src/gtfs-static.service.ts`:
- Around line 585-588: The loop currently assigns a colocated_group_id for every
stop (in the block using consumed and groupAssignments), but singletons should
get null; modify the logic in the grouping code that iterates "for (const sid of
group)" so that if group.length === 1 you set groupAssignments.set(sid, null)
(and still consumed.add(sid)), otherwise set groupAssignments.set(sid, group[0])
for multi-stop groups; update the corresponding second occurrence (around the
other loop at the 602-603 area) similarly; when building the SQL parameter
arrays, use unknown[] and pass null for singleton gids so the SQL CASE can
handle null assignment.

In `@services/stops/package.json`:
- Around line 39-42: Update the package.json Jest configuration's
collectCoverageFrom array so it includes all meaningful TypeScript source files
in this service (not just the three listed). Edit the "collectCoverageFrom"
entry in package.json to add the service files such as stops.service.ts,
stops.controller.ts, health.controller.ts, stops.types.ts,
cache/cache.constants.ts, cache/cache.module.ts, plus any other .ts sources (or
use a wildcard like "src/**/*.ts" or "!(**/*.spec).ts" if project layout
permits) so coverage is collected across the full service codebase.

---

Outside diff comments:
In `@docs/development.md`:
- Around line 127-137: Documentation still lists stale CI commands 'npm run
test:integration' and 'npm run test:contract'; remove these references from the
examples and any gate checklist blocks that mention them (specifically the block
containing "npm run test:integration" and "npm run test:contract" and the second
occurrence around the later gate section) so the docs only list valid gates
(e.g., lint, typecheck, format:check, test:unit, test:a11y, test:performance,
test:e2e); update surrounding text if it describes integration/contract checks
to reflect they are no longer part of the gate runner.

In `@services/stops/package.json`:
- Line 65: Update the TypeScript dependency in the services/stops package.json
to match the workspace root version (change the "typescript" entry from "^5.3.0"
to the root's "6.0.2" specifier), ensure it is listed under devDependencies if
appropriate, then reinstall dependencies (refresh lockfile) so the workspace
uses a single TypeScript version for consistent type-checking across packages.

In `@services/stops/src/stops.service.ts`:
- Around line 87-106: The current logic pulls agencyId = stopRows[0].agency_id
and uses it to filter routeRows, which drops routes for stops belonging to other
agencies; remove the single-agency filter and include agency_id in the route
query (select rs.agency_id) and in the in-memory mapping so lookups are per
agency+stop. Specifically, update the SQL call that produces routeRows to not
restrict by a single agency (remove the rs.agency_id = $2 clause and/or stop
passing a single agencyId), include rs.agency_id in the selected columns, and
then build routesByStopId keyed by a compound key (e.g.,
`${agency_id}:${group_stop_id}`) using the existing variables routeRows,
stopRows and stopIdList so each stop's routes are looked up by agency+stop id
rather than a single agencyId derived from stopRows[0].

---

Nitpick comments:
In `@packages/shared/src/constants.ts`:
- Around line 13-14: The STOP_MERGE_RADIUS_DEG constant and any code using plain
Euclidean degree distance (e.g., sqrt(dLat*dLat + dLon*dLon)) assume degrees are
equivalent for lat/lon; update the merge logic to account for latitude by
scaling longitude differences by cos(latitude) or switch to meter-based checks:
compute distance as sqrt(dLat^2 + (dLon * cos(meanLat))^2) before comparing to
STOP_MERGE_RADIUS_DEG (or replace STOP_MERGE_RADIUS_DEG with a meter radius and
convert differences using a proper lat/lon→meters formula or PostGIS). Ensure
references to STOP_MERGE_RADIUS_DEG and the merge function that computes
dLat/dLon are updated consistently and add a short doc comment explaining the
chosen approach and assumptions.

In `@packages/shared/src/entities/route.entity.ts`:
- Around line 45-47: The boolean column Route.hasStopTimes is currently
decorated with `@Index`(), which may be ineffective due to low cardinality;
inspect query patterns that reference hasStopTimes (search for uses of
hasStopTimes in repository queries and repository/QueryBuilder code) and check
the actual value distribution (COUNTs or histogram via SQL/EXPLAIN) — if queries
are simple filters with highly skewed values, remove the `@Index`() on the
hasStopTimes property to avoid unnecessary index maintenance, otherwise replace
it with a more targeted index (e.g., a partial index for hasStopTimes = true) or
keep it if compound queries benefit from it; update the decorator on the
hasStopTimes property accordingly (remove `@Index`() or change indexing strategy)
and run migrations/tests to apply the change.

In `@services/ingestion/src/gtfs-static.service.ts`:
- Around line 133-153: The current dataSource.query calls that create GIN and
stop_times indexes (e.g., idx_stops_stop_name_trgm, idx_stops_stop_code_trgm,
idx_stop_times_agency_stop_dept, idx_stop_times_agency_trip_seq) swallow errors
with .catch(() => {}); change these catch handlers to surface failures by
logging a warning or error via this.logger (include the index name and the
caught error) and rethrow or handle appropriately so missing
extensions/permissions are visible; update the query invocations in
gtfs-static.service.ts where create index queries are executed to use
.catch((err) => this.logger.warn(`Failed to create <index_name>: ${err}`)) or
similar.
🪄 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

Run ID: 0d47ffa3-eaf0-4a33-858b-30433234f789

📥 Commits

Reviewing files that changed from the base of the PR and between ed34b29 and 9f9993e.

⛔ Files ignored due to path filters (1)
  • backend/package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (75)
  • .github/workflows/ci.yml
  • .lintstagedrc.json
  • .prettierignore
  • README.md
  • backend/.eslintrc.json
  • backend/Dockerfile
  • backend/Dockerfile.worker
  • backend/core
  • backend/nest-cli.json
  • backend/nest-cli.worker.json
  • backend/package.json
  • backend/src/app.module.ts
  • backend/src/common/constants.ts
  • backend/src/common/filters/http-exception.filter.ts
  • backend/src/config/configuration.ts
  • backend/src/main.ts
  • backend/src/modules/agencies/agencies.controller.ts
  • backend/src/modules/agencies/agencies.module.ts
  • backend/src/modules/agencies/agencies.service.ts
  • backend/src/modules/agencies/entities/agency.entity.ts
  • backend/src/modules/alerts/alerts.controller.ts
  • backend/src/modules/alerts/alerts.module.ts
  • backend/src/modules/cache/cache.constants.ts
  • backend/src/modules/cache/cache.module.ts
  • backend/src/modules/cache/cache.service.ts
  • backend/src/modules/health/health.controller.ts
  • backend/src/modules/health/health.module.ts
  • backend/src/modules/ingestion/entities/service-calendar.entity.ts
  • backend/src/modules/ingestion/entities/shape.entity.ts
  • backend/src/modules/ingestion/gtfs-realtime.service.ts
  • backend/src/modules/ingestion/gtfs-static.service.ts
  • backend/src/modules/ingestion/ingestion.module.ts
  • backend/src/modules/ingestion/ingestion.scheduler.ts
  • backend/src/modules/routes/entities/route.entity.ts
  • backend/src/modules/routes/routes.controller.ts
  • backend/src/modules/routes/routes.module.ts
  • backend/src/modules/routes/routes.service.ts
  • backend/src/modules/stops/entities/stop-time.entity.ts
  • backend/src/modules/stops/entities/stop.entity.ts
  • backend/src/modules/stops/mergeColocatedStops.ts
  • backend/src/modules/stops/reconcileAddedTrips.ts
  • backend/src/modules/stops/stops.controller.ts
  • backend/src/modules/stops/stops.module.ts
  • backend/src/modules/stops/stops.service.ts
  • backend/src/modules/stops/stops.types.ts
  • backend/src/modules/trips/entities/trip.entity.ts
  • backend/src/modules/trips/trips.controller.ts
  • backend/src/modules/trips/trips.module.ts
  • backend/src/modules/trips/trips.service.ts
  • backend/src/modules/vehicles/vehicles.controller.ts
  • backend/src/modules/vehicles/vehicles.module.ts
  • backend/src/worker.module.ts
  • backend/src/worker.ts
  • backend/tests/contract/api-contract.spec.ts
  • backend/tests/integration/ci-gates.integration.spec.ts
  • backend/tests/jest-contract.json
  • backend/tests/jest-integration.json
  • backend/tsconfig.json
  • docs/configuration.md
  • docs/data-model.md
  • docs/development.md
  • package.json
  • packages/shared/src/constants.ts
  • packages/shared/src/entities/route.entity.ts
  • packages/shared/src/entities/stop.entity.ts
  • scripts/ci/check-unit-coverage.mjs
  • scripts/ci/run-gate.mjs
  • scripts/ci/typecheck-workspace.mjs
  • services/ingestion/src/gtfs-static.service.ts
  • services/routes/src/routes.service.ts
  • services/stops/package.json
  • services/stops/src/cache/cache.service.spec.ts
  • services/stops/src/merge-colocated-stops.spec.ts
  • services/stops/src/reconcile-added-trips.spec.ts
  • services/stops/src/stops.service.ts
💤 Files with no reviewable changes (56)
  • backend/src/modules/trips/trips.module.ts
  • backend/src/modules/health/health.module.ts
  • backend/.eslintrc.json
  • backend/src/modules/routes/routes.module.ts
  • backend/tsconfig.json
  • backend/src/modules/alerts/alerts.module.ts
  • backend/Dockerfile.worker
  • backend/src/modules/cache/cache.constants.ts
  • backend/Dockerfile
  • backend/nest-cli.worker.json
  • backend/src/modules/ingestion/entities/shape.entity.ts
  • backend/src/modules/agencies/agencies.module.ts
  • backend/src/modules/trips/trips.controller.ts
  • backend/nest-cli.json
  • backend/src/modules/trips/entities/trip.entity.ts
  • backend/src/modules/agencies/agencies.controller.ts
  • backend/src/modules/vehicles/vehicles.module.ts
  • .lintstagedrc.json
  • backend/src/modules/health/health.controller.ts
  • backend/src/config/configuration.ts
  • backend/src/modules/cache/cache.service.ts
  • backend/src/modules/stops/entities/stop-time.entity.ts
  • backend/tests/integration/ci-gates.integration.spec.ts
  • backend/src/modules/stops/reconcileAddedTrips.ts
  • backend/src/worker.module.ts
  • backend/src/modules/stops/entities/stop.entity.ts
  • backend/src/modules/ingestion/ingestion.module.ts
  • backend/tests/contract/api-contract.spec.ts
  • backend/src/modules/vehicles/vehicles.controller.ts
  • .prettierignore
  • backend/src/modules/agencies/entities/agency.entity.ts
  • backend/src/modules/stops/stops.module.ts
  • backend/src/modules/routes/entities/route.entity.ts
  • backend/tests/jest-contract.json
  • backend/src/modules/stops/stops.types.ts
  • backend/tests/jest-integration.json
  • backend/package.json
  • backend/src/modules/routes/routes.controller.ts
  • backend/src/common/constants.ts
  • backend/src/modules/ingestion/gtfs-realtime.service.ts
  • backend/src/common/filters/http-exception.filter.ts
  • backend/src/modules/ingestion/gtfs-static.service.ts
  • backend/src/modules/alerts/alerts.controller.ts
  • backend/src/modules/stops/mergeColocatedStops.ts
  • .github/workflows/ci.yml
  • backend/src/modules/trips/trips.service.ts
  • backend/src/modules/stops/stops.controller.ts
  • backend/src/main.ts
  • backend/src/modules/stops/stops.service.ts
  • backend/src/modules/ingestion/ingestion.scheduler.ts
  • backend/src/modules/ingestion/entities/service-calendar.entity.ts
  • backend/src/app.module.ts
  • backend/src/modules/agencies/agencies.service.ts
  • backend/src/worker.ts
  • backend/src/modules/cache/cache.module.ts
  • backend/src/modules/routes/routes.service.ts

Comment thread package.json Outdated
Comment thread services/ingestion/src/gtfs-static.service.ts
Comment thread services/stops/package.json Outdated

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 7

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
services/routes/src/routes.service.ts (1)

117-135: ⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift

Keep a fallback until route_branches is populated.

This hard-switch makes findOne depend on derived rows that do not exist for already-ingested agencies until the new pipeline runs. On deploy, valid routes can start 404ing even though trips and stop_times are still present. Please keep the old query as a fallback or ship a coordinated backfill before enabling this path.

🤖 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 `@services/routes/src/routes.service.ts` around lines 117 - 135, The new
blocking behavior in routes.service.ts causes findOne to 404 when route_branches
are not yet populated; change the branchReps resolution so that after the
routeRepo.query(...) for route_branches (the branchReps variable) if
branchReps.length === 0 you do not throw NotFoundException but instead fall back
to the legacy query logic that derives representative trips from
trips/stop_times (i.e. run the previous SQL or helper that used trips/stop_times
to build direction_id/trip_headsign/shape_id/stop_count and assign to
branchReps) and only throw NotFoundException if both branchReps and the
legacy-derived set are empty; keep references to routeRepo.query and branchReps
when implementing the fallback.
♻️ Duplicate comments (1)
package.json (1)

27-27: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Confirm coverage strategy for all microservices.

The root test:unit script collects coverage only from services/stops and frontend. The workspace includes five additional microservices (agencies, alerts, vehicles, routes, ingestion) that are not included in coverage collection. Verify whether these services:

  1. Have test suites that should be included in coverage tracking
  2. Intentionally defer coverage collection as part of the incremental migration from the monolith
  3. Require test:cov scripts to be added to their package.json files

If coverage should be tracked across all services, add "test:cov": "jest --coverage" to each microservice's package.json and update this root script accordingly.

#!/bin/bash
# Description: Check which microservices have test files and test scripts

echo "=== Checking for test files in each microservice ==="
for service in agencies alerts vehicles routes ingestion; do
  echo "--- services/$service ---"
  spec_count=$(fd -e ts '\.spec\.ts$' services/$service/src 2>/dev/null | wc -l)
  echo "Test files found: $spec_count"
  echo "Test script: $(jq -r '.scripts.test // "none"' services/$service/package.json 2>/dev/null)"
  echo "Coverage script: $(jq -r '.scripts["test:cov"] // "none"' services/$service/package.json 2>/dev/null)"
  echo ""
done
🤖 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 `@package.json` at line 27, The root "test:unit" script currently only runs
coverage for "services/stops" and "frontend", leaving microservices agencies,
alerts, vehicles, routes, and ingestion out of coverage; confirm whether each
microservice should include coverage (check for existing tests and migration
intent) and if so add a "test:cov": "jest --coverage" script to each
microservice package.json (or the appropriate test runner command) and update
the root "test:unit" script to run npm run test:cov --prefix for each of those
services (and keep existing stops/frontend entries) so coverage is collected
across all services; reference the root package.json "test:unit" entry and each
microservice package.json "scripts.test:cov" when making changes.
🧹 Nitpick comments (1)
package.json (1)

37-37: 💤 Low value

Consider removing orphaned test scripts.

The test:all script no longer invokes test:integration (line 28) and test:contract (line 29), which aligns with the removal of the backend monolith. However, these script definitions still exist in the file but are now unreachable unless manually invoked. Consider removing these orphaned scripts if they're no longer needed, or document their retention if they're kept for future use or manual testing purposes.

🤖 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 `@package.json` at line 37, The package.json contains orphaned npm scripts
"test:integration" and "test:contract" that are no longer invoked by "test:all";
either remove those script entries entirely from package.json (delete the
"test:integration" and "test:contract" keys) or explicitly document why you’re
keeping them (e.g., add a note to the repo README) or reintroduce them into
"test:all" if they should run automatically—update the "test:all" script or the
documentation accordingly so the state of "test:integration" and "test:contract"
is intentional and clear.
🤖 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 `@package.json`:
- Around line 23-24: Update the root "lint" and "lint:fix" npm scripts so they
run linting across the whole workspace (all microservices + frontend) instead of
only the frontend; add or reuse a workspace-level runner similar to
"typecheck-workspace.mjs" (e.g., create "lint-workspace.mjs" that iterates
packages and invokes each package's lint script) and wire "lint" -> "node
./scripts/lint-workspace.mjs" and "lint:fix" -> "node
./scripts/lint-workspace.mjs --fix" (or implement equivalent
npm-run-all/concurrent invocation) so the workspace lint covers agencies,
alerts, vehicles, routes, stops, ingestion and frontend consistently with
typecheck.

In `@packages/shared/src/entities/route.entity.ts`:
- Around line 45-46: The new Route entity field hasStopTimes is false for
existing rows which hides shuttle routes; add a DB migration/backfill to set
has_stop_times = true for any route that has related stop_times (or relevant
criteria) before switching readers, or implement a temporary fallback in the
reader to check the previous stop-times existence logic when Route.hasStopTimes
is false; target symbols: the Route entity (hasStopTimes) and the routes table
update, or the reader method that queries route visibility to prefer a live
existence check if hasStopTimes is false.

In `@services/ingestion/src/gtfs-static.service.ts`:
- Around line 636-649: The UPDATE builds a massive CASE using groupAssignments
and can exceed PostgreSQL's parameter limit; change the logic in the block that
constructs params/cases and calls this.dataSource.query so it processes
groupAssignments in smaller chunks (e.g., compute a maxParams constant like
32767 and derive maxPairsPerBatch = floor((maxParams - 1) / 2) because each
assignment uses two params plus one agencyId), then loop over groupAssignments
in batches building params and cases for each batch and execute the UPDATE per
batch (use the same CASE/ELSE NULL END pattern and pass agencyId as the last
param for each batch). Ensure you reference the same symbols (groupAssignments,
params, cases, agencyId, this.dataSource.query, and the stops.colocated_group_id
update) so the change only adds batching logic without altering the SQL shape.
- Around line 98-107: The destructive DELETEs (calls to this.dataSource.query
for stop_times, trips, stops, routes, shapes, service_calendars, route_stops,
route_branches) must be performed inside a single DB transaction so rollback is
possible on failure; change the ingestion logic in gtfs-static.service.ts (the
method performing the re-ingest) to obtain a QueryRunner, startTransaction(),
execute all DELETEs and the subsequent inserts/derived-table rebuilds via that
QueryRunner (not this.dataSource.query), commitTransaction() only after all
steps succeed, and only then update last_ingested_at; ensure
rollbackTransaction() is called on errors and the QueryRunner is released in a
finally block.
- Around line 178-233: The CREATE TABLE and CREATE INDEX DDL for route_stops and
route_branches (and their indexes idx_route_stops_agency_stop,
idx_route_branches_agency_route) must be removed from the ingestion code in
gtfs-static.service.ts (the dataSource.query calls that create those tables) and
moved into a deployment-time migration/bootstrap step that runs before the new
readers are released; keep the population logic (DELETE/INSERT) that uses
route_stops and route_branches in the ingestion flow, but ensure a migration or
bootstrap script invokes the same CREATE TABLE IF NOT EXISTS and CREATE INDEX IF
NOT EXISTS statements (using the same schema: columns, constraints, and UUID
defaults) so request handlers never see "relation does not exist" after deploy.

In `@services/stops/src/stops.service.ts`:
- Around line 616-654: The grouping must be agency-aware: when building
groupMap, find the matching row for each StopResponse using both stop_id and
agency_id (e.g., rows.find(r => r.stop_id === s.stopId && r.agency_id ===
s.agencyId)) and use its colocated_group_id plus agency_id to form the key like
`${agency_id}:${gid}`; carry that found row (or its agency_id and gid) forward
instead of calling rows.find again so group keys and lookups in groupMap are
consistent and you don't merge stops across agencies (update the code around
groupMap creation, the gid compute and any subsequent uses in the loop that
reference rows/find). Ensure merged.routes, mergedData and nextArrivalByStopId
logic remain unchanged but operate on the agency-scoped groups.
- Around line 131-154: Grouping uses gid derived from colocated_group_id and raw
stopId which is only unique per agency, so groupMap/gid (in the loop setting gid
from stopRows[i].colocated_group_id ?? rawData[i].stopId) can merge stops across
agencies; change the key construction to include agency (e.g.,
`${agencyKey}:${gid}` or combine stopRows[i].agencyKey with
colocated_group_id/rawData[i].stopId) when creating gid and when checking
groupMap so grouping respects agency boundaries; update any references in that
loop and subsequent grouping logic (groupMap, gid, rawData, stopRows,
StopResponse) accordingly.

---

Outside diff comments:
In `@services/routes/src/routes.service.ts`:
- Around line 117-135: The new blocking behavior in routes.service.ts causes
findOne to 404 when route_branches are not yet populated; change the branchReps
resolution so that after the routeRepo.query(...) for route_branches (the
branchReps variable) if branchReps.length === 0 you do not throw
NotFoundException but instead fall back to the legacy query logic that derives
representative trips from trips/stop_times (i.e. run the previous SQL or helper
that used trips/stop_times to build
direction_id/trip_headsign/shape_id/stop_count and assign to branchReps) and
only throw NotFoundException if both branchReps and the legacy-derived set are
empty; keep references to routeRepo.query and branchReps when implementing the
fallback.

---

Duplicate comments:
In `@package.json`:
- Line 27: The root "test:unit" script currently only runs coverage for
"services/stops" and "frontend", leaving microservices agencies, alerts,
vehicles, routes, and ingestion out of coverage; confirm whether each
microservice should include coverage (check for existing tests and migration
intent) and if so add a "test:cov": "jest --coverage" script to each
microservice package.json (or the appropriate test runner command) and update
the root "test:unit" script to run npm run test:cov --prefix for each of those
services (and keep existing stops/frontend entries) so coverage is collected
across all services; reference the root package.json "test:unit" entry and each
microservice package.json "scripts.test:cov" when making changes.

---

Nitpick comments:
In `@package.json`:
- Line 37: The package.json contains orphaned npm scripts "test:integration" and
"test:contract" that are no longer invoked by "test:all"; either remove those
script entries entirely from package.json (delete the "test:integration" and
"test:contract" keys) or explicitly document why you’re keeping them (e.g., add
a note to the repo README) or reintroduce them into "test:all" if they should
run automatically—update the "test:all" script or the documentation accordingly
so the state of "test:integration" and "test:contract" is intentional and clear.
🪄 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

Run ID: f5aa6a76-9301-4f71-b38f-720e96f7bd92

📥 Commits

Reviewing files that changed from the base of the PR and between 9f9993e and 040306f.

📒 Files selected for processing (8)
  • docs/development.md
  • package.json
  • packages/shared/src/constants.ts
  • packages/shared/src/entities/route.entity.ts
  • services/ingestion/src/gtfs-static.service.ts
  • services/routes/src/routes.service.ts
  • services/stops/package.json
  • services/stops/src/stops.service.ts
✅ Files skipped from review due to trivial changes (1)
  • packages/shared/src/constants.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • docs/development.md

Comment thread package.json Outdated
Comment on lines +45 to +46
@Column({ name: 'has_stop_times', type: 'boolean', default: false })
hasStopTimes!: boolean;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift

Backfill has_stop_times before switching readers to it.

Existing routes rows will come up false on deploy, so shuttle routes disappear from the list endpoint until every agency is re-ingested. Please ship a data backfill/migration for this column or keep the old stop-times existence check as a temporary fallback.

🤖 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/shared/src/entities/route.entity.ts` around lines 45 - 46, The new
Route entity field hasStopTimes is false for existing rows which hides shuttle
routes; add a DB migration/backfill to set has_stop_times = true for any route
that has related stop_times (or relevant criteria) before switching readers, or
implement a temporary fallback in the reader to check the previous stop-times
existence logic when Route.hasStopTimes is false; target symbols: the Route
entity (hasStopTimes) and the routes table update, or the reader method that
queries route visibility to prefer a live existence check if hasStopTimes is
false.

Comment on lines +178 to +233
// Precompute route_stops — maps every stop to its serving routes
this.logger.debug(`Creating route_stops table...`);
await this.dataSource.query(`
CREATE TABLE IF NOT EXISTS route_stops (
id UUID PRIMARY KEY DEFAULT gen_random_uuid(),
agency_id UUID NOT NULL REFERENCES agencies("agencyId") ON DELETE CASCADE,
stop_id VARCHAR(100) NOT NULL,
route_id VARCHAR(100) NOT NULL,
short_name VARCHAR(50),
long_name TEXT,
route_type SMALLINT NOT NULL,
UNIQUE (agency_id, stop_id, route_id)
)
`);
await this.dataSource.query(
`CREATE INDEX IF NOT EXISTS idx_route_stops_agency_stop ON route_stops (agency_id, stop_id)`,
);
this.logger.debug(`Populating route_stops from stop_times...`);
await this.dataSource.query(`DELETE FROM route_stops WHERE agency_id = $1`, [agencyId]);
await this.dataSource.query(
`
INSERT INTO route_stops (agency_id, stop_id, route_id, short_name, long_name, route_type)
SELECT DISTINCT st.agency_id, st.stop_id, r.route_id, r.short_name, r.long_name, r.route_type
FROM stop_times st
JOIN trips t ON t.trip_id = st.trip_id AND t.agency_id = st.agency_id
JOIN routes r ON r.route_id = t.route_id AND r.agency_id = t.agency_id
WHERE st.agency_id = $1
UNION
SELECT DISTINCT st.agency_id, s.parent_station_id, r.route_id, r.short_name, r.long_name, r.route_type
FROM stop_times st
JOIN stops s ON s.stop_id = st.stop_id AND s.agency_id = st.agency_id
JOIN trips t ON t.trip_id = st.trip_id AND t.agency_id = st.agency_id
JOIN routes r ON r.route_id = t.route_id AND r.agency_id = t.agency_id
WHERE st.agency_id = $1
AND s.parent_station_id IS NOT NULL AND s.parent_station_id != ''
`,
[agencyId],
);

// Precompute route_branches — representative trip per (route, direction, headsign)
this.logger.debug(`Creating route_branches table...`);
await this.dataSource.query(`
CREATE TABLE IF NOT EXISTS route_branches (
id UUID PRIMARY KEY DEFAULT gen_random_uuid(),
agency_id UUID NOT NULL REFERENCES agencies("agencyId") ON DELETE CASCADE,
route_id VARCHAR(100) NOT NULL,
direction_id SMALLINT,
trip_headsign TEXT,
trip_id VARCHAR(100) NOT NULL,
shape_id VARCHAR(100),
stop_count INTEGER NOT NULL
)
`);
await this.dataSource.query(
`CREATE INDEX IF NOT EXISTS idx_route_branches_agency_route ON route_branches (agency_id, route_id)`,
);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical | 🏗️ Heavy lift

Don't introduce read-path tables lazily from ingestion.

route_stops and route_branches are now queried by request handlers, but they are only created here during a successful ingest. Right after deploy, those reads can fail with relation does not exist until ingestion happens at least once. Move this DDL into a migration/bootstrap step that runs before the new readers are released.

🤖 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 `@services/ingestion/src/gtfs-static.service.ts` around lines 178 - 233, The
CREATE TABLE and CREATE INDEX DDL for route_stops and route_branches (and their
indexes idx_route_stops_agency_stop, idx_route_branches_agency_route) must be
removed from the ingestion code in gtfs-static.service.ts (the dataSource.query
calls that create those tables) and moved into a deployment-time
migration/bootstrap step that runs before the new readers are released; keep the
population logic (DELETE/INSERT) that uses route_stops and route_branches in the
ingestion flow, but ensure a migration or bootstrap script invokes the same
CREATE TABLE IF NOT EXISTS and CREATE INDEX IF NOT EXISTS statements (using the
same schema: columns, constraints, and UUID defaults) so request handlers never
see "relation does not exist" after deploy.

Comment thread services/ingestion/src/gtfs-static.service.ts Outdated
Comment thread services/stops/src/stops.service.ts
Comment thread services/stops/src/stops.service.ts

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (1)
frontend/src/app/routes/[routeId]/page.tsx (1)

94-106: 💤 Low value

Optional: replace sort with single-pass max for longestBranch.

[...trunkBranches].sort((a, b) => b.stops.length - a.stops.length)[0] allocates a copy and runs O(n log n) just to pick the longest branch — longestStopCount is already known, so a single-pass reduce is sufficient and a touch clearer. The reduce-without-initial-value on trunkNameSets is safe today because trunkBranches.length >= 2 is enforced (outer if (canonical.length >= 2) plus fallback to canonical), but passing an explicit seed makes the invariant local to this block.

♻️ Proposed tweak
     const longestStopCount = Math.max(...canonical.map((b) => b.stops.length));
     const fullBranches = canonical.filter((b) => b.stops.length >= longestStopCount * 0.75);
     const trunkBranches = fullBranches.length >= 2 ? fullBranches : canonical;
-    const trunkNameSets = trunkBranches.map((b) => new Set(b.stops.map((s) => s.stopName)));
-    const trunkNames = trunkNameSets.reduce(
-      (acc, set) => new Set([...acc].filter((name) => set.has(name))),
-    );
-    const longestBranch = [...trunkBranches].sort((a, b) => b.stops.length - a.stops.length)[0];
+    const longestBranch = trunkBranches.reduce((a, b) =>
+      b.stops.length > a.stops.length ? b : a,
+    );
+    const trunkNames = trunkBranches.reduce<Set<string>>(
+      (acc, b) => new Set(b.stops.map((s) => s.stopName).filter((name) => acc.has(name))),
+      new Set(longestBranch.stops.map((s) => s.stopName)),
+    );
     sharedStops = longestBranch.stops.filter((s) => trunkNames.has(s.stopName));
🤖 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 `@frontend/src/app/routes/`[routeId]/page.tsx around lines 94 - 106, Replace
the O(n log n) sort used to pick longestBranch with a single-pass selection
(e.g., use Array.prototype.reduce over trunkBranches to return the branch with
the largest stops.length), and make the trunkNames reduce call safer by
supplying an explicit initial accumulator (start with the first trunkNameSets
value or an empty Set) so the operation doesn't rely on external invariants;
update references to longestBranch, trunkBranches, trunkNameSets, and
longestStopCount accordingly.
🤖 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 `@services/ingestion/src/gtfs-static.service.ts`:
- Around line 100-109: The DELETEs for route_stops and route_branches in
ingestAgency run before their CREATE TABLE IF NOT EXISTS DDL, causing failures
on a fresh DB; either (preferred) move the CREATE TABLE IF NOT EXISTS (and
relevant index creation) for route_stops and route_branches into a TypeORM
migration/bootstrap that runs before the service starts, or (minimal stopgap)
hoist the CREATE TABLE IF NOT EXISTS blocks for route_stops and route_branches
to execute before the DELETE statements in ingestAgency so the DELETEs never
target missing relations.

---

Nitpick comments:
In `@frontend/src/app/routes/`[routeId]/page.tsx:
- Around line 94-106: Replace the O(n log n) sort used to pick longestBranch
with a single-pass selection (e.g., use Array.prototype.reduce over
trunkBranches to return the branch with the largest stops.length), and make the
trunkNames reduce call safer by supplying an explicit initial accumulator (start
with the first trunkNameSets value or an empty Set) so the operation doesn't
rely on external invariants; update references to longestBranch, trunkBranches,
trunkNameSets, and longestStopCount accordingly.
🪄 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

Run ID: 5ba59516-f12c-4951-a0a7-040812c65bbd

📥 Commits

Reviewing files that changed from the base of the PR and between 040306f and 2ddeaa6.

📒 Files selected for processing (2)
  • frontend/src/app/routes/[routeId]/page.tsx
  • services/ingestion/src/gtfs-static.service.ts

Comment on lines +100 to +109
// Clean up stale data before re-inserting (idempotent re-ingestion)
this.logger.debug(`Deleting existing data for agency ${agencyId}...`);
await this.dataSource.query(`DELETE FROM stop_times WHERE agency_id = $1`, [agencyId]);
await this.dataSource.query(`DELETE FROM trips WHERE agency_id = $1`, [agencyId]);
await this.dataSource.query(`DELETE FROM stops WHERE agency_id = $1`, [agencyId]);
await this.dataSource.query(`DELETE FROM routes WHERE agency_id = $1`, [agencyId]);
await this.dataSource.query(`DELETE FROM shapes WHERE agency_id = $1`, [agencyId]);
await this.dataSource.query(`DELETE FROM service_calendars WHERE agency_id = $1`, [agencyId]);
await this.dataSource.query(`DELETE FROM route_stops WHERE agency_id = $1`, [agencyId]);
await this.dataSource.query(`DELETE FROM route_branches WHERE agency_id = $1`, [agencyId]);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical | 🏗️ Heavy lift

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Look for any migration / SQL bootstrap file that creates route_stops or route_branches.
fd -t f -e ts -e js -e sql | xargs rg -n -P '(CREATE\s+TABLE.*?\b(route_stops|route_branches)\b|new\s+Table\(\s*\{\s*name:\s*["'\'']?(route_stops|route_branches))' -S || echo "No migration creating route_stops/route_branches found"

# Also check TypeORM datasource for entities/synchronize behavior on these tables.
rg -n -P --type=ts -C3 '\b(route_stops|route_branches)\b' -g '!**/gtfs-static.service.ts'

Repository: rsun19/transit-tracker

Length of output: 4659


First ingest on a fresh DB will fail: DELETE precedes CREATE for route_stops / route_branches.

Lines 108-109 issue DELETE FROM route_stops and DELETE FROM route_branches before any CREATE TABLE IF NOT EXISTS is reached (those statements live at lines 183 and 222 in the same method). On a freshly deployed environment — where these precompute tables haven't been created yet — the very first call to ingestAgency throws relation "route_stops" does not exist and aborts ingestion before the CREATE TABLEs ever run, leaving the system permanently un-ingestable until manual DDL is applied.

This is closely related to the earlier feedback about hosting these tables in ingestion rather than a migration; the practical impact is that even a single successful initial ingest is impossible under the current ordering, regardless of the read-path concern.

Preferred fix: move the CREATE TABLE IF NOT EXISTS (and index) DDL for route_stops and route_branches into a TypeORM migration / bootstrap step that runs before the service starts (this also resolves the read-path concern). Minimal stopgap if the DDL must stay here: hoist both CREATE TABLE IF NOT EXISTS blocks above these DELETEs.

🩹 Stopgap diff (if migration is deferred)
       const agencyEntity = await this.upsertAgency(agency);
       const agencyId = agencyEntity.agencyId;

+      // Ensure precompute tables exist before idempotent cleanup.
+      await this.dataSource.query(`
+        CREATE TABLE IF NOT EXISTS route_stops (
+          id UUID PRIMARY KEY DEFAULT gen_random_uuid(),
+          agency_id UUID NOT NULL REFERENCES agencies("agencyId") ON DELETE CASCADE,
+          stop_id VARCHAR(100) NOT NULL,
+          route_id VARCHAR(100) NOT NULL,
+          short_name VARCHAR(50),
+          long_name TEXT,
+          route_type SMALLINT NOT NULL,
+          UNIQUE (agency_id, stop_id, route_id)
+        )
+      `);
+      await this.dataSource.query(`
+        CREATE TABLE IF NOT EXISTS route_branches (
+          id UUID PRIMARY KEY DEFAULT gen_random_uuid(),
+          agency_id UUID NOT NULL REFERENCES agencies("agencyId") ON DELETE CASCADE,
+          route_id VARCHAR(100) NOT NULL,
+          direction_id SMALLINT,
+          trip_headsign TEXT,
+          trip_id VARCHAR(100) NOT NULL,
+          shape_id VARCHAR(100),
+          stop_count INTEGER NOT NULL
+        )
+      `);
+
       // Clean up stale data before re-inserting (idempotent re-ingestion)
       this.logger.debug(`Deleting existing data for agency ${agencyId}...`);
       await this.dataSource.query(`DELETE FROM stop_times WHERE agency_id = $1`, [agencyId]);
       …
       await this.dataSource.query(`DELETE FROM route_stops WHERE agency_id = $1`, [agencyId]);
       await this.dataSource.query(`DELETE FROM route_branches WHERE agency_id = $1`, [agencyId]);
🤖 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 `@services/ingestion/src/gtfs-static.service.ts` around lines 100 - 109, The
DELETEs for route_stops and route_branches in ingestAgency run before their
CREATE TABLE IF NOT EXISTS DDL, causing failures on a fresh DB; either
(preferred) move the CREATE TABLE IF NOT EXISTS (and relevant index creation)
for route_stops and route_branches into a TypeORM migration/bootstrap that runs
before the service starts, or (minimal stopgap) hoist the CREATE TABLE IF NOT
EXISTS blocks for route_stops and route_branches to execute before the DELETE
statements in ingestAgency so the DELETEs never target missing relations.

@rsun19
rsun19 merged commit a5e9f84 into main May 13, 2026
1 check was pending
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.

1 participant