Improve ambiguity handling in argument processing - #43
Conversation
|
Looks like
|
| return action, args | ||
|
|
||
|
|
||
| def validate_required_args(action: str, args: Optional[Dict[str, Any]], required: list[str]) -> Optional[BaseResult]: |
There was a problem hiding this comment.
This method is new but never used. Is it dead code?
| return await ai_scriptless_manager.add_command( | ||
| args.get("test_id", ""), | ||
| args.get("command_id", ""), | ||
| args.get("arguments"), |
There was a problem hiding this comment.
Im guessing we are completely dropping arguments as a key, we could keep a safeguard for backwards compatible by doing args.get("cmd_arguments") or args.get("arguments")
There was a problem hiding this comment.
Not applied, this is aligned to the docstring, it's not programmatic, it's the declaration of the docstring arguments
There was a problem hiding this comment.
Not applied, this is aligned to the docstring, it's not programmatic, it's the declaration of the docstring arguments
| args.get("test_id", ""), | ||
| args.get("step_path", ""), | ||
| args.get("arguments", {}), | ||
| args.get("cmd_arguments", {}), |
There was a problem hiding this comment.
Same as previous comment in add_command section.
There was a problem hiding this comment.
Not applied, this is aligned to the docstring, it's not programmatic, it's the declaration of the docstring arguments
| icon_data = base64.standard_b64encode(icon_path.read_bytes()).decode() | ||
| return f"data:image/png;base64,{icon_data}" | ||
|
|
||
| def normalize_action_args(arguments: Optional[Dict[str, Any]] = None) -> tuple[str, Dict[str, Any]]: |
There was a problem hiding this comment.
Can we include some tests for this logic?
| ctx: Context = Field(description="Context object providing access to MCP capabilities") | ||
| ) -> BaseResult: | ||
| action, args = normalize_action_args(arguments) | ||
| if args is None: |
There was a problem hiding this comment.
normalize_action_args never returns None and will default to {}, this is now dead code and should be removed on all managers.
2 participants
This pull request introduces a standardized approach for handling tool arguments across multiple managers and patches the MCP argument model to improve payload handling. The main change is the addition of a
normalize_action_argsutility, which ensures consistent parsing of action and argument data, and updates all tool managers to use a singleargumentsparameter. This makes the codebase more robust against variations in client payloads and simplifies argument handling logic.Core infrastructure improvements:
normalize_action_argsand validation utilities totools/utils.pyto standardize and simplify argument parsing for all tool endpoints. This function supports multiple payload formats and ensures all managers receive arguments in a consistent(action, args)tuple.mcp.server.fastmcp.utilities.func_metadata.ArgModelBaseinmain.pyto ensure tools with anargumentsparameter always receive the full payload, even if the client omits theargumentswrapper. This increases compatibility with different client payload styles.Tool manager refactoring:
registerendpoints (ai_scriptless_manager.py,device_manager.py,execution_manager.py,help_manager.py,user_manager.py) to accept a singleargumentsparameter instead of separateactionandargs, and to usenormalize_action_argsfor argument extraction. [1] [2] [3] [4] [5]normalize_action_args. [1] [2] [3] [4] [5]API and documentation consistency:
ai_scriptless_manager.pyto rename parameters fromargumentstocmd_argumentsfor clarity, and updated all related docstrings and dispatch logic to match the new naming and argument handling convention. [1] [2] [3] [4] [5]These changes collectively make the argument handling more robust and maintainable, reducing the risk of errors from inconsistent payloads and simplifying the interface for future tool development.