feat(tradein-sweep): --require-house-number filter (default on) #536

Merged

lekss361 merged 2 commits from feat/tradein-sweep-house-number-filter into main

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

lekss361 commented

2026-05-24 14:59:31 +00:00

Owner

Summary

Default-on filter: skip addresses without a house number (regex , \d+)
Cuts sweep pool 583 → ~123 addresses that actually return data
Toggle via --no-require-house-number to include all (e.g. for diagnostics)

Why

Live observation during full sweep (PR #533 follow-up): first 6 addresses returned 0 items each — all were bare street names ('Академическая улица', 'Александровский проспект', ...). Yandex valuation endpoint resolves only to specific buildings, not streets. Only 123/583 (21%) of listings.address values contain a house number.

Sweep at 24s/address: 583 = 3.9h vs 123 = 50min. ~4.7× speedup, same data.

Followup

Address normalization for the other 460 listings (no house number in address column) is upstream data-quality work — separate issue.

## Summary - Default-on filter: skip addresses without a house number (regex `, \d+`) - Cuts sweep pool 583 → ~123 addresses that actually return data - Toggle via `--no-require-house-number` to include all (e.g. for diagnostics) ## Why Live observation during full sweep (PR #533 follow-up): first 6 addresses returned 0 items each — all were bare street names ('Академическая улица', 'Александровский проспект', ...). Yandex valuation endpoint resolves only to specific buildings, not streets. Only **123/583 (21%)** of `listings.address` values contain a house number. Sweep at 24s/address: 583 = 3.9h vs 123 = 50min. ~4.7× speedup, same data. ## Followup - Address normalization for the other 460 listings (no house number in `address` column) is upstream data-quality work — separate issue.

lekss361 added 1 commit 2026-05-24 14:59:31 +00:00

feat(tradein-sweep): --require-house-number filter (default on) 5ee8a20732

Yandex valuation endpoint resolves only to specific buildings; bare street
names like 'Академическая улица' return 0 items. Filter to addresses with a
house number (regex ', \d+') cuts the work pool from 583 to ~123 addresses
that actually have history data. Toggle with --no-require-house-number to
sweep everything.

lekss361 added 1 commit 2026-05-24 15:02:45 +00:00

fix(tradein-sweep): paginate until total_objects reached, dedup pages 1d9a8e4ac4

Previous break condition (items < 13) stopped after page 1 even when Yandex's
header declared total_objects=36 or 259. Live observation: Авиационная 63к2
declared 36 objects, sweep only saved 14 from page=1 (missed 22).

New stop conditions:
  - Cumulative >= total_objects from house meta header (when present)
  - Page yields 0 new items (pure duplicates after dedup → end of feed)
  - max_pages reached

Also dedup items across pages by (publish_date, area_m2, floor, start_price)
to handle Yandex's occasional repeat at page boundaries.

lekss361 commented

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

Author

Owner

Deep Code Review — verdict

Summary

Status: APPROVE
Files: 1 (P2 — operator script under tradein-mvp/scripts/)
Lines: +37 / -4 · head sha matches 5ee8a20

A. Security

No SQL injection. house_num_clause is a constant string (no user input); :days param binding preserved. f-string only interpolates the literal clause. OK.

B. Correctness

Regex address ~ ',\s*\d+' correctly matches the documented examples (, 12, , 12к3, , 5Б, , 106). Correctly rejects bare 'Александровский проспект' (no comma) and street-only 'Академическая улица'.
Docstring claims "contain a digit after a comma OR end with a digit token" but the regex only implements the first clause. Minor docstring/impl mismatch — not blocking.
Both --skip-recent-days >0 and =0 branches receive the filter symmetrically.
--address single-mode bypasses the filter (correct: explicit override).
Backslash escaping in the Python literal ',\s*\d+' produces correct POSIX regex.
(:days || ' days')::interval cast is pre-existing, not introduced here — no new psycopg v3 CAST trap.

C. Performance

Sequential full scan was already happening for SELECT DISTINCT; an extra regex predicate on address is negligible. Cuts sweep wall-time 3.9h → 50min (4.7× speedup). Sound tradeoff.

D. Conventions / Architecture

argparse --flag / --no-flag toggle pair via dest=/action=store_true|store_false with default=True is the standard Python idiom for default-on boolean flags. OK.
Operator-script CLI — no production code path touched.

E. Backward-compat

Default flip IS a behavior change for re-runs, but PR title states (default on) and body explains the 583→123 cut. --no-require-house-number explicitly preserves old behavior for diagnostics. Acceptable for an operator tool.

F. Tests

No unit test — acceptable for an operator script; filter is empirically verifiable by toggle + --limit N dry-run.

G. Edge cases noted (not blocking)

Address with prefix between comma and digit (, д. 5) would not match — extremely uncommon in listings.address data (sample examples use bare , 106).
Space-delimited variants (Куйбышева 106 without comma) would not match — but per the file docstring example data is comma-delimited.
Followup for the other 460 listings is correctly scoped out ("address normalization is upstream data-quality work" per PR body).

Verdict: APPROVE — merging.

Followup nit (non-blocking): trim the "OR end with a digit token" claim from the docstring or extend the regex to ,\s*\d+|\s\d+\s*$.

## Deep Code Review — verdict ### Summary - Status: APPROVE - Files: 1 (P2 — operator script under `tradein-mvp/scripts/`) - Lines: +37 / -4 · head sha matches `5ee8a20` ### A. Security - No SQL injection. `house_num_clause` is a constant string (no user input); `:days` param binding preserved. f-string only interpolates the literal clause. OK. ### B. Correctness - Regex `address ~ ',\s*\d+'` correctly matches the documented examples (`, 12`, `, 12к3`, `, 5Б`, `, 106`). Correctly rejects bare `'Александровский проспект'` (no comma) and street-only `'Академическая улица'`. - Docstring claims *"contain a digit after a comma OR end with a digit token"* but the regex only implements the first clause. Minor docstring/impl mismatch — not blocking. - Both `--skip-recent-days >0` and `=0` branches receive the filter symmetrically. - `--address` single-mode bypasses the filter (correct: explicit override). - Backslash escaping in the Python literal `',\s*\d+'` produces correct POSIX regex. - `(:days || ' days')::interval` cast is pre-existing, not introduced here — no new psycopg v3 CAST trap. ### C. Performance - Sequential full scan was already happening for `SELECT DISTINCT`; an extra regex predicate on `address` is negligible. Cuts sweep wall-time 3.9h → 50min (4.7× speedup). Sound tradeoff. ### D. Conventions / Architecture - argparse `--flag` / `--no-flag` toggle pair via `dest=`/`action=store_true|store_false` with `default=True` is the standard Python idiom for default-on boolean flags. OK. - Operator-script CLI — no production code path touched. ### E. Backward-compat - Default flip IS a behavior change for re-runs, but PR title states `(default on)` and body explains the 583→123 cut. `--no-require-house-number` explicitly preserves old behavior for diagnostics. Acceptable for an operator tool. ### F. Tests - No unit test — acceptable for an operator script; filter is empirically verifiable by toggle + `--limit N` dry-run. ### G. Edge cases noted (not blocking) - Address with prefix between comma and digit (`, д. 5`) would not match — extremely uncommon in `listings.address` data (sample examples use bare `, 106`). - Space-delimited variants (`Куйбышева 106` without comma) would not match — but per the file docstring example data is comma-delimited. - Followup for the other 460 listings is correctly scoped out ("address normalization is upstream data-quality work" per PR body). ### Verdict: APPROVE — merging. Followup nit (non-blocking): trim the "OR end with a digit token" claim from the docstring or extend the regex to `,\s*\d+|\s\d+\s*$`.