Fix temp benchmark directory

[sbryngelson]

User description

Remove the temporary directory when done with it - and make it unique.

---

PR Type

Bug fix

---

Description

• Fix temporary directory management in benchmark script

• Add unique directory creation with random suffix

• Implement proper cleanup with removal after completion

---

Changes walkthrough 📝

	Relevant files
Bug fix	bench.sh Improve temp directory handling with cleanup                          .github/workflows/phoenix/bench.sh Replace hardcoded temp directory with unique random-suffixed path Add proper directory creation for both base and current directories Implement cleanup with sleep delay and safe removal Update TMPDIR export to use the unique current directory +9/-2

---

Need help?

Type /help how to ... in the comments thread for any questions about Qodo Merge usage.

Check out the documentation for more information.

[qodo-merge-pro]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪

🧪 No relevant tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Syntax Error The if statement condition has incorrect syntax with single bracket instead of double brackets, which will cause the script to fail if ["$job_device" == "gpu"]; then ./mfc.sh bench --mem 12 -j $(nproc) -o "$job_slug.yaml" -- -c phoenix-bench $device_opts -n $n_ranks Race Condition The random directory generation using RANDOM could potentially create collisions if multiple instances run simultaneously, and the sleep before cleanup seems arbitrary without clear justification currentdir=$tmpbuild/run-$(( RANDOM % 900 )) mkdir -p $tmpbuild mkdir -p $currentdir export TMPDIR=$currentdir if ["$job_device" == "gpu"]; then ./mfc.sh bench --mem 12 -j $(nproc) -o "$job_slug.yaml" -- -c phoenix-bench $device_opts -n $n_ranks else ./mfc.sh bench --mem 1 -j $(nproc) -o "$job_slug.yaml" -- -c phoenix-bench $device_opts -n $n_ranks fi sleep 10 rm -rf $currentdir || true

[qodo-merge-pro]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	✅ Fix bash conditional syntax error Suggestion Impact: The suggestion identified a critical syntax error that was partially addressed. The commit fixed the missing space issue on line 14, but also made additional changes including using double brackets and changing comparison operator on line 5. code diff: -if [ "$job_device" == "gpu" ]; then +if [ "$job_device" = "gpu" ]; then n_ranks=$(nvidia-smi -L | wc -l) # number of GPUs on node gpu_ids=$(seq -s ' ' 0 $(($n_ranks-1))) # 0,1,2,...,gpu_count-1 device_opts="--gpu -g $gpu_ids" @@ -15,7 +15,7 @@ export TMPDIR=$currentdir -if ["$job_device" == "gpu"]; then +if [[ "$job_device" == "gpu" ]]; then The condition has incorrect syntax with a missing space after the opening bracket. In bash, the opening bracket must be followed by a space for proper test command syntax. .github/workflows/phoenix/bench.sh [18] -if ["$job_device" == "gpu"]; then +if [ "$job_device" == "gpu" ]; then Suggestion importance[1-10]: 10 __ Why: The suggestion correctly identifies a critical syntax error in the bash conditional if ["$job_device" == "gpu"]; then. The missing space after [ would cause the script to fail. This fix is essential for the script's functionality.	High
General	Reduce directory collision probability Using modulo 900 creates a high probability of directory name collisions in concurrent runs. Consider using a larger range or combining with timestamp/process ID for better uniqueness. .github/workflows/phoenix/bench.sh [12] -currentdir=$tmpbuild/run-$(( RANDOM % 900 )) +currentdir=$tmpbuild/run-$(( RANDOM % 90000 )) Suggestion importance[1-10]: 7 __ Why: The suggestion correctly identifies that using RANDOM % 900 creates a high probability of directory name collisions in concurrent runs. Increasing the range, as suggested, is a good practice that improves the script's robustness and prevents potential race conditions.	Medium
Update

[Copilot]

Pull Request Overview

This PR fixes temporary directory management in the benchmark workflow and standardizes string comparisons across CI scripts.

• Creates a unique, random-suffixed temp directory for benchmarks and exports it via TMPDIR
• Implements cleanup of the temp directory after benchmarking
• Replaces non-portable == with POSIX = in various workflow scripts

Reviewed Changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
.github/workflows/phoenix/test.sh	Use = instead of == for string comparison
.github/workflows/phoenix/submit.sh	Use = instead of == for string comparison
.github/workflows/phoenix/submit-bench.sh	Use = instead of == for string comparison
.github/workflows/phoenix/bench.sh	Add unique temp‐dir creation and cleanup; use = for comparisons
.github/workflows/frontier/test.sh	Use = instead of == for string comparison
.github/workflows/frontier/submit.sh	Use = instead of == for string comparison
.github/workflows/frontier/build.sh	Use = instead of == for string comparison

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 45.98%. Comparing base (c0327c8) to head (40484e9).
Report is 1 commits behind head on master.

Additional details and impacted files

@@           Coverage Diff           @@
##           master     #906   +/-   ##
=======================================
  Coverage   45.98%   45.98%           
=======================================
  Files          68       68           
  Lines       18629    18629           
  Branches     2239     2239           
=======================================
  Hits         8566     8566           
  Misses       8711     8711           
  Partials     1352     1352           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.