investigate: extension throws error during analysis — root cause findings

[moonolgerd]

Overview

This PR documents the investigation into why the Source Doc extension throws (or silently fails) when a user triggers any of the Explain commands (Explain Line, Explain Block, Explain File).

A full static-analysis pass was performed across every file in src/. Seven distinct issues were identified, ranging from a high-severity silent-cache-poison bug to a trivial double-semicolon typo.

---

Findings at a Glance

#	File	Severity	Description
1	explanationProvider.ts	🟠 Medium	Raw LanguageModelError codes surfaced to users with no actionable guidance
2	explanationProvider.ts	🔴 High	Empty model response is cached → feature silently broken for that snippet
3	codeLensProvider.ts	⚪ Trivial	Double semicolon CODE_RE;; (linter warning)
4	codeLensProvider.ts	🟡 Low	isNoiseLine noise-filter gap for user-added language IDs
5	extension.ts	🟠 Medium	languageRegistrations disposables leak on sourceDoc.languages config change
6	extension.ts	🟡 Low	explainFile shows confusing "0 / 0 done" on all-comment/blank files
7	codeLensProvider.ts	🟠 Medium	document.lineAt(0) called without lineCount > 0 guard — potential RangeError

---

Key Bug: Empty Response Cached (Issue #2)

// src/explanationProvider.ts
result = result.trim();
this.addToCache(key, result);  // ← written even when result === ''
return result;

When Copilot returns an empty body (content-filtered, quota exhausted, token cancelled mid-stream) the empty string '' is cached. The next identical request returns '' from cache and the user sees no ghost text and no error — it looks like the feature is permanently broken for that code snippet.

Proposed fix:

result = result.trim();
if (!result) {
    throw new Error('Copilot returned an empty explanation. Please try again.');
}
this.addToCache(key, result);

---

Key Bug: Disposable Leak (Issue #5)

// src/extension.ts
let languageRegistrations: vscode.Disposable[] = [];
// ... inside onDidChangeConfiguration:
languageRegistrations = [];
registerCodeLensProviders(context, codeLensProvider, languageRegistrations);
// ← languageRegistrations is NEVER added to context.subscriptions

Every time the user edits sourceDoc.languages, new CodeLensProvider registrations are pushed into the local array but never tracked by VS Code's disposal system. They accumulate until the window is closed.

---

Reproduction Steps (Issues #1 & #2)

1. Open any .ts / .py file.
2. Sign out of GitHub Copilot (or exhaust the free-tier quota).
3. Click any Explain CodeLens.
4. Observe: error notification with raw LanguageModelError code.
5. Sign back in → click the same lens → no explanation appears (empty string was cached in step 4).

---

Files Changed

• docs/analysis-error-investigation.md — full write-up with code snippets, suggested fixes, and a prioritised matrix for all seven issues.

---

Next Steps

• Fix issue fix: Implement analysis error investigation fixes #2 (empty-response cache poison) — highest priority
• Fix issue #5 (disposable leak)
• Fix issue #7 (empty-file RangeError)
• Improve error messaging for all LanguageModelError codes (issue investigate: extension throws error during analysis — root cause findings #1)
• Add early-exit UX for all-comment files (issue #6)
• Remove double-semicolon typo (issue fix: CodeLens registration leak, cancellation surfaced as error, LanguageModelError codes missing from messages #3)

[Copilot]

Pull request overview

Adds an investigation write-up documenting suspected root causes for Source Doc’s Explain commands failing, with proposed fixes and a prioritization matrix to guide follow-up code changes.

Changes:

• Adds a new investigation document summarizing 7 findings across explanationProvider.ts, codeLensProvider.ts, and extension.ts
• Includes suggested fixes and reproduction steps for the most impactful issues
• Provides a priority matrix to help sequence remediation work

[Copilot]

In section 1, the doc says a non-LanguageModelError propagates up uncaught to runExplain and is shown in the status bar. In the current implementation, runExplain catches all errors and calls vscode.window.showErrorMessage(...), not a status bar message. Please adjust this description to match the actual error handling path/UI.

Suggested change

	timeout), it propagates up uncaught to `runExplain`, which shows it in the
	status bar, but **the LRU cache is not written** — meaning every retry hits
	the model again instead of surfacing a friendlier retry prompt.
	timeout), `runExplain` catches it and surfaces the message via
	`vscode.window.showErrorMessage(...)`, but **the LRU cache is not written** —
	meaning every retry hits the model again instead of surfacing a friendlier,
	cached retry prompt.

[Copilot]

Section 4’s explanation/examples don’t match the current isNoiseLine implementation: the regex already includes --, so the examples “Lua --, Haskell --” aren’t actually missed. Also the heading mentions “misses // mid-regex”, but the snippet shown is anchored to the start of the trimmed line and does match // prefixes. Please revise this finding to reflect a concrete, reproducible gap (or remove it).

[Copilot]

Section 5 suggests “push the languageRegistrations array reference once so VS Code drains it on deactivation”. context.subscriptions only disposes items that implement Disposable; pushing an array won’t be disposed. If you want a single subscription, wrap the array in a Disposable (e.g., a disposer that iterates and disposes the current registrations) or use Disposable.from(...) when registering.

[Copilot]

Section 6 claims the progress UI shows/dismisses with "0 / 0 done" when there are no non-noise lines. In extension.ts, the progress title is Source Doc: explaining ${lines.length} lines… and the progress.report message is only set inside the per-line loop, so with 0 lines the user will more likely see “explaining 0 lines…” (briefly) rather than “0 / 0 done”. Please update the doc to describe the actual behavior you observe from this code path.

Suggested change

	notification is shown and dismissed with `"0 / 0 done"` which is confusing.
	notification is created with the title `Source Doc: explaining 0 lines…` and is dismissed without any per-line `"N / total done"` updates, which can look like a brief, confusing flicker.

[Copilot]

Section 7 motivates the document.lineAt(0) guard using “virtual/untitled documents”, but this CodeLens provider is registered with { scheme: 'file' }, so it won’t be invoked for untitled/virtual documents. If you want to keep this item, please reframe it as a general defensive guard (or cite a concrete case where a file-scheme TextDocument can have lineCount === 0).

Suggested change

	If an empty file is opened (`lineCount === 0`), calling `document.lineAt(0)`
	throws a `RangeError`. VS Code normally guarantees at least one line, but
	virtual/untitled documents and some language servers surface edge cases.
	`provideCodeLenses` assumes that `document.lineCount > 0` and calls
	`document.lineAt(0)` unconditionally. VS Code currently guarantees at least
	one line for normal `file`-scheme documents, so this should not occur in
	typical operation, but it still relies on that invariant — if it is ever
	violated (for example by an extension host or language feature bug that
	surfaces a `file` document with `lineCount === 0`), a `RangeError` would be
	thrown. A small `lineCount` check makes the code resilient to such edge
	cases at essentially zero cost.

[moonolgerd]

@copilot open a new pull request to apply changes based on the comments in this thread