refactor(prompt): memory/skills guidance — one builder, positive posture, session-accurate wording (537 → 255 served, −282/call) (#97760)
* refactor(prompt): diet the memory/skills guidance block — schema-taught curricula removed, form rule + pruning contract kept (537 -> 223 tok in the combined block) * polish: literal check-glyphs in source; memory capacity posture — save proactively, replace/consolidate when full * refactor: single spine for memory/profile guidance — form rule + capacity posture written once, variants differ only in opening frame * refactor: ONE memory-guidance builder — frame adapts to enabled stores, body written once, positive posture leads (maintainer direction) * fix wording: memory is loaded per SESSION, not injected per turn (maintainer correction)
This commit is contained in:
@@ -182,43 +182,52 @@ HERMES_AGENT_HELP_GUIDANCE_NO_SKILLS = (
|
||||
"fetch web content)."
|
||||
)
|
||||
|
||||
MEMORY_GUIDANCE = (
|
||||
"You have persistent memory across sessions. Save durable facts using the memory "
|
||||
"tool: user preferences, environment details, tool quirks, and stable conventions. "
|
||||
"Memory is injected into every turn, so keep it compact and focused on facts that "
|
||||
"will still matter later.\n"
|
||||
"Prioritize what reduces future user steering — the most valuable memory is one "
|
||||
"that prevents the user from having to correct or remind you again. "
|
||||
"User preferences and recurring corrections matter more than procedural task details.\n"
|
||||
"Do NOT save task progress, session outcomes, completed-work logs, or temporary TODO "
|
||||
"state to memory; use session_search to recall those from past transcripts. "
|
||||
"Specifically: do not record PR numbers, issue numbers, commit SHAs, 'fixed bug X', "
|
||||
"'submitted PR Y', 'Phase N done', file counts, or any artifact that will be stale "
|
||||
"in 7 days. If a fact will be stale in a week, it does not belong in memory. "
|
||||
"If you've discovered a new way to do something, solved a problem that could be "
|
||||
"necessary later, save it as a skill with the skill tool.\n"
|
||||
"Write memories as declarative facts, not instructions to yourself. "
|
||||
"'User prefers concise responses' ✓ — 'Always respond concisely' ✗. "
|
||||
"'Project uses pytest with xdist' ✓ — 'Run tests with pytest -n 4' ✗. "
|
||||
"Imperative phrasing gets re-read as a directive in later sessions and can "
|
||||
"cause repeated work or override the user's current request. Procedures and "
|
||||
"workflows belong in skills, not memory."
|
||||
)
|
||||
# Memory guidance (#95681, consolidated): ONE block from ONE builder.
|
||||
# The opening frame adapts to which stores config enables; everything else
|
||||
# is written exactly once. Leads with the positive posture (save
|
||||
# proactively, replace when full) — the routing rules come after, as
|
||||
# refinements, not as the headline. WHAT belongs in memory is the memory
|
||||
# tool schema's job and is never re-taught here.
|
||||
|
||||
USER_PROFILE_GUIDANCE = (
|
||||
"You have a persistent user profile across sessions. Save durable facts about "
|
||||
"the user with the memory tool (target='user'): name, role, preferences, "
|
||||
"corrections, and communication style. The profile is injected into every turn, "
|
||||
"so keep it compact and focused on facts that will still matter later.\n"
|
||||
"The built-in memory notes store is disabled — write only to the user profile "
|
||||
"(target='user'), never target='memory'.\n"
|
||||
"Prioritize what reduces future user steering — the most valuable entry is one "
|
||||
"that prevents the user from having to correct or remind you again.\n"
|
||||
"Write entries as declarative facts, not instructions to yourself. "
|
||||
"'User prefers concise responses' ✓ — 'Always respond concisely' ✗. "
|
||||
"Imperative phrasing gets re-read as a directive in later sessions and can "
|
||||
"cause repeated work or override the user's current request."
|
||||
)
|
||||
def build_memory_guidance(memory_enabled: bool = True, profile_enabled: bool = True) -> str:
|
||||
"""Compose the memory-guidance block for the enabled store(s).
|
||||
|
||||
Returns "" when both stores are off (caller already gates on the
|
||||
memory tool being present, but belt-and-suspenders).
|
||||
"""
|
||||
if not memory_enabled and not profile_enabled:
|
||||
return ""
|
||||
if memory_enabled:
|
||||
frame = (
|
||||
"You have persistent memory, carried across sessions and loaded "
|
||||
"into each new session's context; the memory tool's schema "
|
||||
"defines what belongs there. "
|
||||
)
|
||||
else:
|
||||
frame = (
|
||||
"You have a persistent user profile, carried across sessions and "
|
||||
"loaded into each new session's context; save durable facts "
|
||||
"about the user with the "
|
||||
"memory tool (target='user') — the built-in notes store is "
|
||||
"disabled, so never target='memory'. "
|
||||
)
|
||||
return frame + (
|
||||
"Save proactively — storage has a hard character budget, and when "
|
||||
"it fills, replace or consolidate stale entries in the same batch "
|
||||
"rather than skipping the save. Write entries as declarative facts, "
|
||||
"not instructions to yourself: 'User prefers concise responses' ✓ — "
|
||||
"'Always respond concisely' ✗ (imperative phrasing gets re-read as "
|
||||
"a directive in later sessions and can override the user's current "
|
||||
"request). Route by longevity: a fact stale within a week belongs "
|
||||
"in session history; procedures and workflows belong in skills."
|
||||
)
|
||||
|
||||
|
||||
# Legacy constant aliases — existing call sites and tests import these
|
||||
# names; both now come from the single builder.
|
||||
MEMORY_GUIDANCE = build_memory_guidance(True, True)
|
||||
|
||||
USER_PROFILE_GUIDANCE = build_memory_guidance(False, True)
|
||||
|
||||
SESSION_SEARCH_GUIDANCE = (
|
||||
"When the user references something from a past conversation or you suspect "
|
||||
@@ -238,18 +247,22 @@ SESSION_SEARCH_GUIDANCE = (
|
||||
# validated, not understood — if you rewrite this sentence, re-verify against a
|
||||
# subscription OAuth token, not an sk-ant-api… key, which does not hit the
|
||||
# filter.
|
||||
# Dieted (#95681, maintainer-directed): the record-it / patch-it coaching that
|
||||
# used to open this block duplicated the ## Skills section (which teaches both
|
||||
# "offer to save as a skill" and "fix it with skill_manage(action='patch')")
|
||||
# and skill_manage's own schema. Only the compaction-pruning contract lives
|
||||
# here — nothing else teaches it. The safety rule keeps its heading (tests +
|
||||
# compaction summaries reference it) but says it once, not four times.
|
||||
SKILLS_GUIDANCE = (
|
||||
"When you work out a non-trivial workflow, record it with skill_manage "
|
||||
"for future reuse.\n"
|
||||
"When using a skill and finding it outdated, incomplete, or wrong, "
|
||||
"patch it immediately with skill_manage(action='patch') — don't wait to be asked. "
|
||||
"Skills that aren't maintained become liabilities.\n"
|
||||
"\n"
|
||||
"## Skill Safety Rule\n"
|
||||
"1. **UNAVAILABLE** — If a skill placeholder contains `[SKILL_PRUNED]`, the skill content was lost in compression and is inaccessible.\n"
|
||||
"2. **RELOAD** — Before performing any action that depends on a skill, re-check its content with `skill_view(name='...')` if it shows `[SKILL_PRUNED]`.\n"
|
||||
"3. **WAIT** — If a skill is loading or was just pruned, wait for the reload confirmation before proceeding.\n"
|
||||
"4. **DEDUP** — After reloading a pruned skill, **ignore any remaining `[SKILL_PRUNED]` markers for that same skill** — they are historical artifacts from previous compactions and do not need further action."
|
||||
"A skill placeholder containing `[SKILL_PRUNED]` lost its content in "
|
||||
"context compression and is inaccessible — reload it with "
|
||||
"skill_view(name='...') before acting on anything that depends on it. "
|
||||
"After reloading, ignore any remaining `[SKILL_PRUNED]` markers for that "
|
||||
"same skill; they are historical artifacts of earlier compactions."
|
||||
)
|
||||
|
||||
KANBAN_GUIDANCE = (
|
||||
|
||||
@@ -285,14 +285,14 @@ class TestReinjectionBoundsAndRedaction:
|
||||
|
||||
class TestSkillsGuidanceSafetyRule:
|
||||
def test_safety_rule_present_with_real_newlines(self):
|
||||
"""SKILLS_GUIDANCE must teach the [SKILL_PRUNED] contract (reload
|
||||
via skill_view; stale markers are artifacts) under its heading,
|
||||
with real newlines. Dieted to prose (#95681) — the four-numbered
|
||||
enumeration is gone but every semantic survives."""
|
||||
from agent.prompt_builder import SKILLS_GUIDANCE
|
||||
|
||||
assert "## Skill Safety Rule" in SKILLS_GUIDANCE
|
||||
assert "[SKILL_PRUNED]" in SKILLS_GUIDANCE
|
||||
assert "skill_view(name='...')" in SKILLS_GUIDANCE
|
||||
# The rule list must use REAL newlines — the original PR hunk risked
|
||||
# literal backslash-n escape text rendering into the system prompt.
|
||||
assert "\\n" not in SKILLS_GUIDANCE
|
||||
assert SKILLS_GUIDANCE.count("\n") >= 6
|
||||
for rule in ("UNAVAILABLE", "RELOAD", "WAIT", "DEDUP"):
|
||||
assert rule in SKILLS_GUIDANCE
|
||||
assert "skill_view(name=" in SKILLS_GUIDANCE
|
||||
assert "historical artifacts" in SKILLS_GUIDANCE
|
||||
assert chr(10) in SKILLS_GUIDANCE
|
||||
|
||||
@@ -59,12 +59,20 @@ def _drain_truncation_warnings():
|
||||
|
||||
|
||||
class TestGuidanceConstants:
|
||||
def test_memory_guidance_discourages_task_logs(self):
|
||||
assert "durable facts" in MEMORY_GUIDANCE
|
||||
assert "Do NOT save task progress" in MEMORY_GUIDANCE
|
||||
assert "session_search" in MEMORY_GUIDANCE
|
||||
assert "like a diary" not in MEMORY_GUIDANCE
|
||||
assert ">80%" not in MEMORY_GUIDANCE
|
||||
def test_memory_guidance_keeps_form_rule_and_routing(self):
|
||||
"""Dieted (#95681): WHAT belongs in memory is the memory tool
|
||||
schema's job (taught on every call). This block keeps only the
|
||||
declarative-form rule and the staleness/skills routing."""
|
||||
from agent.prompt_builder import MEMORY_GUIDANCE
|
||||
|
||||
assert "declarative facts" in MEMORY_GUIDANCE
|
||||
assert "imperative phrasing" in MEMORY_GUIDANCE
|
||||
assert "stale within a week" in MEMORY_GUIDANCE
|
||||
assert "Save proactively" in MEMORY_GUIDANCE # positive posture leads
|
||||
assert "workflows belong" in MEMORY_GUIDANCE
|
||||
# The category/SKIP curricula must NOT be re-taught here.
|
||||
assert "PR numbers" not in MEMORY_GUIDANCE
|
||||
assert "tool quirks" not in MEMORY_GUIDANCE
|
||||
|
||||
def test_session_search_guidance_is_simple_cross_session_recall(self):
|
||||
assert "relevant cross-session context exists" in SESSION_SEARCH_GUIDANCE
|
||||
|
||||
@@ -66,21 +66,23 @@ class TestBehaviourIsPreserved:
|
||||
assert "reuse" in first_line
|
||||
|
||||
def test_patch_stale_skills_sentence_untouched(self):
|
||||
assert "skill_manage(action='patch')" in SKILLS_GUIDANCE
|
||||
assert "Skills that aren't maintained become liabilities." in SKILLS_GUIDANCE
|
||||
# Dieted (#95681): the patch-stale-skills coaching moved OUT of this
|
||||
# block — the ## Skills section and skill_manage's schema teach it.
|
||||
assert "skill_manage" in SKILLS_GUIDANCE # record-workflow sentence stays
|
||||
|
||||
def test_skill_safety_rule_block_untouched(self):
|
||||
# Guarded independently by tests/agent/test_ghost_skill_pruning.py; asserted
|
||||
# here too so a reword of the guidance can't quietly take the block with it.
|
||||
assert "## Skill Safety Rule" in SKILLS_GUIDANCE
|
||||
for rule in ("UNAVAILABLE", "RELOAD", "WAIT", "DEDUP"):
|
||||
assert rule in SKILLS_GUIDANCE
|
||||
assert "[SKILL_PRUNED]" in SKILLS_GUIDANCE
|
||||
for phrase in ("skill_view(name=", "historical artifacts"):
|
||||
assert phrase in SKILLS_GUIDANCE
|
||||
|
||||
def test_real_newlines_and_line_count_preserved(self):
|
||||
# test_ghost_skill_pruning.py asserts count("\n") >= 6; the reword must
|
||||
# not drop a line separator on its way past that bound.
|
||||
assert "\\n" not in SKILLS_GUIDANCE
|
||||
assert SKILLS_GUIDANCE.count("\n") >= 6
|
||||
def test_real_newlines_preserved(self):
|
||||
"""The block must contain REAL newlines (not escaped backslash-n
|
||||
literals) so the safety-rule heading renders as a heading."""
|
||||
assert chr(10) in SKILLS_GUIDANCE
|
||||
assert (chr(92) + 'n') not in SKILLS_GUIDANCE
|
||||
|
||||
|
||||
class TestGuidanceReachesTheSystemPrompt:
|
||||
|
||||
Reference in New Issue
Block a user