fix(codex): hold the scarcity demote until the quota actually resets - #102
fix(codex): hold the scarcity demote until the quota actually resets#102acastellana wants to merge 1 commit into
Conversation
A 429 tells the router the subscription is exhausted; a reset header tells it until when. Only the first was used, so the demote decayed after quota_429_window_s (120s by default) and the router re-probed an exhaustion it already knew about — every couple of minutes, indefinitely, paying a failed call at the head of the cascade each time. Worse, the reset header could never arrive. The observer's filter matched ratelimit|usage|quota|percent, and the vendor's own naming is x-codex-primary-* (see the used-percent header in test_codex_scarcity), so a field like x-codex-primary-reset-after-seconds contains none of those words and was dropped. Since polling the endpoint would burn the very quota it measures, observation of real traffic is the only safe signal — a header dropped there is lost for good. - codex_backend: extract the filter as quota_headers/1 and widen it to *reset* and retry-after. Deliberately still narrow: response headers carry cookies and tokens, and this map is stored and rendered on the dashboard. - sources/codex: parse a reset time off a 429 (absolute epoch or seconds-from-observation, whichever the vendor sends) and hold the demote at full while it is still ahead. Falls back to the existing 429 ramp when no such header is present, so behaviour without one is unchanged. A reset header on a healthy 200 reports when the window rolls over, not exhaustion, and is deliberately not read as a demote signal.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Important Review available on request
Reviews should be triggered manually for repositories with fewer than 10 stars. Select Trigger review above or comment ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Problem
A 429 tells the router the codex subscription is exhausted. A reset header tells it until when. Only the first was ever used.
So the demote decays after
quota_429_window_s(120s), and the router re-probes an exhaustion it already knows about — every couple of minutes, indefinitely, paying a failed call at the head of the cascade each time.Observed in production:
openai_codex/gpt-5.5 = rate_limit(429) "codex rate limited"recurring, with the cascade falling through toexhausted: model_unavailableat ~1.5s per attempt.The reset header could never arrive
_notifyfiltered response headers onratelimit|usage|quota|percent. The vendor's own naming isx-codex-primary-*— the existing scarcity test usesx-codex-primary-used-percent— so a field likex-codex-primary-reset-after-secondscontains none of those words and was dropped.That matters more here than for a normal filter. As the module docstring says, polling the codex endpoint would burn the very quota it measures, so observing real traffic is the only safe signal. A header dropped there is lost for good — no later call can recover it.
Change
codex_backend: extract the filter asquota_headers/1(a testable seam — it was a closure) and widen it to*reset*andretry-after. Deliberately still narrow: response headers carry cookies and tokens, and this map is stored inobserved_headersand rendered on the dashboard.sources/codex: parse a reset time off a 429 — absolute epoch or seconds-from-observation, whichever the vendor sends — and hold the demote at full while it is still ahead.Falls back to the existing 429 ramp when no such header is present, so behaviour without one is unchanged. A reset header on a healthy
200reports when the window rolls over, not exhaustion, and is deliberately not read as a demote signal.Why parse both shapes
I could not confirm the vendor's exact field from outside the router (the dashboard is behind SSO, and
observed_headersis where it would show). Rather than guess one shape, the parser accepts either and yieldsNoneon anything it cannot read — falling back to today's behaviour. Onceobserved_headersshows the real field, it will already be captured and handled.Tests
TDD. The two reset tests were observed failing with
assert 1.6666666666666665 == 5.0— the single-429 ramp, i.e. the header ignored. The header-filter test failed withmodule 'codex_backend' has no attribute 'quota_headers'.Two of the four new scarcity tests passed before the change by design — they pin the behaviour that must not move (an elapsed reset, and a reset on a healthy 200).
test_codex,test_codex_scarcity,test_codex_observability,test_sources,test_codex_broker,test_codex_auth_store): 150 passed, 35 skippedtest_policy_templates.pyandtest_skill_md.pyfail identically with and without this branch (verified by stash — 13 failed / 8 passed either way).