Skip to content

Commit a47af29

Browse files
committed
Refactor UI handling and improve error reporting in SetupAgent
- Enhanced UIManager integration in SetupAgent to ensure proper display of final summaries and error states. - Added checks to prevent re-displaying summaries on early exits. - Improved error handling during task execution, ensuring critical errors are reported to the console. - Updated HTML and JavaScript for architecture tabs to enhance user interaction and maintain active state across tab switches.
1 parent 76062fe commit a47af29

5 files changed

Lines changed: 142 additions & 81 deletions

File tree

agent/agent.py

Lines changed: 105 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -243,21 +243,25 @@ def setup_project(
243243
cmd_logger, cmd_logger_id = create_command_logger("project", project_name)
244244
cmd_logger.info(f"Starting project setup: {project_name} (docker_label={docker_label})")
245245

246-
# Initialize UI Manager if in UI mode
247-
if self.config.ui_mode:
248-
self.ui_manager = UIManager(project_name=project_name, console=self.console)
249-
self.ui_manager.start()
250-
else:
251-
self.console.print(
252-
Panel.fit(
253-
f"[bold blue]Setting up project: {project_name}[/bold blue]\n"
254-
f"[dim]Repository: {project_url}[/dim]\n"
255-
f"[dim]Goal: {goal}[/dim]",
256-
border_style="blue",
257-
)
258-
)
246+
# Track whether we already displayed the final UI summary so the
247+
# cleanup `finally` below doesn't re-display or skip it incorrectly.
248+
final_summary_shown = False
259249

260250
try:
251+
# Initialize UI Manager if in UI mode
252+
if self.config.ui_mode:
253+
self.ui_manager = UIManager(project_name=project_name, console=self.console)
254+
self.ui_manager.start()
255+
else:
256+
self.console.print(
257+
Panel.fit(
258+
f"[bold blue]Setting up project: {project_name}[/bold blue]\n"
259+
f"[dim]Repository: {project_url}[/dim]\n"
260+
f"[dim]Goal: {goal}[/dim]",
261+
border_style="blue",
262+
)
263+
)
264+
261265
# Step 1: Setup Docker environment
262266
if self.config.ui_mode:
263267
self.ui_manager.handle_event(UIEvent(
@@ -389,30 +393,54 @@ def setup_project(
389393
))
390394
# Display final summary and stop UI manager
391395
self.ui_manager.display_final_summary()
396+
final_summary_shown = True
392397
else:
393398
# Provide traditional summary
394399
self._provide_setup_summary(success)
395400

396401
cmd_logger.info(f"Project setup completed: success={success}")
397-
398-
# Cleanup command logger
399-
session_logger = get_session_logger()
400-
if session_logger:
401-
session_logger.cleanup_command_logger(cmd_logger_id)
402-
403402
return success
404403

405404
except Exception as e:
406405
cmd_logger.error(f"Setup failed: {e}", exc_info=True)
407406
# Always show critical errors
408407
self.console.print(f"[bold red]❌ Setup failed: {e}[/bold red]")
408+
return False
409409

410-
# Cleanup command logger
410+
finally:
411+
# Tear down the live UI on every exit path (early returns, exceptions,
412+
# normal completion). If the happy-path branch above didn't get to
413+
# `display_final_summary`, do it now so the user sees the failure
414+
# state instead of a frozen spinner.
415+
if self.config.ui_mode and self.ui_manager and not final_summary_shown:
416+
# Any phase still in 'running' was aborted, not completed.
417+
for phase, data in self.ui_manager.phases_data.items():
418+
if data.get("status") == "running":
419+
try:
420+
self.ui_manager.handle_event(UIEvent(
421+
event_type=EventType.PHASE_ERROR,
422+
message="Phase aborted",
423+
phase=phase,
424+
level="error",
425+
))
426+
except Exception:
427+
pass
428+
try:
429+
self.ui_manager.display_final_summary()
430+
except Exception:
431+
# display_final_summary itself failed; at minimum stop Live.
432+
try:
433+
self.ui_manager.stop()
434+
except Exception:
435+
pass
436+
437+
# Always release the command-specific loguru handler.
411438
session_logger = get_session_logger()
412439
if session_logger:
413-
session_logger.cleanup_command_logger(cmd_logger_id)
414-
415-
return False
440+
try:
441+
session_logger.cleanup_command_logger(cmd_logger_id)
442+
except Exception:
443+
pass
416444

417445
def continue_project(self, project_name: str, additional_request: Optional[str] = None) -> bool:
418446
"""Continue working on an existing project."""
@@ -486,20 +514,24 @@ def run_task(self, project_name: str, task_description: str) -> bool:
486514
cmd_logger, cmd_logger_id = create_command_logger("run", project_name)
487515
cmd_logger.info(f"Starting task execution: {task_description}")
488516

489-
# Initialize UI Manager if in UI mode
490-
if self.config.ui_mode:
491-
self.ui_manager = UIManager(project_name=project_name, console=self.console)
492-
self.ui_manager.start()
493-
else:
494-
self.console.print(
495-
Panel.fit(
496-
f"[bold cyan]Running task on: {project_name}[/bold cyan]\n"
497-
f"[dim]Task: {task_description}[/dim]",
498-
border_style="cyan",
499-
)
500-
)
517+
# Track whether we already displayed the final UI summary so the
518+
# cleanup `finally` below doesn't re-display or skip it incorrectly.
519+
final_summary_shown = False
501520

502521
try:
522+
# Initialize UI Manager if in UI mode
523+
if self.config.ui_mode:
524+
self.ui_manager = UIManager(project_name=project_name, console=self.console)
525+
self.ui_manager.start()
526+
else:
527+
self.console.print(
528+
Panel.fit(
529+
f"[bold cyan]Running task on: {project_name}[/bold cyan]\n"
530+
f"[dim]Task: {task_description}[/dim]",
531+
border_style="cyan",
532+
)
533+
)
534+
503535
# Step 1: Ensure Docker container is running
504536
if self.config.ui_mode:
505537
self.ui_manager.handle_event(UIEvent(
@@ -626,33 +658,58 @@ def run_task(self, project_name: str, task_description: str) -> bool:
626658
# Step 7: Provide execution summary
627659
if self.config.ui_mode:
628660
self.ui_manager.display_final_summary()
661+
final_summary_shown = True
629662
else:
630663
self._provide_task_summary(success, task_description)
631664

632665
cmd_logger.info(f"Task execution completed: success={success}")
633-
634-
# Cleanup command logger
635-
session_logger = get_session_logger()
636-
if session_logger:
637-
session_logger.cleanup_command_logger(cmd_logger_id)
638-
639666
return success
640667

641668
except Exception as e:
642669
cmd_logger.error(f"Task execution failed: {e}", exc_info=True)
643670
# Always show critical errors
644671
self.console.print(f"[bold red]❌ Task execution failed: {e}[/bold red]")
645672

646-
# Update last comment with error
647-
error_comment = f"Task failed: {task_description} - Error: {str(e)[:100]}"
673+
# Best-effort: record the failure on the container's last-comment marker.
674+
try:
675+
error_comment = f"Task failed: {task_description} - Error: {str(e)[:100]}"
676+
self.orchestrator.update_last_comment(error_comment)
677+
except Exception:
678+
pass
679+
680+
return False
681+
682+
finally:
683+
# Tear down the live UI on every exit path. Without this, early
684+
# `return False` paths above would leave Rich's Live thread
685+
# refreshing the terminal indefinitely and swallowing any error
686+
# message printed afterwards.
687+
if self.config.ui_mode and self.ui_manager and not final_summary_shown:
688+
for phase, data in self.ui_manager.phases_data.items():
689+
if data.get("status") == "running":
690+
try:
691+
self.ui_manager.handle_event(UIEvent(
692+
event_type=EventType.PHASE_ERROR,
693+
message="Phase aborted",
694+
phase=phase,
695+
level="error",
696+
))
697+
except Exception:
698+
pass
699+
try:
700+
self.ui_manager.display_final_summary()
701+
except Exception:
702+
try:
703+
self.ui_manager.stop()
704+
except Exception:
705+
pass
648706

649-
# Cleanup command logger
650707
session_logger = get_session_logger()
651708
if session_logger:
652-
session_logger.cleanup_command_logger(cmd_logger_id)
653-
self.orchestrator.update_last_comment(error_comment)
654-
655-
return False
709+
try:
710+
session_logger.cleanup_command_logger(cmd_logger_id)
711+
except Exception:
712+
pass
656713

657714
def _setup_docker_environment(self, project_name: str) -> bool:
658715
"""Setup the Docker environment for the project."""

main.py

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -314,9 +314,7 @@ def cli(ctx, log_level, log_file, verbose, ui):
314314
logger.info(f"Logs directory: {session_logger.session_log_dir}")
315315

316316
# Display welcome message for main commands (skip in UI mode, will be shown by UIManager)
317-
# Note: UI mode is only enabled at subcommand level, so we show it here in non-verbose mode
318-
# It will be suppressed when actual UI starts
319-
if ctx.invoked_subcommand not in ["list"] and not config.verbose:
317+
if ctx.invoked_subcommand not in ["list"] and not config.verbose and not config.ui_mode:
320318
console.print(
321319
Panel.fit(
322320
"[bold blue]SAG[/bold blue] - [dim]Setup Agent[/dim]\n"

ui/ui_manager.py

Lines changed: 18 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -201,7 +201,7 @@ def _extract_thought_summary(self, thought: str) -> str:
201201
thought: Full thought content
202202
203203
Returns:
204-
Concise summary (30-80 chars) with ellipsis
204+
Concise summary (up to 80 chars); ellipsis only when truncated
205205
"""
206206
# Remove common prefixes
207207
thought = thought.strip()
@@ -214,12 +214,13 @@ def _extract_thought_summary(self, thought: str) -> str:
214214
# Limit length
215215
if len(summary) > 80:
216216
summary = summary[:77] + "..."
217-
elif len(summary) < 20:
217+
elif len(summary) < 20 and len(sentences) > 1:
218218
# If too short, include second sentence if available
219-
if len(sentences) > 1:
220-
summary = f"{summary}. {sentences[1][:40]}..."
221-
else:
222-
summary = summary + "..."
219+
second = sentences[1].strip()
220+
if len(second) > 40:
221+
summary = f"{summary}. {second[:37]}..."
222+
else:
223+
summary = f"{summary}. {second}"
223224

224225
return summary
225226

@@ -234,7 +235,7 @@ def _extract_observation_summary(self, observation: str) -> str:
234235
observation: Full observation content
235236
236237
Returns:
237-
Concise summary (50-100 chars) with ellipsis
238+
Concise summary (up to 100 chars); ellipsis only when truncated
238239
"""
239240
# Clean up observation
240241
observation = observation.strip()
@@ -246,28 +247,22 @@ def _extract_observation_summary(self, observation: str) -> str:
246247
for line in lines:
247248
if "success" in line.lower():
248249
summary = line.strip()
249-
if len(summary) > 100:
250-
return summary[:97] + "..."
251-
return summary + "..."
250+
return summary[:97] + "..." if len(summary) > 100 else summary
252251

253252
# Look for error indicators
254253
if "error" in observation.lower() or "failed" in observation.lower():
255254
lines = observation.split("\n")
256255
for line in lines:
257256
if "error" in line.lower() or "failed" in line.lower():
258257
summary = line.strip()
259-
if len(summary) > 100:
260-
return summary[:97] + "..."
261-
return summary + "..."
258+
return summary[:97] + "..." if len(summary) > 100 else summary
262259

263260
# Default: first meaningful line
264261
lines = observation.split("\n")
265262
for line in lines:
266263
line = line.strip()
267264
if len(line) > 10: # Skip very short lines
268-
if len(line) > 100:
269-
return line[:97] + "..."
270-
return line + "..."
265+
return line[:97] + "..." if len(line) > 100 else line
271266

272267
# Fallback
273268
return observation[:97] + "..." if len(observation) > 100 else observation
@@ -483,16 +478,16 @@ def _handle_agent_event(self, event: UIEvent):
483478
self.agent_tool_params = tool_params
484479

485480
# Detect phase transition based on tool usage
481+
# NOTE: tool-name detection is a pure heuristic — it carries no signal
482+
# about whether the previous phase actually succeeded. Updating
483+
# `current_phase` is safe (it just drives the live indicator), but we
484+
# must NOT mark the previous phase as "success" here; only explicit
485+
# PHASE_COMPLETE / PHASE_ERROR events may transition phase status.
486486
detected_phase = self._detect_phase_from_action(tool_name, tool_params)
487487
if detected_phase and detected_phase != self.current_phase:
488-
# Transition to new phase
489-
# Complete previous phase if it was running
490-
if self.current_phase and self.phases_data[self.current_phase]["status"] == "running":
491-
self.phases_data[self.current_phase]["status"] = "success"
492-
493-
# Start new phase
494488
self.current_phase = detected_phase
495-
self.phases_data[detected_phase]["status"] = "running"
489+
if self.phases_data[detected_phase]["status"] == "pending":
490+
self.phases_data[detected_phase]["status"] = "running"
496491

497492
# Format parameters for display
498493
params_str = self._format_tool_params(tool_name, tool_params)

website/index.html

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -153,10 +153,10 @@ <h2>Setup<br>Agent</h2>
153153
<span class="arch-title">How SAG Works.</span>
154154

155155
<div class="arch-tabs">
156-
<button class="arch-tab active" onclick="showTab('overview')">Overview</button>
157-
<button class="arch-tab" onclick="showTab('dual')">Dual-Model</button>
158-
<button class="arch-tab" onclick="showTab('docker')">Docker</button>
159-
<button class="arch-tab" onclick="showTab('verify')">Validation</button>
156+
<button class="arch-tab active" data-tab="overview" onclick="showTab(event, 'overview')">Overview</button>
157+
<button class="arch-tab" data-tab="dual" onclick="showTab(event, 'dual')">Dual-Model</button>
158+
<button class="arch-tab" data-tab="docker" onclick="showTab(event, 'docker')">Docker</button>
159+
<button class="arch-tab" data-tab="verify" onclick="showTab(event, 'verify')">Validation</button>
160160
</div>
161161

162162
<!-- Overview -->

website/js/main.js

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,22 +3,33 @@
33
*/
44

55
// Architecture tab switching
6-
function showTab(name) {
6+
function showTab(event, name) {
77
document.querySelectorAll('.arch-panel').forEach(function(p) {
88
p.classList.remove('active');
99
});
1010
document.querySelectorAll('.arch-tab').forEach(function(t) {
1111
t.classList.remove('active');
1212
});
1313
document.getElementById('p-' + name).classList.add('active');
14-
event.target.classList.add('active');
14+
if (event && event.currentTarget) {
15+
event.currentTarget.classList.add('active');
16+
} else {
17+
var btn = document.querySelector('.arch-tab[data-tab="' + name + '"]');
18+
if (btn) btn.classList.add('active');
19+
}
1520
}
1621

1722
// Copy-to-clipboard for command blocks and bibtex
1823
document.addEventListener('DOMContentLoaded', function() {
1924
document.querySelectorAll('.cp').forEach(function(btn) {
2025
btn.addEventListener('click', function() {
21-
var container = btn.closest('.cmd') || btn.closest('.bibtex');
26+
// In the install layout the button is a sibling of .cmd under
27+
// .install-step, so closest('.cmd') would not find it — fall back to
28+
// looking for a .cmd inside the surrounding .install-step wrapper.
29+
var step = btn.closest('.install-step');
30+
var container = btn.closest('.cmd')
31+
|| btn.closest('.bibtex')
32+
|| (step && step.querySelector('.cmd'));
2233
if (!container) return;
2334

2435
var text;

0 commit comments

Comments
 (0)