lekss361/gendesign

Fork 0

fix(tradein-avito): strip Emotion CSS from listings.address #502

Merged

lekss361 merged 1 commit from feat/tradein-avito-clean-address-css into main

2026-05-24 10:52:51 +00:00

lekss361 commented

2026-05-24 10:29:12 +00:00

Owner

Problem

Avito scraper leaks <style>.css-XXX{...}</style> content into listings.address via recursive .text(strip=True). Production example (estimate a0a0b820-e8a8-4eee-aa73-0ab3b98ac233 analog #4):

улица Токарей, 56к1Площадь 1905 года.css-39hgr0{fill:currentColor;...

Fix

Add _clean_address() helper in avito.py — strips Emotion CSS rules + post-address noise (Площадь N года, от N мин.)
Apply at the extraction site in avito.py (SERP card address) and in avito_detail.py (address_full from [itemprop="address"])
Import helper in avito_detail.py from avito.py — no copy-paste
Backfill migration 062_clean_avito_addresses.sql for historical rows
Unit test for the regex behavior

Verify

# After deploy:
docker exec tradein-postgres psql -U tradein -d tradein -c "SELECT count(*) FROM listings WHERE source='avito' AND address ~ 'css-'"
# expected: 0

## Problem Avito scraper leaks `<style>.css-XXX{...}</style>` content into `listings.address` via recursive `.text(strip=True)`. Production example (estimate `a0a0b820-e8a8-4eee-aa73-0ab3b98ac233` analog #4): ``` улица Токарей, 56к1Площадь 1905 года.css-39hgr0{fill:currentColor;... ``` ## Fix - Add `_clean_address()` helper in `avito.py` — strips Emotion CSS rules + post-address noise (Площадь N года, от N мин.) - Apply at the extraction site in `avito.py` (SERP card address) and in `avito_detail.py` (`address_full` from `[itemprop="address"]`) - Import helper in `avito_detail.py` from `avito.py` — no copy-paste - Backfill migration `062_clean_avito_addresses.sql` for historical rows - Unit test for the regex behavior ## Verify ```bash # After deploy: docker exec tradein-postgres psql -U tradein -d tradein -c "SELECT count(*) FROM listings WHERE source='avito' AND address ~ 'css-'" # expected: 0 ```

lekss361 added 1 commit 2026-05-24 10:29:13 +00:00

fix(tradein-avito): strip Emotion CSS from listings.address 9e1c7d5147

Avito DOM leaks inline <style>.css-XXX{...}</style> inside the location <p>,
which gets pulled by .text(strip=True). Add _clean_address() helper that strips
CSS rules + post-address noise (Площадь N года, от N мин.), apply at extraction
site. Also applied to address_full in avito_detail.py via import from avito.py.
Backfill migration cleans the ~hundreds of historical rows.

Source: estimate a0a0b820-e8a8-4eee-aa73-0ab3b98ac233 analog #4.

lekss361 commented

2026-05-24 10:40:43 +00:00

Author

Owner

Deep Code Review — verdict

Summary

Status: APPROVE (with notes)
Files: 4 (P0:1 sql · P0:2 scrapers · P2:1 test)
Lines: +65 / -2 · PR #502 head 9e1c7d5
Scope: scope-appropriate single-purpose fix; matches vault plan Decision_TradeIn_DataQuality_8PR_Roadmap PR 1

✅ ready for human merge

Findings

No BLOCKERS. Two LOW notes worth a follow-up but not blocking.

Low-1 — false-positive on legit `Площадь N…` addresses

avito.py:271 regex \s*(Площадь \d|от \d+\s?мин\.|css-[a-z0-9_-]+) — token Площадь \d will truncate any real ЕКБ address that legitimately contains Площадь + digit (e.g. listing facing Площадь 1905 года, 5). In practice Avito puts metro/landmark hints as suffix to street, so impact is small, but the pattern is a heuristic, not a clean delimiter. Consider tightening (require css- to follow within N chars, or use a small allow-list of landmark names like Площадь 1905 года, Площадь Революции). Low priority — current incidence in listings is small.

Low-2 — backfill SQL slight divergence from Python helper

062_clean_avito_addresses.sql:

regexp_replace(address, '\.?css-…\{[^}]*\}', '', 'gi') matches Python _CSS_NOISE_RE ✓
regexp_replace(…, '\s*(Площадь \d|от \d+\s?мин\.|css-[a-z0-9_-]+).*$', '', 'i') mirrors _NOT_ADDRESS_TAIL_RE.split(...)[0] ✓
trim(both ' ,.' FROM …) is narrower than Python .strip(' ,. ') — newlines/tabs in DB rows will not be trimmed. Likely a no-op in practice (no in stored addresses), but for parity could add E' ' to the trim set.
WHERE filter address ~* 'css-[a-z0-9_-]+' (no leading \.?) is strictly broader than Python helper — fine for the cleanup pass.

Note — `avito_houses.py` correctly excluded

Vault roadmap mentioned avito_houses.py as a third candidate. Verified: that file pulls address from __preloadedState__ JSON via dd.get("address") / dd.get("fullAddress") (structured data, no .text() from HTML), so CSS noise cannot leak there. Excluding it is the right call.

Note — cross-module private import

avito_detail.py:35 imports _clean_address (leading underscore = module-private) from avito.py. Trade-off explicitly chosen to avoid copy-paste. Acceptable in scope; if a third caller emerges, promote to app/services/scrapers/_text_utils.py.

Correctness walkthrough

Dirty example from PR description traces correctly through helper:

_CSS_NOISE_RE.sub strips .css-39hgr0{fill:…box-sizing:border-box;} and trailing .css-39hgr0{...} (leaves :focus{…} variants — they don't match [a-z0-9_-]+\s*\{ because of the :). OK because step 2 catches them.
_NOT_ADDRESS_TAIL_RE.split cuts at first Площадь 1 → returns prefix "ул. Токарей, 56к1".
.strip(" ,. ") → "ул. Токарей, 56к1". Matches assertion in test_strips_css_noise. ✓

Security

No SQL injection vector: 062_*.sql is static migration with no parameters; all regex literals.
No regex DoS risk: bounded [^}]* inside \{…\} and bounded character classes; input is short address strings (<500 chars).
No new auth/token surface.

Idempotency

Migration BEGIN/COMMIT wraps single UPDATE — atomic. ✓
WHERE clause filters out rows that don't match noise patterns → re-run is no-op on clean rows. ✓
For rows where the regex over-strips on first run (low-1), subsequent runs won't re-process (no css- left), so over-strip is one-shot. Acceptable.

Performance

UPDATE listings WHERE source='avito' AND address ~* …: sequential scan over avito rows. Acceptable for one-shot backfill; not a hot path. No new index needed.
Helper is O(n) regex, called once per scraped listing. Negligible.

Test coverage

✓ Dirty case (canonical from incident)
✓ Passthrough clean
✓ None / empty / whitespace
✓ Trailing punctuation
Missing (nice-to-have, not blocking):
- String that becomes empty after stripping → asserts None
- Real address containing Площадь Революции (no digit) → not stripped
- Real "Площадь 1905 года, 5" → currently WOULD be stripped to "" then None — would document the limitation

Project conventions

psycopg v3 / CAST — N/A (no parameterized SQL in this PR).
Ruff line length, type hints (str | None), from __future__ import annotations already in module — compliant.
SQL file numbering: 062 follows 061_drop_legacy_cad_buildings.sql — correct.
BEGIN/COMMIT per .claude/rules/sql.md ✓.

Vault cross-check

Plan: decisions/Decision_TradeIn_DataQuality_8PR_Roadmap.md → PR 1 acceptance: "unit-test на грязной строке analog #4 → 'улица Токарей, 56к1'". Met. ✓
Post-deploy verification command in PR body matches plan's SELECT count(*) … address ~ '\.css-' = 0. ✓
Vault update should record actual deployed regex (not the draft from the plan) — main session task post-merge.

Recommended next steps

Human merge (blocked-scope policy: sql + scrapers).
Post-deploy: run verify SQL SELECT count(*) FROM listings WHERE source='avito' AND address ~ 'css-' → 0.
Re-fetch estimate a0a0b820-e8a8-4eee-aa73-0ab3b98ac233 analog #4 → confirm address is clean.
Vault: append observation to [[avito_scraper]] with deployed helper signature + migration 062 reference.

Complexity / blast radius

Risk: Low — isolated helper, well-tested, scope-bounded WHERE clause.
Reversibility: Code revert trivial; SQL backfill destructive but reversible from DB backup.
Merge window: Anytime; no coordination needed.

Bot review · sha 9e1c7d5 · automated deep-code-reviewer

## Deep Code Review — verdict ### Summary - **Status:** APPROVE (with notes) - **Files:** 4 (P0:1 sql · P0:2 scrapers · P2:1 test) - **Lines:** +65 / -2 · PR #502 head `9e1c7d5` - **Scope:** scope-appropriate single-purpose fix; matches vault plan `Decision_TradeIn_DataQuality_8PR_Roadmap` PR 1 ✅ ready for human merge ### Findings No BLOCKERS. Two LOW notes worth a follow-up but not blocking. #### Low-1 — false-positive on legit `Площадь N…` addresses `avito.py:271` regex `\s*(Площадь \d|от \d+\s?мин\.|css-[a-z0-9_-]+)` — token `Площадь \d` will truncate any real ЕКБ address that legitimately contains `Площадь` + digit (e.g. listing facing `Площадь 1905 года, 5`). In practice Avito puts metro/landmark hints as suffix to street, so impact is small, but the pattern is a heuristic, not a clean delimiter. Consider tightening (require `css-` to follow within N chars, or use a small allow-list of landmark names like `Площадь 1905 года`, `Площадь Революции`). Low priority — current incidence in `listings` is small. #### Low-2 — backfill SQL slight divergence from Python helper `062_clean_avito_addresses.sql`: - `regexp_replace(address, '\.?css-…\{[^}]*\}', '', 'gi')` matches Python `_CSS_NOISE_RE` ✓ - `regexp_replace(…, '\s*(Площадь \d|от \d+\s?мин\.|css-[a-z0-9_-]+).*$', '', 'i')` mirrors `_NOT_ADDRESS_TAIL_RE.split(...)[0]` ✓ - `trim(both ' ,.' FROM …)` is narrower than Python `.strip(' ,. ')` — newlines/tabs in DB rows will not be trimmed. Likely a no-op in practice (no ` ` in stored addresses), but for parity could add `E' '` to the trim set. - WHERE filter `address ~* 'css-[a-z0-9_-]+'` (no leading `\.?`) is strictly broader than Python helper — fine for the cleanup pass. #### Note — `avito_houses.py` correctly excluded Vault roadmap mentioned `avito_houses.py` as a third candidate. Verified: that file pulls address from `__preloadedState__` JSON via `dd.get("address")` / `dd.get("fullAddress")` (structured data, no `.text()` from HTML), so CSS noise cannot leak there. Excluding it is the right call. #### Note — cross-module private import `avito_detail.py:35` imports `_clean_address` (leading underscore = module-private) from `avito.py`. Trade-off explicitly chosen to avoid copy-paste. Acceptable in scope; if a third caller emerges, promote to `app/services/scrapers/_text_utils.py`. ### Correctness walkthrough Dirty example from PR description traces correctly through helper: 1. `_CSS_NOISE_RE.sub` strips `.css-39hgr0{fill:…box-sizing:border-box;}` and trailing `.css-39hgr0{...}` (leaves `:focus{…}` variants — they don't match `[a-z0-9_-]+\s*\{` because of the `:`). OK because step 2 catches them. 2. `_NOT_ADDRESS_TAIL_RE.split` cuts at first `Площадь 1` → returns prefix `"ул. Токарей, 56к1"`. 3. `.strip(" ,. ")` → `"ул. Токарей, 56к1"`. Matches assertion in `test_strips_css_noise`. ✓ ### Security - No SQL injection vector: `062_*.sql` is static migration with no parameters; all regex literals. - No regex DoS risk: bounded `[^}]*` inside `\{…\}` and bounded character classes; input is short address strings (<500 chars). - No new auth/token surface. ### Idempotency - Migration `BEGIN/COMMIT` wraps single `UPDATE` — atomic. ✓ - WHERE clause filters out rows that don't match noise patterns → re-run is no-op on clean rows. ✓ - For rows where the regex over-strips on first run (low-1), subsequent runs won't re-process (no `css-` left), so over-strip is one-shot. Acceptable. ### Performance - `UPDATE listings WHERE source='avito' AND address ~* …`: sequential scan over avito rows. Acceptable for one-shot backfill; not a hot path. No new index needed. - Helper is O(n) regex, called once per scraped listing. Negligible. ### Test coverage - ✓ Dirty case (canonical from incident) - ✓ Passthrough clean - ✓ None / empty / whitespace - ✓ Trailing punctuation - Missing (nice-to-have, not blocking): - String that becomes empty after stripping → asserts `None` - Real address containing `Площадь Революции` (no digit) → not stripped - Real `"Площадь 1905 года, 5"` → currently WOULD be stripped to `""` then `None` — would document the limitation ### Project conventions - psycopg v3 / CAST — N/A (no parameterized SQL in this PR). - Ruff line length, type hints (`str | None`), `from __future__ import annotations` already in module — compliant. - SQL file numbering: `062` follows `061_drop_legacy_cad_buildings.sql` — correct. - `BEGIN/COMMIT` per `.claude/rules/sql.md` ✓. ### Vault cross-check - Plan: `decisions/Decision_TradeIn_DataQuality_8PR_Roadmap.md` → PR 1 acceptance: "unit-test на грязной строке analog #4 → 'улица Токарей, 56к1'". Met. ✓ - Post-deploy verification command in PR body matches plan's `SELECT count(*) … address ~ '\.css-'` = 0. ✓ - Vault update should record actual deployed regex (not the draft from the plan) — main session task post-merge. ### Recommended next steps 1. Human merge (blocked-scope policy: sql + scrapers). 2. Post-deploy: run verify SQL `SELECT count(*) FROM listings WHERE source='avito' AND address ~ 'css-'` → 0. 3. Re-fetch estimate `a0a0b820-e8a8-4eee-aa73-0ab3b98ac233` analog #4 → confirm address is clean. 4. Vault: append observation to `[[avito_scraper]]` with deployed helper signature + migration `062` reference. ### Complexity / blast radius - **Risk:** Low — isolated helper, well-tested, scope-bounded WHERE clause. - **Reversibility:** Code revert trivial; SQL backfill destructive but reversible from DB backup. - **Merge window:** Anytime; no coordination needed. _Bot review · sha 9e1c7d5 · automated deep-code-reviewer_