diff --git a/agent/agent_runtime_helpers.py b/agent/agent_runtime_helpers.py index 3101db3e43..5f3d0ede46 100644 --- a/agent/agent_runtime_helpers.py +++ b/agent/agent_runtime_helpers.py @@ -58,6 +58,17 @@ _STRAY_TOOL_CALL_CLOSER_PATTERN = re.compile( rf'\s*', 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: _THINK_STRIP_PATTERNS = ( *_REASONING_BLOCK_PATTERNS, *_TOOL_CALL_BLOCK_PATTERNS, _NAMED_FUNCTION_BLOCK_PATTERN, _UNTERMINATED_REASONING_BLOCK_PATTERN, _ORPHAN_REASONING_TAG_PATTERN, - _STRAY_TOOL_CALL_CLOSER_PATTERN, + _STRAY_TOOL_CALL_CLOSER_PATTERN, _UNTERMINATED_TOOL_CALL_PATTERN, ) diff --git a/agent/empty_response_guard.py b/agent/empty_response_guard.py index 7e7e66ab5e..1d8a563db0 100644 --- a/agent/empty_response_guard.py +++ b/agent/empty_response_guard.py @@ -15,8 +15,31 @@ Two independent guards, both failing OPEN to legacy behaviour: (default $0.25), the retry budget drops from 3 to 1. Unknown pricing / missing usage / included routes leave it untouched. -Config ``agent.empty_response_guard.{enabled, cost_threshold_usd}`` is resolved once by -``agent_init`` and stashed on the agent so the hot loop never re-reads config (no env vars). +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 + $0.25), the empty-retry budget for this streak drops from 3 to 1. + Unknown pricing, missing usage, or included/subscription routes + leave the budget untouched. + +Configured via the additive ``agent.empty_response_guard`` section in +``config.yaml`` (resolved once at agent init by ``agent_init``):: + + agent: + empty_response_guard: + enabled: true # false = legacy fixed 3-retry behaviour + cost_threshold_usd: 0.25 # per-attempt cost that halves the budget + +Per project policy, no ``HERMES_*`` environment variables are involved — +``.env`` is reserved for credentials; behavioural settings live in +``config.yaml``. """ from __future__ import annotations @@ -51,6 +74,7 @@ class EmptyAttempt: finish_reason: str usage_present: bool zero_output: bool + observed_generation: bool @property def signature(self) -> tuple: @@ -146,7 +170,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. Call BEFORE ``_empty_content_retries`` is incremented: a counter of 0 marks a new @@ -157,10 +187,16 @@ def record_empty_attempt(agent: Any, *, finish_reason: str, response: Any) -> No setattr(agent, _STREAK_COST_ATTR, Decimal("0")) usage_present, zero_output = _zero_output(agent, response) - attempts.append(EmptyAttempt( - model=str(getattr(agent, "model", "") or ""), provider=str(getattr(agent, "provider", "") or ""), - finish_reason=str(finish_reason or ""), usage_present=usage_present, zero_output=zero_output, - )) + attempts.append( + EmptyAttempt( + model=str(getattr(agent, "model", "") or ""), + provider=str(getattr(agent, "provider", "") or ""), + finish_reason=str(finish_reason or ""), + usage_present=usage_present, + zero_output=zero_output, + observed_generation=bool(observed_generation), + ) + ) cost = _estimate_attempt_cost(agent, response) if cost is not None and cost > 0: @@ -169,14 +205,25 @@ def record_empty_attempt(agent: Any, *, finish_reason: str, response: Any) -> No def deterministic_empty(agent: Any) -> bool: - """True when >= 2 consecutive attempts ALL have usage present, zero output and an - identical signature. Any missing-usage / non-zero attempt → False (fail open).""" + """True when the current streak looks deterministic. + + 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 attempts = getattr(agent, _ATTEMPTS_ATTR, None) or [] - return len(attempts) >= 2 and all( - a.usage_present and a.zero_output and a.signature == attempts[0].signature for a in attempts + if len(attempts) < 2: + return False + first = attempts[0] + 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/agent/prompt_builder.py b/agent/prompt_builder.py index e5fb6bbc99..6fc87fee80 100644 --- a/agent/prompt_builder.py +++ b/agent/prompt_builder.py @@ -169,11 +169,20 @@ def build_memory_guidance(memory_enabled: bool = True, profile_enabled: bool = T "memory tool (target='user') — the built-in notes store is disabled, so never target='memory'. " ) return frame + ( - "Save proactively — storage has a hard character budget, and when it fills, replace or consolidate stale " - "entries in the same batch rather than skipping the save. Write entries as declarative facts, not instructions " - "to yourself: 'User prefers concise responses' ✓ — 'Always respond concisely' ✗ (imperative phrasing gets " - "re-read as a directive in later sessions and can override the user's current request). Route by longevity: a " - "fact stale within a week belongs in session history; procedures and workflows belong in skills." + "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). A fact stale within a week belongs in session history; " + "procedures and workflows belong in skills." ) diff --git a/agent/turn_empty_response.py b/agent/turn_empty_response.py index 537b0735d6..60c8ee91de 100644 --- a/agent/turn_empty_response.py +++ b/agent/turn_empty_response.py @@ -43,7 +43,7 @@ class EmptyResponseVerdict: def _retry_empty( agent: Any, response: Any, finish_reason: str, empty_candidate: bool, *, messages: Any, - conversation_history: Any, api_call_count: int, + conversation_history: Any, api_call_count: int, observed_generation: bool = False, ) -> tuple: """Budgeted empty-response retry. Each empty attempt re-bills the full input, so the signature is recorded and deterministic empties stop burning paid retries (fails @@ -52,7 +52,9 @@ def _retry_empty( from agent.conversation_loop import jittered_backoff if empty_candidate: - _empty_guard.record_empty_attempt(agent, finish_reason=finish_reason, response=response) + _empty_guard.record_empty_attempt( + agent, finish_reason=finish_reason, response=response, observed_generation=observed_generation, + ) budget = ( _empty_guard.empty_retry_budget(agent, response) if empty_candidate else _empty_guard.DEFAULT_EMPTY_RETRY_BUDGET @@ -246,18 +248,19 @@ def recover_empty_response( action, interrupt_result, _deterministic_empty = _retry_empty( agent, response, finish_reason, _empty_candidate, messages=messages, conversation_history=conversation_history, api_call_count=api_call_count, + observed_generation=_has_structured, ) if action is not None: return _verdict(action, interrupt_result) if _truly_empty and _deterministic_empty: logger.warning( - "Deterministic empty response detected (consecutive zero-output completions, " - "model=%s provider=%s finish_reason=%s) — skipping remaining retries", + "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" ) diff --git a/agent/turn_usage.py b/agent/turn_usage.py index 87343441d1..2fd8f264c7 100644 --- a/agent/turn_usage.py +++ b/agent/turn_usage.py @@ -72,12 +72,20 @@ def record_response_usage( consume a pending compaction verdict. Returns the loop-visible outcome.""" rearmed = False compressor = agent.context_compressor + # Count every completed provider attempt, including providers that omit usage. + # Token/cost accounting below stays gated on real usage, but the request itself + # must remain observable. + agent.session_api_calls += 1 if not (hasattr(response, 'usage') and response.usage): if getattr(compressor, "awaiting_real_usage_after_compression", False): # No usage -> cannot adjudicate the prior compaction; consume the # pending verdict so later readings aren't charged to it and # preflight deferral isn't latched indefinitely. compressor.update_from_response({}) + 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, + ) return ResponseUsageOutcome(compression_attempts=compression_attempts, rearmed=rearmed) canonical_usage = normalize_usage(response.usage, provider=agent.provider, api_mode=agent.api_mode) @@ -149,7 +157,6 @@ def record_response_usage( 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 diff --git a/cli.py b/cli.py index b8e760e8a1..533d69564c 100644 --- a/cli.py +++ b/cli.py @@ -205,6 +205,15 @@ def _strip_reasoning_tags(text: str) -> str: r'\s*', '', 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<]* 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/agent/test_prompt_builder.py b/tests/agent/test_prompt_builder.py index 910260fff5..6e4fefb359 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/tests/run_agent/test_run_agent.py b/tests/run_agent/test_run_agent.py index f4ed1db3c0..b7610da930 100644 --- a/tests/run_agent/test_run_agent.py +++ b/tests/run_agent/test_run_agent.py @@ -3503,12 +3503,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, ] @@ -3522,7 +3522,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 @@ -3578,10 +3578,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") @@ -3590,10 +3591,13 @@ class TestRunConversation: patch.object(agent, "_persist_session"), patch.object(agent, "_save_trajectory"), patch.object(agent, "_cleanup_task_resources"), + 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 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.""" diff --git a/tests/run_agent/test_strip_reasoning_tags_cli.py b/tests/run_agent/test_strip_reasoning_tags_cli.py index 60525990c5..7d32c538cd 100644 --- a/tests/run_agent/test_strip_reasoning_tags_cli.py +++ b/tests/run_agent/test_strip_reasoning_tags_cli.py @@ -7,8 +7,21 @@ AIAgent instance. It must stay in sync with run_agent.py::_strip_think_blocks for tool-call tag coverage.""" +from agent.agent_runtime_helpers import strip_think_blocks from cli import _strip_reasoning_tags +# GLM text-channel tool call cut mid-serialization by a stream drop (#101899): +# the first key and call name never arrived, only orphan argument markup. +_CUT_FRAGMENT = ( + "Both gates started.\n" + "wait\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 + diff --git a/tools/memory_tool.py b/tools/memory_tool.py index c8d42cb49a..57f97fa81a 100644 --- a/tools/memory_tool.py +++ b/tools/memory_tool.py @@ -240,10 +240,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 "