fix(session): fail empty assistant responses instead of recording success - #41898
Open
patil2001 wants to merge 1 commit into
Open
fix(session): fail empty assistant responses instead of recording success#41898patil2001 wants to merge 1 commit into
patil2001 wants to merge 1 commit into
Conversation
…cess A terminal assistant step that produces no usable output (no text, no tool calls, no non-empty reasoning) was recorded as a successful execution. This surfaced as a silent no-op turn for the user. Treat a settled step with no usable output as a typed empty-response failure so clients can surface the error instead of stopping silently.
Contributor
|
The following comment was made by an LLM, it may be inaccurate: Based on the search results, I found a potentially related PR that should be reviewed: Related PRPR #37843: This appears to address a very similar issue — failing when providers return empty output. It's worth checking if this PR overlaps with or duplicates the current PR #41898, as both deal with handling empty provider responses in the core session logic. The other PRs found (like #40531, #37732, #37110, #26167) are related to empty responses in various contexts but appear to be addressing different scope areas (opencode retries, model responses, tool loops) rather than the specific session V2 execution flow that #41898 targets. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Issue for this PR
Closes #37372
Type of change
What does this PR do?
V2 records a reasoning-only assistant response with no visible text and no tool calls as a successful execution. When a provider settles a step with an empty completion (six output tokens but no publishable content), the runner emits
session.step.ended.1withfinish: "stop"followed bysession.execution.succeeded.1— the user gets neither an answer nor a surfaced failure, and the client stops its typing state silently.The fix makes the runner judge a settled step by its usable assistant output instead of its token count. The event publisher now records whether a step produced non-empty text, non-empty reasoning, or a tool call. When a step settles with no usable output, the runner publishes
Step.Failedwith a typed"Provider returned an empty response"error rather than a successfulStep.Ended, so clients can surface the failure to the user. Non-empty reasoning-only turns (which providers legitimately emit and existing tests cover) still settle as a successful step. The two runner tests that modeled a completely empty second turn now expect the error.How did you verify your code works?
bun test --timeout 30000inpackages/corepasses (1080 pass / 0 fail), including new publisher tests for usable output (text / reasoning / tool call) and empty output (empty text fragment, empty reasoning).tsgo --noEmitclean inpackages/core.Screenshots / recordings
Not a UI change.
Checklist