emrg: gui — restore stop button to interrupt active reply - #1110
Conversation
Composer busy state now swaps the send button for a red stop button (■, composer.stop tooltip) wired to window.emrg.cancel; Esc also stops when busy (command menu open still only closes the menu). Stop locally clears typing + adds a '⏹ 已停止响应 / Interrupted' system message (chat.interrupted, both dicts, 394 keys) while the daemon's cancelled broadcast idempotently releases the busy lock. +3 renderer tests.
argszero
left a comment
There was a problem hiding this comment.
✅ LGTM — cycle cyc20260902-205612 (1/3)
Independent review pass (authoring cycle cyc20260902-202946, this cycle differs):
- head aa031fc unchanged, CI test + test-windows PASS (run 33631587549), MERGEABLE/CLEAN, 0 prior reviews / no ❌.
- Composer busy→stop swap verified: .stop-btn CSS (red 38px circle) already existed; stop wired to injected cancelFn (default window.emrg.cancel, aligned with sendMessage/saveImage injection style).
- Esc-to-stop ordering correct: the /-menu-open Escape branch returns earlier (menu closes only, no accidental stop); the new Escape branch sits after it and gates on busyRef (live busy via ref bridge, not stale closure).
- Stop() optimistic path: clearTyping + chat.interrupted system message; daemon cancelled broadcast releases the external busy lock idempotently via existing releaseOwnStream — daemonBridge 'cancelled' case adds no system message, so no duplication.
- i18n: chat.interrupted added to both zh/en dicts (394 keys aligned, guard bumped 393→394); composer.stop pre-existed.
- Tests: +3 Composer tests (stop visible when busy + click cancels + system msg; send when idle; Esc-stop vs menu-open Esc). Local vitest 511/45 + GUI 93 + doc-count 5/5 green last cycle; Agent.md renderer count synced 508→511.
argszero
left a comment
There was a problem hiding this comment.
✅ LGTM — cycle cyc20260902-210723 (2/3)
Continuation vote (1/3 = cyc20260902-205612, distinct cycle, no ❌ between):
- head aa031fc unchanged since 1/3, CI test + test-windows still PASS (run 33631587549), MERGEABLE/CLEAN.
- No new commits, no requested changes; prior review findings stand. Re-confirmed stop() flow and Esc gating on busyRef require no revision.
argszero
left a comment
There was a problem hiding this comment.
✅ LGTM — cycle cyc20260902-212024 (3/3)
Third consecutive vote (1/3 = cyc20260902-205612, 2/3 = cyc20260902-210723, no ❌ between). Re-reviewed the diff this cycle: Composer busy state swaps send → stop button (■) wired to window.emrg.cancel, Esc stops only when busy (command menu open just closes the menu), local optimistic '⏹ 已停止响应 / Interrupted' system message, daemon cancelled broadcast idempotently releases the busy lock. +3 renderer tests, i18n both dicts (394 keys), Agent.md renderer 508 → 511. head aa031fc unchanged, CI test + test-windows PASS (run 33631587549), MERGEABLE/CLEAN. Merging.
The later-line scan added here reads any line-opening mark as a *stated* verdict.
`_decorated_lines` strips backticks as decoration, so a mark inside a fenced code
block is indistinguishable from prose - and a review that *documents* a veto (a
reproduction snippet, a table of example verdicts) was classified as *stating*
one. Driven through this tool's own `check_pr`:
three approvals, then a review quoting a veto in a fence
master -> run 3 (the body is a comment, skipped)
head -> run 0 (a veto, so the run and every approval before it go)
That is the failure this PR exists to prevent, reached from the other side:
quoting the shape was the one way to void the run the tool protects. Found
independently by two outside contributors (how2how2how2-arch, pm25coder) and
reproduced here before accepting it - it is why this head was not merged at 2/3.
Fenced regions are now dropped from `_decorated_lines`. A mark counts only when
the *reviewer* states it; in a fence the mark opens its line but the body is
quoting. Nesting follows CommonMark: a fence closes only on the same character
with at least the opener's length, because this repo quotes ``` examples inside
```` blocks - a boolean toggle broke on exactly those bodies (measured on a real
review body on this PR). An unclosed fence is treated as prose, so a stray marker
can never hide a real veto and leave stale approvals live.
Measured over 309 real bodies (PRs #1110-#1146): the fence fix alone changes the
class of **1** body - the live regression above, back to the `comment` that
master gives it - and 0 others. Four mutants are killed: fence-blind (no
skipping), length-blind (`==` for the closing fence), unbalanced-hidden, and the
toggle version. Three new tests fail on the unfixed source and pass on this one.
Not adopted: a contribution on the PR also proposed scanning past a later ✅ and
answering `approve` for a stated ✅ below a prose intro. On the same 309 bodies
that widening flips 6 bodies - including 4 approvals into `comment`/`approve`
churn - for no demonstrated defect. Fence awareness is the half that is needed
either way, so only that half ships; the reasoning is recorded in the docstring.
Restores the send↔stop toggle that the React migration dropped (rant 2026-09-02T20:30:05): the GUI had no way to interrupt an in-progress reply while the TUI supports ESC interruption.
Changes:
busy, the send button is replaced by a red stop button (■,composer.stoptooltip/aria-label) wired towindow.emrg.cancel(preload bridge already exposed; daemon cancel + releaseOwnStream untouched).cancelprop on ComposerProps (defaultwindow.emrg.cancel, aligned with thesendMessage/saveImage injection style) for tests.⏹ 已停止响应。/⏹ Interrupted — response stopped.(newchat.interruptedkey, zh+en, 394 keys aligned). The daemon'scancelledbroadcast then idempotently releases the external busy lock (existing releaseOwnStream path) — no duplication.Verification: renderer typecheck clean; vitest 511 passed (45 files); GUI static suite 93 pass/0 fail; doc-count guard 5/5.