Add markdown exporter#5998
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR adds a new background service that periodically exports memos from the store to markdown files on disk. The implementation includes a configurable runner with daily scheduling, per-user directory organization, conditional file writing to preserve mtimes, orphan cleanup, comprehensive tests, documentation, and server integration. ChangesMarkdown Export Runner
🚥 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. 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@server/runner/markdownexport/README.md`:
- Line 66: Update the inaccurate sentence in README.md to reflect that
pruneOrphans removes any .md file not produced by the exporter (so a manually
added README.md in the export tree will be deleted on the next run); reference
the actual behavior implemented in the pruneOrphans function in runner.go and
change the wording to warn that only exporter-written .md files are retained
rather than implying non-.md files are the only ones left alone.
- Line 23: The fenced code block consisting of a bare triple-backtick (```)
should declare a language to satisfy markdownlint MD040; update that fence to
include a language identifier (for example change ``` to ```text or another
appropriate language) so the README.md fenced code block is properly annotated.
In `@server/runner/markdownexport/runner.go`:
- Around line 53-63: Add proper Go doc comments for the exported symbols: the
Runner struct, NewRunner function, Run method, and RunOnce method — place a
concise godoc-style comment immediately above each declaration (starting with
the symbol name) describing its purpose, salient behavior, parameters/returns
and any important side effects; ensure comments for Runner explain the role of
Store and Profile fields, NewRunner describes construction and returned *Runner,
and Run/RunOnce summarize what they execute and how they differ.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 50ee9d1b-1cd2-4a2b-bc74-386b80dd1b73
📒 Files selected for processing (5)
AGENTS.mdserver/runner/markdownexport/README.mdserver/runner/markdownexport/runner.goserver/runner/markdownexport/runner_test.goserver/server.go
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
server/runner/markdownexport/runner.go (2)
150-152:⚠️ Potential issue | 🟠 Major | ⚡ Quick winTrack expected paths before writing, not only successful writes.
A memo path is added to
writtenonly afterwriteIfChangedsucceeds. If rewriting an already-exported memo fails once, its old file is omitted fromwrittenand the prune step deletes the last good export for a memo that still exists. Pruning should be driven by the set of live/expected memo paths, independent of whether this run managed to rewrite them.Suggested fix
- // Track every file we write this run. Anything in the tree that we did not - // write is an orphan (its memo was deleted or archived) and gets pruned. - written := make(map[string]struct{}) + // Track every live memo path we expect to exist after this run. Pruning + // must be based on liveness, not on whether a rewrite happened to succeed. + livePaths := make(map[string]struct{}) @@ relPath := filepath.Join(safeSegment(username), memoDate(memo), safeSegment(memo.UID)+".md") absPath := filepath.Join(exportDir, relPath) + livePaths[absPath] = struct{}{} if err := writeIfChanged(absPath, renderMemo(memo, attachmentsByMemo[memo.ID], dataDir)); err != nil { slog.Error("failed to write memo file", slog.String("path", absPath), slog.String("error", err.Error())) continue } - written[absPath] = struct{}{} } } - pruneOrphans(exportDir, written) + pruneOrphans(exportDir, livePaths) slog.Info("markdown export complete", slog.String("dir", exportDir), - slog.Int("files", len(written))) + slog.Int("files", len(livePaths)))Also applies to: 189-195, 201-204
154-165:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftOffset pagination makes orphan pruning unsafe on a live store.
This pass pages with
Offset/Limit, but the runner is wired into the server and runs while memos can still be created, deleted, or archived. If rows shift between pages, a live memo can be skipped and never enterwritten, thenpruneOrphansdeletes its existing.mdas if it were gone. Use a snapshot-stable traversal here (keyset pagination on an immutable cursor, or export from a captured ID set) before pruning based on the result set.
Based on the live server wiring inserver/server.go:171-184, this export runs concurrently with normal store mutations.Also applies to: 198-205
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@server/runner/markdownexport/runner.go` around lines 154 - 165, The loop currently pages memos using offset/limit (variables offset, limit, listBatchSize) and r.Store.ListMemos which is unsafe on a live store because rows can shift and cause memos to be skipped before pruneOrphans/written runs (also affects the second loop at the other location). Replace offset pagination with a snapshot-stable traversal: either (A) first capture a stable ID set by repeatedly calling a keyset-style ListMemos variant (e.g., order by created_at or ID and use a lastSeenID/createdTs cursor in store.FindMemo) to collect all memo IDs, or (B) if the store supports a snapshot/export API, call that to get the immutable set of IDs; then iterate that fixed ID set to build the written map and run pruneOrphans. Update calls that use store.FindMemo/ListMemos (and remove offset usage) so pagination is driven by a cursor (e.g., lastID or createdTs) and ensure ordering is deterministic to avoid live-store skews.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@server/runner/markdownexport/runner.go`:
- Around line 154-165: The loop currently pages memos using offset/limit
(variables offset, limit, listBatchSize) and r.Store.ListMemos which is unsafe
on a live store because rows can shift and cause memos to be skipped before
pruneOrphans/written runs (also affects the second loop at the other location).
Replace offset pagination with a snapshot-stable traversal: either (A) first
capture a stable ID set by repeatedly calling a keyset-style ListMemos variant
(e.g., order by created_at or ID and use a lastSeenID/createdTs cursor in
store.FindMemo) to collect all memo IDs, or (B) if the store supports a
snapshot/export API, call that to get the immutable set of IDs; then iterate
that fixed ID set to build the written map and run pruneOrphans. Update calls
that use store.FindMemo/ListMemos (and remove offset usage) so pagination is
driven by a cursor (e.g., lastID or createdTs) and ensure ordering is
deterministic to avoid live-store skews.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 09b0f2a5-2274-4e19-ba0d-0ead3607f8f3
📒 Files selected for processing (3)
server/runner/markdownexport/README.mdserver/runner/markdownexport/runner.goserver/runner/markdownexport/runner_test.go
✅ Files skipped from review due to trivial changes (1)
- server/runner/markdownexport/README.md
🚧 Files skipped from review as they are similar to previous changes (1)
- server/runner/markdownexport/runner_test.go
Signed-off-by: Colin Holzman <clnhlzmn@users.noreply.github.com>
1 participant
This adds a simple markdown export background service. See #5988
Adds an optional background runner that mirrors all memos to a directory of markdown files on disk. One-way sync (store → filesystem): new memos become files, edits rewrite in place, deleted/archived memos are pruned.
<exportDir>/<username>/<YYYY-MM-DD>/<uid>.mdSee server/runner/markdownexport/README.md for details.