fix(yandex-valuation): cap area_m2 sanity (drop >10000 or ≤0) #538

Merged

lekss361 merged 1 commit from fix/tradein-yandex-area-sanity-cap into main

2026-05-24 15:14:14 +00:00

lekss361 commented

2026-05-24 15:09:53 +00:00

Owner

Summary

_parse_item_text drops area_m2 values > 10_000 m² or ≤ 0 (sets to None)
Logs warning so we can audit how often the upstream parser produces junk
Preserves remaining fields (price, dates, status) — the row still persists with area_m2=NULL instead of blowing up the entire batch

Why

Live observation from sweep on Базовый переулок, 52 (PR #536 follow-up):

psycopg.errors.NumericValueOutOfRange: 
A field with precision 8, scale 2 must round to <10^6
area: 2025106.7

house_placement_history.area_m2 is NUMERIC(8,2) → values ≥10⁶ overflow. The 2_025_106.7 came from Yandex page text occasionally concatenating digits across DOM boundaries. Whole batch of 30 items from page=1 was lost. After this PR, junk area is dropped and the row still saves with the other fields intact.

Tests

28/28 pytest pass (25 pre-existing + 3 new regression tests for overflow / normal / zero area)
ruff clean

## Summary - `_parse_item_text` drops `area_m2` values > 10_000 m² or ≤ 0 (sets to None) - Logs warning so we can audit how often the upstream parser produces junk - Preserves remaining fields (price, dates, status) — the row still persists with `area_m2=NULL` instead of blowing up the entire batch ## Why Live observation from sweep on `Базовый переулок, 52` (PR #536 follow-up): ``` psycopg.errors.NumericValueOutOfRange: A field with precision 8, scale 2 must round to <10^6 area: 2025106.7 ``` `house_placement_history.area_m2` is `NUMERIC(8,2)` → values ≥10⁶ overflow. The 2_025_106.7 came from Yandex page text occasionally concatenating digits across DOM boundaries. Whole batch of 30 items from page=1 was lost. After this PR, junk area is dropped and the row still saves with the other fields intact. ## Tests - 28/28 pytest pass (25 pre-existing + 3 new regression tests for overflow / normal / zero area) - ruff clean

lekss361 added 1 commit 2026-05-24 15:09:54 +00:00

fix(yandex-valuation): cap area_m2 sanity (drop >10000 or <=0) 4939d04360

Live observation: sweep on "Базовый переулок, 52" produced area_m2=2_025_106.7
(concatenated digits across DOM boundaries) -> psycopg.NumericValueOutOfRange on
INSERT, lost entire batch of 30 items from page=1.

Cap at 10_000 m2 (residential ceiling) and drop <=0. Keeps the rest of the
item (price, dates, status) so the row still persists with area_m2=NULL --
better than losing the whole batch.

lekss361 commented

2026-05-24 15:14:06 +00:00

Author

Owner

Deep Code Review — verdict APPROVE

Summary

Status: APPROVE
Files: 2 (P1: scraper, P2: test) · +58/-1 · ~5 min
PR #538, head sha 4939d04

Key checks

Where applied: parse-time in _parse_item_text — single chokepoint serving both Strategy 1 (data-test container) and Strategy 2 (chunked-text fallback). Correct placement: the bug originates in the chunked-text path (200-char window swallows digits across DOM boundaries), and parse-time drop is preferable to query-time filtering (junk never enters DB → no retroactive cleanup churn).

Lower bound (area_m2 <= 0): handled, test test_parse_item_drops_zero_area covers 0 м². Reasonable to NOT pick an arbitrary residential floor (e.g. <5 m²) — storage rooms and micro-units exist; drop only on impossible-by-definition values.

Upper bound (10 000 m²): safe ceiling. Yandex /otsenka-kvartiry-po-adresu-onlayn/ URL is APARTMENT valuation (not commercial), so false-negatives for промзоны/торговые центры are out of scope. DB column house_placement_history.area_m2 numeric(8,2) overflows at 10⁶ — 10k leaves 100× safety margin.

Retroactive cleanup: not needed. NumericValueOutOfRange aborted the whole batch (cited in PR body: «30 items from page=1 was lost»), so no historical rows with area > 10⁶ in house_placement_history for source='yandex_valuation'. Constraint actively rejected bad data. Zero-area rows may exist historically but are harmless (dedup key includes area_m2, downstream estimator.py:858 area_m2 BETWEEN :area_min AND :area_max correctly excludes NULL).

Observability: logger.warning("yandex_valuation: dropping nonsensical area_m2=%s …") — good audit trail for upstream parser regressions.

Persistence gap check (memory rule): _save_yandex_history_items at estimator.py:380 references item.area_m2 directly, no parallel field to forget — clean.

Dedup interaction: when multiple items in a chunk get cap'd, they collapse to (date, None, floor) in seen set — degradation rather than crash. Acceptable.

Tests: 28/28 passing claimed. Three new regression tests cover overflow, normal pass-through, zero-area. Coverage matches intent.

Vault cross-ref: fits the Yandex parser fix series — PR #508 (listing_date), PR #526 (removed_date+total_floors+NBSP). All driven by chunked-text strategy edge cases; series suggests text-chunking robustness should remain a watch-item.

Minor observations (non-blocking)

No explicit test for negative area_m2 (e.g. -5 м²), but <= 0 semantics are unambiguous — regex unlikely to emit a sign anyway.
No upper-boundary test (10 000.0 keep vs 10 000.01 drop) — also non-critical.
Type annotation area_m2: float | None properly explicit for mypy.

Positive

Surgical fix at the right layer.
Constraint-overflow root cause correctly diagnosed and worked around without DDL changes.
Logging + tests + clear PR body with reproduction text.

Risk / blast radius

Risk: LOW · Reversibility: trivial revert · Merge window: any
No SQL, no auth, no API contract change. Pure parser hardening.

## Deep Code Review — verdict APPROVE ### Summary - Status: APPROVE - Files: 2 (P1: scraper, P2: test) · +58/-1 · ~5 min - PR #538, head sha 4939d04 ### Key checks **Where applied**: parse-time in `_parse_item_text` — single chokepoint serving both Strategy 1 (data-test container) and Strategy 2 (chunked-text fallback). Correct placement: the bug originates in the chunked-text path (200-char window swallows digits across DOM boundaries), and parse-time drop is preferable to query-time filtering (junk never enters DB → no retroactive cleanup churn). **Lower bound** (`area_m2 <= 0`): handled, test `test_parse_item_drops_zero_area` covers `0 м²`. Reasonable to NOT pick an arbitrary residential floor (e.g. <5 m²) — storage rooms and micro-units exist; drop only on impossible-by-definition values. **Upper bound** (10 000 m²): safe ceiling. Yandex `/otsenka-kvartiry-po-adresu-onlayn/` URL is APARTMENT valuation (not commercial), so false-negatives for промзоны/торговые центры are out of scope. DB column `house_placement_history.area_m2 numeric(8,2)` overflows at 10⁶ — 10k leaves 100× safety margin. **Retroactive cleanup**: not needed. NumericValueOutOfRange aborted the whole batch (cited in PR body: «30 items from page=1 was lost»), so no historical rows with area > 10⁶ in `house_placement_history` for `source='yandex_valuation'`. Constraint actively rejected bad data. Zero-area rows may exist historically but are harmless (dedup key includes `area_m2`, downstream `estimator.py:858` `area_m2 BETWEEN :area_min AND :area_max` correctly excludes NULL). **Observability**: `logger.warning("yandex_valuation: dropping nonsensical area_m2=%s …")` — good audit trail for upstream parser regressions. **Persistence gap check** (memory rule): `_save_yandex_history_items` at `estimator.py:380` references `item.area_m2` directly, no parallel field to forget — clean. **Dedup interaction**: when multiple items in a chunk get cap'd, they collapse to `(date, None, floor)` in `seen` set — degradation rather than crash. Acceptable. **Tests**: 28/28 passing claimed. Three new regression tests cover overflow, normal pass-through, zero-area. Coverage matches intent. **Vault cross-ref**: fits the Yandex parser fix series — PR #508 (listing_date), PR #526 (removed_date+total_floors+NBSP). All driven by chunked-text strategy edge cases; series suggests text-chunking robustness should remain a watch-item. ### Minor observations (non-blocking) - No explicit test for negative `area_m2` (e.g. `-5 м²`), but `<= 0` semantics are unambiguous — regex unlikely to emit a sign anyway. - No upper-boundary test (10 000.0 keep vs 10 000.01 drop) — also non-critical. - Type annotation `area_m2: float | None` properly explicit for mypy. ### Positive - Surgical fix at the right layer. - Constraint-overflow root cause correctly diagnosed and worked around without DDL changes. - Logging + tests + clear PR body with reproduction text. ### Risk / blast radius - Risk: LOW · Reversibility: trivial revert · Merge window: any - No SQL, no auth, no API contract change. Pure parser hardening.