Rationale: 50% threshold too permissive for Israeli real estate market.
Within same room count/area, legitimate price variance typically ±30-35%, not 50%.
Example: 13,402 NIS/sqm deal (47% below 25,120 median) was passing with 50% threshold.
This is almost certainly data error or partial deal, not legitimate market variation.
Changes:
- ANALYSIS_PERCENTAGE_THRESHOLD default: 0.5 → 0.4 (40%)
- Update CLAUDE.md docs to reflect 40% threshold
Impact:
- New lower bound: median × 0.6 (was median × 0.5)
- New upper bound: median × 1.4 (was median × 1.5)
- Tighter filtering while preserving legitimate high/low-end deals
- Better aligned with actual Israeli real estate market variance
Tests: All 326 tests pass
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <noreply@anthropic.com>
- Change ANALYSIS_IQR_MULTIPLIER default from 1.5 to 1.0 in config.py
- Add iqr_multiplier parameter to all filtering & statistics functions
- Allow runtime override via MCP tools (get_valuation_comparables, get_deal_statistics)
- Update CLAUDE.md docs with new default & override examples
Rationale: k=1.0 catches more suspicious deals (e.g. 43% below median) while
still preserving legitimate edge cases via hard bounds. Users can override
per-call for more conservative filtering (k=1.5) if needed.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <noreply@anthropic.com>
Adds visibility into the deal filtering pipeline to help diagnose
why certain queries return fewer deals than expected.
New logs show:
1. Number of deals before criteria filtering
2. Number of deals after criteria filtering (with count removed)
3. Whether outlier filtering was applied
4. Number of deals after outlier filtering (with count removed)
5. Outlier filtering method and parameters used
Example output:
INFO: Applying criteria filters to 41 deals
INFO: After criteria filtering: 12 deals (removed 29 deals)
INFO: After outlier filtering (iqr, k=1.5): 4 deals (removed 8 outliers)
This helps users and developers understand:
- If criteria filters are too restrictive
- If outlier filtering is too aggressive
- Where deals are being filtered out in the pipeline
Addresses user question: "Why do I get just 4 deals for a central address
like סירקין 16 תל אביב?" - Now they can see exactly where deals were
filtered out.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <noreply@anthropic.com>
Bug: When the Govmap API returns the same polygon_id multiple times
(which can happen for addresses with multiple associated polygons),
the code was processing each duplicate separately, leading to:
- Duplicate API calls for street/neighborhood deals
- Wasted API quota and slower performance
- Confusing logs showing same polygon queried multiple times
Example from logs:
INFO: Querying polygon 53283601 (distance: 0m from search point)
INFO: Getting street deals for polygon: 53283601
INFO: Querying polygon 53283601 (distance: 0m from search point)
INFO: Getting street deals for polygon: 53283601
Root cause:
- nearby_polygons API response can contain duplicate polygon_ids
- Code was appending all polygons without deduplication
- Log message claimed "unique polygon IDs" but didn't enforce it
Solution:
- Use dict to deduplicate polygons by polygon_id (lines 612-643)
- When duplicates exist, keep the one with shortest distance
- Convert dict to list after deduplication
- Now truly have unique polygon IDs as the log claims
Impact:
- Reduces API calls (fewer duplicates = less load on Govmap API)
- Faster queries (less redundant processing)
- Clearer logs (no confusing duplicate entries)
- Better performance for addresses with many associated polygons
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <noreply@anthropic.com>
The test_market_activity_trend_stable test was failing intermittently
because it used get_recent_date() which approximates months as 30 days,
causing inconsistent month boundary calculations depending on when the
test runs.
Root cause:
- Using months_ago * 30 creates shifting month boundaries
- Calendar dates don't align consistently with 30-day periods
- Deals could fall into unexpected months based on test execution date
- Trend calculation compares first half vs second half of months
- Inconsistent month grouping led to "decreasing" instead of "stable"
Solution:
- Use explicit, deterministic dates (15th of each month)
- Calculate year-month combinations going back from current month
- Ensures exactly 1 deal per month for 12 consecutive months
- All deals guaranteed to be within time_period_months=12 window
- Month boundaries are now consistent regardless of test execution date
Before: Test result varied based on current date (flaky)
After: Test result is always "stable" (deterministic)
This ensures the test validates the trend detection logic without
temporal flakiness.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <noreply@anthropic.com>
The MCP now includes detailed outlier filtering information in the response,
allowing LLMs and bots to inform users about data quality improvements.
Changes to nadlan_mcp/fastmcp_server.py:
- Updated get_valuation_comparables to include outlier filtering metadata
- deal_breakdown now includes when filtering is applied:
- total_deals: Count after filtering (final result)
- total_deals_before_filtering: Original count before filtering
- outliers_removed: Number of deals filtered out
- filtering_method: Method used ("iqr", "percent", or "none")
- iqr_multiplier: IQR multiplier when using IQR method (e.g., 1.5)
- Updated docstring to clarify that outlier filtering is automatic
and metadata is included in response
Changes to tests/e2e/test_mcp_tools_comprehensive.py:
- Added assertions to verify outlier filtering metadata fields
- Test now validates the structure and types of filtering information
Example response:
{
"market_statistics": {
"deal_breakdown": {
"total_deals": 15,
"total_deals_before_filtering": 17,
"outliers_removed": 2,
"filtering_method": "iqr",
"iqr_multiplier": 1.5
}
}
}
This allows bots like nadlan-bot to properly inform users:
"Found 17 comparable deals, filtered 2 outliers using IQR method (k=1.5),
showing 15 deals."
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <noreply@anthropic.com>
Added log_mcp_call() helper function that logs all MCP tool invocations
with their parameters at INFO level. This provides:
- Better debugging and monitoring capabilities
- Clear visibility into which tools are being called and with what parameters
- Smart parameter formatting (truncates long strings, summarizes long lists)
- Consistent logging format across all 10 MCP tools
Example log output:
INFO - MCP tool called: find_recent_deals_for_address(address=סוקולוב 38 חולון, years_back=2, radius_meters=30, max_deals=100, deal_type=2)
This helps track API usage patterns and debug issues in production.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <noreply@anthropic.com>
The same-building detection was failing because the code was accessing
deal.street_name and deal.house_number (Pydantic model field names),
but the Govmap API actually returns streetNameHeb/streetNameEng and
houseNum as extra fields.
Modified address construction to check multiple field names using
getattr() to handle the API's actual field names, falling back to
model field names if needed.
This fix ensures same-building deals are correctly identified and
prioritized with priority=0.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <noreply@anthropic.com>
Implements smart deal prioritization based on distance from search point,
ensuring most relevant comparables are selected first.
Problem:
- Queried all polygons equally without distance consideration
- No filtering for deals that are too far away
- Street/neighborhood deals could be from distant locations
- No way to prefer closest polygon when multiple available
Solution:
1. Sort polygons by distance from search point (closest first)
2. Query polygons progressively, stop early if enough deals found
3. Calculate and store distance_meters for each deal
4. Filter deals by configurable distance thresholds:
- Street deals: max 500m (configurable)
- Neighborhood deals: max 1000m (configurable)
5. Sort by priority → distance → date (prefer closer deals)
Changes:
Config (nadlan_mcp/config.py):
- Added max_street_deal_distance_meters (default: 500m)
- Added max_neighborhood_deal_distance_meters (default: 1000m)
Client (nadlan_mcp/govmap/client.py):
- Extract polygon metadata with coordinates
- Sort polygons by distance using utils.calculate_distance()
- Progressive polygon querying (closest first, stop when enough deals)
- Calculate deal distance (use polygon distance as approximation)
- Filter street deals beyond max_street_deal_distance_meters
- Filter neighborhood deals beyond max_neighborhood_deal_distance_meters
- Store distance_meters on each Deal (dynamic attribute)
- Updated sorting: Priority → Distance → Date
Benefits:
- More relevant comparables (from nearby locations)
- Reduced API calls (query fewer polygons, stop early)
- Better performance (fewer deals to process)
- Configurable distance thresholds
- Transparent (distance info available in each deal)
Example Configuration:
export MAX_STREET_DEAL_DISTANCE_METERS=300 # Conservative
export MAX_NEIGHBORHOOD_DEAL_DISTANCE_METERS=500
Testing:
- All 326 tests pass
- Backward compatible (high default limits)
- Works with existing outlier filtering
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <noreply@anthropic.com>
Problem:
- test_get_market_activity_metrics was failing with "list index out of range"
- _safe_calculate_metric only caught ValueError, not IndexError or other exceptions
- This caused the entire function to crash when metric calculations failed
Root Cause:
- Market metric functions (calculate_market_activity_score, get_market_liquidity,
analyze_investment_potential) can throw IndexError if insufficient data
- The helper function wasn't catching these exceptions
Solution:
- Changed _safe_calculate_metric to catch all exceptions (Exception instead of ValueError)
- Added logging to track which metric failed
- Now returns error dict gracefully instead of crashing
Impact:
- get_market_activity_metrics is now more robust
- Returns partial results even if some metrics fail
- All 326 tests now pass
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <noreply@anthropic.com>
Problem:
- get_valuation_comparables calculated statistics with outlier filtering
- But returned a deals list that still contained outliers (e.g., ₪900K, ₪1.3M deals)
- This caused inconsistency between statistics and the actual deals shown
Root Cause:
- calculate_deal_statistics() filters outliers internally for statistics
- But the deals array returned to user was only filtered by criteria (rooms, price, etc.)
- Outlier filtering was not applied to the returned deals list
Solution:
- Added explicit call to filter_deals_for_analysis() after filter_deals_by_criteria()
- Now both statistics AND deals list use outlier-filtered data
- Moved imports to top of file for better practice
Changes:
- nadlan_mcp/fastmcp_server.py:
- Added imports: get_config, filter_deals_for_analysis
- In get_valuation_comparables(): Apply outlier filtering before calculating stats
- Ensures deals list and statistics are consistent
Testing:
- 325/326 tests pass
- 1 unrelated test failure in get_market_activity_metrics (pre-existing issue)
After deployment, outliers will be automatically filtered from valuation comparables.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <noreply@anthropic.com>
The .dockerignore was excluding pyproject.toml, which is needed for
`pip install .` to work after the project was modernized to use
pyproject.toml instead of setup.py.
This was causing Docker builds to fail with:
"failed to calculate checksum: /pyproject.toml: not found"
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <noreply@anthropic.com>
Implement configurable outlier detection and robust statistical measures to
improve analysis accuracy for real estate data. Addresses issues with data
entry errors, partial deals, and other anomalies that skew statistics.
Key Features:
- IQR-based outlier detection (moderate filtering by default, k=1.5)
- Hard bounds filtering for obvious errors (price_per_sqm, deal_amount)
- Robust volatility using IQR instead of std_dev for investment analysis
- Transparent reporting with both filtered and unfiltered statistics
Implementation:
- Add outlier_detection.py module with IQR/percent/hard bounds methods
- Add OutlierReport model and enhance DealStatistics with filtered fields
- Update calculate_deal_statistics() to support optional outlier filtering
- Update analyze_investment_potential() to use robust volatility
- Add 9 new configuration parameters for customization
- Add comprehensive test suite (24 tests) for outlier detection
- Update CLAUDE.md with usage documentation
Configuration (all via env vars):
- ANALYSIS_OUTLIER_METHOD=iqr (default, or percent/none)
- ANALYSIS_IQR_MULTIPLIER=1.5 (moderate, 3.0=conservative)
- ANALYSIS_PRICE_PER_SQM_MIN/MAX=1000/100000 (bounds in NIS/sqm)
- ANALYSIS_MIN_DEAL_AMOUNT=100000 (catches partial deals)
- ANALYSIS_USE_ROBUST_VOLATILITY=true (IQR-based CV)
- ANALYSIS_USE_ROBUST_TRENDS=true (filter before regression)
Testing:
- All existing tests pass (326 passed)
- 24 new comprehensive outlier detection tests
- Real-world scenario tests (partial deals, data errors)
Backward Compatible:
- Default behavior improves accuracy without breaking changes
- All new fields in models are optional
- Config parameters have sensible defaults
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <noreply@anthropic.com>
- Remove requirements.txt and requirements-dev.txt (duplicates pyproject.toml)
- Update Dockerfile to use 'pip install .' instead of requirements.txt
- Update README.md and GitHub workflows to use 'pip install -e .[dev]'
- Fix test_get_valuation_comparables: check for 'deal_date' instead of non-existent 'asset_room_num'
(rooms field is optional and excluded when None due to exclude_none=True)
- Fix fastmcp version constraint in pyproject.toml (>=2.13.0,<3.0.0)
- Add vcrpy to dev dependencies
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <noreply@anthropic.com>
The Deal model's 'rooms' field was missing the 'assetRoomNum' alias,
causing room data from the API to not be loaded into the model.
This resulted in all room-based filters returning 0 results.
Bug discovered when querying "חנקין 62 חולון" for 3-room apartments.
Query returned 0 results despite multiple 3-room deals existing in the data.
Root cause: Pydantic requires field aliases to map API field names (camelCase)
to Python attributes (snake_case). The 'rooms' field had no alias.
Fix: Added alias="assetRoomNum" to rooms field in Deal model.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <noreply@anthropic.com>
Enables deployment to cloud platforms (Render, Railway, etc.) while maintaining
backward compatibility with existing stdio transport for Claude Desktop.
New features:
- HTTP server entry point (run_http_server.py) using uvicorn
- Docker containerization with Python 3.13
- Health check endpoint at /health
- Comprehensive deployment documentation for Render, Railway, and Docker
Technical changes:
- Added uvicorn dependency for ASGI server
- Created Dockerfile with optimized multi-stage build (343MB)
- Added .dockerignore for efficient Docker builds
- Implemented /health endpoint using Starlette JSONResponse
- Updated README.md and DEPLOYMENT.md with HTTP deployment guides
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <noreply@anthropic.com>
Adjusted sample_deals fixture to ensure 5 deals across 5 distinct months.
Previous dates collapsed into 3 months due to naive 30-day calculations.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <noreply@anthropic.com>
Autocomplete returns different coordinates for vague queries (e.g. "תל אביב דיזנגוף"),
sometimes landing on points with no nearby polygon_ids within 30m radius.
- Try top 3 autocomplete results until one has polygons
- Fallback radius expansion (30m→200m) if no polygons found
- Fix limit validation bug (min 1 for deal queries)
Fixes: compare_addresses, get_valuation_comparables, get_deal_statistics
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <noreply@anthropic.com>
## 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>
Nitzan Pomerantz