fix(context): truncate type definition to match max read line (#8509)

Co-authored-by: daniel-lxs <ricciodaniel98@gmail.com>
This commit is contained in:
Dicha Zelianivan Arkana 2025-10-31 03:55:55 +07:00 committed by GitHub
parent 9dfa496659
commit f583be3c30
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
6 changed files with 623 additions and 2 deletions

View file

@ -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<Task>
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)
})
})

View file

@ -414,6 +414,61 @@ describe("read_file tool with maxReadFileLine setting", () => {
expect(result).toContain(`<list_code_definition_names>`)
expect(result).toContain("<notice>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(`<file><path>${testFilePath}</path>`)
expect(result).toContain(`<content lines="1-30">`)
expect(result).toContain(`<list_code_definition_names>`)
// 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("<notice>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(`<file><path>${testFilePath}</path>`)
expect(result).toContain(`<content lines="1-30">`)
expect(result).toContain(`<list_code_definition_names>`)
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", () => {

View file

@ -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)
})
})

View file

@ -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")
}

View file

@ -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 {

View file

@ -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 += `<list_code_definition_names>${defResult}</list_code_definition_names>\n`
// Truncate definitions to match the truncated file content
const truncatedDefs = truncateDefinitionsToLineLimit(defResult, maxReadFileLine)
xmlInfo += `<list_code_definition_names>${truncatedDefs}</list_code_definition_names>\n`
}
xmlInfo += `<notice>Showing only ${maxReadFileLine} of ${totalLines} total lines. Use line_range if you need to read more lines</notice>\n`
updateFileResult(relPath, {