lekss361/gendesign

Fork 0

feat(tradein): Phase 1.3 backfill scripts — fingerprints + house/listing_sources #475

Merged

lekss361 merged 1 commit from feat/tradein-matching-backfill into main

2026-05-23 14:29:40 +00:00

lekss361 commented

2026-05-23 14:21:59 +00:00

Owner

Motivation

Phase 1.3 of Multi-Source Integration. Manual operator scripts for legacy data migration into the matching link tables (house_sources, listing_sources) + populating houses.address_fingerprint.

NOT auto-applied by deploy.yml — these live in tradein-mvp/scripts/ (not tradein-mvp/backend/data/sql/NN_*.sql). Run explicitly on prod via uv run python + psql after this PR + PR #470 (matching module) are merged.

Files

Path	Lines	Type
`tradein-mvp/scripts/backfill_001_address_fingerprints.py`	86	Python (psycopg v3)
`tradein-mvp/scripts/backfill_002_house_sources.sql`	42	SQL (INSERT ... ON CONFLICT DO NOTHING)
`tradein-mvp/scripts/backfill_003_listing_sources.sql`	52	SQL (INSERT ... ON CONFLICT DO NOTHING)
`tradein-mvp/scripts/README.md`	27	Usage + ordering

Total: 4 files / +207 lines.

What each does

001 — Iterate houses WHERE address_fingerprint IS NULL, call normalize.address_fingerprint() from app.services.matching.normalize (PR #470), UPDATE. Idempotent (only NULL rows). Strips postgresql+psycopg:// dialect prefix per feedback from #469 fix.
002 — INSERT INTO house_sources from existing houses.(source, ext_house_id) pairs. confidence=1.0, matched_method='backfill', last_seen_at = h.last_scraped_at. ON CONFLICT (ext_source, ext_id) DO NOTHING.
003 — Same pattern for listing_sources from listings.(source, source_id). Carries price_rub, area_m2, floor, rooms for warm-start of listing_sources filter columns.
README — Order + exact commands.

Column-name adjustments vs spec

Worker verified column names against 002_core_tables.sql / 009_houses.sql:

Spec used	Actual	File
`houses.last_scrape`	`houses.last_scraped_at`	002
`listings.total_area`	`listings.area_m2`	003
`listings.rooms_count`	`listings.rooms`	003
`listings.scraped_at`	(kept — matches)	003

Idempotency

001: WHERE clause only touches NULL rows → re-running no-op for already-populated rows
002/003: ON CONFLICT (ext_source, ext_id) DO NOTHING — UNIQUE constraint from 028

Reviewer

deep-code-reviewer — please verify:

DSN strip postgresql+psycopg:// → postgresql:// matches docker-compose.prod.yml DATABASE_URL format
INSERT column lists match current schema (after 002 + 028/029 + 040 alterations)
last_scraped_at exists on houses (not last_scrape) — confirmed from 009_houses.sql:44
Python imports use the post-PR #470 path app.services.matching.normalize

Refs

Depends on PR #470 (matching module) — MERGED
Depends on PR #464 (matching schema delta) — MERGED
Master Plan Phase 1.3 spec

## Motivation Phase 1.3 of Multi-Source Integration. Manual operator scripts for legacy data migration into the matching link tables (`house_sources`, `listing_sources`) + populating `houses.address_fingerprint`. **NOT auto-applied** by `deploy.yml` — these live in `tradein-mvp/scripts/` (not `tradein-mvp/backend/data/sql/NN_*.sql`). Run explicitly on prod via `uv run python` + `psql` after this PR + PR #470 (matching module) are merged. ## Files | Path | Lines | Type | |---|---|---| | `tradein-mvp/scripts/backfill_001_address_fingerprints.py` | 86 | Python (psycopg v3) | | `tradein-mvp/scripts/backfill_002_house_sources.sql` | 42 | SQL (INSERT ... ON CONFLICT DO NOTHING) | | `tradein-mvp/scripts/backfill_003_listing_sources.sql` | 52 | SQL (INSERT ... ON CONFLICT DO NOTHING) | | `tradein-mvp/scripts/README.md` | 27 | Usage + ordering | Total: 4 files / +207 lines. ## What each does - **001** — Iterate `houses WHERE address_fingerprint IS NULL`, call `normalize.address_fingerprint()` from `app.services.matching.normalize` (PR #470), UPDATE. Idempotent (only NULL rows). Strips `postgresql+psycopg://` dialect prefix per `feedback` from #469 fix. - **002** — `INSERT INTO house_sources` from existing `houses.(source, ext_house_id)` pairs. `confidence=1.0`, `matched_method='backfill'`, `last_seen_at = h.last_scraped_at`. ON CONFLICT (ext_source, ext_id) DO NOTHING. - **003** — Same pattern for `listing_sources` from `listings.(source, source_id)`. Carries `price_rub`, `area_m2`, `floor`, `rooms` for warm-start of `listing_sources` filter columns. - **README** — Order + exact commands. ## Column-name adjustments vs spec Worker verified column names against `002_core_tables.sql` / `009_houses.sql`: | Spec used | Actual | File | |---|---|---| | `houses.last_scrape` | `houses.last_scraped_at` | 002 | | `listings.total_area` | `listings.area_m2` | 003 | | `listings.rooms_count` | `listings.rooms` | 003 | | `listings.scraped_at` | (kept — matches) | 003 | ## Idempotency - 001: WHERE clause only touches NULL rows → re-running no-op for already-populated rows - 002/003: ON CONFLICT (ext_source, ext_id) DO NOTHING — UNIQUE constraint from 028 ## Reviewer **deep-code-reviewer** — please verify: 1. DSN strip `postgresql+psycopg://` → `postgresql://` matches docker-compose.prod.yml DATABASE_URL format 2. INSERT column lists match current schema (after 002 + 028/029 + 040 alterations) 3. `last_scraped_at` exists on houses (not `last_scrape`) — confirmed from 009_houses.sql:44 4. Python imports use the post-PR #470 path `app.services.matching.normalize` ## Refs - Depends on PR #470 (matching module) — MERGED - Depends on PR #464 (matching schema delta) — MERGED - Master Plan Phase 1.3 spec

lekss361 commented

2026-05-23 14:23:50 +00:00

Author

Owner

deep-code-reviewer — CLOSE recommendation (empty PR)

Verified via Forgejo API — this PR has no content:

Check	Result
`additions`	0
`deletions`	0
`changed_files`	0
`list_pr_files`	`[]`
`list_pr_commits`	`[]`
`get_pr_diff`	empty body
`merge_base`	`b26e0ff7...` == `head.sha` (old main SHA before #473/#474)

The branch feat/tradein-matching-backfill points at the same commit as main was at PR creation time. No commits ahead of base. Merging would create a no-op commit (or fast-forward to itself).

Root cause (likely)

Same pattern as #411/#458/#463 — one of:

Worker created branch from main but never git add / git commit the backfill files (worktree pollution lost them)
git push targeted wrong branch / wrong remote
Commit landed in a different worktree that wasn't pushed

Required action

Do NOT merge — would inject no-op commit into main history.

Pick one:

(A) Push real commits — worker re-runs Phase 1.3 work, git add tradein-mvp/scripts/{backfill_001_address_fingerprints.py,backfill_002_house_sources.sql,backfill_003_listing_sources.sql,README.md} → commit → git push forgejo feat/tradein-matching-backfill. PR will auto-update with content.
(B) Close as duplicate / WIP-lost — if files exist in another branch/worktree, close this PR and open a fresh one from that source.

Until real commits land, this PR cannot be reviewed (nothing to review) and should not be merged.

Refs: feedback feedback_worker_isolation_worktree.md (worker isolation), prior empty-PR incidents #411 / #458 / #463.

## deep-code-reviewer — CLOSE recommendation (empty PR) Verified via Forgejo API — this PR has no content: | Check | Result | |---|---| | `additions` | 0 | | `deletions` | 0 | | `changed_files` | 0 | | `list_pr_files` | `[]` | | `list_pr_commits` | `[]` | | `get_pr_diff` | empty body | | `merge_base` | `b26e0ff7...` == `head.sha` (old main SHA before #473/#474) | The branch `feat/tradein-matching-backfill` points at the same commit as `main` was at PR creation time. No commits ahead of base. Merging would create a no-op commit (or fast-forward to itself). ### Root cause (likely) Same pattern as #411/#458/#463 — one of: 1. Worker created branch from main but never `git add` / `git commit` the backfill files (worktree pollution lost them) 2. `git push` targeted wrong branch / wrong remote 3. Commit landed in a different worktree that wasn't pushed ### Required action **Do NOT merge** — would inject no-op commit into main history. Pick one: - **(A) Push real commits** — worker re-runs Phase 1.3 work, `git add tradein-mvp/scripts/{backfill_001_address_fingerprints.py,backfill_002_house_sources.sql,backfill_003_listing_sources.sql,README.md}` → commit → `git push forgejo feat/tradein-matching-backfill`. PR will auto-update with content. - **(B) Close as duplicate / WIP-lost** — if files exist in another branch/worktree, close this PR and open a fresh one from that source. Until real commits land, this PR cannot be reviewed (nothing to review) and should not be merged. Refs: feedback `feedback_worker_isolation_worktree.md` (worker isolation), prior empty-PR incidents #411 / #458 / #463.

lekss361 added 1 commit 2026-05-23 14:25:02 +00:00

feat(tradein): Phase 1.3 backfill scripts -- fingerprints + house/listing_sources b32628e262

Multi-Source Integration Phase 1.3. Manual operator scripts for legacy data
migration. NOT auto-applied -- run explicitly via uv/psql.

scripts/:
  - backfill_001_address_fingerprints.py -- Python, populates houses.address_fingerprint
    for rows where NULL (uses matching/normalize.address_fingerprint from PR #470)
  - backfill_002_house_sources.sql -- INSERT INTO house_sources FROM houses,
    ON CONFLICT (ext_source, ext_id) DO NOTHING (idempotent)
  - backfill_003_listing_sources.sql -- INSERT INTO listing_sources FROM listings,
    same pattern
  - README.md -- order + usage

All idempotent -- re-running safe. confidence=1.0, matched_method='backfill'.

lekss361 commented

2026-05-23 14:26:59 +00:00

Author

Owner

Fix-up applied 14:24 UTC+3 — branch advanced b26e0ff..b32628e (fast-forward push). PR now contains 4 files / +207 lines / -0 (additions: 207, changed_files: 4).

Worker's commit b32628e was dangling (created in worker's session but the branch pointer was never moved before push completion — see CLOSE-rec root-cause #1). Recovered via:

git branch -f feat/tradein-matching-backfill b32628e
git push forgejo feat/tradein-matching-backfill  # fast-forward

Re-review please. Same content as originally intended in PR description (backfill_001 Python + 002/003 SQL + README).

Fix-up applied 14:24 UTC+3 — branch advanced `b26e0ff..b32628e` (fast-forward push). PR now contains 4 files / +207 lines / -0 (`additions: 207`, `changed_files: 4`). Worker's commit `b32628e` was dangling (created in worker's session but the branch pointer was never moved before push completion — see CLOSE-rec root-cause #1). Recovered via: ``` git branch -f feat/tradein-matching-backfill b32628e git push forgejo feat/tradein-matching-backfill # fast-forward ``` Re-review please. Same content as originally intended in PR description (backfill_001 Python + 002/003 SQL + README).

lekss361 merged commit c0a0bfa413 into main

2026-05-23 14:29:40 +00:00

lekss361 referenced this pull request from a commit

2026-05-23 14:29:40 +00:00

feat(tradein): Phase 1.3 backfill scripts — fingerprints + house/listing_sources (#475)

lekss361 commented

2026-05-23 14:30:05 +00:00

Author

Owner

Merged via deep-code-reviewer — verdict APPROVE (MEDIUM, non-blocking).

Schema cross-check: all column names verified against 028+029 (house_sources/listing_sources) and 002_core+009_houses (source tables). Worker-flagged renames (last_scrape→last_scraped_at, total_area→area_m2, rooms→rooms_count) all correct.

DSN strip: postgresql+psycopg:// → postgresql:// with maxcount=1 — handles SQLAlchemy dialect prefix correctly.

psycopg v3 compliance: import psycopg (not psycopg2), %s param style, with conn.cursor() mgmt — clean.

Empty-PR recovery: head b32628e2 contains all 4 promised files (207/0 — matches PR description). 4th in series (#411/#458/#463) — recommend vault entry fixes/Bug_Empty_PR_Worker_Push_Race.md post-merge.

Non-blocking findings (MEDIUM, fix in follow-up PR if scope permits)

backfill_001:53,65 — dead None-branches: normalize_address() returns str always ("" on empty, never None). Same for address_fingerprint(). Replace is None with not normalized if treating "" as skip is desired; or drop branches entirely since SELECT already filters address IS NOT NULL.
backfill_001:46 — cur.fetchall() unbounded. At current data volume (~K rows) OK. At 100K+ would be ~30 MB Python list. Future-proof with conn.cursor(name="bf001") server-side cursor.
backfill_003:36 — writes last_scraped_at only. Per 029:60 guidance "New code should write to last_seen_at". Add last_seen_at = l.scraped_at to dual-write (both columns kept for compat).

Operator can proceed with the documented manual run sequence. Scripts safe — DDL idempotent, ON CONFLICT DO NOTHING.

Merged via deep-code-reviewer — verdict APPROVE (MEDIUM, non-blocking). **Schema cross-check**: all column names verified against 028+029 (house_sources/listing_sources) and 002_core+009_houses (source tables). Worker-flagged renames (last_scrape→last_scraped_at, total_area→area_m2, rooms→rooms_count) all correct. **DSN strip**: `postgresql+psycopg://` → `postgresql://` with maxcount=1 — handles SQLAlchemy dialect prefix correctly. **psycopg v3 compliance**: import psycopg (not psycopg2), `%s` param style, `with conn.cursor()` mgmt — clean. **Empty-PR recovery**: head b32628e2 contains all 4 promised files (207/0 — matches PR description). 4th in series (#411/#458/#463) — recommend vault entry `fixes/Bug_Empty_PR_Worker_Push_Race.md` post-merge. ### Non-blocking findings (MEDIUM, fix in follow-up PR if scope permits) 1. **backfill_001:53,65** — dead None-branches: `normalize_address()` returns `str` always (`""` on empty, never None). Same for `address_fingerprint()`. Replace `is None` with `not normalized` if treating `""` as skip is desired; or drop branches entirely since SELECT already filters `address IS NOT NULL`. 2. **backfill_001:46** — `cur.fetchall()` unbounded. At current data volume (~K rows) OK. At 100K+ would be ~30 MB Python list. Future-proof with `conn.cursor(name="bf001")` server-side cursor. 3. **backfill_003:36** — writes `last_scraped_at` only. Per 029:60 guidance "New code should write to last_seen_at". Add `last_seen_at = l.scraped_at` to dual-write (both columns kept for compat). Operator can proceed with the documented manual run sequence. Scripts safe — DDL idempotent, ON CONFLICT DO NOTHING.

lekss361 referenced this pull request

2026-05-23 14:39:45 +00:00

feat(tradein): Phase 3.2 — /api/v1/search endpoint + Redis cache #479