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.
This commit is contained in:
@@ -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
|
||||
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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):
|
||||
|
||||
@@ -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
|
||||
Reference in New Issue
Block a user