feat: Rich output formatting with colors, boxes, spinners #595
Conversation
mikejmorgan-ai
requested review from
Anshgrover23 and
Suyashd999
as code owners
📝 WalkthroughWalkthroughThis pull request introduces a comprehensive rich-based terminal UI framework across two modules: Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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 |
Summary of ChangesHello @mikejmorgan-ai, 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 upgrades the CLI's visual output by integrating the Highlights
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
|
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
| """ | ||
|
|
||
| from contextlib import contextmanager | ||
| from dataclasses import dataclass, field |
| from contextlib import contextmanager | ||
| from dataclasses import dataclass, field | ||
| from enum import Enum | ||
| from typing import Any, Generator, List, Optional, Tuple |
| from typing import Any, Generator, List, Optional, Tuple | ||
|
|
||
| from rich import box | ||
| from rich.console import Console, Group |
| TimeElapsedColumn, | ||
| TimeRemainingColumn, | ||
| ) | ||
| from rich.spinner import Spinner |
| ) | ||
| from rich.spinner import Spinner | ||
| from rich.status import Status | ||
| from rich.style import Style |
| from rich.status import Status | ||
| from rich.style import Style | ||
| from rich.table import Table | ||
| from rich.text import Text |
| Issue: #242 - Output Polish: Rich Formatting with Colors, Boxes, Spinners | ||
| """ | ||
|
|
||
| import io |
| """ | ||
|
|
||
| import io | ||
| from unittest.mock import patch |
| import io | ||
| from unittest.mock import patch | ||
|
|
||
| import pytest |
| from unittest.mock import patch | ||
|
|
||
| import pytest | ||
| from rich.console import Console |
![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 pull request introduces a comprehensive set of rich output formatting utilities using the rich library, which is a great enhancement for the CLI's user experience. A new output_formatter.py module provides a structured and powerful way to create styled boxes, tables, and progress indicators. The existing branding.py module is also updated with new formatting functions.
My main feedback is architectural. There is significant code duplication and conceptual overlap between the new functions in branding.py and the utilities in output_formatter.py. To improve maintainability and consistency, branding.py should be refactored to act as a thin wrapper around the more generic output_formatter.py module.
I've also pointed out several opportunities for code simplification and cleanup, such as removing an unused parameter and refactoring repetitive logic. Additionally, I found an incorrect assertion in one of the new tests.
Overall, this is a fantastic addition, and with some architectural refinement, it will be a very solid and maintainable feature.
| # ============================================ | ||
| # Rich Output Formatting (Issue #242) | ||
| # ============================================ |
There was a problem hiding this comment.
This PR introduces a new output_formatter.py module which provides a more structured and reusable way to format output (e.g., using StatusInfo and TableColumn dataclasses). However, many of the new cx_* functions in this file reimplement similar logic instead of using the new module. This creates significant code duplication and two parallel implementations for formatting, which will be hard to maintain.
For example:
cx_status_boxshould useformat_status_boxfrom the new module.cx_tableis very similar toformat_table.cx_package_tableduplicates table creation logic.
To improve the architecture, this branding.py module should be refactored to be a thin wrapper around output_formatter.py. The cx_* functions should delegate the core formatting work to the functions in the new module. This will centralize formatting logic and improve maintainability.
| def test_output_styles_defined(self): | ||
| """Test all output styles are defined.""" | ||
| styles = [OutputStyle.DEFAULT, OutputStyle.SUCCESS, OutputStyle.WARNING, OutputStyle.ERROR] | ||
| assert len(styles) == 4 |
There was a problem hiding this comment.
The test test_output_styles_defined incorrectly asserts that len(styles) == 4. The OutputStyle enum in cortex/output_formatter.py actually has 6 members. This test is incomplete and gives a false sense of correctness. It should be updated to check for all members of the enum.
| def test_output_styles_defined(self): | |
| """Test all output styles are defined.""" | |
| styles = [OutputStyle.DEFAULT, OutputStyle.SUCCESS, OutputStyle.WARNING, OutputStyle.ERROR] | |
| assert len(styles) == 4 | |
| def test_output_styles_defined(self): | |
| """Test all output styles are defined.""" | |
| assert len(OutputStyle) == 6 |
| # Color-code actions | ||
| if "install" in action.lower(): | ||
| action_styled = f"[green]{action}[/green]" | ||
| elif "remove" in action.lower() or "uninstall" in action.lower(): | ||
| action_styled = f"[red]{action}[/red]" | ||
| elif "update" in action.lower() or "upgrade" in action.lower(): | ||
| action_styled = f"[yellow]{action}[/yellow]" | ||
| else: | ||
| action_styled = action |
There was a problem hiding this comment.
The if/elif/else chain for styling the action string is a bit verbose and can be hard to maintain. This can be made more data-driven by using a dictionary to map keywords to styles, which is more scalable if more action types are added in the future.
| # Color-code actions | |
| if "install" in action.lower(): | |
| action_styled = f"[green]{action}[/green]" | |
| elif "remove" in action.lower() or "uninstall" in action.lower(): | |
| action_styled = f"[red]{action}[/red]" | |
| elif "update" in action.lower() or "upgrade" in action.lower(): | |
| action_styled = f"[yellow]{action}[/yellow]" | |
| else: | |
| action_styled = action | |
| # Color-code actions | |
| action_color = "white" | |
| action_lower = action.lower() | |
| if "install" in action_lower: | |
| action_color = "green" | |
| elif "remove" in action_lower or "uninstall" in action_lower: | |
| action_color = "red" | |
| elif "update" in action_lower or "upgrade" in action_lower: | |
| action_color = "yellow" | |
| action_styled = action if action_color == "white" else f"[{action_color}]{action}[/{action_color}]" |
|
|
||
| def cx_error(message: str) -> None: | ||
| """Print an error message with X.""" | ||
| console.print(f"[{CORTEX_ERROR}]✗[/{CORTEX_ERROR}] [{CORTEX_ERROR}]{message}[/{CORTEX_ERROR}]") |
There was a problem hiding this comment.
The cx_error function contains redundant Rich markup for coloring. The icon and the message are styled separately with the same color. This can be simplified by wrapping the entire string in a single color tag, which improves readability and maintainability.
| console.print(f"[{CORTEX_ERROR}]✗[/{CORTEX_ERROR}] [{CORTEX_ERROR}]{message}[/{CORTEX_ERROR}]") | |
| console.print(f"[{CORTEX_ERROR}]✗ {message}[/{CORTEX_ERROR}]") |
|
|
||
| def cx_warning(message: str) -> None: | ||
| """Print a warning message with warning icon.""" | ||
| console.print(f"[{CORTEX_WARNING}]⚠[/{CORTEX_WARNING}] [{CORTEX_WARNING}]{message}[/{CORTEX_WARNING}]") |
There was a problem hiding this comment.
The cx_warning function contains redundant Rich markup for coloring. The icon and the message are styled separately with the same color. This can be simplified by wrapping the entire string in a single color tag, which improves readability and maintainability.
| console.print(f"[{CORTEX_WARNING}]⚠[/{CORTEX_WARNING}] [{CORTEX_WARNING}]{message}[/{CORTEX_WARNING}]") | |
| console.print(f"[{CORTEX_WARNING}]⚠ {message}[/{CORTEX_WARNING}]") |
| def __init__( | ||
| self, | ||
| description: str, | ||
| total: Optional[int] = None, | ||
| show_speed: bool = False, | ||
| ): |
There was a problem hiding this comment.
The __init__ method of ProgressTracker accepts a show_speed parameter, but it is never used within the class. This can be confusing for developers. The parameter should be removed to clean up the method signature.
def __init__(
self,
description: str,
total: Optional[int] = None,
):| for step in self.steps: | ||
| status = self.step_status[step] | ||
| if status == "pending": | ||
| icon = f"[dim]{STATUS_ICONS['pending']}[/dim]" | ||
| style = "dim" | ||
| elif status == "running": | ||
| icon = f"[cyan]{STATUS_ICONS['running']}[/cyan]" | ||
| style = "cyan" | ||
| elif status == "completed": | ||
| icon = f"[green]{STATUS_ICONS['success']}[/green]" | ||
| style = "green" | ||
| elif status == "failed": | ||
| icon = f"[red]{STATUS_ICONS['error']}[/red]" | ||
| style = "red" | ||
| else: | ||
| icon = f"[dim]{STATUS_ICONS['skipped']}[/dim]" | ||
| style = "dim" | ||
|
|
||
| lines.append(f" {icon} [{style}]{step}[/{style}]") |
There was a problem hiding this comment.
The if/elif/else block in the _render method for determining the icon and style based on status is repetitive. This can be refactored into a more concise and data-driven structure, like a dictionary lookup, to improve maintainability.
status_configs = {
"pending": ("dim", STATUS_ICONS["pending"]),
"running": ("cyan", STATUS_ICONS["running"]),
"completed": ("green", STATUS_ICONS["success"]),
"failed": ("red", STATUS_ICONS["error"]),
"skipped": ("dim", STATUS_ICONS["skipped"]),
}
for step in self.steps:
status = self.step_status.get(step, "skipped")
style, icon_char = status_configs.get(status, status_configs["skipped"])
icon = f"[{style}]{icon_char}[/{style}]"
lines.append(f" {icon} [{style}]{step}[/{style}]")|
|
||
| def print_error(message: str): | ||
| """Print an error message with icon.""" | ||
| console.print(f"[red]{STATUS_ICONS['error']}[/red] [red]{message}[/red]") |
There was a problem hiding this comment.
This line has redundant Rich markup. The icon and message are styled separately with the same color. You can simplify this by wrapping the whole string in a single [red] tag.
console.print(f"[red]{STATUS_ICONS['error']} {message}[/red]")|
|
||
| def print_warning(message: str): | ||
| """Print a warning message with icon.""" | ||
| console.print(f"[yellow]{STATUS_ICONS['warning']}[/yellow] [yellow]{message}[/yellow]") |
There was a problem hiding this comment.
This line has redundant Rich markup. The icon and message are styled separately with the same color. You can simplify this by wrapping the whole string in a single [yellow] tag.
console.print(f"[yellow]{STATUS_ICONS['warning']} {message}[/yellow]")![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Fix all issues with AI agents
In `@cortex/branding.py`:
- Around line 223-255: Update the cx_table function signature to use modern PEP
585/604 type syntax: replace List[str] and Optional[...] with built-in generics
and union types (e.g., headers: list[str], rows: list[list[str]], title: str |
None = None, row_styles: list[str] | None = None) and adjust any corresponding
imports if present (remove or keep typing imports as needed); keep the rest of
the function body unchanged and ensure the annotation names reference cx_table
so the change is localized.
- Around line 142-174: The function cx_box uses legacy typing Optional[str];
update its signature to use modern union syntax (str | None) for title and
subtitle (i.e., def cx_box(content: str, title: str | None = None, subtitle: str
| None = None, status: str = "info") -> None:) and replace any other occurrences
of Optional[str] in that function; remove or update unused Optional import if no
longer needed elsewhere. Ensure type hints for title/subtitle and any related
annotations use the new PEP 604 form while leaving the rest of the function
intact.
- Line 14: The import of typing.List and typing.Tuple is deprecated; remove List
and Tuple from the typing import line and switch to built-in generic types and
PEP 604 unions throughout the module: replace usages like List[str] with
list[str], List[Tuple[str, str, str]] with list[tuple[str, str, str]],
Optional[str] with str | None and Optional[List[str]] with list[str] | None,
updating all annotations in cortex/branding.py (e.g., function signatures,
variable annotations, and return types) to use these built-ins and the | None
form.
In `@cortex/output_formatter.py`:
- Around line 169-176: The style mapping for OutputStyle.INFO is inconsistent
between the style_colors dict and format_box: update one to match the other so
both use the same color; locate the style_colors mapping (variable style_colors)
and the function format_box and change whichever uses "cyan" or "blue" so
OutputStyle.INFO is the same string in both places (e.g., set both to "blue" or
both to "cyan") and run the formatter/tests to ensure no other references rely
on the previous value.
In `@tests/test_output_formatting.py`:
- Around line 416-428: The tests for the OutputStyle enum are incomplete: they
only check DEFAULT, SUCCESS, WARNING, ERROR and assert two enum values; update
TestOutputStyle to include the missing OutputStyle.INFO and OutputStyle.MUTED
checks and assert their .value strings match the expected "info" and "muted" (in
the test_output_style_values case), and ensure the styles list in
test_output_styles_defined contains all six enum members (OutputStyle.DEFAULT,
OutputStyle.SUCCESS, OutputStyle.WARNING, OutputStyle.ERROR, OutputStyle.INFO,
OutputStyle.MUTED) so the length assertion reflects six items.
🧹 Nitpick comments (5)
cortex/output_formatter.py (3)
10-13: Use modern type hint syntax.For Python 3.9+, prefer built-in generics and union syntax over
typingimports. This aligns with the coding guidelines and matches the linter configuration.Suggested fix
from contextlib import contextmanager from dataclasses import dataclass, field from enum import Enum -from typing import Any, Generator, List, Optional, Tuple +from typing import Any, GeneratorThen update usages throughout the file:
List[...]→list[...]Optional[...]→... | NoneTuple[...]→tuple[...]
15-35: Remove unused imports.
Group(line 16) andSpinner(line 30) are imported but never used in this module.Suggested fix
from rich import box -from rich.console import Console, Group +from rich.console import Console from rich.live import Live from rich.panel import Panel from rich.progress import ( BarColumn, MofNCompleteColumn, Progress, SpinnerColumn, TaskID, TaskProgressColumn, TextColumn, TimeElapsedColumn, TimeRemainingColumn, ) -from rich.spinner import Spinner from rich.status import Status from rich.style import Style from rich.table import Table from rich.text import Text from rich.tree import Tree
544-550: Avoid mutating the parameter variable.The function reassigns
num_bytesfrominttofloatduring iteration. Using a separate local variable improves clarity and maintains type consistency.Suggested fix
def format_bytes(num_bytes: int) -> str: """Format bytes to human-readable string.""" + size = float(num_bytes) for unit in ["B", "KB", "MB", "GB", "TB"]: - if abs(num_bytes) < 1024.0: - return f"{num_bytes:.1f} {unit}" - num_bytes /= 1024.0 - return f"{num_bytes:.1f} PB" + if abs(size) < 1024.0: + return f"{size:.1f} {unit}" + size /= 1024.0 + return f"{size:.1f} PB"tests/test_output_formatting.py (1)
7-8: Remove unused import.The
iomodule is imported but never used in this test file.Suggested fix
-import io from unittest.mock import patchcortex/branding.py (1)
196-202: Consider consolidating duplicated style mappings.The
style_colorsdictionary here duplicates the one inoutput_formatter.py(lines 169-176). Consider importing and reusing a shared mapping to ensure consistency and reduce maintenance burden.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
cortex/branding.pycortex/output_formatter.pytests/test_output_formatting.py
🧰 Additional context used
📓 Path-based instructions (2)
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
**/*.py: Follow PEP 8 style guide for Python code
Include type hints in Python code
Add docstrings for all public APIs in Python code
Use dry-run mode by default for all installation operations
Do not use silent sudo - require explicit user confirmation for privilege escalation
Implement Firejail sandboxing for execution of untrusted code
Log all operations to ~/.cortex/history.db for audit purposes
Files:
tests/test_output_formatting.pycortex/branding.pycortex/output_formatter.py
tests/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Maintain test coverage above 80% for pull requests
Files:
tests/test_output_formatting.py
🧬 Code graph analysis (2)
cortex/branding.py (5)
cortex/cli.py (1)
status(1173-1181)cortex/kernel_features/kv_cache_manager.py (1)
status(130-137)cortex/kernel_features/accelerator_limits.py (2)
status(97-105)get(69-72)cortex/kernel_features/model_lifecycle.py (1)
status(135-143)cortex/graceful_degradation.py (1)
get(109-141)
cortex/output_formatter.py (2)
cortex/semantic_cache.py (1)
total(32-34)tests/test_progress_indicators.py (1)
steps(315-320)
🪛 GitHub Actions: CI
cortex/branding.py
[error] 14-14: Ruff: UP035 'typing.List' is deprecated. Use 'list' instead.
🪛 GitHub Check: lint
cortex/branding.py
[failure] 14-14: Ruff (UP035)
cortex/branding.py:14:1: UP035 typing.Tuple is deprecated, use tuple instead
[failure] 14-14: Ruff (UP035)
cortex/branding.py:14:1: UP035 typing.List is deprecated, use list instead
[failure] 226-226: Ruff (UP045)
cortex/branding.py:226:12: UP045 Use X | None for type annotations
[failure] 225-225: Ruff (UP006)
cortex/branding.py:225:16: UP006 Use list instead of List for type annotation
[failure] 225-225: Ruff (UP006)
cortex/branding.py:225:11: UP006 Use list instead of List for type annotation
[failure] 224-224: Ruff (UP006)
cortex/branding.py:224:14: UP006 Use list instead of List for type annotation
[failure] 178-178: Ruff (UP006)
cortex/branding.py:178:17: UP006 Use tuple instead of Tuple for type annotation
[failure] 178-178: Ruff (UP006)
cortex/branding.py:178:12: UP006 Use list instead of List for type annotation
[failure] 145-145: Ruff (UP045)
cortex/branding.py:145:15: UP045 Use X | None for type annotations
[failure] 144-144: Ruff (UP045)
cortex/branding.py:144:12: UP045 Use X | None for type annotations
🪛 GitHub Check: Lint
cortex/branding.py
[failure] 14-14: Ruff (UP035)
cortex/branding.py:14:1: UP035 typing.Tuple is deprecated, use tuple instead
[failure] 14-14: Ruff (UP035)
cortex/branding.py:14:1: UP035 typing.List is deprecated, use list instead
[failure] 226-226: Ruff (UP045)
cortex/branding.py:226:12: UP045 Use X | None for type annotations
[failure] 225-225: Ruff (UP006)
cortex/branding.py:225:16: UP006 Use list instead of List for type annotation
[failure] 225-225: Ruff (UP006)
cortex/branding.py:225:11: UP006 Use list instead of List for type annotation
[failure] 224-224: Ruff (UP006)
cortex/branding.py:224:14: UP006 Use list instead of List for type annotation
[failure] 178-178: Ruff (UP006)
cortex/branding.py:178:17: UP006 Use tuple instead of Tuple for type annotation
[failure] 178-178: Ruff (UP006)
cortex/branding.py:178:12: UP006 Use list instead of List for type annotation
[failure] 145-145: Ruff (UP045)
cortex/branding.py:145:15: UP045 Use X | None for type annotations
[failure] 144-144: Ruff (UP045)
cortex/branding.py:144:12: UP045 Use X | None for type annotations
⏰ 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). (3)
- GitHub Check: test (3.10)
- GitHub Check: test (3.12)
- GitHub Check: test (3.11)
🔇 Additional comments (8)
cortex/output_formatter.py (4)
341-408: LGTM!The
ProgressTrackerclass is well-implemented with proper context manager protocol, null checks, and clean separation of concerns.
410-499: LGTM!The
MultiStepProgressclass provides a clean API for multi-step operations with proper lifecycle management. The_rendermethod handles all status states consistently.
306-338: LGTM!The
spinner_contextproperly handles success and error states while re-raising exceptions to preserve error handling semantics.
501-541: LGTM!The print helper functions provide a clean, consistent API for common output patterns.
tests/test_output_formatting.py (2)
486-524: LGTM!The
MultiStepProgresstests cover initialization and all step state transitions (running, completed, failed, skipped). Good coverage of the core functionality.
56-81: LGTM!Good coverage of branding constants and status icons with clear test organization.
cortex/branding.py (2)
257-294: LGTM!The action color-coding logic is well-implemented with case-insensitive matching for install, remove, and update actions.
309-332: LGTM!Clean, focused helper functions with consistent styling. Good addition to the branding API.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| - Consistent visual language | ||
| """ | ||
|
|
||
| from typing import List, Optional, Tuple |
There was a problem hiding this comment.
Fix deprecated type hint imports (CI failure).
The pipeline is failing because typing.List and typing.Tuple are deprecated in Python 3.9+. Use built-in list and tuple directly.
Suggested fix
-from typing import List, Optional, Tuple
+from typing import OptionalThen update the type annotations throughout:
List[str]→list[str]List[Tuple[str, str, str]]→list[tuple[str, str, str]]Optional[str]→str | NoneOptional[List[str]]→list[str] | None
🧰 Tools
🪛 GitHub Actions: CI
[error] 14-14: Ruff: UP035 'typing.List' is deprecated. Use 'list' instead.
🪛 GitHub Check: lint
[failure] 14-14: Ruff (UP035)
cortex/branding.py:14:1: UP035 typing.Tuple is deprecated, use tuple instead
[failure] 14-14: Ruff (UP035)
cortex/branding.py:14:1: UP035 typing.List is deprecated, use list instead
🪛 GitHub Check: Lint
[failure] 14-14: Ruff (UP035)
cortex/branding.py:14:1: UP035 typing.Tuple is deprecated, use tuple instead
[failure] 14-14: Ruff (UP035)
cortex/branding.py:14:1: UP035 typing.List is deprecated, use list instead
🤖 Prompt for AI Agents
In `@cortex/branding.py` at line 14, The import of typing.List and typing.Tuple is
deprecated; remove List and Tuple from the typing import line and switch to
built-in generic types and PEP 604 unions throughout the module: replace usages
like List[str] with list[str], List[Tuple[str, str, str]] with list[tuple[str,
str, str]], Optional[str] with str | None and Optional[List[str]] with list[str]
| None, updating all annotations in cortex/branding.py (e.g., function
signatures, variable annotations, and return types) to use these built-ins and
the | None form.
| def cx_box( | ||
| content: str, | ||
| title: Optional[str] = None, | ||
| subtitle: Optional[str] = None, | ||
| status: str = "info", | ||
| ) -> None: | ||
| """ | ||
| Print content in a styled box/panel. | ||
| Args: | ||
| content: Content to display | ||
| title: Optional box title | ||
| subtitle: Optional box subtitle | ||
| status: Style - "info", "success", "warning", "error" | ||
| """ | ||
| border_colors = { | ||
| "info": CORTEX_CYAN, | ||
| "success": CORTEX_SUCCESS, | ||
| "warning": CORTEX_WARNING, | ||
| "error": CORTEX_ERROR, | ||
| } | ||
| border_style = border_colors.get(status, CORTEX_CYAN) | ||
|
|
||
| panel = Panel( | ||
| content, | ||
| title=f"[bold]{title}[/bold]" if title else None, | ||
| subtitle=f"[dim]{subtitle}[/dim]" if subtitle else None, | ||
| border_style=border_style, | ||
| padding=(1, 2), | ||
| box=box.ROUNDED, | ||
| ) | ||
| console.print(panel) | ||
|
|
There was a problem hiding this comment.
Update type hints to modern syntax.
Per static analysis hints on lines 144-145, use str | None instead of Optional[str]:
Suggested fix
def cx_box(
content: str,
- title: Optional[str] = None,
- subtitle: Optional[str] = None,
+ title: str | None = None,
+ subtitle: str | None = None,
status: str = "info",
) -> None:📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def cx_box( | |
| content: str, | |
| title: Optional[str] = None, | |
| subtitle: Optional[str] = None, | |
| status: str = "info", | |
| ) -> None: | |
| """ | |
| Print content in a styled box/panel. | |
| Args: | |
| content: Content to display | |
| title: Optional box title | |
| subtitle: Optional box subtitle | |
| status: Style - "info", "success", "warning", "error" | |
| """ | |
| border_colors = { | |
| "info": CORTEX_CYAN, | |
| "success": CORTEX_SUCCESS, | |
| "warning": CORTEX_WARNING, | |
| "error": CORTEX_ERROR, | |
| } | |
| border_style = border_colors.get(status, CORTEX_CYAN) | |
| panel = Panel( | |
| content, | |
| title=f"[bold]{title}[/bold]" if title else None, | |
| subtitle=f"[dim]{subtitle}[/dim]" if subtitle else None, | |
| border_style=border_style, | |
| padding=(1, 2), | |
| box=box.ROUNDED, | |
| ) | |
| console.print(panel) | |
| def cx_box( | |
| content: str, | |
| title: str | None = None, | |
| subtitle: str | None = None, | |
| status: str = "info", | |
| ) -> None: | |
| """ | |
| Print content in a styled box/panel. | |
| Args: | |
| content: Content to display | |
| title: Optional box title | |
| subtitle: Optional box subtitle | |
| status: Style - "info", "success", "warning", "error" | |
| """ | |
| border_colors = { | |
| "info": CORTEX_CYAN, | |
| "success": CORTEX_SUCCESS, | |
| "warning": CORTEX_WARNING, | |
| "error": CORTEX_ERROR, | |
| } | |
| border_style = border_colors.get(status, CORTEX_CYAN) | |
| panel = Panel( | |
| content, | |
| title=f"[bold]{title}[/bold]" if title else None, | |
| subtitle=f"[dim]{subtitle}[/dim]" if subtitle else None, | |
| border_style=border_style, | |
| padding=(1, 2), | |
| box=box.ROUNDED, | |
| ) | |
| console.print(panel) |
🧰 Tools
🪛 GitHub Check: lint
[failure] 145-145: Ruff (UP045)
cortex/branding.py:145:15: UP045 Use X | None for type annotations
[failure] 144-144: Ruff (UP045)
cortex/branding.py:144:12: UP045 Use X | None for type annotations
🪛 GitHub Check: Lint
[failure] 145-145: Ruff (UP045)
cortex/branding.py:145:15: UP045 Use X | None for type annotations
[failure] 144-144: Ruff (UP045)
cortex/branding.py:144:12: UP045 Use X | None for type annotations
🤖 Prompt for AI Agents
In `@cortex/branding.py` around lines 142 - 174, The function cx_box uses legacy
typing Optional[str]; update its signature to use modern union syntax (str |
None) for title and subtitle (i.e., def cx_box(content: str, title: str | None =
None, subtitle: str | None = None, status: str = "info") -> None:) and replace
any other occurrences of Optional[str] in that function; remove or update unused
Optional import if no longer needed elsewhere. Ensure type hints for
title/subtitle and any related annotations use the new PEP 604 form while
leaving the rest of the function intact.
| def cx_table( | ||
| headers: List[str], | ||
| rows: List[List[str]], | ||
| title: Optional[str] = None, | ||
| row_styles: Optional[List[str]] = None, | ||
| ) -> None: | ||
| """ | ||
| Print a formatted table with Cortex styling. | ||
| Args: | ||
| headers: Column header names | ||
| rows: List of rows (each row is a list of cell values) | ||
| title: Optional table title | ||
| row_styles: Optional list of styles for each row | ||
| """ | ||
| table = Table( | ||
| title=f"[bold cyan]{title}[/bold cyan]" if title else None, | ||
| show_header=True, | ||
| header_style="bold cyan", | ||
| border_style=CORTEX_CYAN, | ||
| box=box.ROUNDED, | ||
| padding=(0, 1), | ||
| ) | ||
|
|
||
| for header in headers: | ||
| table.add_column(header, style="cyan") | ||
|
|
||
| for i, row in enumerate(rows): | ||
| style = row_styles[i] if row_styles and i < len(row_styles) else None | ||
| table.add_row(*row, style=style) | ||
|
|
||
| console.print(table) | ||
|
|
There was a problem hiding this comment.
Update type hints to modern syntax.
Per static analysis hints on lines 224-226, update to modern Python type annotation syntax:
Suggested fix
def cx_table(
- headers: List[str],
- rows: List[List[str]],
- title: Optional[str] = None,
- row_styles: Optional[List[str]] = None,
+ headers: list[str],
+ rows: list[list[str]],
+ title: str | None = None,
+ row_styles: list[str] | None = None,
) -> None:🧰 Tools
🪛 GitHub Check: lint
[failure] 226-226: Ruff (UP045)
cortex/branding.py:226:12: UP045 Use X | None for type annotations
[failure] 225-225: Ruff (UP006)
cortex/branding.py:225:16: UP006 Use list instead of List for type annotation
[failure] 225-225: Ruff (UP006)
cortex/branding.py:225:11: UP006 Use list instead of List for type annotation
[failure] 224-224: Ruff (UP006)
cortex/branding.py:224:14: UP006 Use list instead of List for type annotation
🪛 GitHub Check: Lint
[failure] 226-226: Ruff (UP045)
cortex/branding.py:226:12: UP045 Use X | None for type annotations
[failure] 225-225: Ruff (UP006)
cortex/branding.py:225:16: UP006 Use list instead of List for type annotation
[failure] 225-225: Ruff (UP006)
cortex/branding.py:225:11: UP006 Use list instead of List for type annotation
[failure] 224-224: Ruff (UP006)
cortex/branding.py:224:14: UP006 Use list instead of List for type annotation
🤖 Prompt for AI Agents
In `@cortex/branding.py` around lines 223 - 255, Update the cx_table function
signature to use modern PEP 585/604 type syntax: replace List[str] and
Optional[...] with built-in generics and union types (e.g., headers: list[str],
rows: list[list[str]], title: str | None = None, row_styles: list[str] | None =
None) and adjust any corresponding imports if present (remove or keep typing
imports as needed); keep the rest of the function body unchanged and ensure the
annotation names reference cx_table so the change is localized.
| style_colors = { | ||
| OutputStyle.SUCCESS: "green", | ||
| OutputStyle.WARNING: "yellow", | ||
| OutputStyle.ERROR: "red", | ||
| OutputStyle.INFO: "cyan", | ||
| OutputStyle.MUTED: "dim", | ||
| OutputStyle.DEFAULT: "white", | ||
| } |
There was a problem hiding this comment.
Inconsistent color mapping for OutputStyle.INFO.
In format_box (line 126), OutputStyle.INFO maps to "blue", but here it maps to "cyan". This inconsistency could cause unexpected visual differences between the two functions.
Suggested fix
style_colors = {
OutputStyle.SUCCESS: "green",
OutputStyle.WARNING: "yellow",
OutputStyle.ERROR: "red",
- OutputStyle.INFO: "cyan",
+ OutputStyle.INFO: "blue",
OutputStyle.MUTED: "dim",
OutputStyle.DEFAULT: "white",
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| style_colors = { | |
| OutputStyle.SUCCESS: "green", | |
| OutputStyle.WARNING: "yellow", | |
| OutputStyle.ERROR: "red", | |
| OutputStyle.INFO: "cyan", | |
| OutputStyle.MUTED: "dim", | |
| OutputStyle.DEFAULT: "white", | |
| } | |
| style_colors = { | |
| OutputStyle.SUCCESS: "green", | |
| OutputStyle.WARNING: "yellow", | |
| OutputStyle.ERROR: "red", | |
| OutputStyle.INFO: "blue", | |
| OutputStyle.MUTED: "dim", | |
| OutputStyle.DEFAULT: "white", | |
| } |
🤖 Prompt for AI Agents
In `@cortex/output_formatter.py` around lines 169 - 176, The style mapping for
OutputStyle.INFO is inconsistent between the style_colors dict and format_box:
update one to match the other so both use the same color; locate the
style_colors mapping (variable style_colors) and the function format_box and
change whichever uses "cyan" or "blue" so OutputStyle.INFO is the same string in
both places (e.g., set both to "blue" or both to "cyan") and run the
formatter/tests to ensure no other references rely on the previous value.
| class TestOutputStyle: | ||
| """Tests for OutputStyle enum.""" | ||
|
|
||
| def test_output_styles_defined(self): | ||
| """Test all output styles are defined.""" | ||
| styles = [OutputStyle.DEFAULT, OutputStyle.SUCCESS, OutputStyle.WARNING, OutputStyle.ERROR] | ||
| assert len(styles) == 4 | ||
|
|
||
| def test_output_style_values(self): | ||
| """Test output style values.""" | ||
| assert OutputStyle.SUCCESS.value == "success" | ||
| assert OutputStyle.ERROR.value == "error" | ||
|
|
There was a problem hiding this comment.
Incomplete test coverage for OutputStyle enum.
The test only verifies 4 of the 6 OutputStyle values. INFO and MUTED are missing from the assertions.
Suggested fix
def test_output_styles_defined(self):
"""Test all output styles are defined."""
- styles = [OutputStyle.DEFAULT, OutputStyle.SUCCESS, OutputStyle.WARNING, OutputStyle.ERROR]
- assert len(styles) == 4
+ styles = [
+ OutputStyle.DEFAULT,
+ OutputStyle.SUCCESS,
+ OutputStyle.WARNING,
+ OutputStyle.ERROR,
+ OutputStyle.INFO,
+ OutputStyle.MUTED,
+ ]
+ assert len(styles) == 6📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| class TestOutputStyle: | |
| """Tests for OutputStyle enum.""" | |
| def test_output_styles_defined(self): | |
| """Test all output styles are defined.""" | |
| styles = [OutputStyle.DEFAULT, OutputStyle.SUCCESS, OutputStyle.WARNING, OutputStyle.ERROR] | |
| assert len(styles) == 4 | |
| def test_output_style_values(self): | |
| """Test output style values.""" | |
| assert OutputStyle.SUCCESS.value == "success" | |
| assert OutputStyle.ERROR.value == "error" | |
| class TestOutputStyle: | |
| """Tests for OutputStyle enum.""" | |
| def test_output_styles_defined(self): | |
| """Test all output styles are defined.""" | |
| styles = [ | |
| OutputStyle.DEFAULT, | |
| OutputStyle.SUCCESS, | |
| OutputStyle.WARNING, | |
| OutputStyle.ERROR, | |
| OutputStyle.INFO, | |
| OutputStyle.MUTED, | |
| ] | |
| assert len(styles) == 6 | |
| def test_output_style_values(self): | |
| """Test output style values.""" | |
| assert OutputStyle.SUCCESS.value == "success" | |
| assert OutputStyle.ERROR.value == "error" | |
🤖 Prompt for AI Agents
In `@tests/test_output_formatting.py` around lines 416 - 428, The tests for the
OutputStyle enum are incomplete: they only check DEFAULT, SUCCESS, WARNING,
ERROR and assert two enum values; update TestOutputStyle to include the missing
OutputStyle.INFO and OutputStyle.MUTED checks and assert their .value strings
match the expected "info" and "muted" (in the test_output_style_values case),
and ensure the styles list in test_output_styles_defined contains all six enum
members (OutputStyle.DEFAULT, OutputStyle.SUCCESS, OutputStyle.WARNING,
OutputStyle.ERROR, OutputStyle.INFO, OutputStyle.MUTED) so the length assertion
reflects six items.
…242) - Add output_formatter.py with comprehensive formatting utilities: - format_box/format_status_box for framed content - format_table/format_package_table for tabular data - ProgressTracker and MultiStepProgress for progress bars - spinner_context for long operations - Color-coded status messages and icons - Enhance branding.py with new formatting functions: - cx_box, cx_status_box for boxed output - cx_table, cx_package_table for formatted tables - cx_success, cx_error, cx_warning, cx_info status messages - cx_divider for section separators - Add 56 comprehensive tests in test_output_formatting.py Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
mikejmorgan-ai
force-pushed
the
feature/issue-242-rich-output-formatting
branch
from
7e63882 to
57d701f
Compare
|
Summary
output_formatter.pywith comprehensive Rich formatting utilitiesbranding.pywith status boxes, tables, dividers, and colored status messagesChanges
format_box(),format_status_box()for framed contentformat_table(),format_package_table()for tabular displayProgressTracker,MultiStepProgressclasses with spinners and barscx_success/error/warning/info()with color-coded iconsExample Output
Test plan
Fixes #242
🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes
New Features
Tests
✏️ Tip: You can customize this high-level summary in your review settings.