-
Notifications
You must be signed in to change notification settings - Fork 129
fix(qa): 修划词追问浮窗 4 个取消 race (closes #161) #176
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -104,21 +104,23 @@ impl OpenAICompatibleLLMProvider { | |
| /// 最后一条必须是新一轮的 user 提问。第一条 user 消息里如果有选区,调用方应在 | ||
| /// content 里就把选区原文注入。`on_delta` 在每个 SSE chunk 到达时被调;最终返回 | ||
| /// 拼好的完整字符串(用于写入 messages 历史)。详见 issue #118 v2。 | ||
| pub async fn answer_chat_streaming<F>( | ||
| pub async fn answer_chat_streaming<F, C>( | ||
| &self, | ||
| messages: &[QaChatMessage], | ||
| working_languages: &[String], | ||
| front_app: Option<&str>, | ||
| on_delta: F, | ||
| should_cancel: C, | ||
| ) -> Result<String, LLMError> | ||
| where | ||
| F: Fn(&str) + Send + Sync, | ||
| C: Fn() -> bool + Send + Sync, | ||
| { | ||
| let mut system_prompt = prompts::qa_system_prompt(); | ||
| if let Some(premise) = context_premise(working_languages, front_app) { | ||
| system_prompt = format!("{}\n\n{}", premise, system_prompt); | ||
| } | ||
| self.chat_completion_history_streaming(&system_prompt, messages, on_delta) | ||
| self.chat_completion_history_streaming(&system_prompt, messages, on_delta, should_cancel) | ||
| .await | ||
| } | ||
|
|
||
|
|
@@ -209,14 +211,16 @@ impl OpenAICompatibleLLMProvider { | |
| /// 与 `chat_completion` 同条 HTTP 通路,但开 `stream: true` 并把 SSE chunk 一边 | ||
| /// 解析、一边通过 `on_delta` 推给调用方(用于实时把答案塞进浮窗气泡)。 | ||
| /// 最终返回拼好的完整字符串供调用方写入对话历史。 | ||
| async fn chat_completion_history_streaming<F>( | ||
| async fn chat_completion_history_streaming<F, C>( | ||
| &self, | ||
| system_prompt: &str, | ||
| history: &[QaChatMessage], | ||
| on_delta: F, | ||
| should_cancel: C, | ||
| ) -> Result<String, LLMError> | ||
| where | ||
| F: Fn(&str) + Send + Sync, | ||
| C: Fn() -> bool + Send + Sync, | ||
| { | ||
| if self.config.api_key.trim().is_empty() { | ||
| return Err(LLMError::MissingCredentials); | ||
|
|
@@ -287,6 +291,12 @@ impl OpenAICompatibleLLMProvider { | |
| let mut buffer = String::new(); | ||
| let mut full_text = String::new(); | ||
| loop { | ||
| // 取消旗标:用户取消 / 关浮窗时立即 break,不再 drain HTTP body。 | ||
| // 否则 reqwest 会读完整个流(包括 LLM 后续 token)烧 quota。详见 issue #161。 | ||
| if should_cancel() { | ||
| log::info!("[llm] stream cancelled by caller; breaking SSE loop"); | ||
| break; | ||
|
Comment on lines
+296
to
+298
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Breaking the SSE loop on Useful? React with 👍 / 👎. |
||
| } | ||
| let chunk_opt = response | ||
| .chunk() | ||
| .await | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
qa_stream_cancelledis shared across all QA sessions, and this reset runs at the start of every new session. If the user closes QA (sets cancel=true) and quickly starts another session before the previous streaming task exits, this line flips the old task back tofalse, so the previous SSE loop keeps draining tokens again. That reintroduces the cancellation race this patch is trying to fix; the cancel signal should be session-scoped (e.g., token/id) rather than a single global boolean.Useful? React with 👍 / 👎.