diff --git a/docs/built-in-web-search.md b/docs/built-in-web-search.md index cf8b58b3..dafad485 100644 --- a/docs/built-in-web-search.md +++ b/docs/built-in-web-search.md @@ -279,6 +279,7 @@ Search uses a **two-stage decision** before spending network or a full answer ca - word list (`latest`, `weather`, `price`, …) or phrase list (`who won`, `right now`, `search for`, …) → **ForceWeb** - current/future year, URL, relative-date math (“how many days until…”) → **ForceWeb** - force-search signals beat skip signals (e.g. “summarise the latest news” still ForceWeb) + - force-search signals apply only to text the user typed: on a turn submitted with an empty ask bar and an auto-captured selection, they yield **Ambiguous** so the classifier judges the highlighted prose instead 4. If nothing matches with certainty → **Ambiguous** (hand to prepass). **Example.** diff --git a/src-tauri/prompts/system_prompt.txt b/src-tauri/prompts/system_prompt.txt index a7fcc7b4..c0882522 100644 --- a/src-tauri/prompts/system_prompt.txt +++ b/src-tauri/prompts/system_prompt.txt @@ -53,6 +53,7 @@ You are activated from the user's workflow. A message may carry up to three sign Read the highlighted text first, then the images for surrounding context, then answer the request with the full picture. - Only highlighted text, no images: answer from your own knowledge. +- Highlighted text, no request: the selection is the subject. Engage it directly: answer it if it asks something, otherwise do what your role calls for with it. Never ask the user what they want done with it. - Only images, no highlighted text: engage the image directly. Answer the question if there is one; otherwise describe what you see and offer useful observations. Never reply "Image received" or ask the user to restate the request. - Neither: a standalone question. Answer it. diff --git a/src-tauri/src/commands.rs b/src-tauri/src/commands.rs index a6d82dfd..9c478518 100644 --- a/src-tauri/src/commands.rs +++ b/src-tauri/src/commands.rs @@ -1066,6 +1066,10 @@ async fn run_builtin_search( warm_state: &crate::warmup::BuiltinWarmState, cache_scope: u64, force_search: bool, + // False when the ask bar was submitted empty and `latest_user` carries only + // the host-app selection, so the pre-filter withholds its deterministic + // force-search shortcut and the classifier decides instead (issue #363). + has_user_request: bool, ) -> BuiltinSearchResult { // The engine is already warm (the caller holds an activity guard); this // re-ensure just reads back the live port for the pre-pass and writer. @@ -1144,6 +1148,7 @@ async fn run_builtin_search( // of the pre-pass decision, with cache read-bypass, write-through // semantics (see `SearchDeps::force_search`). force_search, + has_user_request, // Vision turns only: classifier + writer keep the photo; engines stay text. latest_images, timings: &timing_bag, @@ -2266,6 +2271,28 @@ pub(crate) fn record_conversation_start_if_first_turn( } } +/// Builds the user message content for a turn, labelling highlighted host-app +/// text explicitly so the model treats it as the primary subject and any +/// attached images as surrounding context. +/// +/// A turn may carry a selection with no typed request (issue #363). In that +/// case the highlighted block stands alone: no `[Request]` header is emitted, +/// leaving the system prompt as the sole authority on what to do with the +/// content. Pulled out of [`ask_model`] so the branch is covered by tests +/// instead of the coverage-off Tauri command body. +fn build_user_content(message: String, quoted_text: Option<&str>) -> String { + match quoted_text { + Some(qt) if !qt.trim().is_empty() => { + if message.trim().is_empty() { + format!("[Highlighted Text]\n\"{}\"", qt) + } else { + format!("[Highlighted Text]\n\"{}\"\n\n[Request]\n{}", qt, message) + } + } + _ => message, + } +} + /// Streams a chat response from the local Ollama backend. Appends the user /// message and assistant response to conversation history after completion /// or cancellation (retaining context for follow-up requests). Uses an epoch @@ -2392,15 +2419,17 @@ pub async fn ask_model( // high-precision gate anyway. let clock_probe = message.clone(); + // Whether the user typed anything this turn. An empty ask bar submitted with + // an auto-captured selection (issue #363) sends only the highlighted text, + // so the built-in search pre-filter must not read that prose as the user's + // own freshness intent (see `websearch::prefilter::prefilter`). Captured + // before `message` moves into the wrapper below, which would hide it. + let has_user_request = !message.trim().is_empty(); + // Build user message content. When quoted text is present, label it // explicitly so the model knows the highlighted text is the primary // subject and any attached images provide surrounding context. - let content = match quoted_text { - Some(ref qt) if !qt.trim().is_empty() => { - format!("[Highlighted Text]\n\"{}\"\n\n[Request]\n{}", qt, message) - } - _ => message, - }; + let content = build_user_content(message, quoted_text.as_deref()); // Emit UserMessage before any image base64 work, so the trace // captures the user's intent even if encoding fails. Image paths @@ -2655,6 +2684,7 @@ pub async fn ask_model( &warm_state, epoch_at_start, force, + has_user_request, ) .await } @@ -2944,6 +2974,39 @@ mod tests { time::macros::datetime!(2026-07-10 01:15:30 UTC) } + // ── user message content ────────────────────────────────────────────── + + #[test] + fn build_user_content_wraps_quote_and_request() { + assert_eq!( + build_user_content("summarize this".to_string(), Some("some page text")), + "[Highlighted Text]\n\"some page text\"\n\n[Request]\nsummarize this" + ); + } + + #[test] + fn build_user_content_omits_request_header_for_empty_message() { + assert_eq!( + build_user_content(String::new(), Some("some page text")), + "[Highlighted Text]\n\"some page text\"" + ); + assert_eq!( + build_user_content(" \n".to_string(), Some("some page text")), + "[Highlighted Text]\n\"some page text\"" + ); + } + + #[test] + fn build_user_content_passes_message_through_without_quote() { + assert_eq!( + build_user_content("plain turn".to_string(), None), + "plain turn" + ); + // Blank quote is treated as absent; an images-only turn keeps its + // empty message untouched. + assert_eq!(build_user_content(String::new(), Some(" \n ")), ""); + } + #[test] fn builtin_search_outcome_label_covers_all_variants() { use crate::websearch::orchestrator::SearchOutcome; diff --git a/src-tauri/src/websearch/orchestrator.rs b/src-tauri/src/websearch/orchestrator.rs index 3fda9d0b..d2203f78 100644 --- a/src-tauri/src/websearch/orchestrator.rs +++ b/src-tauri/src/websearch/orchestrator.rs @@ -281,6 +281,14 @@ pub struct SearchDeps<'a> { /// `local_zone` does: it is a per-turn caller input the call sites should not /// each have to thread. pub force_search: bool, + /// Whether the user typed a request this turn. `false` when the ask bar was + /// submitted empty and `latest_user` is only the auto-captured host-app + /// selection (issue #363). Passed to the pre-filter, where it withholds the + /// deterministic force-search shortcut from text the user did not write (see + /// [`crate::websearch::prefilter::prefilter`]); the turn still reaches the + /// classifier, so auto-search stays available on it. Rides in `deps` for the + /// same reason `force_search` does. + pub has_user_request: bool, /// Base64 image payloads for the latest user turn when the active model is /// vision-capable. Passed to the classifier and re-attached on the writer /// so grounded answers keep the photo. `None` or empty keeps the text-only @@ -397,7 +405,7 @@ async fn run_search_inner( } // Stage one: deterministic pre-filter, no model call. - let verdict = prefilter(latest_user, today); + let verdict = prefilter(latest_user, today, deps.has_user_request); eprintln!("[search] prefilter={verdict:?}"); // The `/search` command forces a search even when the pre-filter would // force-skip (e.g. a message that reads like a greeting): the user asked @@ -3048,6 +3056,7 @@ mod tests { // through, so tests here run without one (date-only event lines). local_zone: None, force_search: false, + has_user_request: true, latest_images: None, timings: Box::leak(Box::new(TimingBag::new())), } @@ -3089,6 +3098,7 @@ mod tests { ))), local_zone: None, force_search: false, + has_user_request: true, latest_images: None, timings: Box::leak(Box::new(TimingBag::new())), } @@ -3126,6 +3136,7 @@ mod tests { web_cache, local_zone: None, force_search: false, + has_user_request: true, latest_images: None, timings: Box::leak(Box::new(TimingBag::new())), } @@ -3169,6 +3180,7 @@ mod tests { ))), local_zone: None, force_search: false, + has_user_request: true, latest_images: None, timings: Box::leak(Box::new(TimingBag::new())), } @@ -3602,6 +3614,75 @@ mod tests { ); } + #[tokio::test] + async fn no_user_request_hands_a_force_web_turn_to_the_classifier() { + // Same "latest ..." text as the test above, but submitted with an empty + // ask bar (issue #363): the freshness word is the highlighted author's, + // not the user's, so the classifier decides and its `no` now stands. + // The Deciding phase proves this is the classifier route, not a skip. + let prepass = FakePrePass::returning(Ok(PrePassDecision { + decision: SearchDecision::No, + route: SearchRoute::Web, + standalone_question: "when was the treaty of versailles signed in paris".into(), + queries: vec![], + explicit_search: false, + lang: "en".into(), + })); + let transport = transport_with_serp_and_page(); + let (phases, status) = recorder(); + let mut deps = deps(&prepass, &transport, &Bm25Scorer); + deps.has_user_request = false; + let outcome = run_search( + &deps, + "sys", + &[], + "the latest on the treaty", + 16384, + "2026-07-05", + "en-US", + &CancellationToken::new(), + &status, + ) + .await; + assert!(matches!(outcome, SearchOutcome::NoSearch)); + assert_eq!(*phases.lock().unwrap(), vec![SearchPhase::Deciding]); + } + + #[tokio::test] + async fn no_user_request_still_searches_when_the_classifier_says_web() { + // Auto-search stays reachable on a no-request turn: withholding the + // deterministic shortcut moves the decision to the model, it does not + // close the web off. + let prepass = FakePrePass::returning(Ok(web_decision(vec![ + "when was the treaty of versailles signed in paris", + ]))); + let transport = transport_with_serp_and_page(); + let (phases, status) = recorder(); + let mut deps = deps(&prepass, &transport, &Bm25Scorer); + deps.has_user_request = false; + let outcome = run_search( + &deps, + "sys", + &[], + "the latest on the treaty", + 16384, + "2026-07-05", + "en-US", + &CancellationToken::new(), + &status, + ) + .await; + assert!(matches!(outcome, SearchOutcome::Answer { .. })); + assert_eq!( + *phases.lock().unwrap(), + vec![ + SearchPhase::Deciding, + SearchPhase::Searching, + SearchPhase::Reading + ] + ); + } + #[tokio::test] async fn cancel_during_page_fetch_yields_cancelled() { // A cancel raised WHILE the page-fetch stage is awaiting (after the diff --git a/src-tauri/src/websearch/prefilter.rs b/src-tauri/src/websearch/prefilter.rs index 5fd6b8fa..e0539cc5 100644 --- a/src-tauri/src/websearch/prefilter.rs +++ b/src-tauri/src/websearch/prefilter.rs @@ -27,6 +27,12 @@ //! over skip signals, so "summarise the latest news" searches rather than being //! caught by the "summarise" transform rule. //! +//! Force-search signals only speak for the user when the user wrote them. A turn +//! carrying no typed request (an empty ask bar submitted with an auto-captured +//! host-app selection) is scanned prose, not an expressed intent, so those +//! signals resolve to `Ambiguous` there and a model decides whether the content +//! needs fresh data. See `has_user_request` on [`prefilter`]. +//! //! The scan is bounded ([`PREFILTER_MAX_SCAN_CHARS`]) and tokenised in a single //! linear pass with no backtracking, so a pathologically large pasted message //! cannot turn the per-turn decision into a CPU denial-of-service. @@ -314,8 +320,11 @@ const MAX_CLOCK_QUESTION_TOKENS: usize = 10; /// Resolves the deterministic verdict for `message`. `today` is the `YYYY-MM-DD` /// date string used to recognise current-or-future year tokens as a freshness -/// signal. Pure and total: any input yields a verdict. -pub fn prefilter(message: &str, today: &str) -> PreFilterVerdict { +/// signal. `has_user_request` is `false` on a turn where the user typed nothing +/// and the message is only the auto-captured host-app selection (issue #363): +/// see the force-search branch below for what that changes. Pure and total: any +/// input yields a verdict. +pub fn prefilter(message: &str, today: &str, has_user_request: bool) -> PreFilterVerdict { // Bound the scan so tokenisation cost is a small constant regardless of a // hostile or accidentally huge pasted message. let bounded: String = message @@ -345,9 +354,21 @@ pub fn prefilter(message: &str, today: &str) -> PreFilterVerdict { return PreFilterVerdict::ForceNo; } - // Force-search signals win over every skip rule. + // Force-search signals win over every skip rule, but only when the user + // actually wrote the text they appear in. On a no-request turn the scanned + // text is host-app prose the user merely highlighted, where a link, a year, + // or a word like "latest" is the author's wording, not the user's intent, so + // the deterministic shortcut is handed back to the classifier. Deliberately + // `Ambiguous` and not a fall-through: auto-search must stay reachable on + // these turns, so this never reaches the skip rules below. (A turn with + // neither a request nor a selection cannot arrive here: an empty message + // resolves at the `tokens.is_empty()` check above.) if has_force_web_signal(&bounded, &normalised, &tokens, today) { - return PreFilterVerdict::ForceWeb; + return if has_user_request { + PreFilterVerdict::ForceWeb + } else { + PreFilterVerdict::Ambiguous + }; } if is_greeting_or_ack(&tokens) || is_pure_math(&bounded, &tokens) || has_transform_lead(&tokens) @@ -567,7 +588,14 @@ mod tests { const TODAY: &str = "2026-07-07"; fn verdict(message: &str) -> PreFilterVerdict { - prefilter(message, TODAY) + prefilter(message, TODAY, true) + } + + /// Same turn text, but submitted with no typed request (the empty-ask-bar + + /// selection path). Paired with [`verdict`] so a no-request assertion is + /// always an A/B against the identical string. + fn verdict_no_request(message: &str) -> PreFilterVerdict { + prefilter(message, TODAY, false) } // ── the three live-smoke failures, pinned deterministically ─────────────── @@ -667,6 +695,59 @@ mod tests { ); } + // ── no typed request (empty ask bar + host-app selection, issue #363) ───── + // + // The same text is asserted both ways: with a typed request the force-web + // shortcut still fires; with none it must reach the classifier instead. + + #[test] + fn force_web_words_still_fire_with_a_typed_request() { + for m in [ + "what is the latest release", + "see https://example.com/article", + "the roadmap for 2026", + ] { + assert_eq!(verdict(m), PreFilterVerdict::ForceWeb, "{m}"); + } + } + + #[test] + fn force_web_words_are_ambiguous_without_a_typed_request() { + // Word, URL, and year arms of `has_force_web_signal`, all handed to the + // classifier when the user typed nothing. + for m in [ + "what is the latest release", + "see https://example.com/article", + "the roadmap for 2026", + ] { + assert_eq!(verdict_no_request(m), PreFilterVerdict::Ambiguous, "{m}"); + } + } + + #[test] + fn no_typed_request_never_hard_skips_a_force_web_turn() { + // The point of the no-request route is that a model still decides, so + // this may never collapse into a deterministic skip. + assert_ne!( + verdict_no_request("summarize the latest news on the merger"), + PreFilterVerdict::ForceNo + ); + assert_eq!( + verdict_no_request("summarize the latest news on the merger"), + PreFilterVerdict::Ambiguous + ); + } + + #[test] + fn highlighted_selection_without_a_request_is_ambiguous() { + // The exact content `commands::build_user_content` composes when the ask + // bar is empty and a host-app selection is attached. + let msg = + "[Highlighted Text]\n\"The latest figures were published on www.example.com in 2026.\""; + assert_eq!(verdict(msg), PreFilterVerdict::ForceWeb); + assert_eq!(verdict_no_request(msg), PreFilterVerdict::Ambiguous); + } + // ── relative-date-arithmetic signals ────────────────────────────────────── // // Live-smoke regression (2026-07-11): "how many days until christmas" fell @@ -721,11 +802,11 @@ mod tests { fn year_signal_uses_today_not_a_hardcoded_year() { // With a 2020 "today", 2026 is future -> force; 2019 is past -> not. assert_eq!( - prefilter("outlook for 2026", "2020-01-01"), + prefilter("outlook for 2026", "2020-01-01", true), PreFilterVerdict::ForceWeb ); assert_eq!( - prefilter("what happened in 2019", "2020-01-01"), + prefilter("what happened in 2019", "2020-01-01", true), PreFilterVerdict::Ambiguous ); } @@ -735,11 +816,11 @@ mod tests { // A non-date `today` cannot yield a year, so the year rule is inert, but // other signals still fire. assert_eq!( - prefilter("outlook for 2027", "not-a-date"), + prefilter("outlook for 2027", "not-a-date", true), PreFilterVerdict::Ambiguous ); assert_eq!( - prefilter("tokyo weather", "not-a-date"), + prefilter("tokyo weather", "not-a-date", true), PreFilterVerdict::ForceWeb ); } @@ -961,14 +1042,14 @@ mod tests { // prefix is plain filler text -> falls through to the classifier. let mut msg = "a".repeat(PREFILTER_MAX_SCAN_CHARS); msg.push_str(" weather"); - assert_eq!(prefilter(&msg, TODAY), PreFilterVerdict::Ambiguous); + assert_eq!(prefilter(&msg, TODAY, true), PreFilterVerdict::Ambiguous); } #[test] fn huge_input_is_handled_in_bounded_time() { // Sanity: a multi-megabyte message returns without scanning all of it. let msg = "latest ".to_string() + &"x".repeat(4_000_000); - assert_eq!(prefilter(&msg, TODAY), PreFilterVerdict::ForceWeb); + assert_eq!(prefilter(&msg, TODAY, true), PreFilterVerdict::ForceWeb); } // ── curated eval corpus (the measurement instrument) ────────────────────── @@ -997,7 +1078,7 @@ mod tests { #[test] fn prefilter_never_contradicts_a_labelled_row() { for row in eval_rows() { - let v = prefilter(&row.message, TODAY); + let v = prefilter(&row.message, TODAY, true); // A should-search row may never be force-skipped; a should-not-search // row may never be force-searched. Label validity itself is checked in // `corpus_is_a_meaningful_size_and_balance`. @@ -1048,7 +1129,7 @@ mod tests { let total = search.len(); let forced = search .iter() - .filter(|r| prefilter(&r.message, TODAY) == PreFilterVerdict::ForceWeb) + .filter(|r| prefilter(&r.message, TODAY, true) == PreFilterVerdict::ForceWeb) .count(); assert!( forced * 10 >= total * 6, @@ -1074,7 +1155,7 @@ mod tests { .collect(); assert!(!rows.is_empty(), "expected non-English rows in the corpus"); for row in rows { - let v = prefilter(&row.message, TODAY); + let v = prefilter(&row.message, TODAY, true); let forbidden = if row.label == "search" { PreFilterVerdict::ForceNo } else { diff --git a/src-tauri/tests/live_answer_capture.rs b/src-tauri/tests/live_answer_capture.rs index 5b38d808..601623f5 100644 --- a/src-tauri/tests/live_answer_capture.rs +++ b/src-tauri/tests/live_answer_capture.rs @@ -159,6 +159,8 @@ async fn live_turn( web_cache: &web_cache, local_zone: None, force_search: false, + // Eval turns are typed queries, never a bare host-app selection. + has_user_request: true, latest_images: None, timings: &timings, }; diff --git a/src-tauri/tests/live_answer_quality_eval.rs b/src-tauri/tests/live_answer_quality_eval.rs index a86aa7f1..c5c60812 100644 --- a/src-tauri/tests/live_answer_quality_eval.rs +++ b/src-tauri/tests/live_answer_quality_eval.rs @@ -897,6 +897,8 @@ async fn live_answer_for( web_cache: &web_cache, local_zone: None, force_search: false, + // Eval turns are typed queries, never a bare host-app selection. + has_user_request: true, latest_images: None, timings: &timings, }; diff --git a/src-tauri/tests/live_cache_reuse_repro.rs b/src-tauri/tests/live_cache_reuse_repro.rs index b7e53545..588b7580 100644 --- a/src-tauri/tests/live_cache_reuse_repro.rs +++ b/src-tauri/tests/live_cache_reuse_repro.rs @@ -330,6 +330,8 @@ async fn cache_reuse_repro_delivers_birthdate_to_the_reuse_gate() { web_cache: &web_cache, local_zone: None, force_search: false, + // Eval turns are typed queries, never a bare host-app selection. + has_user_request: true, latest_images: None, timings: &timings, }; diff --git a/src-tauri/tests/live_classifier_eval.rs b/src-tauri/tests/live_classifier_eval.rs index ec4e66c4..e745f3a7 100644 --- a/src-tauri/tests/live_classifier_eval.rs +++ b/src-tauri/tests/live_classifier_eval.rs @@ -48,7 +48,8 @@ async fn would_search( message: &str, today: &str, ) -> (bool, &'static str) { - match prefilter(message, today) { + // Eval turns are typed queries: `has_user_request` is always true. + match prefilter(message, today, true) { PreFilterVerdict::ForceNo => (false, "prefilter"), PreFilterVerdict::ForceWeb => (true, "prefilter"), PreFilterVerdict::Ambiguous => { diff --git a/src-tauri/tests/live_language_parity_eval.rs b/src-tauri/tests/live_language_parity_eval.rs index 34f7b573..322ff79b 100644 --- a/src-tauri/tests/live_language_parity_eval.rs +++ b/src-tauri/tests/live_language_parity_eval.rs @@ -172,7 +172,8 @@ fn true_lang(row: &EvalRow) -> Option<&str> { /// The production two-stage decision collapsed to "would this turn search?", /// against the BRANCH classifier (today's real [`BuiltinPrePass`]). async fn branch_is_search(prepass: &BuiltinPrePass, message: &str, today: &str) -> bool { - match prefilter(message, today) { + // Eval turns are typed queries: `has_user_request` is always true. + match prefilter(message, today, true) { PreFilterVerdict::ForceNo => false, PreFilterVerdict::ForceWeb => true, PreFilterVerdict::Ambiguous => match prepass @@ -190,7 +191,8 @@ async fn branch_is_search(prepass: &BuiltinPrePass, message: &str, today: &str) /// Against BASELINE. async fn baseline_would_search(base_url: &str, message: &str, today: &str) -> bool { - match prefilter(message, today) { + // Eval turns are typed queries: `has_user_request` is always true. + match prefilter(message, today, true) { PreFilterVerdict::ForceNo => false, PreFilterVerdict::ForceWeb => true, PreFilterVerdict::Ambiguous => baseline_is_search(base_url, message, today).await, diff --git a/src-tauri/tests/live_search_smoke.rs b/src-tauri/tests/live_search_smoke.rs index 21f61425..80ee547e 100644 --- a/src-tauri/tests/live_search_smoke.rs +++ b/src-tauri/tests/live_search_smoke.rs @@ -165,6 +165,8 @@ async fn live_turn_with_lang( web_cache: &web_cache, local_zone: None, force_search: false, + // Eval turns are typed queries, never a bare host-app selection. + has_user_request: true, latest_images: None, timings: &timings, }; diff --git a/src/App.tsx b/src/App.tsx index fa2c0fdb..20b28ed4 100644 --- a/src/App.tsx +++ b/src/App.tsx @@ -3207,8 +3207,15 @@ function App() { }, [isExportOpen]); const handleSubmit = useCallback(() => { + // Sanitized selection, resolved up front: it decides whether an empty ask + // bar still has something to send. The backend only ever receives this + // value, so every gate below reads it rather than the raw selection. + const context = sanitizeContext(selectedContext, quote.maxContextLength); + if ( - (query.trim().length === 0 && attachedImages.length === 0) || + (query.trim().length === 0 && + attachedImages.length === 0 && + context === undefined) || isGenerating || isSubmitPending ) @@ -3277,10 +3284,6 @@ function App() { if (hasSearch) { const searchQuery = strippedMessage.trim(); if (!searchQuery) return; - const searchContext = sanitizeContext( - selectedContext, - quote.maxContextLength, - ); // Bubble shows the literal typed text (with `/search`); backend gets the // stripped query without the trigger prefix. Images use resolved paths only. const searchDisplay = trimmedQuery; @@ -3294,35 +3297,40 @@ function App() { URL.revokeObjectURL(img.blobUrl); } setAttachedImages([]); - void askSearch(searchQuery, searchDisplay, searchContext, searchImages); + void askSearch(searchQuery, searchDisplay, context, searchImages); return; } // Nothing to send if the message is only commands with no content or images. // Utility triggers are excluded: they fall through to their own block below // which shakes + shows an error when no input is found. - // Exception: /think with pre-filled selected context is valid. + // Exception: pre-filled selected context is content in its own right, so + // it carries a submit that has no typed text (with or without /think). if ( !strippedMessage && attachedImages.length === 0 && !hasScreen && !utilityTrigger && - !(hasThink && selectedContext?.trim()) + !context ) return; // Maintain sticky rewrite mode. A replaceable command (re)starts it; any // other command exits it; a plain follow-up leaves it intact so its - // refinement inherits the Replace button through `executeSubmit`. Search - // turns have already returned above, so `/search` needs no branch here. + // refinement inherits the Replace button through `executeSubmit`. A + // selection-only submit (no typed text, no command, no images) also exits + // it: that turn is a fresh question about the selection, not a refinement, + // so inheriting a stale `/rewrite` would let auto-replace overwrite the + // user's selection in the source app with a free-form answer. Search turns + // have already returned above, so `/search` needs no branch here. if (utilityTrigger && REPLACEABLE_COMMANDS.has(utilityTrigger)) { stickyReplaceCommandRef.current = utilityTrigger; } else if (utilityTrigger || hasScreen || hasThink || hasExtract) { stickyReplaceCommandRef.current = null; + } else if (!strippedMessage && attachedImages.length === 0) { + stickyReplaceCommandRef.current = null; } - const context = sanitizeContext(selectedContext, quote.maxContextLength); - // Unified pre-flight pending-images gate. Every command that needs // resolved image paths waits here: /extract, /screen, utility-OCR, and // plain submit. If any attached image is still processing, store the diff --git a/src/__tests__/App.test.tsx b/src/__tests__/App.test.tsx index 3b8c3ed9..96a6c903 100644 --- a/src/__tests__/App.test.tsx +++ b/src/__tests__/App.test.tsx @@ -1984,6 +1984,47 @@ describe('App', () => { expect(invoke).not.toHaveBeenCalledWith('ask_model', expect.anything()); }); + it('submits an empty query when selected context is attached', async () => { + render(); + await act(async () => {}); + + await showOverlay('selected snippet'); + + const textarea = getAskInput(); + + // Press Enter with an empty textarea: the selection is the whole message. + act(() => { + fireEvent.keyDown(textarea, { key: 'Enter', shiftKey: false }); + }); + + await act(async () => {}); + + expect(invoke).toHaveBeenCalledWith( + 'ask_model', + expect.objectContaining({ + message: '', + quotedText: 'selected snippet', + }), + ); + }); + + it('does not submit an empty query when the selected context is blank', async () => { + render(); + await act(async () => {}); + + await showOverlay(' \n '); + + const textarea = getAskInput(); + + act(() => { + fireEvent.keyDown(textarea, { key: 'Enter', shiftKey: false }); + }); + + await act(async () => {}); + + expect(invoke).not.toHaveBeenCalledWith('ask_model', expect.anything()); + }); + it('lets the user keep drafting while a response streams, without sending', async () => { enableChannelCapture(); render(); @@ -2442,6 +2483,102 @@ describe('App', () => { ); }); + it('drops sticky rewrite mode on a selection-only empty submit', async () => { + // A deferred /rewrite whose image fails restores both the query and the + // selection, leaving sticky rewrite mode armed with a live selection. + // Submitting the bare selection after that is a fresh question, so it must + // not inherit the rewrite and auto-replace over the user's selection. + let rejectSave: ((err: Error) => void) | null = null; + const savePromise = new Promise((_, reject) => { + rejectSave = reject; + }); + // The app only attaches its handler once it awaits the pending save, so + // this no-op keeps the deliberate rejection from surfacing as unhandled. + void savePromise.catch(() => {}); + enableChannelCaptureWithResponses({ save_image_command: savePromise }); + + render( + + + , + ); + await act(async () => {}); + await showOverlay('draft email text'); + + const textarea = getAskInput(); + const file = new File(['data'], 'img.png', { type: 'image/png' }); + await act(async () => { + fireEvent.paste(textarea, { + clipboardData: { + getData: () => '', + items: [{ type: 'image/png', getAsFile: () => file }], + }, + }); + }); + await vi.waitFor(() => { + expect( + screen.getByRole('list', { name: /attached images/i }), + ).toBeInTheDocument(); + }); + await act(async () => { + await vi.waitFor(() => { + expect(rejectSave).not.toBeNull(); + }); + }); + + // Arms sticky rewrite mode, then defers on the still-processing image. + act(() => { + setAskValue('/rewrite '); + }); + act(() => { + fireEvent.keyDown(textarea, { key: 'Enter', shiftKey: false }); + }); + + // The image fails: query and selection come back, sticky mode stays armed. + await act(async () => { + rejectSave!(new Error('disk full')); + }); + await waitFor(() => expect(getAskInput().textContent).toBe('/rewrite')); + + // Clear the ask bar: the restored selection alone carries this submit. + act(() => { + setAskValue(''); + }); + act(() => { + fireEvent.keyDown(textarea, { key: 'Enter', shiftKey: false }); + }); + await act(async () => {}); + act(() => { + getLastChannel()?.simulateMessage({ + type: 'Token', + data: 'Here is what the selection says', + }); + getLastChannel()?.simulateMessage({ type: 'Done' }); + }); + await act(async () => {}); + + expect( + screen.queryAllByLabelText('Replace selection in source app'), + ).toHaveLength(0); + expect(invoke).not.toHaveBeenCalledWith( + 'replace_selection', + expect.anything(), + ); + }); + it('drops the Replace button when a different command interrupts the rewrite session', async () => { enableChannelCaptureWithResponses({ get_model_picker_state: { @@ -9071,6 +9208,34 @@ describe('App', () => { ).toHaveLength(1); }); + it('sends the sanitized selection as the /search quoted text', async () => { + enableChannelCapture(); + render(); + await act(async () => {}); + // Longer than the 4096-char cap, so the assertion below fails if the + // /search branch ever forwards the raw selection instead of the + // sanitized one. + const longSelection = 'selection '.repeat(500); + await showOverlay(longSelection); + + const textarea = getAskInput(); + act(() => { + setAskValue('/search explain this selection'); + }); + await act(async () => { + fireEvent.keyDown(textarea, { key: 'Enter', shiftKey: false }); + }); + + expect(invoke).toHaveBeenCalledWith( + 'ask_model', + expect.objectContaining({ + message: 'explain this selection', + quotedText: longSelection.slice(0, 4096), + forceSearch: true, + }), + ); + }); + it('completes force-search as one-shot then routes a plain follow-up to ask_model without forceSearch', async () => { enableChannelCapture(); render(); diff --git a/src/hooks/__tests__/useModel.test.tsx b/src/hooks/__tests__/useModel.test.tsx index ef42d328..ede2ad6b 100644 --- a/src/hooks/__tests__/useModel.test.tsx +++ b/src/hooks/__tests__/useModel.test.tsx @@ -816,6 +816,41 @@ describe('useModel', () => { expect(result.current.messages).toHaveLength(0); }); + it('allows ask() with empty text but a non-blank quotedText', async () => { + const { result } = renderHook(() => useModel('')); + + await act(async () => { + await result.current.ask('', 'selected page text'); + }); + + expect(result.current.messages).toHaveLength(2); + expect(result.current.messages[0]).toEqual( + expect.objectContaining({ + role: 'user', + content: '', + quotedText: 'selected page text', + }), + ); + expect(invoke).toHaveBeenCalledWith( + 'ask_model', + expect.objectContaining({ + message: '', + quotedText: 'selected page text', + }), + ); + }); + + it('returns early for empty text AND whitespace-only quotedText', async () => { + const { result } = renderHook(() => useModel('')); + + await act(async () => { + await result.current.ask('', ' \n '); + }); + + expect(invoke).not.toHaveBeenCalled(); + expect(result.current.messages).toHaveLength(0); + }); + it('includes imagePaths in message and invoke when text AND imagePaths are provided', async () => { const { result } = renderHook(() => useModel('')); diff --git a/src/hooks/useModel.ts b/src/hooks/useModel.ts index 84f498fd..5681a583 100644 --- a/src/hooks/useModel.ts +++ b/src/hooks/useModel.ts @@ -433,7 +433,11 @@ export function useModel( forceSearch?: boolean, slashCommand?: string, ) => { - if (!displayContent.trim() && (!imagePaths || imagePaths.length === 0)) { + if ( + !displayContent.trim() && + (!imagePaths || imagePaths.length === 0) && + !quotedText?.trim() + ) { return; } diff --git a/src/view/AskBarView.tsx b/src/view/AskBarView.tsx index 51a84ee6..2f35c26c 100644 --- a/src/view/AskBarView.tsx +++ b/src/view/AskBarView.tsx @@ -344,7 +344,9 @@ export function AskBarView({ downloadStatus?.kind === 'verifying' || downloadStatus?.kind === 'paused'; const canSubmit = - (query.trim().length > 0 || attachedImages.length > 0) && + (query.trim().length > 0 || + attachedImages.length > 0 || + Boolean(selectedText?.trim())) && !isBusy && !(isDownloadHolding && !hasUsableModel); const isAtMaxImages = attachedImages.length >= maxImages; diff --git a/src/view/__tests__/AskBarView.test.tsx b/src/view/__tests__/AskBarView.test.tsx index c676b676..32ee5126 100644 --- a/src/view/__tests__/AskBarView.test.tsx +++ b/src/view/__tests__/AskBarView.test.tsx @@ -427,6 +427,58 @@ describe('AskBarView', () => { ).toBeInTheDocument(); }); + it('disables the send button with an empty query and no selected text', () => { + render( + , + ); + expect(screen.getByRole('button', { name: 'Send message' })).toBeDisabled(); + }); + + it('enables the send button with an empty query when selected text is attached', () => { + render( + , + ); + expect( + screen.getByRole('button', { name: 'Send message' }), + ).not.toBeDisabled(); + }); + + it('keeps the send button disabled when the selected text is blank', () => { + render( + , + ); + expect(screen.getByRole('button', { name: 'Send message' })).toBeDisabled(); + }); + it('renders a model picker trigger in ask-bar mode when models are available', () => { render(