fix: align ClickHouse SQL URL type - #1241
Conversation
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
📝 WalkthroughWalkthroughThis PR implements the second phase of a staged slow-query optimization sequence by introducing a compact ClickHouse incremental read model for provider statistics, migrating server queries to consume it, replacing expensive ChangesSlow Query Optimization - Provider Stats, Admin Counts, and Deduplication
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 1 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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. Comment |
|
Storybook previews for This comment updates automatically on each PR push. |
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
No issues found across 3 files
Confidence score: 5/5
- Automated review surfaced no issues in the provided summaries.
- No files require special attention.
Auto-approved: This change adds the missing URL import from node:url to fix TypeScript type errors, and removes two orphaned symlink files—both are low-risk, trivial adjustments with no impact on runtime behavior or business logic.
Re-trigger cubic
Mobile PreviewScan to open on device:
To test on device:
|
There was a problem hiding this comment.
2 issues found across 17 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
* perf: cache body analytics rows * fix: evict failed body cache fetches
There was a problem hiding this comment.
0 issues found across 3 files (changes from recent commits).
Requires human review: Auto-approval blocked by 2 unresolved issues from previous reviews.
Re-trigger cubic
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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/superpowers/plans/2026-06-04-slow-query-optimization-sequence.md`:
- Line 3: The documentation line that mandates an agent-specific skill ("**For
agentic workers:** REQUIRED SUB-SKILL: Use
superpowers:subagent-driven-development (recommended) or
superpowers:executing-plans") makes the docs dependent on agent capabilities and
violates the rule that docs in docs/ must be human-actionable; update the
sentence in the plan to remove the "REQUIRED SUB-SKILL" phrasing and either
delete the agent-specific requirement entirely or convert it to an optional note
such as "Optional: developers familiar with
superpowers:subagent-driven-development or superpowers:executing-plans may use
those tools" so the document remains directly implementable by human developers
(locate and edit the offending line containing
"superpowers:subagent-driven-development" / "superpowers:executing-plans").
In `@packages/server/src/repositories/body-analytics-repository.ts`:
- Around line 324-327: The catch block that currently only evicts the cache and
rethrows the error should also call captureException(error) so failures are
reported to Sentry; update the catch in body-analytics-repository (the block
that contains this.#bodyWeightRowsCache.delete(cacheKey)) to call
captureException(error) before rethrowing, and add the appropriate import for
captureException at the top of the file if missing. Ensure the call uses the
caught error value (typed unknown) and does not suppress the existing cache
eviction or the subsequent throw.
In `@packages/server/src/routers/mobile-dashboard.test.ts`:
- Around line 119-129: Remove the negative test and its supporting mock setup:
delete the hoisted anomalyRepositoryMock declaration and the vi.mock replacement
for AnomalyDetectionRepository (the anomalyRepositoryMock and vi.mock(...)
block) and remove the test case that asserts deleted anomaly behavior (the test
around the `result.anomalies` null assertion and the expectation that
`anomalyRepositoryMock.check` is not called). If needed, replace the removed
test with a positive behavior test that exercises the current public API rather
than asserting the absence of deleted internals (refer to anomalyRepositoryMock,
AnomalyDetectionRepository.check, and the test that checks result.anomalies).
In `@packages/server/src/routers/provider-detail.test.ts`:
- Around line 621-652: The tests "does not select raw payload columns for
activity list records" and "does not select raw payload columns for metric
stream list records" should be changed from absence checks to positive contract
assertions: remove the expect(...not.toMatch(/SELECT \*/)) and
expect(...not.toContain("raw")) lines and instead assert the specific projected
columns you require when calling caller.records (use the same pattern with
mockExecute and extractSqlText(mockExecute.mock.calls[0][0]) to inspect SQL).
For the activity test (the one that calls caller.records({ providerId: "strava",
dataType: "activities" })) assert the exact column names your projection
contract requires (e.g., id, provider_record_id, recorded_at, etc.) and for the
metricStream test (caller.records({ providerId: "apple_health", dataType:
"metricStream" })) keep the positive assertions that check "recorded_at",
"channel", and "scalar" and add any other required projected columns; remove any
assertions that only test the absence of "raw" or "SELECT *".
🪄 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: 53af8dae-ef79-4381-98dd-bd76eb58ca7e
📒 Files selected for processing (26)
analytics/models/read_models/provider_stats.sqlanalytics/models/sources.ymlcspell.jsondeploy/review-apps/CLAUDE.mddeploy/review-apps/GEMINI.mddocs/production-incident-baseline.mddocs/superpowers/plans/2026-06-04-slow-query-optimization-sequence.mddocs/superpowers/specs/2026-06-04-slow-query-optimization-split-design.mdpackages/mobile/app/(tabs)/index.test.tsxpackages/mobile/app/(tabs)/index.tsxpackages/server/src/repositories/body-analytics-repository.test.tspackages/server/src/repositories/body-analytics-repository.tspackages/server/src/repositories/provider-detail-repository.tspackages/server/src/repositories/sync-repository.test.tspackages/server/src/repositories/sync-repository.tspackages/server/src/routers/admin.test.tspackages/server/src/routers/admin.tspackages/server/src/routers/clickhouse-integration-test-helpers.test.tspackages/server/src/routers/clickhouse-integration-test-helpers.tspackages/server/src/routers/mobile-dashboard.test.tspackages/server/src/routers/mobile-dashboard.tspackages/server/src/routers/provider-detail.test.tssrc/db/clickhouse-migrations.test.tssrc/db/clickhouse-read-models.tssrc/db/clickhouse-sql.tssrc/db/clickhouse.test.ts
💤 Files with no reviewable changes (2)
- deploy/review-apps/CLAUDE.md
- deploy/review-apps/GEMINI.md
There was a problem hiding this comment.
0 issues found across 5 files (changes from recent commits).
Requires human review: This PR contains extensive refactoring across analytics models, server repositories, routers, and tests, including new caching logic, query rewrites, and data model changes that affect core performance paths, which carries notable risk of data inconsistency or behavioral regressions requiring...
Re-trigger cubic
There was a problem hiding this comment.
0 issues found across 1 file (changes from recent commits).
Requires human review: The PR touches over 2000 lines across multiple critical subsystems including dashboard anomaly path, provider stats database model, admin table count queries, body analytics caching, and mobile UI, making the blast radius and risk of breakage too high for auto-approval.
Re-trigger cubic
There was a problem hiding this comment.
0 issues found across 2 files (changes from recent commits).
Requires human review: This PR introduces substantial refactoring across multiple critical backend paths—including a new ClickHouse materialized view, changes to provider stats and admin count queries, removal of anomaly computation from the dashboard, and per-instance caching—which carry high risk of breaking data...
Re-trigger cubic
Summary
Validation
Summary by cubic
Fixes the Node
URLimport and reduces slow queries by moving anomaly computation off the dashboard, serving provider/admin stats from a compact ClickHouse read model and Postgres catalog estimates, narrowing provider list queries, and caching body-weight fetches.Refactors
mobileDashboard.dashboard; mobile loads them viatrpc.anomalyDetection.check, and pull-to-refresh refetches both.SyncRepositoryreadsanalytics.provider_stats(ReplacingMergeTreewithis_deleted,refresh_version,refreshed_at); ClickHouse migrations and isolated test helpers updated to preserve the provider-stats SELECT.pg_class.reltuplesestimates withpg_inherits-based chunk rollups formetric_stream.SELECT *.Bug Fixes
URLfromnode:urlin ClickHouse SQL path handling to resolve TypeScript errors.pg_inheritscatalog column names in cspell to pass linting.deploy/review-apps/CLAUDE.md,deploy/review-apps/GEMINI.md).Written for commit 4d73d17. Summary will update on new commits.