Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThe changes introduce explicit support for branching in processing plans, updating both backend and frontend logic. The backend now annotates processing steps with branch IDs, executes global and branch-specific steps, and improves retry/tweak logic for branches. The frontend modularizes submission rows, enhances job status display, adds polling for job/step updates, and improves user feedback with toasts and tooltips. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Frontend
participant Backend
User->>Frontend: Clicks "Reprocess" or "Retry" button
Frontend->>Backend: Calls reprocess/retry API
Backend->>Backend: Processes job with branching logic
Backend-->>Frontend: Returns updated job/step data
Frontend->>Frontend: Shows toast (success or error)
Frontend->>Backend: Polls for job/step updates if processing
Backend-->>Frontend: Returns fresh data
Frontend->>User: Updates UI with latest status
Possibly related PRs
Poem
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🔭 Outside diff range comments (1)
apps/app/src/lib/api/processing.ts (1)
19-44: Performance issue: Duplicate API calls for polling detectionThe implementation creates two identical queries - one to check processing state (lines 19-29) and another for the actual data (lines 33-44). This doubles the API calls and is inefficient.
Consider refactoring to use a single query with a custom hook or state management:
export function useProcessingJobs( submissionId: string | null, feedId: string | null, options?: { enabled?: boolean }, ) { - const query = useApiQuery<ProcessingJobsResponse, Error, ProcessingJob[]>( - ["processingJobs", submissionId, feedId], - `/processing/jobs/${submissionId}?feedId=${feedId}`, - { - enabled: - options?.enabled !== undefined - ? options.enabled && !!submissionId && !!feedId - : !!submissionId && !!feedId, - select: (data) => data.data.jobs, - }, - ); - - const isProcessing = query.data?.some((job) => job.status === "processing"); - - return useApiQuery<ProcessingJobsResponse, Error, ProcessingJob[]>( + const query = useApiQuery<ProcessingJobsResponse, Error, ProcessingJob[]>( ["processingJobs", submissionId, feedId], `/processing/jobs/${submissionId}?feedId=${feedId}`, { enabled: options?.enabled !== undefined ? options.enabled && !!submissionId && !!feedId : !!submissionId && !!feedId, select: (data) => data.data.jobs, - refetchInterval: isProcessing ? 5000 : false, + refetchInterval: (data) => + data?.data.jobs?.some((job) => job.status === "processing") ? 5000 : false, }, ); + + return query; }
🧹 Nitpick comments (1)
apps/app/src/routes/_layout/feed/$feedId/_tabs/processing.tsx (1)
36-40: Consider optimizing the latest job calculation.The current implementation sorts the entire array to find the latest job. For better performance with large job lists, consider using
reduce:- const latestJob = jobs?.sort((a, b) => { - const dateA = a.startedAt ? new Date(a.startedAt).getTime() : 0; - const dateB = b.startedAt ? new Date(b.startedAt).getTime() : 0; - return dateB - dateA; - })[0]; + const latestJob = jobs?.reduce((latest, job) => { + const jobTime = job.startedAt ? new Date(job.startedAt).getTime() : 0; + const latestTime = latest?.startedAt ? new Date(latest.startedAt).getTime() : 0; + return jobTime > latestTime ? job : latest; + }, undefined);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
apps/app/src/components/processing/ProcessingActions.tsx(2 hunks)apps/app/src/components/processing/ProcessingHistory.tsx(3 hunks)apps/app/src/lib/api/processing.ts(12 hunks)apps/app/src/routes/_layout/feed/$feedId/_tabs/processing.tsx(3 hunks)packages/core-services/src/services/processing.service.ts(4 hunks)packages/shared-db/src/schema/processing.ts(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
apps/app/src/lib/api/processing.ts (3)
apps/app/src/hooks/api-client.ts (1)
useApiQuery(17-44)packages/types/src/api/processing.ts (4)
ProcessingJobsResponse(152-154)ProcessingJob(90-90)ProcessingStepsResponse(164-166)ProcessingStep(141-141)apps/app/src/hooks/use-toast.ts (1)
toast(211-211)
🔇 Additional comments (13)
packages/shared-db/src/schema/processing.ts (1)
60-62: LGTM! Well-structured schema extension.The addition of the optional
branchIdfield is clean and properly documented. This provides a solid foundation for the branching logic implemented in the processing service.apps/app/src/components/processing/ProcessingHistory.tsx (2)
65-79: Good UX improvement with the tooltip.Truncating job IDs to 8 characters with a tooltip for the full ID is a nice balance between UI cleanliness and information accessibility.
197-197: Consistent button styling.Setting the variant to "outline" aligns with the UI patterns used elsewhere.
apps/app/src/components/processing/ProcessingActions.tsx (1)
41-65: Consider the implications of removing tweak functionality.The UI no longer exposes the tweak-and-reprocess functionality, but the backend still implements
tweakAndReprocessStep. This creates dead code in the backend if the feature is being deprecated.If the tweak functionality is being intentionally removed, consider also removing the
tweakAndReprocessStepmethod from the backend processing service to avoid maintaining unused code.apps/app/src/routes/_layout/feed/$feedId/_tabs/processing.tsx (1)
24-101: Excellent refactoring into a separate component.The
SubmissionRowcomponent improves modularity and makes the code more maintainable. The latest job status display with appropriate badge variants enhances the user experience.packages/core-services/src/services/processing.service.ts (3)
85-144: Well-implemented branching logic.The separation of global steps and branch-specific steps with concurrent branch execution is a solid architectural improvement. The use of
branchIdto group steps is clean and maintainable.
456-461: Good defensive programming with bounds checking.The bounds checking for step indices prevents potential out-of-bounds errors. This is consistent between both
retryStepandtweakAndReprocessStepmethods.
117-143: Verify error handling in concurrent branch processingCurrently, you kick off all branch executions in parallel and await them with
Promise.all. If one branch throws,Promise.allrejects but doesn’t cancel the other running branches, which may lead to partial side-effects (e.g. some transformations/distributions applied, others not).Please confirm whether this behavior is intentional. If not, consider one of the following:
• Wrap each branch promise in its own try/catch so you can handle or aggregate errors per branch.
• UsePromise.allSettledif you need to collect both successes and failures without short-circuiting.
• Add or update tests to cover failure scenarios in one branch while others are still executing.Locations to review:
- packages/core-services/src/services/processing.service.ts (lines 117–143)
apps/app/src/lib/api/processing.ts (5)
8-9: LGTM: Proper imports for enhanced functionalityThe new imports for
useQueryClientandtoastare correctly added to support query invalidation and user notifications in the mutation hooks.
82-82: Well-implemented mutation with proper error handlingThe mutation hook correctly:
- Uses
useQueryClientfor cache invalidation- Provides user feedback with toast notifications
- Handles both success and error cases
- Invalidates the appropriate queries to refresh the UI
Also applies to: 94-112
121-121: Proper query invalidation for step retryThe implementation correctly invalidates both processing steps and jobs queries since a step retry affects both the individual step status and the overall job status.
Also applies to: 133-153
163-163: LGTM: Clean reprocess implementationThe reprocess mutation is well-implemented with proper error handling and query invalidation.
Also applies to: 175-192
202-202: Comprehensive query invalidation for tweak and reprocessThe mutation correctly invalidates both steps and jobs queries, which is appropriate since tweaking a step affects both the step data and the overall job status.
Also applies to: 214-234
1 participant
Summary by CodeRabbit
New Features
Improvements
Bug Fixes