fix(spatial): address review feedback - hybrid distance calculation and IDL test coverage

[Copilot]

Addresses review comments on PR #358's spatial optimization. Four issues: (1) missing IDL test coverage, (2) find_nearby_issues returns approximation instead of accurate distance, (3) misleading comments, (4) inconsistent Earth radius constants.

Changes

• Hybrid distance calculation: Use equirectangular squared-distance as pre-filter, compute Haversine only for candidates that pass. Maintains ~4x speedup while returning accurate great-circle distances to callers.

• IDL test coverage: Added test cases for ±180° boundary and high-latitude (60°N) scenarios where longitude compression matters.

• Unified Earth radius: Replaced hardcoded R = 6378137.0 in get_bounding_box with shared EARTH_RADIUS_METERS = 6371000.0. Documented choice of mean radius over WGS84 equatorial radius.

• Clarified documentation: Updated comments and docstrings to specify returned distances are Haversine (great-circle), not approximations.

# Before: Only equirectangular (fast but approximate)
if dist_sq <= radius_sq:
    distance = math.sqrt(dist_sq)  # Approximation
    nearby_issues.append((issue, distance))

# After: Hybrid approach (fast pre-filter + accurate result)
if dist_sq <= radius_sq:
    distance = haversine_distance(target_lat, target_lon, issue.latitude, issue.longitude)
    nearby_issues.append((issue, distance))

---

✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

---

Summary by cubic

Optimized spatial deduplication with a fast equirectangular pre-filter and Haversine for final, sorted distances. Unified Earth-radius usage to the mean radius and expanded tests for edge cases.

• Refactors
  • Pre-filter with equirectangular; compute Haversine for candidates and return distances sorted by proximity. Added focused tests for IDL/high‑latitude handling and small‑distance approximation accuracy.
  • Use EARTH_RADIUS_METERS (6371000) across utils (including bounding boxes) and document why the mean radius is preferred over WGS84 equatorial. Added Earth‑radius consistency checks.

^{Written for commit 6eb751c. Summary will update on new commits.}

[github-actions]

🔍 Quality Reminder

Thanks for the updates! Please ensure:
- Your changes don't break existing functionality
- All tests still pass
- Code quality standards are maintained

*The maintainers will verify that the overall project flow remains intact.*

[cubic-dev-ai]

2 issues found across 4 files

Prompt for AI agents (all issues)

Check if these issues are valid — if so, understand the root cause of each and fix them.


<file name="tests/test_spatial_utils_only.py">

<violation number="1" location="tests/test_spatial_utils_only.py:123">
P2: Misleading comment contradicts test behavior. The comment says "shouldn't match across IDL" but the test uses a 250km radius and asserts that both issues ARE found (including across IDL). The variable name `nearby_small` is also misleading since 250km is not small.</violation>
</file>

<file name="backend/spatial_utils.py">

<violation number="1" location="backend/spatial_utils.py:135">
P2: Missing radius check after computing Haversine distance. The equirectangular pre-filter can underestimate distances, allowing issues outside the radius to be included. The accurate Haversine distance should be verified against `radius_meters` before appending.</violation>
</file>

_{Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.}

[cubic-dev-ai]

P2: Misleading comment contradicts test behavior. The comment says "shouldn't match across IDL" but the test uses a 250km radius and asserts that both issues ARE found (including across IDL). The variable name nearby_small is also misleading since 250km is not small.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At tests/test_spatial_utils_only.py, line 123:

<comment>Misleading comment contradicts test behavior. The comment says "shouldn't match across IDL" but the test uses a 250km radius and asserts that both issues ARE found (including across IDL). The variable name `nearby_small` is also misleading since 250km is not small.</comment>

<file context>
@@ -0,0 +1,205 @@
+        Issue(id=6, latitude=0.0, longitude=-179.0),
+    ]
+    
+    # With small radius, shouldn't match across IDL
+    nearby_small = find_nearby_issues(issues_wrapped, 0.0, 179.0, radius_meters=250000)
+    print(f"  Found {len(nearby_small)} issues within 250km from 179.0°E")
</file context>

[cubic-dev-ai]

P2: Missing radius check after computing Haversine distance. The equirectangular pre-filter can underestimate distances, allowing issues outside the radius to be included. The accurate Haversine distance should be verified against radius_meters before appending.

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 135:

<comment>Missing radius check after computing Haversine distance. The equirectangular pre-filter can underestimate distances, allowing issues outside the radius to be included. The accurate Haversine distance should be verified against `radius_meters` before appending.</comment>

<file context>
@@ -120,16 +123,16 @@ def find_nearby_issues(
-            # Calculate actual distance (sqrt of squared distance)
-            distance = math.sqrt(dist_sq)
+            # Calculate accurate great-circle distance for candidates that passed filter
+            distance = haversine_distance(target_lat, target_lon, issue.latitude, issue.longitude)
             nearby_issues.append((issue, distance))
 
</file context>

Suggested change

	distance = haversine_distance(target_lat, target_lon, issue.latitude, issue.longitude)
	distance = haversine_distance(target_lat, target_lon, issue.latitude, issue.longitude)
	if distance > radius_meters:
	continue

[Copilot]

Pull request overview

Improves the spatial deduplication utilities and related guidance by returning accurate great-circle distances while preserving the equirectangular performance optimization, and adds International Date Line-focused tests.

Changes:

• Hybrid distance calculation: equirectangular squared-distance pre-filter + Haversine for returned distances in find_nearby_issues.
• Unified Earth radius constant usage in spatial helpers.
• Added/expanded IDL and high-latitude test coverage; updated internal optimization notes.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File	Description
backend/spatial_utils.py	Unifies Earth radius constant and switches nearby-issue distance output to accurate Haversine after a fast pre-filter.
tests/test_spatial_deduplication.py	Adds an IDL handling test for spatial utilities.
tests/test_spatial_utils_only.py	Introduces a focused spatial utility test suite (Haversine/equirectangular/IDL/constant checks).
.jules/bolt.md	Updates internal “bolt” guidance to document the hybrid pre-filter + accurate-distance approach.

Comments suppressed due to low confidence (1)

backend/spatial_utils.py:38

• get_bounding_box can return min_lon/max_lon outside the [-180, 180] range (or a range that crosses the International Date Line). Callers use these values directly in SQL Issue.longitude >= min_lon / <= max_lon filters, which will miss valid candidates across the IDL (e.g., target lon=179.9, radius=30km should include lon=-179.9, but won’t). Consider normalizing longitudes and explicitly handling the wrap case (e.g., return a flag / two longitude intervals, or provide a helper that builds the correct SQL filter with an OR when the box crosses the IDL; also consider the near-pole case where longitude should be unconstrained).

def get_bounding_box(lat: float, lon: float, radius_meters: float) -> Tuple[float, float, float, float]:
    """
    Calculate the bounding box coordinates for a given radius.
    Returns (min_lat, max_lat, min_lon, max_lon).
    """
    # Coordinate offsets in radians
    # Prevent division by zero at poles
    effective_lat = max(min(lat, 89.9), -89.9)
    dlat = radius_meters / EARTH_RADIUS_METERS
    dlon = radius_meters / (EARTH_RADIUS_METERS * math.cos(math.pi * effective_lat / 180.0))

    # Offset positions in decimal degrees
    lat_offset = dlat * 180.0 / math.pi
    lon_offset = dlon * 180.0 / math.pi

    min_lat = lat - lat_offset
    max_lat = lat + lat_offset
    min_lon = lon - lon_offset
    max_lon = lon + lon_offset

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

This new IDL test validates find_nearby_issues on an in-memory list, but the production endpoints first pre-filter in SQL using get_bounding_box + longitude >= min_lon AND longitude <= max_lon. Since get_bounding_box doesn’t currently handle International Date Line wrap, the API can still miss cross-IDL candidates even if find_nearby_issues is correct. Add a test that exercises the /api/issues/nearby endpoint (or a unit test for get_bounding_box’s wrap behavior) around ±180° to prevent regressions.

[Copilot]

The comment for this case is contradictory: it says “With small radius, shouldn't match across IDL”, but the radius is 250km and the assertions expect the cross‑IDL point (~222km away) to be included. Please update the comment to reflect what’s being tested (e.g., that cross‑IDL matching occurs only when the radius is large enough).