feat(files): inline rich markdown editor#5133
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
PR SummaryMedium Risk Overview
Sidebar collapse is driven by a Breadcrumb path popover closes before navigation, opens only on enter/focus (not pointer move), and uses a short hover close delay so clicks don’t flash the popover mid-route change. Reviewed by Cursor Bugbot for commit cc2d84c. Configure here. |
|
@greptile review |
|
@cursor review |
Greptile SummaryThis PR replaces the raw-markdown / Monaco split for
Confidence Score: 5/5Safe to merge — the core editing, streaming, and autosave paths are well-constructed and the previous thread issues have all been resolved. The autosave hardening (inFlightRef chaining, unmountedRef guard, save-success content preservation) closes real data-loss windows and is backed by comprehensive unit tests. The round-trip safety gate is conservative by design. The two findings are narrow edge cases: one affects only explicitly-escaped callout markers (a very rare pattern), and one is a project-style note about a single CSS rule. Neither affects correctness for the vast majority of real-world markdown files. markdown-fidelity.ts — the callout-escape restoration in postProcessSerializedMarkdown has a subtle interaction with the round-trip safety gate that allows a backslash-escape to be silently stripped on first edit for intentionally-escaped callout markers. Important Files Changed
Sequence Diagram%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant UI as FileViewer
participant RME as RichMarkdownEditor
participant Engine as useEditableFileContent
participant State as textEditorContentReducer
participant AS as useAutosave
participant API as Server API
UI->>RME: mount (file, canEdit)
RME->>Engine: useEditableFileContent(file)
Engine->>API: useWorkspaceFileContent()
API-->>Engine: fetchedContent
Engine->>State: sync-external (fetchedContent)
State-->>Engine: "phase=ready, content"
RME->>RME: isRoundTripSafe(content) → verdict
RME->>RME: lockSettled (frontmatter + verdict)
RME->>RME: "useEditor (initialContent = parseMarkdownToDoc)"
Note over RME: Agent streaming path
Engine->>State: sync-external (streamingContent)
State-->>Engine: "phase=streaming, isStreamInteractionLocked=true"
RME->>RME: RAF-coalesced setContent (read-only)
Note over RME: stream settles
Engine->>State: "sync-external (streamingContent=undefined)"
State-->>Engine: "phase=reconciling → ready"
RME->>RME: lockSettled on settled content, editor.setEditable(true)
Note over RME: User edit path
RME->>RME: onUpdate → getMarkdown() + postProcess
RME->>Engine: setDraftContent(markdown)
Engine->>State: edit action
State-->>Engine: "content updated, isDirty=true"
Engine->>AS: useAutosave (content, savedContent)
AS->>AS: debounce 1500ms
AS->>API: onSave() → PUT fileContent
API-->>AS: success
AS->>Engine: markSavedContent(content)
Engine->>State: save-success (savedContent only, content preserved)
Note over RME: Unmount
AS->>AS: await inFlightRef, flush if still dirty
AS->>API: onSave() final flush
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
participant UI as FileViewer
participant RME as RichMarkdownEditor
participant Engine as useEditableFileContent
participant State as textEditorContentReducer
participant AS as useAutosave
participant API as Server API
UI->>RME: mount (file, canEdit)
RME->>Engine: useEditableFileContent(file)
Engine->>API: useWorkspaceFileContent()
API-->>Engine: fetchedContent
Engine->>State: sync-external (fetchedContent)
State-->>Engine: "phase=ready, content"
RME->>RME: isRoundTripSafe(content) → verdict
RME->>RME: lockSettled (frontmatter + verdict)
RME->>RME: "useEditor (initialContent = parseMarkdownToDoc)"
Note over RME: Agent streaming path
Engine->>State: sync-external (streamingContent)
State-->>Engine: "phase=streaming, isStreamInteractionLocked=true"
RME->>RME: RAF-coalesced setContent (read-only)
Note over RME: stream settles
Engine->>State: "sync-external (streamingContent=undefined)"
State-->>Engine: "phase=reconciling → ready"
RME->>RME: lockSettled on settled content, editor.setEditable(true)
Note over RME: User edit path
RME->>RME: onUpdate → getMarkdown() + postProcess
RME->>Engine: setDraftContent(markdown)
Engine->>State: edit action
State-->>Engine: "content updated, isDirty=true"
Engine->>AS: useAutosave (content, savedContent)
AS->>AS: debounce 1500ms
AS->>API: onSave() → PUT fileContent
API-->>AS: success
AS->>Engine: markSavedContent(content)
Engine->>State: save-success (savedContent only, content preserved)
Note over RME: Unmount
AS->>AS: await inFlightRef, flush if still dirty
AS->>API: onSave() final flush
Reviews (23): Last reviewed commit: "fix(sidebar): reconcile migrated-legacy ..." | Re-trigger Greptile |
|
@cursor review |
|
@greptile review |
|
@cursor review |
|
@greptile review |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 2ca63c2. Configure here.
Replace the raw/preview split for markdown files with a Linear-style inline WYSIWYG editor (TipTap/ProseMirror): bubble + slash menus, code-block language picker with Prism highlighting and line-wrap, resizable images (HTML <img>), GFM tables, and frontmatter held byte-exact out of band. A round-trip preflight gate (decided once per open) falls back to the raw Monaco editor for any file that can't be edited losslessly, so the rich editor never silently corrupts a file.
The unmount flush no longer fires a concurrent PUT alongside an in-flight save; it awaits the in-flight save and then writes the latest content sequentially, so an out-of-order completion can't clobber newer edits with a stale snapshot (addresses Cursor Bugbot).
Some browsers expose a pasted or copied image only via DataTransfer.items (with an empty files list), so screenshot paste was silently ignored. extractImageFiles now falls back to items; moved to a testable module with unit tests (addresses Cursor Bugbot).
Wrap the probe serialize() in try/finally so the throwaway Editor is always destroyed even if setContent/getMarkdown throws (addresses Greptile). Adds a test proving PipeSafeTable escapes only interior cell pipes, not structural delimiters.
|
cursor review |
|
@greptileai review |
…, image escaping) Final-audit follow-ups: - splitMarkdownBlocks normalizes CRLF/CR first — a closing fence ending in \r no longer fails to match, which had collapsed Windows-authored files with fenced code into one block and defeated the linear chunker (perf regression). - normalizeLinkHref rejects file://, blob:, and other non-network schemes (script/data schemes already rejected); network scheme:// (http/ftp/…) and bare host:port still pass. - Image markdown serialization escapes alt/title delimiters and angle-brackets a src with spaces/parens, so they round-trip losslessly; linked-image anchors open in a new tab (target=_blank). - Markdown paste routes through the chunker so a large pasted blob can't freeze the main thread.
Export and unit-test changeTouchesCodeBlock: prose-only edits map decorations (false), edits inside a code block or a setNodeMarkup language change re-tokenize (true) — the perf-correctness path that keeps highlighting off the keystroke path.
…n the SPA - normalizeLinkHref no longer prefixes `./`/`../` relative paths into `https://./…` (they round-trip and resolve correctly). - Following a same-origin in-app link (e.g. /workspace/…) routes through the Next router (same tab) instead of always opening a new tab; modifier-click and external URLs still open a new tab.
|
cursor review |
|
@greptileai review |
…itor The linked-image anchor's native navigation was firing on a plain click in edit mode (where handleClick intentionally returns false for caret placement). Prevent the anchor's default so the editor's handleClick — gated on editable/modifier, matching text links via openOnClick:false — is the sole navigator.
|
cursor review |
|
@greptileai review |
A substring search for 'sidebar_collapsed=1' also matched 'sidebar_collapsed=10', desyncing the pre-paint sidebar rail and client store from the strict server read. Parse the cookie value and compare it to '1' exactly, in both the pre-paint inline script and readCollapsedCookie. Added a store test.
|
cursor review |
|
@greptileai review |
A user whose collapse lived only in localStorage has no sidebar_collapsed cookie at SSR (initialCollapsed=false), but the pre-paint script migrates them to a cookie. The store's persist.rehydrate() is async (flips _hasHydrated after paint), so the first paint showed expanded labels in the collapsed 51px rail. Reconcile to the cookie synchronously in a useLayoutEffect (first render still matches the server, so no hydration mismatch) — no narrow-rail flash.
|
cursor review |
|
@greptileai review |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit cc2d84c. Configure here.
Summary
/slash menu, code-block language picker with Prism syntax highlighting + line-wrap, resizable images (sized images serialize to HTML<img>), GFM tables, task lists<img>/entity/heading-hardbreak/table-<br>data-loss paths are all closed and gatedType of Change
Testing
Checklist