fix: add null safeguard and source-asymmetry test for buildVehiclePopupData

src/lib/Provider/OpenStreetMapProvider.svelte.js

@@ -26,11 +26,10 @@ import { t } from 'svelte-i18n';
 import { ROUTE_PANE_Z_INDEX } from '$lib/mapPanes.js';
 
 // activeTrip is always truthy here: the sole caller (vehicleUtils.js) guards on
-// it, and buildVehiclePopupData reads activeTrip.tripHeadsign without optional
-// chaining. Keep this contract consistent rather than implying null is expected.
+// it. The optional chaining here and in buildVehiclePopupData is belt-and-braces.
 function getVehicleLabel(activeTrip) {
 	const translate = get(t);
-	return activeTrip.tripHeadsign
+	return activeTrip?.tripHeadsign
 		? translate('vehicle.to_headsign', { values: { headsign: activeTrip.tripHeadsign } })
 		: translate('vehicle.label');
 }

src/lib/__tests__/vehicleUtils.test.js

@@ -72,14 +72,60 @@ describe('buildVehiclePopupData', () => {
 			predicted: false
 		});
 	});
+
+	it('extracts fields from the correct sources when vehicle and activeTrip share properties', () => {
+		const vehicle = {
+			vehicleId: 'v-789',
+			lastUpdateTime: 1600000200,
+			predicted: true,
+			tripHeadsign: 'Wrong Destination' // Testing asymmetry
+		};
+		const activeTrip = {
+			tripHeadsign: 'Right Destination',
+			vehicleId: 'wrong-id', // Testing asymmetry
+			lastUpdateTime: 999999999,
+			predicted: false
+		};
+		const stopsMap = new Map();
+
+		const result = buildVehiclePopupData(vehicle, activeTrip, stopsMap);
+
+		expect(result).toEqual({
+			nextDestination: 'Right Destination',
+			vehicleId: 'v-789',
+			lastUpdateTime: 1600000200,
+			nextStopName: undefined,
+			predicted: true
+		});
+	});
+
+	it('returns undefined nextDestination safely when activeTrip is undefined', () => {
+		const vehicle = {
+			vehicleId: 'v-999',
+			lastUpdateTime: 1600000300,
+			predicted: false
+		};
+		const activeTrip = undefined;
+		const stopsMap = new Map();
+
+		const result = buildVehiclePopupData(vehicle, activeTrip, stopsMap);
+
+		expect(result).toEqual({
+			nextDestination: undefined,
+			vehicleId: 'v-999',
+			lastUpdateTime: 1600000300,
+			nextStopName: undefined,
+			predicted: false
+		});
+	});
 });
 
 // This suite used to drive the now-deleted single-route `updateVehicleMarkers`
 // wrapper directly. That wrapper had zero production callers (SearchPane and
 // RouteMap both go through `fetchAndUpdateVehicles`) and only duplicated what
 // `fetchAndUpdateVehiclesForRoutes` already does for a single-route array, so
-// it was removed. The behavior under test here — highlighting, marker-keying
-// by vehicleId, routeColor forwarding — lives in the shared
+// it was removed. The behavior under test here ΓÇö highlighting, marker-keying
+// by vehicleId, routeColor forwarding ΓÇö lives in the shared
 // `applyRouteVehicles` helper, which the production path also exercises, so
 // driving it through `fetchAndUpdateVehiclesForRoutes([{ id: 'route-1' }], ...)`
 // preserves the same coverage.
@@ -563,11 +609,11 @@ describe('removeVehicleMarkersForRoutes', () => {
 });
 
 // Task 10b: highlightedTripId used to be captured once, by value, at poll
-// start — so it could never change without restarting the whole poll. It can
+// start ΓÇö so it could never change without restarting the whole poll. It can
 // now also be a getter, re-resolved on every tick, so a trip expansion can
 // move the highlight glow onto a live poll instead of waiting for the next
 // route redraw.
-describe('fetchAndUpdateVehiclesForRoutes — live highlight', () => {
+describe('fetchAndUpdateVehiclesForRoutes ΓÇö live highlight', () => {
 	beforeEach(() => {
 		clearVehicleMarkersMap();
 		vi.useFakeTimers();
@@ -603,7 +649,7 @@ describe('fetchAndUpdateVehiclesForRoutes — live highlight', () => {
 		expect(firstPassCalls.find((c) => c[1].id === 't_r_a')[3]).toBe(true);
 		expect(firstPassCalls.find((c) => c[1].id === 't_r_b')[3]).toBe(false);
 
-		// Change which trip is highlighted, then force a tick — a value
+		// Change which trip is highlighted, then force a tick ΓÇö a value
 		// captured once at poll start could never observe this change.
 		highlighted = 't_r_b';
 		provider.updateVehicleMarker.mockClear();
@@ -630,7 +676,7 @@ describe('fetchAndUpdateVehiclesForRoutes — live highlight', () => {
 		);
 		expect(provider.addVehicleMarker).toHaveBeenCalledTimes(1);
 
-		// A second, distinct vehicle appears — proves this ran a real extra
+		// A second, distinct vehicle appears ΓÇö proves this ran a real extra
 		// fetch/apply cycle, not just a no-op re-invocation.
 		vehicleId = 'v2';
 		await tick();
@@ -730,7 +776,7 @@ describe('fetchAndUpdateVehicles (wrapper)', () => {
 
 	// SearchPane.svelte:174 and RouteMap.svelte:100 both do
 	// `currentIntervalId = await fetchAndUpdateVehicles(...)` and later
-	// `clearInterval(currentIntervalId)` — unlike
+	// `clearInterval(currentIntervalId)` ΓÇö unlike
 	// fetchAndUpdateVehiclesForRoutes, this wrapper must keep resolving to a
 	// bare interval id, not `{ intervalId, tick }`, or both call sites break.
 	test('resolves to a bare interval id, not an { intervalId, tick } object', async () => {

src/lib/vehicleUtils.js

@@ -267,7 +267,7 @@ export function removeVehicleMarkersForRoutes(routeIds, mapProvider) {
 
 export function buildVehiclePopupData(vehicle, activeTrip, stopsMap) {
 	return {
-		nextDestination: activeTrip.tripHeadsign,
+		nextDestination: activeTrip?.tripHeadsign,
 		vehicleId: vehicle.vehicleId,
 		lastUpdateTime: vehicle.lastUpdateTime,
 		nextStopName: stopsMap.get(vehicle.nextStop)?.name,