From 8a1bced2cfcc73d449c6037761241181d0a5c911 Mon Sep 17 00:00:00 2001 From: aaron Date: Sun, 18 Oct 2026 16:21:40 -0700 Subject: [PATCH 1/2] Implement the medium-priority backlog items 13, 15, 16, 17, 18 - Resume: unanswered tool calls in a loaded transcript run before the first model call; an invalid transcript raises TranscriptError up front. - ToolSpec.parallel_safe: a turn calling an unsafe tool runs sequentially. - Settings.timeout_seconds / LLM_TIMEOUT_SECONDS, passed to every provider builder; an explicit timeout beats a supplied http_client's. - OpenAI model-aware quirks: o-series models do not get parallel_tool_calls. - Tracing: warn on a second configure_tracing() and on a traced run with no provider installed. Item 14 needs a live call and stays open. Co-Authored-By: Claude Sonnet 5.5 --- BACKLOG.md | 94 ++++++----------------- CHANGELOG.md | 16 ++++ README.md | 1 + docs/api/config.md | 1 + docs/api/loop.md | 4 + docs/api/tooling.md | 1 + docs/getting-started/configuration.md | 1 + docs/guides/tracing.md | 4 + src/agentloop/agent.py | 3 + src/agentloop/config.py | 11 +++ src/agentloop/loop.py | 18 +++++ src/agentloop/providers/_openai_compat.py | 18 ++++- src/agentloop/providers/modal.py | 6 +- src/agentloop/providers/openai.py | 26 ++++++- src/agentloop/providers/openrouter.py | 6 +- src/agentloop/tooling.py | 15 +++- src/agentloop/tracing.py | 38 +++++++++ tests/test_config.py | 37 +++++++++ tests/test_loop.py | 34 ++++++++ tests/test_openai_compat.py | 12 +++ tests/test_tooling.py | 35 +++++++++ tests/test_tracing.py | 43 +++++++++++ 22 files changed, 343 insertions(+), 81 deletions(-) diff --git a/BACKLOG.md b/BACKLOG.md index 135a684..4e49986 100644 --- a/BACKLOG.md +++ b/BACKLOG.md @@ -74,89 +74,43 @@ Left over: ## Medium priority -### 13. Resuming a transcript that ends mid-turn sends an invalid request +Items 13, 15, 16, 17 and 18 landed on `feature/medium-priority` (PR pending). Item 14 needs a +live call and is still open. Left over: -**Status:** gap. `Agent.converse` / `run_conversation` send `transcript.messages` as they -are and never call `check_invariants()`. `from_jsonl` deliberately loads mid-turn logs, so -this is a supported state. +### 13. Resuming a transcript that ends mid-turn: the paused case -**Concrete triggers:** -- The process dies while tools run, and the saved transcript ends with an assistant message - whose tool calls have no results. Resuming sends unanswered calls, every provider rejects - them with a 400, and that `BadRequestError` is not retried. -- A turn paused (`StopReason.PAUSED`) and then hit a provider error. The new `_Run` starts - with `continuing=False`, so the continuation becomes a second consecutive assistant - message instead of extending the first. - -**What implementing it takes:** a preflight in `drive()`. Pending tool calls should be -either executed before the first model call (a natural resume) or rejected with a -`TranscriptError` that says how to fix it. That is an API decision. For the paused case, -`Message` would have to record that the turn is open, for example in `Origin`. +Unanswered tool calls are now run before the first model call, and `check_invariants()` runs +once up front. Still open: a turn that paused (`StopReason.PAUSED`) and then hit a provider +error. The new `_Run` starts with `continuing=False`, so the continuation becomes a second +consecutive assistant message instead of extending the first. `Message` would have to record +that the turn is open, for example in `Origin`. ### 14. Verify Responses reasoning replay without `include: ["reasoning.encrypted_content"]` -**Status:** needs a live check. The docstring in `_openai_responses.py` says the encrypted -reasoning content "`store: false` returns by default". OpenAI's API reference documents it -as an opt-in `include` value for stateless use. If it is opt-in, reasoning items are -replayed without their encrypted content. Reasoning models then lose their chain of -thought across tool calls, or the request is rejected. +**Status:** needs a live check; not done. The docstring in `_openai_responses.py` says the +encrypted reasoning content "`store: false` returns by default". OpenAI's API reference +documents it as an opt-in `include` value for stateless use. If it is opt-in, reasoning items +are replayed without their encrypted content. Reasoning models then lose their chain of thought +across tool calls, or the request is rejected. **What implementing it takes:** run one tool-using turn on a reasoning model live and check `output[*].encrypted_content`. If it is missing, add `"reasoning.encrypted_content"` to `include` whenever the model may reason, and save the captured body as a fixture (item 21). -### 15. No way to mark a tool as unsafe to run in parallel - -**Status:** not implemented. `run_tools` always runs a turn with two or more tool calls in -parallel, on a fixed `ThreadPoolExecutor(max_workers=8)` (`tooling.py`). There is no -`ToolSpec` flag for a handler that isn't thread-safe: a shared DB cursor, a non-atomic -shared counter, an SDK client not documented as thread-safe. - -**Concrete trigger:** two tools that both mutate the same plain `list` or -`sqlite3.Connection` without a lock, called by the model in one turn. Corruption or -exceptions show up only intermittently, when a model happens to call them together. - -**What implementing it takes:** a `ToolSpec.parallel_safe: bool = True` (or similar) that -`run_tools` checks. Any turn that contains an unsafe call runs sequentially. - -### 16. No per-provider timeout override through `Settings` / `build_client()` - -**Status:** not implemented. Timeouts are hard-coded per adapter (`anthropic.py`'s -`DEFAULT_TIMEOUT_SECONDS`, the `TIMEOUT_SECONDS` dicts in `openrouter.py`, `modal.py` and -`openai.py`). Someone who needs a longer read timeout must call `build__client()` -directly and give up `Settings.from_env()`. Item 2's `http_client` does not cover the -Anthropic path, and its timeout heuristic is fragile (item 23). - -**What implementing it takes:** an optional `timeout_seconds` (or a richer shape) on -`Settings`, passed through each `_build_*` helper in `config.py` to the provider builder's -`timeout=`. - -### 17. Model-aware provider `Quirks` - -**Status:** not implemented. `Quirks` is chosen once per *provider*, not per *model*. -`providers/openai.py` sends `parallel_tool_calls` on every request. OpenAI's reasoning -models reject that parameter and its non-reasoning chat models accept it, and the only way -around it is two hand-built clients. - -**What implementing it takes:** a second selection axis, probably a small `model -> Quirks` -override table (regex or prefix match) that `build_openai_client(model=...)` checks before -falling back to the provider default. It needs real design work: what to match on, what -happens when nothing matches, and how Modal's provider-wide quirks interact. - -### 18. Tracing setup fails silently (`configure_tracing()` twice; `trace=True` without it) +### 17. Model-aware provider `Quirks`: what is left -**Status:** not implemented. Two symptoms with one cause: +`providers/openai.py` now picks `Quirks` by model-id prefix (`MODEL_QUIRKS`, `quirks_for_model`), +and `build_openai_client(quirks=...)` overrides it. The table holds only the o-series +(`o1`, `o3`, `o4`). Not done: other OpenAI families that reject parameters (add rows as they +are confirmed live), and OpenRouter and Modal, which stay provider-wide because their models +vary too much for one table. -- OpenTelemetry's global `trace.set_tracer_provider()` takes effect only once per process. - A second `configure_tracing()` returns a provider that looks live but receives no spans. - The real first provider may then never be flushed. -- `Agent(trace=True)` without `configure_tracing()` throws every span away, and nothing - tells the user. +### 18. Tracing setup warnings: what is left -**What implementing it takes:** detect reliably whether the global provider is still the -default proxy or no-op one. Then warn (or raise) on a second install, and log one warning -the first time `OTelObserver.conversation()` runs with no provider installed. The -detection has to be tested across the OpenTelemetry SDK versions the package supports. +A second `configure_tracing()` and a traced run with no provider now log a warning. It warns +rather than raises, and the check uses `ProxyTracerProvider` / `NoOpTracerProvider` from the +OpenTelemetry API. It is untested against older SDK versions than the one in the dev +environment. --- diff --git a/CHANGELOG.md b/CHANGELOG.md index 7721589..bd43e49 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -42,7 +42,23 @@ while `0.x`, a minor bump may break the public API (`agentloop.__all__`). - `OTelObserver(redact=..., allow_unredacted=...)`, with the event trace's rules. - `ConversationStatus.TOKEN_LIMIT` and `LoopBudgetExceeded(kind="max_tokens")`. +- `ToolSpec.parallel_safe` (and `@tool(parallel_safe=...)`): `False` makes a turn that calls + the tool run all its calls sequentially, in call order. +- `Settings.timeout_seconds` / `LLM_TIMEOUT_SECONDS`, and `timeout_seconds=` on + `build_openai_client`, `build_openrouter_client` and `build_modal_client`: replaces the read + timeout of the OpenAI-compatible providers (Anthropic: the SDK's overall timeout). An explicit + timeout now wins over a supplied `http_client`'s, so the "is it 5 seconds?" guess only applies + when none was asked for. +- `build_openai_client(quirks=...)` and `providers.openai.MODEL_QUIRKS` / `quirks_for_model`: the + o-series reasoning models no longer receive `parallel_tool_calls`, which they reject. + ### Changed +- Resuming a transcript that ends in unanswered tool calls (the process died while tools ran) + runs those calls before the first model call instead of sending an invalid request. Any other + invalid transcript raises `TranscriptError` before a model call, not a provider 400. +- `configure_tracing()` logs a warning when a provider is already installed (it cannot replace + it, so the one it returns receives no spans), and a traced conversation logs one warning when + no provider is installed at all. - `OTelObserver(capture_content=True)` now needs `redact` or `allow_unredacted=True`, else it raises `ConfigurationError`. Captured text is redacted, then clipped; a redactor that raises drops the attribute. Exceptions are recorded as an `exception` event with a redacted message diff --git a/README.md b/README.md index 5c6ec21..45fef6f 100644 --- a/README.md +++ b/README.md @@ -287,6 +287,7 @@ and the rest of the package. |---|---|---| | `LLM_PROVIDER` | `anthropic` | `anthropic`, `openai`, `openrouter`, or `modal` | | `LLM_MAX_ATTEMPTS` | `3` | Total attempts, including the first request | +| `LLM_TIMEOUT_SECONDS` | provider default | Read timeout for one model call (Anthropic: the SDK's overall timeout) | | `ANTHROPIC_API_KEY` | unset | Anthropic credential | | `ANTHROPIC_MODEL` | `claude-opus-5` | Anthropic model | | `OPENAI_API_KEY` | unset | OpenAI credential | diff --git a/docs/api/config.md b/docs/api/config.md index a27315c..3573084 100644 --- a/docs/api/config.md +++ b/docs/api/config.md @@ -25,6 +25,7 @@ A frozen Pydantic model. Unknown fields are rejected. | `openai_model` | `str` | `"gpt-5.1"` | | `openai_api` | `"responses" | "completions"` | `"responses"` | | `max_attempts` | `int >= 1` | `3` | +| `timeout_seconds` | `float > 0 \| None` | `None` (provider default) | ### `Settings.model` diff --git a/docs/api/loop.md b/docs/api/loop.md index 272f554..3605235 100644 --- a/docs/api/loop.md +++ b/docs/api/loop.md @@ -42,6 +42,10 @@ A provider that pauses a turn (`StopReason.PAUSED`, for example Anthropic's `pau a hosted tool runs) is continued without sending a user message: the continuation joins the same assistant message, and each continuation counts as an iteration. +If `transcript` ends in tool calls with no results (a log cut off while tools ran), the loop +runs them before the first model call and then validates the transcript with +`check_invariants()`, so a bad one raises `TranscriptError` instead of a provider 400. + The deadline is checked between turns. A provider call, its retries, or a started tool batch may finish after it. `None` disables the deadline. Exhaustion raises `LoopBudgetExceeded`; provider errors propagate unchanged. diff --git a/docs/api/tooling.md b/docs/api/tooling.md index 4cf18d9..3e5da9d 100644 --- a/docs/api/tooling.md +++ b/docs/api/tooling.md @@ -33,6 +33,7 @@ A frozen Pydantic model containing the model-visible definition and handler. | `description` | non-empty `str` | Model-visible instructions | | `parameters` | `dict[str, Any]` | Bare handwritten JSON Schema | | `handler` | `Handler` | Callable whose first argument is the deps bundle | +| `parallel_safe` | `bool` | `True`. Set `False` for a handler that is not thread-safe: a turn that calls it runs all of its tool calls one after another, in call order | | `timeout_seconds` | `float \| None` | Per-call limit; `None` waits forever. On expiry the model gets an error result and the handler thread is abandoned (Python cannot kill it), so the handler should still set its own network timeouts | ```python diff --git a/docs/getting-started/configuration.md b/docs/getting-started/configuration.md index 3c6df8b..68225fd 100644 --- a/docs/getting-started/configuration.md +++ b/docs/getting-started/configuration.md @@ -22,6 +22,7 @@ finally: |---|---|---| | `LLM_PROVIDER` | `anthropic` | `anthropic`, `openai`, `openrouter`, or `modal` | | `LLM_MAX_ATTEMPTS` | `3` | Total attempts, including the first request | +| `LLM_TIMEOUT_SECONDS` | provider default | Read timeout for one model call (Anthropic: the SDK's overall timeout) | | `ANTHROPIC_API_KEY` | unset | Anthropic credential | | `ANTHROPIC_MODEL` | `claude-opus-5` | Anthropic model | | `OPENAI_API_KEY` | unset | OpenAI credential | diff --git a/docs/guides/tracing.md b/docs/guides/tracing.md index 39348a9..8ddfafe 100644 --- a/docs/guides/tracing.md +++ b/docs/guides/tracing.md @@ -33,6 +33,10 @@ Tracing creates one top-level span for the conversation, one child span for each request, and one span for each local tool call. A span is one timed operation in an OpenTelemetry trace. Retries appear as events on the related model-request span. +Tracing needs a provider installed first. A traced conversation with none logs one warning +and discards its spans; calling `configure_tracing()` a second time logs a warning too, because +OpenTelemetry keeps the first provider and the returned one receives nothing. + Prompts, tool arguments, tool results, and provider error messages are not captured by default. Construct an `OTelObserver(..., capture_content=True, redact=scrub)` (a redactor is required, as for the event diff --git a/src/agentloop/agent.py b/src/agentloop/agent.py index 8b80593..a766b34 100644 --- a/src/agentloop/agent.py +++ b/src/agentloop/agent.py @@ -75,6 +75,7 @@ def tool( name: str | None = None, description: str | None = None, timeout_seconds: float | None = None, + parallel_safe: bool = True, ) -> Callable[[Callable[..., Any]], ToolSpec]: """Turn a plain function into a ToolSpec. @@ -88,6 +89,7 @@ def tool( `timeout_seconds` bounds one call: past it the model is told the tool timed out and the loop moves on. The handler's thread is abandoned, not killed (see `ToolSpec`). + `parallel_safe=False` makes any turn that calls this tool run its calls sequentially. """ def decorate(fn: Callable[..., Any]) -> ToolSpec: @@ -104,6 +106,7 @@ def decorate(fn: Callable[..., Any]) -> ToolSpec: parameters=parameters, handler=_make_handler(fn), timeout_seconds=timeout_seconds, + parallel_safe=parallel_safe, ) return decorate diff --git a/src/agentloop/config.py b/src/agentloop/config.py index f3aa4ea..9e77904 100644 --- a/src/agentloop/config.py +++ b/src/agentloop/config.py @@ -62,6 +62,11 @@ class Settings(BaseModel): # every provider -- the Anthropic SDK's own retries are switched off. max_attempts: int = Field(default=3, ge=1) + # Seconds to wait on one model call. None keeps each provider's own default. For the + # OpenAI-compatible providers it replaces the READ timeout (connect, write and pool + # keep theirs); for Anthropic it is the SDK's overall request timeout. + timeout_seconds: float | None = Field(default=None, gt=0) + @field_validator( "anthropic_api_key", "openrouter_api_key", @@ -121,6 +126,7 @@ def from_env(cls, env: Mapping[str, str]) -> Settings: # The raw string: pydantic coerces it, and rejects "three" with a # ValidationError that names the field, where int() would not. max_attempts=env.get("LLM_MAX_ATTEMPTS", "3"), # type: ignore[arg-type] + timeout_seconds=env.get("LLM_TIMEOUT_SECONDS") or None, # type: ignore[arg-type] ) @@ -163,7 +169,9 @@ def _build_anthropic(settings: Settings, http_client: Any) -> LLMClient: from agentloop.providers.anthropic import AnthropicClient # lazy: keeps the extra optional key = settings.anthropic_api_key + timeout = {} if settings.timeout_seconds is None else {"timeout": settings.timeout_seconds} return AnthropicClient( + **timeout, # None lets the SDK resolve credentials itself. api_key=key.get_secret_value() if key else None, model=settings.anthropic_model, @@ -180,6 +188,7 @@ def _build_openrouter(settings: Settings, http_client: Any) -> LLMClient: api_key=settings.openrouter_api_key.get_secret_value(), model=settings.openrouter_model, http_client=http_client, + timeout_seconds=settings.timeout_seconds, ) @@ -203,6 +212,7 @@ def _build_modal(settings: Settings, http_client: Any) -> LLMClient: proxy_token_id=token_id.get_secret_value() if token_id else "", proxy_token_secret=token_secret.get_secret_value() if token_secret else "", http_client=http_client, + timeout_seconds=settings.timeout_seconds, ) @@ -216,4 +226,5 @@ def _build_openai(settings: Settings, http_client: Any) -> LLMClient: model=settings.openai_model, api=settings.openai_api, http_client=http_client, + timeout_seconds=settings.timeout_seconds, ) diff --git a/src/agentloop/loop.py b/src/agentloop/loop.py index 6882c32..0e5dc41 100644 --- a/src/agentloop/loop.py +++ b/src/agentloop/loop.py @@ -361,9 +361,27 @@ def _add_turn( ) ) + def _resume_pending_calls(self) -> None: + """Answer tool calls a loaded transcript left unanswered, before any model call. + + `from_jsonl` deliberately loads a log cut off mid-turn (the process died while + tools ran). Sending that as it stands is a 400 on every provider, which is not + retried. Running the calls is the natural resume; anything else wrong with the + transcript is raised here, with the fix, rather than as an opaque provider error. + """ + pending = self.transcript.pending_tool_calls() + if pending: + logger.warning("resuming: running %d tool call(s) left unanswered", len(pending)) + started = time.monotonic() + results = run_tools(pending, self.registry, self.deps, self.watcher) + self.tool_seconds += time.monotonic() - started + self.transcript.add_tool_results(results) + self.transcript.check_invariants() + def drive(self) -> ConversationStatus: """Loop until the model stops asking for tools; raise if it will not.""" transcript, watcher = self.transcript, self.watcher + self._resume_pending_calls() for _ in range(self.max_iterations): elapsed = time.monotonic() - self.started diff --git a/src/agentloop/providers/_openai_compat.py b/src/agentloop/providers/_openai_compat.py index 859592f..6a645a4 100644 --- a/src/agentloop/providers/_openai_compat.py +++ b/src/agentloop/providers/_openai_compat.py @@ -97,6 +97,18 @@ def httpx_timeout(**kwargs: float) -> httpx.Timeout: return _httpx().Timeout(**kwargs) # type: ignore[no-any-return] +def timeout_with_read(defaults: Mapping[str, float], read_seconds: float | None) -> httpx.Timeout: + """`defaults` as an `httpx.Timeout`, with the read timeout replaced when one is given. + + Read is the one that matters for a model: the gap between bytes of a response that + is still being generated. Connect, write and pool keep the provider's own values. + """ + values = dict(defaults) + if read_seconds is not None: + values["read"] = read_seconds + return httpx_timeout(**values) + + class Quirks(BaseModel): """Behavioural differences between OpenAI-compatible servers. @@ -558,12 +570,14 @@ def __init__( timeout: httpx.Timeout, api: Api = "completions", http_client: httpx.Client | None = None, + timeout_explicit: bool = False, ) -> None: """`http_client` is the escape hatch for trust, proxy and transport: build an `httpx.Client` with the `verify`, `proxy` or `transport` you need and pass it in. It stays yours to close. The URL and auth headers are sent per request, so the client needs no `base_url` or default headers; `timeout` is applied per request - too, unless the client was given a timeout of its own. + too, unless the client was given a timeout of its own -- or `timeout_explicit` says + the caller asked for this `timeout`, which then wins over the client's. """ self.name = name self.model = model @@ -588,7 +602,7 @@ def __init__( self._http = http_client # httpx's own default means "the caller did not choose one". chose_own = http_client.timeout != self._httpx.Timeout(5.0) - self._timeout = None if chose_own else timeout + self._timeout = None if chose_own and not timeout_explicit else timeout @property def api(self) -> Api: diff --git a/src/agentloop/providers/modal.py b/src/agentloop/providers/modal.py index 0cbb2e9..671cb55 100644 --- a/src/agentloop/providers/modal.py +++ b/src/agentloop/providers/modal.py @@ -7,7 +7,7 @@ if TYPE_CHECKING: # pragma: no cover - type checkers only, never imported at runtime import httpx -from agentloop.providers._openai_compat import OpenAICompatClient, Quirks, httpx_timeout +from agentloop.providers._openai_compat import OpenAICompatClient, Quirks, timeout_with_read DEFAULT_MODEL = "Qwen/Qwen3.6-35B-A3B-FP8" # Cold starts spin up a container and load weights: minutes, not seconds. @@ -27,6 +27,7 @@ def build_modal_client( proxy_token_id: str = "", proxy_token_secret: str = "", http_client: httpx.Client | None = None, + timeout_seconds: float | None = None, ) -> OpenAICompatClient: """Proxy tokens are both-or-neither; empty means an --unauthenticated endpoint.""" headers: dict[str, str] = {} @@ -38,6 +39,7 @@ def build_modal_client( base_url=f"{endpoint_url.rstrip('/')}/v1", headers=headers, quirks=QUIRKS, - timeout=httpx_timeout(**TIMEOUT_SECONDS), + timeout=timeout_with_read(TIMEOUT_SECONDS, timeout_seconds), + timeout_explicit=timeout_seconds is not None, http_client=http_client, ) diff --git a/src/agentloop/providers/openai.py b/src/agentloop/providers/openai.py index 44df073..1ef188a 100644 --- a/src/agentloop/providers/openai.py +++ b/src/agentloop/providers/openai.py @@ -17,7 +17,7 @@ Api, OpenAICompatClient, Quirks, - httpx_timeout, + timeout_with_read, ) BASE_URL = "https://api.openai.com/v1" @@ -29,6 +29,23 @@ # default. (Responses always sends `max_output_tokens`.) QUIRKS = Quirks(max_tokens_field="max_completion_tokens") +# OpenAI's o-series reasoning models reject `parallel_tool_calls`; its other chat models +# accept it. Matched by model-id prefix, longest first, so a family can be overridden by a +# more specific entry; no match falls back to QUIRKS. Modal and OpenRouter stay +# provider-wide: their models vary too much for one table to be right. +_NO_PARALLEL = QUIRKS.model_copy(update={"supports_parallel_tool_calls_param": False}) +MODEL_QUIRKS: tuple[tuple[str, Quirks], ...] = ( + ("o1", _NO_PARALLEL), + ("o3", _NO_PARALLEL), + ("o4", _NO_PARALLEL), +) + + +def quirks_for_model(model: str) -> Quirks: + """The quirks for `model`: the longest matching `MODEL_QUIRKS` prefix, else `QUIRKS`.""" + matches = [(prefix, q) for prefix, q in MODEL_QUIRKS if model.startswith(prefix)] + return max(matches, key=lambda m: len(m[0]))[1] if matches else QUIRKS + def build_openai_client( *, @@ -37,7 +54,9 @@ def build_openai_client( organization: str | None = None, project: str | None = None, api: Api = "responses", + quirks: Quirks | None = None, http_client: httpx.Client | None = None, + timeout_seconds: float | None = None, ) -> OpenAICompatClient: headers = {"Authorization": f"Bearer {api_key}"} if organization: @@ -49,8 +68,9 @@ def build_openai_client( model=model, base_url=BASE_URL, headers=headers, - quirks=QUIRKS, - timeout=httpx_timeout(**TIMEOUT_SECONDS), + quirks=quirks if quirks is not None else quirks_for_model(model), + timeout=timeout_with_read(TIMEOUT_SECONDS, timeout_seconds), + timeout_explicit=timeout_seconds is not None, http_client=http_client, api=api, ) diff --git a/src/agentloop/providers/openrouter.py b/src/agentloop/providers/openrouter.py index 9c8cd7c..7940bc6 100644 --- a/src/agentloop/providers/openrouter.py +++ b/src/agentloop/providers/openrouter.py @@ -7,7 +7,7 @@ if TYPE_CHECKING: # pragma: no cover - type checkers only, never imported at runtime import httpx -from agentloop.providers._openai_compat import OpenAICompatClient, Quirks, httpx_timeout +from agentloop.providers._openai_compat import OpenAICompatClient, Quirks, timeout_with_read BASE_URL = "https://openrouter.ai/api/v1" # The `openrouter/free` router spreads across free models and auto-filters for @@ -23,6 +23,7 @@ def build_openrouter_client( app_url: str | None = None, app_name: str | None = None, http_client: httpx.Client | None = None, + timeout_seconds: float | None = None, ) -> OpenAICompatClient: headers = {"Authorization": f"Bearer {api_key}"} if app_url: @@ -35,6 +36,7 @@ def build_openrouter_client( base_url=BASE_URL, headers=headers, quirks=Quirks(), - timeout=httpx_timeout(**TIMEOUT_SECONDS), + timeout=timeout_with_read(TIMEOUT_SECONDS, timeout_seconds), + timeout_explicit=timeout_seconds is not None, http_client=http_client, ) diff --git a/src/agentloop/tooling.py b/src/agentloop/tooling.py index 76e1f5e..93eb130 100644 --- a/src/agentloop/tooling.py +++ b/src/agentloop/tooling.py @@ -72,6 +72,9 @@ class ToolSpec(BaseModel): # Seconds one call may run before the model is told it timed out. None (the # default) waits forever. Python cannot kill a thread: see `_call_with_timeout`. timeout_seconds: float | None = Field(default=None, gt=0) + # False for a handler that is not thread-safe (a shared DB cursor, a non-atomic + # counter). A turn that calls any such tool runs ALL its calls one after another. + parallel_safe: bool = True class Registry: @@ -234,6 +237,14 @@ def reported_tool_name(registry: Registry, name: str) -> str: return name if registry.get(name) is not None else UNREGISTERED_TOOL +def _all_parallel_safe(calls: Sequence[ToolCallPart], registry: Registry) -> bool: + for call in calls: + spec = registry.get(call.tool_name) + if spec is not None and not spec.parallel_safe: + return False + return True + + def run_tools( content: Sequence[ContentPart], registry: Registry, @@ -248,8 +259,8 @@ def run_tools( calls = [part for part in content if isinstance(part, ToolCallPart)] if not calls: return [] - if len(calls) == 1: - return [_run_one(calls[0], registry, deps, watcher)] + if len(calls) == 1 or not _all_parallel_safe(calls, registry): + return [_run_one(call, registry, deps, watcher) for call in calls] with ThreadPoolExecutor(max_workers=MAX_PARALLEL_TOOLS) as pool: futures = [ diff --git a/src/agentloop/tracing.py b/src/agentloop/tracing.py index 7c2c4e0..2a0b165 100644 --- a/src/agentloop/tracing.py +++ b/src/agentloop/tracing.py @@ -41,6 +41,35 @@ ERROR_DETAIL_CHARS = 200 +def _provider_installed() -> bool: + """Whether a real tracer provider is the process-global one. + + Unset, OpenTelemetry's global is a proxy that forwards to nothing; it can also be + set explicitly to the no-op provider. Either way spans are thrown away. + """ + provider = trace.get_tracer_provider() + return not isinstance(provider, (trace.ProxyTracerProvider, trace.NoOpTracerProvider)) + + +_warned_no_provider = False +_warn_lock = threading.Lock() + + +def _warn_once_if_no_provider() -> None: + global _warned_no_provider + if _provider_installed(): + return + with _warn_lock: + if _warned_no_provider: + return + _warned_no_provider = True + logger.warning( + "tracing is on but no OpenTelemetry tracer provider is installed, so every span " + "is discarded. Call agentloop.tracing.configure_tracing(endpoint) or install your " + "own provider before running." + ) + + def _failure_attributes(error: ProviderError, *, prefix: str) -> dict[str, Any]: """Phase and send state as span attributes; None (unknown) is omitted, never False.""" attributes: dict[str, Any] = {} @@ -144,6 +173,7 @@ def conversation(self) -> Iterator[None]: "runs need one observer (one traced Agent) each" ) try: + _warn_once_if_no_provider() with _tracer.start_as_current_span( f"invoke_agent {self._agent_name}", attributes={ @@ -444,6 +474,14 @@ def configure_tracing(endpoint: str, *, service_name: str = "agentloop") -> Trac "Install: pip install 'based-models-agentloop[otel]'" ) from exc + if _provider_installed(): + # OpenTelemetry honours set_tracer_provider once per process. A second provider + # would look live and receive no spans, while the first is never flushed. + logger.warning( + "a tracer provider is already installed; configure_tracing() cannot replace " + "it, so the provider it returns will receive NO spans. Reuse the first " + "provider (trace.get_tracer_provider()) instead of calling this twice." + ) provider = TracerProvider(resource=Resource.create({"service.name": service_name})) provider.add_span_processor( BatchSpanProcessor(OTLPSpanExporter(endpoint=f"{endpoint.rstrip('/')}/v1/traces")) diff --git a/tests/test_config.py b/tests/test_config.py index 55e454d..be8631e 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -255,3 +255,40 @@ def test_an_httpx2_client_reaches_anthropic() -> None: client.close() # type: ignore[attr-defined] assert not http.is_closed # still the caller's http.close() + + +# --- timeout_seconds --- + + +def test_timeout_is_read_from_the_mapping() -> None: + assert Settings.from_env({"LLM_TIMEOUT_SECONDS": "42"}).timeout_seconds == 42 + assert Settings.from_env({}).timeout_seconds is None + assert Settings.from_env({"LLM_TIMEOUT_SECONDS": ""}).timeout_seconds is None + + +def test_a_timeout_must_be_positive() -> None: + with pytest.raises(ValidationError, match="timeout_seconds"): + settings(timeout_seconds=0) + + +def test_the_timeout_reaches_the_openai_compatible_read_timeout() -> None: + client = build_client(settings(provider="openai", openai_api_key="k", timeout_seconds=7)) + inner = client._inner # type: ignore[attr-defined] + assert inner._http.timeout.read == 7 + assert inner._http.timeout.connect == 10.0 # the provider's own value is kept + + +def test_an_explicit_timeout_beats_a_supplied_clients() -> None: + import httpx + + own = httpx.Client(timeout=httpx.Timeout(5.0)) # httpx's default: looks unchosen + chosen = httpx.Client(timeout=httpx.Timeout(99.0)) + for http_client, seconds, expect_override in [ + (own, None, True), (chosen, None, False), (chosen, 7.0, True), + ]: + client = build_client( + settings(provider="openai", openai_api_key="k", timeout_seconds=seconds), + http_client=http_client, + ) + sent = client._inner._timeout # type: ignore[attr-defined] + assert (sent is not None) is expect_override diff --git a/tests/test_loop.py b/tests/test_loop.py index 783496e..57c3a85 100644 --- a/tests/test_loop.py +++ b/tests/test_loop.py @@ -434,3 +434,37 @@ def test_a_turn_that_keeps_pausing_hits_the_iteration_budget() -> None: ]) with pytest.raises(LoopBudgetExceeded, match="3 iterations"): run(client, max_iterations=3) + + +# --- resuming a transcript that ends mid-turn --- + + +def test_a_resumed_transcript_with_unanswered_calls_runs_them_first() -> None: + transcript = Transcript() + transcript.add_user_message("go") + transcript.add_assistant_message([echo_call("c1", "left behind")]) + client = FakeClient(resp(TextPart(text="done"))) + run_conversation(transcript, client, REGISTRY, ToyDeps()) + # the request that went out already carried the answered call + sent = client.requests[0].messages + assert [m.role for m in sent] == ["user", "assistant", "user"] + assert "left behind" in sent[2].content[0].content # type: ignore[union-attr] + transcript.check_invariants() + + +def test_an_invalid_resumed_transcript_fails_before_any_model_call() -> None: + from agentloop.errors import TranscriptError + + transcript = Transcript() + transcript.add_user_message("go") + transcript.add_assistant_message([TextPart(text="x")]) + # a hosted-tool part in a user message is a state no provider accepts + transcript.messages.append( + transcript.messages[0].model_copy( + update={"content": (HostedToolPart(kind="web_search"),)} + ) + ) + client = FakeClient(resp(TextPart(text="never"))) + with pytest.raises(TranscriptError): + run_conversation(transcript, client, REGISTRY, ToyDeps()) + assert client.requests == [] diff --git a/tests/test_openai_compat.py b/tests/test_openai_compat.py index 8727c12..1ad3690 100644 --- a/tests/test_openai_compat.py +++ b/tests/test_openai_compat.py @@ -478,3 +478,15 @@ def test_an_ordinary_429_is_still_a_retryable_rate_limit() -> None: ) assert error.retryable + + +def test_reasoning_models_do_not_send_parallel_tool_calls() -> None: + from agentloop.providers.openai import QUIRKS, build_openai_client, quirks_for_model + + assert quirks_for_model("o3-mini").supports_parallel_tool_calls_param is False + assert quirks_for_model("gpt-5.1") is QUIRKS + client = build_openai_client(api_key="sk-test", model="o4-mini") + assert client._quirks.supports_parallel_tool_calls_param is False + assert client._quirks.max_tokens_field == "max_completion_tokens" # provider default kept + explicit = build_openai_client(api_key="sk-test", model="o4-mini", quirks=QUIRKS) + assert explicit._quirks is QUIRKS diff --git a/tests/test_tooling.py b/tests/test_tooling.py index a109b61..790cfc2 100644 --- a/tests/test_tooling.py +++ b/tests/test_tooling.py @@ -329,3 +329,38 @@ def test_one_hung_tool_does_not_hold_up_its_siblings() -> None: def test_a_timeout_must_be_positive() -> None: with pytest.raises(ValueError): ToolSpec(name="t", description="d", parameters=schema(), handler=boom, timeout_seconds=0) + + +# --- parallel_safe --- + + +def test_an_unsafe_tool_makes_the_whole_turn_sequential() -> None: + running = 0 + overlap = 0 + + def watch(deps: ToyDeps) -> str: + nonlocal running, overlap + running += 1 + overlap = max(overlap, running) + time.sleep(0.05) + running -= 1 + return "ok" + + def run(safe: bool) -> int: + nonlocal overlap + overlap = 0 + registry = Registry([ + ToolSpec(name="a", description="d", parameters=schema(), handler=watch, + parallel_safe=safe), + ToolSpec(name="b", description="d", parameters=schema(), handler=watch), + ]) + results = run_tools( + [ToolCallPart(id="1", tool_name="a", arguments={}), + ToolCallPart(id="2", tool_name="b", arguments={})], + registry, ToyDeps(), + ) + assert [r.call_id for r in results] == ["1", "2"] + return overlap + + assert run(safe=True) == 2 + assert run(safe=False) == 1 diff --git a/tests/test_tracing.py b/tests/test_tracing.py index 6e527ae..0a695bb 100644 --- a/tests/test_tracing.py +++ b/tests/test_tracing.py @@ -565,3 +565,46 @@ def test_an_unregistered_tool_name_stays_out_of_spans(exporter: InMemorySpanExpo spans = exporter.get_finished_spans() assert "made_up_xyz" not in str([(s.name, dict(attrs(s))) for s in spans]) assert "execute_tool " in [s.name for s in spans] + + +# --- setup mistakes are not silent --- + + +def test_a_traced_run_with_no_provider_warns_once( + monkeypatch: pytest.MonkeyPatch, caplog: pytest.LogCaptureFixture +) -> None: + from agentloop import tracing + + monkeypatch.setattr(tracing, "_provider_installed", lambda: False) + monkeypatch.setattr(tracing, "_warned_no_provider", False) + observer = OTelObserver(provider="fake", model="m") + with caplog.at_level("WARNING", logger="agentloop.tracing"): + for _ in range(2): + with observer.conversation(): + pass + assert sum("no OpenTelemetry tracer provider" in r.message for r in caplog.records) == 1 + + +def test_no_warning_when_a_provider_is_installed( + exporter: InMemorySpanExporter, monkeypatch: pytest.MonkeyPatch, + caplog: pytest.LogCaptureFixture, +) -> None: + from agentloop import tracing + + monkeypatch.setattr(tracing, "_warned_no_provider", False) + with ( + caplog.at_level("WARNING", logger="agentloop.tracing"), + OTelObserver(provider="fake", model="m").conversation(), + ): + pass + assert not caplog.records + + +def test_configure_tracing_twice_says_the_second_is_ignored( + exporter: InMemorySpanExporter, caplog: pytest.LogCaptureFixture +) -> None: + from agentloop.tracing import configure_tracing + + with caplog.at_level("WARNING", logger="agentloop.tracing"): + configure_tracing("http://localhost:4318").shutdown() + assert any("already installed" in r.message for r in caplog.records) From 69eaf0473b92ffd4080ed61fe14143b9009a0012 Mon Sep 17 00:00:00 2001 From: aaron Date: Sun, 18 Oct 2026 16:30:50 -0700 Subject: [PATCH 2/2] Record the live check of Responses reasoning replay (item 14) Co-Authored-By: Claude Sonnet 5.5 --- BACKLOG.md | 21 +++++++++------------ 1 file changed, 9 insertions(+), 12 deletions(-) diff --git a/BACKLOG.md b/BACKLOG.md index 4e49986..425ddbf 100644 --- a/BACKLOG.md +++ b/BACKLOG.md @@ -74,8 +74,8 @@ Left over: ## Medium priority -Items 13, 15, 16, 17 and 18 landed on `feature/medium-priority` (PR pending). Item 14 needs a -live call and is still open. Left over: +Items 13, 15, 16, 17 and 18 landed on `feature/medium-priority` (PR pending). Item 14 was checked live +(below). Left over: ### 13. Resuming a transcript that ends mid-turn: the paused case @@ -85,17 +85,14 @@ error. The new `_Run` starts with `continuing=False`, so the continuation become consecutive assistant message instead of extending the first. `Message` would have to record that the turn is open, for example in `Origin`. -### 14. Verify Responses reasoning replay without `include: ["reasoning.encrypted_content"]` +### 14. Responses reasoning replay without `include` (checked live, 2026-10-18) -**Status:** needs a live check; not done. The docstring in `_openai_responses.py` says the -encrypted reasoning content "`store: false` returns by default". OpenAI's API reference -documents it as an opt-in `include` value for stateless use. If it is opt-in, reasoning items -are replayed without their encrypted content. Reasoning models then lose their chain of thought -across tool calls, or the request is rejected. - -**What implementing it takes:** run one tool-using turn on a reasoning model live and check -`output[*].encrypted_content`. If it is missing, add `"reasoning.encrypted_content"` to -`include` whenever the model may reason, and save the captured body as a fixture (item 21). +Confirmed for `o4-mini`: a tool-using turn with `store: false` and no `include` returned a +`reasoning` item with a non-empty `encrypted_content`, and replaying the transcript with it was +accepted. The `_openai_responses.py` docstring is right for that model; no `include` is needed. +Not covered: `gpt-5.5` (the `.env` model) produced no reasoning item at its default effort +(0 reasoning tokens), so it did not exercise replay. Recheck if a request option for reasoning +effort is added. Item 21's fixture for a Responses reasoning item is still unrecorded. ### 17. Model-aware provider `Quirks`: what is left