feat: @ operator for file/directory search suggestions (SEA-83)#683
Open
fluxdiv wants to merge 13 commits into
Open
feat: @ operator for file/directory search suggestions (SEA-83)#683fluxdiv wants to merge 13 commits into
@ operator for file/directory search suggestions (SEA-83)#683fluxdiv wants to merge 13 commits into
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds Changes@ Path Mention Autocomplete
Sequence Diagram(s)sequenceDiagram
rect rgba(173, 216, 230, 0.5)
Note over User,Renderer: @ path mention autocomplete flow
end
participant User
participant ChatState
participant PopupState as PathMentionPopupState
participant Index as PathMentionIndex
participant Composer
participant Renderer
User->>ChatState: text input or @
ChatState->>Composer: update text
ChatState->>PopupState: sync(text, cursor, index)
PopupState->>Index: query_window(query, offset, limit)
Index-->>PopupState: QueryResult (matches, overflow)
PopupState-->>ChatState: popup visible with matches
ChatState->>Renderer: mark dirty
Renderer-->>User: render path mention popup
User->>ChatState: Up/Down arrow
ChatState->>PopupState: select_next/select_previous
PopupState-->>ChatState: selection updated
ChatState->>Renderer: mark dirty
User->>ChatState: Enter or Tab
ChatState->>PopupState: selected_entry()
PopupState-->>ChatState: PathMentionEntry
ChatState->>Composer: replace_range(range, insertion_text + " ")
ChatState->>PopupState: clear_dismissed, sync
ChatState->>Renderer: mark dirty
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Collaborator
Author
How to use the Graphite Merge QueueAdd either label to this PR to merge it via the merge queue:
You must have a Graphite account in order to use the merge queue. Sign up using this link. An organization admin has required the Graphite Merge Queue in this repository. Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue. This stack of pull requests is managed by Graphite. Learn more about stacking. |
|
Docs preview: https://06-22-sea-83.seal-docs.pages.dev |
Greptile SummaryAdds
Confidence Score: 5/5Safe to merge; the new path-mention feature is isolated from existing flows and all edge cases are covered by the included tests. Background index building is off the main thread, the single-threaded event loop avoids races, byte/char boundary handling is consistent throughout active_path_mention and replace_range, and the two previously flagged issues are both resolved in this revision. path_mentions.rs walk_dir loop body — git-worktree .git files pass the current directory-only guard and would appear as completions in worktree setups. Important Files Changed
Reviews (8): Last reviewed commit: "fix: keep selected path mention always v..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/seal-tui/src/app.rs`:
- Around line 439-441: The popup handler handle_path_mention_popup_keys is
running before handle_composer_keys, which allows Enter to complete path
suggestions even when paste mode FSM would treat it as a pasted newline. Reorder
these handler calls so that handle_composer_keys runs before
handle_path_mention_popup_keys to ensure paste-mode Enter is routed through
newline insertion first. Additionally, update the test cases mentioned in the
"Also applies to" section that are currently using ChatState::new() to instead
use ready_app() or explicitly configure paste-FSM settings to opt out of paste
detection, so tests do not rely on this bypass behavior.
- Around line 455-472: The ChatState::handle_paste method mutates composer.text
and composer.cursor without calling sync_path_mention_popup(), which leaves
stale path mention suggestions rendered when pasting over an active mention.
Locate the handle_paste method and ensure it calls sync_path_mention_popup()
after any mutations to composer.text or composer.cursor, following the same
pattern used in other methods like set_path_mention_index that also modify the
composer state.
In `@crates/seal-tui/src/chat/runloop.rs`:
- Around line 161-179: The spawn_path_mention_index function only sends a
message through the tx channel when index building succeeds, but when the error
path is hit at the Err(err) branch, no message is sent, leaving the
path_index_building flag permanently set to true and preventing periodic
rebuilds. Fix this by sending a message through the tx channel in the error case
as well, allowing the calling code to reset the path_index_building flag and
resume periodic rebuild attempts instead of stalling indefinitely.
In `@crates/seal-tui/src/path_mentions.rs`:
- Around line 89-131: The query_window method performs inefficient repeated
scans of the prefix_range on every keystroke, calculating bucket_totals by
iterating the entire range and then iterating again per bucket. This causes O(N)
performance on large repositories. To fix this, either precompute the bucket and
level metadata for each entry at construction time (storing it directly in the
entry structure) so it's immediately available without recalculation, or
refactor the logic to only compute the necessary bucket totals incrementally up
to the limit required, avoiding the need to scan and count the entire range
upfront. This will keep the hot path bounded by the limit parameter rather than
the total index size.
- Around line 254-269: The refresh_matches function fails to re-query the index
after clamping window_start to 0 when the result is empty, leaving the matches
list empty and causing the popup to disappear. After the condition checking if
self.matches.is_empty() resets window_start, selected, and overflow_count to 0,
add another call to index.query_window with the corrected window_start value of
0 and update self.matches and self.total_count with the new results before
returning, so valid matches at the new offset are properly displayed.
In `@crates/seal-tui/src/renderer.rs`:
- Around line 152-168: The renderer in the visible_matches assignment ignores
the window_start field from PathMentionPopupState, always displaying the first
item_rows matches instead of the scrolled window. To fix this, add a public
accessor method in PathMentionPopupState that returns the starting index
accounting for window_start, or use it directly if already exposed. Then in the
renderer, replace the take(item_rows) call to instead skip(window_start) before
taking the items, and adjust the idx comparison in the enumerate callback to use
the offset-adjusted index (idx + window_start) when comparing against
popup.selected to ensure the highlighted item remains visible and consistent
with the selection.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 35c8fd81-89e0-4e11-9b0f-fcacc82357a1
📒 Files selected for processing (11)
crates/seal-cli/src/cli/config.rscrates/seal-cli/src/main.rscrates/seal-tui/src/app.rscrates/seal-tui/src/chat/host.rscrates/seal-tui/src/chat/runloop.rscrates/seal-tui/src/composer.rscrates/seal-tui/src/lib.rscrates/seal-tui/src/path_mentions.rscrates/seal-tui/src/renderer.rsdocs/site/src/content/docs/reference/config-reference.mdxschemas/config.toml.json
![graphite-app[bot]](https://avatars.githubusercontent.com/in/158384?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
crates/seal-tui/src/path_mentions.rs (1)
333-384: 🩺 Stability & Availability | 🟡 MinorAdd recursion depth guard to
walk_dirto prevent stack overflow on pathologically nested directories.The
walk_dirfunction recurses per subdirectory (line 381) without a depth limit. Sufficiently deep directory trees—or vendored subtrees likenode_moduleswhenrespect_gitignore=false—can exhaust the stack. In Rust, stack overflow aborts the entire process rather than returning an error. Symlinks are already skipped, so cycles are not a risk; unbounded depth is.Add a
depthparameter with a constant max depth (e.g.,const MAX_WALK_DEPTH: usize = 64;), check at entry, and increment on recursion.Proposed fix (update both `build` caller at line 62 and recursive call at line 381)
fn walk_dir( dir: &Path, rel_dir: &str, respect_gitignore: bool, inherited_rules: &[IgnoreRule], entries: &mut Vec<PathMentionEntry>, + depth: usize, ) { + if depth >= MAX_WALK_DEPTH { + return; + } let mut rules = inherited_rules.to_vec(); // ... if is_dir { let child = entry.path(); - walk_dir(&child, &rel, respect_gitignore, &rules, entries); + walk_dir(&child, &rel, respect_gitignore, &rules, entries, depth + 1); }Pass
0from thebuildcaller (line 62).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/seal-tui/src/path_mentions.rs` around lines 333 - 384, The `walk_dir` function recurses without depth limits, which can cause stack overflow on deeply nested directories. Add a `depth` parameter to the `walk_dir` function signature, define a constant `MAX_WALK_DEPTH` set to 64, and check at the beginning of the function to return early if depth exceeds this limit. Increment the depth parameter by 1 when making the recursive call to `walk_dir` at the current line 381. Finally, update the initial call to `walk_dir` in the `build` function to pass 0 as the starting depth value.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@crates/seal-tui/src/path_mentions.rs`:
- Around line 333-384: The `walk_dir` function recurses without depth limits,
which can cause stack overflow on deeply nested directories. Add a `depth`
parameter to the `walk_dir` function signature, define a constant
`MAX_WALK_DEPTH` set to 64, and check at the beginning of the function to return
early if depth exceeds this limit. Increment the depth parameter by 1 when
making the recursive call to `walk_dir` at the current line 381. Finally, update
the initial call to `walk_dir` in the `build` function to pass 0 as the starting
depth value.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 12d8b56a-d759-486e-96cd-03d52296556d
📒 Files selected for processing (3)
crates/seal-tui/src/app.rscrates/seal-tui/src/path_mentions.rscrates/seal-tui/src/renderer.rs
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/seal-tui/src/app.rs`:
- Around line 1583-1599: The test
`path_mention_enter_in_paste_mode_inserts_newline` is flaky because it relies on
timing-dependent FSM state transitions to enter paste mode. Instead of depending
on scheduler timing, explicitly set the paste mode to true before testing Enter
key routing. After setting the path mention index, directly set
`app.composer.paste_mode = true` before sending the key presses. This makes the
test deterministic by validating Enter key behavior in paste mode without
depending on sub-millisecond timing of the FSM state machine.
In `@crates/seal-tui/src/path_mentions.rs`:
- Around line 485-528: The custom glob matching implementation in
path_pattern_matches, match_segments, segment_pattern_matches, and
segment_match_inner only supports basic wildcards (*, ?, **) and does not handle
full .gitignore syntax like character classes or brace expansion, which could
cause sensitive files to leak into autocomplete if users expect these patterns
to work. Replace this custom implementation by adding the ignore crate (v0.4.26)
as a direct dependency in the seal-tui Cargo.toml and refactor the path matching
logic to use the ignore crate's built-in pattern matching, which provides
complete .gitignore semantics support.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 4b43537a-47e7-42f2-b05f-d687d1c1195f
📒 Files selected for processing (11)
crates/seal-cli/src/cli/config.rscrates/seal-cli/src/main.rscrates/seal-tui/src/app.rscrates/seal-tui/src/chat/host.rscrates/seal-tui/src/chat/runloop.rscrates/seal-tui/src/composer.rscrates/seal-tui/src/lib.rscrates/seal-tui/src/path_mentions.rscrates/seal-tui/src/renderer.rsdocs/site/src/content/docs/reference/config-reference.mdxschemas/config.toml.json
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
3 issues found across 15 files
Reply with feedback, questions, or to request a fix.
Fix all with cubic | Re-trigger cubic
fluxdiv
changed the title
@ operator for file/directory search suggestions@ operator for file/directory search suggestions (SEA-83)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Pull request
Summary
Adds
@-path mention autocomplete to the TUI composer. Typing@followed by a path prefix opens a popup menu showing matching files and directories from the project root, with keyboard navigation and completion. A new[tui] path_mentions_respect_gitignoreconfig option (defaulttrue) controls whether.gitignorerules are applied when building the file index.Related issues
Refs #83
Changes
path_mentionsmodule withPathMentionIndex,PathMentionPopupState, andactive_path_mention— covering index building, prefix querying with priority ordering (same-level dirs → same-level files → descendant dirs → descendant files), and popup lifecycle (open, navigate, dismiss, insert)..gitignoreparser supporting negation, directory-only rules, anchored patterns, glob wildcards (*,?,**), and inherited rules across subdirectories.spawn_path_mention_index.ChatState::handle_key:Up/Downnavigate,Enter/Tabinsert the selected entry (appending a space),Escdismisses for the current token.Composer::replace_rangeto support in-place token substitution with cursor repositioning.path_mentions_respect_gitignoretoTuiConfig,TuiConfigFile,HostContext, config merging, JSON schema, and documentation.Test plan
path_mentionscover:active_path_mentiondetection (bare@, path prefix, mid-token cursor, last-@-in-token), query ordering and overflow, prefix-only matching,.gitignorerespect vs. ignore, directory-only rules, and negation.appcover: popup opens on matching prefix, stays hidden with no matches, andEnterinserts the selected path and clears the@sigil.path_mentions_respect_gitignoredefaults totrueand that a local config overrides a global config value.Notes for reviewers
Esc; dismissal is cleared on the next insertion or when the token changes.file_menu_demo.mp4