[VSCodeSweeper MVP — test only] Fallback on "legacy" diff algorithm if "advanced" is too slow

[egamma]

⚠️ VSCodeSweeper MVP — test only. This draft PR was generated by an AI experiment
(vscodesweeper) inside a personal fork. It is
not intended for microsoft/vscode and is not a contribution proposal.

Upstream issue: microsoft#216953

Change summary

When the advanced diff algorithm times out on large files, VS Code shows no diff instead of falling back to the legacy algorithm.

Validation

Write a test in the diff provider factory service tests that mocks the advanced algorithm to return quitEarly: true, then asserts that the provider retries with legacy and returns a non-empty diff result.

---

Branch vscodesweeper/fix-216953 · generated fix · review loop runs separately (codex review).

[egamma]

🤖 codex review — round 1

VSCodeSweeper MVP codex cross-review of a copilot-generated fix (fork-only experiment).

The fallback behavior works for the happy path, but it bypasses the cache eviction limit and changes cancellation behavior for requests canceled during the retry. These are correctness/maintainability issues introduced by the patch.

Full review comments:

• [P2] Keep fallback cache entries under the size limit — src/vs/editor/browser/widget/diffEditor/diffProviderFactoryService.ts:157-159
  When a non-legacy diff quits early and the legacy retry succeeds, this early set/return bypasses the existing eviction block below. Repeated timed-out diffs for different URI pairs can therefore grow the static diffCache past its intended 10-entry cap, retaining potentially large diff results indefinitely.

• [P2] Preserve cancellation semantics after the legacy retry — src/vs/editor/browser/widget/diffEditor/diffProviderFactoryService.ts:156-160
  If the cancellation token is signaled while the legacy retry is awaiting, this block falls through and caches/returns the original timed-out result instead of returning the cancellation result used after the first worker call. This only happens when the advanced algorithm quits early, but it means a canceled diff request can still publish a stale cached result.

[egamma]

🔧 Round 1: pushed commits addressing the review above; re-reviewing.

[egamma]

🤖 codex review — round 2

VSCodeSweeper MVP codex cross-review of a copilot-generated fix (fork-only experiment).

The fallback can exceed the requested diff computation time budget by running another full-timeout computation after the first one timed out.

Review comment:

• [P2] Honor the diff timeout when retrying with legacy — src/vs/editor/browser/widget/diffEditor/diffProviderFactoryService.ts:156-156
  When the advanced diff consumes maxComputationTimeMs and returns quitEarly, this retry starts a second legacy computation with the same full timeout. For large inputs this can make a single computeDiff call take roughly twice the caller's requested budget, which defeats the timeout used to keep diffing responsive. Please either skip the fallback once the budget is exhausted or pass only the remaining time to the legacy call.

[egamma]

🔧 Round 2: pushed commits addressing the review above; re-reviewing.

[egamma]

⏹ Reached the 2-round review cap. Remaining items (if any) need a human.

[egamma]

⚠️ Round 1: codex review did not complete (Engine codex timed out after 3600s and was killed.). Stopping the loop.

[egamma]

🤖 codex review — round 1

VSCodeSweeper MVP codex cross-review of a copilot-generated fix (fork-only experiment).

The fallback logic mishandles the existing sentinel value for an infinite computation budget, causing the new fallback behavior to be skipped in that scenario.

Review comment:

• [P2] Preserve infinite diff budget for fallback — src/vs/editor/browser/widget/diffEditor/diffProviderFactoryService.ts:156-157
  When maxComputationTimeMs is 0, the diff computer treats that as an infinite budget, but this subtraction turns it into a negative remaining time after any elapsed work and skips the legacy fallback. In configurations that disable the timeout (or use advanced-external/wasm, which can still report quitEarly), a timed-out advanced result is returned instead of retrying with legacy as intended; keep 0 as 0 for the fallback rather than treating it as exhausted.

[egamma]

🔧 Round 1: pushed commits addressing the review above; re-reviewing.

[egamma]

⏹ Reached the 1-round review cap. Remaining items (if any) need a human.