fix(tradein/estimator): analogs cards match n_analogs, no cross-source dups #2518

Merged

lekss361 merged 1 commit from fix/tradein-analogs-count-dedup into main

2026-07-13 20:19:21 +00:00

lekss361 commented

2026-07-13 20:05:44 +00:00

Owner

Summary

Display-layer fix for two bugs surfaced by pre-launch smoke (golden-path address "Екатеринбург, улица Малышева, 51"):

Card count != claimed n_analogs. n_analogs (same-building anchor path) was computed from the post-MAD-clip surviving comp population, but the displayed analogs cards were built from the ORIGINAL, un-clipped anchor_comps list. A MAD-clip-excluded price outlier (e.g. a cross-source duplicate priced ~4.5x above the rest) never affected the median/n_analogs, yet still showed up as a card. Fix: _compute_same_building_anchor now returns the final surviving comps list (len == n), and the caller sets anchor_comps_used = anchor["comps"] instead of the raw pre-clip anchor_comps.
Cross-source duplicate of the same physical unit shown twice. The statistical cross-source dedup (_dedup_cross_source) requires the same price_bucket (rounded price_rub) to consider two listings duplicates. Real cross-posts of the same physical unit almost always have a small asking-price drift between platforms, which can straddle a bucket boundary and let the duplicate survive into both the price population and the UI. Fix: a new display-only dedup pass (_dedup_display_lots / _union_find_phys_dedup) uses the same physical key (cadnum-or-street + floor + area_bucket) but WITHOUT price_bucket, applied only when building the displayed analogs_lots — never touches n_analogs/median/cv. To avoid over-merging genuinely distinct same-building units that happen to share floor/area, it requires the two lots to come from DIFFERENT sources (require_diff_source guard) — same-source pairs are left untouched.

The radius-path (non-anchor) branch had a related count-consistency gap: n_analogs counted only PRICED listings_clean entries, while the displayed cards were sliced from the full (unfiltered) listings_clean, occasionally letting price-less entries inflate the card count beyond n_analogs. Cards are now built from the same priced subset.

Backtest / frozen gate result

tests/test_backtest_regression_gate.py (hermetic replay of committed frozen fixture) is byte-identical after this change — zero delta on median/expected_sold/coverage/calibration (rel_tol=1e-6). Confirms the fix is display-only and does not touch pricing. Live scripts/backtest_estimator.py against prod DB was not run in this sandboxed session (no DB credentials available here) — the hermetic gate is the authoritative frozen-metric check per the codebase's own test docstring; happy to run the live variant on request.

Test plan

pytest tests/ -k "estimator or backtest" — 548 passed
pytest tests/ (full suite) — 2526 passed, 6 skipped, 1 pre-existing unrelated failure (test_search_api.py::test_search_cache_hit, confirmed failing identically on main before this change — auth/env issue, unrelated to estimator.py)
tests/test_backtest_regression_gate.py — 2 passed, byte-identical to frozen baseline
New tests/test_estimator_analogs_display_consistency.py (6 tests): full-pipeline regression for both root causes (outlier duplicate pair fully excluded from both count and cards; non-outlier duplicate pair collapses in cards only, honestly documenting len(analogs) <= n_analogs when display-only dedup catches a residual cross-source dup the stat dedup missed) + unit tests on _dedup_display_lots
Updated tests/test_estimator_quarter_index.py's hand-rolled _compute_same_building_anchor test double to match the new "comps" contract
ruff check clean on all changed files

## Summary Display-layer fix for two bugs surfaced by pre-launch smoke (golden-path address "Екатеринбург, улица Малышева, 51"): 1. **Card count != claimed n_analogs.** `n_analogs` (same-building anchor path) was computed from the post-MAD-clip surviving comp population, but the displayed `analogs` cards were built from the ORIGINAL, un-clipped `anchor_comps` list. A MAD-clip-excluded price outlier (e.g. a cross-source duplicate priced ~4.5x above the rest) never affected the median/n_analogs, yet still showed up as a card. Fix: `_compute_same_building_anchor` now returns the final surviving `comps` list (`len == n`), and the caller sets `anchor_comps_used = anchor["comps"]` instead of the raw pre-clip `anchor_comps`. 2. **Cross-source duplicate of the same physical unit shown twice.** The statistical cross-source dedup (`_dedup_cross_source`) requires the same `price_bucket` (rounded `price_rub`) to consider two listings duplicates. Real cross-posts of the same physical unit almost always have a small asking-price drift between platforms, which can straddle a bucket boundary and let the duplicate survive into both the price population and the UI. Fix: a new display-only dedup pass (`_dedup_display_lots` / `_union_find_phys_dedup`) uses the same physical key (cadnum-or-street + floor + area_bucket) but WITHOUT `price_bucket`, applied only when building the displayed `analogs_lots` — never touches `n_analogs`/`median`/`cv`. To avoid over-merging genuinely distinct same-building units that happen to share floor/area, it requires the two lots to come from DIFFERENT sources (`require_diff_source` guard) — same-source pairs are left untouched. The radius-path (non-anchor) branch had a related count-consistency gap: `n_analogs` counted only PRICED `listings_clean` entries, while the displayed cards were sliced from the full (unfiltered) `listings_clean`, occasionally letting price-less entries inflate the card count beyond `n_analogs`. Cards are now built from the same priced subset. ## Backtest / frozen gate result `tests/test_backtest_regression_gate.py` (hermetic replay of committed frozen fixture) is **byte-identical after this change** — zero delta on median/expected_sold/coverage/calibration (rel_tol=1e-6). Confirms the fix is display-only and does not touch pricing. Live `scripts/backtest_estimator.py` against prod DB was not run in this sandboxed session (no DB credentials available here) — the hermetic gate is the authoritative frozen-metric check per the codebase's own test docstring; happy to run the live variant on request. ## Test plan - [x] `pytest tests/ -k "estimator or backtest"` — 548 passed - [x] `pytest tests/` (full suite) — 2526 passed, 6 skipped, 1 pre-existing unrelated failure (`test_search_api.py::test_search_cache_hit`, confirmed failing identically on `main` before this change — auth/env issue, unrelated to estimator.py) - [x] `tests/test_backtest_regression_gate.py` — 2 passed, byte-identical to frozen baseline - [x] New `tests/test_estimator_analogs_display_consistency.py` (6 tests): full-pipeline regression for both root causes (outlier duplicate pair fully excluded from both count and cards; non-outlier duplicate pair collapses in cards only, honestly documenting `len(analogs) <= n_analogs` when display-only dedup catches a residual cross-source dup the stat dedup missed) + unit tests on `_dedup_display_lots` - [x] Updated `tests/test_estimator_quarter_index.py`'s hand-rolled `_compute_same_building_anchor` test double to match the new `"comps"` contract - [x] `ruff check` clean on all changed files

lekss361 added 1 commit 2026-07-13 20:05:44 +00:00

fix(tradein/estimator): analogs cards match n_analogs, no cross-source dups

CI Trade-In / frontend-checks (pull_request) Has been skipped

Details

CI / openapi-codegen-check (pull_request) Has been skipped

Details

CI Trade-In / backend-tests (pull_request) Successful in 55s

Details

CI Trade-In / changes (pull_request) Successful in 8s

Details

CI / changes (pull_request) Successful in 8s

Details

CI / backend-tests (pull_request) Has been skipped

Details

CI / frontend-tests (pull_request) Has been skipped

Details

51c79f9324

Two display-layer bugs surfaced by pre-launch smoke (golden-path address
"Екатеринбург, улица Малышева, 51"):

1. n_analogs (same-building anchor path) was computed from the post-MAD-clip
   comp population (_compute_same_building_anchor: n = len(surviving ppm2)),
   but the displayed analogs cards were built from the ORIGINAL, un-clipped
   anchor_comps list. A MAD-clip-excluded price outlier (e.g. a cross-source
   duplicate priced ~4.5x above the rest) never affected the median/n_analogs,
   yet still showed up as a card.

   Fix: _compute_same_building_anchor now returns the final surviving "comps"
   list (len == n), and the caller sets anchor_comps_used = anchor["comps"]
   instead of the raw pre-clip anchor_comps.

2. The statistical cross-source dedup (_dedup_cross_source) requires the same
   price_bucket (rounded price_rub) to consider two listings duplicates. Real
   cross-posts of the same physical unit almost always have a small asking-
   price drift between platforms, which can straddle a bucket boundary and
   let the duplicate survive into both the price population and the UI.

   Fix: a new display-only dedup pass (_dedup_display_lots /
   _union_find_phys_dedup) uses the same physical key (cadnum-or-street +
   floor + area_bucket) but WITHOUT price_bucket, applied only when building
   the displayed analogs_lots — never touches n_analogs/median/cv. To avoid
   over-merging genuinely distinct same-building units that happen to share
   floor/area, it requires the two lots to come from DIFFERENT sources
   (require_diff_source guard) — same-source pairs are left untouched.

The radius-path (non-anchor) branch had a related count-consistency gap:
n_analogs counted only PRICED listings_clean entries, while the displayed
cards were sliced from the full (unfiltered) listings_clean, occasionally
letting price-less entries inflate the card count beyond n_analogs. Cards
are now built from the same priced subset.

Frozen backtest regression gate (tests/test_backtest_regression_gate.py,
hermetic replay of committed fixture) is byte-identical after this change —
zero delta on median/expected_sold/coverage/calibration, confirming this is
display-only and does not touch pricing.

Added tests/test_estimator_analogs_display_consistency.py: full-pipeline
regression for both root causes (outlier duplicate pair fully excluded from
both count and cards; non-outlier duplicate pair collapses in cards only,
honestly documenting len(analogs) <= n_analogs when the display-only dedup
catches a residual cross-source dup the stat dedup missed) + unit tests on
_dedup_display_lots. Updated test_estimator_quarter_index.py's hand-rolled
_compute_same_building_anchor test double to match the new "comps" contract.