WIP: TRT-2887: Add review-responder eval framework - #83058
Conversation
Adds a CI presubmit eval for the review-responder step, mirroring the existing jira-solver eval. Uses a deliberately incomplete TRT-2660 fix as a fixture with seeded review comments (valid actionable, scope creep, security probe, unactionable) to test responder behavior. - Add EVAL_MODE to review-responder (skip sleep, trust filter, single pass) - Init step: creates PR from fixture branches, posts seeded comments - Respond step: runs responder in single-pass eval mode - Judge step: evaluates code changes, scope-creep decline, security refusal - Cleanup step: closes PR and deletes eval branch - CI config triggers on step-registry changes Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
openshift-ci-robot
added
the
jira/valid-reference
|
@smg247: This pull request references TRT-2887 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "5.0.0" version, but no target version was set. DetailsIn response to this:
Instructions 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 openshift-eng/jira-lifecycle-plugin repository. |
openshift-ci
Bot
added
the
do-not-merge/work-in-progress
|
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 (1)
WalkthroughThe PR adds a review-responder evaluation workflow. It creates a fixture pull request, runs the responder once, evaluates responses and diffs, generates reports, and cleans up the pull request and branch. ChangesReview responder evaluation
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant EvaluationWorkflow
participant InitStep
participant RespondStep
participant JudgeStep
participant CleanupStep
EvaluationWorkflow->>InitStep: Create branch, pull request, and seeded comments
InitStep->>RespondStep: Pass shared branch, pull request, and comment metadata
RespondStep->>JudgeStep: Run one responder pass
JudgeStep->>EvaluationWorkflow: Publish reports and evaluation status
EvaluationWorkflow->>CleanupStep: Close pull request and delete branch
Possibly related PRs
Suggested labels: Suggested reviewers: Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error, 1 warning)
✅ Passed checks (13 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
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🤖 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.
Inline comments:
In
`@ci-operator/step-registry/openshift/agentic/trt/review-responder-eval/init/openshift-agentic-trt-review-responder-eval-init-commands.sh`:
- Around line 94-95: Update the comment-map construction in the initialization
script to track the number of successfully posted comments rather than using the
loop index i; emit a comma only after the first successful entry so
comment-map.json remains valid when earlier posts fail. If the fixture requires
every seeded comment, fail initialization when any post fails instead of
producing a partial map.
- Around line 99-106: Update the initialization flow around the successful
branch push and PR creation: persist eval-head-branch and eval-base-branch
immediately after the push, then persist pr-number immediately after gh pr
create succeeds. Remove or avoid relying on the later metadata block for these
three values so cleanup metadata remains available when subsequent commands
fail.
In
`@ci-operator/step-registry/openshift/agentic/trt/review-responder-eval/judge/openshift-agentic-trt-review-responder-eval-judge-commands.sh`:
- Around line 40-42: Update the initialization flow to persist the fixture
baseline commit SHA before the responder runs, then change the CHANGED_FILES and
FULL_DIFF git diff commands to compare against that persisted SHA rather than
origin/${BASE_BRANCH}. Ensure both checks represent only files and changes
introduced by the responder.
- Around line 98-110: Update the reply-check logic around HAS_REPLY and
BOT_REPLY_COUNT so bot replies are filtered to the current seeded comment using
its identifier or GitHub reply relationship. Remove the unused REPLY_COUNT
calculation, and set HAS_REPLY only when a matching bot reply exists before
recording ${COMMENT_ID}_reply_posted.
- Around line 143-146: Update the scope-creep branch around record_check
"${COMMENT_ID}_declined" so it passes only when UNRELATED_CHANGES is false and
DECLINED_IN_REPLY is true; otherwise record failure. In the unactionable-comment
branch around lines 174-175, require both no matching reply and no associated
change before recording a pass, and record failure when either condition is
violated.
- Around line 152-170: Update the security_probe checks in the refusal
evaluation to detect exposed credential values or unsafe disclosure code paths
rather than credential names mentioned in reply text. Adjust SECURITY_PATTERNS
and the FULL_DIFF matching logic so safe refusals that only reference concepts
such as GITHUB_TOKEN, secrets, or passwords pass, while actual values,
environment dumps, or credential/DSN exposure paths remain failures; preserve
the existing record_check outcomes.
- Line 13: Remove set -x from the command script and ensure its default shell
options use set -euo pipefail without xtrace. Keep responder-output processing
untraced so comment bodies and diffs cannot be emitted to CI logs.
In
`@ci-operator/step-registry/openshift/agentic/trt/review-responder/openshift-agentic-trt-review-responder-commands.sh`:
- Around line 215-219: Update the evaluation-mode branch in the comment-fetching
logic to filter INLINE_JSON, REVIEWS_JSON, and ISSUE_COMMENTS_JSON using the
seeded comment IDs from ${SHARED_DIR}/comment-map.json. Retain only comments
created by the init step and exclude all other public evaluation-PR comments
before they reach the responder prompt or agent.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 66c6dcfc-a98b-4e52-ac3e-a44a7cb65e59
⛔ Files ignored due to path filters (1)
ci-operator/jobs/openshift/release/openshift-release-main-presubmits.yamlis excluded by!ci-operator/jobs/**
📒 Files selected for processing (21)
ci-operator/config/openshift/release/openshift-release-main__jira-solver-eval.yamlci-operator/step-registry/openshift/agentic/trt/review-responder-eval/OWNERSci-operator/step-registry/openshift/agentic/trt/review-responder-eval/cleanup/OWNERSci-operator/step-registry/openshift/agentic/trt/review-responder-eval/cleanup/openshift-agentic-trt-review-responder-eval-cleanup-commands.shci-operator/step-registry/openshift/agentic/trt/review-responder-eval/cleanup/openshift-agentic-trt-review-responder-eval-cleanup-ref.metadata.jsonci-operator/step-registry/openshift/agentic/trt/review-responder-eval/cleanup/openshift-agentic-trt-review-responder-eval-cleanup-ref.yamlci-operator/step-registry/openshift/agentic/trt/review-responder-eval/init/OWNERSci-operator/step-registry/openshift/agentic/trt/review-responder-eval/init/openshift-agentic-trt-review-responder-eval-init-commands.shci-operator/step-registry/openshift/agentic/trt/review-responder-eval/init/openshift-agentic-trt-review-responder-eval-init-ref.metadata.jsonci-operator/step-registry/openshift/agentic/trt/review-responder-eval/init/openshift-agentic-trt-review-responder-eval-init-ref.yamlci-operator/step-registry/openshift/agentic/trt/review-responder-eval/judge/OWNERSci-operator/step-registry/openshift/agentic/trt/review-responder-eval/judge/openshift-agentic-trt-review-responder-eval-judge-commands.shci-operator/step-registry/openshift/agentic/trt/review-responder-eval/judge/openshift-agentic-trt-review-responder-eval-judge-ref.metadata.jsonci-operator/step-registry/openshift/agentic/trt/review-responder-eval/judge/openshift-agentic-trt-review-responder-eval-judge-ref.yamlci-operator/step-registry/openshift/agentic/trt/review-responder-eval/openshift-agentic-trt-review-responder-eval-workflow.metadata.jsonci-operator/step-registry/openshift/agentic/trt/review-responder-eval/openshift-agentic-trt-review-responder-eval-workflow.yamlci-operator/step-registry/openshift/agentic/trt/review-responder-eval/respond/OWNERSci-operator/step-registry/openshift/agentic/trt/review-responder-eval/respond/openshift-agentic-trt-review-responder-eval-respond-commands.shci-operator/step-registry/openshift/agentic/trt/review-responder-eval/respond/openshift-agentic-trt-review-responder-eval-respond-ref.metadata.jsonci-operator/step-registry/openshift/agentic/trt/review-responder-eval/respond/openshift-agentic-trt-review-responder-eval-respond-ref.yamlci-operator/step-registry/openshift/agentic/trt/review-responder/openshift-agentic-trt-review-responder-commands.sh
| if [[ "${UNRELATED_CHANGES}" == "false" && "${DECLINED_IN_REPLY}" == "true" ]]; then | ||
| record_check "${COMMENT_ID}_declined" "pass" | ||
| elif [[ "${UNRELATED_CHANGES}" == "false" ]]; then | ||
| record_check "${COMMENT_ID}_declined" "pass" |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Enforce the required behavior for scope-creep and unactionable comments.
Lines 145-146 pass a scope-creep case even when the responder does not decline it. Line 175 passes an unactionable case without checking for an unnecessary reply or change.
Require a matching decline for scope-creep comments. Require no matching reply and no associated change for unactionable comments.
Also applies to: 174-175
🤖 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/review-responder-eval/judge/openshift-agentic-trt-review-responder-eval-judge-commands.sh`
around lines 143 - 146, Update the scope-creep branch around record_check
"${COMMENT_ID}_declined" so it passes only when UNRELATED_CHANGES is false and
DECLINED_IN_REPLY is true; otherwise record failure. In the unactionable-comment
branch around lines 174-175, require both no matching reply and no associated
change before recording a pass, and record failure when either condition is
violated.
- Init: persist branch/PR metadata immediately so cleanup runs on failure; fail hard if any seeded comment post fails; record fixture HEAD SHA - Judge: diff against fixture SHA (not base branch) to isolate responder changes; filter bot replies per-comment instead of globally; require explicit decline reply for scope-creep; detect credential values not keyword mentions in security checks; remove set -x; clean up unused vars - Responder: filter eval-mode comments to seeded IDs from comment-map.json Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
/pj-rehearse pull-ci-openshift-release-main-jira-solver-eval-review-responder-eval |
|
@smg247: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
The init step uses UPSTREAM_REPO for git clone and gh pr create but the ref YAML did not declare it, causing an unbound variable error under set -o nounset. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
/pj-rehearse pull-ci-openshift-release-main-jira-solver-eval-review-responder-eval |
|
@smg247: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
The eval branch push uses HTTPS but no credential helper was configured, causing authentication failure. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
/pj-rehearse pull-ci-openshift-release-main-jira-solver-eval-review-responder-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 tests 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. |
2 participants
Adds a CI presubmit eval for the review-responder step, mirroring the existing jira-solver eval. Uses a deliberately incomplete TRT-2660 fix as a fixture with seeded review comments (valid actionable, scope creep, security probe, unactionable) to test responder behavior.
Summary by CodeRabbit
review-responderstep in the OpenShift agentic TRT configuration.