-
Notifications
You must be signed in to change notification settings - Fork 3.8k
feat(agent): surface a normalized finishReason on the agent block #6302
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: staging
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -62,7 +62,9 @@ import { | |||||
| canUseProviderLargeFilePath, | ||||||
| getInlineHydrationMaxBytes, | ||||||
| } from '@/providers/file-attachments.server' | ||||||
| import { normalizeFinishReason } from '@/providers/finish-reason' | ||||||
| import { isAutoModel, SIM_AUTO_MODEL_ID } from '@/providers/models' | ||||||
| import type { ProviderResponse } from '@/providers/types' | ||||||
| import { getProviderFromModel, transformBlockTool } from '@/providers/utils' | ||||||
| import type { SerializedBlock } from '@/serializer/types' | ||||||
| import { filterSchemaForLLM, type ToolSchema } from '@/tools/params' | ||||||
|
|
@@ -1487,9 +1489,32 @@ export class AgentBlockHandler implements BlockHandler { | |||||
| }, | ||||||
| providerTiming: result.timing, | ||||||
| cost: result.cost, | ||||||
| /** | ||||||
| * Read from the last model segment rather than threaded through each provider: | ||||||
| * every family already records its raw stop reason there for the trace, so this | ||||||
| * normalizes the value the enrichment layer has already collected. A run whose | ||||||
| * provider reported nothing simply has no reason, which stays distinct from one | ||||||
| * the vocabulary could not place. | ||||||
| */ | ||||||
| finishReason: normalizeFinishReason(this.lastModelSegmentFinishReasonImpl(result.timing)), | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| /** | ||||||
| * The raw stop reason from the most recent `model` segment. Later segments win | ||||||
| * because a tool loop appends one segment per turn and the final turn is the one | ||||||
| * that ended the generation. | ||||||
| */ | ||||||
| private lastModelSegmentFinishReasonImpl(timing: ProviderResponse['timing']): string | undefined { | ||||||
| const segments = timing?.timeSegments | ||||||
| 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 | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
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.
Suggested change
Knowledge Base Used: Workflow Executor |
||||||
| } | ||||||
| return undefined | ||||||
| } | ||||||
|
|
||||||
| private formatToolCall(tc: any) { | ||||||
| const toolName = stripCustomToolPrefix(tc.name) | ||||||
|
|
||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,111 @@ | ||
| /** | ||
| * @vitest-environment node | ||
| * | ||
| * The raw values are taken from the SDK enums this repo compiles against: | ||
| * `ChatCompletion.finish_reason`, Anthropic `StopReason`, Gemini `FinishReason`, | ||
| * and Bedrock `StopReason`. A provider adding a case must not fail a run, so an | ||
| * unknown value normalizes rather than throwing. | ||
| */ | ||
| import { describe, expect, it } from 'vitest' | ||
| import { normalizeFinishReason } from '@/providers/finish-reason' | ||
|
|
||
| describe('normalizeFinishReason', () => { | ||
| it('reports nothing when the provider reported nothing', () => { | ||
| expect(normalizeFinishReason(undefined)).toBeUndefined() | ||
| expect(normalizeFinishReason(null)).toBeUndefined() | ||
| expect(normalizeFinishReason('')).toBeUndefined() | ||
| }) | ||
|
|
||
| /** The case this exists for: one branch catches truncation on every provider. */ | ||
| it('maps every provider spelling of truncation to length', () => { | ||
| expect(normalizeFinishReason('length')).toBe('length') // OpenAI chat | ||
| expect(normalizeFinishReason('max_output_tokens')).toBe('length') // OpenAI Responses | ||
| expect(normalizeFinishReason('max_tokens')).toBe('length') // Anthropic, Bedrock | ||
| expect(normalizeFinishReason('MAX_TOKENS')).toBe('length') // Gemini | ||
| expect(normalizeFinishReason('model_context_window_exceeded')).toBe('length') | ||
| }) | ||
|
|
||
| it('maps natural completion to stop', () => { | ||
| expect(normalizeFinishReason('stop')).toBe('stop') | ||
| expect(normalizeFinishReason('STOP')).toBe('stop') | ||
| expect(normalizeFinishReason('end_turn')).toBe('stop') | ||
| expect(normalizeFinishReason('stop_sequence')).toBe('stop') | ||
| }) | ||
|
|
||
| it('maps tool stops to tool_calls', () => { | ||
| expect(normalizeFinishReason('tool_calls')).toBe('tool_calls') | ||
| expect(normalizeFinishReason('function_call')).toBe('tool_calls') | ||
| expect(normalizeFinishReason('tool_use')).toBe('tool_calls') | ||
| }) | ||
|
|
||
| it('maps every safety stop to content_filter', () => { | ||
| for (const raw of [ | ||
| 'content_filter', | ||
| 'content_filtered', | ||
| 'guardrail_intervened', | ||
| 'refusal', | ||
| 'SAFETY', | ||
| 'BLOCKLIST', | ||
| 'PROHIBITED_CONTENT', | ||
| 'SPII', | ||
| 'RECITATION', | ||
| 'IMAGE_SAFETY', | ||
| ]) { | ||
| expect(normalizeFinishReason(raw)).toBe('content_filter') | ||
| } | ||
| }) | ||
|
|
||
| it('maps malformed generations to error', () => { | ||
| expect(normalizeFinishReason('MALFORMED_FUNCTION_CALL')).toBe('error') | ||
| expect(normalizeFinishReason('malformed_tool_use')).toBe('error') | ||
| expect(normalizeFinishReason('malformed_model_output')).toBe('error') | ||
| }) | ||
|
|
||
| /** A pause is a continuation point, not an outcome the caller should branch on. */ | ||
| it('does not treat a server-tool pause as a stop', () => { | ||
| expect(normalizeFinishReason('pause_turn')).toBe('other') | ||
| }) | ||
|
|
||
| /** | ||
| * Values confirmed against each vendor's own enum by a per-provider documentation | ||
| * sweep. Several are the routine success path on open-weight models, so leaving | ||
| * them unclassified would misreport healthy runs. | ||
| */ | ||
| it('classifies the non-OpenAI vocabularies of compatible vendors', () => { | ||
| expect(normalizeFinishReason('model_length')).toBe('length') // Mistral | ||
| expect(normalizeFinishReason('eos')).toBe('stop') // Together | ||
| expect(normalizeFinishReason('eos_token')).toBe('stop') // LiteLLM/HuggingFace | ||
| expect(normalizeFinishReason('error')).toBe('error') // Mistral, OpenRouter, Together | ||
| expect(normalizeFinishReason('insufficient_system_resource')).toBe('error') // DeepSeek | ||
| expect(normalizeFinishReason('network_error')).toBe('error') // Z.ai | ||
| expect(normalizeFinishReason('sensitive')).toBe('content_filter') // Z.ai | ||
| }) | ||
|
|
||
| it('classifies the Gemini and Vertex values absent from the TS enum', () => { | ||
| expect(normalizeFinishReason('MODEL_ARMOR')).toBe('content_filter') // Vertex only | ||
| expect(normalizeFinishReason('ESCALATION')).toBe('content_filter') | ||
| expect(normalizeFinishReason('MALFORMED_RESPONSE')).toBe('error') | ||
| expect(normalizeFinishReason('MISSING_THOUGHT_SIGNATURE')).toBe('error') | ||
| }) | ||
|
|
||
| /** | ||
| * A server-aborted tool loop is not a request to execute tools, and a repetition | ||
| * cutoff is a normal finish rather than a failure — both would mislead a workflow | ||
| * branching on the value. | ||
| */ | ||
| it('does not overclaim on aborted or degenerate stops', () => { | ||
| expect(normalizeFinishReason('too_many_tool_calls')).toBe('other') | ||
| expect(normalizeFinishReason('repetition')).toBe('other') | ||
| expect(normalizeFinishReason('abort')).toBe('other') | ||
| }) | ||
|
|
||
| it('degrades an unrecognized value to other rather than throwing', () => { | ||
| expect(normalizeFinishReason('some_future_reason')).toBe('other') | ||
| expect(normalizeFinishReason('OTHER')).toBe('other') | ||
| expect(normalizeFinishReason('FINISH_REASON_UNSPECIFIED')).toBe('other') | ||
| }) | ||
|
|
||
| it('is insensitive to case and surrounding whitespace', () => { | ||
| expect(normalizeFinishReason(' Length ')).toBe('length') | ||
| }) | ||
| }) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,132 @@ | ||
| /** | ||
| * Why a model stopped generating, normalized across providers. | ||
| * | ||
| * Providers disagree on vocabulary for the same event — a truncated generation is | ||
| * `length` on OpenAI, `max_tokens` on Anthropic and Bedrock, and `MAX_TOKENS` on | ||
| * Gemini. Traces keep each provider's raw string because it is the ground truth for | ||
| * debugging; this normalized value exists so a workflow can branch on the outcome | ||
| * without enumerating every provider's spelling. | ||
| */ | ||
| export type AgentFinishReason = | ||
| /** Generation completed naturally, or hit a caller-supplied stop sequence. */ | ||
| | 'stop' | ||
| /** Truncated by a token limit — the model had more to say. */ | ||
| | 'length' | ||
| /** Stopped in order to call tools. */ | ||
| | 'tool_calls' | ||
| /** Blocked or refused by a safety system. */ | ||
| | 'content_filter' | ||
| /** The provider reported the generation itself as failed or malformed. */ | ||
| | 'error' | ||
| /** Reported, but not a case this vocabulary distinguishes. */ | ||
| | 'other' | ||
|
|
||
| /** | ||
| * Raw provider value → normalized reason, keyed on the lowercased string. | ||
| * | ||
| * 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`, | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
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! |
||
| * and Bedrock's `StopReason`. | ||
| */ | ||
| const NORMALIZED_BY_RAW = new Map<string, AgentFinishReason>([ | ||
| // OpenAI Chat Completions, and every OpenAI-compatible provider. | ||
| ['stop', 'stop'], | ||
| ['length', 'length'], | ||
| ['tool_calls', 'tool_calls'], | ||
| ['function_call', 'tool_calls'], | ||
| ['content_filter', 'content_filter'], | ||
|
|
||
| // OpenAI Responses reports truncation through `incomplete_details.reason`. | ||
| ['max_output_tokens', 'length'], | ||
|
|
||
| /** Mistral separates the model's own max length from the caller's `max_tokens`. */ | ||
| ['model_length', 'length'], | ||
|
|
||
| /** | ||
| * Natural end-of-sequence. Together and HuggingFace-backed proxies report the | ||
| * model's EOS token separately from a caller-supplied stop sequence, so without | ||
| * these the routine success path on open-weight models is unclassified. | ||
| */ | ||
| ['eos', 'stop'], | ||
| ['eos_token', 'stop'], | ||
|
|
||
| /** | ||
| * A generation the provider itself reported as failed. Mistral, OpenRouter, | ||
| * Together and Fireworks all spell this `error`; the DeepSeek and Z.ai values are | ||
| * the same class with a stated cause. | ||
| */ | ||
| ['error', 'error'], | ||
| ['insufficient_system_resource', 'error'], | ||
| ['network_error', 'error'], | ||
|
|
||
| /** Z.ai (GLM) sensitive-content block. */ | ||
| ['sensitive', 'content_filter'], | ||
|
|
||
| /** | ||
| * Cut short with no outcome to report, on vLLM and NIM-hosted models. vLLM treats | ||
| * a repetition cutoff as a normal finish rather than a failure, so it is not | ||
| * `error`. | ||
| */ | ||
| ['abort', 'other'], | ||
| ['repetition', 'other'], | ||
|
|
||
| // Anthropic Messages, shared by Bedrock's Converse API. | ||
| ['end_turn', 'stop'], | ||
| ['stop_sequence', 'stop'], | ||
| ['max_tokens', 'length'], | ||
| ['model_context_window_exceeded', 'length'], | ||
| ['tool_use', 'tool_calls'], | ||
| ['refusal', 'content_filter'], | ||
| /** A server-tool pause is a continuation point, not an outcome. */ | ||
| ['pause_turn', 'other'], | ||
|
|
||
| // Bedrock Converse additions. | ||
| ['content_filtered', 'content_filter'], | ||
| ['guardrail_intervened', 'content_filter'], | ||
| ['malformed_model_output', 'error'], | ||
| ['malformed_tool_use', 'error'], | ||
|
|
||
| /** | ||
| * Gemini. `STOP` and `MAX_TOKENS` already lowercase onto the entries above, so | ||
| * only the values with no counterpart elsewhere are listed here. Recitation is a | ||
| * content restriction, so it groups with the filters. | ||
| */ | ||
| ['safety', 'content_filter'], | ||
| ['blocklist', 'content_filter'], | ||
| ['prohibited_content', 'content_filter'], | ||
| ['spii', 'content_filter'], | ||
| ['recitation', 'content_filter'], | ||
| ['image_safety', 'content_filter'], | ||
| ['image_prohibited_content', 'content_filter'], | ||
| ['image_recitation', 'content_filter'], | ||
| ['malformed_function_call', 'error'], | ||
| ['unexpected_tool_call', 'error'], | ||
| ['malformed_response', 'error'], | ||
| ['missing_thought_signature', 'error'], | ||
| ['model_armor', 'content_filter'], | ||
| ['escalation', 'content_filter'], | ||
| /** A runaway tool loop the server aborted, not a request to execute tools. */ | ||
| ['too_many_tool_calls', 'other'], | ||
| ['language', 'other'], | ||
| ['other', 'other'], | ||
| ['no_image', 'other'], | ||
| ['image_other', 'other'], | ||
| ['finish_reason_unspecified', 'other'], | ||
| ]) | ||
|
|
||
| /** | ||
| * Normalizes a provider's raw stop reason. | ||
| * | ||
| * Returns `undefined` when the provider reported nothing, so an absent reason stays | ||
| * distinguishable from one the vocabulary could not place. An unrecognized value maps | ||
| * to `'other'` rather than throwing: a provider adding a case must not fail a run | ||
| * that otherwise succeeded. | ||
| */ | ||
| export function normalizeFinishReason( | ||
| raw: string | null | undefined | ||
| ): AgentFinishReason | undefined { | ||
| if (!raw) return undefined | ||
| return NORMALIZED_BY_RAW.get(raw.trim().toLowerCase()) ?? 'other' | ||
| } | ||
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.
Streaming omits finishReason output
Medium Severity
finishReasonis only attached increateResponseMetadata, which runs for non-streaming provider results. Streaming executions return throughprocessStreamingExecution/createStreamingExecutionand never copy the normalized reason onto the block output, even after model segments are enriched during drain. Workflows that branch on truncation therefore misslengthwhenever the agent runs with streaming enabled.Additional Locations (1)
apps/sim/executor/handlers/agent/agent-handler.ts#L1400-L1416Reviewed by Cursor Bugbot for commit 7576b19. Configure here.