482 lines
16 KiB
Markdown
482 lines
16 KiB
Markdown
<!-- f18f82e2-05d6-4de1-bb9b-9ca39dca90fc 9249b19a-20d4-4473-be7a-f0b5bf9a0301 -->
|
|
# Nadlan MCP Comprehensive Improvement Plan
|
|
|
|
## Phase 1: Critical Code Quality & Reliability Fixes
|
|
|
|
### 1.1 Error Handling & Resilience
|
|
|
|
**Files: `nadlan_mcp/govmap.py`, `nadlan_mcp/fastmcp_server.py`**
|
|
|
|
- Add retry logic with exponential backoff for API calls (use `tenacity` library)
|
|
- Implement rate limiting protection (track request counts, add delays)
|
|
- Standardize error handling: all GovmapClient methods should raise exceptions (not return empty lists)
|
|
- Add proper input validation/sanitization for all functions
|
|
- Add timeout configuration for HTTP requests
|
|
|
|
**Issues identified:**
|
|
|
|
- Line 111-140 in `govmap.py`: `get_deals_by_radius` catches JSONDecodeError and returns `[]` instead of raising
|
|
- Line 180-185 in `govmap.py`: `get_street_deals` catches JSONDecodeError and returns `[]` instead of raising
|
|
- Similar pattern in `get_neighborhood_deals`
|
|
- No retry logic anywhere
|
|
- No rate limiting protection
|
|
|
|
### 1.2 Configuration Management
|
|
|
|
**New file: `nadlan_mcp/config.py`**
|
|
|
|
- Create configuration class with all settings (API base URL, timeouts, retry counts, rate limits)
|
|
- Support environment variables for configuration
|
|
- Add validation for configuration values
|
|
- Document all configuration options
|
|
|
|
**Currently hard-coded values:**
|
|
|
|
- API base URL in `govmap.py:19`
|
|
- Default timeouts, radii, limits scattered throughout
|
|
- No environment variable support beyond basic setup
|
|
|
|
### 1.3 Documentation Alignment
|
|
|
|
**Files: `USECASES.md`, `README.md`**
|
|
|
|
- Update USECASES.md to clearly mark amenity scoring as "Future Feature" with roadmap
|
|
- Add architecture diagram showing MCP → Govmap API flow
|
|
- Create ARCHITECTURE.md documenting design decisions
|
|
- Update README.md with current accurate feature list
|
|
- Add API limitations and rate limit documentation
|
|
|
|
**Misalignments found:**
|
|
|
|
- USECASES.md lines 23-28, 51-56, 66-69 describe amenity features that don't exist
|
|
- README mentions features not fully implemented
|
|
|
|
## Phase 2: Missing Core Functionality
|
|
|
|
### 2.1 Property Valuation Data Provision
|
|
|
|
**Updates to `nadlan_mcp/govmap.py`, Add tools to `fastmcp_server.py`**
|
|
|
|
**Purpose:** Provide comprehensive data for LLM to perform its own valuation analysis
|
|
|
|
**New Functions:**
|
|
|
|
- `get_comparable_properties(address, filters)` - find similar properties with flexible filtering
|
|
- `calculate_simple_statistics(deals)` - basic math helper for price/sqm calculations (optional utility)
|
|
|
|
**Tools for LLM:**
|
|
|
|
- `get_valuation_comparables` - return detailed comparable deals filtered by criteria
|
|
- Filter by: property type, room count, area range, floor range, time period
|
|
- Include: full deal history, neighborhood data, price trends
|
|
- Let LLM analyze and estimate value
|
|
- `get_deal_statistics` - simple statistical aggregations (mean, median, percentiles)
|
|
- For when LLM needs quick calculations on large datasets
|
|
|
|
**Implementation:**
|
|
|
|
- Enhanced filtering on existing deal data
|
|
- Statistical helper functions (no ML, no predictions)
|
|
- Rich contextual data for LLM decision-making
|
|
- LLM does the valuation, MCP provides the data
|
|
|
|
### 2.2 Market Activity & Investment Analysis
|
|
|
|
**New file: `nadlan_mcp/market_analysis.py`, Add tools to `fastmcp_server.py`**
|
|
|
|
**New Functions:**
|
|
|
|
- `calculate_market_activity_score(address, time_period)` - volume and velocity metrics
|
|
- `analyze_investment_potential(address, criteria)` - ROI, appreciation, risk metrics
|
|
- `get_market_liquidity(neighborhood)` - time-to-sell, supply/demand
|
|
|
|
**Tools for LLM:**
|
|
|
|
- `get_market_activity_metrics` - deal volume, velocity, inventory levels
|
|
- `analyze_investment_opportunity` - comprehensive investment analysis
|
|
- `get_neighborhood_market_health` - market health indicators
|
|
|
|
**Metrics to include:**
|
|
|
|
- Deal volume trends (deals per month)
|
|
- Average time on market (if available)
|
|
- Price appreciation rates
|
|
- Supply/demand indicators (active listings vs deals)
|
|
- Neighborhood investment score (0-100)
|
|
|
|
### 2.3 Enhanced Deal Filtering & Search
|
|
|
|
**Updates to `nadlan_mcp/govmap.py`**
|
|
|
|
**New Functions:**
|
|
|
|
- `filter_deals_by_criteria(deals, criteria)` - filter by rooms, area, price range, property type
|
|
- `search_deals_with_filters(address, filters, years_back)` - combined search with filters
|
|
- `get_deals_by_property_type(address, property_type, years_back)` - specialized property type search
|
|
|
|
**Add to existing functions:**
|
|
|
|
- Property type filtering (apartment, house, penthouse, etc.)
|
|
- Room count filtering (2-room, 3-room, etc.)
|
|
- Price range filtering
|
|
- Area range filtering
|
|
- Floor range filtering
|
|
|
|
## Phase 3: Architecture & Data Model Improvements
|
|
|
|
### 3.1 Create Structured Data Models
|
|
|
|
**New file: `nadlan_mcp/models.py`**
|
|
|
|
Create Pydantic models for:
|
|
|
|
- `Deal` - standardized deal object
|
|
- `Address` - address with coordinates and metadata
|
|
- `MarketAnalysis` - market trend analysis results
|
|
- `PropertyValuation` - valuation estimates
|
|
|
|
**Benefits:**
|
|
|
|
- Type safety and validation
|
|
- Consistent JSON serialization
|
|
- Better IDE support
|
|
- Easier testing
|
|
|
|
### 3.2 Separate Concerns
|
|
|
|
**Refactor existing code:**
|
|
|
|
- Move analysis logic from `fastmcp_server.py` into dedicated modules
|
|
- Create `nadlan_mcp/api_client.py` for pure API interactions
|
|
- Create `nadlan_mcp/analyzers/` package for analysis functions
|
|
- Keep `fastmcp_server.py` thin - just MCP tool definitions
|
|
|
|
**Current issues:**
|
|
|
|
- `fastmcp_server.py` has 537 lines mixing API calls, analysis, and MCP definitions
|
|
- Business logic scattered across client and server
|
|
- Hard to test and maintain
|
|
|
|
### 3.3 Improve Tool Design for LLM Consumption
|
|
|
|
**Update all tools in `fastmcp_server.py`**
|
|
|
|
Each tool should offer:
|
|
|
|
- **Structured mode** (default): Return full structured data for LLM processing
|
|
- **Summarized mode** (optional): Return analyzed/summarized data for simple queries
|
|
- Clear parameter documentation with examples
|
|
- Consistent error responses
|
|
|
|
**Parameter pattern:**
|
|
|
|
```python
|
|
@mcp.tool()
|
|
def find_recent_deals_for_address(
|
|
address: str,
|
|
years_back: int = 2,
|
|
summarized_response: bool = False # Default: structured/detailed
|
|
) -> str:
|
|
"""
|
|
summarized_response:
|
|
- False (default): Full deal data with all fields for LLM processing
|
|
- True: Statistical summary with key insights only
|
|
"""
|
|
```
|
|
|
|
## Phase 4: Testing & Quality Assurance
|
|
|
|
### 4.1 Expand Test Coverage
|
|
|
|
**Updates to `tests/test_govmap_client.py`, new test files**
|
|
|
|
Add:
|
|
|
|
- Integration tests for complete workflows (mark with `@pytest.mark.integration`)
|
|
- Edge case tests (empty results, malformed data, network errors)
|
|
- Parametrized tests for different address formats
|
|
- Tests for new functionality (valuation data, market analysis)
|
|
|
|
**Current coverage gaps:**
|
|
|
|
- No tests for `analyze_market_trends`
|
|
- No tests for `compare_addresses`
|
|
- No tests for error recovery
|
|
- No tests for `_is_same_building` logic
|
|
|
|
### 4.2 Add Validation Tests
|
|
|
|
**New file: `tests/test_validation.py`**
|
|
|
|
- Test input validation (invalid addresses, negative numbers, etc.)
|
|
- Test configuration validation
|
|
- Test data model validation (when Pydantic models added)
|
|
|
|
### 4.3 Mock External APIs
|
|
|
|
**Updates to `tests/conftest.py`**
|
|
|
|
- Create comprehensive fixtures for API responses
|
|
- Add vcr.py for recording/replaying API calls in tests
|
|
|
|
## Phase 5: Documentation & Developer Experience
|
|
|
|
### 5.1 Create Comprehensive Documentation
|
|
|
|
**New files:**
|
|
|
|
- `ARCHITECTURE.md` - system design, data flow, component interactions
|
|
- `DEPLOYMENT.md` - deployment guide for production use
|
|
- `CONTRIBUTING.md` - contribution guidelines
|
|
- `API_REFERENCE.md` - detailed API documentation with examples
|
|
- `CLAUDE.md` - instructions for AI coding agents implementing future tasks
|
|
- Task breakdown format
|
|
- Code standards and patterns
|
|
- Testing requirements
|
|
- Common pitfalls and solutions
|
|
|
|
### 5.2 Improve Code Documentation
|
|
|
|
**All Python files:**
|
|
|
|
- Add module-level docstrings explaining purpose
|
|
- Add comprehensive function docstrings with examples
|
|
- Add type hints to all functions (including returns)
|
|
- Add inline comments for complex logic
|
|
|
|
### 5.3 Add Usage Examples
|
|
|
|
**New directory: `examples/`**
|
|
|
|
Create example scripts:
|
|
|
|
- `examples/basic_search.py` - simple address search
|
|
- `examples/market_analysis.py` - comprehensive market analysis
|
|
- `examples/investment_analysis.py` - investment opportunity analysis
|
|
- `examples/llm_integration.py` - example LLM agent using the MCP
|
|
|
|
### 5.4 Update USECASES.md
|
|
|
|
**File: `USECASES.md`**
|
|
|
|
- Add status indicators for each feature (✅ Implemented, 🚧 In Progress, 📋 Planned)
|
|
- Add implementation priority levels
|
|
- Add expected completion timeline
|
|
- Add links to relevant documentation
|
|
|
|
## Phase 6: Cleanup & Optimization
|
|
|
|
### 6.1 Remove Redundant Code
|
|
|
|
**Files to clean:**
|
|
|
|
- Delete or integrate `mcp_server_concept.py` (appears to be duplicate/old code)
|
|
- Remove unused imports
|
|
- Remove dead code
|
|
- Consolidate duplicate logic
|
|
|
|
### 6.2 Code Style & Linting
|
|
|
|
**New file: `.pre-commit-config.yaml`**
|
|
|
|
- Setup pre-commit hooks (black, isort, flake8, mypy)
|
|
- Format all code consistently
|
|
- Fix all linter warnings
|
|
- Add type checking with mypy
|
|
|
|
### 6.3 Dependency Management
|
|
|
|
**Updates to `requirements.txt`**
|
|
|
|
Current dependencies are minimal. Add:
|
|
|
|
- `tenacity` - for retry logic
|
|
- `pydantic` - for data models
|
|
- `python-dotenv` - already there but ensure proper use
|
|
- `httpx` - for async HTTP (future)
|
|
- Pin all versions with upper bounds for stability
|
|
|
|
Split into:
|
|
|
|
- `requirements.txt` - production dependencies
|
|
- `requirements-dev.txt` - development dependencies (pytest, black, mypy, etc.)
|
|
|
|
## Phase 7: Future Enhancements (Backlog)
|
|
|
|
### 7.1 Amenity Scoring System (Future Feature)
|
|
|
|
**New files: `nadlan_mcp/amenities.py`, Add tools to `fastmcp_server.py`**
|
|
|
|
Implement comprehensive amenity scoring using multiple data sources:
|
|
|
|
**Data Sources:**
|
|
|
|
- Google Places API / OpenStreetMap - basic amenity locations
|
|
- Ministry of Education - school rankings and quality metrics
|
|
- Ministry of Health - healthcare facility ratings
|
|
- CBS (Central Bureau of Statistics) - demographic data
|
|
- Public transport APIs - station locations and frequency
|
|
|
|
**New Functions:**
|
|
|
|
- `get_nearby_amenities(coordinates, radius, amenity_types)` - retrieve raw amenity data
|
|
- `get_school_quality_data(schools)` - fetch education quality metrics
|
|
- `calculate_amenity_score(address, weights)` - score with quality-weighted proximity
|
|
- `compare_amenity_scores(addresses)` - multi-address comparison
|
|
|
|
**Tools for LLM:**
|
|
|
|
- `get_address_amenity_rating` - comprehensive amenity analysis
|
|
- `compare_addresses_by_amenities` - side-by-side comparison
|
|
- `find_amenities_near_address` - raw amenity list with quality data
|
|
|
|
**Implementation approach:**
|
|
|
|
- Combine proximity with quality metrics (not just distance)
|
|
- Weight by category importance (configurable)
|
|
- Include static data sources for institutional quality
|
|
- Return both raw data and computed scores
|
|
|
|
### 7.2 Caching System (Future - Not in MVP)
|
|
|
|
**Task: In-Memory Caching**
|
|
|
|
- Add TTL-based caching for API responses
|
|
- Cache autocomplete results (1 hour TTL)
|
|
- Cache deal results (30 minute TTL)
|
|
- Implement cache invalidation strategy
|
|
- Add cache statistics/monitoring
|
|
|
|
**Task: Production-Ready Caching**
|
|
|
|
- Redis integration for distributed caching
|
|
- Cache warming strategies
|
|
- Cache synchronization across instances
|
|
- Persistent cache storage
|
|
- Cache versioning for schema changes
|
|
|
|
### 7.3 Performance Optimizations (Future)
|
|
|
|
**Task: Async/Parallel Processing**
|
|
|
|
- Convert to async/await for concurrent API calls
|
|
- Use `httpx` for async HTTP requests
|
|
- Parallel processing for multi-polygon queries
|
|
- Batch processing optimizations
|
|
|
|
**Task: Database Integration**
|
|
|
|
- SQLite for local development
|
|
- PostgreSQL for production
|
|
- Historical data tracking
|
|
- Faster query performance for trends
|
|
- Data warehouse for analytics
|
|
|
|
### 7.4 Multi-language Support (Future)
|
|
|
|
**Enhance language support:**
|
|
|
|
- Full English address support (currently Hebrew-focused)
|
|
- Translation services for property descriptions
|
|
- Language detection and auto-switching
|
|
- Multi-language documentation
|
|
|
|
### 7.5 Advanced Valuation Helper (Future)
|
|
|
|
**Optional calculation helper for LLM:**
|
|
|
|
**New Function:**
|
|
|
|
- `calculate_valuation_from_comparables(comparables, weights, adjustments)` - mathematical valuation helper
|
|
- Takes LLM-selected comparable deals
|
|
- Applies LLM-specified weights and adjustments
|
|
- Returns calculated price estimate with breakdown
|
|
- Pure calculation (no intelligence), LLM provides the logic
|
|
|
|
**Purpose:**
|
|
|
|
- LLM selects comparables and determines methodology
|
|
- Function just does the math reliably
|
|
- Returns detailed calculation breakdown for transparency
|
|
|
|
## Implementation Priority
|
|
|
|
**High Priority (MVP):**
|
|
|
|
1. Phase 1: Code Quality & Reliability (critical for stability)
|
|
2. Phase 2: Missing Core Functionality (align with use cases)
|
|
3. Phase 6.1: Cleanup redundant code
|
|
|
|
**Medium Priority (Post-MVP):**
|
|
|
|
4. Phase 3: Architecture improvements (foundation for scaling)
|
|
5. Phase 4: Testing expansion (ensure quality)
|
|
6. Phase 5: Documentation (user experience)
|
|
|
|
**Low Priority (Future):**
|
|
|
|
7. Phase 6.2-6.3: Polish & optimization
|
|
8. Phase 7: Future enhancements (amenities, caching, async, multi-language)
|
|
|
|
## Success Criteria
|
|
|
|
- ✅ All use cases in USECASES.md have corresponding implemented functions
|
|
- ✅ LLM can effectively use tools for all stated use cases
|
|
- ✅ Code is maintainable with <100 lines per function
|
|
- ✅ Test coverage >80% for core functionality
|
|
- ✅ All API calls have proper error handling and retries
|
|
- ✅ Documentation is comprehensive and up-to-date
|
|
- ✅ No duplicate or dead code
|
|
- ✅ Type hints on all public functions
|
|
- ✅ Configuration is externalized and documented
|
|
|
|
## Design Principles
|
|
|
|
- **MCP provides data, LLM provides intelligence**: MCP retrieves and structures data; LLM analyzes, decides, and estimates
|
|
- **Structured by default**: Default responses should be detailed/structured for LLM processing
|
|
- **Optional summarization**: Add `summarized_response` parameter where helpful
|
|
- **No predictions in MVP**: Statistical aggregations yes, ML predictions no
|
|
- **External APIs for future**: Amenity scoring with quality metrics is a future enhancement
|
|
- **Quality over distance**: When amenities are added, include quality metrics not just proximity
|
|
|
|
## Notes
|
|
|
|
- No load testing on external Govmap API (as requested)
|
|
- Focus on LLM-friendly tool design: comprehensive data provision for LLM analysis
|
|
- Amenity scoring with quality data (Ministry of Education, etc.) is future feature
|
|
- Property valuation done by LLM, not MCP - MCP just provides comparable deals data
|
|
- Caching is explicitly not in MVP scope
|
|
- Keep functions focused: one function = one clear purpose
|
|
- Always return structured data that LLMs can easily process
|
|
- Use `summarized_response: bool = False` for optional summarization
|
|
|
|
## Implementation Tracking
|
|
|
|
**For detailed task tracking and current progress, see [TASKS.md](../../TASKS.md)**
|
|
|
|
### Completed (Phase 1)
|
|
- ✅ Configuration management system with environment variable support
|
|
- ✅ Retry logic with exponential backoff (manual implementation)
|
|
- ✅ Rate limiting protection
|
|
- ✅ Standardized error handling (raise exceptions)
|
|
- ✅ Comprehensive input validation
|
|
- ✅ Updated dependencies with version pinning
|
|
- ✅ Created requirements-dev.txt
|
|
- ✅ Updated USECASES.md with status indicators
|
|
- ✅ Created ARCHITECTURE.md
|
|
- ✅ Deleted redundant code files
|
|
|
|
### Completed (Phase 2.1)
|
|
- ✅ Implemented `filter_deals_by_criteria()` with comprehensive filtering
|
|
- ✅ Implemented `calculate_deal_statistics()` with statistical aggregations
|
|
- ✅ Added `get_valuation_comparables` MCP tool
|
|
- ✅ Added `get_deal_statistics` MCP tool
|
|
|
|
### In Progress
|
|
- 🚧 Phase 2.2: Market Activity & Investment Analysis
|
|
- 🚧 Phase 2.3: Enhanced Deal Filtering (partially complete)
|
|
|
|
### Next Up
|
|
- Phase 3: Architecture Improvements (data models, separation of concerns)
|
|
- Phase 4: Testing Expansion
|
|
- Phase 5: Complete Documentation |