diff --git a/src/core/tools/__tests__/listCodeDefinitionNamesTool.spec.ts b/src/core/tools/__tests__/listCodeDefinitionNamesTool.spec.ts new file mode 100644 index 000000000000..7a26c2f8eeb7 --- /dev/null +++ b/src/core/tools/__tests__/listCodeDefinitionNamesTool.spec.ts @@ -0,0 +1,351 @@ +// npx vitest src/core/tools/__tests__/listCodeDefinitionNamesTool.spec.ts + +import { describe, it, expect, vi, beforeEach } from "vitest" +import { listCodeDefinitionNamesTool } from "../listCodeDefinitionNamesTool" +import { Task } from "../../task/Task" +import { ToolUse } from "../../../shared/tools" +import * as treeSitter from "../../../services/tree-sitter" +import fs from "fs/promises" + +// Mock the tree-sitter service +vi.mock("../../../services/tree-sitter", () => ({ + parseSourceCodeDefinitionsForFile: vi.fn(), + parseSourceCodeForDefinitionsTopLevel: vi.fn(), +})) + +// Mock fs module +vi.mock("fs/promises", () => ({ + default: { + stat: vi.fn(), + }, +})) + +describe("listCodeDefinitionNamesTool", () => { + let mockTask: Partial + let mockAskApproval: any + let mockHandleError: any + let mockPushToolResult: any + let mockRemoveClosingTag: any + + beforeEach(() => { + vi.clearAllMocks() + + mockTask = { + cwd: "/test/path", + consecutiveMistakeCount: 0, + recordToolError: vi.fn(), + sayAndCreateMissingParamError: vi.fn(), + ask: vi.fn(), + fileContextTracker: { + trackFileContext: vi.fn(), + }, + providerRef: { + deref: vi.fn(() => ({ + getState: vi.fn(async () => ({ maxReadFileLine: -1 })), + })), + }, + rooIgnoreController: undefined, + } as any + + mockAskApproval = vi.fn(async () => true) + mockHandleError = vi.fn() + mockPushToolResult = vi.fn() + mockRemoveClosingTag = vi.fn((tag: string, value: string) => value) + }) + + describe("truncateDefinitionsToLineLimit", () => { + it("should not truncate when maxReadFileLine is -1 (no limit)", async () => { + const mockDefinitions = `# test.ts +10--20 | function foo() { +30--40 | function bar() { +50--60 | function baz() {` + + vi.mocked(treeSitter.parseSourceCodeDefinitionsForFile).mockResolvedValue(mockDefinitions) + + vi.mocked(fs.stat).mockResolvedValue({ + isFile: () => true, + isDirectory: () => false, + } as any) + + mockTask.providerRef = { + deref: vi.fn(() => ({ + getState: vi.fn(async () => ({ maxReadFileLine: -1 })), + })), + } as any + + const block: ToolUse = { + type: "tool_use", + name: "list_code_definition_names", + params: { path: "test.ts" }, + partial: false, + } + + await listCodeDefinitionNamesTool( + mockTask as Task, + block, + mockAskApproval, + mockHandleError, + mockPushToolResult, + mockRemoveClosingTag, + ) + + expect(mockPushToolResult).toHaveBeenCalledWith(mockDefinitions) + }) + + it("should not truncate when maxReadFileLine is 0 (definitions only mode)", async () => { + const mockDefinitions = `# test.ts +10--20 | function foo() { +30--40 | function bar() { +50--60 | function baz() {` + + vi.mocked(treeSitter.parseSourceCodeDefinitionsForFile).mockResolvedValue(mockDefinitions) + + vi.mocked(fs.stat).mockResolvedValue({ + isFile: () => true, + isDirectory: () => false, + } as any) + + mockTask.providerRef = { + deref: vi.fn(() => ({ + getState: vi.fn(async () => ({ maxReadFileLine: 0 })), + })), + } as any + + const block: ToolUse = { + type: "tool_use", + name: "list_code_definition_names", + params: { path: "test.ts" }, + partial: false, + } + + await listCodeDefinitionNamesTool( + mockTask as Task, + block, + mockAskApproval, + mockHandleError, + mockPushToolResult, + mockRemoveClosingTag, + ) + + expect(mockPushToolResult).toHaveBeenCalledWith(mockDefinitions) + }) + + it("should truncate definitions when maxReadFileLine is set", async () => { + const mockDefinitions = `# test.ts +10--20 | function foo() { +30--40 | function bar() { +50--60 | function baz() {` + + vi.mocked(treeSitter.parseSourceCodeDefinitionsForFile).mockResolvedValue(mockDefinitions) + + vi.mocked(fs.stat).mockResolvedValue({ + isFile: () => true, + isDirectory: () => false, + } as any) + + mockTask.providerRef = { + deref: vi.fn(() => ({ + getState: vi.fn(async () => ({ maxReadFileLine: 25 })), + })), + } as any + + const block: ToolUse = { + type: "tool_use", + name: "list_code_definition_names", + params: { path: "test.ts" }, + partial: false, + } + + await listCodeDefinitionNamesTool( + mockTask as Task, + block, + mockAskApproval, + mockHandleError, + mockPushToolResult, + mockRemoveClosingTag, + ) + + // Should only include definitions starting at or before line 25 + const expectedResult = `# test.ts +10--20 | function foo() {` + + expect(mockPushToolResult).toHaveBeenCalledWith(expectedResult) + }) + + it("should include definitions that start within limit even if they end beyond it", async () => { + const mockDefinitions = `# test.ts +10--50 | function foo() { +60--80 | function bar() {` + + vi.mocked(treeSitter.parseSourceCodeDefinitionsForFile).mockResolvedValue(mockDefinitions) + + vi.mocked(fs.stat).mockResolvedValue({ + isFile: () => true, + isDirectory: () => false, + } as any) + + mockTask.providerRef = { + deref: vi.fn(() => ({ + getState: vi.fn(async () => ({ maxReadFileLine: 30 })), + })), + } as any + + const block: ToolUse = { + type: "tool_use", + name: "list_code_definition_names", + params: { path: "test.ts" }, + partial: false, + } + + await listCodeDefinitionNamesTool( + mockTask as Task, + block, + mockAskApproval, + mockHandleError, + mockPushToolResult, + mockRemoveClosingTag, + ) + + // Should include foo (starts at 10) but not bar (starts at 60) + const expectedResult = `# test.ts +10--50 | function foo() {` + + expect(mockPushToolResult).toHaveBeenCalledWith(expectedResult) + }) + + it("should handle single-line definitions", async () => { + const mockDefinitions = `# test.ts +10 | const foo = 1 +20 | const bar = 2 +30 | const baz = 3` + + vi.mocked(treeSitter.parseSourceCodeDefinitionsForFile).mockResolvedValue(mockDefinitions) + + vi.mocked(fs.stat).mockResolvedValue({ + isFile: () => true, + isDirectory: () => false, + } as any) + + mockTask.providerRef = { + deref: vi.fn(() => ({ + getState: vi.fn(async () => ({ maxReadFileLine: 25 })), + })), + } as any + + const block: ToolUse = { + type: "tool_use", + name: "list_code_definition_names", + params: { path: "test.ts" }, + partial: false, + } + + await listCodeDefinitionNamesTool( + mockTask as Task, + block, + mockAskApproval, + mockHandleError, + mockPushToolResult, + mockRemoveClosingTag, + ) + + // Should include foo and bar but not baz + const expectedResult = `# test.ts +10 | const foo = 1 +20 | const bar = 2` + + expect(mockPushToolResult).toHaveBeenCalledWith(expectedResult) + }) + + it("should preserve header line when truncating", async () => { + const mockDefinitions = `# test.ts +100--200 | function foo() {` + + vi.mocked(treeSitter.parseSourceCodeDefinitionsForFile).mockResolvedValue(mockDefinitions) + + vi.mocked(fs.stat).mockResolvedValue({ + isFile: () => true, + isDirectory: () => false, + } as any) + + mockTask.providerRef = { + deref: vi.fn(() => ({ + getState: vi.fn(async () => ({ maxReadFileLine: 50 })), + })), + } as any + + const block: ToolUse = { + type: "tool_use", + name: "list_code_definition_names", + params: { path: "test.ts" }, + partial: false, + } + + await listCodeDefinitionNamesTool( + mockTask as Task, + block, + mockAskApproval, + mockHandleError, + mockPushToolResult, + mockRemoveClosingTag, + ) + + // Should keep header but exclude all definitions beyond line 50 + const expectedResult = `# test.ts` + + expect(mockPushToolResult).toHaveBeenCalledWith(expectedResult) + }) + }) + + it("should handle missing path parameter", async () => { + const block: ToolUse = { + type: "tool_use", + name: "list_code_definition_names", + params: {}, + partial: false, + } + + mockTask.sayAndCreateMissingParamError = vi.fn(async () => "Missing parameter: path") + + await listCodeDefinitionNamesTool( + mockTask as Task, + block, + mockAskApproval, + mockHandleError, + mockPushToolResult, + mockRemoveClosingTag, + ) + + expect(mockTask.consecutiveMistakeCount).toBe(1) + expect(mockTask.recordToolError).toHaveBeenCalledWith("list_code_definition_names") + expect(mockPushToolResult).toHaveBeenCalledWith("Missing parameter: path") + }) + + it("should handle directory path", async () => { + const mockDefinitions = "# Directory definitions" + + vi.mocked(treeSitter.parseSourceCodeForDefinitionsTopLevel).mockResolvedValue(mockDefinitions) + + vi.mocked(fs.stat).mockResolvedValue({ + isFile: () => false, + isDirectory: () => true, + } as any) + + const block: ToolUse = { + type: "tool_use", + name: "list_code_definition_names", + params: { path: "src" }, + partial: false, + } + + await listCodeDefinitionNamesTool( + mockTask as Task, + block, + mockAskApproval, + mockHandleError, + mockPushToolResult, + mockRemoveClosingTag, + ) + + expect(mockPushToolResult).toHaveBeenCalledWith(mockDefinitions) + }) +}) diff --git a/src/core/tools/__tests__/readFileTool.spec.ts b/src/core/tools/__tests__/readFileTool.spec.ts index e02140b72c47..d693d6ba4432 100644 --- a/src/core/tools/__tests__/readFileTool.spec.ts +++ b/src/core/tools/__tests__/readFileTool.spec.ts @@ -414,6 +414,61 @@ describe("read_file tool with maxReadFileLine setting", () => { expect(result).toContain(``) expect(result).toContain("Showing only 3 of 5 total lines") }) + + it("should truncate code definitions when file exceeds maxReadFileLine", async () => { + // Setup - file with 100 lines but we'll only read first 30 + const content = "Line 1\nLine 2\nLine 3" + const numberedContent = "1 | Line 1\n2 | Line 2\n3 | Line 3" + const fullDefinitions = `# file.txt +10--20 | function foo() { +50--60 | function bar() { +80--90 | function baz() {` + const truncatedDefinitions = `# file.txt +10--20 | function foo() {` + + mockedReadLines.mockResolvedValue(content) + mockedParseSourceCodeDefinitionsForFile.mockResolvedValue(fullDefinitions) + addLineNumbersMock.mockReturnValue(numberedContent) + + // Execute with maxReadFileLine = 30 + const result = await executeReadFileTool({}, { maxReadFileLine: 30, totalLines: 100 }) + + // Verify that only definitions within the first 30 lines are included + expect(result).toContain(`${testFilePath}`) + expect(result).toContain(``) + expect(result).toContain(``) + + // Should include foo (starts at line 10) but not bar (starts at line 50) or baz (starts at line 80) + expect(result).toContain("10--20 | function foo()") + expect(result).not.toContain("50--60 | function bar()") + expect(result).not.toContain("80--90 | function baz()") + + expect(result).toContain("Showing only 30 of 100 total lines") + }) + + it("should handle truncation when all definitions are beyond the line limit", async () => { + // Setup - all definitions start after maxReadFileLine + const content = "Line 1\nLine 2\nLine 3" + const numberedContent = "1 | Line 1\n2 | Line 2\n3 | Line 3" + const fullDefinitions = `# file.txt +50--60 | function foo() { +80--90 | function bar() {` + + mockedReadLines.mockResolvedValue(content) + mockedParseSourceCodeDefinitionsForFile.mockResolvedValue(fullDefinitions) + addLineNumbersMock.mockReturnValue(numberedContent) + + // Execute with maxReadFileLine = 30 + const result = await executeReadFileTool({}, { maxReadFileLine: 30, totalLines: 100 }) + + // Verify that only the header is included (all definitions filtered out) + expect(result).toContain(`${testFilePath}`) + expect(result).toContain(``) + expect(result).toContain(``) + expect(result).toContain("# file.txt") + expect(result).not.toContain("50--60 | function foo()") + expect(result).not.toContain("80--90 | function bar()") + }) }) describe("when maxReadFileLine equals or exceeds file length", () => { diff --git a/src/core/tools/helpers/__tests__/truncateDefinitions.spec.ts b/src/core/tools/helpers/__tests__/truncateDefinitions.spec.ts new file mode 100644 index 000000000000..a221b574055c --- /dev/null +++ b/src/core/tools/helpers/__tests__/truncateDefinitions.spec.ts @@ -0,0 +1,160 @@ +import { describe, it, expect } from "vitest" +import { truncateDefinitionsToLineLimit } from "../truncateDefinitions" + +describe("truncateDefinitionsToLineLimit", () => { + it("should not truncate when maxReadFileLine is -1 (no limit)", () => { + const definitions = `# test.ts +10--20 | function foo() { +30--40 | function bar() { +50--60 | function baz() {` + + const result = truncateDefinitionsToLineLimit(definitions, -1) + expect(result).toBe(definitions) + }) + + it("should not truncate when maxReadFileLine is 0 (definitions only mode)", () => { + const definitions = `# test.ts +10--20 | function foo() { +30--40 | function bar() { +50--60 | function baz() {` + + const result = truncateDefinitionsToLineLimit(definitions, 0) + expect(result).toBe(definitions) + }) + + it("should truncate definitions beyond the line limit", () => { + const definitions = `# test.ts +10--20 | function foo() { +30--40 | function bar() { +50--60 | function baz() {` + + const result = truncateDefinitionsToLineLimit(definitions, 25) + const expected = `# test.ts +10--20 | function foo() {` + + expect(result).toBe(expected) + }) + + it("should include definitions that start within limit even if they end beyond it", () => { + const definitions = `# test.ts +10--50 | function foo() { +60--80 | function bar() {` + + const result = truncateDefinitionsToLineLimit(definitions, 30) + const expected = `# test.ts +10--50 | function foo() {` + + expect(result).toBe(expected) + }) + + it("should handle single-line definitions", () => { + const definitions = `# test.ts +10 | const foo = 1 +20 | const bar = 2 +30 | const baz = 3` + + const result = truncateDefinitionsToLineLimit(definitions, 25) + const expected = `# test.ts +10 | const foo = 1 +20 | const bar = 2` + + expect(result).toBe(expected) + }) + + it("should preserve header line when all definitions are beyond limit", () => { + const definitions = `# test.ts +100--200 | function foo() {` + + const result = truncateDefinitionsToLineLimit(definitions, 50) + const expected = `# test.ts` + + expect(result).toBe(expected) + }) + + it("should handle empty definitions", () => { + const definitions = `# test.ts` + + const result = truncateDefinitionsToLineLimit(definitions, 50) + expect(result).toBe(definitions) + }) + + it("should handle definitions without header", () => { + const definitions = `10--20 | function foo() { +30--40 | function bar() {` + + const result = truncateDefinitionsToLineLimit(definitions, 25) + const expected = `10--20 | function foo() {` + + expect(result).toBe(expected) + }) + + it("should not preserve empty lines (only definition lines)", () => { + const definitions = `# test.ts +10--20 | function foo() { + +30--40 | function bar() {` + + const result = truncateDefinitionsToLineLimit(definitions, 25) + const expected = `# test.ts +10--20 | function foo() {` + + expect(result).toBe(expected) + }) + + it("should handle mixed single and range definitions", () => { + const definitions = `# test.ts +5 | const x = 1 +10--20 | function foo() { +25 | const y = 2 +30--40 | function bar() {` + + const result = truncateDefinitionsToLineLimit(definitions, 26) + const expected = `# test.ts +5 | const x = 1 +10--20 | function foo() { +25 | const y = 2` + + expect(result).toBe(expected) + }) + + it("should handle definitions at exactly the limit", () => { + const definitions = `# test.ts +10--20 | function foo() { +30--40 | function bar() { +50--60 | function baz() {` + + const result = truncateDefinitionsToLineLimit(definitions, 30) + const expected = `# test.ts +10--20 | function foo() { +30--40 | function bar() {` + + expect(result).toBe(expected) + }) + + it("should handle definitions with leading whitespace", () => { + const definitions = `# test.ts + 10--20 | function foo() { + 30--40 | function bar() { + 50--60 | function baz() {` + + const result = truncateDefinitionsToLineLimit(definitions, 25) + const expected = `# test.ts + 10--20 | function foo() {` + + expect(result).toBe(expected) + }) + + it("should handle definitions with mixed whitespace patterns", () => { + const definitions = `# test.ts +10--20 | function foo() { + 30--40 | function bar() { + 50--60 | function baz() {` + + const result = truncateDefinitionsToLineLimit(definitions, 35) + const expected = `# test.ts +10--20 | function foo() { + 30--40 | function bar() {` + + expect(result).toBe(expected) + }) +}) diff --git a/src/core/tools/helpers/truncateDefinitions.ts b/src/core/tools/helpers/truncateDefinitions.ts new file mode 100644 index 000000000000..7c193ef52a5f --- /dev/null +++ b/src/core/tools/helpers/truncateDefinitions.ts @@ -0,0 +1,44 @@ +/** + * Truncate code definitions to only include those within the line limit + * @param definitions - The full definitions string from parseSourceCodeDefinitionsForFile + * @param maxReadFileLine - Maximum line number to include (-1 for no limit, 0 for definitions only) + * @returns Truncated definitions string + */ +export function truncateDefinitionsToLineLimit(definitions: string, maxReadFileLine: number): string { + // If no limit or definitions-only mode (0), return as-is + if (maxReadFileLine <= 0) { + return definitions + } + + const lines = definitions.split("\n") + const result: string[] = [] + let startIndex = 0 + + // Keep the header line (e.g., "# filename.ts") + if (lines.length > 0 && lines[0].startsWith("#")) { + result.push(lines[0]) + startIndex = 1 + } + + // Process definition lines + for (let i = startIndex; i < lines.length; i++) { + const line = lines[i] + + // Match definition format: "startLine--endLine | content" or "lineNumber | content" + // Allow optional leading whitespace to handle indented output or CRLF artifacts + const rangeMatch = line.match(/^\s*(\d+)(?:--(\d+))?\s*\|/) + + if (rangeMatch) { + const startLine = parseInt(rangeMatch[1], 10) + + // Only include definitions that start within the truncated range + if (startLine <= maxReadFileLine) { + result.push(line) + } + } + // Note: We don't preserve empty lines or other non-definition content + // as they're not part of the actual code definitions + } + + return result.join("\n") +} diff --git a/src/core/tools/listCodeDefinitionNamesTool.ts b/src/core/tools/listCodeDefinitionNamesTool.ts index 6ceec0a7257a..0ec80ce9bd04 100644 --- a/src/core/tools/listCodeDefinitionNamesTool.ts +++ b/src/core/tools/listCodeDefinitionNamesTool.ts @@ -8,6 +8,7 @@ import { getReadablePath } from "../../utils/path" import { isPathOutsideWorkspace } from "../../utils/pathUtils" import { parseSourceCodeForDefinitionsTopLevel, parseSourceCodeDefinitionsForFile } from "../../services/tree-sitter" import { RecordSource } from "../context-tracking/FileContextTrackerTypes" +import { truncateDefinitionsToLineLimit } from "./helpers/truncateDefinitions" export async function listCodeDefinitionNamesTool( cline: Task, @@ -51,7 +52,14 @@ export async function listCodeDefinitionNamesTool( if (stats.isFile()) { const fileResult = await parseSourceCodeDefinitionsForFile(absolutePath, cline.rooIgnoreController) - result = fileResult ?? "No source code definitions found in cline file." + + // Apply truncation based on maxReadFileLine setting + if (fileResult) { + const { maxReadFileLine = -1 } = (await cline.providerRef.deref()?.getState()) ?? {} + result = truncateDefinitionsToLineLimit(fileResult, maxReadFileLine) + } else { + result = "No source code definitions found in file." + } } else if (stats.isDirectory()) { result = await parseSourceCodeForDefinitionsTopLevel(absolutePath, cline.rooIgnoreController) } else { diff --git a/src/core/tools/readFileTool.ts b/src/core/tools/readFileTool.ts index 6223d61f87c8..53f0643dbb4c 100644 --- a/src/core/tools/readFileTool.ts +++ b/src/core/tools/readFileTool.ts @@ -23,6 +23,7 @@ import { ImageMemoryTracker, } from "./helpers/imageHelpers" import { validateFileTokenBudget, truncateFileContent } from "./helpers/fileTokenBudget" +import { truncateDefinitionsToLineLimit } from "./helpers/truncateDefinitions" export function getReadFileToolDescription(blockName: string, blockParams: any): string { // Handle both single path and multiple files via args @@ -577,7 +578,9 @@ export async function readFileTool( try { const defResult = await parseSourceCodeDefinitionsForFile(fullPath, cline.rooIgnoreController) if (defResult) { - xmlInfo += `${defResult}\n` + // Truncate definitions to match the truncated file content + const truncatedDefs = truncateDefinitionsToLineLimit(defResult, maxReadFileLine) + xmlInfo += `${truncatedDefs}\n` } xmlInfo += `Showing only ${maxReadFileLine} of ${totalLines} total lines. Use line_range if you need to read more lines\n` updateFileResult(relPath, {