From 56e0d909e83f215599bf7c20463de1b1659993e6 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 13 Feb 2026 03:23:54 +0000 Subject: [PATCH 1/2] Initial plan From 76b8192b1e0c570d64ed48bbacade50006ed1b88 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 13 Feb 2026 03:42:27 +0000 Subject: [PATCH 2/2] Address PR #190 review feedback: improve duration formatting, countdown timer UX, and add tests Co-authored-by: abhimehro <84992105+abhimehro@users.noreply.github.com> --- main.py | 40 +++++++++++--------- tests/test_format_duration.py | 70 +++++++++++++++++++++++++++++++++++ 2 files changed, 93 insertions(+), 17 deletions(-) create mode 100644 tests/test_format_duration.py diff --git a/main.py b/main.py index 0af0471..be361dd 100644 --- a/main.py +++ b/main.py @@ -229,12 +229,23 @@ def sanitize_for_log(text: Any) -> str: def format_duration(seconds: float) -> str: - """Formats duration in a human-readable way (e.g., 2m 05s).""" - if seconds < 60: - return f"{seconds:.1f}s" + """Formats duration in a human-readable way (e.g., 2m 05s). - minutes, rem_seconds = divmod(int(seconds), 60) - return f'{minutes}m {rem_seconds:02d}s' + We first round to the nearest whole second to avoid surprising + outputs around boundaries (e.g., 59.95s -> 1m 00s instead of + 60.0s) and then derive minutes/seconds from that integer. + """ + # Round once to whole seconds so behavior is consistent around the 60s + # boundary and we don't under-report longer durations due to truncation. + total_seconds = int(round(seconds)) + + if total_seconds < 60: + # For sub-minute durations, show whole seconds for clarity and to + # match the rounded value used for longer durations. + return f"{total_seconds}s" + + minutes, rem_seconds = divmod(total_seconds, 60) + return f"{minutes}m {rem_seconds:02d}s" def print_plan_details(plan_entry: Dict[str, Any]) -> None: @@ -276,12 +287,14 @@ def countdown_timer(seconds: int, message: str = "Waiting") -> None: progress = (seconds - remaining + 1) / seconds filled = int(width * progress) bar = "█" * filled + "░" * (width - filled) + # Clear line (\033[K) to prevent trailing characters when digits shrink sys.stderr.write( - f"\r{Colors.CYAN}⏳ {message}: [{bar}] {remaining}s...{Colors.ENDC}" + f"\r\033[K{Colors.CYAN}⏳ {message}: [{bar}] {remaining}s...{Colors.ENDC}" ) sys.stderr.flush() time.sleep(1) + # Clear the line one final time before showing completion message sys.stderr.write(f"\r\033[K{Colors.GREEN}✅ {message}: Done!{Colors.ENDC}\n") sys.stderr.flush() @@ -618,7 +631,8 @@ def _retry_request(request_func, max_retries=MAX_RETRIES, delay=RETRY_DELAY): f"Request failed (attempt {attempt + 1}/{max_retries}): " f"{sanitize_for_log(e)}. Retrying in {wait_time}s..." ) - time.sleep(wait_time) + # Use countdown timer for user feedback during retries (when interactive) + countdown_timer(int(wait_time), "Retrying") def _gh_get(url: str) -> Dict: @@ -1053,16 +1067,8 @@ def create_folder( log.info( f"Folder '{sanitize_for_log(name)}' not found yet. Retrying in {wait_time}s..." ) - # Local countdown with line clearing to avoid trailing characters when digits shrink (e.g., 10→9) - for remaining in range(int(wait_time), 0, -1): - # Clear the current line and print updated countdown on the same line - sys.stdout.write("\r\033[K") - sys.stdout.write(f"Waiting for folder: {remaining}s") - sys.stdout.flush() - time.sleep(1) - # Clear the line once more so subsequent logs start on a fresh line - sys.stdout.write("\r\033[K") - sys.stdout.flush() + # Use countdown timer for consistent UX with other retry operations + countdown_timer(int(wait_time), "Waiting for folder") log.error( f"Folder {sanitize_for_log(name)} was not found after creation and retries." diff --git a/tests/test_format_duration.py b/tests/test_format_duration.py new file mode 100644 index 0000000..57e5824 --- /dev/null +++ b/tests/test_format_duration.py @@ -0,0 +1,70 @@ +"""Tests for the format_duration function.""" + +import main + + +def test_format_duration_sub_minute(): + """Test format_duration with durations less than 60 seconds.""" + # Exact values + assert main.format_duration(0) == "0s" + assert main.format_duration(5) == "5s" + assert main.format_duration(42) == "42s" + assert main.format_duration(59) == "59s" + + # Rounding behavior for sub-minute values + assert main.format_duration(5.4) == "5s" # Rounds down + assert main.format_duration(5.5) == "6s" # Rounds up (banker's rounding to even) + assert main.format_duration(59.4) == "59s" # Rounds down + assert main.format_duration(59.5) == "1m 00s" # Rounds to 60 -> shows as 1m 00s + assert main.format_duration(59.95) == "1m 00s" # Edge case: rounds to 60 -> 1m 00s + + +def test_format_duration_exact_minutes(): + """Test format_duration with exact minute values.""" + assert main.format_duration(60) == "1m 00s" + assert main.format_duration(120) == "2m 00s" + assert main.format_duration(300) == "5m 00s" + assert main.format_duration(3600) == "60m 00s" + + +def test_format_duration_mixed(): + """Test format_duration with minutes and seconds.""" + assert main.format_duration(65) == "1m 05s" + assert main.format_duration(125) == "2m 05s" + assert main.format_duration(185) == "3m 05s" + assert main.format_duration(305.5) == "5m 06s" # Rounds to 306 seconds = 5m 06s + + +def test_format_duration_rounding_boundaries(): + """Test format_duration rounding behavior at boundaries. + + These boundary tests protect against the issue mentioned in the PR review + where 59.95s would show as "60.0s" instead of "1m 00s" due to truncation. + By rounding first, we get consistent behavior: values that round to 60+ + seconds are displayed in minutes format for clarity. + """ + # Just under a minute: should round down and stay in seconds + assert main.format_duration(59.4) == "59s" + + # Halfway to next second at boundary: rounds to 60 -> shown as minutes + assert main.format_duration(59.5) == "1m 00s" + + # Very close to a minute: rounds to 60 -> shown as 1m 00s (clearer than "60s") + assert main.format_duration(59.95) == "1m 00s" + + # Just over a minute: should be in minutes format + assert main.format_duration(60.1) == "1m 00s" + assert main.format_duration(60.5) == "1m 00s" # Banker's rounding: rounds to 60 (even) + assert main.format_duration(61.5) == "1m 02s" # Banker's rounding: rounds to 62 (even) + + # Edge cases around 2 minutes + assert main.format_duration(119.4) == "1m 59s" # Rounds down + assert main.format_duration(119.5) == "2m 00s" # Rounds up + assert main.format_duration(125.9) == "2m 06s" # Example from PR review + + +def test_format_duration_large_values(): + """Test format_duration with large durations.""" + assert main.format_duration(3661) == "61m 01s" + assert main.format_duration(7200) == "120m 00s" + assert main.format_duration(7325.7) == "122m 06s" # 7326 seconds = 122m 06s