⚡ Bolt: Add bounding box pre-filter to spatial query#907
Conversation
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
🙏 Thank you for your contribution, @RohanExploit!PR Details:
Quality Checklist:
Review Process:
Note: The maintainers will monitor code quality and ensure the overall project flow isn't broken. |
|
Warning Review limit reached
Next review available in: 45 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. 📝 WalkthroughWalkthrough
ChangesSpatial filtering
Estimated code review effort: 1 (Trivial) | ~5 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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: 1
🤖 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.
Inline comments:
In `@backend/spatial_utils.py`:
- Around line 79-89: Update the bounding-box longitude filtering in the spatial
query flow around get_bounding_box and the issue loop to correctly handle ranges
crossing the antimeridian, accepting longitudes on either side of ±180° while
preserving the latitude check and normal filtering behavior. Add a regression
test covering nearby coordinates such as 179.9999 and -179.9999 that
haversine_distance() considers within range.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
| # Pre-filter using bounding box with 5% epsilon to optimize expensive haversine calculations | ||
| min_lat, max_lat, min_lon, max_lon = get_bounding_box(target_lat, target_lon, radius_meters * 1.05) | ||
|
|
||
| for issue in issues: | ||
| if issue.latitude is None or issue.longitude is None: | ||
| continue | ||
|
|
||
| # Fast bounding box pre-filter check | ||
| if not (min_lat <= issue.latitude <= max_lat and min_lon <= issue.longitude <= max_lon): | ||
| continue | ||
|
|
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== backend/spatial_utils.py (relevant slice) =="
nl -ba backend/spatial_utils.py | sed -n '1,220p'
echo
echo "== Search for longitude normalization / antimeridian handling =="
rg -n "longitude|antimeridian|180|normalize|bounding box|haversine" backend tests . -g '!**/.git/**' || true
echo
echo "== File list for nearby spatial tests =="
fd -a -t f "test|spec" backend tests . || trueRepository: RohanExploit/VishwaGuru
Length of output: 249
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== backend/spatial_utils.py (relevant slice) =="
cat -n backend/spatial_utils.py | sed -n '1,220p'
echo
echo "== Search for longitude normalization / antimeridian handling =="
rg -n "longitude|antimeridian|180|normalize|bounding box|haversine" backend tests . -g '!**/.git/**' || true
echo
echo "== Files with tests/specs =="
fd -a -t f "test|spec" backend tests . || trueRepository: RohanExploit/VishwaGuru
Length of output: 50380
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== longitude validation contract =="
cat -n backend/schemas.py | sed -n '40,70p'
echo
cat -n backend/main_fixed.py | sed -n '315,335p'
echo
echo "== spatial test coverage =="
cat -n tests/test_spatial_deduplication.py | sed -n '1,220p'
echo
echo "== antimeridian edge-case probe =="
python3 - <<'PY'
import math
def get_bounding_box(lat, lon, radius_meters):
R = 6378137.0
effective_lat = max(min(lat, 89.9), -89.9)
dlat = radius_meters / R
dlon = radius_meters / (R * math.cos(math.pi * effective_lat / 180.0))
lat_offset = dlat * 180.0 / math.pi
lon_offset = dlon * 180.0 / math.pi
return lat - lat_offset, lat + lat_offset, lon - lon_offset, lon + lon_offset
def haversine_distance(lat1, lon1, lat2, lon2):
R = 6371000.0
phi1, phi2 = math.radians(lat1), math.radians(lat2)
dphi = math.radians(lat2 - lat1)
dlambda = math.radians(lon2 - lon1)
a = math.sin(dphi / 2)**2 + math.cos(phi1) * math.cos(phi2) * math.sin(dlambda / 2)**2
return R * (2 * math.atan2(math.sqrt(a), math.sqrt(1 - a)))
target_lat, target_lon, radius = 0.0, 179.9999, 50.0
issue_lat, issue_lon = 0.0, -179.9999
min_lat, max_lat, min_lon, max_lon = get_bounding_box(target_lat, target_lon, radius * 1.05)
print("bbox:", (min_lat, max_lat, min_lon, max_lon))
print("bbox_contains_issue:", min_lat <= issue_lat <= max_lat and min_lon <= issue_lon <= max_lon)
print("distance_m:", haversine_distance(target_lat, target_lon, issue_lat, issue_lon))
PYRepository: RohanExploit/VishwaGuru
Length of output: 11117
Handle longitude ranges that cross the antimeridian. backend/spatial_utils.py:79-89 The bounding-box prefilter can drop valid nearby issues when the longitude window wraps past ±180° (for example, 179.9999 vs -179.9999), even though haversine_distance() would include them. Split or wrap the longitude check in that case, and add a regression test.
🤖 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 `@backend/spatial_utils.py` around lines 79 - 89, Update the bounding-box
longitude filtering in the spatial query flow around get_bounding_box and the
issue loop to correctly handle ranges crossing the antimeridian, accepting
longitudes on either side of ±180° while preserving the latitude check and
normal filtering behavior. Add a regression test covering nearby coordinates
such as 179.9999 and -179.9999 that haversine_distance() considers within range.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 1 file
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="backend/spatial_utils.py">
<violation number="1" location="backend/spatial_utils.py:80">
P3: Bounding box pre-filter can silently drop valid nearby issues when the search crosses the 180th meridian. `get_bounding_box` subtracts/adds `lon_offset` from raw longitude without normalizing to [-180, 180], so near ±180° the box wraps incorrectly. For example, target_lon=179° with a large radius produces min_lon=172°, max_lon=186°, and an issue at -178° (2° away) fails the chained comparison and gets skipped. Consider normalizing the bounding box longitude range in `get_bounding_box`, or at least note the limitation. The PR default radius of 50m makes this very rare in practice; flagging for awareness.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| nearby_issues = [] | ||
|
|
||
| # Pre-filter using bounding box with 5% epsilon to optimize expensive haversine calculations | ||
| min_lat, max_lat, min_lon, max_lon = get_bounding_box(target_lat, target_lon, radius_meters * 1.05) |
There was a problem hiding this comment.
P3: Bounding box pre-filter can silently drop valid nearby issues when the search crosses the 180th meridian. get_bounding_box subtracts/adds lon_offset from raw longitude without normalizing to [-180, 180], so near ±180° the box wraps incorrectly. For example, target_lon=179° with a large radius produces min_lon=172°, max_lon=186°, and an issue at -178° (2° away) fails the chained comparison and gets skipped. Consider normalizing the bounding box longitude range in get_bounding_box, or at least note the limitation. The PR default radius of 50m makes this very rare in practice; flagging for awareness.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At backend/spatial_utils.py, line 80:
<comment>Bounding box pre-filter can silently drop valid nearby issues when the search crosses the 180th meridian. `get_bounding_box` subtracts/adds `lon_offset` from raw longitude without normalizing to [-180, 180], so near ±180° the box wraps incorrectly. For example, target_lon=179° with a large radius produces min_lon=172°, max_lon=186°, and an issue at -178° (2° away) fails the chained comparison and gets skipped. Consider normalizing the bounding box longitude range in `get_bounding_box`, or at least note the limitation. The PR default radius of 50m makes this very rare in practice; flagging for awareness.</comment>
<file context>
@@ -76,10 +76,17 @@ def find_nearby_issues(
nearby_issues = []
+ # Pre-filter using bounding box with 5% epsilon to optimize expensive haversine calculations
+ min_lat, max_lat, min_lon, max_lon = get_bounding_box(target_lat, target_lon, radius_meters * 1.05)
+
for issue in issues:
</file context>
💡 What: Added a bounding box pre-filter (
get_bounding_box) to thefind_nearby_issuesfunction inbackend/spatial_utils.pybefore calculating the expensivehaversine_distance.🎯 Why: The haversine formula uses expensive trig functions (
math.sin,math.cos,math.atan2). By pre-filtering points that fall outside a square bounding box containing the circular radius, we avoid computing the full haversine distance for points that are definitely outside the radius. We use a 5% epsilon (radius_meters * 1.05) for safety.📊 Impact: Reduces computation time for nearby issue queries from O(N) expensive haversine calculations to mostly cheap bounding box checks. Benchmark on 100k points dropped execution time from ~0.125s to ~0.006s (a ~20x improvement).
🔬 Measurement: Verified by running a local performance benchmark (
test_perf.py) generating 100k random coordinates and querying a 1km radius.PR created automatically by Jules for task 12551344620059451069 started by @RohanExploit
Summary by cubic
Add a bounding-box pre-filter to
find_nearby_issuesto skip haversine calculations (~20x faster on 100k points), and fix a SyntaxError increate_issueto restore Render deployment.Written for commit c5b9e64. Summary will update on new commits.
Summary by CodeRabbit