Repository navigation
fix(*): say whether the lane is still busy when a cancel is refused - #1078
Conversation
A stop naming a turn that has just finished is refused, and the web UI went idle and released its send queue unless a frame opening the next turn had already arrived. That frame waits out the subscription's 16 ms coalescing window, while the reply is written as soon as the handler returns, so a refusal handled within one window of the next turn starting arrived first: the web UI went idle, sent its queued message under the turn now running, and the late frame then put it back to streaming. A refusal now carries busy: whether the lane still has a turn to run, one running, one queued, or an inject that will run as a turn of its own. Scheduler.is_busy answers it; unlike has_inflight it does not count a turn that has stopped and only waits for the worker to resolve it. On busy the web UI keeps the conversation busy, with no stop button until that turn's first frame names it; busy false, or a server that does not send the field, releases it as before. One test waited without a bound for a cancel to reach its turn, so disabling the named path hung the suite instead of failing it; that wait is now bounded.
gloryfromca
left a comment
gloryfromca
left a comment
There was a problem hiding this comment.
No blockers; this can merge as far as I am concerned.
Reviewed the PR diff and surrounding scheduler/RPC/web runtime code, relevant callers and history, backward compatibility for older servers, generated wire-contract consistency, and the repository rules in AGENTS.md, CONTEXT-MAP.md, CONTEXT.md, and ui-web/CONTEXT.md. I also checked that the tests exercise both sides of the race without weakening existing assertions.
Verification: 80 focused Python unit tests passed; the touched direct-chat integration test passed; 25 focused web tests passed; 561 RPC schema/contract tests passed; web TypeScript checking passed; both generated RPC clients match the schema. The broader direct-chat file additionally exposed three pre-existing agent-name fixture mismatches (Raven-General versus raven) after 84 passes; the touched test passes in isolation. GitHub CI is green.
|
Windows compatibility review of the complete 12-file PR diff at No Windows compatibility issue found in that inspected scope. The new busy query reads asyncio task/queue/mailbox state synchronously, without introducing a platform API or process operation. Native tests cover the ended-task/future-resolution interval, queued/injected work, named and unnamed refusals, and the modified direct-chat integration case. Native Windows verificationPython 3.13.12 ( From the checkout root (603 passed, including the changed integration case; exit 0): uv run --frozen --no-sync pytest tests/test_rpc_turn_cancel.py tests/test_spine_scheduler.py tests/test_rpc_schema_match.py tests/test_rpc_registration.py tests/integration/test_direct_chat_smoke.py::test_an_untargeted_cancel_does_not_touch_a_direct_turn -m 'not e2e' -p no:randomly -n 0 -x --tb=short --basetemp C:\Users\livxu\AppData\Local\Temp\raven-windows-review\e8580441fb594aaf98a0b05404f9fe6b\checks\1078-python\pytest --junitxml C:\Users\livxu\AppData\Local\Temp\raven-windows-review\e8580441fb594aaf98a0b05404f9fe6b\checks\1078-python\junit.xmlIn npm.cmd test -- src/state/session/runtime.cancel.test.ts scripts/gates/fixture-shape.test.mjs --no-file-parallelism
npm.cmd run type-check
npm.cmd run gen:checkThe first test run had 24 passed / 1 timeout in the existing first cancellation case at its unchanged 5-second deadline. A separate detached control at the exact base passed 22/22; an unchanged-head rerun with the same command/deadline passed 25/25. The initial timeout was not reproduced by either follow-up and is disclosed without attributing a defect. Type checking and the 248-method generated contract check passed. In npm.cmd run type-check
npm.cmd run lint:rpcBoth passed. Shared TUI dependencies were limited to this trigger's scratch checkout after verifying identical package/lock hashes and the vendored Hermes tree; the generated client checked was this PR's own file. Limitations: the integration test uses a stub provider/parked backend; frontend frame ordering is driven by a test harness. No live provider/process-tree cancellation, real browser/WebSocket race, full repository suite, or Python 3.12 run was performed. |
|
Not a blocker, two smaller points from the same review of 1d60a9a (merged onto main 2cbacba): Severity of this one finding, not a verdict on the pull request. Any blocking findings from this pass are review threads on the changed files; GitHub renders those collapsed, as a file name with no text.
|
|
Blocking: the delayed completion must be correlated and suppressed before this can merge. On the two smaller points in comment 6375404472: both hold. During the new busy hold, Focused committed tests remain green: 25 web tests and 81 Python tests passed. |
gloryfromca
left a comment
gloryfromca
left a comment
There was a problem hiding this comment.
Blocking: the busy refusal still releases the queued send when the reply precedes the old turn's completion.
The new evidence invalidates my prior clean stance. I independently reproduced the server-relevant ordering in the page harness: the busy refusal first held the queue, but the delayed message.complete then ran finishTurn, went idle, and sent the queued message before the next turn's opening frame. The committed tests pass because they cover completion-before-reply, not reply-before-completion.
The accompanying prompt-display issue is real but nonblocking, and the named-lane scheduler coverage gap is also valid. Focused verification on the unchanged committed tree: 25 web tests passed and 81 Python tests passed. The temporary failing regression was removed and the worktree is clean.
gloryfromca
left a comment
gloryfromca
left a comment
There was a problem hiding this comment.
No blockers; this can merge as far as I am concerned.
Reviewed the new delta and its interaction with the full cancellation flow, scheduler lane ownership, web runtime stages, event correlation, off-screen/runtime state, older-server behavior, and the prior review record. I also rechecked the repository rules and architecture boundaries relevant to the changed files, history/callers, and that the tests strengthen rather than weaken the earlier coverage.
The previous blocker is addressed: a busy refusal records the stopped turn before the local fold, drops only that turn's delayed terminal frame, keeps the queue held for the next turn, and restores prompt drawing/refiling when that turn opens. The real scheduler is now exercised for lane isolation.
Verification passed: 67 focused web tests, 82 focused Python tests, 561 RPC schema/contract tests, both generated RPC client checks, web TypeScript checking, and diff checks. GitHub's completed checks are green; several unit jobs were still pending when inspected.
|
Windows compatibility re-review at head The previous Windows review was at head The delta is pure in-page turn state and frame routing. The three web files contain no filesystem, path, Inherited, and not a regression: the three failures in Native Windows verification (Python 3.13.12, UTF-8; Node 25.9.0, npm 11.12.1, vitest 4.1.11; Windows 11 10.0.26300 x64):
Coverage limits: the delta's Python surface was exercised through the two changed test files plus the neighbouring cancel module, not the whole Python suite; the web surface was exercised through Exact verification commands and resultsAll commands ran on this machine against detached checkouts of the exact head and base; every command exited as stated. $root = 'C:\Users\livxu\AppData\Local\Temp\raven-windows-review\run-20261024'
$head = "$root\pr1078\new\head"
$base = "$root\pr1078\new\base"
$scratch = "$root\scratch\pr1078delta"
# authoritative head sha; exit 0
gh api repos/EverMind-AI/Raven/pulls/1078 --jq .head.sha
# -> 2af6af02e68011e167b29cd44a7a3796b4c9c400
# merge base confirmed; exit 0
git -C $head merge-base 2af6af02e68011e167b29cd44a7a3796b4c9c400 8a7ea1ccf6da6114b5f8670e18228e9f3a23e940
# -> 8a7ea1ccf6da6114b5f8670e18228e9f3a23e940
# the delta itself; exit 0
git -C $head diff --stat 1d60a9aee130d4d8f8bc99682245bde..2af6af02e68011e167b29cd44a7a3796b4c9c400$env:UV_PROJECT_ENVIRONMENT = "$root\venv"
$env:PYTHONPATH = $head
$env:PYTHONUTF8 = '1'; $env:PYTHONDONTWRITEBYTECODE = '1'
# the two delta cases: 2 passed; exit 0
uv run --frozen --no-sync --project $head pytest -c "$head\pyproject.toml" --rootdir $head "$head\tests\test_spine_scheduler.py::test_is_busy_answers_for_the_lane_it_is_asked_about" "$head\tests\integration\test_direct_chat_smoke.py::test_an_untargeted_cancel_does_not_touch_a_direct_turn" -p no:randomly -p no:cacheprovider -n 0 --tb=short -v -q --basetemp "$scratch\pytest-delta"
# head, both changed python files: 3 failed, 52 passed; exit 1
uv run --frozen --no-sync --project $head pytest -c "$head\pyproject.toml" --rootdir $head "$head\tests\test_spine_scheduler.py" "$head\tests\integration\test_direct_chat_smoke.py" -p no:randomly -p no:cacheprovider -n 0 --tb=short -q --basetemp "$scratch\pytest-head"
# base, same two files: 3 failed, 47 passed (same 3 failures); exit 1
$env:PYTHONPATH = $base
uv run --frozen --no-sync --project $base pytest -c "$base\pyproject.toml" --rootdir $base "$base\tests\test_spine_scheduler.py" "$base\tests\integration\test_direct_chat_smoke.py" -p no:randomly -p no:cacheprovider -n 0 --tb=line -q --basetemp "$scratch\pytest-base"
# neighbouring module using the same bind_scheduler global: 34 passed; exit 0
uv run --frozen --no-sync --project $head pytest -c "$head\pyproject.toml" --rootdir $head "$head\tests\test_rpc_turn_cancel.py" -p no:randomly -p no:cacheprovider -n 0 --tb=short -q --basetemp "$scratch\pytest-cancel"$env:HTTPS_PROXY = 'http://127.0.0.1:7897'; $env:HTTP_PROXY = 'http://127.0.0.1:7897'
# dependency install; exit 0 (283 packages)
npm.cmd ci --prefix "$head\ui-web" --no-audit --no-fund
# the delta's web test file: 1 file, 26 tests, 26 passed; exit 0
npm.cmd test --prefix "$head\ui-web" -- src/state/session/runtime.cancel.test.ts --no-file-parallelism
# the whole session directory: 13 files, 252 tests, 252 passed; exit 0
npm.cmd test --prefix "$head\ui-web" -- src/state/session --no-file-parallelism
# type check; exit 0
npm.cmd run type-check --prefix "$head\ui-web" |
A refusal that holds the lane busy was released by the completion of the turn the reader stopped. The server read the cancel after that turn had already ended, so its end was on the way -- behind the refusal, which is written while the frames wait out the subscription's 16 ms window. The page held the phase off `cancelling` to keep the queue from going out under the turn that was starting, and that phase is what the completion stage drops a late completion on: the stage folded it instead, which released the queue into the turn that was starting. The hold now remembers the turn it is holding the lane for. The page knows that turn's id, from the frame that opened it: `message.start` for a turn a send opened, `turn.started` for one the runtime opened. `stop()` reads it at the press, before the fold at the press clears the runtime, and the refusal's `busy: true` puts it on the runtime as the turn the hold is for. A frame naming that turn is dropped rather than folded: the stop already folded that turn, and folding it again is what released the queue. Any other frame ends the hold, because the turn that ended is not the one the hold was waiting for. A turn that sends no opening frame, a cron wake, still ends the hold on the frame that ends it. The hold also keeps the question of the turn it was waiting for. A hold made the machine busy without a turn running in it, so the `message.start` that opens that turn read as a message this window had already drawn: its question was never drawn and its bubble was never moved out of the ended turn. Both happen now. `busy` also answers for the lane it resolved to and for no other: the direct-chat smoke case binds the real scheduler, so it cannot pass through the "no scheduler, so not busy" fallback, and a scheduler test asks a second lane the same question.
2af6af0 to
4064720
Compare
gloryfromca
left a comment
gloryfromca
left a comment
There was a problem hiding this comment.
No blockers; this can merge as far as I am concerned.
Reviewed the rewritten delta against the previously clean revision and rechecked the affected runtime stages, turn correlation, hold lifecycle, scheduler coverage, wire shapes, callers, backward compatibility, history, repository rules, architecture boundaries, and test strength.
The rewrite preserves the prior fix while correcting its evidence: cron completions use real wire IDs, the stopped turn ID comes from one authoritative runtime field, and the new regression proves the hold is cleared before the queued send's opening frame so its prompt is not drawn twice. No concrete failure remains.
Verification passed: 68 focused web tests, 82 focused Python tests, 561 RPC schema/contract tests, both generated RPC client checks, web TypeScript checking, and diff checks. Completed GitHub checks were green; several broader jobs were still pending when inspected.
|
Windows compatibility re-review at head The previous review was at head The delta is in-page turn state and adds no Windows surface. It gives the runtime the id of the turn running in the conversation, a flag for a refusal that is holding the lane, and the id of the turn such a refusal is waiting for; the stop path reads the turn id before the fold at the press clears the runtime, and the frame handlers clear the hold when the awaited frame arrives. None of that touches the filesystem, a path, a process or an environment: the three files contain no Native Windows verification (Node 25.9.0, npm 11.12.1, vitest 4.1.11; Windows 11 10.0.26300 x64): the changed web test file Coverage limits: the web surface was exercised through the changed test file and the type checker, not the whole Exact verification commands and results$root = 'C:\Users\livxu\AppData\Local\Temp\raven-windows-review\run-20261024'
$web = "$root\pr1078\new2\head\ui-web"
$env:HTTPS_PROXY = 'http://127.0.0.1:7897'; $env:HTTP_PROXY = 'http://127.0.0.1:7897'
# dependency install; exit 0 (283 packages)
npm.cmd ci --prefix $web --no-audit --no-fund
# the delta's web test file: 1 file, 27 tests, 27 passed; exit 0
npm.cmd test --prefix $web -- src/state/session/runtime.cancel.test.ts --no-file-parallelism
# type check; exit 0
npm.cmd run type-check --prefix $web
# tsc --noEmit -p tsconfig.json
# the delta itself
git -C "$root\repo" diff --no-color "8a7ea1ccf6..4064720c1c38d3d58e29907535f0562b61cd0ebe" -- ui-web/src/state/session/runtime.ts ui-web/src/state/session/stages.ts |
… silence (#1092) ## Summary Supersedes #1052, which was closed unmerged. That PR ended a turn without a reply whenever the model returned nothing. After a survey of how other agents handle the same replies, empty and abnormal replies now follow Hermes Agent, and Raven keeps its reasoning-effort step-down only for a reply that stopped while still thinking. This also ends the Codex HTTP 400 "Unsupported value: 'minimal'": the old empty-reply recovery lowered the effort on every resend, down to `minimal`, which Codex refuses. Neither recovery nor the in-turn compaction summary asks for `minimal` any more; a session whose own effort is `minimal` still sends it, and the compacting history strategy's checkpoint summary goes out at the provider's configured effort. Silence is now something the model says, not something it omits: - A reply that is a silence marker ends the turn without a reply. A turn a person started honours only the bracketed `[SILENT]` the runtime teaches, as the whole reply (whitespace aside): such a turn is never taught a marker, so `Silent.` or `No reply.` there is the person's answer. That is stricter than Hermes, whose strict rule takes all four spellings. A proactive turn (cron, heartbeat, Sentinel, a sub-agent report) uses Hermes' rules over all four (`[SILENT]`, `SILENT`, `NO_REPLY`, `NO REPLY`): the whole reply, at most 64 characters, or the marker on the first or last line. An A2A turn, whose peer waits on the text, gets its reply back as written. - The marker rules live in `raven/agent/context/silence.py`, outside the kernel. The kernel carries only the outcome: `TurnOutcome.silent` and the `silent_turn` notice. - The cron reminder and the host's note on an unprompted sub-agent report teach the marker, in Hermes' words. - Streaming holds back text that may still turn out to be a marker, so a silenced reply never flashes on screen. On a person's turn only a reply that opens with whitespace or `[` is held, and an inline think block streams as it arrives while what follows it is held; a proactive turn holds the whole call. The hold lasts across a stream cut and its continuation. - The marker row is stored with `silent: true` and the turn ends with a `silent_turn` notice; the TUI and the web UI draw one muted line ("Raven chose not to reply."). ACP replay, the session list, the session export and the cron run list do not show it as a reply. A reply an `after_send` hook composes in place of the marker is delivered and stored as that reply. - A channel stops its typing indicator (`end_turn`) when a turn a user started, or one a user's message was merged into, ends without a reply. A silent cron run is a success: it wakes nobody and does not count toward the silent-fire limit. An empty reply is recovered by kind, decided by two pure functions (`reply_kind`, `decide`): - only thinking, no text (cut at the output limit or not): two prefills when there is thinking to re-feed, then the same step asked again at `high`, then `low` (only the rungs below the turn's effort, each once per turn), then the fallback models, then a failure; - plain empty: resent after Hermes' jittered backoff, up to three times, or once when the call reported a cost at or above `emptyRetryCostThresholdUsd` (default 0.25 USD); two zero-output replies from one model stop the resends early. On a streamed turn the first resend goes out without streaming (see below); - empty after a tool call: Hermes' nudge; - cut at the output limit with neither text nor thinking: a continuation with max_tokens doubled each time (x2, x4, x8), held to the model's own output ceiling and to the room its prompt leaves; - a refusal or a content filter (read off the finish, or off the `refusal` field, which litellm moves into `provider_specific_fields`): never resent; one fallback model, whose answer opens a step of its own, then a failure that quotes the model's explanation; - fallback models are tried once each, and a switch holds for the rest of the turn. Failures are worded after Hermes, per kind, and `no_output` (on `TurnFailed` and `AnswerlessTurnError`) marks a turn whose last reply was empty. A transient `llm_retry` notice says when a resend is waiting. A stream that dies after showing text is continued (up to three times, stitched into the same reply). If it stays cut, or the turn runs out of iterations or time while a continuation is owed, the partial is delivered with a note that it is partial, status `interrupted`, instead of failing the turn. A cut is read before any marker, so a fragment that happens to spell one is continued, and a stream that had already received a refusal finish keeps it; the reply the turn ends on, the joined parts with their closed think blocks taken out, is then read for a marker like any other, and parts that join into nothing but a thought take the thinking-only path an uncut thought takes. A continuation that calls a tool keeps the words the reader saw on that call's row, as they came. Providers now report the finish they actually got: `unknown` for a missing one, `refusal` / `content_filter`, and an error for a 200 response that carries one. ACP reports a turn its own budget ended as `max_turn_requests` (#1121); a cut stream delivered as a partial spent no budget and stays `end_turn`. Decisions beyond the design, so a reviewer does not rediscover them: - Only a real mid-stream death is continued. The stream hand-back marks it `finish_reason="stream_cut"`; a reply that merely lacks a finish is delivered as is. Continuing on a missing finish would draw three continuations on every reply of a backend that never sends one. - The output-limit continuation and the post-tool nudge keep Hermes' frame, but say that the cut left nothing and that the model may answer or make the call the task needs: Raven sends them only for a cut that produced nothing, which Hermes' wording does not fit. - Once the message tool has replied in a turn (`MessageTool.sent_in_turn`: its send to the turn's own chat went out), an empty last reply that a nudge or a prefill does not mend ends the turn as answered, without the paid resends, step-downs and fallbacks; a refusal there ends it the same way. A marker after such a reply does not make the turn silent, so no `silent_turn` notice is sent. - On a streamed turn, the first resend of a plain empty reply asks the same model without streaming, with no provider-side fallback, and its answer is shown through the same sinks and gates a stream's deltas pass; later resends stream again. A stream can come back empty where the plain call does not: the locked litellm 1.85 drops a streamed chunk that carries only a refusal, so a refusal an OpenAI-compatible model streams arrives as an empty stop, and the plain call reads it and hands it to the refusal path. A gateway whose streams fail is rescued the same way. - Sub-agent and DAG lanes keep their own recovery; besides the note above, only the providers' finish reasons reach them (see Risk). - `after_send` hooks still run on a silent turn, with `outbound_content=None`; `FACTORY_LOOP_SURFACE_VERSION` is now 4. How a stored turn is read: the live turn files `replied_via_tool` on the last assistant row a turn the message tool answered stores, and the session list and the ACP replay read that flag from one place, `raven/session/turn_shape.py`, instead of guessing from the calls they can see. A send that failed or went to another chat answered nobody, so both surfaces agree with the live view about which turn answered. ## Type - [ ] Fix - [x] Feature - [ ] Docs - [ ] CI / tooling - [ ] Refactor - [ ] Other ## Verification At the PR head, on origin/main 06509f8 (Linux, 32 cores). - The full Python suite: 36878 passed, 9 failed, 255 skipped, 2 xfailed. ``` nice -n 19 uv run --frozen --all-extras pytest -n 4 -q -p no:cacheprovider ``` The same 9 fail on a clean archive of origin/main 06509f8 on this machine: five proxy tests in tests/test_config_update_providers.py, test_install_script, test_knowledge_parser_excel, test_subagent_node_runtime, and test_cli_agent_commands' empty-message-file test, whose error box is cut to the terminal width. They come from the environment, not from this branch. - ui-web `npx vitest run`: 260 files, 4461 passed. - ui-tui `npx vitest run --maxWorkers=2`: 145 files, 2275 passed, 13 skipped. - `make lint-python lint-imports lint-deps lint-types lint-ui lint-tui check-commits check-large-files check-source-language`: all exit 0. ui-tui `npm run lint:i18n` and `npm run lint:rpc`, and ui-web `npm run gen:check`: exit 0. - The review round's fixes were each written test first, and each was checked by removing or inverting it and watching a test go red. An independent re-review repeated those mutants, probed the stream and marker cases through the real loop against the previous head, and fuzzed the stream gate over about 35,000 random two-part cut replies without finding a silent turn that showed its marker. - The rebase onto 06509f8 replayed every commit unchanged; its one meeting point with main, #1121's budget-stop marker, is described in the summary. Earlier rebases met #1031 (compacting history), #1078 (refused cancel), #1119 (the context ring a resume restores), #1047 and #1114; what each needed is pinned by tests on this branch. - Prompt text, by a paper exercise with fresh model readers, at an earlier head (no prompt text has changed since): a routine sub-agent report was answered with `[SILENT]` and a finished one with a real report; a plain one-shot reminder was still delivered; an hourly watch with nothing new stayed silent. With the text before this change, the two routine cases produced a message. - [x] Relevant tests pass locally - [x] Relevant lint / type checks pass locally - [x] User-facing docs or screenshots are updated when needed ## Risk What a user will notice: a reply that is only a silence marker is not delivered; an empty reply is asked again, stepped down or switched to a fallback before the turn fails, and fails in Hermes' wording; a cut stream is continued and, if it stays cut, delivered as partial instead of failing the turn. Known gaps, each left as it is here: - A continuation request that raises (a non-retryable error, a cancel, or a retryable error that outlasts the stream's own ladder, about 105 s) fails the turn although the reader saw the partial. - An error the provider raises mid-stream (LiteLLM raises its HTTP errors) is not switched to a fallback model: it is waited out and fails the turn, as before. A stall before any output, and an error that comes back as a response, do fall back. - On a plain (non-streamed) call, a fallback the provider switched to on its own is not known to the loop, so a later empty-reply fallback can ask that model again. - A refusal a fallback model then answers streams live in a step of its own, and only the fallback's answer is stored, so a reload shows the answer alone. Storing the refused words would need a row kind shown to the reader but never sent to the model, and none exists. - When the message tool replied and the model then ended on a marker, a reload draws the muted line after the reply; on the web the reply then sits in the turn's folded steps. - A silent turn the runtime opened is left out of an ACP session/load replay with its tool calls, as the session list leaves it out, unless the message tool replied in it. - The cron rule applies to every proactive origin, including sub-agent result turns that are never taught the marker: such a reply whose first or last line is a bare "No reply" or "Silent" is not delivered. - Silent cron runs never reach the silent-fire limit, so a recurring job that always stays silent is never paused, and each run adds about 3.5 KB to its session file and curator traces until consolidation trims them. A job whose model always returns nothing keeps firing, visible only as lastStatus=error. - A refusal an OpenAI-compatible model streams costs one extra call before it is handled as a refusal, and only the turn's first plain empty resend can read it: a fallback model is never resent to, so a streamed refusal from a fallback still reads as an empty reply. The unstreamed resend's answer is shown whole rather than as it is written, has none of a stream's idle watchdogs, and shows only the retry notice while a long-reasoning model works; a proxy that caps plain responses turns it into an error, which takes the usual error path. A test pins what litellm 1.85 passes on, so an upgrade that starts passing the words is noticed. - The cost guard reads only a reported `usage.cost_usd`, so it does nothing on direct OpenAI, Azure, Codex or the Anthropic API unless a gateway adds the cost. - The failure for a thinking-only reply quotes up to 500 characters of the model's thinking in the TUI and web detail; channels get the bounded 200-character text. - `stream_cut` shows as the call's finish in the trace viewer. - Sub-agent runs keep their own recovery, and with #1047 merged they read the providers' honest finish reasons this way: a refusal with text is the run's answer; a refusal, content filter or missing finish with no text is a blank reply, which gets the one wrap-up call and then fails as having no answer; a 200 response carrying an error fails the run as a failed model call, with no wrap-up. An empty stream whose prompt was never read is asked once more without streaming in a sub-agent's direct chat before it fails. - Near the context window, an output-limit continuation asks for no more than the room its prompt leaves, as the first call did; a reply cut there with nothing in it is therefore usually cut again and fails after its continuations. - A cut keeps its two scaffolding rows (the words so far and the ask to go on) in the window for the rest of the turn, so a continuation that calls a tool shows the model the partial twice, and a cut that joins into a bare thought is prefilled, stepped down or sent to a fallback after them: the re-ask reads as "continue the cut thought", and a model that goes on mid-thought has the thought's tail delivered. The stored history has neither row. Taking the rows out lost a hook's note and left the question autofill reading an old list. - Three rare cut shapes still show or deliver part of a marker: a cut whose continuation is refused and then answered by a fallback shows the held marker in the refused step; a cut marker prefix whose continuation starts the marker over (`[SILEN`, then `[SILENT]`) delivers the fragment, as before; a hook rollback of the continuation's tool call drops the partial from that call's row, as before. - A cron turn that took in a person's message and then fails with no output leaves that chat's typing indicator running until the next send. - When a refused stop holds the lane, the stopped turn's late end is dropped, and with it any silent mark that turn set, so the next turn is not drawn as silent. - Unchanged from main: the think-block pattern and the stream's opening hold are quadratic on very long runs of tags or whitespace. Compatibility and rollback: - #1031 (merged) also changes `raven/session/manager.py`: it adds a caller of `_scan_file` and changes neither its return shape nor the silent-turn rule. - `after_send` hooks now also run on a silent turn with `outbound_content=None`; an out-of-tree hook that assumes a string must accept None. - The new fields (`silent` and `replied_via_tool` on stored rows and on `TranscriptMessage`, `no_output` on failures, the `silent_turn` notice kind) are optional; an older Raven reading a session written by this one shows a marker row's text as a reply. - New config key `agents.defaults.emptyRetryCostThresholdUsd` (default 0.25). Rollback is a revert of the squash commit; there is no data migration. - [x] Security impact considered - [x] Backward compatibility considered - [x] Rollback path is clear for risky changes ## Related Issues N/A. Supersedes #1052 (closed unmerged).
The rebased branch keeps main's refined question-broker cancellation check, so its richer tests replace the PR's simpler expectation. A stale named cancel now answers with busy from #1078. Co-Authored-By: Claude Code <noreply@anthropic.com>
The rebased branch keeps main's refined question-broker cancellation check, so its richer tests replace the PR's simpler expectation. A stale named cancel now answers with busy from #1078.
Summary
A follow-up to #981. A stop that names a turn which has just finished
is refused (
{cancelled: false}), and the web UI keeps the newerturn's stop button only once that turn's opening frame (
turn.startedor
message.start) has moved it out ofcancelling. That frame waitsout the subscription's 16 ms coalescing window
(
raven/rpc/subscriptions.py), while the RPC reply is written as soonas the handler returns (
raven/rpc/transports/ws.py), so a refusalhandled within one window of the next turn starting reaches the web UI
first: it goes idle, sends its queued message under the turn now
running, and the late frame then puts it back to streaming.
The refusal now says whether the lane is still busy.
TurnCancelResultgains an optional
busy, set on both refusal paths ofturn.cancel(
_refusedinraven/rpc/methods/turn.py): whether the lane still hasa turn to run -- one running, one queued, or an inject that will run as
a turn of its own.
Scheduler.is_busyanswers it throughLane.busy;unlike
has_inflightit does not count a turn that has stopped andonly waits for the worker to resolve it, so a lane with nothing behind
a finished turn reads as free. An accepted cancel does not carry the
field.
rpc-schema/openrpc.json,raven/rpc/models.pyand bothgenerated clients carry it, and the offline fixture library's shape
gate lists it as unsent, since the offline stop always lands.
On
busy: truethe web UI (stop()inui-web/src/state/session/runtime.ts) keeps the conversation busy, withno stop button and its queue held, until the newer turn's first frame
names it; a turn that sends no opening frame, such as a cron wake,
keeps it busy until the frame that ends it, which every turn on the
lane sends.
busy: false, or a server that does not send the field,releases it as before.
The hold is now correlated with the turn it is holding the lane for,
which is what the first version of it missed, and which the review of
it found. A refusal is written while the frames wait out the
subscription's window, and the turn the reader stopped had already
ended when the server read the cancel, so that turn's completion
arrived behind the refusal: the page held the phase off
cancellingtokeep the queue from going out under the turn that was starting, and the
completion stage drops a late completion on that phase -- it folded it
instead, which released the queue into the turn that was starting. The
page knows that turn's id, from the frame that opened it
(
message.startfor a turn a send opened,turn.startedfor one theruntime opened), so a frame naming it is now dropped rather than folded:
the stop already folded that turn. Any other frame ends the hold,
because the turn that ended is not the one the hold was waiting for.
The hold also keeps the question of the turn it was waiting for. A hold
made the machine busy without a turn running in it, so the
message.startthat opens that turn read as a message this window hadalready drawn: its question was never drawn and its bubble was never
moved out of the ended turn. Both happen now
(
ui-web/src/state/session/stages.ts).busyalso answers for the lane it resolved to and for no other: thedirect-chat smoke case binds the real scheduler, so it cannot pass
through the "no scheduler, so not busy" fallback, and a scheduler test
asks a second lane the same question.
test_a_waiter_cancelled_mid_unwind_does_not_wedge_the_lane, from#981, waited without a bound for the cancel to reach its turn, so
disabling the named path hung the run instead of failing it; that wait
is now bounded at 5 s.
Type
Verification
Run at this branch's head on
mainat8a7ea1ccf, in the branch's ownenvironment (
uv sync --frozen --all-extras, andnpm ciinui-weband
ui-tui).uv run --frozen --all-extras pytest tests/test_rpc_turn_cancel.py tests/test_spine_scheduler.py -q-> 80 passed.uv run --frozen --all-extras pytest tests/test_rpc_schema_match.py tests/test_rpc_registration.py -q-> 522 passed.uv run --frozen --all-extras pytest -q -n 4-> 34330 passed,8 failed, 182 skipped, 2 xfailed. The eight fail the same way
without this change: the five in
tests/test_config_update_providers.pyread this machine's ownHTTP(S)_PROXYand pass (193 of 193) with those variables unset, andthese three fail on an export of
mainat8a7ea1ccfrun the sameday:
test_install_script.py::test_resolve_node_dir_answers_each_case_it_exists_for,test_knowledge_parser_excel.py::test_a_legacy_workbook_is_converted_and_read,test_subagent_node_runtime.py::test_what_cannot_be_read_names_nothing.ui-web:node node_modules/vitest/vitest.mjs run-> 253 files,4231 passed;
npm run gen:check(the 248-method contract),npm run lintandnpm run type-check-> exit 0; thepage checkssteps (
npm run build,python3 ui-web/build.py,check-page.mjs,check-css.mjs,check-class-namespace.mjs) -> OK.ui-tui:npm run lint,lint:rpc,lint:i18n,type-checkandbuild-> exit 0; the suite after itspretestbuild -> 144 files,2249 passed, 13 skipped.
ruff check,ruff format --check,lint-imports(10 kept, 0 broken),
ty checkanddeptry-> pass.scripts/check_source_language.py origin/main...HEAD,scripts/check_large_files.py origin/main...HEADandscripts/check_commit_messages.py origin/main..HEAD-> exit 0.The new behaviour is pinned by mutation: each remedy was reverted on
its own at this head, tests untouched, and these went red.
Lane.busywithout its queue, without its mailbox, or askingrunning_futureinstead ofrunning_turn: one case each oftest_a_lane_is_busy_only_with_what_waits_behind_a_turn_that_has_stopped(
append,inject, nothing behind).busyalways false:test_a_cancel_naming_another_turn_leaves_the_running_one_alone,test_an_unnamed_cancel_leaves_a_turn_it_did_not_bind_running(
delegated,cron).{cancelled: false}again:test_a_cancel_naming_another_turn_leaves_the_running_one_alone,test_a_refused_cancel_with_nothing_left_on_the_lane_says_so; theunnamed one: the six test cases that assert its reply.
busybranch dropped: "stays busy for a newer turnwhose opening frame the refusal outran", "lets a busy conversation go
when the turn it waited for ends without ever opening"; a stop
offered on it: the first of those;
busy: falseread as busy: "letsthe conversation go when a refused id had nothing newer behind it:
the server says the lane is free".
turn.cancelfails
test_a_waiter_cancelled_mid_unwind_does_not_wedge_the_lane(named
turn.cancel) beside the four tests that already caught it,instead of hanging the run.
The hold's own remedies are pinned the same way, one construct at a
time rather than by reverting the whole of them.
the six new web cases fail -- "stays busy when the refusal lands
before the turn it stopped completes", "stays busy when a send
opened the turn the refusal was about", "drops the late unwind of the
turn the hold was about", "draws the question of the turn the hold
was waiting for", "moves the bubble of a mid-turn message under the
turn the hold was waiting for".
Scheduler.is_busyanswering for any lane instead of the one it wasasked about:
test_is_busy_answers_for_the_lane_it_is_asked_aboutand
test_an_untargeted_cancel_does_not_touch_a_direct_turn(thesmoke case, with the real scheduler bound) go red.
error stage's, the
message.startdraw, eitherturnIdassignment,or either half of the hold's own state turns its own case red.
message.startstage --so each of the writes that end it is individually redundant with the
others, and removing one of them leaves the suite green. Removing all
of them at once turns "does not draw a question the machine already
has once the hold is over" red, and that is what pins the end. They
are kept because the hold has to end at the frame that names the
lane, not at the next one that happens to arrive.
At this head, on Windows with Python 3.12.13 and the worktree's own
npm ci:pytest tests/test_rpc_turn_cancel.py tests/test_spine_scheduler.py tests/test_rpc_schema_match.py tests/test_rpc_registration.py tests/integration/test_direct_chat_smoke.py -q -p no:randomly-> 608passed, 3 failed. The three are the agent-name fixture mismatches
(
ravenagainstRaven-General) that fail the same way on theuntouched tree:
test_direct_chat_round_trip_leaves_records_and_a_handoff,test_a_direct_turn_registers_the_instance_it_addressed,test_a_turn_on_an_instance_lane_still_records_against_the_session.ui-web:node node_modules/vitest/vitest.mjs run-> 253 files,4238 passed, no
Errorsline beside the summary; the second testroot,
node node_modules/vitest/vitest.mjs run --no-file-parallelism scripts/gates-> 51 files, 229 passed;npm run type-check-> exit0;
npm run lint-> 0 errors and 4 warnings, all of thempre-existing and in a file this change does not touch;
npm run gen:check-> the 248-method contract matches.ruff checkandruff format --checkover the lint targets ->pass;
lint-imports-> 10 kept, 0 broken; the repo's own gates --scripts/check_commit_messages.py origin/main..HEAD,scripts/check_large_files.py origin/main...HEAD,scripts/check_source_language.py origin/main...HEAD,scripts/check_pr_title.pyandscripts/check_pr_body.py-> exit 0;the pinned
commitlintover the branch's commits -> exit 0.deptryreports 12 findings, none of them in a file this changetouches, and this change adds no import at all;
ty checkcould notbe run in this environment (it resolved a different interpreter's
site-packages), so its verdict is the
python lintjob's, whichpasses on this head.
Not run at this head:
tests/integration/as a whole, the fullpytest -q -n 4sweep, theui-tuisuite, andmake ci'sbridgetree, none of which this change touches.
No user-facing doc describes the stop button, so none changes.
Risk
User-visible behavior: a stop that lands just after its turn finished
leaves the next turn running and holds the send queue, instead of
going idle and sending the queued message under it; the stop button
comes back once that turn's first frame arrives. Until then the button
reads Send, which queues. The question of that turn is drawn when its
opening frame arrives, and a message that ran as a turn of its own has
its bubble moved out of the ended turn.
Backward compatibility:
busyis optional. The web UI reads a refusalwithout it the way it read every refusal before, and the terminal UI
and the ACP adapter ignore the reply. The assertions on a refused
reply in
tests/test_rpc_turn_cancel.pynow include itsbusyvalue.Security surface is unchanged -- no new endpoints or privileges;
busydescribes only the lane the cancel resolved to, on the caller'sown session. Rollback: revert the PR.
Related Issues
#981