Fix 31 audit findings: red CI, broken fallback, cache collisions, dead model defaults by jmbarrancoml · Pull Request #7 · jmbarrancoml/pureagents

jmbarrancoml · 2026-08-17T19:28:49Z

Description

An audit of the library turned up 31 issues, from a red CI to a default OpenAI
model that does not exist. This fixes all of them across 17 incremental
commits, each with its own tests.

CI was red on main on all three counts. ci.yml runs ruff check,
ruff format --check and pytest; every one failed. Two tests asserted models
clients.py never defined, ruff reported 6 errors and 35 unformatted files,
pyproject.toml declared version 0.1.0 while __init__ declared 0.6.0 (so a
tagged release would have published 0.1.0), and pydantic was a declared
dependency nothing imported.

Coverage went from 17 tests to 206. The old suite stopped at the @tool
decorator and JSONMemory; nothing exercised run(), the tool loop, or either
provider client, because there was no way to answer an HTTP request without a
network. A httpx.MockTransport harness (tests/conftest.py) now scripts both
wire formats, records request bodies, and fails loudly on an unscripted call.

Correctness fixes

The fallback sent the wrong model. provider="mistral", fallback="anthropic"
put mistral-large-latest on the Anthropic API on every failover. Attempts now
pair each client with the model it serves and its own API key.
An explicit api_key= lost to the environment variable. In multi-tenant
code the agent silently authenticated as the wrong account.
The response cache ignored the system prompt. The key was computed before
the system message was in place, so on a fresh agent it hashed only the prompt
and model. Agents with different personalities, providers or tool sets shared
entries. The cache was also unbounded, returned a raw string when output= was
set, and left a gap in the conversation on a hit.
A malformed tool-call argument string crashed the run. json.loads sat
outside the try block, so JSONDecodeError escaped into user code. One failing
call also cancelled its siblings mid-flight.
max_messages produced requests the APIs reject. Slicing by count alone
left a tool message whose assistant tool_calls message had been cut. The
window now snaps to a turn boundary.
@tool(timeout=) did nothing on sync functions, and parallel tool calls
ran serially, because sync tools blocked the event loop. They run on a worker
thread now: a 0.2s timeout on a time.sleep(1.5) tool fires at 0.2s instead of
returning at 1.5s.
Parallel tool calls broke Anthropic. Each result became its own user
message, producing consecutive user turns the API rejects.
Agent(timeout=300) still died at 60s. The clients hardcoded their own
ceiling and nothing propagated the agent's. Connections are pooled now too,
rather than a fresh TLS handshake per model call.
Retries covered everything, including 400s and 401s. Now 429, 5xx and
network failures only, honouring Retry-After, with jitter.
batch() silently changed the configuration, dropping enabled_groups,
hooks, max_messages, the validator and validation_retries, and fired every
prompt at once.
Session ids went straight into a filename, so session="../../evil" wrote
outside the sessions directory. Writes are atomic now.

Type hints to JSON Schema

Two separate converters both stopped at str/int/float/bool and marked
every field required. A single schema.py now handles typed lists and dicts,
Optional, Literal, Enum and nested dataclasses, and derives required
from the signature. The README's own structured-output example advertised
keywords: list[str] to the model as a string.

Documented examples that did not run

Three of the README's ten Python blocks failed if you copied them:
Agent(system=...) raised TypeError, the streaming example iterated
event.type against a generator of strings, and the Graph example never
called set_entry().

Model defaults, verified against each provider

gpt-5.2-instant does not exist — it is in neither OpenAI's active nor its
legacy pricing tables. provider="openai" would have 404'd for every user.
Defaults are now the cheapest current-generation tier that still drives a tool
loop, checked against each provider's own pricing page on 2026-08-17:

Provider	Default	$/1M in	$/1M out
`mistral`	`mistral-large-latest`	0.50	1.50
`openai`	`gpt-5.6-luna`	0.20	1.20
`anthropic`	`claude-sonnet-5`	2.00	10.00

MODEL_PRICING covers all three providers, so usage.cost() returns a number
for the common case instead of None.

Type of change

Breaking changes

All are in CHANGELOG.md. Each replaces behaviour that was itself the bug:

Agent(system_prompt=) → Agent(system=).
agent.stream() yields StreamEvent objects, not strings. This is what the
README already documented, and it makes tool calls visible mid-stream.
Exhausting max_steps raises MaxStepsError instead of returning
"Max steps reached without final answer." as an ordinary string, which a
caller could not tell from an answer and which broke the return type under
output=.
A reply that will not parse into the requested dataclass raises
StructuredOutputError (carrying .raw).
Usage.cost() takes a model, not a provider, and returns None for a model
with no rate on file rather than inventing a figure. The old per-provider
rates were badly out of date (Mistral at $0.25/1M). Usage.set_rates() covers
unlisted models and negotiated pricing.
Session ids are validated against a strict pattern.

Checklist

Tests pass (pytest) — 206 passing on Python 3.10 and 3.14
Linting passes (ruff check . and ruff format --check .)
Documentation updated — README, llms.txt, docs/, examples and CHANGELOG
Changes follow the "one parameter = one feature" philosophy

Related issues

None filed; found by audit.

CI ran ruff check, ruff format --check and pytest, and all three failed on main. This clears the first two and the packaging problems behind them. - Resolve the 6 ruff errors in examples/ (unused variable, unused import, f-string without placeholders, unsorted imports, two long lines). - Apply ruff format across the repo, including the Python blocks in README.md that the CI format check covers. - Single-source the version: pyproject declared 0.1.0 while __init__ declared 0.6.0, so a tagged release would have published 0.1.0. hatch now reads the version from src/pure_agents/__init__.py. - Drop pydantic from dependencies. Nothing imports it.

The two failing tests asserted models that clients.py never defined, so the suite was red on main. The Anthropic default was also claude-sonnet-4-20250514, which retires on 2026-06-15. Defaults are now mistral-large-latest, gpt-5.2-instant and claude-opus-5, with docs, llms.txt and examples brought in line.

@tool

Coverage stopped at the @tool decorator and JSONMemory: nothing exercised run(), the tool loop, or either provider client, because there was no way to answer an HTTP request without one. - Add an injectable `transport` to LLMClient and AnthropicClient. It defaults to None, so production behaviour is unchanged. - tests/conftest.py: FakeLLM queues scripted responses, records request bodies, and fails loudly on an unscripted call. Response builders for both wire formats. An autouse fixture clears provider env vars so a developer's real keys can never leak into a run. - tests/test_run_loop.py: 15 tests over the final-answer path, system prompt placement, history, usage accounting, single and parallel tool calls, unknown tools, tool exceptions, max steps, and the Anthropic conversions.

Two bugs made both features unusable in the cases they exist for. _create_client read `os.environ.get(env_var, self.api_key)`, so an explicit api_key= lost to whatever was in the environment. In multi-tenant code the agent silently authenticated as the wrong account. Precedence is now explicit key first, environment second, as the README always said. _chat_with_retry appended the fallback client to the client list but kept sending the primary provider's model, so `provider="mistral", fallback="anthropic"` sent mistral-large-latest to the Anthropic API on every failover. Attempts now pair each client with the model that client serves, and the fallback resolves its own API key from its own environment variable. Add fallback_model= and fallback_api_key= for explicit control, and reject a fallback with no key at construction rather than at failover.

The cache key was computed before the system prompt was placed into the history, so on a fresh agent it hashed only the prompt and the model name. Two agents with different personalities, different providers or different tool sets shared entries and answered each other's questions. - Key on provider, model, tool set, tool_choice and the full message list, with the system prompt already in place. - Replace the unbounded module dict with a bounded LRU (128 entries, tunable through the new set_cache_size()). It used to grow for the life of the process. - A hit now appends the exchange to the history instead of returning early and leaving a gap, and honours output=Dataclass instead of handing back a raw string where the type says T. - Stop caching a response that failed its validator on the final attempt.

json.loads on the model's argument string sat outside the try block, so a malformed tool call raised JSONDecodeError straight through run() and into user code. Models emit invalid JSON; the surrounding code already knew how to hand an error back to the model, and this path skipped it. - Parse arguments inside the guarded section and report a parse failure, a non-object payload, or a wrong argument name as a tool result. - Tolerate a missing tool_call id by synthesising one, so a provider that omits it no longer raises KeyError. - gather(return_exceptions=True): one failing call used to propagate and leave its siblings orphaned mid-flight. - Wrap the on_tool_call and on_tool_result hooks so a user callback that raises cannot take the agent down.

_trim_messages sliced the history by count alone, so max_messages could leave a `tool` message whose assistant `tool_calls` message had been cut. A 3-message window over a conversation with tool use produced [system, tool, user], which OpenAI-compatible APIs and Anthropic both reject with a 400. max_messages broke exactly the conversations tools are used in. The window now snaps back to the nearest user turn, so no tool result is ever separated from its call and the window still opens on a user message the way Anthropic requires. That can keep a message or two more than the budget asked for; correctness on the wire is worth more than an exact count. Trimming also runs after tool results are appended, so a long tool loop no longer grows the history without bound within a single run.

@tool

Tool.call wrapped a coroutine that invoked the function inline, so a sync tool blocked the event loop for its whole duration. Two consequences: asyncio.wait_for never got a chance to fire, making @tool(timeout=) silently inert for every sync tool, and _execute_tools_parallel ran sync tools one after another despite the asyncio.gather around them. Sync functions now run through asyncio.to_thread. A 0.2s timeout on a time.sleep(1.5) tool now raises after ~0.2s instead of returning after 1.5s, and three 0.3s tools finish in ~0.3s instead of ~0.9s. A timeout still cannot kill the thread, which the docstring now says outright.

@tool

Two independent converters both stopped at str/int/float/bool and sent everything else to the model as a string, and both marked every field required. - Add pure_agents/schema.py as the single converter: typed lists and dicts, Optional and unions, Literal, Enum and nested dataclasses, with bool matched before int rather than after it. - @tool now derives `required` from the signature, so `limit: int = 10` stops being an argument the model must invent. The Anthropic conversion reads the same schema instead of rebuilding its own. - The README's own structured-output example advertised `keywords: list[str]` as a string. It is now an array of strings. - _parse_structured raises StructuredOutputError, carrying the raw reply, instead of letting a JSONDecodeError or TypeError escape from run(). It also copes with a reply that wraps the JSON in prose.

_convert_messages emitted one user message per tool result, so two parallel tool calls produced two consecutive user turns and the API rejected the request with "roles must alternate". Results from the same assistant turn now share a single user message. max_tokens was hardcoded at 4096 with no way to change it. Agent(max_tokens=) now reaches both clients; OpenAI-compatible requests only carry the field when it is set, so their own defaults still apply. A response that stopped at max_tokens left tool_use input truncated, and the json.loads on it crashed the run with no hint about the cause. A truncated tool call now raises TruncatedResponseError naming the limit, and truncated text is flagged in the reply instead of passing as complete.

Both clients built a fresh httpx.AsyncClient inside every call, so each model request paid a full TLS handshake and no connection was ever reused. They now keep one client, created lazily and reused, with aclose() to release it. Agent gains aclose() and async-context-manager support so the connections have an owner. The clients also hardcoded timeout=60.0 and Agent never passed its own value down, so Agent(timeout=300) still failed at 60 seconds. The agent's timeout now sets the httpx deadline; asyncio.wait_for stays as the outer bound on total time, which httpx's per-operation timeouts do not cover.

The retry loop caught bare Exception, so a 401 or a 400 was retried with exponential backoff before finally surfacing. Retries now cover 408, 409, 425, 429, 5xx and network failures only, honour the server's Retry-After header, and use equal jitter so a fleet of agents does not retry in lockstep. Usage had two problems. last_input_tokens was never reset, so a response with no usage block silently re-counted the previous call's tokens. And cost() carried hardcoded per-provider rates that were badly out of date, defaulted to Mistral regardless of which provider the agent used, and returned a plausible number whatever you asked it. Usage now knows its own model and provider. cost() prices against a table of published rates and returns None when the model is not in it, rather than inventing a figure. set_rates() covers unlisted models and negotiated pricing.

stream() yielded bare strings, so a caller could see text arriving but had no way to know a tool had been called, which tool it was, or what it returned. The README documented `event.type` and `event.content` against this, and that example did not run. stream() now yields StreamEvent objects: text for a chunk, tool_call when the model asks for a tool, tool_result once it has run, done with the final answer. It was also a second-class path around the reliability layer, going straight to self.client and skipping retries, timeout, fallback and usage entirely, so agent.usage stayed at zero for any streamed run. Streaming now shares those, retrying only before the first chunk since a stream in flight cannot be restarted. Both clients report streamed tokens (stream_options.include_usage for OpenAI-compatible APIs, message_start and message_delta for Anthropic), and stream() accepts images= like run() does.

batch() built its sub-agents by hand and dropped enabled_groups, the hooks, max_messages, max_tokens, the validator and validation_retries, so a batch run had a different configuration from a direct run: group-scoped tools were all enabled, hooks never fired, and validation was off. It also fired every prompt at once and counted one request per prompt regardless of how many API calls each actually made. A shared _clone() now carries the whole configuration and reuses the parent's connection pool, concurrency is bounded (default 5), and usage aggregates the real counts. Running out of steps returned "Max steps reached without final answer." as an ordinary string, so a caller could not tell failure from success, and with output=Dataclass the return type was a lie. It now raises MaxStepsError, which carries the step count and the model's last partial answer. stream() still ends with a done event, since its events have already reached the caller. Session ids went straight into a filename, so session="../../evil" wrote outside the sessions directory. Ids are now validated against a strict pattern. Session and conversation files are written through a temp file and renamed, so a crash mid-write no longer leaves a truncated file, and a corrupt file reports what is wrong instead of raising a bare JSONDecodeError.

Three of the README's ten code blocks failed if you copied them. Agent(system=) raised TypeError because the parameter was called system_prompt, the streaming example iterated event.type against a generator of plain strings, and the Graph example never called set_entry() so run() raised straight away. - Rename the parameter and attribute to `system`, and update docs, examples and llms.txt to match. - Graph uses the first node added as its entry point, so the documented example runs. set_entry() still wins when called. - Graph gives each run its own copy of its agents, so two runs of the same graph no longer inherit each other's conversation. It also names the known nodes on an unknown target and rejects a function node that does not return the state. - Router clears its LLM router between calls. It used to accumulate every prompt it had ever routed, and its choices drifted as a result. - Document what run(), stream(), batch() and cost() actually return now, add a State and concurrency section, and correct llms.txt entries that named a temperature parameter and usage fields that never existed. - Correct the "~1,500 lines" claim to ~2,200.

@tool

A timeout hands control back to the agent at the deadline, but the thread keeps running. Under run_sync() that surfaces at the end, because asyncio.run drains the thread pool before returning. Say so where someone reading @tool(timeout=) will see it.

gpt-5.2-instant does not exist. It appears in neither OpenAI's active nor its legacy pricing tables; only gpt-5.2 and gpt-5.2-pro do. I took it from the repo's tests without verifying it, so provider="openai" would have 404'd for every user. Defaults are now the cheapest current-generation tier each provider offers that still drives a tool loop: mistral mistral-large-latest $0.50 / $1.50 (already the cheapest general-purpose Mistral; Large 3 undercuts Medium 3.5) openai gpt-5.6-luna $0.20 / $1.20 anthropic claude-sonnet-5 $2.00 / $10.00 (there is no Haiku 5; Haiku 4.5 is a generation behind with no adaptive thinking) MODEL_PRICING now covers all three providers with rates read off their pricing pages on 2026-08-17, so usage.cost() returns a number for the common case instead of None. Sonnet 5 was wrong at 3/15; the published rate is 2/10. A test asserts every provider default has a rate on file. Also drop gpt-5.2-thinking and gpt-5.2-codex from the provider guide. Neither appears in OpenAI's model list.

The tests import shared helpers as `tests.conftest`. `python -m pytest` puts the current directory on sys.path, so it worked locally; CI runs `pytest` directly, which does not, and all 13 new test modules failed to collect with ModuleNotFoundError. Declare pythonpath = ["."] in the pytest config rather than relying on how the runner happens to be invoked.

jmbarrancoidener added 18 commits August 17, 2026 19:16

jmbarrancoml merged commit 74e954d into main Aug 17, 2026
2 checks passed

Navigation Menu

Repository navigation

Fix 31 audit findings: red CI, broken fallback, cache collisions, dead model defaults - #7

Fix 31 audit findings: red CI, broken fallback, cache collisions, dead model defaults#7
jmbarrancoml merged 18 commits into
mainfrom
fix/audit-remediation

jmbarrancoml commented Aug 17, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

jmbarrancoml commented Aug 17, 2026

Description

Correctness fixes

Type hints to JSON Schema

Documented examples that did not run

Model defaults, verified against each provider

Type of change

Breaking changes

Checklist

Related issues

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants