From a87af9c65df77433eb037c4343c6fe9e5789fd11 Mon Sep 17 00:00:00 2001 From: WenjinXie Date: Mon, 8 Jun 2026 15:03:33 +0800 Subject: [PATCH] [fix] Reject parameter_types on Python YAML tools (#774) The YAML loader documented parameter_types as java-only/forbidden for Python tools, but only validated the java-missing direction. A Python tool declaring parameter_types passed through and was silently dropped. Add a fail-fast check and align the ToolSpec docstring with the doc. Co-Authored-By: Claude Opus 4.8 (1M context) --- docs/yaml-schema.json | 2 +- python/flink_agents/api/yaml/loader.py | 3 +++ python/flink_agents/api/yaml/specs.py | 2 +- .../api/yaml/tests/test_loader.py | 24 +++++++++++++++++++ 4 files changed, 29 insertions(+), 2 deletions(-) diff --git a/docs/yaml-schema.json b/docs/yaml-schema.json index ac62bb17e..015fdebd4 100644 --- a/docs/yaml-schema.json +++ b/docs/yaml-schema.json @@ -342,7 +342,7 @@ }, "ToolSpec": { "additionalProperties": false, - "description": "Points ``function:`` at a callable tool.\n\n``function`` is written as ``:`` \u2014 the\ncolon separates the Python module (or Java class FQN) from the\nattribute path inside it. For Python, the right side may be a\nnested ``Class.method``.\n\n``parameter_types`` is required when ``type: java`` and is ignored\notherwise (Python tools are reflected from the callable signature).\nThe list contains one string per declared parameter of the Java\nmethod, in declaration order \u2014 the loader uses it to disambiguate\noverloaded methods on the Java class. Each string is one of:\n\n- A Java primitive name: one of ``boolean``, ``byte``, ``short``,\n ``int``, ``long``, ``float``, ``double``, ``char``.\n- A fully-qualified Java reference type (including boxed\n primitives), e.g. ``java.lang.Double``, ``java.lang.String``,\n ``java.util.List``.\n\nGeneric type arguments are not part of the JVM method descriptor\nand must not be included (``java.util.List``, not\n``java.util.List``).", + "description": "Points ``function:`` at a callable tool.\n\n``function`` is written as ``:`` \u2014 the\ncolon separates the Python module (or Java class FQN) from the\nattribute path inside it. For Python, the right side may be a\nnested ``Class.method``.\n\n``parameter_types`` is required when ``type: java`` and is forbidden\notherwise (Python tools are reflected from the callable signature).\nThe list contains one string per declared parameter of the Java\nmethod, in declaration order \u2014 the loader uses it to disambiguate\noverloaded methods on the Java class. Each string is one of:\n\n- A Java primitive name: one of ``boolean``, ``byte``, ``short``,\n ``int``, ``long``, ``float``, ``double``, ``char``.\n- A fully-qualified Java reference type (including boxed\n primitives), e.g. ``java.lang.Double``, ``java.lang.String``,\n ``java.util.List``.\n\nGeneric type arguments are not part of the JVM method descriptor\nand must not be included (``java.util.List``, not\n``java.util.List``).", "properties": { "function": { "anyOf": [ diff --git a/python/flink_agents/api/yaml/loader.py b/python/flink_agents/api/yaml/loader.py index 4ec97a954..2a0ad28df 100644 --- a/python/flink_agents/api/yaml/loader.py +++ b/python/flink_agents/api/yaml/loader.py @@ -179,6 +179,9 @@ def _build_tool(spec: ToolSpec) -> FunctionTool: if spec.type == "java" and spec.parameter_types is None: msg = f"Tool {spec.name!r}: java tools must declare 'parameter_types' in YAML." raise ValueError(msg) + if spec.type != "java" and spec.parameter_types is not None: + msg = f"Tool {spec.name!r}: 'parameter_types' is only valid for Java tools." + raise ValueError(msg) func = resolve_function( name=spec.name, function=spec.function, diff --git a/python/flink_agents/api/yaml/specs.py b/python/flink_agents/api/yaml/specs.py index 220eccf35..b7b7cf713 100644 --- a/python/flink_agents/api/yaml/specs.py +++ b/python/flink_agents/api/yaml/specs.py @@ -98,7 +98,7 @@ class ToolSpec(BaseModel): attribute path inside it. For Python, the right side may be a nested ``Class.method``. - ``parameter_types`` is required when ``type: java`` and is ignored + ``parameter_types`` is required when ``type: java`` and is forbidden otherwise (Python tools are reflected from the callable signature). The list contains one string per declared parameter of the Java method, in declaration order — the loader uses it to disambiguate diff --git a/python/flink_agents/api/yaml/tests/test_loader.py b/python/flink_agents/api/yaml/tests/test_loader.py index a605cc386..048c6b6a0 100644 --- a/python/flink_agents/api/yaml/tests/test_loader.py +++ b/python/flink_agents/api/yaml/tests/test_loader.py @@ -562,6 +562,30 @@ def test_build_agents_rejects_java_tool_missing_parameter_types( build_agents(p) +def test_build_agents_rejects_python_tool_with_parameter_types( + tmp_path: Path, +) -> None: + # Python tool signatures are reflected from the callable, so + # parameter_types is Java-only and silently dropped otherwise. + yaml_text = ( + "agents:\n" + " - name: a\n" + " tools:\n" + " - name: t1\n" + " type: python\n" + f" function: {_TARGETS_MODULE}:increment\n" + " parameter_types: [java.lang.Integer]\n" + " actions:\n" + " - name: noop\n" + f" function: {_TARGETS_MODULE}:increment\n" + " listen_to: [input]\n" + ) + p = tmp_path / "python_tool_params.yaml" + p.write_text(yaml_text) + with pytest.raises(ValueError, match="parameter_types"): + build_agents(p) + + def test_build_agents_builds_java_tool_descriptor(tmp_path: Path) -> None: """YAML parsing of a Java tool yields an api ``FunctionTool`` wrapping a ``JavaFunction`` descriptor — no JVM needed at parse time.