From 48c531184f0112067a45f77b2f8028f1f95c8c53 Mon Sep 17 00:00:00 2001 From: Sean Date: Wed, 2 Sep 2026 12:04:12 +0800 Subject: [PATCH] fix(llm): keep evaluator-only config out of the request MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Evaluator config and provider request parameters share one bag — ``EvaluatorLLMArgs`` is ``extra="allow"``, so both land in ``model_extra`` — and everything in it was forwarded to ``chat.completions.create``. The SDK takes no unknown keyword arguments, so configuring a documented knob such as ``threshold`` (RAG, agent_eval, instruction_quality) or ``strictness`` made every call by that evaluator raise TypeError, which the caller then shapes into an ordinary "evaluation failed" — indistinguishable from a timeout. What may be sent is now decided by the SDK's own ``create()`` signature rather than a list someone has to remember to update. A denylist was tried first and missed two keys on the first pass; the signature travels with the SDK. Keys that are neither request parameters nor registered local knobs are dropped with a warning, so a misspelled key no longer fails silently. ``llm_custom_metric`` forwarded ``model_extra`` directly and so bypassed the filter entirely; it now goes through the same accessor. ``request_timeout`` and ``max_retries`` become configurable per evaluator, the latter reaching the client rather than the request body. Paths that had no timeout before only get one when configured — inventing a default there would fail long calls that work today. --- dingo/model/llm/base_litellm.py | 24 ++- dingo/model/llm/base_openai.py | 78 +++++++++- dingo/model/llm/llm_custom_metric.py | 10 +- .../model/llm/test_local_config_keys.py | 145 ++++++++++++++++++ 4 files changed, 250 insertions(+), 7 deletions(-) create mode 100644 test/scripts/model/llm/test_local_config_keys.py diff --git a/dingo/model/llm/base_litellm.py b/dingo/model/llm/base_litellm.py index b3ffbe93..e0fbf1cc 100644 --- a/dingo/model/llm/base_litellm.py +++ b/dingo/model/llm/base_litellm.py @@ -2,7 +2,7 @@ from dingo.config.input_args import EvaluatorLLMArgs from dingo.model.llm.base import LLMCallResult -from dingo.model.llm.base_openai import BaseOpenAI +from dingo.model.llm.base_openai import LOCAL_ONLY_CONFIG_KEYS, BaseOpenAI from dingo.utils.exception import ExceedMaxTokens @@ -78,13 +78,33 @@ def send_messages(cls, messages: List) -> str: import litellm model_name = cls.dynamic_config.model or "" - extra_params = cls.dynamic_config.model_extra or {} + # 这里刻意不走 ``get_request_extra_params``：那个方法按 OpenAI SDK 的 + # 签名放行，而 litellm 的入参集合更大（api_base、num_retries 等），拿 + # OpenAI 的签名当判据会把 litellm 自己的参数一并丢掉。 + # + # 换成排除表的代价也小得多：litellm 收 ``**kwargs`` 且开了 drop_params， + # 漏登记一个本地键最多是被它悄悄丢弃，不像 OpenAI SDK 那样直接抛 + # TypeError。要摘掉本地键，仍然是因为「悄悄丢弃」意味着这个键从未 + # 按配置的意图生效过。 + extra_params = { + k: v + for k, v in (cls.dynamic_config.model_extra or {}).items() + if k not in LOCAL_ONLY_CONFIG_KEYS + } cls.validate_config(extra_params) call_kwargs: dict = { "drop_params": True, **extra_params, } + # 摘掉还不够——调用方配了它们是要它们生效的。只在配了的时候传：这个 + # 路径原先没有超时与重试上限，凭空给默认值会改掉既有行为。 + request_timeout = cls.get_local_config_value("request_timeout") + if request_timeout is not None: + call_kwargs["timeout"] = request_timeout + max_retries = cls.get_local_config_value("max_retries") + if max_retries is not None: + call_kwargs["num_retries"] = max_retries if cls.dynamic_config.api_url: call_kwargs["api_base"] = cls.dynamic_config.api_url if cls.dynamic_config.key: diff --git a/dingo/model/llm/base_openai.py b/dingo/model/llm/base_openai.py index d6430069..4dcb9b30 100644 --- a/dingo/model/llm/base_openai.py +++ b/dingo/model/llm/base_openai.py @@ -1,5 +1,7 @@ +import inspect import json import time +from functools import lru_cache from typing import Dict, List from pydantic import ValidationError @@ -12,6 +14,37 @@ from dingo.utils import log from dingo.utils.exception import ConvertJsonError, ExceedMaxTokens +#: 单次请求的默认超时（秒）。评估器可以在 config 里用 ``request_timeout`` 覆盖它。 +#: 保持 90 是既有行为，不动；输入大、又用推理模型的场景应当显式配大。 +DEFAULT_REQUEST_TIMEOUT = 90 + +#: 一次请求最多重试几次，与 OpenAI SDK 的默认值一致，所以不配就是原来的行为。 +#: 它和超时要一起定：超时是单次尝试的代价，重试把这个代价乘起来。 +DEFAULT_MAX_RETRIES = 2 + +#: 已知「给评估器自己看」的配置键。它们和真正的请求参数共用 ``model_extra`` +#: 这一个口袋，所以转发给模型服务之前要摘出来。 +#: +#: 这张表**不是**过滤的判据——判据是 SDK 签名（见 ``_provider_request_params``）。 +#: 它只决定一个被丢弃的键要不要告警：登记过的是有意为之，不必出声；没登记的 +#: 多半是键名拼错了，值得说一句。 +#: +#: 判据之所以不用这张表：黑名单要靠人记得为每个新增的本地键登记一次，而这些 +#: 键分散在各个评估器里（``strictness`` 在 RAG、``agent_config`` 有六处在读）， +#: 漏一个的症状是该评估器每次调用必崩——``create()`` 不收未知关键字参数，抛出的 +#: TypeError 又会被上层塑形成「评估失败」，与超时长得一模一样。 +LOCAL_ONLY_CONFIG_KEYS = frozenset( + { + "request_timeout", # 本模块自己消费，见 send_messages + "max_retries", # 构造客户端时消费，不是请求体参数 + "threshold", # agent_eval / rag / instruction_quality 的判定阈值 + "strictness", # rag 的答案相关性 + "min_difficulty", + "max_difficulty", + "agent_config", # agent 评估器自己的编排配置 + } +) + class BaseOpenAI(BaseLLM): dynamic_config = EvaluatorLLMArgs() @@ -36,7 +69,11 @@ def create_client(cls): else: # 创建主 LLM 客户端 cls.client = OpenAI( - api_key=cls.dynamic_config.key, base_url=cls.dynamic_config.api_url + api_key=cls.dynamic_config.key, + base_url=cls.dynamic_config.api_url, + max_retries=cls.get_local_config_value( + "max_retries", DEFAULT_MAX_RETRIES + ), ) # 如果配置了 embedding_config，初始化 Embedding 客户端 @@ -85,7 +122,7 @@ def send_messages(cls, messages: List): extra_params = cls.get_request_extra_params() cls.validate_config(extra_params) - request_timeout = extra_params.pop("request_timeout", 90) + request_timeout = cls.get_local_config_value("request_timeout", DEFAULT_REQUEST_TIMEOUT) completions = cls.client.chat.completions.create( model=model_name, messages=messages, @@ -107,10 +144,43 @@ def send_messages(cls, messages: List): ), ) + @staticmethod + @lru_cache(maxsize=1) + def _provider_request_params() -> frozenset: + """哪些键是 SDK 真的收的请求参数。取自签名，不靠人维护。""" + from openai.resources.chat.completions import Completions + + return frozenset(inspect.signature(Completions.create).parameters) - {"self"} + @classmethod def get_request_extra_params(cls) -> Dict: - """Return evaluator extras that should be sent to the LLM provider.""" - return dict(cls.dynamic_config.model_extra or {}) + """Return evaluator extras that should be sent to the LLM provider. + + 放行判据见 ``LOCAL_ONLY_CONFIG_KEYS`` 的说明。过滤只放在这一处：这里是 + 唯一回答「什么该发给模型服务」的地方，写在别处的过滤会被下一个调用点忘掉。 + """ + accepted = cls._provider_request_params() + sendable: Dict = {} + unexpected: List[str] = [] + for key, value in (cls.dynamic_config.model_extra or {}).items(): + if key in accepted: + sendable[key] = value + elif key not in LOCAL_ONLY_CONFIG_KEYS: + unexpected.append(key) + if unexpected: + # 丢弃而不是转发，因为转发必崩；但要出声，否则一个拼错的键名会 + # 安静地不生效，比崩还难查。 + log.warning( + "evaluator config keys are not request parameters and were not sent: %s", + ", ".join(sorted(unexpected)), + ) + return sendable + + @classmethod + def get_local_config_value(cls, key: str, default=None): + """Read a knob that steers the evaluator itself, never the request.""" + extras = (cls.dynamic_config.model_extra or {}) if cls.dynamic_config else {} + return extras.get(key, default) @staticmethod def _usage_value(data, key: str): diff --git a/dingo/model/llm/llm_custom_metric.py b/dingo/model/llm/llm_custom_metric.py index 1c049098..15ef8e0b 100644 --- a/dingo/model/llm/llm_custom_metric.py +++ b/dingo/model/llm/llm_custom_metric.py @@ -99,9 +99,17 @@ def send_messages(self, messages: List): else: model_name = self.client.models.list().data[0].id - extra_params = self.dynamic_config.model_extra + # 走和 BaseOpenAI 同一个入口。同一层里两处调用、一处过滤一处不过滤， + # 是这类问题最容易复发的形态。 + extra_params = self.get_request_extra_params() self.validate_config(extra_params) + request_timeout = self.get_local_config_value("request_timeout") + if request_timeout is not None: + # 只在配了的时候传。这个类原先没有任何超时，凭空给它一个默认值 + # 会让本来跑得通的长调用开始失败。 + extra_params["timeout"] = request_timeout + completions = self.client.chat.completions.create( model=model_name, messages=messages, diff --git a/test/scripts/model/llm/test_local_config_keys.py b/test/scripts/model/llm/test_local_config_keys.py new file mode 100644 index 00000000..bea7a747 --- /dev/null +++ b/test/scripts/model/llm/test_local_config_keys.py @@ -0,0 +1,145 @@ +"""评估器自己的配置键不能被当成请求参数发给模型服务。 + +为什么见 ``base_openai.LOCAL_ONLY_CONFIG_KEYS`` 的说明。这里钉住的是行为： +放行按 SDK 签名、本地键不外发、没登记的键丢弃但告警。 +""" + +from dingo.config.input_args import EvaluatorLLMArgs +from dingo.model.llm.agent_eval.llm_agent_step_efficiency import LLMAgentStepEfficiency +from dingo.model.llm.base_openai import DEFAULT_MAX_RETRIES, DEFAULT_REQUEST_TIMEOUT, LOCAL_ONLY_CONFIG_KEYS, BaseOpenAI + + +class _Recorder: + """记下发给 provider 的参数，不联网。""" + + def __init__(self): + self.kwargs = {} + + class _Completions: + @staticmethod + def create(**kw): + self.kwargs = kw + raise _Captured + + class _Chat: + completions = _Completions() + + self.chat = _Chat() + + +class _Captured(Exception): + pass + + +class _FakeOpenAI: + """记下构造客户端时用了哪些参数。""" + + built: dict = {} + + def __init__(self, **kwargs): + type(self).built = kwargs + + +def _send(cls, **config): + cls.dynamic_config = EvaluatorLLMArgs( + model="m", key="k", api_url="http://example.invalid", **config + ) + recorder = _Recorder() + cls.client = recorder + try: + cls.send_messages([{"role": "user", "content": "hi"}]) + except _Captured: + pass + return recorder.kwargs + + +def test_threshold_is_not_forwarded_to_the_provider(): + sent = _send(LLMAgentStepEfficiency, threshold=0.4) + + assert "threshold" not in sent, ( + "threshold 是判定阈值，不是请求参数；转发出去会让 SDK 抛 TypeError" + ) + # 仍然要能被评估器自己读到，否则这个配置项就等于没了 + assert LLMAgentStepEfficiency._get_threshold() == 0.4 + + +def test_request_timeout_steers_the_call_but_is_not_a_body_param(): + sent = _send(LLMAgentStepEfficiency, request_timeout=180) + + assert sent["timeout"] == 180 + assert "request_timeout" not in sent + + +def test_default_timeout_applies_when_unconfigured(): + sent = _send(LLMAgentStepEfficiency) + + assert sent["timeout"] == DEFAULT_REQUEST_TIMEOUT + + +def test_real_request_params_still_reach_the_provider(): + """过滤只针对本地键。把真正的请求参数一起挡掉，是另一种同样糟的失败。""" + sent = _send(LLMAgentStepEfficiency, temperature=0.2, max_tokens=1000) + + assert sent["temperature"] == 0.2 + assert sent["max_tokens"] == 1000 + + +def test_every_local_key_is_filtered(): + """名单里的每一个键都要真的被挡住。 + + 逐个验证而不是只测一个：漏登记一个键的症状是「配上就崩」，而这条断言是 + 唯一会提前发现它的地方。 + """ + sent = _send(LLMAgentStepEfficiency, **{k: 1 for k in LOCAL_ONLY_CONFIG_KEYS}) + + leaked = LOCAL_ONLY_CONFIG_KEYS & set(sent) + assert not leaked, f"这些本地配置键漏给了 provider：{sorted(leaked)}" + + +def test_an_unregistered_local_key_is_dropped_not_forwarded(caplog): + """没登记过的配置键也不能发出去，但要出声。 + + 这是把黑名单换成白名单的理由：``strictness``、``agent_config`` 都曾经漏登记， + 而漏一个的症状是该评估器每次调用必崩。现在判据是 SDK 签名，登记表只决定 + 要不要告警。 + """ + sent = _send(LLMAgentStepEfficiency, strictness=5, definitely_not_a_param=1) + + assert "strictness" not in sent + assert "definitely_not_a_param" not in sent + # 登记过的不吵，没登记的要提示——拼错键名不该悄悄不生效 + assert "definitely_not_a_param" in caplog.text + assert "strictness" not in caplog.text + + +def test_filter_lives_on_the_shared_accessor(): + """过滤必须在取参数那一处，否则下一个调用点会忘掉它。""" + BaseOpenAI.dynamic_config = EvaluatorLLMArgs(threshold=1, temperature=0.5) + try: + assert BaseOpenAI.get_request_extra_params() == {"temperature": 0.5} + assert BaseOpenAI.get_local_config_value("threshold") == 1 + finally: + BaseOpenAI.dynamic_config = EvaluatorLLMArgs() + + +def test_max_retries_reaches_the_client_not_the_request(monkeypatch): + """重试次数是构造客户端时的参数，配错地方就会变成请求体里的未知字段。""" + monkeypatch.setattr("openai.OpenAI", _FakeOpenAI) + LLMAgentStepEfficiency.dynamic_config = EvaluatorLLMArgs( + model="m", key="k", api_url="http://example.invalid", max_retries=1 + ) + LLMAgentStepEfficiency.create_client() + + assert _FakeOpenAI.built["max_retries"] == 1 + assert "max_retries" not in _send(LLMAgentStepEfficiency, max_retries=1) + + +def test_max_retries_defaults_to_the_sdk_value(monkeypatch): + """不配就得是原来的行为，否则这次改动会悄悄改掉所有调用方的重试次数。""" + monkeypatch.setattr("openai.OpenAI", _FakeOpenAI) + LLMAgentStepEfficiency.dynamic_config = EvaluatorLLMArgs( + model="m", key="k", api_url="http://example.invalid" + ) + LLMAgentStepEfficiency.create_client() + + assert _FakeOpenAI.built["max_retries"] == DEFAULT_MAX_RETRIES