feat: version history with inline diff comparison#125
feat: version history with inline diff comparison#125blksmr wants to merge 7 commits intoerictli:mainfrom
Conversation
- Add VersionHistoryPanel component showing git commit history per note - Add git_get_file_history and git_get_file_at_commit Tauri commands - Handle file renames: detect original tracked name via git rename detection when current filename has no git history (temporary stage + diff -M) - Use --name-only in git log to track file path at each commit for git show - Fix core.quotepath for UTF-8 filenames (accents, special chars) - Guard against auto-save during history preview (isLoadingRef + historyPreviewRef) - Add HistoryIcon, useOptionalGit hook, FileVersion service types
- Generate descriptive commit messages from changed files instead of generic "Quick commit from Scratch" (e.g. "Update API Credentials") - Add diff stats badges (added/removed/changed) per version in history panel - Prefetch version contents for instant preview and diff computation - Hide current version from history list (show only previous versions) - Show cat illustration for empty state - Auto-refresh history after commit (via refreshKey prop) - Close history panel with Escape when editor is not focused - Fix flushSync warning by deferring setContent via queueMicrotask
When selecting a past version, the editor now shows the current content with visual diff decorations: green for additions, red strikethrough for deletions. Uses prosemirror-changeset for accurate document-level diffing. DiffBadge stats now normalize markdown to stay consistent with what TipTap renders.
📝 WalkthroughWalkthroughThis PR implements a per-file version history panel enabling users to view git commit history for individual notes, preview previous versions in read-only mode, restore earlier versions, and track history across file renames using git's rename detection capabilities. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Editor as Editor Component
participant VersionPanel as VersionHistoryPanel
participant GitService as Git Service (Tauri)
participant Backend as Backend (Rust)
participant DiffEngine as Diff Engine
User->>Editor: Click HistoryIcon to open history
Editor->>VersionPanel: Open with current noteId
VersionPanel->>GitService: getFileHistory(filePath)
GitService->>Backend: invoke git_get_file_history
Backend->>Backend: git log --follow, parse commits
Backend->>GitService: FileVersion[]
GitService->>VersionPanel: FileVersion[]
VersionPanel->>VersionPanel: Cache versions, render list
User->>VersionPanel: Select previous version
VersionPanel->>GitService: getFileAtCommit(commit, filePath)
GitService->>Backend: invoke git_get_file_at_commit
Backend->>GitService: file content at commit
GitService->>VersionPanel: content string
VersionPanel->>DiffEngine: computeVersionDiff(oldDoc, newDoc)
DiffEngine->>DiffEngine: Use prosemirror-changeset
DiffEngine->>Editor: applyVersionDiffDecorations(changes)
Editor->>Editor: Render green/red diffs in editor
User->>VersionPanel: Click Restore
VersionPanel->>Editor: onRestore(selectedContent)
Editor->>Editor: setContent(content), save note
Editor->>VersionPanel: onClose()
VersionPanel->>Editor: clearDiffDecorations()
Estimated code review effort🎯 4 (Complex) | ⏱️ ~70 minutes Possibly related PRs
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 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 |
…and parser ambiguity - Use temporary index (GIT_INDEX_FILE) with unique nonce in detect_original_path to avoid corrupting user's staging area and prevent concurrent call races - Resolve git dir via rev-parse --git-dir for linked worktree support - Switch git log parser from pipe-delimited to NUL-delimited (%x00 + -z) to handle authors/subjects containing | characters - Clear diff decorations before applying new ones in handleCompare to prevent stale highlights when comparing identical versions - Reset history compare mode on note switch to prevent read-only editor with stale diff when selecting a different note during comparison - Fix stale closure in Escape listener by adding handleHistoryClose to deps - Replace multiset-based DiffBadge stats with LCS algorithm to correctly detect line reorderings
…proved performance
- Extract commit message parsing into testable parse_commit_message_status_line with rename support (rsplit " -> ") - Add error recovery in handleCompare when getDiffSnapshot fails (toast + state cleanup) - Extract diffStats and historyCompare into separate modules with tests - Show all committed versions (remove slice(1) off-by-one) - Always close history panel on note switch
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (3)
src/components/editor/historyCompare.test.ts (1)
30-33: Consider capturing thesetContentargument for verification.The current stub ignores the content passed to
setContent. Capturing it would allow asserting that the correct snapshot is applied.Optional enhancement
commands: { - setContent() { - calls.push("setContent"); + setContent(content: unknown) { + calls.push(`setContent:${JSON.stringify(content)}`); }, },Then assert:
expect(calls).toEqual(["editable:false", 'setContent:{"type":"doc","content":[]}']);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/editor/historyCompare.test.ts` around lines 30 - 33, The test's stub for commands.setContent currently ignores its input; modify the stub to capture the argument and push a serialized string (e.g., `setContent:${JSON.stringify(arg)}`) into the existing calls array so the test can assert the exact snapshot applied. Locate the stubbed function named setContent (inside the commands object) and change its implementation to accept a parameter (e.g., content) and push a stringified representation to calls; then update the assertion to expect the captured value (e.g., ["editable:false", "setContent:..."]). Ensure you only change the setContent stub and the test expectation referencing calls.src/lib/diff.ts (1)
23-26: Minor duplication withdiffStats.ts.
splitVisibleLinesduplicates the normalization logic fromdiffStats.ts. Consider extracting to a shared utility if the duplication grows.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/diff.ts` around lines 23 - 26, splitVisibleLines duplicates the text-normalization logic found in diffStats.ts; extract that normalization into a shared utility function (e.g., normalizeVisibleText) and have splitVisibleLines and the code in diffStats.ts call the new utility. Update imports/usages so splitVisibleLines(text) becomes splitVisibleLines(normalizeVisibleText(text)) or have splitVisibleLines call normalizeVisibleText internally; ensure the utility preserves the current behavior of trimming trailing spaces/tabs per line and trimming trailing newlines.src/components/history/diffStats.test.ts (1)
4-27: Consider expanding test coverage.The current tests cover edge cases well (empty, pure additions, whitespace normalization), but additional cases would strengthen confidence:
- Pure removals:
computeDiffStats("hello\nworld", "")- Modified lines:
computeDiffStats("hello", "world")→{ added: 0, removed: 0, changed: 1 }- Mixed scenario:
computeDiffStats("a\nb\nc", "a\nx\nc")→{ added: 0, removed: 0, changed: 1 }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/history/diffStats.test.ts` around lines 4 - 27, Add unit tests for computeDiffStats to cover pure removals, single-line modification, and a mixed modification scenario: (1) call computeDiffStats("hello\nworld", "") and assert { added: 0, removed: 2, changed: 0 }, (2) call computeDiffStats("hello", "world") and assert { added: 0, removed: 0, changed: 1 }, and (3) call computeDiffStats("a\nb\nc", "a\nx\nc") and assert { added: 0, removed: 0, changed: 1 }; place these as additional test cases in the same describe block so computeDiffStats behavior for removals and modifications is validated.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/editor/Editor.tsx`:
- Around line 2208-2217: The history preview currently only sets the ProseMirror
editor read-only but leaves Markdown source mode writable; update the history
toggle and save path to prevent edits while history is open: when opening
history via the History IconButton handler (the setHistoryOpen call) also
force-disable source mode (call setSourceMode(false)) so the source editor is
not selectable, and additionally add a guard in handleSourceChange (the method
that currently calls saveNote) to early-return if historyOpen is true (or if
gitCtx?.gitEnabled && previewMode/historyOpen) before calling saveNote; this
ensures both UI and programmatic saves are blocked while a historical preview is
active.
- Around line 1258-1269: The saveImmediately call is fire-and-forget, allowing
an in-flight write to later overwrite a restored version; change the flow to
await the save before entering compare/restore state: make the surrounding
handler async (or capture and await the Promise returned by saveImmediately),
e.g., call await saveImmediately(loadedNoteIdRef.current, liveMarkdown) (or
await the returned Promise stored in a saveInFlightRef) and handle errors so the
compare/restore logic only proceeds after the save resolves/rejects; reference
functions/refs: getMarkdown, saveTimeoutRef, needsSaveRef, loadedNoteIdRef,
saveImmediately.
In `@src/components/history/VersionHistoryPanel.tsx`:
- Around line 153-179: handleSelectVersion can race: multiple getFileAtCommit
calls finish out of order causing selectedContent/onCompare to reflect an
earlier click. Fix by tracking the latest requested commit (or an incrementing
requestId) in a ref (e.g., selectedRequestRef) when handleSelectVersion starts;
after awaiting getFileAtCommit(version.commit, ...), check that the ref still
matches this request/commit before calling setSelectedContent, onCompare, or
updating versionContents; ignore results from stale requests so UI state only
updates for the most recent selection. Use existing symbols handleSelectVersion,
getFileAtCommit, setSelectedContent, onCompare, selectedIndex, and
versionContents to implement this guard.
- Around line 100-137: The effect in VersionHistoryPanel clears
selectedIndex/selectedContent on every refreshKey change which desynchronizes
Editor's compare mode; update the useEffect so it preserves the current
selection when refreshKey changes (only reset selection when noteId changes) or
explicitly call the parent reset handler (e.g., handleHistoryClose /
onCloseHistory prop) before clearing selection; locate the useEffect body and
adjust the initial setSelectedIndex(null)/setSelectedContent(null) logic (and/or
call the parent reset function) so refreshing versions via
getFileHistory/getFileAtCommit does not leave the Editor stuck in
compare/read-only state.
In `@src/components/icons/index.tsx`:
- Around line 1490-1505: The HistoryIcon SVG is missing the strokeWidth
attribute which causes inconsistent stroke weight compared to other icons;
update the HistoryIcon component (function HistoryIcon) to include
strokeWidth={2} on the <svg> element so its strokes match the rest of the icons,
keeping the existing className, viewBox, fill, stroke, strokeLinecap, and
strokeLinejoin props unchanged.
---
Nitpick comments:
In `@src/components/editor/historyCompare.test.ts`:
- Around line 30-33: The test's stub for commands.setContent currently ignores
its input; modify the stub to capture the argument and push a serialized string
(e.g., `setContent:${JSON.stringify(arg)}`) into the existing calls array so the
test can assert the exact snapshot applied. Locate the stubbed function named
setContent (inside the commands object) and change its implementation to accept
a parameter (e.g., content) and push a stringified representation to calls; then
update the assertion to expect the captured value (e.g., ["editable:false",
"setContent:..."]). Ensure you only change the setContent stub and the test
expectation referencing calls.
In `@src/components/history/diffStats.test.ts`:
- Around line 4-27: Add unit tests for computeDiffStats to cover pure removals,
single-line modification, and a mixed modification scenario: (1) call
computeDiffStats("hello\nworld", "") and assert { added: 0, removed: 2, changed:
0 }, (2) call computeDiffStats("hello", "world") and assert { added: 0, removed:
0, changed: 1 }, and (3) call computeDiffStats("a\nb\nc", "a\nx\nc") and assert
{ added: 0, removed: 0, changed: 1 }; place these as additional test cases in
the same describe block so computeDiffStats behavior for removals and
modifications is validated.
In `@src/lib/diff.ts`:
- Around line 23-26: splitVisibleLines duplicates the text-normalization logic
found in diffStats.ts; extract that normalization into a shared utility function
(e.g., normalizeVisibleText) and have splitVisibleLines and the code in
diffStats.ts call the new utility. Update imports/usages so
splitVisibleLines(text) becomes splitVisibleLines(normalizeVisibleText(text)) or
have splitVisibleLines call normalizeVisibleText internally; ensure the utility
preserves the current behavior of trimming trailing spaces/tabs per line and
trimming trailing newlines.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: decbeb89-bc62-4c26-a500-9bec872dc904
⛔ Files ignored due to path filters (2)
package-lock.jsonis excluded by!**/package-lock.jsonsrc/assets/no-history-cat.pngis excluded by!**/*.png
📒 Files selected for processing (16)
package.jsonsrc-tauri/src/git.rssrc-tauri/src/lib.rssrc/App.csssrc/components/editor/DiffHighlight.tssrc/components/editor/Editor.tsxsrc/components/editor/historyCompare.test.tssrc/components/editor/historyCompare.tssrc/components/history/VersionHistoryPanel.tsxsrc/components/history/diffStats.test.tssrc/components/history/diffStats.tssrc/components/icons/index.tsxsrc/context/GitContext.tsxsrc/lib/diff.tssrc/services/git.tstsconfig.json
| // Capture live editor content before overwriting (may differ from currentNote.content | ||
| // if the user typed within the auto-save debounce window) | ||
| const liveMarkdown = getMarkdown(editor); | ||
| // Synchronously flush: cancel pending timer, mark as not needing save | ||
| if (saveTimeoutRef.current) { | ||
| clearTimeout(saveTimeoutRef.current); | ||
| saveTimeoutRef.current = null; | ||
| } | ||
| if (needsSaveRef.current && loadedNoteIdRef.current) { | ||
| needsSaveRef.current = false; | ||
| saveImmediately(loadedNoteIdRef.current, liveMarkdown); | ||
| } |
There was a problem hiding this comment.
Await the flush before entering compare mode.
saveImmediately(...) on Line 1268 is fired and forgotten. If the user restores another version before that write finishes, the in-flight save can complete afterward and overwrite the restored content on disk. That makes compare/restore nondeterministic.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/editor/Editor.tsx` around lines 1258 - 1269, The
saveImmediately call is fire-and-forget, allowing an in-flight write to later
overwrite a restored version; change the flow to await the save before entering
compare/restore state: make the surrounding handler async (or capture and await
the Promise returned by saveImmediately), e.g., call await
saveImmediately(loadedNoteIdRef.current, liveMarkdown) (or await the returned
Promise stored in a saveInFlightRef) and handle errors so the compare/restore
logic only proceeds after the save resolves/rejects; reference functions/refs:
getMarkdown, saveTimeoutRef, needsSaveRef, loadedNoteIdRef, saveImmediately.
| {currentNote && gitCtx?.gitEnabled && !previewMode && !sourceMode && ( | ||
| <Tooltip content="Version history"> | ||
| <IconButton | ||
| onClick={() => setHistoryOpen((v) => !v)} | ||
| className={cn(historyOpen && "bg-bg-emphasis")} | ||
| > | ||
| <HistoryIcon className="w-4.25 h-4.25 stroke-[1.6]" /> | ||
| </IconButton> | ||
| </Tooltip> | ||
| )} |
There was a problem hiding this comment.
History preview is still writable through source mode.
This integration only makes the ProseMirror editor read-only. The Markdown source path stays available, and handleSourceChange() still calls saveNote, so users can edit/save while a historical preview is active. Please disable source mode while history preview is active, or guard source-mode saves with the same history-preview check.
Also applies to: 2555-2584
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/editor/Editor.tsx` around lines 2208 - 2217, The history
preview currently only sets the ProseMirror editor read-only but leaves Markdown
source mode writable; update the history toggle and save path to prevent edits
while history is open: when opening history via the History IconButton handler
(the setHistoryOpen call) also force-disable source mode (call
setSourceMode(false)) so the source editor is not selectable, and additionally
add a guard in handleSourceChange (the method that currently calls saveNote) to
early-return if historyOpen is true (or if gitCtx?.gitEnabled &&
previewMode/historyOpen) before calling saveNote; this ensures both UI and
programmatic saves are blocked while a historical preview is active.
| useEffect(() => { | ||
| let cancelled = false; | ||
| setLoading(true); | ||
| setSelectedIndex(null); | ||
| setSelectedContent(null); | ||
| setVersionContents(new Map()); | ||
|
|
||
| getFileHistory(`${noteId}.md`) | ||
| .then((result) => { | ||
| if (!cancelled) { | ||
| setVersions(result); | ||
| for (const version of result) { | ||
| getFileAtCommit(version.commit, version.filePath) | ||
| .then((content) => { | ||
| if (!cancelled) { | ||
| setVersionContents((prev) => { | ||
| const next = new Map(prev); | ||
| next.set(version.commit, content); | ||
| return next; | ||
| }); | ||
| } | ||
| }) | ||
| .catch(() => {}); | ||
| } | ||
| } | ||
| }) | ||
| .catch((err) => { | ||
| console.error("Failed to load file history:", err); | ||
| if (!cancelled) setVersions([]); | ||
| }) | ||
| .finally(() => { | ||
| if (!cancelled) setLoading(false); | ||
| }); | ||
|
|
||
| return () => { | ||
| cancelled = true; | ||
| }; | ||
| }, [noteId, refreshKey]); |
There was a problem hiding this comment.
Refreshing the list here can leave the editor stuck in stale compare mode.
This effect clears selectedIndex and selectedContent, but the compare state lives in Editor.tsx and is only reset through handleHistoryClose(). If refreshKey changes while a version is selected, the sidebar becomes unselected while the editor stays read-only with the old diff decorations and save suppression still active. Either preserve the selection across refreshes or explicitly reset the parent compare state before clearing it.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/history/VersionHistoryPanel.tsx` around lines 100 - 137, The
effect in VersionHistoryPanel clears selectedIndex/selectedContent on every
refreshKey change which desynchronizes Editor's compare mode; update the
useEffect so it preserves the current selection when refreshKey changes (only
reset selection when noteId changes) or explicitly call the parent reset handler
(e.g., handleHistoryClose / onCloseHistory prop) before clearing selection;
locate the useEffect body and adjust the initial
setSelectedIndex(null)/setSelectedContent(null) logic (and/or call the parent
reset function) so refreshing versions via getFileHistory/getFileAtCommit does
not leave the Editor stuck in compare/read-only state.
| const handleSelectVersion = useCallback( | ||
| async (index: number) => { | ||
| const version = versions[index]; | ||
| if (!version) return; | ||
|
|
||
| setSelectedIndex(index); | ||
|
|
||
| const cached = versionContents.get(version.commit); | ||
| if (cached != null) { | ||
| setSelectedContent(cached); | ||
| onCompare(cached); | ||
| return; | ||
| } | ||
|
|
||
| try { | ||
| const content = await getFileAtCommit(version.commit, version.filePath); | ||
| setSelectedContent(content); | ||
| onCompare(content); | ||
| setVersionContents((prev) => { | ||
| const next = new Map(prev); | ||
| next.set(version.commit, content); | ||
| return next; | ||
| }); | ||
| } catch (err) { | ||
| console.error("Failed to load version content:", err); | ||
| setSelectedContent(null); | ||
| } |
There was a problem hiding this comment.
Ignore stale version fetches after the user clicks again.
The async branch applies whichever getFileAtCommit() call finishes last. Clicking version A and then B quickly can leave B highlighted while selectedContent and onCompare() switch back to A, so Restore can target the wrong commit.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/history/VersionHistoryPanel.tsx` around lines 153 - 179,
handleSelectVersion can race: multiple getFileAtCommit calls finish out of order
causing selectedContent/onCompare to reflect an earlier click. Fix by tracking
the latest requested commit (or an incrementing requestId) in a ref (e.g.,
selectedRequestRef) when handleSelectVersion starts; after awaiting
getFileAtCommit(version.commit, ...), check that the ref still matches this
request/commit before calling setSelectedContent, onCompare, or updating
versionContents; ignore results from stale requests so UI state only updates for
the most recent selection. Use existing symbols handleSelectVersion,
getFileAtCommit, setSelectedContent, onCompare, selectedIndex, and
versionContents to implement this guard.
| export function HistoryIcon({ className = "w-4.5 h-4.5" }: IconProps) { | ||
| return ( | ||
| <svg | ||
| className={className} | ||
| viewBox="0 0 24 24" | ||
| fill="none" | ||
| stroke="currentColor" | ||
| strokeLinecap="round" | ||
| strokeLinejoin="round" | ||
| > | ||
| <path d="M3 12a9 9 0 1 0 9-9 9.75 9.75 0 0 0-6.74 2.74L3 8" /> | ||
| <path d="M3 3v5h5" /> | ||
| <path d="M12 7v5l4 2" /> | ||
| </svg> | ||
| ); | ||
| } |
There was a problem hiding this comment.
Missing strokeWidth attribute.
Other icons in this file explicitly set strokeWidth={2}. Adding it ensures consistent stroke weight across all icons.
Proposed fix
export function HistoryIcon({ className = "w-4.5 h-4.5" }: IconProps) {
return (
<svg
className={className}
viewBox="0 0 24 24"
fill="none"
stroke="currentColor"
+ strokeWidth={2}
strokeLinecap="round"
strokeLinejoin="round"
>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export function HistoryIcon({ className = "w-4.5 h-4.5" }: IconProps) { | |
| return ( | |
| <svg | |
| className={className} | |
| viewBox="0 0 24 24" | |
| fill="none" | |
| stroke="currentColor" | |
| strokeLinecap="round" | |
| strokeLinejoin="round" | |
| > | |
| <path d="M3 12a9 9 0 1 0 9-9 9.75 9.75 0 0 0-6.74 2.74L3 8" /> | |
| <path d="M3 3v5h5" /> | |
| <path d="M12 7v5l4 2" /> | |
| </svg> | |
| ); | |
| } | |
| export function HistoryIcon({ className = "w-4.5 h-4.5" }: IconProps) { | |
| return ( | |
| <svg | |
| className={className} | |
| viewBox="0 0 24 24" | |
| fill="none" | |
| stroke="currentColor" | |
| strokeWidth={2} | |
| strokeLinecap="round" | |
| strokeLinejoin="round" | |
| > | |
| <path d="M3 12a9 9 0 1 0 9-9 9.75 9.75 0 0 0-6.74 2.74L3 8" /> | |
| <path d="M3 3v5h5" /> | |
| <path d="M12 7v5l4 2" /> | |
| </svg> | |
| ); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/icons/index.tsx` around lines 1490 - 1505, The HistoryIcon SVG
is missing the strokeWidth attribute which causes inconsistent stroke weight
compared to other icons; update the HistoryIcon component (function HistoryIcon)
to include strokeWidth={2} on the <svg> element so its strokes match the rest of
the icons, keeping the existing className, viewBox, fill, stroke, strokeLinecap,
and strokeLinejoin props unchanged.
|
Might take a longer for me to get to this since it's a big change. |
|
@erictli I'm still encountering some bugs, but this feature is going to be awesome. |
Closes #123
Summary
prosemirror-changesetfor accurate document-level diffing between ProseMirror snapshotsgit log --follow) so history persists across note renamesKey files
src/components/history/VersionHistoryPanel.tsx— History panel UI with version list, diff stats, restoresrc/lib/diff.ts— Diff engine usingprosemirror-changesetsrc/components/editor/DiffHighlight.ts— TipTap extension (ProseMirror Plugin + DecorationSet)src-tauri/src/git.rs— Rust backend:get_file_history,get_file_at_commit, rename detectionTest plan
Summary by CodeRabbit