Add safety heatmap page — safest areas visualization#123
Closed
amastbau wants to merge 3 commits intomaorcc:mainfrom
Closed
Add safety heatmap page — safest areas visualization#123amastbau wants to merge 3 commits intomaorcc:mainfrom
amastbau wants to merge 3 commits intomaorcc:mainfrom
Conversation
New page at /safety.html that visualizes alert frequency per location as a color-coded map: - Green: safest areas (0-5 alerts) - Yellow: moderate (31-50) - Red: high alert zones (80+) Features: - Adjustable time range slider (1-30 days) - Top 10 safest locations panel - Alert distribution histogram - Click any location for details - Uses existing /api/day-history endpoint - Dark theme matching the main app Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdded a new Hebrew RTL Leaflet safety map page ( Changes
Sequence DiagramsequenceDiagram
actor User
participant Page as Safety Page
participant Server as API Server
participant Map as Leaflet Map
participant Chart as Visualization
User->>Page: Open `safety.html`
Page->>Server: GET `/oref_points.json`
Server-->>Page: Coordinates
Page->>Map: Initialize dark RTL map & base layer
Map-->>Page: Ready
User->>Page: Change day range / filters
activate Page
Note over Page: Show loading overlay\nincrement request id
loop for each day in range
Page->>Server: GET `/api/day-history?date=...`
Server-->>Page: Day alerts (or error -> empty)
Page->>Page: Classify alerts and aggregate per location
end
Page->>Map: Update circle markers (color & radius)
Page->>Chart: Update top-10 list & distribution chart
Page->>Page: Update subtitle/legend, hide loading overlay
deactivate Page
Page-->>User: Updated map, list, and chart
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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: 4
🧹 Nitpick comments (1)
web/safety.html (1)
18-27: Add a small-screen layout mode for fixed panels.Current fixed panel layout can overlap on mobile widths. A compact stacked layout will improve usability.
Example mobile media-query
+@media (max-width: 768px) { + `#controls`, `#stats-box`, `#chart-box`, `#back-link` { + position: static; + max-width: none; + margin: 8px; + } + `#map` { + height: calc(100% - 260px); + } +}Also applies to: 85-97, 145-156
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/safety.html` around lines 18 - 27, The fixed `#controls` panel can overlap on small screens; add a mobile media query (e.g. `@media` (max-width: 600px)) that makes `#controls` and the other fixed-panel blocks referenced in the diff switch to a compact stacked layout: keep position: fixed but set left: 12px (or 8px) and right: auto, reduce top/right offsets if needed, set max-width: calc(100% - 24px) (or width: 100%), decrease gap (e.g. gap: 6px), and ensure child panels inside these selectors use width: 100% and box-sizing: border-box so they stack cleanly; apply the same media-query adjustments to the other fixed-panel rule blocks mentioned (lines 85-97 and 145-156) to keep consistent behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@web/safety.html`:
- Around line 325-359: The loadData function can have overlapping async runs
that let a slower call overwrite newer results; add a request-token guard:
generate a unique token at start of loadData (e.g., const reqId = Symbol() or a
monotonically increasing number), store it to a shared variable like
currentRequestId, and before applying results (setting alertData and calling
renderMap and hiding loading) verify reqId === currentRequestId; if not, abort
applying the stale data. Apply the same token check at any other completion
paths mentioned (e.g., the code around lines 380-383) so only the most recent
loadData run updates UI state (currentDays, alertData, renderMap, and hiding
`#loading`).
- Around line 362-371: The fetch chain that loads /oref_points.json currently
has no error handling so failures leave initialization and the loading overlay
stuck; add a .catch handler to the promise chain (or use try/catch with
async/await) around the fetch → r.json sequence to set locationPoints to an
empty object or a sensible default, log the error, and ensure loadData(7) is
called regardless of success or failure; also ensure any loading overlay is
cleared in the error path (call the existing hide/remove overlay helper or
ensure loadData/its finally block clears it). Use the existing symbols
fetch('/oref_points.json'), locationPoints, and loadData to implement this
recovery path.
- Line 16: The CSS rule `.leaflet-interactive:focus { outline: none; }` removes
keyboard focus indication and must be replaced with a visible, high-contrast
focus style; update the `.leaflet-interactive` focus selector (preferably
`.leaflet-interactive:focus, .leaflet-interactive:focus-visible`) to provide a
clear outline or box-shadow (e.g., 2px solid or an accessible box-shadow color)
and ensure the color/contrast meets accessibility standards so keyboard users
can see focus on interactive map elements.
- Around line 196-201: The legend shows red as "80+" but the rendering logic
still treats 80 as orange; update the threshold checks used by the marker color
and histogram binning so that 80 is classified as red. Specifically, find the
functions that determine bar/marker colors (e.g., getColorForAlertCount,
markerColorForAlerts, or the histogram binning logic) and change the orange
upper-bound check from <= 80 (or < 81) to < 80, and/or change the red check to
use >= 80 (instead of > 80); ensure all places that compute colors/bins (markers
and histogram rendering) use the same comparison so 80 maps to the red "#f44336"
legend entry.
---
Nitpick comments:
In `@web/safety.html`:
- Around line 18-27: The fixed `#controls` panel can overlap on small screens; add
a mobile media query (e.g. `@media` (max-width: 600px)) that makes `#controls` and
the other fixed-panel blocks referenced in the diff switch to a compact stacked
layout: keep position: fixed but set left: 12px (or 8px) and right: auto, reduce
top/right offsets if needed, set max-width: calc(100% - 24px) (or width: 100%),
decrease gap (e.g. gap: 6px), and ensure child panels inside these selectors use
width: 100% and box-sizing: border-box so they stack cleanly; apply the same
media-query adjustments to the other fixed-panel rule blocks mentioned (lines
85-97 and 145-156) to keep consistent behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: f5663781-12f2-4d95-b3b9-81ed0cea8028
📒 Files selected for processing (1)
web/safety.html
Comment on lines
+362
to
+371
| // Load location points then start | ||
| fetch('/oref_points.json') | ||
| .then(r => r.json()) | ||
| .then(points => { | ||
| locationPoints = {}; | ||
| for (const [name, coords] of Object.entries(points)) { | ||
| locationPoints[name] = [coords[0], coords[1]]; | ||
| } | ||
| return loadData(7); | ||
| }); |
There was a problem hiding this comment.
Handle initial points-load failures to avoid a stuck loading state.
If /oref_points.json fails, initialization has no recovery path and the loading overlay can remain indefinitely.
Bootstrap error handling
fetch('/oref_points.json')
.then(r => r.json())
.then(points => {
locationPoints = {};
for (const [name, coords] of Object.entries(points)) {
locationPoints[name] = [coords[0], coords[1]];
}
return loadData(7);
- });
+ })
+ .catch((err) => {
+ console.error('Failed to initialize safety map:', err);
+ const loading = document.getElementById('loading');
+ loading.textContent = 'שגיאה בטעינת נתונים';
+ loading.style.display = 'block';
+ });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/safety.html` around lines 362 - 371, The fetch chain that loads
/oref_points.json currently has no error handling so failures leave
initialization and the loading overlay stuck; add a .catch handler to the
promise chain (or use try/catch with async/await) around the fetch → r.json
sequence to set locationPoints to an empty object or a sensible default, log the
error, and ensure loadData(7) is called regardless of success or failure; also
ensure any loading overlay is cleared in the error path (call the existing
hide/remove overlay helper or ensure loadData/its finally block clears it). Use
the existing symbols fetch('/oref_points.json'), locationPoints, and loadData to
implement this recovery path.
Features: - Add alert type filters (red/yellow/green) to control which alerts are counted - Yellow filter allows excluding preliminary warnings like early Iran alerts Fixes: - Fix race condition when rapidly changing filters/slider - Add error handling for location points fetch failure - Fix accessibility: replace outline:none with visible focus style - Fix color threshold: 80 alerts now correctly shows as red (was orange) - Add mobile responsive layout for better small-screen UX Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
web/safety.html (1)
259-260:⚠️ Potential issue | 🟡 MinorUpdate range labels to match the
< 80boundary.The code now correctly treats
80as red, but labels still say51-80. This should be51-79in legend and chart labels to avoid UI inconsistency.Suggested fix
- <div class="row"><div class="bar" style="background:`#ff9800`"></div> רב התרעות (51-80)</div> + <div class="row"><div class="bar" style="background:`#ff9800`"></div> רב התרעות (51-79)</div>- const bucketLabels = ['0-5', '6-15', '16-30', '31-50', '51-80', '80+']; + const bucketLabels = ['0-5', '6-15', '16-30', '31-50', '51-79', '80+'];Also applies to: 387-387
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/safety.html` around lines 259 - 260, Update the range label that currently reads "51-80" to "51-79" so it matches the logic that treats 80 as the red category; locate the two legend/chart label occurrences (the HTML divs containing the text "רב התרעות (51-80)" and the duplicate at the other occurrence) and change their displayed range text to "51-79" to keep UI labels consistent with the category boundaries.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@web/safety.html`:
- Around line 329-331: The function that maps alert titles currently defaults
unknown titles to return 'red' without logging; update the mapping logic that
contains the return 'red' to also emit a console.warn (or processLogger.warn)
with the unknown title (e.g., "Unknown alert title: <title>, defaulting to red")
before returning 'red' so classifier drift is recorded; locate the branch that
falls through to return 'red' and add the warning using the alert title
variable.
- Around line 324-327: The yellow-alert matcher is missing the required
substring "להישאר בסמיכות", causing misclassification; update the conditional
checks that inspect the normalized variable (the normalized.includes(...)
checks) to also return 'yellow' when normalized.includes('להישאר בסמיכות') (in
addition to or instead of the existing 'להישאר בקרבתו' check) so yellow alert
titles match correctly.
---
Duplicate comments:
In `@web/safety.html`:
- Around line 259-260: Update the range label that currently reads "51-80" to
"51-79" so it matches the logic that treats 80 as the red category; locate the
two legend/chart label occurrences (the HTML divs containing the text "רב התרעות
(51-80)" and the duplicate at the other occurrence) and change their displayed
range text to "51-79" to keep UI labels consistent with the category boundaries.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 9eaf1949-57a8-48fa-9354-75931ac55d48
📒 Files selected for processing (1)
web/safety.html
Comment on lines
+324
to
+327
| if (normalized.includes('בדקות הקרובות צפויות להתקבל התרעות')) return 'yellow'; | ||
| if (normalized.includes('לשפר את המיקום למיגון המיטבי')) return 'yellow'; | ||
| if (normalized.includes('יש לשהות בסמיכות למרחב המוגן')) return 'yellow'; | ||
| if (normalized.includes('להישאר בקרבתו')) return 'yellow'; |
There was a problem hiding this comment.
Add the required yellow substring match for להישאר בסמיכות.
Line 327 matches להישאר בקרבתו, but the required yellow matcher is להישאר בסמיכות; this can misclassify yellow alerts as red and skew counts.
Suggested fix
- if (normalized.includes('להישאר בקרבתו')) return 'yellow';
+ if (normalized.includes('להישאר בסמיכות')) return 'yellow';
+ if (normalized.includes('להישאר בקרבתו')) return 'yellow';As per coding guidelines, "Match yellow alert titles by exact string or substring: לשפר את המיקום למיגון המיטבי, להישאר בסמיכות".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/safety.html` around lines 324 - 327, The yellow-alert matcher is missing
the required substring "להישאר בסמיכות", causing misclassification; update the
conditional checks that inspect the normalized variable (the
normalized.includes(...) checks) to also return 'yellow' when
normalized.includes('להישאר בסמיכות') (in addition to or instead of the existing
'להישאר בקרבתו' check) so yellow alert titles match correctly.
Comment on lines
+329
to
+331
| // Red (danger) — default for most alerts | ||
| return 'red'; | ||
| } |
There was a problem hiding this comment.
Log unknown alert titles before defaulting to red.
Line 330 defaults unknown titles to red, but there is no warning log, which makes classifier drift harder to detect.
Suggested fix
- // Red (danger) — default for most alerts
- return 'red';
+ // Red (danger) — default for unknown alerts
+ console.warn('Unknown alert title:', normalized);
+ return 'red';As per coding guidelines, "Default unknown alert titles to red and log a console warning".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/safety.html` around lines 329 - 331, The function that maps alert titles
currently defaults unknown titles to return 'red' without logging; update the
mapping logic that contains the return 'red' to also emit a console.warn (or
processLogger.warn) with the unknown title (e.g., "Unknown alert title: <title>,
defaulting to red") before returning 'red' so classifier drift is recorded;
locate the branch that falls through to return 'red' and add the warning using
the alert title variable.
- Toggle between total alerts and average alerts per day - Per-day mode normalizes counts for fair comparison across time ranges - Legend, chart, popups, and stats all adapt to selected mode - Tighter per-day thresholds (0.1, 0.3, 0.7, 1.5, 3) for better color spread Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Owner
|
היי @amastbau, תודה על העבודה, הדף בנוי טוב והרעיון מעניין. אני מתלבט לגבי מיזוג, ורוצה להיות ישיר לגבי הסיבה: לדעתי הפיצ'ר פונה לקהל יעד שונה ממה שמפת העורף נועדה לו. המפה בנויה לאנשים שנמצאים באזור פעיל או בסמוך אליו, כלי בזמן אמת, וגם פיצ'רי ההיסטוריה קיימים כדי לשחזר "מה קרה בשעה האחרונה". המשתמש שהמפה מכוונת אליו בודק אותה תוך כדי התרעה או מיד אחריה. המפת חום הזו היא בעצם כלי לניתוח היסטורי, שעונה על שאלה כמו "אילו אזורים היו הכי שקטים בחודש האחרון?" זו שאלה לגיטימית ושימושית, אבל מי ששואל אותה הוא חוקר, עיתונאי, או מישהו שמתכנן לאן לעבור לגור. זה קהל יעד שונה, ואני חושב שהוא ראוי לכלי שבנוי סביבו מלכתחילה. גם הכיוון הוויזואלי שונה, עם הרקע הכהה, הסליידרים, תרשים ההתפלגות. זה קרוב יותר ברוחו לוויזואליזציות הסטטיסטיות שיובל הרפז מפרסם (למשל alarms_by_year) ו-Oct7 Database. אני חושב שהעבודה שלך יכולה להתאים טוב לצד הפרויקטים האלה. אם תרצה, אשמח לחבר אותך עם יובל, הוא עובד בדיוק בתחום הזה ואולי יהיה שיתוף פעולה מעניין. |
7 tasks
Author
|
קיבל את ה דף בשמחה :D באמת מקום טוב יותר.תודה על ההפניה.סוגר את זה. |
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
2 participants
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
New page at
/safety.htmlshowing a color-coded heatmap of alert frequency per location, making it easy to see which areas in Israel have the fewest alerts.Features
/api/day-historyendpoint — no new backend neededHow it works
Fetches daily history for the selected range, aggregates alert counts per location using the existing
oref_points.jsoncoordinates, and rendersL.circleMarkerwith color/radius based on alert count. Safe areas are drawn on top so they're always visible.Screenshots
Access via
/safety.htmlor link from the main map.Test plan
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Improvements