From 3b17b4147dc11e4065c003bc5dab81786559b3fe Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Wed, 2 Sep 2026 18:15:22 -0700 Subject: [PATCH 01/13] refactor(agent/bedrock_adapter): unify response accumulator, redacted decode, tool-arg parsing; compact docs (-144 LOC) --- agent/bedrock_adapter.py | 734 ++++++++++++++++----------------------- 1 file changed, 295 insertions(+), 439 deletions(-) diff --git a/agent/bedrock_adapter.py b/agent/bedrock_adapter.py index b62b231ffb..aab7aafd11 100644 --- a/agent/bedrock_adapter.py +++ b/agent/bedrock_adapter.py @@ -1,14 +1,11 @@ """AWS Bedrock Converse API adapter for Hermes Agent. -Talks to Bedrock through the native Converse API (boto3) instead of an -OpenAI-compatible endpoint, so the AWS credential chain (IAM roles, SSO -profiles, env vars, instance metadata), cross-region inference profiles, -guardrails and control-plane model discovery all work without API keys. - -Same layout as ``anthropic_adapter.py``: all Bedrock-specific logic lives -here; OpenAI-format messages/tools are converted to Converse format on the -way in and responses are normalized back to OpenAI-shaped objects for the -agent loop. Requires ``boto3`` (optional dependency). +Talks to Bedrock through the native Converse API (boto3) so the AWS credential +chain (IAM roles, SSO profiles, env vars, instance metadata), cross-region +inference profiles, guardrails and control-plane model discovery all work +without API keys. OpenAI-format messages/tools are converted to Converse format +on the way in and responses normalized back to OpenAI-shaped objects. +Requires ``boto3`` (optional dependency). """ import base64 @@ -24,22 +21,20 @@ import httpx logger = logging.getLogger(__name__) -# boto3 is not in the [all] extras; lazy_deps installs it on demand so the -# provider works without baking boto3 into the base image. +# boto3 is not in the [all] extras; lazy_deps installs it on demand. try: from tools.lazy_deps import ensure ensure("provider.bedrock", prompt=False) except Exception: - pass # lazy_deps unavailable or install failed — let downstream imports surface the real error + pass # let downstream imports surface the real error _bedrock_runtime_client_cache: Dict[str, Any] = {} _bedrock_control_client_cache: Dict[str, Any] = {} -# Bedrock-hosted OpenAI GPT-5.x models are not exposed through the Converse -# runtime; AWS serves them from the Bedrock Mantle OpenAI-compatible Responses -# endpoint (https://bedrock-mantle..api.aws/openai/v1). Keep the -# allowlist narrow so Converse-capable GPT-OSS models stay on the native path. +# Bedrock-hosted OpenAI GPT-5.x models are served from the Bedrock Mantle +# OpenAI-compatible Responses endpoint, not Converse. Keep the allowlist narrow +# so Converse-capable GPT-OSS models stay on the native path. BEDROCK_OPENAI_RESPONSES_MODEL_IDS: Tuple[str, ...] = ( "openai.gpt-5.5", "openai.gpt-5.6-sol", @@ -47,17 +42,11 @@ BEDROCK_OPENAI_RESPONSES_MODEL_IDS: Tuple[str, ...] = ( "openai.gpt-5.6-luna", ) _BEDROCK_OPENAI_HOST_RE = re.compile(r"^bedrock-mantle\.([a-z0-9-]+)\.api\.aws$", re.IGNORECASE) - - _MIN_BOTO3_VERSION = (1, 34, 59) def _require_boto3(): - """Import boto3, raising a clear error if not installed or too old. - - converse()/converse_stream() need boto3 >= 1.34.59; a system boto3 (e.g. - Ubuntu's 1.34.46) can shadow the venv pin under editable installs. - """ + """Import boto3; converse_stream() needs >= 1.34.59 (a system boto3 can shadow the venv pin).""" try: import boto3 except ImportError: @@ -101,10 +90,7 @@ def reset_client_cache(): def invalidate_runtime_client(region: str) -> bool: - """Evict one region's cached ``bedrock-runtime`` client (stale HTTP pool). - - Returns True if an entry was evicted. - """ + """Evict one region's cached ``bedrock-runtime`` client (stale HTTP pool); True if evicted.""" existed = region in _bedrock_runtime_client_cache _bedrock_runtime_client_cache.pop(region, None) return existed @@ -115,8 +101,7 @@ def invalidate_runtime_client(region: str) -> bool: def is_openai_bedrock_model(model_id: str) -> bool: """True for Bedrock-hosted OpenAI models that require Mantle (GPT-OSS excluded).""" - normalized = str(model_id or "").strip().lower() - return normalized in {m.lower() for m in BEDROCK_OPENAI_RESPONSES_MODEL_IDS} + return str(model_id or "").strip().lower() in {m.lower() for m in BEDROCK_OPENAI_RESPONSES_MODEL_IDS} def merge_bedrock_openai_model_ids(model_ids: List[str]) -> List[str]: @@ -138,8 +123,7 @@ def bedrock_openai_base_url(region: str) -> str: def bedrock_openai_region_from_base_url(base_url: str) -> Optional[str]: """Extract the AWS region from a Bedrock Mantle OpenAI base URL.""" - host = urlparse(str(base_url or "")).hostname or "" - match = _BEDROCK_OPENAI_HOST_RE.match(host) + match = _BEDROCK_OPENAI_HOST_RE.match(urlparse(str(base_url or "")).hostname or "") return match.group(1) if match else None @@ -148,8 +132,7 @@ def is_bedrock_openai_base_url(base_url: str) -> bool: parsed = urlparse(str(base_url or "")) if not _BEDROCK_OPENAI_HOST_RE.match(parsed.hostname or ""): return False - path = (parsed.path or "").rstrip("/").lower() - return path in {"", "/openai", "/openai/v1"} + return (parsed.path or "").rstrip("/").lower() in {"", "/openai", "/openai/v1"} def resolve_bedrock_bearer_token(env: Optional[Dict[str, str]] = None) -> str: @@ -179,19 +162,13 @@ class BedrockOpenAISigV4Auth(httpx.Auth): "Configure AWS_ACCESS_KEY_ID/AWS_SECRET_ACCESS_KEY, AWS_PROFILE, " "SSO, or an instance/task role." ) - # SigV4 must own Authorization: drop the SDK's placeholder bearer header - # but keep every other SDK header (content-type, accept, request IDs). + # SigV4 must own Authorization: drop the SDK's placeholder bearer header, keep the rest. headers = { str(k): str(v) for k, v in request.headers.items() if str(k).lower() not in {"authorization", "x-amz-date", "x-amz-security-token"} } - aws_request = AWSRequest( - method=request.method, - url=str(request.url), - data=request.content or b"", - headers=headers, - ) + aws_request = AWSRequest(method=request.method, url=str(request.url), data=request.content or b"", headers=headers) SigV4Auth(credentials.get_frozen_credentials(), self.service, self.region).add_auth(aws_request) request.headers.update(dict(aws_request.headers.items())) yield request @@ -210,9 +187,8 @@ def configure_bedrock_openai_client_kwargs( ) -> Dict[str, Any]: """Install SigV4 auth on OpenAI SDK kwargs for Bedrock Mantle. - Real API keys (``AWS_BEARER_TOKEN_BEDROCK``) keep the SDK's bearer auth; - the ``aws-sdk``/``no-key-required`` placeholders mean IAM credential-chain - auth, so we attach a per-request SigV4 httpx client. + Real API keys (``AWS_BEARER_TOKEN_BEDROCK``) keep the SDK's bearer auth; the + ``aws-sdk``/``no-key-required`` placeholders mean IAM credential-chain auth. """ base_url = str(client_kwargs.get("base_url") or "") if not is_bedrock_openai_base_url(base_url): @@ -227,23 +203,15 @@ def configure_bedrock_openai_client_kwargs( # --- Stale-connection detection --- -# boto3 caches its HTTPS pool inside the client. A pooled connection killed -# out from under us (NAT timeout, VPN flap, RST, proxy idle cull) surfaces as -# botocore ConnectionClosedError / urllib3 ProtocolError, or as a bare -# AssertionError from urllib3's internal pool-state checks. Retrying with the -# same client reproduces the failure, so the fix is to evict the client. +# boto3 caches its HTTPS pool inside the client. A pooled connection killed out +# from under us (NAT timeout, VPN flap, RST) surfaces as botocore +# ConnectionClosedError / urllib3 ProtocolError, or as a bare AssertionError from +# urllib3's pool-state checks. Retrying with the same client reproduces the +# failure, so the fix is to evict the client. _STALE_LIB_MODULE_PREFIXES = ("urllib3.", "botocore.", "boto3.") -def _traceback_frames_modules(exc: BaseException): - """Yield the ``__name__`` of each frame in exc's traceback.""" - tb = getattr(exc, "__traceback__", None) - while tb is not None: - yield tb.tb_frame.f_globals.get("__name__", "") or "" - tb = tb.tb_next - - def _stale_error_types() -> tuple: """botocore + urllib3 transport-failure exception classes (best-effort import).""" types: list = [] @@ -261,18 +229,16 @@ def _stale_error_types() -> tuple: def is_stale_connection_error(exc: BaseException) -> bool: - """True if ``exc`` indicates a dead/stale Bedrock HTTP connection. - - Matches botocore ConnectionError/HTTPClientError subclasses, urllib3 - transport errors, and bare ``AssertionError``s raised from inside - urllib3/botocore/boto3. Application-level AssertionErrors are NOT matched. - """ + """True for botocore/urllib3 transport errors or AssertionErrors raised inside those libs.""" if isinstance(exc, _stale_error_types()): return True - if isinstance(exc, AssertionError): - for module in _traceback_frames_modules(exc): - if module.startswith(_STALE_LIB_MODULE_PREFIXES): - return True + if not isinstance(exc, AssertionError): + return False + tb = exc.__traceback__ + while tb is not None: + if (tb.tb_frame.f_globals.get("__name__", "") or "").startswith(_STALE_LIB_MODULE_PREFIXES): + return True + tb = tb.tb_next return False @@ -280,9 +246,8 @@ def is_streaming_access_denied_error(exc: BaseException) -> bool: """True when IAM denied ``bedrock:InvokeModelWithResponseStream``. InvokeModel-only policies reject converse_stream() permanently, so callers - should fall back to non-streaming converse() instead of retrying. Detection - is message-based because the AnthropicBedrock SDK wraps the same AWS - response in its own exception types but preserves the action name. + should fall back to non-streaming converse(). Message-based because the + AnthropicBedrock SDK wraps the same AWS response but preserves the action name. """ msg = str(exc).lower() if "invokemodelwithresponsestream" not in msg: @@ -323,11 +288,7 @@ def _boto3_chain_has_credentials() -> bool: def resolve_aws_auth_env_var(env: Optional[Dict[str, str]] = None) -> Optional[str]: - """Return the name of the active AWS auth source, or None. - - Env vars first (fast, no I/O), then ``"iam-role"`` if boto3's chain - resolves implicit credentials (EC2 IMDS, ECS task role, Lambda). - """ + """Name of the active AWS auth source: env vars first (no I/O), then ``"iam-role"`` via boto3's chain, else None.""" env = env if env is not None else os.environ for group in _AWS_AUTH_ENV_CHAIN: if all(env.get(var, "").strip() for var in group): @@ -341,11 +302,7 @@ def has_aws_credentials(env: Optional[Dict[str, str]] = None) -> bool: def resolve_bedrock_region(env: Optional[Dict[str, str]] = None) -> str: - """AWS_REGION → AWS_DEFAULT_REGION → botocore configured region → us-east-1. - - The botocore fallback matters for EU/AP users who set the region in - ~/.aws/config via a named profile; without it discovery returns us.* IDs. - """ + """AWS_REGION → AWS_DEFAULT_REGION → botocore configured region (~/.aws/config profiles) → us-east-1.""" env = env if env is not None else os.environ explicit = env.get("AWS_REGION", "").strip() or env.get("AWS_DEFAULT_REGION", "").strip() if explicit: @@ -363,10 +320,9 @@ def resolve_bedrock_region(env: Optional[Dict[str, str]] = None) -> str: def resolve_bedrock_runtime_region(config: Optional[Dict[str, Any]] = None) -> str: """``bedrock.region`` from config.yaml, else :func:`resolve_bedrock_region`. - Matches ``hermes_cli/runtime_provider.py``. Every non-runtime Bedrock - endpoint (auxiliary clients, picker discovery) must use this, otherwise - auxiliary calls can leave the primary runtime's region when the config - and ambient AWS env/profile disagree. Pass *config* to avoid a disk read. + Every non-runtime Bedrock endpoint (auxiliary clients, picker discovery) must + use this so auxiliary calls never leave the primary runtime's region when + config and ambient AWS env/profile disagree. Pass *config* to avoid a disk read. """ if config is None: try: @@ -391,8 +347,8 @@ def bedrock_model_ids_or_none() -> Optional[List[str]]: # --- Tool-calling / prompt-cache capability detection --- -# Models known to reject toolConfig with a ValidationException. Conservative: -# unknown models are assumed to support tools; add here when one fails. +# Models known to reject toolConfig with a ValidationException. Unknown models +# are assumed to support tools. _NON_TOOL_CALLING_PATTERNS = [ "deepseek.r1", # DeepSeek R1 — reasoning only "deepseek-r1", # Alternate ID format @@ -401,6 +357,15 @@ _NON_TOOL_CALLING_PATTERNS = [ "amazon.titan-embed", # Embeddings ] +# cachePoint allowlist — inverted policy vs tools: an unsupported model rejects +# cachePoint with a ValidationException, so unknown models get NO cache markers. +# Claude normally uses the AnthropicBedrock SDK path and only reaches +# build_converse_kwargs under bearer-token auth. +_CACHE_POINT_PATTERNS = [ + "anthropic.claude", # bearer-token fallback path + "amazon.nova", +] + def _model_supports_tool_use(model_id: str) -> bool: """False for denylisted models; unknown models default to True.""" @@ -408,30 +373,17 @@ def _model_supports_tool_use(model_id: str) -> bool: return not any(pattern in model_lower for pattern in _NON_TOOL_CALLING_PATTERNS) -# Converse API cachePoint allowlist — inverted policy vs tools: an unsupported -# model rejects cachePoint with a ValidationException, so unknown models get -# NO cache markers until confirmed. Claude normally uses the AnthropicBedrock -# SDK path and only reaches build_converse_kwargs under bearer-token auth. -# Ref: https://docs.aws.amazon.com/bedrock/latest/userguide/prompt-caching.html -_CACHE_POINT_PATTERNS = [ - "anthropic.claude", # bearer-token fallback path - "amazon.nova", -] - - def _model_supports_prompt_cache(model_id: str) -> bool: - """True if the model accepts a Converse API cachePoint block.""" model_lower = model_id.lower() return any(pattern in model_lower for pattern in _CACHE_POINT_PATTERNS) # --- Server-verdict cachePoint suppression --- -# Bedrock's real cachePoint rule is per-family AND per-field (Nova accepts it -# in system/messages but hard-fails on toolConfig.tools), and any static table -# drifts as AWS ships new families — failing 100% of turns with no workaround. -# So when Bedrock names a placement as unpermitted we record that verdict, -# drop the marker from that placement for the rest of the process, and retry -# the rejected request once without it. +# Bedrock's real cachePoint rule is per-family AND per-field (Nova accepts it in +# system/messages but hard-fails on toolConfig.tools), and any static table +# drifts as AWS ships new families. So when Bedrock names a placement as +# unpermitted we record that verdict, drop the marker from that placement for +# the rest of the process, and retry the rejected request once without it. CACHE_POINT_PLACEMENTS = ("tools", "system", "messages") @@ -440,39 +392,35 @@ _CACHE_POINT_REJECTIONS: Dict[str, set] = {} # "#/toolConfig/tools/18: extraneous key [cachePoint] is not permitted" _CACHE_POINT_PATH_PATTERN = re.compile(r"#/(?P[A-Za-z0-9_./\[\]-]*)", re.IGNORECASE) +_CACHE_POINT = {"cachePoint": {"type": "default"}} def cache_point_rejection_placement(exc: BaseException) -> Optional[str]: """Return the Converse section whose cachePoint Bedrock refused, or None. - Message-based on purpose: the JSON pointer in the ValidationException is - the only thing that says *which* section was rejected, and the same - wording arrives both as a raw botocore ClientError and wrapped by SDKs. - An unlocalisable rejection maps to "tools" — the only placement any - supported family is known to refuse while accepting the others. + Message-based on purpose: the JSON pointer in the ValidationException is the + only thing that says *which* section was rejected, and the same wording + arrives raw from botocore and wrapped by SDKs. An unlocalisable rejection + maps to "tools" — the only placement a supported family is known to refuse. """ msg = str(exc) lowered = msg.lower() - if "cachepoint" not in lowered: - return None - if "not permitted" not in lowered and "extraneous" not in lowered: + if "cachepoint" not in lowered or ("not permitted" not in lowered and "extraneous" not in lowered): return None match = _CACHE_POINT_PATH_PATTERN.search(msg) path = (match.group("path") if match else "").lower() if "toolconfig" in path or "tools" in path: return "tools" - if "system" in path: - return "system" - if "messages" in path: - return "messages" + for placement in ("system", "messages"): + if placement in path: + return placement return "tools" def note_cache_point_rejection(model_id: str, placement: str) -> None: """Record that ``model_id`` refuses cachePoint blocks in ``placement``.""" - if placement not in CACHE_POINT_PLACEMENTS: - return - _CACHE_POINT_REJECTIONS.setdefault(model_id.lower(), set()).add(placement) + if placement in CACHE_POINT_PLACEMENTS: + _CACHE_POINT_REJECTIONS.setdefault(model_id.lower(), set()).add(placement) def cache_point_allowed(model_id: str, placement: str) -> bool: @@ -485,15 +433,11 @@ def reset_cache_point_rejections() -> None: _CACHE_POINT_REJECTIONS.clear() -def _is_cache_point_block(block: Any) -> bool: - return isinstance(block, dict) and set(block.keys()) == {"cachePoint"} - - def _without_cache_points(blocks: Any) -> Optional[list]: - """Return ``blocks`` minus cachePoint entries, or None if not a list / nothing removed.""" + """``blocks`` minus cachePoint entries, or None if not a list / nothing removed.""" if not isinstance(blocks, list): return None - cleaned = [b for b in blocks if not _is_cache_point_block(b)] + cleaned = [b for b in blocks if not (isinstance(b, dict) and set(b.keys()) == {"cachePoint"})] return None if len(cleaned) == len(blocks) else cleaned @@ -506,29 +450,21 @@ def strip_cache_points(kwargs: Dict[str, Any], placement: str) -> Dict[str, Any] if placement == "system": cleaned = _without_cache_points(kwargs.get("system")) return kwargs if cleaned is None else {**kwargs, "system": cleaned} - if placement == "tools": tool_config = kwargs.get("toolConfig") cleaned = _without_cache_points((tool_config or {}).get("tools")) - if cleaned is None: - return kwargs - return {**kwargs, "toolConfig": {**tool_config, "tools": cleaned}} - + return kwargs if cleaned is None else {**kwargs, "toolConfig": {**tool_config, "tools": cleaned}} if placement == "messages": messages = kwargs.get("messages") if not isinstance(messages, list): return kwargs - changed = False cleaned_messages = [] + changed = False for msg in messages: content = _without_cache_points(msg.get("content") if isinstance(msg, dict) else None) - if content is not None: - changed = True - cleaned_messages.append({**msg, "content": content}) - else: - cleaned_messages.append(msg) + changed = changed or content is not None + cleaned_messages.append(msg if content is None else {**msg, "content": content}) return {**kwargs, "messages": cleaned_messages} if changed else kwargs - return kwargs @@ -561,8 +497,8 @@ _REGIONAL_PREFIXES = ("global.", "us.", "eu.", "apac.", "ap.", "au.", "jp.", "ca def is_anthropic_bedrock_model(model_id: str) -> bool: """True for Claude on Bedrock (``anthropic.claude-*`` with any regional prefix). - These use the AnthropicBedrock SDK path for full feature parity (prompt - caching, thinking budgets); non-Claude models use the Converse path. + These use the AnthropicBedrock SDK path (prompt caching, thinking budgets); + non-Claude models use the Converse path. """ model_lower = model_id.lower() for prefix in _REGIONAL_PREFIXES: @@ -579,23 +515,21 @@ def convert_tools_to_converse(tools: List[Dict]) -> List[Dict]: result = [] for t in tools or []: fn = t.get("function", {}) - result.append({ - "toolSpec": { - "name": fn.get("name", ""), - "description": fn.get("description", ""), - "inputSchema": {"json": fn.get("parameters", {"type": "object", "properties": {}})}, - } - }) + result.append({"toolSpec": { + "name": fn.get("name", ""), + "description": fn.get("description", ""), + "inputSchema": {"json": fn.get("parameters", {"type": "object", "properties": {}})}, + }}) return result -# Converse rejects empty OR whitespace-only text blocks ("text content blocks -# must contain non-whitespace text"), so the placeholder must be non-whitespace. +# Converse rejects empty OR whitespace-only text blocks, so the placeholder must be non-whitespace. _EMPTY_TEXT_PLACEHOLDER = "(empty)" +_PLACEHOLDER_BLOCK = {"text": _EMPTY_TEXT_PLACEHOLDER} def _safe_text(text) -> str: - """Return ``text`` if it has non-whitespace content, else the placeholder (None/non-str ok).""" + """``text`` if it has non-whitespace content, else the placeholder (None/non-str ok).""" if text is None: return _EMPTY_TEXT_PLACEHOLDER if not isinstance(text, str): @@ -606,32 +540,26 @@ def _safe_text(text) -> str: def _image_block_from_data_url(url: str) -> Dict: """``data:;base64,...`` → Converse image block with RAW bytes. - boto3 base64-encodes at the wire layer, so passing the base64 string - through double-encodes and Bedrock rejects it ("Failed to sanitize image"). + boto3 base64-encodes at the wire layer, so passing the base64 string through + double-encodes and Bedrock rejects it ("Failed to sanitize image"). """ header, _, data = url.partition(",") - media_type = "image/jpeg" - if header.startswith("data:"): - mime_part = header[5:].split(";")[0] - if mime_part: - media_type = mime_part + media_type = (header[5:].split(";")[0] if header.startswith("data:") else "") or "image/jpeg" try: raw_bytes = base64.b64decode(data) except Exception: raw_bytes = data.encode("utf-8") - return { - "image": { - "format": media_type.split("/")[-1] if "/" in media_type else "jpeg", - "source": {"bytes": raw_bytes}, - } - } + return {"image": { + "format": media_type.split("/")[-1] if "/" in media_type else "jpeg", + "source": {"bytes": raw_bytes}, + }} def _convert_content_to_converse(content) -> List[Dict]: """OpenAI message content (str or parts list) → Converse content blocks. - Empty/whitespace-only text becomes the placeholder; remote image URLs are - not supported by Converse and are included as a text reference instead. + Empty/whitespace text becomes the placeholder; remote image URLs (unsupported + by Converse) become a text reference. """ if not isinstance(content, list): return [{"text": _safe_text(content)}] @@ -639,35 +567,25 @@ def _convert_content_to_converse(content) -> List[Dict]: for part in content: if isinstance(part, str): blocks.append({"text": _safe_text(part)}) + elif not isinstance(part, dict): continue - if not isinstance(part, dict): - continue - part_type = part.get("type", "") - if part_type == "text": + elif part.get("type", "") == "text": blocks.append({"text": _safe_text(part.get("text", ""))}) - elif part_type == "image_url": + elif part.get("type", "") == "image_url": image_url = part.get("image_url", {}) url = image_url.get("url", "") if isinstance(image_url, dict) else "" - if url.startswith("data:"): - blocks.append(_image_block_from_data_url(url)) - else: - blocks.append({"text": f"[Image: {url}]"}) - return blocks if blocks else [{"text": _EMPTY_TEXT_PLACEHOLDER}] + blocks.append(_image_block_from_data_url(url) if url.startswith("data:") else {"text": f"[Image: {url}]"}) + return blocks or [dict(_PLACEHOLDER_BLOCK)] def _system_blocks(content) -> List[Dict]: """System content → text blocks; blank parts are dropped, not placeholder-filled.""" - if isinstance(content, str): - return [{"text": content}] if content.strip() else [] + parts = [content] if isinstance(content, str) else content if isinstance(content, list) else [] blocks: List[Dict] = [] - if isinstance(content, list): - for part in content: - if isinstance(part, dict) and part.get("type") == "text": - text = part.get("text", "") - if isinstance(text, str) and text.strip(): - blocks.append({"text": text}) - elif isinstance(part, str) and part.strip(): - blocks.append({"text": part}) + for part in parts: + text = part.get("text", "") if isinstance(part, dict) and part.get("type") == "text" else part + if isinstance(text, str) and text.strip(): + blocks.append({"text": text}) return blocks @@ -675,11 +593,21 @@ def _tool_use_block(tool_use_id, name, input_dict) -> Dict: return {"toolUse": {"toolUseId": tool_use_id, "name": name, "input": input_dict}} +def _decode_redacted(encoded) -> Optional[bytes]: + """Strict base64 → bytes; None for empty/non-str/undecodable input.""" + if not isinstance(encoded, str) or not encoded: + return None + try: + return base64.b64decode(encoded, validate=True) + except (ValueError, TypeError): + return None + + def _replay_ordered_blocks(ordered_blocks: List) -> List[Dict]: """Rebuild the exact Bedrock block sequence captured at normalization time. - Redacted reasoning bytes are stored base64-encoded (JSON-safe sidecar) and - decoded here; undecodable entries are skipped. + Redacted reasoning bytes are stored base64-encoded (JSON-safe sidecar); + undecodable entries are skipped. """ content_blocks: List[Dict] = [] for block in ordered_blocks: @@ -696,10 +624,10 @@ def _replay_ordered_blocks(ordered_blocks: List) -> List[Dict]: replay["text"] = reasoning["text"] encoded = reasoning.get("redactedContentBase64") if isinstance(encoded, str) and encoded: - try: - replay["redactedContent"] = base64.b64decode(encoded, validate=True) - except (ValueError, TypeError): + redacted = _decode_redacted(encoded) + if redacted is None: continue + replay["redactedContent"] = redacted if replay: content_blocks.append({"reasoningContent": replay}) elif "toolUse" in block and isinstance(block["toolUse"], dict): @@ -708,13 +636,20 @@ def _replay_ordered_blocks(ordered_blocks: List) -> List[Dict]: return content_blocks +def _parse_tool_args(args) -> Any: + """JSON-decode a tool-call argument string; {} on failure; non-str passes through.""" + try: + return json.loads(args) if isinstance(args, str) else args + except (json.JSONDecodeError, TypeError): + return {} + + def _assistant_blocks(msg: Dict, content) -> List[Dict]: """Assistant message → Converse blocks. An ordered ``bedrock_content_blocks`` sidecar is authoritative and replayed - verbatim. Otherwise: redacted thinking from ``reasoning_details`` (so - opaque encrypted reasoning round-trips byte-for-byte), then text, then - tool calls. + verbatim. Otherwise: redacted thinking from ``reasoning_details`` (so opaque + encrypted reasoning round-trips byte-for-byte), then text, then tool calls. """ ordered_blocks = msg.get("bedrock_content_blocks") if isinstance(ordered_blocks, list) and ordered_blocks: @@ -724,16 +659,10 @@ def _assistant_blocks(msg: Dict, content) -> List[Dict]: content_blocks = [] for detail in (msg.get("reasoning_details") or []): - if not isinstance(detail, dict) or detail.get("type") != "redacted_thinking": - continue - encoded = detail.get("data") or detail.get("redactedContentBase64") - if not isinstance(encoded, str) or not encoded: - continue - try: - redacted = base64.b64decode(encoded, validate=True) - except (ValueError, TypeError): - continue - content_blocks.append({"reasoningContent": {"redactedContent": redacted}}) + if isinstance(detail, dict) and detail.get("type") == "redacted_thinking": + redacted = _decode_redacted(detail.get("data") or detail.get("redactedContentBase64")) + if redacted is not None: + content_blocks.append({"reasoningContent": {"redactedContent": redacted}}) if isinstance(content, str) and content.strip(): content_blocks.append({"text": content}) @@ -742,60 +671,48 @@ def _assistant_blocks(msg: Dict, content) -> List[Dict]: for tc in (msg.get("tool_calls", []) or []): fn = tc.get("function", {}) - args_str = fn.get("arguments", "{}") - try: - args_dict = json.loads(args_str) if isinstance(args_str, str) else args_str - except (json.JSONDecodeError, TypeError): - args_dict = {} - content_blocks.append(_tool_use_block(tc.get("id", ""), fn.get("name", ""), args_dict)) + content_blocks.append(_tool_use_block(tc.get("id", ""), fn.get("name", ""), _parse_tool_args(fn.get("arguments", "{}")))) return content_blocks -def _append_turn(converse_msgs: List[Dict], role: str, blocks: List[Dict]) -> None: - """Append blocks as a new turn, merging into the previous turn of the same role (strict alternation).""" - if converse_msgs and converse_msgs[-1]["role"] == role: - converse_msgs[-1]["content"].extend(blocks) - else: - converse_msgs.append({"role": role, "content": blocks}) - - def convert_messages_to_converse(messages: List[Dict]) -> Tuple[Optional[List[Dict]], List[Dict]]: """OpenAI messages → ``(system_blocks_or_None, converse_messages)``. - System messages become the system prompt; tool results become - ``toolResult`` blocks in a user turn. Converse requires strict - user/assistant alternation with a user turn first and last, so same-role - neighbours are merged and placeholder user turns are inserted at the ends. + System messages become the system prompt; tool results become ``toolResult`` + blocks in a user turn. Converse requires strict user/assistant alternation + with a user turn first and last, so same-role neighbours are merged and + placeholder user turns are inserted at the ends. """ system_blocks: List[Dict] = [] converse_msgs: List[Dict] = [] + def append_turn(role: str, blocks: List[Dict]) -> None: + if converse_msgs and converse_msgs[-1]["role"] == role: + converse_msgs[-1]["content"].extend(blocks) + else: + converse_msgs.append({"role": role, "content": blocks}) + for msg in messages: role = msg.get("role", "") content = msg.get("content") - if role == "system": system_blocks.extend(_system_blocks(content)) elif role == "tool": result_content = content if isinstance(content, str) else json.dumps(content) - _append_turn(converse_msgs, "user", [{ - "toolResult": { - "toolUseId": msg.get("tool_call_id", ""), - "content": [{"text": _safe_text(result_content)}], - } - }]) + append_turn("user", [{"toolResult": { + "toolUseId": msg.get("tool_call_id", ""), + "content": [{"text": _safe_text(result_content)}], + }}]) elif role == "assistant": - content_blocks = _assistant_blocks(msg, content) or [{"text": _EMPTY_TEXT_PLACEHOLDER}] - _append_turn(converse_msgs, "assistant", content_blocks) + append_turn("assistant", _assistant_blocks(msg, content) or [dict(_PLACEHOLDER_BLOCK)]) elif role == "user": - _append_turn(converse_msgs, "user", _convert_content_to_converse(content)) + append_turn("user", _convert_content_to_converse(content)) if converse_msgs and converse_msgs[0]["role"] != "user": - converse_msgs.insert(0, {"role": "user", "content": [{"text": _EMPTY_TEXT_PLACEHOLDER}]}) + converse_msgs.insert(0, {"role": "user", "content": [dict(_PLACEHOLDER_BLOCK)]}) if converse_msgs and converse_msgs[-1]["role"] != "user": - converse_msgs.append({"role": "user", "content": [{"text": _EMPTY_TEXT_PLACEHOLDER}]}) - - return (system_blocks if system_blocks else None, converse_msgs) + converse_msgs.append({"role": "user", "content": [dict(_PLACEHOLDER_BLOCK)]}) + return (system_blocks or None, converse_msgs) # --- Response format conversion: Bedrock Converse → OpenAI --- @@ -815,80 +732,74 @@ def _encode_redacted(redacted) -> Optional[str]: """Redacted reasoning payload → base64 str (bytes encoded, str passed through, else None).""" if isinstance(redacted, (bytes, bytearray)): return base64.b64encode(bytes(redacted)).decode("ascii") - if isinstance(redacted, str): - return redacted - return None + return redacted if isinstance(redacted, str) else None def _tool_call_ns(tool_use_id: str, name: str, input_dict) -> SimpleNamespace: return SimpleNamespace( - id=tool_use_id, - type="function", - function=SimpleNamespace(name=name, arguments=json.dumps(input_dict)), + id=tool_use_id, type="function", function=SimpleNamespace(name=name, arguments=json.dumps(input_dict)), ) -def _build_openai_response( - text_parts: List[str], reasoning_parts: List[str], reasoning_details: List[Dict[str, Any]], - tool_calls: List[SimpleNamespace], ordered_blocks: List[Dict[str, Any]], - usage_data: Dict[str, int], stop_reason: str, model: str, -) -> SimpleNamespace: - """Assemble the OpenAI-shaped response shared by the sync and streaming paths. +class _ResponseParts: + """Accumulator shared by the sync and streaming normalizers.""" - Converse's inputTokens EXCLUDES cache read/write tokens (OpenAI's - prompt_tokens includes them), so they are added back here and the - Anthropic-named cache fields are surfaced for downstream normalize_usage(). - """ - msg = SimpleNamespace( - role="assistant", - content="\n".join(text_parts) if text_parts else None, - tool_calls=tool_calls if tool_calls else None, - reasoning_content="\n\n".join(reasoning_parts) if reasoning_parts else None, - reasoning_details=reasoning_details or None, - bedrock_content_blocks=ordered_blocks or None, - ) + def __init__(self) -> None: + self.text_parts: List[str] = [] + self.reasoning_parts: List[str] = [] + self.reasoning_details: List[Dict[str, Any]] = [] + self.tool_calls: List[SimpleNamespace] = [] - input_tokens = usage_data.get("inputTokens", 0) - cache_read_tokens = usage_data.get("cacheReadInputTokens", 0) - cache_write_tokens = usage_data.get("cacheWriteInputTokens", 0) - output_tokens = usage_data.get("outputTokens", 0) - usage = SimpleNamespace( - prompt_tokens=input_tokens + cache_read_tokens + cache_write_tokens, - completion_tokens=output_tokens, - total_tokens=input_tokens + cache_read_tokens + cache_write_tokens + output_tokens, - cache_read_input_tokens=cache_read_tokens, - cache_creation_input_tokens=cache_write_tokens, - ) + def add_redacted(self, encoded: Optional[str]) -> None: + if encoded: + self.reasoning_details.append({"type": "redacted_thinking", "data": encoded}) - finish_reason = _STOP_REASON_TO_FINISH_REASON.get(stop_reason, "stop") - if tool_calls and finish_reason == "stop": - finish_reason = "tool_calls" + def build(self, ordered_blocks: List[Dict[str, Any]], usage_data: Dict[str, int], stop_reason: str, model: str) -> SimpleNamespace: + """Assemble the OpenAI-shaped response. - return SimpleNamespace( - choices=[SimpleNamespace(index=0, message=msg, finish_reason=finish_reason)], - usage=usage, - model=model, - ) + Converse's inputTokens EXCLUDES cache read/write tokens (OpenAI's + prompt_tokens includes them), so they are added back here and the + Anthropic-named cache fields are surfaced for downstream normalize_usage(). + """ + msg = SimpleNamespace( + role="assistant", + content="\n".join(self.text_parts) if self.text_parts else None, + tool_calls=self.tool_calls or None, + reasoning_content="\n\n".join(self.reasoning_parts) if self.reasoning_parts else None, + reasoning_details=self.reasoning_details or None, + bedrock_content_blocks=ordered_blocks or None, + ) + input_tokens = usage_data.get("inputTokens", 0) + cache_read_tokens = usage_data.get("cacheReadInputTokens", 0) + cache_write_tokens = usage_data.get("cacheWriteInputTokens", 0) + output_tokens = usage_data.get("outputTokens", 0) + prompt_tokens = input_tokens + cache_read_tokens + cache_write_tokens + usage = SimpleNamespace( + prompt_tokens=prompt_tokens, + completion_tokens=output_tokens, + total_tokens=prompt_tokens + output_tokens, + cache_read_input_tokens=cache_read_tokens, + cache_creation_input_tokens=cache_write_tokens, + ) + finish_reason = _STOP_REASON_TO_FINISH_REASON.get(stop_reason, "stop") + if self.tool_calls and finish_reason == "stop": + finish_reason = "tool_calls" + return SimpleNamespace( + choices=[SimpleNamespace(index=0, message=msg, finish_reason=finish_reason)], usage=usage, model=model, + ) def normalize_converse_response(response: Dict) -> SimpleNamespace: """Bedrock Converse response → OpenAI ``ChatCompletion``-shaped SimpleNamespace. Exposes ``.choices[0].message.{content,tool_calls,reasoning_content, - reasoning_details,bedrock_content_blocks}``, ``.choices[0].finish_reason`` - and ``.usage``. + reasoning_details,bedrock_content_blocks}``, ``.choices[0].finish_reason``, ``.usage``. """ - content_blocks = response.get("output", {}).get("message", {}).get("content", []) - - text_parts = [] - reasoning_parts = [] - reasoning_details = [] + parts = _ResponseParts() ordered_blocks = [] - tool_calls = [] - - for block in content_blocks: + for block in response.get("output", {}).get("message", {}).get("content", []): if "text" in block: - text_parts.append(block["text"]) + parts.text_parts.append(block["text"]) ordered_blocks.append({"text": block["text"]}) elif "reasoningContent" in block: reasoning = block["reasoningContent"] @@ -898,10 +809,10 @@ def normalize_converse_response(response: Dict) -> SimpleNamespace: encoded = _encode_redacted(reasoning.get("redactedContent")) ordered_reasoning = {} if thinking_text: - reasoning_parts.append(str(thinking_text)) + parts.reasoning_parts.append(str(thinking_text)) ordered_reasoning["text"] = str(thinking_text) if encoded: - reasoning_details.append({"type": "redacted_thinking", "data": encoded}) + parts.add_redacted(encoded) ordered_reasoning["redactedContentBase64"] = encoded if ordered_reasoning: ordered_blocks.append({"reasoningContent": ordered_reasoning}) @@ -909,11 +820,9 @@ def normalize_converse_response(response: Dict) -> SimpleNamespace: tu = block["toolUse"] tool_use_id, name, tool_input = tu.get("toolUseId", ""), tu.get("name", ""), tu.get("input", {}) ordered_blocks.append(_tool_use_block(tool_use_id, name, tool_input)) - tool_calls.append(_tool_call_ns(tool_use_id, name, tool_input)) - - return _build_openai_response( - text_parts, reasoning_parts, reasoning_details, tool_calls, ordered_blocks, - response.get("usage", {}), response.get("stopReason", "end_turn"), response.get("modelId", ""), + parts.tool_calls.append(_tool_call_ns(tool_use_id, name, tool_input)) + return parts.build( + ordered_blocks, response.get("usage", {}), response.get("stopReason", "end_turn"), response.get("modelId", ""), ) @@ -937,14 +846,11 @@ def stream_converse_with_callbacks( ``on_text_delta`` only fires while no toolUse block has been seen (same semantics as the Anthropic/chat_completions paths). ``on_interrupt_check`` runs per event; True stops streaming. ``on_event`` is a wire-level liveness - signal fired for EVERY event before any branching (so a watchdog can tell - "still receiving" from "wedged"); its exceptions are swallowed so it can - never abort the stream. Returns the ``normalize_converse_response()`` shape. + signal fired for EVERY event before any branching; its exceptions are + swallowed so it can never abort the stream. Returns the + ``normalize_converse_response()`` shape. """ - text_parts: List[str] = [] - reasoning_parts: List[str] = [] - reasoning_details: List[Dict[str, Any]] = [] - tool_calls: List[SimpleNamespace] = [] + parts = _ResponseParts() stream_blocks: Dict[int, Dict[str, Any]] = {} current_block_index: Optional[int] = None current_tool: Optional[Dict] = None @@ -953,15 +859,29 @@ def stream_converse_with_callbacks( stop_reason = "end_turn" usage_data: Dict[str, int] = {} - def _current_block(default: Dict[str, Any]) -> Dict[str, Any]: + def current_block(default: Dict[str, Any]) -> Dict[str, Any]: idx = current_block_index if current_block_index is not None else len(stream_blocks) return stream_blocks.setdefault(idx, default) - def _flush_text() -> None: - nonlocal current_text_buffer + def flush_text() -> None: if current_text_buffer: - text_parts.append("".join(current_text_buffer)) - current_text_buffer = [] + parts.text_parts.append("".join(current_text_buffer)) + current_text_buffer.clear() + + def on_reasoning(reasoning: Any) -> None: + if not isinstance(reasoning, dict): + return + thinking_text = reasoning.get("text", "") + if thinking_text: + parts.reasoning_parts.append(str(thinking_text)) + if on_reasoning_delta: + on_reasoning_delta(thinking_text) + block = current_block({"reasoningContent": {}}).setdefault("reasoningContent", {}) + block["text"] = block.get("text", "") + str(thinking_text) + encoded = _encode_redacted(reasoning.get("redactedContent")) + if encoded: + parts.add_redacted(encoded) + current_block({"reasoningContent": {}}).setdefault("reasoningContent", {})["redactedContentBase64"] = encoded for event in event_stream.get("stream", []): if on_event is not None: @@ -978,7 +898,7 @@ def stream_converse_with_callbacks( start = start_event.get("start", {}) if "toolUse" in start: has_tool_use = True - _flush_text() + flush_text() current_tool = { "toolUseId": start["toolUse"].get("toolUseId", ""), "name": start["toolUse"].get("name", ""), @@ -992,7 +912,7 @@ def stream_converse_with_callbacks( delta = event["contentBlockDelta"].get("delta", {}) if "text" in delta: text = delta["text"] - block = _current_block({"text": ""}) + block = current_block({"text": ""}) block["text"] = block.get("text", "") + text current_text_buffer.append(text) if on_text_delta and not has_tool_use: @@ -1001,33 +921,17 @@ def stream_converse_with_callbacks( if current_tool is not None: current_tool["input_json"] += delta["toolUse"].get("input", "") elif "reasoningContent" in delta: - reasoning = delta["reasoningContent"] - if isinstance(reasoning, dict): - thinking_text = reasoning.get("text", "") - if thinking_text: - reasoning_parts.append(str(thinking_text)) - if on_reasoning_delta: - on_reasoning_delta(thinking_text) - block = _current_block({"reasoningContent": {}}) - block.setdefault("reasoningContent", {})["text"] = block["reasoningContent"].get("text", "") + str(thinking_text) - encoded = _encode_redacted(reasoning.get("redactedContent")) - if encoded: - reasoning_details.append({"type": "redacted_thinking", "data": encoded}) - block = _current_block({"reasoningContent": {}}) - block.setdefault("reasoningContent", {})["redactedContentBase64"] = encoded + on_reasoning(delta["reasoningContent"]) elif "contentBlockStop" in event: if current_tool is not None: - try: - input_dict = json.loads(current_tool["input_json"]) if current_tool["input_json"] else {} - except (json.JSONDecodeError, TypeError): - input_dict = {} - tool_calls.append(_tool_call_ns(current_tool["toolUseId"], current_tool["name"], input_dict)) + input_dict = _parse_tool_args(current_tool["input_json"]) if current_tool["input_json"] else {} + parts.tool_calls.append(_tool_call_ns(current_tool["toolUseId"], current_tool["name"], input_dict)) if current_block_index is not None and current_block_index in stream_blocks: stream_blocks[current_block_index]["toolUse"]["input"] = input_dict current_tool = None else: - _flush_text() + flush_text() elif "messageStop" in event: stop_reason = event["messageStop"].get("stopReason", "end_turn") @@ -1035,26 +939,16 @@ def stream_converse_with_callbacks( elif "metadata" in event: meta_usage = event["metadata"].get("usage", {}) usage_data = { - "inputTokens": meta_usage.get("inputTokens", 0), - "outputTokens": meta_usage.get("outputTokens", 0), - "cacheReadInputTokens": meta_usage.get("cacheReadInputTokens", 0), - "cacheWriteInputTokens": meta_usage.get("cacheWriteInputTokens", 0), + key: meta_usage.get(key, 0) + for key in ("inputTokens", "outputTokens", "cacheReadInputTokens", "cacheWriteInputTokens") } - _flush_text() - - return _build_openai_response( - text_parts, reasoning_parts, reasoning_details, tool_calls, - [stream_blocks[i] for i in sorted(stream_blocks)], usage_data, stop_reason, "", - ) + flush_text() + return parts.build([stream_blocks[i] for i in sorted(stream_blocks)], usage_data, stop_reason, "") # --- High-level API: call Bedrock Converse --- -def _cache_point() -> Dict[str, Any]: - return {"cachePoint": {"type": "default"}} - - def build_converse_kwargs( model: str, messages: List[Dict], @@ -1068,30 +962,24 @@ def build_converse_kwargs( """Build kwargs for ``bedrock-runtime.converse()`` / ``converse_stream()``. ``max_tokens=None`` omits ``inferenceConfig.maxTokens`` so Bedrock uses the - model's maximum output (callers wanting the full budget, e.g. uncapped - auxiliary vision calls, pass None explicitly; default stays 4096). - cachePoint markers go on system, tools and the second-newest message - (so the marker survives as only the tail grows — mirrors the Anthropic - system_and_3 strategy) — each only if the model supports caching and - Bedrock has not rejected that placement. + model's maximum output (default stays 4096). cachePoint markers go on system, + tools and the second-newest message (so the marker survives as only the tail + grows — mirrors the Anthropic system_and_3 strategy), each only if the model + supports caching and Bedrock has not rejected that placement. """ system_prompt, converse_messages = convert_messages_to_converse(messages) cache_enabled = _model_supports_prompt_cache(model) + def cache_here(placement: str) -> bool: + return cache_enabled and cache_point_allowed(model, placement) + inference_config: Dict[str, Any] = {} if max_tokens is not None: inference_config["maxTokens"] = max_tokens - - kwargs: Dict[str, Any] = { - "modelId": model, - "messages": converse_messages, - "inferenceConfig": inference_config, - } + kwargs: Dict[str, Any] = {"modelId": model, "messages": converse_messages, "inferenceConfig": inference_config} if system_prompt: - if cache_enabled and cache_point_allowed(model, "system"): - system_prompt = system_prompt + [_cache_point()] - kwargs["system"] = system_prompt + kwargs["system"] = system_prompt + [dict(_CACHE_POINT)] if cache_here("system") else system_prompt from agent.anthropic_adapter import _forbids_sampling_params @@ -1100,40 +988,32 @@ def build_converse_kwargs( inference_config["temperature"] = temperature if top_p is not None: inference_config["topP"] = top_p - if stop_sequences: inference_config["stopSequences"] = stop_sequences - if tools: - converse_tools = convert_tools_to_converse(tools) - if converse_tools: - # Non-tool-calling models (e.g. DeepSeek R1) reject toolConfig with a - # ValidationException → retry loop → failure. Strip tools and warn. - if _model_supports_tool_use(model): - if cache_enabled and cache_point_allowed(model, "tools"): - converse_tools = converse_tools + [_cache_point()] - kwargs["toolConfig"] = {"tools": converse_tools} - else: - logger.warning( - "Model %s does not support tool calling — tools stripped. " - "The agent will operate in text-only mode.", model - ) + converse_tools = convert_tools_to_converse(tools) if tools else [] + if converse_tools: + # Non-tool-calling models (e.g. DeepSeek R1) reject toolConfig with a + # ValidationException → retry loop → failure. Strip tools and warn. + if _model_supports_tool_use(model): + if cache_here("tools"): + converse_tools = converse_tools + [dict(_CACHE_POINT)] + kwargs["toolConfig"] = {"tools": converse_tools} + else: + logger.warning( + "Model %s does not support tool calling — tools stripped. " + "The agent will operate in text-only mode.", model + ) - if ( - cache_enabled - and cache_point_allowed(model, "messages") - and len(converse_messages) >= 2 - ): + if cache_here("messages") and len(converse_messages) >= 2: content = converse_messages[-2].get("content") if isinstance(content, list) and content: - content.append(_cache_point()) + content.append(dict(_CACHE_POINT)) if guardrail_config: kwargs["guardrailConfig"] = guardrail_config - if not inference_config: del kwargs["inferenceConfig"] # optional on the wire; don't send {} - return kwargs @@ -1157,7 +1037,6 @@ def call_converse( kwargs = build_converse_kwargs( model, messages, tools, max_tokens, temperature, top_p, stop_sequences, guardrail_config, ) - try: response = client.converse(**kwargs) except Exception as exc: @@ -1197,12 +1076,12 @@ def _list_foundation_models(client, filter_set: set, models: List[Dict[str, Any] model_prefix = model_id.split(".")[0].lower() if "." in model_id else "" if provider_name not in filter_set and model_prefix not in filter_set: continue - if summary.get("modelLifecycle", {}).get("status", "").upper() != "ACTIVE": - continue - if not summary.get("responseStreamingSupported", False): - continue output_mods = summary.get("outputModalities", []) - if "TEXT" not in output_mods: + if ( + summary.get("modelLifecycle", {}).get("status", "").upper() != "ACTIVE" + or not summary.get("responseStreamingSupported", False) + or "TEXT" not in output_mods + ): continue models.append({ "id": model_id, @@ -1219,8 +1098,7 @@ def _list_inference_profiles(client, filter_set: set, models: List[Dict[str, Any profiles = [] next_token = None while True: - kwargs = {"nextToken": next_token} if next_token else {} - response = client.list_inference_profiles(**kwargs) + response = client.list_inference_profiles(**({"nextToken": next_token} if next_token else {})) profiles.extend(response.get("inferenceProfileSummaries", [])) next_token = response.get("nextToken") if not next_token: @@ -1232,8 +1110,7 @@ def _list_inference_profiles(client, filter_set: set, models: List[Dict[str, Any if not profile_id or profile.get("status") != "ACTIVE" or profile_id.lower() in seen_ids: continue if filter_set and not any( - _extract_provider_from_arn(m.get("modelArn", "")).lower() in filter_set - for m in profile.get("models", []) + _extract_provider_from_arn(m.get("modelArn", "")).lower() in filter_set for m in profile.get("models", []) ): continue models.append({ @@ -1268,24 +1145,18 @@ def discover_bedrock_models(region: str, provider_filter: Optional[List[str]] = logger.warning("Failed to create Bedrock client for model discovery: %s", e) return [] - models = [] + models: List[Dict[str, Any]] = [] filter_set = {f.lower() for f in (provider_filter or [])} - try: _list_foundation_models(client, filter_set, models) except Exception as e: logger.warning("Failed to list Bedrock foundation models: %s", e) - try: _list_inference_profiles(client, filter_set, models) except Exception as e: logger.debug("Skipping inference profile discovery: %s", e) - models.sort(key=lambda m: ( - 0 if m["id"].startswith("global.") else 1, - m["name"].lower(), - )) - + models.sort(key=lambda m: (0 if m["id"].startswith("global.") else 1, m["name"].lower())) _discovery_cache[cache_key] = {"timestamp": time.time(), "models": models} return models @@ -1297,15 +1168,15 @@ def _extract_provider_from_arn(arn: str) -> str: # --- Bedrock model context lengths --- -# Static fallback table for when the live probe is unavailable; used by -# agent/model_metadata.py. Keys are matched by longest substring, so versioned -# entries (opus-4-6/4-7/4-8) win over the generic "anthropic.claude-opus-4". +# Static fallback for when the live probe is unavailable (used by +# agent/model_metadata.py). Keys match by longest substring, so versioned entries +# (opus-4-6/4-7/4-8) win over the generic "anthropic.claude-opus-4". BEDROCK_CONTEXT_LENGTHS: Dict[str, int] = { - # Anthropic Claude (per Anthropic's models overview). Fable / Sonnet 5 / - # Opus 4.8-4.6 / Sonnet 4.6 are 1M GA; Sonnet 4.5 / Sonnet 4 lost their 1M - # beta and are 200K; Haiku 4.5 is 200K. The 1M entries must match - # agent/model_metadata.py DEFAULT_CONTEXT_LENGTHS or context compresses early. + # Anthropic Claude. Fable / Sonnet 5 / Opus 4.8-4.6 / Sonnet 4.6 are 1M GA; + # Sonnet 4.5 / Sonnet 4 lost their 1M beta and are 200K; Haiku 4.5 is 200K. + # The 1M entries must match agent/model_metadata.py DEFAULT_CONTEXT_LENGTHS + # or context compresses early. "anthropic.claude-fable-5": 1_000_000, "anthropic.claude-fable": 1_000_000, "anthropic.claude-sonnet-5": 1_000_000, @@ -1345,38 +1216,27 @@ BEDROCK_CONTEXT_LENGTHS: Dict[str, int] = { # Default for unknown Bedrock models BEDROCK_DEFAULT_CONTEXT_LENGTH = 128_000 -# Probe padding targets (tokens). Tiered rather than one giant request because -# a wildly oversized payload (5M tokens) yields an opaque InternalServerException -# instead of a clean ValidationException, and stepping up discovers 2M+ -# windows without over-padding smaller ones. The error reports the true maximum. +# Probe padding targets (tokens). Tiered because a wildly oversized payload (5M +# tokens) yields an opaque InternalServerException instead of a clean +# ValidationException; stepping up discovers 2M+ windows without over-padding. _BEDROCK_PROBE_TIERS = (1_300_000, 2_200_000) _WORDS_PER_TOKEN = 0.9 # conservative: ensures the padded prompt clears the tier -def _static_bedrock_context_length(model_id: str) -> int: - """Longest-substring match against the static table (versioned IDs resolve correctly).""" - model_lower = model_id.lower() - matches = [key for key in BEDROCK_CONTEXT_LENGTHS if key in model_lower] - return BEDROCK_CONTEXT_LENGTHS[max(matches, key=len)] if matches else BEDROCK_DEFAULT_CONTEXT_LENGTH - - def probe_bedrock_context_length(model_id: str, region: str) -> Optional[int]: """Discover a model's real context window by provoking a length error. - No Bedrock metadata API exposes the window; the only authoritative source - is the ValidationException for an oversized prompt ("prompt is too long: - 1300032 tokens > 1000000 maximum"). Length validation happens before - inference, so the probe is rejected immediately and costs nothing. If a - tier is accepted, that tier is returned as a safe lower bound. - - Returns None if the probe could not run (no credentials, network error, - no parseable limit) so the caller falls back to the static table. + No Bedrock metadata API exposes the window; the only authoritative source is + the ValidationException for an oversized prompt ("prompt is too long: 1300032 + tokens > 1000000 maximum"). Length validation happens before inference, so + the probe costs nothing. If a tier is accepted, that tier is returned as a + safe lower bound. Returns None if the probe could not run (no credentials, + network error, no parseable limit) so the caller falls back to the static table. """ try: from agent.model_metadata import parse_context_limit_from_error except ImportError: # pragma: no cover — same package return None - try: client = _get_bedrock_runtime_client(region) except Exception as exc: # boto3 missing / credential resolution failure @@ -1401,30 +1261,26 @@ def probe_bedrock_context_length(model_id: str, region: str) -> Optional[int]: last_error = str(exc) limit = parse_context_limit_from_error(last_error) if limit and limit >= 1024: - logger.info( - "Probed Bedrock context window for %s: %s tokens", - model_id, f"{limit:,}", - ) + logger.info("Probed Bedrock context window for %s: %s tokens", model_id, f"{limit:,}") return limit # Opaque server error / auth / throttle at this tier — try the next. - logger.debug( - "Bedrock context probe for %s returned no parseable limit: %s", - model_id, last_error[:200], - ) + logger.debug("Bedrock context probe for %s returned no parseable limit: %s", model_id, last_error[:200]) return None def get_bedrock_context_length(model_id: str, region: str = "", probe: bool = True) -> int: """Context window: live probe (if ``probe`` and ``region``) → static table → default. - The static table is a fallback only: AWS ships new versions faster than - the table tracks, and a stale substring match silently caps the window - (e.g. a 1M Opus pinned to 200K via "opus-4"). ``probe=False`` or an empty - region skips the network call for offline/display paths. + The static table is a fallback only: AWS ships new versions faster than the + table tracks, and a stale substring match silently caps the window (e.g. a 1M + Opus pinned to 200K via "opus-4"). ``probe=False`` or an empty region skips + the network call for offline/display paths. """ if probe and region: probed = probe_bedrock_context_length(model_id, region) if probed: return probed - return _static_bedrock_context_length(model_id) + model_lower = model_id.lower() + matches = [key for key in BEDROCK_CONTEXT_LENGTHS if key in model_lower] + return BEDROCK_CONTEXT_LENGTHS[max(matches, key=len)] if matches else BEDROCK_DEFAULT_CONTEXT_LENGTH From ac268d2d7a298e5c11deff49f68c0d35593d33d3 Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Wed, 2 Sep 2026 18:22:57 -0700 Subject: [PATCH 02/13] refactor(agent/codex_responses_adapter): extract _OutputScan from _normalize_codex_response, share part-type/text-type/call-id helpers; compact docs (-49 LOC) --- agent/codex_responses_adapter.py | 475 ++++++++++++++----------------- 1 file changed, 213 insertions(+), 262 deletions(-) diff --git a/agent/codex_responses_adapter.py b/agent/codex_responses_adapter.py index 83a40906e1..da3e23c774 100644 --- a/agent/codex_responses_adapter.py +++ b/agent/codex_responses_adapter.py @@ -28,8 +28,8 @@ def _classify_responses_issuer( """Stable identifier for the endpoint that mints ``reasoning.encrypted_content``. Blobs are sealed to their issuer (replaying across endpoints yields HTTP 400 - ``invalid_encrypted_content``); stamping items with the issuer lets replay - drop foreign blobs after a mid-conversation model switch. + ``invalid_encrypted_content``); stamping items lets replay drop foreign + blobs after a mid-conversation model switch. """ if is_xai_responses: return "xai_responses" @@ -40,13 +40,11 @@ def _classify_responses_issuer( return f"other:{base_url}" if base_url else "other" -# Per-process throttle for the cross-issuer skip warning (long histories can -# carry many stale-issuer reasoning blocks). +# Per-process throttle for the cross-issuer skip warning. _CROSS_ISSUER_WARN_EMITTED = False # Codex/Harmony tool-call serialization leaked into assistant text when the -# model fails to emit a structured ``function_call`` (``to=functions.``, -# optionally prefixed by ``assistant`` or a Harmony channel marker). +# model fails to emit a structured ``function_call``. _TOOL_CALL_LEAK_PATTERN = re.compile(r"(?:^|[\s>|])to=functions\.[A-Za-z_][\w.]*", re.IGNORECASE) # The Codex backend rejects requests containing these literal Harmony wire @@ -57,33 +55,29 @@ _FULLWIDTH_PIPE = "\uff5c" _TEXT_PART_TYPES = {"text", "input_text", "output_text"} _IMAGE_PART_TYPES = {"image_url", "input_image"} +_OUTPUT_TEXT_TYPES = {"output_text", "text"} _ASSISTANT_IMAGE_PLACEHOLDER = "[Assistant image omitted during replay]" _INCOMPLETE_STATUSES = {"queued", "in_progress", "incomplete"} _RESPONSE_MESSAGE_STATUSES = {"completed", "incomplete", "in_progress"} -# input[].id longer than this is a non-retryable 400 ("string too long"). -# Codex-issued assistant message ids can run 400+ chars; Hermes-minted -# ``msg_...`` ids stay under the cap and are kept for prefix-cache hits. -# Function names share the same cap (same non-retryable 400). +# input[].id / function names longer than this are a non-retryable 400 +# ("string too long"). Codex-issued assistant message ids can run 400+ chars; +# Hermes-minted ``msg_...`` ids stay under the cap and are kept for cache hits. _MAX_RESPONSES_ITEM_ID_LENGTH = 64 _VALID_RESPONSES_FN_NAME_RE = re.compile(r"[a-zA-Z0-9_-]{1,64}") -# Provider-executed built-in tools: declared on ``tools`` by ``type`` alone -# (no name/parameters) and run server-side, reporting via the ``*_call`` -# output items below. Hermes injects xAI's ``web_search`` in -# agent/transports/codex.py; the rest are listed so preflight passes them -# through instead of rejecting them as "unsupported type". +# Provider-executed built-in tools: declared on ``tools`` by ``type`` alone and +# run server-side, reporting via the ``*_call`` output items below. Preflight +# passes them through instead of rejecting them as "unsupported type". _RESPONSES_BUILTIN_TOOL_TYPES = { "web_search", "web_search_preview", "file_search", "code_interpreter", "image_generation", "computer_use_preview", "local_shell", } # Server-side ``*_call`` output items. xAI routinely leaves these at -# ``status="in_progress"`` even when the response is ``completed`` (the search -# finished server-side; the per-item status is never reconciled), so they must +# ``status="in_progress"`` even when the response is ``completed``, so they must # NOT flip the incomplete verdict — otherwise every server-search turn burns 3 -# fruitless continuation retries. Client-side function/custom tool calls keep -# their own in_progress handling. +# fruitless continuation retries. _SERVER_SIDE_TOOL_CALL_TYPES = { "web_search_call", "file_search_call", "code_interpreter_call", "image_generation_call", "computer_call", "local_shell_call", "mcp_call", @@ -111,6 +105,14 @@ def _field(obj: Any, name: str, default: Any = None) -> Any: return obj.get(name) if isinstance(obj, dict) else getattr(obj, name, default) +def _part_type(part: Dict[str, Any]) -> str: + return str(part.get("type") or "").strip().lower() + + +def _text_type_for(role: str) -> str: + return "output_text" if role == "assistant" else "input_text" + + def _coerce_arguments(arguments: Any) -> str: """Normalize replayed tool-call arguments to a non-empty JSON string.""" if isinstance(arguments, dict): @@ -155,15 +157,13 @@ def _neutralize_harmony_structure(value: Any) -> Any: if isinstance(value, (list, tuple)): return [_neutralize_harmony_structure(item) for item in value] if isinstance(value, dict): - normalized = {} - for key, item in value.items(): + for key in value: if isinstance(key, str) and _neutralize_harmony_tokens(key) != key: raise ValueError( "Reserved Harmony tokens in a JSON object key cannot be " "neutralized without changing its contract." ) - normalized[key] = _neutralize_harmony_structure(item) - return normalized + return {key: _neutralize_harmony_structure(item) for key, item in value.items()} return value @@ -176,7 +176,7 @@ def _iter_content_parts(content: list) -> Iterator[tuple[str, Any]]: if part: yield "text", part elif isinstance(part, dict): - ptype = str(part.get("type") or "").strip().lower() + ptype = _part_type(part) if ptype in _TEXT_PART_TYPES and _nonempty_str(part.get("text")): yield "text", part["text"] elif ptype in _IMAGE_PART_TYPES: @@ -203,15 +203,12 @@ def _chat_content_to_responses_parts(content: Any, *, role: str = "user") -> Lis """Convert chat-style multimodal content to Responses API input parts. Text becomes ``input_text`` (user) or ``output_text`` (assistant) — the API - rejects the wrong type per role, so callers MUST pass the right role. - ``input_image`` is only legal on user messages; on assistant messages the - image is replaced by a text marker (an assistant ``input_image`` 400s on - every replay and bricks the session, and the wire cannot carry it anyway). - - Returns an empty list when ``content`` is not a list or has no recognized - parts — callers fall back to the string path. + rejects the wrong type per role. ``input_image`` is only legal on user + messages; on assistant messages the image becomes a text marker (an + assistant ``input_image`` 400s on every replay and bricks the session). + Returns [] when ``content`` is not a list or has no recognized parts. """ - text_type = "output_text" if role == "assistant" else "input_text" + text_type = _text_type_for(role) converted: List[Dict[str, Any]] = [] for kind, payload in _iter_content_parts(content if isinstance(content, list) else []): if kind == "text": @@ -230,8 +227,8 @@ def _summarize_user_message_for_log(content: Any, *, sep: str = " ") -> str: Text parts are joined with ``sep`` (``" "`` for logs/spinner/trajectories; ``"\\n"`` for memory providers that feed the text to regexes and text APIs); - images become a ``[N image(s)]`` marker so the attachment is not erased. - Returns ``""`` for None/empty lists and ``str(content)`` for other scalars. + images become a ``[N image(s)]`` marker. ``""`` for None/empty lists, + ``str(content)`` for other scalars. """ if content is None: return "" @@ -255,53 +252,43 @@ def _summarize_user_message_for_log(content: Any, *, sep: str = " ") -> str: # --- ID helpers --------------------------------------------------------------- def _deterministic_call_id(fn_name: str, arguments: str, index: int = 0) -> str: - """Deterministic call_id (random ids would break the prompt-cache prefix). - - Thin wrapper over ``agent.message_sanitization.deterministic_call_id``; - kept here because run_agent and tests import it from this module. - """ + """Deterministic call_id (random ids would break the prompt-cache prefix). Re-exported for run_agent/tests.""" return deterministic_call_id(fn_name, arguments, index) def _clamp_responses_call_id(call_id: str) -> str: """Keep ``call_id`` within the API's 64-char cap. - The codex app-server namespaces MCP call ids (``codex_mcp_____ - ``) past the cap, and the resulting 400 replays on every turn. The - surrogate is a pure function of the original so a ``function_call`` and its - ``function_call_output`` map to the same value; short ids pass through. + The codex app-server namespaces MCP call ids past the cap, and the resulting + 400 replays on every turn. The surrogate is a pure function of the original + so a ``function_call`` and its ``function_call_output`` map to the same value. """ if len(call_id) <= _MAX_RESPONSES_ITEM_ID_LENGTH: return call_id - digest = hashlib.sha256(call_id.encode("utf-8", errors="replace")).hexdigest()[:32] - return f"call_{digest}" + return f"call_{hashlib.sha256(call_id.encode('utf-8', errors='replace')).hexdigest()[:32]}" def _sanitize_replayed_fn_name(name: str) -> str: """Coerce a *replayed* ``function_call.name`` to ``^[a-zA-Z0-9_-]{1,64}$``. - An invalid name stored in history (dots, spaces, unicode from a model - degeneration) otherwise 400s every later turn. Invalid runs collapse to - ``_``; an all-invalid name degrades to ``"fn"`` rather than an empty string - (which would just trade the 400 for a preflight ValueError). Apply ONLY to - replayed items, never to live tool definitions (schema names must match the - dispatch registry); pairing with the output is by call_id, so renaming is safe. + An invalid name stored in history otherwise 400s every later turn. Invalid + runs collapse to ``_``; an all-invalid name degrades to ``"fn"``. Apply ONLY + to replayed items, never to live tool definitions (schema names must match + the dispatch registry); pairing with the output is by call_id. """ if not isinstance(name, str): return "fn" if _VALID_RESPONSES_FN_NAME_RE.fullmatch(name): return name - coerced = re.sub(r"[^A-Za-z0-9_-]", "_", name.strip()) - coerced = re.sub(r"_+", "_", coerced).strip("_") + coerced = re.sub(r"_+", "_", re.sub(r"[^A-Za-z0-9_-]", "_", name.strip())).strip("_") return coerced[:64] or "fn" def _canonical_call_id_from_fc(response_item_id: Any) -> Optional[str]: """Map an ``fc_…`` item id to its canonical ``call_``. - Both sides of a replayed pair must derive the SAME call_id from an - fc_-only stored id, or an oversized pair clamps to two different - surrogates. Every caller must go through this helper. + Both sides of a replayed pair must derive the SAME call_id from an fc_-only + stored id, or an oversized pair clamps to two different surrogates. """ if isinstance(response_item_id, str) and response_item_id.startswith("fc_") and len(response_item_id) > 3: return f"call_{response_item_id[3:]}" @@ -347,7 +334,6 @@ def _derive_responses_function_call_id(call_id: str, response_item_id: Optional[ return f"fc_{candidate[len('call_'):]}" if sanitized: return f"fc_{sanitized[:48]}" - seed = source or str(response_item_id or "") or uuid.uuid4().hex return f"fc_{hashlib.sha1(seed.encode('utf-8')).hexdigest()[:24]}" @@ -356,30 +342,23 @@ def _derive_responses_function_call_id(call_id: str, response_item_id: Optional[ def _responses_tools(tools: Optional[List[Dict[str, Any]]] = None) -> Optional[List[Dict[str, Any]]]: """Convert chat-completions tool schemas to Responses function-tool schemas.""" - if not tools: - return None - converted: List[Dict[str, Any]] = [] - for item in tools: + for item in tools or []: fn = item.get("function", {}) if isinstance(item, dict) else {} name = fn.get("name") - if not _nonblank(name): - continue - converted.append({ - "type": "function", "name": name, "description": fn.get("description", ""), "strict": False, - "parameters": fn.get("parameters", {"type": "object", "properties": {}}), - }) + if _nonblank(name): + converted.append({ + "type": "function", "name": name, "description": fn.get("description", ""), "strict": False, + "parameters": fn.get("parameters", {"type": "object", "properties": {}}), + }) return converted or None # --- Message format conversion (chat history -> Responses input) -------------- def _normalize_responses_message_status(value: Any, *, default: str = "completed") -> str: - """Normalize a replayed assistant message status (completed/incomplete/in_progress). - - Preserved modulo case/hyphen spelling so incomplete Codex continuation - turns are not falsely marked completed. - """ + """Normalize a replayed assistant message status, modulo case/hyphen spelling, so + incomplete Codex continuation turns are not falsely marked completed.""" if isinstance(value, str): status = value.strip().lower().replace("-", "_").replace(" ", "_") if status in _RESPONSE_MESSAGE_STATUSES: @@ -402,11 +381,11 @@ def _message_item( def _assistant_message_item( raw: Dict[str, Any], content: List[Dict[str, Any]], *, is_github_responses: bool, ) -> Dict[str, Any]: - """Build a replayable assistant ``message`` item from a stored one. + """Replayable assistant ``message`` item from a stored one. - ``id`` is kept only when short enough (see _MAX_RESPONSES_ITEM_ID_LENGTH) - and never for GitHub Copilot, which binds ids to a backend connection and - 401s on a stale one; ``phase`` is preserved per OpenAI's cache guidance. + ``id`` is kept only when short enough and never for GitHub Copilot, which + binds ids to a backend connection and 401s on a stale one; ``phase`` is + preserved per OpenAI's cache guidance. """ item_id, phase = raw.get("id"), raw.get("phase") keep_id = not is_github_responses and _nonblank(item_id) and len(item_id.strip()) <= _MAX_RESPONSES_ITEM_ID_LENGTH @@ -422,11 +401,11 @@ def _replay_reasoning_items( """Replay persisted encrypted reasoning/compaction items for one assistant turn. Skips: duplicate ids; ``compaction`` checkpoints unless THIS request carries - ``context_management`` (a persisted checkpoint outlives the gate and would - otherwise erase pre-checkpoint history on a model that cannot decrypt it); - items stamped by a different issuer (undecryptable → HTTP 400). Unstamped - legacy items pass through. ``id`` is stripped (store=False lookups 404) along - with the Hermes-only ``_issuer_kind`` stamp. + ``context_management`` (a persisted checkpoint would otherwise erase + pre-checkpoint history on a model that cannot decrypt it); items stamped by + a different issuer (undecryptable → HTTP 400). Unstamped legacy items pass + through. ``id`` is stripped (store=False lookups 404) with the Hermes-only + ``_issuer_kind`` stamp. """ global _CROSS_ISSUER_WARN_EMITTED codex_reasoning = msg.get("codex_reasoning_items") @@ -472,7 +451,7 @@ def _replay_message_items(msg: Dict[str, Any], *, is_github_responses: bool) -> content = [ {"type": "output_text", "text": _str_or_empty(part.get("text", ""))} for part in raw_item["content"] - if isinstance(part, dict) and str(part.get("type") or "").strip() in {"output_text", "text"} + if isinstance(part, dict) and str(part.get("type") or "").strip() in _OUTPUT_TEXT_TYPES ] if content: replayed.append(_assistant_message_item(raw_item, content, is_github_responses=is_github_responses)) @@ -492,7 +471,6 @@ def _replay_tool_call_items(msg: Dict[str, Any], *, start_index: int) -> List[Di fn_name = fn.get("name") if not _nonblank(fn_name): continue - call_id = _resolve_call_id( tc.get("call_id"), tc.get("id"), fn_name, str(fn.get("arguments", "{}")), start_index + len(replayed), canonicalize_fc=True, @@ -537,37 +515,31 @@ def _chat_messages_to_responses_input( ) -> List[Dict[str, Any]]: """Convert internal chat-style messages to Responses input items. - ``is_xai_responses``: kept for transport signature compatibility; encrypted - reasoning IS replayed on xAI (it relies on cross-turn reasoning threading). + ``is_xai_responses``: transport signature compatibility only; encrypted + reasoning IS replayed on xAI (cross-turn reasoning threading). - ``replay_encrypted_reasoning``: per-session kill switch. Relays that reject - a replayed blob with HTTP 400 ``invalid_encrypted_content`` trigger - ``AIAgent._disable_codex_reasoning_replay``, which strips cached items and - threads False here so later turns send no reasoning items. + ``replay_encrypted_reasoning``: per-session kill switch. Relays that reject a + replayed blob with HTTP 400 ``invalid_encrypted_content`` trigger + ``AIAgent._disable_codex_reasoning_replay``, which threads False here. ``is_github_responses``: drops ``id`` from replayed message items regardless - of length — Copilot binds ids to a backend connection and rejects a stale - one with HTTP 401 even for short ids. phase/status/content still replay. + of length — Copilot binds ids to a backend connection (HTTP 401 on stale). - ``current_issuer_kind``: per-item cross-issuer guard (runs only while replay - is enabled); items stamped by another endpoint are dropped, unstamped legacy - items replay. + ``current_issuer_kind``: per-item cross-issuer guard (only while replay is + enabled); items stamped by another endpoint are dropped, legacy items replay. - ``native_compaction_eligible``: mirrors, for THIS request, whether - ``native_compaction_context_management`` produced a payload. Gates both - replaying ``compaction`` checkpoints and restructuring the wire around them - (``prune_pre_checkpoint_items``). Checkpoints persist in the reasoning - sidecar across model swaps / compression flips / resume; without this gate - one captured checkpoint would delete pre-checkpoint history from every later - request on a model that cannot decrypt it. Default False = pre-feature wire, - correct for callers that never send ``context_management``. Dropping the - checkpoint is lossless: local history is never truncated by native compaction. + ``native_compaction_eligible``: whether THIS request carries + ``context_management``. Gates both replaying ``compaction`` checkpoints and + ``prune_pre_checkpoint_items``. Checkpoints persist in the reasoning sidecar + across model swaps / compression flips / resume; without this gate one + checkpoint would delete pre-checkpoint history from every later request on a + model that cannot decrypt it. Dropping the checkpoint is lossless: local + history is never truncated by native compaction. """ items: List[Dict[str, Any]] = [] # Parallel to ``items``: the raw chat message each item came from. Pruning - # reads a summary carrier's up-to-date, provenance-tagged content from the - # source — the converted item may be a lossy shape (stale exact replay, or - # a typed ``function_call_output`` wrapper) that no longer carries it. + # reads a summary carrier's provenance-tagged content from the source, since + # the converted item may be a lossy shape that no longer carries it. item_sources: List[Optional[Dict[str, Any]]] = [] seen_item_ids: set = set() @@ -590,10 +562,10 @@ def _chat_messages_to_responses_input( content = msg.get("content", "") content_parts = _chat_content_to_responses_parts(content, role=role) # [] unless a list if isinstance(content, list): - text_type = "output_text" if role == "assistant" else "input_text" + text_type = _text_type_for(role) content_text = "".join(p["text"] for p in content_parts if p["type"] == text_type) else: - content_text = "" if content is None else str(content) + content_text = _str_or_empty(content) if role == "user": emit([{"role": role, "content": content_parts or content_text}], msg) @@ -613,18 +585,16 @@ def _chat_messages_to_responses_input( elif content_text.strip(): emit([{"role": "assistant", "content": content_text}], msg) elif reasoning_items: - # Every reasoning item needs a following item (else - # missing_following_item); emit an empty message. + # Every reasoning item needs a following item (else missing_following_item). emit([{"role": "assistant", "content": ""}], msg) emit(_replay_tool_call_items(msg, start_index=len(items)), msg) - # Native server-side compaction: the server renders nothing placed before a - # compaction item, so pre-checkpoint history is dead upload weight and the - # user's plaintext asks / merged local summaries silently vanish. Keep the - # newest checkpoint first, retain pre-checkpoint USER and compression-SUMMARY - # messages verbatim within a token budget, leave the tail untouched. Gated - # on THIS request's eligibility, not merely on a checkpoint being present. + # Native server-side compaction renders nothing placed before a compaction + # item, so pre-checkpoint history is dead upload weight and the user's + # plaintext asks / merged local summaries silently vanish. Keep the newest + # checkpoint first, retain pre-checkpoint USER and compression-SUMMARY + # messages within a token budget, leave the tail untouched. if not native_compaction_eligible: return items @@ -636,8 +606,7 @@ def _chat_messages_to_responses_input( class ResponsesRouteFlags(NamedTuple): """Which special Responses-API route an agent is talking to. - Single owner of the codex/xai/github predicates: every site (request - kwargs, preflight estimation, silent-reject hints) must call + Single owner of the codex/xai/github predicates: every site must call :func:`classify_responses_route` — inline string comparisons drift. """ @@ -656,18 +625,14 @@ def classify_responses_route(agent: Any) -> ResponsesRouteFlags: provider = getattr(agent, "provider", None) base_url = str(getattr(agent, "base_url", "") or "") - hostname = str(getattr(agent, "_base_url_hostname", "") or "").lower() - if not hostname: - hostname = base_url_hostname(base_url) + hostname = str(getattr(agent, "_base_url_hostname", "") or "").lower() or base_url_hostname(base_url) lower = str(getattr(agent, "_base_url_lower", "") or base_url).lower() def _host_is(domain: str) -> bool: return hostname == domain or hostname.endswith("." + domain) return ResponsesRouteFlags( - is_codex_backend=( - provider == "openai-codex" or (_host_is("chatgpt.com") and "/backend-api/codex" in lower) - ), + is_codex_backend=provider == "openai-codex" or (_host_is("chatgpt.com") and "/backend-api/codex" in lower), is_xai_responses=provider in {"xai", "xai-oauth"} or hostname == "api.x.ai", is_github_responses=_host_is("models.github.ai") or _host_is("githubcopilot.com"), ) @@ -682,11 +647,10 @@ def estimate_native_responses_preflight_tokens( ) -> Optional[int]: """Estimate tokens for the checkpoint-pruned Responses payload. - Counting the full durable transcript overstates a natively compacted - session several times over and fires local compression against history - the request will never send. Returns None when native compaction is not - proven eligible or conversion fails — caller falls back to the generic - (conservative) estimate. + Counting the full durable transcript overstates a natively compacted session + several times over and fires local compression against history the request + will never send. None when native compaction is not proven eligible or + conversion fails — caller falls back to the generic (conservative) estimate. """ if getattr(agent, "api_mode", None) != "codex_responses" or not isinstance(messages, list): return None @@ -714,7 +678,6 @@ def estimate_native_responses_preflight_tokens( except Exception: logger.debug("native Responses preflight conversion failed; falling back to generic estimate", exc_info=True) return None - if not isinstance(items, list): return None @@ -732,27 +695,29 @@ class _PreflightCtx(NamedTuple): seen_ids: set -def _preflight_function_call(item: Dict[str, Any], idx: int, ctx: _PreflightCtx) -> Dict[str, Any]: +def _require_call_id(item: Dict[str, Any], idx: int, kind: str) -> str: call_id = item.get("call_id") - name = item.get("name") if not _nonblank(call_id): - raise ValueError(f"Codex Responses input[{idx}] function_call is missing call_id.") + raise ValueError(f"Codex Responses input[{idx}] {kind} is missing call_id.") + return call_id.strip() + + +def _preflight_function_call(item: Dict[str, Any], idx: int, ctx: _PreflightCtx) -> Dict[str, Any]: + call_id = _require_call_id(item, idx, "function_call") + name = item.get("name") if not _nonblank(name): raise ValueError(f"Codex Responses input[{idx}] function_call is missing name.") return { - "type": "function_call", "call_id": call_id.strip(), "name": _sanitize_replayed_fn_name(name), + "type": "function_call", "call_id": call_id, "name": _sanitize_replayed_fn_name(name), "arguments": ctx.sanitize_text(_coerce_arguments(item.get("arguments", "{}"))), } def _preflight_function_call_output(item: Dict[str, Any], idx: int, ctx: _PreflightCtx) -> Dict[str, Any]: - call_id = item.get("call_id") - if not _nonblank(call_id): - raise ValueError(f"Codex Responses input[{idx}] function_call_output is missing call_id.") + call_id = _require_call_id(item, idx, "function_call_output") output = item.get("output", "") if isinstance(output, list): - # Multimodal tool result: keep recognised input_text/input_image - # parts, drop anything else to avoid a 4xx. + # Multimodal tool result: keep recognised input_text/input_image parts, drop the rest (4xx otherwise). cleaned: List[Dict[str, Any]] = [] for part in output: ptype = part.get("type") if isinstance(part, dict) else None @@ -763,15 +728,14 @@ def _preflight_function_call_output(item: Dict[str, Any], idx: int, ctx: _Prefli output_value: Any = cleaned or "" else: output_value = ctx.sanitize_text(_str_or_empty(output)) - return {"type": "function_call_output", "call_id": call_id.strip(), "output": output_value} + return {"type": "function_call_output", "call_id": call_id, "output": output_value} def _preflight_reasoning(item: Dict[str, Any], idx: int, ctx: _PreflightCtx) -> Optional[Dict[str, Any]]: encrypted = item.get("encrypted_content") if not _nonempty_str(encrypted): return None - # ``id`` is used only for local dedup and NOT forwarded: with store=False - # the API resolves ids server-side and 404s. + # ``id`` is used only for local dedup and NOT forwarded (store=False → server-side 404). item_id = item.get("id") if _nonempty_str(item_id): if item_id in ctx.seen_ids: @@ -803,12 +767,11 @@ def _preflight_message(item: Dict[str, Any], idx: int, ctx: _PreflightCtx) -> Di if not isinstance(part, dict): raise ValueError(f"Codex Responses input[{idx}] message content[{part_idx}] must be an object.") part_type = part.get("type") - if part_type not in {"output_text", "text"}: + if part_type not in _OUTPUT_TEXT_TYPES: raise ValueError( f"Codex Responses input[{idx}] message content[{part_idx}] has unsupported type {part_type!r}." ) - text = _str_or_empty(part.get("text", "")) - normalized_content.append({"type": "output_text", "text": ctx.sanitize_text(text)}) + normalized_content.append({"type": "output_text", "text": ctx.sanitize_text(_str_or_empty(part.get("text", "")))}) if not normalized_content: raise ValueError(f"Codex Responses input[{idx}] message item must contain at least one text part.") return _assistant_message_item(item, normalized_content, is_github_responses=ctx.is_github_responses) @@ -819,10 +782,9 @@ def _preflight_role_message(item: Dict[str, Any], idx: int, role: str, ctx: _Pre if not isinstance(content, list): return {"role": role, "content": ctx.sanitize_text(_str_or_empty(content))} - # Parts are already Responses-shaped; validate and re-type text for the - # role (``output_text`` for assistant, ``input_text`` for user). Unlike - # history conversion, empty text / empty image urls are kept, not dropped. - text_type = "output_text" if role == "assistant" else "input_text" + # Parts are already Responses-shaped; validate and re-type text for the role. + # Unlike history conversion, empty text / empty image urls are kept, not dropped. + text_type = _text_type_for(role) validated: List[Dict[str, Any]] = [] for part_idx, part in enumerate(content): if isinstance(part, str): @@ -831,11 +793,10 @@ def _preflight_role_message(item: Dict[str, Any], idx: int, role: str, ctx: _Pre continue if not isinstance(part, dict): raise ValueError(f"Codex Responses input[{idx}].content[{part_idx}] must be an object or string.") - ptype = str(part.get("type") or "").strip().lower() + ptype = _part_type(part) if ptype in _TEXT_PART_TYPES: text = part.get("text", "") - text = text if isinstance(text, str) else str(text or "") - validated.append({"type": text_type, "text": ctx.sanitize_text(text)}) + validated.append({"type": text_type, "text": ctx.sanitize_text(text if isinstance(text, str) else str(text or ""))}) elif ptype in _IMAGE_PART_TYPES: if role == "assistant": # Same output-message invariant as normal history replay. @@ -875,7 +836,6 @@ def _preflight_codex_input_items( for idx, item in enumerate(raw_items): if not isinstance(item, dict): raise ValueError(f"Codex Responses input[{idx}] must be an object.") - item_type = item.get("type") handler = _PREFLIGHT_ITEM_HANDLERS.get(item_type) if isinstance(item_type, str) else None if handler is not None: @@ -890,7 +850,6 @@ def _preflight_codex_input_items( normalized_item = _preflight_role_message(item, idx, role, ctx) if normalized_item is not None: normalized.append(normalized_item) - return normalized @@ -898,13 +857,10 @@ def _preflight_tool(tool: Any, idx: int) -> Dict[str, Any]: if not isinstance(tool, dict): raise ValueError(f"Codex Responses tools[{idx}] must be an object.") tool_type = tool.get("type") - # Provider-executed built-ins carry no name/parameters; pass them through - # verbatim rather than rejecting them below. - if tool_type in _RESPONSES_BUILTIN_TOOL_TYPES: + if tool_type in _RESPONSES_BUILTIN_TOOL_TYPES: # provider-executed built-ins carry no name/parameters return dict(tool) if tool_type != "function": raise ValueError(f"Codex Responses tools[{idx}] has unsupported type {tool.get('type')!r}.") - name = tool.get("name") parameters = tool.get("parameters") if not _nonblank(name): @@ -931,8 +887,7 @@ _PREFLIGHT_OPTIONAL_FIELDS: tuple[tuple[str, Callable[[Any], bool], Optional[Cal ("parallel_tool_calls", lambda v: v is not None, None), ("prompt_cache_key", lambda v: v is not None, None), ("prompt_cache_retention", lambda v: v is not None, None), - # Native compaction directive; eligibility is resolved upstream in - # agent/native_compaction.py — preflight only preserves the shape. + # Native compaction directive; eligibility is resolved in agent/native_compaction.py. ("context_management", lambda v: isinstance(v, list) and bool(v), None), ) @@ -1006,9 +961,8 @@ def _preflight_codex_api_kwargs( if extra_body is not None: if not isinstance(extra_body, dict): raise ValueError("Codex Responses request 'extra_body' must be an object.") - # Verbatim: xAI carries ``prompt_cache_key`` as a body-level field. The - # SDK serializes extra_body without per-field checks, so it survives - # Responses.stream() kwarg-signature changes. + # Verbatim: xAI carries ``prompt_cache_key`` as a body-level field, and + # the SDK serializes extra_body without per-field checks. if extra_body: normalized["extra_body"] = dict(extra_body) @@ -1024,9 +978,7 @@ def _preflight_codex_api_kwargs( raise ValueError("Codex Responses stream flag is only allowed in fallback streaming requests.") # Defense-in-depth slash-enum strip for xAI (rejects ``Qwen/Qwen3.5`` style - # enum values with "Invalid arguments passed to the model"). Gated on the - # model name because native Codex accepts slashes — stripping there would - # silently degrade tool-schema constraints. + # enum values). Gated on the model name because native Codex accepts slashes. is_xai_model = str(api_kwargs.get("model") or "").lower().startswith(("grok-", "x-ai/grok-")) if is_xai_model and normalized.get("tools"): try: @@ -1038,7 +990,6 @@ def _preflight_codex_api_kwargs( unexpected = sorted(key for key in api_kwargs if key not in allowed_keys) if unexpected: raise ValueError(f"Codex Responses request has unsupported field(s): {', '.join(unexpected)}.") - return normalized @@ -1058,23 +1009,19 @@ def _text_chunks(parts: Any, types: Optional[set] = None) -> List[str]: def _extract_responses_message_text(item: Any) -> str: """Extract assistant text from a Responses message output item.""" - return "".join(_text_chunks(getattr(item, "content", None), {"output_text", "text"})).strip() + return "".join(_text_chunks(getattr(item, "content", None), _OUTPUT_TEXT_TYPES)).strip() def _extract_responses_reasoning_text(item: Any) -> str: - """Extract a compact reasoning text from a Responses reasoning item (summary, else ``text``).""" + """Compact reasoning text from a Responses reasoning item (summary, else ``text``).""" chunks = _text_chunks(getattr(item, "summary", None)) text = getattr(item, "text", None) return "\n".join(chunks).strip() if chunks else (text.strip() if isinstance(text, str) else "") def _format_responses_error(error_obj: Any, response_status: str) -> str: - """Human-readable string for a ``response.error`` payload (dict or object). - - Prefers ``": "`` so failure modes (rate limit vs context - length vs overloaded) are distinguishable; falls back to whichever is - present, then ``str(error_obj)``, then a status-based default. - """ + """Human-readable ``": "`` for a ``response.error`` payload (dict or object), + falling back to whichever is present, then ``str(error_obj)``, then a status-based default.""" def field(name: str) -> str: value = _field(error_obj, name) return str(value).strip() if isinstance(value, str) or value else "" @@ -1112,10 +1059,7 @@ def _response_tool_call(item: Any, item_type: str, index: int) -> SimpleNamespac def _stamped_encrypted_item(item: Any, item_type: str, issuer_kind: Optional[str]) -> Optional[Dict[str, Any]]: """``{type, encrypted_content[, _issuer_kind]}`` for replay, or None without a blob. - - ``_issuer_kind`` is stamped so a later model swap can detect an endpoint - that cannot decrypt the blob. - """ + ``_issuer_kind`` lets a later model swap detect an endpoint that cannot decrypt the blob.""" encrypted = getattr(item, "encrypted_content", None) if not _nonempty_str(encrypted): return None @@ -1147,6 +1091,74 @@ def _capture_reasoning_item(item: Any, issuer_kind: Optional[str]) -> Optional[D return raw_item +class _OutputScan: + """Accumulated view of one Responses ``output`` list (phase 1 of normalization).""" + + def __init__(self, response_status: Optional[str]) -> None: + self.content_parts: List[str] = [] + self.reasoning_parts: List[str] = [] + self.reasoning_items_raw: List[Dict[str, Any]] = [] + self.message_items_raw: List[Dict[str, Any]] = [] + self.tool_calls: List[Any] = [] + self.has_incomplete_items = response_status in _INCOMPLETE_STATUSES + self.saw_streaming_or_item_incomplete = response_status in {"queued", "in_progress"} + self.saw_commentary_phase = False + self.saw_final_answer_phase = False + self.saw_reasoning_item = False + + def scan(self, output: List[Any], issuer_kind: Optional[str]) -> None: + for item in output: + item_type = getattr(item, "type", None) + item_status = _lower_or_none(getattr(item, "status", None)) + if item_status in _INCOMPLETE_STATUSES and item_type not in _SERVER_SIDE_TOOL_CALL_TYPES: + self.has_incomplete_items = True + self.saw_streaming_or_item_incomplete = True + + if item_type == "message": + self._message(item, item_status) + elif item_type == "reasoning": + self.saw_reasoning_item = True + reasoning_text = _extract_responses_reasoning_text(item) + if reasoning_text: + self.reasoning_parts.append(reasoning_text) + raw_item = _capture_reasoning_item(item, issuer_kind) + if raw_item is not None: + self.reasoning_items_raw.append(raw_item) + elif item_type == "compaction": + # Native compaction checkpoint rides the codex_reasoning_items sidecar + # so it inherits persistence, replay, the cross-issuer guard and the kill switch. + raw_item = _stamped_encrypted_item(item, "compaction", issuer_kind) + if raw_item is not None: + self.reasoning_items_raw.append(raw_item) + logger.info( + "Native Responses compaction item captured (%d chars encrypted).", + len(raw_item["encrypted_content"]), + ) + elif item_type in {"function_call", "custom_tool_call"}: + if item_type == "function_call" and item_status in _INCOMPLETE_STATUSES: + continue + self.tool_calls.append(_response_tool_call(item, item_type, len(self.tool_calls))) + + def _message(self, item: Any, item_status: Optional[str]) -> None: + normalized_phase = _lower_or_none(getattr(item, "phase", None)) + is_commentary_phase = normalized_phase in {"commentary", "analysis"} + self.saw_commentary_phase = self.saw_commentary_phase or is_commentary_phase + self.saw_final_answer_phase = self.saw_final_answer_phase or normalized_phase in {"final_answer", "final"} + message_text = _extract_responses_message_text(item) + if not message_text: + return + # commentary/analysis text is mid-turn narration, never the final answer: + # keep it out of content but surface it via the reasoning channel. The + # exact item is still preserved for replay/cache continuity. + (self.reasoning_parts if is_commentary_phase else self.content_parts).append(message_text) + item_id = getattr(item, "id", None) + self.message_items_raw.append(_message_item( + [{"type": "output_text", "text": message_text}], + status=_normalize_responses_message_status(item_status), + item_id=item_id if isinstance(item_id, str) else None, phase=normalized_phase, + )) + + def _normalize_codex_response(response: Any, *, issuer_kind: Optional[str] = None) -> tuple[Any, str]: """Normalize a Responses API object to an assistant_message-like object. @@ -1171,9 +1183,8 @@ def _normalize_codex_response(response: Any, *, issuer_kind: Optional[str] = Non ) output = [_synthetic_message([SimpleNamespace(type="output_text", text=out_text.strip())])] elif response_incomplete_content_filter: - # Deterministic provider safety block, not a partial answer: - # synthesize an empty message so finish_reason becomes - # content_filter instead of burning continuation attempts. + # Deterministic provider safety block, not a partial answer: an empty + # message makes finish_reason content_filter instead of burning continuations. output = [_synthetic_message([])] else: raise RuntimeError("Responses API returned no output items") @@ -1182,79 +1193,21 @@ def _normalize_codex_response(response: Any, *, issuer_kind: Optional[str] = Non if response_status in {"failed", "cancelled"}: raise RuntimeError(_format_responses_error(getattr(response, "error", None), response_status)) - content_parts: List[str] = [] - reasoning_parts: List[str] = [] - reasoning_items_raw: List[Dict[str, Any]] = [] - message_items_raw: List[Dict[str, Any]] = [] - tool_calls: List[Any] = [] - has_incomplete_items = response_status in _INCOMPLETE_STATUSES - saw_streaming_or_item_incomplete = response_status in {"queued", "in_progress"} - saw_commentary_phase = False - saw_final_answer_phase = False - saw_reasoning_item = False + scan = _OutputScan(response_status) + scan.scan(output, issuer_kind) + tool_calls, reasoning_parts = scan.tool_calls, scan.reasoning_parts - for item in output: - item_type = getattr(item, "type", None) - item_status = _lower_or_none(getattr(item, "status", None)) - - if item_status in _INCOMPLETE_STATUSES and item_type not in _SERVER_SIDE_TOOL_CALL_TYPES: - has_incomplete_items = True - saw_streaming_or_item_incomplete = True - - if item_type == "message": - normalized_phase = _lower_or_none(getattr(item, "phase", None)) - is_commentary_phase = normalized_phase in {"commentary", "analysis"} - saw_commentary_phase = saw_commentary_phase or is_commentary_phase - saw_final_answer_phase = saw_final_answer_phase or normalized_phase in {"final_answer", "final"} - message_text = _extract_responses_message_text(item) - if message_text: - # commentary/analysis phase text is mid-turn narration, never - # the final answer: keep it out of content (so it cannot leak - # into the response) but surface it via the reasoning channel. - # The exact item is still preserved for replay/cache continuity. - (reasoning_parts if is_commentary_phase else content_parts).append(message_text) - item_id = getattr(item, "id", None) - message_items_raw.append(_message_item( - [{"type": "output_text", "text": message_text}], - status=_normalize_responses_message_status(item_status), - item_id=item_id if isinstance(item_id, str) else None, phase=normalized_phase, - )) - elif item_type == "reasoning": - saw_reasoning_item = True - reasoning_text = _extract_responses_reasoning_text(item) - if reasoning_text: - reasoning_parts.append(reasoning_text) - raw_item = _capture_reasoning_item(item, issuer_kind) - if raw_item is not None: - reasoning_items_raw.append(raw_item) - elif item_type == "compaction": - # Native compaction checkpoint: rides the codex_reasoning_items - # sidecar so it inherits persistence, replay, the cross-issuer - # guard and the kill switch without new state. - raw_item = _stamped_encrypted_item(item, "compaction", issuer_kind) - if raw_item is not None: - reasoning_items_raw.append(raw_item) - logger.info( - "Native Responses compaction item captured (%d chars encrypted).", - len(raw_item["encrypted_content"]), - ) - elif item_type in {"function_call", "custom_tool_call"}: - if item_type == "function_call" and item_status in _INCOMPLETE_STATUSES: - continue - tool_calls.append(_response_tool_call(item, item_type, len(tool_calls))) - - final_text = "\n".join(content_parts).strip() - if not final_text and hasattr(response, "output_text") and (saw_final_answer_phase or not saw_commentary_phase): + final_text = "\n".join(scan.content_parts).strip() + if not final_text and hasattr(response, "output_text") and (scan.saw_final_answer_phase or not scan.saw_commentary_phase): out_text = getattr(response, "output_text", "") if isinstance(out_text, str): final_text = out_text.strip() # Tool-call leak recovery: gpt-5.x sometimes emits the intended - # ``function_call`` as plain Harmony text (``to=functions.foo {json}``) - # with no structured item, so ``tool_calls`` is empty and no tool ran. - # Treat as incomplete so the continuation path re-elicits a real call; - # clear the text so the garbage is not surfaced as a summary (encrypted - # reasoning is preserved for the retry). + # ``function_call`` as plain Harmony text (``to=functions.foo {json}``) with + # no structured item. Treat as incomplete so the continuation path + # re-elicits a real call; clear the text so the garbage is not surfaced + # (encrypted reasoning is preserved for the retry). leaked_tool_call_text = bool(final_text and not tool_calls and _TOOL_CALL_LEAK_PATTERN.search(final_text)) if leaked_tool_call_text: logger.warning( @@ -1266,10 +1219,9 @@ def _normalize_codex_response(response: Any, *, issuer_kind: Optional[str] = Non # Reasoning-channel answer salvage (xAI grok): grok-4.x sometimes puts the # final answer inside the reasoning item after its ```` delimiter. - # Without salvage the reasoning-only rule marks the turn incomplete, and - # since these items carry no encrypted_content every continuation request - # is byte-identical to the failed one. Promote the delimited tail to - # content and keep the untagged prefix as thinking text. + # Without salvage the reasoning-only rule marks the turn incomplete, and since + # these items carry no encrypted_content every continuation request is + # byte-identical to the failed one. Promote the delimited tail to content. if issuer_kind == "xai_responses" and not final_text and not tool_calls and reasoning_parts: joined_reasoning = "\n\n".join(reasoning_parts) marker = joined_reasoning.rfind("") @@ -1290,8 +1242,8 @@ def _normalize_codex_response(response: Any, *, issuer_kind: Optional[str] = Non reasoning="\n\n".join(reasoning_parts).strip() if reasoning_parts else None, reasoning_content=None, reasoning_details=None, - codex_reasoning_items=reasoning_items_raw or None, - codex_message_items=message_items_raw or None, + codex_reasoning_items=scan.reasoning_items_raw or None, + codex_message_items=scan.message_items_raw or None, ) if tool_calls: @@ -1300,16 +1252,15 @@ def _normalize_codex_response(response: Any, *, issuer_kind: Optional[str] = Non finish_reason = "content_filter" elif ( leaked_tool_call_text - or saw_streaming_or_item_incomplete - or ((has_incomplete_items or saw_commentary_phase) and not saw_final_answer_phase) + or scan.saw_streaming_or_item_incomplete + or ((scan.has_incomplete_items or scan.saw_commentary_phase) and not scan.saw_final_answer_phase) ): finish_reason = "incomplete" - elif (reasoning_items_raw or reasoning_parts or saw_reasoning_item) and not final_text: + elif (scan.reasoning_items_raw or reasoning_parts or scan.saw_reasoning_item) and not final_text: # Reasoning-only response. For Codex/xAI/GitHub, reasoning-only with - # status=completed means "still thinking, needs another turn" → - # incomplete so the continuation path retries. Other backends: trust - # response.status — forcing incomplete there stalls for minutes - # (3 retries × up to 240s) on a legitimately final state. + # status=completed means "still thinking, needs another turn" → incomplete + # so the continuation path retries. Other backends: trust response.status — + # forcing incomplete there stalls for minutes on a legitimately final state. trusted_final = ( response_status == "completed" and issuer_kind not in ("codex_backend", "xai_responses", "github_responses") ) From 4f5d9da6dabb10c23ed30288fc97cc506e7253be Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Wed, 2 Sep 2026 18:30:17 -0700 Subject: [PATCH 03/13] refactor(agent/codex_runtime): fold agent-callback guard, dedupe change filtering, extract stream close; collapse signatures (-19 LOC) --- agent/codex_headers.py | 8 +-- agent/codex_runtime.py | 133 ++++++++++++++++++----------------------- 2 files changed, 59 insertions(+), 82 deletions(-) diff --git a/agent/codex_headers.py b/agent/codex_headers.py index b7e72f8ff1..bec0c8fb0e 100644 --- a/agent/codex_headers.py +++ b/agent/codex_headers.py @@ -31,9 +31,7 @@ def is_official_codex_base_url(base_url: str) -> bool: return False -def codex_cloudflare_headers( - access_token: str, *, base_url: str = CODEX_AUX_BASE_URL, -) -> Dict[str, str]: +def codex_cloudflare_headers(access_token: str, *, base_url: str = CODEX_AUX_BASE_URL) -> Dict[str, str]: """Identity and account headers for chatgpt.com/backend-api/codex. OpenAI requires third-party harnesses to identify themselves: the official @@ -65,9 +63,7 @@ def codex_cloudflare_headers( return headers -def apply_required_codex_headers( - client_kwargs: Dict[str, Any], *, access_token: str, base_url: str, -) -> None: +def apply_required_codex_headers(client_kwargs: Dict[str, Any], *, access_token: str, base_url: str) -> None: """Keep required Codex identity after user/provider header overrides.""" if not is_official_codex_base_url(base_url): return diff --git a/agent/codex_runtime.py b/agent/codex_runtime.py index ba8cea2426..0b381c3af9 100644 --- a/agent/codex_runtime.py +++ b/agent/codex_runtime.py @@ -1,9 +1,9 @@ """Codex API runtime — App Server and Responses-API streaming paths. Extracted from :class:`AIAgent`; every entry point takes the parent agent as its -first argument and AIAgent keeps thin forwarders. ``run_codex_app_server_turn`` -drives one ``codex app-server`` subprocess turn (``codex_app_server`` api_mode); -``run_codex_stream`` runs one streaming Codex Responses call (``codex_responses``); +first argument. ``run_codex_app_server_turn`` drives one ``codex app-server`` +subprocess turn (``codex_app_server`` api_mode); ``run_codex_stream`` runs one +streaming Codex Responses call (``codex_responses``); ``run_codex_create_stream_fallback`` is a legacy alias of the latter. """ @@ -109,8 +109,8 @@ def _record_codex_app_server_usage(agent, turn) -> dict[str, Any]: compressor = getattr(agent, "context_compressor", None) if not isinstance(usage, dict) or not usage: if compressor is not None and getattr(compressor, "awaiting_real_usage_after_compression", False): - # No usage cannot adjudicate the pending compaction; consume the marker so - # preflight deferral cannot stay latched. + # No usage cannot adjudicate the pending compaction; consume the marker + # so preflight deferral cannot stay latched. compressor.update_from_response({}) _queue_token_counts( agent, "Codex app-server api-call persistence failed (session=%s): %s", @@ -171,17 +171,10 @@ def _record_codex_app_server_usage(agent, turn) -> dict[str, Any]: model=agent.model, api_call_count=1, ), ) - return {**usage_dict, "last_prompt_tokens": prompt_tokens, **cost_fields} -def _record_codex_app_server_compaction( - agent, - turn, - *, - approx_tokens: int | None = None, - force: bool = False, -) -> bool: +def _record_codex_app_server_compaction(agent, turn, *, approx_tokens: int | None = None, force: bool = False) -> bool: """Record a Codex-native compaction boundary in Hermes state. The app-server owns the compacted thread, so local transcript rows are NOT @@ -263,6 +256,10 @@ _MCP_LIKE_ITEM_TYPES = {"mcpToolCall", "dynamicToolCall"} _PREVIEW_FIELDS = {"commandExecution": "command", "webSearch": "query"} +def _item_changes(item: dict) -> list[dict]: + return [c for c in (item.get("changes") or []) if isinstance(c, dict)] + + def _codex_item_to_tool_name(item: dict) -> str: """Synthetic Hermes tool name for a codex item (mirrors CodexEventProjector).""" item_type = item.get("type") or "" @@ -281,8 +278,7 @@ def _codex_item_to_args(item: dict) -> dict: return {"command": item.get("command") or "", "cwd": item.get("cwd") or ""} if item_type == "fileChange": return {"changes": [ - {"kind": (c.get("kind") or {}).get("type") or "update", "path": c.get("path") or ""} - for c in (item.get("changes") or []) if isinstance(c, dict) + {"kind": (c.get("kind") or {}).get("type") or "update", "path": c.get("path") or ""} for c in _item_changes(item) ]} if item_type in _MCP_LIKE_ITEM_TYPES: args = item.get("arguments") or {} @@ -298,7 +294,7 @@ def _codex_item_to_preview(item: dict) -> Any: if item_type in _PREVIEW_FIELDS: return (item.get(_PREVIEW_FIELDS[item_type]) or "")[:120] or None if item_type == "fileChange": - paths = [c.get("path") for c in (item.get("changes") or []) if isinstance(c, dict) and c.get("path")] + paths = [c.get("path") for c in _item_changes(item) if c.get("path")] if not paths: return None return ", ".join(paths[:3]) + (f", +{len(paths) - 3} more" if len(paths) > 3 else "") @@ -320,9 +316,7 @@ def _codex_item_completion_payload(item: dict) -> tuple[str, bool]: out = item.get("aggregatedOutput") or "" exit_code = item.get("exitCode") is_error = bool(exit_code is not None and exit_code != 0) - if is_error: - out = f"[exit {exit_code}]\n{out}" - return out, is_error + return (f"[exit {exit_code}]\n{out}" if is_error else out), is_error if item_type == "fileChange": status = item.get("status") or "unknown" n = len(item.get("changes") or []) @@ -371,18 +365,20 @@ def make_codex_app_server_event_bridge(agent) -> Callable[[dict], None]: # item_id -> (tool_name, args, started_monotonic); duration even when codex omits durationMs. started: dict[str, tuple[str, dict, float]] = {} + def agent_cb(attr: str, fail_msg: str, *fail_args: Any, args: tuple = (), kwargs: dict | None = None) -> None: + _call_guarded(getattr(agent, attr, None), fail_msg, *fail_args, args=args, kwargs=kwargs) + def _fire_tool_started(item: dict) -> None: item_id = item.get("id") or "" name = _codex_item_to_tool_name(item) args = _codex_item_to_args(item) if item_id: started[item_id] = (name, args, time.monotonic()) - _call_guarded(getattr(agent, "tool_progress_callback", None), - "tool_progress_callback raised on tool.started for %s", name, - args=("tool.started", name, _codex_item_to_preview(item), args)) + agent_cb("tool_progress_callback", "tool_progress_callback raised on tool.started for %s", name, + args=("tool.started", name, _codex_item_to_preview(item), args)) # Stable-ID tool card (TUI/desktop) fires alongside the progress bubble. - _call_guarded(getattr(agent, "tool_start_callback", None), "tool_start_callback raised for %s", name, - args=(_stable_call_id(item, name), name, args)) + agent_cb("tool_start_callback", "tool_start_callback raised for %s", name, + args=(_stable_call_id(item, name), name, args)) def _fire_tool_completed(item: dict) -> None: item_id = item.get("id") or "" @@ -397,30 +393,25 @@ def make_codex_app_server_event_bridge(agent) -> Callable[[dict], None]: elif prior is not None: duration = time.monotonic() - prior[2] result, is_error = _codex_item_completion_payload(item) - _call_guarded(getattr(agent, "tool_progress_callback", None), - "tool_progress_callback raised on tool.completed for %s", name, - args=("tool.completed", name, None, None), - kwargs={"duration": duration, "is_error": is_error, "result": result}) + agent_cb("tool_progress_callback", "tool_progress_callback raised on tool.completed for %s", name, + args=("tool.completed", name, None, None), + kwargs={"duration": duration, "is_error": is_error, "result": result}) args = prior[1] if prior is not None else _codex_item_to_args(item) - _call_guarded(getattr(agent, "tool_complete_callback", None), "tool_complete_callback raised for %s", name, - args=(_stable_call_id(item, name), name, args, result)) + agent_cb("tool_complete_callback", "tool_complete_callback raised for %s", name, + args=(_stable_call_id(item, name), name, args, result)) def _fire_delta(params: dict, attr: str) -> None: text = params.get("delta") or params.get("text") or "" if isinstance(text, str) and text: - _call_guarded(getattr(agent, attr, None), f"{attr} raised", args=(text,)) + agent_cb(attr, f"{attr} raised", args=(text,)) def _fire_agent_message_completed(item: dict) -> None: text = item.get("text") or "" - if not isinstance(text, str) or not text.strip(): - return - # display.show_commentary=false keeps mid-turn narration off the - # interim path here too (same contract as codex_responses commentary). - if not getattr(agent, "show_commentary", True): - return - _call_guarded(getattr(agent, "_emit_interim_assistant_message", None), - "_emit_interim_assistant_message raised", - args=({"role": "assistant", "content": text},)) + # display.show_commentary=false keeps mid-turn narration off the interim + # path here too (same contract as codex_responses commentary). + if isinstance(text, str) and text.strip() and getattr(agent, "show_commentary", True): + agent_cb("_emit_interim_assistant_message", "_emit_interim_assistant_message raised", + args=({"role": "assistant", "content": text},)) def _on_item(params: dict, completed: bool) -> None: item = params.get("item") @@ -561,10 +552,8 @@ def _finish_codex_turn( if not turn.interrupted and turn.error is None: try: agent._sync_external_memory_for_turn( - original_user_message=original_user_message, - final_response=turn.final_text, - interrupted=False, - messages=messages, + original_user_message=original_user_message, final_response=turn.final_text, + interrupted=False, messages=messages, ) except Exception: logger.debug("external memory sync raised", exc_info=True) @@ -573,9 +562,7 @@ def _finish_codex_turn( if turn.final_text and not turn.interrupted and (should_review_memory or should_review_skills): try: agent._spawn_background_review( - messages_snapshot=list(messages), - review_memory=should_review_memory, - review_skills=should_review_skills, + messages_snapshot=list(messages), review_memory=should_review_memory, review_skills=should_review_skills, ) except Exception: logger.debug("background review spawn raised", exc_info=True) @@ -638,8 +625,7 @@ def run_codex_app_server_turn( interrupt, messages, api_calls=1, completed=not turn.interrupted and turn.error is None, error=turn.error, final_response=turn.final_text, # We flushed the projected rows ourselves (see _persist_projected_messages); - # True makes the gateway skip its own DB write, which would duplicate - # the already-flushed user turn. + # True makes the gateway skip its own DB write, which would duplicate the user turn. agent_persisted=True, codex_thread_id=turn.thread_id, codex_turn_id=turn.turn_id, @@ -768,12 +754,9 @@ class _CodexResponseAssembler: def _on_item_added(self, event: Any, event_type: str) -> None: item = _event_field(event, "item") item_type = _event_field(item, "type", "") - if item_type == "message": - self.active_message_phase = _message_phase(item) - if self.active_message_phase == "commentary": - self.commentary_text_deltas = [] - else: - self.active_message_phase = None + self.active_message_phase = _message_phase(item) if item_type == "message" else None + if self.active_message_phase == "commentary": + self.commentary_text_deltas = [] # Record first-observed ordering for EVERY announced item; the .done path must # reuse it, or a mixed announced/pending stream without output_index values reorders the calls. item_id = str(_event_field(item, "id", "")) @@ -861,8 +844,7 @@ class _CodexResponseAssembler: if isinstance(content_parts, list): commentary_text = "".join( str(_event_field(part, "text", "") or "") - for part in content_parts - if _event_field(part, "type", "") == "output_text" + for part in content_parts if _event_field(part, "type", "") == "output_text" ).strip() if commentary_text: self._safe(self.on_commentary_message, "on_commentary_message", commentary_text) @@ -940,9 +922,7 @@ class _CodexResponseAssembler: output: List[Any] = list(self.output_items) if not output and self.text_deltas and not self.has_tool_calls: output = [SimpleNamespace( - type="message", - role="assistant", - status="completed", + type="message", role="assistant", status="completed", content=[SimpleNamespace(type="output_text", text="".join(self.text_deltas))], )] @@ -1077,8 +1057,7 @@ def _bypass_sdk_request_transform(stream_kwargs: dict) -> dict: moved = { field: stream_kwargs[field] for field in _SDK_TRANSFORM_BYPASS_FIELDS - if isinstance(stream_kwargs.get(field), (dict, list)) - and _is_plain_json_data(stream_kwargs[field]) + if isinstance(stream_kwargs.get(field), (dict, list)) and _is_plain_json_data(stream_kwargs[field]) } if not moved: return stream_kwargs @@ -1184,6 +1163,21 @@ def run_codex_stream(agent, api_kwargs: dict, client: Any = None, on_first_delta agent._client_log_context(), exc, ) + def _close_event_stream(event_stream: Any) -> None: + close_fn = getattr(event_stream, "close", None) # None while connect never succeeded + if not callable(close_fn): + return + try: + close_fn() + except Exception: + # A failed close can leave this response's connection checked out of + # the httpx pool while the caller's finally reports a reuse-reason + # close — caching a client with a leaked connection. Poison the slot + # so close really closes the pool. ``client is None`` is the shared + # primary client, which is never reuse-cached and must not be force-shut. + if client is not None: + agent._abort_request_openai_client(active_client, reason="codex_stream_close_failed") + on_commentary_message = ( _fenced(lambda text: agent._fire_streamed_codex_commentary(text)) if getattr(agent, "interim_assistant_callback", None) is not None and getattr(agent, "show_commentary", True) @@ -1266,22 +1260,9 @@ def run_codex_stream(agent, api_kwargs: dict, client: Any = None, on_first_delta sum(len(p) for p in agent._codex_streamed_text_parts), agent._client_log_context(), ) - return final finally: - close_fn = getattr(event_stream, "close", None) # None while connect never succeeded - if callable(close_fn): - try: - close_fn() - except Exception: - # A failed close can leave this response's connection checked - # out of the httpx pool while the caller's finally reports a - # reuse-reason close — caching a client with a leaked - # connection. Poison the slot so close really closes the pool. - # ``client is None`` is the shared primary client, which is - # never reuse-cached and must not be force-shut here. - if client is not None: - agent._abort_request_openai_client(active_client, reason="codex_stream_close_failed") + _close_event_stream(event_stream) def run_codex_create_stream_fallback(agent, api_kwargs: dict, client: Any = None): From deb5bdecd9b5e36c94f9e9a8afb37b0452a3d902 Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Wed, 2 Sep 2026 18:35:02 -0700 Subject: [PATCH 04/13] refactor(agent/bedrock_adapter): table-ize stale-error imports and context lengths, share discovery entry builder (-24 LOC) --- agent/bedrock_adapter.py | 126 ++++++++++++++++----------------------- 1 file changed, 52 insertions(+), 74 deletions(-) diff --git a/agent/bedrock_adapter.py b/agent/bedrock_adapter.py index aab7aafd11..74292e4ccf 100644 --- a/agent/bedrock_adapter.py +++ b/agent/bedrock_adapter.py @@ -214,17 +214,18 @@ _STALE_LIB_MODULE_PREFIXES = ("urllib3.", "botocore.", "boto3.") def _stale_error_types() -> tuple: """botocore + urllib3 transport-failure exception classes (best-effort import).""" + import importlib + types: list = [] - try: - from botocore.exceptions import ConnectionError as BotoConnectionError, HTTPClientError - types += [BotoConnectionError, HTTPClientError] - except ImportError: # pragma: no cover — botocore always present with boto3 - pass - try: - from urllib3.exceptions import ConnectionError as Urllib3ConnectionError, NewConnectionError, ProtocolError - types += [ProtocolError, NewConnectionError, Urllib3ConnectionError] - except ImportError: # pragma: no cover - pass + for module, names in ( + ("botocore.exceptions", ("ConnectionError", "HTTPClientError")), + ("urllib3.exceptions", ("ProtocolError", "NewConnectionError", "ConnectionError")), + ): + try: + mod = importlib.import_module(module) + except ImportError: # pragma: no cover — both present with boto3 + continue + types += [getattr(mod, name) for name in names] return tuple(types) @@ -458,13 +459,12 @@ def strip_cache_points(kwargs: Dict[str, Any], placement: str) -> Dict[str, Any] messages = kwargs.get("messages") if not isinstance(messages, list): return kwargs - cleaned_messages = [] - changed = False - for msg in messages: - content = _without_cache_points(msg.get("content") if isinstance(msg, dict) else None) - changed = changed or content is not None - cleaned_messages.append(msg if content is None else {**msg, "content": content}) - return {**kwargs, "messages": cleaned_messages} if changed else kwargs + cleaned_contents = [_without_cache_points(msg.get("content") if isinstance(msg, dict) else None) for msg in messages] + if all(content is None for content in cleaned_contents): + return kwargs + return {**kwargs, "messages": [ + msg if content is None else {**msg, "content": content} for msg, content in zip(messages, cleaned_contents) + ]} return kwargs @@ -769,11 +769,10 @@ class _ResponseParts: reasoning_details=self.reasoning_details or None, bedrock_content_blocks=ordered_blocks or None, ) - input_tokens = usage_data.get("inputTokens", 0) cache_read_tokens = usage_data.get("cacheReadInputTokens", 0) cache_write_tokens = usage_data.get("cacheWriteInputTokens", 0) output_tokens = usage_data.get("outputTokens", 0) - prompt_tokens = input_tokens + cache_read_tokens + cache_write_tokens + prompt_tokens = usage_data.get("inputTokens", 0) + cache_read_tokens + cache_write_tokens usage = SimpleNamespace( prompt_tokens=prompt_tokens, completion_tokens=output_tokens, @@ -899,11 +898,7 @@ def stream_converse_with_callbacks( if "toolUse" in start: has_tool_use = True flush_text() - current_tool = { - "toolUseId": start["toolUse"].get("toolUseId", ""), - "name": start["toolUse"].get("name", ""), - "input_json": "", - } + current_tool = {"toolUseId": start["toolUse"].get("toolUseId", ""), "name": start["toolUse"].get("name", ""), "input_json": ""} stream_blocks[current_block_index] = _tool_use_block(current_tool["toolUseId"], current_tool["name"], {}) if on_tool_start: on_tool_start(current_tool["name"]) @@ -917,9 +912,8 @@ def stream_converse_with_callbacks( current_text_buffer.append(text) if on_text_delta and not has_tool_use: on_text_delta(text) - elif "toolUse" in delta: - if current_tool is not None: - current_tool["input_json"] += delta["toolUse"].get("input", "") + elif "toolUse" in delta and current_tool is not None: + current_tool["input_json"] += delta["toolUse"].get("input", "") elif "reasoningContent" in delta: on_reasoning(delta["reasoningContent"]) @@ -1065,6 +1059,13 @@ def reset_discovery_cache(): _discovery_cache.clear() +def _model_entry(model_id: str, name: Any, provider: str, input_mods: list, output_mods: list) -> Dict[str, Any]: + return { + "id": model_id, "name": (name or model_id).strip(), "provider": provider, + "input_modalities": input_mods, "output_modalities": output_mods, "streaming": True, + } + + def _list_foundation_models(client, filter_set: set, models: List[Dict[str, Any]]) -> None: """Append active, streaming-capable, text-output foundation models (optionally provider-filtered).""" for summary in client.list_foundation_models().get("modelSummaries", []): @@ -1083,14 +1084,10 @@ def _list_foundation_models(client, filter_set: set, models: List[Dict[str, Any] or "TEXT" not in output_mods ): continue - models.append({ - "id": model_id, - "name": (summary.get("modelName") or model_id).strip(), - "provider": (summary.get("providerName") or "").strip(), - "input_modalities": summary.get("inputModalities", []), - "output_modalities": output_mods, - "streaming": True, - }) + models.append(_model_entry( + model_id, summary.get("modelName"), (summary.get("providerName") or "").strip(), + summary.get("inputModalities", []), output_mods, + )) def _list_inference_profiles(client, filter_set: set, models: List[Dict[str, Any]]) -> None: @@ -1113,14 +1110,7 @@ def _list_inference_profiles(client, filter_set: set, models: List[Dict[str, Any _extract_provider_from_arn(m.get("modelArn", "")).lower() in filter_set for m in profile.get("models", []) ): continue - models.append({ - "id": profile_id, - "name": (profile.get("inferenceProfileName") or profile_id).strip(), - "provider": "inference-profile", - "input_modalities": ["TEXT"], - "output_modalities": ["TEXT"], - "streaming": True, - }) + models.append(_model_entry(profile_id, profile.get("inferenceProfileName"), "inference-profile", ["TEXT"], ["TEXT"])) seen_ids.add(profile_id.lower()) @@ -1177,40 +1167,28 @@ BEDROCK_CONTEXT_LENGTHS: Dict[str, int] = { # Sonnet 4.5 / Sonnet 4 lost their 1M beta and are 200K; Haiku 4.5 is 200K. # The 1M entries must match agent/model_metadata.py DEFAULT_CONTEXT_LENGTHS # or context compresses early. - "anthropic.claude-fable-5": 1_000_000, - "anthropic.claude-fable": 1_000_000, - "anthropic.claude-sonnet-5": 1_000_000, - "anthropic.claude-opus-4-8": 1_000_000, - "anthropic.claude-opus-4-7": 1_000_000, - "anthropic.claude-opus-4-6": 1_000_000, - "anthropic.claude-sonnet-4-6": 1_000_000, - "anthropic.claude-sonnet-4-5": 200_000, - "anthropic.claude-haiku-4-5": 200_000, - "anthropic.claude-opus-4": 200_000, - "anthropic.claude-sonnet-4": 200_000, - "anthropic.claude-3-5-sonnet": 200_000, - "anthropic.claude-3-5-haiku": 200_000, - "anthropic.claude-3-opus": 200_000, - "anthropic.claude-3-sonnet": 200_000, - "anthropic.claude-3-haiku": 200_000, + **dict.fromkeys(( + "anthropic.claude-fable-5", "anthropic.claude-fable", "anthropic.claude-sonnet-5", + "anthropic.claude-opus-4-8", "anthropic.claude-opus-4-7", "anthropic.claude-opus-4-6", + "anthropic.claude-sonnet-4-6", + ), 1_000_000), + **dict.fromkeys(( + "anthropic.claude-sonnet-4-5", "anthropic.claude-haiku-4-5", "anthropic.claude-opus-4", + "anthropic.claude-sonnet-4", "anthropic.claude-3-5-sonnet", "anthropic.claude-3-5-haiku", + "anthropic.claude-3-opus", "anthropic.claude-3-sonnet", "anthropic.claude-3-haiku", + ), 200_000), # Amazon Nova - "amazon.nova-pro": 300_000, - "amazon.nova-lite": 300_000, - "amazon.nova-micro": 128_000, - # Meta Llama - "meta.llama4-maverick": 128_000, - "meta.llama4-scout": 128_000, - "meta.llama3-3-70b-instruct": 128_000, - # Mistral - "mistral.mistral-large": 128_000, - # DeepSeek - "deepseek.v3": 128_000, + "amazon.nova-pro": 300_000, + "amazon.nova-lite": 300_000, + "amazon.nova-micro": 128_000, + # Meta Llama / Mistral / DeepSeek + **dict.fromkeys(( + "meta.llama4-maverick", "meta.llama4-scout", "meta.llama3-3-70b-instruct", + "mistral.mistral-large", "deepseek.v3", + ), 128_000), # OpenAI on Bedrock (Mantle/Responses route) # https://docs.aws.amazon.com/bedrock/latest/userguide/model-cards-openai.html - "openai.gpt-5.5": 272_000, - "openai.gpt-5.6-sol": 272_000, - "openai.gpt-5.6-terra": 272_000, - "openai.gpt-5.6-luna": 272_000, + **dict.fromkeys(BEDROCK_OPENAI_RESPONSES_MODEL_IDS, 272_000), } # Default for unknown Bedrock models From 5634e320055086248537ee0c7b3550f737fe8b9f Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Wed, 2 Sep 2026 18:39:31 -0700 Subject: [PATCH 05/13] refactor(agent/codex_runtime): extract _output_text_of, fold accumulator init/returns, tighten item payload helpers (-40 LOC) --- agent/codex_runtime.py | 145 +++++++++++++++-------------------------- 1 file changed, 53 insertions(+), 92 deletions(-) diff --git a/agent/codex_runtime.py b/agent/codex_runtime.py index 0b381c3af9..9fe119694b 100644 --- a/agent/codex_runtime.py +++ b/agent/codex_runtime.py @@ -55,10 +55,8 @@ def _codex_request_failure_details(error: BaseException) -> tuple[int | None, st request_body_bytes = len(content.encode("utf-8")) elif isinstance(content, (bytes, bytearray, memoryview)): request_body_bytes = len(content) - if current.__cause__ is None and not current.__suppress_context__: - current = current.__context__ - else: - current = current.__cause__ + implicit_chain = current.__cause__ is None and not current.__suppress_context__ + current = current.__context__ if implicit_chain else current.__cause__ return request_body_bytes, " <- ".join(exception_classes) @@ -137,8 +135,10 @@ def _record_codex_app_server_usage(agent, turn) -> dict[str, Any]: field: getattr(canonical_usage, field) for field in ("input_tokens", "output_tokens", "cache_read_tokens", "cache_write_tokens", "reasoning_tokens") } - usage_dict = {"prompt_tokens": prompt_tokens, "completion_tokens": canonical_usage.output_tokens, - "total_tokens": total_tokens, **token_counts} + usage_dict = { + "prompt_tokens": prompt_tokens, "completion_tokens": canonical_usage.output_tokens, "total_tokens": total_tokens, + **token_counts, + } if compressor is not None: try: @@ -221,12 +221,9 @@ def _record_codex_app_server_compaction(agent, turn, *, approx_tokens: int | Non args=("session:compress", { "platform": getattr(agent, "platform", None) or "", "session_id": getattr(agent, "session_id", None) or "", - "old_session_id": "", - "in_place": False, + "old_session_id": "", "in_place": False, "compression_count": getattr(compressor, "compression_count", 0) if compressor is not None else 0, - "runtime": "codex_app_server", - "thread_id": thread_id, - "turn_id": turn_id, + "runtime": "codex_app_server", "thread_id": thread_id, "turn_id": turn_id, })) return True @@ -283,9 +280,7 @@ def _codex_item_to_args(item: dict) -> dict: if item_type in _MCP_LIKE_ITEM_TYPES: args = item.get("arguments") or {} return args if isinstance(args, dict) else {"arguments": args} - if item_type == "webSearch": - return {"query": item.get("query") or ""} - return {} + return {"query": item.get("query") or ""} if item_type == "webSearch" else {} def _codex_item_to_preview(item: dict) -> Any: @@ -330,9 +325,8 @@ def _codex_item_completion_payload(item: dict) -> tuple[str, bool]: if item_type == "dynamicToolCall": content_items = item.get("contentItems") or [] success = item.get("success", True) - if isinstance(content_items, list) and content_items: - return json.dumps(content_items, ensure_ascii=False)[:4000], not bool(success) - return f"success={success}", not bool(success) + has_items = isinstance(content_items, list) and content_items + return (json.dumps(content_items, ensure_ascii=False)[:4000] if has_items else f"success={success}"), not bool(success) return "", False @@ -369,8 +363,7 @@ def make_codex_app_server_event_bridge(agent) -> Callable[[dict], None]: _call_guarded(getattr(agent, attr, None), fail_msg, *fail_args, args=args, kwargs=kwargs) def _fire_tool_started(item: dict) -> None: - item_id = item.get("id") or "" - name = _codex_item_to_tool_name(item) + item_id, name = item.get("id") or "", _codex_item_to_tool_name(item) args = _codex_item_to_args(item) if item_id: started[item_id] = (name, args, time.monotonic()) @@ -381,9 +374,8 @@ def make_codex_app_server_event_bridge(agent) -> Callable[[dict], None]: args=(_stable_call_id(item, name), name, args)) def _fire_tool_completed(item: dict) -> None: - item_id = item.get("id") or "" name = _codex_item_to_tool_name(item) - prior = started.pop(item_id, None) + prior = started.pop(item_id, None) if (item_id := item.get("id") or "") else None # Prefer codex's durationMs; else our started timestamp; else None # (some codex versions only emit completed for fast items). duration: Any = None @@ -488,11 +480,8 @@ def _ensure_codex_session(agent) -> None: logger.debug("codex app-server: approval-bypass lookup failed; keeping fail-closed default", exc_info=True) agent._codex_session = CodexAppServerSession( - cwd=getattr(agent, "session_cwd", None) or str(resolve_agent_cwd()), - approval_callback=approval_callback, - request_routing=_ServerRequestRouting( - auto_approve_exec=auto_approve_requests, auto_approve_apply_patch=auto_approve_requests, - ), + cwd=getattr(agent, "session_cwd", None) or str(resolve_agent_cwd()), approval_callback=approval_callback, + request_routing=_ServerRequestRouting(auto_approve_exec=auto_approve_requests, auto_approve_apply_patch=auto_approve_requests), on_event=make_codex_app_server_event_bridge(agent), ) @@ -542,8 +531,7 @@ def _finish_codex_turn( # Skill nudge check AFTER iters were incremented (same as chat_completions). should_review_skills = ( - agent._skill_nudge_interval > 0 and agent._iters_since_skill >= agent._skill_nudge_interval - and "skill_manage" in agent.valid_tool_names + 0 < agent._skill_nudge_interval <= agent._iters_since_skill and "skill_manage" in agent.valid_tool_names ) if should_review_skills: agent._iters_since_skill = 0 @@ -603,9 +591,7 @@ def run_codex_app_server_turn( _close_codex_session(agent) return _turn_result( _consume_user_interrupt(agent), messages, api_calls=0, completed=False, error=str(exc), - final_response=( - f"Codex app-server turn failed: {exc}. Fall back to default runtime with `/codex-runtime auto`." - ), + final_response=f"Codex app-server turn failed: {exc}. Fall back to default runtime with `/codex-runtime auto`.", ) interrupt = _consume_user_interrupt(agent, turn.interrupted) @@ -640,15 +626,10 @@ def _turn_result( """Result shape shared with the chat_completions path (``partial`` == ``not completed``).""" user_interrupted, interrupt_message = interrupt return { - "final_response": final_response, - "messages": messages, - "api_calls": api_calls, - "completed": completed, - "partial": not completed, - "interrupted": user_interrupted, + "final_response": final_response, "messages": messages, "api_calls": api_calls, + "completed": completed, "partial": not completed, "interrupted": user_interrupted, **({"interrupt_message": interrupt_message} if interrupt_message else {}), - "error": error, - **extra, + "error": error, **extra, } @@ -686,9 +667,7 @@ def _raise_stream_error(event: Any) -> None: return _event_field(nested, name) if value is None and nested is not None else value raw_message = _error_field("message") - if raw_message is not None and not isinstance(raw_message, str): - raw_message = str(raw_message) - message = (raw_message or "stream emitted error event").strip() or "stream emitted error event" + message = (str(raw_message) if raw_message is not None else "stream emitted error event").strip() or "stream emitted error event" raise _StreamErrorEvent(message, code=_error_field("code"), param=_error_field("param")) @@ -697,6 +676,16 @@ def _message_phase(item: Any) -> str | None: return phase.strip().lower() if isinstance(phase, str) else None +def _output_text_of(item: Any) -> str: + """Concatenated ``output_text`` parts of a message item ("" if content is not a list).""" + content_parts = _event_field(item, "content", []) + if not isinstance(content_parts, list): + return "" + return "".join( + str(_event_field(part, "text", "") or "") for part in content_parts if _event_field(part, "type", "") == "output_text" + ).strip() + + class _CodexResponseAssembler: """Assemble a Response-shaped ``SimpleNamespace`` from raw Responses SSE events. @@ -724,11 +713,8 @@ class _CodexResponseAssembler: saw_response_completed = False def __init__(self, *, model, on_text_delta, on_reasoning_delta, on_commentary_message, on_first_delta): - self.model = model - self.on_text_delta = on_text_delta - self.on_reasoning_delta = on_reasoning_delta - self.on_commentary_message = on_commentary_message - self.on_first_delta = on_first_delta + self.model, self.on_text_delta, self.on_reasoning_delta = model, on_text_delta, on_reasoning_delta + self.on_commentary_message, self.on_first_delta = on_commentary_message, on_first_delta self.output_items: List[Any] = [] # output_index / first-observed sequence per output item, in lockstep, so # settled pending calls merge back in stream order. @@ -768,10 +754,8 @@ class _CodexResponseAssembler: if item_id: announced_sequence, announced_index = self.announced_output_order[item_id] self.pending_function_calls[item_id] = { - "item": item, - "arguments": str(_event_field(item, "arguments", "") or ""), - "output_index": announced_index, - "sequence": announced_sequence, + "item": item, "arguments": str(_event_field(item, "arguments", "") or ""), + "output_index": announced_index, "sequence": announced_sequence, } def _on_text_delta(self, event: Any, event_type: str) -> None: @@ -789,11 +773,12 @@ class _CodexResponseAssembler: self._safe(self.on_reasoning_delta, "on_reasoning_delta", delta_text) else: self.text_deltas.append(delta_text) - if not self.has_tool_calls: - if not self.first_delta_fired: - self.first_delta_fired = True - self._safe(self.on_first_delta, "on_first_delta") - self._safe(self.on_text_delta, "on_text_delta", delta_text) + if self.has_tool_calls: + return + if not self.first_delta_fired: + self.first_delta_fired = True + self._safe(self.on_first_delta, "on_first_delta") + self._safe(self.on_text_delta, "on_text_delta", delta_text) def _on_function_call(self, event: Any, event_type: str) -> None: self.has_tool_calls = True @@ -838,14 +823,7 @@ class _CodexResponseAssembler: # Confirmed by the authoritative done event; never settle it twice. self.pending_function_calls.pop(done_id, None) if _message_phase(done_item) == "commentary" and self.on_commentary_message is not None: - commentary_text = "".join(self.commentary_text_deltas).strip() - if not commentary_text: - content_parts = _event_field(done_item, "content", []) - if isinstance(content_parts, list): - commentary_text = "".join( - str(_event_field(part, "text", "") or "") - for part in content_parts if _event_field(part, "type", "") == "output_text" - ).strip() + commentary_text = "".join(self.commentary_text_deltas).strip() or _output_text_of(done_item) if commentary_text: self._safe(self.on_commentary_message, "on_commentary_message", commentary_text) self.commentary_text_deltas = [] @@ -895,14 +873,11 @@ class _CodexResponseAssembler: for pending in self.pending_function_calls.values(): item = pending["item"] indexed.append((pending.get("output_index"), pending["sequence"], SimpleNamespace( - type="function_call", - id=_event_field(item, "id", None), - call_id=_event_field(item, "call_id", None), - name=_event_field(item, "name", None), + type="function_call", id=_event_field(item, "id", None), call_id=_event_field(item, "call_id", None), + name=_event_field(item, "name", None), status="completed", # Empty/whitespace arguments become "{}" so zero-delta calls stay # executable; malformed non-empty JSON passes through untouched. arguments=(pending["arguments"] or "").strip() or "{}", - status="completed", ))) # output_index is optional and a partial ordering over mixed indexed/unindexed @@ -921,10 +896,8 @@ class _CodexResponseAssembler: # synthesize a single message item for downstream normalization. output: List[Any] = list(self.output_items) if not output and self.text_deltas and not self.has_tool_calls: - output = [SimpleNamespace( - type="message", role="assistant", status="completed", - content=[SimpleNamespace(type="output_text", text="".join(self.text_deltas))], - )] + content = [SimpleNamespace(type="output_text", text="".join(self.text_deltas))] + output = [SimpleNamespace(type="message", role="assistant", status="completed", content=content)] # Done items stay authoritative; settlement only fills the gap left by # backends that omit per-item done events on a successful completion. @@ -937,13 +910,8 @@ class _CodexResponseAssembler: raise RuntimeError("Codex Responses stream did not emit a terminal response") return SimpleNamespace( - output=output, - output_text="".join(self.text_deltas), - usage=self.terminal_usage, - status=self.terminal_status, - id=self.terminal_response_id, - model=self.model, - incomplete_details=self.terminal_incomplete_details, + output=output, output_text="".join(self.text_deltas), usage=self.terminal_usage, status=self.terminal_status, + id=self.terminal_response_id, model=self.model, incomplete_details=self.terminal_incomplete_details, error=self.terminal_error, ) @@ -1109,8 +1077,7 @@ def run_codex_stream(agent, api_kwargs: dict, client: Any = None, on_first_delta agent._codex_streamed_text_parts.append(text) agent._fire_stream_delta(text) - def _on_event(event: Any) -> None: - # TTFB watchdog and activity touch — once per SSE event. + def _on_event(event: Any) -> None: # TTFB watchdog and activity touch — once per SSE event. agent._codex_stream_last_event_ts = time.time() agent._touch_activity("receiving stream response") @@ -1211,22 +1178,16 @@ def run_codex_stream(agent, api_kwargs: dict, client: Any = None, on_first_delta accept_chunk=_accept_codex_chunk, completed_response_predicate=lambda r: bool(hasattr(r, "output") and not hasattr(r, "__iter__")), metadata={ - "api_mode": "codex_responses", + "api_mode": "codex_responses", "call_role": call_role, "retry_count": attempt, "api_request_id": getattr(agent, "_current_api_request_id", None), - "call_role": call_role, - "retry_count": attempt, }, defer_logical_completion=True, ) final = _consume_codex_event_stream( - event_stream, - model=model, - on_text_delta=_fenced(_on_text_delta), + event_stream, model=model, on_text_delta=_fenced(_on_text_delta), on_reasoning_delta=_fenced(lambda text: agent._fire_reasoning_delta(text)), - on_commentary_message=on_commentary_message, - on_first_delta=on_first_delta, - on_event=_fenced(_on_event), - interrupt_check=_interrupt_or_superseded, + on_commentary_message=on_commentary_message, on_first_delta=on_first_delta, + on_event=_fenced(_on_event), interrupt_check=_interrupt_or_superseded, ) except transport_errors as exc: if attempt >= max_stream_retries: From e0693c4d3e87fb21cfee8ed88cac3dec9d2e774a Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Wed, 2 Sep 2026 18:47:54 -0700 Subject: [PATCH 06/13] refactor(agent/codex*,bedrock_adapter): drop intra-function blank lines (AST-identical, -117 LOC) --- agent/bedrock_adapter.py | 28 -------------------- agent/codex_headers.py | 1 - agent/codex_responses_adapter.py | 45 -------------------------------- agent/codex_runtime.py | 43 ------------------------------ 4 files changed, 117 deletions(-) diff --git a/agent/bedrock_adapter.py b/agent/bedrock_adapter.py index 74292e4ccf..f5f65ddb8a 100644 --- a/agent/bedrock_adapter.py +++ b/agent/bedrock_adapter.py @@ -154,7 +154,6 @@ class BedrockOpenAISigV4Auth(httpx.Auth): import botocore.session from botocore.auth import SigV4Auth from botocore.awsrequest import AWSRequest - credentials = botocore.session.get_session().get_credentials() if credentials is None: raise RuntimeError( @@ -215,7 +214,6 @@ _STALE_LIB_MODULE_PREFIXES = ("urllib3.", "botocore.", "boto3.") def _stale_error_types() -> tuple: """botocore + urllib3 transport-failure exception classes (best-effort import).""" import importlib - types: list = [] for module, names in ( ("botocore.exceptions", ("ConnectionError", "HTTPClientError")), @@ -656,19 +654,16 @@ def _assistant_blocks(msg: Dict, content) -> List[Dict]: content_blocks = _replay_ordered_blocks(ordered_blocks) if content_blocks: return content_blocks - content_blocks = [] for detail in (msg.get("reasoning_details") or []): if isinstance(detail, dict) and detail.get("type") == "redacted_thinking": redacted = _decode_redacted(detail.get("data") or detail.get("redactedContentBase64")) if redacted is not None: content_blocks.append({"reasoningContent": {"redactedContent": redacted}}) - if isinstance(content, str) and content.strip(): content_blocks.append({"text": content}) elif isinstance(content, list): content_blocks.extend(_convert_content_to_converse(content)) - for tc in (msg.get("tool_calls", []) or []): fn = tc.get("function", {}) content_blocks.append(_tool_use_block(tc.get("id", ""), fn.get("name", ""), _parse_tool_args(fn.get("arguments", "{}")))) @@ -691,7 +686,6 @@ def convert_messages_to_converse(messages: List[Dict]) -> Tuple[Optional[List[Di converse_msgs[-1]["content"].extend(blocks) else: converse_msgs.append({"role": role, "content": blocks}) - for msg in messages: role = msg.get("role", "") content = msg.get("content") @@ -707,7 +701,6 @@ def convert_messages_to_converse(messages: List[Dict]) -> Tuple[Optional[List[Di append_turn("assistant", _assistant_blocks(msg, content) or [dict(_PLACEHOLDER_BLOCK)]) elif role == "user": append_turn("user", _convert_content_to_converse(content)) - if converse_msgs and converse_msgs[0]["role"] != "user": converse_msgs.insert(0, {"role": "user", "content": [dict(_PLACEHOLDER_BLOCK)]}) if converse_msgs and converse_msgs[-1]["role"] != "user": @@ -881,7 +874,6 @@ def stream_converse_with_callbacks( if encoded: parts.add_redacted(encoded) current_block({"reasoningContent": {}}).setdefault("reasoningContent", {})["redactedContentBase64"] = encoded - for event in event_stream.get("stream", []): if on_event is not None: try: @@ -890,7 +882,6 @@ def stream_converse_with_callbacks( pass if on_interrupt_check and on_interrupt_check(): break - if "contentBlockStart" in event: start_event = event["contentBlockStart"] current_block_index = start_event.get("contentBlockIndex", len(stream_blocks)) @@ -902,7 +893,6 @@ def stream_converse_with_callbacks( stream_blocks[current_block_index] = _tool_use_block(current_tool["toolUseId"], current_tool["name"], {}) if on_tool_start: on_tool_start(current_tool["name"]) - elif "contentBlockDelta" in event: delta = event["contentBlockDelta"].get("delta", {}) if "text" in delta: @@ -916,7 +906,6 @@ def stream_converse_with_callbacks( current_tool["input_json"] += delta["toolUse"].get("input", "") elif "reasoningContent" in delta: on_reasoning(delta["reasoningContent"]) - elif "contentBlockStop" in event: if current_tool is not None: input_dict = _parse_tool_args(current_tool["input_json"]) if current_tool["input_json"] else {} @@ -926,17 +915,14 @@ def stream_converse_with_callbacks( current_tool = None else: flush_text() - elif "messageStop" in event: stop_reason = event["messageStop"].get("stopReason", "end_turn") - elif "metadata" in event: meta_usage = event["metadata"].get("usage", {}) usage_data = { key: meta_usage.get(key, 0) for key in ("inputTokens", "outputTokens", "cacheReadInputTokens", "cacheWriteInputTokens") } - flush_text() return parts.build([stream_blocks[i] for i in sorted(stream_blocks)], usage_data, stop_reason, "") @@ -966,17 +952,13 @@ def build_converse_kwargs( def cache_here(placement: str) -> bool: return cache_enabled and cache_point_allowed(model, placement) - inference_config: Dict[str, Any] = {} if max_tokens is not None: inference_config["maxTokens"] = max_tokens kwargs: Dict[str, Any] = {"modelId": model, "messages": converse_messages, "inferenceConfig": inference_config} - if system_prompt: kwargs["system"] = system_prompt + [dict(_CACHE_POINT)] if cache_here("system") else system_prompt - from agent.anthropic_adapter import _forbids_sampling_params - if not _forbids_sampling_params(model): if temperature is not None: inference_config["temperature"] = temperature @@ -984,7 +966,6 @@ def build_converse_kwargs( inference_config["topP"] = top_p if stop_sequences: inference_config["stopSequences"] = stop_sequences - converse_tools = convert_tools_to_converse(tools) if tools else [] if converse_tools: # Non-tool-calling models (e.g. DeepSeek R1) reject toolConfig with a @@ -998,12 +979,10 @@ def build_converse_kwargs( "Model %s does not support tool calling — tools stripped. " "The agent will operate in text-only mode.", model ) - if cache_here("messages") and len(converse_messages) >= 2: content = converse_messages[-2].get("content") if isinstance(content, list) and content: content.append(dict(_CACHE_POINT)) - if guardrail_config: kwargs["guardrailConfig"] = guardrail_config if not inference_config: @@ -1100,7 +1079,6 @@ def _list_inference_profiles(client, filter_set: set, models: List[Dict[str, Any next_token = response.get("nextToken") if not next_token: break - seen_ids = {m["id"].lower() for m in models} for profile in profiles: profile_id = (profile.get("inferenceProfileId") or "").strip() @@ -1123,18 +1101,15 @@ def discover_bedrock_models(region: str, provider_filter: Optional[List[str]] = Returns [] when the client cannot be built. """ import time - cache_key = f"{region}:{','.join(sorted(provider_filter or []))}" cached = _discovery_cache.get(cache_key) if cached and (time.time() - cached["timestamp"]) < _DISCOVERY_CACHE_TTL_SECONDS: return cached["models"] - try: client = _get_bedrock_control_client(region) except Exception as e: logger.warning("Failed to create Bedrock client for model discovery: %s", e) return [] - models: List[Dict[str, Any]] = [] filter_set = {f.lower() for f in (provider_filter or [])} try: @@ -1145,7 +1120,6 @@ def discover_bedrock_models(region: str, provider_filter: Optional[List[str]] = _list_inference_profiles(client, filter_set, models) except Exception as e: logger.debug("Skipping inference profile discovery: %s", e) - models.sort(key=lambda m: (0 if m["id"].startswith("global.") else 1, m["name"].lower())) _discovery_cache[cache_key] = {"timestamp": time.time(), "models": models} return models @@ -1220,7 +1194,6 @@ def probe_bedrock_context_length(model_id: str, region: str) -> Optional[int]: except Exception as exc: # boto3 missing / credential resolution failure logger.debug("Bedrock context probe skipped for %s: %s", model_id, exc) return None - last_error = "" for tier_tokens in _BEDROCK_PROBE_TIERS: oversized = "data " * int(tier_tokens / _WORDS_PER_TOKEN) @@ -1242,7 +1215,6 @@ def probe_bedrock_context_length(model_id: str, region: str) -> Optional[int]: logger.info("Probed Bedrock context window for %s: %s tokens", model_id, f"{limit:,}") return limit # Opaque server error / auth / throttle at this tier — try the next. - logger.debug("Bedrock context probe for %s returned no parseable limit: %s", model_id, last_error[:200]) return None diff --git a/agent/codex_headers.py b/agent/codex_headers.py index bec0c8fb0e..d967ae2e8a 100644 --- a/agent/codex_headers.py +++ b/agent/codex_headers.py @@ -43,7 +43,6 @@ def codex_cloudflare_headers(access_token: str, *, base_url: str = CODEX_AUX_BAS """ if is_official_codex_base_url(base_url): from hermes_cli import __version__ - headers = {"User-Agent": f"HermesAgent/{__version__}", "originator": "hermes-agent"} else: headers = {"User-Agent": "codex_cli_rs/0.0.0 (Hermes Agent)", "originator": "codex_cli_rs"} diff --git a/agent/codex_responses_adapter.py b/agent/codex_responses_adapter.py index da3e23c774..8f55793c16 100644 --- a/agent/codex_responses_adapter.py +++ b/agent/codex_responses_adapter.py @@ -126,11 +126,9 @@ def _neutralize_harmony_tokens(text: str) -> str: """Keep Harmony source readable without emitting reserved wire tokens.""" if not text or "<" not in text or "|" not in text: return text - replacement = rf"<{_FULLWIDTH_PIPE}\1{_FULLWIDTH_PIPE}>" if not any(unicodedata.category(char) == "Cf" for char in text): return _HARMONY_CONTROL_TOKEN_RE.sub(replacement, text) - # The backend strips Unicode format controls (e.g. U+200B) before its # reserved-token check, so match on the visible text and rewrite the # original spans — any Cf-hidden variant is neutralized the same way. @@ -324,7 +322,6 @@ def _derive_responses_function_call_id(call_id: str, response_item_id: Optional[ """Build a valid Responses `function_call.id` (must start with `fc_`).""" if isinstance(response_item_id, str) and response_item_id.strip().startswith("fc_"): return response_item_id.strip() - source = (call_id or "").strip() sanitized = re.sub(r"[^A-Za-z0-9_-]", "", source) for candidate in (source, sanitized): @@ -494,7 +491,6 @@ def _tool_output_item(msg: Dict[str, Any]) -> Optional[Dict[str, Any]]: call_id = raw_tool_call_id.strip() if not _nonblank(call_id): return None - # ``output`` may be a string or an ``input_text``/``input_image`` array. tool_content = msg.get("content") output_value: Any = ( @@ -546,7 +542,6 @@ def _chat_messages_to_responses_input( def emit(new_items: List[Dict[str, Any]], msg: Dict[str, Any]) -> None: items.extend(new_items) item_sources.extend([msg] * len(new_items)) - for msg in messages: if not isinstance(msg, dict): continue @@ -558,7 +553,6 @@ def _chat_messages_to_responses_input( continue if role not in {"user", "assistant"}: continue - content = msg.get("content", "") content_parts = _chat_content_to_responses_parts(content, role=role) # [] unless a list if isinstance(content, list): @@ -566,11 +560,9 @@ def _chat_messages_to_responses_input( content_text = "".join(p["text"] for p in content_parts if p["type"] == text_type) else: content_text = _str_or_empty(content) - if role == "user": emit([{"role": role, "content": content_parts or content_text}], msg) continue - reasoning_items = [] if not replay_encrypted_reasoning else _replay_reasoning_items( msg, seen_item_ids=seen_item_ids, current_issuer_kind=current_issuer_kind, native_compaction_eligible=native_compaction_eligible, @@ -578,7 +570,6 @@ def _chat_messages_to_responses_input( emit(reasoning_items, msg) message_items = _replay_message_items(msg, is_github_responses=is_github_responses) emit(message_items, msg) - if not message_items: if content_parts: emit([{"role": "assistant", "content": content_parts}], msg) @@ -587,9 +578,7 @@ def _chat_messages_to_responses_input( elif reasoning_items: # Every reasoning item needs a following item (else missing_following_item). emit([{"role": "assistant", "content": ""}], msg) - emit(_replay_tool_call_items(msg, start_index=len(items)), msg) - # Native server-side compaction renders nothing placed before a compaction # item, so pre-checkpoint history is dead upload weight and the user's # plaintext asks / merged local summaries silently vanish. Keep the newest @@ -597,9 +586,7 @@ def _chat_messages_to_responses_input( # messages within a token budget, leave the tail untouched. if not native_compaction_eligible: return items - from agent.native_compaction import prune_pre_checkpoint_items - return prune_pre_checkpoint_items(items, item_sources=item_sources) @@ -622,7 +609,6 @@ def classify_responses_route(agent: Any) -> ResponsesRouteFlags: ``https://evil.com/models.github.ai`` must not classify as GitHub. """ from utils import base_url_hostname - provider = getattr(agent, "provider", None) base_url = str(getattr(agent, "base_url", "") or "") hostname = str(getattr(agent, "_base_url_hostname", "") or "").lower() or base_url_hostname(base_url) @@ -630,7 +616,6 @@ def classify_responses_route(agent: Any) -> ResponsesRouteFlags: def _host_is(domain: str) -> bool: return hostname == domain or hostname.endswith("." + domain) - return ResponsesRouteFlags( is_codex_backend=provider == "openai-codex" or (_host_is("chatgpt.com") and "/backend-api/codex" in lower), is_xai_responses=provider in {"xai", "xai-oauth"} or hostname == "api.x.ai", @@ -654,17 +639,13 @@ def estimate_native_responses_preflight_tokens( """ if getattr(agent, "api_mode", None) != "codex_responses" or not isinstance(messages, list): return None - is_codex_backend, is_xai_responses, is_github_responses = classify_responses_route(agent) - from agent.native_compaction import native_compaction_context_management - if not native_compaction_context_management( agent, is_codex_backend=is_codex_backend, is_xai_responses=is_xai_responses, is_github_responses=is_github_responses, ): return None - try: items = _chat_messages_to_responses_input( messages, is_xai_responses=is_xai_responses, is_github_responses=is_github_responses, @@ -680,9 +661,7 @@ def estimate_native_responses_preflight_tokens( return None if not isinstance(items, list): return None - from agent.model_metadata import estimate_request_tokens_rough - return estimate_request_tokens_rough(items, system_prompt=system_prompt or "", tools=tools) @@ -781,7 +760,6 @@ def _preflight_role_message(item: Dict[str, Any], idx: int, role: str, ctx: _Pre content = item.get("content", "") if not isinstance(content, list): return {"role": role, "content": ctx.sanitize_text(_str_or_empty(content))} - # Parts are already Responses-shaped; validate and re-type text for the role. # Unlike history conversion, empty text / empty image urls are kept, not dropped. text_type = _text_type_for(role) @@ -825,7 +803,6 @@ def _preflight_codex_input_items( ) -> List[Dict[str, Any]]: if not isinstance(raw_items, list): raise ValueError("Codex Responses input must be a list of input items.") - ctx = _PreflightCtx( sanitize_text=_neutralize_harmony_tokens if sanitize_harmony_tokens else (lambda text: text), sanitize_harmony_tokens=sanitize_harmony_tokens, @@ -906,19 +883,15 @@ def _preflight_codex_api_kwargs( ) -> Dict[str, Any]: if not isinstance(api_kwargs, dict): raise ValueError("Codex Responses request must be a dict.") - missing = [key for key in ("model", "instructions", "input") if key not in api_kwargs] if missing: raise ValueError(f"Codex Responses request missing required field(s): {', '.join(sorted(missing))}.") - model = api_kwargs.get("model") if not _nonblank(model): raise ValueError("Codex Responses request 'model' must be a non-empty string.") - instructions = _str_or_empty(api_kwargs.get("instructions")).strip() or DEFAULT_AGENT_IDENTITY if sanitize_harmony_tokens: instructions = _neutralize_harmony_tokens(instructions) - normalized: Dict[str, Any] = { "model": model.strip(), "instructions": instructions, @@ -929,7 +902,6 @@ def _preflight_codex_api_kwargs( ), "store": False, } - tools = api_kwargs.get("tools") if tools is not None: if not isinstance(tools, list): @@ -938,15 +910,12 @@ def _preflight_codex_api_kwargs( if sanitize_harmony_tokens: normalized_tools = _neutralize_harmony_structure(normalized_tools) normalized["tools"] = normalized_tools - if api_kwargs.get("store", False) is not False: raise ValueError("Codex Responses contract requires 'store' to be false.") - for key, accept, coerce in _PREFLIGHT_OPTIONAL_FIELDS: value = api_kwargs.get(key) if accept(value): normalized[key] = coerce(value) if coerce else value - extra_headers = api_kwargs.get("extra_headers") if extra_headers is not None: if not isinstance(extra_headers, dict): @@ -956,7 +925,6 @@ def _preflight_codex_api_kwargs( normalized_headers = {key.strip(): str(value) for key, value in extra_headers.items() if value is not None} if normalized_headers: normalized["extra_headers"] = normalized_headers - extra_body = api_kwargs.get("extra_body") if extra_body is not None: if not isinstance(extra_body, dict): @@ -965,7 +933,6 @@ def _preflight_codex_api_kwargs( # the SDK serializes extra_body without per-field checks. if extra_body: normalized["extra_body"] = dict(extra_body) - allowed_keys = set(_PREFLIGHT_ALLOWED_KEYS) if allow_stream: stream = api_kwargs.get("stream") @@ -976,7 +943,6 @@ def _preflight_codex_api_kwargs( allowed_keys.add("stream") elif "stream" in api_kwargs: raise ValueError("Codex Responses stream flag is only allowed in fallback streaming requests.") - # Defense-in-depth slash-enum strip for xAI (rejects ``Qwen/Qwen3.5`` style # enum values). Gated on the model name because native Codex accepts slashes. is_xai_model = str(api_kwargs.get("model") or "").lower().startswith(("grok-", "x-ai/grok-")) @@ -986,7 +952,6 @@ def _preflight_codex_api_kwargs( normalized["tools"], _ = strip_slash_enum(normalized["tools"]) except Exception: pass # Best-effort — the caller-level sanitization should have handled it - unexpected = sorted(key for key in api_kwargs if key not in allowed_keys) if unexpected: raise ValueError(f"Codex Responses request has unsupported field(s): {', '.join(unexpected)}.") @@ -1025,7 +990,6 @@ def _format_responses_error(error_obj: Any, response_status: str) -> str: def field(name: str) -> str: value = _field(error_obj, name) return str(value).strip() if isinstance(value, str) or value else "" - code_str, message_str = field("code"), field("message") if code_str and message_str: return f"{code_str}: {message_str}" @@ -1113,7 +1077,6 @@ class _OutputScan: if item_status in _INCOMPLETE_STATUSES and item_type not in _SERVER_SIDE_TOOL_CALL_TYPES: self.has_incomplete_items = True self.saw_streaming_or_item_incomplete = True - if item_type == "message": self._message(item, item_status) elif item_type == "reasoning": @@ -1170,7 +1133,6 @@ def _normalize_codex_response(response: Any, *, issuer_kind: Optional[str] = Non response_incomplete_content_filter = ( response_status == "incomplete" and str(incomplete_reason or "").strip().lower() == "content_filter" ) - output = getattr(response, "output", None) if not isinstance(output, list) or not output: # Codex can deliver the whole answer via stream events and return an @@ -1189,20 +1151,16 @@ def _normalize_codex_response(response: Any, *, issuer_kind: Optional[str] = Non else: raise RuntimeError("Responses API returned no output items") response.output = output - if response_status in {"failed", "cancelled"}: raise RuntimeError(_format_responses_error(getattr(response, "error", None), response_status)) - scan = _OutputScan(response_status) scan.scan(output, issuer_kind) tool_calls, reasoning_parts = scan.tool_calls, scan.reasoning_parts - final_text = "\n".join(scan.content_parts).strip() if not final_text and hasattr(response, "output_text") and (scan.saw_final_answer_phase or not scan.saw_commentary_phase): out_text = getattr(response, "output_text", "") if isinstance(out_text, str): final_text = out_text.strip() - # Tool-call leak recovery: gpt-5.x sometimes emits the intended # ``function_call`` as plain Harmony text (``to=functions.foo {json}``) with # no structured item. Treat as incomplete so the continuation path @@ -1216,7 +1174,6 @@ def _normalize_codex_response(response: Any, *, issuer_kind: Optional[str] = Non "Leaked snippet: %r", final_text[:300], ) final_text = "" - # Reasoning-channel answer salvage (xAI grok): grok-4.x sometimes puts the # final answer inside the reasoning item after its ```` delimiter. # Without salvage the reasoning-only rule marks the turn incomplete, and since @@ -1235,7 +1192,6 @@ def _normalize_codex_response(response: Any, *, issuer_kind: Optional[str] = Non final_text = salvaged reasoning_prefix = joined_reasoning[:marker].strip() reasoning_parts = [reasoning_prefix] if reasoning_prefix else [] - assistant_message = SimpleNamespace( content=final_text, tool_calls=tool_calls, @@ -1245,7 +1201,6 @@ def _normalize_codex_response(response: Any, *, issuer_kind: Optional[str] = Non codex_reasoning_items=scan.reasoning_items_raw or None, codex_message_items=scan.message_items_raw or None, ) - if tool_calls: finish_reason = "tool_calls" elif response_incomplete_content_filter: diff --git a/agent/codex_runtime.py b/agent/codex_runtime.py index 9fe119694b..d59530785b 100644 --- a/agent/codex_runtime.py +++ b/agent/codex_runtime.py @@ -41,7 +41,6 @@ def _codex_request_failure_details(error: BaseException) -> tuple[int | None, st exception_classes: list[str] = [] current: BaseException | None = error seen: set[int] = set() - while current is not None and id(current) not in seen and len(seen) < 8: seen.add(id(current)) exception_classes.append(type(current).__name__) @@ -102,7 +101,6 @@ def _record_codex_app_server_usage(agent, turn) -> dict[str, Any]: no usage still counts as one API call for session/status accounting. """ agent.session_api_calls += 1 - usage = getattr(turn, "token_usage_last", None) compressor = getattr(agent, "context_compressor", None) if not isinstance(usage, dict) or not usage: @@ -118,9 +116,7 @@ def _record_codex_app_server_usage(agent, turn) -> dict[str, Any]: ), ) return {} - from agent.usage_pricing import CanonicalUsage, estimate_usage_cost - canonical_usage = CanonicalUsage( input_tokens=_coerce_usage_int(usage.get("inputTokens")), output_tokens=_coerce_usage_int(usage.get("outputTokens")), @@ -139,7 +135,6 @@ def _record_codex_app_server_usage(agent, turn) -> dict[str, Any]: "prompt_tokens": prompt_tokens, "completion_tokens": canonical_usage.output_tokens, "total_tokens": total_tokens, **token_counts, } - if compressor is not None: try: compressor.update_from_response(usage_dict) @@ -148,10 +143,8 @@ def _record_codex_app_server_usage(agent, turn) -> dict[str, Any]: compressor.context_length = context_window except Exception: logger.debug("codex app-server usage update failed", exc_info=True) - for key, value in usage_dict.items(): setattr(agent, f"session_{key}", getattr(agent, f"session_{key}") + value) - cost_result = estimate_usage_cost( agent.model, canonical_usage, provider=agent.provider, base_url=agent.base_url, api_key=getattr(agent, "api_key", ""), @@ -161,7 +154,6 @@ def _record_codex_app_server_usage(agent, turn) -> dict[str, Any]: agent.session_estimated_cost_usd += cost_usd agent.session_cost_status, agent.session_cost_source = cost_result.status, cost_result.source cost_fields = {"estimated_cost_usd": cost_usd, "cost_status": cost_result.status, "cost_source": cost_result.source} - _queue_token_counts( agent, "Codex app-server token persistence failed (session=%s, tokens=%d): %s", total_tokens, counts=lambda: dict( @@ -182,7 +174,6 @@ def _record_codex_app_server_compaction(agent, turn, *, approx_tokens: int | Non """ if not force and not getattr(turn, "compacted", False): return False - thread_id = getattr(turn, "thread_id", None) or "" turn_id = getattr(turn, "turn_id", None) or "" logger.info( @@ -195,7 +186,6 @@ def _record_codex_app_server_compaction(agent, turn, *, approx_tokens: int | Non agent._emit_status(COMPACTION_STATUS) except Exception: pass - compressor = getattr(agent, "context_compressor", None) if compressor is not None: compressor.compression_count = getattr(compressor, "compression_count", 0) + 1 @@ -212,7 +202,6 @@ def _record_codex_app_server_compaction(agent, turn, *, approx_tokens: int | Non compressor.last_prompt_tokens = -1 compressor.last_completion_tokens = 0 compressor.awaiting_real_usage_after_compression = True - # Provider-side context was rewritten; the usage anchor's transcript snapshot no longer matches. agent._usage_anchor = None agent._turn_base_usage_anchor = None @@ -333,7 +322,6 @@ def _codex_item_completion_payload(item: dict) -> tuple[str, bool]: def _stable_call_id(item: dict, name: str) -> str: """Deterministic tool_call id mirroring CodexEventProjector (live TUI card correlates with projected history).""" from agent.transports.codex_event_projector import _deterministic_call_id - item_type = item.get("type") or "" tool = item.get("tool") or "unknown" if item_type == "mcpToolCall": @@ -414,7 +402,6 @@ def make_codex_app_server_event_bridge(agent) -> Callable[[dict], None]: (_fire_tool_completed if completed else _fire_tool_started)(item) elif completed and item_type == "agentMessage": _fire_agent_message_completed(item) - handlers: dict[str, Callable[[dict], None]] = { "item/agentMessage/delta": lambda p: _fire_delta(p, "_fire_stream_delta"), "item/reasoning/delta": lambda p: _fire_delta(p, "_fire_reasoning_delta"), @@ -428,7 +415,6 @@ def make_codex_app_server_event_bridge(agent) -> Callable[[dict], None]: if handler is not None: params = note.get("params") or {} handler(params if isinstance(params, dict) else {}) - return on_event @@ -461,7 +447,6 @@ def _ensure_codex_session(agent) -> None: return from agent.runtime_cwd import resolve_agent_cwd from agent.transports.codex_app_server_session import CodexAppServerSession, _ServerRequestRouting - # Approval callback: Hermes' standard prompt flow when a CLI thread installed one. try: from tools.terminal_tool import _get_approval_callback @@ -478,7 +463,6 @@ def _ensure_codex_session(agent) -> None: auto_approve_requests = is_approval_bypass_active() except Exception: logger.debug("codex app-server: approval-bypass lookup failed; keeping fail-closed default", exc_info=True) - agent._codex_session = CodexAppServerSession( cwd=getattr(agent, "session_cwd", None) or str(resolve_agent_cwd()), approval_callback=approval_callback, request_routing=_ServerRequestRouting(auto_approve_exec=auto_approve_requests, auto_approve_apply_patch=auto_approve_requests), @@ -498,10 +482,8 @@ def _persist_projected_messages(agent, turn, messages: List[Dict[str, Any]]) -> if not turn.projected_messages: return from agent.message_metadata import append_message - for projected_message in turn.projected_messages: append_message(messages, projected_message) - if getattr(agent, "_session_db", None) is None: return try: @@ -528,14 +510,12 @@ def _finish_codex_turn( agent._iters_since_skill = getattr(agent, "_iters_since_skill", 0) + turn.tool_iterations _record_codex_app_server_compaction(agent, turn) usage_result = _record_codex_app_server_usage(agent, turn) - # Skill nudge check AFTER iters were incremented (same as chat_completions). should_review_skills = ( 0 < agent._skill_nudge_interval <= agent._iters_since_skill and "skill_manage" in agent.valid_tool_names ) if should_review_skills: agent._iters_since_skill = 0 - # External memory sync skipped on interrupt/error (no partial transcripts). if not turn.interrupted and turn.error is None: try: @@ -545,7 +525,6 @@ def _finish_codex_turn( ) except Exception: logger.debug("external memory sync raised", exc_info=True) - # Background review fork: only when a trigger tripped AND a real final response exists. if turn.final_text and not turn.interrupted and (should_review_memory or should_review_skills): try: @@ -554,7 +533,6 @@ def _finish_codex_turn( ) except Exception: logger.debug("background review spawn raised", exc_info=True) - return usage_result @@ -581,9 +559,7 @@ def run_codex_app_server_turn( "codex_app_server owns the authoritative thread and compacts it " "without a truthful pre-compaction transcript boundary" ) - _ensure_codex_session(agent) - try: turn = agent._codex_session.run_turn(user_input=user_message) except Exception as exc: @@ -593,20 +569,16 @@ def run_codex_app_server_turn( _consume_user_interrupt(agent), messages, api_calls=0, completed=False, error=str(exc), final_response=f"Codex app-server turn failed: {exc}. Fall back to default runtime with `/codex-runtime auto`.", ) - interrupt = _consume_user_interrupt(agent, turn.interrupted) - # Wedged client (deadline blown, watchdog tripped, OAuth refresh died, # subprocess exited): retire the session so the next turn respawns codex. if getattr(turn, "should_retire", False): logger.warning("codex app-server session retired (turn error: %s)", turn.error) _close_codex_session(agent) - _persist_projected_messages(agent, turn, messages) usage_result = _finish_codex_turn( agent, turn, messages, original_user_message=original_user_message, should_review_memory=should_review_memory, ) - return _turn_result( interrupt, messages, api_calls=1, completed=not turn.interrupted and turn.error is None, error=turn.error, final_response=turn.final_text, @@ -659,13 +631,11 @@ def _raise_stream_error(event: Any) -> None: ``run_agent`` is imported lazily to keep this module importable standalone. """ from run_agent import _StreamErrorEvent - nested = _event_field(event, "error") def _error_field(name: str) -> Any: value = _event_field(event, name) return _event_field(nested, name) if value is None and nested is not None else value - raw_message = _error_field("message") message = (str(raw_message) if raw_message is not None else "stream emitted error event").strip() or "stream emitted error event" raise _StreamErrorEvent(message, code=_error_field("code"), param=_error_field("param")) @@ -879,7 +849,6 @@ class _CodexResponseAssembler: # executable; malformed non-empty JSON passes through untouched. arguments=(pending["arguments"] or "").strip() or "{}", ))) - # output_index is optional and a partial ordering over mixed indexed/unindexed # entries is ill-defined: protocol order only when every entry has an index, else wire order. if all(entry[0] is not None for entry in indexed): @@ -898,17 +867,14 @@ class _CodexResponseAssembler: if not output and self.text_deltas and not self.has_tool_calls: content = [SimpleNamespace(type="output_text", text="".join(self.text_deltas))] output = [SimpleNamespace(type="message", role="assistant", status="completed", content=content)] - # Done items stay authoritative; settlement only fills the gap left by # backends that omit per-item done events on a successful completion. if self.pending_function_calls and self.saw_response_completed: output = self._settled_output() - # No terminal frame AND no usable content = truncated / rejected stream, # distinct from "completed with empty body" (what the SDK helper raised as RuntimeError). if not self.saw_terminal and not output: raise RuntimeError("Codex Responses stream did not emit a terminal response") - return SimpleNamespace( output=output, output_text="".join(self.text_deltas), usage=self.terminal_usage, status=self.terminal_status, id=self.terminal_response_id, model=self.model, incomplete_details=self.terminal_incomplete_details, @@ -1021,7 +987,6 @@ def _bypass_sdk_request_transform(stream_kwargs: dict) -> dict: """ if os.environ.get("HERMES_CODEX_SDK_TRANSFORM", "").strip().lower() in {"1", "true", "yes", "on"}: return stream_kwargs - moved = { field: stream_kwargs[field] for field in _SDK_TRANSFORM_BYPASS_FIELDS @@ -1029,7 +994,6 @@ def _bypass_sdk_request_transform(stream_kwargs: dict) -> dict: } if not moved: return stream_kwargs - bypassed = {key: value for key, value in stream_kwargs.items() if key not in moved} extra_body = bypassed.get("extra_body") merged = dict(extra_body) if isinstance(extra_body, dict) else {} @@ -1049,9 +1013,7 @@ def run_codex_stream(agent, api_kwargs: dict, client: Any = None, on_first_delta """ import httpx as _httpx from openai import APIConnectionError as _APIConnectionError - from agent import relay_llm - transport_errors = (_httpx.RemoteProtocolError, _httpx.ReadTimeout, _httpx.ConnectError, ConnectionError) active_client = client or agent._ensure_primary_openai_client(reason="codex_stream_direct") max_stream_retries = 1 @@ -1144,7 +1106,6 @@ def run_codex_stream(agent, api_kwargs: dict, client: Any = None, on_first_delta # primary client, which is never reuse-cached and must not be force-shut. if client is not None: agent._abort_request_openai_client(active_client, reason="codex_stream_close_failed") - on_commentary_message = ( _fenced(lambda text: agent._fire_streamed_codex_commentary(text)) if getattr(agent, "interim_assistant_callback", None) is not None and getattr(agent, "show_commentary", True) @@ -1155,11 +1116,9 @@ def run_codex_stream(agent, api_kwargs: dict, client: Any = None, on_first_delta else "fallback" if int(getattr(agent, "_fallback_index", 0) or 0) > 0 else "primary" ) - for attempt in range(max_stream_retries + 1): if agent._interrupt_requested: raise InterruptedError("Agent interrupted before Codex stream retry") - intercepted_events: list = [] writer_token["value"] = None event_stream = None @@ -1209,10 +1168,8 @@ def run_codex_stream(agent, api_kwargs: dict, client: Any = None, on_first_delta except _APIConnectionError as exc: _log_failure(exc) raise - if not agent._interrupt_requested: _drain_for_finalizer(event_stream) - if final.status in {"incomplete", "failed"}: logger.warning( "Codex Responses stream terminal status=%s " From a7d2bf1466d42960c57fac079d64370a1a175563 Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Wed, 2 Sep 2026 18:51:03 -0700 Subject: [PATCH 07/13] refactor(agent/codex_responses_adapter): table issuer classification, unify optional-dict validation, collapse assistant fallback ladder (-56 LOC) --- agent/codex_responses_adapter.py | 99 ++++++++++++++------------------ 1 file changed, 44 insertions(+), 55 deletions(-) diff --git a/agent/codex_responses_adapter.py b/agent/codex_responses_adapter.py index 8f55793c16..a1b5ecdb0b 100644 --- a/agent/codex_responses_adapter.py +++ b/agent/codex_responses_adapter.py @@ -31,12 +31,9 @@ def _classify_responses_issuer( ``invalid_encrypted_content``); stamping items lets replay drop foreign blobs after a mid-conversation model switch. """ - if is_xai_responses: - return "xai_responses" - if is_github_responses: - return "github_responses" - if is_codex_backend: - return "codex_backend" + for flag, kind in ((is_xai_responses, "xai_responses"), (is_github_responses, "github_responses"), (is_codex_backend, "codex_backend")): + if flag: + return kind return f"other:{base_url}" if base_url else "other" @@ -170,9 +167,8 @@ def _neutralize_harmony_structure(value: Any) -> Any: def _iter_content_parts(content: list) -> Iterator[tuple[str, Any]]: """Yield ``("text", str)`` / ``("image", part)`` for recognized chat parts.""" for part in content: - if isinstance(part, str): - if part: - yield "text", part + if isinstance(part, str) and part: + yield "text", part elif isinstance(part, dict): ptype = _part_type(part) if ptype in _TEXT_PART_TYPES and _nonempty_str(part.get("text")): @@ -232,19 +228,19 @@ def _summarize_user_message_for_log(content: Any, *, sep: str = " ") -> str: return "" if isinstance(content, str): return content - if isinstance(content, list): - parts = list(_iter_content_parts(content)) - text_bits = [payload for kind, payload in parts if kind == "text"] - image_count = len(parts) - len(text_bits) - summary = sep.join(text_bits).strip() - if image_count: - note = f"[{image_count} image{'s' if image_count != 1 else ''}]" - summary = f"{note} {summary}" if summary else note - return summary - try: - return str(content) - except Exception: - return "" + if not isinstance(content, list): + try: + return str(content) + except Exception: + return "" + parts = list(_iter_content_parts(content)) + text_bits = [payload for kind, payload in parts if kind == "text"] + image_count = len(parts) - len(text_bits) + summary = sep.join(text_bits).strip() + if image_count: + note = f"[{image_count} image{'s' if image_count != 1 else ''}]" + summary = f"{note} {summary}" if summary else note + return summary # --- ID helpers --------------------------------------------------------------- @@ -296,11 +292,11 @@ def _canonical_call_id_from_fc(response_item_id: Any) -> Optional[str]: def _split_responses_tool_id(raw_id: Any) -> tuple[Optional[str], Optional[str]]: """Split a stored tool id into (call_id, response_item_id).""" value = raw_id.strip() if isinstance(raw_id, str) else "" - if not value: - return None, None if "|" in value: call_id, response_item_id = value.split("|", 1) return call_id.strip() or None, response_item_id.strip() or None + if not value: + return None, None return (None, value) if value.startswith("fc_") else (value, None) @@ -440,14 +436,13 @@ def _replay_message_items(msg: Dict[str, Any], *, is_github_responses: bool) -> return [] replayed: List[Dict[str, Any]] = [] for raw_item in codex_message_items: - if not ( + is_assistant_message = ( isinstance(raw_item, dict) and raw_item.get("type") == "message" and raw_item.get("role") == "assistant" and isinstance(raw_item.get("content"), list) - ): - continue + ) content = [ {"type": "output_text", "text": _str_or_empty(part.get("text", ""))} - for part in raw_item["content"] + for part in (raw_item["content"] if is_assistant_message else []) if isinstance(part, dict) and str(part.get("type") or "").strip() in _OUTPUT_TEXT_TYPES ] if content: @@ -571,13 +566,10 @@ def _chat_messages_to_responses_input( message_items = _replay_message_items(msg, is_github_responses=is_github_responses) emit(message_items, msg) if not message_items: - if content_parts: - emit([{"role": "assistant", "content": content_parts}], msg) - elif content_text.strip(): - emit([{"role": "assistant", "content": content_text}], msg) - elif reasoning_items: - # Every reasoning item needs a following item (else missing_following_item). - emit([{"role": "assistant", "content": ""}], msg) + # Every reasoning item needs a following item (else missing_following_item), hence the "" fallback. + fallback = content_parts or (content_text if content_text.strip() else "" if reasoning_items else None) + if fallback is not None: + emit([{"role": "assistant", "content": fallback}], msg) emit(_replay_tool_call_items(msg, start_index=len(items)), msg) # Native server-side compaction renders nothing placed before a compaction # item, so pre-checkpoint history is dead upload weight and the user's @@ -874,6 +866,13 @@ _PREFLIGHT_ALLOWED_KEYS = { } +def _optional_dict(api_kwargs: Dict[str, Any], key: str) -> Optional[Dict[str, Any]]: + value = api_kwargs.get(key) + if value is not None and not isinstance(value, dict): + raise ValueError(f"Codex Responses request '{key}' must be an object.") + return value + + def _preflight_codex_api_kwargs( api_kwargs: Any, *, @@ -916,23 +915,18 @@ def _preflight_codex_api_kwargs( value = api_kwargs.get(key) if accept(value): normalized[key] = coerce(value) if coerce else value - extra_headers = api_kwargs.get("extra_headers") + extra_headers = _optional_dict(api_kwargs, "extra_headers") if extra_headers is not None: - if not isinstance(extra_headers, dict): - raise ValueError("Codex Responses request 'extra_headers' must be an object.") if not all(_nonblank(key) for key in extra_headers): raise ValueError("Codex Responses request 'extra_headers' keys must be non-empty strings.") normalized_headers = {key.strip(): str(value) for key, value in extra_headers.items() if value is not None} if normalized_headers: normalized["extra_headers"] = normalized_headers - extra_body = api_kwargs.get("extra_body") - if extra_body is not None: - if not isinstance(extra_body, dict): - raise ValueError("Codex Responses request 'extra_body' must be an object.") - # Verbatim: xAI carries ``prompt_cache_key`` as a body-level field, and - # the SDK serializes extra_body without per-field checks. - if extra_body: - normalized["extra_body"] = dict(extra_body) + # extra_body is verbatim: xAI carries ``prompt_cache_key`` as a body-level + # field, and the SDK serializes extra_body without per-field checks. + extra_body = _optional_dict(api_kwargs, "extra_body") + if extra_body: + normalized["extra_body"] = dict(extra_body) allowed_keys = set(_PREFLIGHT_ALLOWED_KEYS) if allow_stream: stream = api_kwargs.get("stream") @@ -993,9 +987,7 @@ def _format_responses_error(error_obj: Any, response_status: str) -> str: code_str, message_str = field("code"), field("message") if code_str and message_str: return f"{code_str}: {message_str}" - if message_str or code_str or error_obj: - return message_str or code_str or str(error_obj) - return f"Responses API returned status '{response_status}'" + return message_str or code_str or (str(error_obj) if error_obj else f"Responses API returned status '{response_status}'") # --- Full response normalization ---------------------------------------------- @@ -1048,9 +1040,7 @@ def _capture_reasoning_item(item: Any, issuer_kind: Optional[str]) -> Optional[D summary = getattr(item, "summary", None) if isinstance(summary, list): raw_item["summary"] = [ - {"type": "summary_text", "text": text} - for text in (getattr(part, "text", None) for part in summary) - if isinstance(text, str) + {"type": "summary_text", "text": text} for text in (getattr(part, "text", None) for part in summary) if isinstance(text, str) ] return raw_item @@ -1157,10 +1147,9 @@ def _normalize_codex_response(response: Any, *, issuer_kind: Optional[str] = Non scan.scan(output, issuer_kind) tool_calls, reasoning_parts = scan.tool_calls, scan.reasoning_parts final_text = "\n".join(scan.content_parts).strip() - if not final_text and hasattr(response, "output_text") and (scan.saw_final_answer_phase or not scan.saw_commentary_phase): + if not final_text and (scan.saw_final_answer_phase or not scan.saw_commentary_phase): out_text = getattr(response, "output_text", "") - if isinstance(out_text, str): - final_text = out_text.strip() + final_text = out_text.strip() if isinstance(out_text, str) else final_text # Tool-call leak recovery: gpt-5.x sometimes emits the intended # ``function_call`` as plain Harmony text (``to=functions.foo {json}``) with # no structured item. Treat as incomplete so the continuation path From 97c0b89839a7a46466561001ea5159f7a90de96d Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Wed, 2 Sep 2026 19:04:48 -0700 Subject: [PATCH 08/13] refactor(agent/codex_runtime): compact docstrings/comments, keep every invariant and WHY (-66 LOC) --- agent/codex_runtime.py | 250 +++++++++++++++-------------------------- 1 file changed, 92 insertions(+), 158 deletions(-) diff --git a/agent/codex_runtime.py b/agent/codex_runtime.py index d59530785b..7ab05597b1 100644 --- a/agent/codex_runtime.py +++ b/agent/codex_runtime.py @@ -1,10 +1,8 @@ """Codex API runtime — App Server and Responses-API streaming paths. -Extracted from :class:`AIAgent`; every entry point takes the parent agent as its -first argument. ``run_codex_app_server_turn`` drives one ``codex app-server`` -subprocess turn (``codex_app_server`` api_mode); ``run_codex_stream`` runs one -streaming Codex Responses call (``codex_responses``); -``run_codex_create_stream_fallback`` is a legacy alias of the latter. +Every entry point takes the parent AIAgent first. ``run_codex_app_server_turn`` +drives one ``codex app-server`` subprocess turn; ``run_codex_stream`` runs one +streaming Codex Responses call (``run_codex_create_stream_fallback`` is its alias). """ from __future__ import annotations @@ -32,11 +30,10 @@ def _call_guarded(fn: Callable | None, fail_msg: str, *fail_args: Any, args: tup def _codex_request_failure_details(error: BaseException) -> tuple[int | None, str]: - """Return (serialized request bytes, exception class chain) for a failed request. + """(serialized request bytes, exception class chain) for a failed request. OpenAI connection exceptions retain the final ``httpx.Request``; its buffered - content gives the exact byte count without logging payloads or URLs. - """ + content gives the exact byte count without logging payloads or URLs.""" request_body_bytes: int | None = None exception_classes: list[str] = [] current: BaseException | None = error @@ -79,10 +76,8 @@ def _coerce_usage_int(value: Any) -> int: def _queue_token_counts(agent, fail_msg: str, *fail_extra: Any, counts: Callable[[], dict]) -> None: - """Enqueue per-call accounting for the SessionDB background writer (off the turn thread). - - ``counts`` is built lazily inside the guarded try so a stub agent without a - session DB never has its accounting attributes touched.""" + """Enqueue per-call accounting for the SessionDB background writer. ``counts`` is built + lazily inside the guarded try so a stub agent without a session DB is never touched.""" if not (agent._session_db and agent.session_id): return try: @@ -94,19 +89,16 @@ def _queue_token_counts(agent, fail_msg: str, *fail_extra: Any, counts: Callable def _record_codex_app_server_usage(agent, turn) -> dict[str, Any]: - """Translate Codex app-server token usage (thread/tokenUsage/updated) into Hermes accounting. + """Translate Codex app-server token usage into Hermes accounting. - Hermes' prompt bucket = uncached + cached input. The app-server protocol - exposes no cache-write tokens, so that bucket stays zero here. A turn with - no usage still counts as one API call for session/status accounting. - """ + Prompt bucket = uncached + cached input; the protocol exposes no cache-write + tokens. A turn with no usage still counts as one API call.""" agent.session_api_calls += 1 usage = getattr(turn, "token_usage_last", None) compressor = getattr(agent, "context_compressor", None) if not isinstance(usage, dict) or not usage: if compressor is not None and getattr(compressor, "awaiting_real_usage_after_compression", False): - # No usage cannot adjudicate the pending compaction; consume the marker - # so preflight deferral cannot stay latched. + # No usage cannot adjudicate the pending compaction; unlatch preflight deferral. compressor.update_from_response({}) _queue_token_counts( agent, "Codex app-server api-call persistence failed (session=%s): %s", @@ -167,11 +159,8 @@ def _record_codex_app_server_usage(agent, turn) -> dict[str, Any]: def _record_codex_app_server_compaction(agent, turn, *, approx_tokens: int | None = None, force: bool = False) -> bool: - """Record a Codex-native compaction boundary in Hermes state. - - The app-server owns the compacted thread, so local transcript rows are NOT - rewritten; only session event/usage counters record the boundary. - """ + """Record a Codex-native compaction boundary: the app-server owns the compacted thread, + so local transcript rows are NOT rewritten — only session event/usage counters.""" if not force and not getattr(turn, "compacted", False): return False thread_id = getattr(turn, "thread_id", None) or "" @@ -190,9 +179,7 @@ def _record_codex_app_server_compaction(agent, turn, *, approx_tokens: int | Non if compressor is not None: compressor.compression_count = getattr(compressor, "compression_count", 0) + 1 compressor.last_compression_rough_tokens = approx_tokens or 0 - # The boundary already happened server-side; its usage update (when supplied) - # is the same real-vs-real effectiveness verdict the normal path uses. Codex owns - # this summary, so a prior Hermes deterministic-fallback flag must not leak into it. + # Codex owns this summary: a prior Hermes deterministic-fallback flag must not leak into it. record_boundary = getattr(type(compressor), "record_completed_compaction", None) if callable(record_boundary): record_boundary(compressor, used_fallback=False) @@ -218,21 +205,18 @@ def _record_codex_app_server_compaction(agent, turn, *, approx_tokens: int | Non # --- Codex app-server → Hermes UI bridge ------------------------------------- -# The app-server runtime hands the whole turn to a subprocess and bypasses the -# Hermes tool loop, so gateway adapters would see nothing until the final answer. -# The bridge translates JSON-RPC notifications into the callbacks the standard -# runtime fires: tool_progress_callback("tool.started"|"tool.completed"), -# _fire_stream_delta(text), _emit_interim_assistant_message. +# The app-server bypasses the Hermes tool loop, so the bridge translates JSON-RPC +# notifications into the callbacks the standard runtime fires +# (tool_progress_callback, _fire_stream_delta, _emit_interim_assistant_message). # Item types that project to a Hermes tool_call (keep in sync with # agent/transports/codex_event_projector.py so UI names match recorded names). # webSearch is codex's built-in tool: no projector entry, still gets a bubble. _CODEX_TOOL_ITEM_TYPES = frozenset({"commandExecution", "fileChange", "mcpToolCall", "dynamicToolCall", "webSearch"}) -# Internal MCP server wrapping Hermes' native tools. Its inner dispatch runs in a -# separate subprocess with no tool_progress_callback, so the codex-level mcpToolCall -# IS the display event; the mcp.hermes-tools.* prefix is stripped because the -# user thinks of these as Hermes tools. +# Internal MCP server wrapping Hermes' native tools: its inner dispatch has no +# tool_progress_callback, so the codex-level mcpToolCall IS the display event and +# the mcp.hermes-tools.* prefix is stripped (users think of these as Hermes tools). _INTERNAL_MCP_SERVER = "hermes-tools" _STATIC_TOOL_NAMES = {"commandExecution": "exec_command", "fileChange": "apply_patch", "webSearch": "web_search"} @@ -336,14 +320,12 @@ def _stable_call_id(item: dict, name: str) -> str: def make_codex_app_server_event_bridge(agent) -> Callable[[dict], None]: """Build the ``on_event`` callback for ``CodexAppServerSession(on_event=...)``. - Tool items fire ``tool_progress_callback`` ("tool.started" / "tool.completed" - with duration=, is_error=, result=) plus the stable-ID ``tool_start_callback`` - / ``tool_complete_callback`` card hooks; deltas go to ``_fire_stream_delta`` / + Tool items fire ``tool_progress_callback`` plus the stable-ID ``tool_start_callback`` / + ``tool_complete_callback`` card hooks; deltas go to ``_fire_stream_delta`` / ``_fire_reasoning_delta``; a completed agentMessage goes to - ``_emit_interim_assistant_message`` (the gateway's ``already_streamed`` check - dedupes against streamed deltas). Every callback is guarded (DEBUG log) so a - buggy display hook cannot tear down the turn loop. - """ + ``_emit_interim_assistant_message`` (the gateway's ``already_streamed`` check dedupes + against streamed deltas). Every callback is guarded so a buggy display hook cannot + tear down the turn loop.""" # item_id -> (tool_name, args, started_monotonic); duration even when codex omits durationMs. started: dict[str, tuple[str, dict, float]] = {} @@ -431,9 +413,8 @@ def _close_codex_session(agent) -> None: def _consume_user_interrupt(agent, active: bool = True) -> tuple[bool, Any]: - """Mirror the conversation-loop finalizer's interrupt handoff: returns - (user_interrupted, interrupt_message) and clears the agent-level interrupt so a - hard stop cannot poison the next turn.""" + """(user_interrupted, interrupt_message); clears the agent-level interrupt so a hard + stop cannot poison the next turn (mirrors the conversation-loop finalizer).""" interrupted = bool(active and getattr(agent, "_interrupt_requested", False)) message = getattr(agent, "_interrupt_message", None) if interrupted else None if interrupted: @@ -454,9 +435,8 @@ def _ensure_codex_session(agent) -> None: except Exception: approval_callback = None # Gateway/cron have no UI for codex approval requests, so exec/apply_patch fail - # closed (silently decline) by default. Only an explicit approval bypass - # (approvals.mode: off, /yolo, --yolo, HERMES_YOLO_MODE) hands policy to codex's - # own sandbox profile (~/.codex/config.toml). + # closed by default. Only an explicit approval bypass (approvals.mode: off, /yolo, + # --yolo, HERMES_YOLO_MODE) hands policy to codex's own sandbox profile. auto_approve_requests = False try: from tools.approval import is_approval_bypass_active @@ -471,14 +451,11 @@ def _ensure_codex_session(agent) -> None: def _persist_projected_messages(agent, turn, messages: List[Dict[str, Any]]) -> None: - """Splice the projected {role, content, tool_calls, tool_call_id} entries into - ``messages`` and flush them to the session DB. + """Splice the projected messages into ``messages`` and flush them to the session DB. - Bypasses conversation_loop's per-step _persist_session(). The user turn was - flushed at turn start and the flush dedups via _DB_PERSISTED_MARKER, so only the - new codex rows are written. The agent stays the sole persister - (agent_persisted=True): a gateway re-write would re-INSERT the user turn. - """ + Bypasses conversation_loop's per-step _persist_session(); the flush dedups via + _DB_PERSISTED_MARKER so only the new codex rows are written. The agent stays the + sole persister (agent_persisted=True): a gateway re-write would re-INSERT the user turn.""" if not turn.projected_messages: return from agent.message_metadata import append_message @@ -492,8 +469,7 @@ def _persist_projected_messages(agent, turn, messages: List[Dict[str, Any]]) -> flush_ok = False logger.warning("codex app-server projected-message flush failed", exc_info=True) if flush_ok is False: - # Output already streamed, and agent_persisted cannot flip to False (gateway - # fallback would duplicate the user turn): surface the durability gap loudly. + # Output already streamed and agent_persisted cannot flip to False: surface the gap loudly. logger.warning( "codex app-server turn was delivered but could NOT be persisted to the session DB " "(session=%s) — this turn will be missing after restart/resume", @@ -505,8 +481,8 @@ def _finish_codex_turn( agent, turn, messages: List[Dict[str, Any]], *, original_user_message: Any, should_review_memory: bool, ) -> dict[str, Any]: """Post-turn bookkeeping mirroring the chat_completions loop; returns usage fields.""" - # run_conversation()'s pre-loop block already bumped _turns_since_memory / - # _user_turn_count; only _iters_since_skill (per tool iteration in the bypassed loop) is ours. + # run_conversation() already bumped _turns_since_memory / _user_turn_count; only + # _iters_since_skill (per tool iteration in the bypassed loop) is ours. agent._iters_since_skill = getattr(agent, "_iters_since_skill", 0) + turn.tool_iterations _record_codex_app_server_compaction(agent, turn) usage_result = _record_codex_app_server_usage(agent, turn) @@ -547,12 +523,10 @@ def run_codex_app_server_turn( ) -> Dict[str, Any]: """Hand the turn to a ``codex app-server`` subprocess and project its events into ``messages``. - Called from run_conversation() when agent.api_mode == "codex_app_server"; returns - the chat_completions result shape. The user message is ALREADY in ``messages`` — never append it again. - """ + Returns the chat_completions result shape. The user message is ALREADY in + ``messages`` — never append it again.""" # Defense in depth for compression.checkpoint_required: agent init refuses the - # combination, but api_mode is mutable. Fail closed before run_turn() can trigger a - # codex-owned compaction with no checkpoint. Explicit-True check matches compress_context(). + # combination, but api_mode is mutable. Explicit-True check matches compress_context(). if getattr(agent, "compression_checkpoint_required", False) is True: from agent.conversation_compression import _checkpoint_blocked raise _checkpoint_blocked( @@ -582,8 +556,7 @@ def run_codex_app_server_turn( return _turn_result( interrupt, messages, api_calls=1, completed=not turn.interrupted and turn.error is None, error=turn.error, final_response=turn.final_text, - # We flushed the projected rows ourselves (see _persist_projected_messages); - # True makes the gateway skip its own DB write, which would duplicate the user turn. + # We flushed the projected rows ourselves; the gateway must skip its own DB write. agent_persisted=True, codex_thread_id=turn.thread_id, codex_turn_id=turn.turn_id, @@ -606,12 +579,10 @@ def _turn_result( # --- Event-driven Responses streaming ----------------------------------------- -# The consumer Codex backend drifts independently of the openai SDK: the high-level -# ``responses.stream(...)`` helper rebuilds a typed Response from -# ``response.completed.response.output`` and crashes when that field is null. We go -# one level lower (``responses.create(stream=True)`` raw SSE events) and assemble the -# final response from ``response.output_item.done``, so the terminal ``output`` may -# be null / [] / a string / absent. +# The SDK's ``responses.stream(...)`` helper rebuilds a typed Response from +# ``response.completed.response.output`` and crashes when it is null. We consume raw +# ``responses.create(stream=True)`` SSE events and assemble the final response from +# ``output_item.done``, so the terminal ``output`` may be null / [] / a string / absent. def _event_field(event: Any, name: str, default: Any = None) -> Any: @@ -625,11 +596,8 @@ def _event_field(event: Any, name: str, default: Any = None) -> Any: def _raise_stream_error(event: Any) -> None: """Raise ``_StreamErrorEvent`` from a ``type=error`` SSE frame. - The spec puts code/message/param at the top level, but the OpenAI SDK and - several proxies nest them under ``error``. Read top-level first, then the - envelope, so the classifier sees the provider's real code/message. - ``run_agent`` is imported lazily to keep this module importable standalone. - """ + The spec puts code/message/param at the top level, but the SDK and several + proxies nest them under ``error``; read top-level first, then the envelope.""" from run_agent import _StreamErrorEvent nested = _event_field(event, "error") @@ -660,11 +628,9 @@ class _CodexResponseAssembler: """Assemble a Response-shaped ``SimpleNamespace`` from raw Responses SSE events. Only ``usage`` / ``status`` / ``id`` are read from the terminal frame — never - ``response.output``. Output items come from ``output_item.done``, or are - synthesized from text deltas, or settled from function calls announced via - ``output_item.added`` but never confirmed (some compatible backends omit - per-item done events on success). - """ + ``response.output``. Output items come from ``output_item.done``, or are synthesized + from text deltas, or settled from function calls announced via ``output_item.added`` + but never confirmed (some backends omit per-item done events on success).""" has_tool_calls = False next_output_sequence = 0 @@ -702,9 +668,8 @@ class _CodexResponseAssembler: _call_guarded(cb, f"Codex stream {label} raised", args=args) def _on_error(self, event: Any, event_type: str) -> None: - # ``error`` frames carry the provider's real failure reason (quota / model - # unavailable / rejected reasoning replay); surface them so the credential - # pool + error classifier see the body. + # ``error`` frames carry the provider's real failure reason; surface them so + # the credential pool + error classifier see the body. _raise_stream_error(event) def _on_item_added(self, event: Any, event_type: str) -> None: @@ -713,8 +678,8 @@ class _CodexResponseAssembler: self.active_message_phase = _message_phase(item) if item_type == "message" else None if self.active_message_phase == "commentary": self.commentary_text_deltas = [] - # Record first-observed ordering for EVERY announced item; the .done path must - # reuse it, or a mixed announced/pending stream without output_index values reorders the calls. + # Record first-observed ordering for EVERY announced item; .done must reuse it or a + # mixed announced/pending stream without output_index values reorders the calls. item_id = str(_event_field(item, "id", "")) if item_id and item_id not in self.announced_output_order: self.announced_output_order[item_id] = (self.next_output_sequence, _event_field(event, "output_index")) @@ -781,8 +746,8 @@ class _CodexResponseAssembler: if done_item is None: return self.output_items.append(done_item) - # Reuse the announced position when known; fresh tail sequence only for - # unannounced items. The .done event's own output_index wins over the announced one. + # Reuse the announced position when known (fresh tail sequence for unannounced + # items); the .done event's own output_index wins over the announced one. done_id = str(_event_field(done_item, "id", "")) announced_sequence, announced_index = self.announced_output_order.get(done_id, (None, None)) if announced_sequence is None: @@ -849,8 +814,7 @@ class _CodexResponseAssembler: # executable; malformed non-empty JSON passes through untouched. arguments=(pending["arguments"] or "").strip() or "{}", ))) - # output_index is optional and a partial ordering over mixed indexed/unindexed - # entries is ill-defined: protocol order only when every entry has an index, else wire order. + # output_index is optional: protocol order only when every entry has one, else wire order. if all(entry[0] is not None for entry in indexed): try: indexed.sort(key=lambda entry: entry[0]) @@ -861,8 +825,7 @@ class _CodexResponseAssembler: return [entry[2] for entry in indexed] def result(self) -> SimpleNamespace: - # Prefer .done items; with only plain text deltas (no tool calls), - # synthesize a single message item for downstream normalization. + # With only plain text deltas (no tool calls), synthesize one message item. output: List[Any] = list(self.output_items) if not output and self.text_deltas and not self.has_tool_calls: content = [SimpleNamespace(type="output_text", text="".join(self.text_deltas))] @@ -871,8 +834,7 @@ class _CodexResponseAssembler: # backends that omit per-item done events on a successful completion. if self.pending_function_calls and self.saw_response_completed: output = self._settled_output() - # No terminal frame AND no usable content = truncated / rejected stream, - # distinct from "completed with empty body" (what the SDK helper raised as RuntimeError). + # No terminal frame AND no usable content = truncated / rejected stream. if not self.saw_terminal and not output: raise RuntimeError("Codex Responses stream did not emit a terminal response") return SimpleNamespace( @@ -886,24 +848,17 @@ def _consume_codex_event_stream( event_iter: Any, *, model: str, on_text_delta=None, on_reasoning_delta=None, on_commentary_message=None, on_first_delta=None, on_event=None, interrupt_check=None, ) -> SimpleNamespace: - """Consume a Codex Responses SSE stream into a Response-shaped ``SimpleNamespace``. + """Consume a Codex Responses SSE stream into a Response-shaped ``SimpleNamespace`` + (see :class:`_CodexResponseAssembler`; ``status`` is ``completed`` when the stream + ended with content but no terminal frame; ``model`` comes from kwargs). - Result fields: ``output`` (items from ``output_item.done``, or a synthesized - message for plain-text turns), ``output_text``, ``usage``, ``status`` - (``completed`` when the stream ended with content but no terminal frame), - ``id``, ``incomplete_details``, ``error``, ``model`` (from kwargs; the wire - model name is not authoritative). - - Callbacks: ``on_text_delta(str)`` per output_text delta, suppressed once a - function_call is seen so tool-call turns don't bleed text into chat; - ``on_reasoning_delta(str)`` for reasoning and ``phase=analysis`` deltas (also - commentary when no commentary callback is given); ``on_commentary_message(str)`` - once per completed ``phase=commentary`` message, before any following tool item - executes; ``on_first_delta()`` one-shot on the first text delta; ``on_event(event)`` - every event, before any other processing; ``interrupt_check()`` True breaks the - loop early and may raise ``TimeoutError`` / ``InterruptedError`` for request - retirement that must not become a partial final response. - """ + Callbacks: ``on_text_delta`` per output_text delta, suppressed once a function_call + is seen; ``on_reasoning_delta`` for reasoning and ``phase=analysis`` deltas (also + commentary without a commentary callback); ``on_commentary_message`` once per completed + ``phase=commentary`` message, before any following tool item; ``on_first_delta`` + one-shot; ``on_event`` every event before any processing; ``interrupt_check()`` True + breaks the loop and may raise ``TimeoutError`` / ``InterruptedError`` for request + retirement that must not become a partial final response.""" assembler = _CodexResponseAssembler( model=model, on_text_delta=on_text_delta, on_reasoning_delta=on_reasoning_delta, on_commentary_message=on_commentary_message, on_first_delta=on_first_delta, @@ -922,12 +877,9 @@ def _consume_codex_event_stream( def _sanitize_consumer_codex_request(agent: Any, request: dict[str, Any]) -> dict[str, Any]: - """Drop fields the ChatGPT OAuth Codex endpoint rejects, at the final wire boundary. - - Runs after Relay / request middleware and explicit ``request_overrides`` so a - late ``prompt_cache_retention`` (top-level or nested in ``extra_body``, which - the SDK merges into the body) cannot turn a valid follow-up into an HTTP 400. - """ + """Drop fields the ChatGPT OAuth Codex endpoint rejects, at the final wire boundary + (after Relay / middleware / ``request_overrides``): a late ``prompt_cache_retention``, + top-level or nested in ``extra_body``, would otherwise HTTP 400 a valid follow-up.""" sanitized = dict(request) # getattr: run_codex_stream is also driven with stand-in agents carrying only the attrs a path needs. backend_predicate = getattr(agent, "_is_codex_backend", None) @@ -960,12 +912,8 @@ _SDK_TRANSFORM_BYPASS_FIELDS = ("input", "tools") def _is_plain_json_data(value: Any) -> bool: - """True when ``value`` is composed purely of JSON wire types. - - Hermes builds Codex payloads from JSON round-trips, so they are provably wire - format only when every node is plain JSON; anything else (pydantic models, - generators) must keep the typed SDK path. - """ + """True when ``value`` is purely JSON wire types; anything else (pydantic models, + generators) must keep the typed SDK path.""" if value is None or isinstance(value, (str, int, float, bool)): return True if isinstance(value, dict): @@ -978,13 +926,11 @@ def _is_plain_json_data(value: Any) -> bool: def _bypass_sdk_request_transform(stream_kwargs: dict) -> dict: """Route bulk payload fields around the SDK's ``maybe_transform``. - ``responses.create`` re-walks the whole body against the ResponseCreateParams - union graph with the GIL held — multi-MB conversations can wedge for hours and - starve the watchdogs (client-side, pre-network: no socket kill helps). The SDK - merges ``extra_body`` AFTER the transform, so moving already-wire-format bulk - fields there skips the walk and yields a byte-identical request. - HERMES_CODEX_SDK_TRANSFORM=1 disables. - """ + ``responses.create`` re-walks the whole body against the ResponseCreateParams union + with the GIL held — multi-MB conversations can wedge for hours, pre-network, where no + watchdog socket kill helps. The SDK merges ``extra_body`` AFTER the transform, so + moving wire-format bulk fields there yields a byte-identical request without the + walk. HERMES_CODEX_SDK_TRANSFORM=1 disables.""" if os.environ.get("HERMES_CODEX_SDK_TRANSFORM", "").strip().lower() in {"1", "true", "yes", "on"}: return stream_kwargs moved = { @@ -1005,12 +951,8 @@ def _bypass_sdk_request_transform(stream_kwargs: dict) -> dict: def run_codex_stream(agent, api_kwargs: dict, client: Any = None, on_first_delta=None): - """Execute one streaming Responses API request and return the final response. - - Uses ``responses.create(stream=True)`` raw event iteration rather than the - ``responses.stream(...)`` helper, so the SDK never reconstructs a typed - object from the terminal event's ``output`` field. - """ + """Execute one streaming Responses API request (raw ``responses.create(stream=True)`` + events, see module notes) and return the final response.""" import httpx as _httpx from openai import APIConnectionError as _APIConnectionError from agent import relay_llm @@ -1020,10 +962,9 @@ def run_codex_stream(agent, api_kwargs: dict, client: Any = None, on_first_delta model = api_kwargs.get("model") # Accumulate streamed text so callers / compat shims can read it. agent._codex_streamed_text_parts: list = [] - # Retirement token for THIS request, installed by ``interruptible_api_call``. - # A watchdog (TTFB / stream-idle / stale-call) that kills the connection - # clears the agent-level token, so a worker still draining frames can tell - # it was retired. ``None`` = no watchdog owns this call; every check passes. + # Retirement token for THIS request (installed by ``interruptible_api_call``). A + # watchdog that kills the connection clears the agent-level token, so a worker still + # draining frames can tell it was retired. ``None`` = no watchdog; every check passes. request_token = getattr(agent, "_active_codex_stream_request_token", None) # Delta-sink claim for the CURRENT physical attempt (None until the stream opens). writer_token = {"value": None} @@ -1044,9 +985,8 @@ def run_codex_stream(agent, api_kwargs: dict, client: Any = None, on_first_delta agent._touch_activity("receiving stream response") def _interrupt_or_superseded() -> bool: - # A retired request must NOT break out of the consume loop: that returns a - # partial ``final`` (status defaults to "completed") the caller would persist - # as finished. Raise so the watchdog's own TimeoutError is what the retry path sees. + # A retired request must NOT break out of the consume loop (that returns a partial + # ``final`` with status "completed"); raise so the watchdog's TimeoutError is seen. if not _request_is_current(): raise TimeoutError("Codex Responses stream request retired before terminal response") return bool(agent._interrupt_requested) @@ -1076,10 +1016,8 @@ def run_codex_stream(agent, api_kwargs: dict, client: Any = None, on_first_delta return False def _drain_for_finalizer(event_stream: Any) -> None: - # ``final`` is already assembled; draining the rest of the iterator - # only lets Relay run its response finalizer. A transport error here - # must NOT discard the completed, already-billed response or start - # a new physical request — warn and return it. + # ``final`` is already assembled; draining only lets Relay run its finalizer. A + # transport error here must NOT discard the completed, already-billed response. try: for _ignored in event_stream: pass @@ -1099,11 +1037,9 @@ def run_codex_stream(agent, api_kwargs: dict, client: Any = None, on_first_delta try: close_fn() except Exception: - # A failed close can leave this response's connection checked out of - # the httpx pool while the caller's finally reports a reuse-reason - # close — caching a client with a leaked connection. Poison the slot - # so close really closes the pool. ``client is None`` is the shared - # primary client, which is never reuse-cached and must not be force-shut. + # A failed close can leave this connection checked out of the httpx pool while + # the caller reuse-caches the client; poison the slot so close really closes the + # pool. ``client is None`` is the shared primary client — never force-shut. if client is not None: agent._abort_request_openai_client(active_client, reason="codex_stream_close_failed") on_commentary_message = ( @@ -1160,8 +1096,7 @@ def run_codex_stream(agent, api_kwargs: dict, client: Any = None, on_first_delta ) continue except RuntimeError: - # The consumer's "no terminal response" signal; Relay may still - # hold a completed response assembled by its finalizer. + # "No terminal response"; Relay may still hold a finalizer-assembled response. if event_stream is not None and event_stream.final_response is not None: return event_stream.final_response raise @@ -1184,8 +1119,7 @@ def run_codex_stream(agent, api_kwargs: dict, client: Any = None, on_first_delta def run_codex_create_stream_fallback(agent, api_kwargs: dict, client: Any = None): - """Backward-compatible alias: the primary path now does what this fallback did. - Kept public because tests and a few call sites reference it by name.""" + """Backward-compatible alias kept for tests and a few call sites.""" return run_codex_stream(agent, api_kwargs, client=client) From 98ec26620c311c91cfad8a2e04c5b8f44606c410 Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Wed, 2 Sep 2026 19:08:45 -0700 Subject: [PATCH 09/13] refactor(agent/bedrock_adapter,codex_responses_adapter): compact docstrings/comments, keep every WHY (-150 LOC) --- agent/bedrock_adapter.py | 239 ++++++++++-------------------- agent/codex_responses_adapter.py | 241 +++++++++++-------------------- 2 files changed, 165 insertions(+), 315 deletions(-) diff --git a/agent/bedrock_adapter.py b/agent/bedrock_adapter.py index f5f65ddb8a..448ac71a1d 100644 --- a/agent/bedrock_adapter.py +++ b/agent/bedrock_adapter.py @@ -1,11 +1,9 @@ -"""AWS Bedrock Converse API adapter for Hermes Agent. +"""AWS Bedrock Converse API adapter (boto3, optional dependency). -Talks to Bedrock through the native Converse API (boto3) so the AWS credential -chain (IAM roles, SSO profiles, env vars, instance metadata), cross-region -inference profiles, guardrails and control-plane model discovery all work -without API keys. OpenAI-format messages/tools are converted to Converse format -on the way in and responses normalized back to OpenAI-shaped objects. -Requires ``boto3`` (optional dependency). +The AWS credential chain, cross-region inference profiles, guardrails and +control-plane model discovery all work without API keys. OpenAI-format +messages/tools are converted to Converse on the way in and responses normalized +back to OpenAI-shaped objects. """ import base64 @@ -32,9 +30,8 @@ except Exception: _bedrock_runtime_client_cache: Dict[str, Any] = {} _bedrock_control_client_cache: Dict[str, Any] = {} -# Bedrock-hosted OpenAI GPT-5.x models are served from the Bedrock Mantle -# OpenAI-compatible Responses endpoint, not Converse. Keep the allowlist narrow -# so Converse-capable GPT-OSS models stay on the native path. +# Bedrock-hosted GPT-5.x models are served from the Bedrock Mantle OpenAI-compatible +# endpoint, not Converse. Narrow allowlist so GPT-OSS models stay on the native path. BEDROCK_OPENAI_RESPONSES_MODEL_IDS: Tuple[str, ...] = ( "openai.gpt-5.5", "openai.gpt-5.6-sol", @@ -161,7 +158,7 @@ class BedrockOpenAISigV4Auth(httpx.Auth): "Configure AWS_ACCESS_KEY_ID/AWS_SECRET_ACCESS_KEY, AWS_PROFILE, " "SSO, or an instance/task role." ) - # SigV4 must own Authorization: drop the SDK's placeholder bearer header, keep the rest. + # SigV4 must own Authorization: drop the SDK's placeholder bearer header. headers = { str(k): str(v) for k, v in request.headers.items() @@ -184,11 +181,8 @@ def build_bedrock_openai_http_client(region: str, *, timeout: Optional[float] = def configure_bedrock_openai_client_kwargs( client_kwargs: Dict[str, Any], *, timeout: Optional[float] = None, ) -> Dict[str, Any]: - """Install SigV4 auth on OpenAI SDK kwargs for Bedrock Mantle. - - Real API keys (``AWS_BEARER_TOKEN_BEDROCK``) keep the SDK's bearer auth; the - ``aws-sdk``/``no-key-required`` placeholders mean IAM credential-chain auth. - """ + """Install SigV4 auth on OpenAI SDK kwargs for Bedrock Mantle. Real API keys keep the + SDK's bearer auth; the ``aws-sdk``/``no-key-required`` placeholders mean IAM chain auth.""" base_url = str(client_kwargs.get("base_url") or "") if not is_bedrock_openai_base_url(base_url): return client_kwargs @@ -202,11 +196,9 @@ def configure_bedrock_openai_client_kwargs( # --- Stale-connection detection --- -# boto3 caches its HTTPS pool inside the client. A pooled connection killed out -# from under us (NAT timeout, VPN flap, RST) surfaces as botocore -# ConnectionClosedError / urllib3 ProtocolError, or as a bare AssertionError from -# urllib3's pool-state checks. Retrying with the same client reproduces the -# failure, so the fix is to evict the client. +# A pooled connection killed under boto3 (NAT timeout, VPN flap, RST) surfaces as a +# botocore/urllib3 transport error or a bare AssertionError from urllib3's pool +# checks; retrying the same client reproduces it, so the fix is to evict the client. _STALE_LIB_MODULE_PREFIXES = ("urllib3.", "botocore.", "boto3.") @@ -242,12 +234,9 @@ def is_stale_connection_error(exc: BaseException) -> bool: def is_streaming_access_denied_error(exc: BaseException) -> bool: - """True when IAM denied ``bedrock:InvokeModelWithResponseStream``. - - InvokeModel-only policies reject converse_stream() permanently, so callers - should fall back to non-streaming converse(). Message-based because the - AnthropicBedrock SDK wraps the same AWS response but preserves the action name. - """ + """True when IAM denied ``bedrock:InvokeModelWithResponseStream`` (permanent: callers + fall back to non-streaming converse()). Message-based because the AnthropicBedrock SDK + wraps the same AWS response but preserves the action name.""" msg = str(exc).lower() if "invokemodelwithresponsestream" not in msg: return False @@ -317,12 +306,9 @@ def resolve_bedrock_region(env: Optional[Dict[str, str]] = None) -> str: def resolve_bedrock_runtime_region(config: Optional[Dict[str, Any]] = None) -> str: - """``bedrock.region`` from config.yaml, else :func:`resolve_bedrock_region`. - - Every non-runtime Bedrock endpoint (auxiliary clients, picker discovery) must - use this so auxiliary calls never leave the primary runtime's region when - config and ambient AWS env/profile disagree. Pass *config* to avoid a disk read. - """ + """``bedrock.region`` from config.yaml, else :func:`resolve_bedrock_region`. Every + non-runtime Bedrock endpoint must use this so auxiliary calls never leave the primary + runtime's region when config and ambient AWS env disagree. Pass *config* to skip disk.""" if config is None: try: from hermes_cli.config import load_config_readonly @@ -346,8 +332,7 @@ def bedrock_model_ids_or_none() -> Optional[List[str]]: # --- Tool-calling / prompt-cache capability detection --- -# Models known to reject toolConfig with a ValidationException. Unknown models -# are assumed to support tools. +# Models known to reject toolConfig with a ValidationException; unknown models assumed OK. _NON_TOOL_CALLING_PATTERNS = [ "deepseek.r1", # DeepSeek R1 — reasoning only "deepseek-r1", # Alternate ID format @@ -356,10 +341,8 @@ _NON_TOOL_CALLING_PATTERNS = [ "amazon.titan-embed", # Embeddings ] -# cachePoint allowlist — inverted policy vs tools: an unsupported model rejects -# cachePoint with a ValidationException, so unknown models get NO cache markers. -# Claude normally uses the AnthropicBedrock SDK path and only reaches -# build_converse_kwargs under bearer-token auth. +# cachePoint allowlist — inverted policy vs tools: unknown models get NO cache markers +# (they reject cachePoint). Claude only reaches build_converse_kwargs under bearer auth. _CACHE_POINT_PATTERNS = [ "anthropic.claude", # bearer-token fallback path "amazon.nova", @@ -378,11 +361,10 @@ def _model_supports_prompt_cache(model_id: str) -> bool: # --- Server-verdict cachePoint suppression --- -# Bedrock's real cachePoint rule is per-family AND per-field (Nova accepts it in -# system/messages but hard-fails on toolConfig.tools), and any static table -# drifts as AWS ships new families. So when Bedrock names a placement as -# unpermitted we record that verdict, drop the marker from that placement for -# the rest of the process, and retry the rejected request once without it. +# Bedrock's cachePoint rule is per-family AND per-field (Nova accepts it in +# system/messages but hard-fails on toolConfig.tools) and any static table drifts, so +# when Bedrock names a placement as unpermitted we record the verdict, drop the marker +# there for the rest of the process, and retry once without it. CACHE_POINT_PLACEMENTS = ("tools", "system", "messages") @@ -395,13 +377,9 @@ _CACHE_POINT = {"cachePoint": {"type": "default"}} def cache_point_rejection_placement(exc: BaseException) -> Optional[str]: - """Return the Converse section whose cachePoint Bedrock refused, or None. - - Message-based on purpose: the JSON pointer in the ValidationException is the - only thing that says *which* section was rejected, and the same wording - arrives raw from botocore and wrapped by SDKs. An unlocalisable rejection - maps to "tools" — the only placement a supported family is known to refuse. - """ + """Converse section whose cachePoint Bedrock refused, or None. Message-based: the JSON + pointer in the ValidationException is the only thing naming the section, and the same + wording arrives raw or SDK-wrapped. Unlocalisable → "tools" (the only known refusal).""" msg = str(exc) lowered = msg.lower() if "cachepoint" not in lowered or ("not permitted" not in lowered and "extraneous" not in lowered): @@ -441,11 +419,8 @@ def _without_cache_points(blocks: Any) -> Optional[list]: def strip_cache_points(kwargs: Dict[str, Any], placement: str) -> Dict[str, Any]: - """Copy of Converse kwargs with ``placement``'s cachePoint removed. - - Returns the input unchanged (same object) when there was nothing to strip, - which is what callers use to decide a retry cannot help. - """ + """Copy of Converse kwargs with ``placement``'s cachePoint removed; the SAME object + back when nothing was stripped (callers use identity to decide a retry cannot help).""" if placement == "system": cleaned = _without_cache_points(kwargs.get("system")) return kwargs if cleaned is None else {**kwargs, "system": cleaned} @@ -467,11 +442,8 @@ def strip_cache_points(kwargs: Dict[str, Any], placement: str) -> Dict[str, Any] def recover_from_cache_point_rejection(exc: BaseException, kwargs: Dict[str, Any]) -> Optional[Dict[str, Any]]: - """Record Bedrock's cachePoint verdict and return retry kwargs, or None. - - None means the error was not a cachePoint rejection, or the marker was - already absent — retrying cannot change the outcome; caller must re-raise. - """ + """Record Bedrock's cachePoint verdict and return retry kwargs, or None when the error + was not a cachePoint rejection / the marker was already absent (caller re-raises).""" placement = cache_point_rejection_placement(exc) if placement is None: return None @@ -493,11 +465,8 @@ _REGIONAL_PREFIXES = ("global.", "us.", "eu.", "apac.", "ap.", "au.", "jp.", "ca def is_anthropic_bedrock_model(model_id: str) -> bool: - """True for Claude on Bedrock (``anthropic.claude-*`` with any regional prefix). - - These use the AnthropicBedrock SDK path (prompt caching, thinking budgets); - non-Claude models use the Converse path. - """ + """True for Claude on Bedrock (``anthropic.claude-*`` with any regional prefix); these + use the AnthropicBedrock SDK path, everything else the Converse path.""" model_lower = model_id.lower() for prefix in _REGIONAL_PREFIXES: if model_lower.startswith(prefix): @@ -536,11 +505,8 @@ def _safe_text(text) -> str: def _image_block_from_data_url(url: str) -> Dict: - """``data:;base64,...`` → Converse image block with RAW bytes. - - boto3 base64-encodes at the wire layer, so passing the base64 string through - double-encodes and Bedrock rejects it ("Failed to sanitize image"). - """ + """``data:;base64,...`` → Converse image block with RAW bytes (boto3 base64-encodes + at the wire layer; passing the string through double-encodes and Bedrock rejects it).""" header, _, data = url.partition(",") media_type = (header[5:].split(";")[0] if header.startswith("data:") else "") or "image/jpeg" try: @@ -554,11 +520,8 @@ def _image_block_from_data_url(url: str) -> Dict: def _convert_content_to_converse(content) -> List[Dict]: - """OpenAI message content (str or parts list) → Converse content blocks. - - Empty/whitespace text becomes the placeholder; remote image URLs (unsupported - by Converse) become a text reference. - """ + """OpenAI message content → Converse blocks. Blank text becomes the placeholder; remote + image URLs (unsupported by Converse) become a text reference.""" if not isinstance(content, list): return [{"text": _safe_text(content)}] blocks = [] @@ -602,11 +565,8 @@ def _decode_redacted(encoded) -> Optional[bytes]: def _replay_ordered_blocks(ordered_blocks: List) -> List[Dict]: - """Rebuild the exact Bedrock block sequence captured at normalization time. - - Redacted reasoning bytes are stored base64-encoded (JSON-safe sidecar); - undecodable entries are skipped. - """ + """Rebuild the exact Bedrock block sequence captured at normalization time; redacted + reasoning is stored base64 (JSON-safe sidecar) and undecodable entries are skipped.""" content_blocks: List[Dict] = [] for block in ordered_blocks: if not isinstance(block, dict): @@ -643,12 +603,9 @@ def _parse_tool_args(args) -> Any: def _assistant_blocks(msg: Dict, content) -> List[Dict]: - """Assistant message → Converse blocks. - - An ordered ``bedrock_content_blocks`` sidecar is authoritative and replayed - verbatim. Otherwise: redacted thinking from ``reasoning_details`` (so opaque - encrypted reasoning round-trips byte-for-byte), then text, then tool calls. - """ + """Assistant message → Converse blocks. An ordered ``bedrock_content_blocks`` sidecar is + authoritative; otherwise redacted thinking from ``reasoning_details`` (byte-for-byte + round-trip), then text, then tool calls.""" ordered_blocks = msg.get("bedrock_content_blocks") if isinstance(ordered_blocks, list) and ordered_blocks: content_blocks = _replay_ordered_blocks(ordered_blocks) @@ -671,13 +628,9 @@ def _assistant_blocks(msg: Dict, content) -> List[Dict]: def convert_messages_to_converse(messages: List[Dict]) -> Tuple[Optional[List[Dict]], List[Dict]]: - """OpenAI messages → ``(system_blocks_or_None, converse_messages)``. - - System messages become the system prompt; tool results become ``toolResult`` - blocks in a user turn. Converse requires strict user/assistant alternation - with a user turn first and last, so same-role neighbours are merged and - placeholder user turns are inserted at the ends. - """ + """OpenAI messages → ``(system_blocks_or_None, converse_messages)``. Tool results become + ``toolResult`` user blocks. Converse requires strict user/assistant alternation with a + user turn first and last: same-role neighbours merge, placeholder user turns pad the ends.""" system_blocks: List[Dict] = [] converse_msgs: List[Dict] = [] @@ -748,12 +701,8 @@ class _ResponseParts: self.reasoning_details.append({"type": "redacted_thinking", "data": encoded}) def build(self, ordered_blocks: List[Dict[str, Any]], usage_data: Dict[str, int], stop_reason: str, model: str) -> SimpleNamespace: - """Assemble the OpenAI-shaped response. - - Converse's inputTokens EXCLUDES cache read/write tokens (OpenAI's - prompt_tokens includes them), so they are added back here and the - Anthropic-named cache fields are surfaced for downstream normalize_usage(). - """ + """Assemble the OpenAI-shaped response. Converse's inputTokens EXCLUDES cache + read/write tokens (OpenAI's prompt_tokens includes them), so they are added back.""" msg = SimpleNamespace( role="assistant", content="\n".join(self.text_parts) if self.text_parts else None, @@ -782,11 +731,9 @@ class _ResponseParts: def normalize_converse_response(response: Dict) -> SimpleNamespace: - """Bedrock Converse response → OpenAI ``ChatCompletion``-shaped SimpleNamespace. - - Exposes ``.choices[0].message.{content,tool_calls,reasoning_content, - reasoning_details,bedrock_content_blocks}``, ``.choices[0].finish_reason``, ``.usage``. - """ + """Bedrock Converse response → OpenAI ``ChatCompletion``-shaped SimpleNamespace + (``.choices[0].message.{content,tool_calls,reasoning_content,reasoning_details, + bedrock_content_blocks}``, ``.choices[0].finish_reason``, ``.usage``).""" parts = _ResponseParts() ordered_blocks = [] for block in response.get("output", {}).get("message", {}).get("content", []): @@ -833,15 +780,11 @@ def stream_converse_with_callbacks( on_interrupt_check=None, on_event=None, ) -> SimpleNamespace: - """Process a boto3 ``converse_stream()`` response with real-time callbacks. - - ``on_text_delta`` only fires while no toolUse block has been seen (same - semantics as the Anthropic/chat_completions paths). ``on_interrupt_check`` - runs per event; True stops streaming. ``on_event`` is a wire-level liveness - signal fired for EVERY event before any branching; its exceptions are - swallowed so it can never abort the stream. Returns the - ``normalize_converse_response()`` shape. - """ + """Process a boto3 ``converse_stream()`` response with callbacks → the + ``normalize_converse_response()`` shape. ``on_text_delta`` only fires while no toolUse + block has been seen (as on the Anthropic/chat_completions paths); ``on_interrupt_check`` + True stops streaming; ``on_event`` fires for EVERY event before branching and its + exceptions are swallowed so a watchdog hook can never abort the stream.""" parts = _ResponseParts() stream_blocks: Dict[int, Dict[str, Any]] = {} current_block_index: Optional[int] = None @@ -941,12 +884,10 @@ def build_converse_kwargs( ) -> Dict[str, Any]: """Build kwargs for ``bedrock-runtime.converse()`` / ``converse_stream()``. - ``max_tokens=None`` omits ``inferenceConfig.maxTokens`` so Bedrock uses the - model's maximum output (default stays 4096). cachePoint markers go on system, - tools and the second-newest message (so the marker survives as only the tail - grows — mirrors the Anthropic system_and_3 strategy), each only if the model - supports caching and Bedrock has not rejected that placement. - """ + ``max_tokens=None`` omits ``maxTokens`` (model maximum; default stays 4096). cachePoint + markers go on system, tools and the second-newest message (survives as the tail grows — + mirrors Anthropic system_and_3), each only if the model supports caching and Bedrock has + not rejected that placement.""" system_prompt, converse_messages = convert_messages_to_converse(messages) cache_enabled = _model_supports_prompt_cache(model) @@ -968,8 +909,7 @@ def build_converse_kwargs( inference_config["stopSequences"] = stop_sequences converse_tools = convert_tools_to_converse(tools) if tools else [] if converse_tools: - # Non-tool-calling models (e.g. DeepSeek R1) reject toolConfig with a - # ValidationException → retry loop → failure. Strip tools and warn. + # Non-tool-calling models reject toolConfig (ValidationException → retry loop): strip and warn. if _model_supports_tool_use(model): if cache_here("tools"): converse_tools = converse_tools + [dict(_CACHE_POINT)] @@ -1001,11 +941,8 @@ def call_converse( stop_sequences: Optional[List[str]] = None, guardrail_config: Optional[Dict] = None, ) -> SimpleNamespace: - """Non-streaming Converse call → OpenAI-compatible response. - - Retries once without the rejected cachePoint placement; evicts the cached - client on stale-connection errors before re-raising. - """ + """Non-streaming Converse call → OpenAI-compatible response. Retries once without a + rejected cachePoint placement; evicts the cached client on stale-connection errors.""" client = _get_bedrock_runtime_client(region) kwargs = build_converse_kwargs( model, messages, tools, max_tokens, temperature, top_p, stop_sequences, guardrail_config, @@ -1093,13 +1030,8 @@ def _list_inference_profiles(client, filter_set: set, models: List[Dict[str, Any def discover_bedrock_models(region: str, provider_filter: Optional[List[str]] = None) -> List[Dict[str, Any]]: - """Discover Bedrock foundation models + inference profiles (cached 1h per region/filter). - - Each entry has ``id``, ``name``, ``provider`` ("inference-profile" for - profiles), ``input_modalities``, ``output_modalities``, ``streaming``. - Sorted with ``global.`` cross-region profiles first, then by name. - Returns [] when the client cannot be built. - """ + """Discover foundation models + inference profiles (cached 1h per region/filter), sorted + ``global.`` profiles first then by name; [] when the client cannot be built.""" import time cache_key = f"{region}:{','.join(sorted(provider_filter or []))}" cached = _discovery_cache.get(cache_key) @@ -1132,15 +1064,12 @@ def _extract_provider_from_arn(arn: str) -> str: # --- Bedrock model context lengths --- -# Static fallback for when the live probe is unavailable (used by -# agent/model_metadata.py). Keys match by longest substring, so versioned entries -# (opus-4-6/4-7/4-8) win over the generic "anthropic.claude-opus-4". +# Static fallback when the live probe is unavailable (agent/model_metadata.py). Keys match +# by longest substring, so versioned entries win over the generic "anthropic.claude-opus-4". BEDROCK_CONTEXT_LENGTHS: Dict[str, int] = { - # Anthropic Claude. Fable / Sonnet 5 / Opus 4.8-4.6 / Sonnet 4.6 are 1M GA; - # Sonnet 4.5 / Sonnet 4 lost their 1M beta and are 200K; Haiku 4.5 is 200K. - # The 1M entries must match agent/model_metadata.py DEFAULT_CONTEXT_LENGTHS - # or context compresses early. + # Anthropic Claude: 1M GA vs 200K. The 1M entries must match + # agent/model_metadata.py DEFAULT_CONTEXT_LENGTHS or context compresses early. **dict.fromkeys(( "anthropic.claude-fable-5", "anthropic.claude-fable", "anthropic.claude-sonnet-5", "anthropic.claude-opus-4-8", "anthropic.claude-opus-4-7", "anthropic.claude-opus-4-6", @@ -1168,23 +1097,17 @@ BEDROCK_CONTEXT_LENGTHS: Dict[str, int] = { # Default for unknown Bedrock models BEDROCK_DEFAULT_CONTEXT_LENGTH = 128_000 -# Probe padding targets (tokens). Tiered because a wildly oversized payload (5M -# tokens) yields an opaque InternalServerException instead of a clean -# ValidationException; stepping up discovers 2M+ windows without over-padding. +# Probe padding tiers (tokens): a wildly oversized payload yields an opaque +# InternalServerException instead of a clean ValidationException. _BEDROCK_PROBE_TIERS = (1_300_000, 2_200_000) _WORDS_PER_TOKEN = 0.9 # conservative: ensures the padded prompt clears the tier def probe_bedrock_context_length(model_id: str, region: str) -> Optional[int]: - """Discover a model's real context window by provoking a length error. - - No Bedrock metadata API exposes the window; the only authoritative source is - the ValidationException for an oversized prompt ("prompt is too long: 1300032 - tokens > 1000000 maximum"). Length validation happens before inference, so - the probe costs nothing. If a tier is accepted, that tier is returned as a - safe lower bound. Returns None if the probe could not run (no credentials, - network error, no parseable limit) so the caller falls back to the static table. - """ + """Discover a model's real context window by provoking a length error — the only + authoritative source ("prompt is too long: 1300032 tokens > 1000000 maximum"); length + validation runs before inference so the probe costs nothing. An accepted tier is + returned as a safe lower bound; None (no creds / network / unparseable) → static table.""" try: from agent.model_metadata import parse_context_limit_from_error except ImportError: # pragma: no cover — same package @@ -1220,13 +1143,9 @@ def probe_bedrock_context_length(model_id: str, region: str) -> Optional[int]: def get_bedrock_context_length(model_id: str, region: str = "", probe: bool = True) -> int: - """Context window: live probe (if ``probe`` and ``region``) → static table → default. - - The static table is a fallback only: AWS ships new versions faster than the - table tracks, and a stale substring match silently caps the window (e.g. a 1M - Opus pinned to 200K via "opus-4"). ``probe=False`` or an empty region skips - the network call for offline/display paths. - """ + """Context window: live probe (if ``probe`` and ``region``) → static table → default. The + table is fallback only: a stale substring match silently caps the window (a 1M Opus + pinned to 200K via "opus-4"). ``probe=False`` / empty region skips the network call.""" if probe and region: probed = probe_bedrock_context_length(model_id, region) if probed: diff --git a/agent/codex_responses_adapter.py b/agent/codex_responses_adapter.py index a1b5ecdb0b..d71ec7d77a 100644 --- a/agent/codex_responses_adapter.py +++ b/agent/codex_responses_adapter.py @@ -25,12 +25,9 @@ def _classify_responses_issuer( *, is_xai_responses: bool = False, is_github_responses: bool = False, is_codex_backend: bool = False, base_url: Optional[str] = None, ) -> str: - """Stable identifier for the endpoint that mints ``reasoning.encrypted_content``. - - Blobs are sealed to their issuer (replaying across endpoints yields HTTP 400 - ``invalid_encrypted_content``); stamping items lets replay drop foreign - blobs after a mid-conversation model switch. - """ + """Stable identifier for the endpoint that mints ``reasoning.encrypted_content``. Blobs + are sealed to their issuer (HTTP 400 ``invalid_encrypted_content`` across endpoints), so + stamping lets replay drop foreign blobs after a mid-conversation model switch.""" for flag, kind in ((is_xai_responses, "xai_responses"), (is_github_responses, "github_responses"), (is_codex_backend, "codex_backend")): if flag: return kind @@ -40,13 +37,11 @@ def _classify_responses_issuer( # Per-process throttle for the cross-issuer skip warning. _CROSS_ISSUER_WARN_EMITTED = False -# Codex/Harmony tool-call serialization leaked into assistant text when the -# model fails to emit a structured ``function_call``. +# Codex/Harmony tool-call serialization leaked into assistant text (no structured function_call). _TOOL_CALL_LEAK_PATTERN = re.compile(r"(?:^|[\s>|])to=functions\.[A-Za-z_][\w.]*", re.IGNORECASE) -# The Codex backend rejects requests containing these literal Harmony wire -# tokens (``invalid_prompt: Request blocked.``). Fullwidth bars survive -# format-character stripping while keeping the text legible. +# The Codex backend rejects literal Harmony wire tokens (``invalid_prompt: Request +# blocked.``). Fullwidth bars survive format-character stripping and stay legible. _HARMONY_CONTROL_TOKEN_RE = re.compile(r"<\|(start|end|channel|message|constrain|return|call)\|>") _FULLWIDTH_PIPE = "\uff5c" @@ -57,24 +52,21 @@ _ASSISTANT_IMAGE_PLACEHOLDER = "[Assistant image omitted during replay]" _INCOMPLETE_STATUSES = {"queued", "in_progress", "incomplete"} _RESPONSE_MESSAGE_STATUSES = {"completed", "incomplete", "in_progress"} -# input[].id / function names longer than this are a non-retryable 400 -# ("string too long"). Codex-issued assistant message ids can run 400+ chars; -# Hermes-minted ``msg_...`` ids stay under the cap and are kept for cache hits. +# input[].id / function names longer than this are a non-retryable 400 ("string too +# long"). Codex message ids can run 400+ chars; Hermes ``msg_...`` ids stay under the cap. _MAX_RESPONSES_ITEM_ID_LENGTH = 64 _VALID_RESPONSES_FN_NAME_RE = re.compile(r"[a-zA-Z0-9_-]{1,64}") -# Provider-executed built-in tools: declared on ``tools`` by ``type`` alone and -# run server-side, reporting via the ``*_call`` output items below. Preflight -# passes them through instead of rejecting them as "unsupported type". +# Provider-executed built-in tools: declared by ``type`` alone, run server-side, +# reported via the ``*_call`` output items below; preflight passes them through. _RESPONSES_BUILTIN_TOOL_TYPES = { "web_search", "web_search_preview", "file_search", "code_interpreter", "image_generation", "computer_use_preview", "local_shell", } -# Server-side ``*_call`` output items. xAI routinely leaves these at -# ``status="in_progress"`` even when the response is ``completed``, so they must -# NOT flip the incomplete verdict — otherwise every server-search turn burns 3 -# fruitless continuation retries. +# Server-side ``*_call`` output items. xAI leaves these ``in_progress`` even when the +# response is ``completed``, so they must NOT flip the incomplete verdict (else every +# server-search turn burns 3 fruitless continuation retries). _SERVER_SIDE_TOOL_CALL_TYPES = { "web_search_call", "file_search_call", "code_interpreter_call", "image_generation_call", "computer_call", "local_shell_call", "mcp_call", @@ -126,9 +118,8 @@ def _neutralize_harmony_tokens(text: str) -> str: replacement = rf"<{_FULLWIDTH_PIPE}\1{_FULLWIDTH_PIPE}>" if not any(unicodedata.category(char) == "Cf" for char in text): return _HARMONY_CONTROL_TOKEN_RE.sub(replacement, text) - # The backend strips Unicode format controls (e.g. U+200B) before its - # reserved-token check, so match on the visible text and rewrite the - # original spans — any Cf-hidden variant is neutralized the same way. + # The backend strips Unicode format controls (e.g. U+200B) before its reserved-token + # check, so match on the visible text and rewrite the original spans. original_positions = [i for i, char in enumerate(text) if unicodedata.category(char) != "Cf"] visible_text = "".join(text[i] for i in original_positions) result: List[str] = [] @@ -142,11 +133,8 @@ def _neutralize_harmony_tokens(text: str) -> str: def _neutralize_harmony_structure(value: Any) -> Any: - """Neutralize JSON-like values; normalize tuples to lists. - - A reserved token in an object *key* is rejected rather than rewritten — - renaming a key could desynchronize a tool schema from the executor contract. - """ + """Neutralize JSON-like values (tuples → lists). A reserved token in an object *key* is + rejected, not rewritten — renaming could desync a tool schema from the executor contract.""" if isinstance(value, str): return _neutralize_harmony_tokens(value) if isinstance(value, (list, tuple)): @@ -194,14 +182,11 @@ def _input_image_part(url: str, detail: Any) -> Dict[str, Any]: def _chat_content_to_responses_parts(content: Any, *, role: str = "user") -> List[Dict[str, Any]]: - """Convert chat-style multimodal content to Responses API input parts. + """Chat-style multimodal content → Responses API input parts ([] if not a list). - Text becomes ``input_text`` (user) or ``output_text`` (assistant) — the API - rejects the wrong type per role. ``input_image`` is only legal on user - messages; on assistant messages the image becomes a text marker (an - assistant ``input_image`` 400s on every replay and bricks the session). - Returns [] when ``content`` is not a list or has no recognized parts. - """ + Text becomes ``input_text`` (user) or ``output_text`` (assistant) — the API rejects + the wrong type per role. ``input_image`` is only legal on user messages; on assistant + messages it becomes a text marker (an assistant ``input_image`` 400s every replay).""" text_type = _text_type_for(role) converted: List[Dict[str, Any]] = [] for kind, payload in _iter_content_parts(content if isinstance(content, list) else []): @@ -217,13 +202,9 @@ def _chat_content_to_responses_parts(content: Any, *, role: str = "user") -> Lis def _summarize_user_message_for_log(content: Any, *, sep: str = " ") -> str: - """Flatten message content to plain text. - - Text parts are joined with ``sep`` (``" "`` for logs/spinner/trajectories; - ``"\\n"`` for memory providers that feed the text to regexes and text APIs); - images become a ``[N image(s)]`` marker. ``""`` for None/empty lists, - ``str(content)`` for other scalars. - """ + """Flatten message content to plain text: text parts joined with ``sep`` (``" "`` for + logs; ``"\\n"`` for memory providers feeding regexes), images → ``[N image(s)]`` + marker, ``""`` for None/empty, ``str(content)`` for other scalars.""" if content is None: return "" if isinstance(content, str): @@ -251,25 +232,19 @@ def _deterministic_call_id(fn_name: str, arguments: str, index: int = 0) -> str: def _clamp_responses_call_id(call_id: str) -> str: - """Keep ``call_id`` within the API's 64-char cap. - - The codex app-server namespaces MCP call ids past the cap, and the resulting - 400 replays on every turn. The surrogate is a pure function of the original - so a ``function_call`` and its ``function_call_output`` map to the same value. - """ + """Keep ``call_id`` within the API's 64-char cap (the codex app-server namespaces MCP + call ids past it). The surrogate is a pure function of the original so a + ``function_call`` and its ``function_call_output`` map to the same value.""" if len(call_id) <= _MAX_RESPONSES_ITEM_ID_LENGTH: return call_id return f"call_{hashlib.sha256(call_id.encode('utf-8', errors='replace')).hexdigest()[:32]}" def _sanitize_replayed_fn_name(name: str) -> str: - """Coerce a *replayed* ``function_call.name`` to ``^[a-zA-Z0-9_-]{1,64}$``. - - An invalid name stored in history otherwise 400s every later turn. Invalid - runs collapse to ``_``; an all-invalid name degrades to ``"fn"``. Apply ONLY - to replayed items, never to live tool definitions (schema names must match - the dispatch registry); pairing with the output is by call_id. - """ + """Coerce a *replayed* ``function_call.name`` to ``^[a-zA-Z0-9_-]{1,64}$`` (an invalid + stored name 400s every later turn). Invalid runs collapse to ``_``; all-invalid → "fn". + Apply ONLY to replayed items, never live tool definitions (schema names must match the + dispatch registry); pairing with the output is by call_id.""" if not isinstance(name, str): return "fn" if _VALID_RESPONSES_FN_NAME_RE.fullmatch(name): @@ -279,11 +254,8 @@ def _sanitize_replayed_fn_name(name: str) -> str: def _canonical_call_id_from_fc(response_item_id: Any) -> Optional[str]: - """Map an ``fc_…`` item id to its canonical ``call_``. - - Both sides of a replayed pair must derive the SAME call_id from an fc_-only - stored id, or an oversized pair clamps to two different surrogates. - """ + """Map an ``fc_…`` item id to its canonical ``call_``. Both sides of a replayed + pair must derive the SAME call_id, or an oversized pair clamps to two surrogates.""" if isinstance(response_item_id, str) and response_item_id.startswith("fc_") and len(response_item_id) > 3: return f"call_{response_item_id[3:]}" return None @@ -374,12 +346,9 @@ def _message_item( def _assistant_message_item( raw: Dict[str, Any], content: List[Dict[str, Any]], *, is_github_responses: bool, ) -> Dict[str, Any]: - """Replayable assistant ``message`` item from a stored one. - - ``id`` is kept only when short enough and never for GitHub Copilot, which - binds ids to a backend connection and 401s on a stale one; ``phase`` is - preserved per OpenAI's cache guidance. - """ + """Replayable assistant ``message`` item from a stored one. ``id`` is kept only when + short enough and never for GitHub Copilot (ids bind to a backend connection; stale → + 401); ``phase`` is preserved per OpenAI's cache guidance.""" item_id, phase = raw.get("id"), raw.get("phase") keep_id = not is_github_responses and _nonblank(item_id) and len(item_id.strip()) <= _MAX_RESPONSES_ITEM_ID_LENGTH return _message_item( @@ -393,13 +362,10 @@ def _replay_reasoning_items( ) -> List[Dict[str, Any]]: """Replay persisted encrypted reasoning/compaction items for one assistant turn. - Skips: duplicate ids; ``compaction`` checkpoints unless THIS request carries - ``context_management`` (a persisted checkpoint would otherwise erase - pre-checkpoint history on a model that cannot decrypt it); items stamped by - a different issuer (undecryptable → HTTP 400). Unstamped legacy items pass - through. ``id`` is stripped (store=False lookups 404) with the Hermes-only - ``_issuer_kind`` stamp. - """ + Skips duplicate ids, ``compaction`` checkpoints unless THIS request carries + ``context_management`` (else a persisted checkpoint erases pre-checkpoint history on a + model that cannot decrypt it), and items stamped by another issuer (HTTP 400). + Unstamped legacy items pass. ``id`` (store=False lookups 404) and ``_issuer_kind`` are stripped.""" global _CROSS_ISSUER_WARN_EMITTED codex_reasoning = msg.get("codex_reasoning_items") if not isinstance(codex_reasoning, list): @@ -506,31 +472,19 @@ def _chat_messages_to_responses_input( ) -> List[Dict[str, Any]]: """Convert internal chat-style messages to Responses input items. - ``is_xai_responses``: transport signature compatibility only; encrypted - reasoning IS replayed on xAI (cross-turn reasoning threading). - - ``replay_encrypted_reasoning``: per-session kill switch. Relays that reject a - replayed blob with HTTP 400 ``invalid_encrypted_content`` trigger - ``AIAgent._disable_codex_reasoning_replay``, which threads False here. - - ``is_github_responses``: drops ``id`` from replayed message items regardless - of length — Copilot binds ids to a backend connection (HTTP 401 on stale). - - ``current_issuer_kind``: per-item cross-issuer guard (only while replay is - enabled); items stamped by another endpoint are dropped, legacy items replay. - - ``native_compaction_eligible``: whether THIS request carries - ``context_management``. Gates both replaying ``compaction`` checkpoints and - ``prune_pre_checkpoint_items``. Checkpoints persist in the reasoning sidecar - across model swaps / compression flips / resume; without this gate one - checkpoint would delete pre-checkpoint history from every later request on a - model that cannot decrypt it. Dropping the checkpoint is lossless: local - history is never truncated by native compaction. - """ + ``is_xai_responses``: signature compatibility only (xAI DOES replay encrypted reasoning). + ``replay_encrypted_reasoning``: per-session kill switch, threaded False by + ``AIAgent._disable_codex_reasoning_replay`` after an ``invalid_encrypted_content`` 400. + ``is_github_responses``: drops ``id`` from replayed message items (Copilot 401s on stale ids). + ``current_issuer_kind``: cross-issuer guard; foreign-stamped items drop, legacy items replay. + ``native_compaction_eligible``: THIS request carries ``context_management``; gates both + replaying ``compaction`` checkpoints and ``prune_pre_checkpoint_items``. Checkpoints + persist across model swaps / compression flips / resume, so without the gate one + checkpoint would delete pre-checkpoint history on a model that cannot decrypt it. + Dropping it is lossless: local history is never truncated by native compaction.""" items: List[Dict[str, Any]] = [] - # Parallel to ``items``: the raw chat message each item came from. Pruning - # reads a summary carrier's provenance-tagged content from the source, since - # the converted item may be a lossy shape that no longer carries it. + # Parallel to ``items``: source chat message per item. Pruning reads a summary + # carrier's provenance from the source; the converted item may be a lossy shape. item_sources: List[Optional[Dict[str, Any]]] = [] seen_item_ids: set = set() @@ -571,11 +525,10 @@ def _chat_messages_to_responses_input( if fallback is not None: emit([{"role": "assistant", "content": fallback}], msg) emit(_replay_tool_call_items(msg, start_index=len(items)), msg) - # Native server-side compaction renders nothing placed before a compaction - # item, so pre-checkpoint history is dead upload weight and the user's - # plaintext asks / merged local summaries silently vanish. Keep the newest - # checkpoint first, retain pre-checkpoint USER and compression-SUMMARY - # messages within a token budget, leave the tail untouched. + # The server renders nothing placed before a compaction item, so pre-checkpoint + # history is dead weight and plaintext asks / merged summaries silently vanish. Keep + # the newest checkpoint first, retain pre-checkpoint USER and SUMMARY messages within + # a token budget, leave the tail untouched. if not native_compaction_eligible: return items from agent.native_compaction import prune_pre_checkpoint_items @@ -583,11 +536,8 @@ def _chat_messages_to_responses_input( class ResponsesRouteFlags(NamedTuple): - """Which special Responses-API route an agent is talking to. - - Single owner of the codex/xai/github predicates: every site must call - :func:`classify_responses_route` — inline string comparisons drift. - """ + """Which special Responses-API route an agent is talking to. Single owner of the + codex/xai/github predicates — every site must call :func:`classify_responses_route`.""" is_codex_backend: bool is_xai_responses: bool @@ -595,11 +545,8 @@ class ResponsesRouteFlags(NamedTuple): def classify_responses_route(agent: Any) -> ResponsesRouteFlags: - """Classify the agent's Responses route from provider + base URL. - - Host checks are exact-host-or-subdomain, never substring — - ``https://evil.com/models.github.ai`` must not classify as GitHub. - """ + """Classify the agent's Responses route from provider + base URL. Host checks are + exact-host-or-subdomain, never substring (``evil.com/models.github.ai`` is not GitHub).""" from utils import base_url_hostname provider = getattr(agent, "provider", None) base_url = str(getattr(agent, "base_url", "") or "") @@ -622,13 +569,9 @@ def estimate_native_responses_preflight_tokens( system_prompt: str = "", tools: Optional[List[Dict[str, Any]]] = None, ) -> Optional[int]: - """Estimate tokens for the checkpoint-pruned Responses payload. - - Counting the full durable transcript overstates a natively compacted session - several times over and fires local compression against history the request - will never send. None when native compaction is not proven eligible or - conversion fails — caller falls back to the generic (conservative) estimate. - """ + """Estimate tokens for the checkpoint-pruned Responses payload (the full transcript + overstates a natively compacted session and fires local compression needlessly). + None when native compaction is not proven eligible or conversion fails.""" if getattr(agent, "api_mode", None) != "codex_responses" or not isinstance(messages, list): return None is_codex_backend, is_xai_responses, is_github_responses = classify_responses_route(agent) @@ -842,8 +785,8 @@ def _preflight_tool(tool: Any, idx: int) -> Dict[str, Any]: } -# Optional scalar request fields, in wire order: (key, accept(value), coerce). -# Values failing ``accept`` are silently dropped (never an error). +# Optional scalar request fields, in wire order: (key, accept(value), coerce). Values +# failing ``accept`` are silently dropped. _PREFLIGHT_OPTIONAL_FIELDS: tuple[tuple[str, Callable[[Any], bool], Optional[Callable[[Any], Any]]], ...] = ( ("reasoning", lambda v: isinstance(v, dict), None), ("include", lambda v: isinstance(v, list), None), @@ -937,8 +880,8 @@ def _preflight_codex_api_kwargs( allowed_keys.add("stream") elif "stream" in api_kwargs: raise ValueError("Codex Responses stream flag is only allowed in fallback streaming requests.") - # Defense-in-depth slash-enum strip for xAI (rejects ``Qwen/Qwen3.5`` style - # enum values). Gated on the model name because native Codex accepts slashes. + # Defense-in-depth slash-enum strip for xAI (rejects ``Qwen/Qwen3.5`` enum values); + # gated on the model name because native Codex accepts slashes. is_xai_model = str(api_kwargs.get("model") or "").lower().startswith(("grok-", "x-ai/grok-")) if is_xai_model and normalized.get("tools"): try: @@ -979,8 +922,8 @@ def _extract_responses_reasoning_text(item: Any) -> str: def _format_responses_error(error_obj: Any, response_status: str) -> str: - """Human-readable ``": "`` for a ``response.error`` payload (dict or object), - falling back to whichever is present, then ``str(error_obj)``, then a status-based default.""" + """``": "`` for a ``response.error`` payload (dict or object), else whichever + is present, else ``str(error_obj)``, else a status-based default.""" def field(name: str) -> str: value = _field(error_obj, name) return str(value).strip() if isinstance(value, str) or value else "" @@ -1014,8 +957,7 @@ def _response_tool_call(item: Any, item_type: str, index: int) -> SimpleNamespac def _stamped_encrypted_item(item: Any, item_type: str, issuer_kind: Optional[str]) -> Optional[Dict[str, Any]]: - """``{type, encrypted_content[, _issuer_kind]}`` for replay, or None without a blob. - ``_issuer_kind`` lets a later model swap detect an endpoint that cannot decrypt the blob.""" + """``{type, encrypted_content[, _issuer_kind]}`` for replay, or None without a blob.""" encrypted = getattr(item, "encrypted_content", None) if not _nonempty_str(encrypted): return None @@ -1078,8 +1020,8 @@ class _OutputScan: if raw_item is not None: self.reasoning_items_raw.append(raw_item) elif item_type == "compaction": - # Native compaction checkpoint rides the codex_reasoning_items sidecar - # so it inherits persistence, replay, the cross-issuer guard and the kill switch. + # Compaction checkpoints ride the codex_reasoning_items sidecar (persistence, + # replay, cross-issuer guard and kill switch for free). raw_item = _stamped_encrypted_item(item, "compaction", issuer_kind) if raw_item is not None: self.reasoning_items_raw.append(raw_item) @@ -1100,9 +1042,8 @@ class _OutputScan: message_text = _extract_responses_message_text(item) if not message_text: return - # commentary/analysis text is mid-turn narration, never the final answer: - # keep it out of content but surface it via the reasoning channel. The - # exact item is still preserved for replay/cache continuity. + # commentary/analysis text is mid-turn narration, never the final answer: route it + # to the reasoning channel; the exact item is still preserved for replay/cache. (self.reasoning_parts if is_commentary_phase else self.content_parts).append(message_text) item_id = getattr(item, "id", None) self.message_items_raw.append(_message_item( @@ -1113,11 +1054,8 @@ class _OutputScan: def _normalize_codex_response(response: Any, *, issuer_kind: Optional[str] = None) -> tuple[Any, str]: - """Normalize a Responses API object to an assistant_message-like object. - - ``issuer_kind`` is stamped onto captured reasoning items so replay can drop - them once the active endpoint differs from the one that minted the blob. - """ + """Normalize a Responses API object to ``(assistant_message, finish_reason)``. + ``issuer_kind`` is stamped onto captured reasoning items for cross-issuer replay drops.""" response_status = _lower_or_none(getattr(response, "status", None)) incomplete_reason = _field(getattr(response, "incomplete_details", None), "reason", "") response_incomplete_content_filter = ( @@ -1125,8 +1063,7 @@ def _normalize_codex_response(response: Any, *, issuer_kind: Optional[str] = Non ) output = getattr(response, "output", None) if not isinstance(output, list) or not output: - # Codex can deliver the whole answer via stream events and return an - # empty output; fall back to output_text before raising. + # Codex can deliver the whole answer via stream events with an empty output. out_text = getattr(response, "output_text", None) if isinstance(out_text, str) and out_text.strip(): logger.debug( @@ -1135,8 +1072,7 @@ def _normalize_codex_response(response: Any, *, issuer_kind: Optional[str] = Non ) output = [_synthetic_message([SimpleNamespace(type="output_text", text=out_text.strip())])] elif response_incomplete_content_filter: - # Deterministic provider safety block, not a partial answer: an empty - # message makes finish_reason content_filter instead of burning continuations. + # Provider safety block, not a partial answer: finish content_filter, not incomplete. output = [_synthetic_message([])] else: raise RuntimeError("Responses API returned no output items") @@ -1150,11 +1086,9 @@ def _normalize_codex_response(response: Any, *, issuer_kind: Optional[str] = Non if not final_text and (scan.saw_final_answer_phase or not scan.saw_commentary_phase): out_text = getattr(response, "output_text", "") final_text = out_text.strip() if isinstance(out_text, str) else final_text - # Tool-call leak recovery: gpt-5.x sometimes emits the intended - # ``function_call`` as plain Harmony text (``to=functions.foo {json}``) with - # no structured item. Treat as incomplete so the continuation path - # re-elicits a real call; clear the text so the garbage is not surfaced - # (encrypted reasoning is preserved for the retry). + # Tool-call leak recovery: gpt-5.x sometimes emits the intended ``function_call`` as + # plain Harmony text (``to=functions.foo {json}``). Treat as incomplete so the + # continuation re-elicits a real call; clear the text so the garbage is not surfaced. leaked_tool_call_text = bool(final_text and not tool_calls and _TOOL_CALL_LEAK_PATTERN.search(final_text)) if leaked_tool_call_text: logger.warning( @@ -1163,11 +1097,9 @@ def _normalize_codex_response(response: Any, *, issuer_kind: Optional[str] = Non "Leaked snippet: %r", final_text[:300], ) final_text = "" - # Reasoning-channel answer salvage (xAI grok): grok-4.x sometimes puts the - # final answer inside the reasoning item after its ```` delimiter. - # Without salvage the reasoning-only rule marks the turn incomplete, and since - # these items carry no encrypted_content every continuation request is - # byte-identical to the failed one. Promote the delimited tail to content. + # xAI grok-4.x sometimes puts the final answer inside the reasoning item after a + # ```` delimiter; without salvage the reasoning-only rule marks the turn + # incomplete and every continuation is byte-identical. Promote the tail to content. if issuer_kind == "xai_responses" and not final_text and not tool_calls and reasoning_parts: joined_reasoning = "\n\n".join(reasoning_parts) marker = joined_reasoning.rfind("") @@ -1201,9 +1133,8 @@ def _normalize_codex_response(response: Any, *, issuer_kind: Optional[str] = Non ): finish_reason = "incomplete" elif (scan.reasoning_items_raw or reasoning_parts or scan.saw_reasoning_item) and not final_text: - # Reasoning-only response. For Codex/xAI/GitHub, reasoning-only with - # status=completed means "still thinking, needs another turn" → incomplete - # so the continuation path retries. Other backends: trust response.status — + # Reasoning-only: for Codex/xAI/GitHub, status=completed means "still thinking" → + # incomplete so the continuation retries. Other backends trust response.status — # forcing incomplete there stalls for minutes on a legitimately final state. trusted_final = ( response_status == "completed" and issuer_kind not in ("codex_backend", "xai_responses", "github_responses") From 17ce6b2686a3e3e8cdb97d80f2a6c474510af055 Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Wed, 2 Sep 2026 19:16:48 -0700 Subject: [PATCH 10/13] refactor(agent/bedrock_adapter,codex_responses_adapter): unify reasoning-block absorb and role-aware image part builders --- agent/bedrock_adapter.py | 42 +++++++++++++------------------- agent/codex_responses_adapter.py | 27 +++++++++++--------- 2 files changed, 32 insertions(+), 37 deletions(-) diff --git a/agent/bedrock_adapter.py b/agent/bedrock_adapter.py index 448ac71a1d..1d47bf1f89 100644 --- a/agent/bedrock_adapter.py +++ b/agent/bedrock_adapter.py @@ -696,9 +696,20 @@ class _ResponseParts: self.reasoning_details: List[Dict[str, Any]] = [] self.tool_calls: List[SimpleNamespace] = [] - def add_redacted(self, encoded: Optional[str]) -> None: + def absorb_reasoning(self, reasoning: Any, block: Dict[str, Any], on_text=None) -> None: + """Fold a Converse ``reasoningContent`` payload into the accumulators and ``block``.""" + if not isinstance(reasoning, dict): + return + thinking_text = reasoning.get("text", "") + if thinking_text: + self.reasoning_parts.append(str(thinking_text)) + if on_text: + on_text(thinking_text) + block["text"] = block.get("text", "") + str(thinking_text) + encoded = _encode_redacted(reasoning.get("redactedContent")) if encoded: self.reasoning_details.append({"type": "redacted_thinking", "data": encoded}) + block["redactedContentBase64"] = encoded def build(self, ordered_blocks: List[Dict[str, Any]], usage_data: Dict[str, int], stop_reason: str, model: str) -> SimpleNamespace: """Assemble the OpenAI-shaped response. Converse's inputTokens EXCLUDES cache @@ -741,18 +752,8 @@ def normalize_converse_response(response: Dict) -> SimpleNamespace: parts.text_parts.append(block["text"]) ordered_blocks.append({"text": block["text"]}) elif "reasoningContent" in block: - reasoning = block["reasoningContent"] - if not isinstance(reasoning, dict): - continue - thinking_text = reasoning.get("text", "") - encoded = _encode_redacted(reasoning.get("redactedContent")) - ordered_reasoning = {} - if thinking_text: - parts.reasoning_parts.append(str(thinking_text)) - ordered_reasoning["text"] = str(thinking_text) - if encoded: - parts.add_redacted(encoded) - ordered_reasoning["redactedContentBase64"] = encoded + ordered_reasoning: Dict[str, Any] = {} + parts.absorb_reasoning(block["reasoningContent"], ordered_reasoning) if ordered_reasoning: ordered_blocks.append({"reasoningContent": ordered_reasoning}) elif "toolUse" in block: @@ -804,19 +805,10 @@ def stream_converse_with_callbacks( current_text_buffer.clear() def on_reasoning(reasoning: Any) -> None: - if not isinstance(reasoning, dict): + if not isinstance(reasoning, dict) or not (reasoning.get("text", "") or _encode_redacted(reasoning.get("redactedContent"))): return - thinking_text = reasoning.get("text", "") - if thinking_text: - parts.reasoning_parts.append(str(thinking_text)) - if on_reasoning_delta: - on_reasoning_delta(thinking_text) - block = current_block({"reasoningContent": {}}).setdefault("reasoningContent", {}) - block["text"] = block.get("text", "") + str(thinking_text) - encoded = _encode_redacted(reasoning.get("redactedContent")) - if encoded: - parts.add_redacted(encoded) - current_block({"reasoningContent": {}}).setdefault("reasoningContent", {})["redactedContentBase64"] = encoded + block = current_block({"reasoningContent": {}}).setdefault("reasoningContent", {}) + parts.absorb_reasoning(reasoning, block, on_reasoning_delta) for event in event_stream.get("stream", []): if on_event is not None: try: diff --git a/agent/codex_responses_adapter.py b/agent/codex_responses_adapter.py index d71ec7d77a..4e8484d48b 100644 --- a/agent/codex_responses_adapter.py +++ b/agent/codex_responses_adapter.py @@ -181,6 +181,17 @@ def _input_image_part(url: str, detail: Any) -> Dict[str, Any]: return image_part +def _image_part_for_role(part: Dict[str, Any], role: str, *, keep_empty_url: bool) -> Optional[Dict[str, Any]]: + """Responses image part for ``role``: assistant → text placeholder (an assistant + ``input_image`` 400s every replay); user → ``input_image`` (None for an empty url unless kept).""" + if role == "assistant": + return {"type": "output_text", "text": _ASSISTANT_IMAGE_PLACEHOLDER} + url, detail = _resolve_image_ref(part) + if _nonempty_str(url): + return _input_image_part(url, detail) + return _input_image_part(str(url or ""), detail) if keep_empty_url else None + + def _chat_content_to_responses_parts(content: Any, *, role: str = "user") -> List[Dict[str, Any]]: """Chat-style multimodal content → Responses API input parts ([] if not a list). @@ -190,14 +201,11 @@ def _chat_content_to_responses_parts(content: Any, *, role: str = "user") -> Lis text_type = _text_type_for(role) converted: List[Dict[str, Any]] = [] for kind, payload in _iter_content_parts(content if isinstance(content, list) else []): + image_part = None if kind == "text" else _image_part_for_role(payload, role, keep_empty_url=False) if kind == "text": converted.append({"type": text_type, "text": payload}) - elif role == "assistant": - converted.append({"type": "output_text", "text": _ASSISTANT_IMAGE_PLACEHOLDER}) - else: - url, detail = _resolve_image_ref(payload) - if _nonempty_str(url): - converted.append(_input_image_part(url, detail)) + elif image_part is not None: + converted.append(image_part) return converted @@ -711,12 +719,7 @@ def _preflight_role_message(item: Dict[str, Any], idx: int, role: str, ctx: _Pre text = part.get("text", "") validated.append({"type": text_type, "text": ctx.sanitize_text(text if isinstance(text, str) else str(text or ""))}) elif ptype in _IMAGE_PART_TYPES: - if role == "assistant": - # Same output-message invariant as normal history replay. - validated.append({"type": "output_text", "text": _ASSISTANT_IMAGE_PLACEHOLDER}) - else: - url, detail = _resolve_image_ref(part) - validated.append(_input_image_part(url if isinstance(url, str) else str(url or ""), detail)) + validated.append(_image_part_for_role(part, role, keep_empty_url=True)) else: raise ValueError( f"Codex Responses input[{idx}].content[{part_idx}] has unsupported type {part.get('type')!r}." From 50dafd06b741af67bfb86801d8e6127cee5c4c26 Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Wed, 2 Sep 2026 19:18:13 -0700 Subject: [PATCH 11/13] style(agent/codex*,bedrock_adapter): restore one blank line after nested defs --- agent/bedrock_adapter.py | 3 +++ agent/codex_responses_adapter.py | 3 +++ agent/codex_runtime.py | 4 ++++ 3 files changed, 10 insertions(+) diff --git a/agent/bedrock_adapter.py b/agent/bedrock_adapter.py index 1d47bf1f89..0ce2c18258 100644 --- a/agent/bedrock_adapter.py +++ b/agent/bedrock_adapter.py @@ -639,6 +639,7 @@ def convert_messages_to_converse(messages: List[Dict]) -> Tuple[Optional[List[Di converse_msgs[-1]["content"].extend(blocks) else: converse_msgs.append({"role": role, "content": blocks}) + for msg in messages: role = msg.get("role", "") content = msg.get("content") @@ -809,6 +810,7 @@ def stream_converse_with_callbacks( return block = current_block({"reasoningContent": {}}).setdefault("reasoningContent", {}) parts.absorb_reasoning(reasoning, block, on_reasoning_delta) + for event in event_stream.get("stream", []): if on_event is not None: try: @@ -885,6 +887,7 @@ def build_converse_kwargs( def cache_here(placement: str) -> bool: return cache_enabled and cache_point_allowed(model, placement) + inference_config: Dict[str, Any] = {} if max_tokens is not None: inference_config["maxTokens"] = max_tokens diff --git a/agent/codex_responses_adapter.py b/agent/codex_responses_adapter.py index 4e8484d48b..5914d1b142 100644 --- a/agent/codex_responses_adapter.py +++ b/agent/codex_responses_adapter.py @@ -499,6 +499,7 @@ def _chat_messages_to_responses_input( def emit(new_items: List[Dict[str, Any]], msg: Dict[str, Any]) -> None: items.extend(new_items) item_sources.extend([msg] * len(new_items)) + for msg in messages: if not isinstance(msg, dict): continue @@ -563,6 +564,7 @@ def classify_responses_route(agent: Any) -> ResponsesRouteFlags: def _host_is(domain: str) -> bool: return hostname == domain or hostname.endswith("." + domain) + return ResponsesRouteFlags( is_codex_backend=provider == "openai-codex" or (_host_is("chatgpt.com") and "/backend-api/codex" in lower), is_xai_responses=provider in {"xai", "xai-oauth"} or hostname == "api.x.ai", @@ -930,6 +932,7 @@ def _format_responses_error(error_obj: Any, response_status: str) -> str: def field(name: str) -> str: value = _field(error_obj, name) return str(value).strip() if isinstance(value, str) or value else "" + code_str, message_str = field("code"), field("message") if code_str and message_str: return f"{code_str}: {message_str}" diff --git a/agent/codex_runtime.py b/agent/codex_runtime.py index 7ab05597b1..00ef3973b5 100644 --- a/agent/codex_runtime.py +++ b/agent/codex_runtime.py @@ -384,6 +384,7 @@ def make_codex_app_server_event_bridge(agent) -> Callable[[dict], None]: (_fire_tool_completed if completed else _fire_tool_started)(item) elif completed and item_type == "agentMessage": _fire_agent_message_completed(item) + handlers: dict[str, Callable[[dict], None]] = { "item/agentMessage/delta": lambda p: _fire_delta(p, "_fire_stream_delta"), "item/reasoning/delta": lambda p: _fire_delta(p, "_fire_reasoning_delta"), @@ -397,6 +398,7 @@ def make_codex_app_server_event_bridge(agent) -> Callable[[dict], None]: if handler is not None: params = note.get("params") or {} handler(params if isinstance(params, dict) else {}) + return on_event @@ -604,6 +606,7 @@ def _raise_stream_error(event: Any) -> None: def _error_field(name: str) -> Any: value = _event_field(event, name) return _event_field(nested, name) if value is None and nested is not None else value + raw_message = _error_field("message") message = (str(raw_message) if raw_message is not None else "stream emitted error event").strip() or "stream emitted error event" raise _StreamErrorEvent(message, code=_error_field("code"), param=_error_field("param")) @@ -1042,6 +1045,7 @@ def run_codex_stream(agent, api_kwargs: dict, client: Any = None, on_first_delta # pool. ``client is None`` is the shared primary client — never force-shut. if client is not None: agent._abort_request_openai_client(active_client, reason="codex_stream_close_failed") + on_commentary_message = ( _fenced(lambda text: agent._fire_streamed_codex_commentary(text)) if getattr(agent, "interim_assistant_callback", None) is not None and getattr(agent, "show_commentary", True) From db1214a406dfe239317ce665738af54796b4a78a Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Wed, 2 Sep 2026 19:23:16 -0700 Subject: [PATCH 12/13] refactor(agent/codex*,bedrock_adapter): share Mantle URL parse, billing identity, route flag unpacking; fold stream/tools preflight (-29 LOC) --- agent/bedrock_adapter.py | 58 +++++++++++++------------------- agent/codex_responses_adapter.py | 58 +++++++++++++------------------- agent/codex_runtime.py | 23 +++++-------- 3 files changed, 55 insertions(+), 84 deletions(-) diff --git a/agent/bedrock_adapter.py b/agent/bedrock_adapter.py index 0ce2c18258..8696a833b6 100644 --- a/agent/bedrock_adapter.py +++ b/agent/bedrock_adapter.py @@ -118,18 +118,22 @@ def bedrock_openai_base_url(region: str) -> str: return f"https://bedrock-mantle.{resolved}.api.aws/openai/v1" +def _mantle_url_parts(base_url: str) -> Tuple[Optional[str], str]: + """(region or None if not a Mantle host, normalized path) for a base URL.""" + parsed = urlparse(str(base_url or "")) + match = _BEDROCK_OPENAI_HOST_RE.match(parsed.hostname or "") + return (match.group(1) if match else None), (parsed.path or "").rstrip("/").lower() + + def bedrock_openai_region_from_base_url(base_url: str) -> Optional[str]: """Extract the AWS region from a Bedrock Mantle OpenAI base URL.""" - match = _BEDROCK_OPENAI_HOST_RE.match(urlparse(str(base_url or "")).hostname or "") - return match.group(1) if match else None + return _mantle_url_parts(base_url)[0] def is_bedrock_openai_base_url(base_url: str) -> bool: """True for Bedrock Mantle endpoints (bare host or /openai[/v1] path).""" - parsed = urlparse(str(base_url or "")) - if not _BEDROCK_OPENAI_HOST_RE.match(parsed.hostname or ""): - return False - return (parsed.path or "").rstrip("/").lower() in {"", "/openai", "/openai/v1"} + region, path = _mantle_url_parts(base_url) + return region is not None and path in {"", "/openai", "/openai/v1"} def resolve_bedrock_bearer_token(env: Optional[Dict[str, str]] = None) -> str: @@ -421,13 +425,12 @@ def _without_cache_points(blocks: Any) -> Optional[list]: def strip_cache_points(kwargs: Dict[str, Any], placement: str) -> Dict[str, Any]: """Copy of Converse kwargs with ``placement``'s cachePoint removed; the SAME object back when nothing was stripped (callers use identity to decide a retry cannot help).""" - if placement == "system": - cleaned = _without_cache_points(kwargs.get("system")) - return kwargs if cleaned is None else {**kwargs, "system": cleaned} - if placement == "tools": + if placement in ("system", "tools"): tool_config = kwargs.get("toolConfig") - cleaned = _without_cache_points((tool_config or {}).get("tools")) - return kwargs if cleaned is None else {**kwargs, "toolConfig": {**tool_config, "tools": cleaned}} + cleaned = _without_cache_points(kwargs.get("system") if placement == "system" else (tool_config or {}).get("tools")) + if cleaned is None: + return kwargs + return {**kwargs, "system": cleaned} if placement == "system" else {**kwargs, "toolConfig": {**tool_config, "tools": cleaned}} if placement == "messages": messages = kwargs.get("messages") if not isinstance(messages, list): @@ -479,15 +482,10 @@ def is_anthropic_bedrock_model(model_id: str) -> bool: def convert_tools_to_converse(tools: List[Dict]) -> List[Dict]: """OpenAI ``{"function": {...}}`` tool defs → Converse ``{"toolSpec": {...}}``.""" - result = [] - for t in tools or []: - fn = t.get("function", {}) - result.append({"toolSpec": { - "name": fn.get("name", ""), - "description": fn.get("description", ""), - "inputSchema": {"json": fn.get("parameters", {"type": "object", "properties": {}})}, - }}) - return result + return [{"toolSpec": { + "name": fn.get("name", ""), "description": fn.get("description", ""), + "inputSchema": {"json": fn.get("parameters", {"type": "object", "properties": {}})}, + }} for fn in (t.get("function", {}) for t in tools or [])] # Converse rejects empty OR whitespace-only text blocks, so the placeholder must be non-whitespace. @@ -513,10 +511,8 @@ def _image_block_from_data_url(url: str) -> Dict: raw_bytes = base64.b64decode(data) except Exception: raw_bytes = data.encode("utf-8") - return {"image": { - "format": media_type.split("/")[-1] if "/" in media_type else "jpeg", - "source": {"bytes": raw_bytes}, - }} + image_format = media_type.split("/")[-1] if "/" in media_type else "jpeg" + return {"image": {"format": image_format, "source": {"bytes": raw_bytes}}} def _convert_content_to_converse(content) -> List[Dict]: @@ -888,9 +884,7 @@ def build_converse_kwargs( def cache_here(placement: str) -> bool: return cache_enabled and cache_point_allowed(model, placement) - inference_config: Dict[str, Any] = {} - if max_tokens is not None: - inference_config["maxTokens"] = max_tokens + inference_config: Dict[str, Any] = {} if max_tokens is None else {"maxTokens": max_tokens} kwargs: Dict[str, Any] = {"modelId": model, "messages": converse_messages, "inferenceConfig": inference_config} if system_prompt: kwargs["system"] = system_prompt + [dict(_CACHE_POINT)] if cache_here("system") else system_prompt @@ -939,9 +933,7 @@ def call_converse( """Non-streaming Converse call → OpenAI-compatible response. Retries once without a rejected cachePoint placement; evicts the cached client on stale-connection errors.""" client = _get_bedrock_runtime_client(region) - kwargs = build_converse_kwargs( - model, messages, tools, max_tokens, temperature, top_p, stop_sequences, guardrail_config, - ) + kwargs = build_converse_kwargs(model, messages, tools, max_tokens, temperature, top_p, stop_sequences, guardrail_config) try: response = client.converse(**kwargs) except Exception as exc: @@ -1117,9 +1109,7 @@ def probe_bedrock_context_length(model_id: str, region: str) -> Optional[int]: oversized = "data " * int(tier_tokens / _WORDS_PER_TOKEN) try: client.converse( - modelId=model_id, - messages=[{"role": "user", "content": [{"text": oversized}]}], - inferenceConfig={"maxTokens": 8}, + modelId=model_id, messages=[{"role": "user", "content": [{"text": oversized}]}], inferenceConfig={"maxTokens": 8}, ) logger.debug( "Bedrock context probe for %s accepted ~%s-token prompt; " diff --git a/agent/codex_responses_adapter.py b/agent/codex_responses_adapter.py index 5914d1b142..22ecb79b8e 100644 --- a/agent/codex_responses_adapter.py +++ b/agent/codex_responses_adapter.py @@ -434,16 +434,15 @@ def _replay_tool_call_items(msg: Dict[str, Any], *, start_index: int) -> List[Di if not isinstance(tc, dict): continue fn = tc.get("function", {}) - fn_name = fn.get("name") + fn_name, arguments = fn.get("name"), fn.get("arguments", "{}") if not _nonblank(fn_name): continue call_id = _resolve_call_id( - tc.get("call_id"), tc.get("id"), fn_name, str(fn.get("arguments", "{}")), start_index + len(replayed), - canonicalize_fc=True, + tc.get("call_id"), tc.get("id"), fn_name, str(arguments), start_index + len(replayed), canonicalize_fc=True, ) replayed.append({ "type": "function_call", "call_id": _clamp_responses_call_id(call_id), - "name": _sanitize_replayed_fn_name(fn_name), "arguments": _coerce_arguments(fn.get("arguments", "{}")), + "name": _sanitize_replayed_fn_name(fn_name), "arguments": _coerce_arguments(arguments), }) return replayed @@ -584,21 +583,15 @@ def estimate_native_responses_preflight_tokens( None when native compaction is not proven eligible or conversion fails.""" if getattr(agent, "api_mode", None) != "codex_responses" or not isinstance(messages, list): return None - is_codex_backend, is_xai_responses, is_github_responses = classify_responses_route(agent) + route = classify_responses_route(agent)._asdict() from agent.native_compaction import native_compaction_context_management - if not native_compaction_context_management( - agent, is_codex_backend=is_codex_backend, is_xai_responses=is_xai_responses, - is_github_responses=is_github_responses, - ): + if not native_compaction_context_management(agent, **route): return None try: items = _chat_messages_to_responses_input( - messages, is_xai_responses=is_xai_responses, is_github_responses=is_github_responses, + messages, is_xai_responses=route["is_xai_responses"], is_github_responses=route["is_github_responses"], replay_encrypted_reasoning=bool(getattr(agent, "_codex_reasoning_replay_enabled", True)), - current_issuer_kind=_classify_responses_issuer( - is_xai_responses=is_xai_responses, is_github_responses=is_github_responses, - is_codex_backend=is_codex_backend, base_url=getattr(agent, "base_url", None), - ), + current_issuer_kind=_classify_responses_issuer(base_url=getattr(agent, "base_url", None), **route), native_compaction_eligible=True, ) except Exception: @@ -854,9 +847,7 @@ def _preflight_codex_api_kwargs( if not isinstance(tools, list): raise ValueError("Codex Responses request 'tools' must be a list when provided.") normalized_tools = [_preflight_tool(tool, idx) for idx, tool in enumerate(tools)] - if sanitize_harmony_tokens: - normalized_tools = _neutralize_harmony_structure(normalized_tools) - normalized["tools"] = normalized_tools + normalized["tools"] = _neutralize_harmony_structure(normalized_tools) if sanitize_harmony_tokens else normalized_tools if api_kwargs.get("store", False) is not False: raise ValueError("Codex Responses contract requires 'store' to be false.") for key, accept, coerce in _PREFLIGHT_OPTIONAL_FIELDS: @@ -875,16 +866,14 @@ def _preflight_codex_api_kwargs( extra_body = _optional_dict(api_kwargs, "extra_body") if extra_body: normalized["extra_body"] = dict(extra_body) - allowed_keys = set(_PREFLIGHT_ALLOWED_KEYS) - if allow_stream: - stream = api_kwargs.get("stream") - if stream is not None and stream is not True: - raise ValueError("Codex Responses 'stream' must be true when set.") - if stream is True: - normalized["stream"] = True - allowed_keys.add("stream") - elif "stream" in api_kwargs: + allowed_keys = _PREFLIGHT_ALLOWED_KEYS | ({"stream"} if allow_stream else set()) + stream = api_kwargs.get("stream") + if not allow_stream and "stream" in api_kwargs: raise ValueError("Codex Responses stream flag is only allowed in fallback streaming requests.") + if allow_stream and stream is not None and stream is not True: + raise ValueError("Codex Responses 'stream' must be true when set.") + if allow_stream and stream is True: + normalized["stream"] = True # Defense-in-depth slash-enum strip for xAI (rejects ``Qwen/Qwen3.5`` enum values); # gated on the model name because native Codex accepts slashes. is_xai_model = str(api_kwargs.get("model") or "").lower().startswith(("grok-", "x-ai/grok-")) @@ -1022,24 +1011,23 @@ class _OutputScan: reasoning_text = _extract_responses_reasoning_text(item) if reasoning_text: self.reasoning_parts.append(reasoning_text) - raw_item = _capture_reasoning_item(item, issuer_kind) - if raw_item is not None: - self.reasoning_items_raw.append(raw_item) + self._capture(_capture_reasoning_item(item, issuer_kind)) elif item_type == "compaction": # Compaction checkpoints ride the codex_reasoning_items sidecar (persistence, # replay, cross-issuer guard and kill switch for free). raw_item = _stamped_encrypted_item(item, "compaction", issuer_kind) - if raw_item is not None: - self.reasoning_items_raw.append(raw_item) - logger.info( - "Native Responses compaction item captured (%d chars encrypted).", - len(raw_item["encrypted_content"]), - ) + if self._capture(raw_item): + logger.info("Native Responses compaction item captured (%d chars encrypted).", len(raw_item["encrypted_content"])) elif item_type in {"function_call", "custom_tool_call"}: if item_type == "function_call" and item_status in _INCOMPLETE_STATUSES: continue self.tool_calls.append(_response_tool_call(item, item_type, len(self.tool_calls))) + def _capture(self, raw_item: Optional[Dict[str, Any]]) -> bool: + if raw_item is not None: + self.reasoning_items_raw.append(raw_item) + return raw_item is not None + def _message(self, item: Any, item_status: Optional[str]) -> None: normalized_phase = _lower_or_none(getattr(item, "phase", None)) is_commentary_phase = normalized_phase in {"commentary", "analysis"} diff --git a/agent/codex_runtime.py b/agent/codex_runtime.py index 00ef3973b5..99552342ad 100644 --- a/agent/codex_runtime.py +++ b/agent/codex_runtime.py @@ -96,16 +96,15 @@ def _record_codex_app_server_usage(agent, turn) -> dict[str, Any]: agent.session_api_calls += 1 usage = getattr(turn, "token_usage_last", None) compressor = getattr(agent, "context_compressor", None) + def billing(**extra): + return dict(model=agent.model, billing_provider=agent.provider, billing_base_url=agent.base_url, api_call_count=1, **extra) if not isinstance(usage, dict) or not usage: if compressor is not None and getattr(compressor, "awaiting_real_usage_after_compression", False): # No usage cannot adjudicate the pending compaction; unlatch preflight deferral. compressor.update_from_response({}) _queue_token_counts( agent, "Codex app-server api-call persistence failed (session=%s): %s", - counts=lambda: dict( - model=agent.model, billing_provider=agent.provider, billing_base_url=agent.base_url, - billing_mode="subscription_included", api_call_count=1, - ), + counts=lambda: billing(billing_mode="subscription_included"), ) return {} from agent.usage_pricing import CanonicalUsage, estimate_usage_cost @@ -148,11 +147,8 @@ def _record_codex_app_server_usage(agent, turn) -> dict[str, Any]: cost_fields = {"estimated_cost_usd": cost_usd, "cost_status": cost_result.status, "cost_source": cost_result.source} _queue_token_counts( agent, "Codex app-server token persistence failed (session=%s, tokens=%d): %s", total_tokens, - counts=lambda: dict( - **token_counts, **cost_fields, - billing_provider=agent.provider, billing_base_url=agent.base_url, - billing_mode="subscription_included" if cost_result.status == "included" else None, - model=agent.model, api_call_count=1, + counts=lambda: billing( + **token_counts, **cost_fields, billing_mode="subscription_included" if cost_result.status == "included" else None, ), ) return {**usage_dict, "last_prompt_tokens": prompt_tokens, **cost_fields} @@ -770,8 +766,7 @@ class _CodexResponseAssembler: self.saw_terminal = True resp_obj = _event_field(event, "response") if resp_obj is not None: - self.terminal_usage = _event_field(resp_obj, "usage") - self.terminal_response_id = _event_field(resp_obj, "id") + self.terminal_usage, self.terminal_response_id = _event_field(resp_obj, "usage"), _event_field(resp_obj, "id") rstatus = _event_field(resp_obj, "status") if isinstance(rstatus, str): self.terminal_status = rstatus @@ -779,8 +774,7 @@ class _CodexResponseAssembler: self.terminal_incomplete_details = _event_field(resp_obj, "incomplete_details") elif event_type == "response.failed": self.terminal_error = _event_field(resp_obj, "error") - if event_type == "response.completed": - self.saw_response_completed = True + self.saw_response_completed = self.saw_response_completed or event_type == "response.completed" self.terminal_status = self.terminal_status or event_type.removeprefix("response.") return True @@ -1003,8 +997,7 @@ def run_codex_stream(agent, api_kwargs: dict, client: Any = None, on_first_delta _log_codex_request_failure(agent, exc, stream_opened=writer_token["value"] is not None) def _codex_stream_created(_raw_stream: Any) -> None: - # Claim the delta sink for THIS physical attempt; a newer attempt - # supersedes this token and fences late deltas out of the turn. + # Claim the delta sink for THIS attempt; a newer attempt supersedes this token. writer_token["value"] = claim_stream_writer(agent) def _accept_codex_chunk(_chunk: Any) -> bool: From 72f1e2e2c101cf4249d8aa332d0c529cfa07f29a Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Wed, 2 Sep 2026 19:26:47 -0700 Subject: [PATCH 13/13] refactor(agent/codex_runtime,bedrock_adapter): fold assembler state, pending-call guard, toolUse block builder (-16 LOC) --- agent/bedrock_adapter.py | 30 ++++++++++--------------- agent/codex_runtime.py | 48 ++++++++++++++++------------------------ 2 files changed, 31 insertions(+), 47 deletions(-) diff --git a/agent/bedrock_adapter.py b/agent/bedrock_adapter.py index 8696a833b6..04628042d0 100644 --- a/agent/bedrock_adapter.py +++ b/agent/bedrock_adapter.py @@ -550,6 +550,10 @@ def _tool_use_block(tool_use_id, name, input_dict) -> Dict: return {"toolUse": {"toolUseId": tool_use_id, "name": name, "input": input_dict}} +def _tool_use_block_from(tu: Dict) -> Dict: + return _tool_use_block(tu.get("toolUseId", ""), tu.get("name", ""), tu.get("input", {})) + + def _decode_redacted(encoded) -> Optional[bytes]: """Strict base64 → bytes; None for empty/non-str/undecodable input.""" if not isinstance(encoded, str) or not encoded: @@ -585,8 +589,7 @@ def _replay_ordered_blocks(ordered_blocks: List) -> List[Dict]: if replay: content_blocks.append({"reasoningContent": replay}) elif "toolUse" in block and isinstance(block["toolUse"], dict): - tu = block["toolUse"] - content_blocks.append(_tool_use_block(tu.get("toolUseId", ""), tu.get("name", ""), tu.get("input", {}))) + content_blocks.append(_tool_use_block_from(block["toolUse"])) return content_blocks @@ -712,11 +715,9 @@ class _ResponseParts: """Assemble the OpenAI-shaped response. Converse's inputTokens EXCLUDES cache read/write tokens (OpenAI's prompt_tokens includes them), so they are added back.""" msg = SimpleNamespace( - role="assistant", - content="\n".join(self.text_parts) if self.text_parts else None, - tool_calls=self.tool_calls or None, + role="assistant", content="\n".join(self.text_parts) if self.text_parts else None, + tool_calls=self.tool_calls or None, reasoning_details=self.reasoning_details or None, reasoning_content="\n\n".join(self.reasoning_parts) if self.reasoning_parts else None, - reasoning_details=self.reasoning_details or None, bedrock_content_blocks=ordered_blocks or None, ) cache_read_tokens = usage_data.get("cacheReadInputTokens", 0) @@ -724,11 +725,8 @@ class _ResponseParts: output_tokens = usage_data.get("outputTokens", 0) prompt_tokens = usage_data.get("inputTokens", 0) + cache_read_tokens + cache_write_tokens usage = SimpleNamespace( - prompt_tokens=prompt_tokens, - completion_tokens=output_tokens, - total_tokens=prompt_tokens + output_tokens, - cache_read_input_tokens=cache_read_tokens, - cache_creation_input_tokens=cache_write_tokens, + prompt_tokens=prompt_tokens, completion_tokens=output_tokens, total_tokens=prompt_tokens + output_tokens, + cache_read_input_tokens=cache_read_tokens, cache_creation_input_tokens=cache_write_tokens, ) finish_reason = _STOP_REASON_TO_FINISH_REASON.get(stop_reason, "stop") if self.tool_calls and finish_reason == "stop": @@ -755,9 +753,8 @@ def normalize_converse_response(response: Dict) -> SimpleNamespace: ordered_blocks.append({"reasoningContent": ordered_reasoning}) elif "toolUse" in block: tu = block["toolUse"] - tool_use_id, name, tool_input = tu.get("toolUseId", ""), tu.get("name", ""), tu.get("input", {}) - ordered_blocks.append(_tool_use_block(tool_use_id, name, tool_input)) - parts.tool_calls.append(_tool_call_ns(tool_use_id, name, tool_input)) + ordered_blocks.append(_tool_use_block_from(tu)) + parts.tool_calls.append(_tool_call_ns(tu.get("toolUseId", ""), tu.get("name", ""), tu.get("input", {}))) return parts.build( ordered_blocks, response.get("usage", {}), response.get("stopReason", "end_turn"), response.get("modelId", ""), ) @@ -852,10 +849,7 @@ def stream_converse_with_callbacks( stop_reason = event["messageStop"].get("stopReason", "end_turn") elif "metadata" in event: meta_usage = event["metadata"].get("usage", {}) - usage_data = { - key: meta_usage.get(key, 0) - for key in ("inputTokens", "outputTokens", "cacheReadInputTokens", "cacheWriteInputTokens") - } + usage_data = {key: meta_usage.get(key, 0) for key in ("inputTokens", "outputTokens", "cacheReadInputTokens", "cacheWriteInputTokens")} flush_text() return parts.build([stream_blocks[i] for i in sorted(stream_blocks)], usage_data, stop_reason, "") diff --git a/agent/codex_runtime.py b/agent/codex_runtime.py index 99552342ad..8a09e6cda2 100644 --- a/agent/codex_runtime.py +++ b/agent/codex_runtime.py @@ -96,8 +96,10 @@ def _record_codex_app_server_usage(agent, turn) -> dict[str, Any]: agent.session_api_calls += 1 usage = getattr(turn, "token_usage_last", None) compressor = getattr(agent, "context_compressor", None) + def billing(**extra): return dict(model=agent.model, billing_provider=agent.provider, billing_base_url=agent.base_url, api_call_count=1, **extra) + if not isinstance(usage, dict) or not usage: if compressor is not None and getattr(compressor, "awaiting_real_usage_after_compression", False): # No usage cannot adjudicate the pending compaction; unlatch preflight deferral. @@ -109,12 +111,9 @@ def _record_codex_app_server_usage(agent, turn) -> dict[str, Any]: return {} from agent.usage_pricing import CanonicalUsage, estimate_usage_cost canonical_usage = CanonicalUsage( - input_tokens=_coerce_usage_int(usage.get("inputTokens")), - output_tokens=_coerce_usage_int(usage.get("outputTokens")), - cache_read_tokens=_coerce_usage_int(usage.get("cachedInputTokens")), - cache_write_tokens=0, - reasoning_tokens=_coerce_usage_int(usage.get("reasoningOutputTokens")), - raw_usage=usage, + input_tokens=_coerce_usage_int(usage.get("inputTokens")), output_tokens=_coerce_usage_int(usage.get("outputTokens")), + cache_read_tokens=_coerce_usage_int(usage.get("cachedInputTokens")), cache_write_tokens=0, + reasoning_tokens=_coerce_usage_int(usage.get("reasoningOutputTokens")), raw_usage=usage, ) prompt_tokens = canonical_usage.prompt_tokens total_tokens = _coerce_usage_int(usage.get("totalTokens")) or canonical_usage.total_tokens @@ -137,8 +136,7 @@ def _record_codex_app_server_usage(agent, turn) -> dict[str, Any]: for key, value in usage_dict.items(): setattr(agent, f"session_{key}", getattr(agent, f"session_{key}") + value) cost_result = estimate_usage_cost( - agent.model, canonical_usage, - provider=agent.provider, base_url=agent.base_url, api_key=getattr(agent, "api_key", ""), + agent.model, canonical_usage, provider=agent.provider, base_url=agent.base_url, api_key=getattr(agent, "api_key", ""), ) cost_usd = float(cost_result.amount_usd) if cost_result.amount_usd is not None else None if cost_usd is not None: @@ -344,12 +342,11 @@ def make_codex_app_server_event_bridge(agent) -> Callable[[dict], None]: prior = started.pop(item_id, None) if (item_id := item.get("id") or "") else None # Prefer codex's durationMs; else our started timestamp; else None # (some codex versions only emit completed for fast items). - duration: Any = None codex_ms = item.get("durationMs") if isinstance(codex_ms, (int, float)) and codex_ms >= 0: - duration = codex_ms / 1000.0 - elif prior is not None: - duration = time.monotonic() - prior[2] + duration: Any = codex_ms / 1000.0 + else: + duration = time.monotonic() - prior[2] if prior is not None else None result, is_error = _codex_item_completion_payload(item) agent_cb("tool_progress_callback", "tool_progress_callback raised on tool.completed for %s", name, args=("tool.completed", name, None, None), @@ -392,7 +389,7 @@ def make_codex_app_server_event_bridge(agent) -> Callable[[dict], None]: def on_event(note: dict) -> None: handler = handlers.get(note.get("method") or "") if isinstance(note, dict) else None if handler is not None: - params = note.get("params") or {} + params = note.get("params") handler(params if isinstance(params, dict) else {}) return on_event @@ -631,20 +628,15 @@ class _CodexResponseAssembler: from text deltas, or settled from function calls announced via ``output_item.added`` but never confirmed (some backends omit per-item done events on success).""" - has_tool_calls = False + has_tool_calls = first_delta_fired = saw_terminal = False next_output_sequence = 0 - first_delta_fired = False active_message_phase: str | None = None # Reasoning summary parts carry no separator; a summary_index change is where the blank line belongs. active_summary_index: Any = None terminal_status: str = "completed" - terminal_usage: Any = None - terminal_response_id: str = None - terminal_incomplete_details: Any = None - terminal_error: Any = None - saw_terminal = False - # terminal_status defaults to "completed", so settlement needs an - # explicitly observed response.completed frame (not EOF/interrupt). + terminal_usage = terminal_response_id = terminal_incomplete_details = terminal_error = None + # terminal_status defaults to "completed", so settlement needs an explicitly + # observed response.completed frame (not EOF/interrupt). saw_response_completed = False def __init__(self, *, model, on_text_delta, on_reasoning_delta, on_commentary_message, on_first_delta): @@ -717,17 +709,16 @@ class _CodexResponseAssembler: def _on_function_call(self, event: Any, event_type: str) -> None: self.has_tool_calls = True pending = self.pending_function_calls.get(str(_event_field(event, "item_id", ""))) + if pending is None: + return # the item itself lands on output_item.done if "delta" in event_type: - delta_args = _event_field(event, "delta", "") - if pending is not None and delta_args: - pending["arguments"] += delta_args + pending["arguments"] += _event_field(event, "delta", "") or "" elif event_type.endswith("function_call_arguments.done"): # Authoritative for the accumulated string; an explicit "" (zero-arg # call) counts, only a missing field keeps the streamed deltas. done_args = _event_field(event, "arguments", None) - if pending is not None and done_args is not None: + if done_args is not None: pending["arguments"] = str(done_args) - # Other function_call frames: the item itself lands on output_item.done. def _on_reasoning_delta(self, event: Any, event_type: str) -> None: reasoning_text = _event_field(event, "delta", "") @@ -750,8 +741,7 @@ class _CodexResponseAssembler: done_id = str(_event_field(done_item, "id", "")) announced_sequence, announced_index = self.announced_output_order.get(done_id, (None, None)) if announced_sequence is None: - announced_sequence = self.next_output_sequence - self.next_output_sequence += 1 + announced_sequence, self.next_output_sequence = self.next_output_sequence, self.next_output_sequence + 1 self.output_indexes.append(_event_field(event, "output_index", announced_index)) self.output_sequences.append(announced_sequence) # Confirmed by the authoritative done event; never settle it twice.