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