Fix takeoff slice bug on duplicate GPS timestamps#164
Open
pokle wants to merge 3 commits into
Open
Conversation
Adds GET/PUT /api/auth/preferences in the auth-api worker for syncing per-user app preferences and custom theme across devices. Storage is opaque JSON in a new D1 table; partial updates are atomic via a single ON CONFLICT statement so concurrent PUTs from two devices can't lose each other's writes. Bootstraps a vitest+miniflare test harness for auth-api (none existed) and covers the route with 21 cases including auth gating, validation, size cap, partial updates, CASCADE on user delete, and a concurrent-PUT test pinning the atomicity guarantee. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds a frontend sync layer that backs the existing localStorage-based preferences and theme with the cloud API. localStorage remains the synchronous read cache so startup has no flicker and anonymous users keep working as before. When signed in, the layer hydrates from cloud on import, debounces mutations (2s) into PUTs, and flushes any pending writes on pagehide via keepalive fetch. Existing users' localStorage prefs are uploaded once on first cloud-synced session. mapLocation is allowlisted as device-local: stripped on upload and preserved when cloud values overwrite localStorage during hydrate. Different devices have different screens; viewport sync would also generate noisy writes from continuous pan events. Bootstraps a vitest+jsdom harness for the frontend (none existed) with a localStorage polyfill that bypasses Node 22+'s broken built-in web storage shim. Covers the sync layer with 23 cases across hydrate quadrants, debounce coalescing, retry/4xx/keepalive behaviour, and the pagehide flush path. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
detectFlightEvents re-derived the takeoff fix index by scanning fixes for one whose timestamp matched the takeoff event. Cheap GPS loggers stall and emit consecutive fixes with identical timestamps, so the lookup could land on a fix earlier than the real takeoff and silently feed pre-takeoff data (e.g. a stalled climb) into thermal/glide detection, producing spurious events. The takeoff event already carries the correct fix index in its details — use that directly. Also two defensive cleanups in the same area: - Fix the lower-bound guard in evaluateTakeoffCriteria (was guarding the wrong boundary; harmless today because callers start at i=1). - Replace Math.max(...arr) / Math.min(...arr) in gap-scoring's scoreTask with maxBy/minBy from array-utils, matching the policy documented in array-utils.ts. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
detectFlightEventsusedfixes.findIndex(f => f.time.getTime() === takeoffEvent.time.getTime())to locate the takeoff fix. Cheap GPS loggers stall and emit consecutive fixes with identical timestamps, so the lookup can land on an earlier fix and silently feed pre-takeoff data into thermal/glide/altitude detection, producing spurious events. Read the index fromtakeoffEvent.details.fixIndexinstead, whichdetectTakeoffalready stores when it emits the event.evaluateTakeoffCriteriaguarded the upper bound (index < fixes.length - 1) while readingfixes[index - 1]. Harmless today because the only caller starts ati = 1, but the guard now protects the correct boundary (index >= 1).scoreTaskreplacesMath.max(...arr)/Math.min(...arr)withmaxBy/minByfromarray-utils, matching the policy already documented in array-utils.ts.Why
Surfaced during a code-quality review of
web/engine. The takeoff-slice bug is the only one with an observable failure on real-world IGC data — when triggered, it lets pre-takeoff climbs (e.g. a chairlift ride logged before launch) leak into thermal detection. The other two are consistency / robustness with no behavior change at realistic inputs.Test plan
web/engine/tests/event-detector-duplicate-timestamp.test.ts— constructs a track with a pre-takeoff 0.6 m/s climb and a duplicate timestamp at fix 0. Fails on the previous code (thermal_entryemitted at fix 9) and passes on the fix.bun test web/engine/tests/→ 407 / 407 pass.bunx tsc --noEmitinweb/engineclean.🤖 Generated with Claude Code