fix: don't report merged multi-reason content changes as unexpected (fixes #321636)

[vs-code-engineering]

Summary

ModelTracker in mainThreadDocuments.ts listens to ITextModel.onDidChangeContent and asserted that every event carries exactly one detailed edit reason (e.detailedReasonsChangeLengths.length !== 1 → onUnexpectedError). That assumption is false. IModelContentChangedEvent.detailedReasons is typed as a TextModelEditSource array, and DidChangeContentEmitter deliberately merges deferred content-change events, concatenating their detailedReasons / detailedReasonsChangeLengths arrays. When several model mutations are flushed inside a single begin/endDeferredEmit block — as happens during undo/redo — the fired event legitimately carries two or more reasons (telemetry shows unknown,applyEdits), so the assertion raises Error: Unexpected reasons: unknown,applyEdits into error telemetry even though nothing actually went wrong (the document still updates correctly). The fix removes the false-positive assertion and keeps forwarding the first reason, which is all the proposed TextDocumentChangeEvent.detailedReason API can express.

Fixes #321636
Recommended reviewer: @hediet

Culprit Commit

488d6df7955ca02076fd1da3fb8cbdf2a252a407 — "Implements edit source tracking through proposed API (#252430)" by Henning Dieterichs (@hediet), 2025-06-26.

This single commit introduced both sides of the contradiction:

• the reason-concatenating merge in src/vs/editor/common/textModelEvents.ts (_mergeChangeEvents → detailedReasons: a.detailedReasons.concat(b.detailedReasons)), which makes a multi-reason event a first-class, expected state; and
• the consumer assertion in src/vs/workbench/api/browser/mainThreadDocuments.ts (if (e.detailedReasonsChangeLengths.length !== 1) { onUnexpectedError(...) }), which declares that very same state illegal.

Code Flow

flowchart TD
    A["User triggers Undo (coreCommands.ts)"] --> B["TextModel.undo()"]
    B --> C["UndoRedoService.undo()"]
    C --> D["SingleModelEditStackElement.undo (editStack.ts:220)"]
    D --> E["TextModel._applyUndo (textModel.ts:1441)"]
    E --> F["_applyUndoRedoEdits: beginDeferredEmit (textModel.ts:1465)"]
    F --> G["_doApplyEdits with EditSources.applyEdits() (textModel.ts:1472)"]
    F --> H["second mutation in same block, e.g. EditSources.unknown via pushEditOperation (editStack.ts:428) or setEOL"]
    G --> I["DidChangeContentEmitter merges the deferred events"]
    H --> I
    I --> J["_mergeChangeEvents concatenates detailedReasons (textModelEvents.ts:590)"]
    J --> K["endDeferredEmit fires ONE event whose detailedReasons has length 2"]
    K --> L["ModelTracker.onDidChangeContent (mainThreadDocuments.ts:100)"]
    L --> M["length !== 1 -> onUnexpectedError: 'Unexpected reasons: unknown,applyEdits'"]

Loading

Affected Files

• src/vs/workbench/api/browser/mainThreadDocuments.ts — crash site and the file changed by this PR. ModelTracker's onDidChangeContent listener held the wrong single-reason assumption.
• src/vs/editor/common/textModelEvents.ts — _mergeChangeEvents (line ~590) concatenates detailedReasons; this is the mechanism that produces multi-reason events. Working as designed, not modified.
• src/vs/editor/common/model/textModel.ts — _applyUndoRedoEdits (line ~1465) opens one deferred-emit block that applies EditSources.applyEdits() and may also run setEOL. Producer, not modified.
• src/vs/editor/common/model/editStack.ts — pushEditOperation (line ~428) defaults its reason to EditSources.unknown({ name: 'pushEditOperation' }), the origin of the leading unknown reason. Not modified.

Repro Steps

1. Run a VS Code build. ModelTracker is attached to every text model mirrored to the extension host, so any synchronized document exercises this path; an extension using the proposed textDocumentChangeReason API makes the forwarded reason observable.
2. In a text document, perform an operation whose undo flushes more than one model mutation inside a single deferred-emit block — e.g. an edit combined with an end-of-line normalization, or a queued programmatic pushEditOperation (default unknown source) immediately followed by an undo that applies EditSources.applyEdits().
3. Trigger Undo.
4. Observe Error: Unexpected reasons: unknown,applyEdits reported through onUnexpectedError into error telemetry. The document content updates correctly; only the spurious error is logged.

Note: the exact mutation contributing the leading unknown reason depends on the active edit providers. The structural trigger is any deferred-emit block that flushes two differently-sourced mutations, which is why the reason pair can vary while the assertion fires identically.

How the Fix Works

Chosen approach (src/vs/workbench/api/browser/mainThreadDocuments.ts):

• Removed the if (e.detailedReasonsChangeLengths.length !== 1) { onUnexpectedError(...) } block. Its premise — a content-change event always has exactly one detailed reason — directly contradicts the contract the same feature established: detailedReasons is declared as an array and _mergeChangeEvents is purpose-built to concatenate those arrays when deferred emits merge. A merged, multi-reason event is therefore a valid value, not invalid data, so the correct place to fix is the consumer holding the wrong assumption — not a guard bolted onto a producer that is behaving exactly as designed (fix where the wrong assumption lives, not at a non-existent bad-data producer).
• This does not silence a real error. The onUnexpectedError call was a development-time "I don't expect this yet" marker, and the telemetry it produced is a false positive — the model update itself always succeeds. No logService.error or genuine diagnostic is removed, and this path was never a functional failure.
• Hardened the forwarded read from e.detailedReasons[0].metadata to e.detailedReasons[0]?.metadata. The deleted block was the only thing guarding that [0] access; optional chaining keeps the (contractually impossible but defensive) empty-array case from throwing a fresh TypeError, while forwarding the first reason to the extension host exactly as before for the normal single-reason case.

After this change, mainThreadDocuments.ts can no longer raise Unexpected reasons: ... for content-change events whose detailedReasons array has length other than one, because that array is an explicitly supported multi-value contract populated by the merge at textModelEvents.ts:590; the event's first reason is still forwarded unchanged.

Alternatives considered:

• Forward every reason to the extension host (array-valued detailedReason): rejected — the proposed vscode.TextDocumentChangeEvent.detailedReason API is intentionally singular, so this would expand a proposed API surface and touch the protocol, the ext-host converter, and the .d.ts, far beyond a focused error fix.
• Keep the assertion but exempt undo/redo: rejected — any deferred-emit merge (not only undo/redo) can yield multiple reasons, so an undo-specific exemption would still fire for other legitimate merges; the premise itself is wrong.
• Wrap the listener body in try/catch: rejected — that hides the symptom instead of correcting the invalid assumption and would mask genuinely unexpected errors in this hot path.

Recommended Owner

@hediet (Henning Dieterichs) — authored 488d6df7 / #252430, which introduced edit-source tracking, the detailedReasons merge, and the consumer assertion fixed here. He owns the proposed textDocumentChangeReason API and is best placed to confirm that forwarding the first reason (rather than surfacing all reasons) is the desired extension-host behavior.

Generated by errors-fix · 2.2K AIC · ⌖ 243.9 AIC · ⊞ 69.2K · ◷

[Copilot]

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

[Copilot]

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.