mirror of
https://github.com/HKUDS/nanobot.git
synced 2026-08-31 00:03:01 +03:00
fix(agent): keep explicit skills out of system prompt
This commit is contained in:
+32
-13
@@ -94,7 +94,6 @@ class ContextBuilder:
|
|||||||
def build_system_prompt(
|
def build_system_prompt(
|
||||||
self,
|
self,
|
||||||
*,
|
*,
|
||||||
active_skill_names: Sequence[str] | None = None,
|
|
||||||
channel: str | None = None,
|
channel: str | None = None,
|
||||||
session_summary: SessionSummary | None = None,
|
session_summary: SessionSummary | None = None,
|
||||||
workspace: Path | None = None,
|
workspace: Path | None = None,
|
||||||
@@ -119,11 +118,6 @@ class ContextBuilder:
|
|||||||
parts.append(f"# Memory\n\n## Long-term Memory\n{memory}")
|
parts.append(f"# Memory\n\n## Long-term Memory\n{memory}")
|
||||||
|
|
||||||
active_skills = self.skills.get_always_skills()
|
active_skills = self.skills.get_always_skills()
|
||||||
active_skills.extend(
|
|
||||||
name
|
|
||||||
for name in (active_skill_names or ())
|
|
||||||
if name not in active_skills
|
|
||||||
)
|
|
||||||
if active_skills:
|
if active_skills:
|
||||||
active_content = self.skills.load_skills_for_context(active_skills)
|
active_content = self.skills.load_skills_for_context(active_skills)
|
||||||
if active_content:
|
if active_content:
|
||||||
@@ -165,6 +159,33 @@ class ContextBuilder:
|
|||||||
|
|
||||||
return "\n\n---\n\n".join(parts)
|
return "\n\n---\n\n".join(parts)
|
||||||
|
|
||||||
|
def build_runtime_context_blocks(
|
||||||
|
self,
|
||||||
|
current_message: str,
|
||||||
|
blocks: Sequence[RuntimeContextBlock] | None = None,
|
||||||
|
) -> list[RuntimeContextBlock]:
|
||||||
|
"""Add explicitly invoked skill instructions to this turn's runtime context."""
|
||||||
|
merged = list(blocks or ())
|
||||||
|
invoked = self.skills.get_explicitly_invoked_skills(current_message)
|
||||||
|
if not invoked:
|
||||||
|
return merged
|
||||||
|
always_active = set(self.skills.get_always_skills())
|
||||||
|
skill_names = [name for name in invoked if name not in always_active]
|
||||||
|
skill_content = self.skills.load_skills_for_context(skill_names)
|
||||||
|
if not skill_content:
|
||||||
|
return merged
|
||||||
|
skill_block = RuntimeContextBlock(
|
||||||
|
source="explicit_skills",
|
||||||
|
content=(
|
||||||
|
"[Active Skills — instructions for this user turn]\n"
|
||||||
|
f"{skill_content}\n"
|
||||||
|
"[/Active Skills]"
|
||||||
|
),
|
||||||
|
)
|
||||||
|
if skill_block not in merged:
|
||||||
|
merged.append(skill_block)
|
||||||
|
return merged
|
||||||
|
|
||||||
@staticmethod
|
@staticmethod
|
||||||
def _without_duplicate_session_summary(
|
def _without_duplicate_session_summary(
|
||||||
entries: list[dict[str, Any]],
|
entries: list[dict[str, Any]],
|
||||||
@@ -279,16 +300,10 @@ class ContextBuilder:
|
|||||||
) -> list[dict[str, Any]]:
|
) -> list[dict[str, Any]]:
|
||||||
"""Build the complete message list for an LLM call."""
|
"""Build the complete message list for an LLM call."""
|
||||||
root = workspace or self.workspace
|
root = workspace or self.workspace
|
||||||
active_skill_names = (
|
|
||||||
self.skills.get_explicitly_invoked_skills(current_message)
|
|
||||||
if current_role == "user"
|
|
||||||
else []
|
|
||||||
)
|
|
||||||
messages: list[dict[str, Any]] = [
|
messages: list[dict[str, Any]] = [
|
||||||
{
|
{
|
||||||
"role": "system",
|
"role": "system",
|
||||||
"content": self.build_system_prompt(
|
"content": self.build_system_prompt(
|
||||||
active_skill_names=active_skill_names,
|
|
||||||
channel=channel,
|
channel=channel,
|
||||||
session_summary=session_summary,
|
session_summary=session_summary,
|
||||||
workspace=root,
|
workspace=root,
|
||||||
@@ -332,7 +347,11 @@ class ContextBuilder:
|
|||||||
) -> dict[str, Any]:
|
) -> dict[str, Any]:
|
||||||
"""Build only the fresh turn message without merging it into history."""
|
"""Build only the fresh turn message without merging it into history."""
|
||||||
content = self.build_user_content(current_message, image_paths=media)
|
content = self.build_user_content(current_message, image_paths=media)
|
||||||
blocks = list(runtime_context_blocks or ()) if current_role == "user" else []
|
blocks = (
|
||||||
|
self.build_runtime_context_blocks(current_message, runtime_context_blocks)
|
||||||
|
if current_role == "user"
|
||||||
|
else []
|
||||||
|
)
|
||||||
merged, runtime_context_meta = append_runtime_context(content, blocks)
|
merged, runtime_context_meta = append_runtime_context(content, blocks)
|
||||||
current: dict[str, Any] = {"role": current_role, "content": merged}
|
current: dict[str, Any] = {"role": current_role, "content": merged}
|
||||||
if current_role == "user" and runtime_context_meta is not None:
|
if current_role == "user" and runtime_context_meta is not None:
|
||||||
|
|||||||
@@ -797,7 +797,10 @@ class AgentLoop:
|
|||||||
]
|
]
|
||||||
blocks = runtime_context_blocks_from_metadata(request.metadata)
|
blocks = runtime_context_blocks_from_metadata(request.metadata)
|
||||||
blocks.extend(await resolve_runtime_context(providers, request))
|
blocks.extend(await resolve_runtime_context(providers, request))
|
||||||
return blocks
|
return self.context.build_runtime_context_blocks(
|
||||||
|
request.original_user_text or "",
|
||||||
|
blocks,
|
||||||
|
)
|
||||||
|
|
||||||
async def _dispatch_command_inline(
|
async def _dispatch_command_inline(
|
||||||
self,
|
self,
|
||||||
|
|||||||
@@ -407,15 +407,20 @@ class TestBuildMessages:
|
|||||||
builder = _builder(tmp_path)
|
builder = _builder(tmp_path)
|
||||||
|
|
||||||
messages = builder.build_messages([], "Please $review this patch and use $review carefully.")
|
messages = builder.build_messages([], "Please $review this patch and use $review carefully.")
|
||||||
|
plain_messages = builder.build_messages([], "Please review this patch carefully.")
|
||||||
|
|
||||||
system_prompt = messages[0]["content"]
|
system_prompt = messages[0]["content"]
|
||||||
assert "# Active Skills" in system_prompt
|
user_prompt = messages[-1]["content"]
|
||||||
assert "### Skill: review" in system_prompt
|
assert system_prompt == plain_messages[0]["content"]
|
||||||
assert "Follow the unique review checklist." in system_prompt
|
assert "Follow the unique review checklist." not in system_prompt
|
||||||
assert system_prompt.count("### Skill: review") == 1
|
assert "Please $review this patch" in user_prompt
|
||||||
assert messages[-1]["content"] == (
|
assert "[Active Skills — instructions for this user turn]" in user_prompt
|
||||||
"Please $review this patch and use $review carefully."
|
assert "### Skill: review" in user_prompt
|
||||||
)
|
assert "Follow the unique review checklist." in user_prompt
|
||||||
|
assert user_prompt.count("### Skill: review") == 1
|
||||||
|
assert messages[-1]["_meta"]["runtime_context"]["sources"] == [
|
||||||
|
"explicit_skills"
|
||||||
|
]
|
||||||
|
|
||||||
def test_unknown_skill_reference_does_not_change_active_skills(self, tmp_path):
|
def test_unknown_skill_reference_does_not_change_active_skills(self, tmp_path):
|
||||||
messages = _builder(tmp_path).build_messages([], "Keep the shell literal $HOME.")
|
messages = _builder(tmp_path).build_messages([], "Keep the shell literal $HOME.")
|
||||||
|
|||||||
@@ -146,6 +146,16 @@ async def test_runtime_context_is_persisted_as_next_turn_prompt_prefix(tmp_path)
|
|||||||
from nanobot.bus.events import InboundMessage
|
from nanobot.bus.events import InboundMessage
|
||||||
from nanobot.bus.queue import MessageBus
|
from nanobot.bus.queue import MessageBus
|
||||||
|
|
||||||
|
skill_dir = tmp_path / "skills" / "review"
|
||||||
|
skill_dir.mkdir(parents=True)
|
||||||
|
(skill_dir / "SKILL.md").write_text(
|
||||||
|
"---\n"
|
||||||
|
"name: review\n"
|
||||||
|
"description: Review changes.\n"
|
||||||
|
"---\n\n"
|
||||||
|
"Follow the unique review checklist.",
|
||||||
|
encoding="utf-8",
|
||||||
|
)
|
||||||
provider = MagicMock()
|
provider = MagicMock()
|
||||||
provider.get_default_model.return_value = "test-model"
|
provider.get_default_model.return_value = "test-model"
|
||||||
provider.generation = GenerationSettings()
|
provider.generation = GenerationSettings()
|
||||||
@@ -169,7 +179,7 @@ async def test_runtime_context_is_persisted_as_next_turn_prompt_prefix(tmp_path)
|
|||||||
channel="cli",
|
channel="cli",
|
||||||
sender_id="user",
|
sender_id="user",
|
||||||
chat_id="direct",
|
chat_id="direct",
|
||||||
content="first turn",
|
content="first turn $review",
|
||||||
))
|
))
|
||||||
await loop._process_message(InboundMessage(
|
await loop._process_message(InboundMessage(
|
||||||
channel="cli",
|
channel="cli",
|
||||||
@@ -184,6 +194,9 @@ async def test_runtime_context_is_persisted_as_next_turn_prompt_prefix(tmp_path)
|
|||||||
second_wire = LLMProvider._sanitize_empty_content(second_request)
|
second_wire = LLMProvider._sanitize_empty_content(second_request)
|
||||||
assert second_wire[: len(first_wire)] == first_wire
|
assert second_wire[: len(first_wire)] == first_wire
|
||||||
assert first_wire[1] == second_wire[1]
|
assert first_wire[1] == second_wire[1]
|
||||||
|
assert first_wire[0] == second_wire[0]
|
||||||
|
assert "Follow the unique review checklist." not in first_wire[0]["content"]
|
||||||
|
assert "Follow the unique review checklist." in first_wire[1]["content"]
|
||||||
assert second_wire[2]["role"] == "assistant"
|
assert second_wire[2]["role"] == "assistant"
|
||||||
assert second_wire[2]["content"] == "first answer"
|
assert second_wire[2]["content"] == "first answer"
|
||||||
assert second_wire[3]["content"].startswith("second turn")
|
assert second_wire[3]["content"].startswith("second turn")
|
||||||
@@ -191,7 +204,7 @@ async def test_runtime_context_is_persisted_as_next_turn_prompt_prefix(tmp_path)
|
|||||||
|
|
||||||
persisted_first_user = session.messages[0]
|
persisted_first_user = session.messages[0]
|
||||||
assert persisted_first_user["content"] == first_wire[1]["content"]
|
assert persisted_first_user["content"] == first_wire[1]["content"]
|
||||||
assert public_history_message(persisted_first_user)["content"] == "first turn"
|
assert public_history_message(persisted_first_user)["content"] == "first turn $review"
|
||||||
|
|
||||||
|
|
||||||
@pytest.mark.asyncio
|
@pytest.mark.asyncio
|
||||||
|
|||||||
Reference in New Issue
Block a user