feat: parallelize all CLI commands and standardize output rendering#42
Merged
Conversation
ffdb358 to
55f59d0
Compare
status, diff, and validate iterated components sequentially while install and uninstall already used ThreadPoolExecutor with per-thread console buffering. Routing all five through run_components_parallel completes the parallelization started in the plan/apply refactor. Introduces display.py with a ContextVar-backed _ConsoleProxy singleton and print_error/print_warning/print_info/print_hint helpers, replacing 45+ scattered Console() instantiations and eliminating 15 formatting inconsistencies across commands, components, and groups. Fixes buffered replay ordering (component definition order, not completion order), adds defensive ContextVar reset to prevent thread-reuse leakage, centralizes section headers in the orchestrator via display_name, and upgrades _run_unbuffered to a full Progress bar. Migrates all Confirm.ask() to click.confirm(), fixes false-positive stale cache detection when base settings mtime changes without content diff, and adds Points to / Expected output for wrong-target skills and extensions.
Raw Rich markup patterns ([yellow]⚠, [red]✗, [dim]Run...) replaced with shared display helpers (print_warning, print_error, print_hint) throughout the codebase. Adds indent parameter to all display helpers for indented table-row usage. Migrates remaining Confirm.ask sites to click.confirm. Removes Console() fallback in config component.
Add print_success, print_done, print_unchanged, print_skipped, print_absent, print_update, print_would, print_add, print_progress to display.py. Migrate every remaining console.print call with a status symbol to the corresponding helper across all components, commands, and groups. Fixes whole-line dim wrapping inconsistency and eliminates all ctx.console.print calls with status symbols in install() methods.
Add dim() markup builder, print_dim/print_label helpers, and ICON_* constants to display.py. Migrate every remaining [dim], [green]✓, [yellow]○, [red]✗ pattern across all components, commands, and groups to the unified display API. Zero raw Rich symbol or dim markup remains outside display.py.
Crossfire review (3 Claude specialists + Codex + Gemini) surfaced 12 findings. Key changes: - Replace brittle `type() is not ComponentPlan` filter with `has_changes` property check to avoid silently dropping future component plans - Unify four identical parallel wrapper functions into `run_parallel()` with backward-compatible delegates - Remove redundant `display_name` attribute from Component base class; `label` is the single source of truth for section headers - Fix buffered replay to use `Console.print()` instead of raw `file.write()`, enabling proper Rich translation on all platforms - Defer plan-failure warnings until after Progress bar closes to prevent visual corruption - Propagate `KeyboardInterrupt`/`SystemExit` from worker threads instead of swallowing them as component errors - Remove duplicate section headers from buffered component methods - Complete raw Rich markup migration in symlinks.py and cli/__init__.py - Standardize deferred per-method imports across all components - Fix dead mock targets in confirmation tests (Confirm.ask → click.confirm) and add user-cancellation test coverage - Add tests for run_parallel aggregation and buffer replay ordering
6fbf27a to
e632843
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR completes full parallelization and CLI output standardization for ai-rules, mirroring what shell-configs#33 did for that repo — then goes further with a unified display system that centralizes all Rich markup through
display.py.ai-rules status,ai-rules diff, andai-rules validatestill iterated components sequentially whileinstallanduninstallalready usedThreadPoolExecutorwith per-thread console buffering. The CLI had 15+ output inconsistencies across error formatting, warning styles, status symbols, user prompts, and section headers — accumulated since the original parallel install refactor.display.pywith aContextVar-backed_ConsoleProxysingleton, 16 print helpers (print_error/print_warning/print_info/print_hint/print_success/print_done/print_unchanged/print_skipped/print_absent/print_update/print_would/print_add/print_progress/print_dim/print_label),dim()markup builder, and 13ICON_*constants — raw Rich markup remains only in intentional string-building contexts (diff coloring, interactive prompts)status,diff, andvalidatethroughrun_components_parallelvia a unifiedrun_parallel()function;labelonComponentis the single source of truth for orchestrator-injected section headersContextVarreset in_make_task, and upgrade_run_unbufferedto a fullProgressbarConsole.print()for buffered replay instead of rawfile.write(), enabling proper Rich translation on all platformsConfirm.ask()toclick.confirm(), removeConsole()fallback in config component's_display_symlink_statusis_cache_stale()false positives: base settings mtime now falls through to content diff check, preventing "stale" reports after every upgrade when content is unchangedWrong targetstatus in skills and extensions, matching the existing behavior in config filestype() is not ComponentPlanfilter withhas_changesproperty checkKeyboardInterrupt/SystemExitfrom worker threads instead of swallowing themrun_parallelaggregation, buffer replay ordering, and user-cancellation flows