Use timestamp windows for activity power curve by Asherlc · Pull Request #1569 · Asherlc/dofek

Asherlc · 2026-07-09T00:59:48Z

Summary

Rework activity_power_curve to use elapsed timestamp windows instead of inferred sample-count windows
Reject duration windows with large internal sample gaps so sparse/gapped activities do not produce misleading best-power rows
Add executable ClickHouse integration coverage for regular and gapped power fixtures

Validation

pnpm lint
pnpm typecheck
pnpm vitest run --project unit analytics/models/read_models/activity_power_curve.sql.test.ts packages/server/src/repositories/power-repository.test.ts
pnpm vitest run --project integration packages/server/src/routers/activity-power-curve-read-model.integration.test.ts

Summary by cubic

Compute best-power using timestamp-based windows in activity_power_curve and time-weighted average power for accurate results with irregular samples. Fixes Linear #1532.

Bug Fixes
- Use elapsed timestamp windows and time-weighted avg power (by segment duration) instead of naive avg(power).
- Drop windows when the max internal gap exceeds a per-activity threshold (>= 5s or 2x median segment).
- Prevent segment duplication in averages by grouping on window_sample.row_number; add ClickHouse integration tests (regular, gapped, and 5s varying-power filtered by activity_id).

^{Written for commit 2e5e228. Summary will update on new commits.}

Summary by CodeRabbit

Bug Fixes
- Improved activity power curve calculations to use actual elapsed time between samples, producing more accurate duration windows and best-power values.
- Better handles activities with gaps by filtering out windows that exceed allowed continuity limits.
Tests
- Added integration coverage for regular, gapped, and varying-power activities to verify correct windowing and result accuracy.

cursor · 2026-07-09T00:59:52Z

Bugbot is not enabled for your account, so this pull request was not reviewed.

Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs.

qodo-code-review · 2026-07-09T00:59:52Z

Qodo reviews are paused for this user.

Troubleshooting steps vary by plan Learn more →

On a Teams plan?
Reviews resume once this user has a paid seat and their Git account is linked in Qodo.
Link Git account →

Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center?
These require an Enterprise plan - Contact us
Contact us →

sourcery-ai

Sorry @Asherlc, you have reached your weekly rate limit of 500000 diff characters.

Please try again later or upgrade to continue using Sourcery

Copilot

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

greptile-apps

Asherlc has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.

coderabbitai · 2026-07-09T00:59:56Z

📝 Walkthrough

Walkthrough

The activity power curve dbt model is reworked to compute duration windows from per-sample timestamp segments rather than fixed sample-rate assumptions, deriving a per-activity gap threshold and filtering candidate windows by elapsed time and max gap. A new integration test suite validates the read model against regular, gapped, and varying-power activity fixtures.

Changes

Timestamp-based power curve windowing

Layer / File(s)	Summary
Per-sample segment and gap threshold `analytics/models/read_models/activity_power_curve.sql`	Adds `power_segments` computing per-segment durations between consecutive positive-power samples, and `sample_gap_stats` deriving a per-activity max continuous gap threshold (min 5s) from median segment duration.
Candidate and final duration windows `analytics/models/read_models/activity_power_curve.sql`	Replaces inline duration selection with `duration_values`, rebuilds window generation as `candidate_duration_windows` via cross-join with contiguous sequencing to compute `elapsed_seconds`/`max_gap_seconds`/`avg_power`, and filters to final `duration_windows` meeting duration coverage and gap threshold.
Integration test suite `packages/server/src/routers/activity-power-curve-read-model.integration.test.ts`	Adds helpers to render the SQL fixture and seed ClickHouse power samples/Postgres activities, plus tests asserting `best_power` correctness for regular, gapped, and varying-power activities using elapsed timestamp duration.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Test
  participant ActivitySensorStore
  participant ClickHouse
  participant ReadModelSQL

  Test->>Test: insertActivity into fitness.activity
  Test->>ClickHouse: insert powerSampleRows
  Test->>ActivitySensorStore: sync
  Test->>ReadModelSQL: query rendered activity_power_curve.sql
  ReadModelSQL-->>Test: best_power, is_deleted

Assessment against linked issues

Objective	Addressed	Explanation
Rework `activity_power_curve.sql` to calculate windows from timestamp duration rather than sample-count assumptions (`#1532`)	✅
Add executable tests with irregular and missing sample fixtures, covering at least one regular and one gapped activity (`#1532`)	✅
Validate serving repository still reads deduped read model and preserves tombstone behavior (`#1532`)	❓	Only `is_deleted` for the regular activity is asserted; broader dedupe/tombstone flow through the serving repository is not exercised.

Possibly related PRs

Asherlc/dofek#1531: Introduced the initial power-curve window computation in the same activity_power_curve.sql file that this PR reworks.

Suggested labels: area/server, type/bug

Suggested reviewers: cubic-dev-ai

🚥 Pre-merge checks | ✅ 1 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Title check	⚠️ Warning	The title is clear and imperative, but it omits the required area prefix for this cross-cutting change.	Prefix it with the relevant area, e.g. "[server] Use timestamp windows for activity power curve".

✅ Passed checks (1 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

github-actions · 2026-07-09T01:02:31Z

Storybook previews for 79a76407 are ready:

This comment updates automatically on each PR push.

cubic-dev-ai

All reported issues were addressed across 2 files

Confidence score: 5/5

Safe to merge after the addressed issues were fixed.

_{Reply with feedback, questions, or to request a fix.

Re-trigger cubic}

Replace naive avg(power) with timestamp-window-based arithmetic that correctly computes average power across each candidate duration window. The old query averaged sample power values which is incorrect when samples are irregularly spaced or when a moving window spans partial samples.

cursor · 2026-07-09T03:01:24Z

Bugbot is not enabled for your account, so this pull request was not reviewed.

Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs.

greptile-apps

Asherlc has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.

coderabbitai

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@analytics/models/read_models/activity_power_curve.sql`:
- Around line 124-126: The `avg_power` calculation in `activity_power_curve.sql`
is still based on sample count, so replace the
`window_sample.cumulative_sum`/`row_number_delta` formula with a time-weighted
average that uses the available duration information (for example,
`segment.segment_seconds` and/or elapsed time between samples) in the same
SELECT. Keep the change localized to the query expression that computes
`avg_power`, and verify the surrounding windowing logic still references the
correct symbols (`start_sample`, `window_sample`, `segment`) after switching
from count-weighting to time-weighting.

In
`@packages/server/src/routers/activity-power-curve-read-model.integration.test.ts`:
- Around line 168-198: The varying-power fixture in the activity power curve
integration test currently ends too early, so it never truly exercises the
5-second path in the activity power curve read model. Update the power sample
set built with powerSampleRows for varyingActivityId to include an
offsetSeconds: 5 sample, and then adjust the expected best_power in the rows
assertion to match the actual 5-second average produced by
activity_power_curve.sql and the rendered SQL query.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 6c22a49f-e1c1-4ff8-ac1d-2367a84bfa20

📥 Commits

Reviewing files that changed from the base of the PR and between f37b22a and 488a87d.

📒 Files selected for processing (2)

analytics/models/read_models/activity_power_curve.sql
packages/server/src/routers/activity-power-curve-read-model.integration.test.ts

cubic-dev-ai

All reported issues were addressed across 2 files (changes from recent commits).

_{Reply with feedback, questions, or to request a fix.

Re-trigger cubic}

Test seeds 6 samples over 5 seconds so the 5-second power curve window qualifies (elapsed_seconds >= duration_seconds). Also filter by activity_id to avoid picking up other test data.

cursor · 2026-07-09T03:18:35Z

Bugbot is not enabled for your account, so this pull request was not reviewed.

Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs.

greptile-apps

Asherlc has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.

cubic-dev-ai

0 issues found across 1 file (changes from recent commits).

_{Requires human review: Auto-approval blocked by 2 unresolved issues from previous reviews.

Re-trigger cubic}

Replace count-based mean with time-weighted average using segment duration. The old formula (cumulative_sum / row_count) gave equal weight to each sample regardless of spacing. The new formula weights each sample by the time it was held (segment_seconds from power_segments), so irregularly-spaced data produces correct average power values. Simplifies GROUP BY by removing cumulative_sum, power, row_number from candidate_duration_windows since they're no longer needed.

cursor · 2026-07-09T03:58:05Z

Bugbot is not enabled for your account, so this pull request was not reviewed.

Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs.

greptile-apps

Asherlc has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.

cubic-dev-ai

All reported issues were addressed across 1 file (changes from recent commits).

_{Tip: Review your code locally with the cubic CLI to iterate faster.

Re-trigger cubic}

… segment cross-product in avg_power Each (start_sample, window_sample, duration) triple is now its own group, so segments are counted exactly once per window instead of being multiplied across all matching window_samples.

cursor · 2026-07-09T04:22:33Z

Bugbot is not enabled for your account, so this pull request was not reviewed.

Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs.

greptile-apps

Asherlc has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.

cubic-dev-ai

0 issues found across 1 file (changes from recent commits).

_{Requires human review: Changes core power curve calculation logic; requires human review to verify correctness and handling of edge cases.

Re-trigger cubic}

fix: use timestamp windows for activity power curve

ba6385a

Copilot AI review requested due to automatic review settings July 9, 2026 00:59

Asherlc linked an issue Jul 9, 2026 that may be closed by this pull request

Use timestamp-based windows for activity power curve read model #1532

Closed

sourcery-ai Bot reviewed Jul 9, 2026

View reviewed changes

Copilot AI reviewed Jul 9, 2026

greptile-apps Bot reviewed Jul 9, 2026

View reviewed changes

codereviewbot-ai Bot reviewed Jul 9, 2026

View reviewed changes

Comment thread analytics/models/read_models/activity_power_curve.sql

Comment thread analytics/models/read_models/activity_power_curve.sql

Comment thread packages/server/src/routers/activity-power-curve-read-model.integration.test.ts

Asherlc mentioned this pull request Jul 9, 2026

Use timestamp-based windows for activity power curve read model #1532

Closed

cubic-dev-ai Bot reviewed Jul 9, 2026

View reviewed changes

Comment thread analytics/models/read_models/activity_power_curve.sql Outdated

Comment thread analytics/models/read_models/activity_power_curve.sql

Comment thread analytics/models/read_models/activity_power_curve.sql

greptile-apps Bot reviewed Jul 9, 2026

View reviewed changes

coderabbitai Bot reviewed Jul 9, 2026

View reviewed changes

Comment thread analytics/models/read_models/activity_power_curve.sql Outdated

Comment thread packages/server/src/routers/activity-power-curve-read-model.integration.test.ts

cubic-dev-ai Bot reviewed Jul 9, 2026

View reviewed changes

Comment thread packages/server/src/routers/activity-power-curve-read-model.integration.test.ts

fix: extend varying-power test window to span 5 seconds

a1f8cdd

Test seeds 6 samples over 5 seconds so the 5-second power curve window qualifies (elapsed_seconds >= duration_seconds). Also filter by activity_id to avoid picking up other test data.

greptile-apps Bot reviewed Jul 9, 2026

View reviewed changes

cubic-dev-ai Bot reviewed Jul 9, 2026

View reviewed changes

greptile-apps Bot reviewed Jul 9, 2026

View reviewed changes

cubic-dev-ai Bot reviewed Jul 9, 2026

View reviewed changes

Comment thread analytics/models/read_models/activity_power_curve.sql

greptile-apps Bot reviewed Jul 9, 2026

View reviewed changes

cubic-dev-ai Bot reviewed Jul 9, 2026

View reviewed changes

Asherlc merged commit f57f074 into main Jul 9, 2026
96 checks passed

Asherlc deleted the Asherlc/issue-1532 branch July 9, 2026 04:57

This was referenced Jul 22, 2026

chore: share Conductor preferences #1857

Merged

Unify recent climbing activity tables #1858

Merged

qodo-code-review Bot mentioned this pull request Jul 30, 2026

fix(body): preserve timestamp range type #2334

Merged

Conversation

Asherlc commented Jul 9, 2026 • edited by cubic-dev-ai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Validation

Summary by cubic

Summary by CodeRabbit

Uh oh!

cursor Bot commented Jul 9, 2026

Uh oh!

qodo-code-review Bot commented Jul 9, 2026

Qodo reviews are paused for this user.

Uh oh!

sourcery-ai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

greptile-apps Bot left a comment

Choose a reason for hiding this comment

Uh oh!

coderabbitai Bot commented Jul 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Sequence Diagram(s)

❌ Failed checks (1 warning)

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions Bot commented Jul 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cubic-dev-ai Bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

cursor Bot commented Jul 9, 2026

Uh oh!

greptile-apps Bot left a comment

Choose a reason for hiding this comment

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

cubic-dev-ai Bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

cursor Bot commented Jul 9, 2026

Uh oh!

greptile-apps Bot left a comment

Choose a reason for hiding this comment

Uh oh!

cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

cursor Bot commented Jul 9, 2026

Uh oh!

greptile-apps Bot left a comment

Choose a reason for hiding this comment

Uh oh!

cubic-dev-ai Bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

cursor Bot commented Jul 9, 2026

Uh oh!

greptile-apps Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Asherlc commented Jul 9, 2026 •

edited by cubic-dev-ai Bot

Loading

coderabbitai Bot commented Jul 9, 2026 •

edited

Loading

github-actions Bot commented Jul 9, 2026 •

edited

Loading

cubic-dev-ai Bot left a comment •

edited

Loading

cubic-dev-ai Bot left a comment •

edited

Loading

cubic-dev-ai Bot left a comment •

edited

Loading