Improve natural language package install parsing#764
Conversation
CLA Verification FailedThe following contributors have not signed the Contributor License Agreement:
How to Sign
This check runs automatically. Maintainers can update |
📝 WalkthroughWalkthroughThe PR extends the package agent to interpret natural-language package installation requests. ChangesNatural-language package installation support
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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 |
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces natural language processing capabilities for package installation requests, allowing the agent to handle descriptive goals like "set up machine learning" or "I need a web server." It includes a normalization layer to handle common typos and adds a clarification mechanism for ambiguous requests. Feedback focuses on refining the heuristic matching logic to avoid false positives with removal commands, narrowing the "ml" keyword check to avoid matching substrings in unrelated words, and optimizing string processing within loops to reduce unnecessary allocations.
| let is_install_request = input.contains("install") | ||
| || input.contains("set up") | ||
| || input.contains("setup") | ||
| || input.contains("i need") | ||
| || input.contains("something for"); | ||
|
|
||
| if !is_install_request { | ||
| return None; | ||
| } |
There was a problem hiding this comment.
The is_install_request logic is prone to false positives because input.contains("install") matches "uninstall", and generic phrases like "i need" or "something for" don't distinguish between installation and removal. For example, "uninstall nginx" or "I need to remove docker" would incorrectly trigger the natural language installation profiles. Consider explicitly excluding removal intent.
| let is_install_request = input.contains("install") | |
| || input.contains("set up") | |
| || input.contains("setup") | |
| || input.contains("i need") | |
| || input.contains("something for"); | |
| if !is_install_request { | |
| return None; | |
| } | |
| let is_removal = input.contains("uninstall") || input.contains("remove"); | |
| let is_install_request = (input.contains("install") | |
| || input.contains("set up") | |
| || input.contains("setup") | |
| || input.contains("i need") | |
| || input.contains("something for")) && !is_removal; | |
| if !is_install_request { | |
| return None; | |
| } |
| }); | ||
| } | ||
|
|
||
| if input.contains("machine learning") || input.contains("ml") { |
There was a problem hiding this comment.
The check input.contains("ml") is too broad and will trigger for any word containing these characters, such as "yaml", "html", "compiler", or "xml". This leads to incorrect triggering of the machine learning profile. Use a word-boundary check or verify that "ml" exists as a standalone word in the input.
| if input.contains("machine learning") || input.contains("ml") { | |
| if input.contains("machine learning") || input.split_whitespace().any(|w| w == "ml") { |
| let package = words | ||
| .iter() | ||
| .skip_while(|&w| w.to_lowercase() != "install") | ||
| .skip_while(|&w| normalize_package_request(w).to_lowercase() != "install") |
There was a problem hiding this comment.
Calling normalize_package_request(w) inside a loop is inefficient as it performs multiple string allocations, splitting, and joining for every word in the input. Since you only need to check if the word represents an 'install' command, a lighter word-level check is preferred.
.skip_while(|&w| {
let w_lower = w.to_lowercase();
let trimmed = w_lower.trim_matches(|c: char| !c.is_alphanumeric() && c != '-');
!matches!(trimmed, "install" | "instal" | "isntall" | "isntal")
})![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
wezterm-gui/src/agents/runtime.rs (2)
197-197: ⚡ Quick winConsider broader pattern matching for Python-related requests.
The phrase "i need python" is highly specific and won't match reasonable variations like "i want python", "i require python", or "need python". Consider adding a few more common variations to make the natural-language detection more robust and user-friendly.
🔍 Suggested broader patterns
|| input.contains("python development") || input.contains("i need python") + || input.contains("i want python") + || input.contains("need python")🤖 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 `@wezterm-gui/src/agents/runtime.rs` at line 197, The literal check input.contains("i need python") is too narrow; replace it with a case-insensitive pattern match that catches common variants (e.g. "need", "want", "require", "install") near "python". For example, convert the input to lowercase or use a Regex (e.g. r"(?i)\b(?:need|want|require|install)\b.*\bpython\b" or r"(?i)\bpython\b.*\b(?:need|want|require|install)\b") and use Regex::is_match or input.to_lowercase().contains with multiple tokens; update the condition that currently uses input.contains("i need python") to this broader check and add the regex crate import if you opt for Regex.
190-196: ⚡ Quick winAdd comment explaining the intent of these keyword additions.
The coding guidelines specify that comments should explain WHY. Consider adding a brief comment above this block explaining that these keywords support natural-language package installation requests, including typo tolerance ("instal") and thematic profiles ("machine learning", "web server", etc.). As per coding guidelines, comments should explain WHY, never WHAT or HOW.
📝 Example comment
// Package agent keywords +// Extended to support natural-language installation requests and common typos +// per cxlinux-ai/cx-distro#25 if input.contains("package") || input.contains("install") || input.contains("instal")🤖 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 `@wezterm-gui/src/agents/runtime.rs` around lines 190 - 196, Add a brief comment above the series of input.contains checks explaining WHY these specific keywords are included: they enable natural-language detection of package installation or environment setup requests (including common typos like "instal" and broader thematic profiles such as "machine learning" or "web server") so the agent can map user intent to package actions; place the comment immediately above the block containing the input.contains(...) checks referencing "input" so future readers understand the intent rather than the mechanics.
🤖 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 `@wezterm-gui/src/agents/package.rs`:
- Around line 268-277: The package vectors (e.g., the packages: vec![ "python3",
"python3-pip", "python3-venv", ... ]) and the profile lists are using apt-style
names but are passed unchanged to other managers (pacman/dnf/brew/nix), which
will break; replace these hard-coded apt names with abstract package keys and
implement a translation layer that maps each abstract key to the correct package
name per package manager (APT, DNF, Pacman, Homebrew, Nix), add special handling
for Nix to use attribute names rather than apt strings, and call that translator
wherever the "packages" vec or profile package lists are used (e.g., the code
that constructs install commands) so the install commands use manager-specific
names.
- Around line 447-452: The code builds a shell command from packages (packages,
package_list) without validating identifiers which risks injection; before
creating cmd_str based on self.package_manager, validate each package name
against a strict allow-list (e.g., a regex that permits only expected characters
like ASCII alphanumerics, hyphen, underscore, dot—and optionally version
separators) and reject any package that fails by returning AgentResponse::error
with a clear message; apply this check to the packages vector before
packages.join(" ") and ensure different package manager flavors
(self.package_manager) enforce any manager-specific constraints.
- Around line 265-266: The substring check input.contains("ml") is too broad and
causes false positives (e.g., matching "xml"); update the matching logic in the
branch that returns PackageCommand::NaturalInstall to only match standalone
tokens for "ml" (and case-insensitive variants) — for example, split or tokenize
the input or use a word-boundary regex to check for "\bml\b" (or equivalent)
instead of contains("ml"), keeping the existing check for "machine learning" and
using the same variable names (input and PackageCommand::NaturalInstall).
- Around line 119-121: The natural-language parser parse_natural_install
currently runs before explicit install handling and preempts commands like
"install docker-compose"; fix this by ensuring explicit install parsing runs
first (or by adding a guard in parse_natural_install to ignore inputs that begin
with "install " or match the explicit install pattern). Concretely, either move
the parse_natural_install(input, &input_lower) call to after the explicit
install parse function (the code path that handles "install <package>") or
update parse_natural_install to check input/input_lower and return None when the
string starts with "install " (or otherwise matches the explicit-install
syntax), so explicit installs are parsed correctly.
---
Nitpick comments:
In `@wezterm-gui/src/agents/runtime.rs`:
- Line 197: The literal check input.contains("i need python") is too narrow;
replace it with a case-insensitive pattern match that catches common variants
(e.g. "need", "want", "require", "install") near "python". For example, convert
the input to lowercase or use a Regex (e.g.
r"(?i)\b(?:need|want|require|install)\b.*\bpython\b" or
r"(?i)\bpython\b.*\b(?:need|want|require|install)\b") and use Regex::is_match or
input.to_lowercase().contains with multiple tokens; update the condition that
currently uses input.contains("i need python") to this broader check and add the
regex crate import if you opt for Regex.
- Around line 190-196: Add a brief comment above the series of input.contains
checks explaining WHY these specific keywords are included: they enable
natural-language detection of package installation or environment setup requests
(including common typos like "instal" and broader thematic profiles such as
"machine learning" or "web server") so the agent can map user intent to package
actions; place the comment immediately above the block containing the
input.contains(...) checks referencing "input" so future readers understand the
intent rather than the mechanics.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: db534c06-525e-428a-a30f-9fdd04b2b552
📒 Files selected for processing (2)
wezterm-gui/src/agents/package.rswezterm-gui/src/agents/runtime.rs
| if let Some(command) = parse_natural_install(input, &input_lower) { | ||
| return command; | ||
| } |
There was a problem hiding this comment.
Natural-language parsing currently preempts explicit install <package> commands.
Running parse_natural_install before explicit install parsing causes regressions like install docker-compose being interpreted as a profile instead of a direct package install.
Suggested fix
- if let Some(command) = parse_natural_install(input, &input_lower) {
- return command;
- }
-
// Install
if input_lower.contains("install") {
let package = words
.iter()
.skip_while(|&w| normalize_package_request(w).to_lowercase() != "install")
.nth(1)
.map(|s| s.to_string())
.or_else(|| words.last().map(|s| s.to_string()))
.unwrap_or_default();
return Self::Install { package };
}
+
+ if let Some(command) = parse_natural_install(input, &input_lower) {
+ return command;
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if let Some(command) = parse_natural_install(input, &input_lower) { | |
| return command; | |
| } | |
| // Install | |
| if input_lower.contains("install") { | |
| let package = words | |
| .iter() | |
| .skip_while(|&w| normalize_package_request(w).to_lowercase() != "install") | |
| .nth(1) | |
| .map(|s| s.to_string()) | |
| .or_else(|| words.last().map(|s| s.to_string())) | |
| .unwrap_or_default(); | |
| return Self::Install { package }; | |
| } | |
| if let Some(command) = parse_natural_install(input, &input_lower) { | |
| return command; | |
| } |
🤖 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 `@wezterm-gui/src/agents/package.rs` around lines 119 - 121, The
natural-language parser parse_natural_install currently runs before explicit
install handling and preempts commands like "install docker-compose"; fix this
by ensuring explicit install parsing runs first (or by adding a guard in
parse_natural_install to ignore inputs that begin with "install " or match the
explicit install pattern). Concretely, either move the
parse_natural_install(input, &input_lower) call to after the explicit install
parse function (the code path that handles "install <package>") or update
parse_natural_install to check input/input_lower and return None when the string
starts with "install " (or otherwise matches the explicit-install syntax), so
explicit installs are parsed correctly.
| if input.contains("machine learning") || input.contains("ml") { | ||
| return Some(PackageCommand::NaturalInstall { |
There was a problem hiding this comment.
"ml" substring matching is too broad and causes false positives.
input.contains("ml") will match unrelated tokens (e.g., xml), incorrectly selecting the machine-learning profile.
Suggested fix
- if input.contains("machine learning") || input.contains("ml") {
+ let tokens: std::collections::HashSet<&str> = input.split_whitespace().collect();
+ if input.contains("machine learning") || tokens.contains("ml") {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if input.contains("machine learning") || input.contains("ml") { | |
| return Some(PackageCommand::NaturalInstall { | |
| let tokens: std::collections::HashSet<&str> = input.split_whitespace().collect(); | |
| if input.contains("machine learning") || tokens.contains("ml") { | |
| return Some(PackageCommand::NaturalInstall { |
🤖 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 `@wezterm-gui/src/agents/package.rs` around lines 265 - 266, The substring
check input.contains("ml") is too broad and causes false positives (e.g.,
matching "xml"); update the matching logic in the branch that returns
PackageCommand::NaturalInstall to only match standalone tokens for "ml" (and
case-insensitive variants) — for example, split or tokenize the input or use a
word-boundary regex to check for "\bml\b" (or equivalent) instead of
contains("ml"), keeping the existing check for "machine learning" and using the
same variable names (input and PackageCommand::NaturalInstall).
| packages: vec![ | ||
| "python3", | ||
| "python3-pip", | ||
| "python3-venv", | ||
| "python3-numpy", | ||
| "python3-scipy", | ||
| "python3-pandas", | ||
| "python3-sklearn", | ||
| "jupyter-notebook", | ||
| ] |
There was a problem hiding this comment.
Natural-install package lists are distro-specific but executed across all package managers.
Profiles use apt-style names (python3-venv, build-essential, docker.io) and are passed directly to pacman/dnf/brew/nix commands. This will fail on non-apt systems and is especially fragile for Nix attribute installs.
Also applies to: 293-299, 311-312, 452-466
🤖 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 `@wezterm-gui/src/agents/package.rs` around lines 268 - 277, The package
vectors (e.g., the packages: vec![ "python3", "python3-pip", "python3-venv", ...
]) and the profile lists are using apt-style names but are passed unchanged to
other managers (pacman/dnf/brew/nix), which will break; replace these hard-coded
apt names with abstract package keys and implement a translation layer that maps
each abstract key to the correct package name per package manager (APT, DNF,
Pacman, Homebrew, Nix), add special handling for Nix to use attribute names
rather than apt strings, and call that translator wherever the "packages" vec or
profile package lists are used (e.g., the code that constructs install commands)
so the install commands use manager-specific names.
| if packages.is_empty() || packages.iter().any(|p| p.trim().is_empty()) { | ||
| return AgentResponse::error("No package name was provided".to_string()); | ||
| } | ||
|
|
||
| let package_list = packages.join(" "); | ||
| let cmd_str = match self.package_manager { |
There was a problem hiding this comment.
Validate package identifiers with an allow-list before command construction.
packages.join(" ") is used to build executable command text without character allow-list validation. Add strict package-name validation before command generation.
Suggested fix
fn install_packages(
&self,
packages: &[String],
confidence: Option<u8>,
reasoning: Option<&str>,
query: Option<&str>,
) -> AgentResponse {
if packages.is_empty() || packages.iter().any(|p| p.trim().is_empty()) {
return AgentResponse::error("No package name was provided".to_string());
}
+ if packages
+ .iter()
+ .any(|p| !p.chars().all(|c| c.is_ascii_alphanumeric() || c == '+' || c == '.' || c == '-' || c == '_'))
+ {
+ return AgentResponse::error("Invalid package name format".to_string());
+ }
let package_list = packages.join(" ");As per coding guidelines, "Use allow-lists for input validation to prevent injection attacks".
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if packages.is_empty() || packages.iter().any(|p| p.trim().is_empty()) { | |
| return AgentResponse::error("No package name was provided".to_string()); | |
| } | |
| let package_list = packages.join(" "); | |
| let cmd_str = match self.package_manager { | |
| if packages.is_empty() || packages.iter().any(|p| p.trim().is_empty()) { | |
| return AgentResponse::error("No package name was provided".to_string()); | |
| } | |
| if packages | |
| .iter() | |
| .any(|p| !p.chars().all(|c| c.is_ascii_alphanumeric() || c == '+' || c == '.' || c == '-' || c == '_')) | |
| { | |
| return AgentResponse::error("Invalid package name format".to_string()); | |
| } | |
| let package_list = packages.join(" "); | |
| let cmd_str = match self.package_manager { |
🤖 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 `@wezterm-gui/src/agents/package.rs` around lines 447 - 452, The code builds a
shell command from packages (packages, package_list) without validating
identifiers which risks injection; before creating cmd_str based on
self.package_manager, validate each package name against a strict allow-list
(e.g., a regex that permits only expected characters like ASCII alphanumerics,
hyphen, underscore, dot—and optionally version separators) and reject any
package that fails by returning AgentResponse::error with a clear message; apply
this check to the packages vector before packages.join(" ") and ensure different
package manager flavors (self.package_manager) enforce any manager-specific
constraints.
There was a problem hiding this comment.
Pull request overview
This PR enhances the wezterm-gui package agent’s natural-language parsing so user intents like “set up python development” or typo’d install requests map to richer install actions (profiles, clarification prompts), with accompanying unit tests.
Changes:
- Expanded agent keyword inference to route more natural-language install intents to the package agent.
- Added
NaturalInstallandClarifycommand variants, plus request normalization and profile-based parsing. - Updated install handling to support installing multiple packages and to display reasoning/confidence for profile installs; added new parser unit tests.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
wezterm-gui/src/agents/runtime.rs |
Extends keyword-based agent inference to better route install/setup requests to the package agent. |
wezterm-gui/src/agents/package.rs |
Adds normalization + natural-language profile parsing, new command variants, multi-package install formatting, and new unit tests. |
Comments suppressed due to low confidence (2)
wezterm-gui/src/agents/package.rs:312
- The PR description mentions adding a Kubernetes install profile, but the only Kubernetes-related profile here requires both "docker" and "kubernetes". A request like "install kubernetes" (or typo-normalized "kubernets") will fall back to the generic
Installpath and likely try to install a non-existent package name. Either add a Kubernetes-only profile (e.g., installkubectl/kubeadm/minikubedepending on intent) or adjust the PR description to reflect the actual supported cases.
if input.contains("docker") && input.contains("kubernetes") {
return Some(PackageCommand::NaturalInstall {
query: original.to_string(),
packages: vec!["docker.io", "docker-compose-plugin", "kubectl"]
.into_iter()
wezterm-gui/src/agents/package.rs:318
- New natural-language parsing branches don’t have coverage for Kubernetes-only requests and the documented typo normalization for "kubernets" (without also mentioning Docker). Add unit tests that assert the intended
PackageCommandoutput for inputs like "install kubernetes" and "instal kubernets" so regressions are caught.
if input.contains("docker") && input.contains("kubernetes") {
return Some(PackageCommand::NaturalInstall {
query: original.to_string(),
packages: vec!["docker.io", "docker-compose-plugin", "kubectl"]
.into_iter()
.map(str::to_string)
.collect(),
confidence: 82,
reasoning: "I understood this as container tooling plus the Kubernetes command-line client.".to_string(),
});
}
| if input_lower.contains("install") { | ||
| let package = words | ||
| .iter() | ||
| .skip_while(|&w| w.to_lowercase() != "install") | ||
| .skip_while(|&w| normalize_package_request(w).to_lowercase() != "install") | ||
| .nth(1) |
| if input.contains("machine learning") || input.contains("ml") { | ||
| return Some(PackageCommand::NaturalInstall { |
| return Some(PackageCommand::NaturalInstall { | ||
| query: original.to_string(), | ||
| packages: vec![ | ||
| "python3", | ||
| "python3-pip", | ||
| "python3-venv", | ||
| "python3-numpy", | ||
| "python3-scipy", | ||
| "python3-pandas", | ||
| "python3-sklearn", | ||
| "jupyter-notebook", | ||
| ] | ||
| .into_iter() | ||
| .map(str::to_string) | ||
| .collect(), | ||
| confidence: 86, | ||
| reasoning: "I understood this as a Python-based machine learning workstation setup." | ||
| .to_string(), |
Addresses cxlinux-ai/cx-distro#25 by improving the package agent natural-language install parser in cx-core, where the CX package agent implementation lives.
Changes:
instal pyhton,dockr,kubernets, andngnix.Validation:
git diff --checkpasses.cargo fmtorcargo testlocally because this environment does not have Rust tooling installed.Summary by CodeRabbit
New Features
Tests