Skip to content

Close QueueTaskModal only when provider launch request returns#1416

Merged
deep1401 merged 3 commits intomainfrom
fix/closing-queue-task-modal
Feb 27, 2026
Merged

Close QueueTaskModal only when provider launch request returns#1416
deep1401 merged 3 commits intomainfrom
fix/closing-queue-task-modal

Conversation

@deep1401
Copy link
Copy Markdown
Member

@deep1401 deep1401 commented Feb 27, 2026

Summary by CodeRabbit

  • New Features

    • Task submission now shows cycling loading messages beside the Submit button to provide progressive status feedback during submission.
  • Improvements

    • Queue modal remains open while a task is being submitted and closes only after the launch completes (success or failure), with the queued task cleared after completion.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Feb 27, 2026

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 26d3ffc and 4214185.

📒 Files selected for processing (1)
  • src/renderer/components/Experiment/Tasks/QueueTaskModal.tsx

📝 Walkthrough

Walkthrough

Keeps the queue modal visible during task submission and adds client-side cycling loading messages shown next to the Submit button while a submission is in progress.

Changes

Cohort / File(s) Summary
Loading Message Animation
src/renderer/components/Experiment/Tasks/QueueTaskModal.tsx
Adds loadingMessages array, loadingMessageIndex state, and an effect that cycles messages every 1.5s while isSubmitting is true; renders current loading message beside the Submit button.
Modal Lifecycle Management
src/renderer/components/Experiment/Tasks/Tasks.tsx
Updates handleQueueSubmit to keep the queue modal open during submission, close it after successful launch (and in finally), and clear the queued task reference when finished.

Sequence Diagram(s)

sequenceDiagram
    participant User as User
    participant UI as QueueTaskModal
    participant Controller as Tasks handler
    participant API as Launch API

    User->>UI: Click "Submit"
    UI->>Controller: startQueueSubmit() (isSubmitting=true)
    Note right of UI: start cycling loadingMessages every 1.5s
    Controller->>API: send launch request
    API-->>Controller: respond (success/failure)
    Controller-->>UI: update result
    Note right of UI: stop cycling, show final message
    Controller->>UI: setQueueModalOpen(false), clear queued task
    UI-->>User: close modal / show outcome
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Suggested reviewers

  • aliasaria
🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title directly and clearly describes the main change: the QueueTaskModal now closes only after the provider launch request completes, rather than closing at submission start.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/closing-queue-task-modal

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
src/renderer/components/Experiment/Tasks/Tasks.tsx (1)

970-973: Minor redundancy: Modal is closed twice on success path.

On success, setQueueModalOpen(false) is called at line 970, and then again in the finally block at line 995. While this is harmless (React state updates are idempotent), consider removing the call here since the finally block handles closure for all code paths.

Alternatively, if the intent is to close the modal before awaiting the mutations (for faster perceived response), the current structure is acceptable.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/renderer/components/Experiment/Tasks/Tasks.tsx` around lines 970 - 973,
The code calls setQueueModalOpen(false) before awaiting
Promise.all([jobsMutate(), templatesMutate()]) and then again in the finally
block, causing a redundant state update; remove the earlier call to
setQueueModalOpen(false) (the one immediately before await Promise.all) so modal
closure is handled centrally in the finally block, leaving jobsMutate(),
templatesMutate(), and setTaskBeingQueued(null) unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@src/renderer/components/Experiment/Tasks/Tasks.tsx`:
- Around line 970-973: The code calls setQueueModalOpen(false) before awaiting
Promise.all([jobsMutate(), templatesMutate()]) and then again in the finally
block, causing a redundant state update; remove the earlier call to
setQueueModalOpen(false) (the one immediately before await Promise.all) so modal
closure is handled centrally in the finally block, leaving jobsMutate(),
templatesMutate(), and setTaskBeingQueued(null) unchanged.

ℹ️ Review info

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between fae46b4 and 26d3ffc.

📒 Files selected for processing (2)
  • src/renderer/components/Experiment/Tasks/QueueTaskModal.tsx
  • src/renderer/components/Experiment/Tasks/Tasks.tsx

@deep1401 deep1401 merged commit 819729d into main Feb 27, 2026
3 of 4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants