Render Read/Write of any Markdown file as Markdown, not Pygments#234
Render Read/Write of any Markdown file as Markdown, not Pygments#234cboos wants to merge 1 commit into
Conversation
📝 WalkthroughWalkthroughAdds Markdown path detection and updates read/write tool formatting so ChangesMarkdown File Rendering
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
Generalize the auto-memory body rendering (#192) to every .md file: a fully-contained Markdown file should render the usual way (rendered Markdown) rather than syntax-highlighted source. - Add is_markdown_path() (.md/.markdown, case- and separator-insensitive). Memory paths are a subset, so is_memory_path ⊂ is_markdown_path. - Write always carries the whole file → always rendered as Markdown for any .md path (no partial-slice concern). - Read renders as Markdown only for a *full* read (start_line == 1 and not truncated, via _is_full_read). A partial slice (offset/limit) can begin or end mid-code-fence and would render garbled, so partial reads keep Pygments — where a line-numbered source view is more useful anyway. - Reuse the HTML-escaping markdown helper: file content is untrusted regardless of being a memory file, so raw <script>/HTML renders as text. Memory keeps its extra specialization unchanged: the 🧠 title, relative- link resolution, and always-Markdown body (even partial reads). General .md files get neither the 🧠 title nor link rewriting. The Markdown *output* renderer is untouched (it keeps fenced code blocks; embedding raw Markdown-in-Markdown would risk heading bleed and fence collisions). _is_full_read's docstring notes the one residual edge (text-only parser fallback + truncated-from-line-1 → reads as full, cosmetic only). Tests pin the full/partial/truncated split behaviorally through the public format_read_output rather than importing the private helper. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
79025fd to
9b0a164
Compare
|
@coderabbitai full review |
✅ Action performedFull review finished. |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
test/test_markdown_file_rendering.py (1)
91-94: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDecouple partial-read coverage from truncation in this test.
Line 93 currently makes the case both partial and truncated, so this can pass even if the
start_linebranch regresses. Make it non-truncated to pin the partial-read condition specifically.Suggested diff
- html = format_read_output(_read(MD, MD_BODY, start_line=10, total=999)) + html = format_read_output(_read(MD, MD_BODY, start_line=2, total=3))🤖 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 `@test/test_markdown_file_rendering.py` around lines 91 - 94, The partial markdown read test currently mixes the start_line partial-read path with truncation via total=999, so it can still pass even if the partial-read behavior regresses. Update test_partial_md_read_keeps_pygments to use a non-truncated read while keeping start_line > 1, and keep the assertion on format_read_output(_read(...)) focused on the partial-read branch in MD/MD_BODY rather than truncation.
🤖 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.
Nitpick comments:
In `@test/test_markdown_file_rendering.py`:
- Around line 91-94: The partial markdown read test currently mixes the
start_line partial-read path with truncation via total=999, so it can still pass
even if the partial-read behavior regresses. Update
test_partial_md_read_keeps_pygments to use a non-truncated read while keeping
start_line > 1, and keep the assertion on format_read_output(_read(...)) focused
on the partial-read branch in MD/MD_BODY rather than truncation.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c1be2361-a271-405b-8561-c191b3bf91aa
📒 Files selected for processing (3)
claude_code_log/html/tool_formatters.pyclaude_code_log/html/utils.pytest/test_markdown_file_rendering.py
Closes #232.
Problem
A follow-up to the auto-memory work (#192). We already render auto-memory file bodies as rendered Markdown instead of Pygments-highlighted source — but that specialization was keyed on the memory path, not on the file being Markdown. Any other
.mdfile (a README, a doc, a plan) read or written during a session still rendered as highlighted source.Change
Generalize the body rendering to every Markdown file:
is_markdown_path()(.md/.markdown, case- and separator-insensitive) inhtml/utils.py. Memory paths are a subset:is_memory_path ⊂ is_markdown_path..md→ always rendered as Markdown. A Write carries the file's full content, so there's no partial-slice concern..md→ rendered as Markdown only for a full read (start_line == 1 and not truncated, via_is_full_read). A partial slice (offset/limit) can begin or end mid-code-fence and would render garbled, so partial reads keep Pygments — where a line-numbered source view is more useful for a slice anyway.render_user_markdown_collapsible): file content is untrusted regardless of being a memory file, so raw<script>/HTML renders as text, not live DOM.Deliberately unchanged
.mdfiles get neither the 🧠 title nor link rewriting.Testing
New
test/test_markdown_file_rendering.py(15 tests):is_markdown_path(.md/.markdown, case, Windows, memory-subset, negatives incl..mdx); Read full→Markdown / partial→Pygments / truncated→Pygments / non-md→Pygments / escaping / no-link-resolution; Write md→Markdown / non-md→Pygments / escaping / no-link-resolution. The full/partial/truncated split is pinned behaviorally through the publicformat_read_output(no private-symbol import).test_memory_rendering.pyregressions all pass — memory behavior unchanged. No snapshot churn (no existing fixture does a non-memory.mdRead/Write).just cigreen (unit + TUI + browser + snapshots, ruff, pyright, ty).🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
.md/.markdownreads and writes now display as formatted Markdown with safer HTML escaping.Tests