docs+openspec: prior-art research (Organic Maps, Endurain, wanderer) and 15 proposals
Add docs/inspirations.md as the durable record of the 2026-07-05/06 prior-art research — per-project learnings with source paths, canonical credit lines, and the changes each spawned — and extend the acknowledgment lists in philosophy.md/architecture.md (Organic Maps, Endurain, wanderer). New OpenSpec changes (proposal/design/specs/tasks each): - Organic Maps: elevation-profile-hardening, gpx-parser-robustness, hiking-time-estimate, poi-index, hiking-foot-profile - Endurain: account-export, activity-duplicate-review, fit-parsing-hardening, activity-locations, self-hosting-guide, activity-privacy-controls - wanderer: federation-hardening, link-share-tokens - credits-page (user-visible acknowledgments) Updated in-flight changes with wanderer prior-art sections: route-federation, route-discovery. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
parent
be4f7e4ae8
commit
5dd4968626
80 changed files with 2600 additions and 2 deletions
2
openspec/changes/gpx-parser-robustness/.openspec.yaml
Normal file
2
openspec/changes/gpx-parser-robustness/.openspec.yaml
Normal file
|
|
@ -0,0 +1,2 @@
|
|||
schema: spec-driven
|
||||
created: 2026-07-05
|
||||
70
openspec/changes/gpx-parser-robustness/design.md
Normal file
70
openspec/changes/gpx-parser-robustness/design.md
Normal file
|
|
@ -0,0 +1,70 @@
|
|||
## Context
|
||||
|
||||
`packages/gpx/src/parse.ts` parses GPX via DOM queries with no input hardening: `parseFloat(attr ?? "0")` turns missing coordinates into `0,0` and garbage into `NaN`; `NaN` passes the journal's range validation (`NaN < -90` is false) and flows into haversine distance, gain/loss totals (`loss += Math.abs(NaN)` → `NaN` persisted), and PostGIS geometry. `<rte>`/`rtept` is not parsed at all, so route-only exports fail `validateGpx`'s ≥2-track-points check. Timestamps are passed through as raw strings; `moving-time.ts` defends itself per interval, but partially-broken timestamp runs silently shrink moving time, and `startTime` derivation trusts the first point.
|
||||
|
||||
Organic Maps' parser (`serdes_gpx.cpp`) handles each of these with small, explicit policies backed by a corpus of real files from other apps (OsmAnd, Garmin, gpx.studio, OpenTracks). This design adapts those policies; it consciously skips OM behaviors trails doesn't need (track color namespaces, KML interop).
|
||||
|
||||
Consumers of the parser: journal save/import (`gpx-save.server.ts`), Komoot and Wahoo sync, planner GPX load, and all downstream stats (`elevationSeries`, `movingTime`, `computeDays`).
|
||||
|
||||
## Goals / Non-Goals
|
||||
|
||||
**Goals:**
|
||||
- A GPX file that any mainstream app exports imports correctly; a file with some broken points imports with the broken points discarded rather than corrupted stats or rejection.
|
||||
- Deterministic, documented repair policies (skip vs. interpolate vs. drop) pinned by a fixture corpus.
|
||||
- No shape change to `GpxData`; no behavior change for already-well-formed files (except `<cmt>`/track-name fallback enriching metadata).
|
||||
|
||||
**Non-Goals:**
|
||||
- Track color / display extensions (nothing in trails renders per-track color).
|
||||
- GPX 1.0, `<extensions>` beyond the existing `trails:` namespace, KML/FIT import.
|
||||
- Changing `validateGpx` thresholds or error taxonomy — it stays the loud backstop (`gpx-save` spec unchanged).
|
||||
- Fixing files with systematically wrong data (e.g. all-zero elevation) — garbage that parses cleanly is out of scope.
|
||||
|
||||
## Decisions
|
||||
|
||||
### 1. Skip invalid points at parse time; keep validation strict
|
||||
|
||||
Points whose `lat`/`lon` attribute is missing or does not parse to a finite number are skipped. Unparseable `<ele>` becomes `undefined` (the existing "no elevation" representation). Rationale: salvage the usable majority of a file (OM's approach — log and continue) instead of either rejecting the file or corrupting downstream math. `validateGpx` continues to reject files with <2 surviving points or out-of-range coordinates, so "everything was garbage" still fails loudly, satisfying the existing `gpx-save` requirements.
|
||||
|
||||
*Alternative:* throw on the first bad point — rejected; one bad point from a flaky recorder would block an otherwise fine 4-hour activity.
|
||||
|
||||
Numeric parsing stays `parseFloat`-lenient (accepts leading `+`, tolerates trailing junk like `471.0m`) but every result is gated by `Number.isFinite` before the point is accepted.
|
||||
|
||||
Segments left empty (or with a single point) after skipping are dropped, mirroring OM's ≤1-point-segment discard — a 1-point segment renders nothing and breaks distance math assumptions.
|
||||
|
||||
### 2. Parse `<rte>` as tracks
|
||||
|
||||
Each `<rte>` becomes one track segment appended after `<trk>` segments, with `rtept` handled identically to `trkpt` (same lenience, `ele`/`time` supported). Downstream code and specs already speak in "track points"; converting at the parser boundary (exactly OM's model — routes import as tracks) means zero consumer changes. Route-level `<name>`/`<desc>` participate in the metadata fallback (Decision 4).
|
||||
|
||||
### 3. Timestamp repair as a post-parse pass (OM policy)
|
||||
|
||||
New pure function (own module, e.g. `timestamp-repair.ts`) applied per segment after parsing:
|
||||
- Validity = `Date.parse` yields a finite value.
|
||||
- If >50% of a segment's timestamps are invalid → drop all timestamps in that segment (a mostly-broken time channel is noise; downstream treats it as an untimed track).
|
||||
- Otherwise → linearly interpolate invalid runs between their valid neighbors; leading/trailing invalid runs clamp to the nearest valid timestamp. Repaired values are written back as ISO strings.
|
||||
|
||||
This keeps `movingTime`, elapsed duration, and `startTime` meaningful for files from flaky recorders without any change to those functions. Monotonicity is not enforced here — `movingTime` already skips non-positive intervals, and reordering recorded data would be fabrication.
|
||||
|
||||
*Alternative:* validate timestamps lazily in each consumer — rejected; three consumers would each need the same policy and would inevitably drift.
|
||||
|
||||
### 4. Metadata fallback and `<cmt>` merge
|
||||
|
||||
`name`/`description` resolution order: `<metadata>` first, else the first `<trk>`/`<rte>`'s `<name>`/`<desc>`. A `<cmt>` on that track/route is appended to the description (blank-line separated) when present and not identical to it — OM's `BuildDescription` dedup rule. Low-risk enrichment; many apps put the only human-readable title on the `<trk>`.
|
||||
|
||||
### 5. Fixture corpus as files, not inline strings
|
||||
|
||||
New `packages/gpx/fixtures/*.gpx` — small, purpose-named files (`route-only.gpx`, `missing-coords.gpx`, `garbage-ele.gpx`, `timestamps-partial.gpx`, `timestamps-mostly-invalid.gpx`, `multi-track.gpx`, `unicode-name.gpx`, `namespaced-extensions.gpx`, plus a trimmed real-world export per integration we support — Komoot, Wahoo). Loaded by `parse-node.test.ts` via `fs`. Files are trivially addable when the next user bug report arrives — the corpus is the regression mechanism, which is the real lesson from OM's test suite. Inline-string tests stay for focused unit cases.
|
||||
|
||||
## Risks / Trade-offs
|
||||
|
||||
- [Silently skipping points can hide data loss from users] → Skipping is bounded by `validateGpx` (<2 survivors still rejects). If user-visible reporting is ever wanted, a `skippedPoints` count can be added to `GpxData` later without breaking shape.
|
||||
- [Interpolated timestamps are fabricated data] → Bounded by the >50% drop rule and segment locality; interpolation only fills gaps between real measurements, the same trade OM ships. Moving-time impact is small and strictly better than silently dropped intervals.
|
||||
- [`parseFloat` lenience accepts sloppy values like `48.1abc`] → Deliberate (matches OM and real-world files); `Number.isFinite` gate plus range validation still exclude everything harmful.
|
||||
- [Route-only files previously rejected now import — user-visible behavior change] → Intended; strictly permissive, no existing imports change meaning.
|
||||
|
||||
## Migration Plan
|
||||
|
||||
Pure library change in `@trails-cool/gpx`; both apps pick it up on deploy. No schema/API/config changes. Rollback = revert PR. No backfill: previously-rejected files simply import on retry.
|
||||
|
||||
## Open Questions
|
||||
|
||||
- None blocking. Whether to surface a "N unusable points skipped" notice in the import UI is a product question deferred until someone actually hits it.
|
||||
28
openspec/changes/gpx-parser-robustness/proposal.md
Normal file
28
openspec/changes/gpx-parser-robustness/proposal.md
Normal file
|
|
@ -0,0 +1,28 @@
|
|||
## Why
|
||||
|
||||
`@trails-cool/gpx`'s parser trusts its input: a `trkpt` with a missing `lat`/`lon` attribute silently becomes `0` (a Null Island point that passes journal validation), unparseable coordinates or `<ele>` produce `NaN` that survives the range check and poisons distance and gain/loss totals, and `<rte>`/`rtept` files (Garmin Connect courses and many planner exports) parse to zero track points and are rejected. Our review of Organic Maps' GPX parser (`libs/kml/serdes_gpx.cpp` and its fixture corpus, 2026-07-05) catalogued exactly these real-world failure modes plus a principled timestamp-repair policy; users importing files from other apps will keep hitting them until we harden the parser.
|
||||
|
||||
## What Changes
|
||||
|
||||
- **Invalid point handling**: track/route points with missing or unparseable `lat`/`lon` are skipped (not defaulted to 0, not propagated as `NaN`); unparseable `<ele>` is treated as absent instead of `NaN`.
|
||||
- **Route support**: `<rte>`/`rtept` sequences are parsed as tracks, so route-only GPX files import like track files.
|
||||
- **Timestamp repair** (Organic Maps policy): per segment, if more than half of the point timestamps are invalid, all timestamps in that segment are dropped; otherwise invalid timestamps are linearly interpolated between valid neighbors (leading/trailing runs clamp to the nearest valid value). Downstream moving time, elapsed duration, and `startTime` become reliable for files from flaky recorders.
|
||||
- **Description fallback**: track-level `<name>`/`<desc>`/`<cmt>` are used when `<metadata>` has none, and `<cmt>` is merged into the description when distinct.
|
||||
- **Real-world fixture corpus**: add a `fixtures/` set of small GPX files modeled on Organic Maps' test corpus (route-only, missing/garbage coordinates, mixed-validity timestamps, missing elevation, multi-track/multi-segment, namespaced extensions, Unicode names) wired into the parser test suite as regression cases.
|
||||
- Not in scope: track color extensions (journal doesn't render per-track colors), GPX 1.0 support, and changes to the journal's validation thresholds (`validateGpx` still requires ≥2 valid points and in-range coordinates).
|
||||
|
||||
## Capabilities
|
||||
|
||||
### New Capabilities
|
||||
- `gpx-parsing`: Behavior of the shared GPX parser — which elements are recognized (tracks, routes, waypoints, metadata), how invalid points/elevations/timestamps are repaired or discarded, and the regression fixture corpus that pins this behavior.
|
||||
|
||||
### Modified Capabilities
|
||||
|
||||
<!-- none — gpx-save's validation requirements (≥2 points, coordinate range, loud failure) are unchanged; they now operate on cleaner parser output. gpx-import's UI-level requirements are untouched. -->
|
||||
|
||||
## Impact
|
||||
|
||||
- `packages/gpx/src/parse.ts` — point/coordinate/elevation lenience, `<rte>` parsing, description fallback; new `timestamp-repair.ts` (or equivalent) for the repair pass.
|
||||
- `packages/gpx/fixtures/*.gpx` + expanded `parse.test.ts`/`parse-node.test.ts` — fixture corpus and regression tests.
|
||||
- Journal import/sync paths (`gpx-save.server.ts`, Komoot/Wahoo import) and planner GPX loading benefit without code changes; `GpxData` shape is unchanged.
|
||||
- Files previously rejected (route-only) or corrupted (NaN stats) now import correctly — behavior change is strictly permissive.
|
||||
|
|
@ -0,0 +1,70 @@
|
|||
## ADDED Requirements
|
||||
|
||||
### Requirement: Invalid track points are skipped, not defaulted
|
||||
The GPX parser SHALL skip track and route points whose `lat` or `lon` attribute is missing or does not parse to a finite number, and SHALL treat an unparseable `<ele>` value as absent elevation. Skipped or repaired points SHALL never surface as `0,0` coordinates or `NaN` values in parsed output, distance, or elevation totals. Segments left with fewer than two points after skipping SHALL be dropped.
|
||||
|
||||
#### Scenario: Point with missing coordinates is skipped
|
||||
- **WHEN** a track contains a `trkpt` without a `lat` attribute among otherwise valid points
|
||||
- **THEN** that point is absent from the parsed track and no `0,0` point appears
|
||||
|
||||
#### Scenario: Garbage coordinates do not poison stats
|
||||
- **WHEN** a `trkpt` has a non-numeric `lat` value
|
||||
- **THEN** the point is skipped and the computed distance and elevation totals are finite numbers
|
||||
|
||||
#### Scenario: Unparseable elevation treated as missing
|
||||
- **WHEN** a `trkpt` has `<ele>` content that does not parse to a finite number
|
||||
- **THEN** the point is kept with no elevation and gain/loss totals remain finite
|
||||
|
||||
#### Scenario: Degenerate segment dropped
|
||||
- **WHEN** a `trkseg` has only one valid point after skipping
|
||||
- **THEN** that segment does not appear in the parsed tracks
|
||||
|
||||
### Requirement: Route elements parse as tracks
|
||||
The GPX parser SHALL parse each `<rte>` element as a track segment, handling `rtept` identically to `trkpt` (including elevation, time, and invalid-point skipping), appended after any `<trk>` segments.
|
||||
|
||||
#### Scenario: Route-only GPX imports
|
||||
- **WHEN** a GPX file contains only a `<rte>` with valid `rtept` points and no `<trk>`
|
||||
- **THEN** the parsed data contains one track segment with those points
|
||||
- **AND** journal validation accepts the file
|
||||
|
||||
#### Scenario: Mixed tracks and routes
|
||||
- **WHEN** a GPX file contains both `<trk>` and `<rte>` elements
|
||||
- **THEN** all appear as track segments with track segments first
|
||||
|
||||
### Requirement: Timestamp repair
|
||||
The GPX parser SHALL repair point timestamps per segment: when more than half of a segment's timestamps are invalid, all timestamps in that segment SHALL be dropped; otherwise invalid timestamps SHALL be linearly interpolated between the nearest valid neighbors, with leading and trailing invalid runs set to the nearest valid timestamp. Repaired timestamps SHALL be valid ISO 8601 strings.
|
||||
|
||||
#### Scenario: Sparse invalid timestamps interpolated
|
||||
- **WHEN** a segment has a minority of unparseable `<time>` values between valid ones
|
||||
- **THEN** those points receive timestamps interpolated between the surrounding valid times
|
||||
|
||||
#### Scenario: Mostly-invalid timestamps dropped
|
||||
- **WHEN** more than half of a segment's `<time>` values are unparseable
|
||||
- **THEN** every point in that segment has no timestamp
|
||||
- **AND** moving time computation returns null for a track consisting only of such segments
|
||||
|
||||
#### Scenario: Leading invalid run clamps
|
||||
- **WHEN** the first points of a segment have invalid timestamps and later points are valid
|
||||
- **THEN** the leading points receive the first valid timestamp
|
||||
|
||||
### Requirement: Metadata fallback from tracks and routes
|
||||
The GPX parser SHALL fall back to the first track's or route's `<name>` and `<desc>` when `<metadata>` provides none, and SHALL merge a track-level `<cmt>` into the description (separated by a blank line) when it is present and differs from the description.
|
||||
|
||||
#### Scenario: Track name used when metadata absent
|
||||
- **WHEN** a GPX file has no `<metadata><name>` but its `<trk>` has a `<name>`
|
||||
- **THEN** the parsed name is the track's name
|
||||
|
||||
#### Scenario: Identical cmt not duplicated
|
||||
- **WHEN** a track's `<cmt>` equals its `<desc>`
|
||||
- **THEN** the description contains the text once
|
||||
|
||||
### Requirement: Regression fixture corpus
|
||||
The GPX package SHALL maintain a corpus of fixture GPX files exercising the repair policies (route-only, missing/garbage coordinates, unparseable elevation, partially and mostly invalid timestamps, multi-track/multi-segment, Unicode names, foreign-namespace extensions), and the parser test suite SHALL parse every fixture and assert the repaired output, so future parser changes cannot silently regress real-world compatibility.
|
||||
|
||||
#### Scenario: All fixtures parse under test
|
||||
- **WHEN** the parser test suite runs
|
||||
- **THEN** every fixture file is parsed and its expected repaired output is asserted
|
||||
|
||||
#### Scenario: Foreign namespaces ignored gracefully
|
||||
- **WHEN** a fixture contains extension elements from other apps' namespaces (e.g. Garmin, OsmAnd)
|
||||
- **THEN** parsing succeeds and trails-relevant data is extracted unchanged
|
||||
33
openspec/changes/gpx-parser-robustness/tasks.md
Normal file
33
openspec/changes/gpx-parser-robustness/tasks.md
Normal file
|
|
@ -0,0 +1,33 @@
|
|||
## 1. Parser lenience
|
||||
|
||||
- [ ] 1.1 Add a finite-number coordinate gate in `packages/gpx/src/parse.ts`: skip `trkpt`/`rtept` with missing or non-finite `lat`/`lon`; treat non-finite `<ele>` as undefined
|
||||
- [ ] 1.2 Drop segments with fewer than 2 surviving points
|
||||
- [ ] 1.3 Unit tests: missing-lat point skipped, garbage lat skipped with finite stats, garbage ele → undefined with finite gain/loss, single-point segment dropped, well-formed files byte-identical output to before
|
||||
|
||||
## 2. Route (`<rte>`) support
|
||||
|
||||
- [ ] 2.1 Parse `<rte>`/`rtept` as track segments (appended after `<trk>` segments), reusing the same point parsing and lenience
|
||||
- [ ] 2.2 Unit tests: route-only file yields one segment and passes `validateGpx`; mixed trk+rte ordering; rtept with ele/time preserved
|
||||
|
||||
## 3. Timestamp repair
|
||||
|
||||
- [ ] 3.1 Create `packages/gpx/src/timestamp-repair.ts`: per-segment pass — >50% invalid → drop all; else linear interpolation between valid neighbors with leading/trailing clamp; output ISO 8601 strings
|
||||
- [ ] 3.2 Wire the repair pass into `parseTracks` output in `parse.ts`
|
||||
- [ ] 3.3 Unit tests (`timestamp-repair.test.ts`): sparse invalid interpolated, >50% invalid dropped (and `movingTime` returns null), leading/trailing clamp, all-valid untouched, no-timestamps untouched
|
||||
|
||||
## 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
|
||||
|
||||
## 5. Fixture corpus
|
||||
|
||||
- [ ] 5.1 Create `packages/gpx/fixtures/` with purpose-named files: `route-only.gpx`, `missing-coords.gpx`, `garbage-ele.gpx`, `timestamps-partial.gpx`, `timestamps-mostly-invalid.gpx`, `multi-track.gpx`, `unicode-name.gpx`, `namespaced-extensions.gpx` (Garmin/OsmAnd-style extensions)
|
||||
- [ ] 5.2 Add trimmed real-world exports for supported integrations (Komoot, Wahoo) with any personal data scrubbed
|
||||
- [ ] 5.3 Extend `parse-node.test.ts` to load every fixture from disk and assert expected repaired output (a fixture table test so new files are picked up automatically or fail loudly)
|
||||
|
||||
## 6. Verification
|
||||
|
||||
- [ ] 6.1 Typecheck + run full gpx package suite; confirm `GpxData` shape unchanged and journal/planner compile without changes
|
||||
- [ ] 6.2 Manual check: import a route-only GPX and a partially-broken GPX through the journal import flow; confirm both save with sane stats
|
||||
- [ ] 6.3 Run `pnpm typecheck && pnpm lint && pnpm test` and the e2e suites
|
||||
Loading…
Add table
Add a link
Reference in a new issue