feat(review): active-update bias, loaded-skill-first, support-file variants (#17213)

The background skill-review prompts (_SKILL_REVIEW_PROMPT and the **Skills**
half of _COMBINED_REVIEW_PROMPT) steered the reviewer toward passive
behavior — most passes concluded 'Nothing to save.' even when the session
produced real lessons. User-preference corrections (style, format,
legibility, verbosity) were especially lost: they were read as memory
signals only, so skills never carried the fix.

This rewrite changes the stance:

- **Active-update bias.** The reviewer now treats inaction as a missed
  learning opportunity. 'Nothing to save.' remains an explicit escape
  but is no longer framed as the most-common outcome.

- **User-preference corrections are first-class skill signals.** Style,
  tone, format, legibility, verbosity complaints — and the actual
  phrasings users use ('stop doing X', 'this is too verbose', 'I hate
  when you Y', 'remember this') — now warrant patching the skill that
  governs the task, not just writing to memory.

- **Loaded-skill-first preference order.** When a skill was loaded via
  /skill-name or skill_view during the session, the reviewer patches
  THAT one first. It was in play; it's the right place.

- **Four-step ladder: patch-loaded → patch-umbrella → support-file →
  create.** Support files are explicitly enumerated as three kinds:
    * references/<topic>.md — session-specific detail OR condensed
      knowledge banks (quoted research, API docs excerpts, domain notes)
    * templates/<name>.<ext> — starter files to copy and modify
    * scripts/<name>.<ext>  — statically re-runnable actions

- **Name-veto for CREATE.** New skill names MUST be class-level — no PR
  numbers, error strings, codenames, library-alone names, or session
  artifacts ('fix-X / debug-Y / audit-Z-today'). If the proposed name
  only fits today's task, fall back to one of the patch/support-file
  options.

- **Memory scope clarified.** 'who the user is and what the current
  situation and state of your operations are' — MEMORY.md is
  situational/state, USER.md is identity/preferences.

- **Curator handoff.** Reviewer flags overlap; the background curator
  handles consolidation at scale. Single-session reviewer doesn't
  attempt umbrella-rebalancing.

Tests: tests/run_agent/test_review_prompt_class_first.py upgraded to
assert the new behavioral contracts (active bias, user-correction
signals, loaded-skill-first, support-file kinds, name-veto, memory
framing, curator handoff). 17 tests, all pass.

Co-authored-by: teknium1 <teknium@users.noreply.github.com>
This commit is contained in:
Teknium
2026-04-28 21:11:48 -07:00
committed by GitHub
parent 8c892c1453
commit 1d4218be56
2 changed files with 279 additions and 80 deletions

View File

@@ -3230,49 +3230,135 @@ class AIAgent:
)
_SKILL_REVIEW_PROMPT = (
"Review the conversation above and consider whether a skill should be saved or updated.\n\n"
"Work in this order — do not skip steps:\n\n"
"1. SURVEY the existing skill landscape first. Call skills_list to see what you "
"have. If anything looks potentially relevant, skill_view it before deciding. "
"You are looking for the CLASS of task that just happened, not the exact task. "
"Example: a successful Tauri build is in the class \"desktop app build "
"troubleshooting\", not \"fix my specific Tauri error today\".\n\n"
"2. THINK CLASS-FIRST. What general pattern of task did the user just complete? "
"What conditions will trigger this pattern again? Describe the class in one "
"sentence before looking at what to save.\n\n"
"3. PREFER GENERALIZING AN EXISTING SKILL over creating a new one. If a skill "
"already covers the class — even partially — update it (skill_manage patch) "
"with the new insight. Broaden its \"when to use\" trigger if needed.\n\n"
"4. ONLY CREATE A NEW SKILL when no existing skill reasonably covers the class. "
"When you create one, name and scope it at the class level "
"(\"react-i18n-setup\", not \"add-i18n-to-my-dashboard-app\"). The trigger "
"section must describe the class of situations, not this one session.\n\n"
"5. If you notice two existing skills that overlap, note it in your response "
"so a future review can consolidate them. Do not consolidate now unless the "
"overlap is obvious and low-risk.\n\n"
"Only act when something is genuinely worth saving. "
"If nothing stands out, just say 'Nothing to save.' and stop."
"Review the conversation above and update the skill library. Be "
"ACTIVE — most sessions produce at least one skill update, even if "
"small. A pass that does nothing is a missed learning opportunity, "
"not a neutral outcome.\n\n"
"Target shape of the library: CLASS-LEVEL skills, each with a rich "
"SKILL.md and a `references/` directory for session-specific detail. "
"Not a long flat list of narrow one-session-one-skill entries. This "
"shapes HOW you update, not WHETHER you update.\n\n"
"Signals to look for (any one of these warrants action):\n"
" • User corrected your style, tone, format, legibility, or "
"verbosity. Frustration signals like 'stop doing X', 'this is too "
"verbose', 'don't format like this', 'why are you explaining', "
"'just give me the answer', 'you always do Y and I hate it', or an "
"explicit 'remember this' are FIRST-CLASS skill signals, not just "
"memory signals. Update the relevant skill(s) to embed the "
"preference so the next session starts already knowing.\n"
" • User corrected your workflow, approach, or sequence of steps. "
"Encode the correction as a pitfall or explicit step in the skill "
"that governs that class of task.\n"
" • Non-trivial technique, fix, workaround, debugging path, or "
"tool-usage pattern emerged that a future session would benefit "
"from. Capture it.\n"
" • A skill that got loaded or consulted this session turned out "
"to be wrong, missing a step, or outdated. Patch it NOW.\n\n"
"Preference order — prefer the earliest action that fits, but do "
"pick one when a signal above fired:\n"
" 1. UPDATE A CURRENTLY-LOADED SKILL. Look back through the "
"conversation for skills the user loaded via /skill-name or you "
"read via skill_view. If any of them covers the territory of the "
"new learning, PATCH that one first. It is the skill that was in "
"play, so it's the right one to extend.\n"
" 2. UPDATE AN EXISTING UMBRELLA (via skills_list + skill_view). "
"If no loaded skill fits but an existing class-level skill does, "
"patch it. Add a subsection, a pitfall, or broaden a trigger.\n"
" 3. ADD A SUPPORT FILE under an existing umbrella. Skills can be "
"packaged with three kinds of support files — use the right "
"directory per kind:\n"
" • `references/<topic>.md` — session-specific detail (error "
"transcripts, reproduction recipes, provider quirks) AND "
"condensed knowledge banks: quoted research, API docs, external "
"authoritative excerpts, or domain notes you found while working "
"on the problem. Write it concise and for the value of the task, "
"not as a full mirror of upstream docs.\n"
" • `templates/<name>.<ext>` — starter files meant to be "
"copied and modified (boilerplate configs, scaffolding, a "
"known-good example the agent can `reproduce with modifications`).\n"
" • `scripts/<name>.<ext>` — statically re-runnable actions "
"the skill can invoke directly (verification scripts, fixture "
"generators, deterministic probes, anything the agent should run "
"rather than hand-type each time).\n"
" Add support files via skill_manage action=write_file with "
"file_path starting 'references/', 'templates/', or 'scripts/'. "
"The umbrella's SKILL.md should gain a one-line pointer to any "
"new support file so future agents know it exists.\n"
" 4. CREATE A NEW CLASS-LEVEL UMBRELLA SKILL when no existing "
"skill covers the class. The name MUST be at the class level. "
"The name MUST NOT be a specific PR number, error string, feature "
"codename, library-alone name, or 'fix-X / debug-Y / audit-Z-today' "
"session artifact. If the proposed name only makes sense for "
"today's task, it's wrong — fall back to (1), (2), or (3).\n\n"
"User-preference embedding (important): when the user expressed a "
"style/format/workflow preference, the update belongs in the "
"SKILL.md body, not just in memory. Memory captures 'who the user "
"is and what the current situation and state of your operations "
"are'; skills capture 'how to do this class of task for this "
"user'. When they complain about how you handled a task, the "
"skill that governs that task needs to carry the lesson.\n\n"
"If you notice two existing skills that overlap, note it in your "
"reply — the background curator handles consolidation at scale.\n\n"
"'Nothing to save.' is a real option but should NOT be the "
"default. If the session ran smoothly with no corrections and "
"produced no new technique, just say 'Nothing to save.' and stop. "
"Otherwise, act."
)
_COMBINED_REVIEW_PROMPT = (
"Review the conversation above and consider two things:\n\n"
"**Memory**: Has the user revealed things about themselves — their persona, "
"desires, preferences, or personal details? Has the user expressed expectations "
"about how you should behave, their work style, or ways they want you to operate? "
"If so, save using the memory tool.\n\n"
"**Skills**: Was a non-trivial approach used to complete a task that required trial "
"and error, changing course due to experiential findings, or a different method "
"or outcome than the user expected? If so, work in this order:\n"
" a. SURVEY existing skills first (skills_list, then skill_view on candidates).\n"
" b. Identify the CLASS of task, not the specific task "
"(\"desktop app build troubleshooting\", not \"fix my Tauri error\").\n"
" c. PREFER UPDATING/GENERALIZING an existing skill that covers the class.\n"
" d. ONLY CREATE A NEW SKILL if no existing one covers the class. Scope at "
"the class level, not this one session.\n"
" e. If you notice overlapping skills during the survey, note it so a future "
"review can consolidate them.\n\n"
"Only act if there's something genuinely worth saving. "
"If nothing stands out, just say 'Nothing to save.' and stop."
"Review the conversation above and update two things:\n\n"
"**Memory**: who the user is. Did the user reveal persona, "
"desires, preferences, personal details, or expectations about "
"how you should behave? Save facts about the user and durable "
"preferences with the memory tool.\n\n"
"**Skills**: how to do this class of task. Be ACTIVE — most "
"sessions produce at least one skill update. A pass that does "
"nothing is a missed learning opportunity, not a neutral outcome.\n\n"
"Target shape of the skill library: CLASS-LEVEL skills with a rich "
"SKILL.md and a `references/` directory for session-specific detail. "
"Not a long flat list of narrow one-session-one-skill entries.\n\n"
"Signals that warrant a skill update (any one is enough):\n"
" • User corrected your style, tone, format, legibility, "
"verbosity, or approach. Frustration is a FIRST-CLASS skill "
"signal, not just a memory signal. 'stop doing X', 'don't format "
"like this', 'I hate when you Y' — embed the lesson in the skill "
"that governs that task so the next session starts fixed.\n"
" • Non-trivial technique, fix, workaround, or debugging path "
"emerged.\n"
" • A skill that was loaded or consulted turned out wrong, "
"missing, or outdated — patch it now.\n\n"
"Preference order for skills — pick the earliest that fits:\n"
" 1. UPDATE A CURRENTLY-LOADED SKILL. Check what skills were "
"loaded via /skill-name or skill_view in the conversation. If one "
"of them covers the learning, PATCH it first. It was in play; "
"it's the right place.\n"
" 2. UPDATE AN EXISTING UMBRELLA (skills_list + skill_view to "
"find the right one). Patch it.\n"
" 3. ADD A SUPPORT FILE under an existing umbrella via "
"skill_manage action=write_file. Three kinds: "
"`references/<topic>.md` for session-specific detail OR condensed "
"knowledge banks (quoted research, API docs excerpts, domain "
"notes) written concise and task-focused; `templates/<name>.<ext>` "
"for starter files meant to be copied and modified; "
"`scripts/<name>.<ext>` for statically re-runnable actions "
"(verification, fixture generators, probes). Add a one-line "
"pointer in SKILL.md so future agents find them.\n"
" 4. CREATE A NEW CLASS-LEVEL UMBRELLA when nothing exists. "
"Name at the class level — NOT a PR number, error string, "
"codename, library-alone name, or 'fix-X / debug-Y' session "
"artifact. If the name only fits today's task, fall back to (1), "
"(2), or (3).\n\n"
"User-preference embedding: when the user complains about how "
"you handled a task, update the skill that governs that task — "
"memory alone isn't enough. Memory says 'who the user is and "
"what the current situation and state of your operations are'; "
"skills say 'how to do this class of task for this user'. Both "
"should carry user-preference lessons when relevant.\n\n"
"If you notice overlapping existing skills, mention it — the "
"background curator handles consolidation.\n\n"
"Act on whichever of the two dimensions has real signal. If "
"genuinely nothing stands out on either, say 'Nothing to save.' "
"and stop — but don't reach for that conclusion as a default."
)
@staticmethod

View File

@@ -1,67 +1,176 @@
"""Behavior tests for the class-first skill review prompts.
"""Behavior tests for the skill review / combined review prompts.
The skill review / combined review prompts steer the background review agent
toward generalizing existing skills rather than accumulating near-duplicates.
These tests assert the behavioral *instructions* are present — they do NOT
The review prompts steer the background review agent toward actively updating
the skill library after most sessions, with a strong bias toward:
1. Patching currently-loaded skills first,
2. Patching existing umbrellas next,
3. Adding references/ files under an existing umbrella,
4. Creating a new class-level umbrella only when nothing else fits.
User-preference corrections (style, format, verbosity, legibility) are
first-class skill signals, not just memory signals.
These tests assert behavioral *instructions* are present — they do NOT
snapshot the full prompt text (change-detector).
"""
from run_agent import AIAgent
def test_skill_review_prompt_instructs_survey_first():
"""Prompt must tell the reviewer to list existing skills before deciding."""
# ---------------------------------------------------------------------------
# _SKILL_REVIEW_PROMPT
# ---------------------------------------------------------------------------
def test_skill_review_prompt_biases_toward_active_updates():
"""Prompt must frame updating as the default stance, not something rare."""
prompt = AIAgent._SKILL_REVIEW_PROMPT
assert "skills_list" in prompt, "must instruct the reviewer to call skills_list"
assert "skill_view" in prompt, "must instruct the reviewer to skill_view candidates"
assert "SURVEY" in prompt, "must name the survey step explicitly"
def test_skill_review_prompt_is_class_first():
"""Prompt must steer toward the CLASS of task, not the specific task."""
prompt = AIAgent._SKILL_REVIEW_PROMPT
assert "CLASS" in prompt, "must tell the reviewer to think about the task class"
assert "class level" in prompt, "must anchor naming at the class level"
def test_skill_review_prompt_prefers_updating_existing():
"""Prompt must prefer generalizing an existing skill over creating a new one."""
prompt = AIAgent._SKILL_REVIEW_PROMPT
assert "PREFER GENERALIZING" in prompt or "PREFER UPDATING" in prompt, (
"must state the update-over-create preference"
assert "ACTIVE" in prompt or "active" in prompt.lower(), (
"must tell the reviewer to be active"
)
assert "ONLY CREATE A NEW SKILL" in prompt, (
"must gate new-skill creation behind a last-resort clause"
# "missed learning opportunity" or equivalent framing for not acting
assert "missed" in prompt.lower() or "opportunity" in prompt.lower(), (
"must frame inaction as a miss, not a neutral outcome"
)
def test_skill_review_prompt_flags_overlap_for_followup():
"""Prompt must ask the reviewer to note overlapping skills for future review."""
def test_skill_review_prompt_treats_user_corrections_as_skill_signal():
"""Style/format/verbosity complaints must be FIRST-CLASS skill signals, not just memory."""
prompt = AIAgent._SKILL_REVIEW_PROMPT
assert "overlap" in prompt.lower(), "must mention the overlap-flagging protocol"
lower = prompt.lower()
# Must mention style/format/verbosity-family corrections
assert any(k in lower for k in ("style", "format", "verbos", "legib", "tone")), (
"must name style/format/verbosity/legibility as signals"
)
# Must frame these as first-class skill signals (not memory-only)
assert "FIRST-CLASS" in prompt or "first-class" in prompt, (
"must explicitly label user-preference corrections as first-class skill signals"
)
# Must mention the correction-type phrases to tune the model's ear
assert "stop doing" in lower or "don't" in lower or "hate" in lower or "frustrat" in lower, (
"must give concrete phrasing examples so the model recognizes corrections"
)
def test_skill_review_prompt_preserves_opt_out_clause():
"""The 'Nothing to save.' escape clause must remain."""
def test_skill_review_prompt_prefers_loaded_skills_first():
"""Currently-loaded skills must be the first patch target."""
prompt = AIAgent._SKILL_REVIEW_PROMPT
assert "LOADED" in prompt or "loaded" in prompt, (
"must mention currently-loaded skills"
)
# Must name the mechanisms for detecting loaded skills
assert "skill_view" in prompt and "/skill" in prompt, (
"must name skill_view and /skill-name as loaded-skill signals"
)
def test_skill_review_prompt_has_four_step_preference_order():
"""The 4-step patch/support-file/create ladder must be present."""
prompt = AIAgent._SKILL_REVIEW_PROMPT
assert "PATCH" in prompt
assert "references/" in prompt or "REFERENCE" in prompt
assert "CREATE" in prompt
assert "UMBRELLA" in prompt or "umbrella" in prompt
def test_skill_review_prompt_names_three_support_file_kinds():
"""Support-file step must name references/, templates/, and scripts/."""
prompt = AIAgent._SKILL_REVIEW_PROMPT
assert "references/" in prompt, "must name references/ as a support-file kind"
assert "templates/" in prompt, "must name templates/ as a support-file kind"
assert "scripts/" in prompt, "must name scripts/ as a support-file kind"
# Purpose hints for each kind
assert "knowledge" in prompt.lower() or "research" in prompt.lower() or "API docs" in prompt, (
"must mention knowledge-bank / research / API-docs role of references/"
)
assert "copied" in prompt.lower() or "starter" in prompt.lower() or "reproduce" in prompt.lower(), (
"must mention that templates/ are starter files to copy/modify"
)
assert "re-runnable" in prompt.lower() or "verification" in prompt.lower() or "probe" in prompt.lower(), (
"must mention that scripts/ are re-runnable actions"
)
def test_skill_review_prompt_has_name_veto_for_create():
"""Creating a new skill must be gated behind class-level naming."""
prompt = AIAgent._SKILL_REVIEW_PROMPT
assert "class level" in prompt.lower() or "CLASS-LEVEL" in prompt
assert "MUST NOT" in prompt or "must not" in prompt, (
"must have a name-veto clause blocking session-artifact names"
)
def test_skill_review_prompt_embeds_user_preferences_in_skills():
"""Must explicitly say user-preference lessons belong in SKILL.md, not only memory."""
prompt = AIAgent._SKILL_REVIEW_PROMPT
lower = prompt.lower()
assert "preference" in lower, "must mention user preferences"
assert "memory" in lower and "skill" in lower, (
"must contrast memory vs skill responsibilities"
)
def test_skill_review_prompt_flags_overlap_and_defers_to_curator():
"""Reviewer should not consolidate live; flag overlap for the curator."""
prompt = AIAgent._SKILL_REVIEW_PROMPT
assert "overlap" in prompt.lower()
assert "curator" in prompt.lower(), "must defer consolidation to the curator"
def test_skill_review_prompt_still_has_opt_out_clause():
"""'Nothing to save.' must remain as a real-but-not-default option."""
prompt = AIAgent._SKILL_REVIEW_PROMPT
assert "Nothing to save." in prompt
def test_combined_review_prompt_keeps_memory_section():
"""Combined prompt must still cover memory review."""
# ---------------------------------------------------------------------------
# _COMBINED_REVIEW_PROMPT
# ---------------------------------------------------------------------------
def test_combined_review_prompt_has_memory_section():
"""Memory half must still cover user facts and preferences."""
prompt = AIAgent._COMBINED_REVIEW_PROMPT
assert "**Memory**" in prompt
assert "memory tool" in prompt
def test_combined_review_prompt_skills_section_is_class_first():
"""The **Skills** half of the combined prompt must follow the same protocol."""
def test_combined_review_prompt_skills_biased_toward_active_updates():
"""Skills half must carry the active-update bias."""
prompt = AIAgent._COMBINED_REVIEW_PROMPT
assert "**Skills**" in prompt
assert "SURVEY" in prompt
assert "CLASS" in prompt
assert "skills_list" in prompt
assert "ONLY CREATE A NEW SKILL" in prompt
assert "ACTIVE" in prompt or "active" in prompt.lower()
assert "missed" in prompt.lower() or "opportunity" in prompt.lower()
def test_combined_review_prompt_treats_user_corrections_as_skill_signal():
"""Combined prompt must carry the same user-preference-is-skill-signal rule."""
prompt = AIAgent._COMBINED_REVIEW_PROMPT
lower = prompt.lower()
assert any(k in lower for k in ("style", "format", "verbos", "legib", "tone"))
assert "FIRST-CLASS" in prompt or "first-class" in prompt
def test_combined_review_prompt_prefers_loaded_skills_first():
"""Combined prompt must also prefer loaded skills first."""
prompt = AIAgent._COMBINED_REVIEW_PROMPT
assert "LOADED" in prompt or "loaded" in prompt
assert "skill_view" in prompt and "/skill" in prompt
def test_combined_review_prompt_has_four_step_skill_ladder():
"""Combined prompt must keep the patch/support-file/create ladder on the Skills half."""
prompt = AIAgent._COMBINED_REVIEW_PROMPT
assert "PATCH" in prompt
assert "references/" in prompt or "REFERENCE" in prompt
assert "CREATE" in prompt
assert "CLASS-LEVEL" in prompt or "class-level" in prompt or "class level" in prompt.lower()
def test_combined_review_prompt_names_three_support_file_kinds():
"""Combined prompt must also name all three support-file kinds."""
prompt = AIAgent._COMBINED_REVIEW_PROMPT
assert "references/" in prompt
assert "templates/" in prompt
assert "scripts/" in prompt
def test_combined_review_prompt_preserves_opt_out_clause():
@@ -69,10 +178,14 @@ def test_combined_review_prompt_preserves_opt_out_clause():
assert "Nothing to save." in prompt
def test_memory_review_prompt_unchanged_in_structure():
# ---------------------------------------------------------------------------
# _MEMORY_REVIEW_PROMPT — unchanged, still memory-focused
# ---------------------------------------------------------------------------
def test_memory_review_prompt_still_focused_on_user_facts():
"""Memory-only review prompt stays focused on user facts — not touched by this change."""
prompt = AIAgent._MEMORY_REVIEW_PROMPT
# Guardrails: the memory-only prompt must NOT mention skills/surveys.
# The memory-only prompt should NOT drift into skill territory
assert "skills_list" not in prompt
assert "SURVEY" not in prompt
assert "memory tool" in prompt