[ananthsub]

Follows #5528, which renamed ResponsesConverter to ChatCompletionsConverter and moved every in-repo use of the VLLMConverter aliases to the new names.

What changed and why

VLLMConverter and VLLMConverterResponsesToChatCompletionsState are aliases of the Responses API to Chat Completions converter. They name a vendor, but the converter is not specific to vLLM: the LiteLLM and Azure OpenAI model servers use it too. After #5528 nothing in the repository uses them, so this PR deprecates them in favor of ChatCompletionsConverter and ChatCompletionsConverterState.

• Deprecated names. The aliases shipped in nemo_gym.responses_converter since 0.4.0, so they are not deleted. They become entries in the _RENAMED_CONVERTER_NAMES block that refactor(models): rename ResponsesConverter to ChatCompletionsConverter #5528 adds to nemo_gym/_moved_paths.py (since="0.8.0", removed in 0.10.0). Each old path resolves to the identical class and warns once with a DeprecationWarning that names the new name:
  • nemo_gym.responses_converter (released path)
  • nemo_gym.models.chat_completions_converter (the alias assignments are removed)
  • nemo_gym.models (dropped from _PUBLIC_API)
  • responses_api_models.vllm_model.app, which re-exported both aliases, and which other servers imported them from
• Server module. responses_api_models.vllm_model.app drops its plain re-export and installs the same moved_names_getattr __getattr__ that library modules use. It works there because the vLLM model server runs with this repository's nemo_gym, and the module imports cleanly in the core test environment. That keeps the vLLM model server path warning with the other paths instead of serving a silent re-export.
• Import migration. scripts/migrate_imports.py now rewrites imports of the aliases in one pass:
  • A name renamed in a module that moved earlier is listed under the module's new path, and the runtime serves it at the old path too. The script looked only under the old path, so from nemo_gym.responses_converter import VLLMConverter became an import of the same deprecated name from nemo_gym.models.chat_completions_converter, and needed a second pass. ResponsesConverter from refactor(models): rename ResponsesConverter to ChatCompletionsConverter #5528 had the same gap. The script now lists such names under every old path of their module, as the runtime does.
  • The Python rewrite skipped every module outside nemo_gym, and the text fallback handles only one-line imports, so a wrapped import from responses_api_models.vllm_model.app kept VLLMConverter. The rewrite now also covers modules outside nemo_gym that list moved names. Other server modules stay out of it.

Downstream impact

No NeMo RL code imports either alias. A search of local NeMo RL checkouts found no uses, and neither alias appears in what the moved-paths tests record as NeMo RL's imports. Out-of-tree code that uses the aliases keeps working through 0.9.x with a deprecation warning. python scripts/migrate_imports.py <paths> rewrites those imports, because it reads the new entries.

Issue

Part of the package reorganization (#5166). A separate issue is unnecessary: this is the follow-up to #5528's rename.

Tests

The moved-paths test that checks each old converter name now covers both aliases at all four old paths. In a fresh interpreter it asserts that the old name is the same object as the new class, that exactly one deprecation message names the old path, and that the message names the new class and 0.10.0. With the new _moved_paths.py entries removed, all of the new cases fail. The migration script tests now check that an import of a renamed name from its module's old path migrates in one pass, and that a wrapped import from a server module that lists moved names is split, leaving its other names in place. Both fail without the script change. The converter unit test that asserted the aliases equal the new classes is removed, because the moved-paths test now covers it.

Validation

PATH=~/dev/Gym/.venv/bin:$PATH pre-commit run --files <touched files>
PYTHONPATH=$PWD ~/dev/Gym/.venv/bin/python -m pytest tests/unit_tests/test_moved_paths.py tests/unit_tests/models/test_chat_completions_converter.py tests/unit_tests/test_public_api_facades.py tests/unit_tests/tooling/test_migrate_imports.py -n 8
~/dev/Gym/.venv/bin/python scripts/migrate_imports.py <scratch dir with each alias imported from each old path, one-line and wrapped>
~/dev/Gym/.venv/bin/python scripts/migrate_imports.py --check <same scratch dir>   # second pass: no changes
git log -1 --format=%B | grep Signed-off-by

scripts/migrate_imports.py --check . reports the same files with and without the script change, so it rewrites nothing new in the repository. The full core suite and the server suites (gym env test) are left to CI.

Rollout evidence: N/A. This only deprecates names. The converter's behavior is unchanged, and every old name resolves to the identical class.

[copy-pr-bot]

Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually.

Contributors can view more details about this message here.

[svcnvidia-nemo-ci]

SHIP WITH CARE

Two RISK findings affect import migration: imports from the released converter path need two passes, and multiline imports from the vLLM server aren't migrated. Runtime alias resolution retains class identity.

I reviewed all six changed files and their directly affected paths. This was a source-only review; I didn't run tests.

---

Inline comments (escaped v1):

• scripts/migrate_imports.py line 264: RISK — Include the released module path in the alias migration.

  With this PR's mappings, migrating from nemo_gym.responses_converter import VLLMConverter produces from nemo_gym.models.chat_completions_converter import VLLMConverter: the output still imports the newly deprecated alias. A second pass changes it again. The state alias has the same failure. _destinations() looks up name moves under the original module, but the new entries list these renames only under the destination module. The constructor propagates entries from old modules to new modules, not in the reverse direction. This leaves downstream migration output dependent on aliases scheduled for removal.

  Could you add the following entry to _RENAMED_CONVERTER_NAMES alongside the canonical-module entry?

  "nemo_gym.responses_converter": _VLLM_CONVERTER_ALIASES,

  This supplies the missing original-path lookup without changing the canonical-module entries or runtime resolution. The invariant is that one pass removes these deprecated imports while preserving their local bindings. Please verify this suggested addition before adopting it.

  Proposed checks, not executed: Load the real moves and public APIs, then migrate a downstream file importing each alias from nemo_gym.responses_converter. Expect from nemo_gym.models import ChatCompletionsConverter as VLLMConverter for the converter and from nemo_gym.models.chat_completions_converter import ChatCompletionsConverterState as VLLMConverterResponsesToChatCompletionsState for the state. A second pass must leave both unchanged. Check explicit as bindings and an unrelated imported name too. Preserve the current migration destinations for canonical-module and package imports, and retain the identity/warning assertions for all four legacy entry points and the existing ResponsesConverter cases in the moved-path tests.

• scripts/migrate_imports.py line 364: RISK — Admit registered server modules to the Python import rewrite.

  The new server mapping doesn't migrate this normal import:

  from responses_api_models.vllm_model.app import (
      VLLMConverter,
      VLLMModel,
  )

  This AST guard skips modules outside nemo_gym. The subsequent text fallback excludes parentheses, and there is no fully qualified alias for the dotted rewrite to match. The command therefore leaves the deprecated import unchanged, and --check reports no migration needed. Either alias is affected; a single-line server import does migrate.

  Could you replace the guard with this scoped extension?

  if not (
      isinstance(node, ast.ImportFrom)
      and node.module
      and (node.module.split(".")[0] == "nemo_gym" or node.module in self.names)
  ):
      continue

  The preceding relative-import branch still excludes relative imports. Existing destination grouping can then split the converter import from VLLMModel, preserving local bindings and leaving unrelated third-party modules outside the rewrite. Please verify this suggested replacement before adopting it.

  Proposed checks, not executed: Migrate multiline server imports containing each alias, an explicit as binding, and VLLMModel. Expect ChatCompletionsConverter from nemo_gym.models, the state class from its defining converter module, and VLLMModel still from the server. The output must parse and remain unchanged on a second pass. Preserve single-line migration, tab indentation, and the existing inline-suite assertion that a false condition executes neither split import. Preserve the existing handling that leaves split imports with internal comments unchanged and emits a manual-review note, plus trailing pragma propagation and unrelated imports in the migration tests.

Review generated by codex. This was a full review. Request a full review with /review reset=true.