From 5daaf7002167d1f0d5f2dce9b204aa2bb30c6da9 Mon Sep 17 00:00:00 2001 From: Nitzan Pomerantz <9297302+nitzpo@users.noreply.github.com> Date: Sat, 6 Dec 2025 19:56:15 +0200 Subject: [PATCH] Fix: List index bug in outlier detection, add error logging MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- nadlan_mcp/fastmcp_server.py | 20 ++++++------- nadlan_mcp/govmap/outlier_detection.py | 2 +- tests/govmap/test_outlier_detection.py | 41 ++++++++++++++++++++++++++ 3 files changed, 52 insertions(+), 11 deletions(-) diff --git a/nadlan_mcp/fastmcp_server.py b/nadlan_mcp/fastmcp_server.py index af62cc6..23dfca4 100644 --- a/nadlan_mcp/fastmcp_server.py +++ b/nadlan_mcp/fastmcp_server.py @@ -122,7 +122,7 @@ def autocomplete_address(search_text: str) -> str: return json.dumps(formatted_results, ensure_ascii=False, indent=2) 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)}" @@ -164,7 +164,7 @@ def get_deals_by_radius(latitude: float, longitude: float, radius_meters: int = ) 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)}" @@ -221,7 +221,7 @@ def get_street_deals(polygon_id: str, limit: int = 100, deal_type: int = 2) -> s ) 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)}" @@ -343,7 +343,7 @@ def find_recent_deals_for_address( ) 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)}" @@ -400,7 +400,7 @@ def get_neighborhood_deals(polygon_id: str, limit: int = 100, deal_type: int = 2 ) 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)}" @@ -620,7 +620,7 @@ def analyze_market_trends( ) 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)}" @@ -753,7 +753,7 @@ def compare_addresses(addresses: List[str]) -> str: ) 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)}" @@ -941,7 +941,7 @@ def get_valuation_comparables( ) 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)}" @@ -1037,7 +1037,7 @@ def get_deal_statistics( ) 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)}" @@ -1154,7 +1154,7 @@ def get_market_activity_metrics(address: str, years_back: int = 2, radius_meters ) 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)}" diff --git a/nadlan_mcp/govmap/outlier_detection.py b/nadlan_mcp/govmap/outlier_detection.py index bd1640c..9889830 100644 --- a/nadlan_mcp/govmap/outlier_detection.py +++ b/nadlan_mcp/govmap/outlier_detection.py @@ -253,7 +253,7 @@ def filter_deals_for_analysis( if metric == "price_per_sqm": values = [ 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] ] value_indices = [ diff --git a/tests/govmap/test_outlier_detection.py b/tests/govmap/test_outlier_detection.py index 1e64d87..25973bf 100644 --- a/tests/govmap/test_outlier_detection.py +++ b/tests/govmap/test_outlier_detection.py @@ -329,3 +329,44 @@ class TestIntegration: # The data error should be filtered by hard bounds (price_per_sqm > 100K) assert len(filtered) == 12 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"