Project resumption: lint/security cleanup, CI gate, per-node coverage visualization (P6-1) - #20
Merged
Merged
Conversation
eslint.config.js had no plugin providing JSX-usage detection, so plain no-unused-vars couldn't see that a capitalized JSX tag references its import -- this produced ~100 false-positive warnings across the codebase that were masking real ones. Add eslint-plugin-react's jsx-uses-vars rule (only that rule, not its full stylistic/prop-types set). With the false positives gone, fix what was left: a dead `elevation` var in CoverageLayerManager, an unused caught SSE-parse error in useSimulationStore, and annotate two intentional signal-triggered useEffects (CoverageLayerManager, RFContext) that read the latest render's closure rather than tracking every dependency. Also run npm audit fix to close all production vulnerabilities (critical fast-xml-parser via @loaders.gl/xml, high lodash, moderate protocol-buffers-schema) plus the dev-tooling ones (vite, vitest, brace-expansion). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DwJgxrJr8scmomw9B8Lk9W
ROADMAP.md had the P5-6/P5-7 sections duplicated verbatim from a copy-paste error. README.md links to Documentation/pwa-guide.md, which never existed -- write it from the actual vite-plugin-pwa config. Drop itmlogic from rf-engine/requirements.txt: it was declared but never imported (ROADMAP P4-1 is still open, re-add when that's implemented). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DwJgxrJr8scmomw9B8Lk9W
.github/workflows only had docker-publish and cleanup-packages -- nothing ran npm run lint, npm test, or the rf-engine pytest suite on pushes/PRs. Add a ci.yml workflow that runs both on dev/main pushes and PRs targeting them, so regressions don't silently accumulate. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DwJgxrJr8scmomw9B8Lk9W
Multi-Site Analysis previously rendered every selected node's viewshed into one flat cyan composite, making it impossible to tell which node covers which area or spot redundant placements visually (the numeric "unique coverage %" was the only signal). Backend (rf-engine/core/viewshed_proc.py): assign each selected node an evenly-spaced distinct hue and render coverage as alpha-composited color layers instead of a single flat mask -- overlapping nodes blend naturally where their coverage intersects. Each result now carries a `color` field. Also factor the pixel-projection math (mapping a node's local viewshed grid into the shared master-grid coordinates) into one `_project_to_master` helper, replacing three near-identical inline copies used by the greedy optimizer, marginal-coverage pass, and composite renderer. Frontend: color-match the simulation node markers and popups (OptimizationLayerManager.jsx) and the Sites results table (SitesTab.jsx) to the same per-node color, so a node's marker, popup, and coverage patch on the map are visually tied together. Also fixes a pre-existing dead `res.coverage_points` reference in the marker popup (the backend never produced that field) by showing unique coverage % instead. Adds test coverage for the new color assignment and composite rendering, which the prior tests never exercised (they only used all-zero grids). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DwJgxrJr8scmomw9B8Lk9W
Summarizes the lint/security cleanup, new CI workflow, and P6-1 per-node coverage visualization from the preceding commits. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DwJgxrJr8scmomw9B8Lk9W
Now that JSX usage is correctly detected, clean up what was left: - Removed genuinely dead code: unused txIcon/rxIcon constants and an abandoned fresnelRef in LinkLayer.jsx, an unused syncState helper and its backing ref in OptimizationLayer.jsx, and stale proxy-value destructuring left over from the P2-4 per-node-config refactor. - Removed the entire `coverageOverlay` state chain (useLinkTool.js -> MapContainer.jsx -> LinkLayerManager.jsx -> LinkLayer.jsx): it was only ever set to null, never to a real value, so the ImageOverlay it fed never rendered anything. - Fixed a real gap in HeatmapOverlay.jsx: a per-point opacity was computed from score but never applied, the fillOpacity was hardcoded to 0.3 instead. - Fixed a dead `res.coverage_points` style leftover (unused args/index params) across OptimizationResultsPanel, BatchNodesList, ModelComparisonTable, SiteAnalysisResultsPanel. - Annotated intentional signal-triggered useEffects (Sidebar mobile auto-close, DeckGLOverlay mount lifecycle, LinkLayer/OptimizationLayer recalc-on-signal) that eslint's exhaustive-deps flagged -- adding the suggested deps would have introduced real bugs (e.g. Sidebar's toggle button fighting its own auto-close effect). - Disabled react-refresh/only-export-components at the file level for the 5 context files (EnvironmentContext, HardwareContext, RFContext, RadioContext, UIContext). Splitting each Provider from its hook/constants is the "proper" fix but would touch ~25 import sites across the app for a dev-only Fast Refresh granularity concern with no effect on correctness or production builds -- not proportionate here. Verified: npm run lint (0 warnings), npm test (7/7), npm run build, pytest rf-engine/tests (9/9). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DwJgxrJr8scmomw9B8Lk9W
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The project had been dormant since v1.16.1 (Feb 2026). This PR picks it back up on four fronts:
eslint.config.jshad no way to recognize JSX component usage (noeslint-plugin-react), so plainno-unused-varsproduced ~100 false-positive "unused import" warnings (132 → 35 after the fix). The remaining real issues found underneath (dead variable, unused caught error, staleeslint-disabledirectives, two intentional-but-unannotateduseEffectdeps) are fixed/annotated.npm auditfindings (criticalfast-xml-parservia@loaders.gl/xml, highlodash, moderateprotocol-buffers-schema) and the dev-tooling ones (vite,vitest,brace-expansion)..github/workflows/ci.yml— nothing previously ran lint/tests on pushes or PRs (only Docker publish existed).ROADMAP.md(P5-6/P5-7 repeated twice), wrote theDocumentation/pwa-guide.mdthatREADME.mdlinked to but never existed, and dropped the unuseditmlogicdependency fromrf-engine/requirements.txt(declared, never imported).viewshed_proc.py.Test plan
npm run lint— 0 errors (35 pre-existing warnings remain, out of scope for this pass — see below)npm test(vitest) — 7/7 passingnpm run build— succeedspytest rf-engine/tests/— 9/9 passing (added 2 new tests covering the per-node color/composite logic, which the prior tests never exercised since they only used all-zero grids)Not in scope for this PR
The remaining 35 lint warnings (dead vars in
LinkLayer.jsx/MapContainer.jsx/OptimizationLayer.jsx, and the "fast refresh" warnings on context files that mix hook/provider exports) are real but unrelated to what was asked here — flagging as a natural follow-up now that lint output is trustworthy again.Generated by Claude Code