fix(provider): classify a max_tokens-truncated Anthropic response instead of masking it - #4516
Conversation
Anthropic always requests fine-grained tool streaming, under which
`partial_json` is not guaranteed to be valid JSON when `stop_reason` is
`max_tokens`. The stream parser parsed the tool input in the
`content_block_stop` branch and threw "invalid successful stream (tool call
arguments were not valid JSON object text)", so a legitimate output token
limit read as a malformed provider stream.
The parser now defers that parse failure, drops the incomplete tool call from
the yielded parts and from the raw content blocks a replay checkpoint would
persist, and classifies it once `stop_reason` arrives: `max_tokens` throws the
new `ProviderOutputTruncatedError`, anything else still throws the original
malformed-stream error. The new curated failure code
`PROVIDER_OUTPUT_TRUNCATED` carries it across the runtime boundary as a
terminal, non-retryable error.
The internal-agent replay relay also replaced whatever failed with the fixed
message "Provider replay turn failed before its boundary", because the failure
hook took no cause. The hook now carries the runtime's already-sanitized
`{message, code}` pair, and the run error reports that code instead of a
blanket `RUNTIME_ERROR`.
Refs #1467, #1475
… failures
Rework of the previous commit after review. It left the runtime wiring —
the only production code carrying the sanitized `{message, code}` into
`__vfProviderReplayCheckpointTurnFailed` — untested, and reverting
`src/agent/runtime/index.ts` alone kept the suite green.
`run-stream.test.ts` now drives the real path: a real
`ProviderOutputTruncatedError` raised inside a replay-checkpoint run whose
consumer is at a tool boundary with no checkpoint frame behind it. Reverting
the runtime wiring reproduces the staging fingerprint exactly —
`RunError {code:"RUNTIME_ERROR", message:"Provider replay turn failed before
its boundary"}` — and fails the test. `provider-replay-emission.test.ts`
additionally asserts the hook argument itself for a truncation, a
cancellation and a persistence failure; the three pre-existing fixtures
only ever declared zero-argument hooks.
Three behaviour fixes came out of the same review:
- The relay's fallback message is neutral again. Manufacturing "Provider
stream failed" for a causeless failure blamed the provider for a
Veryfront problem. A checkpoint persistence failure now forwards its own
`DURABLE_RUN_EVENT_PERSISTENCE_FAILED` title and code instead.
- A cancellation no longer hands the relay a cause at all. The sanitized
event is resolved after the abort check, so `resolveRuntimeFallbackErrorEvent`
can no longer put a raw client abort reason on a replay boundary.
- The parser withholds every tool call in a turn once one tool input fails
to parse. Deferring the failure to `message_delta` had let a later,
well-formed tool block yield a `tool-call` part that the pre-deferral
parser never emitted, which a consumer could dispatch from a turn that
then throws.
Also covers the deferred failure resolved by `validateCompletion()` through
a buffered trailing `message_delta` on the client tool-use read timeout —
the one reachable route to that arm — and repairs a test that did not type
check.
BEHAVIOUR CHANGE, needs a reviewer decision: `PROVIDER_OUTPUT_TRUNCATED` is
in `CURATED_PROVIDER_FAILURE_CODES`, so `resolveKnownProviderTerminalError`
returns non-null and `child-lifecycle.ts` / `durable-child-fork-execution.ts`
stop retrying a truncation that previously sat in the retryable
`PROVIDER_STREAM_ERROR` bucket. A retry above temperature 0 could
occasionally have produced a shorter tool input and succeeded. The CHANGELOG
entry now states this explicitly; the issue text asked for a "typed
retryable error", and this lands terminal instead.
Still not closed by this change: the truncated turn leaves no durable
frames, because `buffered.splice(0)` is retained by design. The maintainer's
"no truncated finish_reason anywhere in the database" finding needs its own
change.
Refs #1467, #1475
|
You have reached your Codex usage limits for security reviews. Please try again later. |
|
Warning Review limit reachedNext included review available in 5 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (6)
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (15)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe change adds ChangesProvider error contract
Anthropic stream classification
Replay failure propagation
Regression validation and release notes
Priority: ➖ Normal Estimated code review effort: 4 (Complex) | ~45 minutes Change: Bug fix Sequence Diagram(s)sequenceDiagram
participant Anthropic
participant AnthropicStream
participant ProviderErrors
participant ReplayRuntime
participant RunStream
Anthropic->>AnthropicStream: tool input and stop_reason
AnthropicStream->>ProviderErrors: create ProviderOutputTruncatedError
ProviderErrors->>ReplayRuntime: PROVIDER_OUTPUT_TRUNCATED
ReplayRuntime->>RunStream: sanitized message and code
RunStream->>RunStream: emit terminal RunError
Merge Risk: ⚪ Minimal · up to The truncation classification and replay error propagation paths include coverage for sanitized errors, cancellation, persistence failures, and deferred tool parsing. No concrete merge-blocking risk remains identified. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 30.77% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 13 functions across 13 files. (2 skipped: 2 unsupported.) ✨ Finishing Touches 💡 2📝 Generate docstrings 💡
🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
📦 Client bundle boundary
A server module in a client graph aborts hydration in the browser. New leaks fail CI; known leaks are tracked in |
|
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1963bf3e00
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
…type The generated API reference tracks exported provider types; adding PROVIDER_OUTPUT_TRUNCATED left docs/api-reference/veryfront/provider.md stale and failed the lint:ci docs check.
|
You have reached your Codex usage limits for security reviews. Please try again later. |
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1b99e5f8b1
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
Codex P1 on veryfront-code#4516. Forwarding the resolved error event to the replay relay meant a NON-provider failure -- turnPersistence.finalize() throwing, say -- reached a public RunError with its raw Error.message, where the path previously emitted a neutral boundary message. A persistence error can carry a database URL or an internal path, which AGENTS.md:118-128 forbids in user-facing output. resolveRelayableExecutionFailure returns a message only for curated provider terminal errors and explicitly public lifecycle messages; anything else keeps the relay's neutral default. The SSE stream path is unchanged -- it may still show a fallback message; a durable client-visible RunError may not. Fixed on BOTH relay sites. The review flagged the stream path; the generate path at the same file had the identical leak. Pinned by a test asserting a persistence failure's message reaches the SSE event but not the relay, and that a truncation still relays its PROVIDER_OUTPUT_TRUNCATED code -- the point of #1467. It fails if the fallback is relayed again.
|
You have reached your Codex usage limits for security reviews. Please try again later. |
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a29a55948b
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Two Codex findings on veryfront-code#4516. P2: the deferred tool-input failure resolved on every message_delta, including a usage-only one that carries no stop_reason. rawStopReason was still undefined there, so the parser threw the generic malformed-stream error before the later delta that says max_tokens -- turning the truncation this change exists to identify back into the error it was masking. It now decides only once a stop reason has arrived. P1: the changelog is public documentation and my entry named internal source paths to explain the retry change. AGENTS.md prohibits internal implementation paths in public docs, and they go stale after a refactor. Described in public terms instead. The usage-only delta case is pinned and fails without the gate.
|
You have reached your Codex usage limits for security reviews. Please try again later. |
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
|
@codex review |
|
Codex Review: Didn't find any major issues. More of your lovely PRs please. Reviewed commit: ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback". |
|
Problem
A
max_tokens-truncatedtool_usewas treated as a malformed stream.anthropic-stream.ts(content_block_stop) didJSON.parse(joinAnthropicToolInput(...))and, on failure, threwinvalidAnthropicStream(...)— a non-retryableProviderRequestErrorwith status 200. But the Anthropic provider always requests fine-grained tool streaming, under whichpartial_jsonis not guaranteed to be valid JSON when the response is cut bymax_tokens.stop_reasononly arrives later inmessage_delta, so at the point of the throw the parser had the information to defer and did not use it.The internal-agent replay relay then replaced that error with a fixed string, producing the opaque
Provider replay turn failed before its boundary— the incident in veryfront-issue-inbox#1475.Fix
Defer, then classify. The parser records a pending tool-input parse failure instead of throwing, and resolves it once
stop_reasonis known: amax_tokensstop raises a typedPROVIDER_OUTPUT_TRUNCATED; anything else keeps the original strict error.Stop masking. The relay forwards the real classified error rather than a fixed string.
Review findings fixed in the rework
src/agent/runtime/index.ts— the junction between the parser's typed truncation and the internal-agentRunError— had zero coverage. Reverting it alone leftrun-stream.test.ts69/69 green. Now covered by a test that drives the real runtime end to end; reverting the wiring reproduces the exact staging fingerprint and fails.validateCompletiontruncation arm gained a test via the client tool-use read-timeout path.Decision needed before merge
PROVIDER_OUTPUT_TRUNCATEDlands terminal (non-retryable). The issue text asked for a "typed retryable error".Adding it to
CURATED_PROVIDER_FAILURE_CODESmakesresolveKnownProviderTerminalErrorreturn non-null, sochild-lifecycle.ts:362anddurable-child-fork-execution.ts:282stop retrying a truncation that previously landed in the retryablePROVIDER_STREAM_ERRORbucket.The argument for terminal: re-running an identical request that overflowed its cap will overflow again, so retrying burns tokens to reach the same place. The argument against: truncation is not strictly deterministic — above temperature 0 a retry could produce a shorter tool input and succeed.
Stated explicitly in CHANGELOG.md and the commit body rather than slipped in. Please confirm the direction.
Note on the demo
This makes the failure diagnosable; it does not stop it. The 4096 cap that causes the truncation is veryfront-code#4514 (veryfront-issue-inbox#1480).
Refs veryfront/veryfront-issue-inbox#1467
Summary by CodeRabbit
PROVIDER_OUTPUT_TRUNCATED.