Using a fixed setTimeout can lead to flaky tests if the server takes longer than expected to process. A more robust approach would be to wait for the server to signal that it's ready for diagnostics to be pulled. The server does this by sending a workspace/diagnostic/refresh request.

You can create a promise that resolves when this request is received and await it instead of using setTimeout. This would make the test more reliable and potentially faster.

This pattern should be applied to the other new tests in this suite that use setTimeout for waiting.

The tests now use an event-driven approach instead of fixed setTimeout(500). They wait for the server's workspace/diagnostic/refresh notification via waitForDiagnosticRefresh(), making the tests more robust and faster.

Done. All tests now use waitForDiagnosticRefresh() which awaits the server's workspace/diagnostic/refresh notification instead of a fixed setTimeout(500). This is event-driven and resolves immediately when the server signals readiness.

Risk: You are relying on the internal format of ScriptInfo version strings (e.g., "SVC-1-123").

• If the format changes (e.g., to a hash or UUID), this function returns 0.
• Bug: If it returns 0 for changed content (because parsing failed), the cache check currentVersionNum === cached.version (0 === 0) will falsely return DocumentDiagnosticReportKind.Unchanged.
• Fix: Store the full versionString in the cache and compare strings. The LSP version field in the report can be null or valid integer if available, but for our internal change detection, exact string equality is safer and more robust.

Instead of parsing the internal TypeScript ScriptInfo version string format (which could change), the code now stores and compares the full version string directly. This prevents false "unchanged" reports if parsing fails.

Fixed. The extractVersionNumber function is removed. The cache now stores and compares the full version string directly via scriptInfo.getLatestVersion(), using exact string equality. No parsing, no risk of false cache hits.

Observation: onWorkspaceDiagnostic iterates synchronously over all open files and calls getSemanticDiagnostics.

• Impact: On a large project with many open files, this could freeze the Node.js process for a significant time, blocking other LSP requests (like completion or hover).
• Fix: Consider awaiting setImmediate or a small delay between file processing to yield to the event loop, allowing other requests to be handled.

The onWorkspaceDiagnostic function now yields to the event loop between processing files using await yieldToEventLoop() (which uses setImmediate), preventing the server from freezing on large workspaces with many open files.

Fixed. onWorkspaceDiagnostic now awaits yieldToEventLoop() (which uses setImmediate) between processing each file, allowing hover/completion/etc. requests to be handled during workspace diagnostic computation.

Applied. Both console.timeEnd(semanticLabel) and console.time(suggestionLabel) are now in a single if (isDebugMode) block. Also renamed label → semanticLabel for consistency with suggestionLabel.

this appears unused. It should likely be hooked into onDidChangeConfiguration or shutdown, or removed if unnecessary.

Removed. Only clearDiagnosticCache(uri) remains — called when individual documents are closed. Since the cache is module-scoped and the server process terminates on shutdown, a full-cache clear is not needed.

It seems like there could/should be more code sharing between this and the onDocumentDiagnostic function?

Refactored. Extracted a shared computeDiagnosticsForFile(session, uri, previousResultId) function that handles caching, version checking, and diagnostic computation. Both onDocumentDiagnostic and onWorkspaceDiagnostic now delegate to it — onDocumentDiagnostic is just a one-liner wrapper.

From what I gather, the workspace/diagnostic LSP method is explicitly designed to provide diagnostics for the entire project (workspace), not just open files.

Introduced in LSP 3.17, this "pull" model allows the client to actively ask the server for diagnostics across the entire project.
Scope: Specifically designed to handle closed files and provide a project-wide overview.
Workflow: The client can request a "workspace" report. This is what allows you to see errors in a file you haven't even opened yet, similar to how a full build command would behave.

Some servers (like pyright or rust-analyzer) have specific settings to toggle "check all files" vs. "check open files only" because full-workspace scanning is resource-heavy.

Fixed. onWorkspaceDiagnostic now calls session.getProjectFileNames() which enumerates all source files across all configured Angular projects (via project.getFileNames(true, true) — excludes .d.ts library files and config files). Open files are processed first for priority, followed by remaining project files. This provides full project-wide diagnostic coverage.

Why not just use a counter that's incremented?

Changed. Now uses a simple monotonically increasing counter: let nextResultId = 1; function generateResultId(): string { return String(nextResultId++); }

Can we add a shared helper that does the time logging and the two types of diagnostic fetching and share it here and with sendPendingDiagnostics

Implemented. We now use a shared helper for diagnostics computation (getLspDiagnosticsForFile) and a shared file-iteration helper (forEachDiagnosticsFile) used by both pull and push (sendPendingDiagnostics).

Issue: scriptInfo.getLatestVersion() only reflects changes to the file content. It does not reflect changes to the project configuration (e.g. enabling strictTemplates or changing angularCompilerOptions). If the project configuration changes, the server triggers a refreshDiagnostics(). The client will then request diagnostics again (potentially sending the previousResultId). Since the file content hasn't changed, currentVersion === cached.version will be true, and the server will return Unchanged. However, the diagnostics should be recomputed because the compilation settings changed.

Fix: The diagnosticResultCache should be cleared (or version invalidated) whenever a global diagnostic refresh occurs (i.e. inside Session.refreshDiagnostics or when the project updates). You might need to expose a clearAllDiagnosticCache() function from handlers/diagnostics.ts and call it in Session.refreshDiagnostics().

Addressed, but via epochs (not full cache clear). We track project/global diagnostic epochs in cache entries and bump them on project/global invalidation. So config/project updates force Full recomputation even when file text version is unchanged. It is not exacly what You asked for so if I should use simpler approach for invalidating cache please say.

In the comment above, it states:

// For open/change triggers, use lightweight tiered invalidation:
// - external template edits: invalidate template + owning component files
// - TS edits: invalidate project diagnostics epoch

But this adds the project unconditionally, even when the changed file is a template file. Should this instead go in the else block below? Otherwise edits to template files invalidate the project diagnostics when changes there cannot affect additional files in the project.

impactedProjects.add() is now in the else block — template edits only clear cache for the template + owning component files via impactedFiles, and do NOT bump the project epoch.

The diagnosticResultCache Map in handlers/diagnostics.ts only purges entries when a document is explicitly closed (textDocument/didClose). If files in the workspace are deleted or renamed outside the boundaries of the VS Code editor buffers, their URIs persist in the cache indefinitely for the lifetime of the process. While the objects are tiny, it essentially constitutes a small memory leak in scenarios involving mass file moving/deleting. Consider implementing it as an LRU map or adding a mechanism to evict non-existent paths during project epoch bumps or watched-files events.

Aded stale entry eviction in onWorkspaceDiagnostic. After each workspace diagnostic sweep, entries for URIs no longer in any active project are removed. Combined with the didChangeWatchedFiles handler clearing entries on FileChangeType.Deleted, deleted/renamed files are evicted through multiple paths. The only scenario where the sweep doesn't run is if the client doesn't support workspace/diagnostic at all — but then there's nothing to cache because onWorkspaceDiagnostic is never called in the first place.

If You prefer LRU, please say so and will add it today.

I'm not sure diagnostics of the component file are affected by template changes unless this is actually due to the template being deleted?

Is this just because we don't have access to the dependency graph in the language service and you're just invalidating the entire project when a TS file changes as a result?