fix(config): add validation for threshold, ratio, and token count fields

[echobt]

Summary

Fixes #5132, #5125, #5127 - Configuration field validation.

Problems

1. No bounds checking for percentage/ratio values
2. Signed types allow negative token counts

Solution

Added validation and appropriate type constraints.

Changes:

• cortex-compact: Added serde deserialize validation for threshold_percent to enforce 0.0-1.0 range
• cortex-otel: Added serde deserialize validation for sampling_ratio to enforce 0.0-1.0 range, also validates env var parsing
• cortex-engine: Changed model_context_window and model_auto_compact_token_limit from i64 to u64 to prevent negative values
• cortex-protocol: Updated TokenUsageInfo and TaskStartedEvent structs to use u64 for consistency

[greptile-apps]

Greptile Overview

Greptile Summary

PR Description Mismatch: The PR description mentions adding validation for threshold_percent, sampling_ratio, and token count fields across cortex-compact, cortex-otel, and cortex-protocol modules. However, the actual changes implement a completely different feature - a bounded ToolResponseStore for managing tool execution results.

Actual Changes: This PR adds a new bounded storage system for tool responses that addresses unbounded memory growth (#5292) and missing cleanup on read (#5293):

• Introduces ToolResponseStore with configurable capacity (MAX_STORE_SIZE = 500) and TTL (5 minutes default)
• Implements automatic eviction of oldest entries when at capacity to prevent memory growth
• Provides take() method that removes entries after consumption, and get() method for peeking
• Includes periodic cleanup of expired entries and comprehensive test coverage
• Exports the new module and its public API through tools/mod.rs

Issues Found:

• The remove_on_read configuration field is defined but never used in the implementation
• Unused warn import from tracing crate

Confidence Score: 4/5

• Safe to merge with minor fixes needed for unused code
• Well-implemented bounded storage system with comprehensive tests and clear documentation. Two minor issues: unused remove_on_read config field and unused warn import. The core logic for bounded storage, eviction, and cleanup is sound. PR description mismatch should be corrected for clarity.
• src/cortex-engine/src/tools/response_store.rs needs attention for unused config field and import

Important Files Changed

Filename	Overview
src/cortex-engine/src/tools/mod.rs	Added module declaration and public exports for new response_store module
src/cortex-engine/src/tools/response_store.rs	New bounded tool response storage with TTL cleanup and capacity management; minor issues: unused config field and import

Sequence Diagram

sequenceDiagram
    participant Client
    participant Store as ToolResponseStore
    participant HashMap as responses: HashMap
    participant Cleanup as Periodic Cleanup

    Note over Client,Cleanup: Storage Flow with Bounded Capacity

    Client->>Store: store(call_id, tool_name, result)
    Store->>Store: maybe_cleanup() - check if cleanup needed
    alt Cleanup interval exceeded
        Store->>HashMap: cleanup_expired() - remove stale entries
    end
    
    alt Store at capacity (>= MAX_STORE_SIZE)
        Store->>HashMap: find_oldest_key()
        HashMap-->>Store: oldest_key
        Store->>HashMap: remove(oldest_key) - evict oldest
        Note over Store: Eviction prevents unbounded growth
    end
    
    Store->>HashMap: insert(call_id, response)
    Store-->>Client: evicted: bool

    Note over Client,HashMap: Consuming Response (take)
    
    Client->>Store: take(call_id)
    Store->>HashMap: remove(call_id)
    HashMap-->>Store: Some(response)
    Store-->>Client: Some(ToolResult)
    Note over Store: Entry removed after consumption (#5293)

    Note over Client,HashMap: Peeking Response (get)
    
    Client->>Store: get(call_id)
    Store->>HashMap: get_mut(call_id)
    HashMap-->>Store: Some(response)
    Store->>Store: Mark response.read = true
    Store-->>Client: Some(ToolResult.clone())
    Note over HashMap: Entry remains in store

Loading

[greptile-apps]

_{2 files reviewed, 2 comments}

_{Edit Code Review Agent Settings | Greptile}

[greptile-apps]

warn import is unused

Suggested change

	use tracing::{debug, warn};
	use tracing::debug;

_{Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!}

Prompt To Fix With AI

This is a comment left during a code review.
Path: src/cortex-engine/src/tools/response_store.rs
Line: 15:15

Comment:
`warn` import is unused

```suggestion
use tracing::debug;
```

<sub>Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!</sub>

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

[greptile-apps]

remove_on_read config field is never used - neither get() nor take() check this field to determine behavior

Prompt To Fix With AI

This is a comment left during a code review.
Path: src/cortex-engine/src/tools/response_store.rs
Line: 72:72

Comment:
`remove_on_read` config field is never used - neither `get()` nor `take()` check this field to determine behavior

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

[echobt]

Consolidated into #73 - fix: consolidated error handling improvements