From 34af8362b94c57341c747b05187772649ec2b3a1 Mon Sep 17 00:00:00 2001
From: Nitzan Pomerantz <9297302+nitzpo@users.noreply.github.com>
Date: Sun, 26 Oct 2025 23:26:27 +0200
Subject: [PATCH] CR fixes

---
 nadlan_mcp/govmap/models.py     | 22 +++++++++++------
 nadlan_mcp/govmap/statistics.py |  9 +++++--
 tests/govmap/test_models.py     | 44 +++++++++++++++++++++------------
 3 files changed, 50 insertions(+), 25 deletions(-)

diff --git a/nadlan_mcp/govmap/models.py b/nadlan_mcp/govmap/models.py
index b82505d..877e575 100644
--- a/nadlan_mcp/govmap/models.py
+++ b/nadlan_mcp/govmap/models.py
@@ -6,7 +6,7 @@ Israeli real estate MCP system. Models provide validation, serialization,
 and type safety throughout the codebase.
 """
 
-from datetime import datetime
+from datetime import date, datetime
 from typing import Any, Dict, List, Optional
 from pydantic import BaseModel, Field, field_validator, computed_field, ConfigDict
 
@@ -105,7 +105,7 @@ class Deal(BaseModel):
     # Required fields
     objectid: int = Field(..., description="Unique deal identifier")
     deal_amount: float = Field(..., alias="dealAmount", description="Transaction amount in NIS")
-    deal_date: str = Field(..., alias="dealDate", description="Transaction date (ISO format)")
+    deal_date: date = Field(..., alias="dealDate", description="Transaction date")
 
     # Common optional fields
     asset_area: Optional[float] = Field(None, alias="assetArea", description="Property area in sqm")
@@ -150,13 +150,21 @@ class Deal(BaseModel):
 
     @field_validator('deal_date', mode='before')
     @classmethod
-    def parse_deal_date(cls, v: Any) -> str:
-        """Parse deal date to ISO format string."""
-        if isinstance(v, str):
+    def parse_deal_date(cls, v: Any) -> date:
+        """Parse deal date string into a date object."""
+        if isinstance(v, date):
             return v
         if isinstance(v, datetime):
-            return v.isoformat()
-        return str(v)
+            return v.date()
+        if isinstance(v, str):
+            # Handle ISO format with optional time and timezone
+            if 'T' in v:
+                v = v.split('T')[0]
+            try:
+                return date.fromisoformat(v)
+            except ValueError:
+                raise ValueError(f"Invalid date format: {v}")
+        raise TypeError(f"Unsupported type for date parsing: {type(v)}")
 
 
 class DealStatistics(BaseModel):
diff --git a/nadlan_mcp/govmap/statistics.py b/nadlan_mcp/govmap/statistics.py
index ff5bc07..598fcc6 100644
--- a/nadlan_mcp/govmap/statistics.py
+++ b/nadlan_mcp/govmap/statistics.py
@@ -6,10 +6,13 @@ This module provides pure mathematical functions for analyzing real estate deal
 
 from collections import Counter
 from typing import List
-from datetime import datetime
+import logging
+from datetime import date
 
 from .models import Deal, DealStatistics
 
+logger = logging.getLogger(__name__)
+
 
 def calculate_deal_statistics(deals: List[Deal]) -> DealStatistics:
     """
@@ -127,6 +130,7 @@ def calculate_deal_statistics(deals: List[Deal]) -> DealStatistics:
                         date_str = date_str.split('T')[0]
                     parsed_dates.append(date_str)
                 except (ValueError, TypeError):
+                    logger.warning(f"Invalid date format: {date_str}")
                     continue
 
             if parsed_dates:
@@ -135,7 +139,8 @@ def calculate_deal_statistics(deals: List[Deal]) -> DealStatistics:
                     "earliest": sorted_dates[0],
                     "latest": sorted_dates[-1],
                 }
-        except:
+        except (ValueError, TypeError):
+            logger.warning("Invalid date format: {date_range_dict}")
             pass
 
     return DealStatistics(
diff --git a/tests/govmap/test_models.py b/tests/govmap/test_models.py
index a8e75e2..05db86a 100644
--- a/tests/govmap/test_models.py
+++ b/tests/govmap/test_models.py
@@ -425,23 +425,35 @@ class TestDealFilters:
 class TestModelIntegration:
     """Integration tests for models working together."""
 
-    def test_deal_to_statistics_workflow(self):
-        """Test creating deals and calculating statistics."""
-        deals = [
-            Deal(
-                objectid=i,
-                deal_amount=1500000.0 + (i * 10000),
-                deal_date=f"2024-01-{i+1:02d}",
-                asset_area=80.0 + i,
-                property_type_description="דירה"
-            )
-            for i in range(10)
-        ]
+    def test_deal_to_statistics_workflow(self):  
+        """Test creating deals and calculating statistics."""  
+        # Import the function to test integration  
+        from nadlan_mcp.govmap.statistics import calculate_deal_statistics  
 
-        # Verify all deals have price_per_sqm
-        for deal in deals:
-            assert deal.price_per_sqm is not None
-            assert deal.price_per_sqm > 0
+        deals = [  
+            Deal(  
+                objectid=1,  
+                deal_amount=1000000.0,  
+                deal_date="2024-01-01",  
+                asset_area=100.0,  
+                property_type_description="דירה"  
+            ),  
+            Deal(  
+                objectid=2,  
+                deal_amount=2000000.0,  
+                deal_date="2024-01-02",  
+                asset_area=100.0,  
+                property_type_description="דירה"  
+            ),  
+        ]  
+
+        stats = calculate_deal_statistics(deals)  
+
+        assert isinstance(stats, DealStatistics)  
+        assert stats.total_deals == 2  
+        assert stats.price_statistics["mean"] == 1500000.0  
+        assert stats.price_per_sqm_statistics["mean"] == 15000.0  
+        assert stats.property_type_distribution["דירה"] == 2 
 
     def test_autocomplete_to_deals_workflow(self):
         """Test autocomplete response leading to deal search."""