Markdown Store + Memory Content Contract - #2
Conversation
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughThis PR introduces a new memory content storage system ( ChangesMemory Content Storage System
Playwright Configuration Refactoring
Sequence Diagram(s)sequenceDiagram
participant Client
participant resolveOp as resolveMemoryContentPath
participant writeOp as writeMemoryContent
participant readOp as readMemoryContent
participant FileSystem
Client->>resolveOp: config + memoryId
resolveOp->>resolveOp: Validate UUIDv7
resolveOp->>Client: ResolvedMemoryContentPath
Client->>writeOp: config + memoryId + frontmatter + markdown
writeOp->>FileSystem: Create memories/{memoryId}
writeOp->>FileSystem: Write temp .CONTENT.md.tmp
writeOp->>FileSystem: Atomic rename to CONTENT.md
writeOp->>Client: WriteMemoryContentResult
Client->>readOp: config + memoryId
readOp->>FileSystem: Load CONTENT.md
readOp->>readOp: Strip BOM + parse frontmatter
readOp->>readOp: Validate all fields + memoryId match
readOp->>Client: ReadMemoryContentResult
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
✨ 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 |
744710a to
4a787ed
Compare
There was a problem hiding this comment.
Pull request overview
Introduces a filesystem-backed “memory content” contract (memories/{memoryId}/CONTENT.md) with deterministic frontmatter serialization/parsing, plus tests and a small Playwright CI web-server tweak to use the built app.
Changes:
- Added
src/server/store/memory-content.tsimplementing path resolution, fixture generation, atomic-ish writes, and strict frontmatter parsing/validation. - Added Vitest coverage for path isolation, read/write roundtrips, malformed/missing content, and remote-image link preservation.
- Updated Playwright webServer command selection to use
bun run startin CI andbun run devlocally.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| tests/server/store/memory-content.test.ts | Adds filesystem-isolated tests covering resolver, writer/reader roundtrip, and failure modes. |
| src/server/store/memory-content.ts | Implements the CONTENT.md contract, including UUIDv7 path validation and deterministic frontmatter handling. |
| src/server/store/index.ts | Re-exports the new store APIs from the store module entrypoint. |
| playwright.config.ts | Uses bun run start in CI to reuse the prebuilt app, keeping local dev on bun run dev. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4a787ed92a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Summary
{storePath}/memories/{memoryId}/CONTENT.md.bun run startso CI reuses the app built bybun run verifyinstead of timing out onvinxi dev.API Names
resolveMemoryContentPathcreateMemoryContentFixturewriteMemoryContentreadMemoryContentMemoryContentFrontmatterWriteMemoryContentInputReadMemoryContentResultSample
CONTENT.mdValidation
npm test -- tests/server/store/memory-content.test.ts-> failed before implementation with missingsrc/server/store/memory-content, then passed after implementation: 1 file, 8 tests.npm run typecheck-> pass.npm test-> pass, 2 files, 13 tests.npm run build-> pass.env GIT_DIR=.tmp/gitdir GIT_WORK_TREE=. git diff --check-> pass.typecheck,test, andbuildsuccessfully.Verifyon commit7fadc3f-> pass.npm run test:e2e-> blocked locally by macOS Chromium sandbox permission (bootstrap_check_in ... Permission denied); CI is the source of truth for this check.typecheck,test, andbuildsuccessfully.bunis not available onPATHin this Symphony workspace, so I used the equivalent npm scripts against the installed dependencies.Downstream Assumptions
relativePathasmemories/{memoryId}/CONTENT.md.memoryId; the path resolver rejects non-UUID-v7 path segments.readMemoryContentoutput and keep tags/categories sourced from SQLite, not frontmatter.Summary by CodeRabbit
New Features
Tests