Repository navigation
feat: PR Regression Testing Workflow - #199
Conversation
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configuration
Comment |
This script sets up a PR regression testing environment by cloning necessary repositories, installing dependencies, and configuring a GitHub Actions runner.
| exit 2 | ||
| fi | ||
|
|
||
| if [[ ! "${NUM_TASKS}" =~ ^[0-9]+$ ]] || [[ "${NUM_TASKS}" -lt 1 ]]; then |
There was a problem hiding this comment.
The regex ^[0-9]+$ lets leading-zero values like 08/09 through, but bash then parses ${NUM_TASKS} -lt 1 as octal and errors — inside an if, so set -e doesn't catch it, and the malformed value passes validation. The same octal-parse failure later hits the task-count trim at line 265, so it silently no-ops and the full/untrimmed task list runs while the posted comment still reports e.g. Num tasks: 08.
Verified empirically:
$ NUM_TASKS="08"; [[ ! "$NUM_TASKS" =~ ^[0-9]+$ ]] || [[ "$NUM_TASKS" -lt 1 ]]
bash: [[: 08: value too great for base (error token is "08")
# condition still evaluates false -> treated as valid
Suggested fix: force base-10 arithmetic and normalize away the leading zero:
if [[ ! "${NUM_TASKS}" =~ ^[0-9]+$ ]] || (( 10#${NUM_TASKS} < 1 )); then
echo "ERROR: num_tasks must be a positive integer."
...
fi
NUM_TASKS=$((10#${NUM_TASKS}))|
|
||
| def __init__(self, config: dict[str, Any]): | ||
| model_name = config.get("model_name", "openai/gpt-oss-120b-a100") | ||
| end_point = config.get( |
There was a problem hiding this comment.
_create_llm always passes "end_point": api_base even when api_base is None (no LITE_LLM_URL/OPENAI_BASE_URL/RITS_BASE_URL set). config.get("end_point", DEFAULT_URL) only falls back to the default when the key is absent, not when it's None — so this raises AttributeError: 'NoneType' object has no attribute 'rstrip' instead of using the documented default RITS URL. Only avoided today because run-pr-regression-eval.sh always exports OPENAI_BASE_URL first; any direct/local use of settings.rits.toml without those env vars crashes.
Suggested fix:
end_point = config.get("end_point") or "https://inference-3scale-apicast-production.apps.rits.fmaas.res.ibm.com/gpt-oss-120b-a100"| print("\n" + "=" * 80) | ||
| print("EVALUATION COMPLETE") | ||
| print("=" * 80) | ||
| print("######## REPORT START ########") |
There was a problem hiding this comment.
This REPORT START/REPORT END wrapping was added here but not in the identical _print_appworld_summary in appworld_eval.py (cuga, line 466) or appworld_eval_codeact.py (codeact, line 93) — each file has its own separate copy, and only this one was updated. eval.sh --agent cuga/--agent codeact do dispatch to those other two files, even though this script supports agent=cuga/agent=codeact. If the workflow parses these markers to build the PR comment, cuga/codeact runs would silently drop this section.
Suggested fix: mirror the same two print(...) additions ("######## REPORT START ########" / "######## REPORT END ########") around the equivalent summary block in both other files.
|
This PR's scripts are the payload that cuga-agent#759's workflow invokes, and #759 currently has CHANGES_REQUESTED (including from me) over security issues in that workflow — several of which point at code living in this PR, not just the workflow YAML:
Not blocking this PR's own scripts from being reviewed on their merits, but flagging so the sequencing ("merge this after #759") doesn't end up meaning "merge this, then immediately rework it once #759's security fixes land." |
|
Thanks for the review @haroldship. I addressed the PR-regression runner and reporting issues called out here and in the related cuga-agent workflow review. run-pr-regression-eval.sh now handles num_tasks with base-10 normalization, so values like 08 no longer pass validation and then break bash arithmetic as octal. The script also accepts sanitized workflow input and CLI-style flags, tolerates sha= metadata from the cuga-agent workflow, validates supported parameters, and fails closed on unknown arguments. For the React/RITS path, ChatRits now falls back correctly when end_point is None, avoids duplicating /v1, and the RITS-specific branch prioritizes RITS_BASE_URL/RITS_API_KEY before LiteLLM/OpenAI-compatible fallbacks. For report output, generate_eval_report now defaults back to the normal bundle/local behavior: no REPORT START/END sentinels, no forced I added tests for the compare_report PR-comment opt-in behavior and stdout branching. The README/setup guidance was updated for the SHA-pinned /run-pr-eval flow and the current artifact/report behavior. |
Normalize num_tasks as base-10, accept sanitized workflow parameters, and document the SHA-pinned PR eval flow. Fix RITS React-agent defaults and env precedence, and make PR comment report formatting an explicit opt-in so local eval reports keep their normal heading and plain stdout behavior.
|
Re-reviewed after the latest fixes. All 6 issues from my earlier pass are genuinely resolved and test-covered (octal Found a few more while re-checking end-to-end, two of which I'd want fixed before merge since they silently corrupt the artifacts this PR exists to produce: 1. Bundle Suggested fix: generate two separate temp files — one for the PR-comment extraction, one plain-markdown for the bundle — rather than sharing one. 2. Suggested fix: pass 3. (minor) 4. (minor, doc bug) README documents an optional |
|
One more on the One thing to watch: if [[ "${BENCHMARK}" == "appworld" ]]; then
...
EVAL_ARGS+=(--sdk)
...
else
EVAL_ARGS+=(--m3-data --max-samples-per-domain "${NUM_TASKS}")
...
fi |
haroldship
left a comment
haroldship
left a comment
There was a problem hiding this comment.
One more on the --sdk / cost-zeroing issue from earlier (#2 above). --agent cuga should run with --sdk in this workflow. It's a no-op for --agent react/--agent codeact since eval.sh's dispatch checks AGENT before USE_SDK, and it fixes the token accounting gap for cuga.
One thing to watch: run-pr-regression-eval.sh also runs benchmark=m3, and m3/eval.sh has no --sdk concept. Its arg parser forwards any unrecognized flag straight into benchmarks.m3.eval_m3's argparse, which will hard error on --sdk ("unrecognized arguments"). So --sdk needs to go only in the appworld branch of EVAL_ARGS, not added unconditionally:
if [[ "${BENCHMARK}" == "appworld" ]]; then
...
EVAL_ARGS+=(--sdk)
...
else
EVAL_ARGS+=(--m3-data --max-samples-per-domain "${NUM_TASKS}")
...
fi
Summary
Adds a PR-triggered regression evaluation flow for
cuga-agentusing scripts maintained incuga-eval.This PR introduces setup and runtime scripts for a self-hosted GitHub Actions runner that can respond to
/run-pr-evalcomments on PRs and execute selected benchmark evaluations. It supports AppWorld and M3 runs, configurable agents, provider selection, model overrides, task filtering, and structured PR comment output.Changes
scripts/pr-regression-testing/setup-pr-regression-workflow.shto bootstrap the runner VM.scripts/pr-regression-testing/run-pr-regression-eval.shto parse/run-pr-evalcomments and run evaluations.ritsandlitellmprovider configuration in PR regression runs.Usage
Example PR comment:
Sample Output
PR Evaluation
/run-pr-eval benchmark=appworld eval_key=test_hard provider=rits agent=reactfeac9b5d332e3143c5bb6eee31fdb5741da0975bRequested parameters
Evaluation Metrics
Run metadata
Evaluation configuration
EVALUATION COMPLETE
Total Tasks: 45
Completed: 0/45 (0.0%)
Avg Steps: 9.24
Avg Duration: 86.06s
Evaluation Report
Summary
Per-Task Results
Difficulty Breakdown
Test-Set Breakdown (AppWorld)
Run summary