Added plan

2025-10-19 00:57:50 +03:00
parent fb1f276a80
commit ffd7c84aa7
1 changed files with 472 additions and 0 deletions
@@ -0,0 +1,472 @@
+<!-- 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
+
+### To-dos
+
+- [ ] Add retry logic, rate limiting, and standardize error handling in govmap.py and fastmcp_server.py
+- [ ] Create config.py with externalized configuration and environment variable support
+- [ ] Update USECASES.md and README.md to reflect actual current state and future roadmap
+- [ ] Create tools for providing comparable deals and statistical data for LLM valuation
+- [ ] Create market activity scoring and investment analysis functions
+- [ ] Add comprehensive deal filtering by property type, rooms, price, area, floor
+- [ ] Create Pydantic models for Deal, Address, MarketAnalysis, PropertyValuation
+- [ ] Separate concerns: create api_client.py, analyzers package, thin fastmcp_server.py
+- [ ] Update all tools with summarized_response boolean parameter and consistent responses
+- [ ] Add integration tests, edge cases, parametrized tests for all functionality
+- [ ] Create test_validation.py with input validation and data model tests
+- [ ] Create ARCHITECTURE.md, DEPLOYMENT.md, CONTRIBUTING.md, API_REFERENCE.md, CLAUDE.md
+- [ ] Add module docstrings, comprehensive function docs, type hints, inline comments
+- [ ] Create examples/ directory with scripts showing all major use cases
+- [ ] Add status indicators, priority levels, timeline to USECASES.md
+- [ ] Delete/integrate mcp_server_concept.py, remove unused imports, consolidate duplicates
+- [ ] Setup pre-commit hooks, format all code, fix linter warnings, add mypy
+- [ ] Update requirements.txt with new dependencies, create requirements-dev.txt, pin versions