Skip to content

UI improvements #56

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
daniel-thom merged 2 commits into from
Dec 15, 2025
Merged

UI improvements #56

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ec7cdf8
UI improvements
daniel-thom Dec 15, 2025
ceacaf4
tmp
daniel-thom Dec 15, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 21 additions & 0 deletions torc-dash/src/main.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -346,6 +346,7 @@ async fn main() -> Result<()> {
post(cli_check_reinitialize_handler),
)
.route("/api/cli/reset-status", post(cli_reset_status_handler))
.route("/api/cli/execution-plan", post(cli_execution_plan_handler))
.route("/api/cli/run-stream", get(cli_run_stream_handler))
.route("/api/cli/read-file", post(cli_read_file_handler))
.route("/api/cli/plot-resources", post(cli_plot_resources_handler))
Expand Down Expand Up @@ -735,6 +736,26 @@ async fn cli_reset_status_handler(
Json(result)
}

/// Get execution plan for a workflow (includes scheduler allocations per stage)
async fn cli_execution_plan_handler(
State(state): State<Arc<AppState>>,
Json(req): Json<WorkflowIdRequest>,
) -> impl IntoResponse {
let result = run_torc_command(
&state.torc_bin,
&[
"-f",
"json",
"workflows",
"execution-plan",
&req.workflow_id,
],
&state.api_url,
)
.await;
Json(result)
}

/// Streaming workflow run handler using Server-Sent Events
async fn cli_run_stream_handler(
State(state): State<Arc<AppState>>,
Expand Down
9 changes: 9 additions & 0 deletions torc-dash/static/js/api.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -333,6 +333,15 @@ class TorcAPI {
return this.cliRequest('/api/cli/reset-status', { workflow_id: workflowId });
}

/**
* Get execution plan for a workflow (includes scheduler allocations per stage)
* @param {string} workflowId - Workflow ID
* @returns {object} CLI response with JSON execution plan in stdout
*/
async cliExecutionPlan(workflowId) {
return this.cliRequest('/api/cli/execution-plan', { workflow_id: workflowId });
}

/**
* Make a CLI command request
*/
Expand Down
109 changes: 44 additions & 65 deletions torc-dash/static/js/app-modals.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -327,93 +327,72 @@ Object.assign(TorcDashboard.prototype, {
content.innerHTML = '<div class="placeholder-message">Loading execution plan...</div>';

try {
// Get jobs and dependencies
const [jobs, dependencies] = await Promise.all([
api.listJobs(workflowId),
api.getJobsDependencies(workflowId),
]);

// Build dependency graph and compute stages
const stages = this.computeExecutionStages(jobs, dependencies);
content.innerHTML = this.renderExecutionPlan(stages, jobs);
} catch (error) {
content.innerHTML = `<div class="placeholder-message">Error loading execution plan: ${error.message}</div>`;
}
},
// Get execution plan from CLI (includes scheduler allocations)
const result = await api.cliExecutionPlan(workflowId);

computeExecutionStages(jobs, dependencies) {
// Build a map of job dependencies
const blockedBy = {};
dependencies.forEach(dep => {
if (!blockedBy[dep.job_id]) blockedBy[dep.job_id] = [];
blockedBy[dep.job_id].push(dep.depends_on_job_id);
});

// Create job map
const jobMap = {};
jobs.forEach(j => jobMap[j.id] = j);

// Compute stages using topological sort levels
const stages = [];
const completed = new Set();
const remaining = new Set(jobs.map(j => j.id));

let stageNum = 1;
while (remaining.size > 0) {
const ready = [];
remaining.forEach(jobId => {
const deps = blockedBy[jobId] || [];
if (deps.every(d => completed.has(d))) {
ready.push(jobId);
}
});

if (ready.length === 0 && remaining.size > 0) {
// Circular dependency or error - break to avoid infinite loop
break;
if (!result.success) {
content.innerHTML = `<div class="placeholder-message">Error: ${this.escapeHtml(result.stderr || result.stdout || 'Unknown error')}</div>`;
return;
}

stages.push({
stageNumber: stageNum++,
jobs: ready.map(id => jobMap[id]),
});

ready.forEach(id => {
completed.add(id);
remaining.delete(id);
});
// Parse the JSON output
const plan = JSON.parse(result.stdout);
Copy link

Copilot AI Dec 15, 2025

Choose a reason for hiding this comment

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

The JSON.parse() call can throw an exception if the stdout contains invalid JSON. This would be caught by the outer try-catch block, but the error message would be generic ("Error loading execution plan: ...") rather than indicating it was a JSON parsing error. Consider wrapping this in a try-catch block to provide a more specific error message, such as "Invalid JSON response from server".

Suggested change
const plan = JSON.parse(result.stdout);
let plan;
try {
plan = JSON.parse(result.stdout);
} catch (parseError) {
content.innerHTML = `<div class="placeholder-message">Invalid JSON response from server: ${this.escapeHtml(parseError.message)}</div>`;
return;
}

Copilot uses AI. Check for mistakes.
content.innerHTML = this.renderExecutionPlan(plan);
} catch (error) {
content.innerHTML = `<div class="placeholder-message">Error loading execution plan: ${error.message}</div>`;
Copy link

Copilot AI Dec 15, 2025

Choose a reason for hiding this comment

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

The error message is not escaped before being inserted into the HTML. While error.message from JavaScript exceptions is typically safe, for consistency with the rest of the codebase and to prevent potential XSS if the error message contains user-controlled data, this should use this.escapeHtml(error.message) like the other error handlers in this file (see lines 320, 334).

Suggested change
content.innerHTML = `<div class="placeholder-message">Error loading execution plan: ${error.message}</div>`;
content.innerHTML = `<div class="placeholder-message">Error loading execution plan: ${this.escapeHtml(error.message)}</div>`;

Copilot uses AI. Check for mistakes.
}

return stages;
},

renderExecutionPlan(stages, jobs) {
if (stages.length === 0) {
renderExecutionPlan(plan) {
if (!plan.stages || plan.stages.length === 0) {
return '<div class="placeholder-message">No execution stages computed</div>';
}

return `
<div class="plan-summary" style="margin-bottom: 16px;">
<strong>Total Stages:</strong> ${stages.length} |
<strong>Total Jobs:</strong> ${jobs.length}
<strong>Workflow:</strong> ${this.escapeHtml(plan.workflow_name || 'Unnamed')} |
<strong>Total Stages:</strong> ${plan.total_stages} |
<strong>Total Jobs:</strong> ${plan.total_jobs}
</div>
${stages.map(stage => `
${plan.stages.map(stage => `
<div class="plan-stage">
<div class="plan-stage-header">
<div class="plan-stage-number">${stage.stageNumber}</div>
<div class="plan-stage-trigger">Stage ${stage.stageNumber}</div>
<div class="plan-stage-number">${stage.stage_number}</div>
<div class="plan-stage-trigger">${this.escapeHtml(stage.trigger)}</div>
</div>
<div class="plan-stage-content">
<h5>Jobs Ready (${stage.jobs.length})</h5>
<h5>Jobs Becoming Ready (${stage.jobs_becoming_ready.length})</h5>
<ul>
${stage.jobs.slice(0, 10).map(job => `
<li>${this.escapeHtml(job.name || job.id)}</li>
${stage.jobs_becoming_ready.slice(0, 10).map(jobName => `
<li>${this.escapeHtml(jobName)}</li>
`).join('')}
${stage.jobs.length > 10 ? `<li>... and ${stage.jobs.length - 10} more</li>` : ''}
${stage.jobs_becoming_ready.length > 10 ? `<li>... and ${stage.jobs_becoming_ready.length - 10} more</li>` : ''}
</ul>
${this.renderSchedulerAllocations(stage.scheduler_allocations)}
</div>
</div>
`).join('')}
`;
},

renderSchedulerAllocations(allocations) {
if (!allocations || allocations.length === 0) {
return '';
}

return `
<div class="scheduler-allocations" style="margin-top: 12px;">
<h5>Scheduler Allocations</h5>
${allocations.map(alloc => `
<div class="scheduler-allocation" style="margin-left: 16px; margin-bottom: 8px; padding: 8px; background: var(--surface-color); border-radius: 4px;">
<div><strong>Scheduler:</strong> ${this.escapeHtml(alloc.scheduler)} (${this.escapeHtml(alloc.scheduler_type)})</div>
<div><strong>Allocations:</strong> ${alloc.num_allocations}</div>
${alloc.job_names && alloc.job_names.length > 0 ? `
<div><strong>Jobs:</strong> ${alloc.job_names.slice(0, 5).map(n => this.escapeHtml(n)).join(', ')}${alloc.job_names.length > 5 ? ` ... and ${alloc.job_names.length - 5} more` : ''}</div>
` : ''}
</div>
`).join('')}
</div>
`;
},
});
17 changes: 12 additions & 5 deletions torc-dash/static/js/app-wizard.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,8 @@ Object.assign(TorcDashboard.prototype, {

this.resourcePresets = {
'small': { name: 'Small', num_cpus: 1, memory: '1g', num_gpus: 0 },
'medium': { name: 'Medium', num_cpus: 8, memory: '50g', num_gpus: 0 },
'medium': { name: 'Medium', num_cpus: 4, memory: '10g', num_gpus: 0 },
'large': { name: 'Large', num_cpus: 8, memory: '50g', num_gpus: 0 },
'gpu': { name: 'GPU', num_cpus: 1, memory: '10g', num_gpus: 1 },
'custom': { name: 'Custom', num_cpus: 1, memory: '1g', num_gpus: 0 }
};
Expand Down Expand Up @@ -373,16 +374,22 @@ Object.assign(TorcDashboard.prototype, {

wizardAddSchedulerFromJob(jobId) {
const schedulerId = ++this.wizardSchedulerIdCounter;
const schedulerName = `scheduler-${schedulerId}`;
const defaultName = `scheduler-${schedulerId}`;
const schedulerName = prompt('Enter a name for the new scheduler:', defaultName);
Copy link

Copilot AI Dec 15, 2025

Choose a reason for hiding this comment

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

Using the native prompt() dialog can be problematic for user experience and accessibility. The dialog is modal and blocks all JavaScript execution, the styling is inconsistent across browsers and cannot be customized, and it doesn't support screen readers well. Consider using a custom modal dialog for a better user experience, consistent with other UI patterns in the application.

Copilot uses AI. Check for mistakes.
if (!schedulerName) {
// User cancelled
Copy link

Copilot AI Dec 15, 2025

Choose a reason for hiding this comment

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

The prompt cancellation check doesn't handle the case where the user enters only whitespace. When the user enters only spaces and clicks OK, schedulerName will be a non-null string (e.g., " "), pass the null check, but then schedulerName.trim() will be an empty string. This would create a scheduler with an empty name, which would likely fail validation later (as seen in line 147 where schedulers must have a name). Consider checking if the trimmed value is empty as well.

Suggested change
if (!schedulerName) {
// User cancelled
if (!schedulerName || !schedulerName.trim()) {
// User cancelled or entered only whitespace

Copilot uses AI. Check for mistakes.
this.wizardSchedulerIdCounter--;
return;
}
const scheduler = {
id: schedulerId, name: schedulerName, account: '', nodes: 1, walltime: '01:00:00',
id: schedulerId, name: schedulerName.trim(), account: '', nodes: 1, walltime: '01:00:00',
partition: '', qos: '', gres: '', mem: '', tmp: '', extra: ''
};
this.wizardSchedulers.push(scheduler);
const job = this.wizardJobs.find(j => j.id === jobId);
if (job) job.scheduler = schedulerName;
if (job) job.scheduler = schedulerName.trim();
this.wizardRenderJobs();
this.showToast(`Scheduler "${schedulerName}" created. Configure it in step 3.`, 'info');
this.showToast(`Scheduler "${schedulerName.trim()}" created. Configure it in step 3.`, 'info');
},

wizardToggleScheduler(schedulerId) {
Expand Down