feat: enhance performance and UI/UX #8
Conversation
- 【Performance】 - use web workers for sky projection calculations - implement LOD (level of detail) for sky map rendering - optimize data fetching with caching and timeouts - reduce bundle size with manual chunking - add performance monitor component - 【UI/UX】 - improve responsive design for filters drawer and map/table panels - add loading state for initial data loading - enhance sky map with zoom reset button - improve table with pagination and row selection - 【Refactor】 - refactor sky projection logic - memoize components and callbacks - use requestAnimationFrame for map drawing - 【Fix】 - fix table page size exceeding limit
|
Caution Review failedAn error occurred during the review process. Please try again later. ✨ Finishing Touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (6)
dashboard/src/api.ts (4)
89-91: Do not hardcode external endpointHardcoding an ngrok host will break prod and leaks internal infra. Restore env-driven configuration with safe default.
-const RAW_ENDPOINT: string | undefined = "nonarithmetically-undeliberating-janelle.ngrok-free.app"; +const RAW_ENDPOINT: string | undefined = + (import.meta as { env?: Record<string, string> }).env?.VITE_MINIO_ENDPOINT;
107-113: Return type never nullFunction always returns a URL; narrow the type to Promise.
-export const getCutoutObject = async (objectKey: string): Promise<string | null> => { +export const getCutoutObject = async (objectKey: string): Promise<string> => {
118-121: Remove forbidden header 'User-Agent'Browsers forbid setting User-Agent; this throws a TypeError and breaks the fetch path.
- headers: { - 'ngrok-skip-browser-warning': 'true', - 'User-Agent': 'LaStBeRu-Explorer/1.0 (Custom)' - }, + headers: { 'ngrok-skip-browser-warning': 'true' },
129-133: Avoid leaking blob URLsURL.createObjectURL needs a revoke. Current API can't safely revoke upstream.
Two options:
- Return { src, revoke } and let the component call revoke on unmount.
- Or skip blob conversion and return the direct URL (header/query bypass in place).
If you want, I can draft a minimal adapter that tracks and auto-revokes on image unload.dashboard/src/components/CutoutGrid.tsx (2)
30-35: Fix React Query typing and error checks (prevents TS compile errors).
- data is inferred as unknown; img src expects string.
- error is unknown; using it in boolean conditions breaks under strict TS. Prefer isError.
Apply:
- const { data, isLoading, error } = useQuery({ + const { data, isLoading, error, isError } = useQuery<string>({ queryKey: ['cutout', record.file_path], queryFn: () => getCutoutObject(record.file_path), staleTime: 5 * 60 * 1000, // 5 minutos de cache retry: 1, // Only retry once for ngrok issues });- {!isLoading && !data && !error && <Typography variant="caption" color="text.secondary" sx={{ textAlign:'center', px:0.5, position:'absolute', inset:0, display:'flex', alignItems:'center', justifyContent:'center' }}>No image</Typography>} - {error && <Typography variant="caption" color="error" sx={{ textAlign:'center', px:0.5, position:'absolute', inset:0, display:'flex', alignItems:'center', justifyContent:'center' }}>Error</Typography>} + {!isLoading && !data && !isError && <Typography variant="caption" color="text.secondary" sx={{ textAlign:'center', px:0.5, position:'absolute', inset:0, display:'flex', alignItems:'center', justifyContent:'center' }}>No image</Typography>} + {isError && <Typography variant="caption" color="error" sx={{ textAlign:'center', px:0.5, position:'absolute', inset:0, display:'flex', alignItems:'center', justifyContent:'center' }}>Error</Typography>}Also applies to: 71-72
49-70: Make onError fallback robust (avoid dead-end after ngrok attempt and guard URL parsing).Current flow sets dataset.fallbackTried before attempts; for ngrok sources the extension fallback never runs if the first retry also fails.
- onError={(e)=>{ + onError={(e)=>{ const img = e.currentTarget; - if(img.dataset.fallbackTried) return; - img.dataset.fallbackTried = '1'; - - // For ngrok URLs, try adding the bypass parameters as query string - if (img.src.includes('ngrok')) { - const url = new URL(img.src); - url.searchParams.set('ngrok-skip-browser-warning', 'true'); - img.src = url.toString(); - return; - } - - // Extension fallback: try .jpeg -> .png -> .jpg - const order = ['.jpeg','.png','.jpg']; - const current = order.find(ext => img.src.toLowerCase().includes(ext)); - const nextExt = current ? order[(order.indexOf(current)+1)%order.length] : null; - if(nextExt){ - const newSrc = img.src.replace(/\.(jpeg|png|jpg)(?=($|\?))/i, nextExt); - if(newSrc !== img.src) img.src = newSrc; - } - }} />} + const step = Number(img.dataset.fallbackStep ?? '0'); + // Step 0: ngrok param (dev only) + if (step === 0 && import.meta.env.DEV && img.src.includes('ngrok')) { + try { + const url = new URL(img.src); + url.searchParams.set('ngrok-skip-browser-warning', 'true'); + img.dataset.fallbackStep = '1'; + img.src = url.toString(); + return; + } catch { /* ignore */ } + } + // Step 1+: rotate extension: .jpeg -> .png -> .jpg + const order = ['.jpeg', '.png', '.jpg'] as const; + const match = img.src.toLowerCase().match(/\.(jpeg|png|jpg)(?=($|\?))/i); + const current = match?.[0]?.toLowerCase() ?? '.jpeg'; + const idx = Math.max(0, order.indexOf(current as any)); + const nextExt = order[(idx + 1) % order.length]; + const newSrc = img.src.replace(/\.(jpeg|png|jpg)(?=($|\?))/i, nextExt); + if (newSrc !== img.src) { + img.dataset.fallbackStep = String(step + 1); + img.src = newSrc; + return; + } + // Give up + img.dataset.fallbackStep = 'done'; + }} />}
🧹 Nitpick comments (28)
dashboard/package.json (1)
37-37: Pin Terser to patch and declare Node engine
- Change Terser from caret to tilde to avoid unexpected regressions in CI.
- Add a Node engine field to align with Vite/MUI requirements.
--- dashboard/package.json @@ devDependencies - "terser": "^5.44.0", + "terser": "~5.44.0",--- dashboard/package.json @@ "dependencies": { … }, "devDependencies": { … }, + "engines": { + "node": ">=18.18" + } }dashboard/vite.config.ts (4)
12-12: Target es2020 is fine; align dev pre-bundle target to avoid behavior drift.Add optimizeDeps.esbuildOptions.target = 'es2020' so dev and prod parse the same syntax.
Apply:
optimizeDeps: { - include: [ + include: [ 'react', 'react-dom', '@mui/material', '@mui/icons-material', '@mui/x-data-grid', '@tanstack/react-query' - ] + ], + esbuildOptions: { target: 'es2020' } },
14-15: Prod source maps disabled—OK. Consider hidden maps if you need prod stack traces.If you use Sentry or similar, switch to hidden/inlined maps and upload in CI; otherwise keep as-is.
16-22: Drop only noisy logs; avoid removing warn/error.You currently both drop_console and mark specific console funcs as pure. If the intent is to keep warn/error, remove drop_console and keep pure_funcs. Also consider a small extra pass for slightly better compression.
Apply:
terserOptions: { compress: { - drop_console: true, // Remove console.log in production drop_debugger: true, - pure_funcs: ['console.log', 'console.info', 'console.debug'] + // Keep warn/error; strip only log/info/debug + pure_funcs: ['console.log', 'console.info', 'console.debug'], + passes: 2 } },
36-45: Pre-bundle list: add recoil and keep deps in sync with manual chunks.Pre-bundling recoil can speed dev startup when state is touched early.
Apply:
optimizeDeps: { include: [ 'react', 'react-dom', '@mui/material', '@mui/icons-material', '@mui/x-data-grid', - '@tanstack/react-query' + '@tanstack/react-query', + 'recoil' ], esbuildOptions: { target: 'es2020' } },dashboard/src/components/FiltersDrawer.tsx (2)
78-92: Prefer plain sx object over function hereYou're not using theme in this block; avoid recreating a function each render for tiny perf/readability win.
- PaperProps={{ - sx: ({ breakpoints }) => ({ + PaperProps={{ + sx: { width: { xs: '100%', sm: 320, md: 360 }, maxWidth: '100%', background: 'linear-gradient(180deg,#0d1820,#0a141a)', display:'flex', flexDirection:'column', borderRight: '1px solid rgba(255,255,255,0.08)' - }) + } }}
114-114: Make the references list height responsive to viewportHard-capping at 500px can overflow on short viewports. Use vh for XS and step up by breakpoint.
- <Box sx={{ maxHeight:500, overflowY:'auto', pr:0.5, mb:2, border:'1px solid rgba(255,255,255,0.08)', borderRadius:1, p:1, background:'rgba(255,255,255,0.04)' }}> + <Box sx={{ maxHeight:{ xs:'40vh', sm:360, md:500 }, overflowY:'auto', pr:0.5, mb:2, border:'1px solid rgba(255,255,255,0.08)', borderRadius:1, p:1, background:'rgba(255,255,255,0.04)' }}>dashboard/src/hooks/useDebounce.ts (1)
26-34: Remove redundant unmount cleanup effectThe effect on Lines 7–24 already cleans up on unmount; this second effect is duplicate.
- // Cleanup on unmount - useEffect(() => { - return () => { - if (timeoutRef.current) { - clearTimeout(timeoutRef.current); - } - }; - }, []);dashboard/src/api.ts (5)
17-19: Tighten cache typingPrefer Promise over any to avoid leaking any through the API surface.
-const requestCache = new Map<string, Promise<any>>(); +const requestCache = new Map<string, Promise<unknown>>();
33-41: Revisit fetch cache mode and request headers
- cache: 'force-cache' can serve stale JSON; consider 'no-cache' for revalidation or make it configurable.
- 'Cache-Control' on the request is typically ignored by the browser cache; set on server responses instead.
- const res = await fetch(url, { - cache: 'force-cache', // Use browser cache when available + const res = await fetch(url, { + cache: 'no-cache', // allow revalidation; make configurable if needed signal: controller.signal, - headers: { - 'Accept': 'application/json', - 'Cache-Control': 'public, max-age=3600' // Cache for 1 hour - } + headers: { 'Accept': 'application/json' } });Optionally expose a per-call option to override cache mode when calling fetchJson.
68-71: Make cache eviction TTL a constantHardcoded 5s TTL is opaque. Hoist to a named constant/env for tuning and docs.
- fetchPromise.finally(() => { - setTimeout(() => requestCache.delete(url), 5000); // Keep cache for 5s - }); + const REQUEST_CACHE_TTL_MS = 5000; + fetchPromise.finally(() => { + setTimeout(() => requestCache.delete(url), REQUEST_CACHE_TTL_MS); + });
101-103: Optionally append ngrok skip param at sourceFor
fallbacks you can't send headers; adding the query param helps avoid the warning when using ngrok.
- const url = `${base}/slcomp/${path}`; + let url = `${base}/slcomp/${path}`; + if (RAW_ENDPOINT?.includes('ngrok')) { + const u = new URL(url); + u.searchParams.set('ngrok-skip-browser-warning', 'true'); + url = u.toString(); + }
157-159: Gate noisy debug logsReduce console noise in prod; guard behind env flag.
- console.log('[DEBUG] Using direct URL:', url); + if ((import.meta as { env?: Record<string, string> }).env?.VITE_DEBUG_CUTOUTS) { + // eslint-disable-next-line no-console + console.debug('[cutouts] direct URL:', url); + }dashboard/src/components/CutoutGrid.tsx (2)
43-49: Add alt text and tighten privacy defaults.Improve a11y and reduce referrer leakage.
- {!isLoading && data && <img + {!isLoading && data && <img src={data} loading="lazy" decoding="async" crossOrigin="anonymous" - style={{ position:'absolute', inset:0, width:'100%', height:'100%', objectFit:'contain', imageRendering:'auto' }} + referrerPolicy="no-referrer" + alt={`${record.band ?? 'cutout'} preview`} + style={{ position:'absolute', inset:0, width:'100%', height:'100%', objectFit:'contain', imageRendering:'auto' }}
30-35: Guard query execution when file_path is empty.Cheap safety net.
const { data, isLoading, error, isError } = useQuery<string>({ queryKey: ['cutout', record.file_path], queryFn: () => getCutoutObject(record.file_path), staleTime: 5 * 60 * 1000, // 5 minutos de cache retry: 1, // Only retry once for ngrok issues + enabled: !!record?.file_path });dashboard/src/components/PerformanceMonitor.tsx (3)
33-45: Add keyboard/a11y affordances to the toggle.Make the panel operable without a mouse.
- <Box + <Box sx={{ background: 'rgba(0,0,0,0.8)', color: 'white', p: 1, borderRadius: 1, fontSize: '0.75rem', cursor: 'pointer', minWidth: 120 }} - onClick={() => setExpanded(!expanded)} + role="button" + tabIndex={0} + aria-expanded={expanded} + onClick={() => setExpanded(!expanded)} + onKeyDown={(e) => { + if (e.key === 'Enter' || e.key === ' ') { + e.preventDefault(); + setExpanded(v => !v); + } + }} >
55-57: Show memory when it is 0 as well.Truthiness check hides 0.
- {stats.memoryUsage && ( + {typeof stats.memoryUsage === 'number' && ( <div>Memory: {(stats.memoryUsage / 1024 / 1024).toFixed(1)}MB</div> )}
68-75: Gate memory polling to avoid prod overhead.Let callers opt-in outside dev.
-export const usePerformanceStats = () => { +export const usePerformanceStats = (enabled: boolean = import.meta.env.DEV) => { const [stats, setStats] = useState<PerformanceStats>({ renderTime: 0, dataLoadTime: 0, filteredCount: 0, totalCount: 0 });- useEffect(() => { - const interval = setInterval(() => { - updateStats({ memoryUsage: getMemoryUsage() }); - }, 2000); - - return () => clearInterval(interval); - }, []); + useEffect(() => { + if (!enabled) return; + const interval = setInterval(() => { + updateStats({ memoryUsage: getMemoryUsage() }); + }, 2000); + return () => clearInterval(interval); + }, [enabled]);Also applies to: 94-101
dashboard/src/components/ObjectsTable.tsx (2)
56-62: Clamp page when data shrinks so pagination doesn’t point past the end.Prevents a blank grid after filters change.
useEffect(() => { - if (selectedRowIndex >= 0) { - const newPage = Math.floor(selectedRowIndex / pageSize); - if (newPage !== page) setPage(newPage); - } - }, [selectedRowIndex, pageSize, page]); + if (selectedRowIndex >= 0) { + const newPage = Math.floor(selectedRowIndex / pageSize); + if (newPage !== page) setPage(newPage); + return; + } + // Also clamp when rows shrink + const maxPage = Math.max(0, Math.ceil(rows.length / pageSize) - 1); + if (page > maxPage) setPage(maxPage); + }, [selectedRowIndex, pageSize, page, rows.length]);
41-49: Stabilize pagination handler deps and avoid redundant state writes.Minor polish: depend only on setters and primitives; bail if values are unchanged.
- const handlePaginationChange = useCallback((model: any) => { - if (model.page !== page) setPage(model.page); - if (model.pageSize !== pageSize) { + const handlePaginationChange = useCallback((model: any) => { + if (model.page !== page) setPage(model.page); + if (model.pageSize !== pageSize) { // Guard against attempts to exceed MIT cap const safeSize = Math.min(100, model.pageSize || 100); - setPageSize(safeSize); + if (safeSize !== pageSize) setPageSize(safeSize); } - }, [page, pageSize]); + }, [page, pageSize]);dashboard/src/workers/skyProjectionWorker.ts (3)
4-9: Widen SkyObject types to match parsing logic and upstream inputsYou parse possible string inputs with toNum but the interface restricts RA/DEC to number|null. Widening prevents unnecessary casts and future type churn.
-interface SkyObject { - JNAME: string; - RA?: number | null; - DEC?: number | null; - [k: string]: unknown; -} +interface SkyObject { + JNAME: string; + RA?: number | string | null; + DEC?: number | string | null; + [k: string]: unknown; +}
46-71: Guard against missing JNAME before pushing resultsEven if upstream promises JNAME, adding a cheap check avoids odd “undefined” labels if malformed rows slip through.
- result.push({ x: xNorm, y: yNorm, jname: o.JNAME, ra: RA, dec: DEC }); + if (o.JNAME) { + result.push({ x: xNorm, y: yNorm, jname: o.JNAME, ra: RA, dec: DEC }); + }
73-93: Avoid code duplication with inline worker in SkyMapSkyMap.tsx embeds an inline worker with nearly identical logic. Prefer importing this TS worker via the bundler to keep one source of truth.
-// (in SkyMap.tsx constructor) create Blob from inline script... +// Prefer module worker (Vite/Rollup/Webpack): +this.worker = new Worker( + new URL('../workers/skyProjectionWorker.ts', import.meta.url), + { type: 'module' } +);If CSP or older browsers are a concern, keep a Blob fallback but gate it behind a try/catch so the module path is the primary path.
dashboard/src/components/SkyMap.tsx (3)
290-305: Use the proper Mollweide solveTheta for grid lines (not clamp)Clamping theta to latitude warps grid lines, leading to subtle misalignment with plotted points at higher |DEC|. Reuse the Newton solver for lat→θ.
- // Simplified theta calculation for grid lines - const theta = Math.max(-Math.PI/2, Math.min(Math.PI/2, lat)); + // Use proper Mollweide solver for grid lines + const theta = (() => { + let t = Math.max(-Math.PI/2, Math.min(Math.PI/2, lat)); + for (let i = 0; i < 12; i++) { + const f = 2*t + Math.sin(2*t) - Math.PI*Math.sin(lat); + const fp = 2 + 2*Math.cos(2*t); + if (Math.abs(fp) < 1e-12) break; + const d = f/fp; + t -= d; + if (Math.abs(d) < 1e-10) break; + } + return t; + })();Apply the same change to the DEC parallels block to keep consistency.
Also applies to: 309-321, 324-351
82-90: Handle worker error messages explicitly and clear pending requestsCurrently only success paths resolve; errors rely on timeout. Resolve failures immediately to reduce latency.
- this.worker.onmessage = (e) => { - const { requestId, projectedPoints, success } = e.data; + this.worker.onmessage = (e) => { + const { requestId, projectedPoints, success, error } = e.data; const callback = this.pendingRequests.get(requestId); - if (callback && success) { + if (!callback) return; + if (success) { callback(projectedPoints); - this.pendingRequests.delete(requestId); - } + } else { + // Fallback to main thread immediately on error + callback(this.projectObjectsMainThread((e as any).data?.objects ?? [])); + } + this.pendingRequests.delete(requestId); };Also applies to: 96-104
444-458: Simplify throttling; needsRedraw flag is unusedneedsRedraw is always set but never read to coalesce frames. Either remove it or use it to avoid re-queuing inside the 16ms window.
- const scheduleRedraw = useCallback(() => { - needsRedraw.current = true; + const scheduleRedraw = useCallback(() => { if (drawFrame.current) cancelAnimationFrame(drawFrame.current); drawFrame.current = requestAnimationFrame(() => { const now = performance.now(); if (now - lastDrawTime.current >= 16) { // ~60fps cap draw(); lastDrawTime.current = now; } else { - scheduleRedraw(); + // coalesce to a single follow-up frame + drawFrame.current = requestAnimationFrame(() => { + draw(); + lastDrawTime.current = performance.now(); + }); } }); }, [draw]);Or drop needsRedraw entirely.
dashboard/src/App.tsx (2)
198-199: Avoid mutating the references array when sortingSort a copy to prevent accidental side effects if references is reused elsewhere.
- const allReferences = useMemo(()=> references.sort(), [references]); + const allReferences = useMemo(()=> [...references].sort(), [references]);
53-71: Domain sampling: skip obviously invalid values earlyA tiny micro-opt to avoid pushing ±Infinity/NaN into spreads and to normalize strings consistently.
- if(typeof v === 'string'){ const parsed = parseFloat(v); if(!isNaN(parsed)) v = parsed; } - if(typeof v === 'number' && !isNaN(v)) vals.push(v); + if (typeof v === 'string') { + const parsed = parseFloat(v); + if (!Number.isFinite(parsed)) continue; + v = parsed; + } + if (typeof v === 'number' && Number.isFinite(v)) vals.push(v);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
dashboard/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (11)
dashboard/package.json(1 hunks)dashboard/src/App.tsx(8 hunks)dashboard/src/api.ts(1 hunks)dashboard/src/components/CutoutGrid.tsx(2 hunks)dashboard/src/components/FiltersDrawer.tsx(2 hunks)dashboard/src/components/ObjectsTable.tsx(4 hunks)dashboard/src/components/PerformanceMonitor.tsx(1 hunks)dashboard/src/components/SkyMap.tsx(6 hunks)dashboard/src/hooks/useDebounce.ts(1 hunks)dashboard/src/workers/skyProjectionWorker.ts(1 hunks)dashboard/vite.config.ts(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (5)
dashboard/vite.config.ts (1)
26-32: Manual chunks look sensible; validate real-world impact.Ensure chunking doesn’t increase HTTP requests excessively and that MUI/emotion aren’t duplicated across groups. Consider a function-based manualChunks later if dependencies grow.
dashboard/src/hooks/useDebounce.ts (1)
8-23: LGTM: stable debouncing with a persistent refClearing the existing timeout before scheduling the next one is correct and avoids stale updates.
dashboard/src/api.ts (1)
83-95: Verify correct cutouts prefix
Implementation enforces “Cutouts/” while the example comment uses “Processed_Cutouts/…”. Confirm which prefix your MinIO objects actually use to avoid broken URLs.dashboard/src/components/ObjectsTable.tsx (1)
82-85: No changes needed:rowBufferPx/columnBufferPxare valid in v7+
These props were introduced in MUI X v7.0.0 (renamingrowBuffer/columnBufferto pixel-based values) and you’re on ^8.11.1, sorowBufferPx: 100andcolumnBufferPx: 100are correct.Likely an incorrect or invalid review comment.
dashboard/src/App.tsx (1)
73-87: Nice perf wins across memoized maps/filtersEarly exits, prebuilt sets, and sampling are sensible here. Nothing blocking.
Also applies to: 90-123, 131-171
- replace streamlit dashboard gif with new dashboard gif - update dashboard link to cosmoobs github page
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
- use inline worker script to avoid bundling issues - improve performance stats by removing async from measureRenderTime - fix DEC parallels calculation
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
dashboard/src/components/FiltersDrawer.tsx (1)
68-71: Potential crash when committing a slider before localNumeric initializes.localNumeric[key] can be undefined; accessing r[0] will throw. Fallback to domain defaults.
- const commitNumeric = (key: string) => { - const r = localNumeric[key]; - onChange({ ...value, numeric: { ...value.numeric, [key]: [r[0], r[1]] } }); - }; + const commitNumeric = (key: string) => { + const dom = domain[key]; + if (!dom) return; + const r = localNumeric[key] ?? [dom.min, dom.max]; + onChange({ ...value, numeric: { ...value.numeric, [key]: [r[0], r[1]] } }); + };dashboard/src/components/ObjectsTable.tsx (1)
69-85: Use supported DataGrid perf props; remove unsupported ones.disableVirtualization, rowBufferPx, and columnBufferPx aren’t DataGrid props (v6+). Use rowBuffer and columnBuffer.
onPaginationModelChange: handlePaginationChange, - // Performance optimizations - disableVirtualization: false, // Keep virtualization enabled - rowBufferPx: 100, // Reduce buffer for better performance - columnBufferPx: 100, + // Performance optimizations (supported props) + rowBuffer: 2, + columnBuffer: 2,dashboard/src/api.ts (2)
89-104: Do not commit a hard-coded ngrok endpoint. Make it configurable.Hard-coding an ngrok domain is brittle and a security/ops footgun. Use env with sane fallback.
-const RAW_ENDPOINT: string | undefined = "nonarithmetically-undeliberating-janelle.ngrok-free.app"; +const RAW_ENDPOINT: string | undefined = + (import.meta as { env?: Record<string, string> }).env?.VITE_MINIO_ENDPOINT;Optionally support a build-time dev fallback via VITE_MINIO_ENDPOINT in .env.development.
110-124: Remove forbidden ‘User-Agent’ request header.Browsers block setting User-Agent and will throw or ignore; it also triggers a CORS preflight. Keep the ngrok header only.
const response = await fetch(url, { method: 'GET', headers: { - 'ngrok-skip-browser-warning': 'true', - 'User-Agent': 'LaStBeRu-Explorer/1.0 (Custom)' + 'ngrok-skip-browser-warning': 'true' }, mode: 'cors', credentials: 'omit' });
♻️ Duplicate comments (4)
dashboard/src/hooks/useDebounce.ts (1)
5-5: Good fix: DOM-safe timer type.Switching to ReturnType avoids NodeJS.Timeout issues in browsers.
dashboard/src/components/PerformanceMonitor.tsx (1)
80-85: Fix TSX generic arrow parsing; ensure sync/async handling.In .tsx, generic arrow functions require the <T,> disambiguation. Also wrap callback with Promise.resolve for sync callbacks. This aligns with the earlier suggestion.
- const measureRenderTime = async <T>(callback: () => Promise<T> | T) => { - const start = performance.now(); - const result = await callback(); - updateStats({ renderTime: performance.now() - start }); - return result; - }; + const measureRenderTime = async <T,>(callback: () => Promise<T> | T): Promise<T> => { + const start = performance.now(); + const result = await Promise.resolve(callback()); + updateStats({ renderTime: performance.now() - start }); + return result; + };dashboard/src/components/SkyMap.tsx (2)
240-273: DEC parallels Y sign matches points/meridians now.The grid is no longer mirrored vertically. Nice fix.
152-164: Prevent stale worker results from overwriting newer projections.Older promises can resolve last. Gate state updates with a sequence token.
- const [pts, setPts] = useState<ProjectedPoint[]>([]); - const [isProjecting, setIsProjecting] = useState(false); + const [pts, setPts] = useState<ProjectedPoint[]>([]); + const [isProjecting, setIsProjecting] = useState(false); + const projectionSeqRef = useRef(0); @@ - useEffect(() => { + useEffect(() => { if (!objects || objects.length === 0) { setPts([]); return; } setIsProjecting(true); - workerInstance.projectObjects(objects).then((projectedPoints) => { - setPts(projectedPoints); - setIsProjecting(false); - }); + const seq = ++projectionSeqRef.current; + workerInstance.projectObjects(objects).then((projectedPoints) => { + if (seq === projectionSeqRef.current) { + setPts(projectedPoints); + setIsProjecting(false); + } + }); }, [objects]);
🧹 Nitpick comments (17)
dashboard/package.json (1)
37-37: Adding terser is appropriate for Vite's minify: 'terser'.LGTM. Optional: enforce Node >=18 (Vite 5 requirement) via engines to avoid CI surprises.
"private": true, "type": "module", + "engines": { "node": ">=18.0.0" },README.md (1)
49-50: Avoid relying on an ngrok endpoint in docs.The MinIO demo GIF is fine, but the service link below uses ngrok and will expire. Consider noting it's temporary or pointing to a stable hostname.
dashboard/src/components/FiltersDrawer.tsx (2)
78-92: Responsive Drawer styles look good; simplify sx if desired.Using sx as a function is unnecessary here; a plain object works and is a tad clearer.
- PaperProps={{ - sx: ({ breakpoints }) => ({ + PaperProps={{ + sx: { width: { xs: '100%', sm: 320, md: 360 }, maxWidth: '100%', background: 'linear-gradient(180deg,#0d1820,#0a141a)', display:'flex', flexDirection:'column', borderRight: '1px solid rgba(255,255,255,0.08)' - }) + } }}
114-114: Bigger ref list is helpful; consider virtualization if the set grows.maxHeight 500px is fine now; if references become large, virtualize to prevent long layouts and reflow.
dashboard/src/hooks/useDebounce.ts (1)
26-34: Remove redundant unmount cleanup effect.The cleanup returned by the main effect already runs on unmount; this extra effect is duplicate.
-// Cleanup on unmount -useEffect(() => { - return () => { - if (timeoutRef.current) { - clearTimeout(timeoutRef.current); - } - }; -}, []);dashboard/src/components/CutoutGrid.tsx (1)
40-74: Improve a11y and make extension fallback robust.
- Add alt to the img for accessibility.
- Restrict extension detection to the pathname to avoid false positives from query strings.
- {!isLoading && data && <img + {!isLoading && data && <img src={data} loading="lazy" decoding="async" crossOrigin="anonymous" - style={{ position:'absolute', inset:0, width:'100%', height:'100%', objectFit:'contain', imageRendering:'auto' }} + alt={`${record.band} cutout`} + style={{ position:'absolute', inset:0, width:'100%', height:'100%', objectFit:'contain', imageRendering:'auto' }} onError={(e)=>{ const img = e.currentTarget; if(img.dataset.fallbackTried) return; img.dataset.fallbackTried = '1'; // For ngrok URLs, try adding the bypass parameters as query string if (img.src.includes('ngrok')) { const url = new URL(img.src); url.searchParams.set('ngrok-skip-browser-warning', 'true'); img.src = url.toString(); return; } // Extension fallback: try .jpeg -> .png -> .jpg const order = ['.jpeg','.png','.jpg']; - const current = order.find(ext => img.src.toLowerCase().includes(ext)); - const nextExt = current ? order[(order.indexOf(current)+1)%order.length] : null; + const urlObj = new URL(img.src); + const path = urlObj.pathname; + const match = path.match(/\.(jpeg|png|jpg)$/i); + const current = match ? match[0].toLowerCase() : null; + const nextExt = current ? order[(order.indexOf(current)+1) % order.length] : null; if(nextExt){ - const newSrc = img.src.replace(/\.(jpeg|png|jpg)(?=($|\?))/i, nextExt); - if(newSrc !== img.src) img.src = newSrc; + urlObj.pathname = path.replace(/\.(jpeg|png|jpg)$/i, nextExt); + const newSrc = urlObj.toString(); + if(newSrc !== img.src) img.src = newSrc; } }} />}dashboard/src/workers/skyProjectionWorker.ts (1)
46-71: Consider transfer-friendly payloads for large datasets.For very large object lists, sending arrays of primitives (Float32Array for x/y, Float64Array for ra/dec, and a parallel string array for names) can cut structured-clone overhead; you can transfer the typed arrays.
dashboard/src/components/ObjectsTable.tsx (1)
125-137: Reduce style duplication by reusing paperStyles.Non-full-height branch inlines Paper styles; reuse paperStyles for consistency.
- return ( - <Paper sx={{ p:1, background:'rgba(255,255,255,0.02)', backdropFilter:'blur(4px)', border:'1px solid rgba(255,255,255,0.05)' }}> + return ( + <Paper sx={paperStyles}>dashboard/src/api.ts (1)
128-154: Blob URL lifecycle: advise revocation.Returning a blob URL shifts revocation to callers; document this or expose a disposer to avoid leaks on long sessions.
I can add a small helper returning { url, revoke() } and update call-sites.
dashboard/src/components/PerformanceMonitor.tsx (1)
94-101: Memory polling: gate the interval to visible state.Avoid background timers when the widget is hidden; start/stop based on isDev||show.
- useEffect(() => { - const interval = setInterval(() => { - updateStats({ memoryUsage: getMemoryUsage() }); - }, 2000); - - return () => clearInterval(interval); - }, []); + useEffect(() => { + if (!import.meta.env.DEV && !stats && !false) return; // no-op placeholder; or derive a `visible` boolean prop + const interval = setInterval(() => { + updateStats({ memoryUsage: getMemoryUsage() }); + }, 2000); + return () => clearInterval(interval); + }, [/* visible */]);If you expose a
visibleboolean to the hook, depend on it instead of the placeholder.dashboard/vite.config.ts (2)
26-32: Avoid bundling the entire MUI Icons library into a single chunk.Manual-chunking
@mui/icons-materialforces a very large chunk even if you use a handful of icons. Let Rollup tree-shake per-icon imports instead.manualChunks: { 'react-vendor': ['react', 'react-dom'], 'mui-core': ['@mui/material', '@mui/system', '@emotion/react', '@emotion/styled'], - 'mui-icons': ['@mui/icons-material'], 'mui-datagrid': ['@mui/x-data-grid'], 'query-vendor': ['@tanstack/react-query'], 'utils': ['recoil'] }
36-45: Pre-bundle Emotion/MUI system for faster dev cold-start.MUI depends on
@emotion/react,@emotion/styled, and@mui/system. Including them inoptimizeDeps.includeoften reduces dev-time lag.optimizeDeps: { include: [ 'react', 'react-dom', '@mui/material', + '@mui/system', + '@emotion/react', + '@emotion/styled', '@mui/icons-material', '@mui/x-data-grid', '@tanstack/react-query' ] },dashboard/src/App.tsx (2)
53-71: Compute numeric domains in one pass; avoid buildingvalsarrays.Streaming min/max reduces memory and GC, especially for large datasets.
- numericFields.forEach(f=>{ - const vals: number[] = []; - // Sample every 10th record for large datasets to speed up domain calculation - const step = database.length > 10000 ? 10 : 1; - for(let i = 0; i < database.length; i += step){ - const r = database[i]; - if(!r) continue; - let v: unknown = r[f.key]; - if(typeof v === 'string'){ const parsed = parseFloat(v); if(!isNaN(parsed)) v = parsed; } - if(typeof v === 'number' && !isNaN(v)) vals.push(v); - } - if(vals.length){ acc[f.key] = { min: Math.min(...vals), max: Math.max(...vals) }; } - }); + numericFields.forEach((f) => { + const step = database.length > 10000 ? 10 : 1; + let min = Infinity, max = -Infinity, seen = 0; + for (let i = 0; i < database.length; i += step) { + const r = database[i]; if (!r) continue; + let v: unknown = r[f.key]; + if (typeof v === 'string') { const p = parseFloat(v); if (!Number.isNaN(p)) v = p; } + if (typeof v === 'number' && Number.isFinite(v)) { + if (v < min) min = v; + if (v > max) max = v; + seen++; + } + } + if (seen) acc[f.key] = { min, max }; + });
198-199: Don’t mutatereferencesin-place when sorting.
Array.prototype.sortmutates the source. Use a copy to avoid side effects.- const allReferences = useMemo(()=> references.sort(), [references]); + const allReferences = useMemo(()=> [...references].sort(), [references]);dashboard/src/components/SkyMap.tsx (3)
332-343: Set paint styles once outside the loop for the detailed path.Minor perf win; avoid reassigning styles per point.
- for(const p of pts){ + ctx.fillStyle = 'rgba(90,180,220,0.78)'; + ctx.strokeStyle = 'rgba(0,0,0,0.45)'; + ctx.lineWidth = 0.3/Math.max(baseScaleX*zoom, baseScaleY*zoom); + for (const p of pts) { if(p.jname === selected) continue; const rWorld = basePxAdj * pxToWorld; ctx.beginPath(); ctx.arc(p.x,p.y, rWorld,0,Math.PI*2); - ctx.fillStyle = 'rgba(90,180,220,0.78)'; - ctx.strokeStyle = 'rgba(0,0,0,0.45)'; - ctx.lineWidth = 0.3/Math.max(baseScaleX*zoom, baseScaleY*zoom); ctx.fill(); ctx.stroke(); }
40-47: Always clear pending request entries; handle worker errors explicitly.Avoid leaks and ensure timeouts are cleared when messages arrive.
- this.worker.onmessage = (e) => { - const { requestId, projectedPoints, success } = e.data; - const callback = this.pendingRequests.get(requestId); - if (callback && success) { - callback(projectedPoints); - this.pendingRequests.delete(requestId); - } - }; + this.worker.onmessage = (e) => { + const { requestId, projectedPoints, success } = e.data || {}; + const cb = this.pendingRequests.get(requestId); + if (!cb) return; + this.pendingRequests.delete(requestId); + cb(success !== false && Array.isArray(projectedPoints) ? projectedPoints : []); + };
539-544: Consider cleaning up thebeforeunloadlistener on HMR.Not critical, but adding it conditionally during dev can avoid duplicate handlers under HMR.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (4)
.figures/dashboard.gifis excluded by!**/*.gif.figures/minio_service.gifis excluded by!**/*.gif.figures/streamlit_dashboard.gifis excluded by!**/*.gifdashboard/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (12)
README.md(1 hunks)dashboard/package.json(1 hunks)dashboard/src/App.tsx(8 hunks)dashboard/src/api.ts(1 hunks)dashboard/src/components/CutoutGrid.tsx(2 hunks)dashboard/src/components/FiltersDrawer.tsx(2 hunks)dashboard/src/components/ObjectsTable.tsx(4 hunks)dashboard/src/components/PerformanceMonitor.tsx(1 hunks)dashboard/src/components/SkyMap.tsx(6 hunks)dashboard/src/hooks/useDebounce.ts(1 hunks)dashboard/src/workers/skyProjectionWorker.ts(1 hunks)dashboard/vite.config.ts(1 hunks)
🧰 Additional context used
🪛 Biome (2.1.2)
dashboard/src/components/SkyMap.tsx
[error] 56-59: Expected a statement but instead found '}
}
async projectObjects(objects: SkyObject[]): Promise<ProjectedPoint[]>'.
Expected a statement here.
(parse)
[error] 62-62: Illegal return statement outside of a function
(parse)
[error] 65-77: Illegal return statement outside of a function
(parse)
[error] 80-80: Illegal use of reserved keyword private as an identifier in strict mode
(parse)
[error] 80-80: Expected a semicolon or an implicit semicolon after a statement, but found none
An explicit or implicit semicolon is expected here...
...Which is required to end this statement
(parse)
[error] 80-80: expected , but instead found :
Remove :
(parse)
[error] 80-80: expected , but instead found [
Remove [
(parse)
[error] 80-80: Expected a semicolon or an implicit semicolon after a statement, but found none
An explicit or implicit semicolon is expected here...
...Which is required to end this statement
(parse)
[error] 121-121: Illegal return statement outside of a function
(parse)
[error] 124-124: Expected a semicolon or an implicit semicolon after a statement, but found none
An explicit or implicit semicolon is expected here...
...Which is required to end this statement
(parse)
[error] 130-131: Expected a statement but instead found '}'.
Expected a statement here.
(parse)
[error] 306-306: expected , but instead found ctx
Remove ctx
(parse)
[error] 307-307: expected , but instead found ;
Remove ;
(parse)
[error] 396-396: This hook is being called conditionally, but all hooks must be called in the exact same order in every component render.
For React to preserve state between calls, hooks needs to be called unconditionally and always in the same order.
See https://reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level
(lint/correctness/useHookAtTopLevel)
[error] 411-411: This hook is being called conditionally, but all hooks must be called in the exact same order in every component render.
For React to preserve state between calls, hooks needs to be called unconditionally and always in the same order.
See https://reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level
(lint/correctness/useHookAtTopLevel)
[error] 412-412: This hook is being called conditionally, but all hooks must be called in the exact same order in every component render.
For React to preserve state between calls, hooks needs to be called unconditionally and always in the same order.
See https://reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level
(lint/correctness/useHookAtTopLevel)
[error] 437-437: This hook is being called conditionally, but all hooks must be called in the exact same order in every component render.
For React to preserve state between calls, hooks needs to be called unconditionally and always in the same order.
See https://reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level
(lint/correctness/useHookAtTopLevel)
[error] 440-440: This hook is being called conditionally, but all hooks must be called in the exact same order in every component render.
For React to preserve state between calls, hooks needs to be called unconditionally and always in the same order.
See https://reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level
(lint/correctness/useHookAtTopLevel)
dashboard/src/components/PerformanceMonitor.tsx
[error] 80-80: expected , but instead found :
Remove :
(parse)
[error] 80-80: expected , but instead found (
Remove (
(parse)
[error] 80-81: Expected a semicolon or an implicit semicolon after a statement, but found none
An explicit or implicit semicolon is expected here...
...Which is required to end this statement
(parse)
[error] 82-82: await is only allowed within async functions and at the top levels of modules.
(parse)
[error] 94-94: This hook is being called conditionally, but all hooks must be called in the exact same order in every component render.
Hooks should not be called after an early return.
For React to preserve state between calls, hooks needs to be called unconditionally and always in the same order.
See https://reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level
(lint/correctness/useHookAtTopLevel)
🔇 Additional comments (8)
dashboard/src/workers/skyProjectionWorker.ts (1)
19-23: Confirm RA→lon handedness matches the UI expectation.raToLon flips RA so it increases to the left (common in sky maps). Verify SkyMap uses the same convention to avoid mirrored plots.
dashboard/src/components/ObjectsTable.tsx (2)
87-105: Nice: centralized memoized styles.Good consolidation; avoids re-allocation and keeps SX terse.
31-34: Retain pageSize clamp (100 max)
MUI X DataGrid MIT (v6+) enforces a hard cap of 100 onpageSize(v5.mui.com, mui.com).dashboard/src/api.ts (2)
28-73: Solid in-flight de-dupe + timeout.Promise cache + AbortController + concise error surfacing looks good.
33-41: Replace stale-friendly fetch options and drop invalid request directive
Changecache: 'force-cache'tocache: 'default'(orno-cacheif you need revalidation) and remove the'Cache-Control': 'public, max-age=3600'header at lines 33–41, 44–51, and 55–61.dashboard/src/components/PerformanceMonitor.tsx (1)
23-31: Dev-only logging is good; keep it tree-shakeable.This guard matches build settings that drop console in prod. LGTM.
dashboard/vite.config.ts (1)
15-15: Dependency verified:terseris present at version^5.44.0in dashboard/package.json; no action needed.dashboard/src/components/SkyMap.tsx (1)
34-47: Good move to a module worker import.Using
new URL(..., import.meta.url)keeps code single-sourced and bundler-friendly.
| // Memoize rows computation to avoid recalculation on every render | ||
| const rows = useMemo(()=> { | ||
| if (!objects || objects.length === 0) return []; | ||
| return objects | ||
| .filter(o=> !!o?.JNAME) | ||
| .map((o,i)=> ({ id: o.JNAME || i, ...o })); | ||
| }, [objects]); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Stable row ids: drop the index fallback.
You already filter out rows without JNAME, so the index fallback risks unstable ids across filters/sorts. Use JNAME only.
- .map((o,i)=> ({ id: o.JNAME || i, ...o }));
+ .map((o)=> ({ id: o.JNAME, ...o }));📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Memoize rows computation to avoid recalculation on every render | |
| const rows = useMemo(()=> { | |
| if (!objects || objects.length === 0) return []; | |
| return objects | |
| .filter(o=> !!o?.JNAME) | |
| .map((o,i)=> ({ id: o.JNAME || i, ...o })); | |
| }, [objects]); | |
| // Memoize rows computation to avoid recalculation on every render | |
| const rows = useMemo(()=> { | |
| if (!objects || objects.length === 0) return []; | |
| return objects | |
| .filter(o=> !!o?.JNAME) | |
| .map((o)=> ({ id: o.JNAME, ...o })); | |
| }, [objects]); |
🤖 Prompt for AI Agents
In dashboard/src/components/ObjectsTable.tsx around lines 19 to 25, the current
rows memo falls back to the array index as id (id: o.JNAME || i) which can
produce unstable ids when filtering or sorting; remove the index fallback and
use JNAME only as the stable id (e.g. id: o.JNAME) after the existing filter
that ensures JNAME exists, and keep the useMemo dependency on objects.
| const header = useMemo(() => ( | ||
| <Typography variant="caption" sx={{ pl:1, fontWeight:600, letterSpacing:0.5 }}> | ||
| Filtered Objects | ||
| Filtered Objects ({objects.length}) | ||
| </Typography> | ||
| ); | ||
| ), [objects.length]); | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Show the actual filtered count.
Header uses objects.length, which can mismatch rows.length after filtering null JNAMEs.
- Filtered Objects ({objects.length})
+ Filtered Objects ({rows.length})📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const header = useMemo(() => ( | |
| <Typography variant="caption" sx={{ pl:1, fontWeight:600, letterSpacing:0.5 }}> | |
| Filtered Objects | |
| Filtered Objects ({objects.length}) | |
| </Typography> | |
| ); | |
| ), [objects.length]); | |
| const header = useMemo(() => ( | |
| <Typography variant="caption" sx={{ pl:1, fontWeight:600, letterSpacing:0.5 }}> | |
| Filtered Objects ({rows.length}) | |
| </Typography> | |
| ), [objects.length]); |
🤖 Prompt for AI Agents
In dashboard/src/components/ObjectsTable.tsx around lines 63 to 68, the header
currently displays objects.length which can differ from the actual rendered
(filtered) rows when null JNAMEs are removed; update the header to display the
actual filtered count (e.g., use rows.length or compute filteredObjects.length)
and adjust the useMemo dependency to depend on that filtered count (or rows) so
the caption updates correctly after filtering out null JNAMEs.
dashboard/src/components/SkyMap.tsx
Outdated
| // ... rest of class methods ... | ||
| } | ||
| console.warn('Web Worker not available, falling back to main thread'); | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
Fix broken class body: methods defined outside class + stray tokens cause parse/build failures.
SkyProjectionWorker closes early; stray lines and methods live at module scope. This matches the static analysis parse errors. Fold methods back into the class and remove artifacts.
-class SkyProjectionWorker {
- private worker: Worker | null = null;
- private requestId = 0;
- private pendingRequests = new Map<number, (result: ProjectedPoint[]) => void>();
-
- constructor() {
- try {
- // Primary path: module worker handled by bundler
- this.worker = new Worker(
- new URL('../workers/skyProjectionWorker.ts', import.meta.url),
- { type: 'module' }
- );
-
- this.worker.onmessage = (e) => {
- const { requestId, projectedPoints, success } = e.data;
- const callback = this.pendingRequests.get(requestId);
- if (callback && success) {
- callback(projectedPoints);
- this.pendingRequests.delete(requestId);
- }
- };
- } catch (error) {
- console.warn('Module Worker not available; falling back to main thread.');
- }
- }
-
- // ... rest of class methods ...
-}
- console.warn('Web Worker not available, falling back to main thread');
- }
- }
-
- async projectObjects(objects: SkyObject[]): Promise<ProjectedPoint[]> {
+class SkyProjectionWorker {
+ private worker: Worker | null = null;
+ private requestId = 0;
+ private pendingRequests = new Map<number, (result: ProjectedPoint[]) => void>();
+
+ constructor() {
+ try {
+ this.worker = new Worker(
+ new URL('../workers/skyProjectionWorker.ts', import.meta.url),
+ { type: 'module' }
+ );
+ this.worker.onmessage = (e) => {
+ const { requestId, projectedPoints, success } = e.data || {};
+ const cb = this.pendingRequests.get(requestId);
+ if (!cb) return;
+ this.pendingRequests.delete(requestId);
+ cb(success !== false && Array.isArray(projectedPoints) ? projectedPoints : []);
+ };
+ this.worker.onerror = () => {
+ // drain any pending promises with empty result
+ for (const [, resolve] of this.pendingRequests) resolve([]);
+ this.pendingRequests.clear();
+ };
+ } catch {
+ console.warn('Module Worker not available; falling back to main thread.');
+ this.worker = null;
+ }
+ }
+
+ async projectObjects(objects: SkyObject[]): Promise<ProjectedPoint[]> {
// Fallback to main thread if worker unavailable or for small datasets
if (!this.worker || objects.length < 1000) {
return this.projectObjectsMainThread(objects);
}
- return new Promise((resolve) => {
- const id = ++this.requestId;
- this.pendingRequests.set(id, resolve);
- this.worker!.postMessage({ objects, requestId: id });
-
- // Timeout fallback
- setTimeout(() => {
- if (this.pendingRequests.has(id)) {
- this.pendingRequests.delete(id);
- resolve(this.projectObjectsMainThread(objects));
- }
- }, 1000);
- });
+ return new Promise((resolve) => {
+ const id = ++this.requestId;
+ const to = setTimeout(() => {
+ if (this.pendingRequests.delete(id)) {
+ resolve(this.projectObjectsMainThread(objects));
+ }
+ }, 3000);
+ this.pendingRequests.set(id, (res) => { clearTimeout(to); resolve(res); });
+ this.worker!.postMessage({ objects, requestId: id });
+ });
}
- private projectObjectsMainThread(objects: SkyObject[]): ProjectedPoint[] {
+ private projectObjectsMainThread(objects: SkyObject[]): ProjectedPoint[] {
const result: ProjectedPoint[] = [];
const raToLon = (raDeg: number) => {
const wrapped = ((raDeg + 180) % 360) - 180;
return -wrapped;
};
const deg2rad = (d: number) => d * Math.PI / 180;
function solveTheta(phi: number) {
const HALF_PI = Math.PI / 2;
if (Math.abs(Math.abs(phi) - HALF_PI) < 1e-12) {
return Math.sign(phi) * HALF_PI;
}
let theta = Math.max(-HALF_PI, Math.min(HALF_PI, phi));
for (let i = 0; i < 12; i++) {
const f = 2 * theta + Math.sin(2 * theta) - Math.PI * Math.sin(phi);
const fp = 2 + 2 * Math.cos(2 * theta);
if (Math.abs(fp) < 1e-12) break;
const delta = f / fp;
theta -= delta;
if (Math.abs(delta) < 1e-10) break;
}
return theta;
}
for (const o of objects) {
if (!o) continue;
const toNum = (v: any) => typeof v === 'number' ? v : (typeof v === 'string' ? parseFloat(v) : NaN);
let RA = toNum(o.RA);
let DEC = toNum(o.DEC);
if (isNaN(RA) || isNaN(DEC)) continue;
if (RA < 0) RA = ((RA % 360) + 360) % 360;
if (RA >= 360) RA = RA % 360;
if (DEC < -90 || DEC > 90) continue;
const lon = deg2rad(raToLon(RA));
const lat = deg2rad(DEC);
const theta = solveTheta(lat);
const xNorm = (2 * Math.SQRT2 / Math.PI) * lon * Math.cos(theta);
const yNorm = -Math.SQRT2 * Math.sin(theta);
result.push({ x: xNorm, y: yNorm, jname: o.JNAME, ra: RA, dec: DEC });
}
return result;
}
- destroy() {
+ destroy() {
if (this.worker) {
this.worker.terminate();
this.worker = null;
}
this.pendingRequests.clear();
}
}Also applies to: 59-122, 124-130
🤖 Prompt for AI Agents
In dashboard/src/components/SkyMap.tsx around lines 53–130 the
SkyProjectionWorker class body is closed too early which left several methods
and stray tokens at module scope causing parse/build failures; reopen the class
and move the orphaned methods back inside the SkyProjectionWorker class body,
remove any extra closing brace(s) and stray statements (e.g., duplicate tokens
or misplaced console.warn lines) so the class is syntactically complete,
preserve method order/visibility, and ensure the worker fallback logic remains
inside the appropriate constructor or method; after edits run TypeScript/ESLint
to verify the file parses and builds cleanly.
| const scheduleRedraw = useCallback(() => { | ||
| needsRedraw.current = true; | ||
| if (drawFrame.current) cancelAnimationFrame(drawFrame.current); | ||
| drawFrame.current = requestAnimationFrame(() => { | ||
| const now = performance.now(); | ||
| if (now - lastDrawTime.current >= 16) { // ~60fps cap | ||
| draw(); | ||
| lastDrawTime.current = now; | ||
| } else { | ||
| scheduleRedraw(); | ||
| } | ||
| }); |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
About the hook-order errors from static analysis.
These are likely false positives caused by the earlier class syntax errors confusing the parser. After fixing the class body, re-run lint; the hooks are called unconditionally.
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Re-run typecheck/lint after class fix
fd -t f tsconfig*.json
echo "Typecheck:"
npx -y tsc -p dashboard/tsconfig.json --noEmit || true
echo "Biome/Lint:"
npx -y @biomejs/biome lint dashboard/src/components/SkyMap.tsx || trueLength of output: 13445
Resolve SkyMap.tsx class syntax errors
Fix mismatched braces and illegal method definitions in the SkyProjectionWorker class (projectObjects at ~lines 56–65, projectObjectsMainThread at ~80, destroy at ~124), which are currently causing parse failures. Address these critical syntax errors before re-running lint or assessing hook-order issues.
🧰 Tools
🪛 Biome (2.1.2)
[error] 396-396: This hook is being called conditionally, but all hooks must be called in the exact same order in every component render.
For React to preserve state between calls, hooks needs to be called unconditionally and always in the same order.
See https://reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level
(lint/correctness/useHookAtTopLevel)
| // Listen for messages from main thread | ||
| self.onmessage = function(e) { | ||
| const { objects, requestId } = e.data; | ||
|
|
||
| try { | ||
| const projectedPoints = projectObjects(objects); | ||
|
|
||
| // Send result back to main thread | ||
| self.postMessage({ | ||
| requestId, | ||
| projectedPoints, | ||
| success: true | ||
| }); | ||
| } catch (error) { | ||
| self.postMessage({ | ||
| requestId, | ||
| error: error instanceof Error ? error.message : 'Unknown error', | ||
| success: false | ||
| }); | ||
| } | ||
| }; |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Type the worker message and enable proper worker typings.
Prevents noImplicitAny errors and improves DX. Also consider including the WebWorker lib in tsconfig.
+/// <reference lib="webworker" />
+
+declare const self: DedicatedWorkerGlobalScope;
+
-// Listen for messages from main thread
-self.onmessage = function(e) {
- const { objects, requestId } = e.data;
+// Listen for messages from main thread
+self.onmessage = (e: MessageEvent<{ objects: SkyObject[]; requestId: string }>) => {
+ const { objects, requestId } = e.data;
try {
const projectedPoints = projectObjects(objects);
// Send result back to main thread
self.postMessage({
requestId,
projectedPoints,
success: true
});
} catch (error) {
self.postMessage({
requestId,
error: error instanceof Error ? error.message : 'Unknown error',
success: false
});
}
-};
+};🏁 Script executed:
#!/bin/bash
# Ensure TS has webworker lib enabled
fd -a 'tsconfig*.json' dashboard | while read f; do echo "## $f"; cat "$f"; echo; done | rg -nP '"lib"\s*:\s*\[[^\]]*webworker'Length of output: 0
🏁 Script executed:
#!/bin/bash
# Search for lib entries in all tsconfig*.json files and check for “webworker”
rg -nH '"lib"' --include 'tsconfig*.json' .
rg -nH 'webworker' --include 'tsconfig*.json' .Length of output: 883
🏁 Script executed:
#!/bin/bash
# Correctly search for lib entries and presence of webworker in tsconfig.json files
rg -nH '"lib"' --glob 'tsconfig*.json' .
rg -nH 'webworker' --glob 'tsconfig*.json' .Length of output: 73
Type worker messages & enable proper Worker typings
Prevent noImplicitAny in dashboard/src/workers/skyProjectionWorker.ts by adding a worker lib reference, declaring self as DedicatedWorkerGlobalScope, and typing the onmessage handler. Also update dashboard/tsconfig.json to include "WebWorker" in the lib array.
+/// <reference lib="webworker" />
+
+declare const self: DedicatedWorkerGlobalScope;
-// Listen for messages from main thread
-self.onmessage = function(e) {
- const { objects, requestId } = e.data;
+// Listen for messages from main thread
+self.onmessage = (e: MessageEvent<{ objects: SkyObject[]; requestId: string }>) => {
+ const { objects, requestId } = e.data;
try {
const projectedPoints = projectObjects(objects);
// Send result back to main thread
self.postMessage({
requestId,
projectedPoints,
success: true
});
} catch (error) {
self.postMessage({
requestId,
error: error instanceof Error ? error.message : 'Unknown error',
success: false
});
}
-};
+};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Listen for messages from main thread | |
| self.onmessage = function(e) { | |
| const { objects, requestId } = e.data; | |
| try { | |
| const projectedPoints = projectObjects(objects); | |
| // Send result back to main thread | |
| self.postMessage({ | |
| requestId, | |
| projectedPoints, | |
| success: true | |
| }); | |
| } catch (error) { | |
| self.postMessage({ | |
| requestId, | |
| error: error instanceof Error ? error.message : 'Unknown error', | |
| success: false | |
| }); | |
| } | |
| }; | |
| /// <reference lib="webworker" /> | |
| declare const self: DedicatedWorkerGlobalScope; | |
| // Listen for messages from main thread | |
| self.onmessage = (e: MessageEvent<{ objects: SkyObject[]; requestId: string }>) => { | |
| const { objects, requestId } = e.data; | |
| try { | |
| const projectedPoints = projectObjects(objects); | |
| // Send result back to main thread | |
| self.postMessage({ | |
| requestId, | |
| projectedPoints, | |
| success: true | |
| }); | |
| } catch (error) { | |
| self.postMessage({ | |
| requestId, | |
| error: error instanceof Error ? error.message : 'Unknown error', | |
| success: false | |
| }); | |
| } | |
| }; |
🤖 Prompt for AI Agents
In dashboard/src/workers/skyProjectionWorker.ts around lines 73–93, the worker
file lacks proper TypeScript worker typings causing noImplicitAny errors; add a
triple-slash lib reference for WebWorker at the top of the file, declare `self`
as `DedicatedWorkerGlobalScope` (or use `declare const self:
DedicatedWorkerGlobalScope`) and strongly type the onmessage handler parameter
as a MessageEvent with the expected payload shape (objects and requestId) and
return type; finally update dashboard/tsconfig.json to include "WebWorker" in
the "lib" array so the DedicatedWorkerGlobalScope and Worker types are available
to the compiler.
| terserOptions: { | ||
| compress: { | ||
| drop_console: true, // Remove console.log in production | ||
| drop_debugger: true, | ||
| pure_funcs: ['console.log', 'console.info', 'console.debug'] | ||
| } | ||
| }, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Don't drop console.error/warn in production (misconfigured Terser).
drop_console: true will remove all console calls, including console.warn and console.error, which hurts prod debugging. Your intent/comment mentions only removing logs. Switch to keeping errors/warnings and rely on pure_funcs for log/info/debug.
- terserOptions: {
- compress: {
- drop_console: true, // Remove console.log in production
- drop_debugger: true,
- pure_funcs: ['console.log', 'console.info', 'console.debug']
- }
- },
+ terserOptions: {
+ compress: {
+ drop_console: false, // keep warn/error
+ drop_debugger: true,
+ pure_funcs: ['console.log', 'console.info', 'console.debug']
+ },
+ format: { comments: false }
+ },📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| terserOptions: { | |
| compress: { | |
| drop_console: true, // Remove console.log in production | |
| drop_debugger: true, | |
| pure_funcs: ['console.log', 'console.info', 'console.debug'] | |
| } | |
| }, | |
| terserOptions: { | |
| compress: { | |
| drop_console: false, // keep warn/error | |
| drop_debugger: true, | |
| pure_funcs: ['console.log', 'console.info', 'console.debug'] | |
| }, | |
| format: { comments: false } | |
| }, |
🤖 Prompt for AI Agents
In dashboard/vite.config.ts around lines 16 to 22, the terserOptions currently
set drop_console: true which will remove all console calls including
console.warn and console.error; change this so that drop_console is false or
removed, and rely on pure_funcs = ['console.log','console.info','console.debug']
to strip only the intended log/info/debug calls so that console.warn and
console.error are preserved in production.
| <img src=".figures/dashboard.gif" width="60%" alt="Animated GIF of the slcomp interactive dashboard" /> | ||
| </p> |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Check asset availability on GH Pages.
Switched to .figures/dashboard.gif at 60%. Verify the file exists in the repo and renders on GitHub Pages.
🏁 Script executed:
#!/bin/bash
# Confirm the asset exists in the repo
fd -a '.figures/dashboard.gif'Length of output: 0
🏁 Script executed:
#!/bin/bash
# Search for any occurrence of dashboard.gif in the repo (case‐insensitive)
find . -type f -iname 'dashboard.gif'Length of output: 24
Rename asset directory and update README path
The file exists at .figures/dashboard.gif, but GitHub Pages (Jekyll) ignores dot-prefixed folders by default. Rename .figures/ to figures/, move dashboard.gif there, and update README.md (lines 39–40) to:
<img src="figures/dashboard.gif" width="60%" alt="Animated GIF of the slcomp interactive dashboard" />🤖 Prompt for AI Agents
In README.md around lines 39 to 40, the image path points to a dot-prefixed
folder `.figures/` which GitHub Pages (Jekyll) ignores; rename the directory
from `.figures` to `figures`, move dashboard.gif into the new figures/ folder,
and update the README image tag src to use "figures/dashboard.gif" so the GIF is
served correctly.
| </p> | ||
|
|
||
| ➡️ **[Dashboard Link](https://slcomp-public.streamlit.app)** | ||
| ➡️ **[Dashboard Link](https://cosmoobs.github.io/slcomp)** |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Confirm Vite base matches the new GitHub Pages path.
With https://cosmoobs.github.io/slcomp, base should typically be "/slcomp/". Ensure vite.config.ts aligns to avoid broken assets.
🏁 Script executed:
#!/bin/bash
# Look for Vite base config
rg -nP 'base\s*:\s*[\'"]\/slcomp\/[\'"]' -g 'dashboard/**/vite.config.*'Length of output: 68
🏁 Script executed:
#!/bin/bash
# Search for Vite base config with correct quoting in root and dashboard directories
rg -nP "base\s*:\s*['\"]/slcomp/['\"]" -g "vite.config.*" -n
rg -nP "base\s*:\s*['\"]/slcomp/['\"]" -g "dashboard/**/vite.config.*" -nLength of output: 0
🏁 Script executed:
#!/bin/bash
# Locate all Vite config files and check for a base setting
configs=$(find . -type f -maxdepth 3 -iname 'vite.config.*')
if [ -z "$configs" ]; then
echo "No Vite config files found"
else
for file in $configs; do
echo "=== $file ==="
rg -n -C3 "base" "$file" || echo " (no 'base' property found in this file)"
done
fiLength of output: 348
Set Vite basePath to “/slcomp/” for GitHub Pages builds
Vite config (dashboard/vite.config.ts lines 6–9) reads process.env.BASE_PATH with a '/' fallback. To serve assets correctly at https://cosmoobs.github.io/slcomp, either export BASE_PATH=/slcomp/ before building or update the default fallback to '/slcomp/'.
🤖 Prompt for AI Agents
In README.md around line 42, the note links to the dashboard but the Vite build
will serve assets incorrectly unless BASE_PATH is set; update the instruction to
tell developers to either export BASE_PATH=/slcomp/ before running the build or
change the Vite config fallback to '/slcomp/' so assets are served from
https://cosmoobs.github.io/slcomp; explicitly document the exact export command
(export BASE_PATH=/slcomp/) and/or state that changing the default fallback in
dashboard/vite.config.ts (lines 6–9) to '/slcomp/' will fix GitHub Pages
deployments.
Summary by CodeRabbit
New Features
Performance
Documentation
Chores