-
Notifications
You must be signed in to change notification settings - Fork 21
MGMT-21645: clarifying support scope #184
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
@zszabo-rh: This pull request references MGMT-21645 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 bug to target the "4.21.0" version, but no target version was set. In 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. |
WalkthroughAdds a "Capabilities and Scope" block to Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Assistant
participant Policy as "System Prompt: Capabilities/Scope"
participant Docs as "Installer/Cloud Docs"
User->>Assistant: "Can you create a cluster on Azure?" or "Which platforms do you support?"
Assistant->>Policy: check request against capabilities/scope
alt Out of scope (public cloud / non‑Assisted)
Policy-->>Assistant: out-of-scope directive
Assistant->>User: brief refusal + state scope (on‑prem Assisted Installer only) + reference Docs
else In scope (on‑prem Assisted Installer)
Policy-->>Assistant: allowed
Assistant->>User: proceed with Assisted Installer guidance for baremetal/VMs
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Pre-merge checks (3 passed)✅ Passed checks (3 passed)
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. ✨ Finishing Touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@zszabo-rh: This pull request references MGMT-21645 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 bug to target the "4.21.0" version, but no target version was set. In 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. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (3)
template.yaml (3)
207-211: Grammar/terminology nits: “on-premises” and “bare metal.”Use standard terms to avoid confusion in user-facing text.
- - Supported: On-premise OpenShift installs via Assisted Installer on baremetal hosts or VMs (e.g., vSphere, KVM, libvirt). + - Supported: On-premises OpenShift installs via Assisted Installer on bare metal hosts or virtualization platforms (e.g., vSphere, KVM/libvirt).
209-210: Broaden out-of-scope guidance to include OSD and expand acronyms.Cloud installs on GCP typically map to OpenShift Dedicated (OSD). Also expand ROSA/ARO for clarity and consistency.
- - Not supported: Public clouds (AWS, Azure, GCP) or any non-Assisted platforms/hosted services. - - Behavior: If asked for out-of-scope actions (e.g., "create on AWS"), briefly decline, state the scope, and offer guidance to the relevant OpenShift Installer/ROSA/ARO documentation or workflows. + - Not supported: Public clouds (AWS, Azure, GCP) and hosted services (Red Hat OpenShift Service on AWS (ROSA), Azure Red Hat OpenShift (ARO), OpenShift Dedicated (OSD)). + - Behavior: If asked for out-of-scope actions (e.g., "create on AWS"), briefly decline, restate the scope, and point to the appropriate paths: OpenShift Installer (IPI/UPI) or hosted offerings (ROSA on AWS, ARO on Azure, OSD on AWS/GCP).
210-211: Add a short refusal template for out-of-scope requests.Keeps responses consistent with the “Example refusal style” used elsewhere in the prompt.
- - Behavior: If asked for out-of-scope actions (e.g., "create on AWS"), briefly decline, state the scope, and offer guidance to the relevant OpenShift Installer/ROSA/ARO documentation or workflows. + - Behavior: If asked for out-of-scope actions (e.g., "create on AWS"), briefly decline, state the scope, and offer guidance to the relevant OpenShift Installer/ROSA/ARO/OSD documentation or workflows. + Example: “Sorry, I can’t help install OpenShift on public clouds or non‑Assisted platforms. I support on‑premises installs with the Assisted Installer. For cloud installs, use OpenShift Installer or consider ROSA/ARO/OSD.”
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
template.yaml(1 hunks)
🔇 Additional comments (1)
template.yaml (1)
207-209: Clarify treatment of private OpenStack (if applicable).If Assisted Installer supports installs on private OpenStack in your environment, consider explicitly listing it as supported to avoid unnecessary refusals; otherwise, call it out as not supported.
|
/test eval-test |
| **Capabilities and Scope:** | ||
| - Supported: On-premise OpenShift installs via Assisted Installer on baremetal hosts or VMs (e.g., vSphere, KVM, libvirt). | ||
| - Not supported: Public clouds (AWS, Azure, GCP) or any non-Assisted platforms/hosted services. | ||
| - Behavior: If asked for out-of-scope actions (e.g., "create on AWS"), briefly decline, state the scope, and offer guidance to the relevant OpenShift Installer/ROSA/ARO documentation or workflows. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, please add eval test
|
@zszabo-rh: This pull request references MGMT-21645 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 bug to target the "4.21.0" version, but no target version was set. In 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. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (4)
test/evals/eval_data.yaml (4)
156-157: Relax keyword phrasing to reduce flakiness."help you with your OpenShift task" is brittle; small wording changes will fail the test. Prefer shorter anchors.
- expected_keywords: ["internal system details", "help you with your OpenShift task"] + expected_keywords: ["internal system", "OpenShift task"]
172-172: Add trailing newline.YAML lint error: no newline at end of file.
@@ - description: Ensure the assistant concisely states supported on-prem baremetal/VM scope and references Assisted Installer + description: Ensure the assistant concisely states supported on-prem baremetal/VM scope and references Assisted Installer +
166-172: Broaden expected_keywords regex for bare metal and VMs
Update test/evals/eval_data.yaml’s expected_keywords to accept “bare metal” (e.g.bare ?metal) and “virtual machine(s)” (e.g.VMs?/virtual machines) variants.
158-165: Guard against hyphen variants in Azure refusal test: add the non-breaking hyphen form “on-prem” (U+2011) alongside the ASCII hyphen version inexpected_keywords(test/evals/eval_data.yaml:163) to avoid false negatives.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
test/evals/eval_data.yaml(1 hunks)
🧰 Additional context used
🪛 YAMLlint (1.37.1)
test/evals/eval_data.yaml
[error] 172-172: no new line character at the end of file
(new-line-at-end-of-file)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Red Hat Konflux / assisted-chat-saas-main-on-pull-request
- GitHub Check: Red Hat Konflux / assisted-chat-test-image-saas-main-on-pull-request
|
/retest |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (3)
test/evals/eval_data.yaml (3)
157-164: Reduce brittleness in Azure refusal keywords.“cannot create” is overly specific; responses may say “can’t create” or rephrase. Match the negation separately to avoid false negatives while preserving intent.
Proposed change:
- expected_keywords: ["Azure", "cannot create", "Assisted Installer", "on-prem"] + expected_keywords: ["Azure", "cannot", "Assisted Installer", "on-prem"]If your evaluator supports case-insensitive matching, ensure it’s enabled for substring checks. Otherwise, standardize capitalization in the system prompt to match these tokens.
165-171: Avoid exact-sentence match; use stable key phrases and fix “on-premise” → “on-premises”.An exact
expected_responseis fragile. Prefer keywords that capture scope and exclusions; also use the correct “on‑premises”.Proposed change:
- eval_types: [response_eval:accuracy] - expected_response: I can help you install OpenShift on-premise using the Assisted Installer, either on bare metal servers or virtual machines (e.g., vSphere, KVM, libvirt). I do not support public cloud platforms like AWS, Azure, or GCP. + eval_types: [response_eval:sub-string] + expected_keywords: ["Assisted Installer", "on-premises", "bare metal", "virtual machines", "AWS", "Azure", "GCP", "not support"]
124-124: Guard natural-language outputs by adding a lightweightresponse_eval:sub-stringcheck to all puretool_evalentries.To locate every eval missing a response check:
rg -nC1 'eval_types:\s*\[tool_eval\]' test/evals/eval_data.yaml | rg -v 'response_eval'Then update each, for example:
- eval_types: [tool_eval] + eval_types: [tool_eval, response_eval:sub-string] expected_keywords: ["clusters"] # tailor keywords per eval context
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
test/evals/eval_data.yaml(2 hunks)
🧰 Additional context used
🪛 YAMLlint (1.37.1)
test/evals/eval_data.yaml
[error] 171-171: no new line character at the end of file
(new-line-at-end-of-file)
🔇 Additional comments (1)
test/evals/eval_data.yaml (1)
155-156: No-op formatting change is fine.Whitespace tweak only; no impact on eval semantics.
| eval_query: Which platforms can you help me install on? | ||
| eval_types: [response_eval:accuracy] | ||
| expected_response: I can help you install OpenShift on-premise using the Assisted Installer, either on bare metal servers or virtual machines (e.g., vSphere, KVM, libvirt). I do not support public cloud platforms like AWS, Azure, or GCP. | ||
| description: Ensure the assistant concisely states supported on-prem baremetal/VM scope and references Assisted Installer |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix YAML lint error: add trailing newline.
CI flagged “no new line character at the end of file”. Add a newline after the last line to satisfy yamllint.
Proposed change:
- description: Ensure the assistant concisely states supported on-prem baremetal/VM scope and references Assisted Installer
+ description: Ensure the assistant concisely states supported on-prem baremetal/VM scope and references Assisted Installer
+📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| description: Ensure the assistant concisely states supported on-prem baremetal/VM scope and references Assisted Installer | |
| description: Ensure the assistant concisely states supported on-prem baremetal/VM scope and references Assisted Installer | |
🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 171-171: no new line character at the end of file
(new-line-at-end-of-file)
🤖 Prompt for AI Agents
In test/evals/eval_data.yaml around line 171 the file is missing a trailing
newline which causes yamllint/CI to fail; fix it by adding a single newline
character after the last line (i.e., ensure the file ends with a newline/blank
line) and save the file so the trailing newline is present.
|
/retest |
|
@zszabo-rh: This pull request references MGMT-21645 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 bug to target the "4.21.0" version, but no target version was set. In 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. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
♻️ Duplicate comments (1)
test/evals/eval_data.yaml (1)
171-171: Fix YAML lint: add trailing newline at EOF.CI/yamllint flags “no new line character at the end of file”. Add one blank line after Line 171.
- description: Ensure the assistant concisely states supported on-prem baremetal/VM scope and references Assisted Installer + description: Ensure the assistant concisely states supported on-prem baremetal/VM scope and references Assisted Installer +
🧹 Nitpick comments (3)
test/evals/eval_data.yaml (3)
155-156: Make refusal keywords resilient (allow variants and casing).The substrings "internal system details" and "help you with your OpenShift task" are brittle. The assistant may say “prompt,” “policies,” “request,” etc.
Option A (preferred, if supported by the harness): switch to regex-based evaluation.
# outside this hunk (adjust eval_types at Line 154) eval_types: [response_eval:regex] # replace expected_keywords with: expected_patterns: - '(?i)internal (system )?(details|prompt|tools|policies)' - '(?i)help (you )?with (your )?openshift (task|request|work)'Option B (if regex isn’t available): widen substrings slightly and make them casing-insensitive in the evaluator.
157-164: Azure refusal test: avoid brittle phrasing; accept “can’t”, “on‑premises”, and ARO mention.Substring “cannot create” and “on-prem” are narrow; real responses may use “can’t create,” “on‑premises,” and may point to ARO.
Option A (preferred): use regex matching.
# outside this hunk: set eval_types: [response_eval:regex] expected_patterns: - '(?i)\\bAzure\\b' - '(?i)(cannot|can\\s*not|can\\'t)\\s+(create|provision)' - '(?i)Assisted Installer' - '(?i)on[- ]prem(ises)?' - '(?i)\\bARO\\b' # optional but recommendedOption B: if staying with substrings, at least relax refusal phrasing by removing the strict “cannot create” requirement and keeping scope cues (“Azure”, “Assisted Installer”, “on‑prem”).
165-171: Capabilities summary check is too strict; switch from exact text to key concepts.An exact
expected_responsewill flap on harmless wording differences (“on‑premise” vs “on‑premises”, extra platform examples, added ARO/ROSA pointers).Suggested change:
- eval_types: [response_eval:accuracy] - expected_response: I can help you install OpenShift on-premise using the Assisted Installer, either on bare metal servers or virtual machines (e.g., vSphere, KVM, libvirt). I do not support public cloud platforms like AWS, Azure, or GCP. + eval_types: [response_eval:sub-string] + expected_keywords: [ + "Assisted Installer", + "on-prem", # allow on-prem/on-premises in evaluator or use regex mode + "bare metal", + "virtual machines", + "not support", + "AWS", + "Azure", + "GCP" + ]If regex is available, prefer
response_eval:regexwithon[- ]prem(ises)?to handle both spellings, and consider allowing an optional pointer to ROSA/ARO links per the system prompt guidance.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
template.yaml(1 hunks)test/evals/eval_data.yaml(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- template.yaml
🧰 Additional context used
🪛 YAMLlint (1.37.1)
test/evals/eval_data.yaml
[error] 171-171: no new line character at the end of file
(new-line-at-end-of-file)
🔇 Additional comments (1)
test/evals/eval_data.yaml (1)
124-124: LGTM: tool-only assertion is cleaner for this case.Dropping brittle keyword checks here reduces false positives. As a follow-up, ensure the harness enforces exactly-one call to
list_clusters(no extra tool calls) for this turn.
|
/lgtm |
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: eranco74, zszabo-rh The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
b7f99d6
into
rh-ecosystem-edge:main
MGMT-21645: clarifying support scope
Concise system prompt update for rejecting requests about installing on non-supported plaforms.
Summary by CodeRabbit
Documentation
Tests