Commit 62ae31b
authored
fix: degrade gracefully when a model or provider is unavailable (#299)
## Related Issue
No tracked issue — reported directly with a screenshot showing the
stacked toasts and the interrupted-turn banner.
## Problem
Removing a provider, or selecting a model that no longer exists,
produced repeated hard failures instead of degrading.
Two independent bugs, causally chained:
**1. A wire casing mismatch broke the realtime channel.** The gateway
published `event.config.changed` with camelCase `changedFields`; the web
mapper read snake_case `changed_fields`. The resulting
`undefined.length` threw at `usePythinkerWebClient.ts:481`. Because
`config.changed` is a *control* event, the batcher processes it
synchronously — so the throw landed inside `ws.ts`'s single `try`, which
wrapped `JSON.parse`, `traceWsIn` **and** `handleFrame` together. Every
such frame surfaced as:
```
Realtime connection error
Failed to parse WS frame: TypeError: Cannot read properties of undefined (reading 'length')
```
with one uncapped toast per frame. Every other payload on the wire is
snake_case; `changedFields` was the lone outlier.
That handler is also the only mid-session announcer, so it died *before*
`loadModels`/`loadProviders` ran — which is why the picker kept offering
models that had just been deleted.
**2. Model resolution trusted a persisted id.** `resolveDefaultModel`
returned the stored `current` unvalidated, so a deleted model's id
survived as the default. An existing session kept the dead alias and
failed every turn with "Model request failed — this turn was
interrupted". Meanwhile `isRetryableGenerateError` treated an
unclassified `ChatProviderError` as retryable by default, so a
deterministic configuration error burned all 10 attempts — roughly 143s
of backoff — before failing.
## What changed
**Realtime boundary.** Renamed the field to snake_case across
`protocol`, `agent-gateway` and the web client so the wire is internally
consistent. Split `ws.ts`'s `try` so a parse failure and a handler
failure are distinct errors, with the handler message naming the frame
type. Guarded the payload reads and added a boundary gate mirroring the
server's own. Warnings now dedupe by severity+title+message with a
repeat count, capped at 5.
**Model resolution.** Made it total and read-time. The persisted default
survives only while it still resolves ready; otherwise the
highest-ranked **ready** candidate is selected; when none qualifies the
product reaches an explicit empty state instead of failing every turn.
The same predicate runs at turn bind, so a model or provider that
disappears mid-session is handled before the request rather than after
it.
**Retry.** An unclassified provider error is no longer retryable by
default. Rate-limit text patterns are promoted into
`classifyBaseApiError` in the same change, so text-only proxies keep
their retries.
**Deleted.** `AgentModelFallbackService`, its experimental flag, and the
`loopControl.fallbackModel` knob.
Retry-ten-times-then-switch-to-one-configured-model was a shadow
resolution policy that read-time resolution subsumes.
### Behaviour change worth a reviewer's attention
Deleting or rebuilding a provider that owns the default now **re-points
`default_model` in the user's `config.toml`** to the best ready model,
rather than leaving a dangling pointer. The user is told via a
`ConfigWarning` naming the replaced model, the selected model, and the
reason. This follows from the resolution policy, but it is new
observable behaviour on the config file and a deliberate product
decision rather than a bug fix — flagging it explicitly rather than
leaving it in the changeset sentence.
Related: `vertexai` models can never satisfy the readiness predicate,
because `vertexai` has no registered `ProviderDefinition` and no
`ProtocolSchema` entry (the repo's own `protocol.test.ts:23` asserts
this). Such a model already threw `CONFIG_INVALID` at request
construction and `/auth` already reported it not-ready, so nothing that
worked regresses — but a `default_model` that was previously
*advertised* and unusable is now correctly not selected. Vertex remains
reachable via `type: 'google-genai'`.
### Not fixed here, found along the way
- `event.plugin.changed`, `event.capability.changed` and
`event.di.unit_changed` have no web mapping and render as `Unhandled
event: …` toasts — same class as the bug above.
- `packages/tree-sitter-bash/test/fuzz.test.ts:137` omits the `{
timeoutMs: 60_000 }` its siblings at lines 129 and 141 pass, so it
flakes under full-suite load. `agent-gateway/test/expertTalk.test.ts`
has the same fixed-timeout shape.
- Six packages have no `test` script (`kosong`, `node-sdk`, `pyaos`,
`desktop`, `desktop/runtime-host`, `vis`), so `pnpm --filter <pkg> test`
silently runs nothing for them. A stale `node-sdk` assertion in this
change set was caught only by the workspace-wide pre-push run.
- The PUT provider route does not `await IModelService.settled` the way
create does, so its pointer repair lands after the 200.
## Checklist
- [x] I have read the
[CONTRIBUTING](https://github.com/PyModel/pythinker-code/blob/main/CONTRIBUTING.md)
document.
- [ ] I have linked a related issue — none exists; reported directly.
- [x] I have added tests that prove my feature works.
- [x] Ran `gen-changesets` skill, or this PR needs no changeset.
- [ ] Ran `gen-docs` skill — the three OpenAPI route descriptions that
contradicted the new behaviour were corrected in place; no user-facing
docs page covers this.1 parent e9602f0 commit 62ae31b
153 files changed
Lines changed: 1743 additions & 876 deletions
File tree
- .changeset
- apps
- pythinker-code/dist-web
- assets
- pythinker-web
- src
- api
- daemon
- components
- ui
- composables
- client
- i18n/locales/en
- test
- packages
- agent-core-v2
- docs
- src
- agent
- llmRequester
- loop
- profile
- stepRetry
- turnRecovery
- kosong
- contract
- model
- test
- agent
- profile
- stepRetry
- turnRecovery
- app
- kosongConfig
- llmProtocol
- kosong
- contract
- model
- session/agentLifecycle
- agent-core/src/services/config
- agent-gateway
- src
- protocol
- routes
- transport/ws/v1
- test
- kosong
- src
- test
- node-sdk/test
- protocol/src
Some content is hidden
Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1 | 1 | | |
2 | | - | |
3 | | - | |
| 2 | + | |
| 3 | + | |
4 | 4 | | |
Lines changed: 2 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.
Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.
Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.
Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.
Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.
0 commit comments