Skip to content

Add line-anchor tracking to CodeMirror - #5988

Merged
falkoschindler merged 35 commits into
zauberzeug:mainfrom
Jepson2k:cm-line-anchors
Aug 11, 2026
Merged

falkoschindler merged 35 commits into
zauberzeug:mainfrom
Jepson2k:cm-line-anchors

Conversation

@Jepson2k

@Jepson2k Jepson2k commented Apr 24, 2026 •

Copy link
Copy Markdown
Contributor

Motivation

Line anchors give you a more stable reference to specific lines than line numbers. CodeMirror knows where each anchor moved to as the user types — paste 30 lines above an anchor and it follows; delete the line it sits on and it slides to the next one (an anchor only disappears when a deletion runs across it). Useful when the host application needs a handle to a specific line that survives edits without recomputing positions in Python on every keystroke.

The new demo on the docs page shows the idea: two lines are tagged with anchors "A" and "B", and a pair of buttons rewrites whichever line the anchor currently points at. You can edit the code freely — insert blank lines, reformat, reorder — and clicking "Update" still hits the right line.

line_anchors.mp4

Implementation

  • editor.line_anchors is a {id: line} dict; assigning it (or passing it to the constructor) syncs to the client. An optional on_anchor_change= handler fires whenever tracked positions change.
  • editor.line_anchor_positions reflects the current {id: line} mapping. The browser is the source of truth — anchors are remapped through edits by CM6's RangeSet.map() and the snapshot is pushed back after every state change.

Progress

  • The PR title is a short phrase starting with a verb like "Add ...", "Fix ...", "Update ...", "Remove ...", etc.
  • The implementation is complete.
  • This PR does not address a security issue.
  • Pytests have been added.
  • Documentation has been added.
  • No breaking changes to the public API.

Jepson2k and others added 6 commits April 24, 2026 08:56
`set_line_anchors`, `clear_line_anchors`, and the `line_anchor_positions`
property let the host application pin caller-chosen IDs to specific
lines and look up where each ID points after the user has edited the
document. CodeMirror remaps the underlying RangeSet positions through
edits; the Python side mirrors the current 1-indexed line for each ID,
keyed by the named set the anchor lives in.

Two paths keep the Python mirror in sync:

- `set_line_anchors(...)` and `clear_line_anchors(...)` update
  `self._anchor_positions` synchronously *before* dispatching the JS
  call, so the property returns the correct value on the next line of
  Python (no waiting for a round-trip).
- During typing, a CM6 `ViewPlugin` re-emits the remapped positions on
  document changes (only when the anchor field is non-empty), debounced
  50 ms to coalesce bursts. The debounce is what keeps the wire quiet
  on high-latency connections.

Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
- JS: await editorPromise in setLineAnchors/clearLineAnchors so calls made
  during page setup don't race the editor mount (matches getLanguages /
  getThemes pattern).
- Python: validate input in set_line_anchors (raise ValueError on line < 1
  or duplicate ids) so the mirror cannot diverge from the JS-side clamp.
- Python: harden _update_anchor_mirror against malformed event payloads.
- Demo: subscribe to anchor-positions for live label refresh instead of a
  manual button.
- JS: reset _anchorTimer inside the timeout callback; comment AnchorValue.eq.
- Tests: add coverage for input validation; loosen the bounded-emissions
  threshold so the test stays green on slower CI (the assertion still
  proves coalescing).
The previous implementation had two writers into `_anchor_positions`:
`set_line_anchors` / `clear_line_anchors` wrote synchronously, and
`_update_anchor_mirror` wrote on every JS event. This created three
related bugs:

- a stale debounce closure could resurrect cleared anchors when its
  pre-clear field snapshot fired after `clear_line_anchors`
- `clearLineAnchors` did not cancel the pending JS timer
- the Python mirror diverged from JS for `line > doc.lines`, since JS
  silently clamps and Python stored the un-clamped caller value

Treat the browser as the source of truth instead. JS owns the
`StateField<RangeSet<AnchorValue>>` and emits a full snapshot of all sets
after `setLineAnchors`, after `clearLineAnchors`, and on debounced
`docChanged` updates. The fire-time emit reads live editor state, so any
surviving stale timer sees the up-to-date field. The Python methods
validate input (single pass) and dispatch — `_update_anchor_mirror` is
the only writer, replacing `_anchor_positions` wholesale from each
snapshot.

Tests now wait for the JS round-trip via `screen.wait_for(...)` and a
new `test_clear_after_typing_does_not_resurrect_anchors` locks in the
race fix. Documentation demo subscribes to `anchor-positions` before
calling `set_line_anchors` so the label populates from the first JS
emit.
- JS: warn on clamp in setLineAnchors so callers can see when an
  out-of-range line is silently coerced to the last line, instead of
  the mirror differing from the request with no signal.
- JS: cancel the debounce timer inside setLineAnchors for symmetry with
  clearLineAnchors. Without this, a doc-edit-then-set sequence emits
  twice in close succession (the explicit emit, then the leftover
  debounced emit ~50 ms later) — both idempotent, but chatty.
- JS: drop the dead `if (self._anchorTimer)` guard on clearTimeout.
- Python: log a warning on malformed anchor-positions payloads in
  _update_anchor_mirror instead of silently swallowing them.
- Python: clarify in the set_line_anchors docstring that out-of-range
  lines clamp to the last line, log a console.warn, and that
  line_anchor_positions is authoritative for the actual landed line.
- Tests: replace the fixed `screen.wait(0.3)` after `screen.open('/')`
  with a deterministic `screen.wait_for(...)` that polls for the
  `.cm-content` mount marker. The negative-assertion sleeps in
  test_clear_after_typing_does_not_resurrect_anchors and
  test_anchor_emissions_bounded_during_typing are kept — fixed waits
  are correct for "confirm nothing arrives within a window".
- Tests: rewrite test_anchor_emissions_bounded_during_typing to dispatch
  the 10 doc changes synchronously from JS instead of via Selenium
  send_keys. The previous version was flaky on slower hardware where
  send_keys cadence exceeded the 50 ms debounce, producing one emission
  per keystroke and tripping the threshold. The synchronous dispatch
  guarantees all changes land inside one debounce window, so coalescing
  is observable as exactly one emission.
Anchors are most useful for "did anything that matters change?" — pure
whitespace edits leave anchor positions intact. The previous demo only
echoed the position dict; the new one shows the practical payoff: a
status label that flips green/red depending on whether the user's edits
actually moved the anchored landmarks.
The previous demo just compared anchor positions to initial positions and
flipped a status label red when anchors moved — but anchors *moving* is
exactly what they're supposed to do under whitespace edits, so the green/red
framing inverted the message. The new demo mirrors waldo-commander's actual
usage: anchor robot move targets, show their live line numbers as the user
edits, and provide buttons that rewrite a target's *current* line in place.
That demonstrates the value: address a line by its logical identity, not its
line number, even after the user has reformatted the code.
@Jepson2k
Jepson2k marked this pull request as ready for review April 30, 2026 16:47
@falkoschindler falkoschindler added this to the Next milestone May 5, 2026
@falkoschindler falkoschindler added feature Type/scope: New or intentionally changed behavior review Status: PR is open and needs review labels May 5, 2026
@falkoschindler
falkoschindler self-requested a review May 5, 2026 12:57
Jepson2k added 3 commits May 11, 2026 22:58
Collapses set_line_anchors/clear_line_anchors into the `line_anchors`
property, mirroring the cm-line-tooltips and cm-decorations shape per
Falko's review of zauberzeug#5992. line_anchor_positions flattens from
dict[str, dict[str, int]] to dict[str, int].

Python-side line/dup-id validation moves to JS as logAndEmit warnings:
the dict-proxy pattern can't raise on mutation, and a non-blocking
warning fits the same DX as Python's logger.
@Jepson2k Jepson2k mentioned this pull request Jun 4, 2026
6 tasks done
@falkoschindler falkoschindler modified the milestones: Next, 3.14 Jun 5, 2026
evnchn
evnchn previously requested changes Jun 14, 2026

@evnchn evnchn left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Posted by Claude Code (Claude Fable 5) on @evnchn's behalf.

Thanks so much for this, @Jepson2k — line anchors are a genuinely useful addition, and the test suite is a real highlight (behavior-level Screen tests, no mocks, and the debounce-resurrection test shows you thought carefully about the tricky cases). I ran the PR locally and dug into the anchor mapping with a few minimal repros. The core is solid; there are a few things to sort before it lands, and I've tried to keep each one concrete and small. One is a blocker, but it's a contained fix — happy to pair on any of these.

🔴 The one blocker — a single unrelated prop update wipes the tracked positions. Any server-side prop change re-broadcasts all props, which re-fires the deep lineAnchors watcher and re-runs applyLineAnchors([A@1, B@2]) from the original creation-time line field (codemirror.js ~L84-89, ~L218-220), discarding the live remapped positions. Concretely, after an edit has remapped anchors to A:2/B:3: setting editor.theme = 'oneDark' or editor.set_language('Python') snaps them back to {A:1, B:2}, and editor.value = ... corrupts the set differently (I saw {A:1}, with B dropped). The docs demo writes the value after the first action, so the anchors break on the second use. The fix is contained: instead of re-applying from props on every broadcast, deep-compare the incoming list against a stored _lastAppliedAnchors snapshot and no-op when it's unchanged (a deliberate reassignment still applies normally).

🟠 Deleting the last anchor doesn't notify Python. anchorTracker early-returns when the field size is 0 (codemirror.js ~L279), which swallows the 1→0 transition, so line_anchor_positions stays stale afterwards (I confirmed it survives the deletion plus two further edits with no emit). With two or more anchors a partial drop emits correctly — it's only the last-one case. One-line fix: also check update.startState.field(anchorField).size === 0 before returning.

🟠 A small docs mismatch on deletion. The description says "delete the line and it's gone," but anchors are zero-width ranges whose mapMode defaults to MapMode.TrackDel, and per the CM6 docs TrackDel only drops a position when a deletion happens across it (RangeValue.mapMode: "Defaults to MapMode.TrackDel"; MapMode.TrackDel: "Return null if deletion happens across the position"). A natural full-line delete starts at the anchor's leading edge, not across it, so the anchor slides to the next line rather than vanishing. Just worth matching the docs to that (or choosing a different mapMode if "gone on delete" is the intent).

A few minor things, none blocking:

  • the three *Added in version X.Y.Z* placeholders need a real version;
  • id is typed str but {'id': 1} round-trips as '1' (JSON keys + str() coercion);
  • _update_anchor_mirror guards that the payload is a dict but not the value types, so a malformed {'a': 'x'} would make int(v) raise;
  • with a shared element (script-mode page / multiple tabs) the single Python mirror is last-writer-wins — worth a docstring sentence.

And one optional API thought (entirely up to you): line_anchors=[{'id': 'A', 'line': 1}, ...] could become line_anchors={'A': 1} — it'd match the sibling line_tooltips: dict, mirror the line_anchor_positions: dict output, and remove the duplicate-id handling entirely. Is there an ordering need I'm missing? Similarly, the demo teaches the raw event string (editor.on('anchor-positions', ...)); an on_anchor_change= handler param (like on_change) would keep that out of user code.

Good news on the build side — no dist rebuild needed, the bundle already re-exports the CM APIs you use. I have small runnable repros for each of these if they'd help — happy to drop them in a gist. The two JS fixes are small, and I'm glad to take another quick pass once they're in. Really nice first cut. 🙂

@evnchn evnchn added in progress Status: Someone is working on it and removed review Status: PR is open and needs review labels Jun 14, 2026
Jepson2k and others added 2 commits June 16, 2026 19:06
…ct API

Address review feedback on line-anchor tracking (zauberzeug#5988):

- Fix the blocker where any unrelated server-side prop change re-broadcast all
  props, re-fired the deep lineAnchors watcher, and re-applied anchors from their
  original declared lines — discarding the live positions anchorField had remapped
  through edits (e.g. setting theme/language/value snapped anchors back). applyLineAnchors
  now compares the incoming anchors against a stored snapshot and no-ops when unchanged,
  so an unrelated re-broadcast leaves remapped positions intact while a genuine
  reassignment still applies.

- Fix the last anchor's deletion (field size 1 -> 0) not notifying Python:
  anchorTracker now skips only when both the start and end states are empty.

- Switch the line_anchors API from a list of {'id', 'line'} dicts to a flat
  {anchor_id: line} mapping, mirroring the line_anchor_positions output and the
  sibling line_tooltips dict. Keys/values are coerced to str/int so numeric ids
  round-trip consistently, and the duplicate-id handling is gone.

- Add an on_anchor_change handler (constructor param + method) mirroring on_change,
  harden _update_anchor_mirror so a malformed payload can't raise, document the
  shared-element last-writer-wins mirror, correct the anchor-deletion wording (a
  full-line delete slides the anchor rather than dropping it), and fill the version
  placeholders. Tests cover the blocker regression, last-anchor deletion, and the
  new handler.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The line-anchor API targets the 3.14.0 release; only line_tooltips (already on
main) is 3.13.0. Split line anchors into their own *Since version 3.14.0* block
and bump the line_anchors/on_anchor_change/line_anchor_positions markers, leaving
line_tooltips at 3.13.0.

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

Copy link
Copy Markdown
Contributor Author

ok I believe I've address the latest feedback in my latest commits including the suggest dict type change. The failing test appears to be a flaky one related to the tree component. It doesn't appear that I have the ability to re-run the tests, but I'd appreciate a re-run whenever possible. Thank you

@Jepson2k
Jepson2k requested a review from evnchn June 17, 2026 00:30
@evnchn

evnchn commented Jun 18, 2026

Copy link
Copy Markdown
Collaborator

Posted by Claude Code on @evnchn's behalf.

TL;DR:

All my 06-14 review points are addressed at fa656cda — the 🔴 blocker, both 🟠 fixes, the nits, and both 💡 optional suggestions (dict API + on_anchor_change). Checked against the diff, not the commit message.

A holistic re-review with end-to-end verification follows in a separate comment.

Per-point verification (diff at fa656cda)
Review point Status Evidence
🔴 Unrelated prop re-broadcast re-fired the deep lineAnchors watcher and re-applied from declared lines, wiping remapped positions ✅ applyLineAnchors() computes a JSON.stringify(Object.entries(anchors).sort()) snapshot, stores _lastAppliedAnchors, and no-ops when unchanged; a genuine reassignment still applies. Test test_anchor_positions_survive_unrelated_prop_update.
🟠 Last-anchor deletion (1→0) didn't notify ✅ anchorTracker now skips only when both startState and state field sizes are 0. Test test_last_anchor_deletion_notifies.
🟠 Docs vs TrackDel slide/vanish semantics ✅ line_anchors docstring: dropped only when a deletion spans across the position; a full-line delete slides to the next line. Matches CM6 default MapMode.TrackDel.
⚪ X.Y.Z version placeholders ✅ All 3.14.0 (commit fa656cda fixed a 3.13.0 slip).
⚪ id str/int coercion ✅ {str(k): int(v) …} in init + setter.
⚪ malformed value crashes int() ✅ _update_anchor_mirror try/except per anchor, warns + skips.
⚪ shared-element last-writer-wins ✅ Documented in line_anchor_positions.
💡 dict API {'A': 1} ✅ adopted line_anchors: dict[str, int]; the list-of-dicts proxy is gone.
💡 on_anchor_change= handler ✅ adopted New param + method + CodeMirrorAnchorChangeEventArguments.

Nicely done, @Jepson2k.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@falkoschindler falkoschindler self-assigned this Jun 30, 2026
@evnchn

evnchn commented Jul 2, 2026 •

Copy link
Copy Markdown
Collaborator

Posted by Claude Code on @evnchn's behalf.

TL;DR: Re-reviewed the delta since my 06-18 pass — @Jepson2k's preserved_props migration + getter merge (192a632e) and @falkoschindler's follow-up cleanups incl. the mixin extraction (91b7a6d). Clean improvement: the API simplification is a real win, the mixin is behavior-preserving, and I couldn't break the flag mechanism (incl. the _to_dict mutation you flagged, @falkoschindler — safe, analysis folded below). LGTM — one small test worth adding, otherwise good to land from my side (merge call still yours).

Local gates green: pre-commit ✓, mypy ✓, pylint 10.00 ✓, test_codemirror_line_anchors.py 8✅, sibling test_codemirror.py 19✅.

🟡 Missing test for the snap-back direction (the one actionable item)

@falkoschindler you noted "a deliberate-re-assign-snaps-back test would cover the other direction." Still the one real gap: the preserve direction is covered (test_anchor_positions_survive_unrelated_prop_update), but the deliberate-reassign-same-declared-value → snap-back direction (the whole reason the intent flag exists — value-comparison can't see it) isn't. I verified it works empirically (remap 3→4, reassign {mid: 3}, snaps back to 3), so it's coverage, not a bug — but it's the direction most likely to regress if the flag lifecycle is ever touched.

Ready-to-drop test (passes on the current branch, ~1.9s)
def test_reassign_same_declared_value_snaps_back(screen: Screen):
    editor = None
    @ui.page('/')
    def page():
        nonlocal editor
        editor = ui.codemirror('a\nb\nc\nd\ne')
    screen.open('/')
    _wait_for_editor(screen)
    editor.line_anchors = {'mid': 3}
    screen.wait_for(lambda: editor.line_anchors.get('mid') == 3)
    screen.selenium.execute_script(
        f'const el = getElement({editor.id});'
        'el.editor.dispatch({changes: {from: 0, insert: "X\\n"}});')
    screen.wait_for(lambda: editor.line_anchors.get('mid') == 4)  # remapped
    editor.line_anchors = {'mid': 3}                              # deliberate reassign, SAME value
    screen.wait_for(lambda: editor.line_anchors.get('mid') == 3)  # snaps back
✅ Flag-in-_to_dict safety — the sanity-check you asked for (@falkoschindler)

"worth sanity-checking that mutating a flag inside _to_dict is safe (i.e. _to_dict is only called for outgoing sends)."

I chased this hard (it's the one spot where the migration adds server-side state). It holds up. _apply_anchors is set by the setter and consumed by _to_dict(), which has two callers — the per-update outbox flush (outbox.py) and the full page-build (client.py) — but on a live, socket-connected client the reassignment is consumed by exactly one _to_dict() (the single outbox flush, deduped by element id). The full page-build can't race it: each page GET (incl. root/auto-index) builds a fresh Client with fresh element instances (page.py with Client(self, request=request)), so there's no shared live element another viewer could serialize; and build_response() clears outbox.updates before it serializes anyway. So no reachable double-consumption → the deliberate reassignment isn't dropped.

Dead end worth recording: I first suspected a race — a second viewer's page-build _to_dict() eating the flag before the pending outbox update to an open tab flushes (→ that update serializes with preserved_props and the tab silently ignores the reassignment). Built an MRE, but the "production" framing was unfaithful, confirmed via a second-lineage pass over client.py/page.py/outbox.py: fresh Client per GET (no shared live element across viewers; root routes through the same per-request machinery via page('')._wrap(root)), and build_response() does self.outbox.updates.clear() before the full-build. So the only live-element caller of _to_dict() with a pending reassignment is the outbox flush, once. Safe. (Recording so nobody re-walks it.)

One latent note, not a blocker: _to_dict() is a serializer with a side effect (resetting _apply_anchors). Safe under today's per-request-client model, but it would silently break if _to_dict() were ever called speculatively on a live element (debug/repr, a future shared-client or resync path). A one-line comment on the setter/_to_dict — "correctness relies on exactly one outgoing serialization per deliberate reassignment" — would save the next person the walk. Take-or-leave.

✅ Also confirmed correct (so you don't re-verify)
  • preserved_props composes with ValueElement. MRO CodeMirror → KeyBindingElement → LineAnchorElement → ValueElement → …; ValueElement._to_dict hard-assigns ['value'], LineAnchorElement (outer) appends via setdefault(...).append('line-anchors') → ['value','line-anchors']. (Only fragile if the bases were ever reordered so ValueElement ran outermost — its hard-assign would clobber. Fine as-is.)
  • Fresh client applies declared anchors even if its build carries preserved_props — the nicegui.js if (this.elements[id] && …) guard skips preservation when the element doesn't exist yet.
  • Getter merge is clean — no dangling line_anchor_positions references in library/tests/docs.
  • Mixin extraction behavior-preserving — the shared defineRemappableRangeSet scaffold keeps the tooltip path green.

Nice work both of you — solid feature.

Jepson2k and others added 3 commits July 2, 2026 14:08
Covers the direction the intent flag exists for: reassigning the same
declared value must re-apply, which value comparison alone cannot detect.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
# Conflicts:
#	website/documentation/content/codemirror_documentation.py
applyLineAnchors dispatched the anchor effect (which re-arms the
tracker's debounce timer) and then emitted immediately, so every
deliberate assignment produced a duplicate emission ~50ms later.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@falkoschindler falkoschindler modified the milestones: Next, 3.15 Jul 4, 2026
@falkoschindler

Copy link
Copy Markdown
Contributor

Sorry, we didn't manage to land this in 3.15 either, so it has to move once more — but we'll firmly schedule it for 3.16. Thanks for bearing with us.

@falkoschindler falkoschindler modified the milestones: 3.15, 3.16 Jul 23, 2026
falkoschindler and others added 7 commits August 11, 2026 09:30
- fix version markers: line anchors land in 3.16.0, not 3.14.0
  (incl. moving the docstring section out of the 3.14.0 block)
- fix demo description: the button is called "Increment"
- drop useless `deep: true` from the lineAnchors watcher
- name the debounce constant (ANCHOR_DEBOUNCE_MS)
- remove dead `_anchorTimer = null` bookkeeping
- use screen.wait_for_js instead of a raw WebDriverWait
- reword an assert message to state the expectation
- one sentence per line in the mixin docstring

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
The prop held the *declared* lines while only `_anchor_positions` tracked the live
ones, so any re-render re-applied stale positions: switching away from a
tab_panels(keep_alive=False) panel and back snapped every anchor to where it was
declared, and the server mirror silently followed.

The browser-reported snapshot is now mirrored into the prop (via suspend_updates,
so it does not echo back), and beforeUnmount stores the live positions client-side
just like the editor value. A remount therefore restores where the anchors are, not
where they started.

This also replaces the one-shot `_apply_anchors` flag, which `_to_dict` consumed as
a side effect and which relied on exactly one outgoing serialization per assignment:
`_anchors_pending` is set by the setter and cleared when the browser confirms, so
`_to_dict` stays a pure serializer and a lost update heals on the next one.

- keep the prop unset until anchors are declared, so anchor-free editors pay nothing
- return the positions dict raw, like the neighboring line_tooltips getter

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
The tracker scheduled its debounced emit on every document change while any anchor
existed, so typing anywhere — even below every anchor — pushed an identical mapping
to the server once per debounce window and invoked every on_anchor_change handler
with unchanged values, contradicting the documented "whenever tracked anchor
positions change".

emitAnchorPositions now skips the emit when the mapping equals the last one sent.
A deliberate apply still forces it, because the server treats that emit as the
confirmation that its declared anchors have landed — which has to arrive even when
they happen to sit where the live ones already were.

The emission test moved to the public on_anchor_change API and to edits that really
move the anchors, so it pins the coalescing it is named after instead of the
emit-on-unchanged behaviour; the new sibling test covers edits that move nothing.

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
test_line_anchors_populates_mirror asserted exactly what the first two lines of
test_line_anchors_replace_and_clear already cover, so it is gone.

test_clear_after_typing_does_not_resurrect_anchors never reached the race it
described: the explicit clear only arrives in the browser after the 50 ms debounce
has already fired, so no stale timer is ever pending when it lands. Injecting the
full bug — a surviving timer that carries its scheduling-time snapshot — left the
test green; it only turned red once the debounce was stretched to 150 ms. What it
was meant to guard is carried by the timer callback reading live editor state
anyway, so the test is dropped rather than rebuilt around a deterministic race.

The remaining remapping cases share the shape "declare anchors, dispatch one
change, check the resulting mirror" and are now one parametrized test, extended
with a deletion above the anchor.

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
An anchor declared beyond the end of the document was moved to the last line, and
one below line 1 to the first, each with a browser-side warning. That invented a
position nobody asked for: declaring line 50 in a three-line document read back as
line 3, which looks plausible and is wrong, while the warning surfaced far away from
the assignment. The neighbouring line_tooltips already handles the same situation by
dropping the entry, so two adjacent APIs of one element disagreed.

Anchors outside the document are now dropped with a warning, like tooltips, so a
read never reports a position that was not applied — a dropped anchor is visible as
a missing key. A line below 1 needs no knowledge of the document to be recognised as
wrong, so it is rejected at assignment time with a ValueError that points at the
caller instead of appearing asynchronously in the browser log.

Both entry points validate before they touch anything: the setter before it changes
state, and the constructor before super().__init__ registers the element — otherwise
a rejected argument left a half-built editor sitting in the element tree, without a
language, a theme or its event listener.

The feature is unreleased, so this needs no deprecation path.

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Five of the demo's seven lines were string surgery — split, index, parse, rewrite,
join — around a feature that is a single expression: reading line_anchors gives you
a line number. Worse, the description invited the reader to "edit the code freely",
which broke the demo: changing the anchored line to anything not ending in a number
made int() raise inside the click handler, so the button silently stopped working.

The demo now declares an anchor and binds a label to it. Inserting lines above the
anchored one moves the reported number, which is the whole point, and nothing in it
can fail. The description also drops the RST double backticks it had picked up;
demo texts are rendered as Markdown.

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>

@falkoschindler falkoschindler left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for sticking with this, @Jepson2k — line anchors are a genuinely useful addition, and the foundation you picked (a remappable RangeSet whose positions the browser owns, with the server holding a mirror) turned out to be exactly right. Everything below builds on that.

Your fixes from @evnchn's round all check out: the last-anchor deletion notifies Python (the startState size check, with a test), the deletion wording matches MapMode.TrackDel, and both API suggestions landed — line_anchors is a {id: line} dict matching line_tooltips, and on_anchor_change= is a constructor parameter, so the demo no longer teaches the raw event string. The shared-element caveat is moot on 3.x, where every request gets a fresh client.

Rather than send you another list to work through, I reviewed the branch and pushed my follow-ups directly. Here is what changed and why, so nothing arrives as a surprise:

The remaining correctness bug was a remount, not a re-broadcast. The preserved_props route protects against unrelated prop updates, but the same root cause survived in a second form: mounted() re-applied the declared lines, and nothing ever wrote the live positions back into the prop. So any client-side remount — switching away from a ui.tab_panels(keep_alive=False) panel and back, or registering a handler at runtime, which makes NiceGUI re-render the element — snapped every anchor back to where it was declared, and the server mirror silently followed. The browser-reported snapshot is now mirrored into the prop (via suspend_updates, so it does not echo back), and beforeUnmount stores the live positions client-side, just like it already did for the editor value.

The one-shot apply flag became idempotent. _to_dict consumed a flag as a side effect, which only worked as long as there was exactly one outgoing serialization per assignment — an invariant spread across client.py and outbox.py rather than living where it was relied on. The flag is now set by the setter and cleared when the browser confirms, so _to_dict is a pure serializer again and a dropped update heals on the next one instead of being lost for the lifetime of the client.

on_anchor_change now means what it says. The tracker emitted on every document change while any anchor existed, so typing anywhere — even below every anchor — pushed an identical mapping once per debounce window and invoked every handler with unchanged values. The emit now compares against the last mapping sent; a deliberate apply still forces it, since the server treats that emit as confirmation that its declared anchors have landed.

Out-of-range anchors are dropped, not clamped. Declaring line 50 in a three-line document used to read back as line 3 — a plausible-looking position nobody asked for, with the warning surfacing far from the assignment — while the neighboring line_tooltips drops out-of-range entries, so two adjacent APIs of the same element disagreed. Anchors beyond the document are now dropped with a warning like tooltips, and a line below 1, which is wrong regardless of the document, is rejected at assignment time with a ValueError that points at the caller. The feature is unreleased, so no deprecation path was needed. (The version markers now say 3.16.0 — they still said 3.14.0, but both 3.14 and 3.15 shipped without the feature.)

Tests and demo. Two tests are gone: one asserted what its neighbor's first two lines already covered, and test_clear_after_typing_does_not_resurrect_anchors never reached the race it described — injecting the very bug it guards against left it green; it only turned red once the debounce was stretched to 150 ms. For the remount fix, the emission dedup and the constructor's validation order, I verified the new tests by reverting each fix and watching them fail. The demo lost five lines of string surgery that had nothing to do with anchors — and an instruction ("edit the code freely") that broke it, since editing the anchored line to anything not ending in a number made int() raise inside the click handler. It is now an anchor and a label bound to it.

The full suite is green on adcbbf2fe (1051 passed). Approving — nice work, and thanks for your patience through the review. ※

@evnchn
evnchn dismissed their stale review August 11, 2026 14:50

Dismissing my own review from 2026-06-14: it is stale. The points it raised were addressed, and the branch has moved substantially since (mixin extraction, remount fix, idempotent apply flag). Deferring to falkoschindler's approval on the current head.

@falkoschindler
falkoschindler added this pull request to the merge queue Aug 11, 2026
Merged via the queue into zauberzeug:main with commit 8fb479b Aug 11, 2026
8 checks passed
evnchn added a commit to evnchn/nicegui that referenced this pull request Aug 11, 2026
Upstream's line-anchor tracking (zauberzeug#5988) landed on the same spots this branch
touches. Three sites conflicted textually and one did not, which was the
dangerous one:

- `codemirror.py`: both sides added a mixin to the same base-class list.
  Kept both, with `LineAnchorElement` in upstream's position and
  `CrdtElement` last as before.
- `codemirror.js` props: both sides inserted after `highlightWhitespace`.
  Kept both.
- `codemirror.js` extensions: upstream added `anchorTracker`/`anchorField`
  next to `changeSender`, which this branch swaps for `yCollab` in CRDT mode.
  Only `changeSender` belongs inside that conditional — the anchor plugins
  read `anchorField` and emit on `docChanged` independently of `emitting`,
  so remote Yjs edits should remap anchors too. Both are now unconditional.
- `codemirror.js` `beforeUnmount()` merged cleanly and was still wrong: this
  branch's `if (this.crdtDocId) { ...; return; }` sat in front of upstream's
  new anchor snapshot and `clearTimeout(this._anchorTimer)`, so in CRDT mode
  a pending debounce survived unmount and fired `$emit` on a torn-down
  editor, and a client-side remount re-applied the declared anchor lines
  instead of the current ones. Restructured so the snapshot and the timer
  clear always run, with only the `value` write-back staying CRDT-exempt
  (yjs owns the document) and teardown moved after the state read.

The codemirror lockfile's only upstream move is a dev-only `brace-expansion`
bump that does not reach the bundle, so `dist/` needs no rebuild.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
pull Bot pushed a commit to codingwatching/nicegui that referenced this pull request Aug 12, 2026
…cket listener (zauberzeug#6282)

*Opened by Claude Code on @evnchn's behalf.*

### Motivation

`on_anchor_change()` (new in 3.16.0, merged in zauberzeug#5988) registers a
**second** socket listener on the same event
`LineAnchorElement.__init__` already listens to:

```python
self.on('anchor-positions', self._update_anchor_mirror)   # __init__
self.on('anchor-positions', lambda e: handle_event(...))  # on_anchor_change
```

Two consequences:

1. **Every anchor movement costs two round trips**, each carrying the
full anchors dict — working against the 50 ms debounce, whose stated
purpose is sparing high-latency connections.
2. **Registering a handler after the first render mutates the serialized
listener set**, so the late-registration path from PR 5439 fires: the
element is dropped and re-created, and the CodeMirror view is rebuilt.

<details>
<summary><b>Measured on the merged code (2 emits vs 1, with a
control)</b></summary>

Two `ui.codemirror`s, same document, same edit, differing only in
whether `on_anchor_change=` is passed. Socket `emit` instrumented in the
browser and listener ids mapped back to event names:

| element | `anchorPositions` listeners | `anchorPositions` emits per
edit | `update:value` emits |
|---|---|---|---|
| `on_anchor_change=…` | **2** | **2** | 1 |
| control | 1 | 1 | 1 |

`update:value` staying at 1 on both sides confirms the edit itself was
identical. Run against `8fb479bc` (the merge commit of PR 5988), Chrome
141.

</details>

### Implementation

Handlers move to a Python-side list dispatched from
`_update_anchor_mirror`, which is the pattern this codebase already
standardised on in PR 2704 / PR 2708 — and which both neighbours already
use: `ValueElement._change_handlers` and `KeyBindingElement`'s single
`self.on('keybinding', self._dispatch_keybinding)`.

One socket listener now serves any number of handlers, so runtime
registration is free and no longer touches the frontend listener set.

Two details worth flagging:

- **Ordering is now guaranteed rather than incidental.** Handlers run
after the mirror is updated because they are dispatched from it.
Previously that held only because `__init__` happened to register the
mirror listener first.
- **Each handler gets its own `dict`.** Previously each listener had its
own deserialized payload, so handlers could not see each other's
mutations. Passing the live `_anchor_positions` would have let a handler
rewrite the server mirror through `e.anchors`, so the dispatch copies
per handler — preserving the intent already documented on the line above
it.

No public API change: `on_anchor_change()` keeps its signature and
`Self` return.

<details>
<summary><b>Verification</b></summary>

Extended the nearest existing test (`test_on_anchor_change_handler`)
rather than adding a standalone one, asserting observable behaviour only
— that a handler registered after the first render fires, and that the
"Event listeners changed" warning does *not* appear. Per `AGENTS.md`, no
private attributes or patched machinery.

**Seen to fail.** With the fix reverted, the test fails with the
expected symptom in the log:

```
WARNING  nicegui:client.py:408 Event listeners changed after initial definition. Re-rendering affected elements.
FAILED tests/test_codemirror_line_anchors.py::test_on_anchor_change_handler
```

Restored → 12 passed. Local gates on `e41a3f13`:

| gate | result |
|---|---|
| `pre-commit` (changed files) | passed |
| `mypy ./nicegui` | no issues, 264 files |
| `pylint ./nicegui` | 10.00/10 |
| `pytest tests/test_codemirror_line_anchors.py tests/test_codemirror.py
-rs` | 31 passed, **0 skipped** |

Reviewed by Codex (different lineage), which raised the
shared-mutable-dict issue above; that is applied. It scored the result
8/10 and confirmed the premise was worth proposing.

</details>

### Progress

- [x] The PR title is a short phrase starting with a verb like "Add
...", "Fix ...", "Update ...", "Remove ...", etc.
- [x] The implementation is complete.
- [x] This PR does not address a security issue.
- [x] Pytests have been added/updated.
- [x] Documentation is not necessary — the docstring wording is aligned
with `ValueElement.on_value_change`, and behaviour is unchanged from the
user's side.
- [x] No breaking changes to the public API.


[※](https://zauberzeug.github.io/colophon/#m=claude-opus-5&t=Agent%20measured%20the%20behaviour%2C%20wrote%20the%20change%2C%20the%20test%20and%20this%20description%3B%20the%20decision%20to%20propose%20it%20is%20the%20author%27s.&p=agent&a=claude-code
"claude-opus-5: Agent measured the behaviour, wrote the change, the test
and this description; the decision to propose it is the author's.")


<details>
<summary><b>Runnable repro</b> — the script behind the table
above</summary>

Two editors, same document, same edit; only `on_anchor_change=` differs.
Run it, open the page, then paste the instrumentation below into the
browser console.

```python
from nicegui import ui

seen: list[dict] = []

a = ui.codemirror('l1\nl2\nl3\nl4\nl5', line_anchors={'x': 3},
                  on_anchor_change=lambda e: seen.append(e.anchors))  # 2 listeners
b = ui.codemirror('l1\nl2\nl3\nl4\nl5', line_anchors={'x': 3})        # control: 1 listener

ui.label().bind_text_from(a, 'id', lambda i: f'A(with handler) element id = {i}')
ui.label().bind_text_from(b, 'id', lambda i: f'B(control)       element id = {i}')

ui.run(reload=False)
```

```js
window.__ev = [];
const orig = window.socket.emit.bind(window.socket);
window.socket.emit = (name, payload, ...rest) => {
  if (name === 'event') window.__ev.push({id: payload.id, lid: payload.listener_id});
  return orig(name, payload, ...rest);
};
const comps = Object.values(mounted_app.$refs).filter(r => r && r.editor);
comps.forEach(c => c.editor.dispatch({changes: {from: 0, insert: 'NEW\n'}}));
// then map listener_id -> event name via mounted_app.elements[id].events
```

Observed against `8fb479bc`:

```
registered listeners  4: [update:value, anchorPositions, anchorPositions, keybinding]
                      5: [update:value, anchorPositions, keybinding]
emits per edit        4:anchorPositions = 2     5:anchorPositions = 1
                      4:update:value    = 1     5:update:value    = 1
```

`update:value` staying at 1 on both sides confirms the edit itself was
identical; only the anchor event doubles.

</details>

---
*Changelog — 2026-08-12: added the runnable repro behind the measurement
table.*

---------

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Co-authored-by: Falko Schindler <falko@zauberzeug.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature Type/scope: New or intentionally changed behavior review Status: PR is open and needs review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants