Add block-level sync feature for merging Apple Notes and Markdown#2
Add block-level sync feature for merging Apple Notes and Markdown#2paradise-runner wants to merge 1 commit into
Conversation
Introduces a new `cider sync` command and `internal/sync` package that resolves structural differences between a local Markdown file and its linked Apple Note. The sync operates at the block level (headings, paragraphs, lists, code blocks, blockquotes, horizontal rules) using an LCS-based diff algorithm, deliberately avoiding word-level merging. Three merge strategies are supported via --strategy flag: - union (default): keeps blocks from both sides - prefer-local: keeps local blocks, discards remote-only - prefer-remote: keeps remote blocks, discards local-only https://claude.ai/code/session_01VhN3nrmd3r9LEQ7YKvqeo7
There was a problem hiding this comment.
Pull request overview
Adds a new block-level synchronization mechanism to reconcile Markdown files with their linked Apple Notes content, enabling structural diffs and configurable merge strategies.
Changes:
- Introduces
internal/syncpackage to parse Markdown into structural blocks, compute block diffs (LCS), and merge with strategies (union/prefer-local/prefer-remote). - Adds a new
syncCLI command with--strategyflag to perform a bidirectional merge and update both the local file and the Apple Note. - Adds unit tests covering parsing/rendering, diff behavior, merge strategies, and end-to-end sync behavior.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| internal/sync/block.go | Markdown block parser + renderer used as the basis for block-level diff/merge. |
| internal/sync/block_test.go | Test coverage for block parsing/rendering and round-trip behavior. |
| internal/sync/diff.go | LCS-based block diff implementation producing DiffOps. |
| internal/sync/diff_test.go | Test coverage for diff scenarios (adds, deletes, modifications, reorders). |
| internal/sync/merge.go | Merge strategies, Sync() orchestration, and human-readable summary generation. |
| internal/sync/merge_test.go | Test coverage for merge strategies, HasChanges, and end-to-end sync. |
| internal/cli/sync.go | New CLI command wiring block-level sync into file + Apple Notes update flow. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Remaining local-only blocks after last LCS element | ||
| for li < len(local) { | ||
| b := local[li] | ||
| ops = append(ops, DiffOp{Type: OpLocalOnly, Local: &b}) | ||
| li++ | ||
| } | ||
|
|
||
| // Remaining remote-only blocks after last LCS element | ||
| for ri < len(remote) { | ||
| b := remote[ri] | ||
| ops = append(ops, DiffOp{Type: OpRemoteOnly, Remote: &b}) | ||
| ri++ | ||
| } |
There was a problem hiding this comment.
The post-LCS loops also take the address of a loop-scoped variable (b := local[li] / b := remote[ri] then &b). This has the same pointer-aliasing issue as above and can cause all remaining ops to reference the same block value. Store pointers to the slice elements (or allocate distinct copies) instead.
| // computeLCS returns the longest common subsequence of two block slices. | ||
| // Blocks are compared by content equality. | ||
| func computeLCS(a, b []Block) []Block { | ||
| m, n := len(a), len(b) | ||
| // Build DP table | ||
| dp := make([][]int, m+1) | ||
| for i := range dp { | ||
| dp[i] = make([]int, n+1) | ||
| } | ||
| for i := 1; i <= m; i++ { | ||
| for j := 1; j <= n; j++ { | ||
| if a[i-1].Content == b[j-1].Content { | ||
| dp[i][j] = dp[i-1][j-1] + 1 | ||
| } else if dp[i-1][j] >= dp[i][j-1] { | ||
| dp[i][j] = dp[i-1][j] |
There was a problem hiding this comment.
computeLCS (and buildDiffOps alignment) treats blocks as equal when Content matches, ignoring Block.Type. If two blocks have the same text but parse into different block types (or parsing rules differ between local/remote), the diff will incorrectly mark them OpEqual and the merge will silently drop a structural change. Consider comparing both Type and Content when building the LCS and when checking for the next match.
| func computeLCS(a, b []Block) []Block { | ||
| m, n := len(a), len(b) | ||
| // Build DP table | ||
| dp := make([][]int, m+1) | ||
| for i := range dp { | ||
| dp[i] = make([]int, n+1) | ||
| } |
There was a problem hiding this comment.
computeLCS allocates an (m+1)×(n+1) DP table, which is O(m*n) memory/time in number of blocks. For longer notes this can become a noticeable CPU/memory spike (and potentially OOM). If large notes are expected, consider a lower-memory LCS (e.g., Hirschberg) or a different diff algorithm (e.g., patience diff) and/or add guardrails (max blocks) with a user-facing error.
| // Write merged content back to local file | ||
| err = markdown.WriteFile(filePath, updatedContent) | ||
| if err != nil { | ||
| return fmt.Errorf("failed to write to file: %v", err) | ||
| } | ||
| fmt.Printf("File updated: %s\n", filePath) | ||
|
|
||
| // Push merged content to Apple Notes | ||
| fmt.Println("Updating Apple Note...") | ||
| mergedHTML, err := convert.MarkdownToHTML(result.Merged) | ||
| if err != nil { | ||
| return fmt.Errorf("failed to convert merged markdown to HTML: %v", err) | ||
| } | ||
|
|
||
| err = notesClient.Update(noteID, mergedHTML) | ||
| if err != nil { | ||
| return fmt.Errorf("failed to update note: %v", err) | ||
| } | ||
|
|
There was a problem hiding this comment.
syncSingleFile writes the merged content back to disk before attempting to update the Apple Note. If the note update fails, the local file is already modified, leaving local/remote out of sync and requiring manual recovery. Consider updating the note first, or writing the local file atomically only after a successful remote update (or implementing a rollback strategy).
| func Sync(local, remote string, strategy MergeStrategy) SyncResult { | ||
| localBlocks := Parse(local) | ||
| remoteBlocks := Parse(remote) | ||
| ops := Diff(localBlocks, remoteBlocks) | ||
| merged := Merge(ops, strategy) | ||
|
|
||
| return SyncResult{ | ||
| Merged: Render(merged), | ||
| Changes: ops, | ||
| Summary: summarize(ops), | ||
| } |
There was a problem hiding this comment.
SyncResult.Summary is derived purely from the raw diff ops and doesn’t account for the selected merge strategy. For example, with MergePreferLocal the summary can report “X blocks only in remote” even though those blocks will be discarded from the merged result, which can mislead users of the CLI. Consider generating the summary with awareness of the strategy (e.g., reporting kept vs discarded blocks).
| // Emit local-only blocks before the next LCS match | ||
| for li < len(local) && local[li].Content != target { | ||
| b := local[li] | ||
| ops = append(ops, DiffOp{Type: OpLocalOnly, Local: &b}) | ||
| li++ | ||
| } | ||
|
|
||
| // Emit remote-only blocks before the next LCS match | ||
| for ri < len(remote) && remote[ri].Content != target { | ||
| b := remote[ri] | ||
| ops = append(ops, DiffOp{Type: OpRemoteOnly, Remote: &b}) | ||
| ri++ | ||
| } | ||
|
|
||
| // Emit the matched block | ||
| lb := local[li] | ||
| rb := remote[ri] | ||
| ops = append(ops, DiffOp{Type: OpEqual, Local: &lb, Remote: &rb}) | ||
| li++ | ||
| ri++ | ||
| lci++ | ||
| } |
There was a problem hiding this comment.
In buildDiffOps, DiffOp stores pointers to local variables (e.g. b := local[li] then Local: &b, similarly lb := local[li]). Those pointers all reference the same variable reused across iterations, so ops can end up pointing at the last assigned block, producing incorrect diffs/merges. Prefer taking the address of the slice element (e.g. &local[li] / &remote[ri]) or creating a per-op copy with distinct storage.
No description provided.