Revert archive of atomic-gpx-save (will redo via skill)

openspec/changes/archive/2026-05-10-atomic-gpx-save/.openspec.yaml

@@ -1,2 +0,0 @@
-schema: spec-driven
-created: 2026-05-10

openspec/changes/archive/2026-05-10-atomic-gpx-save/design.md

@@ -1,101 +0,0 @@
-## Context
-
-Every write of a GPX track in the Journal follows a two-step pattern: insert/update the row, then call `setGeomFromGpx` to write the PostGIS `geom` column via a raw `UPDATE`. The two steps are not in a transaction. `setGeomFromGpx` wraps its error in a `console.error` and returns, leaving the row with `geom IS NULL`. This affects `createRoute`, `updateRoute`, `createActivity`, `createRouteFromActivity`, and `demo-bot.server.ts` (which bypasses the save functions entirely with raw inserts).
-
-`setGeomFromGpx` is currently exported from `routes.server.ts` and imported by `activities.server.ts` and `demo-bot.server.ts`, making it a de-facto shared utility that lives in the wrong module.
-
-## Goals / Non-Goals
-
-**Goals:**
-- `geom IS NULL` on a row that has `gpx` set is impossible through the normal save path.
-- GPX is validated (≥2 track points, valid coordinate ranges) before any DB write.
-- All callers fail loudly on invalid or unpersistable GPX — no silent degradation.
-- PostGIS write participates in the same `db.transaction()` as the row write and version snapshot.
-- `setGeomFromGpx` is internal to the new `gpx-save.server.ts` module; no other file calls it.
-- `demo-bot.server.ts` uses `createRoute` / `createActivity` and gets the invariant for free.
-
-**Non-Goals:**
-- No API surface changes.
-- No DB schema changes or migrations.
-- No changes to the `@trails-cool/gpx` parser.
-- No changes to how the Planner callback endpoint validates its authorization (JWT stays as-is); validation of the GPX payload is now handled inside `updateRoute`.
-- No changes to `getGeojson` / `getSimplifiedGeojsonBatch` — read paths are out of scope.
-- No fix for the O(N) `getSimplifiedGeojsonBatch` query pattern (separate concern).
-
-## Decisions
-
-### 1. New module: `gpx-save.server.ts`
-
-`setGeomFromGpx` belongs in neither `routes.server.ts` nor `activities.server.ts` — it is shared infrastructure. A dedicated `apps/journal/app/lib/gpx-save.server.ts` owns validation and geometry persistence. `routes.server.ts` and `activities.server.ts` both import from it.
-
-Exports:
-- `validateGpx(gpx: string): Promise<ParsedGpx>` — public
-- `writeGeom(tx, id, table, coords): Promise<void>` — internal (not exported)
-- `GpxValidationError` — public (callers catch it to return 400)
-
-The existing `setGeomFromGpx` export from `routes.server.ts` is removed. Any file that currently imports it must migrate.
-
-### 2. Validation inside save functions, not at call sites
-
-`validateGpx` is called at the start of `createRoute`, `updateRoute`, `createActivity`, and `createRouteFromActivity` — before any transaction is opened. This means:
-
-- Every caller gets validation for free, including the mobile API (`api.v1.routes`) and the Planner callback.
-- The callback endpoint does not need its own validation logic; it catches `GpxValidationError` and returns 400.
-- Future callers can't bypass validation by accident.
-
-Alternative considered: validate only at the callback boundary. Rejected because it leaves mobile API and direct DB writes unprotected.
-
-### 3. `db.transaction()` wrapping row write + geom write + version snapshot
-
-Drizzle supports `db.transaction(async (tx) => { ... })`. The raw PostGIS `UPDATE` inside `writeGeom` receives `tx` as its first argument and executes within the same transaction. If `writeGeom` throws, Drizzle rolls back the entire transaction — the row insert is reversed.
-
-```
-db.transaction(async (tx) => {
-  // 1. validateGpx already ran (before tx opened)
-  await tx.insert(routes).values(...)       // or update
-  await writeGeom(tx, id, "routes", coords) // throws → rollback
-  await tx.insert(routeVersions).values(...)
-})
-```
-
-The transaction is opened *after* `validateGpx` succeeds. This keeps the transaction as short as possible (no async GPX parsing inside a held transaction).
-
-### 4. `activities.server.ts` stat extraction uses `validateGpx` result
-
-Currently `createActivity` calls `parseGpxAsync` inline to extract `distance`, `elevationGain`, `elevationLoss`, `startedAt`. After this change, `validateGpx` returns the `ParsedGpx` object. `createActivity` uses that result directly — one parse, not two.
-
-The activity-specific fields (`startedAt` from `gpxData.tracks[0][0].time`) are still extracted by `createActivity`; `validateGpx` returns the full parsed object so callers have access to everything.
-
-### 5. `demo-bot.server.ts` migrates to `createRoute` / `createActivity`
-
-The demo-bot currently does:
-```ts
-await db.insert(routes).values({ ... distance, elevationGain, ... })
-await setGeomFromGpx(routeId, "routes", result.gpx)
-await db.insert(activities).values({ ... })
-await setGeomFromGpx(activityId, "activities", result.gpx)
-```
-
-After migration it calls `createRoute` and `createActivity`, which own all of this. The demo-bot passes pre-computed stats via `RouteInput` / `ActivityInput` (already supported: the input types accept optional `distance`, `elevationGain`, etc.). `createRoute` will prefer input stats over re-parsed stats when provided, avoiding redundant parsing for the demo-bot's case where BRouter already computed them.
-
-Alternative considered: leave demo-bot as-is and just make `setGeomFromGpx` throw. Rejected because it keeps a second bypass path alive and requires demo-bot to handle its own transaction.
-
-### 6. ADR recorded
-
-ADR-0006 documents that PostGIS geometry writes are always transactional with row writes, so future code doesn't re-introduce the split pattern.
-
-## Risks / Trade-offs
-
-- **Transaction duration**: Opening a DB transaction and then doing a raw SQL UPDATE adds a short hold on the row. For typical GPX sizes this is negligible — `validateGpx` runs before the transaction opens, so the held time is just the two DB statements.
-- **demo-bot stat re-computation**: `createRoute` will call `validateGpx` on GPX that the demo-bot already parsed. Minor redundancy; acceptable given demo-bot is not a hot path.
-- **Existing NULL geom rows**: Rows already in production with `geom IS NULL` are not backfilled by this change. A separate data-repair script would be needed. Out of scope.
-- **`GpxValidationError` surfaces to users**: Routes/activities that previously saved silently will now return errors. This is the intended behaviour (fail loudly), but any caller that didn't expect an exception from `updateRoute` must now handle `GpxValidationError`. All current call sites are audited in the tasks.
-
-## Migration Plan
-
-No DB migration required. The change is purely in application code. Deploy is a standard app rollout — no staged migration, no feature flag.
-
-Existing rows with `geom IS NULL` remain as-is (pre-existing data debt). A follow-up query can identify and optionally repair them:
-```sql
-SELECT id FROM journal.routes WHERE gpx IS NOT NULL AND geom IS NULL;
-```

openspec/changes/archive/2026-05-10-atomic-gpx-save/proposal.md

@@ -1,32 +0,0 @@
-## Why
-
-Route and activity rows can currently be saved with `geom IS NULL` when the PostGIS geometry write fails after the row insert succeeds — the error is swallowed silently with a `console.error`. This means missing map thumbnails, broken route-push deduplication, and corrupted spatial queries with no signal to the user or operator. The Planner callback endpoint also accepts GPX from an external source with no validation before writing.
-
-## What Changes
-
-- **New `gpx-save.server.ts` module** owns GPX validation and atomic geometry persistence; `setGeomFromGpx` moves here and becomes internal.
-- **`validateGpx(gpx)`** parses the GPX string, checks ≥2 track points and valid coordinate ranges, throws `GpxValidationError` on failure, and returns the parsed result so callers don't re-parse.
-- **`createRoute`, `updateRoute`, `createActivity`, `createRouteFromActivity`** all wrap their DB writes (row insert/update + geom write + version snapshot) in `db.transaction()` — partial state (row exists, geom NULL) is no longer possible.
-- **`setGeomFromGpx` stops swallowing errors** — it throws, causing the transaction to roll back.
-- **`demo-bot.server.ts`** migrated from raw inserts + `setGeomFromGpx` to `createRoute` / `createActivity`, eliminating a bypass of the new invariant.
-- **`activities.server.ts`** removes inline `parseGpxAsync` call; uses the `ParsedGpx` returned by `validateGpx` for stat extraction instead.
-- **ADR recorded**: PostGIS geometry writes are always transactional with row writes.
-
-## Capabilities
-
-### New Capabilities
-
-- `gpx-save`: Atomic GPX validation and PostGIS geometry persistence for routes and activities.
-
-### Modified Capabilities
-
-*(none — no user-visible requirement changes; this is an implementation-layer correctness fix)*
-
-## Impact
-
-- `apps/journal/app/lib/routes.server.ts` — `setGeomFromGpx` export removed; callers updated.
-- `apps/journal/app/lib/activities.server.ts` — inline `parseGpxAsync` removed; imports from `gpx-save`.
-- `apps/journal/app/lib/demo-bot.server.ts` — raw insert pattern replaced with `createRoute` / `createActivity`.
-- `apps/journal/app/routes/api.routes.$id.callback.ts` — no validation changes needed (validation now inside `updateRoute`); GPX errors surface as thrown exceptions → callers return 400.
-- `@trails-cool/gpx` — no changes; `parseGpxAsync` is still the underlying parser.
-- No API surface changes, no schema changes, no migration required.

openspec/changes/archive/2026-05-10-atomic-gpx-save/specs/gpx-save/spec.md

@@ -1,83 +0,0 @@
-# gpx-save Specification
-
-## Purpose
-
-Atomic GPX validation and PostGIS geometry persistence for routes and activities. Every write of a GPX track in the Journal SHALL validate the track and persist the row + geometry in a single transaction. Silent geometry failures are not permitted.
-
-## Requirements
-
-### Requirement: GPX validation before any DB write
-
-The Journal SHALL validate any GPX string before writing a route or activity row. Validation SHALL confirm the GPX parses successfully, contains at least 2 track points, and all track points have coordinates within valid ranges (latitude −90..90, longitude −180..180). Validation SHALL throw `GpxValidationError` on failure.
-
-#### Scenario: Valid GPX proceeds to save
-
-- **WHEN** a caller passes a GPX string with ≥2 track points and valid coordinates to `createRoute`, `updateRoute`, `createActivity`, or `createRouteFromActivity`
-- **THEN** the save proceeds and no `GpxValidationError` is thrown
-
-#### Scenario: GPX with fewer than 2 track points is rejected
-
-- **WHEN** a caller passes a GPX string that produces fewer than 2 track points
-- **THEN** `GpxValidationError` is thrown before any DB write occurs
-
-#### Scenario: GPX with out-of-range coordinates is rejected
-
-- **WHEN** a caller passes a GPX string containing coordinates outside valid ranges
-- **THEN** `GpxValidationError` is thrown before any DB write occurs
-
-#### Scenario: Unparseable GPX is rejected
-
-- **WHEN** a caller passes a malformed GPX string that cannot be parsed
-- **THEN** `GpxValidationError` is thrown before any DB write occurs
-
-### Requirement: Atomic row + geometry persistence
-
-Every route or activity save that includes a GPX string SHALL wrap the row write, PostGIS geometry write, and version snapshot in a single database transaction. If any step fails, the entire transaction SHALL be rolled back — a route or activity row with a non-null `gpx` column and a null `geom` column SHALL NOT be possible through the normal save path.
-
-#### Scenario: Successful save stores row and geometry atomically
-
-- **WHEN** `createRoute` is called with valid GPX
-- **THEN** the `routes` row, the `geom` column update, and the `routeVersions` row are all committed in a single transaction
-
-#### Scenario: PostGIS failure rolls back the row insert
-
-- **WHEN** the PostGIS geometry write fails during a `createRoute` call
-- **THEN** the transaction is rolled back and no `routes` row is persisted
-
-#### Scenario: Route update is atomic
-
-- **WHEN** `updateRoute` is called with valid GPX
-- **THEN** the row update, geometry update, and new version snapshot are committed atomically
-
-#### Scenario: Activity save is atomic
-
-- **WHEN** `createActivity` is called with valid GPX
-- **THEN** the `activities` row and `geom` column update are committed atomically
-
-### Requirement: Loud failure on geometry errors
-
-The Journal SHALL NOT swallow PostGIS geometry write errors. Any failure during the geometry write SHALL propagate as a thrown exception, causing the enclosing transaction to roll back and the error to surface to the caller.
-
-#### Scenario: Geometry write error surfaces to caller
-
-- **WHEN** the PostGIS `UPDATE geom` statement fails
-- **THEN** an exception is thrown, the transaction is rolled back, and the caller receives the error
-
-#### Scenario: demo-bot route creation fails loudly
-
-- **WHEN** `createRoute` is called from the demo-bot and the PostGIS write fails
-- **THEN** the exception propagates — no partial route row is committed
-
-### Requirement: Single gpx-save module owns geometry persistence
-
-The Journal SHALL have exactly one module responsible for GPX validation and PostGIS geometry writes: `apps/journal/app/lib/gpx-save.server.ts`. No other module SHALL call PostGIS geometry update statements directly or implement GPX validation independently.
-
-#### Scenario: activities.server.ts uses gpx-save for geometry
-
-- **WHEN** `createActivity` or `createRouteFromActivity` writes geometry
-- **THEN** the write goes through the `gpx-save` module, not an inline raw SQL statement
-
-#### Scenario: demo-bot uses createRoute and createActivity
-
-- **WHEN** the demo-bot creates synthetic routes and activities
-- **THEN** it calls `createRoute` and `createActivity` rather than issuing raw inserts and calling `setGeomFromGpx` directly

openspec/changes/archive/2026-05-10-atomic-gpx-save/tasks.md

@@ -1,39 +0,0 @@
-## 1. New gpx-save module
-
-- [x] 1.1 Create `apps/journal/app/lib/gpx-save.server.ts` with `GpxValidationError` class and `validateGpx(gpx: string): Promise<ParsedGpx>` — parses, checks ≥2 track points, checks coordinate ranges, throws on failure, returns parsed result
-- [x] 1.2 Move `setGeomFromGpx` logic into `gpx-save.server.ts` as an internal `writeGeom(tx, id, table, coords)` function that accepts a Drizzle transaction client and throws on failure (no try/catch)
-- [x] 1.3 Write unit tests in `gpx-save.server.test.ts` covering: valid GPX passes, <2 track points throws, out-of-range coordinates throw, unparseable GPX throws
-
-## 2. Migrate routes.server.ts
-
-- [x] 2.1 Import `validateGpx` and `writeGeom` from `gpx-save.server.ts`; remove local `setGeomFromGpx` implementation and its export
-- [x] 2.2 Wrap `createRoute` in `db.transaction()`: validate GPX → insert row → `writeGeom` → insert `routeVersions`, all within the transaction client
-- [x] 2.3 Wrap `updateRoute` in `db.transaction()`: validate GPX → update row → `writeGeom` → insert new `routeVersions`, all within the transaction client
-- [x] 2.4 Verify `computeRouteStats` is called with the `ParsedGpx` returned by `validateGpx` (avoid re-parsing)
-
-## 3. Migrate activities.server.ts
-
-- [x] 3.1 Replace inline `parseGpxAsync` call in `createActivity` with `validateGpx`; extract stats from the returned `ParsedGpx`
-- [x] 3.2 Wrap `createActivity` in `db.transaction()`: validate GPX → insert row → `writeGeom`, all within the transaction client
-- [x] 3.3 Wrap `createRouteFromActivity` in `db.transaction()`: insert route row → `writeGeom` → update activity's `routeId`, all within the transaction client
-- [x] 3.4 Remove `import { setGeomFromGpx } from "./routes.server.ts"` — no longer needed
-
-## 4. Migrate demo-bot.server.ts
-
-- [x] 4.1 Replace the raw `db.insert(routes).values(...)` + `setGeomFromGpx(routeId, ...)` block with a `createRoute(ownerId, input)` call; pass pre-computed stats via `RouteInput` fields so `createRoute` doesn't re-parse BRouter's output
-- [x] 4.2 Replace the raw `db.insert(activities).values(...)` + `setGeomFromGpx(activityId, ...)` block with a `createActivity(ownerId, input)` call
-- [x] 4.3 Remove `import { setGeomFromGpx } from "./routes.server.ts"` from `demo-bot.server.ts`
-
-## 5. Callback endpoint error handling
-
-- [x] 5.1 In `api.routes.$id.callback.ts`, catch `GpxValidationError` from `updateRoute` and return a 400 response with the validation message (currently the catch block returns 401 for all errors — narrow it)
-
-## 6. ADR
-
-- [x] 6.1 Write `docs/adr/0006-atomic-gpx-save.md` documenting that PostGIS geometry writes are always transactional with row writes and that `gpx-save.server.ts` is the sole owner of geometry persistence
-
-## 7. Verification
-
-- [x] 7.1 Run `pnpm typecheck` — no new type errors
-- [x] 7.2 Run `pnpm test` — all existing tests pass; new `gpx-save.server.test.ts` tests pass
-- [x] 7.3 Confirm `setGeomFromGpx` is no longer exported from any file (`grep -r "setGeomFromGpx" apps/journal/app` returns no results outside `gpx-save.server.ts`)