From f5a4b8ce017dd272fdc598866eed511b7a5ed8a8 Mon Sep 17 00:00:00 2001 From: bot-backend Date: Tue, 11 Aug 2026 08:09:29 +0300 Subject: [PATCH] =?UTF-8?q?fix(tradein/estimate):=20=D0=BD=D0=B5=20=D0=BF?= =?UTF-8?q?=D0=B5=D1=80=D0=B5=D1=82=D0=B8=D1=80=D0=B0=D1=82=D1=8C=20=D0=B4?= =?UTF-8?q?=D0=B0=D1=82=D1=83=20=D0=BE=D0=B1=D1=80=D0=B0=D1=89=D0=B5=D0=BD?= =?UTF-8?q?=D0=B8=D1=8F=20=D0=BF=D1=80=D0=B8=20=D0=BE=D0=B6=D0=B8=D0=B2?= =?UTF-8?q?=D0=BB=D0=B5=D0=BD=D0=B8=D0=B8=20=D0=BE=D1=86=D0=B5=D0=BD=D0=BA?= =?UTF-8?q?=D0=B8?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Оживление мёртвых оценок (#2826) копировало в оригинальную строку поле created_at временной строки, которую создаёт estimate_quality. На проде оценка ff421062, созданная 2026-08-10 12:54, после оживления получила created_at = 2026-08-11 04:30. created_at — дата обращения клиента, а не дата нашего пересчёта. Второй симптом того же бага: /history сортирует по created_at, и оживлённая старая запись прыгала в начало списка. - created_at убран из SET в persist-UPDATE, ответ отдаёт дату исходной строки, а не временной - revival_completed_at (миграция 256_*, идемпотентная) — отдельный аудит-след «когда успешно пересчитали»; revival_attempted_at из 255_* ставится на захвате и включает неудачи с троттлингом - проверены остальные поля исходного обращения (TTL, снимок входных данных, согласие 152-ФЗ) — они и раньше не входили в SET PDF не затронут: там печатается дата формирования отчёта, не created_at. --- tradein-mvp/backend/app/api/v1/trade_in.py | 17 +++++- ...rade_in_estimates_revival_completed_at.sql | 48 ++++++++++++++++ .../backend/tests/test_estimate_revival.py | 56 +++++++++++++++++++ 3 files changed, 118 insertions(+), 3 deletions(-) create mode 100644 tradein-mvp/backend/data/sql/256_trade_in_estimates_revival_completed_at.sql diff --git a/tradein-mvp/backend/app/api/v1/trade_in.py b/tradein-mvp/backend/app/api/v1/trade_in.py index 7c806bf0..2926e7e6 100644 --- a/tradein-mvp/backend/app/api/v1/trade_in.py +++ b/tradein-mvp/backend/app/api/v1/trade_in.py @@ -299,6 +299,16 @@ async def _try_revive_dead_estimate( # into the ORIGINAL row (id/link contract), then drop the throwaway one. # INPUT snapshot (address/area/rooms/...) is untouched — it did not change, # only the outputs were recomputed. + # #incident-2026-08-11: created_at is DELIBERATELY excluded from this SET — + # it is the client's original request date (printed in /history and in + # AggregatedEstimate.created_at, see app/schemas/trade_in.py:317-318), NOT + # a recompute output. It previously got clobbered with the throwaway temp + # row's created_at (=NOW() at recompute time), which also silently + # re-sorted the row to the top of `GET /history ORDER BY created_at DESC`. + # revival_completed_at (migration 256) is the audit trail for "when did a + # revival LAST successfully rewrite this row" — distinct from + # revival_attempted_at (255), which is stamped on every claim regardless + # of outcome (throttle loss / recompute failure included). db.execute( text( """ @@ -337,7 +347,7 @@ async def _try_revive_dead_estimate( ratio_basis = :ratio_basis, relaxations = CAST(:relaxations_json AS jsonb), reliability = :reliability, - created_at = :created_at + revival_completed_at = NOW() WHERE id = CAST(:id AS uuid) """ ), @@ -371,7 +381,6 @@ async def _try_revive_dead_estimate( "ratio_basis": result.ratio_basis, "relaxations_json": json.dumps(result.relaxations, ensure_ascii=False), "reliability": result.reliability, - "created_at": result.created_at, }, ) db.execute( @@ -388,7 +397,9 @@ async def _try_revive_dead_estimate( result.confidence, result.reliability, ) - return result.model_copy(update={"estimate_id": estimate_id}) + # created_at on the returned object must mirror the DB row (untouched + # original request date, NOT the temp row's NOW()) — see UPDATE above. + return result.model_copy(update={"estimate_id": estimate_id, "created_at": row.created_at}) @router.post("/estimate", response_model=AggregatedEstimate) diff --git a/tradein-mvp/backend/data/sql/256_trade_in_estimates_revival_completed_at.sql b/tradein-mvp/backend/data/sql/256_trade_in_estimates_revival_completed_at.sql new file mode 100644 index 00000000..499f2f79 --- /dev/null +++ b/tradein-mvp/backend/data/sql/256_trade_in_estimates_revival_completed_at.sql @@ -0,0 +1,48 @@ +-- 256_trade_in_estimates_revival_completed_at.sql +-- Номер сверен по `ls data/sql | sort` (max applied = 255) непосредственно +-- перед коммитом — см. sql.md § file naming + tradein.md § collision trap +-- (108_*/084_* уже дублировались в прошлом). +-- +-- ── fix/tradein-created (2026-08-11): created_at перезаписывался revival'ом ── +-- `_try_revive_dead_estimate` (app/api/v1/trade_in.py, migration 255) писал +-- пересчитанные поля в ИСХОДНУЮ строку trade_in_estimates, и вместе с ними — +-- `created_at`, скопированный из временной строки (estimate_quality() ставит +-- туда NOW() на момент пересчёта). Факт с прода: оценка +-- ff421062-cc38-4c4c-ad2e-0cfac52d14ff создана 2026-08-10 12:54:47, после +-- revival'а на GET её created_at стал 2026-08-11 04:30:03 — «дата обращения» +-- клиента (печатается в /history и в схеме AggregatedEstimate.created_at, +-- см. app/schemas/trade_in.py:317-318 «для метки «отчёт от DD.MM» в UI») +-- подменилась датой служебного пересчёта. Заодно ломался ORDER BY created_at +-- DESC в GET /history — оживлённая старая заявка выпрыгивала в начало списка. +-- +-- Фикс (app/api/v1/trade_in.py): created_at исключён из UPDATE SET revival'а, +-- исходная дата больше не трогается. Момент, когда revival РЕАЛЬНО пересчитал +-- строку (не просто "попытался" — revival_attempted_at из 255 ставится на +-- claim'е ДО вызова estimate_quality(), в том числе при throttle-проигрыше и +-- при неудачном пересчёте), нужен для аудита отдельно — новая колонка. +-- +-- ── IDEMPOTENCY ─────────────────────────────────────────────────────────── +-- ADD COLUMN IF NOT EXISTS — безопасный re-run. Бэкфилла нет: NULL = либо +-- строка живая и revival никогда успешно не пересчитывал, либо запись +-- создана до этой колонки. +-- +-- Dependencies: 255_trade_in_estimates_revival_relaxations.sql. Apply after: 255_*. + +BEGIN; + +SET LOCAL lock_timeout = '5s'; + +ALTER TABLE trade_in_estimates + ADD COLUMN IF NOT EXISTS revival_completed_at timestamptz; + +COMMENT ON COLUMN trade_in_estimates.revival_completed_at IS + 'Момент УСПЕШНОГО пересчёта «мёртвой» (median_price<=0/NULL) строки ' + 'revival''ом (app/api/v1/trade_in.py::_try_revive_dead_estimate) — ' + 'выставляется, когда пересчёт реально записал новые значения в строку. ' + 'Отличается от revival_attempted_at (255): тот ставится на claim''е ДО ' + 'вызова estimate_quality() и фиксирует ЛЮБУЮ попытку (включая throttled-' + 'проигрыш гонки и неудачный пересчёт), этот — только успех. created_at ' + 'строки при этом НЕ меняется (исходная дата обращения клиента, печатается ' + 'в /history) — см. fix/tradein-created 2026-08-11.'; + +COMMIT; diff --git a/tradein-mvp/backend/tests/test_estimate_revival.py b/tradein-mvp/backend/tests/test_estimate_revival.py index 42a0b439..e1758d29 100644 --- a/tradein-mvp/backend/tests/test_estimate_revival.py +++ b/tradein-mvp/backend/tests/test_estimate_revival.py @@ -335,6 +335,62 @@ def test_dead_row_revives_and_updates_db( assert delete_calls[0].args[1]["id"] == _TEMP_ID +def test_dead_row_revival_preserves_created_at( + trade_in_app: FastAPI, _estimator_stub: SimpleNamespace +) -> None: + """#incident-2026-08-11: created_at is the client's ORIGINAL request date + (printed on /history, see app/schemas/trade_in.py:317-318) — revival must + not clobber it with the throwaway temp row's NOW(). Regression for the + prod incident (estimate ff421062-...: created_at jumped from the original + 2026-08-10 12:54:47 to the revival moment 2026-08-11 04:30:03). + + Also asserts: (a) the persist UPDATE never sets created_at at all — the + fix removes the column from SET, it doesn't just overwrite it with the + right value; (b) migration 256's revival_completed_at IS stamped, as the + separate "when did revival last succeed" audit trail; (c) the JSON + response mirrors the original created_at, not the temp result's. + """ + _original_created_at = datetime(2026, 5, 29, tzinfo=UTC) + + async def _fake_estimate_quality(payload, db, **kwargs): + # The temp row estimate_quality() mints internally always carries + # NOW() as its created_at — deliberately far from the original, so a + # regression (copying result.created_at through) is unmissable. + return _fake_revived_result(created_at=datetime(2026, 8, 11, 4, 30, 3, tzinfo=UTC)) + + _estimator_stub.estimate_quality = _fake_estimate_quality + + row = _make_dead_row() + row.created_at = _original_created_at + db = _dispatch_db(row, claim_result=SimpleNamespace(id=_ESTIMATE_ID)) + client = _client_with(trade_in_app, db) + resp = client.get( + f"/api/v1/trade-in/estimate/{_ESTIMATE_ID}", + headers={"X-Authenticated-User": "kopylov"}, + ) + assert resp.status_code == 200 + body = resp.json() + assert body["created_at"] == "2026-05-29T00:00:00Z" + + persist_calls = _calls_containing(db, "UPDATE trade_in_estimates SET") + assert len(persist_calls) == 1 + persist_sql = persist_calls[0].args[0].text + persist_params = persist_calls[0].args[1] + assert "created_at" not in persist_sql + assert "created_at" not in persist_params + assert "revival_completed_at = NOW()" in persist_sql + # Input-snapshot / TTL columns (what the client originally asked for and + # for how long the row is retained) are likewise not recompute outputs — + # untouched by the revival persist UPDATE. + # NB: "address" is checked via persist_params only (not persist_sql) — + # canonical_address (a legitimate recompute output) ends in "address =", + # which would false-positive a substring check against the raw SQL text. + for protected in ("expires_at", "retain_until", "created_by"): + assert f"{protected} =" not in persist_sql + assert protected not in persist_params + assert "address" not in persist_params + + # ── Live row is never touched ────────────────────────────────────────────