perf(webview): narrow the TABLE-DELIM guard arm to a per-line shape delta - #398
Merged
Conversation
… a line-shape delta Swap the TABLE-DELIM arm of touchesStructuralReparse from a bare pipe- presence check to tableRowShapeChanged, so typing inside an existing table cell no longer forces a full rebuild for tableSkeletonField and the other bounded-recompute consumers. Move the oldLine/newLine reads up so the new arm can use them and drop the now-duplicate declarations below. Rewrite the TABLE-DELIM doc paragraphs to describe the delta contract instead of the old presence-based one.
…e mirror Adds a source-text tripwire that reds if @lezer/markdown 1.6.4's private table-formation internals (delimiterLine, the endLeaf/nextLine raw vs. stripped asymmetry, the parseRow cell-counting loop, and the hasPipe/ parseRow call-site offsets) ever drift out from under the guard's mirror, plus a version pin so a caret-range bump is a conscious edit. Also pins that fenced-code-collapse.ts deliberately keeps its own narrower STRUCTURAL guard rather than sharing the widened one, both as a source-text assertion and as a behavioural fixture (editing a hash comment / equals line / underscore rule / four-space-indented hash inside an 11-line fence body stays equivalent to the full-recompute oracle).
Chained differential fuzz over tableSkeletonField, imageBlockField, calloutMarkerConcealField, and the three fold gutter fields: 160 chains of 5 random single-character edits each, comparing the evolved state against a fresh full build from the same document. Mutant-killed by temporarily forcing tableRowShapeChanged to return false — the exhaustive oracle reds as required; the fuzz stays green, empirically confirming random sampling alone would not have caught this bug class.
Correct the field headers to describe the delta-based TABLE-DELIM arm
instead of the old presence-based one: table-skeleton.ts's PERF
paragraph now explains that only a delimiter row completing/breaking
or a cell-count change takes the full walk, and image-field.ts drops
the false 'a table cell' entry from its list of SHAPE-only regressing
classes.
Repair the two cm-table-skeleton.test.ts comments whose stated reason
('the line carries a |') stopped being true once TABLE-DELIM became a
per-line shape delta; the offsets and negative pins they document are
unchanged.
PERF.md, PERF-log.md, LEARNING.md and TODO.md are updated in the
separate local-only .claude/ tree (per this repo's TODO-local-mode
convention) and are not part of this commit.
…shaped line
The per-line four-fact delta is only sound where the parser's verdict is a pure
function of the one line that changed. For a delimiter-shaped line it is not, at
either @lezer/markdown call site, and neither fact is recoverable from that line:
- TableParser.nextLine reaches `delimiterLine` only behind its own
`line.next == 45 || 58 || 124` gate, and `line.next` is decided by a
`skipSpace` that advances over charCodes 32/9 only, while `delimiterLine`'s
`\s` also accepts NBSP / U+3000 / \f / \v.
- endLeaf runs `parseRow(cx, next, line.basePos)` — the peeked delimiter line
measured from the PRECEDING line's basePos. When the delimiter line is less
indented than its header, `[0, basePos)` of it is table content: neither
whitespace (so the mirrors' offset-equivalence argument fails) nor a
container marker (so SHAPE does not fire either).
Both were reproduced against the real parser destroying a whole Table with all
four facts constant and every arm silent. So a line that is delimiter-shaped on
either side now fires unconditionally; pipe presence and cell count stay a delta
everywhere else. A header or data row is never delimiter-shaped, so the class
this narrowing exists for — typing inside a table cell — stays bounded. The
accepted cost is that editing a delimiter row always takes the full walk.
Also corrects the three parallel descriptions that had drifted: the roster of
"keystroke classes that now fire" still listed a table cell against its own
TABLE-DELIM note; table-skeleton.ts named two of the arm's facts while saying
"only"; image-field.ts described the pre-PR arm as document-global when it was
per changed line.
The oracle stayed green through both holes above because it could not enumerate either class: SHAPE_CORPUS had no geometry pairing an indented header with a less-indented delimiter line, and ALPHABET held only " " and "\t", so no edit could separate `delimiterLine`'s `\s` from lezer's `skipSpace`. Add the two geometries and the four non-ASCII whitespace characters. Measured with the pre-repair predicate restored: 104 residuals over 19 documents, including the ASCII-only `|--|-` -> `|- |-` edit that deletes a Table with every arm silent. Also carries the enumerated edit offset on SingleCharEdit instead of letting the consumer re-derive it with a first-difference scan (measured: 1277 of 75750 offsets differ, 30 on a different line — same document today, nothing red), drops the oldLine/newLine fields nothing read, and hoists the loop-invariant base EditorState to one per corpus document. The predicate never reads the syntax tree, so settling it per edit bought nothing: 93672 edits now build 44 base states instead of 93672.
…serialiser `inCellBounded > 20` passed under the pre-PR presence arm too (measured 23 at the shipped seed), so it pinned nothing about this change: a table's trailing overshoot line carries no pipe, and editing it is bounded under either arm. Require the edited line to carry a `|` as well and raise the threshold. Measured across six seeds: 72 / 94 / 86 / 104 / 97 / 72 under the shipped arm, 0 at every seed with the arm reverted — under which the assertion now reds. serializeGutter took a hand-rolled parameter typing `value` as `unknown` and recovered `elementClass` by unchecked cast. It is the only thing distinguishing one gutter marker from another, so losing that property would degrade every entry to `from-to:undefined` on both sides and silently reduce the differential to range-only. Typed against RangeSet<GutterMarker>, that is a compile error. Corrects the fenced-collapse comment claiming its four in-fence fixtures make the two guards' divergence observable: step 3 of the reducer routes the bounded field and the full-recompute oracle through the same buildFullState, so a widened guard keeps the equivalence green. The fixtures do pin bounded correctness for those markers; the narrowness is pinned by the tripwire's source-text assertions.
The retreat to presence on delimiter-shaped lines left four sites still explaining themselves in terms of the pre-retreat four-fact delta: - the admission test's "raw-only delimiter flip" is not raw-only (the stripped reading of ` :---|` is `:---|`, which also matches); what the case still pins is the consequence — an enclosing list's extent moving across a blank line - the two shape-test rows labelled "RAW delimiter-ness only" isolate nothing now; they are kept as the counterexamples that forced the retreat - "a header or data row is never delimiter-shaped" is false for `| - | - |`, which does match the regex and takes the presence path (a perf edge, not correctness) - the corpus header's "~44k edits" predates the corpus and alphabet growth Also records, against the SHAPE follow-up, that the same presence cost now reaches delimiter-looking lines inside code blocks and prose.
…ndant The second lazy-continuation document added in cycle 2 was measured to be the sole carrier of nothing: with the PRESENCE retreat reverted to the four-fact delta and the Unicode whitespace removed from ALPHABET (which isolates the borrowed-basePos class from the `\s`-vs-`skipSpace` one), all 4 residuals land on the FIRST document. Dropping it costs 2,201 enumerated edits and no detection; the surviving entry's comment now records that measurement so the line is not re-added on a hunch. `SingleCharEdit.before` went dead when the enumerator started carrying `pos` — the first-difference scan was its only reader — so the field and its three initialisers go with it, and `armsFire` takes the edit record instead of three re-spread fields (plus the explicit return type it was missing). Comments: the "do NOT simplify a DELTA clause" warning was stated twice in structural-guard.ts, and `tableRowShapeChanged`'s doc comment restated the cell-body rationale its own header paragraph already carries. One copy of each survives. The two consumer headers this PR edited in place are rewrapped to the width the rest of their block uses, and the fuzz test's seed-sweep numbers are re-measured against the shortened corpus (77 / 96 / 94 / 95 / 79 / 81, still 0 at every seed under the pre-PR presence arm).
… heading Two totals for one enumeration survived the corpus edit — the header was updated while an undated `1277 of 75750 edits` eight lines below was not. Date it and say which enumeration it measured, so the ratio outlives the next corpus change. The shape test's section heading still read `the four facts, one per row`, naming the reverted design; several rows beneath it fire on more than one clause now, and the two it labelled as clause-isolating were relabelled last cycle as isolating nothing.
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
1 participant
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.
Summary
Narrows the
TABLE-DELIMarm of the shared bounded-rebuild guard from a presence test ("the changed line holds a|⇒ full rebuild") to a per-line shape delta, and closes two soundness holes in theSHAPEarm that the presence test had been masking by accident.Typing inside a table cell is
tableSkeletonField's own primary scenario, and every such keystroke was giving up the parse reuse the field exists for. That is now back on the bounded path.Why this is a soundness change, not a tuning change
The narrowing is only correct once two pre-existing holes are closed, both found by the proof this PR adds:
===underline flips a whole run betweenSetextHeadingandTable/Paragraph. No arm saw it — the line has no pipe, no newline, no>, no blank flip, no indent change.SHAPEanchored its fence / HTML-open / ATX / underscore alternations at CommonMark's top-level[ \t]{0,3}. Inside a nested list# his a real heading. The presence-basedTABLE-DELIMarm covered that case only by accident, because the line happened to hold a|.Both are fixed by widening
STRUCTURAL(a wider guard can only cost full rebuilds, never correctness).The predicate
tableRowShapeChangedfires when the changed line is delimiter-shaped on either side (ineither the raw or the whitespace-stripped reading), or when either of two per-line facts
changed:
hasPipe, orparseRow's cell count.The retreat to presence on delimiter rows specifically is the design, not a hedge. A
per-line delta is sound only where the parser's decision is a pure function of that one line,
and for a delimiter row @lezer/markdown's is not:
TableParser.nextLinegatesdelimiterLinebehindline.next ∈ {-,:,|}, computed afterskipping only space/tab, while the regex's
\salso accepts NBSP / U+3000 /\f/\v; andendLeafmeasures the delimiter line'sparseRowfrom the preceding line'sbasePos.Neither is recoverable from the changed line alone. Editing a delimiter row is rare; typing
inside a table cell is not, and that is the class this PR recovers.
What review caught
The first draft of this PR used a four-fact per-line delta with no presence fallback. Two
independent reviewers each found a CRITICAL soundness hole in it, both reproduced against the
production parser, and both cases the pre-PR presence arm did fire on:
" :---|"→":---|"(deleting an NBSP, U+3000,\for\v): all four facts constant, every arm silent, aTableappears. The oraclecould not see it because
ALPHABETheld only space and tab."- l\n h | e\n|--|-\n r1\n r2\n", aone-character edit to the delimiter row leaves all four facts constant with every arm
silent while
Table@L1-L4is destroyed — becauseendLeafmeasured that line from theheader line's
basePos.Both geometries and the four Unicode whitespace characters are now in the corpus, so neither
class can silently return.
How it is proven
A bounded-exhaustive differential oracle (
cm-structural-guard-exhaustive.test.ts): every single-character insert, delete and replace at every offset of a 44-document corpus, run against the production parser. For every edit the guard stays silent on, the block identity the bounded consumers reuse must be unchanged.arms-silentrising from 9,848 to 29,245.A chained differential fuzz over all six consumer StateFields (⚠️ That last metric is the narrowness pin, and its first version was vacuous:
cm-structural-guard-fuzz.test.ts) covers the consumer level: 793 comparisons, 334 on the bounded path, 72 of them on a pipe-carrying line inside a realTablenode, 0 mismatches.inCellBounded > 20passed under the pre-PR arm too (measured 23). It now requires the edited line to carry a|— 0 under the base guard at every seed, 72–114 under the shipped one.A mutant-kill measured why the exhaustive oracle is the primary evidence, not the fuzz. Forcing
tableRowShapeChangedtoreturn false:Reaching the counterexample needs a specific geometry and one specific offset. Uniform sampling does not get there.
Measurements
In-cell keystroke, 30-run averages, before = the presence arm restored:
tableSkeletonField, 25 KB / 150 tablestableSkeletonField, ~200 KB / 1200 tablesimageBlockField, 19 KB / 150 imagesimageBlockField, 154 KB / 1200 imagesEditing a delimiter row itself now always full-rebuilds (it used to bound). That is the price
of the presence fallback and it is the right trade: delimiter rows are edited rarely, cell
bodies constantly.
The widening's cost is stated rather than implied. Six worst-case edits that now full-rebuild where they used not to (each asserted
touchesStructuralReparse(tr) === truebefore timing, ~208 KB / 1200-table document) measured 3.75–4.44 ms — all under the 5 ms bar, with as little as ~0.6 ms headroom. Full breakdown inPERF-log.md.SHAPEstays presence-based, so a list-item body, a blockquote line, an ATX heading and every Enter still take the full walk. Narrowing that arm is filed as a follow-up.Guarding the mirror
@lezer/markdownis declared as^1.6.4— a caret range — and this code mirrors private internals.cm-lezer-table-internals-tripwire.test.tsasserts the mirrored regex, the raw-vs-sliced call-site asymmetry, theparseRowloop, thebasePosstart-position contract, and the resolved version, so an upstream change is a red rather than a silent divergence. It also pins thatfenced-code-collapse.tskeeps its own deliberately narrower guard, with a behavioural fixture proving its in-fence hot path survives.Test plan
pnpm compile/pnpm lintcleanpnpm test:unit— 278 files, 5271 tests greenpnpm build,pnpm package— vsix audit 0 violations