Skip to content

fix(responses): mark truncated output incomplete - #12182

Merged
rmccorm4 merged 11 commits into
mainfrom
rmccormick/dis-2514-responses-api-mark-max-output-truncation-incomplete
Jul 30, 2026
Merged

fix(responses): mark truncated output incomplete#12182
rmccorm4 merged 11 commits into
mainfrom
rmccormick/dis-2514-responses-api-mark-max-output-truncation-incomplete

Conversation

@rmccorm4

@rmccorm4 rmccorm4 commented Jul 26, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Map FinishReason::Length to incomplete response and item states.
  • Report max_output_tokens as the incomplete reason.
  • Emit response.incomplete instead of response.completed for truncated streams.
  • Add unary and streaming regression coverage.

Root cause

The Responses conversion path treated every terminal backend finish reason as successful completion, including length-limited generation.

Impact

Clients can distinguish complete output from output truncated by the configured maximum-token limit.

Validation

  • cargo test -p dynamo-llm protocols::openai::responses (72 passed)
  • cargo fmt --all --check
  • cargo clippy -p dynamo-llm --tests -- -D warnings

Linear: DIS-2514

Summary by CodeRabbit

  • Bug Fixes
    • Responses now accurately report when generation stops because the maximum output token limit is reached.
    • Incomplete responses include the appropriate status and reason, with affected message and function-call items marked as incomplete.
    • Streaming responses now emit a response.incomplete event instead of incorrectly indicating completion when output limits are reached.
  • Tests
    • Added coverage for incomplete non-streaming and streaming responses caused by output limits.

Signed-off-by: Ryan McCormick <rmccormick@nvidia.com>
@github-actions github-actions Bot added fix frontend `python -m dynamo.frontend` and `dynamo-run in=http|text|grpc` labels Jul 26, 2026
@datadog-official

datadog-official Bot commented Jul 26, 2026

Copy link
Copy Markdown

Pipelines

⚠️ Warnings

🚦 2 Pipeline jobs failed

PR | backend-status-check   View in Datadog   GitHub Actions

PR | trtllm-runtime / Test cuda13.1, amd64   View in Datadog   GitHub Actions

Useful? React with 👍 / 👎

This comment will be updated automatically if new data arrives.
🔗 Commit SHA: 9c3b48a | Docs | Datadog PR Page | Give us feedback!

devin-ai-integration[bot]

This comment was marked as resolved.

Signed-off-by: Ryan McCormick <rmccormick@nvidia.com>
@rmccorm4 rmccorm4 added the Codex Related to Codex self-hosting compatibility label Jul 27, 2026
@rmccorm4
rmccorm4 marked this pull request as ready for review July 27, 2026 03:19
@rmccorm4
rmccorm4 requested review from a team as code owners July 27, 2026 03:19
@coderabbitai

coderabbitai Bot commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 650a4176-29e2-4f05-8c4b-c305fc662a6c

📥 Commits

Reviewing files that changed from the base of the PR and between 769ee07 and 50a946f.

📒 Files selected for processing (3)
  • lib/llm/src/protocols/openai/responses/mod.rs
  • lib/llm/src/protocols/openai/responses/stream_converter.rs
  • tests/serve/test_sglang.py

Walkthrough

Changes

The OpenAI Responses converters now represent FinishReason::Length as an incomplete response, including incomplete output-item statuses and max_output_tokens details. Streaming responses emit response.incomplete. SGLang aggregated tests disable reasoning in response payloads.

Responses output-limit handling

Layer / File(s) Summary
Batch conversion status propagation
lib/llm/src/protocols/openai/responses/mod.rs
Length finish reasons produce incomplete response status, unset completed_at, incomplete output items, and max_output_tokens details; unit coverage verifies the behavior.
Streaming termination propagation
lib/llm/src/protocols/openai/responses/stream_converter.rs
Streaming conversion tracks length termination, derives output and terminal statuses, and emits response.incomplete with matching response details; unit coverage verifies partial-text handling.
SGLang response payload setup
tests/serve/test_sglang.py
Aggregated streaming and non-streaming payloads set reasoning effort to none.

Estimated code review effort: 3 (Moderate) | ~25 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description covers the change and validation, but it doesn't follow the required template sections or include the required issue-link choice. Reformat the PR description to match the template: add Overview, Details, Where should reviewer start?, and the required Related Issues section with the issue link or no-issue checkbox.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title is concise and accurately summarizes the main change: treating truncated Responses output as incomplete.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

Comment @coderabbitai help to get the list of available commands.

devin-ai-integration[bot]

This comment was marked as resolved.

@rmccorm4
rmccorm4 enabled auto-merge (squash) July 28, 2026 05:44

Copy link
Copy Markdown
Contributor

Adding downstream validation context from a fork-side investigation.

We hit the same Responses adapter behavior when a backend terminal chat-completion chunk carries finish_reason: "length": the /v1/responses adapter currently reports the terminal Response as status: "completed" with incomplete_details: null, so clients that follow the Responses contract do not know they should continue or retry with a higher output-token budget.

Minimal sanitized repro shape:

{
  "request": {
    "model": "example-model",
    "input": [
      {"role": "user", "content": "Synthetic task that requires more output than the configured cap."}
    ],
    "max_output_tokens": 8
  },
  "backend_chat_completion_terminal_choice": {
    "finish_reason": "length",
    "message": {
      "role": "assistant",
      "content": "partial synthetic output"
    }
  }
}

Expected Responses result:

{
  "status": "incomplete",
  "completed_at": null,
  "incomplete_details": {"reason": "max_output_tokens"}
}

Expected streaming terminal event:

{
  "type": "response.incomplete",
  "response": {
    "status": "incomplete",
    "incomplete_details": {"reason": "max_output_tokens"}
  }
}

This aligns with the OpenAI Responses guidance for max-output-token exhaustion and is important for OpenAI Agents SDK / Codex-style clients, which inspect response.status == "incomplete" and incomplete_details.reason == "max_output_tokens" to decide whether continuation is needed. The repro above is intentionally synthetic and contains no customer prompts, model outputs, tenant/model IDs, request IDs, URLs, headers, or logs.

@michaelfeil

Copy link
Copy Markdown
Contributor

cc borjan on this issue. Once merged, we can pull it in.

@michaelfeil

Copy link
Copy Markdown
Contributor

@codex review with above context

chatgpt-codex-connector[bot]

This comment was marked as resolved.

@indrajit96 indrajit96 left a comment

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.

LGTM!

Just for mu understand why are we doing this?
So as to give unblock clients/benchmarks who rely on finish_reason to fetch tool calls?

…put-truncation-incomplete

Signed-off-by: Ryan McCormick <rmccormick@nvidia.com>
devin-ai-integration[bot]

This comment was marked as resolved.

Signed-off-by: Ryan McCormick <rmccormick@nvidia.com>
devin-ai-integration[bot]

This comment was marked as resolved.

Signed-off-by: Ryan McCormick <rmccormick@nvidia.com>
devin-ai-integration[bot]

This comment was marked as resolved.

rmccorm4 added 3 commits July 28, 2026 18:13
…esponses-api-mark-max-output-truncation-incomplete

Signed-off-by: Ryan McCormick <rmccormick@nvidia.com>
Signed-off-by: Ryan McCormick <rmccormick@nvidia.com>
Signed-off-by: Ryan McCormick <rmccormick@nvidia.com>
@rmccorm4
rmccorm4 requested a review from a team as a code owner July 29, 2026 01:41
@rmccorm4
rmccorm4 disabled auto-merge July 29, 2026 06:00
@rmccorm4
rmccorm4 enabled auto-merge (squash) July 29, 2026 06:56
Signed-off-by: Ryan McCormick <rmccormick@nvidia.com>
devin-ai-integration[bot]

This comment was marked as resolved.

@rmccorm4
rmccorm4 merged commit 245be42 into main Jul 30, 2026
181 of 183 checks passed
@rmccorm4
rmccorm4 deleted the rmccormick/dis-2514-responses-api-mark-max-output-truncation-incomplete branch July 30, 2026 02:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Codex Related to Codex self-hosting compatibility fix frontend `python -m dynamo.frontend` and `dynamo-run in=http|text|grpc` size/XL

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants