Merge pull request #578 from trails-cool/feat/gpx-parser-robustness-metadata

feat(gpx): metadata name/desc fallback + <cmt> merge

openspec/changes/gpx-parser-robustness/tasks.md

@@ -17,8 +17,8 @@
 
 ## 4. Metadata fallback
 
-- [ ] 4.1 Fall back to first `<trk>`/`<rte>` `<name>`/`<desc>` when `<metadata>` lacks them; merge distinct `<cmt>` into description with blank-line separator, dedup identical
-- [ ] 4.2 Unit tests: track-name fallback, metadata precedence, cmt merge, identical cmt deduped
+- [x] 4.1 Fall back to first `<trk>`/`<rte>` `<name>`/`<desc>` when `<metadata>` lacks them; merge distinct `<cmt>` into description with blank-line separator, dedup identical
+- [x] 4.2 Unit tests: track-name fallback, metadata precedence, cmt merge, identical cmt deduped
 
 ## 5. Fixture corpus
 

packages/gpx/src/parse.test.ts

@@ -197,3 +197,49 @@ describe("parseGpxAsync — route (<rte>) support", () => {
     expect(result.tracks[1]![0]!.lat).toBe(10.0);
   });
 });
+
+describe("parseGpxAsync — metadata fallback", () => {
+  const gpx = (body: string) =>
+    `<?xml version="1.0" encoding="UTF-8"?>\n<gpx version="1.1" xmlns="http://www.topografix.com/GPX/1/1">${body}</gpx>`;
+  const seg = `<trkseg><trkpt lat="52.52" lon="13.405"></trkpt><trkpt lat="48.137" lon="11.576"></trkpt></trkseg>`;
+
+  it("falls back to the first track's name/desc when metadata lacks them", async () => {
+    const result = await parseGpxAsync(gpx(`<trk><name>Track Title</name><desc>Track note</desc>${seg}</trk>`));
+    expect(result.name).toBe("Track Title");
+    expect(result.description).toBe("Track note");
+  });
+
+  it("falls back to a route's name when there is no track", async () => {
+    const result = await parseGpxAsync(
+      gpx(`<rte><name>Route Title</name><rtept lat="52.52" lon="13.405"></rtept><rtept lat="48.137" lon="11.576"></rtept></rte>`),
+    );
+    expect(result.name).toBe("Route Title");
+  });
+
+  it("prefers <metadata> over the track name/desc", async () => {
+    const result = await parseGpxAsync(
+      gpx(`<metadata><name>Meta Name</name><desc>Meta desc</desc></metadata><trk><name>Track Name</name><desc>Track desc</desc>${seg}</trk>`),
+    );
+    expect(result.name).toBe("Meta Name");
+    expect(result.description).toBe("Meta desc");
+  });
+
+  it("merges a distinct <cmt> into the description, blank-line separated", async () => {
+    const result = await parseGpxAsync(
+      gpx(`<trk><desc>The description</desc><cmt>An extra comment</cmt>${seg}</trk>`),
+    );
+    expect(result.description).toBe("The description\n\nAn extra comment");
+  });
+
+  it("does not duplicate a <cmt> identical to the description", async () => {
+    const result = await parseGpxAsync(
+      gpx(`<trk><desc>Same text</desc><cmt>Same text</cmt>${seg}</trk>`),
+    );
+    expect(result.description).toBe("Same text");
+  });
+
+  it("uses <cmt> as the description when there is no desc", async () => {
+    const result = await parseGpxAsync(gpx(`<trk><cmt>Only a comment</cmt>${seg}</trk>`));
+    expect(result.description).toBe("Only a comment");
+  });
+});

packages/gpx/src/parse.ts

@@ -29,8 +29,7 @@ function parseGpxWithParser(parser: DOMParser, xml: string): GpxData {
     throw new Error(`Invalid GPX XML: ${parserError.textContent}`);
   }
 
-  const name = doc.querySelector("metadata > name")?.textContent ?? undefined;
-  const description = doc.querySelector("metadata > desc")?.textContent ?? undefined;
+  const { name, description } = parseMetadata(doc);
   const waypoints = parseWaypoints(doc);
   const tracks = repairTimestamps(parseTracks(doc));
   const noGoAreas = parseNoGoAreas(doc);
@@ -39,6 +38,40 @@ function parseGpxWithParser(parser: DOMParser, xml: string): GpxData {
   return { name, description, waypoints, tracks, noGoAreas, distance: totalDistance, elevation };
 }
 
+/** Text of the first direct child of `el` with the given (lowercased) tag. */
+function directChildText(el: Element | null, tag: string): string | undefined {
+  if (!el) return undefined;
+  for (const child of Array.from(el.children)) {
+    if (child.tagName.toLowerCase() === tag) return child.textContent ?? undefined;
+  }
+  return undefined;
+}
+
+/**
+ * Resolve name/description with fallback (spec: gpx-parser-robustness
+ * "Metadata fallback"). Order: `<metadata>` first, else the first
+ * `<trk>`/`<rte>`'s `<name>`/`<desc>` — many apps put the only
+ * human-readable title on the track. That track/route's `<cmt>` is
+ * appended to the description (blank-line separated) when present and not
+ * already identical (OM's BuildDescription dedup).
+ */
+function parseMetadata(doc: Document): { name?: string; description?: string } {
+  const firstTrack = doc.querySelector("trk, rte");
+  const name =
+    doc.querySelector("metadata > name")?.textContent ??
+    directChildText(firstTrack, "name") ??
+    undefined;
+  let description =
+    doc.querySelector("metadata > desc")?.textContent ??
+    directChildText(firstTrack, "desc") ??
+    undefined;
+  const cmt = directChildText(firstTrack, "cmt");
+  if (cmt && cmt !== description) {
+    description = description ? `${description}\n\n${cmt}` : cmt;
+  }
+  return { name, description };
+}
+
 function parseWaypoints(doc: Document): Waypoint[] {
   const wpts = doc.querySelectorAll("wpt");
   return Array.from(wpts).map((wpt) => {