diff --git a/docs/skills.md b/docs/skills.md index 66334dc..62253bb 100644 --- a/docs/skills.md +++ b/docs/skills.md @@ -199,6 +199,23 @@ The log is a row per run with `report`/`reason`; there is no separate UI yet — `protocol(op="status")` and the table are the current view. +## Prompt guidance + +Tool schemas alone do not make a model use the catalog: it meets two unfamiliar +tools with no reason to reach for them, probes them instead of working, and the +run goes nowhere. So `ContextBuilder.build_system_prompt` appends a +`## Methodologies` block for every profile whose `tools.agent.native` lists +`skills` or `protocol` (`methodologies_block` in +[`navi/core/context_builder.py`](../navi/core/context_builder.py)). It carries the +rules the schemas cannot: search the catalog before improvising recurring work, a +skill is advice, a protocol is a commitment run through `protocol(op="start")`, +record what proved itself, and never create a protocol directly. + +The block is composed from the tools actually present, so a profile holding only +one of them is never pointed at the missing one; `dispatcher` and every sub-agent +get neither the tools nor the block. It rides in the cached system prompt, one +copy in code instead of seven profile prompts drifting apart. + ## Agent tools ### `skills` diff --git a/navi/core/context_builder.py b/navi/core/context_builder.py index ad6cb0e..1d561bf 100644 --- a/navi/core/context_builder.py +++ b/navi/core/context_builder.py @@ -123,6 +123,65 @@ from navi.profiles.base import AgentProfile +def methodologies_block(profile: "AgentProfile") -> str | None: + """The "how to use the methodologies catalog" block for profiles that have it. + + ``skills`` and ``protocol`` are useless as bare tool schemas: the model meets + two unfamiliar tools with no reason to reach for them, and probes them + instead of working. The rules that say when to search the catalog, how to run + a protocol and when to write an entry belong in the system prompt, once, + instead of seven profile prompts drifting apart. + """ + scope = getattr(getattr(profile, "tools", None), "agent", None) + native = list(getattr(scope, "native", None) or []) + has_skills = "skills" in native + has_protocol = "protocol" in native + if not (has_skills or has_protocol): + return None + + tools = " and ".join(f"`{t}`" for t in ("skills", "protocol") if t in native) + manuals = " / ".join( + f'`tool_manual("{t}")`' for t in ("skills", "protocol") if t in native + ) + intro = ( + f"{tools} are this Navi's catalog of *how* to do things: what it has already " + f"learned and agreed on. To read up on them use {manuals} — do not call them " + f"with a guess to see what happens." + ) + lines = [ + "## Methodologies — skills and protocols", + intro, + ] + if has_skills: + lines.append( + '- **Recurring work: search first.** "как обычно", "как мы делали", a standard ' + 'shutdown, a deployment, a routine document — call `skills(op="search")` once ' + "before improvising, then `load` the entry that fits. One call replaces a whole " + "rediscovery. Not for a one-off question, never twice for the same query." + ) + lines.append( + "- **A skill is advice** — follow it as far as it fits the task, and say where " + "you departed from it." + ) + if has_protocol: + lines.append( + '- **A protocol is a commitment.** Run it with `protocol(op="start")`, never by ' + "retyping its steps into the todo. A protocol whose triggers match the request is " + "the user's pre-agreed procedure: say you are running it and start. Each injected " + "step is marked done only after its check passes; the run ends with `finish` plus " + "a report on the post-checks, or with `fail`/`cancel` and the reason. Never " + "improvise around a step — that is the error a protocol exists to prevent." + ) + if has_skills: + lines.append( + "- **Record what proved itself.** An approach that worked and will recur is one " + '`skills(op="create")` and a line to the user. A protocol is never created ' + 'directly: `skills(op="propose")` it, say what it does, and wait for the user\'s ' + "decision." + ) + return "\n\n".join(lines) + + async def render_todo_lines(session_id: str) -> list[str]: """Return a list of formatted todo lines for goal anchoring.""" try: @@ -172,6 +231,10 @@ parts.append(persona) parts.append(profile.system_prompt) + methodologies = methodologies_block(profile) + if methodologies is not None: + parts.append(methodologies) + other = [ p for p in self._profiles.all() diff --git a/tests/unit/core/test_context_builder.py b/tests/unit/core/test_context_builder.py index cdb6684..734e4e0 100644 --- a/tests/unit/core/test_context_builder.py +++ b/tests/unit/core/test_context_builder.py @@ -127,6 +127,77 @@ assert "server_admin" not in again +class TestMethodologiesBlock: + """`skills`/`protocol` are only useful if the prompt says what they are for — + without it the model meets two unfamiliar tools and probes them.""" + + def _profile(self, *native: str): + from navi.profiles.base import ToolConfig, ToolScopeConfig + + return make_profile("p", tools=ToolConfig(agent=ToolScopeConfig(native=list(native)))) + + def _prompt(self, *native: str) -> str: + profile = self._profile(*native) + builder = ContextBuilder(profile_registry=make_profile_registry()) + return builder.build_system_prompt(profile) + + def test_a_profile_with_both_tools_gets_the_rules(self): + prompt = self._prompt("skills", "protocol") + + assert "## Methodologies" in prompt + assert '`skills(op="search")`' in prompt + assert '`protocol(op="start")`' in prompt + assert "do not call them with a guess" in prompt + assert '`tool_manual("skills")`' in prompt + assert '`tool_manual("protocol")`' in prompt + + def test_a_profile_without_the_tools_gets_nothing(self): + prompt = self._prompt("todo", "filesystem") + assert "## Methodologies" not in prompt + + def test_skills_only_does_not_advertise_protocol(self): + prompt = self._prompt("skills") + + assert "## Methodologies" in prompt + assert '`skills(op="search")`' in prompt + assert "protocol(op=" not in prompt + assert 'tool_manual("protocol")' not in prompt + + def test_protocol_only_does_not_advertise_skills(self): + prompt = self._prompt("protocol") + + assert '`protocol(op="start")`' in prompt + assert "skills(op=" not in prompt + assert 'tool_manual("skills")' not in prompt + # The "record what proved itself" rule is a skill rule — it must not + # point at a tool the profile does not have. + assert "Record what proved itself" not in prompt + + def test_a_subagent_scope_alone_does_not_count(self): + from navi.profiles.base import ToolConfig, ToolScopeConfig + + profile = make_profile( + "p", + tools=ToolConfig( + agent=ToolScopeConfig(native=["todo"]), + subagent=ToolScopeConfig(native=["skills", "protocol"]), + ), + ) + builder = ContextBuilder(profile_registry=make_profile_registry()) + assert "## Methodologies" not in builder.build_system_prompt(profile) + + def test_the_real_profiles_that_have_the_tools_get_the_block(self): + """Guards the config: adding the tools without the text is the bug this + block exists to prevent, and it is invisible in the tool list.""" + from navi.profiles import ALL_PROFILES + + builder = ContextBuilder(profile_registry=make_profile_registry()) + for profile in ALL_PROFILES: + native = profile.tools.agent.native + has_block = "## Methodologies" in builder.build_system_prompt(profile) + assert has_block == ("skills" in native), profile.id + + class TestBuildGoalAnchor: def test_includes_original_request(self): import asyncio