Skip to content

Commit 2b1b6a6

Browse files
hannesrudolphdaniel-lxs
authored andcommitted
fix: change diagnostic limiting from size-based to count-based
- Modified diagnosticsToProblemsString() to limit by message count instead of content size - Updated UI description to accurately reflect count-based limiting - Updated tests to verify count-based limiting works correctly - When maxDiagnosticMessages is set to 1, it now shows exactly 1 diagnostic message
1 parent a0c25cc commit 2b1b6a6

File tree

3 files changed

+29
-38
lines changed

3 files changed

+29
-38
lines changed

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

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

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

@@ -450,28 +450,28 @@ describe("diagnosticsToProblemsString", () => {
450450
}
451451
vscode.workspace.openTextDocument = vitest.fn().mockResolvedValue(mockDocument)
452452

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

462-
// Verify that diagnostics are included based on size limit, prioritizing errors
462+
// Verify that exactly 3 diagnostics are included, prioritizing errors
463463
expect(result).toContain("Error 1")
464464
expect(result).toContain("Error 2")
465465
expect(result).toContain("Error 3")
466-
// Warnings may or may not be included depending on the size calculation
466+
// Warnings should not be included since we have 3 errors and limit is 3
467+
expect(result).not.toContain("Warning 1")
468+
expect(result).not.toContain("Warning 2")
467469

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-
}
470+
// Verify the limit message is included
471+
expect(result).toContain("2 more problems omitted to prevent context overflow")
472472
})
473473

474-
it("should prioritize errors over warnings when limiting diagnostics by size", async () => {
474+
it("should prioritize errors over warnings when limiting diagnostics by count", async () => {
475475
// Mock file URIs
476476
const fileUri1 = vscode.Uri.file("/path/to/file1.ts")
477477
const fileUri2 = vscode.Uri.file("/path/to/file2.ts")
@@ -506,7 +506,7 @@ describe("diagnosticsToProblemsString", () => {
506506
}
507507
vscode.workspace.openTextDocument = vitest.fn().mockResolvedValue(mockDocument)
508508

509-
// Test with maxDiagnostics set to 2 (which translates to ~2000 character limit)
509+
// Test with maxDiagnostics set to 2 (should include exactly 2 diagnostics)
510510
const result = await diagnosticsToProblemsString(
511511
[
512512
[fileUri1, diagnostics1],
@@ -515,22 +515,19 @@ describe("diagnosticsToProblemsString", () => {
515515
[vscode.DiagnosticSeverity.Error, vscode.DiagnosticSeverity.Warning, vscode.DiagnosticSeverity.Information],
516516
"/path/to",
517517
true, // includeDiagnostics
518-
2, // maxDiagnostics (will be converted to character limit)
518+
2, // maxDiagnostics (count limit)
519519
)
520520

521-
// Verify errors are prioritized over warnings
521+
// Verify exactly 2 errors are included (prioritized over warnings)
522522
expect(result).toContain("Error in file1")
523523
expect(result).toContain("Error in file2")
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-
}
524+
// Warnings and info should not be included
525+
expect(result).not.toContain("Warning in file1")
526+
expect(result).not.toContain("Warning in file2")
527+
expect(result).not.toContain("Info in file2")
528+
529+
// Verify the limit message is included
530+
expect(result).toContain("3 more problems omitted to prevent context overflow")
534531
})
535532

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

src/integrations/diagnostics/index.ts

Lines changed: 8 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -86,12 +86,8 @@ 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 use size-based limiting instead of count-based
89+
// If we have a limit, use count-based limiting
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
9591
let includedCount = 0
9692
let totalCount = 0
9793

@@ -112,10 +108,15 @@ export async function diagnosticsToProblemsString(
112108
return a.diagnostic.range.start.line - b.diagnostic.range.start.line
113109
})
114110

115-
// Process diagnostics and track size
111+
// Process diagnostics up to the count limit
116112
const includedDiagnostics: typeof allDiagnostics = []
117113
for (const item of allDiagnostics) {
118-
// Format the diagnostic to calculate its size
114+
// Stop if we've reached the count limit
115+
if (includedCount >= maxDiagnosticMessages) {
116+
break
117+
}
118+
119+
// Format the diagnostic
119120
let label: string
120121
switch (item.diagnostic.severity) {
121122
case vscode.DiagnosticSeverity.Error:
@@ -156,15 +157,8 @@ export async function diagnosticsToProblemsString(
156157
diagnosticText = `\n- [${source}${label}] ${line} | (unavailable) : ${item.diagnostic.message}`
157158
}
158159

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-
165160
item.formattedText = diagnosticText
166161
includedDiagnostics.push(item)
167-
currentSize += diagnosticSize
168162
includedCount++
169163
}
170164

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -495,7 +495,7 @@
495495
},
496496
"maxMessages": {
497497
"label": "Maximum diagnostic messages",
498-
"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.",
498+
"description": "Limits the number of diagnostic messages (errors, warnings) included in the context. When set, only this many diagnostics will be shown, prioritizing errors over warnings. Set to 0 for unlimited diagnostics.",
499499
"resetTooltip": "Reset to default value (50)",
500500
"unlimitedLabel": "Unlimited"
501501
}

0 commit comments

Comments
 (0)