Identification of pollution using satellite data #84
Conversation
initial setup
move to production
move to production
move to production
move to production
move to production
move to production
move to production
move to production
move to production
move to production
move to production
move to production
move to production
move to production
move to production
move to production
move to production
move to production
move to production
move to production
move to production
…into maplayers
…navigation for map layers
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
📝 WalkthroughWalkthroughThis update introduces new features for pollutant mapping and source pollution analysis in the frontend, adds comprehensive type definitions, and implements a backend module for pollutant identification using machine learning and geospatial analysis. It includes new map visualizations, data export features, and API integrations, along with dependency and style updates. Changes
Sequence Diagram(s)Pollutant Location Mapping FlowsequenceDiagram
participant User
participant PollutantsPage (React)
participant PollutantMap
participant API
participant Toast
User->>PollutantsPage: Draws polygon on map
PollutantsPage->>PollutantMap: Update polygon state
User->>PollutantsPage: Clicks "Submit" for suggestions
PollutantsPage->>API: POST polygon data
API-->>PollutantsPage: Returns suggested locations
PollutantsPage->>PollutantMap: Update suggested locations
PollutantsPage->>Toast: Show success/error message
User->>PollutantsPage: Clicks "Export CSV" or "Save as PNG"
PollutantsPage->>PollutantMap: Gather data or image
PollutantsPage->>User: Triggers download
PollutantsPage->>Toast: Show export status
Parish and Pollution Source VisualizationsequenceDiagram
participant User
participant SourcePollutionPage
participant ParishMap
participant GeoJSON Data
participant Turf.js
participant LandCoverData
SourcePollutionPage->>ParishMap: Render map
ParishMap->>GeoJSON Data: Load parish and pollution source polygons
ParishMap->>Turf.js: Spatially filter sources by parish
ParishMap->>LandCoverData: Fetch land cover for sources
User->>ParishMap: Clicks parish polygon
ParishMap->>ParishMap: Zoom to parish, update sidebar
ParishMap->>User: Display parish info and pollution sources
Backend Pollutant IdentificationsequenceDiagram
participant Backend
participant PredictionAndProcessing
participant TensorFlow Model
participant OSMnx
participant Earth Engine
Backend->>PredictionAndProcessing: Request pollutant identification
PredictionAndProcessing->>TensorFlow Model: Predict on TIFF image
PredictionAndProcessing->>PredictionAndProcessing: Post-process mask, extract polygons
PredictionAndProcessing->>OSMnx: Fetch road/building data
PredictionAndProcessing->>Earth Engine: Query environmental profile
PredictionAndProcessing-->>Backend: Return polygons and features
Poem
Warning Review ran into problems🔥 ProblemsErrors were encountered while retrieving linked issues. Errors (1)
✨ 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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🔭 Outside diff range comments (1)
frontend/src/lib/types.ts (1)
167-290: Remove duplicate interface definitionsAll interfaces from line 167 onwards are exact duplicates of the definitions above. This will cause TypeScript compilation errors.
Delete lines 167-290 to remove these duplicates:
SiteLocation(duplicate of lines 25-33)SiteLocatorResponse(duplicate of lines 36-38)ControlPanelProps(duplicate of lines 40-45)SiteCategoryResponse(duplicate of lines 47-62)GridOption(duplicate of lines 64-67)AirQualityReportPayload(duplicate of lines 69-73)DailyMeanData(duplicate of lines 75-81)DiurnalData(duplicate of lines 83-89)MonthlyData(duplicate of lines 91-101)AirQualityReportResponse(duplicate of lines 103-110)SatelliteDataPayload(duplicate of lines 112-116)SatelliteDataResponse(duplicate of lines 118-123)Grid(duplicate of lines 125-132)Site(duplicate of lines 134-148)
🧹 Nitpick comments (7)
frontend/src/components/navigation/navigation.tsx (1)
17-17: Clean up commented navigation item.The commented-out "Poll" navigation item should either be removed if it's no longer needed or uncommented if it's intended for future use. Leaving commented code can create confusion about the intended functionality.
Apply this diff to remove the commented line:
- //{ name: "Poll", href: "/testfetchdata" },frontend/src/components/map/ParishMap.tsx (4)
340-348: Remove unnecessary continue statement.The continue statement on line 346 is unnecessary since it's the last statement in the loop iteration.
// Validate each coordinate pair for (const coord of coords) { if (!Array.isArray(coord) || coord.length < 2 || typeof coord[0] !== 'number' || typeof coord[1] !== 'number') { console.warn(`Invalid coordinate in parish ${parish.properties.NAME_4}:`, coord); counts[parish.properties.NAME_4] = 0; - continue; + break; } }
580-597: Simplify with optional chaining.The conditional checks can be simplified using optional chaining for better readability.
async function loadAllLandCover() { const newData: Record<string, { name: string; value: number }[]> = {}; for (const source of selectedParishPollutionSources) { const { latitude, longitude } = source.properties; const entry = await fetchLandCoverForSource(latitude, longitude); - if (entry && entry.landcover_summary) { + if (entry?.landcover_summary) { newData[source._id.$oid] = computeLandCoverPercentagesFromSummary(entry.landcover_summary); - } else if (entry && entry.buildings) { + } else if (entry?.buildings) { newData[source._id.$oid] = computeLandCoverPercentages(entry.buildings); } else { newData[source._id.$oid] = []; } } if (!cancelled) setLandCoverDataBySource(newData); }
610-622: Simplify bounds checking with optional chaining.The bounds validation can be simplified using optional chaining.
// Zoom to the parish with proper null checks if (mapRef.current && layer && layer.getBounds) { try { const bounds = layer.getBounds(); - if (bounds && bounds.isValid && bounds.isValid()) { + if (bounds?.isValid?.()) { mapRef.current.fitBounds(bounds); } } catch (e) { console.warn('Error fitting bounds:', e); } }
988-995: Simplify with optional chaining.The nested property access can be simplified using optional chaining.
{/* Parishes - Always show */} - {parishes && parishes.features && parishes.features.length > 0 && ( + {parishes?.features?.length > 0 && ( <GeoJSON data={parishes} style={parishStyle} onEachFeature={onEachParish} /> )}frontend/src/components/map/PollutantMap.tsx (1)
403-418: Use optional chaining for cleaner codeThe conditional rendering can be simplified using optional chaining.
- {pollutionData && - pollutionData.map((data, index) => ( + {pollutionData?.map((data, index) => ( <Marker key={index} position={[data.latitude, data.longitude]}> <Popup> <div> <h3>Pollution Data</h3> <p>Latitude: {data.latitude.toFixed(4)}</p> <p>Longitude: {data.longitude.toFixed(4)}</p> <p>Confidence Score: {data.confidence_score.toFixed(4)}</p> <p> PM2.5 Prediction: {data.pm2_5_prediction?.toFixed(2) || "N/A"} </p> </div> </Popup> </Marker> ))}pollution_prediction/models/pollutant_identification.py (1)
50-59: Avoid loading model at module import timeLoading the ML model during module import can cause issues with testing, deployment, and memory usage. Consider lazy loading or explicit initialization.
Move the model loading into the class as a class method or property:
class PredictionAndProcessing: _model = None @classmethod def get_model(cls): if cls._model is None: h5file = get_trained_model_from_gcs( Config.GOOGLE_CLOUD_PROJECT_ID, Config.PROJECT_BUCKET, "paperunetmodel100-031-0.440052-0.500628.h5", ) cls._model = tf.keras.models.load_model(h5file, compile=False) return cls._modelThen update line 95 to use
cls.get_model().predict(...)instead ofmodel.predict(...).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
frontend/package-lock.jsonis excluded by!**/package-lock.jsonpackage-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (13)
frontend/package.json(2 hunks)frontend/postcss.config.mjs(1 hunks)frontend/public/mapdata/parishes.geojson(1 hunks)frontend/src/app/pollutants/page.tsx(1 hunks)frontend/src/app/sourcepollution/page.tsx(1 hunks)frontend/src/components/Controls/PollutantControlPanel.tsx(1 hunks)frontend/src/components/map/ParishMap.tsx(1 hunks)frontend/src/components/map/PollutantMap.tsx(1 hunks)frontend/src/components/navigation/navigation.tsx(1 hunks)frontend/src/lib/types.ts(1 hunks)frontend/src/services/apiService.tsx(2 hunks)frontend/src/styles/globals.css(1 hunks)pollution_prediction/models/pollutant_identification.py(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (3)
frontend/src/app/sourcepollution/page.tsx (2)
frontend/src/components/map/ParishMap.tsx (1)
SourcePollutionPage(214-1089)frontend/src/components/navigation/navigation.tsx (1)
Navigation(21-77)
frontend/src/services/apiService.tsx (1)
frontend/src/lib/types.ts (1)
PollutionProperties(150-164)
frontend/src/components/map/PollutantMap.tsx (4)
frontend/src/lib/types.ts (2)
PollutionProperties(150-164)Location(1-4)frontend/src/ui/popover.tsx (3)
Popover(31-31)PopoverTrigger(31-31)PopoverContent(31-31)frontend/src/ui/button.tsx (1)
Button(56-56)frontend/src/services/apiService.tsx (2)
fetchPollutionData(143-161)getSatelliteData(82-93)
🪛 Ruff (0.11.9)
pollution_prediction/models/pollutant_identification.py
2-2: os imported but unused
Remove unused import: os
(F401)
3-3: time imported but unused
Remove unused import: time
(F401)
8-8: json imported but unused
Remove unused import: json
(F401)
12-12: pandas imported but unused
Remove unused import: pandas
(F401)
19-19: shapely.geometry.mapping imported but unused
Remove unused import: shapely.geometry.mapping
(F401)
24-24: geemap imported but unused
Remove unused import: geemap
(F401)
31-31: matplotlib.patches.Polygon imported but unused
Remove unused import: matplotlib.patches.Polygon
(F401)
36-36: pymongo.MongoClient imported but unused
Remove unused import: pymongo.MongoClient
(F401)
37-37: bson.json_util imported but unused
Remove unused import: bson.json_util
(F401)
64-64: Undefined name service_account
(F821)
85-85: Undefined name load_tiff
(F821)
90-90: Undefined name normalize_image
(F821)
187-187: Local variable e is assigned to but never used
Remove assignment to unused variable e
(F841)
190-190: Local variable building_types is assigned to but never used
Remove assignment to unused variable building_types
(F841)
🪛 Biome (1.9.4)
frontend/src/components/map/PollutantMap.tsx
[error] 403-419: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
frontend/src/components/map/ParishMap.tsx
[error] 346-346: Unnecessary continue statement
Unsafe fix: Delete the unnecessary continue statement
(lint/correctness/noUnnecessaryContinue)
[error] 585-585: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 587-587: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 613-613: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 989-989: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🔇 Additional comments (15)
frontend/src/styles/globals.css (2)
1-1: LGTM! Proper integration of Leaflet CSS.Adding the Leaflet CSS import at the beginning ensures the map components will render correctly with default styling before custom overrides are applied.
25-286: Comprehensive and well-structured map styling.The extensive CSS styling for map components is well-organized and follows good practices:
- Proper use of
!importantdeclarations to override Leaflet defaults- Modern CSS features like custom properties and transitions
- Appropriate z-index management for layering
- Clean organization with descriptive comments
The styling covers all necessary map components including popups, search controls, legends, and interactive elements.
frontend/package.json (2)
22-22: Appropriate geospatial library addition.Adding
@turf/turfis a good choice for geospatial operations. This library is well-maintained and widely used for spatial filtering and geometric computations that align with the parish-based pollution source filtering described in the AI summary.
50-50: Standard CSS processing tool addition.Adding
autoprefixeras a dev dependency is a standard practice for ensuring CSS compatibility across different browsers, especially when using modern CSS features.frontend/postcss.config.mjs (1)
4-4: Clean PostCSS configuration update.Adding the autoprefixer plugin alongside tailwindcss is a standard and recommended setup for modern CSS processing with vendor prefixing.
frontend/public/mapdata/parishes.geojson (1)
1-56: Valid GeoJSON structure with potential placeholder data.The GeoJSON file follows the correct specification and contains valid polygon features for the three parishes. However, the parish boundaries are simple rectangles, which suggests this might be placeholder data rather than actual administrative boundaries.
Consider verifying that these coordinates represent the actual parish boundaries rather than simplified rectangular approximations.
frontend/src/components/navigation/navigation.tsx (1)
16-16: Good addition of pollution-related navigation.The new "Pollutants" navigation item appropriately connects to the source pollution page, aligning well with the PR's objectives for pollution visualization functionality.
frontend/src/app/sourcepollution/page.tsx (1)
1-13: Clean and well-structured page component.The implementation follows Next.js conventions properly with client-side rendering for interactive map components. The structure is simple and effective.
frontend/src/services/apiService.tsx (2)
64-78: Interface definition looks good.The
PollutionPropertiesinterface is well-structured and matches the corresponding interface in the types file. The property types are appropriate for the pollution data model.
143-161: API function follows established patterns.The
fetchPollutionDatafunction is consistent with other API functions in the file, includes proper error handling, and validates the response structure before processing.frontend/src/app/pollutants/page.tsx (3)
15-18: Proper dynamic import configuration.The dynamic import with SSR disabled and loading component is the correct approach for map components that rely on browser APIs.
61-101: Well-implemented CSV export with duplicate filtering.The export logic properly handles empty data states and includes duplicate filtering based on coordinates. The unique location tracking is a good approach.
103-117: Map screenshot functionality using html2canvas.The implementation correctly targets the Leaflet container and provides user feedback through toast notifications.
frontend/src/components/map/PollutantMap.tsx (1)
209-211: Prevent division by zero in average calculationWhen
pollutionDatais empty, this will divide by zero resulting inNaN.Add a guard clause:
- const average_PM25 = - pollutionData.reduce((sum, data) => sum + (data.pm2_5_prediction || 0), 0) / - (pollutionData.length || 1); + const average_PM25 = pollutionData.length > 0 + ? pollutionData.reduce((sum, data) => sum + (data.pm2_5_prediction || 0), 0) / pollutionData.length + : 0;Likely an incorrect or invalid review comment.
frontend/src/lib/types.ts (1)
18-23: Good structure for site statisticsThe modified
SiteInformationinterface properly structures category counts and total sites for better data organization.
| return ( | ||
| <div className="control-panel space-y-4 mt-9" style={{ width: "400px" }}> | ||
| {/* Search Bar */} | ||
| <SearchBar onSearch={() => {}} onBoundaryFound={onBoundaryFound} /> | ||
|
|
||
| </div> | ||
| ); | ||
| } |
There was a problem hiding this comment.
Incomplete component implementation.
The component contains extensive logic for form validation, state management, and submission handling, but the JSX only renders a SearchBar. The managed state variables (minDistance, numSensors, newLat, newLng) and the handleAddLocation function suggest missing form inputs and buttons.
Consider completing the component by adding the missing form elements:
return (
<div className="control-panel space-y-4 mt-9" style={{ width: "400px" }}>
{/* Search Bar */}
<SearchBar onSearch={() => {}} onBoundaryFound={onBoundaryFound} />
+
+ {/* Form inputs for min distance, number of sensors, and location inputs */}
+ <div className="space-y-3">
+ <Input
+ type="number"
+ placeholder="Min Distance (km)"
+ value={minDistance}
+ onChange={(e) => setMinDistance(e.target.value)}
+ />
+ <Input
+ type="number"
+ placeholder="Number of Sensors"
+ value={numSensors}
+ onChange={(e) => setNumSensors(e.target.value)}
+ />
+ <div className="flex gap-2">
+ <Input
+ type="number"
+ placeholder="Latitude"
+ value={newLat}
+ onChange={(e) => setNewLat(e.target.value)}
+ />
+ <Input
+ type="number"
+ placeholder="Longitude"
+ value={newLng}
+ onChange={(e) => setNewLng(e.target.value)}
+ />
+ <Button onClick={handleAddLocation}>Add</Button>
+ </div>
+ <Button onClick={handleSubmit} disabled={isLoading}>
+ {isLoading ? <Loader2 className="animate-spin" /> : "Submit"}
+ </Button>
+ </div>
</div>
);📝 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.
| return ( | |
| <div className="control-panel space-y-4 mt-9" style={{ width: "400px" }}> | |
| {/* Search Bar */} | |
| <SearchBar onSearch={() => {}} onBoundaryFound={onBoundaryFound} /> | |
| </div> | |
| ); | |
| } | |
| return ( | |
| <div className="control-panel space-y-4 mt-9" style={{ width: "400px" }}> | |
| {/* Search Bar */} | |
| <SearchBar onSearch={() => {}} onBoundaryFound={onBoundaryFound} /> | |
| {/* Form inputs for min distance, number of sensors, and location inputs */} | |
| <div className="space-y-3"> | |
| <Input | |
| type="number" | |
| placeholder="Min Distance (km)" | |
| value={minDistance} | |
| onChange={(e) => setMinDistance(e.target.value)} | |
| /> | |
| <Input | |
| type="number" | |
| placeholder="Number of Sensors" | |
| value={numSensors} | |
| onChange={(e) => setNumSensors(e.target.value)} | |
| /> | |
| <div className="flex gap-2"> | |
| <Input | |
| type="number" | |
| placeholder="Latitude" | |
| value={newLat} | |
| onChange={(e) => setNewLat(e.target.value)} | |
| /> | |
| <Input | |
| type="number" | |
| placeholder="Longitude" | |
| value={newLng} | |
| onChange={(e) => setNewLng(e.target.value)} | |
| /> | |
| <Button onClick={handleAddLocation}>Add</Button> | |
| </div> | |
| <Button onClick={handleSubmit} disabled={isLoading}> | |
| {isLoading ? <Loader2 className="animate-spin" /> : "Submit"} | |
| </Button> | |
| </div> | |
| </div> | |
| ); | |
| } |
🤖 Prompt for AI Agents
In frontend/src/components/Controls/PollutantControlPanel.tsx around lines 130
to 137, the component only renders a SearchBar but is missing form inputs and
buttons for managing state variables like minDistance, numSensors, newLat,
newLng, and the handleAddLocation function. To fix this, add the appropriate
form elements such as input fields for these state variables and a button to
trigger handleAddLocation, ensuring the component fully implements its intended
form validation, state management, and submission handling.
| export default function SourcePollutionPage() { | ||
| const [parishes, setParishes] = useState<FeatureCollection<Polygon, ParishProperties> | null>(null); | ||
| const [pollutionSources, setPollutionSources] = useState<CombinedDataFeature[]>([]); | ||
| const [selectedParish, setSelectedParish] = useState<ParishFeature | null>(null); | ||
| const [loadingState, setLoadingState] = useState<LoadingState>({ | ||
| parishes: true, | ||
| pollutionSources: false, | ||
| }); | ||
| const [errorState, setErrorState] = useState<ErrorState>({ | ||
| parishes: null, | ||
| pollutionSources: null, | ||
| }); | ||
| const [mapZoom, setMapZoom] = useState(10); | ||
| const [showPollutionSources, setShowPollutionSources] = useState(true); | ||
| const mapRef = useRef<any>(null); | ||
| const [landCoverDataBySource, setLandCoverDataBySource] = useState<Record<string, { name: string; value: number }[]>>({}); | ||
|
|
||
| // Load parishes data first (essential for map to work) | ||
| useEffect(() => { | ||
| async function fetchParishes() { | ||
| try { | ||
| setLoadingState(prev => ({ ...prev, parishes: true })); | ||
| setErrorState(prev => ({ ...prev, parishes: null })); | ||
|
|
||
| const response = await fetch(`${DATA_PATH}/jinja_parishes.json`); | ||
| if (!response.ok) { | ||
| throw new Error(`Failed to load parishes: ${response.status}`); | ||
| } | ||
|
|
||
| const parishesData = await response.json(); | ||
| setParishes(parishesData); | ||
| setLoadingState(prev => ({ ...prev, parishes: false })); | ||
| } catch (err: any) { | ||
| console.error('Error loading parishes:', err); | ||
| setErrorState(prev => ({ ...prev, parishes: err.message || "Failed to load parishes" })); | ||
| setLoadingState(prev => ({ ...prev, parishes: false })); | ||
| } | ||
| } | ||
|
|
||
| fetchParishes(); | ||
| }, []); | ||
|
|
||
| // Load pollution sources data | ||
| useEffect(() => { | ||
| const loadPollutionSources = async () => { | ||
| setLoadingState(prev => ({ ...prev, pollutionSources: true })); | ||
| setErrorState(prev => ({ ...prev, pollutionSources: null })); | ||
|
|
||
| try { | ||
| console.log('Loading pollution sources from combined_data_t.json...'); | ||
| const response = await fetch('/mapdata/combined_data_t.json'); | ||
|
|
||
| if (!response.ok) { | ||
| throw new Error(`HTTP error! status: ${response.status}`); | ||
| } | ||
|
|
||
| const data = await response.json(); | ||
| console.log('Pollution sources data loaded:', { | ||
| dataType: typeof data, | ||
| isArray: Array.isArray(data), | ||
| length: Array.isArray(data) ? data.length : 'not an array' | ||
| }); | ||
|
|
||
| if (!Array.isArray(data)) { | ||
| throw new Error('Pollution sources data is not an array'); | ||
| } | ||
|
|
||
| // Validate the data structure | ||
| const validSources = data.filter((item: any) => { | ||
| return item && | ||
| item.type === 'Feature' && | ||
| item.geometry && | ||
| item.geometry.type === 'Polygon' && | ||
| item.geometry.coordinates && | ||
| Array.isArray(item.geometry.coordinates) && | ||
| item.properties; | ||
| }); | ||
|
|
||
| console.log('Valid pollution sources:', { | ||
| total: data.length, | ||
| valid: validSources.length, | ||
| invalid: data.length - validSources.length | ||
| }); | ||
|
|
||
| setPollutionSources(validSources); | ||
| } catch (error) { | ||
| console.error('Error loading pollution sources:', error); | ||
| setErrorState(prev => ({ ...prev, pollutionSources: error instanceof Error ? error.message : 'Failed to load pollution sources' })); | ||
| } finally { | ||
| setLoadingState(prev => ({ ...prev, pollutionSources: false })); | ||
| } | ||
| }; | ||
|
|
||
| loadPollutionSources(); | ||
| }, []); | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider breaking down this large component.
This component is quite large (1089 lines) and handles multiple responsibilities. Consider extracting some functionality into smaller, focused components or custom hooks.
Some suggestions:
- Extract the data fetching logic into custom hooks (
useParishData,usePollutionSources) - Create separate components for the sidebar content (
ParishSidebar,PollutionSourceList) - Move utility functions to a separate module
- Extract the map rendering logic into a separate component
This would improve maintainability and make the component easier to test and understand.
🤖 Prompt for AI Agents
In frontend/src/components/map/ParishMap.tsx around lines 214 to 309, the
SourcePollutionPage component is too large and handles multiple responsibilities
including data fetching, state management, and rendering. To fix this, extract
the parish data fetching logic into a custom hook named useParishData and the
pollution sources fetching logic into another hook named usePollutionSources.
Also, create separate components for sidebar content like ParishSidebar and
PollutionSourceList, move utility functions to a separate module, and isolate
the map rendering logic into its own component. This modularization will improve
maintainability, readability, and testability.
| item, | ||
| error | ||
| ); | ||
| return { ...item, pm2_5_prediction: null }; // Handle error: set PM2.5 to null |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Maintain consistency with optional type definition
The type definition uses pm2_5_prediction?: number which means undefined when missing, but error handling sets it to null.
- return { ...item, pm2_5_prediction: null }; // Handle error: set PM2.5 to null
+ return { ...item, pm2_5_prediction: undefined }; // Handle error: set PM2.5 to undefined📝 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.
| return { ...item, pm2_5_prediction: null }; // Handle error: set PM2.5 to null | |
| return { ...item, pm2_5_prediction: undefined }; // Handle error: set PM2.5 to undefined |
🤖 Prompt for AI Agents
In frontend/src/components/map/PollutantMap.tsx at line 363, the error handling
sets pm2_5_prediction to null, but the type definition uses an optional number
which implies undefined when missing. To maintain consistency, change the
assignment from null to undefined in the error handling code.
| const fetchPollutionData = async (): Promise< | ||
| PollutionProperties[] | null | ||
| > => { | ||
| try { | ||
| const response = await fetch( | ||
| "http://localhost:5001/api/v2/spatial/get-all-data" | ||
| ); | ||
| if (!response.ok) | ||
| throw new Error(`HTTP error! status: ${response.status}`); | ||
| const data = await response.json(); | ||
|
|
||
| if (!isPollutionFeatureCollection(data)) { | ||
| console.error("Invalid data format from API:", data); | ||
| return null; | ||
| } | ||
|
|
||
| return data.features.map((feature) => feature.properties); | ||
| } catch (error) { | ||
| console.error("Failed to fetch pollution data:", error); | ||
| return null; | ||
| } | ||
| }; |
There was a problem hiding this comment.
Use existing API service instead of duplicating functionality
This function duplicates fetchPollutionData from frontend/src/services/apiService.tsx and uses a hardcoded localhost URL that won't work in production.
Remove this function and import the existing one:
-const fetchPollutionData = async (): Promise<
- PollutionProperties[] | null
-> => {
- try {
- const response = await fetch(
- "http://localhost:5001/api/v2/spatial/get-all-data"
- );
- if (!response.ok)
- throw new Error(`HTTP error! status: ${response.status}`);
- const data = await response.json();
-
- if (!isPollutionFeatureCollection(data)) {
- console.error("Invalid data format from API:", data);
- return null;
- }
-
- return data.features.map((feature) => feature.properties);
- } catch (error) {
- console.error("Failed to fetch pollution data:", error);
- return null;
- }
-};
+import { fetchPollutionData } from "@/services/apiService";Note: You may need to adjust the response handling since the service function already extracts properties from features.
📝 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 fetchPollutionData = async (): Promise< | |
| PollutionProperties[] | null | |
| > => { | |
| try { | |
| const response = await fetch( | |
| "http://localhost:5001/api/v2/spatial/get-all-data" | |
| ); | |
| if (!response.ok) | |
| throw new Error(`HTTP error! status: ${response.status}`); | |
| const data = await response.json(); | |
| if (!isPollutionFeatureCollection(data)) { | |
| console.error("Invalid data format from API:", data); | |
| return null; | |
| } | |
| return data.features.map((feature) => feature.properties); | |
| } catch (error) { | |
| console.error("Failed to fetch pollution data:", error); | |
| return null; | |
| } | |
| }; | |
| // --- remove lines 170–191 (the local fetchPollutionData definition) --- | |
| import { fetchPollutionData } from "@/services/apiService"; |
🤖 Prompt for AI Agents
In frontend/src/components/map/PollutantMap.tsx between lines 170 and 191,
remove the local fetchPollutionData function that duplicates the one in
frontend/src/services/apiService.tsx and uses a hardcoded localhost URL.
Instead, import the existing fetchPollutionData function from the apiService
module. Adjust the code to use the imported function directly, noting that it
already returns the extracted properties, so you should remove any redundant
data extraction or mapping logic.
| interface PollutionProperties { | ||
| latitude: number; | ||
| longitude: number; | ||
| confidence_score: number; | ||
| timestamp: string; | ||
| mean_AOD: number; | ||
| mean_CO: number; | ||
| mean_NO2: number; | ||
| pm2_5_prediction?: number; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove duplicate interface definition
The PollutionProperties interface is already defined in frontend/src/lib/types.ts (lines 150-164) with additional fields. This local definition creates inconsistency and maintenance issues.
Import the interface from the types file instead:
-interface PollutionProperties {
- latitude: number;
- longitude: number;
- confidence_score: number;
- timestamp: string;
- mean_AOD: number;
- mean_CO: number;
- mean_NO2: number;
- pm2_5_prediction?: number;
-}
+import type { Location, PollutionProperties as BasePollutionProperties } from "@/lib/types";
+
+// Extend the base type if you need the pm2_5_prediction field
+interface PollutionProperties extends BasePollutionProperties {
+ pm2_5_prediction?: number;
+}📝 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.
| interface PollutionProperties { | |
| latitude: number; | |
| longitude: number; | |
| confidence_score: number; | |
| timestamp: string; | |
| mean_AOD: number; | |
| mean_CO: number; | |
| mean_NO2: number; | |
| pm2_5_prediction?: number; | |
| } | |
| import type { Location, PollutionProperties as BasePollutionProperties } from "@/lib/types"; | |
| // Extend the base type if you need the pm2_5_prediction field | |
| interface PollutionProperties extends BasePollutionProperties { | |
| pm2_5_prediction?: number; | |
| } |
🤖 Prompt for AI Agents
In frontend/src/components/map/PollutantMap.tsx around lines 30 to 39, remove
the local definition of the PollutionProperties interface and instead import it
from frontend/src/lib/types.ts where it is already defined with additional
fields. This will prevent duplication and ensure consistency across the
codebase.
| @staticmethod | ||
| def initialize_earth_engine(): | ||
| ee.Initialize(credentials=service_account.Credentials.from_service_account_file( | ||
| Config.CREDENTIALS, | ||
| scopes=['https://www.googleapis.com/auth/earthengine'] | ||
| ), project=Config.GOOGLE_CLOUD_PROJECT_ID) | ||
| # Function to load TIFF file |
There was a problem hiding this comment.
Fix class structure and method placement
The @staticmethod decorator and method are incorrectly placed outside the class definition.
-@staticmethod
-def initialize_earth_engine():
- ee.Initialize(credentials=service_account.Credentials.from_service_account_file(
- Config.CREDENTIALS,
- scopes=['https://www.googleapis.com/auth/earthengine']
- ), project=Config.GOOGLE_CLOUD_PROJECT_ID)
-# Function to load TIFF file
class PredictionAndProcessing:
+ @staticmethod
+ def initialize_earth_engine():
+ ee.Initialize(credentials=service_account.Credentials.from_service_account_file(
+ Config.CREDENTIALS,
+ scopes=['https://www.googleapis.com/auth/earthengine']
+ ), project=Config.GOOGLE_CLOUD_PROJECT_ID)
+ 📝 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.
| @staticmethod | |
| def initialize_earth_engine(): | |
| ee.Initialize(credentials=service_account.Credentials.from_service_account_file( | |
| Config.CREDENTIALS, | |
| scopes=['https://www.googleapis.com/auth/earthengine'] | |
| ), project=Config.GOOGLE_CLOUD_PROJECT_ID) | |
| # Function to load TIFF file | |
| class PredictionAndProcessing: | |
| @staticmethod | |
| def initialize_earth_engine(): | |
| ee.Initialize( | |
| credentials=service_account.Credentials.from_service_account_file( | |
| Config.CREDENTIALS, | |
| scopes=['https://www.googleapis.com/auth/earthengine'] | |
| ), | |
| project=Config.GOOGLE_CLOUD_PROJECT_ID | |
| ) | |
| # Function to load TIFF file | |
| ... |
🧰 Tools
🪛 Ruff (0.11.9)
64-64: Undefined name service_account
(F821)
🤖 Prompt for AI Agents
In pollution_prediction/models/pollutant_identification.py around lines 62 to
68, the initialize_earth_engine method is defined with @staticmethod outside of
any class. Move this method inside the appropriate class definition and ensure
the @staticmethod decorator is directly above the method within the class body
to fix the class structure and method placement.
| @staticmethod | ||
| def preprocess_image(image_path): | ||
| image, image_profile = load_tiff(image_path) | ||
| # Check if the image has 4 channels (e.g., RGBA) | ||
| if image.shape[-1] == 4: | ||
| # Discard the alpha channel (keep only the first three channels: RGB) | ||
| image = image[:, :, :3] | ||
| image = normalize_image(image) | ||
| return image, image_profile |
There was a problem hiding this comment.
Fix undefined function references
The method calls load_tiff and normalize_image without the class prefix.
@staticmethod
def preprocess_image(image_path):
- image, image_profile = load_tiff(image_path)
+ image, image_profile = PredictionAndProcessing.load_tiff(image_path)
# Check if the image has 4 channels (e.g., RGBA)
if image.shape[-1] == 4:
# Discard the alpha channel (keep only the first three channels: RGB)
image = image[:, :, :3]
- image = normalize_image(image)
+ image = PredictionAndProcessing.normalize_image(image)
return image, image_profile📝 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.
| @staticmethod | |
| def preprocess_image(image_path): | |
| image, image_profile = load_tiff(image_path) | |
| # Check if the image has 4 channels (e.g., RGBA) | |
| if image.shape[-1] == 4: | |
| # Discard the alpha channel (keep only the first three channels: RGB) | |
| image = image[:, :, :3] | |
| image = normalize_image(image) | |
| return image, image_profile | |
| @staticmethod | |
| def preprocess_image(image_path): | |
| image, image_profile = PredictionAndProcessing.load_tiff(image_path) | |
| # Check if the image has 4 channels (e.g., RGBA) | |
| if image.shape[-1] == 4: | |
| # Discard the alpha channel (keep only the first three channels: RGB) | |
| image = image[:, :, :3] | |
| image = PredictionAndProcessing.normalize_image(image) | |
| return image, image_profile |
🧰 Tools
🪛 Ruff (0.11.9)
85-85: Undefined name load_tiff
(F821)
90-90: Undefined name normalize_image
(F821)
🤖 Prompt for AI Agents
In pollution_prediction/models/pollutant_identification.py around lines 83 to
91, the static method preprocess_image calls load_tiff and normalize_image
without referencing the class. Fix this by prefixing these calls with the class
name or using self if appropriate, ensuring the methods are correctly referenced
to avoid undefined function errors.
| building_types = buildings['building'].unique() if 'building' in buildings.columns else [] | ||
| except Exception as e: | ||
| number_of_buildings = 'Error' | ||
| building_density = 'Error' | ||
| building_types = 'Error' |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove unused variable assignments
Variables e and building_types are assigned but never used.
- except Exception as e:
+ except Exception:
number_of_buildings = 'Error'
building_density = 'Error'
- building_types = 'Error'📝 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.
| building_types = buildings['building'].unique() if 'building' in buildings.columns else [] | |
| except Exception as e: | |
| number_of_buildings = 'Error' | |
| building_density = 'Error' | |
| building_types = 'Error' | |
| building_types = buildings['building'].unique() if 'building' in buildings.columns else [] | |
| except Exception: | |
| number_of_buildings = 'Error' | |
| building_density = 'Error' |
🧰 Tools
🪛 Ruff (0.11.9)
187-187: Local variable e is assigned to but never used
Remove assignment to unused variable e
(F841)
190-190: Local variable building_types is assigned to but never used
Remove assignment to unused variable building_types
(F841)
🤖 Prompt for AI Agents
In pollution_prediction/models/pollutant_identification.py around lines 186 to
190, the variable 'e' in the except block and the assignment to 'building_types'
are unused. Remove the variable 'e' from the except clause and eliminate the
assignment to 'building_types' to clean up unused variables.
| # Standard Libraries | ||
| import os | ||
| import time | ||
| import math | ||
| import datetime | ||
| import warnings | ||
| from collections import defaultdict | ||
| import json | ||
|
|
||
| # Data Processing | ||
| import numpy as np | ||
| import pandas as pd | ||
| import geopandas as gpd | ||
|
|
||
| # Geospatial and Mapping | ||
| import rasterio | ||
| from rasterio.features import shapes, geometry_mask | ||
| from rasterio.plot import reshape_as_image | ||
| from shapely.geometry import shape, Point, mapping | ||
| from shapely.ops import transform | ||
| from pyproj import Transformer, CRS | ||
| import osmnx as ox | ||
| import ee | ||
| import geemap | ||
|
|
||
| # Image Processing and Machine Learning | ||
| import tensorflow as tf | ||
| from skimage.morphology import remove_small_objects, binary_closing, disk | ||
|
|
||
| # Visualization | ||
| from matplotlib.patches import Polygon as pltPolygon | ||
|
|
||
|
|
||
|
|
||
| # MongoDB | ||
| from pymongo import MongoClient | ||
| from bson import json_util | ||
|
|
||
| from configure import get_trained_model_from_gcs, Config | ||
|
|
||
| # Configure Warnings | ||
| warnings.filterwarnings('ignore') | ||
|
|
||
|
|
||
| # Ignore warnings | ||
| warnings.filterwarnings('ignore') | ||
| print(f"GOOGLE_CLOUD_PROJECT_ID: {Config.GOOGLE_CLOUD_PROJECT_ID}") |
There was a problem hiding this comment.
Remove unused imports and fix duplicate warning configuration
Multiple imports are unused and warnings are filtered twice.
-# Standard Libraries
-import os
-import time
-import datetime
-import json
-# Data Processing
import numpy as np
-import pandas as pd
import geopandas as gpd
# Geospatial and Mapping
import rasterio
from rasterio.features import shapes, geometry_mask
from rasterio.plot import reshape_as_image
-from shapely.geometry import shape, Point, mapping
+from shapely.geometry import shape, Point
from shapely.ops import transform
from pyproj import Transformer, CRS
import osmnx as ox
import ee
-import geemap
# Image Processing and Machine Learning
import tensorflow as tf
from skimage.morphology import remove_small_objects, binary_closing, disk
-# Visualization
-from matplotlib.patches import Polygon as pltPolygon
-
-
-
-# MongoDB
-from pymongo import MongoClient
-from bson import json_util
+# Authentication
+from google.oauth2 import service_account
from configure import get_trained_model_from_gcs, Config
# Configure Warnings
warnings.filterwarnings('ignore')
-
-
-# Ignore warnings
-warnings.filterwarnings('ignore')📝 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.
| # Standard Libraries | |
| import os | |
| import time | |
| import math | |
| import datetime | |
| import warnings | |
| from collections import defaultdict | |
| import json | |
| # Data Processing | |
| import numpy as np | |
| import pandas as pd | |
| import geopandas as gpd | |
| # Geospatial and Mapping | |
| import rasterio | |
| from rasterio.features import shapes, geometry_mask | |
| from rasterio.plot import reshape_as_image | |
| from shapely.geometry import shape, Point, mapping | |
| from shapely.ops import transform | |
| from pyproj import Transformer, CRS | |
| import osmnx as ox | |
| import ee | |
| import geemap | |
| # Image Processing and Machine Learning | |
| import tensorflow as tf | |
| from skimage.morphology import remove_small_objects, binary_closing, disk | |
| # Visualization | |
| from matplotlib.patches import Polygon as pltPolygon | |
| # MongoDB | |
| from pymongo import MongoClient | |
| from bson import json_util | |
| from configure import get_trained_model_from_gcs, Config | |
| # Configure Warnings | |
| warnings.filterwarnings('ignore') | |
| # Ignore warnings | |
| warnings.filterwarnings('ignore') | |
| print(f"GOOGLE_CLOUD_PROJECT_ID: {Config.GOOGLE_CLOUD_PROJECT_ID}") | |
| import math | |
| import warnings | |
| from collections import defaultdict | |
| import numpy as np | |
| import geopandas as gpd | |
| # Geospatial and Mapping | |
| import rasterio | |
| from rasterio.features import shapes, geometry_mask | |
| from rasterio.plot import reshape_as_image | |
| from shapely.geometry import shape, Point | |
| from shapely.ops import transform | |
| from pyproj import Transformer, CRS | |
| import osmnx as ox | |
| import ee | |
| # Image Processing and Machine Learning | |
| import tensorflow as tf | |
| from skimage.morphology import remove_small_objects, binary_closing, disk | |
| # Authentication | |
| from google.oauth2 import service_account | |
| from configure import get_trained_model_from_gcs, Config | |
| # Configure Warnings | |
| warnings.filterwarnings('ignore') | |
| print(f"GOOGLE_CLOUD_PROJECT_ID: {Config.GOOGLE_CLOUD_PROJECT_ID}") |
🧰 Tools
🪛 Ruff (0.11.9)
2-2: os imported but unused
Remove unused import: os
(F401)
3-3: time imported but unused
Remove unused import: time
(F401)
8-8: json imported but unused
Remove unused import: json
(F401)
12-12: pandas imported but unused
Remove unused import: pandas
(F401)
19-19: shapely.geometry.mapping imported but unused
Remove unused import: shapely.geometry.mapping
(F401)
24-24: geemap imported but unused
Remove unused import: geemap
(F401)
31-31: matplotlib.patches.Polygon imported but unused
Remove unused import: matplotlib.patches.Polygon
(F401)
36-36: pymongo.MongoClient imported but unused
Remove unused import: pymongo.MongoClient
(F401)
37-37: bson.json_util imported but unused
Remove unused import: bson.json_util
(F401)
🤖 Prompt for AI Agents
In pollution_prediction/models/pollutant_identification.py lines 1 to 47, remove
all imports that are not used anywhere in the file to clean up the code and
avoid unnecessary dependencies. Also, eliminate the duplicate
warnings.filterwarnings('ignore') call so that warnings are only filtered once.
This will make the import section cleaner and prevent redundant code.
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (1)
frontend/src/components/map/ParishMap.tsx (1)
232-1111: Break down this extremely large component.This component is 1112 lines and handles multiple responsibilities including data fetching, spatial operations, state management, and complex rendering logic. This makes it difficult to maintain, test, and understand.
Priority refactoring suggestions:
- Extract data fetching into custom hooks (
useParishData,usePollutionSources)- Create separate components (
ParishSidebar,PollutionSourceList,MapLegend,LayerControls)- Move spatial operations to
utils/spatialOperations.ts- Extract the massive pollution source filtering logic into utility functions
- Create a
MapContainerwrapper componentThis refactoring would improve maintainability, enable better testing, and reduce cognitive load.
🧹 Nitpick comments (4)
frontend/src/components/map/ParishMap.client.tsx (2)
298-298: Remove unnecessary continue statement.The continue statement after setting the count to 0 is unnecessary since it's at the end of the loop iteration.
- continue;
364-364: Use optional chaining for cleaner code.The bounds check can be simplified using optional chaining.
- if (bounds && bounds.isValid && bounds.isValid()) { + if (bounds?.isValid?.()) {frontend/src/components/map/ParishMap.tsx (2)
365-365: Remove unnecessary continue statement.The continue statement is unnecessary and should be removed.
- continue;
632-632: Use optional chaining for bounds validation.Simplify the bounds check using optional chaining.
- if (bounds && bounds.isValid && bounds.isValid()) { + if (bounds?.isValid?.()) {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
frontend/src/app/sourcepollution/page.tsx(1 hunks)frontend/src/components/map/ParishMap.client.tsx(1 hunks)frontend/src/components/map/ParishMap.tsx(1 hunks)frontend/src/components/map/ParishMapClient.tsx(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- frontend/src/components/map/ParishMapClient.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- frontend/src/app/sourcepollution/page.tsx
🧰 Additional context used
🪛 Biome (1.9.4)
frontend/src/components/map/ParishMap.tsx
[error] 365-365: Unnecessary continue statement
Unsafe fix: Delete the unnecessary continue statement
(lint/correctness/noUnnecessaryContinue)
[error] 604-604: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 606-606: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 632-632: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 1008-1008: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
frontend/src/components/map/ParishMap.client.tsx
[error] 298-298: Unnecessary continue statement
Unsafe fix: Delete the unnecessary continue statement
(lint/correctness/noUnnecessaryContinue)
[error] 364-364: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 507-507: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 545-545: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 594-594: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🔇 Additional comments (3)
frontend/src/components/map/ParishMap.client.tsx (1)
61-87: Well-implemented data fetching with proper error handling.The land cover data fetching function properly handles exact matches, fallback to proximity search, and error states. The epsilon-based matching and 500m radius fallback provide good resilience for coordinate matching.
frontend/src/components/map/ParishMap.tsx (2)
175-202: Well-structured utility function with proper error handling.The
fetchLandCoverForSourcefunction demonstrates good practices with exact coordinate matching, fallback logic, and comprehensive error handling.
26-78: Good component composition with LayerControls.The LayerControls component is well-structured as a separate, focused component with clear props interface and good accessibility features.
| <strong>Land Cover:</strong> | ||
| <ul className="ml-4 list-disc"> | ||
| {Object.entries(landCoverEntry.landcover_summary).map(([cls, obj]: any) => ( | ||
| <li key={cls}>{cls}: {obj.percentage.toFixed(1)}% ({obj.pixel_count} px)</li> | ||
| ))} | ||
| </ul> | ||
| </div> | ||
| </> | ||
| )} | ||
| {landCoverEntry && landCoverEntry.buildings && ( | ||
| <div className="mt-2"> | ||
| <strong>Buildings ({landCoverEntry.buildings.length}):</strong> | ||
| <ul className="ml-4 list-disc"> | ||
| {landCoverEntry.buildings.length > 0 ? | ||
| landCoverEntry.buildings.slice(0, 10).map((b: any, i: number) => ( | ||
| <li key={i}>Type: {b.type || 'Unknown'}, Area: {b.area_in_meters || '?'} m²</li> | ||
| )) : <li>None found</li>} | ||
| {landCoverEntry.buildings.length > 10 && ( | ||
| <li>...and {landCoverEntry.buildings.length - 10} more</li> | ||
| )} | ||
| </ul> | ||
| </div> | ||
| )} | ||
| </div> | ||
| ); | ||
| })} | ||
| </div> | ||
| </> | ||
| )} | ||
| </aside> | ||
| {/* Map */} | ||
| <main className="w-3/4 relative"> | ||
| <MapContainer | ||
| ref={(map) => { | ||
| if (map) { | ||
| mapRef.current = map; | ||
| try { | ||
| map.on('zoomend', () => { | ||
| if (map && typeof map.getZoom === 'function') { | ||
| setMapZoom(map.getZoom()); | ||
| } | ||
| }); | ||
| if (typeof map.getZoom === 'function') { | ||
| setMapZoom(map.getZoom()); | ||
| } | ||
| map.on('click', handleMapClick); | ||
| } catch (e) {} | ||
| } | ||
| }} | ||
| center={[0.447856, 33.202116]} | ||
| zoom={10} | ||
| scrollWheelZoom={true} | ||
| className="h-full" | ||
| > | ||
| <TileLayer | ||
| url="https://{s}.tile.openstreetmap.org/{z}/{x}/{y}.png" | ||
| attribution='© OpenStreetMap contributors' | ||
| /> | ||
| {parishes && parishes.features && parishes.features.length > 0 && ( | ||
| <GeoJSON | ||
| data={parishes} | ||
| style={parishStyle} | ||
| onEachFeature={onEachParish} | ||
| /> | ||
| )} | ||
| {showPollutionSources && pollutionSources.length > 0 && ( | ||
| <GeoJSON | ||
| data={{ | ||
| type: "FeatureCollection", | ||
| features: pollutionSources | ||
| } as any} | ||
| style={() => ({ | ||
| fillColor: '#ff4444', | ||
| weight: 1, | ||
| opacity: 0.8, | ||
| color: '#cc0000', | ||
| fillOpacity: 0.4 | ||
| })} | ||
| /> | ||
| )} | ||
| </MapContainer> | ||
| <LoadingIndicator loadingState={loadingState} errorState={errorState} /> | ||
| {/* MapLegend at top right */} | ||
| <div className="absolute top-4 right-4 z-[1000]"> | ||
| <MapLegend showPollutionSources={showPollutionSources} /> | ||
| </div> | ||
| {/* LayerControls just below MapLegend, not covering zoom */} | ||
| <div className="absolute top-32 right-4 z-[1000]"> | ||
| <LayerControls | ||
| showPollutionSources={showPollutionSources} | ||
| setShowPollutionSources={setShowPollutionSources} | ||
| pollutionSourcesAvailable={pollutionSources.length > 0} | ||
| /> | ||
| </div> | ||
| </main> | ||
| </div> | ||
| ); | ||
| } | ||
|
|
||
| export default ParishMap; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider extracting shared logic into reusable utilities.
This component shares significant functionality with ParishMap.tsx. The spatial operations, data fetching logic, and utility functions could be extracted into shared modules to reduce duplication and improve maintainability.
Consider creating:
hooks/useParishData.tsfor parish data fetchinghooks/usePollutionSources.tsfor pollution source logicutils/spatialUtils.tsfor Turf.js operationsutils/mapUtils.tsfor common map utilities
🧰 Tools
🪛 Biome (1.9.4)
[error] 298-298: Unnecessary continue statement
Unsafe fix: Delete the unnecessary continue statement
(lint/correctness/noUnnecessaryContinue)
[error] 364-364: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 507-507: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 545-545: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 594-594: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🤖 Prompt for AI Agents
In frontend/src/components/map/ParishMap.client.tsx lines 1 to 635, the
component duplicates spatial operations, data fetching, and utility functions
also found in ParishMap.tsx. To fix this, refactor by extracting parish data
fetching into a custom hook (e.g., hooks/useParishData.ts), pollution source
fetching and filtering into another hook (hooks/usePollutionSources.ts), Turf.js
spatial operations into a utility module (utils/spatialUtils.ts), and common
map-related utilities into utils/mapUtils.ts. Then import and use these shared
modules in the component to reduce duplication and improve maintainability.
| console.log('selectedParish:', selectedParish); | ||
| console.log('pollutionSources:', pollutionSources); | ||
| console.log('landCoverByLocation:', landCoverByLocation); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove debug console.log statements from production code.
These debug logs should be removed from production code as they can impact performance and expose internal application state.
- // Debug logs for sidebar rendering
- console.log('selectedParish:', selectedParish);
- console.log('pollutionSources:', pollutionSources);
- console.log('landCoverByLocation:', landCoverByLocation);Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In frontend/src/components/map/ParishMap.client.tsx around lines 419 to 421,
remove the debug console.log statements that output selectedParish,
pollutionSources, and landCoverByLocation. These logs are meant for debugging
and should not be present in production code to avoid performance issues and
leaking internal state.
| console.log('Loading pollution sources from combined_data_t.json...'); | ||
| const response = await fetch('/mapdata/combined_data_t.json'); | ||
|
|
||
| if (!response.ok) { | ||
| throw new Error(`HTTP error! status: ${response.status}`); | ||
| } | ||
|
|
||
| const data = await response.json(); | ||
| console.log('Pollution sources data loaded:', { | ||
| dataType: typeof data, | ||
| isArray: Array.isArray(data), | ||
| length: Array.isArray(data) ? data.length : 'not an array' | ||
| }); |
There was a problem hiding this comment.
Remove extensive debug logging from production code.
Multiple console.log statements throughout the component should be removed from production code. These can impact performance and expose internal application details.
- console.log('Loading pollution sources from combined_data_t.json...');
- console.log('Pollution sources data loaded:', {
- dataType: typeof data,
- isArray: Array.isArray(data),
- length: Array.isArray(data) ? data.length : 'not an array'
- });
- console.log('Valid pollution sources:', {
- total: data.length,
- valid: validSources.length,
- invalid: data.length - validSources.length
- });Remove all similar debug logging statements throughout the component.
Also applies to: 311-315, 333-336, 418-450, 496-499, 582-586, 860-883
🤖 Prompt for AI Agents
In frontend/src/components/map/ParishMap.tsx between lines 282-294 and other
specified ranges, remove all console.log statements used for debugging to
prevent performance issues and avoid exposing internal details. Search for and
delete all console.log calls in the mentioned line ranges (311-315, 333-336,
418-450, 496-499, 582-586, 860-883) to clean up the production code.
| onEachFeature={onEachParish} | ||
| /> | ||
| )} | ||
|
|
||
| {/* Pollution Sources - Show all when no parish selected, or parish-specific when parish selected */} | ||
| {showPollutionSources && pollutionSources.length > 0 && ( | ||
| <GeoJSON | ||
| data={{ | ||
| type: "FeatureCollection", | ||
| features: pollutionSources.filter(source => { | ||
| // Validate coordinates for Polygon geometry | ||
| if (!source || | ||
| !source.geometry?.coordinates || | ||
| !Array.isArray(source.geometry.coordinates) || | ||
| source.geometry.coordinates.length === 0) { | ||
| return false; | ||
| } | ||
|
|
||
| // Check if it's a valid polygon (should be array of arrays) | ||
| const coords = source.geometry.coordinates[0]; | ||
| if (!Array.isArray(coords) || coords.length < 3) { | ||
| return false; | ||
| } | ||
|
|
||
| // Check if coordinates are valid numbers | ||
| for (const coord of coords) { | ||
| if (!Array.isArray(coord) || coord.length !== 2 || | ||
| typeof coord[0] !== 'number' || typeof coord[1] !== 'number') { | ||
| return false; | ||
| } | ||
| } | ||
|
|
||
| // If a parish is selected, only show sources in that parish | ||
| if (selectedParish) { | ||
| try { | ||
| const parishPoly = turf.polygon(selectedParish.geometry.coordinates); | ||
| const sourcePoly = turf.polygon(source.geometry.coordinates); | ||
| return turf.booleanIntersects(parishPoly, sourcePoly); | ||
| } catch (error) { | ||
| console.warn('Error checking parish intersection for pollution source:', error); | ||
| return false; | ||
| } | ||
| } | ||
|
|
||
| return true; | ||
| }).map(source => ({ | ||
| type: "Feature", | ||
| geometry: source.geometry, | ||
| properties: { | ||
| ...source.properties, | ||
| confidence_score: source.properties.confidence_score, | ||
| timestamp: source.properties.timestamp | ||
| } | ||
| })) | ||
| } as any} | ||
| style={(feature) => ({ | ||
| fillColor: '#ff4444', | ||
| weight: 1, | ||
| opacity: 0.8, | ||
| color: '#cc0000', | ||
| fillOpacity: 0.4 | ||
| })} | ||
| onEachFeature={(feature, layer) => { | ||
| if (feature.properties) { | ||
| const popupContent = ` | ||
| <div> | ||
| <strong>Pollution Source</strong><br/> | ||
| Confidence: ${(feature.properties.confidence_score * 100).toFixed(1)}%<br/> | ||
| Timestamp: ${feature.properties.timestamp}<br/> | ||
| Location: ${feature.properties.latitude.toFixed(6)}, ${feature.properties.longitude.toFixed(6)} | ||
| </div> | ||
| `; | ||
| layer.bindPopup(popupContent); | ||
| } | ||
| }} | ||
| /> | ||
| )} | ||
| </MapContainer> | ||
|
|
||
| {/* Loading and Error Indicators */} | ||
| <LoadingIndicator loadingState={loadingState} errorState={errorState} /> | ||
|
|
||
| {/* Map Legend */} | ||
| <div className="absolute top-4 right-4 z-[1000]"> | ||
| <MapLegend showPollutionSources={showPollutionSources} /> | ||
| </div> | ||
| {/* Layer Controls: position further below the legend, top-right */} | ||
| <div className="absolute right-4 top-60 z-[1000]"> | ||
| <LayerControls | ||
| showPollutionSources={showPollutionSources} | ||
| setShowPollutionSources={setShowPollutionSources} | ||
| pollutionSourcesAvailable={pollutionSources.length > 0} | ||
| showBuildingsOverlay={showBuildingsOverlay} | ||
| setShowBuildingsOverlay={setShowBuildingsOverlay} | ||
| /> | ||
| </div> | ||
| </main> | ||
| </div> | ||
| ); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Significant code duplication with ParishMap.client.tsx.
This component shares substantial functionality with ParishMap.client.tsx including utility functions, spatial operations, and UI components. This duplication increases maintenance burden and risk of inconsistencies.
Recommendations:
- Extract shared utilities into common modules
- Create a base component with shared logic
- Use composition to handle client vs server-side differences
- Consider if both components are actually needed or if one can be refactored to handle both cases
🧰 Tools
🪛 Biome (1.9.4)
[error] 365-365: Unnecessary continue statement
Unsafe fix: Delete the unnecessary continue statement
(lint/correctness/noUnnecessaryContinue)
[error] 604-604: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 606-606: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 632-632: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 1008-1008: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🤖 Prompt for AI Agents
In frontend/src/components/map/ParishMap.tsx lines 1 to 1111, there is
significant duplication with ParishMap.client.tsx including utilities, spatial
logic, and UI components. To fix this, extract shared utility functions and
types into separate common modules that both components can import. Refactor
common logic into a base component or hooks to avoid repeating code. Use
composition or conditional rendering to handle differences between client and
server versions. Evaluate if both components are necessary or if one can be
unified to reduce maintenance overhead and ensure consistency.
Pr for the frontend sources of pollution
Summary by CodeRabbit
New Features
Bug Fixes
Chores
Documentation