From d35ee7bcdd652715864d0d0c262790293a2555c6 Mon Sep 17 00:00:00 2001 From: teknium1 <127238744+teknium1@users.noreply.github.com> Date: Sat, 16 May 2026 19:11:58 -0700 Subject: [PATCH] refactor(run_agent): move review prompts to agent/background_review.py MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The three big review-prompt strings (_MEMORY_REVIEW_PROMPT, _SKILL_REVIEW_PROMPT, _COMBINED_REVIEW_PROMPT — 183 lines combined) move out of the AIAgent class body and into agent/background_review.py where they're consumed. AIAgent re-exposes them as class attributes via 'from ... import' inside the class body — Python binds those names into the class namespace so existing AIAgent._MEMORY_REVIEW_PROMPT references keep working. spawn_background_review_thread also falls back to the module-level constants if an agent doesn't have the attribute (preserves the test pattern of mocking these on the agent). tests/run_agent/ + tests/agent/: 4313 passed (same pre-existing test_auxiliary_client failure). run_agent.py: 9986 -> 9800 lines (-186). --- agent/background_review.py | 202 ++++++++++++++++++++++++++++++++++++- run_agent.py | 188 +--------------------------------- 2 files changed, 203 insertions(+), 187 deletions(-) diff --git a/agent/background_review.py b/agent/background_review.py index 351ab1d43dc..0319bbfa046 100644 --- a/agent/background_review.py +++ b/agent/background_review.py @@ -27,6 +27,195 @@ from typing import Any, Dict, List, Optional logger = logging.getLogger(__name__) +# Review-prompt strings — used by ``spawn_background_review_thread`` to build +# the user-message that the forked review agent receives. AIAgent exposes +# them as class attributes (``_MEMORY_REVIEW_PROMPT`` etc.) for back-compat; +# the actual text lives here so future edits are one-place. +_MEMORY_REVIEW_PROMPT = ( + "Review the conversation above and consider saving to memory if appropriate.\n\n" + "Focus on:\n" + "1. Has the user revealed things about themselves — their persona, desires, " + "preferences, or personal details worth remembering?\n" + "2. Has the user expressed expectations about how you should behave, their work " + "style, or ways they want you to operate?\n\n" + "If something stands out, save it using the memory tool. " + "If nothing is worth saving, just say 'Nothing to save.' and stop." +) + +_SKILL_REVIEW_PROMPT = ( + "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" + "Do NOT capture (these become persistent self-imposed constraints " + "that bite you later when the environment changes):\n" + " • Environment-dependent failures: missing binaries, fresh-install " + "errors, post-migration path mismatches, 'command not found', " + "unconfigured credentials, uninstalled packages. The user can fix " + "these — they are not durable rules.\n" + " • Negative claims about tools or features ('browser tools do not " + "work', 'X tool is broken', 'cannot use Y from execute_code'). These " + "harden into refusals the agent cites against itself for months " + "after the actual problem was fixed.\n" + " • Session-specific transient errors that resolved before the " + "conversation ended. If retrying worked, the lesson is the retry " + "pattern, not the original failure.\n" + " • One-off task narratives. A user asking 'summarize today's " + "market' or 'analyze this PR' is not a class of work that warrants " + "a skill.\n\n" + "If a tool failed because of setup state, capture the FIX (install " + "command, config step, env var to set) under an existing setup or " + "troubleshooting skill — never 'this tool does not work' as a " + "standalone constraint.\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 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" + "Do NOT capture as skills (these become persistent self-imposed " + "constraints that bite you later when the environment changes):\n" + " • Environment-dependent failures: missing binaries, fresh-install " + "errors, post-migration path mismatches, 'command not found', " + "unconfigured credentials, uninstalled packages. The user can fix " + "these — they are not durable rules.\n" + " • Negative claims about tools or features ('browser tools do not " + "work', 'X tool is broken', 'cannot use Y from execute_code'). These " + "harden into refusals the agent cites against itself for months " + "after the actual problem was fixed.\n" + " • Session-specific transient errors that resolved before the " + "conversation ended. If retrying worked, the lesson is the retry " + "pattern, not the original failure.\n" + " • One-off task narratives. A user asking 'summarize today's " + "market' or 'analyze this PR' is not a class of work that warrants " + "a skill.\n\n" + "If a tool failed because of setup state, capture the FIX (install " + "command, config step, env var to set) under an existing setup or " + "troubleshooting skill — never 'this tool does not work' as a " + "standalone constraint.\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." +) + + + def summarize_background_review_actions( review_messages: List[Dict], prior_snapshot: List[Dict], @@ -339,13 +528,15 @@ def spawn_background_review_thread( owns the actual ``threading.Thread`` construction so test-level patches of ``run_agent.threading.Thread`` keep working. """ - # Pick the right prompt based on which triggers fired + # Pick the right prompt based on which triggers fired. Allow per-agent + # override (the prompts moved to module-level constants but old code paths + # that set agent._MEMORY_REVIEW_PROMPT etc. directly keep working). if review_memory and review_skills: - prompt = agent._COMBINED_REVIEW_PROMPT + prompt = getattr(agent, "_COMBINED_REVIEW_PROMPT", _COMBINED_REVIEW_PROMPT) elif review_memory: - prompt = agent._MEMORY_REVIEW_PROMPT + prompt = getattr(agent, "_MEMORY_REVIEW_PROMPT", _MEMORY_REVIEW_PROMPT) else: - prompt = agent._SKILL_REVIEW_PROMPT + prompt = getattr(agent, "_SKILL_REVIEW_PROMPT", _SKILL_REVIEW_PROMPT) def _target() -> None: _run_review_in_thread(agent, messages_snapshot, prompt) @@ -354,6 +545,9 @@ def spawn_background_review_thread( __all__ = [ + "_MEMORY_REVIEW_PROMPT", + "_SKILL_REVIEW_PROMPT", + "_COMBINED_REVIEW_PROMPT", "spawn_background_review_thread", "summarize_background_review_actions", "build_memory_write_metadata", diff --git a/run_agent.py b/run_agent.py index 8d6f7c3f35c..8ea73167ac9 100644 --- a/run_agent.py +++ b/run_agent.py @@ -2521,190 +2521,12 @@ class AIAgent: return cleanup_task_resources(self, task_id) # ------------------------------------------------------------------ - # Background memory/skill review + # Background memory/skill review — prompts live in agent.background_review # ------------------------------------------------------------------ - - _MEMORY_REVIEW_PROMPT = ( - "Review the conversation above and consider saving to memory if appropriate.\n\n" - "Focus on:\n" - "1. Has the user revealed things about themselves — their persona, desires, " - "preferences, or personal details worth remembering?\n" - "2. Has the user expressed expectations about how you should behave, their work " - "style, or ways they want you to operate?\n\n" - "If something stands out, save it using the memory tool. " - "If nothing is worth saving, just say 'Nothing to save.' and stop." - ) - - _SKILL_REVIEW_PROMPT = ( - "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" - "Do NOT capture (these become persistent self-imposed constraints " - "that bite you later when the environment changes):\n" - " • Environment-dependent failures: missing binaries, fresh-install " - "errors, post-migration path mismatches, 'command not found', " - "unconfigured credentials, uninstalled packages. The user can fix " - "these — they are not durable rules.\n" - " • Negative claims about tools or features ('browser tools do not " - "work', 'X tool is broken', 'cannot use Y from execute_code'). These " - "harden into refusals the agent cites against itself for months " - "after the actual problem was fixed.\n" - " • Session-specific transient errors that resolved before the " - "conversation ended. If retrying worked, the lesson is the retry " - "pattern, not the original failure.\n" - " • One-off task narratives. A user asking 'summarize today's " - "market' or 'analyze this PR' is not a class of work that warrants " - "a skill.\n\n" - "If a tool failed because of setup state, capture the FIX (install " - "command, config step, env var to set) under an existing setup or " - "troubleshooting skill — never 'this tool does not work' as a " - "standalone constraint.\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 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" - "Do NOT capture as skills (these become persistent self-imposed " - "constraints that bite you later when the environment changes):\n" - " • Environment-dependent failures: missing binaries, fresh-install " - "errors, post-migration path mismatches, 'command not found', " - "unconfigured credentials, uninstalled packages. The user can fix " - "these — they are not durable rules.\n" - " • Negative claims about tools or features ('browser tools do not " - "work', 'X tool is broken', 'cannot use Y from execute_code'). These " - "harden into refusals the agent cites against itself for months " - "after the actual problem was fixed.\n" - " • Session-specific transient errors that resolved before the " - "conversation ended. If retrying worked, the lesson is the retry " - "pattern, not the original failure.\n" - " • One-off task narratives. A user asking 'summarize today's " - "market' or 'analyze this PR' is not a class of work that warrants " - "a skill.\n\n" - "If a tool failed because of setup state, capture the FIX (install " - "command, config step, env var to set) under an existing setup or " - "troubleshooting skill — never 'this tool does not work' as a " - "standalone constraint.\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." + from agent.background_review import ( + _MEMORY_REVIEW_PROMPT, + _SKILL_REVIEW_PROMPT, + _COMBINED_REVIEW_PROMPT, ) @staticmethod