Fix climbing attempt totals - #1860
Conversation
Store provider-reported attempts canonically so sends no longer imply a 100% send rate. Backfill historical Kaya rows from preserved raw payloads after deploying migration 0055.
|
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. |
|
🤖 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. |
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? |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
📝 WalkthroughWalkthroughThe PR adds validated climbing attempt counts to the database and Kaya importer, changes climbing reports to sum attempts, and introduces an idempotent dry-run or executable backfill. It also updates tests, operational documentation, and records an Expo validation incident. ChangesClimbing attempt count
Expo validation incident
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Kaya
participant KayaImporter
participant Database
participant ClimbingRepository
Kaya->>KayaImporter: CSV attempts
KayaImporter->>Database: store attempt_count
ClimbingRepository->>Database: SUM(attempt_count)
Database-->>ClimbingRepository: aggregated attempts
Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 1 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (1 passed)
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. |
|
🤖 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. |
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code 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. |
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
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. |
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code 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. |
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code 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. |
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code 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. |
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
…m-instructions-v1 # Conflicts: # docs/production-incident-baseline.md # packages/server/src/routers/climbing.integration.test.ts
|
🤖 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. |
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 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 `@packages/server/src/repositories/climbing-repository.ts`:
- Around line 248-249: Remove the ::int casts from both SUM(ce.attempt_count)
and SUM(attempt_count) expressions in the repository queries, preserving
PostgreSQL’s bigint aggregate type. Leave the z.coerce.number() normalization
unchanged so cumulative attempt totals above the 32-bit integer range remain
supported.
In `@scripts/backfill-climbing-attempt-count.test.ts`:
- Around line 23-43: Add a test in the existing main suite covering rejection
from backfillClimbingAttemptCount: configure the mock to reject with an error,
invoke main with the appropriate arguments, and assert Sentry.captureException
receives that error (and preserve the expected rejection behavior if main
rethrows it). Keep the existing dry-run and unknown-option tests unchanged.
In `@scripts/backfill-climbing-attempt-count.ts`:
- Around line 12-37: Update main around createDatabaseFromEnv and
backfillClimbingAttemptCount to retain the database pool and call its close/end
cleanup after the backfill completes, including error paths. Ensure pool cleanup
occurs before or alongside the existing Sentry.close in finally so the one-off
CLI does not leave idle connections open.
In `@src/db/climbing-attempt-count-backfill.ts`:
- Around line 5-36: The backfill function currently calls db.execute and
separately parses the result with countRowSchema. Update
backfillClimbingAttemptCount to use executeWithSchema from typed-sql.ts for both
SQL branches, passing countRowSchema for centralized validation and row
handling, and return the validated count without the manual parse.
In `@src/providers/kaya/import.ts`:
- Around line 320-323: Update the attempts validation in the importer to use the
established Zod-based runtime validation required for untrusted input, accepting
only integer values from 1 through 2,147,483,647. Preserve the existing
row-level error result for invalid values, and add coverage for the upper bound
and values above it.
🪄 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: 54a38fb1-03e2-449e-927e-d71831d95220
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (17)
docs/production-incident-baseline.mddrizzle/0055_climbing_attempt_count.sqldrizzle/meta/_journal.jsonpackage.jsonpackages/server/src/repositories/climbing-repository.test.tspackages/server/src/repositories/climbing-repository.tspackages/server/src/routers/climbing.integration.test.tsscripts/backfill-climbing-attempt-count.test.tsscripts/backfill-climbing-attempt-count.tssrc/db/README.mdsrc/db/climbing-attempt-count-backfill.integration.test.tssrc/db/climbing-attempt-count-backfill.test.tssrc/db/climbing-attempt-count-backfill.tssrc/db/drizzle-schema.test.tssrc/db/schema/activity.tssrc/providers/kaya/import.test.tssrc/providers/kaya/import.ts
…m-instructions-v1 # Conflicts: # docs/production-incident-baseline.md
|
🤖 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. |
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
Summary
Validation
pnpm lintpnpm test(12,966 passed, 21 skipped)Summary by cubic
Fixes climbing attempt totals by adding a canonical
attempt_countand summing it in server queries. Adds an idempotent Kaya backfill and incident docs; also aligns mobileexpopatches and uses Expo’s online dependency validation in CI.Bug Fixes
attempt_counttofitness.climbing_entrywith a positive check; default1.SUM(attempt_count)instead of row counts.attemptsas a positive 32-bit integer, rejects invalid values, and writesattemptCountcanonically.Migration
0055_climbing_attempt_count(CHECK added NOT VALID then VALIDATED).pnpm backfill:climbing-attempt-count(dry run) then--execute(idempotent).Written for commit 4551553. Summary will update on new commits.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation