feat: split backend into microservices - #31
Conversation
…routes, stops, vehicles)
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, 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 have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughConverts a monolithic backend into multiple NestJS microservices with a shared package, adds an ingestion worker for GTFS, updates NGINX to route /api/v1/* to services, reworks docker-compose for dev/prod, and updates documentation and workspace configuration. ChangesMicroservices architecture and shared library
Sequence Diagram(s)sequenceDiagram
participant Client
participant NGINX
participant Services
participant Postgres
participant Redis
participant Ingestion
Client->>NGINX: HTTP /api/v1/*
NGINX->>Services: Proxy to domain service
Services->>Postgres: Query GTFS static data
Services->>Redis: Read/write caches
Ingestion->>Postgres: Ingest GTFS static
Ingestion->>Redis: Publish realtime (vehicles/trip updates/alerts)
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Poem
✨ Finishing Touches🧪 Generate unit tests (beta)
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Note
Due to the large number of review comments, Critical severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
docs/development.md (1)
306-310:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUpdate remaining legacy container references in troubleshooting.
Lines 306–310 still refer to
transit-backend/worker, which conflicts with the newservices/*/ingestionsetup and can send debugging to the wrong containers.🤖 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 306 - 310, Update the troubleshooting doc entries that still mention the old container names `transit-backend` and `worker`: replace `transit-backend` references with the new `services/*` naming and change `worker` commands to the new ingestion service name (`ingestion`) so commands like `docker compose exec worker curl <url>` and `docker compose logs worker` become `docker compose exec ingestion curl <url>` and `docker compose logs ingestion`; also ensure the "Map shows no vehicle markers" row references `config/agencies.json` and the `gtfsRealtimeVehiclePositionsUrl` key still but points readers to check the `ingestion` service logs instead of `worker`.nginx.conf (1)
28-120:⚠️ Potential issue | 🟠 Major | ⚡ Quick winApply the variable pattern to all
/api/v1/*proxy_pass directives—static hostnames ignore theresolverdirective.The
resolver 127.0.0.11 valid=10s ipv6=off;on line 28 only triggers runtime DNS re-resolution when the upstream is referenced through a variable. With literalproxy_pass http://stops:3003;(and the five sibling blocks), nginx resolves the hostname once at config load and caches the IP indefinitely. If any backend container restarts and Docker assigns a new IP (typical duringdocker-compose upor service scaling), nginx continues forwarding to the stale IP until you reload—resulting in 502 errors.The
/health(lines 129–133) and/(lines 135–148) location blocks already use the correct variable pattern. Apply the same to every/api/v1/*location:Example fix for `/api/v1/stops`
location /api/v1/stops { limit_req zone=api burst=10 nodelay; error_page 429 = `@rate_limit_json`; - proxy_pass http://stops:3003; + set $stops_upstream http://stops:3003; + proxy_pass $stops_upstream; proxy_set_header Host $host; proxy_set_header X-Real-IP $remote_addr; proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for; proxy_set_header X-Forwarded-Proto $scheme; }Repeat for
/api/v1/routes,/api/v1/trips,/api/v1/alerts,/api/v1/vehicles, and/api/v1/agencies. With variables, nginx queries the resolver on each request (cached by TTL), so service IP changes are picked up automatically.🤖 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 `@nginx.conf` around lines 28 - 120, The API location blocks use literal proxy_pass targets (e.g., "proxy_pass http://stops:3003;") so nginx resolves their hostnames only at config load instead of via the resolver; change each API block (/api/v1/stops, /api/v1/routes, /api/v1/trips, /api/v1/alerts, /api/v1/vehicles, /api/v1/agencies) to use a variable-backed proxy_pass like the existing /health and / locations do (define a per-block variable such as "set $backend 'stops:3003';" or "set $upstream 'stops:3003';" and replace proxy_pass with "proxy_pass http://$backend;" so nginx will use the resolver 127.0.0.11 and re-resolve backend IPs at runtime).
🟡 Minor comments (14)
services/ingestion/src/cache/cache.service.ts-32-34 (1)
32-34:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winValidate that
ttlSecondsis positive.Negative or zero TTL values could cause unexpected caching behavior. Redis will reject negative values, but catching this earlier provides clearer error messages.
🛡️ Add TTL validation
async set(key: string, value: string, ttlSeconds: number): Promise<void> { + if (ttlSeconds <= 0) { + this.logger.warn(`Invalid TTL ${ttlSeconds} for key "${key}"`); + return; + } try {🤖 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/cache/cache.service.ts` around lines 32 - 34, The set method in CacheService does not validate ttlSeconds, so negative or zero values pass to Redis; update the async set(key: string, value: string, ttlSeconds: number): Promise<void> to check if ttlSeconds is a positive integer (ttlSeconds > 0) and if not throw a clear Error (e.g., "ttlSeconds must be a positive integer") or return a rejected Promise before calling this.redis.set; reference the set method in CacheService to add this guard and ensure callers receive an explicit validation error rather than a Redis rejection.services/ingestion/src/gtfs-realtime.service.ts-94-103 (1)
94-103:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win
addedTripsstructure is defined but never populated.The
addedTripsvariable is initialized (Lines 94-103) and conditionally written to Redis (Lines 158-163), but no code in the loop (Lines 105-143) ever populates it. This means Lines 158-163 will never execute.If this is intentional (stubbed for future functionality), consider adding a TODO comment. Otherwise, either remove the dead code or implement the logic to populate added trips.
♻️ Options
Option 1: Add TODO if this is planned work
// added_trips: hash keyed by trip_id -> { stops: [...] } + // TODO: Implement logic to populate addedTrips from GTFS-RT ADDED trip schedule relationship const addedTrips: Record<Option 2: Remove dead code
- // added_trips: hash keyed by trip_id -> { stops: [...] } - const addedTrips: Record< - string, - { - tripId: string; - routeId: string; - directionId: number | null; - headsign: string | null; - stops: { stopId: string; arrivalTime: number }[]; - } - > = {}; ... - if (Object.keys(addedTrips).length > 0) { - await redis.del(`added_trips:${agency.key}`); - for (const [tripId, trip] of Object.entries(addedTrips)) { - await redis.hset(`added_trips:${agency.key}`, tripId, JSON.stringify(trip)); - } - }🤖 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-realtime.service.ts` around lines 94 - 103, The local variable addedTrips is allocated but never written to inside processRealtimeFeed (so the conditional Redis write later never runs); either implement population logic where new trips are detected inside the loop that processes feedEntity/trip updates (e.g., in the loop around processRealtimeFeed/processFeedEntity or where trips are created/added) by adding entries keyed by a unique id with the shape currently declared, or if this feature is not yet implemented, remove the Redis write block (lines that reference addedTrips) and/or add a clear TODO comment next to the addedTrips declaration to indicate it is intentionally unpopulated; refer to the addedTrips identifier and the Redis write section that checks Object.keys(addedTrips).length to locate and fix the dead code.nginx.dev.conf-80-85 (1)
80-85:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winHealth check only validates agencies service.
The
/healthendpoint proxies only toagencies:3001, so it won't detect failures in the other five services (stops, routes, alerts, vehicles, ingestion). This creates a blind spot in health monitoring.Consider implementing an aggregated health check endpoint that queries all services, or document that this is intentional and each service should be monitored independently.
🤖 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 `@nginx.dev.conf` around lines 80 - 85, The current nginx location /health proxies only to agencies:3001 (see "location /health" and "agencies:3001"), leaving stops/routes/alerts/vehicles/ingestion unmonitored; implement a real aggregated health endpoint that queries each service's health (or a central health-aggregator/gateway route) and returns a combined status, then update the nginx "location /health" proxy_pass to point to that aggregator (or alternatively add/document per-service health endpoints and keep nginx as-is if that is intentional).services/stops/Dockerfile-8-8 (1)
8-8:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winProduction image includes dev dependencies, increasing image size.
npm installwithout the--productionflag installs dev dependencies that aren't needed in production, bloating the image size and potentially including security vulnerabilities from dev packages.📦 Proposed fix
-RUN npm install +RUN npm ci --only=productionOr use a separate production dependency installation in the production stage:
FROM node:20-alpine AS production WORKDIR /app/services/stops COPY --from=builder /app/services/stops/dist ./dist COPY --from=builder /app/services/stops/package.json ./ -COPY --from=builder /app/node_modules ./node_modules +RUN npm ci --only=production🤖 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/Dockerfile` at line 8, The Dockerfile's current RUN npm install installs devDependencies into the production image; change the install step used in the production stage to only install production deps (for example use npm ci --only=production or npm install --production / set NODE_ENV=production before install) or switch to a multi-stage build where the final stage copies only node_modules produced by a production-only install; update the RUN npm install invocation in the Dockerfile to the production-only install command and ensure any lockfile (package-lock.json) is used for deterministic installs.services/alerts/src/main.ts-59-59 (1)
59-59:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winPort parsing lacks validation.
Invalid
PORTenvironment variable can result inNaN, causing binding failures.🛡️ Proposed fix
-const port = process.env['PORT'] ? parseInt(process.env['PORT'], 10) : 3004; +const envPort = process.env['PORT']; +const parsedPort = envPort ? parseInt(envPort, 10) : 3004; +const port = !isNaN(parsedPort) && parsedPort > 0 && parsedPort <= 65535 ? parsedPort : 3004;🤖 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/alerts/src/main.ts` at line 59, The PORT parsing can produce NaN and break binding; update the logic around the port variable assignment (where process.env['PORT'] and parseInt are used) to validate the parsed value and fall back to 3004: parse process.env['PORT'] with parseInt(…, 10), then check Number.isInteger(parsed) and parsed > 0 (or use Number.isFinite and > 0) before assigning to port; if validation fails, use the default 3004 so server.listen/boot won't receive NaN.services/vehicles/src/main.ts-59-59 (1)
59-59:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winPort parsing lacks validation.
parseInt(process.env['PORT'], 10)can returnNaNif the environment variable contains invalid input, causing the server to fail silently or bind to an unexpected port.🛡️ Proposed fix
-const port = process.env['PORT'] ? parseInt(process.env['PORT'], 10) : 3005; +const envPort = process.env['PORT']; +const parsedPort = envPort ? parseInt(envPort, 10) : 3005; +const port = !isNaN(parsedPort) && parsedPort > 0 && parsedPort <= 65535 ? parsedPort : 3005;🤖 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/vehicles/src/main.ts` at line 59, The port parsing needs validation: replace the current inline parseInt usage around the const port declaration so that you parse process.env['PORT'] (using parseInt or Number), check the result for NaN and that it's an integer within the valid TCP range (1–65535), and only then use it; otherwise fall back to the default 3005 (or throw a clear error). Update the logic around the port variable in main.ts (the const port assignment) to perform this validation and choose the safe default.services/routes/Dockerfile-8-8 (1)
8-8:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winProduction image includes dev dependencies, increasing image size.
npm installinstalls dev dependencies unnecessarily in production, bloating image size and potentially including vulnerable dev packages.📦 Proposed fix
-RUN npm install +RUN npm ci --only=production🤖 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/Dockerfile` at line 8, The Dockerfile currently runs RUN npm install which installs devDependencies into the production image; change the build step to install only production dependencies (e.g., use npm ci --only=production or npm install --only=production) and ensure package-lock.json is copied into the image before installation so the lockfile is used; update the Dockerfile step labeled RUN npm install to the production-only install command and, if you rely on reproducible installs, prefer npm ci over npm install.services/stops/src/main.ts-65-65 (1)
65-65:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winPort parsing lacks validation.
Invalid
PORTenvironment variable can result inNaN, causing binding failures.🛡️ Proposed fix
-const port = process.env['PORT'] ? parseInt(process.env['PORT'], 10) : 3003; +const envPort = process.env['PORT']; +const parsedPort = envPort ? parseInt(envPort, 10) : 3003; +const port = !isNaN(parsedPort) && parsedPort > 0 && parsedPort <= 65535 ? parsedPort : 3003;🤖 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/main.ts` at line 65, The PORT parsing currently assigns parseInt(process.env['PORT'], 10) directly to port which can yield NaN and break binding; change to explicitly validate the parsed value: read process.env.PORT (trim), parseInt with radix 10 into a temp (e.g., parsedPort), then set port = (Number.isInteger(parsedPort) && parsedPort > 0) ? parsedPort : 3003 and/or fallback to default if isNaN/invalid; update any code that uses the port variable accordingly to ensure a valid positive integer before calling app.listen.docs/development.md-320-320 (1)
320-320:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUse the same compose-file pair in stale-env recovery commands.
Line 320 uses only
docker-compose.dev.yml, but the dev workflow above is based ondocker-compose.yml+docker-compose.dev.yml. Keep this command consistent to avoid operator confusion.🤖 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` at line 320, Replace the single-file compose command "docker compose -f docker-compose.dev.yml up -d ingestion" with the dual-file form used elsewhere so the same compose-file pair is used for stale-env recovery; update the command to include both files (e.g., "docker compose -f docker-compose.yml -f docker-compose.dev.yml up -d ingestion") in docs/development.md where the current command appears to keep behavior consistent with the dev workflow.services/stops/src/stops.controller.ts-10-44 (1)
10-44:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winNumeric query params accept
NaNand negative values silently.
parseFloat/parseIntonlat/lon/radius/limitwill returnNaNfor non-numeric input. You guardlat/lonagainstNaNand range, but:
radiusonly fails the upper bound —?radius=abcbecomesNaN, passesNaN > NEARBY_MAX_RADIUS_M(false), andradiusM: NaNflows intogetNearbyStops, where it ultimately reaches PostGIS asNaNinST_DWithin(..., $3).limit(and in the other handlers,offset) likewise acceptsNaN, negative numbers, and0, none of which are caught here.Add explicit validation, e.g.:
- const radius = radiusStr ? parseInt(radiusStr, 10) : undefined; - if (radius !== undefined && radius > NEARBY_MAX_RADIUS_M) { - throw new BadRequestException(`radius must not exceed ${NEARBY_MAX_RADIUS_M}m`); - } - - const limit = limitStr ? parseInt(limitStr, 10) : undefined; + const radius = radiusStr ? parseInt(radiusStr, 10) : undefined; + if (radius !== undefined && (!Number.isFinite(radius) || radius <= 0 || radius > NEARBY_MAX_RADIUS_M)) { + throw new BadRequestException(`radius must be between 1 and ${NEARBY_MAX_RADIUS_M}m`); + } + const limit = limitStr ? parseInt(limitStr, 10) : undefined; + if (limit !== undefined && (!Number.isFinite(limit) || limit <= 0)) { + throw new BadRequestException('limit must be a positive integer'); + }The same applies to
limit/offsetinsearch, andlimitingetArrivals. AValidationPipe+ DTO withclass-validatorwould centralize this and remove duplication.🤖 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.controller.ts` around lines 10 - 44, The numeric query parsing in getNearby (lat/lon/radius/limit) allows NaN, zero and negative values to flow into stopsService.getNearbyStops (radiusM/limit) and reach PostGIS; validate parsed numbers explicitly: after parsing lat/lon already validated, add checks that radiusStr and limitStr parse to finite integers (use Number.isFinite or isNaN checks) and enforce sensible bounds (radius >= 0 && radius <= NEARBY_MAX_RADIUS_M; limit > 0 and within your max page size), throwing BadRequestException on invalid input; mirror the same explicit validation for limit/offset in the search handler and limit in getArrivals, or replace all handlers with a shared DTO + ValidationPipe using class-validator to centralize these rules.services/stops/src/stops.service.ts-180-198 (1)
180-198:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winSilent fallback when the stop doesn't exist masks 404s.
If
stopId/agencyIddoesn't match any row, the destructuredstopInfoisundefined, theneffectiveStopIds = [stopId], the arrivals query returns an emptyrows, and the response returns{ data: [], stopName: stopId, ... }— indistinguishable from "this stop has no upcoming arrivals". Clients can't tell typos from quiet schedules, and you're also caching the empty result under that key. Match theNotFoundExceptionsemantics used elsewhere:[stopId, agencyId], ); - const stopName: string = stopInfo?.stop_name ?? stopId; + if (!stopInfo) throw new NotFoundException(`Stop ${stopId} not found for agency ${agencyKey}`); + const stopName: string = stopInfo.stop_name; const childStopIds = stopInfo?.child_stop_ids; const parentStationId = stopInfo?.parent_station_id ?? null;🤖 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 180 - 198, When the SELECT returns no row (stopInfo is undefined) we must surface a 404 instead of silently falling back; after the dataSource.query that populates stopInfo, check if stopInfo is falsy and throw the same NotFoundException used elsewhere (use the existing NotFoundException class/utility in this service) referencing the requested stopId and agencyId; only proceed to compute stopName, childStopIds, parentStationId, and effectiveStopIds if stopInfo exists so we don't cache or return an empty result for a non-existent stop.nginx.conf-122-128 (1)
122-128:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winVerify the
agenciesservice actually serves/health.
/healthis proxied tohttp://agencies:3001without rewriting the path, so the agencies service must register an unauthenticatedGET /healthhandler at the root (not under/api/v1/agencies/health). If it only exposes/api/v1/agencies/health(or no health route at all), this endpoint will 404 and any external uptime checks/load balancers depending on it will break.#!/bin/bash # Look for a health controller/route in the agencies service fd -t f . services/agencies | xargs rg -nP -C2 "@Get\(\s*['\"]?health|@Controller\(\s*['\"]?health|/health\b" 2>/dev/null rg -nP -C2 "@Controller\(" services/agencies 2>/dev/null🤖 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 `@nginx.conf` around lines 122 - 128, The nginx health proxy (location /health -> proxy_pass http://agencies:3001) assumes the agencies service exposes an unauthenticated GET /health at the root; verify the agencies service has a handler matching GET /health (or a controller with "health" in its route) and, if it doesn't, either add a root-level health endpoint in the agencies code or update this nginx block to proxy to the actual route (e.g., /api/v1/agencies/health) so proxy_pass targets the correct path; check controller/route names in the agencies service (health controller or route handlers) to locate where to add or adjust the route.services/routes/src/routes.service.ts-145-145 (1)
145-145:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winMisleading 404 when the route exists but has no scheduled trips.
Line 118 already established that the route exists. If
branchReps.length === 0, the route is real but has notrips/stop_timesrows — that's an empty-data condition, not "not found". Returning 404 here will look identical on the client to "wrong route id", and it short-circuits before the response is built. Prefer returning the route with emptystops/branches:- if (branchReps.length === 0) throw new NotFoundException(`Route ${routeId} not found`); + if (branchReps.length === 0) { + const empty: RouteResponse = { ...this.toResponse(route), stops: [], branches: [] }; + await this.cacheService.set(cacheKey, JSON.stringify(empty), API_CACHE_ROUTES_TTL_S); + return empty; + }🤖 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` at line 145, The check that throws NotFoundException when branchReps.length === 0 is misleading because routeId was already validated; instead of throwing, return the existing route object with empty branches/stops to represent "no scheduled trips." Update the handler that uses branchReps and routeId to build and return the route response when branchReps is empty (populate branches/stops as empty arrays) rather than throwing NotFoundException; ensure downstream response construction (the method that reads branchReps) still runs or is adapted to handle the empty array case.services/routes/src/routes.service.ts-220-241 (1)
220-241:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winSilently emitting
[0, 0]corrupts the shape geometry.When the WKT regex doesn't match (e.g.,
locationisnull/undefined or stored in a different format such as EWKB hex), this falls back to[0, 0]and quietly inserts a point in the Gulf of Guinea into theLineString. A single bad row can produce a route shape that spikes off-map. Either skip the point or fail loud — silently fabricating coordinates is worse than returningnull:- const coords = shapePoints.map((p) => { + const coords: [number, number][] = []; + for (const p of shapePoints) { const location = p.location as unknown; if ( location && typeof location === 'object' && 'coordinates' in (location as Record<string, unknown>) ) { const coordinates = (location as { coordinates?: unknown }).coordinates; if ( Array.isArray(coordinates) && coordinates.length >= 2 && typeof coordinates[0] === 'number' && typeof coordinates[1] === 'number' ) { - return [coordinates[0], coordinates[1]] as [number, number]; + coords.push([coordinates[0], coordinates[1]]); + continue; } } const match = String(location).match(/POINT\(([^ ]+) ([^ )]+)\)/); - return match - ? ([parseFloat(match[1]), parseFloat(match[2])] as [number, number]) - : ([0, 0] as [number, number]); - }); + if (match) { + const lon = parseFloat(match[1]); + const lat = parseFloat(match[2]); + if (Number.isFinite(lon) && Number.isFinite(lat)) coords.push([lon, lat]); + } + } + if (coords.length < 2) return null; return { type: 'LineString', coordinates: coords };🤖 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 220 - 241, The current mapping for coords (involving shapePoints and variable location) silently returns [0,0] on parse failures, corrupting LineString geometry; change the mapping so invalid/unrecognized locations return null (or throw) instead of [0,0], then filter out nulls before constructing the LineString (or propagate an error); update the code around coords, shapePoints and the LineString construction to either skip null points (and optionally log a warning with identifying shape id) or explicitly fail loudly so bad rows cannot inject a Gulf-of-Guinea point.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 8e1f56eb-af33-435d-8316-0b745f982238
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (101)
docker-compose.dev.ymldocker-compose.ymldocs/architecture.mddocs/deployment.mddocs/development.mdnginx.confnginx.dev.confpackage.jsonpackages/shared/package.jsonpackages/shared/src/agency-config.tspackages/shared/src/config/configuration.tspackages/shared/src/constants.tspackages/shared/src/entities/agency.entity.tspackages/shared/src/entities/route.entity.tspackages/shared/src/entities/service-calendar.entity.tspackages/shared/src/entities/shape.entity.tspackages/shared/src/entities/stop-time.entity.tspackages/shared/src/entities/stop.entity.tspackages/shared/src/entities/trip.entity.tspackages/shared/src/index.tspackages/shared/tsconfig.jsonservices/agencies/Dockerfileservices/agencies/nest-cli.jsonservices/agencies/package.jsonservices/agencies/src/agencies.controller.tsservices/agencies/src/agencies.module.tsservices/agencies/src/agencies.service.tsservices/agencies/src/health.controller.tsservices/agencies/src/main.tsservices/agencies/tsconfig.jsonservices/alerts/Dockerfileservices/alerts/nest-cli.jsonservices/alerts/package.jsonservices/alerts/src/agencies/agencies.module.tsservices/alerts/src/agencies/agencies.service.tsservices/alerts/src/alerts.controller.tsservices/alerts/src/alerts.module.tsservices/alerts/src/cache/cache.constants.tsservices/alerts/src/cache/cache.module.tsservices/alerts/src/cache/cache.service.tsservices/alerts/src/health.controller.tsservices/alerts/src/main.tsservices/alerts/tsconfig.jsonservices/ingestion/Dockerfileservices/ingestion/nest-cli.jsonservices/ingestion/package.jsonservices/ingestion/src/agencies/agencies.module.tsservices/ingestion/src/agencies/agencies.service.tsservices/ingestion/src/cache/cache.constants.tsservices/ingestion/src/cache/cache.module.tsservices/ingestion/src/cache/cache.service.tsservices/ingestion/src/gtfs-realtime.service.tsservices/ingestion/src/gtfs-static.service.tsservices/ingestion/src/ingestion.scheduler.tsservices/ingestion/src/worker.module.tsservices/ingestion/src/worker.tsservices/ingestion/tsconfig.jsonservices/routes/Dockerfileservices/routes/nest-cli.jsonservices/routes/package.jsonservices/routes/src/cache/cache.constants.tsservices/routes/src/cache/cache.module.tsservices/routes/src/cache/cache.service.tsservices/routes/src/health.controller.tsservices/routes/src/http-exception.filter.tsservices/routes/src/main.tsservices/routes/src/routes.controller.tsservices/routes/src/routes.module.tsservices/routes/src/routes.service.tsservices/routes/src/trips.controller.tsservices/routes/src/trips.service.tsservices/routes/tsconfig.jsonservices/stops/Dockerfileservices/stops/nest-cli.jsonservices/stops/package.jsonservices/stops/src/cache/cache.constants.tsservices/stops/src/cache/cache.module.tsservices/stops/src/cache/cache.service.tsservices/stops/src/health.controller.tsservices/stops/src/http-exception.filter.tsservices/stops/src/main.tsservices/stops/src/mergeColocatedStops.tsservices/stops/src/reconcileAddedTrips.tsservices/stops/src/stops.controller.tsservices/stops/src/stops.module.tsservices/stops/src/stops.service.tsservices/stops/src/stops.types.tsservices/stops/tsconfig.jsonservices/vehicles/Dockerfileservices/vehicles/nest-cli.jsonservices/vehicles/package.jsonservices/vehicles/src/agencies/agencies.module.tsservices/vehicles/src/agencies/agencies.service.tsservices/vehicles/src/cache/cache.constants.tsservices/vehicles/src/cache/cache.module.tsservices/vehicles/src/cache/cache.service.tsservices/vehicles/src/health.controller.tsservices/vehicles/src/main.tsservices/vehicles/src/vehicles.controller.tsservices/vehicles/src/vehicles.module.tsservices/vehicles/tsconfig.json
| COPY --from=builder /app/services/agencies/dist ./dist | ||
| COPY --from=builder /app/services/agencies/package.json ./ | ||
| COPY --from=builder /app/node_modules ./node_modules | ||
| COPY config/ /app/config/ |
There was a problem hiding this comment.
Critical: Config path mismatch will cause runtime failure.
The production stage sets WORKDIR /app/services/agencies (line 21) and copies config/ to /app/config/ (absolute path). However, services/agencies/src/agencies.service.ts defaults to ./config/agencies.json, which resolves to /app/services/agencies/config/agencies.json at runtime. The config file will not be found.
🐛 Proposed fix
-COPY config/ /app/config/
+COPY services/agencies/config/ ./config/This copies the agencies config into the WORKDIR so ./config/agencies.json resolves correctly.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| COPY config/ /app/config/ | |
| COPY services/agencies/config/ ./config/ |
🤖 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/agencies/Dockerfile` at line 26, The Dockerfile copies config/ to an
absolute path (/app/config/) while the production WORKDIR is set to
/app/services/agencies and services/agencies/src/agencies.service.ts expects
./config/agencies.json; update the Dockerfile COPY to place the config directory
inside the WORKDIR (e.g., COPY config/ ./config/ or COPY config/
/app/services/agencies/config/) so that ./config/agencies.json resolves
correctly at runtime.
| COPY --from=builder /app/services/routes/dist ./dist | ||
| COPY --from=builder /app/services/routes/package.json ./ | ||
| COPY --from=builder /app/node_modules ./node_modules | ||
| COPY config/ /app/config/ |
There was a problem hiding this comment.
Config directory will be missing in production image.
Line 26 copies config/ from the host build context, not from the builder stage. The production image will lack config files, causing runtime failures.
🐛 Proposed fix
COPY --from=builder /app/services/routes/dist ./dist
COPY --from=builder /app/services/routes/package.json ./
COPY --from=builder /app/node_modules ./node_modules
-COPY config/ /app/config/
+COPY --from=builder /app/config/ /app/config/Or if config is service-specific:
-COPY config/ /app/config/
+COPY --from=builder /app/services/routes/config/ /app/config/📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| COPY config/ /app/config/ | |
| COPY --from=builder /app/services/routes/config/ /app/config/ |
🤖 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/Dockerfile` at line 26, The Dockerfile currently copies
config/ from the build context (COPY config/ /app/config/) which will be missing
in the final production image; update the Dockerfile to copy the built/packaged
config files from the builder stage instead (use COPY --from=builder ... or the
appropriate stage name), or, if configs are service-specific, copy the correct
service config directory from the builder stage; locate the COPY config/
/app/config/ line and replace it with a COPY that references the builder stage
so the production image contains the config files.
| COPY --from=builder /app/services/stops/dist ./dist | ||
| COPY --from=builder /app/services/stops/package.json ./ | ||
| COPY --from=builder /app/node_modules ./node_modules | ||
| COPY config/ /app/config/ |
There was a problem hiding this comment.
Config directory will be missing in production image.
Line 26 copies config/ from the host build context, not from the builder stage. Since the builder stage doesn't include this directory, the production image will lack the config files, likely causing runtime failures.
🐛 Proposed fix
COPY --from=builder /app/services/stops/dist ./dist
COPY --from=builder /app/services/stops/package.json ./
COPY --from=builder /app/node_modules ./node_modules
-COPY config/ /app/config/
+COPY --from=builder /app/config/ /app/config/Alternatively, if config is service-specific:
-COPY config/ /app/config/
+COPY --from=builder /app/services/stops/config/ /app/config/📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| COPY config/ /app/config/ | |
| COPY --from=builder /app/services/stops/dist ./dist | |
| COPY --from=builder /app/services/stops/package.json ./ | |
| COPY --from=builder /app/node_modules ./node_modules | |
| COPY --from=builder /app/config/ /app/config/ |
🤖 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/Dockerfile` at line 26, The final image is copying host
config/ instead of inheriting it from the builder stage, so the production image
will miss config files; update the Dockerfile to either (A) ensure the builder
stage includes the config/ directory (add a COPY of config/ into the builder
stage where other build artifacts are assembled) or (B) change the final-stage
COPY to pull config from the builder (use COPY --from=builder to copy
/app/config/ or the builder path into /app/config/), and if config is
service-specific, place it into the builder context for the stops service before
finalizing the image.
…ic ingestion service
1 participant
Summary
Decomposes the monolithic backend into six independent microservices, each with its own NestJS app, Dockerfile, and package.json.
Services
Shared Package
Introduces
packages/sharedcontaining common entities, configuration helpers, and constants consumed across services.Infrastructure
docker-compose.ymlanddocker-compose.dev.ymlto spin up each service independentlynginx.dev.conffor local dev routingnginx.conffor production upstream routing to each serviceSummary by CodeRabbit
/api/v1/path-based architecture.