feat(activity): canonicalize provider activity types - #2416
Merged
Merged
Conversation
5 tasks
|
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. |
|
Too many files changed for review. ( Bypass the limit by tagging |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Sorry @Asherlc, your pull request is larger than the review limit of 150000 diff characters
|
🤖 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. |
5 tasks
|
Important Review skippedToo many files! This PR contains 316 files, which is 216 over the limit of 100. To get a review, narrow the scope: Upgrade to a paid plan to raise the limit. Usage-priced reviews support at most 300 files. ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (316)
You can disable this status message by setting the 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 |
Contributor
Reviewer's GuideThis PR replaces the legacy single activity_type string with a structured canonical_type, provider_type, and modality across Postgres, ClickHouse/dbt, server repositories/routers, provider parsers, training utilities, and web/mobile clients, ensuring synonyms and subtypes are normalized while preserving raw provider labels and modality semantics. Sequence diagram for provider activity canonicalization and persistencesequenceDiagram
participant Provider as StravaProvider
participant Mapper as createActivityTypeMapper
participant Type as resolveProviderActivityType
participant Upsert as upsertProviderActivity
participant DB as fitness_activity
Provider->>Mapper: mapStravaActivityType(sportType)
Mapper->>Type: resolveProviderActivityType(providerType, legacyType)
Type-->>Mapper: ProviderActivityType
Mapper-->>Provider: ProviderActivityType
Provider->>Upsert: upsertProviderActivity(activityType: ProviderActivityType, ...)
Upsert->>DB: insert activity (canonicalType, providerType, modality)
Upsert->>DB: onConflictDoUpdate(canonicalType, providerType, modality)
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Contributor
|
Storybook previews for This comment updates automatically on each PR push. |
PR Summary by QodoCanonicalize provider activity types into canonical type + provider type + modality
AI Description
Diagram
High-Level Assessment
Files changed (26)
|
Code Review by Qodo
Context used✅ Compliance rules (platform):
300 rules✅ Skills:
fix-provider, write-tests, cloudflare 1.
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
|
🤖 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. |
1 similar comment
|
🤖 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. |
Contributor
Mobile PreviewScan to open on device:
To test on device:
|
|
🤖 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. |
1 similar comment
|
🤖 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. |
|
🤖 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. |
|
🤖 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. |
|
🤖 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. |
|
🤖 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. |
Pull Request Review SummaryOverall, this PR provides a thorough refactoring transitioning the activity schema from a single legacy Findings
🤖 Reviewed by codereviewbot.ai - Catch bugs before your team does. |
![codereviewbot-ai[bot]](https://avatars.githubusercontent.com/in/309638?s=60&v=4){kind=small}
|
🤖 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. |
|
🤖 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. |
|
🤖 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. |
|
🤖 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. |
|
🤖 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. |
|
🤖 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. |
|
🤖 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. |
|
🤖 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
added a commit
that referenced
this pull request
Aug 4, 2026
* fix(clickhouse): repair stale canonical activity type views ClickHouse stores the query text a view is created with, and `standardViewHeader()` emits `CREATE VIEW IF NOT EXISTS`. Migration 0069 renamed `activity_type` to `canonical_type` on `postgres_fitness.activity` and the analytics serving tables while PR #2416 updated the matching view bodies, but neither view was dropped first, so the recreate was a no-op against the existing production objects. `analytics.activity_summary` and `analytics.v_activity` kept selecting a column that no longer exists, and every server read of the summary view failed with UNKNOWN_IDENTIFIER. Migration 0071 drops and recreates the three affected views from the current builders. It then backfills the `provider_type` and `modality` columns that 0069 added without values: the dbt models that own those tables pick dirty keys from `_peerdb_synced_at`, which a mutation does not advance, so historical rows would have stayed null indefinitely. The backfill copies provenance from the replicated activity rows through a temporary Join-engine lookup and `joinGet`, bounded to live rows that are still missing it, and drops the lookup when it finishes. Extracts the duplicated column-existence probe into a shared `hasClickHouseColumn` helper and the migration test client into a local `test-helpers.ts`. Fixes DOFEK-SERVER-53 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MeXWX6Z7h5uWtVByYo26zd * test(clickhouse): name the missing-replica activity fixture in words cspell rejected "unreplicated". Rename the fixture and its assertion to describe the same case with dictionary words rather than widening the project dictionary for one coined term. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MeXWX6Z7h5uWtVByYo26zd * refactor(clickhouse): validate column introspection rows with zod Parse the system.columns response with a Zod schema instead of trusting a TypeScript interface across the ClickHouse runtime boundary. The count comes back as a string for 64-bit integers, so the schema admits both. Also records why provider_type alone marks a row as un-backfilled: a null modality is a legitimate resting state for many canonical types, so widening the filter would rewrite correct rows on every run without converging. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MeXWX6Z7h5uWtVByYo26zd * fix(deps): clear high advisories and align the mobile gesture peer Both checks failed on the base branch and tripped fail-fast, cancelling the unit, integration, and E2E jobs, so nothing in this PR was being exercised by CI. pnpm audit --prod --audit-level=high reported three high advisories, all in transitive dependencies the workspace already pins. Advance those pins to the patched releases rather than lowering the audit level: brace-expansion to 5.0.9 (GHSA-rgw5-rvv9-x895), fast-uri to 3.1.5 (GHSA-7p8r-x3mc-p8w7), and ip-address to 10.4.0 (GHSA-mwp4-54f8-5fhr). The fast-uri selector moves with its advisory range, and staying inside each existing major keeps the parents' ranges satisfied. expo install --check required react-native-gesture-handler ~3.1.0 against a 2.32.0 pin. No workspace source imports the package — it is a peer of expo-router and react-navigation — so the major bump carries no API surface of ours. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MeXWX6Z7h5uWtVByYo26zd * docs(incident): record the OTA healthcheck outage as unresolved Adds the two CI failures fixed alongside this change, including the pnpm 10+ detail that overrides live in pnpm-workspace.yaml rather than package.json, and records the Publish Mobile Preview OTA healthcheck 404 as an unresolved production service state rather than omitting it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MeXWX6Z7h5uWtVByYo26zd * fix(deploy): pass EXPO_APP_ID to the OTA update service expo-open-ota reads EXPO_APP_ID at startup and logs "EXPO_APP_ID not set" without it, leaving dofek_ota at 0/1 so ota.dofek.asherlc.com/hc answers 404 and the preview publish workflow fails its healthcheck before reaching eoas publish. The variable was already present in Infisical, but the ota service in deploy/stack.yml never referenced ${EXPO_APP_ID}, so it was never interpolated into the container. The dotenv template renders every secret at the project root and docker stack deploy runs with that dotenv loaded, so naming the variable is all that was missing. Also adds EXPO_APP_ID to REQUIRED_DEPLOY_KEYS so an absent value fails the deploy with a named key rather than surfacing later as a healthcheck 404, closing the fail-fast follow-up left open by the earlier staging incident. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MeXWX6Z7h5uWtVByYo26zd * fix(mobile): align Expo SDK 57 dependency pins Expo's published SDK 57 native-modules map now expects react-native-gesture-handler ~2.32.0, reversing the ~3.1.0 requirement that motivated yesterday's bump in PR #2420. `pnpm expo install --check` validates against Expo's live version map rather than anything in the repo, so it now fails on main's head too, with the exact same dependency set that passed CI this morning. Root cause: expo, expo-linking, expo-modules-core, expo-router, and expo-updates patch versions and react-native-gesture-handler's expected range all changed upstream between this morning's green run and now. Fix: bump the five expo packages to their now-expected patch versions and revert react-native-gesture-handler to 2.32.0, matching https://api.expo.dev/v2/sdks/57.0.0/native-modules. Verified locally with `pnpm expo install --check` (passes) and `pnpm test:mobile` (176 files, 1476 tests pass). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JvC38Y81GUpfMTy2a9YkCu * docs(incident): disambiguate the fail-fast CI failure count CodeRabbit flagged that "Two CI failures" read ambiguously against the separately-described OTA workflow failure later in the same entry. Scope the count to what it actually describes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JvC38Y81GUpfMTy2a9YkCu * fix(deploy): fail closed on the OTA JWT and signing key secrets CodeRabbit flagged that OTA_JWT_SECRET, OTA_PRIVATE_KEY_B64, and OTA_PUBLIC_KEY_B64 rendered as empty strings rather than failing the deploy when absent. This was previously declined pending confirmation that all three are actually populated in Infisical -- the repository owner has now confirmed that. Add :? guards to the three interpolations in deploy/stack.yml and add them to REQUIRED_DEPLOY_KEYS in scripts/validate-deploy-env.ts, so a missing value fails the deploy with a named key instead of starting the OTA container misconfigured. Validated: `docker stack config -c deploy/stack.yml` renders the ota service correctly with all three vars set, and fails with a named "OTA_JWT_SECRET is required" error when one is unset. Unit tests pass (7/7 in validate-deploy-env.test.ts). tsc --noEmit clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JvC38Y81GUpfMTy2a9YkCu --------- Co-authored-by: Claude <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
0067_canonical_activity_types, preserving dependent views, constraints, indexes, triggers, grants, publication membership, and inbound foreign keysValidation
pnpm typecheckpnpm exec biome check . --max-diagnostics=500pnpm lint:migrationspnpm lint:analytics-policyDeployment note
The PostgreSQL replacement changes the
fitness.activityrelation OID. After applying migration 0067, resync the PeerDB raw activity mirror before relying on replicated activity analytics.Fixes #2245
Summary by Sourcery
Introduce a canonical activity type and modality model across the stack, replacing the legacy activity_type contract and migrating storage, analytics, providers, and client consumers to the new schema.
New Features:
Enhancements:
Tests:
Summary by cubic
Canonicalizes activity types across the stack by replacing
activity_typewithcanonical_type,provider_type, andmodality. Updates analytics, APIs, and mobile/web to filter and display canonical types; unknown provider types default toother. Aligns with #2245.New Features
canonical_activity_typeandactivity_modality; updatedocs/schema.*; add rollout plan; expandcspell.ProviderActivityTypevia@dofek/training/activity-types.canonical_type; propagateprovider_type/modalityin source, deduped, and summary rows; simplify endurance sets to canonical types; hiking treats trail as running (modality='trail'); weekly healthspan countsstrengthbycanonical_type.canonical_type; MCPget_activity_summaryacceptscanonical_typesand groups bycanonical_type; calendars returncanonical_types; activity list/detail includemodality.modality; indoor/virtual cycling logic checks modality;VerticalAscentChartgroups by modality.activity_type→canonical_type; provider detail lists filter bycanonical_type,provider_type, andmodality.Migration
0068_canonical_activity_typeswith atomic table replacement preserving views, constraints, indexes, triggers, grants, publication membership, inbound FKs, and local-time fields inv_activity; relation OID changes.canonical_type='other'; adds tests for fallback and migration edge cases.Written for commit 5226713. Summary will update on new commits.