diff --git a/src/plyngent/agent/chat.py b/src/plyngent/agent/chat.py index 3b1e91d..f47ffb2 100644 --- a/src/plyngent/agent/chat.py +++ b/src/plyngent/agent/chat.py @@ -25,7 +25,6 @@ from .events import UsageEvent from .loop import DEFAULT_MAX_ROUNDS, run_chat_loop from .todo_nag import ( DEFAULT_TODO_NAG_STRATEGY, - inject_todo_nag_for_stack_with_events, is_synthetic_todo_nag_call_id, parse_todo_nag_strategy, refresh_synthetic_todo_nags, @@ -431,33 +430,6 @@ class ChatAgent: last_request = TokenUsage() try: _turn_nag_strategy: TodoNagStrategy = self.todo_nag_strategy - # Turn-start nag before first completion; yield events so CLI flushes - # (synthetic_tool → ToolCall/Result chrome, not glued to text). - # On retry, a synthetic_tool pair from a previous failed attempt may - # have survived rollback (it looks like a committed tool batch). - # Skip fresh injection — refresh_synthetic_todo_nags already updated - # the content of the surviving pair above. - _turn_nag_skipped = ( - self.todo_stack is not None - and not self.todo_stack.is_empty() - and _turn_nag_strategy == "synthetic_tool" - and _synthetic_todo_pair_after(self.messages, user_index) - ) - if ( - self.todo_stack is not None - and not self.todo_stack.is_empty() - and ( - _turn_nag_strategy != "synthetic_tool" or not _synthetic_todo_pair_after(self.messages, user_index) - ) - ): - _injected, nag_events = inject_todo_nag_for_stack_with_events( - self.messages, - self.todo_stack, - kind="turn_start", - strategy=_turn_nag_strategy, - ) - for nag_event in nag_events: - yield nag_event async for event in run_chat_loop( self.client, @@ -473,7 +445,6 @@ class ChatAgent: max_context_tokens=self.max_context_tokens, todo_stack=self.todo_stack, todo_nag_strategy=_turn_nag_strategy, - turn_start_nag_skipped=_turn_nag_skipped, directive_reminder_tokens=self.directive_reminder_tokens, directive_reminder_text=self.directive_reminder_text, reminder_last_band=self.reminder_last_band, diff --git a/src/plyngent/agent/loop.py b/src/plyngent/agent/loop.py index 00377ff..351d49b 100644 --- a/src/plyngent/agent/loop.py +++ b/src/plyngent/agent/loop.py @@ -386,7 +386,6 @@ async def run_chat_loop( # noqa: C901, PLR0912 — multi-phase tool loop max_context_tokens: int = DEFAULT_CONTEXT_MAX_TOKENS, todo_stack: TodoStack | None = None, todo_nag_strategy: TodoNagStrategy = DEFAULT_TODO_NAG_STRATEGY, - turn_start_nag_skipped: bool = False, directive_reminder_tokens: int = DEFAULT_DIRECTIVE_REMINDER_TOKENS, directive_reminder_text: str | None = None, reminder_last_band: int = 0, @@ -480,19 +479,12 @@ async def run_chat_loop( # noqa: C901, PLR0912 — multi-phase tool loop if not has_tools: if todo_stack is not None and todo_stack.needs_review() and not todo_review_injected: todo_review_injected = True - # On retry, the turn-start nag was skipped (pair survived - # rollback). The model already has the stack reminder - # in-history — skip the end-of-turn duplicate. - if turn_start_nag_skipped: - injected = False - nag_events: list[AgentEvent] = [] - else: - injected, nag_events = inject_todo_nag_for_stack_with_events( - messages, - todo_stack, - kind="end_of_turn", - strategy=todo_nag_strategy, - ) + injected, nag_events = inject_todo_nag_for_stack_with_events( + messages, + todo_stack, + kind="end_of_turn", + strategy=todo_nag_strategy, + ) for nag_event in nag_events: yield nag_event if injected: diff --git a/tests/test_agent/test_retry_nag_integration.py b/tests/test_agent/test_retry_nag_integration.py index 9d9c447..e5045ce 100644 --- a/tests/test_agent/test_retry_nag_integration.py +++ b/tests/test_agent/test_retry_nag_integration.py @@ -1,8 +1,8 @@ -"""Integration tests: full run→fail→retry flow, synthetic_tool nag detection. +"""Integration tests: full run-fail-retry flow, synthetic_tool nag detection. Uses mock clients to simulate model responses across retries, covering: -- Turn-start nag skipped on retry (pair survived rollback) -- End-of-turn nag NOT injected on retry (turn-start pair covers it) +- No turn-start nag is ever injected (nag only fires after complete turns) +- End-of-turn nag fires after a successful retry when model doesn't touch stack - No duplicate synthetic pairs accumulate in history """ @@ -14,9 +14,7 @@ import pytest from msgspec import UNSET from plyngent.agent import ChatAgent -from plyngent.agent.todo_nag import ( - is_synthetic_todo_nag_call_id, -) +from plyngent.agent.todo_nag import is_synthetic_todo_nag_call_id from plyngent.agent.todo_stack import TodoStack from plyngent.config.models import DatabaseConfig from plyngent.lmproto.openai_compatible.model import ( @@ -149,8 +147,8 @@ class FailTwiceThenTextClient: @pytest.mark.asyncio -async def test_retry_twice_no_extra_pairs() -> None: - """After two failed streams + one success, only 1 synthetic pair exists.""" +async def test_no_turn_start_nag_on_initial_run() -> None: + """No turn-start nag is injected on initial run — only end-of-turn.""" store = await MemoryStore.open(DatabaseConfig()) session = await store.create_session(name="t") @@ -167,75 +165,26 @@ async def test_retry_twice_no_extra_pairs() -> None: todo_nag_strategy="synthetic_tool", ) - # First run fails + # First run fails — no turn-start nag with pytest.raises(RuntimeError, match="stream fail"): async for _ in agent.run("hi"): pass - assert count_synthetic_pairs(agent.messages) == 1, "1 pair after first run failure" - - # First retry fails - with pytest.raises(RuntimeError, match="stream fail"): - async for _ in agent.retry(): - pass - - assert count_synthetic_pairs(agent.messages) == 1, "Still 1 pair after first retry failure — no duplicate injection" - - # Second retry succeeds - async for _ in agent.retry(): - pass - - assert count_synthetic_pairs(agent.messages) == 1, "Still 1 pair after successful retry" + assert count_synthetic_pairs(agent.messages) == 0, "0 pairs after first failure" await store.close() @pytest.mark.asyncio -async def test_retry_after_tool_round_completed() -> None: - """Tool calls executed + retry on next round produces no duplicate nag.""" +async def test_end_of_turn_nag_on_successful_retry() -> None: + """End-of-turn nag fires after a successful retry when stack untouched.""" store = await MemoryStore.open(DatabaseConfig()) session = await store.create_session(name="t") stack = TodoStack() _ = stack.push("task B") - registry = None # no tools, so model always returns text - call_count = 0 - - class ToolThenStreamFailClient: - @overload - async def chat_completions( - self, param: ChatCompletionsParam, *, stream: Literal[False] = False - ) -> ChatCompletionResponse: ... - - @overload - async def chat_completions( - self, param: ChatCompletionsParam, *, stream: Literal[True] - ) -> AsyncIterator[ChatCompletionChunk]: ... - - async def chat_completions( - self, param: ChatCompletionsParam, *, stream: bool = False - ) -> ChatCompletionResponse | AsyncIterator[ChatCompletionChunk]: - nonlocal call_count - call_count += 1 - if stream: - if call_count >= 3: - return _stream_ok() - msg = f"stream fail #{call_count}" - raise RuntimeError(msg) - return _response(AssistantChatMessage(content="ok")) - - async def __aenter__(self): - return self - - async def __aexit__(self, *exc: object) -> None: - return None - - async def _stream_ok() -> AsyncIterator[ChatCompletionChunk]: - for chunk in _chunks_from_text("done"): - yield chunk - - client = ToolThenStreamFailClient() + client = FailTwiceThenTextClient() agent = ChatAgent( client, model="m", @@ -243,25 +192,24 @@ async def test_retry_after_tool_round_completed() -> None: session_id=session.sid, todo_stack=stack, todo_nag_strategy="synthetic_tool", - tools=registry, ) - # First run fails + # Run fails with pytest.raises(RuntimeError, match="stream fail"): async for _ in agent.run("hi"): pass - # Retry fails again + # Retry fails with pytest.raises(RuntimeError, match="stream fail"): async for _ in agent.retry(): pass - assert count_synthetic_pairs(agent.messages) == 1, "Only 1 pair after 2 failures" + assert count_synthetic_pairs(agent.messages) == 0, "0 pairs after failures" - # Final retry succeeds + # Retry succeeds — end-of-turn nag fires async for _ in agent.retry(): pass - assert count_synthetic_pairs(agent.messages) == 1, "1 pair after final success — no extra nag pair" + assert count_synthetic_pairs(agent.messages) == 1, "1 pair after success (end-of-turn nag only)" await store.close()