## 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>
Fixed all remaining test failures after Pydantic v2 migration:
Core fixes:
- Date handling: Convert date objects to ISO strings across 4 files
- Model serialization: Use model_dump(mode='json') for JSON compatibility
- Optional fields: Made time_period_months Optional[int] in models
- Dict access: Replace .get() with getattr() for dynamic attributes
Test updates:
- Updated 50+ test fixtures from dicts to Deal models
- Fixed date-based tests to use recent dates for time filtering
- Added missing imports (CoordinatePoint, MarketActivityScore, DealStatistics)
- Updated assertions from dict keys to model attributes (snake_case)
Files modified:
- nadlan_mcp/govmap/market_analysis.py
- nadlan_mcp/govmap/statistics.py
- nadlan_mcp/govmap/models.py
- nadlan_mcp/fastmcp_server.py
- tests/test_govmap_client.py
- tests/test_fastmcp_tools.py
- .cursor/plans/TEST-UPDATE-STATUS.md (comprehensive documentation)
Result: 174/174 tests passing (100%) ✅🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <noreply@anthropic.com>
Fixed two critical bugs in filter_deals_by_criteria():
Bug #1: CRASH on None property type
- Issue: Code called .lower() on None when propertyTypeDescription was null
- Impact: Would crash MCP tool in production with incomplete data
- Fix: Check if deal_type is None/empty before calling .lower()
Bug #2: Missing data passes through filters
- Issue: Deals with None/missing values passed filters when they shouldn't
- Example: Filtering by area=60-70 would include deals with assetArea=None
- Impact: get_valuation_comparables returned inflated results with bad data
- Fix: Explicitly check for None when filter is active and exclude those deals
Changes:
- Property type filter: Check for None/empty before normalization
- Area filter: Exclude deals with missing area when min/max_area specified
- Room filter: Exclude deals with missing rooms when min/max_rooms specified
- Price filter: Exclude deals with missing price when min/max_price specified
- Invalid data: Changed from 'pass' to 'continue' to exclude bad data
Added 6 comprehensive unit tests:
- test_filter_excludes_missing_property_type
- test_filter_excludes_missing_area
- test_filter_excludes_missing_rooms
- test_filter_excludes_missing_price
- test_filter_excludes_invalid_numeric_data
- test_filter_allows_missing_data_when_no_filter
This fixes the E2E issue where get_valuation_comparables was returning
incomplete results. Now filters properly exclude deals with missing data.
Test results: 34/34 passing (6 new tests added)
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <noreply@anthropic.com>
Fixed both failing tests for 100% test coverage (28/28 passing):
Test 1: test_client_initialization_with_custom_url
- Issue: Test was passing string to GovmapClient constructor
- Fix: Import GovmapConfig and create proper config object
- Changes: Added import and wrapped custom_url in GovmapConfig
Test 2: test_find_recent_deals_for_address_integration
- Issue: Test expected date-first sorting, but function sorts by priority first
- Fix: Updated test expectations to match actual business logic
- Behavior: Results sorted by source priority (building > street > neighborhood), then date
- Changes: Added priority field to mock data and fixed assertions
All 28 tests now passing ✅🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <noreply@anthropic.com>
Fixes three critical bugs discovered during live testing with Claude:
Bug #1: Same building detection always returned 0
- Root cause: deal.get('address') returned empty string
- Fix: Construct address from streetNameHeb + houseNum fields
- Added test: test_same_building_detection_with_api_fields
Bug #2: Property type filter too strict
- Root cause: Exact match failed for variants like 'דירת גג'
- Fix: Use flexible substring matching with normalization
Bug #3: Deal deduplication used non-existent field
- Root cause: Deduplication key referenced deal.get('address')
- Fix: Removed non-existent field from deduplication key
Also added _calculate_distance() helper for future radius filtering.
Test results: 26/28 passing (2 pre-existing failures unrelated)
References: .cursor/plans/MCP-E2E-TEST-SUMMARY.md
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <noreply@anthropic.com>
Update tests to pass time_period_months=None for tests using old dates
(2022-2023) to disable time filtering. This is necessary because the
time_period_months parameter is now properly implemented and filters
out old deals by default.
Tests fixed:
- test_calculate_market_activity_score_success
- test_calculate_market_activity_score_high_activity
- test_get_market_liquidity_success
- test_get_market_liquidity_quarterly_breakdown
All market analysis tests now pass (25/27 tests passing, 2 pre-existing
failures unrelated to our changes).
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <noreply@anthropic.com>
Replace broad assertion (mean > 0) with specific expected value using
pytest.approx for floating-point comparison. This makes the test more
robust and prevents future regressions.
Changes:
- Change area_stats mean assertion from > 0 to == pytest.approx(80.0)
- Add final newline to file per convention
Expected mean: (80 + 90 + 70) / 3 = 80.0
Addresses PR #2 review comments on line 527.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <noreply@anthropic.com>
Nitzan Pomerantz