codeant-ai-for-open-source[bot] commented on code in PR #38775:
URL: https://github.com/apache/superset/pull/38775#discussion_r2967352872
##########
tests/unit_tests/mcp_service/test_tool_search_transform.py:
##########
@@ -171,3 +173,130 @@ def test_serialize_tools_handles_no_output_schema():
assert len(result) == 1
assert result[0]["name"] == "simple_tool"
assert "outputSchema" not in result[0]
+
+
+# -- _normalize_call_tool_arguments tests --
+
+
+def test_normalize_serializes_dict_with_anyof_string():
+ """Dict value is JSON-serialized when schema has anyOf with string type."""
+ arguments = {"request": {"dataset_id": 1, "config": {"key": "val"}}}
+ schema = {
+ "properties": {
+ "request": {
+ "anyOf": [
+ {"type": "string"},
+ {"$ref": "#/$defs/SomeModel"},
+ ]
+ }
+ }
+ }
+
+ result = _normalize_call_tool_arguments(arguments, schema)
+
+ assert isinstance(result["request"], str)
+ assert json.loads(result["request"]) == {
+ "dataset_id": 1,
+ "config": {"key": "val"},
+ }
+
+
+def test_normalize_serializes_dict_with_oneof_string():
+ """Dict value is JSON-serialized when schema has oneOf with string type."""
+ arguments = {"request": {"name": "test"}}
+ schema = {
+ "properties": {
+ "request": {
+ "oneOf": [
+ {"type": "string"},
+ {"type": "object"},
+ ]
+ }
+ }
+ }
+
+ result = _normalize_call_tool_arguments(arguments, schema)
+
+ assert isinstance(result["request"], str)
+
Review Comment:
**Suggestion:** This test only verifies the result is a string, so invalid
non-JSON strings would still pass and fail to catch real regressions in
serialization behavior. Validate that the serialized payload round-trips to the
original dict. [logic error]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ CI may miss oneOf serialization regressions.
- ❌ call_tool requests can fail JSON model parsing.
```
</details>
```suggestion
assert json.loads(result["request"]) == {"name": "test"}
```
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Run `pytest
tests/unit_tests/mcp_service/test_tool_search_transform.py::test_normalize_serializes_dict_with_oneof_string`;
it currently checks only string type at
`.../test_tool_search_transform.py:220`.
2. Trace serialization path in `superset/mcp_service/server.py:241-244`:
oneOf/anyOf
string variants are serialized via `json.dumps(value)`.
3. Trace downstream parser in
`superset/mcp_service/utils/schema_utils.py:186-204` (via
`parse_json_or_passthrough` at `:97-105`): request strings must be valid
JSON for model
parsing.
4. Therefore, a future regression that returns a non-JSON string would still
satisfy
current test type-check, allowing CI to miss a real call_tool parsing
failure.
```
</details>
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** tests/unit_tests/mcp_service/test_tool_search_transform.py
**Line:** 221:221
**Comment:**
*Logic Error: This test only verifies the result is a string, so
invalid non-JSON strings would still pass and fail to catch real regressions in
serialization behavior. Validate that the serialized payload round-trips to the
original dict.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38775&comment_hash=db0c37dc9e80b6cf31e9d60f585bf8a2d3c433fd52f1691bf377e402369e8e35&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38775&comment_hash=db0c37dc9e80b6cf31e9d60f585bf8a2d3c433fd52f1691bf377e402369e8e35&reaction=dislike'>👎</a>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
