Skip to content

fix(llm): unify empty completion recovery and provider signals - #70

Merged
zhanghanduo merged 6 commits into
mainfrom
codex/unify-empty-completion-recovery
Oct 7, 2026
Merged

zhanghanduo merged 6 commits into
mainfrom
codex/unify-empty-completion-recovery

Conversation

@zhanghanduo

@zhanghanduo zhanghanduo commented Oct 7, 2026 •

Copy link
Copy Markdown
Collaborator

Blank responses after PR #68 still recover differently in chat and streaming modes: retries can reset the logical deadline, native fallback chains cannot receive the assembler's late error, and rejected blanks are recorded as accepted attempts. Synthetic usage and dropped refusal fields can also hide or misclassify the response.

This change makes call_llm own blank recovery in both modes. Same-leg resamples, native chain advances and attempt accounting share one logical deadline; per-call cursors preserve bindings, middleware and concurrency isolation. Provider adapters retain usage provenance and OpenAI refusal signals. Direct fallback streams buffer candidate-empty fragments so discarded tool arguments cannot contaminate the next leg. Failed tool-argument replays get independent attempt identities while the original response remains deliverable. Empty refusal placeholders are evaluated against text, tool calls and both reasoning channels; streaming inference waits for normal EOF even when finish_reason is omitted, without changing error or cancellation semantics.

Compatibility details are documented in docs/llm-runtime-boundary.md and a breaking release fragment:

  • empty_completion_max_retries applies to both transports independently of the generic retry allowance, per serving leg; matching native fallback triggers determine advancement.
  • Exhausted blanks stop as empty_completion; explicit refusals and filters stop as refusal / content_filter, including under a nudge policy. Accompanying tools are not executed and history remains replayable.
  • Empty attempts are discarded/failed rather than accepted. Wrappers producing synthetic usage should retain estimated: true or set usage_source="estimated". Unmarked legacy usage remains conservative.
  • Wrappers opting into delayed-assembly fallback explicitly implement for_logical_call and advance_empty_completion; dynamic attribute forwarding alone never removes the wrapper.

Validation:

  • Full suite: 2,284 passed, 1 skipped; 163 regression cases added by this PR.
  • Real OpenAI SDK HTTP mocks cover Chat/Responses refusals, filters, missing usage and gateway estimate flags in both modes; existing Anthropic SDK coverage remains green.
  • Recovery regressions cover budgets, deadlines, mixed failures, barriers, nested and concurrent chains, middleware callbacks, fragmented tool calls and successful/failed argument replays.
  • Ruff, full Pyright, unconsumed-field checks, committed release-fragment validation and offline wheel/sdist build pass.

Middleware consumer corrections: reported usage is distinguished from estimates before cost/budget/event/aggregation effects; tracing retains estimate provenance. Canonical cache read/write fields preserve zeros and cache-only reports. Rate buckets correct zero estimates and reported zero usage against per-instance capped reservations, with legacy-context compatibility and no refund for unknown/estimated usage. Proxy and runtime share indexed tool assembly so real loop detection sees native streamed calls; failed/consumer-closed proposals stay out of history while authentic usage is still accounted. These behaviors are covered by 50 additional real-consumer integration cases using CostSink, BudgetState, usage aggregation and quota buckets. Streaming billing/budget changes and downstream recalibration are documented in the existing breaking fragment.

Verified against current main's stream-terminator guards: protocol validation precedes EOF refusal inference, and truncated Chat/Responses refusal streams still raise rather than becoming accepted declines. Combined full suite: 2,284 passed, 1 skipped.

zhanghanduo and others added 4 commits October 7, 2026 10:10
Some OpenAI-compatible gateways send `refusal: ""` alongside ordinary
content or tool calls. Treating any non-None refusal as a decline made
agent_loop end the run as `refusal` and drop the tool calls. An empty
refusal now counts only when the turn carries nothing else; the streaming
path holds that marker until the finishing chunk.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
LLMProxy.stream handed after_llm an LLMResponse carrying only text and
reasoning, so streamed calls looked usage-free to RateLimitMiddleware
and TokenAccountingMiddleware, and refusal-aware middleware never saw
stop_details. Fold usage, usage_source, finish_reason, model, provider,
refusal, stop_details and stop_reason the same way the loop's stream
assembler does. Transport heartbeats are still ignored.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@zhanghanduo

Copy link
Copy Markdown
Collaborator Author

Review follow-ups: two fixes pushed to this branch

381d821 fix(openai_chat): empty refusal beside real output is not a refusal

Found in review: an OpenAI-compatible gateway can send "refusal": "" next to ordinary content or tool calls. Both the streaming and non-streaming paths checked refusal is not None, so such a turn got stop_details={"type": "refusal"}. agent_loop then dropped the tool calls and stopped the run as refusal on the first turn.

  • An empty refusal now counts only when the turn carries nothing else. That keeps the existing content: None, refusal: "" → refusal expectation.
  • Regression test: test_chat_empty_refusal_beside_content_is_not_a_refusal (streaming and non-streaming). It fails on the previous code.

099b3ca then corrected the streaming side of this. My version decided at the finish_reason chunk, which misfires when output arrives after that chunk. The decision now waits for a clean EOF, also counts reasoning as output, and is never inferred on error, close or cancel.

c4bb872 fix(proxy): pass streamed terminal metadata to after_llm

LLMProxy.stream built the after_llm response from content and reasoning_content only. Effects on streamed calls:

  • RateLimitMiddleware never saw real usage, so its token bucket was never corrected.
  • TokenAccountingMiddleware saw total == 0 and skipped the call, so streamed token and cost usage was never recorded.
  • Refusal-aware middleware never saw stop_details / refusal.

_StreamTerminal now folds usage, usage_source, finish_reason, model, provider, refusal, stop_details and stop_reason using the same rules as the loop's stream assembler (_streaming._assembled_response). It resets on each retry attempt and still ignores transport heartbeats. Test: test_stream_after_llm_sees_terminal_metadata.

⚠️ Behavior change: after merge, token accounting starts counting streamed calls, so reported usage and cost will go up. This is a correction, but downstream budgets or alerts tuned to the old numbers may need a look. There is no changes/ fragment for this yet.

Not covered: streamed tool_calls are still not passed to after_llm. Assembling them by index is a larger change, and no middleware relies on them today.

Verification: full suite 2192 passed, 2 skipped; ruff check clean.

🤖 Generated with Claude Code

@zhanghanduo
zhanghanduo merged commit 4312db6 into main Oct 7, 2026
5 checks passed
@zhanghanduo
zhanghanduo deleted the codex/unify-empty-completion-recovery branch October 7, 2026 06:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant