[Quality-543]Remove check_if_token_has_shell_syntax and pin threshold to 1 for nld_heuristic_v2#10846
Conversation
|
I'm starting a first review of this pull request. You can view the conversation on Warp. I completed the review and no human review was requested for this pull request. Comment Powered by Oz |
evelyn-with-warp
changed the title
![oz-for-oss[bot]](https://avatars.githubusercontent.com/in/3450139?s=60&v=4){kind=small}
There was a problem hiding this comment.
Overview
This PR removes shell-syntax characters from the pre-classifier shell-command voting heuristic and adds focused unit coverage for one-off shell keywords, described-token voting, and URL/path natural-language prompts.
Concerns
- No blocking concerns found in the changed lines.
Verdict
Found: 0 critical, 0 important, 0 suggestions
Approve
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
| nld_heuristic_v1 = ["input_classifier/nld_heuristic_v1"] | ||
| nld_heuristic_v2 = ["input_classifier/nld_heuristic_v2"] |
There was a problem hiding this comment.
Why do we need new flags here? Can't we just bundle these changes under classifier v2?
There was a problem hiding this comment.
so we could decouple them, e,g, we could still launch nld_classifier_v2 without heuristic change
|
I'm starting a first review of this pull request. You can view the conversation on Warp. I completed the review and no human review was requested for this pull request. Comment Powered by Oz |
evelyn-with-warp
changed the title
There was a problem hiding this comment.
Overview
This PR adds feature-gated v1/v2 shell-command heuristic behavior, wires the heuristic features into bundle scripts, and adds unit coverage for the classifier utility behavior.
Security
- The new token-level debug logging records raw input tokens; command input can include credentials, private file paths, or URLs, so this should be removed or redacted before merge.
Concerns
- See the inline security comment on the new debug log in
crates/input_classifier/src/util.rs.
Verdict
Found: 0 critical, 1 important, 0 suggestions
Request changes
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
| warp_completer.workspace = true | ||
|
|
||
| [dev-dependencies] | ||
| warp_features.workspace = true |
There was a problem hiding this comment.
we used warp_features::mark_initialized() in added unit tests
| /// nld_heuristic_v1: current prod, use check_if_token_has_shell_syntax and conditional threshold on input length | ||
| /// nld_heuristic_v2: rm check_if_token_has_shell_syntax and pin threshold to be 1 for all input |
There was a problem hiding this comment.
nit: the comment here should start with describing what is_likely_shell_command does. This is more of a detail that should follow
There was a problem hiding this comment.
sg; will clean up upon launch.
| let check_if_token_has_shell_syntax_result = (!use_nld_heuristic_v2).then(|| { | ||
| natural_language_detection::check_if_token_has_shell_syntax(token.token.as_str()) | ||
| }); |
There was a problem hiding this comment.
nit: let check_if_token_has_shell_syntax = !use_nld_heuristic_v2 && natural_language_detection::check_if_token_has_shell_syntax(token.token.as_str())
No need for an Option here
… to 1 for nld_heuristic_v2 (warpdotdev#10846) ## Description <!-- Please remember to add your design buddy onto the PR for review, if it contains any UI changes! --> Implementation of improving heuristics: by allocating more traffic to nld new classifier; This should resolve misfires as shell for file path or url ## Linked Issue <!-- Link the GitHub issue this PR addresses. Before opening this PR, please confirm: --> - [ ] The linked issue is labeled `ready-to-spec` or `ready-to-implement`. - [ ] Where appropriate, screenshots or a short video of the implementation are included below (especially for user-visible or UI changes). ## Testing <!-- How did you test this change? What automated tests did you add? If you didn't add any new tests, what's your justification for not adding any? Manual testing is required for changes that can be manually tested, and almost all changes can be manually tested. If your change can be manually tested, please include screenshots or a screen recording that show it working end to end. You can run the app locally using `./script/run` - see WARP.md for more details on how to get set up. --> `RUST_LOG=debug ./script/run --features nld_heuristic_v2` `RUST_LOG=debug ./script/run --features nld_classifier_v2, nld_heuristic_v2` - [x] I have manually tested my changes locally with `./script/run` ### Screenshots / Videos <!-- Attach screenshots or a short video demonstrating the change, where appropriate. Remove this section if it is not relevant to your PR. --> Test 543 misfire `read this https://trilogy-eng.atlassian.net/browse/epmlive-17588` - it should be true/shell in v1 and false/non-shell in v2 - with `nld_heuristic_v2` and `nld_classifier_v2`, we could see now classifier could classify this one to prompt <img width="2169" height="336" alt="image" src="https://github.com/user-attachments/assets/d6129bc0-dba6-4ecd-9aab-8f36c64f63c8" /> <img width="2158" height="543" alt="image" src="https://github.com/user-attachments/assets/fc698d9d-9cea-4be1-874f-32f881c6fe02" /> <img width="1315" height="187" alt="image" src="https://github.com/user-attachments/assets/ebc989de-f344-4ddb-82ae-a145f55766aa" /> <img width="2168" height="350" alt="image" src="https://github.com/user-attachments/assets/578eccb0-949d-4a1c-a394-ef95711c7af0" /> ## Agent Mode - [ ] Warp Agent Mode - This PR was created via Warp's AI Agent Mode <!-- ## Changelog Entries for Stable The entries below will be used when constructing a soft-copy of the stable release changelog. Leave blank or remove the lines if no entry in the stable changelog is needed. Entries should be on the same line, without the `{{` `}}` brackets. You can use multiple lines, even of the same type. The valid suffixes are: - NEW-FEATURE: for new, relatively sizable features. Features listed here will likely have docs / social media posts / marketing launches associated with them, so use sparingly. - IMPROVEMENT: for new functionality of existing features. - BUG-FIX: for fixes related to known bugs or regressions. - IMAGE: the image specified by the URL (hosted on GCP) will be added to Dev & Preview releases. For Stable releases, see the pinned doc in the #release Slack channel. - OZ: Oz-related updates. Use `CHANGELOG-OZ`. At most 4 Oz updates are shown in-app per release. - NONE: Explicitly opt out of changelog inclusion. Use `CHANGELOG-NONE` for PRs that should never appear in the changelog (e.g. refactors, internal tooling, CI changes). This prevents the changelog agent from inferring an entry. CHANGELOG-NEW-FEATURE: {{text goes here...}} CHANGELOG-IMPROVEMENT: {{text goes here...}} CHANGELOG-BUG-FIX: {{text goes here...}} CHANGELOG-BUG-FIX: {{more text goes here...}} CHANGELOG-IMAGE: {{GCP-hosted URL goes here...}} CHANGELOG-OZ: {{text goes here...}} CHANGELOG-NONE -->
2 participants
Description
Implementation of improving heuristics: by allocating more traffic to nld new classifier;
This should resolve misfires as shell for file path or url
Linked Issue
ready-to-specorready-to-implement.Testing
RUST_LOG=debug ./script/run --features nld_heuristic_v2RUST_LOG=debug ./script/run --features nld_classifier_v2, nld_heuristic_v2./script/runScreenshots / Videos
Test 543 misfire
read this https://trilogy-eng.atlassian.net/browse/epmlive-17588nld_heuristic_v2andnld_classifier_v2, we could see now classifier could classify this one to promptAgent Mode