From bd3d16a4906ae2a749eff7aa6947667d6bd4259a Mon Sep 17 00:00:00 2001 From: kshitijk4poor <82637225+kshitijk4poor@users.noreply.github.com> Date: Sat, 18 Jul 2026 19:32:47 +0530 Subject: [PATCH] =?UTF-8?q?fix:=20salvage=20follow-up=20=E2=80=94=20remove?= =?UTF-8?q?=20redundant=20coercion,=20fix=20classifier,=20restore=20None?= =?UTF-8?q?=20guard?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 1. Remove PR's redundant strip_think_blocks coercion (Teknium's fix 296494db0 already handles this at the same chokepoint with superior logic that drops thinking/reasoning blocks). 2. Restore 'if not content: return ' guard at top of strip_think_blocks that was lost during cherry-pick auto-merge. Without it, None content hits str(None) → 'None' string instead of returning empty. 3. Fix error classifier design flaw: remove 'conversation_loop' and 'run_agent' from _local_processing_modules — these are the container modules for the try/except, so every exception passes through them, making _hit_local always True and misclassifying transient API/network errors as non-retryable local bugs. 4. Move module sets to module-level frozenset constants (_LOCAL_PROCESSING_MODULES, _API_CALL_MODULES) instead of rebuilding on every exception. 5. Replace traceback.extract_tb() with raw tb walk — avoids disk I/O for source lines that are never used. 6. Remove unused 'import traceback'. 7. Fix docstring corruption: 3 lines where think tags were replaced with Chinese characters during the PR's editing. --- agent/agent_runtime_helpers.py | 21 +++------------- agent/conversation_loop.py | 44 +++++++++++++++++++++------------- 2 files changed, 30 insertions(+), 35 deletions(-) diff --git a/agent/agent_runtime_helpers.py b/agent/agent_runtime_helpers.py index db0c48275cc8..36712ec48281 100644 --- a/agent/agent_runtime_helpers.py +++ b/agent/agent_runtime_helpers.py @@ -643,16 +643,16 @@ def strip_think_blocks(agent, content: str) -> str: """Remove reasoning/thinking blocks from content, returning only visible text. Handles four cases: - 1. Closed tag pairs (`` 执教… ``) — the common path when + 1. Closed tag pairs (`` … ``) — the common path when the provider emits complete reasoning blocks. 2. Unterminated open tag at a block boundary (start of text or after a newline) — e.g. MiniMax M2.7 / NIM endpoints where the closing tag is dropped. Everything from the open tag to end of string is stripped. The block-boundary check mirrors ``gateway/stream_consumer.py``'s filter so models that mention - `` 执教`` in prose aren't over-stripped. + `` `` in prose aren't over-stripped. 3. Stray orphan open/close tags that slip through. - 4. Tag variants: `` 执教``, ````, ````, + 4. Tag variants: `` ``, ````, ````, ````, ```` (Gemma 4), all case-insensitive. @@ -670,21 +670,6 @@ def strip_think_blocks(agent, content: str) -> str: after punctuation and carries a ``name="..."`` attribute) so prose mentions like "Use in JavaScript" are preserved. """ - # Defensive coercion: callers may pass multimodal content-parts lists - # (e.g. after vision turns or context compaction). Extract any text - # blocks and drop non-text parts before running regexes. - if isinstance(content, list): - parts = [] - for part in content: - if isinstance(part, str): - parts.append(part) - elif isinstance(part, dict) and part.get("type") == "text": - text = part.get("text") - if isinstance(text, str): - parts.append(text) - content = "\n".join(parts) - elif not isinstance(content, str): - content = str(content) if content is not None else "" if not content: return "" # Coerce non-string content to text before any regex runs. Providers diff --git a/agent/conversation_loop.py b/agent/conversation_loop.py index ffe114e15842..7a7e6bf33547 100644 --- a/agent/conversation_loop.py +++ b/agent/conversation_loop.py @@ -24,7 +24,6 @@ import re import ssl import threading import time -import traceback import uuid from typing import Any, Dict, List, Optional @@ -79,6 +78,25 @@ logger = logging.getLogger(__name__) # to treat it as cancellation metadata rather than assistant prose. INTERRUPT_WAITING_FOR_MODEL_PREFIX = "Operation interrupted: waiting for model response (" +# Modules that indicate a deterministic local processing error when they +# appear in an exception traceback WITHOUT any API-call module. Used by the +# outer-loop error classifier to avoid retrying bugs that will fail +# identically every time (e.g. TypeError from passing list content into a +# regex helper). IMPORTANT: do NOT include "conversation_loop" or +# "run_agent" here — those are the container modules for the try/except +# itself, so every exception passes through them, which would make +# _hit_local always True and misclassify transient API/network errors as +# non-retryable local bugs. (#66267) +_LOCAL_PROCESSING_MODULES = frozenset({ + "agent_runtime_helpers", + "message_content", + "message_sanitization", + "chat_completion_helpers", # only local when NOT also an API-call module +}) +_API_CALL_MODULES = frozenset({ + "chat_completion_helpers", +}) + def _image_error_max_dimension(error: Exception) -> Optional[int]: """Extract a provider-reported image dimension ceiling, if present.""" @@ -5609,23 +5627,15 @@ def run_conversation( # local post-processing helpers and never entered the interruptible # API-call helpers, it is almost certainly a local processing bug. # (#66267) - _local_processing_modules = { - "agent_runtime_helpers", - "conversation_loop", - "message_content", - "run_agent", - } - _api_call_modules = { - "chat_completion_helpers", - } + tb_module_names: set[str] = set() + _tb = e.__traceback__ + while _tb is not None: + _fname = os.path.splitext(os.path.basename(_tb.tb_frame.f_code.co_filename))[0] + tb_module_names.add(_fname) + _tb = _tb.tb_next - tb_stack = traceback.extract_tb(e.__traceback__) - tb_module_names = { - os.path.splitext(os.path.basename(frame.filename))[0] - for frame in tb_stack - } - _hit_local = bool(tb_module_names & _local_processing_modules) - _hit_api = bool(tb_module_names & _api_call_modules) + _hit_local = bool(tb_module_names & _LOCAL_PROCESSING_MODULES) + _hit_api = bool(tb_module_names & _API_CALL_MODULES) _is_local_processing_error = _hit_local and not _hit_api