diff --git a/README.md b/README.md index 68d5dfc54..74ee2321d 100644 --- a/README.md +++ b/README.md @@ -256,6 +256,9 @@ prose answer; it recognises the model from the model ID, a geo/global inference-profile ID, or a foundation-model / inference-profile ARN. An application-inference-profile ARN hides the model, so add that ARN to the registry (`SKILLSPECTOR_MODEL_REGISTRY`) with `tool_choice: auto`. +The `openai_compatible` provider honours the same `tool_choice: auto` entry +for endpoints that ignore both `response_format` and a forced `tool_choice` +and answer in prose (for example iFlytek's `spark-x2.5`, which is bundled). `SKILLSPECTOR_STRUCTURED_OUTPUT_METHOD=json_schema|function_calling` overrides the method for any provider. diff --git a/src/skillspector/llm_utils.py b/src/skillspector/llm_utils.py index f2a9da738..1b42150b1 100644 --- a/src/skillspector/llm_utils.py +++ b/src/skillspector/llm_utils.py @@ -403,18 +403,36 @@ def bind_structured_output( """``llm.with_structured_output(schema)`` with the method *model* needs. A chat model restricted to ``toolChoice`` ``auto`` (Bedrock models that - reject a forced tool call) is bound with LangChain's default tool method, - the prompt asks for the tool call explicitly, and a prose answer raises - :class:`StructuredOutputParseError`, which the analyzers retry like any - other malformed structured response. + reject a forced tool call, or a ``ChatOpenAI`` with ``tool_choice`` + disabled) and bound with a tool method gets the tool call asked for in + the prompt, and a prose answer raises :class:`StructuredOutputParseError`, + which the analyzers retry like any other malformed structured response. """ kwargs = structured_output_kwargs(model, provider) structured = llm.with_structured_output(schema, **kwargs) # type: ignore[attr-defined] - if kwargs or getattr(llm, "supports_tool_choice_values", None) != ("auto",): + if not _binds_unforced_tool_call(llm, kwargs.get("method")): return structured return _require_tool_call(structured, schema) +def _binds_unforced_tool_call(llm: object, method: str | None) -> bool: + """``True`` when *method* binds the schema as a tool *llm* is not forced to call. + + ``ChatBedrockConverse`` binds a tool by default; ``ChatOpenAI`` defaults to + ``json_schema``, so its ``tool_choice``-disabled form needs an explicit + ``function_calling``. + """ + if getattr(llm, "supports_tool_choice_values", None) == ("auto",): + return method in (None, "function_calling") + disabled = getattr(llm, "disabled_params", None) + return ( + method == "function_calling" + and isinstance(disabled, dict) + and "tool_choice" in disabled + and disabled["tool_choice"] is None + ) + + _TOOL_CALL_INSTRUCTION = ( "Report your result by calling the {tool} tool exactly once. Do not answer in prose." ) diff --git a/src/skillspector/providers/chat_models.py b/src/skillspector/providers/chat_models.py index d16912af2..e69791b66 100644 --- a/src/skillspector/providers/chat_models.py +++ b/src/skillspector/providers/chat_models.py @@ -112,8 +112,13 @@ def create_openai_compatible_chat_model( max_tokens: int, timeout: float | None = 120, default_headers: dict[str, str] | None = None, + disabled_params: dict[str, object] | None = None, ) -> BaseChatModel | None: - """Create ``ChatOpenAI`` for providers serving OpenAI-compatible endpoints.""" + """Create ``ChatOpenAI`` for providers serving OpenAI-compatible endpoints. + + *disabled_params* is passed to ``ChatOpenAI``; ``{"tool_choice": None}`` + keeps ``with_structured_output`` from forcing a tool call. + """ if credentials is None: return None @@ -127,6 +132,8 @@ def create_openai_compatible_chat_model( "timeout": timeout, "default_headers": default_headers, } + if disabled_params: + kwargs["disabled_params"] = disabled_params reasoning_effort = resolve_reasoning_effort() if reasoning_effort: kwargs["reasoning_effort"] = reasoning_effort diff --git a/src/skillspector/providers/openai_compatible/model_registry.yaml b/src/skillspector/providers/openai_compatible/model_registry.yaml index 81f34d7e8..3d128157a 100644 --- a/src/skillspector/providers/openai_compatible/model_registry.yaml +++ b/src/skillspector/providers/openai_compatible/model_registry.yaml @@ -8,6 +8,7 @@ # "": # context_length: # total context window in tokens (required) # max_output_tokens: # model's max output cap (optional) +# tool_choice: auto # when the endpoint ignores a forced tool_choice (optional) models: # Groq @@ -41,3 +42,9 @@ models: "deepseek-ai/DeepSeek-V3": context_length: 65536 max_output_tokens: 8192 + + # iFlytek Astron Token Plan (https://maas-token-api.cn-huabei-1.xf-yun.com/v2). + # The endpoint ignores response_format and a forced tool_choice. + "spark-x2.5": + context_length: 262144 + tool_choice: auto diff --git a/src/skillspector/providers/openai_compatible/provider.py b/src/skillspector/providers/openai_compatible/provider.py index 6a5923c82..1a9938d38 100644 --- a/src/skillspector/providers/openai_compatible/provider.py +++ b/src/skillspector/providers/openai_compatible/provider.py @@ -22,6 +22,11 @@ Required env vars: SKILLSPECTOR_COMPAT_API_KEY — API key for the target provider SKILLSPECTOR_COMPAT_BASE_URL — Base URL (e.g. https://api.groq.com/openai/v1) + +Some endpoints ignore both ``response_format`` and a forced ``tool_choice`` +and answer in prose. A registry entry with ``tool_choice: auto`` binds the +schema as a tool with ``tool_choice`` left at ``auto``; the prompt then asks +for the tool call and a prose answer is retried. """ from __future__ import annotations @@ -64,6 +69,9 @@ def create_chat_model( credentials=self.resolve_credentials(), max_tokens=max_tokens, timeout=timeout, + disabled_params=( + None if self.forced_tool_choice_supported(model) else {"tool_choice": None} + ), ) def get_context_length(self, model: str) -> int | None: @@ -76,3 +84,14 @@ def resolve_model(self, slot: str = "default") -> str: """Resolve model: ``SKILLSPECTOR_MODEL`` env > slot default > ``DEFAULT_MODEL``.""" user_input = os.environ.get("SKILLSPECTOR_MODEL", "").strip() return user_input or self.SLOT_DEFAULTS.get(slot, "") or self.DEFAULT_MODEL + + def forced_tool_choice_supported(self, model: str) -> bool: + """``False`` when the registry declares ``tool_choice: auto`` for *model*.""" + return registry.lookup_setting(REGISTRY_PATH, model, "tool_choice") != "auto" + + def structured_output_method(self, model: str) -> str | None: + """``with_structured_output`` method: registry entry, else tool calling for ``tool_choice: auto``.""" + declared = registry.lookup_structured_output_method(REGISTRY_PATH, model) + if declared: + return declared + return None if self.forced_tool_choice_supported(model) else "function_calling" diff --git a/tests/unit/test_llm_utils.py b/tests/unit/test_llm_utils.py index a9a1ea569..2a687fba9 100644 --- a/tests/unit/test_llm_utils.py +++ b/tests/unit/test_llm_utils.py @@ -771,6 +771,33 @@ class _PlainProvider: pass +class Verdict(BaseModel): + summary: str + + +def _stub_chat_openai( + monkeypatch: pytest.MonkeyPatch, answers: list[AIMessage] +) -> tuple[object, list[tuple[str, dict]]]: + """A ``ChatOpenAI`` with ``tool_choice`` disabled whose transport returns *answers* in order.""" + from langchain_core.outputs import ChatResult + from langchain_openai import ChatOpenAI + + requests: list[tuple[str, dict]] = [] + + def _generate(self, messages, stop=None, run_manager=None, **kwargs): + requests.append((messages[-1].content, kwargs)) + return ChatResult(generations=[ChatGeneration(message=answers.pop(0))]) + + monkeypatch.setattr(ChatOpenAI, "_generate", _generate) + llm = ChatOpenAI( + model="spark-x2.5", + api_key="sk-test", + base_url="https://maas-token-api.cn-huabei-1.xf-yun.com/v2", + disabled_params={"tool_choice": None}, + ) + return llm, requests + + class _RecordingLLM: def __init__(self) -> None: self.calls: list[tuple[type, dict]] = [] @@ -856,6 +883,84 @@ def test_forced_tool_choice_models_are_bound_untouched( llm.supports_tool_choice_values = ("auto", "any", "tool") # type: ignore[attr-defined] assert bind_structured_output(llm, dict, "m", provider=_PlainProvider()) is llm + @pytest.mark.parametrize( + ("method", "wrapped"), + [("function_calling", True), ("json_schema", False)], + ) + def test_auto_only_tool_choice_with_an_explicit_method( + self, monkeypatch: pytest.MonkeyPatch, method: str, wrapped: bool + ) -> None: + """An explicit ``function_calling`` still asks for the call; ``json_schema`` does not bind a tool.""" + from langchain_core.runnables import RunnableLambda + + monkeypatch.setenv("SKILLSPECTOR_STRUCTURED_OUTPUT_METHOD", method) + structured = RunnableLambda(lambda prompt: {"summary": "ok"}) + + class _AutoOnlyLLM(_RecordingLLM): + supports_tool_choice_values = ("auto",) + + def with_structured_output(self, schema: type, **kwargs: object) -> RunnableLambda: + super().with_structured_output(schema, **kwargs) + return structured + + llm = _AutoOnlyLLM() + bound = bind_structured_output(llm, dict, "m", provider=_PlainProvider()) + assert llm.calls == [(dict, {"method": method})] + assert (bound is not structured) is wrapped + + def test_chat_openai_with_tool_choice_disabled_asks_for_the_call( + self, monkeypatch: pytest.MonkeyPatch + ) -> None: + from skillspector.providers.openai_compatible import OpenAICompatibleProvider + + monkeypatch.delenv("SKILLSPECTOR_STRUCTURED_OUTPUT_METHOD", raising=False) + monkeypatch.delenv("SKILLSPECTOR_MODEL_REGISTRY", raising=False) + llm, requests = _stub_chat_openai( + monkeypatch, + [ + AIMessage(content="Verdict: looks fine."), + AIMessage( + content="", + tool_calls=[{"name": "Verdict", "args": {"summary": "ok"}, "id": "call_1"}], + ), + ], + ) + chain = bind_structured_output( + llm, Verdict, "spark-x2.5", provider=OpenAICompatibleProvider() + ) + with pytest.raises(StructuredOutputParseError, match="Verdict"): + chain.invoke("analyse this") # type: ignore[attr-defined] + assert chain.invoke("analyse this") == Verdict(summary="ok") # type: ignore[attr-defined] + prompt, request = requests[0] + assert prompt.startswith("analyse this\n\n") and "calling the Verdict tool" in prompt + assert "tool_choice" not in request + assert request["tools"][0]["function"]["name"] == "Verdict" + + @pytest.mark.parametrize("method", [None, "json_schema"]) + def test_chat_openai_with_tool_choice_disabled_keeps_a_non_tool_method( + self, monkeypatch: pytest.MonkeyPatch, method: str | None + ) -> None: + """Without an explicit ``function_calling`` the binder sends ``response_format`` unwrapped.""" + if method: + monkeypatch.setenv("SKILLSPECTOR_STRUCTURED_OUTPUT_METHOD", method) + else: + monkeypatch.delenv("SKILLSPECTOR_STRUCTURED_OUTPUT_METHOD", raising=False) + llm, requests = _stub_chat_openai( + monkeypatch, + [ + AIMessage( + content='{"summary": "ok"}', + additional_kwargs={"parsed": Verdict(summary="ok")}, + ) + ], + ) + chain = bind_structured_output(llm, Verdict, "spark-x2.5", provider=_PlainProvider()) + assert chain.invoke("analyse this") == Verdict(summary="ok") # type: ignore[attr-defined] + prompt, request = requests[0] + assert prompt == "analyse this" + assert request["response_format"] is Verdict + assert "tools" not in request + def test_cli_adapter_accepts_the_method_keyword(self) -> None: from skillspector.llm_utils import AgentCLIChatModel diff --git a/tests/unit/test_new_providers.py b/tests/unit/test_new_providers.py index e4c4a1c6e..5b40a02f7 100644 --- a/tests/unit/test_new_providers.py +++ b/tests/unit/test_new_providers.py @@ -301,6 +301,56 @@ def test_create_returns_none_without_credentials(self) -> None: is None ) + def test_forced_tool_choice_is_kept_by_default(self, monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setenv("SKILLSPECTOR_COMPAT_API_KEY", "gsk_abc") + monkeypatch.setenv("SKILLSPECTOR_COMPAT_BASE_URL", "https://api.groq.com/openai/v1") + provider = OpenAICompatibleProvider() + llm = provider.create_chat_model("llama-3.1-70b-versatile", max_tokens=1024) + assert isinstance(llm, ChatOpenAI) + assert llm.disabled_params is None + assert provider.forced_tool_choice_supported("llama-3.1-70b-versatile") + assert provider.structured_output_method("llama-3.1-70b-versatile") is None + + def test_tool_choice_auto_entry_disables_forced_tool_choice( + self, monkeypatch: pytest.MonkeyPatch + ) -> None: + monkeypatch.setenv("SKILLSPECTOR_COMPAT_API_KEY", "sk-abc") + monkeypatch.setenv( + "SKILLSPECTOR_COMPAT_BASE_URL", "https://maas-token-api.cn-huabei-1.xf-yun.com/v2" + ) + provider = OpenAICompatibleProvider() + llm = provider.create_chat_model("spark-x2.5", max_tokens=1024) + assert isinstance(llm, ChatOpenAI) + assert llm.disabled_params == {"tool_choice": None} + assert not provider.forced_tool_choice_supported("spark-x2.5") + assert provider.structured_output_method("spark-x2.5") == "function_calling" + assert provider.get_context_length("spark-x2.5") == 262144 + + def test_registry_override_can_declare_tool_choice_auto( + self, monkeypatch: pytest.MonkeyPatch, tmp_path + ) -> None: + override = tmp_path / "registry.yaml" + override.write_text( + "models:\n my-model:\n context_length: 32768\n tool_choice: auto\n", + encoding="utf-8", + ) + monkeypatch.setenv("SKILLSPECTOR_MODEL_REGISTRY", str(override)) + provider = OpenAICompatibleProvider() + assert not provider.forced_tool_choice_supported("my-model") + assert provider.structured_output_method("my-model") == "function_calling" + + def test_declared_structured_output_method_wins( + self, monkeypatch: pytest.MonkeyPatch, tmp_path + ) -> None: + override = tmp_path / "registry.yaml" + override.write_text( + "models:\n my-model:\n context_length: 32768\n" + " tool_choice: auto\n structured_output: json_schema\n", + encoding="utf-8", + ) + monkeypatch.setenv("SKILLSPECTOR_MODEL_REGISTRY", str(override)) + assert OpenAICompatibleProvider().structured_output_method("my-model") == "json_schema" + class TestOpenAICompatibleProviderSelection: """SKILLSPECTOR_PROVIDER=openai_compatible selects the generic provider."""