Skip to content

Conversation

@qandrew
Copy link
Contributor

@qandrew qandrew commented Dec 5, 2025

Purpose

Refactor so we centralize all the files related to parsing for MCP

Test Plan

No functional changes, existing unit tests should pass

Test Result

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request refactors the codebase by moving harmony_utils.py into the vllm/entrypoints/openai/parser/ directory. This change helps to centralize parsing-related utilities for MCP, improving the project structure. The changes consist of file renames and corresponding import path updates across several files. I've reviewed the changes and they are correct and consistent with the refactoring goal. There are no functional changes, as expected. This is a good cleanup.

Signed-off-by: Andrew Xia <axia@fb.com>
@qandrew qandrew force-pushed the move-harmony-utils branch from a9fb82b to c8c8b4c Compare December 5, 2025 17:24
@qandrew qandrew marked this pull request as ready for review December 5, 2025 17:26
Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment on lines +22 to 25
from vllm.entrypoints.openai.parser.harmony_utils import (
get_encoding,
get_streamable_parser_for_assistant,
render_for_completion,

Choose a reason for hiding this comment

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

P1 Badge Preserve compatibility for harmony_utils import

Moving harmony_utils under vllm.entrypoints.openai.parser removes the previous vllm.entrypoints.harmony_utils module, but tests/entrypoints/openai/test_response_api_with_harmony.py still imports the old path at line 729, so running the responses test suite now fails immediately with an ImportError. Either update that call site or add a compatibility shim at the old module path to keep the tests runnable.

Useful? React with 👍 / 👎.

@DarkLight1337
Copy link
Member

cc @WoosukKwon @heheda12345

Copy link
Collaborator

@yeqcharlotte yeqcharlotte left a comment

Choose a reason for hiding this comment

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

LGTM

@github-project-automation github-project-automation bot moved this from To Triage to Ready in gpt-oss Issues & Enhancements Dec 6, 2025
@yeqcharlotte yeqcharlotte enabled auto-merge (squash) December 6, 2025 02:09
@github-actions github-actions bot added the ready ONLY add when PR is ready to merge/full CI is needed label Dec 6, 2025
fix
Signed-off-by: Andrew Xia <axia@fb.com>
auto-merge was automatically disabled December 6, 2025 09:00

Head branch was pushed to by a user without write access

@aarnphm aarnphm merged commit 421125d into vllm-project:main Dec 6, 2025
47 checks passed
@heheda12345
Copy link
Collaborator

sorry for my late review. But we need to update

vllm/.github/mergify.yml

Lines 164 to 180 in 17eb25e

- name: label-gpt-oss
description: Automatically apply gpt-oss label
conditions:
- label != stale
- or:
- files~=^examples/.*gpt[-_]?oss.*\.py
- files~=^tests/.*gpt[-_]?oss.*\.py
- files~=^tests/entrypoints/openai/test_response_api_with_harmony.py
- files~=^tests/entrypoints/test_context.py
- files~=^vllm/model_executor/models/.*gpt[-_]?oss.*\.py
- files~=^vllm/model_executor/layers/.*gpt[-_]?oss.*\.py
- files~=^vllm/entrypoints/harmony_utils.py
- files~=^vllm/entrypoints/tool_server.py
- files~=^vllm/entrypoints/tool.py
- files~=^vllm/entrypoints/context.py
- title~=(?i)gpt[-_]?oss
- title~=(?i)harmony
accordingly.

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

Labels

frontend gpt-oss Related to GPT-OSS models ready ONLY add when PR is ready to merge/full CI is needed tool-calling

Projects

Status: Done
Status: Done

Development

Successfully merging this pull request may close these issues.

5 participants