-
Notifications
You must be signed in to change notification settings - Fork 22
feat(prompt): Enhance assistant persona and user experience #208
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
base: main
Are you sure you want to change the base?
Conversation
WalkthroughUpdated the Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
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 |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: eranco74 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 |
|
/hold |
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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
template.yaml (2)
434-435: Parameter interpolation bug: use ${VAR}, not ${{VAR}}.OpenShift Templates substitute ${PARAM}. ${{…}} will not resolve, breaking replicas and ports and potentially failing probes and Service wiring.
- replicas: ${{REPLICAS_COUNT}} + replicas: ${REPLICAS_COUNT} ... - containerPort: ${{SERVICE_PORT}} + containerPort: ${SERVICE_PORT} ... - port: ${{SERVICE_PORT}} + port: ${SERVICE_PORT} ... - port: ${{SERVICE_PORT}} + port: ${SERVICE_PORT} ... - port: ${{SERVICE_PORT}} - targetPort: ${{SERVICE_PORT}} + port: ${SERVICE_PORT} + targetPort: ${SERVICE_PORT}Also applies to: 450-452, 514-515, 521-522, 602-603
599-606: Headless Service prevents Route from working.Routes require a Service with a ClusterIP.
clusterIP: Nonemakes it headless and typically unroutable.- spec: - clusterIP: None + spec:
🧹 Nitpick comments (4)
template.yaml (4)
238-241: Add a privacy/safety caveat when displaying lists.Explicitly note that lists must not expose sensitive links (e.g., ISO/kubeconfig URLs) even for short lists.
**When a tool provides a list of items, your primary response is to present the information clearly.** * **For short lists (e.g., 10 items or fewer),** present the complete list directly to the user. - * **For long lists (over 10 items),** present a summary (e.g., "I found 25 events, with 3 critical errors.") and then offer to display the full list, filter it, or show the most recent/critical items. + * **For long lists (over 10 items),** present a summary (e.g., "I found 25 events, with 3 critical errors.") and then offer to display the full list, filter it, or show the most recent/critical items. + * If a list contains sensitive values (for example, Discovery ISO or kubeconfig URLs), summarize items without revealing the URLs.
274-279: Qualify hardware requirements as version‑dependent minimums and avoid over‑precision.These numbers vary by OpenShift release and operator set; hardcoding can mislead.
- * For a multi-node cluster: + * Minimums (vary by OpenShift version; operator choices may increase these): * Control plane (master) nodes require: 4 CPU cores, 16 GB RAM, 100 GB storage * Compute (worker) nodes require: 2 CPU cores, 8 GB RAM, 100 GB storage - * For a single node cluster (SNO): 8 CPU cores, 16 GB RAM, 100 GB storage + * For a single node cluster (SNO): 8 CPU cores, 16–32 GB RAM (recommended 32 GB), 100 GB storage
300-301: Troubleshooting flow tweak (optional).Recommend “events first, then logs” to minimize heavy downloads, and prompt for cluster/host IDs if ambiguous.
131-136: Typo in parameter name: LIGHTSSPEED → LIGHTSPEED.Spelling inconsistency invites future misconfiguration. Consider renaming the parameter and its reference (breaking change) or add a migration note.
-- name: LIGHTSSPEED_STACK_POSTGRES_SSL_MODE +- name: LIGHTSPEED_STACK_POSTGRES_SSL_MODE value: "verify-full" description: "SSL mode for the PostgreSQL database connection used by lightspeed-stack" ... - ssl_mode: ${LIGHTSSPEED_STACK_POSTGRES_SSL_MODE} + ssl_mode: ${LIGHTSPEED_STACK_POSTGRES_SSL_MODE}Also applies to: 198-199
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
template.yaml(6 hunks)
🔇 Additional comments (4)
template.yaml (4)
209-209: Persona adjustments LGTM.Collaborative, empathetic tone guidance is clear and aligned with the PR goal.
296-296: Kubeconfig/kubeadmin URL non-disclosure LGTM.
310-311: Ambiguity-handling and evidence-based confirmation LGTM.
256-257: Narrow refusal of “static networking” — refuse only when tooling truly lacks support.
- Do not refuse static networking wholesale. Check whether available tooling in this repo (e.g., nmstate/MCP/assisted‑installer support) can satisfy node-level network parameters; if tooling cannot, explicitly state this is a tooling limitation and direct the user to the assisted‑installer web wizard.
- Only trigger refusal for explicit node-level overrides (static IPs, MAC addresses, gateways, DNS, or other node-specific network settings).
File: template.yaml lines 256-257
- * **If an unsupported configuration is detected, you must refuse the entire request.** Do not proceed with cluster creation. Your response must state that you don't support the requested static networking configuration and then instruct the user to use the assisted-installer web-based wizard instead. + * **If the requested configuration is not supported by available tools,** explain the limitation and guide the user to the assisted‑installer web wizard. Otherwise proceed.
19d6ded to
d82f42e
Compare
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)
206-206: Resolve tone guidance conflict; prefer “plain language, defined terms” over “avoid jargon.”Line 206 partly contradicts Line 205 (“use technical language, spell out abbreviations”). Tighten to “avoid unexplained jargon; define terms on first use” to remove ambiguity.
Apply:
- Be a helpful colleague. Get straight to the point but engage the user like a peer. Ask clarifying questions if you need more information to give a precise answer. For example, "What version are you using?" Give concrete advice, not vague platitudes. Use specific commands, examples, and numbers whenever possible. Present options. If there are multiple valid approaches, lay them out clearly. Don't overpromise. Be honest about limitations and what you can't do. Be conversational. Use contractions and avoid jargon. Empower the user. Position Red Hat's technology as an enabler for their success, not a replacement for their expertise. + Be a helpful colleague. Get straight to the point but engage the user like a peer. Ask clarifying questions if you need more information to give a precise answer (e.g., “What version are you using?”). Give concrete advice with specific commands, examples, and numbers whenever possible. Present options when multiple approaches are valid. Don’t overpromise—be honest about limitations. Use plain language and avoid unexplained jargon; define domain terms and abbreviations on first use. Empower the user—position Red Hat’s technology as an enabler for their success, not a replacement for their expertise.
236-238: Align list‑display rule wording with “don’t mention tools” policy; add safe default cap.Avoid “tool” wording in the instruction itself and set a default top‑N to prevent flooding.
Apply:
- **When a tool provides a list of items, your primary response is to present the information clearly.** + **When you need to present a list returned from internal operations, present the information clearly without mentioning internal tools.** * **For short lists (e.g., 10 items or fewer),** present the complete list directly to the user. - * **For long lists (over 10 items),** present a summary (e.g., "I found 25 events, with 3 critical errors.") and then offer to display the full list, filter it, or show the most recent/critical items. + * **For long lists (over 10 items),** present a one‑line summary (e.g., “I found 25 events; 3 critical.”), then show the top 10 most recent or critical items by default. Offer to display the full list, apply filters, or change the sort.
311-313: Great additions; also instruct to include evidence timestamps.This strengthens the “confirm with evidence” rule and improves auditability.
Apply:
- * **Confirm with Evidence: After executing an action (like creating a cluster or assigning a host role), always use the output from the tool to confirm its success or failure. Do not simply assume the action worked. For example, say "I have created the cluster 'my-cluster' with ID: xyz-123," not just "I have created the cluster."** + * **Confirm with Evidence: After executing an action (like creating a cluster or assigning a host role), always use the output to confirm success or failure. Do not assume it worked. For example: “I created the cluster ‘my-cluster’ (ID: xyz-123).” Include relevant timestamps or status versions where applicable.**
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
template.yaml(4 hunks)
🔇 Additional comments (1)
template.yaml (1)
271-275: Do not raise SNO minimum RAM — match Red Hat's documented minimums and label values as "minimums".Red Hat (OpenShift 4.18) lists SNO minimums as 8 vCPUs, 16 GB RAM, 120 GB storage. Update template.yaml (lines 271–275) to mark multi-node and SNO values as "minimums" and change SNO storage to 120 GB. If you want to advise more RAM for heavier workloads, add a separate "recommended" note (32 GB as recommended, not minimum).
Likely an incorrect or invalid review comment.
d82f42e to
35ffc97
Compare
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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
template.yaml (1)
201-340: Line 207 indentation is still incorrect—requires 1 additional space.Line 207 currently has 5 leading spaces but should have 6 to match the rest of the literal block. Change:
Be honest about limitations...to:
Be honest about limitations...This will resolve the yamllint syntax error:
expected <block end>, but found '<scalar>'
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
template.yaml(3 hunks)
🧰 Additional context used
🪛 YAMLlint (1.37.1)
template.yaml
[error] 207-207: syntax error: expected , but found ''
(syntax)
🔇 Additional comments (3)
template.yaml (3)
206-207: Persona guidance enhancements are clear and well-structured.Once the indentation error is fixed, the collaborative peer-like framing (concrete advice, asking clarifying questions, avoiding overpromising, and emphasizing honesty about limitations) strengthens the assistant's positioning. This aligns well with the PR objectives.
235-237: List output guidance provides clear UX improvement.The distinction between short lists (≤10 items, display in full) and long lists (>10 items, summarize with options) is practical and helps prevent information overload. This is a good addition to the prompt.
339-339: Evidence-based confirmation principle strengthens observability.Requiring the assistant to confirm actions using tool outputs (e.g., "I have created the cluster 'my-cluster' with ID: xyz-123" rather than generic success messages) improves trustworthiness and debuggability. Well-reasoned addition.
|
/retest |
This commit refines the system prompt to align the AI's behavior with the desired brand voice, moving from a purely functional assistant to a more collaborative and user-centric partner. The changes are based on benchmark examples that score well for being conversational, empowering, and context-aware. Key Improvements: - **Enrich Persona:** The core persona is updated to be more collaborative, professional, and empathetic, especially during troubleshooting scenarios. - **Improve UX for Lists:** The assistant now summarizes long lists (e.g., events) instead of displaying them in full, preventing user overload. - **Increase Robustness:** Confirm the success or failure of an actions based on evidence from tool outputs.
35ffc97 to
ab61fff
Compare
|
Keep failing with: /retest |
|
/retest |
1 similar comment
|
/retest |
|
2/3 test failed due to: Error message: Unexpected error in streaming query: [Errno 104] Connection reset by peer capabilities_scope_conv probably needs some adjustments in the expected response |
1b5cce0 to
ab61fff
Compare
|
@eranco74: 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. |
This commit refines the system prompt to align the AI's behavior with the desired brand voice, moving from a purely functional assistant to a more collaborative and user-centric partner. The changes are based on benchmark examples that score well for being conversational, empowering, and context-aware.
Key Improvements:
Summary by CodeRabbit