mirror of
https://github.com/HKUDS/nanobot.git
synced 2026-05-05 17:26:03 +00:00
refactor(agent): tighten comments, extract constant, strengthen edge case test
- Extract synthetic user message string to module-level constant - Tighten comments in _snip_history recovery branch - Strengthen no-user edge case test to verify safety net interaction
This commit is contained in:
parent
44b526c4ee
commit
8c0c4e5b31
@ -933,14 +933,15 @@ class AgentRunner:
|
|||||||
kept = kept[i:]
|
kept = kept[i:]
|
||||||
break
|
break
|
||||||
else:
|
else:
|
||||||
# No user message in the kept window — walk backwards through
|
# Recover nearest user message from outside the kept window;
|
||||||
# non_system to find the nearest user message and keep it plus
|
# GLM rejects system→assistant (error 1214). Budget is
|
||||||
# everything after it. Providers like GLM reject requests
|
# intentionally exceeded — oversized beats invalid.
|
||||||
# where the first non-system message is not ``user`` (error 1214).
|
|
||||||
for idx in range(len(non_system) - 1, -1, -1):
|
for idx in range(len(non_system) - 1, -1, -1):
|
||||||
if non_system[idx].get("role") == "user":
|
if non_system[idx].get("role") == "user":
|
||||||
kept = non_system[idx:]
|
kept = non_system[idx:]
|
||||||
break
|
break
|
||||||
|
# If no user exists at all, _enforce_role_alternation
|
||||||
|
# will insert a synthetic one as a safety net.
|
||||||
start = find_legal_message_start(kept)
|
start = find_legal_message_start(kept)
|
||||||
if start:
|
if start:
|
||||||
kept = kept[start:]
|
kept = kept[start:]
|
||||||
|
|||||||
@ -77,6 +77,9 @@ class GenerationSettings:
|
|||||||
reasoning_effort: str | None = None
|
reasoning_effort: str | None = None
|
||||||
|
|
||||||
|
|
||||||
|
_SYNTHETIC_USER_CONTENT = "(conversation continued)"
|
||||||
|
|
||||||
|
|
||||||
class LLMProvider(ABC):
|
class LLMProvider(ABC):
|
||||||
"""Base class for LLM providers."""
|
"""Base class for LLM providers."""
|
||||||
|
|
||||||
@ -417,7 +420,7 @@ class LLMProvider(ABC):
|
|||||||
for i, msg in enumerate(merged):
|
for i, msg in enumerate(merged):
|
||||||
if msg.get("role") != "system":
|
if msg.get("role") != "system":
|
||||||
if msg.get("role") == "assistant" and not msg.get("tool_calls"):
|
if msg.get("role") == "assistant" and not msg.get("tool_calls"):
|
||||||
merged.insert(i, {"role": "user", "content": "(conversation continued)"})
|
merged.insert(i, {"role": "user", "content": _SYNTHETIC_USER_CONTENT})
|
||||||
break
|
break
|
||||||
|
|
||||||
return merged
|
return merged
|
||||||
|
|||||||
@ -2909,3 +2909,12 @@ def test_snip_history_no_user_at_all_falls_back_gracefully(monkeypatch):
|
|||||||
assert isinstance(trimmed, list)
|
assert isinstance(trimmed, list)
|
||||||
# Must have at least system.
|
# Must have at least system.
|
||||||
assert any(m.get("role") == "system" for m in trimmed)
|
assert any(m.get("role") == "system" for m in trimmed)
|
||||||
|
# The _enforce_role_alternation safety net must be able to fix whatever
|
||||||
|
# _snip_history returns here — verify it produces a valid sequence.
|
||||||
|
from nanobot.providers.base import LLMProvider
|
||||||
|
fixed = LLMProvider._enforce_role_alternation(trimmed)
|
||||||
|
non_system = [m for m in fixed if m["role"] != "system"]
|
||||||
|
if non_system:
|
||||||
|
assert non_system[0]["role"] in ("user", "tool"), (
|
||||||
|
f"Safety net should ensure first non-system is user/tool, got {non_system[0]['role']}"
|
||||||
|
)
|
||||||
|
|||||||
@ -1,6 +1,6 @@
|
|||||||
"""Tests for LLMProvider._enforce_role_alternation."""
|
"""Tests for LLMProvider._enforce_role_alternation."""
|
||||||
|
|
||||||
from nanobot.providers.base import LLMProvider
|
from nanobot.providers.base import LLMProvider, _SYNTHETIC_USER_CONTENT
|
||||||
|
|
||||||
|
|
||||||
class TestEnforceRoleAlternation:
|
class TestEnforceRoleAlternation:
|
||||||
@ -208,7 +208,7 @@ class TestEnforceRoleAlternation:
|
|||||||
result = LLMProvider._enforce_role_alternation(msgs)
|
result = LLMProvider._enforce_role_alternation(msgs)
|
||||||
non_system = [m for m in result if m["role"] != "system"]
|
non_system = [m for m in result if m["role"] != "system"]
|
||||||
assert non_system[0]["role"] == "user"
|
assert non_system[0]["role"] == "user"
|
||||||
assert non_system[0]["content"] == "(conversation continued)"
|
assert non_system[0]["content"] == _SYNTHETIC_USER_CONTENT
|
||||||
# The original assistant should follow.
|
# The original assistant should follow.
|
||||||
assert non_system[1]["role"] == "assistant"
|
assert non_system[1]["role"] == "assistant"
|
||||||
|
|
||||||
|
|||||||
Loading…
x
Reference in New Issue
Block a user