Skip to content

Commit 94a773a

Browse files
committed
docs(website): address Codex round-2 review on PR #71
Three new findings on commit f709d3c, all verified: - resumable-tasks recipe: Agent.resume() does NOT auto-load from the checkpointer; only prompt() does (cubepi/agent/agent.py:228-235 vs 218-224). The advertised `python resume.py job-1` workflow raised `No messages to continue from`. Document manual hydration via `agent.state.messages = (await cp.load(thread_id)).messages` before resume(), update the smart_resume helper to do the same, and add a pitfall entry. - multi-provider-failover recipe: provider.stream() returns immediately and runtime errors arrive as StreamEvent(type="error"), not exceptions (anthropic.py producer task catches BaseException). The original wrapper never triggered its except branch on rate limits. Rewrite the wrapper to peek the first event from each inner stream, fall over on error events, and forward through a fresh outer MessageStream. Document the mid-stream-error limitation. - openai guide: `on_payload` cannot remove `stream_options` because cubepi calls `kwargs.setdefault("stream_options", {})` afterwards (openai.py:107). Replace misleading advice with the actual limitation and three workarounds (subclass, include_usage=False, upstream payload_quirks). Mirrors propagated to versioned_docs/version-0.3. pnpm build passes for both locales.
1 parent e3c9486 commit 94a773a

6 files changed

Lines changed: 258 additions & 88 deletions

File tree

website/docs/guides/providers/openai.md

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -157,8 +157,17 @@ provider.
157157
## Common pitfalls
158158

159159
- **`stream_options.include_usage` rejected** — Some compatibles
160-
reject the whole `stream_options` field. Override via `on_payload`
161-
to delete it before send.
160+
reject the whole `stream_options` field. **`on_payload` cannot fix
161+
this**: cubepi 0.3 calls `kwargs.setdefault("stream_options", {})`
162+
*after* your callback runs, so deleting the key in `on_payload` is
163+
silently undone. Workarounds:
164+
- Subclass `OpenAIProvider` and override `stream()` to skip the
165+
`setdefault` for your backend.
166+
- Set `include_usage=False` in `on_payload` (the field still goes
167+
out, but is usually accepted as a no-op even by strict
168+
backends).
169+
- Open an issue against cubepi to add a `payload_quirks` entry
170+
such as `"no_stream_options"` for native opt-out.
162171
- **Thinking events but no `thinking_*` events** — Your backend
163172
surfaces reasoning under a non-standard field. Either add a fourth
164173
branch via PR or transcode it with `on_payload`.

website/docs/recipes/multi-provider-failover.md

Lines changed: 91 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -16,15 +16,19 @@ crashing the agent. We'll wrap both providers behind a single
1616
```python title="failover.py"
1717
import asyncio
1818
import logging
19+
import time
1920
from typing import Sequence
2021

2122
from cubepi.providers.base import (
23+
AssistantMessage,
2224
Message,
2325
MessageStream,
2426
Model,
2527
Provider,
28+
StreamEvent,
2629
StreamOptions,
2730
ToolDefinition,
31+
Usage,
2832
)
2933
from cubepi.providers.anthropic import AnthropicProvider
3034
from cubepi.providers.openai import OpenAIProvider
@@ -33,7 +37,18 @@ log = logging.getLogger(__name__)
3337

3438

3539
class FailoverProvider:
36-
"""Try a primary provider; fall back to others on retryable errors."""
40+
"""Try providers in order; fall over on construction or first-event errors.
41+
42+
Built-in providers swallow API/network errors and surface them as
43+
`StreamEvent(type="error")` on the returned stream — never as exceptions
44+
out of `provider.stream()`. So we peek at the first event from each
45+
inner stream and only commit to it once we see a non-error event.
46+
47+
Limitation: errors that arrive *after* the first event (e.g. mid-stream
48+
rate limit, server disconnect) are forwarded to the agent as-is.
49+
Fully replaying a half-streamed turn against a fallback provider would
50+
require buffering the whole turn — out of scope here.
51+
"""
3752

3853
def __init__(self, primary_pair: tuple[Provider, Model], *fallbacks: tuple[Provider, Model]) -> None:
3954
self._chain: list[tuple[Provider, Model]] = [primary_pair, *fallbacks]
@@ -47,28 +62,64 @@ class FailoverProvider:
4762
tools: list[ToolDefinition] | None = None,
4863
options: StreamOptions | None = None,
4964
) -> MessageStream:
50-
# `model` is the agent's selected Model — we ignore it and use the
51-
# one paired with each provider in the chain.
52-
last_exc: BaseException | None = None
65+
last_error: str | None = None
66+
5367
for provider, mapped_model in self._chain:
68+
# Construction-time failures (rare — most stay inside the producer task).
5469
try:
55-
stream = await provider.stream(
70+
inner = await provider.stream(
5671
mapped_model,
5772
messages,
5873
system_prompt=system_prompt,
5974
tools=tools,
6075
options=options,
6176
)
62-
# Peek at the first event to validate the stream actually started.
63-
# If the provider produces a stream object but errors on first chunk,
64-
# we want to fall through.
65-
return stream
6677
except Exception as e:
67-
log.warning("provider %s failed: %s — trying fallback", mapped_model.provider, e)
68-
last_exc = e
78+
log.warning("provider %s failed at construction: %s", mapped_model.provider, e)
79+
last_error = repr(e)
80+
continue
81+
82+
# Peek at the first event to learn whether the stream is healthy.
83+
iterator = inner.__aiter__()
84+
try:
85+
first = await iterator.__anext__()
86+
except StopAsyncIteration:
87+
last_error = "stream ended before producing any events"
6988
continue
7089

71-
raise RuntimeError(f"all providers exhausted; last error: {last_exc!r}")
90+
if first.type == "error":
91+
log.warning("provider %s errored on first event: %s",
92+
mapped_model.provider, first.error_message)
93+
last_error = first.error_message or "stream error"
94+
continue
95+
96+
# Healthy — commit to this provider. Forward `first` plus the rest
97+
# through a fresh outer MessageStream so the caller sees a complete
98+
# stream starting at the start event.
99+
outer = MessageStream()
100+
101+
async def _forward(first_event=first, src=iterator, src_stream=inner):
102+
try:
103+
outer.push(first_event)
104+
async for ev in src:
105+
outer.push(ev)
106+
final = await src_stream.result()
107+
outer.set_result(final)
108+
except Exception as exc:
109+
fallback_msg = AssistantMessage(
110+
content=[],
111+
stop_reason="error",
112+
error_message=str(exc),
113+
usage=Usage(),
114+
timestamp=time.time(),
115+
)
116+
outer.push(StreamEvent(type="error", error_message=str(exc)))
117+
outer.set_result(fallback_msg)
118+
119+
outer.attach_task(asyncio.create_task(_forward()))
120+
return outer
121+
122+
raise RuntimeError(f"all providers exhausted; last error: {last_error!r}")
72123
```
73124

74125
## Use it
@@ -114,28 +165,31 @@ asyncio.run(main())
114165

115166
## What about smarter failover policies?
116167

117-
The example above falls back on **any** exception. That's the right
118-
behaviour for `RateLimitError`, `APIConnectionError`, or 5xx — but
119-
arguably wrong for `BadRequestError` (your code is wrong; the next
120-
provider will fail the same way).
168+
The example above falls back on **any** error event. That's fine for
169+
`RateLimitError`, `APIConnectionError`, or 5xx — but arguably wrong for
170+
`BadRequestError` (your code is wrong; the next provider will fail the
171+
same way).
121172

122-
Tighten the catch:
173+
The first-event `error_message` comes from `str(exc)` on the
174+
underlying SDK exception. Filter on substrings, or — better — wrap each
175+
provider's `_produce` to tag the error category:
123176

124177
```python
125-
import anthropic, openai
126-
127-
RETRYABLE = (
128-
anthropic.RateLimitError, anthropic.APIConnectionError, anthropic.APIStatusError,
129-
openai.RateLimitError, openai.APIConnectionError, openai.APIStatusError,
130-
)
131-
132-
# Inside the loop:
133-
except RETRYABLE as e:
134-
...
135-
except Exception:
136-
raise # not retryable
178+
NON_RETRYABLE_HINTS = ("bad request", "invalid_request_error", "401", "403")
179+
180+
if first.type == "error":
181+
msg = (first.error_message or "").lower()
182+
if any(h in msg for h in NON_RETRYABLE_HINTS):
183+
raise RuntimeError(f"non-retryable error from {mapped_model.provider}: {msg}")
184+
last_error = first.error_message
185+
continue
137186
```
138187

188+
A more robust approach is to fork the built-in providers and re-raise
189+
specific SDK exceptions from `_produce` so they reach `provider.stream()`
190+
as real Python exceptions — but that's a larger change against
191+
cubepi itself.
192+
139193
## Adding circuit breaking
140194

141195
Don't keep retrying a provider that's clearly down. A simple counter:
@@ -187,9 +241,14 @@ for that pattern.
187241
- **Different cost** — Failover from Anthropic to OpenAI changes
188242
per-token cost. Track which provider answered (via `on_response` or
189243
`AssistantMessage.provider_id`) and bill accordingly.
190-
- **Streaming consistency** — The wrapper passes streams through
191-
unchanged, so consumers see the same `StreamEvent` shape regardless
192-
of which provider answered.
244+
- **Streaming consistency** — The wrapper forwards events through a
245+
fresh `MessageStream`, so consumers see the same `StreamEvent` shape
246+
regardless of which provider answered. The original `start` event
247+
comes from the inner provider unchanged.
248+
- **Mid-stream errors aren't recovered** — Once we've seen a healthy
249+
first event, the wrapper commits to that provider. If it errors
250+
halfway through a long response, the agent sees the error. Full
251+
mid-stream replay would require buffering — out of scope here.
193252

194253
## See also
195254

website/docs/recipes/resumable-tasks.md

Lines changed: 27 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -121,11 +121,22 @@ async def main(thread_id: str, initial_prompt: str | None):
121121
agent.subscribe(lambda e, s=None: None)
122122

123123
if initial_prompt:
124-
# Fresh run.
124+
# Fresh run. prompt() auto-loads history on first call before
125+
# appending the new user message.
125126
await agent.prompt(initial_prompt)
126127
else:
127-
# Resume. If the last message is an assistant message with no
128-
# follow-up queued, this will raise; otherwise it picks up.
128+
# Resume. agent.resume() does NOT auto-load — only prompt() does.
129+
# Hydrate the agent state manually first.
130+
data = await cp.load(thread_id)
131+
if data is None:
132+
raise RuntimeError(f"No saved state for thread {thread_id!r}")
133+
agent.state.messages = list(data.messages)
134+
# `extra` is restored too; it's private on Agent, so use the
135+
# checkpointer's view if your middleware reads it.
136+
137+
# Resume picks up from the last persisted message:
138+
# ToolResultMessage / UserMessage → re-invokes the model
139+
# AssistantMessage with no queued steer/follow_up → raises
129140
await agent.resume()
130141

131142

@@ -150,13 +161,15 @@ python resume.py job-1
150161
## The three resume scenarios in code
151162

152163
```python
153-
async def smart_resume(agent):
154-
msgs = agent.state.messages
155-
if not msgs:
156-
# Brand-new conversation. Caller must prompt.
157-
return False
158-
159-
last = msgs[-1]
164+
async def smart_resume(agent, cp, thread_id):
165+
# resume() doesn't auto-load — hydrate the agent first if its state is empty.
166+
if not agent.state.messages:
167+
data = await cp.load(thread_id)
168+
if data is None or not data.messages:
169+
return False # nothing to resume from
170+
agent.state.messages = list(data.messages)
171+
172+
last = agent.state.messages[-1]
160173
last_role = type(last).__name__
161174

162175
if last_role == "AssistantMessage":
@@ -197,6 +210,10 @@ tool args. That's what `transcode_video` above does with `JOB_DIR`.
197210
- **`resume()` after an assistant message with no queue** — Raises.
198211
Either prompt the user for the next message or call `prompt()`
199212
fresh.
213+
- **`resume()` on a fresh agent** — Raises `No messages to continue
214+
from`. `resume()` does not auto-load from the checkpointer; only
215+
`prompt()` does. Hydrate manually with `agent.state.messages =
216+
(await cp.load(thread_id)).messages` first.
200217
- **Forgetting the signal check inside the tool** — A long
201218
`await asyncio.sleep(...)` or a `for ... in stream` that ignores
202219
`signal.is_set()` won't honour `abort`. Drop a check inside any

website/versioned_docs/version-0.3/guides/providers/openai.md

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -157,8 +157,17 @@ provider.
157157
## Common pitfalls
158158

159159
- **`stream_options.include_usage` rejected** — Some compatibles
160-
reject the whole `stream_options` field. Override via `on_payload`
161-
to delete it before send.
160+
reject the whole `stream_options` field. **`on_payload` cannot fix
161+
this**: cubepi 0.3 calls `kwargs.setdefault("stream_options", {})`
162+
*after* your callback runs, so deleting the key in `on_payload` is
163+
silently undone. Workarounds:
164+
- Subclass `OpenAIProvider` and override `stream()` to skip the
165+
`setdefault` for your backend.
166+
- Set `include_usage=False` in `on_payload` (the field still goes
167+
out, but is usually accepted as a no-op even by strict
168+
backends).
169+
- Open an issue against cubepi to add a `payload_quirks` entry
170+
such as `"no_stream_options"` for native opt-out.
162171
- **Thinking events but no `thinking_*` events** — Your backend
163172
surfaces reasoning under a non-standard field. Either add a fourth
164173
branch via PR or transcode it with `on_payload`.

0 commit comments

Comments
 (0)