feat: expose safe model run failures#243
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e5116cc3f3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const status = AgentModelTurnRetryService.readStatusCode(error); | ||
| const failureCode = status === undefined ? 'unknown' : MODEL_FAILURE_BY_STATUS.get(status) ?? 'unknown'; | ||
| const failure = AgentModelTurnRetryService.modelFailure(failureCode); | ||
| const message = AgentModelTurnRetryService.safeMessage(failureCode); |
There was a problem hiding this comment.
When a provider rejects an oversized prompt/context with HTTP 400/422, this replaces the provider error text with Model request was rejected before chat persistence sees the result. I checked ConversationTurnArtifacts.build: it drives shouldForceCompactionAfterFailure(args.result.summary) and failure hint formatting from args.result.summary, looking for context-window/prompt-too-long phrases. After this change those phrases are gone, so long sessions no longer force compaction or show the recovery hint, and retrying the same conversation can keep failing.
Useful? React with 👍 / 👎.
| const message = AgentModelTurnRetryService.formatErrorMessage(error); | ||
| const providerMessage = AgentModelTurnRetryService.formatErrorMessage(error); | ||
| const status = AgentModelTurnRetryService.readStatusCode(error); | ||
| const failureCode = status === undefined ? 'unknown' : MODEL_FAILURE_BY_STATUS.get(status) ?? 'unknown'; |
There was a problem hiding this comment.
When using the built-in OpenAI-compatible providers such as Hugging Face, OpenRouter, Together, or Groq, OpenAiCompatibleAdapter.readJsonResponse throws a plain Error whose message contains response.status but has no status/statusCode property. This line therefore classifies those 401/403/429 responses as unknown (and some 429s may not retry unless the body happens to say rate limit), so hosted BYOK callers relying on result.failure.code cannot distinguish rejected credentials or rate limits for these supported providers.
Useful? React with 👍 / 👎.
1 participant
What changed
RunFailurethroughRunResult, terminal traces, loop state/activity/checkpoints, and programmatic conversation turn resultsresult.failureinstead of parsing provider textWhy
A hosted BYOK product needs to distinguish a rejected user credential from other model failures without duplicating provider HTTP/status logic. The existing Heddle model-turn boundary already owns retry classification, so this extends that service rather than adding a parallel classifier.
This also closes a secret-safety gap: arbitrary provider error text no longer flows into durable traces, logs, or product-facing summaries.
Impact
The new failure field is optional and backward-compatible. Existing hosts keep their current result flow; hosts that need stable product errors can branch on
result.failure?.code. Human-readable model error summaries are intentionally safer and less provider-specific.Verification
yarn test: 114 unit files / 674 tests; 32 integration files / 293 testsyarn eslintnpm pack --silent --pack-destination /private/tmp(full build, remote-package compile, web build, hosted React example build)