fix(tradein-estimator): atomic IMV link, batched yandex history, drop redundant CAST (#509)
This commit is contained in:
parent
d4cbd4bb9f
commit
7d8c22ff39
2 changed files with 78 additions and 66 deletions
|
|
@ -394,8 +394,15 @@ def _save_yandex_history_items(
|
|||
Idempotent via UNIQUE (source, ext_item_id); we synthesize ext_item_id from
|
||||
(address|date|area|floor) hash since Yandex history items don't carry an
|
||||
explicit ID.
|
||||
|
||||
Batch semantics (closes finding #5 from 2026-05-24 audit): all items
|
||||
inserted under a single try/except; on any failure the whole batch is
|
||||
rolled back as a unit (prior per-item rollback() destroyed the parent tx).
|
||||
"""
|
||||
saved = 0
|
||||
if not result.history_items:
|
||||
return 0
|
||||
|
||||
rows = []
|
||||
for item in result.history_items:
|
||||
# Synthesize stable ext_item_id (no native ID in valuation page)
|
||||
ext_seed = (
|
||||
|
|
@ -403,48 +410,51 @@ def _save_yandex_history_items(
|
|||
f"{item.start_price}|{item.last_price}"
|
||||
)
|
||||
ext_item_id = hashlib.sha256(ext_seed.encode("utf-8")).hexdigest()[:32]
|
||||
try:
|
||||
db.execute(
|
||||
text(
|
||||
"""
|
||||
INSERT INTO house_placement_history (
|
||||
source, ext_item_id,
|
||||
rooms, area_m2, floor,
|
||||
start_price, start_price_date,
|
||||
last_price, last_price_date,
|
||||
exposure_days,
|
||||
raw_payload
|
||||
) VALUES (
|
||||
'yandex_valuation', :ext_id,
|
||||
:rooms, :area, :floor,
|
||||
:start_price, :publish_date,
|
||||
:last_price, :publish_date,
|
||||
:exposure,
|
||||
CAST(:raw AS jsonb)
|
||||
)
|
||||
ON CONFLICT (source, ext_item_id) DO NOTHING
|
||||
"""
|
||||
),
|
||||
{
|
||||
"ext_id": ext_item_id,
|
||||
"rooms": item.rooms,
|
||||
"area": item.area_m2,
|
||||
"floor": item.floor,
|
||||
"start_price": item.start_price,
|
||||
"last_price": item.last_price,
|
||||
"publish_date": item.publish_date,
|
||||
"exposure": item.exposure_days,
|
||||
"raw": json.dumps(item.model_dump(mode="json"), ensure_ascii=False),
|
||||
},
|
||||
)
|
||||
saved += 1
|
||||
except Exception as e:
|
||||
logger.warning("yandex_valuation: failed to save history item: %s", e)
|
||||
db.rollback()
|
||||
continue
|
||||
if saved:
|
||||
rows.append({
|
||||
"ext_id": ext_item_id,
|
||||
"rooms": item.rooms,
|
||||
"area": item.area_m2,
|
||||
"floor": item.floor,
|
||||
"start_price": item.start_price,
|
||||
"last_price": item.last_price,
|
||||
"publish_date": item.publish_date,
|
||||
"exposure": item.exposure_days,
|
||||
"raw": json.dumps(item.model_dump(mode="json"), ensure_ascii=False),
|
||||
})
|
||||
|
||||
sql = text(
|
||||
"""
|
||||
INSERT INTO house_placement_history (
|
||||
source, ext_item_id,
|
||||
rooms, area_m2, floor,
|
||||
start_price, start_price_date,
|
||||
last_price, last_price_date,
|
||||
exposure_days,
|
||||
raw_payload
|
||||
) VALUES (
|
||||
'yandex_valuation', :ext_id,
|
||||
:rooms, :area, :floor,
|
||||
:start_price, :publish_date,
|
||||
:last_price, :publish_date,
|
||||
:exposure,
|
||||
CAST(:raw AS jsonb)
|
||||
)
|
||||
ON CONFLICT (source, ext_item_id) DO NOTHING
|
||||
"""
|
||||
)
|
||||
|
||||
try:
|
||||
for row in rows:
|
||||
db.execute(sql, row)
|
||||
db.commit()
|
||||
return saved
|
||||
return len(rows)
|
||||
except Exception as e:
|
||||
logger.warning(
|
||||
"yandex_valuation: failed to save history batch (%d items): %s",
|
||||
len(rows), e,
|
||||
)
|
||||
db.rollback()
|
||||
return 0
|
||||
|
||||
|
||||
# ── Public ───────────────────────────────────────────────────────────────────
|
||||
|
|
@ -718,25 +728,25 @@ async def estimate_quality(
|
|||
"expires_at": expires_at,
|
||||
},
|
||||
)
|
||||
db.commit()
|
||||
|
||||
# Link saved IMV evaluation к этому estimate_id (для analytics joining)
|
||||
# Link saved IMV evaluation к этому estimate_id атомарно с основным INSERT
|
||||
# (closes finding #4 from 2026-05-24 audit — prior code committed estimate first,
|
||||
# then UPDATEd IMV in a separate tx, racing against concurrent estimators
|
||||
# sharing the same cache_key).
|
||||
if imv_eval is not None:
|
||||
try:
|
||||
db.execute(
|
||||
text(
|
||||
"""
|
||||
UPDATE avito_imv_evaluations
|
||||
SET estimate_id = CAST(:estimate_id AS uuid)
|
||||
WHERE cache_key = :cache_key
|
||||
AND (estimate_id IS NULL OR estimate_id = CAST(:estimate_id AS uuid))
|
||||
"""
|
||||
),
|
||||
{"estimate_id": str(estimate_id), "cache_key": imv_eval.cache_key},
|
||||
)
|
||||
db.commit()
|
||||
except Exception as e:
|
||||
logger.warning("imv: failed to link estimate_id to evaluation: %s", e)
|
||||
db.execute(
|
||||
text(
|
||||
"""
|
||||
UPDATE avito_imv_evaluations
|
||||
SET estimate_id = CAST(:estimate_id AS uuid)
|
||||
WHERE cache_key = :cache_key
|
||||
AND (estimate_id IS NULL OR estimate_id = CAST(:estimate_id AS uuid))
|
||||
"""
|
||||
),
|
||||
{"estimate_id": str(estimate_id), "cache_key": imv_eval.cache_key},
|
||||
)
|
||||
|
||||
db.commit()
|
||||
|
||||
logger.info(
|
||||
"estimate: id=%s addr=%s rooms=%d area=%.1f → median=%d (n=%d, conf=%s)%s%s",
|
||||
|
|
@ -870,13 +880,13 @@ def _fetch_analogs(
|
|||
-- без типа → PostgreSQL "could not determine data type of parameter"
|
||||
-- (AmbiguousParameter). Явный тип снимает неоднозначность.
|
||||
+ CASE
|
||||
WHEN CAST(:target_year AS integer) IS NOT NULL
|
||||
WHEN :target_year IS NOT NULL
|
||||
AND year_built IS NOT NULL
|
||||
THEN abs(year_built - CAST(:target_year AS integer)) / 12.0
|
||||
ELSE 0
|
||||
END
|
||||
+ CASE
|
||||
WHEN CAST(:target_house_type AS text) IS NOT NULL
|
||||
WHEN :target_house_type IS NOT NULL
|
||||
AND house_type IS NOT NULL
|
||||
AND house_type <> CAST(:target_house_type AS text)
|
||||
THEN 1.5
|
||||
|
|
@ -889,13 +899,13 @@ def _fetch_analogs(
|
|||
ST_Distance(geom::geography, ST_MakePoint(:lon, :lat)::geography)
|
||||
/ 1000.0
|
||||
+ CASE
|
||||
WHEN CAST(:target_year AS integer) IS NOT NULL
|
||||
WHEN :target_year IS NOT NULL
|
||||
AND year_built IS NOT NULL
|
||||
THEN abs(year_built - CAST(:target_year AS integer)) / 12.0
|
||||
ELSE 0
|
||||
END
|
||||
+ CASE
|
||||
WHEN CAST(:target_house_type AS text) IS NOT NULL
|
||||
WHEN :target_house_type IS NOT NULL
|
||||
AND house_type IS NOT NULL
|
||||
AND house_type <> CAST(:target_house_type AS text)
|
||||
THEN 1.5
|
||||
|
|
|
|||
|
|
@ -188,10 +188,12 @@ def test_save_history_items_ext_id_stable_across_calls():
|
|||
assert ext_ids_1 == ext_ids_2
|
||||
|
||||
|
||||
def test_save_history_items_db_error_continues():
|
||||
"""One item failing doesn't abort the others."""
|
||||
def test_save_history_items_db_error_rolls_back_batch():
|
||||
"""Any item failing rolls back the whole batch — batch semantics (finding #5)."""
|
||||
db = MagicMock()
|
||||
db.execute.side_effect = [RuntimeError("first row fails"), None]
|
||||
result = _sample_result()
|
||||
saved = _save_yandex_history_items(db, result)
|
||||
assert saved == 1 # second item still saved
|
||||
assert saved == 0 # whole batch rolled back
|
||||
db.rollback.assert_called_once()
|
||||
db.commit.assert_not_called()
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue