Skip to content

πŸ” Code Quality Report - 2026-05-04Β #708

@github-actions

Description

@github-actions

Weekly Code Quality Report - 2026-05-04

This is an automated report generated by the weekly code quality workflow.

Code Quality Analysis Report

Generated: 2026-05-04
Codebase: OpenHands CLI (openhands_cli/)
Files Analyzed: 124 Python files


Executive Summary

The OpenHands CLI codebase demonstrates strong overall code quality with clean pyright output (0 errors, 0 warnings), well-structured state management using Textual's reactive patterns, and clear separation between ACP and TUI implementations.

Key findings:

  • Type Safety: ~78% type annotation coverage with no type errors
  • State Management: Excellent reactive pattern in TUI, proper persistence in stores
  • Separation of Concerns: Good modular architecture with some opportunities for improvement
  • Code Duplication: Moderate duplication between ACP and TUI that could be extracted to shared modules

Priority Areas:

  1. Extract shared tool formatting logic to reduce ACP/TUI duplication
  2. Add return type annotations to ~40 key functions
  3. Split richlog_visualizer.py into smaller focused modules

Type Checking Issues

Critical Issues

βœ… None found - Pyright analysis completed with 0 errors, 0 warnings across 237 files.

Missing Type Hints

Functions Missing Return Type Annotations (~155 total, key ones listed):

File Function Estimated Effort
tui/widgets/richlog_visualizer.py _update_widget_in_ui trivial
tui/widgets/richlog_visualizer.py _handle_observation_event trivial
tui/widgets/richlog_visualizer.py _build_observation_content trivial
tui/widgets/richlog_visualizer.py _truncate_for_display trivial
tui/widgets/richlog_visualizer.py _make_collapsible trivial
tui/widgets/richlog_visualizer.py _create_system_prompt_collapsible trivial
tui/widgets/richlog_visualizer.py _create_titled_collapsible trivial
tui/core/conversation_runner.py process_message_async trivial
tui/core/conversation_runner.py _execute_conversation trivial
tui/core/runner_factory.py create trivial
tui/core/state.py attach_conversation_state trivial
tui/widgets/main_display.py watch_conversation_id trivial
tui/widgets/splash.py watch_loaded_resources trivial
tui/widgets/splash.py watch_conversation_id trivial
tui/panels/history_side_panel.py toggle trivial
tui/panels/history_side_panel.py update_conversation_title_if_needed trivial
tui/panels/mcp_side_panel.py _format_server_details trivial
tui/panels/mcp_side_panel.py _server_spec_to_dict trivial
tui/utils/critic/refinement.py get_high_probability_issues small
tui/utils/critic/refinement.py build_refinement_message small
tui/utils/critic/refinement.py should_trigger_refinement small
tui/utils/critic/feedback.py send_critic_inference_event trivial
tui/content/splash.py get_splash_content trivial
tui/content/resources.py collect_loaded_resources small
argparsers/auth_parser.py add_logout_parser trivial
argparsers/util.py add_confirmation_mode_args trivial
tui/modals/settings/utils.py save_settings small

Textual Watcher Methods (convention-based, lower priority):

  • Most watch_* methods lack return types (Textual convention allows this)
  • Consider adding -> None for consistency

Type Improvement Opportunities

Uses of Any that could be more specific:

File Location Current Suggested
tui/utils/critic/refinement.py _format_feature_for_prompt dict[str, Any] Create CriticFeature TypedDict
tui/utils/critic/refinement.py get_high_probability_issues return list[dict[str, Any]] list[CriticFeature]
tui/utils/critic/visualization.py categorized param dict[str, Any] Create CategorizedFeatures TypedDict
tui/panels/mcp_side_panel.py server param dict[str, Any] Use existing `StdioMCPServer
mcp/mcp_utils.py get_config_status return dict[str, Any] Create MCPConfigStatus TypedDict
setup.py mcp_servers param dict[str, dict[str, Any]] Create MCPServerConfig TypedDict

State Management Issues

Current State Architecture

The codebase uses a well-designed reactive state pattern:

β”Œβ”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”
β”‚                    TUI State Management                      β”‚
β”œβ”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€
β”‚  ConversationContainer (reactive var() properties)          β”‚
β”‚    β”œβ”€β”€ running: bool                                         β”‚
β”‚    β”œβ”€β”€ conversation_id: UUID | None                          β”‚
β”‚    β”œβ”€β”€ confirmation_policy: ConfirmationPolicyBase           β”‚
β”‚    β”œβ”€β”€ pending_action_count: int                             β”‚
β”‚    β”œβ”€β”€ metrics: Metrics | None                               β”‚
β”‚    └── ... (data_bind to UI widgets)                         β”‚
β”œβ”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€
β”‚  ConversationManager (message router)                        β”‚
β”‚    β”œβ”€β”€ RunnerRegistry (runner lifecycle)                     β”‚
β”‚    └── Controllers (business logic)                          β”‚
β”œβ”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€
β”‚  AgentStore (file-based persistence)                         β”‚
β”‚    └── agent_settings.json                                   β”‚
β”œβ”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€
β”‚  CliSettings (Pydantic model)                                β”‚
β”‚    └── ~/.openhands/cli_settings.json                        β”‚
β””β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”˜

Problems Identified

  1. Module-level Singleton (Low severity)

    • Location: openhands_cli/theme.py:31
    • Code: OPENHANDS_THEME = create_openhands_theme()
    • Impact: Theme is created at import time, no way to override for testing
    • Risk: Low (theme is read-only configuration)
  2. Scattered Constants (Very low severity)

    • Constants like SUCCESS_ICON, ERROR_ICON, MAX_LINE_LENGTH are defined in individual modules
    • Not a problem (they're immutable), but could be centralized for consistency
  3. ACP Session State Management

    • Location: openhands_cli/acp_impl/agent/base_agent.py
    • Uses _active_sessions: dict[str, BaseConversation] and _running_tasks: dict[str, asyncio.Task]
    • Pattern: Dictionary-based tracking
    • Assessment: Appropriate for ACP's async model

Recommendations

  1. βœ… No critical state management issues found
  2. Consider extracting visual constants to openhands_cli/constants.py (optional)
  3. Theme singleton is acceptable - testing can mock the module

Separation of Concerns Issues

Well-Separated Components βœ…

  1. Auth Module (openhands_cli/auth/)

    • Clean layered architecture
    • token_storage.py β†’ http_client.py β†’ api_client.py β†’ device_flow.py β†’ commands
    • No mixing of concerns
  2. TUI Core (openhands_cli/tui/core/)

    • Controller pattern with single responsibilities
    • ConversationContainer (state) vs ConversationManager (operations)
    • Clear message-based communication
  3. Stores (openhands_cli/stores/)

    • AgentStore handles agent configuration lifecycle
    • CliSettings uses Pydantic for validation
    • Clean persistence abstraction

Coupled Components

  1. richlog_visualizer.py (805 lines) - Needs refactoring

    Responsibility Lines Suggested Extraction
    Event handling ~150 event_handlers.py
    Widget creation ~200 Keep in visualizer
    Title/content formatting ~250 shared/tool_formatting.py
    Critic handling ~100 critic_handler.py
    Agent name formatting ~80 shared/agent_formatting.py
    Observation pairing ~100 observation_tracker.py
  2. entrypoint.py (200 lines) - Mixed concerns

    Currently handles:

    • Argument parsing (delegated to argparsers βœ…)
    • Resume logic (handle_resume_logic)
    • Command dispatch (serve, web, acp, login, logout, mcp, cloud, view, default)
    • Error handling

    Suggestion: Consider a command pattern for better organization

Mixed Responsibilities

File Issue Suggested Fix
tui/widgets/richlog_visualizer.py Handles critic display AND event formatting Extract critic handling to tui/utils/critic/handler.py
entrypoint.py Contains resume logic that could be shared Move handle_resume_logic to conversations/utils.py
acp_impl/slash_commands.py Contains both parsing AND response formatting Keep together (cohesive unit)

Code Duplication (ACP/TUI)

Duplicated Patterns

1. Tool Title Formatting

ACP (acp_impl/events/utils.py:140-180):

def get_tool_title(tool_name: str, *, action: Action | None = None, summary: str | None = None) -> str:
    if isinstance(action, FileEditorAction):
        op = "Reading" if action.command == "view" else "Editing"
        path = action.path or ""
        if clean_summary:
            return f"{clean_summary}: {op} {path}"
        return f"{op} {path}"

    if isinstance(action, TerminalAction):
        cmd = action.command.strip().replace("\n", " ") if action.command else ""
        if clean_summary:
            return f"{clean_summary}: $ {cmd}"
        return f"$ {cmd}" if cmd else tool_name
    # ...

TUI (tui/widgets/richlog_visualizer.py:460-510):

def _build_action_title(self, event: ActionEvent) -> str:
    if isinstance(action, TerminalAction) and action.command:
        cmd = self._escape_rich_markup(action.command.strip().replace("\n", " "))
        cmd = self._truncate_for_display(cmd)
        if summary:
            return f"{agent_prefix}[bold]{summary}[/bold][dim]: $ {cmd}[/dim]"
        return f"{agent_prefix}[dim]$ {cmd}[/dim]"

    elif isinstance(action, FileEditorAction) and action.path:
        op = "Reading" if action.command == "view" else "Editing"
        path = self._escape_rich_markup(action.path)
        if summary:
            return f"{agent_prefix}[bold]{summary}[/bold][dim]: {op} {path}[/dim]"
        return f"{agent_prefix}[bold]{op}[/bold][dim] {path}[/dim]"
    # ...

Duplication Level: HIGH - Same logic with different formatting (plain vs Rich markup)

2. Confirmation Mode Definitions

ACP (acp_impl/confirmation.py:29-48):

CONFIRMATION_MODES: dict[ConfirmationMode, dict[str, str]] = {
    "always-ask": {
        "short": "Ask for permission before every action",
        "long": "Agent will ask for permission before executing every action.",
    },
    "always-approve": {
        "short": "Automatically approve all actions",
        "long": "Agent will automatically approve all actions...",
    },
    "llm-approve": {
        "short": "Use LLM security analyzer to auto-approve safe actions",
        "long": "Agent will use LLM security analyzer...",
    },
}

TUI: Uses SDK policy classes (AlwaysConfirm, NeverConfirm, ConfirmRisky) directly with separate descriptions in various places.

Duplication Level: MEDIUM - Conceptually similar, different representations

3. Slash Command Help Text

ACP (acp_impl/slash_commands.py:55-62):

def create_help_text() -> str:
    commands = get_available_slash_commands()
    lines = ["Available slash commands:", ""]
    for cmd in commands:
        lines.append(f"  /{cmd.name} - {cmd.description}")
    return "\n".join(lines)

TUI (tui/core/commands.py:18-29):

COMMANDS = [
    DropdownItem(main="/help - Display available commands"),
    DropdownItem(main="/new - Start a new conversation"),
    DropdownItem(main="/history - Toggle conversation history"),
    # ...
]

Duplication Level: LOW - Different data structures for different purposes (ACP protocol vs Textual UI)

Shared Code Opportunities

Opportunity Estimated Effort Impact
Tool title formatting core logic medium HIGH - Reduce 100+ lines of duplication
Create shared/tool_formatting.py with format functions
ACP and TUI call shared logic with their own wrappers
Command definitions small MEDIUM - Single source of truth
Create shared/commands.py with command metadata
ACP converts to AvailableCommand, TUI to DropdownItem
Confirmation mode metadata small LOW - Improves consistency
Create shared/confirmation_modes.py

Recommendations

  1. Create openhands_cli/shared/tool_formatting.py:

    def format_terminal_title(command: str, summary: str | None = None) -> tuple[str, str]:
        """Returns (summary_part, detail_part) for both ACP and TUI to format."""
        clean_cmd = command.strip().replace("\n", " ")
        return (summary or "", f"$ {clean_cmd}")
    
    def format_file_editor_title(action: FileEditorAction, summary: str | None = None) -> tuple[str, str]:
        """Returns (summary_part, detail_part) for both ACP and TUI to format."""
        op = "Reading" if action.command == "view" else "Editing"
        return (summary or "", f"{op} {action.path or ''}")
  2. Create openhands_cli/shared/commands.py:

    @dataclass
    class CommandDef:
        name: str
        description: str
        hint: str = ""
    
    SLASH_COMMANDS = [
        CommandDef("help", "Display available commands"),
        CommandDef("new", "Start a new conversation"),
        # ...
    ]

Low-Hanging Fruit

Trivial Fixes (< 5 minutes each)

# Fix File Impact
1 Add -> None return type tui/widgets/richlog_visualizer.py:_update_widget_in_ui Type safety
2 Add -> bool return type tui/widgets/richlog_visualizer.py:_handle_observation_event Type safety
3 Add -> str return type tui/widgets/richlog_visualizer.py:_build_observation_content Type safety
4 Add -> str return type tui/widgets/richlog_visualizer.py:_truncate_for_display Type safety
5 Add -> Collapsible return type tui/widgets/richlog_visualizer.py:_make_collapsible Type safety
6 Add -> None to argparser functions argparsers/util.py, argparsers/auth_parser.py Type safety
7 Add -> None to send_critic_inference_event tui/utils/critic/feedback.py Type safety

Small Fixes (< 30 minutes each)

# Fix File Impact
1 Create CriticFeature TypedDict tui/utils/critic/refinement.py Replace dict[str, Any]
2 Add -> None to all Textual watch_* methods Multiple files Consistency
3 Extract _format_agent_name to shared tui/widgets/richlog_visualizer.py Reusability
4 Add return types to collect_loaded_resources tui/content/resources.py Type safety
5 Add return types to save_settings tui/modals/settings/utils.py Type safety

Medium Fixes (1-2 hours each)

# Fix File(s) Impact
1 Extract tool formatting to shared module acp_impl/events/utils.py, tui/widgets/richlog_visualizer.py Reduce duplication
2 Split richlog_visualizer.py into focused modules tui/widgets/ Maintainability
3 Create TypedDict for MCP config structures mcp/mcp_utils.py, setup.py Type safety
4 Create shared command definitions acp_impl/slash_commands.py, tui/core/commands.py Single source of truth

Statistics

Type Annotation Coverage

Category Count Percentage
Functions with return types 566 78.5%
Functions missing return types 155 21.5%
Total functions 721 100%

Files by Directory

Directory Files Purpose
tui/ 49 Terminal UI (Textual)
acp_impl/ 17 ACP protocol implementation
auth/ 7 Authentication
stores/ 3 Data persistence
shared/ 4 Shared utilities
mcp/ 5 MCP integration
argparsers/ 8 CLI argument parsing
conversations/ 8 Conversation management
Other 23 Entry points, utils, etc.
Total 124

Code Duplication Summary

Pattern ACP TUI Severity
Tool title formatting βœ“ βœ“ HIGH
Confirmation mode definitions βœ“ βœ“ MEDIUM
Slash command definitions βœ“ βœ“ LOW
Delegate title formatting βœ“ (shared) βœ“ (shared) βœ… RESOLVED
Slash command parsing βœ“ (shared) βœ“ (shared) βœ… RESOLVED
Conversation summary extraction βœ“ (shared) βœ“ (shared) βœ… RESOLVED

Issues by Category

Category Critical High Medium Low Total
Type Checking 0 0 5 27 32
State Management 0 0 0 2 2
Separation of Concerns 0 1 2 0 3
Code Duplication 0 1 1 1 3
Total 0 2 8 30 40

Prioritized Action Items

P0 - High Impact, Low Effort (Do First)

  1. ☐ Add return type annotations to ~7 key functions in richlog_visualizer.py (trivial)
  2. ☐ Add return type annotations to argparser utility functions (trivial)
  3. ☐ Add return type annotations to critic utility functions (trivial)

P1 - High Impact, Medium Effort (Plan Soon)

  1. ☐ Extract tool title formatting to shared/tool_formatting.py (medium)

    • Reduces ~100 lines of duplication
    • Single source of truth for formatting logic
    • ACP and TUI use different wrappers for their output formats
  2. ☐ Create CriticFeature and related TypedDicts (small)

    • Improves type safety in critic visualization code
    • Self-documents the expected structure

P2 - Medium Impact, Medium Effort (Backlog)

  1. ☐ Split richlog_visualizer.py into focused modules
  2. ☐ Create shared command definitions module
  3. ☐ Add TypedDicts for MCP configuration structures
  4. ☐ Add -> None to all Textual watcher methods for consistency

P3 - Low Impact, Low Effort (When Convenient)

  1. ☐ Centralize visual constants to constants.py
  2. ☐ Extract handle_resume_logic to conversations module
  3. ☐ Document the TypedDict patterns in AGENTS.md

Appendix: Key Files Reference

State Management

  • openhands_cli/tui/core/state.py - Reactive state container
  • openhands_cli/stores/agent_store.py - Agent persistence
  • openhands_cli/stores/cli_settings.py - CLI configuration

Event Handling

  • openhands_cli/acp_impl/events/shared_event_handler.py - ACP event handling
  • openhands_cli/tui/widgets/richlog_visualizer.py - TUI event visualization

Shared Code

  • openhands_cli/shared/slash_commands.py - Slash command parsing
  • openhands_cli/shared/conversation_summary.py - Summary extraction
  • openhands_cli/shared/delegate_formatter.py - Delegate title formatting

Workflow Run: View Details

Categories Analyzed:

  • πŸ” Type checking and type hints
  • πŸ“¦ State management patterns
  • πŸ”€ Separation of concerns
  • πŸ” Code duplication between ACP and TUI

Next Steps:

  • Review the findings above
  • PRs for low-hanging fruit may be automatically created
  • Create specific issues for larger refactoring items

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions