[Bugfix][Frontend][gpt-oss] Return raw output when Harmony parser ends non-terminal

[Achyuthan-S]

Purpose

Fixes #45736. When GPT-OSS Harmony output ends in a non-terminal parser
state — e.g. a malformed final channel that omits the <|message|>
delimiter, trapping the body in the header — vLLM silently returned
content: null with finish_reason="stop" and completion_tokens > 0,
dropping the generated answer with no exception, log, or recovery.

This implements the consensus from #45736 (after #46437 added
flush()/process_eos()): catch the HarmonyError, return the raw unparsed
output as content, keep a standard finish_reason, and log a warning for
observability. No 4xx/5xx and no out-of-spec finish_reason.

Root cause

flush() ran process_eos() inside contextlib.suppress(HarmonyError),
swallowing the library's own non-terminal signal (the # TODO: Consider reraising left by #46437). parse() then returned content=None and the
serving layer emitted a clean stop.

Changes

vllm/parser/harmony.py:

• flush() re-raises HarmonyError after resetting parser state for the next
  turn.
• parse() catches it and returns the raw model_output as content.
• parse_delta() catches it, returns DeltaMessage(content=delta_text), and
  resets the tool-call index.
• A logger.warning is emitted at each fallback site.

tests/parser/test_harmony.py:

• New malformed-final tests for both parse() and parse_delta().
• Updated the flush EOS test to assert the new raise-and-reset contract.
• Fixed get_model_output_tokens to render with render_conversation instead
  of render_conversation_for_completion, dropping the trailing next-turn
  <|start|>assistant so synthetic streams match real output.token_ids. That
  trailing generation prompt left the parser non-terminal for every turn
  (previously masked by contextlib.suppress), which would otherwise make
  well-formed turns look malformed.

Test plan

.venv/bin/python -m pytest tests/parser/test_harmony.py -v
.venv/bin/pre-commit run --files vllm/parser/harmony.py tests/parser/test_harmony.py
.venv/bin/pre-commit run mypy-3.12 --files vllm/parser/harmony.py --hook-stage manual

Result: 39 passed; ruff, typos, and mypy (3.10 + 3.12) all pass.
(Local-only teardown errors come from an unrelated Apple-MPS
torch.accelerator.empty_cache() call in the shared conftest cleanup — they are
ERROR at teardown, not test failures, and do not occur on CI.)

Notes

• On a malformed turn the structured reasoning field is dropped; the reasoning
  text still appears inside the recovered raw content. This matches the agreed
  "return what we can of the unparsed output" approach.
• Streaming recovery is best-effort: the trapped body was never emitted as
  deltas, so the final delta surfaces the raw delta text plus the warning.
• Builds on the merged [Frontend][Gpt-oss] Use process_eos() to flush Harmony Parser outputs. #46437 and supersedes the closed warn-only PR [Bugfix][Frontend] Warn on silent GPT-OSS Harmony non-terminal parse drops #45796 —
  not a duplicate.

cc @yzong-rh @sfeng33

[claude]

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.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[sfeng33]

cc @bbrowning @yzong-rh

[yzong-rh]

Thanks for the quick turnaround.

Looks good. Could dedup separate logger.warning calls and remove extra comments (the content of the warning and the test names document the fix quite well already).

tests/parser/test_harmony.py passes for me.

[yzong-rh]

I think try ... finally would be preferable over storing the eos_error then re-raising it? Is there a reason you did it this way?

[Achyuthan-S]

No good reason — yours is cleaner. Switched to try/finally: process_eos() + poll in the try, the warning + re-raise in except HarmonyError, and the parser reset in finally so it always runs. Dropped the stored eos_error.

[yzong-rh]

You could consolidate logger.warning in flush() itself. No need for two separate warnings for parse() and parse_delta imo when the cause is the same.

[Achyuthan-S]

Done — there's now a single logger.warning in flush() (same cause for both paths), and parse()/parse_delta() just catch and fall back.

[Achyuthan-S]

Thanks for the quick turnaround.

Looks good. Could dedup separate logger.warning calls and remove extra comments (the content of the warning and the test names document the fix quite well already).

tests/parser/test_harmony.py passes for me.

Thanks @yzong-rh! Addressed everything: consolidated the warning into flush(), switched to try/finally, and removed the extra comments. tests/parser/test_harmony.py still passes (39) and ruff + mypy are clean.

[Achyuthan-S]

@yzong-rh thank you for the support, I would appreciate it if you assign me to any other interesting issues too
I would love to contribute more , thanks.

[yzong-rh]

cc @sfeng33 or @bbrowning could you add ready status so CI could run?