mirror of
https://github.com/RooVetGit/Roo-Code.git
synced 2026-08-28 05:27:24 +00:00
Fixes #4860: Cap problems output to prevent context overflow
- Add MAX_PROBLEMS_IN_CONTEXT constant (100) to limit problems sent to session - Update diagnosticsToProblemsString to accept optional maxProblems parameter - Add truncation message when problems exceed limit - Update getWorkspaceProblems to use the new limit - Maintain backward compatibility for DiffViewProvider (no limit for edit-specific problems) - Add comprehensive tests for the new limiting functionality This prevents scenarios where large numbers of problems (e.g., 60,000+ from failed pnpm install) can overwhelm the session context with 800k+ tokens, making sessions unresponsive and expensive.
This commit is contained in:
parent
2e2f83be60
commit
82a4fa988f
4 changed files with 206 additions and 0 deletions
|
|
@ -245,12 +245,16 @@ async function getFileOrFolderContent(
|
|||
}
|
||||
}
|
||||
|
||||
// Maximum number of problems to include in session context to prevent overwhelming the session
|
||||
const MAX_PROBLEMS_IN_CONTEXT = 100
|
||||
|
||||
async function getWorkspaceProblems(cwd: string): Promise<string> {
|
||||
const diagnostics = vscode.languages.getDiagnostics()
|
||||
const result = await diagnosticsToProblemsString(
|
||||
diagnostics,
|
||||
[vscode.DiagnosticSeverity.Error, vscode.DiagnosticSeverity.Warning],
|
||||
cwd,
|
||||
MAX_PROBLEMS_IN_CONTEXT,
|
||||
)
|
||||
if (!result) {
|
||||
return "No errors or warnings detected."
|
||||
|
|
|
|||
|
|
@ -384,4 +384,188 @@ describe("diagnosticsToProblemsString", () => {
|
|||
expect(vscode.workspace.fs.stat).toHaveBeenCalledWith(fileUri)
|
||||
expect(vscode.workspace.openTextDocument).toHaveBeenCalledWith(fileUri)
|
||||
})
|
||||
|
||||
it("should limit the number of problems when maxProblems is specified", async () => {
|
||||
// Mock file URI
|
||||
const fileUri = vscode.Uri.file("/path/to/file.ts")
|
||||
|
||||
// Create more diagnostics than the limit
|
||||
const diagnostics = [
|
||||
new vscode.Diagnostic(new vscode.Range(0, 0, 0, 10), "Error 1", vscode.DiagnosticSeverity.Error),
|
||||
new vscode.Diagnostic(new vscode.Range(1, 0, 1, 10), "Error 2", vscode.DiagnosticSeverity.Error),
|
||||
new vscode.Diagnostic(new vscode.Range(2, 0, 2, 10), "Error 3", vscode.DiagnosticSeverity.Error),
|
||||
new vscode.Diagnostic(new vscode.Range(3, 0, 3, 10), "Error 4", vscode.DiagnosticSeverity.Error),
|
||||
new vscode.Diagnostic(new vscode.Range(4, 0, 4, 10), "Error 5", vscode.DiagnosticSeverity.Error),
|
||||
]
|
||||
|
||||
// Mock fs.stat to return file type
|
||||
const mockStat = {
|
||||
type: vscode.FileType.File,
|
||||
}
|
||||
vscode.workspace.fs.stat = vitest.fn().mockResolvedValue(mockStat)
|
||||
|
||||
// Mock document content
|
||||
const mockDocument = {
|
||||
lineAt: vitest.fn((line) => ({
|
||||
text: `Line ${line + 1} content`,
|
||||
})),
|
||||
}
|
||||
vscode.workspace.openTextDocument = vitest.fn().mockResolvedValue(mockDocument)
|
||||
|
||||
// Test with a limit of 3 problems
|
||||
const result = await diagnosticsToProblemsString(
|
||||
[[fileUri, diagnostics]],
|
||||
[vscode.DiagnosticSeverity.Error],
|
||||
"/path/to",
|
||||
3,
|
||||
)
|
||||
|
||||
// Verify only first 3 problems are included
|
||||
expect(result).toContain("Error 1")
|
||||
expect(result).toContain("Error 2")
|
||||
expect(result).toContain("Error 3")
|
||||
expect(result).not.toContain("Error 4")
|
||||
expect(result).not.toContain("Error 5")
|
||||
|
||||
// Verify truncation message is included
|
||||
expect(result).toContain("... and 2 more problems (truncated to prevent context overflow)")
|
||||
})
|
||||
|
||||
it("should not show truncation message when problem count is within limit", async () => {
|
||||
// Mock file URI
|
||||
const fileUri = vscode.Uri.file("/path/to/file.ts")
|
||||
|
||||
// Create fewer diagnostics than the limit
|
||||
const diagnostics = [
|
||||
new vscode.Diagnostic(new vscode.Range(0, 0, 0, 10), "Error 1", vscode.DiagnosticSeverity.Error),
|
||||
new vscode.Diagnostic(new vscode.Range(1, 0, 1, 10), "Error 2", vscode.DiagnosticSeverity.Error),
|
||||
]
|
||||
|
||||
// Mock fs.stat to return file type
|
||||
const mockStat = {
|
||||
type: vscode.FileType.File,
|
||||
}
|
||||
vscode.workspace.fs.stat = vitest.fn().mockResolvedValue(mockStat)
|
||||
|
||||
// Mock document content
|
||||
const mockDocument = {
|
||||
lineAt: vitest.fn((line) => ({
|
||||
text: `Line ${line + 1} content`,
|
||||
})),
|
||||
}
|
||||
vscode.workspace.openTextDocument = vitest.fn().mockResolvedValue(mockDocument)
|
||||
|
||||
// Test with a limit of 5 problems (more than we have)
|
||||
const result = await diagnosticsToProblemsString(
|
||||
[[fileUri, diagnostics]],
|
||||
[vscode.DiagnosticSeverity.Error],
|
||||
"/path/to",
|
||||
5,
|
||||
)
|
||||
|
||||
// Verify all problems are included
|
||||
expect(result).toContain("Error 1")
|
||||
expect(result).toContain("Error 2")
|
||||
|
||||
// Verify no truncation message is included
|
||||
expect(result).not.toContain("more problems")
|
||||
expect(result).not.toContain("truncated")
|
||||
})
|
||||
|
||||
it("should work correctly when maxProblems is undefined (no limit)", async () => {
|
||||
// Mock file URI
|
||||
const fileUri = vscode.Uri.file("/path/to/file.ts")
|
||||
|
||||
// Create many diagnostics
|
||||
const diagnostics = Array.from(
|
||||
{ length: 10 },
|
||||
(_, i) =>
|
||||
new vscode.Diagnostic(new vscode.Range(i, 0, i, 10), `Error ${i + 1}`, vscode.DiagnosticSeverity.Error),
|
||||
)
|
||||
|
||||
// Mock fs.stat to return file type
|
||||
const mockStat = {
|
||||
type: vscode.FileType.File,
|
||||
}
|
||||
vscode.workspace.fs.stat = vitest.fn().mockResolvedValue(mockStat)
|
||||
|
||||
// Mock document content
|
||||
const mockDocument = {
|
||||
lineAt: vitest.fn((line) => ({
|
||||
text: `Line ${line + 1} content`,
|
||||
})),
|
||||
}
|
||||
vscode.workspace.openTextDocument = vitest.fn().mockResolvedValue(mockDocument)
|
||||
|
||||
// Test without specifying maxProblems (should include all)
|
||||
const result = await diagnosticsToProblemsString(
|
||||
[[fileUri, diagnostics]],
|
||||
[vscode.DiagnosticSeverity.Error],
|
||||
"/path/to",
|
||||
)
|
||||
|
||||
// Verify all problems are included
|
||||
for (let i = 1; i <= 10; i++) {
|
||||
expect(result).toContain(`Error ${i}`)
|
||||
}
|
||||
|
||||
// Verify no truncation message is included
|
||||
expect(result).not.toContain("more problems")
|
||||
expect(result).not.toContain("truncated")
|
||||
})
|
||||
|
||||
it("should handle limit across multiple files correctly", async () => {
|
||||
// Mock URIs for different files
|
||||
const fileUri1 = vscode.Uri.file("/path/to/file1.ts")
|
||||
const fileUri2 = vscode.Uri.file("/path/to/file2.ts")
|
||||
|
||||
// Create diagnostics for each file
|
||||
const diagnostics1 = [
|
||||
new vscode.Diagnostic(new vscode.Range(0, 0, 0, 10), "File1 Error 1", vscode.DiagnosticSeverity.Error),
|
||||
new vscode.Diagnostic(new vscode.Range(1, 0, 1, 10), "File1 Error 2", vscode.DiagnosticSeverity.Error),
|
||||
]
|
||||
|
||||
const diagnostics2 = [
|
||||
new vscode.Diagnostic(new vscode.Range(0, 0, 0, 10), "File2 Error 1", vscode.DiagnosticSeverity.Error),
|
||||
new vscode.Diagnostic(new vscode.Range(1, 0, 1, 10), "File2 Error 2", vscode.DiagnosticSeverity.Error),
|
||||
new vscode.Diagnostic(new vscode.Range(2, 0, 2, 10), "File2 Error 3", vscode.DiagnosticSeverity.Error),
|
||||
]
|
||||
|
||||
// Mock fs.stat to return file type
|
||||
const mockStat = {
|
||||
type: vscode.FileType.File,
|
||||
}
|
||||
vscode.workspace.fs.stat = vitest.fn().mockResolvedValue(mockStat)
|
||||
|
||||
// Mock document content
|
||||
const mockDocument = {
|
||||
lineAt: vitest.fn((line) => ({
|
||||
text: `Line ${line + 1} content`,
|
||||
})),
|
||||
}
|
||||
vscode.workspace.openTextDocument = vitest.fn().mockResolvedValue(mockDocument)
|
||||
|
||||
// Test with a limit of 3 problems (should include all from file1 and only 1 from file2)
|
||||
const result = await diagnosticsToProblemsString(
|
||||
[
|
||||
[fileUri1, diagnostics1],
|
||||
[fileUri2, diagnostics2],
|
||||
],
|
||||
[vscode.DiagnosticSeverity.Error],
|
||||
"/path/to",
|
||||
3,
|
||||
)
|
||||
|
||||
// Verify first file's problems are included
|
||||
expect(result).toContain("File1 Error 1")
|
||||
expect(result).toContain("File1 Error 2")
|
||||
|
||||
// Verify only first problem from second file is included
|
||||
expect(result).toContain("File2 Error 1")
|
||||
expect(result).not.toContain("File2 Error 2")
|
||||
expect(result).not.toContain("File2 Error 3")
|
||||
|
||||
// Verify truncation message shows correct remaining count
|
||||
expect(result).toContain("... and 2 more problems (truncated to prevent context overflow)")
|
||||
})
|
||||
})
|
||||
|
|
|
|||
|
|
@ -74,10 +74,19 @@ export async function diagnosticsToProblemsString(
|
|||
diagnostics: [vscode.Uri, vscode.Diagnostic[]][],
|
||||
severities: vscode.DiagnosticSeverity[],
|
||||
cwd: string,
|
||||
maxProblems?: number,
|
||||
): Promise<string> {
|
||||
const documents = new Map<vscode.Uri, vscode.TextDocument>()
|
||||
const fileStats = new Map<vscode.Uri, vscode.FileStat>()
|
||||
let result = ""
|
||||
let problemCount = 0
|
||||
let totalProblems = 0
|
||||
|
||||
// First, count total problems to determine if we need to show a truncation message
|
||||
for (const [, fileDiagnostics] of diagnostics) {
|
||||
totalProblems += fileDiagnostics.filter((d) => severities.includes(d.severity)).length
|
||||
}
|
||||
|
||||
for (const [uri, fileDiagnostics] of diagnostics) {
|
||||
const problems = fileDiagnostics
|
||||
.filter((d) => severities.includes(d.severity))
|
||||
|
|
@ -85,6 +94,13 @@ export async function diagnosticsToProblemsString(
|
|||
if (problems.length > 0) {
|
||||
result += `\n\n${path.relative(cwd, uri.fsPath).toPosix()}`
|
||||
for (const diagnostic of problems) {
|
||||
// Check if we've reached the maximum number of problems
|
||||
if (maxProblems && problemCount >= maxProblems) {
|
||||
const remainingProblems = totalProblems - problemCount
|
||||
result += `\n\n... and ${remainingProblems} more problems (truncated to prevent context overflow)`
|
||||
return result.trim()
|
||||
}
|
||||
|
||||
let label: string
|
||||
switch (diagnostic.severity) {
|
||||
case vscode.DiagnosticSeverity.Error:
|
||||
|
|
@ -121,6 +137,7 @@ export async function diagnosticsToProblemsString(
|
|||
} catch {
|
||||
result += `\n- [${source}${label}] ${line} | (unavailable) : ${diagnostic.message}`
|
||||
}
|
||||
problemCount++
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -220,6 +220,7 @@ export class DiffViewProvider {
|
|||
vscode.DiagnosticSeverity.Error, // only including errors since warnings can be distracting (if user wants to fix warnings they can use the @problems mention)
|
||||
],
|
||||
this.cwd,
|
||||
// No limit for new problems since these are specifically related to the current edit
|
||||
) // Will be empty string if no errors.
|
||||
|
||||
const newProblemsMessage =
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue