Commit Graph

24 Commits

Author SHA1 Message Date
Nitzan Pomerantz 8e591423da Change default IQR multiplier to 1.0 for more aggressive outlier filtering
- 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>
2025-11-28 22:40:29 +02:00
Nitzan P 1bc94e39ba Add detailed logging for filtering stages in get_valuation_comparables
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>
2025-11-27 00:15:33 +02:00
Nitzan P 6882d7ab71 Add outlier filtering metadata to get_valuation_comparables response
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>
2025-11-26 23:45:56 +02:00
Nitzan P 623d79947b Add logging for MCP tool function calls with parameters
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>
2025-11-26 23:24:23 +02:00
Nitzan P aa3639b05c Fix: Make _safe_calculate_metric catch all exceptions
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>
2025-11-24 22:54:03 +02:00
Nitzan P a25e0e406a Fix: Apply outlier filtering to deals list in get_valuation_comparables
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>
2025-11-24 22:51:01 +02:00
Nitzan P e791d3c1ed Normalize MCP response structures across all tools
Implement changes from MCP_NORMALIZATION_FIX.md to provide consistent
response structure across all MCP tools. This fixes the bot integration
issue where different tools returned data in different structures.

Changes:
- get_valuation_comparables:
  - Rename "comparables" → "deals"
  - Move "total_comparables" → "market_statistics.deal_breakdown.total_deals"
  - Add "search_parameters" section with "filters_applied"
  - Move "statistics" → "market_statistics"

- find_recent_deals_for_address:
  - Rename "price_stats.average_price" → "price_statistics.mean"
  - Rename "price_stats.median_price" → "price_statistics.median"
  - Rename "area_stats" → "area_statistics"
  - Rename "price_per_sqm_stats" → "price_per_sqm_statistics"

- get_deal_statistics:
  - Add "search_parameters" section
  - Move "statistics" → "market_statistics"
  - Add "market_statistics.deal_breakdown.total_deals"

- analyze_market_trends:
  - Add "market_statistics.deal_breakdown.total_deals"
  - Keep existing tool-specific fields (yearly_trends, etc.)

- get_market_activity_metrics:
  - Add "market_statistics.deal_breakdown.total_deals"
  - Keep existing tool-specific metrics

All tools now follow standard structure:
{
  "search_parameters" or "analysis_parameters": {...},
  "market_statistics": {
    "deal_breakdown": {"total_deals": N},
    "price_statistics": {"mean": ..., "median": ...},
    "area_statistics": {...},
    "price_per_sqm_statistics": {...}
  },
  "deals": [...]
}

Updated tests to match new normalized structure.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
2025-11-23 09:49:52 +02:00
Nitzan P 87b0a355f8 Add HTTP transport support for cloud deployment
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>
2025-11-17 09:43:18 +02:00
Nitzan Pomerantz e4aa6487ff Ruff fixes 2025-10-30 22:24:40 +02:00
Nitzan Pomerantz 247ddad8cd CRITICAL FIX: get_deals_by_radius returns polygon metadata, not deals
## 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>
2025-10-27 09:38:21 +02:00
Nitzan Pomerantz ff8c7e6509 Complete Phase 4.1 test suite updates - all 174 tests passing
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>
2025-10-26 23:55:42 +02:00
Nitzan Pomerantz 0ac9d136cd Implementation of phase 4.1 2025-10-26 10:58:46 +02:00
Nitzan Pomerantz 4d9febf527 Fix autocomplete_address bug and add test coverage report
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>
2025-10-25 13:45:52 +03:00
Nitzan Pomerantz fde0afa7a1 Apply suggestion from @gemini-code-assist[bot]
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
2025-10-25 00:43:26 +03:00
Nitzan Pomerantz b62815ec08 Fix: Strip bloat fields to resolve MCP token limit issues
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>
2025-10-25 00:40:31 +03:00
Nitzan Pomerantz ac6c780419 Improve fastmcp_server error handling and code quality
Refactor repetitive try-except blocks using _safe_calculate_metric helper
function to reduce code duplication and improve maintainability.

Changes:
- Add _safe_calculate_metric helper to centralize error handling
- Remove misleading default values (0/"unknown") in summary fields
- Use None defaults instead to clearly indicate unavailable data
- Add final newline to file per convention

This prevents LLMs from misinterpreting 0 as "zero investment potential"
when it actually means "data unavailable".

Addresses PR #2 review comments on lines 733, 749, and 759.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
2025-10-24 19:26:52 +03:00
Nitzan Pomerantz 2968711307 Complete Phase 2: Market Analysis, Filtering & Documentation
This commit implements all Phase 2 functionality with architectural
improvements over the original plan.

## Phase 2.1: Property Valuation Data 
- filter_deals_by_criteria() with comprehensive filtering
- calculate_deal_statistics() for statistical aggregations
- _extract_floor_number() for Hebrew floor parsing
- _calculate_std_dev() helper function
- MCP tools: get_valuation_comparables, get_deal_statistics

## Phase 2.2: Market Activity & Investment Analysis 
- calculate_market_activity_score() - deal frequency & velocity
  * Activity score (0-100), trend analysis, monthly distribution
  * Classifies markets: very_high, high, moderate, low, very_low
- analyze_investment_potential() - price trends & stability
  * Price appreciation rate via linear regression
  * Volatility score using coefficient of variation
  * Investment score combining appreciation & stability
- get_market_liquidity() - turnover & liquidity metrics
  * Quarterly/monthly breakdowns, velocity scoring
  * Trend direction, most active periods
- MCP tool: get_market_activity_metrics (unified tool)

## Phase 2.3: Enhanced Deal Filtering 
- Property type, room count, price, area, floor filtering
- All integrated into existing tools
- Hebrew floor number parsing support

## Testing 
- Added 15 comprehensive unit tests (all passing)
- Coverage: market activity, investment analysis, liquidity, filtering
- Edge cases: empty data, invalid dates, insufficient data

## Documentation 
- Created CLAUDE.md (~250 lines) - AI agent guidance
  * Development commands, architecture overview
  * Product vision from USECASES.md
  * Available tools with status indicators
- Updated TASKS.md - Phase 2 marked 100% complete

## Architectural Improvements
- 1 unified MCP tool instead of 6 separate tools (simpler API)
- 1 flexible filtering function instead of 3 (more composable)
- All logic in govmap.py (no new files, better cohesion)
- ~955 lines added with comprehensive documentation

## Design Principles Followed
 MCP provides data, LLM provides intelligence
 No predictions - only statistical calculations
 Comprehensive error handling & input validation
 Well-documented with detailed docstrings

Phase 2 Progress: 100% complete (60% overall project completion)

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
2025-10-24 18:51:50 +03:00
Nitzan Pomerantz 85f52a8108 Main updates of Phase 1 2025-10-19 00:58:46 +03:00
Nitzan Pomerantz 9ee5372b1c Another iteration 2025-07-14 14:26:53 +03:00
Nitzan Pomerantz 650048445f Optimization because of long running times 2025-07-14 13:40:36 +03:00
Nitzan Pomerantz 80dfaa28f0 Improving parameters 2025-07-14 13:12:44 +03:00
Nitzan Pomerantz 4f3b74ceaf Refactor fastmcp 2025-07-13 01:05:16 +03:00
Nitzan Pomerantz a1fa75a878 mcp clean up 2025-07-13 00:56:47 +03:00
Nitzan Pomerantz d8c18cb2f7 mcp second iteration 2025-07-13 00:50:51 +03:00