From 66f7c2fbd51d5158e327972bc5ad836432532fb1 Mon Sep 17 00:00:00 2001 From: "fabrica-cc-engineering-agent[bot]" Date: Thu, 4 Jun 2026 13:34:24 -0400 Subject: [PATCH 1/4] Fix: dupe-date-price heuristic no longer drops distinct parcels of one deed run_heuristics' flag_dupe_date_price keyed duplicate detection on (sale_date, sale_price) alone. Distinct parcels conveyed together in a single multi-parcel deed share a sale date and price (per-parcel records frequently carry the deed total), so every lot but one was flagged as a duplicate and dropped -- silently discarding most comps of a subdivision sale. Include the parcel key in the signature so only true same-parcel repeats are flagged; distinct parcels are kept. Backward-compatible: only applied when a `key` column is present. Adds tests/test_sales_scrutiny_heuristics.py. Co-Authored-By: Claude Opus 4.8 (1M context) --- openavmkit/sales_scrutiny_study.py | 14 ++++ tests/test_sales_scrutiny_heuristics.py | 101 ++++++++++++++++++++++++ 2 files changed, 115 insertions(+) create mode 100644 tests/test_sales_scrutiny_heuristics.py diff --git a/openavmkit/sales_scrutiny_study.py b/openavmkit/sales_scrutiny_study.py index f5f68a46..4b7fc62c 100644 --- a/openavmkit/sales_scrutiny_study.py +++ b/openavmkit/sales_scrutiny_study.py @@ -620,6 +620,20 @@ def run_heuristics( df_sales["date_price"] = df_sales[jurisdiction].astype(str) + "---" + df_sales["sale_date"].astype(str) + "---" + df_sales["sale_price"].astype(str) else: df_sales["date_price"] = df_sales["sale_date"].astype(str) + "---" + df_sales["sale_price"].astype(str) + # A genuine duplicate is the SAME parcel reported more than once at the same + # date and price (duplicate data entry / shell trade). DISTINCT parcels that + # merely share a date and price -- e.g. multiple lots conveyed in a single + # multi-parcel deed -- are NOT duplicates and must not be flagged (doing so + # silently discards every lot but one of a subdivision sale). Append the + # parcel key when present so only true same-parcel repeats are flagged. A + # missing key would otherwise stringify to "nan" and re-collide across + # distinct parcels, so null keys fall back to the unique per-sale key. + if "key" in df_sales.columns: + parcel_id = df_sales["key"].astype(str) + null_key = df_sales["key"].isna() + if null_key.any() and "key_sale" in df_sales.columns: + parcel_id = parcel_id.mask(null_key, df_sales["key_sale"].astype(str)) + df_sales["date_price"] = df_sales["date_price"] + "---" + parcel_id vcs_date_price = df_sales["date_price"].value_counts() idx_dupe_date_price = vcs_date_price[vcs_date_price > 1].index.values df_sales.loc[ diff --git a/tests/test_sales_scrutiny_heuristics.py b/tests/test_sales_scrutiny_heuristics.py new file mode 100644 index 00000000..fe38d455 --- /dev/null +++ b/tests/test_sales_scrutiny_heuristics.py @@ -0,0 +1,101 @@ +"""Tests for the duplicate-detection heuristics in ``run_heuristics``. + +Regression coverage for the ``flag_dupe_date_price`` heuristic: distinct parcels +conveyed in a single multi-parcel deed share a sale date and price but are NOT +duplicates and must be kept, while a genuine same-parcel repeat must still be +dropped. +""" +import pandas as pd + +from openavmkit.data import SalesUniversePair +from openavmkit.sales_scrutiny_study import run_heuristics + + +SETTINGS = {"analysis": {"sales_scrutiny": {}}} + + +def _sale(key, key_sale, date, price): + return { + "key": key, + "key_sale": key_sale, + "sale_date": date, + "sale_price": price, + "sale_year": int(date[:4]), + "vacant_sale": False, + "bldg_year_built": 0, + } + + +def _make_sup(sales_rows): + sales = pd.DataFrame(sales_rows) + keys = sorted(set(sales["key"])) + universe = pd.DataFrame({"key": keys, "is_vacant": [False] * len(keys)}) + return SalesUniversePair(sales=sales, universe=universe) + + +def test_dupe_date_price_keeps_distinct_parcels_in_one_deed(tmp_path, monkeypatch): + monkeypatch.chdir(tmp_path) + # Three DISTINCT parcels conveyed in a single multi-parcel deed: same date, + # same (deed-total) price. These are not duplicate reports and must survive. + sup = _make_sup( + [ + _sale("p1", "s1", "2020-01-01", 75000), + _sale("p2", "s2", "2020-01-01", 75000), + _sale("p3", "s3", "2020-01-01", 75000), + ] + ) + out = run_heuristics(sup, SETTINGS, drop=True) + assert set(out.sales["key_sale"]) == {"s1", "s2", "s3"} + + +def test_dupe_date_price_still_flags_same_parcel_repeat(tmp_path, monkeypatch): + monkeypatch.chdir(tmp_path) + # The SAME parcel reported twice at the same date and price is a genuine + # duplicate report and must still be dropped; the distinct-parcel deed lots + # alongside it must be kept. + sup = _make_sup( + [ + _sale("p1", "s1", "2020-01-01", 75000), + _sale("p2", "s2", "2020-01-01", 75000), + _sale("p5", "s4", "2021-01-01", 50000), + _sale("p5", "s5", "2021-01-01", 50000), + ] + ) + out = run_heuristics(sup, SETTINGS, drop=True) + survivors = set(out.sales["key_sale"]) + assert {"s1", "s2"}.issubset(survivors) + assert survivors.isdisjoint({"s4", "s5"}) + + +def test_dupe_date_price_keeps_one_parcel_sold_twice_on_different_dates(tmp_path, monkeypatch): + monkeypatch.chdir(tmp_path) + # One parcel with two legitimate sales at DIFFERENT dates is not a duplicate + # and both rows must be kept. + sup = _make_sup( + [ + _sale("p1", "s1", "2018-05-01", 40000), + _sale("p1", "s2", "2022-09-01", 60000), + ] + ) + out = run_heuristics(sup, SETTINGS, drop=True) + assert set(out.sales["key_sale"]) == {"s1", "s2"} + + +def test_dupe_date_price_with_jurisdiction_keeps_distinct_parcels(tmp_path, monkeypatch): + monkeypatch.chdir(tmp_path) + # The jurisdiction-scoped branch must still keep distinct parcels that share + # a date and price, and still drop a true same-parcel repeat. + sales = [ + _sale("p1", "s1", "2020-01-01", 75000), + _sale("p2", "s2", "2020-01-01", 75000), + _sale("p3", "s3", "2021-01-01", 50000), + _sale("p3", "s4", "2021-01-01", 50000), + ] + for row in sales: + row["county"] = "Acme" + sup = _make_sup(sales) + settings = {"analysis": {"sales_scrutiny": {"jurisdiction": "county"}}} + out = run_heuristics(sup, settings, drop=True) + survivors = set(out.sales["key_sale"]) + assert {"s1", "s2"}.issubset(survivors) + assert survivors.isdisjoint({"s3", "s4"}) From fee6197446d4c7a2ecfc09453da6a7c74ebe4156 Mon Sep 17 00:00:00 2001 From: "fabrica-cc-engineering-agent[bot]" Date: Mon, 22 Jun 2026 16:42:42 -0400 Subject: [PATCH 2/4] Isolate dupe-date-price regression helper --- openavmkit/sales_scrutiny_study.py | 30 +-------- openavmkit/utilities/sales_scrutiny.py | 33 ++++++++++ tests/test_sales_scrutiny_heuristics.py | 83 +++++++++++-------------- 3 files changed, 70 insertions(+), 76 deletions(-) create mode 100644 openavmkit/utilities/sales_scrutiny.py diff --git a/openavmkit/sales_scrutiny_study.py b/openavmkit/sales_scrutiny_study.py index 4b7fc62c..facc6ea1 100644 --- a/openavmkit/sales_scrutiny_study.py +++ b/openavmkit/sales_scrutiny_study.py @@ -36,6 +36,7 @@ combine_dfs, ) from openavmkit.utilities.excel import write_to_excel +from openavmkit.utilities.sales_scrutiny import flag_dupe_date_price from openavmkit.utilities.settings import get_fields_categorical, _apply_dd_to_df_cols, area_unit, get_locations, warn_if_location_collapsed @@ -615,34 +616,7 @@ def run_heuristics( warnings.warn(f"You provided a `deed_id`: \"{deed_id}\", but it wasn't found in in the sales dataframe, so no deed-based sales validation heuristic can be run") # 2 -- Flag sales made on the same date for the same price - - if jurisdiction != None: - df_sales["date_price"] = df_sales[jurisdiction].astype(str) + "---" + df_sales["sale_date"].astype(str) + "---" + df_sales["sale_price"].astype(str) - else: - df_sales["date_price"] = df_sales["sale_date"].astype(str) + "---" + df_sales["sale_price"].astype(str) - # A genuine duplicate is the SAME parcel reported more than once at the same - # date and price (duplicate data entry / shell trade). DISTINCT parcels that - # merely share a date and price -- e.g. multiple lots conveyed in a single - # multi-parcel deed -- are NOT duplicates and must not be flagged (doing so - # silently discards every lot but one of a subdivision sale). Append the - # parcel key when present so only true same-parcel repeats are flagged. A - # missing key would otherwise stringify to "nan" and re-collide across - # distinct parcels, so null keys fall back to the unique per-sale key. - if "key" in df_sales.columns: - parcel_id = df_sales["key"].astype(str) - null_key = df_sales["key"].isna() - if null_key.any() and "key_sale" in df_sales.columns: - parcel_id = parcel_id.mask(null_key, df_sales["key_sale"].astype(str)) - df_sales["date_price"] = df_sales["date_price"] + "---" + parcel_id - vcs_date_price = df_sales["date_price"].value_counts() - idx_dupe_date_price = vcs_date_price[vcs_date_price > 1].index.values - df_sales.loc[ - df_sales["date_price"].isin(idx_dupe_date_price), - "flag_dupe_date_price", - ] = True - - # drop extraneous column - df_sales = df_sales.drop(columns="date_price") + df_sales = flag_dupe_date_price(df_sales, jurisdiction) #### Misclassified vacant sales detection heuristics diff --git a/openavmkit/utilities/sales_scrutiny.py b/openavmkit/utilities/sales_scrutiny.py new file mode 100644 index 00000000..4b5fab92 --- /dev/null +++ b/openavmkit/utilities/sales_scrutiny.py @@ -0,0 +1,33 @@ +import pandas as pd + + +def flag_dupe_date_price(df_sales: pd.DataFrame, jurisdiction=None) -> pd.DataFrame: + """Flag same-parcel duplicate sales that share a date and price.""" + if jurisdiction is not None: + date_price = ( + df_sales[jurisdiction].astype(str) + + "---" + + df_sales["sale_date"].astype(str) + + "---" + + df_sales["sale_price"].astype(str) + ) + else: + date_price = ( + df_sales["sale_date"].astype(str) + + "---" + + df_sales["sale_price"].astype(str) + ) + + # Distinct parcels can legitimately share one date/price in a multi-parcel + # deed. Include the parcel key so only same-parcel repeats are flagged. + if "key" in df_sales.columns: + parcel_id = df_sales["key"].astype(str) + null_key = df_sales["key"].isna() + if null_key.any() and "key_sale" in df_sales.columns: + parcel_id = parcel_id.mask(null_key, df_sales["key_sale"].astype(str)) + date_price = date_price + "---" + parcel_id + + dupes = date_price.value_counts() + dupe_keys = dupes[dupes > 1].index.values + df_sales.loc[date_price.isin(dupe_keys), "flag_dupe_date_price"] = True + return df_sales diff --git a/tests/test_sales_scrutiny_heuristics.py b/tests/test_sales_scrutiny_heuristics.py index fe38d455..f79b8663 100644 --- a/tests/test_sales_scrutiny_heuristics.py +++ b/tests/test_sales_scrutiny_heuristics.py @@ -1,17 +1,12 @@ -"""Tests for the duplicate-detection heuristics in ``run_heuristics``. +"""Tests for the duplicate-detection heuristic ``flag_dupe_date_price``. -Regression coverage for the ``flag_dupe_date_price`` heuristic: distinct parcels -conveyed in a single multi-parcel deed share a sale date and price but are NOT -duplicates and must be kept, while a genuine same-parcel repeat must still be -dropped. +Regression coverage for ``flag_dupe_date_price``: distinct parcels conveyed in a +single multi-parcel deed share a sale date and price but are NOT duplicates and +must be kept, while a genuine same-parcel repeat must still be flagged. """ import pandas as pd -from openavmkit.data import SalesUniversePair -from openavmkit.sales_scrutiny_study import run_heuristics - - -SETTINGS = {"analysis": {"sales_scrutiny": {}}} +from openavmkit.utilities.sales_scrutiny import flag_dupe_date_price def _sale(key, key_sale, date, price): @@ -20,40 +15,39 @@ def _sale(key, key_sale, date, price): "key_sale": key_sale, "sale_date": date, "sale_price": price, - "sale_year": int(date[:4]), - "vacant_sale": False, - "bldg_year_built": 0, } -def _make_sup(sales_rows): - sales = pd.DataFrame(sales_rows) - keys = sorted(set(sales["key"])) - universe = pd.DataFrame({"key": keys, "is_vacant": [False] * len(keys)}) - return SalesUniversePair(sales=sales, universe=universe) +def _df(sales_rows): + return pd.DataFrame(sales_rows) + + +def _flagged_sales(df): + if "flag_dupe_date_price" not in df.columns: + return set() + return set(df.loc[df["flag_dupe_date_price"].eq(True), "key_sale"]) -def test_dupe_date_price_keeps_distinct_parcels_in_one_deed(tmp_path, monkeypatch): - monkeypatch.chdir(tmp_path) +def test_dupe_date_price_keeps_distinct_parcels_in_one_deed(): # Three DISTINCT parcels conveyed in a single multi-parcel deed: same date, - # same (deed-total) price. These are not duplicate reports and must survive. - sup = _make_sup( + # same (deed-total) price. These are not duplicate reports and must not be + # flagged. + df = _df( [ _sale("p1", "s1", "2020-01-01", 75000), _sale("p2", "s2", "2020-01-01", 75000), _sale("p3", "s3", "2020-01-01", 75000), ] ) - out = run_heuristics(sup, SETTINGS, drop=True) - assert set(out.sales["key_sale"]) == {"s1", "s2", "s3"} + out = flag_dupe_date_price(df) + assert _flagged_sales(out) == set() -def test_dupe_date_price_still_flags_same_parcel_repeat(tmp_path, monkeypatch): - monkeypatch.chdir(tmp_path) +def test_dupe_date_price_still_flags_same_parcel_repeat(): # The SAME parcel reported twice at the same date and price is a genuine - # duplicate report and must still be dropped; the distinct-parcel deed lots - # alongside it must be kept. - sup = _make_sup( + # duplicate report and must still be flagged; the distinct-parcel deed lots + # alongside it must not be. + df = _df( [ _sale("p1", "s1", "2020-01-01", 75000), _sale("p2", "s2", "2020-01-01", 75000), @@ -61,30 +55,26 @@ def test_dupe_date_price_still_flags_same_parcel_repeat(tmp_path, monkeypatch): _sale("p5", "s5", "2021-01-01", 50000), ] ) - out = run_heuristics(sup, SETTINGS, drop=True) - survivors = set(out.sales["key_sale"]) - assert {"s1", "s2"}.issubset(survivors) - assert survivors.isdisjoint({"s4", "s5"}) + out = flag_dupe_date_price(df) + assert _flagged_sales(out) == {"s4", "s5"} -def test_dupe_date_price_keeps_one_parcel_sold_twice_on_different_dates(tmp_path, monkeypatch): - monkeypatch.chdir(tmp_path) +def test_dupe_date_price_keeps_one_parcel_sold_twice_on_different_dates(): # One parcel with two legitimate sales at DIFFERENT dates is not a duplicate - # and both rows must be kept. - sup = _make_sup( + # and neither row must be flagged. + df = _df( [ _sale("p1", "s1", "2018-05-01", 40000), _sale("p1", "s2", "2022-09-01", 60000), ] ) - out = run_heuristics(sup, SETTINGS, drop=True) - assert set(out.sales["key_sale"]) == {"s1", "s2"} + out = flag_dupe_date_price(df) + assert _flagged_sales(out) == set() -def test_dupe_date_price_with_jurisdiction_keeps_distinct_parcels(tmp_path, monkeypatch): - monkeypatch.chdir(tmp_path) +def test_dupe_date_price_with_jurisdiction_keeps_distinct_parcels(): # The jurisdiction-scoped branch must still keep distinct parcels that share - # a date and price, and still drop a true same-parcel repeat. + # a date and price, and still flag a true same-parcel repeat. sales = [ _sale("p1", "s1", "2020-01-01", 75000), _sale("p2", "s2", "2020-01-01", 75000), @@ -93,9 +83,6 @@ def test_dupe_date_price_with_jurisdiction_keeps_distinct_parcels(tmp_path, monk ] for row in sales: row["county"] = "Acme" - sup = _make_sup(sales) - settings = {"analysis": {"sales_scrutiny": {"jurisdiction": "county"}}} - out = run_heuristics(sup, settings, drop=True) - survivors = set(out.sales["key_sale"]) - assert {"s1", "s2"}.issubset(survivors) - assert survivors.isdisjoint({"s3", "s4"}) + df = _df(sales) + out = flag_dupe_date_price(df, jurisdiction="county") + assert _flagged_sales(out) == {"s3", "s4"} From 857edfec5f4ba9bbe3d59d9247229a975f826f4a Mon Sep 17 00:00:00 2001 From: "fabrica-cc-engineering-agent[bot]" Date: Mon, 22 Jun 2026 17:07:40 -0400 Subject: [PATCH 3/4] Cover dupe-date-price missing-key branches --- openavmkit/utilities/sales_scrutiny.py | 3 ++- tests/test_sales_scrutiny_heuristics.py | 29 +++++++++++++++++++++++++ 2 files changed, 31 insertions(+), 1 deletion(-) diff --git a/openavmkit/utilities/sales_scrutiny.py b/openavmkit/utilities/sales_scrutiny.py index 4b5fab92..e2a17adc 100644 --- a/openavmkit/utilities/sales_scrutiny.py +++ b/openavmkit/utilities/sales_scrutiny.py @@ -17,7 +17,6 @@ def flag_dupe_date_price(df_sales: pd.DataFrame, jurisdiction=None) -> pd.DataFr + "---" + df_sales["sale_price"].astype(str) ) - # Distinct parcels can legitimately share one date/price in a multi-parcel # deed. Include the parcel key so only same-parcel repeats are flagged. if "key" in df_sales.columns: @@ -26,6 +25,8 @@ def flag_dupe_date_price(df_sales: pd.DataFrame, jurisdiction=None) -> pd.DataFr if null_key.any() and "key_sale" in df_sales.columns: parcel_id = parcel_id.mask(null_key, df_sales["key_sale"].astype(str)) date_price = date_price + "---" + parcel_id + elif "key_sale" in df_sales.columns: + date_price = date_price + "---" + df_sales["key_sale"].astype(str) dupes = date_price.value_counts() dupe_keys = dupes[dupes > 1].index.values diff --git a/tests/test_sales_scrutiny_heuristics.py b/tests/test_sales_scrutiny_heuristics.py index f79b8663..7f02e99c 100644 --- a/tests/test_sales_scrutiny_heuristics.py +++ b/tests/test_sales_scrutiny_heuristics.py @@ -86,3 +86,32 @@ def test_dupe_date_price_with_jurisdiction_keeps_distinct_parcels(): df = _df(sales) out = flag_dupe_date_price(df, jurisdiction="county") assert _flagged_sales(out) == {"s3", "s4"} + + +def test_dupe_date_price_without_parcel_key_falls_back_to_sale_key(): + df = pd.DataFrame( + { + "key_sale": ["s1", "s2"], + "sale_date": ["2020-01-01", "2020-01-01"], + "sale_price": [75000, 75000], + } + ) + + out = flag_dupe_date_price(df) + + assert _flagged_sales(out) == set() + + +def test_dupe_date_price_null_parcel_key_falls_back_to_sale_key(): + df = _df( + [ + _sale(None, "s1", "2020-01-01", 75000), + _sale(None, "s2", "2020-01-01", 75000), + _sale(None, "s3", "2021-01-01", 50000), + _sale(None, "s3", "2021-01-01", 50000), + ] + ) + + out = flag_dupe_date_price(df) + + assert _flagged_sales(out) == {"s3"} From 026c2cf390e4bfca5a1bab491a57a69676f601b6 Mon Sep 17 00:00:00 2001 From: "fabrica-cc-engineering-agent[bot]" Date: Mon, 22 Jun 2026 17:27:22 -0400 Subject: [PATCH 4/4] Handle missing sale parcel keys --- openavmkit/utilities/sales_scrutiny.py | 6 +++--- tests/test_sales_scrutiny_heuristics.py | 23 +++++++++++++++++++---- 2 files changed, 22 insertions(+), 7 deletions(-) diff --git a/openavmkit/utilities/sales_scrutiny.py b/openavmkit/utilities/sales_scrutiny.py index e2a17adc..779ac03e 100644 --- a/openavmkit/utilities/sales_scrutiny.py +++ b/openavmkit/utilities/sales_scrutiny.py @@ -21,9 +21,9 @@ def flag_dupe_date_price(df_sales: pd.DataFrame, jurisdiction=None) -> pd.DataFr # deed. Include the parcel key so only same-parcel repeats are flagged. if "key" in df_sales.columns: parcel_id = df_sales["key"].astype(str) - null_key = df_sales["key"].isna() - if null_key.any() and "key_sale" in df_sales.columns: - parcel_id = parcel_id.mask(null_key, df_sales["key_sale"].astype(str)) + missing_key = df_sales["key"].isna() | df_sales["key"].astype(str).str.strip().eq("") + if missing_key.any() and "key_sale" in df_sales.columns: + parcel_id = parcel_id.mask(missing_key, df_sales["key_sale"].astype(str)) date_price = date_price + "---" + parcel_id elif "key_sale" in df_sales.columns: date_price = date_price + "---" + df_sales["key_sale"].astype(str) diff --git a/tests/test_sales_scrutiny_heuristics.py b/tests/test_sales_scrutiny_heuristics.py index 7f02e99c..9d8d6cef 100644 --- a/tests/test_sales_scrutiny_heuristics.py +++ b/tests/test_sales_scrutiny_heuristics.py @@ -91,15 +91,15 @@ def test_dupe_date_price_with_jurisdiction_keeps_distinct_parcels(): def test_dupe_date_price_without_parcel_key_falls_back_to_sale_key(): df = pd.DataFrame( { - "key_sale": ["s1", "s2"], - "sale_date": ["2020-01-01", "2020-01-01"], - "sale_price": [75000, 75000], + "key_sale": ["s1", "s2", "s3", "s3"], + "sale_date": ["2020-01-01", "2020-01-01", "2021-01-01", "2021-01-01"], + "sale_price": [75000, 75000, 50000, 50000], } ) out = flag_dupe_date_price(df) - assert _flagged_sales(out) == set() + assert _flagged_sales(out) == {"s3"} def test_dupe_date_price_null_parcel_key_falls_back_to_sale_key(): @@ -115,3 +115,18 @@ def test_dupe_date_price_null_parcel_key_falls_back_to_sale_key(): out = flag_dupe_date_price(df) assert _flagged_sales(out) == {"s3"} + + +def test_dupe_date_price_blank_parcel_key_falls_back_to_sale_key(): + df = _df( + [ + _sale("", "s1", "2020-01-01", 75000), + _sale(" ", "s2", "2020-01-01", 75000), + _sale("", "s3", "2021-01-01", 50000), + _sale("", "s3", "2021-01-01", 50000), + ] + ) + + out = flag_dupe_date_price(df) + + assert _flagged_sales(out) == {"s3"}