From 716a1849cf08f9d72fcff23de5f43d5764e04232 Mon Sep 17 00:00:00 2001 From: chaewon-huh Date: Wed, 5 Aug 2026 21:01:22 +0900 Subject: [PATCH] fix: roll up [label](url) chrome on first editor paint MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- lib/editor/links.test.ts | 72 ++++++++++++++++++++++++++++++++++++++++ lib/editor/links.ts | 46 ++++++++++++++++++++++--- 2 files changed, 114 insertions(+), 4 deletions(-) diff --git a/lib/editor/links.test.ts b/lib/editor/links.test.ts index 0ecc71b..cedd0d2 100644 --- a/lib/editor/links.test.ts +++ b/lib/editor/links.test.ts @@ -216,3 +216,75 @@ describe("agentnoteLinks decorations", () => { view.destroy(); }); }); + +describe("agentnoteLinks first paint", () => { + const LINK = "[Deploy checklist](/n/abc-mnop-xyz)"; + /** + * Over `Work.InitViewport` (3000 chars), so the language field's initial parse + * cannot finish. Its tree snapshot then stops short of the final paragraph — + * which is exactly where the link lives. + */ + const LONG_NOTE = `${"a paragraph of ordinary prose\n\n".repeat(200)}see ${LINK} here`; + const LABEL_FROM = LONG_NOTE.indexOf(LINK) + 1; + const LABEL_TO = LABEL_FROM + "Deploy checklist".length; + + let view: EditorView | null = null; + + function mount(doc: string) { + const parent = document.createElement("div"); + document.body.appendChild(parent); + view = new EditorView({ + parent, + state: EditorState.create({ + doc, + extensions: [markdown(), agentnoteLinks()], + }), + }); + return view; + } + + /** + * Decorations read out of state rather than the DOM: jsdom has no layout, so + * a link below the fold is never rendered even when correctly decorated. + */ + function decorations(v: EditorView) { + const labels: { from: number; to: number }[] = []; + const hidden: { from: number; to: number }[] = []; + for (const source of v.state.facet(EditorView.decorations)) { + const set = typeof source === "function" ? source(v) : source; + for (const iter = set.iter(); iter.value; iter.next()) { + const className = (iter.value.spec as { class?: string }).class ?? ""; + const into = className.includes("cm-md-link") ? labels : hidden; + into.push({ from: iter.from, to: iter.to }); + } + } + return { labels, hidden }; + } + + afterEach(() => { + view?.destroy(); + view = null; + document.body.replaceChildren(); + }); + + // The bug: `[label](/n/id)` painted raw until the first keystroke rolled it up. + it("rolls up a link past the initially parsed region without any edit", () => { + const { labels, hidden } = decorations(mount(LONG_NOTE)); + + expect(labels).toContainEqual({ from: LABEL_FROM, to: LABEL_TO }); + // `](` and the `/n/…` destination are replaced away, not left as raw text. + expect( + hidden.some((range) => range.from === LABEL_TO && range.to > LABEL_TO), + ).toBe(true); + }); + + it("makes that link clickable on first paint too", () => { + const v = mount(LONG_NOTE); + expect(hrefAtPos(v.state, LABEL_FROM + 2)).toBe("/n/abc-mnop-xyz"); + }); + + it("still decorates a link inside the initially parsed region", () => { + const v = mount(`see ${LINK} here\n\n${"filler\n\n".repeat(400)}`); + expect(decorations(v).labels).toContainEqual({ from: 5, to: 21 }); + }); +}); diff --git a/lib/editor/links.ts b/lib/editor/links.ts index 28120fb..4d2585a 100644 --- a/lib/editor/links.ts +++ b/lib/editor/links.ts @@ -8,6 +8,7 @@ import { RangeSetBuilder, StateField, type Extension, + type Transaction, } from "@codemirror/state"; import { Decoration, @@ -76,18 +77,41 @@ const FILE_LIKE_TLDS = new Set([ export type LinkHit = { from: number; to: number; url: string }; +/** Milliseconds granted to finish parsing before falling back to a partial tree. */ +const PARSE_BUDGET_MS = 50; + +/** + * The most complete syntax tree available for the whole document. + * + * `ensureSyntaxTree` advances the language field's *shared, mutable* parse + * context and returns the resulting tree, but `syntaxTree(state)` reads the + * snapshot taken when the `LanguageState` was constructed — which is still the + * pre-parse one. Reading that snapshot is why a note opened with a link past the + * initially parsed region painted raw `[label](/n/id)` until the first edit + * produced a `LanguageState` carrying the advanced tree. + * + * The fallback covers a parse that ran out of budget: a partial tree still + * decorates everything it does cover, and the fields below rebuild when the + * background parser gets further. + */ +function documentTree(state: EditorState) { + return ( + ensureSyntaxTree(state, state.doc.length, PARSE_BUDGET_MS) ?? + syntaxTree(state) + ); +} + function collectMarkdownLinks(state: EditorState): { /** Clickable span = visible link text only (label / autolink URL). */ hits: LinkHit[]; hide: { from: number; to: number }[]; labels: { from: number; to: number }[]; } { - ensureSyntaxTree(state, state.doc.length, 50); const hits: LinkHit[] = []; const hide: { from: number; to: number }[] = []; const labels: { from: number; to: number }[] = []; - syntaxTree(state).iterate({ + documentTree(state).iterate({ enter(node) { if (node.name !== "Link" && node.name !== "Autolink") return; @@ -237,10 +261,24 @@ function buildLabelDecorations(state: EditorState): DecorationSet { return builder.finish(); } +/** + * True when this transaction changed the document or moved the parse forward. + * + * Rebuilding on `docChanged` alone is why a long note kept raw link markup after + * the first edit too: the background parser reports progress through a + * `Language.setState` transaction that changes no text, so the decorations never + * caught up with the region it had just parsed. Comparing the tree covers both, + * and `tr.state` is safe to read here because the language field is installed + * before these fields, so its value is already computed. + */ +function needsRebuild(tr: Transaction): boolean { + return tr.docChanged || syntaxTree(tr.startState) !== syntaxTree(tr.state); +} + const hiddenLinkMarks = StateField.define({ create: buildHideDecorations, update(deco, tr) { - if (tr.docChanged) return buildHideDecorations(tr.state); + if (needsRebuild(tr)) return buildHideDecorations(tr.state); return deco.map(tr.changes); }, provide: (field) => [ @@ -252,7 +290,7 @@ const hiddenLinkMarks = StateField.define({ const visibleLinkMarks = StateField.define({ create: buildLabelDecorations, update(deco, tr) { - if (tr.docChanged) return buildLabelDecorations(tr.state); + if (needsRebuild(tr)) return buildLabelDecorations(tr.state); return deco.map(tr.changes); }, provide: (field) => EditorView.decorations.from(field),