mirror of
https://github.com/NCBM/plyngent.git
synced 2026-11-08 16:37:55 +08:00
core/agent: remove turn-start todo nag -- nag only fires after complete turn
This commit is contained in:
@@ -25,7 +25,6 @@ from .events import UsageEvent
|
|||||||
from .loop import DEFAULT_MAX_ROUNDS, run_chat_loop
|
from .loop import DEFAULT_MAX_ROUNDS, run_chat_loop
|
||||||
from .todo_nag import (
|
from .todo_nag import (
|
||||||
DEFAULT_TODO_NAG_STRATEGY,
|
DEFAULT_TODO_NAG_STRATEGY,
|
||||||
inject_todo_nag_for_stack_with_events,
|
|
||||||
is_synthetic_todo_nag_call_id,
|
is_synthetic_todo_nag_call_id,
|
||||||
parse_todo_nag_strategy,
|
parse_todo_nag_strategy,
|
||||||
refresh_synthetic_todo_nags,
|
refresh_synthetic_todo_nags,
|
||||||
@@ -431,33 +430,6 @@ class ChatAgent:
|
|||||||
last_request = TokenUsage()
|
last_request = TokenUsage()
|
||||||
try:
|
try:
|
||||||
_turn_nag_strategy: TodoNagStrategy = self.todo_nag_strategy
|
_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(
|
async for event in run_chat_loop(
|
||||||
self.client,
|
self.client,
|
||||||
@@ -473,7 +445,6 @@ class ChatAgent:
|
|||||||
max_context_tokens=self.max_context_tokens,
|
max_context_tokens=self.max_context_tokens,
|
||||||
todo_stack=self.todo_stack,
|
todo_stack=self.todo_stack,
|
||||||
todo_nag_strategy=_turn_nag_strategy,
|
todo_nag_strategy=_turn_nag_strategy,
|
||||||
turn_start_nag_skipped=_turn_nag_skipped,
|
|
||||||
directive_reminder_tokens=self.directive_reminder_tokens,
|
directive_reminder_tokens=self.directive_reminder_tokens,
|
||||||
directive_reminder_text=self.directive_reminder_text,
|
directive_reminder_text=self.directive_reminder_text,
|
||||||
reminder_last_band=self.reminder_last_band,
|
reminder_last_band=self.reminder_last_band,
|
||||||
|
|||||||
@@ -386,7 +386,6 @@ async def run_chat_loop( # noqa: C901, PLR0912 — multi-phase tool loop
|
|||||||
max_context_tokens: int = DEFAULT_CONTEXT_MAX_TOKENS,
|
max_context_tokens: int = DEFAULT_CONTEXT_MAX_TOKENS,
|
||||||
todo_stack: TodoStack | None = None,
|
todo_stack: TodoStack | None = None,
|
||||||
todo_nag_strategy: TodoNagStrategy = DEFAULT_TODO_NAG_STRATEGY,
|
todo_nag_strategy: TodoNagStrategy = DEFAULT_TODO_NAG_STRATEGY,
|
||||||
turn_start_nag_skipped: bool = False,
|
|
||||||
directive_reminder_tokens: int = DEFAULT_DIRECTIVE_REMINDER_TOKENS,
|
directive_reminder_tokens: int = DEFAULT_DIRECTIVE_REMINDER_TOKENS,
|
||||||
directive_reminder_text: str | None = None,
|
directive_reminder_text: str | None = None,
|
||||||
reminder_last_band: int = 0,
|
reminder_last_band: int = 0,
|
||||||
@@ -480,13 +479,6 @@ async def run_chat_loop( # noqa: C901, PLR0912 — multi-phase tool loop
|
|||||||
if not has_tools:
|
if not has_tools:
|
||||||
if todo_stack is not None and todo_stack.needs_review() and not todo_review_injected:
|
if todo_stack is not None and todo_stack.needs_review() and not todo_review_injected:
|
||||||
todo_review_injected = True
|
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(
|
injected, nag_events = inject_todo_nag_for_stack_with_events(
|
||||||
messages,
|
messages,
|
||||||
todo_stack,
|
todo_stack,
|
||||||
|
|||||||
@@ -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:
|
Uses mock clients to simulate model responses across retries, covering:
|
||||||
- Turn-start nag skipped on retry (pair survived rollback)
|
- No turn-start nag is ever injected (nag only fires after complete turns)
|
||||||
- End-of-turn nag NOT injected on retry (turn-start pair covers it)
|
- End-of-turn nag fires after a successful retry when model doesn't touch stack
|
||||||
- No duplicate synthetic pairs accumulate in history
|
- No duplicate synthetic pairs accumulate in history
|
||||||
"""
|
"""
|
||||||
|
|
||||||
@@ -14,9 +14,7 @@ import pytest
|
|||||||
from msgspec import UNSET
|
from msgspec import UNSET
|
||||||
|
|
||||||
from plyngent.agent import ChatAgent
|
from plyngent.agent import ChatAgent
|
||||||
from plyngent.agent.todo_nag import (
|
from plyngent.agent.todo_nag import is_synthetic_todo_nag_call_id
|
||||||
is_synthetic_todo_nag_call_id,
|
|
||||||
)
|
|
||||||
from plyngent.agent.todo_stack import TodoStack
|
from plyngent.agent.todo_stack import TodoStack
|
||||||
from plyngent.config.models import DatabaseConfig
|
from plyngent.config.models import DatabaseConfig
|
||||||
from plyngent.lmproto.openai_compatible.model import (
|
from plyngent.lmproto.openai_compatible.model import (
|
||||||
@@ -149,8 +147,8 @@ class FailTwiceThenTextClient:
|
|||||||
|
|
||||||
|
|
||||||
@pytest.mark.asyncio
|
@pytest.mark.asyncio
|
||||||
async def test_retry_twice_no_extra_pairs() -> None:
|
async def test_no_turn_start_nag_on_initial_run() -> None:
|
||||||
"""After two failed streams + one success, only 1 synthetic pair exists."""
|
"""No turn-start nag is injected on initial run — only end-of-turn."""
|
||||||
store = await MemoryStore.open(DatabaseConfig())
|
store = await MemoryStore.open(DatabaseConfig())
|
||||||
session = await store.create_session(name="t")
|
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",
|
todo_nag_strategy="synthetic_tool",
|
||||||
)
|
)
|
||||||
|
|
||||||
# First run fails
|
# First run fails — no turn-start nag
|
||||||
with pytest.raises(RuntimeError, match="stream fail"):
|
with pytest.raises(RuntimeError, match="stream fail"):
|
||||||
async for _ in agent.run("hi"):
|
async for _ in agent.run("hi"):
|
||||||
pass
|
pass
|
||||||
|
|
||||||
assert count_synthetic_pairs(agent.messages) == 1, "1 pair after first run failure"
|
assert count_synthetic_pairs(agent.messages) == 0, "0 pairs after first 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"
|
|
||||||
|
|
||||||
await store.close()
|
await store.close()
|
||||||
|
|
||||||
|
|
||||||
@pytest.mark.asyncio
|
@pytest.mark.asyncio
|
||||||
async def test_retry_after_tool_round_completed() -> None:
|
async def test_end_of_turn_nag_on_successful_retry() -> None:
|
||||||
"""Tool calls executed + retry on next round produces no duplicate nag."""
|
"""End-of-turn nag fires after a successful retry when stack untouched."""
|
||||||
store = await MemoryStore.open(DatabaseConfig())
|
store = await MemoryStore.open(DatabaseConfig())
|
||||||
session = await store.create_session(name="t")
|
session = await store.create_session(name="t")
|
||||||
|
|
||||||
stack = TodoStack()
|
stack = TodoStack()
|
||||||
_ = stack.push("task B")
|
_ = stack.push("task B")
|
||||||
registry = None # no tools, so model always returns text
|
|
||||||
|
|
||||||
call_count = 0
|
client = FailTwiceThenTextClient()
|
||||||
|
|
||||||
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()
|
|
||||||
agent = ChatAgent(
|
agent = ChatAgent(
|
||||||
client,
|
client,
|
||||||
model="m",
|
model="m",
|
||||||
@@ -243,25 +192,24 @@ async def test_retry_after_tool_round_completed() -> None:
|
|||||||
session_id=session.sid,
|
session_id=session.sid,
|
||||||
todo_stack=stack,
|
todo_stack=stack,
|
||||||
todo_nag_strategy="synthetic_tool",
|
todo_nag_strategy="synthetic_tool",
|
||||||
tools=registry,
|
|
||||||
)
|
)
|
||||||
|
|
||||||
# First run fails
|
# Run fails
|
||||||
with pytest.raises(RuntimeError, match="stream fail"):
|
with pytest.raises(RuntimeError, match="stream fail"):
|
||||||
async for _ in agent.run("hi"):
|
async for _ in agent.run("hi"):
|
||||||
pass
|
pass
|
||||||
|
|
||||||
# Retry fails again
|
# Retry fails
|
||||||
with pytest.raises(RuntimeError, match="stream fail"):
|
with pytest.raises(RuntimeError, match="stream fail"):
|
||||||
async for _ in agent.retry():
|
async for _ in agent.retry():
|
||||||
pass
|
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():
|
async for _ in agent.retry():
|
||||||
pass
|
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()
|
await store.close()
|
||||||
|
|||||||
Reference in New Issue
Block a user