fix(yandex-valuation): tighten area regex + chronological date ordering #541

Merged

lekss361 merged 1 commit from fix/tradein-yandex-parser-quality into main

2026-05-24 15:37:48 +00:00

lekss361 commented

2026-05-24 15:29:03 +00:00

Owner

Summary

3 parser quality bugs observed live on Базовый 52 sweep (132 rows):

A. area_m2 captured year+area concat like 202442.6 from chunked text where DOM whitespace was stripped between publish-year and area tokens → 52% of rows had area dropped by sanity cap. Fix: RE_ITEM_AREA gets negative lookbehind (?<![\d.,]) + 4-digit cap on whole part.

B. area_m2 = 2.20 for 2-к/8M ₽ flats — same regex matched a sub-fragment 2,2 inside 52,2. Same fix as A.

C. Date inversion — start_price_date > removed_date on ~1% of rows. _parse_item_text assigned by page-text order, not chronologically. Fix: sort parsed dates list before splitting into publish/removed.

Tests

33/33 pytest pass (28 prior + 5 new regression tests)
ruff clean
Sanity cap from PR #538 stays as belt-and-suspenders for any edge case

Followup

Operator: clear external_valuations cache for yandex_valuation before next sweep (cached rows still hold old parses)
Re-sweep observes: more area_m2 filled, no inverted dates

## Summary 3 parser quality bugs observed live on Базовый 52 sweep (132 rows): **A. `area_m2` captured year+area concat** like `202442.6` from chunked text where DOM whitespace was stripped between publish-year and area tokens → 52% of rows had area dropped by sanity cap. Fix: `RE_ITEM_AREA` gets negative lookbehind `(?<![\d.,])` + 4-digit cap on whole part. **B. `area_m2 = 2.20`** for 2-к/8M ₽ flats — same regex matched a sub-fragment `2,2` inside `52,2`. Same fix as A. **C. Date inversion** — `start_price_date > removed_date` on ~1% of rows. `_parse_item_text` assigned by page-text order, not chronologically. Fix: sort parsed dates list before splitting into publish/removed. ## Tests - 33/33 pytest pass (28 prior + 5 new regression tests) - ruff clean - Sanity cap from PR #538 stays as belt-and-suspenders for any edge case ## Followup - Operator: clear `external_valuations` cache for yandex_valuation before next sweep (cached rows still hold old parses) - Re-sweep observes: more area_m2 filled, no inverted dates

lekss361 added 1 commit 2026-05-24 15:29:03 +00:00

fix(yandex-valuation): tighten area regex + chronological date ordering 961bdc0225

Three parser quality bugs observed on prod (Базовый 52 sweep, 132 rows):

A. RE_ITEM_AREA captured year+area concat like '202442,6' from chunked text
   where DOM whitespace was stripped between publish-year and area tokens.
   Added negative lookbehind + 4-digit cap on whole part.

B. Same regex sometimes matched a sub-fragment '2,2' inside '52,2' — same
   root cause, fixed by the tightened regex.

C. publish_date / removed_date were assigned by page-text order, not
   chronologically. ~1% of rows on prod had removed < publish. Sort the
   parsed date list before splitting.

Cap from prior PR #538 stays as belt-and-suspenders for any junk that
still slips through.

lekss361 commented

2026-05-24 15:37:27 +00:00

Author

Owner

Deep Code Review — verdict APPROVE

Scope: 2 files (P1 scraper + tests), +78 / -3, single-domain (backend tradein scraper). Surgical fix on the heels of #508/#526/#538.

Correctness — sound

A. Regex tightening (?<![\d.,])(\d{1,4}(?:[.,]\d{1,2})?)\s*м²

Negative lookbehind on [\d.,] correctly blocks the 2024 + 42,6 concat → 20242,6 failure on chunked text. Traced position-by-position for 202452,2 м² input — regex engine has no valid starting position after fix.
\d{1,4} whole-part cap is safe: flats are <500 m², 4-digit ceiling = 9999 m² leaves room without enabling overflow rows. Existing > 10_000 sanity cap at line 297 stays as belt-and-suspenders (acknowledged in PR body).
[.,]\d{1,2} decimal cap matches observed Yandex precision (1–2 dp).
No false-negative on legitimate edges I could construct: leading char of text/chunk, post-whitespace, post-paren, post-dash all match. 42,6 м² at start-of-string passes lookbehind (no prior char).

B. Chronological date sort

Sort + index [0]/[1] is the correct semantic fix for removed_date > start_price_date inversion observed on ~1% prod rows. publish_date is by definition min(dates), removed_date is max(dates) for ≤2 dates.
The redundant for d in dates_parsed if d is not None inside sorted(...) is harmless — dates_parsed.append at line 325 already gates on non-None.

Cross-file impact — author-aware

estimator.py:511 builds ext_item_id = sha256(address|publish_date|area_m2|floor|prices). Both fixes change area_m2 from bogus → None (or different parsed value) → fresh ext_item_id → ON CONFLICT (source, ext_item_id) DO NOTHING won't dedupe old zombie rows.
PR body explicitly notes operator action: clear external_valuations cache for yandex_valuation before next sweep. Acknowledged followup — not a blocker.
Downstream readers (trade_in.py:520, schema start_price_date/removed_date) consume dates with ORDER BY COALESCE(last_price_date, start_price_date) — chronological sort is monotonic-friendly, no breakage.

Tests — 4/5 sharp, 1 non-discriminating

test_area_regex_rejects_year_concat — verified by hand: regex finds no valid starting position in 202452,2 м². Test will fail if lookbehind ever drops. ✓
test_area_regex_accepts_isolated_token — 58,2 м² after whitespace → matches. ✓
test_area_regex_capped_at_4_digits — 12345 м² rejected (greedy \d{1,4} matches 1234, then \s*м² requires space/м next, but next is 5; backtracking impossible due to lookbehind blocking lower-pos starts on digit-runs). ✓
test_single_date_is_publish_only — single date assigned to publish, not removed. ✓
test_dates_sorted_publish_before_removed — 🟢 Low (test quality): text input has dates 13.08.2025 then 12.02.2026 which are already in chronological order in page-text. Pre-PR code (dates_parsed[0] / dates_parsed[1]) would yield the same assignment, so this test would pass even with the sort line removed. Comment claims "reversed order" but it isn't. Suggest swapping to e.g. 13.08.2026 В экспозиции 156 дней 12.02.2025 (later date first) to actually exercise the sort. Not a blocker — the other 4 tests + the prod observation justify the change.

Performance / Security / Conventions

Bounded \d{1,4} / \d{1,2} quantifiers are faster than the prior unbounded \d+[.,]?\d*. Sort of ≤2-3 items is O(1) practically.
No new I/O, no SQL, no token handling.
ruff-clean per PR body; matches existing module style.

Positive

Clear # Negative lookbehind on... and # Sort dates chronologically — comments inline document the why, including production observation rate.
PR body lists exact bug A/B/C with prod observation rates and provides explicit operator followup. Reviewer-friendly.
Builds incrementally on #538's sanity cap without removing it (defense in depth preserved).

Recommended next steps (post-merge)

Operator: clear external_valuations cache for source = 'yandex_valuation' before next Yandex sweep so the zombie rows get re-parsed with the tighter regex.
On next sweep, spot-check house_placement_history for: any area_m2 > 1000, any rows with removed_date < start_price_date, distribution of area_m2 IS NULL (should drop from 52% to single digits if PR A landed).
Followup test tightening (optional): swap dates in test_dates_sorted_publish_before_removed so the page-text order is genuinely reversed.

Complexity / blast radius

Risk: Low — parser-only, no schema/API/contract change, no migrations.
Reversibility: Trivial — revert the regex + sort hunks, 13 lines.
Merge window: Any.

## Deep Code Review — verdict APPROVE **Scope:** 2 files (P1 scraper + tests), +78 / -3, single-domain (backend tradein scraper). Surgical fix on the heels of #508/#526/#538. ### Correctness — sound **A. Regex tightening `(?<![\d.,])(\d{1,4}(?:[.,]\d{1,2})?)\s*м²`** - Negative lookbehind on `[\d.,]` correctly blocks the `2024` + `42,6` concat → `20242,6` failure on chunked text. Traced position-by-position for `202452,2 м²` input — regex engine has no valid starting position after fix. - `\d{1,4}` whole-part cap is safe: flats are <500 m², 4-digit ceiling = 9999 m² leaves room without enabling overflow rows. Existing `> 10_000` sanity cap at line 297 stays as belt-and-suspenders (acknowledged in PR body). - `[.,]\d{1,2}` decimal cap matches observed Yandex precision (1–2 dp). - No false-negative on legitimate edges I could construct: leading char of text/chunk, post-whitespace, post-paren, post-dash all match. `42,6 м²` at start-of-string passes lookbehind (no prior char). **B. Chronological date sort** - Sort + index `[0]/[1]` is the correct semantic fix for `removed_date > start_price_date` inversion observed on ~1% prod rows. `publish_date` is by definition `min(dates)`, `removed_date` is `max(dates)` for ≤2 dates. - The redundant `for d in dates_parsed if d is not None` inside `sorted(...)` is harmless — `dates_parsed.append` at line 325 already gates on non-None. ### Cross-file impact — author-aware - `estimator.py:511` builds `ext_item_id = sha256(address|publish_date|area_m2|floor|prices)`. Both fixes change `area_m2` from bogus → `None` (or different parsed value) → fresh `ext_item_id` → `ON CONFLICT (source, ext_item_id) DO NOTHING` won't dedupe old zombie rows. - PR body **explicitly** notes operator action: clear `external_valuations` cache for yandex_valuation before next sweep. Acknowledged followup — not a blocker. - Downstream readers (`trade_in.py:520`, schema `start_price_date`/`removed_date`) consume dates with `ORDER BY COALESCE(last_price_date, start_price_date)` — chronological sort is monotonic-friendly, no breakage. ### Tests — 4/5 sharp, 1 non-discriminating - `test_area_regex_rejects_year_concat` — verified by hand: regex finds no valid starting position in `202452,2 м²`. Test will fail if lookbehind ever drops. ✓ - `test_area_regex_accepts_isolated_token` — `58,2 м²` after whitespace → matches. ✓ - `test_area_regex_capped_at_4_digits` — `12345 м²` rejected (greedy `\d{1,4}` matches `1234`, then `\s*м²` requires space/`м` next, but next is `5`; backtracking impossible due to lookbehind blocking lower-pos starts on digit-runs). ✓ - `test_single_date_is_publish_only` — single date assigned to publish, not removed. ✓ - `test_dates_sorted_publish_before_removed` — 🟢 **Low (test quality):** text input has dates `13.08.2025` then `12.02.2026` which are already in chronological order in page-text. Pre-PR code (`dates_parsed[0]` / `dates_parsed[1]`) would yield the same assignment, so this test would pass even with the sort line removed. Comment claims "reversed order" but it isn't. Suggest swapping to e.g. `13.08.2026 В экспозиции 156 дней 12.02.2025` (later date first) to actually exercise the sort. Not a blocker — the other 4 tests + the prod observation justify the change. ### Performance / Security / Conventions - Bounded `\d{1,4}` / `\d{1,2}` quantifiers are faster than the prior unbounded `\d+[.,]?\d*`. Sort of ≤2-3 items is O(1) practically. - No new I/O, no SQL, no token handling. - ruff-clean per PR body; matches existing module style. ### Positive - Clear `# Negative lookbehind on...` and `# Sort dates chronologically —` comments inline document the *why*, including production observation rate. - PR body lists exact bug A/B/C with prod observation rates and provides explicit operator followup. Reviewer-friendly. - Builds incrementally on #538's sanity cap without removing it (defense in depth preserved). ### Recommended next steps (post-merge) 1. Operator: clear `external_valuations` cache for `source = 'yandex_valuation'` before next Yandex sweep so the zombie rows get re-parsed with the tighter regex. 2. On next sweep, spot-check `house_placement_history` for: any `area_m2 > 1000`, any rows with `removed_date < start_price_date`, distribution of `area_m2 IS NULL` (should drop from 52% to single digits if PR A landed). 3. Followup test tightening (optional): swap dates in `test_dates_sorted_publish_before_removed` so the page-text order is genuinely reversed. ### Complexity / blast radius - **Risk:** Low — parser-only, no schema/API/contract change, no migrations. - **Reversibility:** Trivial — revert the regex + sort hunks, 13 lines. - **Merge window:** Any.