Improve natural language install intent handling#763
Conversation
CLA Verification FailedThe following contributors have not signed the Contributor License Agreement:
How to Sign
This check runs automatically. Maintainers can update |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThis PR introduces a natural-language install intent resolver that interprets user queries like "install python development tools" into concrete ChangesInstall Intent Resolution Core
Sequence Diagram(s)sequenceDiagram
participant User as User
participant CLI as CLI command
participant Resolver as resolve_install_intent
participant Tokenizer as tokenize
participant Matchers as ProfileMatchers
participant Selector as CandidateSelector
participant Shell as sudo apt
User->>CLI: issue install/setup description
CLI->>Resolver: call resolve_install_intent(query)
Resolver->>Tokenizer: tokenize + normalize
Tokenizer->>Matchers: tokens
Matchers->>Selector: candidates with confidence
Selector->>CLI: InstallPlan (Ready/NeedsClarification/Unknown)
CLI->>User: print summary_lines
alt Ready and confirmed
CLI->>Shell: run sudo apt install <packages>
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
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 |
![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)
wezterm/src/cli/install_intent.rs (2)
293-314: 💤 Low valueConsider expanding ambiguity detection.
The ambiguity detection relies on a small stopword list and limited patterns. Consider whether additional ambiguous patterns should be caught before a demo, such as:
- Generic tech terms without specifics ("install development tools", "install programming stuff")
- Requests mentioning multiple unrelated categories
🤖 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/src/cli/install_intent.rs` around lines 293 - 314, is_ambiguous_request currently only catches a few vague tokens and a single "server" case; extend its ambiguity heuristics by (1) adding more generic tech terms to the stopword/ambiguous set (e.g. "development", "programming", "dev", "libs", "packages", "tools", "stuff") — update is_stopword or create a small is_generic_tech_token helper; (2) add pattern checks inside is_ambiguous_request to treat tokens like "development tools", "programming", or "libraries" as ambiguous even when alone (extend the matches! check on meaningful[0]); and (3) detect requests mentioning multiple unrelated categories by defining small category keyword lists (e.g. web: ["web","http","nginx","apache"], db: ["database","postgres","mysql"], infra: ["ssh","server","docker"], dev: ["development","programming","tools"]) and returning true if tokens match two or more different categories (use has_any or a new function to count category hits). Update references to is_stopword, is_ambiguous_request, and has_any when implementing these changes.
99-99: ⚡ Quick winDocument the confidence threshold rationale.
The confidence thresholds (0.68 for Ready, 0.42 for clarification, 0.18 for unknown) appear to be magic numbers. Comments should explain WHY these specific values were chosen rather than WHAT they do.
📝 Example documentation
+ // Confidence threshold of 0.68 ensures high certainty before executing package installs, + // chosen to balance typo tolerance with safety for demo-critical operations. if best.confidence >= 0.68 {Also applies to: 73-73, 66-66
🤖 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/src/cli/install_intent.rs` at line 99, The comparisons against best.confidence (e.g., if best.confidence >= 0.68 and the other checks at 0.42 and 0.18) use unexplained magic numbers; add brief inline comments near each threshold in wezterm::cli::install_intent (around the checks using best.confidence) that state the rationale — e.g., that 0.68 was chosen from calibration/ROC analysis to balance precision/recall, 0.42 triggers a clarification when probability is ambiguous, and 0.18 marks model uncertainty — and include how these values were derived (data/calibration method or heuristic) and a TODO referencing where to update them if recalibrating.
🤖 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/src/cli/shortcuts.rs`:
- Line 123: Replace the unsafe shell invocation
Command::new("sh").arg("-c").arg(&command).status()? by invoking the executable
and arguments directly: stop building a single shell string and instead call
Command::new with the concrete program (e.g., "sudo" or "apt") and pass each
argument via .arg(...) so the package name comes in as a parameter, not
interpolated into a shell; locate the call to Command::new("sh") that uses the
variable command and change it to a parameterized Command::new(...) invocation
that supplies the program and each argument separately (for example,
Command::new("sudo").arg("apt").arg("install").arg(package_name).status()),
preserving existing error handling around status().
---
Nitpick comments:
In `@wezterm/src/cli/install_intent.rs`:
- Around line 293-314: is_ambiguous_request currently only catches a few vague
tokens and a single "server" case; extend its ambiguity heuristics by (1) adding
more generic tech terms to the stopword/ambiguous set (e.g. "development",
"programming", "dev", "libs", "packages", "tools", "stuff") — update is_stopword
or create a small is_generic_tech_token helper; (2) add pattern checks inside
is_ambiguous_request to treat tokens like "development tools", "programming", or
"libraries" as ambiguous even when alone (extend the matches! check on
meaningful[0]); and (3) detect requests mentioning multiple unrelated categories
by defining small category keyword lists (e.g. web:
["web","http","nginx","apache"], db: ["database","postgres","mysql"], infra:
["ssh","server","docker"], dev: ["development","programming","tools"]) and
returning true if tokens match two or more different categories (use has_any or
a new function to count category hits). Update references to is_stopword,
is_ambiguous_request, and has_any when implementing these changes.
- Line 99: The comparisons against best.confidence (e.g., if best.confidence >=
0.68 and the other checks at 0.42 and 0.18) use unexplained magic numbers; add
brief inline comments near each threshold in wezterm::cli::install_intent
(around the checks using best.confidence) that state the rationale — e.g., that
0.68 was chosen from calibration/ROC analysis to balance precision/recall, 0.42
triggers a clarification when probability is ambiguous, and 0.18 marks model
uncertainty — and include how these values were derived (data/calibration method
or heuristic) and a TODO referencing where to update them if recalibrating.
🪄 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: f87e6acb-7401-469b-a975-a98252d476a0
📒 Files selected for processing (3)
wezterm/src/cli/install_intent.rswezterm/src/cli/mod.rswezterm/src/cli/shortcuts.rs
![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 a deterministic natural language intent resolver to handle common installation requests offline, covering stacks like Docker, Machine Learning, and Python. It integrates this into the install and setup CLI commands as a primary check before falling back to AI-based processing. Review feedback highlights a logic error in the install command that prevents the AI fallback from being reached for unknown packages and suggests optimizing the Levenshtein distance implementation to reduce memory allocations.
| let query = format!("install {}", self.description.join(" ")); | ||
| let query = self.description.join(" "); | ||
|
|
||
| if handle_install_intent(&query, self.auto_confirm, true)? { |
There was a problem hiding this comment.
The handle_unknown parameter is set to true here, which causes handle_install_intent to return Ok(true) and print a generic "I do not know a safe package set" message for any request that doesn't match the deterministic resolver. This effectively blocks the AI fallback logic on lines 51-62, making it unreachable for unknown packages (e.g., cx install vim). To allow the intended AI fallback for requests outside the demo-critical set, this should be false.
| if handle_install_intent(&query, self.auto_confirm, true)? { | |
| if handle_install_intent(&query, self.auto_confirm, false)? { |
| fn levenshtein(left: &str, right: &str) -> usize { | ||
| let mut costs: Vec<usize> = (0..=right.len()).collect(); | ||
|
|
||
| for (i, left_char) in left.chars().enumerate() { | ||
| let mut previous = i; | ||
| costs[0] = i + 1; | ||
|
|
||
| for (j, right_char) in right.chars().enumerate() { | ||
| let insert = costs[j + 1] + 1; | ||
| let delete = costs[j] + 1; | ||
| let replace = previous + usize::from(left_char != right_char); | ||
| previous = costs[j + 1]; | ||
| costs[j + 1] = insert.min(delete).min(replace); | ||
| } | ||
| } | ||
|
|
||
| costs[right.len()] | ||
| } |
There was a problem hiding this comment.
The levenshtein function allocates a new Vec<usize> on every call. Since token_matches is called in a loop for every token against every keyword in the candidate functions, this results in many small allocations. While acceptable for a CLI tool, reusing a pre-allocated buffer or using a fixed-size array for the costs (since tokens are typically short) would improve efficiency.
There was a problem hiding this comment.
Pull request overview
This PR adds a deterministic, offline “install intent” resolver to make cx install/cx setup handle a handful of demo-critical natural-language installation requests without needing AI fallback, and to present the selected package plan (reasoning/confidence/alternatives) before running anything.
Changes:
- Introduces
install_intentmodule with tokenization + heuristic matching and unit tests. - Routes
cx installandcx setupthrough the resolver before falling back tocx ask. - Adds interactive confirmation + execution for “Ready” plans.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
wezterm/src/cli/shortcuts.rs |
Calls the new resolver for install/setup and executes the resulting apt install plan when ready. |
wezterm/src/cli/mod.rs |
Exposes the new install_intent module. |
wezterm/src/cli/install_intent.rs |
Implements deterministic intent resolution, plan summaries, and adds unit tests for expected demo cases. |
| println!("Command: {}", command); | ||
|
|
||
| if auto_confirm || confirm_install()? { | ||
| let status = Command::new("sh").arg("-c").arg(&command).status()?; | ||
|
|
| if self.status != InstallPlanStatus::Ready || self.packages.is_empty() { | ||
| return None; | ||
| } | ||
|
|
||
| Some(format!("sudo apt install {}", self.packages.join(" "))) |
| .map(|alt| alt.to_string()) | ||
| .collect(), | ||
| reasoning: format!( | ||
| "{} I am not confident enough to install packages without confirmation.", |
|
|
Pushed follow-ups for the review items:
Local verification:
Note: full |
Summary
Addresses cxlinux-ai/cx-distro#25.
Tests
Note: cargo test -p cx-terminal install_intent is blocked in this environment before test execution because Cargo tries to resolve the optional private git dependency mikejmorgan-ai/hrm-ai-standalone and GitHub returns repository not found.
Summary by CodeRabbit