feat(agent): surface a normalized finishReason on the agent block - #6302
feat(agent): surface a normalized finishReason on the agent block#6302waleedlatif1 wants to merge 2 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
PR SummaryLow Risk Overview Introduces Vitest covers absent values, every mapped family, case/whitespace, and unknown → Reviewed by Cursor Bugbot for commit 7576b19. Configure here. |
Greptile SummaryThis PR declares a normalized agent
Confidence Score: 4/5The stale earlier-turn finish reason must be fixed before merging because it can send workflows down the wrong branch when the final provider turn reports no reason. The backward scan does not stop at the newest model segment, so an absent final reason can be replaced by an earlier tool-loop reason even though the output is documented as describing the final generation. Files Needing Attention: apps/sim/executor/handlers/agent/agent-handler.ts and apps/sim/providers/finish-reason.ts
|
| Filename | Overview |
|---|---|
| apps/sim/executor/handlers/agent/agent-handler.ts | Adds finish-reason extraction to agent output, but the reverse scan can select a stale earlier-turn reason when the final model segment has none. |
| apps/sim/providers/finish-reason.ts | Adds a comprehensive provider-independent normalization table; its ordinary line comments conflict with the repository TSDoc convention. |
| apps/sim/providers/finish-reason.test.ts | Covers known provider vocabularies, absent values, case normalization, and unknown values, but does not test final-segment selection across tool-loop turns. |
| apps/sim/blocks/blocks/agent.ts | Declares the new string output consistently with the block’s existing optional output fields. |
| apps/sim/providers/types.ts | Adds an optional normalized finish-reason field to ProviderResponse without changing existing provider behavior. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart LR
Provider[Provider response] --> Enrich[Enrich model timing segment]
Enrich --> Segments[Model and tool timing segments]
Segments --> Scan[Scan backward for finishReason]
Scan --> Normalize[Normalize provider vocabulary]
Normalize --> Output[Agent block finishReason output]
Reviews (1): Last reviewed commit: "feat(agent): surface a normalized finish..." | Re-trigger Greptile
| if (!segments) return undefined | ||
| for (let i = segments.length - 1; i >= 0; i--) { | ||
| const segment = segments[i] | ||
| if (segment.type === 'model' && segment.finishReason) return segment.finishReason |
There was a problem hiding this comment.
Stale reason crosses model turns
When the final model segment has no finish reason, this condition skips it and returns a reason from an earlier tool-loop turn, causing downstream workflow conditions to branch on a stale value instead of the documented absent result.
| if (segment.type === 'model' && segment.finishReason) return segment.finishReason | |
| if (segment.type === 'model') return segment.finishReason |
Knowledge Base Used: Workflow Executor
| * A single table rather than a per-provider mapper because the vocabularies do not | ||
| * collide: no raw value means one thing to one provider and something else to | ||
| * another. Sources are the SDK types this repo compiles against — | ||
| * `ChatCompletion.finish_reason`, Anthropic's `StopReason`, Gemini's `FinishReason`, |
There was a problem hiding this comment.
Non-TSDoc documentation comments
The new normalization implementation and tests use ordinary line comments for provider and test-case documentation, contrary to the repository requirement that documentation use TSDoc; converting this changed-code pattern keeps documentation and convention checks consistent.
Context Used: CLAUDE.md (source)
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 7576b19. Configure here.
| * provider reported nothing simply has no reason, which stays distinct from one | ||
| * the vocabulary could not place. | ||
| */ | ||
| finishReason: normalizeFinishReason(this.lastModelSegmentFinishReasonImpl(result.timing)), |
There was a problem hiding this comment.
Streaming omits finishReason output
Medium Severity
finishReason is only attached in createResponseMetadata, which runs for non-streaming provider results. Streaming executions return through processStreamingExecution / createStreamingExecution and never copy the normalized reason onto the block output, even after model segments are enriched during drain. Workflows that branch on truncation therefore miss length whenever the agent runs with streaming enabled.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 7576b19. Configure here.
|
Ran a per-provider validation sweep across all 25 provider IDs — each agent read our table, traced which family that provider routes through in our code, then checked that vendor's own SDK enum or API reference. Every reported gap went through an adversarial refutation pass before being accepted (one vLLM claim was correctly rejected: Result: zero mismappings across 23 providers, but 12 values from 9 providers were falling through to
Independently verified the two highest-impact ones against the vendored SDK rather than taking the sweep's word: Behaviour delta worth naming: Two judgment calls for a human to ratify: Not fully verified: sakana, nvidia, and xai publish no authoritative finish-reason enum — their coverage rests on inference from the backends they proxy. LiteLLM is unbounded by construction, since it copies |


Summary
finishReasonas a declared agent-block output so a workflow can branch on why generation stopped — most importantlylength, meaning the model was truncated by the token cap and its answer is partial.lengthon OpenAI,max_output_tokenson the Responses API,max_tokenson Anthropic and Bedrock, andMAX_TOKENSon Gemini; a workflow shouldn't have to enumerate those.stop|length|tool_calls|content_filter|error|other, plus absent when the provider reported nothing.Why this shape
Every provider family already records its raw stop reason on the last
modeltrace segment (ProviderTimingSegment.finishReason, populated viaenrichLastModelSegment*). Rather than thread a new field through ~10 response-construction sites across five families, this normalizes the value the enrichment layer already collects, at the single point where the agent handler builds its block output.Traces keep the raw provider string unchanged — it's the ground truth for debugging. Only the new block output is normalized.
Validation
Mappings were taken from the SDK enums this repo compiles against, not from prose docs:
ChatCompletion.finish_reason:stop | length | tool_calls | content_filter | function_callResponses.incomplete_details.reason:max_output_tokens | content_filterStopReason:end_turn | max_tokens | stop_sequence | tool_use | pause_turn | refusal | model_context_window_exceededFinishReason:STOP | MAX_TOKENS | SAFETY | RECITATION | LANGUAGE | OTHER | BLOCKLIST | PROHIBITED_CONTENT | SPII | MALFORMED_FUNCTION_CALL | IMAGE_* | UNEXPECTED_TOOL_CALL | NO_IMAGEStopReason:end_turn | tool_use | max_tokens | stop_sequence | content_filtered | guardrail_intervened | malformed_model_output | malformed_tool_use | model_context_window_exceededThe raw values don't collide across families — nothing means one thing to one provider and something else to another — so a single case-insensitive table is sufficient and no provider parameter needs threading.
No regressions
Purely additive: a new optional field on
ProviderResponse, a new declared block output, and one new read. No provider code paths change, and trace behaviour is untouched. An unrecognized value degrades tootherrather than throwing, so a provider adding an enum case cannot fail a run.Type of Change
Testing
9 tests covering every family's vocabulary, case-insensitivity, absent values, and unknown-value degradation. Verified fail-detectable by breaking the lowercasing (5 red) and by remapping
max_tokens(1 red).Providers + agent-handler suites: 110 files / 1425 tests passing. Typecheck, lint, and
check:api-validationclean.Context
Requested by a customer whose agent hit a runaway generation. Capping
max_output_tokensstops the 4.6-minute hang, but a capped prose runaway completes without error, so their fallback path can't catch it.finishReason === 'length'is the branch they need.Checklist