Code Quality

Code Quality - Principles & Commands

This document covers code quality principles, analysis dimensions, and CLI commands for maintaining code quality in this Python project.

Quick Command (Run After Every Edit)

# Auto-fix linting + format + type check
.venv/bin/ruff check . --fix && .venv/bin/ruff format . && .venv/bin/pyright

Code Quality Principles

Core Principles

1. Context-driven analysis - Always gather project standards before analyzing code
2. Comprehensive element mapping - Outline ALL code elements (functions, classes, variables, imports)
3. Multi-dimensional quality assessment - Evaluate across 11 quality dimensions
4. Evidence-based findings - Every quality issue must have concrete code examples with file:line references
5. Project standards first - Prioritize project conventions over generic best practices
6. Security awareness - Always check for OWASP Top 10 vulnerabilities
7. Actionable recommendations - Every suggestion must be specific with exact locations
8. Minimum quality target - Maintain 9.1/10 quality score across all dimensions

Quality Dimensions (11 Total)

Score each dimension 1-10:

Target: 9.1/10 weighted average

Code Quality Analysis Checklist

Phase 1: Context Gathering

Before analyzing code, gather project context:

• Read CLAUDE.md and .claude/CLAUDE.md for project-specific instructions
• Read README.md for project overview
• Check .claude/skills/*.md for project patterns
• Find files that import the target (consumers of its API)
• Find sibling files in same directory (for pattern consistency)
• Find test files for the target

Phase 2: Code Element Extraction

Catalog ALL code elements:

Imports

• Standard library, third-party, local imports
• Mark used/unused status with line numbers

Globals/Constants

• Naming convention (SCREAMING_CASE for constants)
• Mutable global warnings

Classes

• Base classes, decorators
• Class variables with visibility
• Instance variables from constructor
• Methods with visibility and call relationships

Functions

• Visibility (public/private via underscore prefix)
• Parameters with types and defaults
• Return types and paths
• Local variables

Type Definitions

• Type aliases, generics, protocols
• Data classes, enums

Phase 3: Scope & Visibility Analysis

Private Elements

For every element marked private (underscore prefix):

• Verify only used within its class/module
• Flag external access as violations

Public Elements

For every public element:

• Track internal and external usage
• Recommend making private if not used externally

Unused Elements

• Imports never referenced
• Variables assigned but not read
• Functions/methods never called
• Parameters never used
• Types only in their own definition

Phase 4: Call Hierarchy Mapping

• Build call graph within file
• Identify entry points (not called by anything in file)
• Identify internal-only functions (called but not entry points)
• Flag orphaned code (defined but never called)
• Detect recursive and circular calls

Code Smell Detection

Complexity Issues

• Functions > 50 lines
• Cyclomatic complexity > 10
• Cognitive complexity > 15
• Nesting depth > 4
• Too many parameters (> 5)
• Too many return statements

Cognitive Complexity Thresholds

Design Issues

• God class (too many responsibilities)
• Feature envy (method uses other class more than its own)
• Data class (only getters/setters, no behavior)
• Inappropriate intimacy (excessive coupling)
• Primitive obsession (overuse of primitives instead of objects)
• Long parameter list (> 3-4 params without object grouping)

Naming Issues

• Single-letter names (except i,j,k in loops)
• Misleading names (name doesn't match behavior)
• Inconsistent naming style
• Names too similar

Duplication

• Duplicate code blocks (>5 lines similar)
• Copy-paste patterns
• Logic that should be extracted

Redundant Logic

• Redundant conditionals (identical branches)
• Unnecessary conditional expressions
• Magic numbers that should be constants

SOLID Principles Violations

Single Responsibility (SRP)

• Classes with multiple reasons to change
• Functions doing more than one thing
• Mixed abstraction levels in same function

Open/Closed (OCP)

• Classes requiring modification for extension
• Switch/if-else chains that grow with new types
• Missing strategy/plugin patterns

Liskov Substitution (LSP)

• Subclasses that change parent behavior unexpectedly
• Overridden methods with different contracts
• Type checks for specific subclasses

Interface Segregation (ISP)

• Large interfaces forcing unused implementations
• Classes implementing methods they don't need

Dependency Inversion (DIP)

• High-level modules depending on low-level details
• Missing abstractions/interfaces
• Concrete class instantiation in business logic

DRY/KISS/YAGNI Violations

DRY (Don't Repeat Yourself)

• Duplicate code blocks (>3 lines similar)
• Copy-paste logic that should be extracted
• Repeated magic numbers/strings
• Similar functions that could be parameterized

KISS (Keep It Simple, Stupid)

• Over-engineered solutions
• Unnecessary abstractions
• Premature optimization
• Complex one-liners that should be expanded

YAGNI (You Aren't Gonna Need It)

• Unused parameters kept "for future use"
• Dead feature flags
• Speculative generality
• Commented-out code blocks

Best Practices

Python Language Idioms

• Using concrete type checks instead of duck typing
• Mutable default arguments
• Catching all exceptions without specificity
• Not using context managers (with statements)
• Inefficient string building in loops

Modern Python Features

• Could use pattern matching (Python 3.10+)
• Could use union types (X | Y instead of Union[X, Y])
• Old-style formatting instead of f-strings
• Not using dataclasses where appropriate

Error Handling

• Catching too broad exceptions (bare except, Exception)
• Empty except blocks
• Not re-raising when appropriate
• Missing exception chaining (from clause)

Resource Management

• File handles not closed properly (missing with)
• Database connections not closed
• Missing cleanup in finally blocks

Security Patterns (OWASP-Aligned)

Injection Vulnerabilities

• SQL injection risks (string concatenation in queries)
• Command injection (shell execution with unsanitized input)
• Path traversal (unsanitized file paths)

Authentication & Session

• Hardcoded credentials/secrets
• Weak password handling
• Missing authentication checks

Data Exposure

• Sensitive data in logs
• Secrets in source code
• Unencrypted sensitive data

Input Validation

• Missing input validation at entry points
• Insufficient output encoding
• Regex DoS (ReDoS) patterns

Dangerous Functions

• Dynamic code evaluation (eval, exec)
• Unsafe deserialization (pickle with untrusted data)

Performance & Efficiency

Memory Management

• Memory leaks from objects not released
• Excessive allocation in loops
• Large objects copied instead of referenced
• Unbounded caches without eviction

Algorithm Efficiency

• O(n^2) or worse where O(n log n) possible
• Redundant computations that could be cached
• Nested loops that could be optimized

Database & I/O

• N+1 query problems (queries in loops)
• Excessive database roundtrips
• Large file operations without streaming
• Synchronous I/O blocking main thread

Concurrency & Thread Safety

• Shared mutable state without synchronization
• Race conditions on shared variables
• Non-atomic operations on shared data
• Promises/async not handled properly
• Missing error handling in async code

Advanced Metrics

Halstead Complexity

ABC Metrics (Assignment, Branch, Condition)

• A (Assignment Count): Variable assignments, mutations
• B (Branch Count): Function calls, method invocations
• C (Condition Count): if/else, switch, ternary, boolean logic
• Magnitude: sqrt(A^2 + B^2 + C^2) - Thresholds: <20 simple, 20-50 moderate, >50 complex

Coupling Metrics

• CBO (Coupling Between Objects): 0-5 low, 6-10 moderate, >10 high
• Ce (Efferent coupling): Classes this class depends on
• Ca (Afferent coupling): Classes that depend on this class
• LCOM (Lack of Cohesion): 0-20% cohesive, 20-50% moderate, >50% should split

Maintainability Index

Technical Debt Categories

• Code debt: Poor quality code (estimated hours to fix)
• Design debt: Architectural issues
• Test debt: Missing/inadequate tests
• Documentation debt: Missing/outdated docs

Priority Matrix: Impact (High/Med/Low) x Effort (High/Med/Low) = Priority (P1-P4)

Project Standards Compliance

Check Against Project Docs

• Required documentation format followed
• All public functions documented
• Type hints complete per project requirements
• Function naming matches project style
• Error handling follows project pattern
• Logging follows project pattern

Cross-File Consistency

• Uses same patterns as imported modules
• API matches consumer usage patterns
• Structure matches similar sibling files
• Test patterns match project conventions

CLI Commands

Individual Commands

Linting

# Check for issues
.venv/bin/ruff check .
.venv/bin/ruff check agent/
.venv/bin/ruff check tui/

# Auto-fix issues
.venv/bin/ruff check . --fix
.venv/bin/ruff check agent/ --fix

# Verify clean (no fixes)
.venv/bin/ruff check .

Formatting

# Format all files
.venv/bin/ruff format .

# Format specific directory
.venv/bin/ruff format agent/
.venv/bin/ruff format tui/

What ruff format does:

• Breaks long lines (120 char limit)
• Fixes indentation/spacing
• Enforces double quotes
• Adds/removes trailing commas

Type Checking

# Check all files
.venv/bin/pyright

# Check specific file
.venv/bin/pyright agent/agent.py
.venv/bin/pyright tui/app.py

Targeted Rule Checking

# Find unused imports (F401), variables (F841), arguments (ARG)
.venv/bin/ruff check . --select F401,F841,ARG

# Auto-fix unused imports
.venv/bin/ruff check . --select F401 --fix

# Check import sorting only
.venv/bin/ruff check . --select I --fix

# Check for common bugs (flake8-bugbear)
.venv/bin/ruff check . --select B

# Check naming conventions
.venv/bin/ruff check . --select N

Dead Code Detection

Quick Dead Code Scan

# Find all dead code (unused imports, variables, arguments)
.venv/bin/ruff check . --select F401,F841,ARG

# Auto-fix safely removable dead code (unused imports only)
.venv/bin/ruff check . --select F401 --fix

Ruff Dead Code Rules

When to Use --fix

Safe to auto-fix:

• F401 (unused imports): Always safe, removes clutter

Review before fixing:

• F841 (unused variables): May indicate incomplete implementation
• ARG* (unused arguments): May be required for API compatibility

# Preview what --fix would change
.venv/bin/ruff check . --select F841,ARG --fix --diff

# Fix only after reviewing the diff
.venv/bin/ruff check . --select F841 --fix

Optional: Vulture for Deeper Analysis

# Install vulture (if not already installed)
pip install vulture

# Run vulture on the codebase
vulture agent/ tui/ common/

# With confidence threshold (higher = fewer false positives)
vulture agent/ tui/ common/ --min-confidence 80

Preview Changes (Diff Mode)

# Show diff of all auto-fixes
.venv/bin/ruff check . --fix --diff

# Preview formatting changes
.venv/bin/ruff format . --diff

# Check specific file changes
.venv/bin/ruff check agent/hitl.py --fix --diff

Before Committing

# Complete pre-commit workflow
.venv/bin/ruff check . --fix && \
.venv/bin/ruff format . && \
.venv/bin/ruff check . && \
.venv/bin/pyright

If all checks pass, then commit:

git add .
git commit -m "Your message"

Understanding Output

Ruff Output

Clean:

All checks passed!

Issues found:

agent/agent.py:136:5: SIM108 Use ternary operator instead of if-else-block
Found 4 errors (3 fixable).

• Line format: file:line:col: CODE Description
• 3 fixable = can auto-fix with --fix

Pyright Output

Shows type errors that must be fixed:

• Missing type annotations
• None type errors
• Import errors
• Type mismatches

Configuration Files

Ruff: ruff.toml

• Line length: 120
• Linting rules enabled
• CLI only (no IDE extension)

Pyright: pyrightconfig.json

• Type checking rules
• Used by both IDE and CLI

Pylint: .pylintrc

• IDE warnings only
• Configured to match Ruff (120 char)
• Not required for commits

Enabled Rule Categories

Based on ruff.toml, these rule sets are active:

• E/W - pycodestyle (PEP 8 style violations)
• F - Pyflakes (unused imports, variables, undefined names)
• I - isort (import sorting/organization)
• N - pep8-naming (naming conventions)
• UP - pyupgrade (modernize Python syntax)
• B - flake8-bugbear (likely bugs and design problems)
• C4 - flake8-comprehensions (better list/dict comprehensions)
• SIM - flake8-simplify (code simplification suggestions)

Troubleshooting

"Command not found: ruff"

# Use full path to venv
.venv/bin/ruff check .

"Import could not be resolved"

Reload IDE (Cmd+Shift+P -> "Reload Window")

Type errors

Fix by:

• Adding type annotations
• Checking for None before accessing
• Using hasattr() for attribute checks
• Adding type guards

Long line warnings from Pylint

# Auto-fix with ruff format
.venv/bin/ruff format .

Quick Status Check

# Check if tools are available
which .venv/bin/ruff
which .venv/bin/pyright

# Verify versions
.venv/bin/ruff --version
.venv/bin/pyright --version

Reference

• Ruff docs: https://docs.astral.sh/ruff/
• Ruff rules: https://docs.astral.sh/ruff/rules/
• Pyright docs: https://github.com/microsoft/pyright