Skip to content

review-agent: use review URL instead of opaque GraphQL node ID#78042

Merged
openshift-merge-bot[bot] merged 1 commit into
openshift:mainfrom
bryan-cox:fix-review-agent-prr-id
Apr 21, 2026
Merged

review-agent: use review URL instead of opaque GraphQL node ID#78042
openshift-merge-bot[bot] merged 1 commit into
openshift:mainfrom
bryan-cox:fix-review-agent-prr-id

Conversation

@bryan-cox

@bryan-cox bryan-cox commented Apr 20, 2026

Copy link
Copy Markdown
Member

Summary

  • Review body replies were formatted as Re: review PRR_kwDOE7ekcc72sY6X which is an opaque GraphQL node ID meaningless to humans (example)
  • Switched to using the review URL so replies show a clickable link like Re: https://github.com/openshift/hypershift/pull/8280#pullrequestreview-4138831511
  • Duplicate detection checks for both the old node ID and new URL format for backward compatibility

Test plan

  • Verify next review-agent run produces replies with clickable review URLs instead of PRR_ IDs
  • Verify old replies with PRR_ node IDs are still detected as "already replied" (no duplicate responses)

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes
    • Improved bot comment deduplication to more reliably detect and prevent duplicate responses by considering multiple review reference methods.
    • Enhanced review linkage in bot replies for clearer tracking.

…lies

Review body replies were formatted as "Re: review PRR_kwDOE7ekcc72sY6X"
which is meaningless to humans. Switch to using the review URL so replies
show a clickable link instead. Duplicate detection checks for both the
old node ID and new URL format for backward compatibility.

Co-Authored-By: Claude Opus 4.6 <[email protected]>
@coderabbitai

coderabbitai Bot commented Apr 20, 2026

Copy link
Copy Markdown
Contributor

Walkthrough

Modified the review-agent script to reference reviews by URL instead of node ID. Extended the GraphQL query to fetch review URLs, updated deduplication logic to match by either ID or URL, and changed response rules to link reviews via URL rather than ID.

Changes

Cohort / File(s) Summary
Review Reference Update
ci-operator/step-registry/hypershift/review-agent/process/hypershift-review-agent-process-commands.sh
Extended GraphQL query to include review url field. Updated reply-deduplication logic to check bot comments against both review id and url. Propagated review url into needs_attention entries. Modified RESPONSE RULES to reference reviews via Re: <url> instead of Re: review <id>.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

🚥 Pre-merge checks | ✅ 10
✅ Passed checks (10 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and specifically describes the main change: replacing opaque GraphQL node IDs with review URLs in the review-agent process.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Stable And Deterministic Test Names ✅ Passed The modified file is a bash script containing Python code for processing review-agent commands, not a Go test file with Ginkgo tests. No Ginkgo test patterns were found.
Test Structure And Quality ✅ Passed This pull request does not contain any Ginkgo test code. The only file changed is a bash script that is part of the CI/CD step registry. Since the PR contains no Go test files or Ginkgo tests, this check is not applicable.
Microshift Test Compatibility ✅ Passed PR modifies shell scripts for CI/CD review-agent tool; contains no Ginkgo e2e tests so custom check is not applicable.
Single Node Openshift (Sno) Test Compatibility ✅ Passed PR modifies only bash shell script for CI/CD review automation; does not add, modify, or introduce Ginkgo e2e tests, so check is not applicable and passes by default.
Topology-Aware Scheduling Compatibility ✅ Passed The custom check for topology-aware scheduling compatibility is not applicable to this pull request. The check is designed to flag changes that introduce Kubernetes scheduling constraints in deployment manifests, operator code, or controllers. This PR modifies only a bash CI orchestration script with embedded Python code for GitHub PR review processing—it introduces no Kubernetes workload specifications or scheduling constraints.
Ote Binary Stdout Contract ✅ Passed The OTE Binary Stdout Contract check is not applicable to this pull request as the modified file is a bash shell script, not a Go test binary.
Ipv6 And Disconnected Network Test Compatibility ✅ Passed This PR modifies only a bash shell script for the hypershift review-agent process and does not add any Ginkgo e2e tests.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands and usage tips.

@openshift-ci openshift-ci Bot requested review from celebdor and enxebre April 20, 2026 16:12
@openshift-ci openshift-ci Bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Apr 20, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In
`@ci-operator/step-registry/hypershift/review-agent/process/hypershift-review-agent-process-commands.sh`:
- Around line 469-470: The dedup check using substring matching for replied =
any(str(review_id) in b or (review_url and review_url in b) for b in bot_bodies)
can yield false positives when IDs/URLs share prefixes; change the logic to
perform exact comparisons instead: parse each entry in bot_bodies to extract the
referenced review identifier or full URL (or use a regex with
anchors/word-boundaries) and compare equality against str(review_id) and
review_url, ensuring you update the check in the block that sets replied so it
only returns true for exact matches of review_id or review_url.
🪄 Autofix (Beta)

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: openshift/coderabbit/.coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 0855ca97-28a1-4417-a0c1-e851a265608e

📥 Commits

Reviewing files that changed from the base of the PR and between b221866 and f4f48c8.

📒 Files selected for processing (1)
  • ci-operator/step-registry/hypershift/review-agent/process/hypershift-review-agent-process-commands.sh

@bryan-cox

Copy link
Copy Markdown
Member Author

/retest

@enxebre

enxebre commented Apr 21, 2026

Copy link
Copy Markdown
Member

/lgtm

@openshift-ci openshift-ci Bot added the lgtm Indicates that a PR is ready to be merged. label Apr 21, 2026
@openshift-ci

openshift-ci Bot commented Apr 21, 2026

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: bryan-cox, enxebre

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@bryan-cox

Copy link
Copy Markdown
Member Author

/pj-rehearse ack

@openshift-merge-bot

Copy link
Copy Markdown
Contributor

@bryan-cox: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel.

@openshift-merge-bot openshift-merge-bot Bot added the rehearsals-ack Signifies that rehearsal jobs have been acknowledged label Apr 21, 2026
@openshift-ci

openshift-ci Bot commented Apr 21, 2026

Copy link
Copy Markdown
Contributor

@bryan-cox: all tests passed!

Full PR test history. Your PR dashboard.

Details

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 kubernetes-sigs/prow repository. I understand the commands that are listed here.

@openshift-merge-bot openshift-merge-bot Bot merged commit da6b0a4 into openshift:main Apr 21, 2026
10 checks passed
Prucek pushed a commit to Prucek/release that referenced this pull request Apr 29, 2026
…lies (openshift#78042)

Review body replies were formatted as "Re: review PRR_kwDOE7ekcc72sY6X"
which is meaningless to humans. Switch to using the review URL so replies
show a clickable link instead. Duplicate detection checks for both the
old node ID and new URL format for backward compatibility.

Co-authored-by: Claude Opus 4.6 <[email protected]>
BATMAN-JD pushed a commit to BATMAN-JD/release that referenced this pull request May 1, 2026
…lies (openshift#78042)

Review body replies were formatted as "Re: review PRR_kwDOE7ekcc72sY6X"
which is meaningless to humans. Switch to using the review URL so replies
show a clickable link instead. Duplicate detection checks for both the
old node ID and new URL format for backward compatibility.

Co-authored-by: Claude Opus 4.6 <[email protected]>
fjglira pushed a commit to fjglira/release that referenced this pull request Jun 3, 2026
…lies (openshift#78042)

Review body replies were formatted as "Re: review PRR_kwDOE7ekcc72sY6X"
which is meaningless to humans. Switch to using the review URL so replies
show a clickable link instead. Duplicate detection checks for both the
old node ID and new URL format for backward compatibility.

Co-authored-by: Claude Opus 4.6 <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. rehearsals-ack Signifies that rehearsal jobs have been acknowledged

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants