fix(diagnostics): stop a superseded discardedReturn pass instead of r…

[geircodes]

fix(diagnostics): stop a superseded discardedReturn pass instead of running it to completion

Branch fix/validation-stale-pass-abort, targets version-1.0.4.

What happens

In a large module, member completion after a dot (obj., then obj.L, obj.Li) becomes slow — 1.9–3.4 s per keystroke on a 770 KB / 14,512-line file — while the same requests take 140–150 ms when nothing else is running. Bare-word completion on the same file stays fast. Instrumenting the server showed the member walk itself was unchanged (11–28 ms); the time went to sharing the single thread with background validation.

Root cause

validateTextDocument runs its async validators 500 ms after the last edit. It has a stale-version guard that discards the pass's answer when the document version has moved on — but the guard ran after all ten validators, so a superseded pass still did every bit of its work first.

The discardedReturn validator (ReturnValueDiagnostics.validateDiscardedReturnValues) resolves the receiver type of every dot-call in the document and enumerates each receiver class cross-file. Its memos are keyed by document version, so each pass during typing is cold: on the file above, 67 class enumerations and 98 receiver-type resolutions per pass. A cold pass ran 6.6–9.8 s with up to five superseded passes overlapping (about 1 s when running alone). Every await inside a completion request let one of those passes advance, which is why member completion — many await points — stretched from 24 ms to 2.5 s while bare-word completion, with almost none, did not.

Checking staleness only between validators does not help: discardedReturn is the second of ten and runs uninterrupted, so that check always fired after it had finished.

The fix

server/src/server.ts, in validateTextDocument:

const isStale = () => documents.get(document.uri)?.version !== startVersion;
const validatorThunks: [string, () => Promise<Diagnostic[]>][] = [
    ...
    ['discardedReturn', () => DiagnosticProvider.validateDiscardedReturnValues(tokens, document, memberLocator, getOpenDocumentContent, isStale)],
    ...
];
...
for (const [name, thunk] of validatorThunks) {
    if (isStale()) break;
    validatorResults.push(await timeIt(name, thunk()));
    ...

server/src/providers/DiagnosticProvider.ts forwards the new optional isStale?: () => boolean parameter, and server/src/providers/diagnostics/ReturnValueDiagnostics.ts checks it at the top of its per-line loop, at the time-slice yield that already exists there:

for (let lineIdx = 0; lineIdx < docLines.length; lineIdx++) {
    await timeSlice();
    if (isStale?.()) {
        perfLogger.perf("validateDiscardedReturnValues aborted (document changed)", { ... });
        return [];
    }

The pre-existing guard after the validators is unchanged and still discards the (now empty) result of a superseded pass.

Measured

With the in-loop check, superseded passes end in 42–526 ms instead of 6.6–9.8 s, only the pass for the final version runs all ten validators, and the same member-completion requests take 140–152 ms.

Testing

New server/src/test/ReturnValueDiagnostics.StaleAbort.test.ts (3 tests) on a fixture with four discarded obj.Method() calls and a MemberLocatorService whose receiver-type resolutions are counted:

• control (no callback): all four warnings are reported;
• stale from the start: the result is empty and the locator is never called;
• superseded mid-pass (the callback flips after the first resolution completes): the result is empty and no further resolution is started.

Without the fix the two stale-path tests fail — the callback is ignored and all four warnings come back — while the control test passes either way. With the fix the full suite is 3071 passing, 0 failing, 4 pending.

Scope

Only discardedReturn receives the callback; it is the validator whose per-pass cost made superseded passes matter. The other async validators are short enough (tens of ms, undeclaredVar and missingImpl a few hundred) that the between-validator check covers them. Two related costs are deliberately left for their own changes: the version-keyed memos that make every pass cold in the first place (a warm pass of the same validator measures ~0.5 s against ~1 s cold and 6–10 s under contention), and the ~0.55 s of synchronous validators that run at the start of every pass. Sibling change in the same investigation: fix/member-scan-use-loaded-text.