feat: add @ path autocomplete#111
Conversation
|
Caution Review failedPull request was closed or merged during review No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughAdds async, repo-relative Changes
Sequence DiagramsequenceDiagram
participant User
participant UIModel as UI Model
participant SearchSvc as Path Reference<br/>Search Service
participant CorpusBuilder as Corpus Builder<br/>(rg)
participant Matcher as Fuzzy Matcher
User->>UIModel: Type '@' query
UIModel->>UIModel: detectPathReferenceQuery & refreshPathReferenceFromInput
UIModel->>SearchSvc: Search(normalizedQuery, workspaceRoot)
alt corpus not ready
SearchSvc->>CorpusBuilder: Build corpus (rg)
CorpusBuilder-->>SearchSvc: File list / error
SearchSvc-->>UIModel: uiPathReferenceCorpusReadyMsg / uiPathReferenceCorpusFailedMsg
end
SearchSvc->>Matcher: FuzzyMatch(normalizedQuery, corpus)
Matcher-->>SearchSvc: Match results
SearchSvc-->>UIModel: uiPathReferenceMatchResultMsg
UIModel->>UIModel: handlePathReferenceMatchResult -> update picker state
UIModel-->>User: Render active picker
User->>UIModel: Press Tab/Enter
UIModel->>UIModel: acceptPathReferenceSelection -> applyPathReferenceCompletion
UIModel-->>User: Updated input and cursor
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly Related PRs
Suggested Labelsautorelease 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a5f75e9e94
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
cli/app/ui_layout_rendering_viewport.go (1)
33-34: Avoid rendering the picker just to count its rows.
renderActivePicker()already builds the styled output. Calling it here meanscalcChatLines()renders the picker once for sizing and then the frame render does it again.activePickerPresentation().lineCountgives you the same layout signal without the extra work.♻️ Proposed simplification
- pickerLines := len(l.renderActivePicker(l.effectiveWidth())) + pickerLines := l.model.activePickerPresentation().lineCountSame cleanup is worth applying to the native ongoing line-count path so both sizing flows stay aligned.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cli/app/ui_layout_rendering_viewport.go` around lines 33 - 34, The code currently calls renderActivePicker(l.effectiveWidth()) just to get its row count, causing the picker to be rendered twice; replace that call with l.activePickerPresentation().lineCount to obtain the same line count without extra work, and update the native ongoing line-count path to use activePickerPresentation().lineCount as well so both sizing flows remain aligned (ensure you keep the existing helpLines calculation using helpPaneLineCount/helpPaneMaxLines and only swap the pickerLines source).cli/app/ui.go (1)
558-561: Verify the default path-search service has a teardown path.
NewProjectedUIModel()now allocates a freshuiPathReferenceSearchfor everyuiModel, but nothing onuiModelreleases it. IfnewUIPathReferenceSearch()owns goroutines, channels, or a cached corpus, recreating the UI will leak workers/state unless the service is intentionally process-scoped.Run this to confirm whether the implementation starts background workers and whether a shutdown hook exists. The concern is confirmed if the constructor spins goroutines/channels but the interface/callers expose no
Close/Stoppath.#!/bin/bash set -euo pipefail echo "== uiPathReferenceSearch interface ==" ast-grep --lang go --pattern 'type uiPathReferenceSearch interface { $$$ }' echo echo "== constructor and lifecycle-related methods ==" rg -n -C2 --type=go '\bnewUIPathReferenceSearch\b|\btype uiPathReferenceSearch\b|\b(StartPrewarm|Search|Events|Close|Shutdown|Stop)\b' cli/app echo echo "== worker/cancellation signals in path-reference files ==" fd -i 'ui_path_reference*.go' cli/app | xargs rg -n -C2 --type=go 'go func|make\(chan|context\.WithCancel|cancel\(|Close\(|Shutdown\(|Stop\('🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cli/app/ui.go` around lines 558 - 561, The new allocation of uiPathReferenceSearch via newUIPathReferenceSearch in NewProjectedUIModel gives uiModel ownership of a service that may spawn goroutines/channels but never gets torn down; add a lifecycle method (e.g., Close/Stop/Shutdown) to the uiPathReferenceSearch interface and implement it in the concrete uiPathReferenceSearch, then update uiModel (where pathReferenceSearch and pathReferenceEvents are set) to call that teardown from its own Close/Dispose or when replacing the service; alternatively accept an injected, process-scoped uiPathReferenceSearch to avoid per-model allocation—refer to newUIPathReferenceSearch, uiPathReferenceSearch, uiModel, pathReferenceSearch and pathReferenceEvents when making the changes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@cli/app/ui_input_controller_keys.go`:
- Around line 54-60: The Enter/Tab branch lets a fast keypress submit raw
`@query` because `acceptPathReferenceSelection()` returns false while an async
path-reference lookup is still pending; fix by adding a guard before falling
through that checks the path-reference picker visibility/state (e.g., the picker
visible flag and its state being `pending`/`loading`) and if the picker is
visible and still loading, short-circuit the key handling (return m, nil)
instead of proceeding to submission; update the same guard in the other
identical block (the later Enter/Tab handling) so both locations use
`m.isInputLocked()`, the picker-visible flag, the picker's state, and
`acceptPathReferenceSelection()` consistently.
---
Nitpick comments:
In `@cli/app/ui_layout_rendering_viewport.go`:
- Around line 33-34: The code currently calls
renderActivePicker(l.effectiveWidth()) just to get its row count, causing the
picker to be rendered twice; replace that call with
l.activePickerPresentation().lineCount to obtain the same line count without
extra work, and update the native ongoing line-count path to use
activePickerPresentation().lineCount as well so both sizing flows remain aligned
(ensure you keep the existing helpLines calculation using
helpPaneLineCount/helpPaneMaxLines and only swap the pickerLines source).
In `@cli/app/ui.go`:
- Around line 558-561: The new allocation of uiPathReferenceSearch via
newUIPathReferenceSearch in NewProjectedUIModel gives uiModel ownership of a
service that may spawn goroutines/channels but never gets torn down; add a
lifecycle method (e.g., Close/Stop/Shutdown) to the uiPathReferenceSearch
interface and implement it in the concrete uiPathReferenceSearch, then update
uiModel (where pathReferenceSearch and pathReferenceEvents are set) to call that
teardown from its own Close/Dispose or when replacing the service; alternatively
accept an injected, process-scoped uiPathReferenceSearch to avoid per-model
allocation—refer to newUIPathReferenceSearch, uiPathReferenceSearch, uiModel,
pathReferenceSearch and pathReferenceEvents when making the changes.
🪄 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: CHILL
Plan: Pro
Run ID: bb6eafc7-6128-42b4-b0c1-181ffef01fcb
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (16)
cli/app/ui.gocli/app/ui_input_buffer.gocli/app/ui_input_controller_keys.gocli/app/ui_layout.gocli/app/ui_layout_rendering.gocli/app/ui_layout_rendering_helpers.gocli/app/ui_layout_rendering_viewport.gocli/app/ui_path_reference_integration_test.gocli/app/ui_path_reference_picker.gocli/app/ui_path_reference_picker_test.gocli/app/ui_path_reference_search.gocli/app/ui_path_reference_search_test.gocli/app/ui_path_reference_types.gocli/app/ui_slash_command_picker.godocs/dev/decisions.mdgo.mod
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c5ef03f6fc
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
🧹 Nitpick comments (1)
cli/app/ui_path_reference_search.go (1)
56-83: Consider caching rune length computation in sorting comparator.The sorting comparator calls
len([]rune(left))andlen([]rune(right))which converts strings to rune slices on each comparison. For large match sets, this allocates repeatedly.♻️ Optional optimization
func (fuzzyUIPathReferenceMatcher) Match(query string, candidates []uiPathReferenceCandidate, limit int) []uiPathReferenceCandidate { if strings.TrimSpace(query) == "" || len(candidates) == 0 || limit <= 0 { return nil } matches := fuzzy.FindFrom(query, uiPathReferenceCandidateSource(candidates)) if len(matches) == 0 { return nil } + // Pre-compute rune lengths to avoid repeated allocations during sort + runeLengths := make(map[int]int, len(matches)) + for _, m := range matches { + runeLengths[m.Index] = len([]rune(candidates[m.Index].Path)) + } sort.SliceStable(matches, func(i, j int) bool { if matches[i].Score != matches[j].Score { return matches[i].Score > matches[j].Score } - left := candidates[matches[i].Index].Path - right := candidates[matches[j].Index].Path - if len([]rune(left)) != len([]rune(right)) { - return len([]rune(left)) < len([]rune(right)) + leftLen := runeLengths[matches[i].Index] + rightLen := runeLengths[matches[j].Index] + if leftLen != rightLen { + return leftLen < rightLen } + left := candidates[matches[i].Index].Path + right := candidates[matches[j].Index].Path return left < right })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cli/app/ui_path_reference_search.go` around lines 56 - 83, In fuzzyUIPathReferenceMatcher.Match, the sort comparator repeatedly calls len([]rune(left)) and len([]rune(right)), causing allocations; precompute rune lengths for each candidate (e.g., build a slice or map of rune lengths keyed by candidate index before calling sort.SliceStable) and use those cached lengths inside the comparator when comparing matches[i].Index and matches[j].Index so the comparator uses the cached lengths for tie-breaking instead of converting strings on every comparison.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@cli/app/ui_path_reference_search.go`:
- Around line 56-83: In fuzzyUIPathReferenceMatcher.Match, the sort comparator
repeatedly calls len([]rune(left)) and len([]rune(right)), causing allocations;
precompute rune lengths for each candidate (e.g., build a slice or map of rune
lengths keyed by candidate index before calling sort.SliceStable) and use those
cached lengths inside the comparator when comparing matches[i].Index and
matches[j].Index so the comparator uses the cached lengths for tie-breaking
instead of converting strings on every comparison.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 53f578ec-a791-4062-8c25-f95b021a0559
📒 Files selected for processing (10)
cli/app/ui.gocli/app/ui_input_controller_keys.gocli/app/ui_layout.gocli/app/ui_layout_rendering_viewport.gocli/app/ui_loop.gocli/app/ui_path_reference_integration_test.gocli/app/ui_path_reference_picker.gocli/app/ui_path_reference_picker_test.gocli/app/ui_path_reference_search.gocli/app/ui_path_reference_search_test.go
✅ Files skipped from review due to trivial changes (2)
- cli/app/ui_layout_rendering_viewport.go
- cli/app/ui_path_reference_picker.go
🚧 Files skipped from review as they are similar to previous changes (2)
- cli/app/ui_input_controller_keys.go
- cli/app/ui_path_reference_search_test.go
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 077845408e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c627832298
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
1 participant
Summary
@path autocomplete incli/appwith shared picker rendering, slash priority, directory completion, and rollback-edit supportrg --files --hidden -g '!.git', fuzzy-match in process, and handle loading/retry/stale async results safelyVerification
./scripts/test.sh ./cli/app./scripts/test.sh./scripts/build.sh --output ./bin/builderSummary by CodeRabbit
New Features
@-prefix path autocomplete with an async, cached repo-relative corpus; prewarms per workspace, shows loading, and appends/for directories without submitting on accept.UI
Tests
Documentation
Chores