From 12abaeefc021fbb8a915a489c2c77830cff4e1d0 Mon Sep 17 00:00:00 2001 From: Dizhan Xue Date: Thu, 29 Oct 2026 06:42:43 +0000 Subject: [PATCH 01/13] fix(spine): read a failure's no_output off the instance dictionary The scheduler read no_output with plain attribute access while filing a turn's failure, right below full_detail_of, which reads the instance dictionary because a foreign getter can throw. A runner's AnswerlessTurnError subclass that skipped the base __init__, or guarded no_output with a getter that throws, made the failure branch raise itself: no TurnFailed went out and the lane's worker died. no_output_of reads it the way full_detail_of reads its field, and anything but True means no. A test files a failure for both subclass shapes. --- raven/spine/scheduler.py | 13 ++++++++++++- tests/test_spine_scheduler.py | 30 ++++++++++++++++++++++++++++++ 2 files changed, 42 insertions(+), 1 deletion(-) diff --git a/raven/spine/scheduler.py b/raven/spine/scheduler.py index b7c73b206..73c9f501a 100644 --- a/raven/spine/scheduler.py +++ b/raven/spine/scheduler.py @@ -63,6 +63,17 @@ def full_detail_of(exc: BaseException) -> str | None: return value if isinstance(value, str) and value.strip() else None +def no_output_of(exc: BaseException) -> bool: + """Whether a runner's ``AnswerlessTurnError`` says the turn's last reply was empty. + + Read off the instance dictionary for the reason ``full_detail_of`` is: a foreign getter can throw, + and a throw here would leave the turn's failure unfiled. Anything but ``True`` means no. + """ + if not isinstance(exc, AnswerlessTurnError): + return False + return BaseException.__dict__["__dict__"].__get__(exc).get("no_output") is True + + def conversation_id(req: TurnRequest) -> str: """The lane key a request runs on: its explicit conversation, else ``:``. @@ -572,7 +583,7 @@ def drain() -> list[TurnRequest]: # hosts and paths. reported=isinstance(exc, AnswerlessTurnError), full_detail=full_detail_of(exc), - no_output=isinstance(exc, AnswerlessTurnError) and exc.no_output, + no_output=no_output_of(exc), ) await self._sink(failed) self._payload_reported = True diff --git a/tests/test_spine_scheduler.py b/tests/test_spine_scheduler.py index c090b0e7e..2a8202153 100644 --- a/tests/test_spine_scheduler.py +++ b/tests/test_spine_scheduler.py @@ -126,6 +126,36 @@ async def test_a_no_output_failure_reaches_turn_failed(): assert failed.no_output is True and failed.reported is True +class _SkipsInit(AnswerlessTurnError): + """A runner's subclass that never sets ``no_output``.""" + + def __init__(self, text: str) -> None: + Exception.__init__(self, text) + + +class _GuardsNoOutput(AnswerlessTurnError): + """A runner's subclass whose ``no_output`` is a getter that throws.""" + + def __init__(self, text: str) -> None: + Exception.__init__(self, text) + + @property + def no_output(self) -> bool: + raise RuntimeError("a foreign getter") + + +@pytest.mark.parametrize( + "exc", [_SkipsInit("no answer"), _GuardsNoOutput("no answer")], ids=["skips_init", "throwing_getter"] +) +async def test_a_foreign_answerless_error_still_has_its_turn_failed(exc): + """``no_output`` is read off the instance dictionary, as ``full_detail`` is: a subclass that never set + it, or guards it with a getter that throws, still gets its ``TurnFailed`` rather than killing the lane.""" + events = await _lifecycle_of(RaisingRunner(exc)) + failed = [e for e in events if isinstance(e, TurnFailed)] + assert len(failed) == 1 + assert failed[0].no_output is False and failed[0].reported is True + + async def test_an_answerless_failure_is_no_output_only_when_the_runner_says_so(): exc = AnswerlessTurnError("Error calling LLM (rate_limit@openrouter): busy") events = await _lifecycle_of(RaisingRunner(exc)) From 8dd99238f2296e6dabf6a1507d8b70bcb048a792 Mon Sep 17 00:00:00 2001 From: Dizhan Xue Date: Thu, 29 Oct 2026 06:42:43 +0000 Subject: [PATCH 02/13] fix(session): list a transcript saved with a BOM or in a code page The session list read every transcript as plain UTF-8. A file a Windows editor saved with a byte order mark put the mark in front of the metadata line, which then failed to parse and took the session off the list; one saved in a Windows code page raised UnicodeDecodeError out of the scan and failed the listing as a whole. The scan now reads utf-8-sig, and leaves an undecodable transcript out as it already leaves out an unreadable one. The ACP replay tests resolve their working directory under tmp_path, so the paths they expect are the ones the replay builds on Windows too. --- raven/session/manager.py | 8 ++++++-- tests/test_acp_replay.py | 16 ++++++++-------- tests/test_session_manager.py | 32 ++++++++++++++++++++++++++++++++ 3 files changed, 46 insertions(+), 10 deletions(-) diff --git a/raven/session/manager.py b/raven/session/manager.py index ceea47263..fb26eea95 100644 --- a/raven/session/manager.py +++ b/raven/session/manager.py @@ -908,7 +908,9 @@ def _scan_file(path: Path) -> tuple[dict[str, Any] | None, int, str, str, str]: # turn that ends silent to be taken back whole. before_runtime_turn: tuple[str, str] | None = None try: - with path.open(encoding="utf-8") as f: + # utf-8-sig: a Windows editor that saves UTF-8 with a byte order mark puts it in front of the + # metadata line, which would then fail to parse and take the session off the list. + with path.open(encoding="utf-8-sig") as f: for line in f: line = line.strip() if not line: @@ -939,7 +941,9 @@ def _scan_file(path: Path) -> tuple[dict[str, Any] | None, int, str, str, str]: last_ts = ts elif before_runtime_turn is not None and not replied_via_message_tool(data): latest_preview, last_ts = before_runtime_turn - except OSError: + except (OSError, UnicodeDecodeError): + # A transcript that is not UTF-8 (one saved in a Windows code page) is left out as an + # unreadable one is, rather than its decoding error escaping the whole list. return None, 0, "", "", "" return meta, count, last_ts, first_user, latest_preview diff --git a/tests/test_acp_replay.py b/tests/test_acp_replay.py index 41992f8e6..681f22839 100644 --- a/tests/test_acp_replay.py +++ b/tests/test_acp_replay.py @@ -60,7 +60,7 @@ def test_a_delegated_report_preserves_the_sending_agents_text(): class TestOrder: - def test_a_whole_conversation_replays_in_the_order_it_happened(self): + def test_a_whole_conversation_replays_in_the_order_it_happened(self, tmp_path): transcript = [ {"role": "system", "text": "you are raven"}, {"role": "user", "text": "read a.py and fix it"}, @@ -74,7 +74,7 @@ def test_a_whole_conversation_replays_in_the_order_it_happened(self): {"role": "assistant", "text": "Done."}, ] - updates = _valid(replay(transcript, session_id="acp:s1", cwd="/work")) + updates = _valid(replay(transcript, session_id="acp:s1", cwd=str(tmp_path))) assert _kinds(updates) == [ "user_message_chunk", @@ -145,18 +145,18 @@ def test_a_replayed_call_states_its_own_name_too(self): assert updates[0]["_meta"] == {"raven.toolName": "todowrite"} assert updates[0]["kind"] == "other" - def test_stored_arguments_are_parsed_so_the_row_can_be_labelled(self): + def test_stored_arguments_are_parsed_so_the_row_can_be_labelled(self, tmp_path): """They are kept as the JSON string the provider sent, so a title or a location needs them parsed.""" updates = replay( [{"role": "assistant", "tool_calls": [{"id": "c1", "name": "read_file", "arguments": '{"path":"a.py"}'}]}], session_id="acp:s1", - cwd="/work", + cwd=str(tmp_path), ) assert updates[0]["title"] == "read_file: a.py" assert updates[0]["kind"] == "read" - assert updates[0]["locations"] == [{"path": "/work/a.py"}] + assert updates[0]["locations"] == [{"path": str(tmp_path / "a.py")}] def test_arguments_that_will_not_parse_still_produce_a_row(self): """Guessing at half-parsed arguments would put a fragment of JSON on a @@ -217,17 +217,17 @@ def test_a_result_with_nothing_in_it_still_closes_the_row(self): assert updates == [{"sessionUpdate": "tool_call_update", "toolCallId": "c1", "status": "completed"}] - def test_arguments_already_parsed_are_used_as_they_are(self): + def test_arguments_already_parsed_are_used_as_they_are(self, tmp_path): """A live-cache message holds them as a mapping rather than as the JSON string a stored one carries, and both shapes reach here.""" updates = replay( [{"role": "assistant", "tool_calls": [{"id": "c1", "name": "read_file", "arguments": {"path": "a.py"}}]}], session_id="acp:s1", - cwd="/work", + cwd=str(tmp_path), ) assert updates[0]["title"] == "read_file: a.py" - assert updates[0]["locations"] == [{"path": "/work/a.py"}] + assert updates[0]["locations"] == [{"path": str(tmp_path / "a.py")}] @pytest.mark.parametrize("arguments", [None, [], 5, "", " ", "[1, 2]", '"a string"']) def test_arguments_of_an_unusable_shape_leave_the_row_unlabelled(self, arguments): diff --git a/tests/test_session_manager.py b/tests/test_session_manager.py index 4d10931ea..af0816b9e 100644 --- a/tests/test_session_manager.py +++ b/tests/test_session_manager.py @@ -339,6 +339,38 @@ def test_a_session_holding_only_its_open_question_is_a_complete_row(tmp_path: Pa assert row["last_message_at"], "the row has no stamp to sort the picker by" +def test_a_transcript_saved_with_a_byte_order_mark_is_still_listed(tmp_path: Path): + """A Windows editor that saves UTF-8 with a byte order mark puts it in front of the metadata line. + Read as plain UTF-8, that line did not parse and the whole session left the list.""" + mgr = SessionManager(tmp_path) + session = mgr.get_or_create("tui:bom01") + session.add_message("user", "hello") + mgr.save(session) + path = tmp_path / "sessions" / "tui" / "bom01.jsonl" + path.write_bytes(b"\xef\xbb\xbf" + path.read_bytes()) + + rows = [info for info in SessionManager(tmp_path).list_sessions() if info["key"] == "tui:bom01"] + + assert len(rows) == 1 + assert rows[0]["first_user_message"] == "hello" + + +def test_a_transcript_in_a_legacy_code_page_is_left_out_without_breaking_the_list(tmp_path: Path): + """A transcript saved in a Windows ANSI code page is not UTF-8. It is left out as an unreadable file + is, rather than its decoding error escaping the whole list.""" + mgr = SessionManager(tmp_path) + for key in ("tui:good01", "tui:cp01"): + session = mgr.get_or_create(key) + session.add_message("user", "\u4f60\u597d") + mgr.save(session) + path = tmp_path / "sessions" / "tui" / "cp01.jsonl" + path.write_bytes(path.read_text(encoding="utf-8").replace("\\u4f60\\u597d", "\u4f60\u597d").encode("cp936")) + + keys = [info["key"] for info in SessionManager(tmp_path).list_sessions()] + + assert keys == ["tui:good01"] + + def test_list_sessions_sees_nested_layout(tmp_path: Path): """list_sessions enumerates nested per-channel files.""" mgr = SessionManager(tmp_path) From d595ee7c749e71a0a64ed4e2aea18f970ec7a4a7 Mon Sep 17 00:00:00 2001 From: Dizhan Xue Date: Thu, 29 Oct 2026 06:42:43 +0000 Subject: [PATCH 03/13] fix(agent): file no unanswered notice into a turn that is taken back The session list and the ACP replay take back whole a turn the runtime opened and the model closed with a Silence Marker the message tool did not answer. A question that turn could not put to anyone was still filed as a notice after the marker, and it outlived the take-back: an orphan question row in the replay, and a session list moved by a turn it otherwise hides. Such a turn now files none, by the rule those two readers apply: the last user row before the marker carries an origin. A person's silent turn, which both keep, still files its notice. --- CONTEXT.md | 4 ++- raven/agent/loop/turn_path.py | 29 ++++++++++++++-- tests/test_agent_loop_empty_recovery.py | 44 +++++++++++++++++++++++++ 3 files changed, 74 insertions(+), 3 deletions(-) diff --git a/CONTEXT.md b/CONTEXT.md index e4b26bc08..c8c68ca29 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -862,7 +862,9 @@ runtime files after the turn -- carries that live verdict as `replied_via_tool`, surfaces that read a stored turn -- the ACP replay, which draws it, and the session list, which orders sessions by it -- read the flag from one place (`session/turn_shape.py`) rather than guess it from the calls they can see, where a send -that failed or went to another chat would look like an answer. +that failed or went to another chat would look like an answer. A Silent Turn the runtime opened, +which those two surfaces take back whole, files no unanswered-question notice after its marker, so +none is left behind as an orphan; a person's Silent Turn, which they keep, still files it. Distinct from a `no_output` failure. A Silent Turn is the model saying nothing on purpose, so it succeeds; `no_output` is what a turn is failed with when its last reply was empty -- no text, no tool call, no reasoning -- once every retry was spent, or, with recovery switched off, when its diff --git a/raven/agent/loop/turn_path.py b/raven/agent/loop/turn_path.py index a9172ab83..d17bc06cc 100644 --- a/raven/agent/loop/turn_path.py +++ b/raven/agent/loop/turn_path.py @@ -466,6 +466,28 @@ def _file_unanswered(messages: list[dict]) -> None: messages.append({"role": "assistant", "content": text, _NOTICE_KEY: notice}) +def _taken_back_whole(messages: list[dict], skip: int) -> bool: + """Whether the session list and the ACP replay will take this turn back whole. + + By the rule they read a transcript with: the turn's closing assistant row is a Silence Marker + the message tool did not answer, and the last user row before it carries an origin, so the + runtime opened the turn. A person's message opens one they keep. ``skip`` is where the turn's + new rows start; the row that opened it may be older, filed by the opening write. + """ + for index in range(len(messages) - 1, skip - 1, -1): + if messages[index].get("role") == "assistant": + closing = messages[index] + break + else: + return False + if not closing.get(_SILENT_KEY) or closing.get(_REPLIED_VIA_TOOL_KEY): + return False + for row in reversed(messages[:index]): + if row.get("role") == "user" and not row.get("_recovery_synthetic"): + return bool(row.get(_ORIGIN_KEY) or row.get("origin")) + return False + + def _turn_trace_id() -> str | None: """The trace id of the ``session.turn`` span this code runs directly inside. @@ -3770,8 +3792,11 @@ def _save_turn( # Both exits that persist a turn come through here, so a question noted # on the turn is filed once whichever exit runs. The early inbound write - # also comes through, before any question, and files nothing. - _file_unanswered(messages) + # also comes through, before any question, and files nothing. A turn the + # session list and the ACP replay take back whole files none: a notice + # after its marker would outlive the take-back as an orphan. + if not _taken_back_whole(messages, skip): + _file_unanswered(messages) first_user_pending = received_at is not None # The turn's first user entry is the inbound message; hooks may have # rewritten what the model saw (a memo prepended, a reminder appended), diff --git a/tests/test_agent_loop_empty_recovery.py b/tests/test_agent_loop_empty_recovery.py index 38ff07345..945fed800 100644 --- a/tests/test_agent_loop_empty_recovery.py +++ b/tests/test_agent_loop_empty_recovery.py @@ -5149,6 +5149,50 @@ async def execute(name, arguments, **kwargs): ] == [("message", False), ("list_dir", True), ("notice", False)] +@pytest.mark.asyncio +@pytest.mark.parametrize( + "origin,first,last,noticed", + [ + (Origin.CRON, None, "[SILENT]", False), + (Origin.CRON, None, "Report.", True), + (Origin.CRON, "message", "[SILENT]", True), + (Origin.USER, None, "[SILENT]", True), + ], + ids=["silent", "answered", "silent_after_the_message_tool_sent", "a_persons_silent_turn"], +) +async def test_a_silent_runtime_turn_files_no_unanswered_notice(workspace, monkeypatch, origin, first, last, noticed): + """A runtime-opened turn that ends silent is taken back whole by the session list and the ACP replay. + A notice filed after its marker outlived that take-back: an orphan question row in the replay, and the + session list moved by a turn it otherwise hides. Such a turn files none; one that answered still does, + and so does a person's silent turn, which both keep: its notice is what the next turn reads.""" + import raven.permissions.turn as permissions_turn + from raven.acp.replay import _without_silent_runtime_turns + + asked: list[str] = [] + filing = ("A question went unanswered.", {"kind": permissions_turn.UNANSWERED_KIND, "text": "q"}) + monkeypatch.setattr(permissions_turn, "unanswered_filing", lambda: filing if asked else None) + _served(monkeypatch, [_tool_reply("message", _SEND) if first else _tool_reply(), _reply(last)]) + agent = _litellm_agent(workspace) + if first: + _wire_message_tool(agent) + real_execute = agent.tools.execute + + async def execute(name, arguments, **kwargs): + asked.append(name) + return await real_execute(name, arguments, **kwargs) + + monkeypatch.setattr(agent.tools, "execute", execute) + + await agent._process_message(_turn(origin=origin), session_key="s1") + + rows = agent.sessions.get_or_create("s1").messages + notices = [m for m in rows if isinstance(m.get("notice"), dict)] + assert asked, "the turn met its question" + assert bool(notices) is noticed + kept = _without_silent_runtime_turns([dict(m) for m in rows if m.get("role")]) + assert any(m.get("role") == "user" for m in kept) is noticed, "a notice is filed exactly where the turn stays" + + @pytest.mark.asyncio async def test_a_marker_after_the_message_tool_sent_is_not_a_silent_turn(workspace, monkeypatch): """The reader has the answer from the tool; the marker only says there is nothing more. From 006bc033ab02aa6a1b54b625a2d85c407a3e66af Mon Sep 17 00:00:00 2001 From: Dizhan Xue Date: Thu, 29 Oct 2026 06:42:43 +0000 Subject: [PATCH 04/13] fix(agent): end on what streamed when a continuation raises A cut answer's continuation can fail by raising rather than by answering with an error: LiteLLMProvider.chat_stream raises its HTTP errors once the stream's own reconnects are spent. The loop let that escape and failed the whole turn, though the reader already held the answer so far. It now ends the turn on the partial with PARTIAL_STREAM_NOTE, as interrupted, as it does for a continuation that answered with an error. A call that raises with no cut behind it still fails the turn, and a cancel still propagates. --- CONTEXT.md | 3 +- raven/agent/loop/turn_path.py | 69 +++++++++++++--------- tests/test_agent_loop_broken_turn.py | 31 +++++++--- tests/test_agent_loop_empty_recovery.py | 76 +++++++++++++++++++++++++ 4 files changed, 143 insertions(+), 36 deletions(-) diff --git a/CONTEXT.md b/CONTEXT.md index c8c68ca29..241b86e49 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -899,7 +899,8 @@ continuation that comes back empty leaves what streamed as the answer, and so do marker on its own, unless the cut left a think block open, when it is read only joined to what came before it; one that comes back as an error, a stream cut off before any new text included, ends the turn at once on the partial with -the note, as `interrupted`, with no wait and no fallback model -- and so does a turn that runs out +the note, as `interrupted`, with no wait and no fallback model, and so does one that raises once the +stream's own reconnects are spent -- and so does a turn that runs out of iterations or time while a continuation is still owed, with no wrap-up. On each of those exits the reply the turn ends on is the joined parts with their closed think blocks taken out of the whole -- a cut can split one -- and a block still unclosed left in as text, read for a Silence Marker by diff --git a/raven/agent/loop/turn_path.py b/raven/agent/loop/turn_path.py index d17bc06cc..c816f62ed 100644 --- a/raven/agent/loop/turn_path.py +++ b/raven/agent/loop/turn_path.py @@ -1524,33 +1524,50 @@ async def _end_on_the_partial(last: LLMResponse) -> str: usage_context.bind(session_key) if session_key else contextlib.nullcontext(), ): unstreamed, unstreamed_next = unstreamed_next, False - response = await self.harness.action.decide( - ActionRequest( - provider=self.provider, - messages=call_messages, - tools=call_tools, - model=call_model, - # An unstreamed resend is still the streamed turn's call: the provider's - # own chain is not walked, and a model switch stays the loop's to make. - fallback_models=[] if unstreamed else list(fallbacks_left), - # The one streaming call that asks for a cut answer back: a - # continuation (``ReplyKind.NETWORK_CUT``) follows it. The - # wrap-up and the watch-work judgement call the same method - # and have nothing to follow them. - stream_call=functools.partial(self._llm_call_stream, hand_back_partial=True), - # Spliced here rather than inside the module: the gates are - # the shell's own, and which sink a delta reaches is not a - # strategy decision. The module reads the field it is handed, - # so the stream/retry branch stays exactly the one the loop - # took -- no gate is built unless ``on_token_delta`` - # already is, so the spliced value is None on exactly the - # turns the raw sink was, and a reasoning sink alone still - # streams. - on_token_delta=None if unstreamed else (draft or gate or downstream), - on_reasoning_delta=None if unstreamed else on_reasoning_delta, - generation_overrides=gen_overrides, + continuing = bool(recovery.parts) + try: + response = await self.harness.action.decide( + ActionRequest( + provider=self.provider, + messages=call_messages, + tools=call_tools, + model=call_model, + # An unstreamed resend is still the streamed turn's call: the provider's + # own chain is not walked, and a model switch stays the loop's to make. + fallback_models=[] if unstreamed else list(fallbacks_left), + # The one streaming call that asks for a cut answer back: a + # continuation (``ReplyKind.NETWORK_CUT``) follows it. The + # wrap-up and the watch-work judgement call the same method + # and have nothing to follow them. + stream_call=functools.partial(self._llm_call_stream, hand_back_partial=True), + # Spliced here rather than inside the module: the gates are + # the shell's own, and which sink a delta reaches is not a + # strategy decision. The module reads the field it is handed, + # so the stream/retry branch stays exactly the one the loop + # took -- no gate is built unless ``on_token_delta`` + # already is, so the spliced value is None on exactly the + # turns the raw sink was, and a reasoning sink alone still + # streams. + on_token_delta=None if unstreamed else (draft or gate or downstream), + on_reasoning_delta=None if unstreamed else on_reasoning_delta, + generation_overrides=gen_overrides, + ) ) - ) + except Exception as exc: + if not continuing: + raise + # A continuation can fail by raising rather than by answering with an error + # (``LiteLLMProvider.chat_stream`` raises its HTTP errors), once the stream's + # own reconnects are spent. The reader holds the answer so far either way, so + # either way the turn ends on it, as a continuation that errored does. + logger.warning( + "empty-recovery: the continuation of a cut answer raised; ending the turn on the " + "{} chars that streamed: {}", + sum(len(part) for part in recovery.parts), + exc, + ) + final_content = await _end_on_the_partial(response) + break # Read inside the bind: the count belongs to it and ends with it. recorded_at_seam = usage_record.recorded_inbound() if unstreamed and response.finish_reason != "error": diff --git a/tests/test_agent_loop_broken_turn.py b/tests/test_agent_loop_broken_turn.py index 6aaa434ad..0a0966af8 100644 --- a/tests/test_agent_loop_broken_turn.py +++ b/tests/test_agent_loop_broken_turn.py @@ -21,7 +21,7 @@ from raven.agent.hook import AgentHook, HookDecision from raven.agent.loop import AgentLoop from raven.agent.loop.bundles import HostWiring, ToolWiring, TurnPolicy -from raven.agent.loop.recovery import EMPTY_REPLY_FAILURE, RecoveryLimits +from raven.agent.loop.recovery import EMPTY_REPLY_FAILURE, PARTIAL_STREAM_NOTE, RecoveryLimits from raven.contracts.llm_provider import CallRecord from raven.providers.base import ErrorClassification, LLMProvider, LLMResponse, ToolCallRequest from raven.spine.message import ChatType, Source @@ -256,25 +256,38 @@ class _Refused(Exception): @pytest.mark.asyncio -@pytest.mark.parametrize( - ("death", "status"), [(_Refused("invalid request"), "failed"), (asyncio.CancelledError(), "cancelled")] -) -async def test_the_words_of_a_cut_answer_are_filed_when_its_continuation_dies_unheard(workspace, death, status): +async def test_the_words_of_a_cut_answer_are_filed_when_its_continuation_is_cancelled_unheard(workspace): """The cut answer's words sit in the loop's list as a recovery row that is never - persisted, and a continuation that dies before saying anything leaves the reader's + persisted, and a continuation cancelled before saying anything leaves the reader's words as exactly that row's text. The rescue must not take them for committed, or the answer the reader watched is gone on the next reload.""" - agent = _agent(workspace, _CutThenDies(death)) + agent = _agent(workspace, _CutThenDies(asyncio.CancelledError())) async def _sink(_text: str) -> None: return None - with pytest.raises(type(death)): + with pytest.raises(asyncio.CancelledError): await agent._process_message(_make_msg("hello"), on_token_delta=_sink) msgs = _persisted(workspace) assert [(m["role"], m["content"]) for m in msgs[:-1]] == [("user", "hello"), ("assistant", "half an ")] - assert msgs[-1]["turn_ended"]["status"] == status + assert msgs[-1]["turn_ended"]["status"] == "cancelled" + + +@pytest.mark.asyncio +async def test_a_cut_answer_whose_continuation_is_refused_ends_on_its_words(workspace): + """A continuation refused outright leaves the reader holding the answer so far, so the turn ends on + it, said to be partial, and files it as the turn's reply rather than failing.""" + agent = _agent(workspace, _CutThenDies(_Refused("invalid request"))) + + async def _sink(_text: str) -> None: + return None + + out = await agent._process_message(_make_msg("hello"), on_token_delta=_sink) + + msgs = _persisted(workspace) + assert out is not None and out[0] == "half an\n\n" + PARTIAL_STREAM_NOTE + assert [(m["role"], m["content"]) for m in msgs] == [("user", "hello"), ("assistant", out[0])] # --- the other way a turn dies: an error response the ladder could not wait out --- diff --git a/tests/test_agent_loop_empty_recovery.py b/tests/test_agent_loop_empty_recovery.py index 945fed800..ee525ead8 100644 --- a/tests/test_agent_loop_empty_recovery.py +++ b/tests/test_agent_loop_empty_recovery.py @@ -2427,6 +2427,82 @@ async def test_a_continuation_that_errors_outright_delivers_what_streamed_with_t assert "".join(sink.text) == final, "the note reached the reader once, after the words" +class _ContinuationRaises(LLMProvider): + """Streams its words and dies; every call asked to continue them then raises, as + ``LiteLLMProvider.chat_stream`` does with an HTTP error, instead of answering with an error.""" + + def __init__(self) -> None: + super().__init__(api_key="test") + self.calls = 0 + + async def chat(self, *args, **kwargs): # pragma: no cover - the streaming path is under test + raise AssertionError("the streaming path is the one under test") + + async def chat_stream(self, messages, tools=None, model=None, **_kw): + self.calls += 1 + if self.calls == 1: + yield ChatDelta(content="The answer is 42.") + raise ConnectionError("connection reset by peer") + raise RuntimeError("the gateway refused the request") + yield # pragma: no cover - makes this an async generator + + def get_default_model(self) -> str: + return "stub" + + +@pytest.mark.asyncio +async def test_a_continuation_that_raises_delivers_what_streamed_with_the_note(workspace, monkeypatch): + """A continuation can fail by raising rather than by answering with an error; the reader holds the + answer so far either way, so either way the turn ends on it, said to be partial, not as a failure.""" + provider = _ContinuationRaises() + agent = _make_agent(workspace, provider) + _recorded_sleeps(monkeypatch) + sink = _Sink() + + final, _used, messages, outcome = await agent._run_agent_loop(_seed(), on_token_delta=sink.delta) + + note = "\n\n" + PARTIAL_STREAM_NOTE + assert final == "The answer is 42." + note + assert outcome.status == "interrupted" and outcome.error is None + assert "".join(sink.text) == final + assert (messages[-1]["role"], messages[-1]["content"]) == ("assistant", final) + + +class _ToolThenRaises(LLMProvider): + """Calls a tool, then raises on the call after it, with no cut answer behind that call.""" + + def __init__(self) -> None: + super().__init__(api_key="test") + self.calls = 0 + + async def chat(self, *args, **kwargs): # pragma: no cover - the streaming path is under test + raise AssertionError("the streaming path is the one under test") + + async def chat_stream(self, messages, tools=None, model=None, **_kw): + self.calls += 1 + if self.calls == 1: + call = {"index": 0, "id": "t1", "function": {"name": "no_such_tool", "arguments": "{}"}} + yield ChatDelta(content=None, tool_call_delta={"tool_calls": [call]}) + yield ChatDelta(content=None, finish_reason="tool_calls") + return + raise RuntimeError("the gateway refused the request") + yield # pragma: no cover - makes this an async generator + + def get_default_model(self) -> str: + return "stub" + + +@pytest.mark.asyncio +async def test_a_call_that_raises_with_no_cut_behind_it_still_fails_the_turn(workspace, monkeypatch): + """The control: only a continuation ends on what streamed. A call with no cut answer behind it has + nothing to end on, so its error is the turn's, as it always was.""" + agent = _make_agent(workspace, _ToolThenRaises()) + _recorded_sleeps(monkeypatch) + + with pytest.raises(RuntimeError, match="the gateway refused the request"): + await agent._run_agent_loop(_seed(), on_token_delta=_Sink().delta) + + @pytest.mark.asyncio async def test_an_error_before_any_cut_still_waits_and_fails_as_it_did(workspace, monkeypatch): """The control: with no cut answer behind it, the same error is the turn's to wait out.""" From 8b1112c2c035c8dea6d0660a7ac2d74836020641 Mon Sep 17 00:00:00 2001 From: Dizhan Xue Date: Thu, 29 Oct 2026 06:42:43 +0000 Subject: [PATCH 05/13] fix(agent): read a held note's marker continuation as one reply On a proactive turn the silence gate holds the whole call, so a cut partial was never shown. A continuation that was the marker alone was still dropped as one that adds nothing, which left the held note to be delivered as the turn's answer. That drop rests on the reader already holding what streamed, which is true only of a turn that showed it. Where it did not, the marker is read with what came before it, on its own line, as the same reply uncut would be. --- CONTEXT.md | 14 +++--- raven/agent/loop/turn_path.py | 8 ++++ tests/test_agent_loop_empty_recovery.py | 63 +++++++++++++++++++++---- 3 files changed, 70 insertions(+), 15 deletions(-) diff --git a/CONTEXT.md b/CONTEXT.md index 241b86e49..5c89f2549 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -895,12 +895,14 @@ continued rather than taken for silence, and a stream that had already received keeps it, so the refusal is handled as one rather than continued. The model is asked to go on, and the parts are joined, up to `MAX_CONTINUATIONS` times, after which the partial is delivered with `PARTIAL_STREAM_NOTE` as an `interrupted` turn. A -continuation that comes back empty leaves what streamed as the answer, and so does one that is a -marker on its own, unless the cut left a think block open, when it is read only joined to what came -before it; one that comes back as an -error, a stream cut off before any new text included, ends the turn at once on the partial with -the note, as `interrupted`, with no wait and no fallback model, and so does one that raises once the -stream's own reconnects are spent -- and so does a turn that runs out +continuation that comes back empty leaves what streamed as the answer, and on a person's turn, which +showed what streamed, so does one that is a marker on its own; a turn from a proactive origin showed +nothing of it, so there such a marker is read with what came before it, on its own line, as the +reply uncut would be. Either way a continuation inside a think block the cut left open is read only +joined to what came before it. One that comes back as an error, a stream cut off before any new text +included, ends the turn at once on the partial with the note, as `interrupted`, with no wait and no +fallback model, and so does one that raises once the stream's own reconnects are spent -- and so +does a turn that runs out of iterations or time while a continuation is still owed, with no wrap-up. On each of those exits the reply the turn ends on is the joined parts with their closed think blocks taken out of the whole -- a cut can split one -- and a block still unclosed left in as text, read for a Silence Marker by diff --git a/raven/agent/loop/turn_path.py b/raven/agent/loop/turn_path.py index c816f62ed..beab05e79 100644 --- a/raven/agent/loop/turn_path.py +++ b/raven/agent/loop/turn_path.py @@ -2512,6 +2512,14 @@ async def _end_on_the_partial(last: LLMResponse) -> str: # no longer be continued -- the joined text is what may be the marker. if recovery.parts or kind is ReplyKind.NETWORK_CUT: ends_silent = not reply_awaited and is_silence(clean, origin) + if own_marker and not ends_silent and not reply_awaited: + # Dropping a continuation that is only the marker rests on the reader already + # holding what streamed, which a turn holding its whole call never showed. So + # the marker is read with what came before it, on its own line, as the same + # reply uncut would be; only the loose rule can find it there. + with_marker = clean + "\n" + (response.content or "").strip() + if is_silence(with_marker, origin): + clean, ends_silent = with_marker, True else: ends_silent = action is RecoveryAction.SILENT dropped_marker = own_marker and not ends_silent diff --git a/tests/test_agent_loop_empty_recovery.py b/tests/test_agent_loop_empty_recovery.py index ee525ead8..b08f8fabc 100644 --- a/tests/test_agent_loop_empty_recovery.py +++ b/tests/test_agent_loop_empty_recovery.py @@ -2614,6 +2614,38 @@ async def _ends_on(agent: AgentLoop, origin: Origin, **kwargs): return final, outcome, messages[-1], sink.text +@pytest.mark.asyncio +@pytest.mark.parametrize( + "origin,continuation,silent,shown", + [ + pytest.param(Origin.CRON, "[SILENT]", True, "", id="proactive_note_then_marker"), + pytest.param(Origin.CRON, "nothing new\n\n[SILENT]", True, "", id="proactive_note_then_a_noted_marker"), + pytest.param( + Origin.CRON, + " Two files changed.", + False, + "Here is the update: Two files changed.", + id="proactive_note_then_more", + ), + pytest.param(Origin.USER, "[SILENT]", False, "Here is the update:", id="person_note_then_marker"), + ], +) +async def test_a_marker_continuing_a_held_note_is_read_with_it( + workspace, monkeypatch, origin, continuation, silent, shown +): + """A proactive turn holds its whole call, so nothing of a cut note was shown, and a continuation that + is only the marker is read together with the note, as the same reply uncut ("note", then the marker on + its own line) is: silent. A person's turn saw the note already, and only a whole ``[SILENT]`` is silence + there, so the note stays its answer.""" + _served(monkeypatch, [_DyingStream("Here is the update:"), _finished_stream(continuation)]) + + final, outcome, last, streamed = await _ends_on(_litellm_agent(workspace), origin) + + assert outcome.silent is silent + assert "".join(streamed).replace("\n\n" + PARTIAL_STREAM_NOTE, "") == shown + assert bool(last.get("_silent")) is silent + + @pytest.mark.asyncio @pytest.mark.parametrize( "origin,cuts,continuation", @@ -2694,13 +2726,25 @@ async def test_a_no_reply_cut_that_goes_on_to_a_summary_is_delivered_whole(works @pytest.mark.asyncio -@pytest.mark.parametrize("origin", [Origin.USER, Origin.CRON]) -async def test_a_marker_after_a_held_cut_shows_what_streamed_and_drops_the_marker(workspace, monkeypatch, origin): - """A continuation that is a marker adds nothing, so what streamed is the answer: a start that was held - across the cut goes out, and the marker after it does not.""" +async def test_a_proactive_marker_cut_and_started_over_ends_silent(workspace, monkeypatch): + """A proactive turn cut inside its marker, whose continuation starts the marker over, said nothing: + the fragment and the marker are read together, and neither reaches the reader.""" _served(monkeypatch, [_DyingStream("[SIL"), _finished_stream("[SILENT]")]) - final, outcome, last, streamed = await _ends_on(_litellm_agent(workspace), origin) + final, outcome, last, streamed = await _ends_on(_litellm_agent(workspace), Origin.CRON) + + assert outcome.silent is True and last.get("_silent") is True + assert streamed == [] + + +@pytest.mark.asyncio +async def test_a_marker_after_a_held_cut_shows_what_streamed_and_drops_the_marker(workspace, monkeypatch): + """On a person's turn a continuation that is a marker adds nothing, so what streamed is the answer: a + start that was held across the cut goes out, and the marker after it does not. A proactive turn reads + the two together instead (``test_a_marker_continuing_a_held_note_is_read_with_it``).""" + _served(monkeypatch, [_DyingStream("[SIL"), _finished_stream("[SILENT]")]) + + final, outcome, last, streamed = await _ends_on(_litellm_agent(workspace), Origin.USER) assert final == "[SIL" and outcome.silent is False assert "_silent" not in last @@ -4768,7 +4812,7 @@ async def test_a_stream_cut_right_after_a_marker_is_continued_not_ended_silent(w @pytest.mark.asyncio -@pytest.mark.parametrize("origin", [Origin.USER, Origin.CRON]) +@pytest.mark.parametrize("origin", [Origin.USER]) @pytest.mark.parametrize( "continuation", [ @@ -4779,9 +4823,10 @@ async def test_a_stream_cut_right_after_a_marker_is_continued_not_ended_silent(w async def test_a_continuation_that_is_a_marker_ends_the_turn_as_an_empty_one_does( workspace, monkeypatch, origin, continuation ): - """After a cut answer the reader holds, a continuation that is a marker under the turn's rule has - nothing to add, as an empty one has not: what streamed is the answer, with the same row, status - and stream as the empty continuation leaves.""" + """After a cut answer the reader holds -- a person's turn, which shows what streams -- a continuation + that is a marker under the turn's rule has nothing to add, as an empty one has not: what streamed is + the answer, with the same row, status and stream as the empty continuation leaves. A proactive turn + showed nothing and reads the two together (``test_a_marker_continuing_a_held_note_is_read_with_it``).""" async def run(second): _served(monkeypatch, [_DyingStream("Hello "), second]) From 39fca077fc3b7ea4e641904770df851a9d6fea79 Mon Sep 17 00:00:00 2001 From: Dizhan Xue Date: Thu, 29 Oct 2026 06:42:44 +0000 Subject: [PATCH 06/13] fix(agent): count every send and file delivery as the turn's answer MessageTool counted a send as answering the turn only when it named the turn's own channel and chat. The send callback never sees a target, though: it hands every message to the turn's own reader. A send that named another chat reached the reader and was still read as no answer, and a silent runtime turn whose reply was a file delivery looked unanswered too, so the session list and the ACP replay took back turns whose reply the reader had seen. sent_in_turn now reports any send that went out, and a turn whose tools delivered files is stamped replied_via_tool as well. A send that failed still answers nobody. The wire description of replied_via_tool says so, and the generated clients are regenerated. --- CONTEXT.md | 12 +++--- raven/agent/loop/turn_path.py | 12 +++++- raven/agent/tools/message.py | 9 +++-- raven/rpc/models.py | 5 ++- raven/session/turn_shape.py | 18 ++++----- rpc-schema/openrpc.json | 2 +- tests/test_acp_replay.py | 7 ++-- tests/test_agent_loop_empty_recovery.py | 51 +++++++++++++++++++++++-- tests/test_session_manager.py | 14 ++++--- tests/test_session_turn_shape.py | 14 ++----- ui-tui/src/rpc/generated.ts | 2 +- ui-web/src/rpc/generated.ts | 2 +- 12 files changed, 102 insertions(+), 46 deletions(-) diff --git a/CONTEXT.md b/CONTEXT.md index 5c89f2549..6c10f604b 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -852,19 +852,21 @@ The `after_send` hook chain still runs, with queued for the next reply, and the personalizer learns nothing; text a hook composes there is delivered as the reply, the closing row is filed as that text rather than the marker, and the turn is then not silent. A turn the message tool has already answered (`MessageTool.sent_in_turn`: a send -to the turn's own chat that went out) is not a Silent Turn whatever it ends on -- its answer reached +that went out, which the tool hands to the turn's own reader whatever chat the call names) is not a +Silent Turn whatever it ends on -- its answer reached the reader, so no `SILENT_TURN` notice goes out; a marker it ends on is still filed `silent: true`, so a reload draws the muted line after the delivered reply where the live view drew none. The turn's last assistant row with text or tool calls -- the marker's row when it ends on one, else, after an empty closing reply, which is not stored, its last tool call, not necessarily the send; never a notice the -runtime files after the turn -- carries that live verdict as `replied_via_tool`, and the two +runtime files after the turn -- carries that live verdict, or that a tool delivered files the reader +now holds, as `replied_via_tool`, and the two surfaces that read a stored turn -- the ACP replay, which draws it, and the session list, which orders sessions by it -- read the flag from one place (`session/turn_shape.py`) rather than guess it from the calls they can see, where a send -that failed or went to another chat would look like an answer. A Silent Turn the runtime opened, -which those two surfaces take back whole, files no unanswered-question notice after its marker, so -none is left behind as an orphan; a person's Silent Turn, which they keep, still files it. +that failed would look like an answer. A Silent Turn the runtime opened, which those two surfaces +take back whole, files no unanswered-question notice after its marker, so none is left behind as an +orphan; a person's Silent Turn, which they keep, still files it. Distinct from a `no_output` failure. A Silent Turn is the model saying nothing on purpose, so it succeeds; `no_output` is what a turn is failed with when its last reply was empty -- no text, no tool call, no reasoning -- once every retry was spent, or, with recovery switched off, when its diff --git a/raven/agent/loop/turn_path.py b/raven/agent/loop/turn_path.py index beab05e79..3f53f44ab 100644 --- a/raven/agent/loop/turn_path.py +++ b/raven/agent/loop/turn_path.py @@ -466,6 +466,16 @@ def _file_unanswered(messages: list[dict]) -> None: messages.append({"role": "assistant", "content": text, _NOTICE_KEY: notice}) +def _delivered_files(turn_rows: list[dict]) -> bool: + """Whether a tool handed the reader files this turn: a call that left a delivery manifest with files.""" + for row in turn_rows: + metadata = row.get(_TOOL_METADATA_KEY) + delivery = metadata.get("raven_delivery") if isinstance(metadata, dict) else None + if isinstance(delivery, dict) and delivery.get("files"): + return True + return False + + def _taken_back_whole(messages: list[dict], skip: int) -> bool: """Whether the session list and the ACP replay will take this turn back whole. @@ -3481,7 +3491,7 @@ def _rerun_pending(final_content: str | None, msgs: list[dict], status: str) -> # A hook composed the reply the reader is sent in place of the marker. A turn the # message tool answered sends no reply of its own, so its marker row stays. _file_composed_reply(turn_msgs, final_content) - if sent_by_tool: + if sent_by_tool or _delivered_files(turn_msgs[turn_start_idx:]): _stamp_replied_via_tool(turn_msgs, turn_start_idx) if len(self.hooks) > 0: _stamp_turn_observers(turn_msgs, turn_hook_meta, turn_start_idx) diff --git a/raven/agent/tools/message.py b/raven/agent/tools/message.py index 6a7dd050e..6fd15fbcd 100644 --- a/raven/agent/tools/message.py +++ b/raven/agent/tools/message.py @@ -70,7 +70,11 @@ def start_turn(self) -> None: @property def sent_in_turn(self) -> bool: - """Whether a reply was sent to the turn's own target this turn.""" + """Whether a send went out this turn. + + The send callback hands every message to the turn's own reader, whatever + channel or chat the call names, so a send that went out reached the reader. + """ return self._cur().sent @property @@ -119,8 +123,7 @@ async def execute( try: await st.send_callback(content, media or []) - if channel == st.channel and chat_id == st.chat_id: - self._turn.set(replace(st, sent=True)) + self._turn.set(replace(st, sent=True)) media_info = f" with {len(media)} attachments" if media else "" return f"Message sent to {channel}:{chat_id}{media_info}" except Exception as e: diff --git a/raven/rpc/models.py b/raven/rpc/models.py index b6b1841d5..d72a42ecb 100644 --- a/raven/rpc/models.py +++ b/raven/rpc/models.py @@ -4008,8 +4008,9 @@ class TranscriptMessage(_Strict): replied_via_tool: bool | None = Field( default=None, description=( - "Set on one assistant entry of a turn whose reply the message tool sent to the turn's own chat: " - "the live turn's verdict, so a send that failed or went to another chat does not set it. The entry " + "Set on one assistant entry of a turn whose reader got its reply from a tool: a message-tool send " + "that went out (the tool hands every send to the turn's own reader, whatever chat the call names) or a " + "file delivery. A send that failed does not set it. The entry " "is the turn's last assistant entry with text or tool calls, not counting a notice the runtime files " "after the turn: the `silent` marker's entry when the turn ends on one, and otherwise, when its last " "reply had no text, the last entry that made a tool call, which need not be the call that sent the " diff --git a/raven/session/turn_shape.py b/raven/session/turn_shape.py index c7d9b3bac..2a3e53be7 100644 --- a/raven/session/turn_shape.py +++ b/raven/session/turn_shape.py @@ -7,14 +7,14 @@ rather than in either surface: a rule the two copies do not share is a rule that drifts, and the drift shows as one surface drawing a turn the other hides. -Whether the message tool answered is the live turn's verdict -(``MessageTool.sent_in_turn``: a send to the turn's own chat that went out), filed -as ``replied_via_tool`` on the turn's last assistant row with text or tool calls -- the -marker's row when the turn ends on one, which is the only row a reader asks. A reader does not work it -out from the calls it can see: a call that failed, or that sent to another chat, -answered nobody. The flag has one name in both shapes a transcript comes in -- the -stored row and the flattened row the wire carries -- so a caller hands over -whichever it holds. +Whether a tool answered is the live turn's verdict -- ``MessageTool.sent_in_turn``, a +send that went out (the tool hands every send to the turn's own reader, whatever chat +the call names), or a file delivery -- filed as ``replied_via_tool`` on the turn's last +assistant row with text or tool calls: the marker's row when the turn ends on one, +which is the only row a reader asks. A reader does not work it out from the calls it +can see: a call that failed answered nobody. The flag has one name in both shapes a +transcript comes in -- the stored row and the flattened row the wire carries -- so a +caller hands over whichever it holds. """ from __future__ import annotations @@ -27,7 +27,7 @@ def replied_via_message_tool(row: Mapping[str, Any]) -> bool: - """Whether this row carries the verdict that the message tool sent its turn's reply to the turn's own chat. + """Whether this row carries the verdict that a tool put the turn's reply in front of its reader. Such a turn has answered, so whatever the model ends it on -- a marker, a cut, nothing at all -- the reader has it, and the turn is not a Silent Turn. diff --git a/rpc-schema/openrpc.json b/rpc-schema/openrpc.json index cbed12fc2..8ba315b1b 100644 --- a/rpc-schema/openrpc.json +++ b/rpc-schema/openrpc.json @@ -12487,7 +12487,7 @@ "type": "boolean" }, "replied_via_tool": { - "description": "Set on one assistant entry of a turn whose reply the message tool sent to the turn's own chat: the live turn's verdict, so a send that failed or went to another chat does not set it. The entry is the turn's last assistant entry with text or tool calls, not counting a notice the runtime files after the turn: the `silent` marker's entry when the turn ends on one, and otherwise, when its last reply had no text, the last entry that made a tool call, which need not be the call that sent the message. Such a turn answered whatever it ends on, and is not a Silent Turn.", + "description": "Set on one assistant entry of a turn whose reader got its reply from a tool: a message-tool send that went out (the tool hands every send to the turn's own reader, whatever chat the call names) or a file delivery. A send that failed does not set it. The entry is the turn's last assistant entry with text or tool calls, not counting a notice the runtime files after the turn: the `silent` marker's entry when the turn ends on one, and otherwise, when its last reply had no text, the last entry that made a tool call, which need not be the call that sent the message. Such a turn answered whatever it ends on, and is not a Silent Turn.", "type": "boolean" } }, diff --git a/tests/test_acp_replay.py b/tests/test_acp_replay.py index 681f22839..1a8e57404 100644 --- a/tests/test_acp_replay.py +++ b/tests/test_acp_replay.py @@ -422,13 +422,12 @@ def test_a_runtime_turn_the_message_tool_answered_still_replays(self): "arguments,result", [ pytest.param({}, "Error sending message: the channel is down", id="a_send_that_failed"), - pytest.param({"chat_id": "c2"}, "Message sent to acp:c2", id="sent_to_another_chat"), ], ) def test_a_runtime_turn_whose_message_call_answered_nobody_replays_as_nothing(self, arguments, result): - """A message call is not an answer by being made: the live turn counted it only when it - sent to the turn's own chat, and filed that verdict on the row that closes the turn. A - turn that closes on the marker without it is a Silent Turn, taken out whole.""" + """A message call is not an answer by being made: the live turn counted it only when the send + went out, and filed that verdict on the row that closes the turn. A turn that closes on the + marker without it is a Silent Turn, taken out whole.""" said = json.dumps({"content": "The build finished with 3 failing tests.", **arguments}) calls = [{"id": "m1", "type": "function", "function": {"name": "message", "arguments": said}}] stored = [ diff --git a/tests/test_agent_loop_empty_recovery.py b/tests/test_agent_loop_empty_recovery.py index b08f8fabc..48dc5a548 100644 --- a/tests/test_agent_loop_empty_recovery.py +++ b/tests/test_agent_loop_empty_recovery.py @@ -65,6 +65,7 @@ from raven.agent.window import shrink from raven.config.schema import CompactionConfig from raven.contracts.llm_provider import REASONING_EFFORT_LADDER +from raven.contracts.tool import Tool from raven.providers import rates from raven.providers.base import ( ChatDelta, @@ -5182,14 +5183,14 @@ async def test_a_proactive_turn_that_skips_after_send_stays_silent(workspace, mo [ pytest.param(_SEND, False, True, id="sent_to_its_own_chat"), pytest.param(_SEND, True, False, id="a_send_that_failed"), - pytest.param('{"content": "here is your answer", "chat_id": "c2"}', False, False, id="sent_to_another_chat"), + pytest.param('{"content": "here is your answer", "chat_id": "c2"}', False, True, id="sent_naming_another_chat"), ], ) async def test_a_stored_turn_reads_the_message_tools_live_verdict(workspace, monkeypatch, arguments, fails, answered): """A turn the runtime opened calls the message tool and closes on the marker. The live turn counts it - answered only when the tool sent to the turn's own chat (``MessageTool.sent_in_turn``); the session - list and an ACP replay read the stored turn by that same verdict, so a send that failed or went to - another chat leaves a Silent Turn there too, and is taken back whole.""" + answered when the send went out (``MessageTool.sent_in_turn``) -- the tool hands every send to the + turn's own reader, whatever chat the call names -- and the session list and an ACP replay read the + stored turn by that same verdict. Only a send that failed leaves a Silent Turn there, taken back whole.""" from raven.acp.replay import replay from raven.rpc.methods.session import _map_to_wire @@ -5218,6 +5219,48 @@ async def unreachable(content: str, media: list[str]) -> None: assert bool(listed["last_message_at"]) is answered, "and does not move the session list" +class _DeliversAFile(Tool): + """Hands the reader one file, leaving its manifest for the turn stream as ``deliver_files`` does.""" + + @property + def name(self) -> str: + return "deliver_files" + + @property + def description(self) -> str: + return "delivers files" + + @property + def parameters(self) -> dict: + return {"type": "object", "properties": {}} + + async def execute(self, **_) -> str: + return "Delivered report.txt (1 KB)" + + def take_metadata(self) -> dict: + return {"raven_delivery": {"message": "", "files": [{"name": "report.txt", "size": 1024}], "invalid": []}} + + +@pytest.mark.asyncio +async def test_a_silent_runtime_turn_that_delivered_files_is_not_taken_back(workspace, monkeypatch): + """Files a turn delivered are in front of the reader, as a message the tool sent is: a turn that hands + over a file and closes on the marker is kept by the session list and the ACP replay.""" + from raven.acp.replay import replay + from raven.rpc.methods.session import _map_to_wire + + _served(monkeypatch, [_tool_reply("deliver_files", "{}"), _reply("[SILENT]")]) + agent = _litellm_agent(workspace) + agent.tools.register(_DeliversAFile()) + + await agent._process_message(_turn("check the build", Origin.CRON), session_key="cron:job", origin=Origin.CRON) + + rows = agent.sessions.get_or_create("cron:job").messages + assert rows[-1]["silent"] is True and rows[-1].get("replied_via_tool") is True + assert replay(_map_to_wire(rows, "cron:job"), session_id="cron:job"), "the turn replays" + [listed] = agent.sessions.list_sessions() + assert listed["last_message_at"], "and stands in the session list" + + @pytest.mark.asyncio async def test_a_turn_the_message_tool_answered_with_no_reply_of_its_own_files_the_verdict(workspace, monkeypatch): """The commonest shape: the tool sends, and the model then says nothing. The empty closing row is not diff --git a/tests/test_session_manager.py b/tests/test_session_manager.py index af0816b9e..c6f7b1e9a 100644 --- a/tests/test_session_manager.py +++ b/tests/test_session_manager.py @@ -1024,18 +1024,22 @@ def test_a_silent_runtime_turn_is_taken_back_after_a_turn_the_message_tool_answe def test_a_silent_runtime_turn_whose_message_call_answered_nobody_is_taken_back(tmp_path: Path): - """A message call is not an answer by being made. The live turn counted it only when it sent to - the turn's own chat and filed that verdict on the closing row; without it, a call that failed or - went to another chat leaves a Silent Turn, taken back whole.""" + """A message call is not an answer by being made. The live turn counted it only when the send went + out and filed that verdict on the closing row; without it, a call that failed leaves a Silent Turn, + taken back whole.""" mgr = SessionManager(tmp_path) - calls = [{"id": "m1", "type": "function", "function": {"name": "message", "arguments": '{"chat_id": "c2"}'}}] + calls = [{"id": "m1", "type": "function", "function": {"name": "message", "arguments": '{"content": "x"}'}}] session = mgr.get_or_create("cron:job02") session.add_message("user", "anything new?", timestamp="2026-06-10T10:00:00") session.add_message("assistant", "still compiling", timestamp="2026-06-10T10:00:05") session.add_message("user", "[Scheduled Task] check the build", timestamp="2026-06-10T11:00:00", origin="cron") session.add_message("assistant", "", timestamp="2026-06-10T11:00:02", tool_calls=calls) session.add_message( - "tool", "Message sent to cron:c2", timestamp="2026-06-10T11:00:03", tool_call_id="m1", name="message" + "tool", + "Error sending message: the channel is down", + timestamp="2026-06-10T11:00:03", + tool_call_id="m1", + name="message", ) session.add_message("assistant", "[SILENT]", timestamp="2026-06-10T11:00:09", silent=True) mgr.save(session) diff --git a/tests/test_session_turn_shape.py b/tests/test_session_turn_shape.py index 32d297463..3206c77df 100644 --- a/tests/test_session_turn_shape.py +++ b/tests/test_session_turn_shape.py @@ -4,18 +4,18 @@ from raven.session.turn_shape import replied_via_message_tool -def _message_call(chat_id: str | None = None) -> dict: - arguments = '{"content": "3 tests failed"' + (f', "chat_id": "{chat_id}"' if chat_id else "") + "}" +def _message_call() -> dict: + arguments = '{"content": "3 tests failed"}' return {"id": "m1", "type": "function", "function": {"name": "message", "arguments": arguments}} -def _turn(*, answered: bool, chat_id: str | None = None, sent: str = "Message sent to cron:job") -> list[dict]: +def _turn(*, answered: bool, sent: str = "Message sent to cron:job") -> list[dict]: closing = {"role": "assistant", "content": "[SILENT]", "silent": True} if answered: closing["replied_via_tool"] = True return [ {"role": "user", "content": "[Scheduled Task] check the build", "origin": "cron"}, - {"role": "assistant", "content": "", "tool_calls": [_message_call(chat_id)]}, + {"role": "assistant", "content": "", "tool_calls": [_message_call()]}, {"role": "tool", "content": sent, "tool_call_id": "m1", "name": "message"}, closing, ] @@ -35,12 +35,6 @@ def test_a_message_call_that_failed_did_not_answer_in_either_shape(): assert not any(replied_via_message_tool(row) for row in _map_to_wire(stored, "cron:job")) -def test_a_message_call_to_another_chat_did_not_answer_in_either_shape(): - stored = _turn(answered=False, chat_id="c2", sent="Message sent to cron:c2") - assert not any(replied_via_message_tool(row) for row in stored) - assert not any(replied_via_message_tool(row) for row in _map_to_wire(stored, "cron:job")) - - def test_only_an_assistant_row_carries_the_verdict(): assert not replied_via_message_tool({"role": "user", "content": "hi", "replied_via_tool": True}) assert not replied_via_message_tool({"role": "assistant", "content": "[SILENT]", "replied_via_tool": "yes"}) diff --git a/ui-tui/src/rpc/generated.ts b/ui-tui/src/rpc/generated.ts index 2c512f948..f62be5a73 100644 --- a/ui-tui/src/rpc/generated.ts +++ b/ui-tui/src/rpc/generated.ts @@ -304,7 +304,7 @@ export interface TranscriptMessage { */ silent?: boolean; /** - * Set on one assistant entry of a turn whose reply the message tool sent to the turn's own chat: the live turn's verdict, so a send that failed or went to another chat does not set it. The entry is the turn's last assistant entry with text or tool calls, not counting a notice the runtime files after the turn: the `silent` marker's entry when the turn ends on one, and otherwise, when its last reply had no text, the last entry that made a tool call, which need not be the call that sent the message. Such a turn answered whatever it ends on, and is not a Silent Turn. + * Set on one assistant entry of a turn whose reader got its reply from a tool: a message-tool send that went out (the tool hands every send to the turn's own reader, whatever chat the call names) or a file delivery. A send that failed does not set it. The entry is the turn's last assistant entry with text or tool calls, not counting a notice the runtime files after the turn: the `silent` marker's entry when the turn ends on one, and otherwise, when its last reply had no text, the last entry that made a tool call, which need not be the call that sent the message. Such a turn answered whatever it ends on, and is not a Silent Turn. */ replied_via_tool?: boolean; } diff --git a/ui-web/src/rpc/generated.ts b/ui-web/src/rpc/generated.ts index ee0a9d89a..a76d3f49a 100644 --- a/ui-web/src/rpc/generated.ts +++ b/ui-web/src/rpc/generated.ts @@ -255,7 +255,7 @@ export interface TranscriptMessage { */ silent?: boolean; /** - * Set on one assistant entry of a turn whose reply the message tool sent to the turn's own chat: the live turn's verdict, so a send that failed or went to another chat does not set it. The entry is the turn's last assistant entry with text or tool calls, not counting a notice the runtime files after the turn: the `silent` marker's entry when the turn ends on one, and otherwise, when its last reply had no text, the last entry that made a tool call, which need not be the call that sent the message. Such a turn answered whatever it ends on, and is not a Silent Turn. + * Set on one assistant entry of a turn whose reader got its reply from a tool: a message-tool send that went out (the tool hands every send to the turn's own reader, whatever chat the call names) or a file delivery. A send that failed does not set it. The entry is the turn's last assistant entry with text or tool calls, not counting a notice the runtime files after the turn: the `silent` marker's entry when the turn ends on one, and otherwise, when its last reply had no text, the last entry that made a tool call, which need not be the call that sent the message. Such a turn answered whatever it ends on, and is not a Silent Turn. */ replied_via_tool?: boolean; } From c4b1cbd3db9092365ee9109db54f930543ccc273 Mon Sep 17 00:00:00 2001 From: Dizhan Xue Date: Thu, 29 Oct 2026 06:42:44 +0000 Subject: [PATCH 07/13] fix(agent): read the loose silence rule only on a turn taught it Every proactive turn read the loose marker rule, but only the cron reminder and a sub-agent's unprompted report teach the model the marker. A heartbeat task, a relayed sub-agent result and a sentinel notice never name it, so a reply that was, or ended on, "No reply" or "Silent" was swallowed there. The rule is now picked by the origin and by whether the prompt the model read carries SILENCE_INSTRUCTION (teaches_silence). A taught proactive turn reads the loose rule and holds its whole call; every other turn honours only the bracketed [SILENT] and streams as a person's does. The lesson is read off the prompt rather than a flag beside it, so an entry that teaches the marker cannot forget to say so. --- CONTEXT.md | 19 +-- docs-site/docs/proactivity-design.md | 10 +- docs-site/docs/proactivity-design.zh.md | 5 +- raven/agent/context/silence.py | 49 +++++--- raven/agent/loop/recovery.py | 17 +-- raven/agent/loop/turn_path.py | 24 ++-- raven/spine/turn.py | 9 +- tests/test_agent_context_silence.py | 55 +++++++-- tests/test_agent_loop_empty_recovery.py | 155 ++++++++++++++++++++++-- tests/test_core_cron_stack.py | 3 +- tests/test_subagent_manager.py | 24 +++- 11 files changed, 294 insertions(+), 76 deletions(-) diff --git a/CONTEXT.md b/CONTEXT.md index 6c10f604b..8e2d23a4d 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -811,11 +811,14 @@ _Avoid_: "shadow git" as the term — Checkpoint is the per-turn snapshot it pro **Silence Marker** (`agent/context/silence.py`): What a model answers with to say a turn has nothing to report: `[SILENT]`, `SILENT`, `NO_REPLY` or `NO REPLY` (`SILENCE_MARKERS`); `SILENCE_INSTRUCTION` is the two sentences that teach the -bracketed form. The turn's `Origin` picks the rule, in one place (`is_silence`). A `USER` turn, or -one with no origin, is never taught a marker, so it honours only `[SILENT]` itself with nothing but -whitespace around it: a one-word answer such as `Silent.` or `No reply.` is the person's answer, and -is delivered. A turn from a proactive origin -- `SENTINEL`, `CRON`, `HEARTBEAT` or `SUBAGENT` -- -reads the whole set by two rules: the strict one (`is_exact_silence`), a reply that is exactly a +bracketed form. The turn's `Origin`, and whether its prompt taught the marker, pick the rule, in one +place (`is_silence`). A turn whose prompt does not carry `SILENCE_INSTRUCTION` (`teaches_silence`) +honours only `[SILENT]` itself with nothing but whitespace around it: a `USER` turn, one with no +origin, and a proactive one that was never taught -- a heartbeat task, a sub-agent's relayed result, +a sentinel notice. There a one-word answer such as `Silent.` or `No reply.`, or a reply ending on +one, is the model's answer, and is delivered. A turn from a proactive origin (`PROACTIVE_ORIGINS`) +whose prompt carries it -- the cron reminder and a sub-agent's unprompted report -- +reads the whole set by two rules (`reads_loose_silence`): the strict one (`is_exact_silence`), a reply that is exactly a marker, with case, edge whitespace and stray edge punctuation forgiven up to `SILENCE_MAX_CHARS`; and the loose one (`is_loose_silence`): nobody is waiting on such a turn and models wrap the marker in a note on why they stayed quiet, so a marker alone on the first or last line, or a bracketed @@ -837,9 +840,9 @@ bracket goes out at once), reads past an inline think block the way the loop doe as it arrives, and what follows it is held; a closing tag with no block open is debris, as the loop reads it, and does not end the hold), keeps holding across a stream cut while the reply could still be the marker, and drops what it held when the loop reads the reply as -a marker (a turn from a proactive origin, whose marker may close a report, holds the +a marker (a turn that reads the loose rule, whose marker may close a report, holds the whole call instead). An exhausted turn's wrap-up is gated and read the same way. The marker row -stays in the history holding the model's whole reply -- the marker, or, from a proactive origin, +stays in the history holding the model's whole reply -- the marker, or, under the loose rule, the note around it -- filed with `silent: true` so a reload does not read it as an answer, and it carries the turn's Response Meta. The turn reports itself as `TurnOutcome.silent` and, for a surface drawing it live, as a closing `NoticeKind.SILENT_TURN` @@ -898,7 +901,7 @@ keeps it, so the refusal is handled as one rather than continued. The model is a the parts are joined, up to `MAX_CONTINUATIONS` times, after which the partial is delivered with `PARTIAL_STREAM_NOTE` as an `interrupted` turn. A continuation that comes back empty leaves what streamed as the answer, and on a person's turn, which -showed what streamed, so does one that is a marker on its own; a turn from a proactive origin showed +showed what streamed, so does one that is a marker on its own; a turn that reads the loose rule showed nothing of it, so there such a marker is read with what came before it, on its own line, as the reply uncut would be. Either way a continuation inside a think block the cut left open is read only joined to what came before it. One that comes back as an error, a stream cut off before any new text diff --git a/docs-site/docs/proactivity-design.md b/docs-site/docs/proactivity-design.md index 76e569503..fb204beae 100644 --- a/docs-site/docs/proactivity-design.md +++ b/docs-site/docs/proactivity-design.md @@ -297,10 +297,12 @@ while user work is busy. Wake drives Heartbeat, not Sentinel's tick loop. A successful Cron turn that explicitly replies uses the Spine's delivery path and does not enqueue a completion event or wake Heartbeat to repeat the -reminder. The reminder also teaches the `[SILENT]` marker. Cron is a -proactive origin, so a marker alone on the first or last line, or `[SILENT]` -opening the reply, counts too, and a note on why the model stayed quiet does -not defeat it. A run that ends this way is recorded as a success and delivers +reminder. The reminder also teaches the `[SILENT]` marker, and a proactive +turn whose prompt teaches it reads the marker loosely: a marker alone on the +first or last line, or `[SILENT]` opening the reply, counts too, and a note on +why the model stayed quiet does not defeat it. A proactive turn that was never +taught the marker (a heartbeat task, a relayed sub-agent result, a sentinel +notice) honours only an exact `[SILENT]`. A run that ends this way is recorded as a success and delivers nothing: it enqueues no event and wakes no Heartbeat. A run in which the model produced no output after every retry is recorded as failed, delivers nothing, and does not wake Heartbeat either. Every other failure still wakes diff --git a/docs-site/docs/proactivity-design.zh.md b/docs-site/docs/proactivity-design.zh.md index 75482403a..364eb2a9f 100644 --- a/docs-site/docs/proactivity-design.zh.md +++ b/docs-site/docs/proactivity-design.zh.md @@ -239,9 +239,10 @@ HeartbeatService（`schedulers/heartbeat/service.py`）通过结构化模型决 限制频率，并在用户工作繁忙时延后。Wake 驱动 Heartbeat，不驱动 Sentinel 的节拍循环。 已明确回复的成功 Cron 轮次通过 Spine 投递，不再生成完成事件或唤醒 Heartbeat 重复提醒。 -提醒文本还会教模型使用 `[SILENT]` 标记。Cron 属于主动来源（proactive origin）， +提醒文本还会教模型使用 `[SILENT]` 标记；prompt 教过标记的主动来源（proactive origin）轮次按宽松规则读： 标记单独占第一行或最后一行、或回复以 `[SILENT]` 开头，同样算沉默， -所以模型附带的一句说明不会让标记失效。以标记结束的运行记为成功、不投递： +所以模型附带的一句说明不会让标记失效。没有教过标记的主动轮次（heartbeat 任务、转述的子代理结果、 +sentinel 通知）只认完整的 `[SILENT]`。以标记结束的运行记为成功、不投递： 不生成事件，也不唤醒 Heartbeat。模型在所有重试后仍什么都没给的运行记为失败、不投递， 同样不唤醒 Heartbeat。其他失败、既没有回复也没有标记的成功轮次、错过的提醒以及自动禁用 仍会唤醒 Heartbeat。任务重试成功后会清除自身待处理的失败事件；明确回复或以 `[SILENT]` diff --git a/raven/agent/context/silence.py b/raven/agent/context/silence.py index 3c603c5bd..d8de21205 100644 --- a/raven/agent/context/silence.py +++ b/raven/agent/context/silence.py @@ -8,15 +8,17 @@ completed, and a marker never hides a failure; * a marker inside a report -- prose that merely mentions one is delivered like any other prose. -The rule depends on who the turn is for, and ``is_silence`` is the one place that picks it, so the -lanes cannot drift apart. A turn a person started is never taught a marker, so it honours only the -bracketed ``[SILENT]`` the runtime teaches, as the whole reply: a one-word answer such as ``Silent.`` -or ``No reply.`` is the person's answer. A turn from a proactive origin (cron, sub-agent, sentinel, -heartbeat) is told to answer ``[SILENT]`` when nothing is new, and models reliably vary the spelling -and wrap it in a short note on why they stayed quiet, so it reads the whole marker set by two rules: -the strict one (``is_exact_silence``), a reply that is exactly a marker, and the loose one -(``is_loose_silence``), where the bracketed ``[SILENT]`` opening the reply, or any marker alone on -its first or last line, counts too. +The rule depends on who the turn is for and whether its prompt taught the marker, and +``is_silence`` is the one place that picks it, so the lanes cannot drift apart. A proactive turn +whose prompt carries ``SILENCE_INSTRUCTION`` (the cron reminder, a sub-agent's unprompted report) is +told to answer ``[SILENT]`` when nothing is new, and models reliably vary the spelling and wrap it in +a short note on why they stayed quiet, so it reads the whole marker set by two rules: the strict one +(``is_exact_silence``), a reply that is exactly a marker, and the loose one (``is_loose_silence``), +where the bracketed ``[SILENT]`` opening the reply, or any marker alone on its first or last line, +counts too. Every other turn honours only the bracketed ``[SILENT]``, as the whole reply: a person's +turn, and a proactive one whose prompt never named the marker (a heartbeat task, a sub-agent's +result relayed, a sentinel notice), where a one-word answer such as ``Silent.`` or ``No reply.``, +or a report that ends on one, is the model's answer. """ import unicodedata @@ -103,14 +105,29 @@ def _is_token(line: str) -> bool: return stripped.upper().startswith(SILENT_MARKER) -def is_silence(text: object, origin: Origin | None) -> bool: - """Whether ``text`` is silence under the rules ``origin`` picks. +def teaches_silence(prompt: object) -> bool: + """Whether a turn's prompt taught the model the marker: it carries ``SILENCE_INSTRUCTION``. - A turn from a proactive origin reads the loose rule and the strict one: the loose matcher takes a - whole-reply marker as written, so ``NO_REPLY.`` needs the strict rule's forgiveness of edge - punctuation. Every other origin, and none, honours only ``[SILENT]`` with whitespace around it, - so an origin this table does not know delivers a reply rather than swallowing it. + Read off the prompt rather than a flag beside it, so an entry that teaches the marker cannot + forget to say so, and one that never named it cannot claim the loose rule. """ - if origin in PROACTIVE_ORIGINS: + return isinstance(prompt, str) and SILENCE_INSTRUCTION in prompt + + +def reads_loose_silence(origin: Origin | None, *, taught: bool) -> bool: + """Whether a turn reads the loose rule: one from a proactive origin whose prompt taught the marker.""" + return taught and origin in PROACTIVE_ORIGINS + + +def is_silence(text: object, origin: Origin | None, *, taught: bool) -> bool: + """Whether ``text`` is silence under the rules ``origin`` and ``taught`` pick. + + A proactive turn whose prompt taught the marker (``teaches_silence``) reads the loose rule and the + strict one: the loose matcher takes a whole-reply marker as written, so ``NO_REPLY.`` needs the + strict rule's forgiveness of edge punctuation. Every other turn, and one from an origin this + table does not know, honours only ``[SILENT]`` with whitespace around it, so a reply it was never + asked to shape is delivered rather than swallowed. + """ + if reads_loose_silence(origin, taught=taught): return is_loose_silence(text) or is_exact_silence(text) return isinstance(text, str) and text.strip() == SILENT_MARKER diff --git a/raven/agent/loop/recovery.py b/raven/agent/loop/recovery.py index c537d82f5..54875b875 100644 --- a/raven/agent/loop/recovery.py +++ b/raven/agent/loop/recovery.py @@ -375,6 +375,7 @@ def reply_kind( prev_had_tool_calls: bool, streamed: bool, reply_awaited: bool = False, + silence_taught: bool = False, ) -> ReplyKind: """What a reply that carries no tool call is, by the first of these rules it meets. @@ -383,14 +384,15 @@ def reply_kind( not read as a marker. A refusal is named before text because the text is the model's explanation of it, and thinking before a cut because a reply that thought and was cut is thinking-only: TRUNCATED is a cut with no reasoning behind it. ``reply_awaited`` is a turn whose caller waits on - its text, where a marker is text like any other. + its text, where a marker is text like any other. ``silence_taught`` says the turn's prompt taught + the marker, which with ``origin`` picks the rule a marker is read by (``is_silence``). """ has_text = bool(visible and visible.strip()) # Only a stream that died carries ``STREAM_CUT``. A clean close without a terminal chunk, and a body # that names no finish, read ``unknown`` and are whole answers. if has_text and streamed and response.finish_reason == STREAM_CUT: return ReplyKind.NETWORK_CUT - if has_text and not reply_awaited and is_silence(visible, origin): + if has_text and not reply_awaited and is_silence(visible, origin, taught=silence_taught): return ReplyKind.SILENT if response.finish_reason in DECLINED_FINISH_REASONS: return ReplyKind.REFUSAL @@ -744,8 +746,8 @@ class SilenceGate: """Holds the start of a reply back while it could still turn out to be a silence marker. A marker is the model saying nothing, so a surface that streams must never show it and then - take it back. On a person's turn, which honours only ``[SILENT]`` with whitespace around it - (:func:`is_silence`), deltas are held while the text so far could still become that, and go + take it back. On a turn that honours only ``[SILENT]`` with whitespace around it + (:func:`is_silence`), a person's or a proactive one never taught the marker, deltas are held while the text so far could still become that, and go out together with everything after them the moment it cannot -- which a reply that begins with anything but whitespace or the marker's bracket does at its first delta, so an ordinary answer waits for nothing. The loop reads a reply with its inline think blocks taken out @@ -753,9 +755,10 @@ class SilenceGate: streams as it arrives, each delta of it out the moment it comes, only the opening tag waiting for the delta that completes it; what follows the block is held while it could still be the marker. Each delta is read on its own against what the ones before it left, so a long hold costs a delta - no more than a short one. A turn from a proactive origin (``hold_all``) is read by the loose - rule, whose marker may close a report on its last line, and only the whole call can show that, - so every delta waits for :meth:`release` or :meth:`discard`. + no more than a short one. A turn that reads the loose rule (``hold_all``: a proactive turn whose + prompt taught the marker, ``reads_loose_silence``) may close a report with its marker on the + last line, and only the whole call can show that, so every delta waits for :meth:`release` or + :meth:`discard`. The loop decides once it has read the reply: ``discard`` for a marker, which goes to no one, ``release`` for anything else, and for a reply that ends in tool calls, whose words are not a diff --git a/raven/agent/loop/turn_path.py b/raven/agent/loop/turn_path.py index 3f53f44ab..23787825d 100644 --- a/raven/agent/loop/turn_path.py +++ b/raven/agent/loop/turn_path.py @@ -8,7 +8,7 @@ import functools from typing import TYPE_CHECKING -from raven.agent.context.silence import is_silence +from raven.agent.context.silence import is_silence, reads_loose_silence, teaches_silence from raven.agent.loop._shared import ( _ABORTED_ACTION_REPLY, _CONTEXT_USED_KEY, @@ -141,7 +141,7 @@ from raven.providers.tool_calls import openai_tool_call from raven.session.refs import TOOL_RESULT_TRUNCATION_MARKER, tool_result_view from raven.spine.events import bound_failure_text -from raven.spine.turn import PROACTIVE_ORIGINS, AnswerlessTurnError +from raven.spine.turn import AnswerlessTurnError from raven.token_wise import usage_context from raven.token_wise.turn_spend import TurnSpend from raven.tracing import config as tracing_config @@ -942,12 +942,14 @@ async def _run_agent_loop( # noqa: C901 (cc 100: pre-existing, above the ceilin turn_log: TurnLog | None = None, rungs_used: set[str] | None = None, reply_awaited: bool = False, + silence_taught: bool = False, ) -> tuple[str | None, list[str], list[dict], LoopOutcome]: """Run the agent iteration loop. ``origin`` is the turn's: the watch-work judgement reads the owner's request off the messages, and a turn the runtime re-injected has none - to read (``watch_work.asked_for``). It also picks the silence rule a + to read (``watch_work.asked_for``). With ``silence_taught``, whether + the turn's prompt taught the marker, it also picks the silence rule a reply is read by -- unless ``reply_awaited`` says the caller waits on the turn's text, as an inbound A2A call's peer does: such a turn takes no part in the silence protocol, so nothing is held back for a marker @@ -1289,7 +1291,7 @@ async def _end_on_the_partial(last: LLMResponse) -> str: nonlocal messages, status, silent final = self._strip_think("".join(recovery.parts)) or "" held = carried_gate.handed_over() if carried_gate is not None else None - silent = not reply_awaited and is_silence(final, origin) + silent = not reply_awaited and is_silence(final, origin, taught=silence_taught) if silent: if held is not None: held.discard() @@ -1500,7 +1502,7 @@ async def _end_on_the_partial(last: LLMResponse) -> str: if reply_room := self._reply_room(call_messages, call_tools, call_model): gen_overrides["max_tokens"] = min(gen_overrides.get("max_tokens", reply_room), reply_room) # A reply that is a silence marker is shown to no one, so the start of one is - # held until the loop has read the reply. A turn from a proactive origin holds the + # held until the loop has read the reply. A turn that reads the loose rule holds the # whole call, because its marker may close a report and only the end of the call # shows that. The gates below deliver into this one. if on_token_delta is None or reply_awaited: @@ -1508,7 +1510,7 @@ async def _end_on_the_partial(last: LLMResponse) -> str: elif carried_gate is not None: silence_gate = carried_gate.handed_over() else: - silence_gate = SilenceGate(on_token_delta, hold_all=origin in PROACTIVE_ORIGINS) + silence_gate = SilenceGate(on_token_delta, hold_all=reads_loose_silence(origin, taught=silence_taught)) downstream = silence_gate or on_token_delta # A hook can send this whole response back, and a rollback pops the # history the stream has already left -- so where hooks are installed @@ -2271,6 +2273,7 @@ async def _end_on_the_partial(last: LLMResponse) -> str: prev_had_tool_calls=prev_had_tool_calls, streamed=streamed_call, reply_awaited=reply_awaited, + silence_taught=silence_taught, ) if kind in (ReplyKind.EMPTY, ReplyKind.POST_TOOL_EMPTY): recovery.empty_streak.append(EmptyAttempt.of(response, call_model)) @@ -2521,14 +2524,14 @@ async def _end_on_the_partial(last: LLMResponse) -> str: # continued rather than judged on its fragment, so after one -- or when one can # no longer be continued -- the joined text is what may be the marker. if recovery.parts or kind is ReplyKind.NETWORK_CUT: - ends_silent = not reply_awaited and is_silence(clean, origin) + ends_silent = not reply_awaited and is_silence(clean, origin, taught=silence_taught) if own_marker and not ends_silent and not reply_awaited: # Dropping a continuation that is only the marker rests on the reader already # holding what streamed, which a turn holding its whole call never showed. So # the marker is read with what came before it, on its own line, as the same # reply uncut would be; only the loose rule can find it there. with_marker = clean + "\n" + (response.content or "").strip() - if is_silence(with_marker, origin): + if is_silence(with_marker, origin, taught=silence_taught): clean, ends_silent = with_marker, True else: ends_silent = action is RecoveryAction.SILENT @@ -2645,7 +2648,7 @@ async def _end_on_the_partial(last: LLMResponse) -> str: # answer may: its stream is held back as the answering call's is, and a # marker ends the turn silent. wrap_up_gate = ( - SilenceGate(on_token_delta, hold_all=origin in PROACTIVE_ORIGINS) + SilenceGate(on_token_delta, hold_all=reads_loose_silence(origin, taught=silence_taught)) if on_token_delta is not None and not reply_awaited else None ) @@ -2666,7 +2669,7 @@ async def _end_on_the_partial(last: LLMResponse) -> str: ), synthesis_policy=turn_synthesis(turn_meta), ) - silent = not reply_awaited and is_silence(final_content, origin) + silent = not reply_awaited and is_silence(final_content, origin, taught=silence_taught) await _settle_silence(wrap_up_gate, silent=silent) # Persist the wrap-up into history like any normal final reply. # Persistence downstream reads only the returned ``messages`` list, @@ -3242,6 +3245,7 @@ async def _attempt(seed: list[dict], attempt: int): turn_log=live["log"], rungs_used=rungs_used, reply_awaited=reply_awaited, + silence_taught=teaches_silence(content), ) # Taken BEFORE the first attempt, because the loop appends to the list it is diff --git a/raven/spine/turn.py b/raven/spine/turn.py index 3605abca9..df75778b3 100644 --- a/raven/spine/turn.py +++ b/raven/spine/turn.py @@ -17,10 +17,11 @@ class Origin(StrEnum): SUBAGENT = "subagent" -# The origins the runtime starts on its own: they share the system pool and read the loose silence -# rule. SUBAGENT is the turn that re-injects a result; a sub-agent's own execution runs off the -# scheduler behind a separate gate. An inbound A2A call runs as HEARTBEAT too, and its peer does wait -# on the text, so that entrance runs its turn with ``reply_awaited``, outside the silence protocol. +# The origins the runtime starts on its own: they share the system pool, and one whose prompt teaches +# the silence marker reads the loose silence rule. SUBAGENT is the turn that re-injects a result; a +# sub-agent's own execution runs off the scheduler behind a separate gate. An inbound A2A call runs as +# HEARTBEAT too, and its peer does wait on the text, so that entrance runs its turn with +# ``reply_awaited``, outside the silence protocol. PROACTIVE_ORIGINS = frozenset({Origin.SENTINEL, Origin.CRON, Origin.HEARTBEAT, Origin.SUBAGENT}) diff --git a/tests/test_agent_context_silence.py b/tests/test_agent_context_silence.py index 5fcef9d37..120bf8f08 100644 --- a/tests/test_agent_context_silence.py +++ b/tests/test_agent_context_silence.py @@ -10,6 +10,7 @@ is_exact_silence, is_loose_silence, is_silence, + teaches_silence, ) from raven.spine import PROACTIVE_ORIGINS, Origin @@ -48,33 +49,49 @@ def test_a_marker_mid_sentence_is_a_report(text): assert not is_loose_silence(text) -def test_the_origin_picks_the_rule(): - assert is_silence("[SILENT] No changes", Origin.CRON) and is_silence("[SILENT] No changes", Origin.SUBAGENT) - assert not is_silence("[SILENT] No changes", Origin.USER) and not is_silence("[SILENT] No changes", None) - assert is_silence("NO_REPLY", Origin.CRON) +def test_the_origin_and_the_lesson_pick_the_rule(): + assert is_silence("[SILENT] No changes", Origin.CRON, taught=True) + assert is_silence("[SILENT] No changes", Origin.SUBAGENT, taught=True) + assert not is_silence("[SILENT] No changes", Origin.USER, taught=True) + assert not is_silence("[SILENT] No changes", None, taught=True) + assert not is_silence("[SILENT] No changes", Origin.CRON, taught=False) + assert is_silence("NO_REPLY", Origin.CRON, taught=True) +@pytest.mark.parametrize("taught", [False, True]) @pytest.mark.parametrize("origin", [Origin.USER, None]) @pytest.mark.parametrize("text", ["[SILENT]", " [SILENT]\n"]) -def test_a_persons_turn_honours_the_taught_marker(text, origin): - assert is_silence(text, origin) +def test_a_persons_turn_honours_the_taught_marker(text, origin, taught): + assert is_silence(text, origin, taught=taught) +@pytest.mark.parametrize("taught", [False, True]) @pytest.mark.parametrize("origin", [Origin.USER, None]) @pytest.mark.parametrize("text", ["Silent.", "silent", "No reply.", "NO_REPLY", "*NO_REPLY*", "[silent]", "[SILENT]."]) -def test_a_persons_one_word_answer_is_not_silence(text, origin): - assert not is_silence(text, origin) +def test_a_persons_one_word_answer_is_not_silence(text, origin, taught): + assert not is_silence(text, origin, taught=taught) @pytest.mark.parametrize("origin", sorted(PROACTIVE_ORIGINS)) -def test_every_proactive_origin_reads_the_loose_rule(origin): - assert is_silence("[SILENT] No changes", origin) +def test_every_taught_proactive_origin_reads_the_loose_rule(origin): + assert is_silence("[SILENT] No changes", origin, taught=True) + + +@pytest.mark.parametrize("origin", sorted(PROACTIVE_ORIGINS)) +@pytest.mark.parametrize( + "text", ["No reply.", "silent", "NO_REPLY", "*NO_REPLY*", "Checked the queue.\n\nNo reply", "[SILENT] No changes"] +) +def test_an_untaught_proactive_turn_reads_the_rule_a_persons_does(text, origin): + """Heartbeat, a sub-agent's result and a sentinel notice are never told the marker, so a reply + that happens to be or end on one of its spellings is an answer, and only ``[SILENT]`` is not.""" + assert not is_silence(text, origin, taught=False) + assert is_silence(" [SILENT]\n", origin, taught=False) @pytest.mark.parametrize("origin", [Origin.CRON, Origin.SUBAGENT]) @pytest.mark.parametrize("text", ["*NO_REPLY*", ".NO_REPLY", "NO_REPLY."]) def test_a_proactive_turn_forgives_edge_punctuation_around_a_bare_marker(text, origin): - assert is_silence(text, origin) + assert is_silence(text, origin, taught=True) def test_the_length_cap_ignores_edge_whitespace_but_not_edge_punctuation(): @@ -87,3 +104,19 @@ def test_the_length_cap_ignores_edge_whitespace_but_not_edge_punctuation(): def test_the_instruction_teaches_the_bracketed_marker(): assert '"[SILENT]"' in SILENCE_INSTRUCTION and "Never combine [SILENT] with content" in SILENCE_INSTRUCTION + + +@pytest.mark.parametrize( + ("prompt", "taught"), + [ + (f"[Scheduled Task] Timer finished.\n\n{SILENCE_INSTRUCTION}", True), + (SILENCE_INSTRUCTION, True), + ("Summarize this naturally for the user.", False), + ("Reply [SILENT] if there is nothing new.", False), + ("", False), + (None, False), + ([{"type": "text", "text": SILENCE_INSTRUCTION}], False), + ], +) +def test_a_prompt_teaches_the_marker_only_by_carrying_the_instruction(prompt, taught): + assert teaches_silence(prompt) is taught diff --git a/tests/test_agent_loop_empty_recovery.py b/tests/test_agent_loop_empty_recovery.py index 48dc5a548..e716b1eb0 100644 --- a/tests/test_agent_loop_empty_recovery.py +++ b/tests/test_agent_loop_empty_recovery.py @@ -25,6 +25,7 @@ import pytest +from raven.agent.context.silence import SILENCE_INSTRUCTION from raven.agent.hook import AgentHook from raven.agent.loop import AgentLoop, recovery, turn_path from raven.agent.loop._shared import _MAX_ITER_STATIC_FALLBACK @@ -88,6 +89,10 @@ def workspace(): yield Path(td) +# A prompt that teaches the marker, as the cron reminder's does: only such a proactive turn reads the loose rule. +_TAUGHT = f"[Scheduled Task] Timer finished.\n\n{SILENCE_INSTRUCTION}" + + def _turn(text: str = "hi", origin: Origin = Origin.USER) -> TurnRequest: return TurnRequest( origin=origin, @@ -187,11 +192,21 @@ def test_limits_from_defaults_uses_defaults_for_missing_attrs(): # --------------------------------------------------------------------------- # -def _kind(content, finish="stop", *, origin=Origin.USER, streamed=False, prev_had_tool_calls=False, **fields): - """``reply_kind`` asked the way the loop asks it, of the text left once think blocks are out.""" +def _kind( + content, finish="stop", *, origin=Origin.USER, streamed=False, prev_had_tool_calls=False, taught=True, **fields +): + """``reply_kind`` asked the way the loop asks it, of the text left once think blocks are out, on a + turn whose prompt taught the marker unless ``taught`` says otherwise.""" response = LLMResponse(content=content or None, finish_reason=finish, **fields) visible = (strip_think_blocks(content) or None) if content else None - return reply_kind(response, visible, origin=origin, prev_had_tool_calls=prev_had_tool_calls, streamed=streamed) + return reply_kind( + response, + visible, + origin=origin, + prev_had_tool_calls=prev_had_tool_calls, + streamed=streamed, + silence_taught=taught, + ) @pytest.mark.parametrize( @@ -271,6 +286,13 @@ def test_reply_kind(content, finish, extra, origin, streamed, kind): assert _kind(content, finish, origin=origin, streamed=streamed, **extra) is kind +@pytest.mark.parametrize("origin", [Origin.CRON, Origin.HEARTBEAT, Origin.SENTINEL, Origin.SUBAGENT]) +def test_reply_kind_reads_a_loose_marker_as_text_on_a_turn_never_taught_it(origin): + assert _kind("No reply.", origin=origin, taught=False) is ReplyKind.TEXT + assert _kind("nothing new\n\n[SILENT]", origin=origin, taught=False) is ReplyKind.TEXT + assert _kind("[SILENT]", origin=origin, taught=False) is ReplyKind.SILENT + + def test_an_empty_reply_after_tools_is_its_own_kind(): assert _kind("", prev_had_tool_calls=True) is ReplyKind.POST_TOOL_EMPTY assert _kind("", prev_had_tool_calls=True, reasoning_content="hmm") is ReplyKind.THINKING_ONLY @@ -2607,10 +2629,11 @@ def _empty_finish() -> _Stream: async def _ends_on(agent: AgentLoop, origin: Origin, **kwargs): - """Run the loop on a streamed turn and return what it ended on, what was filed and what streamed.""" + """Run the loop on a streamed turn whose prompt taught the marker, and return what it ended on, + what was filed and what streamed.""" sink = _Sink() final, _used, messages, outcome = await agent._run_agent_loop( - _seed(), on_token_delta=sink.delta, origin=origin, **kwargs + _seed(), on_token_delta=sink.delta, origin=origin, silence_taught=True, **kwargs ) return final, outcome, messages[-1], sink.text @@ -4489,7 +4512,7 @@ async def test_a_marker_wrapped_in_punctuation_is_silent_on_a_proactive_turn(wor sink = _Sink() final, _used, messages, outcome = await _litellm_agent(workspace)._run_agent_loop( - _seed(), on_token_delta=sink.delta, origin=Origin.CRON + _seed(), on_token_delta=sink.delta, origin=Origin.CRON, silence_taught=True ) assert outcome.silent is True and final == "".join(deltas) @@ -4570,11 +4593,105 @@ async def test_a_proactive_turn_still_reads_a_loosely_spelled_marker_as_silence( agent = _litellm_agent(workspace) silence: dict[str, bool] = {} - out = await agent._process_message(_turn(origin=Origin.CRON), session_key="s1", origin=Origin.CRON, silence=silence) + out = await agent._process_message( + _turn(_TAUGHT, origin=Origin.CRON), session_key="s1", origin=Origin.CRON, silence=silence + ) + + assert out is None and silence == {"silent": True} + + +class _RewritesTheInbound(AgentHook): + """A user-inbound hook that hands the model its own text in place of what was sent.""" + + def __init__(self, text: str) -> None: + self._text = text + + @property + def name(self) -> str: + return "RewritesTheInbound" + + async def before_user_inbound(self, ctx): + from raven.agent.hook import HookDecision + + return HookDecision(modified_content=self._text) + + +@pytest.mark.asyncio +@pytest.mark.parametrize( + "sent,read,silent", + [(_TAUGHT, "Timer finished.", False), ("Timer finished.", _TAUGHT, True)], + ids=["lesson_rewritten_out", "lesson_rewritten_in"], +) +async def test_the_lesson_is_read_off_the_prompt_the_model_read(workspace, monkeypatch, sent, read, silent): + """A hook may rewrite the inbound before the model reads it, so whether the turn was taught the + marker is read off the text the model got, not the request that carried it.""" + calls = _served(monkeypatch, [_reply("No reply.")]) + agent = _litellm_agent(workspace, hooks=[_RewritesTheInbound(read)]) + silence: dict[str, bool] = {} + + out = await agent._process_message( + _turn(sent, origin=Origin.CRON), session_key="s1", origin=Origin.CRON, silence=silence + ) + + asked = calls[0]["messages"][-1]["content"] + assert read in str(asked) and (SILENCE_INSTRUCTION in str(asked)) is silent + assert (out is None) is silent and silence == ({"silent": True} if silent else {}) + + +@pytest.mark.asyncio +@pytest.mark.parametrize("origin", [Origin.HEARTBEAT, Origin.SUBAGENT, Origin.SENTINEL]) +@pytest.mark.parametrize("reply", ["No reply.", "Silent", "Checked the queue twice.\n\nNo reply"]) +async def test_a_proactive_turn_never_taught_the_marker_delivers_a_reply_that_looks_like_one( + workspace, monkeypatch, origin, reply +): + """Heartbeat, a sub-agent's relayed result and a sentinel notice never tell the model the marker, + so a reply that is, or ends on, one of its spellings is the model's answer and reaches the reader.""" + _served(monkeypatch, [_reply(reply)]) + agent = _litellm_agent(workspace) + silence: dict[str, bool] = {} + + out = await agent._process_message( + _turn("Summarize this naturally for the user.", origin=origin), + session_key="s1", + origin=origin, + silence=silence, + ) + + assert out is not None and out[0] == reply and silence == {} + + +@pytest.mark.asyncio +async def test_a_proactive_turn_never_taught_the_marker_still_honours_the_bracketed_one(workspace, monkeypatch): + _served(monkeypatch, [_reply("[SILENT]")]) + agent = _litellm_agent(workspace) + silence: dict[str, bool] = {} + + out = await agent._process_message( + _turn("Summarize this naturally for the user.", origin=Origin.SUBAGENT), + session_key="s1", + origin=Origin.SUBAGENT, + silence=silence, + ) assert out is None and silence == {"silent": True} +@pytest.mark.asyncio +@pytest.mark.parametrize("taught,shown", [(False, ["Checked ", "the queue.\n\n", "No reply"]), (True, [])]) +async def test_only_a_taught_proactive_turn_holds_its_whole_call(workspace, monkeypatch, taught, shown): + """The loose rule's marker may close a report, so a taught turn holds every delta until the call + ends; a turn that reads the bracketed marker alone streams each one as a person's turn does.""" + _served(monkeypatch, [_finished_stream("Checked ", "the queue.\n\n", "No reply")]) + sink = _Sink() + + final, _used, _messages, outcome = await _litellm_agent(workspace)._run_agent_loop( + _seed(), on_token_delta=sink.delta, origin=Origin.HEARTBEAT, silence_taught=taught + ) + + assert final == "Checked the queue.\n\nNo reply" and outcome.silent is taught + assert sink.text == shown + + @pytest.mark.asyncio @pytest.mark.parametrize("origin,silent", [(Origin.USER, False), (Origin.CRON, True)]) async def test_a_wrap_up_that_closes_a_note_with_the_marker_is_read_by_its_turns_rule( @@ -4587,7 +4704,7 @@ async def test_a_wrap_up_that_closes_a_note_with_the_marker_is_read_by_its_turns sink = _Sink() final, _used, messages, outcome = await _litellm_agent(workspace, max_iterations=1)._run_agent_loop( - _seed(), on_token_delta=sink.delta, origin=origin + _seed(), on_token_delta=sink.delta, origin=origin, silence_taught=True ) assert final == wrap_up and outcome.silent is silent @@ -4602,7 +4719,7 @@ async def test_a_wrap_up_that_spells_a_marker_loosely_is_read_by_its_turns_rule( sink = _Sink() final, _used, messages, outcome = await _litellm_agent(workspace, max_iterations=1)._run_agent_loop( - _seed(), on_token_delta=sink.delta, origin=origin + _seed(), on_token_delta=sink.delta, origin=origin, silence_taught=True ) assert final == "No reply." and outcome.silent is silent @@ -4610,13 +4727,27 @@ async def test_a_wrap_up_that_spells_a_marker_loosely_is_read_by_its_turns_rule( assert "".join(sink.text) == ("" if silent else "No reply.") +@pytest.mark.asyncio +@pytest.mark.parametrize("taught,shown", [(False, ["Checked ", "twice.\n\n", "No reply"]), (True, [])]) +async def test_only_a_taught_proactive_wrap_up_holds_its_whole_call(workspace, monkeypatch, taught, shown): + _served(monkeypatch, [_tool_stream(), _finished_stream("Checked ", "twice.\n\n", "No reply")]) + sink = _Sink() + + final, _used, _messages, outcome = await _litellm_agent(workspace, max_iterations=1)._run_agent_loop( + _seed(), on_token_delta=sink.delta, origin=Origin.HEARTBEAT, silence_taught=taught + ) + + assert final == "Checked twice.\n\nNo reply" and outcome.silent is taught + assert sink.text == shown + + @pytest.mark.asyncio async def test_a_cron_turn_may_put_the_marker_on_its_last_line(workspace, monkeypatch): _served(monkeypatch, [_finished_stream("2 deals ", "filtered\n\n", "[SILENT]")]) sink = _Sink() final, _used, messages, outcome = await _litellm_agent(workspace)._run_agent_loop( - _seed(), on_token_delta=sink.delta, origin=Origin.CRON + _seed(), on_token_delta=sink.delta, origin=Origin.CRON, silence_taught=True ) assert outcome.silent is True and final == "2 deals filtered\n\n[SILENT]" @@ -4654,7 +4785,7 @@ async def test_a_reply_that_is_no_marker_is_shown_whole(workspace, monkeypatch, sink = _Sink() final, _used, _messages, outcome = await _litellm_agent(workspace)._run_agent_loop( - _seed(), on_token_delta=sink.delta, origin=origin + _seed(), on_token_delta=sink.delta, origin=origin, silence_taught=True ) assert final == "2 deals filtered" and outcome.silent is False @@ -5170,7 +5301,7 @@ async def test_a_proactive_turn_that_skips_after_send_stays_silent(workspace, mo silence: dict[str, bool] = {} out = await _litellm_agent(workspace, hooks=[sees])._process_message( - _turn(origin=Origin.SENTINEL), session_key="s1", origin=Origin.SENTINEL, silence=silence + _turn(_TAUGHT, origin=Origin.SENTINEL), session_key="s1", origin=Origin.SENTINEL, silence=silence ) assert out is None and silence == {"silent": True} diff --git a/tests/test_core_cron_stack.py b/tests/test_core_cron_stack.py index 7be1fb912..8031c1bd4 100644 --- a/tests/test_core_cron_stack.py +++ b/tests/test_core_cron_stack.py @@ -20,7 +20,7 @@ import pytest -from raven.agent.context.silence import SILENCE_INSTRUCTION +from raven.agent.context.silence import SILENCE_INSTRUCTION, teaches_silence from raven.core.cron_stack import make_on_cron_job from raven.proactive_engine.schedulers.cron.types import ( CronJob, @@ -136,6 +136,7 @@ async def test_the_reminder_teaches_the_silence_marker(spine): text = spine.captured[0].text assert text.endswith("\n\n" + SILENCE_INSTRUCTION) assert text.count(SILENCE_INSTRUCTION) == 1 + assert teaches_silence(text), "so the turn reads the loose rule the lesson asks for" # ───────────────────────────────────────────────────────────────────── diff --git a/tests/test_subagent_manager.py b/tests/test_subagent_manager.py index 6319f619f..035134c92 100644 --- a/tests/test_subagent_manager.py +++ b/tests/test_subagent_manager.py @@ -26,7 +26,7 @@ from pydantic import ValidationError from raven.agent import workdir -from raven.agent.context.silence import SILENCE_INSTRUCTION +from raven.agent.context.silence import SILENCE_INSTRUCTION, teaches_silence from raven.agent.subagent import manager as manager_mod from raven.agent.subagent.backends.base import clamp_output from raven.agent.subagent.builtin_agents import GENERIC_AGENT @@ -1307,6 +1307,27 @@ async def test_announce_result_routes_to_tui_session_key(monkeypatch): assert submitted[0].conversation == "tui:sess123" +async def test_a_relayed_result_does_not_teach_the_silence_marker(): + """The person asked for the work, so its result is relayed to them, never withheld: the turn is + not told the marker, and a reply that only looks like one is delivered.""" + mgr = _make_manager(max_concurrent=1) + submitted = [] + mgr.set_submit(submitted.append) + + await mgr._announce_result( + task_id="t1", + task_summary="label", + task="task", + result="No reply needed.", + origin={"channel": "web", "chat_id": "d", "session_key": "web:s1"}, + status="ok", + ) + + assert len(submitted) == 1 + assert "Summarize this naturally for the user" in submitted[0].text + assert not teaches_silence(submitted[0].text) + + async def test_announce_result_routes_non_tui_origin_unchanged(monkeypatch): """Non-TUI origins (e.g. a channel with a real chat_id) still announce on their existing channel:chat_id conversation — no regression.""" @@ -5208,6 +5229,7 @@ async def test_an_unprompted_report_teaches_the_silence_marker() -> None: text = submitted[0].text assert text.count(SILENCE_INSTRUCTION) == 1 assert text.endswith("\n\n" + SILENCE_INSTRUCTION) + assert teaches_silence(text), "so the turn reads the loose rule the lesson asks for" assert text.rindex(SILENCE_INSTRUCTION) > text.rindex("END UNTRUSTED") assert "no reply to the user are needed" not in text assert "If it is routine progress only, no action is needed." in text From c5d6018e173f1d9ba4317a75f63c4ee1f82feab5 Mon Sep 17 00:00:00 2001 From: Dizhan Xue Date: Thu, 29 Oct 2026 06:42:44 +0000 Subject: [PATCH 08/13] fix(*): take back only what a silent turn's reply showed A silent turn's notice told a client to take back "the reply", and the web page found it at the step's last tool row. A call whose only tool was a message that went nowhere draws no tool row, so an exhausted turn's wrap-up erased the step's narration with it. The TUI cleared its whole stream buffer, so narration that no tool call had flushed went too. The loop now counts what the silence gates put in front of the reader in the step drawn now -- one call and its cut answer's continuations, or the wrap-up alone -- and the SILENT_TURN notice carries it as its detail (LoopOutcome.shown). The web page and the TUI take back exactly that suffix of the step's text and keep everything before it, and the line they draw no longer shows the detail. ACP writes nothing for the notice, as before. --- CONTEXT.md | 5 +- raven/agent/loop/_shared.py | 6 ++ raven/agent/loop/turn_path.py | 39 ++++++++-- raven/rpc/models.py | 6 +- raven/spine/events.py | 1 + rpc-schema/openrpc.json | 2 +- tests/test_acp_updates.py | 6 ++ tests/test_agent_loop_empty_recovery.py | 73 +++++++++++++++++-- tests/test_agent_loop_run_emit.py | 18 ++++- tests/test_rpc_spine.py | 10 +++ .../createGatewayEventHandler.test.ts | 4 +- ui-tui/src/__tests__/turn-subscribe.test.ts | 38 +++++++++- ui-tui/src/app/chatStream.ts | 8 +- ui-tui/src/app/turnController.ts | 28 ++++--- ui-tui/src/rpc/generated.ts | 2 +- .../transcript/TranscriptPage.test.tsx | 68 +++++++++++++++-- .../src/features/transcript/TurnNav.test.tsx | 2 +- ui-web/src/features/transcript/store.ts | 10 ++- ui-web/src/features/transcript/types.ts | 12 +-- ui-web/src/rpc/generated.ts | 2 +- .../src/state/session/pipeline.events.test.ts | 27 +++++-- ui-web/src/state/session/stages.ts | 14 ++-- 22 files changed, 309 insertions(+), 72 deletions(-) diff --git a/CONTEXT.md b/CONTEXT.md index 8e2d23a4d..86510d1cd 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -846,7 +846,10 @@ stays in the history holding the model's whole reply -- the marker, or, under th the note around it -- filed with `silent: true` so a reload does not read it as an answer, and it carries the turn's Response Meta. The turn reports itself as `TurnOutcome.silent` and, for a surface drawing it live, as a closing `NoticeKind.SILENT_TURN` -notice. The kernel holds no marker rules -- they live in `agent/context/silence.py` -- and the +notice. Its detail is what that reply had streamed before it was read as the marker -- at most a +think block the model inlined ahead of it -- counted over the step a reader draws it in (its call and +a cut answer's continuations, or the wrap-up alone; `LoopOutcome.shown`), so a client takes back +exactly that suffix of the step's text and keeps the narration before it. The kernel holds no marker rules -- they live in `agent/context/silence.py` -- and the spine's `TurnEnded` carries no flag of its own; what the kernel names of silence is the outcome (`TurnOutcome.silent`), the notice (`NoticeKind.SILENT_TURN`) and which origins are proactive (`PROACTIVE_ORIGINS`). diff --git a/raven/agent/loop/_shared.py b/raven/agent/loop/_shared.py index 2b88fb942..1a0b1033a 100644 --- a/raven/agent/loop/_shared.py +++ b/raven/agent/loop/_shared.py @@ -291,6 +291,11 @@ class LoopOutcome: ``silent`` is set when the turn's last reply is a silence marker. That is the model's answer, not a failure and not an empty reply: the row stays in the history and carries ``_silent``, and the reader is shown none of it. + ``shown`` is what that reply had already streamed before it was read as the + marker -- a think block the model inlined ahead of it goes out as it comes -- + counted over the step a reader draws it in: its call and the continuations + of a cut answer, or the wrap-up alone. A client that drew it takes back + exactly this, and nothing it drew before. Empty when the reply showed nothing. """ status: str = "completed" # "completed" | "interrupted" | "error" @@ -298,6 +303,7 @@ class LoopOutcome: error_full: str | None = None no_output: bool = False silent: bool = False + shown: str = "" checkpoint_id: str | None = None edited_files: list[str] = field(default_factory=list) diff --git a/raven/agent/loop/turn_path.py b/raven/agent/loop/turn_path.py index 23787825d..c606541b1 100644 --- a/raven/agent/loop/turn_path.py +++ b/raven/agent/loop/turn_path.py @@ -1032,6 +1032,15 @@ async def _run_agent_loop( # noqa: C901 (cc 100: pre-existing, above the ceilin # Set when the turn's last reply is a silence marker: the model's own choice to say # nothing, which the caller does not deliver. silent = False + # What the silence gates have put in front of the reader in the step drawn now: one model + # call with its cut answer's continuations, or the wrap-up. A reply read as the marker + # reports it (``LoopOutcome.shown``), for a client to take back that much and no more. + step_shown: list[str] = [] + + async def _shown(delta: str) -> None: + step_shown.append(delta) + if on_token_delta is not None: + await on_token_delta(delta) # The turn's window bookkeeping: the last billed context size, the retry # budgets every shrink draws on, and the picture window. Held here and @@ -1335,6 +1344,8 @@ async def _end_on_the_partial(last: LLMResponse) -> str: # group this call's reasoning + text + tools into a single step. Not # for the continuation of a cut answer: a boundary there would show # the reader one answer as two steps. + if not merge_episode: + step_shown.clear() if on_episode_start is not None and not merge_episode: # A bounded per-outlet queue with a wedged worker behind it parks # here forever; the boundary is a grouping hint, so losing one is @@ -1510,7 +1521,7 @@ async def _end_on_the_partial(last: LLMResponse) -> str: elif carried_gate is not None: silence_gate = carried_gate.handed_over() else: - silence_gate = SilenceGate(on_token_delta, hold_all=reads_loose_silence(origin, taught=silence_taught)) + silence_gate = SilenceGate(_shown, hold_all=reads_loose_silence(origin, taught=silence_taught)) downstream = silence_gate or on_token_delta # A hook can send this whole response back, and a rollback pops the # history the stream has already left -- so where hooks are installed @@ -2647,8 +2658,10 @@ async def _end_on_the_partial(last: LLMResponse) -> str: # The wrap-up is the turn's last reply, so it may be a Silence Marker as an # answer may: its stream is held back as the answering call's is, and a # marker ends the turn silent. + # The wrap-up streams into the last call's step, after the narration that step keeps. + step_shown.clear() wrap_up_gate = ( - SilenceGate(on_token_delta, hold_all=reads_loose_silence(origin, taught=silence_taught)) + SilenceGate(_shown, hold_all=reads_loose_silence(origin, taught=silence_taught)) if on_token_delta is not None and not reply_awaited else None ) @@ -2753,7 +2766,12 @@ async def _end_on_the_partial(last: LLMResponse) -> str: # pipeline can gate on completion. outcome = LoopOutcome( - status=status, error=error_detail, error_full=error_full, no_output=no_output, silent=silent + status=status, + error=error_detail, + error_full=error_full, + no_output=no_output, + silent=silent, + shown="".join(step_shown) if silent else "", ) checkpoint = self._turn_checkpoint() if checkpoint is not None: @@ -2794,7 +2812,7 @@ async def _process_message( # noqa: C901 (cc 47: pre-existing, above the ceilin origin: Origin | None = None, drain: Drain | None = None, hook_sink: dict[str, str] | None = None, - silence: dict[str, bool] | None = None, + silence: dict[str, bool | str] | None = None, reply_awaited: bool = False, ) -> tuple[str | None, list[str]] | None: """Process a single turn request and return its reply. @@ -2808,7 +2826,8 @@ async def _process_message( # noqa: C901 (cc 47: pre-existing, above the ceilin reply: a streamed reply has already left as deltas by then, so the caller has to send that tail itself. ``silence``, when given, receives ``"silent": True`` for the last of those: a turn the model ended on a - marker, and that nothing replaced with a reply of its own. + marker, and that nothing replaced with a reply of its own -- and, when + that reply had streamed anything first, ``"shown"`` (``LoopOutcome.shown``). ``reply_awaited`` keeps a turn whose caller waits on its text out of the silence protocol (``_run_agent_loop``). """ @@ -3562,6 +3581,8 @@ async def _post_learn(): logger.info("Silent turn for {}:{}: the model chose not to reply", channel, sender_id) if silence is not None: silence["silent"] = True + if outcome.shown: + silence["shown"] = outcome.shown return None preview = final_content[:120] + "..." if len(final_content) > 120 else final_content @@ -4148,7 +4169,7 @@ async def on_direct_delta(text: str) -> None: streamed = False hook_sink: dict[str, str] = {} - silence: dict[str, bool] = {} + silence: dict[str, bool | str] = {} async def on_token(text: str) -> None: nonlocal streamed @@ -4331,9 +4352,11 @@ async def _route_to_stream(content: str, media: list[str]) -> None: await emit(StreamDelta(delta=hook_sink["appended"])) if text_sink is not None and reply_content: text_sink["text"] = reply_content - silent = silence.get("silent", False) + silent = silence.get("silent", False) is True if silent: - await emit(Notice(kind=NoticeKind.SILENT_TURN)) + # Named so a client that drew the reply's think block takes back that and no more. + shown = silence.get("shown") + await emit(Notice(kind=NoticeKind.SILENT_TURN, detail=shown if isinstance(shown, str) and shown else None)) usage = Usage( prompt_tokens=int(usage_sink.get("prompt_tokens", 0) or 0), diff --git a/raven/rpc/models.py b/raven/rpc/models.py index d72a42ecb..2779133b5 100644 --- a/raven/rpc/models.py +++ b/raven/rpc/models.py @@ -572,7 +572,11 @@ class NoticePayload(_Strict): description=( "What `kind` says it is: the blocking tool's own first line, the failed call's error category " "(or `empty_reply` / `thinking_only` for the wait before an empty-reply resend or a reasoning " - "step-down), or the organ that dropped out. Empty for `silent_turn`." + "step-down), or the organ that dropped out. For `silent_turn`, what the reply read as the marker had " + "streamed before it was: a think block the model inlined ahead of it, which goes out as it comes, counted " + "over the step it was drawn in (its call and a cut answer's continuations, or the wrap-up alone). A " + "client that drew it takes back exactly that suffix of the step's text, and nothing it drew before. " + "Empty when the reply showed nothing." ), ) transient: bool = Field( diff --git a/raven/spine/events.py b/raven/spine/events.py index 9cb85b5d3..82ea8687c 100644 --- a/raven/spine/events.py +++ b/raven/spine/events.py @@ -37,6 +37,7 @@ class NoticeKind(StrEnum): # `thinking_only`, never the vendor's own body. LLM_RETRY = "llm_retry" # The turn ended on a silence marker: an outlet draws one faint line, a channel sends nothing. + # The detail, when set, is what that reply had streamed anyway, for a client to take back. SILENT_TURN = "silent_turn" diff --git a/rpc-schema/openrpc.json b/rpc-schema/openrpc.json index 8ba315b1b..e1ba6a97b 100644 --- a/rpc-schema/openrpc.json +++ b/rpc-schema/openrpc.json @@ -14940,7 +14940,7 @@ "detail": { "type": "string", "default": "", - "description": "What `kind` says it is: the blocking tool's own first line, the failed call's error category (or `empty_reply` / `thinking_only` for the wait before an empty-reply resend or a reasoning step-down), or the organ that dropped out. Empty for `silent_turn`." + "description": "What `kind` says it is: the blocking tool's own first line, the failed call's error category (or `empty_reply` / `thinking_only` for the wait before an empty-reply resend or a reasoning step-down), or the organ that dropped out. For `silent_turn`, what the reply read as the marker had streamed before it was: a think block the model inlined ahead of it, which goes out as it comes, counted over the step it was drawn in (its call and a cut answer's continuations, or the wrap-up alone). A client that drew it takes back exactly that suffix of the step's text, and nothing it drew before. Empty when the reply showed nothing." }, "transient": { "type": "boolean", diff --git a/tests/test_acp_updates.py b/tests/test_acp_updates.py index 4f9635251..c625ab4fa 100644 --- a/tests/test_acp_updates.py +++ b/tests/test_acp_updates.py @@ -556,6 +556,12 @@ def test_other_notices_stay_off_the_wire(self): for kind in ("progress", "tool_hint", "injected", "delivery_failed"): assert translate({"type": "notice", "payload": {"kind": kind}}).updates == () + def test_a_silent_turn_notice_writes_nothing_of_what_its_reply_showed(self): + """ACP has no way to take a chunk back, and the detail is text the client already holds.""" + payload = {"kind": "silent_turn", "detail": "nothing new", "transient": False} + result = translate({"type": "notice", "payload": payload}) + assert (result.updates, result.latch, result.stop) == ((), None, None) + class TestUnmappedEvents: @pytest.mark.parametrize( diff --git a/tests/test_agent_loop_empty_recovery.py b/tests/test_agent_loop_empty_recovery.py index e716b1eb0..f87e7c92f 100644 --- a/tests/test_agent_loop_empty_recovery.py +++ b/tests/test_agent_loop_empty_recovery.py @@ -2926,26 +2926,53 @@ async def test_a_proactive_turn_shows_only_the_answer_a_thought_was_prefilled_in assert "".join(streamed) == "Report." +@pytest.mark.asyncio +async def test_a_marker_its_continuation_finished_reports_the_thought_its_step_showed(workspace, monkeypatch): + """A cut answer's continuation is drawn in the step the cut call opened, so what that step showed + before the marker -- here the thought, ahead of a marker the cut split -- counts across both calls.""" + _served(monkeypatch, [_DyingStream("a [SIL"), _finished_stream("ENT]")]) + started: list[int] = [] + + async def _step(index: int) -> None: + started.append(index) + + final, outcome, _last, streamed = await _ends_on(_litellm_agent(workspace), Origin.USER, on_episode_start=_step) + + assert outcome.silent is True and len(started) == 1 + assert "".join(streamed) == outcome.shown == "a" + + @pytest.mark.asyncio @pytest.mark.parametrize( - "script", + "script,shown,steps", [ - pytest.param(lambda: [_finished_stream("a", "b", "[SILENT]")], id="uncut"), pytest.param( - lambda: [_DyingStream("a"), _DyingStream("b"), _finished_stream("[SILENT]")], id="cut" + lambda: [_finished_stream("a", "b", "[SILENT]")], "ab", 1, id="uncut" + ), + pytest.param( + lambda: [_DyingStream("a"), _DyingStream("b"), _finished_stream("[SILENT]")], "", 2, id="cut" ), ], ) -async def test_a_marker_after_a_thought_the_cuts_closed_is_the_reply(workspace, monkeypatch, script): +async def test_a_marker_after_a_thought_the_cuts_closed_is_the_reply(workspace, monkeypatch, script, shown, steps): """With nothing but a thought before it, the marker that continues it is the whole reply, not a - continuation that adds nothing: the turn ends silent as the uncut reply does.""" + continuation that adds nothing: the turn ends silent as the uncut reply does. What the reply + reports having shown is its own step's: uncut, the thought it opened with; cut, nothing, since a + thought the cuts closed is answered as a thinking-only reply, and the marker comes in a call and + a step of its own.""" _served(monkeypatch, script()) + started: list[int] = [] - final, outcome, last, streamed = await _ends_on(_litellm_agent(workspace), Origin.USER) + async def _step(index: int) -> None: + started.append(index) + + final, outcome, last, streamed = await _ends_on(_litellm_agent(workspace), Origin.USER, on_episode_start=_step) assert final == "[SILENT]" and outcome.silent is True assert last.get("_silent") is True - assert "[SIL" not in "".join(streamed) + assert "".join(streamed) == "ab" + assert len(started) == steps + assert outcome.shown == shown @pytest.mark.asyncio @@ -5091,6 +5118,38 @@ async def test_a_marker_reply_returns_nothing_and_files_a_flagged_row(workspace, assert rows[-1]["silent"] is True and "silent" not in rows[0] +@pytest.mark.asyncio +async def test_a_silent_wrap_up_names_only_what_it_showed_not_the_narration_before_it(workspace, monkeypatch): + """An exhausted turn's wrap-up shares the reader's step with the last call's narration, and a call whose + only tool sent nothing leaves no tool row to mark where that narration ends. The notice names what the + wrap-up itself showed, so a client takes back the think block and keeps the narration.""" + from raven.spine.events import Notice, StreamDelta + + narrated = _Stream( + [_chunk("Let me check the queue."), *_tool_stream()._chunks], received_finish_reason="tool_calls" + ) + _served(monkeypatch, [narrated, _finished_stream("still nothing", "\n[SILENT]")]) + agent = _litellm_agent(workspace, max_iterations=1) + + async def _noop(**_kw) -> None: + return None + + agent._start_executor = _noop + agent._connect_mcp = _noop + events: list = [] + + async def emit(event) -> None: + events.append(event) + + outcome = await agent.run_turn(_turn("anything new?"), emit, lambda: []) + + assert outcome.silent is True + streamed = "".join(e.delta for e in events if isinstance(e, StreamDelta)) + assert streamed == "Let me check the queue.still nothing" + silent = [e.detail for e in events if isinstance(e, Notice) and e.kind is NoticeKind.SILENT_TURN] + assert silent == ["still nothing"] + + @pytest.mark.asyncio async def test_a_marker_reply_ends_the_turn_silently(workspace, monkeypatch): """A person's turn, the reply is ``[SILENT]``: the turn's exit, end to end through the real adapter.""" diff --git a/tests/test_agent_loop_run_emit.py b/tests/test_agent_loop_run_emit.py index f052c561b..74cda7334 100644 --- a/tests/test_agent_loop_run_emit.py +++ b/tests/test_agent_loop_run_emit.py @@ -1880,11 +1880,27 @@ async def test_a_silent_turn_emits_its_notice_and_reports_itself_silent(tmp_path assert not any(isinstance(e, EvStreamDelta) for e in sink.events), "no delta carried any of the marker" assert not any(isinstance(e, EvText) for e in sink.events) - assert [e.kind for e in sink.events if isinstance(e, EvNotice)] == [NoticeKind.SILENT_TURN] + assert [(e.kind, e.detail) for e in sink.events if isinstance(e, EvNotice)] == [(NoticeKind.SILENT_TURN, None)] assert outcome.silent is True assert outcome.explicit_reply is False +async def test_a_silent_turns_notice_names_what_its_reply_showed(tmp_path): + """The marker is held back, but a think block the model inlined before it streams as it comes. The + notice names exactly that text, so a client takes back what the closing reply drew and nothing else.""" + chunks = [ChatDelta(content="nothing new"), ChatDelta(content="\n[SIL"), ChatDelta(content="ENT]")] + loop = AgentLoop(provider=_FakeStreamProvider(chunks), workspace=tmp_path) + _stub_edges(loop) + sink = _EmitCollector() + + outcome = await loop.run_turn(_req("anything new?"), sink, _drain) + + shown = "".join(e.delta for e in sink.events if isinstance(e, EvStreamDelta)) + assert shown == "nothing new" + assert [(e.kind, e.detail) for e in sink.events if isinstance(e, EvNotice)] == [(NoticeKind.SILENT_TURN, shown)] + assert outcome.silent is True + + async def test_a_turn_that_replied_emits_no_silent_notice(tmp_path): loop = AgentLoop(provider=_FakeStreamProvider([ChatDelta(content="all quiet")]), workspace=tmp_path) _stub_edges(loop) diff --git a/tests/test_rpc_spine.py b/tests/test_rpc_spine.py index 5610f1900..d91825065 100644 --- a/tests/test_rpc_spine.py +++ b/tests/test_rpc_spine.py @@ -859,6 +859,16 @@ async def test_a_silent_turn_notice_reaches_clients(): TypeAdapter(TurnEvent).validate_python(emitter.emitted[0][1]) +async def test_a_silent_turn_notice_carries_what_its_reply_showed(): + """The detail is the text the reply had streamed before it was read as the marker, for a client + to take back; the outlet passes it through as it came.""" + emitter = FakeEmitter() + outlet = RpcOutlet("tui", emitter) + await outlet.deliver(Notice(kind=NoticeKind.SILENT_TURN, detail="x", conversation_id="tui:c1")) + payload = {"kind": "silent_turn", "detail": "x", "transient": False} + assert emitter.emitted == [("tui:c1", {"type": "notice", "payload": payload})] + + async def test_a_retry_wait_rides_notice_marked_transient_and_tagged_with_its_lane(): """The other notice the runtime raises about a turn, and the opposite of a blocked action: the turn is still running, so a client must draw it where the diff --git a/ui-tui/src/__tests__/createGatewayEventHandler.test.ts b/ui-tui/src/__tests__/createGatewayEventHandler.test.ts index 7402466d6..da263458a 100644 --- a/ui-tui/src/__tests__/createGatewayEventHandler.test.ts +++ b/ui-tui/src/__tests__/createGatewayEventHandler.test.ts @@ -405,7 +405,7 @@ describe('createGatewayEventHandler', () => { onEvent({ payload: { text: 'abc' }, type: 'message.delta' } as any) // The typed notice marks the turn on the shared controller; this path has // no frame of its own for it. - turnController.recordSilentTurn() + turnController.recordSilentTurn('abc') onEvent({ payload: { text: '[SILENT]' }, type: 'message.complete' } as any) // Neither the marker nor the preview is drawn, and the fallback that stands @@ -436,7 +436,7 @@ describe('createGatewayEventHandler', () => { const onEvent = createGatewayEventHandler(bellCtx([], write)) onEvent({ payload: {}, type: 'message.start' } as any) - turnController.recordSilentTurn() + turnController.recordSilentTurn('') onEvent({ payload: { text: '[SILENT]' }, type: 'message.complete' } as any) // A bell is a ping, and a ping is a delivery: nothing was delivered. diff --git a/ui-tui/src/__tests__/turn-subscribe.test.ts b/ui-tui/src/__tests__/turn-subscribe.test.ts index 2997a25b3..1c91006d1 100644 --- a/ui-tui/src/__tests__/turn-subscribe.test.ts +++ b/ui-tui/src/__tests__/turn-subscribe.test.ts @@ -354,7 +354,7 @@ describe('a turn the model chose not to answer', () => { vi.advanceTimersByTime(STREAM_IDLE_BATCH_MS) expect(getTurnState().streaming).toBe('abc') - fake.__pushEvent({ type: 'notice', payload: { kind: 'silent_turn' } }) + fake.__pushEvent({ type: 'notice', payload: { kind: 'silent_turn', detail: 'abc' } }) // Taken off the screen when the signal arrives, not when the turn lands. expect(getTurnState().streaming).toBe('') @@ -372,7 +372,7 @@ describe('a turn the model chose not to answer', () => { fake.__pushEvent({ type: 'message.start', payload: { turn_id: 'turn-1' } }) fake.__pushEvent({ type: 'token.delta', payload: { text: 'abc' } }) - fake.__pushEvent({ type: 'notice', payload: { kind: 'silent_turn' } }) + fake.__pushEvent({ type: 'notice', payload: { kind: 'silent_turn', detail: 'abc' } }) complete(fake) expect(appended).toEqual([{ role: 'system', text: SILENT_LINE }]) @@ -394,7 +394,7 @@ describe('a turn the model chose not to answer', () => { }) fake.__pushEvent({ type: 'episode.start', payload: { index: 1 } }) fake.__pushEvent({ type: 'token.delta', payload: { text: 'abc' } }) - fake.__pushEvent({ type: 'notice', payload: { kind: 'silent_turn' } }) + fake.__pushEvent({ type: 'notice', payload: { kind: 'silent_turn', detail: 'abc' } }) complete(fake) // The work stays; there is no answer under it, and what was said on the way @@ -408,6 +408,38 @@ describe('a turn the model chose not to answer', () => { expect(JSON.stringify(appended)).not.toContain('abc') }) + it('keeps what the turn said before the reply the notice takes back', async () => { + const { appended, fake } = await open() + + fake.__pushEvent({ type: 'message.start', payload: { turn_id: 'turn-1' } }) + fake.__pushEvent({ type: 'episode.start', payload: { index: 0 } }) + fake.__pushEvent({ type: 'token.delta', payload: { text: 'Let me check the queue.' } }) + // That call's only tool was a message that went nowhere, which sends no + // tool.start: nothing moved the narration out of the stream buffer. + fake.__pushEvent({ type: 'episode.start', payload: { index: 1 } }) + fake.__pushEvent({ type: 'token.delta', payload: { text: 'still nothing' } }) + fake.__pushEvent({ type: 'notice', payload: { kind: 'silent_turn', detail: 'still nothing' } }) + complete(fake) + + // The narration lands as the segment a tool call's flush would have made of + // it; the thought the closing reply showed is gone, and the line closes the turn. + expect(appended).toEqual([ + { role: 'assistant', text: 'Let me check the queue.' }, + { role: 'system', text: SILENT_LINE } + ]) + }) + + it('takes back a reply it saw only the end of', async () => { + const { appended, fake } = await open() + + fake.__pushEvent({ type: 'message.start', payload: { turn_id: 'turn-1' } }) + fake.__pushEvent({ type: 'token.delta', payload: { text: 'nothing' } }) + fake.__pushEvent({ type: 'notice', payload: { kind: 'silent_turn', detail: 'still nothing' } }) + complete(fake) + + expect(appended).toEqual([{ role: 'system', text: SILENT_LINE }]) + }) + it('does not carry the mark into the next turn', async () => { const { appended, fake } = await open() diff --git a/ui-tui/src/app/chatStream.ts b/ui-tui/src/app/chatStream.ts index d8a36c57f..58fda4599 100644 --- a/ui-tui/src/app/chatStream.ts +++ b/ui-tui/src/app/chatStream.ts @@ -415,9 +415,13 @@ const dispatch = ( return } // The model ended the turn on its silence marker: the line below is all - // the turn has to say, and any preview already streamed is taken back. + // the turn has to say, and what the detail says the reply had already + // streamed is taken back. The detail is that text, so the line omits it. if (event.payload.kind === 'silent_turn') { - turnController.recordSilentTurn() + turnController.recordSilentTurn(event.payload.detail ?? '') + turnController.recordNotice(noticeLine({ kind: 'silent_turn' })) + + return } // Runtime prose, not the model's: never merged into the streamed answer. // Handed to the turn rather than appended here, because it arrives mid-turn diff --git a/ui-tui/src/app/turnController.ts b/ui-tui/src/app/turnController.ts index 08fd5f636..e32da39b8 100644 --- a/ui-tui/src/app/turnController.ts +++ b/ui-tui/src/app/turnController.ts @@ -552,25 +552,29 @@ class TurnController { * the turn reads as the one muted line `recordNotice` carries. * * The reply streams through `bufRef` and the live `streaming` text until the - * turn lands, so whatever of it was already drawn is taken back here and the - * commit in `recordMessageComplete` writes none of it. Whatever an earlier - * flush took out of the buffer (`flushStreamingSegment`, chiefly run when a - * tool call starts) is in the episodes and segments as narration, not the - * reply, and it stays. - * - * A message-tool reply is the exception: that tool streams its reply as token - * deltas and sends no `tool.start`, so the reply is still in `bufRef` unless a - * flush came after it, and this would take it back. It stays on screen only - * because the server sends no silent-turn notice for a turn the message tool - * answered, whatever the model ends it on. + * turn lands. `shown` is what the server says that reply had already streamed + * (a think block it inlined ahead of the marker), the end of the buffer, so + * that is taken back here and the commit in `recordMessageComplete` writes + * none of it. On a view that joined part-way, all the buffer holds may be the + * end of it. What the buffer held before it was said on the way -- narration + * a call with no `tool.start` left there, as one whose only tool was a + * message that went nowhere does -- and moves to the segments, as a flush + * when a tool call starts moves it: it stays, and it is not the reply. */ - recordSilentTurn() { + recordSilentTurn(shown: string) { if (this.interrupted) { return } this.turnSilent = true this.streamTimer = clear(this.streamTimer) + const buf = this.bufRef + this.bufRef = buf.endsWith(shown) ? buf.slice(0, buf.length - shown.length) : shown.endsWith(buf) ? '' : buf + + if (this.bufRef.trim()) { + this.flushStreamingSegment() + } + this.bufRef = '' patchTurnState({ streaming: '' }) } diff --git a/ui-tui/src/rpc/generated.ts b/ui-tui/src/rpc/generated.ts index f62be5a73..ae541aada 100644 --- a/ui-tui/src/rpc/generated.ts +++ b/ui-tui/src/rpc/generated.ts @@ -1695,7 +1695,7 @@ export interface NoticeEvent { */ kind: string; /** - * What `kind` says it is: the blocking tool's own first line, the failed call's error category (or `empty_reply` / `thinking_only` for the wait before an empty-reply resend or a reasoning step-down), or the organ that dropped out. Empty for `silent_turn`. + * What `kind` says it is: the blocking tool's own first line, the failed call's error category (or `empty_reply` / `thinking_only` for the wait before an empty-reply resend or a reasoning step-down), or the organ that dropped out. For `silent_turn`, what the reply read as the marker had streamed before it was: a think block the model inlined ahead of it, which goes out as it comes, counted over the step it was drawn in (its call and a cut answer's continuations, or the wrap-up alone). A client that drew it takes back exactly that suffix of the step's text, and nothing it drew before. Empty when the reply showed nothing. */ detail?: string; /** diff --git a/ui-web/src/features/transcript/TranscriptPage.test.tsx b/ui-web/src/features/transcript/TranscriptPage.test.tsx index 255b5b30b..2025589a9 100644 --- a/ui-web/src/features/transcript/TranscriptPage.test.tsx +++ b/ui-web/src/features/transcript/TranscriptPage.test.tsx @@ -2904,12 +2904,13 @@ describe('transcript island, a turn the model chose not to answer', () => { const labels = (): Array => quiet().map((n) => n.querySelector('.tx')?.textContent) /* What the stage does on the notice, in its order: the open step is sealed - and its reply taken back, then the quiet row is drawn; the turn lands - flagged silent after. The notice has let go of the open step by then, so - finishTurn is handed none -- only the list of steps the turn ran. */ - const hush = (open: ReturnType | null): void => { + and what the notice says the reply showed is taken back, then the quiet row + is drawn; the turn lands flagged silent after. The notice has let go of the + open step by then, so finishTurn is handed none -- only the list of steps + the turn ran. */ + const hush = (open: ReturnType | null, shown = ''): void => { act(() => { - if (open) { open.seal(); open.dropReply() } + if (open) { open.seal(); open.dropReply(shown) } mount.note(LABEL, '', { quiet: true }) }) } @@ -2959,7 +2960,7 @@ describe('transcript island, a turn the model chose not to answer', () => { act(() => { rafQ.forEach((cb) => cb(0)) }) expect($('.say')?.textContent).toContain('abc') - hush(st) + hush(st, 'abc') act(() => { mount.finishTurn(null, [st], '3s', true) }) openFolds() @@ -2968,6 +2969,61 @@ describe('transcript island, a turn the model chose not to answer', () => { expect(labels()).toEqual([LABEL]) }) + /* An exhausted turn's wrap-up streams into the last call's step, after that + call's narration, and a call whose only tool was a message that went + nowhere drew no tool row to mark where the narration ends. Only what the + notice names is the wrap-up's. */ + it('takes back only what the closing reply showed, and keeps the narration before it', () => { + act(() => { mount.ask('anything new?') }) + let st!: ReturnType + act(() => { + st = mount.step() + st.sayDelta('Let me check the queue.') + st.sayDelta('still nothing') + }) + + hush(st, 'still nothing') + + expect(st.seg.say).toBe('Let me check the queue.') + }) + + it('keeps everything a step said when the closing reply showed nothing', () => { + act(() => { mount.ask('anything new?') }) + let st!: ReturnType + act(() => { + st = mount.step() + st.sayDelta('Checking the logs.') + st.tool('read_file', { path: '/tmp/a.log' }, null).done(true, 'quiet', 9) + st.sayDelta('Sent the summary.') + }) + + hush(st) + + expect(st.seg.say).toBe('Checking the logs.Sent the summary.') + }) + + /* A page that joined the turn part-way holds only the end of what the reply + showed: all of what it holds is the reply's, so all of it goes. */ + it('takes back a reply it saw only the end of', () => { + act(() => { mount.ask('anything new?') }) + let st!: ReturnType + act(() => { st = mount.step(); st.sayDelta('nothing') }) + + hush(st, 'still nothing') + + expect(st.seg.say).toBe('') + }) + + it('leaves a step alone when what it holds does not end on what the reply showed', () => { + act(() => { mount.ask('anything new?') }) + let st!: ReturnType + act(() => { st = mount.step(); st.sayDelta('Checking the logs.') }) + + hush(st, 'still nothing') + + expect(st.seg.say).toBe('Checking the logs.') + }) + it('keeps what the turn said on the way as narration, never as its answer', () => { act(() => { mount.ask('anything new?') }) let first!: ReturnType diff --git a/ui-web/src/features/transcript/TurnNav.test.tsx b/ui-web/src/features/transcript/TurnNav.test.tsx index 6dc755eb0..d59854d0d 100644 --- a/ui-web/src/features/transcript/TurnNav.test.tsx +++ b/ui-web/src/features/transcript/TurnNav.test.tsx @@ -302,7 +302,7 @@ describe('the turn nav', () => { const st = mount.step() st.sayDelta('Checking the logs.') st.seal() - st.dropReply() + st.dropReply('') mount.note('Raven chose not to reply.', '', { quiet: true }) mount.finishTurn(null, [st], '3s', true) mount.ask('and now?') diff --git a/ui-web/src/features/transcript/store.ts b/ui-web/src/features/transcript/store.ts index 471af06e8..dd919f5f7 100644 --- a/ui-web/src/features/transcript/store.ts +++ b/ui-web/src/features/transcript/store.ts @@ -1142,7 +1142,6 @@ export function newStep(lane: Lane): StepHandle { comes before the tools it calls, so prose after the step's last tool call is a later call's: an exhausted turn's wrap-up opens no episode and streams into the step the last call left open. */ - let replyAt = 0 const h: StepHandle = { seg, @@ -1183,8 +1182,12 @@ export function newStep(lane: Lane): StepHandle { seg.sayCaret = true scheduleFlush(lane, seg) }, - dropReply() { - seg.say = seg.say.slice(0, replyAt) + dropReply(shown: string) { + /* What the closing reply put on screen is the end of what the step holds, + or, on a page that joined the turn part-way, all of what it holds is the + end of that reply. Anything else the step said is not the reply's. */ + const say = seg.say + seg.say = say.endsWith(shown) ? say.slice(0, say.length - shown.length) : shown.endsWith(say) ? '' : say seg.hasSay = !!seg.say seg.sayCaret = false if (!seg.hasThink && !seg.calls.length && !seg.hasQA && replaceStep(lane, seg, null)) bumpList(lane) @@ -1211,7 +1214,6 @@ export function newStep(lane: Lane): StepHandle { poke(lane) paintWork(lane, seg, grew) bumpList(lane) - replyAt = seg.say.length return { done: (ok, res, ms, diff, truncated) => callDone(lane, seg, c, ok, res, ms, diff, truncated), spawnTask: (taskId) => { c.spawnTaskId = String(taskId || '') }, diff --git a/ui-web/src/features/transcript/types.ts b/ui-web/src/features/transcript/types.ts index abd246287..5befd1576 100644 --- a/ui-web/src/features/transcript/types.ts +++ b/ui-web/src/features/transcript/types.ts @@ -354,12 +354,12 @@ export interface StepHandle { setThinkOpen(open: boolean): void setSay(text: string): void sayDelta(text: string): void - /* Take back the reply the step's latest model call streamed -- the prose - after the step's last tool call, or all of it in a step that made none: - the model ended the turn on its silence marker, so that reply is not an - answer. Narration an earlier call said before its tools stays, as a reload - draws it. A step left with nothing else on it is removed. */ - dropReply(): void + /* Take back `shown`, what the server says the reply it read as the silence + marker had already streamed (a think block it inlined ahead of the marker): + that reply is not an answer. Only that suffix of the step's prose goes, so + narration before it stays, as a reload draws it. A step left with nothing + else on it is removed. */ + dropReply(shown: string): void tool(name: string, args: unknown, display?: string | null, callId?: string | null): CallHandle seal(): void } diff --git a/ui-web/src/rpc/generated.ts b/ui-web/src/rpc/generated.ts index a76d3f49a..c48944d9a 100644 --- a/ui-web/src/rpc/generated.ts +++ b/ui-web/src/rpc/generated.ts @@ -1379,7 +1379,7 @@ export interface NoticeEvent { */ kind: string; /** - * What `kind` says it is: the blocking tool's own first line, the failed call's error category (or `empty_reply` / `thinking_only` for the wait before an empty-reply resend or a reasoning step-down), or the organ that dropped out. Empty for `silent_turn`. + * What `kind` says it is: the blocking tool's own first line, the failed call's error category (or `empty_reply` / `thinking_only` for the wait before an empty-reply resend or a reasoning step-down), or the organ that dropped out. For `silent_turn`, what the reply read as the marker had streamed before it was: a think block the model inlined ahead of it, which goes out as it comes, counted over the step it was drawn in (its call and a cut answer's continuations, or the wrap-up alone). A client that drew it takes back exactly that suffix of the step's text, and nothing it drew before. Empty when the reply showed nothing. */ detail?: string; /** diff --git a/ui-web/src/state/session/pipeline.events.test.ts b/ui-web/src/state/session/pipeline.events.test.ts index af9a4c649..e2698a4ac 100644 --- a/ui-web/src/state/session/pipeline.events.test.ts +++ b/ui-web/src/state/session/pipeline.events.test.ts @@ -20,7 +20,7 @@ type Pipeline = typeof import('./pipeline') interface Row { id: string; title?: string; status?: string | null; last?: string; naming?: boolean } interface Step { seal(): void - dropReply(): void + dropReply(shown: string): void thinkAppend(text: string): void sayDelta(text: string): void tool(name: string, args: unknown, display: unknown, callId: string): { done(...a: unknown[]): void } @@ -40,7 +40,7 @@ async function harness({ const step = (): Step => { const st: Step = { seal: () => log.push(['seal']), - dropReply: () => log.push(['dropReply']), + dropReply: (shown: string) => log.push(['dropReply', shown]), thinkAppend: (text: string) => log.push(['thinkAppend', text]), sayDelta: (text: string) => log.push(['sayDelta', text]), tool: (name: string, args: unknown, display: unknown, callId: string) => { @@ -445,6 +445,16 @@ describe('notice', () => { expect(h.live.silent).toBe(true) }) + it('takes back what the notice says the reply showed, and draws none of it in the row', async () => { + const h = await harness() + h.dispatch({ type: 'token.delta', payload: { text: 'Let me check.nothing new' } }) + + h.dispatch({ type: 'notice', payload: { kind: 'silent_turn', detail: 'nothing new' } }) + + expect(h.did('dropReply')).toEqual([['dropReply', 'nothing new']]) + expect(h.did('noteRow')).toEqual([['noteRow', 'gui.notice.silent_turn', '', ['quiet']]]) + }) + it('marks the turn when no step was open, with nothing to take back', async () => { const h = await harness() @@ -649,6 +659,7 @@ describe('a turn that ends silent, drawn by the real transcript island', () => { afterEach(() => { vi.unstubAllGlobals() }) const SILENT = { type: 'notice', payload: { kind: 'silent_turn', detail: '', transient: false } } + const silentShowing = (shown: string) => ({ type: 'notice', payload: { kind: 'silent_turn', detail: shown, transient: false } }) it('keeps the narration of the step an exhausted turn\'s wrap-up streamed into, as a reload does', async () => { /* The wire of a person's turn that ran out of iterations: one episode per @@ -704,10 +715,10 @@ describe('a turn that ends silent, drawn by the real transcript island', () => { }) it('takes back only what the wrap-up streamed when it did stream its marker', async () => { - /* A model that inlines its thinking in the reply: the server's hold lets - the reply through at its first character, and only once the call ended - and the thinking was stripped does the loop read a marker. That stream is - the one the notice takes back, and the narration before it stays. */ + /* Whatever the closing reply streamed -- here a thought and the marker + after it, as a server that did not hold the marker back would send them -- + is what the notice names. Only that is taken back, and the narration the + step said before it stays. */ const h = await islandHarness() h.dispatch({ type: 'message.start', payload: { turn_id: 't1', content: 'tidy the logs' } }) h.dispatch({ type: 'episode.start', payload: { index: 0 } }) @@ -716,7 +727,7 @@ describe('a turn that ends silent, drawn by the real transcript island', () => { h.dispatch({ type: 'tool.complete', payload: { tool_call_id: 'c1', result_preview: 'quiet', ok: true } }) h.dispatch({ type: 'token.delta', payload: { text: 'nothing new' } }) h.dispatch({ type: 'token.delta', payload: { text: '[SILENT]' } }) - h.dispatch(SILENT) + h.dispatch(silentShowing('nothing new[SILENT]')) h.dispatch({ type: 'message.complete', payload: { turn_id: 't1', usage: {}, duration_ms: 4000 } }) const live = h.seen() @@ -733,7 +744,7 @@ describe('a turn that ends silent, drawn by the real transcript island', () => { h.dispatch({ type: 'message.start', payload: { turn_id: 't1', content: 'anything new?' } }) h.dispatch({ type: 'episode.start', payload: { index: 0 } }) h.dispatch({ type: 'token.delta', payload: { text: 'nothing new[SILENT]' } }) - h.dispatch(SILENT) + h.dispatch(silentShowing('nothing new[SILENT]')) h.dispatch({ type: 'message.complete', payload: { turn_id: 't1', usage: {}, duration_ms: 2000 } }) const live = h.seen() diff --git a/ui-web/src/state/session/stages.ts b/ui-web/src/state/session/stages.ts index c6107638b..b9a864ab8 100644 --- a/ui-web/src/state/session/stages.ts +++ b/ui-web/src/state/session/stages.ts @@ -197,11 +197,11 @@ export const STAGES: readonly Stage[] = [ if (p.transient) { transcript.status(t('gui.notice.' + (p.kind || ''), null, p.kind || '')); return } transcript.killStatus() /* The model ended the turn on its silence marker, so the turn has no answer: - the reply the open step was streaming is taken back, and the turn lands - without promoting anything it said on the way. The row below is all of it. - The server holds a marker back while it streams -- past a think block the - model inlined, which streams as it comes, and across a stream cut and its - continuation -- so there is usually nothing to take back. An exhausted + what the notice's detail says that reply showed is taken back from the open + step, and the turn lands without promoting anything it said on the way. The + row below is all of it. The server holds a marker back while it streams, so + the detail is at most a think block the model inlined ahead of it, and is + empty when nothing went out; it is not drawn in the row. An exhausted turn's wrap-up shares its step with the last call's narration, which stays. Not marked while this page is cancelling the turn: the stop already folded it, and the message.complete that would read and clear the mark is @@ -212,11 +212,11 @@ export const STAGES: readonly Stage[] = [ above stays where it was said. */ if (rt.st) { rt.st.seal() - if (silent) rt.st.dropReply() + if (silent) rt.st.dropReply(p.detail || '') rt.st = null } flushSay(rt) - noteRow(t('gui.notice.' + (p.kind || ''), null, p.kind || ''), p.detail || '', { quiet: true }) + noteRow(t('gui.notice.' + (p.kind || ''), null, p.kind || ''), silent ? '' : p.detail || '', { quiet: true }) }), arm('thinking.delta', (rt, p) => { From 6b2efabf522af03d6a7e5706acbcc58219ee1729 Mon Sep 17 00:00:00 2001 From: Dizhan Xue Date: Fri, 30 Oct 2026 08:57:41 +0000 Subject: [PATCH 09/13] fix(session): keep a save onto a code-page transcript failing Catching UnicodeDecodeError inside _scan_file kept a transcript saved in a Windows code page out of the session list, but the save path and append_metadata_patch read through the same scan. A turn on such a transcript then took the file for an empty one: the save appended UTF-8 lines no reader could open, so the turn was lost and nothing said so, and append_metadata_patch reported nothing to patch. Before the catch the save raised and wrote nothing. The catch moves to the two readers that only list, the session list and find_most_recent_chat_id, through _scan_listed. A save or a metadata patch onto such a file raises again and leaves it as it was. --- raven/session/manager.py | 21 ++++++++++++++++----- tests/test_session_manager.py | 30 ++++++++++++++++++++++++++++-- 2 files changed, 44 insertions(+), 7 deletions(-) diff --git a/raven/session/manager.py b/raven/session/manager.py index fb26eea95..0d41f2d19 100644 --- a/raven/session/manager.py +++ b/raven/session/manager.py @@ -825,7 +825,7 @@ def find_most_recent_chat_id( else: candidates = self.sessions_dir.glob("*/*.jsonl") for p in candidates: - meta, count, _last, _first, _preview = self._scan_file(p) + meta, count, _last, _first, _preview = self._scan_listed(p) if meta is None: continue key_val = meta.get("key", "") @@ -941,12 +941,23 @@ def _scan_file(path: Path) -> tuple[dict[str, Any] | None, int, str, str, str]: last_ts = ts elif before_runtime_turn is not None and not replied_via_message_tool(data): latest_preview, last_ts = before_runtime_turn - except (OSError, UnicodeDecodeError): - # A transcript that is not UTF-8 (one saved in a Windows code page) is left out as an - # unreadable one is, rather than its decoding error escaping the whole list. + except OSError: return None, 0, "", "", "" return meta, count, last_ts, first_user, latest_preview + def _scan_listed(self, path: Path) -> tuple[dict[str, Any] | None, int, str, str, str]: + """``_scan_file`` for a reader that only lists: a transcript that is not UTF-8 (one saved in a + Windows code page) is left out as an unreadable one is, rather than its decoding error escaping + the whole scan. + + Only for such a reader. A save reading it that way would take the file for an empty one and + append UTF-8 lines no reader can open, so on the way to a write the error still stops it. + """ + try: + return self._scan_file(path) + except UnicodeDecodeError: + return None, 0, "", "", "" + def get_or_create(self, key: str) -> Session: """ Get an existing session or create a new one. @@ -1366,7 +1377,7 @@ def list_sessions( sessions = [] for path in self.sessions_dir.glob("*/*.jsonl"): - data, message_count, last_ts, first_user, latest_preview = self._scan_file(path) + data, message_count, last_ts, first_user, latest_preview = self._scan_listed(path) if data is None: continue key = data.get("key") or self.key_from_path(path) diff --git a/tests/test_session_manager.py b/tests/test_session_manager.py index c6f7b1e9a..3cb49b61f 100644 --- a/tests/test_session_manager.py +++ b/tests/test_session_manager.py @@ -355,6 +355,10 @@ def test_a_transcript_saved_with_a_byte_order_mark_is_still_listed(tmp_path: Pat assert rows[0]["first_user_message"] == "hello" +def _in_a_legacy_code_page(path: Path) -> None: + path.write_bytes(path.read_text(encoding="utf-8").replace("\\u4f60\\u597d", "\u4f60\u597d").encode("cp936")) + + def test_a_transcript_in_a_legacy_code_page_is_left_out_without_breaking_the_list(tmp_path: Path): """A transcript saved in a Windows ANSI code page is not UTF-8. It is left out as an unreadable file is, rather than its decoding error escaping the whole list.""" @@ -363,12 +367,34 @@ def test_a_transcript_in_a_legacy_code_page_is_left_out_without_breaking_the_lis session = mgr.get_or_create(key) session.add_message("user", "\u4f60\u597d") mgr.save(session) - path = tmp_path / "sessions" / "tui" / "cp01.jsonl" - path.write_bytes(path.read_text(encoding="utf-8").replace("\\u4f60\\u597d", "\u4f60\u597d").encode("cp936")) + _in_a_legacy_code_page(tmp_path / "sessions" / "tui" / "cp01.jsonl") keys = [info["key"] for info in SessionManager(tmp_path).list_sessions()] assert keys == ["tui:good01"] + assert SessionManager(tmp_path).find_most_recent_chat_id("tui") == "good01" + + +def test_a_turn_on_a_transcript_in_a_legacy_code_page_fails_without_writing(tmp_path: Path): + """Leaving such a transcript out of the list must not reach a write: UTF-8 lines appended to a file + no reader can open lose the turn without a word, where failing the save reports it.""" + mgr = SessionManager(tmp_path) + session = mgr.get_or_create("tui:cp02") + session.add_message("user", "\u4f60\u597d") + mgr.save(session) + path = tmp_path / "sessions" / "tui" / "cp02.jsonl" + _in_a_legacy_code_page(path) + before = path.read_bytes() + + fresh = SessionManager(tmp_path) + opened = fresh.get_or_create("tui:cp02") + opened.add_message("user", "again") + with pytest.raises(UnicodeDecodeError): + fresh.save(opened) + with pytest.raises(UnicodeDecodeError): + fresh.append_metadata_patch("tui:cp02", {"archived": True}) + + assert path.read_bytes() == before def test_list_sessions_sees_nested_layout(tmp_path: Path): From ecadc3bc634897ac81e756089bca67543d9eb32f Mon Sep 17 00:00:00 2001 From: Dizhan Xue Date: Fri, 30 Oct 2026 08:57:41 +0000 Subject: [PATCH 10/13] fix(session): open a transcript saved with a BOM with its metadata The session list reads a transcript as utf-8-sig, but the loader (read_session_file) and _stored_key still read plain UTF-8. A file a Windows editor saved with a byte order mark was listed with its title and archive flag, opened without them because its metadata line did not parse, and the first save then wrote over both. _stored_key found no key in it, so session_path would hand the file to a session of any channel with the same chat id. Both now read utf-8-sig, as the list does. A file with no mark reads as before. --- raven/session/manager.py | 9 +++++++-- tests/test_session_manager.py | 24 ++++++++++++++++++++++++ 2 files changed, 31 insertions(+), 2 deletions(-) diff --git a/raven/session/manager.py b/raven/session/manager.py index 0d41f2d19..d51ead235 100644 --- a/raven/session/manager.py +++ b/raven/session/manager.py @@ -75,7 +75,10 @@ def _stored_key(path: Path) -> str | None: treats the two differently. """ try: - with open(path, encoding="utf-8") as f: + # utf-8-sig, as every reader of a transcript: a byte order mark in front of the metadata line + # would leave it unparsed, and a file claiming no key goes to a session of any channel with + # the same chat id. + with open(path, encoding="utf-8-sig") as f: head = f.readline() except OSError: return None @@ -533,7 +536,9 @@ def read_session_file(path: Path, key: str) -> Session | None: revision: tuple[str, int, str | None] | None = None created_raw = None - with open(path, encoding="utf-8") as f: + # utf-8-sig, as the session list reads it: a byte order mark in front of the metadata line + # would leave it unparsed, and the session would open without its title or archive flag. + with open(path, encoding="utf-8-sig") as f: # Of the file this read, taken before reading it: a write that lands # while the file is read (or replaces it) leaves the stamp behind the # file, so the next save re-reads and merges rather than writing over diff --git a/tests/test_session_manager.py b/tests/test_session_manager.py index 3cb49b61f..4a5b1bbf2 100644 --- a/tests/test_session_manager.py +++ b/tests/test_session_manager.py @@ -355,6 +355,30 @@ def test_a_transcript_saved_with_a_byte_order_mark_is_still_listed(tmp_path: Pat assert rows[0]["first_user_message"] == "hello" +def test_a_transcript_saved_with_a_byte_order_mark_opens_with_what_the_list_shows(tmp_path: Path): + """Opening reads the metadata line the list reads. Read as plain UTF-8 it did not parse: the session + opened without its title and archive flag, the next save wrote over both, and the file claimed no key.""" + mgr = SessionManager(tmp_path) + session = mgr.get_or_create("tui:bom02") + session.add_message("user", "hello") + session.metadata.update(title="My Title", archived=True) + mgr.save(session) + path = tmp_path / "sessions" / "tui" / "bom02.jsonl" + path.write_bytes(b"\xef\xbb\xbf" + path.read_bytes()) + + assert manager_mod._stored_key(path) == "tui:bom02" + fresh = SessionManager(tmp_path) + opened = fresh.get_or_create("tui:bom02") + assert opened.metadata.get("title") == "My Title" + assert opened.metadata.get("archived") is True + opened.add_message("user", "again") + fresh.save(opened) + + rows = [info for info in SessionManager(tmp_path).list_sessions() if info["key"] == "tui:bom02"] + assert rows[0]["metadata"].get("title") == "My Title" + assert rows[0]["metadata"].get("archived") is True + + def _in_a_legacy_code_page(path: Path) -> None: path.write_bytes(path.read_text(encoding="utf-8").replace("\\u4f60\\u597d", "\u4f60\u597d").encode("cp936")) From 25aa46e88546ff30e82dd29b5a50fe536cfba2b7 Mon Sep 17 00:00:00 2001 From: Dizhan Xue Date: Fri, 30 Oct 2026 08:57:41 +0000 Subject: [PATCH 11/13] fix(agent): treat a turn that delivered files as answered live too A turn whose tools delivered files is stamped replied_via_tool, so the session list and the ACP replay keep it as answered. Live, a turn that delivered files and then ended on the silence marker was still a Silent Turn: a SILENT_TURN notice went out and TurnOutcome.silent was true. Cron therefore left it out of silent_fire_limit and the sentinel ledger, as if it had sent the user nothing. Such a turn is now answered live as well: no SILENT_TURN notice, TurnOutcome.silent false, and explicit_reply true, which _run_turn learns from the silence sink ("delivered_files"), so cron counts it and does not hand it to heartbeat as a run that said nothing. The files already went out with the call that delivered them, so a turn that delivers and then answers in words still sends the words. --- CONTEXT.md | 11 +++---- docs-site/docs/proactivity-design.md | 14 +++++---- docs-site/docs/proactivity-design.zh.md | 6 ++-- raven/agent/loop/turn_path.py | 19 +++++++++-- tests/test_agent_loop_empty_recovery.py | 23 ++++++++++++-- tests/test_agent_loop_run_emit.py | 42 +++++++++++++++++++++++++ 6 files changed, 96 insertions(+), 19 deletions(-) diff --git a/CONTEXT.md b/CONTEXT.md index 86510d1cd..d31dc651c 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -858,15 +858,14 @@ The `after_send` hook chain still runs, with queued for the next reply, and the personalizer learns nothing; text a hook composes there is delivered as the reply, the closing row is filed as that text rather than the marker, and the turn is then not silent. A turn the message tool has already answered (`MessageTool.sent_in_turn`: a send -that went out, which the tool hands to the turn's own reader whatever chat the call names) is not a -Silent Turn whatever it ends on -- its answer reached -the reader, so no -`SILENT_TURN` notice goes out; a marker it ends on is still filed `silent: true`, so a reload +that went out, which the tool hands to the turn's own reader whatever chat the call names), or one in +which a tool delivered files the reader now holds, is not a Silent Turn whatever it ends on -- its +answer reached the reader, so no `SILENT_TURN` notice goes out and the turn reports an explicit +reply rather than `TurnOutcome.silent`; a marker it ends on is still filed `silent: true`, so a reload draws the muted line after the delivered reply where the live view drew none. The turn's last assistant row with text or tool calls -- the marker's row when it ends on one, else, after an empty closing reply, which is not stored, its last tool call, not necessarily the send; never a notice the -runtime files after the turn -- carries that live verdict, or that a tool delivered files the reader -now holds, as `replied_via_tool`, and the two +runtime files after the turn -- carries that live verdict as `replied_via_tool`, and the two surfaces that read a stored turn -- the ACP replay, which draws it, and the session list, which orders sessions by it -- read the flag from one place (`session/turn_shape.py`) rather than guess it from the calls they can see, where a send diff --git a/docs-site/docs/proactivity-design.md b/docs-site/docs/proactivity-design.md index fb204beae..6f2686ae3 100644 --- a/docs-site/docs/proactivity-design.md +++ b/docs-site/docs/proactivity-design.md @@ -203,9 +203,10 @@ check-and-reserve transaction: execution path, not a second pass through ProactiveSpawn for every action. - Cron is explicitly user-scheduled and bypasses `check()`. With a Sentinel runner wired, successful fires update its shared counters and topic ledger, - except a run the model ends with `[SILENT]`: it nudged no one, so it records - nothing. This helps suppress overlapping Sentinel reminders; it is not a - guarantee of semantic deduplication across all messages. + except a run the model ends with `[SILENT]` before any tool reached the user + (a message sent, files delivered): it nudged no one, so it records nothing. + This helps suppress overlapping Sentinel reminders; it is not a guarantee of + semantic deduplication across all messages. ## State and feedback @@ -284,9 +285,10 @@ broadcast. `raven/core/cron_stack.py` submits the work as a `CRON` turn in Successful recurring fires increase `silent_fire_count`; matching user activity resets it. Reaching `silent_fire_limit` (default `12`) disables the job. This counts fires without user activity, not failures. A run the model -ends with `[SILENT]` is not counted either: it sent the user nothing to -respond to, so a check that mostly stays quiet is not disabled for it. Cron's -feedback record is marked neutral so it does not lower Sentinel's learned +ends with `[SILENT]` before any tool reached the user (a message sent, files +delivered) is not counted either: it sent the user nothing to respond to, so a +check that mostly stays quiet is not disabled for it. Cron's feedback record is +marked neutral so it does not lower Sentinel's learned acceptance rate. Fixed-delay `every` schedules compute their next run from completion, not from the previous due time. diff --git a/docs-site/docs/proactivity-design.zh.md b/docs-site/docs/proactivity-design.zh.md index 364eb2a9f..08f6c6a4a 100644 --- a/docs-site/docs/proactivity-design.zh.md +++ b/docs-site/docs/proactivity-design.zh.md @@ -163,7 +163,8 @@ ProactiveSpawn 校验任务，以任务文字作为去重内容检查共享策 在空闲检查后直接投递，不重新检查策略，因此没有完整的发送时配额或免打扰保障。 - 任务发现菜单受策略约束。用户选择后的执行是独立路径，并非每种动作都再经 ProactiveSpawn。 - Cron 是用户显式安排的任务，绕过 `check()`。接入 Sentinel runner 时，成功触发会更新 - 共享计数与话题账本；模型以 `[SILENT]` 结束的运行没有提醒任何人，不做记录。 + 共享计数与话题账本；模型以 `[SILENT]` 结束、此前也没有工具触达用户（发出消息、投递文件）的运行 + 没有提醒任何人，不做记录。 这有助于抑制重叠的 Sentinel 提醒，但不能保证所有消息语义上绝不重复。 ## 状态与反馈 { #state-and-feedback } @@ -229,7 +230,8 @@ CronService（`schedulers/cron/service.py`）在文件锁下持久化任务、 周期任务成功触发会增加 `silent_fire_count`，匹配的用户活动会将其重置。 达到 `silent_fire_limit`（默认 `12`）后任务被禁用。这里计数的是没有用户活动的触发， -不是失败次数。模型以 `[SILENT]` 结束的运行同样不计数：它没有给用户发任何内容， +不是失败次数。模型以 `[SILENT]` 结束、此前也没有工具触达用户（发出消息、投递文件）的运行 +同样不计数：它没有给用户发任何内容， 用户无从回应，所以多数时候保持沉默的检查类任务不会因此被禁用。 Cron 的反馈记录记为中性，不会拉低 Sentinel 学习到的接受率。 固定延迟的 `every` 计划从完成时刻计算下一次运行时间，而不是从上一次应触发时刻计算。 diff --git a/raven/agent/loop/turn_path.py b/raven/agent/loop/turn_path.py index c606541b1..b12320921 100644 --- a/raven/agent/loop/turn_path.py +++ b/raven/agent/loop/turn_path.py @@ -2828,6 +2828,9 @@ async def _process_message( # noqa: C901 (cc 47: pre-existing, above the ceilin ``"silent": True`` for the last of those: a turn the model ended on a marker, and that nothing replaced with a reply of its own -- and, when that reply had streamed anything first, ``"shown"`` (``LoopOutcome.shown``). + It receives ``"delivered_files": True`` for a turn a tool handed the + reader files in: those went out with the call, so such a turn answered + whatever it ends on, and is never a Silent Turn. ``reply_awaited`` keeps a turn whose caller waits on its text out of the silence protocol (``_run_agent_loop``). """ @@ -3514,8 +3517,11 @@ def _rerun_pending(final_content: str | None, msgs: list[dict], status: str) -> # A hook composed the reply the reader is sent in place of the marker. A turn the # message tool answered sends no reply of its own, so its marker row stays. _file_composed_reply(turn_msgs, final_content) - if sent_by_tool or _delivered_files(turn_msgs[turn_start_idx:]): + delivered_files = _delivered_files(turn_msgs[turn_start_idx:]) + if sent_by_tool or delivered_files: _stamp_replied_via_tool(turn_msgs, turn_start_idx) + if delivered_files and silence is not None: + silence["delivered_files"] = True if len(self.hooks) > 0: _stamp_turn_observers(turn_msgs, turn_hook_meta, turn_start_idx) @@ -3577,6 +3583,12 @@ async def _post_learn(): ) return None + if silent and delivered_files: + # The files reached the reader with the call that delivered them, so the turn answered, + # by the verdict filed above: not a Silent Turn, and the marker is no reply to send. + logger.info("Turn for {}:{} delivered files and ended on the silence marker", channel, sender_id) + return None + if silent: logger.info("Silent turn for {}:{}: the model chose not to reply", channel, sender_id) if silence is not None: @@ -4364,6 +4376,7 @@ async def _route_to_stream(content: str, media: list[str]) -> None: total_tokens=int(usage_sink.get("total_tokens", 0) or 0), ) # A message-tool reply returns None from _process_message but did reply, - # so it counts as an explicit reply too. - replied_via_tool = isinstance(message_tool, MessageTool) and message_tool.sent_in_turn + # so it counts as an explicit reply too, and so do files a tool delivered. + sent_by_tool = isinstance(message_tool, MessageTool) and message_tool.sent_in_turn + replied_via_tool = sent_by_tool or silence.get("delivered_files") is True return TurnOutcome(usage=usage, explicit_reply=out is not None or replied_via_tool, silent=silent) diff --git a/tests/test_agent_loop_empty_recovery.py b/tests/test_agent_loop_empty_recovery.py index f87e7c92f..d386d5822 100644 --- a/tests/test_agent_loop_empty_recovery.py +++ b/tests/test_agent_loop_empty_recovery.py @@ -5434,16 +5434,22 @@ def take_metadata(self) -> dict: @pytest.mark.asyncio async def test_a_silent_runtime_turn_that_delivered_files_is_not_taken_back(workspace, monkeypatch): """Files a turn delivered are in front of the reader, as a message the tool sent is: a turn that hands - over a file and closes on the marker is kept by the session list and the ACP replay.""" + over a file and closes on the marker is no Silent Turn live, and the session list and the ACP replay + keep it by the same verdict.""" from raven.acp.replay import replay from raven.rpc.methods.session import _map_to_wire _served(monkeypatch, [_tool_reply("deliver_files", "{}"), _reply("[SILENT]")]) agent = _litellm_agent(workspace) agent.tools.register(_DeliversAFile()) + silence: dict[str, bool | str] = {} - await agent._process_message(_turn("check the build", Origin.CRON), session_key="cron:job", origin=Origin.CRON) + out = await agent._process_message( + _turn("check the build", Origin.CRON), session_key="cron:job", origin=Origin.CRON, silence=silence + ) + assert out is None, "the marker is no reply to send" + assert silence == {"delivered_files": True}, "the live verdict" rows = agent.sessions.get_or_create("cron:job").messages assert rows[-1]["silent"] is True and rows[-1].get("replied_via_tool") is True assert replay(_map_to_wire(rows, "cron:job"), session_id="cron:job"), "the turn replays" @@ -5451,6 +5457,19 @@ async def test_a_silent_runtime_turn_that_delivered_files_is_not_taken_back(work assert listed["last_message_at"], "and stands in the session list" +@pytest.mark.asyncio +async def test_a_turn_that_delivered_files_still_sends_the_words_it_ends_on(workspace, monkeypatch): + """The files went out with the call that delivered them, so they are not the turn's reply the way a + message the tool sent is: a turn that delivers and then answers in words still sends the words.""" + _served(monkeypatch, [_tool_reply("deliver_files", "{}"), _reply("Here is the report.")]) + agent = _litellm_agent(workspace) + agent.tools.register(_DeliversAFile()) + + out = await agent._process_message(_turn("send me the report"), session_key="s1") + + assert out is not None and out[0] == "Here is the report." + + @pytest.mark.asyncio async def test_a_turn_the_message_tool_answered_with_no_reply_of_its_own_files_the_verdict(workspace, monkeypatch): """The commonest shape: the tool sends, and the model then says nothing. The empty closing row is not diff --git a/tests/test_agent_loop_run_emit.py b/tests/test_agent_loop_run_emit.py index 74cda7334..58ac633f2 100644 --- a/tests/test_agent_loop_run_emit.py +++ b/tests/test_agent_loop_run_emit.py @@ -1997,3 +1997,45 @@ async def test_a_marker_after_the_message_tool_sent_does_not_make_the_turn_silen assert [e.delta for e in sink.events if isinstance(e, EvStreamDelta)] == ["here you go"] assert not any(isinstance(e, EvNotice) and e.kind is NoticeKind.SILENT_TURN for e in sink.events) assert outcome.silent is False and outcome.explicit_reply is True + + +class _DeliversAFile(_FakeTool): + """Hands the reader one file, leaving its manifest for the turn stream as ``deliver_files`` does.""" + + @property + def name(self) -> str: + return "deliver_files" + + def take_metadata(self) -> dict: + return {"raven_delivery": {"message": "", "files": [{"name": "report.txt", "size": 1024}], "invalid": []}} + + +async def test_a_marker_after_a_file_delivery_does_not_make_the_turn_silent(tmp_path): + """The files reached the reader with the call that delivered them, so the turn answered as one the + message tool answered does: no notice that it was silent, and the spine is told it replied -- a cron + run that delivered is counted against its limit and is not handed to heartbeat as one that said nothing.""" + provider = _FakeStreamToolProvider( + [ + [ + ChatDelta( + content=None, + tool_call_delta={ + "tool_calls": [ + {"index": 0, "id": "d1", "function": {"name": "deliver_files", "arguments": "{}"}} + ] + }, + ) + ], + [ChatDelta(content="[SILENT]")], + ] + ) + loop = AgentLoop(provider=provider, workspace=tmp_path) + _stub_edges(loop) + loop.tools.register(_DeliversAFile()) + sink = _EmitCollector() + + outcome = await loop.run_turn(_req("send me the report"), sink, _drain) + + assert not any(isinstance(e, EvNotice) and e.kind is NoticeKind.SILENT_TURN for e in sink.events) + assert not any(isinstance(e, EvText) for e in sink.events), "the marker is no reply to send" + assert outcome.silent is False and outcome.explicit_reply is True From 45356cfad22b956b2c64004f7524eec3d2f1997e Mon Sep 17 00:00:00 2001 From: Dizhan Xue Date: Fri, 30 Oct 2026 08:57:41 +0000 Subject: [PATCH 12/13] docs: say how long a continuation that raises waits before it ends CONTEXT.md put a continuation that raises in the same sentence as one that answers with an error, which ends the turn "with no wait". A raising continuation ends on the partial too, but only after stream_llm_call has spent its reconnects and, on a failure it still reads as transient, waited out llm_error_retry_delays (15, 30 and 60 seconds by default), with no retry notice shown. The entry now says so in a sentence of its own. --- CONTEXT.md | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/CONTEXT.md b/CONTEXT.md index d31dc651c..eb881c08d 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -908,10 +908,12 @@ nothing of it, so there such a marker is read with what came before it, on its o reply uncut would be. Either way a continuation inside a think block the cut left open is read only joined to what came before it. One that comes back as an error, a stream cut off before any new text included, ends the turn at once on the partial with the note, as `interrupted`, with no wait and no -fallback model, and so does one that raises once the stream's own reconnects are spent -- and so -does a turn that runs out -of iterations or time while a continuation is still owed, with no wrap-up. On each of those exits -the reply the turn ends on is the joined parts with their closed think blocks taken out of the whole +fallback model, and so does a turn that runs out of iterations or time while a continuation is still +owed, with no wrap-up. One that raises ends on the partial the same way, but not at once: only after +`stream_llm_call` has spent its reconnects and, on a failure it still reads as transient, waited out +`llm_error_retry_delays` (15, 30 and 60 seconds by default), with no retry notice shown. +On each of those exits the reply the turn ends on is the joined parts with their closed think blocks +taken out of the whole -- a cut can split one -- and a block still unclosed left in as text, read for a Silence Marker by the turn's rule as a reply with no cut is, and one that is a marker ends the turn as a **Silent Turn**: nothing of it shown, its row filed From 755e2e86801075575a9cebff9f49a57e451c9d5d Mon Sep 17 00:00:00 2001 From: Dizhan Xue Date: Fri, 30 Oct 2026 08:57:41 +0000 Subject: [PATCH 13/13] refactor(*): read which turns are taken back from one shared rule Three readers decided which turns are taken back whole: the session list over stored rows, the ACP replay over wire rows, and the live turn, which skips the unanswered-question notice after a marker that both stored readers drop. Each re-derived the rule, and they already disagreed on one shape: the replay closed a turn at its marker, while the session list and the live turn kept it open, so a second marker after a row in between took that row back from the list but not from the replay. The rule is now TakeBack in session/turn_shape.py, which reads a transcript row by row and says where a turn that can be taken back opens and where one is taken back. All three readers use it; the live turn hands it the rows as they will be stored, without the recovery nudges that never are. A marker closes its turn for all three. A test feeds the same turns to the three readers, with and without a nudge and with the opening row filed by an earlier write, and holds their answers equal. --- CONTEXT.md | 8 ++- raven/acp/replay.py | 33 +++++------ raven/agent/loop/turn_path.py | 49 ++++++++++------ raven/session/manager.py | 49 ++++++++-------- raven/session/turn_shape.py | 45 ++++++++++++++- tests/test_session_turn_shape.py | 98 +++++++++++++++++++++++++++++++- 6 files changed, 216 insertions(+), 66 deletions(-) diff --git a/CONTEXT.md b/CONTEXT.md index eb881c08d..bd5a4044c 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -869,9 +869,11 @@ runtime files after the turn -- carries that live verdict as `replied_via_tool`, surfaces that read a stored turn -- the ACP replay, which draws it, and the session list, which orders sessions by it -- read the flag from one place (`session/turn_shape.py`) rather than guess it from the calls they can see, where a send -that failed would look like an answer. A Silent Turn the runtime opened, which those two surfaces -take back whole, files no unanswered-question notice after its marker, so none is left behind as an -orphan; a person's Silent Turn, which they keep, still files it. +that failed would look like an answer. Which turns they take back whole is one rule in the same +module (`TakeBack`), read by both and by the live turn: a turn the runtime opened that ends on a +marker no tool answered goes, the marker's row closing it. Such a turn files no unanswered-question +notice after its marker, so none is left behind as an orphan; a person's Silent Turn, which they +keep, still files it. Distinct from a `no_output` failure. A Silent Turn is the model saying nothing on purpose, so it succeeds; `no_output` is what a turn is failed with when its last reply was empty -- no text, no tool call, no reasoning -- once every retry was spent, or, with recovery switched off, when its diff --git a/raven/acp/replay.py b/raven/acp/replay.py index abb3dc432..44bd233a8 100644 --- a/raven/acp/replay.py +++ b/raven/acp/replay.py @@ -39,7 +39,7 @@ from raven.acp.tool_kinds import locations, title_for, tool_kind from raven.acp.updates import notice_text, turn_failure_text from raven.security import redact -from raven.session.turn_shape import replied_via_message_tool +from raven.session.turn_shape import TakeBack, TakeBackStep # A replayed transcript is bounded by what a client can draw, not by what is # stored: a session with two thousand messages would otherwise emit two thousand @@ -92,26 +92,23 @@ def _without_silent_runtime_turns(entries: list[dict[str, Any]]) -> list[dict[st The marker is sent as no text, so such a turn replayed as the runtime's prose for the model, drawn as a user message, with no answer under it. The whole - turn goes, the narration and tool calls on the way included, by the rule the - session list reads a transcript with (``SessionManager._scan_file``): only a - user row carrying ``origin`` opens a turn that can be taken back, so a - person's own message stays when the model answers it with silence. A turn - the message tool answered stays too: it is not a Silent Turn whatever it ends - on, and its reply reached the reader. Its marker's row carries the live turn's - verdict, read from one place (``raven/session/turn_shape.py``), so the session - list and this replay cannot come to disagree about which turn answered. + turn goes, the narration and tool calls on the way included. Which turns go is + the rule the session list and the live turn read too (``TakeBack`` in + ``raven/session/turn_shape.py``): only a user row carrying ``origin`` opens a + turn that can be taken back, so a person's own message stays when the model + answers it with silence, and a turn a tool answered stays as well, its reply + having reached the reader. """ kept: list[dict[str, Any]] = [] - opened_at: int | None = None + opened_at = 0 + take_back = TakeBack() for entry in entries: - role = entry.get("role") - if role == "user": - opened_at = len(kept) if entry.get("origin") else None - elif role == "assistant" and entry.get("silent") is True and opened_at is not None: - start, opened_at = opened_at, None - if not replied_via_message_tool(entry): - del kept[start:] - continue + step = take_back.read(entry) + if step is TakeBackStep.OPENS: + opened_at = len(kept) + elif step is TakeBackStep.TAKES_BACK: + del kept[opened_at:] + continue kept.append(entry) return kept diff --git a/raven/agent/loop/turn_path.py b/raven/agent/loop/turn_path.py index b12320921..c00b4ac6d 100644 --- a/raven/agent/loop/turn_path.py +++ b/raven/agent/loop/turn_path.py @@ -140,6 +140,7 @@ from raven.providers.first_byte import first_byte_budget from raven.providers.tool_calls import openai_tool_call from raven.session.refs import TOOL_RESULT_TRUNCATION_MARKER, tool_result_view +from raven.session.turn_shape import REPLIED_VIA_TOOL, TakeBack, TakeBackStep from raven.spine.events import bound_failure_text from raven.spine.turn import AnswerlessTurnError from raven.token_wise import usage_context @@ -477,25 +478,39 @@ def _delivered_files(turn_rows: list[dict]) -> bool: def _taken_back_whole(messages: list[dict], skip: int) -> bool: - """Whether the session list and the ACP replay will take this turn back whole. + """Whether the session list and the ACP replay will take back the row that closes this turn. - By the rule they read a transcript with: the turn's closing assistant row is a Silence Marker - the message tool did not answer, and the last user row before it carries an origin, so the - runtime opened the turn. A person's message opens one they keep. ``skip`` is where the turn's - new rows start; the row that opened it may be older, filed by the opening write. + Asked of the rule they read a transcript with (``TakeBack``), over the rows as ``_save_turn`` + will store them: a recovery nudge is never stored, and the turn's own rows carry their marks + under the live names. ``skip`` is where the turn's new rows start; the row that opened it may be + older, filed by the opening write. """ - for index in range(len(messages) - 1, skip - 1, -1): - if messages[index].get("role") == "assistant": - closing = messages[index] - break - else: - return False - if not closing.get(_SILENT_KEY) or closing.get(_REPLIED_VIA_TOOL_KEY): - return False - for row in reversed(messages[:index]): - if row.get("role") == "user" and not row.get("_recovery_synthetic"): - return bool(row.get(_ORIGIN_KEY) or row.get("origin")) - return False + # Every user row sets the rule's state afresh, so reading from the last one before the turn's new + # rows reads what the whole transcript would, without walking a long session on every save. + start = next( + ( + i + for i in range(skip - 1, -1, -1) + if messages[i].get("role") == "user" and not messages[i].get("_recovery_synthetic") + ), + 0, + ) + take_back = TakeBack() + taken = False + for index, row in enumerate(messages[start:], start): + if row.get("_recovery_synthetic"): + continue + step = take_back.read( + { + "role": row.get("role"), + "origin": row.get(_ORIGIN_KEY) or row.get("origin"), + "silent": row.get(_SILENT_KEY) or row.get("silent"), + REPLIED_VIA_TOOL: row.get(_REPLIED_VIA_TOOL_KEY) or row.get(REPLIED_VIA_TOOL), + } + ) + if index >= skip and row.get("role") == "assistant": + taken = step is TakeBackStep.TAKES_BACK + return taken def _turn_trace_id() -> str | None: diff --git a/raven/session/manager.py b/raven/session/manager.py index d51ead235..7b79cd5c3 100644 --- a/raven/session/manager.py +++ b/raven/session/manager.py @@ -16,7 +16,7 @@ from raven.contracts.context import SessionRevision from raven.session.title import TITLE_STORAGE_MAX, collapse_to_line -from raven.session.turn_shape import replied_via_message_tool +from raven.session.turn_shape import TakeBack, TakeBackStep from raven.utils.atomic_io import atomic_replace, locked_append, write_transaction from raven.utils.paths import ensure_dir, safe_filename, safe_path_segment @@ -890,13 +890,13 @@ def _scan_file(path: Path) -> tuple[dict[str, Any] | None, int, str, str, str]: tool calls the run made on the way delivered nothing either. A person's own message still counts when the model answers it with silence. - A turn the message tool answered is the exception: the tool is what - delivers a reply to the reader, so such a turn answered whatever the - model ended it on, and it stays the activity the session is ordered by. - Its marker's row carries the live turn's verdict, read from one place - (``raven/session/turn_shape.py``), the same one the ACP replay reads the - turn with, so the two surfaces cannot come to disagree about which turn - answered. + A turn a tool answered is the exception: the tool is what delivers a + reply to the reader, so such a turn answered whatever the model ended it + on, and it stays the activity the session is ordered by. Its marker's + row carries the live turn's verdict. Which turns go is read from one + place (``TakeBack`` in ``raven/session/turn_shape.py``), the one the ACP + replay and the live turn read it from, so the surfaces cannot come to + disagree about which turn is taken back. The first user message is what session pickers title an untitled session with, so its text (flattened to one line, capped) rides along. @@ -911,7 +911,8 @@ def _scan_file(path: Path) -> tuple[dict[str, Any] | None, int, str, str, str]: latest_preview = "" # The preview and the clock as they stood before the runtime-opened turn now under way, for a # turn that ends silent to be taken back whole. - before_runtime_turn: tuple[str, str] | None = None + before_runtime_turn: tuple[str, str] = ("", "") + take_back = TakeBack() try: # utf-8-sig: a Windows editor that saves UTF-8 with a byte order mark puts it in front of the # metadata line, which would then fail to parse and take the session off the list. @@ -929,23 +930,21 @@ def _scan_file(path: Path) -> tuple[dict[str, Any] | None, int, str, str, str]: else: count += 1 if isinstance(data, dict): + step = take_back.read(data) + if step is TakeBackStep.OPENS: + before_runtime_turn = (latest_preview, last_ts) + elif step is TakeBackStep.TAKES_BACK: + latest_preview, last_ts = before_runtime_turn role = data.get("role") - if role == "user": - # Only a turn the runtime opened can be taken back: a person's message - # counts even when the model answers it with silence. - before_runtime_turn = (latest_preview, last_ts) if data.get("origin") else None - if not first_user: - first_user = _message_text(data.get("content")) - if role in {"user", "assistant"}: - if data.get("silent") is not True: - text = _message_text(data.get("content")) - if text: - latest_preview = text - ts = data.get("timestamp") - if isinstance(ts, str) and ts > last_ts: - last_ts = ts - elif before_runtime_turn is not None and not replied_via_message_tool(data): - latest_preview, last_ts = before_runtime_turn + if role == "user" and not first_user: + first_user = _message_text(data.get("content")) + if role in {"user", "assistant"} and data.get("silent") is not True: + text = _message_text(data.get("content")) + if text: + latest_preview = text + ts = data.get("timestamp") + if isinstance(ts, str) and ts > last_ts: + last_ts = ts except OSError: return None, 0, "", "", "" return meta, count, last_ts, first_user, latest_preview diff --git a/raven/session/turn_shape.py b/raven/session/turn_shape.py index 2a3e53be7..79875cd0d 100644 --- a/raven/session/turn_shape.py +++ b/raven/session/turn_shape.py @@ -15,11 +15,16 @@ can see: a call that failed answered nobody. The flag has one name in both shapes a transcript comes in -- the stored row and the flattened row the wire carries -- so a caller hands over whichever it holds. + +Which turns those surfaces take back whole is the same kind of question with the same +answer owed by each, so its rule lives here too (``TakeBack``), and the live turn asks +it before filing a notice after a marker that neither surface will keep. """ from __future__ import annotations from collections.abc import Mapping +from enum import Enum from typing import Any REPLIED_VIA_TOOL = "replied_via_tool" @@ -35,4 +40,42 @@ def replied_via_message_tool(row: Mapping[str, Any]) -> bool: return isinstance(row, Mapping) and row.get("role") == "assistant" and row.get(REPLIED_VIA_TOOL) is True -__all__ = ["REPLIED_VIA_TOOL", "replied_via_message_tool"] +class TakeBackStep(Enum): + """What a row means to ``TakeBack``.""" + + OPENS = "opens" + """A user row that opens a turn which can be taken back.""" + + TAKES_BACK = "takes_back" + """The row that closes such a turn on a Silence Marker no tool answered: the turn goes, this row included.""" + + +class TakeBack: + """Reads a transcript's rows in order and says which turns are taken back whole. + + A turn the runtime opened -- its user row carries ``origin`` -- that the model closed with its + Silence Marker is taken back whole, the narration and tool calls on the way included: the marker + is sent as no text, and the runtime's prose for the model would otherwise stand as a message with + no answer under it. A person's message opens a turn they keep however the model answers it, and a + turn a tool answered (``replied_via_message_tool``) is no Silent Turn whatever it ends on. The + marker's row closes the turn either way, so a later row is not read as part of it. + + Rows come in the stored shape, keys without the live turn's underscore. + """ + + def __init__(self) -> None: + self._open = False + + def read(self, row: Mapping[str, Any]) -> TakeBackStep | None: + """What ``row``, the next in the transcript, means to the rule; None for a row that means nothing to it.""" + role = row.get("role") + if role == "user": + self._open = bool(row.get("origin")) + return TakeBackStep.OPENS if self._open else None + if role != "assistant" or row.get("silent") is not True or not self._open: + return None + self._open = False + return None if replied_via_message_tool(row) else TakeBackStep.TAKES_BACK + + +__all__ = ["REPLIED_VIA_TOOL", "TakeBack", "TakeBackStep", "replied_via_message_tool"] diff --git a/tests/test_session_turn_shape.py b/tests/test_session_turn_shape.py index 3206c77df..cb716c525 100644 --- a/tests/test_session_turn_shape.py +++ b/tests/test_session_turn_shape.py @@ -1,7 +1,11 @@ -"""What a stored transcript says about the turn it holds: did the message tool answer it?""" +"""What a stored transcript says about the turn it holds: did a tool answer it, and is it taken back?""" + +import json + +import pytest from raven.rpc.methods.session import _map_to_wire -from raven.session.turn_shape import replied_via_message_tool +from raven.session.turn_shape import REPLIED_VIA_TOOL, TakeBack, TakeBackStep, replied_via_message_tool def _message_call() -> dict: @@ -38,3 +42,93 @@ def test_a_message_call_that_failed_did_not_answer_in_either_shape(): def test_only_an_assistant_row_carries_the_verdict(): assert not replied_via_message_tool({"role": "user", "content": "hi", "replied_via_tool": True}) assert not replied_via_message_tool({"role": "assistant", "content": "[SILENT]", "replied_via_tool": "yes"}) + + +def _steps(rows: list[dict]) -> list[TakeBackStep | None]: + reader = TakeBack() + return [reader.read(row) for row in rows] + + +def test_a_turn_the_runtime_opened_and_closed_on_its_marker_is_taken_back(): + assert _steps(_turn(answered=False)) == [TakeBackStep.OPENS, None, None, TakeBackStep.TAKES_BACK] + + +def test_a_turn_a_tool_answered_is_kept_whatever_it_ends_on(): + assert TakeBackStep.TAKES_BACK not in _steps(_turn(answered=True)) + + +def test_a_persons_turn_is_kept_when_the_model_answers_it_with_silence(): + rows = [{"role": "user", "content": "anything new?"}, {"role": "assistant", "content": "[SILENT]", "silent": True}] + assert _steps(rows) == [None, None] + + +def test_the_marker_closes_the_turn_it_takes_back(): + """A row after the marker is no part of the turn it closed, so a second marker before the next user + row takes nothing back.""" + rows = [*_turn(answered=False), {"role": "assistant", "content": "[SILENT]", "silent": True}] + assert _steps(rows)[-2:] == [TakeBackStep.TAKES_BACK, None] + + +_PRIOR = [ + {"role": "user", "content": "hi", "timestamp": "2026-10-30T01:00:00"}, + {"role": "assistant", "content": "hello", "timestamp": "2026-10-30T01:00:01"}, +] + + +def _timed(rows: list[dict]) -> list[dict]: + return [{**row, "timestamp": f"2026-10-30T02:00:{second:02d}"} for second, row in enumerate(rows)] + + +_MARKER = {"role": "assistant", "content": "[SILENT]", "silent": True} +_OPENED = {"role": "user", "content": "[Scheduled Task] check the build", "origin": "cron"} +_SHAPES = { + "the_runtime_opened_it_and_it_ended_silent": _turn(answered=False), + "a_tool_answered_it": _turn(answered=True), + "a_person_opened_it": [{"role": "user", "content": "anything new?"}, _MARKER], + "it_answered_in_words": [_OPENED, {"role": "assistant", "content": "Report."}], + "a_row_follows_its_marker": [_OPENED, _MARKER, {"role": "assistant", "content": "Noted."}, _MARKER], +} + + +def _live(row: dict) -> dict: + from raven.agent.loop._shared import _ORIGIN_KEY, _REPLIED_VIA_TOOL_KEY, _SILENT_KEY + + names = {"origin": _ORIGIN_KEY, "silent": _SILENT_KEY, REPLIED_VIA_TOOL: _REPLIED_VIA_TOOL_KEY} + return {names.get(key, key): value for key, value in row.items()} + + +def _scanned(path, rows: list[dict]) -> tuple[str, str]: + from raven.session.manager import SessionManager + + lines = [{"_type": "metadata", "key": "cron:job"}, *rows] + path.write_text("".join(json.dumps(line) + "\n" for line in lines), encoding="utf-8") + _meta, _count, last_ts, _first, preview = SessionManager._scan_file(path) + return last_ts, preview + + +_NUDGE = {"role": "user", "content": "Go on.", "_recovery_synthetic": True} + + +@pytest.mark.parametrize("opened_by_an_earlier_write", [False, True], ids=["opened_now", "opened_earlier"]) +@pytest.mark.parametrize("nudged", [False, True], ids=["", "nudged"]) +@pytest.mark.parametrize("shape", list(_SHAPES), ids=list(_SHAPES)) +def test_the_live_turn_the_session_list_and_the_replay_take_back_the_same_rows( + tmp_path, shape, nudged, opened_by_an_earlier_write +): + """Three readers answer one question about a stored turn: the ACP replay draws what stays, the + session list orders sessions by it, and the live turn files a notice after its marker only where the + marker stays. Each answers at the level it reads, and the answers have to agree. The live turn reads + rows the store never sees -- a recovery nudge -- and may find its opening row filed by an earlier write.""" + from raven.acp.replay import _without_silent_runtime_turns + from raven.agent.loop.turn_path import _taken_back_whole + + rows = [*_PRIOR, *_timed(_SHAPES[shape])] + closing = [row for row in rows if row["role"] == "assistant"][-1] + kept = _without_silent_runtime_turns(rows) + + live = [*_PRIOR, *(_live(row) for row in rows[len(_PRIOR) :])] + if nudged: + live.insert(len(live) - 1, dict(_NUDGE)) + skip = len(_PRIOR) + (1 if opened_by_an_earlier_write else 0) + assert _taken_back_whole(live, skip) is all(row is not closing for row in kept) + assert _scanned(tmp_path / "all.jsonl", rows) == _scanned(tmp_path / "kept.jsonl", kept)