Skip to content

Make Stop cancel chat generation and preserve the partial answer - #961

Closed
Floating-Y wants to merge 1 commit into
deeplethe:devfrom
Floating-Y:fix/chat-stop-934
Closed

Floating-Y wants to merge 1 commit into
deeplethe:devfrom
Floating-Y:fix/chat-stop-934

Conversation

@Floating-Y

@Floating-Y Floating-Y commented Sep 26, 2026 •

Copy link
Copy Markdown
Contributor

Fixes #934.

Stop signals the active generation, cancels pending model/tool work, saves the published partial answer once with stopped: true, and emits done after persistence. Navigation and ordinary SSE disconnections still let generation continue. Generation IDs protect later turns from delayed Stop requests, and the browser waits for the terminal outcome before allowing a follow-up.

New questions and #960 retries share the exclusive Registry::begin admission check. Busy requests return HTTP 409 with answer_running in the existing {error, code} envelope. Retry eligibility is checked while holding the conversation reservation; retries reuse the stored question. The conversation frame carries both message_id and generation_id.

Mapping retrieval now runs inside the cancellable producer. If it fails, the question has already been stored and the stream emits search_failed, so the question can be retried in place; previously this returned HTTP 500 before storing anything.

Includes migration 0095 and CURRENT_SCHEMA_VERSION = 86, matching the migration file count. Implements ADR 0063 with its status and index updated, and documents Stop in docs/design/chat-and-mcp.md. New Rust comments are Chinese. The Stop route remains /kbs/{id}/chat/{conversation_id}/stop. Rebased onto dev at 6ef840a, including #958, #960 and #962.

Validation

  • Rust formatting, Clippy with warnings denied, and workspace build passed.
  • All 86 migrations ran successfully on a fresh PostgreSQL database and on a second run; the CLI schema-version guard passed.
  • Stop during a real search_chunks tool embedding request, for both new questions and in-place retries: the blocked HTTP request closes, one partial answer is saved, unfinished tool calls are omitted, and overlapping questions/retries return answer_running. Existing early/late Stop, save race, stale generation, disconnect and citation regressions pass.
  • 196 frontend unit tests and 40 browser subtests passed, including failed answer -> repeated Retry clicks -> Stop, preserving one question and targeting the retry generation. TypeScript, style guard and production build passed.
  • cargo test --workspace --no-fail-fast -- --test-threads=4 with a dedicated PostgreSQL database: 484 server tests and 277 main store integration tests passed. The only failing target was the existing pdf_gbk::gbk_euc_h_pdf_extracts_chinese_text test: this Windows TeX Live Poppler cannot read its CJK CMap data. CI installs poppler-utils and poppler-data.

WaylandYang added a commit to Maya-Kid/utopia that referenced this pull request Sep 26, 2026
…versation

Maintainer edits on deeplethe#960: the doc on answer_running and the en.ts note pointed at the issue; the
PR that adds that 409 is deeplethe#961, and whichever lands second keeps the first one's code. The retry
tests' header no longer claims they compile on older code, since the sibling test files' ChatReq
literals would not.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@WaylandYang

Copy link
Copy Markdown
Contributor

Thank you, this is a thorough piece of work. I read the whole change: the cancellation token through the hook boundaries, the single save with the stopped flag, the generation id guarding a late Stop, the handle released on every early return through Drop, and the browser waiting for the terminal frame. The races I looked for (Stop before the first poll, Stop after done, Stop during save, a cancel landing between the two select polls) are all handled and each has a test. Approving the design as is.

Before it can land, a few things, none of them logic changes:

  1. Rebase onto dev once Let a question whose answer failed be answered again in place, without storing it a second time #960 (retry in place) has landed, which will be shortly. It adds begin_if_idle, is_running and running to the registry; with your exclusive begin those are unnecessary, so delete them and route the retry branch through begin as well (it already skips append_message, which is the order you want). It also extends identify with the question's message id, so identify needs both ids, and the conversation frame carries message_id and generation_id. Per the agreement on A chat answer cannot be checked, retried or copied #936, the second PR keeps the first one's code: use answer_running for the 409, and drop conversation_busy from the i18n files.
  2. Migration number. Speed graph rebuilds with foreign-key indexes #958 is landing with 0094 to 0119, so this becomes 0120 and CURRENT_SCHEMA_VERSION in crates/utopia-cli/src/main.rs becomes 111.
  3. ADR 0063. The status line follows the other records: Implemented 2026-09-27 (Make Stop cancel chat generation and preserve the partial answer #961, migration 0120), not the issue number, and the README index row must say the same. Written carries the (conventions in the README) suffix.
  4. docs/design/chat-and-mcp.md is the condensed current design; add a short paragraph on Stop and one generation per conversation, citing 0063.
  5. Comments in Rust are Chinese in this repo (docs/decisions/README.md). chat_generation.rs and the new comments in chat.rs, agent.rs and live.rs are English, and live.rs replaces existing Chinese doc comments on Registry, begin and finish with English ones. Please translate them; the why-content is good and should stay.
  6. Optional: replace the cfg(test) finish and done_event shims with drop(handle) in the four tests; drop the second streaming guard in send in Chat.tsx, which repeats the first; and mention in the description that moving mapping retrieval into the producer changes a mapping failure from an HTTP 500 with nothing written into a stored question plus an error frame.

Route path is your call; /conversations/{id}/stop would sit beside the existing stream route. Thanks again.

@WaylandYang

Copy link
Copy Markdown
Contributor

Correction to item 2: #958 is landing as a single migration, so this one keeps 0094 only if it lands before #960; otherwise it is 0095 and CURRENT_SCHEMA_VERSION 86. Either way, count the files in migrations/ at rebase time; the CLI test checks that the constant equals the count.

WaylandYang added a commit to Maya-Kid/utopia that referenced this pull request Sep 26, 2026
…versation

Maintainer edits on deeplethe#960: the doc on answer_running and the en.ts note pointed at the issue; the
PR that adds that 409 is deeplethe#961, and whichever lands second keeps the first one's code. The retry
tests' header no longer claims they compile on older code, since the sibling test files' ChatReq
literals would not.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Signed-off-by: Wayland Yang <wayland0916@gmail.com>
WaylandYang added a commit that referenced this pull request Sep 26, 2026
…t storing it a second time (#960)

* Let a question whose answer failed be answered again in place, without storing it a second time

Signed-off-by: dada-yan <BinjunYann@gmail.com>

* Name #961, not #934, as the other 409 on a busy conversation

Maintainer edits on #960: the doc on answer_running and the en.ts note pointed at the issue; the
PR that adds that 409 is #961, and whichever lands second keeps the first one's code. The retry
tests' header no longer claims they compile on older code, since the sibling test files' ChatReq
literals would not.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Signed-off-by: Wayland Yang <wayland0916@gmail.com>

---------

Signed-off-by: dada-yan <BinjunYann@gmail.com>
Signed-off-by: Wayland Yang <wayland0916@gmail.com>
Co-authored-by: Wayland Yang <wayland0916@gmail.com>
Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
Co-authored-by: WaylandYang <145302500+WaylandYang@users.noreply.github.com>
Signed-off-by: Floating-Y <118035379+Floating-Y@users.noreply.github.com>
@WaylandYang

Copy link
Copy Markdown
Contributor

Thank you for turning every item around so quickly. I re-read the rebased change: the retry branch reserves the conversation before it reads history, each refusal releases it, a Stop during a retry stores one stopped answer after the existing question, and none of the retry tests lost an assertion. It is ready.

Your branch does not allow maintainer edits, so I could not bring it up to date with dev from here. To avoid another round trip I opened #969, which carries your commit unchanged with dev merged in and one small commit that restores four comments the rebase dropped. The squash keeps you as co-author and #934 closes with it. I will close this one when #969 lands. For next time, ticking Allow edits by maintainers on the PR lets me do this in place.

Two things for a follow-up if you want them: a mapping-retrieval failure now reports search_failed, which is worded as a document search failure, so a code of its own would be more accurate; and a retry refused while another tab reattaches can show that tab stream_ended for an instant. Thanks again.

WaylandYang added a commit that referenced this pull request Sep 27, 2026
* Stop chat generation explicitly and preserve its partial answer

Signed-off-by: Floating-Y <118035379+Floating-Y@users.noreply.github.com>

* Keep the reasons the rebase dropped from chat.rs

Four why-comments from dev went missing when #961 moved their code: what the semantic layer's
mappings are for and why they are picked per question (#574), now with the reason they moved
into the producer and what that changes when they fail; why a retry exists (#936); why the
question's stored id travels in the conversation frame; and that the server assembles context
from stored messages.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Signed-off-by: Wayland Yang <wayland0916@gmail.com>

---------

Signed-off-by: Floating-Y <118035379+Floating-Y@users.noreply.github.com>
Signed-off-by: Wayland Yang <wayland0916@gmail.com>
Co-authored-by: Floating-Y <118035379+Floating-Y@users.noreply.github.com>
Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
@WaylandYang

Copy link
Copy Markdown
Contributor

Landed as #969 (d9c3aab), with you as co-author. #934 is closed by it. Thank you, Floating-Y.

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.

Stop leaves the answer running on the server

2 participants