diff --git a/nadlan_mcp/fastmcp_server.py b/nadlan_mcp/fastmcp_server.py index 61db3d7..a620565 100644 --- a/nadlan_mcp/fastmcp_server.py +++ b/nadlan_mcp/fastmcp_server.py @@ -22,6 +22,28 @@ mcp = FastMCP("nadlan-mcp") # Initialize the Govmap client client = GovmapClient() +def strip_bloat_fields(deals: List[Dict]) -> List[Dict]: + """ + Remove bloat fields from deal objects to reduce token usage in MCP responses. + + Removes: + - shape: Large MULTIPOLYGON coordinate data (~40-50% of tokens, not useful for LLM analysis) + - sourceorder: Internal ordering field + - source_polygon_id: Internal reference field + + Args: + deals: List of deal dictionaries + + Returns: + List of deals with bloat fields removed + """ + bloat_fields = {'shape', 'sourceorder', 'source_polygon_id'} + + return [ + {k: v for k, v in deal.items() if k not in bloat_fields} + for deal in deals + ] + @mcp.tool() def autocomplete_address(search_text: str) -> str: """Search and autocomplete Israeli addresses. @@ -77,7 +99,7 @@ def get_deals_by_radius(latitude: float, longitude: float, radius_meters: int = "total_deals": len(deals), "search_radius_meters": radius_meters, "center_coordinates": {"latitude": latitude, "longitude": longitude}, - "deals": deals + "deals": strip_bloat_fields(deals) }, ensure_ascii=False, indent=2) except Exception as e: @@ -134,15 +156,15 @@ def get_street_deals(polygon_id: str, limit: int = 100, deal_type: int = 2) -> s "deal_type": deal_type, "deal_type_description": deal_type_desc, "market_statistics": stats, - "deals": deals + "deals": strip_bloat_fields(deals) }, ensure_ascii=False, indent=2) - + except Exception as e: logger.error(f"Error in get_street_deals: {e}") return f"Error fetching street deals: {str(e)}" @mcp.tool() -def find_recent_deals_for_address(address: str, years_back: int = 2, radius_meters: int = 30, max_deals: int = 50, deal_type: int = 2) -> str: +def find_recent_deals_for_address(address: str, years_back: int = 2, radius_meters: int = 30, max_deals: int = 100, deal_type: int = 2) -> str: """Find recent real estate deals for a specific address. Args: @@ -150,7 +172,7 @@ def find_recent_deals_for_address(address: str, years_back: int = 2, radius_mete years_back: How many years back to search (default: 2) radius_meters: Search radius in meters from the address (default: 30) Small radius since street deals cover the entire street anyway - max_deals: Maximum number of deals to return (default: 50, optimized for LLM token limits) + max_deals: Maximum number of deals to return (default: 100, provides good context for LLM analysis) deal_type: Deal type filter (1=first hand/new, 2=second hand/used, default: 2) Returns: @@ -221,7 +243,7 @@ def find_recent_deals_for_address(address: str, years_back: int = 2, radius_mete "deal_type_description": deal_type_desc }, "market_statistics": stats, - "deals": deals + "deals": strip_bloat_fields(deals) }, ensure_ascii=False, indent=2) except Exception as e: @@ -278,9 +300,9 @@ def get_neighborhood_deals(polygon_id: str, limit: int = 100, deal_type: int = 2 "deal_type": deal_type, "deal_type_description": deal_type_desc, "market_statistics": stats, - "deals": deals + "deals": strip_bloat_fields(deals) }, ensure_ascii=False, indent=2) - + except Exception as e: logger.error(f"Error in get_neighborhood_deals: {e}") return f"Error fetching neighborhood deals: {str(e)}" @@ -544,13 +566,16 @@ def get_valuation_comparables( min_area: Optional[float] = None, max_area: Optional[float] = None, min_floor: Optional[int] = None, - max_floor: Optional[int] = None + max_floor: Optional[int] = None, + radius_meters: int = 100, + max_comparables: int = 50 ) -> str: """Get comparable properties for valuation analysis. - + This tool provides detailed comparable deals filtered by your criteria. - The LLM can then analyze these comparables and estimate property values. - + Returns a generous number of comparables by default - the LLM analyzing + the results can determine which are most similar based on the full details. + Args: address: The address to find comparables for (in Hebrew or English) years_back: How many years back to search (default: 2) @@ -563,13 +588,21 @@ def get_valuation_comparables( max_area: Maximum asset area (square meters) min_floor: Minimum floor number max_floor: Maximum floor number - + radius_meters: Search radius in meters (default: 100, larger than find_recent_deals to get more comparables) + max_comparables: Maximum number of deals to return (default: 50, optimized for MCP token limits) + Returns: - JSON string containing filtered comparable deals with full details + JSON string containing filtered comparable deals with full details. + Returns many comparables so LLM can assess similarity and relevance. """ try: - # Get all deals for the address - deals = client.find_recent_deals_for_address(address, years_back) + # Get all deals for the address with higher limits for valuation + deals = client.find_recent_deals_for_address( + address, + years_back, + radius=radius_meters, + max_deals=max_comparables + ) if not deals: return json.dumps({ @@ -608,9 +641,9 @@ def get_valuation_comparables( }, "total_comparables": len(filtered_deals), "statistics": stats, - "comparables": filtered_deals + "comparables": strip_bloat_fields(filtered_deals) }, ensure_ascii=False, indent=2) - + except Exception as e: logger.error(f"Error in get_valuation_comparables: {e}") return f"Error getting valuation comparables: {str(e)}" diff --git a/nadlan_mcp/govmap.py b/nadlan_mcp/govmap.py index 8a89432..3ef27b6 100644 --- a/nadlan_mcp/govmap.py +++ b/nadlan_mcp/govmap.py @@ -510,7 +510,7 @@ class GovmapClient: address: str, years_back: int = 2, radius: int = 30, - max_deals: int = 50, + max_deals: int = 100, deal_type: int = 2, ) -> List[Dict[str, Any]]: """ @@ -524,7 +524,7 @@ class GovmapClient: years_back: How many years back to search (default: 2) radius: Search radius in meters for initial coordinate search (default: 30) Small radius since street deals cover the entire street anyway - max_deals: Maximum number of deals to return (default: 50) + max_deals: Maximum number of deals to return (default: 100) deal_type: Deal type filter (1=first hand/new, 2=second hand/used, default: 2) Returns: @@ -622,14 +622,18 @@ class GovmapClient: # Process street deals and separate building deals for deal in current_street_deals: - deal_id = f"{deal.get('dealId', '')}{deal.get('address', '')}{deal.get('dealDate', '')}" + # Create unique deal ID for deduplication + deal_id = f"{deal.get('dealId', '')}{deal.get('dealDate', '')}" if deal_id not in seen_deals: seen_deals.add(deal_id) deal["source_polygon_id"] = polygon_id deal["deal_source"] = "street" # Check if this is from the same building - deal_address = deal.get("address", "").lower().strip() + # Construct address from API fields (API doesn't have single "address" field) + street = deal.get("streetNameHeb", "") + house_num = str(deal.get("houseNum", "")) + deal_address = f"{street} {house_num}".lower().strip() if self._is_same_building( search_address_normalized, deal_address ): @@ -642,7 +646,8 @@ class GovmapClient: # Add neighborhood deals with lowest priority for deal in current_neighborhood_deals: - deal_id = f"{deal.get('dealId', '')}{deal.get('address', '')}{deal.get('dealDate', '')}" + # Create unique deal ID for deduplication + deal_id = f"{deal.get('dealId', '')}{deal.get('dealDate', '')}" if deal_id not in seen_deals: seen_deals.add(deal_id) deal["source_polygon_id"] = polygon_id @@ -700,6 +705,24 @@ class GovmapClient: logger.error(f"Error in find_recent_deals_for_address: {e}") raise + def _calculate_distance(self, point1: Tuple[float, float], point2: Tuple[float, float]) -> float: + """ + Calculate Euclidean distance between two points in ITM coordinates. + + ITM (Israeli Transverse Mercator) uses meters as units, so Euclidean + distance provides accurate results for distances within Israel. + + Args: + point1: (longitude, latitude) in ITM + point2: (longitude, latitude) in ITM + + Returns: + Distance in meters + """ + dx = point2[0] - point1[0] + dy = point2[1] - point1[1] + return (dx * dx + dy * dy) ** 0.5 + def _is_same_building(self, search_address: str, deal_address: str) -> bool: """ Check if a deal is from the same building as the search address. @@ -819,12 +842,24 @@ class GovmapClient: deal_type = deal.get( "propertyTypeDescription", deal.get("assetTypeHeb", "") ) - if property_type.lower() not in deal_type.lower(): + # Skip deals with missing property type data when filter is active + if not deal_type: + continue + + # Normalize both strings for flexible matching + property_type_normalized = property_type.lower().strip() + deal_type_normalized = deal_type.lower().strip() + + # Check if the filter term appears in the deal type + # This allows "דירה" to match "דירת גג", "דירה בבניין", etc. + if property_type_normalized not in deal_type_normalized: continue # Room count filter - rooms = deal.get("assetRoomNum") - if rooms is not None: + if min_rooms is not None or max_rooms is not None: + rooms = deal.get("assetRoomNum") + if rooms is None: + continue # Skip deals with missing room data when filter is active try: rooms = float(rooms) if min_rooms is not None and rooms < min_rooms: @@ -832,11 +867,13 @@ class GovmapClient: if max_rooms is not None and rooms > max_rooms: continue except (TypeError, ValueError): - pass # Skip deals with invalid room data + continue # Skip deals with invalid room data when filter is active # Price filter - price = deal.get("dealAmount") - if price is not None: + if min_price is not None or max_price is not None: + price = deal.get("dealAmount") + if price is None: + continue # Skip deals with missing price data when filter is active try: price = float(price) if min_price is not None and price < min_price: @@ -844,11 +881,13 @@ class GovmapClient: if max_price is not None and price > max_price: continue except (TypeError, ValueError): - pass # Skip deals with invalid price data + continue # Skip deals with invalid price data when filter is active # Area filter - area = deal.get("assetArea") - if area is not None: + if min_area is not None or max_area is not None: + area = deal.get("assetArea") + if area is None: + continue # Skip deals with missing area data when filter is active try: area = float(area) if min_area is not None and area < min_area: @@ -856,7 +895,7 @@ class GovmapClient: if max_area is not None and area > max_area: continue except (TypeError, ValueError): - pass # Skip deals with invalid area data + continue # Skip deals with invalid area data when filter is active # Floor filter floor_str = deal.get("floorNo", "") diff --git a/tests/test_govmap_client.py b/tests/test_govmap_client.py index ed844f8..14246ca 100644 --- a/tests/test_govmap_client.py +++ b/tests/test_govmap_client.py @@ -6,6 +6,7 @@ import pytest import requests from unittest.mock import Mock, patch from nadlan_mcp.govmap import GovmapClient +from nadlan_mcp.config import GovmapConfig class TestGovmapClient: @@ -22,7 +23,8 @@ class TestGovmapClient: def test_client_initialization_with_custom_url(self): """Test that GovmapClient can be initialized with custom URL.""" custom_url = "https://custom-api.example.com/api/" - client = GovmapClient(custom_url) + custom_config = GovmapConfig(base_url=custom_url) + client = GovmapClient(custom_config) assert client.base_url == "https://custom-api.example.com/api" @patch('requests.Session') @@ -226,26 +228,31 @@ class TestGovmapClient: "dealId": "deal1", "dealAmount": 1000000, "dealDate": "2025-01-01T00:00:00.000Z", - "address": "Test Street 1" + "address": "Test Street 1", + "priority": 1 } ] - + # Mock neighborhood deals response mock_neighborhood.return_value = [ { "dealId": "deal2", "dealAmount": 2000000, "dealDate": "2025-01-15T00:00:00.000Z", - "address": "Test Street 2" + "address": "Test Street 2", + "priority": 2 } ] client = GovmapClient() result = client.find_recent_deals_for_address("test address", years_back=1) - + assert len(result) == 2 - assert result[0]["dealDate"] == "2025-01-15T00:00:00.000Z" # Should be sorted by date - assert result[1]["dealDate"] == "2025-01-01T00:00:00.000Z" + # Should be sorted by priority first (street=1 before neighborhood=2), then by date + assert result[0]["priority"] == 1 # Street deal comes first + assert result[0]["dealDate"] == "2025-01-01T00:00:00.000Z" + assert result[1]["priority"] == 2 # Neighborhood deal comes second + assert result[1]["dealDate"] == "2025-01-15T00:00:00.000Z" @patch('requests.Session') def test_http_error_handling(self, mock_session_class): @@ -525,3 +532,121 @@ class TestMarketAnalysisFunctions: assert stats["count"] == 3 assert stats["price_stats"]["mean"] > 0 assert stats["area_stats"]["mean"] == pytest.approx(80.0) + + def test_is_same_building_comparisons(self): + """Test `_is_same_building` correctly compares address strings.""" + client = GovmapClient() + + # Test that _is_same_building works with addresses constructed from API fields + search_address = "חנקין 62" + + # Deal from same building (should match) + deal_address_same = "חנקין 62" + assert client._is_same_building(search_address, deal_address_same) is True + + # Deal from different building on same street (should not match) + deal_address_different = "חנקין 50" + assert client._is_same_building(search_address, deal_address_different) is False + + # Deal from different street (should not match) + deal_address_other_street = "בילינסון 6" + assert client._is_same_building(search_address, deal_address_other_street) is False + + def test_filter_excludes_missing_property_type(self): + """Test that deals with missing property type are excluded when filter is active.""" + client = GovmapClient() + deals = [ + {"dealId": "1", "propertyTypeDescription": "דירה"}, + {"dealId": "2", "propertyTypeDescription": None}, + {"dealId": "3", "propertyTypeDescription": "בית"}, + {"dealId": "4"}, # Missing key entirely + ] + + filtered = client.filter_deals_by_criteria(deals, property_type="דירה") + + assert len(filtered) == 1 + assert filtered[0]["dealId"] == "1" + + def test_filter_excludes_missing_area(self): + """Test that deals with missing area are excluded when area filter is active.""" + client = GovmapClient() + deals = [ + {"dealId": "1", "assetArea": 65}, + {"dealId": "2", "assetArea": None}, + {"dealId": "3", "assetArea": 50}, + {"dealId": "4"}, # Missing key entirely + ] + + filtered = client.filter_deals_by_criteria(deals, min_area=60, max_area=70) + + assert len(filtered) == 1 + assert filtered[0]["dealId"] == "1" + + def test_filter_excludes_missing_rooms(self): + """Test that deals with missing room count are excluded when room filter is active.""" + client = GovmapClient() + deals = [ + {"dealId": "1", "assetRoomNum": 3}, + {"dealId": "2", "assetRoomNum": None}, + {"dealId": "3", "assetRoomNum": 2}, + {"dealId": "4"}, # Missing key entirely + ] + + filtered = client.filter_deals_by_criteria(deals, min_rooms=2.5, max_rooms=4) + + assert len(filtered) == 1 + assert filtered[0]["dealId"] == "1" + + def test_filter_excludes_missing_price(self): + """Test that deals with missing price are excluded when price filter is active.""" + client = GovmapClient() + deals = [ + {"dealId": "1", "dealAmount": 2000000}, + {"dealId": "2", "dealAmount": None}, + {"dealId": "3", "dealAmount": 1500000}, + {"dealId": "4"}, # Missing key entirely + ] + + filtered = client.filter_deals_by_criteria(deals, min_price=1800000, max_price=2200000) + + assert len(filtered) == 1 + assert filtered[0]["dealId"] == "1" + + def test_filter_excludes_invalid_numeric_data(self): + """Test that deals with invalid numeric data are excluded when filter is active.""" + client = GovmapClient() + deals = [ + {"dealId": "1", "assetArea": 65, "assetRoomNum": 3, "dealAmount": 2000000}, + {"dealId": "2", "assetArea": "invalid", "assetRoomNum": 3, "dealAmount": 2000000}, + {"dealId": "3", "assetArea": 65, "assetRoomNum": "bad", "dealAmount": 2000000}, + {"dealId": "4", "assetArea": 65, "assetRoomNum": 3, "dealAmount": "wrong"}, + ] + + # Area filter should exclude deal 2 + filtered_area = client.filter_deals_by_criteria(deals, min_area=60, max_area=70) + assert len(filtered_area) == 3 + assert all(d["dealId"] in ["1", "3", "4"] for d in filtered_area) + + # Room filter should exclude deal 3 + filtered_rooms = client.filter_deals_by_criteria(deals, min_rooms=2, max_rooms=4) + assert len(filtered_rooms) == 3 + assert all(d["dealId"] in ["1", "2", "4"] for d in filtered_rooms) + + # Price filter should exclude deal 4 + filtered_price = client.filter_deals_by_criteria(deals, min_price=1500000, max_price=2500000) + assert len(filtered_price) == 3 + assert all(d["dealId"] in ["1", "2", "3"] for d in filtered_price) + + def test_filter_allows_missing_data_when_no_filter(self): + """Test that deals with missing data pass through when no filter is active for that field.""" + client = GovmapClient() + deals = [ + {"dealId": "1", "propertyTypeDescription": "דירה", "assetArea": 65}, + {"dealId": "2", "propertyTypeDescription": "דירה", "assetArea": None}, + {"dealId": "3", "propertyTypeDescription": "דירה"}, # Missing assetArea entirely + ] + + # Filter by property type only - missing area should pass through + filtered = client.filter_deals_by_criteria(deals, property_type="דירה") + + assert len(filtered) == 3 # All should pass since we're not filtering by area