Skip to content

fix: roll up [label](/n/…) chrome on first editor paint - #85

Merged
chasehuh merged 1 commit into
mainfrom
task/link-first-paint
Aug 5, 2026
Merged

fix: roll up [label](/n/…) chrome on first editor paint#85
chasehuh merged 1 commit into
mainfrom
task/link-first-paint

Conversation

@chasehuh

@chasehuh chasehuh commented Aug 5, 2026

Copy link
Copy Markdown
Member

Closes #84.

The bug

On first open of a note, in-app Markdown links painted raw[label](/n/id) with
all chrome visible. One Enter rolled them up into the blue label. Worse than
cosmetic: hrefAtPos reads the same tree, so those links were unclickable until
that first edit.

Root cause — not the race it looks like

The obvious read is "ensureSyntaxTree(…, 50) times out on a long note". It doesn't —
the 50ms budget is almost never the binding constraint. It's a deterministic
stale-snapshot read.

// @codemirror/language
class LanguageState {
  constructor(context) { this.context = context; this.tree = context.tree }   // snapshot, once
  static init(state) {
    let vpTo = Math.min(3000 /* Work.InitViewport */, state.doc.length)
                                                                              // 20ms budget
  }
}
function syntaxTree(state)      { return field ? field.tree : Tree.empty }      // the SNAPSHOT
function ensureSyntaxTree()    {  return result }                            // the ADVANCED tree

collectMarkdownLinks called ensureSyntaxTree for its side effect on the shared
mutable parse context, discarded the return value, and then iterated
syntaxTree(state) — the snapshot from LanguageState.init, which covers at most the
first 3000 chars. Links outside it have no Link node: no decorations, no click target.
The first edit constructs a LanguageState around the by-then-advanced context, so the
links roll up. Hence "press Enter once".

Measured (jsdom, decorations read from the facet)

doc doc len syntaxTree(state).length ensureSyntaxTree first paint after one edit
link at start, short 39 39 ok
link at start, 87k 86930 86930 ok
link at end, 87k 86930 86890 ok none
link at end, 889k 888930 888890 ok none still none

The snapshot ends exactly 40 chars — the final paragraph — short of the doc while
ensureSyntaxTree reports success: the complete tree was available and thrown away.
Deterministic, because an incremental Markdown parse deliberately leaves the last line
open, so once a doc passes the 3000-char init viewport the snapshot systematically
excludes the trailing paragraph.

The 889k row is why docChanged was the other half of the bug: LanguageState.apply
only advances to max(mapPos(treeLen), viewport.to), so "press Enter" is not even a
general workaround.

The fix

  1. Iterate the tree ensureSyntaxTree returns (documentTree), falling back to
    syntaxTree when the budget is exhausted — a partial tree still decorates everything
    it covers.
  2. Rebuild when the tree changes, not only on docChanged. The background
    parseWorker reports progress with an effect-only transaction
    (Language.setState.of(new LanguageState(field.context))), which
    syntaxTree(tr.startState) !== syntaxTree(tr.state) catches — the canonical CM6 idiom
    for syntax-dependent decorations. This makes the budget-exhausted case self-healing
    instead of permanent.

Reading tr.state inside a StateField.update is safe here: @codemirror/state assigns
tr._state = this before computing any slot, and field() routes through ensureAddr,
which computes a dependency on demand with cycle detection. The language field has no
dependency on these fields, so no cycle — and it does not rely on markdown() being
installed first, though it is.

Tests

New agentnoteLinks first paint block in lib/editor/links.test.ts. Assertions read
decorations out of view.state.facet(EditorView.decorations) rather than the DOM —
jsdom has no layout, so the viewport is a couple of lines and a link further down
never renders regardless of correctness. That is why the existing DOM-based decoration
test never caught this.

  • link past the initially parsed region is label-marked and its ](url) replaced, with
    no doc change — fails on main
  • hrefAtPos resolves it on first paint — fails on main
  • control: a link inside the initially parsed region still decorates — passes both ways,
    so the suite proves the fix rather than the fixture

Verification

  • pnpm vitest run — 358 passed / 34 files (was 355; +3)
  • pnpm exec tsc --noEmit — clean
  • pnpm build — compiled successfully
  • eslint lib/editor components — 12 problems before, 12 after (all pre-existing)

Notes / residuals

  • Parse-progress transactions now trigger a full-document rebuild. Bounded: it stops once
    the parse completes, and each rebuild's ensureSyntaxTree short-circuits through
    parse.isDone(upto).
  • Part 2 has no dedicated test. Isolating it needs a document large enough to
    deterministically blow the 50ms budget, which is machine-dependent and would be flaky in
    CI; part 1 alone already fixes every case measured above, including 889k chars. Part 2 is
    defense-in-depth for slower hardware and bigger notes.
  • Decoration layer only — no wire-format change, and image/video widgets build their own
    decorations and are untouched.

🤖 Generated with Claude Code

Opening a note painted its links as raw `[label](/n/id)` until the first
keystroke rolled them up, and `hrefAtPos` reads the same tree, so those
links were unclickable too — not just mispainted.

Not the parse-budget race it looks like. `LanguageState` snapshots
`context.tree` once, at construction, and `syntaxTree(state)` returns
that snapshot; `ensureSyntaxTree` advances the shared mutable context and
returns the advanced tree. `collectMarkdownLinks` called
`ensureSyntaxTree` for its side effect and then read the stale snapshot,
which after `LanguageState.init` covers at most the first 3000 chars
(`Work.InitViewport`) — and, once a doc exceeds that, systematically
stops short of the final paragraph, because an incremental Markdown
parse leaves the last line open. The first edit builds a LanguageState
around the by-then-advanced context, which is why one Enter "fixed" it.

Iterating the tree `ensureSyntaxTree` returns fixes that. Rebuilding on
`docChanged` alone was the other half: `LanguageState.apply` only
advances to `max(mapPos(treeLen), viewport.to)`, so on a large enough
note the tail stayed raw even after edits. Both fields now also rebuild
when tree identity changes, which is how the background parseWorker
reports progress (an effect-only transaction). Reading `tr.state` in a
field update is safe — `tr._state` is assigned before any slot is
computed, and `field()` computes dependencies on demand.

Tests read decorations out of the facet rather than the DOM: jsdom has
no layout, so a link below the fold never renders even when correctly
decorated. The two new cases fail against the old code; the control (a
link inside the initially parsed region) passes both ways.

Closes #84

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@vercel

vercel Bot commented Aug 5, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
memo Ready Ready Preview Aug 5, 2026 12:02pm

Request Review

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix: roll up [label](/n/…) chrome on first editor paint

1 participant