From ad58d6cdc6559c81e1cf3de75ee3218ee8efae1d Mon Sep 17 00:00:00 2001 From: worldmozara Date: Sat, 1 Aug 2026 00:33:25 +0800 Subject: [PATCH] lint: drop dead code that blocks basedpyright (unused helper and import) --- src/plyngent/agent/chat.py | 32 -------- src/plyngent/cli/limits.py | 1 - tests/test_agent/test_retry_todo_nag.py | 103 +++++------------------- 3 files changed, 19 insertions(+), 117 deletions(-) diff --git a/src/plyngent/agent/chat.py b/src/plyngent/agent/chat.py index e44aafc..2e7d992 100644 --- a/src/plyngent/agent/chat.py +++ b/src/plyngent/agent/chat.py @@ -26,7 +26,6 @@ from .events import UsageEvent from .loop import DEFAULT_MAX_ROUNDS, run_chat_loop from .todo_nag import ( DEFAULT_TODO_NAG_STRATEGY, - is_synthetic_todo_nag_call_id, parse_todo_nag_strategy, refresh_synthetic_todo_nags, ) @@ -153,37 +152,6 @@ def committed_prefix_end(messages: Sequence[AnyChatMessage], user_index: int) -> return end -def _synthetic_todo_pair_after( - messages: Sequence[AnyChatMessage], - index: int, -) -> bool: - """True when a synthetic todo nag pair sits at ``messages[index+1:index+3]``. - - The pattern is ``AssistantChatMessage`` (all tool calls synthetic) followed - by a matching ``ToolChatMessage`` with a synthetic call id. - On retry after a failed turn, a previously injected turn-start nag may still - be in messages (survived rollback). This detects it so we can skip a fresh - injection. - """ - if index + 2 >= len(messages): - return False - first = messages[index + 1] - if not isinstance(first, AssistantChatMessage): - return False - tool_calls = first.tool_calls - if tool_calls is UNSET or not tool_calls: - return False - if not all( - isinstance(call, AssistantFunctionToolCall) and is_synthetic_todo_nag_call_id(call.id) for call in tool_calls - ): - return False - synth_ids = {call.id for call in tool_calls if isinstance(call, AssistantFunctionToolCall)} - second = messages[index + 2] - if not isinstance(second, ToolChatMessage): - return False - return second.tool_call_id in synth_ids - - class ChatAgent: """Thin wrapper: chat client + optional tools + optional memory bind.""" diff --git a/src/plyngent/cli/limits.py b/src/plyngent/cli/limits.py index 5c58f41..649d2fd 100644 --- a/src/plyngent/cli/limits.py +++ b/src/plyngent/cli/limits.py @@ -16,7 +16,6 @@ from plyngent.prompting import ( choose_async, configure_prompting, confirm, - confirm_async, get_prompt_backend, ) from plyngent.tools.process.pty_session import PtyManager diff --git a/tests/test_agent/test_retry_todo_nag.py b/tests/test_agent/test_retry_todo_nag.py index 71714ff..ee303ea 100644 --- a/tests/test_agent/test_retry_todo_nag.py +++ b/tests/test_agent/test_retry_todo_nag.py @@ -1,22 +1,19 @@ -"""Tests for synthetic_tool todo nag behaviour on retry. +"""Tests for committed-prefix rollback preserving synthetic todo nag pairs. -Tests the core rollback and _synthetic_todo_pair_after logic. +Covers committed_prefix_end: after a failed turn, rollback must keep committed +tool batches (including synthetic todo nag pairs) so retry does not re-execute +side effects or duplicate nags. """ from __future__ import annotations from msgspec import UNSET -from plyngent.agent.chat import ( - _synthetic_todo_pair_after, - committed_prefix_end, -) +from plyngent.agent.chat import committed_prefix_end from plyngent.agent.todo_nag import ( _append_synthetic_todo_list, is_synthetic_todo_nag_call_id, - refresh_synthetic_todo_nags, ) -from plyngent.agent.todo_stack import TodoStack from plyngent.lmproto.openai_compatible.model import ( AnyChatMessage, AssistantChatMessage, @@ -28,23 +25,6 @@ from plyngent.lmproto.openai_compatible.model import ( ) -def test_synthetic_pair_after_find_at_index() -> None: - """_synthetic_todo_pair_after correctly identifies a pair at index+1/+2.""" - stack = TodoStack() - _ = stack.push("do this") - - messages: list[AnyChatMessage] = [ - SystemChatMessage(content="sys"), - UserChatMessage(content="hi"), - ] - _append_synthetic_todo_list(messages, stack.render()) - - assert _synthetic_todo_pair_after(messages, 1), "Should find synthetic pair at user_index=1" - - # When user is at different index, same pair should NOT be found - assert not _synthetic_todo_pair_after(messages, 0), "Should NOT find pair at user_index=0 (pair is at 2/3)" - - def test_synthetic_pair_survives_rollback() -> None: """The synthetic pair survives committed_prefix_end + deletion.""" messages: list[AnyChatMessage] = [ @@ -61,7 +41,6 @@ def test_synthetic_pair_survives_rollback() -> None: assert len(messages) == 4 assert isinstance(messages[2], AssistantChatMessage) assert isinstance(messages[3], ToolChatMessage) - assert _synthetic_todo_pair_after(messages, user_index) def test_synthetic_pair_survives_rollback_with_subsequent_committed_content() -> None: @@ -99,8 +78,8 @@ def test_synthetic_pair_survives_rollback_with_subsequent_committed_content() -> assert end == 6, f"Expected end=6, got {end}" del messages[end:] - # Pair should survive - assert _synthetic_todo_pair_after(messages, user_index) + # Both the synthetic pair and the real tool batch survive. + assert len(messages) == 6 def test_multiple_rollbacks_preserve_single_pair() -> None: @@ -117,14 +96,10 @@ def test_multiple_rollbacks_preserve_single_pair() -> None: end = committed_prefix_end(messages, user_index) del messages[end:] - assert _synthetic_todo_pair_after(messages, user_index) - # Simulate second failure: roll back again end = committed_prefix_end(messages, user_index) del messages[end:] - assert _synthetic_todo_pair_after(messages, user_index) - # Count pairs count = sum( 1 @@ -139,57 +114,11 @@ def test_multiple_rollbacks_preserve_single_pair() -> None: assert count == 1, f"Expected 1 synthetic pair, got {count}" -def test_retry_skips_injection_when_pair_present() -> None: - """Simulating retry logic: should skip injection when pair exists.""" +def test_rollback_keeps_real_tool_batch_not_matching() -> None: + """Real tool calls are committed prefix, not treated as synthetic nag.""" messages: list[AnyChatMessage] = [ SystemChatMessage(content="sys"), UserChatMessage(content="hi"), - ] - _append_synthetic_todo_list(messages, "task") - - stack = TodoStack() - _ = stack.push("task") - - user_index = 1 - - # Refresh synthetic nags (as _run_from_user_message does) - _ = refresh_synthetic_todo_nags(messages, stack) - - should_inject = not _synthetic_todo_pair_after(messages, user_index) - - assert not should_inject, "Should NOT inject on retry when pair exists" - - # For developer and user strategies, the outer nag check passes - # (injects) regardless of pair existence — the pair_exists branch is - # only exercised by synthetic_tool. - assert _synthetic_todo_pair_after(messages, user_index), "pair should still exist" - - -def test_synthetic_pair_after_empty_list() -> None: - """_synthetic_todo_pair_after returns False for short lists.""" - messages: list[AnyChatMessage] = [ - SystemChatMessage(content="sys"), - UserChatMessage(content="hi"), - ] - assert not _synthetic_todo_pair_after(messages, 1) - assert not _synthetic_todo_pair_after(messages, 0) - - # Edge: user at end with no room for pair - assert not _synthetic_todo_pair_after(messages, 0) - - -def test_synthetic_pair_after_non_matching() -> None: - """_synthetic_todo_pair_after returns False for non-synthetic content.""" - messages: list[AnyChatMessage] = [ - SystemChatMessage(content="sys"), - UserChatMessage(content="hi"), - AssistantChatMessage(content="normal response"), - ] - # messages[2] is AssistantChatMessage but has no tool_calls - assert not _synthetic_todo_pair_after(messages, 1) - - # Real tool call instead of synthetic - messages.append( AssistantChatMessage( content=UNSET, tool_calls=[ @@ -198,7 +127,13 @@ def test_synthetic_pair_after_non_matching() -> None: function=AssistantFunctionTool(name="tool", arguments="{}"), ) ], - ) - ) - messages.append(ToolChatMessage(tool_call_id="call_real", content="done")) - assert not _synthetic_todo_pair_after(messages, 1), "Real tool calls should not match" + ), + ToolChatMessage(tool_call_id="call_real", content="done"), + AssistantChatMessage(content="uncommitted response"), + ] + user_index = 1 + end = committed_prefix_end(messages, user_index) + # user + real tool batch = 4; trailing text assistant not committed. + assert end == 4, f"Expected end=4, got {end}" + del messages[end:] + assert len(messages) == 4