1871: feat(tui): Show job failed status below the Jobs table - #74
1871: feat(tui): Show job failed status below the Jobs table#74martin-augment wants to merge 2 commits into
Conversation
WalkthroughThe ✨ 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 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a job_status field to the Job struct to capture human-readable failure details and updates the TUI to display this reason in a panel below the jobs table when a failed job is selected. It also refactors the scrollbar layout logic to return a fixed-size array. The review feedback suggests adding #[serde(default)] to the job_status field to ensure backward compatibility during deserialization, and implementing a height guard in the UI layout to prevent rendering issues on small terminal screens.
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.
| pub status: String, // Running, Completed, Failed, Canceled | ||
| pub job_status: String, // human-readable status/failure detail, e.g. "Failed: <reason>" |
There was a problem hiding this comment.
To ensure backward compatibility and robustness against older scheduler versions or cases where the job_status field is omitted from the API response, it is highly recommended to mark this field with #[serde(default)]. This prevents deserialization failures when the field is missing.
| pub status: String, // Running, Completed, Failed, Canceled | |
| pub job_status: String, // human-readable status/failure detail, e.g. "Failed: <reason>" | |
| pub status: String, // Running, Completed, Failed, Canceled | |
| #[serde(default)] | |
| pub job_status: String, // human-readable status/failure detail, e.g. "Failed: <reason>" |
| match selected_job { | ||
| Some(job) if job.status == "Failed" => { | ||
| let areas = Layout::vertical([ | ||
| Constraint::Min(5), // Table | ||
| Constraint::Length(5), // Failure reason | ||
| ]) | ||
| .split(area); | ||
| (areas[0], Some((areas[1], job))) | ||
| } | ||
| _ => (area, None), | ||
| } |
There was a problem hiding this comment.
If the terminal window is resized to a very small height, splitting the area with a fixed constraint of Constraint::Length(5) for the failure reason and Constraint::Min(5) for the table can cause layout overlap or hide the table entirely. Adding a guard to only split the area when there is sufficient vertical space (e.g., area.height >= 10) ensures a more robust and adaptive UI.
| match selected_job { | |
| Some(job) if job.status == "Failed" => { | |
| let areas = Layout::vertical([ | |
| Constraint::Min(5), // Table | |
| Constraint::Length(5), // Failure reason | |
| ]) | |
| .split(area); | |
| (areas[0], Some((areas[1], job))) | |
| } | |
| _ => (area, None), | |
| } | |
| match selected_job { | |
| Some(job) if job.status == "Failed" && area.height >= 10 => { | |
| let areas = Layout::vertical([ | |
| Constraint::Min(5), // Table | |
| Constraint::Length(5), // Failure reason | |
| ]) | |
| .split(area); | |
| (areas[0], Some((areas[1], job))) | |
| } | |
| _ => (area, None), | |
| } |
AI Code Review
|
🤖 Augment PR SummarySummary: Enhances the Ballista CLI TUI Jobs view to show detailed failure information for failed jobs. Key changes:
🤖 Was this summary useful? React with 👍 or 👎 |
![augmentcode[bot]](https://avatars.githubusercontent.com/in/1027498?s=60&v=4){kind=small}
| pub job_name: String, | ||
| pub status: String, // Running, Completed, Failed, Canceled | ||
| pub status: String, // Running, Completed, Failed, Canceled | ||
| pub job_status: String, // human-readable status/failure detail, e.g. "Failed: <reason>" |
There was a problem hiding this comment.
Job derives Deserialize, and the new required job_status field means the CLI will fail to parse /jobs responses from schedulers that don’t include this field (i.e., reduced backward-compatibility). If mixed-version deployments are expected, this can become a runtime break rather than a compile-time one.
Severity: medium
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
| Some(job) if job.status == "Failed" => { | ||
| let areas = Layout::vertical([ | ||
| Constraint::Min(5), // Table | ||
| Constraint::Length(5), // Failure reason |
There was a problem hiding this comment.
The failure-detail panel is hard-capped to 5 rows, so longer job.job_status messages will be truncated with no way to view the full error text from the UI.
Severity: low
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@ballista-cli/src/tui/domain/jobs.rs`:
- Around line 28-29: Add a serde default attribute to the job_status field in
the Job struct to allow deserialization to succeed when the field is omitted
from scheduler responses (due to version skew). Apply the #[serde(default)]
macro attribute directly above the job_status field declaration to provide a
default empty string value when the field is missing, preventing deserialization
failures that currently cause the UI to fall back to an empty jobs list.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ecf42687-77f0-48c5-a68f-611f360f5bae
📒 Files selected for processing (8)
.cursor/rules.md.gemini/rules.mdAGENTS.mdCLAUDE.mdballista-cli/src/tui/app.rsballista-cli/src/tui/domain/jobs.rsballista-cli/src/tui/ui/main/jobs/mod.rsballista-cli/src/tui/ui/vertical_scrollbar.rs
| pub status: String, // Running, Completed, Failed, Canceled | ||
| pub job_status: String, // human-readable status/failure detail, e.g. "Failed: <reason>" |
There was a problem hiding this comment.
Add a serde default for job_status to prevent decode failures under version skew.
Job is Deserialize, and Line 29 makes job_status mandatory. If a scheduler response omits that field, jobs deserialization fails and the UI falls back to an empty jobs list (see ballista-cli/src/tui/ui/main/jobs/mod.rs, Lines 58-61).
Proposed fix
#[derive(Deserialize, Clone, Debug)]
pub struct Job {
pub job_id: String,
pub job_name: String,
pub status: String, // Running, Completed, Failed, Canceled
+ #[serde(default)]
pub job_status: String, // human-readable status/failure detail, e.g. "Failed: <reason>"
pub start_time: i64,📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| pub status: String, // Running, Completed, Failed, Canceled | |
| pub job_status: String, // human-readable status/failure detail, e.g. "Failed: <reason>" | |
| pub status: String, // Running, Completed, Failed, Canceled | |
| #[serde(default)] | |
| pub job_status: String, // human-readable status/failure detail, e.g. "Failed: <reason>" |
🤖 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 `@ballista-cli/src/tui/domain/jobs.rs` around lines 28 - 29, Add a serde
default attribute to the job_status field in the Job struct to allow
deserialization to succeed when the field is omitted from scheduler responses
(due to version skew). Apply the #[serde(default)] macro attribute directly
above the job_status field declaration to provide a default empty string value
when the field is missing, preventing deserialization failures that currently
cause the UI to fall back to an empty jobs list.
2 participants
1871: To review by AI