mirror of
https://github.com/RooVetGit/Roo-Code.git
synced 2026-08-28 05:27:24 +00:00
fix: improve omission detection to reduce false positives
- Implement more sophisticated pattern matching that distinguishes between actual code omissions and legitimate documentation - Add strong omission patterns for clear indicators like "... rest of code unchanged" - Require multiple weak keywords to trigger detection, reducing false positives - Add comprehensive tests for various file types and documentation patterns - Fixes issue #9785 where markdown files with code examples were incorrectly rejected
This commit is contained in:
Roo Code
parent
23605bed93
commit
a724d513d1
2 changed files with 222 additions and 64 deletions
|
|
@ -15,66 +15,183 @@ describe("detectCodeOmission", () => {
|
|||
const y = 2;`
|
||||
}
|
||||
|
||||
it("should skip comment checks for files under 100 lines", () => {
|
||||
const newContent = `// Lines 1-50 remain unchanged
|
||||
describe("Basic functionality", () => {
|
||||
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(false)
|
||||
})
|
||||
expect(detectCodeOmission(originalContent, newContent)).toBe(false)
|
||||
})
|
||||
|
||||
it("should not detect regular comments without omission keywords", () => {
|
||||
const newContent = generateLongContent("// Adding new functionality")
|
||||
expect(detectCodeOmission(originalContent, newContent)).toBe(false)
|
||||
})
|
||||
it("should not detect regular comments without omission keywords", () => {
|
||||
const newContent = generateLongContent("// Adding new functionality")
|
||||
expect(detectCodeOmission(originalContent, newContent)).toBe(false)
|
||||
})
|
||||
|
||||
it("should not detect when comment is part of original content", () => {
|
||||
const originalWithComment = `// Content remains unchanged
|
||||
it("should not detect when comment is part of original content", () => {
|
||||
const originalWithComment = `// Content remains unchanged
|
||||
${originalContent}`
|
||||
const newContent = generateLongContent("// Content remains unchanged")
|
||||
expect(detectCodeOmission(originalWithComment, newContent)).toBe(false)
|
||||
})
|
||||
const newContent = generateLongContent("// Content remains unchanged")
|
||||
expect(detectCodeOmission(originalWithComment, newContent)).toBe(false)
|
||||
})
|
||||
|
||||
it("should not detect code that happens to contain omission keywords", () => {
|
||||
const newContent = generateLongContent(`const remains = 'some value';
|
||||
it("should not detect code that happens to contain omission keywords", () => {
|
||||
const newContent = generateLongContent(`const remains = 'some value';
|
||||
const unchanged = true;`)
|
||||
expect(detectCodeOmission(originalContent, newContent)).toBe(false)
|
||||
expect(detectCodeOmission(originalContent, newContent)).toBe(false)
|
||||
})
|
||||
})
|
||||
|
||||
it("should detect suspicious single-line comment for files with 100+ lines", () => {
|
||||
const newContent = generateLongContent("// Previous content remains here\nconst x = 1;")
|
||||
expect(detectCodeOmission(originalContent, newContent)).toBe(true)
|
||||
describe("Strong omission patterns (should detect)", () => {
|
||||
it("should detect ellipsis patterns", () => {
|
||||
const newContent = generateLongContent("// ... rest of code unchanged")
|
||||
expect(detectCodeOmission(originalContent, newContent)).toBe(true)
|
||||
})
|
||||
|
||||
it("should detect 'rest of code remains' pattern", () => {
|
||||
const newContent = generateLongContent("// Rest of the code remains unchanged")
|
||||
expect(detectCodeOmission(originalContent, newContent)).toBe(true)
|
||||
})
|
||||
|
||||
it("should detect 'previous code remains' pattern", () => {
|
||||
const newContent = generateLongContent("// Previous code remains here")
|
||||
expect(detectCodeOmission(originalContent, newContent)).toBe(true)
|
||||
})
|
||||
|
||||
it("should detect 'code truncated' pattern", () => {
|
||||
const newContent = generateLongContent("// Code truncated for brevity")
|
||||
expect(detectCodeOmission(originalContent, newContent)).toBe(true)
|
||||
})
|
||||
|
||||
it("should detect line range patterns", () => {
|
||||
const newContent = generateLongContent("// [lines 50-100 remain unchanged]")
|
||||
expect(detectCodeOmission(originalContent, newContent)).toBe(true)
|
||||
})
|
||||
|
||||
it("should detect Python-style omission comments", () => {
|
||||
const newContent = generateLongContent("# Previous content remains here")
|
||||
expect(detectCodeOmission(originalContent, newContent)).toBe(true)
|
||||
})
|
||||
|
||||
it("should detect multi-line comment omissions", () => {
|
||||
const newContent = generateLongContent("/* Previous content remains the same */")
|
||||
expect(detectCodeOmission(originalContent, newContent)).toBe(true)
|
||||
})
|
||||
|
||||
it("should detect JSX comment omissions", () => {
|
||||
const newContent = generateLongContent("{/* Rest of the code remains the same */}")
|
||||
expect(detectCodeOmission(originalContent, newContent)).toBe(true)
|
||||
})
|
||||
|
||||
it("should detect HTML comment omissions", () => {
|
||||
const newContent = generateLongContent("<!-- Existing content unchanged -->")
|
||||
expect(detectCodeOmission(originalContent, newContent)).toBe(true)
|
||||
})
|
||||
|
||||
it("should detect square bracket notation", () => {
|
||||
const newContent = generateLongContent("[Previous content from line 1-305 remains exactly the same]")
|
||||
expect(detectCodeOmission(originalContent, newContent)).toBe(true)
|
||||
})
|
||||
})
|
||||
|
||||
it("should detect suspicious Python-style comment for files with 100+ lines", () => {
|
||||
const newContent = generateLongContent("# Previous content remains here\nconst x = 1;")
|
||||
expect(detectCodeOmission(originalContent, newContent)).toBe(true)
|
||||
describe("Legitimate documentation patterns (should NOT detect)", () => {
|
||||
it("should not flag single keyword documentation comments", () => {
|
||||
const newContent = generateLongContent("// Add to existing configuration")
|
||||
expect(detectCodeOmission(originalContent, newContent)).toBe(false)
|
||||
})
|
||||
|
||||
it("should not flag 'NEW' annotations in markdown examples", () => {
|
||||
const newContent = generateLongContent("# NEW: Service configuration")
|
||||
expect(detectCodeOmission(originalContent, newContent)).toBe(false)
|
||||
})
|
||||
|
||||
it("should not flag makefile comments", () => {
|
||||
const newContent = generateLongContent("# Add to orchestrator startup sequence")
|
||||
expect(detectCodeOmission(originalContent, newContent)).toBe(false)
|
||||
})
|
||||
|
||||
it("should not flag step-by-step instructions", () => {
|
||||
const newContent = generateLongContent("// Step 1: Initialize the service")
|
||||
expect(detectCodeOmission(originalContent, newContent)).toBe(false)
|
||||
})
|
||||
|
||||
it("should not flag configuration comments", () => {
|
||||
const newContent = generateLongContent("// Configuration for production environment")
|
||||
expect(detectCodeOmission(originalContent, newContent)).toBe(false)
|
||||
})
|
||||
|
||||
it("should not flag TODO comments", () => {
|
||||
const newContent = generateLongContent("// TODO: Add error handling here")
|
||||
expect(detectCodeOmission(originalContent, newContent)).toBe(false)
|
||||
})
|
||||
|
||||
it("should not flag FIXME comments", () => {
|
||||
const newContent = generateLongContent("// FIXME: This needs to be refactored")
|
||||
expect(detectCodeOmission(originalContent, newContent)).toBe(false)
|
||||
})
|
||||
|
||||
it("should not flag NOTE comments", () => {
|
||||
const newContent = generateLongContent("// NOTE: This is important for performance")
|
||||
expect(detectCodeOmission(originalContent, newContent)).toBe(false)
|
||||
})
|
||||
|
||||
it("should not flag WARNING comments", () => {
|
||||
const newContent = generateLongContent("// WARNING: Do not modify this section")
|
||||
expect(detectCodeOmission(originalContent, newContent)).toBe(false)
|
||||
})
|
||||
|
||||
it("should not flag single 'unchanged' word in different context", () => {
|
||||
const newContent = generateLongContent("// Keep this value unchanged during migration")
|
||||
expect(detectCodeOmission(originalContent, newContent)).toBe(false)
|
||||
})
|
||||
|
||||
it("should not flag single 'remains' word in different context", () => {
|
||||
const newContent = generateLongContent("// The responsibility remains with the caller")
|
||||
expect(detectCodeOmission(originalContent, newContent)).toBe(false)
|
||||
})
|
||||
|
||||
it("should not flag legitimate YAML comments", () => {
|
||||
const newContent = generateLongContent("# Service configuration for Docker")
|
||||
expect(detectCodeOmission(originalContent, newContent)).toBe(false)
|
||||
})
|
||||
|
||||
it("should not flag legitimate JSON5 comments", () => {
|
||||
const newContent = generateLongContent("// API endpoint configuration")
|
||||
expect(detectCodeOmission(originalContent, newContent)).toBe(false)
|
||||
})
|
||||
|
||||
it("should not flag legitimate shell script comments", () => {
|
||||
const newContent = generateLongContent("# Install dependencies first")
|
||||
expect(detectCodeOmission(originalContent, newContent)).toBe(false)
|
||||
})
|
||||
|
||||
it("should not flag legitimate SQL comments", () => {
|
||||
const newContent = generateLongContent("-- Create indexes for better performance")
|
||||
expect(detectCodeOmission(originalContent, newContent)).toBe(false)
|
||||
})
|
||||
})
|
||||
|
||||
it("should detect suspicious multi-line comment for files with 100+ lines", () => {
|
||||
const newContent = generateLongContent("/* Previous content remains the same */\nconst x = 1;")
|
||||
expect(detectCodeOmission(originalContent, newContent)).toBe(true)
|
||||
})
|
||||
|
||||
it("should detect suspicious JSX comment for files with 100+ lines", () => {
|
||||
const newContent = generateLongContent("{/* Rest of the code remains the same */}\nconst x = 1;")
|
||||
expect(detectCodeOmission(originalContent, newContent)).toBe(true)
|
||||
})
|
||||
|
||||
it("should detect suspicious HTML comment for files with 100+ lines", () => {
|
||||
const newContent = generateLongContent("<!-- Existing content unchanged -->\nconst x = 1;")
|
||||
expect(detectCodeOmission(originalContent, newContent)).toBe(true)
|
||||
})
|
||||
|
||||
it("should detect suspicious square bracket notation for files with 100+ lines", () => {
|
||||
const newContent = generateLongContent(
|
||||
"[Previous content from line 1-305 remains exactly the same]\nconst x = 1;",
|
||||
)
|
||||
expect(detectCodeOmission(originalContent, newContent)).toBe(true)
|
||||
})
|
||||
|
||||
it("should not flag legitimate comments in files with 100+ lines when in original", () => {
|
||||
const originalWithComment = `// This is a legitimate comment that remains here
|
||||
describe("Edge cases", () => {
|
||||
it("should not flag legitimate comments when in original", () => {
|
||||
const originalWithComment = `// This is a legitimate comment that remains here
|
||||
${originalContent}`
|
||||
const newContent = generateLongContent("// This is a legitimate comment that remains here")
|
||||
expect(detectCodeOmission(originalWithComment, newContent)).toBe(false)
|
||||
const newContent = generateLongContent("// This is a legitimate comment that remains here")
|
||||
expect(detectCodeOmission(originalWithComment, newContent)).toBe(false)
|
||||
})
|
||||
|
||||
it("should handle mixed legitimate and suspicious patterns correctly", () => {
|
||||
const legitimateComment = "// Add this to your configuration"
|
||||
const suspiciousComment = "// ... rest of code remains unchanged"
|
||||
const newContent = generateLongContent(`${legitimateComment}\n${suspiciousComment}`)
|
||||
expect(detectCodeOmission(originalContent, newContent)).toBe(true)
|
||||
})
|
||||
|
||||
it("should handle empty files", () => {
|
||||
expect(detectCodeOmission("", "")).toBe(false)
|
||||
})
|
||||
|
||||
it("should handle files with only comments", () => {
|
||||
const newContent = generateLongContent("// This is just a comment file")
|
||||
expect(detectCodeOmission("", newContent)).toBe(false)
|
||||
})
|
||||
})
|
||||
})
|
||||
|
|
|
|||
|
|
@ -1,6 +1,7 @@
|
|||
/**
|
||||
* Detects potential AI-generated code omissions in the given file content.
|
||||
* Looks for comments containing omission keywords that weren't in the original file.
|
||||
* Uses pattern matching to identify comments that indicate truncated or omitted code,
|
||||
* while avoiding false positives from legitimate documentation comments.
|
||||
* @param originalFileContent The original content of the file.
|
||||
* @param newFileContent The new content of the file to check.
|
||||
* @returns True if a potential omission is detected, false otherwise.
|
||||
|
|
@ -15,18 +16,26 @@ export function detectCodeOmission(originalFileContent: string, newFileContent:
|
|||
|
||||
const originalLines = originalFileContent.split("\n")
|
||||
const newLines = newFileContent.split("\n")
|
||||
const omissionKeywords = [
|
||||
"remain",
|
||||
"remains",
|
||||
"unchanged",
|
||||
"rest",
|
||||
"previous",
|
||||
"existing",
|
||||
"content",
|
||||
"same",
|
||||
"...",
|
||||
|
||||
// More specific patterns that strongly indicate actual code omissions
|
||||
// These patterns are much more likely to be AI-generated placeholders
|
||||
const strongOmissionPatterns = [
|
||||
/\.\.\.\s*(rest|remainder|remaining|more)\s+(of|code|content|unchanged)/i,
|
||||
/\.\.\.\s*$/, // Just ellipsis at end of line
|
||||
/^\s*\/\/\s*\.\.\./, // Comment starting with ellipsis
|
||||
/^\s*#\s*\.\.\./, // Python comment starting with ellipsis
|
||||
/(rest|remainder|remaining)\s+of\s+(the\s+)?(code|file|content)\s+(remains?|unchanged|here|omitted)/i,
|
||||
/(previous|existing|original)\s+code\s+(remains?|unchanged|here|omitted)/i,
|
||||
/code\s+(truncated|omitted|removed|skipped|abbreviated)/i,
|
||||
/\[.*\s+(truncated|omitted|removed|skipped)\s*.*\]/i,
|
||||
/\/\/\s*\[.*lines?\s+\d+.*\]/i, // e.g., // [lines 50-100 remain unchanged]
|
||||
/#\s*\[.*lines?\s+\d+.*\]/i, // Python version
|
||||
/<!--\s*\[.*lines?\s+\d+.*\].*-->/i, // HTML version
|
||||
]
|
||||
|
||||
// Weaker patterns that need additional context to be considered omissions
|
||||
const weakOmissionKeywords = ["remain", "remains", "unchanged", "rest", "previous", "existing", "content", "same"]
|
||||
|
||||
const commentPatterns = [
|
||||
/^\s*\/\//, // Single-line comment for most languages
|
||||
/^\s*#/, // Single-line comment for Python, Ruby, etc.
|
||||
|
|
@ -36,13 +45,45 @@ 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
|
||||
// Check for strong omission patterns first
|
||||
for (const line of newLines) {
|
||||
// Skip if this line was in the original file
|
||||
if (originalLines.includes(line)) {
|
||||
continue
|
||||
}
|
||||
|
||||
// Check for strong patterns that clearly indicate omissions
|
||||
if (strongOmissionPatterns.some((pattern) => pattern.test(line))) {
|
||||
return true
|
||||
}
|
||||
|
||||
// For weak patterns, require multiple indicators
|
||||
if (commentPatterns.some((pattern) => pattern.test(line))) {
|
||||
const words = line.toLowerCase().split(/\s+/)
|
||||
if (omissionKeywords.some((keyword) => words.includes(keyword))) {
|
||||
if (!originalLines.includes(line)) {
|
||||
const lowerLine = line.toLowerCase()
|
||||
const words = lowerLine.split(/\s+/)
|
||||
|
||||
// Count how many weak keywords are present
|
||||
let keywordCount = 0
|
||||
for (const keyword of weakOmissionKeywords) {
|
||||
if (words.includes(keyword)) {
|
||||
keywordCount++
|
||||
}
|
||||
}
|
||||
|
||||
// Only flag if we have multiple weak keywords together
|
||||
// This reduces false positives from single words like "// Add to config"
|
||||
if (keywordCount >= 2) {
|
||||
// Additional check: look for phrases that indicate actual omission
|
||||
// rather than documentation
|
||||
const omissionPhrases = [
|
||||
/remains?\s+(unchanged|the\s+same|here)/i,
|
||||
/rest\s+of\s+(the\s+)?(code|file|content)/i,
|
||||
/previous\s+(code|content|implementation)/i,
|
||||
/existing\s+(code|content|implementation)/i,
|
||||
/content\s+(remains?|unchanged)/i,
|
||||
]
|
||||
|
||||
if (omissionPhrases.some((phrase) => phrase.test(lowerLine))) {
|
||||
return true
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue