feat: slash command popup, /feedback, and collection pre-allocation#219
feat: slash command popup, /feedback, and collection pre-allocation#219jexShain wants to merge 1 commit into
Conversation
|
Thanks for the pull request. A maintainer will review it when available. Please keep the PR focused, explain the why in the description, and make sure local checks pass before requesting review. Contribution guide: https://github.com/AI-Shell-Team/aish/blob/main/CONTRIBUTING.md |
|
This pull request description looks incomplete. Please update the missing sections below before review. Missing items:
|
|
Warning Review limit reached
More reviews will be available in 3 minutes and 32 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (13)
📝 WalkthroughWalkthroughThis PR adds memory pre-allocation optimizations across seven crates (cli, context, llm, memory, pty, security, tools) to reduce runtime reallocation overhead, and introduces an interactive slash-command feature allowing shell users to press ChangesMemory allocation optimizations
Interactive slash-command feature
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Suggested labels
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)
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 |
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
crates/aish-shell/src/readline.rs (1)
220-284:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDon't let slash completions mask absolute-path completion.
At the start of the line, any matching slash command returns early from this branch, so absolute paths with the same prefixes stop tab-completing. For example,
/,/s,/r,/m,/t,/p, and/fnow prefer/setup,/resume,/model, etc. instead of filesystem entries like/usr,/root,/srv,/proc, or/tmp. Please merge file candidates into this branch or prioritize file completion for path-like tokens before returning.🤖 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/aish-shell/src/readline.rs` around lines 220 - 284, The branch that handles start-of-line completion currently pushes SLASH_COMMANDS and returns before filesystem matches, causing slash commands like "/setup" to shadow absolute-paths such as "/usr"; change the logic in the completion block so path-like tokens are prioritized or merged: before returning, detect path-like prefixes (word.starts_with("/") or "./" etc.), call file_completer.complete(line, pos, ctx) to obtain file candidates, convert those results into Pair entries and merge them into the candidates vector (deduplicating against BUILTINS/SLASH_COMMANDS and PTY results), and only return Ok((word_start, candidates)) after merging; use the existing functions and types (BUILTINS, SLASH_COMMANDS, query_pty_completions, file_completer.complete, Pair) to locate and implement this change.
🧹 Nitpick comments (1)
crates/aish-shell/src/input.rs (1)
15-20: ⚡ Quick winReuse the shared slash-command source here.
This hardcoded matcher now duplicates the popup/completion list and the dispatcher. The next slash command can easily show up in
SLASH_COMMANDSbut still classify as unknown here. Please derive this check from one shared source or helper so the popup, classifier, and handler stay in sync.🤖 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/aish-shell/src/input.rs` around lines 15 - 20, The code in input.rs currently hardcodes the slash-command matcher (using trimmed.starts_with and matches!(... "/model" | "/setup" ...)) which duplicates SLASH_COMMANDS; change it to derive the check from the shared command source by extracting the first token (e.g., trimmed.split_whitespace().next()) and testing it against the existing SLASH_COMMANDS collection or the shared helper used by the popup/completion/dispatcher, so the classifier uses the same source of truth (refer to SLASH_COMMANDS and the current classifier/dispatcher helper) and remove the hardcoded literal list.
🤖 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/aish-shell/src/app.rs`:
- Around line 325-337: The code fails rustfmt; run cargo fmt --all (or cargo fmt
--all -- --check locally to verify) and reformat the hunks that include the
SearchSelectPanel instantiation and the PanelRuntime::new().run(...) match (and
the similar section around the SearchSelectPanel at 1431-1437) so the file
complies with rustfmt rules; commit the formatted changes.
---
Outside diff comments:
In `@crates/aish-shell/src/readline.rs`:
- Around line 220-284: The branch that handles start-of-line completion
currently pushes SLASH_COMMANDS and returns before filesystem matches, causing
slash commands like "/setup" to shadow absolute-paths such as "/usr"; change the
logic in the completion block so path-like tokens are prioritized or merged:
before returning, detect path-like prefixes (word.starts_with("/") or "./"
etc.), call file_completer.complete(line, pos, ctx) to obtain file candidates,
convert those results into Pair entries and merge them into the candidates
vector (deduplicating against BUILTINS/SLASH_COMMANDS and PTY results), and only
return Ok((word_start, candidates)) after merging; use the existing functions
and types (BUILTINS, SLASH_COMMANDS, query_pty_completions,
file_completer.complete, Pair) to locate and implement this change.
---
Nitpick comments:
In `@crates/aish-shell/src/input.rs`:
- Around line 15-20: The code in input.rs currently hardcodes the slash-command
matcher (using trimmed.starts_with and matches!(... "/model" | "/setup" ...))
which duplicates SLASH_COMMANDS; change it to derive the check from the shared
command source by extracting the first token (e.g.,
trimmed.split_whitespace().next()) and testing it against the existing
SLASH_COMMANDS collection or the shared helper used by the
popup/completion/dispatcher, so the classifier uses the same source of truth
(refer to SLASH_COMMANDS and the current classifier/dispatcher helper) and
remove the hardcoded literal list.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 3ab99244-c0d6-4928-83c3-d08b367cd69a
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (13)
crates/aish-cli/src/update.rscrates/aish-context/src/manager.rscrates/aish-llm/src/session.rscrates/aish-memory/src/manager.rscrates/aish-pty/src/offload.rscrates/aish-pty/src/persistent.rscrates/aish-security/src/policy.rscrates/aish-shell/Cargo.tomlcrates/aish-shell/src/app.rscrates/aish-shell/src/input.rscrates/aish-shell/src/readline.rscrates/aish-tools/src/glob_tool.rscrates/aish-tools/src/grep_tool.rs
…llections - Add interactive slash command popup triggered by typing `/` on empty line - Add `/feedback` command to open GitHub Issues in browser (via `open` crate) - Replace Vec::new()/HashMap::new()/String::new() with with_capacity() across context, llm, memory, pty, security, and tools crates for better performance - Set max history size to 500 entries
1 participant
Summary
/on empty line, with search/filter support/feedbackcommand that opens GitHub Issues in the browser via theopencrateVec::new()/HashMap::new()/String::new()withwith_capacity()across context, llm, memory, pty, security, and tools crates for reduced reallocation overheadTest plan
/on an empty input line — popup should appear with all slash commands/feedback— browser should open GitHub Issues pagecargo testpassesSummary by CodeRabbit
New Features
/on an empty line to browse and insert commands/feedbackcommand: opens the project's Issues page in your browserChores