-
Notifications
You must be signed in to change notification settings - Fork 53
✨ Add ProgressBarReporter for improved CLI user experience #950
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
✨ Add ProgressBarReporter for improved CLI user experience #950
Conversation
This commit adds a new ProgressBarReporter that displays a visual progress bar with real-time updates during rule execution. This enables containerized kantra to display the same progress experience as containerless mode. Changes: - Add ProgressBarReporter with in-place progress bar updates - Update CLI to support --progress-format=bar (now default) - Add comprehensive tests for ProgressBarReporter - Add benchmark for performance validation The progress bar format: Processing rules 42% |██████████░░░░░░░░░░░░░░░| 99/235 rule-name The reporter is thread-safe and updates in-place using carriage returns (\r), printing a final newline when analysis completes. Performance: ~531ns/op with minimal overhead (~125µs for 235 rules) Signed-off-by: tsanders <[email protected]>
Add more detailed documentation to the Report method to match the level of detail in other reporters. Documents the output format for each stage and clarifies the in-place update behavior during rule execution. Signed-off-by: tsanders <[email protected]>
WalkthroughThis pull request introduces a new ProgressBarReporter component to the progress subsystem and changes the CLI's default progress format from "text" to "bar". The reporter provides thread-safe, in-place updating visual progress bars with stage-specific output handling and comprehensive test coverage. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
cmd/analyzer/main.go (1)
468-472: Consider simplifying the default case.Both the
"bar"case and thedefaultcase return the same reporter type. While this works correctly, the explicit case at lines 468-469 makes thedefaultcase slightly redundant.Optionally, you could simplify by removing the explicit
"bar"case and relying on the default, or make the default more explicit:switch progressFormat { case "json": return progress.NewJSONReporter(writer), cleanup case "text": return progress.NewTextReporter(writer), cleanup - case "bar": - return progress.NewProgressBarReporter(writer), cleanup default: - // Default to progress bar + // Default to progress bar for "bar" or unknown formats return progress.NewProgressBarReporter(writer), cleanup }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
cmd/analyzer/main.go(2 hunks)progress/progress_bar_reporter.go(1 hunks)progress/progress_test.go(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
cmd/analyzer/main.go (1)
progress/progress_bar_reporter.go (1)
NewProgressBarReporter(39-44)
progress/progress_test.go (2)
progress/progress_bar_reporter.go (1)
NewProgressBarReporter(39-44)progress/progress.go (7)
ProgressEvent(64-88)Stage(112-112)StageRuleExecution(128-128)StageProviderInit(120-120)StageRuleParsing(124-124)StageDependencyAnalysis(131-131)StageComplete(134-134)
progress/progress_bar_reporter.go (1)
progress/progress.go (7)
ProgressEvent(64-88)Stage(112-112)StageProviderInit(120-120)StageRuleParsing(124-124)StageRuleExecution(128-128)StageDependencyAnalysis(131-131)StageComplete(134-134)
⏰ 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: e2e / e2e-api-integration-tests
🔇 Additional comments (8)
cmd/analyzer/main.go (1)
399-399: LGTM! Default format change aligns with PR objectives.The change to "bar" as the default progress format matches the PR's goal of providing improved CLI user experience with visual progress bars.
progress/progress_test.go (1)
604-894: Excellent test coverage for ProgressBarReporter!The test suite is comprehensive and well-structured, covering:
- Basic functionality and output format verification
- Boundary conditions (0%, 50%, 100% completion)
- Stage-specific output across all stages
- Long message truncation behavior
- In-place updates with carriage returns and final newline
- Thread safety under concurrent access
- Performance benchmarking
The tests appropriately verify the presence of expected strings, special characters (█, ░), and proper line endings.
progress/progress_bar_reporter.go (6)
22-28: LGTM! Well-designed struct for thread-safe progress reporting.The struct includes appropriate fields for tracking state (
lastLineLen,inProgress) and uses async.Mutexfor safe concurrent access.
39-44: LGTM! Simple and correct constructor.The constructor properly initializes the reporter with a fixed bar width of 25 characters, which is appropriate for typical terminal widths.
60-104: LGTM! Report method correctly handles all stages.The implementation properly:
- Uses mutex for thread safety
- Normalizes events before processing
- Clears previous progress bars before printing new messages
- Handles stage-specific formatting appropriately
- Updates progress bar in place only for rule execution stage
109-134: LGTM! Progress bar update logic is correct.The method properly:
- Clears previous output using carriage returns and spaces
- Updates the bar in place during progress
- Adds a final newline at 100% completion
- Maintains state for subsequent updates
139-174: LGTM! Progress bar construction is well-implemented.The method correctly:
- Calculates filled/empty portions based on percentage
- Clamps values to prevent overflow
- Uses appropriate Unicode box-drawing characters (█, ░)
- Formats the output clearly with percentage, visual bar, count, and rule name
- Truncates long rule names at 50 characters with a "..." indicator
177-186: LGTM! Line clearing logic is correct.The method properly clears any existing progress bar by:
- Checking if there's content to clear
- Using carriage returns and spaces to overwrite the line
- Resetting state variables
dymurray
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested this locally and looks great. Much better improvement
This commit implements real-time progress reporting in the VS Code extension
for kai analysis operations, providing users with detailed progress updates.
**Changes:**
1. **Progress Parser** (progressParser.ts)
- Created ProgressParser class to parse NDJSON progress events from stderr
- TypeScript types for ProgressEvent matching kai-analyzer-rpc format
- Handles all progress stages: init, provider_init, rule_parsing,
rule_execution, dependency_analysis, complete
- Robust parsing with error handling for non-JSON stderr content
2. **Analyzer Client Updates** (analyzerClient.ts)
- Added progress CLI flags to kai-analyzer-rpc spawn:
--progress-output=stderr --progress-format=json
- Integrated ProgressParser with stderr stream
- Added currentProgressCallback field for dynamic callback management
- Updated runAnalysis() to set progress callback during analysis
- Maps progress events to VS Code progress UI messages
**Progress UI Messages:**
- "Initializing analysis..." (init)
- "Provider: nodejs" (provider_init with message)
- "Loaded 45 rules" (rule_parsing with count)
- "Processing rules: 23/45 (51.1%)" (rule_execution with progress)
- "Analysis complete!" (complete)
**Implementation Details:**
- Progress parser feeds from stderr while preserving logging
- Callback is set when analysis starts, cleared in finally block
- Per-rule progress updates via rule_execution events
- Automatic percentage calculation from current/total
**Requirements:**
Requires kai-analyzer-rpc with progress reporting support
(konveyor/kai#895)
Related to: konveyor/analyzer-lsp#949, konveyor/analyzer-lsp#950
Signed-off-by: tsanders <[email protected]>
Add ProgressBarReporter for improved CLI user experience
Summary
This PR adds a new
ProgressBarReporterthat displays visual progress bars during rule execution, enabling consistent user experience between containerless and containerized Kantra deployments.Background
Kantra uses analyzer-lsp in two different modes:
ChannelReporter, and renders progress bars in Kantra codekonveyor-analyzerbinary as a container and streams its outputCurrent Problem
These two modes provide inconsistent user experiences:
This creates confusion for users who expect the same experience regardless of deployment mode.
Solution
This PR adds a
ProgressBarReporterthat:\r) for clean terminal experience--progress-format=bar)Example Output
The progress bar updates in-place during rule execution, providing real-time feedback.
Changes
New Files
progress/progress_bar_reporter.go- ProgressBarReporter implementation with in-place updatesModified Files
cmd/analyzer/main.go- Updated CLI to support--progress-format=bar(now default)progress/progress_test.go- Added comprehensive tests for ProgressBarReporterKey Features
CLI Flags
The analyzer binary now supports:
--progress-output=stderr|stdout|<file>- Where to write progress (default: none)--progress-format=bar|text|json- Output format (default: bar)Testing
Unit Tests (7 test cases + benchmark)
TestProgressBarReporter- Basic functionalityTestProgressBarReporterProgressPercentages- 0%, 50%, 100% scenariosTestProgressBarReporterStages- All stages (init, parsing, execution, completion)TestProgressBarReporterRuleNameTruncation- Long rule namesTestProgressBarReporterMultipleUpdates- Progress updates with carriage returnsTestProgressBarReporterConcurrency- Thread safetyBenchmarkProgressBarReporter- Performance validationBenchmark Results
Performance overhead: ~531ns per event × 235 rules = ~125µs total (negligible)
Integration Testing
Backward Compatibility
--progress-format=textor--progress-format=json--progress-format=textfor old behavior)Benefits
Related Issues
Checklist
Screenshots / Demo
Notes for Reviewers
Ready for review! Happy to address any feedback or make adjustments.
Summary by CodeRabbit
New Features
Improvements