lekss361/gendesign

Fork 0

feat(tradein-estimator): cohort filter + Tier 0 for analog selection quality #519

Merged

lekss361 merged 1 commit from feat/tradein-estimator-cohort-filter into main

2026-05-24 13:04:59 +00:00

lekss361 commented

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

Owner

Summary

Production audit (estimate d4ec4610-92a7-4985-b857-1dc6753435fc, 2026-05-24) показал смешивание разных эпох застройки в выборке аналогов: для target Малышева 84 (1978, кирпич, 9-эт) в top-10 попали 31-этажные новостройки и центральные сталинки. relevance_score даёт слишком слабый вес году постройки (year_mismatch/12) — distance-в-км перевешивает. Median расплывается, CV доходит до 32%.

Fix — когортный hard-filter с graceful fallback

Добавлен Tier 0 в начало каскада fallback:

Tier	Условие	Применяется
0 (NEW)	1км + ±15% area + cohort match	если `target_year` задан
A	1км + ±15% area (без cohort)	если Tier 0 даёт <5
B	2км + ±15% area	если Tier A даёт <5
C	2км + ±25% area	если Tier B даёт <3

5 когорт массовой застройки РФ:

Cohort	Years
`khrushchev`	1955–1969
`brezhnev`	1970–1989
`late_soviet`	1985–1999
`2000s`	2000–2010
`modern`	2011+

_target_cohort_range(year_built) возвращает (year_min, year_max) или None. None → Tier 0 skipped (graceful — поведение как до этого PR).

SQL filter в _fetch_analogs

AND (
    CAST(:cohort_year_min AS integer) IS NULL
    OR year_built IS NULL                              -- gentle: don't drop listings with unknown year
    OR year_built BETWEEN CAST(:cohort_year_min AS integer)
                      AND CAST(:cohort_year_max AS integer)
)

CAST(:x AS integer) обязателен — psycopg3 prepared statement не выводит тип $N в IS NULL predicate (известный trap, см. PR #518).
year_built IS NULL пропускается — не штрафуем листинги с неполными данными (типично для Avito anonymous).

What didn't change

relevance_score weights в SQL — НЕ трогали (это была Опция 1, отдельный потенциальный PR).
DB schema, API контракт, frontend — нетронуты.
Tier B / Tier C — БЕЗ cohort (когда уже расширяем радиус, хотим max данных).
Existing tests — не сломаны (cohort_year_min/max defaults None → fully backward-compatible).

Diff

1 файл, +83/-9. Тесты pytest local не прогнал в worker'е (venv проблема в worktree, pre-existing), AST parse OK.

Test plan

Деплой → submit Малышева 84 (1978) повторно → analogs не должны содержать 31-этажные новостройки.
Submit без year_built → ведёт себя как старая логика (Tier A first).
Submit с year_built=2020 → cohort modern (2011-2100) — должны попадать только новостройки.
Submit с year_built=1900 (out-of-range) → cohort skipped, Tier A first (старое поведение).
EXPLAIN ANALYZE на запросе с cohort — план должен использовать index по year_built если есть, иначе seq scan within ST_DWithin filtered set (приемлемо при ≤300 кандидатов).

## Summary Production audit (estimate `d4ec4610-92a7-4985-b857-1dc6753435fc`, 2026-05-24) показал смешивание разных эпох застройки в выборке аналогов: для target Малышева 84 (1978, кирпич, 9-эт) в top-10 попали 31-этажные новостройки и центральные сталинки. `relevance_score` даёт слишком слабый вес году постройки (`year_mismatch/12`) — distance-в-км перевешивает. Median расплывается, CV доходит до 32%. ## Fix — когортный hard-filter с graceful fallback Добавлен **Tier 0** в начало каскада fallback: | Tier | Условие | Применяется | |---|---|---| | **0 (NEW)** | 1км + ±15% area + cohort match | если `target_year` задан | | A | 1км + ±15% area (без cohort) | если Tier 0 даёт <5 | | B | 2км + ±15% area | если Tier A даёт <5 | | C | 2км + ±25% area | если Tier B даёт <3 | 5 когорт массовой застройки РФ: | Cohort | Years | |---|---| | `khrushchev` | 1955–1969 | | `brezhnev` | 1970–1989 | | `late_soviet` | 1985–1999 | | `2000s` | 2000–2010 | | `modern` | 2011+ | `_target_cohort_range(year_built)` возвращает `(year_min, year_max)` или `None`. None → Tier 0 skipped (graceful — поведение как до этого PR). ## SQL filter в _fetch_analogs ```sql AND ( CAST(:cohort_year_min AS integer) IS NULL OR year_built IS NULL -- gentle: don't drop listings with unknown year OR year_built BETWEEN CAST(:cohort_year_min AS integer) AND CAST(:cohort_year_max AS integer) ) ``` - `CAST(:x AS integer)` обязателен — psycopg3 prepared statement не выводит тип $N в `IS NULL` predicate (известный trap, см. PR #518). - `year_built IS NULL` пропускается — не штрафуем листинги с неполными данными (типично для Avito anonymous). ## What didn't change - `relevance_score` weights в SQL — НЕ трогали (это была Опция 1, отдельный потенциальный PR). - DB schema, API контракт, frontend — нетронуты. - Tier B / Tier C — БЕЗ cohort (когда уже расширяем радиус, хотим max данных). - Existing tests — не сломаны (cohort_year_min/max defaults None → fully backward-compatible). ## Diff 1 файл, +83/-9. Тесты pytest local не прогнал в worker'е (venv проблема в worktree, pre-existing), AST parse OK. ## Test plan - [ ] Деплой → submit Малышева 84 (1978) повторно → analogs не должны содержать 31-этажные новостройки. - [ ] Submit без year_built → ведёт себя как старая логика (Tier A first). - [ ] Submit с year_built=2020 → cohort `modern` (2011-2100) — должны попадать только новостройки. - [ ] Submit с year_built=1900 (out-of-range) → cohort skipped, Tier A first (старое поведение). - [ ] EXPLAIN ANALYZE на запросе с cohort — план должен использовать index по year_built если есть, иначе seq scan within ST_DWithin filtered set (приемлемо при ≤300 кандидатов).

lekss361 added 1 commit 2026-05-24 12:53:16 +00:00

feat(tradein-estimator): cohort filter + Tier 0 for analog selection quality 1d4d58538d

Production audit (2026-05-24, estimate d4ec4610) showed analogs mixing
new high-rises (31-этаж, ~2015+) with target 1978 Khrushchev/Brezhnev-era
9-floor building — relevance_score gave too little weight to year mismatch.
Median price drifted; CV at 32%.

Add cohort hard-filter as new Tier 0 in fallback cascade:
  Tier 0 (NEW): 1km + ±15% area + year cohort match
  Tier A:       1km + ±15% area  (no cohort — graceful drop if Tier 0 < 5)
  Tier B:       2km + ±15% area
  Tier C:       2km + ±25% area

5 cohorts (khrushchev 1955-69, brezhnev 1970-89, late_soviet 1985-99,
2000s 2000-10, modern 2011+). target_year=None or out-of-range -> cohort
skipped, Tier 0 not attempted.

Listings with year_built IS NULL pass through cohort filter (gentle --
don't penalize scrapers with incomplete data).

No schema/API changes. relevance_score weights untouched (separate concern).

lekss361 commented

2026-05-24 13:03:34 +00:00

Author

Owner

Deep Code Review — verdict

Summary

Status: APPROVE (with notes)
Files: 1 (P0: estimator.py) · +83/-9 · 1 SQL block touched
PR #519 · head 1d4d585 · base main

Strengths

CAST preserved correctly in IS NULL predicate (CAST(:cohort_year_min AS integer) IS NULL) — matches PR #518 fix and explicit comment cites it. No psycopg v3 AmbiguousParameter trap.
Graceful fallback when target_year is None or out-of-range (1900/2050) → cohort_range=None → Tier 0 skipped → behaviour identical to pre-PR. Fully backward-compatible default (cohort_year_min/max=None).
NULL year_built tolerated in WHERE clause (OR year_built IS NULL) — avoids dropping Avito anonymous-address listings with incomplete metadata. Correct trade-off.
5 cohort definitions match РФ массовой застройки typology; first-match semantics documented.
API contract / DB schema / frontend / Tier B-C logic — untouched. Blast radius is correctly minimised.

MEDIUM (non-blocking, worth follow-up)

1. Cohort filter only applies on Tier W path of _fetch_analogs call.

When cohort_range is set and _fetch_analogs(...cohort_year_min=...) runs, the SQL clause lives only inside the Tier W SELECT (line 1285-1290 of head). Tier S (line 1097, address ILIKE prefix) and Tier H (line 1175-1180, year_built BETWEEN target-15 AND target+15) execute BEFORE the cohort filter ever evaluates — they have early-return at return _stratify_candidates(...), ..., "S"|"H" (lines 1122, 1214).

Concrete leak: target_year=1978 (brezhnev cohort = 1970–1989). Tier H allows year_built ∈ [1963, 1993]. A 1965 khrushchev or 1992 late_soviet building within 1km, ≥5 same-rooms+area listings → Tier H wins → cohort filter is bypassed → analog_tier='H' returned. PR description claims "Tier 0: 1km + ±15% area + cohort match" but in reality cohort match is enforced only on Tier W.

For the target audit case (Малышева 84, 1978, 9-эт brezhnev) impact is small — year±15 = 1963-1993 leaks ≤7 years across cohort boundaries vs current ±50-year noise from 31-эт новостроек. Still an improvement. But the PR over-states intent.

Suggested follow-up (separate PR): pass cohort to Tier H too:

# inside Tier H WHERE
AND year_built BETWEEN CAST(:year_min AS integer) AND CAST(:year_max AS integer)
AND (
    CAST(:cohort_year_min AS integer) IS NULL
    OR year_built BETWEEN CAST(:cohort_year_min AS integer)
                      AND CAST(:cohort_year_max AS integer)
)

2. Double-execute on Tier 0 fallback.

When len(listings_tier0) < MIN_ANALOGS_TIER_0 (5), Tier A re-calls _fetch_analogs without cohort. Inside, Tier S/H run identically to the first call (cohort filter doesn't touch them). So Tier S/H SQL roundtrips fire 2× whenever Tier 0 falls back. ~2 extra DB queries per estimate on the fallback path. Sub-second, but unnecessary. Optimisation deferred — out of scope here.

3. No tier-0 observability in API response.

The analog_tier returned from Tier 0 call is whatever internal tier resolved (S/H/W). tier_note (line 634-643) doesn't distinguish "matched via cohort" from "matched without cohort". User-facing explanation won't reflect whether cohort filter actually ran. If you want to A/B verify the audit fix (Малышева 84) — there's no signal in the response. Consider emitting cohort_applied: bool in confidence_explanation or adding a structured field. Out of scope here.

MINOR

4. No tests added. PR body acknowledges venv issue in worker. _target_cohort_range is the cleanest place to add a 6-case parametrized unit test (None / 1900 / 1969 / 1970 / 1988 → brezhnev / 1990 → late_soviet / 2050 → None). Smoke test plan in PR body is reasonable. Not a blocker because logic is small and graceful-fallback design contains blast radius.

5. late_soviet cohort effectively redundant (1985-1999 overlaps brezhnev 1970-1989). First-match picks brezhnev for 1985-1989 → late_soviet covers only 1990-1999. Comment says "overlap intentional" — confirm semantics. TODO worth: rename to early_90s or shift to late_soviet=1990–1999 to remove confusion.

6. Vault decision not extended. Decision_TradeIn_DataQuality_8PR_Roadmap ends at PR 8. This is effectively a PR 9 added from a second audit (UUID d4ec4610). Main session should append PR 9 entry to the decision doc post-merge. Out of scope for this PR.

Cross-file impact

Single-file change. Caller estimate_quality (line 528) is the only entry point invoking the new cohort_range orchestration.
API endpoint tradein-mvp/backend/app/api/v1/trade_in.py → calls estimate_quality → no schema change in AggregatedEstimate.
Frontend untouched.
Tier S/H cohort blind-spot above (item 1) is logical, not cross-file.

Vault cross-check

Decision_TradeIn_DataQuality_8PR_Roadmap — original PR 3 was "house-match tiered S→H→W" (already merged in PR #507). This PR #519 builds on that as a new tier (Tier 0 = cohort hard-filter wrapping Tier W). Direction consistent with audit intent (year mismatch dominates relevance score).
cast-in-is-not-null-predicates memory rule — respected (CAST present, comment cites PR #518).

Pre-flight

Branch base: main · mergeable: true · 1 commit
No --no-verify / --force / --amend indicators
Diff is 1 file, 1 worker, no scope creep

Risk / blast radius

Risk: low. Pure additive logic with default-None graceful no-op. Existing behaviour unchanged when target_year not provided or out-of-cohort-range. Tier A fallback preserves current results.
Reversibility: easy revert (single file).
Merge window: any time.

Verdict

Approve and merge. Notes (1) and (3) above are worth tracking as follow-up tasks but don't block this PR — the primary audit issue (Малышева 84 returning 31-эт novostroyki) gets meaningfully addressed by the Tier W cohort hard-filter, and the architecture leaves room for tightening Tier H later.

## Deep Code Review — verdict ### Summary - Status: APPROVE (with notes) - Files: 1 (P0: estimator.py) · +83/-9 · 1 SQL block touched - PR #519 · head 1d4d585 · base main ### Strengths - **CAST preserved correctly** in `IS NULL` predicate (`CAST(:cohort_year_min AS integer) IS NULL`) — matches PR #518 fix and explicit comment cites it. No psycopg v3 AmbiguousParameter trap. - **Graceful fallback** when `target_year is None` or out-of-range (1900/2050) → `cohort_range=None` → Tier 0 skipped → behaviour identical to pre-PR. Fully backward-compatible default (`cohort_year_min/max=None`). - **NULL year_built tolerated** in WHERE clause (`OR year_built IS NULL`) — avoids dropping Avito anonymous-address listings with incomplete metadata. Correct trade-off. - 5 cohort definitions match РФ массовой застройки typology; first-match semantics documented. - API contract / DB schema / frontend / Tier B-C logic — untouched. Blast radius is correctly minimised. ### MEDIUM (non-blocking, worth follow-up) **1. Cohort filter only applies on Tier W path of `_fetch_analogs` call.** When `cohort_range` is set and `_fetch_analogs(...cohort_year_min=...)` runs, the SQL clause lives only inside the Tier W SELECT (line 1285-1290 of head). Tier S (line 1097, `address ILIKE prefix`) and Tier H (line 1175-1180, `year_built BETWEEN target-15 AND target+15`) execute BEFORE the cohort filter ever evaluates — they have early-return at `return _stratify_candidates(...), ..., "S"|"H"` (lines 1122, 1214). Concrete leak: `target_year=1978` (brezhnev cohort = 1970–1989). Tier H allows `year_built ∈ [1963, 1993]`. A 1965 khrushchev or 1992 late_soviet building within 1km, ≥5 same-rooms+area listings → Tier H wins → cohort filter is bypassed → `analog_tier='H'` returned. PR description claims "Tier 0: 1km + ±15% area + cohort match" but in reality cohort match is enforced only on Tier W. For the target audit case (Малышева 84, 1978, 9-эт brezhnev) impact is small — year±15 = 1963-1993 leaks ≤7 years across cohort boundaries vs current ±50-year noise from 31-эт новостроек. Still an improvement. But the PR over-states intent. **Suggested follow-up** (separate PR): pass cohort to Tier H too: ```python # inside Tier H WHERE AND year_built BETWEEN CAST(:year_min AS integer) AND CAST(:year_max AS integer) AND ( CAST(:cohort_year_min AS integer) IS NULL OR year_built BETWEEN CAST(:cohort_year_min AS integer) AND CAST(:cohort_year_max AS integer) ) ``` **2. Double-execute on Tier 0 fallback.** When `len(listings_tier0) < MIN_ANALOGS_TIER_0` (5), Tier A re-calls `_fetch_analogs` without cohort. Inside, Tier S/H run identically to the first call (cohort filter doesn't touch them). So Tier S/H SQL roundtrips fire 2× whenever Tier 0 falls back. ~2 extra DB queries per estimate on the fallback path. Sub-second, but unnecessary. Optimisation deferred — out of scope here. **3. No tier-0 observability in API response.** The `analog_tier` returned from Tier 0 call is whatever internal tier resolved (S/H/W). `tier_note` (line 634-643) doesn't distinguish "matched via cohort" from "matched without cohort". User-facing `explanation` won't reflect whether cohort filter actually ran. If you want to A/B verify the audit fix (Малышева 84) — there's no signal in the response. Consider emitting `cohort_applied: bool` in `confidence_explanation` or adding a structured field. Out of scope here. ### MINOR **4. No tests added.** PR body acknowledges venv issue in worker. `_target_cohort_range` is the cleanest place to add a 6-case parametrized unit test (None / 1900 / 1969 / 1970 / 1988 → brezhnev / 1990 → late_soviet / 2050 → None). Smoke test plan in PR body is reasonable. Not a blocker because logic is small and graceful-fallback design contains blast radius. **5. `late_soviet` cohort effectively redundant** (1985-1999 overlaps brezhnev 1970-1989). First-match picks brezhnev for 1985-1989 → late_soviet covers only 1990-1999. Comment says "overlap intentional" — confirm semantics. TODO worth: rename to `early_90s` or shift to `late_soviet=1990–1999` to remove confusion. **6. Vault decision not extended.** `Decision_TradeIn_DataQuality_8PR_Roadmap` ends at PR 8. This is effectively a PR 9 added from a second audit (UUID d4ec4610). Main session should append PR 9 entry to the decision doc post-merge. Out of scope for this PR. ### Cross-file impact - Single-file change. Caller `estimate_quality` (line 528) is the only entry point invoking the new `cohort_range` orchestration. - API endpoint `tradein-mvp/backend/app/api/v1/trade_in.py` → calls `estimate_quality` → no schema change in `AggregatedEstimate`. - Frontend untouched. - Tier S/H cohort blind-spot above (item 1) is logical, not cross-file. ### Vault cross-check - [[Decision_TradeIn_DataQuality_8PR_Roadmap]] — original PR 3 was "house-match tiered S→H→W" (already merged in PR #507). This PR #519 builds on that as a new tier (Tier 0 = cohort hard-filter wrapping Tier W). Direction consistent with audit intent (year mismatch dominates relevance score). - [[cast-in-is-not-null-predicates]] memory rule — respected (CAST present, comment cites PR #518). ### Pre-flight - Branch base: main · mergeable: true · 1 commit - No `--no-verify` / `--force` / `--amend` indicators - Diff is 1 file, 1 worker, no scope creep ### Risk / blast radius - Risk: **low**. Pure additive logic with default-None graceful no-op. Existing behaviour unchanged when `target_year` not provided or out-of-cohort-range. Tier A fallback preserves current results. - Reversibility: easy revert (single file). - Merge window: any time. ### Verdict Approve and merge. Notes (1) and (3) above are worth tracking as follow-up tasks but don't block this PR — the primary audit issue (Малышева 84 returning 31-эт novostroyki) gets meaningfully addressed by the Tier W cohort hard-filter, and the architecture leaves room for tightening Tier H later.

lekss361 commented

2026-05-24 13:04:49 +00:00

Author

Owner

Deep Code Review — verdict

Summary

Status: APPROVE (with notes)
Files: 1 (P0: estimator.py) · +83/-9 · 1 SQL block touched
PR #519 · head 1d4d585 · base main

Strengths

CAST preserved correctly in IS NULL predicate (CAST(:cohort_year_min AS integer) IS NULL) — matches PR #518 fix and explicit comment cites it. No psycopg v3 AmbiguousParameter trap.
Graceful fallback when target_year is None or out-of-range (1900/2050) → cohort_range=None → Tier 0 skipped → behaviour identical to pre-PR. Fully backward-compatible default (cohort_year_min/max=None).
NULL year_built tolerated in WHERE clause (OR year_built IS NULL) — avoids dropping Avito anonymous-address listings with incomplete metadata. Correct trade-off.
5 cohort definitions match РФ массовой застройки typology; first-match semantics documented.
API contract / DB schema / frontend / Tier B-C logic — untouched. Blast radius is correctly minimised.

MEDIUM (non-blocking, worth follow-up)

1. Cohort filter only applies on Tier W path of _fetch_analogs call.

When cohort_range is set and _fetch_analogs(...cohort_year_min=...) runs, the SQL clause lives only inside the Tier W SELECT (head line 1285-1290). Tier S (line 1097, address ILIKE prefix) and Tier H (line 1175-1180, year_built BETWEEN target-15 AND target+15) execute BEFORE the cohort filter ever evaluates — they have early-return at lines 1122 / 1214.

Concrete leak: target_year=1978 (brezhnev cohort = 1970–1989). Tier H allows year_built ∈ [1963, 1993]. A 1965 khrushchev or 1992 late_soviet building within 1km with ≥5 same-rooms+area listings → Tier H wins → cohort filter is bypassed → analog_tier='H' returned. PR description claims Tier 0: 1km + ±15% area + cohort match but in reality cohort match is enforced only on Tier W.

For the target audit case (Малышева 84, 1978, 9-эт brezhnev) impact is small — year±15 = 1963-1993 leaks ≤7 years across cohort boundaries vs current ±50-year noise from 31-эт novostroyki. Still an improvement. But the PR over-states intent.

Suggested follow-up (separate PR): pass cohort to Tier H too:

# inside Tier H WHERE
AND year_built BETWEEN CAST(:year_min AS integer) AND CAST(:year_max AS integer)
AND (
    CAST(:cohort_year_min AS integer) IS NULL
    OR year_built BETWEEN CAST(:cohort_year_min AS integer)
                      AND CAST(:cohort_year_max AS integer)
)

2. Double-execute on Tier 0 fallback.

3. No tier-0 observability in API response.

The analog_tier returned from Tier 0 call is whatever internal tier resolved (S/H/W). tier_note (line 634-643) doesn't distinguish matched via cohort from matched without cohort. User-facing explanation won't reflect whether cohort filter actually ran. If you want to A/B verify the audit fix (Малышева 84) — there's no signal in the response. Consider emitting cohort_applied: bool in confidence_explanation or adding a structured field. Out of scope here.

MINOR

5. late_soviet cohort effectively redundant (1985-1999 overlaps brezhnev 1970-1989). First-match picks brezhnev for 1985-1989 → late_soviet covers only 1990-1999. Comment says overlap intentional — confirm semantics. TODO worth: rename to early_90s or shift to late_soviet=1990–1999 to remove confusion.

Cross-file impact

Single-file change. Caller estimate_quality (line 528) is the only entry point invoking the new cohort_range orchestration.
API endpoint tradein-mvp/backend/app/api/v1/trade_in.py → calls estimate_quality → no schema change in AggregatedEstimate.
Frontend untouched.
Tier S/H cohort blind-spot (item 1) is logical, not cross-file.

Vault cross-check

Decision_TradeIn_DataQuality_8PR_Roadmap — original PR 3 was house-match tiered S→H→W (already merged in PR #507). PR #519 builds on that as a new tier (Tier 0 = cohort hard-filter wrapping Tier W). Direction consistent with audit intent (year mismatch dominates relevance score).
cast-in-is-not-null-predicates memory rule — respected (CAST present, comment cites PR #518).

Pre-flight

Branch base: main · mergeable: true · 1 commit
No --no-verify / --force / --amend indicators
Diff is 1 file, 1 worker, no scope creep

Risk / blast radius

Risk: low. Pure additive logic with default-None graceful no-op. Existing behaviour unchanged when target_year not provided or out-of-cohort-range. Tier A fallback preserves current results.
Reversibility: easy revert (single file).
Merge window: any time.

Verdict

## Deep Code Review — verdict ### Summary - Status: APPROVE (with notes) - Files: 1 (P0: estimator.py) · +83/-9 · 1 SQL block touched - PR #519 · head 1d4d585 · base main ### Strengths - **CAST preserved correctly** in `IS NULL` predicate (`CAST(:cohort_year_min AS integer) IS NULL`) — matches PR #518 fix and explicit comment cites it. No psycopg v3 AmbiguousParameter trap. - **Graceful fallback** when `target_year is None` or out-of-range (1900/2050) → `cohort_range=None` → Tier 0 skipped → behaviour identical to pre-PR. Fully backward-compatible default (`cohort_year_min/max=None`). - **NULL year_built tolerated** in WHERE clause (`OR year_built IS NULL`) — avoids dropping Avito anonymous-address listings with incomplete metadata. Correct trade-off. - 5 cohort definitions match РФ массовой застройки typology; first-match semantics documented. - API contract / DB schema / frontend / Tier B-C logic — untouched. Blast radius is correctly minimised. ### MEDIUM (non-blocking, worth follow-up) **1. Cohort filter only applies on Tier W path of `_fetch_analogs` call.** When `cohort_range` is set and `_fetch_analogs(...cohort_year_min=...)` runs, the SQL clause lives only inside the Tier W SELECT (head line 1285-1290). Tier S (line 1097, `address ILIKE prefix`) and Tier H (line 1175-1180, `year_built BETWEEN target-15 AND target+15`) execute BEFORE the cohort filter ever evaluates — they have early-return at lines 1122 / 1214. Concrete leak: `target_year=1978` (brezhnev cohort = 1970–1989). Tier H allows `year_built ∈ [1963, 1993]`. A 1965 khrushchev or 1992 late_soviet building within 1km with ≥5 same-rooms+area listings → Tier H wins → cohort filter is bypassed → `analog_tier='H'` returned. PR description claims `Tier 0: 1km + ±15% area + cohort match` but in reality cohort match is enforced only on Tier W. For the target audit case (Малышева 84, 1978, 9-эт brezhnev) impact is small — year±15 = 1963-1993 leaks ≤7 years across cohort boundaries vs current ±50-year noise from 31-эт novostroyki. Still an improvement. But the PR over-states intent. Suggested follow-up (separate PR): pass cohort to Tier H too: ```python # inside Tier H WHERE AND year_built BETWEEN CAST(:year_min AS integer) AND CAST(:year_max AS integer) AND ( CAST(:cohort_year_min AS integer) IS NULL OR year_built BETWEEN CAST(:cohort_year_min AS integer) AND CAST(:cohort_year_max AS integer) ) ``` **2. Double-execute on Tier 0 fallback.** When `len(listings_tier0) < MIN_ANALOGS_TIER_0` (5), Tier A re-calls `_fetch_analogs` without cohort. Inside, Tier S/H run identically to the first call (cohort filter doesn't touch them). So Tier S/H SQL roundtrips fire 2× whenever Tier 0 falls back. ~2 extra DB queries per estimate on the fallback path. Sub-second, but unnecessary. Optimisation deferred — out of scope here. **3. No tier-0 observability in API response.** The `analog_tier` returned from Tier 0 call is whatever internal tier resolved (S/H/W). `tier_note` (line 634-643) doesn't distinguish `matched via cohort` from `matched without cohort`. User-facing `explanation` won't reflect whether cohort filter actually ran. If you want to A/B verify the audit fix (Малышева 84) — there's no signal in the response. Consider emitting `cohort_applied: bool` in `confidence_explanation` or adding a structured field. Out of scope here. ### MINOR **4. No tests added.** PR body acknowledges venv issue in worker. `_target_cohort_range` is the cleanest place to add a 6-case parametrized unit test (None / 1900 / 1969 / 1970 / 1988 → brezhnev / 1990 → late_soviet / 2050 → None). Smoke test plan in PR body is reasonable. Not a blocker because logic is small and graceful-fallback design contains blast radius. **5. `late_soviet` cohort effectively redundant** (1985-1999 overlaps brezhnev 1970-1989). First-match picks brezhnev for 1985-1989 → late_soviet covers only 1990-1999. Comment says `overlap intentional` — confirm semantics. TODO worth: rename to `early_90s` or shift to `late_soviet=1990–1999` to remove confusion. **6. Vault decision not extended.** `Decision_TradeIn_DataQuality_8PR_Roadmap` ends at PR 8. This is effectively a PR 9 added from a second audit (UUID d4ec4610). Main session should append PR 9 entry to the decision doc post-merge. Out of scope for this PR. ### Cross-file impact - Single-file change. Caller `estimate_quality` (line 528) is the only entry point invoking the new `cohort_range` orchestration. - API endpoint `tradein-mvp/backend/app/api/v1/trade_in.py` → calls `estimate_quality` → no schema change in `AggregatedEstimate`. - Frontend untouched. - Tier S/H cohort blind-spot (item 1) is logical, not cross-file. ### Vault cross-check - `Decision_TradeIn_DataQuality_8PR_Roadmap` — original PR 3 was `house-match tiered S→H→W` (already merged in PR #507). PR #519 builds on that as a new tier (Tier 0 = cohort hard-filter wrapping Tier W). Direction consistent with audit intent (year mismatch dominates relevance score). - `cast-in-is-not-null-predicates` memory rule — respected (CAST present, comment cites PR #518). ### Pre-flight - Branch base: main · mergeable: true · 1 commit - No `--no-verify` / `--force` / `--amend` indicators - Diff is 1 file, 1 worker, no scope creep ### Risk / blast radius - Risk: **low**. Pure additive logic with default-None graceful no-op. Existing behaviour unchanged when `target_year` not provided or out-of-cohort-range. Tier A fallback preserves current results. - Reversibility: easy revert (single file). - Merge window: any time. ### Verdict Approve and merge. Notes (1) and (3) above are worth tracking as follow-up tasks but don't block this PR — the primary audit issue (Малышева 84 returning 31-эт novostroyki) gets meaningfully addressed by the Tier W cohort hard-filter, and the architecture leaves room for tightening Tier H later.