Skip to content

feat(*): align empty-reply recovery with Hermes and let a turn end in silence - #1092

Merged
0xKT merged 139 commits into
mainfrom
feat/align_empty_reply_with_hermes
Oct 28, 2026
Merged

0xKT merged 139 commits into
mainfrom
feat/align_empty_reply_with_hermes

Conversation

@LivXue

@LivXue LivXue commented Oct 24, 2026 •

Copy link
Copy Markdown
Member

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
  • 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, fix(*): report a delegated turn's real outcome and cost #1121's budget-stop marker, is described in
    the summary. Earlier rebases met feat(context_engine): compacting history strategy with source refs and checkpoints #1031 (compacting history), fix(*): say whether the lane is still busy when a cancel is refused #1078
    (refused cancel), fix(*): restore the context ring from the window the last call measured #1119 (the context ring a resume restores), fix(agent): a sub-agent's blank reply is no answer, not the answer #1047
    and feat(ui-web): add a turn nav that marks each turn beside the transcript #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.

  • Relevant tests pass locally

  • Relevant lint / type checks pass locally

  • 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 fix(agent): a sub-agent's blank reply is no answer, not the answer #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:

  • feat(context_engine): compacting history strategy with source refs and checkpoints #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.

  • Security impact considered

  • Backward compatibility considered

  • Rollback path is clear for risky changes

Related Issues

N/A. Supersedes #1052 (closed unmerged).

@LivXue
LivXue requested review from 0xKT and gloryfromca October 24, 2026 04: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.

Blocking: session.list must preserve activity from a proactive turn that already delivered through the message tool.

I found one merge-blocking cross-surface mismatch, marked inline. I tried to refute it against the intended silence semantics: an ordinary runtime-origin Silent Turn should disappear from activity/replay, but this revision explicitly treats a turn answered by the message tool as answered, and ACP preserves that exact case. The session scanner does not, and a direct stored-row reproduction returns the previous turn's timestamp.

Coverage: I reviewed the repository rules and canonical Runtime/TUI/Web boundaries, the full diff, core recovery and provider paths, silence propagation through gateway/RPC/session/ACP/cron, both UI consumers, relevant callers, commit history, backward-compatibility seams, and test changes for weakening.

Verification: the focused Python slice passed (834 passed); web passed (4,281 tests, type-check, generated RPC check); TUI passed (2,266 passed, 13 skipped, type-check, RPC/i18n generation checks); commit-message, source-language, and large-file checks passed. The full Python run finished with 34,625 passed, 187 skipped, 1 xfailed, 15 failed, and 2 collection errors: fourteen failures/errors require unavailable optional datrie/nltk dependencies (datrie could not build because this image has no gcc), and one failure is an untouched CLI-theme assertion.

Comment thread raven/session/manager.py Resolved
@LivXue
LivXue force-pushed the feat/align_empty_reply_with_hermes branch from 7269126 to f4fb262 Compare October 24, 2026 10:45

@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.

Re-reviewed the current head after its rebase and the blocker fix. The range-diff shows the prior 97-patch series is unchanged; the only new patch centralizes message-tool turn detection and makes the session scanner preserve delivered runtime activity, matching ACP replay. The regression exercises the concrete stale-session failure, and the helper tests cover both stored provider calls and flattened wire calls. No tests were weakened.

Covered the repository rules and context terminology, the current diff, affected session-list and replay callers, patch history across the rebase, compatibility with both transcript shapes, and the shared-rule architecture. Verification: 249 focused session/replay tests passed; git diff --check and the source-language, large-file, and commit-message gates also passed. The prior thread is resolved.

@LivXue

LivXue commented Oct 24, 2026

Copy link
Copy Markdown
Member Author

Windows compatibility review at head 72691268c5a6ad47397163a900fb0d37be75a845, base/merge base 538a19b307a661a0f16ed16ec5e6b45fd8779a3a.

What was reviewed. All 115 changed files were tiered by Windows risk and the whole diff was read. Tier 1 (read fully and verified carefully): 20 files - the session and export readers, raven/config/schema.py, raven/core/cron_stack.py, raven/acp/replay.py, raven/gateway/spine.py, raven/spine/scheduler.py, raven/spine/silence.py, raven/agent/window/compaction.py, the console and session RPC methods, tests/conftest.py, and the 9 changed test files that touch temp dirs, session files or paths. Tier 2 (diff read, surrounding code skimmed): 82 files - the agent loop, the providers, the channel adapters, the RPC layer and the UI state. Tier 3 (scan only): 13 files - 7 docs, 4 generated files (rpc-schema/openrpc.json, both rpc/generated.ts, ui-tui/src/i18n/messages.generated.ts) and 2 data fixtures. Every changed line was also scanned mechanically for filesystem, path, subprocess, signal, config/env, encoding and Windows-only API usage.

Verdict: this PR introduces no Windows compatibility regression. The changed lines contain no new filesystem, subprocess, signal, path-join, encoding or Windows-only API calls at all. The only new persistent artifact is a JSON key (silent) in the existing transcript format, and the new end_turn channel hook and the gateway teardown changes use asyncio task APIs only. Every failure I could produce on Windows also reproduces at the base commit, so the failures below are inherited and are not filed as regressions.

Inherited issue (present at the base commit too, not introduced here). raven/acp/tool_kinds.py:173 resolves tool-call locations with Path(value).is_absolute(). On Windows a rooted but drive-less path such as /work is not absolute, so absolute_path() returns None at line 176, locations() returns [], and raven/acp/replay.py:222 never sets update["locations"]. The two assertions in tests/test_acp_replay.py:159 and tests/test_acp_replay.py:230 therefore fail with KeyError: 'locations'. This file is unchanged by the PR and the same two failures reproduce at the base commit (tests/test_acp_replay.py on its own: 2 failed, 50 passed at the head and 2 failed at the base), so it is inherited rather than a regression. Suggested fix, in a separate PR: treat a drive-less rooted path as absolute on Windows (or fall back to Path.cwd()), so absolute_path("a.py", Path("/work")) yields a usable absolute path instead of None.

Inherited coverage note. The windows-acp job in .github/workflows/ci.yml runs a fixed list of test files and does not include tests/test_acp_replay.py, which is why the inherited failure above has never been caught on windows-latest. No CI file is changed by this PR.

Native Windows verification (Python 3.13.12, UTF-8; Node 25.9.0, npm 11.12.1; Windows 11 10.0.26300 x64):

  • The five affected suites tests/test_spine_silence.py, tests/test_acp_replay.py, tests/test_rpc_session.py, tests/test_session_export.py and tests/test_config_schema.py at this head: 317 passed, 2 failed, and the failures are the inherited KeyError: 'locations' described above. The four of those files that exist at the base commit: 260 passed, 2 failed - the same two.
  • A second batch of 33 changed test files (the empty-recovery suite, the loop and stream suites, cron, the gateway spine, the transport-failure paths, the spine and provider suites, the channel and RPC suites) at this head: 2039 passed, 28 failed, 1 skipped (542.58s). The identical 33 files at the base commit: 1668 passed, 28 failed, 1 skipped (391.21s), with the same 28 failing test ids on both sides, so the PR adds no new failure. The set is the Windows-only POSIX-path and Unix-socket cases in test_subagent_manager.py (21, including test_a_symlink_out_of_the_roots_refuses_the_spawn, which needs Developer Mode), test_rpc_console.py (4: CRLF config bytes, two symlink cases, the state-dir path comparison), test_ppt_engine_plugin.py (2) and test_rpc_contract_shapes.py::test_session_usage (1).
  • The touched ui-web suites: 4 files, 297 passed (63.02s, exit 0), and npm run type-check on ui-web: exit 0.
  • The touched ui-tui suites: 115 tests, 108 passed, 7 failed, all 7 in src/__tests__/messages.test.ts ("transcript reply gutter" glyph cases). The base checkout's messages.test.ts fails with the same 7 test names, so they are inherited and not introduced here.
  • node scripts/gen-i18n.mjs --check in ui-tui: "i18n: generated catalogue is up to date" (exit 0).
  • Probe on Windows path semantics: Path("/work").is_absolute() is False while PurePosixPath("/work").is_absolute() is True; locations({"path": "a.py"}, "/work") is [] while locations({"path": "a.py"}, "C:/work") is [{"path": "C:\\work\\a.py"}].
  • Probe on the PR's own behaviour, over a real CRLF-written transcript on disk: the session preview and timestamp roll back to the answer before the silent turn (preview == "Two deals closed.", last_ts == "2026-10-24T09:00:05Z"), and the ACP replay drops the silent runtime turn with no [SILENT] marker sent as an answer.
Exact verification commands and results
$root = 'C:\Users\livxu\AppData\Local\Temp\raven-windows-review\run-20261024'
$env:UV_PROJECT_ENVIRONMENT = "$root\venv"
$env:PYTHONUTF8 = '1'; $env:PYTHONDONTWRITEBYTECODE = '1'

# batch 1 at the head; exit 1
$head = "$root\pr1092\head"
$env:PYTHONPATH = $head
uv run --frozen --no-sync --project $head pytest -c "$head\pyproject.toml" --rootdir $head `
  "$head\tests\test_spine_silence.py" "$head\tests\test_acp_replay.py" `
  "$head\tests\test_rpc_session.py" "$head\tests\test_session_export.py" "$head\tests\test_config_schema.py" `
  -p no:randomly -p no:cacheprovider -n 0 --tb=no -q --basetemp "$root\scratch\pr1092\head-batch1"
# 317 passed, 2 failed - both test_acp_replay.py TestToolCalls cases, KeyError: 'locations'

# batch 1 at the base commit (test_spine_silence.py does not exist there); exit 1
$base = "$root\pr1092\base"
$env:PYTHONPATH = $base
uv run --frozen --no-sync --project $base pytest -c "$base\pyproject.toml" --rootdir $base `
  "$base\tests\test_acp_replay.py" "$base\tests\test_rpc_session.py" `
  "$base\tests\test_session_export.py" "$base\tests\test_config_schema.py" `
  -p no:randomly -p no:cacheprovider -n 0 --tb=no -q --basetemp "$root\scratch\pr1092\base-batch1"
# 2 failed, 260 passed in 32.40s - the same two

# test_acp_replay.py alone, both sides; exit 1 on each
uv run --frozen --no-sync --project $head pytest -c "$head\pyproject.toml" --rootdir $head `
  "$head\tests\test_acp_replay.py" -p no:randomly -p no:cacheprovider -n 0 --tb=no -q --basetemp "$root\scratch\pr1092\head-replay"
# 2 failed, 50 passed in 12.92s
# batch 2: 33 changed test files, head then base; exit 1 on each
# (the file list is the 33 changed test files of this PR)
$env:PYTHONPATH = $head
uv run --frozen --no-sync --project $head pytest -c "$head\pyproject.toml" --rootdir $head <33 files> `
  -p no:randomly -p no:cacheprovider -n 0 --tb=no -q --basetemp "$root\scratch\pr1092\head-batch2"
# 28 failed, 2039 passed, 1 skipped, 26 warnings in 542.58s (0:09:02)

$env:PYTHONPATH = $base
uv run --frozen --no-sync --project $base pytest -c "$base\pyproject.toml" --rootdir $base <33 files> `
  -p no:randomly -p no:cacheprovider -n 0 --tb=no -q --basetemp "$root\scratch\pr1092\base-batch2"
# 28 failed, 1668 passed, 1 skipped, 26 warnings in 391.21s (0:06:31)
# the two sets of 28 failing test ids are identical
# the touched ui-web suites and the type check; exit 0
npm.cmd ci --prefix "$root\pr1092\head\ui-web" --no-audit --no-fund
npm.cmd test --prefix "$root\pr1092\head\ui-web" -- src/features/transcript/TranscriptPage.test.tsx src/state/session/pipeline.events.test.ts src/state/session/pipeline.notify.test.ts src/state/session/runtime.cancel.test.ts --no-file-parallelism
# Test Files  4 passed (4); Tests  297 passed (297)
npm.cmd run type-check --prefix "$root\pr1092\head\ui-web"
# exit 0
# the touched ui-tui suites and the generated-catalog check; exit 1 (the 7 inherited), then exit 0
npm.cmd ci --prefix "$root\pr1092\head\ui-tui" --no-audit --no-fund
npm.cmd ci --prefix "$root\pr1092\head\ui-tui\packages\hermes-ink" --no-audit --no-fund
npm.cmd test --prefix "$root\pr1092\head\ui-tui" -- src/__tests__/createGatewayEventHandler.test.ts src/__tests__/messages.test.ts src/__tests__/turn-subscribe.test.ts
# Test Files  1 failed | 2 passed (3); Tests  7 failed | 108 passed (115)
node "$root\pr1092\head\ui-tui\scripts\gen-i18n.mjs" --check
# i18n: generated catalogue is up to date   (exit 0)

# the base checkout's messages.test.ts fails with the same 7 names
npm.cmd test --prefix "$root\pr1092\base\ui-tui" -- src/__tests__/messages.test.ts
# Test Files  1 failed (1); Tests  7 failed | 51 passed (58)
node --version   # v25.9.0
uv --version     # uv 0.13.0 (4ecd0eff2 2026-10-09 x86_64-pc-windows-msvc)

Coverage limits: no live model call was made, no Windows ACP stdio or process-tree scenario was exercised, and the full test suite was not run. The raven/acp_client/windows.py Job-Object launcher was not exercised, because this PR does not touch it. This is a scoped compatibility report, not a merge verdict.

@LivXue

LivXue commented Oct 24, 2026

Copy link
Copy Markdown
Member Author

Windows compatibility re-review at head f4fb262082fac2c82645af9f26c6110dadcac86a, base/merge base e8332e5d4c9690215e6962899064d07648d9badf.

The previous review was at head 72691268c5a6ad47397163a900fb0d37be75a845. This one covers only the delta since then: CONTEXT.md, raven/acp/replay.py, raven/session/manager.py, tests/test_rpc_session.py, and the two new files raven/session/turn_shape.py and tests/test_session_turn_shape.py. Verdict: the delta holds up on Windows, and no Windows regression is introduced by it. The new rule is pure in-memory logic over already-parsed transcript rows, adds no filesystem, path, subprocess, signal or env use, and the rows it reads are identical whether the transcript was written LF or CRLF.

Inherited issues only. These are already present at base e8332e5d4c and at the previously reviewed head 72691268c5a6ad47397163a900fb0d37be75a845, so this delta does not cause them.

  1. Non-blocking, inherited - a transcript saved with a UTF-8 BOM makes the session vanish from session.list. raven/session/manager.py:770 reads with encoding="utf-8", so the first line fails json.loads and is skipped; the metadata record is that line, so raven/session/manager.py:1176 then drops the whole session. A Windows tool that saves UTF-8 with a BOM makes the session invisible. Suggested fix: read with encoding="utf-8-sig", which accepts a BOM and a plain UTF-8 file alike.

  2. Non-blocking, inherited - a transcript saved in a Windows ANSI code page (cp936 on a Chinese Windows, cp1252 elsewhere) crashes session.list with UnicodeDecodeError. raven/session/manager.py:770 decodes strictly as UTF-8, and UnicodeDecodeError is a ValueError, so the except OSError at raven/session/manager.py:804 does not catch it and the error escapes the session list. Suggested fix: catch UnicodeDecodeError alongside OSError, or read with errors="replace".

  3. Non-blocking, inherited, test only - tests/test_acp_replay.py:159 and tests/test_acp_replay.py:230 hardcode cwd="/work". On Windows Path("/work").is_absolute() is False, so absolute_path at raven/acp/tool_kinds.py:175 correctly refuses the base and the locations key is dropped, giving KeyError: 'locations'. The production behaviour is right; the test literal is POSIX-only. Suggested fix: use a platform-neutral base such as str(tmp_path) and derive the expectation from the same Path.

Native Windows verification (Python 3.13.12, UTF-8): at head f4fb262082fac2c82645af9f26c6110dadcac86a, tests/test_session_turn_shape.py passed 7 of 7 and tests/test_rpc_session.py passed 190 of 190 (exit 0); tests/test_acp_replay.py passed 50 and failed 2 (exit 1), both failures being the inherited POSIX cwd cases above, which reproduce identically at base e8332e5d4c (2 failed, 231 passed) and at 72691268c5a6ad47397163a900fb0d37be75a845 (2 failed, 239 passed). The delta's own new cases pass 8 of 8. A probe confirmed that atomic_replace writes CRLF on Windows while locked_append writes LF, that LF and CRLF transcripts parse to identical rows and produce identical _scan_file results, that the new message-tool rule fires on both the stored provider shape and the wire shape, and that a non-ASCII transcript round-trips intact. The same probe reproduced the two inherited failures at base, at the previously reviewed head and at head.

Exact verification commands and results
$env:UV_PROJECT_ENVIRONMENT = 'C:\Users\livxu\AppData\Local\Temp\raven-windows-review\run-20261024\venv'
$env:PYTHONPATH = 'C:\Users\livxu\AppData\Local\Temp\raven-windows-review\run-20261024\pr1092\new\head'
$env:PYTHONUTF8 = '1'; $env:PYTHONDONTWRITEBYTECODE = '1'
uv run --frozen --no-sync --project 'C:\Users\livxu\AppData\Local\Temp\raven-windows-review\run-20261024\pr1092\new\head' pytest -c 'C:\Users\livxu\AppData\Local\Temp\raven-windows-review\run-20261024\pr1092\new\head\pyproject.toml' --rootdir 'C:\Users\livxu\AppData\Local\Temp\raven-windows-review\run-20261024\pr1092\new\head' 'C:\Users\livxu\AppData\Local\Temp\raven-windows-review\run-20261024\pr1092\new\head\tests\test_session_turn_shape.py' 'C:\Users\livxu\AppData\Local\Temp\raven-windows-review\run-20261024\pr1092\new\head\tests\test_rpc_session.py' 'C:\Users\livxu\AppData\Local\Temp\raven-windows-review\run-20261024\pr1092\new\head\tests\test_acp_replay.py' -p no:randomly -p no:cacheprovider -n 0 --tb=short -q --basetemp 'C:\Users\livxu\AppData\Local\Temp\raven-windows-review\run-20261024\scratch\pr1092delta\pytest'

exit code 1 - 2 failed, 247 passed (both failures inherited, item 3 above)

$env:UV_PROJECT_ENVIRONMENT = 'C:\Users\livxu\AppData\Local\Temp\raven-windows-review\run-20261024\venv'
$env:PYTHONPATH = 'C:\Users\livxu\AppData\Local\Temp\raven-windows-review\run-20261024\pr1092\new\head'
$env:PYTHONUTF8 = '1'; $env:PYTHONDONTWRITEBYTECODE = '1'
uv run --frozen --no-sync --project 'C:\Users\livxu\AppData\Local\Temp\raven-windows-review\run-20261024\pr1092\new\head' pytest -c 'C:\Users\livxu\AppData\Local\Temp\raven-windows-review\run-20261024\pr1092\new\head\pyproject.toml' --rootdir 'C:\Users\livxu\AppData\Local\Temp\raven-windows-review\run-20261024\pr1092\new\head' 'C:\Users\livxu\AppData\Local\Temp\raven-windows-review\run-20261024\pr1092\new\head\tests\test_session_turn_shape.py' -p no:randomly -p no:cacheprovider -n 0 --tb=short -q --basetemp 'C:\Users\livxu\AppData\Local\Temp\raven-windows-review\run-20261024\scratch\pr1092delta\pytest-newtests'

exit code 0 - 7 passed (new file) and, together with the new test_rpc_session.py case, 8 passed

$env:UV_PROJECT_ENVIRONMENT = 'C:\Users\livxu\AppData\Local\Temp\raven-windows-review\run-20261024\venv'
$env:PYTHONPATH = 'C:\Users\livxu\AppData\Local\Temp\raven-windows-review\run-20261024\pr1092\new\base'
$env:PYTHONUTF8 = '1'; $env:PYTHONDONTWRITEBYTECODE = '1'
uv run --frozen --no-sync --project 'C:\Users\livxu\AppData\Local\Temp\raven-windows-review\run-20261024\pr1092\new\base' pytest -c 'C:\Users\livxu\AppData\Local\Temp\raven-windows-review\run-20261024\pr1092\new\base\pyproject.toml' --rootdir 'C:\Users\livxu\AppData\Local\Temp\raven-windows-review\run-20261024\pr1092\new\base' 'C:\Users\livxu\AppData\Local\Temp\raven-windows-review\run-20261024\pr1092\new\base\tests\test_rpc_session.py' 'C:\Users\livxu\AppData\Local\Temp\raven-windows-review\run-20261024\pr1092\new\base\tests\test_acp_replay.py' -p no:randomly -p no:cacheprovider -n 0 --tb=short -q --basetemp 'C:\Users\livxu\AppData\Local\Temp\raven-windows-review\run-20261024\scratch\pr1092delta\pytest-base'

exit code 1 - 2 failed, 231 passed (base, same two failures: inherited)

git -C 'C:\Users\livxu\AppData\Local\Temp\raven-windows-review\run-20261024\pr1092\new\head' archive --format=zip 72691268c5a6ad47397163a900fb0d37be75a845 -o 'C:\Users\livxu\AppData\Local\Temp\raven-windows-review\run-20261024\scratch\pr1092delta\prev72691268.zip'
Expand-Archive -Path 'C:\Users\livxu\AppData\Local\Temp\raven-windows-review\run-20261024\scratch\pr1092delta\prev72691268.zip' -DestinationPath 'C:\Users\livxu\AppData\Local\Temp\raven-windows-review\run-20261024\scratch\pr1092delta\prev72691268' -Force

exit code 0 - previously reviewed head extracted into the scratch dir, unmodified repo

$env:UV_PROJECT_ENVIRONMENT = 'C:\Users\livxu\AppData\Local\Temp\raven-windows-review\run-20261024\venv'
$env:PYTHONPATH = 'C:\Users\livxu\AppData\Local\Temp\raven-windows-review\run-20261024\scratch\pr1092delta\prev72691268'
$env:PYTHONUTF8 = '1'; $env:PYTHONDONTWRITEBYTECODE = '1'
uv run --frozen --no-sync --project 'C:\Users\livxu\AppData\Local\Temp\raven-windows-review\run-20261024\scratch\pr1092delta\prev72691268' pytest -c 'C:\Users\livxu\AppData\Local\Temp\raven-windows-review\run-20261024\scratch\pr1092delta\prev72691268\pyproject.toml' --rootdir 'C:\Users\livxu\AppData\Local\Temp\raven-windows-review\run-20261024\scratch\pr1092delta\prev72691268' 'C:\Users\livxu\AppData\Local\Temp\raven-windows-review\run-20261024\scratch\pr1092delta\prev72691268\tests\test_rpc_session.py' 'C:\Users\livxu\AppData\Local\Temp\raven-windows-review\run-20261024\scratch\pr1092delta\prev72691268\tests\test_acp_replay.py' -p no:randomly -p no:cacheprovider -n 0 --tb=short -q --basetemp 'C:\Users\livxu\AppData\Local\Temp\raven-windows-review\run-20261024\scratch\pr1092delta\pytest-prev'

exit code 1 - 2 failed, 239 passed (previously reviewed head, same two failures: inherited)

$env:UV_PROJECT_ENVIRONMENT = 'C:\Users\livxu\AppData\Local\Temp\raven-windows-review\run-20261024\venv'
$env:PYTHONUTF8 = '1'; $env:PYTHONDONTWRITEBYTECODE = '1'
uv run --frozen --no-sync --project '<checkout>' python 'C:\Users\livxu\AppData\Local\Temp\raven-windows-review\run-20261024\scratch\pr1092delta\probe_bom.py' 'C:\Users\livxu\AppData\Local\Temp\raven-windows-review\run-20261024\scratch\pr1092delta\bom-<base|prev|head>'
uv run --frozen --no-sync --project '<checkout>' python 'C:\Users\livxu\AppData\Local\Temp\raven-windows-review\run-20261024\scratch\pr1092delta\probe_codepage.py' 'C:\Users\livxu\AppData\Local\Temp\raven-windows-review\run-20261024\scratch\pr1092delta\cp-<base|prev|head>'

exit code 0 for all three checkouts - "BOM hides the session: True" at base, at the previously reviewed head and at head; a cp936 transcript raises UnicodeDecodeError out of list_sessions at all three

Scope covered: the delta only, on native Windows, for the session transcript read path, the ACP replay path, the new raven/session/turn_shape.py module and its tests. The JS/TS surfaces and the rest of the PR are out of scope. This is a scoped compatibility report, not a merge verdict.

@LivXue
LivXue force-pushed the feat/align_empty_reply_with_hermes branch 2 times, most recently from 154f8ea to 67357ab Compare October 24, 2026 11:35

@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 revision delta through the live head 67357abfffc3. The rewritten fix retains the previously verified message-tool exception, adds a meaningful cross-turn regression for resetting answered at each user row, and the final commit only formats that test. The call-based behavior remains shared with ACP replay and compatible with both stored and flattened tool-call shapes; no tests were weakened.

Covered the applicable repository rules and context terminology, the delta and affected callers, history across the rewritten commit and formatting tip, backward compatibility, and the shared-rule architecture. Verification at the live head: 389 focused session/replay tests passed; Ruff formatting, git diff --check, source-language, large-file, and commit-message gates passed. My thread is resolved.

@LivXue

LivXue commented Oct 24, 2026

Copy link
Copy Markdown
Member Author

Windows compatibility re-review at head 67357abfffc3940663a66ca4f217a9dcfdc06936, base/merge base e8332e5d4c9690215e6962899064d07648d9badf.

The previous review was at head f4fb262082fac2c82645af9f26c6110dadcac86a. This one covers the delta since then, which is small: one new import and the rewritten read rule in raven/session/manager.py, and a new test file tests/test_session_manager.py. Verdict: the delta holds up on Windows, and no Windows regression is introduced by it.

The production change adds no Windows surface. It imports calls_message_tool from raven/session/turn_shape.py (the module the earlier review already cleared as pure in-memory dict logic) and uses it to decide whether a turn that ends with a silence marker still counts as the activity a session is ordered by. The transcript is still read with the same path.open(encoding="utf-8") the earlier review covered, line by line, with no new decoding, no new path construction and no new I/O. The two inherited findings of the earlier review (a UTF-8 BOM hides a session from session.list, and a transcript in a Windows ANSI code page raises UnicodeDecodeError out of it) are unchanged by this delta: the reader it writes is the same reader those findings are about.

The new test file is written for Windows rather than against it. That is the part worth recording, because a test file for the session manager is exactly where a POSIX-only assumption would land. It takes the multiprocessing start method explicitly - multiprocessing.get_context("spawn") at line 476 - and says why in the comment above it: spawn is the Windows default, the Linux default is fork, and pytest leaves the parent multi-threaded, which fork would then carry into the child. The worker it spawns, _turn_worker, is a module-level function taking plain arguments, so it pickles under the spawn method (a closure or a lambda would not). The file also carries a case built for the Windows clock: test_a_saved_time_windows_cannot_move_to_local_time_leaves_the_session_loadable substitutes a datetime whose astimezone raises OSError(22, "Invalid argument"), which is what Windows does for a local time before 1970 where Linux converts it, and asserts the session still loads.

Native Windows verification (Python 3.13.12, UTF-8; Windows 11 10.0.26300 x64): the new tests/test_session_manager.py at this head: 140 passed, 0 failed, 0 skipped (14.29s, exit 0). The spawn-based two-process case runs and passes here, which is the part that would have failed on a fork-only assumption. No other test file changed in this delta, so no other suite was re-run; the earlier review's numbers for tests/test_session_turn_shape.py, tests/test_rpc_session.py and tests/test_acp_replay.py still stand for the code they cover, which this delta does not touch.

Coverage limits: the delta was exercised through the new test file and the reading path it covers; the full session suite, the ACP launcher and the Node side were not run, and the inherited BOM and code-page findings were not re-measured in this round because the reader they concern is unchanged. This is a scoped compatibility report, not a merge verdict.

Exact verification commands and results
$root = 'C:\Users\livxu\AppData\Local\Temp\raven-windows-review\run-20261024'
$ck = "$root\pr1092\new2\head"
$env:UV_PROJECT_ENVIRONMENT = "$root\venv"
$env:PYTHONPATH = $ck
$env:PYTHONUTF8 = '1'; $env:PYTHONDONTWRITEBYTECODE = '1'

# the new test file at the current head; exit 0
uv run --frozen --no-sync --project $ck pytest -c "$ck\pyproject.toml" --rootdir $ck `
  "$ck\tests\test_session_manager.py" -p no:randomly -p no:cacheprovider -n 0 --tb=short -q `
  --basetemp "$root\scratch\pr1092\sessmgr"
# 140 passed in 14.29s   (exit 0)

# the delta itself
git -C "$root\repo" diff --no-color "e8332e5d4c..67357abfffc3940663a66ca4f217a9dcfdc06936" -- raven/session/manager.py
# the spawn context the new test takes
Select-String -Path "$ck\tests\test_session_manager.py" -Pattern 'multiprocessing|get_context'

@0xKT

0xKT commented Oct 26, 2026

Copy link
Copy Markdown
Member

Merge conflict: @LivXue this PR no longer merges cleanly into main.

Measured just now with git merge-tree --write-tree of main dacfc64c and this head 67357abf. Main moved with the merge batch #1098, #1095, #1084, #1087, #1078, #1081, #1100 and #1031. Conflicting files: raven/agent/loop/turn_path.py, raven/agent/window/compaction.py, ui-web/src/state/session/runtime.ts, ui-web/src/state/session/stages.ts.
The first two overlap #1031 (compacting history strategy), the page files overlap #1078 (the busy flag on a refused cancel).
After a rebase onto current main I re-check what moved before the grade comes back.

@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: rebase onto current main and resolve the four merge conflicts before merge.

I reproduced the report with git merge-tree --write-tree using current main dacfc64c and this head 67357abf. It conflicts in raven/agent/loop/turn_path.py, raven/agent/window/compaction.py, ui-web/src/state/session/runtime.ts, and ui-web/src/state/session/stages.ts. This meets the late-round blocker bar: this branch's overlapping changes produce the conflict, it is reached by the ordinary merge path, and there is no working merge path until the branch is refreshed and the conflicts are resolved.

The unchanged branch itself still passes the focused session/replay suite (389 passed), but that cannot validate the unresolved integration with current main.

@LivXue
LivXue force-pushed the feat/align_empty_reply_with_hermes branch from 67357ab to a71f068 Compare October 27, 2026 03:24
@LivXue

LivXue commented Oct 27, 2026

Copy link
Copy Markdown
Member Author

@gloryfromca @0xKT Rebased onto main 471842132 and pushed as a71f06880. What changed since 67357abf, for the re-review:

git range-diff e8332e5d..67357abf 471842132..a71f0688 over the first 99 commits: 86 = and 13 !. Ten of the ! differ only in context lines; the zero-context diff differs only at the recorded resolutions.

@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 rebase resolves the four conflicts against main at 471842132; git merge-tree --write-tree now succeeds. I reviewed the range-diff and the substantive resolutions: continuation requests are capped to their remaining context room without overriding hook-owned ceilings, and a held cancel now drops its stale silent mark with the late end so the following turn is not misclassified. The added sub-agent and turn-navigation controls preserve the merged behavior. I found no concrete regression, and the previous merge-conflict blocker is cleared.

Covered the repository rules and context terminology, the current diff and conflict-resolution history, affected callers, compatibility with the merged main behaviors, test strength, and the shared runtime/UI architecture. Verification: 1,011 focused Python tests passed; the targeted web rerun passed 96/96 after one load-related timeout in the initial parallel run; the complete web suite passed 4,425/4,425; web type-check, generated-client check, diff check, source-language, large-file, and commit-message gates passed. My review thread remains resolved.

@LivXue

LivXue commented Oct 27, 2026

Copy link
Copy Markdown
Member Author

Windows compatibility review — PR #1092 @ a71f06880b87de1f7388aee54a28ea92b0b16771

Scope: full base 4718421324 → this head (90 files, +8700/−1687): the empty-reply recovery ladder, silent-turn ending, session refs, transcript origin, spine/scheduler changes, the provider streaming changes, RPC schema/clients, and the tests. This pass re-reads the PR's own delta since the previously Windows-reviewed head 67357abf (42 files, of which 17 under raven/), and runs the platform-sensitive suites at head and base.

Result: no Windows-specific code added and no new Windows test failures.

What I checked:

  • A strict scan of the production delta (raven/**) for subprocess creation, Popen, signals, os.kill, sockets, fcntl/termios/pty, chmod/symlinks, temp files, Path(/os.path, open(, explicit encodings and environ found none — the matches were all prose ("empty", "open" in docstrings). The changes are turn/recovery/verdict logic, session-record and transcript bookkeeping, and provider stream handling.
  • The previously reported inherited findings for this PR (a UTF-8 BOM hides a session from session.list; a transcript in a Windows ANSI code page raises from the reader) are unchanged: the transcript reader in raven/session/manager.py is the same open(encoding="utf-8") code, and this delta does not touch its decoding.
  • tests/test_spine_silence.py is new in this PR and passes on Windows.

Verification (native Windows 11, CPython 3.12.13, isolated worktrees D:\tmp\rv-1092 and D:\tmp\rv-1092-base, scratch HOME/USERPROFILE, PYTHONUTF8=1):

pytest tests/test_subagent_manager.py tests/test_session_manager.py \
       tests/test_rpc_session.py tests/test_spine_scheduler.py \
       tests/test_spine_silence.py tests/test_transport_failure_paths.py \
       tests/test_rpc_console.py -q
→ head: 21 failed, 957 passed, 1 skipped in 31.00s

same set minus the new test_spine_silence.py at base 4718421324
→ base: 21 failed, 881 passed, 1 skipped in 28.72s

The failing-ID sets are identical (diffed as sets — no additions, no removals): the 20 test_subagent_manager.py spawn/path/symlink cases and 4 test_rpc_console.py path/ownership cases are inherited from main, not this PR. The 76 extra passing tests at head are this PR's.

Limitations: the UI/TUI portions of the diff were not run (generated clients only read); the full 90-file suite was not run; no live provider or gateway was exercised. Only the six platform-sensitive suites above were measured.

@0xKT

0xKT commented Oct 27, 2026

Copy link
Copy Markdown
Member

Merge conflict: @LivXue this PR no longer merges cleanly into main.

Measured just now with git merge-tree --write-tree of main 26352da0 and this head a71f0688. Main moved with #1119 (restore the context ring from the window the last call measured), which changed raven/agent/loop/turn_path.py. Conflicting files: raven/agent/loop/turn_path.py.

After a rebase onto current main I re-check what moved before the grade comes back.

@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: rebase onto current main and resolve the turn_path.py conflict before merge.

I reproduced the report with git merge-tree --write-tree using current main 26352da0 and this head a71f0688; raven/agent/loop/turn_path.py conflicts after #1119 changed the same call-window bookkeeping. This meets the late-round blocker bar: the branch's overlapping changes produce it, it is reached on the ordinary merge path, and there is no working merge until the branch is refreshed and the resolution is validated.

The unchanged head still passes the directly affected agent-loop suites (365 passed), but those tests cannot validate an unresolved integration tree.

@LivXue
LivXue force-pushed the feat/align_empty_reply_with_hermes branch from a71f068 to d5c19e0 Compare October 27, 2026 07:59
@LivXue

LivXue commented Oct 27, 2026

Copy link
Copy Markdown
Member Author

@gloryfromca @0xKT Rebased onto main 26352da09 and pushed as d5c19e07d. What changed since a71f0688, for the re-review:

  • The rebase. It stopped once, in raven/agent/loop/turn_path.py, where fix(*): restore the context ring from the window the last call measured #1119 stamps the window reading (_stamp_context_used) on the reply at the same spot where this branch files a silent row. Both were kept; neither reads the other. git range-diff 471842132..a71f0688 26352da09..d5c19e07^ gives 98 = and 4 !, all four context lines only, and the zero-context diff of the branch is identical before and after.
  • d5c19e07: fix(*): restore the context ring from the window the last call measured #1119 restores a resume's context ring from the newest reply that carries a reading. This branch's cut-answer exit (_end_on_the_partial: a failed continuation, or a turn out of iterations or time with a continuation owed) wrote its reply without one, so a resume read an earlier, smaller reading. That reply now carries the reading of the turn's newest call; a call that reported no usage leaves it unstamped, as any other reply is. The other ending paths already stamp, or match main.
  • The description: only Verification changed, re-measured at this head.

@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 rebase onto 26352da0 resolves the outstanding conflict cleanly. I checked the rewritten series and the turn_path.py integration: the four non-identical range-diff entries retain both this PR's silent-turn behavior and main's context-window stamp. The added tip change also correctly stamps a reply assembled from a cut answer with the usage from the call it represents, while preserving the established unknown-usage behavior.

This re-review covered the repository rules and domain context, the changed diff and conflict resolution, the affected callers and history, backward compatibility, test strength, and the context-stamping architecture. The new regression exercises the integration rather than weakening prior coverage.

Verification at d5c19e07d8e3:

  • git merge-tree --write-tree HEAD github/main: clean
  • affected recovery/context tests: 432 passed
  • Ruff, diff, source-language, large-file, and commit-message checks: passed

@LivXue

LivXue commented Oct 27, 2026

Copy link
Copy Markdown
Member Author

Windows compatibility re-review — PR #1092 @ d5c19e07d8e38bf032f190ed764d3842c5f98ee0

Scope: the head moved from the previously reviewed a71f0688. Against the new main tip 26352da0, the PR's own production diff is essentially unchanged (44 files under raven/, +2027/−567 vs +2025/−567 before — two lines); the raw delta is main's merges, which now include the host-path-rules fix.

Result: no Windows-specific issue; the earlier conclusion holds, and the previously noted inherited ref-path failures are gone on main.

Verification (native Windows 11, CPython 3.12.13, isolated worktrees D:\tmp\rv-1092-n2 and D:\tmp\rv-1092-main, scratch HOME/USERPROFILE, PYTHONUTF8=1):

pytest tests/test_subagent_manager.py tests/test_session_manager.py \
       tests/test_rpc_session.py tests/test_spine_scheduler.py \
       tests/test_spine_silence.py tests/test_transport_failure_paths.py \
       tests/test_rpc_console.py -q
→ PR head: 15 failed, 970 passed, 1 skipped in 21.73s

same set minus the PR's new test_spine_silence.py at main 26352da0
→ main: 15 failed, 894 passed, 1 skipped in 21.82s

The failing-ID sets are identical (diffed — no additions, no removals): 11 test_subagent_manager.py spawn/path cases and 4 test_rpc_console.py path/ownership cases, all present on main without this PR. The previous run's 21 failures fell to 15 because main now carries the prompt_paths host-path-rules fix; the 76 extra passing tests at head are this PR's.

Limitations: the UI/TUI portions of the diff were not run; the full suite was not run; no live provider or gateway.

Comment thread raven/agent/context/silence.py Resolved
@0xKT

0xKT commented Oct 27, 2026

Copy link
Copy Markdown
Member

Not a blocker -- behaviour findings from the same acceptance pass, separate from the blocking thread. Most were reproduced by running probes through the PR's own test helpers in an isolated tree, against both this head and base 984d47d4; none of them stops the merge on its own.

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. A person's real one-word answer can be swallowed as silence (raven/spine/silence.py:108-119, decided at raven/agent/loop/recovery.py:348). On a USER turn the strict rule folds case and strips edge punctuation, and two markers are plain English (SILENT, NO REPLY). So an answer of Silent., silent, No reply. or "No reply" ends the turn silent. A channel gets nothing, the UIs draw "Raven chose not to reply.", and the row is stored as silent. A person's turn never teaches the marker; SILENCE_INSTRUCTION is added only for cron and sub-agent reports. Reproduced through AgentLoop.run_turn with a scripted reply: on this head 'Silent.' and 'No reply.' give silent=True with no text; base delivers both, and controls ('Four.', 'No reply yet.') are delivered on both. Accepting only the taught spelling on person-started turns, or no silence there at all, would avoid it.

  2. A reply an after_send hook composes on a silent turn is delivered but stored as silent. turn_path.py:2446-2448 flags the row before the hook runs, and :3316-3323 clears only the local silent. Reloads then draw "Raven chose not to reply.", ACP replay drops a runtime-opened turn, and cron.runs previews nothing. This contradicts CONTEXT.md's "the turn is then not silent".

  3. "The message tool answered this turn" has two predicates. Live it is MessageTool.sent_in_turn (own channel and chat only); stored, session/turn_shape.py counts any message call, including a failed one or one aimed elsewhere. A turn whose message call named another chat and then ended [SILENT] is reported silent live, while a reload shows a reply.

  4. A cut stream's fragment is judged as a whole reply (recovery.py:348 checks SILENT before NETWORK_CUT). A stream cut right after "No reply" ends the turn silent instead of continuing, and a continuation that is itself marker-shaped is dropped without the partial note.

  5. Output-limit continuation doubles max_tokens from the model ceiling (turn_path.py:2336-2340). On the streamed path max_tokens_base is already the ceiling, and _reply_room clamps only when room is short, so a provider that does not clamp is asked for twice its maximum.

  6. Streamed refusal then fallback: the refusal text streams live, then the fallback model's answer streams after it. The reader sees both, while the stored reply keeps only the answer.

  7. A cut answer whose continuation is a tool call: the partial words the reader saw are dropped from the session, so a reload shows an empty tool-call row.

  8. OpenAI-shaped structured refusals are not recognised with the pinned litellm 1.85.0. _refusal_of reads .refusal, which litellm moves into provider_specific_fields, and the streamed path drops refusal-only chunks.

  9. Typing indicator left running: gateway/spine.py:218-228 calls end_turn only for a USER-origin turn. A person's message merged into a proactive turn that then ends silent leaves Telegram, Discord and Matrix showing "typing" until the next send.

  10. Smaller:

    • The web UI's silent notice calls dropSay() on the open step, which erases a wrap-up step's narration (ui-web/src/state/session/stages.ts:208).
    • upstream_transport_failure is now built in two places.
    • Two test survivors: the exhausted wrap-up's silence check reading the loose rule on a USER turn, and a stream that dies after saying refusal being handed back as stream_cut.

Ran in an isolated tree (this head merged onto 984d47d4): all 45 changed Python test files, 2806 passed; tests/test_kernel_budget.py, 5 passed.

LivXue added 10 commits October 27, 2026 22:18
Each part of a cut answer had its think blocks taken out on its own,
so a cut inside an inline think block left both halves of the block
in place: the opening half unclosed in the first part, the closing tag
in the continuation. The joined reply delivered the raw reasoning,
"<think>abcdef</think>Four." for "Four.", and a marker after the
thought was never read as one. On a proactive turn the loose rule read
the continuation alone, "def</think>" over "[SILENT]", as a marker,
dropped it, and delivered the unclosed "<think>abc" as the report.

The parts are now kept as they came, and on every exit that ends on
them -- a continuation that completes or comes back empty, one that
fails, the spent continuations, a budget that stops the turn, and a
continuation that calls a tool -- the reply is the joined text with
its think blocks taken out of the whole, read for the marker by the
turn's rule the way a reply with no cut is. A continuation inside a
block the cut left open is read only that way, never as a marker or
as debris of its own.

A cut thought that the continuation only closes leaves no answer, and
the turn now ends with no text rather than with the reasoning as its
reply.
A cut "[SILENT]" whose continuation was only "</think>" ended the turn
silent: the loop reads a reply that is nothing but a closing tag with
no block open as the debris a reasoning parser leaves, and the
continuation added nothing to the marker. The gate read the same tag
as text that could not be the marker and let the held "[SILENT]" out
with it, so the reader saw the marker of a turn that ended silent.

The gate now reads a closing think tag with no block open, in any of
the spellings the loop strips, as debris: not a word of the reply, so
the hold goes on past it. A reply the loop then reads as text is let
out at the end of the call, as any held reply is.
The wire said the flag sits on the turn's closing entry, or on "the
entry that made the call" when the turn closed on a reply with no text.
Neither holds in general. The stamp goes on the turn's last assistant
row with text or tool calls, so a turn that sent a message, then
listed a directory, then said nothing has it on the listing's call,
not the send; and a notice the runtime files after the turn, for a
question nobody answered, comes after the stamped row without the
flag.

The field description, the schema, both TypeScript mirrors
(regenerated), CONTEXT.md and the docstrings that repeat it now say
exactly that, and a test pins the shape: the send unflagged, the
listing's call flagged, the notice after it unflagged.
…ut one

A thought cut in two, "<think>abc" and then "</think>" or
"def</think>", joined into a reply with no text once its think block
was taken out. The loop then ended the turn as a success with no text:
final content empty, no_output set, only the question stored, no
notice and no failure, while the same reply uncut took the
thinking-only moves and failed when they were spent. The answerless
guard only fails a turn whose content is None, and the join turned
None into "".

Before a reply with no tool call is read, a cut answer whose join with
what continued it is nothing but a thought now becomes that joined
reply, the cut parts done with: reply_kind and decide read it as they
read the uncut one, so it is prefilled, stepped down, sent to a
fallback model and failed in the same order. Each part a cut adds
leaves the joined text with something to read, so no later exit ends
on parts with no text.

The test that pinned the turn ending with no text is replaced by
tests that run the cut and uncut shapes side by side.
The row a tool call is filed on keeps what the model said beside the
call as it came, inline thought included. After a cut, the row took
the joined words with their think blocks stripped and trimmed, so a
cut inside a thought filed "Let me look." where the same call uncut
filed "<think>abc</think>Let me look.", and whether the stream had been
cut changed the history.

The row now takes the parts and the call's own text joined as they
came, which is what one uncut call would have said. The tests run the
cut and uncut shapes side by side; the one that read the trimmed
partial now reads it with the space at the cut kept.
CONTEXT.md said every exit that ends on a cut answer's parts reads
them with their think blocks taken out. Only closed blocks are: a
thought the cut left open and nothing closed stays in as text, on the
failed-continuation exit and with recovery switched off, just as a
reply with no cut and an unclosed think tag is read as text. The
entry and the loop's comment now say that, and a test runs the
unclosed thought cut and uncut side by side.
When cut parts join into a bare thought, the loop rereads them as the
uncut reply and drops the hold the cut carried. Without that drop, a
proactive turn let the held thought out together with the answer the
prefill drew, and no test noticed. The new test runs the uncut, cut
and cut-at-its-close shapes and expects only the answer on screen.
litellm's stream wrapper drops a chunk whose only payload is a refusal,
so a refusal an OpenAI-compatible model streams reaches Raven as an
empty stop. It was resent three times with backoff, every time as a
stream that dropped it again, then switched to the fallbacks and failed
in empty-reply words. The same call made without streaming reads the
refusal, and a gateway whose streams come back empty while its plain
calls work is rescued by the same move.

On a streamed turn, the first resend of a plain empty reply, the one
decide answers RETRY to, now asks the same model without streaming. It
is one of the empty_content_max_retries resends, under the same backoff
and notice; later resends stream again, and an empty reply right after
tools is resent as it was. The call hands the provider no fallback
chain, so a model switch stays the loop's.

What comes back is read as any reply: a refusal goes to its one
fallback switch or fails with the model's explanation, tool calls run,
an empty reply goes on down the ladder. What a stream would have shown
is shown once the reply is in, through the same sinks: the thought
through the reasoning sink, then the words through the gates a streamed
delta passes, so a marker is still held and dropped, a reply reaches
the reader once, and the resend opens the step a streamed one would.
deterministic_empty is unchanged in meaning: it stops only on a streak
of zero-output replies of one model and finish, so mixing a streamed
and an unstreamed attempt can only fail open.

Tests drive litellm's own OpenAI route over a mock transport through
the loop, and the loop's sinks, gates, steps and budget directly.
The unstreamed resend of an empty streamed reply replays what a stream
would have shown through the reader's sinks, but only for a call that
did not fail: a failed call's content is the account of the failure,
not a reply. Nothing pinned that guard, so dropping it left the suite
green while an error's text would have reached the reader as words of
the answer before the error handling ran.

The test resends an empty streamed reply and has the unstreamed call
come back as a non-retryable error: none of its text reaches the token
sink, and the turn ends as an error does.
@LivXue
LivXue force-pushed the feat/align_empty_reply_with_hermes branch from d5c19e0 to f55e814 Compare October 28, 2026 02:28
@LivXue

LivXue commented Oct 28, 2026

Copy link
Copy Markdown
Member Author

@0xKT Thanks for the acceptance pass. All ten are answered in f55e814ca, rebased onto main 06509f8e4; per item:

  1. One-word answers. Fixed (148534b45, abd2a0964). A person's turn honours only [SILENT] with whitespace around it, so Silent., silent, No reply., "No reply", NO_REPLY and *NO_REPLY* are delivered; proactive origins keep both of Hermes' rules. The stream gate holds only while the text could still become [SILENT], and reads past an inline think block the way the loop does (d60f2a644, e380170c3): before, <think>x</think>[SILENT] streamed whole on a person's turn, because the < released the gate.
  2. Hook reply on a silent turn. Fixed (5c3bd2105). The closing row holds the text the hook composed and is not filed silent, so a reload, the ACP replay and cron.runs show it.
  3. Two predicates. Fixed (3665d1585, f9a0a7d1e). The live verdict (MessageTool.sent_in_turn) is stamped as replied_via_tool on the turn's last stored assistant row with text or tool calls, and the session list and the ACP replay read that flag through turn_shape instead of looking for a message call. A failed send, or one to another chat, no longer counts as an answer. The flag crosses the wire on TranscriptMessage.
  4. A cut fragment judged as a reply. Fixed (5cbd2feeb, 4f53e673e, e742379e7). A reply finished stream_cut is read as a cut before any marker, and the gate keeps holding across the cut and its continuation. The reply the turn ends on, the joined parts with their closed think blocks taken out, is then read for a marker by the turn's rule on every exit. So No reply cut and continued with real text is delivered whole, and a stream cut right after a complete [SILENT] still ends silent with nothing shown. Parts that join into nothing but a thought are read as that thought uncut would be, prefill and step-down first (6fd7a1129). A continuation that is a marker on its own ends like an empty one: what streamed is the answer.
  5. Doubled ceiling. Fixed (a8bb1623d). The doubled max_tokens goes through send_max_tokens pinned for the model it is sent to, so it never passes that model's own ceiling whatever the provider clamps; the room beside the prompt still lowers it.
  6. Streamed refusal, then a fallback. Partly. The fallback's answer already opened a step of its own; bb2bb37d6 pins that. The refused words are not stored, by rule: the record keeps what the conversation goes on from, and a refusal the fallback replaced is not part of it, while the cut words in 7 are the model's own answer and the turn goes on from them. Keeping them for a reload would need a row kind shown to the reader but never sent to the model, and none exists; listed under Risk.
  7. A cut answer continued by a tool call. Fixed (8cd924be5, c32f2f019). The call's row carries the joined parts and whatever the continuation said beside the call, as the reader saw them. Within that turn the model still reads the partial twice (the cut row and the call row); taking the cut rows out of the window lost a before_execute_tools note and left the question autofill reading an old list, so that stays, listed under Risk.
  8. OpenAI-shaped refusals. Fixed (43ea4d1bb, 09959fff1). A non-streamed reply's refusal is read from provider_specific_fields["refusal"], where litellm moves it; a test drives litellm's own OpenAI route against a mocked endpoint. A streamed one never reaches Raven: litellm 1.85's stream wrapper drops a chunk whose only payload is a refusal (is_delta_empty counts content, tool calls and function calls only), so the stream reads as an empty stop. The first resend of an empty streamed reply therefore goes out without streaming, which reads the refusal and hands it to the refusal path: the one fallback switch, or a failure that quotes it. A test drives that end to end through litellm's OpenAI route; another still pins litellm dropping the chunk, so an upgrade that passes it on is noticed.
  9. Typing indicator. Fixed (5e11a657f, 2e60a095a). The gateway runner records, by turn id, the chat of each person's message a turn takes in, and when the turn ends with no reply the sink ends the typing on each of those chats too. A cron turn that took in a person's message and then fails with no output still leaves it on; listed under Risk.
  10. Smaller. dropSay is now dropReply and takes back only what the final call streamed after its last tool call (daf8d9d3a). upstream_transport_failure has one builder, transport_failure_response (ba2e9a55f). Both survivors are pinned (44cddebf4, 500a4cc1b); for the second, a stream that dies after a refusal finish now keeps that finish, so the refusal takes the one fallback switch or fails with REFUSAL_FAILURE instead of being continued.

Two more from the rebase and the gates:

  • fix(*): report a delegated turn's real outcome and cost #1121 marks a turn its budget ended (budget_stop_turn_at, reported as ACP max_turn_requests) by status interrupted, which this branch also gives a cut stream delivered as a partial. d835d50c1 writes the marker only for a run the budget ended; a cut answer that the last iteration or the wall clock left no room to continue still counts.
  • make lint-types refused the gateway record's key (turn_id is optional); 2e60a095a keys it as the turn's events are, req.turn_id or "".

Verification at f55e814ca is in the description.

@LivXue

LivXue commented Oct 28, 2026

Copy link
Copy Markdown
Member Author

Windows compatibility re-review — PR #1092 @ f55e814ca2995903eda400dcfa1952d2cb552216

Scope: the head moved from the previously reviewed d5c19e07. The PR's own raven/** diff is now 46 files (+2346/−602); the delta includes main's merges plus refinements in spine/events.py (the silent field is gone), spine/scheduler.py, and token_wise/usage_tracker.py (counts_known stores absent counts as null rather than zeros). tests/test_spine_silence.py was renamed to tests/test_agent_context_silence.py (54 tests, all passing).

Result: no Windows-specific issue, and no reproducible new Windows failure.

Verification (native Windows 11, CPython 3.12.13, isolated worktrees D:\tmp\rv-1092-n3 and D:\tmp\rv-1092-m2, scratch HOME/USERPROFILE, PYTHONUTF8=1, PYTHONPATH pinned to the tree under test):

pytest tests/test_subagent_manager.py tests/test_session_manager.py \
       tests/test_rpc_session.py tests/test_spine_scheduler.py \
       tests/test_agent_context_silence.py tests/test_transport_failure_paths.py \
       tests/test_rpc_console.py -q
→ head run 1: 16 failed, 982 passed, 1 skipped
→ head run 2: 15 failed, 983 passed, 1 skipped

same set minus the new agent-context file at main 06509f8e
→ main: 15 failed, 898 passed, 1 skipped

The failing-ID sets are identical between the second head run and main (diffed — no additions, no removals): 11 test_subagent_manager.py spawn/path cases and 4 test_rpc_console.py path/ownership cases, all inherited. The first head run had one extra failure that did not reproduce on the rerun; no name in either run is outside the inherited set.

Limitations: the UI portion was not re-run this pass (unchanged behaviour, main's merge only); no live gateway.

@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 35-commit delta beyond the previously assessed series; range-diff shows the earlier 103 commits replayed unchanged. The frozen-kernel fix is complete, and the acceptance changes consistently cover the per-origin silence rule, live message-tool verdict, cut/continued replies, OpenAI-shaped refusals, merged-message typing cleanup, and transcript rollback without weakening prior behavior. I found no plain error worth raising.

This pass covered the repository rules and domain context, the delta and affected callers, history/range-diff, backward compatibility, test changes, and the kernel/layer architecture.

Verification at f55e814ca299:

  • affected Python suites: 1,353 passed, 27 warnings (Pydantic/LiteLLM warnings plus one LiteLLM logging-worker RuntimeWarning)
  • affected web suites: 275 passed
  • import contracts: 10 kept, 0 broken
  • Python and UI type checks, Ruff, RPC generation, source-language, large-file, commit-message, and diff checks: passed
  • this host has no make, so I ran the lint-types target's exact uv ... ty check recipe directly; it passed
  • current github/main is the merge base; merge-tree is clean and GitHub reports MERGEABLE

The silence gate now reads think tags as it holds a person's reply,
and it read every character of a held delta through that reader. A
long hold is only ever whitespace before the marker or after the whole
of it, which moves no reading, yet each blank delta paid the full pass:
eighty thousand of them took 1.72 s in CI's coverage-traced run, past
the bound the hold's cost test sets.

A whitespace delta that every reading would pass over unchanged is now
kept as it is. Under coverage the same run takes about 0.22 s, close to
what it took before the tag reader. A new test counts the characters
the readers see; a randomized comparison of 20,000 delta sequences
against the per-character path found the same output every time.
@LivXue

LivXue commented Oct 28, 2026

Copy link
Copy Markdown
Member Author

CI on f55e814ca failed one test in shard 2/4: test_a_long_hold_costs_each_delta_only_its_own_length, where 80,000 one-character deltas took 1.72 s under coverage against a 1.0 s bound. This round's gate reads think tags, and it passed every held character through that reader. The cost stayed linear (twice the deltas, twice the time), but each blank delta got about 4.5 times dearer under coverage.

fab98f576 keeps a whitespace delta that no reading would move without reading it, and a long hold is only ever that. The same run now takes about 0.22 s under coverage; it took 0.17 s before the tag reader. A new test counts the characters the readers see, and a randomized comparison of 20,000 delta sequences against the per-character path gave the same output every time. make coverage-shard COVERAGE_SHARD=2/4 PYTEST_ARGS=--idle-ceiling-strict passes locally at fab98f576.

@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 one-commit delta from f55e814ca to fab98f576. The whitespace fast path is behavior-preserving: it applies only when every active marker reading is outside a think block/tag and is either before the marker or after the complete marker, exactly the states where reading whitespace returns true without changing state. It removes the coverage-triggered per-character overhead without weakening the timing bound.

Verification at the new head: the full empty-recovery file passes (446 tests); the timing regression and new reader-count regression pass under coverage with strict idle ceilings; Ruff passes on both changed files. I covered the repository rules, the delta and surrounding state machine, the CI-triggering history, caller/backward behavior, test strength, and architecture impact. The clean-thread sweep found my only review thread already resolved.

@LivXue

LivXue commented Oct 28, 2026

Copy link
Copy Markdown
Member Author

Windows compatibility re-review — PR #1092 @ fab98f57681fdf389cfb1f1a4ab6d3559f705039

Scope: the delta from the previously reviewed head f55e814ca (one commit: raven/agent/loop/recovery.py +8, tests/test_agent_loop_empty_recovery.py +29), re-checked on native Windows. This commit removes a CI timing regression in the previous head; it is pure string/state-machine logic with no OS-facing calls. No Windows-specific change and no new Windows finding.

What changed: SilenceGate.__call__ now appends a whitespace-only delta to the hold without running each character through _Reading.read when every reading has block is None, an empty tag, and body in ("", SILENT_MARKER).

Equivalence check (code reading): under exactly those preconditions the per-character path returns True for every whitespace character and mutates nothing (line 726-728 of raven/agent/loop/recovery.py), so no reading can die, no closed step can fire, and the only observable outcome of the slow path is also "append to _held, deliver nothing". _nothing_of_a_marker_held() and the post-loop block is not None check therefore make the same decision. The fast path uses str.isspace() while the slow path uses char.isspace() for each character, so \r\n and Unicode whitespace behave identically, with no line-ending or encoding divergence between Windows and POSIX. Empty deltas are unaffected ("".isspace() is false and takes the existing loop, which appends ""). No filesystem, subprocess, signal, socket, or path API is involved.

Verification on native Windows 11, detached worktree D:\tmp\rv-1092 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_empty_recovery.py tests/test_agent_context_silence.py -q → 500 passed in 32.36s
  • pytest ...::test_a_run_of_blank_deltas_around_the_marker_is_held_without_reading_each_one ...::test_a_long_hold_costs_each_delta_only_its_own_length → 2 passed in 5.72s

Gaps: the full suite and the coverage-traced shard were not rerun on Windows for this delta; the timing figures in the commit message come from the author's CI run. Within those limits, no Windows compatibility issue was found for this head.

@0xKT

0xKT commented Oct 28, 2026

Copy link
Copy Markdown
Member

Not a blocker - delta acceptance on fab98f57 (onto 06509f8e). The previous blocking thread (kernel seat of silence.py, dead TurnEnded.silent) was re-verified fixed and closed. The ten non-blocking items from the last pass were re-checked one by one, with probes and mutants, and all hold on this head:

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.

  • a person's one-word Silent. / No reply. is delivered;
  • a hook-composed reply on a silent turn is stored as the reply;
  • live and stored message-tool verdict agree;
  • cut fragments are no longer read as markers;
  • the continuation ceiling is held to the model's own;
  • a cut answer continued by a tool call keeps its words;
  • the OpenAI refusal is read;
  • typing stops after a merged message;
  • the wrap-up narration is kept in the main shape;
  • one transport-failure builder.

What is left is non-blocking:

  1. Kernel: no_output is read through normal attribute access (raven/spine/scheduler.py:575, isinstance(exc, AnswerlessTurnError) and exc.no_output). full_detail_of right above it (:53-63) reads the instance dict precisely so that "a foreign getter can throw". An out-of-tree runner whose AnswerlessTurnError subclass skips the base __init__, or defines no_output as a raising property, makes that line raise inside the failure branch. Then no TurnFailed is emitted, the worker dies, and the queued turn waits for a later submit. Measured on this head with the public Scheduler; base gives one TurnFailed and runs the next turn. No in-tree subclass exists. Reading it the same way as full_detail (BaseException.__dict__["__dict__"].__get__(exc).get("no_output") is True) fixes both shapes, and the kernel suites stay green with it.
  2. Untaught proactive turns read the loose markers. is_silence (raven/agent/context/silence.py:114-115) applies the loose rule to every PROACTIVE_ORIGINS turn. SILENCE_INSTRUCTION is taught only on the cron reminder and the sub-agent unprompted report. The heartbeat task run, the sub-agent RESULT relay ("Summarize this naturally for the user") and the sentinel notice relay are never taught it, but still swallow an answer that is No reply. or ends on a No reply line. This is the same class as the person-turn fix in 148534b45.
  3. A proactive stream cut between the note and the marker delivers the note, while the same output uncut is silent (turn_path.py:2457-2478). The "reader already holds the answer" premise behind dropping a marker-only continuation is false under hold_all, where nothing was shown.
  4. A cut answer's continuation that raises fails the turn (after up to about 105 s of in-stream waiting) instead of ending on the partial with the note. Only an error response takes the documented path (turn_path.py:2131-2143), and LiteLLMProvider.chat_stream raises.
  5. Take-back hides turns the reader did see. acp/replay.py:105-115 and session/manager.py:930-941 drop a runtime-opened turn that ends on [SILENT] unless replied_via_tool is set. Files delivered by deliver_files, and a message call aimed at another chat (which _route_to_stream still shows to the turn's own reader), do not set it. Those turns vanish from ACP replay and the session list.
  6. Take-back leaves the turn's unanswered-question notice (filed after the turn by _file_unanswered). It then replays as an orphan question_unanswered message and moves the session list.
  7. ui-web: a silent wrap-up still erases narration when the last call's only tool was message. replyAt advances only on tool.start, and the loop emits none for message (turn_path.py:1746), so dropReply slices at 0 (ui-web/src/features/transcript/store.ts:1179-1185, 1207).

Tests here on the merged tree: all 45 changed Python test files via the board runner gave 2972 passed. ui-web, 6 changed test files: 340 passed. ui-tui, 3 changed test files: 116 passed. lint-imports: 10 kept.

@LivXue

LivXue commented Oct 29, 2026

Copy link
Copy Markdown
Member Author

Thanks for the pass. #1092 merged before I answered it, so the seven
items are fixed in #1146, one commit each. Every fix was watched failing
before the change and passing after, and each guard was mutated after.

  1. no_output through attribute access - e9153952e.
    no_output_of reads the instance dictionary, the reading you
    suggested. Both shapes (a skipped base __init__, a raising
    property) now file one TurnFailed; at the base the test gets none.
  2. Untaught proactive turns - 227ea4f86. The loose rule now needs
    the prompt the model read to carry SILENCE_INSTRUCTION
    (teaches_silence), read after the inbound hooks. The heartbeat
    task, the RESULT relay and the sentinel notice honour only
    [SILENT] and stream as a person's turn does; the cron reminder and
    the unprompted report keep the loose rule and the whole-call hold.
  3. A held note cut before its marker - f3703833c. On a turn that
    showed nothing of the partial, a marker-only continuation is read
    with it on its own line, as the uncut reply is, so it ends silent
    too. A person's turn, which showed the partial, keeps it as before.
  4. A continuation that raises - 146ea5eba. It now ends on the
    partial with the note, interrupted. A raise with no cut behind it
    still fails the turn, and a cancel still propagates.
  5. Take-back hiding turns the reader saw - 454d1c2f4. The send
    callback has no target and always reaches the turn's own reader, so
    sent_in_turn now counts any send that went out rather than one
    naming the turn's own chat; a turn whose tools delivered files is
    stamped replied_via_tool too. A failed send still answers nobody.
  6. The orphaned unanswered notice - 81d11b7c3. A turn the two
    readers take back files none, by their rule: the last user row
    before the marker carries an origin. The test runs the replay's own
    _without_silent_runtime_turns over the stored rows. A person's
    silent turn, which both keep, still files its notice.
  7. ui-web erasing narration - 3f5805d94. A different shape from a
    web-side guard: the TUI loses the same narration by another route (it
    cleared its whole stream buffer, and a message-only call flushes
    nothing), so the fix is on the notice. silent_turn now carries in
    detail what the closing reply had streamed (at most a think block;
    the gates hold the rest), counted over the step the reader draws it
    in. The web page and the TUI strip exactly that suffix and keep the
    narration.

Verification on the follow-up head: the full Python suite 36967 passed,
with 8 failures local to this box, none in a module the diff changes;
ui-web 4475 and its gates 239 passed; ui-tui 2275 passed, with two
load flakes that pass alone; the changed test files pass under
coverage with the strict idle ceiling.

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