Report BullMQ progress for all jobs - #1597
Conversation
|
🤖 Review skipped: Rate limit exceeded. Free accounts are limited to 2 reviews per 4 hours. 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.
Asherlc has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.
📝 WalkthroughWalkthroughProgress reporting was added to analytics refresh, FIT import, provider import, post-sync, scheduled sync, and Garmin dump workflows. Job interfaces and provider options now accept progress callbacks, with tests covering staged percentages, messages, and failure handling. ChangesProgress reporting
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant processImportJob
participant importGarminDumpFile
participant enqueueFitFileImportJobs
participant BullMQJob
processImportJob->>importGarminDumpFile: provide onProgress callback
importGarminDumpFile->>enqueueFitFileImportJobs: enqueue FIT imports
enqueueFitFileImportJobs->>importGarminDumpFile: report child completion
importGarminDumpFile->>processImportJob: forward progress payload
processImportJob->>BullMQJob: updateProgress percentage and message
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 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 |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 12 files
Confidence score: 4/5
- In
src/jobs/process-import-job.ts, the Garmin progress helper fire-and-forgets BullMQ progress writes, soimportGarminDumpFilecan report parent progress out of order or late relative to later stages; merging as-is mainly risks misleading job status/UX rather than import correctness — make the progress callback await (or otherwise serialize) BullMQ writes before merging to keep stage progress monotonic.
You’re at about 97% of the monthly reviewed-line limit. You may want to disable incremental reviews to conserve quota. Reviews will continue until that limit is exceeded. If you need help avoiding interruptions, please contact contact@cubic.dev.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/jobs/process-import-job.ts">
<violation number="1" location="src/jobs/process-import-job.ts:37">
P2: Garmin parent progress can lag or arrive after a later stage because this helper fire-and-forgets every BullMQ progress write; `importGarminDumpFile` explicitly awaits its `onProgress` callback, but this callback returns `void`. Returning the update promise and awaiting the stage reports would preserve progress ordering before the processor advances or completes.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
|
🤖 Review skipped: Rate limit exceeded. Free accounts are limited to 2 reviews per 4 hours. Upgrade to a paid plan for unlimited reviews. |
There was a problem hiding this comment.
Asherlc has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.
There was a problem hiding this comment.
0 issues found across 2 files (changes from recent commits).
You’re at about 98% of the monthly reviewed-line limit. You may want to disable incremental reviews to conserve quota. Reviews will continue until that limit is exceeded. If you need help avoiding interruptions, please contact contact@cubic.dev.
Requires human review: Auto-approval blocked by 11 unresolved issues from previous reviews.
Re-trigger cubic
|
🤖 Review skipped: Rate limit exceeded. Free accounts are limited to 2 reviews per 4 hours. Upgrade to a paid plan for unlimited reviews. |
There was a problem hiding this comment.
Asherlc has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.
There was a problem hiding this comment.
0 issues found across 1 file (changes from recent commits).
You’re at about 98% of the monthly reviewed-line limit. You may want to disable incremental reviews to conserve quota. Reviews will continue until that limit is exceeded. If you need help avoiding interruptions, please contact contact@cubic.dev.
Requires human review: Auto-approval blocked by 11 unresolved issues from previous reviews.
Re-trigger cubic
There was a problem hiding this comment.
Asherlc has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.
|
🤖 Review skipped: Rate limit exceeded. Free accounts are limited to 2 reviews per 4 hours. Upgrade to a paid plan for unlimited reviews. |
There was a problem hiding this comment.
0 issues found across 4 files (changes from recent commits).
You’re at about 98% of the monthly reviewed-line limit. You may want to disable incremental reviews to conserve quota. Reviews will continue until that limit is exceeded. If you need help avoiding interruptions, please contact contact@cubic.dev.
Requires human review: Auto-approval blocked by 11 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: 11
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/jobs/process-import-job.ts (2)
1-1: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winProgress-update catches skip
captureExceptionin both new helpers. BothupdateImportJobProgressandupdateFitFileImportProgresswere introduced by this PR to report BullMQ progress, but both catches only calllogger.warn, neverSentry.captureException. As per path instructions,**/*.{ts,tsx}: "Never silently swallow errors — every unexpected catch must call captureException()."
src/jobs/process-import-job.ts#L37-41: addSentry.captureException(error, { tags: { phase: "import-progress-update" } })inside the catch (Sentry is already imported and used elsewhere in this file at lines 233-234/241-242).src/jobs/process-fit-file-import-job.ts#L83-90: add the equivalentcaptureExceptioncall inupdateFitFileImportProgress's catch (verify Sentry is imported in this file; add the import if not).🤖 Prompt for 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. In `@src/jobs/process-import-job.ts` at line 1, Update the catch blocks in updateImportJobProgress and updateFitFileImportProgress to call Sentry.captureException with the caught error and the specified phase tag, while preserving their existing logger.warn behavior. Ensure process-fit-file-import-job.ts imports Sentry if needed, and do not alter unrelated error handling.Source: Path instructions
172-196: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winMove the ZOS App completion update after the failure check
reportImportProgress(job, 90, "ZOS App import complete.")runs before therecordsSynced === 0 && errors.length > 0throw, so a failing import can surface as “complete” just before rejecting. The0x00test case still passes here becauseimportZosAppBinis mocked to resolve success by default, so it does not hit the decode-failure path.🤖 Prompt for 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. In `@src/jobs/process-import-job.ts` around lines 172 - 196, Move the `reportImportProgress(job, 90, "ZOS App import complete.")` call in the `zos-app` branch to after the `result.recordsSynced === 0 && result.errors.length > 0` failure check, ensuring failed imports throw before reporting completion.src/providers/garmin-dump.ts (1)
439-470: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winCall
onJobFinishedoutside the job-failure catch
Ifoptions.onProgressthrows, thiscatchturns a successful FIT job into a synthetic failure, incrementscompletedCounttwice, and hides the original result. AGENTS.md also says every unexpectedcatchmust callcaptureException(). Split the progress callback from thewaitUntilFinished()/schema-parse failure path.🤖 Prompt for 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. In `@src/providers/garmin-dump.ts` around lines 439 - 470, Separate the onJobFinished progress callback from the try/catch around waitUntilFinished and fitFileImportJobResultSchema.parse so a progress error cannot convert a successful job into a synthetic failure or increment completedCount twice. Preserve failure-result creation only for job wait/parse errors, and call captureException() for unexpected caught errors as required by AGENTS.md.
🤖 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 `@src/jobs/process-activity-delete-analytics-job.ts`:
- Around line 30-73: Extract the duplicated rebuild, cache invalidation,
logging, and completion-progress sequence from the job handler into a shared
helper, parameterized by the branch-specific log text and an optional pre-wait
callback. Update the activity recompute, restore, and delete branches to perform
only their distinct waiting/progress steps before invoking the helper,
preserving their existing messages and behavior.
- Around line 13-14: Replace the broad updateProgress payload type in the
relevant job interface with an explicit object contract containing percentage
and message, using the appropriate primitive types inferred from every call site
in the file. Keep updateProgress returning Promise<void> and update all related
references only if needed to satisfy the narrowed contract.
- Around line 29-73: The updateActivityAnalyticsProgress helper should make
job.updateProgress failures non-fatal: wrap the awaited progress update in a
try/catch, report caught errors through captureException(), and return without
propagating them so analytics rebuild and cache invalidation remain successful
independently of progress reporting.
In `@src/jobs/process-fit-file-import-job.ts`:
- Around line 83-90: Update updateFitFileImportProgress so its catch handler
reports the caught error to Sentry via captureException in addition to the
existing logger.warn call. Preserve the current progress-update behavior and
warning context.
In `@src/jobs/process-import-job.ts`:
- Around line 37-41: Update updateImportJobProgress so its catch handler reports
the caught error through Sentry.captureException in addition to the existing
warning log, matching the error-reporting pattern used by other unexpected
catches in this file.
- Around line 37-45: Update updateImportJobProgress to return the promise from
job.updateProgress(info), while preserving its warning handling, and change
reportImportProgress to return that promise so callers such as
GarminDumpImportOptions.onProgress can await progress delivery and maintain
ordering.
In `@src/jobs/process-post-sync-job.ts`:
- Around line 11-19: Update updatePostSyncProgress to use a typed payload
containing only percentage and message, and catch failures from
job.updateProgress so progress-reporting errors are tolerated rather than
propagated into processPostSyncJob. Preserve the existing progress payload
values and job-processing flow.
In `@src/jobs/process-scheduled-sync-job.test.ts`:
- Around line 74-80: Update createScheduledSyncJob and the listed test call
sites to use direct processScheduledSyncJob invocations instead of Reflect.apply
with an undefined receiver and argument array. Ensure processScheduledSyncJob
accepts the local ScheduledSyncJob interface so each call passes the job and
dependencies directly while preserving existing test behavior.
In `@src/jobs/process-scheduled-sync-job.ts`:
- Around line 16-22: Apply the consolidated progress-callback error-handling
behavior to updateScheduledSyncProgress, keeping it consistent with the
corresponding implementation in process-post-sync-job.ts. Update the
job.updateProgress call as required while preserving the function’s existing
inputs and Promise<void> contract.
- Around line 11-14: Update the processScheduledSyncJob parameter type from
Job<ScheduledSyncJobData> to the local ScheduledSyncJob interface, then remove
the unused Job import. Preserve the existing function behavior while allowing
tests to invoke it directly without Reflect.apply.
In `@src/providers/garmin-dump.ts`:
- Around line 159-166: Update reportGarminDumpProgress to protect the optional
options.onProgress callback from rejected promises, matching the error-handling
behavior of updateFitFileImportProgress and updateImportJobProgress. Handle the
rejection within this helper so errors do not propagate into
importGarminDumpFile or its enqueueFitFileImportJobs callbacks.
---
Outside diff comments:
In `@src/jobs/process-import-job.ts`:
- Line 1: Update the catch blocks in updateImportJobProgress and
updateFitFileImportProgress to call Sentry.captureException with the caught
error and the specified phase tag, while preserving their existing logger.warn
behavior. Ensure process-fit-file-import-job.ts imports Sentry if needed, and do
not alter unrelated error handling.
- Around line 172-196: Move the `reportImportProgress(job, 90, "ZOS App import
complete.")` call in the `zos-app` branch to after the `result.recordsSynced ===
0 && result.errors.length > 0` failure check, ensuring failed imports throw
before reporting completion.
In `@src/providers/garmin-dump.ts`:
- Around line 439-470: Separate the onJobFinished progress callback from the
try/catch around waitUntilFinished and fitFileImportJobResultSchema.parse so a
progress error cannot convert a successful job into a synthetic failure or
increment completedCount twice. Preserve failure-result creation only for job
wait/parse errors, and call captureException() for unexpected caught errors as
required by AGENTS.md.
🪄 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: 9d47ea82-861c-4c62-9aeb-d188c1073965
📒 Files selected for processing (12)
src/jobs/process-activity-delete-analytics-job.test.tssrc/jobs/process-activity-delete-analytics-job.tssrc/jobs/process-fit-file-import-job.test.tssrc/jobs/process-fit-file-import-job.tssrc/jobs/process-import-job.test.tssrc/jobs/process-import-job.tssrc/jobs/process-post-sync-job.test.tssrc/jobs/process-post-sync-job.tssrc/jobs/process-scheduled-sync-job.test.tssrc/jobs/process-scheduled-sync-job.tssrc/providers/garmin-dump.test.tssrc/providers/garmin-dump.ts
|
🤖 Review skipped: Rate limit exceeded. Free accounts are limited to 2 reviews per 4 hours. Upgrade to a paid plan for unlimited reviews. |
There was a problem hiding this comment.
Asherlc has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.
|
Addressed the actionable PR-level review summaries from Cubic (submitted 2026-07-13T21:55:11Z) and CodeRabbit (submitted 2026-07-13T22:33:31Z).\n\nFixed.\n- Import progress writes now return/await Promise, so Garmin dump parent progress preserves ordering.\n- Progress-write failures in import, FIT import, post-sync, scheduled sync, activity analytics, and Garmin dump are best-effort and reported to Sentry.\n- ZOS App completion progress now runs after the zero-record failure check.\n- Garmin child FIT completion progress is separated from child job wait/parse failures so progress failures do not replace successful child results.\n- Scheduled-sync tests now call the processor directly, and progress milestone tests cover the missing assertions.\n- Commit: 1d8b8c2 |
There was a problem hiding this comment.
All reported issues were addressed across 12 files (changes from recent commits).
You’re at about 98% of the monthly reviewed-line limit. You may want to disable incremental reviews to conserve quota. Reviews will continue until that limit is exceeded. If you need help avoiding interruptions, please contact contact@cubic.dev.
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
|
🤖 Review skipped: Rate limit exceeded. Free accounts are limited to 2 reviews per 4 hours. Upgrade to a paid plan for unlimited reviews. |
There was a problem hiding this comment.
Asherlc has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.
There was a problem hiding this comment.
0 issues found across 1 file (changes from recent commits).
You’re at about 98% of the monthly reviewed-line limit. You may want to disable incremental reviews to conserve quota. Reviews will continue until that limit is exceeded. If you need help avoiding interruptions, please contact contact@cubic.dev.
Requires human review: Auto-approval blocked by 2 unresolved issues from previous reviews.
Re-trigger cubic
|
🤖 Review skipped: Rate limit exceeded. Free accounts are limited to 2 reviews per 4 hours. Upgrade to a paid plan for unlimited reviews. |
There was a problem hiding this comment.
Asherlc has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.
There was a problem hiding this comment.
0 issues found across 4 files (changes from recent commits).
You’re at about 98% of the monthly reviewed-line limit. You may want to disable incremental reviews to conserve quota. Reviews will continue until that limit is exceeded. If you need help avoiding interruptions, please contact contact@cubic.dev.
Auto-approved: Adds BullMQ progress reporting to job processors with defensive error handling.
Re-trigger cubic
There was a problem hiding this comment.
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 `@src/jobs/process-post-sync-job.ts`:
- Around line 14-23: Extract the duplicated progress failure-reporting logic
from updatePostSyncProgress into a shared reportJobProgress helper in a common
jobs module, parameterized by the Sentry tag key while preserving the existing
warning and exception-capture behavior. Update src/jobs/process-post-sync-job.ts
lines 14-23 to use the helper with postSyncStep, and replace
updateScheduledSyncProgress in src/jobs/process-scheduled-sync-job.ts lines
16-25 with the same helper using scheduledSyncStep.
In `@src/providers/garmin-dump.ts`:
- Around line 461-472: Update the error construction in the FIT child import
failure path to avoid using parsedResult.error.message directly, which exposes
the raw JSON-formatted Zod issues array. Use a concise, user-readable validation
failure message for SyncResult.errors while preserving the detailed
parsedResult.error for Sentry.captureException.
🪄 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: da28887c-88ea-42b1-840e-93f603d36c0e
📒 Files selected for processing (12)
src/jobs/process-activity-delete-analytics-job.test.tssrc/jobs/process-activity-delete-analytics-job.tssrc/jobs/process-fit-file-import-job.test.tssrc/jobs/process-fit-file-import-job.tssrc/jobs/process-import-job.test.tssrc/jobs/process-import-job.tssrc/jobs/process-post-sync-job.test.tssrc/jobs/process-post-sync-job.tssrc/jobs/process-scheduled-sync-job.test.tssrc/jobs/process-scheduled-sync-job.tssrc/providers/garmin-dump.test.tssrc/providers/garmin-dump.ts
|
🤖 Review skipped: Rate limit exceeded. Free accounts are limited to 2 reviews per 4 hours. Upgrade to a paid plan for unlimited reviews. |
There was a problem hiding this comment.
Asherlc has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.
|
Addressed the CodeRabbit PR-level summary submitted 2026-07-13T23:39:55Z. Fixed.
|
|
🤖 Review skipped: Rate limit exceeded. Free accounts are limited to 2 reviews per 4 hours. Upgrade to a paid plan for unlimited reviews. |
There was a problem hiding this comment.
Asherlc has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.
Summary
Tests
Summary by cubic
Adds resilient, standardized BullMQ progress reporting across imports, scheduled sync, post‑sync, and analytics.
garmin-dumpnow reports start/read/found counts and per‑FIT child updates; malformed child results are handled safely and do not block the parent.New Features
strong-csv,cronometer-csv,kaya-export,zos-appwith start/read/import/complete;garmin-dumpprogress forwarded to BullMQ.garmin-dump: start/reading/found counts; per‑FIT completion updates as each child finishes; final complete.Bug Fixes
import-progress-update,scheduledSyncStep,postSyncStep,fitImportStep,activityAnalyticsStep,garminDumpStep.garmin-dumpsafely handles malformed FIT child results (reported to Sentry) and continues; child failures and progress callback errors are captured without aborting.zos-app(completion logged with error details before throwing).reportJobProgresshelper standardizes progress logging and Sentry tagging across jobs.Written for commit 236892b. Summary will update on new commits.
Summary by CodeRabbit
New Features
Bug Fixes