🎨 Palette: Improved CLI feedback with countdown timers and duration formatting#187
🎨 Palette: Improved CLI feedback with countdown timers and duration formatting#187
Conversation
- Replaces silent `time.sleep()` with a visual countdown timer during API retries and folder polling. - Adds `format_duration` helper to display time in `Xm Ys` format in the summary table. - Improves perceived responsiveness and readability of long-running sync operations. - Includes unit tests for new functionality in `tests/test_ux.py`. Co-authored-by: abhimehro <84992105+abhimehro@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
|
Merging to
|
Summary of ChangesHello @abhimehro, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the command-line interface (CLI) user experience by making background operations more transparent and output more digestible. It introduces visual countdown timers for previously silent waiting periods and formats duration displays into a more intuitive, human-readable format. These changes ensure users are better informed about the tool's progress and can quickly interpret performance metrics. Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
|
👋 Development Partner is reviewing this PR. Will provide feedback shortly. |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| ]) | ||
| def test_format_duration(seconds, expected): | ||
| """Verify format_duration output for various inputs.""" | ||
| assert main.format_duration(seconds) == expected |
Check notice
Code scanning / Bandit
Use of assert detected. The enclosed code will be removed when compiling to optimised byte code. Note test
| import sys | ||
| from unittest.mock import MagicMock | ||
| import main | ||
| import pytest |
Check warning
Code scanning / Pylintpython3 (reported by Codacy)
third party import "import pytest" should be placed before "import main" Warning test
| ]) | ||
| def test_format_duration(seconds, expected): | ||
| """Verify format_duration output for various inputs.""" | ||
| assert main.format_duration(seconds) == expected |
Check notice
Code scanning / Bandit (reported by Codacy)
Use of assert detected. The enclosed code will be removed when compiling to optimised byte code. Note test
| import sys | ||
| from unittest.mock import MagicMock | ||
| import main | ||
| import pytest |
Check warning
Code scanning / Prospector (reported by Codacy)
Unable to import 'pytest' (import-error) Warning test
There was a problem hiding this comment.
Pull request overview
Improves the CLI’s “waiting” UX during retries/polling and makes the sync summary duration column easier to scan by formatting durations into minute/second strings.
Changes:
- Add
format_duration()and use it in the summary table instead of printing raw seconds. - Replace
time.sleep(...)in retry/polling paths withcountdown_timer(...)to provide visible feedback in TTY mode. - Add unit tests covering duration formatting cases.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
main.py |
Adds duration formatting helper and wires countdown timers into retry/polling + summary output. |
tests/test_ux.py |
Adds parametrized tests for the new duration formatting behavior. |
| 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" |
There was a problem hiding this comment.
format_duration() can produce the inconsistent output "60.0s" for inputs just under 60 seconds (e.g., 59.95 rounds to 60.0 with :.1f while still taking the < 60 branch). Consider truncating (not rounding) sub-minute values, or adding a guard so any value that formats to 60.0s is emitted as "1m 00s" instead.
| return f"{seconds:.1f}s" | |
| # Use truncation instead of rounding for sub-minute values so that | |
| # inputs like 59.95 do not produce the inconsistent "60.0s" output. | |
| # We keep exactly one decimal place by truncating toward zero. | |
| truncated = int(seconds * 10) / 10.0 | |
| return f"{truncated:.1f}s" |
![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 is a great pull request that significantly improves the CLI user experience by adding countdown timers and human-readable duration formatting. The code is clean, and the addition of unit tests is much appreciated. I've found one medium-severity issue in the new format_duration function related to inconsistent precision and information loss when formatting durations over 60 seconds. I've provided a suggestion to make the formatting more accurate and consistent. Overall, excellent work!
| 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" | ||
|
|
||
| minutes = int(seconds // 60) | ||
| rem_seconds = int(seconds % 60) | ||
| return f"{minutes}m {rem_seconds:02d}s" |
There was a problem hiding this comment.
The current implementation of format_duration has an inconsistency in precision. For durations less than 60 seconds, it preserves one decimal place (e.g., 59.9s), but for durations greater than or equal to 60 seconds, it truncates the remaining seconds by using int(). For example, a duration of 60.9s becomes 1m 00s, losing almost a full second of information. This can be misleading.
A more consistent and accurate approach would be to round the duration to the nearest second and then format it. This avoids information loss and also handles the transition around 60 seconds more gracefully (e.g., 59.9s would be rounded to 1m 00s).
Here is a suggested implementation that addresses this. Note that this change will require updating the corresponding test cases in tests/test_ux.py (e.g., (59.9, "59.9s") would become (59.9, "1m 00s") and (65.5, "1m 05s") would become (65.5, "1m 06s")).
| 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" | |
| minutes = int(seconds // 60) | |
| rem_seconds = int(seconds % 60) | |
| return f"{minutes}m {rem_seconds:02d}s" | |
| def format_duration(seconds: float) -> str: | |
| """Formats duration in a human-readable way (e.g., 2m 05s).""" | |
| if seconds < 59.5: | |
| return f"{seconds:.1f}s" | |
| total_seconds = int(round(seconds)) | |
| minutes = total_seconds // 60 | |
| rem_seconds = total_seconds % 60 | |
| return f"{minutes}m {rem_seconds:02d}s" |
|
Closing: superseded by #190 (auto-merge enabled). |
abhimehro
deleted the
palette-ux-duration-countdown-1403772942436215757
branch
Understood. Acknowledging that this work is superseded and stopping work on this task. |
This change improves the CLI user experience by:
2m 05sinstead of125.0s), making it easier to parse at a glance.No external dependencies were added. Existing tests pass.
PR created automatically by Jules for task 1403772942436215757 started by @abhimehro