refactor: consolidated code quality improvements - constants, lazy init, and documentation

[echobt]

Summary

This PR consolidates 4 refactoring PRs into a single, cohesive change.

Included PRs:

• refactor: Centralize timeout constants #27: Centralize timeout constants
• refactor: replace magic numbers with documented named constants #43: Replace magic numbers with documented named constants
• refactor(config): use LazyLock for static regex initialization #67: Use LazyLock for static regex initialization
• refactor(tui): extract subagent timeout values as named constants #68: Extract subagent timeout values as named constants

Key Changes:

• Created centralized cortex_common::timeout module with documented constants
• Replaced scattered magic numbers throughout codebase with well-documented constants
• Improved static initialization using LazyLock for regex patterns
• Added comprehensive documentation for the timeout hierarchy across Cortex services

Files Modified:

• src/cortex-common/src/timeout.rs (new)
• src/cortex-common/src/config_substitution.rs
• src/cortex-app-server/src/auth.rs
• src/cortex-app-server/src/streaming.rs
• src/cortex-engine/src/tools/handlers/batch.rs
• src/cortex-engine/src/tools/handlers/grep.rs
• src/cortex-engine/src/tools/unified_executor.rs
• src/cortex-exec/src/runner.rs
• src/cortex-tui/src/runner/event_loop/subagent.rs
• src/cortex-tui/src/runner/event_loop/streaming.rs

Closes #27, #43, #67, #68

[greptile-apps]

Greptile Overview

Greptile Summary

This PR consolidates four refactoring efforts into a cohesive code quality improvement that centralizes timeout constants, replaces magic numbers, and optimizes static initialization patterns.

Key Improvements

• Centralized timeout module: created cortex_common::timeout with 7 well-documented timeout constants that establish a clear hierarchy across the codebase
• LazyLock optimization: migrated regex patterns in config_substitution.rs from per-instance compilation to static LazyLock initialization, improving performance and reducing memory overhead
• Magic number elimination: replaced hardcoded timeout values across TUI streaming and subagent modules with named constants
• Enhanced documentation: added comprehensive timeout hierarchy table in http_client.rs documenting the relationship between all timeout values
• Code cleanup: removed unused fields (base_dir in SessionStorage, replace_all in SearchReplace) and dead code (reconnection methods in MCP transport)
• Safer error handling: replaced unwrap() with unwrap_or_default() in time-related operations

Issues Found

• Unused parameter in batch tool: timeout_secs parameter in BatchToolArgs is defined but never used after the refactor introduced per-tool timeouts via tool_timeout_secs. This creates API confusion and should be removed from both the struct and tool definition schema.

Confidence Score: 4/5

• Safe to merge after addressing the unused timeout_secs parameter in batch tool
• Excellent refactoring with clear improvements to code quality, maintainability, and performance. The LazyLock pattern is correctly implemented, timeout constants are well-documented with tests, and dead code removal is appropriate. However, the batch tool has an unused parameter that could confuse users and should be cleaned up before merge.
• src/cortex-engine/src/tools/handlers/batch.rs requires cleanup to remove unused timeout_secs parameter

Important Files Changed

Filename	Overview
src/cortex-common/src/timeout.rs	new centralized timeout module with well-documented constants and unit tests
src/cortex-common/src/config_substitution.rs	replaced instance regex with static LazyLock patterns for better performance
src/cortex-common/src/http_client.rs	added comprehensive timeout hierarchy documentation table
src/cortex-engine/src/tools/handlers/batch.rs	added tool_timeout_secs parameter for individual tool timeouts, but timeout_secs is now unused
src/cortex-tui/src/runner/event_loop/streaming.rs	replaced magic numbers with named constants for streaming timeouts
src/cortex-tui/src/runner/event_loop/subagent.rs	replaced magic numbers with named constants for subagent timeouts

Sequence Diagram

sequenceDiagram
    participant Client as Client/Tool Executor
    participant Batch as BatchToolHandler
    participant Tools as Individual Tools
    participant Timeout as timeout module
    
    Note over Client,Timeout: Centralized Timeout Architecture
    
    Client->>Timeout: Import timeout constants
    Timeout-->>Client: DEFAULT_BATCH_TIMEOUT_SECS (300s)
    
    Client->>Batch: execute(BatchToolArgs)
    Note over Batch: Parse tool_timeout_secs<br/>(default: 60s)
    
    par Parallel Tool Execution
        Batch->>Tools: execute_tool_1() with timeout
        Batch->>Tools: execute_tool_2() with timeout
        Batch->>Tools: execute_tool_N() with timeout
    end
    
    alt Tool completes in time
        Tools-->>Batch: Success result
    else Tool times out after 60s
        Tools-->>Batch: Timeout error
    end
    
    Batch-->>Client: BatchResult (aggregated)
    
    Note over Client,Timeout: LazyLock Pattern for Regex
    
    Client->>cortex_common: config_substitution
    activate cortex_common
    Note over cortex_common: ENV_REGEX & FILE_REGEX<br/>initialized once via LazyLock
    cortex_common-->>Client: Compiled patterns (shared)
    deactivate cortex_common

Loading

[greptile-apps]

_{6 files reviewed, 2 comments}

_{Edit Code Review Agent Settings | Greptile}

[greptile-apps]

timeout_secs parameter is no longer used after switching to per-tool timeouts. The code now only uses tool_timeout_secs (line 341), making this parameter obsolete.

Suggested change

	/// Optional timeout in seconds for the entire batch (default: 300s).
	#[serde(default)]
	pub timeout_secs: Option<u64>,
	/// Optional timeout in seconds for individual tools (default: 60s).
	/// This prevents a single tool from consuming the entire batch timeout.

Prompt To Fix With AI

This is a comment left during a code review.
Path: src/cortex-engine/src/tools/handlers/batch.rs
Line: 46:48

Comment:
`timeout_secs` parameter is no longer used after switching to per-tool timeouts. The code now only uses `tool_timeout_secs` (line 341), making this parameter obsolete.

```suggestion
    /// Optional timeout in seconds for individual tools (default: 60s).
    /// This prevents a single tool from consuming the entire batch timeout.
```

How can I resolve this? If you propose a fix, please make it concise.

[greptile-apps]

Additional Comments (1)

src/cortex-engine/src/tools/handlers/batch.rs
timeout_secs is no longer used in the code and should be removed from the tool definition schema to avoid confusion.

                "tool_timeout_secs": {
                    "type": "integer",
                    "description": "Optional timeout in seconds for individual tool execution (default: 60). Prevents a single tool from consuming the entire batch timeout.",
                    "minimum": 1,
                    "maximum": 300

Prompt To Fix With AI

This is a comment left during a code review.
Path: src/cortex-engine/src/tools/handlers/batch.rs
Line: 391:396

Comment:
`timeout_secs` is no longer used in the code and should be removed from the tool definition schema to avoid confusion.

```suggestion
                "tool_timeout_secs": {
                    "type": "integer",
                    "description": "Optional timeout in seconds for individual tool execution (default: 60). Prevents a single tool from consuming the entire batch timeout.",
                    "minimum": 1,
                    "maximum": 300
```

How can I resolve this? If you propose a fix, please make it concise.

[echobt]

Closing this PR to consolidate into a single mega-PR combining all refactoring and documentation changes.