fix(places): read an imported row for the dedup set in one helper

The candidate a GPX or KML row is checked as is built in lineCandidate, so
places.service.ts stays within its size baseline, and the atomic import test
imports the same Ridge line it later expects to be skipped.

server/src/nest/places/places.helpers.ts

@@ -86,15 +86,66 @@ export async function reclaimPhotoCache(
 
 export interface DedupSet {
   names: Set<string>;
-  /** The names of the places with a line: a track to import matches only these (#2810). Absent where none is read. */
-  trackNames?: Set<string>;
+  /**
+   * The places with a line, keyed by name and course (`trackKey`): a track to import
+   * matches only these (#2810), and only one that runs the same way (#2870). Absent
+   * where none is read.
+   */
+  tracks?: Set<string>;
   coords: Array<{ lat: number; lng: number }>;
   /** Provider ids (google_place_id, google_ftid, osm_id, amap_poi_id) of the places already in the trip. */
   externalIds: Set<string>;
 }
 
-/** A candidate for the dedup set; `track` when it is a line (a GPX track or route, a KML path). */
-export type DedupCandidate = PlaceMatchCandidate & { track?: boolean };
+/**
+ * A candidate for the dedup set; `track` when it is a line (a GPX track or route, a
+ * KML path), with that line in `routeGeometry`.
+ */
+export type DedupCandidate = PlaceMatchCandidate & { track?: boolean; routeGeometry?: string | null };
+
+/**
+ * Where a line starts, ends and how far it reaches, to about a hundred metres: two
+ * hikes a watch names alike ("Afternoon Hike") differ here, while the same file read
+ * again gives the same answer however its points were rounded or thinned. Empty for
+ * a line that does not parse.
+ */
+function lineCourse(routeGeometry: string | null | undefined): string {
+  let points: unknown;
+  try {
+    points = JSON.parse(routeGeometry ?? '');
+  } catch {
+    return '';
+  }
+  if (!Array.isArray(points)) return '';
+  const line = points.filter(
+    (p): p is [number, number] => Array.isArray(p) && Number.isFinite(p[0]) && Number.isFinite(p[1]),
+  );
+  if (line.length === 0) return '';
+  // A loop rather than Math.min(...): a recorded day holds more points than a call takes arguments.
+  let [south, west] = line[0];
+  let [north, east] = line[0];
+  for (const [lat, lng] of line) {
+    south = Math.min(south, lat);
+    north = Math.max(north, lat);
+    west = Math.min(west, lng);
+    east = Math.max(east, lng);
+  }
+  const [first, last] = [line[0], line[line.length - 1]];
+  return [first[0], first[1], last[0], last[1], south, west, north, east].map((v) => v.toFixed(3)).join(',');
+}
+
+/** An imported row as the dedup set reads it: a line when it carries one. */
+export function lineCandidate(
+  name: string | null | undefined,
+  row: { lat: number | null; lng: number | null; routeGeometry?: string | null },
+): DedupCandidate {
+  return { name, lat: row.lat, lng: row.lng, track: !!row.routeGeometry, routeGeometry: row.routeGeometry };
+}
+
+/** A track's entry in `DedupSet.tracks`: its normalized name and its course. */
+function trackKey(name: string, routeGeometry: string | null | undefined): string {
+  return `${name}\u0000${lineCourse(routeGeometry)}`;
+}
 
 /**
  * Returns true if a candidate place is already represented in the dedup set.
@@ -114,9 +165,14 @@ export function isPlaceDuplicate(candidate: DedupCandidate, dedup: DedupSet): bo
     if (strategy.by === 'externalId') {
       if (dedup.externalIds.has(strategy.id)) return true;
     } else if (strategy.by === 'name') {
-      // A track is the same as a track of that name, never as a sight of that name:
-      // a watch names a hike after the place it went to (#2810).
-      if ((candidate.track ? dedup.trackNames : dedup.names)?.has(strategy.name)) return true;
+      // A track is the same as a track of that name that runs the same way, never as a
+      // sight of that name: a watch names a hike after the place it went to (#2810), and
+      // two hikes of one day alike (#2870).
+      if (candidate.track) {
+        if (dedup.tracks?.has(trackKey(strategy.name, candidate.routeGeometry))) return true;
+      } else if (dedup.names.has(strategy.name)) {
+        return true;
+      }
     } else if (
       dedup.coords.some(
         (c) =>
@@ -143,19 +199,19 @@ export function dedupSetOf(
   >,
 ): DedupSet {
   const names = new Set<string>();
-  const trackNames = new Set<string>();
+  const tracks = new Set<string>();
   const coords: Array<{ lat: number; lng: number }> = [];
   const externalIds = new Set<string>();
   for (const row of rows) {
     for (const id of externalIdsOf(row)) externalIds.add(id);
     if (row.name) {
       names.add(row.name.trim().toLowerCase());
-      if (row.route_geometry) trackNames.add(row.name.trim().toLowerCase());
+      if (row.route_geometry) tracks.add(trackKey(row.name.trim().toLowerCase(), row.route_geometry));
     } else if (row.lat != null && row.lng != null) {
       coords.push({ lat: row.lat, lng: row.lng });
     }
   }
-  return { names, trackNames, coords, externalIds };
+  return { names, tracks, coords, externalIds };
 }
 
 /** Record a newly inserted place so subsequent candidates in the same batch are checked against it. */
@@ -164,7 +220,7 @@ export function trackInsertedInDedupSet(place: DedupCandidate, dedup: DedupSet):
   const normalizedName = normalizePlaceName(place.name);
   if (normalizedName) {
     dedup.names.add(normalizedName);
-    if (place.track) (dedup.trackNames ??= new Set()).add(normalizedName);
+    if (place.track) (dedup.tracks ??= new Set()).add(trackKey(normalizedName, place.routeGeometry));
   } else if (place.lat != null && place.lng != null) {
     dedup.coords.push({ lat: place.lat, lng: place.lng });
   }

server/src/nest/places/places.service.ts

@@ -58,7 +58,7 @@ import {
   buildCategoryNameLookup,
   escapeLikePattern,
   dedupSetOf,
-  isPlaceDuplicate,
+  isPlaceDuplicate, lineCandidate,
   mapWithConcurrency,
   pickEnrichmentMatch,
   reclaimPhotoCache,
@@ -758,7 +758,7 @@ export class PlacesService {
   async gpxDuplicates(tripId: string, rows: PreparedGpxPlace[]): Promise<boolean[]> {
     const dedup = await this.buildDedupSet(tripId);
     return rows.map((wp) => {
-      const candidate = { name: wp.name, lat: wp.lat, lng: wp.lng, track: !!wp.routeGeometry };
+      const candidate = lineCandidate(wp.name, wp);
       if (isPlaceDuplicate(candidate, dedup)) return true;
       trackInsertedInDedupSet(candidate, dedup);
       return false;
@@ -925,7 +925,7 @@ export class PlacesService {
     // `findWithTagsAndRatings` INSIDE the caller's transaction, so the read
     // sees the still-uncommitted row.
     for (const wp of waypoints) {
-      if (isPlaceDuplicate({ name: wp.name, lat: wp.lat, lng: wp.lng, track: !!wp.routeGeometry }, dedup)) {
+      if (isPlaceDuplicate(lineCandidate(wp.name, wp), dedup)) {
         skipped++;
         continue;
       }
@@ -960,7 +960,7 @@ export class PlacesService {
       });
       const place = (await this.placesRepo.findWithTagsAndRatings(placeId))!;
       created.push(place);
-      trackInsertedInDedupSet({ name: wp.name, lat: wp.lat, lng: wp.lng, track: !!wp.routeGeometry }, dedup);
+      trackInsertedInDedupSet(lineCandidate(wp.name, wp), dedup);
     }
 
     return { places: created, count: created.length, skipped };
@@ -1033,7 +1033,7 @@ export class PlacesService {
         const fallbackName = `Placemark ${fallbackIndex}`;
         const name = parsedPlacemark.name || fallbackName;
 
-        if (isPlaceDuplicate({ name, lat: parsedPlacemark.lat, lng: parsedPlacemark.lng, track: isPath }, dedup)) {
+        if (isPlaceDuplicate(lineCandidate(name, parsedPlacemark), dedup)) {
           summary.skippedCount += 1;
           dupCount++;
           fallbackIndex += 1;
@@ -1079,7 +1079,7 @@ export class PlacesService {
 
         const place = (await this.placesRepo.findWithTagsAndRatings(placeId))!;
         created.push(place);
-        trackInsertedInDedupSet({ name, lat: parsedPlacemark.lat, lng: parsedPlacemark.lng, track: isPath }, dedup);
+        trackInsertedInDedupSet(lineCandidate(name, parsedPlacemark), dedup);
         summary.createdCount += 1;
         fallbackIndex += 1;
       }

server/tests/e2e/tours.e2e.test.ts

@@ -535,6 +535,22 @@ describe('Tours e2e (real guards + temp SQLite)', () => {
       expect(await countRows(orm, Tours)).toBe(1);
     });
 
+    it('imports two different tracks of the same name, and still skips either one the second time (#2870)', async () => {
+      // A second hike the watch called "Alpine loop" too, somewhere else entirely.
+      const other = TRACK_GPX.replace(/lat="49(\.\d+)?" lon="12/g, (m) => m.replace('49', '47').replace('12', '11'));
+      expect((await importGpx(OWNER, TRACK_GPX)).body).toMatchObject({ skipped: 0 });
+      expect((await previewGpx(OWNER, other)).body.tracks[0]).toMatchObject({ duplicate: false });
+      const second = await importGpx(OWNER, other);
+      expect(second.status).toBe(201);
+      expect(second.body).toMatchObject({ skipped: 0 });
+      expect(second.body.tours).toHaveLength(1);
+      expect(await countRows(orm, Tours)).toBe(2);
+
+      expect((await importGpx(OWNER, TRACK_GPX)).body).toEqual({ tours: [], caution: false, skipped: 1 });
+      expect((await importGpx(OWNER, other)).body).toEqual({ tours: [], caution: false, skipped: 1 });
+      expect(await countRows(orm, Tours)).toBe(2);
+    });
+
     it('takes over a track from before Tours with the same name instead of skipping it (#2810)', async () => {
       // 4.x stored a GPX track as a plain place: a line, no tours row.
       const line = '[[49,12,100],[49.01,12.01,150],[49.02,12.02,120]]';

server/tests/unit/nest/places.helpers.test.ts

@@ -33,7 +33,9 @@ describe('isPlaceDuplicate / trackInsertedInDedupSet', () => {
       { name: ' 008 ', lat: 1, lng: 2, route_geometry: '[[1,2],[3,4]]' },
     ]);
     expect(isPlaceDuplicate({ name: 'hokitika gorge', lat: -42.9, lng: 171.0, track: true }, dedup)).toBe(false);
-    expect(isPlaceDuplicate({ name: '008', lat: 1, lng: 2, track: true }, dedup)).toBe(true);
+    expect(isPlaceDuplicate({ name: '008', lat: 1, lng: 2, track: true, routeGeometry: '[[1,2],[3,4]]' }, dedup)).toBe(
+      true,
+    );
     // A pin still matches every place of its name, a track included.
     expect(isPlaceDuplicate({ name: 'Hokitika Gorge', lat: null, lng: null }, dedup)).toBe(true);
     expect(isPlaceDuplicate({ name: '008', lat: null, lng: null }, dedup)).toBe(true);
@@ -46,7 +48,37 @@ describe('isPlaceDuplicate / trackInsertedInDedupSet', () => {
     trackInsertedInDedupSet({ name: 'Ridge', lat: 1, lng: 2 }, bare);
     expect(isPlaceDuplicate({ name: 'Ridge', lat: 1, lng: 2, track: true }, bare)).toBe(false);
     trackInsertedInDedupSet({ name: 'Lake', lat: 1, lng: 2, track: true }, bare);
-    expect([...(bare.trackNames ?? [])]).toEqual(['lake']);
+    expect([...(bare.tracks ?? [])]).toEqual(['lake\u0000']);
+  });
+
+  it('keeps two tracks of one name apart when they run differently (#2870)', () => {
+    // Two watch hikes, both called "Afternoon Hike": one up the valley, one along the coast.
+    const valley = '[[46.5,8.0,1200],[46.52,8.03,1500],[46.55,8.06,1900]]';
+    const coast = '[[43.7,7.25],[43.72,7.3],[43.74,7.35]]';
+    const dedup = dedupSetOf([{ name: 'Afternoon Hike', lat: 46.5, lng: 8.0, route_geometry: valley }]);
+    expect(
+      isPlaceDuplicate({ name: 'afternoon hike', lat: 43.7, lng: 7.25, track: true, routeGeometry: coast }, dedup),
+    ).toBe(false);
+    // The same file again is still the same track, its points rounded or thinned as they may be.
+    const valleyAgain = '[[46.50001,8.00002],[46.55,8.06]]';
+    expect(
+      isPlaceDuplicate({ name: 'Afternoon Hike', lat: 46.5, lng: 8.0, track: true, routeGeometry: valleyAgain }, dedup),
+    ).toBe(true);
+
+    // Within one file: the first of the two goes in, the second is not taken for it.
+    const batch = emptyDedup();
+    const first = { name: 'Day 1', lat: 46.5, lng: 8.0, track: true, routeGeometry: valley };
+    const second = { name: 'Day 1', lat: 43.7, lng: 7.25, track: true, routeGeometry: coast };
+    expect(isPlaceDuplicate(first, batch)).toBe(false);
+    trackInsertedInDedupSet(first, batch);
+    expect(isPlaceDuplicate(second, batch)).toBe(false);
+    trackInsertedInDedupSet(second, batch);
+    expect(isPlaceDuplicate({ ...second }, batch)).toBe(true);
+
+    // A line that does not parse falls back to the name alone, on both sides.
+    const broken = dedupSetOf([{ name: 'Loop', lat: 1, lng: 2, route_geometry: 'not json' }]);
+    expect(isPlaceDuplicate({ name: 'Loop', lat: 1, lng: 2, track: true, routeGeometry: '{}' }, broken)).toBe(true);
+    expect(isPlaceDuplicate({ name: 'Loop', lat: 1, lng: 2, track: true, routeGeometry: coast }, broken)).toBe(false);
   });
 
   it('matches a named place case- and whitespace-insensitively', () => {

server/tests/unit/nest/tours.gpx.atomic.test.ts

@@ -418,9 +418,10 @@ describe('Tours GPX atomic persistence and postcommit publication', () => {
   });
 
   it('imports only the new tracks and publishes only committed rows in a mixed result', async () => {
+    // The same Ridge as in the mixed file, line and all: a track is a duplicate only of one that runs alike (#2870).
     const existing = Buffer.from(`<gpx><trk><name>Ridge</name><trkseg>
       <trkpt lat="49" lon="12"><ele>100</ele></trkpt><trkpt lat="49.01" lon="12.01"><ele>150</ele></trkpt>
-    </trkseg></trk></gpx>`);
+    </trkseg><trkseg><trkpt lat="49.02" lon="12.02"><ele>120</ele></trkpt></trkseg></trk></gpx>`);
     expect((await tours.importGpxAsTour(tripId, existing))?.tours).toHaveLength(1);
     broadcast.mockClear();