Skip to content

feat: Implement CLI Liveness Tracking and Sync with Dev#12

Merged
yuxiang-wu merged 8 commits intodevfrom
feature/cli-liveness-tracking
May 16, 2025
Merged

feat: Implement CLI Liveness Tracking and Sync with Dev#12
yuxiang-wu merged 8 commits intodevfrom
feature/cli-liveness-tracking

Conversation

@yuxiang-wu
Copy link
Contributor

@yuxiang-wu yuxiang-wu commented May 13, 2025

This PR introduces the CLI liveness tracking feature and incorporates recent updates from the dev branch.

Key changes include:

  • CLI Liveness Tracking: Core functionality for tracking the liveness of CLI processes has been implemented (related to feature/cli-liveness-tracking).
  • Bugfix: Corrected an indentation error in a try-except block (commit 6ad7b80).
  • Dev Sync: Merged the latest changes from the dev branch, which includes an update to README.md (commits 39182a9 via 7dd6c53).

Introduces mechanisms for the CLI to signal its liveness to the backend and report its termination reason, ensuring more robust state management.

- Adds a background heartbeat thread (`HeartbeatSender`) that periodically sends PUT requests to `/api/sessions/{session_id}/heartbeat` every 30 seconds during an active run via the new `send_heartbeat` API function. Heartbeat failures are logged but non-fatal.
- Implements signal handlers for SIGINT and SIGTERM to gracefully stop the heartbeat thread and report termination due to user interruption via a new `report_termination` API function and endpoint (`/terminate`).
- Wraps the main optimization loop in a `try...except...finally` block:
    - Catches runtime errors during the optimization loop.
    - Ensures the heartbeat thread is stopped on exit (normal completion, error, or signal).
    - Calls `report_termination` in the `finally` block to inform the backend of the final status (completed or error), unless already reported by the signal handler.
- Improves error handling for existing API calls (`start_optimization_session`, `evaluate_feedback_then_suggest_next_solution`) to be more specific and prevent unexpected exits.
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR implements CLI liveness tracking for the Weco CLI by adding a heartbeat thread and signal handling to gracefully terminate sessions. Key changes include:

  • Introducing a HeartbeatSender thread to periodically send heartbeat signals.
  • Adding signal handlers to manage termination and report session status.
  • Updating API functions to better handle errors and termination reporting.

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
weco/cli.py Implements heartbeat functionality, signal handling, and improved error management in CLI main loop.
weco/api.py Enhances API error handling, adds heartbeat and termination reporting endpoints, and wraps session functions with exception handling.
Comments suppressed due to low confidence (3)

weco/api.py:97

  • Avoid using a mutable default argument for auth_headers. Consider defaulting to None and initializing it inside the function to prevent unintended side effects.
def get_optimization_session_status(session_id: str, include_history: bool = False, auth_headers: dict = {}, timeout: int = 800) -> Dict[str, Any]:

weco/api.py:118

  • Avoid using a mutable default argument for auth_headers. Replace it with a None default and initialize a new dict inside the function.
def send_heartbeat(session_id: str, auth_headers: dict = {}, timeout: int = 10 # Shorter timeout for non-critical heartbeat

weco/api.py:150

  • Avoid using a mutable default argument for auth_headers. Consider defaulting auth_headers to None and initializing it within the function.
def report_termination(session_id: str, status_update: str, reason: str, details: Optional[str] = None, auth_headers: dict = {}, timeout: int = 30 # Reasonably longer timeout for important termination message

@yuxiang-wu yuxiang-wu changed the title Feature/cli liveness tracking feat: Implement CLI Liveness Tracking and Sync with Dev May 13, 2025
using the rich Console's print method instead of print with sys.stderr to maintain consistent output styling across the CLI.

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@DhruvSrikanth DhruvSrikanth added bug Something isn't working enhancement New feature or request labels May 13, 2025
Copy link
Member

@DhruvSrikanth DhruvSrikanth left a comment

Choose a reason for hiding this comment

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

Looks good to me! It's ready to be merged once the comments are resolved

weco/cli.py Outdated
transition_delay=0.1,
)

# Send initial heartbeat immediately after starting
Copy link
Member

Choose a reason for hiding this comment

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

Add a comment saying:
# NOTE: "Can remove later but test first"

…ination

The HeartbeatSender thread was intermittently dying silently, leading to the backend not receiving heartbeats and prematurely marking the CLI session as errored.

This commit addresses the issue by:
1.  Replacing Rich `console.print` calls within the `HeartbeatSender.run()` method with standard Python `print(..., file=sys.stderr)`. This mitigates potential intermittent exceptions when using Rich `Console` methods from a background thread, which was identified as the likely primary cause of the silent thread crashes.
2.  Wrapping the main loop within `HeartbeatSender.run()` with a `try...except Exception...finally` block. This ensures that any other unexpected exceptions are caught, logged with a traceback to `stderr`, and allow the thread to terminate gracefully rather than silently.
3.  Ensuring a log message is printed to `stderr` when the thread starts and when it is stopping (in the `finally` block).

These changes make the heartbeat mechanism more resilient and improve debuggability by ensuring that thread failures are logged.
Dynamically load package version in weco/__init__.py from
pyproject.toml using importlib.metadata. This removes
the need to maintain the version number in multiple locations,
making version bumps simpler and less error-prone.
…cli.py to streamline error handling by removing redundant try-except blocks, enhancing code clarity and maintainability.
@yuxiang-wu yuxiang-wu merged commit c24178a into dev May 16, 2025
1 check passed
@yuxiang-wu yuxiang-wu deleted the feature/cli-liveness-tracking branch May 16, 2025 21:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants