Skip to content

Commit daecdae

Browse files
pablogsalpablogsal
committed
Fix python version filtering and add better error handling
1 parent e454271 commit daecdae

File tree

11 files changed

+210
-25
lines changed

11 files changed

+210
-25
lines changed

‎backend/app/crud.py‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,8 @@ async def get_benchmark_trends(
1919
benchmark_name: str,
2020
binary_id: str,
2121
environment_id: str,
22+
python_major: int,
23+
python_minor: int,
2224
limit: int = 50,
2325
) -> List[Dict[str, Any]]:
2426
"""
@@ -40,6 +42,8 @@ async def get_benchmark_trends(
4042
WHERE r.binary_id = :binary_id
4143
AND r.environment_id = :environment_id
4244
AND br.benchmark_name = :benchmark_name
45+
AND c.python_major = :python_major
46+
AND c.python_minor = :python_minor
4347
ORDER BY c.timestamp DESC
4448
LIMIT :limit
4549
""")
@@ -50,6 +54,8 @@ async def get_benchmark_trends(
5054
"benchmark_name": benchmark_name,
5155
"binary_id": binary_id,
5256
"environment_id": environment_id,
57+
"python_major": python_major,
58+
"python_minor": python_minor,
5359
"limit": limit,
5460
},
5561
)

‎backend/app/routers/benchmarks.py‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -163,6 +163,8 @@ async def get_benchmark_trends(
163163
benchmark_name: str,
164164
binary_id: str,
165165
environment_id: str,
166+
python_major: int,
167+
python_minor: int,
166168
limit: int = 50,
167169
db: AsyncSession = Depends(get_database),
168170
):
@@ -175,6 +177,8 @@ async def get_benchmark_trends(
175177
benchmark_name=benchmark_name,
176178
binary_id=binary_id,
177179
environment_id=environment_id,
180+
python_major=python_major,
181+
python_minor=python_minor,
178182
limit=limit,
179183
)
180184
return trends
@@ -199,6 +203,8 @@ async def get_batch_benchmark_trends(
199203
benchmark_name=trend_query.benchmark_name,
200204
binary_id=trend_query.binary_id,
201205
environment_id=trend_query.environment_id,
206+
python_major=trend_query.python_major,
207+
python_minor=trend_query.python_minor,
202208
limit=trend_query.limit,
203209
)
204210
results[key] = trends

‎backend/app/schemas.py‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -153,6 +153,8 @@ class TrendRequest(BaseModel):
153153
benchmark_name: str
154154
binary_id: str
155155
environment_id: str
156+
python_major: int
157+
python_minor: int
156158
limit: int = 50
157159

158160

‎frontend/src/app/binaries/[id]/page.tsx‎

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ import { api } from '@/lib/api';
3939
import { format } from 'date-fns';
4040
import Link from 'next/link';
4141
import { useRouter } from 'next/navigation';
42+
import { useToast } from '@/hooks/use-toast';
4243

4344
type EnvironmentData = {
4445
id: string;
@@ -241,6 +242,7 @@ export default function BinaryDetailPage({
241242
{}
242243
);
243244
const [error, setError] = useState<string | null>(null);
245+
const { toast } = useToast();
244246

245247
useEffect(() => {
246248
async function loadBinaryAndEnvironments() {
@@ -263,9 +265,13 @@ export default function BinaryDetailPage({
263265
setBinary(binaryData);
264266
setEnvironments(environmentsData);
265267
} catch (err) {
266-
setError(
267-
err instanceof Error ? err.message : 'Failed to load binary details'
268-
);
268+
const errorMessage = err instanceof Error ? err.message : 'Failed to load binary details';
269+
setError(errorMessage);
270+
toast({
271+
title: 'Error',
272+
description: errorMessage,
273+
variant: 'destructive',
274+
});
269275
} finally {
270276
setLoading(false);
271277
}
@@ -287,6 +293,11 @@ export default function BinaryDetailPage({
287293
setCommitsData((prev) => ({ ...prev, [key]: commits }));
288294
} catch (err) {
289295
console.error('Failed to load commits:', err);
296+
toast({
297+
title: 'Error',
298+
description: 'Failed to load commits for environment',
299+
variant: 'destructive',
300+
});
290301
} finally {
291302
setLoadingCommits((prev) => ({ ...prev, [key]: false }));
292303
}

‎frontend/src/app/build-comparison/page.tsx‎

Lines changed: 52 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@ import { api } from '@/lib/api';
5454
import { ScrollArea } from '@/components/ui/scroll-area';
5555
import { Badge } from '@/components/ui/badge';
5656
import { Slider } from '@/components/ui/slider';
57+
import { useToast } from '@/hooks/use-toast';
5758

5859
const formatBytes = (bytes: number, decimals = 2) => {
5960
if (!bytes && bytes !== 0) return 'N/A';
@@ -115,6 +116,7 @@ export default function BuildComparisonPage() {
115116
);
116117

117118
const [mounted, setMounted] = useState(false);
119+
const { toast } = useToast();
118120
useEffect(() => setMounted(true), []);
119121

120122
// Debounce maxDataPoints changes
@@ -159,7 +161,13 @@ export default function BuildComparisonPage() {
159161
setSelectedBinaries([binariesData[0].id, binariesData[1].id]);
160162
}
161163
} catch (err) {
162-
setError(err instanceof Error ? err.message : 'Failed to load data');
164+
const errorMessage = err instanceof Error ? err.message : 'Failed to load data';
165+
setError(errorMessage);
166+
toast({
167+
title: 'Error',
168+
description: errorMessage,
169+
variant: 'destructive',
170+
});
163171
} finally {
164172
setLoading(false);
165173
}
@@ -209,6 +217,11 @@ export default function BuildComparisonPage() {
209217
}
210218
} catch (err) {
211219
console.error('Failed to load benchmark data:', err);
220+
toast({
221+
title: 'Error',
222+
description: 'Failed to load benchmark names',
223+
variant: 'destructive',
224+
});
212225
} finally {
213226
setDataProcessing(false);
214227
}
@@ -240,12 +253,22 @@ export default function BuildComparisonPage() {
240253

241254
setDataProcessing(true);
242255
try {
256+
const versionOption = pythonVersionOptions.find(
257+
(v) => v.label === selectedPythonVersionKey
258+
);
259+
if (!versionOption) {
260+
setDataProcessing(false);
261+
return;
262+
}
263+
243264
// Create batch request for ALL benchmarks and selected binaries (load everything upfront)
244265
const trendQueries = allBenchmarkNames.flatMap((benchmark) =>
245266
selectedBinaries.map((binaryId) => ({
246267
benchmark_name: benchmark,
247268
binary_id: binaryId,
248269
environment_id: selectedEnvironmentId,
270+
python_major: versionOption.major,
271+
python_minor: versionOption.minor,
249272
limit: debouncedMaxDataPoints,
250273
}))
251274
);
@@ -267,6 +290,11 @@ export default function BuildComparisonPage() {
267290
setTrendData(newTrendData); // Replace entirely since we're loading all data
268291
} catch (err) {
269292
console.error('Failed to load trend data:', err);
293+
toast({
294+
title: 'Error',
295+
description: 'Failed to load trend data',
296+
variant: 'destructive',
297+
});
270298
} finally {
271299
setDataProcessing(false);
272300
}
@@ -276,6 +304,8 @@ export default function BuildComparisonPage() {
276304
}, [
277305
selectedBinaries,
278306
selectedEnvironmentId,
307+
selectedPythonVersionKey,
308+
pythonVersionOptions,
279309
allBenchmarkNames,
280310
debouncedMaxDataPoints,
281311
]); // Remove selectedBenchmarks and benchmarkMode from dependencies
@@ -758,7 +788,11 @@ export default function BuildComparisonPage() {
758788
})
759789
.catch((error) => {
760790
console.error('Failed to export chart as PNG:', error);
761-
alert('PNG export failed. Please try again.');
791+
toast({
792+
title: 'Export Error',
793+
description: 'PNG export failed. Please try again.',
794+
variant: 'destructive',
795+
});
762796
});
763797
};
764798

@@ -1284,23 +1318,30 @@ export default function BuildComparisonPage() {
12841318
formatter={(value: number, name: string, props) => {
12851319
const binary = binaries.find((b) => b.id === name);
12861320
const displayName = binary?.name || name;
1321+
const fullVersion = props.payload.fullVersion
1322+
? `(py ${props.payload.fullVersion})`
1323+
: '';
12871324

12881325
if (viewMode === 'relative') {
1289-
return [`${value.toFixed(2)}%`, displayName];
1326+
return [`${value.toFixed(2)}% ${fullVersion}`, displayName];
12901327
} else {
1291-
return [formatBytes(value), displayName];
1328+
return [`${formatBytes(value)} ${fullVersion}`, displayName];
12921329
}
12931330
}}
12941331
labelFormatter={(label, payload) => {
12951332
const commitData = payload?.[0]?.payload;
1296-
if (commitData) {
1297-
const message = commitData.commitMessage || 'No message';
1298-
return `${commitData.commitSha}: ${message.substring(
1299-
0,
1300-
50
1301-
)}${message.length > 50 ? '...' : ''}`;
1333+
if (commitData && commitData.commitSha && commitData.fullVersion) {
1334+
const message = commitData.commitMessage;
1335+
if (message && message.trim()) {
1336+
const truncatedMessage = message.length > 50
1337+
? `${message.substring(0, 50)}...`
1338+
: message;
1339+
return `${commitData.commitSha} (py ${commitData.fullVersion}): ${truncatedMessage}`;
1340+
} else {
1341+
return `${commitData.commitSha} (py ${commitData.fullVersion})`;
1342+
}
13021343
}
1303-
return label;
1344+
return label || 'No data';
13041345
}}
13051346
contentStyle={{
13061347
backgroundColor: 'hsl(var(--background))',

‎frontend/src/app/diff/page.tsx‎

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@ import type {
5454
import { METRIC_OPTIONS } from '@/lib/types';
5555
import { api } from '@/lib/api';
5656
import { useSearchParams } from 'next/navigation';
57+
import { useToast } from '@/hooks/use-toast';
5758

5859
interface EnhancedDiffTableRow {
5960
benchmark_name: string;
@@ -98,6 +99,7 @@ function DiffTableContent() {
9899
const [loading, setLoading] = useState(true);
99100
const [dataProcessing, setDataProcessing] = useState(false);
100101
const [error, setError] = useState<string | null>(null);
102+
const { toast } = useToast();
101103
const [selectedPythonVersion, setSelectedPythonVersion] = useState<
102104
string | undefined
103105
>();
@@ -194,7 +196,13 @@ function DiffTableContent() {
194196
setSelectedEnvironmentId(environmentsData[0].id);
195197
}
196198
} catch (err) {
197-
setError(err instanceof Error ? err.message : 'Failed to load data');
199+
const errorMessage = err instanceof Error ? err.message : 'Failed to load data';
200+
setError(errorMessage);
201+
toast({
202+
title: 'Error',
203+
description: errorMessage,
204+
variant: 'destructive',
205+
});
198206
} finally {
199207
setLoading(false);
200208
}
@@ -237,6 +245,11 @@ function DiffTableContent() {
237245
} catch (err) {
238246
console.error('Failed to load environments for binary:', err);
239247
setAvailableEnvironments([]);
248+
toast({
249+
title: 'Error',
250+
description: 'Failed to load environments for selected binary',
251+
variant: 'destructive',
252+
});
240253
}
241254
}
242255

@@ -286,6 +299,11 @@ function DiffTableContent() {
286299
'Failed to load commits for binary and environment:',
287300
err
288301
);
302+
toast({
303+
title: 'Error',
304+
description: 'Failed to load commits for binary and environment',
305+
variant: 'destructive',
306+
});
289307
setAvailableCommits([]);
290308
}
291309
}
@@ -368,6 +386,11 @@ function DiffTableContent() {
368386
} catch (err) {
369387
console.error('Error loading diff data:', err);
370388
setDiffData([]);
389+
toast({
390+
title: 'Error',
391+
description: 'Failed to load diff data',
392+
variant: 'destructive',
393+
});
371394
} finally {
372395
setDataProcessing(false);
373396
}

‎frontend/src/app/flamegraph/[id]/page.tsx‎

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import { Card, CardContent, CardHeader, CardTitle } from '@/components/ui/card';
66
import { Button } from '@/components/ui/button';
77
import { ArrowLeft, Loader2 } from 'lucide-react';
88
import { api } from '@/lib/api';
9+
import { useToast } from '@/hooks/use-toast';
910

1011
interface FlamegraphPageProps {
1112
params: Promise<{ id: string }>;
@@ -17,6 +18,7 @@ export default function FlamegraphPage({ params }: FlamegraphPageProps) {
1718
const [flamegraphHtml, setFlamegraphHtml] = useState<string>('');
1819
const [loading, setLoading] = useState(true);
1920
const [error, setError] = useState<string>('');
21+
const { toast } = useToast();
2022

2123
useEffect(() => {
2224
const fetchFlamegraph = async () => {
@@ -26,9 +28,13 @@ export default function FlamegraphPage({ params }: FlamegraphPageProps) {
2628
setFlamegraphHtml(data.flamegraph_html || '');
2729
} catch (err) {
2830
console.error('Error fetching flamegraph:', err);
29-
setError(
30-
err instanceof Error ? err.message : 'Failed to load flamegraph'
31-
);
31+
const errorMessage = err instanceof Error ? err.message : 'Failed to load flamegraph';
32+
setError(errorMessage);
33+
toast({
34+
title: 'Error',
35+
description: errorMessage,
36+
variant: 'destructive',
37+
});
3238
} finally {
3339
setLoading(false);
3440
}

0 commit comments

Comments
 (0)