test(webview): settle every mounted harness whose fields read the tree - #389
Merged
Conversation
Under happy-dom CM's background-parse ViewPlugin never gets scheduler time, so a mounted EditorView does not self-heal: its StateFields stay built on the init-viewport parse fragment. Measured here, a 4616-code-unit doc publishes a 3012-code-unit snapshot and never advances. Settle the 15 mounted sites whose assertions trace to a production tree reader, via settledMount / settledState. Fixtures under the ~3000-char init viewport parse synchronously at create time, so these were latent rather than broken — they rested on an undocumented 20ms budget holding under CPU load, the same sensitivity behind the fold flake. Triaging all 329 construct hits across 99 files cut the roster from the TODO's "32 files" to 15 sites in 10 files: the lint suite parses via markdownLanguage.parser.parse directly, cm-table-widget-render uses the host-side parseTable, and outline-panel forces its own ensureSyntaxTree on open — none are exposed. Deliberately not settled, with reasons recorded in the PR body: five harnesses whose assertions would be damaged by a settle, three of which would fail silently green rather than red. No post-edit settles are added — in this suite a post-dispatch read is almost always the pin that the field responded to the edit, so settling there would mask a failure to recompute. Also fixes a mount-ordering leak in cm-table-field's drag-reveal test (three views constructed before its try) and a settledView(new EditorView(...)) in cm-decoration-orchestrator, which strands the view if the settle throws.
…le comments Review caught that both settle comments credited collectTableRanges with the syntaxTree(state) read. It does not do that read: table-ranges.ts takes an already-resolved tree and its header states "Pure: a lazy reader of the passed tree, no field/state dependency". The read is tableModels(state) in table-skeleton.ts, and this fixture reaches it only through resolveModels' fallback arm, because mount() registers tableBlockField without tableSkeletonField. Say that instead, so a reader chasing the comment does not land on a function documenting the opposite. Comment-only; the settle itself was and remains correct.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Settle the parse in every MOUNTED
test/webviewharness whose assertions depend on syntax-treeCONTENT. 15 sites across 10 files, all
test/**— no production source changes.Why
Under happy-dom, CodeMirror's background-parse
ViewPluginnever gets scheduler time, so amounted
EditorViewdoes NOT self-heal: itsStateFields stay built on the init-viewport parsefragment forever. Measured on this branch:
treeBeforeis CM'smin(3000, doc.length)init viewport; it never advances on its own.Honest severity: nearly every fixture here is UNDER that 3000-char viewport, so its init parse
completes synchronously at
EditorState.createtime. These sites are therefore LATENT, notcurrently broken — they rest on an undocumented internal budget (20 ms over
min(3000, doc.length)) continuing to hold under CPU load. That is the same load-sensitivitybehind the fold flake in LEARNING.md (2026-07-23), and this suite has already been bitten: the
baseline run for this PR had
validate-for-write.test.tsblow its 15 s timeout purely becausefive triage agents were loading the machine. It passes alone.
The roster was not what the TODO said
The TODO listed "32 files use no settle helper at all" and noted the list "has NOT been triaged
one-by-one". That count is of unsettled mounts, not of defects. Triaging all 329 construct hits
(93
new EditorView(, 233EditorState.create(, 3.setState() across 99 files reduced it to15 real sites, and found several roster entries to be architecturally immune rather than merely
small-doc:
lint/*files —lintMarkdown()/createIncrementalLinter()callmarkdownLanguage.parser.parse(text)directly (complete, un-budgeted, synchronous); zerosyntaxTreereferences undersrc/webview/cm/lint/;table/cm-table-widget-render— widgets built from the host-sideparseTable()string parser,not Lezer at all;
outline/outline-panel— production forcesensureSyntaxTree(state, doc.length, 500)on everysidebar open, and every content-asserting test calls
toggle()first, so it self-heals;paste/*—list-tree.ts'scaretInCode/listItemAtforce their own local bounded parse;frontmatter/*— detection is line-native by design ("NOT a Lezer-tree walk");cm-context-handoff— its states carry NO language extension, so settling would throwassertHasLanguage.Sites that must NOT be settled
More valuable than the 15 fixes. Settling any of these damages a working guard, and three of the
five fail GREEN:
list/cm-list-hang-integration.test.ts:141toBeLessThan(3)) under a mocked clock, as the non-vacuity guard for thefullTreefix beneath it. Settling breaks it outright — fails RED.fenced-code/cm-fenced-collapse-bounded.test.ts:379-387table/cm-table-skeleton.test.ts:158-164cm-block-widget-byte-identity.test.ts:314-325syntaxTreeAvailablegate.decorations/cm-decoration-block-style.test.ts:1131/:1166/:1185/:1204 andtable/cm-table-field.test.ts:291/:297Mount-time settle is safe for the last group because their identity baselines (
bqBefore,span1) are captured AFTER the factory returns —forceParsing's own empty transaction hasalready happened and is not observed by the comparison.
Post-edit reads: audited, deliberately not settled
A mount-time settle covers the INIT read only; CM's per-transaction incremental reparse has its
own 20 ms budget, so a snapshot can in principle re-truncate after an edit. Every read-point was
classified (INIT-READ / POST-EDIT-READ / per-dispatch pin) and the result is that NO post-edit
settles are added, because the remedy would be actively harmful here:
almost every POST-EDIT-READ in this suite exists to assert THAT THE FIELD RESPONDED TO THE EDIT.
table/cm-table-field.test.ts:223assertstoHaveLength(0), dispatches, then assertstoHaveLength(1). A settle between them fires an extra transaction that recomputes the field —so if
tableBlockFieldever failed to recompute ondocChanged, the settle would supply therecompute and the test would still pass. "POST-EDIT-READ" and "pins the update cycle" are nearly
the same set in practice.
The residual risk is real, bounded (all fixtures far under the init viewport), and recorded here
rather than silently closed. Fixing it properly needs a settle that does not perturb the field,
which is a different change from this PR's purpose.
Scope boundary
This PR closes the current gaps; it does NOT prevent recurrence. Nothing stops a new harness from
calling
new EditorViewagainst a production tree reader and reintroducing the bug — theinventory is a convention, not CI enforcement. That enforcement question is its own TODO DECISION
entry, which already records two prior failures of the naive text-scan approach, so it is not
re-litigated here.
One measurement that feeds that decision without pre-empting it: its option (b), "a narrow lint
rule banning
forceParsingimports outsidetest/webview/helpers/", would land GREEN today.Measured on this branch — exactly one import (
helpers/settled-view.ts:1) and one call (:151),both inside the helper; every other
forceParsingoccurrence undertest/**is prose in acomment.
Also fixed
table/cm-table-field.test.ts— the "drag reveal" test constructed three views BEFORE itstry.Once
mount()settles, a throw on the 2nd or 3rd stranded the earlier ones, becausesettledMountby contract destroys only the view IT built. Constructions moved inside thetry,disposal via
?.destroy().decorations/cm-decoration-orchestrator.test.ts— usedsettledView(new EditorView(…)), theexact leak shape the helper's docblock calls out (a throw strands the view before anything owns
it). Changed to
settledMount.Two
readOnlyinline mounts (cm-task-checkbox-widget-toggle-target.test.ts:87,cm-task-checkbox-widget-toggle.test.ts:213) are settled too, though their guard aborts beforethe tree read. Settling is inert there, and applying it uniformly removes a silent dependence on
the internal ORDER of
toggleTaskCheckbox's guards.Coverage
"Every hit accounted for" is measured, not assumed:
new EditorView(. 45 triaged site-by-site; theother 2 are
helpers/settled-view.tsand its own test — the settle machinery itself. Nountriaged mount.
settledState/fullTree(PR2). The single exception,inline/cm-inline-formatting-commands.test.ts, is NO-SETTLE BY DESIGN: it builds adeliberately parser-free state as the pin proving
computeInlineFormatnever consults theparser. Settling would attach a language and destroy that pin.
Non-vacuity
This migration adds call sites, not new guarantees, so it adds no test cases. The load-bearing
proof already exists upstream, shipped in PR1:
helpers/settled-view.test.ts:70-90asserts on aAn earlier draft of this PR planned to prove non-vacuity by passing a 0 ms budget at one site and
watching it go RED. Two independent design reviews killed that, correctly:
settledStatetakesno budget parameter at all, and even for
settledMounta 0 ms budget proves nothing here becauseParseContext.workshort-circuits onisDoneBEFORE consuming budget or checking its deadline —so any sub-3000-char fixture returns
truein 0 ms and never throws. That probe would have comeback GREEN and been read as "the settle is decorative".
Verification
pnpm compile— clean (5 tsconfig projects)pnpm test:unit— 270 files, 5145 tests, all passpnpm lint— clean on all 11 changed files (the 5 remaining repo warnings are pre-existing intest/markdown/validate-for-write-incremental.test.ts, untouched here)pnpm build— cleangit diff -U4 | grep -A4 "dispatch(" | grep '^\+.*settled'— empty: no settle inserted betweenany dispatch and a following assertion
pnpm package/ force-install do not apply:test/**only, no shipped bytes.