Skip to content

fix(qa): 修划词追问浮窗 4 个取消 race (closes #161)#176

Merged
appergb merged 1 commit into
mainfrom
fix/issue-161-qa-cancel-race
May 2, 2026
Merged

fix(qa): 修划词追问浮窗 4 个取消 race (closes #161)#176
appergb merged 1 commit into
mainfrom
fix/issue-161-qa-cancel-race

Conversation

@appergb
Copy link
Copy Markdown
Collaborator

@appergb appergb commented May 2, 2026

摘要

Closes #161

issue #161 4 个子修合 1 PR(紧密耦合的取消通路)。

改动

1. Esc 路由

`coordinator.rs::handle_window_hotkey_event` Esc 分支先查 `qa_state.panel_visible`,是 → `close_qa_panel`;否 → 走 dictation `cancel_session`。修"QA 浮窗按 Esc 杀的是 dictation"。

2. SSE 流取消旗标

`Inner.qa_stream_cancelled: Arc`:begin reset false / cancel set true / polish loop 每帧 `should_cancel()` 检查 → break。修"取消后 LLM 继续 drain 烧 token"。

3. session_id 守卫

`on_delta` 闭包捕获 `captured_session_id`,emit chunk 前比对 `qa_state.session_id`。修"旧会话 chunk 漏进新气泡"。

4. loading 帧

`types.ts::QaStateKind` 加 `'loading'`;`QaPanel.tsx` switch 加 case → setStatus('thinking')。修 ASR finalize 期间 UI 卡 recording 反馈缺失。

API 变化

  • `polish::answer_chat_streaming<F, C>` / `chat_completion_history_streaming<F, C>` 加 `should_cancel`
  • `coordinator::answer_chat_dispatch<F, C>` 同

测试

  • `cargo check` ✅
  • `npm run build` ✅

@codex review。

issue #161 4 个子修复合并为一个 PR(取消相关,紧密耦合)。

## 1. Esc 路由
coordinator.rs::handle_window_hotkey_event 之前无条件 cancel_session(dictation
取消),即使在 QA 浮窗里按 Esc 也杀的是 dictation。现在先查 qa_state.panel_visible
+ phase != Idle,是 → close_qa_panel;否 → 走 dictation 取消。

## 2. SSE 流取消旗标
- Inner 加 qa_stream_cancelled: Arc<AtomicBool>
- begin_qa_session 重置 false
- cancel_qa_session set true
- polish.rs::chat_completion_history_streaming 的 SSE loop 每帧检查
  should_cancel() flag,true 时立即 break,避免取消后 LLM 继续 drain
  HTTP body 烧 token

## 3. session_id 守卫
on_delta 闭包捕获本会话的 captured_session_id;emit chunk 前比对当前
qa_state.session_id,不等就跳过。修复"取消后开新会话,旧 LLM 流的
chunk 漏进新气泡"。

## 4. loading 帧
types.ts::QaStateKind 加 'loading';QaPanel.tsx switch 加 case:把 status
切到 'thinking' 视图,避免 ASR finalize 期间 UI 卡 'recording' 几百 ms
反馈缺失。

## API 变化
- polish::answer_chat_streaming<F, C> 签名加 should_cancel: C
- polish::chat_completion_history_streaming<F, C> 同
- coordinator::answer_chat_dispatch<F, C> 同

## 测试
- cargo check ✅
- npm run build ✅
Copy link
Copy Markdown

@sourcery-ai sourcery-ai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry @appergb, you have reached your weekly rate limit of 500000 diff characters.

Please try again later or upgrade to continue using Sourcery

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: db27107e7a

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

state.selection = None;
}
// 重置 SSE 取消标志:上一轮可能 set 过的 true 留着会让本轮流式立即 break。
inner.qa_stream_cancelled.store(false, Ordering::SeqCst);
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Scope QA stream cancel flag per session

qa_stream_cancelled is 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 to false, 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 👍 / 👎.

Comment on lines +296 to +298
if should_cancel() {
log::info!("[llm] stream cancelled by caller; breaking SSE loop");
break;
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Return cancellation explicitly instead of generic stream break

Breaking the SSE loop on should_cancel() without a cancellation-specific return path means the function can fall through with empty full_text, which is later treated as InvalidResponse("empty stream"). In the QA flow this turns user-initiated cancel/close into an error path (finish_qa_with_error), causing spurious failure handling instead of a silent cancel. The loop should propagate an explicit cancelled outcome so callers can short-circuit normally.

Useful? React with 👍 / 👎.

@appergb appergb merged commit 0c4fbb9 into main May 2, 2026
2 checks passed
@appergb appergb deleted the fix/issue-161-qa-cancel-race branch May 2, 2026 03:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[qa] 划词追问浮窗多处取消 race:Esc 关不掉流 / 烧 token / 跨会话漏字 / loading 帧丢失

1 participant