diff --git a/src/plyngent/tools/chat/choose.py b/src/plyngent/tools/chat/choose.py index 042ee70..dd98a1f 100644 --- a/src/plyngent/tools/chat/choose.py +++ b/src/plyngent/tools/chat/choose.py @@ -1,27 +1,17 @@ from __future__ import annotations -import json from typing import cast from plyngent.agent import ToolTag, tool from plyngent.prompting import ChoiceOption, NonInteractiveError, choose_async -def parse_options(raw: str) -> list[ChoiceOption]: - """Parse options JSON: list of strings or objects with label/description/value.""" - text = raw.strip() - if not text: +def parse_options(options: list[object]) -> list[ChoiceOption]: + """Normalize a list of strings or dicts into ``ChoiceOption`` list.""" + if not options: return [] - try: - data: object = json.loads(text) - except json.JSONDecodeError as exc: - msg = f"options must be JSON: {exc}" - raise ValueError(msg) from exc - if not isinstance(data, list): - msg = "options must be a JSON array" - raise TypeError(msg) out: list[ChoiceOption] = [] - for item_obj in cast("list[object]", data): + for item_obj in options: if isinstance(item_obj, str): out.append(ChoiceOption(label=item_obj)) continue @@ -41,7 +31,7 @@ def parse_options(raw: str) -> list[ChoiceOption]: ) ) continue - msg = "options items must be strings or objects" + msg = "options items must be strings or objects with label/description/value" raise TypeError(msg) return out @@ -49,24 +39,24 @@ def parse_options(raw: str) -> list[ChoiceOption]: @tool(name="ask_user_choice", tags=ToolTag.LOCAL) async def choose_user( question: str, - options: str, + options: list[str] | list[dict[str, object]], default: str = "", *, allow_custom: bool = True, ) -> str: """Ask the human to pick from a list of options (or type a custom answer). - ``options`` is a JSON array of strings, or objects with - ``label``, optional ``description``, optional ``value``. + ``options`` is a list of strings, or objects with + ``label`` (required), optional ``description``, optional ``value``. When ``allow_custom`` is true (default), free-text answers are accepted. Returns the chosen option value (or custom text). """ try: - parsed = parse_options(options) + parsed = parse_options([cast("object", o) for o in options]) except (TypeError, ValueError) as exc: return f"error: {exc}" if not parsed and not allow_custom: - return "error: options must be a non-empty JSON array when allow_custom is false" + return "error: options must be a non-empty list when allow_custom is false" try: return await choose_async( question, diff --git a/src/plyngent/tools/chat/form.py b/src/plyngent/tools/chat/form.py index 9c8799e..08895ef 100644 --- a/src/plyngent/tools/chat/form.py +++ b/src/plyngent/tools/chat/form.py @@ -8,25 +8,14 @@ from plyngent.prompting import FormField, NonInteractiveError, form_async from plyngent.tools.chat.choose import parse_options -def parse_fields(raw: str) -> list[FormField]: - """Parse form fields JSON array of objects.""" - text = raw.strip() - if not text: - return [] - try: - data: object = json.loads(text) - except json.JSONDecodeError as exc: - msg = f"fields must be JSON: {exc}" - raise ValueError(msg) from exc - if not isinstance(data, list) or not data: - msg = "fields must be a non-empty JSON array" +def parse_fields(fields: list[dict[str, object]]) -> list[FormField]: + """Normalize a list of field dicts into ``FormField`` list.""" + if not fields: + msg = "fields must be a non-empty list" raise ValueError(msg) out: list[FormField] = [] - for item_obj in cast("list[object]", data): - if not isinstance(item_obj, dict): - msg = "each field must be a JSON object" - raise TypeError(msg) - raw_map = {str(key): value for key, value in cast("dict[object, object]", item_obj).items()} + for item_obj in fields: + raw_map = {str(key): value for key, value in item_obj.items()} name = raw_map.get("name") prompt = raw_map.get("prompt") if not isinstance(name, str) or not name: @@ -39,7 +28,10 @@ def parse_fields(raw: str) -> list[FormField]: options_raw = raw_map.get("options") options = None if options_raw is not None: - options = parse_options(json.dumps(options_raw)) + if not isinstance(options_raw, list): + msg = f"field {name!r} options must be a list" + raise TypeError(msg) + options = parse_options(cast("list[object]", options_raw)) allow_custom_obj = raw_map.get("allow_custom", True) allow_custom = allow_custom_obj if isinstance(allow_custom_obj, bool) else True out.append( @@ -55,12 +47,18 @@ def parse_fields(raw: str) -> list[FormField]: @tool(name="ask_user_form", tags=ToolTag.LOCAL) -async def form_user(title: str, fields: str, *, confirm_submit: bool = True) -> str: +async def form_user( + title: str, + fields: list[dict[str, object]], + *, + confirm_submit: bool = True, +) -> str: """Run a multi-step form with the human; returns JSON object of answers. - ``fields`` is a JSON array of objects: - ``name``, ``prompt``, optional ``default``, optional ``options`` (same shape - as ask_user_choice), optional ``allow_custom`` (default true). + ``fields`` is a list of objects: + ``name`` (required), ``prompt`` (required), optional ``default``, + optional ``options`` (same shape as ask_user_choice), + optional ``allow_custom`` (default true). When ``confirm_submit`` is true, the human reviews a summary before submit. """ try: diff --git a/tests/test_tools/test_chat_tools.py b/tests/test_tools/test_chat_tools.py index 270e5c7..8255c68 100644 --- a/tests/test_tools/test_chat_tools.py +++ b/tests/test_tools/test_chat_tools.py @@ -25,7 +25,7 @@ async def test_choose_user_tool_index() -> None: json.dumps( { "question": "Pick", - "options": json.dumps(["alpha", "beta"]), + "options": ["alpha", "beta"], "allow_custom": False, } ), @@ -33,23 +33,44 @@ async def test_choose_user_tool_index() -> None: assert out == "alpha" -async def test_choose_user_bad_options() -> None: +async def test_choose_user_dict_options() -> None: + backend = ScriptedBackend(["1"]) + with temporary_backend(backend): + registry = ToolRegistry([choose_user]) + out = await registry.execute( + "ask_user_choice", + json.dumps( + { + "question": "Pick", + "options": [{"label": "yes", "description": "affirm"}], + } + ), + ) + assert out == "yes" + + +async def test_choose_user_invalid_options() -> None: registry = ToolRegistry([choose_user]) out = await registry.execute( "ask_user_choice", - json.dumps({"question": "Pick", "options": "not-json"}), + json.dumps({"question": "Pick", "options": [42]}), ) - assert out.startswith("error:") + assert "error" in out async def test_form_user_tool() -> None: backend = ScriptedBackend(["ncbm"], confirms=[True]) - fields = json.dumps([{"name": "user", "prompt": "User?"}]) with temporary_backend(backend): registry = ToolRegistry([form_user]) out = await registry.execute( "ask_user_form", - json.dumps({"title": "Setup", "fields": fields, "confirm_submit": True}), + json.dumps( + { + "title": "Setup", + "fields": [{"name": "user", "prompt": "User?"}], + "confirm_submit": True, + } + ), ) assert json.loads(out) == {"user": "ncbm"}