Skip to content

Conversation

@echobt
Copy link
Contributor

@echobt echobt commented Feb 4, 2026

Summary

Fixes #5096 and #5226 - Arithmetic underflow in compaction calculations.

Problem

Token count subtractions can underflow when removed tokens exceed original count.

Solution

Used saturating_sub() for safe arithmetic without panic or wrap.

@greptile-apps
Copy link

greptile-apps bot commented Feb 4, 2026

Greptile Overview

Greptile Summary

Replaced unsafe arithmetic operations with saturating arithmetic to prevent integer underflow in token count calculations during conversation compaction.

Key Changes:

  • Changed current_tokens - tokens_in_compacted + summary_tokens to use saturating_sub() and saturating_add()
  • Prevents panic when tokens_in_compacted exceeds current_tokens (which could happen due to token estimation inaccuracies)
  • Maintains correctness by clamping to 0 instead of wrapping around on underflow

Safety Improvement:
The fix aligns with Rust best practices seen throughout the codebase, where saturating arithmetic is used extensively for UI calculations and other scenarios where underflow is possible.

Confidence Score: 5/5

  • This PR is safe to merge with no risk
  • The change is minimal, well-documented, and follows established patterns in the codebase. It replaces potentially unsafe arithmetic with saturating operations, which is a defensive programming practice that prevents panics without changing the logical behavior in normal circumstances
  • No files require special attention

Important Files Changed

Filename Overview
src/cortex-compact/src/compactor.rs replaced unsafe arithmetic with saturating_sub and saturating_add to prevent integer underflow in token count calculations

Sequence Diagram

sequenceDiagram
    participant Client
    participant Compactor
    participant Summarizer
    
    Client->>Compactor: compact(items, summary_text, current_tokens)
    Compactor->>Summarizer: select_items_to_compact(items)
    Summarizer-->>Compactor: items to compact
    
    alt No items to compact
        Compactor-->>Client: no_compaction result
    else Items to compact
        Compactor->>Compactor: Calculate tokens_in_compacted
        Compactor->>Summarizer: estimate_tokens(summary_text)
        Summarizer-->>Compactor: summary_tokens
        
        Note over Compactor: Build new items list with summary
        
        Note over Compactor: Calculate tokens_after using<br/>saturating_sub & saturating_add<br/>to prevent underflow
        
        Compactor->>Compactor: tokens_after = current_tokens<br/>.saturating_sub(tokens_in_compacted)<br/>.saturating_add(summary_tokens)
        
        Compactor-->>Client: CompactionResult with new items
    end
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.

1 file reviewed, no comments

Edit Code Review Agent Settings | Greptile

echobt added a commit that referenced this pull request Feb 4, 2026
…ng operations

This PR consolidates the following numeric safety fixes:
- #39: Use saturating casts in git_info to prevent overflow
- #40: Use saturating casts for token counts in streaming
- #41: Use saturating subtraction to prevent underflow in compaction

All changes use saturating arithmetic operations:
- Replaced direct casts with saturating_sub and try_into
- Prevents panic on numeric overflow/underflow conditions
@echobt
Copy link
Contributor Author

echobt commented Feb 4, 2026

Consolidated into #71 - fix: consolidated numeric overflow/underflow prevention with saturating operations

@echobt echobt closed this Feb 4, 2026
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