Skip to content

Conversation

@echobt
Copy link
Contributor

@echobt echobt commented Feb 4, 2026

Summary

Fixes #5199 and #5193 - Autocomplete popup issues.

Problems

  1. Popup goes off-screen when rendered above
  2. Width too wide when filtered list is smaller

Solution

Added screen bounds check for positioning and use filtered items for width.

@greptile-apps
Copy link

greptile-apps bot commented Feb 4, 2026

Greptile Overview

Greptile Summary

Fixed two autocomplete popup issues: prevents off-screen rendering at the top by checking if there's room above before positioning, and calculates width based on filtered items instead of all items to prevent unnecessarily wide popups.

Key Changes

  • Width calculation now uses visible_items() instead of items to match filtered list size
  • Added screen bounds check: positions popup above input if room exists, otherwise below
  • Improved comments explaining the positioning logic

Potential Issue

  • The fallback positioning (below input) doesn't verify bottom screen bounds, which could cause overflow at the bottom edge when there's insufficient room both above and below the input cursor

Confidence Score: 4/5

  • Safe to merge with minor edge case noted
  • The changes correctly fix the two reported issues (off-screen above and width calculation). The implementation is clean and well-commented. However, there's a minor edge case where positioning below the input doesn't check if the popup will overflow the bottom of the screen, which could occur in small terminals or when the cursor is near the bottom.
  • No files require special attention beyond the noted edge case

Important Files Changed

Filename Overview
src/cortex-tui/src/widgets/autocomplete.rs Improved popup positioning with screen bounds check and fixed width calculation to use filtered items - minor edge case with bottom overflow

Sequence Diagram

sequenceDiagram
    participant User
    participant Widget as AutocompletePopup
    participant State as AutocompleteState
    participant Screen as Terminal Screen

    User->>Widget: Trigger autocomplete
    Widget->>State: Check visible_items()
    State-->>Widget: Return filtered items slice
    
    Widget->>Widget: calculate_dimensions()
    Note over Widget: Use visible_items() for width<br/>Calculate height from item count
    
    Widget->>Screen: Check area.y >= height?
    alt Room above input
        Widget->>Screen: Position at area.y - height
    else No room above
        Widget->>Screen: Position at area.bottom()
        Note over Widget,Screen: May overflow if not enough<br/>room below either
    end
    
    Widget->>Screen: Render popup with items
    Widget->>Widget: render_item() for each visible item
    Screen-->>User: Display autocomplete popup
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, 1 comment

Edit Code Review Agent Settings | Greptile

Comment on lines +210 to +216
let y = if area.y >= height {
// Enough room above - position popup above the input
area.y.saturating_sub(height)
} else {
// Not enough room above - position popup below the input
area.bottom()
};
Copy link

Choose a reason for hiding this comment

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

positioning below doesn't check screen bounds - popup may overflow bottom edge

when area.y < height, popup positions at area.bottom() without verifying if area.bottom() + height exceeds terminal height

Suggested change
let y = if area.y >= height {
// Enough room above - position popup above the input
area.y.saturating_sub(height)
} else {
// Not enough room above - position popup below the input
area.bottom()
};
let y = if area.y >= height {
// Enough room above - position popup above the input
area.y.saturating_sub(height)
} else {
// Not enough room above - position popup below the input
// Note: caller should ensure area.bottom() + height doesn't exceed screen bounds
area.bottom()
};
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/cortex-tui/src/widgets/autocomplete.rs
Line: 210:216

Comment:
positioning below doesn't check screen bounds - popup may overflow bottom edge

when `area.y < height`, popup positions at `area.bottom()` without verifying if `area.bottom() + height` exceeds terminal height

```suggestion
        let y = if area.y >= height {
            // Enough room above - position popup above the input
            area.y.saturating_sub(height)
        } else {
            // Not enough room above - position popup below the input
            // Note: caller should ensure area.bottom() + height doesn't exceed screen bounds
            area.bottom()
        };
```

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

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

This PR consolidates the following fixes:
- #38: Prevent usize to u16 overflow in interactive renderer
- #42: Prevent usize to u16 overflow in card count displays
- #58: Fix cursor positioning and underflow in selection list
- #59: Fix mention popup positioning and Unicode width calculation
- #60: Improve autocomplete popup positioning and width calculation
- #64: Prevent underflow in dropdown navigation and scroll calculations
- #66: Prevent panic in HelpBrowserState when sections empty

All changes target the TUI components to improve robustness:
- Added saturating casts for u16 conversions
- Fixed cursor positioning calculations
- Added bounds checking for empty sections
- Improved Unicode width handling for popups
@echobt
Copy link
Contributor Author

echobt commented Feb 4, 2026

Consolidated into #69 - fix(tui): consolidated TUI fixes for overflow, positioning, and panic prevention

@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