Fix: List index bug in outlier detection, add error logging
Bug #1: Fixed IndexError in outlier_detection.py:256 - Missing enumerate() caused stale loop var to access beyond bounds - Occurred when hard bounds filtered deals before IQR processing - Added test reproducing exact scenario (21 deals → 8 filtered) Bug #2: Added stack traces to all MCP tool error logs - Added exc_info=True to 10 MCP tools' error handlers - Improves debugging by logging full stack traces 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
@@ -122,7 +122,7 @@ def autocomplete_address(search_text: str) -> str:
|
|||||||
return json.dumps(formatted_results, ensure_ascii=False, indent=2)
|
return json.dumps(formatted_results, ensure_ascii=False, indent=2)
|
||||||
|
|
||||||
except Exception as e:
|
except Exception as e:
|
||||||
logger.error(f"Error in autocomplete_address: {e}")
|
logger.error(f"Error in autocomplete_address: {e}", exc_info=True)
|
||||||
return f"Error searching for address: {str(e)}"
|
return f"Error searching for address: {str(e)}"
|
||||||
|
|
||||||
|
|
||||||
@@ -164,7 +164,7 @@ def get_deals_by_radius(latitude: float, longitude: float, radius_meters: int =
|
|||||||
)
|
)
|
||||||
|
|
||||||
except Exception as e:
|
except Exception as e:
|
||||||
logger.error(f"Error in get_deals_by_radius: {e}")
|
logger.error(f"Error in get_deals_by_radius: {e}", exc_info=True)
|
||||||
return f"Error fetching polygons by radius: {str(e)}"
|
return f"Error fetching polygons by radius: {str(e)}"
|
||||||
|
|
||||||
|
|
||||||
@@ -221,7 +221,7 @@ def get_street_deals(polygon_id: str, limit: int = 100, deal_type: int = 2) -> s
|
|||||||
)
|
)
|
||||||
|
|
||||||
except Exception as e:
|
except Exception as e:
|
||||||
logger.error(f"Error in get_street_deals: {e}")
|
logger.error(f"Error in get_street_deals: {e}", exc_info=True)
|
||||||
return f"Error fetching street deals: {str(e)}"
|
return f"Error fetching street deals: {str(e)}"
|
||||||
|
|
||||||
|
|
||||||
@@ -343,7 +343,7 @@ def find_recent_deals_for_address(
|
|||||||
)
|
)
|
||||||
|
|
||||||
except Exception as e:
|
except Exception as e:
|
||||||
logger.error(f"Error in find_recent_deals_for_address: {e}")
|
logger.error(f"Error in find_recent_deals_for_address: {e}", exc_info=True)
|
||||||
return f"Error analyzing address: {str(e)}"
|
return f"Error analyzing address: {str(e)}"
|
||||||
|
|
||||||
|
|
||||||
@@ -400,7 +400,7 @@ def get_neighborhood_deals(polygon_id: str, limit: int = 100, deal_type: int = 2
|
|||||||
)
|
)
|
||||||
|
|
||||||
except Exception as e:
|
except Exception as e:
|
||||||
logger.error(f"Error in get_neighborhood_deals: {e}")
|
logger.error(f"Error in get_neighborhood_deals: {e}", exc_info=True)
|
||||||
return f"Error fetching neighborhood deals: {str(e)}"
|
return f"Error fetching neighborhood deals: {str(e)}"
|
||||||
|
|
||||||
|
|
||||||
@@ -620,7 +620,7 @@ def analyze_market_trends(
|
|||||||
)
|
)
|
||||||
|
|
||||||
except Exception as e:
|
except Exception as e:
|
||||||
logger.error(f"Error in analyze_market_trends: {e}")
|
logger.error(f"Error in analyze_market_trends: {e}", exc_info=True)
|
||||||
return f"Error analyzing market trends: {str(e)}"
|
return f"Error analyzing market trends: {str(e)}"
|
||||||
|
|
||||||
|
|
||||||
@@ -753,7 +753,7 @@ def compare_addresses(addresses: List[str]) -> str:
|
|||||||
)
|
)
|
||||||
|
|
||||||
except Exception as e:
|
except Exception as e:
|
||||||
logger.error(f"Error in compare_addresses: {e}")
|
logger.error(f"Error in compare_addresses: {e}", exc_info=True)
|
||||||
return f"Error comparing addresses: {str(e)}"
|
return f"Error comparing addresses: {str(e)}"
|
||||||
|
|
||||||
|
|
||||||
@@ -941,7 +941,7 @@ def get_valuation_comparables(
|
|||||||
)
|
)
|
||||||
|
|
||||||
except Exception as e:
|
except Exception as e:
|
||||||
logger.error(f"Error in get_valuation_comparables: {e}")
|
logger.error(f"Error in get_valuation_comparables: {e}", exc_info=True)
|
||||||
return f"Error getting valuation comparables: {str(e)}"
|
return f"Error getting valuation comparables: {str(e)}"
|
||||||
|
|
||||||
|
|
||||||
@@ -1037,7 +1037,7 @@ def get_deal_statistics(
|
|||||||
)
|
)
|
||||||
|
|
||||||
except Exception as e:
|
except Exception as e:
|
||||||
logger.error(f"Error in get_deal_statistics: {e}")
|
logger.error(f"Error in get_deal_statistics: {e}", exc_info=True)
|
||||||
return f"Error calculating deal statistics: {str(e)}"
|
return f"Error calculating deal statistics: {str(e)}"
|
||||||
|
|
||||||
|
|
||||||
@@ -1154,7 +1154,7 @@ def get_market_activity_metrics(address: str, years_back: int = 2, radius_meters
|
|||||||
)
|
)
|
||||||
|
|
||||||
except Exception as e:
|
except Exception as e:
|
||||||
logger.error(f"Error in get_market_activity_metrics: {e}")
|
logger.error(f"Error in get_market_activity_metrics: {e}", exc_info=True)
|
||||||
return f"Error analyzing market activity: {str(e)}"
|
return f"Error analyzing market activity: {str(e)}"
|
||||||
|
|
||||||
|
|
||||||
|
|||||||
@@ -253,7 +253,7 @@ def filter_deals_for_analysis(
|
|||||||
if metric == "price_per_sqm":
|
if metric == "price_per_sqm":
|
||||||
values = [
|
values = [
|
||||||
deal.price_per_sqm
|
deal.price_per_sqm
|
||||||
for deal in deals
|
for i, deal in enumerate(deals)
|
||||||
if deal.price_per_sqm is not None and not filters_to_remove[i]
|
if deal.price_per_sqm is not None and not filters_to_remove[i]
|
||||||
]
|
]
|
||||||
value_indices = [
|
value_indices = [
|
||||||
|
|||||||
@@ -329,3 +329,44 @@ class TestIntegration:
|
|||||||
# The data error should be filtered by hard bounds (price_per_sqm > 100K)
|
# The data error should be filtered by hard bounds (price_per_sqm > 100K)
|
||||||
assert len(filtered) == 12
|
assert len(filtered) == 12
|
||||||
assert report["outliers_removed"] == 1
|
assert report["outliers_removed"] == 1
|
||||||
|
|
||||||
|
def test_edge_case_few_deals_with_hard_bounds_filter(self):
|
||||||
|
"""
|
||||||
|
Test edge case: Small dataset where hard bounds filter removes some deals.
|
||||||
|
|
||||||
|
This reproduces the bug where list index goes out of range when:
|
||||||
|
- Initial dataset is small (e.g., 21 deals)
|
||||||
|
- Hard bounds filter removes some deals (e.g., 8 outliers)
|
||||||
|
- Remaining dataset is processed with IQR
|
||||||
|
|
||||||
|
The bug was in line 257 where enumerate() was missing, causing
|
||||||
|
the value_indices to be misaligned with the statistical_outliers list.
|
||||||
|
"""
|
||||||
|
# Create 21 deals with some extreme outliers
|
||||||
|
deals = []
|
||||||
|
|
||||||
|
# 13 normal deals (around 10K per sqm)
|
||||||
|
for i in range(13):
|
||||||
|
deals.append(
|
||||||
|
Deal(objectid=i, deal_amount=1000000, deal_date=date(2023, 1, 1), asset_area=100)
|
||||||
|
)
|
||||||
|
|
||||||
|
# 8 extreme outliers that will be caught by hard bounds (price_per_sqm > 100K)
|
||||||
|
for i in range(13, 21):
|
||||||
|
deals.append(
|
||||||
|
Deal(objectid=i, deal_amount=200000000, deal_date=date(2023, 1, 1), asset_area=100)
|
||||||
|
)
|
||||||
|
|
||||||
|
config = GovmapConfig(
|
||||||
|
analysis_outlier_method="iqr",
|
||||||
|
analysis_iqr_multiplier=1.0,
|
||||||
|
analysis_price_per_sqm_max=100000,
|
||||||
|
)
|
||||||
|
|
||||||
|
# This should not raise IndexError
|
||||||
|
filtered, report = filter_deals_for_analysis(deals, config, metric="price_per_sqm")
|
||||||
|
|
||||||
|
# Hard bounds should filter the 8 extreme outliers
|
||||||
|
assert len(filtered) == 13
|
||||||
|
assert report["outliers_removed"] == 8
|
||||||
|
assert report["method_used"] == "iqr"
|
||||||
|
|||||||
Reference in New Issue
Block a user