Skip to content

Commit 6deb74c

Browse files
Copilotankurrera
andcommitted
Address code review feedback: improve error handling and performance
Co-authored-by: ankurrera <186232326+ankurrera@users.noreply.github.com>
1 parent 29dae24 commit 6deb74c

3 files changed

Lines changed: 21 additions & 6 deletions

File tree

src/components/system/RadarChart.tsx

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { useEffect, useRef, useState, useCallback } from "react";
1+
import { useEffect, useRef, useState, useCallback, useMemo } from "react";
22
import { useCoreMetrics } from "@/hooks/useCoreMetrics";
33
import { MAX_METRIC_XP, CoreMetricName } from "@/lib/coreMetrics";
44
import { Dialog, DialogContent, DialogHeader, DialogTitle } from "@/components/ui/dialog";
@@ -103,6 +103,16 @@ const RadarChart = () => {
103103
// radarData is computed from Skills and Characteristics XP
104104
// It automatically updates when skill XP changes, attendance is marked, or time is edited
105105
const data = radarData;
106+
107+
// Memoize total contributing skills count to avoid recalculating on every render
108+
const totalContributingSkills = useMemo(() => {
109+
return coreMetrics.reduce((sum, m) => sum + m.contributions.length, 0);
110+
}, [coreMetrics]);
111+
112+
// Memoize non-zero metrics count
113+
const nonZeroMetricsCount = useMemo(() => {
114+
return coreMetrics.filter(m => m.xp > 0).length;
115+
}, [coreMetrics]);
106116

107117
// Handle canvas click to detect which axis was clicked
108118
const handleCanvasClick = useCallback((event: React.MouseEvent<HTMLCanvasElement>) => {
@@ -311,8 +321,8 @@ const RadarChart = () => {
311321
<div className="space-y-1 text-blue-700">
312322
<div>Radar Points: {data.length}</div>
313323
<div>Core Metrics: {coreMetrics.length}</div>
314-
<div>Total Contributing Skills: {coreMetrics.reduce((sum, m) => sum + m.contributions.length, 0)}</div>
315-
<div>Non-Zero Metrics: {coreMetrics.filter(m => m.xp > 0).length}</div>
324+
<div>Total Contributing Skills: {totalContributingSkills}</div>
325+
<div>Non-Zero Metrics: {nonZeroMetricsCount}</div>
316326
<div className="text-blue-500 text-[10px] mt-2">
317327
Click metrics to see contributors
318328
</div>

src/hooks/useCoreMetrics.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -182,10 +182,15 @@ export function useCoreMetrics(): UseCoreMetricsResult {
182182

183183
// Assert that radar data length matches core metrics length
184184
if (data.length !== coreMetrics.length) {
185-
console.error('[CRITICAL] Radar data length mismatch!', {
185+
const error = `[CRITICAL] Radar data length mismatch! Expected ${coreMetrics.length}, got ${data.length}`;
186+
console.error(error, {
186187
expected: coreMetrics.length,
187188
actual: data.length,
188189
});
190+
// In development, throw error to catch issues immediately
191+
if (process.env.NODE_ENV === 'development') {
192+
throw new Error(error);
193+
}
189194
}
190195
}
191196

src/test/radarIntegration.test.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -261,9 +261,9 @@ describe("Radar Integration - Full CRUD Flow", () => {
261261
// Raw metric should have full XP
262262
expect(metrics.find(m => m.name === 'Programming')?.xp).toBe(5000);
263263

264-
// Radar should clamp to MAX_METRIC_XP
264+
// Radar should clamp to MAX_METRIC_XP (2000)
265265
expect(radarData.find(d => d.label === 'Programming')?.value).toBe(MAX_METRIC_XP);
266-
expect(radarData.find(d => d.label === 'Programming')?.value).toBe(2000);
266+
expect(MAX_METRIC_XP).toBe(2000); // Verify constant value
267267
});
268268

269269
it("CRITICAL: verifies skill with 0 XP contributes 0 to all metrics", () => {

0 commit comments

Comments
 (0)