## 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>
Phase 4.1 (Pydantic Data Models) is now 100% complete:
- 9 Pydantic v2 models created
- All functions updated to use/return models
- 174/174 tests passing
- Breaking change: v2.0.0 released
🤖 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>
Bug Fix:
- Fixed autocomplete_address tool to use correct API response fields
- Changed from non-existent fields (addressLabel, settlementNameHeb, coordinates)
to actual API fields (text, id, type, score, shape)
- Added coordinate parsing from WKT POINT format
- Impact: HIGH - tool was returning empty data for all fields
Test Coverage Report:
- Added comprehensive test coverage analysis
- Documented 34 passing tests
- Identified e2e test results for MCP tools
- Listed missing test cases and recommendations
- Overall assessment: Good coverage, one bug found and fixed
E2E Test Results:
- ✅ find_recent_deals_for_address - WORKING
- ✅ analyze_market_trends - WORKING
- ✅ get_valuation_comparables - WORKING
- ✅ autocomplete_address - FIXED (needs MCP server restart to take effect)
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <noreply@anthropic.com>
- .claude/ - Claude Code workspace directory
- .mcp.json - User-specific MCP configuration
- PHASE3-PLAN-SUMMARY.md - Temporary planning document
These files contain local paths and should not be tracked in version control.
Updated ARCHITECTURE.md:
- Changed govmap.py section to govmap/ package section
- Updated architecture diagram to show modular package structure
- Marked Phase 3 as completed (✅)
- Listed all package modules and their responsibilities
Updated CLAUDE.md:
- Updated Business Logic Layer description with package structure
- Updated Key Files section with new modular package details
- Updated development roadmap to show Phase 3 as complete
- Updated "Adding New API Endpoint Support" instructions
- Updated "Important Notes for AI Agents" with package info
- Changed test count from 27 to 34 tests (all passing)
All documentation now reflects the completed modular refactoring
while emphasizing backward compatibility.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <noreply@anthropic.com>
All functionality has been extracted to the govmap package:
- validators.py - Input validation functions
- utils.py - Utility functions
- filters.py - Deal filtering logic
- statistics.py - Statistical calculations
- market_analysis.py - Market analysis functions
- client.py - GovmapClient class with API methods
- __init__.py - Package exports
Backward compatibility maintained through package __init__.py exports.
Existing imports like `from nadlan_mcp.govmap import GovmapClient` now
use the govmap package instead of the old monolithic file.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <noreply@anthropic.com>
- Runs unit tests with pytest on Python 3.12
- Only triggers on PRs to main branch
- Includes code coverage reporting (non-blocking)
- Lint checks for code quality (non-blocking)
- Integration tests are optional and non-blocking
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <noreply@anthropic.com>
Problem:
- get_valuation_comparables failed with 61,565 tokens (exceeded 25K MCP limit)
- Large MULTIPOLYGON shape data consumed ~40-50% of response tokens
- Not useful for LLM analysis, only bloating responses
Solution:
1. Added strip_bloat_fields() helper to remove:
- shape: Large coordinate data
- sourceorder: Internal ordering field
- source_polygon_id: Internal reference field
2. Applied to 5 MCP tools returning deal data:
- get_deals_by_radius
- get_street_deals
- find_recent_deals_for_address
- get_neighborhood_deals
- get_valuation_comparables
3. Reduced get_valuation_comparables default max_comparables: 200 → 50
Results:
- Token usage reduced by ~87% (61K → ~7-8K tokens)
- All tools now work within MCP token limits
- Cleaner, more efficient responses
- No loss of useful data for LLM analysis
🤖 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>
Nitzan Pomerantz