From 6938fd1d17fd4b87e151bbe7652e95d3943dbd99 Mon Sep 17 00:00:00 2001 From: Cristhian Zanforlin Lousa Date: Tue, 7 Apr 2026 11:17:23 -0300 Subject: [PATCH] fix: Display proper error messages and strip null params from tool calls (#12437) --- .../tests/unit/base/mcp/test_mcp_util.py | 125 +++++++++++++++ .../extract-api-error-message.test.ts | 150 ++++++++++++++++++ .../API/helpers/extract-api-error-message.ts | 35 ++++ .../file-management/use-post-upload-file.ts | 14 ++ .../API/queries/mcp/use-add-mcp-server.ts | 15 +- .../API/queries/mcp/use-delete-mcp-server.ts | 15 +- .../API/queries/mcp/use-patch-install-mcp.ts | 15 +- .../API/queries/mcp/use-patch-mcp-server.ts | 15 +- .../src/modals/addMcpServerModal/index.tsx | 15 +- .../Text Sentiment Analysis.spec.ts | 14 +- .../core/unit/fileUploadComponent.spec.ts | 15 +- .../tests/utils/ensure-checkbox-checked.ts | 30 ++-- src/frontend/tests/utils/upload-file.ts | 5 +- src/lfx/src/lfx/base/mcp/util.py | 21 ++- .../components/processing/parse_json_data.py | 7 +- 15 files changed, 409 insertions(+), 82 deletions(-) create mode 100644 src/frontend/src/controllers/API/helpers/__tests__/extract-api-error-message.test.ts create mode 100644 src/frontend/src/controllers/API/helpers/extract-api-error-message.ts diff --git a/src/backend/tests/unit/base/mcp/test_mcp_util.py b/src/backend/tests/unit/base/mcp/test_mcp_util.py index 62522f336e..d43adad79c 100644 --- a/src/backend/tests/unit/base/mcp/test_mcp_util.py +++ b/src/backend/tests/unit/base/mcp/test_mcp_util.py @@ -2557,3 +2557,128 @@ class TestSnakeToCamelConversion: # Metadata fields assert _snake_to_camel("_meta_data") == "_metaData" assert _snake_to_camel("_created_at") == "_createdAt" + + +class TestStripNoneRecursive: + """Tests for _strip_none_recursive. + + Ensures null values are removed from nested dicts and arrays before + sending arguments to MCP servers. + + Bug: antvis/mcp-server-chart returns "Expected string, received null" + because LLMs send explicit null for optional fields inside arrays of objects. + """ + + def test_should_remove_none_from_flat_dict(self): + """Top-level None values must be stripped.""" + from lfx.base.mcp.util import _strip_none_recursive + + # Arrange + data = {"name": "chart", "style": None, "title": "My Chart"} + + # Act + result = _strip_none_recursive(data) + + # Assert + assert result == {"name": "chart", "title": "My Chart"} + assert "style" not in result + + def test_should_remove_none_from_nested_objects_in_arrays(self): + """The exact bug scenario: data[N].group = null inside an array.""" + from lfx.base.mcp.util import _strip_none_recursive + + # Arrange — reproduces the exact payload from the bug report + data = { + "data": [ + {"name": "Revenue", "value": 100, "group": None}, + {"name": "Costs", "value": 50, "group": None}, + {"name": "Profit", "value": 50, "group": None}, + ], + "style": None, + } + + # Act + result = _strip_none_recursive(data) + + # Assert — group and style must be absent + assert result == { + "data": [ + {"name": "Revenue", "value": 100}, + {"name": "Costs", "value": 50}, + {"name": "Profit", "value": 50}, + ], + } + assert "style" not in result + for item in result["data"]: + assert "group" not in item + + def test_should_handle_deeply_nested_none(self): + """None values several levels deep must also be stripped.""" + from lfx.base.mcp.util import _strip_none_recursive + + # Arrange + data = { + "config": { + "axis": {"label": None, "color": "red"}, + "legend": None, + } + } + + # Act + result = _strip_none_recursive(data) + + # Assert + assert result == {"config": {"axis": {"color": "red"}}} + + def test_should_preserve_falsy_non_none_values(self): + """Zero, empty string, False, empty list, empty dict must NOT be stripped.""" + from lfx.base.mcp.util import _strip_none_recursive + + # Arrange + data = { + "count": 0, + "label": "", + "enabled": False, + "items": [], + "meta": {}, + } + + # Act + result = _strip_none_recursive(data) + + # Assert — all falsy-but-not-None values preserved + assert result == { + "count": 0, + "label": "", + "enabled": False, + "items": [], + "meta": {}, + } + + def test_should_return_primitives_unchanged(self): + """Non-dict, non-list values pass through unchanged.""" + from lfx.base.mcp.util import _strip_none_recursive + + assert _strip_none_recursive("hello") == "hello" + assert _strip_none_recursive(42) == 42 + assert _strip_none_recursive(True) is True # noqa: FBT003 + + def test_should_handle_empty_structures(self): + """Empty dict and empty list return empty.""" + from lfx.base.mcp.util import _strip_none_recursive + + assert _strip_none_recursive({}) == {} + assert _strip_none_recursive([]) == [] + + def test_should_handle_list_of_primitives_with_none(self): + """None items inside a plain list are NOT removed (only dict keys).""" + from lfx.base.mcp.util import _strip_none_recursive + + # Arrange — list items that are None stay (we only strip dict keys) + data = [1, None, "hello", None] + + # Act + result = _strip_none_recursive(data) + + # Assert — None items in list preserved (stripping applies to dict keys only) + assert result == [1, None, "hello", None] diff --git a/src/frontend/src/controllers/API/helpers/__tests__/extract-api-error-message.test.ts b/src/frontend/src/controllers/API/helpers/__tests__/extract-api-error-message.test.ts new file mode 100644 index 0000000000..03d0df85f7 --- /dev/null +++ b/src/frontend/src/controllers/API/helpers/__tests__/extract-api-error-message.test.ts @@ -0,0 +1,150 @@ +import { extractApiErrorMessage } from "../extract-api-error-message"; + +describe("extractApiErrorMessage", () => { + const FALLBACK = "Default error"; + + it("should_return_string_detail_when_detail_is_string", () => { + // Arrange — simple string detail (e.g. HTTPException) + const error = { + response: { data: { detail: "Server not found" } }, + }; + + // Act + const result = extractApiErrorMessage(error, FALLBACK); + + // Assert + expect(result).toBe("Server not found"); + }); + + it("should_return_joined_msg_fields_when_detail_is_array_of_validation_errors", () => { + // Arrange — FastAPI RequestValidationError format (the exact bug scenario) + const error = { + response: { + data: { + detail: [ + { + type: "value_error", + loc: ["body", "args"], + msg: "Value error, Argument '-y' is not allowed for security reasons", + input: ["-y", "@antv/mcp-server-chart"], + ctx: { + error: "Argument '-y' is not allowed for security reasons", + }, + }, + ], + }, + }, + }; + + // Act + const result = extractApiErrorMessage(error, FALLBACK); + + // Assert — must be the readable msg, NOT "[object Object]" + expect(result).toBe( + "Value error, Argument '-y' is not allowed for security reasons", + ); + expect(result).not.toContain("[object Object]"); + }); + + it("should_join_multiple_validation_errors_with_semicolon", () => { + // Arrange — multiple validation errors in one response + const error = { + response: { + data: { + detail: [ + { msg: "Field 'name' is required" }, + { msg: "Field 'command' is required" }, + ], + }, + }, + }; + + // Act + const result = extractApiErrorMessage(error, FALLBACK); + + // Assert + expect(result).toBe( + "Field 'name' is required; Field 'command' is required", + ); + }); + + it("should_stringify_array_items_without_msg_field", () => { + // Arrange — array items that don't have a msg field + const error = { + response: { + data: { + detail: [{ error: "something went wrong" }], + }, + }, + }; + + // Act + const result = extractApiErrorMessage(error, FALLBACK); + + // Assert — should use String() coercion, not crash + expect(result).toBe("[object Object]"); + }); + + it("should_extract_msg_from_single_object_detail", () => { + // Arrange — detail is a single object with msg + const error = { + response: { data: { detail: { msg: "Invalid configuration" } } }, + }; + + // Act + const result = extractApiErrorMessage(error, FALLBACK); + + // Assert + expect(result).toBe("Invalid configuration"); + }); + + it("should_extract_message_from_single_object_detail", () => { + // Arrange — detail is a single object with message (not msg) + const error = { + response: { + data: { detail: { message: "Connection refused" } }, + }, + }; + + // Act + const result = extractApiErrorMessage(error, FALLBACK); + + // Assert + expect(result).toBe("Connection refused"); + }); + + it("should_json_stringify_object_without_msg_or_message", () => { + // Arrange — object detail without known keys + const error = { + response: { data: { detail: { code: 500, info: "crash" } } }, + }; + + // Act + const result = extractApiErrorMessage(error, FALLBACK); + + // Assert + expect(result).toBe('{"code":500,"info":"crash"}'); + }); + + it("should_use_error_message_when_no_detail", () => { + // Arrange — no response.data.detail, just error.message + const error = { message: "Network Error" }; + + // Act + const result = extractApiErrorMessage(error, FALLBACK); + + // Assert + expect(result).toBe("Network Error"); + }); + + it("should_use_fallback_when_no_detail_and_no_message", () => { + // Arrange — completely empty error + const error = {}; + + // Act + const result = extractApiErrorMessage(error, FALLBACK); + + // Assert + expect(result).toBe(FALLBACK); + }); +}); diff --git a/src/frontend/src/controllers/API/helpers/extract-api-error-message.ts b/src/frontend/src/controllers/API/helpers/extract-api-error-message.ts new file mode 100644 index 0000000000..dae7ac4f89 --- /dev/null +++ b/src/frontend/src/controllers/API/helpers/extract-api-error-message.ts @@ -0,0 +1,35 @@ +/** + * Extracts a human-readable error message from an Axios error response. + * + * FastAPI validation errors return `detail` as an array of objects + * (e.g. [{type, loc, msg, input, ctx}]). Directly coercing these + * to a string produces "[object Object]". This helper normalises + * all known shapes of `detail` into a readable string. + */ +export function extractApiErrorMessage( + error: { response?: { data?: { detail?: unknown } }; message?: string }, + fallback: string, +): string { + const detail = error.response?.data?.detail; + + if (typeof detail === "string") { + return detail; + } + + if (Array.isArray(detail)) { + return detail + .map((d: Record) => + typeof d.msg === "string" ? d.msg : String(d), + ) + .join("; "); + } + + if (detail && typeof detail === "object") { + const obj = detail as Record; + if (typeof obj.msg === "string") return obj.msg; + if (typeof obj.message === "string") return obj.message; + return JSON.stringify(detail); + } + + return error.message || fallback; +} diff --git a/src/frontend/src/controllers/API/queries/file-management/use-post-upload-file.ts b/src/frontend/src/controllers/API/queries/file-management/use-post-upload-file.ts index 57bbd68c93..24773a0dec 100644 --- a/src/frontend/src/controllers/API/queries/file-management/use-post-upload-file.ts +++ b/src/frontend/src/controllers/API/queries/file-management/use-post-upload-file.ts @@ -92,6 +92,20 @@ export const usePostUploadFileV2: useMutationFunctionType< }, }, ); + + // Replace the optimistic "temp" entry with the actual server data so + // that file.path in the cache matches the path returned to callers + // (via handleUpload → internalSelectedFiles). Without this, there is + // a race window where the optimistic path (just the filename) differs + // from the server path (user_id/filename), causing checkboxes to + // appear unchecked until the background refetch completes. + queryClient.setQueryData(["useGetFilesV2"], (old: any) => { + if (!Array.isArray(old)) return [response.data]; + return old.map((file: any) => + file?.id === "temp" ? { ...response.data } : file, + ); + }); + return response.data; } catch (e) { queryClient.setQueryData(["useGetFilesV2"], (old: FileType[]) => { diff --git a/src/frontend/src/controllers/API/queries/mcp/use-add-mcp-server.ts b/src/frontend/src/controllers/API/queries/mcp/use-add-mcp-server.ts index 0202a1cffc..e4e6c635d8 100644 --- a/src/frontend/src/controllers/API/queries/mcp/use-add-mcp-server.ts +++ b/src/frontend/src/controllers/API/queries/mcp/use-add-mcp-server.ts @@ -3,6 +3,7 @@ import type { useMutationFunctionType } from "@/types/api"; import type { MCPServerType } from "@/types/mcp"; import { api } from "../../api"; import { getURL } from "../../helpers/constants"; +import { extractApiErrorMessage } from "../../helpers/extract-api-error-message"; import { UseRequestProcessor } from "../../services/request-processor"; interface AddMCPServerResponse { @@ -44,13 +45,13 @@ export const useAddMCPServer: useMutationFunctionType< ); return { message: res.data?.message || "MCP Server added successfully" }; - } catch (error: any) { - // Transform the error to include a message that can be handled by the UI - const errorMessage = - error.response?.data?.detail || - error.message || - "Failed to install MCP"; - throw new Error(errorMessage); + } catch (error: unknown) { + throw new Error( + extractApiErrorMessage( + error as Parameters[0], + "Failed to install MCP", + ), + ); } } diff --git a/src/frontend/src/controllers/API/queries/mcp/use-delete-mcp-server.ts b/src/frontend/src/controllers/API/queries/mcp/use-delete-mcp-server.ts index a3e0f1cad0..99e14004cb 100644 --- a/src/frontend/src/controllers/API/queries/mcp/use-delete-mcp-server.ts +++ b/src/frontend/src/controllers/API/queries/mcp/use-delete-mcp-server.ts @@ -3,6 +3,7 @@ import type { useMutationFunctionType } from "@/types/api"; import type { MCPServerType } from "@/types/mcp"; import { api } from "../../api"; import { getURL } from "../../helpers/constants"; +import { extractApiErrorMessage } from "../../helpers/extract-api-error-message"; import { UseRequestProcessor } from "../../services/request-processor"; interface DeleteMCPServerResponse { @@ -31,13 +32,13 @@ export const useDeleteMCPServer: useMutationFunctionType< return { message: res.data?.message || "MCP Server deleted successfully", }; - } catch (error: any) { - // Transform the error to include a message that can be handled by the UI - const errorMessage = - error.response?.data?.detail || - error.message || - "Failed to delete MCP Server"; - throw new Error(errorMessage); + } catch (error: unknown) { + throw new Error( + extractApiErrorMessage( + error as Parameters[0], + "Failed to delete MCP Server", + ), + ); } } diff --git a/src/frontend/src/controllers/API/queries/mcp/use-patch-install-mcp.ts b/src/frontend/src/controllers/API/queries/mcp/use-patch-install-mcp.ts index 2df8f8b88b..c1f25c2b05 100644 --- a/src/frontend/src/controllers/API/queries/mcp/use-patch-install-mcp.ts +++ b/src/frontend/src/controllers/API/queries/mcp/use-patch-install-mcp.ts @@ -2,6 +2,7 @@ import type { UseMutationResult } from "@tanstack/react-query"; import type { useMutationFunctionType } from "@/types/api"; import { api } from "../../api"; import { getURL } from "../../helpers/constants"; +import { extractApiErrorMessage } from "../../helpers/extract-api-error-message"; import { UseRequestProcessor } from "../../services/request-processor"; interface PatchInstallMCPParams { @@ -36,13 +37,13 @@ export const usePatchInstallMCP: useMutationFunctionType< ); return { message: res.data?.message || "MCP installed successfully" }; - } catch (error: any) { - // Transform the error to include a message that can be handled by the UI - const errorMessage = - error.response?.data?.detail || - error.message || - "Failed to install MCP"; - throw new Error(errorMessage); + } catch (error: unknown) { + throw new Error( + extractApiErrorMessage( + error as Parameters[0], + "Failed to install MCP", + ), + ); } } diff --git a/src/frontend/src/controllers/API/queries/mcp/use-patch-mcp-server.ts b/src/frontend/src/controllers/API/queries/mcp/use-patch-mcp-server.ts index 7d71061112..b2885722c7 100644 --- a/src/frontend/src/controllers/API/queries/mcp/use-patch-mcp-server.ts +++ b/src/frontend/src/controllers/API/queries/mcp/use-patch-mcp-server.ts @@ -3,6 +3,7 @@ import type { useMutationFunctionType } from "@/types/api"; import type { MCPServerType } from "@/types/mcp"; import { api } from "../../api"; import { getURL } from "../../helpers/constants"; +import { extractApiErrorMessage } from "../../helpers/extract-api-error-message"; import { UseRequestProcessor } from "../../services/request-processor"; import type { getMCPServersResponse } from "./use-get-mcp-servers"; @@ -58,13 +59,13 @@ export const usePatchMCPServer: useMutationFunctionType< return { message: res.data?.message || "MCP Server patched successfully", }; - } catch (error: any) { - // Transform the error to include a message that can be handled by the UI - const errorMessage = - error.response?.data?.detail || - error.message || - "Failed to patch MCP Server"; - throw new Error(errorMessage); + } catch (error: unknown) { + throw new Error( + extractApiErrorMessage( + error as Parameters[0], + "Failed to patch MCP Server", + ), + ); } } diff --git a/src/frontend/src/modals/addMcpServerModal/index.tsx b/src/frontend/src/modals/addMcpServerModal/index.tsx index 1e22839013..f45d9a54cd 100644 --- a/src/frontend/src/modals/addMcpServerModal/index.tsx +++ b/src/frontend/src/modals/addMcpServerModal/index.tsx @@ -3,7 +3,6 @@ import { nanoid } from "nanoid"; import { useEffect, useState } from "react"; import { useLocation } from "react-router-dom"; import { ForwardedIconComponent } from "@/components/common/genericIconComponent"; -import ShadTooltip from "@/components/common/shadTooltipComponent"; import InputListComponent from "@/components/core/parameterRenderComponent/components/inputListComponent"; import { Button } from "@/components/ui/button"; import { Input } from "@/components/ui/input"; @@ -27,7 +26,6 @@ import IOKeyPairInputWithVariables from "@/modals/IOModal/components/IOFieldView import type { MCPServerType } from "@/types/mcp"; import { extractMcpServersFromJson } from "@/utils/mcpUtils"; import { parseString } from "@/utils/stringManipulation"; -import { cn } from "@/utils/utils"; const MCP_SETTINGS_PAGE = "/settings/mcp-servers"; @@ -364,16 +362,9 @@ export default function AddMcpServerModal({ id="global-variable-modal-inputs" > {error && ( - -
- {error} -
- +
+ {error} +
)} diff --git a/src/frontend/tests/core/integrations/Text Sentiment Analysis.spec.ts b/src/frontend/tests/core/integrations/Text Sentiment Analysis.spec.ts index 71a15f2449..c222d08509 100644 --- a/src/frontend/tests/core/integrations/Text Sentiment Analysis.spec.ts +++ b/src/frontend/tests/core/integrations/Text Sentiment Analysis.spec.ts @@ -4,6 +4,7 @@ import { expect, test } from "../../fixtures"; import { awaitBootstrapTest } from "../../utils/await-bootstrap-test"; import { initialGPTsetup } from "../../utils/initialGPTsetup"; import { unselectNodes } from "../../utils/unselect-nodes"; +import { uploadFile } from "../../utils/upload-file"; import { withEventDeliveryModes } from "../../utils/withEventDeliveryModes"; withEventDeliveryModes( @@ -27,18 +28,7 @@ withEventDeliveryModes( .click(); await initialGPTsetup(page); - await page.getByTestId("input-file-component").last().click(); - const fileChooserPromise = page.waitForEvent("filechooser"); - await page.getByTestId("drag-files-component").last().click(); - - const fileChooser = await fileChooserPromise; - await fileChooser.setFiles( - path.join(__dirname, "../../assets/test_file.txt"), - ); - await page.getByText("test_file.txt").last().isVisible(); - - await page.waitForTimeout(500); - await page.getByTestId("select-files-modal-button").click(); + await uploadFile(page, "test_file.txt"); await page.waitForSelector('[data-testid="title-Chat Output"]', { timeout: 3000, diff --git a/src/frontend/tests/core/unit/fileUploadComponent.spec.ts b/src/frontend/tests/core/unit/fileUploadComponent.spec.ts index f1cd84a641..fc5a26fdfe 100644 --- a/src/frontend/tests/core/unit/fileUploadComponent.spec.ts +++ b/src/frontend/tests/core/unit/fileUploadComponent.spec.ts @@ -4,7 +4,7 @@ import { expect, test } from "../../fixtures"; import { addLegacyComponents } from "../../utils/add-legacy-components"; import { adjustScreenView } from "../../utils/adjust-screen-view"; import { awaitBootstrapTest } from "../../utils/await-bootstrap-test"; -import { ensureCheckboxChecked } from "../../utils/ensure-checkbox-checked"; +import { ensureFileSelected } from "../../utils/ensure-checkbox-checked"; import { generateRandomFilename } from "../../utils/generate-filename"; import { disableInspectPanel, @@ -95,8 +95,7 @@ test( timeout: 5000, }); - const checkbox = page.getByTestId(`checkbox-${sourceFileName}`).last(); - await ensureCheckboxChecked(checkbox); + await ensureFileSelected(page); // Create DataTransfer object and file const dataTransfer = await page.evaluateHandle((jsonFileName) => { @@ -130,16 +129,16 @@ test( await expect( page.getByTestId(`checkbox-${sourceFileName}`).last(), - ).toHaveAttribute("data-state", "checked", { timeout: 1000 }); + ).toHaveAttribute("data-state", "checked", { timeout: 5000 }); // Test checkbox await expect( page.getByTestId(`checkbox-${sourceFileName}`).last(), - ).toHaveAttribute("data-state", "checked"); + ).toHaveAttribute("data-state", "checked", { timeout: 5000 }); await expect( page.getByTestId(`checkbox-${jsonFileName}`).last(), - ).toHaveAttribute("data-state", "checked"); + ).toHaveAttribute("data-state", "checked", { timeout: 5000 }); await page.getByTestId(`checkbox-${sourceFileName}`).last().click(); await page.getByTestId(`checkbox-${jsonFileName}`).last().click(); @@ -240,10 +239,10 @@ test( await expect( page.getByTestId(`checkbox-${renamedTxtFile}`).last(), - ).toHaveAttribute("data-state", "checked"); + ).toHaveAttribute("data-state", "checked", { timeout: 5000 }); await expect( page.getByTestId(`checkbox-${renamedJsonFile}`).last(), - ).toHaveAttribute("data-state", "checked"); + ).toHaveAttribute("data-state", "checked", { timeout: 5000 }); await page.getByTestId("select-files-modal-button").click(); diff --git a/src/frontend/tests/utils/ensure-checkbox-checked.ts b/src/frontend/tests/utils/ensure-checkbox-checked.ts index c987a25f5e..fc6881c682 100644 --- a/src/frontend/tests/utils/ensure-checkbox-checked.ts +++ b/src/frontend/tests/utils/ensure-checkbox-checked.ts @@ -1,21 +1,19 @@ -import type { Locator } from "@playwright/test"; +import type { Page } from "@playwright/test"; import { expect } from "../fixtures"; /** - * Ensures a checkbox reaches the "checked" state. - * On Windows CI, auto-select after upload may not trigger due to a race - * condition between focus/change events. This helper clicks the checkbox - * manually if it's not auto-checked within the initial timeout. + * Ensures a file was selected in the file management modal after upload. + * + * On Windows CI, the upload response path may not exactly match the file.path + * from the list query (due to a query refetch race), which means the checkbox + * visual state may stay "unchecked" even though internalSelectedFiles already + * contains the correct path from handleUpload. Since clicking "Select files" + * submits internalSelectedFiles (not the checkbox state), we only need to + * confirm that at least one file is in the selection — visible via the + * "N selected" counter. */ -export async function ensureCheckboxChecked(checkbox: Locator, timeout = 5000) { - try { - await expect(checkbox).toHaveAttribute("data-state", "checked", { - timeout, - }); - } catch { - await checkbox.click(); - await expect(checkbox).toHaveAttribute("data-state", "checked", { - timeout: 3000, - }); - } +export async function ensureFileSelected(page: Page) { + await expect(page.getByText("selected")).toBeVisible({ + timeout: 5000, + }); } diff --git a/src/frontend/tests/utils/upload-file.ts b/src/frontend/tests/utils/upload-file.ts index 7b840daa68..c5617d76ea 100644 --- a/src/frontend/tests/utils/upload-file.ts +++ b/src/frontend/tests/utils/upload-file.ts @@ -2,7 +2,7 @@ import type { Page } from "@playwright/test"; import fs from "fs"; import path from "path"; import { expect } from "../fixtures"; -import { ensureCheckboxChecked } from "./ensure-checkbox-checked"; +import { ensureFileSelected } from "./ensure-checkbox-checked"; import { generateRandomFilename } from "./generate-filename"; import { unselectNodes } from "./unselect-nodes"; @@ -92,8 +92,7 @@ export async function uploadFile(page: Page, fileName: string) { timeout: 30000, }); - const checkbox = page.getByTestId(`checkbox-${sourceFileName}`).last(); - await ensureCheckboxChecked(checkbox); + await ensureFileSelected(page); await page.getByTestId("select-files-modal-button").click(); diff --git a/src/lfx/src/lfx/base/mcp/util.py b/src/lfx/src/lfx/base/mcp/util.py index 287a5fb617..9c17de17f3 100644 --- a/src/lfx/src/lfx/base/mcp/util.py +++ b/src/lfx/src/lfx/base/mcp/util.py @@ -471,6 +471,21 @@ def _handle_tool_validation_error( raise ValueError(msg) from e +def _strip_none_recursive(obj: Any) -> Any: + """Recursively remove None values from dicts (including inside lists). + + ``model_dump(exclude_none=True)`` handles top-level and nested-model + None fields, but when LLMs explicitly send ``null`` for fields inside + arrays of objects the serialised dict may still contain ``None``. + This helper guarantees a clean payload before it reaches the MCP server. + """ + if isinstance(obj, dict): + return {k: _strip_none_recursive(v) for k, v in obj.items() if v is not None} + if isinstance(obj, list): + return [_strip_none_recursive(item) for item in obj] + return obj + + def create_tool_coroutine(tool_name: str, arg_schema: type[BaseModel], client) -> Callable[..., Awaitable]: async def tool_coroutine(*args, **kwargs): # Get field names from the model (preserving order) @@ -494,7 +509,8 @@ def create_tool_coroutine(tool_name: str, arg_schema: type[BaseModel], client) - _handle_tool_validation_error(e, tool_name, original_args, arg_schema) try: - return await client.run_tool(tool_name, arguments=validated.model_dump(exclude_none=True)) + arguments = _strip_none_recursive(validated.model_dump(exclude_none=True)) + return await client.run_tool(tool_name, arguments=arguments) except Exception as e: await logger.aerror(f"Tool '{tool_name}' execution failed: {e}") # Re-raise with more context @@ -523,7 +539,8 @@ def create_tool_func(tool_name: str, arg_schema: type[BaseModel], client) -> Cal _handle_tool_validation_error(e, tool_name, original_args, arg_schema) try: - return run_until_complete(client.run_tool(tool_name, arguments=validated.model_dump(exclude_none=True))) + arguments = _strip_none_recursive(validated.model_dump(exclude_none=True)) + return run_until_complete(client.run_tool(tool_name, arguments=arguments)) except Exception as e: logger.error(f"Tool '{tool_name}' execution failed: {e}") # Re-raise with more context diff --git a/src/lfx/src/lfx/components/processing/parse_json_data.py b/src/lfx/src/lfx/components/processing/parse_json_data.py index 28aa03eafd..c06c347324 100644 --- a/src/lfx/src/lfx/components/processing/parse_json_data.py +++ b/src/lfx/src/lfx/components/processing/parse_json_data.py @@ -1,7 +1,6 @@ import json from json import JSONDecodeError -import jq from json_repair import repair_json from lfx.custom.custom_component.component import Component @@ -86,6 +85,12 @@ class ParseJSONDataComponent(Component): logger.info("to_filter: %s", to_filter) + try: + import jq + except ImportError: + msg = "jq is required for Parse JSON. Install with: pip install jq" + raise ImportError(msg) from None + results = jq.compile(self.query).input_text(full_filter_str).all() logger.info("results: %s", results) return [Data(data=value) if isinstance(value, dict) else Data(text=str(value)) for value in results]