fix(body): preserve timestamp range type

[Asherlc]

Summary

• filter body-measurement ranges while recorded_at is still a native
  ClickHouse DateTime64
• serialize timestamps only after the inclusive local-date predicate runs
• add a real ClickHouse regression covering measurements on opposite sides of
  a Los Angeles midnight boundary

Root cause

BodyRepository.listRange() projected toString(recorded_at) AS recorded_at
in the same query scope where WHERE passed recorded_at to toTimeZone.
ClickHouse rebound that identifier to the projected String alias, producing
Illegal type String of argument of function toTimezone.

Validation

• pnpm vitest run --project unit packages/server/src/repositories/body-repository.test.ts
  (16 passed)
• root, server, and web pnpm tsc --noEmit passed
• focused Biome and git diff --check passed
• full lint passed through every repository policy check; SQLFluff compiled the
  dbt project but then stalled under the same host I/O pressure
• local real-engine execution was blocked in isolated ClickHouse bootstrap
  (beforeAll timed out at 120 seconds); hosted integration is the required
  real-engine red/green gate before this PR leaves draft

Fixes #2294

Summary by Sourcery

Fix body metrics range queries to filter on native ClickHouse timestamps before string serialization, preventing timezone alias type errors and ensuring correct local-date ranges.

Bug Fixes:

• Prevent ClickHouse timezone conversion errors in body measurement range queries by avoiding reuse of a stringified timestamp alias in filtering predicates.

Enhancements:

• Align the body range query structure with existing patterns by introducing an inner subquery that filters raw timestamps and an outer projection that handles serialization.

Documentation:

• Add a TDD plan document describing the timezone-filter behavior, test strategy, and implementation steps for body metrics range queries.

Tests:

• Extend unit tests to assert the updated body range query shape and add an integration test that verifies exact local-date range filtering against a real ClickHouse database.

---

Summary by cubic

Fix body metrics range filtering by applying the date filter to native ClickHouse DateTime64 values and only stringifying after, preventing timezone alias type errors and returning correct inclusive local-date results. Fixes #2294.

• Bug Fixes

  • Reworked BodyRepository.listRange() to filter in an inner subquery on native recorded_at, then serialize in the outer projection; orders by the inner recorded_at to avoid alias rebound errors.

• Tests

  • Added a real ClickHouse regression around a Los Angeles midnight boundary; rebuilds analytics.v_body_measurement before asserting exact local-date filtering.
  • Updated the unit test to assert the new query shape in packages/server/src/repositories/body-repository.test.ts.
  • Added a TDD plan in docs/superpowers/plans/2026-07-29-body-metrics-timezone-filter.md.

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

[codereviewbot-ai]

🤖 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]

Warning

Review limit reached

@Asherlc, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 36 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 1d699ec9-7c76-43dd-98db-044e0051f103

📥 Commits

Reviewing files that changed from the base of the PR and between cba05b6 and e4f1dfd.

📒 Files selected for processing (4)

• docs/superpowers/plans/2026-07-29-body-metrics-timezone-filter.md
• packages/server/src/repositories/body-repository.integration.test.ts
• packages/server/src/repositories/body-repository.test.ts
• packages/server/src/repositories/body-repository.ts

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[sourcery-ai]

Reviewer's Guide

Adjust BodyRepository.listRange() to filter ClickHouse native timestamps in a subquery before string serialization, and add tests and documentation to cover timezone-range behavior and prevent alias collisions.

Sequence diagram for BodyRepository.listRange ClickHouse timestamp filtering

sequenceDiagram
  actor Client
  participant BodyRepository
  participant BodyClickHouseStore as BodyClickHouseStore
  participant ClickHouse

  Client->>BodyRepository: listRange(startDate, endDate)
  BodyRepository->>BodyClickHouseStore: query(listRange SQL)
  BodyClickHouseStore->>ClickHouse: execute(listRange SQL)
  Note over ClickHouse: Inner body_measurements subquery
  ClickHouse-->>ClickHouse: toTimeZone(recorded_at, timezone)
  ClickHouse-->>ClickHouse: toDate(toTimeZone(...)) range filter
  Note over ClickHouse: Outer projection
  ClickHouse-->>ClickHouse: toString(body_measurements.recorded_at)
  ClickHouse-->>BodyClickHouseStore: filtered and serialized rows
  BodyClickHouseStore-->>BodyRepository: rows
  BodyRepository-->>Client: BodyMeasurement[]

Loading

File-Level Changes

Change	Details	Files
Scope listRange() filtering to an inner subquery so recorded_at remains a native DateTime64 during range filtering and ordering, then serialize timestamps only in the outer projection.	Change the SELECT projection to stringify body_measurements.recorded_at instead of the bare recorded_at identifier. Wrap analytics.v_body_measurement in an inner body_measurements subquery that performs user_id and local-date range filtering on the native recorded_at column. Move ORDER BY to use body_measurements.recorded_at from the inner subquery, keeping ordering on the raw timestamp rather than a string alias.	packages/server/src/repositories/body-repository.ts
Extend the unit test for listRange() to assert the updated query shape, ensuring the subquery and alias usage remain correct.	Capture the generated SQL text from the mocked query call in the listRange() test. Assert that the query text stringifies body_measurements.recorded_at, introduces the body_measurements subquery alias, and orders by body_measurements.recorded_at.	packages/server/src/repositories/body-repository.test.ts
Introduce a TDD plan document describing the timezone-filtering bug, desired behavior, and integration-test strategy for body metrics range queries.	Add a docs page outlining the root cause (ClickHouse alias rebinding), behavior requirements for listRange(), and constraints on the scope of changes. Document concrete TDD tasks and commands for adding the failing integration test, implementing the query fix, and running verification steps.	docs/superpowers/plans/2026-07-29-body-metrics-timezone-filter.md
Add a ClickHouse integration test exercising listRange() across a Los Angeles local-date boundary to guard against regressions in timestamp filtering and alias handling.	Set up an isolated ClickHouse test database and activity sensor store for the integration scenario. Seed two body measurement rows with recorded_at timestamps straddling a LA midnight boundary in analytics.body_measurement_sample. Call BodyRepository.listRange() with an exact local-date range and assert that only the in-range measurement is returned, proving native timestamp filtering before serialization.	packages/server/src/repositories/body-repository.integration.test.ts

Assessment against linked issues

Issue	Objective	Addressed	Explanation
#2294	Ensure BodyRepository.listRange uses the native DateTime/DateTime64 recorded_at in its date-range WHERE clause instead of a String alias, so get_body_metrics executes successfully and correctly filters by the requested local date range.	✅
#2294	Add regression coverage and documentation to prevent reintroducing the recorded_at String alias collision in body range queries.	✅

---

Tips and commands

Interacting with Sourcery

• Trigger a new review: Comment @sourcery-ai review on the pull request.
• Continue discussions: Reply directly to Sourcery's review comments.
• Generate a GitHub issue from a review comment: Ask Sourcery to create an
  issue from a review comment by replying to it. You can also reply to a
  review comment with @sourcery-ai issue to create an issue from it.
• Generate a pull request title: Write @sourcery-ai anywhere in the pull
  request title to generate a title at any time. You can also comment
  @sourcery-ai title on the pull request to (re-)generate the title at any time.
• Generate a pull request summary: Write @sourcery-ai summary anywhere in
  the pull request body to generate a PR summary at any time exactly where you
  want it. You can also comment @sourcery-ai summary on the pull request to
  (re-)generate the summary at any time.
• Generate reviewer's guide: Comment @sourcery-ai guide on the pull
  request to (re-)generate the reviewer's guide at any time.
• Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
  pull request to resolve all Sourcery comments. Useful if you've already
  addressed all the comments and don't want to see them anymore.
• Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
  request to dismiss all existing Sourcery reviews. Especially useful if you
  want to start fresh with a new review - don't forget to comment
  @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

• Enable or disable review features such as the Sourcery-generated pull request
  summary, the reviewer's guide, and others.
• Change the review language.
• Add, remove or edit custom review instructions.
• Adjust other review settings.

Getting Help

• Contact our support team for questions or feedback.
• Visit our documentation for detailed guides and information.
• Keep in touch with the Sourcery team by following us on X/Twitter, LinkedIn or GitHub.

[github-actions]

Storybook previews for 19922884 are ready:

• Web Storybook
• Mobile Storybook

This comment updates automatically on each PR push.

[codereviewbot-ai]

LGTM! The subquery refactoring in BodyRepository.listRange() prevents ClickHouse alias shadowing, ensuring toTimeZone() evaluates against the native DateTime64 column rather than a stringified alias. Both unit and integration tests thoroughly verify this behavior.

---

🤖 Reviewed by codereviewbot.ai - Catch bugs before your team does.

[cursor]

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.

[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.

[qodo-code-review]

PR Summary by Qodo

Fix BodyRepository.listRange by filtering DateTime64 before string projection

🐞 Bug fix 🧪 Tests 📝 Documentation 🕐 40+ Minutes

AI Description

• Scope listRange() filtering to native DateTime64 to avoid ClickHouse alias rebinding
• Serialize timestamps only after the inclusive local-date predicate is applied
• Add a real ClickHouse regression test across a Los Angeles midnight boundary

Diagram

graph TD
  T["Integration test"] --> R["BodyRepository.listRange()"] --> S["Inner subquery"] --> P["Outer projection"] --> O["BodyMeasurement rows"]
  S --> V[("ClickHouse view")]
  S --> F["toTimeZone + toDate filter"]

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Rename projected alias (e.g., recorded_at_str)

• ➕ Avoids ClickHouse rebinding recorded_at to a String alias while keeping single query scope
• ➕ Smaller SQL change than introducing a subquery
• ➖ Requires downstream schema/mapping changes (API expects recorded_at)
• ➖ Still relies on careful alias discipline across future edits

2. Fully qualify `recorded_at` in predicates (where supported)

• ➕ Potentially avoids identifier rebinding without extra subquery level
• ➕ Keeps projection shape unchanged
• ➖ ClickHouse alias resolution rules can still be surprising across scopes/functions
• ➖ More brittle/less portable than explicit subquery scoping

Recommendation: The chosen subquery approach is the most robust: it creates an explicit scope where recorded_at remains DateTime64 for timezone/date filtering, then safely stringifies in the outer projection. This matches the existing list() pattern and is less error-prone than relying on alias naming or qualification rules.

Files changed (4) +180 / -6

Bug fix (1) +28 / -6

body-repository.tsFix listRange by filtering before stringifying recorded_at +28/-6

Fix listRange by filtering before stringifying recorded_at

• Reworks 'listRange()' to select from an inner subquery that applies the timezone/local-date predicate while 'recorded_at' is still 'DateTime64'. The outer query then serializes timestamps/IDs and orders by 'body_measurements.recorded_at' to avoid ClickHouse alias rebound errors.

packages/server/src/repositories/body-repository.ts

Tests (2) +74 / -0

body-repository.integration.test.tsAdd real ClickHouse regression for inclusive local-date range +70/-0

Add real ClickHouse regression for inclusive local-date range

• Introduces an integration test that seeds two measurements around a Los Angeles midnight boundary. Verifies 'listRange()' returns only the row within the requested local-date range.

packages/server/src/repositories/body-repository.integration.test.ts

body-repository.test.tsAssert listRange query uses inner subquery + ordered raw recorded_at +4/-0

Assert listRange query uses inner subquery + ordered raw recorded_at

• Extends the unit test to validate the updated query shape: 'recorded_at' is stringified from 'body_measurements.recorded_at', uses an inner subquery alias, and orders by the raw timestamp column.

packages/server/src/repositories/body-repository.test.ts

Documentation (1) +78 / -0

2026-07-29-body-metrics-timezone-filter.mdAdd TDD plan for timezone-safe body range filtering +78/-0

Add TDD plan for timezone-safe body range filtering

• Documents the alias-rebinding root cause in ClickHouse and outlines a TDD workflow. Defines unit/integration test strategy and the minimal repository query change scope.

docs/superpowers/plans/2026-07-29-body-metrics-timezone-filter.md

[qodo-code-review]

Code Review by Qodo

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

Context used

✅ Compliance rules (platform): 168 rules

✅ Skills: fix-provider, write-tests, cloudflare

1. Backfill-heavy ClickHouse test setup 📘 Rule violation ▣ Testability

Description

The new integration test bootstraps ClickHouse via createClickHouseTestActivitySensorStore(),
which truncates analytics tables, runs backfill SQL, and rebuilds many analytics views, making this
test broad and potentially slow/flaky. The compliance checklist requires ClickHouse integration
tests to seed minimal rows using the final schema without running backfill pipelines as part of
standard test setup.

Code

packages/server/src/repositories/body-repository.integration.test.ts[R16-19]

+  beforeAll(async () => {
+    testContext = await setupTestDatabase();
+    store = await createClickHouseTestActivitySensorStore(testContext);
+  }, 120_000);

Relevance

● Weak

Repo frequently uses createClickHouseTestActivitySensorStore() in ClickHouse integration tests
without pushback (e.g., PR #1569, #2036).

PR-#1569
PR-#2036

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

PR Compliance ID 773503 requires ClickHouse integration tests to avoid invoking broad backfill
pipelines and instead seed minimal rows with the final schema. The added test calls
createClickHouseTestActivitySensorStore(), and that helper syncs raw tables and then calls
rebuildClickHouseSensorAnalyticsWithClient(), which executes backfill SQL
(buildSensorScalarSampleBackfillSql(), buildDedupedSensorBackfillSql()) and rebuilds a full
analytics view build order.

Rule 773503: Avoid broad tests for one-off production backfills; hydrate ClickHouse tests with final schema
packages/server/src/repositories/body-repository.integration.test.ts[16-19]
packages/server/src/routers/clickhouse-integration-test-helpers.ts[528-562]
packages/server/src/routers/clickhouse-integration-test-helpers.ts[774-788]
packages/server/src/routers/clickhouse-integration-test-helpers.ts[908-919]

Agent prompt

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

## Issue description
A new integration test uses `createClickHouseTestActivitySensorStore()` which rebuilds wide ClickHouse analytics state by running backfill SQL and rebuilding many views; the rule requires integration tests to avoid broad backfill/migration pipelines and instead hydrate only the minimal final-schema state needed.

## Issue Context
This test only needs `analytics.v_body_measurement` for `BodyRepository.listRange()` behavior, but the current setup runs sensor backfill SQL and rebuilds multiple analytics views during test bootstrap.

## Fix Focus Areas
- packages/server/src/repositories/body-repository.integration.test.ts[16-19]
- packages/server/src/routers/clickhouse-integration-test-helpers.ts[528-562]
- packages/server/src/routers/clickhouse-integration-test-helpers.ts[774-788]
- packages/server/src/routers/clickhouse-integration-test-helpers.ts[908-919]

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

---

2. Doc uses raw vitest 📘 Rule violation ≡ Correctness

Description

The new TDD plan instructs running a ClickHouse-backed integration test via `pnpm vitest run
--project integration instead of the repo’s integration test commands that set TEST_DATABASE_URL`
and bring up Compose. This can cause database-backed tests to be executed without the required
environment wiring, leading to misleading results and inconsistent developer workflows.

Code

docs/superpowers/plans/2026-07-29-body-metrics-timezone-filter.md[58]

+  `rtk bash -lc 'set -a; . ./.env.local; set +a; pnpm vitest run --project integration packages/server/src/repositories/body-repository.integration.test.ts'`.

Relevance

● Weak

Docs historically allow running pnpm exec vitest run --project integration after sourcing
.env.local (PR #1807, #1158).

PR-#1807
PR-#1158

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The plan doc added in this PR explicitly tells readers to run `pnpm vitest run --project integration
...`, which bypasses the standardized integration-test entrypoint required by the checklist. The
repo’s test:integration script routes through scripts/run-tests.ts, which sets
TEST_DATABASE_URL and runs Compose before invoking Vitest.

Rule 2237056: Database-backed tests must be executed only via integration test commands that provide TEST_DATABASE_URL
docs/superpowers/plans/2026-07-29-body-metrics-timezone-filter.md[56-60]
package.json[157-163]
scripts/run-tests.ts[46-65]

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 plan document instructs running a DB-backed integration test with a raw `pnpm vitest run --project integration ...` command. Per compliance, database-backed tests must be run via the standardized integration test commands that ensure Docker Compose deps are up and `TEST_DATABASE_URL` is set.

## Issue Context
The repo already provides `pnpm test:integration` (backed by `scripts/run-tests.ts`) which brings up Compose and exports `TEST_DATABASE_URL` from `.env.local`.

## Fix Focus Areas
- docs/superpowers/plans/2026-07-29-body-metrics-timezone-filter.md[56-60]

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

---

_{To customize comments, go to the Qodo configuration screen, or learn more in the docs.}