mcp-servers/nadlan-mcp
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Normalize MCP response structures across all tools

Browse Source 
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>
This commit is contained in:
Nitzan P
2025-11-23 09:49:52 +02:00
parent c444e88a10
commit e791d3c1ed
4 changed files with 476 additions and 76 deletions
Download Patch File Download Diff File Expand all files Collapse all files
MCP_NORMALIZATION_FIX.md
+327
View File
@@ -0,0 +1,327 @@
# Instructions for nadlan-mcp Project: Normalize Response Structure
## Problem
Different MCP tools return data in **inconsistent structures**, causing the bot to fail when interpreting results.
### Current Inconsistent Structures
**`get_valuation_comparables`** returns:
```json
{
"total_comparables": 3,
"statistics": {
"total_deals": 3,
"price_statistics": { "mean": ..., "median": ... }
},
"comparables": [...] // ← Array of deals
}
```
**`find_recent_deals_for_address`** returns:
```json
{
"market_statistics": {
"deal_breakdown": {
"total_deals": 4
},
"price_stats": { "average_price": ..., "median_price": ... }
},
"deals": [...] // ← Different field name!
}
```
### The Impact
The bot code looks for:
- Deal count in: `market_statistics.deal_breakdown.total_deals`
- Deal array in: `deals`
When `get_valuation_comparables` is called:
- Bot looks for `deals` array → finds nothing (field is called `comparables`)
- Bot looks for `market_statistics.deal_breakdown.total_deals` → finds nothing (field is `total_comparables` or `statistics.total_deals`)
- Bot thinks there are **0 deals** even when MCP returned **20 deals**
- User gets: "לא מצאתי עסקאות ספציפיות" (I didn't find specific deals)
---
## Solution: Normalize All MCP Tool Responses
All MCP tools should return data in a **consistent structure**. I recommend standardizing on this format:
### Recommended Standard Structure
```json
{
"search_parameters": {
// Tool-specific search parameters that were used
"address": "...",
"years_back": 2,
// ... other params
},
"market_statistics": {
"deal_breakdown": {
"total_deals": N, // ← Always here
"same_building_deals": 0, // Optional: only if relevant
"street_deals": 0, // Optional: only if relevant
"neighborhood_deals": 0 // Optional: only if relevant
},
"price_statistics": { // ← Standardize field name
"mean": 1750000.0, // Use "mean" not "average_price"
"median": 1800000.0, // Use "median" not "median_price"
"min": 1443000.0,
"max": 2100000.0,
"p25": 1500000.0,
"p75": 2000000.0,
"std_dev": 250000.0,
"total": 7000000.0
},
"area_statistics": { // ← Standardize field name
"mean": 68.8, // Use "mean" not "average_area"
"median": 76.0,
"min": 42.0,
"max": 106.0,
"p25": 50.0,
"p75": 90.0
},
"price_per_sqm_statistics": { // ← Standardize field name
"mean": 27892.0, // Use "mean" not "average_price_per_sqm"
"median": 34357.0,
"min": 19811.0,
"max": 35294.0,
"p25": 25000.0,
"p75": 32000.0
},
"property_type_distribution": { // Optional
"דירה": 20,
"בית": 5
},
"date_range": { // Optional
"earliest": "2024-01-31",
"latest": "2025-11-05"
}
},
"deals": [ // ← Always "deals", never "comparables"
{
"objectid": 1975198,
"deal_amount": 1800000.0,
"deal_date": "2025-08-21",
"asset_area": 51.0,
"settlement_name_heb": "חולון",
"property_type_description": "דירה",
"neighborhood": "קרית עבודה",
"rooms": 3.0,
"streetNameHeb": "חנקין",
"streetNameEng": "Hankin",
"houseNum": 62,
"floorNo": "שניה",
"price_per_sqm": 35294.12,
"deal_source": "street", // Optional: same_building, street, neighborhood
"priority": 1, // Optional: relevance ranking
// ... other fields
}
]
}
```
---
## Specific Changes Needed
### 1. `get_valuation_comparables` Tool
**Current response**:
```json
{
"total_comparables": 3,
"statistics": { "total_deals": 3, "price_statistics": {...} },
"comparables": [...]
}
```
**Should be changed to**:
```json
{
"search_parameters": {
"address": "חנקין 62 חולון",
"years_back": 2,
"filters_applied": {
"property_type": null,
"rooms": "2.5-3.5",
"price": null,
"area": null,
"floor": null
},
"radius_meters": 100,
"max_comparables": 50
},
"market_statistics": {
"deal_breakdown": {
"total_deals": 3 // ← Move from "total_comparables"
},
"price_statistics": { // ← Keep, matches standard
"mean": 1743333.33,
"median": 1750000.0,
"min": 1680000.0,
"max": 1800000.0,
"p25": 1680000.0,
"p75": 1800000.0,
"std_dev": 60277.14
},
"area_statistics": {...}, // ← Keep
"price_per_sqm_statistics": {...} // ← Keep
},
"deals": [...] // ← Rename from "comparables"
}
```
**Changes**:
1. Remove `total_comparables` field
2. Add `market_statistics.deal_breakdown.total_deals` instead
3. Rename `comparables` → `deals`
4. Move `statistics` → `market_statistics`
5. Add `search_parameters` section with `filters_applied`
### 2. `find_recent_deals_for_address` Tool
**Current response**: ✅ Already follows the standard!
Keep as-is, except:
- 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`
### 3. `analyze_market_trends` Tool
**Should return**:
```json
{
"analysis_parameters": {
"address": "...",
"years_back": 3
},
"market_statistics": { // ← Add this
"deal_breakdown": {
"total_deals": 50 // ← Move from market_summary
},
"price_statistics": {...}, // ← From trend analysis
"area_statistics": {...}
},
"yearly_trends": {...}, // Keep tool-specific data
"trend_analysis": {...}, // Keep tool-specific data
"deals": [...] // Optional: sample deals
}
```
### 4. `get_deal_statistics` Tool
**Should return**:
```json
{
"search_parameters": {...},
"market_statistics": { // ← Rename from "statistics"
"deal_breakdown": {
"total_deals": 25
},
"price_statistics": {...},
"area_statistics": {...},
"price_per_sqm_statistics": {...}
},
"deals": [] // Can be empty for statistics-only queries
}
```
### 5. `get_market_activity_metrics` Tool
**Should return**:
```json
{
"analysis_parameters": {...},
"market_statistics": {
"deal_breakdown": {
"total_deals": 20 // ← Move from total_deals_analyzed
}
},
"market_activity": { // Keep tool-specific metrics
"activity_score": 75.0,
"liquidity_score": 80.0,
...
},
"investment_potential": {...}, // Keep tool-specific metrics
"deals": [] // Optional
}
```
---
## Benefits of Normalization
1. **Bot compatibility**: Bot can use the same code path for all tools
2. **Consistency**: Developers always know where to find deal count and deal array
3. **Maintainability**: Adding new tools is easier with a standard structure
4. **Testing**: Can write generic tests that work for all tools
5. **Documentation**: Single structure to document instead of 5+ different formats
---
## Implementation Checklist
For each MCP tool that returns deals:
- [ ] Add `market_statistics.deal_breakdown.total_deals` field
- [ ] Ensure deals array is named `deals` (not `comparables`, etc.)
- [ ] Standardize statistics field names:
- [ ] `price_statistics` with `mean`, `median`, `min`, `max`, `p25`, `p75`
- [ ] `area_statistics` with same structure
- [ ] `price_per_sqm_statistics` with same structure
- [ ] Add `search_parameters` or `analysis_parameters` section
- [ ] Keep tool-specific fields (like `yearly_trends`, `market_activity`, etc.) but always include the standard structure
- [ ] Test with bot to verify it correctly interprets results
---
## Migration Strategy
**Option 1: Breaking change (recommended)**
- Update all tools at once to use new structure
- Update bot to expect new structure
- Deploy both together
**Option 2: Backward compatible**
- Return BOTH old and new fields temporarily:
```json
{
"total_comparables": 3, // Old (deprecated)
"comparables": [...], // Old (deprecated)
"market_statistics": { // New (standard)
"deal_breakdown": {
"total_deals": 3
}
},
"deals": [...] // New (standard)
}
```
- Update bot to prefer new fields, fall back to old
- Remove old fields after bot is updated
I recommend **Option 1** since this is an internal MCP server with a single client (the bot).
---
## Testing After Changes
Once normalized, test with the bot:
```
User: "כמה עולה דירת 3 חדרים בחנקין 62 חולון?"
Expected: Bot should return actual deals and prices, NOT "לא מצאתי עסקאות"
```
Verify in bot logs:
```
Total deals found: 20 // Should show actual count
Sample deals (most recent 10):
1. חנקין 62 חולון - ₪1,800,000 - 3 rooms - 51 sqm - 2025-08-21
...
```
nadlan_mcp/fastmcp_server.py
+111 -52
View File
@@ -258,33 +258,30 @@ def find_recent_deals_for_address(
}
}
# Standardize field names to match other tools
if prices:
stats["price_stats"] = {
"average_price": round(sum(prices) / len(prices), 0),
"min_price": min(prices),
"max_price": max(prices),
"median_price": sorted(prices)[len(prices) // 2] if prices else 0,
"total_volume": sum(prices),
stats["price_statistics"] = {
"mean": round(sum(prices) / len(prices), 0),
"min": min(prices),
"max": max(prices),
"median": sorted(prices)[len(prices) // 2] if prices else 0,
"total": sum(prices),
}
if areas:
stats["area_stats"] = {
"average_area": round(sum(areas) / len(areas), 1),
"min_area": min(areas),
"max_area": max(areas),
"median_area": sorted(areas)[len(areas) // 2] if areas else 0,
stats["area_statistics"] = {
"mean": round(sum(areas) / len(areas), 1),
"min": min(areas),
"max": max(areas),
"median": sorted(areas)[len(areas) // 2] if areas else 0,
}
if price_per_sqm_values:
stats["price_per_sqm_stats"] = {
"average_price_per_sqm": round(
sum(price_per_sqm_values) / len(price_per_sqm_values), 0
),
"min_price_per_sqm": round(min(price_per_sqm_values), 0),
"max_price_per_sqm": round(max(price_per_sqm_values), 0),
"median_price_per_sqm": round(
sorted(price_per_sqm_values)[len(price_per_sqm_values) // 2], 0
)
stats["price_per_sqm_statistics"] = {
"mean": round(sum(price_per_sqm_values) / len(price_per_sqm_values), 0),
"min": round(min(price_per_sqm_values), 0),
"max": round(max(price_per_sqm_values), 0),
"median": round(sorted(price_per_sqm_values)[len(price_per_sqm_values) // 2], 0)
if price_per_sqm_values
else 0,
}
@@ -531,6 +528,7 @@ def analyze_market_trends(
deal_type_desc = "first hand (new)" if deal_type == 1 else "second hand (used)"
# Return summarized analysis (NO raw deals to save tokens)
# Normalize structure with standard market_statistics while keeping tool-specific analysis
return json.dumps(
{
"analysis_parameters": {
@@ -541,8 +539,12 @@ def analyze_market_trends(
"deal_type": deal_type,
"deal_type_description": deal_type_desc,
},
"market_statistics": {
"deal_breakdown": {
"total_deals": len(deals),
},
},
"market_summary": {
"total_deals": len(deals),
"years_with_data": len(yearly_trends),
"unique_property_types": len(property_type_analysis),
"unique_neighborhoods": len(neighborhood_analysis),
@@ -564,6 +566,7 @@ def analyze_market_trends(
else None,
"deal_source_summary": f"Building: {len([d for d in deals if getattr(d, 'deal_source', None) == 'same_building'])}, Street: {len([d for d in deals if getattr(d, 'deal_source', None) == 'street'])}, Neighborhood: {len([d for d in deals if getattr(d, 'deal_source', None) == 'neighborhood'])}",
},
"deals": [], # Trend analysis doesn't return raw deals to save tokens
},
ensure_ascii=False,
indent=2,
@@ -756,9 +759,18 @@ def get_valuation_comparables(
if not deals:
return json.dumps(
{
"address": address,
"years_back": years_back,
"comparables": [],
"search_parameters": {
"address": address,
"years_back": years_back,
"radius_meters": radius_meters,
"max_comparables": max_comparables,
},
"market_statistics": {
"deal_breakdown": {
"total_deals": 0,
},
},
"deals": [],
"message": "No deals found for this address",
},
ensure_ascii=False,
@@ -782,20 +794,33 @@ def get_valuation_comparables(
# Calculate statistics on filtered comparables
stats = client.calculate_deal_statistics(filtered_deals)
# Normalize response structure to match other tools
return json.dumps(
{
"address": address,
"years_back": years_back,
"filters_applied": {
"property_type": property_type,
"rooms": f"{min_rooms}-{max_rooms}" if min_rooms or max_rooms else None,
"price": f"{min_price}-{max_price}" if min_price or max_price else None,
"area": f"{min_area}-{max_area}" if min_area or max_area else None,
"floor": f"{min_floor}-{max_floor}" if min_floor or max_floor else None,
"search_parameters": {
"address": address,
"years_back": years_back,
"radius_meters": radius_meters,
"max_comparables": max_comparables,
"filters_applied": {
"property_type": property_type,
"rooms": f"{min_rooms}-{max_rooms}" if min_rooms or max_rooms else None,
"price": f"{min_price}-{max_price}" if min_price or max_price else None,
"area": f"{min_area}-{max_area}" if min_area or max_area else None,
"floor": f"{min_floor}-{max_floor}" if min_floor or max_floor else None,
},
},
"total_comparables": len(filtered_deals),
"statistics": stats.model_dump(exclude_none=True), # Serialize DealStatistics model
"comparables": strip_bloat_fields(filtered_deals),
"market_statistics": {
"deal_breakdown": {
"total_deals": len(filtered_deals),
},
"price_statistics": stats.price_statistics,
"area_statistics": stats.area_statistics,
"price_per_sqm_statistics": stats.price_per_sqm_statistics,
"property_type_distribution": stats.property_type_distribution,
"date_range": stats.date_range,
},
"deals": strip_bloat_fields(filtered_deals),
},
ensure_ascii=False,
indent=2,
@@ -836,9 +861,15 @@ def get_deal_statistics(
if not deals:
return json.dumps(
{
"address": address,
"years_back": years_back,
"statistics": {"count": 0, "message": "No deals found for this address"},
"search_parameters": {
"address": address,
"years_back": years_back,
},
"market_statistics": {
"deal_breakdown": {"total_deals": 0},
"message": "No deals found for this address",
},
"deals": [],
},
ensure_ascii=False,
indent=2,
@@ -853,15 +884,28 @@ def get_deal_statistics(
# Calculate statistics
stats = client.calculate_deal_statistics(deals)
# Normalize response structure to match other tools
return json.dumps(
{
"address": address,
"years_back": years_back,
"filters_applied": {
"property_type": property_type,
"rooms": f"{min_rooms}-{max_rooms}" if min_rooms or max_rooms else None,
"search_parameters": {
"address": address,
"years_back": years_back,
"filters_applied": {
"property_type": property_type,
"rooms": f"{min_rooms}-{max_rooms}" if min_rooms or max_rooms else None,
},
},
"statistics": stats.model_dump(exclude_none=True), # Serialize DealStatistics model
"market_statistics": {
"deal_breakdown": {
"total_deals": stats.total_deals,
},
"price_statistics": stats.price_statistics,
"area_statistics": stats.area_statistics,
"price_per_sqm_statistics": stats.price_per_sqm_statistics,
"property_type_distribution": stats.property_type_distribution,
"date_range": stats.date_range,
},
"deals": [], # Statistics-only query, no full deals returned
},
ensure_ascii=False,
indent=2,
@@ -924,10 +968,18 @@ def get_market_activity_metrics(address: str, years_back: int = 2, radius_meters
if not deals:
return json.dumps(
{
"address": address,
"analysis_parameters": {
"address": address,
"years_back": years_back,
"radius_meters": radius_meters,
},
"market_statistics": {
"deal_breakdown": {
"total_deals": 0,
},
},
"deals": [],
"error": "No deals found for analysis",
"years_back": years_back,
"radius_meters": radius_meters,
},
ensure_ascii=False,
indent=2,
@@ -938,13 +990,19 @@ def get_market_activity_metrics(address: str, years_back: int = 2, radius_meters
liquidity_metrics = _safe_calculate_metric(client.get_market_liquidity, deals)
investment_metrics = _safe_calculate_metric(client.analyze_investment_potential, deals)
# Combine all metrics
# Combine all metrics with normalized structure
return json.dumps(
{
"address": address,
"years_back": years_back,
"radius_meters": radius_meters,
"total_deals_analyzed": len(deals),
"analysis_parameters": {
"address": address,
"years_back": years_back,
"radius_meters": radius_meters,
},
"market_statistics": {
"deal_breakdown": {
"total_deals": len(deals),
},
},
"market_activity": activity_metrics,
"market_liquidity": liquidity_metrics,
"investment_potential": investment_metrics,
@@ -957,6 +1015,7 @@ def get_market_activity_metrics(address: str, years_back: int = 2, radius_meters
"price_trend": investment_metrics.get("price_trend"),
"market_stability": investment_metrics.get("market_stability"),
},
"deals": [], # Activity metrics don't return raw deals
},
ensure_ascii=False,
indent=2,
tests/e2e/test_mcp_tools_comprehensive.py
+23 -16
View File
@@ -75,11 +75,12 @@ class TestMCPToolsE2E:
result = analyze_market_trends(self.TEST_ADDRESS_1, years_back=3, radius_meters=100)
data = json.loads(result)
# Check response structure
assert "market_summary" in data
assert "total_deals" in data["market_summary"]
assert isinstance(data["market_summary"]["total_deals"], int)
assert data["market_summary"]["total_deals"] >= 0
# Check response structure (normalized in MCP_NORMALIZATION_FIX)
assert "market_statistics" in data
assert "deal_breakdown" in data["market_statistics"]
assert "total_deals" in data["market_statistics"]["deal_breakdown"]
assert isinstance(data["market_statistics"]["deal_breakdown"]["total_deals"], int)
assert data["market_statistics"]["deal_breakdown"]["total_deals"] >= 0
def test_get_valuation_comparables(self):
"""Test getting valuation comparables."""
@@ -88,13 +89,17 @@ class TestMCPToolsE2E:
)
data = json.loads(result)
assert "total_comparables" in data
assert isinstance(data["total_comparables"], int)
assert data["total_comparables"] >= 0
# Normalized structure: total_comparables -> market_statistics.deal_breakdown.total_deals
assert "market_statistics" in data
assert "deal_breakdown" in data["market_statistics"]
assert "total_deals" in data["market_statistics"]["deal_breakdown"]
assert isinstance(data["market_statistics"]["deal_breakdown"]["total_deals"], int)
assert data["market_statistics"]["deal_breakdown"]["total_deals"] >= 0
if data["total_comparables"] > 0:
assert "comparables" in data
comp = data["comparables"][0]
# Normalized structure: comparables -> deals
if data["market_statistics"]["deal_breakdown"]["total_deals"] > 0:
assert "deals" in data
comp = data["deals"][0]
assert "deal_amount" in comp
# rooms field is optional and excluded when None
# Just verify the comparable has basic required fields
@@ -105,11 +110,13 @@ class TestMCPToolsE2E:
result = get_deal_statistics(self.TEST_ADDRESS_1, years_back=3)
data = json.loads(result)
# Check response structure
assert "statistics" in data
if "sample_size" in data["statistics"]:
assert isinstance(data["statistics"]["sample_size"], int)
assert data["statistics"]["sample_size"] >= 0
# Check response structure (normalized: statistics -> market_statistics)
assert "market_statistics" in data
# Check for total_deals in normalized location
if "deal_breakdown" in data["market_statistics"]:
assert "total_deals" in data["market_statistics"]["deal_breakdown"]
assert isinstance(data["market_statistics"]["deal_breakdown"]["total_deals"], int)
assert data["market_statistics"]["deal_breakdown"]["total_deals"] >= 0
def test_get_market_activity_metrics(self):
"""Test market activity metrics."""
tests/test_fastmcp_tools.py
+15 -8
View File
@@ -364,10 +364,13 @@ class TestGetValuationComparables:
)
parsed = json.loads(result)
assert "filters_applied" in parsed
assert "statistics" in parsed
assert "comparables" in parsed
assert parsed["filters_applied"]["property_type"] == "דירה"
# Normalized structure: filters_applied is now in search_parameters
assert "search_parameters" in parsed
assert "filters_applied" in parsed["search_parameters"]
assert parsed["search_parameters"]["filters_applied"]["property_type"] == "דירה"
# Normalized structure: comparables -> deals
assert "deals" in parsed
assert "market_statistics" in parsed
@patch("nadlan_mcp.fastmcp_server.client")
def test_comparables_strips_bloat(self, mock_client):
@@ -392,7 +395,8 @@ class TestGetValuationComparables:
result = fastmcp_server.get_valuation_comparables("test address")
parsed = json.loads(result)
comparable = parsed["comparables"][0]
# Normalized structure: comparables -> deals
comparable = parsed["deals"][0]
assert "shape" not in comparable
assert "sourceorder" not in comparable
# source_polygon_id is kept when added by processing
@@ -421,9 +425,12 @@ class TestGetDealStatistics:
result = fastmcp_server.get_deal_statistics("test address")
parsed = json.loads(result)
assert "address" in parsed
assert "statistics" in parsed
assert parsed["statistics"]["total_deals"] == 2 # Field name is total_deals in model
# Normalized structure: address is now in search_parameters, statistics -> market_statistics
assert "search_parameters" in parsed
assert "address" in parsed["search_parameters"]
assert "market_statistics" in parsed
assert "deal_breakdown" in parsed["market_statistics"]
assert parsed["market_statistics"]["deal_breakdown"]["total_deals"] == 2
class TestGetMarketActivityMetrics: