CRITICAL FIX: get_deals_by_radius returns polygon metadata, not deals
## Root Cause
After Pydantic migration, get_deals_by_radius() was attempting to validate
API responses as Deal objects. However, this endpoint returns POLYGON METADATA
(with fields: dealscount, polygon_id, settlementNameHeb), NOT individual deals.
All responses failed Pydantic validation (missing dealAmount, dealDate),
resulting in empty lists and 0 deals returned from ALL queries.
## Fixes Applied
### 1. client.py - get_deals_by_radius()
- Return type: `List[Deal]` → `List[Dict[str, Any]]`
- Remove Pydantic validation - return raw metadata dicts
- Update docstring to clarify this returns polygon metadata
- Add note to use find_recent_deals_for_address() for actual deals
### 2. client.py - find_recent_deals_for_address()
- Update to handle polygon metadata dicts (not Deal objects)
- Use dict.get('polygon_id') instead of model attribute access
- Rename variable: `nearby_deals` → `nearby_polygons` for clarity
### 3. fastmcp_server.py - get_deals_by_radius() tool
- Update to handle dict responses (not Deal objects)
- Remove strip_bloat_fields() call (not needed for metadata)
- Update docstring with WARNING about polygon metadata
- Change response keys: "deals" → "polygons", "total_deals" → "total_polygons"
### 4. Tests
- test_govmap_client.py: Update to expect dicts, not Deal objects
- test_fastmcp_tools.py: Update 3 tests to mock dict responses
## Impact
- ✅ find_recent_deals_for_address() NOW WORKS (was returning 0 deals)
- ✅ All 174 tests passing
- ✅ E2E API test confirmed working with real data
## API Behavior Documented
get_deals_by_radius endpoint design:
1. Returns polygon/area metadata (not individual deals)
2. Extract polygon_ids from metadata
3. Call get_street_deals(polygon_id) to get actual deals
4. This workflow is automated in find_recent_deals_for_address()
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
@@ -96,33 +96,37 @@ def autocomplete_address(search_text: str) -> str:
|
|||||||
|
|
||||||
@mcp.tool()
|
@mcp.tool()
|
||||||
def get_deals_by_radius(latitude: float, longitude: float, radius_meters: int = 500) -> str:
|
def get_deals_by_radius(latitude: float, longitude: float, radius_meters: int = 500) -> str:
|
||||||
"""Get real estate deals within a radius of coordinates.
|
"""Get polygon metadata within a radius of coordinates.
|
||||||
|
|
||||||
|
**NOTE**: This returns polygon/area metadata, NOT individual deal transactions!
|
||||||
|
Use find_recent_deals_for_address() to get actual deals.
|
||||||
|
|
||||||
Args:
|
Args:
|
||||||
latitude: Latitude coordinate
|
latitude: Latitude coordinate
|
||||||
longitude: Longitude coordinate
|
longitude: Longitude coordinate
|
||||||
radius_meters: Search radius in meters (default: 500)
|
radius_meters: Search radius in meters (default: 500)
|
||||||
|
|
||||||
Returns:
|
Returns:
|
||||||
JSON string containing recent real estate deals in the area
|
JSON string containing polygon metadata (areas with deals nearby)
|
||||||
"""
|
"""
|
||||||
try:
|
try:
|
||||||
# Note: GovmapClient expects (longitude, latitude) tuple
|
# Note: GovmapClient expects (longitude, latitude) tuple
|
||||||
deals = client.get_deals_by_radius((longitude, latitude), radius_meters)
|
# Returns polygon metadata dicts, not Deal objects
|
||||||
|
polygons = client.get_deals_by_radius((longitude, latitude), radius_meters)
|
||||||
if not deals:
|
|
||||||
return f"No deals found within {radius_meters}m of coordinates ({latitude}, {longitude})"
|
if not polygons:
|
||||||
|
return f"No polygons found within {radius_meters}m of coordinates ({latitude}, {longitude})"
|
||||||
|
|
||||||
return json.dumps({
|
return json.dumps({
|
||||||
"total_deals": len(deals),
|
"total_polygons": len(polygons),
|
||||||
"search_radius_meters": radius_meters,
|
"search_radius_meters": radius_meters,
|
||||||
"center_coordinates": {"latitude": latitude, "longitude": longitude},
|
"center_coordinates": {"latitude": latitude, "longitude": longitude},
|
||||||
"deals": strip_bloat_fields(deals)
|
"polygons": polygons # Return dicts directly, no stripping needed
|
||||||
}, ensure_ascii=False, indent=2)
|
}, ensure_ascii=False, indent=2)
|
||||||
|
|
||||||
except Exception as e:
|
except Exception as e:
|
||||||
logger.error(f"Error in get_deals_by_radius: {e}")
|
logger.error(f"Error in get_deals_by_radius: {e}")
|
||||||
return f"Error fetching deals by radius: {str(e)}"
|
return f"Error fetching polygons by radius: {str(e)}"
|
||||||
|
|
||||||
@mcp.tool()
|
@mcp.tool()
|
||||||
def get_street_deals(polygon_id: str, limit: int = 100, deal_type: int = 2) -> str:
|
def get_street_deals(polygon_id: str, limit: int = 100, deal_type: int = 2) -> str:
|
||||||
|
|||||||
+24
-19
@@ -256,16 +256,22 @@ class GovmapClient:
|
|||||||
|
|
||||||
def get_deals_by_radius(
|
def get_deals_by_radius(
|
||||||
self, point: Tuple[float, float], radius: int = 50
|
self, point: Tuple[float, float], radius: int = 50
|
||||||
) -> List[Deal]:
|
) -> List[Dict[str, Any]]:
|
||||||
"""
|
"""
|
||||||
Find real estate deals within a specified radius of a point.
|
Find polygon metadata within a specified radius of a point.
|
||||||
|
|
||||||
|
**NOTE**: This endpoint returns polygon metadata, NOT actual deal transactions!
|
||||||
|
The response contains polygon_id, dealscount, settlementNameHeb, etc.
|
||||||
|
To get actual deals, extract polygon_ids and call get_street_deals() or get_neighborhood_deals().
|
||||||
|
|
||||||
|
Use find_recent_deals_for_address() for the complete workflow.
|
||||||
|
|
||||||
Args:
|
Args:
|
||||||
point: A tuple of (longitude, latitude)
|
point: A tuple of (longitude, latitude)
|
||||||
radius: The search radius in meters (default: 50)
|
radius: The search radius in meters (default: 50)
|
||||||
|
|
||||||
Returns:
|
Returns:
|
||||||
List of Deal models found within the radius
|
List of polygon metadata dicts (not Deal objects)
|
||||||
|
|
||||||
Raises:
|
Raises:
|
||||||
requests.RequestException: If the API request fails after retries
|
requests.RequestException: If the API request fails after retries
|
||||||
@@ -293,17 +299,16 @@ class GovmapClient:
|
|||||||
f"Expected list response, got {type(data).__name__}"
|
f"Expected list response, got {type(data).__name__}"
|
||||||
)
|
)
|
||||||
|
|
||||||
# Parse each deal dict into Deal model
|
# NOTE: This endpoint returns polygon metadata, not actual deals!
|
||||||
deals = []
|
# The response contains: dealscount, polygon_id, settlementNameHeb, streetNameHeb, houseNum, objectid
|
||||||
for deal_dict in data:
|
# We return these as-is (raw dicts) since they're used only for extracting polygon_ids
|
||||||
try:
|
# in find_recent_deals_for_address()
|
||||||
deal = Deal.model_validate(deal_dict)
|
logger.info(f"Found {len(data)} polygon metadata records")
|
||||||
deals.append(deal)
|
|
||||||
except Exception as e:
|
|
||||||
logger.warning(f"Failed to parse deal: {e}. Skipping deal.")
|
|
||||||
continue
|
|
||||||
|
|
||||||
return deals
|
# For backward compatibility, return empty list of Deals since these aren't actual deals
|
||||||
|
# The raw data is available in the response but we don't try to validate as Deal objects
|
||||||
|
# TODO: Consider creating a PolygonMetadata model for type safety
|
||||||
|
return data # Return raw dicts temporarily
|
||||||
|
|
||||||
except (requests.RequestException, requests.Timeout) as e:
|
except (requests.RequestException, requests.Timeout) as e:
|
||||||
if attempt < self.config.max_retries:
|
if attempt < self.config.max_retries:
|
||||||
@@ -586,14 +591,14 @@ class GovmapClient:
|
|||||||
search_address_normalized = address.lower().strip()
|
search_address_normalized = address.lower().strip()
|
||||||
logger.info(f"Found coordinates: {point}")
|
logger.info(f"Found coordinates: {point}")
|
||||||
|
|
||||||
# Step 2: Get deals by radius to find polygon IDs
|
# Step 2: Get polygon metadata by radius to find polygon IDs
|
||||||
nearby_deals = self.get_deals_by_radius(point, radius=radius)
|
nearby_polygons = self.get_deals_by_radius(point, radius=radius)
|
||||||
|
|
||||||
# Extract unique polygon IDs
|
# Extract unique polygon IDs from metadata dicts
|
||||||
polygon_ids = set()
|
polygon_ids = set()
|
||||||
for deal in nearby_deals:
|
for metadata in nearby_polygons:
|
||||||
# Try to get polygon_id from the deal model
|
# Extract polygon_id from dict (these are polygon metadata, not deals)
|
||||||
polygon_id = getattr(deal, 'polygon_id', None) or deal.source_polygon_id
|
polygon_id = metadata.get('polygon_id')
|
||||||
if polygon_id:
|
if polygon_id:
|
||||||
polygon_ids.add(str(polygon_id))
|
polygon_ids.add(str(polygon_id))
|
||||||
|
|
||||||
|
|||||||
+31
-33
@@ -130,59 +130,57 @@ class TestGetDealsByRadius:
|
|||||||
|
|
||||||
@patch('nadlan_mcp.fastmcp_server.client')
|
@patch('nadlan_mcp.fastmcp_server.client')
|
||||||
def test_successful_get_deals(self, mock_client):
|
def test_successful_get_deals(self, mock_client):
|
||||||
"""Test successful deal retrieval."""
|
"""Test successful polygon metadata retrieval."""
|
||||||
# Mock with Deal models
|
# Mock with polygon metadata dicts (not Deal objects)
|
||||||
mock_deals = [
|
mock_polygons = [
|
||||||
Deal(
|
{
|
||||||
objectid=123,
|
"objectid": 123,
|
||||||
deal_amount=2000000,
|
"dealscount": "30",
|
||||||
deal_date="2023-01-01",
|
"settlementNameHeb": "תל אביב-יפו",
|
||||||
asset_area=80.0,
|
"streetNameHeb": "דיזנגוף",
|
||||||
street_name="דיזנגוף"
|
"houseNum": 50,
|
||||||
)
|
"polygon_id": "123-456"
|
||||||
|
}
|
||||||
]
|
]
|
||||||
mock_client.get_deals_by_radius.return_value = mock_deals
|
mock_client.get_deals_by_radius.return_value = mock_polygons
|
||||||
|
|
||||||
result = fastmcp_server.get_deals_by_radius(650000.0, 180000.0, 500)
|
result = fastmcp_server.get_deals_by_radius(650000.0, 180000.0, 500)
|
||||||
parsed = json.loads(result)
|
parsed = json.loads(result)
|
||||||
|
|
||||||
assert len(parsed["deals"]) == 1
|
assert len(parsed["polygons"]) == 1
|
||||||
assert parsed["deals"][0]["deal_amount"] == 2000000 # Use snake_case field name
|
assert parsed["polygons"][0]["dealscount"] == "30"
|
||||||
assert parsed["total_deals"] == 1
|
assert parsed["total_polygons"] == 1
|
||||||
mock_client.get_deals_by_radius.assert_called_once()
|
mock_client.get_deals_by_radius.assert_called_once()
|
||||||
|
|
||||||
@patch('nadlan_mcp.fastmcp_server.client')
|
@patch('nadlan_mcp.fastmcp_server.client')
|
||||||
def test_get_deals_no_results(self, mock_client):
|
def test_get_deals_no_results(self, mock_client):
|
||||||
"""Test deal retrieval with no results."""
|
"""Test polygon metadata retrieval with no results."""
|
||||||
mock_client.get_deals_by_radius.return_value = []
|
mock_client.get_deals_by_radius.return_value = []
|
||||||
|
|
||||||
result = fastmcp_server.get_deals_by_radius(650000.0, 180000.0, 500)
|
result = fastmcp_server.get_deals_by_radius(650000.0, 180000.0, 500)
|
||||||
assert "No deals found" in result or json.loads(result)["total_deals"] == 0
|
assert "No polygons found" in result
|
||||||
|
|
||||||
@patch('nadlan_mcp.fastmcp_server.client')
|
@patch('nadlan_mcp.fastmcp_server.client')
|
||||||
def test_get_deals_strips_bloat_fields(self, mock_client):
|
def test_get_deals_strips_bloat_fields(self, mock_client):
|
||||||
"""Test that bloat fields are stripped from response."""
|
"""Test that polygon metadata is returned as-is."""
|
||||||
# Mock with Deal models, not dicts
|
# Mock with polygon metadata dicts
|
||||||
mock_deals = [
|
mock_polygons = [
|
||||||
Deal(
|
{
|
||||||
objectid=123,
|
"objectid": 123,
|
||||||
deal_amount=2000000,
|
"dealscount": "10",
|
||||||
deal_date="2023-01-01",
|
"polygon_id": "abc123",
|
||||||
shape="MULTIPOLYGON(...huge data...)",
|
"settlementNameHeb": "Tel Aviv"
|
||||||
sourceorder=1,
|
}
|
||||||
source_polygon_id="abc123"
|
|
||||||
)
|
|
||||||
]
|
]
|
||||||
mock_client.get_deals_by_radius.return_value = mock_deals
|
mock_client.get_deals_by_radius.return_value = mock_polygons
|
||||||
|
|
||||||
result = fastmcp_server.get_deals_by_radius(650000.0, 180000.0, 500)
|
result = fastmcp_server.get_deals_by_radius(650000.0, 180000.0, 500)
|
||||||
parsed = json.loads(result)
|
parsed = json.loads(result)
|
||||||
|
|
||||||
# Verify bloat fields are removed
|
# Polygon metadata is returned as-is (no stripping needed)
|
||||||
deal = parsed["deals"][0]
|
polygon = parsed["polygons"][0]
|
||||||
assert "shape" not in deal
|
assert polygon["polygon_id"] == "abc123"
|
||||||
assert "sourceorder" not in deal
|
assert polygon["dealscount"] == "10"
|
||||||
# source_polygon_id is kept when added by our processing
|
|
||||||
|
|
||||||
@patch('nadlan_mcp.fastmcp_server.client')
|
@patch('nadlan_mcp.fastmcp_server.client')
|
||||||
def test_get_deals_error_handling(self, mock_client):
|
def test_get_deals_error_handling(self, mock_client):
|
||||||
|
|||||||
@@ -128,14 +128,16 @@ class TestGovmapClient:
|
|||||||
|
|
||||||
@patch('requests.Session')
|
@patch('requests.Session')
|
||||||
def test_get_deals_by_radius_success(self, mock_session_class):
|
def test_get_deals_by_radius_success(self, mock_session_class):
|
||||||
"""Test successful deals by radius query."""
|
"""Test successful polygon metadata retrieval by radius."""
|
||||||
mock_response = Mock()
|
mock_response = Mock()
|
||||||
|
# API returns polygon metadata (not actual deals)
|
||||||
mock_response.json.return_value = [
|
mock_response.json.return_value = [
|
||||||
{
|
{
|
||||||
"objectid": 12345,
|
"objectid": 12345,
|
||||||
"dealAmount": 1500000.0,
|
"dealscount": "30",
|
||||||
"dealDate": "2024-01-15",
|
|
||||||
"settlementNameHeb": "תל אביב-יפו",
|
"settlementNameHeb": "תל אביב-יפו",
|
||||||
|
"streetNameHeb": "דיזנגוף",
|
||||||
|
"houseNum": 50,
|
||||||
"polygon_id": "123-456"
|
"polygon_id": "123-456"
|
||||||
}
|
}
|
||||||
]
|
]
|
||||||
@@ -148,11 +150,11 @@ class TestGovmapClient:
|
|||||||
client = GovmapClient()
|
client = GovmapClient()
|
||||||
result = client.get_deals_by_radius((3870000.123, 3770000.456), radius=50)
|
result = client.get_deals_by_radius((3870000.123, 3770000.456), radius=50)
|
||||||
|
|
||||||
# Now returns List[Deal]
|
# Now returns List[Dict] - polygon metadata, not Deal objects
|
||||||
assert len(result) == 1
|
assert len(result) == 1
|
||||||
assert isinstance(result[0], Deal)
|
assert isinstance(result[0], dict)
|
||||||
assert result[0].objectid == 12345
|
assert result[0]["objectid"] == 12345
|
||||||
assert result[0].settlement_name_heb == "תל אביב-יפו"
|
assert result[0]["polygon_id"] == "123-456"
|
||||||
mock_session.get.assert_called_once()
|
mock_session.get.assert_called_once()
|
||||||
|
|
||||||
@patch('requests.Session')
|
@patch('requests.Session')
|
||||||
@@ -245,9 +247,9 @@ class TestGovmapClient:
|
|||||||
]
|
]
|
||||||
)
|
)
|
||||||
|
|
||||||
# Mock radius response - now returns List[Deal]
|
# Mock radius response - now returns List[Dict] (polygon metadata)
|
||||||
mock_radius.return_value = [
|
mock_radius.return_value = [
|
||||||
Deal(objectid=1, deal_amount=1500000, deal_date="2025-01-01", polygon_id="123-456")
|
{"objectid": 1, "dealscount": "10", "polygon_id": "123-456", "settlementNameHeb": "Tel Aviv"}
|
||||||
]
|
]
|
||||||
|
|
||||||
# Mock street deals response - now returns List[Deal]
|
# Mock street deals response - now returns List[Deal]
|
||||||
|
|||||||
Reference in New Issue
Block a user