diff --git a/docs/architecture.md b/docs/architecture.md index c264615..5be933a 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -22,7 +22,8 @@ These principles inform key architectural and product decisions: visualizations and tools around their data. - **Open source (MIT)**: The code is MIT licensed — both because of the commitment to the open web, and out of respect for the open-source projects - that inspire trails.cool (BRouter, bikerouter.de, brouter-web). + that inspire trails.cool (BRouter, bikerouter.de, brouter-web, Organic Maps, + Endurain, wanderer). - **AI-assisted development**: AI (Claude Code) and spec-driven development (OpenSpec) are core to how this project is built. Human contributions are very welcome. This is also an experiment in how far AI-assisted development diff --git a/docs/inspirations.md b/docs/inspirations.md new file mode 100644 index 0000000..0069c6e --- /dev/null +++ b/docs/inspirations.md @@ -0,0 +1,240 @@ +# Inspirations & prior-art research + +Durable record of the open-source projects we've studied, what we learned, +and which OpenSpec changes each insight spawned. This is the source of truth +for acknowledgments: the **credit line** under each project is the canonical +one-liner reused by `docs/philosophy.md` (Open Source section) and the +`/about/credits` page (see `openspec/changes/credits-page/`). Update all +three together when adding a project. + +Research method: shallow-clone the repo, fan out parallel exploration agents +over focused areas (data model, integrations, product, ops, community), then +synthesize trails-relevant learnings and turn the actionable ones into +OpenSpec proposals. Clones live in ephemeral scratch space; this document is +what survives. + +--- + +## BRouter, bikerouter.de, brouter-web (founding inspirations) + +> **Credit line:** BRouter — the routing engine · bikerouter.de — inspiration +> for the Planner · brouter-web — the web client + +The projects that made the Planner conceivable. BRouter is also a runtime +dependency (self-hosted routing engine, `docker/brouter/`). Not "researched" +in the sense below — they're in the product's DNA. + +--- + +## Organic Maps — reviewed 2026-07-05 + +> **Credit line:** Organic Maps — inspiration for outdoor-focused map +> presentation, elevation handling, hiking-time estimation (Tobler), GPX +> robustness, and privacy-as-proof practices + +Offline, privacy-absolutist OSM app (C++, [github.com/organicmaps/organicmaps](https://github.com/organicmaps/organicmaps)), +~10 years of map-domain experience inherited from MAPS.ME. Different +architecture (native offline vs web/federated), but it has solved many +problems on our roadmap. + +### Key learnings (with their source locations) + +- **Elevation profile hardening** (`libs/map/elevation_info.cpp`): + opposite-sign slope-outlier despiking (single-point spikes interpolated + out; monotonic climbs never touched), dual raw/threshold-filtered + ascent totals (hysteresis accumulation), Douglas-Peucker profile + simplification. Their Easy/Medium/Hard track difficulty rating is an + unimplemented `@todo` — a feature we could ship first. +- **Hiking ETA** (`libs/routing/edge_estimator.cpp`): Tobler's hiking + function for pedestrian speed-by-slope, downhill penalized at 0.35× of + uphill, altitude penalty above 2500 m. Also the weight-speed vs ETA-speed + split: route *choice* uses inflated speeds for nice paths; time estimates + use realistic ones. +- **GPX robustness** (`libs/kml/serdes_gpx.cpp` + fixture corpus in + `data/test_data/gpx/`): timestamp repair (>50% invalid per segment → + drop all; else linear interpolation), leading `+` in coordinates, + `` imported as tracks, ``/desc merging with dedup, + triple-namespace track-color export (Garmin gpxx + gpx_style + osmand) + for interop. Their fixture corpus is real files from OsmAnd, gpx.studio, + OpenTracks, Garmin. +- **Outdoor map presentation** (`data/styles/outdoors/`, + `generator/osm2type.cpp`): a separate Outdoors style that promotes peaks, + saddles, springs, drinking water to low zooms; `sac_scale` + + `trail_visibility` collapsed into just two path grades (difficult/expert) + for rendering — two values, big safety payoff. OSM route-relation + membership shown on a path's page ("E5, GR5"). +- **Own the data pipeline** (`tools/python/maps_generator/`, + `generator/`): one batch pipeline from planet file to per-region packs + carrying rendering + search + routing + elevation; precompute per-segment + elevation at build time instead of querying a DEM live; `.poly`-file + region splitting; category-as-pseudo-language search-trie trick (category + IDs indexed as a fake language alongside names). +- **Practices**: privacy proven by third-party audit evidence published + in-repo (Exodus reports) rather than asserted; opt-in email-with-log + diagnostics instead of telemetry; excellent `docs/PR_GUIDE.md` (small-PR + norm, LLM-use disclosure); anti-roadmap stance (milestones over a + ROADMAP file); root CLAUDE.md with AGENTS.md symlink + nested per-domain + CLAUDE.md files; GPS track-recording filter (`libs/map/gps_track_filter.cpp`: + accuracy >250 m rejected, acceleration >2 m/s², bearing-flip rejection, + file-backed ring buffer) — reference for `docs/ideas/mobile-activity-recording`. + +### Changes spawned + +`elevation-profile-hardening`, `gpx-parser-robustness`, +`hiking-time-estimate`, `poi-index` (pipeline pattern + evidence), +`hiking-foot-profile` (the review exposed the trekking-labeled-"Hiking" +mismatch). + +### Explicitly not pursued + +Deep-link codec (session URLs cover it), track-color export (nothing +renders colors), MWM/offline formats (wrong architecture for web). + +--- + +## Endurain — reviewed 2026-07-06 + +> **Credit line:** Endurain — inspiration for the Journal's activity privacy +> controls, account export, duplicate handling, and self-hosting operator +> experience + +Self-hosted Strava alternative ([codeberg.org/endurain-project/endurain](https://codeberg.org/endurain-project/endurain); +FastAPI + Vue, AGPL, single maintainer). Overlaps the Journal only; its +federation is frontend scaffolding — trails is ahead there. + +### Key learnings (with their source locations) + +- **Granular privacy** (`users/users_privacy_settings/models.py`, + `activities/activity/models.py`, `apply_visibility_mask()`): ~13 + independent per-signal hide flags (start time, location, map, HR, power, + pace, laps, gear…), per-user defaults stamped at import, per-activity + overrides, one server-side mask, one frontend gate component. +- **Account export/import** (`users/users_profile/export_service.py`): + streaming, memory-bounded ZIP of everything including original uploaded + FIT/GPX files; matching import with ID remapping. +- **Cross-source duplicate handling** (`activities/activity/crud.py`): + same-start-time match → store anyway, flag `is_hidden`, notify for manual + review. Flag-and-review over silent auto-merge. +- **FIT parsing quirks catalog** (`activity_file_import/utils_fit.py`, + 1,147 lines of fitdecode handling): multi-session files → sliced + per-session records, semicircle→degrees, timezone via TimezoneFinder + + device-offset fallback for indoor, zero-HR sensor-dropout exclusion, + `enhanced_*` fields preferred, cursor resets across pauses, + `timestamp_16` rollover. Tests: `backend/tests/.../test_utils_fit.py`. +- **Elevation smoothing convergence** (`compute_elevation_gain_and_loss`): + median(6) → moving-average(3) → 0.1 m threshold — independent + confirmation that naive delta-summing overstates ascent. +- **Gear tracking** (`gears/`): types + retirement + mileage offset, + components/consumables with `expected_kms` wear thresholds, time-based + wear for racquets, default gear per activity type. +- **Operator experience** (`docs/getting-started/advanced-started.md`, + three compose variants incl. Docker-secrets): ~45-variable documented env + table; BYO-OAuth-credentials pattern (each self-hoster registers their + own provider app). Auth stack is mature (refresh rotation, TOTP, OIDC, + API keys, SSRF allowlists). +- **Contrasts that validate trails' choices**: unofficial password-based + Garmin login with unlink-on-expiry (vs our official-API path), polling + without webhooks (vs our Wahoo webhooks), no health endpoint/metrics, no + changelog, offset pagination. `TRADEMARK.md` (AGPL code + registered + word mark restricting commercial hosting) is an interesting governance + read. + +### Changes spawned + +`account-export`, `activity-duplicate-review`, `fit-parsing-hardening`, +`activity-locations` (their feature, our privacy-preserving offline +redesign), `self-hosting-guide`, `activity-privacy-controls`. + +--- + +## wanderer — reviewed 2026-07-06 + +> **Credit line:** wanderer — a self-hosted, federated trail database; the +> closest neighbor to the Journal's federated route sharing and proof the +> ActivityPub-for-trails idea works + +Self-hosted federated trail database ([wanderer.to](https://wanderer.to), +[github.com/Flomp/wanderer](https://github.com/Flomp/wanderer); AGPL, +Go/PocketBase + SvelteKit + Meilisearch + WASM provider plugins). Same +route-vs-activity split as trails ("trails" + "summit logs"). The only +studied project that has shipped ActivityPub for trail content. + +### Key learnings (with their source locations) + +- **Federation is real and Mastodon-interops** (`db/federation/*.go`, + `db/util/activitypub.go`, protocol doc + `docs/src/content/docs/develop/federation.md` with full JSON examples): + per-user RSA keys, signed fetch ("authorized fetch"), WebFinger, + Follow/auto-Accept, Likes, comments as `Note` + `inReplyTo`, summit logs + federate as first-class objects. **Object model: everything is a plain + `Note`** — metrics ride in bespoke `tag[]` note entries, GPX as a + `Document` attachment (3-photo cap, lossy unit encoding). Interop with + trails' Fedify journal is feasible at the social layer; exchanging + actual route data would need a wanderer-specific adapter. Their + Note-with-attachments shape *is* why trails render in Mastodon — worth + considering a Note-compatible representation alongside structured + objects. +- **Federation weaknesses to avoid**: fire-and-forget delivery (detached + goroutine, semaphore(5), no queue/retry — activities lost on transient + failures), no inbound dedup/replay defense, no moderation or + instance-blocking at all, no shared instance actor, signature + verification coupled to `X-Forwarded-Path` proxy headers. Their SSRF-safe + outbound HTTP client (`db/util/network.go`: private-IP blocking, + 30 req/min/origin) is worth copying. +- **Discovery at scale** (`db/util/meilisearch.go`, + `db/routes/search_token.go`, `web/src/routes/api/v1/search/trails/cluster/`): + Meilisearch with `_geo` + precomputed bbox scalars (antimeridian-aware + viewport filters); **access control enforced at the index layer via + tenant tokens** (anonymous → `public=true`; authed → public OR author OR + shared-with, minted per session); server-side `supercluster` clustering + where the largest-bbox trails render as polylines and the rest cluster; + random-offset sampling for recommendations; remote/federated trails + indexed as local stubs so cross-instance content is searchable. +- **Trail data model** (`db/migrations/1747064968_collections_snapshot.go`, + `web/src/lib/models/`): polyline + bbox computed server-side on GPX + change; stats recomputed from files, never trusted; dual-threshold + elevation filter (5 m horizontal anchor / 5 m vertical, raw + smoothed + kept — third independent confirmation that naive ascent summing is + wrong); **optional DEM elevation correction via self-hosted Valhalla + height service** (`gpx.correctElevation()`) — an idea trails hasn't + considered; all formats (FIT/TCX/KML/KMZ) normalized to GPX at import. +- **Product features**: ordered trail lists with per-actor view/edit + shares; **link-share tokens** (view/edit) for private content; + 9 notification types with **per-type web/email preferences**; + **print view with map, elevation profile, scale bar, and QR code** + (jsPDF) — paper backup for hikes; collection export as ZIP (GPX or JSON, + photos + summit logs); hashed long-lived **API tokens** for third + parties; sandboxed **WASM provider plugins** (Strava/Komoot/Hammerhead — + plugins can't open sockets; a host executor injects auth and enforces + policy; `plugins/README.md` is an excellent architecture doc). +- **Ops practices**: sectioned semver CHANGELOG with GHSA/PR references; + boot-time hard failure on missing/default encryption keys; search index + treated as rebuildable cache and excluded from backups; explicit + AI-assisted-contribution policy in CONTRIBUTING. Cautionary tales: + real default secrets committed in `docker-compose.yml`, no + reverse-proxy/TLS setup docs, backup restore only supported within the + same minor version. + +### Changes spawned + +`federation-hardening` (durable Fedify queue replacing the in-process one, +replay dedup, instance blocklist, FEDERATION.md), `link-share-tokens` +(revocable tokenized view links for private content). + +### Changes updated with these findings + +`route-federation` (prior-art section: Note-model contrast, interop +verdict, dual-representation consideration, hardening dependency), +`route-discovery` (prior-art section: query-layer access control, +clustering/recommendation patterns), `self-hosting-guide` (no default +secrets + boot fail-fast, rebuildable-state backup guidance, +restore-version rule). Not yet acted on: per-type notification channel +preferences (future `notifications` delta), print view with QR code, +hashed API tokens, Valhalla-style DEM elevation correction. + +### Explicitly not pursued + +PocketBase/SQLite single-binary direction (Postgres+PostGIS gives native +geo without a mandatory search sidecar); WASM plugin system (in-app +providers are simpler at trails' provider count); Meilisearch itself +(PostGIS + SQL covers trails' discovery scale for now). diff --git a/docs/philosophy.md b/docs/philosophy.md index 04f5806..bf9d49e 100644 --- a/docs/philosophy.md +++ b/docs/philosophy.md @@ -37,11 +37,22 @@ trails.cool is MIT licensed. The entire codebase — Planner, Journal, shared packages, infrastructure — is open source. This is both a commitment to the open web and an acknowledgment of the -projects that made trails.cool possible: +projects that made trails.cool possible (what we learned from each is +recorded in [docs/inspirations.md](inspirations.md); keep the two lists in +sync): - [BRouter](https://github.com/abrensch/brouter) — the routing engine - [bikerouter.de](https://bikerouter.de) — inspiration for the Planner - [brouter-web](https://github.com/nrenner/brouter-web) — the web client +- [Organic Maps](https://github.com/organicmaps/organicmaps) — inspiration for + outdoor-focused map presentation, elevation handling, hiking-time estimation + (Tobler), GPX robustness, and privacy-as-proof practices +- [Endurain](https://codeberg.org/endurain-project/endurain) — inspiration for + the Journal's activity privacy controls, account export, duplicate handling, + and self-hosting operator experience +- [wanderer](https://wanderer.to) — a self-hosted, federated trail database; + the closest neighbor to the Journal's federated route sharing and proof the + ActivityPub-for-trails idea works We benefit from open source. We contribute back to it. diff --git a/openspec/changes/account-export/.openspec.yaml b/openspec/changes/account-export/.openspec.yaml new file mode 100644 index 0000000..dd9a1d9 --- /dev/null +++ b/openspec/changes/account-export/.openspec.yaml @@ -0,0 +1,2 @@ +schema: spec-driven +created: 2026-07-06 diff --git a/openspec/changes/account-export/design.md b/openspec/changes/account-export/design.md new file mode 100644 index 0000000..4e2e4de --- /dev/null +++ b/openspec/changes/account-export/design.md @@ -0,0 +1,62 @@ +## Context + +Journal data per user (schema `packages/db/src/schema/journal.ts`): `routes` (GPX text + PostGIS geom + metadata + `route_versions` history), `activities` (GPX text + geom + stats + `photos` paths), `follows`, `notifications`, `connected_services` (encrypted tokens), `sync_imports`/`sync_pushes`, profile fields on `users`. GPX blobs are stored in-DB as text; photos on S3-compatible storage (Garage). Background jobs run via pg-boss (`@trails-cool/jobs`); notifications have an established row + rendering pattern. + +Endurain's reference implementation (`export_service.py`) streams a ZIP of JSON dumps plus original uploaded files, memory-bounded, with a matching import service. + +## Goals / Non-Goals + +**Goals:** +- One action → one ZIP with everything the user owns, GPX as the primary format, JSON only for what GPX can't express. +- Bounded memory regardless of account size; no request-timeout coupling (async job). +- A versioned, documented archive layout a future import can rely on. + +**Non-Goals:** +- Import/restore (follow-up change; the manifest version field is its hook). +- Selective/partial export, scheduled exports, or export of other users' content beyond follow handles. +- GDPR legal workflows (this is the technical capability; policy text is separate). + +## Decisions + +### 1. Archive layout: GPX files first, one manifest for the rest + +``` +export/ + manifest.json — format version, generated_at, user profile, counts + routes/.gpx — current version + routes/.versions/.gpx + routes.json — per-route metadata (visibility, tags, day breaks, timestamps) + activities/.gpx + activities.json — stats, sport type, visibility, route link, provider provenance + media// + follows.json, notifications.json, connections.json (names + provider only) +``` +Open formats carry the payload; JSON carries relations and metadata keyed by the same ids as the filenames. `manifest.json.formatVersion = 1` is the import contract. + +### 2. Async job + temporary artifact, not a streaming response + +Export runs as a pg-boss job that streams entities from the DB in batches into a ZIP written to the media store (Garage) under an `exports/` prefix with a 7-day expiry; completion fires a notification with a signed, owner-only download URL served through the app (no public bucket URLs). Rationale: accounts with hundreds of GPX blobs exceed sane request lifetimes, and a resumable artifact beats re-generating on a dropped connection. + +*Alternative:* synchronous streaming HTTP response — simpler, but ties memory/time to the request and breaks on large accounts; rejected. + +### 3. Job bookkeeping in a small table + +`export_jobs` (id, user_id, status, artifact_key, error, created_at, expires_at). One active job per user (re-request returns the running job). A daily sweep (existing jobs worker) deletes expired artifacts and rows. Rationale over reusing pg-boss state: the download link must outlive job retention and be queryable by the settings page. + +### 4. Exclusions are structural, not filtered + +The export queries enumerate included columns explicitly (allowlist per table) rather than dumping rows and deleting secret fields — a new secret column added later can never leak by default. Connected services export as `{provider, connectedAt}` only. + +## Risks / Trade-offs + +- [Large accounts → big artifacts on Garage] → 7-day expiry + one-active-job cap; size is bounded by the user's own data. +- [Export contains private content in a downloadable file] → owner-only auth on the download route + unguessable artifact key + expiry; the notification link goes through app auth, not a raw bucket URL. +- [Format drifts before import lands] → `formatVersion` + a `docs/export-format.md` page written as part of this change; import will validate the version. + +## Migration Plan + +Additive: new table (`db:push`/migration), new job, new routes, new settings UI. No changes to existing data. Rollback = revert; sweep removes artifacts. + +## Open Questions + +- None blocking. Whether the archive should also include a ready-to-serve ActivityPub outbox snapshot (for migration to another instance) is deferred to the import change. diff --git a/openspec/changes/account-export/proposal.md b/openspec/changes/account-export/proposal.md new file mode 100644 index 0000000..c1d3ecc --- /dev/null +++ b/openspec/changes/account-export/proposal.md @@ -0,0 +1,27 @@ +## Why + +"All user data must be exportable in open formats" is a stated trails principle, but today it only exists as per-route/per-activity GPX downloads. There is no way to take *everything* with you — which a federated, self-hostable platform owes its users both as a data-ownership promise and as the practical instance-migration story. Endurain (reviewed 2026-07-06) ships a streaming full-account ZIP export; trails should match it with an open-format equivalent. + +## What Changes + +- New **"Export my data"** action in account settings: produces a ZIP containing every route and activity as its original GPX file (open format first), plus JSON manifests for entities GPX can't carry — profile, route/activity metadata (visibility, tags, sport type, day breaks), route version history, follows, connected-service *names* (never tokens), and notification history. +- Export is generated **streamed** (bounded memory) as a background job with a notification + download link when ready; the artifact is stored temporarily and expires. +- Media (activity photos) included when present. +- Explicitly excluded: OAuth tokens/credentials, sessions, other users' data (followers' profiles beyond handle), federation-internal state. +- Not in scope: **import/restore** of the archive (instance migration) — the export format is versioned and documented so a future import change can consume it. + +## Capabilities + +### New Capabilities +- `account-export`: The full-account export — contents, format guarantees (GPX originals + versioned JSON manifest), generation lifecycle (async job, expiry), and access control. + +### Modified Capabilities + + + +## Impact + +- `apps/journal`: new settings surface (`/settings/account`), export job in `@trails-cool/jobs` (pg-boss), streaming ZIP assembly, temporary artifact storage (S3/Garage or local disk with expiry), download route with owner-only auth. +- `packages/db`: small `export_jobs` table (or reuse job state) for status + artifact pointer + expiry. +- Notifications: reuses existing notification pattern for "your export is ready". +- Privacy manifest: document what the export contains (it's the user's own data leaving via the user's own request). diff --git a/openspec/changes/account-export/specs/account-export/spec.md b/openspec/changes/account-export/specs/account-export/spec.md new file mode 100644 index 0000000..09733d1 --- /dev/null +++ b/openspec/changes/account-export/specs/account-export/spec.md @@ -0,0 +1,41 @@ +## ADDED Requirements + +### Requirement: Full-account export archive +The Journal SHALL let a user export all data they own as a single ZIP archive: every route (current GPX plus version-history GPX files), every activity (GPX when present), activity media, and JSON manifests for profile, route/activity metadata, follows, notification history, and connected-service names. The archive SHALL carry a `manifest.json` with a format version. OAuth tokens, credentials, sessions, and other users' content SHALL NOT be included. + +#### Scenario: Archive contains originals in open formats +- **WHEN** a user with routes and activities requests an export +- **THEN** the archive contains one GPX file per route and per activity with GPX, and JSON manifests keyed by the same ids + +#### Scenario: Secrets never exported +- **WHEN** a user with a connected Wahoo account exports +- **THEN** the archive lists the connection's provider and date only, with no tokens or credentials + +### Requirement: Asynchronous generation with bounded memory +Export generation SHALL run as a background job that streams data in batches (memory use independent of account size), notify the user when the archive is ready, and allow only one active export job per user. + +#### Scenario: Export of a large account completes +- **WHEN** a user with thousands of activities requests an export +- **THEN** the job completes without exhausting memory and a "export ready" notification links to the download + +#### Scenario: Duplicate request returns the running job +- **WHEN** a user requests an export while one is already generating +- **THEN** no second job starts and the settings page shows the in-progress state + +### Requirement: Owner-only, expiring download +The export artifact SHALL be downloadable only by the authenticated owner through an app route, and SHALL expire (deleted from storage) after 7 days. + +#### Scenario: Another user cannot download +- **WHEN** a different authenticated user requests the export download URL +- **THEN** the request is rejected + +#### Scenario: Expired artifact is gone +- **WHEN** 7 days pass after generation +- **THEN** the artifact is removed from storage and the settings page offers a fresh export + +### Requirement: Documented archive format +The archive layout and manifest schema SHALL be documented in the repository (`docs/export-format.md`) and versioned via `manifest.formatVersion`, so a future import capability can consume archives across versions. + +#### Scenario: Format documented +- **WHEN** the export capability ships +- **THEN** `docs/export-format.md` describes every file and manifest field for format version 1 diff --git a/openspec/changes/account-export/tasks.md b/openspec/changes/account-export/tasks.md new file mode 100644 index 0000000..ac5dd1e --- /dev/null +++ b/openspec/changes/account-export/tasks.md @@ -0,0 +1,26 @@ +## 1. Schema & job plumbing + +- [ ] 1.1 Add `export_jobs` table to `packages/db` (id, user_id FK cascade, status, artifact_key, error, created_at, expires_at) + migration +- [ ] 1.2 Add `account-export` job type to `@trails-cool/jobs` and a daily sweep job that deletes expired artifacts + rows + +## 2. Export generation + +- [ ] 2.1 Implement the export assembler: batched queries per table (explicit column allowlists), streaming ZIP writer, layout per design (`routes/`, `activities/`, `media/`, `*.json`, `manifest.json` with formatVersion 1) +- [ ] 2.2 Stream media files from Garage into the archive; tolerate missing media gracefully +- [ ] 2.3 Upload artifact to storage under `exports/` with expiry metadata; mark job complete; fire "export ready" notification (existing notification pattern) +- [ ] 2.4 Unit tests: allowlists exclude token columns; manifest counts match; a synthetic large dataset stays within a memory budget; one-active-job constraint + +## 3. UI & download + +- [ ] 3.1 Settings surface: "Export my data" with request button, in-progress state, ready-with-expiry state (i18n en/de) +- [ ] 3.2 Owner-only download route streaming the artifact from storage; reject non-owners; 404 after expiry +- [ ] 3.3 E2E: request export on a seeded account → notification → download → assert archive contents (GPX count, manifest, no token fields) + +## 4. Docs + +- [ ] 4.1 Write `docs/export-format.md` (format version 1: layout + manifest schema) +- [ ] 4.2 Update privacy documentation: export exists, what it contains, expiry + +## 5. Verification + +- [ ] 5.1 Run `pnpm typecheck && pnpm lint && pnpm test && pnpm test:e2e` diff --git a/openspec/changes/activity-duplicate-review/.openspec.yaml b/openspec/changes/activity-duplicate-review/.openspec.yaml new file mode 100644 index 0000000..dd9a1d9 --- /dev/null +++ b/openspec/changes/activity-duplicate-review/.openspec.yaml @@ -0,0 +1,2 @@ +schema: spec-driven +created: 2026-07-06 diff --git a/openspec/changes/activity-duplicate-review/design.md b/openspec/changes/activity-duplicate-review/design.md new file mode 100644 index 0000000..afb82c6 --- /dev/null +++ b/openspec/changes/activity-duplicate-review/design.md @@ -0,0 +1,55 @@ +## Context + +Activity creation happens through several paths — Wahoo webhook/manual import, Komoot import, manual GPX upload, and soon Garmin backfill — all converging on the gpx-save layer. Per-provider dedup (`sync_imports` unique on provider + external id) prevents same-source re-imports; `import_batches` counts those. Nothing compares *across* sources. Activities carry `startedAt` (nullable — planned-route-style uploads may lack it) and `duration`. Feeds/stats/federation currently filter by `visibility` and `synthetic`. + +Endurain's reference: exact same-start-time match → store with `is_hidden=true` + notification for manual review. We adopt the pattern but widen the match window (devices disagree by seconds) and tighten the quarantine semantics. + +## Goals / Non-Goals + +**Goals:** +- The same physical activity arriving twice never double-counts in stats or appears twice to followers — without silently discarding data the user may want. +- One detection point, one exclusion predicate — impossible for a new surface to forget the filter accidentally... (see risk). +- Resolution is a two-tap user decision. + +**Non-Goals:** +- Merging (picking best-of fields from both copies). +- Geometry-similarity matching (same route ≠ same activity; time overlap is the honest signal). +- Retroactive dedup of pre-existing rows (could be a later one-shot job). +- Route dedup. + +## Decisions + +### 1. Heuristic: time overlap, not exact equality + +Suspect when same owner AND both have `startedAt` AND |Δstart| ≤ 120 s AND (if both have durations) the intervals overlap ≥ 70%. Rationale: Garmin and Wahoo timestamps for one ride differ by GPS-fix seconds; exact equality (Endurain) misses those, while a bare ±2 min window without overlap check would false-positive back-to-back track repeats. Activities without `startedAt` are never suspected (nothing to compare). + +*Alternative:* geometry proximity — rejected as both expensive and wrong (riding the same loop twice is two activities). + +### 2. Quarantine = a self-referential FK, filtered everywhere via one predicate + +`activities.suspected_duplicate_of_id` references the *earlier* activity. A shared query helper (e.g. `visibleActivities()` scope used by feed/listings/stats/outbox) gains `AND suspected_duplicate_of_id IS NULL`. The activity detail page remains reachable by the owner (that's the review surface); non-owners get 404 as if private. Federation: the Create is simply not emitted while quarantined; on "keep both", normal publish fires then. + +*Alternative:* reuse `visibility='private'` + a reason column — rejected: it would destroy the user's chosen visibility (which must survive "keep both") and overload a user-facing concept with system state. + +### 3. Resolution actions + +Banner on the activity page (and notification deep-link): shows both copies' source (provider from `sync_imports`/upload), start time, duration, distance. **Keep both** → clear the FK; activity publishes with its original visibility (federation Create fires now). **Discard** → hard-delete the flagged copy (its `sync_imports` row remains, so the provider won't re-import it). The *original* activity is never touched. + +### 4. Detection lives in the shared save path + +The check runs inside the same transaction as activity insert for import-shaped creations (provider imports, uploads). Planner-saved activities don't exist (planner saves routes), so no exclusions needed. Import batches increment a new `suspectedDuplicateCount`. + +## Risks / Trade-offs + +- [A forgotten surface still shows quarantined rows] → The exclusion lives in the shared activity-listing helper; the task list includes an audit of every query site + a regression test that a quarantined activity is absent from feed, profile, stats, API list, and outbox. +- [False positives annoy users (e.g. two people's devices on one account)] → Both copies survive; resolution is one tap; the window is deliberately tight. +- [Race: two imports in parallel create both copies with neither flagged] → Detection query runs `FOR UPDATE` on the owner's recent-activity window inside the insert transaction; worst case (different processes) is caught by whichever commits second. +- [Nullable startedAt uploads bypass detection] → Accepted; without a timestamp there is no honest signal, and manual uploads are user-intentional. + +## Migration Plan + +Additive column + index (`owner_id, started_at` index already exists). Ship detection + exclusion + UI together. Rollback = revert; quarantined rows become visible again (harmless). + +## Open Questions + +- None blocking. A retroactive scan job for existing duplicates can be a follow-up once the review UI exists. diff --git a/openspec/changes/activity-duplicate-review/proposal.md b/openspec/changes/activity-duplicate-review/proposal.md new file mode 100644 index 0000000..99da64d --- /dev/null +++ b/openspec/changes/activity-duplicate-review/proposal.md @@ -0,0 +1,27 @@ +## Why + +Dedup today is per-provider only: `sync_imports` keys on `(provider, externalWorkoutId)`, so re-delivery from the *same* provider is caught. But the same real-world ride can arrive from two sources — Wahoo webhook + Garmin backfill (pending), Komoot import + manual GPX upload — and nothing catches that: the user gets two activities, double distance in weekly stats, and a duplicated feed entry for followers. With Garmin and COROS on the roadmap this will become routine. Endurain's pattern (reviewed 2026-07-06): store the incoming activity anyway, flag it as a suspected duplicate, hide it from all surfaces, and let the user resolve — no silent auto-merge, no data loss. + +## What Changes + +- On every activity creation from an import path (provider sync, manual upload, Komoot), a **cross-source duplicate check** runs: same owner + overlapping time window (`started_at` within ±2 minutes and durations overlapping ≥ 70%, when both have times). +- A suspected duplicate is stored with `suspectedDuplicateOfId` set and is **excluded from feeds, listings, profile stats, federation, and follower notifications** until resolved. +- The owner gets a "possible duplicate import" notification and a **review UI** showing both activities side by side (source, time, distance, map) with two actions: **keep both** (clears the flag, activity becomes normal) or **discard** (deletes the flagged copy). +- Import batches report suspected duplicates distinctly from exact-ID duplicates (`import_batches.duplicateCount` counts exact; a new counter for suspected). +- Not in scope: auto-merging data from two sources into one activity, retroactive scanning of existing activities, and route (non-activity) dedup. + +## Capabilities + +### New Capabilities +- `duplicate-detection`: Cross-source duplicate detection on activity import — the heuristic, the quarantine semantics (hidden-from-everything until resolved), the notification, and the resolve flow. + +### Modified Capabilities + + + +## Impact + +- `packages/db`: `activities.suspected_duplicate_of_id` (nullable text, FK activities, on delete set null); new counter column on `import_batches`. +- All import call sites converge on the shared save path (`gpx-save.server.ts` / activity creation helper) — the check lives once, there. +- Query-layer exclusion: feed, listings, profile stats, weekly distance, federation outbox, follower notifications must filter `suspected_duplicate_of_id IS NULL` (single shared predicate). +- New review UI (activity page banner + notification deep link), notification type, i18n strings. diff --git a/openspec/changes/activity-duplicate-review/specs/duplicate-detection/spec.md b/openspec/changes/activity-duplicate-review/specs/duplicate-detection/spec.md new file mode 100644 index 0000000..6fe73d6 --- /dev/null +++ b/openspec/changes/activity-duplicate-review/specs/duplicate-detection/spec.md @@ -0,0 +1,41 @@ +## ADDED Requirements + +### Requirement: Cross-source duplicate detection on import +When an activity is created via any import path (provider sync, manual GPX upload, Komoot), the Journal SHALL check for an existing activity of the same owner whose start time is within 120 seconds and, when both durations are known, whose time interval overlaps at least 70%. On a match, the new activity SHALL be stored with a reference to the suspected original rather than discarded. Activities without a start time SHALL never be flagged. + +#### Scenario: Same ride from two providers +- **WHEN** a Wahoo import created an activity and a Garmin backfill delivers the same ride starting 30 seconds apart +- **THEN** the second activity is stored flagged as a suspected duplicate of the first + +#### Scenario: Back-to-back repeats are not duplicates +- **WHEN** two activities start 90 seconds apart but the first lasts only 1 minute (intervals barely overlap) +- **THEN** neither is flagged + +### Requirement: Quarantine until resolved +A suspected-duplicate activity SHALL be excluded from feeds, listings, profile statistics, the public API, follower notifications, and federation until resolved. The owner SHALL still be able to open it; other users SHALL NOT. + +#### Scenario: Stats not double-counted +- **WHEN** a suspected duplicate exists +- **THEN** weekly distance and profile stats count the original only + +#### Scenario: Not federated while quarantined +- **WHEN** an activity is flagged +- **THEN** no ActivityPub Create is emitted for it + +### Requirement: Owner notification and review +The owner SHALL receive a "possible duplicate import" notification linking to a review surface that shows both activities' source, start time, duration, and distance, offering **keep both** (clears the flag; the activity publishes with its original visibility, including federation) and **discard** (deletes the flagged copy while keeping the provider import record so it is not re-imported). + +#### Scenario: Keep both +- **WHEN** the owner chooses keep both +- **THEN** the flag is cleared and the activity appears in feeds/stats and federates normally + +#### Scenario: Discard +- **WHEN** the owner chooses discard +- **THEN** the flagged activity is deleted, the original is untouched, and a provider re-sync does not recreate it + +### Requirement: Import batch reporting +Import batches SHALL count suspected duplicates separately from exact same-provider duplicates. + +#### Scenario: Batch summary distinguishes +- **WHEN** a backfill imports 10 activities of which 2 match existing Wahoo activities by time overlap +- **THEN** the batch reports 2 suspected duplicates distinct from exact-ID skips diff --git a/openspec/changes/activity-duplicate-review/tasks.md b/openspec/changes/activity-duplicate-review/tasks.md new file mode 100644 index 0000000..c2b8e22 --- /dev/null +++ b/openspec/changes/activity-duplicate-review/tasks.md @@ -0,0 +1,26 @@ +## 1. Schema + +- [ ] 1.1 Add `activities.suspected_duplicate_of_id` (nullable, FK activities on delete set null) and `import_batches.suspected_duplicate_count` + migration + +## 2. Detection + +- [ ] 2.1 Implement the overlap heuristic (±120 s start delta, ≥70% interval overlap when durations known) as a pure, unit-tested function +- [ ] 2.2 Wire it into the shared activity-creation path inside the insert transaction (row-locked window query); set the FK on match; increment batch counter for batch imports +- [ ] 2.3 Unit tests: two-provider same ride flagged; back-to-back repeats not flagged; missing startedAt bypasses; concurrent insert race resolves to one flagged copy + +## 3. Quarantine enforcement + +- [ ] 3.1 Add `suspected_duplicate_of_id IS NULL` to the shared activity-listing scope; audit every query site (feed, listings, profile stats, weekly distance, api.v1 lists, federation outbox, follower notification trigger) to confirm they use the scope +- [ ] 3.2 Owner can open the flagged activity; non-owners get the private-equivalent 404 +- [ ] 3.3 Regression test: a flagged activity appears in none of feed / profile stats / API list / outbox, and its follower notification never fires + +## 4. Notification + review UI + +- [ ] 4.1 New notification type "possible duplicate import" (payload: both activity ids) following the versioned-payload pattern; deep link to the flagged activity +- [ ] 4.2 Review banner on the flagged activity page: side-by-side source/time/duration/distance, keep-both and discard actions (i18n en/de) +- [ ] 4.3 Keep both → clear flag + emit federation Create + follower notification now; Discard → delete flagged copy, retain `sync_imports` row; tests for both paths + +## 5. Verification + +- [ ] 5.1 E2E: import the same GPX twice via two paths on a seeded account → banner appears → resolve both ways +- [ ] 5.2 Run `pnpm typecheck && pnpm lint && pnpm test && pnpm test:e2e` diff --git a/openspec/changes/activity-locations/.openspec.yaml b/openspec/changes/activity-locations/.openspec.yaml new file mode 100644 index 0000000..dd9a1d9 --- /dev/null +++ b/openspec/changes/activity-locations/.openspec.yaml @@ -0,0 +1,2 @@ +schema: spec-driven +created: 2026-07-06 diff --git a/openspec/changes/activity-locations/design.md b/openspec/changes/activity-locations/design.md new file mode 100644 index 0000000..a693800 --- /dev/null +++ b/openspec/changes/activity-locations/design.md @@ -0,0 +1,50 @@ +## Context + +Endurain resolves activity locations by HTTP-geocoding start coordinates against Nominatim or geocode.maps.co (`activity/utils.py:1271`) — exactly the third-party location leak trails' privacy posture forbids (same reasoning that just drove the `poi-index` change to remove Overpass). trails stores route/activity geometry in PostGIS and GPX; start point is trivially available at save time (`gpx-save.server.ts` already extracts coords and startTime). + +GeoNames publishes redistributable (CC-BY) place datasets; `cities500` (~200k populated places with name, admin1, country, lat/lon) compresses to a few MB — small enough to bundle, dense enough that a nearest-place lookup names the right town for outdoor activity starts. + +## Goals / Non-Goals + +**Goals:** +- "Freiburg im Breisgau, Germany" on activities and routes with zero third-party requests and zero config for self-hosters. +- Deterministic, offline, fast (sub-millisecond lookup), computed once at write time. +- The label never reveals more than the visible geometry already does. + +**Non-Goals:** +- Street/trailhead-level naming, multilingual place names (dataset ships primary names; country names localize via i18n), location-based search or grouping, online fallback of any kind. + +## Decisions + +### 1. Bundled GeoNames `cities500` + nearest-neighbor lookup, in-process + +New `packages/geocode`: build script converts the GeoNames TSV into a compact sorted binary/JSON artifact checked into the package (with attribution + license file); runtime loads it once and answers `nearestPlace(lat, lon)` via a static grid index (place count is small enough that a 1°-cell grid with neighbor scan beats pulling in a kd-tree dep). Returns `{name, admin1, countryCode, distanceKm}`. + +*Alternatives:* PostGIS table + spatial query — works, but turns a pure function into a schema + seed concern and complicates self-host setup; Nominatim self-hosted — absurd footprint for naming a start town; per-request third-party geocoding — ruled out by principle. + +### 2. Label rules + +Use the first point of the geometry. If the nearest place is > 30 km away (open ocean, wilderness), fall back to country only; if > 200 km or no country, store null (no label). Store `location_name` ("Freiburg im Breisgau") and `location_country` (ISO code, rendered localized client-side). Computed in `gpx-save.server.ts`'s derive step so every creation path (upload, planner save, Wahoo/Garmin/Komoot import, federation ingest of remote activities is **excluded** — remote rows keep whatever the origin published, we don't re-derive from remote geometry). + +### 3. Masking follows the geometry, one rule + +The label is shown only where the viewer may see the track start: today that means it renders wherever the map/GPX renders for that viewer. When `activity-privacy-controls` lands, its location/start-hiding flag governs the label through the same mask helper. This coupling is stated in both changes so neither ships a leak. + +### 4. Backfill as a one-shot job + +pg-boss job iterates rows with geometry and null `location_name` in batches. Runs once on deploy; re-runnable safely (idempotent predicate). + +## Risks / Trade-offs + +- [Nearest-place can name the wrong side of a border/ridge] → Cosmetic; 200k-place density makes gross errors rare; label is presentation-only, never used for logic. +- [Dataset staleness (renames, new places)] → Refresh = re-run the build script in a PR; places churn slowly. +- [CC-BY attribution obligation] → GeoNames attribution added to the package license file and the journal's about/credits, alongside existing OSM attribution. +- [Memory: dataset resident in the journal process] → ~200k compact records ≈ tens of MB worst case; measured in tasks, with the grid artifact designed for lazy load. + +## Migration Plan + +Additive columns + package + backfill job; display changes are progressive (null label renders nothing). Rollback = revert code; columns can stay. + +## Open Questions + +- None blocking. Whether route *end* location is also worth labeling ("Freiburg → Basel") is a display-only follow-up. diff --git a/openspec/changes/activity-locations/proposal.md b/openspec/changes/activity-locations/proposal.md new file mode 100644 index 0000000..5e9ae74 --- /dev/null +++ b/openspec/changes/activity-locations/proposal.md @@ -0,0 +1,29 @@ +## Why + +Activities and routes display as bare stat rows — nothing tells a feed reader *where* "Sunday ride, 62 km" happened, which is often the most interesting fact. Endurain reverse-geocodes activity start points into city/country names (reviewed 2026-07-06), but does it by calling Nominatim/geocode.maps.co — a third-party request carrying user location that trails' privacy principles rule out. We can have the feature without the leak: an **offline** reverse geocoder using a bundled place dataset, running entirely on-instance. + +## What Changes + +- New shared package capability: offline reverse geocoding — nearest populated place (name, admin region, ISO country) for a coordinate, from a bundled GeoNames-derived dataset (`cities500`-class, a few MB), no network calls. +- Activities and routes gain a derived **location label** ("Freiburg, Germany"), computed at save/import time from the geometry's start point and stored on the row. +- Location shows on detail pages and feed/list cards; localized country names via the existing i18n locales. +- **Privacy-coupled**: the label derives from where the track starts, so it is masked whenever the start location is hidden — it follows the same rules as the map/start data under the `activity-privacy-controls` change (if that lands, its `hideLocation`-style flag governs this label; until then, the label is only shown where the geometry itself is visible). +- Backfill of existing rows via a one-shot background job. +- Not in scope: full address geocoding, location search/filtering, per-point location, and any online geocoding fallback. + +## Capabilities + +### New Capabilities +- `activity-locations`: Offline reverse geocoding of route/activity start points into display labels — dataset, derivation timing, storage, display, masking rule, and backfill. + +### Modified Capabilities + + + +## Impact + +- New `packages/geocode` (or module in an existing package): bundled dataset + nearest-place lookup (kd-tree/grid), pure and unit-tested; used by the journal only. +- `packages/db`: `location_name`, `location_country` (text, nullable) on `activities` and `routes` + migration. +- Journal: set at gpx-save/import time; one-shot pg-boss backfill job; feed card + detail page display (i18n). +- Privacy manifest: note that location labels are derived on-instance with no third-party calls. +- Bundle size: the dataset ships server-side only (journal), never to the browser. diff --git a/openspec/changes/activity-locations/specs/activity-locations/spec.md b/openspec/changes/activity-locations/specs/activity-locations/spec.md new file mode 100644 index 0000000..63505d0 --- /dev/null +++ b/openspec/changes/activity-locations/specs/activity-locations/spec.md @@ -0,0 +1,37 @@ +## ADDED Requirements + +### Requirement: Offline reverse geocoding +The Journal SHALL derive location labels from a bundled place dataset entirely on-instance; deriving a location SHALL NOT trigger any network request. The lookup SHALL return the nearest populated place with its region and ISO country for a coordinate, falling back to country-only beyond 30 km from any place and to no label beyond 200 km. + +#### Scenario: No third-party calls +- **WHEN** an activity is saved and its location derived +- **THEN** no request leaves the instance + +#### Scenario: Wilderness start +- **WHEN** a track starts 50 km from the nearest populated place +- **THEN** the label is the country only + +### Requirement: Location labels on activities and routes +Activities and routes with geometry SHALL store a derived location label (place name + country) computed from the track's start point at save/import time for locally-created content. Remote (federated) activities SHALL NOT be re-derived locally. Detail pages and feed/list cards SHALL display the label with the country name localized. + +#### Scenario: Label on import +- **WHEN** a Wahoo activity starting in Freiburg is imported +- **THEN** the activity shows "Freiburg im Breisgau, Deutschland" in a German locale + +#### Scenario: No geometry, no label +- **WHEN** an activity has no GPS data +- **THEN** no location label is stored or shown + +### Requirement: Label visibility follows geometry visibility +The location label SHALL be visible to a viewer only when that viewer may see the track's start (same masking as the map/geometry; governed by the activity privacy flags once those exist). + +#### Scenario: Hidden geometry hides label +- **WHEN** a viewer is not permitted to see an activity's map/start +- **THEN** the location label is also absent for that viewer + +### Requirement: Backfill of existing content +Existing local activities and routes with geometry SHALL receive labels via an idempotent one-shot background job. + +#### Scenario: Backfill run +- **WHEN** the backfill job completes +- **THEN** previously saved activities with geometry display location labels diff --git a/openspec/changes/activity-locations/tasks.md b/openspec/changes/activity-locations/tasks.md new file mode 100644 index 0000000..69584f0 --- /dev/null +++ b/openspec/changes/activity-locations/tasks.md @@ -0,0 +1,22 @@ +## 1. Geocode package + +- [ ] 1.1 Create `packages/geocode`: build script converting GeoNames `cities500` TSV → compact artifact (name, admin1, country, lat, lon), checked in with CC-BY attribution/license file +- [ ] 1.2 Implement `nearestPlace(lat, lon)` with a 1°-grid index + neighbor scan; distance thresholds (30 km place / 200 km country) as constants; measure load time + resident memory and record in the package README +- [ ] 1.3 Unit tests: city center, border town, mid-ocean (country-only/none), poles/antimeridian edge cells + +## 2. Schema & derivation + +- [ ] 2.1 Add `location_name` + `location_country` to `activities` and `routes` + migration +- [ ] 2.2 Derive in the gpx-save shared step from the first geometry point for all local creation paths; skip remote/federated ingest +- [ ] 2.3 One-shot idempotent pg-boss backfill job over rows with geometry and null label + +## 3. Display + +- [ ] 3.1 Detail pages + feed/list cards render the label (place + localized country name via i18n); null renders nothing +- [ ] 3.2 Masking: render the label only where the viewer can see the geometry (shared visibility path); coordinate with `activity-privacy-controls` flag if landed +- [ ] 3.3 Add GeoNames attribution to the journal credits alongside OSM attribution; update the privacy manifest (derived on-instance, no third parties) + +## 4. Verification + +- [ ] 4.1 E2E: import a fixture GPX → label appears on detail + feed; private activity's label invisible to another user +- [ ] 4.2 Run `pnpm typecheck && pnpm lint && pnpm test && pnpm test:e2e` diff --git a/openspec/changes/activity-privacy-controls/.openspec.yaml b/openspec/changes/activity-privacy-controls/.openspec.yaml new file mode 100644 index 0000000..dd9a1d9 --- /dev/null +++ b/openspec/changes/activity-privacy-controls/.openspec.yaml @@ -0,0 +1,2 @@ +schema: spec-driven +created: 2026-07-06 diff --git a/openspec/changes/activity-privacy-controls/design.md b/openspec/changes/activity-privacy-controls/design.md new file mode 100644 index 0000000..e3a7aac --- /dev/null +++ b/openspec/changes/activity-privacy-controls/design.md @@ -0,0 +1,58 @@ +## Context + +Activities carry `visibility` (private/unlisted/public) plus `audience` for federation; non-owner access control happens in the detail loaders and list scopes, and federation publishes activity objects via `federation-objects.server.ts`/outbox. Signals stored per activity: `startedAt`, geometry/GPX, elevation series (whose samples include lat/lng for chart↔map sync), distance, duration, moving time, derived pace/speed (`stats.ts`), photos, and (per the `activity-locations` change) a location label. + +Endurain's reference: 13 boolean `hide_*` columns + user defaults stamped at import + `apply_visibility_mask()` nulling fields for non-owners + one frontend gate. trails needs fewer flags (no HR/power/cadence stored) but a wider enforcement surface (federation). + +## Goals / Non-Goals + +**Goals:** +- Share a ride without sharing when you left, where exactly, or how fast — four independent, comprehensible flags. +- Masked data never reaches a non-owner through *any* surface, including ActivityPub objects, GPX downloads, and chart data. +- Defaults make it a set-once decision; per-activity override stays available. + +**Non-Goals:** +- Privacy zones (geometry editing near saved places) — separate change layered on this mask. +- Route-level flags, per-follower exceptions, HR/power flags (nothing stored to hide). +- Retracting already-federated copies on flag changes (Update activities may be sent, but remote caches are best-effort — honest UI note instead of a false promise). + +## Decisions + +### 1. Four flags, jsonb, typed + +`activities.privacy: { hideStartTime, hideLocation, hideMap, hidePace }` (jsonb, default `{}` = nothing hidden) and `users.activity_privacy_defaults` with the same shape. Jsonb over boolean columns: the set will grow (privacy zones, photo location stripping), flags are never queried server-side as predicates (masking is row-level at serialization), and the schema convention already favors typed jsonb for extensible shapes (cf. `SurfaceBreakdown`). + +Flag semantics (what a non-owner loses): +- `hideStartTime` → `startedAt` truncated to date (feed ordering uses the real value server-side). +- `hideLocation` → no location label; start/end markers not distinguished on the map (map itself governed by `hideMap`). +- `hideMap` → no geometry, no map, no thumbnail, no GPX download, elevation profile served as distance/elevation only (positions stripped) or omitted. +- `hidePace` → no pace/speed/moving time; distance + elapsed duration remain. + +### 2. One mask at the serialization boundary + +`applyActivityPrivacyMask(data, { isOwner })` lives beside `stats.ts` and is the only place flag semantics are implemented. Loaders/serializers call it before returning non-owner payloads; the GPX/thumbnail/elevation endpoints check `hideMap` explicitly (they return binary, not the masked object). Owners always receive everything plus the flag state (so the UI can show "hidden from others" badges). + +*Alternative:* masking in the frontend gate (Endurain's `VisibilityGate`) — rejected as the primary mechanism: data must not reach the client at all; a UI gate is cosmetic. Frontend only renders what it receives. + +### 3. Federation: mask at object-build time + +`federation-objects.server.ts` builds AP objects through the same mask with `isOwner: false` — a hidden map means the published object carries no geometry attachment; hidden start time publishes date precision only. Flag changes after publish affect subsequent serves and trigger an AP `Update`; remote re-fetch honesty is best-effort (documented in UI copy: "already-shared copies may persist on other instances"). + +### 4. Stamping and editing + +Defaults stamp at creation in the shared save path (upload, provider import, planner handoff). The activity edit surface exposes the four toggles; the settings page edits defaults (affecting future activities only, stated in copy). Demo-bot and e2e fixtures default to nothing hidden. + +## Risks / Trade-offs + +- [A new endpoint forgets the mask] → Single helper + a table-driven regression test that walks every activity-serving route as a non-owner and asserts hidden fields absent for a fully-flagged fixture; code-review rule noted in the spec. +- [Elevation chart without positions degrades chart↔map sync] → Intended: with `hideMap` there is no map to sync to; chart renders standalone. +- [Federated copies predate a newly-set flag] → Honest UI note + AP Update on change; no false retraction promise. +- [Flag sprawl later] → jsonb shape + single mask keep additions one-file changes. + +## Migration Plan + +Additive columns (default `{}` = current behavior — nothing hidden). Ship mask + UI together; existing activities unaffected until a user sets flags. Rollback = revert; stored flags become inert. + +## Open Questions + +- None blocking. Privacy zones (trim/blur geometry within N meters of saved locations) are the designed-for follow-up on top of `hideMap`'s enforcement points. diff --git a/openspec/changes/activity-privacy-controls/proposal.md b/openspec/changes/activity-privacy-controls/proposal.md new file mode 100644 index 0000000..aa9260a --- /dev/null +++ b/openspec/changes/activity-privacy-controls/proposal.md @@ -0,0 +1,27 @@ +## Why + +Activity visibility today is all-or-nothing (`private`/`unlisted`/`public`): sharing a ride means sharing exactly when you left home, where the track starts, and your pace. For a privacy-first platform this forces the wrong trade — users who'd happily share the route and photos keep activities private because one field is sensitive. Endurain (reviewed 2026-07-06) demonstrates the answer: independent per-signal hide flags with per-user defaults, per-activity overrides, and a single server-side mask for non-owners. trails should offer the equivalent for the signals it actually stores. + +## What Changes + +- **Four per-activity privacy flags**, each independent of `visibility`: `hideStartTime` (viewers see the date, not the time), `hideLocation` (no location label, per `activity-locations`), `hideMap` (no map, geometry, elevation-chart positions, or GPX download for non-owners — stats remain), `hidePace` (no pace/speed/moving time; distance and duration remain). +- **Per-user defaults** in profile settings, stamped onto each new activity at creation/import; each activity's flags editable afterward on its edit surface. +- **One server-side mask** applied everywhere a non-owner receives activity data: detail loaders, feeds, public API, GPX download, map thumbnails, and — critically — **federation**: masked fields are omitted from ActivityPub objects so they never leave the instance. +- Scope: activities only (recorded presence is the sensitive data). Routes keep plain visibility; extending flags to routes is a possible follow-up. +- Not in scope: privacy zones (geometry trimming/blurring near saved locations) — a natural later layer on top of this masking infrastructure; per-follower exceptions; retroactive re-federation of already-published objects (changing flags affects what is served/federated from then on; note in UI). + +## Capabilities + +### New Capabilities +- `activity-privacy`: The per-signal privacy flags — their semantics, user defaults, stamping, the non-owner mask, and its enforcement surface including federation. + +### Modified Capabilities + + + +## Impact + +- `packages/db`: `activities.privacy` jsonb (typed four-flag object, default all-false) + `users.activity_privacy_defaults` jsonb + migration. +- Journal: `applyPrivacyMask(activity, viewerIsOwner)` helper used by detail loaders, feed/listing serializers, `api.v1` activity routes, GPX download route, thumbnail/elevation endpoints, and `federation-objects.server.ts` / outbox emission. +- UI: defaults section in profile/privacy settings; per-activity toggles on the activity edit surface; owner-visible "hidden from others" indicators on masked fields (i18n en/de). +- Interacts with in-flight changes: `activity-locations` (label obeys `hideLocation`), `activity-duplicate-review` (quarantine is orthogonal), privacy manifest update. diff --git a/openspec/changes/activity-privacy-controls/specs/activity-privacy/spec.md b/openspec/changes/activity-privacy-controls/specs/activity-privacy/spec.md new file mode 100644 index 0000000..3f19717 --- /dev/null +++ b/openspec/changes/activity-privacy-controls/specs/activity-privacy/spec.md @@ -0,0 +1,38 @@ +## ADDED Requirements + +### Requirement: Per-signal privacy flags +Each activity SHALL carry four independent privacy flags — `hideStartTime`, `hideLocation`, `hideMap`, `hidePace` — orthogonal to its visibility level. For non-owner viewers: `hideStartTime` reduces the start time to date precision; `hideLocation` removes the location label; `hideMap` removes geometry, map, thumbnail, GPX download, and position data in the elevation profile; `hidePace` removes pace, speed, and moving time while keeping distance and elapsed duration. + +#### Scenario: Share the ride, hide the departure +- **WHEN** a public activity has `hideStartTime` and `hidePace` set and a non-owner views it +- **THEN** they see the date (not time), distance, duration, map, and photos, but no pace, speed, or moving time + +#### Scenario: Hidden map removes all geometry surfaces +- **WHEN** `hideMap` is set and a non-owner requests the activity page, its GPX download, its thumbnail, or its elevation data +- **THEN** no coordinates reach them through any of those surfaces (elevation, if shown, is distance/altitude only) + +#### Scenario: Owner sees everything +- **WHEN** the owner views their flagged activity +- **THEN** all data renders, with indicators showing which fields are hidden from others + +### Requirement: User defaults stamped at creation +Users SHALL have activity-privacy defaults in settings; every locally created or imported activity SHALL receive the defaults at creation, and the flags SHALL be editable per activity afterwards. Changing defaults SHALL NOT alter existing activities. + +#### Scenario: Default applies to next import +- **WHEN** a user enables `hideStartTime` in defaults and a Wahoo activity syncs +- **THEN** the new activity has `hideStartTime` set + +#### Scenario: Per-activity override +- **WHEN** the user clears the flag on that one activity +- **THEN** only that activity's start time becomes visible to others + +### Requirement: Masking enforced server-side including federation +Masked fields SHALL be removed before data leaves the server for any non-owner surface — detail loaders, feeds, the public API, and ActivityPub objects (masked fields are omitted from published objects; flag changes trigger an Update). Masking SHALL be implemented in a single shared helper. + +#### Scenario: Federated object carries no hidden data +- **WHEN** an activity with `hideMap` federates to a follower on another instance +- **THEN** the published object contains no track geometry + +#### Scenario: Every serving route masked +- **WHEN** the regression suite requests a fully-flagged activity as a non-owner across all activity-serving endpoints +- **THEN** no hidden field appears in any response diff --git a/openspec/changes/activity-privacy-controls/tasks.md b/openspec/changes/activity-privacy-controls/tasks.md new file mode 100644 index 0000000..2161f77 --- /dev/null +++ b/openspec/changes/activity-privacy-controls/tasks.md @@ -0,0 +1,26 @@ +## 1. Schema + +- [ ] 1.1 Add `activities.privacy` jsonb (typed `ActivityPrivacy` four-flag shape, default `{}`) and `users.activity_privacy_defaults` jsonb + migration + +## 2. Mask helper + +- [ ] 2.1 Implement `applyActivityPrivacyMask` beside `stats.ts`: flag semantics per design (date-truncated start, label removal, geometry/positions stripping, pace/speed/moving-time removal); unit tests per flag and combined +- [ ] 2.2 Stamp defaults in the shared activity-creation path (upload, provider imports, planner handoff); demo bot and fixtures default to `{}` + +## 3. Enforcement + +- [ ] 3.1 Apply the mask in: activity detail loader, feed/list serializers, `api.v1` activity routes, GPX download route, thumbnail endpoint, elevation-series payload +- [ ] 3.2 Federation: build AP objects through the mask (`federation-objects.server.ts` + outbox); flag edits fire an AP Update; UI copy notes remote copies are best-effort +- [ ] 3.3 Table-driven regression test: fully-flagged fixture requested as non-owner across every activity-serving endpoint → no hidden field present; owner requests → all present + +## 4. UI + +- [ ] 4.1 Privacy defaults section in settings (four toggles, i18n en/de, copy explains each flag affects future activities) +- [ ] 4.2 Per-activity toggles on the activity edit surface; "hidden from others" indicators on the owner's detail view +- [ ] 4.3 Coordinate with `activity-locations` (label obeys `hideLocation`/`hideMap`) if that change has landed + +## 5. Docs & verification + +- [ ] 5.1 Update the privacy manifest: per-signal controls exist; federation masking semantics; remote-copy caveat +- [ ] 5.2 E2E: set defaults → import → verify masked view as second user; toggle per-activity flag → verify change +- [ ] 5.3 Run `pnpm typecheck && pnpm lint && pnpm test && pnpm test:e2e` diff --git a/openspec/changes/credits-page/.openspec.yaml b/openspec/changes/credits-page/.openspec.yaml new file mode 100644 index 0000000..dd9a1d9 --- /dev/null +++ b/openspec/changes/credits-page/.openspec.yaml @@ -0,0 +1,2 @@ +schema: spec-driven +created: 2026-07-06 diff --git a/openspec/changes/credits-page/design.md b/openspec/changes/credits-page/design.md new file mode 100644 index 0000000..2b95f2c --- /dev/null +++ b/openspec/changes/credits-page/design.md @@ -0,0 +1,42 @@ +## Context + +Static informational pages already follow a pattern: journal routes (`legal.terms.tsx`, `legal.privacy.tsx`, `privacy.tsx`) registered in `routes.ts`, linked from the shared `Footer.tsx`; the planner's footer links to absolute Journal URLs because the planner is stateless (spec `legal-disclaimers`, "Footer legal links"). The acknowledgment list lives in `docs/philosophy.md` (Open Source section, updated 2026-07-06 with Organic Maps and Endurain). Map attribution renders in the Leaflet attribution control; data-level attribution (GeoNames CC-BY from `activity-locations`, extended OSM usage from `poi-index`) has no page-level home yet — both changes reference "journal credits" as a pending surface. + +## Goals / Non-Goals + +**Goals:** +- One user-visible page that carries inspirations, data attributions, and key components — reachable from both apps' footers. +- Curated and short: every entry says *why* it's there in one line. +- Stays consistent with `docs/philosophy.md` without a build-time sync mechanism. + +**Non-Goals:** +- Generated dependency/license inventory, sponsor/donor lists, per-page dynamic attribution logic (the Leaflet control keeps doing on-map attribution). + +## Decisions + +### 1. Route: `/about/credits` on the Journal, static component + +A plain route module like the legal pages — no loader, content as JSX with `useTranslation`. Chosen over `/legal/credits` because credits aren't a legal disclosure and shouldn't imply one; `/about/` leaves room for future informational pages. Planner links to `${JOURNAL_URL}/about/credits`, mirroring the legal-links pattern exactly. + +### 2. Content structure: three typed lists, strings in i18n + +The entries (name, URL, description-key) live as a small typed array in the route module; descriptions and headings resolve through i18n so de/en stay complete. Entry list: Inspirations (BRouter, bikerouter.de, brouter-web, Organic Maps, Endurain, wanderer), Data (OSM/ODbL with the required "© OpenStreetMap contributors" wording, BRouter segment data, tile providers, GeoNames slot commented until `activity-locations` lands), Built with (BRouter, Leaflet, Yjs, Fedify, PostGIS). + +*Alternative:* markdown file rendered at build time — rejected; i18n for prose is already the established mechanism and a JSX page matches the legal-page precedent. + +### 3. Sync with the docs by convention, not tooling + +`docs/inspirations.md` is the canonical record: each project entry carries a **credit line** that is the exact acknowledgment text for both the philosophy list and this page's entries (translated for de). A comment atop the entries array points there; a generator would be over-engineering for a six-item list (simplicity principle). + +## Risks / Trade-offs + +- [Lists drift between docs and page] → cross-reference comments; both files touched in the same PR whenever an inspiration is added (as this change itself demonstrates). +- [Attribution wording obligations (ODbL, CC-BY)] → use each project's canonical attribution string; verify against their attribution guidelines in tasks. + +## Migration Plan + +Additive UI + route registration; no data or API changes. Rollback = revert. + +## Open Questions + +- None blocking. diff --git a/openspec/changes/credits-page/proposal.md b/openspec/changes/credits-page/proposal.md new file mode 100644 index 0000000..c3a4ffd --- /dev/null +++ b/openspec/changes/credits-page/proposal.md @@ -0,0 +1,30 @@ +## Why + +trails.cool owes its shape to other open projects — BRouter, bikerouter.de, brouter-web, wanderer, and (as of the 2026-07 research) Organic Maps and Endurain — and builds on data that carries attribution obligations (OpenStreetMap; GeoNames once `activity-locations` lands). Today that acknowledgment lives only in `docs/philosophy.md`, invisible to users. A user-visible credits page makes the acknowledgment real, gives data attributions a durable home beyond the map corner, and matches how the projects we admire credit their own inputs (Organic Maps' `copyright.html`, its required-credit NOTICE). + +## What Changes + +- New **`/about/credits`** page on the Journal with three curated sections: + 1. **Inspirations** — the projects from the philosophy doc's acknowledgment list, each with a link and a one-line "what we learned from them". + 2. **Data** — OpenStreetMap (© OpenStreetMap contributors, ODbL), BRouter routing segments, tile providers, and a slot for GeoNames (CC-BY) when `activity-locations` ships. + 3. **Built with** — a short curated list of load-bearing open-source components (BRouter, Leaflet, Yjs, Fedify, PostGIS), not a generated dependency dump. +- **Footer link** "Credits" in both apps (planner links to the Journal page, same pattern as the legal links). +- The page and `docs/philosophy.md`'s acknowledgment list are cross-referenced so they stay in sync (comment in both pointing at the other). +- Localized (en/de) like every user-facing surface. +- Not in scope: an auto-generated full dependency-license inventory (can be a later `licenses` page if ever needed), donation/sponsor listings. + +## Capabilities + +### New Capabilities +- `credits-page`: The user-visible credits & inspirations page — content sections, localization, and its footer entry point. + +### Modified Capabilities +- `legal-disclaimers`: the "Footer legal links" requirement gains the Credits link in both apps. + +## Impact + +- `apps/journal`: new route `about.credits.tsx` (+ registration in `routes.ts`), Footer link. +- `apps/planner`: footer variant gains the absolute link to the Journal page. +- `packages/i18n`: footer label + page strings (en/de). +- `docs/philosophy.md`: cross-reference comment. +- `activity-locations` and `poi-index` tasks that mention "journal credits" now have their concrete target surface. diff --git a/openspec/changes/credits-page/specs/credits-page/spec.md b/openspec/changes/credits-page/specs/credits-page/spec.md new file mode 100644 index 0000000..414735f --- /dev/null +++ b/openspec/changes/credits-page/specs/credits-page/spec.md @@ -0,0 +1,16 @@ +## ADDED Requirements + +### Requirement: Credits page +The Journal SHALL serve a credits page at `/about/credits` with three sections — inspirations (linked projects with a one-line acknowledgment each, matching the list in `docs/philosophy.md`), data attributions (including "© OpenStreetMap contributors" per ODbL guidelines and, once shipped, GeoNames CC-BY), and key open-source components. All strings SHALL be localized. + +#### Scenario: Page renders with inspirations +- **WHEN** a user (authenticated or not) opens `/about/credits` +- **THEN** the page lists BRouter, bikerouter.de, brouter-web, Organic Maps, Endurain, and wanderer with links and acknowledgment lines + +#### Scenario: Localized content +- **WHEN** the UI locale is German +- **THEN** headings and acknowledgment lines render in German + +#### Scenario: OSM attribution wording +- **WHEN** the data section renders +- **THEN** it contains the canonical "© OpenStreetMap contributors" attribution with a link to the OSM copyright page diff --git a/openspec/changes/credits-page/specs/legal-disclaimers/spec.md b/openspec/changes/credits-page/specs/legal-disclaimers/spec.md new file mode 100644 index 0000000..1639160 --- /dev/null +++ b/openspec/changes/credits-page/specs/legal-disclaimers/spec.md @@ -0,0 +1,12 @@ +## MODIFIED Requirements + +### Requirement: Footer legal links +Both the Journal and Planner SHALL render a footer with links to Impressum, Datenschutzerklärung, Terms of Service, Credits, and the source repository. + +#### Scenario: Footer on Journal +- **WHEN** a user views any Journal page +- **THEN** the footer contains links labeled "Impressum", "Privacy", and "Terms" pointing to the respective `/legal/*` pages, a "Credits" link pointing to `/about/credits`, plus a link to the source repository + +#### Scenario: Footer on Planner +- **WHEN** a user views any Planner page +- **THEN** the footer contains the same legal and credits links, pointing at absolute URLs on the Journal instance (since the Planner is stateless and these pages live on the Journal), plus a link to the source repository diff --git a/openspec/changes/credits-page/tasks.md b/openspec/changes/credits-page/tasks.md new file mode 100644 index 0000000..276aa39 --- /dev/null +++ b/openspec/changes/credits-page/tasks.md @@ -0,0 +1,20 @@ +## 1. Page + +- [ ] 1.1 Create `apps/journal/app/routes/about.credits.tsx` with the three sections (typed entries array + i18n strings); register in `apps/journal/app/routes.ts` +- [ ] 1.2 Add en/de strings to `packages/i18n` (headings, per-entry acknowledgment lines, attribution wording) +- [ ] 1.3 Verify OSM attribution wording against the ODbL attribution guidelines; leave a commented GeoNames entry slot referencing `activity-locations` + +## 2. Footer links + +- [ ] 2.1 Add the Credits link to the journal `Footer.tsx` +- [ ] 2.2 Add the absolute-URL Credits link to the planner footer (same pattern as legal links) + +## 3. Cross-references + +- [ ] 3.1 Source acknowledgment lines from the credit lines in `docs/inspirations.md`; add mutual "update together" comments to the entries array, `docs/philosophy.md`, and `docs/inspirations.md` +- [ ] 3.2 Note the credits page as the target surface in the still-open `poi-index` and `activity-locations` attribution tasks + +## 4. Verification + +- [ ] 4.1 E2E: `/about/credits` renders unauthenticated with all six inspirations; footer link present on a journal page and a planner page +- [ ] 4.2 Run `pnpm typecheck && pnpm lint && pnpm test && pnpm test:e2e` diff --git a/openspec/changes/elevation-profile-hardening/.openspec.yaml b/openspec/changes/elevation-profile-hardening/.openspec.yaml new file mode 100644 index 0000000..e089cfa --- /dev/null +++ b/openspec/changes/elevation-profile-hardening/.openspec.yaml @@ -0,0 +1,2 @@ +schema: spec-driven +created: 2026-07-05 diff --git a/openspec/changes/elevation-profile-hardening/design.md b/openspec/changes/elevation-profile-hardening/design.md new file mode 100644 index 0000000..e3e7cf6 --- /dev/null +++ b/openspec/changes/elevation-profile-hardening/design.md @@ -0,0 +1,70 @@ +## Context + +`@trails-cool/gpx` computes elevation stats in three places, all naive: + +- `parse.ts` `computeElevation()` sums every positive/negative point-to-point delta into `gain`/`loss`. GPS and barometric traces jitter by a few metres per point, so every wobble counts as ascent — long activities overstate gain badly. +- `elevation-series.ts` `elevationSeries()` builds the chart series from raw points (stride-downsampled); single-point altitude spikes survive into the chart. +- `compute-days.ts` builds cumulative ascent/descent arrays (same naive delta sum) to derive per-day totals. + +Consumers: journal `gpx-save.server.ts` persists `parsed.elevation.gain/loss` to route/activity rows; route and activity detail loaders call `elevationSeries()`; the planner's multi-day sidebar uses `computeDays`. Planner *planned-route* ascent comes from BRouter's `totalAscend` and is not touched by this change. + +Organic Maps (`libs/map/elevation_info.cpp`, reviewed 2026-07-05) handles the same problems with two algorithms we adapt here: opposite-sign slope-outlier despiking, and threshold (hysteresis) filtered accumulation with a raw fallback. + +## Goals / Non-Goals + +**Goals:** +- Ascent/descent totals that are stable under GPS/barometer noise and close to what users see on other platforms (Garmin, Komoot). +- A despiked elevation series so chart, totals, and per-day stats are all computed from the same cleaned data and agree with each other. +- Pure, unit-testable primitives in `@trails-cool/gpx` with no API-shape change for consumers. + +**Non-Goals:** +- No backfill of stored `elevationGain`/`elevationLoss` on existing journal rows (values refresh on the next parse: re-import, sync, edit). +- No DEM-based elevation correction (replacing recorded altitude with terrain-model altitude) — separate, much larger feature. +- No change to BRouter-derived planner stats, no smoothing of route *geometry* (lat/lng untouched), no difficulty rating (possible follow-up). + +## Decisions + +### 1. New module `elevation-clean.ts` with two pure primitives + +`despike(points)` and `filteredTotals(points)` operate on the `{ distance, elevation }` sequence that already exists internally (`ElevationProfile[]`). Both `computeElevation` and `elevationSeries` call `despike` first, then derive totals/series from the cleaned sequence. One cleaning pass, consistent numbers everywhere. + +*Alternative considered:* cleaning inside each consumer (parse, series, days) independently — rejected; the chart and the headline stats would disagree whenever thresholds drift apart. + +### 2. Despiking: opposite-sign slope outliers, interpolated out + +A point is a spike iff the slope to its previous neighbor and the slope to its next neighbor both exceed `MAX_SLOPE_PERCENT` **and** have opposite signs (an isolated peak or pit). Spikes are replaced by linear interpolation between neighbors. This is Organic Maps' `SmoothSlopeOutliers`. The opposite-sign condition is the safety property: genuinely steep terrain climbs monotonically (same-sign slopes) and is never altered. + +Default `MAX_SLOPE_PERCENT = 100` (45°). Segment boundaries are respected — no interpolation across `trkseg` gaps. + +*Alternative considered:* moving-average smoothing — rejected; it flattens real terrain (summits, cols) and shifts totals everywhere instead of only where data is broken. + +### 3. Ascent/descent: hysteresis accumulation with raw fallback + +Maintain a reference altitude; accumulate ascent (or descent) only once the current altitude deviates from the reference by more than `NOISE_THRESHOLD_M`, then move the reference. This suppresses sub-threshold jitter while capturing every real climb larger than the threshold. Compute **both** raw and filtered totals; report filtered unless it is zero while raw is non-zero (near-flat tracks where all variation is sub-threshold — report raw so short flat walks don't show 0 m against a technically non-zero trace). This mirrors Organic Maps' dual `m_ascent`/`m_lowestPassedAltitude`-style bookkeeping and its `GetTotalAscent()` preference order. + +Default `NOISE_THRESHOLD_M = 5` (typical barometric jitter; GPS-only altitude is worse, but 5 m already removes the bulk of the inflation). Constants exported and overridable via an options argument for tests and future tuning. + +*Alternative considered:* distance-binned resampling (sum deltas over N-metre bins) — simpler but conflates horizontal point density with vertical noise; hysteresis is independent of point spacing. + +### 4. Per-day totals: cumulative filtered arrays + +`compute-days.ts` keeps its cumulative-array design (day total = cumulative[end] − cumulative[start]) but the arrays are produced by running the hysteresis accumulator once over the despiked full track, recording the running filtered ascent/descent at every point index. Day totals therefore sum exactly to the route total by construction. + +### 5. Chart series: despiked input, keep stride downsampling + +`elevationSeries()` consumes despiked points. The existing stride downsample stays — Douglas-Peucker on the distance/elevation polyline (Organic Maps' `Simplify`) would preserve shape better at 400 points, but stride is adequate for current chart sizes and DP is an isolated follow-up, not a dependency of correctness. + +## Risks / Trade-offs + +- [Filtered totals change displayed numbers on re-parse while old rows keep naive values] → Accepted; documented in proposal. Mixed old/new values coexist until rows are re-parsed. A backfill job is possible later since GPX blobs are retained. +- [Threshold too aggressive for genuinely rolling terrain (many real 3–4 m undulations)] → 5 m is conservative versus the 10 m+ some platforms use; raw totals remain available in the return shape (`gainRaw`/`lossRaw` added alongside `gain`/`loss`) so we can compare in the field before tuning. +- [Despiking legitimate data (e.g. real cliff-edge out-and-back)] → Opposite-sign + both-slopes-exceed condition makes false positives require a physically implausible single-point spike; fixture tests pin the behavior. +- [`elevationSeries` flattens across segments while despiking is per-segment] → Keep the flattening behavior; despike runs before flattening so no cross-segment interpolation occurs. + +## Migration Plan + +Pure library change: ship `@trails-cool/gpx` update, both apps pick it up in the same deploy. No schema, API, or config changes. Rollback = revert the PR. + +## Open Questions + +- None blocking. Threshold tuning (5 m default) may warrant revisiting once real re-parsed activities can be compared against Garmin/Komoot numbers for the same tracks. diff --git a/openspec/changes/elevation-profile-hardening/proposal.md b/openspec/changes/elevation-profile-hardening/proposal.md new file mode 100644 index 0000000..9216922 --- /dev/null +++ b/openspec/changes/elevation-profile-hardening/proposal.md @@ -0,0 +1,30 @@ +## Why + +Ascent/descent totals for imported and synced GPX are computed by summing every positive point-to-point elevation delta (`packages/gpx/src/parse.ts` `computeElevation`), with no noise handling. Real-world GPS/barometer traces are noisy, so the journal systematically overstates elevation gain — often by 20–50% on long activities — and single-point altitude spikes distort the elevation profile chart. Organic Maps solved both problems with two small, well-tested algorithms (slope-outlier despiking + threshold-filtered accumulation) that we reviewed and can adapt to `@trails-cool/gpx`. + +## What Changes + +- Add **spike removal** to elevation processing: single points whose slopes to both neighbors exceed a maximum slope and have opposite signs (peak/pit spikes) are replaced by interpolation before any totals or series are computed. +- Add **threshold-filtered ascent/descent**: accumulate gain/loss only when altitude deviates from a moving reference by more than a threshold (hysteresis), suppressing jitter. Both raw and filtered totals are computed; filtered is preferred, raw is the fallback when filtering removes everything (near-flat tracks). +- Apply the same cleaned elevation data to the **elevation profile series** (`elevationSeries`) so the chart and the headline stats agree. +- Apply filtered accumulation to **per-day ascent/descent** (`compute-days.ts`) so day totals stay consistent with the route total. +- No change to planner routing stats: planned-route ascent continues to come from BRouter (`totalAscend`). +- No backfill of stored `elevationGain`/`elevationLoss` on existing journal rows; values update when a GPX is re-parsed (new imports, syncs, edits). A backfill job is explicitly out of scope. + +## Capabilities + +### New Capabilities +- `elevation-computation`: How elevation series, ascent/descent totals, and per-day elevation stats are computed from GPX track points in `@trails-cool/gpx` — despiking, threshold-filtered accumulation, raw fallback, and series consistency guarantees. + +### Modified Capabilities + + + +## Impact + +- `packages/gpx/src/parse.ts` — `computeElevation` gains despike + filtered accumulation. +- `packages/gpx/src/elevation-series.ts` — series built from despiked points. +- `packages/gpx/src/compute-days.ts` — cumulative ascent/descent arrays use the filtered accumulator. +- New `packages/gpx/src/elevation-clean.ts` (despike + filter primitives) with co-located unit tests and noisy-track fixtures. +- Consumers (journal `gpx-save.server.ts`, route/activity detail loaders, planner multi-day breakdown) get corrected numbers with no API change — `GpxData["elevation"]` shape stays `{ gain, loss, profile }`. +- No DB schema change, no migration, no API contract change. diff --git a/openspec/changes/elevation-profile-hardening/specs/elevation-computation/spec.md b/openspec/changes/elevation-profile-hardening/specs/elevation-computation/spec.md new file mode 100644 index 0000000..8efa4ac --- /dev/null +++ b/openspec/changes/elevation-profile-hardening/specs/elevation-computation/spec.md @@ -0,0 +1,50 @@ +## ADDED Requirements + +### Requirement: Elevation spike removal +The GPX package SHALL remove single-point elevation spikes before computing elevation totals or the elevation profile series. A point is a spike when the slopes to both its neighbors exceed the maximum-slope constant and have opposite signs; spike elevations SHALL be replaced by linear interpolation between the neighboring points. Track segment boundaries SHALL NOT be interpolated across. + +#### Scenario: Isolated spike is interpolated out +- **WHEN** a track contains a point whose elevation jumps 30 m above both neighbors over a few metres of distance +- **THEN** the computed series replaces that point's elevation with the interpolation of its neighbors +- **AND** ascent/descent totals do not include the spike + +#### Scenario: Steep but real climb is preserved +- **WHEN** a track climbs steeply and monotonically (successive slopes share the same sign) +- **THEN** no point on the climb is altered + +#### Scenario: No interpolation across segment gaps +- **WHEN** a spike-like pattern spans the boundary between two track segments +- **THEN** points are not interpolated across the segment boundary + +### Requirement: Threshold-filtered ascent and descent totals +The GPX package SHALL compute ascent and descent using hysteresis accumulation: elevation change is accumulated only when altitude deviates from a moving reference by more than the noise-threshold constant. Both raw (unfiltered) and filtered totals SHALL be computed; the reported `gain`/`loss` SHALL be the filtered values, except when both filtered totals are zero and raw totals are non-zero, in which case the raw values SHALL be reported. Raw totals SHALL remain available to callers alongside the reported values. + +#### Scenario: Jitter does not inflate ascent +- **WHEN** a track's elevation oscillates by ±2 m around a constant altitude over many points +- **THEN** the reported ascent and descent are 0 m + +#### Scenario: Real climbs are fully counted +- **WHEN** a track climbs 400 m, descends 100 m, and climbs 200 m, each leg exceeding the noise threshold +- **THEN** the reported ascent is approximately 600 m and descent approximately 100 m + +#### Scenario: Near-flat track falls back to raw totals +- **WHEN** every elevation variation in a track is below the noise threshold so filtered totals are zero +- **THEN** the raw totals are reported instead of 0 m + +### Requirement: Consistent cleaned data across totals, series, and per-day stats +The elevation profile series, the ascent/descent totals, and the per-day elevation statistics SHALL all be derived from the same despiked elevation data, and per-day ascent/descent SHALL use the same filtered accumulation such that the per-day values sum to the whole-track totals. + +#### Scenario: Chart and headline stats agree +- **WHEN** a GPX with elevation spikes is parsed and rendered on a detail page +- **THEN** the elevation chart and the ascent/descent stats reflect the same despiked data + +#### Scenario: Day totals sum to route total +- **WHEN** a multi-day route with day breaks is processed +- **THEN** the sum of per-day ascent values equals the route's total ascent + +### Requirement: Unchanged public data shape +The elevation hardening SHALL NOT change the existing `GpxData["elevation"]` fields (`gain`, `loss`, `profile`) or the `ElevationSample` series shape consumed by the apps; raw totals SHALL be exposed as additional fields only. + +#### Scenario: Existing consumers compile and run unchanged +- **WHEN** the journal save path and detail loaders run against the updated package +- **THEN** they persist and render gain/loss and the elevation series without code changes diff --git a/openspec/changes/elevation-profile-hardening/tasks.md b/openspec/changes/elevation-profile-hardening/tasks.md new file mode 100644 index 0000000..38e4c58 --- /dev/null +++ b/openspec/changes/elevation-profile-hardening/tasks.md @@ -0,0 +1,19 @@ +## 1. Cleaning primitives + +- [ ] 1.1 Create `packages/gpx/src/elevation-clean.ts` with `despike(points, opts?)` — opposite-sign slope-outlier detection (default `MAX_SLOPE_PERCENT = 100`) with linear interpolation, operating per segment on `{ distance, elevation }` sequences +- [ ] 1.2 Add `filteredTotals(points, opts?)` to the same module — hysteresis accumulator (default `NOISE_THRESHOLD_M = 5`) returning `{ gain, loss, gainRaw, lossRaw }` with the raw-fallback rule (filtered zero + raw non-zero → report raw) +- [ ] 1.3 Add `cumulativeFilteredTotals(points, opts?)` returning per-point running filtered ascent/descent arrays (for per-day splits) +- [ ] 1.4 Write `elevation-clean.test.ts`: isolated spike interpolated; steep monotonic climb untouched; no cross-segment interpolation; ±2 m jitter → 0 totals; multi-leg climb counted fully; near-flat raw fallback; cumulative arrays sum to totals + +## 2. Wire into existing computations + +- [ ] 2.1 Update `computeElevation` in `packages/gpx/src/parse.ts` to despike per segment, then use `filteredTotals`; keep `gain`/`loss`/`profile` shape and expose `gainRaw`/`lossRaw` as additional fields (profile built from despiked data) +- [ ] 2.2 Update `elevationSeries` in `packages/gpx/src/elevation-series.ts` to build from despiked points (despike per segment before flattening) +- [ ] 2.3 Update `compute-days.ts` to build its cumulative ascent/descent arrays via `cumulativeFilteredTotals` over the despiked track +- [ ] 2.4 Update/extend existing tests (`parse.test.ts`, `elevation-series.test.ts`, `compute-days.test.ts`) with a shared noisy-track fixture; assert chart series, totals, and day sums agree + +## 3. Verification + +- [ ] 3.1 Confirm no consumer changes needed: typecheck journal + planner; spot-check `gpx-save.server.ts`, detail loaders, and planner `computeDays` usage compile and behave +- [ ] 3.2 Manual sanity check: parse a real noisy activity GPX (e.g. an existing import fixture) and compare gain before/after — filtered value should drop noticeably and match the chart +- [ ] 3.3 Run `pnpm typecheck && pnpm lint && pnpm test` and the journal/planner e2e suites diff --git a/openspec/changes/federation-hardening/.openspec.yaml b/openspec/changes/federation-hardening/.openspec.yaml new file mode 100644 index 0000000..dd9a1d9 --- /dev/null +++ b/openspec/changes/federation-hardening/.openspec.yaml @@ -0,0 +1,2 @@ +schema: spec-driven +created: 2026-07-06 diff --git a/openspec/changes/federation-hardening/design.md b/openspec/changes/federation-hardening/design.md new file mode 100644 index 0000000..bb40966 --- /dev/null +++ b/openspec/changes/federation-hardening/design.md @@ -0,0 +1,58 @@ +## Context + +Fedify is configured with `kv: new PostgresKvStore()` but `queue: new InProcessMessageQueue()` (`apps/journal/app/lib/federation.server.ts:146,156`). Fedify uses the queue for outbound delivery (including its retry scheduling) and inbox processing; in-process means a deploy or crash drops everything in flight. Meanwhile `federation-delivery.server.ts` already enqueues *fan-out* jobs via pg-boss (`enqueueOptional`) — so the codebase has two queueing layers, one durable (ours) and one volatile (Fedify's). Inbound: Creates dedup via the `activities.remoteOriginIri` unique constraint; other activity types (Like, Undo, Delete, Update, Accept) have no replay protection. There is no blocklist of any kind (grep-confirmed). wanderer's review (`docs/inspirations.md`) catalogs the same gaps on their side and validates the fix list; their SSRF-safe outbound client and 490-line protocol doc are the positive references. + +## Goals / Non-Goals + +**Goals:** +- No federation activity is lost to a restart; retries follow the spec's promised backoff/budget durably. +- Replayed or duplicated inbound activities are harmless no-ops. +- An operator can block a hostile instance with one documented action, effective in both directions. +- The protocol surface is documented well enough that another implementation (wanderer, a future project) can interoperate deliberately. + +**Non-Goals:** +- Moderation UI, per-actor blocks, allowlist mode, object-level reporting flows. +- Changing the object model, mirror semantics, or delivery topology (`route-federation`'s scope). +- Replacing Fedify. + +## Decisions + +### 1. Fedify `MessageQueue` implemented over pg-boss + +New `federation-queue.server.ts` implementing Fedify's `MessageQueue` interface (`enqueue` with delay support + `listen`) on pg-boss, mirroring how `PostgresKvStore` adapted `KvStore`. Rationale: pg-boss already provides durable storage, delayed jobs, retry bookkeeping, and a worker loop the journal runs — a second bespoke queue table would duplicate it. Fedify's own retry policy stays the policy owner; pg-boss supplies durability. The existing fan-out enqueue in `federation-delivery.server.ts` is unchanged (it feeds Fedify; Fedify's sends become durable underneath). + +*Alternative:* Fedify's Redis/Postgres community queue adapters — adds a dependency for less control than ~100 lines over infrastructure we already operate. + +### 2. Replay defense: processed-activity table with TTL + +`federation_processed_activities` (activity IRI primary key, received_at). Inbox handlers insert-or-skip (`ON CONFLICT DO NOTHING`; conflict → drop as duplicate) before side effects. Rows older than 30 days swept by the jobs worker — replay of month-old activities is already rejected by HTTP-signature date freshness. Creates keep their existing `remoteOriginIri` idempotency as a second layer. + +### 3. Blocklist checked at three boundaries, domain-keyed + +`federation_blocked_instances(domain, reason, created_at)`. Checks: (a) inbox — activities from actors on blocked domains are dropped before processing (202-and-discard, no error oracle); (b) delivery enqueue — recipients on blocked domains are filtered out; (c) outbox polling / actor fetch — refuses blocked domains. Domain matching is exact-host (subdomain wildcarding deferred until needed). v1 management is a documented SQL statement in the ops runbook; the table is the API an admin UI can later sit on. + +### 4. `FEDERATION.md` at repo root + +The fediverse-convention filename, documenting: NodeInfo self-identification, actor/WebFinger shape, object types (today's Note-based activities; `trails:Route` once route-federation lands), addressing/audience rules, inbox expectations (signatures, dedup semantics), delivery retry policy (now true), and moderation behavior (what being blocked means). Written from the wanderer-doc model: real JSON examples over prose. + +### 5. Observability + +pg-boss job outcomes already flow to metrics where instrumented; add `federation_delivery_total{outcome}`, `federation_queue_depth`, `federation_inbox_dropped_total{reason=duplicate|blocked}` and a dashboard row. The queue-depth gauge is the restart-loss regression detector. + +## Risks / Trade-offs + +- [Two queueing layers (our fan-out jobs + Fedify's queue) remain] → Accepted for now; collapsing them is a refactor with no user-visible payoff. Documented in code comments. +- [Blocklist drops silently (202)] → Deliberate: no error oracle for blocked instances; the drop counter keeps it observable to us. +- [TTL dedup window vs very-late redelivery] → 30 days ≫ signature freshness window; residual risk negligible. +- [pg-boss throughput for delivery spikes] → Fan-out sizes are follower counts on a young network; pg-boss handles far larger job volumes today (sync imports). + +## Migration Plan + +1. Land queue implementation + swap (behavior-compatible; in-flight in-process queue drains on the old process before cutover deploy). +2. Land dedup + blocklist tables and checks (additive). +3. Publish `FEDERATION.md` + dashboard. +Rollback: revert queue swap (falls back to in-process — the status quo), tables are inert. + +## Open Questions + +- None blocking. Whether `route-federation` should require blocklist-awareness in mirror sync is noted in that change's update rather than here. diff --git a/openspec/changes/federation-hardening/proposal.md b/openspec/changes/federation-hardening/proposal.md new file mode 100644 index 0000000..14eb312 --- /dev/null +++ b/openspec/changes/federation-hardening/proposal.md @@ -0,0 +1,28 @@ +## Why + +Reviewing wanderer's federation (2026-07-06, `docs/inspirations.md`) showed exactly which operational gaps make an ActivityPub implementation fragile in practice: fire-and-forget delivery, no replay defense, no moderation tooling. trails has one of the same gaps today — the `social-federation` spec promises deliveries "retrying with exponential backoff… after a documented retry budget", but the journal runs Fedify with `InProcessMessageQueue` (`federation.server.ts:156`), so every queued delivery and retry is **lost on container restart** (and restarts are routine: deploys, the OOM/restart-loop history). Before `route-federation` builds mirrors and cross-instance editing on this foundation, delivery must be durable, inbound processing replay-safe, and instances blockable. + +## What Changes + +- **Durable delivery queue**: replace `InProcessMessageQueue` with a PostgreSQL-backed Fedify `MessageQueue` (built on the existing pg-boss infrastructure), so queued activities and their retry state survive restarts — closing the gap with what the spec already promises. +- **Inbound replay/dedup defense**: processed remote activity IDs are recorded (TTL-bounded) and duplicates are dropped idempotently; complements the existing `remoteOriginIri` unique constraint which only covers ingested Creates. +- **Instance blocklist**: a `federation_blocked_instances` table (domain, reason) enforced at all three boundaries — inbox processing, delivery enqueue, and outbox polling. Managed via documented SQL/ops procedure for v1 (admin UI out of scope). +- **Published protocol documentation**: a root `FEDERATION.md` (the fediverse convention) documenting objects, activities, addressing, retry semantics, NodeInfo identification, and moderation behavior — wanderer's `federation.md` with real JSON examples is the model. +- **Delivery observability**: metrics for delivery success/failure/retry counts and queue depth, on the existing Grafana journal dashboard. +- Not in scope: allowlist-mode federation, admin moderation UI, per-user (as opposed to per-instance) blocks, and object-model changes (that's `route-federation`). + +## Capabilities + +### New Capabilities +- `federation-operations`: The operational layer of federation — durable delivery, replay defense, instance blocking, protocol documentation, and observability. + +### Modified Capabilities +- `social-federation`: the "Push delivery on local activity create" requirement is strengthened to require queue persistence across process restarts. + +## Impact + +- `apps/journal/app/lib/federation.server.ts` — queue swap; new `federation-queue.server.ts` (Fedify `MessageQueue` over pg-boss) beside the existing `PostgresKvStore`. +- `packages/db` — `federation_blocked_instances` + processed-activity dedup table (TTL sweep via existing jobs worker). +- Inbox/outbound/poll paths — blocklist + dedup checks. +- `FEDERATION.md` (root) + docs link; journal Grafana dashboard panels. +- `route-federation` (in flight) builds on this; noted there as a dependency. diff --git a/openspec/changes/federation-hardening/specs/federation-operations/spec.md b/openspec/changes/federation-hardening/specs/federation-operations/spec.md new file mode 100644 index 0000000..b86e2c0 --- /dev/null +++ b/openspec/changes/federation-hardening/specs/federation-operations/spec.md @@ -0,0 +1,40 @@ +## ADDED Requirements + +### Requirement: Durable federation queue +Fedify's message queue SHALL be backed by PostgreSQL so that queued activities, inbox processing tasks, and retry state survive process restarts and deploys. + +#### Scenario: Delivery survives a restart +- **WHEN** deliveries are queued and the journal container restarts before they complete +- **THEN** the deliveries execute after restart without loss + +#### Scenario: Retry state persists +- **WHEN** a delivery has failed twice with backoff pending and the process restarts +- **THEN** the retry continues on schedule rather than starting over or disappearing + +### Requirement: Inbound replay defense +The Journal SHALL record processed inbound activity IRIs (retained at least 30 days) and SHALL drop duplicate activities idempotently before any side effects. + +#### Scenario: Replayed activity is a no-op +- **WHEN** the same signed Like activity is delivered twice +- **THEN** the second delivery produces no state change and is counted as a dropped duplicate + +### Requirement: Instance blocklist +The Journal SHALL support blocking federation instances by domain, enforced at inbox processing (activities dropped without an error oracle), delivery enqueue (recipients filtered), and outbox polling/actor fetch (refused). Blocking SHALL be manageable via a documented operator procedure. + +#### Scenario: Blocked instance is inert in both directions +- **WHEN** a domain is added to the blocklist +- **THEN** its activities are silently dropped, no deliveries are sent to it, and its actors/outboxes are not fetched + +### Requirement: Published federation protocol documentation +The repository SHALL contain a `FEDERATION.md` documenting actor discovery, object and activity types with JSON examples, addressing rules, signature and deduplication expectations, delivery retry policy, and moderation semantics, kept current as federation capabilities change. + +#### Scenario: Third-party implementer can interoperate +- **WHEN** another project implements against `FEDERATION.md` +- **THEN** the documented flows (follow, create fan-out, dedup) behave as described + +### Requirement: Federation delivery observability +The Journal SHALL expose metrics for delivery outcomes, queue depth, and inbox drops (duplicate/blocked), surfaced on the monitoring dashboard. + +#### Scenario: Operator sees delivery health +- **WHEN** deliveries to an unreachable instance keep failing +- **THEN** failure counts and queue depth are visible on the dashboard diff --git a/openspec/changes/federation-hardening/specs/social-federation/spec.md b/openspec/changes/federation-hardening/specs/social-federation/spec.md new file mode 100644 index 0000000..ed8b23b --- /dev/null +++ b/openspec/changes/federation-hardening/specs/social-federation/spec.md @@ -0,0 +1,16 @@ +## MODIFIED Requirements + +### Requirement: Push delivery on local activity create +The Journal SHALL deliver a `Create(Note)` activity to every accepted remote follower's inbox when a local user with `profile_visibility = 'public'` creates a new `public` activity. Delivery queueing and retry state SHALL be persistent: queued deliveries and scheduled retries survive process restarts. + +#### Scenario: New public activity fans out +- **WHEN** a local user with N accepted remote followers creates a new public activity +- **THEN** N delivery jobs are enqueued (one per follower's inbox), each retrying with exponential backoff on 5xx, giving up after a documented retry budget + +#### Scenario: Rate-limited per remote host +- **WHEN** multiple deliveries target the same remote host +- **THEN** they are rate-limited so we never exceed 1 request per second per remote host (configurable; chosen for safety, not throughput) + +#### Scenario: Fan-out survives a deploy +- **WHEN** a deploy restarts the journal while fan-out deliveries are queued or awaiting retry +- **THEN** the deliveries complete after the restart without loss diff --git a/openspec/changes/federation-hardening/tasks.md b/openspec/changes/federation-hardening/tasks.md new file mode 100644 index 0000000..cc3d6b2 --- /dev/null +++ b/openspec/changes/federation-hardening/tasks.md @@ -0,0 +1,27 @@ +## 1. Durable queue + +- [ ] 1.1 Implement `federation-queue.server.ts`: Fedify `MessageQueue` over pg-boss (enqueue with delay, listen loop), mirroring the `PostgresKvStore` adapter pattern +- [ ] 1.2 Swap `InProcessMessageQueue` for it in `federation.server.ts`; document the two-layer queueing (fan-out jobs + Fedify queue) in a code comment +- [ ] 1.3 Tests: enqueue/delay/consume roundtrip; simulated restart (new listener picks up previously enqueued messages) + +## 2. Replay defense + +- [ ] 2.1 Add `federation_processed_activities` table (activity IRI PK, received_at) + migration; insert-or-drop check in inbox handlers before side effects +- [ ] 2.2 30-day TTL sweep in the jobs worker +- [ ] 2.3 Tests: duplicate Like/Delete/Update dropped as no-ops; Create double-delivery still covered by remoteOriginIri constraint + +## 3. Blocklist + +- [ ] 3.1 Add `federation_blocked_instances` table + migration; exact-host check helper +- [ ] 3.2 Enforce at inbox (silent 202 drop + counter), delivery enqueue (filter recipients), and outbox poll/actor fetch (refuse) +- [ ] 3.3 Document the operator procedure (SQL insert/delete) in the ops docs; tests for all three boundaries + +## 4. Protocol doc & observability + +- [ ] 4.1 Write `FEDERATION.md` (repo root): NodeInfo, actors/WebFinger, object + activity types with JSON examples, addressing, signatures + dedup expectations, retry policy, moderation semantics; link from README and docs +- [ ] 4.2 Add `federation_delivery_total{outcome}`, `federation_queue_depth`, `federation_inbox_dropped_total{reason}` metrics + journal dashboard row + +## 5. Verification + +- [ ] 5.1 Staging check: queue a fan-out, restart the journal container, confirm deliveries complete; block a test domain and verify both directions +- [ ] 5.2 Run `pnpm typecheck && pnpm lint && pnpm test && pnpm test:e2e` diff --git a/openspec/changes/fit-parsing-hardening/.openspec.yaml b/openspec/changes/fit-parsing-hardening/.openspec.yaml new file mode 100644 index 0000000..dd9a1d9 --- /dev/null +++ b/openspec/changes/fit-parsing-hardening/.openspec.yaml @@ -0,0 +1,2 @@ +schema: spec-driven +created: 2026-07-06 diff --git a/openspec/changes/fit-parsing-hardening/design.md b/openspec/changes/fit-parsing-hardening/design.md new file mode 100644 index 0000000..2a54e2f --- /dev/null +++ b/openspec/changes/fit-parsing-hardening/design.md @@ -0,0 +1,48 @@ +## Context + +`fitToGpx(buffer, name)` uses `fit-file-parser` (`force: true`), reads only `parsed.records`, filters on presence of lat/lon, and emits `generateGpx({ tracks: [trackPoints] })` — one segment. The parser also exposes `sessions` (with `start_time`, `total_timer_time`, `sport`, `sub_sport`) and `events` (`event: 'timer'`, `event_type: 'stop_all' | 'start'`), which we currently discard. Downstream, `movingTime` in `@trails-cool/gpx` treats segment boundaries as hard breaks and per-interval gaps > 60 s as pauses — but only if segments/timestamps are honest. GPX validation requires ≥2 points. + +Endurain's `utils_fit.py` informs the quirk list: enhanced fields preferred, per-session record slicing, cursor resets at session boundaries so speed doesn't bridge gaps, coordinate conversion sanity, zero-sentinel handling. + +## Goals / Non-Goals + +**Goals:** +- Honest GPX out of every FIT file trails will realistically receive (Wahoo, Garmin, COROS): pauses and sessions become segment boundaries; corrected fields win; garbage records can't poison stats. +- One conversion module stays the single seam for all providers, with a richer but backward-compatible return shape. + +**Non-Goals:** +- Sensor streams (HR/power/cadence/temp) — no journal model for them; revisit if activity streams ever land. +- Laps, structured workouts, developer fields, monitoring/wellness messages. +- Splitting multisport files into multiple *activities* (Endurain does; trails keeps one activity per provider workout — a triathlon imports as one activity with three segments). + +## Decisions + +### 1. Segmentation from events first, gaps second + +Split segments on `timer stop_all` → next `start` pairs from the events stream. Files whose events are missing/unreliable (some devices) get a fallback: a record-to-record timestamp gap > 5 min starts a new segment. Both thresholds as named constants. Rationale: events are the device's own truth; the gap fallback matches `movingTime`'s MAX_GAP philosophy at a coarser grain and catches event-less files. + +### 2. One activity, one `` per session + +Records are sliced into sessions by `[start_time, start_time + total_elapsed_time]` windows (Endurain's `split_records_by_activity` approach), each becoming a segment (further split by pauses within). Keeping one *activity* preserves trails' one-workout-one-activity invariant and avoids inventing a multi-activity import UX; segment boundaries are enough to keep stats honest. + +### 3. Validation gates mirror the GPX-parser-robustness rules + +Coordinates must be finite and in range or the record is dropped; altitude non-finite → `undefined` (point kept); record without timestamp → dropped (FIT records are timestamped by spec; absence means corruption). This intentionally matches the policies in the `gpx-parser-robustness` change so both import formats behave identically downstream. + +### 4. Return shape: `{ gpx, sport }` instead of `string` + +`fitToGpx` returns `{ gpx: string | null, sport: SportType | null }` where sport maps FIT `sport`/`sub_sport` (session-level, first session wins) through a small table to trails' `SPORT_TYPES` (running→run, trail_running sub-sport→run, cycling→ride, gravel_cycling→gravel, mountain→mtb, hiking→hike, walking→walk, alpine/backcountry ski→ski, else→other/null). Call sites destructure; Wahoo's existing workout-type mapping stays as fallback when no FIT file exists. + +## Risks / Trade-offs + +- [fit-file-parser may not surface events/sessions uniformly across devices] → Fixture-driven: gather real files (Wahoo export, Garmin sample) before coding; the gap fallback covers event-less files. +- [Multi-segment GPX changes stats for re-imported activities] → Only new imports are affected (no reprocessing); moving time gets *more* accurate. +- [Sport mapping disagreements with providers' own type fields] → Provider-specific mapping wins where the provider sends an explicit type; FIT sport is the fallback, not the override — call-site rule documented in code. + +## Migration Plan + +Library-shaped change inside the journal app; ships with normal deploy; no schema change; rollback = revert. Existing activities untouched. + +## Open Questions + +- None blocking. Whether COROS FIT files carry quirks beyond this set is checked when that integration starts (fixtures first). diff --git a/openspec/changes/fit-parsing-hardening/proposal.md b/openspec/changes/fit-parsing-hardening/proposal.md new file mode 100644 index 0000000..cd5a1ef --- /dev/null +++ b/openspec/changes/fit-parsing-hardening/proposal.md @@ -0,0 +1,27 @@ +## Why + +The shared FIT→GPX converter (`apps/journal/app/lib/connected-services/fit.ts`) is 40 lines: it takes every record with coordinates and emits one flat track segment. It ignores pause/resume events (moving time and speed bridge lunch breaks), ignores sessions (a multisport file becomes one merged track), passes device altitude through without preferring the corrected `enhanced_altitude`, and does no coordinate sanity checks. Wahoo already feeds it; the Garmin importer (in flight) and COROS (planned) will too, multiplying every gap. Endurain's 1,100-line FIT handler (reviewed 2026-07-06) is a catalog of exactly which quirks matter in the wild — we adopt the subset relevant to trails' GPX-centric model. + +## What Changes + +- **Pause-aware segmentation**: FIT `event` stop/start pairs (and, as fallback, record gaps > 5 minutes) split the output into multiple ``s, so downstream moving-time and instant-speed math never bridges a pause. +- **Session-aware output**: multi-session files (multisport) produce one `` per session with records sliced to each session's time window; single-session behavior unchanged. +- **Field preference and validation**: prefer `enhanced_altitude`/`enhanced_speed` over base fields; drop records with non-finite or out-of-range coordinates or non-finite altitude (altitude → undefined, record kept); require a timestamp per record. +- **Sport extraction**: expose the file's session sport/sub-sport mapped to trails' `SportType` so importers stop guessing (Wahoo importer currently maps workout-type codes itself; it can consume this when a FIT file is present). +- Not in scope: HR/cadence/power/temperature streams (trails doesn't model them), lap extraction, developer fields, FIT *writing* (unchanged in `@trails-cool/fit`). + +## Capabilities + +### New Capabilities + + + +### Modified Capabilities +- `wahoo-import`: the "FIT to GPX conversion" requirement is extended with pause/session segmentation, enhanced-field preference, and record validation guarantees. + +## Impact + +- `apps/journal/app/lib/connected-services/fit.ts` — the converter grows the four behaviors above; returns optional sport metadata alongside the GPX. +- Wahoo importer/webhook + Garmin importer consume the richer return type (backward-compatible: GPX string still primary). +- Tests: fixture FIT files (pause, multisport, garbage-coordinate) added next to the module; Endurain's `backend/tests/.../test_utils_fit.py` catalog is the reference for cases. +- No schema or API changes. diff --git a/openspec/changes/fit-parsing-hardening/specs/wahoo-import/spec.md b/openspec/changes/fit-parsing-hardening/specs/wahoo-import/spec.md new file mode 100644 index 0000000..568c339 --- /dev/null +++ b/openspec/changes/fit-parsing-hardening/specs/wahoo-import/spec.md @@ -0,0 +1,35 @@ +## MODIFIED Requirements + +### Requirement: FIT to GPX conversion +The system SHALL convert FIT binary files to GPX format. The conversion logic lives at `apps/journal/app/lib/connected-services/fit.ts` — a provider-agnostic location shared across any future provider that produces FIT files (Garmin, Coros, etc.). Wahoo's importer and webhook both import from this shared module; they do not contain their own copy. The conversion SHALL be pause- and session-aware: timer stop/start events (or record gaps over 5 minutes in files without events) and session boundaries each start a new track segment. The converter SHALL prefer enhanced (corrected) altitude and speed fields, drop records with missing timestamps or non-finite/out-of-range coordinates, treat non-finite altitude as absent, and expose the file's session sport mapped to the Journal's sport types alongside the GPX. + +#### Scenario: Convert FIT with GPS data +- **WHEN** a FIT file contains GPS track records +- **THEN** track points with lat, lon, elevation, and ISO 8601 timestamps are extracted +- **AND** coordinates are used as-is from the FIT parser (which already converts semicircles to degrees) +- **AND** a valid GPX string is produced using `generateGpx` + +#### Scenario: FIT without GPS data +- **WHEN** a FIT file has no GPS records (e.g., indoor trainer workout) +- **THEN** the activity is created without GPX or geometry (stats only) + +#### Scenario: Workout without FIT file +- **WHEN** a workout has no file URL (e.g., aborted recording, third-party app data) +- **THEN** the activity is created without GPX or geometry + +#### Scenario: Pause becomes a segment boundary +- **WHEN** a FIT file contains a timer stop event followed by a start event 20 minutes later +- **THEN** the GPX contains two track segments split at the pause +- **AND** moving time computed downstream does not include the paused span + +#### Scenario: Multisport file keeps one activity with per-session segments +- **WHEN** a FIT file contains multiple sessions +- **THEN** one GPX is produced with one or more track segments per session, records sliced to each session's time window + +#### Scenario: Corrupt records dropped +- **WHEN** a FIT file contains records with out-of-range coordinates or missing timestamps +- **THEN** those records are excluded and the remaining track converts normally + +#### Scenario: Sport surfaced from the file +- **WHEN** a FIT file's session declares sport "cycling" with sub-sport "gravel_cycling" +- **THEN** the converter reports sport `gravel` for the importer to use when the provider sends no explicit type diff --git a/openspec/changes/fit-parsing-hardening/tasks.md b/openspec/changes/fit-parsing-hardening/tasks.md new file mode 100644 index 0000000..1b2ed77 --- /dev/null +++ b/openspec/changes/fit-parsing-hardening/tasks.md @@ -0,0 +1,17 @@ +## 1. Fixtures first + +- [ ] 1.1 Collect fixture FIT files under `apps/journal/app/lib/connected-services/__fixtures__/`: a real Wahoo export with a pause, a multisport (multi-session) file, an indoor no-GPS file, and a synthetic file with corrupt records (out-of-range coords, missing timestamps) — reference Endurain's `backend/tests/.../test_utils_fit.py` case list +- [ ] 1.2 Verify what `fit-file-parser` exposes for `events` and `sessions` on each fixture; note device gaps in a code comment + +## 2. Converter + +- [ ] 2.1 Segmentation: split on timer stop/start event pairs, fallback on record gaps > 5 min (named constants) +- [ ] 2.2 Session slicing: records windowed per session, one-or-more segments per session, single-session unchanged +- [ ] 2.3 Validation gates: finite/in-range coordinates, timestamp required, non-finite altitude → undefined; prefer `enhanced_altitude`/`enhanced_speed` +- [ ] 2.4 Return `{ gpx, sport }` with the FIT→SportType mapping table; update call sites (Wahoo importer + webhook, Garmin importer) — provider-explicit type wins, FIT sport is fallback +- [ ] 2.5 Unit tests over the fixtures: segment counts, pause exclusion from moving time (via `movingTime` on parsed output), corrupt-record drops, sport mapping, no-GPS returns null gpx + +## 3. Verification + +- [ ] 3.1 Re-run the Wahoo importer test suite; confirm existing single-session imports produce identical stats +- [ ] 3.2 Run `pnpm typecheck && pnpm lint && pnpm test` diff --git a/openspec/changes/gpx-parser-robustness/.openspec.yaml b/openspec/changes/gpx-parser-robustness/.openspec.yaml new file mode 100644 index 0000000..e089cfa --- /dev/null +++ b/openspec/changes/gpx-parser-robustness/.openspec.yaml @@ -0,0 +1,2 @@ +schema: spec-driven +created: 2026-07-05 diff --git a/openspec/changes/gpx-parser-robustness/design.md b/openspec/changes/gpx-parser-robustness/design.md new file mode 100644 index 0000000..f6b28c2 --- /dev/null +++ b/openspec/changes/gpx-parser-robustness/design.md @@ -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. ``/`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 ``/track-name fallback enriching metadata). + +**Non-Goals:** +- Track color / display extensions (nothing in trails renders per-track color). +- GPX 1.0, `` 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 `` 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 `` as tracks + +Each `` becomes one track segment appended after `` 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 ``/`` 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 `` merge + +`name`/`description` resolution order: `` first, else the first ``/``'s ``/``. A `` 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 ``. + +### 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. diff --git a/openspec/changes/gpx-parser-robustness/proposal.md b/openspec/changes/gpx-parser-robustness/proposal.md new file mode 100644 index 0000000..bad095a --- /dev/null +++ b/openspec/changes/gpx-parser-robustness/proposal.md @@ -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 `` produce `NaN` that survives the range check and poisons distance and gain/loss totals, and ``/`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 `` is treated as absent instead of `NaN`. +- **Route support**: ``/`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 ``/``/`` are used when `` has none, and `` 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 + + + +## Impact + +- `packages/gpx/src/parse.ts` — point/coordinate/elevation lenience, `` 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. diff --git a/openspec/changes/gpx-parser-robustness/specs/gpx-parsing/spec.md b/openspec/changes/gpx-parser-robustness/specs/gpx-parsing/spec.md new file mode 100644 index 0000000..043fc62 --- /dev/null +++ b/openspec/changes/gpx-parser-robustness/specs/gpx-parsing/spec.md @@ -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 `` 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 `` 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 `` element as a track segment, handling `rtept` identically to `trkpt` (including elevation, time, and invalid-point skipping), appended after any `` segments. + +#### Scenario: Route-only GPX imports +- **WHEN** a GPX file contains only a `` with valid `rtept` points and no `` +- **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 `` and `` 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 `