Skip to content

Commit 8b6b4f9

Browse files
refactor: improve error handling and split large components (#3817)
## Summary - **Backend**: Add specific exception types (`DatabaseQueryError`, `CacheError`) and improve error handling in plots router - **Frontend**: Split large `SpecPage.tsx` (807→500 lines) into `SpecOverview` and `SpecDetailView` components - **Frontend**: Split large `useFilterState.ts` (375→258 lines) into `useUrlSync` and `useFilterFetch` hooks ## Test plan - [x] All 863 unit tests pass - [x] Ruff lint check passes - [x] Ruff format check passes - [x] Manual testing: Homepage, SpecPage overview, SpecPage detail, Filter API - [x] No console errors or warnings in browser DevTools - [x] All API calls return 200 OK 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
1 parent 6844f02 commit 8b6b4f9

File tree

16 files changed

+1134
-836
lines changed

16 files changed

+1134
-836
lines changed

api/exceptions.py

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,15 @@ def __init__(self, detail: str):
7474
super().__init__(f"Validation failed: {detail}", status_code=400)
7575

7676

77+
class DatabaseQueryError(PyplotsException):
78+
"""Database query failed (500)."""
79+
80+
def __init__(self, operation: str, detail: str):
81+
message = f"Database query failed during '{operation}': {detail}"
82+
super().__init__(message, status_code=500)
83+
self.operation = operation
84+
85+
7786
# ===== Exception Handlers =====
7887

7988

@@ -153,3 +162,17 @@ def raise_validation_error(detail: str) -> None:
153162
ValidationError: Always raises
154163
"""
155164
raise ValidationError(detail)
165+
166+
167+
def raise_database_query_error(operation: str, detail: str) -> None:
168+
"""
169+
Raise a standardized 500 error for database query failures.
170+
171+
Args:
172+
operation: The operation that failed (e.g., "fetch_specs", "filter_plots")
173+
detail: Error details
174+
175+
Raises:
176+
DatabaseQueryError: Always raises
177+
"""
178+
raise DatabaseQueryError(operation, detail)

api/routers/plots.py

Lines changed: 27 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,21 @@
11
"""Filter endpoint for plots."""
22

3+
import logging
4+
35
from fastapi import APIRouter, Depends, Request
6+
from sqlalchemy.exc import SQLAlchemyError
47
from sqlalchemy.ext.asyncio import AsyncSession
58

69
from api.cache import get_cache, set_cache
710
from api.dependencies import require_db
11+
from api.exceptions import DatabaseQueryError
812
from api.schemas import FilteredPlotsResponse
913
from core.database import SpecRepository
1014

1115

16+
logger = logging.getLogger(__name__)
17+
18+
1219
router = APIRouter(tags=["plots"])
1320

1421

@@ -446,13 +453,21 @@ async def get_filtered_plots(request: Request, db: AsyncSession = Depends(requir
446453

447454
# Check cache
448455
cache_key = _build_cache_key(filter_groups)
449-
cached = get_cache(cache_key)
450-
if cached:
451-
return cached
456+
try:
457+
cached = get_cache(cache_key)
458+
if cached:
459+
return cached
460+
except Exception as e:
461+
# Cache failures are non-fatal, log and continue
462+
logger.warning("Cache read failed for key %s: %s", cache_key, e)
452463

453464
# Fetch data from database
454-
repo = SpecRepository(db)
455-
all_specs = await repo.get_all()
465+
try:
466+
repo = SpecRepository(db)
467+
all_specs = await repo.get_all()
468+
except SQLAlchemyError as e:
469+
logger.error("Database query failed in get_filtered_plots: %s", e)
470+
raise DatabaseQueryError("fetch_specs", str(e)) from e
456471

457472
# Build data structures
458473
spec_lookup = _build_spec_lookup(all_specs)
@@ -476,5 +491,11 @@ async def get_filtered_plots(request: Request, db: AsyncSession = Depends(requir
476491
globalCounts=global_counts,
477492
orCounts=or_counts,
478493
)
479-
set_cache(cache_key, result)
494+
495+
try:
496+
set_cache(cache_key, result)
497+
except Exception as e:
498+
# Cache failures are non-fatal, log and continue
499+
logger.warning("Cache write failed for key %s: %s", cache_key, e)
500+
480501
return result

app/src/components/Breadcrumb.tsx

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,88 @@
1+
/**
2+
* Shared Breadcrumb component for consistent navigation across pages.
3+
*/
4+
5+
import { Link } from 'react-router-dom';
6+
import Box from '@mui/material/Box';
7+
import type { SxProps, Theme } from '@mui/material/styles';
8+
9+
export interface BreadcrumbItem {
10+
label: string;
11+
to?: string; // If undefined, this is the current page (not linked)
12+
}
13+
14+
export interface BreadcrumbProps {
15+
items: BreadcrumbItem[];
16+
rightAction?: React.ReactNode;
17+
/** Additional sx props for the container */
18+
sx?: SxProps<Theme>;
19+
}
20+
21+
/**
22+
* Breadcrumb navigation component.
23+
*
24+
* @example
25+
* // Simple: pyplots.ai > catalog
26+
* <Breadcrumb items={[{ label: 'pyplots.ai', to: '/' }, { label: 'catalog' }]} />
27+
*
28+
* @example
29+
* // With right action
30+
* <Breadcrumb
31+
* items={[{ label: 'pyplots.ai', to: '/' }, { label: 'catalog' }]}
32+
* rightAction={<Link to="/suggest">suggest spec</Link>}
33+
* />
34+
*/
35+
export function Breadcrumb({ items, rightAction, sx }: BreadcrumbProps) {
36+
return (
37+
<Box
38+
sx={{
39+
display: 'flex',
40+
justifyContent: 'space-between',
41+
alignItems: 'center',
42+
mx: { xs: -2, sm: -4, md: -8, lg: -12 },
43+
mt: -5,
44+
px: 2,
45+
py: 1,
46+
mb: 2,
47+
bgcolor: '#f3f4f6',
48+
borderBottom: '1px solid #e5e7eb',
49+
fontFamily: '"MonoLisa", monospace',
50+
fontSize: '0.85rem',
51+
...sx,
52+
}}
53+
>
54+
{/* Breadcrumb links */}
55+
<Box sx={{ display: 'flex', alignItems: 'center' }}>
56+
{items.map((item, index) => (
57+
<Box key={index} sx={{ display: 'flex', alignItems: 'center' }}>
58+
{index > 0 && (
59+
<Box component="span" sx={{ mx: 1, color: '#9ca3af' }}>
60+
61+
</Box>
62+
)}
63+
{item.to ? (
64+
<Box
65+
component={Link}
66+
to={item.to}
67+
sx={{
68+
color: '#3776AB',
69+
textDecoration: 'none',
70+
'&:hover': { textDecoration: 'underline' },
71+
}}
72+
>
73+
{item.label}
74+
</Box>
75+
) : (
76+
<Box component="span" sx={{ color: '#4b5563' }}>
77+
{item.label}
78+
</Box>
79+
)}
80+
</Box>
81+
))}
82+
</Box>
83+
84+
{/* Right action slot */}
85+
{rightAction}
86+
</Box>
87+
);
88+
}
Lines changed: 189 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,189 @@
1+
/**
2+
* SpecDetailView component - Single implementation detail view.
3+
*
4+
* Shows large image with library carousel and action buttons.
5+
*/
6+
7+
import { Link } from 'react-router-dom';
8+
import Box from '@mui/material/Box';
9+
import IconButton from '@mui/material/IconButton';
10+
import Tooltip from '@mui/material/Tooltip';
11+
import Skeleton from '@mui/material/Skeleton';
12+
import DownloadIcon from '@mui/icons-material/Download';
13+
import OpenInNewIcon from '@mui/icons-material/OpenInNew';
14+
import ContentCopyIcon from '@mui/icons-material/ContentCopy';
15+
import CheckIcon from '@mui/icons-material/Check';
16+
17+
import type { Implementation } from '../types';
18+
19+
interface SpecDetailViewProps {
20+
specId: string;
21+
specTitle: string;
22+
selectedLibrary: string;
23+
currentImpl: Implementation | null;
24+
implementations: Implementation[];
25+
imageLoaded: boolean;
26+
codeCopied: string | null;
27+
onImageLoad: () => void;
28+
onImageClick: () => void;
29+
onCopyCode: (impl: Implementation) => void;
30+
onDownload: (impl: Implementation) => void;
31+
onTrackEvent: (event: string, props?: Record<string, string | undefined>) => void;
32+
}
33+
34+
export function SpecDetailView({
35+
specId,
36+
specTitle,
37+
selectedLibrary,
38+
currentImpl,
39+
implementations,
40+
imageLoaded,
41+
codeCopied,
42+
onImageLoad,
43+
onImageClick,
44+
onCopyCode,
45+
onDownload,
46+
onTrackEvent,
47+
}: SpecDetailViewProps) {
48+
// Sort implementations alphabetically for the counter
49+
const sortedImpls = [...implementations].sort((a, b) => a.library_id.localeCompare(b.library_id));
50+
const currentIndex = sortedImpls.findIndex((impl) => impl.library_id === selectedLibrary);
51+
52+
return (
53+
<Box
54+
sx={{
55+
maxWidth: { xs: '100%', md: 1200, lg: 1400, xl: 1600 },
56+
mx: 'auto',
57+
}}
58+
>
59+
<Box
60+
onClick={onImageClick}
61+
sx={{
62+
position: 'relative',
63+
borderRadius: 2,
64+
overflow: 'hidden',
65+
bgcolor: '#fff',
66+
boxShadow: '0 2px 8px rgba(0,0,0,0.08)',
67+
aspectRatio: '16/9',
68+
cursor: 'pointer',
69+
'&:hover .impl-counter': {
70+
opacity: 1,
71+
},
72+
}}
73+
>
74+
{!imageLoaded && (
75+
<Skeleton
76+
variant="rectangular"
77+
sx={{
78+
position: 'absolute',
79+
inset: 0,
80+
width: '100%',
81+
height: '100%',
82+
}}
83+
/>
84+
)}
85+
{currentImpl?.preview_url && (
86+
<Box
87+
component="img"
88+
src={currentImpl.preview_url}
89+
alt={`${specTitle} - ${selectedLibrary}`}
90+
onLoad={onImageLoad}
91+
sx={{
92+
width: '100%',
93+
height: '100%',
94+
objectFit: 'contain',
95+
display: imageLoaded ? 'block' : 'none',
96+
}}
97+
/>
98+
)}
99+
100+
{/* Action Buttons (top-right) - stop propagation */}
101+
<Box
102+
onClick={(e) => e.stopPropagation()}
103+
sx={{
104+
position: 'absolute',
105+
top: 8,
106+
right: 8,
107+
display: 'flex',
108+
gap: 0.5,
109+
}}
110+
>
111+
{currentImpl?.code && (
112+
<Tooltip title={codeCopied === currentImpl.library_id ? 'Copied!' : 'Copy Code'}>
113+
<IconButton
114+
onClick={() => onCopyCode(currentImpl)}
115+
sx={{
116+
bgcolor: 'rgba(255,255,255,0.9)',
117+
'&:hover': { bgcolor: '#fff' },
118+
}}
119+
size="small"
120+
>
121+
{codeCopied === currentImpl.library_id ? (
122+
<CheckIcon fontSize="small" color="success" />
123+
) : (
124+
<ContentCopyIcon fontSize="small" />
125+
)}
126+
</IconButton>
127+
</Tooltip>
128+
)}
129+
{currentImpl && (
130+
<Tooltip title="Download PNG">
131+
<IconButton
132+
onClick={() => onDownload(currentImpl)}
133+
sx={{
134+
bgcolor: 'rgba(255,255,255,0.9)',
135+
'&:hover': { bgcolor: '#fff' },
136+
}}
137+
size="small"
138+
>
139+
<DownloadIcon fontSize="small" />
140+
</IconButton>
141+
</Tooltip>
142+
)}
143+
{currentImpl?.preview_html && (
144+
<Tooltip title="Open Interactive">
145+
<IconButton
146+
component={Link}
147+
to={`/interactive/${specId}/${selectedLibrary}`}
148+
onClick={(e: React.MouseEvent) => {
149+
e.stopPropagation();
150+
onTrackEvent('open_interactive', { spec: specId, library: selectedLibrary });
151+
}}
152+
sx={{
153+
bgcolor: 'rgba(255,255,255,0.9)',
154+
'&:hover': { bgcolor: '#fff' },
155+
}}
156+
size="small"
157+
>
158+
<OpenInNewIcon fontSize="small" />
159+
</IconButton>
160+
</Tooltip>
161+
)}
162+
</Box>
163+
164+
{/* Implementation counter (hover) */}
165+
{implementations.length > 1 && (
166+
<Box
167+
className="impl-counter"
168+
sx={{
169+
position: 'absolute',
170+
bottom: 8,
171+
right: 8,
172+
px: 1,
173+
py: 0.25,
174+
bgcolor: 'rgba(0,0,0,0.6)',
175+
borderRadius: 1,
176+
fontSize: '0.75rem',
177+
fontFamily: '"MonoLisa", monospace',
178+
color: '#fff',
179+
opacity: 0,
180+
transition: 'opacity 0.2s',
181+
}}
182+
>
183+
{currentIndex + 1}/{implementations.length}
184+
</Box>
185+
)}
186+
</Box>
187+
</Box>
188+
);
189+
}

0 commit comments

Comments
 (0)