fix: flag output-token-truncated turns that carry no tool call - #477
Conversation
toolsCondition only inspects tool_calls, so a plain-text/reasoning turn cut off at the provider's output token ceiling (finish_reason: length / stop_reason: max_tokens, etc.) has nothing for it to catch and routes to END exactly like an ordinary completion. assertNotTruncatedToolCall only guards the tool-call case, so this path was silently treated as finished — hosts persisted a genuinely cut-off answer as complete, with no error and no unfinished flag. routeMessage now checks getTruncationStopReason on the last message when toolsCondition resolves to END, and sets a new outputTruncatedIncomplete flag on StandardGraph (exposed via Run.getOutputTruncated()). Kept separate from preemptIncomplete/preemptHaltReason since the latter has a side effect at each model node's entry (declining the call) that must stay scoped to the preempt/seal machinery. FakeChatModel gains an opt-in finalChunkGenerationInfo so tests can drive a truncated finish through a real Run/StandardGraph instead of asserting against internals.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
I found one cross-seam issue during maintainer review: the new flag survived cleanup, but the SDK's own AgentSession and Stop hooks would still classify the turn as completed because they consume getHaltReason(). Commit ad18af6 now surfaces output_truncated through that existing terminal contract while preserving the separate graph flag. Focused Jest, TypeScript, and ESLint checks pass. @codex review Please review the current PR head ad18af6. Confirm that this exact commit is the reviewed commit and ignore findings that apply only to earlier heads. |
![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: ad18af69c5
ℹ️ 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".
|
Codex Review: Didn't find any major issues. Breezy! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
2 participants
toolsCondition only inspects tool_calls, so a plain-text/reasoning turn cut off at the provider's output token ceiling (finish_reason: length / stop_reason: max_tokens, etc.) has nothing for it to catch and routes to END exactly like an ordinary completion. assertNotTruncatedToolCall only guards the tool-call case, so this path was silently treated as finished — hosts persisted a genuinely cut-off answer as complete, with no error and no unfinished flag.
routeMessage now checks getTruncationStopReason on the last message when toolsCondition resolves to END, and sets a new outputTruncatedIncomplete flag on StandardGraph (exposed via Run.getOutputTruncated()). Kept separate from preemptIncomplete/preemptHaltReason since the latter has a side effect at each model node's entry (declining the call) that must stay scoped to the preempt/seal machinery.
FakeChatModel gains an opt-in finalChunkGenerationInfo so tests can drive a truncated finish through a real Run/StandardGraph instead of asserting against internals.