Skip to content

fix(*): keep the context ring right after failed and empty-ending turns - #1126

Open
LivXue wants to merge 10 commits into
mainfrom
fix/context_ring_failed_and_empty_turns
Open

LivXue wants to merge 10 commits into
mainfrom
fix/context_ring_failed_and_empty_turns

Conversation

@LivXue

@LivXue LivXue commented Oct 27, 2026 •

Copy link
Copy Markdown
Member

Summary

A follow-up to #1119, which stores the window each reply's call measured and has session.resume read it back. Some turns still left the context ring on a number the reader never saw live, and three notes were left on #1119 after it merged. This resolves all of them.

The branch is rebased onto the current main. The only conflict was one import line in raven/cli/tui_commands.py, where main's Callable and this branch's Any merged into one; a range-diff against the pre-rebase branch shows every commit's change unchanged apart from that line.

  1. A turn that ends on an empty reply. A turn answered through the message tool can end on a call whose reply is empty: recovery stands scaffolding in for it and then gives up, or recovery is off and the empty row is dropped on save. The live meter showed that call's window, but no stored row carried it, so a resume showed the turn's earlier tool-call reading (6200 against a live 7400). The reading of a call whose reply is not filed now goes on the newest row the turn keeps (_stamp_context_used_on_kept, skipping the _TURN_TRANSIENT_KEYS rows dropped before persistence); that row ends the window the call measured. A call that reports no usage leaves the reading before it in place.

    A turn that fails before any of its calls filed a row (its first reply empty with recovery off, every retry empty, or a provider error) had nowhere to keep the reading: the newest row it keeps is its question, filed before the first call, so the stamp never reached disk and a resume fell back to an estimate (22 against a live 7000). The turn now holds the newest reading its calls measured, across attempts, and the marker _save_broken_turn files carries it, whether the turn was answerless, stopped or crashed; that marker is the turn's newest row.

    A turn that answered through the message tool and then ended on a provider error kept the reading of the call before it (6200 against a live 7100): the error exit left before that call's reading went on the newest kept row. It now puts it there, as an empty reply's exit does.

  2. A turn that fails or is stopped. A failed turn's error event carried no usage, so the page's ring stayed on the turn before (3100) while a resume read the turn's own stored rows (6200 on the code fix(*): restore the context ring from the window the last call measured #1119 merged, 7000 with the first change here), and a stopped turn's cancel had the same gap (3100 live, 6200 after a reload). The RPC layer now keeps each streamed turn's usage sink under its turn id in live_usages, a map build_rpc_spine shares with turn.cancel: a failed turn's error and a stopped turn's cancelled_by_client error both carry what the turn measured, as an optional usage in message.complete's shape, and every end of a turn lets go of its entry. The map is keyed by turn rather than by lane, because a queued turn shares its lane with the turn that is running, and it stays in raven/rpc/spine.py, where its only writer and readers are, so the kernel's TurnFailed is unchanged. The usage is sent only when it read a window (reported_usage), and the loop moves the sink's reading only on a call that read one, on the terms the turn's filed reading moves on: a last call that reported zeros leaves the ring on the turn's reading instead of emptying it. Both entrances, bootstrap and the TUI's own server, hand the spine and the turn methods the same map. The page applies the usage ahead of a stop's own early exits, and it now reads every report the ring is drawn from, a completion, an error and a resumed session's usage, through one helper (applyUsage in ui-web/src/state/session/ctxReport.ts); a completion keeps its prompt-count fallback and a resumed session's zero still draws an empty ring. The TUI applies it in chatStream's onError, the typed turn stream a turn's frames arrive on, for a failed turn and a stopped one. Added to rpc-schema/openrpc.json, raven/rpc/models.py, both generated clients and the TUI's hand-written gatewayTypes.ts.

  3. A call that failed in transport. A response the provider flagged as an upstream transport failure (upstream_transport_failure) carries the upstream's account of a call that failed, and that account can be what flagged it, a prompt billed at a size the request cannot have been. It was taken as a window reading: shown live, stamped on the newest kept row and fed to the proactive compaction trigger. Such a call now gives no reading (_reads_no_window); the gauge stays on the call before it and only the turn's cost moves on.

  4. The notes left on fix(*): restore the context ring from the window the last call measured #1119 after it merged:

    • A stored context_used past float range raised OverflowError on the way to a percentage, and session.resume then handed the reader a freshly minted empty session carrying the 401-digit reading; one under that range but past any window drew an absurd ring. A reading over a billion tokens now counts as no reading, so the newest real one stands.
    • The _window_reading docstring named a transport_failure.never_processed that does not exist and called its add-back the same as one that adds cache counts unconditionally. It now states its own rule: add the cached counts back only where the provider reports them apart from prompt_tokens.
    • The resume fallback's comment named loop/main.py as a get_history caller, which it is not. It now names the two that are.

Type

  • Fix
  • Feature
  • Docs
  • CI / tooling
  • Refactor
  • Other

Verification

All on this head, on top of the current origin/main, Python 3.12.13:

  • Each change went red first for its reason, then green:

    • empty reply: pytest tests/test_agent_loop_usage_sink.py -k ends_on_an_empty_reply (live 7400 or 7000, restored 6200) -> 3 passed;
    • a failed turn that filed no row: pytest tests/test_agent_loop_usage_sink.py -k "failed_turn or cut_short" (the failure marker carried no reading, so a resume read an estimate of 22, 35, 35 and 24 against a live 7000, 7400, 7000 and 7100, and a stopped or crashed turn's marker carried none either) -> 7 passed, one of them the guard that a turn which measured nothing files nothing, green on both sides;
    • an answered turn that ends on a provider error: pytest tests/test_agent_loop_usage_sink.py -k answered_turn_that_ends_on_a_provider_error (restored 6200 against a live 7100) -> 1 passed;
    • failed turn, through the RPC layer: the wire tests pass on both sides, and the all-zero cases of test_a_failed_turn_hands_its_measured_usage_to_its_error went red (a last call that read no window still sent its zeros); pytest tests/test_rpc_spine.py -k "measured_usage_to_its_error or keeps_what_a_turn_measured or lets_go or non_owning" -> 10 passed, the map's own cases written with it; the failed case of vitest run src/__tests__/chatStream.test.ts in ui-tui (the bar did not move on the typed turn stream) -> 58 passed;
    • stopped turn: pytest tests/test_rpc_turn_cancel.py -k stopped_turns_cancel (the cancel carried no usage) -> 4 passed; the stop cases of vitest run src/state/session/pipeline.events.test.ts in ui-web (the arm left before the ring) -> 42 passed, and of chatStream.test.ts in ui-tui (the bar did not move);
    • transport failure: pytest tests/test_agent_loop_usage_sink.py -k failed_in_transport (the live gauge read the failed call's 5 tokens instead of 6200, and on a streamed turn, the path RpcTurnRunner takes, the sink, the failure marker and a resume read the 2 tokens of the call the upstream failed) -> 4 passed. The streaming exit's own verdict is main's since feat(*): align empty-reply recovery with Hermes and let a turn end in silence #1092 (transport_failure_response, which both exits return), so this branch no longer changes the providers;
    • a last call that read no window: pytest tests/test_agent_loop_usage_sink.py -k "last_call_that_read_no_window or whole_usage" (a block of zeros emptied the sink, so the error carried no usage while the marker and a resume read 6200) -> 2 passed, one of them the guard that an answered turn whose calls read no window still sends a whole TurnUsage, green on both sides;
    • reading bound: pytest tests/test_rpc_session.py -k no_token_count (a reading past any window was used, and one past float range turned the resume into a fresh empty session) -> 7 passed.
    • Written after or with what they pin, so the mutants below are what show they bite: pytest tests/test_rpc_bootstrap.py tests/test_cli_tui_commands.py -k share_one_map (both entrances hand the spine and the turn methods one map) -> 2 passed, and vitest run src/state/session/ctxReport.test.ts in ui-web (the one helper) -> 4 passed.
  • Rebased onto main, which carries feat(*): align empty-reply recovery with Hermes and let a turn end in silence #1092: test_a_switch_after_a_streamed_error_asks_the_fallback_the_original_question compared the question a fallback model is asked with the first request dict for dict, and failed on the _context_used key the empty-reply stamp puts on the question when the first reply only thought. No provider sends that key (each sends only its allowed message keys), so the test now compares the two requests as they are sent; it still fails when the switch leaves the prefill on -> 446 passed in tests/test_agent_loop_empty_recovery.py.

  • Mutation: 40 in-place mutants at this head, all killed, each turning red the case it targets: for the empty reply, drop the kept-row stamp, stamp a transient row, walk from the oldest row, or drop the zero-usage guard, and for the provider error, drop its stamp; for the failure marker, the loop records no reading, the attempt is handed no holder for it, any one of the three rescue paths passes none, the marker files none, or the holder keeps the oldest reading; for a failed or stopped turn's usage, the runner registers nothing, a failure reads its entry without letting go, a completion keeps its entry, the failure or either cancel path sends none, the rule accepts any usage or the cancel skips it, emit_error drops it, or any of the four hand-offs of the map drops it; for the transport failure, it counts as a reading, it moves no cost, or the window ignores the check; for the live reading, a call that read none overwrites it, so does a reading of zero, or such a call leaves a sink that holds no reading without its counts; on the page, the error arm draws nothing, the helper ignores the fallback or takes it for a resumed zero, a completion passes none, or the helper draws without a report; in the TUI, onError ignores the usage or reads it only off a failure, and the gateway handler ignores it; and drop the reading ceiling. The files were restored byte for byte.

  • Related files: pytest -q -n 2 over 33 files (agent loop usage sink, iteration hooks, stream, overflow, part durations, empty recovery, broken turn, max-iteration synthesis, failure text, transport failure paths; agent hook contract; message tool turn-local state and RPC route; rpc session, session naming, init bundle, spine, turn send, turn cancel, turn subscribe, bootstrap, contract shapes, schema match, turn wire shape; spine events, scheduler and lane, kernel budget; session manager; acp updates; the TUI command server; the TUI in-flight cancel integration) -> 2459 passed; and over 15 provider and streaming files (litellm provider attribution, model names, model overrides, reasoning, stream, stream end and timeout; provider stream fallback, stream idle budget and protocol; LLM error patience, reply room, call record, the Mirasim ACP stream, tool name normalization) -> 658 passed.

  • Full suite: uv run --frozen --python 3.12 --all-extras pytest -q -n 4 -> 37247 passed, 9 failed, 255 skipped. All nine also fail on a clean origin/main worktree under the same interpreter, in the same files: five tests/test_config_update_providers.py cases that pick up the dev box's HTTP proxy, one terminal-style case in tests/test_cli_theme.py, one LibreOffice conversion, one tests/test_install_script.py case that finds a node elsewhere on PATH, and one tests/test_subagent_node_runtime.py case about what cannot be read. So 0 introduced.

  • Front ends: vitest run src/state/ in ui-web (736 passed), tsc --noEmit in ui-web (with --listFiles showing this tree's ctxReport.ts) and ui-tui, eslint on the touched client files, prettier on the touched ui-tui files and the two new ui-web ones (the edited ui-web files are not prettier-formatted on main either and are left as they were), the TUI's chatStream, createGatewayEventHandler and gatewayTypesDrift tests (101 passed), and both generators' --check: all clean. The full ui-tui suite: 10 failed, 2270 passed; nine are the render cases that also fail on a clean origin/main worktree (which showed 11 in the same run, with different extra ones), and the tenth, modelPicker.test.tsx's OAuth-providers case, passes 26 of 26 on its own in three runs.

  • ruff check, ruff format --check, ty check on the touched modules, lint-imports (10 kept, 0 broken), scripts/check_commit_messages.py, commitlint over the range, scripts/check_large_files.py, scripts/check_source_language.py and git diff --check: all clean.

  • Real browser, at this head: an isolated raven web against a scripted OpenAI-compatible upstream, four runs of an answer followed by a turn that ends without one. Where that turn calls a tool and then gets empty replies until the loop gives up, and where its first reply is already empty, the ring read Context 7.0k/40k 18% after the failure and again after a reload, the error frame carrying context_used: 7000; the first run on the code fix(*): restore the context ring from the window the last call measured #1119 merged kept Context 3.1k/40k 8% after the failure and read Context 6.2k/40k 16% after the reload, and the second without the failure-marker change fell back to the turn before's 3.1k on the reload. Where the empty replies after the tool call are billed at one token each, as an upstream that fails a call bills it, the ring read Context 6.2k/40k 16% after the failure and again after a reload, the error frame carrying context_used: 6200; on the head the review was on, before the streaming exit carried the verdict, the same run read Context 2/40k 0% on both. Where the reader stops the turn while the call after its tool call hangs, the ring read Context 6.2k/40k 16% right after Stop and again after a reload, the cancel frame carrying context_used: 6200; the same run without the cancel change stayed at Context 3.1k/40k 8% after Stop and read Context 6.2k/40k 16% only after the reload.

  • Relevant tests pass locally

  • Relevant lint / type checks pass locally

  • User-facing docs or screenshots are updated when needed

Left unchecked as not applicable: no docs page describes the ring or the error event's fields.

Risk

  • Security impact considered

  • Backward compatibility considered

  • Rollback path is clear for risky changes

  • Security: the usage on the error and cancel events is integers the runtime computed from the provider's usage reports. It is kept in an RPC-layer map keyed by turn id that each turn's end clears, and nothing new reaches a provider payload.

  • Compatibility: the error event, a failed turn's and now a stopped turn's, gains an optional usage, which a client that does not read it ignores. A call flagged as an upstream transport failure no longer counts as a window reading, so it no longer feeds the proactive compaction trigger either. A turn that ends on an empty reply leaves its reading on the newest kept row, which can be a tool row, about 25 bytes more.

  • Rollback: revert the squash commit.

Related Issues

#1119 (the pull request this follows; nothing to close)

@LivXue
LivXue requested review from 0xKT and gloryfromca October 27, 2026 08:18

@gloryfromca gloryfromca left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Blocking: failed turns with no persisted tail still reload with a different context reading.

I found one blocking persistence gap. I reviewed the full diff and the relevant loop persistence path, RPC/lane propagation, both frontend consumers, schema/generated types, and the history that introduced stored context readings. I also checked the repository rules and domain terms, backward compatibility, architecture boundaries, and that the added tests were not weakened.

Verification:

  • uv run --python 3.12 --all-extras pytest -q -n 0 tests/test_agent_loop_usage_sink.py tests/test_rpc_session.py tests/test_rpc_spine.py tests/test_spine_events.py tests/test_spine_scheduler_lane.py -> 393 passed.
  • A real-loop reproduction with a first-call empty response measured context_used=7000, but persisted both the user row and failure marker without context_used.
  • The focused web and TUI tests could not start because this worktree has no installed Node dependencies (vitest: not found; esbuild: not found).
  • git diff --check github/main...HEAD passed.

Comment thread raven/agent/loop/turn_path.py Resolved
@LivXue

LivXue commented Oct 27, 2026

Copy link
Copy Markdown
Member Author

Windows compatibility review — PR #1126 @ 50891ec9e36ccb9088922fa87cefd456b16d1a32

Scope: full base 26352da0 → this head (21 files, +409/−25): the context-ring follow-up to #1119 — _shared.py/turn_path.py stamp the window reading on the entry that stands for a failed or empty-ended call, spine/events.py and spine/scheduler.py carry it, rpc/spine.py/rpc/methods/session.py deliver and read it, plus RPC models/clients and tests.

Result: no Windows-specific issue. The change is token accounting and event plumbing: integers on message dicts and event payloads, no path, filesystem, subprocess, signal, encoding, or platform primitive.

Verification (native Windows 11, CPython 3.12.13, isolated worktree D:\tmp\rv-1126 at this head; scratch HOME/USERPROFILE, PYTHONUTF8=1; node_modules borrowed read-only from the main checkout):

pytest tests/test_agent_loop_usage_sink.py tests/test_rpc_session.py \
       tests/test_rpc_spine.py tests/test_spine_events.py \
       tests/test_spine_scheduler_lane.py -q
→ 393 passed in 14.28s

vitest run src/state/session/pipeline.events.test.ts
→ 1 file, 38 passed

Limitations: no live provider or gateway; the TUI side was not run (the change there is the generated client only).

@LivXue
LivXue force-pushed the fix/context_ring_failed_and_empty_turns branch from 50891ec to e1bcea5 Compare October 27, 2026 10:10
@LivXue
LivXue requested a review from gloryfromca October 27, 2026 10:10

@gloryfromca gloryfromca left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No blockers; this can merge as far as I am concerned.

The reported persistence gap is fixed and the thread is resolved. I reviewed the new commit and refreshed the full PR diff against the current github/main, including the loop persistence and rerun callers, broken-turn rescue paths, RPC/UI behavior, backward compatibility, repository rules and domain terms, architecture boundaries, history, and the new tests for whether they genuinely exercise the failure.

Verification: uv run --python 3.12 --all-extras pytest -q -n 0 tests/test_agent_loop_usage_sink.py tests/test_agent_loop_broken_turn.py tests/test_agent_loop_max_iter_synthesis.py tests/test_rpc_session.py tests/test_rpc_spine.py tests/test_spine_events.py tests/test_spine_scheduler_lane.py -> 434 passed. git diff --check github/main...HEAD also passed.

@LivXue

LivXue commented Oct 27, 2026

Copy link
Copy Markdown
Member Author

Windows compatibility re-review — PR #1126 @ e1bcea525554e270c9c5cfcd5336cb85ae7365bb

Scope: the head moved from the previously reviewed 50891ec9. Against the new main tip 06509f8e, the PR's own change grew to 21 files (+576/−29): besides the event plumbing, failed and empty-ended turns now carry the newest measured window — _stamp_context_used_on_kept writes it on the newest non-transient row, and _save_broken_turn files it on the cancelled/failed marker (context_used), which session.resume reads back; the usage-sink and RPC/spine tests grew accordingly.

Result: no Windows-specific issue. The additions are integer window readings on message dicts and event payloads plus a transient-key filter; no path, filesystem, subprocess, signal, encoding, or platform primitive.

Verification (native Windows 11, CPython 3.12.13, isolated worktree D:\tmp\rv-1126-n at this head; scratch HOME/USERPROFILE, PYTHONUTF8=1; node_modules borrowed read-only from the main checkout):

pytest tests/test_agent_loop_usage_sink.py tests/test_rpc_session.py \
       tests/test_rpc_spine.py tests/test_spine_events.py \
       tests/test_spine_scheduler_lane.py -q
→ 401 passed in 13.80s

vitest run src/state/session/pipeline.events.test.ts
→ 1 file, 38 passed

Limitations: no live provider or gateway; the TUI side was not run (generated client only).

Comment thread raven/spine/events.py Resolved
@0xKT

0xKT commented Oct 27, 2026

Copy link
Copy Markdown
Member

Not a blocker -- smaller points from the same pass, separate from the kernel thread.

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.

  1. After Stop, the live ring and a resume disagree. A cancelled turn (CancelledError is not caught by the runner's except Exception) sends no usage, so the live ring stays on the previous turn's reading. The stopped turn's rows can still carry a newer reading, which a later session.resume shows. Either send the reading on the cancel path too, or keep cancelled turns from stamping one.
  2. Nits:
    • A call that reports all-zero usage drops a failed turn's live ring to 0 while the stored record keeps the earlier reading.
    • The web applies the error's usage with its own field spelling instead of reusing the message.complete path.
    • A laundered transport failure's accounting is filed as a window reading.
    • The private-dict access BaseException.__dict__["__dict__"].__get__(exc) is now written twice for the writer and twice for the reader, with no shared helper; this goes away if the channel moves into rpc.
    • The kernel docstring names the wrong shape for the finished/failed usage split.

@LivXue

LivXue commented Oct 28, 2026

Copy link
Copy Markdown
Member Author

Thanks. All six points are addressed in this push.

  1. After Stop: the cancel now carries the reading (8fe9e10, 7610c2e). turn.cancel takes the running turn's usage sink from the live_usages map before cancelling it, and the cancelled_by_client error carries what the turn measured. The page applies it ahead of the stop's own early exits, and the TUI in chatStream's onError. In a real browser, stopping a turn mid-call after a 6200-token tool call: the ring read Context 6.2k/40k 16% right after Stop and again after a reload; on the reviewed head it stayed on the turn before's 3.1k after Stop and read 6.2k only after the reload.
  2. Nits:
    • All-zero usage: a failed turn's error and a stopped turn's cancel now carry usage only when it read a window (reported_usage), so a last call that reported zeros leaves the ring where it was. (8fe9e10)
    • The web's own field spelling: a completion, an error and a resumed session's usage now all reach the ring through one helper, applyUsage in ui-web/src/state/session/ctxReport.ts. A completion keeps its prompt-count fallback, and a resumed session's zero still draws an empty ring. (7610c2e)
    • The laundered transport failure: a call flagged upstream_transport_failure now gives no window reading anywhere, neither the live gauge, nor the stored stamp, nor the proactive compaction trigger, while the turn's cost still counts it. (124bc78)
    • The private-dict access and the kernel docstring went with the kernel field. (8fe9e10)

One more thing, found while wiring the TUI side: the failed-turn change in the earlier push sat in createGatewayEventHandler, which only receives top-level notifications, while a turn's frames go to the typed stream in chatStream.ts. So the TUI bar did not move on a failed turn either. onError there now applies the usage for a failed turn and a stopped one, with tests on that path.

@LivXue
LivXue requested review from 0xKT and gloryfromca October 28, 2026 02:12

@gloryfromca gloryfromca left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No blockers; this can merge as far as I am concerned.

I reviewed the three-commit delta and refreshed the full PR diff against current github/main. The failed/stopped usage channel now stays inside RPC, is keyed by turn rather than lane, is shared by both server wiring paths, and is removed on every terminal event. Cancellation captures the live sink before unwind, both clients apply failure/stop usage before their early exits, zero/no-window reports preserve the prior ring, and upstream transport failures no longer overwrite a real window reading. I also rechecked the persistence callers, schema/generated shapes, backward compatibility, repository rules and domain vocabulary, architecture boundaries, history, and whether the replacement tests exercise the reported failures.

Verification:

  • uv run --python 3.12 --all-extras pytest -q -n 0 tests/test_agent_loop_usage_sink.py tests/test_agent_loop_broken_turn.py tests/test_agent_loop_max_iter_synthesis.py tests/test_rpc_session.py tests/test_rpc_spine.py tests/test_rpc_turn_cancel.py tests/test_rpc_bootstrap.py tests/test_cli_tui_commands.py tests/test_spine_events.py tests/test_spine_scheduler_lane.py -> 574 passed.
  • git diff --check github/main...HEAD passed.
  • The focused web and TUI tests could not start because Node dependencies are not installed in this worktree (vitest: not found; esbuild: not found).

@LivXue

LivXue commented Oct 28, 2026

Copy link
Copy Markdown
Member Author

Windows compatibility re-review — PR #1126 @ 124bc78b54c1f84c3c6e6f8e5e2492ab014667a6

Scope: the head moved from the previously reviewed e1bcea52. The delta is 30 files; the raven/** side (+135/−48) adds _reads_no_window/_call_window (a response the provider classified as an upstream transport failure contributes no window reading, while its cost still flows), threads live_usages through build_rpc_spine/turn.cancel so a stopped turn's reading can be read back, and the Web side adds ctxReport/ctx chip plumbing.

Result: no Windows-specific issue. The additions are integer window readings and dict slots passed through existing in-process wiring; no path, filesystem, subprocess, signal, encoding, or platform primitive.

Verification (native Windows 11, CPython 3.12.13, isolated worktree D:\tmp\rv-1126-n2 at this head; scratch HOME/USERPROFILE, PYTHONUTF8=1; node_modules borrowed read-only from the main checkout):

pytest tests/test_agent_loop_usage_sink.py tests/test_rpc_session.py \
       tests/test_rpc_spine.py tests/test_spine_events.py \
       tests/test_spine_scheduler_lane.py -q
→ 405 passed in 15.06s

vitest run src/state/session/pipeline.events.test.ts src/state/session/ctxReport.test.ts
→ 2 files, 46 passed

Limitations: no live provider or gateway; the TUI side was not run (generated client only).

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Blocking: the "a call that failed in transport reads no window" rule (124bc78b5) never fires on the streaming path, the only path a gauge has in production. On that path this PR now sends the failed call's reading to the client and files it, so after a failed TUI/web turn both the live ring and a resume show about 0%. On the tip, a resume of the same turn showed the real reading.

Mechanism.

  • _reads_no_window (raven/agent/loop/_shared.py:190-200) keys on error_classification.category == "upstream_transport_failure". Only the non-streaming exit sets that: LiteLLMProvider._parse_response, litellm_provider.py:1126-1138, reached from chat only.
  • The streaming exit asks the same verdict, but only to reconnect. Once the reconnects are spent it returns the empty response with its usage and no classification (raven/providers/streaming.py:508-522, "No verdict here").
  • RpcTurnRunner.run always streams (raven/rpc/spine.py:266, stream=True, usage_sink=...). So on the TUI / served page / ACP, _call_window takes the failed call's 1-2 tokens. That number then reaches:
    • the gauge and the sink (turn_path.py:1374-1414);
    • the kept-row stamp;
    • the failure marker (_save_broken_turn(context_used=...));
    • the new error usage (reported_usage, rpc/spine.py:693), which both clients apply.

Reproduced (lane probe, one fault, two exits; the streaming case is driven the way run_turn(stream=True) drives it):

head, streaming     : sink 2, marker 2, resume 2, error_event.usage.context_used 2
head, non_streaming : 6200 everywhere
tip,  streaming     : sink 2, marker None, resume 6200, error usage not sent

With an honest control (6100+0, no verdict), every reading is 6100 on both exits. So the non-streaming exit, which has no gauge in production, behaves as the commit says, and the streaming exit does not. The test test_a_call_that_failed_in_transport_reads_no_window gives a usage sink to a non-streaming turn, a pairing no production caller makes, so it passes without reaching the real path.

Why blocking. This is the exact fault 124bc78b5 says it removes, and the PR's other commits newly expose it: the error usage and the marker / kept-row reading are new here. A failed streamed turn now leaves a worse ring and a worse resume than the tip did.

Fix direction: carry the streaming verdict on the response stream_llm_call returns once reconnects are spent (attach the same classification, keeping the empty-response recovery as the owner of what happens next), so the one rule in _reads_no_window sees it on both exits. Then pin it with a streaming-turn test that has a usage sink.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed, and reproduced. On the reviewed head, a streamed turn whose call after a tool call came back empty and billed at one token put 2 on the sink, the failure marker and a resume, where main resumed at 6200. As you say, test_a_call_that_failed_in_transport_reads_no_window gave the sink to a non-streaming turn, a pairing production never makes, so it never reached this exit.

Fixed in 7ab0ba4, the way you suggest. Once its reconnects are spent, stream_llm_call returns the empty response with the classification the non-streaming exit gives the same call, both taken from one transport_failure_verdict(), so _reads_no_window sees it on either exit. The finish reason stays stop, so the loop's empty-response recovery still owns what happens next: every reader that retries or falls back on a classification checks finish_reason == "error" first.

Tests, red on the reviewed head and green now:

  • test_a_stream_that_stays_empty_carries_the_verdict_the_other_exit_gives (tests/test_transport_failure_paths.py): the spent stream carries the classification _parse_response gives the same call (it carried none);
  • test_a_streamed_call_that_failed_in_transport_reads_no_window[failed|answered] (tests/test_agent_loop_usage_sink.py): a streamed turn with a usage sink, the pairing RpcTurnRunner makes; the sink and a resume read 6200 (the sink read 2).

test_a_silent_stream_with_honest_usage_carries_no_verdict is your honest control: an empty stream billed at a size its prompt can have carries no verdict. It passes on both heads.

In a real browser, through raven web against a scripted upstream that has the turn call a tool and then sends empty replies billed at one token until the loop gives up, the ring read Context 6.2k/40k 16% after the failure and after a reload, with context_used: 6200 on the error frame. On the reviewed head the same run read Context 2/40k 0% on both.

@0xKT could you take another look, and resolve the thread if this settles it?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A follow-up after rebasing onto main: #1092 merged with its own fix for this gap. transport_failure_response is what both exits now return once the verdict stands, so on the streaming exit a call the upstream failed comes back, once its reconnects are spent, as finish_reason="error" with the upstream_transport_failure classification, the same as the non-streaming exit. I dropped transport_failure_verdict and the classified stop response in favour of it, so what I wrote above about the finish reason staying stop no longer holds: the error ladder owns that call now, as #1092 decided.

_reads_no_window reads the same classification, so the gauge keeps the reading of the call before it on that path too. The branch keeps the streamed-turn test, test_a_streamed_call_that_failed_in_transport_reads_no_window[failed|answered], now in 0b02cb2: on the rebased head the sink and a resume read 6200, and it fails with the reading rule removed. In a real browser on the rebased head the same run still reads Context 6.2k/40k 16% after the failure and after a reload.

@0xKT

0xKT commented Oct 28, 2026

Copy link
Copy Markdown
Member

Not a blocker - non-blocking items from the delta acceptance on 124bc78b.

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.

  1. Zero usage block: wire and record disagree. The loop clears and rewrites the usage sink on every call whose response.usage is truthy (turn_path.py:1398-1414). But window_reading, which the marker is filed from, only moves on a reading > 0 (:1372-1378). So when a turn's last usage-bearing call reports an all-zero block (one that is not flagged as a transport failure, see transport_failure.py:236-240), reported_usage (rpc/spine.py:573-583) sends no usage on the error or cancel. Meanwhile the marker and a resume keep the earlier reading: measured 6200 on the marker, 6200 on resume, and no usage on the wire, against a control (last call {}) that gives 6200 on all three. _stopped_usage's docstring (rpc/methods/turn.py:779-780, "the record it leaves is filed from the same sink") is not true of the code: the marker is filed from reading.
  2. Two stores of a turn's usage. live_usages sits beside the existing usages owner in build_rpc_spine. They serve different ends (failed/stopped vs answered), which is fine, but the "only a usage that read the window is sent" rule now lives only in reported_usage. Worth one sentence saying that the success path's rule is the same.

Tests here on the merged tree: the 6 changed Python test files via the board runner gave 469 passed. ui-web ctxReport.test.ts + pipeline.events.test.ts gave 46 passed. ui-tui chatStream.test.ts gave 58 passed.

@gloryfromca gloryfromca left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Blocking: the streaming transport-failure path must preserve the prior real window reading.

The newly demonstrated streaming-path gap invalidates my prior clean stance. I verified the mechanism in raven/providers/streaming.py: once reconnects are exhausted, the streaming response returns without the upstream_transport_failure classification that _reads_no_window requires, while the production RPC runner always uses stream=True. The open finding from @0xKT owns the requested correction; I am not duplicating or re-grading that thread.

Verification: uv run --python 3.12 --all-extras pytest -q -n 0 tests/test_agent_loop_stream.py tests/test_agent_loop_usage_sink.py tests/test_transport_failure_paths.py -> 152 passed. Those passing tests do not cover the reported production streaming pairing, which is the gap the open thread identifies.

@LivXue

LivXue commented Oct 28, 2026

Copy link
Copy Markdown
Member Author

@0xKT thanks for both notes; they are in 1cc9bb6.

  1. Zero usage block. Reproduced as you measured: the last call's block of zeros emptied the sink, so the error carried no usage while the marker and a resume read 6200. The loop now moves the sink's reading on the terms the filed reading moves on, a reading above zero. A call that read no window, a block of zeros or a transport failure alike, moves only the turn's cost, which replaces the transport-only branch from 124bc78b5. A sink that holds no reading yet is still filled whole, because an answered turn's message.complete sends it as it is and its counts are what make it a TurnUsage.

    • test_a_last_call_that_read_no_window_leaves_the_live_reading_where_the_record_is, a streamed turn, red on the reviewed head (the sink read 0): the sink, what reported_usage sends, the marker and a resume now all read 6200;
    • test_a_turn_whose_calls_read_no_window_still_reports_a_whole_usage guards the other side: an answered turn whose only call reports zeros still sends a whole TurnUsage. It passes on both heads and turns red if the sink skips a call that read no window before it holds a reading.

    _stopped_usage's docstring now says what holds: the sink carries the reading the stopped turn files because the loop moves both on the same terms, not because the record is filed from it.

  2. Two stores. One paragraph in the RpcTurnRunner docstring: usages and live_usages both hold the loop's sink, whose reading moves only on a call that read one, so either end reports the reading its record keeps; reported_usage adds one rule for an error, that a turn that read no window sends no usage. The sentence you suggested, that the success path's rule is the same, did not hold before 1: message.complete sends the sink as it is, so an emptied reading reached it too.

@gloryfromca the streaming Blocking in your latest review is fixed in 7ab0ba4, with the details on the thread above. If nothing else stands in your way, could you approve?

@LivXue
LivXue requested review from 0xKT and gloryfromca October 28, 2026 07:20

@gloryfromca gloryfromca left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No blockers; this can merge as far as I am concerned.

The streaming-path blocker that invalidated my prior stance is fixed on this revision. Once reconnects are spent, the streaming exit now carries the shared upstream_transport_failure verdict while still returning an empty response for the loop's recovery; _call_window therefore preserves the prior real reading on the production RPC path. The usage sink now advances its context reading only when the persisted reading advances, so all-zero/no-window calls keep wire, marker, and resume aligned while cost still moves. I reviewed the two-commit delta, its production streaming callers, persistence/RPC consumers, shared classification contract, backward compatibility, architecture boundaries, repository rules, and the replacement regression tests. I did not resolve or re-grade the other reviewer's thread.

Verification: uv run --python 3.12 --all-extras pytest -q -n 0 tests/test_agent_loop_stream.py tests/test_agent_loop_usage_sink.py tests/test_transport_failure_paths.py tests/test_rpc_spine.py tests/test_rpc_turn_cancel.py -> 290 passed. git diff --check github/main...HEAD also passed.

@LivXue

LivXue commented Oct 28, 2026

Copy link
Copy Markdown
Member Author

Windows compatibility re-review — PR #1126 @ 1cc9bb67f9295755539420b7b6922374c2c5a823

Scope: the delta since the previously reviewed head 124bc78b — two commits, 8 files (+176/−20): a spent stream now carries the same upstream_transport_failure verdict as the non-streaming exit, and the loop's live usage sink moves its window reading only on a call that actually read one, with the matching reported_usage rule for an error turn. Reviewed for Windows behaviour and run on native Windows.

Result: no Windows-specific defect. The change is accounting and classification logic over token counts and ErrorClassification values; the transport verdict is decided from prompt/output accounting, not from socket exception types, so there is no Windows-specific errno/WinError path to diverge. No path, subprocess, signal, encoding, or event-loop surface is touched (raven/rpc/turn.py and raven/rpc/spine.py changes here are docstrings only). This also answers 0xKT's two non-blocking notes from the previous acceptance pass.

Verification on native Windows 11, detached worktree D:\tmp\rv-1126 at the reviewed head, offline venv (uv sync --frozen --offline --all-extras --no-extra segmenter --dev), scratch HOME/USERPROFILE, PYTHONUTF8=1, PYTHONPATH pinned to the worktree:

  • uv run --no-sync pytest tests/test_agent_loop_usage_sink.py tests/test_transport_failure_paths.py -q → 116 passed. No failures, so no base comparison was needed.

Coverage gaps: the full suite was not rerun for this delta; the provider paths here are exercised with stub streams, not live vendor calls.

@0xKT

0xKT commented Oct 28, 2026

Copy link
Copy Markdown
Member

Merge conflict: @LivXue, this PR no longer merges onto the current main tip 959cd0a9 (the #1092 merge). Measured with git merge-tree --write-tree 959cd0a9 1cc9bb67 -> exit 1, conflicts in:

  • raven/agent/loop/turn_path.py
  • raven/providers/litellm_provider.py
  • raven/providers/streaming.py
  • raven/providers/transport_failure.py
  • ui-web/src/state/session/stages.ts

The judge's verdict on 1cc9bb67 is clean, so a rebase onto main is what stands between this and the acceptance queue. Note that #1092 reworked the same streaming transport-failure path this PR's open thread is about (ba2e9a55f builds the upstream transport failure in one place), so the resolution in streaming.py / transport_failure.py is worth doing with that thread in mind.

@LivXue
LivXue force-pushed the fix/context_ring_failed_and_empty_turns branch from 1cc9bb6 to 07342fe Compare October 28, 2026 08:27
@LivXue

LivXue commented Oct 28, 2026

Copy link
Copy Markdown
Member Author

@0xKT rebased onto 959cd0a9f; it merges cleanly now. In streaming.py, transport_failure.py and litellm_provider.py I took #1092's side whole: transport_failure_response is what both exits return, so this branch no longer touches the providers, and the reply on the open thread says what that means for its fix. The other conflicts were both sides adding to the same lines, and keep both: in turn_path.py the reading carrier sits beside rungs_used and reply_awaited, and the empty-reply stamp goes after #1092's joined-thought block, before reply_kind; in stages.ts the error's applyUsage follows rt.silent = false. One test from #1092 compared a fallback's question dict for dict and met the stamp's _context_used key, which no provider sends; the description has the details.

@LivXue
LivXue requested a review from gloryfromca October 28, 2026 08:27

@gloryfromca gloryfromca left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No blockers; this can merge as far as I am concerned.

Reviewed the rebased head 07342fe80541. The range-diff shows that the branch dropped its duplicate provider patch and now relies on main's shared transport_failure_response; both streaming and non-streaming failures retain the upstream_transport_failure classification, so the PR's no-window rule still preserves the prior real reading. The turn_path.py and web stage conflict resolutions keep both main's empty-recovery/silent-turn behavior and this PR's reading carrier. The final test-only commit filters Raven's underscore bookkeeping before comparing requests, which matches the providers' request-message sanitization and does not relax the wire-level behavior being tested.

I covered the repository rules and context documents, the full github/main...HEAD diff, the rebase range-diff and relevant history, runtime/RPC/UI callers, wire compatibility, test-strength changes, and the kernel/RPC architecture constraint. The kernel remains unchanged; the live channel remains RPC-local and turn-keyed.

Verification: git merge-tree --write-tree github/main HEAD and git diff --check github/main...HEAD passed. uv run --python 3.12 --all-extras pytest -q -n 0 tests/test_agent_loop_stream.py tests/test_agent_loop_usage_sink.py tests/test_transport_failure_paths.py tests/test_rpc_spine.py tests/test_rpc_turn_cancel.py tests/test_agent_loop_empty_recovery.py tests/test_rpc_session.py tests/test_rpc_bootstrap.py tests/test_cli_tui_commands.py passed (1052 tests). The targeted web test command could not start because vitest is not installed; the targeted TUI command could not start because esbuild is not installed.

@LivXue

LivXue commented Oct 28, 2026

Copy link
Copy Markdown
Member Author

Windows compatibility re-review — PR #1126 @ 07342fe80541eaaadf9e6fae84d24ba19ba5d585

Scope: the branch is rebased onto current main; the PR's own diff at this head is 959cd0a9..07342fe8 (31 files, +1114/−40). range-diff against the previously reviewed head 1cc9bb67 shows the earlier commits carried forward with only the rebase's context changes (main's recovery/no_output refactor in turn_path.py and spine/events.py), and the new commits are the turn_usage-on-TurnFailed and web context-report work already present in that set. Re-ran the affected suites on Windows.

Result: no Windows-specific defect. The change is usage/context-reading plumbing across the loop, RPC methods, spine events, and the ui-web/ui-tui context display; it touches no path, subprocess, signal, socket, file, encoding, or permission surface.

Verification on native Windows 11, detached worktree D:\tmp\rv-1126b at the reviewed head (venv from the same lock), scratch HOME/USERPROFILE, PYTHONUTF8=1, PYTHONPATH pinned:

  • pytest tests/test_agent_loop_empty_recovery.py tests/test_agent_loop_usage_sink.py tests/test_cli_tui_commands.py tests/test_rpc_bootstrap.py tests/test_rpc_session.py tests/test_rpc_spine.py tests/test_rpc_turn_cancel.py -q → 1 failed, 924 passed
  • the same seven files at the new base 959cd0a9 → 1 failed, 890 passed
  • the same single failing ID on both (test_cli_tui_commands.py::test_python_dash_m_finds_the_script_beside_the_interpreter), an inherited Windows venv-layout case; the 34 added tests pass.
  • ui-web: vitest run src/state/session/ctxReport.test.ts src/state/session/pipeline.events.test.ts → 2 passed, 61 passed; ui-tui: vitest run src/__tests__/chatStream.test.ts src/__tests__/createGatewayEventHandler.test.ts → 2 passed, 100 passed.

Coverage gaps: the full suite was not rerun; no live RPC/UI session was driven end to end.

@0xKT

0xKT commented Oct 30, 2026

Copy link
Copy Markdown
Member

Merge conflict: @LivXue, this PR no longer merges onto the current main tip a0265d15 (the #1149 merge). Measured with git merge-tree --write-tree a0265d15 07342fe8 -> exit 1, one conflict:

  • raven/cli/tui_commands.py

The judge's verdict on 07342fe8 is clean, so a rebase onto main is what stands between this and the acceptance queue.

LivXue added 10 commits October 30, 2026 10:14
A turn answered through the message tool can end on a call whose reply
is empty: recovery stands scaffolding in for it and then gives up, or
recovery is off and the empty row is dropped on save. The live meter
showed that call's window, but no stored row carried it, so a resume
showed the turn's earlier tool-call reading (6200 against a live 7400
in the regression test).

An empty reply's reading now goes on the newest row the turn keeps,
skipping the transient rows dropped before persistence; that row ends
the window the call measured. A call that reports no usage leaves the
reading before it in place.
A failed turn never moved the page's context ring: its error event
carried no usage, so the ring stayed on the turn before while the
failed turn's own stored record, which session.resume reads, had moved
on (3100 live against 7000 restored after a turn that called a tool and
then never answered).

The RPC runner now sets what the turn measured on the exception it lets
out, the lane copies it onto TurnFailed.turn_usage the way it copies
full_detail, and the error event carries it as an optional usage in
message.complete's shape; the page moves the ring and the TUI its
context bar when one is present.
It rides the failure rather than the per-lane usage register, which a
queued turn shares with the turn that owns the lane.

The test that pinned TurnFailed as carrying no usage now pins that it
carries no outcome Usage, the type TurnEnded accounts with, and lists
turn_usage beside full_detail.
A stored context_used past float range raised OverflowError on the way
to a percentage in session.resume, which then handed the reader a
freshly minted empty session carrying the 401-digit reading; one under
that range but past any window drew an absurd ring. A reading over a
billion tokens now counts as no reading, so the newest real one stands.

Two notes are corrected as well. The _window_reading docstring named a
transport_failure.never_processed that does not exist and called its
add-back the same as one that adds cache counts unconditionally. The
resume fallback's comment named loop/main.py as a get_history caller,
which it is not.
A turn whose calls each came back empty or as a provider error filed no
row of its own, and lost its window reading. The stamp for an unfiled
reply went on the newest kept row, which was the turn's question; that
row is filed before the first call, so the stamp never reached disk.
The error event showed the live reading (7000) while a resume fell back
to an estimate (22).

The turn now holds the newest reading its calls measured, across
attempts, and the marker _save_broken_turn files carries it on all
three rescue paths: answerless, stopped and crashed. The marker is the
turn's newest row, so session.resume reads the window from it.

A turn that answered through the message tool and then ended on a
provider error kept the reading of the call before it (6200 against a
live 7100): the error exit left before that call's reading went on the
newest kept row. It now puts it there, as an empty reply's exit does.
A failed turn's reading reached its error event through the kernel:
the runner set it on the exception, the lane copied it onto
TurnFailed.turn_usage, and the RPC sink read it back. Its only writer
and its only reader both live in raven/rpc/spine.py, so the kernel
field and turn_usage_of are gone, and raven/spine, its tests and
CONTEXT.md are back to what main has.

The runner now keeps each streamed turn's usage sink under its turn id
in live_usages, a map build_rpc_spine shares with turn.cancel. A failed
turn's end takes its entry onto the error event, and every end lets go
of it. It is keyed by turn rather than by lane, because a queued turn
shares its lane with the turn that is running.

A stopped turn's cancel now carries the same reading. turn.cancel takes
the running turn's sink before cancelling it and attaches what the turn
measured, so a client's context ring moves with the record the stopped
turn files. The map reaches turn.cancel from both entrances, bootstrap
and the TUI's own server, the way direct_targets does.

Either event sends the usage only when it read a window: a last call
that reported zeros, or a sink with no reading, leaves the ring where
it was instead of emptying it.
A stopped turn's cancel now carries what the turn measured, and both
clients take it.

The page applies an error's usage before a stop's own early exits,
through applyUsage in state/session/ctxReport.ts. A completion and a
resumed session's usage go through it too, so the three reports read
the fields one way. A completion still falls back to its prompt count
when it measured no window, and a resumed session's zero still draws
an empty ring.

The TUI applies the usage in chatStream's onError, for a failed turn
and a stopped one alike. A turn's frames arrive on the typed turn
stream, so the earlier change, made in the gateway event handler,
never saw a turn's error.
A response the provider flagged as an upstream transport failure
carries the upstream's account of a call that failed, and that account
can be what flagged it: a prompt billed at a size the request cannot
have been. It was still taken as a window reading: shown on the live
gauge, stamped on the newest kept row, and fed to the proactive
compaction trigger.

Such a call now gives no reading. The gauge stays on the call before
it, the stored record keeps the earlier stamp, and only the turn's cost
moves on, being a flow rather than a gauge.
The rule that a call failed in transport gives no window reading was
pinned on a non-streaming turn only, a pairing production never makes:
the RPC runner, the one caller with a usage sink, always streams. The
streaming exit returns such a call with the failure and classification
the other exit gives it, and this drives that path the way the runner
does: a streamed turn whose call after a tool call the upstream failed
keeps the reading of the call before it, 6200 on the sink and on a
resume, where the failed call's bill read 2.
The loop rewrote the usage sink on every call that reported usage,
while the reading a failed turn files moves only on a reading above
zero. A last call that reported a block of zeros emptied the sink, so
the error carried no usage and the ring stayed on the turn before,
while the failure marker and a resume read 6200. The sink now keeps
the reading it holds through a call that read no window, a block of
zeros or a transport failure alike, and only the turn's cost moves on;
a sink that holds no reading yet is still filled whole, since an
answered turn's message.complete sends it as it is.

The RpcTurnRunner and _stopped_usage docstrings now say why the usage
a failed or stopped turn reports matches the reading it files.
An empty reply's reading now goes on the newest row the turn keeps,
which on a turn whose first reply only thought is its question. The
fallback test compared the question it asked again with the first
request dict for dict, and failed on that bookkeeping key, which no
provider sends: each sends only its allowed message keys. It now
compares the two requests as they are sent, and still fails when the
switch leaves the prefill on.
@LivXue
LivXue force-pushed the fix/context_ring_failed_and_empty_turns branch from 07342fe to fd9a155 Compare October 31, 2026 08:48

@gloryfromca gloryfromca left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No blockers; this can merge as far as I am concerned.

Reviewed the rebase onto current main. The range-diff leaves every substantive PR commit patch-equivalent to the previously clean revision; the sole conflict resolution keeps main's new Callable import beside this PR's Any import in raven/cli/tui_commands.py. Main's new find_node(accept=...) path and this PR's RPC live_usages wiring are independent and both remain intact.

For this revision I rechecked the repository/context rules, the complete github/main...HEAD diff, callers and history through the range-diff, backward compatibility, test strength, and the RPC-local architecture constraint. I found no new defect. git merge-tree --write-tree github/main HEAD and git diff --check github/main...HEAD passed. The focused backend suite passed: 1052 tests via uv run --python 3.12 --all-extras pytest -q -n 0 tests/test_agent_loop_stream.py tests/test_agent_loop_usage_sink.py tests/test_transport_failure_paths.py tests/test_rpc_spine.py tests/test_rpc_turn_cancel.py tests/test_agent_loop_empty_recovery.py tests/test_rpc_session.py tests/test_rpc_bootstrap.py tests/test_cli_tui_commands.py.

@LivXue

LivXue commented Oct 31, 2026

Copy link
Copy Markdown
Member Author

Windows compatibility re-review — PR #1126 @ fd9a15576c36643e90d8909398f94431baac6d2e

Scope: the branch is rebased onto current main; the PR's own diff at this head is 9d9bec90..fd9a15576 (31 files, +1114/−40). range-diff against the previously reviewed head 07342fe805 shows all ten commits identical except commit 5, whose only change is the typing import line main itself edited between the two bases (Callable, Optional, Tuple → Optional, Tuple); the semantic content is unchanged, so the earlier findings carry.

Result: no Windows-specific defect. The change is usage/context-reading plumbing through the loop, the RPC methods/spine, and the ui-tui/ui-web display; it adds no path, subprocess, signal, socket, file, encoding, or permission surface. Re-ran the affected suites on Windows at the rebased head.

Verification (native Windows 11; detached worktrees D:\tmp\rv-1126 at head and D:\tmp\rv-base-1126 at base 9d9bec90; offline venv from the same lock; scratch HOME/USERPROFILE; PYTHONUTF8=1; PYTHONPATH pinned):

  • uv run --no-sync pytest tests/test_agent_loop_empty_recovery.py tests/test_agent_loop_usage_sink.py tests/test_cli_tui_commands.py tests/test_rpc_bootstrap.py tests/test_rpc_session.py tests/test_rpc_spine.py tests/test_rpc_turn_cancel.py -q -n 0
    • head: 1 failed, 924 passed (119.8 s)
    • base: 1 failed, 890 passed (110.0 s)
    • the same single failing ID at both — test_cli_tui_commands.py::test_python_dash_m_finds_the_script_beside_the_interpreter (KeyError: 'RAVEN_BIN') — an inherited Windows venv-layout case, not introduced here; the 34 added tests pass.
  • ui-tui at head (node_modules junctioned from the main checkout, junction removed afterwards): npx --no-install vitest run src/__tests__/chatStream.test.ts src/__tests__/createGatewayEventHandler.test.ts → Test Files 2 passed (2), Tests 100 passed (100).
  • ui-web vitest could not run: D:/projects/Raven/ui-web/node_modules contains only .vite-temp.

Coverage gaps: the full suite was not rerun; ui-web's ctxReport/pipeline.events tests were run at the previous head but not at this one; no live RPC/UI session was driven.

No Windows-specific issue found at this head.

This branch has not been deployed

No deployments
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.

3 participants