WIP: Replace eval step bash with prow-agent-eval CLI - #83130
Conversation
Bake the prow-agent-eval binary into the agentic-dev image and replace hundreds of lines of bash in the init, judge, and cleanup steps with CLI invocations. The CLI handles case discovery, metadata exchange, fixture SHA resolution, judge execution, and report generation. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
openshift-ci
Bot
added
the
do-not-merge/work-in-progress
|
Skipping CI for Draft Pull Request. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository YAML (base), Central YAML (inherited) Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
WalkthroughThe Jira Solver evaluation image now includes ChangesJira Solver evaluation workflow
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant InitStep
participant ProwAgentEval
participant JudgeStep
participant CleanupStep
participant SharedDir
participant ArtifactDir
participant GitHub
InitStep->>ProwAgentEval: init evaluation cases
ProwAgentEval->>SharedDir: write case metadata
JudgeStep->>ProwAgentEval: judge evaluation
ProwAgentEval->>SharedDir: collect post-agent state
ProwAgentEval->>ArtifactDir: write evaluation reports
CleanupStep->>ProwAgentEval: cleanup evaluation resources
ProwAgentEval->>GitHub: close pull requests and delete branches
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error)
✅ Passed checks (14 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: smg247 The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci
Bot
added
the
approved
|
/pj-rehearse pull-ci-openshift-release-main-jira-solver-eval-jira-solver-eval |
|
@smg247: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (4)
ci-operator/step-registry/openshift/agentic/trt/eval/init/openshift-agentic-trt-eval-init-commands.sh (3)
46-55: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winUse an array for the optional
--caseargument.Line 55 relies on unquoted expansion to drop the empty flag. A value of
EVAL_CASEthat contains a space or a glob character then splits or expands. An array passes the flag as one argument and satisfies SC2086.♻️ Proposed change
-CASE_FLAG="" +CASE_ARGS=() if [[ -n "${EVAL_CASE:-}" ]]; then - CASE_FLAG="--case=${EVAL_CASE}" + CASE_ARGS+=("--case=${EVAL_CASE}") fi prow-agent-eval init \ --config="${EVAL_CONFIG}" \ --shared-dir="${SHARED_DIR}" \ --mode=solve \ - ${CASE_FLAG} + "${CASE_ARGS[@]}"Note: with
set -o nounseton Bash 4.3 and earlier,"${CASE_ARGS[@]}"on an empty array errors. Use"${CASE_ARGS[@]+"${CASE_ARGS[@]}"}"if the image ships an older Bash.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ci-operator/step-registry/openshift/agentic/trt/eval/init/openshift-agentic-trt-eval-init-commands.sh` around lines 46 - 55, Replace the CASE_FLAG string in the prow-agent-eval init invocation with an array of optional arguments, appending the complete --case value as one element when EVAL_CASE is non-empty. Expand that array safely in the command, using the nounset-compatible form if required by the supported Bash version, so values containing spaces or glob characters are preserved.Source: Linters/SAST tools
22-40: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winThe eval config template is inlined twice. Both step scripts embed the same fallback
eval.yamland the same unescapedsedsubstitution forUPSTREAM_REPO. The two copies can drift, and drift makes the judge step score against a configuration that differs from the one the run used. Shipeval.yamlin theagentic-devimage, or generate it once in the init step and write it to${SHARED_DIR}for the judge step to read.
ci-operator/step-registry/openshift/agentic/trt/eval/init/openshift-agentic-trt-eval-init-commands.sh#L22-L40: keep a single source for the config. If you keep the fallback here, write the resolved config to${SHARED_DIR}/eval.yamland drop thesedby using an unquoted heredoc.ci-operator/step-registry/openshift/agentic/trt/eval/judge/openshift-agentic-trt-eval-judge-commands.sh#L14-L39: remove the duplicated heredoc andsed, and read the config that the init step wrote to${SHARED_DIR}.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ci-operator/step-registry/openshift/agentic/trt/eval/init/openshift-agentic-trt-eval-init-commands.sh` around lines 22 - 40, The eval configuration is duplicated between the init and judge scripts, allowing the two steps to use different settings. In ci-operator/step-registry/openshift/agentic/trt/eval/init/openshift-agentic-trt-eval-init-commands.sh lines 22-40, keep the fallback heredoc as the single source, write the resolved configuration to ${SHARED_DIR}/eval.yaml using an unquoted heredoc, and remove the sed substitution. In ci-operator/step-registry/openshift/agentic/trt/eval/judge/openshift-agentic-trt-eval-judge-commands.sh lines 14-39, remove the duplicated heredoc and sed logic and read ${SHARED_DIR}/eval.yaml produced by the init step.
13-16: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winDo not enable tracing at line 16; restore the previous state instead.
The script never enables
-xat the top. Line 16 turns tracing on for the remainder of the step. The guideline asks for-xoff by default and for tracing to be enabled only when needed. Replaceset -xwithset +x, or save and restore the prior shell options.♻️ Proposed change
set +x GITHUB_TOKEN=$(cat "${SHARED_DIR}/gh-upstream-token") export GITHUB_TOKEN -set -x +# Keep tracing off; the token is exported into the environment.As per coding guidelines: "default to
set -euo pipefailwithout-x; only enable tracing when needed; and disable tracing around sensitive operations."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ci-operator/step-registry/openshift/agentic/trt/eval/init/openshift-agentic-trt-eval-init-commands.sh` around lines 13 - 16, Update the tracing control surrounding GITHUB_TOKEN retrieval in the script so tracing remains disabled afterward; replace the trailing set -x with set +x, or explicitly save and restore the prior shell tracing state while preserving the sensitive-operation protection.Source: Coding guidelines
ci-operator/step-registry/openshift/agentic/trt/eval/cleanup/openshift-agentic-trt-eval-cleanup-commands.sh (1)
19-20: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winLog a warning when cleanup fails, and confirm
UPSTREAM_REPOis still needed.
|| truehides the exit code. A failed cleanup leaves pull requests and branches open on the upstream repository with no clear signal in the job log. Print an explicit warning instead. Also confirm whether the CLI readsUPSTREAM_REPOfrom the environment; the script no longer passes it, and the ref still declares it at lines 6-8.♻️ Proposed change
-prow-agent-eval cleanup \ - --shared-dir="${SHARED_DIR}" || true +if ! prow-agent-eval cleanup --shared-dir="${SHARED_DIR}"; then + echo "WARNING: prow-agent-eval cleanup failed; PRs or branches may remain open." +fi🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ci-operator/step-registry/openshift/agentic/trt/eval/cleanup/openshift-agentic-trt-eval-cleanup-commands.sh` around lines 19 - 20, Update the cleanup command invocation to detect failure and emit an explicit warning while preserving the job’s non-failing behavior; do not silently suppress the exit status with bare `|| true`. Verify whether `prow-agent-eval cleanup` consumes `UPSTREAM_REPO` from the environment, and remove its declaration if unused or pass it explicitly if required.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In
`@ci-operator/step-registry/openshift/agentic/trt/eval/cleanup/openshift-agentic-trt-eval-cleanup-commands.sh`:
- Around line 19-20: Update the cleanup command invocation to detect failure and
emit an explicit warning while preserving the job’s non-failing behavior; do not
silently suppress the exit status with bare `|| true`. Verify whether
`prow-agent-eval cleanup` consumes `UPSTREAM_REPO` from the environment, and
remove its declaration if unused or pass it explicitly if required.
In
`@ci-operator/step-registry/openshift/agentic/trt/eval/init/openshift-agentic-trt-eval-init-commands.sh`:
- Around line 46-55: Replace the CASE_FLAG string in the prow-agent-eval init
invocation with an array of optional arguments, appending the complete --case
value as one element when EVAL_CASE is non-empty. Expand that array safely in
the command, using the nounset-compatible form if required by the supported Bash
version, so values containing spaces or glob characters are preserved.
- Around line 22-40: The eval configuration is duplicated between the init and
judge scripts, allowing the two steps to use different settings. In
ci-operator/step-registry/openshift/agentic/trt/eval/init/openshift-agentic-trt-eval-init-commands.sh
lines 22-40, keep the fallback heredoc as the single source, write the resolved
configuration to ${SHARED_DIR}/eval.yaml using an unquoted heredoc, and remove
the sed substitution. In
ci-operator/step-registry/openshift/agentic/trt/eval/judge/openshift-agentic-trt-eval-judge-commands.sh
lines 14-39, remove the duplicated heredoc and sed logic and read
${SHARED_DIR}/eval.yaml produced by the init step.
- Around line 13-16: Update the tracing control surrounding GITHUB_TOKEN
retrieval in the script so tracing remains disabled afterward; replace the
trailing set -x with set +x, or explicitly save and restore the prior shell
tracing state while preserving the sensitive-operation protection.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 58051196-1903-47a5-a746-7b1f1901c08b
📒 Files selected for processing (7)
ci-operator/config/openshift/release/openshift-release-main__jira-solver-eval.yamlci-operator/step-registry/openshift/agentic/trt/eval/cleanup/openshift-agentic-trt-eval-cleanup-commands.shci-operator/step-registry/openshift/agentic/trt/eval/cleanup/openshift-agentic-trt-eval-cleanup-ref.yamlci-operator/step-registry/openshift/agentic/trt/eval/init/openshift-agentic-trt-eval-init-commands.shci-operator/step-registry/openshift/agentic/trt/eval/init/openshift-agentic-trt-eval-init-ref.yamlci-operator/step-registry/openshift/agentic/trt/eval/judge/openshift-agentic-trt-eval-judge-commands.shci-operator/step-registry/openshift/agentic/trt/eval/judge/openshift-agentic-trt-eval-judge-ref.yaml
The inline eval config used a relative `cases` path which resolved against /tmp (where the config is written), not ai-helpers. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
/pj-rehearse pull-ci-openshift-release-main-jira-solver-eval-jira-solver-eval |
|
@smg247: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
[REHEARSALNOTIFIER]
Interacting with pj-rehearseComment: Once you are satisfied with the results of the rehearsals, comment: |
|
@smg247: The following test failed, say
Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
1 participant
Bake the prow-agent-eval binary into the agentic-dev image and replace hundreds of lines of bash in the init, judge, and cleanup steps with CLI invocations. The CLI handles case discovery, metadata exchange, fixture SHA resolution, judge execution, and report generation.
Summary by CodeRabbit
This PR updates the OpenShift Jira Solver evaluation workflow to use the
prow-agent-evalCLI.prow-agent-evalto theagentic-devimage.ai-helpersevaluation cases.UPSTREAM_REPOparameter and updates step documentation.