Detect types of tool call arguments.#141
Conversation
There was a problem hiding this comment.
Pull request overview
Updates the tool-calling argument parsing so Gemini tool call parameters can be interpreted as proper JSON types (e.g., numbers/booleans/objects) instead of being treated as strings, improving compatibility with strict JSON schema validators.
Changes:
- Updated the tool-calling protocol hint to instruct quoting of string values.
- Added
parse_arg_valueto JSON-parse individual tool argument values (with fallback to raw string). - Switched tool argument extraction to use typed parsing before re-serializing arguments to JSON.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| def parse_arg_value(val): | ||
| result = _strip_param_fences(val) | ||
| try: | ||
| return orjson.loads(result) | ||
| except orjson.JSONDecodeError: | ||
| return result |
There was a problem hiding this comment.
parse_arg_value is the only top-level helper in this module without type hints and a docstring, which makes its intended input/output (and JSON-coercion behavior) less clear. Please add at least a val: str parameter annotation and an appropriate return type (e.g., object/Any), plus a short docstring describing the JSON-parsing fallback behavior.
| @@ -277,6 +277,12 @@ def strip_system_hints(text: str) -> str: | |||
|
|
|||
| return cleaned | |||
|
|
|||
There was a problem hiding this comment.
PEP 8 expects two blank lines between top-level function definitions; currently there is only one blank line before def parse_arg_value(...) after strip_system_hints. Please insert an extra blank line to match the rest of this module’s formatting.
| return orjson.loads(result) | ||
| except orjson.JSONDecodeError: | ||
| return result | ||
|
|
There was a problem hiding this comment.
PEP 8 expects two blank lines between top-level function definitions; there is only one blank line between parse_arg_value and _process_tools_internal. Please add the extra blank line for consistent formatting.
| "If tool execution is required, you MUST adhere to this EXACT protocol. No exceptions.\n\n" | ||
| "1. OUTPUT RESTRICTION: Your response MUST contain ONLY the [ToolCalls] block. Conversational filler, preambles, or concluding remarks are STRICTLY PROHIBITED.\n" | ||
| "2. WRAPPING LOGIC: Every parameter value MUST be enclosed in a markdown code block. Use 3 backticks (```) by default. If the value contains backticks, the outer fence MUST be longer than any sequence inside (e.g., ````).\n" | ||
| "2. WRAPPING LOGIC: Every parameter value MUST be enclosed in a markdown code block. Use 3 backticks (```) by default. If the value contains backticks, the outer fence MUST be longer than any sequence inside (e.g., ````). String typed values MUST be quoted (\")\n" |
There was a problem hiding this comment.
The protocol text says "String typed values"; this is grammatically awkward/ambiguous. Consider changing to "String-typed values" (or "Values of type string") and add a terminating period for clarity since this string is shown to the model verbatim.
| "2. WRAPPING LOGIC: Every parameter value MUST be enclosed in a markdown code block. Use 3 backticks (```) by default. If the value contains backticks, the outer fence MUST be longer than any sequence inside (e.g., ````). String typed values MUST be quoted (\")\n" | |
| "2. WRAPPING LOGIC: Every parameter value MUST be enclosed in a markdown code block. Use 3 backticks (```) by default. If the value contains backticks, the outer fence MUST be longer than any sequence inside (e.g., ````). String-typed values MUST be quoted (\").\n" |
This PR makes sure Gemini tool call arguments are properly typed and are not all considered strings. This allows MCP calls through strict JSON schema validators (ie. librechat).