Skip to content

feat(agent): surface a normalized finishReason on the agent block - #6302

Open
waleedlatif1 wants to merge 2 commits into
stagingfrom
feat/agent-finish-reason
Open

feat(agent): surface a normalized finishReason on the agent block#6302
waleedlatif1 wants to merge 2 commits into
stagingfrom
feat/agent-finish-reason

Conversation

@waleedlatif1

Copy link
Copy Markdown
Collaborator

Summary

  • Adds finishReason as a declared agent-block output so a workflow can branch on why generation stopped — most importantly length, meaning the model was truncated by the token cap and its answer is partial.
  • Normalized across every provider. Truncation is length on OpenAI, max_output_tokens on the Responses API, max_tokens on Anthropic and Bedrock, and MAX_TOKENS on Gemini; a workflow shouldn't have to enumerate those.
  • Vocabulary: 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 model trace segment (ProviderTimingSegment.finishReason, populated via enrichLastModelSegment*). 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_call
  • Responses.incomplete_details.reason: max_output_tokens | content_filter
  • Anthropic StopReason: end_turn | max_tokens | stop_sequence | tool_use | pause_turn | refusal | model_context_window_exceeded
  • Gemini FinishReason: STOP | MAX_TOKENS | SAFETY | RECITATION | LANGUAGE | OTHER | BLOCKLIST | PROHIBITED_CONTENT | SPII | MALFORMED_FUNCTION_CALL | IMAGE_* | UNEXPECTED_TOOL_CALL | NO_IMAGE
  • Bedrock StopReason: end_turn | tool_use | max_tokens | stop_sequence | content_filtered | guardrail_intervened | malformed_model_output | malformed_tool_use | model_context_window_exceeded

The 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 to other rather than throwing, so a provider adding an enum case cannot fail a run.

Type of Change

  • New feature

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-validation clean.

Context

Requested by a customer whose agent hit a runaway generation. Capping max_output_tokens stops 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

  • Code follows project style guidelines
  • Self-reviewed my changes
  • Tests added/updated and passing
  • No new warnings introduced
  • I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

@vercel

vercel Bot commented Aug 5, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
docs Skipped Skipped Aug 5, 2026 11:38pm

Request Review

@cursor

cursor Bot commented Aug 5, 2026

Copy link
Copy Markdown

PR Summary

Low Risk
Additive optional output and normalization only; existing provider paths and trace raw strings are unchanged.

Overview
Adds a finishReason output on the Agent block so workflows can branch on why generation stopped—especially length when output was cut by a token cap and the answer may be partial.

Introduces normalizeFinishReason with a cross-provider map (OpenAI, Anthropic, Gemini, Bedrock spellings → stop, length, tool_calls, content_filter, error, other). The agent handler reads the raw reason from the last model timing segment (already populated for traces) and normalizes it when building block metadata—no changes to individual provider response builders. ProviderResponse optionally documents the same normalized type.

Vitest covers absent values, every mapped family, case/whitespace, and unknown → other without throwing.

Reviewed by Cursor Bugbot for commit 7576b19. Configure here.

@greptile-apps

greptile-apps Bot commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR declares a normalized agent finishReason output, adds cross-provider normalization, and derives the value from model timing segments.

  • Adds the agent-block output and normalized finish-reason vocabulary.
  • Maps raw OpenAI, Anthropic, Bedrock, and Gemini reasons while preserving unknown values as other.
  • Adds provider-focused normalization tests.

Confidence Score: 4/5

The 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

Important Files Changed

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]
Loading

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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 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.

Suggested change
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`,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 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!

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.

Fix All in Cursor

❌ 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)),

Copy link
Copy Markdown

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

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)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 7576b19. Configure here.

@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

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: error exists in the engine enum but isn't reachable through our path).

Result: zero mismappings across 23 providers, but 12 values from 9 providers were falling through to other. Now added — all pure additions, no existing key remapped:

  • model_length → length (Mistral's model-side max, distinct from caller max_tokens)
  • eos, eos_token → stop (Together, LiteLLM/HuggingFace — this is the routine success path on open-weight models, previously unclassified)
  • error, insufficient_system_resource, network_error → error (Mistral/OpenRouter/Together/Fireworks, DeepSeek, Z.ai)
  • sensitive → content_filter (Z.ai GLM)
  • model_armor, escalation → content_filter (Vertex/Gemini; absent from the @google/genai TS enum, so they arrive untyped)
  • malformed_response, missing_thought_signature → error (Gemini)
  • abort, repetition, too_many_tool_calls → other (documenting the intended bucket; zero runtime change)

Independently verified the two highest-impact ones against the vendored SDK rather than taking the sweep's word: @mistralai/mistralai declares stop | length | model_length | error | tool_calls, and Together routes through openai-compat.

Behaviour delta worth naming: eos (other → stop) and error (other → error) will move real traffic between buckets for anyone branching on the normalized value. Both are corrections — eos currently makes a successful Together generation read as unclassified.

Two judgment calls for a human to ratify: missing_thought_signature (error vs other — it's a request-side defect, not malformed model output) and repetition (other vs error — vLLM treats it as a normal finish, which is why I kept other).

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 finish_reason verbatim from any upstream. The ?? 'other' fallback is what keeps those safe.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant