From 56132ca711d50602528a3ba4ce1f5036b864cc96 Mon Sep 17 00:00:00 2001 From: "claude[bot]" <41898282+claude[bot]@users.noreply.github.com> Date: Thu, 29 Jan 2026 01:48:05 +0000 Subject: [PATCH 1/2] Add enroot import validation for launch scripts Add PR review validation to warn when runners/launch_*.sh scripts don't transform public Docker images to enroot local images using the enroot import pattern. This ensures benchmark reproducibility. Co-authored-by: functionstackx --- .github/workflows/claude-pr-review.yml | 28 ++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/.github/workflows/claude-pr-review.yml b/.github/workflows/claude-pr-review.yml index 9fa334f2a..1b16417ae 100644 --- a/.github/workflows/claude-pr-review.yml +++ b/.github/workflows/claude-pr-review.yml @@ -144,6 +144,34 @@ jobs: - Comment: "Image must be publicly accessible on NGC, Docker Hub, or another public registry. Local paths like `/scratch/...` or `.sqsh` files are generally not accepted. Please push the container to a public registry (e.g., `nvcr.io/nvidia/...` for NGC) and update the config with the public image reference." - Link to the specific line with the invalid image path + ## Enroot Import Validation for Launch Scripts: + When reviewing changes to `runners/launch_*.sh` files, verify that the script properly transforms public Docker images to enroot local images for reproducibility. + + **Expected pattern:** + The script should include an enroot import command like: + ```bash + srun --jobid=$JOB_ID bash -c "enroot import -o $SQUASH_FILE docker://$IMAGE" + ``` + or similar variations such as: + ```bash + srun -N 1 -A $SLURM_ACCOUNT -p $SLURM_PARTITION bash -c "enroot import -o $SQUASH_FILE docker://$IMAGE" + ``` + + **Why this matters:** + - Ensures the exact same public NGC/Docker Hub image is used + - Makes benchmarks reproducible by anyone with access to the public image + - Prevents reliance on pre-existing local container images that others cannot access + + **Validation rules:** + 1. Look for `enroot import` commands that convert `docker://` images to local `.sqsh` files + 2. The image source should be a public registry (NGC, Docker Hub, etc.), not a local path + 3. If the script uses containers but does NOT have an `enroot import docker://` pattern: + - This is a 🟡 **WARNING** issue + - Comment: "This launch script uses container images but does not appear to transform a public Docker image to an enroot local image using `enroot import -o $SQUASH_FILE docker://$IMAGE`. For reproducibility, please either: + 1. Add the enroot import pattern to pull from a public registry, OR + 2. Explain why this script has a different workflow (e.g., uses a different container runtime, pre-built images are acceptable for this use case, etc.)" + - Ask the developer to provide a reasonable explanation if the pattern is intentionally omitted + ## vLLM and SGLang Source Code Access: You have access to vLLM and SGLang source code via the inferencemax-repos MCP server: - Use `mcp__inferencemax-repos__*` tools to access repository source code From 5ee351f5e6918d4034b899bd096f0084abfb3e7f Mon Sep 17 00:00:00 2001 From: functionstackx <47992694+functionstackx@users.noreply.github.com> Date: Wed, 28 Jan 2026 21:21:40 -0500 Subject: [PATCH 2/2] Update validation steps to validation steps in workflow --- .github/workflows/claude-pr-review.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/claude-pr-review.yml b/.github/workflows/claude-pr-review.yml index 1b16417ae..2a1cbc80d 100644 --- a/.github/workflows/claude-pr-review.yml +++ b/.github/workflows/claude-pr-review.yml @@ -162,7 +162,7 @@ jobs: - Makes benchmarks reproducible by anyone with access to the public image - Prevents reliance on pre-existing local container images that others cannot access - **Validation rules:** + **Validation Steps:** 1. Look for `enroot import` commands that convert `docker://` images to local `.sqsh` files 2. The image source should be a public registry (NGC, Docker Hub, etc.), not a local path 3. If the script uses containers but does NOT have an `enroot import docker://` pattern: