diff --git a/src/services/lsp/LSPDiagnosticRegistry.test.ts b/src/services/lsp/LSPDiagnosticRegistry.test.ts new file mode 100644 index 000000000..2ffca4587 --- /dev/null +++ b/src/services/lsp/LSPDiagnosticRegistry.test.ts @@ -0,0 +1,253 @@ +import { beforeEach, describe, expect, mock, test } from 'bun:test' +import type { Diagnostic, DiagnosticFile } from '../diagnosticTracking.js' + +const debugMessages: string[] = [] + +const realDebugModule = await import( + `../../utils/debug.js?real=${Date.now()}-${Math.random()}`, +) + +mock.module('../../utils/debug.js', () => ({ + ...realDebugModule, + logForDebugging: mock((message: string) => { + debugMessages.push(message) + }), +})) +// Other tests mock slowOperations process-wide; restore the real serializer so +// diagnostic keys keep message/range/code entropy under full-suite ordering. +mock.module('../../utils/slowOperations.js', () => ({ + jsonStringify: JSON.stringify, +})) + +const registry = await import( + `./LSPDiagnosticRegistry.ts?test=${Date.now()}-${Math.random()}` +) + +function diagnostic(message: string, line = 0): Diagnostic { + return { + message, + severity: 'Error', + range: { + start: { line, character: 0 }, + end: { line, character: 1 }, + }, + source: 'typescript', + code: `TS${line}`, + } +} + +function diagnosticFile(uri: string, messages: string[]): DiagnosticFile { + return { + uri, + diagnostics: messages.map((message, index) => diagnostic(message, index)), + } +} + +function diagnosticCount(files: DiagnosticFile[]): number { + return files.reduce((sum, file) => sum + file.diagnostics.length, 0) +} + +describe('LSPDiagnosticRegistry storm control', () => { + beforeEach(() => { + registry.resetAllLSPDiagnosticState() + debugMessages.length = 0 + }) + + test('dedupes repeated identical diagnostics before delivery', () => { + const repeated = diagnostic('same missing import') + registry.registerPendingLSPDiagnostic({ + serverName: 'typescript', + files: [{ uri: '/repo/a.ts', diagnostics: [repeated, repeated] }], + }) + + const diagnosticSets = registry.checkForLSPDiagnostics() + + expect(diagnosticSets).toHaveLength(1) + expect(diagnosticSets[0]?.files).toEqual([ + { uri: '/repo/a.ts', diagnostics: [repeated] }, + ]) + }) + + test('does not reattach unchanged diagnostics across turns', () => { + const file = diagnosticFile('/repo/a.ts', ['same missing import']) + + registry.registerPendingLSPDiagnostic({ + serverName: 'typescript', + files: [file], + }) + const firstDiagnosticSets = registry.checkForLSPDiagnostics() + expect(firstDiagnosticSets).toHaveLength(1) + expect(diagnosticCount(firstDiagnosticSets[0]!.files)).toBe(1) + + registry.registerPendingLSPDiagnostic({ + serverName: 'typescript', + files: [file], + }) + + expect(registry.checkForLSPDiagnostics()).toEqual([]) + }) + + test('allows edited files to resend diagnostics when cleared by file URI', () => { + const file = diagnosticFile('/repo/a.ts', ['same missing import']) + + registry.registerPendingLSPDiagnostic({ + serverName: 'typescript', + files: [file], + }) + const firstDiagnosticSets = registry.checkForLSPDiagnostics() + expect(firstDiagnosticSets).toHaveLength(1) + expect(diagnosticCount(firstDiagnosticSets[0]!.files)).toBe(1) + + // Intentionally clear by file:// URI while diagnostics use a plain path; + // both forms must normalize to the same delivered-diagnostic key. + registry.clearDeliveredDiagnosticsForFile('file:///repo/a.ts') + registry.registerPendingLSPDiagnostic({ + serverName: 'typescript', + files: [file], + }) + + const secondDiagnosticSets = registry.checkForLSPDiagnostics() + expect(secondDiagnosticSets).toHaveLength(1) + expect(diagnosticCount(secondDiagnosticSets[0]!.files)).toBe(1) + }) + + test('enforces per-file and per-turn diagnostic caps', () => { + registry.registerPendingLSPDiagnostic({ + serverName: 'typescript', + files: [ + diagnosticFile( + '/repo/crowded.ts', + Array.from({ length: 12 }, (_, index) => `crowded ${index}`), + ), + ...Array.from({ length: 25 }, (_, index) => + diagnosticFile(`/repo/file-${index}.ts`, [`other ${index}`]), + ), + ], + }) + + const files = registry.checkForLSPDiagnostics()[0]?.files ?? [] + + expect(diagnosticCount(files)).toBe(30) + expect( + files.find(file => file.uri === '/repo/crowded.ts')?.diagnostics.length, + ).toBe(10) + }) + + test('preserves recently active file diagnostics when total turn cap is exceeded', () => { + registry.recordLSPDiagnosticFileActivity('/repo/recent.ts') + registry.registerPendingLSPDiagnostic({ + serverName: 'typescript', + files: [ + ...Array.from({ length: 30 }, (_, index) => + diagnosticFile(`/repo/old-${index}.ts`, [`old ${index}`]), + ), + diagnosticFile('/repo/recent.ts', ['recent file should survive']), + ], + }) + + const files = registry.checkForLSPDiagnostics()[0]?.files ?? [] + + expect(diagnosticCount(files)).toBe(30) + expect(files.some(file => file.uri === '/repo/recent.ts')).toBe(true) + }) + + test('emits one compact storm summary with rolling top files and no diagnostic text', () => { + const firstStormFile = diagnosticFile( + '/home/alice/project/src/noisy-a.ts', + Array.from( + { length: 120 }, + (_, index) => `do not leak raw diagnostic text A ${index}`, + ), + ) + const secondStormFile = diagnosticFile( + '/home/alice/project/src/noisy-b.ts', + Array.from( + { length: 90 }, + (_, index) => `do not leak raw diagnostic text B ${index}`, + ), + ) + + registry.registerPendingLSPDiagnostic({ + serverName: 'typescript', + files: [firstStormFile, secondStormFile], + }) + + const files = registry.checkForLSPDiagnostics()[0]?.files ?? [] + const stormSummary = files.find(file => + file.uri.startsWith('lsp://diagnostic-storm/typescript'), + ) + const stormLogs = debugMessages.filter(message => + message.startsWith('LSP diagnostic storm: server=typescript'), + ) + + expect(diagnosticCount(files)).toBeLessThanOrEqual(30) + expect(stormSummary?.diagnostics).toHaveLength(1) + expect(stormSummary?.diagnostics[0]?.message).toContain('raw=210') + expect(stormSummary?.diagnostics[0]?.message).toContain('dropped=') + expect(stormSummary?.diagnostics[0]?.message).toContain('delivered=') + expect(stormSummary?.diagnostics[0]?.message).toContain( + 'topFiles=[noisy-a.ts:120, noisy-b.ts:90]', + ) + expect(stormSummary?.diagnostics[0]?.message).not.toContain( + 'do not leak raw diagnostic text', + ) + expect(stormLogs).toHaveLength(1) + }) + + test('does not trickle capped storm diagnostics into later turns', () => { + const stormFile = diagnosticFile( + '/repo/noisy.ts', + Array.from({ length: 210 }, (_, index) => `storm diagnostic ${index}`), + ) + + registry.registerPendingLSPDiagnostic({ + serverName: 'typescript', + files: [stormFile], + }) + const firstFiles = registry.checkForLSPDiagnostics()[0]?.files ?? [] + const firstRegularFile = firstFiles.find(file => file.uri === stormFile.uri) + + expect(firstRegularFile?.diagnostics.map(diag => diag.code)).toEqual( + Array.from({ length: 10 }, (_, index) => `TS${index}`), + ) + + registry.registerPendingLSPDiagnostic({ + serverName: 'typescript', + files: [stormFile], + }) + const secondFiles = registry.checkForLSPDiagnostics()[0]?.files ?? [] + + expect(secondFiles.map(file => file.uri)).toEqual([ + 'lsp://diagnostic-storm/typescript', + ]) + }) + + test('reserves compact summaries for multiple storming servers before full diagnostics', () => { + registry.registerPendingLSPDiagnostic({ + serverName: 'typescript', + files: Array.from({ length: 220 }, (_, index) => + diagnosticFile(`/repo/typescript-${index}.ts`, [ + `typescript storm ${index}`, + ]), + ), + }) + registry.registerPendingLSPDiagnostic({ + serverName: 'eslint', + files: [ + diagnosticFile( + '/repo/eslint.ts', + Array.from({ length: 220 }, (_, index) => `eslint storm ${index}`), + ), + ], + }) + + const files = registry.checkForLSPDiagnostics()[0]?.files ?? [] + const summaryUris = files + .filter(file => file.uri.startsWith('lsp://diagnostic-storm/')) + .map(file => file.uri) + + expect(diagnosticCount(files)).toBeLessThanOrEqual(30) + expect(summaryUris).toContain('lsp://diagnostic-storm/typescript') + expect(summaryUris).toContain('lsp://diagnostic-storm/eslint') + }) +}) diff --git a/src/services/lsp/LSPDiagnosticRegistry.ts b/src/services/lsp/LSPDiagnosticRegistry.ts index 65330b764..e605b696b 100644 --- a/src/services/lsp/LSPDiagnosticRegistry.ts +++ b/src/services/lsp/LSPDiagnosticRegistry.ts @@ -1,5 +1,6 @@ import { randomUUID } from 'crypto' import { LRUCache } from 'lru-cache' +import * as path from 'path' import { logForDebugging } from '../../utils/debug.js' import { toError } from '../../utils/errors.js' import { logError } from '../../utils/log.js' @@ -44,6 +45,54 @@ const MAX_TOTAL_DIAGNOSTICS = 30 // Max files to track for deduplication - prevents unbounded memory growth const MAX_DELIVERED_FILES = 500 +const MAX_RECENT_FILES = 500 + +const DIAGNOSTIC_STORM_WINDOW_MS = 60_000 +const DIAGNOSTIC_STORM_RAW_THRESHOLD = 200 +const DIAGNOSTIC_STORM_LOG_THROTTLE_MS = 60_000 +const RECENT_FILE_PRIORITY_WINDOW_MS = 5 * 60_000 +const MAX_STORM_TOP_FILES = 5 +const STORM_SUMMARY_URI_PREFIX = 'lsp://diagnostic-storm' + +type DiagnosticWindowEvent = { + timestamp: number + rawCount: number + duplicateCount: number + droppedCount: number + deliveredCount: number + fileCounts: Map +} + +type DiagnosticWindowState = { + events: DiagnosticWindowEvent[] + lastStormSummaryLoggedAt: number | undefined +} + +type RollingDiagnosticStats = { + rawCount: number + duplicateCount: number + droppedCount: number + deliveredCount: number + topFiles: Array<{ uri: string; count: number }> +} + +type DeduplicationResult = { + files: DiagnosticFile[] + duplicateCount: number +} + +type LimitResult = { + files: DiagnosticFile[] + droppedCount: number + deliveredCount: number +} + +type ServerDeliveryPlan = { + serverName: string + deduplicationResult: DeduplicationResult + prioritizedFiles: DiagnosticFile[] + shouldSummarizeStorm: boolean +} // Global registry state const pendingDiagnostics = new Map() @@ -55,6 +104,239 @@ const deliveredDiagnostics = new LRUCache>({ max: MAX_DELIVERED_FILES, }) +const recentDiagnosticFileActivity = new LRUCache({ + max: MAX_RECENT_FILES, +}) + +const diagnosticWindows = new Map() + +function normalizeDiagnosticUri(uri: string): string { + for (const prefix of ['file://', '_claude_fs_right:', '_claude_fs_left:']) { + if (uri.startsWith(prefix)) { + return uri.slice(prefix.length) + } + } + return uri +} + +function displayFileForStormSummary(uri: string): string { + const normalized = normalizeDiagnosticUri(uri).replace(/\\/g, '/') + return path.basename(normalized) || normalized || '' +} + +function getDiagnosticWindowState(serverName: string): DiagnosticWindowState { + let state = diagnosticWindows.get(serverName) + if (!state) { + state = { events: [], lastStormSummaryLoggedAt: undefined } + diagnosticWindows.set(serverName, state) + } + return state +} + +function pruneDiagnosticWindow(serverName: string, now: number): void { + const state = diagnosticWindows.get(serverName) + if (!state) { + return + } + + state.events = state.events.filter( + event => now - event.timestamp <= DIAGNOSTIC_STORM_WINDOW_MS, + ) + + if ( + state.events.length === 0 && + (state.lastStormSummaryLoggedAt === undefined || + now - state.lastStormSummaryLoggedAt > DIAGNOSTIC_STORM_LOG_THROTTLE_MS) + ) { + diagnosticWindows.delete(serverName) + } +} + +function recordDiagnosticWindowEvent( + serverName: string, + event: DiagnosticWindowEvent, +): void { + const state = getDiagnosticWindowState(serverName) + state.events.push(event) + pruneDiagnosticWindow(serverName, event.timestamp) +} + +function recordDiagnosticsReceived( + serverName: string, + files: DiagnosticFile[], + now: number, +): void { + const fileCounts = new Map() + let rawCount = 0 + + for (const file of files) { + const count = file.diagnostics.length + if (count === 0) { + continue + } + rawCount += count + const normalizedUri = normalizeDiagnosticUri(file.uri) + fileCounts.set(normalizedUri, (fileCounts.get(normalizedUri) ?? 0) + count) + } + + if (rawCount === 0) { + return + } + + recordDiagnosticWindowEvent(serverName, { + timestamp: now, + rawCount, + duplicateCount: 0, + droppedCount: 0, + deliveredCount: 0, + fileCounts, + }) +} + +function recordDiagnosticsDelivery( + serverName: string, + stats: { + duplicateCount: number + droppedCount: number + deliveredCount: number + }, + now: number, +): void { + if ( + stats.duplicateCount === 0 && + stats.droppedCount === 0 && + stats.deliveredCount === 0 + ) { + return + } + + recordDiagnosticWindowEvent(serverName, { + timestamp: now, + rawCount: 0, + duplicateCount: stats.duplicateCount, + droppedCount: stats.droppedCount, + deliveredCount: stats.deliveredCount, + fileCounts: new Map(), + }) +} + +function getRollingDiagnosticStats( + serverName: string, + now: number, +): RollingDiagnosticStats { + pruneDiagnosticWindow(serverName, now) + const state = diagnosticWindows.get(serverName) + const fileCounts = new Map() + const stats: RollingDiagnosticStats = { + rawCount: 0, + duplicateCount: 0, + droppedCount: 0, + deliveredCount: 0, + topFiles: [], + } + + if (!state) { + return stats + } + + for (const event of state.events) { + stats.rawCount += event.rawCount + stats.duplicateCount += event.duplicateCount + stats.droppedCount += event.droppedCount + stats.deliveredCount += event.deliveredCount + + for (const [uri, count] of event.fileCounts) { + fileCounts.set(uri, (fileCounts.get(uri) ?? 0) + count) + } + } + + stats.topFiles = Array.from(fileCounts.entries()) + .map(([uri, count]) => ({ uri, count })) + .sort((a, b) => { + if (b.count !== a.count) { + return b.count - a.count + } + return displayFileForStormSummary(a.uri).localeCompare( + displayFileForStormSummary(b.uri), + ) + }) + .slice(0, MAX_STORM_TOP_FILES) + + return stats +} + +function shouldAttachStormSummary(stats: RollingDiagnosticStats): boolean { + return stats.rawCount > DIAGNOSTIC_STORM_RAW_THRESHOLD +} + +function formatStormSummary( + serverName: string, + stats: RollingDiagnosticStats, +): string { + const topFiles = + stats.topFiles.length === 0 + ? 'none' + : stats.topFiles + .map(file => `${displayFileForStormSummary(file.uri)}:${file.count}`) + .join(', ') + + return ( + `LSP diagnostic storm: server=${serverName} ` + + `raw=${stats.rawCount} duplicates=${stats.duplicateCount} ` + + `dropped=${stats.droppedCount} delivered=${stats.deliveredCount} ` + + `topFiles=[${topFiles}]` + ) +} + +function buildStormSummaryFile( + serverName: string, + stats: RollingDiagnosticStats, +): DiagnosticFile { + return { + uri: `${STORM_SUMMARY_URI_PREFIX}/${encodeURIComponent(serverName)}`, + diagnostics: [ + { + message: formatStormSummary(serverName, stats), + severity: 'Info', + range: { + start: { line: 0, character: 0 }, + end: { line: 0, character: 0 }, + }, + source: 'openclaude-lsp', + code: 'diagnostic-storm', + }, + ], + } +} + +function maybeLogStormSummary( + serverName: string, + stats: RollingDiagnosticStats, + now: number, +): void { + const state = getDiagnosticWindowState(serverName) + if ( + state.lastStormSummaryLoggedAt !== undefined && + now - state.lastStormSummaryLoggedAt < DIAGNOSTIC_STORM_LOG_THROTTLE_MS + ) { + return + } + + state.lastStormSummaryLoggedAt = now + logForDebugging(formatStormSummary(serverName, stats)) +} + +/** + * Record an LSP file interaction so diagnostics for recently opened or edited + * files are preserved first when a diagnostic burst exceeds the per-turn cap. + */ +export function recordLSPDiagnosticFileActivity( + fileUri: string, + timestamp = Date.now(), +): void { + recentDiagnosticFileActivity.set(normalizeDiagnosticUri(fileUri), timestamp) +} + /** * Register LSP diagnostics received from a server. * These will be delivered as attachments in the next query. @@ -65,21 +347,21 @@ const deliveredDiagnostics = new LRUCache>({ export function registerPendingLSPDiagnostic({ serverName, files, + timestamp = Date.now(), }: { serverName: string files: DiagnosticFile[] + timestamp?: number }): void { // Use UUID for guaranteed uniqueness (handles rapid registrations) const diagnosticId = randomUUID() - logForDebugging( - `LSP Diagnostics: Registering ${files.length} diagnostic file(s) from ${serverName} (ID: ${diagnosticId})`, - ) + recordDiagnosticsReceived(serverName, files, timestamp) pendingDiagnostics.set(diagnosticId, { serverName, files, - timestamp: Date.now(), + timestamp, attachmentSent: false, }) } @@ -135,22 +417,28 @@ function createDiagnosticKey(diag: { */ function deduplicateDiagnosticFiles( allFiles: DiagnosticFile[], -): DiagnosticFile[] { +): DeduplicationResult { // Group diagnostics by file URI const fileMap = new Map>() + const dedupedFileMap = new Map() const dedupedFiles: DiagnosticFile[] = [] + let duplicateCount = 0 for (const file of allFiles) { - if (!fileMap.has(file.uri)) { - fileMap.set(file.uri, new Set()) - dedupedFiles.push({ uri: file.uri, diagnostics: [] }) + const normalizedUri = normalizeDiagnosticUri(file.uri) + if (!fileMap.has(normalizedUri)) { + fileMap.set(normalizedUri, new Set()) + const dedupedFile = { uri: file.uri, diagnostics: [] } + dedupedFileMap.set(normalizedUri, dedupedFile) + dedupedFiles.push(dedupedFile) } - const seenDiagnostics = fileMap.get(file.uri)! - const dedupedFile = dedupedFiles.find(f => f.uri === file.uri)! + const seenDiagnostics = fileMap.get(normalizedUri)! + const dedupedFile = dedupedFileMap.get(normalizedUri)! // Get previously delivered diagnostics for this file (for cross-turn dedup) - const previouslyDelivered = deliveredDiagnostics.get(file.uri) || new Set() + const previouslyDelivered = + deliveredDiagnostics.get(normalizedUri) || new Set() for (const diag of file.diagnostics) { try { @@ -158,6 +446,7 @@ function deduplicateDiagnosticFiles( // Skip if already seen in this batch OR already delivered in previous turns if (seenDiagnostics.has(key) || previouslyDelivered.has(key)) { + duplicateCount++ continue } @@ -180,119 +469,87 @@ function deduplicateDiagnosticFiles( } // Filter out files with no diagnostics after deduplication - return dedupedFiles.filter(f => f.diagnostics.length > 0) + return { + files: dedupedFiles.filter(f => f.diagnostics.length > 0), + duplicateCount, + } } -/** - * Get all pending LSP diagnostics that haven't been delivered yet. - * Deduplicates diagnostics to prevent sending the same diagnostic multiple times. - * Marks diagnostics as sent to prevent duplicate delivery. - * - * @returns Array of pending diagnostics ready for delivery (deduplicated) - */ -export function checkForLSPDiagnostics(): Array<{ - serverName: string - files: DiagnosticFile[] -}> { - logForDebugging( - `LSP Diagnostics: Checking registry - ${pendingDiagnostics.size} pending`, - ) +function prioritizeDiagnosticFiles( + files: DiagnosticFile[], + now: number, +): DiagnosticFile[] { + return files + .map((file, index) => { + const lastActivity = recentDiagnosticFileActivity.get( + normalizeDiagnosticUri(file.uri), + ) + const isRecent = + lastActivity !== undefined && + now - lastActivity <= RECENT_FILE_PRIORITY_WINDOW_MS - // Collect all diagnostic files from all pending notifications - const allFiles: DiagnosticFile[] = [] - const serverNames = new Set() - const diagnosticsToMark: PendingLSPDiagnostic[] = [] + return { file, index, isRecent, lastActivity: lastActivity ?? 0 } + }) + .sort((a, b) => { + if (a.isRecent !== b.isRecent) { + return a.isRecent ? -1 : 1 + } + if (a.isRecent && b.isRecent && a.lastActivity !== b.lastActivity) { + return b.lastActivity - a.lastActivity + } + return a.index - b.index + }) + .map(item => item.file) +} - for (const diagnostic of pendingDiagnostics.values()) { - if (!diagnostic.attachmentSent) { - allFiles.push(...diagnostic.files) - serverNames.add(diagnostic.serverName) - diagnosticsToMark.push(diagnostic) - } - } +function limitDiagnosticFiles( + files: DiagnosticFile[], + capacity: number, +): LimitResult { + const limitedFiles: DiagnosticFile[] = [] + let remainingCapacity = Math.max(0, capacity) + let droppedCount = 0 + let deliveredCount = 0 - if (allFiles.length === 0) { - return [] - } - - // Deduplicate diagnostics across all files - let dedupedFiles: DiagnosticFile[] - try { - dedupedFiles = deduplicateDiagnosticFiles(allFiles) - } catch (error: unknown) { - const err = toError(error) - logError(new Error(`Failed to deduplicate LSP diagnostics: ${err.message}`)) - // Fall back to undedup'd files to avoid losing diagnostics - dedupedFiles = allFiles - } - - // Only mark as sent AFTER successful deduplication, then delete from map. - // Entries are tracked in deliveredDiagnostics LRU for dedup, so we don't - // need to keep them in pendingDiagnostics after delivery. - for (const diagnostic of diagnosticsToMark) { - diagnostic.attachmentSent = true - } - for (const [id, diagnostic] of pendingDiagnostics) { - if (diagnostic.attachmentSent) { - pendingDiagnostics.delete(id) - } - } - - const originalCount = allFiles.reduce( - (sum, f) => sum + f.diagnostics.length, - 0, - ) - const dedupedCount = dedupedFiles.reduce( - (sum, f) => sum + f.diagnostics.length, - 0, - ) - - if (originalCount > dedupedCount) { - logForDebugging( - `LSP Diagnostics: Deduplication removed ${originalCount - dedupedCount} duplicate diagnostic(s)`, - ) - } - - // Apply volume limiting: cap per file and total - let totalDiagnostics = 0 - let truncatedCount = 0 - for (const file of dedupedFiles) { - // Sort by severity (Error=1 < Warning=2 < Info=3 < Hint=4) to prioritize errors - file.diagnostics.sort( + for (const file of files) { + const sortedDiagnostics = [...file.diagnostics].sort( (a, b) => severityToNumber(a.severity) - severityToNumber(b.severity), ) - // Cap per file - if (file.diagnostics.length > MAX_DIAGNOSTICS_PER_FILE) { - truncatedCount += file.diagnostics.length - MAX_DIAGNOSTICS_PER_FILE - file.diagnostics = file.diagnostics.slice(0, MAX_DIAGNOSTICS_PER_FILE) + let diagnostics = sortedDiagnostics + if (diagnostics.length > MAX_DIAGNOSTICS_PER_FILE) { + droppedCount += diagnostics.length - MAX_DIAGNOSTICS_PER_FILE + diagnostics = diagnostics.slice(0, MAX_DIAGNOSTICS_PER_FILE) } - // Cap total - const remainingCapacity = MAX_TOTAL_DIAGNOSTICS - totalDiagnostics - if (file.diagnostics.length > remainingCapacity) { - truncatedCount += file.diagnostics.length - remainingCapacity - file.diagnostics = file.diagnostics.slice(0, remainingCapacity) + if (remainingCapacity <= 0) { + droppedCount += diagnostics.length + continue } - totalDiagnostics += file.diagnostics.length + if (diagnostics.length > remainingCapacity) { + droppedCount += diagnostics.length - remainingCapacity + diagnostics = diagnostics.slice(0, remainingCapacity) + } + + remainingCapacity -= diagnostics.length + deliveredCount += diagnostics.length + + if (diagnostics.length > 0) { + limitedFiles.push({ uri: file.uri, diagnostics }) + } } - // Filter out files that ended up with no diagnostics after limiting - dedupedFiles = dedupedFiles.filter(f => f.diagnostics.length > 0) + return { files: limitedFiles, droppedCount, deliveredCount } +} - if (truncatedCount > 0) { - logForDebugging( - `LSP Diagnostics: Volume limiting removed ${truncatedCount} diagnostic(s) (max ${MAX_DIAGNOSTICS_PER_FILE}/file, ${MAX_TOTAL_DIAGNOSTICS} total)`, - ) - } - - // Track delivered diagnostics for cross-turn deduplication - for (const file of dedupedFiles) { - if (!deliveredDiagnostics.has(file.uri)) { - deliveredDiagnostics.set(file.uri, new Set()) +function trackDeliveredDiagnostics(files: DiagnosticFile[]): void { + for (const file of files) { + const normalizedUri = normalizeDiagnosticUri(file.uri) + if (!deliveredDiagnostics.has(normalizedUri)) { + deliveredDiagnostics.set(normalizedUri, new Set()) } - const delivered = deliveredDiagnostics.get(file.uri)! + const delivered = deliveredDiagnostics.get(normalizedUri)! for (const diag of file.diagnostics) { try { delivered.add(createDiagnosticKey(diag)) @@ -310,29 +567,178 @@ export function checkForLSPDiagnostics(): Array<{ } } } +} - const finalCount = dedupedFiles.reduce( - (sum, f) => sum + f.diagnostics.length, - 0, +/** + * Get all pending LSP diagnostics that haven't been delivered yet. + * Deduplicates diagnostics to prevent sending the same diagnostic multiple times. + * Marks diagnostics as sent to prevent duplicate delivery. + * + * @returns Array of pending diagnostics ready for delivery (deduplicated) + */ +export function checkForLSPDiagnostics(): Array<{ + serverName: string + files: DiagnosticFile[] +}> { + const now = Date.now() + logForDebugging( + `LSP Diagnostics: Checking registry - ${pendingDiagnostics.size} pending`, ) + // Collect pending diagnostic files by server so storm stats remain per-server. + const filesByServer = new Map() + const diagnosticsToMark: PendingLSPDiagnostic[] = [] + + for (const diagnostic of pendingDiagnostics.values()) { + if (!diagnostic.attachmentSent) { + if (!filesByServer.has(diagnostic.serverName)) { + filesByServer.set(diagnostic.serverName, []) + } + filesByServer.get(diagnostic.serverName)!.push(...diagnostic.files) + diagnosticsToMark.push(diagnostic) + } + } + + if (filesByServer.size === 0) { + return [] + } + + // Only mark as sent AFTER successful deduplication, then delete from map. + // Entries are tracked in deliveredDiagnostics LRU for dedup, so we don't + // need to keep them in pendingDiagnostics after delivery. + for (const diagnostic of diagnosticsToMark) { + diagnostic.attachmentSent = true + } + for (const [id, diagnostic] of pendingDiagnostics) { + if (diagnostic.attachmentSent) { + pendingDiagnostics.delete(id) + } + } + + const serverNames: string[] = [] + const deliveredFiles: DiagnosticFile[] = [] + let duplicateCount = 0 + let droppedCount = 0 + let deliveredCount = 0 + const deliveryPlans: ServerDeliveryPlan[] = [] + + for (const [serverName, files] of filesByServer) { + let deduplicationResult: DeduplicationResult + try { + deduplicationResult = deduplicateDiagnosticFiles(files) + } catch (error: unknown) { + const err = toError(error) + logError( + new Error(`Failed to deduplicate LSP diagnostics: ${err.message}`), + ) + // Fall back to undedup'd files to avoid losing diagnostics. + deduplicationResult = { files, duplicateCount: 0 } + } + + const statsBeforeDelivery = getRollingDiagnosticStats(serverName, now) + const shouldSummarizeStorm = shouldAttachStormSummary(statsBeforeDelivery) + const prioritizedFiles = prioritizeDiagnosticFiles( + deduplicationResult.files, + now, + ) + + deliveryPlans.push({ + serverName, + deduplicationResult, + prioritizedFiles, + shouldSummarizeStorm, + }) + } + + const reservedStormSummaryCount = Math.min( + MAX_TOTAL_DIAGNOSTICS, + deliveryPlans.filter(plan => plan.shouldSummarizeStorm).length, + ) + let remainingCapacity = MAX_TOTAL_DIAGNOSTICS - reservedStormSummaryCount + let remainingStormSummarySlots = reservedStormSummaryCount + + // The total diagnostic cap is global for the turn. Reserve compact storm + // summary slots before allocating full diagnostics so one server cannot hide + // another storming server's summary by exhausting the payload budget first. + for (const plan of deliveryPlans) { + const { + serverName, + deduplicationResult, + prioritizedFiles, + shouldSummarizeStorm, + } = plan + serverNames.push(serverName) + + const summarySlotReserved = + shouldSummarizeStorm && remainingStormSummarySlots > 0 + if (summarySlotReserved) { + remainingStormSummarySlots-- + } + + const limitResult = limitDiagnosticFiles(prioritizedFiles, remainingCapacity) + + recordDiagnosticsDelivery( + serverName, + { + duplicateCount: deduplicationResult.duplicateCount, + droppedCount: limitResult.droppedCount, + deliveredCount: limitResult.deliveredCount, + }, + now, + ) + + const statsAfterDelivery = getRollingDiagnosticStats(serverName, now) + if (summarySlotReserved) { + deliveredFiles.push(buildStormSummaryFile(serverName, statsAfterDelivery)) + maybeLogStormSummary(serverName, statsAfterDelivery, now) + } + + // Volume caps intentionally drop diagnostics for the turn; account for the + // full deduplicated batch so unchanged storms cannot trickle old diagnostics + // into later turns one capped slice at a time. + trackDeliveredDiagnostics(deduplicationResult.files) + deliveredFiles.push(...limitResult.files) + remainingCapacity -= limitResult.deliveredCount + duplicateCount += deduplicationResult.duplicateCount + droppedCount += limitResult.droppedCount + deliveredCount += limitResult.deliveredCount + + if (remainingCapacity <= 0 && serverNames.length < filesByServer.size) { + logForDebugging( + `LSP Diagnostics: Global turn capacity exhausted after ${serverName}; later server diagnostics will be summarized or dropped`, + ) + } + } + // Return empty if no diagnostics to deliver (all filtered by deduplication) - if (finalCount === 0) { + if (deliveredFiles.length === 0) { logForDebugging( `LSP Diagnostics: No new diagnostics to deliver (all filtered by deduplication)`, ) return [] } + if (duplicateCount > 0) { + logForDebugging( + `LSP Diagnostics: Deduplication removed ${duplicateCount} duplicate diagnostic(s)`, + ) + } + + if (droppedCount > 0) { + logForDebugging( + `LSP Diagnostics: Volume limiting removed ${droppedCount} diagnostic(s) (max ${MAX_DIAGNOSTICS_PER_FILE}/file, ${MAX_TOTAL_DIAGNOSTICS} total)`, + ) + } + logForDebugging( - `LSP Diagnostics: Delivering ${dedupedFiles.length} file(s) with ${finalCount} diagnostic(s) from ${serverNames.size} server(s)`, + `LSP Diagnostics: Delivering ${deliveredFiles.length} file(s) with ${deliveredCount} diagnostic(s) from ${serverNames.length} server(s)`, ) // Return single result with all deduplicated diagnostics return [ { - serverName: Array.from(serverNames).join(', '), - files: dedupedFiles, + serverName: serverNames.join(', '), + files: deliveredFiles, }, ] } @@ -360,6 +766,8 @@ export function resetAllLSPDiagnosticState(): void { ) pendingDiagnostics.clear() deliveredDiagnostics.clear() + recentDiagnosticFileActivity.clear() + diagnosticWindows.clear() } /** @@ -370,11 +778,12 @@ export function resetAllLSPDiagnosticState(): void { * @param fileUri - URI of the file that was edited */ export function clearDeliveredDiagnosticsForFile(fileUri: string): void { - if (deliveredDiagnostics.has(fileUri)) { + const normalizedUri = normalizeDiagnosticUri(fileUri) + if (deliveredDiagnostics.has(normalizedUri)) { logForDebugging( `LSP Diagnostics: Clearing delivered diagnostics for ${fileUri}`, ) - deliveredDiagnostics.delete(fileUri) + deliveredDiagnostics.delete(normalizedUri) } } diff --git a/src/services/lsp/LSPServerManager.ts b/src/services/lsp/LSPServerManager.ts index cca207a06..aadf284c7 100644 --- a/src/services/lsp/LSPServerManager.ts +++ b/src/services/lsp/LSPServerManager.ts @@ -4,6 +4,7 @@ import { logForDebugging } from '../../utils/debug.js' import { errorMessage } from '../../utils/errors.js' import { logError } from '../../utils/log.js' import { getAllLspServers } from './config.js' +import { recordLSPDiagnosticFileActivity } from './LSPDiagnosticRegistry.js' import { createLSPServerInstance, type LSPServerInstance, @@ -271,7 +272,9 @@ export function createLSPServerManager(): LSPServerManager { const server = await ensureServerStarted(filePath) if (!server) return - const fileUri = pathToFileURL(path.resolve(filePath)).href + const resolvedFilePath = path.resolve(filePath) + const fileUri = pathToFileURL(resolvedFilePath).href + recordLSPDiagnosticFileActivity(resolvedFilePath) // Skip if already opened on this server if (openedFiles.get(fileUri) === server.name) { @@ -315,7 +318,8 @@ export function createLSPServerManager(): LSPServerManager { return openFile(filePath, content) } - const fileUri = pathToFileURL(path.resolve(filePath)).href + const resolvedFilePath = path.resolve(filePath) + const fileUri = pathToFileURL(resolvedFilePath).href // If file hasn't been opened on this server yet, open it first // LSP servers require didOpen before didChange @@ -323,6 +327,8 @@ export function createLSPServerManager(): LSPServerManager { return openFile(filePath, content) } + recordLSPDiagnosticFileActivity(resolvedFilePath) + try { await server.sendNotification('textDocument/didChange', { textDocument: { @@ -350,10 +356,13 @@ export function createLSPServerManager(): LSPServerManager { const server = getServerForFile(filePath) if (!server || server.state !== 'running') return + const resolvedFilePath = path.resolve(filePath) + recordLSPDiagnosticFileActivity(resolvedFilePath) + try { await server.sendNotification('textDocument/didSave', { textDocument: { - uri: pathToFileURL(path.resolve(filePath)).href, + uri: pathToFileURL(resolvedFilePath).href, }, }) logForDebugging(`LSP: Sent didSave for ${filePath}`) diff --git a/src/services/lsp/passiveFeedback.ts b/src/services/lsp/passiveFeedback.ts index 71098088d..f09707f05 100644 --- a/src/services/lsp/passiveFeedback.ts +++ b/src/services/lsp/passiveFeedback.ts @@ -161,9 +161,6 @@ export function registerLSPNotificationHandlers( serverInstance.onNotification( 'textDocument/publishDiagnostics', (params: unknown) => { - logForDebugging( - `[PASSIVE DIAGNOSTICS] Handler invoked for ${serverName}! Params type: ${typeof params}`, - ) try { // Validate params structure before casting if ( @@ -183,9 +180,6 @@ export function registerLSPNotificationHandlers( } const diagnosticParams = params as PublishDiagnosticsParams - logForDebugging( - `Received diagnostics from ${serverName}: ${diagnosticParams.diagnostics.length} diagnostic(s) for ${diagnosticParams.uri}`, - ) // Convert LSP diagnostics to Claude format (can throw on invalid URIs) const diagnosticFiles = @@ -212,10 +206,6 @@ export function registerLSPNotificationHandlers( files: diagnosticFiles, }) - logForDebugging( - `LSP Diagnostics: Registered ${diagnosticFiles.length} diagnostic file(s) from ${serverName} for async delivery`, - ) - // Success - reset failure counter for this server diagnosticFailures.delete(serverName) } catch (error) {