diff --git a/.changeset/pink-ways-behave.md b/.changeset/pink-ways-behave.md new file mode 100644 index 0000000..7e996cd --- /dev/null +++ b/.changeset/pink-ways-behave.md @@ -0,0 +1,5 @@ +--- +"roo-cline": patch +--- + +Take predicted file length into account when detecting omissions diff --git a/.clinerules b/.clinerules new file mode 100644 index 0000000..9eaef00 --- /dev/null +++ b/.clinerules @@ -0,0 +1 @@ +- Before attempting completion, always make sure that any code changes have test coverage and that the tests pass. \ No newline at end of file diff --git a/src/core/Cline.ts b/src/core/Cline.ts index e3e0f8d..63704c2 100644 --- a/src/core/Cline.ts +++ b/src/core/Cline.ts @@ -1041,6 +1041,7 @@ export class Cline { case "write_to_file": { const relPath: string | undefined = block.params.path let newContent: string | undefined = block.params.content + let predictedLineCount: number | undefined = parseInt(block.params.line_count ?? "0") if (!relPath || !newContent) { // checking for newContent ensure relPath is complete // wait so we can determine if it's a new file or editing an existing file @@ -1109,6 +1110,12 @@ export class Cline { await this.diffViewProvider.reset() break } + if (!predictedLineCount) { + this.consecutiveMistakeCount++ + pushToolResult(await this.sayAndCreateMissingParamError("write_to_file", "line_count")) + await this.diffViewProvider.reset() + break + } this.consecutiveMistakeCount = 0 // if isEditingFile false, that means we have the full contents of the file already. @@ -1125,11 +1132,11 @@ export class Cline { this.diffViewProvider.scrollToFirstDiff() // Check for code omissions before proceeding - if (detectCodeOmission(this.diffViewProvider.originalContent || "", newContent)) { + if (detectCodeOmission(this.diffViewProvider.originalContent || "", newContent, predictedLineCount)) { if (this.diffStrategy) { await this.diffViewProvider.revertChanges() pushToolResult(formatResponse.toolError( - "Content appears to be truncated. Found comments indicating omitted code (e.g., '// rest of code unchanged', '/* previous code */'). Please provide the complete file content without any omissions if possible, or otherwise use the 'apply_diff' tool to apply the diff to the original file." + `Content appears to be truncated (file has ${newContent.split("\n").length} lines but was predicted to have ${predictedLineCount} lines), and found comments indicating omitted code (e.g., '// rest of code unchanged', '/* previous code */'). Please provide the complete file content without any omissions if possible, or otherwise use the 'apply_diff' tool to apply the diff to the original file.` )) break } else { diff --git a/src/core/assistant-message/index.ts b/src/core/assistant-message/index.ts index 241f8c7..353e7b8 100644 --- a/src/core/assistant-message/index.ts +++ b/src/core/assistant-message/index.ts @@ -30,6 +30,7 @@ export const toolParamNames = [ "command", "path", "content", + "line_count", "regex", "file_pattern", "recursive", @@ -71,7 +72,7 @@ export interface ReadFileToolUse extends ToolUse { export interface WriteToFileToolUse extends ToolUse { name: "write_to_file" - params: Partial, "path" | "content">> + params: Partial, "path" | "content" | "line_count">> } export interface SearchFilesToolUse extends ToolUse { diff --git a/src/core/prompts/system.ts b/src/core/prompts/system.ts index 1bb4513..e7ae452 100644 --- a/src/core/prompts/system.ts +++ b/src/core/prompts/system.ts @@ -63,12 +63,14 @@ Description: Request to write full content to a file at the specified path. If t Parameters: - path: (required) The path of the file to write to (relative to the current working directory ${cwd.toPosix()}) - content: (required) The content to write to the file. ALWAYS provide the COMPLETE intended content of the file, without any truncation or omissions. You MUST include ALL parts of the file, even if they haven't been modified. Do NOT include the line numbers in the content though, just the actual content of the file. +- line_count: (required) The number of lines in the file. Make sure to compute this based on the actual content of the file, not the number of lines in the content you're providing. Usage: File path here Your file content here +total number of lines in the file, including empty lines ${diffStrategy ? diffStrategy.getToolDescription(cwd.toPosix()) : ""} @@ -224,6 +226,7 @@ Your final result description here "version": "1.0.0" } +14 ## Example 3: Requesting to use an MCP tool diff --git a/src/integrations/editor/__tests__/detect-omission.test.ts b/src/integrations/editor/__tests__/detect-omission.test.ts index 4740b1f..558617e 100644 --- a/src/integrations/editor/__tests__/detect-omission.test.ts +++ b/src/integrations/editor/__tests__/detect-omission.test.ts @@ -8,59 +8,127 @@ describe('detectCodeOmission', () => { return x + y; }` - it('should detect square bracket line range omission', () => { - const newContent = `[Previous content from line 1-305 remains exactly the same] -const z = 3;` - expect(detectCodeOmission(originalContent, newContent)).toBe(true) - }) + const generateLongContent = (commentLine: string, length: number = 90) => { + return `${commentLine} + ${Array.from({ length }, (_, i) => `const x${i} = ${i};`).join('\n')} + const y = 2;` + } - it('should detect single-line comment omission', () => { + it('should skip comment checks for files under 100 lines', () => { const newContent = `// Lines 1-50 remain unchanged const z = 3;` - expect(detectCodeOmission(originalContent, newContent)).toBe(true) - }) - - it('should detect multi-line comment omission', () => { - const newContent = `/* Previous content remains the same */ -const z = 3;` - expect(detectCodeOmission(originalContent, newContent)).toBe(true) - }) - - it('should detect HTML-style comment omission', () => { - const newContent = ` -const z = 3;` - expect(detectCodeOmission(originalContent, newContent)).toBe(true) - }) - - it('should detect JSX-style comment omission', () => { - const newContent = `{/* Rest of the code remains the same */} -const z = 3;` - expect(detectCodeOmission(originalContent, newContent)).toBe(true) - }) - - it('should detect Python-style comment omission', () => { - const newContent = `# Previous content remains unchanged -const z = 3;` - expect(detectCodeOmission(originalContent, newContent)).toBe(true) + const predictedLineCount = 50 + expect(detectCodeOmission(originalContent, newContent, predictedLineCount)).toBe(false) }) it('should not detect regular comments without omission keywords', () => { - const newContent = `// Adding new functionality -const z = 3;` - expect(detectCodeOmission(originalContent, newContent)).toBe(false) + const newContent = generateLongContent('// Adding new functionality') + const predictedLineCount = 150 + expect(detectCodeOmission(originalContent, newContent, predictedLineCount)).toBe(false) }) it('should not detect when comment is part of original content', () => { const originalWithComment = `// Content remains unchanged ${originalContent}` - const newContent = `// Content remains unchanged -const z = 3;` - expect(detectCodeOmission(originalWithComment, newContent)).toBe(false) + const newContent = generateLongContent('// Content remains unchanged') + const predictedLineCount = 150 + expect(detectCodeOmission(originalWithComment, newContent, predictedLineCount)).toBe(false) }) it('should not detect code that happens to contain omission keywords', () => { - const newContent = `const remains = 'some value'; -const unchanged = true;` - expect(detectCodeOmission(originalContent, newContent)).toBe(false) + const newContent = generateLongContent(`const remains = 'some value'; +const unchanged = true;`) + const predictedLineCount = 150 + expect(detectCodeOmission(originalContent, newContent, predictedLineCount)).toBe(false) }) -}) \ No newline at end of file + + it('should detect suspicious single-line comment when content is more than 20% shorter', () => { + const newContent = generateLongContent('// Previous content remains here\nconst x = 1;') + const predictedLineCount = 150 + expect(detectCodeOmission(originalContent, newContent, predictedLineCount)).toBe(true) + }) + + it('should not flag suspicious single-line comment when content is less than 20% shorter', () => { + const newContent = generateLongContent('// Previous content remains here', 130) + const predictedLineCount = 150 + expect(detectCodeOmission(originalContent, newContent, predictedLineCount)).toBe(false) + }) + + it('should detect suspicious Python-style comment when content is more than 20% shorter', () => { + const newContent = generateLongContent('# Previous content remains here\nconst x = 1;') + const predictedLineCount = 150 + expect(detectCodeOmission(originalContent, newContent, predictedLineCount)).toBe(true) + }) + + it('should not flag suspicious Python-style comment when content is less than 20% shorter', () => { + const newContent = generateLongContent('# Previous content remains here', 130) + const predictedLineCount = 150 + expect(detectCodeOmission(originalContent, newContent, predictedLineCount)).toBe(false) + }) + + it('should detect suspicious multi-line comment when content is more than 20% shorter', () => { + const newContent = generateLongContent('/* Previous content remains the same */\nconst x = 1;') + const predictedLineCount = 150 + expect(detectCodeOmission(originalContent, newContent, predictedLineCount)).toBe(true) + }) + + it('should not flag suspicious multi-line comment when content is less than 20% shorter', () => { + const newContent = generateLongContent('/* Previous content remains the same */', 130) + const predictedLineCount = 150 + expect(detectCodeOmission(originalContent, newContent, predictedLineCount)).toBe(false) + }) + + it('should detect suspicious JSX comment when content is more than 20% shorter', () => { + const newContent = generateLongContent('{/* Rest of the code remains the same */}\nconst x = 1;') + const predictedLineCount = 150 + expect(detectCodeOmission(originalContent, newContent, predictedLineCount)).toBe(true) + }) + + it('should not flag suspicious JSX comment when content is less than 20% shorter', () => { + const newContent = generateLongContent('{/* Rest of the code remains the same */}', 130) + const predictedLineCount = 150 + expect(detectCodeOmission(originalContent, newContent, predictedLineCount)).toBe(false) + }) + + it('should detect suspicious HTML comment when content is more than 20% shorter', () => { + const newContent = generateLongContent('\nconst x = 1;') + const predictedLineCount = 150 + expect(detectCodeOmission(originalContent, newContent, predictedLineCount)).toBe(true) + }) + + it('should not flag suspicious HTML comment when content is less than 20% shorter', () => { + const newContent = generateLongContent('', 130) + const predictedLineCount = 150 + expect(detectCodeOmission(originalContent, newContent, predictedLineCount)).toBe(false) + }) + + it('should detect suspicious square bracket notation when content is more than 20% shorter', () => { + const newContent = generateLongContent('[Previous content from line 1-305 remains exactly the same]\nconst x = 1;') + const predictedLineCount = 150 + expect(detectCodeOmission(originalContent, newContent, predictedLineCount)).toBe(true) + }) + + it('should not flag suspicious square bracket notation when content is less than 20% shorter', () => { + const newContent = generateLongContent('[Previous content from line 1-305 remains exactly the same]', 130) + const predictedLineCount = 150 + expect(detectCodeOmission(originalContent, newContent, predictedLineCount)).toBe(false) + }) + + it('should not flag content very close to predicted length', () => { + const newContent = generateLongContent(`const x = 1; +const y = 2; +// This is a legitimate comment that remains here`, 130) + const predictedLineCount = 150 + expect(detectCodeOmission(originalContent, newContent, predictedLineCount)).toBe(false) + }) + + it('should not flag when content is longer than predicted', () => { + const newContent = generateLongContent(`const x = 1; +const y = 2; +// Previous content remains here but we added more +const z = 3; +const w = 4;`, 160) + const predictedLineCount = 150 + expect(detectCodeOmission(originalContent, newContent, predictedLineCount)).toBe(false) + }) +}) diff --git a/src/integrations/editor/detect-omission.ts b/src/integrations/editor/detect-omission.ts index 5cb0f8e..d8c3e14 100644 --- a/src/integrations/editor/detect-omission.ts +++ b/src/integrations/editor/detect-omission.ts @@ -2,9 +2,22 @@ * Detects potential AI-generated code omissions in the given file content. * @param originalFileContent The original content of the file. * @param newFileContent The new content of the file to check. + * @param predictedLineCount The predicted number of lines in the new content. * @returns True if a potential omission is detected, false otherwise. */ -export function detectCodeOmission(originalFileContent: string, newFileContent: string): boolean { +export function detectCodeOmission( + originalFileContent: string, + newFileContent: string, + predictedLineCount: number +): boolean { + // Skip all checks if predictedLineCount is less than 100 + if (!predictedLineCount || predictedLineCount < 100) { + return false + } + + const actualLineCount = newFileContent.split("\n").length + const lengthRatio = actualLineCount / predictedLineCount + const originalLines = originalFileContent.split("\n") const newLines = newFileContent.split("\n") const omissionKeywords = ["remain", "remains", "unchanged", "rest", "previous", "existing", "content", "same", "..."] @@ -18,17 +31,21 @@ export function detectCodeOmission(originalFileContent: string, newFileContent: /^\s*\[/, // Square bracket notation ] + // Consider comments as suspicious if they weren't in the original file + // and contain omission keywords for (const line of newLines) { if (commentPatterns.some((pattern) => pattern.test(line))) { const words = line.toLowerCase().split(/\s+/) if (omissionKeywords.some((keyword) => words.includes(keyword))) { if (!originalLines.includes(line)) { - return true + // For files with 100+ lines, only flag if content is more than 20% shorter + if (lengthRatio <= 0.80) { + return true + } } } } } return false -} - +} \ No newline at end of file