Skip to content

Conversation

@tianzhou
Copy link
Contributor

No description provided.

Copilot AI review requested due to automatic review settings November 14, 2025 11:09
Copilot finished reviewing on behalf of tianzhou November 14, 2025 11:12
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR removes the home tab/view and replaces it with automatic redirection to the first data source. The home route now redirects users directly to the first available source, streamlining the user experience by eliminating an intermediary landing page.

  • Removed the HomeView component and its route
  • Added NotFoundView component with a 404 page
  • Implemented automatic redirection from "/" to the first data source
  • Cleaned up unused navigation components (NavItem, HomeIcon) from the sidebar

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
frontend/src/App.tsx Added redirect logic to navigate to first source on home route; added NotFoundView route; removed HomeView import
frontend/src/components/views/HomeView.tsx Deleted the entire home view component
frontend/src/components/views/NotFoundView.tsx Added new 404 error page with helpful navigation
frontend/src/components/views/SourceDetailView.tsx Added error handling to redirect to 404 page for "not found" errors
frontend/src/components/Sidebar/Sidebar.tsx Removed Home navigation item and related imports (NavItem, HomeIcon)


if (error) {
// If source not found, redirect to 404 page
if (error.includes('not found') || error.includes('404')) {
Copy link

Copilot AI Nov 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The error message check using string comparison is fragile and may not catch all 404 cases. The API already returns a 404 status code (see fetchSource in api/sources.ts line 44-45), but the error message format is "Source not found: {sourceId}". Consider checking for "Source not found" instead of "not found" to be more specific, or better yet, parse the HTTP status code from the error if available.

Copilot uses AI. Check for mistakes.
Comment on lines +20 to +23
// This should never happen as backend validates at least one source
return (
<div className="container mx-auto px-8 py-12">
<div className="text-destructive">No data sources configured</div>
Copy link

Copilot AI Nov 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment "This should never happen as backend validates at least one source" is misleading. The backend validation happens at startup, but there could be scenarios where sources fail to load due to network errors, API issues, or other runtime problems. This case should be handled more gracefully, perhaps with an error message suggesting the user check their configuration or try refreshing.

Suggested change
// This should never happen as backend validates at least one source
return (
<div className="container mx-auto px-8 py-12">
<div className="text-destructive">No data sources configured</div>
// Sources may be empty due to network errors, API issues, or misconfiguration.
return (
<div className="container mx-auto px-8 py-12">
<div className="text-destructive">
No data sources available.<br />
Please check your configuration or try refreshing the page.
</div>

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants