From 2598247ff55ecef33c094e84cde6d1ca0b5a55b6 Mon Sep 17 00:00:00 2001 From: Hermes Agent <127238744+teknium1@users.noreply.github.com> Date: Sat, 29 Aug 2026 20:15:35 -0700 Subject: [PATCH] Port from can1357/oh-my-pi#9566: uppercase finish_reason (STOP/MAX_TOKENS) no longer bypasses stop/length handling Some OpenAI-compatible gateways fronting Gemini backends emit the native uppercase finish reasons (STOP, MAX_TOKENS) instead of the lowercase OpenAI contract values. Every downstream comparison in Hermes uses lowercase literals, so an uppercase reason silently fell through: a clean STOP completion missed the stop handling and a MAX_TOKENS truncation never entered the length-recovery path. Adds normalize_finish_reason() as the single owner in agent/message_sanitization.py (case fold + alias map: max_tokens->length, end->stop, function_call->tool_calls) and wires it at both wire-intake choke points: ChatCompletionsTransport.normalize_response and the streaming chunk-capture loop in chat_completion_helpers. Non-string and empty values pass through unchanged so existing 'or "stop"' defaults and the Poolside int-reason path keep their behavior. --- agent/chat_completion_helpers.py | 6 +- agent/message_sanitization.py | 26 +++++ agent/transports/chat_completions.py | 5 +- .../test_finish_reason_normalization.py | 107 ++++++++++++++++++ 4 files changed, 141 insertions(+), 3 deletions(-) create mode 100644 tests/agent/transports/test_finish_reason_normalization.py diff --git a/agent/chat_completion_helpers.py b/agent/chat_completion_helpers.py index 6dda84e38a..2eac6fc7b3 100644 --- a/agent/chat_completion_helpers.py +++ b/agent/chat_completion_helpers.py @@ -37,7 +37,9 @@ from agent.gemini_native_adapter import is_native_gemini_base_url from agent.model_metadata import is_local_endpoint from agent.message_content import flatten_message_text from agent.message_metadata import append_message, stamp_message_timestamp -from agent.message_sanitization import (_sanitize_surrogates, _repair_tool_call_arguments) +from agent.message_sanitization import ( + _sanitize_surrogates, _repair_tool_call_arguments, normalize_finish_reason as _normalize_finish_reason, +) from agent.reasoning_summaries import separate_glued_reasoning_blocks from agent.stream_single_writer import claim_stream_writer, stream_writer_is_current from tools.terminal_tool_lifecycle import is_persistent_env @@ -2831,7 +2833,7 @@ class _StreamingCall(StreamingWaitMonitor): delta = choice.delta # Read finish_reason/usage BEFORE any content-shape `continue`: the SSE-echo # guard can swallow a merged finish chunk (vLLM standalone ':' tokens). - finish_reason = getattr(choice, "finish_reason", None) or finish_reason + finish_reason = _normalize_finish_reason(getattr(choice, "finish_reason", None)) or finish_reason if hasattr(chunk, "usage") and chunk.usage: usage_obj = chunk.usage diff --git a/agent/message_sanitization.py b/agent/message_sanitization.py index 4d9bb17ed2..b91c08a3eb 100644 --- a/agent/message_sanitization.py +++ b/agent/message_sanitization.py @@ -197,6 +197,32 @@ def close_interrupted_tool_sequence(messages: list, final_response: Any = None) return True +# finish_reason wire normalization. Some OpenAI-compatible gateways fronting +# Gemini backends emit the native uppercase reasons (STOP, MAX_TOKENS); every +# downstream comparison uses the lowercase OpenAI literals, so an uppercase +# reason silently skips stop handling and length recovery. Single owner — +# call at wire intake (transport normalize_response, stream chunk capture), +# never re-fold at comparison sites. +_FINISH_REASON_ALIASES = { + "max_tokens": "length", # Gemini-native / Anthropic-style cap reason + "end": "stop", # some gateways' clean-completion spelling + "function_call": "tool_calls", # OpenAI legacy pre-tools spelling +} + + +def normalize_finish_reason(raw: Any) -> Any: + """Fold a wire ``finish_reason`` to the lowercase OpenAI contract value. + + Non-string and empty values pass through unchanged (callers keep their + ``or "stop"`` defaults and the Poolside int-reason path); contract values + are returned byte-identical. + """ + if not isinstance(raw, str) or not raw: + return raw + lowered = raw.lower() + return _FINISH_REASON_ALIASES.get(lowered, lowered) + + def serialized_messages_bytes(messages: list) -> int: """Exact serialized byte size of ``messages`` (HTTP 413 is a BYTE-size error the token estimator, pricing images flat, cannot score). Non-serializable values fall back to diff --git a/agent/transports/chat_completions.py b/agent/transports/chat_completions.py index ae48873efe..2e4a6d95e1 100644 --- a/agent/transports/chat_completions.py +++ b/agent/transports/chat_completions.py @@ -13,6 +13,7 @@ from agent.reasoning_effort import ( KIMI_K3_EFFORTS, KIMI_K3_OVERRIDES, OPENAI_COMPAT_WIRE_EFFORTS, TOKENHUB_EFFORTS, clamp_effort, kimi_supported_efforts, requested_effort, ) +from agent.message_sanitization import normalize_finish_reason as _normalize_finish_reason from agent.moonshot_schema import is_moonshot_model, sanitize_moonshot_tools from agent.prompt_builder import DEVELOPER_ROLE_MODELS from agent.transports.base import ProviderTransport @@ -512,7 +513,9 @@ class ChatCompletionsTransport(ProviderTransport): choice = response.choices[0] msg = getattr(choice, "message", None) _fr = getattr(choice, "finish_reason", None) - finish_reason = (str(_fr) if isinstance(_fr, int) else _fr) or "stop" # Poolside returns int finish_reason + # Poolside returns int finish_reason; Gemini-fronting gateways return + # uppercase STOP / MAX_TOKENS — fold to the OpenAI contract here. + finish_reason = _normalize_finish_reason(str(_fr) if isinstance(_fr, int) else _fr) or "stop" tool_calls = None if getattr(msg, "tool_calls", None): diff --git a/tests/agent/transports/test_finish_reason_normalization.py b/tests/agent/transports/test_finish_reason_normalization.py new file mode 100644 index 0000000000..32f8b18312 --- /dev/null +++ b/tests/agent/transports/test_finish_reason_normalization.py @@ -0,0 +1,107 @@ +"""finish_reason wire normalization (port of oh-my-pi#9566). + +Some OpenAI-compatible gateways fronting Gemini backends emit the native +uppercase finish reasons (``STOP``, ``MAX_TOKENS``) instead of the lowercase +OpenAI contract values. Every downstream comparison in Hermes uses lowercase +literals, so without normalization a clean STOP completion misses the stop +handling and a MAX_TOKENS truncation never enters the length-recovery path. + +Covers the single owner (``normalize_finish_reason``) plus both wire-intake +choke points: the chat_completions transport ``normalize_response`` and the +streaming chunk-capture loop's alias import. +""" + +from types import SimpleNamespace + +import pytest + +from agent.message_sanitization import normalize_finish_reason +from agent.transports.chat_completions import ChatCompletionsTransport + + +# ── single owner ───────────────────────────────────────────────────── + + +@pytest.mark.parametrize( + "raw,expected", + [ + # Gemini-fronting gateways: native uppercase reasons + ("STOP", "stop"), + ("MAX_TOKENS", "length"), + # mixed case defensive fold + ("Stop", "stop"), + ("Tool_Calls", "tool_calls"), + # aliases + ("end", "stop"), + ("END", "stop"), + ("max_tokens", "length"), + ("function_call", "tool_calls"), + # contract values pass through byte-identical + ("stop", "stop"), + ("length", "length"), + ("tool_calls", "tool_calls"), + ("content_filter", "content_filter"), + ("incomplete", "incomplete"), + ], +) +def test_normalize_finish_reason_folds_to_contract(raw, expected): + assert normalize_finish_reason(raw) == expected + + +@pytest.mark.parametrize("raw", [None, "", 24, {"x": 1}]) +def test_normalize_finish_reason_passes_non_string_unchanged(raw): + # Callers keep their existing ``or "stop"`` defaults for falsy values; + # non-string values (Poolside int reasons pre-str()) are untouched. + assert normalize_finish_reason(raw) is raw + + +# ── transport intake choke point ───────────────────────────────────── + + +def _fake_response(finish_reason): + msg = SimpleNamespace(content="hello", tool_calls=None, refusal=None) + choice = SimpleNamespace(finish_reason=finish_reason, message=msg) + return SimpleNamespace(choices=[choice], usage=None, model="gemini-3-pro") + + +def test_transport_normalizes_uppercase_stop(): + transport = ChatCompletionsTransport() + normalized = transport.normalize_response(_fake_response("STOP")) + assert normalized.finish_reason == "stop" + + +def test_transport_normalizes_uppercase_max_tokens_to_length(): + transport = ChatCompletionsTransport() + normalized = transport.normalize_response(_fake_response("MAX_TOKENS")) + assert normalized.finish_reason == "length" + + +def test_transport_keeps_lowercase_contract_values(): + transport = ChatCompletionsTransport() + for reason in ("stop", "length", "tool_calls", "content_filter"): + normalized = transport.normalize_response(_fake_response(reason)) + assert normalized.finish_reason == reason + + +def test_transport_poolside_integer_reason_still_stringified(): + # Pre-existing Poolside behavior: int finish_reason → str, not folded. + transport = ChatCompletionsTransport() + normalized = transport.normalize_response(_fake_response(24)) + assert normalized.finish_reason == "24" + + +def test_transport_missing_reason_defaults_to_stop(): + transport = ChatCompletionsTransport() + normalized = transport.normalize_response(_fake_response(None)) + assert normalized.finish_reason == "stop" + + +# ── streaming intake choke point ───────────────────────────────────── + + +def test_streaming_capture_uses_shared_normalizer(): + # The streaming loop imports the same single owner under a private + # alias — verify the alias is the shared function, not a fork. + from agent import chat_completion_helpers as cch + + assert cch._normalize_finish_reason is normalize_finish_reason