Repository navigation
Record streams the caller abandons - #243
Merged
Merged
Conversation
Cancellation already worked: a hang-up stops the graph and the model call, which is why the website's proxy needs to do nothing beyond closing the connection. What was missing was any trace of it. An abandoned stream is indistinguishable from a healthy one in every other signal -- the request 200s, tokens flow, and then nothing further happens. The website found a bug where a keystroke unmounted their panel mid-answer and left our stream running for up to the full ceiling. From this side that was ordinary traffic, and they were right that we would have looked for the cause in our own code. A rate of these is the symptom of a caller starting answers it does not want. Both forms are caught, and the second was found by the test failing: Starlette cancelling the task raises CancelledError, while closing the generator raises GeneratorExit. A handler catching only CancelledError passed the live hang-up and failed the generator close. Re-raised, never swallowed. Cancellation is not an error to report to a caller who has already gone, and suppressing it would leave the task pretending to run. Verified both ways: the unit test closes the response iterator mid-stream, and a live hang-up over HTTP logged "answer abandoned by the caller after 1.5s and 3 token events" with exactly 3 events produced server-side for 3 read. Also audited every caller of the graph for the postprocess defect, since that is the shape both repos hit today -- a second caller kept doing the wrong thing after the first was fixed. Three drive the graph: the endpoint and the sweep pass enable_postprocess=False, and the chat UI passes its feature flag and renders the results. No fourth. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Pushed the previous commit with mypy failing: I read its output and not its exit code, which is the mistake the pipe-to-tail pattern invites. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Prompted by the website session finding a bug on their side: a keystroke unmounted their panel mid-answer and left our stream running. From here that looked like ordinary traffic — and they were right that we would have gone looking in our own code.
What was missing
Cancellation already worked. A hang-up stops the graph and the model call, which is why their proxy needs to do nothing beyond closing the connection. What was missing was any record of it: an abandoned stream is indistinguishable from a healthy one in every other signal, because the request 200s, tokens flow, and then nothing further happens.
Now:
A rate of these is the symptom of a caller starting answers it does not want.
The test found a second mechanism
A hang-up arrives as either exception depending on who notices first:
asyncio.CancelledErrorGeneratorExitA handler catching only
CancelledErrorpassed the live hang-up and failed the generator-close test. Both are caught now, and both are re-raised — cancellation is not an error to report to a caller who has already gone, and swallowing it would leave the task pretending to run.Verified both ways: the unit test closes the response iterator mid-stream; a live HTTP hang-up produced the log line above with exactly 3 events produced server-side for 3 read.
Audit, from their generalisation
They observed that both bugs found today were the same shape — a second caller kept doing the wrong thing after the first was fixed. That is exactly the endpoint-then-sweep postprocess defect, so I checked every caller of the graph rather than assuming two was all of them:
src/api/answer.pyFalsesrc/evaluation/answer_sweep.pyFalse(#242)bin/chat-chainlit.pyNo fourth. The remaining
ainvokecalls are inner chains that never reach the postprocess node.CI-equivalent locally: ruff, format, mypy (129 files), full suite with no API keys set.
🤖 Generated with Claude Code