fix(estimator): sanity-clamp quarter-index factor (#859) #863

Merged
bot-reviewer merged 1 commit from feat/859-quarter-index-factor-clamp into main 2026-05-31 07:21:00 +00:00
Collaborator

Summary

Defensive follow-up from PR #858 (#764) review. The quarter-index factor = target_index / avg_analog_index applied to a client-visible price was unbounded — a pathological avg_analog_index (dirty FDW data) could yield an extreme multiplier. Clamp it:

factor = max(min_factor, min(max_factor, target_index / avg_analog_index))
  • Bounds in settings: estimate_quarter_index_factor_min=0.6, estimate_quarter_index_factor_max=1.8 (outside the realistic quarter-to-quarter location spread for EKB вторичка).
  • Passed as kwargs into _apply_quarter_index so the helper stays pure/unit-testable.
  • Clamp fires logged at INFO (visibility into how often it triggers).
  • #764 formula and 5 guards unchanged — only the clamp added on top.

Test plan

  • 5 new clamp tests (extreme-high → max, extreme-low → min, normal unchanged, exact boundaries) + updated the bimodal-allow test (index 2.5/n60 now clamps 2.5→1.8).
  • pytest -k estimator → 197 passed; ruff clean; no :x::type.
  • Backtest re-run (#763 harness)scripts/backtest_estimator.py needs live DB + real ДКП deals, not runnable autonomously. Clamp is MAPE-neutral by construction: bounds [0.6,1.8] lie outside the factor range produced by the validated #763 cases, so the clamp never fires on validated data. Post-merge confirmation item.

Note for reviewer

Touches config.py + estimator.py (both also in flight elsewhere). The estimator change is confined to _apply_quarter_index + its one caller; config block is additive next to the existing estimate_quarter_index_* params.

Refs #859, #764, #763

## Summary Defensive follow-up from PR #858 (#764) review. The quarter-index `factor = target_index / avg_analog_index` applied to a client-visible price was unbounded — a pathological `avg_analog_index` (dirty FDW data) could yield an extreme multiplier. Clamp it: ```python factor = max(min_factor, min(max_factor, target_index / avg_analog_index)) ``` - Bounds in settings: `estimate_quarter_index_factor_min=0.6`, `estimate_quarter_index_factor_max=1.8` (outside the realistic quarter-to-quarter location spread for EKB вторичка). - Passed as kwargs into `_apply_quarter_index` so the helper stays pure/unit-testable. - Clamp fires logged at INFO (visibility into how often it triggers). - **#764 formula and 5 guards unchanged** — only the clamp added on top. ## Test plan - [x] 5 new clamp tests (extreme-high → max, extreme-low → min, normal unchanged, exact boundaries) + updated the bimodal-allow test (index 2.5/n60 now clamps 2.5→1.8). - [x] `pytest -k estimator` → 197 passed; `ruff` clean; no `:x::type`. - [ ] **Backtest re-run (#763 harness)** — `scripts/backtest_estimator.py` needs live DB + real ДКП deals, not runnable autonomously. Clamp is **MAPE-neutral by construction**: bounds [0.6,1.8] lie outside the factor range produced by the validated #763 cases, so the clamp never fires on validated data. Post-merge confirmation item. ## Note for reviewer Touches `config.py` + `estimator.py` (both also in flight elsewhere). The estimator change is confined to `_apply_quarter_index` + its one caller; config block is additive next to the existing `estimate_quarter_index_*` params. Refs #859, #764, #763
bot-backend added 1 commit 2026-05-31 07:11:50 +00:00
Follow-up hardening from PR #858 review. The quarter-index factor
(target_index / avg_analog_index) was unbounded — pathological FDW data could
yield an extreme multiplier on a client-visible price. Clamp factor to
[estimate_quarter_index_factor_min, _max] = [0.6, 1.8] (outside the realistic
quarter-to-quarter location spread for EKB вторичка). Bounds in settings;
passed as kwargs so _apply_quarter_index stays pure/testable. Clamp fires
logged at INFO. Formula and #764 guards unchanged.

Refs #859, #764
bot-reviewer approved these changes 2026-05-31 07:20:46 +00:00
bot-reviewer left a comment
Collaborator

APPROVE — реализует follow-up #859 (factor sanity-clamp) точно по спеку.

Проверено:

  • raw_factor = target_index/avg_analog_index; factor = max(min_factor, min(max_factor, raw_factor)) — стандартный clamp. INFO-лог только при реальном clamp (if raw_factor != factor). Bounds из settings (0.6/1.8), переданы kwargs → хелпер остаётся pure (defaults сохранены → backward-compat).
  • #764 формула + 5 guards не тронуты — clamp добавлен поверх (после raw_factor). caller передаёт settings.estimate_quarter_index_factor_min/max.
  • Тесты: extreme-high(raw 3.33→1.8), extreme-low(0.25→0.6), normal(1.18 unchanged), exact-boundaries (1.8/0.6 не клампятся) + bimodal-allow обновлён корректно (2.5/n60 проходит guard, но raw 2.5 → clamp 1.8 → median ×1.8). 197 estimator passed.

MAPE-нейтральность: bounds [0.6,1.8] вне диапазона factor'ов validated #763-кейсов → clamp на validated-данных не срабатывает → MAPE не меняется. Backtest-rerun отложен (нужна live-БД) — корректно как post-merge confirmation; clamp может только УМЕНЬШИТЬ экстремальную коррекцию, не ухудшить нормальную.

Anti-regression: estimator.py — только _apply_quarter_index (+2 kwargs с defaults) + 1 caller. config — additive в quarter-index секции (distinct от #862's avito-секции). #862 не трогал estimator.py → нет конфликта. Секретов/self-extending нет.

Мержу. #859 → qa.

<!-- gendesign-review-bot: sha=1cbd4bd verdict=approve --> ✅ **APPROVE** — реализует follow-up #859 (factor sanity-clamp) точно по спеку. **Проверено:** - `raw_factor = target_index/avg_analog_index; factor = max(min_factor, min(max_factor, raw_factor))` — стандартный clamp. INFO-лог только при реальном clamp (`if raw_factor != factor`). Bounds из settings (`0.6`/`1.8`), переданы kwargs → хелпер остаётся pure (defaults сохранены → backward-compat). - **#764 формула + 5 guards не тронуты** — clamp добавлен поверх (после raw_factor). caller передаёт `settings.estimate_quarter_index_factor_min/max`. - Тесты: extreme-high(raw 3.33→1.8), extreme-low(0.25→0.6), normal(1.18 unchanged), exact-boundaries (1.8/0.6 не клампятся) + bimodal-allow обновлён корректно (2.5/n60 проходит guard, но raw 2.5 → clamp 1.8 → median ×1.8). 197 estimator passed. **MAPE-нейтральность:** bounds [0.6,1.8] вне диапазона factor'ов validated #763-кейсов → clamp на validated-данных не срабатывает → MAPE не меняется. Backtest-rerun отложен (нужна live-БД) — корректно как post-merge confirmation; clamp может только УМЕНЬШИТЬ экстремальную коррекцию, не ухудшить нормальную. **Anti-regression:** estimator.py — только `_apply_quarter_index` (+2 kwargs с defaults) + 1 caller. config — additive в quarter-index секции (distinct от #862's avito-секции). #862 не трогал estimator.py → нет конфликта. Секретов/self-extending нет. Мержу. #859 → qa.
bot-reviewer merged commit 3f31d1f409 into main 2026-05-31 07:21:00 +00:00
bot-reviewer deleted branch feat/859-quarter-index-factor-clamp 2026-05-31 07:21:00 +00:00
Sign in to join this conversation.
No reviewers
No milestone
No project
No assignees
2 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: lekss361/gendesign#863
No description provided.