fix(lsp): coalesce diagnostic bursts (#1861)

* fix(lsp): coalesce diagnostic bursts

* fix(lsp): tighten diagnostic debounce coverage

* test(lsp): share zero-diagnostic log assertion

* test(lsp): isolate diagnostic attachment debounce coverage
This commit is contained in:
Bogdan
2026-07-07 11:10:03 +08:00
committed by GitHub
parent fb40d49e68
commit 11f4661ea9
7 changed files with 623 additions and 66 deletions
+276 -9
View File
@@ -47,12 +47,22 @@ function diagnosticCount(files: DiagnosticFile[]): number {
return files.reduce((sum, file) => sum + file.diagnostics.length, 0)
}
function checkWithDebounce(now: number) {
return registry.checkForLSPDiagnostics({ now, respectDebounce: true })
}
function deliveryLogs(): string[] {
return debugMessages.filter(message =>
message.startsWith('LSP Diagnostics: Delivering '),
)
}
function expectNoZeroDiagnosticDeliveryLog(): void {
expect(
deliveryLogs().some(message => message.includes(' with 0 diagnostic(s) ')),
).toBe(false)
}
describe('LSPDiagnosticRegistry storm control', () => {
beforeEach(() => {
registry.resetAllLSPDiagnosticState()
@@ -74,6 +84,256 @@ describe('LSPDiagnosticRegistry storm control', () => {
])
})
test('coalesces repeated same-file burst snapshots into one stable delivery', () => {
registry.registerPendingLSPDiagnostic({
serverName: 'typescript',
files: [diagnosticFile('/repo/a.ts', ['stale diagnostic'])],
timestamp: 1_000,
})
registry.registerPendingLSPDiagnostic({
serverName: 'typescript',
files: [diagnosticFile('/repo/a.ts', ['final diagnostic'])],
timestamp: 1_100,
})
expect(checkWithDebounce(1_150)).toEqual([])
const diagnosticSets = checkWithDebounce(1_400)
expect(diagnosticSets).toHaveLength(1)
expect(diagnosticSets[0]?.files).toEqual([
diagnosticFile('/repo/a.ts', ['final diagnostic']),
])
expect(registry.getPendingLSPDiagnosticCount()).toBe(0)
})
test('coalesces several files into a bounded deterministic stable delivery', () => {
registry.registerPendingLSPDiagnostic({
serverName: 'typescript',
files: [
...Array.from({ length: 32 }, (_, index) =>
diagnosticFile(`/repo/file-${index}.ts`, [`initial ${index}`]),
),
],
timestamp: 2_000,
})
registry.registerPendingLSPDiagnostic({
serverName: 'typescript',
files: [diagnosticFile('/repo/file-1.ts', ['latest file 1'])],
timestamp: 2_100,
})
const files = checkWithDebounce(2_400)[0]?.files ?? []
expect(diagnosticCount(files)).toBe(30)
expect(files.map(file => file.uri)).toEqual([
'/repo/file-0.ts',
'/repo/file-1.ts',
'/repo/file-2.ts',
'/repo/file-3.ts',
'/repo/file-4.ts',
'/repo/file-5.ts',
'/repo/file-6.ts',
'/repo/file-7.ts',
'/repo/file-8.ts',
'/repo/file-9.ts',
'/repo/file-10.ts',
'/repo/file-11.ts',
'/repo/file-12.ts',
'/repo/file-13.ts',
'/repo/file-14.ts',
'/repo/file-15.ts',
'/repo/file-16.ts',
'/repo/file-17.ts',
'/repo/file-18.ts',
'/repo/file-19.ts',
'/repo/file-20.ts',
'/repo/file-21.ts',
'/repo/file-22.ts',
'/repo/file-23.ts',
'/repo/file-24.ts',
'/repo/file-25.ts',
'/repo/file-26.ts',
'/repo/file-27.ts',
'/repo/file-28.ts',
'/repo/file-29.ts',
])
expect(files[1]?.diagnostics[0]?.message).toBe('latest file 1')
})
test('delivers new diagnostics after the debounce window', () => {
registry.registerPendingLSPDiagnostic({
serverName: 'typescript',
files: [diagnosticFile('/repo/a.ts', ['first diagnostic'])],
timestamp: 3_000,
})
expect(checkWithDebounce(3_100)).toEqual([])
const firstDelivery = checkWithDebounce(3_300)
expect(firstDelivery[0]?.files).toEqual([
diagnosticFile('/repo/a.ts', ['first diagnostic']),
])
registry.registerPendingLSPDiagnostic({
serverName: 'typescript',
files: [diagnosticFile('/repo/a.ts', ['second diagnostic'])],
timestamp: 3_650,
})
expect(checkWithDebounce(3_700)).toEqual([])
const secondDelivery = checkWithDebounce(3_950)
expect(secondDelivery[0]?.files).toEqual([
diagnosticFile('/repo/a.ts', ['second diagnostic']),
])
})
test('flushes active bursts after the max coalescing delay', () => {
registry.registerPendingLSPDiagnostic({
serverName: 'typescript',
files: [diagnosticFile('/repo/a.ts', ['first diagnostic'])],
timestamp: 4_000,
})
registry.registerPendingLSPDiagnostic({
serverName: 'typescript',
files: [diagnosticFile('/repo/a.ts', ['latest diagnostic'])],
timestamp: 5_900,
})
const diagnosticSets = checkWithDebounce(6_100)
expect(diagnosticSets[0]?.files).toEqual([
diagnosticFile('/repo/a.ts', ['latest diagnostic']),
])
})
test('reports the next stable delivery delay for pending diagnostics', () => {
registry.registerPendingLSPDiagnostic({
serverName: 'typescript',
files: [diagnosticFile('/repo/a.ts', ['first diagnostic'])],
timestamp: 15_000,
})
expect(registry.getNextLSPDiagnosticDeliveryDelay(15_100)).toBe(150)
const diagnosticSets = checkWithDebounce(15_250)
expect(diagnosticSets[0]?.files).toEqual([
diagnosticFile('/repo/a.ts', ['first diagnostic']),
])
expect(registry.getNextLSPDiagnosticDeliveryDelay(15_260)).toBeNull()
})
test('clearing diagnostics updates state without producing attachments', () => {
const file = diagnosticFile('/repo/a.ts', ['transient diagnostic'])
registry.registerPendingLSPDiagnostic({
serverName: 'typescript',
files: [file],
timestamp: 7_000,
})
expect(checkWithDebounce(7_300)).toHaveLength(1)
registry.registerPendingLSPDiagnostic({
serverName: 'typescript',
files: [{ uri: '/repo/a.ts', diagnostics: [] }],
timestamp: 7_600,
})
expect(registry.getPendingLSPDiagnosticCount()).toBe(0)
expect(checkWithDebounce(7_900)).toEqual([])
registry.registerPendingLSPDiagnostic({
serverName: 'typescript',
files: [file],
timestamp: 8_200,
})
expect(checkWithDebounce(8_500)[0]?.files).toEqual([
file,
])
})
test('does not dedupe identical diagnostics across different servers', () => {
const sharedFile = diagnosticFile('/repo/a.ts', ['same text and range'])
registry.registerPendingLSPDiagnostic({
serverName: 'typescript',
files: [sharedFile],
timestamp: 9_000,
})
expect(checkWithDebounce(9_300)[0]?.files).toEqual([
sharedFile,
])
registry.registerPendingLSPDiagnostic({
serverName: 'eslint',
files: [sharedFile],
timestamp: 9_600,
})
expect(checkWithDebounce(9_900)[0]?.files).toEqual([
sharedFile,
])
})
test('does not let one active server burst block stable diagnostics from another server', () => {
const stableFile = diagnosticFile('/repo/stable.ts', ['stable diagnostic'])
const activeFile = diagnosticFile('/repo/active.ts', ['active diagnostic'])
registry.registerPendingLSPDiagnostic({
serverName: 'typescript',
files: [stableFile],
timestamp: 10_000,
})
registry.registerPendingLSPDiagnostic({
serverName: 'eslint',
files: [activeFile],
timestamp: 10_390,
})
const firstDelivery = checkWithDebounce(10_400)
expect(firstDelivery[0]?.serverName).toBe('typescript')
expect(firstDelivery[0]?.files).toEqual([stableFile])
expect(registry.getPendingLSPDiagnosticCount()).toBe(1)
const secondDelivery = checkWithDebounce(10_700)
expect(secondDelivery[0]?.serverName).toBe('eslint')
expect(secondDelivery[0]?.files).toEqual([activeFile])
expect(registry.getPendingLSPDiagnosticCount()).toBe(0)
})
test('does not let one same-server file hitting max delay flush a fresh file', () => {
const oldFile = diagnosticFile('/repo/old.ts', ['old diagnostic'])
const freshFile = diagnosticFile('/repo/fresh.ts', ['fresh diagnostic'])
registry.registerPendingLSPDiagnostic({
serverName: 'typescript',
files: [oldFile],
timestamp: 11_000,
})
registry.registerPendingLSPDiagnostic({
serverName: 'typescript',
files: [freshFile],
timestamp: 12_990,
})
const firstDelivery = checkWithDebounce(13_001)
expect(firstDelivery[0]?.serverName).toBe('typescript')
expect(firstDelivery[0]?.files).toEqual([oldFile])
expect(registry.getPendingLSPDiagnosticCount()).toBe(1)
expect(checkWithDebounce(13_100)).toEqual([])
const secondDelivery = checkWithDebounce(13_250)
expect(secondDelivery[0]?.files).toEqual([freshFile])
expect(registry.getPendingLSPDiagnosticCount()).toBe(0)
})
test('does not reattach unchanged diagnostics across turns', () => {
const file = diagnosticFile('/repo/a.ts', ['same missing import'])
@@ -91,9 +351,7 @@ describe('LSPDiagnosticRegistry storm control', () => {
})
expect(registry.checkForLSPDiagnostics()).toEqual([])
expect(deliveryLogs()).not.toContain(
'LSP Diagnostics: Delivering 1 file(s) with 0 diagnostic(s) from 1 server(s)',
)
expectNoZeroDiagnosticDeliveryLog()
})
test('returns no diagnostic set for raw empty diagnostic files', () => {
@@ -107,6 +365,19 @@ describe('LSPDiagnosticRegistry storm control', () => {
expect(deliveryLogs()).toEqual([])
})
test('clock injection does not enable debounce unless requested', () => {
const file = diagnosticFile('/repo/a.ts', ['clock-only diagnostic'])
registry.registerPendingLSPDiagnostic({
serverName: 'typescript',
files: [file],
timestamp: 14_000,
})
expect(registry.checkForLSPDiagnostics({ now: 14_001 })[0]?.files).toEqual([
file,
])
})
test('snapshots pending diagnostics without consuming delivery', () => {
const file = diagnosticFile('/repo/a.ts', ['same missing import'])
@@ -336,9 +607,7 @@ describe('LSPDiagnosticRegistry storm control', () => {
'lsp://diagnostic-storm/typescript',
])
expect(diagnosticCount(secondFiles)).toBe(1)
expect(deliveryLogs()).not.toContain(
'LSP Diagnostics: Delivering 1 file(s) with 0 diagnostic(s) from 1 server(s)',
)
expectNoZeroDiagnosticDeliveryLog()
})
test('returns compact storm summaries when volume limiting leaves only reserved summaries', () => {
@@ -365,9 +634,7 @@ describe('LSPDiagnosticRegistry storm control', () => {
.toBe(true)
expect(diagnosticCount(files)).toBe(30)
expect(registry.getPendingLSPDiagnosticCount()).toBe(0)
expect(deliveryLogs()).not.toContain(
'LSP Diagnostics: Delivering 30 file(s) with 0 diagnostic(s) from 30 server(s)',
)
expectNoZeroDiagnosticDeliveryLog()
})
test('reserves compact summaries for multiple storming servers before full diagnostics', () => {
+182 -32
View File
@@ -1,4 +1,3 @@
import { randomUUID } from 'crypto'
import { LRUCache } from 'lru-cache'
import * as path from 'path'
import { logForDebugging } from '../../utils/debug.js'
@@ -17,6 +16,8 @@ export type PendingLSPDiagnostic = {
files: DiagnosticFile[]
/** When diagnostic was received */
timestamp: number
/** When the current coalesced burst for this server/file began */
firstTimestamp: number
/** Whether attachment was already sent to conversation */
attachmentSent: boolean
}
@@ -56,6 +57,9 @@ 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
export const DIAGNOSTIC_DELIVERY_DEBOUNCE_MS = 250
const DIAGNOSTIC_DELIVERY_MAX_DELAY_MS = 2_000
const DIAGNOSTIC_COALESCING_LOG_THROTTLE_MS = 1_000
const MAX_STORM_TOP_FILES = 5
const STORM_SUMMARY_URI_PREFIX = 'lsp://diagnostic-storm'
@@ -99,11 +103,16 @@ type ServerDeliveryPlan = {
shouldSummarizeStorm: boolean
}
type CheckLSPDiagnosticsOptions = {
now?: number
respectDebounce?: boolean
}
// Global registry state
const pendingDiagnostics = new Map<string, PendingLSPDiagnostic>()
// Cross-turn deduplication: tracks diagnostics that have been delivered
// Maps file URI to a set of diagnostic keys (hash of message+severity+range)
// Cross-turn deduplication: tracks diagnostics that have been delivered.
// Maps server/file keys to diagnostic keys (hash of message+severity+range).
// Using LRUCache to prevent unbounded growth in long sessions
const deliveredDiagnostics = new LRUCache<string, Set<string>>({
max: MAX_DELIVERED_FILES,
@@ -114,6 +123,7 @@ const recentDiagnosticFileActivity = new LRUCache<string, number>({
})
const diagnosticWindows = new Map<string, DiagnosticWindowState>()
const coalescingLogTimestamps = new Map<string, number>()
function normalizeDiagnosticUri(uri: string): string {
for (const prefix of ['file://', '_claude_fs_right:', '_claude_fs_left:']) {
@@ -124,6 +134,10 @@ function normalizeDiagnosticUri(uri: string): string {
return uri
}
function createServerFileKey(serverName: string, uri: string): string {
return `${serverName}\0${normalizeDiagnosticUri(uri)}`
}
function displayFileForStormSummary(uri: string): string {
const normalized = normalizeDiagnosticUri(uri).replace(/\\/g, '/')
return path.basename(normalized) || normalized || '<unknown>'
@@ -331,6 +345,21 @@ function maybeLogStormSummary(
logForDebugging(formatStormSummary(serverName, stats))
}
function maybeLogCoalescedDelivery(serverName: string, now: number): void {
const lastLoggedAt = coalescingLogTimestamps.get(serverName)
if (
lastLoggedAt !== undefined &&
now - lastLoggedAt < DIAGNOSTIC_COALESCING_LOG_THROTTLE_MS
) {
return
}
coalescingLogTimestamps.set(serverName, now)
logForDebugging(
`LSP Diagnostics: Coalescing pending diagnostics from ${serverName} until burst is stable`,
)
}
/**
* 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.
@@ -358,17 +387,26 @@ export function registerPendingLSPDiagnostic({
files: DiagnosticFile[]
timestamp?: number
}): void {
// Use UUID for guaranteed uniqueness (handles rapid registrations)
const diagnosticId = randomUUID()
recordDiagnosticsReceived(serverName, files, timestamp)
pendingDiagnostics.set(diagnosticId, {
serverName,
files,
timestamp,
attachmentSent: false,
})
for (const file of files) {
const diagnosticId = createServerFileKey(serverName, file.uri)
const existingDiagnostic = pendingDiagnostics.get(diagnosticId)
if (file.diagnostics.length === 0) {
pendingDiagnostics.delete(diagnosticId)
deliveredDiagnostics.delete(diagnosticId)
continue
}
pendingDiagnostics.set(diagnosticId, {
serverName,
files: [file],
timestamp,
firstTimestamp: existingDiagnostic?.firstTimestamp ?? timestamp,
attachmentSent: false,
})
}
}
/**
@@ -422,6 +460,7 @@ function createDiagnosticKey(diag: {
* - Source and code (if present)
*/
function deduplicateDiagnosticFiles(
serverName: string,
allFiles: DiagnosticFile[],
options: { filterPreviouslyDelivered?: boolean } = {
filterPreviouslyDelivered: true,
@@ -449,7 +488,8 @@ function deduplicateDiagnosticFiles(
const previouslyDelivered =
options.filterPreviouslyDelivered === false
? new Set<string>()
: deliveredDiagnostics.get(normalizedUri) || new Set()
: deliveredDiagnostics.get(createServerFileKey(serverName, file.uri)) ||
new Set()
for (const diag of file.diagnostics) {
try {
@@ -558,13 +598,16 @@ function countDiagnostics(files: DiagnosticFile[]): number {
return files.reduce((total, file) => total + file.diagnostics.length, 0)
}
function trackDeliveredDiagnostics(files: DiagnosticFile[]): void {
function trackDeliveredDiagnostics(
serverName: string,
files: DiagnosticFile[],
): void {
for (const file of files) {
const normalizedUri = normalizeDiagnosticUri(file.uri)
if (!deliveredDiagnostics.has(normalizedUri)) {
deliveredDiagnostics.set(normalizedUri, new Set())
const deliveredKey = createServerFileKey(serverName, file.uri)
if (!deliveredDiagnostics.has(deliveredKey)) {
deliveredDiagnostics.set(deliveredKey, new Set())
}
const delivered = deliveredDiagnostics.get(normalizedUri)!
const delivered = deliveredDiagnostics.get(deliveredKey)!
for (const diag of file.diagnostics) {
try {
delivered.add(createDiagnosticKey(diag))
@@ -584,24 +627,78 @@ function trackDeliveredDiagnostics(files: DiagnosticFile[]): void {
}
}
function shouldDeferDiagnosticDelivery(
serverName: string,
diagnostic: PendingLSPDiagnostic,
now: number,
respectDebounce: boolean,
): boolean {
if (!respectDebounce) {
return false
}
const burstAge = now - diagnostic.firstTimestamp
const stableAge = now - diagnostic.timestamp
const shouldDefer =
stableAge < DIAGNOSTIC_DELIVERY_DEBOUNCE_MS &&
burstAge < DIAGNOSTIC_DELIVERY_MAX_DELAY_MS
if (shouldDefer) {
maybeLogCoalescedDelivery(serverName, now)
}
return shouldDefer
}
export function getNextLSPDiagnosticDeliveryDelay(
now = Date.now(),
): number | null {
let nextDelay: number | null = null
for (const diagnostic of pendingDiagnostics.values()) {
if (diagnostic.attachmentSent) {
continue
}
const burstAge = now - diagnostic.firstTimestamp
const stableAge = now - diagnostic.timestamp
if (
stableAge >= DIAGNOSTIC_DELIVERY_DEBOUNCE_MS ||
burstAge >= DIAGNOSTIC_DELIVERY_MAX_DELAY_MS
) {
return 0
}
const stableDelay = DIAGNOSTIC_DELIVERY_DEBOUNCE_MS - stableAge
const maxDelay = DIAGNOSTIC_DELIVERY_MAX_DELAY_MS - burstAge
const delay = Math.max(0, Math.min(stableDelay, maxDelay))
nextDelay = nextDelay === null ? delay : Math.min(nextDelay, delay)
}
return nextDelay
}
function collectUndeliveredDiagnosticFiles(): {
filesByServer: Map<string, DiagnosticFile[]>
diagnosticsToMark: PendingLSPDiagnostic[]
pendingByServer: Map<string, PendingLSPDiagnostic[]>
} {
const filesByServer = new Map<string, DiagnosticFile[]>()
const diagnosticsToMark: PendingLSPDiagnostic[] = []
const pendingByServer = new Map<string, PendingLSPDiagnostic[]>()
for (const diagnostic of pendingDiagnostics.values()) {
if (!diagnostic.attachmentSent) {
if (!filesByServer.has(diagnostic.serverName)) {
filesByServer.set(diagnostic.serverName, [])
}
if (!pendingByServer.has(diagnostic.serverName)) {
pendingByServer.set(diagnostic.serverName, [])
}
filesByServer.get(diagnostic.serverName)!.push(...diagnostic.files)
diagnosticsToMark.push(diagnostic)
pendingByServer.get(diagnostic.serverName)!.push(diagnostic)
}
}
return { filesByServer, diagnosticsToMark }
return { filesByServer, pendingByServer }
}
/**
@@ -626,7 +723,7 @@ export function getPendingLSPDiagnosticsSnapshot(): LSPDiagnosticSet[] {
for (const [serverName, files] of filesByServer) {
let deduplicationResult: DeduplicationResult
try {
deduplicationResult = deduplicateDiagnosticFiles(files, {
deduplicationResult = deduplicateDiagnosticFiles(serverName, files, {
filterPreviouslyDelivered: false,
})
} catch (error: unknown) {
@@ -674,19 +771,54 @@ function cloneDiagnosticFile(file: DiagnosticFile): DiagnosticFile {
*
* @returns Array of pending diagnostics ready for delivery (deduplicated)
*/
export function checkForLSPDiagnostics(): LSPDiagnosticSet[] {
const now = Date.now()
export function checkForLSPDiagnostics(
options: CheckLSPDiagnosticsOptions = {},
): LSPDiagnosticSet[] {
const now = options.now ?? Date.now()
const respectDebounce = options.respectDebounce ?? false
logForDebugging(
`LSP Diagnostics: Checking registry - ${pendingDiagnostics.size} pending`,
)
const { filesByServer, diagnosticsToMark } =
const { filesByServer, pendingByServer } =
collectUndeliveredDiagnosticFiles()
if (filesByServer.size === 0) {
return []
}
const deliverableFilesByServer = new Map<string, DiagnosticFile[]>()
const diagnosticsToMark: PendingLSPDiagnostic[] = []
for (const [serverName] of filesByServer) {
const pendingForServer = pendingByServer.get(serverName) ?? []
for (const pendingDiagnostic of pendingForServer) {
if (
shouldDeferDiagnosticDelivery(
serverName,
pendingDiagnostic,
now,
respectDebounce,
)
) {
continue
}
if (!deliverableFilesByServer.has(serverName)) {
deliverableFilesByServer.set(serverName, [])
}
deliverableFilesByServer
.get(serverName)!
.push(...pendingDiagnostic.files)
diagnosticsToMark.push(pendingDiagnostic)
}
}
if (deliverableFilesByServer.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.
@@ -706,10 +838,10 @@ export function checkForLSPDiagnostics(): LSPDiagnosticSet[] {
let deliveredCount = 0
const deliveryPlans: ServerDeliveryPlan[] = []
for (const [serverName, files] of filesByServer) {
for (const [serverName, files] of deliverableFilesByServer) {
let deduplicationResult: DeduplicationResult
try {
deduplicationResult = deduplicateDiagnosticFiles(files)
deduplicationResult = deduplicateDiagnosticFiles(serverName, files)
} catch (error: unknown) {
const err = toError(error)
logError(
@@ -780,14 +912,17 @@ export function checkForLSPDiagnostics(): LSPDiagnosticSet[] {
// 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)
trackDeliveredDiagnostics(serverName, 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) {
if (
remainingCapacity <= 0 &&
serverNames.length < deliverableFilesByServer.size
) {
logForDebugging(
`LSP Diagnostics: Global turn capacity exhausted after ${serverName}; later server diagnostics will be summarized or dropped`,
)
@@ -854,6 +989,7 @@ export function resetAllLSPDiagnosticState(): void {
deliveredDiagnostics.clear()
recentDiagnosticFileActivity.clear()
diagnosticWindows.clear()
coalescingLogTimestamps.clear()
}
/**
@@ -865,11 +1001,25 @@ export function resetAllLSPDiagnosticState(): void {
*/
export function clearDeliveredDiagnosticsForFile(fileUri: string): void {
const normalizedUri = normalizeDiagnosticUri(fileUri)
if (deliveredDiagnostics.has(normalizedUri)) {
let clearedDelivered = false
for (const key of deliveredDiagnostics.keys()) {
if (key.endsWith(`\0${normalizedUri}`)) {
deliveredDiagnostics.delete(key)
clearedDelivered = true
}
}
for (const key of pendingDiagnostics.keys()) {
if (key.endsWith(`\0${normalizedUri}`)) {
pendingDiagnostics.delete(key)
}
}
if (clearedDelivered) {
logForDebugging(
`LSP Diagnostics: Clearing delivered diagnostics for ${fileUri}`,
)
deliveredDiagnostics.delete(normalizedUri)
}
}
+5 -7
View File
@@ -185,15 +185,13 @@ export function registerLSPNotificationHandlers(
const diagnosticFiles =
formatDiagnosticsForAttachment(diagnosticParams)
// Only send notification if there are diagnostics
// Register empty diagnostic snapshots too: in LSP they mean the
// file was cleared, and the registry uses them to update state
// without producing an empty model attachment.
const firstFile = diagnosticFiles[0]
if (
!firstFile ||
diagnosticFiles.length === 0 ||
firstFile.diagnostics.length === 0
) {
if (!firstFile) {
logForDebugging(
`Skipping empty diagnostics from ${serverName} for ${diagnosticParams.uri}`,
`Skipping invalid diagnostics from ${serverName} for ${diagnosticParams.uri}`,
)
return
}
+2 -2
View File
@@ -490,10 +490,10 @@ export const FileEditTool = buildTool({
// 5. Write to disk
writeTextContent(absoluteFilePath, updatedFile, encoding, endings)
// Notify LSP servers about file modification (didChange) and save (didSave)
const lspManager = getLspServerManager()
if (lspManager) {
// Clear previously delivered diagnostics so new ones will be shown
// Clear previously delivered diagnostics after a successful write so
// new diagnostics will be shown.
clearDeliveredDiagnosticsForFile(`file://${absoluteFilePath}`)
// didChange: Content has been modified
lspManager
+2 -2
View File
@@ -304,10 +304,10 @@ export const FileWriteTool = buildTool({
// overwriting a CRLF file or when binaries in cwd poisoned the repo sample.
writeTextContent(fullFilePath, content, enc, 'LF')
// Notify LSP servers about file modification (didChange) and save (didSave)
const lspManager = getLspServerManager()
if (lspManager) {
// Clear previously delivered diagnostics so new ones will be shown
// Clear previously delivered diagnostics after a successful write so
// new diagnostics will be shown.
clearDeliveredDiagnosticsForFile(`file://${fullFilePath}`)
// didChange: Content has been modified
lspManager.changeFile(fullFilePath, content).catch((err: Error) => {
+95 -3
View File
@@ -13,11 +13,22 @@ const realLSPRegistry = await import(
)
let diagnosticSets: Array<{ serverName: string; files: DiagnosticFile[] }> = []
let nextDeliveryDelay: number | null = null
const checkForLSPDiagnosticsOptions: unknown[] = []
const getNextLSPDiagnosticDeliveryDelayCalls: Array<number | undefined> = []
const { DIAGNOSTIC_DELIVERY_DEBOUNCE_MS } = realLSPRegistry
const checkForLSPDiagnosticsMock = mock(() => diagnosticSets)
const checkForLSPDiagnosticsMock = mock((options?: unknown) => {
checkForLSPDiagnosticsOptions.push(options)
return diagnosticSets
})
const clearAllLSPDiagnosticsMock = mock(() => {
diagnosticSets = []
})
const getNextLSPDiagnosticDeliveryDelayMock = mock((now?: number) => {
getNextLSPDiagnosticDeliveryDelayCalls.push(now)
return nextDeliveryDelay
})
mock.module('./debug.js', () => ({
...realDebugModule,
@@ -30,9 +41,10 @@ mock.module('../services/lsp/LSPDiagnosticRegistry.js', () => ({
...realLSPRegistry,
checkForLSPDiagnostics: checkForLSPDiagnosticsMock,
clearAllLSPDiagnostics: clearAllLSPDiagnosticsMock,
getNextLSPDiagnosticDeliveryDelay: getNextLSPDiagnosticDeliveryDelayMock,
}))
const { getAttachmentMessages } = await import(
const { getAttachmentMessages, __test } = await import(
`./attachments.ts?test=${Date.now()}-${Math.random()}`
)
@@ -41,6 +53,24 @@ const SAVED_DISABLE_ATTACHMENTS = process.env.CLAUDE_CODE_DISABLE_ATTACHMENTS
type DiagnosticsAttachment = Extract<Attachment, { type: 'diagnostics' }>
function lspDiagnosticFile(message = 'stable diagnostic'): DiagnosticFile {
return {
uri: '/repo/a.ts',
diagnostics: [
{
message,
severity: 'Error',
range: {
start: { line: 0, character: 0 },
end: { line: 0, character: 1 },
},
source: 'typescript',
code: 'TS1000',
},
],
}
}
function makeToolUseContext(): ToolUseContext {
let inProgressToolUseIDs = new Set<string>()
@@ -110,9 +140,13 @@ describe('LSP diagnostic attachment filtering', () => {
delete process.env.CLAUDE_CODE_SIMPLE
delete process.env.CLAUDE_CODE_DISABLE_ATTACHMENTS
diagnosticSets = []
nextDeliveryDelay = null
checkForLSPDiagnosticsOptions.length = 0
getNextLSPDiagnosticDeliveryDelayCalls.length = 0
debugMessages.length = 0
checkForLSPDiagnosticsMock.mockClear()
clearAllLSPDiagnosticsMock.mockClear()
getNextLSPDiagnosticDeliveryDelayMock.mockClear()
})
afterEach(() => {
@@ -169,9 +203,67 @@ describe('LSP diagnostic attachment filtering', () => {
expect(attachments).toEqual([
{ type: 'diagnostics', files: [summaryFile], isNew: true },
])
expect(clearAllLSPDiagnosticsMock).toHaveBeenCalledTimes(1)
expect(clearAllLSPDiagnosticsMock).not.toHaveBeenCalled()
expect(debugMessages).toContain(
'LSP Diagnostics: Returning 1 diagnostic attachment(s)',
)
})
test('waits once for debounced diagnostics at the query boundary', async () => {
const file = lspDiagnosticFile()
let now = 100
const waits: number[] = []
nextDeliveryDelay = 150
const attachments = await __test.getLSPDiagnosticAttachments(
makeToolUseContext(),
{
now: () => now,
wait: async ms => {
waits.push(ms)
now += ms
diagnosticSets = [{ serverName: 'typescript', files: [file] }]
},
},
)
expect(checkForLSPDiagnosticsOptions).toEqual([
{ respectDebounce: true, now: 100 },
{ respectDebounce: true, now: 250 },
])
expect(getNextLSPDiagnosticDeliveryDelayCalls).toEqual([100])
expect(waits).toEqual([150])
expect(attachments).toEqual([
{
type: 'diagnostics',
files: [file],
isNew: true,
},
])
})
test('caps the query-boundary wait when the next ready delay is longer', async () => {
let now = 0
const waits: number[] = []
nextDeliveryDelay = DIAGNOSTIC_DELIVERY_DEBOUNCE_MS + 250
const attachments = await __test.getLSPDiagnosticAttachments(
makeToolUseContext(),
{
now: () => now,
wait: async ms => {
waits.push(ms)
now += ms
},
},
)
expect(checkForLSPDiagnosticsOptions).toEqual([
{ respectDebounce: true, now: 0 },
{ respectDebounce: true, now: DIAGNOSTIC_DELIVERY_DEBOUNCE_MS },
])
expect(getNextLSPDiagnosticDeliveryDelayCalls).toEqual([0])
expect(waits).toEqual([DIAGNOSTIC_DELIVERY_DEBOUNCE_MS])
expect(attachments).toEqual([])
})
})
+61 -11
View File
@@ -187,9 +187,12 @@ import {
} from './hooks/AsyncHookRegistry.js'
import {
checkForLSPDiagnostics,
clearAllLSPDiagnostics,
DIAGNOSTIC_DELIVERY_DEBOUNCE_MS,
getNextLSPDiagnosticDeliveryDelay,
type LSPDiagnosticSet,
} from '../services/lsp/LSPDiagnosticRegistry.js'
import { logForDebugging } from './debug.js'
import { sleep } from './sleep.js'
import {
extractTextContent,
getUserMessageText,
@@ -2932,8 +2935,57 @@ async function getDiagnosticAttachments(
* Get LSP diagnostic attachments from passive LSP servers.
* Follows the AsyncHookRegistry pattern for consistent async attachment delivery.
*/
type LSPDiagnosticAttachmentDeps = {
now?: () => number
wait?: (ms: number, signal?: AbortSignal) => Promise<void>
}
const LSP_DIAGNOSTIC_ATTACHMENT_MAX_WAIT_MS = DIAGNOSTIC_DELIVERY_DEBOUNCE_MS
function getLSPDiagnosticCheckOptions(now?: () => number): {
respectDebounce: true
now?: number
} {
const currentTime = now?.()
return currentTime === undefined
? { respectDebounce: true }
: { respectDebounce: true, now: currentTime }
}
async function checkForStableLSPDiagnosticsAtQueryBoundary(
toolUseContext: ToolUseContext,
deps: LSPDiagnosticAttachmentDeps,
): Promise<LSPDiagnosticSet[]> {
const diagnosticSets = checkForLSPDiagnostics(
getLSPDiagnosticCheckOptions(deps.now),
)
if (diagnosticSets.length > 0) {
return diagnosticSets
}
const nextDelay = getNextLSPDiagnosticDeliveryDelay(deps.now?.())
if (nextDelay === null) {
return []
}
const waitMs = Math.min(nextDelay, LSP_DIAGNOSTIC_ATTACHMENT_MAX_WAIT_MS)
if (waitMs > 0) {
logForDebugging(
`LSP Diagnostics: Waiting ${waitMs}ms for pending diagnostics to stabilize`,
)
const wait =
deps.wait ??
((ms: number, signal?: AbortSignal) =>
sleep(ms, signal, { unref: true }))
await wait(waitMs, toolUseContext.abortController.signal)
}
return checkForLSPDiagnostics(getLSPDiagnosticCheckOptions(deps.now))
}
async function getLSPDiagnosticAttachments(
toolUseContext: ToolUseContext,
deps: LSPDiagnosticAttachmentDeps = {},
): Promise<Attachment[]> {
// LSP diagnostics are only useful if the agent has the Bash tool to act on them
if (
@@ -2945,7 +2997,10 @@ async function getLSPDiagnosticAttachments(
logForDebugging('LSP Diagnostics: getLSPDiagnosticAttachments called')
try {
const diagnosticSets = checkForLSPDiagnostics()
const diagnosticSets = await checkForStableLSPDiagnosticsAtQueryBoundary(
toolUseContext,
deps,
)
if (diagnosticSets.length === 0) {
return []
@@ -2981,15 +3036,6 @@ async function getLSPDiagnosticAttachments(
isNew: true,
}))
// Clear delivered diagnostics from registry to prevent memory leak
// Follows same pattern as removeDeliveredAsyncHooks
if (diagnosticSets.length > 0) {
clearAllLSPDiagnostics()
logForDebugging(
`LSP Diagnostics: Cleared ${diagnosticSets.length} delivered diagnostic(s) from registry`,
)
}
logForDebugging(
`LSP Diagnostics: Returning ${attachments.length} diagnostic attachment(s)`,
)
@@ -3005,6 +3051,10 @@ async function getLSPDiagnosticAttachments(
}
}
export const __test = {
getLSPDiagnosticAttachments,
}
export async function* getAttachmentMessages(
input: string | null,
toolUseContext: ToolUseContext,