Skip to content

feat: request-time model fallback at streamFn — honor --fallback-model (#51)#65

Merged
yasha-dev1 merged 1 commit into
mainfrom
feat/request-time-model-fallback
Jun 16, 2026
Merged

feat: request-time model fallback at streamFn — honor --fallback-model (#51)#65
yasha-dev1 merged 1 commit into
mainfrom
feat/request-time-model-fallback

Conversation

@yasha-dev1

Copy link
Copy Markdown
Collaborator

Closes #51.

Summary

Implements Layer 1 / availability fallback: when the primary model's request fails before the first chunk with a retryable error (429 / 5xx / network), the current turn is transparently retried against a configured fallback model. Wires the previously-reserved --fallback-model flag end-to-end.

The seam — and a correction to the issue's sketch

The issue proposed try { return await streamSimple(...) } catch. That can't work: pi-ai's streamSimple returns an AssistantMessageEventStream synchronously and does not throw on a failed request — the failure arrives as a single { type: 'error' } event (carrying an AssistantMessage with stopReason: 'error' + errorMessage) with no preceding start event (verified in pi-ai's provider code).

pi-agent-core's StreamFn may return a Promise<stream>, so the correct approach is:

  1. Peek the primary stream's first event.
  2. If it's a retryable error (and a fallback exists) → discard and try the fallback.
  3. Otherwise (success first chunk, non-retryable error, or last attempt) → return a stream that replays the peeked event(s) and forwards the rest, so the Agent loop's for await + response.result() behave exactly as with a direct streamSimple call.

A mid-stream failure (error after a start) is not retried — too late to swap cleanly (out of scope, as the issue notes).

Changes

  • core/model-fallback.ts (new):
    • isRetryableStreamError(msg) — retryable on 408/409/425/429/5xx/529 and network phrases (ECONNRESET, timeout, overloaded, …); not retryable on auth/4xx (401/403/400/404/422). Guards against matching a code embedded in a larger number.
    • streamWithFallback(ctx, attempts, streamFn, isRetryable?) — peek/retry/replay. streamFn is injectable for testing. Single attempt = thin pass-through.
  • core/sdk.ts — factored resolveModel(provider, modelId) and withProviderAuth(model, opts, primaryProvider) (shared by primary + fallback; a cross-provider fallback resolves its own env key). Added the fallback?: { provider?; modelId } option (provider defaults to primary). streamFn now builds the attempt list and delegates to streamWithFallback.
  • cli/args.ts + cli/main.ts — plumb --fallback-model (+ optional --fallback-provider); dropped the “not yet enforced” comment; updated help text.

Acceptance criteria

  • createAgentSession({ fallback }) retries on a retryable pre-stream error and succeeds against the fallback.
  • Non-retryable errors (auth/4xx) propagate unchanged.
  • --fallback-model honored end-to-end (CLI → main.ts → createAgentSession).
  • Help text / args comment updated.
  • Unit tests with a stubbed streamFn: primary succeeds (no fallback used), primary 429 → fallback used, primary non-retryable → throws.

Tests

  • core/tests/model-fallback.test.ts — 13 tests (success/no-fallback, 429→fallback, non-retryable→propagate, all-fail→surface last error, mid-stream error → no retry, multi-event replay, sync-throw propagate vs retry, custom predicate, empty attempts).
  • core/tests/sdk-fallback.test.ts — 3 integration tests (option plumbing + resolveModel for primary & fallback; no-fallback pass-through; unknown fallback id throws eagerly).
  • cli/tests/args-sdk-flags.test.ts--fallback-model / --fallback-provider parsing.

🤖 Generated with Claude Code

#51)

When the primary provider is down / rate-limited (429) / returns a
5xx/network error, transparently retry the same turn against a configured
fallback model. Wires the previously-reserved --fallback-model flag.

Key runtime detail: pi-ai's streamSimple returns the event stream
*synchronously* and never throws on a failed request — a failure surfaces
as a single `{type:'error'}` event with no preceding `start`. So the issue's
`await streamSimple` + try/catch sketch can't work. Instead we peek the first
event (streamFn may return a Promise<stream>): a retryable error before the
first chunk advances to the fallback; otherwise we return a stream that
replays the peeked events and forwards the rest, so the Agent loop (iteration
+ result()) is unaffected.

- model-fallback.ts (new): isRetryableStreamError (429/5xx/network, NOT
  auth/4xx) + streamWithFallback (peek/retry/replay). Pure, injectable streamFn.
- sdk.ts: factor resolveModel() + withProviderAuth() (shared by primary &
  fallback; cross-provider fallback resolves its own env key); add
  `fallback` option; wire streamWithFallback into streamFn (thin pass-through
  when no fallback).
- main.ts / args.ts: plumb --fallback-model (+ --fallback-provider); drop the
  "not yet enforced" note; update help.

Tests: 13 unit (stubbed streamFn) covering success/429-fallback/non-retryable/
all-fail/mid-stream-no-retry/replay/sync-throw/custom-predicate; 3 SDK
integration (option plumbing + resolveModel, unknown-id throw); 2 args.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@yasha-dev1

Copy link
Copy Markdown
Collaborator Author

✅ Verification

Full suite + typecheck + lint (clean)

Test Files  74 passed (74)
     Tests  646 passed (646)
  • npm run build (core + cli) → success
  • npm run typecheck → pass
  • npm run lint → pass

New tests for this feature (24)

✓ packages/core/tests/model-fallback.test.ts (13 tests)
✓ packages/core/tests/sdk-fallback.test.ts  (3 tests)
✓ packages/cli/tests/args-sdk-flags.test.ts (8 tests, +2 for fallback flags)

End-to-end runtime check against the built binary

Help text:

--fallback-model <model>     Model to retry the turn against on a 429/5xx/network error
--fallback-provider <name>   Provider for --fallback-model (default: --provider)

Driving the built packages/core/dist with a stubbed streamFn where the primary emits a 429 error event and the fallback succeeds:

429 primary -> events seen: ["start","done"] | fallback content: [{"type":"text","text":"ok"}]
isRetryable -> 429: true | 401: false

The consumer sees only the fallback's events — the primary's error is swallowed, the turn survives.

Behavior confirmed

  • Primary succeeds → fallback never invoked (single streamFn call); events/result() pass through unchanged.
  • Primary 429 (pre-stream) → transparently retried on the fallback; consumer sees only the fallback stream.
  • Non-retryable (401/4xx) → error propagates, fallback NOT attempted.
  • Mid-stream error (after the first start) → NOT retried (can't swap cleanly); error surfaces.
  • Both attempts fail → last error surfaces as a terminal event (no hang).
  • Sync throw (e.g. missing API key) → propagates if non-retryable, falls back if the message is retryable.
  • Heuristic classifier doesn't false-match a status code embedded in a larger number (5000/429000).

Behavior is exercised against the real pi-ai AssistantMessageEventStream contract (synchronous return; failures as {type:'error'} events), not a hand-rolled mock of it.

@yasha-dev1 yasha-dev1 added the PR OK Grok PR verified and review passed label Jun 16, 2026
@yasha-dev1

Copy link
Copy Markdown
Collaborator Author

PR OK — verified end-to-end (posted as comment; GitHub blocks self-approval).

Addresses #51: request-time availability fallback at the streamFn chokepoint, with --fallback-model wired end-to-end. All acceptance criteria met.

Notable: the PR correctly identifies that the issue's own sketch (try { streamSimple } catch) cannot work — pi-ai's streamSimple returns synchronously and surfaces a failed request as a { type: 'error', error: AssistantMessage } event, not a throw. I verified this against the installed pi-ai types:

  • AssistantMessageEvent error variant is { type: 'error'; reason; error: AssistantMessage }peekErrorMessage reads first.value.error.errorMessage correctly.
  • createAssistantMessageEventStream() is exported with push/end/result()/async-iterator — the replay stream uses them correctly, and result() is reproduced faithfully because it's extracted from the replayed terminal event.
  • StreamFn is typed ... | Promise<ReturnType<typeof streamSimple>>, so the peek-then-return pattern is valid.

The peek/retry/replay design is sound: retry only when the first event is a retryable error (clean pre-stream swap); a start-then-error mid-stream is correctly not retried (out of scope, and tested). isRetryableStreamError is conservative — auth/4xx (401/403/400/404/422) not retried, with a guard so a code embedded in a larger number (e.g. 14290) doesn't match 429.

Refactor is behavior-preserving: resolveModel + withProviderAuth factor the existing per-provider logic; the primary path keeps the Agent-resolved apiKey unchanged, only a cross-provider fallback resolves its own env key.

Verification: build ✓ · typecheck ✓ · lint ✓ · full suite 646/646 · 24 new tests (model-fallback 13, sdk-fallback 3, args-sdk-flags +5) covering primary-success/no-fallback, 429→fallback, non-retryable→propagate, all-fail→surface-last (no hang), mid-stream→no-retry, multi-event replay, sync-throw cases, custom predicate, empty-attempts, and CLI parsing.

Excellent work — correct, well-scoped, thoroughly tested. LGTM.

@yasha-dev1 yasha-dev1 merged commit f2f9898 into main Jun 16, 2026
2 checks passed
@yasha-dev1 yasha-dev1 deleted the feat/request-time-model-fallback branch June 16, 2026 09:34
LivinTribunal added a commit that referenced this pull request Jul 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

PR OK Grok PR verified and review passed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

request-time model fallback (availability) at streamFn — honor --fallback-model

1 participant