Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions nemo_gym/_moved_paths.py
Original file line number Diff line number Diff line change
Expand Up @@ -838,16 +838,27 @@ def _names(new: str, since: str, names: str) -> dict[str, MovedPath]:
_listed.update(_renamed)

# ResponsesConverter renamed ChatCompletionsConverter: AnthropicConverter also converts to and from the Responses API.
# The VLLMConverter aliases go with it: the LiteLLM and Azure OpenAI model servers use the converter too.
# The chat_completions_converter entries also serve its old module path, nemo_gym.responses_converter.
_CONVERTER = "nemo_gym.models.chat_completions_converter"
_VLLM_CONVERTER_ALIASES = {
"VLLMConverter": MovedPath(new=f"{_CONVERTER}:ChatCompletionsConverter", since="0.8.0"),
"VLLMConverterResponsesToChatCompletionsState": MovedPath(
new=f"{_CONVERTER}:ChatCompletionsConverterState", since="0.8.0"
),
}
_RENAMED_CONVERTER_NAMES: dict[str, dict[str, MovedPath]] = {
_CONVERTER: {
"ResponsesConverter": MovedPath(new=f"{_CONVERTER}:ChatCompletionsConverter", since="0.8.0"),
"ResponsesConverterState": MovedPath(new=f"{_CONVERTER}:ChatCompletionsConverterState", since="0.8.0"),
**_VLLM_CONVERTER_ALIASES,
},
"nemo_gym.models": {
"ResponsesConverter": MovedPath(new=f"{_CONVERTER}:ChatCompletionsConverter", since="0.8.0"),
**_VLLM_CONVERTER_ALIASES,
},
# Servers imported the aliases from the vLLM model server, which re-exported them.
"responses_api_models.vllm_model.app": _VLLM_CONVERTER_ALIASES,
"nemo_gym.models.token_capture.completion": {
"ResponsesConverter": MovedPath(new=f"{_CONVERTER}:ChatCompletionsConverter", since="0.8.0"),
},
Expand Down
4 changes: 0 additions & 4 deletions nemo_gym/models/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,6 @@
)
from nemo_gym.models.chat_completions_converter import (
ChatCompletionsConverter,
VLLMConverter,
VLLMConverterResponsesToChatCompletionsState,
chat_completion_usage_to_response_usage,
message_content_to_text,
split_responses_input_output_items,
Expand Down Expand Up @@ -78,8 +76,6 @@
"PROPAGATED_REFUSAL_CODE_ATTRIBUTE": "base_responses_api_model",
"PolicyModelParticipant": "_checkpoint",
"SimpleResponsesAPIModel": "base_responses_api_model",
"VLLMConverter": "chat_completions_converter",
"VLLMConverterResponsesToChatCompletionsState": "chat_completions_converter",
"VLLMWorkerCaptureHandler": "token_capture.external_capture",
"chat_completion_usage_to_response_usage": "chat_completions_converter",
"context_overflow_error_fields": "base_responses_api_model",
Expand Down
6 changes: 1 addition & 5 deletions nemo_gym/models/chat_completions_converter.py
Original file line number Diff line number Diff line change
Expand Up @@ -1417,9 +1417,5 @@ def split_responses_input_output_items(
return items[:split_at], items[split_at:]


# Backward-compatible aliases
VLLMConverter = ChatCompletionsConverter
VLLMConverterResponsesToChatCompletionsState = ChatCompletionsConverterState

# Names this module had before ResponsesConverter was renamed ChatCompletionsConverter, with a warning.
# Names this module had before the converter was renamed ChatCompletionsConverter, with a warning.
__getattr__ = moved_names_getattr(__name__)
7 changes: 5 additions & 2 deletions responses_api_models/vllm_model/app.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
from openai.types.shared import ReasoningEffort
from pydantic import Field, PositiveFloat, PrivateAttr, TypeAdapter, ValidationError, model_validator

from nemo_gym._moved_paths import moved_names_getattr
from nemo_gym.client import MAX_NUM_TRIES, SESSION_ID_KEY, NeMoGymAsyncOpenAI
from nemo_gym.client.http_client import _redacted_url
from nemo_gym.core import (
Expand All @@ -55,8 +56,6 @@
ChatCompletionsConverter,
ExternalCaptureHandler,
SimpleResponsesAPIModel,
VLLMConverter, # noqa: F401 # re-exported for importers of this module
VLLMConverterResponsesToChatCompletionsState, # noqa: F401
context_overflow_error_fields,
is_propagated_refusal,
make_external_capture_handler,
Expand Down Expand Up @@ -2575,6 +2574,10 @@ def _note_endpoint_success(self, client: NeMoGymAsyncOpenAI) -> None:
health.failed_at = None


# The VLLMConverter aliases this module re-exported, with a warning naming ChatCompletionsConverter.
__getattr__ = moved_names_getattr(__name__)


if __name__ == "__main__":
VLLMModel.run_webserver()
elif is_nemo_gym_fastapi_entrypoint(__file__):
Expand Down
14 changes: 13 additions & 1 deletion scripts/migrate_imports.py
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,13 @@ def __init__(
moved_to = self.module_path(module)
if moved_to != module:
self.names.setdefault(moved_to, {}).update(moved)
# A name renamed in a module that moved earlier is listed under the new path, and is served at the old one.
for module, moved in list(self.names.items()):
for old, new in self.modules.items():
if module == new or module.startswith(new + "."):
listed = self.names.setdefault(old + module[len(new) :], {})
for name, target in moved.items():
listed.setdefault(name, target)
dotted = {old: new for old, new in self.modules.items()}
for module, moved in self.names.items():
for name, (new_module, new_name) in moved.items():
Expand Down Expand Up @@ -361,7 +368,12 @@ def span(node: ast.AST) -> tuple[int, int]:
)
continue
# `import nemo_gym.old` needs no statement rewrite: the dotted-path pass below covers it.
if not (isinstance(node, ast.ImportFrom) and node.module and node.module.split(".")[0] == "nemo_gym"):
# A module outside nemo_gym, such as a server module, is rewritten only when it lists moved names.
if not (
isinstance(node, ast.ImportFrom)
and node.module
and (node.module.split(".")[0] == "nemo_gym" or node.module in self.names)
):
continue
start, end = span(node)
if any(KEEP_LINE_MARKER in lines[i] for i in range(node.lineno - 1, node.end_lineno)):
Expand Down
12 changes: 0 additions & 12 deletions tests/unit_tests/models/test_chat_completions_converter.py
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,6 @@
from nemo_gym.models.chat_completions_converter import (
ChatCompletionsConverter,
ChatCompletionsConverterState,
VLLMConverter,
VLLMConverterResponsesToChatCompletionsState,
_usage_detail,
chat_completion_usage_to_response_usage,
split_responses_input_output_items,
Expand Down Expand Up @@ -112,16 +110,6 @@ def _fixed_uuid(monkeypatch: pytest.MonkeyPatch):
monkeypatch.setattr("nemo_gym.models.chat_completions_converter.uuid4", lambda: FakeUUID())


# ===========================================================================
# Backward-compatible aliases
# ===========================================================================


def test_backwards_compatible_aliases():
assert VLLMConverter is ChatCompletionsConverter
assert VLLMConverterResponsesToChatCompletionsState is ChatCompletionsConverterState


def test_usage_detail_ignores_ambiguous_top_level_names():
usage = {"cached_tokens": 99, "cached_input_tokens": 7, "reasoning_tokens": 88, "reasoning_output_tokens": 3}
assert _usage_detail(usage, "prompt_tokens_details", "cached_tokens", "cached_input_tokens") == 7
Expand Down
21 changes: 21 additions & 0 deletions tests/unit_tests/test_moved_paths.py
Original file line number Diff line number Diff line change
Expand Up @@ -787,6 +787,27 @@ def test_package_facade_warns_once_for_a_renamed_name() -> None:
("nemo_gym.models", "ResponsesConverter", "ChatCompletionsConverter"),
("nemo_gym.models.token_capture.completion", "ResponsesConverter", "ChatCompletionsConverter"),
("nemo_gym.token_id_capture.completion", "ResponsesConverter", "ChatCompletionsConverter"),
# The vLLM-named aliases, released in 0.7.0 at nemo_gym.responses_converter and the vLLM model server.
("nemo_gym.responses_converter", "VLLMConverter", "ChatCompletionsConverter"),
(
"nemo_gym.responses_converter",
"VLLMConverterResponsesToChatCompletionsState",
"ChatCompletionsConverterState",
),
("nemo_gym.models.chat_completions_converter", "VLLMConverter", "ChatCompletionsConverter"),
(
"nemo_gym.models.chat_completions_converter",
"VLLMConverterResponsesToChatCompletionsState",
"ChatCompletionsConverterState",
),
("nemo_gym.models", "VLLMConverter", "ChatCompletionsConverter"),
("nemo_gym.models", "VLLMConverterResponsesToChatCompletionsState", "ChatCompletionsConverterState"),
("responses_api_models.vllm_model.app", "VLLMConverter", "ChatCompletionsConverter"),
(
"responses_api_models.vllm_model.app",
"VLLMConverterResponsesToChatCompletionsState",
"ChatCompletionsConverterState",
),
],
)
def test_renamed_converter_resolves_at_each_old_path_with_a_warning(module: str, old_name: str, new_name: str) -> None:
Expand Down
35 changes: 35 additions & 0 deletions tests/unit_tests/tooling/test_migrate_imports.py
Original file line number Diff line number Diff line change
Expand Up @@ -343,6 +343,41 @@ def test_names_split_out_of_a_module_that_moved_later_are_found_at_its_new_path(
"from nemo_gym.models.chat_streaming import stream\n"
)

def test_names_renamed_after_their_module_moved_are_found_at_its_old_path_in_one_pass(self) -> None:
migrator = migrate_imports.Migrator(
{"nemo_gym.responses_converter": "nemo_gym.models.chat_completions_converter"},
{
"nemo_gym.models.chat_completions_converter": {
"VLLMConverter": ("nemo_gym.models.chat_completions_converter", "ChatCompletionsConverter")
}
},
)
once = _python(migrator, "from nemo_gym.responses_converter import VLLMConverter, split\n")
assert once == (
"from nemo_gym.models.chat_completions_converter import ChatCompletionsConverter as VLLMConverter, split\n"
)
assert _python(migrator, once) == once

def test_wrapped_import_from_a_server_module_that_lists_moved_names_is_split(self) -> None:
migrator = migrate_imports.Migrator(
{},
{
"responses_api_models.vllm_model.app": {
"VLLMConverter": ("nemo_gym.models.chat_completions_converter", "ChatCompletionsConverter")
}
},
)
source = "from responses_api_models.vllm_model.app import (\n VLLMConverter,\n VLLMModel,\n)\n"
once = _python(migrator, source)
assert once == (
"from nemo_gym.models.chat_completions_converter import ChatCompletionsConverter as VLLMConverter\n"
"from responses_api_models.vllm_model.app import VLLMModel\n"
)
assert _python(migrator, once) == once
# Server modules that list no moved names stay out of the rewrite.
other = "from responses_api_models.openai_model.app import (\n SimpleModelServer,\n)\n"
assert _python(migrator, other) == other


class TestRewriteText:
def test_library_reference_urls_are_left_for_the_docs_redirects(self, migrator) -> None:
Expand Down
Loading