From 4b0fd10cb041daedd46e1687b57e0c873be28a92 Mon Sep 17 00:00:00 2001
From: bot-backend <bot-backend@gendsgn.local>
Date: Mon, 1 Jun 2026 00:13:16 +0300
Subject: [PATCH] fix(tradein): #928 cache-key collision, migration-098 band
 guard, tier-signal note

M1 (bug): flag-OFF path now caches under (bucket, '_legacy') instead of
(bucket, None), preventing stale old-table ratio being served when
tier_aware_ratio_enabled is flipped ON without process restart.

M2 (guard): add band-drift comment to migration 098 seed + regression test
asserting _PPM2_MIN==30000 and asking_ratio_ppm2_max default==1_200_000.

M3 (doc): append NOTE(#928) to estimator call-site comment explaining that
tier placement uses the pre-anchor radius median, not the same-building Tukey
anchor; flags the revisit needed before enabling the flag in production.
---
 tradein-mvp/backend/app/services/estimator.py | 10 +++++++
 .../sql/098_asking_to_sold_ratios_tiered.sql  |  3 +++
 .../services/test_asking_sold_tier_ratio.py   | 23 ++++++++++++++++
 .../tests/test_asking_to_sold_ratio.py        | 26 +++++++++++++++++++
 4 files changed, 62 insertions(+)

diff --git a/tradein-mvp/backend/app/services/estimator.py b/tradein-mvp/backend/app/services/estimator.py
index 112bb74d..931c1d15 100644
--- a/tradein-mvp/backend/app/services/estimator.py
+++ b/tradein-mvp/backend/app/services/estimator.py
@@ -238,6 +238,11 @@ def _get_asking_sold_ratio(
 
     if not use_tier_path:
         # ── Flag-OFF path: byte-identical behavior (legacy asking_to_sold_ratios) ──
+        # Use a distinct key "_legacy" to avoid collision with the flag-ON Step-3 'all'
+        # fallback which also caches under (bucket, None). Without this, a process that
+        # flips tier_aware_ratio_enabled ON without restart could serve the old-table ratio
+        # from the flag-OFF entry for up to the TTL — wrong table.
+        cache_key = (bucket, "_legacy")
         cached = _asking_sold_ratio_cache.get(cache_key)
         if cached is not None:
             ratio, basis, fetched = cached
@@ -2014,6 +2019,11 @@ async def estimate_quality(
     # #928: pass median_ppm2 (best proxy available at this point — anchor_ppm2 from
     # same-building anchor computed below, but that's post-call). median_ppm2 = 0 when
     # no radius analogs yet; tier lookup uses it as the ppm2 placement signal.
+    # NOTE(#928): tier placement uses the pre-anchor radius median (median_ppm2), not the
+    # same-building Tukey anchor that #928 ideally specifies (anchor is computed post-call).
+    # Coarse 3-tier bucketing is robust to this for the golden case; revisit (resolve ratio
+    # after anchor) before flipping tier_aware_ratio_enabled ON — validate in the held-out
+    # backtest.
     asking_to_sold_ratio, ratio_basis = _get_asking_sold_ratio(
         db, payload.rooms, anchor_ppm2=median_ppm2 if median_ppm2 > 0 else None
     )
diff --git a/tradein-mvp/backend/data/sql/098_asking_to_sold_ratios_tiered.sql b/tradein-mvp/backend/data/sql/098_asking_to_sold_ratios_tiered.sql
index da6ab16d..c10dbf2a 100644
--- a/tradein-mvp/backend/data/sql/098_asking_to_sold_ratios_tiered.sql
+++ b/tradein-mvp/backend/data/sql/098_asking_to_sold_ratios_tiered.sql
@@ -90,6 +90,9 @@ COMMENT ON TABLE asking_to_sold_ratios_tiered IS
 -- Seed derivation ----------------------------------------------------------------
 -- ppm2-полоса [30000, 1200000] -- settings-default (#767). Окно = трейлинг 12 мес.
 -- ON CONFLICT DO UPDATE -- идемпотентный re-apply / dry-run.
+-- ppm² band literals MUST equal _PPM2_MIN (30000) and settings.asking_ratio_ppm2_max
+-- default (1_200_000) — see app/tasks/asking_to_sold_ratio.py. If that setting's DEFAULT
+-- changes, update these literals (or fresh-seed vs daily-refresh diverge).
 
 -- Step 1: bounds (ppm2-квантили по listings per бакет) ---------------------------
 WITH bounds AS (
diff --git a/tradein-mvp/backend/tests/services/test_asking_sold_tier_ratio.py b/tradein-mvp/backend/tests/services/test_asking_sold_tier_ratio.py
index c87c2cde..ecf922ee 100644
--- a/tradein-mvp/backend/tests/services/test_asking_sold_tier_ratio.py
+++ b/tradein-mvp/backend/tests/services/test_asking_sold_tier_ratio.py
@@ -86,6 +86,29 @@ def test_flag_off_no_anchor_ppm2_still_works() -> None:
     assert basis == "per_rooms"
 
 
+def test_flag_off_cache_key_is_legacy_not_none() -> None:
+    """Flag OFF must cache under (bucket, '_legacy'), NOT (bucket, None).
+
+    This prevents key collision with the flag-ON Step-3 'all' fallback which also
+    caches under (bucket, None). If the process flips tier_aware_ratio_enabled ON
+    without restart, the '_legacy' entry is not accidentally served as the 'all' row.
+    """
+    from app.core.config import settings
+    from app.services import estimator
+    from app.services.estimator import _get_asking_sold_ratio
+
+    _clear_ratio_cache()
+    bucket = min(max(2, 0), 4)
+    db = _db_sequence([_FakeRow(ratio=0.77, basis="per_rooms")])
+
+    with patch.object(settings, "tier_aware_ratio_enabled", False):
+        _get_asking_sold_ratio(db, 2)
+
+    # Flag-OFF entry must be under "_legacy", not None.
+    assert (bucket, "_legacy") in estimator._asking_sold_ratio_cache
+    assert (bucket, None) not in estimator._asking_sold_ratio_cache
+
+
 # ── 2. Flag ON, anchor in low band -> shrunk tier ratio ──────────────────────
 
 
diff --git a/tradein-mvp/backend/tests/test_asking_to_sold_ratio.py b/tradein-mvp/backend/tests/test_asking_to_sold_ratio.py
index 085e6b84..0a7c90d4 100644
--- a/tradein-mvp/backend/tests/test_asking_to_sold_ratio.py
+++ b/tradein-mvp/backend/tests/test_asking_to_sold_ratio.py
@@ -358,3 +358,29 @@ def test_counter_logic_failure_path_marks_failed(monkeypatch: pytest.MonkeyPatch
         ratio_mod.recompute_asking_to_sold_ratios(db, run_id=7)  # type: ignore[arg-type]
     assert db.rolled_back is True
     assert failed["run_id"] == 7
+
+
+# ── Migration 098 band / settings-default consistency ────────────────────────
+
+
+def test_migration_098_band_matches_settings_default() -> None:
+    """Migration 098 seed hardcodes ppm² band [30000, 1_200_000].
+
+    These literals MUST stay in sync with:
+      - app/tasks/asking_to_sold_ratio._PPM2_MIN  (lower bound)
+      - app.core.config.Settings.asking_ratio_ppm2_max default (upper bound)
+
+    If either default changes, the one-time migration seed diverges from the
+    daily-refresh output — this test is the regression guard.
+    """
+    from app.core.config import Settings
+    from app.tasks.asking_to_sold_ratio import _PPM2_MIN
+
+    # Lower bound — matches the 30000 literal in migration 098.
+    assert (
+        _PPM2_MIN == 30_000
+    ), f"_PPM2_MIN changed ({_PPM2_MIN}); update migration 098 seed literals to match"
+    # Upper bound — matches the 1200000 literal in migration 098.
+    assert (
+        Settings().asking_ratio_ppm2_max == 1_200_000
+    ), "asking_ratio_ppm2_max default changed; update migration 098 seed literals to match"