fix(analyze): a budget-exhausted empty completion retries once at a raised, SDK-safe cap (#569, choice c) - #571
Conversation
…aised, SDK-safe cap — the deterministic length class attacks the cause instead of a same-cap coin flip (#569, choice c) The #292 decision classified every empty-completion error as same-cap-retryable — a rationale that predates #561's cause split. Since #561 the message for a length-stop empty states the DETERMINISTIC cause, and the classifier retried it at the same cap anyway: a billed coin flip against a cause the message itself names. The fix (choice c, both review seats' convergence; the refutation round's blocker folded): - is_budget_exhausted_error discriminates the class by the budget wording across ALL adapters (the openai chat + anthropic 'output budget was consumed'; gemini-thinking 'consumed the token budget'; the o-series Responses path 'reasoning consumed the budget' — the refutation's parity extension) — nothing is DEclassified, only the retry's CAP differs; - the analyze retry loop splits: the budget class retries once at BUDGET_RETRY_MAX_TOKENS = min(2x default, 21000) — raised above the default but UNDER the Anthropic non-streaming ceiling (~21,333: the SDK rejects higher with 'Streaming is required', which would have made the retry a guaranteed failure on that adapter — the refutation's blocker); every other retryable keeps the #292 same-cap re-roll. The retry log line names the cap used; the per-index decision is the production budget_retry_cap helper (testable); - the cap threads through _process_unit -> analysis_core.analyze_unit -> simple_text (None = the unchanged default, byte-identical); - the contradictory stale CONC-C2 notes unified (post-#537/#564 the rejected reply's usage rides the error — a retry is billed AND recorded). Named deferred (out of scope): context_enhancer.py's retry loop is the same class (single-shot call pins 4096) — a follow-up; the true 2x cap needs a provider-layer streaming/timeout change (the SDK ceiling).
…mini/Responses producers branch on the finish signal, and the retry loop gets a real end-to-end pin (#569 review round) The review round's two request-changes: (1) the gemini and openai-Responses empty-completion raises emitted the budget wording UNCONDITIONALLY, so the filtered/malformed class also took the raised cap on those two adapters — the stated invariant was false for 2 of 4 producers. Both producers now branch on the finish signal (MAX_TOKENS / stop_reason == "max_tokens"), mirroring the openai-chat #561 pattern: the truncated wordings carry the marker, the filtered wordings do not. Producer-side pins added in both adapter suites; the parity markers flipped with negative cases; the retryable-pin literal updated to the new wording. (2) the "end-to-end" loop test re-implemented the loop and never invoked run_analysis — deleting the wiring line left every test green. It now drives the REAL run_analysis retry pass (stubbed _run_detection + a _process_unit capture) and asserts the raised cap threads ONLY into the budget-class unit; verified must-trip: wiring broken -> FAILED, restored -> passed. Evidence: pytest tests/test_issue569_budget_retry.py tests/test_llm_google_adapter.py tests/test_llm_openai_adapter.py tests/test_retry_empty_completion.py — 107 passed; full suite — 4037 passed, 33 skipped, 0 FAIL (Docker up this run; earlier head runs with it down showed the same tests skipped).
| "the request may have been truncated (reasoning consumed the budget) " | ||
| "or filtered") | ||
| "the request was truncated — reasoning consumed the budget") | ||
| _EMPTY_OPENAI_RESPONSES_FILTERED = ("OpenAI Responses returned no usable content (status='completed'); " |
gadievron
left a comment
There was a problem hiding this comment.
Verification review — recommend-merge. (all evidence pinned to head 570ddf9; re-verify on any new head)
The fix: is_budget_exhausted_error (rate_limiter.py) discriminates the deterministic budget-exhaustion empty completions by the adapter wording; run_analysis's retry pass gives those units ONE retry at BUDGET_RETRY_MAX_TOKENS = min(2×DEFAULT, 21000) (under the Anthropic non-streaming ceiling), threaded through _process_unit → analyze_unit → simple_text. Every other retryable keeps the #292 same-cap rationale.
The review round, fixed on this branch (570ddf9):
- The gemini and OpenAI-Responses empty-completion raises emitted the budget wording UNCONDITIONALLY, so the filtered/malformed class also took the raised cap on two of four adapters. Both producers now branch on the finish signal (google:
raw_finishin the MAX_TOKENS forms; Responses:stop_reason == "max_tokens"), mirroring the openai-chat #561 pattern — the truncated wordings carry the marker, the filtered wordings do not. Producer-side pins added in both adapter suites (driving the real raises, positive and negative). - The "end-to-end" retry-loop test re-implemented the loop and never invoked
run_analysis— deleting the wiring left every test green. It now drives the REALrun_analysisretry pass (stubbed_run_detection+ a_process_unitcapture) and asserts the raised cap threads only into the budget-class unit. Must-trip verified: with the wiring line deleted the test fails; restored it passes.
Verification (run at the pinned head): targeted slices (issue569 + google/openai adapters + retry-empty-completion) — 107 passed; full suite — 4037 passed, 33 skipped, 0 FAIL; CI at 570ddf9: tests + gitleaks success. Cross-PR: the combined tree of all six open PRs passed the full suite (3912, pre-delta); no retry stacking with #575 (the raised cap is analyze-phase-only; LLR failures go to batches_failed → absence-as-retry).
Known trade-offs (documented, accepted by the review round): the raised cap is +5% over the 20000 default — efficacy-limited against deep reasoning exhaustion (an Anthropic-envelope constraint; a per-adapter ceiling would raise it for the other providers); the min() clamps non-Anthropic providers to Anthropic's ceiling.
Follow-ups (named, none blocking):
- The Responses empty-content marker gate could key on the raw
incomplete_details.reason == "max_output_tokens"instead of the derivedstop_reason— the unknown-incomplete/unknown-status relabels also setmax_tokens, so those exotic shapes carry the budget wording as a false causal statement (retry direction safe; message honesty only). - The same deterministic budget class remains same-cap in verify/enhance and LLR (scoped out; the cheap LLR composition is to split the batch when
_attempt's exception arm seesis_budget_exhausted_error). - The resume path re-runs budget-error checkpoint rows at the default cap before the retry pass (pre-existing shape).
test_retry_empty_completion.py's_EMPTY_GEMINIliteral is stale vs the current producer wordings (non-load-bearing — retryability rides on the shared substrings);_EMPTY_OPENAI_RESPONSES_FILTEREDis defined but not asserted.
Approve + merge are code-owner actions; never on red CI.
Summary
The #292 decision classified every empty-completion error as same-cap-retryable — a rationale that predates #561's cause split. Since #561 the message for a length-stop empty states the deterministic cause ("the output budget was consumed before any visible content"), and the classifier retried it at the same cap anyway: a billed coin flip against a cause the message itself names.
The fix (the choice-(c) ruling — both review seats converged):
is_budget_exhausted_errordiscriminates the class by the budget wording across all adapters (the openai chat + anthropic "output budget was consumed"; gemini-thinking "consumed the token budget"; the o-series Responses path "reasoning consumed the budget") — nothing is declassified; only the retry's cap differs;BUDGET_RETRY_MAX_TOKENS = min(2× default, 21000)— raised above the default but under the Anthropic non-streaming ceiling (~21,333; the SDK rejects higher with "Streaming is required", which would make the retry a guaranteed failure on that adapter — the adversarial review's blocker, folded); every other retryable keeps theis_retryable_errorhonours "no usable content" only on the string path, soenhancenever retries the one error class that actually occurs #292 same-cap re-roll. The retry log line names the cap used; the per-index decision is a testable production helper;_process_unit → analysis_core.analyze_unit → simple_text(None= the unchanged default, byte-identical);Named deferred:
context_enhancer.py's retry loop is the same class (its single-shot call pins 4096) — a follow-up; a true 2× cap needs a provider-layer streaming/timeout change (the SDK ceiling).Closes #569.
Test plan
12 tests: the discriminator (the length/stop/transient/dict shapes + the three adapters' parity wordings + the filtered class not budget); the cap chain exists; the SDK-ceiling pin (above the default, at/below 21000); the production
budget_retry_capdecision; the end-to-end split (a length-empty unit's retry carries the raised cap, a stop-empty unit's carries None).Verification evidence
pytest tests/test_issue569_budget_retry.py -qpytest tests/ -q