From 3755dca7d6aa8b4da01cb3aec69fac8ede9c6c3c Mon Sep 17 00:00:00 2001 From: fangliquan Date: Thu, 3 Sep 2026 14:31:16 +0800 Subject: [PATCH 1/4] fix(agent): handle usage-less empty responses --- agent/conversation_loop.py | 32 ++++++++++---- agent/empty_response_guard.py | 42 +++++++++++-------- tests/agent/test_empty_response_guard.py | 30 ++++++++++--- .../test_empty_terminal_reasoning_surface.py | 14 +++---- tests/run_agent/test_run_agent.py | 26 ++++++++---- 5 files changed, 98 insertions(+), 46 deletions(-) diff --git a/agent/conversation_loop.py b/agent/conversation_loop.py index 50d49be442..b6978abbc8 100644 --- a/agent/conversation_loop.py +++ b/agent/conversation_loop.py @@ -4609,6 +4609,11 @@ def run_conversation( "error": "First response truncated due to output length limit" } + # Count every completed provider attempt, including providers + # that omit usage. Token/cost accounting below remains gated on + # real usage, but the request itself must stay observable. + agent.session_api_calls += 1 + # Track actual token usage from response for context management if hasattr(response, 'usage') and response.usage: canonical_usage = normalize_usage( @@ -4783,7 +4788,6 @@ def run_conversation( agent.session_prompt_tokens += prompt_tokens agent.session_completion_tokens += completion_tokens agent.session_total_tokens += total_tokens - agent.session_api_calls += 1 agent.session_input_tokens += canonical_usage.input_tokens agent.session_output_tokens += canonical_usage.output_tokens agent.session_cache_read_tokens += canonical_usage.cache_read_tokens @@ -4930,6 +4934,15 @@ def run_conversation( f"{cached:,}/{prompt:,} tokens " f"({hit_pct:.0f}% hit, {written:,} written)" ) + else: + logger.info( + "API call #%d: model=%s provider=%s in=? out=? total=? " + "latency=%.1fs usage=unavailable", + agent.session_api_calls, + agent.model, + agent.provider or "unknown", + api_duration, + ) _retry.has_retried_429 = False # Reset on success # Note: don't clear the retry buffer here — an "API call @@ -8552,6 +8565,7 @@ def run_conversation( agent, finish_reason=finish_reason, response=response, + observed_generation=_has_structured, ) _empty_retry_budget = ( _empty_guard.empty_retry_budget(agent, response) @@ -8619,15 +8633,14 @@ def run_conversation( if _truly_empty and _deterministic_empty: logger.warning( - "Deterministic empty response detected " - "(consecutive zero-output completions, " - "model=%s provider=%s finish_reason=%s) — " + "Repeated empty response detected " + "(model=%s provider=%s finish_reason=%s) — " "skipping remaining retries", agent.model, agent.provider, finish_reason, ) agent._buffer_status( - "⚠️ Model is deterministically returning empty " - "(zero output tokens) — skipping further retries " + "⚠️ Model is repeatedly returning empty content — " + "skipping further retries " "to avoid repeat charges" ) @@ -8748,7 +8761,12 @@ def run_conversation( "answer:\n\n" + reasoning_preview ) else: - final_response = "(empty)" + final_response = agent._format_turn_completion_explanation( + _turn_exit_reason + ) or ( + "⚠️ No reply: the model returned empty content after " + "all retries and fallback attempts." + ) break # Reset retry counter/signature on successful content diff --git a/agent/empty_response_guard.py b/agent/empty_response_guard.py index fbde4b58b0..293d677bfd 100644 --- a/agent/empty_response_guard.py +++ b/agent/empty_response_guard.py @@ -15,15 +15,13 @@ like this). Two independent guards, both failing OPEN to today's behaviour: -1. **Deterministic-empty detection** — two consecutive empty attempts, - both with usage present and ``output_tokens == 0``, from the same - (model, provider, finish_reason), are treated as deterministic: the - same prompt will keep producing the same empty. Remaining retries are - skipped and the loop proceeds straight to the fallback chain (a - different model may behave differently). Attempts with missing usage - or ``output_tokens > 0`` (model generated *something* — think-block - stripping, whitespace, flaky decoding) never classify as deterministic - and keep the full retry budget. +1. **Deterministic-empty detection** — two consecutive empty attempts from + the same (model, provider, finish_reason) are treated as deterministic + when usage proves zero output, or when usage is absent and the assembled + responses contain neither content nor reasoning. Remaining retries are + skipped and the loop proceeds straight to the fallback chain (a different + model may behave differently). Mixed evidence or any generated tokens keep + the full retry budget. 2. **Cost-aware retry budget** — when the estimated input cost of a single empty attempt exceeds the configured threshold (default @@ -78,6 +76,7 @@ class EmptyAttempt: finish_reason: str usage_present: bool zero_output: bool + observed_generation: bool @property def signature(self) -> tuple: @@ -201,7 +200,13 @@ def _zero_output(agent: Any, response: Any) -> tuple: return (True, (output + reasoning) == 0) -def record_empty_attempt(agent: Any, *, finish_reason: str, response: Any) -> None: +def record_empty_attempt( + agent: Any, + *, + finish_reason: str, + response: Any, + observed_generation: bool = True, +) -> None: """Record one empty completion in the current streak. Must be called before ``_empty_content_retries`` is incremented for @@ -222,6 +227,7 @@ def record_empty_attempt(agent: Any, *, finish_reason: str, response: Any) -> No finish_reason=str(finish_reason or ""), usage_present=usage_present, zero_output=zero_output, + observed_generation=bool(observed_generation), ) ) @@ -234,10 +240,10 @@ def record_empty_attempt(agent: Any, *, finish_reason: str, response: Any) -> No def deterministic_empty(agent: Any) -> bool: """True when the current streak looks deterministic. - Requires >= 2 consecutive attempts, ALL with usage present, zero - output tokens, and an identical (model, provider, finish_reason) - signature. Any attempt with missing usage or non-zero output keeps - this False (fail open — transients deserve their retries). + Requires >= 2 consecutive attempts with an identical (model, provider, + finish_reason) signature. Usage-backed attempts must all prove zero output. + Usage-absent attempts must all have no observed content or reasoning. Mixed + evidence fails open so ambiguous transients keep their retries. """ if not guard_enabled(agent): return False @@ -245,10 +251,12 @@ def deterministic_empty(agent: Any) -> bool: if len(attempts) < 2: return False first = attempts[0] - return all( - a.usage_present and a.zero_output and a.signature == first.signature - for a in attempts + same_signature = all(a.signature == first.signature for a in attempts) + usage_proves_empty = all(a.usage_present and a.zero_output for a in attempts) + response_proves_empty = all( + not a.usage_present and not a.observed_generation for a in attempts ) + return same_signature and (usage_proves_empty or response_proves_empty) def empty_retry_budget(agent: Any, response: Any) -> int: diff --git a/tests/agent/test_empty_response_guard.py b/tests/agent/test_empty_response_guard.py index 3ec1a0c437..c8e6f0dac4 100644 --- a/tests/agent/test_empty_response_guard.py +++ b/tests/agent/test_empty_response_guard.py @@ -5,7 +5,8 @@ deterministic empties (unsignaled provider refusals with zero output tokens) while never tightening behaviour on ambiguous evidence. Fail-open contract under test: -- Missing usage -> never deterministic, default budget. +- Missing usage + no observed generation -> deterministic after two attempts. +- Missing usage + observed reasoning -> never deterministic. - Any generated tokens (output or reasoning) -> never deterministic. - Different model/provider/finish_reason across attempts -> not deterministic. - Guard disabled via config (agent.empty_response_guard.enabled: false) -> @@ -42,11 +43,21 @@ def _response(prompt_tokens=25_900, completion_tokens=0, usage_present=True): return SimpleNamespace(usage=usage) -def _record_streak(agent, responses, finish_reasons=None): +def _record_streak( + agent, responses, finish_reasons=None, observed_generations=None +): """Record attempts the way the loop does: record, then increment.""" finish_reasons = finish_reasons or ["stop"] * len(responses) - for resp, reason in zip(responses, finish_reasons): - guard.record_empty_attempt(agent, finish_reason=reason, response=resp) + observed_generations = observed_generations or [False] * len(responses) + for resp, reason, observed_generation in zip( + responses, finish_reasons, observed_generations + ): + guard.record_empty_attempt( + agent, + finish_reason=reason, + response=resp, + observed_generation=observed_generation, + ) agent._empty_content_retries += 1 @@ -62,12 +73,21 @@ class TestDeterministicEmpty: _record_streak(agent, [_response()]) assert guard.deterministic_empty(agent) is False - def test_missing_usage_fails_open(self): + def test_missing_usage_without_observed_generation_is_deterministic(self): agent = _agent() _record_streak( agent, [_response(usage_present=False), _response(usage_present=False)], ) + assert guard.deterministic_empty(agent) is True + + def test_missing_usage_with_observed_reasoning_fails_open(self): + agent = _agent() + _record_streak( + agent, + [_response(usage_present=False), _response(usage_present=False)], + observed_generations=[True, True], + ) assert guard.deterministic_empty(agent) is False def test_mixed_usage_presence_fails_open(self): diff --git a/tests/run_agent/test_empty_terminal_reasoning_surface.py b/tests/run_agent/test_empty_terminal_reasoning_surface.py index 7cfd1aea02..3b56144cfe 100644 --- a/tests/run_agent/test_empty_terminal_reasoning_surface.py +++ b/tests/run_agent/test_empty_terminal_reasoning_surface.py @@ -11,7 +11,7 @@ Invariants pinned here: ``_empty_terminal_sentinel`` marker (replay semantics unchanged). - Raw reasoning is NEVER promoted earlier in the ladder — a reasoning-only response still goes through prefill continuation first. -- A truly empty exhaustion (no reasoning either) still returns "(empty)". +- A truly empty exhaustion returns an actionable no-reply explanation. """ from __future__ import annotations @@ -109,11 +109,11 @@ def test_exhausted_reasoning_only_delivers_labeled_excerpt(tmp_path, monkeypatch ) -def test_exhausted_truly_empty_keeps_existing_behavior(tmp_path, monkeypatch): - """No reasoning anywhere → behavior unchanged from main: the '(empty)' - terminal (possibly rewritten by the downstream turn-completion explainer) - is delivered, and no reasoning excerpt appears.""" +def test_exhausted_truly_empty_delivers_no_reply_explanation(tmp_path, monkeypatch): + """No reasoning anywhere produces an honest terminal explanation even + when the downstream completion explainer is disabled.""" agent = _build_agent(tmp_path, monkeypatch) + monkeypatch.setattr(agent, "_turn_completion_explainer_enabled", lambda: False) monkeypatch.setattr( agent, "_interruptible_api_call", lambda api_kwargs: _truly_empty_response(), @@ -122,9 +122,7 @@ def test_exhausted_truly_empty_keeps_existing_behavior(tmp_path, monkeypatch): result = agent.run_conversation("hello?") final = result["final_response"] - # Either the raw sentinel (explainer off) or the explainer's rewrite — - # never the reasoning-excerpt frame, which requires reasoning to exist. - assert final == "(empty)" or final.startswith("⚠️ No reply:") + assert final.startswith("⚠️ No reply:") assert "only internal reasoning" not in final diff --git a/tests/run_agent/test_run_agent.py b/tests/run_agent/test_run_agent.py index 05af3b8d1d..3c03e5cde1 100644 --- a/tests/run_agent/test_run_agent.py +++ b/tests/run_agent/test_run_agent.py @@ -3513,12 +3513,12 @@ class TestRunConversation: assert result["api_calls"] == 6 # 1 original + 2 prefill + 3 retries - def test_truly_empty_response_retries_3_times_then_empty(self, agent): - """Truly empty response (no content, no reasoning) retries 3 times then falls through to (empty).""" + def test_truly_empty_response_stops_after_repeated_empty(self, agent): + """Repeated empty responses stop after one retry and return an explanation.""" self._setup_agent(agent) agent.base_url = "http://127.0.0.1:1234/v1" empty_resp = _mock_response(content=None, finish_reason="stop") - # 4 responses: 1 original + 3 nudge retries, all empty + # Extra responses prove the guard stops consuming after repetition. agent.client.chat.completions.create.side_effect = [ empty_resp, empty_resp, empty_resp, empty_resp, ] @@ -3532,7 +3532,7 @@ class TestRunConversation: # #34452: explanation replaces the bare "(empty)" sentinel. assert result["final_response"] != "(empty)" assert "No reply:" in result["final_response"] - assert result["api_calls"] == 4 # 1 original + 3 retries + assert result["api_calls"] == 2 # 1 original + 1 retry def test_deterministic_empty_stops_retries_early(self, agent): """NS-503: consecutive zero-output-token empties with identical @@ -3588,10 +3588,11 @@ class TestRunConversation: assert result["completed"] is True assert result["api_calls"] == 4 # legacy: 1 original + 3 retries - def test_empty_without_usage_keeps_full_retry_budget(self, agent): - """NS-503 fail-open: no usage data means no evidence of a - deterministic empty — legacy 3-retry behaviour must be preserved - (this is the flaky-provider case retries exist for).""" + def test_empty_without_usage_stops_after_one_retry_and_logs_calls( + self, agent, caplog + ): + """Two complete empty responses are enough evidence to stop even when + the provider omits usage; both attempts remain observable.""" self._setup_agent(agent) agent.base_url = "http://127.0.0.1:1234/v1" empty_resp = _mock_response(content=None, finish_reason="stop") @@ -3600,10 +3601,17 @@ class TestRunConversation: patch.object(agent, "_persist_session"), patch.object(agent, "_save_trajectory"), patch.object(agent, "_cleanup_task_resources"), + patch.object( + agent, "_turn_completion_explainer_enabled", return_value=False + ), + caplog.at_level(logging.INFO, logger="agent.conversation_loop"), ): result = agent.run_conversation("answer me") assert result["completed"] is True - assert result["api_calls"] == 4 # unchanged: 1 original + 3 retries + assert result["api_calls"] == 2 + assert agent.session_api_calls == 2 + assert result["final_response"].startswith("⚠️ No reply:") + assert caplog.text.count("usage=unavailable") == 2 def test_truly_empty_response_succeeds_on_nudge(self, agent): """Model produces content after being nudged for empty response.""" From c3e9defd18388da39365af8603d9a6f81086b54c Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Thu, 3 Sep 2026 02:41:52 -0700 Subject: [PATCH 2/4] fix(agent): trim usage-less empty fix to guard + observability Drop the loop-side '(empty)' rewrite (the turn-completion explainer already owns that at delivery, and gateway/desktop match on the sentinel) and the extra token-count persistence. Keeps: usage-absent empty streaks arm the deterministic fast-fail after two attempts with no content or reasoning, and every completed API call logs even when the provider omits usage (#101898). --- agent/conversation_loop.py | 7 +------ .../test_empty_terminal_reasoning_surface.py | 14 ++++++++------ tests/run_agent/test_run_agent.py | 4 ---- 3 files changed, 9 insertions(+), 16 deletions(-) diff --git a/agent/conversation_loop.py b/agent/conversation_loop.py index b6978abbc8..bc01fc2dea 100644 --- a/agent/conversation_loop.py +++ b/agent/conversation_loop.py @@ -8761,12 +8761,7 @@ def run_conversation( "answer:\n\n" + reasoning_preview ) else: - final_response = agent._format_turn_completion_explanation( - _turn_exit_reason - ) or ( - "⚠️ No reply: the model returned empty content after " - "all retries and fallback attempts." - ) + final_response = "(empty)" break # Reset retry counter/signature on successful content diff --git a/tests/run_agent/test_empty_terminal_reasoning_surface.py b/tests/run_agent/test_empty_terminal_reasoning_surface.py index 3b56144cfe..7cfd1aea02 100644 --- a/tests/run_agent/test_empty_terminal_reasoning_surface.py +++ b/tests/run_agent/test_empty_terminal_reasoning_surface.py @@ -11,7 +11,7 @@ Invariants pinned here: ``_empty_terminal_sentinel`` marker (replay semantics unchanged). - Raw reasoning is NEVER promoted earlier in the ladder — a reasoning-only response still goes through prefill continuation first. -- A truly empty exhaustion returns an actionable no-reply explanation. +- A truly empty exhaustion (no reasoning either) still returns "(empty)". """ from __future__ import annotations @@ -109,11 +109,11 @@ def test_exhausted_reasoning_only_delivers_labeled_excerpt(tmp_path, monkeypatch ) -def test_exhausted_truly_empty_delivers_no_reply_explanation(tmp_path, monkeypatch): - """No reasoning anywhere produces an honest terminal explanation even - when the downstream completion explainer is disabled.""" +def test_exhausted_truly_empty_keeps_existing_behavior(tmp_path, monkeypatch): + """No reasoning anywhere → behavior unchanged from main: the '(empty)' + terminal (possibly rewritten by the downstream turn-completion explainer) + is delivered, and no reasoning excerpt appears.""" agent = _build_agent(tmp_path, monkeypatch) - monkeypatch.setattr(agent, "_turn_completion_explainer_enabled", lambda: False) monkeypatch.setattr( agent, "_interruptible_api_call", lambda api_kwargs: _truly_empty_response(), @@ -122,7 +122,9 @@ def test_exhausted_truly_empty_delivers_no_reply_explanation(tmp_path, monkeypat result = agent.run_conversation("hello?") final = result["final_response"] - assert final.startswith("⚠️ No reply:") + # Either the raw sentinel (explainer off) or the explainer's rewrite — + # never the reasoning-excerpt frame, which requires reasoning to exist. + assert final == "(empty)" or final.startswith("⚠️ No reply:") assert "only internal reasoning" not in final diff --git a/tests/run_agent/test_run_agent.py b/tests/run_agent/test_run_agent.py index 3c03e5cde1..77128fd092 100644 --- a/tests/run_agent/test_run_agent.py +++ b/tests/run_agent/test_run_agent.py @@ -3601,16 +3601,12 @@ class TestRunConversation: patch.object(agent, "_persist_session"), patch.object(agent, "_save_trajectory"), patch.object(agent, "_cleanup_task_resources"), - patch.object( - agent, "_turn_completion_explainer_enabled", return_value=False - ), caplog.at_level(logging.INFO, logger="agent.conversation_loop"), ): result = agent.run_conversation("answer me") assert result["completed"] is True assert result["api_calls"] == 2 assert agent.session_api_calls == 2 - assert result["final_response"].startswith("⚠️ No reply:") assert caplog.text.count("usage=unavailable") == 2 def test_truly_empty_response_succeeds_on_nudge(self, agent): From 7b72fd12476aedc06a993d92c4337e2ceb214bc7 Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Thu, 3 Sep 2026 02:44:08 -0700 Subject: [PATCH 3/4] fix(agent): stream cut mid tool-call markup no longer persists as assistant prose (#101899) GLM-style models serialize tool calls as XML in the text channel; when the stream drops mid-serialization with finish_reason=stop, the orphan / fragment (or a bare unclosed opener) matched neither the complete-block stripper nor the partial-stream guard and was stored and displayed as ordinary assistant content. strip_think_blocks (storage boundary) and the CLI display copy now strip an unterminated block-boundary tool-call opener, or any line carrying stray argument markup, to end of text. The response then reads as empty and flows through the existing empty-retry path. Complete blocks and inline prose mentions are unchanged. --- agent/agent_runtime_helpers.py | 14 ++++++++++ cli.py | 9 +++++++ .../test_strip_reasoning_tags_cli.py | 26 +++++++++++++++++++ 3 files changed, 49 insertions(+) diff --git a/agent/agent_runtime_helpers.py b/agent/agent_runtime_helpers.py index 4927938655..1880698e81 100644 --- a/agent/agent_runtime_helpers.py +++ b/agent/agent_runtime_helpers.py @@ -100,6 +100,17 @@ _STRAY_TOOL_CALL_CLOSER_PATTERN = re.compile( re.IGNORECASE, ) +# A tool-call opener with no closer, or GLM-style argument markup +# (/) outside any closed block, means the stream was +# cut mid-serialization of a text-channel tool call (#101899). The call +# can't be recovered; strip from the block-boundary opener (or the line +# holding the first stray argument tag) to the end of the text. +_UNTERMINATED_TOOL_CALL_PATTERN = re.compile( + rf'(?:^|\n)[ \t]*<(?:{"|".join(_TOOL_CALL_TAG_NAMES)})\b[^>]*>.*$' + r'|(?:^|\n)[^\n<]* str: # during streaming may still be valuable to the user; matches # OpenClaw's intentional asymmetry.) content = _STRAY_TOOL_CALL_CLOSER_PATTERN.sub('', content) + # 3c. Tool-call openers or argument markup surviving 1b belong to a + # block that never closed — a mid-serialization stream cut (#101899). + content = _UNTERMINATED_TOOL_CALL_PATTERN.sub('', content) return content diff --git a/cli.py b/cli.py index 20c160931f..19536ffe8d 100644 --- a/cli.py +++ b/cli.py @@ -314,6 +314,15 @@ def _strip_reasoning_tags(text: str) -> str: cleaned, flags=re.IGNORECASE, ) + # Unterminated opener / stray / markup = stream cut + # mid tool-call serialization (#101899); strip to end of text. + cleaned = re.sub( + r'(?:^|\n)[ \t]*<(?:tool_call|tool_calls|tool_result|function_call|function_calls)\b[^>]*>.*$' + r'|(?:^|\n)[^\n<]*\nsession_id\nabc\n" + "timeout\n59" +) +_COMPLETE_WITH_PROSE = ( + "Use in JS. The arg_key field maps to arg_value.\n" + "xa1\nDone." +) + class TestToolCallStripping: def test_tool_call_block_stripped(self): @@ -26,3 +39,16 @@ class TestToolCallStripping: def test_empty_string(self): assert _strip_reasoning_tags("") == "" + def test_cut_tool_call_stripped_to_visible_prefix(self): + """Both strippers drop the unrecoverable tail; only prose survives.""" + assert _strip_reasoning_tags(_CUT_FRAGMENT) == "Both gates started." + assert strip_think_blocks(None, _CUT_FRAGMENT).strip() == "Both gates started." + assert strip_think_blocks(None, "Waiting.\nprocess_manage").strip() == "Waiting." + + def test_complete_block_and_inline_prose_mentions_untouched(self): + for out in (_strip_reasoning_tags(_COMPLETE_WITH_PROSE), + strip_think_blocks(None, _COMPLETE_WITH_PROSE)): + assert "Use in JS. The arg_key field maps to arg_value." in out + assert out.rstrip().endswith("Done.") + assert "" not in out + From 562ee8ab76a703b7f524172cae0b1d52f9f94bd3 Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Thu, 3 Sep 2026 03:55:45 -0700 Subject: [PATCH 4/4] fix(prompt): agent routes task-learned knowledge (incl. user preferences/corrections) to skills; memory is the every-session exception The memory guidance led with 'Save proactively' and the memory tool schema ranked 'user preferences & corrections' as top priority, while the skills nudge was a conditional 'offer to save'. In practice that asymmetry made the agent end sessions writing memory entries (fighting a 2,200-char budget) and skip updating the skill it had just used, even though the procedure was the reusable artifact. Both surfaces now state the same rule with skills first: what you learn doing a task, including the user's preferences and corrections for that kind of work, goes in the task's skill; memory is only for facts that apply to every session. --- agent/prompt_builder.py | 14 ++++++++++---- tests/agent/test_prompt_builder.py | 7 ++++++- tools/memory_tool.py | 10 ++++++---- 3 files changed, 22 insertions(+), 9 deletions(-) diff --git a/agent/prompt_builder.py b/agent/prompt_builder.py index f6e53b5b18..f1c5ab0890 100644 --- a/agent/prompt_builder.py +++ b/agent/prompt_builder.py @@ -281,14 +281,20 @@ def build_memory_guidance(memory_enabled: bool = True, profile_enabled: bool = T "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 " + "Skills come first: when you learn something while doing a task — a " + "procedure, a pitfall, and the user's preferences and corrections " + "for that kind of work — record it in the skill you used or built " + "for the task (skill_manage), where it loads only when relevant. " + "Memory is the narrow exception for facts that apply to EVERY " + "session regardless of task (who the user is, environment facts, " + "standing conventions with no task home); it has a hard character " + "budget, so when it fills, replace or consolidate stale entries " "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." + "request). A fact stale within a week belongs in session history; " + "procedures and workflows belong in skills." ) diff --git a/tests/agent/test_prompt_builder.py b/tests/agent/test_prompt_builder.py index 6b4190f841..4331c2bc9d 100644 --- a/tests/agent/test_prompt_builder.py +++ b/tests/agent/test_prompt_builder.py @@ -70,7 +70,12 @@ class TestGuidanceConstants: 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 + # Skills are the default home for task-learned knowledge (incl. the + # user's preferences/corrections for that work); memory is the narrow + # every-session exception. The routing rule must LEAD, not trail. + assert MEMORY_GUIDANCE.index("Skills come first") < MEMORY_GUIDANCE.index("Memory is the narrow exception") + assert "preferences and corrections" in MEMORY_GUIDANCE + assert "Save proactively" not in MEMORY_GUIDANCE assert "workflows belong" in MEMORY_GUIDANCE # The category/SKIP curricula must NOT be re-taught here. assert "PR numbers" not in MEMORY_GUIDANCE diff --git a/tools/memory_tool.py b/tools/memory_tool.py index 54dcaa71a6..eccbe82674 100644 --- a/tools/memory_tool.py +++ b/tools/memory_tool.py @@ -1271,10 +1271,12 @@ MEMORY_SCHEMA = { "reports current/limit chars and confirms completion; one batch call finishes the " "update, so don't repeat it. Use the bare action/content/old_text fields only for a " "single lone change.\n\n" - "WHEN: save proactively when the user states a preference, correction, or personal " - "detail, or you learn a stable fact about their environment, conventions, or workflow. " - "Priority: user preferences & corrections > environment facts > procedures. The best " - "memory stops the user repeating themselves.\n\n" + "WHEN: only for facts that apply to EVERY session regardless of task: who the user " + "is, stable environment facts, standing conventions with no task home. Anything " + "learned while doing a task (procedures, pitfalls, and the user's preferences and " + "corrections for that kind of work) belongs in the task's skill via skill_manage, " + "where it loads only when relevant; memory is injected into every turn and must " + "stay small.\n\n" "IF FULL: an add is rejected with the current entries shown. Reissue as ONE batch that " "removes or shortens enough stale entries and adds the new one together.\n\n" "TARGETS: 'user' = who the user is (name, role, preferences, style). 'memory' = your "