feat(web): render markdown files in browse#1382
Conversation
Render Markdown blobs by default while preserving an explicit raw source mode, so README-style files are readable without losing the existing CodeMirror path. Constraint: Issue sourcebot-dev#794 asks for rendered Markdown with a source option similar to GitHub.\nRejected: Always using CodeMirror for Markdown | Keeps documentation-style files hard to read and leaves no rendered mode.\nRejected: Enabling raw HTML rendering | Avoids expanding the browse page XSS surface for the first slice.\nConfidence: high\nScope-risk: moderate\nDirective: Keep raw source mode available when extending Markdown previews.\nTested: node .yarn/releases/yarn-4.7.0.cjs workspace @sourcebot/web test 'src/app/(app)/browse/[...path]/components/codePreviewPanel/markdownPreview.test.tsx' 'src/app/(app)/browse/hooks/utils.test.ts'\nTested: node .yarn/releases/yarn-4.7.0.cjs workspace @sourcebot/web lint\nTested: node .yarn/releases/yarn-4.7.0.cjs workspace @sourcebot/web build\nTested: git diff --check\nNot-tested: Live browser screenshot against a running indexed repository.
|
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:
WalkthroughAdds rendered Markdown preview mode in the code browser. Blob view URLs now carry a view mode, markdown files can render with browse-aware links and images, and the page passes the computed mode into the preview and diff panels. ChangesMarkdown Rendered Preview in Code Browser
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
Add the required Unreleased changelog entry after the draft PR number is known. Constraint: Sourcebot contribution rules require every PR to include a changelog follow-up commit.\nConfidence: high\nScope-risk: narrow\nDirective: Keep this as the PR-number-specific follow-up commit.\nTested: git diff --check\nNot-tested: Full test suite not rerun for changelog-only text change.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
packages/web/src/app/(app)/browse/[...path]/components/codePreviewPanel/markdownPreview.test.tsx (1)
50-94: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd coverage for the
blameandpreviewRefmarkdown opt-out paths.This suite locks down the
viewModeswitch, but the new contract also requires Markdown files to stay on the raw/source path whenblameorpreviewRefis set. A pair of focused tests here would protect the behavior called out in the PR objective and mirrored incodePreviewPanel.tsx.🤖 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 `@packages/web/src/app/`(app)/browse/[...path]/components/codePreviewPanel/markdownPreview.test.tsx around lines 50 - 94, The markdown preview tests currently cover the default and source viewMode paths, but they do not verify the new opt-out behavior for blame and previewRef. Add two focused cases in markdownPreview.test.tsx around CodePreviewPanel to assert that when blame or previewRef is present, Markdown content stays on the raw/source rendering path instead of the rendered markdown path.
🤖 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
`@packages/web/src/app/`(app)/browse/[...path]/components/codePreviewPanel/codePreviewPanel.tsx:
- Around line 38-41: The close-preview navigation in CodePreviewPanel is
dropping the current BlobViewMode, so preserve the active view when building the
browse link. Update the close link that uses getBrowsePath in CodePreviewPanel
to pass through viewMode alongside repoName, revisionName, and path, ensuring
closing a preview keeps source/rendered state unchanged.
In
`@packages/web/src/app/`(app)/browse/[...path]/components/codePreviewPanel/markdownPreviewPanel.tsx:
- Around line 6-8: MarkdownPreviewPanel only accepts source, so react-markdown
cannot resolve repo-relative assets correctly in blob preview mode. Update
MarkdownPreviewPanelProps and the MarkdownPreviewPanel renderer to accept blob
context (such as the blob path/revision or a raw-file base URL), then use that
context to rewrite markdown links/images before rendering so references like
./diagram.png point to raw bytes instead of the browse page route.
---
Nitpick comments:
In
`@packages/web/src/app/`(app)/browse/[...path]/components/codePreviewPanel/markdownPreview.test.tsx:
- Around line 50-94: The markdown preview tests currently cover the default and
source viewMode paths, but they do not verify the new opt-out behavior for blame
and previewRef. Add two focused cases in markdownPreview.test.tsx around
CodePreviewPanel to assert that when blame or previewRef is present, Markdown
content stays on the raw/source rendering path instead of the rendered markdown
path.
🪄 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: 4603aaf6-ab44-4a4a-ae00-306a61ce6bcb
📒 Files selected for processing (8)
CHANGELOG.mdpackages/web/src/app/(app)/browse/[...path]/components/codePreviewPanel/codePreviewPanel.tsxpackages/web/src/app/(app)/browse/[...path]/components/codePreviewPanel/markdownPreview.test.tsxpackages/web/src/app/(app)/browse/[...path]/components/codePreviewPanel/markdownPreviewPanel.tsxpackages/web/src/app/(app)/browse/[...path]/components/codePreviewPanel/markdownViewToggle.tsxpackages/web/src/app/(app)/browse/[...path]/page.tsxpackages/web/src/app/(app)/browse/hooks/utils.test.tspackages/web/src/app/(app)/browse/hooks/utils.ts
Repository-authored Markdown is untrusted browser content, so rendered browse previews must preserve existing source-navigation semantics while avoiding automatic remote asset loads. This keeps rendered links useful, turns images into explicit non-fetching links, and forces source mode for line-highlight URLs. Constraint: Browse markdown rendering is now the default path for Markdown blobs. Rejected: Auto-rendering markdown images | it can trigger third-party or local-network requests from viewer browsers. Confidence: high Scope-risk: narrow Directive: Keep repository Markdown rendering conservative unless a hardened asset proxy/raw-file pipeline is added. Tested: yarn workspace @sourcebot/web test src/app/(app)/browse/[...path]/components/codePreviewPanel/markdownPreview.test.tsx src/app/(app)/browse/[...path]/page.test.tsx src/app/(app)/browse/hooks/utils.test.ts; yarn workspace @sourcebot/web lint; yarn workspace @sourcebot/web build; yarn workspace @sourcebot/web test; git diff --check --cached Not-tested: Browser screenshot of rendered Markdown preview
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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
`@packages/web/src/app/`(app)/browse/[...path]/components/codePreviewPanel/markdownPreviewPanel.tsx:
- Around line 110-128: The markdown image renderer in markdownPreviewPanel
currently replaces every image with an anchor, so rendered README diagrams and
screenshots are lost. Update the img override to render an actual inline image
element for valid src values, using the existing getRelativeBrowseHref fallback
only to resolve the image source when needed, and keep the alt text for
accessibility. Preserve the current browse-aware URL handling in
markdownPreviewPanel and only fall back to a link when an image truly cannot be
rendered.
- Around line 65-77: The href builder in getRelativeBrowseHref always forces
pathType: 'blob', so markdown links ending in a trailing slash are routed to the
file preview instead of the directory tree. Update markdownPreviewPanel.tsx to
detect directory-style links (for example, paths ending in '/'), and when that
happens build a browse URL that uses the tree view path type instead of blob
while still preserving query and hash. Keep the existing resolveRepoRelativePath
and getBrowsePath flow, but branch on the link shape before returning the final
href.
🪄 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: a5f14a39-6ef8-49b5-b075-d5ddd99445ff
📒 Files selected for processing (5)
packages/web/src/app/(app)/browse/[...path]/components/codePreviewPanel/codePreviewPanel.tsxpackages/web/src/app/(app)/browse/[...path]/components/codePreviewPanel/markdownPreview.test.tsxpackages/web/src/app/(app)/browse/[...path]/components/codePreviewPanel/markdownPreviewPanel.tsxpackages/web/src/app/(app)/browse/[...path]/page.test.tsxpackages/web/src/app/(app)/browse/[...path]/page.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/web/src/app/(app)/browse/[...path]/page.tsx
- packages/web/src/app/(app)/browse/[...path]/components/codePreviewPanel/codePreviewPanel.tsx
Repository markdown links need to stay useful without introducing broken browse routes or invalid nested anchors. This decodes relative URL paths before browse-path generation, routes directory-style links to tree views, and converts markdown images through the AST so linked images remain a single link. Constraint: Rendered repository Markdown is untrusted user-authored content shown inside the browse UI. Rejected: Rendering images with a custom img component | linked image markdown can create nested interactive anchors. Confidence: high Scope-risk: narrow Directive: Keep markdown image handling non-fetching until a hardened raw asset route exists. Tested: yarn workspace @sourcebot/web test src/app/(app)/browse/[...path]/components/codePreviewPanel/markdownPreview.test.tsx src/app/(app)/browse/[...path]/page.test.tsx src/app/(app)/browse/hooks/utils.test.ts; yarn workspace @sourcebot/web lint; yarn workspace @sourcebot/web build; rm -rf packages/web/.next && yarn workspace @sourcebot/web test; git diff --check Not-tested: Browser screenshot of rendered Markdown preview
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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
`@packages/web/src/app/`(app)/browse/[...path]/components/codePreviewPanel/markdownPreviewPanel.tsx:
- Around line 85-94: The markdown browse URL builder is still passing an
undefined path into getBrowsePath for tree links, which can happen with
trailing-slash links like ../../. Update markdownPreviewPanel’s browse URL
assembly logic to reject resolvedPath === undefined before calling
getBrowsePath, while still allowing an empty string for repo-root tree links.
Use the existing resolvedPath, pathType, and getBrowsePath flow in
markdownPreviewPanel to keep the undefined case out of the URL builder.
🪄 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: 3ad9ec4b-4741-4266-8210-1908606d4451
📒 Files selected for processing (2)
packages/web/src/app/(app)/browse/[...path]/components/codePreviewPanel/markdownPreview.test.tsxpackages/web/src/app/(app)/browse/[...path]/components/codePreviewPanel/markdownPreviewPanel.tsx
Markdown previews need hash anchors and source-mode transitions to preserve existing browse navigation expectations. This adds heading ids for fragment links and carries source view through blame and focused diff entry/exit. Constraint: Rendered Markdown is now default for Markdown blobs, but source/blame/diff flows must remain stable. Rejected: Dropping view state at blame/diff boundaries | it returns source-mode users to rendered preview unexpectedly. Confidence: high Scope-risk: narrow Directive: Keep markdown preview routing state explicit when adding browse modes. Tested: node .yarn/releases/yarn-4.7.0.cjs workspace @sourcebot/web test 'src/app/(app)/browse/[...path]/components/codePreviewPanel/markdownPreview.test.tsx' 'src/app/(app)/browse/[...path]/components/codePreviewPanel/blameViewToggle.test.tsx' 'src/app/(app)/browse/[...path]/page.test.tsx' 'src/app/(app)/browse/hooks/utils.test.ts'; node .yarn/releases/yarn-4.7.0.cjs workspace @sourcebot/web lint; node .yarn/releases/yarn-4.7.0.cjs workspace @sourcebot/web build; rm -rf packages/web/.next && node .yarn/releases/yarn-4.7.0.cjs workspace @sourcebot/web test; git diff --check origin/main...HEAD && git diff --check Not-tested: Browser screenshot of source-to-blame-to-diff transition
Closing a commit preview should not reset users who chose raw source back to rendered Markdown. Preserve the active source view in the preview-close browse URL and lock it with a regression test. Constraint: Markdown previews default to rendered mode, but explicit source-mode navigation must remain sticky across preview, blame, and diff paths. Rejected: Letting preview close omit view state | it creates an unexpected rendered-preview transition for source-mode users. Confidence: high Scope-risk: narrow Directive: When adding browse transitions, carry BlobViewMode intentionally instead of relying on defaults. Tested: node .yarn/releases/yarn-4.7.0.cjs workspace @sourcebot/web test 'src/app/(app)/browse/[...path]/components/codePreviewPanel/markdownPreview.test.tsx' 'src/app/(app)/browse/[...path]/components/codePreviewPanel/blameViewToggle.test.tsx' 'src/app/(app)/browse/[...path]/page.test.tsx' 'src/app/(app)/browse/hooks/utils.test.ts' (4 files, 45 tests); git diff --check origin/main...HEAD && git diff --check; node .yarn/releases/yarn-4.7.0.cjs workspace @sourcebot/web lint (0 errors, 5 existing warnings); node .yarn/releases/yarn-4.7.0.cjs workspace @sourcebot/web build; rm -rf packages/web/.next && node .yarn/releases/yarn-4.7.0.cjs workspace @sourcebot/web test (64 files, 814 tests) Not-tested: Browser screenshot of source-to-preview-close transition
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/web/src/app/(app)/browse/[...path]/components/codePreviewPanel/blameViewToggle.test.tsx (1)
34-36: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert query params semantically, not by full serialized URL.
These expectations are order-sensitive even though the behavior under test is just which params exist. Parsing the pushed URL and checking
pathnameplussearchParamswill make the suite more stable.♻️ Suggested test shape
- expect(mocks.push).toHaveBeenCalledWith( - '/browse/github.com/sourcebot-dev/sourcebot@main/-/blob/README.md?blame=true&view=source' - ); + const [href] = mocks.push.mock.calls.at(-1)!; + const url = new URL(href, 'http://localhost'); + expect(url.pathname).toBe('/browse/github.com/sourcebot-dev/sourcebot@main/-/blob/README.md'); + expect(url.searchParams.get('blame')).toBe('true'); + expect(url.searchParams.get('view')).toBe('source');Also applies to: 52-54, 70-72
🤖 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 `@packages/web/src/app/`(app)/browse/[...path]/components/codePreviewPanel/blameViewToggle.test.tsx around lines 34 - 36, The URL assertions in blameViewToggle.test.tsx are too brittle because they depend on the exact serialized query string order. Update the expectations in the affected tests for the push call in the blameViewToggle suite to parse the passed URL and assert the pathname and individual searchParams values instead of comparing the full string. Use the existing mocks.push assertions and check for the presence of view and blame parameters semantically.
🤖 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
`@packages/web/src/app/`(app)/browse/[...path]/components/codePreviewPanel/blameViewToggle.test.tsx:
- Around line 34-36: The URL assertions in blameViewToggle.test.tsx are too
brittle because they depend on the exact serialized query string order. Update
the expectations in the affected tests for the push call in the blameViewToggle
suite to parse the passed URL and assert the pathname and individual
searchParams values instead of comparing the full string. Use the existing
mocks.push assertions and check for the presence of view and blame parameters
semantically.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ccd27914-37da-426d-b257-d8aedb088849
📒 Files selected for processing (9)
packages/web/src/app/(app)/browse/[...path]/components/codePreviewPanel/blameViewToggle.test.tsxpackages/web/src/app/(app)/browse/[...path]/components/codePreviewPanel/blameViewToggle.tsxpackages/web/src/app/(app)/browse/[...path]/components/codePreviewPanel/codePreviewPanel.tsxpackages/web/src/app/(app)/browse/[...path]/components/codePreviewPanel/markdownPreview.test.tsxpackages/web/src/app/(app)/browse/[...path]/components/codePreviewPanel/markdownPreviewPanel.tsxpackages/web/src/app/(app)/browse/[...path]/components/codePreviewPanel/pureCodePreviewPanel.tsxpackages/web/src/app/(app)/browse/[...path]/components/commitDiffPanel/focusedCommitDiffPanel.tsxpackages/web/src/app/(app)/browse/[...path]/page.test.tsxpackages/web/src/app/(app)/browse/[...path]/page.tsx
🚧 Files skipped from review as they are similar to previous changes (4)
- packages/web/src/app/(app)/browse/[...path]/page.tsx
- packages/web/src/app/(app)/browse/[...path]/page.test.tsx
- packages/web/src/app/(app)/browse/[...path]/components/codePreviewPanel/codePreviewPanel.tsx
- packages/web/src/app/(app)/browse/[...path]/components/codePreviewPanel/markdownPreviewPanel.tsx
Markdown source mode should remain sticky when users enter historical previews or focused diffs from file history. Thread the current source view through those history links and label the non-blame state as Preview or Source for Markdown so the toolbar matches the rendered panel. Constraint: Rendered Markdown remains the default URL state, so only explicit source mode should add view=source. Rejected: Preserving source only on exit links | history entry links can already drop the mode before the exit path runs. Confidence: high Scope-risk: narrow Directive: Carry BlobViewMode at both entry and exit points for any new blob navigation path. Tested: node .yarn/releases/yarn-4.7.0.cjs workspace @sourcebot/web test 'src/app/(app)/browse/[...path]/components/codePreviewPanel/markdownPreview.test.tsx' 'src/app/(app)/browse/[...path]/components/codePreviewPanel/blameViewToggle.test.tsx' 'src/app/(app)/browse/[...path]/components/commitDiffPanel/focusedCommitDiffPanel.test.tsx' 'src/app/(app)/browse/components/historyRow.test.tsx' 'src/app/(app)/browse/[...path]/page.test.tsx' 'src/app/(app)/browse/hooks/utils.test.ts' (6 files, 50 tests); git diff --check origin/main...HEAD && git diff --check; node .yarn/releases/yarn-4.7.0.cjs workspace @sourcebot/web lint (0 errors, 5 existing warnings); node .yarn/releases/yarn-4.7.0.cjs workspace @sourcebot/web build; rm -rf packages/web/.next && node .yarn/releases/yarn-4.7.0.cjs workspace @sourcebot/web test (66 files, 819 tests) Not-tested: Browser screenshot of the file-history source-mode round trip
Markdown preview state needs to stay consistent across implicit source-mode links and every supported image syntax. Preserve source mode for highlight-driven history links, convert reference-style images into the same non-fetching links as inline images, and lock the Markdown view toggle labels and active state with direct tests. Constraint: This PR keeps repository Markdown images non-fetching until Sourcebot has a dedicated raw asset pipeline. Rejected: Adding a broad raw-file endpoint here | it expands the PR beyond browse rendering and needs separate binary/auth/cache design. Confidence: high Scope-risk: narrow Directive: Treat new Markdown URL syntaxes as untrusted until tests prove they cannot trigger unexpected fetches or state loss. Tested: node .yarn/releases/yarn-4.7.0.cjs workspace @sourcebot/web test 'src/app/(app)/browse/[...path]/components/codePreviewPanel/markdownPreview.test.tsx' 'src/app/(app)/browse/[...path]/components/codePreviewPanel/markdownViewToggle.test.tsx' 'src/app/(app)/browse/[...path]/components/codePreviewPanel/blameViewToggle.test.tsx' 'src/app/(app)/browse/[...path]/components/commitDiffPanel/focusedCommitDiffPanel.test.tsx' 'src/app/(app)/browse/components/historyRow.test.tsx' 'src/app/(app)/browse/[...path]/page.test.tsx' 'src/app/(app)/browse/hooks/utils.test.ts' (7 files, 55 tests); git diff --check origin/main...HEAD && git diff --check; node .yarn/releases/yarn-4.7.0.cjs workspace @sourcebot/web lint (0 errors, 5 existing warnings); node .yarn/releases/yarn-4.7.0.cjs workspace @sourcebot/web build; rm -rf packages/web/.next && node .yarn/releases/yarn-4.7.0.cjs workspace @sourcebot/web test (67 files, 824 tests) Not-tested: Browser screenshot of reference-style image Markdown
Fixes #794
Problem
Markdown blobs in the browse view currently open as raw CodeMirror source. That makes README and documentation files harder to read, and issue #794 specifically asks for rendered Markdown with an option to switch back to source.
Root cause
CodePreviewPanelsends every blob through the source editor path. There was no Markdown-specific view mode, even though the web package already depends onreact-markdownandremark-gfm.Solution
react-markdownandremark-gfmby default.Preview/Sourcetoggle backed by?view=sourceso the raw CodeMirror source path remains available.view=sourcethrough blame entry/exit, history preview/focused-diff entry, highlight-driven history links, focused commit diff exit, and commit-preview close navigation so source-mode users are not returned to rendered preview unexpectedly.PrevieworSourcefor Markdown files so the visible and accessible state matches the rendered panel.Tests
node .yarn/releases/yarn-4.7.0.cjs workspace @sourcebot/web test 'src/app/(app)/browse/[...path]/components/codePreviewPanel/markdownPreview.test.tsx' 'src/app/(app)/browse/[...path]/components/codePreviewPanel/markdownViewToggle.test.tsx' 'src/app/(app)/browse/[...path]/components/codePreviewPanel/blameViewToggle.test.tsx' 'src/app/(app)/browse/[...path]/components/commitDiffPanel/focusedCommitDiffPanel.test.tsx' 'src/app/(app)/browse/components/historyRow.test.tsx' 'src/app/(app)/browse/[...path]/page.test.tsx' 'src/app/(app)/browse/hooks/utils.test.ts'(7 files, 55 tests)node .yarn/releases/yarn-4.7.0.cjs workspace @sourcebot/web lint(passes with the five existing unrelated warnings)node .yarn/releases/yarn-4.7.0.cjs workspace @sourcebot/web buildrm -rf packages/web/.next && node .yarn/releases/yarn-4.7.0.cjs workspace @sourcebot/web test(67 files, 824 tests)git diff --check origin/main...HEAD && git diff --checkRisk
Moderate-low. The change is scoped to Markdown blob rendering and browse navigation state in the browse page. Non-Markdown files, blame mode, commit preview mode, focused diffs, line-highlight links, and explicit source mode still use the existing CodeMirror or diff path. The main product tradeoff is conservative image handling: repository Markdown images are linked rather than auto-loaded until Sourcebot has a hardened raw asset pipeline.
Summary by CodeRabbit
view=sourcecorrectly select Raw Source, while focused diff views take precedence.