Browse Source

Fix: Allow write_to_file to handle newline-only and empty content (#3550)

* Fix: Allow write_to_file to handle newline-only and empty content

* fix: update writeToFileTool to return early without error on missing or empty parameters

* fix: preserve newlines in content parameters and update error handling in writeToFileTool tests

* fix: update parseAssistantMessage and parseAssistantMessageV2 to preserve newlines in content parameters while stripping leading and trailing newlines

---------

Co-authored-by: Daniel Riccio <[email protected]>
Thomas Brugman 8 months ago
parent
commit
245c8f3269

+ 13 - 3
src/core/assistant-message/parseAssistantMessage.ts

@@ -24,7 +24,12 @@ export function parseAssistantMessage(assistantMessage: string): AssistantMessag
 			const paramClosingTag = `</${currentParamName}>`
 			if (currentParamValue.endsWith(paramClosingTag)) {
 				// End of param value.
-				currentToolUse.params[currentParamName] = currentParamValue.slice(0, -paramClosingTag.length).trim()
+				// Don't trim content parameters to preserve newlines, but strip first and last newline only
+				const paramValue = currentParamValue.slice(0, -paramClosingTag.length)
+				currentToolUse.params[currentParamName] =
+					currentParamName === "content"
+						? paramValue.replace(/^\n/, "").replace(/\n$/, "")
+						: paramValue.trim()
 				currentParamName = undefined
 				continue
 			} else {
@@ -72,9 +77,11 @@ export function parseAssistantMessage(assistantMessage: string): AssistantMessag
 					const contentEndIndex = toolContent.lastIndexOf(contentEndTag)
 
 					if (contentStartIndex !== -1 && contentEndIndex !== -1 && contentEndIndex > contentStartIndex) {
+						// Don't trim content to preserve newlines, but strip first and last newline only
 						currentToolUse.params[contentParamName] = toolContent
 							.slice(contentStartIndex, contentEndIndex)
-							.trim()
+							.replace(/^\n/, "")
+							.replace(/\n$/, "")
 					}
 				}
 
@@ -138,7 +145,10 @@ export function parseAssistantMessage(assistantMessage: string): AssistantMessag
 		// Stream did not complete tool call, add it as partial.
 		if (currentParamName) {
 			// Tool call has a parameter that was not completed.
-			currentToolUse.params[currentParamName] = accumulator.slice(currentParamValueStartIndex).trim()
+			// Don't trim content parameters to preserve newlines, but strip first and last newline only
+			const paramValue = accumulator.slice(currentParamValueStartIndex)
+			currentToolUse.params[currentParamName] =
+				currentParamName === "content" ? paramValue.replace(/^\n/, "").replace(/\n$/, "") : paramValue.trim()
 		}
 
 		contentBlocks.push(currentToolUse)

+ 14 - 12
src/core/assistant-message/parseAssistantMessageV2.ts

@@ -76,13 +76,13 @@ export function parseAssistantMessageV2(assistantMessage: string): AssistantMess
 				)
 			) {
 				// Found the closing tag for the parameter.
-				const value = assistantMessage
-					.slice(
-						currentParamValueStart, // Start after the opening tag.
-						currentCharIndex - closeTag.length + 1, // End before the closing tag.
-					)
-					.trim()
-				currentToolUse.params[currentParamName] = value
+				const value = assistantMessage.slice(
+					currentParamValueStart, // Start after the opening tag.
+					currentCharIndex - closeTag.length + 1, // End before the closing tag.
+				)
+				// Don't trim content parameters to preserve newlines, but strip first and last newline only
+				currentToolUse.params[currentParamName] =
+					currentParamName === "content" ? value.replace(/^\n/, "").replace(/\n$/, "") : value.trim()
 				currentParamName = undefined // Go back to parsing tool content.
 				// We don't continue loop here, need to check for tool close or other params at index i.
 			} else {
@@ -146,10 +146,11 @@ export function parseAssistantMessageV2(assistantMessage: string): AssistantMess
 					const contentEnd = toolContentSlice.lastIndexOf(contentEndTag)
 
 					if (contentStart !== -1 && contentEnd !== -1 && contentEnd > contentStart) {
+						// Don't trim content to preserve newlines, but strip first and last newline only
 						const contentValue = toolContentSlice
 							.slice(contentStart + contentStartTag.length, contentEnd)
-							.trim()
-
+							.replace(/^\n/, "")
+							.replace(/\n$/, "")
 						currentToolUse.params[contentParamName] = contentValue
 					}
 				}
@@ -251,9 +252,10 @@ export function parseAssistantMessageV2(assistantMessage: string): AssistantMess
 
 	// Finalize any open parameter within an open tool use.
 	if (currentToolUse && currentParamName) {
-		currentToolUse.params[currentParamName] = assistantMessage
-			.slice(currentParamValueStart) // From param start to end of string.
-			.trim()
+		const value = assistantMessage.slice(currentParamValueStart) // From param start to end of string.
+		// Don't trim content parameters to preserve newlines, but strip first and last newline only
+		currentToolUse.params[currentParamName] =
+			currentParamName === "content" ? value.replace(/^\n/, "").replace(/\n$/, "") : value.trim()
 		// Tool use remains partial.
 	}
 

+ 2 - 2
src/core/tools/writeToFileTool.ts

@@ -73,11 +73,11 @@ export async function writeToFileTool(
 	// pre-processing newContent for cases where weaker models might add artifacts like markdown codeblock markers (deepseek/llama) or extra escape characters (gemini)
 	if (newContent.startsWith("```")) {
 		// cline handles cases where it includes language specifiers like ```python ```js
-		newContent = newContent.split("\n").slice(1).join("\n").trim()
+		newContent = newContent.split("\n").slice(1).join("\n")
 	}
 
 	if (newContent.endsWith("```")) {
-		newContent = newContent.split("\n").slice(0, -1).join("\n").trim()
+		newContent = newContent.split("\n").slice(0, -1).join("\n")
 	}
 
 	if (!cline.api.getModel().id.includes("claude")) {

+ 5 - 5
src/integrations/editor/DiffViewProvider.ts

@@ -130,7 +130,8 @@ export class DiffViewProvider {
 		// Replace all content up to the current line with accumulated lines.
 		const edit = new vscode.WorkspaceEdit()
 		const rangeToReplace = new vscode.Range(0, 0, endLine, 0)
-		const contentToReplace = accumulatedLines.slice(0, endLine + 1).join("\n") + "\n"
+		const contentToReplace =
+			accumulatedLines.slice(0, endLine).join("\n") + (accumulatedLines.length > 0 ? "\n" : "")
 		edit.replace(document.uri, rangeToReplace, this.stripAllBOMs(contentToReplace))
 		await vscode.workspace.applyEdit(edit)
 		// Update decorations.
@@ -230,12 +231,11 @@ export class DiffViewProvider {
 		// show a diff with all the EOL differences.
 		const newContentEOL = this.newContent.includes("\r\n") ? "\r\n" : "\n"
 
-		// `trimEnd` to fix issue where editor adds in extra new line
-		// automatically.
-		const normalizedEditedContent = editedContent.replace(/\r\n|\n/g, newContentEOL).trimEnd() + newContentEOL
+		// Normalize EOL characters without trimming content
+		const normalizedEditedContent = editedContent.replace(/\r\n|\n/g, newContentEOL)
 
 		// Just in case the new content has a mix of varying EOL characters.
-		const normalizedNewContent = this.newContent.replace(/\r\n|\n/g, newContentEOL).trimEnd() + newContentEOL
+		const normalizedNewContent = this.newContent.replace(/\r\n|\n/g, newContentEOL)
 
 		if (normalizedEditedContent !== normalizedNewContent) {
 			// User made changes before approving edit.