Skip to content

[Dashboard] Add recovery count to managed job detail page#10004

Open
stephenkevn wants to merge 1 commit into
skypilot-org:masterfrom
stephenkevn:se/jobs-recovery-count
Open

[Dashboard] Add recovery count to managed job detail page#10004
stephenkevn wants to merge 1 commit into
skypilot-org:masterfrom
stephenkevn:se/jobs-recovery-count

Conversation

@stephenkevn

@stephenkevn stephenkevn commented Jul 1, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • Add a "Recoveries" field to the managed job detail page's Details
    section, so the recovery count is visible at a glance instead of only
    in the jobs list.
  • When a job has recovered, the count itself links to the events section
    (smooth-scroll). The link only renders when an events view is present;
    otherwise the count shows as plain text, and the field shows 0 when
    there are no recoveries.

Test plan

  • Ran the dashboard locally against an API server and opened a managed
    job detail page. Verified the Recoveries field renders in Details:
    shows 0 with no recoveries, shows the count as a link that scrolls to
    the events section when > 0, and shows the count as plain text when no
    events view is present.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request adds a "Recoveries" section to the job details page, allowing users to view the number of recoveries and smoothly scroll to the events section if the events plugin is active. The feedback recommends improving the robustness of the recoveries check by explicitly verifying if jobData.recoveries > 0 instead of using a loose truthy check, which could lead to unexpected behavior with falsy or non-numeric values.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread sky/dashboard/src/pages/jobs/[job].js Outdated
Comment on lines +1391 to +1409
{jobData.recoveries ? (
<span className="inline-flex items-center">
{jobData.recoveries}
{eventsSlotHasPlugin && (
<button
onClick={() =>
document
.getElementById('events-section')
?.scrollIntoView({ behavior: 'smooth' })
}
className="text-blue-600 hover:underline ml-2"
>
View events
</button>
)}
</span>
) : (
0
)}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

Using a truthy check (jobData.recoveries ? ...) can lead to unexpected behavior if recoveries is a string like "0" (which is truthy in JavaScript) or if it is undefined/null. It is safer and more robust to explicitly check if jobData.recoveries > 0 to ensure the "View events" link is only displayed when there are actual recoveries.

Suggested change
{jobData.recoveries ? (
<span className="inline-flex items-center">
{jobData.recoveries}
{eventsSlotHasPlugin && (
<button
onClick={() =>
document
.getElementById('events-section')
?.scrollIntoView({ behavior: 'smooth' })
}
className="text-blue-600 hover:underline ml-2"
>
View events
</button>
)}
</span>
) : (
0
)}
{jobData.recoveries > 0 ? (
<span className="inline-flex items-center">
{jobData.recoveries}
{eventsSlotHasPlugin && (
<button
onClick={() =>
document
.getElementById('events-section')
?.scrollIntoView({ behavior: 'smooth' })
}
className="text-blue-600 hover:underline ml-2"
>
View events
</button>
)}
</span>
) : (
0
)}

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Thanks — adopted the explicit jobData.recoveries > 0 check. The field was also revised so the recovery count itself is the link (the separate "View events" button was removed); the link only renders when an events view is registered, otherwise the count shows as plain text.

@stephenkevn stephenkevn force-pushed the se/jobs-recovery-count branch from a5d1614 to 084150f Compare July 1, 2026 01:00
The managed job detail page showed no recovery count in its Details
section, so users had to scroll to the events table (or infer it) to see
how many times a job recovered.

Add a "Recoveries" field to the Details grid. When a job has recovered,
show the count plus a "View events" link that scrolls to the events
table; otherwise show 0. Reuses the existing scroll-anchor pattern.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@stephenkevn stephenkevn force-pushed the se/jobs-recovery-count branch from 084150f to 955fb54 Compare July 1, 2026 01:07
@stephenkevn stephenkevn requested a review from Michaelvll July 1, 2026 01:12
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.

1 participant