Repository navigation
Make Stop cancel chat generation and preserve the partial answer - #961
Floating-Y wants to merge 1 commit into
Conversation
…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>
|
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:
Route path is your call; /conversations/{id}/stop would sit beside the existing stream route. Thanks again. |
…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>
…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>
fbbbb12 to
2fd1811
Compare
Signed-off-by: Floating-Y <118035379+Floating-Y@users.noreply.github.com>
2fd1811 to
2dfce78
Compare
|
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. |
* 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>
Fixes #934.
Stop signals the active generation, cancels pending model/tool work, saves the published partial answer once with
stopped: true, and emitsdoneafter 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::beginadmission check. Busy requests return HTTP 409 withanswer_runningin the existing{error, code}envelope. Retry eligibility is checked while holding the conversation reservation; retries reuse the stored question. The conversation frame carries bothmessage_idandgeneration_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 indocs/design/chat-and-mcp.md. New Rust comments are Chinese. The Stop route remains/kbs/{id}/chat/{conversation_id}/stop. Rebased ontodevat6ef840a, including #958, #960 and #962.Validation
search_chunkstool 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 returnanswer_running. Existing early/late Stop, save race, stale generation, disconnect and citation regressions pass.cargo test --workspace --no-fail-fast -- --test-threads=4with a dedicated PostgreSQL database: 484 server tests and 277 main store integration tests passed. The only failing target was the existingpdf_gbk::gbk_euc_h_pdf_extracts_chinese_texttest: this Windows TeX Live Poppler cannot read its CJK CMap data. CI installspoppler-utilsandpoppler-data.