Skip to content

Add Qwen3 / Qwen3-Coder support to swe-agent-v2 example (ROCm-safe)#1645

Merged
yushengsu-thu merged 6 commits into
radixark:mainfrom
lizamd:qwen3-swe-agent-v2
Jul 17, 2026
Merged

Add Qwen3 / Qwen3-Coder support to swe-agent-v2 example (ROCm-safe)#1645
yushengsu-thu merged 6 commits into
radixark:mainfrom
lizamd:qwen3-swe-agent-v2

Conversation

@lizamd

@lizamd lizamd commented Jul 13, 2026

Copy link
Copy Markdown
Contributor
  • run.py: parameterize SGLang/TITO parsers (GLM-4.7-Flash defaults unchanged); set RAY_EXPERIMENTAL_NOSET_HIP_VISIBLE_DEVICES only under torch.version.hip so it is a no-op on NVIDIA (CUDA device management unchanged there)
  • run-qwen3-swe.py: thin Qwen3/Qwen3-Coder launcher reusing run.py; --coder uses Qwen3-Coder-30B-A3B-Instruct (rope_theta=1e7 via MODEL_ARGS_ROTARY_BASE)
  • server.py: add the missing task-agnostic Harbor /run server (README references it; closely related to the codecontests server in Multi-turn RL on CodeContests on AMD MI35x #1603) plus turns/tool_calls metrics so agent/turns_mean populates
  • README: Qwen3/Coder usage and ROCm prerequisites

- run.py: parameterize SGLang/TITO parsers (GLM-4.7-Flash defaults unchanged);
  set RAY_EXPERIMENTAL_NOSET_HIP_VISIBLE_DEVICES only under torch.version.hip so
  it is a no-op on NVIDIA (CUDA device management unchanged there)
- run-qwen3-swe.py: thin Qwen3/Qwen3-Coder launcher reusing run.py; --coder uses
  Qwen3-Coder-30B-A3B-Instruct (rope_theta=1e7 via MODEL_ARGS_ROTARY_BASE)
- server.py: add the missing task-agnostic Harbor /run server (README references
  it; closely related to the codecontests server in radixark#1603) plus turns/tool_calls
  metrics so agent/turns_mean populates
- README: Qwen3/Coder usage and ROCm prerequisites

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

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

Copy link
Copy Markdown
Contributor

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 introduces support for Qwen3 and Qwen3-Coder models in the Agent V2 pipeline, including documentation, a dedicated launcher script, and configuration updates for ROCm/AMD environments. It also adds a FastAPI server wrapping Harbor for generalized agent-environment orchestration. Feedback on the changes highlights a potential path traversal vulnerability in the server's path validation, a potential TypeError during reward extraction, and a potential AttributeError when checking torch.version.hip on environments with older PyTorch installations.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment on lines +201 to +221
tasks_dir_str = str(tasks_dir)
task_path = os.path.normpath(os.path.join(tasks_dir_str, raw_id))
if not task_path.startswith(tasks_dir_str):
logger.error(f"Path traversal blocked: {raw_id!r}")
return _error_response("InvalidInstanceId")

# Case-insensitive fallback: the SWE-Gym adapter lowercases task dir names
# (Docker repo names must be lowercase), but dataset instance_ids keep the
# original case (e.g. Project-MONAI__MONAI-1030). Try the lowercased dir if
# the exact-case one is absent so those instances aren't TaskNotFound.
if not os.path.exists(task_path) and raw_id != raw_id.lower():
lower_path = os.path.normpath(os.path.join(tasks_dir_str, raw_id.lower()))
if lower_path.startswith(tasks_dir_str) and os.path.exists(lower_path):
logger.info(f"Resolved {raw_id!r} -> lowercase task dir {raw_id.lower()!r}")
task_path = lower_path

if not os.path.exists(task_path):
logger.error(f"Task directory not found: {task_path}")
return _error_response("TaskNotFound")

task_path = Path(task_path)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

security-high high

The path traversal check uses startswith on a string representation of the path (task_path.startswith(tasks_dir_str)). This is vulnerable to partial path traversal (e.g., if tasks_dir is /root/harbor_tasks, a raw_id of ../harbor_tasks_secret resolves to /root/harbor_tasks_secret, which starts with /root/harbor_tasks and bypasses the check). Using Path.is_relative_to is much safer and prevents this vulnerability.

        try:
            resolved_task_path = (tasks_dir / raw_id).resolve()
            if not resolved_task_path.is_relative_to(tasks_dir):
                logger.error(f"Path traversal blocked: {raw_id!r}")
                return _error_response("InvalidInstanceId")
            task_path = resolved_task_path
        except Exception:
            logger.error(f"Invalid path resolution: {raw_id!r}")
            return _error_response("InvalidInstanceId")

        # Case-insensitive fallback: the SWE-Gym adapter lowercases task dir names
        # (Docker repo names must be lowercase), but dataset instance_ids keep the
        # original case (e.g. Project-MONAI__MONAI-1030). Try the lowercased dir if
        # the exact-case one is absent so those instances aren't TaskNotFound.
        if not task_path.exists() and raw_id != raw_id.lower():
            try:
                lower_path = (tasks_dir / raw_id.lower()).resolve()
                if lower_path.is_relative_to(tasks_dir) and lower_path.exists():
                    logger.info(f"Resolved {raw_id!r} -> lowercase task dir {raw_id.lower()!r}")
                    task_path = lower_path
            except Exception:
                pass

        if not task_path.exists():
            logger.error(f"Task directory not found: {task_path}")
            return _error_response("TaskNotFound")

Comment on lines +112 to +117
vr = getattr(result, "verifier_result", None)
if vr is None:
return 0.0, {}
rewards = getattr(vr, "rewards", None) or {}
reward = float(rewards.get("reward", next(iter(rewards.values()), 0.0)))
return reward, dict(rewards)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

If rewards contains a "reward" key with a None value (e.g., {"reward": None}), rewards.get("reward", ...) will return None, causing float(None) to raise a TypeError. Handling this defensively ensures the server won't crash during reward extraction.

    vr = getattr(result, "verifier_result", None)
    if vr is None:
        return 0.0, {}
    rewards = getattr(vr, "rewards", None) or {}
    reward_val = rewards.get("reward")
    if reward_val is None:
        reward_val = next(iter(rewards.values()), 0.0)
    try:
        reward = float(reward_val) if reward_val is not None else 0.0
    except (TypeError, ValueError):
        reward = 0.0
    return reward, dict(rewards)

Comment on lines +250 to +253
import torch

if torch.version.hip:
extra_env_vars["RAY_EXPERIMENTAL_NOSET_HIP_VISIBLE_DEVICES"] = "1"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

Directly accessing torch.version.hip can raise an AttributeError on environments with custom or older PyTorch installations where the hip attribute is not defined. Using getattr is safer and adheres to defensive programming practices.

Suggested change
import torch
if torch.version.hip:
extra_env_vars["RAY_EXPERIMENTAL_NOSET_HIP_VISIBLE_DEVICES"] = "1"
import torch
if getattr(torch.version, "hip", None):
extra_env_vars["RAY_EXPERIMENTAL_NOSET_HIP_VISIBLE_DEVICES"] = "1"

lizamd and others added 3 commits July 13, 2026 16:31
- server.py: use Path.is_relative_to instead of a string startswith() check for
  the task-dir path validation (partial-prefix path-traversal, high severity)
- server.py: handle None / non-numeric reward values defensively so a
  {'reward': None} verifier result can't raise TypeError in float()
- run.py: getattr(torch.version, 'hip', None) to avoid AttributeError on older
  or custom PyTorch builds without the hip attribute

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@lizamd

lizamd commented Jul 14, 2026

Copy link
Copy Markdown
Contributor Author

CI infra flake: CPU test stages fail at conftest import when HuggingFace can't serve Qwen/Qwen3-0.6B

The stage-a-cpu / stage-b-cpu failures on this PR are unrelated to the diff (which only touches examples/experimental/swe-agent-v2/; Run pre-commit is green). Root cause is a pre-existing test-collection issue.

Symptom — pytest aborts during collection with exit code 4:

ImportError while loading conftest '.../tests/conftest.py'.
...
miles/utils/test_utils/mock_tools.py:144: in <module>
    _TOKENIZER = load_tokenizer("Qwen/Qwen3-0.6B", trust_remote_code=True)
...
E   OSError: Can't load tokenizer for 'Qwen/Qwen3-0.6B'.

Root causemiles/utils/test_utils/mock_tools.py:144 loads the Qwen/Qwen3-0.6B tokenizer from HuggingFace at module-import time. It's pulled into every CPU test run via tests/conftest.pytests/fast/fixtures/generation_fixtures.py:21 (from miles.utils.test_utils import mock_tools). Because the fetch happens at import, a single failed HF download aborts all collection (exit 4), not just tests that use the tokenizer.

Intermittent, not deterministic (→ HF rate-limit / transient fetch) — within the same commit 33a95a1, stage-a-cpu (2) passed while stage-a-cpu (0)/(1)/(3) and stage-b-cpu failed with the tokenizer error; all CPU shards passed earlier on 7d887f4. Consistent with anonymous HF rate-limiting when shards fetch concurrently.

Suggested fixes (maintainer side):

  1. Add an HF_TOKEN secret to the CPU test workflow (authenticated pulls avoid the anonymous rate limit), and/or
  2. actions/cache on ~/.cache/huggingface so shards don't each re-download, and/or
  3. Vendor the small Qwen/Qwen3-0.6B tokenizer as a test asset + HF_HUB_OFFLINE=1.
    Optionally make mock_tools._TOKENIZER lazy so a plain import no longer performs a network call (limits blast radius to tests that actually need it).

@yushengsu-thu yushengsu-thu left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

create an examples/experimental/swe-agent-v2-amd and put your files into in

Per @yushengsu-thu review: keep the shared examples/experimental/swe-agent-v2
example untouched and move the ROCm/AMD + Qwen3 work into its own directory.

- swe-agent-v2/ reverted to its pre-PR base state (no shared-file edits)
- swe-agent-v2-amd/ is self-contained: own run.py (generalized parser config +
  ROCm HIP_VISIBLE_DEVICES handling), run-qwen3-swe.py, server.py, and copies of
  the runtime deps generate.py / swe_agent_function.py so it runs standalone

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@lizamd

lizamd commented Jul 14, 2026

Copy link
Copy Markdown
Contributor Author

create an examples/experimental/swe-agent-v2-amd and put your files into in

@yushengsu-thu done — moved the AMD/Qwen3 work into examples/experimental/swe-agent-v2-amd/ and reverted examples/experimental/swe-agent-v2/ to its base state (no shared-file edits now).

The new dir is self-contained: it has its own run.py (the generalized parser config + ROCm HIP_VISIBLE_DEVICES handling), plus run-qwen3-swe.py, server.py, and a standalone README.md. It also carries copies of the runtime deps generate.py / swe_agent_function.py so it runs without touching the shared example — happy to reference the shared modules instead if you'd prefer no duplication.

(FYI the earlier Gemini findings were already addressed in an earlier commit. The remaining CI red is the pre-existing Qwen/Qwen3-0.6B tokenizer-fetch flake I noted above, unrelated to this diff.) PTAL 🙏

@lizamd
lizamd requested a review from yushengsu-thu July 14, 2026 04:35
@yushengsu-thu
yushengsu-thu merged commit 0607c36 into radixark:main Jul 17, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants