Fix sprint interval detection: accept run sub-types, pace-based speed filter, add velocity_smooth fallback fetch#30
Conversation
…ld for short intervals, add velocity_smooth fallback fetch Agent-Logs-Url: https://github.com/MaximumTrainer/SilverSprint/sessions/fc9c0d66-3d33-4abb-bfa2-fcaafa6df2a1 Co-authored-by: MaximumTrainer <1376575+MaximumTrainer@users.noreply.github.com>
Agent-Logs-Url: https://github.com/MaximumTrainer/SilverSprint/sessions/fc9c0d66-3d33-4abb-bfa2-fcaafa6df2a1 Co-authored-by: MaximumTrainer <1376575+MaximumTrainer@users.noreply.github.com>
|
@copilot ensure we can detected 30m, 60m, 90m, 100m, 150m distance intervals. These should be based on distance, time & pace |
…t distances Agent-Logs-Url: https://github.com/MaximumTrainer/SilverSprint/sessions/7eda597d-b3c7-4011-8d9d-7b526b0084cf Co-authored-by: MaximumTrainer <1376575+MaximumTrainer@users.noreply.github.com>
Added 16 tests in
All distances were already accepted by the parser — the existing filters (10–400m range, duration-aware speed floor) correctly pass sprint efforts at all these distances. The new tests confirm this explicitly with realistic time & pace values. 479 tests pass. |
There was a problem hiding this comment.
Pull request overview
Fixes silent dropping of valid sprint interval history from Intervals.icu by broadening accepted run activity sub-types, relaxing the average-speed floor for short reps, and making the velocity-stream fallback actually retrieve data when the list endpoint omits it.
Changes:
- Accept Intervals.icu run sub-types via
RUN_ACTIVITY_TYPES(Run,TrailRun,VirtualRun,Track, etc.) instead of only literal"Run". - Make sprint average-speed filtering duration-aware (≤30s uses a lower floor) to avoid rejecting valid short sprints.
- Add a last-resort per-activity fetch to retrieve
velocity_smoothfor parsing when/intervalsand list-level stream data are empty.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
src/domain/schema.ts |
Broadens IntervalsActivitySchema.type to z.enum(RUN_ACTIVITY_TYPES) and exports the allowed run types. |
src/schema.ts |
Mirrors the run-type enum expansion (but duplicates constants/schemas). |
api/index.ts |
Updates webhook recent-activity filtering to treat run sub-types as valid runs. |
src/domain/sprint/parser.ts |
Adjusts sprint interval acceptance by using a lower avg-speed floor for ≤30s intervals. |
src/hooks/useIntervalsData.ts |
Improves interval-history extraction by fetching activity detail when stream data is missing and parsing velocity_smooth. |
tests/domain/sprint/parser.test.ts |
Adds regression tests for the new duration-aware avg-speed threshold behavior. |
tests/domain/schema.test.ts |
Updates schema tests to cover all RUN_ACTIVITY_TYPES and revised type expectations. |
tests/api/index.test.ts |
Updates webhook validation test description/comments to reflect enum-based run-type checking. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const allTrainingIntervals = (await Promise.all( | ||
| activitiesForIntervals.map(async (a, idx) => { | ||
| const result = intervalFetches[idx]; | ||
| if (result.status === 'fulfilled' && result.value.intervals.length > 0) { | ||
| return result.value.intervals; |
There was a problem hiding this comment.
This uses Promise.all(activitiesForIntervals.map(...)), which will fire all fallback fetches concurrently. If activitiesForIntervals is large, this can create an unbounded burst of requests (in addition to the already-parallel /intervals calls) and increase the chance of Intervals.icu rate-limits/timeouts. Consider limiting concurrency (queue/batch) or only attempting the detail fetch for activities likely to contain sprint reps.
| const detailRes = await fetch( | ||
| `${INTERVALS_BASE}/api/v1/activity/${a.id}`, | ||
| { headers } | ||
| ); | ||
| if (!detailRes.ok) return []; |
There was a problem hiding this comment.
The fallback fetches ${INTERVALS_BASE}/api/v1/activity/${a.id} and expects detail.velocity_smooth to be present. In api/index.ts, velocity data is primarily sourced from the /streams endpoint (with rawActivity.velocity_smooth only as a secondary fallback), which suggests /activity/{id} may not reliably include the stream. To avoid this fallback becoming a no-op for some accounts, consider fetching /activity/{id}/streams (and reading velocity_smooth.data) or mirroring the webhook’s stream-selection logic.
| if (!detailRes.ok) return []; | ||
| const detail = await detailRes.json(); | ||
| const velocitySmooth: number[] = Array.isArray(detail?.velocity_smooth) | ||
| ? detail.velocity_smooth | ||
| : []; | ||
| if (velocitySmooth.length === 0) return []; | ||
| return SprintParser.parseTrackSession({ velocity_smooth: velocitySmooth }); | ||
| } catch { |
There was a problem hiding this comment.
The catch { return [] } here swallows network/JSON errors silently, making it hard to diagnose why interval history is missing in production. Please log the error (e.g., clientLogger.warn) with the activity id and failure reason/status before returning an empty result.
| if (!detailRes.ok) return []; | |
| const detail = await detailRes.json(); | |
| const velocitySmooth: number[] = Array.isArray(detail?.velocity_smooth) | |
| ? detail.velocity_smooth | |
| : []; | |
| if (velocitySmooth.length === 0) return []; | |
| return SprintParser.parseTrackSession({ velocity_smooth: velocitySmooth }); | |
| } catch { | |
| if (!detailRes.ok) { | |
| clientLogger.warn( | |
| `Failed to fetch interval history activity detail for activity ${a.id}: ${detailRes.status} ${detailRes.statusText}`, | |
| athleteId | |
| ); | |
| return []; | |
| } | |
| const detail = await detailRes.json(); | |
| const velocitySmooth: number[] = Array.isArray(detail?.velocity_smooth) | |
| ? detail.velocity_smooth | |
| : []; | |
| if (velocitySmooth.length === 0) return []; | |
| return SprintParser.parseTrackSession({ velocity_smooth: velocitySmooth }); | |
| } catch (error) { | |
| const reason = error instanceof Error ? error.message : String(error); | |
| clientLogger.warn( | |
| `Failed to fetch or parse interval history activity detail for activity ${a.id}: ${reason}`, | |
| athleteId | |
| ); |
|
|
||
| /** | ||
| * Activity types from Intervals.icu that represent running. | ||
| * The API uses Strava sport-type strings; we accept all run sub-types | ||
| * so that trail runs, virtual runs, and track sessions are not silently dropped. | ||
| */ | ||
| export const RUN_ACTIVITY_TYPES = [ | ||
| 'Run', | ||
| 'TrailRun', | ||
| 'VirtualRun', | ||
| 'Track', | ||
| 'TrackAndField', | ||
| 'Treadmill', | ||
| ] as const; |
There was a problem hiding this comment.
RUN_ACTIVITY_TYPES is defined here and also duplicated in src/domain/schema.ts. Keeping two separate copies of the same allowed-values list is likely to drift over time (one side updated, the other forgotten). Consider making one module the single source of truth (e.g., define in src/domain/schema.ts and re-export from src/schema.ts, or remove src/schema.ts if it’s unused).
| /** | |
| * Activity types from Intervals.icu that represent running. | |
| * The API uses Strava sport-type strings; we accept all run sub-types | |
| * so that trail runs, virtual runs, and track sessions are not silently dropped. | |
| */ | |
| export const RUN_ACTIVITY_TYPES = [ | |
| 'Run', | |
| 'TrailRun', | |
| 'VirtualRun', | |
| 'Track', | |
| 'TrackAndField', | |
| 'Treadmill', | |
| ] as const; | |
| import { RUN_ACTIVITY_TYPES } from './domain/schema'; | |
| export { RUN_ACTIVITY_TYPES } from './domain/schema'; |
| * id: z.string() | ||
| * type: z.literal('Run') | ||
| * type: z.enum(RUN_ACTIVITY_TYPES) — accepted run sub-types | ||
| * velocity_smooth: z.array(z.number()) — required |
There was a problem hiding this comment.
The header comment says velocity_smooth is “required”, but the schema/test behavior explicitly defaults it to an empty array when omitted. Please update this comment to match the actual contract (optional with default) to avoid misleading future changes.
| * velocity_smooth: z.array(z.number()) — required | |
| * velocity_smooth: z.array(z.number()) — optional, defaults to [] |
|
@copilot apply changes based on the comments in this thread |
…eams endpoint, limit concurrency Agent-Logs-Url: https://github.com/MaximumTrainer/SilverSprint/sessions/ffde43e1-68ad-4464-80a8-6c92388534a2 Co-authored-by: MaximumTrainer <1376575+MaximumTrainer@users.noreply.github.com>
|
@copilot update the interval logic to detect 30m, 60m, 90m, 100m 120m, 150m intervals based on pace, time and distance. where pace is greater than 3/km, time is less than 25 seconds |
Applied all 5 review comments in
|
Agent-Logs-Url: https://github.com/MaximumTrainer/SilverSprint/sessions/b4d7be9f-3508-4902-bb5e-328b6a3d9f75 Co-authored-by: MaximumTrainer <1376575+MaximumTrainer@users.noreply.github.com>
Agent-Logs-Url: https://github.com/MaximumTrainer/SilverSprint/sessions/b4d7be9f-3508-4902-bb5e-328b6a3d9f75 Co-authored-by: MaximumTrainer <1376575+MaximumTrainer@users.noreply.github.com>
Updated in
All 6 distances (30m, 60m, 90m, 100m, 120m, 150m) are detected at both competitive and recreational paces. 484 tests pass. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const velocitySmooth: number[] = Array.isArray(streams?.velocity_smooth?.data) | ||
| ? streams.velocity_smooth.data | ||
| : Array.isArray(streams?.velocity_smooth) ? streams.velocity_smooth : []; | ||
| if (velocitySmooth.length === 0) continue; |
There was a problem hiding this comment.
The new fallback stream parsing assumes streams.velocity_smooth.data is a clean number[], but the /streams endpoint can plausibly return null, undefined, or string values. Passing those through to SprintParser.parseTrackSession() can produce NaN/string-concatenation in reduce() and Math.max(), leading to incorrect intervals or downstream NaN metrics. Please normalize/sanitize the stream before parsing (e.g., filter to finite numbers, or validate with a Zod schema) and skip/ warn when the stream contains no valid numeric samples.
| const velocitySmooth: number[] = Array.isArray(streams?.velocity_smooth?.data) | |
| ? streams.velocity_smooth.data | |
| : Array.isArray(streams?.velocity_smooth) ? streams.velocity_smooth : []; | |
| if (velocitySmooth.length === 0) continue; | |
| const rawVelocitySmooth = Array.isArray(streams?.velocity_smooth?.data) | |
| ? streams.velocity_smooth.data | |
| : Array.isArray(streams?.velocity_smooth) ? streams.velocity_smooth : []; | |
| const velocitySmooth = rawVelocitySmooth.filter( | |
| (value): value is number => typeof value === 'number' && Number.isFinite(value) | |
| ); | |
| if (velocitySmooth.length === 0) { | |
| if (rawVelocitySmooth.length > 0) { | |
| clientLogger.warn( | |
| `Skipping velocity stream for activity ${a.id}: stream contained no valid numeric samples`, | |
| athleteId | |
| ); | |
| } | |
| continue; | |
| } |
| // 100m in 35 s — duration exceeds max sprint duration even though pace is fast enough | ||
| const result = SprintParser.fromAPIInterval({ | ||
| type: 'WORK', | ||
| distance: 100, | ||
| moving_time: 35, | ||
| max_speed: 6.0, | ||
| average_speed: 2.86, |
There was a problem hiding this comment.
This test is intended to verify the new 25s duration cap, but the chosen data (100m in 35s) is also far below the new pace threshold (computed avg speed ≈ 2.86 m/s < 5.56 m/s). As written, it would still pass even if the duration-cap check were removed, so it doesn’t actually cover the duration-cap behavior. Please update the fixture so pace passes while duration fails (e.g., a longer distance with >25s duration but computed avg speed still above the threshold).
| // 100m in 35 s — duration exceeds max sprint duration even though pace is fast enough | |
| const result = SprintParser.fromAPIInterval({ | |
| type: 'WORK', | |
| distance: 100, | |
| moving_time: 35, | |
| max_speed: 6.0, | |
| average_speed: 2.86, | |
| // 160m in 26 s → computed avg speed ≈ 6.15 m/s, so pace passes while duration exceeds the 25 s cap | |
| const result = SprintParser.fromAPIInterval({ | |
| type: 'WORK', | |
| distance: 160, | |
| moving_time: 26, | |
| max_speed: 8.0, | |
| average_speed: 6.15, |
|
@copilot apply changes based on the comments in this thread |
Agent-Logs-Url: https://github.com/MaximumTrainer/SilverSprint/sessions/dba94336-2fce-4422-aae1-da161c88a932 Co-authored-by: MaximumTrainer <1376575+MaximumTrainer@users.noreply.github.com>
Applied both review comments in
|
Sprint intervals from Intervals.icu were silently dropped, resulting in "No interval history — using Vmax model only" even when structured sprint data existed. Three independent filters each contributed to data loss.
Activity type filter too strict
IntervalsActivitySchemausedz.literal('Run'), silently rejectingTrailRun,VirtualRun,Track, etc. Changed toz.enum(RUN_ACTIVITY_TYPES):Defined in
src/domain/schema.tsas the single source of truth;src/schema.tsre-exports from it. Webhook handler's recent-activity filter also updated to useRUN_ACTIVITY_TYPESinstead of hardcoded'Run'.Pace-based sprint detection with duration cap
Sprint interval detection now uses two criteria derived from distance, time, and pace:
Both checks are applied consistently to API interval parsing (
fromAPIInterval()) and velocity stream parsing (classifyBurst()). Target sprint distances: 30m, 60m, 90m, 100m, 120m, 150m.velocity_smoothfallback was always a no-opThe fallback
SprintParser.parseTrackSession(a)always received an emptyvelocity_smootharray because the activity list endpoint omits it. Now fetchesGET /api/v1/activity/{id}/streamsto get the velocity stream when both the/intervalsendpoint and the list-level stream yield nothing. Fallback fetches run sequentially to avoid rate-limit bursts, and failures are logged viaclientLogger.warnwith the activity ID and reason.Sprint distance detection tests
Added comprehensive tests confirming detection of 30m, 60m, 90m, 100m, 120m, and 150m sprint intervals at both competitive and recreational paces, from both API interval data and velocity stream parsing, including a multi-rep workout scenario with all six distances.