fix(map): restore boosted-marker clustering below zoom 6#1055
Conversation
The Leaflet -> MapLibre migration (#1003) routed every boosted place to a permanent cluster:false source, so orange boosted pins never grouped at any zoom. The legacy /map clustered them together with regular markers at zoom <= BOOSTED_CLUSTERING_MAX_ZOOM (5) and only pulled them into a standalone layer above it, keeping the zoomed-out world view uncluttered. Reintroduce that zoom gate: route boosted places into the clustered `places` source at/below zoom 5 and into the non-clustered `places-boosted` source above it, re-syncing on the boundary crossing — the MapLibre analogue of the old boostedLayer swap. Routing is extracted to $lib/map/boostedClustering with unit tests. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
✅ Deploy Preview for btcmap ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThis PR introduces zoom-dependent routing for boosted places on a MapLibre map. New utilities partition places into clustered vs. standalone arrays based on zoom level and boost status, with comprehensive tests. The map page integrates these helpers, adds state tracking for re-sync optimization, and updates pulse and feature rendering logic. ChangesBoosted Place Zoom-Based Routing
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/routes/map/+page.svelte (1)
228-242:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winPulse visibility skips existence checks for boosted features in
places-boosted.When the selected place is boosted and above the clustering threshold, visibility defaults to
truewithout checking whether that feature is actually present in theplaces-boostedsource. This can leave an orphan pulse when the place is filtered out.💡 Proposed fix
const updatePulseVisibility = () => { if (!map || !pulseMarker || pulsePinId === null) return; const place = get(placesById).get(pulsePinId); - let visible = true; + if (!place) { + pulseMarker.getElement().style.display = "none"; + return; + } + let visible = true; const inClusteredSource = - !!place && (!isBoosted(place) || shouldClusterBoostedAtZoom(map.getZoom())); + !isBoosted(place) || shouldClusterBoostedAtZoom(map.getZoom()); if (inClusteredSource) { const filter: FilterSpecification = [ "all", ["!", ["has", "point_count"]], ["==", ["get", "id"], pulsePinId], ]; visible = map.querySourceFeatures("places", { filter }).length > 0; + } else { + const filter: FilterSpecification = ["==", ["get", "id"], pulsePinId]; + visible = map.querySourceFeatures("places-boosted", { filter }).length > 0; } pulseMarker.getElement().style.display = visible ? "" : "none"; };🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/routes/map/`+page.svelte around lines 228 - 242, updatePulseVisibility currently assumes boosted features exist when isBoosted(place) && shouldClusterBoostedAtZoom(map.getZoom()), causing an orphan pulse; update the function (updatePulseVisibility) to query the correct source when the place is boosted and above threshold (use "places-boosted" instead of "places") by constructing the same filter (["all", ["!", ["has","point_count"]], ["==", ["get","id"], pulsePinId]]) and setting visible based on map.querySourceFeatures("places-boosted", { filter }).length > 0; keep the existing non-boosted branch querying "places", and continue to fall back to hidden when map, pulseMarker, or pulsePinId are missing.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src/routes/map/`+page.svelte:
- Around line 228-242: updatePulseVisibility currently assumes boosted features
exist when isBoosted(place) && shouldClusterBoostedAtZoom(map.getZoom()),
causing an orphan pulse; update the function (updatePulseVisibility) to query
the correct source when the place is boosted and above threshold (use
"places-boosted" instead of "places") by constructing the same filter (["all",
["!", ["has","point_count"]], ["==", ["get","id"], pulsePinId]]) and setting
visible based on map.querySourceFeatures("places-boosted", { filter }).length >
0; keep the existing non-boosted branch querying "places", and continue to fall
back to hidden when map, pulseMarker, or pulsePinId are missing.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 799b9c0d-8298-4fee-958d-3116bf68d6a7
📒 Files selected for processing (3)
src/lib/map/boostedClustering.test.tssrc/lib/map/boostedClustering.tssrc/routes/map/+page.svelte
There was a problem hiding this comment.
Pull request overview
This PR restores the legacy “boosted markers cluster when zoomed out” behavior in the MapLibre-based /map by dynamically routing boosted places into the clustered vs. non-clustered GeoJSON sources based on zoom, and re-syncing when crossing the zoom boundary.
Changes:
- Added
$lib/map/boostedClusteringto decide whether boosted places should be clustered at a given zoom and to route places intoplacesvsplaces-boosted. - Updated
/mapsource syncing to use the new routing and to re-sync onmoveendwhen crossingBOOSTED_CLUSTERING_MAX_ZOOM. - Added Vitest unit tests covering zoom-based routing, expired boosts, and deleted places.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/routes/map/+page.svelte | Routes boosted places into clustered vs. standalone sources based on zoom; re-syncs routing on zoom boundary crossings. |
| src/lib/map/boostedClustering.ts | Introduces zoom gate + routing helper for boosted clustering behavior. |
| src/lib/map/boostedClustering.test.ts | Adds unit tests validating routing behavior across zoom thresholds and place states. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
MapLibre zoom is fractional but clustering transitions on integer zoom levels (the regular places source declusters at z17+ via clusterMaxZoom). Comparing the raw zoom (zoom <= 5) declustered boosted markers at display zoom 5.x, so they broke out of clusters earlier than the legacy 'zoom 1-5 clustered, 6+ standalone' intent while regular places were still clustered at the z5 level. Floor the zoom so boosted markers stay clustered across the whole integer-5 band and only break out at z6. Adds fractional-zoom regression tests. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The Leaflet -> MapLibre migration (#1003) routed every boosted place to a permanent cluster:false source, so orange boosted pins never grouped at any zoom. The legacy /map clustered them together with regular markers at zoom <= BOOSTED_CLUSTERING_MAX_ZOOM (5) and only pulled them into a standalone layer above it, keeping the zoomed-out world view uncluttered.
Reintroduce that zoom gate: route boosted places into the clustered
placessource at/below zoom 5 and into the non-clusteredplaces-boostedsource above it, re-syncing on the boundary crossing — the MapLibre analogue of the old boostedLayer swap. Routing is extracted to $lib/map/boostedClustering with unit tests.Summary by CodeRabbit
New Features
Bug Fixes
Tests