-
Notifications
You must be signed in to change notification settings - Fork 3.7k
fix: handle non-text response parts in GitHub action #6173
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: dev
Are you sure you want to change the base?
Conversation
0f60493 to
0396a9e
Compare
|
This is good to go. Ran into this a bunch of times over the last week and figured I'd get a patch up. |
|
/review |
|
lgtm |
|
wondering if we should just send another prompt to the agent and tell it to summarize the convo or something to keep it "cleaner"? If the last message isn't a text part? |
|
@rekram1-node not a bad idea: let me stub that out and test locally. |
56b66da to
4fda449
Compare
|
Here's a take on attempting to re-summarize @rekram1-node - e.g. we make one (1) attempt to summarize the output when there are no
|
| const text = extractResponseText(result.parts) | ||
| if (text) return text | ||
|
|
||
| // No text part (tool-only or reasoning-only) - ask agent to summarize |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@rekram1-node FYI
| providerID, | ||
| modelID, | ||
| }, | ||
| tools: { "*": false }, // Disable all tools to force text response |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also note - we explicitly disable tools here as we only want to summarize. This feels right, but not sure if it's surprising to users.
This PR fixes the
Failed to parse the text responseerror inopencode github runwhen the LLM responds with only tool calls or reasoning parts (no text). This is rare (about ~5/100 runs?) but frustrating as the entire invocation is discarded.The new
extractResponseText()function handles response parts with fallback logic:[Reasoning]prefix (for thinking models like o1, Claude extended thinking)specifically:
todowriteonly) causedfindLastto returnundefined, throwing an errorPart types found: [tool, step-start]repro
This is easy to repro directly by just prompting opencode via the GitHub Action:
before (fails):
or:
after (succeeds):
tests:
extractResponseTexttested for text parts, reasoning fallback, tool-only responsesrelated:
Failed to parse the text responsewith github workflow #2240 - SameFailed to parse the text responseerror (closed but not fully addressed)storage.ts(different root cause)