Fix takeoff slice bug on duplicate GPS timestamps#164
Merged
Conversation
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>
pokle
force-pushed
the
claude/dazzling-clarke-c2d99b
branch
from
a6acaaa to
a5433c1
Compare
|
Preview Deployment |
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 run test→ 412 / 412 pass.bun run typecheck:allclean.