From 1cd349c00d314cf24b26e986ff4d6c0727ceee79 Mon Sep 17 00:00:00 2001 From: Aleksander Grygier Date: Sun, 6 Sep 2026 03:29:20 +0200 Subject: [PATCH] refactor : address review remarks Name the tool-arg string-field pattern, move the file tools' path field aliases and the JSON container gates into lib/constants, and export the write_file / edit_file meta types from $lib/types instead of the parser modules. Assisted-by: pi:zai-org/GLM-5.3 --- .../ChatMessageToolCall/parsers/_shared.ts | 8 +++- .../ChatMessageToolCall/parsers/edit-file.ts | 33 ++----------- .../parsers/run-javascript.ts | 3 +- .../ChatMessageToolCall/parsers/write-file.ts | 30 ++---------- tools/ui/src/lib/constants/index.ts | 1 + .../lib/constants/tool-call-args.constants.ts | 23 +++++++++ tools/ui/src/lib/types/index.ts | 11 ++++- tools/ui/src/lib/types/tools.d.ts | 47 +++++++++++++++++++ tools/ui/src/lib/utils/tool-call-meta.ts | 4 +- tools/ui/tests/unit/tool-calls.test.ts | 5 +- 10 files changed, 100 insertions(+), 65 deletions(-) create mode 100644 tools/ui/src/lib/constants/tool-call-args.constants.ts diff --git a/tools/ui/src/lib/components/app/chat/ChatMessages/ChatMessage/ChatMessageToolCall/parsers/_shared.ts b/tools/ui/src/lib/components/app/chat/ChatMessages/ChatMessage/ChatMessageToolCall/parsers/_shared.ts index 7d7f0c315c..79d7c2e27e 100644 --- a/tools/ui/src/lib/components/app/chat/ChatMessages/ChatMessage/ChatMessageToolCall/parsers/_shared.ts +++ b/tools/ui/src/lib/components/app/chat/ChatMessages/ChatMessage/ChatMessageToolCall/parsers/_shared.ts @@ -4,6 +4,7 @@ // args-present check, JSON parse) - keeping them here lets each parser // stay focused on its own format quirks. +import { TOOL_ARG_STRING_FIELD_PATTERN_TEMPLATE } from '$lib/constants'; import { BuiltInTool } from '$lib/enums'; import type { AgenticSection } from '$lib/types/agentic'; import { parsePartialJsonArgs } from '$lib/utils/parse-partial-json-args'; @@ -39,12 +40,15 @@ const toolArgStringRegexes = new Map(); * O(path) instead of O(blob). Returns undefined when the key is missing * or its value is not a string; callers fall back to the full parse. */ -export function extractToolArgString(toolArgs: string, keys: string[]): string | undefined { +export function extractToolArgString( + toolArgs: string, + keys: readonly string[] +): string | undefined { for (const key of keys) { let pattern = toolArgStringRegexes.get(key); if (!pattern) { - pattern = new RegExp(`"${key}"\\s*:\\s*"((?:[^"\\\\]|\\\\.)*)"`); + pattern = new RegExp(TOOL_ARG_STRING_FIELD_PATTERN_TEMPLATE.replace('{key}', key)); toolArgStringRegexes.set(key, pattern); } diff --git a/tools/ui/src/lib/components/app/chat/ChatMessages/ChatMessage/ChatMessageToolCall/parsers/edit-file.ts b/tools/ui/src/lib/components/app/chat/ChatMessages/ChatMessage/ChatMessageToolCall/parsers/edit-file.ts index d88186b581..d711466cb2 100644 --- a/tools/ui/src/lib/components/app/chat/ChatMessages/ChatMessage/ChatMessageToolCall/parsers/edit-file.ts +++ b/tools/ui/src/lib/components/app/chat/ChatMessages/ChatMessage/ChatMessageToolCall/parsers/edit-file.ts @@ -4,38 +4,11 @@ // `error` fields. import { extractToolArgString, parseToolArgs } from './_shared'; -import { FILE_PATH_SEPARATOR_REGEX } from '$lib/constants'; +import { FILE_PATH_SEPARATOR_REGEX, TOOL_ARG_PATH_KEYS } from '$lib/constants'; import { BuiltInTool } from '$lib/enums'; -import type { AgenticSection } from '$lib/types'; +import type { AgenticSection, EditFileEdit, EditFileMeta, EditFileTitleMeta } from '$lib/types'; import { tryParseToolResultObject } from '$lib/utils'; -export type EditFileEdit = { - oldText: string; - newText: string; -}; - -export type EditFileMeta = { - fileName: string; - filePath: string; - edits: EditFileEdit[]; - resultMessage?: string; - editsApplied?: number; - errorMessage?: string; -}; - -/** Everything the block title and status pill show; the full meta (with - * the embedded edit strings) stays body-only so collapsed blocks never - * parse the args blob. */ -export type EditFileTitleMeta = { - fileName: string; - filePath: string; - resultMessage?: string; - editsApplied?: number; - errorMessage?: string; -}; - -const PATH_KEYS = ['path', 'file_path', 'filePath']; - export function parseEditFileMeta(section: AgenticSection): EditFileMeta | null { const args = parseToolArgs(BuiltInTool.SERVER_EDIT_FILE, section, { partial: true }); @@ -102,7 +75,7 @@ export function parseEditFileMeta(section: AgenticSection): EditFileMeta | null export function parseEditFileTitleMeta(section: AgenticSection): EditFileTitleMeta | null { if (section.toolName !== BuiltInTool.SERVER_EDIT_FILE || !section.toolArgs) return null; - let rawPath: string | undefined = extractToolArgString(section.toolArgs, PATH_KEYS); + let rawPath: string | undefined = extractToolArgString(section.toolArgs, TOOL_ARG_PATH_KEYS); if (!rawPath) { const args = parseToolArgs(BuiltInTool.SERVER_EDIT_FILE, section, { partial: true }); diff --git a/tools/ui/src/lib/components/app/chat/ChatMessages/ChatMessage/ChatMessageToolCall/parsers/run-javascript.ts b/tools/ui/src/lib/components/app/chat/ChatMessages/ChatMessage/ChatMessageToolCall/parsers/run-javascript.ts index 4d6910caa3..bd97cd2feb 100644 --- a/tools/ui/src/lib/components/app/chat/ChatMessages/ChatMessage/ChatMessageToolCall/parsers/run-javascript.ts +++ b/tools/ui/src/lib/components/app/chat/ChatMessages/ChatMessage/ChatMessageToolCall/parsers/run-javascript.ts @@ -6,6 +6,7 @@ // are handled. import { parseToolArgs } from './_shared'; +import { JSON_ARRAY_OPEN, JSON_OBJECT_OPEN } from '$lib/constants'; import { BuiltInTool } from '$lib/enums'; import type { AgenticSection } from '$lib/types'; @@ -43,7 +44,7 @@ export function parseRunJavascriptMeta(section: AgenticSection): RunJavascriptMe // when the blob starts with a JSON container const trimmedResult = toolResultString.trimStart(); - if (trimmedResult[0] === '{' || trimmedResult[0] === '[') { + if (trimmedResult[0] === JSON_OBJECT_OPEN || trimmedResult[0] === JSON_ARRAY_OPEN) { try { const parsed: unknown = JSON.parse(trimmedResult); diff --git a/tools/ui/src/lib/components/app/chat/ChatMessages/ChatMessage/ChatMessageToolCall/parsers/write-file.ts b/tools/ui/src/lib/components/app/chat/ChatMessages/ChatMessage/ChatMessageToolCall/parsers/write-file.ts index de40bdc80b..4a8e1a9c98 100644 --- a/tools/ui/src/lib/components/app/chat/ChatMessages/ChatMessage/ChatMessageToolCall/parsers/write-file.ts +++ b/tools/ui/src/lib/components/app/chat/ChatMessages/ChatMessage/ChatMessageToolCall/parsers/write-file.ts @@ -4,35 +4,11 @@ // result blob. import { extractToolArgString, parseToolArgs } from './_shared'; -import { CODE_BLOCK, FILE_PATH_SEPARATOR_REGEX } from '$lib/constants'; +import { CODE_BLOCK, FILE_PATH_SEPARATOR_REGEX, TOOL_ARG_PATH_KEYS } from '$lib/constants'; import { BuiltInTool } from '$lib/enums'; -import type { AgenticSection } from '$lib/types'; +import type { AgenticSection, WriteFileMeta, WriteFileTitleMeta } from '$lib/types'; import { getFileTypeByExtension, tryParseToolResultObject } from '$lib/utils'; -export type WriteFileMeta = { - fileName: string; - filePath: string; - language: string; - content: string; - bytesWritten?: number; - resultMessage?: string; - errorMessage?: string; -}; - -/** Everything the block title and status pill show; the full meta (with - * the embedded file content) stays body-only so collapsed blocks never - * parse the content blob. */ -export type WriteFileTitleMeta = { - fileName: string; - filePath: string; - language: string; - bytesWritten?: number; - resultMessage?: string; - errorMessage?: string; -}; - -const PATH_KEYS = ['path', 'file_path', 'filePath']; - export function parseWriteFileMeta(section: AgenticSection): WriteFileMeta | null { const args = parseToolArgs(BuiltInTool.SERVER_WRITE_FILE, section, { partial: true }); @@ -75,7 +51,7 @@ export function parseWriteFileMeta(section: AgenticSection): WriteFileMeta | nul export function parseWriteFileTitleMeta(section: AgenticSection): WriteFileTitleMeta | null { if (section.toolName !== BuiltInTool.SERVER_WRITE_FILE || !section.toolArgs) return null; - let rawPath: string | undefined = extractToolArgString(section.toolArgs, PATH_KEYS); + let rawPath: string | undefined = extractToolArgString(section.toolArgs, TOOL_ARG_PATH_KEYS); if (!rawPath) { const args = parseToolArgs(BuiltInTool.SERVER_WRITE_FILE, section, { partial: true }); diff --git a/tools/ui/src/lib/constants/index.ts b/tools/ui/src/lib/constants/index.ts index e3241373e8..d93ae64293 100644 --- a/tools/ui/src/lib/constants/index.ts +++ b/tools/ui/src/lib/constants/index.ts @@ -16,6 +16,7 @@ export * from './context-gauge-popup.constants'; export * from './conversation-import.constants'; export * from './binary-detection.constants'; export * from './content-detection.constants'; +export * from './tool-call-args.constants'; export * from './tool-ui.constants'; export * from './cache.constants'; export * from './chat-form.constants'; diff --git a/tools/ui/src/lib/constants/tool-call-args.constants.ts b/tools/ui/src/lib/constants/tool-call-args.constants.ts new file mode 100644 index 0000000000..e74260be29 --- /dev/null +++ b/tools/ui/src/lib/constants/tool-call-args.constants.ts @@ -0,0 +1,23 @@ +// Tool-args and tool-result parsing helpers: the file tools' path field +// aliases, the JSON container gates for result blobs, and the targeted +// string-field pattern used for cheap title-tier extraction. + +/** + * Field aliases the file tools accept for the path argument. Tool contracts + * drifted over time: some models emit `file_path` / `filePath`. + */ +export const TOOL_ARG_PATH_KEYS: readonly string[] = ['path', 'file_path', 'filePath']; + +/** Opening character of a JSON object; only an object root can carry fields. */ +export const JSON_OBJECT_OPEN = '{'; + +/** Opening character of a JSON array; successful sandbox output is one. */ +export const JSON_ARRAY_OPEN = '['; + +/** + * Matches `"": ""` in a JSON args blob ( whitespace between + * tokens allowed ), capturing the raw string literal so only that literal + * gets decoded; escaped quotes stay inside the value group. `{key}` is + * replaced with the field name before use. + */ +export const TOOL_ARG_STRING_FIELD_PATTERN_TEMPLATE = '"{key}"\\s*:\\s*"((?:[^"\\\\]|\\\\.)*)"'; diff --git a/tools/ui/src/lib/types/index.ts b/tools/ui/src/lib/types/index.ts index d91c2811a4..333c1bd3cd 100644 --- a/tools/ui/src/lib/types/index.ts +++ b/tools/ui/src/lib/types/index.ts @@ -209,7 +209,16 @@ export type { export type { DesktopIconStripItem } from './navigation'; // Tools types -export type { ToolEntry, ToolGroup, ToolUiEntry } from './tools'; +export type { + EditFileEdit, + EditFileMeta, + EditFileTitleMeta, + ToolEntry, + ToolGroup, + ToolUiEntry, + WriteFileMeta, + WriteFileTitleMeta +} from './tools'; // Reasoning export type { ReasoningEffortLevel } from './reasoning'; diff --git a/tools/ui/src/lib/types/tools.d.ts b/tools/ui/src/lib/types/tools.d.ts index edcec65c70..fa8963bd1f 100644 --- a/tools/ui/src/lib/types/tools.d.ts +++ b/tools/ui/src/lib/types/tools.d.ts @@ -31,3 +31,50 @@ export interface ToolGroup { serverId?: string; tools: ToolEntry[]; } + +export interface WriteFileMeta { + fileName: string; + filePath: string; + language: string; + content: string; + bytesWritten?: number; + resultMessage?: string; + errorMessage?: string; +} + +/** Everything the write_file block title and status pill show; the full meta + * ( with the embedded file content ) stays body-only so collapsed blocks + * never parse the content blob. */ +export interface WriteFileTitleMeta { + fileName: string; + filePath: string; + language: string; + bytesWritten?: number; + resultMessage?: string; + errorMessage?: string; +} + +export interface EditFileEdit { + oldText: string; + newText: string; +} + +export interface EditFileMeta { + fileName: string; + filePath: string; + edits: EditFileEdit[]; + resultMessage?: string; + editsApplied?: number; + errorMessage?: string; +} + +/** Everything the edit_file block title and status pill show; the full meta + * ( with the embedded edit strings ) stays body-only so collapsed blocks + * never parse the args blob. */ +export interface EditFileTitleMeta { + fileName: string; + filePath: string; + resultMessage?: string; + editsApplied?: number; + errorMessage?: string; +} diff --git a/tools/ui/src/lib/utils/tool-call-meta.ts b/tools/ui/src/lib/utils/tool-call-meta.ts index 75ecdccd5a..2c035446d3 100644 --- a/tools/ui/src/lib/utils/tool-call-meta.ts +++ b/tools/ui/src/lib/utils/tool-call-meta.ts @@ -4,6 +4,8 @@ // Each tool needs to surface fields like `error`, `result`, `bytes`, // `edits_applied` without repeating the try/JSON.parse/object guard inline. +import { JSON_OBJECT_OPEN } from '$lib/constants'; + /** * Parse a tool-result blob into a JSON object, or `null` if it isn't * one. Returns null for: @@ -20,7 +22,7 @@ export function tryParseToolResultObject( // a JSON object root can carry fields, so skip the parse otherwise const trimmed = toolResultString.trimStart(); - if (trimmed[0] !== '{') return null; + if (trimmed[0] !== JSON_OBJECT_OPEN) return null; try { const parsed: unknown = JSON.parse(trimmed); diff --git a/tools/ui/tests/unit/tool-calls.test.ts b/tools/ui/tests/unit/tool-calls.test.ts index d1c5272932..a2274f9d28 100644 --- a/tools/ui/tests/unit/tool-calls.test.ts +++ b/tools/ui/tests/unit/tool-calls.test.ts @@ -10,11 +10,10 @@ import { parseReadFileMeta } from '$lib/components/app/chat/ChatMessages/ChatMes import { parseRunJavascriptMeta } from '$lib/components/app/chat/ChatMessages/ChatMessage/ChatMessageToolCall/parsers/run-javascript'; import { parseWriteFileMeta, - parseWriteFileTitleMeta, - type WriteFileMeta + parseWriteFileTitleMeta } from '$lib/components/app/chat/ChatMessages/ChatMessage/ChatMessageToolCall/parsers/write-file'; import { AgenticSectionType, BuiltInTool } from '$lib/enums'; -import type { AgenticSection } from '$lib/types'; +import type { AgenticSection, WriteFileMeta } from '$lib/types'; import { abbreviateHome, formatCwdMessage, lastPathSegment, parseCwdMessage } from '$lib/utils'; import { describe, expect, it } from 'vitest';