457 lines
12 KiB
Markdown
457 lines
12 KiB
Markdown
# RFCP - Iteration 10.1: Critical Bugfixes
|
||
|
||
**Status:** Ready for Claude Code Implementation
|
||
**Priority:** P0 CRITICAL
|
||
**Time:** 30-60 minutes
|
||
|
||
---
|
||
|
||
## 🐛 Three Critical Issues Found
|
||
|
||
After Iteration 10 deployment, three critical bugs remain:
|
||
|
||
1. ❌ **Stack overflow crash at 50m resolution** - **ROOT CAUSE FOUND: Spread operator on large arrays**
|
||
2. ❌ **No confirmation on Delete key** - keyboard shortcut bypasses dialog
|
||
3. ❌ **Cyan/green coverage zone** - синьо-зелене коло на карті (visible on screenshot)
|
||
|
||
---
|
||
|
||
## 🔍 Bug Analysis
|
||
|
||
### Bug 1: Stack Overflow at 50m Resolution ⚡ ROOT CAUSE IDENTIFIED
|
||
|
||
**User Report:** "крашує на білий екран саме при розрахунку, натискання на кнопку"
|
||
|
||
**Stack Trace Analysis:**
|
||
```javascript
|
||
const M = b.map(ht => ht.rsrp) // Maps to RSRP array
|
||
const B = Math.min(...M) // ❌ CRASHES HERE!
|
||
const A = Math.max(...M) // ❌ AND HERE!
|
||
const C = M.reduce((ht, Nt) => ht + Nt, 0) / z
|
||
```
|
||
|
||
**ROOT CAUSE: Spread Operator Argument Limit**
|
||
|
||
JavaScript has a hard limit of ~65,000-125,000 function arguments (varies by engine):
|
||
- 50m resolution, 10km radius = ~158,000 points
|
||
- `Math.min(...array)` tries to pass 158,000 arguments
|
||
- **Exceeds engine limit → RangeError: Maximum call stack size exceeded**
|
||
|
||
**Why Previous Fix Didn't Work:**
|
||
|
||
Iteration 10 added `MAX_GRID_POINTS` cap and replaced `results.flat()`, but the **spread operator in Math.min/max was still there**!
|
||
|
||
```typescript
|
||
// ❌ This is still in the code:
|
||
const minRsrp = Math.min(...rsrpValues) // CRASHES with 150k+ items
|
||
const maxRsrp = Math.max(...rsrpValues) // CRASHES with 150k+ items
|
||
```
|
||
|
||
**Solution: Replace Spread with Reduce**
|
||
|
||
```typescript
|
||
// ❌ BAD (crashes on large arrays)
|
||
const minRsrp = Math.min(...rsrpValues)
|
||
const maxRsrp = Math.max(...rsrpValues)
|
||
|
||
// ✅ GOOD (works with ANY size array)
|
||
const minRsrp = rsrpValues.reduce((min, val) => Math.min(min, val), rsrpValues[0])
|
||
const maxRsrp = rsrpValues.reduce((max, val) => Math.max(max, val), rsrpValues[0])
|
||
|
||
// ✅ EVEN BETTER (slightly faster for large arrays)
|
||
const minRsrp = rsrpValues.reduce((min, val) => val < min ? val : min, rsrpValues[0])
|
||
const maxRsrp = rsrpValues.reduce((max, val) => val > max ? val : max, rsrpValues[0])
|
||
```
|
||
|
||
**Files to Fix:**
|
||
|
||
Search for ALL instances of:
|
||
```bash
|
||
grep -rn "Math.min\(\.\.\..*\)" src/
|
||
grep -rn "Math.max\(\.\.\..*\)" src/
|
||
```
|
||
|
||
Most likely locations:
|
||
- `src/store/coverage.ts` - coverage statistics calculation
|
||
- `src/lib/calculator.ts` - coverage calculations
|
||
- `src/components/panels/CoverageStats.tsx` - stats display
|
||
|
||
**Performance Comparison:**
|
||
- `Math.min(...array)`: ✅ Fast BUT ❌ Crashes >65k elements
|
||
- `reduce()`: ✅ Always works, ✅ Only ~2x slower
|
||
- For 200k elements: ~30ms (still imperceptible to user)
|
||
|
||
---
|
||
|
||
### Bug 2: No Confirmation on Delete Key
|
||
|
||
**User Report:** "нема підтвердження видалення сайту коли видаляєш кнопкою del"
|
||
|
||
**Current Behavior:**
|
||
- Delete button in UI → Shows confirmation dialog ✅
|
||
- Keyboard shortcut (Delete key) → Deletes immediately ❌
|
||
|
||
**Root Cause:**
|
||
|
||
Keyboard handler calls `deleteSite()` directly, bypassing the confirmation wrapper.
|
||
|
||
**Likely code pattern:**
|
||
```typescript
|
||
// Somewhere in App.tsx or SitesPanel.tsx
|
||
useEffect(() => {
|
||
const handleKeyDown = (e: KeyboardEvent) => {
|
||
if (e.key === 'Delete' && selectedSiteId) {
|
||
deleteSite(selectedSiteId); // ❌ No confirmation!
|
||
}
|
||
};
|
||
|
||
window.addEventListener('keydown', handleKeyDown);
|
||
return () => window.removeEventListener('keydown', handleKeyDown);
|
||
}, [selectedSiteId, deleteSite]);
|
||
```
|
||
|
||
**Solution:**
|
||
|
||
Find the existing delete button's onClick handler and reuse it:
|
||
|
||
```typescript
|
||
// Example: In SitesPanel.tsx or similar
|
||
|
||
// Existing delete button logic (already has confirmation)
|
||
const handleDeleteClick = useCallback(() => {
|
||
if (!selectedSite) return;
|
||
|
||
if (!window.confirm(`Delete site "${selectedSite.name}"?`)) {
|
||
return;
|
||
}
|
||
|
||
const deletedSite = { ...selectedSite };
|
||
deleteSite(selectedSiteId);
|
||
|
||
// Show undo toast
|
||
toast.success('Site deleted', {
|
||
duration: 10000,
|
||
action: {
|
||
label: 'Undo',
|
||
onClick: () => addSite(deletedSite),
|
||
},
|
||
});
|
||
}, [selectedSite, selectedSiteId, deleteSite, addSite]);
|
||
|
||
// Update keyboard handler to use SAME function
|
||
useEffect(() => {
|
||
const handleKeyDown = (e: KeyboardEvent) => {
|
||
if (e.key === 'Delete' && selectedSiteId) {
|
||
e.preventDefault();
|
||
handleDeleteClick(); // ✅ Reuses confirmation logic!
|
||
}
|
||
};
|
||
|
||
window.addEventListener('keydown', handleKeyDown);
|
||
return () => window.removeEventListener('keydown', handleKeyDown);
|
||
}, [selectedSiteId, handleDeleteClick]);
|
||
```
|
||
|
||
**Search for:**
|
||
```bash
|
||
grep -rn "key.*Delete\|Delete.*key" src/
|
||
grep -rn "handleKeyDown\|onKeyDown" src/
|
||
```
|
||
|
||
**Files to check:**
|
||
- `src/App.tsx` - main keyboard handlers
|
||
- `src/components/panels/SitesPanel.tsx` - site list with delete button
|
||
- `src/components/map/Map.tsx` - map-level keyboard handlers
|
||
|
||
---
|
||
|
||
### Bug 3: Cyan/Green Coverage Zone
|
||
|
||
**User Report:** "колір зони синьо - зелений :)" (visible on screenshot)
|
||
|
||
**Visual Evidence:**
|
||
- Синьо-зелене (cyan) коло видно навколо сайту
|
||
- Це НЕ частина RSRP gradient (який orange → red → dark red)
|
||
- Виглядає як dashed circle або зона
|
||
|
||
**Most Likely Source: Leaflet Circle Component**
|
||
|
||
```typescript
|
||
// Probably in src/components/map/SiteMarker.tsx or Map.tsx
|
||
<Circle
|
||
center={[site.lat, site.lon]}
|
||
radius={site.radius * 1000}
|
||
pathOptions={{
|
||
color: '#00bcd4', // ❌ Cyan color - this is it!
|
||
weight: 2,
|
||
dashArray: '10, 5',
|
||
fillOpacity: 0
|
||
}}
|
||
/>
|
||
```
|
||
|
||
**Investigation Steps:**
|
||
|
||
```bash
|
||
# 1. Find all Circle components
|
||
grep -rn "<Circle" src/components/map/
|
||
|
||
# 2. Check for cyan/blue colors
|
||
grep -rn "#00bcd4\|#00ff00\|cyan\|#0000ff" src/
|
||
|
||
# 3. Check pathOptions
|
||
grep -rn "pathOptions" src/components/map/
|
||
```
|
||
|
||
**Solution Options:**
|
||
|
||
**Option A: Remove the circle completely** (recommended if not needed)
|
||
```typescript
|
||
// Just delete or comment out:
|
||
{/* <Circle ... /> */}
|
||
```
|
||
|
||
**Option B: Change color to orange** (if circle is useful)
|
||
```typescript
|
||
<Circle
|
||
center={[site.lat, site.lon]}
|
||
radius={site.radius * 1000}
|
||
pathOptions={{
|
||
color: '#ff9800', // ✅ Orange (not in RSRP gradient)
|
||
weight: 2,
|
||
opacity: 0.6,
|
||
dashArray: '15, 10', // Longer dashes
|
||
fillOpacity: 0
|
||
}}
|
||
/>
|
||
```
|
||
|
||
**Option C: Make it toggleable**
|
||
```typescript
|
||
// Add to coverage settings store
|
||
const [showCalculationBounds, setShowCalculationBounds] = useState(false);
|
||
|
||
// In settings panel:
|
||
<label>
|
||
<input
|
||
type="checkbox"
|
||
checked={showCalculationBounds}
|
||
onChange={(e) => setShowCalculationBounds(e.target.checked)}
|
||
/>
|
||
Show Calculation Bounds
|
||
</label>
|
||
|
||
// In map:
|
||
{showCalculationBounds && (
|
||
<Circle
|
||
center={[site.lat, site.lon]}
|
||
radius={site.radius * 1000}
|
||
pathOptions={{ color: '#ff9800', ... }}
|
||
/>
|
||
)}
|
||
```
|
||
|
||
**Recommended:** Start with Option A (remove), can add back later if needed.
|
||
|
||
---
|
||
|
||
## 📋 Implementation Checklist for Claude Code
|
||
|
||
### Phase 1: Fix Spread Operator Crash (P0 - HIGHEST PRIORITY)
|
||
|
||
**Task 1.1: Find all Math.min/max with spread operators**
|
||
```bash
|
||
cd /opt/rfcp/frontend
|
||
grep -rn "Math.min(\.\.\." src/
|
||
grep -rn "Math.max(\.\.\." src/
|
||
```
|
||
|
||
**Task 1.2: Replace ALL instances**
|
||
|
||
Find patterns like:
|
||
```typescript
|
||
Math.min(...array)
|
||
Math.max(...array)
|
||
```
|
||
|
||
Replace with:
|
||
```typescript
|
||
array.reduce((min, val) => val < min ? val : min, array[0])
|
||
array.reduce((max, val) => val > max ? val : max, array[0])
|
||
```
|
||
|
||
**Task 1.3: Add safety checks**
|
||
```typescript
|
||
// Before using reduce, ensure array is not empty:
|
||
if (array.length === 0) {
|
||
return { minRsrp: 0, maxRsrp: 0, avgRsrp: 0 };
|
||
}
|
||
```
|
||
|
||
**Expected files to modify:**
|
||
- `src/store/coverage.ts` - most likely location
|
||
- `src/lib/calculator.ts` - possible location
|
||
- `src/components/panels/CoverageStats.tsx` - stats display
|
||
|
||
---
|
||
|
||
### Phase 2: Fix Delete Key Confirmation (P0)
|
||
|
||
**Task 2.1: Find keyboard event handler**
|
||
```bash
|
||
grep -rn "key.*Delete\|Delete.*key" src/
|
||
grep -rn "e.key === 'Delete'" src/
|
||
```
|
||
|
||
**Task 2.2: Find existing delete button confirmation**
|
||
```bash
|
||
grep -rn "window.confirm.*[Dd]elete" src/
|
||
grep -rn "handleDelete" src/
|
||
```
|
||
|
||
**Task 2.3: Make keyboard handler use same confirmation logic**
|
||
|
||
Pattern to find:
|
||
```typescript
|
||
if (e.key === 'Delete' && selectedSiteId) {
|
||
deleteSite(selectedSiteId); // ❌ No confirmation
|
||
}
|
||
```
|
||
|
||
Replace with:
|
||
```typescript
|
||
if (e.key === 'Delete' && selectedSiteId) {
|
||
e.preventDefault();
|
||
handleDeleteClick(); // ✅ Uses existing confirmation
|
||
}
|
||
```
|
||
|
||
---
|
||
|
||
### Phase 3: Fix Cyan/Green Circle (P1)
|
||
|
||
**Task 3.1: Find Circle component with cyan color**
|
||
```bash
|
||
grep -rn "<Circle" src/components/map/
|
||
grep -rn "#00bcd4\|cyan" src/
|
||
```
|
||
|
||
**Task 3.2: Decide on solution**
|
||
- If circle not needed: delete it
|
||
- If useful: change color to `#ff9800` (orange)
|
||
- If optional: add toggle in settings
|
||
|
||
**Task 3.3: Implement chosen solution**
|
||
|
||
---
|
||
|
||
### Phase 4: Testing (P0)
|
||
|
||
**Test 1: Stack Overflow Fix**
|
||
- [ ] Create site with 10km radius
|
||
- [ ] Set resolution to 50m
|
||
- [ ] Click "Calculate Coverage"
|
||
- [ ] **Expected:** No crash, calculation completes
|
||
- [ ] **Verify:** Console has no errors
|
||
|
||
**Test 2: Delete Confirmation**
|
||
- [ ] Create a site
|
||
- [ ] Select it (click marker)
|
||
- [ ] Press Delete key
|
||
- [ ] **Expected:** Confirmation dialog appears
|
||
- [ ] Click OK → Site deleted + undo toast shown
|
||
- [ ] Create another site, select, press Delete
|
||
- [ ] Click Cancel → Nothing happens
|
||
|
||
**Test 3: Circle Color**
|
||
- [ ] Create a site
|
||
- [ ] Calculate coverage
|
||
- [ ] Zoom in/out
|
||
- [ ] **Expected:** No cyan/green circles visible
|
||
- [ ] **Verify:** Only RSRP gradient (orange→red→dark red)
|
||
|
||
---
|
||
|
||
### Phase 5: Deployment
|
||
|
||
```bash
|
||
# After all fixes:
|
||
npm run build
|
||
npm run type-check # Should pass
|
||
npm run preview # Test locally
|
||
|
||
# Then deploy to VPS
|
||
# (copy dist/ to /opt/rfcp/frontend/dist/)
|
||
sudo systemctl reload caddy
|
||
```
|
||
|
||
---
|
||
|
||
## 🤖 Instructions for Claude Code
|
||
|
||
**Context:** This is a React + TypeScript + Vite frontend project for RF coverage planning.
|
||
|
||
**Project Location:** `/opt/rfcp/frontend/` on VPS (10.10.10.1)
|
||
|
||
**Critical Fixes Needed:**
|
||
|
||
1. **Spread operator crash** - Replace `Math.min(...array)` with `reduce()` pattern
|
||
2. **Delete key bypass** - Add confirmation to keyboard handler
|
||
3. **Cyan circle** - Find and remove/recolor Circle component
|
||
|
||
**Priority:** Fix #1 first (blocks all usage), then #2, then #3.
|
||
|
||
**Search Strategy:**
|
||
- Use `grep -rn` to find problematic patterns
|
||
- Focus on `src/store/coverage.ts` first for Bug #1
|
||
- Look in `src/App.tsx` or `src/components/panels/SitesPanel.tsx` for Bug #2
|
||
- Check `src/components/map/` for Bug #3
|
||
|
||
**Testing:**
|
||
- Must test 50m resolution after Bug #1 fix
|
||
- Must test Delete key after Bug #2 fix
|
||
- Verify no cyan circles after Bug #3 fix
|
||
|
||
**Build Commands:**
|
||
```bash
|
||
npm run build # Production build
|
||
npm run type-check # Verify TypeScript
|
||
npm run preview # Test locally
|
||
```
|
||
|
||
**Success Criteria:**
|
||
- ✅ No crash at 50m resolution
|
||
- ✅ Delete key shows confirmation
|
||
- ✅ No cyan/green circles on map
|
||
- ✅ TypeScript passes
|
||
- ✅ No console errors
|
||
|
||
---
|
||
|
||
## 📝 Additional Context
|
||
|
||
**RSRP Gradient Colors (for reference):**
|
||
- Excellent (>-80 dBm): `#ff6b35` (orange)
|
||
- Good (-80 to -95): `#ff4444` (red)
|
||
- Fair (-95 to -105): `#cc0000` (dark red)
|
||
- Poor (<-105): `#8b0000` (very dark red)
|
||
|
||
**Acceptable Colors for Circles (not in gradient):**
|
||
- Orange: `#ff9800`
|
||
- Purple: `#9c27b0`
|
||
- Black: `#000000`
|
||
|
||
**NOT acceptable (conflicts with gradient):**
|
||
- Red shades
|
||
- Orange shades
|
||
- Cyan/blue: `#00bcd4` ❌ (this is the bug!)
|
||
|
||
---
|
||
|
||
## 🎯 Success = All Three Bugs Fixed + Tested
|
||
|
||
**Estimated time:** 30-60 minutes
|
||
|
||
**Deploy when:** All tests pass + no TypeScript errors
|
||
|