refactor(runtime): extract shared LLM call layer - #8
Merged
Merged
Conversation
There was a problem hiding this comment.
Pull request overview
This PR extracts and centralizes a product-neutral LLM “physical call” runtime layer into AgentCore, including binding/overrides, streaming assembly + watchdogs, retry/backoff classification, and response/usage/model normalization, while leaving product-specific execution context decisions behind explicit callback hooks.
Changes:
- Introduces a shared runtime LLM call stack (
call_llm, streaming assembler, retries/backoffs, watchdogs, runaway handling) plus a publicllm_clientfacade. - Adds a unified error classification module (
runtime/retriable.py) and environment-prefix cascade helper (runtime/env.py). - Adds extensive regression/unit tests covering streaming metadata, stalls/TTFT, proxy-wrapped 400s, Retry-After clamping, usage extraction, tool-call edge cases, and thinking stripping.
Reviewed changes
Copilot reviewed 24 out of 24 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/test_retriable.py | Adds pure-function tests for retriable/fallback error classification predicates and labels. |
| tests/test_llm_runtime_strip_thinking.py | Tests stripping <think>...</think> artifacts from final-visible content. |
| tests/test_llm_runtime_stream_metadata.py | Tests streaming assembly invariants (content concat, usage/model/finish_reason carry-through, provider stamping, tool-call delta stitching). |
| tests/test_llm_runtime_stall.py | Tests stream stall + TTFT watchdog behavior and chain-advance threshold behavior. |
| tests/test_llm_runtime_retry_after.py | Tests Retry-After clamping and default/jittered backoff schedules. |
| tests/test_llm_runtime_proxy_wrap.py | Tests proxy-wrapped-400 transient detection and 401/403/404 behavior. |
| tests/test_llm_runtime_nameless_tool_calls.py | Tests dropping streamed tool calls missing a function name and logging the drop. |
| tests/test_llm_runtime_hooks.py | Tests error hierarchy, sticky-session binding hook, wall-deadline hook, and ContextVar thinking override behavior. |
| tests/test_llm_runtime_extract_usage.py | Tests normalization across multiple provider usage shapes + cache schema split semantics. |
| README.md | Updates repository boundary/extraction narrative to include the LLM runtime layer now in AgentCore. |
| pyproject.toml | Adds a Ruff per-file ignore for B023 in _call.py and keeps pytest asyncio auto-mode. |
| docs/llm-runtime-boundary.md | Documents the runtime boundary and the three remaining product-owned callbacks. |
| agent_core/runtime/retriable.py | Implements central error classification predicates + classify_error label helper. |
| agent_core/runtime/loop/llm_client.py | Provides a public facade that re-exports binding/call/response/stream helpers and errors. |
| agent_core/runtime/loop/_streaming.py | Implements native streaming assembly, tool-call stitching, stall/TTFT watchdogs, and reasoning guard. |
| agent_core/runtime/loop/_runaway.py | Implements reasoning-runaway detection and retry policy with env-driven tuning. |
| agent_core/runtime/loop/_response.py | Implements content/model/usage extraction helpers and thinking-block stripping. |
| agent_core/runtime/loop/_call.py | Implements call_llm with retries, backoff, deadline clamping, admission gate, streaming replay recovery, and attempt hooks. |
| agent_core/runtime/loop/_bind.py | Implements _BoundLLM and bind helpers for tools/session/temperature/max_tokens. |
| agent_core/runtime/loop/init.py | Re-exports runtime LLM loop utilities from the agent_core.runtime.loop package. |
| agent_core/runtime/llm_request_overrides.py | Adds task-local ContextVar-based request overrides for retry semantics (thinking controls). |
| agent_core/runtime/env.py | Adds shared env prefix cascade helper (first_configured) and canonical prefix list. |
| agent_core/errors.py | Adds/updates the shared error hierarchy for runtime LLM failures (stall/runaway/exhausted). |
| agent_core/init.py | Exposes AgentCoreError at the top-level public API. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+14
to
+19
| Lives in ``agent_core/infra`` rather than under a specific workflow so | ||
| ``agent_core/core/runtime/loop/llm_client.py`` and | ||
| ``workflows/heavy_mode/utils/provider_chain.py`` can converge on the same | ||
| patterns — keeping behaviour consistent for swarm sub-agents (which go | ||
| through the generic engine) and heavy_reporter (which goes through the | ||
| chain helper). |
Comment on lines
+483
to
+486
| Precedence (top wins): | ||
| ``context_length`` → ``safety_filter`` → ``model_unavailable`` → | ||
| ``auth_failure`` → ``overloaded`` → ``credit_exhausted`` → | ||
| ``stream_stall`` → ``rate_limited`` → ``transient_network`` → ``other``. |
Comment on lines
+189
to
+193
| tool-calls are stitched by ``index`` (id set-once, name/arguments | ||
| appended). Note: OpenAI streaming surfaces usage / finish_reason / model | ||
| only on the final chunk, which the client adapter does not forward into | ||
| ``StreamDelta`` — so the assembled ``LLMResponse`` carries empty | ||
| usage/finish_reason for streamed calls (the non-streaming path has them). |
Comment on lines
+322
to
+324
| raise LLMCallExhausted( | ||
| last_exc or deadline_exc, deadline_reason, | ||
| ) from deadline_exc |
| rate-limits, proxy-wrapped upstream blips. | ||
| - *Different key (or provider), no sleep* — overload, credit exhaustion. | ||
| - *Short-circuit to salvage* — input exceeded the model's context. | ||
| - *Surface immediately* — structural bugs (auth, schema, etc.). |
Addresses the PR review pass over the extracted LLM runtime. Behaviour: - ``LLMCallExhausted.last_exc`` now always agrees with ``.reason``. A deadline refusal carried whichever exception happened to fail earlier, so a chain wrapper classifying ``last_exc`` read ``rate_limited`` off a stale 429 under reason=``wall_deadline`` and would retry past the very deadline the reason announced. The superseded failure moves to a new ``prior_exc`` field, outside what classification reads. - Abandoning a retry backoff that would cross the wall deadline raises reason=``wall_deadline`` instead of breaking into the generic ``exhausted`` raise. The attempt event alongside it already reported the deadline; only the exception disagreed, leaving callers unable to tell "out of run budget" from "this key's retries are spent". - ``extract_usage`` returns the same key set on every path: the native ``LLMResponse`` branch omitted ``reasoning_tokens`` when zero while the legacy branches always emitted it, so indexing the documented shape worked on one response object and raised KeyError on the other. - ``_runaway`` no longer keeps its own copy of the env prefix order; ``runtime.env.ENV_PREFIXES`` owns it, which is why it was extracted. Documentation — the extracted modules still described their pre-extraction homes, which is precisely what this package claims not to have: - retriable: fixed paths that resolve nowhere here, restored auth to chain-advance in the summary (the module classifies it that way), and added the missing ``empty_completion`` step to the documented precedence. - streaming: the assembler docstring still claimed streamed usage / finish_reason / model are dropped, contradicting ``StreamDelta`` and the fold right below it. - ``extract_leaked_reasoning`` documented ``additional_kwargs``; it reads ``response_metadata``. - env: the prefix comment named the wrong two compatibility products. - Remaining langchain-era wording and unresolvable product symbols are described by role instead. Tests: four regressions for the reason/``last_exc`` contract, covering the pre-attempt refusal, the abandoned backoff, the logical deadline, and ordinary exhaustion (where ``last_exc`` must still be the real failure). The B023 per-file ignore stays: the fires are 21 usages, and the usual default-argument remedy would break the replay path, which needs ``_finish_attempt`` to observe the attempt fields rebound mid-iteration. The comment now records that. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Extract the complete product-neutral LLM physical-call layer shared by ApodexHarness and FrontierAgentInternal in one batch:
llm_clientfacade and shared runtime errorsProduct-specific execution context remains outside AgentCore. The remaining decisions are explicit callbacks for wall-deadline lookup, provider-chain state, and sticky-session policy.
See
docs/llm-runtime-boundary.mdfor the ownership boundary and downstream integration contract.Compatibility
The two source implementations were converged as compatible supersets:
bind_max_tokensandThinkTagSplitterpublictextand legacycontentresponse blocksAGENT_CORE_environment variables, while retaining both legacy product prefixes during migrationAgentCore has no imports from either product package.
Validation
.venv/bin/ruff check agent_core tests.venv/bin/pyright agent_core— 0 errors, 0 warnings.venv/bin/pytest -q— 267 passeduv lock --checkuv buildFollow-up
After this merges, update ApodexHarness and FrontierAgentInternal in parallel with thin callback adapters and compatibility re-exports, then delete their duplicated LLM runtime implementations.