diff --git a/src/prompt-cache.js b/src/prompt-cache.js index b5514aa..3eb72c3 100644 --- a/src/prompt-cache.js +++ b/src/prompt-cache.js @@ -195,6 +195,7 @@ Code suggestions enable reviewers to apply fixes directly as GitHub suggestions. */ const LINE_NUMBERS_RULE = `**CRITICAL 'lineNumbers' RULE - MANDATORY COMPLIANCE**: - **ALWAYS provide line numbers** - this field is REQUIRED for every issue +- **Line numbers are shown in the file content** as a prefix on each line (e.g., " 42 | const x = 1;"). Use THESE line numbers directly - do NOT try to count lines yourself. - If you can identify specific lines, provide them (max 3-5 for repeated issues) - If the issue affects the entire file or cannot be pinpointed, provide [1] or relevant section line numbers - For ANY issue that occurs multiple times in a file, list ONLY the first 3-5 occurrences maximum diff --git a/src/rag-analyzer.js b/src/rag-analyzer.js index b19143f..2185c7c 100644 --- a/src/rag-analyzer.js +++ b/src/rag-analyzer.js @@ -27,6 +27,7 @@ import { isGenericDocument, getGenericDocumentContext } from './utils/document-d import { isTestFile, shouldProcessFile } from './utils/file-validation.js'; import { detectFileType, detectLanguageFromExtension } from './utils/language-detection.js'; import { debug } from './utils/logging.js'; +import { addLineNumbers } from './utils/string-utils.js'; // Constants for content processing const MAX_QUERY_CONTEXT_LENGTH = 1500; @@ -1047,10 +1048,10 @@ You have access to TWO pieces of information: - Do NOT flag functions/variables as missing if they exist elsewhere in the full file - The unchanged code is part of the file - check it before making assumptions -**FULL FILE CONTENT (for context - DO NOT review unchanged code):** +**FULL FILE CONTENT (for context - DO NOT review unchanged code, line numbers shown for reference):** \`\`\`${file.language} -${file.fullFileContent || file.content} +${addLineNumbers(file.fullFileContent || file.content)} \`\`\` **GIT DIFF TO REVIEW (critique ONLY these changes):** @@ -1063,7 +1064,7 @@ Path: ${file.path} Language: ${file.language} \`\`\`${file.language} -${file.content} +${addLineNumbers(file.content)} \`\`\``; // Add project architecture context if available @@ -1166,10 +1167,10 @@ You have access to TWO pieces of information: - Check the full file for existing test cases before making assumptions - The unchanged test code is part of the file - review it before suggesting additions -**FULL TEST FILE CONTENT (for context - check for existing tests):** +**FULL TEST FILE CONTENT (for context - check for existing tests, line numbers shown for reference):** \`\`\`${file.language} -${file.fullFileContent || file.content} +${addLineNumbers(file.fullFileContent || file.content)} \`\`\` **GIT DIFF TO REVIEW (critique ONLY these changes):** @@ -1182,7 +1183,7 @@ Path: ${file.path} Language: ${file.language} \`\`\`${file.language} -${file.content} +${addLineNumbers(file.content)} \`\`\``; // Use shared helpers for custom docs and role definition @@ -1317,9 +1318,9 @@ ${g.content} ${prFile.diff} \`\`\` -### Full File Content (For Context): +### Full File Content (For Context - line numbers shown for reference): \`\`\`${prFile.language} -${prFile.fullContent} +${addLineNumbers(prFile.fullContent)} \`\`\` `; }) diff --git a/src/utils/string-utils.js b/src/utils/string-utils.js index b544be5..950acd2 100644 --- a/src/utils/string-utils.js +++ b/src/utils/string-utils.js @@ -26,3 +26,28 @@ export function slugify(text) { .replace(/[^\w-]+/g, '') // Remove all non-word chars .replace(/--+/g, '-'); // Replace multiple - with single - } + +/** + * Add line numbers to source code content so LLMs can accurately reference + * specific lines when performing code reviews. + * + * Each line is prefixed with its 1-based line number followed by a pipe separator. + * The line numbers are right-aligned with consistent padding based on the total + * number of lines, making the output easy to read. + * + * @param {string} content - The source code content to annotate + * @returns {string} The content with line numbers prepended to each line + * + * @example + * addLineNumbers('const a = 1;\nconst b = 2;'); + * // '1 | const a = 1;\n2 | const b = 2;' + * + * @example + * addLineNumbers(''); // '' + */ +export function addLineNumbers(content) { + if (!content) return ''; + const lines = content.split('\n'); + const padding = String(lines.length).length; + return lines.map((line, i) => `${String(i + 1).padStart(padding)} | ${line}`).join('\n'); +} diff --git a/src/utils/string-utils.test.js b/src/utils/string-utils.test.js index b96d2a0..d020834 100644 --- a/src/utils/string-utils.test.js +++ b/src/utils/string-utils.test.js @@ -1,4 +1,4 @@ -import { slugify } from './string-utils.js'; +import { slugify, addLineNumbers } from './string-utils.js'; describe('slugify', () => { describe('basic transformations', () => { @@ -84,3 +84,72 @@ describe('slugify', () => { }); }); }); + +describe('addLineNumbers', () => { + describe('basic functionality', () => { + it('should add line numbers to each line', () => { + const input = 'const a = 1;\nconst b = 2;'; + const result = addLineNumbers(input); + expect(result).toBe('1 | const a = 1;\n2 | const b = 2;'); + }); + + it('should pad line numbers for files with 10+ lines', () => { + const lines = Array.from({ length: 12 }, (_, i) => `line ${i + 1}`); + const input = lines.join('\n'); + const result = addLineNumbers(input); + const outputLines = result.split('\n'); + // Single-digit lines should be padded with a leading space + expect(outputLines[0]).toBe(' 1 | line 1'); + expect(outputLines[8]).toBe(' 9 | line 9'); + // Double-digit lines should not be padded + expect(outputLines[9]).toBe('10 | line 10'); + expect(outputLines[11]).toBe('12 | line 12'); + }); + + it('should pad line numbers for files with 100+ lines', () => { + const lines = Array.from({ length: 105 }, (_, i) => `line ${i + 1}`); + const input = lines.join('\n'); + const result = addLineNumbers(input); + const outputLines = result.split('\n'); + expect(outputLines[0]).toBe(' 1 | line 1'); + expect(outputLines[9]).toBe(' 10 | line 10'); + expect(outputLines[99]).toBe('100 | line 100'); + }); + + it('should handle a single line', () => { + expect(addLineNumbers('hello')).toBe('1 | hello'); + }); + + it('should preserve empty lines', () => { + const input = 'line1\n\nline3'; + const result = addLineNumbers(input); + expect(result).toBe('1 | line1\n2 | \n3 | line3'); + }); + + it('should preserve indentation', () => { + const input = 'function foo() {\n return 1;\n}'; + const result = addLineNumbers(input); + expect(result).toBe('1 | function foo() {\n2 | return 1;\n3 | }'); + }); + }); + + describe('edge cases', () => { + it('should return empty string for null input', () => { + expect(addLineNumbers(null)).toBe(''); + }); + + it('should return empty string for undefined input', () => { + expect(addLineNumbers(undefined)).toBe(''); + }); + + it('should return empty string for empty string input', () => { + expect(addLineNumbers('')).toBe(''); + }); + + it('should handle content with trailing newline', () => { + const input = 'line1\nline2\n'; + const result = addLineNumbers(input); + expect(result).toBe('1 | line1\n2 | line2\n3 | '); + }); + }); +});