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.
This commit is contained in:
parent
46db7ebcba
commit
4b0fd10cb0
4 changed files with 62 additions and 0 deletions
|
|
@ -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
|
||||
)
|
||||
|
|
|
|||
|
|
@ -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 (
|
||||
|
|
|
|||
|
|
@ -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 ──────────────────────
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -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"
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue