fix(web): show missing-file state in browse#1381
Open
DivyamTalwar wants to merge 7 commits into
Open
Conversation
Render a dedicated missing-file state when the browse file-source API returns FILE_NOT_FOUND, preserving the active revision in the return-to-root link. Constraint: Prior closed PR sourcebot-dev#837 lost branch context and introduced a client-side component; this keeps the new panel server-rendered and revision-aware. Rejected: Keeping the raw service-error text | users need a browse-specific recovery path for deleted, moved, or branch-specific files. Confidence: high Scope-risk: narrow Directive: Keep other file-source service errors on the existing diagnostic error path; only FILE_NOT_FOUND should render this panel. Tested: node .yarn/releases/yarn-4.7.0.cjs workspace @sourcebot/web test 'src/app/(app)/browse/[...path]/components/codePreviewPanel/codePreviewPanel.test.tsx' Tested: node .yarn/releases/yarn-4.7.0.cjs workspace @sourcebot/web lint Not-tested: Manual browser navigation to a missing blob path.
Contributor
|
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:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
WalkthroughA dedicated missing-file panel is added for browse views. ChangesFile Not Found State
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
Constraint: Sourcebot requires a follow-up changelog entry for each PR under the matching Unreleased section. Confidence: high Scope-risk: narrow Directive: Keep this commit scoped to the changelog entry for PR sourcebot-dev#1381. Tested: Not run; changelog-only follow-up. Not-tested: Runtime tests unchanged from the implementation commit.
Merge upstream main after sourcebot-dev#1376 changed the same Unreleased changelog block, keeping both release-note entries. Constraint: GitHub reported PR sourcebot-dev#1381 as dirty against current main.\nRejected: Rewriting the open PR branch | Avoids force-pushing an existing review branch.\nConfidence: high\nScope-risk: narrow\nDirective: This merge commit should only resolve the changelog ordering conflict.\nTested: git diff --check\nNot-tested: Targeted regression test rerun follows after the merge commit.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
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/codePreviewPanel.tsx:
- Around line 66-69: The FileNotFoundPanel is using the fetch ref instead of the
user’s browse revision, which can send the recovery link to the wrong root when
previewRef is present. In codePreviewPanel.tsx, update the FileNotFoundPanel
call to pass the browse revision value for its revisionName prop, and introduce
a separate prop for previewRef only if the panel needs it for display copy. Use
the codePreviewPanel and FileNotFoundPanel symbols to locate and adjust the prop
wiring.
🪄 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: 73b03735-ab51-40db-8940-7ef21daeae8d
📒 Files selected for processing (4)
CHANGELOG.mdpackages/web/src/app/(app)/browse/[...path]/components/codePreviewPanel/codePreviewPanel.test.tsxpackages/web/src/app/(app)/browse/[...path]/components/codePreviewPanel/codePreviewPanel.tsxpackages/web/src/app/(app)/browse/[...path]/components/codePreviewPanel/fileNotFoundPanel.tsx
Split the ref used for failed content lookup from the ref used for browse navigation, so preview-mode 404s keep their normal return path. Constraint: Existing previewRef semantics fetch content at the preview ref while keeping the surrounding browse context at revisionName. Rejected: Reusing contentRef for all missing-file panel navigation | that sends preview 404 recovery links to the commit root instead of the active browse revision. Confidence: high Scope-risk: narrow Directive: Keep missing-file copy tied to the failed content ref, but keep navigation tied to the browse revision. Tested: node .yarn/releases/yarn-4.7.0.cjs workspace @sourcebot/web test 'src/app/(app)/browse/[...path]/components/codePreviewPanel/codePreviewPanel.test.tsx' Tested: node .yarn/releases/yarn-4.7.0.cjs workspace @sourcebot/web lint Tested: git diff --check origin/main...HEAD Not-tested: Manual browser navigation through a real deleted-file preview link.
Contributor
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/codePreviewPanel.test.tsx:
- Around line 96-98: The root-link test in codePreviewPanel.test.tsx is
hard-coding the browse URL without the trailing slash, so it can fail even when
the component is correct. Update the expectation in the Return to repository
root assertion to match the actual output from getBrowsePath({ pathType: 'tree',
path: '' }) and keep the test aligned with the href produced by
codePreviewPanel.
🪄 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: b9b1d038-f9f2-4701-807e-aae9c554d298
📒 Files selected for processing (3)
packages/web/src/app/(app)/browse/[...path]/components/codePreviewPanel/codePreviewPanel.test.tsxpackages/web/src/app/(app)/browse/[...path]/components/codePreviewPanel/codePreviewPanel.tsxpackages/web/src/app/(app)/browse/[...path]/components/codePreviewPanel/fileNotFoundPanel.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/web/src/app/(app)/browse/[...path]/components/codePreviewPanel/codePreviewPanel.tsx
Keep preview-mode header navigation anchored to the active browse revision while file content still loads from the preview ref. Constraint: The browse route treats previewRef as a content fetch override, not as the surrounding browse context. Rejected: Leaving PathHeader on contentRef | successful previews would keep breadcrumb/root navigation on the preview commit. Confidence: high Scope-risk: narrow Directive: Use contentRef only for content retrieval and editor context; use revisionName for browse navigation surfaces. Tested: node .yarn/releases/yarn-4.7.0.cjs workspace @sourcebot/web test 'src/app/(app)/browse/[...path]/components/codePreviewPanel/codePreviewPanel.test.tsx' Tested: node .yarn/releases/yarn-4.7.0.cjs workspace @sourcebot/web lint Tested: git diff --check origin/main...HEAD Not-tested: Manual browser navigation through a successful preview link.
Use the tooltip trigger as the existing link control so preview-mode close remains valid accessible markup. Constraint: The successful-preview header now participates in the same browse-context contract as the missing-file state. Rejected: Leaving a button wrapper around the link | creates nested interactive controls and weakens keyboard/accessibility behavior. Confidence: high Scope-risk: narrow Directive: Keep tooltip triggers composed with `asChild` when the child is already the interactive control. Tested: node .yarn/releases/yarn-4.7.0.cjs workspace @sourcebot/web test 'src/app/(app)/browse/[...path]/components/codePreviewPanel/codePreviewPanel.test.tsx' Tested: node .yarn/releases/yarn-4.7.0.cjs workspace @sourcebot/web lint Tested: git diff --check origin/main...HEAD Not-tested: Manual browser hover/focus over the preview close tooltip.
Prevent long missing paths and refs from overflowing the browse 404 panel on narrow panes. Constraint: Missing-file copy can contain full repository paths and full preview commit refs. Rejected: Truncating the missing path/ref | users need exact text for recovery and debugging. Confidence: high Scope-risk: narrow Directive: Prefer wrapping over truncation for diagnostic identifiers in error-state copy. Tested: node .yarn/releases/yarn-4.7.0.cjs workspace @sourcebot/web test 'src/app/(app)/browse/[...path]/components/codePreviewPanel/codePreviewPanel.test.tsx' Tested: node .yarn/releases/yarn-4.7.0.cjs workspace @sourcebot/web lint Tested: git diff --check origin/main...HEAD Not-tested: Manual browser resize with an extremely long missing path.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #827
Problem
When a user browses to a blob path that does not exist, Sourcebot currently renders the raw service error (
Error loading file source: ...). This can happen after changing branches, following an old link, or browsing a file that was deleted or moved.Root cause
CodePreviewPaneltreats allgetFileSourceservice errors the same, soFILE_NOT_FOUNDnever gets a browse-specific recovery UI. Preview-mode browse also needs to keep two refs separate: the content ref used for the file lookup and the active browse revision used for navigation.Solution
getFileSourcereturnsFILE_NOT_FOUND.Return to repository rootlink.?ref=still shows the preview ref, butClose previewand root navigation return to the original browse revision.FILE_NOT_FOUNDservice errors on the existing diagnostic path.This also avoids the review issues from the old closed PR #837: the new panel is server-rendered and the root link keeps branch context.
Tests
node .yarn/releases/yarn-4.7.0.cjs workspace @sourcebot/web test 'src/app/(app)/browse/[...path]/components/codePreviewPanel/codePreviewPanel.test.tsx'node .yarn/releases/yarn-4.7.0.cjs workspace @sourcebot/web lintgit diff --check origin/main...HEADRisk
Low. The branch is gated on the existing
FILE_NOT_FOUNDerror code, and all other file-source errors still render their existing message. Preview ref handling is covered for both missing-file and successful-preview states.Not tested
Manual browser navigation to a missing blob path.
Summary by CodeRabbit
Bug Fixes
Tests