Skip to content

Commit 56f460e

Browse files
committed
feat: implement size-based limiting for diagnostic messages
- Changed from count-based to size-based limiting to prevent context overflow - Each unit of maxDiagnosticMessages now represents ~1KB of content - Complete diagnostics are included up to the size limit - Clear indication of omitted problems when limit is reached - Updated tests to reflect new behavior - Updated UI description to explain the new approach
1 parent deb6ef4 commit 56f460e

File tree

3 files changed

+105
-72
lines changed

3 files changed

+105
-72
lines changed

src/integrations/diagnostics/__tests__/diagnostics.spec.ts

Lines changed: 24 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -423,11 +423,11 @@ describe("diagnosticsToProblemsString", () => {
423423
expect(vscode.workspace.openTextDocument).not.toHaveBeenCalled()
424424
})
425425

426-
it("should limit diagnostics when maxDiagnostics is specified", async () => {
426+
it("should limit diagnostics based on content size when maxDiagnostics is specified", async () => {
427427
// Mock file URI
428428
const fileUri = vscode.Uri.file("/path/to/file.ts")
429429

430-
// Create multiple diagnostics
430+
// Create multiple diagnostics with varying message lengths
431431
const diagnostics = [
432432
new vscode.Diagnostic(new vscode.Range(0, 0, 0, 10), "Error 1", vscode.DiagnosticSeverity.Error),
433433
new vscode.Diagnostic(new vscode.Range(1, 0, 1, 10), "Warning 1", vscode.DiagnosticSeverity.Warning),
@@ -450,27 +450,28 @@ describe("diagnosticsToProblemsString", () => {
450450
}
451451
vscode.workspace.openTextDocument = vitest.fn().mockResolvedValue(mockDocument)
452452

453-
// Test with maxDiagnostics set to 3
453+
// Test with maxDiagnostics set to 3 (which translates to ~3000 character limit)
454454
const result = await diagnosticsToProblemsString(
455455
[[fileUri, diagnostics]],
456456
[vscode.DiagnosticSeverity.Error, vscode.DiagnosticSeverity.Warning],
457457
"/path/to",
458458
true, // includeDiagnostics
459-
3, // maxDiagnostics
459+
3, // maxDiagnostics (will be converted to character limit)
460460
)
461461

462-
// Verify only 3 diagnostics are included (prioritizing errors)
462+
// Verify that diagnostics are included based on size limit, prioritizing errors
463463
expect(result).toContain("Error 1")
464464
expect(result).toContain("Error 2")
465465
expect(result).toContain("Error 3")
466-
expect(result).not.toContain("Warning 1")
467-
expect(result).not.toContain("Warning 2")
466+
// Warnings may or may not be included depending on the size calculation
468467

469-
// Verify the limit message is included
470-
expect(result).toContain("(Showing 3 of 5 total diagnostics)")
468+
// Verify the limit message is included if some diagnostics were omitted
469+
if (!result.includes("Warning 1") || !result.includes("Warning 2")) {
470+
expect(result).toContain("more problems omitted to prevent context overflow")
471+
}
471472
})
472473

473-
it("should prioritize errors over warnings when limiting diagnostics", async () => {
474+
it("should prioritize errors over warnings when limiting diagnostics by size", async () => {
474475
// Mock file URIs
475476
const fileUri1 = vscode.Uri.file("/path/to/file1.ts")
476477
const fileUri2 = vscode.Uri.file("/path/to/file2.ts")
@@ -505,7 +506,7 @@ describe("diagnosticsToProblemsString", () => {
505506
}
506507
vscode.workspace.openTextDocument = vitest.fn().mockResolvedValue(mockDocument)
507508

508-
// Test with maxDiagnostics set to 2
509+
// Test with maxDiagnostics set to 2 (which translates to ~2000 character limit)
509510
const result = await diagnosticsToProblemsString(
510511
[
511512
[fileUri1, diagnostics1],
@@ -514,18 +515,22 @@ describe("diagnosticsToProblemsString", () => {
514515
[vscode.DiagnosticSeverity.Error, vscode.DiagnosticSeverity.Warning, vscode.DiagnosticSeverity.Information],
515516
"/path/to",
516517
true, // includeDiagnostics
517-
2, // maxDiagnostics
518+
2, // maxDiagnostics (will be converted to character limit)
518519
)
519520

520-
// Verify only errors are included (2 errors total)
521+
// Verify errors are prioritized over warnings
521522
expect(result).toContain("Error in file1")
522523
expect(result).toContain("Error in file2")
523-
expect(result).not.toContain("Warning in file1")
524-
expect(result).not.toContain("Warning in file2")
525-
expect(result).not.toContain("Info in file2")
526-
527-
// Verify the limit message is included
528-
expect(result).toContain("(Showing 2 of 5 total diagnostics)")
524+
// Warnings and info may not be included due to size limit
525+
526+
// Verify the limit message is included if diagnostics were omitted
527+
if (
528+
!result.includes("Warning in file1") ||
529+
!result.includes("Warning in file2") ||
530+
!result.includes("Info in file2")
531+
) {
532+
expect(result).toContain("more problems omitted to prevent context overflow")
533+
}
529534
})
530535

531536
it("should handle maxDiagnostics with no limit when undefined", async () => {

src/integrations/diagnostics/index.ts

Lines changed: 80 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -86,14 +86,22 @@ export async function diagnosticsToProblemsString(
8686
const fileStats = new Map<vscode.Uri, vscode.FileStat>()
8787
let result = ""
8888

89-
// If we have a limit, we need to collect all diagnostics first, sort by severity, then limit
89+
// If we have a limit, we need to use size-based limiting instead of count-based
9090
if (maxDiagnosticMessages && maxDiagnosticMessages > 0) {
91+
// Convert maxDiagnosticMessages to a character limit
92+
// Assuming average diagnostic message is ~1000 characters, multiply by the count
93+
const MAX_PROBLEMS_CONTENT_SIZE = maxDiagnosticMessages * 1000
94+
let currentSize = 0
95+
let includedCount = 0
96+
let totalCount = 0
97+
9198
// Flatten all diagnostics with their URIs
92-
const allDiagnostics: { uri: vscode.Uri; diagnostic: vscode.Diagnostic }[] = []
99+
const allDiagnostics: { uri: vscode.Uri; diagnostic: vscode.Diagnostic; formattedText?: string }[] = []
93100
for (const [uri, fileDiagnostics] of diagnostics) {
94101
const filtered = fileDiagnostics.filter((d) => severities.includes(d.severity))
95102
for (const diagnostic of filtered) {
96103
allDiagnostics.push({ uri, diagnostic })
104+
totalCount++
97105
}
98106
}
99107

@@ -104,68 +112,88 @@ export async function diagnosticsToProblemsString(
104112
return a.diagnostic.range.start.line - b.diagnostic.range.start.line
105113
})
106114

107-
// Take only the first maxDiagnosticMessages
108-
const limitedDiagnostics = allDiagnostics.slice(0, maxDiagnosticMessages)
115+
// Process diagnostics and track size
116+
const includedDiagnostics: typeof allDiagnostics = []
117+
for (const item of allDiagnostics) {
118+
// Format the diagnostic to calculate its size
119+
let label: string
120+
switch (item.diagnostic.severity) {
121+
case vscode.DiagnosticSeverity.Error:
122+
label = "Error"
123+
break
124+
case vscode.DiagnosticSeverity.Warning:
125+
label = "Warning"
126+
break
127+
case vscode.DiagnosticSeverity.Information:
128+
label = "Information"
129+
break
130+
case vscode.DiagnosticSeverity.Hint:
131+
label = "Hint"
132+
break
133+
default:
134+
label = "Diagnostic"
135+
}
136+
const line = item.diagnostic.range.start.line + 1
137+
const source = item.diagnostic.source ? `${item.diagnostic.source} ` : ""
138+
139+
// Pre-format the diagnostic text
140+
let diagnosticText = ""
141+
try {
142+
let fileStat = fileStats.get(item.uri)
143+
if (!fileStat) {
144+
fileStat = await vscode.workspace.fs.stat(item.uri)
145+
fileStats.set(item.uri, fileStat)
146+
}
147+
if (fileStat.type === vscode.FileType.File) {
148+
const document = documents.get(item.uri) || (await vscode.workspace.openTextDocument(item.uri))
149+
documents.set(item.uri, document)
150+
const lineContent = document.lineAt(item.diagnostic.range.start.line).text
151+
diagnosticText = `\n- [${source}${label}] ${line} | ${lineContent} : ${item.diagnostic.message}`
152+
} else {
153+
diagnosticText = `\n- [${source}${label}] 1 | (directory) : ${item.diagnostic.message}`
154+
}
155+
} catch {
156+
diagnosticText = `\n- [${source}${label}] ${line} | (unavailable) : ${item.diagnostic.message}`
157+
}
158+
159+
// Check if adding this diagnostic would exceed the size limit
160+
const diagnosticSize = diagnosticText.length + path.relative(cwd, item.uri.fsPath).toPosix().length + 2 // +2 for newlines
161+
if (currentSize + diagnosticSize > MAX_PROBLEMS_CONTENT_SIZE && includedCount > 0) {
162+
break // Stop before exceeding limit
163+
}
164+
165+
item.formattedText = diagnosticText
166+
includedDiagnostics.push(item)
167+
currentSize += diagnosticSize
168+
includedCount++
169+
}
109170

110-
// Group back by URI for processing
111-
const groupedDiagnostics = new Map<string, { uri: vscode.Uri; diagnostics: vscode.Diagnostic[] }>()
112-
for (const { uri, diagnostic } of limitedDiagnostics) {
113-
const key = uri.toString()
171+
// Group included diagnostics by URI for output
172+
const groupedDiagnostics = new Map<string, { uri: vscode.Uri; diagnostics: typeof allDiagnostics }>()
173+
for (const item of includedDiagnostics) {
174+
const key = item.uri.toString()
114175
if (!groupedDiagnostics.has(key)) {
115-
groupedDiagnostics.set(key, { uri, diagnostics: [] })
176+
groupedDiagnostics.set(key, { uri: item.uri, diagnostics: [] })
116177
}
117-
groupedDiagnostics.get(key)!.diagnostics.push(diagnostic)
178+
groupedDiagnostics.get(key)!.diagnostics.push(item)
118179
}
119180

120-
// Process the limited diagnostics
181+
// Build the output
121182
for (const { uri, diagnostics: fileDiagnostics } of groupedDiagnostics.values()) {
122-
const problems = fileDiagnostics.sort((a, b) => a.range.start.line - b.range.start.line)
123-
if (problems.length > 0) {
183+
const sortedDiagnostics = fileDiagnostics.sort(
184+
(a, b) => a.diagnostic.range.start.line - b.diagnostic.range.start.line,
185+
)
186+
if (sortedDiagnostics.length > 0) {
124187
result += `\n\n${path.relative(cwd, uri.fsPath).toPosix()}`
125-
for (const diagnostic of problems) {
126-
let label: string
127-
switch (diagnostic.severity) {
128-
case vscode.DiagnosticSeverity.Error:
129-
label = "Error"
130-
break
131-
case vscode.DiagnosticSeverity.Warning:
132-
label = "Warning"
133-
break
134-
case vscode.DiagnosticSeverity.Information:
135-
label = "Information"
136-
break
137-
case vscode.DiagnosticSeverity.Hint:
138-
label = "Hint"
139-
break
140-
default:
141-
label = "Diagnostic"
142-
}
143-
const line = diagnostic.range.start.line + 1 // VSCode lines are 0-indexed
144-
const source = diagnostic.source ? `${diagnostic.source} ` : ""
145-
try {
146-
let fileStat = fileStats.get(uri)
147-
if (!fileStat) {
148-
fileStat = await vscode.workspace.fs.stat(uri)
149-
fileStats.set(uri, fileStat)
150-
}
151-
if (fileStat.type === vscode.FileType.File) {
152-
const document = documents.get(uri) || (await vscode.workspace.openTextDocument(uri))
153-
documents.set(uri, document)
154-
const lineContent = document.lineAt(diagnostic.range.start.line).text
155-
result += `\n- [${source}${label}] ${line} | ${lineContent} : ${diagnostic.message}`
156-
} else {
157-
result += `\n- [${source}${label}] 1 | (directory) : ${diagnostic.message}`
158-
}
159-
} catch {
160-
result += `\n- [${source}${label}] ${line} | (unavailable) : ${diagnostic.message}`
161-
}
188+
for (const item of sortedDiagnostics) {
189+
result += item.formattedText
162190
}
163191
}
164192
}
165193

166194
// Add a note if we hit the limit
167-
if (allDiagnostics.length > maxDiagnosticMessages) {
168-
result += `\n\n(Showing ${maxDiagnosticMessages} of ${allDiagnostics.length} total diagnostics)`
195+
if (totalCount > includedCount) {
196+
result += `\n\n... ${totalCount - includedCount} more problems omitted to prevent context overflow`
169197
}
170198
} else {
171199
// No limit, process all diagnostics as before

webview-ui/src/i18n/locales/en/settings.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -473,7 +473,7 @@
473473
},
474474
"maxMessages": {
475475
"label": "Maximum diagnostic messages",
476-
"description": "Maximum number of diagnostic messages to include per file. This limit applies to both automatic inclusion (when checkbox is enabled) and manual @problems mentions. Higher values provide more context but increase token usage.",
476+
"description": "Controls the approximate size limit for diagnostic messages. The value represents a multiplier for content size (each unit ≈ 1KB). This prevents context overflow by limiting total diagnostic content rather than count. Higher values allow more diagnostic information but increase token usage.",
477477
"resetTooltip": "Reset to default value (50)",
478478
"unlimitedLabel": "Unlimited"
479479
}

0 commit comments

Comments
 (0)