fix(tradein/quota): per-user quota override + no burn on empty result #2517

Merged

lekss361 merged 4 commits from fix/tradein-quota-per-user-and-no-burn into main

2026-07-13 20:18:43 +00:00

bot-backend commented

2026-07-13 19:53:46 +00:00

Collaborator

Summary

Task 1 (per-user quota override): replace the negative-used bonus hack with a proper account_quota_overrides(username, monthly_limit) table (migration 184_account_quota_overrides.sql). account_quota.user_limit(db, username) reads the override and is now used across get_status/check_and_raise/increment instead of the hardcoded global MONTHLY_LIMIT (incl. the atomic WHERE used < :lim predicate). get_status clamps remaining = max(0, limit - max(0, used)) so remaining can never exceed limit even if a negative used leaks in again. Migration seeds user2 (Брусника) -> monthly_limit=50 and resets the old hack (used < 0 -> 0) so /quota now returns {limit:50, used:0, remaining:50} instead of the absurd {limit:15, used:-35, remaining:50}.
Task 2 (no burn on empty result): POST /estimate no longer unconditionally calls account_quota.increment() after estimate_quality. When the address fails to geocode and the handler falls back to _empty_estimate (median=0, n_analogs=0, insufficient_data=True, HTTP 200), the quota slot is preserved -- increment is now gated on not result.insufficient_data. The #747 TOCTOU-safe atomic increment/429 gate is unchanged for real (non-empty) results.

Test plan

cd tradein-mvp/backend && uv run --frozen python -m pytest tests/test_account_quota.py -q -> 44 passed (added override + insufficient-data coverage)
uv run --frozen python -m pytest tests/test_api_avito_stage4a.py tests/test_estimate_idor.py tests/test_estimator_anchor_dedup_gap_2265.py tests/test_sell_time_sensitivity.py -q -> 38 passed (other /estimate callers unaffected)
uv run --with ruff ruff check app/services/account_quota.py app/api/v1/trade_in.py tests/test_account_quota.py -> clean
Post-deploy: verify prod GET /api/v1/trade-in/quota for user2 returns {limit:50, used:0, remaining:50} after migration 184 auto-applies

## Summary - **Task 1 (per-user quota override)**: replace the negative-`used` bonus hack with a proper `account_quota_overrides(username, monthly_limit)` table (migration `184_account_quota_overrides.sql`). `account_quota.user_limit(db, username)` reads the override and is now used across `get_status`/`check_and_raise`/`increment` instead of the hardcoded global `MONTHLY_LIMIT` (incl. the atomic `WHERE used < :lim` predicate). `get_status` clamps `remaining = max(0, limit - max(0, used))` so remaining can never exceed limit even if a negative `used` leaks in again. Migration seeds `user2` (Брусника) -> `monthly_limit=50` and resets the old hack (`used < 0 -> 0`) so `/quota` now returns `{limit:50, used:0, remaining:50}` instead of the absurd `{limit:15, used:-35, remaining:50}`. - **Task 2 (no burn on empty result)**: `POST /estimate` no longer unconditionally calls `account_quota.increment()` after `estimate_quality`. When the address fails to geocode and the handler falls back to `_empty_estimate` (median=0, n_analogs=0, `insufficient_data=True`, HTTP 200), the quota slot is preserved -- increment is now gated on `not result.insufficient_data`. The `#747` TOCTOU-safe atomic increment/429 gate is unchanged for real (non-empty) results. ## Test plan - [x] `cd tradein-mvp/backend && uv run --frozen python -m pytest tests/test_account_quota.py -q` -> 44 passed (added override + insufficient-data coverage) - [x] `uv run --frozen python -m pytest tests/test_api_avito_stage4a.py tests/test_estimate_idor.py tests/test_estimator_anchor_dedup_gap_2265.py tests/test_sell_time_sensitivity.py -q` -> 38 passed (other `/estimate` callers unaffected) - [x] `uv run --with ruff ruff check app/services/account_quota.py app/api/v1/trade_in.py tests/test_account_quota.py` -> clean - [ ] Post-deploy: verify prod `GET /api/v1/trade-in/quota` for user2 returns `{limit:50, used:0, remaining:50}` after migration 184 auto-applies

bot-backend added 1 commit 2026-07-13 19:53:47 +00:00

fix(tradein/quota): per-user quota override + no burn on empty result

CI Trade-In / backend-tests (pull_request) Successful in 4m54s

Details

CI Trade-In / changes (pull_request) Successful in 8s

Details

CI / changes (pull_request) Successful in 8s

Details

CI Trade-In / frontend-checks (pull_request) Has been skipped

Details

CI / backend-tests (pull_request) Has been skipped

Details

CI / frontend-tests (pull_request) Has been skipped

Details

CI / openapi-codegen-check (pull_request) Has been skipped

Details

9c12407c09

Two bugs hit the first paying pilot (user2/Брусника):

1. Bonus attempts were granted via negative `used` hack
   (account_estimate_usage.used = -35), which made /quota return
   {limit:15, used:-35, remaining:50} -> frontend rendered the absurd
   "Осталось 50 из 15". Replaced with a proper personal monthly limit:
   account_quota_overrides table (migration 184) + account_quota.user_limit()
   used across get_status/check_and_raise/increment instead of the hardcoded
   global MONTHLY_LIMIT. get_status now clamps remaining = max(0, limit -
   max(0, used)) so remaining can never exceed limit even if a negative
   used leaks through again. Migration seeds user2 -> monthly_limit=50 and
   resets the old hack (used<0 -> 0), so user2 now sees "50 из 50".

2. POST /estimate unconditionally called account_quota.increment() after
   estimate_quality, even when the address failed to geocode and the
   handler returned the _empty_estimate fallback (median=0, n_analogs=0,
   insufficient_data=True, HTTP 200) -- burning a paid slot for a null
   result. Now increment is skipped when result.insufficient_data is True;
   the #747 TOCTOU-safe atomic increment/429 gate is unchanged for real
   results.

bot-backend added 3 commits 2026-07-13 20:08:27 +00:00

Merge remote-tracking branch 'forgejo/main' into fix/tradein-quota-per-user-and-no-burn 52e5a32fd5

fix(tradein/quota): renumber migration 184 -> 185 (prefix collision) 0875d5b9e6

184_user_events.sql merged into main (#2515) after this branch's base,
claiming the 184 prefix first. Renumber account_quota_overrides to 185
(next free slot) and update the "migration 184" references in the SQL
header comment and account_quota.py docstrings so
test_migrations_manifest.py::test_new_files_do_not_reuse_prefix stays green.

fix(tradein/quota): fix stale 'Migration 184' header comment in 185_account_quota_overrides.sql

CI / changes (pull_request) Successful in 9s

Details

CI / backend-tests (pull_request) Has been skipped

Details

CI / frontend-tests (pull_request) Has been skipped

Details

CI Trade-In / changes (pull_request) Successful in 9s

Details

CI Trade-In / frontend-checks (pull_request) Has been skipped

Details

CI / openapi-codegen-check (pull_request) Has been skipped