Skip to content

docs: Add comprehensive timeout configuration documentation#49

Closed
echobt wants to merge 2 commits intomainfrom
docs/standardize-timeout-documentation
Closed

docs: Add comprehensive timeout configuration documentation#49
echobt wants to merge 2 commits intomainfrom
docs/standardize-timeout-documentation

Conversation

@echobt
Copy link
Contributor

@echobt echobt commented Feb 4, 2026

Summary

This PR adds comprehensive documentation for timeout configuration across the Cortex codebase. It documents the timeout hierarchy, explains when each timeout applies, and provides guidance on which constants to use for consistency.

Changes

  • Add detailed timeout hierarchy documentation in cortex-common/http_client.rs
  • Document all timeout constants with their purpose and recommended usage
  • Add inline comments referencing standard timeout values in config files
  • Include a table showing which timeout applies to each use case

Timeout Hierarchy

Use Case Module Constant Value Rationale
Health checks cortex-common/http_client HEALTH_CHECK_TIMEOUT 5s Quick validation
Standard HTTP requests cortex-common/http_client DEFAULT_TIMEOUT 30s Normal API calls
Connection pool idle cortex-common/http_client POOL_IDLE_TIMEOUT 60s DNS refresh
LLM Request (non-streaming) cortex-exec/runner DEFAULT_REQUEST_TIMEOUT_SECS 120s Model inference
LLM Streaming total cortex-common/http_client STREAMING_TIMEOUT 300s Long-running streams
Server request lifecycle cortex-app-server/config request_timeout 300s Full request handling
Per-chunk reads cortex-app-server/config read_timeout 30s Chunk timeout
Graceful shutdown cortex-app-server/config shutdown_timeout 30s Time for cleanup
Entire execution cortex-exec/runner DEFAULT_TIMEOUT_SECS 600s Headless exec limit

Verification

cargo check -p cortex-common
cargo doc -p cortex-common --no-deps

@greptile-apps
Copy link

greptile-apps bot commented Feb 4, 2026

Greptile Overview

Greptile Summary

This PR adds comprehensive documentation for timeout configuration across the Cortex codebase. The changes are purely documentation-focused with some dead code cleanup.

Documentation additions:

  • Added extensive timeout hierarchy table in cortex-common/src/http_client.rs covering all timeout constants across the system (5s to 600s)
  • Added cross-references from timeout-related fields in cortex-app-server/config.rs and cortex-exec/runner.rs pointing to the central documentation
  • Provided clear guidance on when to use each timeout constant and recommendations for adding new timeouts

Code cleanup (unrelated to docs):

  • Removed unused base_dir field from SessionStorage in storage.rs
  • Removed unused replace_all field and with_replace_all method from SearchReplace in hunk.rs
  • Removed unused reconnect and test_connection methods from transport.rs
  • Cleaned up unused imports (sleep, error from tracing)

The documentation follows a clear hierarchy from shortest (health checks: 5s) to longest (full exec session: 600s), with proper rationale for each timeout value.

Confidence Score: 5/5

  • This PR is safe to merge with no risk
  • The changes are purely additive documentation with verified dead code removal. No logic changes, no behavioral modifications. The documentation accurately reflects the existing timeout constants in the codebase, and the dead code removal was verified to have no callers.
  • No files require special attention

Important Files Changed

Filename Overview
src/cortex-common/src/http_client.rs Added comprehensive timeout hierarchy documentation as module-level doc comments
src/cortex-exec/src/runner.rs Enhanced timeout constant documentation with cross-references to central docs
src/cortex-app-server/src/config.rs Added cross-references to timeout hierarchy documentation in config fields

Sequence Diagram

sequenceDiagram
    participant User
    participant Server as cortex-app-server<br/>(request_timeout: 300s)
    participant Runner as cortex-exec<br/>(session timeout: 600s)
    participant HTTPClient as cortex-common<br/>http_client
    participant LLM as LLM API

    Note over User,LLM: Timeout Hierarchy in Action

    User->>Server: HTTP Request
    activate Server
    Note right of Server: request_timeout: 300s<br/>Full lifecycle limit

    Server->>Runner: Execute Task
    activate Runner
    Note right of Runner: DEFAULT_TIMEOUT_SECS: 600s<br/>Multi-turn session limit

    Runner->>HTTPClient: create_streaming_client()
    Note right of HTTPClient: STREAMING_TIMEOUT: 300s

    Runner->>LLM: LLM Request (via HTTPClient)
    activate LLM
    Note right of Runner: DEFAULT_REQUEST_TIMEOUT_SECS: 120s<br/>Single request timeout

    LLM-->>Runner: Streaming Response
    Note right of Server: read_timeout: 30s<br/>Per-chunk timeout
    deactivate LLM

    Runner-->>Server: Result
    deactivate Runner

    Server-->>User: HTTP Response
    deactivate Server

    Note over User,LLM: Health Check Flow (Fast Path)
    User->>HTTPClient: Health Check
    activate HTTPClient
    Note right of HTTPClient: HEALTH_CHECK_TIMEOUT: 5s<br/>Quick validation
    HTTPClient-->>User: Health Status
    deactivate HTTPClient
Loading

Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

3 files reviewed, no comments

Edit Code Review Agent Settings | Greptile

@echobt
Copy link
Contributor Author

echobt commented Feb 4, 2026

Closing to consolidate: This timeout documentation will be merged with PR #86 into a consolidated documentation PR for timeout configuration.

@echobt echobt closed this Feb 4, 2026
echobt added a commit that referenced this pull request Feb 4, 2026
## Summary

This PR consolidates **2 documentation PRs** for timeout configuration.

### Included PRs:
- #49: Add comprehensive timeout configuration documentation
- #86: Add comprehensive timeout hierarchy documentation to HTTP client

### Key Changes:
- Added detailed header documentation explaining the timeout hierarchy
- Documented each timeout constant with its purpose and use case
- Established naming conventions for constants vs config fields
- Added timeout hierarchy table for quick reference

### Timeout Hierarchy

| Use Case                    | Module                      | Constant                    | Value | Rationale |
|-----------------------------|-----------------------------|-----------------------------|-------|-----------|
| Health checks               | cortex-common/http_client   | HEALTH_CHECK_TIMEOUT        | 5s    | Quick validation |
| Standard HTTP requests      | cortex-common/http_client   | DEFAULT_TIMEOUT             | 30s   | Normal API calls |
| Connection pool idle        | cortex-common/http_client   | POOL_IDLE_TIMEOUT           | 60s   | DNS refresh |
| LLM Request (non-streaming) | cortex-exec/runner          | DEFAULT_REQUEST_TIMEOUT_SECS| 120s  | Model inference |
| LLM Streaming total         | cortex-common/http_client   | STREAMING_TIMEOUT           | 300s  | Long-running streams |
| Server request lifecycle    | cortex-app-server/config    | request_timeout             | 300s  | Full request handling |
| Per-chunk reads             | cortex-app-server/config    | read_timeout                | 30s   | Chunk timeout |
| Graceful shutdown           | cortex-app-server/config    | shutdown_timeout            | 30s   | Time for cleanup |
| Entire execution            | cortex-exec/runner          | DEFAULT_TIMEOUT_SECS        | 600s  | Headless exec limit |

### Files Modified:
- src/cortex-common/src/http_client.rs
- src/cortex-app-server/src/config.rs
- src/cortex-exec/src/runner.rs

Closes #49, #86
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant