feat: add simple one-line progress display mode #112
Conversation
Add a new simple progress display mode that shows task execution status in a compact single-line format. Features include TASKI_PROGRESS_MODE environment variable and Taski.progress_mode API. Closes #97 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
WalkthroughThis PR introduces a configurable progress display mode system for Taski. It adds a new "simple" one-line progress display as an alternative to the existing tree-based display, selectable via environment variable Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
README.md (1)
277-280: Add language specifier to fenced code block.The code block showing terminal output should specify a language identifier for proper rendering and to satisfy markdownlint.
🔎 Proposed fix
-``` +```text ⠹ [3/5] DeployTask | Uploading files... ✓ [5/5] All tasks completed (1234ms)</details> Based on static analysis hints (markdownlint). </blockquote></details> <details> <summary>docs/GUIDE.md (1)</summary><blockquote> `290-342`: **Add language specifiers to fenced code blocks.** Multiple code blocks showing terminal output should specify a language identifier (e.g., `text`) for proper rendering and to satisfy markdownlint. Affected lines: 298, 310, 318, 323. <details> <summary>🔎 Proposed fix for line 298</summary> ```diff -``` +```text WebServer (Task) ├── ⠋ Config (Task) | Reading config.yml... │ ├── ✅ Database (Task) 45.2ms │ └── ⠙ Cache (Task) | Connecting... └── ◻ Server (Task)Apply similar changes to code blocks at lines 310, 318, and 323. </details> Based on static analysis hints (markdownlint). </blockquote></details> <details> <summary>examples/simple_progress_demo.rb (1)</summary><blockquote> `1-1`: **Add execute permission to script file.** The script has a shebang (`#!/usr/bin/env ruby`) but lacks execute permission. Add it with: ```shell chmod +x examples/simple_progress_demo.rbBased on static analysis hints (RuboCop).
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
README.mddocs/GUIDE.mdexamples/simple_progress_demo.rblib/taski.rblib/taski/execution/simple_progress_display.rbtest/test_simple_progress_display.rb
🧰 Additional context used
🧬 Code graph analysis (3)
examples/simple_progress_demo.rb (2)
lib/taski.rb (2)
progress_mode(194-196)progress_mode(200-205)lib/taski/execution/task_output_router.rb (1)
puts(150-161)
lib/taski.rb (2)
lib/taski/execution/simple_progress_display.rb (1)
stop(175-191)lib/taski/execution/tree_progress_display.rb (1)
stop(428-445)
lib/taski/execution/simple_progress_display.rb (2)
lib/taski/execution/tree_progress_display.rb (1)
build_tree_node(87-112)lib/taski/execution/task_output_router.rb (2)
flush(173-175)
🪛 markdownlint-cli2 (0.18.1)
README.md
277-277: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
docs/GUIDE.md
298-298: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
310-310: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
318-318: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
323-323: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🪛 RuboCop (1.82.0)
examples/simple_progress_demo.rb
[warning] 1-1: Script file simple_progress_demo.rb doesn't have execute permission.
(Lint/ScriptPermission)
lib/taski/execution/simple_progress_display.rb
[convention] 101-135: Assignment Branch Condition size for update_task is too high. [<18, 25, 12> 33.06/23]
(Metrics/AbcSize)
[convention] 101-135: Cyclomatic complexity for update_task is too high. [13/7]
(Metrics/CyclomaticComplexity)
[convention] 227-248: Assignment Branch Condition size for render_final is too high. [<11, 28, 12> 32.39/23]
(Metrics/AbcSize)
[convention] 227-248: Cyclomatic complexity for render_final is too high. [8/7]
(Metrics/CyclomaticComplexity)
[convention] 227-248: Perceived complexity for render_final is too high. [9/8]
(Metrics/PerceivedComplexity)
[convention] 250-284: Assignment Branch Condition size for build_status_line is too high. [<18, 43, 23> 51.98/23]
(Metrics/AbcSize)
[convention] 250-284: Cyclomatic complexity for build_status_line is too high. [16/7]
(Metrics/CyclomaticComplexity)
[convention] 250-284: Perceived complexity for build_status_line is too high. [18/8]
(Metrics/PerceivedComplexity)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (10)
test/test_simple_progress_display.rb (1)
1-309: LGTM! Comprehensive test coverage.The test suite thoroughly validates the SimpleProgressDisplay implementation, including:
- Task registration and state management
- TTY and non-TTY behavior
- Rendering with spinners, task counts, and status icons
- Progress mode configuration via environment and API
- Display factory behavior based on mode selection
The use of sleep statements in TTY tests is appropriate for allowing the renderer thread to execute.
lib/taski.rb (3)
14-14: LGTM! Clean integration of simple progress display.The require statement properly integrates the new SimpleProgressDisplay class into the module structure.
192-211: Consider thread safety for progress mode state.The
progress_modegetter/setter methods modify@progress_modewithout synchronization. While this is likely acceptable for typical usage (setting mode once at application startup), concurrent access from multiple threads could theoretically cause race conditions.If the mode is only intended to be set during initialization (before any tasks run), this is fine. Otherwise, consider adding synchronization similar to the
@args_monitorpattern used elsewhere in this file.Do you want to add synchronization, or is the current approach acceptable given expected usage patterns?
214-231: LGTM! Clean factory pattern and environment integration.The
create_progress_displayfactory method provides clean separation between modes, andprogress_mode_from_envproperly defaults to:treefor backward compatibility.examples/simple_progress_demo.rb (1)
11-80: LGTM! Clear demonstration of simple progress mode.The demo effectively illustrates:
- Setting progress mode via API
- Task definitions with exports
- Automatic dependency resolution
- Final result retrieval
The task structure with parallel downloads (Layer1, Layer2, Layer3) followed by extraction and verification is a good real-world example.
lib/taski/execution/simple_progress_display.rb (5)
1-62: LGTM! Clean initialization and constants.The implementation provides:
- Appropriate spinner frames and render interval
- Clear icon and color definitions
- Well-structured TaskProgress struct with proper initialization
- Thread-safe monitor-based synchronization
64-148: LGTM! Comprehensive public API.The public interface methods properly:
- Handle output capture configuration
- Register tasks and sections with idempotency
- Update task states with proper synchronization
- Track task registration status
150-191: LGTM! Robust lifecycle management.The start/stop methods correctly:
- Handle nested calls with nest level tracking
- Check for TTY before starting renderer thread
- Hide/show cursor appropriately
- Synchronize state changes with monitor
- Join renderer thread on stop
This matches the pattern used in TreeProgressDisplay.
195-215: LGTM! Effective code reuse.Reusing
TreeProgressDisplay.build_tree_nodefor tree structure building is an appropriate design choice. This:
- Avoids code duplication
- Ensures consistent dependency resolution
- Maintains a single source of truth for tree structure
The coupling is acceptable given the code reuse benefits.
217-305: LGTM! Well-structured rendering logic.The rendering methods effectively:
- Build compact single-line status displays
- Handle multiple running tasks
- Show appropriate icons for different states
- Truncate output for readability
- Provide informative final status
The RuboCop complexity warnings for
update_task,render_final, andbuild_status_lineare false positives—the complexity is inherent to state management and conditional formatting. The code is well-organized and readable.
Summary
Add a new simple progress display mode as an alternative to the verbose tree display. This provides a compact single-line progress indicator for users who prefer minimal output.
Display Format
During execution:
Parallel execution (multiple tasks running):
Completion:
Failure:
Configuration
Via environment variable:
TASKI_PROGRESS_MODE=simple ruby my_task.rb TASKI_PROGRESS_MODE=tree ruby my_task.rb # defaultVia API:
Changes
SimpleProgressDisplayclass with same interface asTreeProgressDisplayTaski.progress_modeandTaski.progress_mode=APITASKI_PROGRESS_MODEenvironment variable supportexamples/simple_progress_demo.rbTest plan
SimpleProgressDisplayaddedprogress_modeconfiguration addedCloses #97
🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes
New Features
progress_modeAPI orTASKI_PROGRESS_MODEenvironment variableDocumentation
Tests
✏️ Tip: You can customize this high-level summary in your review settings.