fix: md editor doing round-trip html-md-html on save#442
Closed
edgarsskore wants to merge 25 commits into
Closed
Conversation
Let markdown previews open linked notes, follow headings, and switch into a fullscreen editing workspace so note updates can happen without leaving the preview.
…tent height, reset view on fullscreen exit - Scope min-height to .markdown-workspace--edit only so preview mode shrinks to content - Set --content-height to 920px inline, 89vh in fullscreen - Reset editorView to 'markdown' when exiting fullscreen so inline doesn't stay in raw mode Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Resolved conflicts in src/ui/file-preview/src/app.ts where the markdown workspace state on the feature branch overlapped with the directory-tree browser added on main: - Kept both directoryBackPayload (main) and the markdown editor state vars (feature branch) at module scope. - Renamed feature branch's local parseDirectoryEntries helper to splitListingLines to avoid clashing with main's directory-tree parser of the same name. - Combined the renderApp panel-topbar additions: keeps backButton from main and hideSummaryRow conditional from the feature branch. - Removed a duplicate stripReadStatusLine that already comes via workspace-controller import on the feature branch. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…p stale state leaking across files Three related fixes for the markdown workspace inside the file preview: 1. Markdown link clicks were dead in edit mode. The old handler queried `.markdown-doc` synchronously, but in edit mode that element is created lazily inside `mountMarkdownEditor`, so the query returned null and the handler never attached. Switch to event delegation on the stable `.panel-content-wrapper` and scope to `.markdown-doc` via `closest`. The Cmd/Ctrl gate is no longer needed because preventDefault plus delegation handles contentEditable cleanly. 2. The 120ms artificial delay before the first render was a leftover from a removed cache-restore race. Render fresh `tool_result` payloads immediately so opens stop flashing a loading state. 3. Sidebar file switches showed the previous file's content because the sessionStorage cache (shared across all preview iframes that sit on the same parent origin, e.g. dc-app) was eagerly restored on init and beat the host's fresh `tool_result`. Replace the eager restore with a targeted `ontoolinput` match: stash the cached payload at `onConnected`, then in `ontoolinput` only render it if the host's announced file path matches the cache. Fresh `tool_result` always wins. Reopens of the same document still feel instant; switches to a different file no longer flash stale content. The 8s "Preview unavailable" fallback continues to surface an error if the host never re-sends `tool_result`. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…istency and partial read handling Major restructuring of the file preview UI: - Refactor markdown workspace into modular files under markdown/ directory - Extract document-layout, panel-actions, file-type-handlers, model, and payload-utils into standalone modules - Unify toolbar into a single consistent bar with conditional button visibility instead of separate code paths for markdown vs non-markdown files - Make all action bar buttons icon-only with tooltips for consistency - Fix button ordering: copy before open-in-folder across all file types Fix partial markdown read handling: - Partial reads show preview mode with expand/copy/open-in-folder buttons - Expanding to fullscreen loads full document and enters edit mode - Exiting fullscreen restores the original line range view - Re-reads the same line range from disk on exit so edits are reflected - refreshFromDisk now respects the original line range instead of always loading the full file (which was overwriting partial reads) Fix fullscreen/inline transitions: - Entering fullscreen from partial read triggers requestEditMode to load full document - Exiting fullscreen from full file read preserves saved content instead of reverting to original tool result - refreshFromDisk on initial load ensures content reflects disk state after page refresh for both partial and full reads Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The markdown WYSIWYG editor's HTML-to-markdown serialization corrupted badge images, links and formatting in the README. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…and preview after write/edit Make file preview work after write_file/edit_block by adding structuredContent to their responses and auto-fetching content when needed. Remove read-only preview mode so markdown always opens in the editor. Replace htmlToMarkdown serializer with block-aware patchMarkdownFromHtml that preserves original source via text diffing. Add source-level formatting, link hover popover with edit/open actions, and collapsible TOC sidebar. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…iew centering and link editing - Intercept Enter keydown to insert \n\n directly into raw source at cursor position, avoiding DOM serialization that could lose formatting or content - Fix edit_block preview centering: use known search position instead of indexOf(replacement) which could match wrong occurrence - Fix formatting/link insertion targeting wrong occurrence by restricting indexOf to the block containing the selection - Fix link edit creating duplicates: replace full [text](old-url) instead of just the visible text - Expose maybeAutosave on controller and call before display mode changes so edits save when clicking expand Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…Tiptap The contenteditable + HTML-to-markdown serialization approach was fundamentally unsound for the mixed content in our READMEs — serializeNode lost images inside anchor links, code fences, and nested HTML; LCS-based block alignment misfired and duplicated or dropped content on complex documents. Swap the markdown-view branch of mountMarkdownEditor to Tiptap (ProseMirror) with tiptap-markdown for round-tripping. Source of truth becomes a structured document instead of a rendered-HTML-we-reverse-engineer, so edits go through typed transactions that can only produce valid document states. Trade-off: first save normalizes the doc to tiptap-markdown's canonical form (asterisk vs underscore emphasis, bullet markers, paragraph reflow). After that, the saved file round-trips to itself and subsequent edits are stable. Also: - Remove the "newer version on disk" conflict prompt: refreshFromDisk only runs at mount (no file watcher), so disk-vs-payload mismatch can only mean the host sent a stale payload — just reload silently. - Remove unused @ts-expect-error in parser.ts (markdown-it types now present transitively via tiptap-markdown). - Enable minify in build-ui-runtime.cjs. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…after Tiptap swap
Post-Tiptap cleanup pass, mostly from a code review:
- controller.ts: refreshFromDisk was calling isMissingFileErrorResult(null)
after the !freshPayload guard, so the "file deleted" branch was dead.
Introduce callReadFile returning { rawResult, payload } and pass the raw
tool result to the error check; readPayload delegates to it.
- model.ts / controller.ts / test: pendingExternalPayload is only ever set
to null now (the sole writer was the removed disk-conflict notice branch).
Drop the field, the dead branch in revertEditing, and the extra clause in
isUndoAvailable. Remove the stale test assertion.
- linking.ts: add restoreWikiLinks as the inverse of rewriteWikiLinks, so
the mcp-wiki: title contract lives in one file. editor.ts uses it
directly; drop the trivial preprocessForTiptap alias.
- editor.ts: syncHeadingIds now skips no-op id/data-heading-id writes so
identical slugs don't dirty the style engine on every keystroke.
- editor.ts: drop redundant tiptap.commands.focus() calls in handleFormatClick
and handleBlockStyleChange — .chain().focus() already focuses.
- editor.ts: name the popover mouseenter/mouseleave, linkMode file/url click,
and popover hover handlers and remove them in destroy(). Fixes a listener
leak on Raw↔Preview view toggles.
- editor.ts: drop three WHAT comments.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…wrapper
Two fixes from code review:
- app.ts: needsContentRead now short-circuits on http(s):// paths. URL
payloads from read_file(isUrl:true) have no [Reading ...] marker, which
previously looked like "content missing — re-fetch" and triggered a
read_file({ path: url }) call without isUrl, producing a guaranteed-
failed tool round-trip on every URL render. Error was swallowed by
the catch but the round-trip was wasted work.
- controller.ts + app.ts: remove maybeAutosave wrapper and the autosave
trigger on display-mode changes. Clicking a mode-change button blurs
the editor first, and the editor's onBlur now calls saveDocument
directly (which self-gates on dirty/saving/fileDeleted). One save
path, no race with the editor remount that follows a mode change.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Wire 5 events through the existing ui-event-tracker so we can measure whether the new markdown editor is actually being used: - markdown_edit_started: first dirty onChange per mount (engagement) - markdown_saved: successful saveDocument; params: blocks count - markdown_save_failed: save catch path; params: reloaded_from_disk - markdown_view_toggled: raw ↔ preview switch - markdown_reverted: revertEditing All events carry file_extension via the shared analytics helper, matching the convention used elsewhere in file-preview. trackUiEvent is added to MarkdownControllerDependencies and wired from app.ts with a lazy closure — the controller is built at module load but the tracker is set up later inside bootstrapApp. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Commit 8fd8f94 ("feat(file-preview): always-edit markdown, source-preserving WYSIWYG, and preview after write/edit") changed handleEditBlock's exact-match path to return a file preview (status line + snippet of the edited region) plus structuredContent carrying fileName/filePath/fileType for the preview UI, replacing the old "Successfully applied N edit(s) to <path>" text message. Both test-edit-block-line-endings.js and test-file-handlers.js still asserted on the old string, so they failed after the refactor even though the edit itself was working correctly. Updates: - test-edit-block-line-endings.js: add assertEditBlockSuccess() helper checking content[0].type === 'text', structuredContent.filePath present, and the [Reading N lines from ...] preview header. Replace all 11 assertion sites. - test-file-handlers.js (Test 10): same check inline. The file-read-back assertion on the next line already verifies the edit landed, so the old text-match assertion was redundant signal anyway.
edit_block deliberately uses soft-failure returns (no isError flag) for cases the LLM should recover from — "Search content not found", fuzzy match below threshold, "Expected N occurrences but found M". These look like success from the MCP protocol level; the server returns isError: undefined and the client's existing assertSuccessfulEditBlockResult only threw on isError === true. Symptom: open a preview, something else modifies the file on disk, edit and save in the preview. Every edit_block call soft-fails (search string no longer on disk), but the editor reports "Saved", clears the dirty flag, advances fullDocumentContent to match the draft, and fires markdown_saved telemetry. Disk is untouched. Subsequent saves diff against the phantom fullDocumentContent and keep failing silently. Fix: successful edit_block always carries structuredContent with fileName/filePath/fileType (per commit 8fd8f94). Absence is a reliable signal of a soft-failure return. assertSuccessfulEditBlockResult now also throws when structuredContent is missing, surfacing the server's message as the error. Throwing routes these through saveDocument's existing catch, which already does the right thing: re-reads disk, compares to our fullDocumentContent, and if they differ reloads the payload with keepDraft: true and shows "Save failed. Reloaded the file from disk." If disk matches (non-staleness failure — e.g. expected_replacements mismatch on unchanged content), surfaces the server's message verbatim. Either way: draft preserved, dirty not cleared, save button remains actionable. Verified with a probe against the built dist: success still passes, "Search content not found" throws, count mismatch throws, disk untouched on soft-fail.
…tected on save
When edit_block fails because the file changed on disk (e.g. a concurrent
agent tool call or the user editing in another app), the previous fix
surfaced this as a small inline notice "Save failed. Reloaded the file
from disk." That message was easy to miss, and the passive swap of
content underneath the editor meant users often didn't realize what had
happened — they'd keep typing against content that had changed beneath
them.
Add a modal that appears specifically when a save attempt detected an
external change, giving the user two concrete actions:
Use disk version — discard draft, accept external changes
(syncStateFromContent without keepDraft).
Save my changes — re-run saveDocument. The earlier fix already
synced sourceContent to disk with keepDraft: true,
so computeEditBlocks now diffs draft vs. the
fresh disk content. Non-overlapping edits merge
in automatically; overlapping edits win over
disk on just the lines the user touched, leaving
the rest of the external change intact.
Dismiss (Esc / ✕) — neither action. Leaves the draft dirty and shows
a lighter inline note so the save button remains
intelligible.
The dialog only opens when reloadedFromDisk === true in saveDocument's
catch. Other save failures (e.g. expected_replacements count mismatch
on unchanged disk — not actually a conflict) keep the existing inline
error path — a modal would be overkill there.
Implementation notes:
- conflict-dialog.ts: self-contained module modeled after config-editor's
array-modal pattern. renderConflictDialogMarkup() emits hidden HTML;
createConflictDialogController() wires events and returns { open,
close, isOpen }. Minimal focus trap between the three buttons,
Escape/✕ cancel, click-outside deliberately does not dismiss
(conflict resolution is not something users should blow past by
accident), default focus on "Save my changes" as the non-destructive
intent.
- Markup is mounted once at document.body in bootstrapApp so that
position: fixed z-index works and app re-renders don't wipe it while
it's open.
- Callbacks guard against the user having navigated away during the
conflict: if workspaceState no longer matches the captured state,
mutations are skipped. Telemetry still fires.
- Three new ui events for funnel analysis:
markdown_save_conflict_shown
markdown_save_conflict_resolved (action: use_disk | save_mine | dismissed)
- CSS uses existing file-preview theme variables (--panel, --text,
--border, --text-secondary, --accent) so it inherits light/dark mode
without further work.
…ocks
Test 12 ("successful saves reset the undo baseline") was failing
because its edit_block mock returned only { content: [...] } — no
structuredContent. After commit d9c15a3 (detect edit_block soft-failures
by structuredContent absence), a success mock without structuredContent
looks indistinguishable from a soft-failure to the client, so the save
went through the catch branch instead of the success branch and the
draft never advanced to become the new baseline.
Fix: add structuredContent to the success mocks in Tests 11 and 12 to
match what the real handleEditBlock has returned since commit 8fd8f94.
Test 11 was also relying on the old lax contract — its first edit_block
mock didn't have structuredContent either. It was passing by accident
because the mock's side effect (diskContent = nextContent) ran before
the new assertion threw, so the observable final state happened to
match expectations anyway. Fixed proactively so the coincidence doesn't
unravel on the next change.
…st outcome
Previously, saveDocument ran all hunks in a single for-loop and threw on
the first edit_block that soft-failed. That aborted the rest of the
loop but the earlier hunks had already been written to disk. The UI
showed "Save failed. Reloaded the file from disk." — which was
misleading: disk had actually been partially overwritten, external
changes on the successful hunks were gone, and the user could not tell
which of their edits had landed.
Now each hunk is wrapped in its own try/catch. Failures are collected
as skippedHunks instead of bailing out. Post-loop:
appliedCount === total -> full success, as before.
appliedCount > 0 && skipped > 0 -> partial success. Tell the user
honestly: "N of M edits saved. K edits did not apply because that
text changed on disk — your draft still has them; save again to
merge." Status badge reads "Saved (partial)" instead of
"Save failed". Draft stays dirty; next save re-tries only the
remaining hunks against the fresh disk baseline.
appliedCount === 0 && skipped > 0 -> total failure and disk did
change. This is the case the conflict dialog was actually designed
for — nothing landed, so the keep-mine / use-disk choice is real.
The partial-success branch reuses the existing syncStateFromContent
pass (keepDraft: true) that already re-reads disk in the catch, so
fullDocumentContent reflects what disk actually has after the partial
writes, not what the editor wishfully thought it had.
New telemetry event markdown_save_partial { applied, skipped, total }
so we can see how often this happens in practice. Existing
markdown_save_failed now also carries applied/skipped/total.
app.ts: harden showConflictDialog dependency against the (unlikely)
case where the lambda fires before bootstrapApp has initialized the
controller — log a warning and invoke options.onCancel so the editor
still surfaces an inline note instead of silently no-op'ing.
test(markdown-preview) Test 11: the simulated-failure scenario now
resolves to partial-success instead of total-failure, so the assertion
was relaxed from exact 'File changed on disk' to the shared substring
'changed on disk' that both the partial-success message and the
onCancel/total-failure messages contain.
Add a 1-second debounced autosave in addition to the existing onBlur save. onChange in the markdown editor schedules a save 1s after the last keystroke; rapid typing resets the timer, so a single save runs once the user pauses. onBlur still fires save immediately and cancels any pending debounce. saveDocument cancels the debounce at start to cover non-debounce callers (conflict-dialog "save my changes", external saveDocument()). disposeHandles clears the timer so a pending save can't fire into a torn-down editor after remount. The AUTOSAVE_DEBOUNCE_MS constant is local to createMarkdownController so tuning the interval later is a one-line change. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
|
CodeAnt AI is reviewing your PR. Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
Contributor
📝 WalkthroughWalkthroughThis PR replaces the TipTap/markdown-it markdown editor and preview stack with CodeMirror + Lezer, removes wiki-link rewrite/restore and markdown-it rendering utilities, migrates outline extraction to Lezer, changes controller save/dirty/partial-payload flows to be race-aware and debounced, updates CSS for the new editor, and adjusts tests and package dependencies accordingly. Changes
Sequence DiagramsequenceDiagram
participant User
participant Editor as CodeMirror (EditorView)
participant Controller
participant Outline as Lezer Outline Extractor
participant Save as Save Handler
participant Disk
User->>Editor: Edit markdown
Editor->>Controller: onChange (docChanged, may suppress)
Controller->>Controller: Schedule debounced outline refresh
loop Debounce window
User->>Editor: More edits
Editor->>Controller: onChange
end
Controller->>Outline: extractMarkdownOutline(draftContent)
Note right of Outline: Lezer GFM parse tree traversal
Outline-->>Controller: heading items (with line numbers)
Controller->>Controller: compare/update TOC & activeHeadingId
User->>Editor: Trigger save (autosave/explicit)
Editor->>Controller: requestSave
Controller->>Save: capture savedContent snapshot, compute diff
Save->>Disk: write content
Disk-->>Save: success
Controller->>Controller: detect edits during save
alt No new edits
Controller-->>User: mark as saved
else New edits occurred
Controller->>Controller: mark dirty, schedule autosave
Controller-->>User: keep dirty state
end
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
codeant-ai
Bot
added
the
size:XXL
edgarsskore
changed the title
Contributor
|
CodeAnt AI is running the review. Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
![codeant-ai[bot]](https://avatars.githubusercontent.com/in/646884?s=60&v=4){kind=small}
Comment on lines
+237
to
+240
| outlineRefreshTimer = setTimeout(() => { | ||
| outlineRefreshTimer = null; | ||
| applyOutlineUpdate(state, extractMarkdownOutline(state.draftContent)); | ||
| }, OUTLINE_REFRESH_DEBOUNCE_MS); |
Contributor
There was a problem hiding this comment.
Suggestion: The debounced outline refresh captures a specific state object and later applies it unconditionally, which can refresh the TOC with stale data after state transitions. If workspace state changes before the timer fires, this callback can update outline/UI from an outdated draft. Gate the callback to the current workspace instance (or cancel when state changes) before applying updates. [race condition]
Severity Level: Major ⚠️
- ⚠️ Document outline headings can lag behind editor content.
- ⚠️ Active heading highlight may not match current cursor section.Steps of Reproduction ✅
1. With a markdown file open as in steps 1–2 above, `attachHandlers(payload)` mounts the
editor in `controller.ts:947-989`. The `onChange` handler at `controller.ts:961-982`
updates `state.draftContent` for the current `workspaceState` and calls
`scheduleOutlineRefresh(state)` (`controller.ts:974`), passing the current `state` object
by reference.
2. `scheduleOutlineRefresh` at `controller.ts:214-221` debounces outline updates by
clearing any existing `outlineRefreshTimer` and setting a new timeout
(`outlineRefreshTimer = setTimeout(...)` at line 237). The closure captures the specific
`state` object passed from `attachHandlers`, and after `OUTLINE_REFRESH_DEBOUNCE_MS` it
will call `applyOutlineUpdate(state, extractMarkdownOutline(state.draftContent))` at
`controller.ts:239-240`.
3. While this debounce timer is pending, `workspaceState` can be replaced by
`getState(payload)` when a new `RenderPayload` for the same file path but with different
content arrives (see autosave/save flow in suggestion 1): `buildBody(payload)` at
`controller.ts:338-369` calls `getState(payload)`; if `workspaceState.fullDocumentContent
!== cleanedContent`, `getState` reinitializes `workspaceState` to a new object at
`controller.ts:311-329`, leaving the previously captured `state` reference from
`scheduleOutlineRefresh` stale.
4. When the outline refresh timer eventually fires (`controller.ts:237-241`), it executes
`applyOutlineUpdate(state, extractMarkdownOutline(state.draftContent))` using the stale
`state` object instead of the current `workspaceState`. `applyOutlineUpdate`
(`controller.ts:222-230`) mutates that stale state's `outline` and `activeHeadingId` and
then calls `markdownTocHandle?.refresh(state.outline, state.activeHeadingId)`, causing the
document outline UI (attached in `attachHandlers` at `controller.ts:1013-1026`) to be
refreshed with headings derived from the outdated draft rather than the current editor
content and active state.Fix in Cursor | Fix in VSCode Claude
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** src/ui/file-preview/src/markdown/controller.ts
**Line:** 237:240
**Comment:**
*Race Condition: The debounced outline refresh captures a specific `state` object and later applies it unconditionally, which can refresh the TOC with stale data after state transitions. If workspace state changes before the timer fires, this callback can update outline/UI from an outdated draft. Gate the callback to the current workspace instance (or cancel when state changes) before applying updates.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
Comment on lines
+310
to
317
| if (!workspaceState || workspaceState.filePath !== payload.filePath || workspaceState.fullDocumentContent !== cleanedContent) { | ||
| const outline = extractMarkdownOutline(cleanedContent); | ||
| workspaceState = { | ||
| filePath: payload.filePath, | ||
| sourceContent: cleanedContent, | ||
| fullDocumentContent: cleanedContent, | ||
| draftContent: cleanedContent, | ||
| outline, | ||
| mode: 'edit', | ||
| dirty: false, |
Contributor
There was a problem hiding this comment.
Suggestion: Reinitializing workspace state whenever incoming payload content differs from fullDocumentContent will discard in-memory draft edits and reset dirty to false on rerender. If the host sends a refreshed payload while the user has unsaved changes, this branch recreates state from disk content and loses the draft. Preserve the existing state when there are unsaved edits instead of recreating it purely on content mismatch. [logic error]
Severity Level: Critical 🚨
- ❌ Markdown editor can drop unsaved text during autosave refresh.
- ⚠️ Users may lose recent keystrokes without any conflict warning.Steps of Reproduction ✅
1. Open a markdown file in the File Preview app so the host calls the `read_file` tool and
delivers a `RenderPayload` to `app.ontoolresult` at
`src/ui/file-preview/src/app.ts:247-258`. This resolves into
`renderAndSync(getEffectiveIncomingPayload(payload))`, which calls `renderApp(container,
payload, ...)` at `app.ts:143-151`.
2. In `renderApp` (`app.ts:201-239`), when `payload.fileType === 'markdown'`, the markdown
body is rendered via `renderPayloadBody` (`file-type-handlers.ts:115-122`), which in turn
calls `markdownController.buildBody(payload)` (`file-type-handlers.ts:69-80`). `buildBody`
in `controller.ts:338-369` calls `getState(payload)` at `controller.ts:339`, initializing
`workspaceState` with `fullDocumentContent` and `draftContent` equal to the cleaned
payload content (`controller.ts:311-318`).
3. The editor DOM is then wired up by `markdownController.attachHandlers(payload)` in
`renderApp` at `app.ts:241-243`, which calls `attachHandlers` in `controller.ts:940-1032`.
Inside `attachHandlers`, `mountMarkdownEditor` is invoked (`controller.ts:950-988`) with
an `onChange` handler at `controller.ts:961-982` that updates `state.draftContent`, sets
`state.dirty = value !== state.fullDocumentContent`, and calls `scheduleAutosave()`
(`controller.ts:198-205`), creating unsaved in-memory edits (`dirty === true`,
`draftContent !== fullDocumentContent`).
4. After the debounce period, `scheduleAutosave`'s timer fires and calls `saveDocument()`
at `controller.ts:699`. Inside `saveDocument`, per hunk, the code issues
`dependencies.callTool('edit_block', ...)` at `controller.ts:729-735`. Each `edit_block`
result is handled globally by `app.ontoolresult` (`app.ts:247-260`), which detects that
the tool result text lacks a read status line and therefore calls `readAndResolvePayload`
(`app.ts:164-181`). `readAndResolvePayload` uses
`markdownController.readPayload(filePath)` (`controller.ts:285-287`) to invoke the
`read_file` tool again; the resulting `RenderPayload` (with updated on-disk content for
the same `filePath`) is passed back into `renderAndSync(getEffectiveIncomingPayload(p))`
(`app.ts:508-513`), which re-invokes `renderApp` and then
`markdownController.buildBody(payload)` for the updated payload. At this point
`workspaceState` still refers to the previous in-memory state and is still `dirty`, but
the incoming `cleanedContent = stripReadStatusLine(payload.content)` now reflects the
newer on-disk content. The condition at `controller.ts:311`
(`workspaceState.fullDocumentContent !== cleanedContent`) is true, so `getState`
reinitializes `workspaceState` with `draftContent: cleanedContent` and `dirty: false`
(`controller.ts:311-318`), discarding any unsaved edits held only in the previous
`workspaceState` (including keystrokes typed while autosave/saveDocument was in progress).Fix in Cursor | Fix in VSCode Claude
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** src/ui/file-preview/src/markdown/controller.ts
**Line:** 310:317
**Comment:**
*Logic Error: Reinitializing workspace state whenever incoming payload content differs from `fullDocumentContent` will discard in-memory draft edits and reset `dirty` to false on rerender. If the host sends a refreshed payload while the user has unsaved changes, this branch recreates state from disk content and loses the draft. Preserve the existing state when there are unsaved edits instead of recreating it purely on content mismatch.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
Comment on lines
+1195
to
+1197
| if (button.id === 'link-popover-open') { | ||
| void options.onOpenLink?.(link.href); | ||
| return; |
Contributor
There was a problem hiding this comment.
Suggestion: Opening links directly from editor popover forwards raw href values (including non-http schemes) to the navigation path without scheme validation. Combined with external-link handling, this allows dangerous URLs like javascript: to be executed. Restrict allowed schemes (e.g., http/https/mailto) before invoking link opening. [security]
Severity Level: Critical 🚨
- ❌ File preview can open javascript: links from markdown content.
- ❌ Host executes attacker-controlled URLs without scheme allowlisting.
- ⚠️ Untrusted repositories can weaponize markdown links for abuse.Steps of Reproduction ✅
1. Open the File Preview app (`src/ui/file-preview/src/app.ts:1-200`) on any markdown
payload so that `markdownController` is constructed at `app.ts:116-144` and used to render
the document via `renderPayloadBody()` (`file-type-handlers.ts:16-23`).
2. Ensure the markdown content being rendered contains a link with a dangerous scheme,
e.g. `[Click me](javascript:alert(1))`. This content is loaded into the markdown editor
when `attachHandlers()` in `src/ui/file-preview/src/markdown/controller.ts:41-88` calls
`mountMarkdownEditor({ value: state.draftContent, ... })` at `controller.ts:51-58`.
3. In the rendered markdown editor (CodeMirror preview), hover the mouse over the `Click
me` link so the link popover is shown, then click the "Open link" button in that popover.
This triggers `handleLinkPopoverClick()` in
`src/ui/file-preview/src/markdown/editor.ts:1180-1203`, which, when `button.id ===
'link-popover-open'` (`editor.ts:1195-1197`), calls `options.onOpenLink(link.href)` with
the raw `href` value `javascript:alert(1)`.
4. The `onOpenLink` callback was wired in `attachHandlers()` (`controller.ts:59-61`) to
`navigateLink(payload, href)`. `navigateLink()` in
`src/ui/file-preview/src/markdown/controller.ts:12-31` passes this `href` to
`resolveMarkdownLink()` in `src/ui/file-preview/src/markdown/linking.ts:138-62`, which
uses `isExternalHref()` at `linking.ts:120-121` to treat any `scheme:` (including
`javascript:`) as external. For `javascript:alert(1)`, `resolvedLink.kind === 'external'`
and `navigateLink()` calls `dependencies.openExternalLink(resolvedLink.url)` or falls back
to `window.open(resolvedLink.url, '_blank', 'noopener')` (`controller.ts:25-29`), causing
the unsafe `javascript:` URL to be executed/handed off without any scheme allowlist.Fix in Cursor | Fix in VSCode Claude
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** src/ui/file-preview/src/markdown/editor.ts
**Line:** 1195:1197
**Comment:**
*Security: Opening links directly from editor popover forwards raw `href` values (including non-http schemes) to the navigation path without scheme validation. Combined with external-link handling, this allows dangerous URLs like `javascript:` to be executed. Restrict allowed schemes (e.g., http/https/mailto) before invoking link opening.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
Contributor
Sequence DiagramThis diagram shows how the file preview now uses a source-backed CodeMirror editor to edit markdown, sending minimal edit blocks to the backend while keeping the raw markdown as the canonical state and preserving newer unsaved edits if an older autosave completes later. sequenceDiagram
participant User
participant FilePreviewUI
participant MarkdownController
participant MarkdownEditor
participant ToolsBackend
User->>FilePreviewUI: Open markdown file in edit mode
FilePreviewUI->>MarkdownController: Render editor for payload
MarkdownController-->>FilePreviewUI: Editor shell HTML with source-based editor
FilePreviewUI->>MarkdownEditor: Mount editor with markdown source and callbacks
User->>MarkdownEditor: Edit markdown text
MarkdownEditor-->>MarkdownController: onChange(newText)
MarkdownController->>ToolsBackend: Autosave minimal edit blocks from baseline to snapshot
ToolsBackend-->>MarkdownController: Edits applied to disk
MarkdownController-->>FilePreviewUI: Update baseline and outline while keeping newer unsaved draft dirty
Generated by CodeAnt AI |
Comment on lines
+335
to
+452
| const spanningInlineRanges = collectSpanningWrapperRanges(); | ||
|
|
||
| const collectInlineRanges = (text: string, lineFrom: number): MarkerRange[] => { | ||
| const ranges: MarkerRange[] = []; | ||
| for (const match of text.matchAll(/\[([^\]\n]+)\]\(([^)\n]+)\)/g)) { | ||
| const start = match.index ?? 0; | ||
| const label = match[1] ?? ''; | ||
| if (text[start - 1] === '!' || label.startsWith('![')) { | ||
| continue; | ||
| } | ||
| addMark(ranges, lineFrom, start, start + 1, 'cm-md-hidden-marker'); | ||
| addMark(ranges, lineFrom, start + 1, start + 1 + label.length, 'cm-md-link-text'); | ||
| addMark(ranges, lineFrom, start + 1 + label.length, start + match[0].length, 'cm-md-hidden-marker'); | ||
| } | ||
| for (const match of text.matchAll(/\[\[([^\]\n]+)\]\]/g)) { | ||
| const start = match.index ?? 0; | ||
| const body = match[1] ?? ''; | ||
| const pipeIndex = body.lastIndexOf('|'); | ||
| const labelStart = pipeIndex >= 0 ? pipeIndex + 1 : 0; | ||
| const label = body.slice(labelStart); | ||
| addMark(ranges, lineFrom, start, start + 2, 'cm-md-hidden-marker'); | ||
| if (labelStart > 0) { | ||
| addMark(ranges, lineFrom, start + 2, start + 2 + labelStart, 'cm-md-hidden-marker'); | ||
| } | ||
| addMark(ranges, lineFrom, start + 2 + labelStart, start + 2 + labelStart + label.length, 'cm-md-link-text'); | ||
| addMark(ranges, lineFrom, start + 2 + labelStart + label.length, start + match[0].length, 'cm-md-hidden-marker'); | ||
| } | ||
| for (const match of text.matchAll(/(`+)([^`\n]+)\1/g)) { | ||
| const start = match.index ?? 0; | ||
| const ticks = match[1]?.length ?? 1; | ||
| addMark(ranges, lineFrom, start, start + ticks, 'cm-md-hidden-marker'); | ||
| addMark(ranges, lineFrom, start + ticks, start + match[0].length - ticks, 'cm-md-inline-code-text'); | ||
| addMark(ranges, lineFrom, start + match[0].length - ticks, start + match[0].length, 'cm-md-hidden-marker'); | ||
| } | ||
| for (const match of text.matchAll(/\*\*([^*\n]+)\*\*/g)) { | ||
| const start = match.index ?? 0; | ||
| addMark(ranges, lineFrom, start, start + 2, 'cm-md-hidden-marker'); | ||
| addMark(ranges, lineFrom, start + 2, start + match[0].length - 2, 'cm-md-strong-text'); | ||
| addMark(ranges, lineFrom, start + match[0].length - 2, start + match[0].length, 'cm-md-hidden-marker'); | ||
| } | ||
| const headingId = nextSlug(text); | ||
| if (heading.id !== headingId) { | ||
| heading.id = headingId; | ||
| for (const match of text.matchAll(/~~([^~\n]+)~~/g)) { | ||
| const start = match.index ?? 0; | ||
| addMark(ranges, lineFrom, start, start + 2, 'cm-md-hidden-marker'); | ||
| addMark(ranges, lineFrom, start + 2, start + match[0].length - 2, 'cm-md-strike-text'); | ||
| addMark(ranges, lineFrom, start + match[0].length - 2, start + match[0].length, 'cm-md-hidden-marker'); | ||
| } | ||
| if (heading.getAttribute('data-heading-id') !== headingId) { | ||
| heading.setAttribute('data-heading-id', headingId); | ||
| for (const match of text.matchAll(/(^|[^*])\*([^*\n]+)\*/g)) { | ||
| const start = (match.index ?? 0) + (match[1]?.length ?? 0); | ||
| addMark(ranges, lineFrom, start, start + 1, 'cm-md-hidden-marker'); | ||
| addMark(ranges, lineFrom, start + 1, start + match[0].length - (match[1]?.length ?? 0) - 1, 'cm-md-emphasis-text'); | ||
| addMark(ranges, lineFrom, start + match[0].length - (match[1]?.length ?? 0) - 1, start + match[0].length - (match[1]?.length ?? 0), 'cm-md-hidden-marker'); | ||
| } | ||
| return ranges.sort((left, right) => left.from - right.from || left.to - right.to); | ||
| }; | ||
|
|
||
| for (const { from, to } of view.visibleRanges) { | ||
| let line = view.state.doc.lineAt(from); | ||
| while (line.from <= to) { | ||
| const text = line.text; | ||
| const headingMatch = /^(#{1,6})\s+/.exec(text); | ||
| let className = ''; | ||
| const markerRanges: MarkerRange[] = []; | ||
| if (headingMatch) { | ||
| className = `cm-md-heading cm-md-heading-${headingMatch[1].length}`; | ||
| addMark(markerRanges, line.from, 0, headingMatch[0].length, 'cm-md-hidden-marker'); | ||
| } else if (BLOCKQUOTE_PREFIX.test(text)) { | ||
| className = 'cm-md-quote'; | ||
| const marker = text.match(BLOCKQUOTE_PREFIX); | ||
| addMark(markerRanges, line.from, 0, marker?.[0].length ?? 0, 'cm-md-hidden-marker'); | ||
| } else { | ||
| const unorderedMatch = UNORDERED_LIST_PREFIX.exec(text); | ||
| if (unorderedMatch) { | ||
| className = 'cm-md-list cm-md-list-unordered'; | ||
| const markerStart = unorderedMatch[1].length; | ||
| markerRanges.push({ | ||
| from: line.from + markerStart, | ||
| to: line.from + markerStart + 1, | ||
| widget: SHARED_BULLET_WIDGET, | ||
| }); | ||
| } else if (ORDERED_LIST_PREFIX.test(text)) { | ||
| className = 'cm-md-list cm-md-list-ordered'; | ||
| } else if (HORIZONTAL_RULE_LINE.test(text)) { | ||
| className = 'cm-md-rule'; | ||
| } | ||
| } | ||
|
|
||
| if (className) { | ||
| builder.add(line.from, line.from, Decoration.line({ class: className })); | ||
| } | ||
| markerRanges.push(...spanningInlineRanges.filter((range) => range.to > line.from && range.from < line.to)); | ||
| markerRanges.push(...collectInlineRanges(text, line.from)); | ||
| for (const range of markerRanges.sort((left, right) => left.from - right.from || left.to - right.to)) { | ||
| if (range.widget) { | ||
| builder.add(range.from, range.to, Decoration.replace({ widget: range.widget })); | ||
| } else if (range.className) { | ||
| builder.add(range.from, range.to, Decoration.mark({ class: range.className })); | ||
| } | ||
| } | ||
| if (line.to >= to || line.number >= view.state.doc.lines) { | ||
| break; | ||
| } | ||
| line = view.state.doc.line(line.number + 1); | ||
| } | ||
| } | ||
| return builder.finish(); | ||
| } | ||
|
|
||
| const markdownLinePreviewPlugin = ViewPlugin.fromClass(class { | ||
| decorations: DecorationSet; | ||
|
|
||
| constructor(view: EditorView) { | ||
| this.decorations = buildMarkdownLineDecorations(view); | ||
| } | ||
|
|
||
| update(update: { docChanged: boolean; viewportChanged: boolean; view: EditorView }) { | ||
| if (update.docChanged || update.viewportChanged) { | ||
| this.decorations = buildMarkdownLineDecorations(update.view); | ||
| } |
Contributor
There was a problem hiding this comment.
Suggestion: Decoration rebuilding does a full-document scan on every document or viewport change and then re-filters those global ranges for each visible line, which creates heavy O(document_size × visible_lines/ranges) work in a hot path. On large markdown files this can cause typing and scrolling lag. Restrict wrapper scanning to visible ranges or maintain incremental state instead of recomputing globally each update. [performance]
Severity Level: Major ⚠️
- ⚠️ Typing in markdown preview lags on large documents.
- ⚠️ Scrolling markdown preview stutters for big notes.
- ⚠️ Markdown preview decoration work dominates hot update path.Steps of Reproduction ✅
1. Open a large markdown file (thousands of lines) in the UI so that `attachHandlers()` in
`src/ui/file-preview/src/markdown/controller.ts:26-91` mounts the editor by calling
`mountMarkdownEditor()` at `controller.ts:36-73` with `view: 'markdown'`.
2. The markdown editor created in `mountMarkdownEditor()`
(`src/ui/file-preview/src/markdown/editor.ts:458-379`) includes the
`markdownLinePreviewPlugin` extension (`editor.ts:442-456`) which constructs its
decorations via `buildMarkdownLineDecorations(view)` at `editor.ts:260-440`.
3. On each document edit or viewport scroll, the plugin `update()` method at
`editor.ts:449-452` is called with `docChanged` or `viewportChanged` true; it recomputes
`this.decorations = buildMarkdownLineDecorations(update.view)` for every such update.
4. Inside `buildMarkdownLineDecorations()`, `collectSpanningWrapperRanges()` at
`editor.ts:284-333` first converts the entire document to a string (`const source =
view.state.doc.toString();`) and linearly scans it to compute `spanningInlineRanges`
(`editor.ts:335`), and then for each visible line in `view.visibleRanges` it filters these
global ranges (`markerRanges.push(...spanningInlineRanges.filter(...))` at
`editor.ts:424`) and builds per-line decorations. On large documents, this
O(document_size) scan and per-visible-line filtering on every keystroke or scroll causes
measurable typing and scrolling lag in the markdown preview editor.Fix in Cursor | Fix in VSCode Claude
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** src/ui/file-preview/src/markdown/editor.ts
**Line:** 335:452
**Comment:**
*Performance: Decoration rebuilding does a full-document scan on every document or viewport change and then re-filters those global ranges for each visible line, which creates heavy O(document_size × visible_lines/ranges) work in a hot path. On large markdown files this can cause typing and scrolling lag. Restrict wrapper scanning to visible ranges or maintain incremental state instead of recomputing globally each update.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
Comment on lines
1046
to
1066
| } else if (selectedLinkItem) { | ||
| const selectedHeadingId = linkHeadingSelect?.value?.trim(); | ||
| const selectedHeadingText = linkHeadingSelect?.selectedOptions[0]?.dataset.headingText?.trim(); | ||
| const alias = linkAliasInput?.value?.trim(); | ||
| const pathPart = selectedLinkItem.path === options.currentFilePath ? '' : selectedLinkItem.wikiPath; | ||
| const wikiLink = `[[${pathPart}${selectedHeadingId ? `#${selectedHeadingId}` : ''}${alias ? `|${alias}` : ''}]]`; | ||
| const href = `${selectedLinkItem.relativePath}${selectedHeadingId ? `#${selectedHeadingId}` : ''}`; | ||
| const label = alias || selectedHeadingText || selectedLinkItem.title; | ||
| const { from, to, empty } = tiptap.state.selection; | ||
| const insertChain = tiptap.chain().focus(); | ||
| if (!empty) { | ||
| insertChain.deleteRange({ from, to }); | ||
| const existingLink = findEnclosingMarkdownLink(); | ||
| if (existingLink) { | ||
| view.dispatch({ | ||
| changes: { from: existingLink.from, to: existingLink.to, insert: wikiLink }, | ||
| selection: EditorSelection.range(existingLink.from + 2, existingLink.from + 2 + label.length), | ||
| userEvent: 'input', | ||
| }); | ||
| view.focus(); | ||
| window.requestAnimationFrame(() => positionToolbar()); | ||
| } else { | ||
| insertText(wikiLink, 2, 2 + label.length); | ||
| } | ||
| insertChain.insertContent({ | ||
| type: 'text', | ||
| text: label, | ||
| marks: [{ | ||
| type: 'link', | ||
| attrs: { | ||
| href, | ||
| title: `mcp-wiki:${encodeURIComponent(wikiLink)}`, | ||
| }, | ||
| }], | ||
| }).run(); | ||
| } | ||
| closeLinkModal(); |
Contributor
There was a problem hiding this comment.
Suggestion: Editing an existing wiki link through the link modal can silently do nothing because apply only executes when selectedLinkItem is set, but opening the modal for an existing link clears that selection. This makes "edit link" fail unless the user re-runs search and re-selects a file. Pre-populate the selected item from the existing wiki link (or allow apply when an existing link is being edited). [logic error]
Severity Level: Major ⚠️
- ⚠️ Editing existing wiki links via popover has no effect.
- ⚠️ Users must re-search/reselect file to update wiki link.
- ⚠️ Markdown note editing UX is inconsistent between URL and wiki links.Steps of Reproduction ✅
1. Open any markdown file in the UI so that the markdown editor is mounted via
`attachHandlers()` in `src/ui/file-preview/src/markdown/controller.ts:26-36`, which calls
`mountMarkdownEditor()` at `controller.ts:36-73` with `view: state.editorView` set to
`'markdown'`.
2. In the markdown editor created by `mountMarkdownEditor()`
(`src/ui/file-preview/src/markdown/editor.ts:458-379`), ensure the document contains a
wiki link such as `[[Some/Note#heading|Alias]]`; the inline link parsing for wiki links is
handled by `findMarkdownLinkInLine()` at `editor.ts:937-973`.
3. Hover the wiki link in the markdown editor until the link popover appears, then click
the "Edit link" button; this triggers `handleLinkPopoverClick()` at `editor.ts:1266-1285`,
which calls `openLinkModalForSelection(existingLink)` at `editor.ts:992-1024` with
`existingLink.kind === 'wiki'`.
4. Observe that `openLinkModalForSelection()` switches the modal to file mode and
explicitly clears `selectedLinkItem` and `linkSearchResults` (`editor.ts:999-1021`), so
when you click "Insert" in the modal, `handleLinkApply()` at `editor.ts:1026-1066` runs
the `else if (selectedLinkItem)` wiki-link branch only if a file search result is
selected. Because `selectedLinkItem` is always `null` when editing an existing wiki link
(unless you manually run a new search and select a result), the wiki-link branch is
skipped and only `closeLinkModal()` at `editor.ts:1066` executes. The modal closes without
dispatching any `view.dispatch()` changes, so alias/heading edits to an existing wiki link
are silently dropped.Fix in Cursor | Fix in VSCode Claude
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** src/ui/file-preview/src/markdown/editor.ts
**Line:** 1046:1066
**Comment:**
*Logic Error: Editing an existing wiki link through the link modal can silently do nothing because apply only executes when `selectedLinkItem` is set, but opening the modal for an existing link clears that selection. This makes "edit link" fail unless the user re-runs search and re-selects a file. Pre-populate the selected item from the existing wiki link (or allow apply when an existing link is being edited).
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
Contributor
|
CodeAnt AI finished running the review. Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
Contributor
|
CodeAnt AI is running the review. Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
codeant-ai
Bot
added
size:XXL
Contributor
Sequence DiagramThis PR replaces the WYSIWYG markdown editor with a CodeMirror-based source editor that keeps raw markdown as the canonical state, adds live preview formatting, and saves changes via minimal diffs with safer autosave behavior. sequenceDiagram
participant User
participant FilePreviewUI
participant MarkdownController
participant MarkdownEditor
participant ToolsBackend
User->>FilePreviewUI: Open markdown file
FilePreviewUI->>MarkdownController: Initialize workspace from markdown source
MarkdownController->>MarkdownEditor: Mount source editor with live preview styling
User->>MarkdownEditor: Edit markdown text and interact with links
MarkdownEditor-->>MarkdownController: Report content changes and link events
MarkdownController->>MarkdownController: Debounce outline refresh and autosave
MarkdownController->>ToolsBackend: Send edit_block diff between baseline and draft
ToolsBackend-->>MarkdownController: Confirm applied edits and return updated file info
MarkdownController-->>FilePreviewUI: Update baseline content, outline, copy text, and save status
Generated by CodeAnt AI |
Contributor
|
CodeAnt AI finished running the review. Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
Contributor
|
CodeAnt AI is running the review. Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
codeant-ai
Bot
added
size:XXL
Contributor
Sequence DiagramThis PR replaces the WYSIWYG markdown stack with a CodeMirror-based source editor and updates the controller so saves apply diff blocks to the file while keeping the raw markdown as the canonical state and preserving newer unsaved edits when older saves finish. sequenceDiagram
participant User
participant FilePreview
participant MarkdownController
participant MarkdownEditor
participant Tools
User->>FilePreview: Open markdown file in preview
FilePreview->>MarkdownController: Build markdown workspace
MarkdownController->>MarkdownEditor: Mount source-backed editor with markdown text
User->>MarkdownEditor: Edit markdown content
MarkdownEditor-->>MarkdownController: onChange updated text
MarkdownController->>MarkdownController: Mark draft dirty and debounce outline refresh
MarkdownController->>Tools: Autosave edit_block diffs based on baseline
Tools-->>MarkdownController: Report applied edits to disk
MarkdownController->>FilePreview: Update baseline content and save status
alt Disk changed during save
Tools-->>MarkdownController: Edit error due to stale content
MarkdownController->>MarkdownController: Refresh disk baseline, keep newer draft
MarkdownController-->>User: Show conflict dialog to choose disk or my changes
end
Generated by CodeAnt AI |
| }); | ||
| } | ||
|
|
||
| const tocShell = document.querySelector('.document-outline-shell') as HTMLElement | null; |
Contributor
There was a problem hiding this comment.
🟠 Architect Review — HIGH
In fullscreen markdown edit mode, the table of contents no longer tracks the active heading while scrolling because attachDocumentOutline still relies on DOM heading elements with ids, but the new CodeMirror-based editor no longer renders such elements. As a result, the scroll listener in attachDocumentOutline never finds headings and the TOC highlight becomes stale unless the user explicitly clicks a TOC item.
Suggestion: Replace the scroll-based DOM heading detection with editor-driven active-heading updates: use the outline line numbers plus the editor's scroll/selection state (via the MarkdownEditorHandle or CodeMirror APIs) to compute the currently active heading and pass that back into attachDocumentOutline.refresh, instead of relying on document.getElementById and the outer scroll container.
Fix in Cursor | Fix in VSCode Claude
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is an **Architect / Logical Review** comment left during a code review. These reviews are first-class, important findings — not optional suggestions. Do NOT dismiss this as a 'big architectural change' just because the title says architect review; most of these can be resolved with a small, localized fix once the intent is understood.
**Path:** src/ui/file-preview/src/markdown/controller.ts
**Line:** 1038:1051
**Comment:**
*HIGH: In fullscreen markdown edit mode, the table of contents no longer tracks the active heading while scrolling because `attachDocumentOutline` still relies on DOM heading elements with `id`s, but the new CodeMirror-based editor no longer renders such elements. As a result, the scroll listener in `attachDocumentOutline` never finds headings and the TOC highlight becomes stale unless the user explicitly clicks a TOC item.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
If a suggested approach is provided above, use it as the authoritative instruction. If no explicit code suggestion is given, you MUST still draft and apply your own minimal, localized fix — do not punt back with 'no suggestion provided, review manually'. Keep the change as small as possible: add a guard clause, gate on a loading state, reorder an await, wrap in a conditional, etc. Do not refactor surrounding code or expand scope beyond the finding.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
| const selectedText = existingLink?.label ?? getSelectedText().trim(); | ||
| linkModal.removeAttribute('hidden'); | ||
| updateLinkMode('url'); | ||
| updateLinkMode(existingLink?.kind === 'wiki' ? 'file' : 'url'); |
Contributor
There was a problem hiding this comment.
Suggestion: Editing an existing wikilink opens file mode but clears all selected file state, while apply only executes the wikilink branch when a file is selected. If the user clicks "Insert" without re-searching and reselecting, the operation silently no-ops instead of updating the link, breaking the edit flow. Prepopulate selectedLinkItem for existing wikilinks or allow apply to update from parsed existing link data. [logic error]
Severity Level: Major ⚠️
- ❌ Wikilink edit popover can't update existing link.
- ⚠️ Alias changes silently discarded when editing wikilinks.
- ⚠️ Markdown editor link UX inconsistent between URL, wiki.Steps of Reproduction ✅
1. Open a markdown file that contains an existing wikilink (e.g.
`[[notes/Example#heading|Alias]]`) in the UI; `attachHandlers()` at
`src/ui/file-preview/src/markdown/controller.ts:26-36` mounts the markdown editor via
`mountMarkdownEditor()` at `src/ui/file-preview/src/markdown/editor.ts:458-469` with
`view: 'markdown'` and `currentFilePath` set.
2. In the editor, hover the existing wikilink so `handleLinkMouseMove()` at
`editor.ts:1248-1259` finds it via `findMarkdownLinkAtPosition()` (`editor.ts:976-981`)
and `showLinkPopover()` (`editor.ts:1201-1246`) displays the popover containing "Edit
link" and "Open link" buttons.
3. Click the "Edit link" button; `handleLinkPopoverClick()` at `editor.ts:1266-1283`
detects `button.id === 'link-popover-edit'` and calls `openLinkModalForSelection(link)`
(`editor.ts:992-1024`), which sets the selection to the link (`editor.ts:996-998`), opens
the modal, switches to file mode for wikilinks via `updateLinkMode(existingLink?.kind ===
'wiki' ? 'file' : 'url')` at `editor.ts:1001`, but then clears all file selection state by
setting `linkSearchResults = []`, `selectedLinkItem = null`, and resetting the results
message at `editor.ts:1019-1021` before calling `setLinkHeadingOptions()` and
`renderLinkResults()` at `editor.ts:1022-1023`.
4. Without typing a new search or clicking any search result (so `selectedLinkItem`
remains `null`), edit the alias or just click "Insert"; `handleLinkApply()` at
`editor.ts:1026-1065` runs with `linkMode === 'file'` but skips the wikilink update branch
because it is guarded by `else if (selectedLinkItem)` at `editor.ts:1046`, resulting in no
`view.dispatch()` call and no text changes, while `closeLinkModal()` at
`editor.ts:1026-1067` is still invoked, causing the modal to close and the user's edit to
be silently discarded.Fix in Cursor | Fix in VSCode Claude
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** src/ui/file-preview/src/markdown/editor.ts
**Line:** 1001:1023
**Comment:**
*Logic Error: Editing an existing wikilink opens file mode but clears all selected file state, while apply only executes the wikilink branch when a file is selected. If the user clicks "Insert" without re-searching and reselecting, the operation silently no-ops instead of updating the link, breaking the edit flow. Prepopulate `selectedLinkItem` for existing wikilinks or allow apply to update from parsed existing link data.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
Contributor
|
CodeAnt AI finished running the review. Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
Contributor
|
CodeAnt AI is running the review. Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
codeant-ai
Bot
added
size:XXL
Contributor
Sequence DiagramThis PR replaces the old WYSIWYG markdown stack with a CodeMirror-based, source-backed editor and updates the controller so saves diff against the raw markdown, refresh the outline from source, and keep newer unsaved edits if a save completes later. sequenceDiagram
participant User
participant FilePreview
participant MarkdownController
participant MarkdownEditor
participant EditTool
User->>FilePreview: Open markdown file
FilePreview->>MarkdownController: Initialize markdown workspace from payload
MarkdownController->>MarkdownEditor: Mount source editor with draft content and callbacks
User->>MarkdownEditor: Type and format markdown
MarkdownEditor->>MarkdownController: Report raw markdown change
MarkdownController->>EditTool: On autosave, send edit blocks based on source diff
EditTool-->>MarkdownController: Return applied edits result
MarkdownController->>FilePreview: Update saved baseline, outline, and payload (keep newer draft dirty if present)
FilePreview-->>User: Show updated editor and save indicator
Generated by CodeAnt AI |
Contributor
|
CodeAnt AI finished running the review. Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
Contributor
|
CodeAnt AI is running the review. Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
codeant-ai
Bot
added
size:XXL
Contributor
Sequence DiagramThis PR replaces the markdown WYSIWYG stack with a CodeMirror-based editor that keeps raw markdown as the canonical state, and updates the autosave and outline flows accordingly. It also lets partial files grow into full documents by merging newly read chunks into the in-memory baseline and draft content without losing unsaved edits. sequenceDiagram
participant User
participant FilePreview
participant MarkdownEditor
participant MarkdownController
participant Tools
User->>FilePreview: Open markdown file
FilePreview->>MarkdownController: buildBody with payload
MarkdownController-->>FilePreview: HTML with editor shell
FilePreview->>MarkdownEditor: mountMarkdownEditor with callbacks
User->>MarkdownEditor: Edit markdown source
MarkdownEditor-->>MarkdownController: onChange(updated markdown)
MarkdownController->>MarkdownController: Update draftContent, mark dirty, debounce autosave and outline refresh
MarkdownController->>MarkdownController: Autosave timer fires
MarkdownController->>Tools: edit_block for changed ranges
Tools-->>MarkdownController: Applied edits result
MarkdownController->>MarkdownController: Update fullDocumentContent baseline and outline, keep newer draft dirty if it changed again
MarkdownController-->>FilePreview: Update save indicator and payload override
User->>FilePreview: Click load more lines for partial file
FilePreview->>Tools: read_file(offset, length)
Tools-->>FilePreview: Additional markdown chunk
FilePreview->>MarkdownController: expandPartialPayload(payload, direction, chunk)
MarkdownController->>MarkdownController: Merge chunk into baseline and draft, recompute outline
MarkdownController-->>FilePreview: Expanded payload for re render
FilePreview-->>MarkdownEditor: Re mount editor with merged markdown source
Generated by CodeAnt AI |
Contributor
|
CodeAnt AI finished running the review. Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
Contributor
|
CodeAnt AI is running the review. Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
codeant-ai
Bot
added
size:XXL
Contributor
Sequence DiagramThis PR replaces the WYSIWYG markdown editor with a source-backed CodeMirror editor that drives autosave and outline updates from raw markdown, and centralizes partial document expansion through the markdown controller when loading additional lines. sequenceDiagram
participant User
participant FilePreview as File preview panel
participant Controller as Markdown controller
participant Editor as Markdown editor
participant Tools as Tools backend
%% Source-backed editing with autosave
User->>FilePreview: Open markdown file
FilePreview->>Controller: buildBody(payload)
Controller-->>FilePreview: HTML with markdown editor shell
FilePreview->>Editor: mountMarkdownEditor(draftContent, callbacks)
User->>Editor: Edit markdown text
Editor-->>Controller: onChange(updated markdown)
Controller->>Controller: Mark draft dirty and debounce autosave and outline refresh
Controller->>Tools: edit_block(oldText, savedDraft)
Tools-->>Controller: Edits applied on disk
Controller->>FilePreview: storePayloadOverride(updated content and outline)
%% Expanding a partial document
User->>FilePreview: Click load more lines
FilePreview->>Tools: read_file(path, range)
Tools-->>FilePreview: Additional markdown text
FilePreview->>Controller: expandPartialPayload(payload, direction, newText)
Controller-->>FilePreview: Updated payload with merged source and refreshed outline
Generated by CodeAnt AI |
Contributor
|
CodeAnt AI finished running the review. Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
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
2 participants
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.
User description
Summary
Fixes #437 #440
CodeAnt-AI Description
Switch markdown file preview editing to a live source editor
What Changed
Impact
✅ Exact markdown saves✅ Clearer link editing in notes✅ Fewer lost edits during autosave🔄 Retrigger CodeAnt AI Review
Details
💡 Usage Guide
Checking Your Pull Request
Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.
Talking to CodeAnt AI
Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:
This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.
Example
Preserve Org Learnings with CodeAnt
You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:
This helps CodeAnt AI learn and adapt to your team's coding style and standards.
Example
Retrigger review
Ask CodeAnt AI to review the PR again, by typing:
Check Your Repository Health
To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.