diff --git a/run_agent.py b/run_agent.py index 1d38d4a27..f5729dcd4 100644 --- a/run_agent.py +++ b/run_agent.py @@ -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/.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/.` — starter files meant to be " + "copied and modified (boilerplate configs, scaffolding, a " + "known-good example the agent can `reproduce with modifications`).\n" + " • `scripts/.` — 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/.md` for session-specific detail OR condensed " + "knowledge banks (quoted research, API docs excerpts, domain " + "notes) written concise and task-focused; `templates/.` " + "for starter files meant to be copied and modified; " + "`scripts/.` 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 diff --git a/tests/run_agent/test_review_prompt_class_first.py b/tests/run_agent/test_review_prompt_class_first.py index 4a7fed1d7..c9f30fa57 100644 --- a/tests/run_agent/test_review_prompt_class_first.py +++ b/tests/run_agent/test_review_prompt_class_first.py @@ -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