fix: map review line numbers from unified diff

Author: 777genius

__tests__/unit/analysis/finding-filter.test.ts

@@ -830,19 +830,23 @@ describe('FindingFilter', () => {
 
     test('keeps SQL interpolation regressions even when phrased cautiously', () => {
       const diff = `diff --git a/src/users.js b/src/users.js
-@@ -7,7 +7,7 @@ export function normalizeEmail(email) {
+index 51097d9..d0723db 100644
+--- a/src/users.js
++++ b/src/users.js
+@@ -3,8 +3,7 @@ function normalizeEmail(email) {
  }
 
  export async function findUserByEmail(db, email) {
--  const rows = await db.query('SELECT * FROM users WHERE email = ? LIMIT 1', [email]);
+-  const normalized = normalizeEmail(email);
+-  const rows = await db.query('SELECT * FROM users WHERE email = ? LIMIT 1', [normalized]);
 +  const rows = await db.query(\`SELECT * FROM users WHERE email = '\${email}' LIMIT 1\`);
    return rows[0] || null;
  }`;
 
       const findings: Finding[] = [
         {
           file: 'src/users.js',
-          line: 10,
+          line: 6,
           severity: 'major',
           title: 'Email is interpolated into SQL',
           message:

dist/index.js

@@ -23129,18 +23129,73 @@ var FindingFilter = class {
     if (!finding.line) {
       return false;
     }
-    const lines = diffContent.split("\n");
-    const lineIndex = finding.line - 1;
-    if (lineIndex < 0 || lineIndex >= lines.length) {
+    if (this.isTrueSecurityIssue(finding)) {
+      return false;
+    }
+    const patchLike = diffContent.includes("@@") || diffContent.includes("diff --git ");
+    const mappedLine = this.findNewFileLineInDiff(finding.file, finding.line, diffContent);
+    if (patchLike && mappedLine === null) {
       return false;
     }
-    const line = lines[lineIndex].trim();
+    if (!patchLike && mappedLine === null) {
+      const lines = diffContent.split("\n");
+      const lineIndex = finding.line - 1;
+      if (lineIndex < 0 || lineIndex >= lines.length) {
+        return false;
+      }
+      return this.isBlankBraceOrCommentLine(lines[lineIndex].trim(), finding.line);
+    }
+    if (mappedLine === null) {
+      return false;
+    }
+    return this.isBlankBraceOrCommentLine(mappedLine.trim(), finding.line);
+  }
+  isBlankBraceOrCommentLine(line, lineNumber) {
     if (line === "" || line === "}" || line === "};" || line === "])" || line === "]);" || line.startsWith("//") || line.startsWith("/*") || line.startsWith("*")) {
-      logger.debug(`Line ${finding.line} is blank/brace/comment, likely incorrect line number`);
+      logger.debug(`Line ${lineNumber} is blank/brace/comment, likely incorrect line number`);
       return true;
     }
     return false;
   }
+  findNewFileLineInDiff(file, lineNumber, diffContent) {
+    let currentFile = null;
+    let newLine = null;
+    for (const rawLine of diffContent.split("\n")) {
+      const fileMatch = rawLine.match(/^diff --git a\/.+ b\/(.+)$/);
+      if (fileMatch) {
+        currentFile = fileMatch[1];
+        newLine = null;
+        continue;
+      }
+      const newFileMatch = rawLine.match(/^\+\+\+ b\/(.+)$/);
+      if (newFileMatch) {
+        currentFile = newFileMatch[1];
+        continue;
+      }
+      const hunkMatch = rawLine.match(/^@@ -\d+(?:,\d+)? \+(\d+)(?:,\d+)? @@/);
+      if (hunkMatch) {
+        newLine = Number(hunkMatch[1]);
+        continue;
+      }
+      if (newLine === null || currentFile !== file) {
+        continue;
+      }
+      if (rawLine.startsWith("---") || rawLine.startsWith("+++")) {
+        continue;
+      }
+      if (rawLine.startsWith("-")) {
+        continue;
+      }
+      if (rawLine.startsWith("+") || rawLine.startsWith(" ")) {
+        const lineText = rawLine.slice(1);
+        if (newLine === lineNumber) {
+          return lineText;
+        }
+        newLine++;
+      }
+    }
+    return null;
+  }
   /**
    * Check for invalid or suspicious line numbers that will cause GitHub API errors
    */

dist/index.js.map

@@ -674,16 +674,39 @@ export class FindingFilter {
       return false; // No line number to check
     }
 
-    // Extract the line from the diff
-    const lines = diffContent.split('\n');
-    const lineIndex = finding.line - 1;
+    // Do not suppress concrete security findings just because the model's line
+    // maps near hunk context. GitHub validation later decides whether it is postable.
+    if (this.isTrueSecurityIssue(finding)) {
+      return false;
+    }
 
-    if (lineIndex < 0 || lineIndex >= lines.length) {
-      return false; // Line number out of bounds
+    const patchLike = diffContent.includes('@@') || diffContent.includes('diff --git ');
+    const mappedLine = this.findNewFileLineInDiff(finding.file, finding.line, diffContent);
+
+    if (patchLike && mappedLine === null) {
+      return false; // Cannot prove the line is wrong from this patch.
     }
 
-    const line = lines[lineIndex].trim();
+    if (!patchLike && mappedLine === null) {
+      // Legacy fallback for tests/plain snippets that pass raw code instead of a patch.
+      const lines = diffContent.split('\n');
+      const lineIndex = finding.line - 1;
+
+      if (lineIndex < 0 || lineIndex >= lines.length) {
+        return false; // Line number out of bounds
+      }
+
+      return this.isBlankBraceOrCommentLine(lines[lineIndex].trim(), finding.line);
+    }
 
+    if (mappedLine === null) {
+      return false;
+    }
+
+    return this.isBlankBraceOrCommentLine(mappedLine.trim(), finding.line);
+  }
+
+  private isBlankBraceOrCommentLine(line: string, lineNumber: number): boolean {
     // Check if the line is just a closing brace, blank, or comment
     if (
       line === '' ||
@@ -695,13 +718,61 @@ export class FindingFilter {
       line.startsWith('/*') ||
       line.startsWith('*')
     ) {
-      logger.debug(`Line ${finding.line} is blank/brace/comment, likely incorrect line number`);
+      logger.debug(`Line ${lineNumber} is blank/brace/comment, likely incorrect line number`);
       return true;
     }
 
     return false;
   }
 
+  private findNewFileLineInDiff(file: string, lineNumber: number, diffContent: string): string | null {
+    let currentFile: string | null = null;
+    let newLine: number | null = null;
+
+    for (const rawLine of diffContent.split('\n')) {
+      const fileMatch = rawLine.match(/^diff --git a\/.+ b\/(.+)$/);
+      if (fileMatch) {
+        currentFile = fileMatch[1];
+        newLine = null;
+        continue;
+      }
+
+      const newFileMatch = rawLine.match(/^\+\+\+ b\/(.+)$/);
+      if (newFileMatch) {
+        currentFile = newFileMatch[1];
+        continue;
+      }
+
+      const hunkMatch = rawLine.match(/^@@ -\d+(?:,\d+)? \+(\d+)(?:,\d+)? @@/);
+      if (hunkMatch) {
+        newLine = Number(hunkMatch[1]);
+        continue;
+      }
+
+      if (newLine === null || currentFile !== file) {
+        continue;
+      }
+
+      if (rawLine.startsWith('---') || rawLine.startsWith('+++')) {
+        continue;
+      }
+
+      if (rawLine.startsWith('-')) {
+        continue;
+      }
+
+      if (rawLine.startsWith('+') || rawLine.startsWith(' ')) {
+        const lineText = rawLine.slice(1);
+        if (newLine === lineNumber) {
+          return lineText;
+        }
+        newLine++;
+      }
+    }
+
+    return null;
+  }
+
   /**
    * Check for invalid or suspicious line numbers that will cause GitHub API errors
    */