fix(backoff): escalate across natural expiry so rate-limits actually back off #3

Merged

devtrev merged 1 commit from trevin/fix/backoff-escalation into dev

2026-07-09 23:48:37 -06:00

devtrev commented

2026-07-09 23:44:23 -06:00

Owner

Problem

Backoff windows never escalated — they stayed at 5s forever. The admin panel showed failure_count climbing (level 64) while Window remained 5s, causing one wasted probe against a known-bad key every 5s indefinitely. This is the exact pathology §3's exponential curve was meant to prevent.

Root cause

Design contradiction between two parts of the same feature:

record_failure (aor/backoff.py) escalated the window only when a failure arrived while still backed off (until_ts > now); a failure arriving after the window elapsed naturally reset to 5s.
select_sequence (aor/routing.py §4 step 6) filters out keys whose until_ts > now at request entry, so a key is only retried after its backoff window elapses — i.e. precisely when until_ts <= now.

Every real failure therefore hit the reset-to-5s branch; the doubling path was unreachable through normal routing. The spec (docs/requirements-sticky-backoff.md §3) explicitly codified the broken rule: "a failure that arrives after the previous window elapsed naturally resets the window to 5s (the key had its chance to recover)." That recovery assumption is false when combined with backoff filtering — natural expiry just means the router waited out the window, not that the key recovered. The only true recovery signal is a success, which already clear()s the row.

test_escalation_doubles_until_cap only passed because it manually drove failures within the window — something the router never does.

Fix

record_failure now keys escalation on row existence, not on window expiry:

row is None (first failure, or after a success cleared the row) → fresh 5s window
row exists (no success has cleared it → key has not recovered) → double the window, capped at 24h

Success still calls backoff.clear(), so the recovery path is unchanged. A transient flake that succeeds in between still resets to 5s on the next failure.

Behavior after fix

Scenario	Before	After
Persistent rate-limit, never succeeds	5s → 5s → 5s … forever	5s → 10s → 20s → … → 24h, stays at 24h
Transient flake, succeeds in between	resets to 5s on next failure	resets to 5s on next failure (unchanged)
Idle key with lingering expired row, fails much later	resets to 5s	escalates 5s → 10s (slightly aggressive, harmless — clears on next success)

No changes to routing.py, proxy.py, stats.py, admin.py, or templates — none depend on the reset semantics; they only read window_s/failure_count for display and use is_backed_off for filtering.

Testing

uv run ruff check . — pass
uv run ruff format --check . — pass
uv run pytest -q — 135 passed

Rewrote the two tests that encoded the buggy behavior (test_natural_expiry_resets_window_to_initial, test_record_failure_at_exact_expiry_resets_to_initial) to assert escalation continues across natural and exact expiry. test_escalation_doubles_until_cap and test_clear_drops_row_on_success pass unchanged.

Spec

Updated docs/requirements-sticky-backoff.md §3 to correct the "natural expiry resets to 5s" wording, with a note explaining why the old rule combined with §4-step-6 filtering to make escalation unreachable.

## Problem Backoff windows never escalated — they stayed at 5s forever. The admin panel showed `failure_count` climbing (level 64) while `Window` remained `5s`, causing one wasted probe against a known-bad key every 5s indefinitely. This is the exact pathology §3's exponential curve was meant to prevent. ## Root cause Design contradiction between two parts of the same feature: - **`record_failure`** (`aor/backoff.py`) escalated the window only when a failure arrived *while still backed off* (`until_ts > now`); a failure arriving after the window elapsed naturally reset to 5s. - **`select_sequence`** (`aor/routing.py` §4 step 6) filters out keys whose `until_ts > now` at request entry, so a key is only retried **after** its backoff window elapses — i.e. precisely when `until_ts <= now`. Every real failure therefore hit the reset-to-5s branch; the doubling path was unreachable through normal routing. The spec (`docs/requirements-sticky-backoff.md` §3) explicitly codified the broken rule: *"a failure that arrives after the previous window elapsed naturally resets the window to 5s (the key had its chance to recover)."* That recovery assumption is false when combined with backoff filtering — natural expiry just means the router waited out the window, not that the key recovered. The only true recovery signal is a **success**, which already `clear()`s the row. `test_escalation_doubles_until_cap` only passed because it manually drove failures *within* the window — something the router never does. ## Fix `record_failure` now keys escalation on **row existence**, not on window expiry: - `row is None` (first failure, or after a success cleared the row) → fresh 5s window - `row exists` (no success has cleared it → key has not recovered) → double the window, capped at 24h Success still calls `backoff.clear()`, so the recovery path is unchanged. A transient flake that succeeds in between still resets to 5s on the next failure. ## Behavior after fix | Scenario | Before | After | |---|---|---| | Persistent rate-limit, never succeeds | 5s → 5s → 5s … forever | 5s → 10s → 20s → … → 24h, stays at 24h | | Transient flake, succeeds in between | resets to 5s on next failure | resets to 5s on next failure (unchanged) | | Idle key with lingering expired row, fails much later | resets to 5s | escalates 5s → 10s (slightly aggressive, harmless — clears on next success) | No changes to `routing.py`, `proxy.py`, `stats.py`, `admin.py`, or templates — none depend on the reset semantics; they only read `window_s`/`failure_count` for display and use `is_backed_off` for filtering. ## Testing - `uv run ruff check .` — pass - `uv run ruff format --check .` — pass - `uv run pytest -q` — 135 passed Rewrote the two tests that encoded the buggy behavior (`test_natural_expiry_resets_window_to_initial`, `test_record_failure_at_exact_expiry_resets_to_initial`) to assert escalation continues across natural and exact expiry. `test_escalation_doubles_until_cap` and `test_clear_drops_row_on_success` pass unchanged. ## Spec Updated `docs/requirements-sticky-backoff.md` §3 to correct the "natural expiry resets to 5s" wording, with a note explaining why the old rule combined with §4-step-6 filtering to make escalation unreachable.

devtrev added 1 commit

2026-07-09 23:44:23 -06:00

fix(backoff): escalate across natural expiry so rate-limits actually back off 4dd67ddfb7

record_failure keyed its escalation decision on whether the previous
backoff window had elapsed (row['until_ts'] <= now → reset to 5s). But
the router filters out keys whose until_ts > now (select_sequence step
6), so a key is only retried *after* its window elapses — meaning every
real failure landed in the reset-to-5s branch and the doubling path was
unreachable through normal routing. A persistently rate-limited key was
probed every 5s forever (admin panel showed failure_count climbing while
window_s stayed at 5s), the exact pathology §3's curve was meant to fix.

Escalation now keys on row existence: if a row is present, no success has
cleared it → the key has not recovered → double the window (capped 24h).
Only an absent row (first failure, or after a success dropped it) starts
a fresh 5s window. Success still clears the row via backoff.clear(), so
the recovery path is unchanged.

Rewrites the two tests that encoded the buggy reset-on-expiry behavior
(test_natural_expiry_resets_window_to_initial,
test_record_failure_at_exact_expiry_resets_to_initial) to assert
escalation continues across expiry. test_escalation_doubles_until_cap
and test_clear_drops_row_on_success pass unchanged.

Updates docs/requirements-sticky-backoff.md §3 to correct the
'natural expiry resets to 5s' wording, which combined with §4-step-6
filtering to make escalation unreachable.

devtrev merged commit 10bbb1aa4e into dev

2026-07-09 23:48:37 -06:00

devtrev referenced this pull request from a commit

2026-07-09 23:48:38 -06:00

Merge pull request 'fix(backoff): escalate across natural expiry so rate-limits actually back off' (#3) from trevin/fix/backoff-escalation into dev

devtrev referenced this pull request from a commit

2026-08-20 17:00:01 -06:00

fix(modelsdev): address review findings