From b7383950fdfbb7aaad798c9378f7a30205d88149 Mon Sep 17 00:00:00 2001 From: Nitzan Pomerantz <9297302+nitzpo@users.noreply.github.com> Date: Sat, 25 Oct 2025 13:24:13 +0300 Subject: [PATCH] Update documentation to reflect Phase 3 refactoring completion MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- ARCHITECTURE.md | 56 +++++++++++++++++++++++++++++++------------------ CLAUDE.md | 56 +++++++++++++++++++++++++++++++++---------------- 2 files changed, 74 insertions(+), 38 deletions(-) diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index f3a8548..ca8d92f 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -58,15 +58,18 @@ All settings (timeouts, retries, rate limits) are configurable via environment v ↓ β”Œβ”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β” β”‚ Business Logic Layer β”‚ -β”‚ β”Œβ”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β” β”Œβ”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β” β”‚ -β”‚ β”‚ Market Analysis β”‚ β”‚ Deal Processing & Filtering β”‚ β”‚ -β”‚ β”‚ (analyze_market β”‚ β”‚ (_is_same_building, etc.) β”‚ β”‚ -β”‚ β”‚ _trends logic) β”‚ β”‚ β”‚ β”‚ -β”‚ β””β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”˜ β””β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”˜ β”‚ -β”‚ β”Œβ”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β” β”‚ -β”‚ β”‚ govmap.py (GovmapClient) β”‚ β”‚ -β”‚ β”‚ β€’ Validation β€’ Retry Logic β€’ Rate Limiting β”‚ β”‚ -β”‚ β””β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”¬β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”˜ β”‚ +β”‚ β”Œβ”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β” β”‚ +β”‚ β”‚ govmap/ Package β”‚ β”‚ +β”‚ β”‚ β”Œβ”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β” β”Œβ”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β” β”Œβ”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β” β”‚ β”‚ +β”‚ β”‚ β”‚ client.py β”‚ β”‚ validators.pyβ”‚ β”‚ filters.py β”‚ β”‚ β”‚ +β”‚ β”‚ β”‚ (API calls) β”‚ β”‚ (validation) β”‚ β”‚ (filtering) β”‚ β”‚ β”‚ +β”‚ β”‚ β””β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”˜ β””β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”˜ β””β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”˜ β”‚ β”‚ +β”‚ β”‚ β”Œβ”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β” β”Œβ”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β” β”Œβ”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β” β”‚ β”‚ +β”‚ β”‚ β”‚statistics.pyβ”‚ β”‚market_ β”‚ β”‚ utils.py β”‚ β”‚ β”‚ +β”‚ β”‚ β”‚ (stats) β”‚ β”‚analysis.py β”‚ β”‚ (helpers) β”‚ β”‚ β”‚ +β”‚ β”‚ β””β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”˜ β””β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”˜ β””β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”˜ β”‚ β”‚ +β”‚ β”‚ β€’ Retry Logic β€’ Rate Limiting β€’ Validation β”‚ β”‚ +β”‚ β””β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”¬β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”˜ β”‚ β””β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”¬β”΄β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”˜ β”‚ HTTP/JSON ↓ @@ -111,22 +114,28 @@ custom_config = GovmapConfig(max_retries=5, requests_per_second=10.0) set_config(custom_config) ``` -### API Client Layer (`govmap.py`) +### API Client Layer (`govmap/` package) -**Purpose:** Communicate with Govmap API with reliability features +**Purpose:** Modular package for Govmap API interaction and data processing -**Key Components:** +**Package Structure:** +- `client.py` - GovmapClient class with API methods +- `validators.py` - Input validation functions +- `filters.py` - Deal filtering logic +- `statistics.py` - Statistical calculations +- `market_analysis.py` - Market analysis functions +- `utils.py` - Helper utilities +- `__init__.py` - Public API exports -#### GovmapClient Class +#### GovmapClient Class (`govmap/client.py`) **Responsibilities:** - Make HTTP requests to Govmap API - Implement retry logic with exponential backoff - Enforce rate limiting -- Validate inputs -- Parse and validate responses +- Delegate to specialized modules for validation, filtering, analysis -**Key Methods:** +**Core API Methods:** - `autocomplete_address()` - Search for addresses - `get_gush_helka()` - Get block/parcel data - `get_deals_by_radius()` - Get nearby deals @@ -134,10 +143,17 @@ set_config(custom_config) - `get_neighborhood_deals()` - Get neighborhood deals - `find_recent_deals_for_address()` - Main comprehensive search +**Business Logic Methods** (delegate to respective modules): +- `filter_deals_by_criteria()` - Filter deals by various criteria +- `calculate_deal_statistics()` - Calculate statistical metrics +- `calculate_market_activity_score()` - Analyze market activity +- `analyze_investment_potential()` - Analyze investment potential +- `get_market_liquidity()` - Analyze market liquidity + **Reliability Features:** 1. **Retry Logic** - Exponential backoff on failures 2. **Rate Limiting** - Tracks request times, enforces delays -3. **Input Validation** - Validates all inputs before API calls +3. **Input Validation** - Delegates to validators module 4. **Timeouts** - Configurable connect and read timeouts ### MCP Tools Layer (`fastmcp_server.py`) @@ -371,11 +387,11 @@ GOVMAP_DEFAULT_DEAL_LIMIT=100 ## Future Architecture Evolution -### Phase 3: Package Refactoring (PLANNED) +### Phase 3: Package Refactoring (βœ… COMPLETED) -**See `.cursor/plans/PHASE3-REFACTORING.md` for detailed plan** +**βœ… COMPLETED - See `.cursor/plans/PHASE3-REFACTORING.md` for detailed plan** -Refactor `govmap.py` (1,378 lines) into modular package: +The monolithic `govmap.py` (1,454 lines) has been refactored into a modular package: ``` nadlan_mcp/ diff --git a/CLAUDE.md b/CLAUDE.md index 032b3bf..a56ed51 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -93,10 +93,15 @@ The codebase follows a three-layer architecture: - Main tools: `find_recent_deals_for_address`, `analyze_market_trends`, `compare_addresses` - **Important:** Tools return structured JSON by default; use `summarized_response=True` for condensed summaries -### 2. Business Logic Layer (`nadlan_mcp/govmap.py`) -- `GovmapClient` class implements all API interactions +### 2. Business Logic Layer (`nadlan_mcp/govmap/` package) +- Modular package with specialized modules (see ARCHITECTURE.md for details) +- `client.py` - GovmapClient class with core API methods +- `validators.py` - Input validation functions +- `filters.py` - Deal filtering logic +- `statistics.py` - Statistical calculations +- `market_analysis.py` - Market analysis functions +- `utils.py` - Helper utilities - Reliability features: retry logic with exponential backoff, rate limiting, input validation -- Helper functions for data processing, filtering, and analysis - **Key Design Principle:** MCP provides data, LLM provides intelligence - avoid complex analysis in the MCP layer ### 3. Configuration Layer (`nadlan_mcp/config.py`) @@ -106,16 +111,22 @@ The codebase follows a three-layer architecture: ## Key Files -- `nadlan_mcp/govmap.py` - Core API client with ~1,378 lines of business logic - - **NOTE:** Will be refactored into package in Phase 3 (see `.cursor/plans/PHASE3-REFACTORING.md`) +- `nadlan_mcp/govmap/` - **βœ… Refactored modular package** (Phase 3 complete) + - `client.py` - Core API client (~30KB, GovmapClient class) + - `validators.py` - Input validation (~3KB) + - `filters.py` - Deal filtering (~5KB) + - `statistics.py` - Statistical calculations (~4KB) + - `market_analysis.py` - Market analysis (~17KB) + - `utils.py` - Helper utilities (~4KB) + - `__init__.py` - Package exports for backward compatibility - `nadlan_mcp/fastmcp_server.py` - MCP tool definitions (10 implemented tools) - `nadlan_mcp/config.py` - Configuration management - `run_fastmcp_server.py` - Server entry point -- `tests/test_govmap_client.py` - Main test suite (27 tests) +- `tests/test_govmap_client.py` - Main test suite (34 tests, all passing) - `USECASES.md` - **Product roadmap and feature status** (essential reading) - `ARCHITECTURE.md` - Detailed system architecture and design decisions - `TASKS.md` - Implementation tasks and progress tracking -- `.cursor/plans/PHASE3-REFACTORING.md` - Detailed refactoring plan for Phase 3 +- `.cursor/plans/PHASE3-REFACTORING.md` - Detailed refactoring plan (βœ… completed) ## Available MCP Tools @@ -212,11 +223,12 @@ GOVMAP_DEFAULT_DEAL_LIMIT=100 ## Development Roadmap -See `TASKS.md` for complete implementation plan. Current focus: -- **Phase 2.2:** Market analysis tools (in progress) -- **Phase 2.3:** Enhanced filtering (mostly complete) -- **Phase 3:** Architecture improvements with Pydantic models -- **Phase 4:** Expanded test coverage +See `TASKS.md` for complete implementation plan. Current status: +- **Phase 2.2:** Market analysis tools (βœ… complete) +- **Phase 2.3:** Enhanced filtering (βœ… complete) +- **Phase 3:** Package refactoring (βœ… complete - monolithic govmap.py refactored into modular package) +- **Phase 4:** Pydantic data models (planned) +- **Phase 5:** Expanded test coverage (ongoing) ## Common Tasks @@ -230,10 +242,11 @@ See `TASKS.md` for complete implementation plan. Current focus: ### Adding New API Endpoint Support -1. Add method to `GovmapClient` class in `govmap.py` -2. Implement validation, retry logic, rate limiting -3. Add unit tests in `tests/test_govmap_client.py` -4. Optionally expose as MCP tool +1. Add method to `GovmapClient` class in `govmap/client.py` +2. Implement validation (use `validators.py` functions) +3. Implement retry logic and rate limiting (follow existing patterns) +4. Add unit tests in `tests/test_govmap_client.py` +5. Optionally expose as MCP tool in `fastmcp_server.py` ### Modifying Configuration @@ -265,6 +278,13 @@ The client uses these Govmap API endpoints: - This project uses **FastMCP**, not the standard MCP library - All coordinate tuples are `(longitude, latitude)` in ITM projection (Israeli Transverse Mercator) -- When reading `govmap.py`, note it's ~1000 lines with multiple helper functions - use search/grep to find specific methods -- Floor numbers in Hebrew (e.g., "Χ§Χ¨Χ§Χ’", "מרΧͺΧ£") are parsed by `_extract_floor_number()` +- The `govmap` package is now modular - each module has a specific responsibility: + - `client.py` - API calls and HTTP logic + - `validators.py` - Input validation + - `filters.py` - Deal filtering + - `statistics.py` - Statistical calculations + - `market_analysis.py` - Market analysis metrics + - `utils.py` - Helper utilities +- Floor numbers in Hebrew (e.g., "Χ§Χ¨Χ§Χ’", "מרΧͺΧ£") are parsed by `extract_floor_number()` in `utils.py` - Deal types: 1 = first hand/new construction, 2 = second hand/resale +- Backward compatibility maintained: `from nadlan_mcp.govmap import GovmapClient` still works