Skip to content

perf(server): use monthly serving models - #2029

Merged
Asherlc merged 21 commits into
mainfrom
Asherlc/use-subagents
Jul 26, 2026
Merged

Asherlc merged 21 commits into
mainfrom
Asherlc/use-subagents

Conversation

@Asherlc

@Asherlc Asherlc commented Jul 26, 2026

Copy link
Copy Markdown
Owner

Summary

  • remove recursive activity, sleep, metrics, and resting-heart-rate views from the monthly report request path
  • read the compact activity, daily sleep, and daily recovery serving models instead
  • add a real-ClickHouse regression that succeeds with the recursive views absent

Production evidence

  • old exact query: 120.012s timeout, 3,777,652 rows / 995.16 MiB read, 1.04 GiB peak
  • compact-model query: 0.101s on the same production data

Validation

  • pnpm test:integration -- packages/server/src/repositories/monthly-report-repository.integration.test.ts
  • pnpm exec vitest run packages/server/src/repositories/monthly-report-repository.test.ts
  • targeted Biome check
  • pnpm typecheck
  • pnpm --dir packages/server typecheck

Fixes DOFEK-SERVER-5C.


Summary by cubic

Switch the monthly report to ClickHouse serving models to eliminate timeouts and cut query time from ~120s to ~0.101s. Fixes DOFEK-SERVER-5C with no changes to the response shape.

  • Bug Fixes

    • Read activity from analytics.activity_summary, sleep from analytics.daily_sleep FINAL, and recovery from analytics.daily_recovery FINAL; removed recursive views and the resting‑HR CTE.
    • Added a real ClickHouse integration test that seeds only serving models, drops recursive views, and validates a 12‑month report and serving lifecycle; added a unit test to verify user and month parameter bindings.
    • Recorded exact ClickHouse query‑log evidence and runbook classification in the production incident baseline.
  • Refactors

    • Removed the repository/router timezone param; queries rely on UTC and serving‑model dates. Deleted the related unit test.

Written for commit fbdb8db. Summary will update on new commits.

Review in cubic

Summary by CodeRabbit

  • Bug Fixes

    • Improved monthly report performance to prevent ClickHouse timeouts.
    • Monthly reports now use optimized analytics data sources while preserving existing calculations and response formats.
    • Applied timezone-aware date handling and excluded deleted sleep and recovery records.
  • Tests

    • Added integration coverage validating monthly report values, averages, and historical results against ClickHouse data.
  • Documentation

    • Documented the production incident, mitigation, validation results, and remaining follow-up steps.

Asherlc added 17 commits July 25, 2026 22:42
Reuse activity summary freshness and cap dirty-key batches so analytics cycles do not expand the entire power backlog. Consume precomputed normalized power during refits.
# Conflicts:
#	docs/production-incident-baseline.md
Treat partial and runtime failures as failed while allowing intentional no-op results. Fixes DOFEK-SERVER-5D.
Avoid recursive ClickHouse views on the request path and verify the compact-model query against a real database. Fixes DOFEK-SERVER-5C.
Copilot AI review requested due to automatic review settings July 26, 2026 09:07
@cursor

cursor Bot commented Jul 26, 2026

Copy link
Copy Markdown

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.

@codereviewbot-ai

Copy link
Copy Markdown

🤖 Review skipped: Repository rate limit exceeded. Free accounts are limited to 2 reviews per 4 hours per repository. Upgrade to a paid plan for unlimited reviews.

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Please try again later or upgrade to continue using Sourcery

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Use ClickHouse serving models for monthly report queries

🐞 Bug fix 🧪 Tests 📝 Documentation 🕐 40+ Minutes

Grey Divider

AI Description

• Switch monthly report aggregation to compact ClickHouse serving tables to avoid recursive views.
• Preserve existing report shape while applying FINAL/dedup semantics on sleep and recovery models.
• Add a real ClickHouse regression test that fails if recursive views are required.
Diagram

graph TD
  MRR["MonthlyReportRepository"] --> STORE["ActivitySensorStore"] --> CH[("ClickHouse")]
  CH --> MODELS["Serving models"]
  CH -.->|"no longer reads"| VIEWS["Recursive views"]
  subgraph Legend
    direction LR
    _comp["Code component"] ~~~ _db[("Database")]
  end
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Add caching/async precompute for monthly reports
  • ➕ Avoids request-path pressure even if upstream models regress
  • ➕ Can amortize report generation across many requests
  • ➖ Adds new invalidation/freshness complexity
  • ➖ Does not address root cause of expensive recursive view reads
2. Keep recursive views but tighten predicates / materialize intermediate results
  • ➕ Smaller code change; preserves existing source-of-truth views
  • ➖ Still couples request path to complex recursive/dedup logic
  • ➖ Hard to guarantee performance across users/time ranges
3. Create a dedicated monthly rollup table/model
  • ➕ Fastest possible reads; minimal query complexity at runtime
  • ➖ New pipeline to maintain and backfill
  • ➖ Potential duplication of logic already represented by daily serving models

Recommendation: Prefer the PR’s approach: querying existing compact serving models is the simplest and most robust way to remove recursive-view recomputation from the request path while preserving response semantics. Caching or new rollups could further reduce latency, but add operational complexity and aren’t necessary given the observed production performance improvement.

Files changed (3) +162 / -23

Bug fix (1) +14 / -23
monthly-report-repository.tsRewrite monthly report query to use serving tables (no recursive views) +14/-23

Rewrite monthly report query to use serving tables (no recursive views)

• Removes dependencies on v_activity/v_sleep/v_daily_metrics and the resting-heart-rate CTE, instead reading analytics.activity_summary plus analytics.daily_sleep FINAL and analytics.daily_recovery FINAL with is_deleted filtering. Adjusts activity day-bucketing to respect the configured timezone and keeps the output shape/trend calculations intact.

packages/server/src/repositories/monthly-report-repository.ts

Tests (1) +109 / -0
monthly-report-repository.integration.test.tsAdd ClickHouse integration regression for serving-model monthly report +109/-0

Add ClickHouse integration regression for serving-model monthly report

• Introduces a real ClickHouse integration test that seeds activity_summary, daily_sleep, and daily_recovery, then drops legacy recursive views to ensure the repository no longer depends on them. Verifies the current-month summary and history length for a 12-month report.

packages/server/src/repositories/monthly-report-repository.integration.test.ts

Documentation (1) +39 / -0
production-incident-baseline.mdDocument DOFEK-SERVER-5C monthly report timeout and mitigation +39/-0

Document DOFEK-SERVER-5C monthly report timeout and mitigation

• Adds a production incident entry describing monthly report ClickHouse timeouts caused by recursive view usage. Records evidence, root cause, mitigation (serving models + FINAL), and validation/follow-up steps.

docs/production-incident-baseline.md

@coderabbitai

coderabbitai Bot commented Jul 26, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

Monthly report aggregation now uses timezone-aware bucketing and compact finalized sleep/recovery serving tables instead of recursive analytics views. A ClickHouse integration test validates the report without those views, and production incident documentation records the mitigation and follow-up.

Changes

Monthly report read-model update

Layer / File(s) Summary
Replace recursive monthly report sources
packages/server/src/repositories/monthly-report-repository.ts
Monthly report activity dates use the requested timezone, while sleep and recovery metrics come from non-deleted FINAL daily serving tables with simplified query parameters.
Validate compact-model report generation
packages/server/src/repositories/monthly-report-repository.integration.test.ts, docs/production-incident-baseline.md
The ClickHouse integration test seeds serving data, removes recursive views, verifies current and historical report results, and cleans up resources. Incident documentation records the timeout, mitigation, validation, and deployment follow-up.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related PRs

  • Asherlc/dofek#1100: Related changes to monthly-report ClickHouse data sources and derived resting-heart-rate handling.

Suggested labels: area/server, type/bug

Suggested reviewers: copilot

🚥 Pre-merge checks | ✅ 1 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Title check ⚠️ Warning The title is relevant and imperative, but it does not use the required area prefix format. Rename it to use the bracketed area prefix, e.g. "[server] use monthly serving models", and keep it under 70 characters.
✅ 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

github-actions Bot commented Jul 26, 2026

Copy link
Copy Markdown
Contributor

Storybook previews for 92f17f07 are ready:

This comment updates automatically on each PR push.

@codereviewbot-ai

Copy link
Copy Markdown

🤖 Review skipped: Repository rate limit exceeded. Free accounts are limited to 2 reviews per 4 hours per repository. Upgrade to a paid plan for unlimited reviews.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 `@docs/production-incident-baseline.md`:
- Line 17965: Rename the incident heading “2026-07-26 — Monthly Report
Recomputed Recursive Analytics Views” to state that the monthly report query
replaced recursive analytics views, reflecting the actual mitigation.

In `@packages/server/src/repositories/monthly-report-repository.ts`:
- Around line 128-130: Make report boundaries timezone-aware in the monthly
report query anchored by the per_activity CTE: derive local report-date/month
start and end values from {timezone:String} and reuse them consistently for
activity, sleep, recovery, and date_series ranges. In
packages/server/src/repositories/monthly-report-repository.integration.test.ts
lines 93-108, add a non-UTC month-boundary fixture including deleted and
superseded serving rows, and assert the resulting aggregates to cover the
corrected behavior.
🪄 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: c732e4ae-86aa-450a-a71b-eab3606b6b0e

📥 Commits

Reviewing files that changed from the base of the PR and between 0da6abd and a867312.

📒 Files selected for processing (3)
  • docs/production-incident-baseline.md
  • packages/server/src/repositories/monthly-report-repository.integration.test.ts
  • packages/server/src/repositories/monthly-report-repository.ts

Comment thread docs/production-incident-baseline.md Outdated
Comment thread packages/server/src/repositories/monthly-report-repository.ts Outdated
@qodo-code-review

qodo-code-review Bot commented Jul 26, 2026

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (1) 📜 Skill insights (0)

Context used
✅ Compliance rules (platform): 164 rules

Grey Divider


Action required

1. Timezone window mismatch ✓ Resolved 🐞 Bug ≡ Correctness
Description
MonthlyReportRepository buckets activities by local date (toTimeZone(..., {timezone})) but
computes the reporting window and date_series from ClickHouse today()/toStartOfMonth(today())
without the same timezone basis, so non-UTC users can lose or mis-bucket activities near day/month
boundaries. This can under/over-count trainingHours, activityCount, and avgDailyStrain for the
affected months because daily_training rows may not join to the generated day series or may fall
outside the UTC-anchored window.
Code

packages/server/src/repositories/monthly-report-repository.ts[R128-137]

+      `WITH per_activity AS (
        SELECT
-          toDate(asum.started_at) AS date,
+          toDate(toTimeZone(asum.started_at, {timezone:String})) AS date,
          dateDiff('second', asum.started_at, asum.ended_at) / 3600.0 AS hours,
          dateDiff('second', asum.started_at, asum.ended_at) / 60.0
            * asum.avg_hr / nullIf(toFloat64(asum.max_hr), 0) AS load
          FROM analytics.activity_summary asum
-          INNER JOIN analytics.v_activity va
-            ON va.id = asum.activity_id
-           AND va.user_id = asum.user_id
          WHERE asum.user_id = {userId:UUID}
            AND asum.started_at >= toStartOfMonth(today()) - INTERVAL {months:Int32} MONTH
            AND asum.ended_at IS NOT NULL
Relevance

⭐⭐⭐ High

Timezone-boundary correctness issues have prior accepted fixes: align filtering/windowing with the
same local-date predicate used for bucketing.

PR-#867
PR-#1151
PR-#1527

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The monthly report query computes per-activity date using the provided timezone, but its window
lower bound and the generated day series are derived from today()/toStartOfMonth(today())
without applying the timezone, creating inconsistent day/month boundaries. The router passes
ctx.timezone, and the weekly report repository demonstrates the correct pattern of filtering on
the same timezone-transformed date used for bucketing.

packages/server/src/repositories/monthly-report-repository.ts[119-186]
packages/server/src/routers/monthly-report.ts[26-37]
packages/server/src/repositories/weekly-report-repository.ts[152-213]
PR-#1151

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`MonthlyReportRepository.getReport()` computes activity `date` using the user timezone, but the report window (filters) and `date_series` are anchored to ClickHouse server `today()`/`toStartOfMonth(today())` with no timezone conversion. This mixes local-day activity rows with a UTC (or server-tz) day series/month boundaries, causing boundary-day/month rows to be dropped or grouped incorrectly.

## Issue Context
- The router passes `ctx.timezone` into `MonthlyReportRepository`, so this runs for non-UTC users.
- Weekly report code already uses a consistent local-date basis for activity filtering (`toDate(toTimeZone(...)) >= windowStart`).

## Fix Focus Areas
- packages/server/src/repositories/monthly-report-repository.ts[125-186]

### Concrete implementation direction
- Introduce a `local_today` (Date) expression derived from `now()`/`now64()` converted via `toTimeZone(..., {timezone})` and then `toDate(...)`.
- Derive `local_month_start := toStartOfMonth(local_today)`.
- Use `local_month_start - INTERVAL {months} MONTH` as the lower bound consistently for:
 - activity filtering (compare against the *same* local-date transform used for bucketing, e.g. `toDate(toTimeZone(asum.started_at, {timezone})) >= ...`)
 - sleep/recovery date filters (if those `date` columns are intended to align to the same calendar basis)
 - `date_series` generation start
- Avoid using raw `asum.started_at >= toStartOfMonth(today()) ...` when bucketing by local date.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Remediation recommended

2. Integration test clock skew ✓ Resolved 🐞 Bug ☼ Reliability
Description
The new ClickHouse integration test seeds timestamps using ClickHouse
today()/toStartOfMonth(today()) but asserts monthStart using Node's `new
Date().toISOString()`, so if ClickHouse and Node observe different calendar dates (clock skew/config
or a boundary crossing) the test can fail intermittently. This makes the regression less reliable
even though production behavior is unaffected.
Code

packages/server/src/repositories/monthly-report-repository.integration.test.ts[R21-106]

+    await executeClickHouseTestCommand(
+      testContext,
+      `INSERT INTO analytics.activity_summary (
+        activity_id,
+        user_id,
+        activity_type,
+        started_at,
+        ended_at,
+        avg_hr,
+        max_hr
+      ) VALUES (
+        toUUID('${activityId}'),
+        toUUID('${userId}'),
+        'cycling',
+        toDateTime64(toStartOfMonth(today()) + INTERVAL 5 DAY, 6, 'UTC'),
+        toDateTime64(toStartOfMonth(today()) + INTERVAL 5 DAY + INTERVAL 1 HOUR, 6, 'UTC'),
+        100,
+        200
+      )`,
+    );
+    await executeClickHouseTestCommand(
+      testContext,
+      `INSERT INTO analytics.daily_sleep (
+        user_id,
+        date,
+        provider_id,
+        started_at,
+        duration_minutes,
+        refresh_version,
+        is_deleted,
+        refreshed_at
+      ) VALUES (
+        toUUID('${userId}'),
+        toDate(toStartOfMonth(today()) + INTERVAL 5 DAY),
+        'test-provider',
+        toDateTime64(toStartOfMonth(today()) + INTERVAL 5 DAY, 6, 'UTC'),
+        480,
+        1,
+        0,
+        now64(9)
+      )`,
+    );
+    await executeClickHouseTestCommand(
+      testContext,
+      `INSERT INTO analytics.daily_recovery (
+        user_id,
+        date,
+        hrv,
+        resting_hr,
+        is_deleted,
+        refresh_version,
+        refreshed_at
+      ) VALUES (
+        toUUID('${userId}'),
+        toDate(toStartOfMonth(today()) + INTERVAL 5 DAY),
+        60,
+        50,
+        0,
+        1,
+        now64(9)
+      )`,
+    );
+
+    for (const recursiveView of ["v_activity", "v_sleep", "v_daily_metrics"]) {
+      await executeClickHouseTestCommand(testContext, `DROP TABLE analytics.${recursiveView} SYNC`);
+    }
+  }, 120_000);
+
+  afterAll(async () => {
+    await testContext?.cleanup();
+  });
+
+  it("builds the report from compact serving models without recursive views", async () => {
+    const report = await new MonthlyReportRepository(userId, sensorStore).getReport(12);
+
+    expect(report.current).toEqual({
+      monthStart: `${new Date().toISOString().slice(0, 7)}-01`,
+      trainingHours: 1,
+      activityCount: 1,
+      avgDailyStrain: 30,
+      avgSleepMinutes: 480,
+      avgRestingHr: 50,
+      avgHrv: 60,
+      trainingHoursTrend: null,
+      avgSleepTrend: null,
+    });
Relevance

⭐⭐⭐ High

Team has accepted fixes to make time-based tests deterministic and avoid midnight/clock-source
flakiness.

PR-#1151
PR-#1526

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The test inserts rows anchored to ClickHouse toStartOfMonth(today()) but compares the report’s
monthStart to a value derived from Node’s wall clock, which can diverge across environments or
boundaries. A previously accepted bug highlights that using multiple "now" sources in tests can
cause midnight-boundary flakiness; this test compounds that risk by mixing Node and ClickHouse
clocks.

packages/server/src/repositories/monthly-report-repository.integration.test.ts[17-108]
PR-#1151

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The test uses two independent time sources:
- ClickHouse-side `today()` for fixture seeding
- Node-side `new Date()` for the expected `monthStart`
If those are not on the same calendar date (or cross a boundary during execution), the seeded rows can land in a different month than the asserted monthStart.

## Issue Context
This is an integration test; stability is important to keep CI trustworthy.

## Fix Focus Areas
- packages/server/src/repositories/monthly-report-repository.integration.test.ts[17-108]

### Concrete implementation direction
Pick one deterministic source of truth and use it for both fixture seeding and assertions:
- Option A (simplest): compute a `const monthStart = new Date().toISOString().slice(0, 7) + '-01'` once, and use that string in ClickHouse inserts via `toDateTime64('${monthStart} 00:00:00', 6, 'UTC')` / `toDate('${monthStart}')`.
- Option B: query ClickHouse once for `toString(toStartOfMonth(today()))` and assert against that (and seed from it).

Optional hardening:
- Use `DROP TABLE IF EXISTS ...` when dropping legacy recursive tables to avoid unexpected failures if the bootstrap changes.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


3. Missing runbook evidence reference ✓ Resolved 📘 Rule violation ➹ Performance
Description
This PR changes the monthly report query behavior due to a documented production slowdown, but the
added incident entry does not reference docs/performance/loading-performance-runbook.md nor
include an Axiom (or equivalent) classification artifact link as required. Without that linkage,
future performance fixes are harder to audit and may skip the required evidence gate.
Code

docs/production-incident-baseline.md[R17965-18002]

+## 2026-07-26 — Monthly Report Recomputed Recursive Analytics Views
+
+- **Status:** Direct fix validated locally and against production data; merge
+  and production deployment pending.
+- **Symptoms:** Monthly report requests repeatedly raised ClickHouse client
+  timeouts in
+  [Sentry issue DOFEK-SERVER-5C](https://east-bay-software.sentry.io/issues/DOFEK-SERVER-5C).
+- **User impact:** The monthly report could not load before the request's
+  120-second execution deadline.
+- **Evidence:** The exact parameterized production repository query timed out
+  after 120.012 seconds while reading 3,777,652 rows and 995.16 MiB, with a
+  1.04 GiB peak. The equivalent aggregation over the compact
+  `activity_summary`, `daily_sleep`, and `daily_recovery` serving models
+  completed against the same production data in 0.101 seconds. The physical
+  serving tables contained only 3,653 activity-summary rows, 330 daily-sleep
+  rows, and 119 daily-recovery rows.
+- **Root cause:** The request path joined `analytics.v_activity` and read
+  `analytics.v_sleep`, `analytics.v_daily_metrics`, and the resting-heart-rate
+  view, forcing global recursive deduplication and sensor aggregation for a
+  small user-and-month result that already existed in compact dbt-owned
+  serving models.
+- **Fix / mitigation:** Read current activities from
+  `analytics.activity_summary`, daily sleep from `analytics.daily_sleep FINAL`,
+  and daily vitals from `analytics.daily_recovery FINAL`. Preserve the
+  monthly response shape and calculations without adding a timeout, retry, or
+  cache fallback. ClickHouse documents `FINAL` as the query modifier that
+  applies an engine's merge logic to the selected data:
+  <https://clickhouse.com/docs/sql-reference/statements/select/from#final-modifier>.
+- **Validation:** A real-ClickHouse regression test first failed after its
+  recursive test views were removed, reproducing the old repository's hard
+  dependency on `v_activity`. It now seeds only the three compact serving
+  models and verifies the complete monthly report result with those recursive
+  views absent. The focused integration test passes.
+- **Remaining risk / follow-up:** Merge through normal CI, deploy the query,
+  invoke the production monthly report, confirm sub-second completion in
+  ClickHouse query history, and resolve `DOFEK-SERVER-5C` if no fixed-release
+  event recurs. The weekly report contains a similar recursive query and should
+  be investigated separately rather than silently expanded into this fix.
Relevance

⭐⭐⭐ High

Docs incident entries commonly require linking primary evidence/artifacts; missing runbook/evidence
link likely requested.

PR-#1858
PR-#1864
PR-#1394

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The new incident section documents a performance-motivated behavior/query change but contains no
mention of docs/performance/loading-performance-runbook.md and no Axiom (or similar) evidence link
demonstrating slowdown classification, which the rule requires.

Rule 1540813: Classify and record dashboard slowdowns before modifying behavior
docs/production-incident-baseline.md[17965-18002]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The incident/perf-driven behavior change is documented, but it does not explicitly reference the loading-performance runbook and does not include an Axiom (or equivalent recorded) evidence link/classification artifact.

## Issue Context
Compliance requires that before modifying behavior in response to dashboard/query slowdowns, we record the classification evidence and reference the runbook used to gather it.

## Fix Focus Areas
- docs/production-incident-baseline.md[17965-18002]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Informational

4. Complex SQL transformation in TS 📘 Rule violation ⌂ Architecture
Description
The modified monthly report query still performs multi-CTE aggregation/transformation logic inline
in TypeScript rather than using a dbt model, which violates the requirement to keep analytics
transformation SQL in dbt. This increases duplication risk and makes warehouse logic harder to
govern and optimize.
Code

packages/server/src/repositories/monthly-report-repository.ts[R128-137]

+      `WITH per_activity AS (
        SELECT
-          toDate(asum.started_at) AS date,
+          toDate(toTimeZone(asum.started_at, {timezone:String})) AS date,
          dateDiff('second', asum.started_at, asum.ended_at) / 3600.0 AS hours,
          dateDiff('second', asum.started_at, asum.ended_at) / 60.0
            * asum.avg_hr / nullIf(toFloat64(asum.max_hr), 0) AS load
          FROM analytics.activity_summary asum
-          INNER JOIN analytics.v_activity va
-            ON va.id = asum.activity_id
-           AND va.user_id = asum.user_id
          WHERE asum.user_id = {userId:UUID}
            AND asum.started_at >= toStartOfMonth(today()) - INTERVAL {months:Int32} MONTH
            AND asum.ended_at IS NOT NULL
Relevance

⭐ Low

Repo has rejected requests to move non-trivial SQL out of TypeScript into models/dbt for “drift”
reasons.

PR-#1966
PR-#1969

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The rule disallows analytics transformation SQL embedded in TypeScript; the modified query is a
multi-CTE transformation with aggregations and joins implemented directly in the repository method.

Rule 784485: Define analytics transformation SQL in dbt models, not in TypeScript
packages/server/src/repositories/monthly-report-repository.ts[125-185]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
TypeScript code contains a complex, multi-CTE analytics transformation query. Compliance requires that analytics transformation SQL live in dbt models under `analytics/models/`, with application code querying the materialized model via minimal SQL.

## Issue Context
This query computes derived metrics (daily rollups and monthly aggregation) across serving tables (`analytics.activity_summary`, `analytics.daily_sleep`, `analytics.daily_recovery`) using CTEs and group-bys.

## Fix Focus Areas
- packages/server/src/repositories/monthly-report-repository.ts[128-184]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Qodo Logo

Comment thread docs/production-incident-baseline.md Outdated
@codereviewbot-ai

Copy link
Copy Markdown

🤖 Review skipped: Repository rate limit exceeded. Free accounts are limited to 2 reviews per 4 hours per repository. Upgrade to a paid plan for unlimited reviews.

@codereviewbot-ai

Copy link
Copy Markdown

🤖 Review skipped: Repository rate limit exceeded. Free accounts are limited to 2 reviews per 4 hours per repository. Upgrade to a paid plan for unlimited reviews.

@codereviewbot-ai

Copy link
Copy Markdown

🤖 Review skipped: Repository rate limit exceeded. Free accounts are limited to 2 reviews per 4 hours per repository. Upgrade to a paid plan for unlimited reviews.

@Asherlc
Asherlc merged commit 06d9c08 into main Jul 26, 2026
100 checks passed
@Asherlc
Asherlc deleted the Asherlc/use-subagents branch July 26, 2026 09:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants