Skip to content

[Bugfix][Frontend] Report Cohere stop sequences correctly - #51556

Merged
DarkLight1337 merged 2 commits into
vllm-project:mainfrom
taking-lying-flat:agent/fix-cohere-stop-sequence
Aug 11, 2026
Merged

DarkLight1337 merged 2 commits into
vllm-project:mainfrom
taking-lying-flat:agent/fix-cohere-stop-sequence

Conversation

@taking-lying-flat

Copy link
Copy Markdown
Contributor

Summary

  • report Cohere v2 stop-sequence termination as STOP_SEQUENCE instead of COMPLETE
  • preserve the OpenAI-compatible stop_reason through the Cohere streaming state machine
  • distinguish matched string stop sequences from EOS and integer stop-token IDs
  • add non-streaming and streaming regression coverage

Root cause

vLLM represents EOS, stop-token, and stop-string termination with finish_reason="stop". A matched stop string is carried separately in stop_reason, but the Cohere v2 conversion only inspected finish_reason. As a result, its existing STOP_SEQUENCE mapping was unreachable in both response paths.

User impact

Cohere v2 clients can now distinguish a configured stop-sequence match from ordinary completion in both regular responses and SSE message-end events.

Why this is not duplicating an existing PR

I searched open pull requests using cohere stop_reason, cohere stop sequence, and Cohere STOP_SEQUENCE, and found no open PR covering this behavior. PR #45807 addresses the analogous Anthropic Messages API conversion, not the Cohere v2 endpoint or its streaming state.

Testing

.venv/bin/python -m pytest -q \
  tests/entrypoints/cohere/test_serving_conversion.py \
  tests/entrypoints/cohere/test_serving_streaming.py \
  tests/entrypoints/cohere/test_protocol.py

Result: 150 passed.

uvx ruff check \
  vllm/entrypoints/cohere/serving.py \
  tests/entrypoints/cohere/test_serving_conversion.py \
  tests/entrypoints/cohere/test_serving_streaming.py

uvx ruff format --check \
  vllm/entrypoints/cohere/serving.py \
  tests/entrypoints/cohere/test_serving_conversion.py \
  tests/entrypoints/cohere/test_serving_streaming.py

Result: all checks passed; all three files were already formatted.

Model evaluation

Not run (not applicable). This change only maps already-produced termination metadata to the Cohere wire enum; it does not change model execution, token generation, or accuracy.

AI Assistance

AI assistance was used to investigate the lifecycle mismatch, construct regression cases, draft the implementation, and review counterexamples. I reviewed the final diff and the test results below.

The submitting author reviewed every changed line in the final diff.

Co-authored-by: OpenAI Codex <codex@openai.com>
Signed-off-by: cherry77-cloud <1615405@qq.com>
@mergify mergify Bot added frontend bug Something isn't working labels Aug 9, 2026
@taking-lying-flat
taking-lying-flat marked this pull request as ready for review August 9, 2026 15:04

@claude claude 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.

Claude Code Review

This pull request is from a fork — automated review is disabled. A repository maintainer can comment @claude review to run a one-time review.

@DarkLight1337 DarkLight1337 self-assigned this Aug 10, 2026
@DarkLight1337
DarkLight1337 enabled auto-merge (squash) August 10, 2026 23:12
@github-actions github-actions Bot added the ready ONLY add when PR is ready to merge/full CI is needed label Aug 10, 2026
@DarkLight1337

Copy link
Copy Markdown
Member

/ci run

@github-actions

Copy link
Copy Markdown

✅ Triggered Buildkite CI #83232 for commit 757a50264d76.

@taking-lying-flat

Copy link
Copy Markdown
Contributor Author

The previous full CI run (Buildkite #83232) was triggered for commit 757a502. I subsequently merged main into this branch, which changed the current HEAD to 3a3cc93. The checks currently attached to the new HEAD are passing, but the full Buildkite CI run is still associated with the old commit.

Sorry for invalidating the previous run. Could a maintainer please trigger full CI again for the current HEAD?

@DarkLight1337

Copy link
Copy Markdown
Member

/ci run

@github-actions

Copy link
Copy Markdown

✅ Triggered Buildkite CI #83283 for commit 3a3cc930bb21.

@DarkLight1337
DarkLight1337 merged commit 65b7662 into vllm-project:main Aug 11, 2026
62 checks passed
@taking-lying-flat
taking-lying-flat deleted the agent/fix-cohere-stop-sequence branch August 11, 2026 05:17
zyp2014 pushed a commit to zyp2014/vllm that referenced this pull request Aug 21, 2026
…ct#51556)

Signed-off-by: cherry77-cloud <1615405@qq.com>
Co-authored-by: OpenAI Codex <codex@openai.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working frontend ready ONLY add when PR is ready to merge/full CI is needed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants