features: viewing/cancelling jobs from the projectdetail page, improving "view" on the popup toasts - #201
Conversation
…m the projectdetail page, in the event the user leaves the page -> this also persists upon page reload
…t step once process was done
ex. "view" predict_job succeeded -> opens generated files tab on projectdetail page
|
Warning Review limit reached
Next review available in: 31 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (6)
📝 WalkthroughWalkthroughChangesActive job flow
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant App
participant AppRouter
participant ProjectDetail
participant ProcessingPage
App->>AppRouter: pass resumeJob and pendingProjectTab
ProjectDetail->>AppRouter: request active-job view
AppRouter->>ProcessingPage: reconnect with job ID and kind
ProcessingPage-->>AppRouter: report completion or back navigation
AppRouter->>App: clear resumed-job state
Possibly related issues
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
landing-page/scripts/job_store.py (1)
121-149: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winAlign the active-job documentation with the returned shapes.
get_active_job_for_projectreturnsstatus, but its docstring omits it.claim_project_jobreturnsactivewithoutstatus, so its “same shape” claim is incorrect. The current 409 handlers read onlyjob_idandkind. Update both docstrings to state the actual shapes.🤖 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 `@landing-page/scripts/job_store.py` around lines 121 - 149, Update the docstrings for get_active_job_for_project and claim_project_job to document their actual return shapes: get_active_job_for_project returns job_id, kind, and status, while claim_project_job returns active without status. Remove or revise any claim that both results have the same shape, without changing the 409 handlers or runtime behavior.
🧹 Nitpick comments (2)
landing-page/scripts/projects_api.py (1)
238-239: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRename the unused
conbinding to satisfy Ruff.Ruff reports RUF059 for line 238. Only
curis used.♻️ Proposed fix
- with db_cursor() as (con, cur): + with db_cursor() as (_con, cur): require_project_owner(cur, project_id, user["id"])🤖 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 `@landing-page/scripts/projects_api.py` around lines 238 - 239, Rename the unused con binding in the db_cursor() context manager unpacking to the convention for intentionally ignored values, while retaining cur for require_project_owner().Source: Linters/SAST tools
landing-page/src/hooks/useProjectActiveJob.ts (1)
47-51: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valuePolling stops when a new job starts in this tab while
jobisnull.The effect at line 47 only starts an interval when
jobis already set. The comment documents the cross-tab tradeoff. There is a same-tab case too:ProjectDetailstays mounted, the user starts a job from another route, returns to the project page without a remount, and this hook keepsjob === null. The in-memory registry covers that path, so the impact is limited to the missingstatuslabel.Consider exposing
refetchActiveJobcalls on registry changes, or accept the gap and record it in the comment.🤖 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 `@landing-page/src/hooks/useProjectActiveJob.ts` around lines 47 - 51, The polling effect in useProjectActiveJob currently stops when job is null, so same-tab registry updates do not refresh the status label. Update the hook to trigger refetchActiveJob when the in-memory active-job registry changes, including the null-job state, while preserving the existing polling behavior and cleanup.
🤖 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 `@landing-page/src/App.tsx`:
- Around line 134-148: The project step update currently runs concurrently with
the refetch and can be overwritten by a stale GET response. In the project
refresh flow around apiFetch and updateProjectSteps, chain the update after the
refetch applies, and enforce the minimum using normalized.stepsUnlocked so the
server value is preserved.
In `@landing-page/src/components/AppRouter.tsx`:
- Around line 490-501: Update the onResult handler in AppRouter to branch on the
effective job kind, using resumeJob?.kind when resuming and the existing
kickoff-derived kind otherwise, rather than directly checking batchRunIds.
Ensure resumed text_batch jobs use batch-result handling and resumed predict
jobs use annotation handling, regardless of stale or missing batchRunIds.
- Around line 359-362: Update the ProcessingPage element rendered by AppRouter
to use a React key derived from resumeJob.jobId, ensuring a changed resumed job
forces a remount and restarts its stream. Apply this to the shared
processing-view render path so both onViewActiveJob and the App.tsx toast
handler are covered.
In `@landing-page/src/components/project/ProjectDetail.tsx`:
- Around line 993-999: Update the status text in the ProjectDetail component to
select the article dynamically based on jobKindLabel(activeJobForProject.kind),
using “a” for consonant-starting labels and “an” for vowel-starting labels.
Preserve the existing queued/running status wording and job label rendering.
- Around line 192-205: Update handleCancelActiveJob to validate the apiFetch
cancellation response before calling markJobSettled, preserving the local job
and prompt when the server returns a non-success status. Add cancelError state
handling for failed responses and network rejections, clear it on a new attempt,
and render cancelError next to the status text while retaining the existing
cancellingJob cleanup.
In `@landing-page/src/hooks/useProjectActiveJob.ts`:
- Around line 24-27: The active-job response uses snake_case while
ProjectActiveJob consumers expect camelCase. In
landing-page/src/hooks/useProjectActiveJob.ts lines 24-27, explicitly map job_id
to jobId while preserving kind and status; then verify no direct changes are
needed in ProjectDetail.tsx lines 175-188, 192-205, and 1001-1009, since the
mapping should restore job matching, produce a valid cancel URL, and provide
kind for the progress flow.
---
Outside diff comments:
In `@landing-page/scripts/job_store.py`:
- Around line 121-149: Update the docstrings for get_active_job_for_project and
claim_project_job to document their actual return shapes:
get_active_job_for_project returns job_id, kind, and status, while
claim_project_job returns active without status. Remove or revise any claim that
both results have the same shape, without changing the 409 handlers or runtime
behavior.
---
Nitpick comments:
In `@landing-page/scripts/projects_api.py`:
- Around line 238-239: Rename the unused con binding in the db_cursor() context
manager unpacking to the convention for intentionally ignored values, while
retaining cur for require_project_owner().
In `@landing-page/src/hooks/useProjectActiveJob.ts`:
- Around line 47-51: The polling effect in useProjectActiveJob currently stops
when job is null, so same-tab registry updates do not refresh the status label.
Update the hook to trigger refetchActiveJob when the in-memory active-job
registry changes, including the null-job state, while preserving the existing
polling behavior and cleanup.
🪄 Autofix
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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 410f6e18-c38e-4895-84c7-c043a0922c3c
📒 Files selected for processing (9)
landing-page/scripts/job_store.pylanding-page/scripts/projects_api.pylanding-page/src/App.tsxlanding-page/src/components/AppRouter.tsxlanding-page/src/components/project/ProjectDetail.tsxlanding-page/src/components/workflow/ProcessingPage.tsxlanding-page/src/hooks/useProjectActiveJob.tslanding-page/src/lib/activeJobs.tslanding-page/src/types.ts
1 participant
refs: #195, #196
Summary by CodeRabbit