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/helpers/__tests__/truncateDefinitions.spec.ts b/src/core/tools/helpers/__tests__/truncateDefinitions.spec.ts new file mode 100644 index 000000000000..e45c6a42a8d4 --- /dev/null +++ b/src/core/tools/helpers/__tests__/truncateDefinitions.spec.ts @@ -0,0 +1,133 @@ +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) + }) +}) diff --git a/src/core/tools/helpers/truncateDefinitions.ts b/src/core/tools/helpers/truncateDefinitions.ts new file mode 100644 index 000000000000..1f60cc0c3e56 --- /dev/null +++ b/src/core/tools/helpers/truncateDefinitions.ts @@ -0,0 +1,43 @@ +/** + * 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" + const rangeMatch = line.match(/^(\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 01427f4d9dc7..be5fa64d745e 100644 --- a/src/core/tools/readFileTool.ts +++ b/src/core/tools/readFileTool.ts @@ -22,6 +22,7 @@ import { processImageFile, ImageMemoryTracker, } from "./helpers/imageHelpers" +import { truncateDefinitionsToLineLimit } from "./helpers/truncateDefinitions" export function getReadFileToolDescription(blockName: string, blockParams: any): string { // Handle both single path and multiple files via args @@ -576,7 +577,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, {